Skip to content

Commit

Permalink
REST Server/Client allows configuring the removal of trailing slashs
Browse files Browse the repository at this point in the history
Before these changes, the REST server and client extensions were removing the trailing slashes that were set by users in the `@Path` annotations.

For example, when creating a resource like:

```java
@path("/a/")
@produces(MediaType.TEXT_PLAIN)
public class HelloResource {

        @get
        public String echo() {
            // ...
        }
}
```

The effective path was `/a` instead of `/a/`. Note that it does not matter whether we place the `@Path` annotation at method level because the behaviour will be the same.

At the client side, when having the following client:

```
@RegisterRestClient(configKey = "test")
@path("/a/") <1>
public interface HelloClient {
        @get
        @produces(MediaType.TEXT_PLAIN)
        @path("/b/")  <2>
        String echo();
}
```

In this case, the trailing slash will be removed from <1> but not from <2>.

After these changes, we are adding the following properties:
- `quarkus.rest.removes-trailing-slash`

To configure the server extension to not remove any trailing slash from the `@Path` annotations.

- `quarkus.rest-client.removes-trailing-slash`

To configure the client extension to not remove any traling slash from the `@Path` annotations used at interface level. The annotations set at method level will behave the same.

Note that this does not introduce any change in behaviour, so everything should keep working as before unless users use the new properties.
  • Loading branch information
Sgitario committed Nov 8, 2024
1 parent ae36baf commit 606143b
Show file tree
Hide file tree
Showing 12 changed files with 266 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,13 @@
import io.quarkus.runtime.annotations.ConfigPhase;
import io.quarkus.runtime.annotations.ConfigRoot;
import io.smallrye.config.ConfigMapping;
import io.smallrye.config.ConfigValue;
import io.smallrye.config.SmallRyeConfig;
import io.smallrye.config.SmallRyeConfigBuilder;
import io.smallrye.config.SmallRyeConfigBuilderCustomizer;
import io.smallrye.config.WithDefault;
import io.smallrye.config.WithDefaults;
import io.smallrye.config.WithName;
import io.smallrye.config.WithParentName;

