Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make RESTEasy Reactive work with Optional temporal types #28095

Merged
merged 1 commit into from
Sep 21, 2022
Merged

Make RESTEasy Reactive work with Optional temporal types #28095

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions ...processor/src/main/java/org/jboss/resteasy/reactive/common/processor/EndpointIndexer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ public abstract class EndpointIndexer<T extends EndpointIndexer<T, PARAM, METHOD
DotName.createSimple("org.jboss.resteasy.reactive.server.SimpleResourceInfo"), //TODO: fixme
RESOURCE_INFO)));

private static final Set<DotName> SUPPORT_TEMPORAL_PARAMS = Set.of(INSTANT, LOCAL_DATE, LOCAL_TIME, LOCAL_DATE_TIME,
protected static final Set<DotName> SUPPORT_TEMPORAL_PARAMS = Set.of(INSTANT, LOCAL_DATE, LOCAL_TIME, LOCAL_DATE_TIME,
OFFSET_TIME,
OFFSET_DATE_TIME, ZONED_DATE_TIME);

Expand Down Expand Up @@ -1281,8 +1281,8 @@ && isContextType(paramType.asClassType())) {
currentClassInfo, actualEndpointInfo, index);
}

handleOptionalParam(existingConverters, errorLocation, hasRuntimeConverters, builder, elementType,
genericElementType);
handleOptionalParam(existingConverters, anns, errorLocation, hasRuntimeConverters, builder, elementType,
genericElementType, currentMethodInfo);
}
builder.setOptional(true);
} else if (convertible) {
Expand Down Expand Up @@ -1380,8 +1380,11 @@ protected void handleSortedSetParam(Map<String, String> existingConverters, Stri
boolean hasRuntimeConverters, PARAM builder, String elementType) {
}

protected void handleOptionalParam(Map<String, String> existingConverters, String errorLocation,
boolean hasRuntimeConverters, PARAM builder, String elementType, String genericElementType) {
protected void handleOptionalParam(Map<String, String> existingConverters,
Map<DotName, AnnotationInstance> parameterAnnotations,
String errorLocation,
boolean hasRuntimeConverters, PARAM builder, String elementType, String genericElementType,
MethodInfo currentMethodInfo) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it already supported by other containers? List, Set...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No it does not.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it? :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likely yes, but that requires a different fix, which is probably best done in a follow up

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW, It's an honest question, I don't know if it makes sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it does :). I was iniatially sceptical, but came to the conclusion that does make sense.

}

protected void handleSetParam(Map<String, String> existingConverters, String errorLocation, boolean hasRuntimeConverters,
Expand Down
36 changes: 20 additions & 16 deletions ...sor/src/main/java/org/jboss/resteasy/reactive/server/processor/ServerEndpointIndexer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -333,8 +333,11 @@ protected void handleSortedSetParam(Map<String, String> existingConverters, Stri
builder.setConverter(new SortedSetConverter.SortedSetSupplier(converter));
}

protected void handleOptionalParam(Map<String, String> existingConverters, String errorLocation,
boolean hasRuntimeConverters, ServerIndexedParameter builder, String elementType, String genericElementType) {
protected void handleOptionalParam(Map<String, String> existingConverters,
Map<DotName, AnnotationInstance> parameterAnnotations,
String errorLocation,
boolean hasRuntimeConverters, ServerIndexedParameter builder, String elementType, String genericElementType,
MethodInfo currentMethodInfo) {
ParameterConverterSupplier converter = null;

if (genericElementType != null) {
Expand All @@ -350,6 +353,9 @@ protected void handleOptionalParam(Map<String, String> existingConverters, Strin
converter = new SortedSetConverter.SortedSetSupplier(genericTypeConverter);
builder.setSingle(false);
}
} else if (SUPPORT_TEMPORAL_PARAMS.contains(DotName.createSimple(elementType))) {
converter = determineTemporalConverter(DotName.createSimple(elementType), parameterAnnotations,
currentMethodInfo);
}

if (converter == null) {
Expand Down Expand Up @@ -393,6 +399,11 @@ protected String handleTrailingSlash(String path) {
protected void handleTemporalParam(ServerIndexedParameter builder, DotName paramType,
Map<DotName, AnnotationInstance> parameterAnnotations,
MethodInfo currentMethodInfo) {
builder.setConverter(determineTemporalConverter(paramType, parameterAnnotations, currentMethodInfo));
}

private ParameterConverterSupplier determineTemporalConverter(DotName paramType,
Map<DotName, AnnotationInstance> parameterAnnotations, MethodInfo currentMethodInfo) {
String format = null;
String dateTimeFormatterProviderClassName = null;

Expand All @@ -416,8 +427,7 @@ protected void handleTemporalParam(ServerIndexedParameter builder, DotName param
"'java.time.Instant' types must not be annotated with '@DateFormat'",
currentMethodInfo));
}
builder.setConverter(new InstantParamConverter.Supplier());
return;
return new InstantParamConverter.Supplier();
}

if ((format != null) && (dateTimeFormatterProviderClassName != null)) {
Expand All @@ -430,23 +440,17 @@ protected void handleTemporalParam(ServerIndexedParameter builder, DotName param
}

if (LOCAL_DATE.equals(paramType)) {
builder.setConverter(new LocalDateParamConverter.Supplier(format, dateTimeFormatterProviderClassName));
return;
return new LocalDateParamConverter.Supplier(format, dateTimeFormatterProviderClassName);
} else if (LOCAL_DATE_TIME.equals(paramType)) {
builder.setConverter(new LocalDateTimeParamConverter.Supplier(format, dateTimeFormatterProviderClassName));
return;
return new LocalDateTimeParamConverter.Supplier(format, dateTimeFormatterProviderClassName);
} else if (LOCAL_TIME.equals(paramType)) {
builder.setConverter(new LocalTimeParamConverter.Supplier(format, dateTimeFormatterProviderClassName));
return;
return new LocalTimeParamConverter.Supplier(format, dateTimeFormatterProviderClassName);
} else if (OFFSET_DATE_TIME.equals(paramType)) {
builder.setConverter(new OffsetDateTimeParamConverter.Supplier(format, dateTimeFormatterProviderClassName));
return;
return new OffsetDateTimeParamConverter.Supplier(format, dateTimeFormatterProviderClassName);
} else if (OFFSET_TIME.equals(paramType)) {
builder.setConverter(new OffsetTimeParamConverter.Supplier(format, dateTimeFormatterProviderClassName));
return;
return new OffsetTimeParamConverter.Supplier(format, dateTimeFormatterProviderClassName);
} else if (ZONED_DATE_TIME.equals(paramType)) {
builder.setConverter(new ZonedDateTimeParamConverter.Supplier(format, dateTimeFormatterProviderClassName));
return;
return new ZonedDateTimeParamConverter.Supplier(format, dateTimeFormatterProviderClassName);
}

throw new RuntimeException(
Expand Down
16 changes: 16 additions & 0 deletions ...est/java/org/jboss/resteasy/reactive/server/vertx/test/simple/LocalDateTimeParamTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import io.restassured.RestAssured;
import java.time.LocalDateTime;
import java.time.format.DateTimeFormatter;
import java.util.Optional;
import javax.ws.rs.FormParam;
import javax.ws.rs.GET;
import javax.ws.rs.POST;
Expand Down Expand Up @@ -30,6 +31,15 @@ public void localDateTimeAsQueryParam() {
.then().statusCode(200).body(Matchers.equalTo("hello#1984"));
}

@Test
public void localDateTimeAsOptionalQueryParam() {
RestAssured.get("/hello/optional?date=1984-08-08T01:02:03")
.then().statusCode(200).body(Matchers.equalTo("hello#1984"));

RestAssured.get("/hello/optional")
.then().statusCode(200).body(Matchers.equalTo("hello#2022"));
}

@Test
public void localDateTimeAsPathParam() {
RestAssured.get("/hello/1995-09-21 01:02:03")
Expand All @@ -50,6 +60,12 @@ public String helloQuery(@RestQuery LocalDateTime date) {
return "hello#" + date.getYear();
}

@Path("optional")
@GET
public String helloOptionalQuery(@RestQuery Optional<LocalDateTime> date) {
return "hello#" + date.orElse(LocalDateTime.of(2022, 1, 1, 0, 0)).getYear();
}

@GET
@Path("{date}")
public String helloPath(@RestPath @DateFormat(pattern = "yyyy-MM-dd HH:mm:ss") LocalDateTime date) {
Expand Down