Skip to content
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

Validator: expanded JSON-LD type checks #27

Closed

Conversation

acdha
Copy link

@acdha acdha commented Sep 18, 2015

The validator is currently stricter than http://iiif.io/api/image/2.1/index.html#information-request and requires all responses to be application/ld+json.

This branch splits the existing jsonld check into separate checks for both specified behaviours:

  • When the request does not have Accept: application/ld+json the response MUST be application/json
  • When JSON-LD is requested, the response must be either application/ld+json or have both the application/json Content-Type and a Link header specifying the standard context

The second check is still slightly unsatisfactory because we don't appear to have a way to specify warnings as opposed to outright errors. The specification recommends that JSON responses have the Link header but it's not a hard requirement and it might be useful to acknowledge that this is tolerable but should be fixed if possible.

@zimeon
Copy link
Member

zimeon commented Sep 18, 2015

I agree with @acdha that we need an extra color (closer to green than red) to signify SHOULDs that are not implemented. Showing them as "errors" isn't quite right.

@azaroth42
Copy link
Member

Agree that there should be a warning color.

But I don't think the logic of the provided tests is correct either. If the Accept header says that the client wants application/json+ld, we shouldn't be giving the server credit for returning the application/json media type and the context link header.

The context link header should be a separate test for when the response is application/json, thus without an Accept header. At which point, the current jsonld test is okay.
I didn't make a separate test for application/json beyond the info_json test that also does the structure of the response object. We could separate that out, but I'm concerned that the number of info.json tests would be very high.

@acdha
Copy link
Author

acdha commented Sep 18, 2015

“If the Accept header says that the client wants application/json+ld, we shouldn't be giving the server credit for returning the application/json media type and the context link header”

If that's the case, I think the spec should make it clear that this is not permissible:

The server MUST return the regular JSON content-type unless the client explicitly requests the JSON-LD content-type in an Accept header. If the JSON-LD content-type is explicitly requested, the server MUST return the JSON-LD content-type

@azaroth42
Copy link
Member

👍 Will open an issue on the image API to clarify. At the moment it does read like the context header flows on from the previous discussion.

acdha added a commit to acdha/image-api that referenced this pull request Sep 21, 2015
As per the discussion on IIIF#27, this test now requires a server to answer
a request with `Accept: application/ld+json` with a response which has
`Content-Type: application/ld+json`.
@acdha
Copy link
Author

acdha commented Sep 21, 2015

@azaroth42 I've updated this pull request to remove the option to respond with application/json to a request for application/ld+json.

One concern I have, which might reasonably be considered out of scope, is that it's not uncommon for large sites to behind CDNs or other proxies which scrub the Accept header. That's a perfect use-case for having the validator support a warning level but I wonder whether it might also be grounds to have the spec suggest that clients tolerate this.

http://iiif.io/api/image/2.1/index.html#information-request:

* When JSON-LD was not requested, the response MUST be
  `application/json`
* When JSON-LD is requested, the response must either be
  `application/ld+json` or `application/json` with a Link
  header specifying the standard context
As per the discussion on IIIF#27, this test now requires a server to answer
a request with `Accept: application/ld+json` with a response which has
`Content-Type: application/ld+json`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants