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

fix #125 #126

Merged
merged 3 commits into from
Apr 11, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ class SHACLValidator {
*/
validateNode(data, focusNode, shapeNode) {
this.$data = clownface({ dataset: data, factory: this.factory })
this.nodeConformsToShape(focusNode, shapeNode)
this.nodeConformsToShape(focusNode, shapeNode, this.validationEngine)
return this.validationEngine.getReport()
}

Expand All @@ -67,16 +67,20 @@ class SHACLValidator {
}

// Exposed to be available from validation functions as `SHACL.nodeConformsToShape`
nodeConformsToShape(focusNode, shapeNode) {
nodeConformsToShape(focusNode, shapeNode, engine = this.validationEngine.clone()) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why clone?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using a fresh instance of validation engine we avoid side effects (collection of result details) on the current instance.
Only when calling from a sh:node constraint we do want to affect the current instance, in the other cases (sh:and, sh:or, sh:not...) we are only interested in the boolean result and we don't care about the details.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, I wonder if that is entirely true. I was always thinking that to improve the error messages in complex scenarios would require a smarter analysis of the nested results. But maybe it's better to reorganise the shapes and write good, targeted messages...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

admittedly, this PR is a bit of hacking trying to fix the anomaly. Redesigning all the result details to provide more insights for violations of logical constraints is intriguing but beyond the scope of this PR.

const shape = this.shapesGraph.getShape(shapeNode)
try {
this.depth++
const foundViolations = this.validationEngine.validateNodeAgainstShape(focusNode, shape, this.$data)
const foundViolations = engine.validateNodeAgainstShape(focusNode, shape, this.$data)
return !foundViolations
} finally {
this.depth--
}
}

validateNodeAgainstShape (focusNode, shapeNode) {
return this.nodeConformsToShape(focusNode, shapeNode, this.validationEngine)
}
}

export default SHACLValidator
4 changes: 4 additions & 0 deletions src/validation-engine.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ class ValidationEngine {
this.nestedResults = {}
}

clone () {
return new ValidationEngine(this.context, { maxErrors: this.maxErrors })
}

initReport() {
const { rdf, sh } = this.context.ns

Expand Down
2 changes: 1 addition & 1 deletion src/validators.js
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ function validateNodeKind(context, focusNode, valueNode, constraint) {
function validateNode(context, focusNode, valueNode, constraint) {
const { sh } = context.ns
const nodeNode = constraint.getParameterValue(sh.node)
return context.nodeConformsToShape(valueNode, nodeNode)
return context.validateNodeAgainstShape(valueNode, nodeNode)
}

function validateNot(context, focusNode, valueNode, constraint) {
Expand Down
7 changes: 7 additions & 0 deletions test/data/validation-repro/repro125-data.ttl
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
@prefix ex: <https://example.org/> .

ex:person1 a ex:Person ; ex:address ex:address1 .

ex:address1 ex:city "London" .

ex:tv ex:size ex:big .
23 changes: 23 additions & 0 deletions test/data/validation-repro/repro125-shapes.ttl
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
@prefix sh: <http://www.w3.org/ns/shacl#> .
@prefix ex: <https://example.org/> .

ex:personShape a sh:NodeShape ;
sh:targetClass ex:Person ;
sh:property [
sh:path ex:address ;
sh:message "ex:city should be sh:IRI" ;
sh:node ex:cityShape
] .

ex:cityShape
sh:property [
sh:path ex:city ;
sh:nodeKind sh:IRI
] .

ex:sizeShape
sh:targetObjectsOf ex:size ;
sh:or (
[ sh:hasValue ex:small ]
[ sh:hasValue ex:big ]
) .
22 changes: 22 additions & 0 deletions test/validation_repro.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/* eslint-env mocha */
import path from 'path'
import assert from 'assert'
import * as url from 'url'
import SHACLValidator from '../index.js'
import { loadDataset } from './utils.js'

const __dirname = url.fileURLToPath(new URL('.', import.meta.url))
const rootPath = path.join(__dirname, '/data/validation-repro')

describe('validation repro', () => {
it('repro #125', async () => {
const data = await loadDataset(path.join(rootPath, 'repro125-data.ttl'))
const shapes = await loadDataset(path.join(rootPath, 'repro125-shapes.ttl'))

const validator = new SHACLValidator(shapes)
const report = validator.validate(data)

assert.equal(1, report.results.length)
assert.equal(1, report.results[0].detail.length)
})
})
Loading