Skip to content

Support existing document store and index creation behind toggle#10

Open
lahma wants to merge 1 commit intoravendb:masterfrom
lahma:document-store-from-services
Open

Support existing document store and index creation behind toggle#10
lahma wants to merge 1 commit intoravendb:masterfrom
lahma:document-store-from-services

Conversation

@lahma
Copy link
Contributor

@lahma lahma commented Mar 24, 2022

This PR adds support to reuse an existing document store from service proivder (we configure it globally and share it with other services too) instead of creating a new document store.

Also adding a toggle whether indexes should be deployed automatically - we have custom migrations (RavenMigrations) and the process also includes refreshing the indexes. We don't want indexes to refresh outside of deployment process.

services.AddSingleton(provider => new ConfigurationDocumentStoreHolder(options));

services.AddSingleton(provider => options.ResolveDocumentStoreFromServices
? new ConfigurationDocumentStoreHolder(provider.GetRequiredService<IDocumentStore>(), options)
Copy link
Collaborator

@lukaszdobrzynski lukaszdobrzynski Mar 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a few issues with this approach.
Firstly, it introduces redundant complexity, which in turn requires additional validation. What if ResolveDocumentStoreFromServices is true and the IDocumentStore has not been added to the services container?
Secondly, a user might want to provide the library with their own IDocumentStore which is not necessarily resolved by the DI mechanism directly. For example, it might be a property existing on some other object. This is the pattern that we often apply in our internal projects at HibernatingRhinos. In that case we would need another option pointing to where the library should look for the IDocumentStore object.

How about we add at least two more IIdentityServerBuilder extension methods? One of them accepting IDocumentStore object and another one taking a Func<IDocumentStore> provider as a parameter. That way we would let users inject their DocumentStores wherever they want from.

@lukaszdobrzynski
Copy link
Collaborator

@lahma Thank you for drawing out attention to the IDocumentStore issue. Generally speaking, I belive you're right and we should add support for reusing an existing document store. Please read my comment and let's find an optimal solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants