Fix optionals when empty (#5194)

Signed-off-by: Jorge Bescos Gascon <jorge.bescos.gascon@oracle.com>

core-common/src/main/java/org/glassfish/jersey/internal/inject/ParamConverters.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012, 2021 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2012, 2022 Oracle and/or its affiliates. All rights reserved.
  * Copyright (c) 2018 Payara Foundation and/or its affiliates.
  *
  * This program and the accompanying materials are made available under the
@@ -279,11 +279,15 @@ public T fromString(String value) {
                     } else {
                         final List<ClassTypePair> ctps = ReflectionHelper.getTypeArgumentAndClass(genericType);
                         final ClassTypePair ctp = (ctps.size() == 1) ? ctps.get(0) : null;
-
+                        final boolean empty = value.isEmpty();
                         for (ParamConverterProvider provider : Providers.getProviders(manager, ParamConverterProvider.class)) {
                             final ParamConverter<?> converter = provider.getConverter(ctp.rawClass(), ctp.type(), annotations);
                             if (converter != null) {
-                                return (T) Optional.of(value).map(s -> converter.fromString(value));
+                                if (empty) {
+                                    return (T) Optional.empty();
+                                } else {
+                                    return (T) Optional.of(value).map(s -> converter.fromString(value));
+                                }
                             }
                         }
                         /*
@@ -322,7 +326,7 @@ public <T> ParamConverter<T> getConverter(Class<T> rawType, Type genericType, An
 
                 @Override
                 public T fromString(String value) {
-                    if (value == null) {
+                    if (value == null || value.isEmpty()) {
                         return (T) optionals.empty();
                     } else {
                         return (T) optionals.of(value);

tests/e2e-server/src/test/java/org/glassfish/jersey/tests/e2e/server/OptionalParamConverterTest.java

@@ -16,14 +16,17 @@
 
 package org.glassfish.jersey.tests.e2e.server;
 
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+
 import java.lang.annotation.Annotation;
 import java.lang.reflect.Type;
 import java.text.ParseException;
 import java.time.Instant;
 import java.util.Date;
 import java.util.List;
 import java.util.Optional;
-
+import javax.validation.constraints.NotNull;
 import javax.ws.rs.GET;
 import javax.ws.rs.Path;
 import javax.ws.rs.QueryParam;
@@ -32,20 +35,30 @@
 import javax.ws.rs.ext.ParamConverter;
 import javax.ws.rs.ext.ParamConverterProvider;
 import javax.ws.rs.ext.Provider;
-
 import org.glassfish.jersey.internal.LocalizationMessages;
 import org.glassfish.jersey.internal.inject.ExtractorException;
 import org.glassfish.jersey.server.ResourceConfig;
 import org.glassfish.jersey.test.JerseyTest;
 import org.junit.jupiter.api.Test;
 
-import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertFalse;
-
 public class OptionalParamConverterTest extends JerseyTest {
 
     private static final String PARAM_NAME = "paramName";
 
+    @Path("/IntegerResource")
+    public static class IntegerResource {
+        @GET
+        @Path("/fromInteger")
+        public Response fromInteger(@QueryParam(PARAM_NAME) Integer data) {
+            return Response.ok(0).build();
+        }
+        @GET
+        @Path("/fromIntegerNotNull")
+        public Response fromIntegerNotNull(@NotNull @QueryParam(PARAM_NAME) Integer data) {
+            return Response.ok(0).build();
+        }
+    }
+
     @Path("/OptionalResource")
     public static class OptionalResource {
 
@@ -61,6 +74,12 @@ public Response fromInteger(@QueryParam(PARAM_NAME) Optional<Integer> data) {
             return Response.ok(data.orElse(0)).build();
         }
 
+        @GET
+        @Path("/fromIntegerNotNull")
+        public Response fromIntegerNotNull(@NotNull @QueryParam(PARAM_NAME) Optional<Integer> data) {
+            return Response.ok(data.orElse(0)).build();
+        }
+
         @GET
         @Path("/fromDate")
         public Response fromDate(@QueryParam(PARAM_NAME) Optional<Date> data) throws ParseException {
@@ -118,7 +137,7 @@ public String toString(T value) {
 
     @Override
     protected Application configure() {
-        return new ResourceConfig(OptionalResource.class, InstantParamConverterProvider.class);
+        return new ResourceConfig(OptionalResource.class, IntegerResource.class, InstantParamConverterProvider.class);
     }
 
     @Test
@@ -137,6 +156,7 @@ public void fromOptionalInteger() {
         Response empty = target("/OptionalResource/fromInteger").queryParam(PARAM_NAME, "").request().get();
         Response notEmpty = target("/OptionalResource/fromInteger").queryParam(PARAM_NAME, 1).request().get();
         Response invalid = target("/OptionalResource/fromInteger").queryParam(PARAM_NAME, "invalid").request().get();
+        Response missingNotNull = target("/OptionalResource/fromIntegerNotNull").request().get();
         assertEquals(200, missing.getStatus());
         assertEquals(Integer.valueOf(0), missing.readEntity(Integer.class));
         assertEquals(200, empty.getStatus());
@@ -145,6 +165,17 @@ public void fromOptionalInteger() {
         assertEquals(Integer.valueOf(1), notEmpty.readEntity(Integer.class));
         assertEquals(404, invalid.getStatus());
         assertFalse(invalid.hasEntity());
+        assertEquals(200, missingNotNull.getStatus());
+        assertEquals(Integer.valueOf(0), missingNotNull.readEntity(Integer.class));
+    }
+
+    @Test
+    public void fromInteger() {
+        Response missing = target("/IntegerResource/fromInteger").request().get();
+        Response missingNotNull = target("/IntegerResource/fromIntegerNotNull").request().get();
+        assertEquals(200, missing.getStatus());
+        assertEquals(Integer.valueOf(0), missing.readEntity(Integer.class));
+        assertEquals(400, missingNotNull.getStatus());
     }
 
     @Test
@@ -156,8 +187,8 @@ public void fromOptionalDate() {
         Response invalid = target("/OptionalResource/fromDate").queryParam(PARAM_NAME, "invalid").request().get();
         assertEquals(200, missing.getStatus());
         assertEquals(new Date(1609459200000L), missing.readEntity(Date.class));
-        assertEquals(404, empty.getStatus());
-        assertFalse(empty.hasEntity());
+        assertEquals(200, empty.getStatus());
+        assertEquals(new Date(1609459200000L), empty.readEntity(Date.class));
         assertEquals(200, notEmpty.getStatus());
         assertEquals(new Date(1619870400000L), notEmpty.readEntity(Date.class));
         assertEquals(404, invalid.getStatus());
@@ -173,8 +204,8 @@ public void fromOptionalInstant() {
         Response invalid = target("/OptionalResource/fromInstant").queryParam(PARAM_NAME, "invalid").request().get();
         assertEquals(200, missing.getStatus());
         assertEquals("2021-01-01T00:00:00Z", missing.readEntity(String.class));
-        assertEquals(404, empty.getStatus());
-        assertFalse(empty.hasEntity());
+        assertEquals(200, empty.getStatus());
+        assertEquals("2021-01-01T00:00:00Z", empty.readEntity(String.class));
         assertEquals(200, notEmpty.getStatus());
         assertEquals("2021-05-01T12:00:00Z", notEmpty.readEntity(String.class));
         assertEquals(404, invalid.getStatus());