Skip to content

Commit

Permalink
Resteasy Reactive: Fix support media types with suffixes
Browse files Browse the repository at this point in the history
Given a media type with suffix, for example: "application/test+json".

Before these changes, Resteasy Reactive was trying to find the writer/reader using only the suffix. For example, with a media type "application/test+json", it would only use "json" to locate the right writer/reader.

Spite of this has been working well so far, if users provide a custom writer/reader for a concrete media type with suffix:

```java
@Provider
    @produces("text/test+suffix")
    public static class SuffixMessageBodyWriter implements ServerMessageBodyWriter<Object> {

        @OverRide
        public boolean isWriteable(Class<?> type, Type genericType, ResteasyReactiveResourceInfo target, MediaType mediaType) {
           // ...
        }

        @OverRide
        public void writeResponse(Object o, Type genericType, ServerRequestContext context) {
            // ...
        }

        // ...
    }
```

The above writer will never be used.

After these changes, we will use the media type with suffix, plus the split parts of the suffix. For example, if the media type with suffix is: "application/test+json", it will try to find the best writer/reader for "application/test+json", "application/test" and then "application/json" (In this order).

Fix #15982

(cherry picked from commit d6a5178)
  • Loading branch information
Sgitario authored and gsmet committed May 24, 2022
1 parent 8acfcb8 commit 5407ac5
Show file tree
Hide file tree
Showing 11 changed files with 363 additions and 55 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,216 @@
package io.quarkus.resteasy.reactive.server.test.resource.basic;

import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.lang.annotation.Annotation;
import java.lang.reflect.Type;
import java.nio.charset.StandardCharsets;

import javax.ws.rs.Consumes;
import javax.ws.rs.GET;
import javax.ws.rs.Path;
import javax.ws.rs.Produces;
import javax.ws.rs.WebApplicationException;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.MultivaluedMap;
import javax.ws.rs.ext.Provider;

import org.hamcrest.Matchers;
import org.jboss.resteasy.reactive.server.spi.ResteasyReactiveResourceInfo;
import org.jboss.resteasy.reactive.server.spi.ServerMessageBodyReader;
import org.jboss.resteasy.reactive.server.spi.ServerMessageBodyWriter;
import org.jboss.resteasy.reactive.server.spi.ServerRequestContext;
import org.jboss.shrinkwrap.api.ShrinkWrap;
import org.jboss.shrinkwrap.api.spec.JavaArchive;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.test.QuarkusUnitTest;
import io.restassured.RestAssured;

