Return a better error message if the server finds multiple resource

providers for the same resource type
This commit is contained in:
James Agnew 2014-12-04 11:04:53 -05:00
parent 29dab8bad9
commit ecadd83711
5 changed files with 164 additions and 6 deletions

View File

@ -223,6 +223,13 @@
</profile> </profile>
<profile> <profile>
<id>ANDROID</id> <id>ANDROID</id>
<dependencies>
<dependency>
<groupId>ca.uhn.hapi.fhir</groupId>
<artifactId>hapi-fhir-structures-dstu</artifactId>
<version>0.8-SNAPSHOT</version>
</dependency>
</dependencies>
<build> <build>
<plugins> <plugins>
<plugin> <plugin>
@ -238,6 +245,18 @@
<configuration> <configuration>
<shadedArtifactAttached>false</shadedArtifactAttached> <shadedArtifactAttached>false</shadedArtifactAttached>
<createDependencyReducedPom>true</createDependencyReducedPom> <createDependencyReducedPom>true</createDependencyReducedPom>
<artifactSet>
<includes>
<!--
<include>javax.json:javax.json-api</include>
-->
<include>ca.uhn.hapi.fhir:hapi-fhir-structures-dstu</include>
<include>org.glassfish:javax.json</include>
<include>org.codehaus.woodstox:woodstox-core-asl</include>
<include>javax.xml.stream:stax-api</include>
<include>org.codehaus.woodstox:stax2-api</include>
</includes>
</artifactSet>
<relocations> <relocations>
<relocation> <relocation>
<pattern>javax.xml.stream</pattern> <pattern>javax.xml.stream</pattern>

View File

@ -294,7 +294,7 @@ class ModelScanner {
} }
if (blockDefinition == null && datatypeDefinition == null && resourceDefinition == null) { if (blockDefinition == null && datatypeDefinition == null && resourceDefinition == null) {
throw new ConfigurationException("Resource type does not contain any valid HAPI-FHIR annotations: " + theClass.getCanonicalName()); throw new ConfigurationException("Resource class[" + theClass.getName() + "] does not contain any valid HAPI-FHIR annotations");
} }
} }

View File

