Skip to content

Commit

Permalink
issue #3319 - normalize handling of _type parameters
Browse files Browse the repository at this point in the history 
Previously, I updated whole-system search to support combining multiple
instances of the `_type` parameter.  Now we will handle multiple '_type'
parameters in the extended operations $everything and $export as well.

I updated the search section in Conformance.md to indicate this change
and added a note in the extended operations section to indicate that
query parameters will be ignored for operations invoked via POST.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
  • Loading branch information
@lmsurpre
lmsurpre committed Mar 18, 2022
1 parent ce78738 commit 15a231c
Show file tree
Hide file tree
Showing 12 changed files with 385 additions and 396 deletions.
11 changes: 5 additions & 6 deletions docs/src/pages/Conformance.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ To return all changes that have occurred before a known point in time, use `_bef
curl -k -u '<username>:<password>' 'https://<host>:<port>/fhir-server/api/v4/_history?_before=2021-02-21T00:00:00Z&_sort=-_lastUpdated'
```

To return changes within a known window of time, specify both the `_since` and `_before` query parameters and choose either ascending or descending sort.
To return changes within a known window of time, specify both the `_since` and `_before` query parameters and choose either ascending or descending sort.

When sorting with ascending time, the `_since` parameter in the `next` link is calculated to fetch the next page of resources and the `_before` parameter (if given) remains constant. When sorting with descending time, the `_before` parameter in the `next` link is calculated to fetch the next page of resources and the `_since` parameter (if given) remains constant.

Expand Down Expand Up @@ -201,7 +201,7 @@ The IBM FHIR Server requires clocks in a cluster to be synchronized and expects
### Whole System History - The Transaction Timeout Window
Clients must exercise caution when reading recently ingested resources. When processing large bundles in parallel, an id may be assigned by the database but ACID isolation means that the record will not be visible to a reader until the transaction is committed. This could be up to 120s or longer if a larger transaction-timeout property has been defined. If a smaller bundle starts after the larger bundle and its transaction is committed first, its change ids and timestamps will be visible to readers before the resources from the other bundle, which will have some earlier change ids and timestamps. If clients do not take this into account, they may miss some resources. This behavior is a common concern in databases and not specific to the IBM FHIR Server.

To guarantee no data is skipped, clients should not process resources with a `_lastUpdated` timestamp which is after `{current_time} - {transaction_timeout} - {max_cluster_clock_drift}`. By waiting for this time window to close, the client can be sure the data being returned is complete and in order, and can safely checkpoint using the `_since`, `_before` or `_changeIdMarker` values depending on the chosen sort option. The default value for transaction timeout is 120s but this is configurable. A value of 2 seconds is a reasonable default to consider for `max_cluster_clock_drift` in lieu of specific information about the infrastructure. Implementers should check with server administrators on the appropriate values to use.
To guarantee no data is skipped, clients should not process resources with a `_lastUpdated` timestamp which is after `{current_time} - {transaction_timeout} - {max_cluster_clock_drift}`. By waiting for this time window to close, the client can be sure the data being returned is complete and in order, and can safely checkpoint using the `_since`, `_before` or `_changeIdMarker` values depending on the chosen sort option. The default value for transaction timeout is 120s but this is configurable. A value of 2 seconds is a reasonable default to consider for `max_cluster_clock_drift` in lieu of specific information about the infrastructure. Implementers should check with server administrators on the appropriate values to use.

To simplify the handling of this scenario, clients may specify the optional query parameter `_excludeTransactionTimeoutWindow=true` to perform this filtering within the server. This relies on the IBM FHIR Server transaction timeout having been configured using the `FHIR_TRANSACTION_MANAGER_TIMEOUT` environment variable. If this environment variable is not configured, a default transaction timeout of 120s is assumed, although this may not be the actual timeout if the server is otherwise configured in a non-standard way.

Expand Down Expand Up @@ -238,9 +238,7 @@ In addition, the following search parameters are supported on all resources:

These parameters can be used while searching any single resource type or while searching across resource types (whole system search).

The `_type` parameter has two restrictions:
* It may only be used with whole system search.
* It may only be specified once in a search. In `lenient` mode, only the first occurrence is used; additional occurrences are ignored.
The `_type` parameter may only be used with whole system search.

The `_has` parameter has two restrictions:
* It cannot be used with whole system search.
Expand Down Expand Up @@ -470,11 +468,12 @@ The IBM FHIR Server implements a handful of extended operations and provides ext

Operations are invoked via HTTP POST.
* All operations can be invoked by passing a Parameters resource instance in the body of the request.
* For operations with no input parameters, no body is required.
* For operations with a single input parameter named "resource", the Parameters wrapper can be omitted.

