Fix #267

hapi-fhir-base/src/main/java/ca/uhn/fhir/model/primitive/BaseDateTimeDt.java

@@ -355,13 +355,13 @@ public abstract class BaseDateTimeDt extends BasePrimitive<Date> {
 		} else if (theValue.charAt(0) == 'Z') {
 			clearTimeZone();
 			setTimeZoneZulu(true);
-		} else if (theValue.length() != 5) {
+		} else if (theValue.length() != 6) {
 			throwBadDateFormat(theWholeValue, "Timezone offset must be in the form \"Z\", \"-HH:mm\", or \"+HH:mm\"");
 		} else if (theValue.charAt(3) != ':' || !(theValue.charAt(0) == '+' || theValue.charAt(0) == '-')) {
 			throwBadDateFormat(theWholeValue, "Timezone offset must be in the form \"Z\", \"-HH:mm\", or \"+HH:mm\"");
 		} else {
-			parseInt(theWholeValue, theValue.substring(0, 2), 0, 23);
+			parseInt(theWholeValue, theValue.substring(1, 3), 0, 23);
-			parseInt(theWholeValue, theValue.substring(3, 5), 0, 59);
+			parseInt(theWholeValue, theValue.substring(4, 6), 0, 59);
 			clearTimeZone();
 			setTimeZone(TimeZone.getTimeZone("GMT" + theValue));
 		}

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

@@ -265,7 +265,7 @@ public class ServerConformanceProvider implements IServerConformanceProvider<Con
 						String opName = myOperationBindingToName.get(methodBinding);
 						if (operationNames.add(opName)) {
 							// Only add each operation (by name) once
-							rest.addOperation().setName(methodBinding.getName()).getDefinition().setReference("OperationDefinition/" + opName);
+							rest.addOperation().setName(opName).getDefinition().setReference("OperationDefinition/" + opName);
 						}
 					}
 

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

@@ -47,6 +47,7 @@ import ca.uhn.fhir.rest.annotation.Operation;
 import ca.uhn.fhir.rest.annotation.OperationParam;
 import ca.uhn.fhir.rest.annotation.Read;
 import ca.uhn.fhir.rest.client.IGenericClient;
+import ca.uhn.fhir.rest.client.interceptor.LoggingInterceptor;
 import ca.uhn.fhir.util.PortUtil;
 import ca.uhn.fhir.util.TestUtil;
 
@@ -76,11 +77,14 @@ public class OperationServerDstu2Test {
 		ourLastMethod = "";
 	}
 
