Fixed class loader PriorityAwareServiceProvider unable to find all SPI classes.

[bpasson]

The PriorityAwareServiceProvider, the default service provider for Moneta uses the class loader in which the Money class was loaded for finding additional SPI classes using ServiceLoader.load(serviceType, Monetary.class.getClassLoader()). This breaks in environments where SPI classes are not loaded in the same class loader as the libraries. E.g. when using Quarkus in dev-mode. For a reproducer see https://github.com/bpasson/quarkus-class-loading.

Why the choice for the more specific ServiceLoader.load(serviceType, classLoader) with a class loader parameter over ServiceLoader.load(serviceType), which uses the Thread.currentThread().getContextClassLoader()?

Needs #354

[keilw]

Would you be able to propose a PR for the mentioned approach?

[bpasson]

@keilw Sure I can create a PR to change the behavior. One question though about the caching. The service provider now uses caching. If it loaded some type once, it will never look for new ones if classes get reloaded dynamically. Is there a reason behind this logic or can it safely be removed.

[keilw]

@bpasson Maybe for performance reasons, but if you don't see a possible performance penalty, you could try without the caching.

[bpasson]

@keilw Anything special I need to configure when building? Running mvn clean verify on master hangs somewhere in running tests and has numerous failing tests. Using Java 11 and maven 3.8 for building.

[keilw]

This may be blocked by other issues especially the one around that whole config problem, see #370. @bpasson did you try building without certain modules like ECB?

[bpasson]

Running mvn clean verify on moneta-core gives failing test cases too, and those don't even fail the build.

[ERROR] Failures: 
[ERROR]   FastMoneyTest.testToString:1174 expected [XXX 1.00] but found [XXX 0.00]
[ERROR]   FastMoneyTest.testToStringWithReverseOrder:1190 expected [XXX 1.00] but found [XXX 0.00]
[ERROR]   MoneyTest.testDivideAndRemainder_BigDecimal:93 expected [0.49999999999999999999] but found [0.5]
[ERROR]   MoneyTest.testToString:1100 expected [XXX 1.00] but found [XXX 0.00]
[ERROR]   MoneyTest.testToString_ReverseOrder:1082 expected [XXX 1.00] but found [XXX 0.00]
[ERROR]   RoundedMoneyTest.testToString:967 expected [XXX 1.00] but found [XXX 0.00]
[ERROR]   MonetaryRoundingsTest.testGetCashRoundingCurrencyUnitLong:186 expected object to not be null
[ERROR]   MonetaryRoundingsTest.testGetCustomRoundingNames:214 expected [true] but found [false]
[ERROR]   MonetaryRoundingsTest.testGetRoundingCurrencyUnitLong:170 expected object to not be null
[ERROR]   MonetaryRoundingsTest.testGetRoundingString:198 » Monetary No rounding provide...
[ERROR]   MonetaryConfigTest.testConfigOverride:35 expected [theWinner1] but found [theLooser2]
[INFO] 
[ERROR] Tests run: 610, Failures: 11, Errors: 0, Skipped: 0

Is this the normal behavior atm? Shouldn't a mvn clean verify on a clean checkout just work?

[keilw]

With Java 11? I suspect it also has something to do with the config issues.

[bpasson]

Yes with Java 11. Could be linked to the config issues, but I only ran the build in moneta-core. I'll give Java 8 a go.

[bpasson]

Java 8 isn't working either, it won't compile the source.

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.7.0:compile (default-compile) on project moneta-core: Fatal error compiling: invalid flag: --module-path -> [Help 1]

Guess due to the modules in use it needs a minimum of Java 9.

[keilw]

You can't build with Java 8 anymore, but run it in example or app code.

[bpasson]

Some test failures are related to the config issues. But the following test seems to be invalid imo.

assertEquals("XXX 0.00", FastMoney.of(new BigDecimal("1.23455"), "XXX").toString());

There are two thing odd here:

1. The expected value should be the 2nd parameter to actually get correct failure feedback.
2. The expected output should be XXX 1.00 if you ask me.

[keilw]

That could have been related to a JUnit upgrade.

[bpasson]

Hope you don't mind me asking too much questions, but why the complicated structure to target 1.8 but still having modules? I always assumed its either modules or not, am I wrong?

[keilw]

Because it's supposed to be backward-compatible with at least Java 8 for 1.x We may some day change that and set the minimum to e.g. Java 11 with 2.x but for now that makes sense, there are still many Java 8 based systems especially in banking and e-commerce. Now a question for you since you do quite some digging, are you a JCP member or consider joining the JCP? Because larger PRs or code changes in the API, RI or TCK should come from JCP members even if they are "just" Associate Members.

[bpasson]

Not a JCP member at the moment. Not sure what is expected. My time is limited currently. Digging into stuff is my nature, I just can't stand it when things don't work and I then need to know why. If the fixes are relatively easy I am always happy to offer PRs.

[keilw]

Please check out https://jcp.org/en/participation/membership for JCP membership. Most individuals unless hey are self-employed would usually join as Associate Members now. There is no expectation towards the level of contribution, but if you want everyone who contributed more than a line or a few comments may be listed as contributor on the JSR detail page We highly appreciate help given the call for participation towards a new JSR (2.x of the Money standard) was not overwhelmingly replied so far, but it was holiday season and hopefully that changes later that year, as we also do not envision any new version (here marked as ".Next") before early 2022.

[code-uri]

Running mvn clean verify on moneta-core gives failing test cases too, and those don't even fail the build.

[ERROR] Failures: 
[ERROR]   FastMoneyTest.testToString:1174 expected [XXX 1.00] but found [XXX 0.00]
[ERROR]   FastMoneyTest.testToStringWithReverseOrder:1190 expected [XXX 1.00] but found [XXX 0.00]
[ERROR]   MoneyTest.testDivideAndRemainder_BigDecimal:93 expected [0.49999999999999999999] but found [0.5]
[ERROR]   MoneyTest.testToString:1100 expected [XXX 1.00] but found [XXX 0.00]
[ERROR]   MoneyTest.testToString_ReverseOrder:1082 expected [XXX 1.00] but found [XXX 0.00]
[ERROR]   RoundedMoneyTest.testToString:967 expected [XXX 1.00] but found [XXX 0.00]
[ERROR]   MonetaryRoundingsTest.testGetCashRoundingCurrencyUnitLong:186 expected object to not be null
[ERROR]   MonetaryRoundingsTest.testGetCustomRoundingNames:214 expected [true] but found [false]
[ERROR]   MonetaryRoundingsTest.testGetRoundingCurrencyUnitLong:170 expected object to not be null
[ERROR]   MonetaryRoundingsTest.testGetRoundingString:198 » Monetary No rounding provide...
[ERROR]   MonetaryConfigTest.testConfigOverride:35 expected [theWinner1] but found [theLooser2]
[INFO] 
[ERROR] Tests run: 610, Failures: 11, Errors: 0, Skipped: 0

Is this the normal behavior atm? Shouldn't a mvn clean verify on a clean checkout just work?

Any update on this?

[keilw]

Not really, since @bpasson is not a JCP member and said, he has limited time, there was nobody looking into it so far. Are you a JCP member @code-uri? Some especially "MoneyTest.testDivideAndRemainder_BigDecimal:93 expected [0.49999999999999999999] but found [0.5]" must be due to changes in the rounding mode of the JDK iself, either there is a way to resolve those or they might have to be thrown out.

[keilw]

Those tests are all fixed, mainly due to #357 being fixed, will close this.