From 1208fe8b0ea62f5e1f3fab46160f2ce0ee94de4b Mon Sep 17 00:00:00 2001 From: Jonathan Percival Date: Wed, 25 Oct 2023 16:19:42 -0600 Subject: [PATCH 1/2] Fix bug in caching decorator --- Src/java/.vscode/settings.json | 14 +++++++++++++- .../elm/executing/FunctionRefEvaluator.java | 2 +- .../model/CachingModelResolverDecorator.java | 2 +- .../model/CachingModelResolverDecoratorTest.java | 16 ++++++++++++++++ Src/java/gradle.properties | 2 +- 5 files changed, 32 insertions(+), 4 deletions(-) diff --git a/Src/java/.vscode/settings.json b/Src/java/.vscode/settings.json index f0692a8e5..2e1e006ae 100644 --- a/Src/java/.vscode/settings.json +++ b/Src/java/.vscode/settings.json @@ -1,5 +1,6 @@ { "java.configuration.updateBuildConfiguration": "automatic", + "java.debug.settings.onBuildFailureProceed": true, "java.compile.nullAnalysis.mode": "automatic", "java.jdt.ls.vmargs": "-noverify -Xmx8G -XX:+UseG1GC -XX:+UseStringDeduplication", "cSpell.words": [ @@ -12,5 +13,16 @@ "testng", "trackback" ], - "java.debug.settings.onBuildFailureProceed": true + "cSpell.enabledLanguageIds": [ + "java", + "json", + "xml", + "markdown", + "cql" + ], + "cSpell.ignorePaths": [ + ".git", + "**/*.gradle", + "**/test/resources/**" + ] } \ No newline at end of file diff --git a/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/elm/executing/FunctionRefEvaluator.java b/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/elm/executing/FunctionRefEvaluator.java index ccf5b186b..b8a9db7a0 100644 --- a/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/elm/executing/FunctionRefEvaluator.java +++ b/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/elm/executing/FunctionRefEvaluator.java @@ -103,7 +103,7 @@ private static FunctionDef getResolvedFunctionDef(State state, final String name } if (candidateDefs.size() > 1 && !hasSignature) { - logger.debug( + logger.warn( "Using runtime function resolution for '{}'. It's recommended to always include signatures in ELM", name); } diff --git a/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/model/CachingModelResolverDecorator.java b/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/model/CachingModelResolverDecorator.java index 76a8824fc..00c84b537 100644 --- a/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/model/CachingModelResolverDecorator.java +++ b/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/model/CachingModelResolverDecorator.java @@ -88,7 +88,7 @@ public Class resolveType(Object value) { .computeIfAbsent(pn, p -> new ConcurrentHashMap<>()); var result = packageTypeResolutions - .computeIfAbsent(valueClass, t -> Optional.ofNullable(this.innerResolver.resolveType(t))); + .computeIfAbsent(valueClass, t -> Optional.ofNullable(this.innerResolver.resolveType(value))); if (result.isPresent()) { return result.get(); diff --git a/Src/java/engine/src/test/java/org/opencds/cqf/cql/engine/model/CachingModelResolverDecoratorTest.java b/Src/java/engine/src/test/java/org/opencds/cqf/cql/engine/model/CachingModelResolverDecoratorTest.java index fb2281f9d..7dd3604d0 100644 --- a/Src/java/engine/src/test/java/org/opencds/cqf/cql/engine/model/CachingModelResolverDecoratorTest.java +++ b/Src/java/engine/src/test/java/org/opencds/cqf/cql/engine/model/CachingModelResolverDecoratorTest.java @@ -14,6 +14,7 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static org.mockito.ArgumentMatchers.isA; import java.util.Date; @@ -51,6 +52,21 @@ public void context_path_resolved_only_once() { verify(m, times(1)).getContextPath("Patient", "Patient"); } + @Test + public void type_resolved_only_once() { + var m = mock(ModelResolver.class); + when(m.getPackageName()).thenReturn("test.package"); + when(m.resolveType(isA(Integer.class))).thenReturn((Class)Integer.class); + when(m.resolveType(isA(Class.class))).thenThrow(new RuntimeException("Can't get a class of a class")); + + var cache = new CachingModelResolverDecorator(m); + cache.resolveType(5); + var result = cache.resolveType(5); + + assertEquals(Integer.class, result); + verify(m, times(1)).resolveType(5); + } + @Test void testResolveIdString() { final String object = "object"; diff --git a/Src/java/gradle.properties b/Src/java/gradle.properties index 726ac1e16..5cabac154 100644 --- a/Src/java/gradle.properties +++ b/Src/java/gradle.properties @@ -2,7 +2,7 @@ org.gradle.caching=true org.gradle.parallel=true group=info.cqframework -version=3.3.0 +version=3.3.1 specification.version=1.5.2 hapi.version=6.8.3 fhir-core.version=6.0.22.2 From a0f5140eb26093d27eab1576f65bbed323a8d5b5 Mon Sep 17 00:00:00 2001 From: Jonathan Percival Date: Wed, 25 Oct 2023 16:31:04 -0600 Subject: [PATCH 2/2] Change back to debug from warning, overflowing the logs --- .../cqf/cql/engine/elm/executing/FunctionRefEvaluator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/elm/executing/FunctionRefEvaluator.java b/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/elm/executing/FunctionRefEvaluator.java index b8a9db7a0..ccf5b186b 100644 --- a/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/elm/executing/FunctionRefEvaluator.java +++ b/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/elm/executing/FunctionRefEvaluator.java @@ -103,7 +103,7 @@ private static FunctionDef getResolvedFunctionDef(State state, final String name } if (candidateDefs.size() > 1 && !hasSignature) { - logger.warn( + logger.debug( "Using runtime function resolution for '{}'. It's recommended to always include signatures in ELM", name); }