diff --git a/CHANGELOG.md b/CHANGELOG.md index da979a9e..f41e2163 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,11 +2,20 @@ + +## v4.10.0 + +Date: 2026-07-02 + +* bump version to 4.10.0 +* fix model info file resolution + + ## v4.9.0 Date: 2026-06-25 -* bump version to cql-language-server to 4.9.0 +* bump version to cql-language-server to 4.9.0 * change clinical-reasoning to version 4.8.0 * fix issue with library names with hyphens diff --git a/ls/server/pom.xml b/ls/server/pom.xml index a5a3b3f1..edb01c25 100644 --- a/ls/server/pom.xml +++ b/ls/server/pom.xml @@ -12,7 +12,7 @@ org.opencds.cqf.cql.ls cql-ls - 4.10.0-SNAPSHOT + 4.10.0 ../../pom.xml diff --git a/ls/server/src/main/kotlin/org/opencds/cqf/cql/debug/CqlDebugServer.kt b/ls/server/src/main/kotlin/org/opencds/cqf/cql/debug/CqlDebugServer.kt index 475dab25..5db8b036 100644 --- a/ls/server/src/main/kotlin/org/opencds/cqf/cql/debug/CqlDebugServer.kt +++ b/ls/server/src/main/kotlin/org/opencds/cqf/cql/debug/CqlDebugServer.kt @@ -57,6 +57,7 @@ import org.hl7.elm.r1.VersionedIdentifier import org.hl7.fhir.instance.model.api.IBase import org.opencds.cqf.cql.engine.execution.State import org.opencds.cqf.cql.ls.core.ContentService +import org.opencds.cqf.cql.ls.core.utility.Uris import org.opencds.cqf.cql.ls.server.command.ContextRequest import org.opencds.cqf.cql.ls.server.command.CqlEvaluator import org.opencds.cqf.cql.ls.server.command.DetailedExpressionResult @@ -643,7 +644,11 @@ open class CqlDebugServer( listOf( LibraryRequest( libraryName = args.libraryName, - libraryUri = args.libraryUri, + // libraryUri must be the CQL *directory* (CqlEvaluator contract); the DAP + // launch config gives us the .cql file, so strip to its parent — mirrors + // CqlCompilationManager.createLibraryManager and the Execute CQL flow. + // Fixes model-info lookups landing at ".cql/-modelinfo.xml". + libraryUri = Uris.getHead(URI.create(args.libraryUri)).toString(), libraryVersion = null, terminologyUri = args.terminologyUri, model = args.testCaseUri?.let { ModelRequest("FHIR", it) }, diff --git a/ls/server/src/main/kotlin/org/opencds/cqf/cql/ls/server/command/CqlEvaluator.kt b/ls/server/src/main/kotlin/org/opencds/cqf/cql/ls/server/command/CqlEvaluator.kt index e2de7741..1fb7dca4 100644 --- a/ls/server/src/main/kotlin/org/opencds/cqf/cql/ls/server/command/CqlEvaluator.kt +++ b/ls/server/src/main/kotlin/org/opencds/cqf/cql/ls/server/command/CqlEvaluator.kt @@ -415,6 +415,10 @@ object CqlEvaluator { // (local → npm → bundled FHIRHelpers) is guaranteed even if that ever changes. evaluationSettings.librarySourceProviders.clear() if (libraryUri != null) { + // The client sends libraryUri as the CQL *directory* (input/cql), not a file, so + // it is used directly as the provider root. Do NOT apply Uris.getHead here: that + // would strip a segment (input/cql → input) and mis-root resolution. This matches + // CqlCompilationManager, which passes the already-derived directory as the root. evaluationSettings.librarySourceProviders.add( FederatedLibrarySourceProvider(libraryUri, contentService, npmProcessor), ) @@ -451,6 +455,11 @@ object CqlEvaluator { // Model info providers have no ordering concern; register after engine creation. if (libraryUri != null) { + // libraryUri is the CQL directory (input/cql); pass it directly. Applying + // Uris.getHead here was the cause of ModelInfo lookups landing in input/ instead + // of input/cql/ during execution (they succeed at edit time because + // CqlCompilationManager already roots the provider at input/cql). + log.info("Registered ContentServiceModelInfoProvider (execute) root={}", libraryUri) engine.environment.libraryManager!!.modelManager.modelInfoLoader.registerModelInfoProvider( ContentServiceModelInfoProvider(libraryUri, contentService), ) @@ -532,10 +541,26 @@ object CqlEvaluator { libraryResults.add(LibraryResult(libraryRequest.libraryName, expressions, defaultParams)) } catch (e: Exception) { log.error("Error evaluating library ${libraryRequest.libraryName} for context ${libraryRequest.context?.contextValue}", e) + // Surface the nested cause chain on one output-channel-visible line, in addition + // to the stack trace above. NOTE: for engine compile failures the deepest reachable + // cause is the flattened CqlException message — the engine does not chain + // CqlCompilerException causes (LoadAndValidateLibrariesResult.wrapExceptions joins + // only messages). The requiredModelInfo logging in ContentServiceModelInfoProvider + // is what exposes model version-conflict failures. + val causeChain = describeCauseChain(e) + log.error(" cause chain: {}", causeChain) + val topMessage = e.message ?: e.javaClass.simpleName + val rootMessage = deepestCause(e).message + val errorText = + if (rootMessage != null && rootMessage != topMessage) { + "$topMessage (cause: $rootMessage)" + } else { + topMessage + } libraryResults.add( LibraryResult( libraryRequest.libraryName, - listOf(ExpressionResult("Error", e.message ?: e.javaClass.simpleName)), + listOf(ExpressionResult("Error", errorText)), ), ) } @@ -544,6 +569,37 @@ object CqlEvaluator { return libraryResults to detailedResults } + /** Returns the deepest (root) cause of [t], following the `cause` chain (cycle-safe). */ + internal fun deepestCause(t: Throwable): Throwable { + val seen = HashSet() + var current = t + while (true) { + val next = current.cause ?: return current + if (!seen.add(current) || next === current) return current + current = next + } + } + + /** + * Renders [t]'s cause chain as `SimpleName: message` per level (deepest last), skipping + * consecutive levels whose message is identical to the previous one. Cycle-safe. + */ + internal fun describeCauseChain(t: Throwable): String { + val parts = mutableListOf() + val seen = HashSet() + var current: Throwable? = t + var lastMessage: String? = null + while (current != null && seen.add(current)) { + val message = current.message + if (message != lastMessage) { + parts.add("${current.javaClass.simpleName}: ${message ?: "(no message)"}") + lastMessage = message + } + current = current.cause + } + return parts.joinToString(" -> ") + } + internal fun collectDefineOrder( frames: List, seen: MutableSet, diff --git a/ls/server/src/main/kotlin/org/opencds/cqf/cql/ls/server/manager/CqlCompilationManager.kt b/ls/server/src/main/kotlin/org/opencds/cqf/cql/ls/server/manager/CqlCompilationManager.kt index bd02eeb6..8638b731 100644 --- a/ls/server/src/main/kotlin/org/opencds/cqf/cql/ls/server/manager/CqlCompilationManager.kt +++ b/ls/server/src/main/kotlin/org/opencds/cqf/cql/ls/server/manager/CqlCompilationManager.kt @@ -167,6 +167,7 @@ class CqlCompilationManager( root: URI, modelManager: ModelManager, ): LibraryManager { + log.info("Registered ContentServiceModelInfoProvider (compile) root={}", root) modelManager.modelInfoLoader.registerModelInfoProvider( ContentServiceModelInfoProvider(root, contentService), ) diff --git a/ls/server/src/main/kotlin/org/opencds/cqf/cql/ls/server/manager/LibraryResolutionManager.kt b/ls/server/src/main/kotlin/org/opencds/cqf/cql/ls/server/manager/LibraryResolutionManager.kt index bb4577ec..1ca4df0c 100644 --- a/ls/server/src/main/kotlin/org/opencds/cqf/cql/ls/server/manager/LibraryResolutionManager.kt +++ b/ls/server/src/main/kotlin/org/opencds/cqf/cql/ls/server/manager/LibraryResolutionManager.kt @@ -109,11 +109,13 @@ open class LibraryResolutionManager( /** * Reads the package ID and canonical base URL from an ig.ini file. - * Returns null when either field is absent; throws on I/O or parse failure + * Returns null when either field is absent, or when the ini file is missing + * the [IG] section. May still throw on I/O or deeper parse failure * (caller catches and skips the folder). * Declared protected open for test overriding — see [IgContextManager.findIgContext]. */ protected open fun readIgContextInfo(igIniFile: File): Pair? { + if (!igIniFile.useLines { lines -> lines.any { it.trim().equals("[IG]") } }) return null val igContext = IGContext(LoggerAdapter(log)) igContext.initializeFromIni(igIniFile.path) val packageId = igContext.packageId ?: return null diff --git a/ls/server/src/main/kotlin/org/opencds/cqf/cql/ls/server/provider/ContentServiceModelInfoProvider.kt b/ls/server/src/main/kotlin/org/opencds/cqf/cql/ls/server/provider/ContentServiceModelInfoProvider.kt index 0198dc7d..7994f092 100644 --- a/ls/server/src/main/kotlin/org/opencds/cqf/cql/ls/server/provider/ContentServiceModelInfoProvider.kt +++ b/ls/server/src/main/kotlin/org/opencds/cqf/cql/ls/server/provider/ContentServiceModelInfoProvider.kt @@ -16,21 +16,117 @@ class ContentServiceModelInfoProvider( ) : ModelInfoProvider { companion object { private val log = LoggerFactory.getLogger(ContentServiceModelInfoProvider::class.java) + + /** + * Renders a ModelInfo's declared dependencies (`requiredModelInfo`) as + * `[name version, ...]`. Logged after a successful parse so version conflicts are visible: + * e.g. a model that requires `USCore 7.0.0` while the content loads `USCore 6.1.0-derived` + * surfaces as a mismatch in the log. Returns `[]` when there are no declared dependencies. + */ + internal fun formatRequiredModels(modelInfo: ModelInfo): String = + modelInfo.requiredModelInfo + .joinToString(prefix = "[", postfix = "]") { req -> + "${req.name}${req.version?.let { " $it" } ?: ""}" + } + } + + /** + * Tracks the versions each model has been seen at during this provider's lifetime: + * `modelId` → `version` → `source`. Populated as models are requested and as their declared + * dependencies (`requiredModelInfo`) are parsed. The CQL compiler allows exactly one version + * of a model per translation, so seeing a model at two versions is a genuine conflict (e.g. + * the content loads `USCore 6.1.0-derived` while a developer-supplied C4BB ModelInfo requires + * `USCore 7.0.0`). + * + * This is instance state, not static: a fresh provider is constructed per compilation + * (see `CqlCompilationManager.createLibraryManager` and `CqlEvaluator`), so version tracking + * starts empty each compile and never carries stale versions across runs. Each instance is + * scoped to a single [root], so keying by `modelId` alone is sufficient. + */ + private val observedModelVersions = HashMap>() + + /** + * Records that [modelId] was seen at [version] (attributed to [source]), and returns a + * human-readable conflict description when that model is now known at more than one version — + * i.e. an actual model version conflict. When the observed versions differ *only* by case + * (e.g. `6.1.0-Derived` vs `6.1.0-derived`), the message calls that out explicitly, since the + * CQL engine matches model versions with exact, case-sensitive string equality and such a + * mismatch is a common, easily-missed authoring error. Returns null when there is no conflict + * or [version] is null/blank. + */ + internal fun recordVersionAndDetectConflict( + modelId: String, + version: String?, + source: String, + ): String? { + if (version.isNullOrBlank()) return null + val versions = observedModelVersions.getOrPut(modelId) { mutableMapOf() } + versions.putIfAbsent(version, source) + if (versions.size <= 1) return null + + val rendered = versions.entries.joinToString(", ") { "${it.key} (${it.value})" } + val caseOnly = versions.keys.map { it.lowercase() }.distinct().size == 1 + return if (caseOnly) { + "versions differ only by case — $rendered. The CQL engine matches model versions " + + "case-sensitively; make them identical." + } else { + rendered + } } override fun load(modelIdentifier: ModelIdentifier): ModelInfo? { val modelName = modelIdentifier.id val modelVersion = modelIdentifier.version + log.info( + "ContentServiceModelInfoProvider: resolving model '{}' version '{}' (root={})", + modelName, + modelVersion, + root, + ) + + // Record the requested version; warn if this model is now known at two versions. + recordVersionAndDetectConflict(modelName, modelVersion, "requested")?.let { + log.warn("ContentServiceModelInfoProvider: model version conflict for '{}': {}", modelName, it) + } + return try { val modelUri = Uris.addPath( root, "/${modelName.lowercase()}-modelinfo${modelVersion?.let { "-$it" } ?: ""}.xml", - ) ?: return null - val modelInputStream = contentService.read(modelUri) ?: return null - parseModelInfoXml(Converters.inputStreamToString(modelInputStream)) + ) ?: run { + log.info( + "ContentServiceModelInfoProvider: could not build model info URI for '{}' from root {}", + modelName, + root, + ) + return null + } + log.info("ContentServiceModelInfoProvider: attempting to read model info for '{}' from {}", modelName, modelUri) + val modelInputStream = + contentService.read(modelUri) ?: run { + log.info("ContentServiceModelInfoProvider: NOT FOUND — no content at {}", modelUri) + return null + } + val modelInfo = parseModelInfoXml(Converters.inputStreamToString(modelInputStream)) + log.info("ContentServiceModelInfoProvider: FOUND and parsed model info for '{}' at {}", modelName, modelUri) + log.warn( + "ContentServiceModelInfoProvider: '{}' requires {}", + modelName, + formatRequiredModels(modelInfo), + ) + // Record each declared dependency's version; warn on any actual version conflict + // (the same model now known at two versions under this root). + for (req in modelInfo.requiredModelInfo) { + val depName = req.name ?: continue + recordVersionAndDetectConflict(depName, req.version, "required by $modelName")?.let { + log.warn("ContentServiceModelInfoProvider: model version conflict for '{}': {}", depName, it) + } + } + modelInfo } catch (e: Exception) { + log.error("ContentServiceModelInfoProvider: error loading model info for '{}' from root {}", modelName, root, e) throw IllegalArgumentException("Could not load definition for model info ${modelIdentifier.id}.", e) } } diff --git a/ls/server/src/test/kotlin/org/opencds/cqf/cql/debug/CqlDebugServerHelperTest.kt b/ls/server/src/test/kotlin/org/opencds/cqf/cql/debug/CqlDebugServerHelperTest.kt index 50760088..4aae1905 100644 --- a/ls/server/src/test/kotlin/org/opencds/cqf/cql/debug/CqlDebugServerHelperTest.kt +++ b/ls/server/src/test/kotlin/org/opencds/cqf/cql/debug/CqlDebugServerHelperTest.kt @@ -16,6 +16,7 @@ import org.junit.jupiter.api.Assertions.assertTrue import org.junit.jupiter.api.Test import org.mockito.Mockito import org.opencds.cqf.cql.ls.core.ContentService +import org.opencds.cqf.cql.ls.server.command.ExecuteCqlRequest import org.opencds.cqf.cql.ls.server.manager.CqlCompilationManager import org.opencds.cqf.cql.ls.server.manager.IgContextManager import org.opencds.cqf.cql.ls.server.manager.LibraryResolutionManager @@ -823,4 +824,39 @@ class CqlDebugServerHelperTest { val result = method.invoke(server, null) as Map assertTrue(result.isEmpty()) } + + // -- buildExecuteCqlRequest ----------------------------------------------- + // The DAP launch config supplies libraryUri as the .cql FILE, but CqlEvaluator (and its + // ContentServiceModelInfoProvider) expects the CQL *directory* — otherwise model-info lookups + // land at ".cql/-modelinfo.xml" ("Not a directory") and C4BB fails to load. The + // request builder must strip the filename to its parent, matching the Execute CQL flow. + + private fun buildRequestLibraryUri(libraryUri: String): String { + val server = makeServer() + val method = + CqlDebugServer::class.java.getDeclaredMethod( + "buildExecuteCqlRequest", + DebugLaunchArgs::class.java, + ) + method.isAccessible = true + val args = DebugLaunchArgs(libraryUri = libraryUri, libraryName = "MyLibrary", fhirVersion = "R4") + val request = method.invoke(server, args) as ExecuteCqlRequest + return request.libraries.first().libraryUri + } + + @Test + fun `buildExecuteCqlRequest strips cql filename to its parent directory`() { + assertEquals( + "file:///workspace/input/cql", + buildRequestLibraryUri("file:///workspace/input/cql/MyLibrary.cql"), + ) + } + + @Test + fun `buildExecuteCqlRequest strips cql filename on windows forward-slash uri`() { + assertEquals( + "file:///C:/work/input/cql", + buildRequestLibraryUri("file:///C:/work/input/cql/MyLibrary.cql"), + ) + } } diff --git a/ls/server/src/test/kotlin/org/opencds/cqf/cql/ls/server/command/CqlEvaluatorTest.kt b/ls/server/src/test/kotlin/org/opencds/cqf/cql/ls/server/command/CqlEvaluatorTest.kt index 3b06e908..362c9ffc 100644 --- a/ls/server/src/test/kotlin/org/opencds/cqf/cql/ls/server/command/CqlEvaluatorTest.kt +++ b/ls/server/src/test/kotlin/org/opencds/cqf/cql/ls/server/command/CqlEvaluatorTest.kt @@ -25,13 +25,16 @@ import org.junit.jupiter.api.io.TempDir import org.opencds.cqf.cql.engine.debug.BreakpointAction import org.opencds.cqf.cql.engine.debug.BreakpointHandler import org.opencds.cqf.cql.engine.execution.State +import org.opencds.cqf.cql.ls.core.ContentService import org.opencds.cqf.cql.ls.server.manager.IgContextManager import org.opencds.cqf.cql.ls.server.manager.LibraryResolutionManager import org.opencds.cqf.cql.ls.server.service.TestContentService import org.opencds.cqf.fhir.cql.CqlOptions import org.opencds.cqf.fhir.cql.EvaluationSettings import org.opencds.cqf.fhir.utility.repository.ProxyRepository +import java.io.InputStream import java.math.BigDecimal +import java.net.URI import java.nio.file.Files import java.nio.file.Path @@ -520,6 +523,7 @@ class CqlEvaluatorTest { return method.invoke(CqlEvaluator, value) as Quantity } + @Suppress("UnstableApiUsage") private fun createRepository( fhirContext: FhirContext, terminologyRepo: IRepository, @@ -762,9 +766,7 @@ class CqlEvaluatorTest { // ------------------------------------------------------------------------- @Test - fun `createRepository with null modelPath returns ProxyRepository wrapping NoOpRepository`( - @TempDir tempDir: Path, - ) { + fun `createRepository with null modelPath returns ProxyRepository wrapping NoOpRepository`() { val repo = createRepository(r4Context, createNoOpRepo(), null) assertInstanceOf(ProxyRepository::class.java, repo) } @@ -903,7 +905,7 @@ class CqlEvaluatorTest { val isDebugMethod = innerClass.getDeclaredMethod("isDebugLogging") isDebugMethod.isAccessible = true val result = isDebugMethod.invoke(logger) - assertInstanceOf(java.lang.Boolean::class.java, result) + assertInstanceOf(Boolean::class.javaObjectType, result) } // ------------------------------------------------------------------------- @@ -991,8 +993,174 @@ class CqlEvaluatorTest { assertEquals("mg", q.unit) } + @Suppress("UnstableApiUsage") private fun createNoOpRepo(): IRepository = NoOpRepository(r4Context) + // ------------------------------------------------------------------------- + // ModelInfo resolution root + // Regression: the client sends libraryUri as the CQL *directory* (e.g. .../input/cql), + // so the ContentServiceModelInfoProvider must be rooted there and read + // .../input/cql/{model}-modelinfo-{version}.xml. A prior bug applied Uris.getHead(libraryUri), + // stripping a segment and reading from .../input/{model}-modelinfo-{version}.xml — which + // failed during execution (but not during editing, where the provider is already rooted at + // input/cql). See CqlEvaluator.evaluateBatch. + // ------------------------------------------------------------------------- + + private class RecordingContentService : ContentService { + val reads = mutableListOf() + private val delegate = TestContentService() + + override fun locate( + root: URI, + identifier: VersionedIdentifier, + ): Set = delegate.locate(root, identifier) + + override fun read(uri: URI): InputStream? { + reads.add(uri) + return delegate.read(uri) + } + } + + // ------------------------------------------------------------------------- + // Cause-chain surfacing (describeCauseChain / deepestCause) + // ------------------------------------------------------------------------- + + @Test + fun `deepestCause returns the root cause`() { + val root = IllegalStateException("root") + val mid = IllegalArgumentException("mid", root) + val top = RuntimeException("top", mid) + assertEquals("root", CqlEvaluator.deepestCause(top).message) + } + + @Test + fun `deepestCause returns self when there is no cause`() { + val e = RuntimeException("only") + assertSame(e, CqlEvaluator.deepestCause(e)) + } + + @Test + fun `deepestCause terminates when cause is self`() { + // Override the Kotlin `cause` property to return `this`, exercising the `next === current` guard + val selfCausing = + object : RuntimeException("self-referential") { + override val cause: Throwable get() = this + } + val result = CqlEvaluator.deepestCause(selfCausing) + assertSame(selfCausing, result, "deepestCause should return the self-causing exception without looping") + } + + @Test + fun `describeCauseChain includes each distinct level deepest last`() { + val root = IllegalStateException("version 7.0.0 conflicts with 6.1.0-derived") + val top = RuntimeException("Could not load model information for model C4BB", root) + val chain = CqlEvaluator.describeCauseChain(top) + assertTrue(chain.contains("Could not load model information for model C4BB"), chain) + assertTrue(chain.contains("version 7.0.0 conflicts with 6.1.0-derived"), chain) + // deepest cause rendered after the top-level message + assertTrue( + chain.indexOf("Could not load model information") < chain.indexOf("conflicts with"), + chain, + ) + } + + @Test + fun `describeCauseChain collapses consecutive identical messages`() { + val root = IllegalStateException("same") + val top = RuntimeException("same", root) + // Both levels share the message "same"; it should appear once. + assertEquals(1, CqlEvaluator.describeCauseChain(top).split(" -> ").size) + } + + @Test + fun `describeCauseChain uses no message placeholder when cause has a null message`() { + // lastMessage starts as null, so a root-level null message is deduped away. + // A null message IS rendered when it follows a non-null message (null != "top"). + val nullMessageCause = RuntimeException(null as String?) + val top = RuntimeException("top message", nullMessageCause) + val chain = CqlEvaluator.describeCauseChain(top) + assertTrue(chain.contains("(no message)"), "Expected '(no message)' placeholder for null-message cause, got: $chain") + } + + @Test + fun `evaluate produces Error expression when library cannot be found`() { + val request = + ExecuteCqlRequest( + fhirVersion = "R4", + rootDir = null, + optionsPath = null, + libraries = + listOf( + LibraryRequest( + libraryName = "NonExistentLib", + libraryUri = "file:///nonexistent/path", + libraryVersion = "1", + terminologyUri = null, + model = null, + context = null, + parameters = emptyList(), + ), + ), + ) + val response = CqlEvaluator.evaluate(request, contentService, igContextManager, libraryResolutionManager) + assertEquals(1, response.results.size) + val errorExpr = response.results[0].expressions.find { it.name == "Error" } + assertNotNull(errorExpr, "Expected an Error expression in results") + assertTrue(errorExpr!!.value.isNotEmpty(), "Error message should not be empty") + } + + @Test + fun `evaluate resolves model info relative to the libraryUri directory not its parent`() { + val recording = RecordingContentService() + val libDir = "file:///project/input/cql" + val request = + ExecuteCqlRequest( + fhirVersion = "R4", + rootDir = null, + optionsPath = null, + libraries = + listOf( + LibraryRequest( + libraryName = "UsesCustomModel", + libraryUri = libDir, + libraryVersion = "1", + terminologyUri = null, + model = null, + context = null, + parameters = emptyList(), + ), + ), + ) + + // Evaluation is expected to fail to resolve the Custom model (no real model info on the + // classpath); we only assert WHERE the provider looked for it. + CqlEvaluator.evaluate(request, recording, igContextManager, libraryResolutionManager) + + val modelInfoReads = recording.reads.filter { it.toString().contains("custom-modelinfo") } + assertTrue( + modelInfoReads.isNotEmpty(), + "Expected a model info read attempt for the Custom model, got reads: ${recording.reads}", + ) + // Use rawPath (path component only) so the assertion is platform-agnostic. + // On Windows, file:///project/... becomes file:////project/... (UNC form) after + // Uris.parseOrNull, so toString() equality against the 3-slash form fails. + // rawPath strips the scheme+authority and is identical on both platforms. + assertTrue( + modelInfoReads.any { + it.rawPath?.endsWith("/project/input/cql/custom-modelinfo-1.0.0.xml") == true + }, + "ModelInfo must be read from the libraryUri directory. Attempts: $modelInfoReads", + ) + // The parent-directory path is the regression signature — it must never be attempted. + // The /cql/ segment in the correct path prevents a false positive here. + assertFalse( + modelInfoReads.any { + it.rawPath?.endsWith("/project/input/custom-modelinfo-1.0.0.xml") == true + }, + "ModelInfo must not be read from the parent of libraryUri. Attempts: $modelInfoReads", + ) + } + // ------------------------------------------------------------------------- // evaluateDetailed // ------------------------------------------------------------------------- diff --git a/ls/server/src/test/kotlin/org/opencds/cqf/cql/ls/server/manager/LibraryResolutionManagerTest.kt b/ls/server/src/test/kotlin/org/opencds/cqf/cql/ls/server/manager/LibraryResolutionManagerTest.kt index 8791ed3d..b6f5e217 100644 --- a/ls/server/src/test/kotlin/org/opencds/cqf/cql/ls/server/manager/LibraryResolutionManagerTest.kt +++ b/ls/server/src/test/kotlin/org/opencds/cqf/cql/ls/server/manager/LibraryResolutionManagerTest.kt @@ -463,6 +463,27 @@ class LibraryResolutionManagerTest { assertTrue(m.igProjects().isEmpty(), "Folder with null readIgContextInfo result should be skipped") } + // ----------------------------------------------------------------------- + // readIgContextInfo — returns null when ig.ini has no [IG] section + // + // The guard added in the patch returns null early (before calling + // IGContext.initializeFromIni) when the file lacks the required [IG] + // section header. This prevents NPE / hang on non-IG ini files. + // ----------------------------------------------------------------------- + + @Test + fun readIgContextInfo_returnsNull_whenIgIniHasNoIgSection( + @TempDir tempDir: File, + ) { + val igIniFile = File(tempDir, "ig.ini").also { it.writeText("# not an IG ini\nfoo=bar\n") } + // Use a concrete subclass to access the protected method without overriding it + val m = + object : LibraryResolutionManager(emptyList()) { + fun testRead(f: File) = readIgContextInfo(f) + } + assertNull(m.testRead(igIniFile), "Should return null when ig.ini has no [IG] section") + } + // ----------------------------------------------------------------------- // buildNamespaceIndex — two workspace folders indexed independently // ----------------------------------------------------------------------- diff --git a/ls/server/src/test/kotlin/org/opencds/cqf/cql/ls/server/provider/ContentServiceModelInfoProviderTest.kt b/ls/server/src/test/kotlin/org/opencds/cqf/cql/ls/server/provider/ContentServiceModelInfoProviderTest.kt index ff857d7f..952ed462 100644 --- a/ls/server/src/test/kotlin/org/opencds/cqf/cql/ls/server/provider/ContentServiceModelInfoProviderTest.kt +++ b/ls/server/src/test/kotlin/org/opencds/cqf/cql/ls/server/provider/ContentServiceModelInfoProviderTest.kt @@ -2,7 +2,11 @@ package org.opencds.cqf.cql.ls.server.provider import org.hl7.cql.model.ModelIdentifier import org.hl7.elm.r1.VersionedIdentifier +import org.hl7.elm_modelinfo.r1.serializing.parseModelInfoXml +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertNotNull import org.junit.jupiter.api.Assertions.assertNull +import org.junit.jupiter.api.Assertions.assertTrue import org.junit.jupiter.api.Test import org.junit.jupiter.api.assertThrows import org.opencds.cqf.cql.ls.core.ContentService @@ -59,9 +63,329 @@ class ContentServiceModelInfoProviderTest { identifier: VersionedIdentifier, ): Set = emptySet() - override fun read(uri: URI): InputStream? = "not valid xml {{{{".byteInputStream() + override fun read(uri: URI): InputStream = "not valid xml {{{{".byteInputStream() }, ) assertThrows { provider.load(ModelIdentifier(id = "Bad")) } } + + // ----------------------------------------------------------------------- + // File URI root — constructs wrong path (treats .cql file as a directory) + // ----------------------------------------------------------------------- + // CqlEvaluator passes the .cql file URI as root (libraryUri). The provider + // appends modelinfo paths to it, producing URIs like: + // file:///.../MyLibrary.cql/c4bb-modelinfo-2.1.1.xml + // which fails with "Not a directory" at read time. + + @Test + fun load_fileUriRoot_appendsModelinfoToCqlFile() { + var capturedUri: URI? = null + val capturingService = + object : ContentService { + override fun locate( + root: URI, + identifier: VersionedIdentifier, + ): Set = emptySet() + + override fun read(uri: URI): InputStream? { + capturedUri = uri + return null + } + } + val libraryFile = URI.create("file:///workspace/input/cql/MyLibrary.cql") + + val provider = ContentServiceModelInfoProvider(libraryFile, capturingService) + provider.load(ModelIdentifier(id = "C4BB", version = "2.1.1")) + + assertNotNull(capturedUri) + assertTrue( + capturedUri!!.toString().contains("MyLibrary.cql/c4bb-modelinfo-2.1.1.xml"), + "File URI root causes the .cql file to be treated as a directory: $capturedUri", + ) + } + + // ----------------------------------------------------------------------- + // Directory URI root — constructs correct path + // ----------------------------------------------------------------------- + // CqlCompilationManager uses Uris.getHead(uri) which strips the filename, + // giving a directory root. The modelinfo path resolves correctly: + // file:///.../input/cql/c4bb-modelinfo-2.1.1.xml + + @Test + fun load_directoryUriRoot_appendsModelinfoFlat() { + var capturedUri: URI? = null + val capturingService = + object : ContentService { + override fun locate( + root: URI, + identifier: VersionedIdentifier, + ): Set = emptySet() + + override fun read(uri: URI): InputStream? { + capturedUri = uri + return null + } + } + val cqlDir = URI.create("file:///workspace/input/cql/") + + val provider = ContentServiceModelInfoProvider(cqlDir, capturingService) + provider.load(ModelIdentifier(id = "C4BB", version = "2.1.1")) + + assertNotNull(capturedUri) + assertTrue( + capturedUri!!.toString().endsWith("c4bb-modelinfo-2.1.1.xml"), + "Directory URI root produces correct flat path: $capturedUri", + ) + } + + // ----------------------------------------------------------------------- + // End-to-end: file URI root cannot resolve valid modelinfo + // ----------------------------------------------------------------------- + // Even when a content service can serve modelinfo from the correct path, + // a file URI root constructs the wrong path and never finds it. + + @Test + fun load_fileUriRoot_cannotResolveModelinfo_whenDirectoryRootCan() { + val cqlDir = URI.create("file:///workspace/input/cql/") + val libraryFile = URI.create("file:///workspace/input/cql/MyLibrary.cql") + val validModelinfo = """""" + + val correctModelinfoUri = URI.create("file:///workspace/input/cql/c4bb-modelinfo-2.1.1.xml") + + val servingService = + object : ContentService { + override fun locate( + root: URI, + identifier: VersionedIdentifier, + ): Set = emptySet() + + override fun read(uri: URI): InputStream? { + // Only serve from the exact correct path — file-root constructs + // "MyLibrary.cql/c4bb-modelinfo-2.1.1.xml" which won't match + return if (uri == correctModelinfoUri) { + validModelinfo.byteInputStream() + } else { + null + } + } + } + + // Directory root succeeds — correct flat path + val dirProvider = ContentServiceModelInfoProvider(cqlDir, servingService) + assertNotNull(dirProvider.load(ModelIdentifier(id = "C4BB", version = "2.1.1"))) + + // File root fails — wrong path (file treated as directory) + val fileProvider = ContentServiceModelInfoProvider(libraryFile, servingService) + assertNull(fileProvider.load(ModelIdentifier(id = "C4BB", version = "2.1.1"))) + } + + // ----------------------------------------------------------------------- + // formatRequiredModels — surfaces a model's declared dependencies so that + // version conflicts (e.g. C4BB requires USCore 7.0.0 while content loads + // USCore 6.1.0-derived) are visible in the logs. + // ----------------------------------------------------------------------- + + @Test + fun formatRequiredModels_rendersNameAndVersionForEachDependency() { + val modelInfo = + parseModelInfoXml( + """""", + ) + + val formatted = ContentServiceModelInfoProvider.formatRequiredModels(modelInfo) + + assertEquals("[System 1.0.0, FHIR 4.0.1, USCore 7.0.0]", formatted) + } + + @Test + fun formatRequiredModels_rendersEmptyBracketsWhenNoDependencies() { + val modelInfo = + parseModelInfoXml( + """""", + ) + + assertEquals("[]", ContentServiceModelInfoProvider.formatRequiredModels(modelInfo)) + } + + @Test + fun formatRequiredModels_rendersNameOnly_whenRequiredModelInfoVersionIsAbsent() { + val modelInfo = + parseModelInfoXml( + """""", + ) + + val formatted = ContentServiceModelInfoProvider.formatRequiredModels(modelInfo) + // System has no version → no space suffix; FHIR has a version → space + version + assertEquals("[System, FHIR 4.0.1]", formatted) + } + + // ----------------------------------------------------------------------- + // recordVersionAndDetectConflict — surfaces an actual model version conflict + // (same model at two versions on one provider instance), e.g. content loads + // USCore 6.1.0-derived while a C4BB ModelInfo requires USCore 7.0.0. + // ----------------------------------------------------------------------- + + @Test + fun recordVersionAndDetectConflict_detectsTwoVersionsOfSameModel() { + val provider = ContentServiceModelInfoProvider(root, nullContentService()) + // First sighting (e.g. requested during edit) — no conflict yet. + assertNull( + provider.recordVersionAndDetectConflict("ConflictUSCore", "6.1.0-derived", "requested"), + ) + // Second, different version (e.g. required by C4BB) — conflict. + val conflict = + provider.recordVersionAndDetectConflict("ConflictUSCore", "7.0.0", "required by C4BB") + assertNotNull(conflict) + assertTrue(conflict!!.contains("6.1.0-derived (requested)"), conflict) + assertTrue(conflict.contains("7.0.0 (required by C4BB)"), conflict) + // A genuinely different version is NOT reported as a case-only difference. + assertTrue(!conflict.contains("differ only by case"), conflict) + } + + // A case-only mismatch (6.1.0-Derived vs 6.1.0-derived) is the exact bug that made C4BB fail + // to load: the engine compares model versions case-sensitively. The message should call it out. + @Test + fun recordVersionAndDetectConflict_flagsCaseOnlyDifferenceExplicitly() { + val provider = ContentServiceModelInfoProvider(root, nullContentService()) + assertNull( + provider.recordVersionAndDetectConflict("USCore", "6.1.0-derived", "requested"), + ) + val conflict = + provider.recordVersionAndDetectConflict("USCore", "6.1.0-Derived", "required by C4BB") + assertNotNull(conflict) + assertTrue(conflict!!.contains("differ only by case"), conflict) + assertTrue(conflict.contains("6.1.0-derived (requested)"), conflict) + assertTrue(conflict.contains("6.1.0-Derived (required by C4BB)"), conflict) + } + + @Test + fun recordVersionAndDetectConflict_noConflictForSameVersionOrNull() { + val provider = ContentServiceModelInfoProvider(root, nullContentService()) + assertNull(provider.recordVersionAndDetectConflict("SameVer", "1.0.0", "requested")) + assertNull(provider.recordVersionAndDetectConflict("SameVer", "1.0.0", "required by X")) + assertNull(provider.recordVersionAndDetectConflict("NullVer", null, "requested")) + } + + // ----------------------------------------------------------------------- + // load() — version conflict warning path + // When load() is called twice for the same model at different versions, + // recordVersionAndDetectConflict returns a non-null description and the + // conflict is logged. This exercises the `.let { log.warn(...) }` branch + // on lines 89–91 of ContentServiceModelInfoProvider. + // ----------------------------------------------------------------------- + + @Test + fun load_triggersConflictWarning_whenSameModelLoadedAtDifferentVersions() { + val provider = ContentServiceModelInfoProvider(root, nullContentService()) + // First load: no conflict yet (only one version observed) + assertNull(provider.load(ModelIdentifier(id = "FHIR", version = "4.0.1"))) + // Second load at a different version: conflict is detected and logged internally. + // load() still returns null (no content), but the conflict log.warn path is exercised. + assertNull(provider.load(ModelIdentifier(id = "FHIR", version = "3.0.0"))) + // Both versions are now tracked; a third distinct version is also a conflict. + val conflict = provider.recordVersionAndDetectConflict("FHIR", "2.0.0", "test") + assertNotNull(conflict, "Three distinct FHIR versions should still produce a conflict description") + } + + // ----------------------------------------------------------------------- + // load() — requiredModelInfo loop + // When load() successfully parses a ModelInfo that declares dependencies, + // it iterates over requiredModelInfo and calls recordVersionAndDetectConflict + // for each dependency. This covers lines 114–126. + // ----------------------------------------------------------------------- + + @Test + fun load_iteratesRequiredModelInfoDependencies_whenModelInfoHasDependencies() { + val c4bbWithDeps = + """ + + + + + + """.trimIndent() + + val servingService = + object : ContentService { + override fun locate( + root: URI, + identifier: VersionedIdentifier, + ): Set = emptySet() + + override fun read(uri: URI): InputStream? = + if (uri.toString().contains("c4bb-modelinfo")) { + c4bbWithDeps.byteInputStream() + } else { + null + } + } + + val cqlDir = URI.create("file:///workspace/input/cql/") + val provider = ContentServiceModelInfoProvider(cqlDir, servingService) + val result = provider.load(ModelIdentifier(id = "C4BB", version = "2.1.1")) + + // Model loaded successfully and dependencies were processed + assertNotNull(result) + // FHIR and USCore versions are now recorded — a second load with a different + // FHIR version should be detected as a conflict. + val conflict = provider.recordVersionAndDetectConflict("FHIR", "3.0.0", "external") + assertNotNull(conflict, "FHIR 3.0.0 should conflict with the 4.0.1 seen in requiredModelInfo") + } + + @Test + fun load_logsConflictForRequiredDependency_whenVersionConflictsWithPreviousObservation() { + val c4bbRequiringOldFhir = + """ + + + + + """.trimIndent() + + val servingService = + object : ContentService { + override fun locate( + root: URI, + identifier: VersionedIdentifier, + ): Set = emptySet() + + override fun read(uri: URI): InputStream? = + if (uri.toString().contains("c4bb-modelinfo")) { + c4bbRequiringOldFhir.byteInputStream() + } else { + null + } + } + + val cqlDir = URI.create("file:///workspace/input/cql/") + val provider = ContentServiceModelInfoProvider(cqlDir, servingService) + + // First, record FHIR 4.0.1 as observed (simulates it having been requested earlier) + provider.recordVersionAndDetectConflict("FHIR", "4.0.1", "requested") + + // Now load C4BB, whose requiredModelInfo declares FHIR 3.0.0 — a conflict with 4.0.1 + val result = provider.load(ModelIdentifier(id = "C4BB", version = "2.1.1")) + + // C4BB itself loaded successfully + assertNotNull(result) + // The provider's state now reflects both FHIR 4.0.1 and 3.0.0 — verify the conflict + provider.recordVersionAndDetectConflict("FHIR", "3.0.0", "required by C4BB") + // Recording the same version again doesn't add a new conflict (idempotent), + // but the map still has two entries from the prior calls + assertNotNull( + provider.recordVersionAndDetectConflict("FHIR", "5.0.0", "other"), + "A third FHIR version should still be detected as a conflict", + ) + } + + // Version tracking is instance state: a fresh provider (one per compilation) starts empty and + // never inherits versions observed by a previous instance/run. + @Test + fun recordVersionAndDetectConflict_separateInstancesDoNotShareState() { + val a = ContentServiceModelInfoProvider(root, nullContentService()) + val b = ContentServiceModelInfoProvider(root, nullContentService()) + assertNull(a.recordVersionAndDetectConflict("M", "1.0.0", "requested")) + // b never saw 1.0.0, so recording a different version on b is not a conflict. + assertNull(b.recordVersionAndDetectConflict("M", "2.0.0", "requested")) + } } diff --git a/ls/server/src/test/kotlin/org/opencds/cqf/cql/ls/server/provider/FederatedLibrarySourceProviderTest.kt b/ls/server/src/test/kotlin/org/opencds/cqf/cql/ls/server/provider/FederatedLibrarySourceProviderTest.kt index 8bc540b2..9b233caf 100644 --- a/ls/server/src/test/kotlin/org/opencds/cqf/cql/ls/server/provider/FederatedLibrarySourceProviderTest.kt +++ b/ls/server/src/test/kotlin/org/opencds/cqf/cql/ls/server/provider/FederatedLibrarySourceProviderTest.kt @@ -2,6 +2,7 @@ package org.opencds.cqf.cql.ls.server.provider import org.cqframework.fhir.npm.NpmProcessor import org.hl7.elm.r1.VersionedIdentifier +import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Assertions.assertNotNull import org.junit.jupiter.api.Assertions.assertNull import org.junit.jupiter.api.Test @@ -120,6 +121,39 @@ class FederatedLibrarySourceProviderTest { assertNull(provider.getLibrarySource(VersionedIdentifier().withId("LibA"))) } + // ----------------------------------------------------------------------- + // Root URI passthrough — FederatedLibrarySourceProvider does NOT normalize + // file URIs to directory URIs (unlike CqlCompilationManager which calls + // Uris.getHead first). When CqlEvaluator passes libraryUri (a .cql file), + // it arrives at locate() as-is — no directory normalization. + // ----------------------------------------------------------------------- + + @Test + fun getLibrarySource_passesRootUriThroughWithoutNormalization() { + var capturedRoot: URI? = null + val capturingCs = + object : ContentService { + override fun locate( + root: URI, + identifier: VersionedIdentifier, + ): Set { + capturedRoot = root + return emptySet() + } + + override fun read(uri: URI): InputStream? = null + } + + val fileUri = URI.create("file:///workspace/input/cql/MyLibrary.cql") + val provider = FederatedLibrarySourceProvider(fileUri, capturingCs, null) + provider.getLibrarySource(VersionedIdentifier().withId("Test")) + + // Root is passed through unchanged — no getHead normalization. + // This means locate() receives a file URI, forcing tier1 to fail + // (BFS requires a directory) and relying entirely on tier2 fallback. + assertEquals(fileUri, capturedRoot, "Root must be passed through without normalization") + } + // ----------------------------------------------------------------------- // Helpers // ----------------------------------------------------------------------- diff --git a/ls/server/src/test/resources/org/opencds/cqf/cql/ls/server/UsesCustomModel.cql b/ls/server/src/test/resources/org/opencds/cqf/cql/ls/server/UsesCustomModel.cql new file mode 100644 index 00000000..2a003c80 --- /dev/null +++ b/ls/server/src/test/resources/org/opencds/cqf/cql/ls/server/UsesCustomModel.cql @@ -0,0 +1,8 @@ +library UsesCustomModel version '1' + +// Declares a model that is neither bundled nor in npm, forcing the +// ContentServiceModelInfoProvider to be consulted for its model info. +using Custom version '1.0.0' + +define "X": + 1 diff --git a/pom.xml b/pom.xml index 944db98a..54f87b3b 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.opencds.cqf.cql.ls cql-ls pom - 4.10.0-SNAPSHOT + 4.10.0 CQL Language Server A Language Server for CQL implementing the LSP