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.

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

[3.2] fix: use raw path and avoid double encoding, adapt tests accordingly #38241

Merged
merged 1 commit into from
Jan 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
package io.quarkus.rest.client.reactive.stork;

import jakarta.ws.rs.Consumes;
import jakarta.ws.rs.GET;
import jakarta.ws.rs.POST;
import jakarta.ws.rs.Path;
import jakarta.ws.rs.PathParam;
import jakarta.ws.rs.core.MediaType;

import org.eclipse.microprofile.rest.client.inject.RegisterRestClient;

Expand All @@ -10,4 +14,13 @@
public interface HelloClient {
@GET
String hello();

@POST
@Consumes(MediaType.TEXT_PLAIN)
@Path("/")
String echo(String name);

@GET
@Path("/{name}")
public String helloWithPathParam(@PathParam("name") String name);
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
package io.quarkus.rest.client.reactive.stork;

import jakarta.ws.rs.GET;
import jakarta.ws.rs.POST;
import jakarta.ws.rs.Path;
import jakarta.ws.rs.PathParam;
import jakarta.ws.rs.Produces;
import jakarta.ws.rs.core.Context;
import jakarta.ws.rs.core.MediaType;
import jakarta.ws.rs.core.Request;

@Path("/hello")
public class HelloResource {
Expand All @@ -12,4 +18,16 @@ public class HelloResource {
public String hello() {
return HELLO_WORLD;
}

@GET
@Path("/{name}")
@Produces(MediaType.TEXT_PLAIN)
public String invoke(@PathParam("name") String name) {
return "Hello, " + name;
}

@POST
public String echo(String name, @Context Request request) {
return "hello, " + name;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import jakarta.ws.rs.GET;
import jakarta.ws.rs.Path;
import jakarta.ws.rs.PathParam;

import org.eclipse.microprofile.rest.client.RestClientBuilder;
import org.eclipse.microprofile.rest.client.inject.RestClient;
Expand All @@ -22,6 +23,18 @@ public String invokeClient() {
return client.hello();
}

@Path("/v2/{name}")
@GET
public String invokeClientWithPathParamContainingSlash(@PathParam("name") String name) {
return client.helloWithPathParam(name + "/" + name);
}

@Path("/{name}")
@GET
public String invokeClientWithPathParam(@PathParam("name") String name) {
return client.helloWithPathParam(name);
}

@Path("/cdi")
@GET
public String invokeCdiClient() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,4 +67,25 @@ void shouldModifyStorkSettings() {
.body(equalTo(WIREMOCK_RESPONSE));
// @formatter:on
}

@Test
void shouldSayHelloNameWithSlash() {
when()
.get("/helper/v2/stork")
.then()
.statusCode(200)
// The response contains an encoded `/`
.body(equalTo("Hello, stork/stork"));

}

@Test
void shouldSayHelloNameWithBlank() {
when()
.get("/helper/smallrye stork")
.then()
.statusCode(200)
// The response contains an encoded blank espace
.body(equalTo("Hello, smallrye stork"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,54 +15,65 @@
import org.junit.jupiter.api.Timeout;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.rest.client.reactive.HelloClient2;
import io.quarkus.rest.client.reactive.HelloResource;
import io.quarkus.test.QuarkusUnitTest;
import io.smallrye.stork.api.NoSuchServiceDefinitionException;

public class StorkIntegrationTest {
@RegisterExtension
static final QuarkusUnitTest TEST = new QuarkusUnitTest()
.withApplicationRoot((jar) -> jar
.addClasses(HelloClient2.class, HelloResource.class))
.addClasses(HelloClient.class, HelloResource.class))
.withConfigurationResource("stork-application.properties");

@RestClient
HelloClient2 client;
HelloClient client;

@Test
void shouldDetermineUrlViaStork() {
String greeting = RestClientBuilder.newBuilder().baseUri(URI.create("stork://hello-service/hello"))
.build(HelloClient2.class)
.build(HelloClient.class)
.echo("black and white bird");
assertThat(greeting).isEqualTo("hello, black and white bird");

greeting = RestClientBuilder.newBuilder().baseUri(URI.create("stork://hello-service/hello"))
.build(HelloClient.class)
.helloWithPathParam("black and white bird");
assertThat(greeting).isEqualTo("Hello, black and white bird");
}

@Test
void shouldDetermineUrlViaStorkWhenUsingTarget() throws URISyntaxException {
String greeting = ClientBuilder.newClient().target("stork://hello-service/hello").request().get(String.class);
assertThat(greeting).isEqualTo("Hello");
String greeting = ClientBuilder.newClient().target("stork://hello-service/hello").request()
.get(String.class);
assertThat(greeting).isEqualTo("Hello, World!");

greeting = ClientBuilder.newClient().target(new URI("stork://hello-service/hello")).request().get(String.class);
assertThat(greeting).isEqualTo("Hello");
assertThat(greeting).isEqualTo("Hello, World!");

greeting = ClientBuilder.newClient().target(UriBuilder.fromUri("stork://hello-service/hello")).request()
.get(String.class);
assertThat(greeting).isEqualTo("Hello");
assertThat(greeting).isEqualTo("Hello, World!");

greeting = ClientBuilder.newClient().target("stork://hello-service/hello").path("big bird").request()
.get(String.class);
assertThat(greeting).isEqualTo("Hello, big bird");
}

@Test
void shouldDetermineUrlViaStorkCDI() {
String greeting = client.echo("big bird");
assertThat(greeting).isEqualTo("hello, big bird");

greeting = client.helloWithPathParam("big bird");
assertThat(greeting).isEqualTo("Hello, big bird");
}

@Test
@Timeout(20)
void shouldFailOnUnknownService() {
HelloClient2 client2 = RestClientBuilder.newBuilder()
HelloClient client = RestClientBuilder.newBuilder()
.baseUri(URI.create("stork://nonexistent-service"))
.build(HelloClient2.class);
assertThatThrownBy(() -> client2.echo("foo")).isInstanceOf(NoSuchServiceDefinitionException.class);
.build(HelloClient.class);
assertThatThrownBy(() -> client.echo("foo")).isInstanceOf(NoSuchServiceDefinitionException.class);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@
import com.github.tomakehurst.wiremock.WireMockServer;
import com.github.tomakehurst.wiremock.client.WireMock;

import io.quarkus.rest.client.reactive.HelloClient2;
import io.quarkus.rest.client.reactive.HelloResource;
import io.quarkus.test.QuarkusUnitTest;

public class StorkResponseTimeLoadBalancerTest {
Expand All @@ -28,7 +26,7 @@ public class StorkResponseTimeLoadBalancerTest {
@RegisterExtension
static final QuarkusUnitTest TEST = new QuarkusUnitTest()
.withApplicationRoot((jar) -> jar
.addClasses(HelloClient2.class, HelloResource.class))
.addClasses(HelloClient.class, HelloResource.class))
.withConfigurationResource("stork-stat-lb.properties");

@BeforeAll
Expand All @@ -46,7 +44,7 @@ public static void shutDown() {
}

@RestClient
HelloClient2 client;
HelloClient client;

@Test
void shouldUseFasterService() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,54 +15,64 @@
import org.junit.jupiter.api.Timeout;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.rest.client.reactive.HelloClient2;
import io.quarkus.rest.client.reactive.HelloResource;
import io.quarkus.test.QuarkusUnitTest;
import io.smallrye.stork.api.NoSuchServiceDefinitionException;

public class StorkWithPathIntegrationTest {
@RegisterExtension
static final QuarkusUnitTest TEST = new QuarkusUnitTest()
.withApplicationRoot((jar) -> jar
.addClasses(HelloClient2.class, HelloResource.class))
.addClasses(HelloClient.class, HelloResource.class))
.withConfigurationResource("stork-application-with-path.properties");

@RestClient
HelloClient2 client;
HelloClient client;

@Test
void shouldDetermineUrlViaStork() {
String greeting = RestClientBuilder.newBuilder().baseUri(URI.create("stork://hello-service"))
.build(HelloClient2.class)
.build(HelloClient.class)
.echo("black and white bird");
assertThat(greeting).isEqualTo("hello, black and white bird");

greeting = RestClientBuilder.newBuilder().baseUri(URI.create("stork://hello-service"))
.build(HelloClient.class)
.helloWithPathParam("black and white bird");
assertThat(greeting).isEqualTo("Hello, black and white bird");
}

@Test
void shouldDetermineUrlViaStorkWhenUsingTarget() throws URISyntaxException {
String greeting = ClientBuilder.newClient().target("stork://hello-service").request().get(String.class);
assertThat(greeting).isEqualTo("Hello");
assertThat(greeting).isEqualTo("Hello, World!");

greeting = ClientBuilder.newClient().target(new URI("stork://hello-service")).request().get(String.class);
assertThat(greeting).isEqualTo("Hello");
assertThat(greeting).isEqualTo("Hello, World!");

greeting = ClientBuilder.newClient().target(UriBuilder.fromUri("stork://hello-service/")).request()
.get(String.class);
assertThat(greeting).isEqualTo("Hello");
assertThat(greeting).isEqualTo("Hello, World!");

greeting = ClientBuilder.newClient().target("stork://hello-service/").path("big bird").request()
.get(String.class);
assertThat(greeting).isEqualTo("Hello, big bird");
}

@Test
void shouldDetermineUrlViaStorkCDI() {
String greeting = client.echo("big bird");
assertThat(greeting).isEqualTo("hello, big bird");

greeting = client.helloWithPathParam("big bird");
assertThat(greeting).isEqualTo("Hello, big bird");
}

@Test
@Timeout(20)
void shouldFailOnUnknownService() {
HelloClient2 client2 = RestClientBuilder.newBuilder()
HelloClient client = RestClientBuilder.newBuilder()
.baseUri(URI.create("stork://nonexistent-service"))
.build(HelloClient2.class);
assertThatThrownBy(() -> client2.echo("foo")).isInstanceOf(NoSuchServiceDefinitionException.class);
.build(HelloClient.class);
assertThatThrownBy(() -> client.echo("foo")).isInstanceOf(NoSuchServiceDefinitionException.class);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import jakarta.annotation.Priority;
import jakarta.ws.rs.Priorities;
import jakarta.ws.rs.core.GenericType;
import jakarta.ws.rs.core.UriBuilder;
import jakarta.ws.rs.ext.Provider;

import org.jboss.logging.Logger;
Expand Down Expand Up @@ -62,7 +63,7 @@ public void filter(ResteasyReactiveClientRequestContext requestContext) {
}
// Service instance can also contain an optional path.
Optional<String> path = instance.getPath();
String actualPath = uri.getPath();
String actualPath = uri.getRawPath();
if (path.isPresent()) {
var p = path.get();
if (!p.startsWith("/")) {
Expand All @@ -79,11 +80,12 @@ public void filter(ResteasyReactiveClientRequestContext requestContext) {
}
}
}

//To avoid the path double encoding we create uri with path=null and set the path after
URI newUri = new URI(scheme,
uri.getUserInfo(), host, port,
actualPath, uri.getQuery(), uri.getFragment());
requestContext.setUri(newUri);
null, uri.getQuery(), uri.getFragment());
URI build = UriBuilder.fromUri(newUri).path(actualPath).build();
requestContext.setUri(build);
if (measureTime && instance.gatherStatistics()) {
requestContext.setCallStatsCollector(instance);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

import jakarta.ws.rs.Consumes;
import jakarta.ws.rs.GET;
import jakarta.ws.rs.Path;
import jakarta.ws.rs.PathParam;
import jakarta.ws.rs.Produces;
import jakarta.ws.rs.core.MediaType;

import org.eclipse.microprofile.rest.client.inject.RegisterRestClient;
Expand All @@ -11,4 +14,9 @@ public interface Client {
@GET
@Consumes(MediaType.TEXT_PLAIN)
String echo(String name);

@GET
@Path("/v2/{name}")
@Produces(MediaType.TEXT_PLAIN)
String invoke(@PathParam("name") String name);
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
import jakarta.enterprise.context.ApplicationScoped;
import jakarta.ws.rs.GET;
import jakarta.ws.rs.Path;
import jakarta.ws.rs.PathParam;
import jakarta.ws.rs.Produces;
import jakarta.ws.rs.core.MediaType;

import org.eclipse.microprofile.rest.client.inject.RestClient;

Expand All @@ -17,4 +20,11 @@ public class ClientCallingResource {
public String passThrough() {
return client.echo("World!");
}

@GET
@Path("/{name}")
@Produces(MediaType.TEXT_PLAIN)
public String invoke(@PathParam("name") String name) {
return client.invoke(name + "/" + name);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ int httpsPort() {
protected Map<String, String> initWireMock(WireMockServer server) {
server.stubFor(WireMock.get("/hello")
.willReturn(aResponse().withBody(FAST_RESPONSE).withStatus(200)));
server.stubFor(WireMock.get("/hello/v2/quarkus%2Fquarkus")
.willReturn(aResponse().withBody(FAST_RESPONSE).withStatus(200)));
return Map.of("fast-service", "localhost:8443");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -54,4 +54,17 @@ void shouldUseFasterService() {
// after hitting the slow endpoint, we should only use the fast one:
assertThat(responses).containsOnly(FAST_RESPONSE, FAST_RESPONSE, FAST_RESPONSE);
}

@Test
void shouldUseV2Service() {
Set<String> responses = new HashSet<>();

for (int i = 0; i < 2; i++) {
Response response = when().get("/client/quarkus");
response.then().statusCode(200);
}

responses.clear();

}
}
Loading
Loading