Part of Constructor Injection for the DI pattern. #645
Conversation
|
|
| /// <summary> | ||
| /// Interface to implement by Api classes to get the context. | ||
| /// </summary> | ||
| public interface IDbContextProvider |
There was a problem hiding this comment.
Interfaces are concepts that imply a contract that will be available over an extended period of time. I do not anticipate us supporting this contract in v2, so I don't think we should include it here. If you could please undo the changes in this commit, we can talk later about what this would look like in v2. Thanks!
There was a problem hiding this comment.
Don't think it's ideal either. However, without an interface, it's impossible to get the DB context as EntityFrameworkApi is generic and you don't know the concrete type. I'm trying to get rid of the "pass the Service provider around and resolve from it everywhere".
There is logic to an interface: If the ApiBase is an EntityFrameworkApi instance, it must be able to provide a DB context. This will be true now, but for every version in the future. We can discuss about whether it should be a method or property, but you'll need a DB Context in every Filter or Authorizer method you add to your EntityFrameworkApi descendant.
There was a problem hiding this comment.
I had a more long-winded answer for this, but the short version is that DbContext already implements an interface, and I think we can register the context differently to achieve the same goals without introducing an unnecessary interface. We shouldn't be introducing any new contracts at this stage, we're too close to shipping. If you can back this out for now, we can talk about if we're going to fix it before we ship V1.
There was a problem hiding this comment.
Done. This means that I will have to find a different solution to GetApiService for the DBContext. Or we postpone this until V2.
…ces now need to be resolved by the DI container on creation of an ApiBase descendant, you must register implementations.
anywhere. References #5
…RequestItem. References #21
…guments are supplied.
References #19
This reverts commit 39e5208.
- Making DbContext public again (EasyAF uses it extensively) - Make the sample app run again.
* Removed ApplyTo methods and using Microsoft.Extensions.DependencyInjection statements from Conventions classes. Fixes #1 * Made conventionbased classes public. Fixes jspuij#2. * Moved extensions methods into the Invocation context. References jspuij#5. * Remove enumeration GetApiService as it isn't used anywhere. References jspuij#5 * Created constructor parameters for DefaultSubmitHandler. Closes jspuij#6. * Fixed unit tests. As IChangeSetInitializer and ISubmitExecutor instances now need to be resolved by the DI container on creation of an ApiBase descendant, you must register implementations. * Made ServiceProvider private. Fixes jspuij#9. * Added a reference to an ApiBase instance in InvocationContext. References jspuij#10 * Replaced DI calls to ApiBase with a reference. Fixes jspuij#10 * Replaced a few GetApiService calls with constructor injection. References: jspuij#16 * Get the model from the API using GetModelAsync. References jspuij#16. * Added constructor arguments to DefaultQueryHandler. References jspuij#17 * Fixed unit tests. earlier dependency checks. References jspuij#17 * Changed ApiBase to a constructor dependency for RestierBatchChangeSetRequestItem. References jspuij#21 * Replaced Service location on Restier Serializers with constructor arguments. Fixes jspuij#22. * Replaced IServiceProvider with IEdmModel in EdmHelpers. Fixes jspuij#23. * Move ApplyTo To ServiceCollectionExtensions. Fixes jspuij#25 * Moved RestierOperationModelBuilder.ApplyTo to ServiceCollectionExtensions. Fixes jspuij#26. * Removed dependency on Microsoft.Extensions.DependencyInection. References jspuij#27 * Creates constructor arguments for RestierController. References jspuij#27. * Introduced interface for DbContext. References jspuij#19 * Added await to pre- and postevents. Fixes jspuij#15. * Don't get the provider from the request message if all constructor arguments are supplied. * Revert "Introduced interface for DbContext." This reverts commit 39e5208. * Replace IEdmModel lookup with constructor argument. Fixes jspuij#32. * - Updating to latest OData packages - Adding Authenticode signing - Removing legacy test projects. * More project cleanup. * Adding global.json * Strong name signing (#647) * Attempt to get strong name signing working Gonna need to call in the Cavalry for this. * Strong name key signing (unfortunately breaks unit tests). * Updating to the new logo system to fix the unnecessary iconurl deprecation. * Update Breakdance to a strong-named version so that the tests compile again. * Removed ApplyTo methods and using Microsoft.Extensions.DependencyInjection statements from Conventions classes. Fixes #1 * Made conventionbased classes public. Fixes jspuij#2. * Moved extensions methods into the Invocation context. References jspuij#5. * Remove enumeration GetApiService as it isn't used anywhere. References jspuij#5 * Created constructor parameters for DefaultSubmitHandler. Closes jspuij#6. * Fixed unit tests. As IChangeSetInitializer and ISubmitExecutor instances now need to be resolved by the DI container on creation of an ApiBase descendant, you must register implementations. * Made ServiceProvider private. Fixes jspuij#9. * Added a reference to an ApiBase instance in InvocationContext. References jspuij#10 * Replaced DI calls to ApiBase with a reference. Fixes jspuij#10 * Replaced a few GetApiService calls with constructor injection. References: jspuij#16 * Get the model from the API using GetModelAsync. References jspuij#16. * Added constructor arguments to DefaultQueryHandler. References jspuij#17 * Fixed unit tests. earlier dependency checks. References jspuij#17 * Changed ApiBase to a constructor dependency for RestierBatchChangeSetRequestItem. References jspuij#21 * Replaced Service location on Restier Serializers with constructor arguments. Fixes jspuij#22. * Replaced IServiceProvider with IEdmModel in EdmHelpers. Fixes jspuij#23. * Move ApplyTo To ServiceCollectionExtensions. Fixes jspuij#25 * Moved RestierOperationModelBuilder.ApplyTo to ServiceCollectionExtensions. Fixes jspuij#26. * Removed dependency on Microsoft.Extensions.DependencyInection. References jspuij#27 * Creates constructor arguments for RestierController. References jspuij#27. * Introduced interface for DbContext. References jspuij#19 * Added await to pre- and postevents. Fixes jspuij#15. * Don't get the provider from the request message if all constructor arguments are supplied. * Revert "Introduced interface for DbContext." This reverts commit 39e5208. * Replace IEdmModel lookup with constructor argument. Fixes jspuij#32. * PR Adjustments - Making DbContext public again (EasyAF uses it extensively) - Make the sample app run again.
Issues
This pull request fixes part of #629.
Description
These are the quick wins that could be done without major discussion about redesign of certain classes. It also does not change the public API of Restier. All unit tests still pass.
The following sub-issues of #629 are fixed by this PR:
jspuij#1
jspuij#5
jspuij#6
jspuij#9
jspuij#10
jspuij#16
jspuij#17
jspuij#21
jspuij#22
jspuij#23
jspuij#25
jspuij#26
jspuij#27 (Partial, needs a bit of discussion).
Checklist (Uncheck if it is not completed)