Skip to content

Commit

Permalink
Reduce HttpProblem dependency on Response.StatusType to the minimum (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
lwitkowski authored Oct 29, 2023
1 parent dab4cd2 commit 6e71fff
Show file tree
Hide file tree
Showing 13 changed files with 36 additions and 42 deletions.
10 changes: 5 additions & 5 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ or add your own suggestions and start discussion. Then create fork or branch and
Quarkus has a strictly enforced code style, so as this extension. Code formatting is done by the Eclipse code formatter, using the config files
found in the `ide-config` directory. By default when you run `./mvnw install` the code will be formatted automatically.

If you want to run the formatting without doing a full build, you can run `./mvnw process-sources`.
If you want to run the formatting without doing a full build, you can run `./mvnw process-sources` or `./mvnw formatter:format`.

More details on how to setup your ide can be found in official [Quarkus Contributing guide](https://github.com/quarkusio/quarkus/blob/main/CONTRIBUTING.md#ide-config-and-code-style)

Expand All @@ -26,10 +26,10 @@ Everything in this extension revolves around `ExceptionMapperBase` abstract clas
exception mappers extending this class.

Exception lifecycle:
1. JaxRS implementation (RESTeasy) catches exception, and immediately looks for best matching `ExceptionMapper`. Hopefully it will be one of our mappers :)
2. `ExceptionMapperBase::toResponse` method is called, where original exception is turned into `HttpProblem` object by a specific subclass mapper (e.g `WebApplicationExceptionMapper`).
3. `Problem` goes into post-processing phase to apply logging, metrics generation, MCD properties injection etc.
4. Enhanced `Problem` is turned into JaxRS `Response` object, with `Problem` object placed as entity (response body). This is where `ExceptionMapper` work is finished.
1. JaxRS/JakartaRS implementation (RESTeasy) catches exception, and immediately looks for best matching `ExceptionMapper`. Hopefully it will be one of our mappers :)
2. `ExceptionMapperBase::toResponse` method is called, where original exception is turned into `HttpProblem` object by the specific subclass mapper (e.g `WebApplicationExceptionMapper`).
3. `HttpProblem` goes into post-processing phase to apply logging, metrics generation, MCD properties injection etc.
4. Enhanced `HttpProblem` is turned into JaxRS/JakartaRS `Response` object, with `HttpProblem` object placed as entity (response body). This is where `ExceptionMapper` work is finished.
5. RESTeasy serializes `Response` object into raw HTTP response, with little help from our JSON serializer (either `JacksonProblemSerializer` or `JsonBProblemSerializer`)
6. End user gets nice `application/problem+json` HTTP response.

Expand Down
2 changes: 1 addition & 1 deletion deployment/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<parent>
<groupId>com.tietoevry.quarkus</groupId>
<artifactId>quarkus-resteasy-problem-parent</artifactId>
<version>3.0.1-SNAPSHOT</version>
<version>3.1.0-SNAPSHOT</version>
</parent>

<artifactId>quarkus-resteasy-problem-deployment</artifactId>
Expand Down
4 changes: 2 additions & 2 deletions integration-test/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@
<parent>
<groupId>com.tietoevry.quarkus</groupId>
<artifactId>quarkus-resteasy-problem-parent</artifactId>
<version>3.0.1-SNAPSHOT</version>
<version>3.1.0-SNAPSHOT</version>
</parent>

<artifactId>quarkus-resteasy-problem-integration-test</artifactId>
<name>Quarkus - RESTeasy - Problem - Integration Tests</name>

<properties>
<quarkus-resteasy-problem.version>3.0.1-SNAPSHOT</quarkus-resteasy-problem.version>
<quarkus-resteasy-problem.version>3.1.0-SNAPSHOT</quarkus-resteasy-problem.version>
</properties>

<dependencies>
Expand Down
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

<groupId>com.tietoevry.quarkus</groupId>
<artifactId>quarkus-resteasy-problem-parent</artifactId>
<version>3.0.1-SNAPSHOT</version>
<version>3.1.0-SNAPSHOT</version>
<packaging>pom</packaging>

