Stop double encoding body when we have a response encoder

http-server-jetty/src/main/java/io/micronaut/servlet/jetty/JettyFactory.java

@@ -38,6 +38,9 @@
 import org.eclipse.jetty.util.ssl.SslContextFactory;
 
 import jakarta.inject.Singleton;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 import java.io.IOException;
 import java.util.List;
 import java.util.function.Function;
@@ -54,6 +57,9 @@
 public class JettyFactory extends ServletServerFactory {
 
     public static final String RESOURCE_BASE = "resourceBase";
+
+    private static final Logger LOG = LoggerFactory.getLogger(JettyFactory.class);
+
     private final JettyConfiguration jettyConfiguration;
 
     /**
@@ -158,7 +164,12 @@ public Resource newResource(String urlOrPath) throws IOException {
                 servletHolder,
                 configuration.getMapping()
         );
-        servletHolder.setAsyncSupported(true);
+
+        Boolean isAsync = applicationContext.getEnvironment().getProperty("micronaut.server.testing.async", Boolean.class, true);
+        if (Boolean.FALSE.equals(isAsync)) {
+            LOG.warn("Async support disabled for testing purposes.");
+        }
+        servletHolder.setAsyncSupported(isAsync);
 
         configuration.getMultipartConfigElement().ifPresent(multipartConfiguration ->
                 servletHolder.getRegistration().setMultipartConfig(multipartConfiguration)

http-server-jetty/src/test/groovy/io/micronaut/servlet/jetty/JettyResponseEncoderSpec.groovy

@@ -0,0 +1,79 @@
+package io.micronaut.servlet.jetty
+
+import groovy.transform.Canonical
+import io.micronaut.context.annotation.Property
+import io.micronaut.context.annotation.Requires
+import io.micronaut.core.annotation.AnnotationMetadata
+import io.micronaut.core.annotation.Introspected
+import io.micronaut.core.annotation.NonNull
+import io.micronaut.http.MutableHttpResponse
+import io.micronaut.http.annotation.Controller
+import io.micronaut.http.annotation.Get
+import io.micronaut.http.client.HttpClient
+import io.micronaut.http.client.annotation.Client
+import io.micronaut.servlet.http.ServletExchange
+import io.micronaut.servlet.http.ServletHttpResponse
+import io.micronaut.servlet.http.ServletResponseEncoder
+import io.micronaut.test.extensions.spock.annotation.MicronautTest
+import jakarta.inject.Inject
+import jakarta.inject.Singleton
+import org.reactivestreams.Publisher
+import reactor.core.publisher.Flux
+import spock.lang.Specification
+
+@MicronautTest
+@Property(name = "spec.name", value = SPEC_NAME)
+@Property(name = "micronaut.server.testing.async", value = "false")
+class JettyResponseEncoderSpec extends Specification {
+
+    private static final String SPEC_NAME = "JettyResponseEncoderSpec"
+
+    @Inject
+    @Client("/")
+    HttpClient client
+
+    void "custom encoder applied once"() {
+        when:
+        def response = client.toBlocking().exchange("/test", String)
+
+        then:
+        response.body() == "SRE{bar}"
+    }
+
+    @Requires(property = "spec.name", value = SPEC_NAME)
+    @Controller
+    static class TestController {
+
+        @Get("/test")
+        SomeResponseType test() {
+            new SomeResponseType(foo: "bar")
+        }
+    }
+
+    @Canonical
+    @Introspected
+    static class SomeResponseType {
+        String foo
+
+        @Override
+        String toString() {
+            "NOPE!"
+        }
+    }
+
+    @Requires(property = "spec.name", value = SPEC_NAME)
+    @Singleton
+    static class SomeResponseEncoder implements ServletResponseEncoder<SomeResponseType> {
+        @Override
+        Class<SomeResponseType> getResponseType() {
+            return SomeResponseType.class
+        }
+
+        @Override
+        Publisher<MutableHttpResponse<?>> encode(@NonNull ServletExchange<?, ?> exchange, AnnotationMetadata annotationMetadata, @NonNull SomeResponseType value) {
+            ServletHttpResponse<?, ?> response = exchange.getResponse().contentType("text/plain")
+            response.getOutputStream() << "SRE{$value.foo}"
+            Flux.just(response)
+        }
+    }
+}

http-server-tomcat/src/main/java/io/micronaut/servlet/tomcat/TomcatFactory.java

@@ -36,6 +36,8 @@
 import org.apache.catalina.startup.Tomcat;
 import org.apache.tomcat.util.net.SSLHostConfig;
 import org.apache.tomcat.util.net.SSLHostConfigCertificate;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * Factory for the {@link Tomcat} instance.
@@ -46,6 +48,8 @@
 @Factory
 public class TomcatFactory extends ServletServerFactory {
 
+    private static final Logger LOG = LoggerFactory.getLogger(TomcatFactory.class);
+
     /**
      * Default constructor.
      *
@@ -101,7 +105,12 @@ protected Tomcat tomcatServer(Connector connector, MicronautServletConfiguration
                 configuration.getName(),
                 new DefaultMicronautServlet(getApplicationContext())
         );
-        servlet.setAsyncSupported(true);
+
+        Boolean isAsync = getApplicationContext().getEnvironment().getProperty("micronaut.server.testing.async", Boolean.class, true);
+        if (Boolean.FALSE.equals(isAsync)) {
+            LOG.warn("Async support disabled for testing purposes.");
+        }
+        servlet.setAsyncSupported(isAsync);
         servlet.addMapping(configuration.getMapping());
         getStaticResourceConfigurations().forEach(config ->
             servlet.addMapping(config.getMapping())

http-server-tomcat/src/test/groovy/io/micronaut/servlet/tomcat/TomcatResponseEncoderSpec.groovy

@@ -0,0 +1,79 @@
+package io.micronaut.servlet.tomcat
+
+import groovy.transform.Canonical
+import io.micronaut.context.annotation.Property
+import io.micronaut.context.annotation.Requires
+import io.micronaut.core.annotation.AnnotationMetadata
+import io.micronaut.core.annotation.Introspected
+import io.micronaut.core.annotation.NonNull
+import io.micronaut.http.MutableHttpResponse
+import io.micronaut.http.annotation.Controller
+import io.micronaut.http.annotation.Get
+import io.micronaut.http.client.HttpClient
+import io.micronaut.http.client.annotation.Client
+import io.micronaut.servlet.http.ServletExchange
+import io.micronaut.servlet.http.ServletHttpResponse
+import io.micronaut.servlet.http.ServletResponseEncoder
+import io.micronaut.test.extensions.spock.annotation.MicronautTest
+import jakarta.inject.Inject
+import jakarta.inject.Singleton
+import org.reactivestreams.Publisher
+import reactor.core.publisher.Flux
+import spock.lang.Specification
+
+@MicronautTest
+@Property(name = "spec.name", value = SPEC_NAME)
+@Property(name = "micronaut.server.testing.async", value = "false")
+class TomcatResponseEncoderSpec extends Specification {
+
+    private static final String SPEC_NAME = "JettyResponseEncoderSpec"
+
+    @Inject
+    @Client("/")
+    HttpClient client
+
+    void "custom encoder applied once"() {
+        when:
+        def response = client.toBlocking().exchange("/test", String)
+
+        then:
+        response.body() == "SRE{bar}"
+    }
+
+    @Requires(property = "spec.name", value = SPEC_NAME)
+    @Controller
+    static class TestController {
+
+        @Get("/test")
+        SomeResponseType test() {
+            new SomeResponseType(foo: "bar")
+        }
+    }
+
+    @Canonical
+    @Introspected
+    static class SomeResponseType {
+        String foo
+
+        @Override
+        String toString() {
+            "NOPE!"
+        }
+    }
+
+    @Requires(property = "spec.name", value = SPEC_NAME)
+    @Singleton
+    static class SomeResponseEncoder implements ServletResponseEncoder<SomeResponseType> {
+        @Override
+        Class<SomeResponseType> getResponseType() {
+            return SomeResponseType.class
+        }
+
+        @Override
+        Publisher<MutableHttpResponse<?>> encode(@NonNull ServletExchange<?, ?> exchange, AnnotationMetadata annotationMetadata, @NonNull SomeResponseType value) {
+            ServletHttpResponse<?, ?> response = exchange.getResponse().contentType("text/plain")
+            response.getOutputStream() << "SRE{$value.foo}"
+            Flux.just(response)
+        }
+    }
+}

http-server-undertow/src/main/java/io/micronaut/servlet/undertow/UndertowFactory.java

@@ -34,6 +34,8 @@
 import io.undertow.server.handlers.PathHandler;
 import io.undertow.servlet.Servlets;
 import io.undertow.servlet.api.*;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.xnio.Option;
 import org.xnio.Options;
 
@@ -52,6 +54,8 @@
 @Factory
 public class UndertowFactory extends ServletServerFactory {
 
+    private static final Logger LOG = LoggerFactory.getLogger(UndertowFactory.class);
+
     private final UndertowConfiguration configuration;
 
     /**
@@ -218,7 +222,11 @@ public void release() {
                     }
                 }
         );
-        servletInfo.setAsyncSupported(true);
+        Boolean isAsync = getApplicationContext().getEnvironment().getProperty("micronaut.server.testing.async", Boolean.class, true);
+        if (Boolean.FALSE.equals(isAsync)) {
+            LOG.warn("Async support disabled for testing purposes.");
+        }
+        servletInfo.setAsyncSupported(isAsync);
         servletInfo.addMapping(servletConfiguration.getMapping());
         getStaticResourceConfigurations().forEach(config -> {
             servletInfo.addMapping(config.getMapping());

http-server-undertow/src/test/groovy/io/micronaut/servlet/undertow/UndertowResponseEncoderSpec.groovy

@@ -0,0 +1,79 @@
+package io.micronaut.servlet.undertow
+
+import groovy.transform.Canonical
+import io.micronaut.context.annotation.Property
+import io.micronaut.context.annotation.Requires
+import io.micronaut.core.annotation.AnnotationMetadata
+import io.micronaut.core.annotation.Introspected
+import io.micronaut.core.annotation.NonNull
+import io.micronaut.http.MutableHttpResponse
+import io.micronaut.http.annotation.Controller
+import io.micronaut.http.annotation.Get
+import io.micronaut.http.client.HttpClient
+import io.micronaut.http.client.annotation.Client
+import io.micronaut.servlet.http.ServletExchange
+import io.micronaut.servlet.http.ServletHttpResponse
+import io.micronaut.servlet.http.ServletResponseEncoder
+import io.micronaut.test.extensions.spock.annotation.MicronautTest
+import jakarta.inject.Inject
+import jakarta.inject.Singleton
+import org.reactivestreams.Publisher
+import reactor.core.publisher.Flux
+import spock.lang.Specification
+
+@MicronautTest
+@Property(name = "spec.name", value = SPEC_NAME)
+@Property(name = "micronaut.server.testing.async", value = "false")
+class UndertowResponseEncoderSpec extends Specification {
+
+    private static final String SPEC_NAME = "JettyResponseEncoderSpec"
+
+    @Inject
+    @Client("/")
+    HttpClient client
+
+    void "custom encoder applied once"() {
+        when:
+        def response = client.toBlocking().exchange("/test", String)
+
+        then:
+        response.body() == "SRE{bar}"
+    }
+
+    @Requires(property = "spec.name", value = SPEC_NAME)
+    @Controller
+    static class TestController {
+
+        @Get("/test")
+        SomeResponseType test() {
+            new SomeResponseType(foo: "bar")
+        }
+    }
+
+    @Canonical
+    @Introspected
+    static class SomeResponseType {
+        String foo
+
+        @Override
+        String toString() {
+            "NOPE!"
+        }
+    }
+
+    @Requires(property = "spec.name", value = SPEC_NAME)
+    @Singleton
+    static class SomeResponseEncoder implements ServletResponseEncoder<SomeResponseType> {
+        @Override
+        Class<SomeResponseType> getResponseType() {
+            return SomeResponseType.class
+        }
+
+        @Override
+        Publisher<MutableHttpResponse<?>> encode(@NonNull ServletExchange<?, ?> exchange, AnnotationMetadata annotationMetadata, @NonNull SomeResponseType value) {
+            ServletHttpResponse<?, ?> response = exchange.getResponse().contentType("text/plain")
+            response.getOutputStream() << "SRE{$value.foo}"
+            Flux.just(response)
+        }
+    }
+}

servlet-core/src/main/java/io/micronaut/servlet/http/ServletHttpHandler.java

@@ -363,15 +363,14 @@ private void encodeResponse(ServletExchange<REQ, RES> exchange,
                 if (exchange.getRequest().isAsyncSupported()) {
                     Flux.from(responseEncoder.encode(exchange, routeAnnotationMetadata, body))
                         .subscribe(responsePublisherCallback);
-                    return;
                 } else {
                     // NOTE[moss]: blockLast() here *was* subscribe(), but that returns immediately, which was
                     // sometimes allowing the main response publisher to complete before this responseEncoder
                     // could fill out the response! Blocking here will ensure that the response is filled out
                     // before the main response publisher completes. This will be improved later to avoid the block.
                     Flux.from(responseEncoder.encode(exchange, routeAnnotationMetadata, body)).blockLast();
-                    // Continue blocking execution
                 }
+                return;
             }
 
             MediaType mediaType = response.getContentType().orElse(null);