Fix #378 - Include mandatory elements in server generated

OperationDefinition resources

hapi-fhir-structures-dstu2/src/main/java/ca/uhn/fhir/rest/server/provider/dstu2/ServerConformanceProvider.java

@@ -1,5 +1,6 @@
 package ca.uhn.fhir.rest.server.provider.dstu2;
 
+import static org.apache.commons.lang3.StringUtils.isBlank;
 /*
  * #%L
  * HAPI FHIR Structures - DSTU2 (FHIR v1.0.0)
@@ -53,6 +54,7 @@ import ca.uhn.fhir.model.dstu2.resource.OperationDefinition.Parameter;
 import ca.uhn.fhir.model.dstu2.valueset.ConditionalDeleteStatusEnum;
 import ca.uhn.fhir.model.dstu2.valueset.ConformanceResourceStatusEnum;
 import ca.uhn.fhir.model.dstu2.valueset.ConformanceStatementKindEnum;
+import ca.uhn.fhir.model.dstu2.valueset.OperationKindEnum;
 import ca.uhn.fhir.model.dstu2.valueset.OperationParameterUseEnum;
 import ca.uhn.fhir.model.dstu2.valueset.ResourceTypeEnum;
 import ca.uhn.fhir.model.dstu2.valueset.RestfulConformanceModeEnum;
@@ -502,6 +504,7 @@ public class ServerConformanceProvider implements IServerConformanceProvider<Con
 
 		OperationDefinition op = new OperationDefinition();
 		op.setStatus(ConformanceResourceStatusEnum.ACTIVE);
+		op.setKind(OperationKindEnum.OPERATION);
 		op.setIdempotent(true);
 
 		Set<String> inParams = new HashSet<String>();
@@ -557,6 +560,21 @@ public class ServerConformanceProvider implements IServerConformanceProvider<Con
 			}
 		}
 
+		if (isBlank(op.getName())) {
+			if (isNotBlank(op.getDescription())) {
+				op.setName(op.getDescription());
+			} else {
+				op.setName(op.getCode());
+			}
+		}
+		
+		if (op.getSystem() == null) {
+			op.setSystem(false);
+		}
+		if (op.getInstance() == null) {
+			op.setInstance(false);
+		}
+
 		return op;
 	}
 

hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/rest/server/ServerConformanceProviderDstu2Test.java

@@ -76,13 +76,32 @@ import ca.uhn.fhir.rest.param.ReferenceAndListParam;
 import ca.uhn.fhir.rest.param.TokenOrListParam;
 import ca.uhn.fhir.rest.server.provider.dstu2.ServerConformanceProvider;
 import ca.uhn.fhir.util.TestUtil;
+import ca.uhn.fhir.validation.FhirValidator;
 import ca.uhn.fhir.validation.ValidationResult;
 
 public class ServerConformanceProviderDstu2Test {
 
-	private static FhirContext ourCtx = FhirContext.forDstu2();
+	private static FhirContext ourCtx;
+	private static FhirValidator ourValidator;
 	private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(ServerConformanceProviderDstu2Test.class);
 
+	static {
+		ourCtx = FhirContext.forDstu2();
+		ourValidator = ourCtx.newValidator();
+		ourValidator.setValidateAgainstStandardSchema(true);
+		ourValidator.setValidateAgainstStandardSchematron(true);
+	}
+
+	private void validate(OperationDefinition theOpDef) {
+		String conf = ourCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(theOpDef);
+		ourLog.info("Def: {}", conf);
+
+		ValidationResult result = ourValidator.validateWithResult(theOpDef);
+		String outcome = ourCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(result.toOperationOutcome());
+		ourLog.info("Outcome: {}", outcome);
+		
+		assertTrue(outcome, result.isSuccessful());
+	}
 
 	private HttpServletRequest createHttpServletRequest() {
 		HttpServletRequest req = mock(HttpServletRequest.class);
@@ -168,9 +187,7 @@ public class ServerConformanceProviderDstu2Test {
 		rs.init(createServletConfig());
 
 		OperationDefinition opDef = sc.readOperationDefinition(new IdDt("OperationDefinition/Patient_i_everything"));
-
+		validate(opDef);
-		String conf = ourCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(opDef);
-		ourLog.info(conf);
 
 		assertEquals("everything", opDef.getCode());
 		assertEquals(true, opDef.getIdempotent().booleanValue());
@@ -277,11 +294,12 @@ public class ServerConformanceProviderDstu2Test {
 		
 		{
 			OperationDefinition opDef = sc.readOperationDefinition(new IdDt("OperationDefinition/Patient_i_someOp"));
-			ourLog.info(ourCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(opDef));
+			validate(opDef);
+			
 			Set<String> types = toStrings(opDef.getType());
 			assertEquals("someOp", opDef.getCode());
 			assertEquals(true, opDef.getInstance());
-			assertEquals(null, opDef.getSystem());
+			assertEquals(false, opDef.getSystem());
 			assertThat(types, containsInAnyOrder("Patient"));
 			assertEquals(2, opDef.getParameter().size());
 			assertEquals("someOpParam1", opDef.getParameter().get(0).getName());
@@ -291,11 +309,12 @@ public class ServerConformanceProviderDstu2Test {
 		}
 		{
 			OperationDefinition opDef = sc.readOperationDefinition(new IdDt("OperationDefinition/Encounter_i_someOp"));
-			ourLog.info(ourCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(opDef));
+			validate(opDef);
+
 			Set<String> types = toStrings(opDef.getType());
 			assertEquals("someOp", opDef.getCode());
 			assertEquals(true, opDef.getInstance());
-			assertEquals(null, opDef.getSystem());
+			assertEquals(false, opDef.getSystem());
 			assertThat(types, containsInAnyOrder("Encounter"));
 			assertEquals(2, opDef.getParameter().size());
 			assertEquals("someOpParam1", opDef.getParameter().get(0).getName());
@@ -305,11 +324,12 @@ public class ServerConformanceProviderDstu2Test {
 		}
 		{
 			OperationDefinition opDef = sc.readOperationDefinition(new IdDt("OperationDefinition/Patient_i_validate"));
-			ourLog.info(ourCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(opDef));
+			validate(opDef);
+
 			Set<String> types = toStrings(opDef.getType());
 			assertEquals("validate", opDef.getCode());
 			assertEquals(true, opDef.getInstance());
-			assertEquals(null, opDef.getSystem());
+			assertEquals(false, opDef.getSystem());
 			assertThat(types, containsInAnyOrder("Patient"));
 			assertEquals(1, opDef.getParameter().size());
 			assertEquals("resource", opDef.getParameter().get(0).getName());
@@ -357,9 +377,7 @@ public class ServerConformanceProviderDstu2Test {
 		assertEquals("OperationDefinition/_is_plain", sconf.getRest().get(0).getOperation().get(0).getDefinition().getReference().getValue());
 
 		OperationDefinition opDef = sc.readOperationDefinition(new IdDt("OperationDefinition/_is_plain"));
-
+		validate(opDef);
-		String conf = ourCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(opDef);
-		ourLog.info(conf);
 
 		assertEquals("plain", opDef.getCode());
 		assertEquals(true, opDef.getIdempotent().booleanValue());

hapi-fhir-structures-dstu3/src/main/java/org/hl7/fhir/dstu3/hapi/rest/server/ServerConformanceProvider.java

@@ -1,5 +1,6 @@
 package org.hl7.fhir.dstu3.hapi.rest.server;
 
+import static org.apache.commons.lang3.StringUtils.isBlank;
 /*
  * #%L
  * HAPI FHIR Structures - DSTU2 (FHIR v1.0.0)
@@ -56,6 +57,7 @@ import org.hl7.fhir.dstu3.model.Conformance.TypeRestfulInteraction;
 import org.hl7.fhir.dstu3.model.Conformance.UnknownContentCode;
 import org.hl7.fhir.dstu3.model.Enumerations.ConformanceResourceStatus;
 import org.hl7.fhir.dstu3.model.OperationDefinition.OperationDefinitionParameterComponent;
+import org.hl7.fhir.dstu3.model.OperationDefinition.OperationKind;
 import org.hl7.fhir.dstu3.model.OperationDefinition.OperationParameterUse;
 import org.hl7.fhir.dstu3.model.Reference;
 import org.hl7.fhir.instance.model.api.IBaseResource;
@@ -523,6 +525,7 @@ public class ServerConformanceProvider implements IServerConformanceProvider<Con
 
 		OperationDefinition op = new OperationDefinition();
 		op.setStatus(ConformanceResourceStatus.ACTIVE);
+		op.setKind(OperationKind.OPERATION);
 		op.setIdempotent(true);
 
 		Set<String> inParams = new HashSet<String>();
@@ -532,6 +535,12 @@ public class ServerConformanceProvider implements IServerConformanceProvider<Con
 			if (isNotBlank(sharedDescription.getDescription())) {
 				op.setDescription(sharedDescription.getDescription());
 			}
+			if (sharedDescription.isCanOperateAtInstanceLevel()) {
+				op.setInstance(true);
+			}
+			if (sharedDescription.isCanOperateAtServerLevel()) {
+				op.setSystem(true);
+			}
 			if (!sharedDescription.isIdempotent()) {
 				op.setIdempotent(sharedDescription.isIdempotent());
 			}
@@ -581,6 +590,21 @@ public class ServerConformanceProvider implements IServerConformanceProvider<Con
 			}
 		}
 
+		if (isBlank(op.getName())) {
+			if (isNotBlank(op.getDescription())) {
+				op.setName(op.getDescription());
+			} else {
+				op.setName(op.getCode());
+			}
+		}
+		
+		if (op.hasSystem() == false) {
+			op.setSystem(false);
+		}
+		if (op.hasInstance() == false) {
+			op.setInstance(false);
+		}
+		
 		return op;
 	}
 

hapi-fhir-structures-dstu3/src/test/java/org/hl7/fhir/dstu3/hapi/rest/server/ServerConformanceProviderDstu3Test.java

@@ -73,12 +73,21 @@ import ca.uhn.fhir.rest.server.IResourceProvider;
 import ca.uhn.fhir.rest.server.ResourceBinding;
 import ca.uhn.fhir.rest.server.RestfulServer;
 import ca.uhn.fhir.util.TestUtil;
+import ca.uhn.fhir.validation.FhirValidator;
 import ca.uhn.fhir.validation.ValidationResult;
 
 public class ServerConformanceProviderDstu3Test {
 
-	private static FhirContext ourCtx = FhirContext.forDstu3();
+	private static FhirContext ourCtx;
 	private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(ServerConformanceProviderDstu3Test.class);
+	private static FhirValidator ourValidator;
+
+	static {
+		ourCtx = FhirContext.forDstu3();
+		ourValidator = ourCtx.newValidator();
+		ourValidator.setValidateAgainstStandardSchema(true);
+		ourValidator.setValidateAgainstStandardSchematron(true);
+	}
 
 	private HttpServletRequest createHttpServletRequest() {
 		HttpServletRequest req = mock(HttpServletRequest.class);
@@ -150,7 +159,8 @@ public class ServerConformanceProviderDstu3Test {
 		assertEquals(1, conformance.getRest().get(0).getOperation().size());
 		assertEquals("everything", conformance.getRest().get(0).getOperation().get(0).getName());
 
-		OperationDefinition opDef = sc.readOperationDefinition(new IdType("OperationDefinition/everything"));
+		OperationDefinition opDef = sc.readOperationDefinition(new IdType("OperationDefinition/Patient_i_everything"));
+		validate(opDef);
 		assertEquals("everything", opDef.getCode());
 	}
 
@@ -167,6 +177,7 @@ public class ServerConformanceProviderDstu3Test {
 		rs.init(createServletConfig());
 
 		OperationDefinition opDef = sc.readOperationDefinition(new IdType("OperationDefinition/Patient_i_everything"));
+		validate(opDef);
 
 		String conf = ourCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(opDef);
 		ourLog.info(conf);
@@ -250,14 +261,6 @@ public class ServerConformanceProviderDstu3Test {
 		assertNull(res.getConditionalUpdateElement().getValue());
 	}
 
-	private List<String> toOperationIdParts(List<ConformanceRestOperationComponent> theOperation) {
-		ArrayList<String> retVal = Lists.newArrayList();
-		for (ConformanceRestOperationComponent next : theOperation) {
-			retVal.add(next.getDefinition().getReferenceElement().getIdPart());
-		}
-		return retVal;
-	}
-
 	/** See #379 */
 	@Test
 	public void testOperationAcrossMultipleTypes() throws Exception {
@@ -277,12 +280,13 @@ public class ServerConformanceProviderDstu3Test {
 		assertEquals(4, conformance.getRest().get(0).getOperation().size());
 		List<String> operationNames = toOperationNames(conformance.getRest().get(0).getOperation());
 		assertThat(operationNames, containsInAnyOrder("someOp", "validate", "someOp", "validate"));
-		
+
 		List<String> operationIdParts = toOperationIdParts(conformance.getRest().get(0).getOperation());
-		assertThat(operationIdParts, containsInAnyOrder("Patient_i_someOp","Encounter_i_someOp","Patient_i_validate","Encounter_i_validate"));
+		assertThat(operationIdParts, containsInAnyOrder("Patient_i_someOp", "Encounter_i_someOp", "Patient_i_validate", "Encounter_i_validate"));
-		
+
 		{
 			OperationDefinition opDef = sc.readOperationDefinition(new IdType("OperationDefinition/Patient_i_someOp"));
+			validate(opDef);
 			ourLog.info(ourCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(opDef));
 			Set<String> types = toStrings(opDef.getType());
 			assertEquals("someOp", opDef.getCode());
@@ -297,6 +301,7 @@ public class ServerConformanceProviderDstu3Test {
 		}
 		{
 			OperationDefinition opDef = sc.readOperationDefinition(new IdType("OperationDefinition/Encounter_i_someOp"));
+			validate(opDef);
 			ourLog.info(ourCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(opDef));
 			Set<String> types = toStrings(opDef.getType());
 			assertEquals("someOp", opDef.getCode());
@@ -311,6 +316,7 @@ public class ServerConformanceProviderDstu3Test {
 		}
 		{
 			OperationDefinition opDef = sc.readOperationDefinition(new IdType("OperationDefinition/Patient_i_validate"));
+			validate(opDef);
 			ourLog.info(ourCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(opDef));
 			Set<String> types = toStrings(opDef.getType());
 			assertEquals("validate", opDef.getCode());
@@ -322,7 +328,7 @@ public class ServerConformanceProviderDstu3Test {
 			assertEquals("Patient", opDef.getParameter().get(0).getType());
 		}
 	}
-
+	
 	@Test
 	public void testOperationDocumentation() throws Exception {
 
@@ -359,13 +365,13 @@ public class ServerConformanceProviderDstu3Test {
 		rs.init(createServletConfig());
 
 		OperationDefinition opDef = sc.readOperationDefinition(new IdType("OperationDefinition/_is_plain"));
-
+		validate(opDef);
-		String conf = ourCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(opDef);
-		ourLog.info(conf);
 
 		assertEquals("plain", opDef.getCode());
 		assertEquals(true, opDef.getIdempotent());
 		assertEquals(3, opDef.getParameter().size());
+
+		assertTrue(opDef.getParameter().get(0).hasName());
 		assertEquals("start", opDef.getParameter().get(0).getName());
 		assertEquals("in", opDef.getParameter().get(0).getUse().toCode());
 		assertEquals("0", opDef.getParameter().get(0).getMinElement().getValueAsString());
@@ -613,6 +619,14 @@ public class ServerConformanceProviderDstu3Test {
 		assertTrue(result.getMessages().toString(), result.isSuccessful());
 	}
 
+	private List<String> toOperationIdParts(List<ConformanceRestOperationComponent> theOperation) {
+		ArrayList<String> retVal = Lists.newArrayList();
+		for (ConformanceRestOperationComponent next : theOperation) {
+			retVal.add(next.getDefinition().getReferenceElement().getIdPart());
+		}
+		return retVal;
+	}
+
 	private List<String> toOperationNames(List<ConformanceRestOperationComponent> theOperation) {
 		ArrayList<String> retVal = Lists.newArrayList();
 		for (ConformanceRestOperationComponent next : theOperation) {
@@ -629,6 +643,17 @@ public class ServerConformanceProviderDstu3Test {
 		return retVal;
 	}
 
+	private void validate(OperationDefinition theOpDef) {
+		String conf = ourCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(theOpDef);
+		ourLog.info("Def: {}", conf);
+
+		ValidationResult result = ourValidator.validateWithResult(theOpDef);
+		String outcome = ourCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(result.toOperationOutcome());
+		ourLog.info("Outcome: {}", outcome);
+		
+		assertTrue(outcome, result.isSuccessful());
+	}
+
 	@AfterClass
 	public static void afterClassClearContext() {
 		TestUtil.clearAllStaticFieldsForUnitTest();

src/changes/changes.xml

@@ -324,6 +324,18 @@
 				server conformance statement should not include the $ prefix in the operation
 				name or code. Thanks to Dion McMurtrie for reporting!
 			</action>
+			<action type="fix" issue="378">
+				Server generated OperationDefinition resources did not validate
+				due to some missing elements (kind, status, etc.).
+				Thanks to
+				Michael Lawley for reporting!
+			</action>
+			<action type="fix" issue="379">
+				Operations that are defined on multiple resource provider types with
+				the same name (e.g. "$everything") are now automatically exposed by the server
+				as separate OperationDefinition resources per resource type. Thanks to
+				Michael Lawley for reporting!
+			</action>
 			<action type="fix" issue="380">
 				OperationDefinition resources generated automatically by the server for operations
 				that are defined within resource/plain providers incorrectly stated that
@@ -332,12 +344,6 @@
 				<![CDATA[@OperationParam]]> annotation describes. Thanks to Michael Lawley
 				for reporting!
 			</action>
-			<action type="fix" issue="379">
-				Operations that are defined on multiple resource provider types with
-				the same name (e.g. "$everything") are now automatically exposed by the server
-				as separate OperationDefinition resources per resource type. Thanks to
-				Michael Lawley for reporting!
-			</action>
 		</release>
 		<release version="1.5" date="2016-04-20">
 			<action type="fix" issue="339">