Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ICache instances and their initialization #36

Open
vvdb-architecture opened this issue Oct 7, 2022 · 0 comments
Open

ICache instances and their initialization #36

vvdb-architecture opened this issue Oct 7, 2022 · 0 comments

Comments

@vvdb-architecture
Copy link
Contributor

vvdb-architecture commented Oct 7, 2022

Current implementations of ICache include:

The first 4 are transient implementations, the last two are shared (singleton) implementations. This is important in the discussion below.

ICache contains this method:

      /// <summary>
      /// Initialize the cache. This must be called once ervery time a cache is created.
      /// </summary>
      /// <param name="store">The store name for identify the cache used.</param>
        void Initialize(string store);

This relies on the ICache client to do the right thing, i.e. calling the method exactly once before calling any other method.
It also forces the implementer of an ICache type for every method to verify if the cache has been property initialized. This is wasteful overhead but necessary just because an ICache instance can be created in an uninitialized state.

In fact, the implementer can forget to check the state, which sometimes gives strange behaviors.
If you forget to call Initialize, some methods will silently do nothing (giving you the impression that everything works), and other methods fail immedately.
For example, in DaprCache, if you forget to call Initialize, calling GetAsync will return null for every call and calling Get will fail immediately with a null reference exception!

It would be better to remove the method and put it in a separate interface:

interface ICacheInitializer
{
  void Initialize(string store);
}

All existing transient implementations (see above) would implement this interface.

To make sure that the users don't access the cache instance directly, the transient instances (so everything except SecureCache and ServerPrincipalCache) need to be removed from dependency injection. Instead, the only accessible interface via DI would be some ICacheFactory interface which would only issue properly initialized ICache instances:

interface ICacheFactory
{
  ICache CreateInstance(string cacheType, string store);
  void Register<T>(string cacheTpe) where T: ICache, ICacheInitializer
}

Note that this is a factory pattern, not a service locator pattern. Relax, Jan 😄.

For the cacheType, we would use the same names used now in the DI case.

The cache factory will be a singleton object. It will need to be populated at startup with the appropriate creation methods for each type of store. The Register method registers the concrete transient cache types the system knows about, and allows for dynamic extension to new cache types. If we don't allow registration beyond the startup phase, we can separate this method as well, but I don't think it's a requirement.

The cache instances returned by CreateInstance would be correctly initialized objects by construction. The Initialize method in ICache has disappeared, as well as the corresponding checks in every ICache method implementation.

For shared ICache implementations, like SecureCache and ServerPrincipalCache, there is no problem. In the former case, the code in Initialize can be performed in the constructor since it's just creating a dictionary. In the latter case, the Initialize method is empty. In both cases, there is no need to make them part of a cache factory since they are singletons. The code there simplifies as well.

This can probably simplify CacheContext, which seems to be some kind of holding object for transient instances of cache implementations, even though CacheContext itself is a shared object, so the cached instances also become shared when accessed through the CacheContext, but transient when accessed through their DI name. The reasons for this design is not well understood.

The actual implementation of the cache factory is left as an exercise for the reader 😀 .

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

No branches or pull requests

1 participant