removeMetadata #633

bourgeoa · 2024-02-04T15:35:04Z

store.remove() fails on triple containing a Collection

angelo-v · 2024-02-07T17:53:56Z

src/store.ts

+    const meta = this.fetcher?.appNode // this.sym('chrome://TheCurrentSession')
+    const linkNamespaceURI = 'http://www.w3.org/2007/ont/link#'
+    const kb = this
+    // removeMatches() --> removeMany() --> remove() fails on Collection


Wouldn't it be better to fix the remove() function to work with Collection instead of applying a workarround here?

Yes it would. I did not find howto.

What goes wrong with when one f the nodes being removed is a collection?

@timbl
This is I hope is a detailed enough issue #631

angelo-v · 2024-02-07T17:54:21Z

src/store.ts

    for (var r = 0; r < requests.length; r++) {
      const request = requests[r]
-      if (request !== undefined) {
-        this.removeMatches(request, null, null, meta)
+      if (request != null) { // null or undefined


The comment says "or undefined" but the code onyl checks for null. Why?

Loose inequality does it
It is used in other places in rdflib

I think Angelo is correct. If you remove line 884 which checks for undefined, the !=null will throw an undefined error if request is undefined.

angelo-v · 2024-02-07T17:57:59Z

tests/unit/update-manager-test.js

@@ -223,6 +223,17 @@ describe('UpdateManager', () => {
      expect(updater.editable(doc1)).to.equal(undefined)
    })

+    it('Should not detect a document is editable from metadata after removeMetadata', () => {


How are these tests related to the changes? I would have expected a test that assures that Collections are now working.

They test the new function removeMetadata

bourgeoa · 2024-03-09T16:40:22Z

@jeff-zucker your test return not defined because foo is not declared. ``` const foo = undefined if (foo == null) ``` Here foo is declared and has an undefined value with the equality operator the result will be true when foo is null, undefined or 0 https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Equality This is used in multiple location in Rdflib Recently Tim has discovered an issue #639 that seems due to using deep equality and not equality Le mer. 6 mars 2024, 18:46, Jeff Zucker ***@***.***> a écrit :

…

***@***.**** commented on this pull request. ------------------------------ In src/store.ts <#633 (comment)>: > for (var r = 0; r < requests.length; r++) { const request = requests[r] - if (request !== undefined) { - this.removeMatches(request, null, null, meta) + if (request != null) { // null or undefined **** I am not sure I understand this - if I create a file containing only if(foo != null) console.log(1); I get ReferenceError: foo is not **defined**. So I don't see how this can work. I so much miss perl in this regard where you could just say if(foo) and that would cover null, undefined, and empty. — Reply to this email directly, view it on GitHub <#633 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAQ5TZTTKIWTQOXFBP27L2LYW5I6RAVCNFSM6AAAAABCY6JUSWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTSMRQGQZDINJXGU> . You are receiving this because you authored the thread.Message ID: ***@***.***>

jeff-zucker · 2024-03-09T18:19:20Z

AH! Thank you. It works if the variable is declared but not defined but does not work if the variable is not declared, makes sense.

…

On Sat, Mar 9, 2024 at 8:40 AM Alain Bourgeois ***@***.***> wrote: @jeff-zucker your test return not defined because foo is not declared. ``` const foo = undefined if (foo == null) ``` Here foo is declared and has an undefined value with the equality operator the result will be true when foo is null, undefined or 0 https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Equality This is used in multiple location in Rdflib Recently Tim has discovered an issue #639 that seems due to using deep equality and not equality Le mer. 6 mars 2024, 18:46, Jeff Zucker ***@***.***> a écrit : > ***@***.**** commented on this pull request. > ------------------------------ > > In src/store.ts > <#633 (comment)>: > > > for (var r = 0; r < requests.length; r++) { > const request = requests[r] > - if (request !== undefined) { > - this.removeMatches(request, null, null, meta) > + if (request != null) { // null or undefined > > **** I am not sure I understand this - if I create a file containing only if(foo > != null) console.log(1); I get ReferenceError: foo is not **defined**. So > I don't see how this can work. I so much miss perl in this regard where you > could just say if(foo) and that would cover null, undefined, and empty. > > — > Reply to this email directly, view it on GitHub > <#633 (comment)>, > or unsubscribe > < https://github.com/notifications/unsubscribe-auth/AAQ5TZTTKIWTQOXFBP27L2LYW5I6RAVCNFSM6AAAAABCY6JUSWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTSMRQGQZDINJXGU> > . > You are receiving this because you authored the thread.Message ID: > ***@***.***> > — Reply to this email directly, view it on GitHub <#633 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AKVJCJELWNPTZWVI4JLJFY3YXM3QZAVCNFSM6AAAAABCY6JUSWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOBWHEYTAOJYGI> . You are receiving this because you were mentioned.Message ID: ***@***.***>

jeff-zucker · 2024-03-17T00:18:18Z

I ran the attached script on solidcommunity.net and it errored with statement to be removed is not in store. I got this error even though there was no collection so I don't think the collection is the problem.

The same script on the test server did not produce an error. But it also did not remove all metadata about the document. Even after the new removeDocument(), there were three statements left over. The predicates related to the document that were not removed were acl, describedBy, and type - things from the link relations.

I am not sure if it was intentional to leave these, but if not, it should be easy to fix.

<script src="https://solidcommunity.net:8443/mashlib.js"></script>                                  
<script type="module">                                                                              
                                                                                                    
let doc = UI.rdf.sym("https://localhost:8443/public/s/test/tmpr/test.ttl");                         
                                                                                                    
async function test(){                                                                              
  await UI.store.fetcher.load(doc);                                                                 
  show('load');                                                                               
  await UI.store.removeDocument(doc);                                                                     
  show('removeDocument');;                                                               
}                                                                                                   
test();                                                                                             
                                                                                                    
function show(msg){                                                                                 
  let stmts  = UI.store.match(null,null,null,doc);                                                  
  console.log(`${stmts.length} statements with graph specified after ` + msg);                      
  stmts  = UI.store.match();                                                                        
  console.log(`${stmts.length} statements with graph not specified after ` + msg);                  
}                                                                                                   
                                                                                                    
</script>

The document in question looks like this:

<#A> <#isa> <#B>.                                                                                   
<#C> <#isa> <#D>.

The script output was this (notice the 3 in the last one) :

2 statements with graph specified after load
30 statements with graph not specified after load
0 statements with graph specified after removeDocument
3 statements with graph not specified after removeDocument

jeff-zucker · 2024-03-17T02:39:54Z

Also note that removeDocument did remove everything that had the document as the graph, but the three which were not removed had the document as subject and a blank node as the graph.

bourgeoa · 2024-03-17T09:33:44Z

Thanks @jeff-zucker for looking at this in detail.

I ran the attached script on solidcommunity.net and it errored with statement to be removed is not in store. I got this error even though there was no collection so I don't think the collection is the problem.

The collection is in the metadata graph. (const meta = this.fetcher?.appNode // this.sym('chrome://TheCurrentSession'))
That is were the collection is an issue, not on the document graph
The issue is with the last object Collection (full metadata content here #631 (comment))

  [
     .......
     tabont:status
              ( "[0:16:35.259] N3 parsed: 13 triples in 26 lines."
              "[0:16:35.259] Done." )
  ].

Also note that removeDocument did remove everything that had the document as the graph, but the three which were not removed had the document as subject and a blank node as the graph.

Then these 3 statements are not related to either the graph document or the document part of the metadata graph
I shall look where these are created and if they may be removed.

They make no harm for the time being. So I prefer to keep these as an other issue.

jeff-zucker

I'll raise a separate issue on the un-removed link relations. Other than that, this looks good to go.

jeff-zucker · 2024-03-06T17:50:04Z

src/store.ts

    for (var r = 0; r < requests.length; r++) {
      const request = requests[r]
-      if (request !== undefined) {
-        this.removeMatches(request, null, null, meta)
+      if (request != null) { // null or undefined


I think Angelo is correct. If you remove line 884 which checks for undefined, the !=null will throw an undefined error if request is undefined.

removeMetadata

1beb2c7

bourgeoa requested review from angelo-v and timbl February 4, 2024 15:35

bourgeoa linked an issue Feb 4, 2024 that may be closed by this pull request

deleteDocument() : Error : Statement to be removed is not on store #631

Open

angelo-v reviewed Feb 7, 2024

View reviewed changes

bourgeoa mentioned this pull request Feb 10, 2024

deleteDocument() : Error : Statement to be removed is not on store #631

Open

bourgeoa mentioned this pull request Feb 18, 2024

Chat thread with signed messages SolidOS/solid-ui#546

Open

29 tasks

add commment

9ca72b5

jeff-zucker self-requested a review March 16, 2024 22:19

jeff-zucker approved these changes Mar 17, 2024

View reviewed changes

jeff-zucker mentioned this pull request Mar 17, 2024

RemoveDocument() leaves link relations in the store. #640

Open

bourgeoa merged commit 7a40abf into main Mar 17, 2024
6 checks passed

bourgeoa deleted the issue#631 branch March 17, 2024 14:21

bourgeoa restored the issue#631 branch March 22, 2024 18:20

bourgeoa mentioned this pull request Mar 22, 2024

Revert "removeMetadata" #641

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

removeMetadata #633

removeMetadata #633

bourgeoa commented Feb 4, 2024 •

edited

Loading

angelo-v Feb 7, 2024

bourgeoa Feb 7, 2024

timbl Feb 8, 2024

bourgeoa Feb 10, 2024 •

edited

Loading

angelo-v Feb 7, 2024

bourgeoa Feb 7, 2024

jeff-zucker Mar 6, 2024

angelo-v Feb 7, 2024

bourgeoa Feb 7, 2024 •

edited

Loading

bourgeoa commented Mar 9, 2024 via email

jeff-zucker commented Mar 9, 2024 via email

jeff-zucker commented Mar 17, 2024 •

edited

Loading

jeff-zucker commented Mar 17, 2024

bourgeoa commented Mar 17, 2024 •

edited

Loading

jeff-zucker left a comment

jeff-zucker Mar 6, 2024

removeMetadata #633

removeMetadata #633

Conversation

bourgeoa commented Feb 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bourgeoa Feb 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bourgeoa Feb 7, 2024 • edited Loading

Choose a reason for hiding this comment

bourgeoa commented Mar 9, 2024 via email

jeff-zucker commented Mar 9, 2024 via email

jeff-zucker commented Mar 17, 2024 • edited Loading

jeff-zucker commented Mar 17, 2024

bourgeoa commented Mar 17, 2024 • edited Loading

jeff-zucker left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bourgeoa commented Feb 4, 2024 •

edited

Loading

bourgeoa Feb 10, 2024 •

edited

Loading

bourgeoa Feb 7, 2024 •

edited

Loading

jeff-zucker commented Mar 17, 2024 •

edited

Loading

bourgeoa commented Mar 17, 2024 •

edited

Loading