@ -41,6 +41,7 @@ import ca.uhn.fhir.rest.server.exceptions.InternalErrorException;
import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
import ca.uhn.fhir.rest.server.interceptor.IServerInterceptor; import ca.uhn.fhir.rest.server.interceptor.IServerInterceptor;
import ca.uhn.fhir.util.VersionUtil; import ca.uhn.fhir.util.VersionUtil;
import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.Validate; import org.apache.commons.lang3.Validate;
import org.apache.commons.lang3.exception.ExceptionUtils; import org.apache.commons.lang3.exception.ExceptionUtils;
@ -50,6 +51,7 @@ import javax.servlet.ServletOutputStream;
import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpServletResponse;
import java.io.IOException; import java.io.IOException;
import java.io.OutputStreamWriter; import java.io.OutputStreamWriter;
import java.io.UnsupportedEncodingException; import java.io.UnsupportedEncodingException;
@ -723,16 +725,19 @@ public class RestfulServer extends HttpServlet {
Collection<IResourceProvider> resourceProvider = getResourceProviders(); Collection<IResourceProvider> resourceProvider = getResourceProviders();
if (resourceProvider != null) { if (resourceProvider != null) {
Map<Class<? extends IResource>, IResourceProvider> typeToProvider = new HashMap<Class<? extends IResource>, IResourceProvider>(); Map<String, IResourceProvider> typeToProvider = new HashMap<String, IResourceProvider>();
for (IResourceProvider nextProvider : resourceProvider) { for (IResourceProvider nextProvider : resourceProvider) {
Class<? extends IResource> resourceType = nextProvider.getResourceType(); Class<? extends IResource> resourceType = nextProvider.getResourceType();
if (resourceType == null) { if (resourceType == null) {
throw new NullPointerException("getResourceType() on class '" + nextProvider.getClass().getCanonicalName() + "' returned null"); throw new NullPointerException("getResourceType() on class '" + nextProvider.getClass().getCanonicalName() + "' returned null");
} }
if (typeToProvider.containsKey(resourceType)) {
throw new ServletException("Multiple providers for type: " + resourceType.getCanonicalName()); String resourceName = myFhirContext.getResourceDefinition(resourceType).getName();
if (typeToProvider.containsKey(resourceName)) {
throw new ServletException("Multiple resource providers return resource type[" + resourceName + "]: First[" + typeToProvider.get(resourceName).getClass().getCanonicalName() + "] and Second[" + nextProvider.getClass().getCanonicalName() + "]");
} }
typeToProvider.put(resourceType, nextProvider); typeToProvider.put(resourceName, nextProvider);
providedResourceScanner.scanForProvidedResources(nextProvider); providedResourceScanner.scanForProvidedResources(nextProvider);
} }
ourLog.info("Got {} resource providers", typeToProvider.size()); ourLog.info("Got {} resource providers", typeToProvider.size());

View File

@ -3,6 +3,7 @@ package ca.uhn.fhir.rest.server;
import static org.junit.Assert.*; import static org.junit.Assert.*;
import java.util.List; import java.util.List;
import java.util.Map;
import javax.servlet.ServletException; import javax.servlet.ServletException;
@ -10,8 +11,13 @@ import org.hamcrest.core.StringContains;
import org.junit.Test; import org.junit.Test;
import ca.uhn.fhir.model.api.BaseResource; import ca.uhn.fhir.model.api.BaseResource;
import ca.uhn.fhir.model.api.IElement;
import ca.uhn.fhir.model.api.IResource; import ca.uhn.fhir.model.api.IResource;
import ca.uhn.fhir.model.api.ResourceMetadataKeyEnum;
import ca.uhn.fhir.model.dstu.composite.ContainedDt;
import ca.uhn.fhir.model.dstu.composite.NarrativeDt;
import ca.uhn.fhir.model.dstu.resource.Patient; import ca.uhn.fhir.model.dstu.resource.Patient;
import ca.uhn.fhir.model.primitive.CodeDt;
import ca.uhn.fhir.model.primitive.IdDt; import ca.uhn.fhir.model.primitive.IdDt;
import ca.uhn.fhir.rest.annotation.IdParam; import ca.uhn.fhir.rest.annotation.IdParam;
import ca.uhn.fhir.rest.annotation.Read; import ca.uhn.fhir.rest.annotation.Read;
@ -62,6 +68,21 @@ public class ServerInvalidDefinitionTest {
} }
} }
@Test
public void testMultipleResourceProviderForSameType() {
RestfulServer srv = new RestfulServer();
srv.setResourceProviders(new PatientResourceProvider1(), new PatientResourceProvider2());
try {
srv.init();
fail();
} catch (ServletException e) {
assertThat(e.getCause().toString(), StringContains.containsString("[Patient]"));
assertThat(e.getCause().toString(), StringContains.containsString("PatientResourceProvider1]"));
assertThat(e.getCause().toString(), StringContains.containsString("PatientResourceProvider2]"));
}
}
@Test @Test
public void testSearchWithId() { public void testSearchWithId() {
RestfulServer srv = new RestfulServer(); RestfulServer srv = new RestfulServer();
@ -76,6 +97,19 @@ public class ServerInvalidDefinitionTest {
} }
} }
@Test
public void testProviderWithNonResourceType() {
RestfulServer srv = new RestfulServer();
srv.setResourceProviders(new ProviderWithNonResourceType());
try {
srv.init();
fail();
} catch (ServletException e) {
assertThat(e.getCause().toString(), StringContains.containsString("ConfigurationException"));
assertThat(e.getCause().toString(), StringContains.containsString("does not contain any valid HAPI-FHIR annotations"));
}
}
/** /**
* Normal, should initialize properly * Normal, should initialize properly
*/ */
@ -127,6 +161,74 @@ public class ServerInvalidDefinitionTest {
} }
} }
public static class ProviderWithNonResourceType implements IResourceProvider {
@Override
public Class<? extends IResource> getResourceType() {
return new IResource() {
@Override
public boolean isEmpty() {
return false;
}
@Override
public <T extends IElement> List<T> getAllPopulatedChildElementsOfType(Class<T> theType) {
return null;
}
@Override
public void setResourceMetadata(Map<ResourceMetadataKeyEnum<?>, Object> theMap) {
}
@Override
public void setLanguage(CodeDt theLanguage) {
}
@Override
public void setId(IdDt theId) {
}
@Override
public NarrativeDt getText() {
return null;
}
@Override
public String getResourceName() {
return null;
}
@Override
public Map<ResourceMetadataKeyEnum<?>, Object> getResourceMetadata() {
return null;
}
@Override
public CodeDt getLanguage() {
return null;
}
@Override
public IdDt getId() {
return null;
}
@Override
public ContainedDt getContained() {
return null;
}
}.getClass();
}
@Search
public List<Patient> read(@IdParam IdDt theId, @RequiredParam(name = "aaa") StringParam theParam) {
return null;
}
}
public static class InvalidSpecialParameterNameResourceProvider implements IResourceProvider { public static class InvalidSpecialParameterNameResourceProvider implements IResourceProvider {
@ -156,4 +258,32 @@ public class ServerInvalidDefinitionTest {
} }
public static class PatientResourceProvider1 implements IResourceProvider {
@Override
public Class<Patient> getResourceType() {
return Patient.class;
}
@Read
public Patient read(@IdParam IdDt theId) {
return null;
}
}
public static class PatientResourceProvider2 implements IResourceProvider {
@Override
public Class<Patient> getResourceType() {
return Patient.class;
}
@Read
public Patient read(@IdParam IdDt theId) {
return null;
}
}
} }

View File

@ -169,7 +169,11 @@
Encoding a Binary resource without a content type set should not result in a NullPointerException. Thanks Encoding a Binary resource without a content type set should not result in a NullPointerException. Thanks
to Alexander Kley for reporting! to Alexander Kley for reporting!
</action> </action>
</release> <action type="add">
Server gives a more helpful error message if multiple IResourceProvider implementations
are provided for the same resource type. Thanks to wanghaisheng for the idea!
</action>
</release>
<release version="0.7" date="2014-Oct-23"> <release version="0.7" date="2014-Oct-23">
<action type="add" issue="30"> <action type="add" issue="30">
<![CDATA[<b>API CHANGE:</b>]]> The TagList class previously implemented ArrayList semantics, <![CDATA[<b>API CHANGE:</b>]]> The TagList class previously implemented ArrayList semantics,