From d1c032791eb52499e1a580f4346febb5666aad32 Mon Sep 17 00:00:00 2001 From: mhenke-vg Date: Thu, 24 Mar 2022 16:25:43 +0100 Subject: [PATCH 1/2] fix ehrbase#753: node ids null and annotations missing --- .../OperationalTemplateTestData.java | 3 +- .../resources/operationaltemplate/nullid.opt | 1076 +++++++++++++++++ .../webtemplate/example.initialassesment.json | 30 +- .../ehrbase/webtemplate/parser/OPTParser.java | 86 +- .../webtemplate/parser/OPTParserTest.java | 82 +- 5 files changed, 1210 insertions(+), 67 deletions(-) create mode 100644 test-data/src/main/resources/operationaltemplate/nullid.opt diff --git a/test-data/src/main/java/org/ehrbase/test_data/operationaltemplate/OperationalTemplateTestData.java b/test-data/src/main/java/org/ehrbase/test_data/operationaltemplate/OperationalTemplateTestData.java index 5134f71bd..6b9c6fb2a 100644 --- a/test-data/src/main/java/org/ehrbase/test_data/operationaltemplate/OperationalTemplateTestData.java +++ b/test-data/src/main/java/org/ehrbase/test_data/operationaltemplate/OperationalTemplateTestData.java @@ -112,7 +112,8 @@ public enum OperationalTemplateTestData { RE_SPECT("ReSPECT-3.v0", "ReSPECT-3.v0.opt", "ReSPECT-3.v0"), IPS("International Patient Summary", "ips.v0.opt", "International Patient Summary"), ERROR_TEST("Contains choice between interval and not interval", "ErrorTest.opt", "ErrorTest"), - SECTION_CARDINALITY("Cardinality test template sections", "section_cardinality.opt", "cardinality_of_section"); + SECTION_CARDINALITY("Cardinality test template sections", "section_cardinality.opt", "cardinality_of_section"), + NULLID("My Nullid test", "nullid.opt", "nullid"); private final String filename; private final String templateId; diff --git a/test-data/src/main/resources/operationaltemplate/nullid.opt b/test-data/src/main/resources/operationaltemplate/nullid.opt new file mode 100644 index 000000000..0ad97445e --- /dev/null +++ b/test-data/src/main/resources/operationaltemplate/nullid.opt @@ -0,0 +1,1076 @@ + + \ No newline at end of file diff --git a/test-data/src/main/resources/webtemplate/example.initialassesment.json b/test-data/src/main/resources/webtemplate/example.initialassesment.json index 3f94ac9bd..1515ec0d1 100644 --- a/test-data/src/main/resources/webtemplate/example.initialassesment.json +++ b/test-data/src/main/resources/webtemplate/example.initialassesment.json @@ -166,7 +166,10 @@ } ] } - ] + ], + "annotations": { + "comment": "Most commonly, the score for eye response will be selected from one of the ordinal values, however if a response cannot be tested, for example if the subject of care cannot physically open their eyes due to other injuries, then the \"Not Applicable\" null flavour should be recorded." + } }, { "id": "coded_text_value", @@ -197,7 +200,10 @@ ], "terminology": "openehr" } - ] + ], + "annotations": { + "comment": "Most commonly, the score for eye response will be selected from one of the ordinal values, however if a response cannot be tested, for example if the subject of care cannot physically open their eyes due to other injuries, then the \"Not Applicable\" null flavour should be recorded." + } } ] }, @@ -295,7 +301,10 @@ } ] } - ] + ], + "annotations": { + "comment": "Most commonly, the score for verbal response will be selected from one of the ordinal values, however if a response cannot be tested, for example if the subject of care is intubated, then the \"Not Applicable\" null flavour should be recorded." + } }, { "id": "coded_text_value", @@ -326,7 +335,10 @@ ], "terminology": "openehr" } - ] + ], + "annotations": { + "comment": "Most commonly, the score for verbal response will be selected from one of the ordinal values, however if a response cannot be tested, for example if the subject of care is intubated, then the \"Not Applicable\" null flavour should be recorded." + } } ] }, @@ -435,7 +447,10 @@ } ] } - ] + ], + "annotations": { + "comment": "Most commonly, the score for motor response will be selected from one of the ordinal values, however if a response cannot be tested, for example if the subject of care cannot move their limbs due to injury or paralysis, then the \"Not Applicable\" null flavour should be recorded." + } }, { "id": "coded_text_value", @@ -466,7 +481,10 @@ ], "terminology": "openehr" } - ] + ], + "annotations": { + "comment": "Most commonly, the score for motor response will be selected from one of the ordinal values, however if a response cannot be tested, for example if the subject of care cannot move their limbs due to injury or paralysis, then the \"Not Applicable\" null flavour should be recorded." + } } ] }, diff --git a/web-template/src/main/java/org/ehrbase/webtemplate/parser/OPTParser.java b/web-template/src/main/java/org/ehrbase/webtemplate/parser/OPTParser.java index 893b241d0..e7133279b 100644 --- a/web-template/src/main/java/org/ehrbase/webtemplate/parser/OPTParser.java +++ b/web-template/src/main/java/org/ehrbase/webtemplate/parser/OPTParser.java @@ -49,7 +49,6 @@ import java.util.function.Function; import java.util.stream.Collectors; - public class OPTParser { public static final String PATH_DIVIDER = "/"; @@ -64,6 +63,7 @@ public class OPTParser { private final String defaultLanguage; private final Map defaultValues = new HashMap<>(); private final InputHandler inputHandler = new InputHandler(defaultValues); + private final Map> annotationMap = new HashMap<>(); private List languages; public OPTParser(OPERATIONALTEMPLATE operationaltemplate) { @@ -94,7 +94,17 @@ public WebTemplate parse() { languages = webTemplate.getLanguages(); - webTemplate.setTree(parseCARCHETYPEROO(operationaltemplate.getDefinition(), AqlPath.EMPTY_PATH).get(0)); + for (ANNOTATION annotation : operationaltemplate.getAnnotationsArray()) { + Map items = new HashMap<>(); + for (StringDictionaryItem item : annotation.getItemsArray()) { + items.put(item.getId(), item.getStringValue()); + } + // remove archetype id from path because nodes do not contain that in their path + annotationMap.put(annotation.getPath().replaceAll("^[^/]+", ""), items); + } + + webTemplate.setTree( + parseCARCHETYPEROO(operationaltemplate.getDefinition(), AqlPath.EMPTY_PATH).get(0)); return webTemplate; } @@ -103,7 +113,7 @@ static XmlObject[] extractChildren(XmlObject c, String attributes) { } private List> extractDefault(TATTRIBUTE xmlObject) { - //XXX performance List> + // XXX performance List> List> defaults = new ArrayList<>(); String differentialPath = StringUtils.substringAfter(xmlObject.getDifferentialPath(), "/"); @@ -288,7 +298,7 @@ private List parseCCOMPLEXOBJECT( ccomplexobject, aqlPath, termDefinitionMap, rmAttributeName, e); Optional localNameNode = nameNode.map(WebTemplateNode::new); localNameNode.ifPresent(n -> setExplicitName(e, node, n)); - + addAnnotations(node, aqlPath.toString(), annotationMap); parseComplexObjectSingle( ccomplexobject, node.getAqlPathDto(), termDefinitionMap, localNameNode, node); return node; @@ -307,12 +317,26 @@ private List parseCCOMPLEXOBJECT( explicitName.orElse(null)); nameNode.ifPresent(n -> setExplicitName(explicitName.orElse(null), node, n)); + addAnnotations(node, aqlPath.toString(), annotationMap); parseComplexObjectSingle( ccomplexobject, node.getAqlPathDto(), termDefinitionMap, nameNode, node); return Collections.singletonList(node); } } + private void addAnnotations( + WebTemplateNode node, String aqlPath, Map> annotationMap) { + Map annotationItems = annotationMap.get(aqlPath); + if (annotationItems != null) { + WebTemplateAnnotation annotation = node.getAnnotations(); + if (annotation == null) { + annotation = new WebTemplateAnnotation(); + node.setAnnotations(annotation); + } + annotation.getOther().putAll(annotationItems); + } + } + private void setExplicitName(Name explicitName, WebTemplateNode node, WebTemplateNode n) { n.setAqlPath(node.getAqlPathDto().addEnd("name")); @@ -371,8 +395,8 @@ private void parseComplexObjectSingle( for (CATTRIBUTE cattribute : ccomplexobject.getAttributesArray()) { AqlPath pathLoop = aqlPath.addEnd(cattribute.getRmAttributeName()); if ( - // Will be set via Attributes - pathLoop.getLastNode().getName().equals("name")) { + // Will be set via Attributes + pathLoop.getLastNode().getName().equals("name")) { continue; } @@ -420,15 +444,17 @@ private void parseComplexObjectSingle( WebTemplateNode careflowStep = new WebTemplateNode(); WebTemplateNode careflowStepProto = - firstChild.findChildById(CAREFLOW_STEP).orElseThrow(() -> new SdkException(String.format("Missing node: %s", CAREFLOW_STEP))); + firstChild + .findChildById(CAREFLOW_STEP) + .orElseThrow( + () -> new SdkException(String.format("Missing node: %s", CAREFLOW_STEP))); careflowStep.setMin(careflowStepProto.getMin()); careflowStep.setMax(careflowStepProto.getMin()); careflowStep.setName("Careflow_step"); careflowStep.setId(CAREFLOW_STEP); careflowStep.setRmType(DV_CODED_TEXT); careflowStep.setInContext(true); - careflowStep.setAqlPath( - aqlPath.addEnd(cattribute.getRmAttributeName(), CAREFLOW_STEP)); + careflowStep.setAqlPath(aqlPath.addEnd(cattribute.getRmAttributeName(), CAREFLOW_STEP)); WebTemplateInput code = new WebTemplateInput(); code.setSuffix("code"); code.setType(CODED_TEXT); @@ -450,16 +476,14 @@ private void parseComplexObjectSingle( ismTransition.getChildren().add(careflowStep); WebTemplateNode currentState = new WebTemplateNode(); - WebTemplateNode currentStateProto = - firstChild.findChildById(CURRENT_STATE).orElseThrow(); + WebTemplateNode currentStateProto = firstChild.findChildById(CURRENT_STATE).orElseThrow(); currentState.setMin(currentStateProto.getMin()); currentState.setMax(currentStateProto.getMin()); currentState.setRmType(DV_CODED_TEXT); currentState.setName("Current_state"); currentState.setId(CURRENT_STATE); currentState.setInContext(true); - currentState.setAqlPath( - aqlPath.addEnd(cattribute.getRmAttributeName(), CURRENT_STATE)); + currentState.setAqlPath(aqlPath.addEnd(cattribute.getRmAttributeName(), CURRENT_STATE)); WebTemplateInput code2 = new WebTemplateInput(); code2.setSuffix("code"); code2.setType(CODED_TEXT); @@ -476,8 +500,7 @@ private void parseComplexObjectSingle( .collect(Collectors.toList())); currentState.getInputs().add(code2); ismTransition.getChildren().add(currentState); - WebTemplateNode transition = - firstChild.findChildById("transition").orElseThrow(); + WebTemplateNode transition = firstChild.findChildById("transition").orElseThrow(); transition.setAqlPath(aqlPath.addEnd(cattribute.getRmAttributeName(), "transition")); transition.setInContext(true); ismTransition.getChildren().add(transition); @@ -508,15 +531,12 @@ private void parseComplexObjectSingle( .filter( c -> newChildren.stream() - .filter(n -> !n.getAqlPathDto().equals(c.getAqlPathDto())) - .anyMatch( - n -> n.getAqlPathDto().equals(c.getAqlPathDto(), false))) + .filter(n -> !n.getAqlPathDto().equals(c.getAqlPathDto())) + .anyMatch(n -> n.getAqlPathDto().equals(c.getAqlPathDto(), false))) .forEach( c -> { AqlPath path = AqlPath.parse(c.getAqlPath(true), c.getName()); - c.getChildren() - .forEach( - n -> replaceParentAql(n, c.getAqlPathDto(), path)); + c.getChildren().forEach(n -> replaceParentAql(n, c.getAqlPathDto(), path)); c.setAqlPath(path); }); @@ -557,13 +577,8 @@ private void parseComplexObjectSingle( List trueChildren = WebTemplateUtils.getTrueChildrenElement(node); trueChildren.forEach(c -> pushProperties(node, c)); - if (trueChildren.size() == 1) { - WebTemplateNode value = trueChildren.get(0); - value.setAnnotations(node.getAnnotations()); - - } // choice between value and null_flavour - else if (node.getChoicesInChildren().isEmpty()) { + if (trueChildren.size() != 1 && node.getChoicesInChildren().isEmpty()) { WebTemplateUtils.getTrueChildrenElement(node).stream() .filter(n -> n.getRmType().startsWith("DV_")) .forEach( @@ -608,8 +623,8 @@ else if (node.getChoicesInChildren().isEmpty()) { .collect(Collectors.toList()); // only add non-trivial cardinalities. if ((p.getKey().getMax() != null - && p.getKey().getMax() != -1 - && p.getKey().getMax() < nodeIds.size()) + && p.getKey().getMax() != -1 + && p.getKey().getMax() < nodeIds.size()) || (p.getKey().getMin() != null && p.getKey().getMin() > 1)) { p.getKey().getIds().addAll(nodeIds); node.getCardinalities().add(p.getKey()); @@ -626,7 +641,7 @@ private void replaceParentAql(WebTemplateNode node, AqlPath oldBase, AqlPath new node.setAqlPath(newBase.addEnd(relativeAql)); - node.getChildren().forEach(n -> replaceParentAql(n, childAql, node.getAqlPathDto()) ); + node.getChildren().forEach(n -> replaceParentAql(n, childAql, node.getAqlPathDto())); } private Optional buildNameNode( @@ -727,6 +742,8 @@ private WebtemplateCardinality buildCardinality(CARDINALITY xmlObject) { } private void pushProperties(WebTemplateNode node, WebTemplateNode value) { + value.setNodeId(node.getNodeId()); + value.setAnnotations(node.getAnnotations()); value.setName(node.getName()); value.getLocalizedDescriptions().putAll(node.getLocalizedDescriptions()); value.getLocalizedNames().putAll(node.getLocalizedNames()); @@ -768,9 +785,9 @@ private void addRMAttributes( RMTypeInfo typeInfo = ARCHIE_RM_INFO_LOOKUP.getTypeInfo(node.getRmType()); if (typeInfo != null && (Locatable.class.isAssignableFrom(typeInfo.getJavaClass()) - || EventContext.class.isAssignableFrom(typeInfo.getJavaClass()) - || DvInterval.class.isAssignableFrom(typeInfo.getJavaClass()) - || IsmTransition.class.isAssignableFrom(typeInfo.getJavaClass()))) { + || EventContext.class.isAssignableFrom(typeInfo.getJavaClass()) + || DvInterval.class.isAssignableFrom(typeInfo.getJavaClass()) + || IsmTransition.class.isAssignableFrom(typeInfo.getJavaClass()))) { node.getChildren() .addAll( @@ -1061,7 +1078,7 @@ private WebTemplateNode parseCDOMAINTYPE( .map( o -> StringUtils.isBlank(code.getTerminology()) - || "local".equals(code.getTerminology()) + || "local".equals(code.getTerminology()) ? o : code.getTerminology() + "::" + o) .forEach( @@ -1210,5 +1227,4 @@ public static String buildId(String term) { return normalTerm; } - } diff --git a/web-template/src/test/java/org/ehrbase/webtemplate/parser/OPTParserTest.java b/web-template/src/test/java/org/ehrbase/webtemplate/parser/OPTParserTest.java index 6e22fe758..df7b92580 100644 --- a/web-template/src/test/java/org/ehrbase/webtemplate/parser/OPTParserTest.java +++ b/web-template/src/test/java/org/ehrbase/webtemplate/parser/OPTParserTest.java @@ -16,21 +16,7 @@ package org.ehrbase.webtemplate.parser; -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatNoException; - import com.fasterxml.jackson.databind.ObjectMapper; -import java.io.IOException; -import java.nio.charset.StandardCharsets; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; -import java.util.List; -import java.util.Map; -import java.util.Objects; -import java.util.function.Function; -import java.util.stream.Collectors; -import java.util.stream.Stream; import org.apache.commons.collections4.CollectionUtils; import org.apache.commons.io.IOUtils; import org.apache.commons.lang3.tuple.ImmutablePair; @@ -40,16 +26,23 @@ import org.ehrbase.test_data.operationaltemplate.OperationalTemplateTestData; import org.ehrbase.test_data.webtemplate.WebTemplateTestData; import org.ehrbase.webtemplate.filter.Filter; -import org.ehrbase.webtemplate.model.WebTemplate; -import org.ehrbase.webtemplate.model.WebTemplateInput; -import org.ehrbase.webtemplate.model.WebTemplateInputValue; -import org.ehrbase.webtemplate.model.WebTemplateNode; +import org.ehrbase.webtemplate.model.*; import org.junit.Test; import org.openehr.schemas.v1.OPERATIONALTEMPLATE; import org.openehr.schemas.v1.TemplateDocument; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.util.*; +import java.util.function.Function; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatNoException; + /** - * @author Stefan Spiska + * @author Stefan Spiska * @since 1.0 */ public class OPTParserTest { @@ -429,15 +422,16 @@ public void parseAllTypes() throws IOException, XmlException { @Test public void parseMultimediaTest() throws Exception { - var optTemplate = TemplateDocument.Factory.parse(OperationalTemplateTestData.MULTIMEDIA_TEST.getStream()) + var optTemplate = + TemplateDocument.Factory.parse(OperationalTemplateTestData.MULTIMEDIA_TEST.getStream()) .getTemplate(); var parser = new OPTParser(optTemplate); var webTemplate = parser.parse(); assertThat(webTemplate).isNotNull(); - var nodes = webTemplate.getTree() - .findMatching(node -> node.getRmType().equals("PARTICIPATION")); + var nodes = + webTemplate.getTree().findMatching(node -> node.getRmType().equals("PARTICIPATION")); assertThat(nodes).hasSize(2); } @@ -814,9 +808,9 @@ private Collection compareInputValue( @Test public void testReadWrite() throws IOException, XmlException { - var template = TemplateDocument.Factory.parse( - OperationalTemplateTestData.CORONA_ANAMNESE.getStream()) - .getTemplate(); + var template = + TemplateDocument.Factory.parse(OperationalTemplateTestData.CORONA_ANAMNESE.getStream()) + .getTemplate(); var parser = new OPTParser(template); var webTemplate = parser.parse(); @@ -825,4 +819,42 @@ public void testReadWrite() throws IOException, XmlException { assertThatNoException().isThrownBy(() -> objectMapper.writeValueAsString(webTemplate)); } + + @Test + public void missingNodeIdAndAnnotationsTest() throws IOException, XmlException { + OPERATIONALTEMPLATE template = + TemplateDocument.Factory.parse(OperationalTemplateTestData.NULLID.getStream()) + .getTemplate(); + + WebTemplate webTemplate = new OPTParser(template).parse(); + assertThat(webTemplate).isNotNull(); + List nodes = + webTemplate.findAllByAqlPath( + "[openEHR-EHR-COMPOSITION.encounter.v1]/content[openEHR-EHR-OBSERVATION.blood_pressure.v2]/data[at0001]/events[at0006]/data[at0003]/items[at0004]", + true); + assertThat(nodes).isNotNull(); + assertThat(nodes.size()).isGreaterThan(0); + WebTemplateNode node = nodes.get(0); + assertThat(node).isNotNull(); + assertThat(node.getNodeId()).isNotNull(); + WebTemplateAnnotation annotation = node.getAnnotations(); + assertThat(annotation).isNotNull(); + assertThat(annotation.getOther()).isNotNull(); + assertThat(annotation.getOther().size()).isEqualTo(3); + + webTemplate = new Filter().filter(webTemplate); + nodes = + webTemplate.findAllByAqlPath( + "[openEHR-EHR-COMPOSITION.encounter.v1]/content[openEHR-EHR-OBSERVATION.blood_pressure.v2]/data[at0001]/events[at0006]/data[at0003]/items[at0004]/value", + true); + assertThat(nodes).isNotNull(); + assertThat(nodes.size()).isGreaterThan(0); + node = nodes.get(0); + assertThat(node).isNotNull(); + assertThat(node.getNodeId()).isNotNull(); + annotation = node.getAnnotations(); + assertThat(annotation).isNotNull(); + assertThat(annotation.getOther()).isNotNull(); + assertThat(annotation.getOther().size()).isEqualTo(3); + } } From 7052251018f1a997d1d213482bb8d2f5fe2fde29 Mon Sep 17 00:00:00 2001 From: mhenke-vg Date: Mon, 28 Mar 2022 10:16:12 +0200 Subject: [PATCH 2/2] fix tests --- CHANGELOG.md | 1 + .../operationaltemplate/OperationalTemplateTestData.java | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9038e2ed2..ec4506f3d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ Note: version releases in the 0.x.y range may introduce breaking changes. see https://github.com/ehrbase/openEHR_SDK/pull/325, https://github.com/ehrbase/openEHR_SDK/pull/332 ) - dto: Fix handling of element wich contains a choice with one an interval (see https://github.com/ehrbase/openEHR_SDK/pull/334) - Fixes AqlParseException while using boolean in where clause ([#338](hhttps://github.com/ehrbase/openEHR_SDK/pull/338)) +- Fixes null nodeIds and annotations missing (https://github.com/ehrbase/openEHR_SDK/pull/343) ## 1.17.0 diff --git a/test-data/src/main/java/org/ehrbase/test_data/operationaltemplate/OperationalTemplateTestData.java b/test-data/src/main/java/org/ehrbase/test_data/operationaltemplate/OperationalTemplateTestData.java index 6b9c6fb2a..2c7d1b8dc 100644 --- a/test-data/src/main/java/org/ehrbase/test_data/operationaltemplate/OperationalTemplateTestData.java +++ b/test-data/src/main/java/org/ehrbase/test_data/operationaltemplate/OperationalTemplateTestData.java @@ -113,7 +113,7 @@ public enum OperationalTemplateTestData { IPS("International Patient Summary", "ips.v0.opt", "International Patient Summary"), ERROR_TEST("Contains choice between interval and not interval", "ErrorTest.opt", "ErrorTest"), SECTION_CARDINALITY("Cardinality test template sections", "section_cardinality.opt", "cardinality_of_section"), - NULLID("My Nullid test", "nullid.opt", "nullid"); + NULLID("My Nullid test", "nullid.opt", "null-nodeid-example"); private final String filename; private final String templateId;