Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

issue #2207 - consistently throw FHIRPatchException for bad patches #2210

Merged
merged 1 commit into from
Apr 7, 2021
Merged

issue #2207 - consistently throw FHIRPatchException for bad patches #2210

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 13 additions & 17 deletions fhir-model/src/main/java/com/ibm/fhir/model/visitor/CopyingVisitor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@
import javax.lang.model.SourceVersion;

import com.ibm.fhir.model.builder.Builder;
import com.ibm.fhir.model.resource.Bundle;
import com.ibm.fhir.model.resource.Parameters;
import com.ibm.fhir.model.resource.Resource;
import com.ibm.fhir.model.type.Code;
import com.ibm.fhir.model.type.Element;
Expand Down Expand Up @@ -171,16 +169,14 @@ private void _visitEnd(java.lang.String elementName, int index, Visitable visite
Builder<?> parentBuilder = parent.getBuilder();
Object obj = wrapper.getBuilder().build();

MethodHandle methodHandle;
Class<?> expectedType = ModelSupport.getElementType(parentBuilder.getClass().getEnclosingClass(), elementName);
if (obj != null && !expectedType.isInstance(obj)) {
throw new IllegalStateException("Expected argument of type " + expectedType + " but found " + obj.getClass());
}

try {
MethodType mt;
if ((visited instanceof Element && ModelSupport.isChoiceElement(parentBuilder.getClass().getEnclosingClass(), elementName))
|| (visited instanceof Resource && isResourceContainer(parentBuilder, elementName))) {
mt = MethodType.methodType(parentBuilder.getClass(), elementOrResource);
} else {
mt = MethodType.methodType(parentBuilder.getClass(), visited.getClass());
}
methodHandle = MethodHandles.publicLookup().findVirtual(parentBuilder.getClass(), setterName(elementName), mt);
MethodType mt = MethodType.methodType(parentBuilder.getClass(), expectedType);
MethodHandle methodHandle = MethodHandles.publicLookup().findVirtual(parentBuilder.getClass(), setterName(elementName), mt);
methodHandle.invoke(parentBuilder, obj);
} catch (Throwable t) {
throw new RuntimeException("Unexpected error while visiting " + parentBuilder.getClass() + "." + elementName, t);
Expand All @@ -191,12 +187,6 @@ private void _visitEnd(java.lang.String elementName, int index, Visitable visite
}
}

private boolean isResourceContainer(Builder<?> parentBuilder, String elementName) {
return (parentBuilder instanceof Bundle.Entry.Builder && "resource".equals(elementName)) ||
(parentBuilder instanceof Bundle.Entry.Response.Builder && "outcome".equals(elementName)) ||
(parentBuilder instanceof Parameters.Parameter.Builder && "resource".equals(elementName));
}

/**
* Subclasses may override doVisitListStart
*/
Expand All @@ -214,10 +204,16 @@ public void visitEnd(String elementName, List<? extends Visitable> visitables, C
doVisitListEnd(elementName, visitables, type);
ListWrapper listWrapper = listStack.pop();
if (listWrapper.isDirty()) {
for (Visitable obj : listWrapper.getList()) {
if (!type.isInstance(obj)) {
throw new IllegalStateException("Expected argument of type " + type + " but found " + obj.getClass());
}
}
BuilderWrapper parent = builderStack.peek();
parent.dirty(true);
Builder<?> parentBuilder = parent.getBuilder();
MethodHandles.Lookup lookup = MethodHandles.publicLookup();

MethodType mt = MethodType.methodType(parentBuilder.getClass(), java.util.Collection.class);
MethodHandle methodHandle;
try {
Expand Down
40 changes: 20 additions & 20 deletions fhir-path/src/main/java/com/ibm/fhir/path/patch/FHIRPathPatch.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* (C) Copyright IBM Corp. 2020
* (C) Copyright IBM Corp. 2020, 2021
*
* SPDX-License-Identifier: Apache-2.0
*/
Expand All @@ -26,15 +26,15 @@ public class FHIRPathPatch implements FHIRPatch {
private FHIRPathPatch(Builder builder) {
this.operations = Collections.unmodifiableList(builder.operations);
}

@Override
public <T extends Resource> T apply(T resource) throws FHIRPatchException {
for (FHIRPathPatchOperation fhirPathPatchOperation : operations) {
resource = fhirPathPatchOperation.apply(resource);
}
return resource;
}

/**
* Convert the FHIRPathPatch to a FHIR Parameters resource
*/
Expand All @@ -45,84 +45,84 @@ public Parameters toParameters() {
}
return builder.build();
}

public Builder toBuilder() {
return new Builder().from(this);
}

public static Builder builder() {
return new Builder();
}
public static class Builder {

public static class Builder {
private List<FHIRPathPatchOperation> operations = new ArrayList<>(3);

private Builder() {
// hidden constructor
}

/**
* Add an add operation to the FHIRPathPatch
*/
public Builder add(String path, String elementName, Element element) {
operations.add(new FHIRPathPatchAdd(path, elementName, element));
return this;
}

/**
* Add a delete operation to the FHIRPathPatch
*/
public Builder delete(String path) {
operations.add(new FHIRPathPatchDelete(path));
return this;
}

/**
* Add an insert operation to the FHIRPathPatch
*/
public Builder insert(String path, Element element, Integer index) {
operations.add(new FHIRPathPatchInsert(path, element, index));
return this;
}

/**
* Add a move operation to the FHIRPathPatch
*/
public Builder move(String path, Integer source, Integer destination) {
operations.add(new FHIRPathPatchMove(path, source, destination));
return this;
}

/**
* Add an add operation to the FHIRPathPatch
*/
public Builder replace(String path, Element element) {
operations.add(new FHIRPathPatchReplace(path, element));
return this;
}

/**
* Add all patch operations from the passed FHIRPathPatch
*/
public Builder from(FHIRPathPatch patch) {
operations.addAll(patch.operations);
return this;
}

/**
* Build the {@link FHIRPathPatch}
*
*
* @return
* An immutable object of type {@link FHIRPathPatch}
*/
public FHIRPathPatch build() {
return new FHIRPathPatch(this);
}
}

/**
* Parse a FHIRPathPatch from a FHIR Parameters resource
*
*
* @throws IllegalArgumentException if the Parameters object does not satisfy the requirements of a FHIRPath Patch
*/
public static FHIRPathPatch from(Parameters params) {
Expand All @@ -138,10 +138,10 @@ public static FHIRPathPatch from(Parameters params) {

return builder.build();
}

/**
* Parse the passed Parameter and add it to the builder
*
*
* @throws IllegalArgumentException if the Parameter object does not represent a valid FHIRPath Patch operation
*/
private static void addOperation(Builder builder, Parameter operation) {
Expand Down
18 changes: 10 additions & 8 deletions fhir-path/src/main/java/com/ibm/fhir/path/util/AddingVisitor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* (C) Copyright IBM Corp. 2020
* (C) Copyright IBM Corp. 2020, 2021
*
* SPDX-License-Identifier: Apache-2.0
*/
Expand All @@ -26,6 +26,7 @@ class AddingVisitor<T extends Visitable> extends CopyingVisitor<T> {
* @param parentPath a "simple" FHIRPath path to the parent of the element being added
* @param elementName the name of the element to add
* @param value the element to add
* @throws IllegalArgumentException
*/
public AddingVisitor(Visitable parent, String parentPath, String elementName, Visitable value) {
this.path = Objects.requireNonNull(parentPath);
Expand All @@ -34,17 +35,18 @@ public AddingVisitor(Visitable parent, String parentPath, String elementName, Vi
this.value = Objects.requireNonNull(value) instanceof Code ?
convertToCodeSubtype(parent, elementName, (Code)value) : value;
}

@Override
protected void doVisitListEnd(String elementName, List<? extends Visitable> visitables, Class<?> type) {
if (getPath().equals(path) &&
type.isAssignableFrom(value.getClass()) &&
elementName.equals(this.elementNameToAdd)) {
if (getPath().equals(path) && elementName.equals(this.elementNameToAdd)) {
if (!type.isAssignableFrom(value.getClass())) {
throw new IllegalStateException("target " + type + " is not assignable from " + value.getClass());
}
getList().add(value);
markListDirty();
}
}

@Override
public boolean visit(String elementName, int index, Visitable value) {
if (!isRepeatingElement) {
Expand All @@ -56,7 +58,7 @@ public boolean visit(String elementName, int index, Visitable value) {
}
return true;
}

@Override
protected void doVisitEnd(String elementName, int elementIndex, Resource resource) {
if (!isRepeatingElement) {
Expand All @@ -65,7 +67,7 @@ protected void doVisitEnd(String elementName, int elementIndex, Resource resourc
}
}
}

@Override
protected void doVisitEnd(String elementName, int elementIndex, Element element) {
if (!isRepeatingElement) {
Expand Down
46 changes: 26 additions & 20 deletions fhir-path/src/main/java/com/ibm/fhir/path/util/FHIRPathUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* (C) Copyright IBM Corp. 2019, 2020
* (C) Copyright IBM Corp. 2019, 2021
*
* SPDX-License-Identifier: Apache-2.0
*/
Expand Down Expand Up @@ -865,14 +865,13 @@ public static <T extends Visitable> T add(T elementOrResource, String fhirPath,
Visitable parent = node.isResourceNode() ?
node.asResourceNode().resource() : node.asElementNode().element();

AddingVisitor<T> addingVisitor = new AddingVisitor<>(parent, node.path(), elementName, value);

try {
AddingVisitor<T> addingVisitor = new AddingVisitor<>(parent, node.path(), elementName, value);
elementOrResource.accept(addingVisitor);
} catch (IllegalStateException e) {
return addingVisitor.getResult();
} catch (IllegalArgumentException | IllegalStateException e) {
throw new FHIRPatchException("An error occurred while adding the value", fhirPath, e);
}
return addingVisitor.getResult();
}

/**
Expand All @@ -883,14 +882,14 @@ public static <T extends Visitable> T add(T elementOrResource, String fhirPath,
*/
public static <T extends Visitable> T delete(T elementOrResource, String fhirPath) throws FHIRPathException, FHIRPatchException {
FHIRPathNode node = evaluateToSingle(elementOrResource, fhirPath);
DeletingVisitor<T> deletingVisitor = new DeletingVisitor<T>(node.path());

try {
DeletingVisitor<T> deletingVisitor = new DeletingVisitor<T>(node.path());
elementOrResource.accept(deletingVisitor);
} catch (IllegalStateException e) {
return deletingVisitor.getResult();
} catch (IllegalArgumentException | IllegalStateException e) {
throw new FHIRPatchException("An error occurred while deleting the value", fhirPath, e);
}
return deletingVisitor.getResult();
}

/**
Expand All @@ -909,16 +908,22 @@ public static <T extends Visitable> T replace(T elementOrResource, String fhirPa
Visitable parent = parentNode.isResourceNode() ?
parentNode.asResourceNode().resource() : parentNode.asElementNode().element();

ReplacingVisitor<T> replacingVisitor = new ReplacingVisitor<T>(parent, elementName, node.path(), value);

try {
ReplacingVisitor<T> replacingVisitor = new ReplacingVisitor<T>(parent, elementName, node.path(), value);
elementOrResource.accept(replacingVisitor);
} catch (IllegalStateException e) {
return replacingVisitor.getResult();
} catch (IllegalArgumentException | IllegalStateException e) {
throw new FHIRPatchException("An error occurred while replacing the value", fhirPath, e);
}
return replacingVisitor.getResult();
}

/**
* @param elementOrResource
* @param fhirPath
* @return
* @throws FHIRPathException
* @throws FHIRPatchException if the fhirPath does not evaluate to a single node
*/
private static FHIRPathNode evaluateToSingle(Visitable elementOrResource, String fhirPath) throws FHIRPathException, FHIRPatchException {
/*
* 1. The FHIRPath statement must return a single element.
Expand All @@ -933,6 +938,9 @@ private static FHIRPathNode evaluateToSingle(Visitable elementOrResource, String
* 5. Except for the delete operation, it is an error if no element matches the specified path.
*/
Collection<FHIRPathNode> nodes = evaluator.evaluate(elementOrResource, fhirPath);
if (!isSingleton(nodes)) {
throw new FHIRPatchException("Expected a singleton but instead found " + nodes.size() + " nodes", fhirPath);
}
return getSingleton(nodes);
}

Expand All @@ -959,14 +967,13 @@ public static <T extends Visitable> T insert(T elementOrResource, String fhirPat
Visitable parent = parentNode.isResourceNode() ?
parentNode.asResourceNode().resource() : parentNode.asElementNode().element();

InsertingVisitor<T> insertingVisitor = new InsertingVisitor<T>(parent, parentNode.path(), elementName, index, value);

try {
InsertingVisitor<T> insertingVisitor = new InsertingVisitor<T>(parent, parentNode.path(), elementName, index, value);
elementOrResource.accept(insertingVisitor);
} catch (IllegalStateException e) {
return insertingVisitor.getResult();
} catch (IllegalArgumentException | IllegalStateException e) {
throw new FHIRPatchException("An error occurred while inserting the value", fhirPath, e);
}
return insertingVisitor.getResult();
}

/**
Expand All @@ -990,14 +997,13 @@ public static <T extends Visitable> T move(T elementOrResource, String fhirPath,
FHIRPathTree tree = evaluator.getEvaluationContext().getTree();
FHIRPathNode parent = getCommonParent(fhirPath, nodes, tree);

MovingVisitor<T> movingVisitor = new MovingVisitor<T>(parent.path(), elementName, source, target);

try {
MovingVisitor<T> movingVisitor = new MovingVisitor<T>(parent.path(), elementName, source, target);
elementOrResource.accept(movingVisitor);
} catch (IllegalStateException e) {
return movingVisitor.getResult();
} catch (IllegalArgumentException | IllegalStateException e) {
throw new FHIRPatchException("An error occurred while moving the value", fhirPath, e);
}
return movingVisitor.getResult();
}

/**
Expand Down
6 changes: 5 additions & 1 deletion fhir-path/src/main/java/com/ibm/fhir/path/util/InsertingVisitor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* (C) Copyright IBM Corp. 2020
* (C) Copyright IBM Corp. 2020, 2021
*
* SPDX-License-Identifier: Apache-2.0
*/
Expand All @@ -23,6 +23,7 @@ class InsertingVisitor<T extends Visitable> extends CopyingVisitor<T> {
* @param elementName
* @param index
* @param value
* @throws IllegalArgumentException
*/
public InsertingVisitor(Visitable parent, String parentPath, String elementName, int index, Visitable value) {
this.parentPath = Objects.requireNonNull(parentPath);
Expand All @@ -35,6 +36,9 @@ public InsertingVisitor(Visitable parent, String parentPath, String elementName,
@Override
protected void doVisitListEnd(String elementName, List<? extends Visitable> visitables, Class<?> type) {
if (getPath().equals(parentPath) && elementName.equals(this.elementNameToInsert)) {
if (!type.isInstance(value)) {
throw new IllegalStateException("target " + type + " is not assignable from " + value.getClass());
}
getList().add(index, value);
markListDirty();
}
Expand Down
Loading