responseCode is ignored from controllers returning kotlin.Unit #1545

Second try. 204 is set as default response for void/unit DELETE routes

Author: jknack

modules/jooby-apt/src/main/java/io/jooby/internal/apt/HandlerCompiler.java

@@ -42,6 +42,7 @@
 import static org.objectweb.asm.Opcodes.ASTORE;
 import static org.objectweb.asm.Opcodes.CHECKCAST;
 import static org.objectweb.asm.Opcodes.GETSTATIC;
+import static org.objectweb.asm.Opcodes.IFEQ;
 import static org.objectweb.asm.Opcodes.INVOKEINTERFACE;
 import static org.objectweb.asm.Opcodes.INVOKESTATIC;
 import static org.objectweb.asm.Opcodes.INVOKEVIRTUAL;
@@ -222,14 +223,14 @@ private void processArguments(ClassWriter classWriter, MethodVisitor visitor,
 
   private void setDefaultResponseType(MethodVisitor visitor) throws Exception {
     TypeKind kind = executable.getReturnType().getKind();
-    if (kind == TypeKind.VOID) {
+    if (kind == TypeKind.VOID && getHttpMethod().equalsIgnoreCase(Router.DELETE)) {
       visitor.visitVarInsn(ALOAD, 1);
       visitor
           .visitFieldInsn(GETSTATIC, STATUS_CODE.getInternalName(), "NO_CONTENT",
               STATUS_CODE.getDescriptor());
-      Method sendStatusCode = Context.class.getDeclaredMethod("send", StatusCode.class);
-      visitor.visitMethodInsn(INVOKEINTERFACE, CTX.getInternalName(), sendStatusCode.getName(),
-          getMethodDescriptor(sendStatusCode), true);
+      Method setResponseCode = Context.class.getDeclaredMethod("setResponseCode", StatusCode.class);
+      visitor.visitMethodInsn(INVOKEINTERFACE, CTX.getInternalName(), setResponseCode.getName(),
+          getMethodDescriptor(setResponseCode), true);
       visitor.visitInsn(POP);
     }
   }
@@ -238,6 +239,24 @@ private void processReturnType(MethodVisitor visitor) throws Exception {
     TypeKind kind = executable.getReturnType().getKind();
     if (kind == TypeKind.VOID) {
       visitor.visitVarInsn(ALOAD, 1);
+      Method isResponseStarted = Context.class.getDeclaredMethod("isResponseStarted");
+      visitor.visitMethodInsn(INVOKEINTERFACE, CTX.getInternalName(), isResponseStarted.getName(),
+          getMethodDescriptor(isResponseStarted), true);
+      Label label0 = new Label();
+      visitor.visitJumpInsn(IFEQ, label0);
+      visitor.visitVarInsn(ALOAD, 1);
+      visitor.visitInsn(ARETURN);
+      visitor.visitLabel(label0);
+      visitor.visitFrame(Opcodes.F_SAME, 0, null, 0, null);
+
+      visitor.visitVarInsn(ALOAD, 1);
+      visitor.visitVarInsn(ALOAD, 1);
+      Method getResponseCode = Context.class.getDeclaredMethod("getResponseCode");
+      visitor.visitMethodInsn(INVOKEINTERFACE, CTX.getInternalName(), getResponseCode.getName(),
+          getMethodDescriptor(getResponseCode), true);
+      Method sendStatusCode = Context.class.getDeclaredMethod("send", StatusCode.class);
+      visitor.visitMethodInsn(INVOKEINTERFACE, CTX.getInternalName(), sendStatusCode.getName(),
+          getMethodDescriptor(sendStatusCode), true);
     } else {
       Method wrapper = Primitives.wrapper(kind);
       if (wrapper == null) {

modules/jooby-apt/src/test/java/output/MyControllerHandler.java

@@ -22,6 +22,10 @@ public MyControllerHandler(Provider<MyController> provider) {
   @Nonnull @Override public Object apply(@Nonnull Context ctx) throws Exception {
     ctx.setResponseCode(StatusCode.NO_CONTENT);
     provider.get().controllerMethod();
-    return ctx;
+    if (ctx.isResponseStarted()) {
+      return ctx;
+    } else {
+      return ctx.send(ctx.getResponseCode());
+    }
   }
 }

modules/jooby-apt/src/test/java/source/Controller1545.java

@@ -13,6 +13,11 @@ public class Controller1545 {
   public void voidDefault() {
   }
 
+  @DELETE("/success")
+  public void voidDeleteSuccess(Context ctx) {
+    ctx.setResponseCode(StatusCode.OK);
+  }
+
   @POST
   public void voidCreated(Context ctx) {
     ctx.setResponseCode(StatusCode.CREATED);

modules/jooby-apt/src/test/java/source/Provisioning.java

@@ -9,6 +9,7 @@
 import io.jooby.Session;
 import io.jooby.StatusCode;
 import io.jooby.annotations.CookieParam;
+import io.jooby.annotations.DELETE;
 import io.jooby.annotations.FlashParam;
 import io.jooby.annotations.FormParam;
 import io.jooby.annotations.GET;
@@ -305,7 +306,7 @@ public StatusCode statusCode(@QueryParam StatusCode statusCode, @QueryParam Stri
     return statusCode;
   }
 
-  @GET("/noContent")
+  @DELETE("/noContent")
   public void noContent() {
   }
 

modules/jooby-apt/src/test/java/source/VoidRoute.java

@@ -1,12 +1,12 @@
 package source;
 
-import io.jooby.annotations.GET;
+import io.jooby.annotations.DELETE;
 import io.jooby.annotations.Path;
 
 @Path("/void")
 public class VoidRoute {
 
-  @GET
+  @DELETE
   public void noContent() {
 
   }

modules/jooby-apt/src/test/java/tests/HandlerCompilerTest.java

@@ -334,7 +334,7 @@ public void returnTypes() throws Exception {
 
           ctx = MockContextHelper.mockContext();
           assertEquals(ctx,
-              router.get("/p/noContent", ctx.setQueryString(null)).value());
+              router.delete("/p/noContent", ctx.setQueryString(null)).value());
           assertEquals(StatusCode.NO_CONTENT, ctx.getResponseCode());
 
           ctx = MockContextHelper.mockContext();

modules/jooby-apt/src/test/java/tests/Issue1545.java

@@ -19,6 +19,10 @@ public void shouldSetNoContentCodeForVoidRoute() throws Exception {
             assertEquals(StatusCode.NO_CONTENT, rsp.getStatusCode());
           });
 
+          router.delete("/1545/success", rsp -> {
+            assertEquals(StatusCode.OK, rsp.getStatusCode());
+          });
+
           router.post("/1545", rsp -> {
             assertEquals(StatusCode.CREATED, rsp.getStatusCode());
           });

modules/jooby-apt/src/test/java/tests/ModuleCompilerTest.java

@@ -78,7 +78,7 @@ public void voidRoutes() throws Exception {
     new MvcModuleCompilerRunner(new VoidRoute())
         .module(app -> {
           MockRouter router = new MockRouter(app);
-          router.get("/void", rsp -> {
+          router.delete("/void", rsp -> {
             assertEquals(StatusCode.NO_CONTENT, rsp.getStatusCode());
           });
         });

tests/src/test/java/examples/InstanceRouter.java

@@ -2,9 +2,11 @@
 
 import io.jooby.Context;
 import io.jooby.Route;
+import io.jooby.annotations.DELETE;
 import io.jooby.annotations.GET;
 import io.jooby.annotations.POST;
 import io.jooby.annotations.Path;
+import org.slf4j.LoggerFactory;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 
@@ -25,7 +27,7 @@ public String subpath() {
     return "OK";
   }
 
-  @GET
+  @DELETE
   @Path("/void")
   public void noContent() {
 
@@ -34,6 +36,7 @@ public void noContent() {
   @GET
   @Path("/voidwriter")
   public void writer(Context ctx) throws Exception {
+    LoggerFactory.getLogger(getClass()).info("blocking");
     ctx.responseWriter(writer -> {
       writer.println("writer");
     });

tests/src/test/java/io/jooby/MvcTest.java

@@ -14,6 +14,7 @@
 import io.jooby.json.JacksonModule;
 import io.jooby.junit.ServerTest;
 import io.jooby.junit.ServerTestRunner;
+import io.jooby.netty.Netty;
 import okhttp3.FormBody;
 import okhttp3.MediaType;
 import okhttp3.MultipartBody;
@@ -51,7 +52,7 @@ public void routerInstance(ServerTestRunner runner) {
         assertEquals("OK", rsp.body().string());
       });
 
-      client.get("/void", rsp -> {
+      client.delete("/void", rsp -> {
         assertEquals("", rsp.body().string());
         assertEquals(204, rsp.code());
       });
@@ -81,7 +82,7 @@ public void routerImporting(ServerTestRunner runner) {
         assertEquals("OK", rsp.body().string());
       });
 
-      client.get("/sub/void", rsp -> {
+      client.delete("/sub/void", rsp -> {
         assertEquals("", rsp.body().string());
         assertEquals(204, rsp.code());
       });