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

Add test suite to readme; term definition for jsonschema #211

Merged
merged 5 commits into from
Sep 7, 2023

Conversation

decentralgabe
Copy link
Collaborator

@decentralgabe decentralgabe commented Sep 5, 2023

Add href-able term definition for jsonSchema


Preview | Diff

@decentralgabe
Copy link
Collaborator Author

@iherman please review

index.html Outdated Show resolved Hide resolved
index.html Outdated
Comment on lines 416 to 417
intended to be used with the <a href="#jsonschemacredential"></a> type. The value of
the <code>jsonSchema</code> property MUST be a valid [[JSON-SCHEMA]].
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are two related statements:

  • §2.1 JsonSchema says that, w.r.t. the JsonSchema class, "When dereferencing the id property associated with the JsonSchema type value the result is a valid JSON Schema document according to its specification version.".
  • the statement above that I have just highlighted

There are two alternative interpretations of these two statements when put together.

  1. The jsonSchema property could be seen as an alternative to the dereferencing process for a JsonSchema class instance to get to a real Json Schema. In other words, the usage of the jsonSchema property is not related to the notion of JsonSchemaCredential, and it would be perfectly fine to use a jsonSchema property in the example 1, by simply "folding in" Example 2 into the credentialSchema construct.

  2. The usage of jsonSchema is restricted to the situation where the JsonSchema class instance is the object of the credentialSubject property within a JsonSchemaCredential instance.

I suspect that the intention of the spec is alternative (2). If so, this statement should be very explicitly added to the definition of the property in §2.1 (I know that it is between the lines in §2.2, but not really explicitly). Unfortunately, expressing that in the RDFS ontology is not really possible (or would require some complex OWL ontology construct that I do not think we should use), which means one more reason for this restrictions to appear very explicitly.

Note that, personally, I would not have any problems with alternative (1), which is simpler. But it is not my decision.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, expressing that in the RDFS ontology is not really possible (or would require some complex OWL ontology construct that I do not think we should use), which means one more reason for this restrictions to appear very explicitly.

Actually, there would be one way of introducing this restriction into the vocabulary: define a subclass of JsonSchema class, something like EmbeddedJsonSchema, define jsonSchema's domain as being EmbeddedJsonSchema, and requiring that the object of a credentialSchema property, when used in conjunction with JsonSchemaCredential, would be of type EmbeddedJsonSchema. A bit more complex, but it works. Not sure it is worth it, though...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I intended for (2) and will firm up the language.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iherman please review the latest change, I've added a note with the text you've suggested in section 2.1

@iherman
Copy link
Member

iherman commented Sep 6, 2023

See w3c/vc-data-model#1268 for the corresponding PR for the vocabulary

index.html Outdated Show resolved Hide resolved
@decentralgabe decentralgabe linked an issue Sep 6, 2023 that may be closed by this pull request
@decentralgabe decentralgabe merged commit 243c1a7 into main Sep 7, 2023
1 check passed
@decentralgabe decentralgabe deleted the cleanup branch September 7, 2023 15:09
@@ -7,6 +7,10 @@ https://w3c.github.io/vc-json-schema/
We encourage contributions meeting the [Contribution Guidelines](CONTRIBUTING.md). While we prefer the creation of issues
and Pull Requests in the GitHub repository, discussions may also occur on the [public-credentials](http://lists.w3.org/Archives/Public/public-credentials/) mailing list.

### Test Suite

A [docker](https://www.docker.com/)-based test suite for the specification can [be found here](https://github.com/w3c/vc-json-schema-test-suite). All impelementers are encouraged to add to the test suite.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The things that jump out after a PR is merged...

Suggested change
A [docker](https://www.docker.com/)-based test suite for the specification can [be found here](https://github.com/w3c/vc-json-schema-test-suite). All impelementers are encouraged to add to the test suite.
A [docker](https://www.docker.com/)-based test suite for the specification can [be found here](https://github.com/w3c/vc-json-schema-test-suite). All implementers are encouraged to add to the test suite.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Formalize the jsonSchema property in the VCDM vocabulary?
4 participants