Follow alternate document location #292
Conversation
Complements the last commit.
The ignored test would test an expected behaviour of java.net.URL.HttpURLConnection, i.e. not to automatically follow redirects when this involves a protocol switching (e.g. from HTTP to HTTPS). See #289.
ansell
left a comment
ansell
left a comment
There was a problem hiding this comment.
There is a resource leak in the new version with a submethod. The resource cleanup pattern in use works in pointer-based languages, but not Java. The cleanup needs to be moved down into the submethod or the creation point moved back into the original method.
Also not sure why we are only looking for Link headers on JSON-LD responses. My impression was that the Link header was suitable for use on any other content type, potentially even HTTP 204 responses with no content, and the Link would be the way to go through to a JSON-LD document.
Also concerned about infinite recursion in the current model. Recursion shouldn't be necessary, but if it is simpler to understand that way, we need to do it safely.
| // We prefer application/ld+json, but fallback to application/json | ||
| // or whatever is available | ||
| request.addHeader("Accept", ACCEPT_HEADER); | ||
| response = httpClient.execute(request); |
There was a problem hiding this comment.
This will not work in Java to close the response in the outer method. You will need to do the try-catch or try-with-resources in here.
There was a problem hiding this comment.
Comment about another possible resource leak in the latest version of this.
- remove unused imports - sorted alphabetically
As proposed in jsonld-java#292 (comment) Doesn't work for when the connection is closed the stream is broken: "javax.net.ssl.SSLException: closing inbound before receiving peer's close_notify"
As proposed in #292 (comment)
This avoids a possible endless loop. See #292 (comment).
It should be checked if an Entity has a contentType, not if an Entity has content. See #292 (comment) and #292 (comment).
- remove superfluous check (was always false) - remove superfluous IOException - remove logger as it is no more needed See #292 (comment).
|
Thx for the reviews! Also, I now installed |
|
Thanks, have merged it and will release it today. |
Implements https://www.w3.org/TR/json-ld11/#alternate-document-location.
Resolves #289.