@ConfigMapping(prefix = "quarkus.rest-client")
Expand All @@ -30,6 +32,14 @@ public interface RestClientsBuildTimeConfig {
@WithDefaults
Map<String, RestClientBuildConfig> clients();

/**
* If true, the extension will automatically remove the trailing slash in the paths if any.
* This property is not applicable to the RESTEasy Client.
*/
@WithName("removes-trailing-slash")
@WithDefault("true")
boolean removesTrailingSlash();

interface RestClientBuildConfig {

/**
Expand Down Expand Up @@ -69,6 +79,14 @@ interface RestClientBuildConfig {
* </ul>
*/
Optional<String> localProxyProvider();

/**
* If true, the extension will automatically remove the trailing slash in the paths if any.
* This property is not applicable to the RESTEasy Client.
*/
@WithName("removes-trailing-slash")
@WithDefault("true")
boolean removesTrailingSlash();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ quarkus.rest-client."io.quarkus.restclient.config.FullNameRestClient".connection
quarkus.rest-client."io.quarkus.restclient.config.FullNameRestClient".connection-pool-size=10
quarkus.rest-client."io.quarkus.restclient.config.FullNameRestClient".max-chunk-size=1024

quarkus.rest-client.key.removes-trailing-slash=false
quarkus.rest-client.key.url=http://localhost:8080
quarkus.rest-client.key.uri=http://localhost:8081
quarkus.rest-client.key.scope=Singleton
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,9 @@ void setupClientProxies(JaxrsClientReactiveRecorder recorder,
List<RestClientDefaultProducesBuildItem> defaultConsumes,
List<RestClientDefaultConsumesBuildItem> defaultProduces,
List<RestClientDisableSmartDefaultProduces> disableSmartDefaultProduces,
List<RestClientDisableRemovalTrailingSlashBuildItem> disableRemovalTrailingSlashProduces,
List<ParameterContainersBuildItem> parameterContainersBuildItems) {

String defaultConsumesType = defaultMediaType(defaultConsumes, MediaType.APPLICATION_OCTET_STREAM);
String defaultProducesType = defaultMediaType(defaultProduces, MediaType.TEXT_PLAIN);

Expand Down Expand Up @@ -403,8 +405,9 @@ public void accept(EndpointIndexer.ResourceMethodCallbackEntry entry) {
ClassInfo clazz = index.getClassByName(i.getKey());
//these interfaces can also be clients
//so we generate client proxies for them
MaybeRestClientInterface maybeClientProxy = clientEndpointIndexer.createClientProxy(clazz,
i.getValue());
String path = sanitizePath(i.getValue(),
isRemovalTrailingSlashEnabled(i.getKey(), disableRemovalTrailingSlashProduces));
MaybeRestClientInterface maybeClientProxy = clientEndpointIndexer.createClientProxy(clazz, path);
if (maybeClientProxy.exists()) {
RestClientInterface clientProxy = maybeClientProxy.getRestClientInterface();
try {
Expand Down Expand Up @@ -3076,6 +3079,30 @@ private void addCookieParam(BytecodeCreator invoBuilderEnricher, AssignableResul
invocationBuilder, notNullValue.load(paramName), cookieParamHandle));
}

private String sanitizePath(String path, boolean removesTrailingSlash) {
if (!path.startsWith("/")) {
path = "/" + path;
}

// For the client side, by default, we're only removing the trailing slash for the
// `@Path` annotations at class level which is configurable.
if (removesTrailingSlash && path.endsWith("/")) {
path = path.substring(0, path.length() - 1);
}
return path;
}

private boolean isRemovalTrailingSlashEnabled(DotName restClientName,
List<RestClientDisableRemovalTrailingSlashBuildItem> buildItems) {
for (RestClientDisableRemovalTrailingSlashBuildItem buildItem : buildItems) {
if (buildItem.getClients().contains(restClientName)) {
return false;
}
}

return true;
}

private enum ReturnCategory {
BLOCKING,
COMPLETION_STAGE,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package io.quarkus.jaxrs.client.reactive.deployment;

import java.util.List;

import org.jboss.jandex.DotName;

import io.quarkus.builder.item.MultiBuildItem;

/**
* This build item disables the removal of trailing slashes from the paths.
*/
public final class RestClientDisableRemovalTrailingSlashBuildItem extends MultiBuildItem {
private final List<DotName> clients;

public RestClientDisableRemovalTrailingSlashBuildItem(List<DotName> clients) {
this.clients = clients;
}

public List<DotName> getClients() {
return clients;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@
import io.quarkus.jaxrs.client.reactive.deployment.JaxrsClientReactiveEnricherBuildItem;
import io.quarkus.jaxrs.client.reactive.deployment.RestClientDefaultConsumesBuildItem;
import io.quarkus.jaxrs.client.reactive.deployment.RestClientDefaultProducesBuildItem;
import io.quarkus.jaxrs.client.reactive.deployment.RestClientDisableRemovalTrailingSlashBuildItem;
import io.quarkus.jaxrs.client.reactive.deployment.RestClientDisableSmartDefaultProduces;
import io.quarkus.rest.client.reactive.runtime.AnnotationRegisteredProviders;
import io.quarkus.rest.client.reactive.runtime.HeaderCapturingServerFilter;
Expand All @@ -108,6 +109,7 @@
import io.quarkus.rest.client.reactive.runtime.RestClientReactiveConfig;
import io.quarkus.rest.client.reactive.runtime.RestClientRecorder;
import io.quarkus.rest.client.reactive.spi.RestClientAnnotationsTransformerBuildItem;
import io.quarkus.restclient.config.RegisteredRestClient;
import io.quarkus.restclient.config.RestClientsBuildTimeConfig;
import io.quarkus.restclient.config.RestClientsConfig;
import io.quarkus.restclient.config.deployment.RestClientConfigUtils;
Expand Down Expand Up @@ -159,15 +161,32 @@ ServiceProviderBuildItem nativeSpiSupport() {
}

@BuildStep
void setUpDefaultMediaType(BuildProducer<RestClientDefaultConsumesBuildItem> consumes,
void setUpClientBuildTimeProperties(BuildProducer<RestClientDefaultConsumesBuildItem> consumes,
BuildProducer<RestClientDefaultProducesBuildItem> produces,
BuildProducer<RestClientDisableSmartDefaultProduces> disableSmartProduces,
RestClientReactiveConfig config) {
BuildProducer<RestClientDisableRemovalTrailingSlashBuildItem> disableRemovalTrailingSlash,
RestClientReactiveConfig config,
RestClientsBuildTimeConfig configsPerClient,
List<RegisteredRestClientBuildItem> registeredRestClientBuildItems) {
consumes.produce(new RestClientDefaultConsumesBuildItem(MediaType.APPLICATION_JSON, 10));
produces.produce(new RestClientDefaultProducesBuildItem(MediaType.APPLICATION_JSON, 10));
if (config.disableSmartProduces()) {
disableSmartProduces.produce(new RestClientDisableSmartDefaultProduces());
}

List<RegisteredRestClient> registeredRestClients = toRegisteredRestClients(registeredRestClientBuildItems);
RestClientsBuildTimeConfig buildTimeConfig = configsPerClient.get(registeredRestClients);

List<DotName> clientsToDisable = new ArrayList<>();
for (RegisteredRestClientBuildItem registeredRestClient : registeredRestClientBuildItems) {
if (removesTrailingSlashIsDisabled(buildTimeConfig, registeredRestClient)) {
clientsToDisable.add(registeredRestClient.getClassInfo().name());
}
}

if (!clientsToDisable.isEmpty()) {
disableRemovalTrailingSlash.produce(new RestClientDisableRemovalTrailingSlashBuildItem(clientsToDisable));
}
}

@BuildStep
Expand Down Expand Up @@ -412,7 +431,6 @@ void determineRegisteredRestClients(CombinedIndexBuildItem combinedIndexBuildIte
Set<DotName> seen = new HashSet<>();

List<AnnotationInstance> actualInstances = index.getAnnotations(REGISTER_REST_CLIENT);
List<RegisteredRestClientBuildItem> registeredRestClientBuildItems = new ArrayList<>();
for (AnnotationInstance instance : actualInstances) {
AnnotationTarget annotationTarget = instance.target();
ClassInfo classInfo = annotationTarget.asClass();
Expand Down Expand Up @@ -841,11 +859,17 @@ private boolean isRestMethod(MethodInfo method) {
return false;
}

private Optional<String> getConfigKey(AnnotationInstance registerRestClientAnnotation) {
AnnotationValue configKeyValue = registerRestClientAnnotation.value("configKey");
return configKeyValue != null
? Optional.of(configKeyValue.asString())
: Optional.empty();
private boolean removesTrailingSlashIsDisabled(RestClientsBuildTimeConfig config,
RegisteredRestClientBuildItem registeredRestClient) {
// is disabled for all the clients
if (!config.removesTrailingSlash()) {
return true;
}

// is disabled for this concrete client
return !config.clients()
.get(registeredRestClient.getClassInfo().name().toString())
.removesTrailingSlash();
}

private ScopeInfo computeDefaultScope(Capabilities capabilities, Config config,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
package io.quarkus.rest.client.reactive;

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

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

import org.eclipse.microprofile.rest.client.inject.RegisterRestClient;
import org.eclipse.microprofile.rest.client.inject.RestClient;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.test.QuarkusUnitTest;

public class SlashPathRestClientTest {

@RegisterExtension
static final QuarkusUnitTest TEST = new QuarkusUnitTest()
.withApplicationRoot((jar) -> jar
.addClasses(HelloClientRemovingSlashes.class,
HelloClientUsingConfigKey.class,
HelloClientUsingSimpleName.class,
HelloClientUsingClassName.class,
HelloResource.class))
// disable the removal of trailing slash at client side
.overrideConfigKey("quarkus.rest-client.test.removes-trailing-slash", "false")
.overrideConfigKey("quarkus.rest-client.HelloClientUsingSimpleName.removes-trailing-slash", "false")
.overrideConfigKey(
"quarkus.rest-client.\"io.quarkus.rest.client.reactive.SlashPathRestClientTest$HelloClientUsingClassName\".removes-trailing-slash",
"false")
// disable the removal of trailing slash at server side
.overrideConfigKey("quarkus.rest.removes-trailing-slash", "false")
.overrideRuntimeConfigKey("quarkus.rest-client.test.url",
"http://localhost:${quarkus.http.test-port:8081}")
.overrideRuntimeConfigKey("quarkus.rest-client.default.url",
"http://localhost:${quarkus.http.test-port:8081}")
.overrideRuntimeConfigKey("quarkus.rest-client.HelloClientUsingSimpleName.url",
"http://localhost:${quarkus.http.test-port:8081}")
.overrideRuntimeConfigKey(
"quarkus.rest-client.\"io.quarkus.rest.client.reactive.SlashPathRestClientTest$HelloClientUsingClassName\".url",
"http://localhost:${quarkus.http.test-port:8081}");

@RestClient
HelloClientRemovingSlashes clientUsingDefaultBehaviour;

@RestClient
HelloClientUsingConfigKey clientUsingConfigKey;

@RestClient
HelloClientUsingSimpleName clientUsingSimpleName;

@RestClient
HelloClientUsingClassName clientUsingClassName;

@Test
void shouldRemoveTrailingSlashByDefault() {
assertThat(clientUsingDefaultBehaviour.echo()).isEqualTo("/slash/without");
}

@Test
void shouldRemoveTrailingSlashUsingConfigKey() {
assertThat(clientUsingConfigKey.echo()).isEqualTo("/slash/with/");
}

@Test
void shouldRemoveTrailingSlashUsingSimpleName() {
assertThat(clientUsingSimpleName.echo()).isEqualTo("/slash/with/");
}

@Test
void shouldRemoveTrailingSlashUsingClassName() {
assertThat(clientUsingClassName.echo()).isEqualTo("/slash/with/");
}

@RegisterRestClient(configKey = "default")
@Path("/slash/without/")
public interface HelloClientRemovingSlashes {
@GET
@Produces(MediaType.TEXT_PLAIN)
String echo();
}

@RegisterRestClient(configKey = "test")
@Path("/slash/with/")
public interface HelloClientUsingConfigKey {
@GET
@Produces(MediaType.TEXT_PLAIN)
String echo();
}

@RegisterRestClient
@Path("/slash/with/")
public interface HelloClientUsingSimpleName {
@GET
@Produces(MediaType.TEXT_PLAIN)
String echo();
}

@RegisterRestClient
@Path("/slash/with/")
public interface HelloClientUsingClassName {
@GET
@Produces(MediaType.TEXT_PLAIN)
String echo();
}

@Path("/slash")
@Produces(MediaType.TEXT_PLAIN)
public static class HelloResource {

@GET
@Path("/without")
public String withoutSlash(@Context UriInfo uriInfo) {
return uriInfo.getPath();
}

@GET
@Path("/with/")
public String usingSlash(@Context UriInfo uriInfo) {
return uriInfo.getPath();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -75,4 +75,10 @@ public interface ResteasyReactiveConfig {
*/
@WithDefault("false")
boolean resumeOn404();

/**
* If true, the extension will automatically remove the trailing slash in the paths if any.
*/
@WithDefault("true")
boolean removesTrailingSlash();
}
Original file line number Diff line number Diff line change
Expand Up @@ -538,6 +538,7 @@ public void setupEndpoints(ApplicationIndexBuildItem applicationIndexBuildItem,
.setInjectableBeans(injectableBeans)
.setAdditionalWriters(additionalWriters)
.setDefaultBlocking(appResult.getBlockingDefault())
.setRemovesTrailingSlash(config.removesTrailingSlash())
.setApplicationScanningResult(appResult)
.setMultipartReturnTypeIndexerExtension(
new GeneratedHandlerMultipartReturnTypeIndexerExtension(classOutput))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,19 +60,12 @@ public ClientEndpointIndexer(AbstractBuilder builder, String defaultProduces, bo
this.smartDefaultProduces = smartDefaultProduces;
}

public MaybeRestClientInterface createClientProxy(ClassInfo classInfo,
String path) {
public MaybeRestClientInterface createClientProxy(ClassInfo classInfo, String path) {
try {
RestClientInterface clazz = new RestClientInterface();
clazz.setClassName(classInfo.name().toString());
clazz.setEncoded(classInfo.hasDeclaredAnnotation(ENCODED));
if (path != null) {
if (!path.startsWith("/")) {
path = "/" + path;
}
if (path.endsWith("/")) {
path = path.substring(0, path.length() - 1);
}
clazz.setPath(path);
}
List<ResourceMethod> methods = createEndpoints(classInfo, classInfo, new HashSet<>(), new HashSet<>(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ public Optional<ResourceClass> createEndpoints(ClassInfo classInfo, boolean cons
clazz.setClassName(classInfo.name().toString());
if (path != null) {
if (path.endsWith("/")) {
path = path.substring(0, path.length() - 1);
path = handleTrailingSlash(path);
}
if (!path.startsWith("/")) {
path = "/" + path;
Expand Down Expand Up @@ -502,6 +502,9 @@ private static List<FoundEndpoint> collectEndpoints(ClassInfo currentClassInfo,
return ret;
}

/**
* By default, we are not removing the trailing slash.
*/
protected String handleTrailingSlash(String path) {
return path;
}
Expand Down
Loading

0 comments on commit 606143b

Please sign in to comment.