feat: jsonld parsing without re-serialization#849
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
This comment was marked as resolved.
This comment was marked as resolved.
| export type VerifiableCredentialBase = JsonLd & { | ||
| id: Iri; | ||
| /** | ||
| * @deprecated Use RDFJS API instead | ||
| */ | ||
| type: Iri[]; | ||
| /** | ||
| * @deprecated Use RDFJS API instead | ||
| */ | ||
| issuer: Iri; | ||
| /** | ||
| * ISO-8601 formatted date | ||
| * @deprecated Use RDFJS API instead | ||
| */ | ||
| issuanceDate: string; | ||
| /** | ||
| * Entity the credential makes claim about. | ||
| * @deprecated Use RDFJS API instead | ||
| */ | ||
| credentialSubject: { | ||
| /** | ||
| * @deprecated Use RDFJS API instead | ||
| */ | ||
| id: Iri; | ||
| /** | ||
| * The claim set is open, as any RDF graph is suitable for a set of claims. | ||
| * @deprecated Use RDFJS API instead | ||
| */ | ||
| [property: string]: unknown; | ||
| }; | ||
| /** | ||
| * @deprecated Use RDFJS API instead | ||
| */ | ||
| proof: Proof; | ||
| }; |
There was a problem hiding this comment.
Wouldn't it be easier to have something like
export type WithId = { id: IRI };
// @deprecated Prefer using the RDFJS API
export type VcObject = {
issuer: IRI;
...
};
export type VerifiableCredential = JsonLd & WithId & VcObject & DatasetCore;
to allow having one deprecations warning for all properties instead of individual warnings for each property?
There was a problem hiding this comment.
Unfortuantely this isn't sufficient to show strikethrough deprecation of the properties on editors like vccode
vs
| getVerifiableCredential, | ||
| getVerifiableCredentialApiConfiguration, | ||
| /** | ||
| * @hidden @deprecated |
There was a problem hiding this comment.
Is the @deprecated annotation used just to be more explicit than the hidden one if used by a dependant?
There was a problem hiding this comment.
Correct - I expect the only consumers of this function to be us in the access-grants library, and I want us to have the liberty to remove it at any time.
src/common/common.ts
Outdated
| ); | ||
| } | ||
|
|
||
| const parsedVc = await verifiableCredentialToDataset(vc as { id: string }, { |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| expect(presentationAsDataset.match(null, cred.holder, null).size).toBe(0); | ||
| expect( | ||
| isRdfjsVerifiablePresentation( | ||
| presentationAsDataset, | ||
| namedNode(mockedPresentation.id!), | ||
| ), | ||
| ).toBe(true); |
There was a problem hiding this comment.
I was expecting this to return false, in consistence with the legacy API.
There was a problem hiding this comment.
This case is similar to #849 (comment). The parser will ignore the holder because the context requires it to be an IRI.
So in RDFJS there is no holder triple; and since holder is allowed to be undefined this function returns true.
Similar to #849 (comment), if this makes sense to you, thumbs up this comment and I will add a comment.
| expect( | ||
| mockedPresentationAsDataset.match(null, cred.verifiableCredential, null) | ||
| .size, | ||
| ).toBe(0); | ||
| expect(isVerifiablePresentation(mockedPresentation)).toBe(false); | ||
| expect( | ||
| isRdfjsVerifiablePresentation( | ||
| mockedPresentationAsDataset, | ||
| namedNode(mockedPresentation.id!), | ||
| ), | ||
| ).toBe(true); |
There was a problem hiding this comment.
When this is parsed as JSON-LD it is treated as there being no verifiable credentials; so this is a valid presentation in RDFJS; but when treated as json we have an empty object.
So in this case I would actually expect them to return different results.
If that makes sense, thumbs up this comment and I'll comment this on the test.
src/common/common.test.ts
Outdated
| }); | ||
| }); | ||
|
|
||
| it.skip("throws if the dereferenced data has an unsupported content type", async () => { |
There was a problem hiding this comment.
Because this would be a new check (the code for this is also currently deleted and/or disabled) - I suggest we enable it in the next major version.
Co-authored-by: Zwifi <nseydoux@inrupt.com>
Co-authored-by: Zwifi <nseydoux@inrupt.com>
2 participants
Note that the custom caching mechanism here is only temporary. There is work upstream to: