Skip to content

Commit

Permalink
Merge pull request #145 from HL7/gg-work-117
Browse files Browse the repository at this point in the history 
Fixes for 1.1.7
  • Loading branch information
@grahamegrieve
grahamegrieve authored Aug 3, 2020
2 parents 7e48c4a + cc71ab3 commit a6711a4
Show file tree
Hide file tree
Showing 5 changed files with 277 additions and 25 deletions.
7 changes: 7 additions & 0 deletions RELEASE_NOTES.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
* Renderer: check that pre-existing narratives do not identify as "Generated narrative"
* Renderer: fix broken links in Bundle rendering
* Renderer: fill out narrative for Parameters resource
* Renderer: major overhaul of DiagnosticReport rendering
* Validator: better validation of version specific profiles in meta.profile
* Validator: look for references inside other parameters in Parameters resource
* Previous Version Comparison: Fix NPE bug in value set comparison
53 changes: 40 additions & 13 deletions ...ublisher.core/src/main/java/org/hl7/fhir/igtools/publisher/PreviousVersionComparator.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
package org.hl7.fhir.igtools.publisher;

import java.io.File;
import java.io.FileInputStream;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.InputStream;
import java.text.DateFormat;
Expand Down Expand Up @@ -30,6 +33,7 @@
import org.hl7.fhir.r5.model.StructureDefinition;
import org.hl7.fhir.r5.model.UsageContext;
import org.hl7.fhir.r5.utils.KeyGenerator;
import org.hl7.fhir.utilities.IniFile;
import org.hl7.fhir.utilities.Logger;
import org.hl7.fhir.utilities.Utilities;
import org.hl7.fhir.utilities.VersionUtilities;
Expand Down Expand Up @@ -76,10 +80,12 @@ private class VersionInstance {
private SimpleWorkerContext context;
private List<CanonicalResource> resources = new ArrayList<>();
private String errMsg;
private IniFile ini;

public VersionInstance(String version) {
public VersionInstance(String version, IniFile ini) {
super();
this.version = version;
this.ini = ini;
}
}

Expand All @@ -96,22 +102,23 @@ public VersionInstance(String version) {
private ILoggingService logger;
private List<CanonicalResource> resources;

public PreviousVersionComparator(SimpleWorkerContext context, String version, String dstDir, String canonical, ProfileKnowledgeProvider pkp, ILoggingService logger, List<String> versions) {
public PreviousVersionComparator(SimpleWorkerContext context, String version, String rootDir, String dstDir, String canonical, ProfileKnowledgeProvider pkp, ILoggingService logger, List<String> versions) {
super();

this.context = context;
this.version = version;
this.dstDir = dstDir;
this.keygen = new KeyGenerator(canonical);
this.pkp = pkp;
this.logger = logger;
try {
processVersions(canonical, versions);
processVersions(canonical, versions, rootDir);
} catch (Exception e) {
errMsg = "Unable to find version history at "+canonical+" ("+e.getMessage()+")";
}
}

private void processVersions(String canonical, List<String> versions) throws IOException {
private void processVersions(String canonical, List<String> versions, String rootDir) throws IOException {
JsonArray publishedVersions = null;
for (String v : versions) {
if (Utilities.existsInList(v, "{last}", "{current}")) {
Expand All @@ -137,22 +144,31 @@ private void processVersions(String canonical, List<String> versions) throws IOE
if(last == null) {
throw new FHIRException("no {last} version found in package-list.json");
} else {
versionList.add(new VersionInstance(last));
versionList.add(new VersionInstance(last, makeIni(rootDir, last)));
}
}
if ("{current}".equals(v)) {
if(last == null) {
throw new FHIRException("no {current} version found in package-list.json");
} else {
versionList.add(new VersionInstance(major));
versionList.add(new VersionInstance(major, makeIni(rootDir, major)));
}
}
} else {
versionList.add(new VersionInstance(v));
versionList.add(new VersionInstance(v, makeIni(rootDir, v)));
}
}
}

private IniFile makeIni(String rootDir, String v) throws IOException {
File ini = new File(Utilities.path(rootDir, "url-map-v-"+v+".ini"));
if (ini.exists()) {
return new IniFile(new FileInputStream(ini));
} else {
return null;
}
}

private JsonArray fetchVersionHistory(String canonical) {
try {
String ppl = Utilities.pathURL(canonical, "package-list.json");
Expand All @@ -172,7 +188,7 @@ private JsonArray fetchVersionHistory(String canonical) {
}
}
} catch (Exception e) {
throw new FHIRException("Problem with package-lists.json at "+canonical+": "+e.getMessage(), e);
throw new FHIRException("Problem with package-list.json at "+canonical+": "+e.getMessage(), e);
}
}

Expand Down Expand Up @@ -224,12 +240,13 @@ public void finishChecks() throws IOException {
for (VersionInstance vi : versionList) {
Set<String> set = new HashSet<>();
for (CanonicalResource rl : vi.resources) {
comparisons.add(new ProfilePair(rl, findByUrl(rl.getUrl(), resources)));
comparisons.add(new ProfilePair(rl, findByUrl(rl.getUrl(), resources, vi.ini)));
set.add(rl.getUrl());
}
for (CanonicalResource rr : resources) {
if (!set.contains(rr.getUrl())) {
comparisons.add(new ProfilePair(findByUrl(rr.getUrl(), vi.resources), rr));
String url = fixForIniMap(rr.getUrl(), vi.ini);
if (!set.contains(url)) {
comparisons.add(new ProfilePair(findByUrl(url, vi.resources, null), rr));
}
}

Expand All @@ -254,6 +271,16 @@ public void finishChecks() throws IOException {
}
}
}
private String fixForIniMap(String url, IniFile ini) {
if (ini == null) {
return url;
}
if (ini.hasProperty("urls", url)) {
return ini.getStringProperty("urls", url);
}
return url;
}

//
// private void buildindexPage(String path) throws IOException {
// StringBuilder b = new StringBuilder();
Expand Down Expand Up @@ -301,9 +328,9 @@ public void finishChecks() throws IOException {
// }


private CanonicalResource findByUrl(String url, List<CanonicalResource> list) {
private CanonicalResource findByUrl(String url, List<CanonicalResource> list, IniFile ini) {
for (CanonicalResource r : list) {
if (r.getUrl().equals(url)) {
if (fixForIniMap(r.getUrl(), ini).equals(url)) {
return r;
}
}
Expand Down
74 changes: 64 additions & 10 deletions org.hl7.fhir.publisher.core/src/main/java/org/hl7/fhir/igtools/publisher/Publisher.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,7 @@
import org.hl7.fhir.r5.openapi.OpenApiGenerator;
import org.hl7.fhir.r5.openapi.Writer;
import org.hl7.fhir.r5.renderers.BundleRenderer;
import org.hl7.fhir.r5.renderers.ParametersRenderer;
import org.hl7.fhir.r5.renderers.RendererFactory;
import org.hl7.fhir.r5.renderers.utils.BaseWrappers.ResourceWrapper;
import org.hl7.fhir.r5.renderers.utils.DirectWrappers;
Expand All @@ -235,6 +236,7 @@
import org.hl7.fhir.r5.renderers.utils.RenderingContext.ResourceRendererMode;
import org.hl7.fhir.r5.renderers.utils.Resolver.IReferenceResolver;
import org.hl7.fhir.r5.renderers.utils.Resolver.ResourceContext;
import org.hl7.fhir.r5.renderers.utils.Resolver.ResourceContextType;
import org.hl7.fhir.r5.renderers.utils.Resolver.ResourceWithReference;
import org.hl7.fhir.r5.terminologies.ValueSetExpander.ValueSetExpansionOutcome;
import org.hl7.fhir.r5.test.utils.ToolsHelper;
Expand Down Expand Up @@ -526,6 +528,7 @@ public enum GenerationTool {
private String specPath;
private String qaDir;
private String version;
private long fshTimeout = FSH_TIMEOUT;
private Map<String, String> suppressedMessages = new HashMap<>();

private String igName;
Expand Down Expand Up @@ -849,8 +852,6 @@ public void createIg() throws Exception, IOException, EOperationOutcome, FHIRExc
long startTime = System.nanoTime();
log("Processing Conformance Resources");
loadConformance();
log("Generating Narratives");
generateNarratives();
log("Validating Resources");
try {
validate();
Expand Down Expand Up @@ -1040,6 +1041,16 @@ public ResourceWithReference resolve(RenderingContext context, String url) {
return new ResourceWithReference(path, new DirectWrappers.ResourceWrapperDirect(context, r.getResource()));
}
}
if (r.getElement().fhirType().equals("Bundle")) {
List<Element> entries = r.getElement().getChildrenByName("entry");
for (Element entry : entries) {
Element res = entry.getNamedChild("resource");
if (res != null && res.fhirType().equals(parts[0]) && res.getNamedChildValue("id").equals(parts[1])) {
String path = igpkp.getLinkFor(r, true)+"#"+parts[0]+"_"+parts[1];
return new ResourceWithReference(path, new ElementWrappers.ResourceWrapperMetaElement(context, r.getElement()));
}
}
}
}
}
for (SpecMapManager sp : specMaps) {
Expand All @@ -1060,7 +1071,7 @@ public ResourceWithReference resolve(RenderingContext context, String url) {
private void generateNarratives() throws Exception {
logDebugMessage(LogCategory.PROGRESS, "gen narratives");
for (FetchedFile f : fileList) {
for (FetchedResource r : f.getResources()) {
for (FetchedResource r : f.getResources()) {
if (r.getExampleUri()==null || genExampleNarratives) {
logDebugMessage(LogCategory.PROGRESS, "narrative for "+f.getName()+" : "+r.getId());
if (r.getResource() != null && isConvertableResource(r.getResource().fhirType())) {
Expand All @@ -1072,6 +1083,12 @@ private void generateNarratives() throws Exception {
} else if (r.getResource() instanceof Bundle) {
regen = true;
new BundleRenderer(rc).render((Bundle) r.getResource());
} else if (r.getResource() instanceof Parameters) {
regen = true;
Parameters p = (Parameters) r.getResource();
new ParametersRenderer(rc, new ResourceContext(ResourceContextType.PARAMETERS , p, null)).render(p);
} else if (r.getResource() instanceof DomainResource) {
checkExistingNarrative(f, r, ((DomainResource) r.getResource()).getText().getDiv());
}
if (regen)
r.setElement(convertToElement(r.getResource()));
Expand All @@ -1085,9 +1102,11 @@ private void generateNarratives() throws Exception {
Element res = e.getNamedChild("resource");
if (res!=null && "http://hl7.org/fhir/StructureDefinition/DomainResource".equals(res.getProperty().getStructure().getBaseDefinition()) && !hasNarrative(res)) {
ResourceWrapper rw = new ElementWrappers.ResourceWrapperMetaElement(lrc, res);
RendererFactory.factory(rw, lrc).render(rw);
RendererFactory.factory(rw, lrc, new ResourceContext(ResourceContextType.BUNDLE, r.getElement(), res)).render(rw);
}
}
} else if ("http://hl7.org/fhir/StructureDefinition/DomainResource".equals(r.getElement().getProperty().getStructure().getBaseDefinition()) && hasNarrative(r.getElement())) {
checkExistingNarrative(f, r, r.getElement().getNamedChild("text").getNamedChild("div").getXhtml());
}
}
} else {
Expand All @@ -1097,6 +1116,25 @@ private void generateNarratives() throws Exception {
}
}

private void checkExistingNarrative(FetchedFile f, FetchedResource r, XhtmlNode xhtml) {
boolean hasGenNarrative = scanForGeneratedNarrative(xhtml);
if (hasGenNarrative) {
f.getErrors().add(new ValidationMessage(Source.Publisher, IssueType.NOTFOUND, r.fhirType()+".text.div", "Resource has provided narrative, but the narrative indicates that it is generated - remove the narrative or fix it up", IssueSeverity.ERROR));
}
}

private boolean scanForGeneratedNarrative(XhtmlNode x) {
if (x.getContent() != null && x.getContent().contains("Generated Narrative")) {
return true;
}
for (XhtmlNode c : x.getChildNodes()) {
if (scanForGeneratedNarrative(c)) {
return true;
}
}
return false;
}

private boolean isConvertableResource(String t) {
return Utilities.existsInList(t, "StructureDefinition", "ValueSet", "CodeSystem", "Conformance", "CapabilityStatement", "Questionnaire", "NamingSystem",
"ConceptMap", "OperationOutcome", "CompartmentDefinition", "OperationDefinition", "ImplementationGuide");
Expand Down Expand Up @@ -1445,17 +1483,23 @@ public void write(int b) throws IOException {
}
}

private void runFsh(File file) throws IOException {
private void runFsh(File file) throws IOException {
File inif = new File(Utilities.path(Utilities.getDirectoryForFile(file.getAbsolutePath()), "fsh.ini"));
if (inif.exists()) {
IniFile ini = new IniFile(new FileInputStream(inif));
if (ini.hasProperty("FSH", "timeout")) {
fshTimeout = ini.getLongProperty("FSH", "timeout") * 1000;
}
}
log("Run Sushi on "+file.getAbsolutePath());
DefaultExecutor exec = new DefaultExecutor();
exec.setExitValue(0);
MySushiHandler pumpHandler = new MySushiHandler();
PumpStreamHandler pump = new PumpStreamHandler(pumpHandler);
exec.setStreamHandler(pump);
exec.setWorkingDirectory(file);
ExecuteWatchdog watchdog = new ExecuteWatchdog(FSH_TIMEOUT);
ExecuteWatchdog watchdog = new ExecuteWatchdog(fshTimeout);
exec.setWatchdog(watchdog);

try {
if (SystemUtils.IS_OS_WINDOWS)
exec.execute(org.apache.commons.exec.CommandLine.parse("cmd /C sushi ./fsh -o ."));
Expand Down Expand Up @@ -2284,7 +2328,7 @@ else if (npmName.contains("hl7") || npmName.contains("argonaut") || npmName.cont
}

private void loadPubPack() throws FHIRException, IOException {
NpmPackage npm = pcm.loadPackage("hl7.fhir.pubpack", "0.0.6");
NpmPackage npm = pcm.loadPackage("hl7.fhir.pubpack", "0.0.7");
context.loadFromPackage(npm, null);
npm = pcm.loadPackage("hl7.fhir.xver-extensions", "0.0.4");
context.loadFromPackage(npm, null);
Expand Down Expand Up @@ -3643,8 +3687,11 @@ private void loadConformance() throws Exception {
loadInfo();
for (String s : metadataResourceNames())
load(s);
log("Generating Snapshots");
generateSnapshots();
log("Generating Narratives");
generateNarratives();
log("Validating Conformance Resources");
for (String s : metadataResourceNames())
validate(s);

Expand Down Expand Up @@ -3742,7 +3789,7 @@ private PreviousVersionComparator makePreviousVersionComparator() throws IOExcep
comparisonVersions = new ArrayList<>();
comparisonVersions.add("{last}");
}
return new PreviousVersionComparator(context, version, tempDir, igpkp.getCanonical(), igpkp, logger, comparisonVersions);
return new PreviousVersionComparator(context, version, rootDir, tempDir, igpkp.getCanonical(), igpkp, logger, comparisonVersions);
}

private void checkJurisdiction(FetchedFile f, CanonicalResource resource, IssueSeverity error, String verb) {
Expand Down Expand Up @@ -4406,7 +4453,7 @@ private void validateExpression(FetchedFile f, StructureDefinition sd, FHIRPathE
}
fpe.check(null, sd, ed.getPath(), n);
} catch (Exception e) {
f.getErrors().add(new ValidationMessage(Source.ProfileValidator, IssueType.INVALID, "StructureDefinition.where(url = '"+sd.getUrl()+"').snapshot.element.where('apth = '"+ed.getPath()+"').constraint.where(key = '"+inv.getKey()+"')", e.getMessage(), IssueSeverity.ERROR));
f.getErrors().add(new ValidationMessage(Source.ProfileValidator, IssueType.INVALID, "StructureDefinition.where(url = '"+sd.getUrl()+"').snapshot.element.where('path = '"+ed.getPath()+"').constraint.where(key = '"+inv.getKey()+"')", e.getMessage(), IssueSeverity.ERROR));
}
}
}
Expand Down Expand Up @@ -7393,9 +7440,16 @@ private XhtmlNode getXhtml(FetchedFile f, FetchedResource r) throws Exception {
Bundle b = (Bundle) r.getResource();
return new BundleRenderer(rc).render(b);
}
if (r.getResource() != null && r.getResource() instanceof Parameters) {
Parameters p = (Parameters) r.getResource();
return new ParametersRenderer(rc, new ResourceContext(ResourceContextType.PARAMETERS, p, null)).render(p);
}
if (r.getElement().fhirType().equals("Bundle")) {
RenderingContext lrc = rc.copy().setParser(getTypeLoader(f, r));
return new BundleRenderer(lrc).render(new ElementWrappers.ResourceWrapperMetaElement(lrc, r.getElement()));
} else if (r.getElement().fhirType().equals("Parameters")) {
RenderingContext lrc = rc.copy().setParser(getTypeLoader(f, r));
return new ParametersRenderer(lrc, new ResourceContext(ResourceContextType.PARAMETERS, r.getElement(), r.getElement())).render(new ElementWrappers.ResourceWrapperMetaElement(lrc, r.getElement()));
} else {
return getHtmlForResource(r.getElement());
}
Expand Down
Loading

0 comments on commit a6711a4

Please sign in to comment.