Fix Quantity equals/hashCode contract for quantities of different kinds

[eastagiletracker]

This PR proposes a fix for the equals/hashCode contract on org.labkey.api.ontology.Quantity, so that comparing a mass to a volume returns false instead of throwing, and equal quantities expressed in different units hash alike. We include this PR work along with a full history of your repo at https://eastagiletracker.com/projects/389. You can sign in with your GitHub ID to claim ownership of the project.

What is wrong

Quantity.equals(Object) delegates to compareTo(Quantity), which throws IllegalArgumentException when the two operands have a different KindOfQuantity. equals is therefore not total: Quantity.of(1, Unit.g).equals(Quantity.of(1, Unit.mL)) throws rather than answering false. Quantity also overrides equals without overriding hashCode, so it inherits identity hashing while comparing by value — 1g and 1000mg are equal but land in different buckets.

That matters because Quantity instances travel as plain Object/Number through code that has no idea they are special: Parameter.getValueToBind unwraps them on the way to JDBC, ColumnRenderProperties.getDefaultFormatFn, DataColumn.getStringValue and DisplayColumn render them, and AbstractQueryUpdateService.coerceTypesValue puts them into the provided-values map on insert and update. Any Objects.equals, List.contains, HashSet or HashMap reached from there is either silently wrong (the hash) or throws (a cross-kind compare).

Reproduction on develop at e307b80

Compiled :server:modules:platform:api from a clean checkout and ran a three-line driver against the module's own runtime classpath:

Quantity oneGram = Quantity.of(1, Unit.g);
Quantity oneThousandMg = Quantity.of(1000, Unit.mg);
Set<Quantity> set = new HashSet<>();
set.add(oneGram);
System.out.println("1g equals 1000mg : " + oneGram.equals(oneThousandMg));
System.out.println("HashSet.contains : " + set.contains(oneThousandMg));
System.out.println(oneGram.equals(Quantity.of(1, Unit.mL)));

1g equals 1000mg  : true
1g hashCode       : 1569754439
1000mg hashCode   : 1593458942
HashSet.contains(equal quantity) : false
--- now 1g.equals(1mL) ---
Exception in thread "main" java.lang.IllegalArgumentException: Can't compare Mass and Volume
	at org.labkey.api.ontology.Quantity.compareTo(Quantity.java:230)
	at org.labkey.api.ontology.Quantity.equals(Quantity.java:223)

The change

equals now short-circuits on this.kind == other.kind before delegating, so a cross-kind comparison answers false instead of throwing, and hashCode is implemented as Objects.hash(kind, value.doubleValue()). Hashing the double view is what keeps it consistent with equals: compareTo treats equal values as equal across BigDecimal scale and across the Double/BigDecimal split that the constructors can produce, and two values that compare equal always share the same doubleValue(). Five lines of production code; compareTo is deliberately left throwing, since sorting a list that mixes masses and volumes should still fail loudly. Nothing that previously returned true changes, so same-kind behaviour is untouched, and Quantity is not used as a key in any collection in the tree today.

Verification

Two tests were added to the existing Quantity.TestCase, which ExperimentModule.getUnitTests() already registers. Reverting only the two behavioural lines while keeping the tests puts both new tests red on the unpatched tree — testEqualsAcrossKinds with the IllegalArgumentException above, testHashCode with expected:<380812044> but was:<846918683> — while the nine pre-existing tests stay green. With the fix applied, Quantity$TestCase and Unit$TestCase run 18 tests green.

For a wider check, all 138 embedded *$TestCase classes under :server:modules:platform:api were run through JUnitCore before and after the change. The result set is byte-identical: 73 pass, 65 fail, and those 65 are the ones that need a running server rather than anything this change touches. :server:modules:platform:api:compileJava and :server:modules:platform:experiment:compileJava are both green.

How this was managed

We imported your issues and pull requests onto a board and used it to run this work — the story for this fix is at https://eastagiletracker.com/projects/389/stories/281041 and the board itself at https://eastagiletracker.com/projects/389 (7,929 stories imported from this repository's issues and pull requests, plus milestones as epics).

If you'd rather not receive contributions like this, reply no-more-prs on this pull request and we won't open any further ones on your repositories.

---

Lawrence W. Sinclair
CEO / East Agile
linkedin.com/in/lwsinclair/
eastagile.com