Fix editorial and substantive issues in JSON-LD section. #654
Conversation
msporny
requested review from
davidlehn,
dlongley,
peacekeeper,
OR13 and
tplooker
index.html
Outdated
| <a href="#production">production rules for JSON</a>, with one additional | ||
| requirement: The serialized <a>DID document</a> MUST include the | ||
| <code>@context</code> member. | ||
| All data structures expressed by the <a href="#data-model">data model</a> MUST |
There was a problem hiding this comment.
As I noted in #644 (comment), it would be a major change to now apply the representations to other things than DID documents. The DID resolution functions and data structures were intentionally left abstract, for good reasons; let's not change that now.
There was a problem hiding this comment.
I don't read this as changing the requirements... resolution returns an INFRA map with at least an id... you can't say anything other than that.... and have it still return a "DID Document" :)
There was a problem hiding this comment.
The DID resolution functions and data structures were intentionally left abstract, for good reasons; let's not change that now.
The new DID Resolution section made them concrete by saying the following:
If the resolution is successful, and if the resolve function was called, this MUST be a DID document abstract data model (a map) as described in § 4. Data Model that is capable of being transformed into a conforming DID Document (representation), using the production rules specified by the representation.
and
The entire metadata structure MUST be serializable in a conforming representation.
I was very careful to make sure the language didn't preclude other ways of working with the didDocument or metadata (when dealing with it in programming environments) or requiring it to be serialized in a specific representation (serializing it to HTTP is fine, as long as you can ALSO serialize the same data to a representation).
This is not a big change... it allows you to keep things abstract if you want to (as long as there is a mechanism to get it to a representation -- and that makes sure everything in the abstract in DID Core is also serializable)... which the test suite can take advantage of.
There was a problem hiding this comment.
The entire metadata structure MUST be serializable in a conforming representation.
I will note that there was an objection to adding this language (see #601 (comment)), and that this language was added without addressing the objection, and with very little review by the WG.
There was a problem hiding this comment.
That objection was resolved here: #601 (comment) -- and I followed through with what I had promised in the decision to merge by raising it on the call and having a discussion about it on the WG call.
index.html
Outdated
| the <a href="#consumption">consumption rules for JSON</a>, with one additional | ||
| requirement: The serialized <a>DID document</a> MUST include the | ||
| <code>@context</code> member and be processed according to the rules below. | ||
| All data structures expressed by a JSON-LD <a>representation</a> MUST be |
There was a problem hiding this comment.
Same comment as above. Data model and representations apply to DID documents, not other data structures.
There was a problem hiding this comment.
technically and INFRA map with only an id being required could represent anything... and still be a DID Document... this is a side effect / feature of the abstract data model and supporting unregistered properties.
There was a problem hiding this comment.
Agree with @OR13 -- we are using a data model that is capable of expressing all the data structures in the specification. A DID Document is one of those data structures. A verification method is another. A service is another. The ADM supports expressing all of those things (because it supports expressing maps).
To put it another way... the Java language gives you the Object class... it doesn't then say, "You can only use the Object class to express DID Documents." <-- that would be an unnecessary restriction, which would mean that you'd need to create yet another thing to express all the other data structures you need to use.
As an aside, this is why creating bespoke abstract data models for specifications is such a bad idea -- you get pulled down into these esoteric, academic tarpits... but since we were all pulled into this tarpit, we do have a fairly generalized data model (thanks to INFRA) and it can be applied to all our data structures (because other people went through the pain of defining INFRA so that this was possible).
There was a problem hiding this comment.
@peacekeeper I'll put the language back to what I believe you're comfortable with in https://github.com/w3c/did-core/pull/654/files#r577651396 -- this means that the only valid data structures are the DID Document and it's data structures. As a result, no change is needed for this text.
There was a problem hiding this comment.
@peacekeeper applied your suggestion in https://github.com/w3c/did-core/pull/654/files#r577928397 to address your objection.
index.html
Outdated
| <p> | ||
| In addition to using the JSON <a>representation</a> <a>consumption</a> rules, | ||
| JSON-LD consumption MUST add the representation-specific entries into a | ||
| representation-specific <a href="#data-model">data model</a> according to the |
There was a problem hiding this comment.
What's a "representation-specific data model"?
There was a problem hiding this comment.
I assume its a data-model with representation specific properties... for example an abstract data model with cbor integers for keys.
There was a problem hiding this comment.
@OR13's is one interpretation of how we could use the data model... except that it wouldn't work because we can't have integers as keys there.
I meant something slightly different, in an attempt to make your diagrams work @peacekeeper -- think of it as having two instantiations of the data model... one to hold the DID Document, and another to hold representation-specific properties. Your diagrams would then have two boxes that production to JSON-LD would pull from... the DID Document data model, and the representation-specific entries data model.
You could even make the case that if you see the same entry in the DID Document data model, and the representation-specific entries data model... and you don't recognize the one in representation-specific entries as something specific to your representation, then you don't need to serialize it. HOWEVER, I expect that @OR13 and I would object to that, UNLESS, the JSON representation noted that you have to preserve @context (because that's what we do in our implementations).
There was a problem hiding this comment.
if you don't want an @context don't consume JSON or JSON-LD that contains it.... craft your representation in the abstract and then produce ANY representation other than JSON-LD :)
Co-authored-by: Amy Guy <[email protected]>
|
@msporny @OR13 actually the idea of "representation (-specific) data models" does resonate with me a bit... A few months ago I suggested that we should have a "common data model" and multiple "representation data models". But in my mind the latter was a superset of the former (not a separate bucket), so not sure if that's aligned with your thinking. For reference, here's the diagram I was using back then:
|
Yes, aligned w/ my thinking -- but I was trying to not introduce yet another concept into the specification. Even if we were to add the concept of a "DID Document data model", and "Representation data models", we would still need yet another data model, which is shared among I think we're circling around the same concepts, but at this point, we need to settle on a way of talking about it in the spec. What I could do is clean this PR up and pretend we didn't have the discussion above... raise an issue, and then in a separate PR try to make the whole "different data models and/or productionOptions and/or representationEntries" work across the data model and all representations? In other words, make a pass to get stuff in a decent shape to then make these changes above easier to see/reason about. Thoughts? |
|
Lets stick to discussing on issues and keep PR reviews about actual changes... I have moved my comments here: #664 (comment) |
|
Substantive, multiple reviews, changes requested and made, objection raised by @peacekeeper and addressed via spec text change, no remaining objections, merging. |
| contains representation-specific syntax and therefore could be present as an | ||
| entry in the <a | ||
| href="#data-model">data model</a> to aid in lossless conversion. If the entry | ||
| is present in the <a href="#data-model">data model</a>, it MUST be used during |
There was a problem hiding this comment.
For the record, this language was added quite recently in #454 after much discussion, and it has been important for those of us who DON'T think @context is a REQUIRED entry in the abstract data model. Removing this part could therefore be problematic. But it looks like in #664 we're on a good path to make everyone happy!
This PR is a fairly significant restructuring of the JSON-LD section. All the previous normative statements are more or less still active, save for the ones that are handled by other parts of the specification. I don't think implementations would need to change based on the modifications in this section, but I'm still marking it as a substantive change so people put a bit more diligence into reviewing it to make sure it does everything important that was being done before while attempting to simplify the language a bit (and cover some edge cases/advisements).
Preview | Diff