Introduce build time flag property to disable the cache extension

[gwenneg]

Fixes #11138.

[geoand]

Minor nitpick: Can we call give this a more descriptive name, something like CacheEnabled or something?
That would make it more consistent with the the existing uses of onlyIf

[gwenneg]

That's a good point, I just applied your suggestion. Thanks for reviewing this!

[jaikiran]

Hello @gwenneg, if this config set to "disable this extension", then would adding this extension to the application's dependency (pom.xml for example) be of any value? In other words, can't the extension be disabled by just removing it from the application's dependency?

[geoand]

@jaikiran this kind of configuration usually proves helpful in testing scenarios

[jaikiran]

@jaikiran this kind of configuration usually proves helpful in testing scenarios

Hello @geoand, on the other hand, wouldn't this mean that every single build step method of every build step processor that corresponds to this extension will now have to add:

    @BuildStep(onlyIf = CacheEnabled.class)

Perhaps, if we do need something like this, maybe this should be some generic config which can then be implemented in the build step processor invocation layer where with a single check it can decide if the processor and its build steps are relevant or not depending on that enabled/disabled config?

[geoand]

Interesting idea @jaikiran.

Are you thinking something @BuildStep(onlyIfProperty ="some.value")?

[gwenneg]

Hello @gwenneg, if this config set to "disable this extension", then would adding this extension to the application's dependency (pom.xml for example) be of any value? In other words, can't the extension be disabled by just removing it from the application's dependency?

This PR started from a Quarkus user who needed to disable the extension temporarily for testing purposes. This cannot be done by removing the extension from the application pom.xml since it would also mean that all caching annotations must be removed from the source code.

[gwenneg]

@jaikiran this kind of configuration usually proves helpful in testing scenarios

Hello @geoand, on the other hand, wouldn't this mean that every single build step method of every build step processor that corresponds to this extension will now have to add:

    @BuildStep(onlyIf = CacheEnabled.class)

Perhaps, if we do need something like this, maybe this should be some generic config which can then be implemented in the build step processor invocation layer where with a single check it can decide if the processor and its build steps are relevant or not depending on that enabled/disabled config?

For this specific case, I didn't disable all build steps. The one providing the FeatureBuildItem instance is still active since the extension is still part of the application (even if it is disabled). Also, when the programmatic API of this extension will be there, some other build steps will certainly remain active while the annotations caching API might be disabled.

[gwenneg]

@geoand Now that I wrote the previous comment, I am wondering if the property included in this PR should contain a part indicating that it will disable the annotations only... Something like quarkus.cache.annotations.enabled or quarkus.cache.annotation-api.enabled.

[geoand]

@gwenneg I don't think that is necessary. Maybe just update the javadoc of the property to mention that the caching annotations will have no effect

[gwenneg]

@geoand I did a small CacheConfig Javadoc update to include your suggestion.

[geoand]

Looks good, thanks!

[gsmet]

I'm a bit torn on that one as I wonder if it would not make more sense to just disable the interceptors and keep the rest of the logic running.

But maybe it could be an issue if it was using an external cache service which we might support at some point.

Merging anyway.

[gwenneg]

@gsmet I also hesitated between disabling the entire extension logic at build time or disabling the interceptors logic only at run time. But since this PR started from a user who wanted to compare performances with and without the cache extension, disabling it at build time seemed more relevant to me. Anyway, this could easily be changed in the future.

Thanks for merging!