Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change default media type to JSON #12566

Merged
merged 1 commit into from
Oct 13, 2020

Conversation

stuartwdouglas
Copy link
Member

This adds a new RESTEasy provider that will produce
JSON by default. This removes the need to add
@produces(MediaType.APPLICATION_JSON) and
@consumes(MediaType.APPLICATION_JSON) everywhere.

The old behaviour can be retained by setting:

quarkus.resteasy-json.default-json=false

in application.properties.

Copy link
Member

@FroMage FroMage left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great idea!

@stuartwdouglas stuartwdouglas force-pushed the defaut-json branch 2 times, most recently from 81122f3 to 18456a9 Compare October 7, 2020 23:30
@stuartwdouglas stuartwdouglas force-pushed the defaut-json branch 5 times, most recently from 6dd7000 to ed67977 Compare October 9, 2020 04:51
gsmet
gsmet previously requested changes Oct 12, 2020
Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like I forgot to post my review... You added the Jackson impl so that's one less thing to add but there are still a few details that needs fixing.

I can do it if you have better things to do, just ping me!

@Produces(MediaType.WILDCARD)
@Consumes(MediaType.WILDCARD)
@Priority(Priorities.USER - 200)
public class QuarkusJSONBSerializer extends JsonBindingProvider {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be called QuarkusJsonbSerializer for consistency with other classes.

@@ -0,0 +1,77 @@
package io.quarkus.resteasy.jsonb;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not API so it should be in the runtime package.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And same for the Jackson one.

This adds a new RESTEasy provider that will produce
JSON by default. This removes the need to add
@produces(MediaType.APPLICATION_JSON) and
@consumes(MediaType.APPLICATION_JSON) everywhere.

The old behaviour can be retained by setting:

quarkus.resteasy-json.default-json=false

in application.properties.
@stuartwdouglas
Copy link
Member Author

Done

@stuartwdouglas stuartwdouglas requested a review from gsmet October 13, 2020 00:13
@gastaldi gastaldi dismissed gsmet’s stale review October 13, 2020 20:10

Changes addressed

@gastaldi gastaldi merged commit 151c461 into quarkusio:master Oct 13, 2020
@gastaldi gastaldi added this to the 1.10 - master milestone Oct 13, 2020
@gastaldi
Copy link
Contributor

@stuartwdouglas apparently this PR does not support this:

    @GET
    public Map<String,String> hello() {
        Map<String,String> t = new HashMap<>();
        t.put("Hey", "Ho");
        return t;
    }

I get the following error while hitting the endpoint:

2020-10-14 00:15:35,726 ERROR [org.jbo.res.res.i18n] (executor-thread-1) RESTEASY002005: Failed executing GET /greeting: org.jboss.resteasy.core.NoMessageBodyWriterFoundFailure: Could not find MessageBodyWriter for response object of type: java.util.HashMap of media type: application/octet-stream
	at org.jboss.resteasy.core.ServerResponseWriter.lambda$writeNomapResponse$3(ServerResponseWriter.java:125)
	at org.jboss.resteasy.core.interception.jaxrs.ContainerResponseContextImpl.filter(ContainerResponseContextImpl.java:404)
	at org.jboss.resteasy.core.ServerResponseWriter.executeFilters(ServerResponseWriter.java:252)
	at org.jboss.resteasy.core.ServerResponseWriter.writeNomapResponse(ServerResponseWriter.java:101)
	at org.jboss.resteasy.core.ServerResponseWriter.writeNomapResponse(ServerResponseWriter.java:74)
	at org.jboss.resteasy.core.SynchronousDispatcher.writeResponse(SynchronousDispatcher.java:594)
	at org.jboss.resteasy.core.SynchronousDispatcher.invoke(SynchronousDispatcher.java:524)
	at org.jboss.resteasy.core.SynchronousDispatcher.lambda$invoke$4(SynchronousDispatcher.java:261)
	at org.jboss.resteasy.core.SynchronousDispatcher.lambda$preprocess$0(SynchronousDispatcher.java:161)
	at org.jboss.resteasy.core.interception.jaxrs.PreMatchContainerRequestContext.filter(PreMatchContainerRequestContext.java:364)
	at org.jboss.resteasy.core.SynchronousDispatcher.preprocess(SynchronousDispatcher.java:164)
	at org.jboss.resteasy.core.SynchronousDispatcher.invoke(SynchronousDispatcher.java:247)
	at io.quarkus.resteasy.runtime.standalone.RequestDispatcher.service(RequestDispatcher.java:73)
	at io.quarkus.resteasy.runtime.standalone.VertxRequestHandler.dispatch(VertxRequestHandler.java:131)
	at io.quarkus.resteasy.runtime.standalone.VertxRequestHandler.access$000(VertxRequestHandler.java:37)
	at io.quarkus.resteasy.runtime.standalone.VertxRequestHandler$1.run(VertxRequestHandler.java:94)
	at io.quarkus.runtime.CleanableExecutor$CleaningRunnable.run(CleanableExecutor.java:231)
	at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at org.jboss.threads.ContextClassLoaderSavingRunnable.run(ContextClassLoaderSavingRunnable.java:35)
	at org.jboss.threads.EnhancedQueueExecutor.safeRun(EnhancedQueueExecutor.java:2046)
	at org.jboss.threads.EnhancedQueueExecutor$ThreadBody.doRunTask(EnhancedQueueExecutor.java:1578)
	at org.jboss.threads.EnhancedQueueExecutor$ThreadBody.run(EnhancedQueueExecutor.java:1452)
	at org.jboss.threads.DelegatingRunnable.run(DelegatingRunnable.java:29)
	at org.jboss.threads.ThreadLocalResettingRunnable.run(ThreadLocalResettingRunnable.java:29)
	at java.base/java.lang.Thread.run(Thread.java:834)
	at org.jboss.threads.JBossThread.run(JBossThread.java:479)

@gastaldi
Copy link
Contributor

I created #12701

@gastaldi
Copy link
Contributor

Ignore it, I forgot to add quarkus-resteasy-jackson to my sample project

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants