From f208d793623ab2c54647ac5b9e9e5c2c6b23928c Mon Sep 17 00:00:00 2001 From: Jorge Bescos Gascon Date: Mon, 26 Apr 2021 11:37:26 +0200 Subject: [PATCH 1/2] Support more optionals Signed-off-by: Jorge Bescos Gascon --- .../internal/inject/ParamConverters.java | 93 ++++++++++++++++--- .../server/OptionalParamConverterTest.java | 55 ++++++++++- 2 files changed, 134 insertions(+), 14 deletions(-) diff --git a/core-common/src/main/java/org/glassfish/jersey/internal/inject/ParamConverters.java b/core-common/src/main/java/org/glassfish/jersey/internal/inject/ParamConverters.java index 3f396549fb..75edd78424 100644 --- a/core-common/src/main/java/org/glassfish/jersey/internal/inject/ParamConverters.java +++ b/core-common/src/main/java/org/glassfish/jersey/internal/inject/ParamConverters.java @@ -27,6 +27,9 @@ import java.text.ParseException; import java.util.Date; import java.util.Optional; +import java.util.OptionalDouble; +import java.util.OptionalInt; +import java.util.OptionalLong; import javax.inject.Inject; import javax.inject.Singleton; @@ -266,24 +269,31 @@ public OptionalProvider(AggregatedProvider aggregated) { @Override public ParamConverter getConverter(Class rawType, Type genericType, Annotation[] annotations) { - return (rawType != Optional.class) ? null : new ParamConverter() { + final Optionals optionals = Optionals.getOptional(rawType); + return (optionals == null) ? null : new ParamConverter() { @Override public T fromString(String value) { if (value == null) { - return (T) Optional.empty(); + return (T) optionals.empty(); } else { - ParameterizedType parametrized = (ParameterizedType) genericType; - Type type = parametrized.getActualTypeArguments()[0]; - T val = aggregated.getConverter((Class) type, type, annotations).fromString(value.toString()); - if (val != null) { - return (T) Optional.of(val); + if (genericType instanceof ParameterizedType) { + // Optional is parameterized + ParameterizedType parametrized = (ParameterizedType) genericType; + Type type = parametrized.getActualTypeArguments()[0]; + T val = aggregated.getConverter((Class) type, type, annotations).fromString(value.toString()); + if (val != null) { + return (T) optionals.of(val); + } else { + /* + * In this case we don't send Optional.empty() because 'value' is not null. + * But we return null because the provider didn't find how to parse it. + */ + return null; + } } else { - /* - * In this case we don't send Optional.empty() because 'value' is not null. - * But we return null because the provider didn't find how to parse it. - */ - return null; + // Others are not parameterized + return (T) optionals.of(value); } } } @@ -300,6 +310,65 @@ public String toString(T value) throws IllegalArgumentException { }; } + private static enum Optionals { + + OPTIONAL(Optional.class) { + @Override + Object empty() { + return Optional.empty(); + } + @Override + Object of(Object value) { + return Optional.of(value); + } + }, OPTIONAL_INT(OptionalInt.class) { + @Override + Object empty() { + return OptionalInt.empty(); + } + @Override + Object of(Object value) { + return OptionalInt.of(Integer.parseInt((String) value)); + } + }, OPTIONAL_DOUBLE(OptionalDouble.class) { + @Override + Object empty() { + return OptionalDouble.empty(); + } + @Override + Object of(Object value) { + return OptionalDouble.of(Double.parseDouble((String) value)); + } + }, OPTIONAL_LONG(OptionalLong.class) { + @Override + Object empty() { + return OptionalLong.empty(); + } + @Override + Object of(Object value) { + return OptionalLong.of(Long.parseLong((String) value)); + } + }; + + private final Class clazz; + + private Optionals(Class clazz) { + this.clazz = clazz; + } + + private static Optionals getOptional(Class clazz) { + for (Optionals optionals : Optionals.values()) { + if (optionals.clazz == clazz) { + return optionals; + } + } + return null; + } + + abstract Object empty(); + + abstract Object of(Object value); + } } /** diff --git a/tests/e2e-server/src/test/java/org/glassfish/jersey/tests/e2e/server/OptionalParamConverterTest.java b/tests/e2e-server/src/test/java/org/glassfish/jersey/tests/e2e/server/OptionalParamConverterTest.java index ebbc92a195..8061ffe684 100644 --- a/tests/e2e-server/src/test/java/org/glassfish/jersey/tests/e2e/server/OptionalParamConverterTest.java +++ b/tests/e2e-server/src/test/java/org/glassfish/jersey/tests/e2e/server/OptionalParamConverterTest.java @@ -16,8 +16,13 @@ package org.glassfish.jersey.tests.e2e.server; +import static org.junit.Assert.assertEquals; + import java.util.List; import java.util.Optional; +import java.util.OptionalDouble; +import java.util.OptionalInt; +import java.util.OptionalLong; import javax.ws.rs.GET; import javax.ws.rs.Path; @@ -29,8 +34,6 @@ import org.glassfish.jersey.test.JerseyTest; import org.junit.Test; -import static org.junit.Assert.assertEquals; - public class OptionalParamConverterTest extends JerseyTest { private static final String PARAM_NAME = "paramName"; @@ -59,6 +62,24 @@ public Response fromList(@QueryParam(PARAM_NAME) List> data) { } return Response.ok(builder.toString()).build(); } + + @GET + @Path("/fromInteger2") + public Response fromInteger2(@QueryParam(PARAM_NAME) OptionalInt data) { + return Response.ok(data.orElse(0)).build(); + } + + @GET + @Path("/fromLong") + public Response fromLong(@QueryParam(PARAM_NAME) OptionalLong data) { + return Response.ok(data.orElse(0)).build(); + } + + @GET + @Path("/fromDouble") + public Response fromDouble(@QueryParam(PARAM_NAME) OptionalDouble data) { + return Response.ok(data.orElse(0.1)).build(); + } } @Override @@ -86,6 +107,16 @@ public void fromOptionalInt() { assertEquals(Integer.valueOf(1), notEmpty.readEntity(Integer.class)); } + @Test + public void fromOptionalInt2() { + Response empty = target("/OptionalResource/fromInteger2").request().get(); + Response notEmpty = target("/OptionalResource/fromInteger2").queryParam(PARAM_NAME, 1).request().get(); + assertEquals(200, empty.getStatus()); + assertEquals(Integer.valueOf(0), empty.readEntity(Integer.class)); + assertEquals(200, notEmpty.getStatus()); + assertEquals(Integer.valueOf(1), notEmpty.readEntity(Integer.class)); + } + @Test public void fromOptionalList() { Response empty = target("/OptionalResource/fromList").request().get(); @@ -96,4 +127,24 @@ public void fromOptionalList() { assertEquals(200, notEmpty.getStatus()); assertEquals("12", notEmpty.readEntity(String.class)); } + + @Test + public void fromOptionalLong() { + Response empty = target("/OptionalResource/fromLong").request().get(); + Response notEmpty = target("/OptionalResource/fromLong").queryParam(PARAM_NAME, 1L).request().get(); + assertEquals(200, empty.getStatus()); + assertEquals(Long.valueOf(0L), empty.readEntity(Long.class)); + assertEquals(200, notEmpty.getStatus()); + assertEquals(Long.valueOf(1L), notEmpty.readEntity(Long.class)); + } + + @Test + public void fromOptionalDouble() { + Response empty = target("/OptionalResource/fromDouble").request().get(); + Response notEmpty = target("/OptionalResource/fromDouble").queryParam(PARAM_NAME, 1.1).request().get(); + assertEquals(200, empty.getStatus()); + assertEquals(Double.valueOf(0.1), empty.readEntity(Double.class)); + assertEquals(200, notEmpty.getStatus()); + assertEquals(Double.valueOf(1.1), notEmpty.readEntity(Double.class)); + } } From b4304cd06f065daef641b2fb4ff930e55f72bd40 Mon Sep 17 00:00:00 2001 From: Jorge Bescos Gascon Date: Mon, 12 Jul 2021 07:50:08 +0200 Subject: [PATCH 2/2] Removing duplicated behaviour Signed-off-by: Jorge Bescos Gascon --- .../internal/inject/ParamConverters.java | 44 ++----------- .../OptionalParamConverterNoProviderTest.java | 63 +++++++++++++++++++ 2 files changed, 68 insertions(+), 39 deletions(-) diff --git a/core-common/src/main/java/org/glassfish/jersey/internal/inject/ParamConverters.java b/core-common/src/main/java/org/glassfish/jersey/internal/inject/ParamConverters.java index 9109848e6e..144189b9f1 100644 --- a/core-common/src/main/java/org/glassfish/jersey/internal/inject/ParamConverters.java +++ b/core-common/src/main/java/org/glassfish/jersey/internal/inject/ParamConverters.java @@ -310,20 +310,12 @@ public String toString(T value) throws IllegalArgumentException { } /** - * Provider of {@link ParamConverter param converter} that produce the Optional instance - * by invoking {@link AggregatedProvider}. + * Provider of {@link ParamConverter param converter} that produce the OptionalInt, OptionalDouble + * or OptionalLong instance. */ @Singleton public static class OptionalProvider implements ParamConverterProvider { - // Delegates to this provider when the type of Optional is extracted. - private final AggregatedProvider aggregated; - - @Inject - public OptionalProvider(AggregatedProvider aggregated) { - this.aggregated = aggregated; - } - @Override public ParamConverter getConverter(Class rawType, Type genericType, Annotation[] annotations) { final Optionals optionals = Optionals.getOptional(rawType); @@ -334,24 +326,7 @@ public T fromString(String value) { if (value == null) { return (T) optionals.empty(); } else { - if (genericType instanceof ParameterizedType) { - // Optional is parameterized - ParameterizedType parametrized = (ParameterizedType) genericType; - Type type = parametrized.getActualTypeArguments()[0]; - T val = aggregated.getConverter((Class) type, type, annotations).fromString(value.toString()); - if (val != null) { - return (T) optionals.of(val); - } else { - /* - * In this case we don't send Optional.empty() because 'value' is not null. - * But we return null because the provider didn't find how to parse it. - */ - return null; - } - } else { - // Others are not parameterized - return (T) optionals.of(value); - } + return (T) optionals.of(value); } } @@ -369,16 +344,7 @@ public String toString(T value) throws IllegalArgumentException { private static enum Optionals { - OPTIONAL(Optional.class) { - @Override - Object empty() { - return Optional.empty(); - } - @Override - Object of(Object value) { - return Optional.of(value); - } - }, OPTIONAL_INT(OptionalInt.class) { + OPTIONAL_INT(OptionalInt.class) { @Override Object empty() { return OptionalInt.empty(); @@ -450,7 +416,7 @@ public AggregatedProvider(InjectionManager manager) { new TypeFromString(), new StringConstructor(), new OptionalCustomProvider(manager), - new OptionalProvider(this) + new OptionalProvider() }; } diff --git a/tests/e2e-server/src/test/java/org/glassfish/jersey/tests/e2e/server/OptionalParamConverterNoProviderTest.java b/tests/e2e-server/src/test/java/org/glassfish/jersey/tests/e2e/server/OptionalParamConverterNoProviderTest.java index 0c97633ec4..162692920a 100644 --- a/tests/e2e-server/src/test/java/org/glassfish/jersey/tests/e2e/server/OptionalParamConverterNoProviderTest.java +++ b/tests/e2e-server/src/test/java/org/glassfish/jersey/tests/e2e/server/OptionalParamConverterNoProviderTest.java @@ -63,6 +63,36 @@ public Response fromList(@QueryParam(PARAM_NAME) List> data) { return Response.ok(builder.toString()).build(); } + @GET + @Path("/fromListOptionalInt") + public Response fromListOptionalInt(@QueryParam(PARAM_NAME) List data) { + StringBuilder builder = new StringBuilder(""); + for (OptionalInt val : data) { + builder.append(val.orElse(0)); + } + return Response.ok(builder.toString()).build(); + } + + @GET + @Path("/fromListOptionalDouble") + public Response fromListOptionalDouble(@QueryParam(PARAM_NAME) List data) { + StringBuilder builder = new StringBuilder(""); + for (OptionalDouble val : data) { + builder.append(val.orElse(0)); + } + return Response.ok(builder.toString()).build(); + } + + @GET + @Path("/fromListOptionalLong") + public Response fromListOptionalLong(@QueryParam(PARAM_NAME) List data) { + StringBuilder builder = new StringBuilder(""); + for (OptionalLong val : data) { + builder.append(val.orElse(0)); + } + return Response.ok(builder.toString()).build(); + } + @GET @Path("/fromInteger2") public Response fromInteger2(@QueryParam(PARAM_NAME) OptionalInt data) { @@ -128,6 +158,39 @@ public void fromOptionalList() { assertEquals("12", notEmpty.readEntity(String.class)); } + @Test + public void fromOptionalListOptionalInt() { + Response empty = target("/OptionalResource/fromListOptionalInt").request().get(); + Response notEmpty = target("/OptionalResource/fromListOptionalInt").queryParam(PARAM_NAME, 1) + .queryParam(PARAM_NAME, 2).request().get(); + assertEquals(200, empty.getStatus()); + assertEquals("", empty.readEntity(String.class)); + assertEquals(200, notEmpty.getStatus()); + assertEquals("12", notEmpty.readEntity(String.class)); + } + + @Test + public void fromOptionalListOptionalDouble() { + Response empty = target("/OptionalResource/fromListOptionalDouble").request().get(); + Response notEmpty = target("/OptionalResource/fromListOptionalDouble").queryParam(PARAM_NAME, 1) + .queryParam(PARAM_NAME, 2).request().get(); + assertEquals(200, empty.getStatus()); + assertEquals("", empty.readEntity(String.class)); + assertEquals(200, notEmpty.getStatus()); + assertEquals("1.02.0", notEmpty.readEntity(String.class)); + } + + @Test + public void fromOptionalListOptionalLong() { + Response empty = target("/OptionalResource/fromListOptionalLong").request().get(); + Response notEmpty = target("/OptionalResource/fromListOptionalLong").queryParam(PARAM_NAME, 1) + .queryParam(PARAM_NAME, 2).request().get(); + assertEquals(200, empty.getStatus()); + assertEquals("", empty.readEntity(String.class)); + assertEquals(200, notEmpty.getStatus()); + assertEquals("12", notEmpty.readEntity(String.class)); + } + @Test public void fromOptionalLong() { Response empty = target("/OptionalResource/fromLong").request().get();