<name>Quarkus - RESTeasy - Problem - Parent</name>
Expand Down
2 changes: 1 addition & 1 deletion runtime/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<parent>
<groupId>com.tietoevry.quarkus</groupId>
<artifactId>quarkus-resteasy-problem-parent</artifactId>
<version>3.0.1-SNAPSHOT</version>
<version>3.1.0-SNAPSHOT</version>
</parent>

<artifactId>quarkus-resteasy-problem</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import jakarta.ws.rs.core.Response;
import jakarta.ws.rs.core.UriInfo;
import jakarta.ws.rs.ext.ExceptionMapper;
import java.util.Objects;

/**
* Base class for all ExceptionMappers in this extension, takes care of mapping Exceptions to Problems, triggering
Expand All @@ -22,8 +21,6 @@ public abstract class ExceptionMapperBase<E extends Throwable> implements Except
@Override
public final Response toResponse(E exception) {
HttpProblem problem = toProblem(exception);
Objects.requireNonNull(problem.getStatus(), "Status must not be null");

ProblemContext context = ProblemContext.of(exception, uriInfo);
HttpProblem finalProblem = postProcessorsRegistry.applyPostProcessing(problem, context);
return finalProblem.toResponse();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
package com.tietoevry.quarkus.resteasy.problem;

import static jakarta.ws.rs.core.Response.Status.INTERNAL_SERVER_ERROR;

import jakarta.ws.rs.core.MediaType;
import jakarta.ws.rs.core.Response;
import java.net.URI;
Expand All @@ -28,7 +26,7 @@ public class HttpProblem extends RuntimeException {

private final URI type;
private final String title;
private final Response.StatusType status;
private final int statusCode;
private final String detail;
private final URI instance;
private final Map<String, Object> parameters;
Expand All @@ -39,7 +37,7 @@ protected HttpProblem(Builder builder) {

this.type = builder.type;
this.title = builder.title;
this.status = Optional.ofNullable(builder.status).orElse(INTERNAL_SERVER_ERROR);
this.statusCode = builder.statusCode;
this.detail = builder.detail;
this.instance = builder.instance;
this.parameters = Collections.unmodifiableMap(Optional.ofNullable(builder.parameters).orElseGet(LinkedHashMap::new));
Expand Down Expand Up @@ -82,7 +80,7 @@ public static Builder builder(HttpProblem original) {
.withType(original.getType())
.withInstance(original.getInstance())
.withTitle(original.getTitle())
.withStatus(original.getStatus())
.withStatus(original.getStatusCode())
.withDetail(original.getDetail());
original.parameters.forEach(builder::with);
original.headers.forEach(builder::withHeader);
Expand All @@ -97,8 +95,15 @@ public String getTitle() {
return this.title;
}

public int getStatusCode() {
return this.statusCode;
}

/**
* Returns null if status code has no representation in Response.Status enum.
*/
public Response.StatusType getStatus() {
return this.status;
return Response.Status.fromStatusCode(statusCode);
}

