Removed blank node filter in internal additions changelog to support setting a Thing with blank node(s)
This PR fixes #1544.
Description of the problem In May 2021, a commit was made that adds a blank node filter to the internal additions/deletions changelog of a dataset: https://github.com/inrupt/solid-client-js/commit/01133a85ca4a7d003fe60738127ae0ddcb69bf76#diff-66655e826d3c9a3644992d4184386acb8eab85f2f9f829d4dde6a1ac869ef9d7R207 The reason for this was the following:
This is needed because we cannot reconcile Blank Nodes in additions and deletions. Down the road, we should do a diff before saving a SolidDataset against a saved copy of the originally-fetched one, based on our own data structures, which should make it easier to reconcile.
This reason is valid, in the sense that there is no knowledge between adding/removing blank nodes to know "what blank node" you want to remove (in the current implementation). However, this is mainly an issue with deletions and not additions. Having this filter prevents any possibility for developers to add a Thing with a blank node.
I have kept the filter in the deletions changelog (where the issue outlined in the original problem statement). This should enable setThing to store blank nodes when developers use the (undocumented) addTerm method or when they bypass the API entirely (i.e. when serializing N3 Quads to a Thing).
Why I feel this change is needed
Blank nodes is something that is implicitly supported with addTerm (unit test example for blank node https://github.com/inrupt/solid-client-js/blob/main/src/thing/add.test.ts#L2048-L2060) and the interface itself that implies its support. A general explanation on why I would need blank nodes seems a bit trivial as this applies to the general use of blank nodes on the semantic web.
getThing itself supports blank nodes, enforcing the 'assumption' that setThing would support it as well.
For example a simple observation from the SOSA ontology: https://www.w3.org/TR/vocab-ssn/#apartment-134 that uses nested blank nodes (lets assume you want to share your electricity consumption with your energy provider for this use case). The information contained in these blank nodes are very specific to the named node Observation and should not be named in this particular case.
<Observation/235714> rdf:type sosa:Observation ;
sosa:observedProperty <apartment/134/electricConsumption> ;
sosa:madeBySensor <sensor/926> ;
sosa:hasResult [
rdf:type qudt-1-1:QuantityValue ;
qudt-1-1:numericValue "22.4"^^xsd:double ;
qudt-1-1:unit qudt-unit-1-1:Kilowatthour ] ;
sosa:phenomenonTime [
rdf:type time:Interval ;
time:hasBeginning [
rdf:type time:Instant ;
time:inXSDDateTimeStamp "2017-04-15T00:00:00+00:00"^^xsd:dateTimeStamp ] ;
time:hasEnd [
rdf:type time:Instant ;
time:inXSDDateTimeStamp "2017-04-16T00:00:00+00:00"^^xsd:dateTimeStamp ] ] ;
sosa:resultTime "2017-04-16T00:00:12+00:00"^^xsd:dateTimeStamp .
Compatibility
The change does not break any unit tests. By keeping the filter in the deletions, it will not break documented API behaviour without limiting functionalities for developers using addTerm or bypassing the Thing builder.
A unit test is created to demonstrate the change.
This pull request is automatically built and testable in CodeSandbox.
To see build info of the built libraries, click here or the icon next to each commit SHA.
Latest deployment of this branch, based on commit 580fd39af11422293456542c99d6a44623490eac:
| Sandbox | Source |
|---|---|
| solid-client-sandbox | Configuration |
This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.
🔍 Inspect: https://vercel.com/inrupt/solid-client-js/9PQMquMZC8Vuy2MYJmq6NuPiwLEZ
✅ Preview: https://solid-client-js-git-fork-maximvdw-main-inrupt.vercel.app
Any particular info or changes requested to this PR?
Can this PR get some attention, blank node support is a very important feature in our opinion - and after testing this PR for a few months I have not found any breaking issues yet?
@Maximvdw could you rebase this branch? I've flagged it with the team to make a decision and try to get this PR resolved.
Sorry, I lost track on this PR for a while - I will see if I can rebase it
I just introduced a couple of changes that are somewhat related to this PR, as they add more support for Blank Nodes. I'll see if I can rebase this and align it with the latest changes.