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

Make DID Resolution section concrete and testable (abstract + Typescript). #592

Closed
wants to merge 24 commits into from

Conversation

msporny
Copy link
Member

@msporny msporny commented Feb 2, 2021

This PR is intended to address issue #549 by adding to the abstract functions discussed in the DID Resolution and DID URL Dereferencing sections and describing potentially conforming expressions in Typescript as concrete, testable functions. The PR uses an Typescript interface to provide an example while keeping all of what we had before. It also tries to strike a balance by providing at least one concrete realization of the interface without preventing other concrete interfaces from being realized.

There are a LOT of reformatting changes in this PR, it might be best to look at the rendered preview for this PR.


Preview | Diff

(See <a href="#read-verify"></a>.) The details of how this process is
accomplished are outside the scope of this specification, but all conformant
implementations implement two functions which have the following abstract
The <a>DID resolution</a> function resolves a <a>DID</a> into a <a>DID
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
The <a>DID resolution</a> function resolves a <a>DID</a> into a <a>DID
The <a>DID resolution</a> functions resolves a <a>DID</a> into a <a>DID

Copy link
Member

@TallTed TallTed Feb 4, 2021

Choose a reason for hiding this comment

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

@peacekeeper --
If the functions are plural, the resolve should be singular.
If the function is singular, the resolves should be plural.

Please change resolves to resolve or switch functions back to function.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed via a reword.

index.html Outdated
Comment on lines 3728 to 3731
The <code>resolve</code> function returns the <a>DID document</a> in its
abstract form.
The <code>resolveRepresentation</code> function returns a byte stream of the
<a>DID Document</a> formatted in the corresponding representation.
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest we don't remove this paragraph, as it was just added very recently by @shigeya in #560.

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, please.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done in 028302a.

index.html Outdated
</dd>
</dl>

<p>
The output variables of these functions are as follows:
The return value of <code>resolve()</code> is <dfn>Resolution</dfn> which is
Copy link
Contributor

Choose a reason for hiding this comment

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

As we discussed on yesterday's 2021-02-02 Topic Call, the resolution and dereferencing functions really return three separate values, rather than a single map with three properties. When you dereference an HTTP URL, the HTTP body (representation of a resource) and the HTTP headers are also separate things. RFC3986 says:

the most common form of URI dereference is "retrieval": making use of a URI in order to retrieve a representation of its associated resource [...] additional information might be supplied about the resource (resource metadata)

Having said that, concrete implementations e.g. in TypeScript may choose to return a single JSON object that contains all three return values, and we could add this as an example, similar to how in the DID Resolution spec we define a JSON-LD structure that can hold all three return values.

relating to the results of the <a>DID resolution</a> process. This structure is
REQUIRED and MUST NOT be empty. This metadata is defined by
<a href="#did-resolution-metadata"></a>. If the resolution is successful this
structure MUST contain a <code>contentType</code> property containing the
Copy link
Contributor

Choose a reason for hiding this comment

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

This line and several others in the PR now still assume that there's only one function, but there are two. Returning the contentType property in the metadata only makes sense for the resolveRepresentation() function, not for resolve().

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in latest commits.

Comment on lines -3772 to -3774
This metadata typically changes between invocations of the
<code>resolve</code> and <code>resolveRepresentation</code> functions as it
represents data about the resolution process itself. Properties defined by
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason for removing this? This is meant to describe the difference between didResolutionMetadata and didDocumentMetadata.

index.html Outdated
Comment on lines 3844 to 3845
specification defines the following common properties for a
<dfn>ResolutionOptions</dfn> <a data-cite="INFRA#maps">map</a>:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
specification defines the following common properties for a
<dfn>ResolutionOptions</dfn> <a data-cite="INFRA#maps">map</a>:
specification defines the following common properties for the
<dfn>options</dfn> <a href="metadata-structure">metadata structure</a>:

The naming in this section should be independent of the TypeScript example.

Copy link
Member Author

@msporny msporny Feb 6, 2021

Choose a reason for hiding this comment

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

Removed all types and examples and approached testability through a "proof by construction" rather than by providing concrete examples.

index.html Outdated
Comment on lines 4207 to 4208
specification defines the following common properties for a
<dfn>DereferencingOptions</dfn> <a data-cite="INFRA#maps">map</a>:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
specification defines the following common properties for a
<dfn>DereferencingOptions</dfn> <a data-cite="INFRA#maps">map</a>:
specification defines the following common properties for the
<dfn>options</dfn> <a href="metadata-structure">metadata structure</a>:

The naming in this section should be independent of the TypeScript example.

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed all types and examples and approached testability through a "proof by construction" rather than by providing concrete examples.

@msporny
Copy link
Member Author

msporny commented Feb 6, 2021

Taking a new approach for testability in PR #601.

Closing this PR in favor of that one.

Comment on lines +4042 to +4046
<code><a>equivalentId</a></code> property except: a) it is associated with a
single value rather than a ordered set, and b) the <a>DID</a> is defined to be
the canonical ID for the <a>DID subject</a> within the scope of the containing
<a>DID document</a>.
</p>
Copy link
Member

@TallTed TallTed Feb 8, 2021

Choose a reason for hiding this comment

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

Suggested change
<code><a>equivalentId</a></code> property except: a) it is associated with a
single value rather than a ordered set, and b) the <a>DID</a> is defined to be
the canonical ID for the <a>DID subject</a> within the scope of the containing
<a>DID document</a>.
</p>
<code><a>equivalentId</a></code> property except:
</p>
<ul>
<li>
<code><a>canonicalId</a></code> is associated with a single value rather
than an ordered set
</li>
<li>
the <a>DID</a> is defined to be the canonical ID for the <a>DID subject</a>
within the scope of the containing <a>DID document</a>
</li>
</ul>

<code>id</code> property value and that the <code><a>canonicalId</a></code>
value is defined by the <a>DID Method</a> to be the canonical ID for the <a>DID
subject</a> in the scope of the containing <a>DID document</a>. A
<code><a>canonicalId</a></code> value MUST be produced by, and a form of, the
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<code><a>canonicalId</a></code> value MUST be produced by, and a form of, the
<code><a>canonicalId</a></code> value MUST be produced by, and be a form of, the

Comment on lines +4064 to +4065
as its primary ID value for the <a>DID subject</a> and treat all other
equivalent values as secondary aliases (e.g. update corresponding primary
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
as its primary ID value for the <a>DID subject</a> and treat all other
equivalent values as secondary aliases (e.g. update corresponding primary
as its primary ID value for the <a>DID subject</a> and to treat all other
equivalent values as secondary aliases (e.g., to update corresponding primary

@TallTed
Copy link
Member

TallTed commented Feb 8, 2021

bah. github brought this PR to me as if it hadn't been closed yet. obviously, ignore my last few suggestions here...

@msporny msporny deleted the msporny-issue-549-abstract-typescript branch February 21, 2021 21:23
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.

4 participants