Skip to content

Commit

Permalink
issues #3654 and #3661 - better support for versioned extensions
Browse files Browse the repository at this point in the history
1. Added a FHIRTermService.lookup overload which allows the caller to
pass the specific version of the CodeSystem to use. Previously, I found
that we were passing a CodeSystem to validateCode but this method
delegated to lookup which fetched its own CodeSystem. This was
problematic when the Coding didn't specify a particular CodeSystem
version when the underlying profile constraint did.

2. Moved logic for generating `conformsTo` constraints (generated-ext-1)
for each extension in the resource instance. Now it will be grouped with
the rest of the constraints and we can introspect those other
constraints to shape the constraint generation.
  - To implement this, I introduced a new "PathAwareCollectingVisitor"
although alternatively I could have executed a FHIRPath like
`descendents(extension)`
  - Instead of a constraint like `conformTo(url)` which will only
validate against the latest version of this extension in the registry,
we now generate an expression like `conformTo(url|1) or conformTo(url|2)
or conformTo(url|3)` ...one clause for each version in the registry.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
  • Loading branch information
lmsurpre committed May 24, 2022
1 parent 42e7925 commit 6f5d4ef
Show file tree
Hide file tree
Showing 10 changed files with 400 additions and 104 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.util.Collection;
import java.util.List;

import org.testng.Assert;
import org.testng.annotations.Test;

import com.ibm.fhir.model.format.Format;
Expand Down Expand Up @@ -340,9 +341,12 @@ public void testUSCoreEthnicityExtension4() throws Exception {

issues.forEach(System.out::println);

assertEquals(countWarnings(issues), 0);
assertEquals(countErrors(issues), 2);
assertEquals(countInformation(issues), 1);
// one for generated-ext-1: Extension must conform to definition 'http://hl7.org/fhir/us/core/StructureDefinition/us-core-race'
// and one for each version of the extension due to the generated constraint "conformsTo('a'|1) or conformsTo('a'|2) or conformsTo('a'|3)..."
Assert.assertEquals(countErrors(issues), 4);
Assert.assertEquals(countWarnings(issues), 0);
// one for each version of the extension due to the generated constraint "conformsTo('a'|1) or conformsTo('a'|2) or conformsTo('a'|3)..."
Assert.assertEquals(countInformation(issues), 3);
}

/**
Expand Down Expand Up @@ -523,9 +527,12 @@ public void testUSCoreRaceExtension4() throws Exception {

issues.forEach(System.out::println);

assertEquals(countWarnings(issues), 1);
assertEquals(countErrors(issues), 2);
assertEquals(countInformation(issues), 1);
// one for generated-ext-1: Extension must conform to definition 'http://hl7.org/fhir/us/core/StructureDefinition/us-core-race'
// and one for each version of the extension due to the generated constraint "conformsTo('a'|1) or conformsTo('a'|2) or conformsTo('a'|3)..."
Assert.assertEquals(countErrors(issues), 4);
Assert.assertEquals(countWarnings(issues), 1);
// one for each version of the extension due to the generated constraint "conformsTo('a'|1) or conformsTo('a'|2) or conformsTo('a'|3)..."
Assert.assertEquals(countInformation(issues), 3);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -333,9 +333,12 @@ public void testUSCoreEthnicityExtension4() throws Exception {

issues.forEach(System.out::println);

// one for generated-ext-1: Extension must conform to definition 'http://hl7.org/fhir/us/core/StructureDefinition/us-core-race'
// and one for each version of the extension due to the generated constraint "conformsTo('a'|1) or conformsTo('a'|2) or conformsTo('a'|3)..."
Assert.assertEquals(countErrors(issues), 4);
Assert.assertEquals(countWarnings(issues), 1);
Assert.assertEquals(countErrors(issues), 2);
Assert.assertEquals(countInformation(issues), 1);
// one for each version of the extension due to the generated constraint "conformsTo('a'|1) or conformsTo('a'|2) or conformsTo('a'|3)..."
Assert.assertEquals(countInformation(issues), 3);
}

/**
Expand Down Expand Up @@ -516,9 +519,12 @@ public void testUSCoreRaceExtension4() throws Exception {

issues.forEach(System.out::println);

// one for generated-ext-1: Extension must conform to definition 'http://hl7.org/fhir/us/core/StructureDefinition/us-core-race'
// and one for each version of the extension due to the generated constraint "conformsTo('a'|1) or conformsTo('a'|2) or conformsTo('a'|3)..."
Assert.assertEquals(countErrors(issues), 4);
Assert.assertEquals(countWarnings(issues), 1);
Assert.assertEquals(countErrors(issues), 2);
Assert.assertEquals(countInformation(issues), 1);
// one for each version of the extension due to the generated constraint "conformsTo('a'|1) or conformsTo('a'|2) or conformsTo('a'|3)..."
Assert.assertEquals(countInformation(issues), 3);
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -333,9 +333,12 @@ public void testUSCoreEthnicityExtension4() throws Exception {

issues.forEach(System.out::println);

// one for generated-ext-1: Extension must conform to definition 'http://hl7.org/fhir/us/core/StructureDefinition/us-core-race'
// and one for each version of the extension due to the generated constraint "conformsTo('a'|1) or conformsTo('a'|2) or conformsTo('a'|3)..."
Assert.assertEquals(countErrors(issues), 4);
Assert.assertEquals(countWarnings(issues), 1);
Assert.assertEquals(countErrors(issues), 2);
Assert.assertEquals(countInformation(issues), 1);
// one for each version of the extension due to the generated constraint "conformsTo('a'|1) or conformsTo('a'|2) or conformsTo('a'|3)..."
Assert.assertEquals(countInformation(issues), 3);
}

/**
Expand Down Expand Up @@ -516,9 +519,12 @@ public void testUSCoreRaceExtension4() throws Exception {

issues.forEach(System.out::println);

// one for generated-ext-1: Extension must conform to definition 'http://hl7.org/fhir/us/core/StructureDefinition/us-core-race'
// and one for each version of the extension due to the generated constraint "conformsTo('a'|1) or conformsTo('a'|2) or conformsTo('a'|3)..."
Assert.assertEquals(countErrors(issues), 4);
Assert.assertEquals(countWarnings(issues), 1);
Assert.assertEquals(countErrors(issues), 2);
Assert.assertEquals(countInformation(issues), 1);
// one for each version of the extension due to the generated constraint "conformsTo('a'|1) or conformsTo('a'|2) or conformsTo('a'|3)..."
Assert.assertEquals(countInformation(issues), 3);
}

@Test
Expand Down
70 changes: 35 additions & 35 deletions fhir-model/src/main/java/com/ibm/fhir/model/type/Xhtml.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,19 @@

import javax.annotation.Generated;

import org.owasp.encoder.Encode;

import com.ibm.fhir.model.annotation.Required;
import com.ibm.fhir.model.util.ValidationSupport;
import com.ibm.fhir.model.visitor.Visitor;
import org.owasp.encoder.Encode;

/**
* XHTML
*/
@Generated("com.ibm.fhir.tools.CodeGenerator")
public class Xhtml extends Element {
private static final java.lang.String DIV_OPEN = "<div xmlns=\"http://www.w3.org/1999/xhtml\">";

private static final java.lang.String DIV_CLOSE = "</div>";
public static final java.lang.String DIV_OPEN = "<div xmlns=\"http://www.w3.org/1999/xhtml\">";
public static final java.lang.String DIV_CLOSE = "</div>";

@Required
private final java.lang.String value;
Expand All @@ -35,7 +35,7 @@ private Xhtml(Builder builder) {

/**
* Actual xhtml
*
*
* @return
* An immutable object of type {@link java.lang.String} that is non-null.
*/
Expand All @@ -55,7 +55,7 @@ public boolean hasChildren() {

/**
* Factory method for creating Xhtml objects from an XHTML java.lang.String
*
*
* @param value
* A java.lang.String with valid XHTML content, not null
*/
Expand All @@ -66,7 +66,7 @@ public static Xhtml of(java.lang.String value) {

/**
* Factory method for creating Xhtml objects from an XHTML java.lang.String
*
*
* @param value
* A java.lang.String with valid XHTML content, not null
*/
Expand All @@ -77,10 +77,10 @@ public static Xhtml xhtml(java.lang.String value) {

/**
* Factory method for creating Xhtml objects from a plain text string
*
*
* <p>This method will automatically encode the passed string for use within XHTML,
* then wrap it in an XHTML {@code <div>} element with a namespace of {@code http://www.w3.org/1999/xhtml}
*
*
* @param plainText
* The text to encode and wrap for use within a Narrative, not null
*/
Expand Down Expand Up @@ -116,17 +116,17 @@ public boolean equals(Object obj) {
return false;
}
Xhtml other = (Xhtml) obj;
return Objects.equals(id, other.id) &&
Objects.equals(extension, other.extension) &&
return Objects.equals(id, other.id) &&
Objects.equals(extension, other.extension) &&
Objects.equals(value, other.value);
}

@Override
public int hashCode() {
int result = hashCode;
if (result == 0) {
result = Objects.hash(id,
extension,
result = Objects.hash(id,
extension,
value);
hashCode = result;
}
Expand All @@ -151,10 +151,10 @@ private Builder() {

/**
* unique id for the element within a resource (for internal references)
*
*
* @param id
* xml:id (or equivalent in JSON)
*
*
* @return
* A reference to this Builder instance
*/
Expand All @@ -164,19 +164,19 @@ public Builder id(java.lang.String id) {
}

/**
* May be used to represent additional information that is not part of the basic definition of the resource. To make the
* use of extensions safe and manageable, there is a strict set of governance applied to the definition and use of
* extensions. Though any implementer can define an extension, there is a set of requirements that SHALL be met as part
* May be used to represent additional information that is not part of the basic definition of the resource. To make the
* use of extensions safe and manageable, there is a strict set of governance applied to the definition and use of
* extensions. Though any implementer can define an extension, there is a set of requirements that SHALL be met as part
* of the definition of the extension.
*
*
* <p>Adds new element(s) to the existing list.
* If any of the elements are null, calling {@link #build()} will fail.
*
*
* <p>This element is prohibited.
*
*
* @param extension
* Additional content defined by implementations
*
*
* @return
* A reference to this Builder instance
*/
Expand All @@ -186,22 +186,22 @@ public Builder extension(Extension... extension) {
}

/**
* May be used to represent additional information that is not part of the basic definition of the resource. To make the
* use of extensions safe and manageable, there is a strict set of governance applied to the definition and use of
* extensions. Though any implementer can define an extension, there is a set of requirements that SHALL be met as part
* May be used to represent additional information that is not part of the basic definition of the resource. To make the
* use of extensions safe and manageable, there is a strict set of governance applied to the definition and use of
* extensions. Though any implementer can define an extension, there is a set of requirements that SHALL be met as part
* of the definition of the extension.
*
*
* <p>Replaces the existing list with a new one containing elements from the Collection.
* If any of the elements are null, calling {@link #build()} will fail.
*
*
* <p>This element is prohibited.
*
*
* @param extension
* Additional content defined by implementations
*
*
* @return
* A reference to this Builder instance
*
*
* @throws NullPointerException
* If the passed collection is null
*/
Expand All @@ -212,12 +212,12 @@ public Builder extension(Collection<Extension> extension) {

/**
* Actual xhtml
*
*
* <p>This element is required.
*
*
* @param value
* Actual xhtml
*
*
* @return
* A reference to this Builder instance
*/
Expand All @@ -228,12 +228,12 @@ public Builder value(java.lang.String value) {

/**
* Build the {@link Xhtml}
*
*
* <p>Required elements:
* <ul>
* <li>value</li>
* </ul>
*
*
* @return
* An immutable object of type {@link Xhtml}
* @throws IllegalStateException
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
/*
* (C) Copyright IBM Corp. 2022
*
* SPDX-License-Identifier: Apache-2.0
*/
package com.ibm.fhir.model.visitor;

import java.math.BigDecimal;
import java.time.LocalDate;
import java.time.LocalTime;
import java.time.Year;
import java.time.YearMonth;
import java.time.ZonedDateTime;
import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.Map;

/**
* Visits a Resource or Element and collects all of its descendants of a given type into a collection
* of those elements, indexed by their simple FHIRPath path.
*
* @param <T> The type of object to collect
* @implNote The order of the list will be consistent with a depth-first traversal of the visited object
*/
public class PathAwareCollectingVisitor<T> extends PathAwareVisitor {
protected final Map<String, T> result = new LinkedHashMap<>();
protected final Class<T> type;

public PathAwareCollectingVisitor(Class<T> type) {
super();
this.type = type;
}

protected void collect(Object object) {
if (type.isInstance(object)) {
result.put(getPath(), type.cast(object));
}
}

public Map<String, T> getResult() {
return Collections.unmodifiableMap(result);
}

@Override
public boolean visit(java.lang.String elementName, int elementIndex, Visitable visitable) {
collect(visitable);
return true;
}

@Override
public void doVisit(java.lang.String elementName, byte[] value) {
collect(value);
}

@Override
public void doVisit(java.lang.String elementName, BigDecimal value) {
collect(value);
}

@Override
public void doVisit(java.lang.String elementName, java.lang.Boolean value) {
collect(value);
}

@Override
public void doVisit(java.lang.String elementName, java.lang.Integer value) {
collect(value);
}

@Override
public void doVisit(java.lang.String elementName, LocalDate value) {
collect(value);
}

@Override
public void doVisit(java.lang.String elementName, LocalTime value) {
collect(value);
}

@Override
public void doVisit(java.lang.String elementName, java.lang.String value) {
collect(value);
}

@Override
public void doVisit(java.lang.String elementName, Year value) {
collect(value);
}

@Override
public void doVisit(java.lang.String elementName, YearMonth value) {
collect(value);
}

@Override
public void doVisit(java.lang.String elementName, ZonedDateTime value) {
collect(value);
}
}
Loading

0 comments on commit 6f5d4ef

Please sign in to comment.