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

Support java.util.Optional<T> passing to @QueryParam/@RestQuery for RestСlient #45672

Merged
merged 1 commit into from
Jan 17, 2025
+90 −0
Merged

Support java.util.Optional<T> passing to @QueryParam/@RestQuery for RestСlient #45672

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
4 changes: 4 additions & 0 deletions docs/src/main/asciidoc/rest-client.adoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ import jakarta.ws.rs.QueryParam;
import jakarta.ws.rs.core.MultivaluedMap;
import java.util.Map;
import java.util.Set;
import java util.Optional;

@Path("/extensions")
@RegisterRestClient(configKey = "extensions-api")
Expand All @@ -168,6 +169,9 @@ public interface ExtensionsService {
@GET
Set<Extension> getByName(@RestQuery String name); <1>

@GET
Set<Extension> getByOptionalName(@RestQuery Optional<String> name);

@GET
Set<Extension> getByFilter(@RestQuery Map<String, String> filter); <2>

Expand Down
18 changes: 18 additions & 0 deletions ...c/main/java/io/quarkus/jaxrs/client/reactive/deployment/JaxrsClientReactiveProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import static org.jboss.resteasy.reactive.common.processor.ResteasyReactiveDotNames.MAP;
import static org.jboss.resteasy.reactive.common.processor.ResteasyReactiveDotNames.MULTI;
import static org.jboss.resteasy.reactive.common.processor.ResteasyReactiveDotNames.OBJECT;
import static org.jboss.resteasy.reactive.common.processor.ResteasyReactiveDotNames.OPTIONAL;
import static org.jboss.resteasy.reactive.common.processor.ResteasyReactiveDotNames.PART_TYPE_NAME;
import static org.jboss.resteasy.reactive.common.processor.ResteasyReactiveDotNames.REST_FORM_PARAM;
import static org.jboss.resteasy.reactive.common.processor.ResteasyReactiveDotNames.REST_MULTI;
Expand Down Expand Up @@ -2861,6 +2862,19 @@ private ResultHandle addQueryParam(MethodInfo jandexMethod, BytecodeCreator meth
paramArray = notNullParam.invokeStaticMethod(
MethodDescriptor.ofMethod(ToObjectArray.class, "collection", Object[].class, Collection.class),
queryParamHandle);
} else if (isOptional(type, index)) {
if (type.kind() == PARAMETERIZED_TYPE) {
Type paramType = type.asParameterizedType().arguments().get(0);
if ((paramType.kind() == CLASS) || (paramType.kind() == PARAMETERIZED_TYPE)) {
Copy link
Contributor

@Ladicek Ladicek Jan 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Technically the type argument of Optional could also be an array.

EDIT: And a wildcard. Let's not get too wild with type variables :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, i am tried to understand these mechanisms for hours, but still having some troubles :)
Do you mean, that i can just drop

if ((paramType.kind() == CLASS) || (paramType.kind() == PARAMETERIZED_TYPE)) {

and assign componentType directly to componentType = paramType.name().toString(); and that's safe ?
or i need to limit support of parameters type (exclude arrays and etc)

Copy link
Contributor

Choose a reason for hiding this comment

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

I honestly have no idea, I'm not familiar with this code at all. Just pointed out something that I saw while briefly going through this PR for no particular reason :-)

Copy link
Contributor

Choose a reason for hiding this comment

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

Indeed. At some point we might need to handle all the possible types, but we can keep that change for another day

componentType = paramType.name().toString();
}
}
if (componentType == null) {
componentType = DotNames.OBJECT.toString();
}
paramArray = notNullParam.invokeStaticMethod(
MethodDescriptor.ofMethod(ToObjectArray.class, "optional", Object[].class, Optional.class),
queryParamHandle);
} else {
componentType = type.name().toString();
paramArray = notNullParam.invokeStaticMethod(
Expand Down Expand Up @@ -2930,6 +2944,10 @@ private boolean isMap(Type type, IndexView index) {
return isAssignableFrom(MAP, type.name(), index);
}

private boolean isOptional(Type type, IndexView index) {
return isAssignableFrom(OPTIONAL, type.name(), index);
}

private void addHeaderParam(BytecodeCreator invoBuilderEnricher, AssignableResultHandle invocationBuilder,
String paramName, ResultHandle headerParamHandle, String paramType, ResultHandle client,
ResultHandle genericType, ResultHandle annotations) {
Expand Down
5 changes: 5 additions & 0 deletions ...t-jaxrs/runtime/src/main/java/io/quarkus/jaxrs/client/reactive/runtime/ToObjectArray.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package io.quarkus.jaxrs.client.reactive.runtime;

import java.util.Collection;
import java.util.Optional;

/**
* used by query param handling mechanism, in generated code
Expand All @@ -16,6 +17,10 @@ public static Object[] value(Object value) {
return new Object[] { value };
}

public static Object[] optional(Optional<?> optional) {
return optional.isPresent() ? new Object[] { optional.get() } : new Object[] {};
}

private ToObjectArray() {
}
}
63 changes: 63 additions & 0 deletions ...ent/deployment/src/test/java/io/quarkus/rest/client/reactive/OptionalQueryParamsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
package io.quarkus.rest.client.reactive;

import static org.assertj.core.api.Assertions.assertThat;

import java.net.URI;
import java.util.*;

import jakarta.ws.rs.GET;
import jakarta.ws.rs.Path;
import jakarta.ws.rs.core.Context;
import jakarta.ws.rs.core.UriInfo;

import org.jboss.resteasy.reactive.RestQuery;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.test.QuarkusUnitTest;
import io.quarkus.test.common.http.TestHTTPResource;

public class OptionalQueryParamsTest {

@RegisterExtension
static final QuarkusUnitTest config = new QuarkusUnitTest()
.withApplicationRoot((jar) -> jar.addClasses(Resource.class, Client.class));

@TestHTTPResource
URI baseUri;

@Test
void testQueryParamsWithOptionals() {
Client client = QuarkusRestClientBuilder.newBuilder().baseUri(baseUri).build(Client.class);
String responseForEmptyOptional = client.optional(Optional.empty());
assertThat(responseForEmptyOptional).isNullOrEmpty();

String responseForValueOptional = client.optional(Optional.of("value"));
assertThat(responseForValueOptional).isEqualTo("parameter=value");
}

@Path("optional")
public static class Resource {

@Path("query")
@GET
public String queryParams(@Context UriInfo uriInfo) {

var entries = new ArrayList<String>(uriInfo.getQueryParameters().size());
for (var entry : uriInfo.getQueryParameters().entrySet()) {
entries.add(entry.getKey() + "=" + String.join(",", entry.getValue()));
}
return String.join(";", entries);

}
}

@Path("optional")
public interface Client {

@Path("query")
@GET
String optional(@RestQuery Optional<String> parameter);

}
}
Loading