Fix removal of blank nodes #207
Conversation
removeMany already removes the statements related to the top-level blank node, but we need to keep the recursive call to support nested blank nodes
this ensures that RDF type statements are added properly after removal type statements may be removed from `store` in the `clear()` call, but not from `rdf` so, if we use `rdf` these statements will never be re-created
note this is what I inferred from the code, so I may be wrong here...
|
|
||
| store.addAll(createQuads(data, rdf, shape)) | ||
| // Create new statements from form data, and add them to the RDF store | ||
| store.addAll(createQuads(data, store, shape)) |
There was a problem hiding this comment.
rdf changed here to store but then serializer uses rdf still? and in the end store statements are serialized... really don't understand this part, sorry
There was a problem hiding this comment.
Hi @MarekSuchanek thanks for taking a look at this.
I changed rdf to store in the call to createQuads() because, as far as I can see, that function needs access to the modified store (store), instead of the original unmodified one from the response (rdf). I think this is in line with the previous implementation, where the original rdf argument was modified.
The createQuads() function only uses this store (the originalRdf argument) to determine whether to add RDF type statements:
FAIRDataPoint-client/src/components/ShaclForm/formData.ts
Lines 83 to 86 in 39e56f5
So, if originalRdf equals rdf, i.e. the original unmodified rdf store from the response, createQuads will never recreate type statements, even if they have been removed from the copied store in the clear() call.
Actually I wonder if the filter in createQuads() L85 is even still necessary now that clear() removes everything properly...
As for the serializer part... I did not touch that.
It is indeed strange that the Serializer is instantiated with rdf
only to add statements from the store in the end
FAIRDataPoint-client/src/components/ShaclForm/formData.ts
Lines 177 to 178 in 39e56f5
Although I would expect the Serializer to be instantiated with store as well, instead of rdf, it has been like this since 2020 and appears to work.
Also not sure why the serialization is done this way instead of using e.g. $rdf.serialize().
(actually it turns out that was used in the previous implementation)
Unfortunately, the rdflib.js docs are not very helpful here...
There was a problem hiding this comment.
OK, sounds good... unfortunately, cannot help much with this. Maybe custom serialization has been done because of requirement on "nice" prefixes and this sir flag on serializer (98952cb#diff-ef3921a4436ad0844211507355b994db998926fe0693538d6972c9711ab8a71aR87)?
rdflib.js docs are and always were a big pain, but that is the case for most of RDF libraries in many languages for some reason 😞
From the code, I inferred that the
clear()function informData.tsis supposed to remove all RDF statements related to the fields rendered in the HTML form from the in-memory RDF store.These statements are reconstructed later, in
createQuads(), based on the form data.However, the
clear()function failed to remove the blank node type statements, because these are not included infield.nodeshape.fields(instead, they are found infield.nodeshape.targetClasses). So we were left with orphaned statements like[ a foaf:Agent ].as described in #200.To fix this, I added the line
store.removeMany(statement.object), which (afaik) actually removes all statements related to the blank node, including the type statement. The recursive call toclear()is still necessary to cover the case of nested blank nodes.This fixed the removal of the blank node, so we did not see the orphaned type statements anymore (
[ a foaf:Agent ].). However,createQuads()subsequently failed to restore these type statements when processing the form data.I assumed this was caused by the fact that
createQuads()used the original RDF store (rdf) for reference, instead of the updated RDF store (store), so I changed that as well.fixes #200