public String getDetail() {
Expand All @@ -118,10 +123,8 @@ public Map<String, Object> getHeaders() {
}

public Response toResponse() {
Objects.requireNonNull(getStatus());

Response.ResponseBuilder builder = Response
.status(getStatus().getStatusCode())
.status(getStatusCode())
.type(HttpProblem.MEDIA_TYPE)
.entity(this);

Expand All @@ -137,7 +140,7 @@ public static class Builder {

private URI type;
private String title;
private Response.StatusType status;
private int statusCode = 500;
private String detail;
private URI instance;
private final Map<String, Object> headers = new LinkedHashMap<>();
Expand All @@ -156,13 +159,14 @@ public Builder withTitle(@Nullable String title) {
return this;
}

public Builder withStatus(@Nullable Response.StatusType status) {
this.status = status;
public Builder withStatus(Response.StatusType status) {
Objects.requireNonNull(status);
this.statusCode = status.getStatusCode();
return this;
}

public Builder withStatus(int statusCode) {
this.status = Response.Status.fromStatusCode(statusCode);
this.statusCode = statusCode;
return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,8 @@ public void serialize(final HttpProblem problem, final JsonGenerator json, final
if (problem.getType() != null) {
json.writeStringField("type", problem.getType().toASCIIString());
}
if (problem.getStatus() != null) {
json.writeNumberField("status", problem.getStatus().getStatusCode());
}
json.writeNumberField("status", problem.getStatusCode());

if (problem.getTitle() != null) {
json.writeStringField("title", problem.getTitle());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,8 @@ public void serialize(HttpProblem problem, JsonGenerator generator, Serializatio
if (problem.getType() != null) {
generator.write("type", problem.getType().toASCIIString());
}
if (problem.getStatus() != null) {
generator.write("status", problem.getStatus().getStatusCode());
}
generator.write("status", problem.getStatusCode());

if (problem.getTitle() != null) {
generator.write("title", problem.getTitle());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import com.tietoevry.quarkus.resteasy.problem.HttpProblem;
import io.smallrye.metrics.MetricRegistries;
import java.util.Objects;
import org.eclipse.microprofile.metrics.Metadata;
import org.eclipse.microprofile.metrics.MetricRegistry;
import org.eclipse.microprofile.metrics.Tag;
Expand Down Expand Up @@ -45,9 +44,7 @@ public int priority() {

@Override
public HttpProblem apply(HttpProblem problem, ProblemContext context) {
Objects.requireNonNull(problem.getStatus());

Tag tag = new Tag(STATUS_TAG, String.valueOf(problem.getStatus().getStatusCode()));
Tag tag = new Tag(STATUS_TAG, String.valueOf(problem.getStatusCode()));
registry.counter(METRIC_NAME, tag).inc();

return problem;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,7 @@ public HttpProblem apply(HttpProblem problem, ProblemContext context) {
return problem;
}

Objects.requireNonNull(problem.getStatus());

if (problem.getStatus().getStatusCode() >= 500) {
if (problem.getStatusCode() >= 500) {
if (logger.isErrorEnabled()) {
logger.error(serialize(problem), context.cause);
}
Expand All @@ -42,7 +40,7 @@ public HttpProblem apply(HttpProblem problem, ProblemContext context) {

private String serialize(HttpProblem problem) {
Stream<String> basicFields = Stream.of(
(problem.getStatus() == null) ? null : ("status=" + problem.getStatus().getStatusCode()),
("status=" + problem.getStatusCode()),
(problem.getTitle() == null) ? null : ("title=\"" + problem.getTitle() + "\""),
(problem.getDetail() == null) ? null : ("detail=\"" + problem.getDetail() + "\""),
(problem.getInstance() == null) ? null : ("instance=\"" + problem.getInstance() + "\""),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ void builderShouldPassAllFields() {

assertThat(problem.getType()).hasHost("tietoevry.com").hasPath("/problem");
assertThat(problem.getInstance()).hasPath("/endpoint");
assertThat(problem.getStatus().getStatusCode()).isEqualTo(400);
assertThat(problem.getStatusCode()).isEqualTo(400);
assertThat(problem.getDetail()).isEqualTo("Deep down wrongness, zażółć gęślą jaźń for Håkensth");
assertThat(problem.getHeaders())
.containsEntry("X-Numeric-Header", 123)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ void shouldPreserveAllFields() {
HttpProblem enhancedProblem = processor.apply(badRequestProblem(), simpleContext());

assertThat(enhancedProblem.getTitle()).isEqualTo("There's something wrong with your request");
assertThat(enhancedProblem.getStatus()).isEqualTo(BAD_REQUEST);
assertThat(enhancedProblem.getStatusCode()).isEqualTo(BAD_REQUEST.getStatusCode());
}

@Test
Expand Down

0 comments on commit 6e71fff

Please sign in to comment.