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 relatedResource and digestSRI to the vocabulary #1296

Merged
merged 9 commits into from
Oct 16, 2023

Conversation

iherman
Copy link
Member

@iherman iherman commented Oct 1, 2023

This is the action I took at the F2F: adding the resource integrity terms to the VCDM vocabulary. Here is what I did:

  • Added anchors to the digestSRI and relatedResource terms in the specification itself (to be used by the vocabulary)
  • Added a section to define the sriString datatype
  • Added the digestSRI, relatedResource, mediaType to the v2 @context file
  • Added the digestSRI and relatedResource properties, as well as the sriString datatype, to vocabulary.yml
  • Added a section on data types in the vocabulary’s template.html file for the vocabulary generation
  • Expanded the diagram’s alt text to also refer to the new terms

Note that the mediaType term has not been added to the vocabulary; instead, it is mapped to https://schema.org/encodingFormat term. Let us avoid the "not invented here" syndrome...

Not strictly related to the underlying issue, but:

  • Added the vocabulary.drawio file to the repository and expanded the README.md file in the corresponding directory (this makes the maintenance of the diagrams easier and puts the raw data into our repo)
  • Updated the diagram (there were syntax errors in the links)

The automatic preview below works for the spec text, but not for the vocabulary. For the vocabulary files, see the separate vocabulary preview

This PR takes care of the issues #1237 and #1265.


Preview | Diff

- Added anchors to the `digestSRI` and `relatedResource` terms (to be used by the vocabulary
- Added a section to define the `sristring` datatype
- Added the `digestSRI`, `relatedResource`, `mediaType` to the v2 context
- Added the `digestSRI` and `relatedResource` properties, as well as the `sristring` datatype to `vocabulary.yml`
- Added a section on datatypes in the `template.html` file for the vocabulary generation
- Added the `vocabulary.drawio` file to the repository and expanded the `README.md` file in the the corresponding directory

T.B.D.:

- Add the new terms to the diagram
- Extend the diagram description to include the new terms.
…o-vocab

* main:
  Add normative text requiring protecting VCs and VPs.
…rors)

- `sristring` -> `sriString`
- Generated previews for the vocabulary
@iherman
Copy link
Member Author

iherman commented Oct 1, 2023

Note: the SRI spec defines the integrity attribute in such a way that the attribute value can be a list of hash expressions (see Example 7 in the spec). By formally referring to those sections, the VCDM inherits this feature. I do not think that is a problem, I would rather think this is a feature, but I just wanted to draw attention to this fact.

@iherman
Copy link
Member Author

iherman commented Oct 1, 2023

To be a bit picky about the RDFS aspects of the vocabulary: at the moment, the range of the relatedResource (resp. the domain of digestSRI) is unspecified, except to say that the domain of relatedResource must be an object with a URI (i.e., not a literal). I was tempted to use https://schema.org/CreativeWork for both, which is a class for very general, but nevertheless “concrete” objects (as opposed to abstract concepts like “the Moon”, which can also have a URI in the RDF world). Using CreativeWork makes sense because there is no way to calculate the hash of the Moon… That assignment would also be in line with the fact that the mediaType property has been aliased (in the context file) to https://schema.org/encodingFormat, whose domain is indeed CreativeWork. However, I did not want to use this class without a further discussion with, and consensus of, the Working Group.

I am happy to update the files if there is a consensus on this detail.

Cc @pchampin

@iherman iherman requested a review from TallTed October 1, 2023 08:51
index.html Outdated Show resolved Hide resolved
vocab/credentials/v2/README.md Outdated Show resolved Hide resolved
Co-authored-by: Ted Thibodeau Jr <[email protected]>
Copy link
Contributor

@seabass-labrax seabass-labrax left a comment

Choose a reason for hiding this comment

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

I would also be happy if this used https://schema.org/CreativeWork.

@iherman
Copy link
Member Author

iherman commented Oct 12, 2023

The issue was discussed in a meeting on 2023-10-11

  • no resolutions were taken
View the transcript

1.9. Add relatedResource and digestSRI to the vocabulary (pr vc-data-model#1296)

See github pull request vc-data-model#1296.

Brent Zundel: One approval, a bit of explanation from Ivan.
… This feels like a PR that just should be merged ... to me, but happy to hear otherwise.
… I'm not hearing otherwise, encourage folks to jump on, review and approve to get it in.

Manu Sporny: +1 to this PR.

Copy link
Contributor

@davidlehn davidlehn left a comment

Choose a reason for hiding this comment

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

  • Attempted to align sriString capitalization and URL.
  • Comments on vocab URL confusion. It seems like the vocab doc, spec, and context should be using strictly the same URL strings for identifiers. Or am I missing something there?
  • Were the diagram object fill colors an accessibility problem?

.gitignore Outdated Show resolved Hide resolved
contexts/credentials/v2 Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
contexts/credentials/v2 Outdated Show resolved Hide resolved
vocab/credentials/v2/vocabulary.yml Show resolved Hide resolved
vocab/credentials/v2/vocabulary.drawio Show resolved Hide resolved
vocab/credentials/v2/vocabulary.drawio Outdated Show resolved Hide resolved
vocab/credentials/v2/vocabulary.yml Outdated Show resolved Hide resolved
Comment on lines +69 to +76
"digestSRI": {
"@id": "https://www.w3.org/2018/credentials#digestSRI",
"@type": "https://www.w3.org/2018/credentials#sriString"
},
"mediaType": {
"@id": "https://schema.org/encodingFormat"
},

Copy link
Member

@msporny msporny Oct 16, 2023

Choose a reason for hiding this comment

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

Hmm, do we want both of these as top-level values... or do we want them under relatedResource. I would imagine we'd want the latter? /cc @dlongley @davidlehn

Copy link
Member Author

Choose a reason for hiding this comment

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

@msporny I am open either way, I leave the decision to you guys.

My argument for this setup is that digestSRI might be useful elsewhere; there is nothing in the definition that is really bound to relatedResource. After all, this is a general mechanism. (Note that the domain of digestSRI is essentially open-ended, see also #1296 (comment)). But, again, I regard this as a VCDM domain issue, and I am not a VCDM expert, so to say...

@msporny
Copy link
Member

msporny commented Oct 16, 2023

Waiting on updates from @iherman to merge.

@iherman
Copy link
Member Author

iherman commented Oct 16, 2023

Waiting on updates from @iherman to merge.

@msporny made the changes; for further issues see #1296 (comment) and #1296 (comment). I have settled everything else.

For the former: I will look at this issue again, but I would not want the PR to be held on this one. I promise to look into the issue in any case, just do not know when. Edited: in the meantime, I have solved this problem.

For the latter: I leave you the decision. It is fine if you guys do the change on the context file if you decide to do so and then merge the PR.

@msporny
Copy link
Member

msporny commented Oct 16, 2023

I'm going to merge this, pending the discussion on mediaType and digestSRI -- while the latter is useful everywhere, the former might conflict w/ other vocabularies. Whatever changes we'd need to make would be in the context, so we can probably just merge this for now and make changes later (if necessary).

@msporny
Copy link
Member

msporny commented Oct 16, 2023

Normative, multiple reviews, changes requested and made, no objections, merging.

@msporny msporny merged commit 5d8849b into main Oct 16, 2023
1 check passed
@msporny msporny deleted the iherman-add-related-resource-and-digestSRI-to-vocab branch October 16, 2023 13:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants