armeria: Unable to use custom JsonFormat.Printer for ProtobufResponseConverterFunction

Let’s say we have the following annotated service:

class MyService {
  @ProduceJson
  fun flowProtobufJson(...): Flow<MyProtobuf> {
      ...
  }

}

Server.buider()
      .annotatedService()
      .responseConverters(MyProtobufResponseConverter())
      .build(MyService())

We need two 2 response converters to completely convert Flow<MyProtobuf> into a JSON response. First, MyProtobufResponseConverter will be used to convert each MyProtobuf into a JSON object. Next, FlowResponseConverterFunction should be chosen to collect the JSON objects and convert them into a JSON array.

Currently, we provide two APIs for a custom response converter.

• ResponseConverterFunction defines how to convert an object.
• ResponseConverterFunctionProvider defines how to create a ResponseConverterFunction via SPI.

The downside of the APIs is that they don’t tell a ResponseConverterFunction is a primitive converter or a delegating ResponseConverterFunction. It is difficult to prioritize between them. So we’d like to change our API to show their explicit behavior. Note: As the API is @UnstableApi, we can still make a breaking change. 😅

Proposal:

• Split the original ResponseConverterFunctionProvider to two providers. DelegatingResponseConverterFunctionProvider requires a delegate to complete to convert a response like FlowResponseConverterFunction and the new ResponseConverterFunctionProvider does not take any parameters.
• Add ResponseConverterFunctionProvider that create a primitive ResponseConverterFunction which converters an object to an HttpResponse without depending on other ResponseConverterFunctions.
• A response converter created from DelegatingResponseConverterFunctionProvider has higher priority than primitive converters. Because it needs primitive converters to create a delegating response converter.

public interface ResponseConverterFunctionProvider {
    @Nullable
    ResponseConverterFunction newResponseConverterFunction();
}

public interface DelegatingResponseConverterFunctionProvider {

    @Nullable
    ResponseConverterFunction createResponseConverterFunction(
         Type responseType,
         ResponseConverterFunction responseConverter);
}

Just for clarification:

Change the ResponseConverterFunctionProvider to use the configured responseConverterFunctions or to be applied after the responseConverterFunctions

By this, I think we mean that we will define the behavior of armeria’s default ResponseConverterFunctionProvider implementation such that it returns a function:

1. Try to apply the passed responseConverterFunction
2. If the passed responseConverterFunction falls through, the ResponseConverterFunctionProvider will try to apply its own ResponseConverterFunction

so I guess the PR would look roughly like follows (the same would be applied to ScalaPbResponseConverterFunctionProvider):

public final class ProtobufResponseConverterFunctionProvider implements ResponseConverterFunctionProvider {
...
    public ResponseConverterFunction createResponseConverterFunction(
            Type returnType,
            ResponseConverterFunction responseConverter) {
        if (isSupportedType(returnType)) {
            return new responseConverter.orElse(ProtobufResponseConverterFunction);
        }
...

I wanted to make sure I also understood @minwoox 's comments correctly 😅

Ah, I just meant I didn’t write a test case for the exact scenario you mentioned for Flow<MyProtobuf> so I can’t confirm that it’s actually working

Had a chat with the team and we realized:

• ResponseConverterFunctionProvider(found via SPI) uses the responseConverterFunctions set via annotation, service builder and default ones.
• So the relationship between ResponseConverterFunctionProvider and others is not about which one is used beforehand.
  • It’s just that ResponseConverterFunctionProvider uses the configured responseConverterFunctions.

So here’s what we need to do:

• Change the ResponseConverterFunctionProviders that Armeria provides to use the configured responseConverterFunctions or to be applied after the responseConverterFunctions
  • so that users are always able to use the ResponseConverterFunction they want via setter
  • ProtobufResponseConverterFunctionProvider and ScalaPbResponseConverterFunctionProvider should be fixed to abide by the rule.
• Add ResponseConverterFunction.orElse(responseConverterFunction) for easy chaining.
  • So we can just do return responseConverter.orElse(new ProtobufResponseConverterFunction()); here
• Update the Javadoc of ResponseConverterFunctionProvider.createResponseConverterFunction(...) to inform the ResponseConverterFunctions parameter.

To be precise, I think ResponseConverterFunctions should be applied in the following order:

• Annotated ResponseConverterFunction
• Set via ServerBuilder.annotatedService() and equivalent
• Found via SPI
• Default ResponseConverterFunctions

Will give it a shot 👍