From 8f378a07cec640807a588b411e92fffb6d4eee69 Mon Sep 17 00:00:00 2001 From: Mads Baardsgaard Date: Thu, 4 Sep 2014 22:17:22 +0200 Subject: [PATCH] Produce first declared content-type on Accept:*/* This is a fix to reported issue JERSEY-2635, stemming from the incorrect ordering of for loops in MethodSelectingRouter. --- .../routing/MethodSelectingRouter.java | 20 +++--- .../jersey/server/model/Resource.java | 4 +- .../e2e/server/ContentNegotiationTest.java | 67 ++++++++++++------- 3 files changed, 55 insertions(+), 36 deletions(-) diff --git a/core-server/src/main/java/org/glassfish/jersey/server/internal/routing/MethodSelectingRouter.java b/core-server/src/main/java/org/glassfish/jersey/server/internal/routing/MethodSelectingRouter.java index 01aa6f4a10..b0022e0d3b 100644 --- a/core-server/src/main/java/org/glassfish/jersey/server/internal/routing/MethodSelectingRouter.java +++ b/core-server/src/main/java/org/glassfish/jersey/server/internal/routing/MethodSelectingRouter.java @@ -602,16 +602,15 @@ private MediaType determineResponseMediaType(final Class entityClass, CombinedClientServerMediaType selected = null; for (final MediaType acceptableMediaType : acceptableMediaTypes) { - // Use writers suitable for entity class to determine the media type. - for (final MessageBodyWriter writer : workers.getMessageBodyWritersForType(responseEntityClass)) { - for (final MediaType writerProduces : MediaTypes.createFrom(writer.getClass().getAnnotation(Produces.class))) { - - if (writerProduces.isCompatible(acceptableMediaType)) { - // Media types producible by method. - final List methodProducesTypes = !resourceMethod.getProducedTypes().isEmpty() ? - resourceMethod.getProducedTypes() : Lists.newArrayList(MediaType.WILDCARD_TYPE); - - for (final MediaType methodProducesType : methodProducesTypes) { + // Media types producible by method. + final List methodProducesTypes = !resourceMethod.getProducedTypes().isEmpty() ? + resourceMethod.getProducedTypes() : Lists.newArrayList(MediaType.WILDCARD_TYPE); + + for (final MediaType methodProducesType : methodProducesTypes) { + // Use writers suitable for entity class to determine the media type. + for (final MessageBodyWriter writer : workers.getMessageBodyWritersForType(responseEntityClass)) { + for (final MediaType writerProduces : MediaTypes.createFrom(writer.getClass().getAnnotation(Produces.class))) { + if (writerProduces.isCompatible(acceptableMediaType)) { if (methodProducesType.isCompatible(writerProduces)) { final CombinedClientServerMediaType.EffectiveMediaType effectiveProduces = new @@ -627,7 +626,6 @@ private MediaType determineResponseMediaType(final Class entityClass, || CombinedClientServerMediaType.COMPARATOR.compare(candidate, selected) > 0) { if (writer.isWriteable(responseEntityClass, entityType, handlingMethod.getDeclaredAnnotations(), candidate.getCombinedMediaType())) { - selected = candidate; } } diff --git a/core-server/src/main/java/org/glassfish/jersey/server/model/Resource.java b/core-server/src/main/java/org/glassfish/jersey/server/model/Resource.java index ca2bf39bf6..52e7db2376 100644 --- a/core-server/src/main/java/org/glassfish/jersey/server/model/Resource.java +++ b/core-server/src/main/java/org/glassfish/jersey/server/model/Resource.java @@ -203,7 +203,7 @@ public static final class Builder { private List names; private String path; - private final Set methodBuilders; + private final List methodBuilders; private final Set childResourceBuilders; private final List childResources; @@ -219,7 +219,7 @@ public static final class Builder { private Builder(final Resource.Builder parentResource) { - this.methodBuilders = Sets.newIdentityHashSet(); + this.methodBuilders = Lists.newLinkedList(); this.childResourceBuilders = Sets.newIdentityHashSet(); this.childResources = Lists.newLinkedList(); this.resourceMethods = Lists.newLinkedList(); diff --git a/tests/e2e/src/test/java/org/glassfish/jersey/tests/e2e/server/ContentNegotiationTest.java b/tests/e2e/src/test/java/org/glassfish/jersey/tests/e2e/server/ContentNegotiationTest.java index 7475647f78..c8c07bee0f 100644 --- a/tests/e2e/src/test/java/org/glassfish/jersey/tests/e2e/server/ContentNegotiationTest.java +++ b/tests/e2e/src/test/java/org/glassfish/jersey/tests/e2e/server/ContentNegotiationTest.java @@ -67,39 +67,30 @@ public class ContentNegotiationTest extends JerseyTest { @Path("persons") public static class MyResource { + private static final Person[] LIST = new Person[] { + new Person("Penny", 1), + new Person("Howard", 2), + new Person("Sheldon", 3) + }; + @GET @Produces({"application/xml;qs=0.75", "application/json;qs=1.0"}) public Person[] getList() { - Person[] list = new Person[3]; - list[0] = new Person("Penny", 1); - list[1] = new Person("Howard", 2); - list[2] = new Person("Sheldon", 3); - - return list; + return LIST; } @GET @Produces({"application/json;qs=1", "application/xml;qs=0.75"}) @Path("reordered") public Person[] getListReordered() { - Person[] list = new Person[3]; - list[0] = new Person("Penny", 1); - list[1] = new Person("Howard", 2); - list[2] = new Person("Sheldon", 3); - - return list; + return LIST; } @GET @Produces({"application/json;qs=0.75", "application/xml;qs=1"}) @Path("inverted") public Person[] getListInverted() { - Person[] list = new Person[3]; - list[0] = new Person("Penny", 1); - list[1] = new Person("Howard", 2); - list[2] = new Person("Sheldon", 3); - - return list; + return LIST; } @@ -107,12 +98,28 @@ public Person[] getListInverted() { @Produces({"application/xml;qs=0.75", "application/json;qs=0.9", "unknown/hello;qs=1.0"}) @Path("unkownMT") public Person[] getListWithUnkownType() { - Person[] list = new Person[3]; - list[0] = new Person("Penny", 1); - list[1] = new Person("Howard", 2); - list[2] = new Person("Sheldon", 3); + return LIST; + } + + @GET + @Produces({"application/json", "application/xml", "text/plain"}) + @Path("shouldPickFirst") + public Person[] getJsonArrayUnlessOtherwiseSpecified() { + return LIST; + } + + @GET + @Produces("application/json") + @Path("twoMethodsOneEndpoint") + public Person[] getJsonArray() { + return LIST; + } - return list; + @GET + @Produces("application/xml") + @Path("twoMethodsOneEndpoint") + public Person[] getXmlArray() { + return LIST; } } @@ -173,6 +180,20 @@ public void testWithoutDefinedRequestedMediaType() { Assert.assertEquals(MediaType.APPLICATION_JSON_TYPE, response.getMediaType()); } + @Test + public void testWithoutDefinedRequestedMediaTypeAndTwoMethods() { + Response response = target().path("/persons/twoMethodsOneEndpoint").request().get(); + Assert.assertEquals(200, response.getStatus()); + Assert.assertEquals(MediaType.APPLICATION_JSON_TYPE, response.getMediaType()); + } + + @Test + public void testWithoutDefinedRequestedMediaTypeOrQualityModifiers() { + Response response = target().path("/persons/shouldPickFirst").request().get(); + Assert.assertEquals(200, response.getStatus()); + Assert.assertEquals(MediaType.APPLICATION_JSON_TYPE, response.getMediaType()); + } + @Test public void test() { WebTarget target = target().path("/persons");