From e2f77f95ed432c4b7f2121a1074039efadcf4c15 Mon Sep 17 00:00:00 2001 From: Giacomo Citi Date: Fri, 12 Apr 2024 15:50:51 +0200 Subject: [PATCH 1/4] lists in result messages --- .changeset/eighty-snakes-hide.md | 5 +++++ src/validation-engine.js | 20 +++++++++++++++++-- src/validators-registry.js | 2 +- .../validation-message/message-with-list.ttl | 11 ++++++++++ test/validation_message_test.js | 13 ++++++++++++ 5 files changed, 48 insertions(+), 3 deletions(-) create mode 100644 .changeset/eighty-snakes-hide.md create mode 100644 test/data/validation-message/message-with-list.ttl diff --git a/.changeset/eighty-snakes-hide.md b/.changeset/eighty-snakes-hide.md new file mode 100644 index 0000000..e9c3eb3 --- /dev/null +++ b/.changeset/eighty-snakes-hide.md @@ -0,0 +1,5 @@ +--- +"rdf-validate-shacl": patch +--- + +Improved result messages with lists diff --git a/src/validation-engine.js b/src/validation-engine.js index 417edce..8c926fc 100644 --- a/src/validation-engine.js +++ b/src/validation-engine.js @@ -320,7 +320,16 @@ function localName(uri) { return uri.substring(index + 1) } -function nodeLabel(node) { +function * take(n, iterable) { + let i = 0 + for (const item of iterable) { + if (i++ === n) break + yield item + } +} + +function nodeLabel(constraint, param) { + const node = constraint.getParameterValue(param) if (!node) { return 'NULL' } @@ -331,6 +340,13 @@ function nodeLabel(node) { } if (node.termType === 'BlankNode') { + const pointer = constraint.shapeNodePointer.out(param) + const list = pointer.list() + if (list) { + const items = Array.from(take(3, list)) + return items.join(', ') + (items.length === 3 ? ' ...' : '') + } + return 'Blank node ' + node.value } @@ -340,7 +356,7 @@ function nodeLabel(node) { function withSubstitutions(messageTerm, constraint, factory) { const message = constraint.component.parameters.reduce((message, param) => { const paramName = localName(param.value) - const paramValue = nodeLabel(constraint.getParameterValue(param)) + const paramValue = nodeLabel(constraint, param) return message .replace(`{$${paramName}}`, paramValue) .replace(`{?${paramName}}`, paramValue) diff --git a/src/validators-registry.js b/src/validators-registry.js index 577c188..8db4191 100644 --- a/src/validators-registry.js +++ b/src/validators-registry.js @@ -54,7 +54,7 @@ export default { [ns.sh.InConstraintComponent.value]: { validator: { func: validators.validateIn, - message: 'Value is not in {$in}', + message: 'Value is not one of the allowed values: {$in}', }, }, [ns.sh.LanguageInConstraintComponent.value]: { diff --git a/test/data/validation-message/message-with-list.ttl b/test/data/validation-message/message-with-list.ttl new file mode 100644 index 0000000..b4f8c4a --- /dev/null +++ b/test/data/validation-message/message-with-list.ttl @@ -0,0 +1,11 @@ +@prefix sh: . +@prefix xsd: . + + a ; "f" . + + a sh:NodeShape ; + sh:targetClass ; + sh:property [ + sh:path ; + sh:in ("a" "b" "c" "d" "e") ; + ] . diff --git a/test/validation_message_test.js b/test/validation_message_test.js index ac482e6..45b6b6e 100644 --- a/test/validation_message_test.js +++ b/test/validation_message_test.js @@ -89,4 +89,17 @@ describe('validation messages', () => { { value: 'Mon message de validation', language: 'fr' }, ])) }) + + it('Lists first items in message', async () => { + const dataPath = path.join(rootPath, 'message-with-list.ttl') + const data = await loadDataset(dataPath) + const shapes = data + + const validator = new SHACLValidator(shapes) + const report = validator.validate(data) + + assert.strictEqual(report.results.length, 1) + assert.strictEqual(report.results[0].message.length, 1) + assert.strictEqual(report.results[0].message[0].value, 'Value is not one of the allowed values: a, b, c ...') + }) }) From 97421ee54081addc5ae6b2922a37410985ee745d Mon Sep 17 00:00:00 2001 From: Giacomo Citi Date: Thu, 18 Apr 2024 12:49:06 +0200 Subject: [PATCH 2/4] replace sh:in with comment in sourceShape --- src/dataset-utils.js | 47 +++++++++++++++---- src/validation-engine.js | 15 ++++-- .../data/validation-sourceShape/long-list.ttl | 12 +++++ test/validation_sourceShape_test.js | 33 +++++++++++++ 4 files changed, 95 insertions(+), 12 deletions(-) create mode 100644 test/data/validation-sourceShape/long-list.ttl create mode 100644 test/validation_sourceShape_test.js diff --git a/src/dataset-utils.js b/src/dataset-utils.js index c686550..daba9fd 100644 --- a/src/dataset-utils.js +++ b/src/dataset-utils.js @@ -7,21 +7,52 @@ import NodeSet from './node-set.js' * * @param {DatasetCore} dataset * @param {Term} startNode - * @returns Array of quads + * @yields {Quad} */ -export function extractStructure(dataset, startNode, visited = new TermSet()) { +export function * extractStructure(dataset, startNode, visited = new TermSet()) { if (startNode.termType !== 'BlankNode' || visited.has(startNode)) { - return [] + return } visited.add(startNode) - const quads = [...dataset.match(startNode, null, null)] + for (const quad of dataset.match(startNode, null, null)) { + yield quad + yield * extractStructure(dataset, quad.object, visited) + } +} + +/** + * Extracts all the quads forming the structure under a blank shape node. Stops at + * non-blank nodes. Replaces sh:in with a comment if the list is too long. + * + * @param {Shape} shape + * @param {DatasetCore} dataset + * @param {Term} startNode + * @yields {Quad} + */ +export function * extractSourceShapeStructure(shape, dataset, startNode, visited = new TermSet()) { + if (startNode.termType !== 'BlankNode' || visited.has(startNode)) { + return + } - const children = quads.map((quad) => { - return extractStructure(dataset, quad.object, visited) - }) + const { factory } = shape.context + const { sh, rdfs } = shape.context.ns - return quads.concat(...children) + const inListSize = term => { + const inConstraint = shape.constraints.find(x => x.paramValue.equals(term)) + return inConstraint?.nodeSet.size + } + + visited.add(startNode) + for (const quad of dataset.match(startNode, null, null)) { + if (quad.predicate.equals(sh.in) && inListSize(quad.object) > 3) { + const msg = `sh:in has ${inListSize(quad.object)} elements and has been removed from the report for brevity. Please refer the original shape` + yield factory.quad(quad.subject, rdfs.comment, factory.literal(msg)) + } else { + yield quad + yield * extractSourceShapeStructure(shape, dataset, quad.object, visited) + } + } } /** diff --git a/src/validation-engine.js b/src/validation-engine.js index 8c926fc..930dc92 100644 --- a/src/validation-engine.js +++ b/src/validation-engine.js @@ -1,7 +1,7 @@ import clownface from 'clownface' import debug from 'debug' import ValidationReport from './validation-report.js' -import { extractStructure } from './dataset-utils.js' +import { extractStructure, extractSourceShapeStructure } from './dataset-utils.js' const error = debug('validation-enging::error') @@ -17,10 +17,10 @@ class ValidationEngine { this.nestedResults = {} } - clone () { + clone() { return new ValidationEngine(this.context, { maxErrors: this.maxErrors }) } - + initReport() { const { rdf, sh } = this.context.ns @@ -241,7 +241,7 @@ class ValidationEngine { .addOut(sh.sourceShape, sourceShape) .addOut(sh.focusNode, focusNode) - this.copyNestedStructure(sourceShape, result) + this.copySourceShapeStructure(constraint.shape, result) this.copyNestedStructure(focusNode, result) const children = this.nestedResults[this.recordErrorsLevel + 1] @@ -266,6 +266,13 @@ class ValidationEngine { } } + copySourceShapeStructure(shape, result) { + const structureQuads = extractSourceShapeStructure(shape, this.context.$shapes.dataset, shape.shapeNode) + for (const quad of structureQuads) { + result.dataset.add(quad) + } + } + /** * Creates a result message from the validation result and the message pattern in the constraint */ diff --git a/test/data/validation-sourceShape/long-list.ttl b/test/data/validation-sourceShape/long-list.ttl new file mode 100644 index 0000000..3dbb2cd --- /dev/null +++ b/test/data/validation-sourceShape/long-list.ttl @@ -0,0 +1,12 @@ +@prefix sh: . +@prefix ex: . + +ex:p1 a ex:Person ; ex:category 3 . +ex:p2 a ex:Person ; ex:category 30, 40 . + +ex:shape sh:targetClass ex:Person ; + sh:property [ + sh:path ex:category ; + sh:minCount 2 ; + sh:in (1 2 3 4 5) ; + ] . \ No newline at end of file diff --git a/test/validation_sourceShape_test.js b/test/validation_sourceShape_test.js new file mode 100644 index 0000000..691f593 --- /dev/null +++ b/test/validation_sourceShape_test.js @@ -0,0 +1,33 @@ +/* eslint-env mocha */ +import path from 'path' +import assert from 'assert' +import * as url from 'url' +import SHACLValidator from '../index.js' +import ns from '../src/namespaces.js' +import { loadDataset } from './utils.js' + +const { rdfs, sh } = ns + +const __dirname = url.fileURLToPath(new URL('.', import.meta.url)) +const rootPath = path.join(__dirname, '/data/validation-sourceShape') + +describe('validation source shapes', () => { + it('Includes source shape without long list', async () => { + const dataPath = path.join(rootPath, 'long-list.ttl') + const data = await loadDataset(dataPath) + const shapes = data + + const validator = new SHACLValidator(shapes) + const report = validator.validate(data) + + // three results for the same source shape + assert.strictEqual(report.results.length, 3) + const sourceShapes = new Set(report.results.map(result => result.sourceShape)) + assert.strictEqual(sourceShapes.size, 1) + const [shape] = sourceShapes + // the shape has a comment instead of sh:in + assert.deepEqual([], [...report.dataset.match(shape, sh.in, null)]) + const [comment] = report.dataset.match(shape, rdfs.comment, null) + assert.strictEqual(comment.object.value, 'sh:in has 5 elements and has been removed from the report for brevity. Please refer the original shape') + }) +}) From 9999751bbbbe4e29becc952f3999373cdfb9448c Mon Sep 17 00:00:00 2001 From: Giacomo Citi Date: Thu, 18 Apr 2024 13:46:51 +0200 Subject: [PATCH 3/4] leverage constraint.nodeSet for result message --- src/validation-engine.js | 13 ++++++++----- test/validation_message_test.js | 2 +- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/validation-engine.js b/src/validation-engine.js index 930dc92..b4bbfbc 100644 --- a/src/validation-engine.js +++ b/src/validation-engine.js @@ -347,11 +347,14 @@ function nodeLabel(constraint, param) { } if (node.termType === 'BlankNode') { - const pointer = constraint.shapeNodePointer.out(param) - const list = pointer.list() - if (list) { - const items = Array.from(take(3, list)) - return items.join(', ') + (items.length === 3 ? ' ...' : '') + if (constraint.nodeSet) { + const limit = 3 + if (constraint.nodeSet.size > limit) { + const prefix = Array.from(take(limit, constraint.nodeSet)).map(x => x.value) + return prefix.join(', ') + ` ... (and ${constraint.nodeSet.size - limit} more)` + } else { + return Array.from(constraint.nodeSet).map(x => x.value).join(', ') + } } return 'Blank node ' + node.value diff --git a/test/validation_message_test.js b/test/validation_message_test.js index 45b6b6e..d8f75e2 100644 --- a/test/validation_message_test.js +++ b/test/validation_message_test.js @@ -100,6 +100,6 @@ describe('validation messages', () => { assert.strictEqual(report.results.length, 1) assert.strictEqual(report.results[0].message.length, 1) - assert.strictEqual(report.results[0].message[0].value, 'Value is not one of the allowed values: a, b, c ...') + assert.strictEqual(report.results[0].message[0].value, 'Value is not one of the allowed values: a, b, c ... (and 2 more)') }) }) From f732f93478c88fe3f42afae31835a85fb22ea1ff Mon Sep 17 00:00:00 2001 From: Giacomo Citi Date: Mon, 6 May 2024 17:04:25 +0200 Subject: [PATCH 4/4] fix: ensure nodeList availability --- src/shapes-graph.js | 10 +++++++++- src/validators.js | 9 +-------- .../validation-sourceShape/mandatory-list.ttl | 11 +++++++++++ test/validation_sourceShape_test.js | 19 +++++++++++++++++++ 4 files changed, 40 insertions(+), 9 deletions(-) create mode 100644 test/data/validation-sourceShape/mandatory-list.ttl diff --git a/src/shapes-graph.js b/src/shapes-graph.js index 46e6ef3..270067b 100644 --- a/src/shapes-graph.js +++ b/src/shapes-graph.js @@ -20,7 +20,7 @@ import NodeSet from './node-set.js' import ValidationFunction from './validation-function.js' import validatorsRegistry from './validators-registry.js' import { extractPropertyPath, getPathObjects } from './property-path.js' -import { getInstancesOf, isInstanceOf } from './dataset-utils.js' +import { getInstancesOf, isInstanceOf, rdfListToArray } from './dataset-utils.js' class ShapesGraph { constructor(context) { @@ -124,6 +124,14 @@ class Constraint { get componentMessages() { return this.component.getMessages(this.shape) } + + get nodeSet() { + const { sh } = this.shape.context.ns + if (!this.inNodeSet) { + this.inNodeSet = new NodeSet(rdfListToArray(this.shapeNodePointer.out(sh.in))) + } + return this.inNodeSet + } } class ConstraintComponent { diff --git a/src/validators.js b/src/validators.js index ca376a5..918206d 100644 --- a/src/validators.js +++ b/src/validators.js @@ -131,14 +131,7 @@ function validateHasValueProperty(context, focusNode, valueNode, constraint) { } function validateIn(context, focusNode, valueNode, constraint) { - const { sh } = context.ns - if (!constraint.nodeSet) { - const inNode = constraint.getParameterValue(sh.in) - constraint.nodeSet = new NodeSet(rdfListToArray(context.$shapes.node(inNode))) - } - const { nodeSet } = constraint - - return nodeSet.has(valueNode) + return constraint.nodeSet.has(valueNode) } function validateLanguageIn(context, focusNode, valueNode, constraint) { diff --git a/test/data/validation-sourceShape/mandatory-list.ttl b/test/data/validation-sourceShape/mandatory-list.ttl new file mode 100644 index 0000000..43f6d3a --- /dev/null +++ b/test/data/validation-sourceShape/mandatory-list.ttl @@ -0,0 +1,11 @@ +@prefix sh: . +@prefix ex: . + +ex:p1 a ex:Person . + +ex:shape sh:targetClass ex:Person ; + sh:property [ + sh:path ex:category ; + sh:minCount 1 ; + sh:in (1 2 3) ; + ] . \ No newline at end of file diff --git a/test/validation_sourceShape_test.js b/test/validation_sourceShape_test.js index 691f593..d829371 100644 --- a/test/validation_sourceShape_test.js +++ b/test/validation_sourceShape_test.js @@ -2,8 +2,10 @@ import path from 'path' import assert from 'assert' import * as url from 'url' +import clownface from 'clownface' import SHACLValidator from '../index.js' import ns from '../src/namespaces.js' +import { rdfListToArray } from '../src/dataset-utils.js' import { loadDataset } from './utils.js' const { rdfs, sh } = ns @@ -30,4 +32,21 @@ describe('validation source shapes', () => { const [comment] = report.dataset.match(shape, rdfs.comment, null) assert.strictEqual(comment.object.value, 'sh:in has 5 elements and has been removed from the report for brevity. Please refer the original shape') }) + it('Includes source shape with list', async () => { + const dataPath = path.join(rootPath, 'mandatory-list.ttl') + const data = await loadDataset(dataPath) + const shapes = data + + const validator = new SHACLValidator(shapes) + const report = validator.validate(data) + + assert.strictEqual(report.results.length, 1) + const sourceShapes = new Set(report.results.map(result => result.sourceShape)) + assert.strictEqual(sourceShapes.size, 1) + const [shape] = sourceShapes + // the shape has the lst of allowed values even though the violated constraint is sh:minCount and not sh:in + const list = clownface({ dataset: report.dataset, term: shape }).out(sh.in) + const actual = rdfListToArray(list).map(term => term.value) + assert.deepEqual(actual, ['1', '2', '3']) + }) })