Stop nesting blank nodes in internal structure #2368
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
NSeydoux
force-pushed
the
fix/SDK-3296/nesting-blank-nodes
branch
from
abcdff3 to
f552370
Compare
NSeydoux
had a problem deploying
to
ESS PodSpaces
GitHub Actions
Failure
NSeydoux
commented
Mar 27, 2024
| @prefix ex: <https://example.org/> . | ||
| @prefix foaf: <http://xmlns.com/foaf/0.1/> . | ||
| @prefix vcard: <http://www.w3.org/2006/vcard/ns#> . | ||
| @base <https://example.org/> . |
There was a problem hiding this comment.
This change introduces the @base directive, because the URl resolution was incorrect.
| @@ -212,7 +214,7 @@ describe("fromRdfJsDataset", () => { | |||
| expect(fromRdfJsDataset(rdfJsDataset)).toStrictEqual({ | |||
| type: "Dataset", | |||
| graphs: { | |||
| default: { | |||
| default: expect.objectContaining({ | |||
There was a problem hiding this comment.
We no longer do exact matches on the internal representation, because of the unstable blank nodes identifiers.
Comment on lines
+253
to
+264
| const subjectsExcludingBlankNodes = getThingAll( | ||
| fromRdfJsDataset(rdfJsDataset), | ||
| { scope: acrGraphIriString }, | ||
| ); | ||
| const subjectsIncludingBlankNodes = getThingAll( | ||
| fromRdfJsDataset(rdfJsDataset), | ||
| { scope: acrGraphIriString, acceptBlankNodes: true }, | ||
| ); | ||
| // There should be two blank nodes in the resulting dataset. | ||
| expect( | ||
| subjectsIncludingBlankNodes.length - subjectsExcludingBlankNodes.length, | ||
| ).toBe(2); |
There was a problem hiding this comment.
This replaces the previous exact match of the internal structure
src/rdfjs.internal.ts
Outdated
There was a problem hiding this comment.
The changes in this file are centered around removing the code that nested blank nodes.
src/resource/solidDataset.ts
Outdated
There was a problem hiding this comment.
There used to be two separate implementations of the conversion of an RDFJS dataset into a SolidDataset. This refactors the code so that it is implemented only once.
This was referenced Mar 27, 2024
NSeydoux
had a problem deploying
to
ESS PodSpaces
GitHub Actions
Failure
chelseapinka
temporarily deployed
to
ESS PodSpaces
GitHub Actions
Inactive
chelseapinka
temporarily deployed
to
ESS PodSpaces
GitHub Actions
Inactive
chelseapinka
temporarily deployed
to
ESS PodSpaces
GitHub Actions
Inactive
chelseapinka
temporarily deployed
to
ESS PodSpaces
GitHub Actions
Inactive
chelseapinka
approved these changes
Apr 9, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This changes the internal representation of RDF datasets so that Blank Nodes subjects are no longer nested, even if they are part of a single predicate chain. This approach was initially chosen to avoid generating unstable identifiers for these Blank Nodes, but it prevents browsing the potential Blank Nodes subjects that are part of a single chain when querying a Dataset. In particular, the library relying on immutability, a BN without an identifier cannot be used as the subject of a new quad.
As a result, more blank nodes may be returned by
getThingAll(..., { acceptBlankNodes: true }), if they only appear as a link in a chain of blank nodes.This is work that will help resolving #2339.
Checklist