From 471073103f963e7fa1a7d8170b0dd862bed2c80a Mon Sep 17 00:00:00 2001 From: James Agnew Date: Tue, 28 Jan 2020 14:36:15 -0500 Subject: [PATCH 1/6] Further reduce memory usage in the terminology delta operations --- .../hapi/fhir/changelog/4_2_0/changes.yaml | 2 +- .../term/TermCodeSystemStorageSvcImpl.java | 115 +++--------------- .../jpa/term/TerminologySvcDeltaR4Test.java | 82 ++++++------- 3 files changed, 57 insertions(+), 142 deletions(-) diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/4_2_0/changes.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/4_2_0/changes.yaml index 51c674c14d9..5aff4b3cbf7 100644 --- a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/4_2_0/changes.yaml +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/4_2_0/changes.yaml @@ -84,4 +84,4 @@ Sean McIlvenna for reporting!" Petro Mykhailysyn for the pull request!" - item: type: "fix" - title: "A meomery leak was resolved in the JPA terminology service delta upload operations." + title: "A memory leak was resolved in the JPA terminology service delta upload operations." diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/TermCodeSystemStorageSvcImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/TermCodeSystemStorageSvcImpl.java index a4abf99c328..43648e3e99c 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/TermCodeSystemStorageSvcImpl.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/TermCodeSystemStorageSvcImpl.java @@ -164,86 +164,12 @@ public class TermCodeSystemStorageSvcImpl implements ITermCodeSystemStorageSvc { IIdType codeSystemId = cs.getResource().getIdDt(); - // Load all concepts for the code system - Map codeToConceptPid = new HashMap<>(); - { - ourLog.info("Loading all concepts in CodeSystem versionPid[{}] and url[{}]", cs.getPid(), theSystem); - StopWatch sw = new StopWatch(); - CriteriaBuilder criteriaBuilder = myEntityManager.getCriteriaBuilder(); - CriteriaQuery query = criteriaBuilder.createQuery(TermConcept.class); - Root root = query.from(TermConcept.class); - Predicate predicate = criteriaBuilder.equal(root.get("myCodeSystemVersionPid").as(Long.class), csv.getPid()); - query.where(predicate); - TypedQuery typedQuery = myEntityManager.createQuery(query.select(root)); - org.hibernate.query.Query hibernateQuery = (org.hibernate.query.Query) typedQuery; - ScrollableResults scrollableResults = hibernateQuery.scroll(ScrollMode.FORWARD_ONLY); - int count = 0; - try (ScrollableResultsIterator scrollableResultsIterator = new ScrollableResultsIterator<>(scrollableResults)) { - while (scrollableResultsIterator.hasNext()) { - TermConcept next = scrollableResultsIterator.next(); - codeToConceptPid.put(next.getCode(), next.getId()); - - // We don't want to keep the loaded entities in the L1 cache because they can take up a loooot of memory - if (count % 100 == 0) { - myEntityManager.clear(); - } - count++; - } - } - ourLog.info("Loaded {} concepts in {}", codeToConceptPid.size(), sw.toString()); - } - - // Load all parent/child links - ListMultimap parentCodeToChildCodes = ArrayListMultimap.create(); - ListMultimap childCodeToParentCodes = ArrayListMultimap.create(); - { - ourLog.info("Loading all parent/child relationships in CodeSystem url[" + theSystem + "]"); - int count = 0; - StopWatch sw = new StopWatch(); - CriteriaBuilder criteriaBuilder = myEntityManager.getCriteriaBuilder(); - CriteriaQuery query = criteriaBuilder.createQuery(TermConceptParentChildLink.class); - Root root = query.from(TermConceptParentChildLink.class); - Predicate predicate = criteriaBuilder.equal(root.get("myCodeSystemVersionPid").as(Long.class), csv.getPid()); - root.fetch("myChild"); - root.fetch("myParent"); - query.where(predicate); - TypedQuery typedQuery = myEntityManager.createQuery(query.select(root)); - org.hibernate.query.Query hibernateQuery = (org.hibernate.query.Query) typedQuery; - ScrollableResults scrollableResults = hibernateQuery.scroll(ScrollMode.FORWARD_ONLY); - try (ScrollableResultsIterator scrollableResultsIterator = new ScrollableResultsIterator<>(scrollableResults)) { - while (scrollableResultsIterator.hasNext()) { - TermConceptParentChildLink next = scrollableResultsIterator.next(); - String parentCode = next.getParent().getCode(); - String childCode = next.getChild().getCode(); - parentCodeToChildCodes.put(parentCode, childCode); - childCodeToParentCodes.put(childCode, parentCode); - - // We don't want to keep the loaded entities in the L1 cache because they can take up a loooot of memory - if (count % 100 == 0) { - myEntityManager.clear(); - } - - count++; - } - } - ourLog.info("Loaded {} parent/child relationships in {}", count, sw.toString()); - } - - ourLog.trace("Starting delta application"); - - // Account for root codes in the parent->child map - for (String nextCode : codeToConceptPid.keySet()) { - if (childCodeToParentCodes.get(nextCode).isEmpty()) { - parentCodeToChildCodes.put("", nextCode); - } - } - UploadStatistics retVal = new UploadStatistics(codeSystemId); // Add root concepts for (TermConcept nextRootConcept : theAdditions.getRootConcepts()) { List parentCodes = Collections.emptyList(); - addConcept(csv, codeToConceptPid, parentCodes, nextRootConcept, parentCodeToChildCodes, retVal, theAdditions.getRootConceptCodes(), true); + addConcept(csv, parentCodes, nextRootConcept, retVal, true, 0); } return retVal; @@ -519,48 +445,37 @@ public class TermCodeSystemStorageSvcImpl implements ITermCodeSystemStorageSvc { Validate.isTrue(myContext.getVersion().getVersion().isEqualOrNewerThan(FhirVersionEnum.DSTU3), "Terminology operations only supported in DSTU3+ mode"); } - private void addConcept(TermCodeSystemVersion theCsv, Map theCodeToConceptPid, Collection theParentCodes, TermConcept theConceptToAdd, ListMultimap theParentCodeToChildCodes, UploadStatistics theStatisticsTracker, Set theAdditionSetRootConceptCodes, boolean theRootConcept) { + private void addConcept(TermCodeSystemVersion theCsv, Collection theParentCodes, TermConcept theConceptToAdd, UploadStatistics theStatisticsTracker, boolean theRootConcept, int theSequence) { TermConcept conceptToAdd = theConceptToAdd; List childrenToAdd = theConceptToAdd.getChildren(); String nextCodeToAdd = conceptToAdd.getCode(); String parentDescription = "(root concept)"; Set parentConcepts = new HashSet<>(); + if (!theParentCodes.isEmpty()) { parentDescription = "[" + String.join(", ", theParentCodes) + "]"; for (String nextParentCode : theParentCodes) { - Long nextParentCodePid = theCodeToConceptPid.get(nextParentCode); - if (nextParentCodePid == null) { + Optional nextParentOpt = myConceptDao.findByCodeSystemAndCode(theCsv, nextParentCode); + if (nextParentOpt.isPresent() == false) { throw new InvalidRequestException("Unable to add code \"" + nextCodeToAdd + "\" to unknown parent: " + nextParentCode); } - parentConcepts.add(myConceptDao.getOne(nextParentCodePid)); + parentConcepts.add(nextParentOpt.get()); } } ourLog.info("Saving concept {} with parent {}", theStatisticsTracker.getUpdatedConceptCount(), parentDescription); - if (theCodeToConceptPid.containsKey(nextCodeToAdd)) { - - TermConcept existingCode = myConceptDao.getOne(theCodeToConceptPid.get(nextCodeToAdd)); + Optional existingCodeOpt = myConceptDao.findByCodeSystemAndCode(theCsv, nextCodeToAdd); + if (existingCodeOpt.isPresent()) { + TermConcept existingCode = existingCodeOpt.get(); existingCode.setIndexStatus(null); existingCode.setDisplay(conceptToAdd.getDisplay()); conceptToAdd = existingCode; - } - if (conceptToAdd.getSequence() == null || !theRootConcept) { - // If this is a new code, give it a sequence number based on how many concepts the - // parent already has (or the highest number, if the code has multiple parents) - int sequence = 0; - for (String nextParentCode : theParentCodes) { - theParentCodeToChildCodes.put(nextParentCode, nextCodeToAdd); - sequence = Math.max(sequence, theParentCodeToChildCodes.get(nextParentCode).size()); - } - if (theParentCodes.isEmpty()) { - theParentCodeToChildCodes.put("", nextCodeToAdd); - sequence = Math.max(sequence, theParentCodeToChildCodes.get("").size()); - } - conceptToAdd.setSequence(sequence); + if (conceptToAdd.getSequence() == null) { + conceptToAdd.setSequence(theSequence); } // Drop any old parent-child links if they aren't explicitly specified in the @@ -586,7 +501,6 @@ public class TermCodeSystemStorageSvcImpl implements ITermCodeSystemStorageSvc { Long nextConceptPid = conceptToAdd.getId(); Validate.notNull(nextConceptPid); - theCodeToConceptPid.put(nextCodeToAdd, nextConceptPid); theStatisticsTracker.incrementUpdatedConceptCount(); // Add link to new child to the parent @@ -605,6 +519,7 @@ public class TermCodeSystemStorageSvcImpl implements ITermCodeSystemStorageSvc { ourLog.trace("About to save parent-child links"); // Save children recursively + int childIndex = 0; for (TermConceptParentChildLink nextChildConceptLink : new ArrayList<>(childrenToAdd)) { TermConcept nextChild = nextChildConceptLink.getChild(); @@ -612,15 +527,15 @@ public class TermCodeSystemStorageSvcImpl implements ITermCodeSystemStorageSvc { for (int i = 0; i < nextChild.getParents().size(); i++) { if (nextChild.getParents().get(i).getId() == null) { String parentCode = nextChild.getParents().get(i).getParent().getCode(); - Long parentPid = theCodeToConceptPid.get(parentCode); - TermConcept parentConcept = myConceptDao.findById(parentPid).orElseThrow(() -> new IllegalArgumentException("Unknown parent code: " + parentCode)); + TermConcept parentConcept = myConceptDao.findByCodeSystemAndCode(theCsv, parentCode).orElseThrow(() -> new IllegalArgumentException("Unknown parent code: " + parentCode)); nextChild.getParents().get(i).setParent(parentConcept); } } Collection parentCodes = nextChild.getParents().stream().map(t -> t.getParent().getCode()).collect(Collectors.toList()); - addConcept(theCsv, theCodeToConceptPid, parentCodes, nextChild, theParentCodeToChildCodes, theStatisticsTracker, theAdditionSetRootConceptCodes, false); + addConcept(theCsv, parentCodes, nextChild, theStatisticsTracker, false, childIndex); + childIndex++; } } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/TerminologySvcDeltaR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/TerminologySvcDeltaR4Test.java index ffb704b7a68..f663e7357b5 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/TerminologySvcDeltaR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/TerminologySvcDeltaR4Test.java @@ -47,8 +47,8 @@ public class TerminologySvcDeltaR4Test extends BaseJpaR4Test { delta.addRootConcept("RootB", "Root B"); myTermCodeSystemStorageSvc.applyDeltaCodeSystemsAdd("http://foo/cs", delta); assertHierarchyContains( - "RootA seq=1", - "RootB seq=2" + "RootA seq=0", + "RootB seq=0" ); delta = new CustomTerminologySet(); @@ -56,10 +56,10 @@ public class TerminologySvcDeltaR4Test extends BaseJpaR4Test { delta.addRootConcept("RootD", "Root D"); myTermCodeSystemStorageSvc.applyDeltaCodeSystemsAdd("http://foo/cs", delta); assertHierarchyContains( - "RootA seq=1", - "RootB seq=2", - "RootC seq=3", - "RootD seq=4" + "RootA seq=0", + "RootB seq=0", + "RootC seq=0", + "RootD seq=0" ); } @@ -103,8 +103,8 @@ public class TerminologySvcDeltaR4Test extends BaseJpaR4Test { delta.addRootConcept("RootB", "Root B"); myTermCodeSystemStorageSvc.applyDeltaCodeSystemsAdd("http://foo/cs", delta); assertHierarchyContains( - "RootA seq=1", - "RootB seq=2" + "RootA seq=0", + "RootB seq=0" ); ourLog.info("Have performed add"); @@ -120,10 +120,10 @@ public class TerminologySvcDeltaR4Test extends BaseJpaR4Test { root.addChild(TermConceptParentChildLink.RelationshipTypeEnum.ISA).setCode("ChildAB").setDisplay("Child AB"); myTermCodeSystemStorageSvc.applyDeltaCodeSystemsAdd("http://foo/cs", delta); assertHierarchyContains( - "RootA seq=1", - " ChildAA seq=1", - " ChildAB seq=2", - "RootB seq=2" + "RootA seq=0", + " ChildAA seq=0", + " ChildAB seq=1", + "RootB seq=0" ); } @@ -142,10 +142,10 @@ public class TerminologySvcDeltaR4Test extends BaseJpaR4Test { delta.addRootConcept("RootB", "Root B"); outcome = myTermCodeSystemStorageSvc.applyDeltaCodeSystemsAdd("http://foo/cs", delta); assertHierarchyContains( - "RootA seq=1", - " ChildAA seq=1", - " ChildAAA seq=1", - "RootB seq=2" + "RootA seq=0", + " ChildAA seq=0", + " ChildAAA seq=0", + "RootB seq=0" ); assertEquals(4, outcome.getUpdatedConceptCount()); @@ -154,10 +154,10 @@ public class TerminologySvcDeltaR4Test extends BaseJpaR4Test { .addChild(TermConceptParentChildLink.RelationshipTypeEnum.ISA).setCode("ChildAA").setDisplay("Child AA"); outcome = myTermCodeSystemStorageSvc.applyDeltaCodeSystemsAdd("http://foo/cs", delta); assertHierarchyContains( - "RootA seq=1", - "RootB seq=2", - " ChildAA seq=1", - " ChildAAA seq=1" + "RootA seq=0", + "RootB seq=0", + " ChildAA seq=0", + " ChildAAA seq=0" ); assertEquals(2, outcome.getUpdatedConceptCount()); @@ -199,8 +199,8 @@ public class TerminologySvcDeltaR4Test extends BaseJpaR4Test { // Check so far assertHierarchyContains( - "ParentA seq=1", - " ChildA seq=1" + "ParentA seq=0", + " ChildA seq=0" ); // Add sub-child to existing child @@ -212,9 +212,9 @@ public class TerminologySvcDeltaR4Test extends BaseJpaR4Test { // Check so far assertHierarchyContains( - "ParentA seq=1", - " ChildA seq=1", - " ChildAA seq=1" + "ParentA seq=0", + " ChildA seq=0", + " ChildAA seq=0" ); } @@ -285,14 +285,14 @@ public class TerminologySvcDeltaR4Test extends BaseJpaR4Test { UploadStatistics outcome = myTermCodeSystemStorageSvc.applyDeltaCodeSystemsAdd("http://foo/cs", delta); assertEquals(8, outcome.getUpdatedConceptCount()); assertHierarchyContains( - "CodeA seq=1", - " CodeAA seq=1", - " CodeAAA seq=1", - " CodeAAB seq=2", - "CodeB seq=2", - " CodeBA seq=1", - " CodeBAA seq=1", - " CodeBAB seq=2" + "CodeA seq=0", + " CodeAA seq=0", + " CodeAAA seq=0", + " CodeAAB seq=1", + "CodeB seq=0", + " CodeBA seq=0", + " CodeBAA seq=0", + " CodeBAB seq=1" ); // Move a single child code to a new spot and make sure the hierarchy comes along @@ -304,14 +304,14 @@ public class TerminologySvcDeltaR4Test extends BaseJpaR4Test { outcome = myTermCodeSystemStorageSvc.applyDeltaCodeSystemsAdd("http://foo/cs", delta); assertEquals(2, outcome.getUpdatedConceptCount()); assertHierarchyContains( - "CodeA seq=1", - "CodeB seq=2", - " CodeBA seq=1", - " CodeBAA seq=1", - " CodeBAB seq=2", - " CodeAA seq=2", // <-- CodeAA got added here so it comes second - " CodeAAA seq=1", - " CodeAAB seq=2" + "CodeA seq=0", + "CodeB seq=0", + " CodeBA seq=0", + " CodeBAA seq=0", + " CodeBAB seq=1", + " CodeAA seq=0", // <-- CodeAA got added here so it comes second + " CodeAAA seq=0", + " CodeAAB seq=1" ); } From 9548e74b17464158906d9af6a14d5cc4e4ae228f Mon Sep 17 00:00:00 2001 From: James Agnew Date: Tue, 28 Jan 2020 15:46:46 -0500 Subject: [PATCH 2/6] Fix a test i missed --- .../provider/r4/TerminologyUploaderProviderR4Test.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/TerminologyUploaderProviderR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/TerminologyUploaderProviderR4Test.java index 1eafcc637d7..a084cf4217f 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/TerminologyUploaderProviderR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/TerminologyUploaderProviderR4Test.java @@ -244,11 +244,11 @@ public class TerminologyUploaderProviderR4Test extends BaseResourceProviderR4Tes )); assertHierarchyContains( - "CHEM seq=1", - " HB seq=1", - " NEUT seq=2", - "MICRO seq=2", - " C&S seq=1" + "CHEM seq=0", + " HB seq=0", + " NEUT seq=1", + "MICRO seq=0", + " C&S seq=0" ); } From 95254406bc4b25180f7691cbc5b3c4b1b669eaef Mon Sep 17 00:00:00 2001 From: dionmcm Date: Wed, 29 Jan 2020 12:48:25 +1000 Subject: [PATCH 3/6] updated custom narrative generation documentation I found that the documented pattern of [name].class=[fully qualified classname] No longer works. The pattern of [name].resourceType=[ResourceType] worked for me, I updated the datatype example as well in kind, but I've not used it that way. Maybe some extended examples for the other property type extensions would be good - e.g. .profile, .style, .contextPath, .narrative, .title --- .../ca/uhn/hapi/fhir/docs/model/narrative_generation.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/docs/model/narrative_generation.md b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/docs/model/narrative_generation.md index 697d5a04d61..8d709a3bdce 100644 --- a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/docs/model/narrative_generation.md +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/docs/model/narrative_generation.md @@ -64,7 +64,7 @@ The first (name.class) defines the class name of the resource to define a templa ```properties # Two property lines in the file per template -practitioner.class=ca.uhn.fhir.model.dstu.resource.Practitioner +practitioner.resourceType=Practitioner practitioner.narrative=file:src/test/resources/narrative/Practitioner.html observation.class=ca.uhn.fhir.model.dstu.resource.Observation @@ -77,7 +77,7 @@ You may also override/define behaviour for datatypes. These datatype narrative d ```properties # datatypes use the same format as resources -humanname.class=ca.uhn.fhir.model.dstu.composite.HumanNameDt +humanname.resourceType=HumanNameDt humanname.narrative=classpath:ca/uhn/fhir/narrative/HumanNameDt.html]]> ``` From beab5b3a9c9f11ca8670b805215421c916636a5b Mon Sep 17 00:00:00 2001 From: jamesagnew Date: Wed, 29 Jan 2020 05:56:36 -0500 Subject: [PATCH 4/6] Credit for #1689 --- .../resources/ca/uhn/hapi/fhir/changelog/4_2_0/changes.yaml | 5 +++++ pom.xml | 3 +++ 2 files changed, 8 insertions(+) diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/4_2_0/changes.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/4_2_0/changes.yaml index 5aff4b3cbf7..c9429bc3292 100644 --- a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/4_2_0/changes.yaml +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/4_2_0/changes.yaml @@ -85,3 +85,8 @@ Sean McIlvenna for reporting!" - item: type: "fix" title: "A memory leak was resolved in the JPA terminology service delta upload operations." +- item: + type: "fix" + issue: 1689 + title: "A correction was made to the narrative generation documentation. Thanks to GitHub user + dionmcm for the pull request!" diff --git a/pom.xml b/pom.xml index 0364f1a9b00..890013750e2 100644 --- a/pom.xml +++ b/pom.xml @@ -587,6 +587,9 @@ jiaola Dazhi Jiao + + dionmcm + From 00fdc978cc37b02bf24f7884f454c52aa4b8544c Mon Sep 17 00:00:00 2001 From: dionmcm Date: Wed, 29 Jan 2020 12:58:41 +1000 Subject: [PATCH 5/6] narrative gen removed old code and enhanced error message I ran into an issue trying to use the old .class style property. The validation message was useful, but I spotted some unreachable code investigating which I've removed. I also enhanced the configuration exception to indicate what was wrong with a supplied key and expected extension types. --- .../narrative2/NarrativeTemplateManifest.java | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/narrative2/NarrativeTemplateManifest.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/narrative2/NarrativeTemplateManifest.java index cadaf3855b5..60c65a9300f 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/narrative2/NarrativeTemplateManifest.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/narrative2/NarrativeTemplateManifest.java @@ -155,18 +155,6 @@ public class NarrativeTemplateManifest implements INarrativeTemplateManifest { .map(t -> t.trim()) .filter(t -> isNotBlank(t)) .forEach(t -> nextTemplate.addAppliesToDatatype(t)); - } else if (nextKey.endsWith(".class")) { - String className = file.getProperty(nextKey); - Class clazz; - try { - clazz = (Class) Class.forName(className); - } catch (ClassNotFoundException e) { - ourLog.debug("Unknown datatype class '{}' identified in manifest", name); - clazz = null; - } - if (clazz != null) { - nextTemplate.addAppliesToResourceClass(clazz); - } } else if (nextKey.endsWith(".style")) { String templateTypeName = file.getProperty(nextKey).toUpperCase(); TemplateTypeEnum templateType = TemplateTypeEnum.valueOf(templateTypeName); @@ -183,7 +171,9 @@ public class NarrativeTemplateManifest implements INarrativeTemplateManifest { } else if (nextKey.endsWith(".title")) { ourLog.debug("Ignoring title property as narrative generator no longer generates titles: {}", nextKey); } else { - throw new ConfigurationException("Invalid property name: " + nextKey); + throw new ConfigurationException("Invalid property name: " + nextKey + + " - the key must end in one of the expected extensions " + + "'.profile', '.resourceType', '.dataType', '.style', '.contextPath', '.narrative', '.title'"); } } From a6f3ee83199e53d34d7a881c3cad16c081b637bc Mon Sep 17 00:00:00 2001 From: James Agnew Date: Wed, 29 Jan 2020 09:35:22 -0500 Subject: [PATCH 6/6] Add a unit test --- .../jpa/stresstest/StressTestParserTest.java | 47 ++++++++++++------- .../ca/uhn/fhir/parser/JsonParserR4Test.java | 13 +++++ 2 files changed, 43 insertions(+), 17 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/stresstest/StressTestParserTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/stresstest/StressTestParserTest.java index 99a9759f409..493d493239e 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/stresstest/StressTestParserTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/stresstest/StressTestParserTest.java @@ -20,27 +20,40 @@ public class StressTestParserTest extends BaseTest { FhirContext ctx = FhirContext.forR4(); String input = loadResource("/org/hl7/fhir/r4/model/valueset/valuesets.xml"); - String json = ctx.newJsonParser().encodeResourceToString(ctx.newXmlParser().parseResource(input)); + Bundle parsed = ctx.newXmlParser().parseResource(Bundle.class, input); + String json = ctx.newJsonParser().encodeResourceToString(parsed); StopWatch sw = null; - int loops = 100; + int loops = 200; + +// for (int i = 0; i < loops; i++) { +// ctx.newXmlParser().parseResource(input); +// if (i < 50) { +// ourLog.info("Parsed XML {} times", i); +// continue; +// } else if (i == 50) { +// sw = new StopWatch(); +// continue; +// } +// ourLog.info("Parsed XML {} times - {}ms/pass", i, sw.getMillisPerOperation(i - 50)); +// } + +// for (int i = 0; i < loops; i++) { +// Bundle parsed = (Bundle) ctx.newJsonParser().parseResource(json); +// if (i < 50) { +// ourLog.info("Parsed JSON with {} entries {} times", parsed.getEntry().size(), i); +// continue; +// } else if (i == 50) { +// sw = new StopWatch(); +// continue; +// } +// ourLog.info("Parsed JSON {} times - {}ms/pass", i, sw.getMillisPerOperation(i - 50)); +// } for (int i = 0; i < loops; i++) { - ctx.newXmlParser().parseResource(input); + ctx.newJsonParser().encodeResourceToString(parsed); if (i < 50) { - ourLog.info("Parsed XML {} times", i); - continue; - } else if (i == 50) { - sw = new StopWatch(); - continue; - } - ourLog.info("Parsed XML {} times - {}ms/pass", i, sw.getMillisPerOperation(i - 50)); - } - - for (int i = 0; i < loops; i++) { - Bundle parsed = (Bundle) ctx.newJsonParser().parseResource(json); - if (i < 50) { - ourLog.info("Parsed JSON with {} entries {} times", parsed.getEntry().size(), i); + ourLog.info("Serialized JSON with {} entries {} times", parsed.getEntry().size(), i); continue; } else if (i == 50) { sw = new StopWatch(); @@ -49,6 +62,6 @@ public class StressTestParserTest extends BaseTest { ourLog.info("Parsed JSON {} times - {}ms/pass", i, sw.getMillisPerOperation(i - 50)); } - } + } diff --git a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/parser/JsonParserR4Test.java b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/parser/JsonParserR4Test.java index cc5dfa8e9d7..2f3d7d92a5b 100644 --- a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/parser/JsonParserR4Test.java +++ b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/parser/JsonParserR4Test.java @@ -79,6 +79,19 @@ public class JsonParserR4Test extends BaseTest { assertThat(output, containsString("\"Questionnaire/123/_history/456\"")); } + @Test + public void testPrettyPrint() { + ourCtx.getParserOptions().setDontStripVersionsFromReferencesAtPaths("QuestionnaireResponse.questionnaire"); + + QuestionnaireResponse qr = new QuestionnaireResponse(); + qr.getQuestionnaireElement().setValueAsString("Questionnaire/123/_history/456"); + + String output = ourCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(qr); + ourLog.info(output); + + assertThat(output, containsString("\n \"resourceType\"")); + } + /** * See #814 */