Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Circular references lead to "too much recursion" #27

Closed
s-tittel opened this issue Jul 11, 2024 · 5 comments · Fixed by #28
Closed

Circular references lead to "too much recursion" #27

s-tittel opened this issue Jul 11, 2024 · 5 comments · Fixed by #28

Comments

@s-tittel
Copy link

s-tittel commented Jul 11, 2024

Hi and thanks for your great library!

When validating a data graph that contains circular references (e.g. hasPart / isPartOf relations) , I get:

Uncaught (in promise) InternalError: too much recursion

Here is an example to reproduce:

import N3 from 'n3'
import { Validator } from 'shacl-engine'

const dataset = new N3.Store()
const parser = new N3.Parser()
parser.parse(
`
@prefix sh:      <http://www.w3.org/ns/shacl#> .
@prefix rdfs:    <http://www.w3.org/2000/01/rdf-schema#> .
@prefix ex: <http://example.org/> .

ex:Thing
    a sh:NodeShape, rdfs:Class ;
    sh:property [ 
	          sh:path ex:hasPart ;
	          sh:node ex:ThingPart
	        ] .

ex:ThingPart
    a sh:NodeShape ;
    sh:property [ 
	          sh:path ex:isPartOf ;
	          sh:node ex:Thing ;
	        ] .        

ex:thing
    a ex:Thing ;
    ex:hasPart ex:part .

ex:part
    ex:isPartOf ex:thing .
`,
async (error, quad) => {
    if(error) throw error
    if (quad)
        dataset.addQuad(quad)
    else {
        // create a validator instance for the shapes in the given dataset
        const validator = new Validator(dataset, { factory: N3.DataFactory })

        // run the validation process
        const report = await validator.validate({ dataset })
        console.log(`conforms: ${report.conforms}`)
    }
})

Edit: I think this directly relates to rdf-ext/grapoi#13

@bergos
Copy link
Member

bergos commented Jul 30, 2024

@s-tittel Thanks for reporting! Sorry that it took a bit longer, but I was on vacation. I had a brief look, could reproduce the problem, and already have a solution in mind. It's similar to the grapoi problem but a bit different. Besides the visited focus node, we also need to keep the term of the shape in memory, as multiple shapes will visit the same focus node. And grapoi handles only a single sh:path walk and doesn't have the shape content. The check needs to be implemented in the shacl-engine. I will have a closer look in the next few days and implement a fix.

@s-tittel
Copy link
Author

Hi, thanks! Just fyi, the rdf-validate-shacl library had the same issue which had been fixed by this PR. I am curious about your solution since I would like to switch to your library because of its ~200 KB smaller bundle size :-)

@bergos
Copy link
Member

bergos commented Aug 4, 2024

I have a working solution, but I want to double-check if it is correct and works for some edge cases. A report edge case is mentioned here. That was easy to solve as grapoi keeps track of the path and it only requires adding the path to the id of the processed shapes/focus nodes. I will get back to you once I finish my further tests.

@bergos
Copy link
Member

bergos commented Aug 5, 2024

I merged my changes and released a new patch version. No parameter like maxNodeChecks was required. The test was adapted a bit. Some names have been changed, making it clearer what is a shape and what is a class. Also, blank nodes have been changed to named nodes, which makes it easier to compare the expected results. And one sh:node has been replaced with a sh:or. A separate report is generated internal for logical shapes (or, and, ...). That use case should also be covered by the test.

@s-tittel
Copy link
Author

s-tittel commented Aug 6, 2024

Perfect, thanks! I am now using your library.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants