Skip to content

Commit

Permalink
Merge pull request #3178 from IBM/issue-2026-fix1
Browse files Browse the repository at this point in the history 
issue #2026 #3174 #3175 history updates for QA
  • Loading branch information
@punktilious
punktilious authored Jan 12, 2022
2 parents 65ad63c + 8e8186b commit 6915377
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 10 deletions.
4 changes: 3 additions & 1 deletion docs/src/pages/Conformance.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ There are three possible traversal paths through the data which are requested us

Option 1 is useful for users interested in finding the most recent changes that have been applied to the server. Option 2 is useful for system-to-system synchronization because it represents the change history of the server. Option 3 is also useful for system-to-system synchronization cases, but guarantees uniqueness across pages when following the `next` links (versus option 2 which may repeat entries at the time window boundary when multiple resources share the same modification timestamp).

To return all changes that have occurred since a known point in time, use both `_since` and `_sort=_lastUpdated` query parameters:
The `_since` and `_before` search parameters can be used to specify a time range filter. They are both defined as [instant](https://www.hl7.org/fhir/datatypes.html#instant) datatypes which must include a time specified to at least second accuracy and include a timezone. To return all changes that have occurred since a known point in time, use both `_since` and `_sort=_lastUpdated` query parameters:

```
curl -k -u '<username>:<password>' 'https://<host>:<port>/fhir-server/api/v4/_history?_since=2021-02-21T00:00:00Z&_sort=_lastUpdated'
Expand Down Expand Up @@ -165,6 +165,8 @@ As mentioned above, to simplify client implementations in system-to-system synch

In this case, the IBM FHIR Server uses `_changeIdMarker` as a custom paging attribute which references a single `Bundle.entry.id` value. This value can be used by clients to checkpoint where they are in the sequence of changes, and ask for only changes that come after the given id. The simplest way to do this is to follow the `next` link returned in the response Bundle. If the next link is not present in the response Bundle, the end has been reached for the current point in time. Note that `_lastUpdated` and `Bundle.entry.id` values are not perfectly correlated - clients should not mix ordering. The ids used for `Bundle.entry.id` are guaranteed to be unique within a single IBM FHIR Server database (tenant/datasource).

The `_changeIdMarker` query parameter is intended to be used only as part of a `next` link defined within a search result. It is not intended for general use by an end user.

### Whole System History - Type Filters
The query parameter `_type` can be used to limit which resource types are returned from a `_history` request. For example, the following request will return only Patient and Observation resources:

Expand Down
3 changes: 2 additions & 1 deletion ...stence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/impl/FHIRPersistenceJDBCImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2907,7 +2907,8 @@ private String getResourcePayloadKeyFromContext(FHIRPersistenceContext context)
public List<Resource> readResourcesForRecords(List<ResourceChangeLogRecord> records) throws FHIRPersistenceException {
// TODO support async read from payloadPersistence after issue #2900 is merged.

// Make sure we read deleted resources
// Make sure we read deleted resources...this is important because the result list must
// line up row-for-row with the provided records list
FHIRPersistenceContext readContext = FHIRPersistenceContextFactory.createPersistenceContext(null, true);
List<Resource> result = new ArrayList<>(records.size());
for (ResourceChangeLogRecord r: records) {
Expand Down
44 changes: 36 additions & 8 deletions fhir-server/src/main/java/com/ibm/fhir/server/util/FHIRRestHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@
import com.ibm.fhir.persistence.HistorySortOrder;
import com.ibm.fhir.persistence.InteractionStatus;
import com.ibm.fhir.persistence.ResourceChangeLogRecord;
import com.ibm.fhir.persistence.ResourceChangeLogRecord.ChangeType;
import com.ibm.fhir.persistence.ResourceEraseRecord;
import com.ibm.fhir.persistence.SingleResourceResult;
import com.ibm.fhir.persistence.context.FHIRHistoryContext;
Expand Down Expand Up @@ -3026,10 +3027,18 @@ public Bundle doHistory(MultivaluedMap<String, String> queryParameters, String r
historyContext.getHistorySortOrder());
}

if (historyContext.getReturnPreference() == HTTPReturnPreference.REPRESENTATION) {
if (historyContext.getReturnPreference() == HTTPReturnPreference.REPRESENTATION
|| historyContext.getReturnPreference() == HTTPReturnPreference.OPERATION_OUTCOME) {
// vread the resources so that we can include them in the response bundle
log.fine("Fetching resources for history response bundle, count=" + records.size());
if (log.isLoggable(Level.FINE)) {
log.fine("Fetching resources for history response bundle, count=" + records.size());
}
resources = persistence.readResourcesForRecords(records);

if (resources.size() != records.size()) {
// very unlikely...unless we overlap with a patient erase operation
throw new FHIRPersistenceException("Could not read all required resource records");
}
}
} catch (FHIRPersistenceDataAccessException x) {
log.log(Level.SEVERE, "Error reading history; params = {" + historyContext + "}",
Expand All @@ -3050,7 +3059,7 @@ public Bundle doHistory(MultivaluedMap<String, String> queryParameters, String r
// output them in the same order.
for (int i=0; i<records.size(); i++) {
ResourceChangeLogRecord changeRecord = records.get(i);
Resource resource = resources != null ? resources.get(i) : null; // REPRESENTATION
Resource resource = resources != null ? resources.get(i) : null; // REPRESENTATION or OPERATION_OUTCOME
if (historyContext.getHistorySortOrder() == HistorySortOrder.ASC_LAST_UPDATED) {
if (lastChangeTime == null || changeRecord.getChangeTstamp().isAfter(lastChangeTime)) {
// Keep track of the latest timestamp and the corresponding resource (change) id
Expand Down Expand Up @@ -3098,7 +3107,12 @@ public Bundle doHistory(MultivaluedMap<String, String> queryParameters, String r
entryBuilder.fullUrl(Url.of(changeRecord.getResourceTypeName() + "/" + changeRecord.getLogicalId()));
entryBuilder.request(requestBuilder.build());
entryBuilder.response(responseBuilder.build());
entryBuilder.resource(resource); // remember resource may be null if returnPreference=minimal

// history bundle entry should not include any resource for deleted entries,
// also resource may be null if returnPreference=minimal
if (changeRecord.getChangeType() != ChangeType.DELETE && resource != null) {
entryBuilder.resource(resource);
}
bundleBuilder.entry(entryBuilder.build());
}

Expand Down Expand Up @@ -3135,9 +3149,9 @@ public Bundle doHistory(MultivaluedMap<String, String> queryParameters, String r
case ASC_LAST_UPDATED:
nextRequest.append("&_sort=" + HistorySortOrder.ASC_LAST_UPDATED.toString());
nextRequest.append("&_since=").append(lastChangeTime.atZone(UTC).format(DateTime.PARSER_FORMATTER));
if (before != null) {
if (historyContext.getBefore() != null && historyContext.getBefore().getValue() != null) {
// make sure we keep including the before filter specified in the original request
nextRequest.append("&_before=").append(before.atZone(UTC).format(DateTime.PARSER_FORMATTER));
nextRequest.append("&_before=").append(historyContext.getBefore().getValue().format(DateTime.PARSER_FORMATTER));
}

if (changeIdMarker != null) {
Expand All @@ -3148,9 +3162,9 @@ public Bundle doHistory(MultivaluedMap<String, String> queryParameters, String r
case DESC_LAST_UPDATED:
nextRequest.append("&_sort=" + HistorySortOrder.DESC_LAST_UPDATED.toString());
nextRequest.append("&_before=").append(lastChangeTime.atZone(UTC).format(DateTime.PARSER_FORMATTER));
if (since != null) {
if (historyContext.getSince() != null && historyContext.getSince().getValue() != null) {
// make sure we keep including the since filter specified in the original request
nextRequest.append("&_since=").append(since.atZone(UTC).format(DateTime.PARSER_FORMATTER));
nextRequest.append("&_since=").append(historyContext.getSince().getValue().format(DateTime.PARSER_FORMATTER));
}
if (changeIdMarker != null) {
// good way to exclude this resource on the next call
Expand All @@ -3160,6 +3174,16 @@ public Bundle doHistory(MultivaluedMap<String, String> queryParameters, String r
case NONE:
nextRequest.append("&_sort=" + HistorySortOrder.NONE.toString());
nextRequest.append("&_changeIdMarker=").append(changeIdMarker);
// since/before range filters should not be common when using system-defined
// sort order (HistorySortOrder.NONE) but are supported. Note the before
// value will interact with _excludeTransactionTimeoutWindow
if (historyContext.getSince() != null && historyContext.getSince().getValue() != null) {
nextRequest.append("&_since=").append(historyContext.getSince().getValue().format(DateTime.PARSER_FORMATTER));
}

if (historyContext.getBefore() != null && historyContext.getBefore().getValue() != null) {
nextRequest.append("&_before=").append(historyContext.getBefore().getValue().format(DateTime.PARSER_FORMATTER));
}
break;
}

Expand Down Expand Up @@ -3187,6 +3211,10 @@ public Bundle doHistory(MultivaluedMap<String, String> queryParameters, String r
selfRequest.append("&_since=").append(historyContext.getSince().getValue().format(DateTime.PARSER_FORMATTER));
}

if (historyContext.getBefore() != null && historyContext.getBefore().getValue() != null) {
selfRequest.append("&_before=").append(historyContext.getBefore().getValue().format(DateTime.PARSER_FORMATTER));
}

if (resourceType == null && historyContext.getResourceTypes().size() > 0) {
selfRequest.append("&_type=");
selfRequest.append(String.join(",", historyContext.getResourceTypes()));
Expand Down

0 comments on commit 6915377

Please sign in to comment.