solid-client-js icon indicating copy to clipboard operation
solid-client-js copied to clipboard

Removed blank node filter in internal additions changelog to support setting a Thing with blank node(s)

Open Maximvdw opened this issue 3 years ago • 7 comments

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.

Maximvdw avatar Mar 27 '22 18:03 Maximvdw

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

codesandbox-ci[bot] avatar Mar 27 '22 18:03 codesandbox-ci[bot]

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

vercel[bot] avatar Apr 12 '22 00:04 vercel[bot]

Any particular info or changes requested to this PR?

Maximvdw avatar Apr 18 '22 21:04 Maximvdw

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 avatar Jul 27 '22 13:07 Maximvdw

@Maximvdw could you rebase this branch? I've flagged it with the team to make a decision and try to get this PR resolved.

ThisIsMissEm avatar Jul 27 '22 16:07 ThisIsMissEm

Sorry, I lost track on this PR for a while - I will see if I can rebase it

Maximvdw avatar Dec 11 '22 15:12 Maximvdw

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.

NSeydoux avatar Mar 27 '24 14:03 NSeydoux