Extracts Caching Libraries into Service Loaders (Caffeine -> Java, Guava -> Android)

[vitorpamplona]

Background

Caffeine 3.0+ does not plan to support Android. The 3.0 version requires a java.lang.System.getLogger method from the JEP-264: Platform Logging API to redirect logs to slf4j. Unfortunately, Android has not provided a System.getLogger interface yet. This means a HAPI migration to Caffeine 3.0+ will simply crash in runtime on Android.

Solution

Abstract the Caching function, remove direct dependencies on Caffeine, and build an Android-friendly alternative.

1. This PR establishes a new Caching structure via ServiceLoaders in 4 new maven modules:

   • hapi-fhir-cache: Cache service loader interfaces and factory methods.
   • hapi-fhir-cache-guava: Delegator classes to Guava (ideal for Android)
   • hapi-fhir-cache-caffeine: Delegator classes to Caffeine (current implementation)
   • hapi-fhir-cache-testing: Utility module to quickly alternate between guava and caffeine and run the project's test cases on.

2. All modules dependent on caffeine now depend on hapi-fhir-cache directly. Users must choose a caching implementation by also importing hapi-fhir-cache-caffeine or hapi-fhir-cache-guava. If none is present, the following error message will be displayed: No Cache Service Providers found. Choose between hapi-fhir-cache-caffeine (Default) and hapi-fhir-cache-guava (Android)

3. hapi-fhir-cache-guava contains brand new Cache and LoadingCache implementations that mimic Caffeine's behavior completely and it's ideal for the use of HAPI on Android. In my tests, it fully replaces Caffeine for the needs of this project.

Closes: #2444, Helps: #3690

[codecov]

Codecov Report

Merging #3977 (739743a) into master (e7a4c49) will decrease coverage by 1.62%.
The diff coverage is n/a.

❗ Current head 739743a differs from pull request most recent head 1ead327. Consider uploading reports for the commit 1ead327 to get more accurate results

@@             Coverage Diff              @@
##             master    #3977      +/-   ##
============================================
- Coverage     81.86%   80.24%   -1.63%     
- Complexity    20838    22463    +1625     
============================================
  Files          1421     1438      +17     
  Lines         77246    83118    +5872     
  Branches      11078    11615     +537     
============================================
+ Hits          63236    66694    +3458     
- Misses         9866    11384    +1518     
- Partials       4144     5040     +896     

Impacted Files	Coverage Δ
...-fhir-base/src/main/java/ca/uhn/fhir/i18n/Msg.java
...n/java/ca/uhn/fhir/parser/ErrorHandlerAdapter.java
...e/src/main/java/ca/uhn/fhir/parser/JsonParser.java
...n/java/ca/uhn/fhir/parser/LenientErrorHandler.java
...ain/java/ca/uhn/fhir/parser/ParseErrorHandler.java
.../src/main/java/ca/uhn/fhir/parser/ParserState.java
...in/java/ca/uhn/fhir/parser/StrictErrorHandler.java
...ava/ca/uhn/fhir/parser/json/BaseJsonLikeArray.java
...va/ca/uhn/fhir/parser/json/BaseJsonLikeObject.java
...ava/ca/uhn/fhir/parser/json/BaseJsonLikeValue.java
... and 2840 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[ben-manes]

Just want to make sure this wasn't forgotten in the discussion. If the cache-api project fit your needs, it might be a good way for you and others to collaborate. It may not and I am not familiar with it or the developers, but an fyi in case that avoids duplication of efforts.

[vitorpamplona]

Thank you, Ben! I didn't know the project existed. Maybe we can collaborate with them in the future.

At a first glance, it does a similar job. However, the cache-api does not standardize the interface among all implementations and it doesn't offer a provider for the LoadingCache class. This makes testing for all implementations really difficult. There are behavioral differences between Caffeine and Guava that need to be sorted out by the Delegator. You can see them in the Cache.get method in the Guava module that it:

1. Has to wrap ExecutionExeptions as RuntimeExeptions
2. Has to unwrap UncheckedExecutionException to expose the original Exception
3. Has to not throw exceptions when the CacheLoader returns null

These are needed to completely match Caffeine's API.

The CacheDelegator ends up looking like this:

try {
	return cache.get(key, () -> mappingFunction.apply(key));
} catch (ExecutionException e) {
	e.printStackTrace();
	throw new RuntimeException(e);
} catch (UncheckedExecutionException e) {
	if (e.getCause() instanceof RuntimeException) {
		// Unwrap exception to match Caffeine
		throw (RuntimeException)e.getCause();
	}
	throw e;
} catch (CacheLoader.InvalidCacheLoadException e) {
	// If the entry is not found or load as null, returns null instead of an exception
	// This matches the behaviour of Caffeine
	return null;
}

[ben-manes]

It might be unsatisfactory, but would using Caffeine's Guava Cache adapters fit your goals? That attempts to make Caffeine conform to Guava, with similar workarounds to your snippet. Just trying to make sure you have all the options to consider, and whatever the answer will be the best choice for your project.

[vitorpamplona]

I did see those components (thank you for making them) but I discarded that approach because it adds significant complexity to the exception handling on the HAPI side. We could go there, but it wouldn't be a simple 3hr PR by then.

HAPI was clearly coded with Caffeine's API in mind. And I do believe that, after looking at both, Caffeine's API is superior for users.

[jamesagnew]

@vitorpamplona - I want to get this merged in time for the 6.2.0 release. I've pulled these changes into a fresh PR that I'll be submitting to our internal review process tomorrow: #4196

[vitorpamplona]

Nice! Yes, let's go! :)

[jamesagnew]

@vitorpamplona - My copy of your PR has now merged. Thanks again for the contribution!

A couple of notes:

• We didn't make the feature freeze cutoff for 6.2.0 unfortunately, so this will be released in 6.3.0. It'll be in any SNAPSHOT builds going forward though.
• I renamed the parent module (the one containing the projects you created) to hapi-fhir-serviceloaders. I figure this may well not be the last one of these, so might as well group them.
• This is perhaps the most contentious part - I decided to make the hapi-fhir-caching-caffeine module be a default module dependency for the JPA and Validation modules. I hate to make breaking changes and without that change this is a breaking change. My hope is that you can just exclude it in your pom/gradle file from the android library. Does that make sense?
• You are of course credited in the changelog for the feature.

[vitorpamplona]

Thank you, James!

[vitorpamplona]

Hi @jamesagnew

This is perhaps the most contentious part - I decided to make the hapi-fhir-caching-caffeine module be a default module dependency for the JPA and Validation modules. I hate to make breaking changes and without that change this is a breaking change. My hope is that you can just exclude it in your pom/gradle file from the android library. Does that make sense?

Is there any chance HAPI can avoid using caffeine as default? Today every Android developer hits a roadblock when trying to use HAPI because they need to figure out that they must manually remove caffeine and add guava in their build files.

The current dependency stack doesn't provide a great dev experience for Android. :(

CC @jingtang10 @brynrhodes @JPercival

[jamesagnew]

@vitorpamplona I thought we had? What is pulling it in?

[vitorpamplona]

@vitorpamplona I thought we had? What is pulling it in?

The Validation Module :(

hapi-fhir/hapi-fhir-validation/pom.xml

Line 37 in 2e4f8fe

<artifactId>hapi-fhir-caching-caffeine</artifactId>

.

[jamesagnew]

@vitorpamplona pushing through a fix in #5472

[vitorpamplona]

Thank you, James!