From 51934de457b17331df7c6453d813f0a83f7b949f Mon Sep 17 00:00:00 2001 From: Grahame Grieve Date: Mon, 9 Sep 2024 17:47:33 +0800 Subject: [PATCH 01/12] Fix bug processing value set includes / excludes that are just value sets (no system value) --- .../fhir/r5/context/BaseWorkerContext.java | 8 ++- .../expansion/ValueSetExpander.java | 53 ++++++++++--------- 2 files changed, 33 insertions(+), 28 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 e517398be..e33cc0bfa 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 @@ -934,10 +934,14 @@ public abstract class BaseWorkerContext extends I18nBase implements IWorkerConte throw new Error(formatMessage(I18nConstants.NO_VALUE_SET_IN_URL)); } for (ConceptSetComponent inc : vs.getCompose().getInclude()) { - codeSystemsUsed.add(inc.getSystem()); + if (inc.hasSystem()) { + codeSystemsUsed.add(inc.getSystem()); + } } for (ConceptSetComponent inc : vs.getCompose().getExclude()) { - codeSystemsUsed.add(inc.getSystem()); + if (inc.hasSystem()) { + codeSystemsUsed.add(inc.getSystem()); + } } CacheToken cacheToken = txCache.generateExpandToken(vs, hierarchical); diff --git a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/terminologies/expansion/ValueSetExpander.java b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/terminologies/expansion/ValueSetExpander.java index 28058becb..7e1cfbbf7 100644 --- a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/terminologies/expansion/ValueSetExpander.java +++ b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/terminologies/expansion/ValueSetExpander.java @@ -606,33 +606,35 @@ public class ValueSetExpander extends ValueSetProcessBase { excludeCodes(wc, importValueSetForExclude(wc, imp.getValue(), exp, expParams, false, vs).getExpansion()); } - CodeSystem cs = context.fetchSupplementedCodeSystem(exc.getSystem()); - if ((cs == null || cs.getContent() != CodeSystemContentMode.COMPLETE) && context.supportsSystem(exc.getSystem(), opContext.getOptions().getFhirVersion())) { - ValueSetExpansionOutcome vse = context.expandVS(exc, false, false); - ValueSet valueset = vse.getValueset(); - if (valueset == null) - throw failTSE("Error Expanding ValueSet: "+vse.getError()); - excludeCodes(wc, valueset.getExpansion()); - return; - } - - for (ConceptReferenceComponent c : exc.getConcept()) { - excludeCode(wc, exc.getSystem(), c.getCode()); - } - - if (exc.getFilter().size() > 0) { - if (cs.getContent() == CodeSystemContentMode.FRAGMENT) { - addFragmentWarning(exp, cs); + if (exc.hasSystem()) { + CodeSystem cs = context.fetchSupplementedCodeSystem(exc.getSystem()); + if ((cs == null || cs.getContent() != CodeSystemContentMode.COMPLETE) && context.supportsSystem(exc.getSystem(), opContext.getOptions().getFhirVersion())) { + ValueSetExpansionOutcome vse = context.expandVS(exc, false, false); + ValueSet valueset = vse.getValueset(); + if (valueset == null) + throw failTSE("Error Expanding ValueSet: "+vse.getError()); + excludeCodes(wc, valueset.getExpansion()); + return; } - List filters = new ArrayList<>(); - for (int i = 1; i < exc.getFilter().size(); i++) { - WorkingContext wc1 = new WorkingContext(); - filters.add(wc1); - processFilter(exc, exp, expParams, null, cs, false, exc.getFilter().get(i), wc1, null, true); + + for (ConceptReferenceComponent c : exc.getConcept()) { + excludeCode(wc, exc.getSystem(), c.getCode()); + } + + if (exc.getFilter().size() > 0) { + if (cs.getContent() == CodeSystemContentMode.FRAGMENT) { + addFragmentWarning(exp, cs); + } + List filters = new ArrayList<>(); + for (int i = 1; i < exc.getFilter().size(); i++) { + WorkingContext wc1 = new WorkingContext(); + filters.add(wc1); + processFilter(exc, exp, expParams, null, cs, false, exc.getFilter().get(i), wc1, null, true); + } + ConceptSetFilterComponent fc = exc.getFilter().get(0); + WorkingContext wc1 = dwc; + processFilter(exc, exp, expParams, null, cs, false, fc, wc1, filters, true); } - ConceptSetFilterComponent fc = exc.getFilter().get(0); - WorkingContext wc1 = dwc; - processFilter(exc, exp, expParams, null, cs, false, fc, wc1, filters, true); } } @@ -728,7 +730,6 @@ public class ValueSetExpander extends ValueSetProcessBase { expParams = makeDefaultExpansion(); altCodeParams.seeParameters(expParams); altCodeParams.seeValueSet(source); - source.checkNoModifiers("ValueSet", "expanding"); focus = source.copy(); focus.setIdBase(null); From 810f4b8e3186a392f7b78a29738c767e3c4125c0 Mon Sep 17 00:00:00 2001 From: Grahame Grieve Date: Mon, 9 Sep 2024 17:48:01 +0800 Subject: [PATCH 02/12] fix value set rendering creating wrong references --- .../fhir/r5/renderers/ValueSetRenderer.java | 45 +++++++++---------- 1 file changed, 22 insertions(+), 23 deletions(-) diff --git a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/renderers/ValueSetRenderer.java b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/renderers/ValueSetRenderer.java index eb303d742..4a78a67c8 100644 --- a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/renderers/ValueSetRenderer.java +++ b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/renderers/ValueSetRenderer.java @@ -754,16 +754,7 @@ public class ValueSetRenderer extends TerminologyRenderer { if (ref == null) { ref = (String) cs.getWebPath(); } - if (ref == null && cs.hasUserData("webroot")) { - ref = (String) cs.getUserData("webroot"); - } - if (ref == null) { - return "?ngen-14?.html"; - } - if (!ref.contains(".html")) { - ref = ref + ".html"; - } - return ref.replace("\\", "/"); + return ref == null ? null : ref.replace("\\", "/"); } private void scanForDesignations(ValueSetExpansionContainsComponent c, List langs, Map designations) { @@ -922,14 +913,18 @@ public class ValueSetRenderer extends TerminologyRenderer { td.addText(code); } else { String href = context.fixReference(getCsRef(e)); - if (href.contains("#")) - href = href + "-"+Utilities.nmtokenize(code); - else - href = href + "#"+e.getId()+"-"+Utilities.nmtokenize(code); - if (isAbstract) - td.ah(context.prefixLocalHref(href)).setAttribute("title", context.formatPhrase(RenderingContext.VS_ABSTRACT_CODE_HINT)).i().addText(code); - else - td.ah(context.prefixLocalHref(href)).addText(code); + if (href == null) { + td.code().tx(code); + } else { + if (href.contains("#")) + href = href + "-"+Utilities.nmtokenize(code); + else + href = href + "#"+e.getId()+"-"+Utilities.nmtokenize(code); + if (isAbstract) + td.ah(context.prefixLocalHref(href)).setAttribute("title", context.formatPhrase(RenderingContext.VS_ABSTRACT_CODE_HINT)).i().addText(code); + else + td.ah(context.prefixLocalHref(href)).addText(code); + } } } @@ -1247,11 +1242,15 @@ public class ValueSetRenderer extends TerminologyRenderer { wli.tx(f.getProperty()+" "+describe(f.getOp())+" "); if (e != null && codeExistsInValueSet(e, f.getValue())) { String href = getContext().fixReference(getCsRef(e)); - if (href.contains("#")) - href = href + "-"+Utilities.nmtokenize(f.getValue()); - else - href = href + "#"+e.getId()+"-"+Utilities.nmtokenize(f.getValue()); - wli.ah(context.prefixLocalHref(href)).addText(f.getValue()); + if (href == null) { + wli.code().tx(f.getValue()); + } else { + if (href.contains("#")) + href = href + "-"+Utilities.nmtokenize(f.getValue()); + else + href = href + "#"+e.getId()+"-"+Utilities.nmtokenize(f.getValue()); + wli.ah(context.prefixLocalHref(href)).addText(f.getValue()); + } } else if (inc.hasSystem()) { wli.addText(f.getValue()); ValidationResult vr = getContext().getWorker().validateCode(getContext().getTerminologyServiceOptions(), inc.getSystem(), inc.getVersion(), f.getValue(), null); From cfdd1f93803ecffdb7c4f7aab619bc927f1db3a3 Mon Sep 17 00:00:00 2001 From: Grahame Grieve Date: Mon, 9 Sep 2024 17:48:46 +0800 Subject: [PATCH 03/12] Update SQL-On-FHIR implementation for latest cases, and clone test cases to general test care repository --- .../r5/renderers/utils/RenderingContext.java | 3 + .../org/hl7/fhir/r5/utils/sql/Runner.java | 65 ++++++--- .../hl7/fhir/r5/utils/sql/StorageJson.java | 17 +-- .../org/hl7/fhir/r5/utils/sql/Validator.java | 131 ++++++++++++------ .../org/hl7/fhir/r5/sql/SQLOnFhirTests.java | 29 ++-- 5 files changed, 157 insertions(+), 88 deletions(-) diff --git a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/renderers/utils/RenderingContext.java b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/renderers/utils/RenderingContext.java index 572523a1e..687d7c672 100644 --- a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/renderers/utils/RenderingContext.java +++ b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/renderers/utils/RenderingContext.java @@ -554,6 +554,9 @@ public class RenderingContext extends RenderingI18nContext { } public String fixReference(String ref) { + if (ref == null) { + return null; + } if (!Utilities.isAbsoluteUrl(ref)) { return (localPrefix == null ? "" : localPrefix)+ref; } diff --git a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/utils/sql/Runner.java b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/utils/sql/Runner.java index 432fff492..aa5d8b4e1 100644 --- a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/utils/sql/Runner.java +++ b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/utils/sql/Runner.java @@ -103,7 +103,7 @@ public class Runner implements IEvaluationContext { for (JsonObject w : vd.getJsonObjects("where")) { String expr = w.asString("path"); ExpressionNode node = fpe.parse(expr); - boolean pass = fpe.evaluateToBoolean(null, b, b, b, node); + boolean pass = fpe.evaluateToBoolean(vd, b, b, b, node); if (!pass) { ok = false; break; @@ -114,7 +114,7 @@ public class Runner implements IEvaluationContext { rows.add(new ArrayList()); for (JsonObject select : vd.getJsonObjects("select")) { - executeSelect(select, b, rows); + executeSelect(vd, select, b, rows); } for (List row : rows) { storage.addRow(store, row); @@ -124,14 +124,14 @@ public class Runner implements IEvaluationContext { storage.finish(store); } - private void executeSelect(JsonObject select, Base b, List> rows) { + private void executeSelect(JsonObject vd, JsonObject select, Base b, List> rows) { List focus = new ArrayList<>(); if (select.has("forEach")) { - focus.addAll(executeForEach(select, b)); + focus.addAll(executeForEach(vd, select, b)); } else if (select.has("forEachOrNull")) { - focus.addAll(executeForEachOrNull(select, b)); + focus.addAll(executeForEachOrNull(vd, select, b)); if (focus.isEmpty()) { List columns = (List) select.getUserData("columns"); for (List row : rows) { @@ -159,20 +159,20 @@ public class Runner implements IEvaluationContext { List> rowsToAdd = cloneRows(tempRows); for (JsonObject column : select.getJsonObjects("column")) { - executeColumn(column, f, rowsToAdd); + executeColumn(vd, column, f, rowsToAdd); } for (JsonObject sub : select.getJsonObjects("select")) { - executeSelect(sub, f, rowsToAdd); + executeSelect(vd, sub, f, rowsToAdd); } - executeUnionAll(select.getJsonObjects("unionAll"), f, rowsToAdd); + executeUnionAll(vd, select.getJsonObjects("unionAll"), f, rowsToAdd); rows.addAll(rowsToAdd); } } - private void executeUnionAll(List unionList, Base b, List> rows) { + private void executeUnionAll(JsonObject vd, List unionList, Base b, List> rows) { if (unionList.isEmpty()) { return; } @@ -183,7 +183,7 @@ public class Runner implements IEvaluationContext { for (JsonObject union : unionList) { List> tempRows = new ArrayList<>(); tempRows.addAll(sourceRows); - executeSelect(union, b, tempRows); + executeSelect(vd, union, b, tempRows); rows.addAll(tempRows); } } @@ -204,25 +204,25 @@ public class Runner implements IEvaluationContext { return list; } - private List executeForEach(JsonObject focus, Base b) { + private List executeForEach(JsonObject vd, JsonObject focus, Base b) { ExpressionNode n = (ExpressionNode) focus.getUserData("forEach"); List result = new ArrayList<>(); - result.addAll(fpe.evaluate(b, n)); + result.addAll(fpe.evaluate(vd, b, n)); return result; } - private List executeForEachOrNull(JsonObject focus, Base b) { + private List executeForEachOrNull(JsonObject vd, JsonObject focus, Base b) { ExpressionNode n = (ExpressionNode) focus.getUserData("forEachOrNull"); List result = new ArrayList<>(); - result.addAll(fpe.evaluate(b, n)); + result.addAll(fpe.evaluate(vd, b, n)); return result; } - private void executeColumn(JsonObject column, Base b, List> rows) { + private void executeColumn(JsonObject vd, JsonObject column, Base b, List> rows) { ExpressionNode n = (ExpressionNode) column.getUserData("path"); List bl2 = new ArrayList<>(); if (b != null) { - bl2.addAll(fpe.evaluate(b, n)); + bl2.addAll(fpe.evaluate(vd, b, n)); } Column col = (Column) column.getUserData("column"); if (col == null) { @@ -344,14 +344,43 @@ public class Runner implements IEvaluationContext { @Override public List resolveConstant(FHIRPathEngine engine, Object appContext, String name, boolean beforeContext, boolean explicitConstant) throws PathEngineException { - throw new Error("Not implemented yet: resolveConstant"); + List list = new ArrayList(); + if (explicitConstant) { + JsonObject vd = (JsonObject) appContext; + JsonObject constant = findConstant(vd, name); + if (constant != null) { + Base b = (Base) constant.getUserData("value"); + if (b != null) { + list.add(b); + } + } + } + return list; } @Override public TypeDetails resolveConstantType(FHIRPathEngine engine, Object appContext, String name, boolean explicitConstant) throws PathEngineException { - throw new Error("Not implemented yet: resolveConstantType"); + if (explicitConstant) { + JsonObject vd = (JsonObject) appContext; + JsonObject constant = findConstant(vd, name.substring(1)); + if (constant != null) { + Base b = (Base) constant.getUserData("value"); + if (b != null) { + return new TypeDetails(CollectionStatus.SINGLETON, b.fhirType()); + } + } + } + return null; } + private JsonObject findConstant(JsonObject vd, String name) { + for (JsonObject o : vd.getJsonObjects("constant")) { + if (name.equals(o.asString("name"))) { + return o; + } + } + return null; + } @Override public boolean log(String argument, List focus) { throw new Error("Not implemented yet: log"); diff --git a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/utils/sql/StorageJson.java b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/utils/sql/StorageJson.java index d0e3aaeed..d76404c03 100644 --- a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/utils/sql/StorageJson.java +++ b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/utils/sql/StorageJson.java @@ -2,6 +2,7 @@ package org.hl7.fhir.r5.utils.sql; import java.util.List; +import org.hl7.fhir.exceptions.FHIRException; import org.hl7.fhir.r5.model.Base; import org.hl7.fhir.utilities.json.model.JsonArray; import org.hl7.fhir.utilities.json.model.JsonBoolean; @@ -33,16 +34,16 @@ public class StorageJson implements Storage { JsonObject row = new JsonObject(); rows.add(row); for (Cell cell : cells) { - if (cell.getValues().size() == 0) { - row.add(cell.getColumn().getName(), new JsonNull()); - } else if (cell.getValues().size() == 1) { - row.add(cell.getColumn().getName(), makeJsonNode(cell.getValues().get(0))); - } else { + if (cell.getColumn().isColl() || cell.getValues().size() > 1) { JsonArray arr = new JsonArray(); - row.add(cell.getColumn().getName(), arr); + row.add(cell.getColumn().getName(), arr); for (Value value : cell.getValues()) { arr.add(makeJsonNode(value)); - } + } + } else if (cell.getValues().size() == 0) { + row.add(cell.getColumn().getName(), new JsonNull()); + } else { + row.add(cell.getColumn().getName(), makeJsonNode(cell.getValues().get(0))); } } } @@ -87,7 +88,7 @@ public class StorageJson implements Storage { @Override public String getKeyForSourceResource(Base res) { - return res.getIdBase(); + return res.fhirType()+"/"+res.getIdBase(); } @Override diff --git a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/utils/sql/Validator.java b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/utils/sql/Validator.java index 669fa32de..00c862b5d 100644 --- a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/utils/sql/Validator.java +++ b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/utils/sql/Validator.java @@ -11,6 +11,27 @@ import org.hl7.fhir.r5.context.IWorkerContext; import org.hl7.fhir.r5.fhirpath.ExpressionNode; import org.hl7.fhir.r5.fhirpath.FHIRPathEngine; import org.hl7.fhir.r5.fhirpath.TypeDetails; +import org.hl7.fhir.r5.formats.JsonParser; +import org.hl7.fhir.r5.model.Base64BinaryType; +import org.hl7.fhir.r5.model.BooleanType; +import org.hl7.fhir.r5.model.CanonicalType; +import org.hl7.fhir.r5.model.CodeType; +import org.hl7.fhir.r5.model.DateTimeType; +import org.hl7.fhir.r5.model.DateType; +import org.hl7.fhir.r5.model.DecimalType; +import org.hl7.fhir.r5.model.IdType; +import org.hl7.fhir.r5.model.InstantType; +import org.hl7.fhir.r5.model.Integer64Type; +import org.hl7.fhir.r5.model.IntegerType; +import org.hl7.fhir.r5.model.OidType; +import org.hl7.fhir.r5.model.PositiveIntType; +import org.hl7.fhir.r5.model.PrimitiveType; +import org.hl7.fhir.r5.model.StringType; +import org.hl7.fhir.r5.model.TimeType; +import org.hl7.fhir.r5.model.UnsignedIntType; +import org.hl7.fhir.r5.model.UriType; +import org.hl7.fhir.r5.model.UrlType; +import org.hl7.fhir.r5.model.UuidType; import org.hl7.fhir.r5.fhirpath.ExpressionNode.CollectionStatus; import org.hl7.fhir.r5.fhirpath.FHIRPathEngine.IssueMessage; import org.hl7.fhir.utilities.Utilities; @@ -99,7 +120,7 @@ public class Validator { i = 0; if (checkAllObjects(path, viewDefinition, "where")) { for (JsonObject where : viewDefinition.getJsonObjects("where")) { - checkWhere(path+".where["+i+"]", where); + checkWhere(viewDefinition, path+".where["+i+"]", where); i++; } } @@ -108,7 +129,7 @@ public class Validator { i = 0; if (checkAllObjects(path, viewDefinition, "select")) { for (JsonObject select : viewDefinition.getJsonObjects("select")) { - columns.addAll(checkSelect(path+".select["+i+"]", select, t)); + columns.addAll(checkSelect(viewDefinition, path+".select["+i+"]", select, t)); i++; } if (i == 0) { @@ -119,15 +140,15 @@ public class Validator { } } - private List checkSelect(String path, JsonObject select, TypeDetails t) { + private List checkSelect(JsonObject vd, String path, JsonObject select, TypeDetails t) { List columns = new ArrayList<>(); select.setUserData("columns", columns); checkProperties(select, path, "column", "select", "forEach", "forEachOrNull", "unionAll"); if (select.has("forEach")) { - t = checkForEach(path, select, select.get("forEach"), t); + t = checkForEach(vd, path, select, select.get("forEach"), t); } else if (select.has("forEachOrNull")) { - t = checkForEachOrNull(path, select, select.get("forEachOrNull"), t); + t = checkForEachOrNull(vd, path, select, select.get("forEachOrNull"), t); } if (t != null) { @@ -142,7 +163,7 @@ public class Validator { if (!(e instanceof JsonObject)) { error(path+".column["+i+"]", a, "column["+i+"] is a "+e.type().toName()+" not an object", IssueType.INVALID); } else { - columns.add(checkColumn(path+".column["+i+"]", (JsonObject) e, t)); + columns.add(checkColumn(vd, path+".column["+i+"]", (JsonObject) e, t)); } } } @@ -158,14 +179,14 @@ public class Validator { if (!(e instanceof JsonObject)) { error(path+".select["+i+"]", e, "select["+i+"] is not an object", IssueType.INVALID); } else { - columns.addAll(checkSelect(path+".select["+i+"]", (JsonObject) e, t)); + columns.addAll(checkSelect(vd, path+".select["+i+"]", (JsonObject) e, t)); } } } } if (select.has("unionAll")) { - columns.addAll(checkUnion(path, select, select.get("unionAll"), t)); + columns.addAll(checkUnion(vd, path, select, select.get("unionAll"), t)); } if (columns.isEmpty()) { error(path, select, "The select has no columns or selects", IssueType.REQUIRED); @@ -191,7 +212,7 @@ public class Validator { } } - private List checkUnion(String path, JsonObject focus, JsonElement expression, TypeDetails t) { + private List checkUnion(JsonObject vd, String path, JsonObject focus, JsonElement expression, TypeDetails t) { JsonElement a = focus.get("unionAll"); if (!(a instanceof JsonArray)) { error(path+".unionAll", a, "union is not an array", IssueType.INVALID); @@ -203,7 +224,7 @@ public class Validator { if (!(e instanceof JsonObject)) { error(path+".unionAll["+i+"]", e, "unionAll["+i+"] is not an object", IssueType.INVALID); } else { - unionColumns.add(checkSelect(path+".unionAll["+i+"]", (JsonObject) e, t)); + unionColumns.add(checkSelect(vd, path+".unionAll["+i+"]", (JsonObject) e, t)); } i++; } @@ -242,7 +263,7 @@ public class Validator { } } - private Column checkColumn(String path, JsonObject column, TypeDetails t) { + private Column checkColumn(JsonObject vd, String path, JsonObject column, TypeDetails t) { checkProperties(column, path, "path", "name", "description", "collection", "type", "tag"); if (!column.has("path")) { @@ -260,7 +281,7 @@ public class Validator { try { node = fpe.parse(expr); column.setUserData("path", node); - td = fpe.checkOnTypes(null, resourceName, t, node, warnings); + td = fpe.checkOnTypes(vd, resourceName, t, node, warnings); } catch (Exception e) { error(path, expression, e.getMessage(), IssueType.INVALID); } @@ -296,25 +317,31 @@ public class Validator { // ok, name is sorted! if (columnName != null) { column.setUserData("name", columnName); - boolean isColl = (td.getCollectionStatus() != CollectionStatus.SINGLETON); + boolean isColl = false; if (column.has("collection")) { JsonElement collectionJ = column.get("collection"); if (!(collectionJ instanceof JsonBoolean)) { error(path+".collection", collectionJ, "collection is not a boolean", IssueType.INVALID); } else { boolean collection = collectionJ.asJsonBoolean().asBoolean(); - if (!collection && isColl) { - isColl = false; - warning(path, column, "collection is false, but the path statement(s) might return multiple values for the column '"+columnName+"' some inputs"); + if (collection) { + isColl = true; } } } if (isColl) { + if (td.getCollectionStatus() == CollectionStatus.SINGLETON) { + hint(path, column, "collection is true, but the path statement(s) can only return single values for the column '"+columnName+"'"); + } + } else { if (arrays == null) { warning(path, expression, "The column '"+columnName+"' appears to be a collection based on it's path. Collections are not supported in all execution contexts"); } else if (!arrays) { warning(path, expression, "The column '"+columnName+"' appears to be a collection based on it's path, but this is not allowed in the current execution context"); } + if (td.getCollectionStatus() != CollectionStatus.SINGLETON) { + warning(path, column, "collection is not true, but the path statement(s) might return multiple values for the column '"+columnName+"' for some inputs"); + } } Set types = new HashSet<>(); if (node.isNullSet()) { @@ -330,7 +357,7 @@ public class Validator { if (typeJ instanceof JsonString) { String type = typeJ.asString(); if (!td.hasType(type)) { - error(path+".type", typeJ, "The path expression does not return a value of the type '"+type, IssueType.VALUE); + error(path+".type", typeJ, "The path expression does not return a value of the type '"+type+"' - found "+td.describe(), IssueType.VALUE); } else { types.clear(); types.add(simpleType(type)); @@ -377,6 +404,8 @@ public class Validator { case "integer": return ColumnKind.Integer; case "decimal": return ColumnKind.Decimal; case "string": return ColumnKind.String; + case "id": return ColumnKind.String; + case "code": return ColumnKind.String; case "base64Binary": return ColumnKind.Binary; case "time": return ColumnKind.Time; default: return ColumnKind.Complex; @@ -384,7 +413,7 @@ public class Validator { } private boolean isSimpleType(String type) { - return Utilities.existsInList(type, "dateTime", "boolean", "integer", "decimal", "string", "base64Binary"); + return Utilities.existsInList(type, "dateTime", "boolean", "integer", "decimal", "string", "base64Binary", "id", "code", "date", "time"); } private String simpleType(String type) { @@ -413,7 +442,7 @@ public class Validator { return type; } - private TypeDetails checkForEach(String path, JsonObject focus, JsonElement expression, TypeDetails t) { + private TypeDetails checkForEach(JsonObject vd, String path, JsonObject focus, JsonElement expression, TypeDetails t) { if (!(expression instanceof JsonString)) { error(path+".forEach", expression, "forEach is not a string", IssueType.INVALID); return null; @@ -425,7 +454,7 @@ public class Validator { try { ExpressionNode n = fpe.parse(expr); focus.setUserData("forEach", n); - td = fpe.checkOnTypes(null, resourceName, t, n, warnings); + td = fpe.checkOnTypes(vd, resourceName, t, n, warnings); } catch (Exception e) { error(path, expression, e.getMessage(), IssueType.INVALID); } @@ -438,7 +467,7 @@ public class Validator { } } - private TypeDetails checkForEachOrNull(String path, JsonObject focus, JsonElement expression, TypeDetails t) { + private TypeDetails checkForEachOrNull(JsonObject vd, String path, JsonObject focus, JsonElement expression, TypeDetails t) { if (!(expression instanceof JsonString)) { error(path+".forEachOrNull", expression, "forEachOrNull is not a string", IssueType.INVALID); return null; @@ -450,7 +479,7 @@ public class Validator { try { ExpressionNode n = fpe.parse(expr); focus.setUserData("forEachOrNull", n); - td = fpe.checkOnTypes(null, resourceName, t, n, warnings); + td = fpe.checkOnTypes(vd, resourceName, t, n, warnings); } catch (Exception e) { error(path, expression, e.getMessage(), IssueType.INVALID); } @@ -477,69 +506,79 @@ public class Validator { } } if (constant.has("valueBase64Binary")) { - checkIsString(path, constant, "valueBase64Binary"); + checkIsString(path, constant, "valueBase64Binary", new Base64BinaryType()); } else if (constant.has("valueBoolean")) { - checkIsBoolean(path, constant, "valueBoolean"); + checkIsBoolean(path, constant, "valueBoolean", new BooleanType()); } else if (constant.has("valueCanonical")) { - checkIsString(path, constant, "valueCanonical"); + checkIsString(path, constant, "valueCanonical", new CanonicalType()); } else if (constant.has("valueCode")) { - checkIsString(path, constant, "valueCode"); + checkIsString(path, constant, "valueCode", new CodeType()); } else if (constant.has("valueDate")) { - checkIsString(path, constant, "valueDate"); + checkIsString(path, constant, "valueDate", new DateType()); } else if (constant.has("valueDateTime")) { - checkIsString(path, constant, "valueDateTime"); + checkIsString(path, constant, "valueDateTime", new DateTimeType()); } else if (constant.has("valueDecimal")) { - checkIsNumber(path, constant, "valueDecimal"); + checkIsNumber(path, constant, "valueDecimal", new DecimalType()); } else if (constant.has("valueId")) { - checkIsString(path, constant, "valueId"); + checkIsString(path, constant, "valueId", new IdType()); } else if (constant.has("valueInstant")) { - checkIsString(path, constant, "valueInstant"); + checkIsString(path, constant, "valueInstant", new InstantType()); } else if (constant.has("valueInteger")) { - checkIsNumber(path, constant, "valueInteger"); + checkIsNumber(path, constant, "valueInteger", new IntegerType()); } else if (constant.has("valueInteger64")) { - checkIsNumber(path, constant, "valueInteger64"); + checkIsNumber(path, constant, "valueInteger64", new Integer64Type()); } else if (constant.has("valueOid")) { - checkIsString(path, constant, "valueOid"); + checkIsString(path, constant, "valueOid", new OidType()); } else if (constant.has("valueString")) { - checkIsString(path, constant, "valueString"); + checkIsString(path, constant, "valueString", new StringType()); } else if (constant.has("valuePositiveInt")) { - checkIsNumber(path, constant, "valuePositiveInt"); + checkIsNumber(path, constant, "valuePositiveInt", new PositiveIntType()); } else if (constant.has("valueTime")) { - checkIsString(path, constant, "valueTime"); + checkIsString(path, constant, "valueTime", new TimeType()); } else if (constant.has("valueUnsignedInt")) { - checkIsNumber(path, constant, "valueUnsignedInt"); + checkIsNumber(path, constant, "valueUnsignedInt", new UnsignedIntType()); } else if (constant.has("valueUri")) { - checkIsString(path, constant, "valueUri"); + checkIsString(path, constant, "valueUri", new UriType()); } else if (constant.has("valueUrl")) { - checkIsString(path, constant, "valueUrl"); + checkIsString(path, constant, "valueUrl", new UrlType()); } else if (constant.has("valueUuid")) { - checkIsString(path, constant, "valueUuid"); + checkIsString(path, constant, "valueUuid", new UuidType()); } else { error(path, constant, "No value found", IssueType.REQUIRED); } } - private void checkIsString(String path, JsonObject constant, String name) { + private void checkIsString(String path, JsonObject constant, String name, PrimitiveType value) { JsonElement j = constant.get(name); if (!(j instanceof JsonString)) { error(path+"."+name, j, name+" must be a string", IssueType.INVALID); + } else { + value.setValueAsString(j.asString()); + constant.setUserData("value", value); } } - private void checkIsBoolean(String path, JsonObject constant, String name) { + private void checkIsBoolean(String path, JsonObject constant, String name, PrimitiveType value) { JsonElement j = constant.get(name); if (!(j instanceof JsonBoolean)) { error(path+"."+name, j, name+" must be a boolean", IssueType.INVALID); + } else { + value.setValueAsString(j.asString()); + constant.setUserData("value", value); } } - private void checkIsNumber(String path, JsonObject constant, String name) { + private void checkIsNumber(String path, JsonObject constant, String name, PrimitiveType value) { JsonElement j = constant.get(name); if (!(j instanceof JsonNumber)) { error(path+"."+name, j, name+" must be a number", IssueType.INVALID); + } else { + value.setValueAsString(j.asString()); + constant.setUserData("value", value); } } - private void checkWhere(String path, JsonObject where) { + + private void checkWhere(JsonObject vd, String path, JsonObject where) { checkProperties(where, path, "path", "description"); String expr = where.asString("path"); @@ -553,7 +592,7 @@ public class Validator { try { ExpressionNode n = fpe.parse(expr); where.setUserData("path", n); - td = fpe.checkOnTypes(null, resourceName, types, n, warnings); + td = fpe.checkOnTypes(vd, resourceName, types, n, warnings); } catch (Exception e) { error(path, where.get("path"), e.getMessage(), IssueType.INVALID); } diff --git a/org.hl7.fhir.r5/src/test/java/org/hl7/fhir/r5/sql/SQLOnFhirTests.java b/org.hl7.fhir.r5/src/test/java/org/hl7/fhir/r5/sql/SQLOnFhirTests.java index 9dba811a8..d650980fb 100644 --- a/org.hl7.fhir.r5/src/test/java/org/hl7/fhir/r5/sql/SQLOnFhirTests.java +++ b/org.hl7.fhir.r5/src/test/java/org/hl7/fhir/r5/sql/SQLOnFhirTests.java @@ -83,23 +83,21 @@ public class SQLOnFhirTests { this.resources = resources; this.testCase = testCase; } - } public static Stream data() throws ParserConfigurationException, SAXException, IOException { List objects = new ArrayList<>(); - File dir = ManagedFileAccess.file("/Users/grahamegrieve/work/sql-on-fhir-v2/tests/content"); - for (File f : dir.listFiles()) { - if (f.getName().endsWith(".json")) { - JsonObject json = JsonParser.parseObject(f); - String name1 = f.getName().replace(".json", ""); - List resources = json.getJsonObjects("resources"); - int i = 0; - for (JsonObject test : json.getJsonObjects("tests")) { - String name2 = test.asString("title"); - objects.add(Arguments.of(name1+":"+name2, new TestDetails(name1+":"+name2, "$.tests["+i+"]", resources, test))); - i++; - } + JsonArray testFiles = (JsonArray) JsonParser.parse(TestingUtilities.loadTestResourceStream("sql-on-fhir", "manifest.json")); + + for (String s : testFiles.asStrings()) { + JsonObject json = JsonParser.parseObject(TestingUtilities.loadTestResourceStream("sql-on-fhir", s)); + String name1 = s.replace(".json", ""); + List resources = json.getJsonObjects("resources"); + int i = 0; + for (JsonObject test : json.getJsonObjects("tests")) { + String name2 = test.asString("title"); + objects.add(Arguments.of(name1+":"+name2, new TestDetails(name1+":"+name2, "$.tests["+i+"]", resources, test))); + i++; } } return objects.stream(); @@ -110,7 +108,6 @@ public class SQLOnFhirTests { @SuppressWarnings("deprecation") @ParameterizedTest(name = "{index}: file {0}") @MethodSource("data") - @Disabled public void test(String name, TestDetails test) throws FileNotFoundException, IOException, FHIRException, org.hl7.fhir.exceptions.FHIRException, UcumException { this.details = test; Runner runner = new Runner(); @@ -137,8 +134,8 @@ public class SQLOnFhirTests { rows.add("rows", results); JsonObject exp = new JsonObject(); exp.add("rows", test.testCase.getJsonArray("expect")); - sortResults(exp); - sortResults(rows); +// sortResults(exp); +// sortResults(rows); String expS = JsonParser.compose(exp, true); String rowS = JsonParser.compose(rows, true); String c = CompareUtilities.checkJsonSrcIsSame(name, expS, rowS, null); From ef69d12111c68f069c5df4bc8cf3082f4a7560f7 Mon Sep 17 00:00:00 2001 From: Grahame Grieve Date: Mon, 9 Sep 2024 17:49:07 +0800 Subject: [PATCH 04/12] Fix expression for con-3 properly (fix validation problem on some condition resources) --- .../fhir/validation/instance/utils/FHIRPathExpressionFixer.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/instance/utils/FHIRPathExpressionFixer.java b/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/instance/utils/FHIRPathExpressionFixer.java index c4437511c..55376a293 100644 --- a/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/instance/utils/FHIRPathExpressionFixer.java +++ b/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/instance/utils/FHIRPathExpressionFixer.java @@ -61,7 +61,7 @@ public class FHIRPathExpressionFixer { } // con-3 in R4 if (expr.equals("clinicalStatus.exists() or verificationStatus.coding.where(system='http://terminology.hl7.org/CodeSystem/condition-ver-status' and code = 'entered-in-error').exists() or category.select($this='problem-list-item').empty()")) { - return "clinicalStatus.exists() or verificationStatus.coding.where(system='http://terminology.hl7.org/CodeSystem/condition-ver-status' and code = 'entered-in-error').exists() or category.coding.exists(system='http://terminology.hl7.org/CodeSystem/condition-category' and code ='problem-list-item').empty()"; + return "(verificationStatus.coding.where(system='http://terminology.hl7.org/CodeSystem/condition-ver-status' and code = 'entered-in-error').exists() and category.coding.exists(system='http://terminology.hl7.org/CodeSystem/condition-category' and code ='problem-list-item').empty()) implies (clinicalStatus.exists())"; } // R5 ballot From d219a20de89a6846b5b4b747ae10da14794fb27c Mon Sep 17 00:00:00 2001 From: Grahame Grieve Date: Mon, 9 Sep 2024 17:49:28 +0800 Subject: [PATCH 05/12] FHIRPath: Allow _ in constant names (per FHIRPath spec) --- .../src/main/java/org/hl7/fhir/r5/fhirpath/FHIRLexer.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/fhirpath/FHIRLexer.java b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/fhirpath/FHIRLexer.java index b0d101cdc..080600c69 100644 --- a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/fhirpath/FHIRLexer.java +++ b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/fhirpath/FHIRLexer.java @@ -187,7 +187,7 @@ public class FHIRLexer { cursor++; } else while (cursor < source.length() && ((source.charAt(cursor) >= 'A' && source.charAt(cursor) <= 'Z') || (source.charAt(cursor) >= 'a' && source.charAt(cursor) <= 'z') || - (source.charAt(cursor) >= '0' && source.charAt(cursor) <= '9') || source.charAt(cursor) == ':' || source.charAt(cursor) == '-')) + (source.charAt(cursor) >= '0' && source.charAt(cursor) <= '9') || source.charAt(cursor) == ':' || source.charAt(cursor) == '-' || source.charAt(cursor) == '_')) cursor++; current = source.substring(currentStart, cursor); } else if (ch == '/') { From 07a2b7d2e70728c654b6dbba35db02f62a433d01 Mon Sep 17 00:00:00 2001 From: Grahame Grieve Date: Mon, 9 Sep 2024 17:51:38 +0800 Subject: [PATCH 06/12] Fix FHIRPath bug using wrong type on simple elements when checking FHIRPath types --- .../hl7/fhir/r5/fhirpath/FHIRPathEngine.java | 23 ++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/fhirpath/FHIRPathEngine.java b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/fhirpath/FHIRPathEngine.java index 5d1efdcec..7a63e0c59 100644 --- a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/fhirpath/FHIRPathEngine.java +++ b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/fhirpath/FHIRPathEngine.java @@ -773,6 +773,25 @@ public class FHIRPathEngine { return execute(new ExecutionContext(null, base != null && base.isResource() ? base : null, base != null && base.isResource() ? base : null, base, base), list, ExpressionNode, true); } + + /** + * evaluate a path and return the matching elements + * + * @param base - the object against which the path is being evaluated + * @param ExpressionNode - the parsed ExpressionNode statement to use + * @return + * @throws FHIRException + * @ + */ + public List evaluate(Object appContext, Base base, ExpressionNode ExpressionNode) throws FHIRException { + List list = new ArrayList(); + if (base != null) { + list.add(base); + } + log = new StringBuilder(); + return execute(new ExecutionContext(appContext, base != null && base.isResource() ? base : null, base != null && base.isResource() ? base : null, base, base), list, ExpressionNode, true); + } + /** * evaluate a path and return the matching elements * @@ -3741,6 +3760,8 @@ public class FHIRPathEngine { } if ((focus.hasType("date") || focus.hasType("datetime") || focus.hasType("instant"))) { return new TypeDetails(CollectionStatus.SINGLETON, TypeDetails.FP_Decimal, TypeDetails.FP_DateTime); + } else if ((focus.hasType("time"))) { + return new TypeDetails(CollectionStatus.SINGLETON, TypeDetails.FP_Time, TypeDetails.FP_Time); } else if (focus.hasType("decimal") || focus.hasType("integer")) { return new TypeDetails(CollectionStatus.SINGLETON, TypeDetails.FP_Decimal); } else { @@ -6351,7 +6372,7 @@ public class FHIRPathEngine { } result.addTypes(worker.getResourceNames()); } else { - pt = new ProfiledType(t.getCode()); + pt = new ProfiledType(t.getWorkingCode()); } if (pt != null) { if (t.hasProfile()) { From 4692962820aa2acc6b7575a4ed078fc0da17b94e Mon Sep 17 00:00:00 2001 From: Grahame Grieve Date: Mon, 9 Sep 2024 17:56:07 +0800 Subject: [PATCH 07/12] release notes --- RELEASE_NOTES.md | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 7b06c6ab5..f9e202bce 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -1,7 +1,18 @@ ## Validator Changes -* no changes +* Fix expression for con-3 properly (fix validation problem on some condition resources) +* Fix FHIRPath bug using wrong type on simple elements when checking FHIRPath types +* FHIRPath: Allow _ in constant names (per FHIRPath spec) +* Fix value set rendering creating wrong references +* Fix bug processing value set includes / excludes that are just value sets (no system value) +* Alter processing of unknown code systems per discussion at ,https://chat.fhir.org/#narrow/stream/179252-IG-creation/topic/Don't.20error.20when.20you.20can't.20find.20code.20system and implement unknown-codesystems-cause-errors +* Improve message for when elements are out of order in profile differentials + ## Other code changes -* no changes \ No newline at end of file +* fix problem where profile rendering had spurious 'slices for' nodes everywhere +* Update SQL-On-FHIR implementation for latest cases, and clone test cases to general test care repository +* Fix problem generating value set spreadsheets +* fix concurrent modification error processing language translations +* Check for null fetcher processing ConceptMaps (#1728) From 52668c1c7897cd579f30d17aac3c4b8bf0cdac33 Mon Sep 17 00:00:00 2001 From: Grahame Grieve Date: Mon, 9 Sep 2024 21:23:42 +0800 Subject: [PATCH 08/12] fix sql-on-fhir tests --- .../main/java/org/hl7/fhir/utilities/Utilities.java | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/Utilities.java b/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/Utilities.java index 5c117d455..4042ee2f1 100644 --- a/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/Utilities.java +++ b/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/Utilities.java @@ -1819,9 +1819,14 @@ public class Utilities { private static Object applyDatePrecision(String v, int precision) { switch (precision) { - case 4: return v.substring(0, 4); - case 6: return v.substring(0, 7); - case 8: return v.substring(0, 10); + case 4: + return v.substring(0, 4); + case 6: + case 7: + return v.substring(0, 7); + case 8: + case 10: + return v.substring(0, 10); case 14: return v.substring(0, 17); case 17: return v; } From 44464de8077de575b1530b14aa770fd050fb88c2 Mon Sep 17 00:00:00 2001 From: dotasek Date: Tue, 10 Sep 2024 09:38:02 -0400 Subject: [PATCH 09/12] Remove cqlframework dependencies --- org.hl7.fhir.validation/pom.xml | 31 ------------------- .../instance/InstanceValidator.java | 2 +- .../validation/special/TxTesterSorters.java | 4 +-- 3 files changed, 3 insertions(+), 34 deletions(-) diff --git a/org.hl7.fhir.validation/pom.xml b/org.hl7.fhir.validation/pom.xml index 1dcd143bb..cc68b5cb2 100644 --- a/org.hl7.fhir.validation/pom.xml +++ b/org.hl7.fhir.validation/pom.xml @@ -119,37 +119,6 @@ true - - - info.cqframework - cql - ${info_cqframework_version} - - - info.cqframework - model - ${info_cqframework_version} - - - info.cqframework - elm - ${info_cqframework_version} - - - info.cqframework - cql-to-elm - ${info_cqframework_version} - - - info.cqframework - quick - ${info_cqframework_version} - - - info.cqframework - qdm - ${info_cqframework_version} - com.squareup.okhttp3 diff --git a/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/instance/InstanceValidator.java b/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/instance/InstanceValidator.java index dbc8eb164..302bc448d 100644 --- a/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/instance/InstanceValidator.java +++ b/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/instance/InstanceValidator.java @@ -46,7 +46,7 @@ import javax.annotation.Nonnull; import org.apache.commons.lang3.NotImplementedException; import org.apache.commons.lang3.StringUtils; import org.fhir.ucum.Decimal; -import org.hl7.elm.r1.Code; + import org.hl7.fhir.exceptions.DefinitionException; import org.hl7.fhir.exceptions.FHIRException; import org.hl7.fhir.exceptions.PathEngineException; diff --git a/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/special/TxTesterSorters.java b/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/special/TxTesterSorters.java index 35d069e8a..a85c4db8b 100644 --- a/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/special/TxTesterSorters.java +++ b/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/special/TxTesterSorters.java @@ -8,10 +8,10 @@ import java.util.Collections; import java.util.Comparator; import java.util.List; -import org.hl7.fhir.ParametersParameter; + import org.hl7.fhir.r5.formats.IParser.OutputStyle; import org.hl7.fhir.r5.formats.JsonParser; -import org.hl7.fhir.r5.model.Base; + import org.hl7.fhir.r5.model.Extension; import org.hl7.fhir.r5.model.OperationOutcome; import org.hl7.fhir.r5.model.OperationOutcome.OperationOutcomeIssueComponent; From 337aaf80cfa83946d5fe5266a8c78dcfd3e20eff Mon Sep 17 00:00:00 2001 From: Grahame Grieve Date: Thu, 12 Sep 2024 11:54:40 +0800 Subject: [PATCH 10/12] Process relative URLs properly in base when generating snapshots --- .../profile/ProfilePathProcessor.java | 24 ++++++++--------- .../conformance/profile/ProfileUtilities.java | 27 ++++++++++--------- 2 files changed, 26 insertions(+), 25 deletions(-) diff --git a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/conformance/profile/ProfilePathProcessor.java b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/conformance/profile/ProfilePathProcessor.java index 6a939f95a..a26d81569 100644 --- a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/conformance/profile/ProfilePathProcessor.java +++ b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/conformance/profile/ProfilePathProcessor.java @@ -289,7 +289,7 @@ public class ProfilePathProcessor { start++; } else { // we're just going to accept the differential slicing at face value - ElementDefinition outcome = profileUtilities.updateURLs(getUrl(), getWebUrl(), currentBase.copy()); + ElementDefinition outcome = profileUtilities.updateURLs(getUrl(), getWebUrl(), currentBase.copy(), true); outcome.setPath(profileUtilities.fixedPathDest(getContextPathTarget(), outcome.getPath(), getRedirector(), getContextPathSource())); profileUtilities.updateFromBase(outcome, currentBase, getSourceStructureDefinition().getUrl()); @@ -667,14 +667,14 @@ public class ProfilePathProcessor { // some of what's in currentBase overrides template template = profileUtilities.fillOutFromBase(template, currentBase); - ElementDefinition outcome = profileUtilities.updateURLs(getUrl(), getWebUrl(), template); + ElementDefinition outcome = profileUtilities.updateURLs(getUrl(), getWebUrl(), template, true); outcome.setPath(profileUtilities.fixedPathDest(getContextPathTarget(), outcome.getPath(), getRedirector(), getContextPathSource())); res = outcome; profileUtilities.updateFromBase(outcome, currentBase, getSourceStructureDefinition().getUrl()); if (diffMatches.get(0).hasSliceName()) { template = currentBase.copy(); - template = profileUtilities.updateURLs(getUrl(), getWebUrl(), template); + template = profileUtilities.updateURLs(getUrl(), getWebUrl(), template, true); template.setPath(profileUtilities.fixedPathDest(getContextPathTarget(), template.getPath(), getRedirector(), getContextPathSource())); checkToSeeIfSlicingExists(diffMatches.get(0), template); @@ -866,13 +866,13 @@ public class ProfilePathProcessor { private void processSimplePathWithEmptyDiffMatches(ElementDefinition currentBase, String currentBasePath, List diffMatches, ProfilePathProcessorState cursors, MappingAssistant mapHelper) { - ElementDefinition outcome = profileUtilities.updateURLs(getUrl(), getWebUrl(), currentBase.copy()); + ElementDefinition outcome = profileUtilities.updateURLs(getUrl(), getWebUrl(), currentBase.copy(), true); outcome.setPath(profileUtilities.fixedPathDest(getContextPathTarget(), outcome.getPath(), getRedirector(), getContextPathSource())); profileUtilities.updateFromBase(outcome, currentBase, getSourceStructureDefinition().getUrl()); profileUtilities.updateConstraintSources(outcome, getSourceStructureDefinition().getUrl()); profileUtilities.checkExtensions(outcome); profileUtilities.updateFromObligationProfiles(outcome); - profileUtilities.updateURLs(url, webUrl, outcome); + profileUtilities.updateURLs(url, webUrl, outcome, true); profileUtilities.markDerived(outcome); if (cursors.resultPathBase == null) cursors.resultPathBase = outcome.getPath(); @@ -1033,7 +1033,7 @@ public class ProfilePathProcessor { if (!currentBase.isChoice() && !profileUtilities.ruleMatches(dSlice.getRules(), bSlice.getRules())) throw new DefinitionException(profileUtilities.getContext().formatMessage(I18nConstants.SLICING_RULES_ON_DIFFERENTIAL__DO_NOT_MATCH_THOSE_ON_BASE___RULE___, profileUtilities.summarizeSlicing(dSlice), profileUtilities.summarizeSlicing(bSlice), path, cursors.contextName)); } - ElementDefinition outcome = profileUtilities.updateURLs(getUrl(), getWebUrl(), currentBase.copy()); + ElementDefinition outcome = profileUtilities.updateURLs(getUrl(), getWebUrl(), currentBase.copy(), true); outcome.setPath(profileUtilities.fixedPathDest(getContextPathTarget(), outcome.getPath(), getRedirector(), getContextPathSource())); profileUtilities.updateFromBase(outcome, currentBase, getSourceStructureDefinition().getUrl()); if (diffMatches.get(0).hasSlicing() || !diffMatches.get(0).hasSliceName()) { @@ -1095,7 +1095,7 @@ public class ProfilePathProcessor { // We need to copy children of the backbone element before we start messing around with slices int newBaseLimit = profileUtilities.findEndOfElement(cursors.base, cursors.baseCursor); for (int i = cursors.baseCursor + 1; i <= newBaseLimit; i++) { - outcome = profileUtilities.updateURLs(getUrl(), getWebUrl(), cursors.base.getElement().get(i).copy()); + outcome = profileUtilities.updateURLs(getUrl(), getWebUrl(), cursors.base.getElement().get(i).copy(), true); outcome.setPath(profileUtilities.fixedPathDest(getContextPathTarget(), outcome.getPath(), getRedirector(), getContextPathSource())); debugCheck(outcome); getResult().getElement().add(outcome); @@ -1106,7 +1106,7 @@ public class ProfilePathProcessor { List baseMatches = profileUtilities.getSiblings(cursors.base.getElement(), currentBase); for (ElementDefinition baseItem : baseMatches) { cursors.baseCursor = cursors.base.getElement().indexOf(baseItem); - outcome = profileUtilities.updateURLs(getUrl(), getWebUrl(), baseItem.copy()); + outcome = profileUtilities.updateURLs(getUrl(), getWebUrl(), baseItem.copy(), true); profileUtilities.updateFromBase(outcome, currentBase, getSourceStructureDefinition().getUrl()); outcome.setPath(profileUtilities.fixedPathDest(getContextPathTarget(), outcome.getPath(), getRedirector(), getContextPathSource())); outcome.setSlicing(null); @@ -1139,7 +1139,7 @@ public class ProfilePathProcessor { cursors.baseCursor++; // just copy any children on the base while (cursors.baseCursor < cursors.base.getElement().size() && cursors.base.getElement().get(cursors.baseCursor).getPath().startsWith(path) && !cursors.base.getElement().get(cursors.baseCursor).getPath().equals(path)) { - outcome = profileUtilities.updateURLs(getUrl(), getWebUrl(), cursors.base.getElement().get(cursors.baseCursor).copy()); + outcome = profileUtilities.updateURLs(getUrl(), getWebUrl(), cursors.base.getElement().get(cursors.baseCursor).copy(), true); outcome.setPath(profileUtilities.fixedPathDest(getContextPathTarget(), outcome.getPath(), getRedirector(), getContextPathSource())); if (!outcome.getPath().startsWith(cursors.resultPathBase)) throw new DefinitionException(profileUtilities.getContext().formatMessage(I18nConstants.ADDING_WRONG_PATH)); @@ -1166,7 +1166,7 @@ public class ProfilePathProcessor { for (ElementDefinition baseItem : baseMatches) if (baseItem.getSliceName().equals(diffItem.getSliceName())) throw new DefinitionException(profileUtilities.getContext().formatMessage(I18nConstants.NAMED_ITEMS_ARE_OUT_OF_ORDER_IN_THE_SLICE)); - outcome = profileUtilities.updateURLs(getUrl(), getWebUrl(), currentBase.copy()); + outcome = profileUtilities.updateURLs(getUrl(), getWebUrl(), currentBase.copy(), true); // outcome = updateURLs(url, diffItem.copy()); outcome.setPath(profileUtilities.fixedPathDest(getContextPathTarget(), outcome.getPath(), getRedirector(), getContextPathSource())); profileUtilities.updateFromBase(outcome, currentBase, getSourceStructureDefinition().getUrl()); @@ -1409,7 +1409,7 @@ public class ProfilePathProcessor { private void processPathWithSlicedBaseAndEmptyDiffMatches(ElementDefinition currentBase, String currentBasePath, List diffMatches, ProfilePathProcessorState cursors, String path, MappingAssistant mapHelper) { if (profileUtilities.hasInnerDiffMatches(getDifferential(), path, cursors.diffCursor, getDiffLimit(), cursors.base.getElement(), true)) { // so we just copy it in - ElementDefinition outcome = profileUtilities.updateURLs(getUrl(), getWebUrl(), currentBase.copy()); + ElementDefinition outcome = profileUtilities.updateURLs(getUrl(), getWebUrl(), currentBase.copy(), true); outcome.setPath(profileUtilities.fixedPathDest(getContextPathTarget(), outcome.getPath(), getRedirector(), getContextPathSource())); profileUtilities.updateFromBase(outcome, currentBase, getSourceStructureDefinition().getUrl()); profileUtilities.markDerived(outcome); @@ -1457,7 +1457,7 @@ public class ProfilePathProcessor { // the differential doesn't say anything about this item // copy across the currentbase, and all of its children and siblings while (cursors.baseCursor < cursors.base.getElement().size() && cursors.base.getElement().get(cursors.baseCursor).getPath().startsWith(path)) { - ElementDefinition outcome = profileUtilities.updateURLs(getUrl(), getWebUrl(), cursors.base.getElement().get(cursors.baseCursor).copy()); + ElementDefinition outcome = profileUtilities.updateURLs(getUrl(), getWebUrl(), cursors.base.getElement().get(cursors.baseCursor).copy(), true); outcome.setPath(profileUtilities.fixedPathDest(getContextPathTarget(), outcome.getPath(), getRedirector(), getContextPathSource())); if (!outcome.getPath().startsWith(cursors.resultPathBase)) throw new DefinitionException(profileUtilities.getContext().formatMessage(I18nConstants.ADDING_WRONG_PATH_IN_PROFILE___VS_, getProfileName(), outcome.getPath(), cursors.resultPathBase)); diff --git a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/conformance/profile/ProfileUtilities.java b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/conformance/profile/ProfileUtilities.java index 79d668b36..7a75e0358 100644 --- a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/conformance/profile/ProfileUtilities.java +++ b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/conformance/profile/ProfileUtilities.java @@ -739,7 +739,7 @@ public class ProfileUtilities { if (existing != null) { updateFromDefinition(existing, e, profileName, false, url, base, derived, "StructureDefinition.differential.element["+i+"]", mappingDetails); } else { - ElementDefinition outcome = updateURLs(url, webUrl, e.copy()); + ElementDefinition outcome = updateURLs(url, webUrl, e.copy(), true); e.setUserData(UD_GENERATED_IN_SNAPSHOT, outcome); derived.getSnapshot().addElement(outcome); if (walksInto(diff.getElement(), e)) { @@ -1042,7 +1042,7 @@ public class ProfileUtilities { // don't do this. should already be in snapshot ... addInheritedElementsForSpecialization(snapshot, focus, sd.getBaseDefinition(), path, url, weburl); for (ElementDefinition ed : sd.getSnapshot().getElement()) { if (ed.getPath().contains(".")) { - ElementDefinition outcome = updateURLs(url, weburl, ed.copy()); + ElementDefinition outcome = updateURLs(url, weburl, ed.copy(), true); outcome.setPath(outcome.getPath().replace(sd.getTypeName(), path)); snapshot.getElement().add(outcome); } else { @@ -1548,7 +1548,6 @@ public class ProfileUtilities { protected void removeStatusExtensions(ElementDefinition outcome) { outcome.removeExtension(ToolingExtensions.EXT_FMM_LEVEL); outcome.removeExtension(ToolingExtensions.EXT_FMM_SUPPORT); - outcome.removeExtension(ToolingExtensions.EXT_FMM_DERIVED); outcome.removeExtension(ToolingExtensions.EXT_STANDARDS_STATUS); outcome.removeExtension(ToolingExtensions.EXT_NORMATIVE_VERSION); outcome.removeExtension(ToolingExtensions.EXT_WORKGROUP); @@ -1911,7 +1910,7 @@ public class ProfileUtilities { * @param element - the Element to update * @return - the updated Element */ - public ElementDefinition updateURLs(String url, String webUrl, ElementDefinition element) { + public ElementDefinition updateURLs(String url, String webUrl, ElementDefinition element, boolean processRelatives) { if (element != null) { ElementDefinition defn = element; if (defn.hasBinding() && defn.getBinding().hasValueSet() && defn.getBinding().getValueSet().startsWith("#")) @@ -1929,24 +1928,24 @@ public class ProfileUtilities { if (webUrl != null) { // also, must touch up the markdown if (element.hasDefinition()) { - element.setDefinition(processRelativeUrls(element.getDefinition(), webUrl, context.getSpecUrl(), context.getResourceNames(), masterSourceFileNames, localFileNames, false)); + element.setDefinition(processRelativeUrls(element.getDefinition(), webUrl, context.getSpecUrl(), context.getResourceNames(), masterSourceFileNames, localFileNames, processRelatives)); } if (element.hasComment()) { - element.setComment(processRelativeUrls(element.getComment(), webUrl, context.getSpecUrl(), context.getResourceNames(), masterSourceFileNames, localFileNames, false)); + element.setComment(processRelativeUrls(element.getComment(), webUrl, context.getSpecUrl(), context.getResourceNames(), masterSourceFileNames, localFileNames, processRelatives)); } if (element.hasRequirements()) { - element.setRequirements(processRelativeUrls(element.getRequirements(), webUrl, context.getSpecUrl(), context.getResourceNames(), masterSourceFileNames, localFileNames, false)); + element.setRequirements(processRelativeUrls(element.getRequirements(), webUrl, context.getSpecUrl(), context.getResourceNames(), masterSourceFileNames, localFileNames, processRelatives)); } if (element.hasMeaningWhenMissing()) { - element.setMeaningWhenMissing(processRelativeUrls(element.getMeaningWhenMissing(), webUrl, context.getSpecUrl(), context.getResourceNames(), masterSourceFileNames, localFileNames, false)); + element.setMeaningWhenMissing(processRelativeUrls(element.getMeaningWhenMissing(), webUrl, context.getSpecUrl(), context.getResourceNames(), masterSourceFileNames, localFileNames, processRelatives)); } if (element.hasBinding() && element.getBinding().hasDescription()) { - element.getBinding().setDescription(processRelativeUrls(element.getBinding().getDescription(), webUrl, context.getSpecUrl(), context.getResourceNames(), masterSourceFileNames, localFileNames, false)); + element.getBinding().setDescription(processRelativeUrls(element.getBinding().getDescription(), webUrl, context.getSpecUrl(), context.getResourceNames(), masterSourceFileNames, localFileNames, processRelatives)); } for (Extension ext : element.getExtension()) { if (ext.hasValueMarkdownType()) { MarkdownType md = ext.getValueMarkdownType(); - md.setValue(processRelativeUrls(md.getValue(), webUrl, context.getSpecUrl(), context.getResourceNames(), masterSourceFileNames, localFileNames, false)); + md.setValue(processRelativeUrls(md.getValue(), webUrl, context.getSpecUrl(), context.getResourceNames(), masterSourceFileNames, localFileNames, processRelatives)); } } } @@ -2371,7 +2370,6 @@ public class ProfileUtilities { if (elist.size() == 2) { dest.getExtension().remove(elist.get(1)); } - updateExtensionsFromDefinition(dest, source); for (ElementDefinition ed : obligationProfileElements) { @@ -2423,6 +2421,9 @@ public class ProfileUtilities { if (e.hasDefinition()) { base.setDefinition(processRelativeUrls(e.getDefinition(), webroot, context.getSpecUrl(), context.getResourceNames(), masterSourceFileNames, localFileNames, true)); } + if (e.getBinding().hasDescription()) { + base.getBinding().setDescription(processRelativeUrls(e.getBinding().getDescription(), webroot, context.getSpecUrl(), context.getResourceNames(), masterSourceFileNames, localFileNames, true)); + } base.setShort(e.getShort()); if (e.hasCommentElement()) base.setCommentElement(e.getCommentElement()); @@ -2466,9 +2467,9 @@ public class ProfileUtilities { if (derived.hasDefinitionElement()) { if (derived.getDefinition().startsWith("...")) base.setDefinition(Utilities.appendDerivedTextToBase(base.getDefinition(), derived.getDefinition())); - else if (!Base.compareDeep(derived.getDefinitionElement(), base.getDefinitionElement(), false)) + else if (!Base.compareDeep(derived.getDefinitionElement(), base.getDefinitionElement(), false)) { base.setDefinitionElement(derived.getDefinitionElement().copy()); - else if (trimDifferential) + } else if (trimDifferential) derived.setDefinitionElement(null); else if (derived.hasDefinitionElement()) derived.getDefinitionElement().setUserData(UD_DERIVATION_EQUALS, true); From 8056dced3efb3a44d66178aa52351dac367dd389 Mon Sep 17 00:00:00 2001 From: Grahame Grieve Date: Thu, 12 Sep 2024 11:55:24 +0800 Subject: [PATCH 11/12] Allow JSON named extensions to be structure types other than logical --- .../main/java/org/hl7/fhir/r5/context/ContextUtilities.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/context/ContextUtilities.java b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/context/ContextUtilities.java index 02890a037..12bc847a2 100644 --- a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/context/ContextUtilities.java +++ b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/context/ContextUtilities.java @@ -366,7 +366,11 @@ public class ContextUtilities implements ProfileKnowledgeProvider { public StructureDefinition fetchByJsonName(String key) { for (StructureDefinition sd : context.fetchResourcesByType(StructureDefinition.class)) { ElementDefinition ed = sd.getSnapshot().getElementFirstRep(); - if (sd.getKind() == StructureDefinitionKind.LOGICAL && ed != null && ed.hasExtension(ToolingExtensions.EXT_JSON_NAME, ToolingExtensions.EXT_JSON_NAME_DEPRECATED) && + if (/*sd.getKind() == StructureDefinitionKind.LOGICAL && */ + // this is turned off because it's valid to use a FHIR type directly in + // an extension of this kind, and that can't be a logical model. Any profile on + // a type is acceptable as long as it has the json name on it + ed != null && ed.hasExtension(ToolingExtensions.EXT_JSON_NAME, ToolingExtensions.EXT_JSON_NAME_DEPRECATED) && key.equals(ToolingExtensions.readStringExtension(ed, ToolingExtensions.EXT_JSON_NAME, ToolingExtensions.EXT_JSON_NAME_DEPRECATED))) { return sd; } From 07f1981a8c49d1843e1391977bddd33ff0b7e6ae Mon Sep 17 00:00:00 2001 From: Grahame Grieve Date: Thu, 12 Sep 2024 11:56:04 +0800 Subject: [PATCH 12/12] Fix for NPE processing packages --- .../fhir/r5/renderers/StructureDefinitionRenderer.java | 2 +- .../main/java/org/hl7/fhir/utilities/npm/NpmPackage.java | 8 +++++--- pom.xml | 2 +- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/renderers/StructureDefinitionRenderer.java b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/renderers/StructureDefinitionRenderer.java index bcb245cc6..5e7b63d20 100644 --- a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/renderers/StructureDefinitionRenderer.java +++ b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/renderers/StructureDefinitionRenderer.java @@ -3431,7 +3431,7 @@ public class StructureDefinitionRenderer extends ResourceRenderer { sdMapCache.put(url, sdCache); String webroot = sd.getUserString("webroot"); for (ElementDefinition e : sd.getSnapshot().getElement()) { - context.getProfileUtilities().updateURLs(sd.getUrl(), webroot, e); + context.getProfileUtilities().updateURLs(sd.getUrl(), webroot, e, false); sdCache.put(e.getId(), e); } } diff --git a/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/npm/NpmPackage.java b/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/npm/NpmPackage.java index 5ab8a9113..6daa184af 100644 --- a/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/npm/NpmPackage.java +++ b/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/npm/NpmPackage.java @@ -237,9 +237,11 @@ public class NpmPackage { public List listFiles() { List res = new ArrayList<>(); if (folder != null) { - for (File f : folder.listFiles()) { - if (!f.isDirectory() && !Utilities.existsInList(f.getName(), "package.json", ".index.json", ".index.db", ".oids.json", ".oids.db")) { - res.add(f.getName()); + if (folder.exists()) { + for (File f : folder.listFiles()) { + if (!f.isDirectory() && !Utilities.existsInList(f.getName(), "package.json", ".index.json", ".index.db", ".oids.json", ".oids.db")) { + res.add(f.getName()); + } } } } else { diff --git a/pom.xml b/pom.xml index 4f8248ecc..e842abb9d 100644 --- a/pom.xml +++ b/pom.xml @@ -21,7 +21,7 @@ 1.26.0 32.0.1-jre 6.4.1 - 1.5.21 + 1.5.22-SNAPSHOT 2.17.0 5.9.2 1.8.2