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

Using custom config source for JCache #312

Closed
cen1 opened this issue Apr 9, 2019 · 23 comments
Closed

Using custom config source for JCache #312

cen1 opened this issue Apr 9, 2019 · 23 comments

Comments

@cen1
Copy link

@cen1 cen1 commented Apr 9, 2019

Working setup: microprofile framework, jcache-api, cache-annotations-ri-cdi, caffeine and typesafe config file.

We are using microprofile-config style configuration files and I would preferrably like to eliminate the typesafe's application.conf and instead read the JCache config from our own yaml file. Writing a mapper on framework bootstrap to do the conversion to com.typesafe.config.Config is easy enough but I am not sure how to programatically set this new config instance to caffeine.

Any pointer in the right direction is welcome.

@ben-manes

This comment has been minimized.

Copy link
Owner

@ben-manes ben-manes commented Apr 9, 2019

If you are okay with creating all caches upfront, you can use createCache(name, configuration) with CaffeineConfiguration. The lazy resolution in CacheManagerImpl is tied to CacheFactory, which resolves using Typesafe config (see TypesafeConfigurator). We could potentially make CacheFactory use a pluggable source if you wanted to replace it, as currently it is a private implementation detail.

You might also consider using the library's system properties, e.g. config.file to override the default location to load from. This might let you generate an application.conf at runtime.

@cen1

This comment has been minimized.

Copy link
Author

@cen1 cen1 commented Apr 10, 2019

Thanks, I'll leave this open for now if I encounter any problems along the way.

@ben-manes

This comment has been minimized.

Copy link
Owner

@ben-manes ben-manes commented Aug 4, 2019

@cen1 any feedback on this? Are there changes we should invest in, close this out, or...?

@cen1

This comment has been minimized.

Copy link
Author

@cen1 cen1 commented Aug 4, 2019

I was very low on time so I haven't been able to start with this yet but I still plan to. I will reopen the issue if I encounter any problems when I come around to it. At least based on the first response, I don't expect any.

@cen1 cen1 closed this Aug 4, 2019
@cen1

This comment has been minimized.

Copy link
Author

@cen1 cen1 commented Sep 2, 2019

@ben-manes Creating a new Config instance from custom source and merging it with default Config was easy enough.

Config defaultConfig = ConfigFactory.load();
Config customConfig = ConfigFactory.parseString(myCustomConfigJson);
return customConfig.withFallback(defaultConfig);

Now I am missing the part where I could give this Config instance for caffeine to use.

Looks like one way would be to write my own provider, extend and override CaffeineCachingProvider#getCacheManager using the longer constructor for CacheManagerImpl instead and passing my own config there.

Finally, I would have to add my own META-INF.services with my own provider impl but also need to somehow disable the default service provided by caffeine (is this possible)?

If you have some ideas how to tackle this the best way I'll gladly hear it.

@cen1 cen1 reopened this Sep 2, 2019
@ben-manes

This comment has been minimized.

Copy link
Owner

@ben-manes ben-manes commented Sep 2, 2019

JCache is static and service-loader based, and is actually quite hostile to DI. The classic JDK trick to replace reference implementations is to use a system property (e.g. how javax.xml works). If we introduce a new interface, CacheConfigurator, then a system property could override from the default of TypesafeConfigurator with the class's FQN. If set then the custom configurator is instantiated (probably assuming an empty constructor) and used by CacheFactory.

I think that would be the most natural and least invasive approach, with only a little refactoring needed to introduce a CacheConfigurator interface. What do you think?

@cen1

This comment has been minimized.

Copy link
Author

@cen1 cen1 commented Sep 2, 2019

One thing I still need to understand is what is the trigger for jcache initialization (what triggers the service loading). Because I would still need to do two things from framework's main() which need to be executed before jcache initializes:

  1. Set this system property programatically (I would prefer to avoid run args if possible)
  2. Initialize the framework's config instance so I can use it in the CacheConfigurator impl.

I couldn't find anything but I speculate the cache init is done by some kind of CDI observer? If that is the case, I guess the answer is: before CDI component is loaded by the framework.

As for the CacheConfigurator interface, I think that is perhaps too much, since the reimplementation would then need to be kept in sync with upstream. I think staying on Typesafe as the only source would be good enough first step and instead, what you could override with the system property is an implementation class that simply returns a Config. Default impl. would have the existing return ConfigFactory.load() while custom implementations could do whatever they want but still return typesafe Config in the end. At least based on my previous comment, it seems that even from custom config sources, producing a typesafe Config is relatively simple.