public class MediaTypesWithSuffixHandlingTest {

@RegisterExtension
static QuarkusUnitTest testExtension = new QuarkusUnitTest()
.setArchiveProducer(() -> {
JavaArchive archive = ShrinkWrap.create(JavaArchive.class);
archive.addClasses(TestResource.class, NoSuffixMessageBodyWriter.class, SuffixMessageBodyWriter.class);
return archive;
});

@Test
public void testWriterWithoutSuffix() {
RestAssured.get("/test/writer/with-no-suffix")
.then()
.statusCode(200)
.body(Matchers.equalTo("result - no suffix writer"));
}

@Test
public void testReaderWithoutSuffix() {
RestAssured.get("/test/reader/with-no-suffix")
.then()
.statusCode(200)
.body(Matchers.equalTo("from reader - result"));
}

@Test
public void testWriterWithSuffix() {
RestAssured.get("/test/writer/with-suffix")
.then()
.statusCode(200)
.body(Matchers.equalTo("result - suffix writer"));
}

@Test
public void testReaderWithSuffix() {
RestAssured.get("/test/reader/with-suffix")
.then()
.statusCode(200)
.body(Matchers.equalTo("from reader suffix - result"));
}

@Path("/test")
public static class TestResource {

@GET
@Path("/writer/with-no-suffix")
@Produces("text/test")
public String writerSimple() {
return "result";
}

@GET
@Path("/writer/with-suffix")
@Produces("text/test+suffix")
public String writerSuffix() {
return "result";
}

@GET
@Path("/reader/with-no-suffix")
@Consumes("text/test")
public String readerSimple(Object fromReader) {
return fromReader + " - result";
}

@GET
@Path("/reader/with-suffix")
@Consumes("text/test+suffix")
public String readerSuffix(Object fromReader) {
return fromReader + " - result";
}
}

@Provider
@Consumes("text/test")
@Produces("text/test")
public static class NoSuffixMessageBodyWriter implements ServerMessageBodyWriter<Object>, ServerMessageBodyReader<Object> {

@Override
public boolean isWriteable(Class<?> type, Type genericType, ResteasyReactiveResourceInfo target, MediaType mediaType) {
return true;
}

@Override
public void writeResponse(Object o, Type genericType, ServerRequestContext context)
throws WebApplicationException, IOException {
String response = (String) o;
response += " - no suffix writer";
context.getOrCreateOutputStream().write(response.getBytes(StandardCharsets.UTF_8));
}

@Override
public boolean isWriteable(Class<?> type, Type genericType, Annotation[] annotations, MediaType mediaType) {
throw new IllegalStateException("should never have been called");
}

@Override
public void writeTo(Object o, Class<?> type, Type genericType, Annotation[] annotations, MediaType mediaType,
MultivaluedMap<String, Object> httpHeaders, OutputStream entityStream)
throws IOException, WebApplicationException {
throw new IllegalStateException("should never have been called");
}

@Override
public boolean isReadable(Class<?> type, Type genericType, ResteasyReactiveResourceInfo lazyMethod,
MediaType mediaType) {
return true;
}

@Override
public Object readFrom(Class<Object> type, Type genericType, MediaType mediaType,
ServerRequestContext context) throws WebApplicationException {
return "from reader";
}

@Override
public boolean isReadable(Class<?> aClass, Type type, Annotation[] annotations, MediaType mediaType) {
throw new IllegalStateException("should never have been called");
}

@Override
public Object readFrom(Class<Object> aClass, Type type, Annotation[] annotations, MediaType mediaType,
MultivaluedMap<String, String> multivaluedMap, InputStream inputStream)
throws WebApplicationException {
throw new IllegalStateException("should never have been called");
}
}

@Provider
@Consumes("text/test+suffix")
@Produces("text/test+suffix")
public static class SuffixMessageBodyWriter implements ServerMessageBodyWriter<Object>, ServerMessageBodyReader<Object> {

@Override
public boolean isWriteable(Class<?> type, Type genericType, ResteasyReactiveResourceInfo target, MediaType mediaType) {
return true;
}

@Override
public void writeResponse(Object o, Type genericType, ServerRequestContext context)
throws WebApplicationException, IOException {
String response = (String) o;
response += " - suffix writer";
context.getOrCreateOutputStream().write(response.getBytes(StandardCharsets.UTF_8));
}

@Override
public boolean isWriteable(Class<?> type, Type genericType, Annotation[] annotations, MediaType mediaType) {
throw new IllegalStateException("should never have been called");
}

@Override
public void writeTo(Object o, Class<?> type, Type genericType, Annotation[] annotations, MediaType mediaType,
MultivaluedMap<String, Object> httpHeaders, OutputStream entityStream)
throws IOException, WebApplicationException {
throw new IllegalStateException("should never have been called");
}

@Override
public boolean isReadable(Class<?> type, Type genericType, ResteasyReactiveResourceInfo lazyMethod,
MediaType mediaType) {
return true;
}

@Override
public Object readFrom(Class<Object> type, Type genericType, MediaType mediaType,
ServerRequestContext context) throws WebApplicationException {
return "from reader suffix";
}

@Override
public boolean isReadable(Class<?> aClass, Type type, Annotation[] annotations, MediaType mediaType) {
throw new IllegalStateException("should never have been called");
}

@Override
public Object readFrom(Class<Object> aClass, Type type, Annotation[] annotations, MediaType mediaType,
MultivaluedMap<String, String> multivaluedMap, InputStream inputStream)
throws WebApplicationException {
throw new IllegalStateException("should never have been called");
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
import org.jboss.resteasy.reactive.common.core.Serialisers;
import org.jboss.resteasy.reactive.common.jaxrs.ConfigurationImpl;
import org.jboss.resteasy.reactive.common.util.CaseInsensitiveMap;
import org.jboss.resteasy.reactive.common.util.MediaTypeHelper;

public class ClientReaderInterceptorContextImpl extends AbstractClientInterceptorContextImpl
implements ReaderInterceptorContext {
Expand All @@ -35,7 +34,7 @@ public ClientReaderInterceptorContextImpl(Annotation[] annotations, Class<?> ent
Map<String, Object> properties, MultivaluedMap<String, String> headers,
ConfigurationImpl configuration, Serialisers serialisers, InputStream inputStream,
ReaderInterceptor[] interceptors) {
super(annotations, entityClass, entityType, MediaTypeHelper.withSuffixAsSubtype(mediaType), properties);
super(annotations, entityClass, entityType, mediaType, properties);
this.configuration = configuration;
this.serialisers = serialisers;
this.inputStream = inputStream;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public List<MessageBodyReader<?>> findReaders(ConfigurationImpl configuration, C

public List<MessageBodyReader<?>> findReaders(ConfigurationImpl configuration, Class<?> entityType,
MediaType mediaType, RuntimeType runtimeType) {
List<MediaType> mt = Collections.singletonList(mediaType);
List<MediaType> desired = MediaTypeHelper.getUngroupedMediaTypes(mediaType);
List<MessageBodyReader<?>> ret = new ArrayList<>();
Deque<Class<?>> toProcess = new LinkedList<>();
Class<?> klass = entityType;
Expand All @@ -66,7 +66,7 @@ public List<MessageBodyReader<?>> findReaders(ConfigurationImpl configuration, C
while (!toProcess.isEmpty()) {
Class<?> iface = toProcess.poll();
List<ResourceReader> goodTypeReaders = readers.get(iface);
readerLookup(mediaType, runtimeType, mt, ret, goodTypeReaders);
readerLookup(mediaType, runtimeType, desired, ret, goodTypeReaders);
for (Class<?> i : iface.getInterfaces()) {
if (!seen.contains(i)) {
seen.add(i);
Expand All @@ -76,7 +76,7 @@ public List<MessageBodyReader<?>> findReaders(ConfigurationImpl configuration, C
}
}
List<ResourceReader> goodTypeReaders = readers.get(klass);
readerLookup(mediaType, runtimeType, mt, ret, goodTypeReaders);
readerLookup(mediaType, runtimeType, desired, ret, goodTypeReaders);
if (klass.isInterface()) {
klass = Object.class;
} else {
Expand All @@ -87,7 +87,8 @@ public List<MessageBodyReader<?>> findReaders(ConfigurationImpl configuration, C
return ret;
}

private void readerLookup(MediaType mediaType, RuntimeType runtimeType, List<MediaType> mt, List<MessageBodyReader<?>> ret,
private void readerLookup(MediaType mediaType, RuntimeType runtimeType, List<MediaType> desired,
List<MessageBodyReader<?>> ret,
List<ResourceReader> goodTypeReaders) {
if (goodTypeReaders != null && !goodTypeReaders.isEmpty()) {
List<ResourceReader> mediaTypeMatchingReaders = new ArrayList<>(goodTypeReaders.size());
Expand All @@ -96,13 +97,15 @@ private void readerLookup(MediaType mediaType, RuntimeType runtimeType, List<Med
if (!goodTypeReader.matchesRuntimeType(runtimeType)) {
continue;
}
MediaType match = MediaTypeHelper.getFirstMatch(mt, goodTypeReader.mediaTypes());
MediaType match = MediaTypeHelper.getFirstMatch(desired, goodTypeReader.mediaTypes());
if (match != null || mediaType == null) {
mediaTypeMatchingReaders.add(goodTypeReader);
}
}
mediaTypeMatchingReaders.sort(ResourceReader.ResourceReaderComparator.INSTANCE);
for (ResourceReader mediaTypeMatchingReader : mediaTypeMatchingReaders) {

mediaTypeMatchingReaders.sort(new ResourceReader.ResourceReaderComparator(Collections.singletonList(mediaType)));
for (int i = 0; i < mediaTypeMatchingReaders.size(); i++) {
ResourceReader mediaTypeMatchingReader = mediaTypeMatchingReaders.get(i);
ret.add(mediaTypeMatchingReader.instance());
}
}
Expand Down Expand Up @@ -151,17 +154,19 @@ public List<MessageBodyWriter<?>> findBuildTimeWriters(Class<?> entityType, Runt

protected List<ResourceWriter> findResourceWriters(QuarkusMultivaluedMap<Class<?>, ResourceWriter> writers, Class<?> klass,
List<MediaType> produces, RuntimeType runtimeType) {
Class<?> currentClass = klass;
List<MediaType> desired = MediaTypeHelper.getUngroupedMediaTypes(produces);
List<ResourceWriter> ret = new ArrayList<>();
Deque<Class<?>> toProcess = new LinkedList<>();
do {
if (klass == Object.class) {
if (currentClass == Object.class) {
//spec extension, look for interfaces as well
//we match interfaces before Object
Set<Class<?>> seen = new HashSet<>(toProcess);
while (!toProcess.isEmpty()) {
Class<?> iface = toProcess.poll();
List<ResourceWriter> goodTypeWriters = writers.get(iface);
writerLookup(runtimeType, produces, ret, goodTypeWriters);
writerLookup(runtimeType, produces, desired, ret, goodTypeWriters);
for (Class<?> i : iface.getInterfaces()) {
if (!seen.contains(i)) {
seen.add(i);
Expand All @@ -170,15 +175,16 @@ protected List<ResourceWriter> findResourceWriters(QuarkusMultivaluedMap<Class<?
}
}
}
List<ResourceWriter> goodTypeWriters = writers.get(klass);
writerLookup(runtimeType, produces, ret, goodTypeWriters);
toProcess.addAll(Arrays.asList(klass.getInterfaces()));
List<ResourceWriter> goodTypeWriters = writers.get(currentClass);
writerLookup(runtimeType, produces, desired, ret, goodTypeWriters);
toProcess.addAll(Arrays.asList(currentClass.getInterfaces()));
// if we're an interface, pretend our superclass is Object to get us through the same logic as a class
if (klass.isInterface())
klass = Object.class;
else
klass = klass.getSuperclass();
} while (klass != null);
if (currentClass.isInterface()) {
currentClass = Object.class;
} else {
currentClass = currentClass.getSuperclass();
}
} while (currentClass != null);

return ret;
}
Expand All @@ -200,22 +206,25 @@ protected List<MessageBodyWriter<?>> toMessageBodyWriters(List<ResourceWriter> r
return ret;
}

private void writerLookup(RuntimeType runtimeType, List<MediaType> mt, List<ResourceWriter> ret,
List<ResourceWriter> goodTypeWriters) {
private void writerLookup(RuntimeType runtimeType, List<MediaType> produces, List<MediaType> desired,
List<ResourceWriter> ret, List<ResourceWriter> goodTypeWriters) {
if (goodTypeWriters != null && !goodTypeWriters.isEmpty()) {
List<ResourceWriter> mediaTypeMatchingWriters = new ArrayList<>(goodTypeWriters.size());

for (int i = 0; i < goodTypeWriters.size(); i++) {
ResourceWriter goodTypeWriter = goodTypeWriters.get(i);
if (!goodTypeWriter.matchesRuntimeType(runtimeType)) {
continue;
}
MediaType match = MediaTypeHelper.getFirstMatch(mt, goodTypeWriter.mediaTypes());
MediaType match = MediaTypeHelper.getFirstMatch(desired, goodTypeWriter.mediaTypes());
if (match != null) {
mediaTypeMatchingWriters.add(goodTypeWriter);
}
}

// we sort here because the spec mentions that the writers closer to the requested java type are tried first
mediaTypeMatchingWriters.sort(ResourceWriter.ResourceWriterComparator.INSTANCE);
mediaTypeMatchingWriters.sort(new ResourceWriter.ResourceWriterComparator(produces));

ret.addAll(mediaTypeMatchingWriters);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,11 @@ public boolean matchesRuntimeType(RuntimeType runtimeType) {
*/
public static class ResourceReaderComparator implements Comparator<ResourceReader> {

public static final ResourceReaderComparator INSTANCE = new ResourceReaderComparator();
private final List<MediaType> produces;

public ResourceReaderComparator(List<MediaType> produces) {
this.produces = produces;
}

@Override
public int compare(ResourceReader o1, ResourceReader o2) {
Expand Down Expand Up @@ -144,6 +148,14 @@ public int compare(ResourceReader o1, ResourceReader o2) {
return mediaTypeCompare;
}

// try to compare using the number of matching produces media types
if (!produces.isEmpty()) {
mediaTypeCompare = MediaTypeHelper.compareMatchingMediaTypes(produces, mediaTypes1, mediaTypes2);
if (mediaTypeCompare != 0) {
return mediaTypeCompare;
}
}

// done to make the sorting result deterministic
return Integer.compare(mediaTypes1.size(), mediaTypes2.size());
}
Expand Down
Loading

0 comments on commit 5407ac5

Please sign in to comment.