Alternatively, for operations with only primitive input parameters (i.e. no complex datatypes like 'Identifier' or 'Reference'), operations can be invoked via HTTP GET by passing the parameters in the URL.

Note: operations invoked via POST will ignore all query parameters and operations invoked via GET will ignore any body.

### System operations
System operations are invoked at `[base]/$[operation]`

Expand Down
5 changes: 5 additions & 0 deletions fhir-server-spi/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@
<version>${project.version}</version>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>org.testng</groupId>
<artifactId>testng</artifactId>
<scope>test</scope>
</dependency>
</dependencies>

</project>
180 changes: 101 additions & 79 deletions fhir-server-spi/src/main/java/com/ibm/fhir/server/spi/operation/FHIROperationUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import com.ibm.fhir.model.resource.Resource;
import com.ibm.fhir.model.type.Canonical;
import com.ibm.fhir.model.type.Code;
import com.ibm.fhir.model.type.CodeableConcept;
import com.ibm.fhir.model.type.Date;
import com.ibm.fhir.model.type.DateTime;
import com.ibm.fhir.model.type.Element;
Expand Down Expand Up @@ -72,103 +73,124 @@ public static void init() {
}
}

/**
* Construct a Parameters object from the input parameters passed via query parameters
*
* @param definition not null
* @param queryParameters not null
* @return
* @throws FHIROperationException
*/
public static Parameters getInputParameters(OperationDefinition definition,
Map<String, List<String>> queryParameters) throws FHIROperationException {
Map<String, List<String>> queryParameters) throws FHIROperationException {
try {
Parameters.Builder parametersBuilder = Parameters.builder();
parametersBuilder.id("InputParameters");
if (definition != null) {
for (OperationDefinition.Parameter parameter : definition.getParameter()) {
if (OperationParameterUse.IN.getValue().equals(parameter.getUse().getValue())) {
String name = parameter.getName().getValue();
FHIRAllTypes type = parameter.getType();
if (type == null) {
continue;
}
String typeName = type.getValue();
List<String> values = queryParameters.get(name);
if (values != null) {
String value = values.get(0);
Parameter.Builder parameterBuilder = Parameter.builder().name(string(name));
if ("boolean".equals(typeName)) {
parameterBuilder.value(com.ibm.fhir.model.type.Boolean.of(value));
} else if ("integer".equals(typeName)) {
parameterBuilder.value(com.ibm.fhir.model.type.Integer.of(value));
} else if ("string".equals(typeName)) {
parameterBuilder.value(com.ibm.fhir.model.type.String.of(value));
} else if ("decimal".equals(typeName)) {
parameterBuilder.value(com.ibm.fhir.model.type.Decimal.of(value));
} else if ("uri".equals(typeName)) {
parameterBuilder.value(Uri.of(value));
} else if ("url".equals(typeName)) {
parameterBuilder.value(Url.of(value));
} else if ("canonical".equals(typeName)) {
parameterBuilder.value(Canonical.of(value));
} else if ("instant".equals(typeName)) {
parameterBuilder.value(Instant.of(value));
} else if ("date".equals(typeName)) {
parameterBuilder.value(Date.of(value));
} else if ("dateTime".equals(typeName)) {
parameterBuilder.value(DateTime.of(value));
} else if ("time".equals(typeName)) {
parameterBuilder.value(Time.of(value));
} else if ("code".equals(typeName)) {
parameterBuilder.value(Code.of(value));
} else if ("oid".equals(typeName)) {
parameterBuilder.value(Oid.of(value));
} else if ("id".equals(typeName)) {
parameterBuilder.value(Id.of(value));
} else if ("unsignedInt".equals(typeName)) {
parameterBuilder.value(UnsignedInt.of(value));
} else if ("positiveInt".equals(typeName)) {
parameterBuilder.value(PositiveInt.of(value));
} else if ("uuid".equals(typeName)) {
parameterBuilder.value(Uuid.of(value));
} else {
// Originally returned 500 when it should be 400 (it's on the client).
FHIROperationException operationException =
new FHIROperationException("Query parameter '" + name + "' is an invalid type: '" + typeName + "'");

List<Issue> issues = new ArrayList<>();
Issue.Builder builder = Issue.builder();
builder.code(IssueType.INVALID);
builder.diagnostics(string("Query parameter '" + name + "' is an invalid type: '" + typeName + "'"));
builder.severity(IssueSeverity.ERROR);
issues.add(builder.build());

operationException.setIssues(issues);

throw operationException;
}
parametersBuilder.parameter(parameterBuilder.build());
}

for (OperationDefinition.Parameter parameter : definition.getParameter()) {
if (OperationParameterUse.IN.getValue().equals(parameter.getUse().getValue())) {
String paramName = parameter.getName().getValue();
if (!queryParameters.containsKey(paramName)) {
continue;
}

FHIRAllTypes type = parameter.getType();
if (type == null) {
continue;
}
String typeName = type.getValue();

for (String value : queryParameters.get(paramName)) {
parametersBuilder.parameter(parseParameter(typeName, paramName, value));
}
}
}
return parametersBuilder.build();
} catch (FHIROperationException e) {
throw e;
} catch (Exception e) {
// Originally returned 500 when it should be 400 (it's on the client).
FHIROperationException operationException =
new FHIROperationException("Unable to process query parameters", e);
throw new FHIROperationException("Unable to process query parameters", e);
}
}

private static Parameter parseParameter(String typeName, String paramName, String value) throws FHIROperationException {

List<Issue> issues = new ArrayList<>();
Issue.Builder builder = Issue.builder();
builder.code(IssueType.INVALID);
builder.diagnostics(string("Unable to process query parameters"));
builder.severity(IssueSeverity.ERROR);
issues.add(builder.build());
Parameter.Builder parameterBuilder = Parameter.builder().name(paramName);
try {
if ("boolean".equals(typeName)) {
parameterBuilder.value(com.ibm.fhir.model.type.Boolean.of(value));
} else if ("integer".equals(typeName)) {
parameterBuilder.value(com.ibm.fhir.model.type.Integer.of(value));
} else if ("string".equals(typeName)) {
parameterBuilder.value(com.ibm.fhir.model.type.String.of(value));
} else if ("decimal".equals(typeName)) {
parameterBuilder.value(com.ibm.fhir.model.type.Decimal.of(value));
} else if ("uri".equals(typeName)) {
parameterBuilder.value(Uri.of(value));
} else if ("url".equals(typeName)) {
parameterBuilder.value(Url.of(value));
} else if ("canonical".equals(typeName)) {
parameterBuilder.value(Canonical.of(value));
} else if ("instant".equals(typeName)) {
parameterBuilder.value(Instant.of(value));
} else if ("date".equals(typeName)) {
parameterBuilder.value(Date.of(value));
} else if ("dateTime".equals(typeName)) {
parameterBuilder.value(DateTime.of(value));
} else if ("time".equals(typeName)) {
parameterBuilder.value(Time.of(value));
} else if ("code".equals(typeName)) {
parameterBuilder.value(Code.of(value));
} else if ("oid".equals(typeName)) {
parameterBuilder.value(Oid.of(value));
} else if ("id".equals(typeName)) {
parameterBuilder.value(Id.of(value));
} else if ("unsignedInt".equals(typeName)) {
parameterBuilder.value(UnsignedInt.of(value));
} else if ("positiveInt".equals(typeName)) {
parameterBuilder.value(PositiveInt.of(value));
} else if ("uuid".equals(typeName)) {
parameterBuilder.value(Uuid.of(value));
} else {
String msg = "Query parameter '" + paramName + "' is an invalid type: '" + typeName + "'";
FHIROperationException operationException =
new FHIROperationException(msg);

operationException.setIssues(issues);
operationException.withIssue(Issue.builder()
.code(IssueType.INVALID)
.details(CodeableConcept.builder()
.text(msg)
.build())
.severity(IssueSeverity.ERROR)
.build());

throw operationException;
throw operationException;
}
} catch (NullPointerException | IllegalStateException e) {
String msg = "Unable to parse query parameter '" + paramName + "' to its expected type: '" + typeName + "'";
throw new FHIROperationException(msg, e)
.withIssue(Issue.builder()
.severity(IssueSeverity.ERROR)
.code(IssueType.INVALID)
.details(CodeableConcept.builder()
.text("Unable to process query parameters")
.build())
.build());
}

return parameterBuilder.build();
}


public static Parameters getInputParameters(OperationDefinition definition, Resource resource)
throws Exception {
/**
* Construct a Parameters object with a single parameter named resource
*
* @param definition
* @param resource
* @return
* @throws Exception
*/
public static Parameters getInputParameters(OperationDefinition definition, Resource resource) throws Exception {
Parameters.Builder parametersBuilder = Parameters.builder();
parametersBuilder.id("InputParameters");
for (OperationDefinition.Parameter parameterDefinition : definition.getParameter()) {
Expand Down
Loading

0 comments on commit 15a231c

Please sign in to comment.