@ben-manes

This comment has been minimized.

Copy link
Owner

@ben-manes ben-manes commented Sep 2, 2019

One thing I still need to understand is what is the trigger for jcache initialization (what triggers the service loading).

Take a look at Caching from the JCache api jar. It is all done via statics and service loaders.

@cen1

This comment has been minimized.

Copy link
Author

@cen1 cen1 commented Sep 2, 2019

Sorry, I thought there is some kind of init going on but apparently the answer is "whenever the Caching methods are explicitely called" or in the case I am specifically interested in (cache-annoations-ri-cdi) it appears to be when an interceptor is triggered. So nothing really gets called until cache is triggered for the first time and I should be all good.

Whatever you decide the config abstraction in caffeine ends up being I'll try to implement it and give feedback.

@ben-manes

This comment has been minimized.

Copy link
Owner

@ben-manes ben-manes commented Sep 2, 2019

Would a static method on TypesafeConfigurator to specify the configuration loader (Supplier) be ideal then?

@cen1

This comment has been minimized.

Copy link
Author

@cen1 cen1 commented Sep 3, 2019

For my use case yes, that would be ideal.

@cen1

This comment has been minimized.

Copy link
Author

@cen1 cen1 commented Sep 12, 2019

I hacked together a simple patch cen1@0eae3ec probably not exactly what you had in mind but just to get me going.

POC also successfully tested.

@ben-manes

This comment has been minimized.

Copy link
Owner

@ben-manes ben-manes commented Sep 12, 2019

Pretty close, sorry I haven't had time for this. The main difference was to use Java 8's Supplier<Config> and default the static field using ConfigFactory::load.

@cen1

This comment has been minimized.

Copy link
Author

@cen1 cen1 commented Sep 12, 2019

I am in no particular hurry so if I can get something similar at least as a -SNAPSHOT to depend on in a month or so that would be cool.

@ben-manes ben-manes closed this in 92c8d59 Sep 29, 2019
@ben-manes

This comment has been minimized.

Copy link
Owner

@ben-manes ben-manes commented Sep 29, 2019

The only difference from your patch is the method is named setConfigSource.

@cen1

This comment has been minimized.

Copy link
Author

@cen1 cen1 commented Nov 22, 2019

@ben-manes I have published kumuluzee-jcache as a snapshot preview. Are there any near term plans for 2.8.1 or 2.9 release of caffeine?

@ben-manes

This comment has been minimized.

Copy link
Owner

@ben-manes ben-manes commented Nov 22, 2019

Oh, I forgot this didn't make it into 2.8. Thanks for being patient. I can probably cut a release for you over the weekend. There would be nothing else of interest going out, though.

@cen1

This comment has been minimized.

Copy link
Author

@cen1 cen1 commented Nov 22, 2019

That would be great.

@cen1

This comment has been minimized.

Copy link
Author

@cen1 cen1 commented Jan 7, 2020

@ben-manes just a small reminder to get a release done.

@ben-manes

This comment has been minimized.

Copy link
Owner

@ben-manes ben-manes commented Jan 7, 2020

oh! sorry for forgetting this (again). I'll try to release tonight.

@ben-manes

This comment has been minimized.

Copy link
Owner

@ben-manes ben-manes commented Jan 8, 2020

Sorry, I need to rewrite the release process to use the maven-publish-plugin. Currently it uses the older maven-plugin via the nexus-plugin, which inspired the DSL for maven-publish-plugin. Unfortunately with Gradle's deprecations and an upgraded GPG migrating away from short key ids, I can't sign the release.

I'll have to rewrite publish.gradle to use the newer approach. I gave that a shot briefly but it is a little painful, so it will take some dedicated time to relearn this process.

@ben-manes

This comment has been minimized.

Copy link
Owner

@ben-manes ben-manes commented Jan 14, 2020

I think the root problem was that gpg2 upgrade dropped my old key when it rewrote the secring.gpg file. Even though it shows up in gpg -k, it doesn't get exported by ~/.gnupg/secring.gpg since it is insecure.

I only figured that out after mostly getting the maven-publish integration working and running into the same problem. By switching to a new key, but still required to use the short id, I was able to run publishToMavenLocal. I just need to add the testJar back and do a release dry run. Hopefully I'll have this worked out soon since I'm doing this during my off hours.

@ben-manes

This comment has been minimized.

Copy link
Owner

@ben-manes ben-manes commented Jan 15, 2020

Released 2.8.1. Sorry for the delays!

I think it takes ~20 min to promote from their staging to central repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.