From c757c6982df1e618b2663890ed16a3dfef041e5e Mon Sep 17 00:00:00 2001 From: dotasek Date: Wed, 31 Jul 2024 18:19:33 -0400 Subject: [PATCH] Move defensive copy directly to TerminologyCache --- .../fhir/r5/context/BaseWorkerContext.java | 4 +- .../utilities/TerminologyCache.java | 4 +- .../r5/context/BaseWorkerContextTests.java | 10 +-- .../r5/context/TerminologyCacheTests.java | 75 ++++++++++++++++--- 4 files changed, 71 insertions(+), 22 deletions(-) diff --git a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/context/BaseWorkerContext.java b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/context/BaseWorkerContext.java index 1d8e9959a..540c50261 100644 --- a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/context/BaseWorkerContext.java +++ b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/context/BaseWorkerContext.java @@ -1286,7 +1286,7 @@ public abstract class BaseWorkerContext extends I18nBase implements IWorkerConte } if (res != null) { updateUnsupportedCodeSystems(res, code, getCodeKey(code)); - return new ValidationResult(res); + return res; } List issues = new ArrayList<>(); @@ -1566,7 +1566,7 @@ public abstract class BaseWorkerContext extends I18nBase implements IWorkerConte if (cachingAllowed) { res = txCache.getValidation(cacheToken); if (res != null) { - return new ValidationResult(res); + return res; } } for (Coding c : code.getCoding()) { diff --git a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/terminologies/utilities/TerminologyCache.java b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/terminologies/utilities/TerminologyCache.java index 4de741209..b65c63a2e 100644 --- a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/terminologies/utilities/TerminologyCache.java +++ b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/terminologies/utilities/TerminologyCache.java @@ -628,7 +628,7 @@ public class TerminologyCache { return null; } else { hitCount++; - return e.v; + return new ValidationResult(e.v); } } } @@ -640,7 +640,7 @@ public class TerminologyCache { CacheEntry e = new CacheEntry(); e.request = cacheToken.request; e.persistent = persistent; - e.v = res; + e.v = new ValidationResult(res); store(cacheToken, persistent, nc, e); } } diff --git a/org.hl7.fhir.r5/src/test/java/org/hl7/fhir/r5/context/BaseWorkerContextTests.java b/org.hl7.fhir.r5/src/test/java/org/hl7/fhir/r5/context/BaseWorkerContextTests.java index 3592bd6e5..9d646387b 100644 --- a/org.hl7.fhir.r5/src/test/java/org/hl7/fhir/r5/context/BaseWorkerContextTests.java +++ b/org.hl7.fhir.r5/src/test/java/org/hl7/fhir/r5/context/BaseWorkerContextTests.java @@ -330,7 +330,6 @@ public class BaseWorkerContextTests { ValidationResult actualValidationResult = context.validateCode(validationOptions, coding, valueSet, ctxt); - assertNotSame(cachedValidationResult, actualValidationResult); assertEquals(cachedValidationResult, actualValidationResult); Mockito.verify(valueSetCheckerSimple, times(0)).validateCode("Coding", coding); @@ -353,7 +352,7 @@ public class BaseWorkerContextTests { ValidationResult actualValidationResult = context.validateCode(validationOptions, coding, valueSet, ctxt); - assertSame(createdValidationResult, actualValidationResult); + assertEquals(createdValidationResult, actualValidationResult); Mockito.verify(valueSetCheckerSimple).validateCode(eq("Coding"), argThat(new CodingMatcher(coding))); Mockito.verify(terminologyCache).getValidation(cacheToken); @@ -379,7 +378,7 @@ public class BaseWorkerContextTests { ValidationResult actualValidationResult = context.validateCode(validationOptions, coding, valueSet, ctxt); - assertSame(createdValidationResult, actualValidationResult); + assertEquals(createdValidationResult, actualValidationResult); Mockito.verify(valueSetCheckerSimple, times(0)).validateCode("Coding", coding); Mockito.verify(terminologyCache).getValidation(cacheToken); @@ -395,7 +394,6 @@ public class BaseWorkerContextTests { Mockito.doReturn(cachedValidationResult).when(terminologyCache).getValidation(cacheToken); ValidationResult actualValidationResult = context.validateCode(CacheTestUtils.validationOptions, codeableConcept, valueSet); - assertNotSame(cachedValidationResult, actualValidationResult); assertEquals(cachedValidationResult, actualValidationResult); Mockito.verify(valueSetCheckerSimple, times(0)).validateCode("CodeableConcept", codeableConcept); @@ -414,7 +412,7 @@ public class BaseWorkerContextTests { Mockito.doReturn(cacheToken).when(terminologyCache).generateValidationToken(CacheTestUtils.validationOptions, codeableConcept, valueSet, expParameters); ValidationResult validationResultB = context.validateCode(CacheTestUtils.validationOptions, codeableConcept, valueSet); - assertSame(createdValidationResult, validationResultB); + assertEquals(createdValidationResult, validationResultB); Mockito.verify(valueSetCheckerSimple).validateCode("CodeableConcept", codeableConcept); Mockito.verify(terminologyCache).cacheValidation(eq(cacheToken), same(createdValidationResult), eq(false)); @@ -462,7 +460,7 @@ public class BaseWorkerContextTests { ValueSetExpansionOutcome actualExpansionResult = context.expandVS(inc, true, false); - assertSame(expectedExpansionResult, actualExpansionResult); + assertEquals(expectedExpansionResult, actualExpansionResult); Mockito.verify(terminologyCache).getExpansion(cacheToken); Mockito.verify(terminologyCache, times(0)).cacheExpansion(any(), any(), anyBoolean()); diff --git a/org.hl7.fhir.r5/src/test/java/org/hl7/fhir/r5/context/TerminologyCacheTests.java b/org.hl7.fhir.r5/src/test/java/org/hl7/fhir/r5/context/TerminologyCacheTests.java index 42ed1326f..8a3a0a3d4 100644 --- a/org.hl7.fhir.r5/src/test/java/org/hl7/fhir/r5/context/TerminologyCacheTests.java +++ b/org.hl7.fhir.r5/src/test/java/org/hl7/fhir/r5/context/TerminologyCacheTests.java @@ -1,10 +1,5 @@ package org.hl7.fhir.r5.context; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertNull; -import static org.junit.jupiter.api.Assertions.assertTrue; - import java.io.File; import java.io.IOException; import java.io.InputStream; @@ -44,6 +39,8 @@ import org.junit.jupiter.params.provider.MethodSource; import com.google.gson.JsonElement; import com.google.gson.JsonParser; +import static org.junit.jupiter.api.Assertions.*; + public class TerminologyCacheTests implements ResourceLoaderTests { static final ValueSet.ConceptSetComponent include = new ValueSet.ConceptSetComponent(); @@ -93,7 +90,7 @@ public class TerminologyCacheTests implements ResourceLoaderTests { } @Test - public void testCachePersistence() throws IOException, URISyntaxException { + public void testCachePersistence() throws IOException { String address = "/..."; Object lock = new Object(); @@ -115,8 +112,6 @@ public class TerminologyCacheTests implements ResourceLoaderTests { CodeableConcept concept = new CodeableConcept(); concept.addCoding(new Coding().setCode("dummyCode")); - ValueSet ccvalueSet = new ValueSet(); - // Add dummy results to the cache TerminologyCache terminologyCacheA = new TerminologyCache(lock, tempCacheDirectory.toString()); @@ -143,8 +138,13 @@ public class TerminologyCacheTests implements ResourceLoaderTests { assertEquals(terminologyCapabilities, terminologyCacheA.getTerminologyCapabilities(address)); assertEquals(capabilityStatement, terminologyCacheA.getCapabilityStatement(address)); - assertValidationResultEquals(codingResultA, terminologyCacheA.getValidation(codingTokenA)); - assertValidationResultEquals(codeableConceptResultA, terminologyCacheA.getValidation(codeableConceptTokenA)); + ValidationResult retrievedCodingResultA = terminologyCacheA.getValidation(codingTokenA); + assertNotSame(codingResultA, retrievedCodingResultA); + assertValidationResultEquals(codingResultA, retrievedCodingResultA); + + ValidationResult retrievedCodeableConceptResultA = terminologyCacheA.getValidation(codeableConceptTokenA); + assertNotSame(codeableConceptResultA, retrievedCodeableConceptResultA); + assertValidationResultEquals(codeableConceptResultA, retrievedCodeableConceptResultA); assertExpansionOutcomeEquals(expansionOutcomeA,terminologyCacheA.getExpansion(expansionTokenA)); } @@ -155,13 +155,64 @@ public class TerminologyCacheTests implements ResourceLoaderTests { assertCanonicalResourceEquals(terminologyCapabilities, terminologyCacheB.getTerminologyCapabilities(address)); assertCanonicalResourceEquals(capabilityStatement, terminologyCacheB.getCapabilityStatement(address)); - assertValidationResultEquals(codingResultA, terminologyCacheB.getValidation(terminologyCacheA.generateValidationToken(CacheTestUtils.validationOptions, coding, valueSet, new Parameters()))); - assertValidationResultEquals(codeableConceptResultA, terminologyCacheB.getValidation(terminologyCacheA.generateValidationToken(CacheTestUtils.validationOptions, concept, valueSet, new Parameters()))); + ValidationResult retrievedCodingResultA = terminologyCacheB.getValidation(terminologyCacheA.generateValidationToken(CacheTestUtils.validationOptions, coding, valueSet, new Parameters())); + assertNotSame(codingResultA, retrievedCodingResultA); + assertValidationResultEquals(codingResultA, retrievedCodingResultA); + + ValidationResult retrievedCodeableConceptResultA = terminologyCacheB.getValidation(terminologyCacheA.generateValidationToken(CacheTestUtils.validationOptions, concept, valueSet, new Parameters())); + assertNotSame(codeableConceptResultA, retrievedCodeableConceptResultA); + assertValidationResultEquals(codeableConceptResultA, retrievedCodeableConceptResultA); assertExpansionOutcomeEquals(expansionOutcomeA,terminologyCacheB.getExpansion(terminologyCacheA.generateExpandToken(valueSet, true))); } deleteTempCacheDirectory(tempCacheDirectory); } + @Test + public void testCacheMakesCopiesOfResults() throws IOException{ + + Object lock = new Object(); + Path tempCacheDirectory = createTempCacheDirectory(); + + TerminologyCache terminologyCache = new TerminologyCache(lock, tempCacheDirectory.toString()); + + Coding coding = new Coding(); + coding.setCode("dummyCode"); + + CodeableConcept concept = new CodeableConcept(); + concept.addCoding(new Coding().setCode("dummyCode")); + + ValueSet valueSet = new ValueSet(); + valueSet.setUrl("dummyValueSetURL"); + + ValidationResult codingResult = new ValidationResult(ValidationMessage.IssueSeverity.INFORMATION, "dummyInfo", null); + TerminologyCache.CacheToken codingToken = terminologyCache.generateValidationToken(CacheTestUtils.validationOptions, + coding, valueSet, new Parameters()); + terminologyCache.cacheValidation(codingToken, codingResult, true); + + ValidationResult retrievedCodingResult = terminologyCache.getValidation(codingToken); + assertEquals(codingResult, retrievedCodingResult); + + codingResult.setMessage("changed"); + + retrievedCodingResult = terminologyCache.getValidation(codingToken); + assertNotEquals(codingResult, retrievedCodingResult); + assertEquals("dummyInfo", retrievedCodingResult.getMessage()); + + ValidationResult codeableConceptResult = new ValidationResult(ValidationMessage.IssueSeverity.INFORMATION, "dummyInfo", null); + TerminologyCache.CacheToken codeableConceptToken = terminologyCache.generateValidationToken(CacheTestUtils.validationOptions, + concept, valueSet, new Parameters()); + terminologyCache.cacheValidation(codeableConceptToken, codeableConceptResult, true); + + ValidationResult retrievedCodeableConceptResult = terminologyCache.getValidation(codeableConceptToken); + assertEquals(codeableConceptResult, retrievedCodeableConceptResult); + + codeableConceptResult.setMessage("changed"); + + retrievedCodeableConceptResult = terminologyCache.getValidation(codeableConceptToken); + assertNotEquals(codeableConceptResult, retrievedCodeableConceptResult); + assertEquals("dummyInfo", retrievedCodeableConceptResult.getMessage()); + } + private void assertCanonicalResourceEquals(CanonicalResource a, CanonicalResource b) { assertTrue(a.equalsDeep(b)); }