internal.HttpMessageEncoder should consider the Route produces for determining MediaType

[agentgt]

The message encoder resolution based on MediaType is slightly flawed and disregards the route produces media type.

If you have a custom MessageEncoder for say text/html and you are using the Jackson encoders as well it very difficult to enforce the Jackson encoder gets run even if the route can only deliver JSON.

Lets say my custom Encoder is this:

public class ThemeResultEncoder implements MessageEncoder {

    @Override
    public byte[] encode(
            Context ctx,
            Object value)
            throws Exception {
        if (value instanceof ThemeResult r) {
            ctx.setDefaultResponseType(MediaType.html);
            return r.html()
                .getBytes(StandardCharsets.UTF_8);
        }
        return null;
    }

}

(btw I thought that returning null would make it go to the next encoder but I was wrong).

I register my encoders like:

jooby.encoder(MediaType.html, themeResultEncoder)
jooby.encoder(MediaType.json, jacksonEncoder);

Lets say I have a controller

public Controller {
    
    @GET(value= {"/json"}, produces = MediaType.json) // N.B. the produces
    public Map<String,Object> get() {
        return someMap;
    }
}

If I hit the /json controller with say a web browser it will not use the Jackson Encoder but the the custom Theme Encoder (since its first). Since the custom theme encoder cannot handle it we get a 500 with response not encoded.

Now I could register the Jackson encoders first but you have a similar problem in that JSON will be produced for ThemeResult return types if the MediaType isn't passed by the user agent.

The Route produces if specified should trump whatever the User agent (browser) requests.

Regardless it just seems wrong any user agent that doesn't pass the correct accept type can kick off a 500 even if I specify that the controller can only produce say JSON.

A workaround is to make your own aggregate Encoder that is the only one registered and thus does the proper resolution but that isn't ideal if you want to use other people's extensions.

[agentgt]

@jknack I was going to put in PR but determined there are several ways to solve this problem.

For one I really wish the MessageEncoders could say "I cant encode this... use the next encoder". That is why I thought returning null would do this. Unfortunately returning null cannot be used to indicate that since it might mean I have handled the request via the Context.

One option though is to check if isResponseStarted()

e.g.

for (MessageEncoder e : encoders) {
    byte[] b = e.encode(...);
    if (b == null && ! ctx.isResponseStarted()) {
        continue;
    }
    return b;
}

Or we could make a special public static final byte[] NEXT_ENCODER; as a return value.

Hence why I wanted to talk about it before implementing a solution.

[agentgt]

Here is a work around for others:

class MediaTypeProducesMessageEncoder implements MessageEncoder {

    private Map<MediaType, MessageEncoder> encoders = new LinkedHashMap<>();

    @SuppressWarnings("null")
    @Override
    public byte @Nullable [] encode(
            Context ctx,
            Object value)
            throws Exception {
        List<MediaType> produces = ctx.getRoute()
            .getProduces();
        MessageEncoder foundEncoder = null;
        if (!produces.isEmpty()) {
            MediaType mediaType = ctx.accept(produces);
            // This next condition disregards the user agents accept
            // This is debatable
            if (mediaType == null) {
                mediaType = produces.get(0);
            }
            foundEncoder = encoders.get(mediaType);
        }
        if (foundEncoder == null) {
            // This is the default behavior in HttpMessageEncoder
            MediaType mediaType = ctx.accept(new ArrayList<>(encoders.keySet()));
            foundEncoder = encoders.getOrDefault(mediaType, TO_STRING);
        }
        return foundEncoder.encode(ctx, value);
    }

    public void add(
            MediaType contentType,
            MessageEncoder encoder) {
        encoders.put(contentType, encoder);
    }

}

Then you subclass Jooby:

public class MyJooby {
    private final MediaTypeProducesMessageEncoder encoder = new MediaTypeProducesMessageEncoder();

    public MyJooby() {
       setEncoder(encoder);
    }

     @Override
    public MyJooby encoder(
            MessageEncoder encoder) {
        this.encoder.add(MediaType.all, encoder);
        return this;
    }

    @Override
    public MyJooby encoder(
            MediaType contentType,
            MessageEncoder encoder) {
        this.encoder.add(contentType, encoder);
        return this;
    }
}