From bc0110cc29277d342e979cebe24e1e24bb7c6a05 Mon Sep 17 00:00:00 2001 From: Jorge Bornhausen Date: Thu, 24 Jul 2025 11:11:53 +0200 Subject: [PATCH] chore: more sonarqube improvements, rename tracing service method --- .../DefaultRevisionContextProviderTest.java | 1 - .../quarkus/commons/audit/RevisionTest.java | 5 +- .../commons/tracing/TracingService.java | 2 +- .../commons/tracing/TracingServiceImpl.java | 2 +- .../commons/tracing/RoutePatternTest.java | 205 ++++-------------- .../tracing/TracingServiceImplTest.java | 98 +++++++++ .../tracing/resource/SlashResource.java | 12 +- 7 files changed, 151 insertions(+), 174 deletions(-) create mode 100644 quarkus-tracing-service/src/test/java/ch/phoenix/oss/quarkus/commons/tracing/TracingServiceImplTest.java diff --git a/quarkus-audit-tools/src/test/java/ch/phoenix/oss/quarkus/commons/audit/DefaultRevisionContextProviderTest.java b/quarkus-audit-tools/src/test/java/ch/phoenix/oss/quarkus/commons/audit/DefaultRevisionContextProviderTest.java index 5ad9ff7..8ef3a73 100644 --- a/quarkus-audit-tools/src/test/java/ch/phoenix/oss/quarkus/commons/audit/DefaultRevisionContextProviderTest.java +++ b/quarkus-audit-tools/src/test/java/ch/phoenix/oss/quarkus/commons/audit/DefaultRevisionContextProviderTest.java @@ -1,7 +1,6 @@ package ch.phoenix.oss.quarkus.commons.audit; import static org.assertj.core.api.AssertionsForInterfaceTypes.assertThat; -import static org.junit.jupiter.api.Assertions.*; import static org.mockito.Mockito.mockStatic; import io.quarkus.test.junit.QuarkusTest; diff --git a/quarkus-audit-tools/src/test/java/ch/phoenix/oss/quarkus/commons/audit/RevisionTest.java b/quarkus-audit-tools/src/test/java/ch/phoenix/oss/quarkus/commons/audit/RevisionTest.java index 61c3743..d0f5d1f 100644 --- a/quarkus-audit-tools/src/test/java/ch/phoenix/oss/quarkus/commons/audit/RevisionTest.java +++ b/quarkus-audit-tools/src/test/java/ch/phoenix/oss/quarkus/commons/audit/RevisionTest.java @@ -1,7 +1,6 @@ package ch.phoenix.oss.quarkus.commons.audit; import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.jupiter.api.Assertions.*; import io.quarkus.test.junit.QuarkusTest; import org.junit.jupiter.api.Test; @@ -47,8 +46,6 @@ class RevisionTest { var rev = new Revision(); rev.rev = 1; - assertThat(rev.toString()) - .as("Revision's toString should match expected value") - .isEqualTo("Revision{rev=1}"); + assertThat(rev).as("Revision's toString should match expected value").hasToString("Revision{rev=1}"); } } diff --git a/quarkus-tracing-service/src/main/java/ch/phoenix/oss/quarkus/commons/tracing/TracingService.java b/quarkus-tracing-service/src/main/java/ch/phoenix/oss/quarkus/commons/tracing/TracingService.java index 2b1ac38..812867d 100644 --- a/quarkus-tracing-service/src/main/java/ch/phoenix/oss/quarkus/commons/tracing/TracingService.java +++ b/quarkus-tracing-service/src/main/java/ch/phoenix/oss/quarkus/commons/tracing/TracingService.java @@ -8,7 +8,7 @@ public interface TracingService { String getActor(); - String getRequestPath(); + String getRequestPathRaw(); String getRequestMethod(); diff --git a/quarkus-tracing-service/src/main/java/ch/phoenix/oss/quarkus/commons/tracing/TracingServiceImpl.java b/quarkus-tracing-service/src/main/java/ch/phoenix/oss/quarkus/commons/tracing/TracingServiceImpl.java index b5f295b..d46f684 100644 --- a/quarkus-tracing-service/src/main/java/ch/phoenix/oss/quarkus/commons/tracing/TracingServiceImpl.java +++ b/quarkus-tracing-service/src/main/java/ch/phoenix/oss/quarkus/commons/tracing/TracingServiceImpl.java @@ -33,7 +33,7 @@ class TracingServiceImpl implements TracingService { } @Override - public String getRequestPath() { + public String getRequestPathRaw() { return (String) MDC.get(TracingConstants.REQUEST_PATH_RAW); } diff --git a/quarkus-tracing-service/src/test/java/ch/phoenix/oss/quarkus/commons/tracing/RoutePatternTest.java b/quarkus-tracing-service/src/test/java/ch/phoenix/oss/quarkus/commons/tracing/RoutePatternTest.java index 11f1689..156c0a4 100644 --- a/quarkus-tracing-service/src/test/java/ch/phoenix/oss/quarkus/commons/tracing/RoutePatternTest.java +++ b/quarkus-tracing-service/src/test/java/ch/phoenix/oss/quarkus/commons/tracing/RoutePatternTest.java @@ -1,5 +1,6 @@ package ch.phoenix.oss.quarkus.commons.tracing; +import static org.junit.jupiter.params.provider.Arguments.arguments; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.startsWith; import static org.mockito.Mockito.verify; @@ -10,7 +11,11 @@ import io.quarkus.test.junit.mockito.InjectSpy; import io.restassured.RestAssured; import io.restassured.http.ContentType; import java.util.Map; +import java.util.stream.Stream; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; @QuarkusTest class RoutePatternTest { @@ -18,12 +23,46 @@ class RoutePatternTest { @InjectSpy TracingService tracingService; - @Test - void getBlankResource() { - var route = "/"; - RestAssured.given().accept(ContentType.TEXT).when().get(route).then().statusCode(200); + static Stream get() { + return Stream.of( + arguments("/", Map.of()), + arguments("/leading-and-no-trailing", Map.of()), + arguments("/leading/{param}/{param2}", Map.of("param", "1", "param2", "2")), + arguments("/{param}/{param2}/trailing", Map.of("param", "1", "param2", "2")), + arguments("/leading-and-no-trailing/{param}", Map.of("param", "1")), + arguments("/leading-and-no-trailing/{param}/{param2}", Map.of("param", "1", "param2", "2")), + arguments("/leading-and-trailing", Map.of()), + arguments("/leading-and-trailing/{param}", Map.of("param", "1")), + arguments("/leading-and-trailing/{param}/{param2}", Map.of("param", "1", "param2", "2")), + arguments("/no-leading-and-no-trailing", Map.of()), + arguments("/no-leading-and-no-trailing/{param}", Map.of("param", "1")), + arguments("/no-leading-and-no-trailing/{param}/{param2}", Map.of("param", "1", "param2", "2")), + arguments("/no-leading-and-trailing", Map.of()), + arguments("/no-leading-and-trailing/{param}", Map.of("param", "1")), + arguments("/no-leading-and-trailing/{param}/{param2}", Map.of("param", "1", "param2", "2"))); + } - verifyGetTracing(route, Map.of()); + @MethodSource + @ParameterizedTest + void get(String route, Map pathParams) { + RestAssured.given() + .accept(ContentType.TEXT) + .when() + .get(route, pathParams) + .then() + .statusCode(200); + + verify(tracingService).trace("actor", "anonymous"); + verify(tracingService).trace("request.method", "GET"); + verify(tracingService).trace("request.route", route); + pathParams.forEach((key, value) -> verify(tracingService).trace("request.path.params." + key, value)); + verify(tracingService).trace("request.headers.accept", "text/plain"); + verify(tracingService).trace("request.headers.accept-encoding", "gzip,deflate"); + verify(tracingService).trace("request.headers.connection", "Keep-Alive"); + verify(tracingService).trace(eq("request.headers.host"), startsWith("localhost:")); + verify(tracingService).trace(eq("request.headers.user-agent"), startsWith("Apache-HttpClient")); + verify(tracingService).trace("request.client.ip", "127.0.0.1"); + verifyNoMoreInteractions(tracingService); } @Test @@ -45,160 +84,4 @@ class RoutePatternTest { verify(tracingService).trace("request.client.ip", "127.0.0.1"); verifyNoMoreInteractions(tracingService); } - - @Test - void getLeadingResource() { - var route = "/leading/{id}/{anotherId}"; - RestAssured.given() - .accept(ContentType.TEXT) - .when() - .get(route, 1, 2) - .then() - .statusCode(200); - - verifyGetTracing(route, Map.of("id", "1", "anotherId", "2")); - } - - @Test - void getTrailingResource() { - var route = "/{id}/{anotherId}/trailing"; - RestAssured.given() - .accept(ContentType.TEXT) - .when() - .get(route, 1, 2) - .then() - .statusCode(200); - - verifyGetTracing(route, Map.of("id", "1", "anotherId", "2")); - } - - @Test - void getLeadingAndNoTrailingResource() { - var route = "/leading-and-no-trailing"; - RestAssured.given().accept(ContentType.TEXT).when().get(route).then().statusCode(200); - - verifyGetTracing(route, Map.of()); - } - - @Test - void getLeadingAndNoTrailingWithSingleParamResource() { - var route = "/leading-and-no-trailing/{param}"; - RestAssured.given().accept(ContentType.TEXT).when().get(route, 1).then().statusCode(200); - - verifyGetTracing(route, Map.of("param", "1")); - } - - @Test - void getLeadingAndNoTrailingWithMultiParamResource() { - var route = "/leading-and-no-trailing/{param}/{param2}"; - RestAssured.given() - .accept(ContentType.TEXT) - .when() - .get(route, 1, 2) - .then() - .statusCode(200); - - verifyGetTracing(route, Map.of("param", "1", "param2", "2")); - } - - @Test - void getLeadingAndTrailingResource() { - var route = "/leading-and-trailing"; - RestAssured.given().accept(ContentType.TEXT).when().get(route).then().statusCode(200); - - verifyGetTracing(route, Map.of()); - } - - @Test - void getLeadingAndTrailingWithSingleParamResource() { - var route = "/leading-and-trailing/{param}"; - RestAssured.given().accept(ContentType.TEXT).when().get(route, 1).then().statusCode(200); - - verifyGetTracing(route, Map.of("param", "1")); - } - - @Test - void getLeadingAndTrailingWithMultiParamResource() { - var route = "/leading-and-trailing/{param}/{param2}"; - RestAssured.given() - .accept(ContentType.TEXT) - .when() - .get(route, 1, 2) - .then() - .statusCode(200); - - verifyGetTracing(route, Map.of("param", "1", "param2", "2")); - } - - @Test - void getNoLeadingAndNoTrailingResource() { - var route = "/no-leading-and-no-trailing"; - RestAssured.given().accept(ContentType.TEXT).when().get(route).then().statusCode(200); - - verifyGetTracing(route, Map.of()); - } - - @Test - void geNoLeadingAndNoTrailingWithSingleParamResource() { - var route = "/no-leading-and-no-trailing/{param}"; - RestAssured.given().accept(ContentType.TEXT).when().get(route, 1).then().statusCode(200); - - verifyGetTracing(route, Map.of("param", "1")); - } - - @Test - void getNoLeadingAndNoTrailingWithMultiParamResource() { - var route = "/no-leading-and-no-trailing/{param}/{param2}"; - RestAssured.given() - .accept(ContentType.TEXT) - .when() - .get(route, 1, 2) - .then() - .statusCode(200); - - verifyGetTracing(route, Map.of("param", "1", "param2", "2")); - } - - @Test - void getNoLeadingAndTrailingResource() { - var route = "/no-leading-and-trailing"; - RestAssured.given().accept(ContentType.TEXT).when().get(route).then().statusCode(200); - - verifyGetTracing(route, Map.of()); - } - - @Test - void getNoLeadingAndTrailingWithSingleParamResource() { - var route = "/no-leading-and-trailing/{param}"; - RestAssured.given().accept(ContentType.TEXT).when().get(route, 1).then().statusCode(200); - - verifyGetTracing(route, Map.of("param", "1")); - } - - @Test - void getNoLeadingAndTrailingWithMultiParamResource() { - var route = "/no-leading-and-trailing/{param}/{param2}"; - RestAssured.given() - .accept(ContentType.TEXT) - .when() - .get(route, 1, 2) - .then() - .statusCode(200); - - verifyGetTracing(route, Map.of("param", "1", "param2", "2")); - } - - private void verifyGetTracing(String route, Map pathParams) { - verify(tracingService).trace("actor", "anonymous"); - verify(tracingService).trace("request.method", "GET"); - verify(tracingService).trace("request.route", route); - pathParams.forEach((key, value) -> verify(tracingService).trace("request.path.params." + key, value)); - verify(tracingService).trace("request.headers.accept", "text/plain"); - verify(tracingService).trace("request.headers.accept-encoding", "gzip,deflate"); - verify(tracingService).trace("request.headers.connection", "Keep-Alive"); - verify(tracingService).trace(eq("request.headers.host"), startsWith("localhost:")); - verify(tracingService).trace(eq("request.headers.user-agent"), startsWith("Apache-HttpClient")); - verify(tracingService).trace("request.client.ip", "127.0.0.1"); - verifyNoMoreInteractions(tracingService); - } } diff --git a/quarkus-tracing-service/src/test/java/ch/phoenix/oss/quarkus/commons/tracing/TracingServiceImplTest.java b/quarkus-tracing-service/src/test/java/ch/phoenix/oss/quarkus/commons/tracing/TracingServiceImplTest.java new file mode 100644 index 0000000..f728e14 --- /dev/null +++ b/quarkus-tracing-service/src/test/java/ch/phoenix/oss/quarkus/commons/tracing/TracingServiceImplTest.java @@ -0,0 +1,98 @@ +package ch.phoenix.oss.quarkus.commons.tracing; + +import static org.assertj.core.api.Assertions.assertThat; + +import io.opentelemetry.api.trace.Span; +import io.opentelemetry.instrumentation.annotations.WithSpan; +import io.quarkus.test.junit.QuarkusTest; +import jakarta.inject.Inject; +import org.jboss.logging.MDC; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +@QuarkusTest +class TracingServiceImplTest { + + @Inject + TracingService tracingService; + + @Inject + Span span; + + @BeforeEach + void setUp() { + MDC.clear(); + } + + @Test + void getActor() { + tracingService.trace("actor", "abc"); + assertThat(tracingService.getActor()) + .as("Actor should match expected value") + .isEqualTo("abc"); + } + + @Test + void getRequestPathRaw() { + tracingService.trace("request.path.raw", "/foo/bar"); + assertThat(tracingService.getRequestPathRaw()) + .as("Request Path Raw should match expected value") + .isEqualTo("/foo/bar"); + } + + @Test + void getRequestMethod() { + tracingService.trace("request.method", "GET"); + assertThat(tracingService.getRequestMethod()) + .as("Request Method should match expected value") + .isEqualTo("GET"); + } + + @Test + void getRequestId() { + tracingService.trace("request.headers.x-request-id", "ba458367-bfeb-46ba-87da-50b9343be8f9"); + assertThat(tracingService.getRequestId()) + .as("Request Id should match expected value") + .isEqualTo("ba458367-bfeb-46ba-87da-50b9343be8f9"); + } + + @Test + @WithSpan + void getTraceId() { + assertThat(tracingService.getTraceId()) + .as("Request Trace Id should match expected value") + .isEqualTo(span.getSpanContext().getTraceId()); + } + + @Test + @WithSpan + void getSpanId() { + assertThat(tracingService.getSpanId()) + .as("Request Span Id should match expected value") + .isEqualTo(span.getSpanContext().getSpanId()); + } + + @Test + void getClientIp() { + tracingService.trace("request.client.ip", "127.0.0.1"); + assertThat(tracingService.getClientIp()) + .as("Request Client Iü should match expected value") + .isEqualTo("127.0.0.1"); + } + + @Test + void getSchedulerJob() { + tracingService.trace("scheduler.job.name", "scheduler/abc"); + assertThat(tracingService.getSchedulerJob()) + .as("Scheduler Job Name should match expected value") + .isEqualTo("scheduler/abc"); + } + + @Test + void clearAll() { + tracingService.trace("aaa", "bbb"); + assertThat(MDC.get("aaa")).isEqualTo("bbb"); + tracingService.clearAll(); + assertThat(MDC.get("aaa")).isNull(); + } +} diff --git a/quarkus-tracing-service/src/test/java/ch/phoenix/oss/quarkus/commons/tracing/resource/SlashResource.java b/quarkus-tracing-service/src/test/java/ch/phoenix/oss/quarkus/commons/tracing/resource/SlashResource.java index 690c84a..8c8bdc7 100644 --- a/quarkus-tracing-service/src/test/java/ch/phoenix/oss/quarkus/commons/tracing/resource/SlashResource.java +++ b/quarkus-tracing-service/src/test/java/ch/phoenix/oss/quarkus/commons/tracing/resource/SlashResource.java @@ -10,14 +10,14 @@ import jakarta.ws.rs.core.MediaType; public class SlashResource { @GET - @Path("/leading/{id}/{anotherId}") - public String doubleLeading(int id, int anotherId) { - return "leading/" + id + "/" + anotherId; + @Path("/leading/{param}/{param2}") + public String doubleLeading(int param, int param2) { + return "leading/" + param + "/" + param2; } @GET - @Path("{id}/{anotherId}/trailing/") - public String doubleTrailing(int id, int anotherId) { - return id + "/" + anotherId + "/trailing"; + @Path("{param}/{param2}/trailing/") + public String doubleTrailing(int param, int param2) { + return param + "/" + param2 + "/trailing"; } }