-
 	@Test
 	public void testConformance() throws Exception {
 		IGenericClient client = ourCtx.newRestfulGenericClient("http://localhost:" + ourPort);
-		Conformance p = client.fetchConformance().ofType(Conformance.class).execute();
+		LoggingInterceptor loggingInterceptor = new LoggingInterceptor();
+		loggingInterceptor.setLogResponseBody(true);
+		client.registerInterceptor(loggingInterceptor);
+
+		Conformance p = client.fetchConformance().ofType(Conformance.class).prettyPrint().execute();
 		List<RestOperation> ops = p.getRest().get(0).getOperation();
 		assertThat(ops.size(), greaterThan(1));
 		assertNull(ops.get(0).getDefinition().getReference().getBaseUrl());
@@ -88,6 +92,19 @@ public class OperationServerDstu2Test {
 
 		OperationDefinition def = client.read().resource(OperationDefinition.class).withId(ops.get(0).getDefinition().getReference()).execute();
 		assertThat(def.getCode(), not(blankOrNullString()));
+
+		List<String> opNames = toOpNames(ops);
+		assertThat(opNames, containsInRelativeOrder("OP_TYPE"));
+		
+		assertEquals("OperationDefinition/OP_TYPE", ops.get(opNames.indexOf("OP_TYPE")).getDefinition().getReference().getValue());
+	}
+
+	private List<String> toOpNames(List<RestOperation> theOps) {
+		ArrayList<String> retVal = new ArrayList<String>();
+		for (RestOperation next : theOps) {
+			retVal.add(next.getName());
+		}
+		return retVal;
 	}
 
 	@Test
@@ -115,7 +132,6 @@ public class OperationServerDstu2Test {
 		assertEquals("instance $everything", ourLastMethod);
 		assertEquals("Patient/123", ourLastId.toUnqualifiedVersionless().getValue());
 
-		
 	}
 
 	@Test
@@ -367,7 +383,6 @@ public class OperationServerDstu2Test {
 		assertThat(response, containsString("Can not invoke operation $OP_TYPE using HTTP GET because parameter PARAM2 is not a primitive datatype"));
 	}
 
-	
 	@Test
 	public void testOperationWithListParam() throws Exception {
 		Parameters p = new Parameters();
@@ -479,7 +494,6 @@ public class OperationServerDstu2Test {
 		assertEquals("read", ourLastMethod);
 	}
 
-
 	@AfterClass
 	public static void afterClassClearContext() throws Exception {
 		ourServer.stop();
@@ -644,7 +658,7 @@ public class OperationServerDstu2Test {
 			return retVal;
 		}
 
-		@Operation(name = "$everything", idempotent=true)
+		@Operation(name = "$everything", idempotent = true)
 		public Bundle patientEverything(@IdParam IdDt thePatientId) {
 			ourLastMethod = "instance $everything";
 			ourLastId = thePatientId;

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

@@ -88,21 +88,28 @@ import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException;
  * Server FHIR Provider which serves the conformance statement for a RESTful server implementation
  * 
  * <p>
- * Note: This class is safe to extend, but it is important to note that the same instance of {@link Conformance} is
+ * Note: This class is safe to extend, but it is important to note that the same instance of {@link Conformance} is always returned unless {@link #setCache(boolean)} is called with a value of
- * always returned unless {@link #setCache(boolean)} is called with a value of <code>false</code>. This means that if
+ * <code>false</code>. This means that if you are adding anything to the returned conformance instance on each call you should call <code>setCache(false)</code> in your provider constructor.
- * you are adding anything to the returned conformance instance on each call you should call
- * <code>setCache(false)</code> in your provider constructor.
  * </p>
  */
 public class ServerConformanceProvider implements IServerConformanceProvider<Conformance> {
 
+	private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(ServerConformanceProvider.class);
 	private boolean myCache = true;
 	private volatile Conformance myConformance;
 	private IdentityHashMap<OperationMethodBinding, String> myOperationBindingToName;
 	private HashMap<String, List<OperationMethodBinding>> myOperationNameToBindings;
 	private String myPublisher = "Not provided";
+
 	private RestulfulServerConfiguration myServerConfiguration;
 
+	/*
+	 * Add a no-arg constructor and seetter so that the ServerConfirmanceProvider can be Spring-wired with the RestfulService avoiding the potential reference cycle that would happen.
+	 */
+	public ServerConformanceProvider() {
+		super();
+	}
+
 	public ServerConformanceProvider(RestfulServer theRestfulServer) {
 		this.myServerConfiguration = theRestfulServer.createConfiguration();
 	}
@@ -111,18 +118,6 @@ public class ServerConformanceProvider implements IServerConformanceProvider<Con
 		this.myServerConfiguration = theServerConfiguration;
 	}
 
-	/*
-	 * Add a no-arg constructor and seetter so that the ServerConfirmanceProvider can be Spring-wired with the
-	 * RestfulService avoiding the potential reference cycle that would happen.
-	 */
-	public ServerConformanceProvider() {
-		super();
-	}
-
-	public void setRestfulServer(RestfulServer theRestfulServer) {
-		myServerConfiguration = theRestfulServer.createConfiguration();
-	}
-
 	private void checkBindingForSystemOps(ConformanceRestComponent rest, Set<SystemRestfulInteraction> systemOps, BaseMethodBinding<?> nextMethodBinding) {
 		if (nextMethodBinding.getRestOperationType() != null) {
 			String sysOpCode = nextMethodBinding.getRestOperationType().getCode();
@@ -165,14 +160,25 @@ public class ServerConformanceProvider implements IServerConformanceProvider<Con
 		return resourceToMethods;
 	}
 
+	private DateTimeType conformanceDate() {
+		String buildDate = myServerConfiguration.getConformanceDate();
+		if (buildDate != null) {
+			try {
+				return new DateTimeType(buildDate);
+			} catch (DataFormatException e) {
+				// fall through
+			}
+		}
+		return DateTimeType.now();
+	}
+
 	private String createOperationName(OperationMethodBinding theMethodBinding) {
 		return theMethodBinding.getName().substring(1);
 	}
 
 	/**
-	 * Gets the value of the "publisher" that will be placed in the generated conformance statement. As this is a
+	 * Gets the value of the "publisher" that will be placed in the generated conformance statement. As this is a mandatory element, the value should not be null (although this is not enforced). The
-	 * mandatory element, the value should not be null (although this is not enforced). The value defaults to
+	 * value defaults to "Not provided" but may be set to null, which will cause this element to be omitted.
-	 * "Not provided" but may be set to null, which will cause this element to be omitted.
 	 */
 	public String getPublisher() {
 		return myPublisher;
@@ -283,7 +289,7 @@ public class ServerConformanceProvider implements IServerConformanceProvider<Con
 						String opName = myOperationBindingToName.get(methodBinding);
 						if (operationNames.add(opName)) {
 							// Only add each operation (by name) once
-							rest.addOperation().setName(methodBinding.getName()).setDefinition(new Reference(readOperationDefinition(new IdType(opName))));
+							rest.addOperation().setName(opName).setDefinition(new Reference(readOperationDefinition(new IdType(opName))));
 						}
 					}
 
@@ -317,7 +323,8 @@ public class ServerConformanceProvider implements IServerConformanceProvider<Con
 						OperationMethodBinding methodBinding = (OperationMethodBinding) nextMethodBinding;
 						String opName = myOperationBindingToName.get(methodBinding);
 						if (operationNames.add(opName)) {
-							rest.addOperation().setName(methodBinding.getName()).setDefinition(new Reference(readOperationDefinition(new IdType(opName))));
+							ourLog.info("Found bound operation: {}", opName);
+							rest.addOperation().setName(opName).setDefinition(new Reference(readOperationDefinition(new IdType(opName))));
 						}
 					}
 				}
@@ -328,18 +335,6 @@ public class ServerConformanceProvider implements IServerConformanceProvider<Con
 		return retVal;
 	}
 
-	private DateTimeType conformanceDate() {
-		String buildDate = myServerConfiguration.getConformanceDate();
-		if (buildDate != null) {
-			try {
-				return new DateTimeType(buildDate);
-			} catch (DataFormatException e) {
-				// fall through
-			}
-		}
-		return DateTimeType.now();
-	}
-
 	private void handleDynamicSearchMethodBinding(ConformanceRestResourceComponent resource, RuntimeResourceDefinition def, TreeSet<String> includes, DynamicSearchMethodBinding searchMethodBinding) {
 		includes.addAll(searchMethodBinding.getIncludes());
 
@@ -384,7 +379,8 @@ public class ServerConformanceProvider implements IServerConformanceProvider<Con
 		}
 	}
 
-	private void handleSearchMethodBinding(ConformanceRestComponent rest, ConformanceRestResourceComponent resource, String resourceName, RuntimeResourceDefinition def, TreeSet<String> includes, SearchMethodBinding searchMethodBinding) {
+	private void handleSearchMethodBinding(ConformanceRestComponent rest, ConformanceRestResourceComponent resource, String resourceName, RuntimeResourceDefinition def, TreeSet<String> includes,
+			SearchMethodBinding searchMethodBinding) {
 		includes.addAll(searchMethodBinding.getIncludes());
 
 		List<IParameter> params = searchMethodBinding.getParameters();
@@ -485,6 +481,8 @@ public class ServerConformanceProvider implements IServerConformanceProvider<Con
 					}
 
 					String name = createOperationName(methodBinding);
+					ourLog.info("Detected operation: {}", name);
+
 					myOperationBindingToName.put(methodBinding, name);
 					if (myOperationNameToBindings.containsKey(name) == false) {
 						myOperationNameToBindings.put(name, new ArrayList<OperationMethodBinding>());
@@ -519,7 +517,7 @@ public class ServerConformanceProvider implements IServerConformanceProvider<Con
 			if (!sharedDescription.isIdempotent()) {
 				op.setIdempotent(sharedDescription.isIdempotent());
 			}
-			op.setCode(sharedDescription.getName());
+			op.setCode(createOperationName(sharedDescription));
 			if (sharedDescription.isCanOperateAtInstanceLevel()) {
 				op.setInstance(sharedDescription.isCanOperateAtInstanceLevel());
 			}
@@ -579,14 +577,17 @@ public class ServerConformanceProvider implements IServerConformanceProvider<Con
 	}
 
 	/**
-	 * Sets the value of the "publisher" that will be placed in the generated conformance statement. As this is a
+	 * Sets the value of the "publisher" that will be placed in the generated conformance statement. As this is a mandatory element, the value should not be null (although this is not enforced). The
-	 * mandatory element, the value should not be null (although this is not enforced). The value defaults to
+	 * value defaults to "Not provided" but may be set to null, which will cause this element to be omitted.
-	 * "Not provided" but may be set to null, which will cause this element to be omitted.
 	 */
 	public void setPublisher(String thePublisher) {
 		myPublisher = thePublisher;
 	}
 
+	public void setRestfulServer(RestfulServer theRestfulServer) {
+		myServerConfiguration = theRestfulServer.createConfiguration();
+	}
+
 	private void sortRuntimeSearchParameters(List<RuntimeSearchParam> searchParameters) {
 		Collections.sort(searchParameters, new Comparator<RuntimeSearchParam>() {
 			@Override

hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/OperationServerDstu3Test.java

@@ -1,6 +1,7 @@
 package ca.uhn.fhir.rest.server;
 
 import static org.hamcrest.Matchers.blankOrNullString;
+import static org.hamcrest.Matchers.containsInRelativeOrder;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.greaterThan;
 import static org.hamcrest.Matchers.not;
@@ -45,12 +46,12 @@ import org.junit.BeforeClass;
 import org.junit.Test;
 
 import ca.uhn.fhir.context.FhirContext;
-import ca.uhn.fhir.model.api.IResource;
 import ca.uhn.fhir.rest.annotation.IdParam;
 import ca.uhn.fhir.rest.annotation.Operation;
 import ca.uhn.fhir.rest.annotation.OperationParam;
 import ca.uhn.fhir.rest.annotation.Read;
 import ca.uhn.fhir.rest.client.IGenericClient;
+import ca.uhn.fhir.rest.client.interceptor.LoggingInterceptor;
 import ca.uhn.fhir.util.PortUtil;
 import ca.uhn.fhir.util.TestUtil;
 
@@ -84,10 +85,27 @@ public class OperationServerDstu3Test {
 	@Test
 	public void testConformance() throws Exception {
 		IGenericClient client = ourCtx.newRestfulGenericClient("http://localhost:" + ourPort);
-		Conformance p = client.fetchConformance().ofType(Conformance.class).execute();
+		LoggingInterceptor loggingInterceptor = new LoggingInterceptor();
+		loggingInterceptor.setLogResponseBody(true);
+		client.registerInterceptor(loggingInterceptor);
+
+		Conformance p = client.fetchConformance().ofType(Conformance.class).prettyPrint().execute();
 		List<ConformanceRestOperationComponent> ops = p.getRest().get(0).getOperation();
 		assertThat(ops.size(), greaterThan(1));
-		assertThat(ops.get(0).getDefinition().getReferenceElement().getValue(), startsWith("#"));
+
+		List<String> opNames = toOpNames(ops);
+		assertThat(opNames, containsInRelativeOrder("OP_TYPE"));
+		
+		OperationDefinition def = (OperationDefinition) ops.get(opNames.indexOf("OP_TYPE")).getDefinition().getResource();
+		assertEquals("OP_TYPE", def.getCode());
+	}
+
+	private List<String> toOpNames(List<ConformanceRestOperationComponent> theOps) {
+		ArrayList<String> retVal = new ArrayList<String>();
+		for (ConformanceRestOperationComponent next : theOps) {
+			retVal.add(next.getName());
+		}
+		return retVal;
 	}
 
 	@Test

src/changes/changes.xml

@@ -319,6 +319,11 @@
 				<![CDATA[<code>_raw=true</code>]]> mode has been deprecated and
 				will be removed at some point.
 			</action>
+			<action type="fix" issue="267">
+				Operation definitions (e.g. for $everything operation) in the generated
+				server conformance statement should not include the $ prefix in the operation
+				name or code. Thanks to Dion McMurtrie for reporting!
+			</action>
 		</release>
 		<release version="1.5" date="2016-04-20">
 			<action type="fix" issue="339">