From 39a699f1abb5e16afd2545bacf6a8692063c2b59 Mon Sep 17 00:00:00 2001 From: Nicolas Ayral Seydoux Date: Wed, 10 Apr 2024 14:50:44 +0200 Subject: [PATCH 01/25] JCL-403: SolidClient builds ProblemDetails `SolidClient` now parses HTTP error responses if they are compliant with RFC9457 in order to throw a structured exception. In case when no JSON parser is available in the classpath, the default behavior is to build a ProblemDetails object that only relies on the data available in the HTTP response status code. --- .../com/inrupt/client/solid/SolidClient.java | 70 +++- .../inrupt/client/solid/SolidClientTest.java | 385 +++++++++++++++++- 2 files changed, 418 insertions(+), 37 deletions(-) diff --git a/solid/src/main/java/com/inrupt/client/solid/SolidClient.java b/solid/src/main/java/com/inrupt/client/solid/SolidClient.java index 2b20c3a5105..4b7138ac8c6 100644 --- a/solid/src/main/java/com/inrupt/client/solid/SolidClient.java +++ b/solid/src/main/java/com/inrupt/client/solid/SolidClient.java @@ -20,17 +20,10 @@ */ package com.inrupt.client.solid; -import static java.nio.charset.StandardCharsets.UTF_8; - -import com.inrupt.client.Client; -import com.inrupt.client.ClientProvider; -import com.inrupt.client.Headers; -import com.inrupt.client.RDFSource; -import com.inrupt.client.Request; -import com.inrupt.client.Resource; -import com.inrupt.client.Response; -import com.inrupt.client.ValidationResult; +import com.inrupt.client.*; import com.inrupt.client.auth.Session; +import com.inrupt.client.spi.JsonService; +import com.inrupt.client.spi.ServiceProvider; import java.io.ByteArrayInputStream; import java.io.IOException; @@ -64,11 +57,22 @@ public class SolidClient { private final Client client; private final Headers defaultHeaders; private final boolean fetchAfterWrite; + private final JsonService jsonService; SolidClient(final Client client, final Headers headers, final boolean fetchAfterWrite) { this.client = Objects.requireNonNull(client, "Client may not be null!"); this.defaultHeaders = Objects.requireNonNull(headers, "Headers may not be null!"); this.fetchAfterWrite = fetchAfterWrite; + + // It is acceptable for a SolidClient instance to be in a classpath without any implementation for + // JsonService, in which case the ProblemDetails exceptions will fallback to default and not be parsed. + JsonService js; + try { + js = ServiceProvider.getJsonService(); + } catch (IllegalStateException e) { + js = null; + } + this.jsonService = js; } /** @@ -134,8 +138,19 @@ public CompletionStage read(final URI identifier, final return client.send(request, Response.BodyHandlers.ofByteArray()) .thenApply(response -> { if (response.statusCode() >= ERROR_STATUS) { - throw SolidClientException.handle("Unable to read resource at " + request.uri(), request.uri(), - response.statusCode(), response.headers(), new String(response.body())); + final ProblemDetails pd = ProblemDetails.fromErrorResponse( + response.statusCode(), + response.headers(), + response.body(), + this.jsonService + ); + throw SolidClientException.handle( + "Unable to read resource at " + request.uri(), + pd, + response.uri(), + response.headers(), + new String(response.body()) + ); } else { final String contentType = response.headers().firstValue(CONTENT_TYPE) .orElse("application/octet-stream"); @@ -284,8 +299,19 @@ public CompletionStage delete(final T resource, final if (isSuccess(res.statusCode())) { return null; } else { - throw SolidClientException.handle("Unable to delete resource", resource.getIdentifier(), - res.statusCode(), res.headers(), new String(res.body(), UTF_8)); + final ProblemDetails pd = ProblemDetails.fromErrorResponse( + res.statusCode(), + res.headers(), + res.body(), + this.jsonService + ); + throw SolidClientException.handle( + "Unable to delete resource", + pd, + resource.getIdentifier(), + res.headers(), + new String(res.body()) + ); } }); } @@ -371,8 +397,19 @@ Function, CompletionStage> handleRespon final Headers headers, final String message) { return res -> { if (!isSuccess(res.statusCode())) { - throw SolidClientException.handle(message, resource.getIdentifier(), - res.statusCode(), res.headers(), new String(res.body(), UTF_8)); + final ProblemDetails pd = ProblemDetails.fromErrorResponse( + res.statusCode(), + res.headers(), + res.body(), + this.jsonService + ); + throw SolidClientException.handle( + message, + pd, + resource.getIdentifier(), + res.headers(), + new String(res.body()) + ); } if (!fetchAfterWrite) { @@ -382,7 +419,6 @@ Function, CompletionStage> handleRespon @SuppressWarnings("unchecked") final Class clazz = (Class) resource.getClass(); return read(resource.getIdentifier(), headers, clazz); - }; } diff --git a/solid/src/test/java/com/inrupt/client/solid/SolidClientTest.java b/solid/src/test/java/com/inrupt/client/solid/SolidClientTest.java index fab400019f5..c7f51ba21b6 100644 --- a/solid/src/test/java/com/inrupt/client/solid/SolidClientTest.java +++ b/solid/src/test/java/com/inrupt/client/solid/SolidClientTest.java @@ -23,16 +23,16 @@ import static java.nio.charset.StandardCharsets.UTF_8; import static org.junit.jupiter.api.Assertions.*; -import com.inrupt.client.ClientProvider; -import com.inrupt.client.Headers; -import com.inrupt.client.Request; -import com.inrupt.client.Response; +import com.inrupt.client.*; import com.inrupt.client.auth.Session; +import com.inrupt.client.jackson.JacksonService; +import com.inrupt.client.spi.JsonService; import com.inrupt.client.spi.RDFFactory; import com.inrupt.client.util.URIBuilder; import com.inrupt.client.vocabulary.PIM; import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; import java.net.URI; @@ -57,6 +57,7 @@ class SolidClientTest { private static final Map config = new HashMap<>(); private static final RDF rdf = RDFFactory.getInstance(); private static final SolidClient client = SolidClient.getClient().session(Session.anonymous()); + private static final JsonService jsonService = new JacksonService(); @BeforeAll static void setup() { @@ -379,9 +380,10 @@ private static Stream testExceptionalResources() { @ParameterizedTest @MethodSource - void testSpecialisedExceptions( + void testLegacySpecialisedExceptions( final Class clazz, - final int statusCode + final int statusCode, + final String expectedStatusMessage ) { final Headers headers = Headers.of(Collections.singletonMap("x-key", Arrays.asList("value"))); final SolidClient solidClient = new SolidClient(ClientProvider.getClient(), headers, false); @@ -413,23 +415,366 @@ public int statusCode() { }) ); assertEquals(statusCode, exception.getStatusCode()); + // The following assertions check that in absence of an RFC9457 compliant response, we properly initialize the + // default values for the attached Problem Details. + assertEquals(ProblemDetails.DEFAULT_TYPE, exception.getProblemDetails().getType().toString()); + // The Problem Details title should default to the status message + assertEquals(expectedStatusMessage, exception.getProblemDetails().getTitle()); + assertEquals(statusCode, exception.getProblemDetails().getStatus()); + assertNull(exception.getProblemDetails().getDetails()); + assertNull(exception.getProblemDetails().getInstance()); } - private static Stream testSpecialisedExceptions() { + private static Stream testLegacySpecialisedExceptions() { return Stream.of( - Arguments.of(BadRequestException.class, 400), - Arguments.of(UnauthorizedException.class, 401), - Arguments.of(ForbiddenException.class, 403), - Arguments.of(NotFoundException.class, 404), - Arguments.of(MethodNotAllowedException.class, 405), - Arguments.of(NotAcceptableException.class, 406), - Arguments.of(ConflictException.class, 409), - Arguments.of(GoneException.class, 410), - Arguments.of(PreconditionFailedException.class, 412), - Arguments.of(UnsupportedMediaTypeException.class, 415), - Arguments.of(TooManyRequestsException.class, 429), - Arguments.of(InternalServerErrorException.class, 500), - Arguments.of(SolidClientException.class, 418) + Arguments.of(BadRequestException.class, 400, "Bad Request"), + Arguments.of(UnauthorizedException.class, 401, "Unauthorized"), + Arguments.of(ForbiddenException.class, 403, "Forbidden"), + Arguments.of(NotFoundException.class, 404, "Not Found"), + Arguments.of(MethodNotAllowedException.class, 405, "Method Not Allowed"), + Arguments.of(NotAcceptableException.class, 406, "Not Acceptable"), + Arguments.of(ConflictException.class, 409, "Conflict"), + Arguments.of(GoneException.class, 410, "Gone"), + Arguments.of(PreconditionFailedException.class, 412, "Precondition Failed"), + Arguments.of(UnsupportedMediaTypeException.class, 415, "Unsupported Media Type"), + Arguments.of(TooManyRequestsException.class, 429, "Too Many Requests"), + Arguments.of(InternalServerErrorException.class, 500, "Internal Server Error"), + Arguments.of(SolidClientException.class, 418, "Bad Request") ); } + + @ParameterizedTest + @MethodSource + void testRfc9457SpecialisedExceptions( + final Class clazz, + final ProblemDetails problemDetails + ) { + final Headers headers = Headers.of(Collections.singletonMap("x-key", Arrays.asList("value"))); + final SolidClient solidClient = new SolidClient(ClientProvider.getClient(), headers, false); + final SolidContainer resource = new SolidContainer(URI.create("http://example.com")); + + final SolidClientException exception = assertThrows( + clazz, + () -> solidClient.handleResponse(resource, headers, "message") + .apply(new Response() { + @Override + public byte[] body() { + final ByteArrayOutputStream bos = new ByteArrayOutputStream(); + try { + jsonService.toJson(problemDetails, bos); + } catch (IOException e) { + throw new RuntimeException(e); + } + return bos.toByteArray(); + } + + @Override + public Headers headers() { + final List headerValues = new ArrayList<>(); + headerValues.add("application/problem+json"); + final Map> headerMap = new HashMap<>(); + headerMap.put("Content-Type", headerValues); + return Headers.of(headerMap); + } + + @Override + public URI uri() { + return null; + } + + @Override + public int statusCode() { + return problemDetails.getStatus(); + } + }) + ); + assertEquals(problemDetails.getStatus(), exception.getStatusCode()); + assertEquals(problemDetails.getType(), exception.getProblemDetails().getType()); + assertEquals(problemDetails.getTitle(), exception.getProblemDetails().getTitle()); + assertEquals(problemDetails.getStatus(), exception.getProblemDetails().getStatus()); + assertEquals(problemDetails.getDetails(), exception.getProblemDetails().getDetails()); + assertEquals(problemDetails.getInstance(), exception.getProblemDetails().getInstance()); + } + + private static Stream testRfc9457SpecialisedExceptions() { + return Stream.of( + Arguments.of( + BadRequestException.class, + new ProblemDetails( + URI.create("https://example.org/type"), + "Bad Request", + "Some details", + 400, + URI.create("https://example.org/instance") + ) + ), Arguments.of( + UnauthorizedException.class, + new ProblemDetails( + URI.create("https://example.org/type"), + "Unauthorized", + "Some details", + 401, + URI.create("https://example.org/instance") + ) + ), Arguments.of( + ForbiddenException.class, + new ProblemDetails( + URI.create("https://example.org/type"), + "Forbidden", + "Some details", + 403, + URI.create("https://example.org/instance") + ) + ), Arguments.of( + NotFoundException.class, + new ProblemDetails( + URI.create("https://example.org/type"), + "Not Found", + "Some details", + 404, + URI.create("https://example.org/instance") + ) + ), Arguments.of( + MethodNotAllowedException.class, + new ProblemDetails( + URI.create("https://example.org/type"), + "Method Not Allowed", + "Some details", + 405, + URI.create("https://example.org/instance") + ) + ), Arguments.of( + NotAcceptableException.class, + new ProblemDetails( + URI.create("https://example.org/type"), + "Not Acceptable", + "Some details", + 406, + URI.create("https://example.org/instance") + ) + ), Arguments.of( + ConflictException.class, + new ProblemDetails( + URI.create("https://example.org/type"), + "Conflict", + "Some details", + 409, + URI.create("https://example.org/instance") + ) + ), Arguments.of( + GoneException.class, + new ProblemDetails( + URI.create("https://example.org/type"), + "Gone", + "Some details", + 410, + URI.create("https://example.org/instance") + ) + ), Arguments.of( + PreconditionFailedException.class, + new ProblemDetails( + URI.create("https://example.org/type"), + "Precondition Failed", + "Some details", + 412, + URI.create("https://example.org/instance") + ) + ), Arguments.of( + UnsupportedMediaTypeException.class, + new ProblemDetails( + URI.create("https://example.org/type"), + "Unsupported Media Type", + "Some details", + 415, + URI.create("https://example.org/instance") + ) + ), Arguments.of( + TooManyRequestsException.class, + new ProblemDetails( + URI.create("https://example.org/type"), + "Too Many Requests", + "Some details", + 429, + URI.create("https://example.org/instance") + ) + ), Arguments.of( + InternalServerErrorException.class, + new ProblemDetails( + URI.create("https://example.org/type"), + "Internal Server Error", + "Some details", + 500, + URI.create("https://example.org/instance") + ) + ), Arguments.of( + // Custom errors that do not map to a predefined Exception class + // default to the generic SolidClientException + SolidClientException.class, + new ProblemDetails( + URI.create("https://example.org/type"), + "I'm a Teapot", + "Some details", + 418, + URI.create("https://example.org/instance") + ) + ), Arguments.of( + // Custom errors that do not map to a predefined Exception class + // default to the generic SolidClientException. + SolidClientException.class, + new ProblemDetails( + URI.create("https://example.org/type"), + "Custom server error", + "Some details", + 599, + URI.create("https://example.org/instance") + ) + ) + ); + } + + @ParameterizedTest + @MethodSource + void testLegacyCustomStatusExceptions( + final int statusCode, + final String expectedTitle + ) { + final Headers headers = Headers.of(Collections.singletonMap("x-key", Arrays.asList("value"))); + final SolidClient solidClient = new SolidClient(ClientProvider.getClient(), headers, false); + final SolidContainer resource = new SolidContainer(URI.create("http://example.com")); + + final SolidClientException exception = assertThrows( + SolidClientException.class, + () -> solidClient.handleResponse(resource, headers, "message") + .apply(new Response() { + @Override + public byte[] body() { + return new byte[0]; + } + + @Override + public Headers headers() { + return null; + } + + @Override + public URI uri() { + return null; + } + + @Override + public int statusCode() { + return statusCode; + } + }) + ); + assertEquals(statusCode, exception.getStatusCode()); + assertEquals(expectedTitle, exception.getProblemDetails().getTitle()); + assertEquals(statusCode, exception.getProblemDetails().getStatus()); + assertNull(exception.getProblemDetails().getDetails()); + assertNull(exception.getProblemDetails().getInstance()); + } + + private static Stream testLegacyCustomStatusExceptions() { + // Error codes for which the HttpStatusMessage isn't well-known should default to 400 or 500. + return Stream.of( + Arguments.of(418, "Bad Request"), + Arguments.of(599, "Internal Server Error"), + Arguments.of(600, "Internal Server Error") + ); + } + + @Test + void testMalformedProblemDetails() { + // The specific error code is irrelevant to this test. + final int statusCode = 400; + final Headers headers = Headers.of(Collections.singletonMap("x-key", Arrays.asList("value"))); + final SolidClient solidClient = new SolidClient(ClientProvider.getClient(), headers, false); + final SolidContainer resource = new SolidContainer(URI.create("http://example.com")); + + final SolidClientException exception = assertThrows( + BadRequestException.class, + () -> solidClient.handleResponse(resource, headers, "message") + .apply(new Response() { + // Pretend we return RFC9457 content... + @Override + public Headers headers() { + final List headerValues = new ArrayList<>(); + headerValues.add("application/problem+json"); + final Map> headerMap = new HashMap<>(); + headerMap.put("Content-Type", headerValues); + return Headers.of(headerMap); + } + + // ... but actually return malformed JSON. + @Override + public byte[] body() { + return "This isn't valid application/problem+json.".getBytes(); + } + + @Override + public URI uri() { + return null; + } + + @Override + public int statusCode() { + return statusCode; + } + }) + ); + assertEquals(statusCode, exception.getStatusCode()); + // On malformed response, the ProblemDetails should fall back to defaults. + assertEquals(ProblemDetails.DEFAULT_TYPE, exception.getProblemDetails().getType().toString()); + assertEquals("Bad Request", exception.getProblemDetails().getTitle()); + assertEquals(statusCode, exception.getProblemDetails().getStatus()); + assertNull(exception.getProblemDetails().getDetails()); + assertNull(exception.getProblemDetails().getInstance()); + } + + @Test + void testMinimalProblemDetails() { + // The specific error code is irrelevant to this test. + final int statusCode = 400; + final Headers headers = Headers.of(Collections.singletonMap("x-key", Arrays.asList("value"))); + final SolidClient solidClient = new SolidClient(ClientProvider.getClient(), headers, false); + final SolidContainer resource = new SolidContainer(URI.create("http://example.com")); + + final SolidClientException exception = assertThrows( + BadRequestException.class, + () -> solidClient.handleResponse(resource, headers, "message") + .apply(new Response() { + @Override + public Headers headers() { + final List headerValues = new ArrayList<>(); + headerValues.add("application/problem+json"); + final Map> headerMap = new HashMap<>(); + headerMap.put("Content-Type", headerValues); + return Headers.of(headerMap); + } + + // Return minimal problem details.. + @Override + public byte[] body() { + return "{\"status\":400}".getBytes(); + } + + @Override + public URI uri() { + return null; + } + + @Override + public int statusCode() { + return statusCode; + } + }) + ); + assertEquals(statusCode, exception.getStatusCode()); + // On malformed response, the ProblemDetails should fall back to defaults. + assertEquals(ProblemDetails.DEFAULT_TYPE, exception.getProblemDetails().getType().toString()); + assertEquals("Bad Request", exception.getProblemDetails().getTitle()); + assertEquals(statusCode, exception.getProblemDetails().getStatus()); + assertNull(exception.getProblemDetails().getDetails()); + assertNull(exception.getProblemDetails().getInstance()); + } } From 162e37b4c946a6f621440ad4345c7644484ba57b Mon Sep 17 00:00:00 2001 From: Nicolas Ayral Seydoux Date: Tue, 16 Apr 2024 10:39:23 +0200 Subject: [PATCH 02/25] fixup! JCL-403: SolidClient builds ProblemDetails --- .../com/inrupt/client/solid/SolidClient.java | 35 ++----------------- 1 file changed, 3 insertions(+), 32 deletions(-) diff --git a/solid/src/main/java/com/inrupt/client/solid/SolidClient.java b/solid/src/main/java/com/inrupt/client/solid/SolidClient.java index 4b7138ac8c6..30eb1eeb9c5 100644 --- a/solid/src/main/java/com/inrupt/client/solid/SolidClient.java +++ b/solid/src/main/java/com/inrupt/client/solid/SolidClient.java @@ -57,22 +57,11 @@ public class SolidClient { private final Client client; private final Headers defaultHeaders; private final boolean fetchAfterWrite; - private final JsonService jsonService; SolidClient(final Client client, final Headers headers, final boolean fetchAfterWrite) { this.client = Objects.requireNonNull(client, "Client may not be null!"); this.defaultHeaders = Objects.requireNonNull(headers, "Headers may not be null!"); this.fetchAfterWrite = fetchAfterWrite; - - // It is acceptable for a SolidClient instance to be in a classpath without any implementation for - // JsonService, in which case the ProblemDetails exceptions will fallback to default and not be parsed. - JsonService js; - try { - js = ServiceProvider.getJsonService(); - } catch (IllegalStateException e) { - js = null; - } - this.jsonService = js; } /** @@ -138,16 +127,10 @@ public CompletionStage read(final URI identifier, final return client.send(request, Response.BodyHandlers.ofByteArray()) .thenApply(response -> { if (response.statusCode() >= ERROR_STATUS) { - final ProblemDetails pd = ProblemDetails.fromErrorResponse( - response.statusCode(), - response.headers(), - response.body(), - this.jsonService - ); throw SolidClientException.handle( "Unable to read resource at " + request.uri(), - pd, response.uri(), + response.statusCode(), response.headers(), new String(response.body()) ); @@ -299,16 +282,10 @@ public CompletionStage delete(final T resource, final if (isSuccess(res.statusCode())) { return null; } else { - final ProblemDetails pd = ProblemDetails.fromErrorResponse( - res.statusCode(), - res.headers(), - res.body(), - this.jsonService - ); throw SolidClientException.handle( "Unable to delete resource", - pd, resource.getIdentifier(), + res.statusCode(), res.headers(), new String(res.body()) ); @@ -397,16 +374,10 @@ Function, CompletionStage> handleRespon final Headers headers, final String message) { return res -> { if (!isSuccess(res.statusCode())) { - final ProblemDetails pd = ProblemDetails.fromErrorResponse( - res.statusCode(), - res.headers(), - res.body(), - this.jsonService - ); throw SolidClientException.handle( message, - pd, resource.getIdentifier(), + res.statusCode(), res.headers(), new String(res.body()) ); From 983a474a0c21a78c78e48189b48ad6f4a2802fe0 Mon Sep 17 00:00:00 2001 From: Nicolas Ayral Seydoux Date: Tue, 16 Apr 2024 10:45:41 +0200 Subject: [PATCH 03/25] Specify utf-8 encoding --- solid/src/main/java/com/inrupt/client/solid/SolidClient.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/solid/src/main/java/com/inrupt/client/solid/SolidClient.java b/solid/src/main/java/com/inrupt/client/solid/SolidClient.java index 30eb1eeb9c5..8ad3064fabc 100644 --- a/solid/src/main/java/com/inrupt/client/solid/SolidClient.java +++ b/solid/src/main/java/com/inrupt/client/solid/SolidClient.java @@ -29,6 +29,7 @@ import java.io.IOException; import java.io.InputStream; import java.net.URI; +import java.nio.charset.StandardCharsets; import java.util.Collections; import java.util.List; import java.util.Map; @@ -287,7 +288,7 @@ public CompletionStage delete(final T resource, final resource.getIdentifier(), res.statusCode(), res.headers(), - new String(res.body()) + new String(res.body(), StandardCharsets.UTF_8) ); } }); From ea661c2b53578f62ab1fb4018e7edd698810e0f4 Mon Sep 17 00:00:00 2001 From: Nicolas Ayral Seydoux Date: Tue, 16 Apr 2024 16:57:50 +0200 Subject: [PATCH 04/25] Use throwing body mapper --- .../com/inrupt/client/solid/SolidClient.java | 87 +++++++++---------- .../client/solid/SolidClientException.java | 31 +++++++ 2 files changed, 73 insertions(+), 45 deletions(-) diff --git a/solid/src/main/java/com/inrupt/client/solid/SolidClient.java b/solid/src/main/java/com/inrupt/client/solid/SolidClient.java index 8ad3064fabc..afd20b3e946 100644 --- a/solid/src/main/java/com/inrupt/client/solid/SolidClient.java +++ b/solid/src/main/java/com/inrupt/client/solid/SolidClient.java @@ -125,42 +125,39 @@ public CompletionStage read(final URI identifier, final headers.firstValue(USER_AGENT).ifPresent(agent -> builder.setHeader(USER_AGENT, agent)); final Request request = builder.build(); - return client.send(request, Response.BodyHandlers.ofByteArray()) - .thenApply(response -> { - if (response.statusCode() >= ERROR_STATUS) { - throw SolidClientException.handle( - "Unable to read resource at " + request.uri(), - response.uri(), - response.statusCode(), - response.headers(), - new String(response.body()) - ); - } else { - final String contentType = response.headers().firstValue(CONTENT_TYPE) - .orElse("application/octet-stream"); - try { - // Check that this is an RDFSoure - if (RDFSource.class.isAssignableFrom(clazz)) { - final Dataset dataset = SolidResourceHandlers.buildDataset(contentType, response.body(), - request.uri().toString()).orElse(null); - final T obj = construct(request.uri(), clazz, dataset, response.headers()); - final ValidationResult res = RDFSource.class.cast(obj).validate(); - if (!res.isValid()) { - throw new DataMappingException( - "Unable to map resource into type: [" + clazz.getSimpleName() + "] ", - res.getResults()); - } - return obj; - // Otherwise, create a non-RDF-bearing resource - } else { - return construct(request.uri(), clazz, contentType, - new ByteArrayInputStream(response.body()), response.headers()); + return client.send( + request, + Response.BodyHandlers.throwOnError(Response.BodyHandlers.ofByteArray(), (r) -> isSuccess(r.statusCode())) + ).thenApply(response -> { + final String contentType = response.headers().firstValue(CONTENT_TYPE) + .orElse("application/octet-stream"); + try { + // Check that this is an RDFSoure + if (RDFSource.class.isAssignableFrom(clazz)) { + final Dataset dataset = SolidResourceHandlers.buildDataset(contentType, response.body(), + request.uri().toString()).orElse(null); + final T obj = construct(request.uri(), clazz, dataset, response.headers()); + final ValidationResult res = RDFSource.class.cast(obj).validate(); + if (!res.isValid()) { + throw new DataMappingException( + "Unable to map resource into type: [" + clazz.getSimpleName() + "] ", + res.getResults()); } - } catch (final ReflectiveOperationException ex) { - throw new SolidResourceException("Unable to read resource into type " + clazz.getName(), - ex); + return obj; + // Otherwise, create a non-RDF-bearing resource + } else { + return construct(request.uri(), clazz, contentType, + new ByteArrayInputStream(response.body()), response.headers()); } + } catch (final ReflectiveOperationException ex) { + throw new SolidResourceException("Unable to read resource into type " + clazz.getName(), + ex); } + }).exceptionally(exception -> { + if (exception instanceof ClientHttpException) { + throw SolidClientException.handle((ClientHttpException) exception); + } + throw new RuntimeException("Something went wrong reading "+request.uri(), exception); }); } @@ -279,19 +276,19 @@ public CompletionStage delete(final T resource, final defaultHeaders.firstValue(USER_AGENT).ifPresent(agent -> builder.setHeader(USER_AGENT, agent)); headers.firstValue(USER_AGENT).ifPresent(agent -> builder.setHeader(USER_AGENT, agent)); - return client.send(builder.build(), Response.BodyHandlers.ofByteArray()).thenApply(res -> { - if (isSuccess(res.statusCode())) { - return null; - } else { - throw SolidClientException.handle( - "Unable to delete resource", - resource.getIdentifier(), - res.statusCode(), - res.headers(), - new String(res.body(), StandardCharsets.UTF_8) - ); + return client.send( + builder.build(), + Response.BodyHandlers.throwOnError( + Response.BodyHandlers.ofByteArray(), + (r) -> isSuccess(r.statusCode()) + ) + ).exceptionally(exception -> { + if (exception instanceof ClientHttpException) { + throw SolidClientException.handle((ClientHttpException) exception); } - }); + throw new RuntimeException("Something went wrong reading "+resource.getIdentifier(), exception); + // FIXME I don't understand why the following is required. + }).thenAccept(o -> {}); } /** diff --git a/solid/src/main/java/com/inrupt/client/solid/SolidClientException.java b/solid/src/main/java/com/inrupt/client/solid/SolidClientException.java index bf6decd11ee..b572d7def2b 100644 --- a/solid/src/main/java/com/inrupt/client/solid/SolidClientException.java +++ b/solid/src/main/java/com/inrupt/client/solid/SolidClientException.java @@ -90,5 +90,36 @@ public static SolidClientException handle( return new SolidClientException(message, uri, statusCode, headers, body); } } + + public static SolidClientException handle(final ClientHttpException exception){ + switch (exception.getStatusCode()) { + case BadRequestException.STATUS_CODE: + return (BadRequestException) exception; + case UnauthorizedException.STATUS_CODE: + return (UnauthorizedException) exception; + case ForbiddenException.STATUS_CODE: + return (ForbiddenException) exception; + case NotFoundException.STATUS_CODE: + return (NotFoundException) exception; + case MethodNotAllowedException.STATUS_CODE: + return (MethodNotAllowedException) exception; + case NotAcceptableException.STATUS_CODE: + return (NotAcceptableException) exception; + case ConflictException.STATUS_CODE: + return (ConflictException) exception; + case GoneException.STATUS_CODE: + return (GoneException) exception; + case PreconditionFailedException.STATUS_CODE: + return (PreconditionFailedException) exception; + case UnsupportedMediaTypeException.STATUS_CODE: + return (UnsupportedMediaTypeException) exception; + case TooManyRequestsException.STATUS_CODE: + return (TooManyRequestsException) exception; + case InternalServerErrorException.STATUS_CODE: + return (InternalServerErrorException) exception; + default: + return (SolidClientException) exception; + } + } } From 4cec943758298c2ffcfea8695cd03b08df916b0d Mon Sep 17 00:00:00 2001 From: Nicolas Ayral Seydoux Date: Tue, 16 Apr 2024 17:07:14 +0200 Subject: [PATCH 05/25] Lint --- .../com/inrupt/client/solid/SolidClient.java | 26 +++++++++---------- .../client/solid/SolidClientException.java | 2 +- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/solid/src/main/java/com/inrupt/client/solid/SolidClient.java b/solid/src/main/java/com/inrupt/client/solid/SolidClient.java index afd20b3e946..b3cae93a6eb 100644 --- a/solid/src/main/java/com/inrupt/client/solid/SolidClient.java +++ b/solid/src/main/java/com/inrupt/client/solid/SolidClient.java @@ -22,14 +22,11 @@ import com.inrupt.client.*; import com.inrupt.client.auth.Session; -import com.inrupt.client.spi.JsonService; -import com.inrupt.client.spi.ServiceProvider; import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InputStream; import java.net.URI; -import java.nio.charset.StandardCharsets; import java.util.Collections; import java.util.List; import java.util.Map; @@ -127,7 +124,10 @@ public CompletionStage read(final URI identifier, final final Request request = builder.build(); return client.send( request, - Response.BodyHandlers.throwOnError(Response.BodyHandlers.ofByteArray(), (r) -> isSuccess(r.statusCode())) + Response.BodyHandlers.throwOnError( + Response.BodyHandlers.ofByteArray(), + (r) -> isSuccess(r.statusCode()) + ) ).thenApply(response -> { final String contentType = response.headers().firstValue(CONTENT_TYPE) .orElse("application/octet-stream"); @@ -157,7 +157,7 @@ public CompletionStage read(final URI identifier, final if (exception instanceof ClientHttpException) { throw SolidClientException.handle((ClientHttpException) exception); } - throw new RuntimeException("Something went wrong reading "+request.uri(), exception); + throw new RuntimeException("Something went wrong reading " + request.uri(), exception); }); } @@ -277,18 +277,18 @@ public CompletionStage delete(final T resource, final headers.firstValue(USER_AGENT).ifPresent(agent -> builder.setHeader(USER_AGENT, agent)); return client.send( - builder.build(), - Response.BodyHandlers.throwOnError( - Response.BodyHandlers.ofByteArray(), - (r) -> isSuccess(r.statusCode()) - ) - ).exceptionally(exception -> { + builder.build(), + Response.BodyHandlers.throwOnError( + Response.BodyHandlers.ofByteArray(), + (r) -> isSuccess(r.statusCode()) + ) + ).exceptionally(exception -> { if (exception instanceof ClientHttpException) { throw SolidClientException.handle((ClientHttpException) exception); } - throw new RuntimeException("Something went wrong reading "+resource.getIdentifier(), exception); + throw new RuntimeException("Something went wrong reading " + resource.getIdentifier(), exception); // FIXME I don't understand why the following is required. - }).thenAccept(o -> {}); + }).thenAccept(o -> { /* no-op */ }); } /** diff --git a/solid/src/main/java/com/inrupt/client/solid/SolidClientException.java b/solid/src/main/java/com/inrupt/client/solid/SolidClientException.java index b572d7def2b..62a2fa0287f 100644 --- a/solid/src/main/java/com/inrupt/client/solid/SolidClientException.java +++ b/solid/src/main/java/com/inrupt/client/solid/SolidClientException.java @@ -91,7 +91,7 @@ public static SolidClientException handle( } } - public static SolidClientException handle(final ClientHttpException exception){ + public static SolidClientException handle(final ClientHttpException exception) { switch (exception.getStatusCode()) { case BadRequestException.STATUS_CODE: return (BadRequestException) exception; From 955214a66becdd5446b32cc92739c13616d09313 Mon Sep 17 00:00:00 2001 From: Nicolas Ayral Seydoux Date: Wed, 17 Apr 2024 09:24:14 +0200 Subject: [PATCH 06/25] Provide exception mapper to throwing handler --- .../com/inrupt/client/solid/SolidClient.java | 33 +++++++++++-------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/solid/src/main/java/com/inrupt/client/solid/SolidClient.java b/solid/src/main/java/com/inrupt/client/solid/SolidClient.java index b3cae93a6eb..bdc3482ff3a 100644 --- a/solid/src/main/java/com/inrupt/client/solid/SolidClient.java +++ b/solid/src/main/java/com/inrupt/client/solid/SolidClient.java @@ -27,6 +27,7 @@ import java.io.IOException; import java.io.InputStream; import java.net.URI; +import java.nio.charset.StandardCharsets; import java.util.Collections; import java.util.List; import java.util.Map; @@ -62,6 +63,18 @@ public class SolidClient { this.fetchAfterWrite = fetchAfterWrite; } + private static Function httpExceptionMapper(final String message) { + return res -> { + return SolidClientException.handle( + message, + res.uri(), + res.statusCode(), + res.headers(), + new String(res.body().array(), StandardCharsets.UTF_8) + ); + }; + } + /** * Create a session-scoped client. * @@ -126,7 +139,8 @@ public CompletionStage read(final URI identifier, final request, Response.BodyHandlers.throwOnError( Response.BodyHandlers.ofByteArray(), - (r) -> isSuccess(r.statusCode()) + (r) -> isSuccess(r.statusCode()), + httpExceptionMapper("Reading resource " + request.uri() + " failed.") ) ).thenApply(response -> { final String contentType = response.headers().firstValue(CONTENT_TYPE) @@ -153,11 +167,6 @@ public CompletionStage read(final URI identifier, final throw new SolidResourceException("Unable to read resource into type " + clazz.getName(), ex); } - }).exceptionally(exception -> { - if (exception instanceof ClientHttpException) { - throw SolidClientException.handle((ClientHttpException) exception); - } - throw new RuntimeException("Something went wrong reading " + request.uri(), exception); }); } @@ -280,15 +289,11 @@ public CompletionStage delete(final T resource, final builder.build(), Response.BodyHandlers.throwOnError( Response.BodyHandlers.ofByteArray(), - (r) -> isSuccess(r.statusCode()) + (r) -> isSuccess(r.statusCode()), + httpExceptionMapper("Deleting resource " + resource.getIdentifier() + " failed.") ) - ).exceptionally(exception -> { - if (exception instanceof ClientHttpException) { - throw SolidClientException.handle((ClientHttpException) exception); - } - throw new RuntimeException("Something went wrong reading " + resource.getIdentifier(), exception); - // FIXME I don't understand why the following is required. - }).thenAccept(o -> { /* no-op */ }); + )// FIXME I don't understand why the following is required. + .thenAccept(o -> { /* no-op */ }); } /** From fac8471febfe5a1873cc6b1ad3bd8a2604533326 Mon Sep 17 00:00:00 2001 From: Nicolas Ayral Seydoux Date: Wed, 17 Apr 2024 09:34:10 +0200 Subject: [PATCH 07/25] Remove unnecessary SolidClientException::handle --- .../client/solid/SolidClientException.java | 31 ------------------- 1 file changed, 31 deletions(-) diff --git a/solid/src/main/java/com/inrupt/client/solid/SolidClientException.java b/solid/src/main/java/com/inrupt/client/solid/SolidClientException.java index 62a2fa0287f..bf6decd11ee 100644 --- a/solid/src/main/java/com/inrupt/client/solid/SolidClientException.java +++ b/solid/src/main/java/com/inrupt/client/solid/SolidClientException.java @@ -90,36 +90,5 @@ public static SolidClientException handle( return new SolidClientException(message, uri, statusCode, headers, body); } } - - public static SolidClientException handle(final ClientHttpException exception) { - switch (exception.getStatusCode()) { - case BadRequestException.STATUS_CODE: - return (BadRequestException) exception; - case UnauthorizedException.STATUS_CODE: - return (UnauthorizedException) exception; - case ForbiddenException.STATUS_CODE: - return (ForbiddenException) exception; - case NotFoundException.STATUS_CODE: - return (NotFoundException) exception; - case MethodNotAllowedException.STATUS_CODE: - return (MethodNotAllowedException) exception; - case NotAcceptableException.STATUS_CODE: - return (NotAcceptableException) exception; - case ConflictException.STATUS_CODE: - return (ConflictException) exception; - case GoneException.STATUS_CODE: - return (GoneException) exception; - case PreconditionFailedException.STATUS_CODE: - return (PreconditionFailedException) exception; - case UnsupportedMediaTypeException.STATUS_CODE: - return (UnsupportedMediaTypeException) exception; - case TooManyRequestsException.STATUS_CODE: - return (TooManyRequestsException) exception; - case InternalServerErrorException.STATUS_CODE: - return (InternalServerErrorException) exception; - default: - return (SolidClientException) exception; - } - } } From b9572ecd10933024ccbcb85a467ece5d9320ba13 Mon Sep 17 00:00:00 2001 From: Nicolas Ayral Seydoux Date: Wed, 17 Apr 2024 09:46:43 +0200 Subject: [PATCH 08/25] Remove default status message tests --- .../inrupt/client/solid/SolidClientTest.java | 143 ++++++------------ 1 file changed, 46 insertions(+), 97 deletions(-) diff --git a/solid/src/test/java/com/inrupt/client/solid/SolidClientTest.java b/solid/src/test/java/com/inrupt/client/solid/SolidClientTest.java index c7f51ba21b6..bff8d25ac54 100644 --- a/solid/src/test/java/com/inrupt/client/solid/SolidClientTest.java +++ b/solid/src/test/java/com/inrupt/client/solid/SolidClientTest.java @@ -380,72 +380,72 @@ private static Stream testExceptionalResources() { @ParameterizedTest @MethodSource - void testLegacySpecialisedExceptions( + void testLegacyExceptions( final Class clazz, - final int statusCode, - final String expectedStatusMessage + final int statusCode ) { final Headers headers = Headers.of(Collections.singletonMap("x-key", Arrays.asList("value"))); final SolidClient solidClient = new SolidClient(ClientProvider.getClient(), headers, false); final SolidContainer resource = new SolidContainer(URI.create("http://example.com")); final SolidClientException exception = assertThrows( - clazz, - () -> solidClient.handleResponse(resource, headers, "message") - .apply(new Response() { - @Override - public byte[] body() { - return new byte[0]; - } - - @Override - public Headers headers() { - return null; - } - - @Override - public URI uri() { - return null; - } - - @Override - public int statusCode() { - return statusCode; - } - }) + clazz, + () -> solidClient.handleResponse(resource, headers, "message") + .apply(new Response() { + @Override + public byte[] body() { + return new byte[0]; + } + + @Override + public Headers headers() { + return null; + } + + @Override + public URI uri() { + return null; + } + + @Override + public int statusCode() { + return statusCode; + } + }) ); assertEquals(statusCode, exception.getStatusCode()); // The following assertions check that in absence of an RFC9457 compliant response, we properly initialize the // default values for the attached Problem Details. assertEquals(ProblemDetails.DEFAULT_TYPE, exception.getProblemDetails().getType().toString()); - // The Problem Details title should default to the status message - assertEquals(expectedStatusMessage, exception.getProblemDetails().getTitle()); assertEquals(statusCode, exception.getProblemDetails().getStatus()); + assertNull(exception.getProblemDetails().getTitle()); assertNull(exception.getProblemDetails().getDetails()); assertNull(exception.getProblemDetails().getInstance()); } - private static Stream testLegacySpecialisedExceptions() { + private static Stream testLegacyExceptions() { return Stream.of( - Arguments.of(BadRequestException.class, 400, "Bad Request"), - Arguments.of(UnauthorizedException.class, 401, "Unauthorized"), - Arguments.of(ForbiddenException.class, 403, "Forbidden"), - Arguments.of(NotFoundException.class, 404, "Not Found"), - Arguments.of(MethodNotAllowedException.class, 405, "Method Not Allowed"), - Arguments.of(NotAcceptableException.class, 406, "Not Acceptable"), - Arguments.of(ConflictException.class, 409, "Conflict"), - Arguments.of(GoneException.class, 410, "Gone"), - Arguments.of(PreconditionFailedException.class, 412, "Precondition Failed"), - Arguments.of(UnsupportedMediaTypeException.class, 415, "Unsupported Media Type"), - Arguments.of(TooManyRequestsException.class, 429, "Too Many Requests"), - Arguments.of(InternalServerErrorException.class, 500, "Internal Server Error"), - Arguments.of(SolidClientException.class, 418, "Bad Request") + Arguments.of(BadRequestException.class, 400), + Arguments.of(UnauthorizedException.class, 401), + Arguments.of(ForbiddenException.class, 403), + Arguments.of(NotFoundException.class, 404), + Arguments.of(MethodNotAllowedException.class, 405), + Arguments.of(NotAcceptableException.class, 406), + Arguments.of(ConflictException.class, 409), + Arguments.of(GoneException.class, 410), + Arguments.of(PreconditionFailedException.class, 412), + Arguments.of(UnsupportedMediaTypeException.class, 415), + Arguments.of(TooManyRequestsException.class, 429), + Arguments.of(InternalServerErrorException.class, 500), + Arguments.of(SolidClientException.class, 418), + Arguments.of(SolidClientException.class,599), + Arguments.of(SolidClientException.class,600) ); } @ParameterizedTest @MethodSource - void testRfc9457SpecialisedExceptions( + void testRfc9457Exceptions( final Class clazz, final ProblemDetails problemDetails ) { @@ -496,7 +496,7 @@ public int statusCode() { assertEquals(problemDetails.getInstance(), exception.getProblemDetails().getInstance()); } - private static Stream testRfc9457SpecialisedExceptions() { + private static Stream testRfc9457Exceptions() { return Stream.of( Arguments.of( BadRequestException.class, @@ -632,57 +632,6 @@ private static Stream testRfc9457SpecialisedExceptions() { ); } - @ParameterizedTest - @MethodSource - void testLegacyCustomStatusExceptions( - final int statusCode, - final String expectedTitle - ) { - final Headers headers = Headers.of(Collections.singletonMap("x-key", Arrays.asList("value"))); - final SolidClient solidClient = new SolidClient(ClientProvider.getClient(), headers, false); - final SolidContainer resource = new SolidContainer(URI.create("http://example.com")); - - final SolidClientException exception = assertThrows( - SolidClientException.class, - () -> solidClient.handleResponse(resource, headers, "message") - .apply(new Response() { - @Override - public byte[] body() { - return new byte[0]; - } - - @Override - public Headers headers() { - return null; - } - - @Override - public URI uri() { - return null; - } - - @Override - public int statusCode() { - return statusCode; - } - }) - ); - assertEquals(statusCode, exception.getStatusCode()); - assertEquals(expectedTitle, exception.getProblemDetails().getTitle()); - assertEquals(statusCode, exception.getProblemDetails().getStatus()); - assertNull(exception.getProblemDetails().getDetails()); - assertNull(exception.getProblemDetails().getInstance()); - } - - private static Stream testLegacyCustomStatusExceptions() { - // Error codes for which the HttpStatusMessage isn't well-known should default to 400 or 500. - return Stream.of( - Arguments.of(418, "Bad Request"), - Arguments.of(599, "Internal Server Error"), - Arguments.of(600, "Internal Server Error") - ); - } - @Test void testMalformedProblemDetails() { // The specific error code is irrelevant to this test. @@ -725,7 +674,7 @@ public int statusCode() { assertEquals(statusCode, exception.getStatusCode()); // On malformed response, the ProblemDetails should fall back to defaults. assertEquals(ProblemDetails.DEFAULT_TYPE, exception.getProblemDetails().getType().toString()); - assertEquals("Bad Request", exception.getProblemDetails().getTitle()); + assertNull(exception.getProblemDetails().getTitle()); assertEquals(statusCode, exception.getProblemDetails().getStatus()); assertNull(exception.getProblemDetails().getDetails()); assertNull(exception.getProblemDetails().getInstance()); @@ -772,7 +721,7 @@ public int statusCode() { assertEquals(statusCode, exception.getStatusCode()); // On malformed response, the ProblemDetails should fall back to defaults. assertEquals(ProblemDetails.DEFAULT_TYPE, exception.getProblemDetails().getType().toString()); - assertEquals("Bad Request", exception.getProblemDetails().getTitle()); + assertNull(exception.getProblemDetails().getTitle()); assertEquals(statusCode, exception.getProblemDetails().getStatus()); assertNull(exception.getProblemDetails().getDetails()); assertNull(exception.getProblemDetails().getInstance()); From 3c26a2cddcccc08364030445c62d769836e488a4 Mon Sep 17 00:00:00 2001 From: Nicolas Ayral Seydoux Date: Thu, 18 Apr 2024 10:04:30 +0200 Subject: [PATCH 09/25] Specify encoding --- solid/src/main/java/com/inrupt/client/solid/SolidClient.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/solid/src/main/java/com/inrupt/client/solid/SolidClient.java b/solid/src/main/java/com/inrupt/client/solid/SolidClient.java index bdc3482ff3a..e673c2ebc2c 100644 --- a/solid/src/main/java/com/inrupt/client/solid/SolidClient.java +++ b/solid/src/main/java/com/inrupt/client/solid/SolidClient.java @@ -382,7 +382,7 @@ Function, CompletionStage> handleRespon resource.getIdentifier(), res.statusCode(), res.headers(), - new String(res.body()) + new String(res.body(), StandardCharsets.UTF_8) ); } From 7c6a31ccb06d8b64e8921bc39bd9c577e24c14e6 Mon Sep 17 00:00:00 2001 From: Nicolas Ayral Seydoux Date: Thu, 18 Apr 2024 10:05:45 +0200 Subject: [PATCH 10/25] Remove FIXME --- solid/src/main/java/com/inrupt/client/solid/SolidClient.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/solid/src/main/java/com/inrupt/client/solid/SolidClient.java b/solid/src/main/java/com/inrupt/client/solid/SolidClient.java index e673c2ebc2c..e7381e8183b 100644 --- a/solid/src/main/java/com/inrupt/client/solid/SolidClient.java +++ b/solid/src/main/java/com/inrupt/client/solid/SolidClient.java @@ -292,7 +292,7 @@ public CompletionStage delete(final T resource, final (r) -> isSuccess(r.statusCode()), httpExceptionMapper("Deleting resource " + resource.getIdentifier() + " failed.") ) - )// FIXME I don't understand why the following is required. + ) .thenAccept(o -> { /* no-op */ }); } From 662ac78727a183da101f32363b81f18d281b7d50 Mon Sep 17 00:00:00 2001 From: Nicolas Ayral Seydoux Date: Thu, 18 Apr 2024 13:55:44 +0200 Subject: [PATCH 11/25] Remove PII from error message --- solid/src/main/java/com/inrupt/client/solid/SolidClient.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/solid/src/main/java/com/inrupt/client/solid/SolidClient.java b/solid/src/main/java/com/inrupt/client/solid/SolidClient.java index e7381e8183b..cf17b9ac967 100644 --- a/solid/src/main/java/com/inrupt/client/solid/SolidClient.java +++ b/solid/src/main/java/com/inrupt/client/solid/SolidClient.java @@ -290,7 +290,7 @@ public CompletionStage delete(final T resource, final Response.BodyHandlers.throwOnError( Response.BodyHandlers.ofByteArray(), (r) -> isSuccess(r.statusCode()), - httpExceptionMapper("Deleting resource " + resource.getIdentifier() + " failed.") + httpExceptionMapper("Unable to delete resource.") ) ) .thenAccept(o -> { /* no-op */ }); From 7c5e9223758f52b99b3be4c35efc42b349abc5d4 Mon Sep 17 00:00:00 2001 From: Nicolas Ayral Seydoux Date: Thu, 18 Apr 2024 14:28:42 +0200 Subject: [PATCH 12/25] Lift `isSuccess` to api module It will be used in Jena and RDF4J modules too --- api/src/main/java/com/inrupt/client/Response.java | 6 ++++-- .../main/java/com/inrupt/client/solid/SolidClient.java | 10 +++------- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/api/src/main/java/com/inrupt/client/Response.java b/api/src/main/java/com/inrupt/client/Response.java index 30acaffa1d7..98a74e620e6 100644 --- a/api/src/main/java/com/inrupt/client/Response.java +++ b/api/src/main/java/com/inrupt/client/Response.java @@ -97,6 +97,10 @@ interface ResponseInfo { ByteBuffer body(); } + static boolean isSuccess(final int statusCode) { + return statusCode >= 200 && statusCode < 300; + } + /** * An interface for mapping an HTTP response into a specific Java type. * @param the body type @@ -197,8 +201,6 @@ public static Response.BodyHandler throwOnError( return throwOnError(handler, isSuccess, defaultMapper); } - - private BodyHandlers() { // Prevent instantiation } diff --git a/solid/src/main/java/com/inrupt/client/solid/SolidClient.java b/solid/src/main/java/com/inrupt/client/solid/SolidClient.java index cf17b9ac967..32d34ff8281 100644 --- a/solid/src/main/java/com/inrupt/client/solid/SolidClient.java +++ b/solid/src/main/java/com/inrupt/client/solid/SolidClient.java @@ -139,7 +139,7 @@ public CompletionStage read(final URI identifier, final request, Response.BodyHandlers.throwOnError( Response.BodyHandlers.ofByteArray(), - (r) -> isSuccess(r.statusCode()), + (r) -> Response.isSuccess(r.statusCode()), httpExceptionMapper("Reading resource " + request.uri() + " failed.") ) ).thenApply(response -> { @@ -289,7 +289,7 @@ public CompletionStage delete(final T resource, final builder.build(), Response.BodyHandlers.throwOnError( Response.BodyHandlers.ofByteArray(), - (r) -> isSuccess(r.statusCode()), + (r) -> Response.isSuccess(r.statusCode()), httpExceptionMapper("Unable to delete resource.") ) ) @@ -376,7 +376,7 @@ public SolidClient build() { Function, CompletionStage> handleResponse(final T resource, final Headers headers, final String message) { return res -> { - if (!isSuccess(res.statusCode())) { + if (!Response.isSuccess(res.statusCode())) { throw SolidClientException.handle( message, resource.getIdentifier(), @@ -455,10 +455,6 @@ static void decorateHeaders(final Request.Builder builder, final Headers headers } } - static boolean isSuccess(final int statusCode) { - return statusCode >= 200 && statusCode < 300; - } - static Request.BodyPublisher cast(final Resource resource) { try { return Request.BodyPublishers.ofInputStream(resource.getEntity()); From 17128b971dd1b1043838f7266c70dfbc149bebc7 Mon Sep 17 00:00:00 2001 From: Nicolas Ayral Seydoux Date: Fri, 19 Apr 2024 10:09:07 +0200 Subject: [PATCH 13/25] Remove PII from read error message --- solid/src/main/java/com/inrupt/client/solid/SolidClient.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/solid/src/main/java/com/inrupt/client/solid/SolidClient.java b/solid/src/main/java/com/inrupt/client/solid/SolidClient.java index 32d34ff8281..5743c9f7ac6 100644 --- a/solid/src/main/java/com/inrupt/client/solid/SolidClient.java +++ b/solid/src/main/java/com/inrupt/client/solid/SolidClient.java @@ -140,7 +140,7 @@ public CompletionStage read(final URI identifier, final Response.BodyHandlers.throwOnError( Response.BodyHandlers.ofByteArray(), (r) -> Response.isSuccess(r.statusCode()), - httpExceptionMapper("Reading resource " + request.uri() + " failed.") + httpExceptionMapper("Reading resource failed.") ) ).thenApply(response -> { final String contentType = response.headers().firstValue(CONTENT_TYPE) From 6f3012a71d8f47435744d0734a70e4469aeb1e26 Mon Sep 17 00:00:00 2001 From: Nicolas Ayral Seydoux Date: Fri, 19 Apr 2024 10:20:04 +0200 Subject: [PATCH 14/25] Add missing Javadoc entry --- .../com/inrupt/client/ProblemDetails.java | 42 +++++++++++++++++++ .../main/java/com/inrupt/client/Response.java | 5 +++ 2 files changed, 47 insertions(+) diff --git a/api/src/main/java/com/inrupt/client/ProblemDetails.java b/api/src/main/java/com/inrupt/client/ProblemDetails.java index 998dd534980..617e748cb37 100644 --- a/api/src/main/java/com/inrupt/client/ProblemDetails.java +++ b/api/src/main/java/com/inrupt/client/ProblemDetails.java @@ -34,7 +34,13 @@ * @see RFC 9457 Problem Details for HTTP APIs */ public class ProblemDetails { + /** + * {@link RFC9457} default MIME type + */ public static final String MIME_TYPE = "application/problem+json"; + /** + * {@link RFC9457} default problem type + */ public static final String DEFAULT_TYPE = "about:blank"; private final URI type; private final String title; @@ -44,6 +50,15 @@ public class ProblemDetails { private static JsonService jsonService; private static boolean isJsonServiceInitialized; + /** + * Build a ProblemDetails instance providing the expected fields as described in + * {@link RFC9457}. + * @param type the problem type + * @param title the problem title + * @param details the problem details + * @param status the error response status code + * @param instance the problem instance + */ public ProblemDetails( final URI type, final String title, @@ -58,22 +73,42 @@ public ProblemDetails( this.instance = instance; } + /** + * The problem type. + * @return the type + */ public URI getType() { return this.type; }; + /** + * The problem title. + * @return the title + */ public String getTitle() { return this.title; }; + /** + * The problem details. + * @return the details + */ public String getDetails() { return this.details; }; + /** + * The problem status code. + * @return the status code + */ public int getStatus() { return this.status; }; + /** + * The problem instance. + * @return the instance + */ public URI getInstance() { return this.instance; }; @@ -104,6 +139,13 @@ private static JsonService getJsonService() { return ProblemDetails.jsonService; } + /** + * Builds a {@link ProblemDetails} instance from an HTTP error response. + * @param statusCode the HTTP error response status code + * @param headers the HTTP error response headers + * @param body the HTTP error response body + * @return a {@link ProblemDetails} instance + */ public static ProblemDetails fromErrorResponse( final int statusCode, final Headers headers, diff --git a/api/src/main/java/com/inrupt/client/Response.java b/api/src/main/java/com/inrupt/client/Response.java index 98a74e620e6..e641b7ece88 100644 --- a/api/src/main/java/com/inrupt/client/Response.java +++ b/api/src/main/java/com/inrupt/client/Response.java @@ -97,6 +97,11 @@ interface ResponseInfo { ByteBuffer body(); } + /** + * Indicates whether a status code reflects a successful HTTP response. + * @param statusCode the HTTP response status code + * @return true if the status code is in the success range, namely [200, 299]. + */ static boolean isSuccess(final int statusCode) { return statusCode >= 200 && statusCode < 300; } From e009dc1516d6ed1ad53194081fff10b57df49884 Mon Sep 17 00:00:00 2001 From: Nicolas Ayral Seydoux Date: Fri, 19 Apr 2024 10:36:14 +0200 Subject: [PATCH 15/25] fixup! Add missing Javadoc entry --- .../com/inrupt/client/ProblemDetails.java | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/api/src/main/java/com/inrupt/client/ProblemDetails.java b/api/src/main/java/com/inrupt/client/ProblemDetails.java index 617e748cb37..ba89e5ba2c7 100644 --- a/api/src/main/java/com/inrupt/client/ProblemDetails.java +++ b/api/src/main/java/com/inrupt/client/ProblemDetails.java @@ -35,11 +35,11 @@ */ public class ProblemDetails { /** - * {@link RFC9457} default MIME type + * The RFC9457 default MIME type */ public static final String MIME_TYPE = "application/problem+json"; /** - * {@link RFC9457} default problem type + * The RFC9457 default problem type */ public static final String DEFAULT_TYPE = "about:blank"; private final URI type; @@ -117,10 +117,25 @@ public URI getInstance() { * This inner class is only ever used for JSON deserialization. Please do not use in any other context. */ public static class Data { + /** + * The problem type. + */ public URI type; + /** + * The problem title. + */ public String title; + /** + * The problem details. + */ public String details; + /** + * The problem status code. + */ public int status; + /** + * The problem instance. + */ public URI instance; } From ab5dd2e5101e0503c57a985463c2e7b1aac4d35f Mon Sep 17 00:00:00 2001 From: Nicolas Ayral Seydoux Date: Fri, 19 Apr 2024 10:40:40 +0200 Subject: [PATCH 16/25] fixup! fixup! Add missing Javadoc entry --- api/src/main/java/com/inrupt/client/ProblemDetails.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/main/java/com/inrupt/client/ProblemDetails.java b/api/src/main/java/com/inrupt/client/ProblemDetails.java index ba89e5ba2c7..75f31c42fac 100644 --- a/api/src/main/java/com/inrupt/client/ProblemDetails.java +++ b/api/src/main/java/com/inrupt/client/ProblemDetails.java @@ -52,7 +52,7 @@ public class ProblemDetails { /** * Build a ProblemDetails instance providing the expected fields as described in - * {@link RFC9457}. + * RFC9457. * @param type the problem type * @param title the problem title * @param details the problem details From 1bb756b255b94dc81c7d4afea2593f1c03feaad0 Mon Sep 17 00:00:00 2001 From: Nicolas Ayral Seydoux Date: Fri, 19 Apr 2024 11:20:48 +0200 Subject: [PATCH 17/25] fixup! fixup! fixup! Add missing Javadoc entry --- api/src/main/java/com/inrupt/client/ProblemDetails.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/src/main/java/com/inrupt/client/ProblemDetails.java b/api/src/main/java/com/inrupt/client/ProblemDetails.java index 75f31c42fac..6ea244f92eb 100644 --- a/api/src/main/java/com/inrupt/client/ProblemDetails.java +++ b/api/src/main/java/com/inrupt/client/ProblemDetails.java @@ -35,11 +35,11 @@ */ public class ProblemDetails { /** - * The RFC9457 default MIME type + * The RFC9457 default MIME type. */ public static final String MIME_TYPE = "application/problem+json"; /** - * The RFC9457 default problem type + * The RFC9457 default problem type. */ public static final String DEFAULT_TYPE = "about:blank"; private final URI type; From 5ce73718f4f43b956113e838c4666ded6b262890 Mon Sep 17 00:00:00 2001 From: Nicolas Ayral Seydoux Date: Fri, 19 Apr 2024 11:35:42 +0200 Subject: [PATCH 18/25] Replace Arguments.of with Arguments.arguments --- .../inrupt/client/solid/SolidClientTest.java | 84 ++++++++++--------- 1 file changed, 46 insertions(+), 38 deletions(-) diff --git a/solid/src/test/java/com/inrupt/client/solid/SolidClientTest.java b/solid/src/test/java/com/inrupt/client/solid/SolidClientTest.java index bff8d25ac54..08351bf531c 100644 --- a/solid/src/test/java/com/inrupt/client/solid/SolidClientTest.java +++ b/solid/src/test/java/com/inrupt/client/solid/SolidClientTest.java @@ -22,6 +22,7 @@ import static java.nio.charset.StandardCharsets.UTF_8; import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.params.provider.Arguments.arguments; import com.inrupt.client.*; import com.inrupt.client.auth.Session; @@ -367,15 +368,22 @@ void testExceptionalResources( private static Stream testExceptionalResources() { return Stream.of( - Arguments.of( - URI.create(config.get("solid_resource_uri") + "/unauthorized"), 401, - UnauthorizedException.class), - Arguments.of( - URI.create(config.get("solid_resource_uri") + "/forbidden"), 403, - ForbiddenException.class), - Arguments.of( - URI.create(config.get("solid_resource_uri") + "/missing"), 404, - NotFoundException.class)); + arguments( + URI.create(config.get("solid_resource_uri") + "/unauthorized"), + 401, + UnauthorizedException.class + ), + arguments( + URI.create(config.get("solid_resource_uri") + "/forbidden"), + 403, + ForbiddenException.class + ), + arguments( + URI.create(config.get("solid_resource_uri") + "/missing"), + 404, + NotFoundException.class + ) + ); } @ParameterizedTest @@ -425,21 +433,21 @@ public int statusCode() { private static Stream testLegacyExceptions() { return Stream.of( - Arguments.of(BadRequestException.class, 400), - Arguments.of(UnauthorizedException.class, 401), - Arguments.of(ForbiddenException.class, 403), - Arguments.of(NotFoundException.class, 404), - Arguments.of(MethodNotAllowedException.class, 405), - Arguments.of(NotAcceptableException.class, 406), - Arguments.of(ConflictException.class, 409), - Arguments.of(GoneException.class, 410), - Arguments.of(PreconditionFailedException.class, 412), - Arguments.of(UnsupportedMediaTypeException.class, 415), - Arguments.of(TooManyRequestsException.class, 429), - Arguments.of(InternalServerErrorException.class, 500), - Arguments.of(SolidClientException.class, 418), - Arguments.of(SolidClientException.class,599), - Arguments.of(SolidClientException.class,600) + arguments(BadRequestException.class, 400), + arguments(UnauthorizedException.class, 401), + arguments(ForbiddenException.class, 403), + arguments(NotFoundException.class, 404), + arguments(MethodNotAllowedException.class, 405), + arguments(NotAcceptableException.class, 406), + arguments(ConflictException.class, 409), + arguments(GoneException.class, 410), + arguments(PreconditionFailedException.class, 412), + arguments(UnsupportedMediaTypeException.class, 415), + arguments(TooManyRequestsException.class, 429), + arguments(InternalServerErrorException.class, 500), + arguments(SolidClientException.class, 418), + arguments(SolidClientException.class,599), + arguments(SolidClientException.class,600) ); } @@ -498,7 +506,7 @@ public int statusCode() { private static Stream testRfc9457Exceptions() { return Stream.of( - Arguments.of( + arguments( BadRequestException.class, new ProblemDetails( URI.create("https://example.org/type"), @@ -507,7 +515,7 @@ private static Stream testRfc9457Exceptions() { 400, URI.create("https://example.org/instance") ) - ), Arguments.of( + ), arguments( UnauthorizedException.class, new ProblemDetails( URI.create("https://example.org/type"), @@ -516,7 +524,7 @@ private static Stream testRfc9457Exceptions() { 401, URI.create("https://example.org/instance") ) - ), Arguments.of( + ), arguments( ForbiddenException.class, new ProblemDetails( URI.create("https://example.org/type"), @@ -525,7 +533,7 @@ private static Stream testRfc9457Exceptions() { 403, URI.create("https://example.org/instance") ) - ), Arguments.of( + ), arguments( NotFoundException.class, new ProblemDetails( URI.create("https://example.org/type"), @@ -534,7 +542,7 @@ private static Stream testRfc9457Exceptions() { 404, URI.create("https://example.org/instance") ) - ), Arguments.of( + ), arguments( MethodNotAllowedException.class, new ProblemDetails( URI.create("https://example.org/type"), @@ -543,7 +551,7 @@ private static Stream testRfc9457Exceptions() { 405, URI.create("https://example.org/instance") ) - ), Arguments.of( + ), arguments( NotAcceptableException.class, new ProblemDetails( URI.create("https://example.org/type"), @@ -552,7 +560,7 @@ private static Stream testRfc9457Exceptions() { 406, URI.create("https://example.org/instance") ) - ), Arguments.of( + ), arguments( ConflictException.class, new ProblemDetails( URI.create("https://example.org/type"), @@ -561,7 +569,7 @@ private static Stream testRfc9457Exceptions() { 409, URI.create("https://example.org/instance") ) - ), Arguments.of( + ), arguments( GoneException.class, new ProblemDetails( URI.create("https://example.org/type"), @@ -570,7 +578,7 @@ private static Stream testRfc9457Exceptions() { 410, URI.create("https://example.org/instance") ) - ), Arguments.of( + ), arguments( PreconditionFailedException.class, new ProblemDetails( URI.create("https://example.org/type"), @@ -579,7 +587,7 @@ private static Stream testRfc9457Exceptions() { 412, URI.create("https://example.org/instance") ) - ), Arguments.of( + ), arguments( UnsupportedMediaTypeException.class, new ProblemDetails( URI.create("https://example.org/type"), @@ -588,7 +596,7 @@ private static Stream testRfc9457Exceptions() { 415, URI.create("https://example.org/instance") ) - ), Arguments.of( + ), arguments( TooManyRequestsException.class, new ProblemDetails( URI.create("https://example.org/type"), @@ -597,7 +605,7 @@ private static Stream testRfc9457Exceptions() { 429, URI.create("https://example.org/instance") ) - ), Arguments.of( + ), arguments( InternalServerErrorException.class, new ProblemDetails( URI.create("https://example.org/type"), @@ -606,7 +614,7 @@ private static Stream testRfc9457Exceptions() { 500, URI.create("https://example.org/instance") ) - ), Arguments.of( + ), arguments( // Custom errors that do not map to a predefined Exception class // default to the generic SolidClientException SolidClientException.class, @@ -617,7 +625,7 @@ private static Stream testRfc9457Exceptions() { 418, URI.create("https://example.org/instance") ) - ), Arguments.of( + ), arguments( // Custom errors that do not map to a predefined Exception class // default to the generic SolidClientException. SolidClientException.class, From be110dbc6b3bd00edc640498606118f9e942abfd Mon Sep 17 00:00:00 2001 From: Nicolas Ayral Seydoux Date: Fri, 19 Apr 2024 12:50:36 +0200 Subject: [PATCH 19/25] Improve readability of unit tests --- .../inrupt/client/solid/SolidClientTest.java | 179 ++++++------------ 1 file changed, 53 insertions(+), 126 deletions(-) diff --git a/solid/src/test/java/com/inrupt/client/solid/SolidClientTest.java b/solid/src/test/java/com/inrupt/client/solid/SolidClientTest.java index 08351bf531c..4a10ad51bd4 100644 --- a/solid/src/test/java/com/inrupt/client/solid/SolidClientTest.java +++ b/solid/src/test/java/com/inrupt/client/solid/SolidClientTest.java @@ -368,21 +368,9 @@ void testExceptionalResources( private static Stream testExceptionalResources() { return Stream.of( - arguments( - URI.create(config.get("solid_resource_uri") + "/unauthorized"), - 401, - UnauthorizedException.class - ), - arguments( - URI.create(config.get("solid_resource_uri") + "/forbidden"), - 403, - ForbiddenException.class - ), - arguments( - URI.create(config.get("solid_resource_uri") + "/missing"), - 404, - NotFoundException.class - ) + arguments(URI.create(config.get("solid_resource_uri") + "/unauthorized"), 401, UnauthorizedException.class), + arguments(URI.create(config.get("solid_resource_uri") + "/forbidden"), 403, ForbiddenException.class), + arguments(URI.create(config.get("solid_resource_uri") + "/missing"), 404, NotFoundException.class) ); } @@ -504,138 +492,77 @@ public int statusCode() { assertEquals(problemDetails.getInstance(), exception.getProblemDetails().getInstance()); } + private static ProblemDetails mockProblemDetails(final String title, final String details, final int status) { + return new ProblemDetails( + URI.create("https://example.org/type"), + title, + details, + status, + URI.create("https://example.org/instance") + ); + } + private static Stream testRfc9457Exceptions() { return Stream.of( arguments( BadRequestException.class, - new ProblemDetails( - URI.create("https://example.org/type"), - "Bad Request", - "Some details", - 400, - URI.create("https://example.org/instance") - ) - ), arguments( + mockProblemDetails("Bad Request", "Some details", 400) + ), + arguments( UnauthorizedException.class, - new ProblemDetails( - URI.create("https://example.org/type"), - "Unauthorized", - "Some details", - 401, - URI.create("https://example.org/instance") - ) - ), arguments( + mockProblemDetails("Unauthorized", "Some details", 401) + ), + arguments( ForbiddenException.class, - new ProblemDetails( - URI.create("https://example.org/type"), - "Forbidden", - "Some details", - 403, - URI.create("https://example.org/instance") - ) - ), arguments( + mockProblemDetails("Forbidden", "Some details", 403) + ), + arguments( NotFoundException.class, - new ProblemDetails( - URI.create("https://example.org/type"), - "Not Found", - "Some details", - 404, - URI.create("https://example.org/instance") - ) - ), arguments( + mockProblemDetails("Not Found", "Some details", 404) + ), + arguments( MethodNotAllowedException.class, - new ProblemDetails( - URI.create("https://example.org/type"), - "Method Not Allowed", - "Some details", - 405, - URI.create("https://example.org/instance") - ) - ), arguments( + mockProblemDetails("Method Not Allowed", "Some details", 405) + ), + arguments( NotAcceptableException.class, - new ProblemDetails( - URI.create("https://example.org/type"), - "Not Acceptable", - "Some details", - 406, - URI.create("https://example.org/instance") - ) - ), arguments( + mockProblemDetails("Not Acceptable", "Some details", 406) + ), + arguments( ConflictException.class, - new ProblemDetails( - URI.create("https://example.org/type"), - "Conflict", - "Some details", - 409, - URI.create("https://example.org/instance") - ) - ), arguments( + mockProblemDetails("Conflict", "Some details", 409) + ), + arguments( GoneException.class, - new ProblemDetails( - URI.create("https://example.org/type"), - "Gone", - "Some details", - 410, - URI.create("https://example.org/instance") - ) - ), arguments( + mockProblemDetails("Gone", "Some details", 410) + ), + arguments( PreconditionFailedException.class, - new ProblemDetails( - URI.create("https://example.org/type"), - "Precondition Failed", - "Some details", - 412, - URI.create("https://example.org/instance") - ) - ), arguments( + mockProblemDetails("Precondition Failed", "Some details", 412) + ), + arguments( UnsupportedMediaTypeException.class, - new ProblemDetails( - URI.create("https://example.org/type"), - "Unsupported Media Type", - "Some details", - 415, - URI.create("https://example.org/instance") - ) - ), arguments( + mockProblemDetails("Unsupported Media Type", "Some details", 415) + ), + arguments( TooManyRequestsException.class, - new ProblemDetails( - URI.create("https://example.org/type"), - "Too Many Requests", - "Some details", - 429, - URI.create("https://example.org/instance") - ) - ), arguments( + mockProblemDetails("Too Many Requests", "Some details", 429) + ), + arguments( InternalServerErrorException.class, - new ProblemDetails( - URI.create("https://example.org/type"), - "Internal Server Error", - "Some details", - 500, - URI.create("https://example.org/instance") - ) - ), arguments( + mockProblemDetails("Internal Server Error", "Some details", 500) + ), + arguments( // Custom errors that do not map to a predefined Exception class // default to the generic SolidClientException SolidClientException.class, - new ProblemDetails( - URI.create("https://example.org/type"), - "I'm a Teapot", - "Some details", - 418, - URI.create("https://example.org/instance") - ) - ), arguments( + mockProblemDetails("I'm a Teapot", "Some details", 418) + ), + arguments( // Custom errors that do not map to a predefined Exception class // default to the generic SolidClientException. SolidClientException.class, - new ProblemDetails( - URI.create("https://example.org/type"), - "Custom server error", - "Some details", - 599, - URI.create("https://example.org/instance") - ) + mockProblemDetails("Custom server error", "Some details", 599) ) ); } From 05b9f0e0a0de25f4468bb29494c355fc0e431b96 Mon Sep 17 00:00:00 2001 From: Nicolas Ayral Seydoux Date: Fri, 19 Apr 2024 12:53:40 +0200 Subject: [PATCH 20/25] fixup! Improve readability of unit tests --- .../inrupt/client/solid/SolidClientTest.java | 75 +++++++++---------- 1 file changed, 37 insertions(+), 38 deletions(-) diff --git a/solid/src/test/java/com/inrupt/client/solid/SolidClientTest.java b/solid/src/test/java/com/inrupt/client/solid/SolidClientTest.java index 4a10ad51bd4..db0ad9976fb 100644 --- a/solid/src/test/java/com/inrupt/client/solid/SolidClientTest.java +++ b/solid/src/test/java/com/inrupt/client/solid/SolidClientTest.java @@ -493,76 +493,75 @@ public int statusCode() { } private static ProblemDetails mockProblemDetails(final String title, final String details, final int status) { - return new ProblemDetails( - URI.create("https://example.org/type"), - title, - details, - status, - URI.create("https://example.org/instance") + return new ProblemDetails(URI.create("https://example.org/type"), + title, + details, + status, + URI.create("https://example.org/instance") ); } private static Stream testRfc9457Exceptions() { return Stream.of( arguments( - BadRequestException.class, - mockProblemDetails("Bad Request", "Some details", 400) + BadRequestException.class, + mockProblemDetails("Bad Request", "Some details", 400) ), arguments( - UnauthorizedException.class, - mockProblemDetails("Unauthorized", "Some details", 401) + UnauthorizedException.class, + mockProblemDetails("Unauthorized", "Some details", 401) ), arguments( - ForbiddenException.class, - mockProblemDetails("Forbidden", "Some details", 403) + ForbiddenException.class, + mockProblemDetails("Forbidden", "Some details", 403) ), arguments( - NotFoundException.class, - mockProblemDetails("Not Found", "Some details", 404) + NotFoundException.class, + mockProblemDetails("Not Found", "Some details", 404) ), arguments( - MethodNotAllowedException.class, - mockProblemDetails("Method Not Allowed", "Some details", 405) + MethodNotAllowedException.class, + mockProblemDetails("Method Not Allowed", "Some details", 405) ), arguments( - NotAcceptableException.class, - mockProblemDetails("Not Acceptable", "Some details", 406) + NotAcceptableException.class, + mockProblemDetails("Not Acceptable", "Some details", 406) ), arguments( - ConflictException.class, - mockProblemDetails("Conflict", "Some details", 409) + ConflictException.class, + mockProblemDetails("Conflict", "Some details", 409) ), arguments( - GoneException.class, - mockProblemDetails("Gone", "Some details", 410) + GoneException.class, + mockProblemDetails("Gone", "Some details", 410) ), arguments( - PreconditionFailedException.class, - mockProblemDetails("Precondition Failed", "Some details", 412) + PreconditionFailedException.class, + mockProblemDetails("Precondition Failed", "Some details", 412) ), arguments( - UnsupportedMediaTypeException.class, - mockProblemDetails("Unsupported Media Type", "Some details", 415) + UnsupportedMediaTypeException.class, + mockProblemDetails("Unsupported Media Type", "Some details", 415) ), arguments( - TooManyRequestsException.class, - mockProblemDetails("Too Many Requests", "Some details", 429) + TooManyRequestsException.class, + mockProblemDetails("Too Many Requests", "Some details", 429) ), arguments( - InternalServerErrorException.class, - mockProblemDetails("Internal Server Error", "Some details", 500) + InternalServerErrorException.class, + mockProblemDetails("Internal Server Error", "Some details", 500) ), arguments( - // Custom errors that do not map to a predefined Exception class - // default to the generic SolidClientException - SolidClientException.class, - mockProblemDetails("I'm a Teapot", "Some details", 418) + // Custom errors that do not map to a predefined Exception class + // default to the generic SolidClientException + SolidClientException.class, + mockProblemDetails("I'm a Teapot", "Some details", 418) ), arguments( - // Custom errors that do not map to a predefined Exception class - // default to the generic SolidClientException. - SolidClientException.class, - mockProblemDetails("Custom server error", "Some details", 599) + // Custom errors that do not map to a predefined Exception class + // default to the generic SolidClientException. + SolidClientException.class, + mockProblemDetails("Custom server error", "Some details", 599) ) ); } From febe60b4120c04b74a9928e966698e6c4f55b083 Mon Sep 17 00:00:00 2001 From: Nicolas Ayral Seydoux Date: Mon, 22 Apr 2024 15:13:43 +0200 Subject: [PATCH 21/25] Remove throwing body handler HTTP body handlers are applied by the low-level HTTP client, so they should only ever map to an HTTP response. The processing of that response is then the responsibility of the caller. --- .../main/java/com/inrupt/client/Response.java | 43 ------------------- 1 file changed, 43 deletions(-) diff --git a/api/src/main/java/com/inrupt/client/Response.java b/api/src/main/java/com/inrupt/client/Response.java index e641b7ece88..af4b0f800bd 100644 --- a/api/src/main/java/com/inrupt/client/Response.java +++ b/api/src/main/java/com/inrupt/client/Response.java @@ -163,49 +163,6 @@ public static BodyHandler discarding() { return responseInfo -> null; } - /** - * Throws on HTTP error using the provided mapper, or apply the provided body handler. - * @param handler the body handler to apply on non-error HTTP responses - * @param isSuccess a callback determining error cases - * @param exceptionMapper the exception mapper - * @return the body handler - * @param the type of the body handler - */ - public static Response.BodyHandler throwOnError( - final Response.BodyHandler handler, - final Function isSuccess, - final Function exceptionMapper - ) { - return responseinfo -> { - if (!isSuccess.apply(responseinfo)) { - throw exceptionMapper.apply(responseinfo); - } - return handler.apply(responseinfo); - }; - } - - /** - * Throws on HTTP error, or apply the provided body handler. - * @param handler the body handler to apply on non-error HTTP responses - * @param isSuccess a callback determining error cases - * @return the body handler - * @param the type of the body handler - */ - public static Response.BodyHandler throwOnError( - final Response.BodyHandler handler, - final Function isSuccess - ) { - final Function defaultMapper = responseInfo -> - new ClientHttpException( - "An HTTP error has been returned, with status code " + responseInfo.statusCode(), - responseInfo.uri(), - responseInfo.statusCode(), - responseInfo.headers(), - new String(responseInfo.body().array(), StandardCharsets.UTF_8) - ); - return throwOnError(handler, isSuccess, defaultMapper); - } - private BodyHandlers() { // Prevent instantiation } From be04d54d57a56f6c2b4a8bd863f14d4e4f90db20 Mon Sep 17 00:00:00 2001 From: Nicolas Ayral Seydoux Date: Mon, 22 Apr 2024 15:22:11 +0200 Subject: [PATCH 22/25] Remove throwOnError from solid module --- .../com/inrupt/client/solid/SolidClient.java | 49 ++++++++++--------- 1 file changed, 25 insertions(+), 24 deletions(-) diff --git a/solid/src/main/java/com/inrupt/client/solid/SolidClient.java b/solid/src/main/java/com/inrupt/client/solid/SolidClient.java index 5743c9f7ac6..d13adacbe47 100644 --- a/solid/src/main/java/com/inrupt/client/solid/SolidClient.java +++ b/solid/src/main/java/com/inrupt/client/solid/SolidClient.java @@ -63,18 +63,6 @@ public class SolidClient { this.fetchAfterWrite = fetchAfterWrite; } - private static Function httpExceptionMapper(final String message) { - return res -> { - return SolidClientException.handle( - message, - res.uri(), - res.statusCode(), - res.headers(), - new String(res.body().array(), StandardCharsets.UTF_8) - ); - }; - } - /** * Create a session-scoped client. * @@ -137,12 +125,18 @@ public CompletionStage read(final URI identifier, final final Request request = builder.build(); return client.send( request, - Response.BodyHandlers.throwOnError( - Response.BodyHandlers.ofByteArray(), - (r) -> Response.isSuccess(r.statusCode()), - httpExceptionMapper("Reading resource failed.") - ) + Response.BodyHandlers.ofByteArray() ).thenApply(response -> { + if (!Response.isSuccess(response.statusCode())) { + throw SolidClientException.handle( + "Reading resource failed.", + response.uri(), + response.statusCode(), + response.headers(), + new String(response.body(), StandardCharsets.UTF_8) + ); + } + final String contentType = response.headers().firstValue(CONTENT_TYPE) .orElse("application/octet-stream"); try { @@ -285,15 +279,22 @@ public CompletionStage delete(final T resource, final defaultHeaders.firstValue(USER_AGENT).ifPresent(agent -> builder.setHeader(USER_AGENT, agent)); headers.firstValue(USER_AGENT).ifPresent(agent -> builder.setHeader(USER_AGENT, agent)); + return client.send( builder.build(), - Response.BodyHandlers.throwOnError( - Response.BodyHandlers.ofByteArray(), - (r) -> Response.isSuccess(r.statusCode()), - httpExceptionMapper("Unable to delete resource.") - ) - ) - .thenAccept(o -> { /* no-op */ }); + Response.BodyHandlers.ofByteArray() + ).thenApply(response -> { + if (!Response.isSuccess(response.statusCode())) { + throw SolidClientException.handle( + "Deleting resource failed.", + response.uri(), + response.statusCode(), + response.headers(), + new String(response.body(), StandardCharsets.UTF_8) + ); + } + return null; + }); } /** From 20675d925ebc23ee4efdaf8abcf22397a1fd310f Mon Sep 17 00:00:00 2001 From: Nicolas Ayral Seydoux Date: Mon, 22 Apr 2024 16:56:57 +0200 Subject: [PATCH 23/25] Lint --- api/src/main/java/com/inrupt/client/Response.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/api/src/main/java/com/inrupt/client/Response.java b/api/src/main/java/com/inrupt/client/Response.java index af4b0f800bd..38bbd540163 100644 --- a/api/src/main/java/com/inrupt/client/Response.java +++ b/api/src/main/java/com/inrupt/client/Response.java @@ -26,8 +26,6 @@ import java.io.InputStream; import java.net.URI; import java.nio.ByteBuffer; -import java.nio.charset.StandardCharsets; -import java.util.function.Function; /** * An HTTP Response. From 9e0967da683a37793e17965b702f7627b6b55d0e Mon Sep 17 00:00:00 2001 From: Nicolas Ayral Seydoux Date: Tue, 23 Apr 2024 09:44:54 +0200 Subject: [PATCH 24/25] Use try-with-resource --- .../test/java/com/inrupt/client/solid/SolidClientTest.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/solid/src/test/java/com/inrupt/client/solid/SolidClientTest.java b/solid/src/test/java/com/inrupt/client/solid/SolidClientTest.java index db0ad9976fb..8b14d6f1651 100644 --- a/solid/src/test/java/com/inrupt/client/solid/SolidClientTest.java +++ b/solid/src/test/java/com/inrupt/client/solid/SolidClientTest.java @@ -455,13 +455,12 @@ void testRfc9457Exceptions( .apply(new Response() { @Override public byte[] body() { - final ByteArrayOutputStream bos = new ByteArrayOutputStream(); - try { + try (ByteArrayOutputStream bos = new ByteArrayOutputStream()){ jsonService.toJson(problemDetails, bos); + return bos.toByteArray(); } catch (IOException e) { throw new RuntimeException(e); } - return bos.toByteArray(); } @Override From ea2d7a6066e4514a7e1ae697bf57e98a9355b290 Mon Sep 17 00:00:00 2001 From: Nicolas Ayral Seydoux Date: Tue, 23 Apr 2024 09:50:52 +0200 Subject: [PATCH 25/25] Lint --- .../src/test/java/com/inrupt/client/solid/SolidClientTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/solid/src/test/java/com/inrupt/client/solid/SolidClientTest.java b/solid/src/test/java/com/inrupt/client/solid/SolidClientTest.java index 8b14d6f1651..dbe200c135b 100644 --- a/solid/src/test/java/com/inrupt/client/solid/SolidClientTest.java +++ b/solid/src/test/java/com/inrupt/client/solid/SolidClientTest.java @@ -455,7 +455,7 @@ void testRfc9457Exceptions( .apply(new Response() { @Override public byte[] body() { - try (ByteArrayOutputStream bos = new ByteArrayOutputStream()){ + try (ByteArrayOutputStream bos = new ByteArrayOutputStream()) { jsonService.toJson(problemDetails, bos); return bos.toByteArray(); } catch (IOException e) {