-
Notifications
You must be signed in to change notification settings - Fork 6
JCL-402: Low-level client throws enriched exceptions #1157
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
25 commits
Select commit
Hold shift + click to select a range
39a699f
JCL-403: SolidClient builds ProblemDetails
NSeydoux 162e37b
fixup! JCL-403: SolidClient builds ProblemDetails
NSeydoux 983a474
Specify utf-8 encoding
NSeydoux ea661c2
Use throwing body mapper
NSeydoux 4cec943
Lint
NSeydoux 955214a
Provide exception mapper to throwing handler
NSeydoux fac8471
Remove unnecessary SolidClientException::handle
NSeydoux b9572ec
Remove default status message tests
NSeydoux 3c26a2c
Specify encoding
NSeydoux 7c6a31c
Remove FIXME
NSeydoux 662ac78
Remove PII from error message
NSeydoux 7c5e922
Lift `isSuccess` to api module
NSeydoux 17128b9
Remove PII from read error message
NSeydoux 6f3012a
Add missing Javadoc entry
NSeydoux e009dc1
fixup! Add missing Javadoc entry
NSeydoux ab5dd2e
fixup! fixup! Add missing Javadoc entry
NSeydoux 1bb756b
fixup! fixup! fixup! Add missing Javadoc entry
NSeydoux 5ce7371
Replace Arguments.of with Arguments.arguments
NSeydoux be110db
Improve readability of unit tests
NSeydoux 05b9f0e
fixup! Improve readability of unit tests
NSeydoux febe60b
Remove throwing body handler
NSeydoux be04d54
Remove throwOnError from solid module
NSeydoux 20675d9
Lint
NSeydoux 9e0967d
Use try-with-resource
NSeydoux ea2d7a6
Lint
NSeydoux File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,22 +20,14 @@ | |
| */ | ||
| 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 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; | ||
|
|
@@ -131,36 +123,43 @@ public <T extends Resource> CompletionStage<T> 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()) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This looks like a large change, but essentially the error branch of the initial |
||
| .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())); | ||
| } 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.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 { | ||
| // 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); | ||
| } | ||
| }); | ||
| } | ||
|
|
@@ -280,13 +279,21 @@ public <T extends Resource> CompletionStage<Void> 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(), UTF_8)); | ||
NSeydoux marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| return client.send( | ||
| builder.build(), | ||
| 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; | ||
| }); | ||
| } | ||
|
|
||
|
|
@@ -370,9 +377,14 @@ public SolidClient build() { | |
| <T extends Resource> Function<Response<byte[]>, CompletionStage<T>> handleResponse(final T resource, | ||
| 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)); | ||
| if (!Response.isSuccess(res.statusCode())) { | ||
| throw SolidClientException.handle( | ||
| message, | ||
| resource.getIdentifier(), | ||
| res.statusCode(), | ||
| res.headers(), | ||
| new String(res.body(), StandardCharsets.UTF_8) | ||
| ); | ||
| } | ||
|
|
||
| if (!fetchAfterWrite) { | ||
|
|
@@ -382,7 +394,6 @@ <T extends Resource> Function<Response<byte[]>, CompletionStage<T>> handleRespon | |
| @SuppressWarnings("unchecked") | ||
| final Class<T> clazz = (Class<T>) resource.getClass(); | ||
| return read(resource.getIdentifier(), headers, clazz); | ||
|
|
||
| }; | ||
| } | ||
|
|
||
|
|
@@ -445,10 +456,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()); | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really need this method?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It avoids some duplication, and my impression was that it is a quite common pattern to have this affordance on HTTP response object: JS
Responsehas a.okgetter, andOkHttphasisSuccessful. I'm not strongly opposed to removing this, but I find it reads slightly better than checking the status range each time.