Issue #2504 - Check affectsState

fhir-server-test/src/test/java/com/ibm/fhir/server/test/PlanDefinitionApplyOperationTest.java

@@ -173,6 +173,41 @@ public void testSubjectPatient() {
         assertNotNull(carePlan);
     }
 
+
+    @Test(groups = { TEST_GROUP_NAME }, dependsOnMethods = "loadTestData")
+    public void testSubjectPatient_GET() {
+        List<String> subjects = Arrays.asList("Patient/" + patientId);
+
+        // Valid - Instance Level.
+        // ApplyOperationResult result =
+        // runIndividualPlanDefinition(FHIRMediaType.APPLICATION_FHIR_JSON, false, false, planDefinitionId, subjects,
+        // null, practitionerId, null, null, null, null, null, null);
+
+        Response response =
+                doGet(FHIRMediaType.APPLICATION_FHIR_JSON, false, false, planDefinitionId, subjects, null, "Practitioner/" + practitionerId, "Organization/my-org", null, null, null, null, null);
+        assertEquals(response.getStatus(), 200);
+
+        CarePlan carePlan = response.readEntity(CarePlan.class);
+        if (DEBUG_APPLY) {
+            System.out.println(carePlan);
+        }
+        assertNotNull(carePlan);
+    }
+
+    @Test(groups = { TEST_GROUP_NAME }, dependsOnMethods = "loadTestData")
+    public void testSubjectPatient_GET_nonPrimitiveParameters() {
+        List<String> subjects = Arrays.asList("Patient/" + patientId);
+
+        // Valid - Instance Level.
+        // ApplyOperationResult result =
+        // runIndividualPlanDefinition(FHIRMediaType.APPLICATION_FHIR_JSON, false, false, planDefinitionId, subjects,
+        // null, practitionerId, null, null, null, null, null, null);
+
+        Response response =
+                doGet(FHIRMediaType.APPLICATION_FHIR_JSON, false, false, planDefinitionId, subjects, null, "Practitioner/" + practitionerId, "Organization/my-org", "user-type", "user-language", "user-task-context", "my-setting", "my-setting-context");
+        assertEquals(response.getStatus(), 400);
+    }
+
     public Response doPost(String mimeType, boolean root, boolean invalid, String planDefinition, List<String> subject, String encounter, String practitioner,
         String organization, String userType, String userLanguage, String userTaskContext, String setting, String settingContext) {
 
@@ -219,6 +254,50 @@ public Response doPost(String mimeType, boolean root, boolean invalid, String pl
         return target.request(mimeType).post(entity, Response.class);
     }
 
+    public Response doGet(String mimeType, boolean root, boolean invalid, String planDefinition, List<String> subject, String encounter, String practitioner,
+        String organization, String userType, String userLanguage, String userTaskContext, String setting, String settingContext) {
+
+        WebTarget target = getWebTarget();
+
+        // valid && root by default
+        // URL: [base]/PlanDefinition/$apply
+        String path = BASE_VALID_URL;
+        if (invalid && root) {
+            // URL: [base]/Patient/$apply
+            // Result in 400
+            path = String.format(BASE_INVALID_URL, "Patient");
+        } else if (invalid && !root) {
+            // URL: [base]/Patient/[ID]/$apply
+            // Result in 400
+            path = String.format(RESOURCE_INVALID_URL, "Patient", planDefinition);
+        } else if (!invalid && !root) {
+            // URL: [base]/PlanDefinition/[ID]/$apply
+            path = String.format(RESOURCE_VALID_URL, planDefinition);
+        }
+
+        target = target.path(path);
+
+        // Only if at the root.
+        if (root) {
+            target = addQueryParameter(target, "planDefinition", planDefinition);
+        }
+        target = addQueryParameterList(target, "subject", subject);
+        target = addQueryParameter(target, "encounter", encounter);
+        target = addQueryParameter(target, "practitioner", practitioner);
+        target = addQueryParameter(target, "organization", organization);
+        target = addQueryParameter(target, "userType", userType);
+        target = addQueryParameter(target, "userLanguage", userLanguage);
+        target = addQueryParameter(target, "userTaskContext", userTaskContext);
+        target = addQueryParameter(target, "setting", setting);
+        target = addQueryParameter(target, "settingContext", settingContext);
+
+        if (DEBUG_APPLY) {
+            System.out.println("URL -> " + target.getUri());
+        }
+
+        return target.request(mimeType).accept(FHIRMediaType.APPLICATION_FHIR_JSON).get();
+    }
+
     @Test(groups = { TEST_GROUP_NAME }, dependsOnMethods = "loadTestData")
     public void testEmptySubjectPatient() {
         List<String> subjects = Arrays.asList("");

fhir-server-test/src/test/java/com/ibm/fhir/server/test/operation/EraseOperationTest.java

@@ -1119,12 +1119,15 @@ public void testForbiddenResourceExists() throws IOException, FHIRParserExceptio
      */
     @Test
     public void testForbiddenResourceDoesNotExists() {
+        Entity<Parameters> entity = Entity.entity(generateParameters(true, true, null, null),
+            FHIRMediaType.APPLICATION_FHIR_JSON);
+
         Response r = getWebTarget()
                 .path("/Patient/1-2-3-4-5-BAD/$erase")
                 .request(FHIRMediaType.APPLICATION_FHIR_JSON)
                 .header("X-FHIR-TENANT-ID", "tenant1")
                 .header("X-FHIR-DSID", "study1")
-                .get(Response.class);
+                .post(entity, Response.class);
        assertEquals(r.getStatus(), Status.FORBIDDEN.getStatusCode());
        OperationOutcome outcome = r.readEntity(OperationOutcome.class);
        assertEquals(outcome.getIssue().get(0).getCode().getValue(), "forbidden");
@@ -1192,6 +1195,24 @@ public void testEraseWhenLastResourceVersionDeleted() throws Exception {
         checkResourceNoLongerExists("Patient", id);
     }
 
+    /**
+     * Acceptance Criteria 14 - $erase does not support GET method
+     * When the $erase operation is run with GET method
+     * Then result is bad request
+     */
+    @Test
+    public void testGetMethodNotSupported() {
+        Response r = getWebTarget()
+                .path("/Patient/1-2-3-4-5-BAD/$erase")
+                .request(FHIRMediaType.APPLICATION_FHIR_JSON)
+                .header("X-FHIR-TENANT-ID", "tenant1")
+                .header("X-FHIR-DSID", "study1")
+                .get(Response.class);
+       assertEquals(r.getStatus(), Status.BAD_REQUEST.getStatusCode());
+       OperationOutcome outcome = r.readEntity(OperationOutcome.class);
+       assertEquals(outcome.getIssue().get(0).getCode().getValue(), "not-supported");
+    }
+
     public void loadLargeBundle(Patient r, String id) {
         WebTarget target = getWebTarget();
 

fhir-server-test/src/test/java/com/ibm/fhir/server/test/operation/ReindexOperationTest.java

@@ -18,6 +18,7 @@
 import java.util.UUID;
 
 import javax.ws.rs.client.Entity;
+import javax.ws.rs.client.WebTarget;
 import javax.ws.rs.core.Response;
 import javax.ws.rs.core.Response.Status;
 
@@ -72,6 +73,24 @@ public void testReindex() {
         assertEquals(r.getStatus(), Status.OK.getStatusCode());
     }
 
+    @Test(groups = { "reindex" })
+    public void testReindexWith_GET() {
+        if (!runIt) {
+            System.out.println("Skipping over $reindex IT test");
+            return;
+        }
+        WebTarget target = getWebTarget();
+        target = target.path("/$reindex")
+                .queryParam("resourceCount", "5");
+
+        Response r = target.request(FHIRMediaType.APPLICATION_FHIR_JSON)
+                .header("X-FHIR-TENANT-ID", "default")
+                .header("X-FHIR-DSID", "default")
+                .get();
+
+        assertEquals(r.getStatus(), Status.BAD_REQUEST.getStatusCode());
+    }
+
     @Test(groups = { "reindex" })
     public void testReindexWithType() {
         if (!runIt) {

fhir-server/src/main/java/com/ibm/fhir/server/operation/spi/AbstractOperation.java

@@ -14,6 +14,7 @@
 import com.ibm.fhir.model.resource.OperationDefinition;
 import com.ibm.fhir.model.resource.OperationOutcome;
 import com.ibm.fhir.model.resource.Parameters;
+import com.ibm.fhir.model.resource.Parameters.Parameter;
 import com.ibm.fhir.model.resource.Resource;
 import com.ibm.fhir.model.type.code.IssueType;
 import com.ibm.fhir.model.type.code.OperationParameterUse;
@@ -51,7 +52,7 @@ public final Parameters invoke(
             String logicalId, String versionId,
             Parameters parameters,
             FHIRResourceHelpers resourceHelper) throws FHIROperationException {
-        validateOperationContext(operationContext, resourceType);
+        validateOperationContext(operationContext, resourceType, parameters);
         validateInputParameters(operationContext, resourceType, logicalId, versionId, parameters);
         Parameters result = doInvoke(operationContext, resourceType, logicalId, versionId, parameters, resourceHelper);
         validateOutputParameters(result);
@@ -129,8 +130,18 @@ protected void validateInputParameters(
         validateParameters(parameters, OperationParameterUse.IN);
     }
 
-    protected void validateOperationContext(FHIROperationContext operationContext, Class<? extends Resource> resourceType) throws FHIROperationException {
+    protected void validateOperationContext(FHIROperationContext operationContext, Class<? extends Resource> resourceType, Parameters parameters) throws FHIROperationException {
         OperationDefinition definition = getDefinition();
+
+        // Check which methods are allowed
+        String method = (String) operationContext.getProperty(FHIROperationContext.PROPNAME_METHOD_TYPE);
+        if (!"POST".equalsIgnoreCase(method)
+                && (!"GET".equalsIgnoreCase(method) || !isGetMethodAllowed(definition, parameters))
+                && !isAdditionalMethodAllowed(method)) {
+            String msg = "HTTP method '" + method + "' is not supported for operation: '" + getName() + "'";
+            throw buildExceptionWithIssue(msg, IssueType.NOT_SUPPORTED);
+        }
+
         switch (operationContext.getType()) {
         case INSTANCE:
             if (definition.getInstance().getValue() == false) {
@@ -157,6 +168,45 @@ protected void validateOperationContext(FHIROperationContext operationContext, C
         }
     }
 
+    /**
+     * Determines if the operation disallows the GET method.
+     * This is determined by the affectsState value of the OperatorDefinition and whether the
+     * OperatorDefinition contains any non-primitive parameters.
+     * @param operationDefinition the operation definition
+     * @param parameters the parameters
+     * @return true or false
+     */
+    private boolean isGetMethodAllowed(OperationDefinition operationDefinition, Parameters parameters) {
+        // Check if affectState is true
+        if (operationDefinition.getAffectsState() != null && operationDefinition.getAffectsState().getValue() == Boolean.TRUE) {
+            return false;
+        }
+        // Check for any non-primitive parameters passed in
+        if (parameters != null && operationDefinition.getParameter() != null) {
+            for (Parameter parameter : parameters.getParameter()) {
+                // Determine if parameter is non-primative by checking the operation definition for the parameter type
+                for (OperationDefinition.Parameter odParameter : operationDefinition.getParameter()) {
+                    if (parameter.getName().getValue() != null && odParameter.getName() != null
+                            && parameter.getName().getValue().equals(odParameter.getName().getValue())
+                            && (odParameter.getType() == null || !ModelSupport.isPrimitiveType(ModelSupport.getDataType(odParameter.getType().getValue()))
+                                    || (odParameter.getPart() != null && !odParameter.getPart().isEmpty()))) {
+                        return false;
+                    }
+                }
+            }
+        }
+        return true;
+    }
+
+    /**
+     * Determines if any methods (except GET and POST) are allowed for the operation.
+     * This can be overridden by an operation to allow additional methods.
+     * @return true or false
+     */
+    protected boolean isAdditionalMethodAllowed(String method) {
+        return false;
+    }
+
     /**
      * Determines if the operation disallows abstract resource types, Resource and DomainResource.
      * TODO: Remove this method when Issue #2526 is implemented, at which time, abstract resource types

fhir-server/src/main/java/com/ibm/fhir/server/util/FHIROperationUtil.java

@@ -125,7 +125,20 @@ public static Parameters getInputParameters(OperationDefinition definition,
                             } else if ("uuid".equals(typeName)) {
                                 parameterBuilder.value(Uuid.of(value));
                             } else {
-                                throw new FHIROperationException("Invalid parameter type: '" + typeName + "'");
+                                // 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());
                         }

operation/fhir-operation-bulkdata/src/main/java/com/ibm/fhir/operation/bulkdata/ImportOperation.java

@@ -56,11 +56,6 @@ protected Parameters doInvoke(FHIROperationContext operationContext, Class<? ext
         common.checkEnabled();
         common.checkAllowed(operationContext, true);
 
-        if (!"POST".equals(operationContext.getProperty(FHIROperationContext.PROPNAME_METHOD_TYPE))) {
-            throw buildExceptionWithIssue("Invalid call $import operation only POST allowed",
-                IssueType.INVALID);
-        }
-
         // Checks the Import Type
         checkImportType(operationContext.getType());
 

operation/fhir-operation-bulkdata/src/main/java/com/ibm/fhir/operation/bulkdata/StatusOperation.java

@@ -76,4 +76,10 @@ protected Parameters doInvoke(FHIROperationContext operationContext, Class<? ext
             throw buildExceptionWithIssue("Invalid call $bulkdata-status operation call", IssueType.INVALID);
         }
     }
+
+    @Override
+    protected boolean isAdditionalMethodAllowed(String method) {
+        return "DELETE".equalsIgnoreCase(method);
+    }
+
 }

operation/fhir-operation-bulkdata/src/main/resources/import.json

@@ -25,6 +25,7 @@
         }
     ],
     "description": "Pre ballot version of Bulk Data Import using the Parameters Object",
+    "affectsState": true,
     "code": "import",
     "system": true,
     "type": false,

operation/fhir-operation-reindex/src/main/java/com/ibm/fhir/operation/reindex/ReindexOperation.java

@@ -78,12 +78,6 @@ protected Parameters doInvoke(FHIROperationContext operationContext, Class<? ext
             String logicalId, String versionId, Parameters parameters, FHIRResourceHelpers resourceHelper)
             throws FHIROperationException {
 
-        // Allow only POST because we're changing the state of the database
-        String method = (String) operationContext.getProperty(FHIROperationContext.PROPNAME_METHOD_TYPE);
-        if (!"POST".equalsIgnoreCase(method)) {
-            throw FHIROperationUtil.buildExceptionWithIssue("HTTP method not supported: " + method, IssueType.NOT_SUPPORTED);
-        }
-
         try {
             Instant tstamp = Instant.now();
             List<Long> indexIds = null;

operation/fhir-operation-reindex/src/main/java/com/ibm/fhir/operation/reindex/RetrieveIndexOperation.java

@@ -25,7 +25,6 @@
 import com.ibm.fhir.model.resource.OperationDefinition;
 import com.ibm.fhir.model.resource.Parameters;
 import com.ibm.fhir.model.resource.Resource;
-import com.ibm.fhir.model.type.code.IssueType;
 import com.ibm.fhir.server.operation.spi.AbstractOperation;
 import com.ibm.fhir.server.operation.spi.FHIROperationContext;
 import com.ibm.fhir.server.operation.spi.FHIRResourceHelpers;
@@ -74,12 +73,6 @@ protected Parameters doInvoke(FHIROperationContext operationContext, Class<? ext
             String logicalId, String versionId, Parameters parameters, FHIRResourceHelpers resourceHelper)
             throws FHIROperationException {
 
-        // Only GET or POST is allowed
-        String method = (String) operationContext.getProperty(FHIROperationContext.PROPNAME_METHOD_TYPE);
-        if (!"GET".equalsIgnoreCase(method) && !"POST".equalsIgnoreCase(method)) {
-            throw FHIROperationUtil.buildExceptionWithIssue("HTTP method not supported: " + method, IssueType.NOT_SUPPORTED);
-        }
-
         try {
             String indexIdsString = "";
             int count = MAX_COUNT;

operation/fhir-operation-test/src/test/java/com/ibm/fhir/operation/test/MyOperation.java

@@ -40,7 +40,7 @@ protected OperationDefinition buildOperationDefinition() {
                 .status(PublicationStatus.DRAFT)
                 .kind(OperationKind.OPERATION)
                 .code(Code.of("hello"))
-                .affectsState(Boolean.of(true))
+                .affectsState(Boolean.of(false))
                 .system(Boolean.of(true))
                 .type(Boolean.of(false))
                 .experimental(Boolean.of(true))