Skip to content

Commit

Permalink
issue #2263 - support skippable updates
Browse files Browse the repository at this point in the history 
1. add a skippableUpdate flag to the FHIRResourceHelpers interface
2. set this flag from a new header X-FHIR-UPDATE-IF-MODIFIED
3. if skippableUpdate is true, use the resource fingerprinter to compare
the resource from the update request to the resource in the database and
skip the update when they match

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
  • Loading branch information
@lmsurpre
lmsurpre committed Apr 22, 2021
1 parent 51f1648 commit 7d77e2d
Show file tree
Hide file tree
Showing 8 changed files with 109 additions and 65 deletions.
2 changes: 2 additions & 0 deletions fhir-core/src/main/java/com/ibm/fhir/core/FHIRConstants.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ public class FHIRConstants {

public static final String ELEMENTS = "_elements";

public static final String UPDATE_IF_MODIFIED_HEADER = "X-FHIR-UPDATE-IF-MODIFIED";

/**
* General parameter names that can be used with any FHIR interaction.
*
Expand Down
2 changes: 1 addition & 1 deletion fhir-model/src/main/java/com/ibm/fhir/model/visitor/PathAwareVisitor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public final String getPath() {
*
* Invoke this method when visiting has failed and you want to clear the path in order to re-use the visitor.
*/
public final void reset() {
protected void reset() {
if (!pathStack.isEmpty()) {
pathStack.clear();
}
Expand Down
60 changes: 33 additions & 27 deletions fhir-persistence/src/main/java/com/ibm/fhir/persistence/util/ResourceFingerprintVisitor.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* (C) Copyright IBM Corp. 2019
* (C) Copyright IBM Corp. 2019, 2021
*
* SPDX-License-Identifier: Apache-2.0
*/
Expand All @@ -24,37 +24,35 @@
/**
* Compute a cryptographic hash of the visited nodes, skipping those which
* may be altered by the persistence layer.
*/
public class ResourceFingerprintVisitor extends PathAwareVisitor {

// 32 bytes chosen as a matching entropy of SHA-256
private static final int BYTES_FOR_256_BITS = 256 / 8;
private static final SecureRandom RANDOM = new SecureRandom();

// the salt we use for computing the hash
private final byte[] salt;

// The name of the resource we first encounter
private String currentResourceName;
private final MessageDigest digest;

private MessageDigest digest;

// for tracking array elements
int index;

/**
* Public constructor. Uses the given salt
* @param salt
*/
public ResourceFingerprintVisitor(byte[] salt) {
this.salt = salt;

try {
digest = MessageDigest.getInstance("SHA-256");
digest.update(salt);
}
catch (NoSuchAlgorithmException x) {
} catch (NoSuchAlgorithmException x) {
throw new IllegalStateException(x);
}
}
Expand All @@ -67,48 +65,56 @@ public ResourceFingerprintVisitor(byte[] salt) {
public ResourceFingerprintVisitor(SaltHash baseline) {
this(baseline.getSalt());
}

/**
* Public constructor. Generates a new salt
*/
public ResourceFingerprintVisitor() {
this.salt = new byte[BYTES_FOR_256_BITS];
RANDOM.nextBytes(salt);

try {
digest = MessageDigest.getInstance("SHA-256");
digest.update(salt);
}
catch (NoSuchAlgorithmException x) {
} catch (NoSuchAlgorithmException x) {
throw new IllegalStateException(x);
}
}


/**
* Compute the digest and return the result along with the salt that
* was used
* Compute the digest and return the result along with the salt that was used
* @return
*/
public SaltHash getSaltAndHash() {
return new SaltHash(salt, digest.digest());
}


@Override
public final void reset() {
super.reset();
try {
digest = MessageDigest.getInstance("SHA-256");
digest.update(salt);
} catch (NoSuchAlgorithmException x) {
throw new IllegalStateException(x);
}
}

@Override
protected void doVisitStart(String elementName, int elementIndex, Resource resource) {
if (this.currentResourceName == null) {
this.currentResourceName = resource.getClass().getSimpleName();
}
}

@Override
public void visit(java.lang.String elementName, byte[] value) {
if (includePath()) {
digest.update(getPath().getBytes(StandardCharsets.UTF_8));
digest.update(value);
}
}

@Override
public void visit(java.lang.String elementName, BigDecimal value) {
if (includePath()) {
Expand All @@ -122,7 +128,7 @@ public void visit(java.lang.String elementName, java.lang.Boolean value) {
updateDigest(getPath(), value.toString());
}
}

@Override
public void visit(java.lang.String elementName, java.lang.Integer value) {
if (includePath()) {
Expand All @@ -146,7 +152,7 @@ public void visit(java.lang.String elementName, LocalTime value) {
}
@Override
public void doVisit(java.lang.String elementName, java.lang.String value) {
// exclude the id and meta.versionId values from the fingerprint
// exclude the id and meta.versionId values from the fingerprint
// because they are injected by FHIR. NOTE: startsWith is important
// because we need to ignore any extension fields which may be
// present
Expand All @@ -173,9 +179,9 @@ public void visit(java.lang.String elementName, ZonedDateTime value) {
if (includePath()) {
updateDigest(getPath(), value.toString());
}

}

/**
* Update the digest with the name/value pair
* @param name
Expand All @@ -185,7 +191,7 @@ protected void updateDigest(String name, String value) {
digest.update(name.getBytes(StandardCharsets.UTF_8));
digest.update(value.getBytes(StandardCharsets.UTF_8));
}

/**
* Test whether or not the current path value should be included in the fingerprint
* @return
Expand All @@ -196,6 +202,6 @@ protected boolean includePath() {
String lastUpdatedName = currentResourceName + ".meta.lastUpdated";
String path = getPath();
return !path.startsWith(idName) && !path.startsWith(versionIdName) && !path.startsWith(lastUpdatedName);

}
}
42 changes: 30 additions & 12 deletions fhir-server/src/main/java/com/ibm/fhir/server/operation/spi/FHIRResourceHelpers.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,10 @@
* implementations.
*/
public interface FHIRResourceHelpers {
// Useful constant for indicating the need to validate a resource
// Constant for indicating the need to validate a resource
public static final boolean DO_VALIDATION = true;
// Constant for indicating whether an update can be skipped when the requested update resource matches the existing one
public static final boolean SKIPPABLE_UPDATE = true;

/**
* Performs the heavy lifting associated with a 'create' interaction. Validates the resource.
Expand Down Expand Up @@ -74,11 +76,15 @@ default FHIRRestOperationResponse doCreate(String type, Resource resource, Strin
* an optional "If-Match" header value to request a version-aware update
* @param searchQueryString
* an optional search query string to request a conditional update
* @param skippableUpdate
* if true, and the resource content in the update matches the existing resource on the server, then skip the update;
* if false, then always attempt the update
* @return a FHIRRestOperationResponse that contains the results of the operation
* @throws Exception
*/
default FHIRRestOperationResponse doUpdate(String type, String id, Resource newResource, String ifMatchValue, String searchQueryString, Map<String, String> requestProperties) throws Exception {
return doUpdate(type, id, newResource, ifMatchValue, searchQueryString, requestProperties, DO_VALIDATION);
default FHIRRestOperationResponse doUpdate(String type, String id, Resource newResource, String ifMatchValue,
String searchQueryString, Map<String, String> requestProperties, boolean skippableUpdate) throws Exception {
return doUpdate(type, id, newResource, ifMatchValue, searchQueryString, requestProperties, skippableUpdate, DO_VALIDATION);
}

/**
Expand All @@ -94,12 +100,16 @@ default FHIRRestOperationResponse doUpdate(String type, String id, Resource newR
* an optional "If-Match" header value to request a version-aware update
* @param searchQueryString
* an optional search query string to request a conditional update
* @param skippableUpdate
* if true, and the resource content in the update matches the existing resource on the server, then skip the update;
* if false, then always attempt the update
* @param doValidation
* if true, validate the resource; if false, assume the resource has already been validated
* @return a FHIRRestOperationResponse that contains the results of the operation
* @throws Exception
*/
public FHIRRestOperationResponse doUpdate(String type, String id, Resource newResource, String ifMatchValue, String searchQueryString, Map<String, String> requestProperties, boolean doValidation) throws Exception;
public FHIRRestOperationResponse doUpdate(String type, String id, Resource newResource, String ifMatchValue,
String searchQueryString, Map<String, String> requestPropertie, boolean skippableUpdate, boolean doValidation) throws Exception;

/**
* Performs a patch operation (a new version of the Resource will be stored).
Expand All @@ -114,10 +124,14 @@ default FHIRRestOperationResponse doUpdate(String type, String id, Resource newR
* an optional "If-Match" header value to request a version-aware update
* @param searchQueryString
* an optional search query string to request a conditional update
* @param skippableUpdate
* if true, and the result of the patch matches the existing resource on the server, then skip the update;
* if false, then always attempt the update
* @return a FHIRRestOperationResponse that contains the results of the operation
* @throws Exception
*/
public FHIRRestOperationResponse doPatch(String type, String id, FHIRPatch patch, String ifMatchValue, String searchQueryString, Map<String, String> requestProperties) throws Exception;
public FHIRRestOperationResponse doPatch(String type, String id, FHIRPatch patch, String ifMatchValue,
String searchQueryString, Map<String, String> requestProperties, boolean skippableUpdate) throws Exception;


/**
Expand All @@ -144,7 +158,8 @@ default FHIRRestOperationResponse doUpdate(String type, String id, Resource newR
* @return the Resource
* @throws Exception
*/
public Resource doRead(String type, String id, boolean throwExcOnNull, boolean includeDeleted, Map<String, String> requestProperties, Resource contextResource, MultivaluedMap<String, String> queryParameters) throws Exception;
public Resource doRead(String type, String id, boolean throwExcOnNull, boolean includeDeleted, Map<String, String> requestProperties,
Resource contextResource, MultivaluedMap<String, String> queryParameters) throws Exception;

/**
* Performs a 'read' operation to retrieve a Resource.
Expand All @@ -156,7 +171,8 @@ default FHIRRestOperationResponse doUpdate(String type, String id, Resource newR
* @return the Resource
* @throws Exception
*/
public Resource doRead(String type, String id, boolean throwExcOnNull, boolean includeDeleted, Map<String, String> requestProperties, Resource contextResource) throws Exception;
public Resource doRead(String type, String id, boolean throwExcOnNull, boolean includeDeleted, Map<String, String> requestProperties,
Resource contextResource) throws Exception;

/**
* Performs a 'vread' operation by retrieving the specified version of a Resource.
Expand All @@ -174,7 +190,8 @@ default FHIRRestOperationResponse doUpdate(String type, String id, Resource newR
* @return the Resource
* @throws Exception
*/
public Resource doVRead(String type, String id, String versionId, Map<String, String> requestProperties, MultivaluedMap<String, String> queryParameters) throws Exception;
public Resource doVRead(String type, String id, String versionId, Map<String, String> requestProperties,
MultivaluedMap<String, String> queryParameters) throws Exception;

/**
* Performs a 'vread' operation by retrieving the specified version of a Resource.
Expand Down Expand Up @@ -206,7 +223,8 @@ default FHIRRestOperationResponse doUpdate(String type, String id, Resource newR
* @return a Bundle containing the history of the specified Resource
* @throws Exception
*/
public Bundle doHistory(String type, String id, MultivaluedMap<String, String> queryParameters, String requestUri, Map<String, String> requestProperties) throws Exception;
public Bundle doHistory(String type, String id, MultivaluedMap<String, String> queryParameters, String requestUri,
Map<String, String> requestProperties) throws Exception;

/**
* Implement the system level history operation to obtain a list of changes to resources
Expand All @@ -230,8 +248,8 @@ default FHIRRestOperationResponse doUpdate(String type, String id, Resource newR
* @return a Bundle containing the search result set
* @throws Exception
*/
public Bundle doSearch(String type, String compartment, String compartmentId, MultivaluedMap<String, String> queryParameters, String requestUri, Map<String, String> requestProperties, Resource contextResource)
throws Exception;
public Bundle doSearch(String type, String compartment, String compartmentId, MultivaluedMap<String, String> queryParameters,
String requestUri, Map<String, String> requestProperties, Resource contextResource) throws Exception;


/**
Expand All @@ -258,7 +276,7 @@ public Bundle doSearch(String type, String compartment, String compartmentId, Mu
* @throws Exception
*/
public Resource doInvoke(FHIROperationContext operationContext, String resourceTypeName, String logicalId, String versionId, String operationName,
Resource resource, MultivaluedMap<String, String> queryParameters, Map<String, String> requestProperties) throws Exception;
Resource resource, MultivaluedMap<String, String> queryParameters, Map<String, String> requestProperties) throws Exception;

/**
* Processes a bundled request (batch or transaction type).
Expand Down
19 changes: 11 additions & 8 deletions fhir-server/src/main/java/com/ibm/fhir/server/resources/Patch.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import org.eclipse.microprofile.jwt.JsonWebToken;

import com.ibm.fhir.config.FHIRRequestContext;
import com.ibm.fhir.core.FHIRConstants;
import com.ibm.fhir.core.FHIRMediaType;
import com.ibm.fhir.core.HTTPReturnPreference;
import com.ibm.fhir.exception.FHIROperationException;
Expand Down Expand Up @@ -73,7 +74,7 @@ public Patch() throws Exception {
@Produces({ FHIRMediaType.APPLICATION_FHIR_JSON, MediaType.APPLICATION_JSON })
@Path("{type}/{id}")
public Response patch(@PathParam("type") String type, @PathParam("id") String id, JsonArray array,
@HeaderParam(HttpHeaders.IF_MATCH) String ifMatch) {
@HeaderParam(HttpHeaders.IF_MATCH) String ifMatch, @HeaderParam(FHIRConstants.UPDATE_IF_MODIFIED_HEADER) Boolean onlyIfModified) {
log.entering(this.getClass().getName(), "patch(String,String,JsonArray)");
Date startTime = new Date();
Response.Status status = null;
Expand All @@ -85,7 +86,7 @@ public Response patch(@PathParam("type") String type, @PathParam("id") String id
FHIRPatch patch = createPatch(array);

FHIRRestHelper helper = new FHIRRestHelper(getPersistenceImpl());
ior = helper.doPatch(type, id, patch, ifMatch, null, null);
ior = helper.doPatch(type, id, patch, ifMatch, null, null, onlyIfModified != null ? onlyIfModified : false);

status = ior.getStatus();
ResponseBuilder response = Response.status(status)
Expand Down Expand Up @@ -125,7 +126,7 @@ public Response patch(@PathParam("type") String type, @PathParam("id") String id
@PATCH
@Path("{type}/{id}")
public Response patch(@PathParam("type") String type, @PathParam("id") String id, Parameters parameters,
@HeaderParam(HttpHeaders.IF_MATCH) String ifMatch) {
@HeaderParam(HttpHeaders.IF_MATCH) String ifMatch, @HeaderParam(FHIRConstants.UPDATE_IF_MODIFIED_HEADER) Boolean onlyIfModified) {
log.entering(this.getClass().getName(), "patch(String,String,Parameters)");
Date startTime = new Date();
Response.Status status = null;
Expand All @@ -142,7 +143,7 @@ public Response patch(@PathParam("type") String type, @PathParam("id") String id
}

FHIRRestHelper helper = new FHIRRestHelper(getPersistenceImpl());
ior = helper.doPatch(type, id, patch, ifMatch, null, null);
ior = helper.doPatch(type, id, patch, ifMatch, null, null, onlyIfModified != null ? onlyIfModified : false);

ResponseBuilder response =
Response.ok().location(toUri(getAbsoluteUri(getRequestBaseUri(type), ior.getLocationURI().toString())));
Expand Down Expand Up @@ -185,7 +186,8 @@ public Response patch(@PathParam("type") String type, @PathParam("id") String id
@Consumes({ FHIRMediaType.APPLICATION_JSON_PATCH })
@Produces({ FHIRMediaType.APPLICATION_FHIR_JSON, MediaType.APPLICATION_JSON })
@Path("{type}")
public Response conditionalPatch(@PathParam("type") String type, JsonArray array, @HeaderParam(HttpHeaders.IF_MATCH) String ifMatch) {
public Response conditionalPatch(@PathParam("type") String type, JsonArray array, @HeaderParam(HttpHeaders.IF_MATCH) String ifMatch,
@HeaderParam(FHIRConstants.UPDATE_IF_MODIFIED_HEADER) Boolean onlyIfModified) {
log.entering(this.getClass().getName(), "conditionalPatch(String,String,JsonArray)");
Date startTime = new Date();
Response.Status status = null;
Expand All @@ -204,7 +206,7 @@ public Response conditionalPatch(@PathParam("type") String type, JsonArray array
}

FHIRRestHelper helper = new FHIRRestHelper(getPersistenceImpl());
ior = helper.doPatch(type, null, patch, ifMatch, searchQueryString, null);
ior = helper.doPatch(type, null, patch, ifMatch, searchQueryString, null, onlyIfModified != null ? onlyIfModified : false);

ResponseBuilder response =
Response.ok().location(toUri(getAbsoluteUri(getRequestBaseUri(type), ior.getLocationURI().toString())));
Expand Down Expand Up @@ -247,7 +249,8 @@ public Response conditionalPatch(@PathParam("type") String type, JsonArray array

@PATCH
@Path("{type}")
public Response conditionalPatch(@PathParam("type") String type, Parameters parameters, @HeaderParam(HttpHeaders.IF_MATCH) String ifMatch) {
public Response conditionalPatch(@PathParam("type") String type, Parameters parameters, @HeaderParam(HttpHeaders.IF_MATCH) String ifMatch,
@HeaderParam(FHIRConstants.UPDATE_IF_MODIFIED_HEADER) Boolean onlyIfModified) {
log.entering(this.getClass().getName(), "conditionalPatch(String,String,Parameters)");
Date startTime = new Date();
Response.Status status = null;
Expand All @@ -271,7 +274,7 @@ public Response conditionalPatch(@PathParam("type") String type, Parameters para
}

FHIRRestHelper helper = new FHIRRestHelper(getPersistenceImpl());
ior = helper.doPatch(type, null, patch, ifMatch, searchQueryString, null);
ior = helper.doPatch(type, null, patch, ifMatch, searchQueryString, null, onlyIfModified != null ? onlyIfModified : false);

status = ior.getStatus();
ResponseBuilder response = Response.status(status)
Expand Down
Loading

0 comments on commit 7d77e2d

Please sign in to comment.