-
Notifications
You must be signed in to change notification settings - Fork 0
Description
Api.GetModelAsync is an asynchronous operation that asynchronously builds a model. This can potentially be an operation that takes a little time, with possibly I/O which makes it a good candidate to execute it asynchronously.
Once the model is Built, It's cached internally in the API method and added to the DI container.
This creates multiple problems:
-
Not every method in the project is asynchronous. This creates problems when trying to execute Api.GetModelAsync inside a synchronous method. Calling api.GetModelAsync().Result (or GetAwaiter().GetResult() will block the calling context and potentially cause a deadlock. This is done a few times in the code base. It's beginners mistake nr Remove ApplyTo methods and service locator antipattern from Microsoft.Restier.Core.Conventions. #1 with async code.
-
Multiple methods call GetService() and other methods call api.GetModelAsync. Now we may hope that the service registration for IEdmModel calls api.GetModelAsync, (and the default implementation does) but if it does not, we have different models in different places.
A bigger problem is that we cannot add multiple IEdmModel instances anymore into the DI container, so now we need multiple containers, one for each ApiBase instance. IF you put both instances in a single container you get random (or defined by the DI implementation) behaviour. And GetService() != api.GetModelAsync anymore. What a mess. -
You hide the fact that model building takes time behind GetService() if the DI supports lazy initialization and can create blocking code like this.
To counter this I propose a simple solution:
- Create an ApiBase (derived) instance.
- Asynchronously get (or refresh) the Model.
- Store the model on a property of ApiBase (or configuration class).
- Register it in the DI, or store it somewhere for the duration of your app in case you do not use DI.
- Use the cached model EVERYWHERE by simply getting it from the property on ApiBase or ApiBase.ApiConfiguration.
This is way better than the SO COOL lazy initialization of the model, which will bite you the way it's implemented now. If you want lazy initialization you can always do that through DI as most DI frameworks can delay instantiation of a dependency until it's necessary and can execute a delegate that calls the GetModelAsync (GenerateModelAsync would be a better name) after instantiation but before it's injected as a dependency.