DRAFT: RDF-Star support #78

jeswr · 2022-07-18T01:13:55Z

This is to track the branch containing the addition of RDF-Star support - it is still in progress, and no action is required by anyone but myself at this time.

Currently following the suggestion of w3c/rdf-star#269 (comment) to customize some of the loading to support the way the manifest file is published.

Solves #77

There are 2 key issues that I am running into at the moment

The evaluation tests (as opposed to syntax tests) error out and then say 0/0 tests succeeded (see below) - I think the error is because the file extenions in the test suite use .trig and .ttl rather than .trig-star and .ttl-star which means that the N3Parser then has RDFStar parsing disabled for the parsing in the eval tests.

 > node ../rdf-test-suite.js/bin/Runner.js spec/parser.js https://w3c.github.io/rdf-star/tests/trig/eval/manifest.jsonld -i '{ "format": "trig-star" }' -c .rdf-test-suite-cache/

Error: Unexpected RDF* syntax on line 1.
Error: Unexpected RDF* syntax on line 1.
Error: Unexpected RDF* syntax on line 2.
Error: Unexpected RDF* syntax on line 2.
Error: Unexpected RDF* syntax on line 2.
Error: Unexpected RDF* syntax on line 4.
Error: Unexpected RDF* syntax on line 2.
Error: Unexpected RDF* syntax on line 2.
Error: Unexpected RDF* syntax on line 3.
Error: Unexpected RDF* syntax on line 2.
Error: Unexpected RDF* syntax on line 1.
Error: Unexpected RDF* syntax on line 1.
✔ 0 / 0 tests succeeded!

is that it appears to work if you use manifest for an individual syntax or evaluation suite such as https://w3c.github.io/rdf-star/tests/nt/syntax/; but if you use the manifest for the full test suite https://w3c.github.io/rdf-star/tests/nt/syntax/manifest.ttl then the submanifests contain no test cases and hence no tests are run.

I'm yet to diagnose if these have the same root cause

jeswr · 2022-11-17T14:54:52Z

lib/IManifest.ts

@@ -35,7 +36,7 @@ export async function manifestFromResource(testCaseHandlers: {[uri: string]: ITe
                manifestFromSpecificationResource(testCaseHandlers, options, specificationResource) }))))) : null,
    subManifests: await Promise.all<IManifest>([].concat.apply([],
      resource.properties.include.map((includeList: Resource) => includeList.list
-        .map(manifestFromResource.bind(null, testCaseHandlers, options))))),
+        .map(resource => new ManifestLoader({ testCaseHandlers }).from(resource.value, options))))),


@rubensworks - everything is working as I would now expect when I'm running the spec tests on SPARQL.js.

Without this change the test suites of nested submanifests were not run. This change also causes 3 failures in the test suite; as the test suite is now trying to fetch non-existent manifests when previously the fetch function was not invoked.

I believe that this is actually the behavior that is wanted, and that the tests should be fixed. But since I'm not overly familiar with this repository I would appreciate if you could confirm my logic is correct before I forge ahead and mock those tests.

It's been a while since I worked in this code, but if I remember correctly, the helpers inside IManifest were not supposed to be aware of ManifestLoader, only the other way around (to avoid cycles in the architecture).

I suspect there may be a mismatch in how (sub)manifests are structured in the RDF-star test suite versus how its currently implemented here.
So we may to loosen the implementation here to handle that case, which should be possible without directly invoking ManifestLoader I think.

If I remember correctly, submanifests are also used in the SPARQL 1.1 test suite (as used in comunica).
Perhaps this can help in identifying the mismatch?

@rubensworks so the problem is that the root manifest does mf:include on uris with the .ttl extension (see https://w3c.github.io/rdf-star/tests/manifest.ttl); however the manifest within the document that is traversed to does not contain that extension (see https://w3c.github.io/rdf-star/tests/turtle/syntax/manifest.ttl).

On the other hand

rdf-test-suite.js/lib/IManifest.ts

Lines 37 to 38 in 5b795e0

resource.properties.include.map((includeList: Resource) => includeList.list

.map(manifestFromResource.bind(null, testCaseHandlers, options))))),

assumes they have the same URI

This is the same problem that is discussed in w3c/rdf-star#269 (comment)

IMO this is poor practise for linked data publishing. However w3c/rdf-star#269 (comment) doesn't actually specify whether the URI in the entries list is the URL at which the manifest entries should be located or whether it is the IRI of the manifest itself.

Hrm, that's annoying indeed. Let's report it (again?), and add a hardcoded workaround for this case?

coveralls · 2022-11-19T00:36:55Z

Pull Request Test Coverage Report for Build 3513089150

11 of 11 (100.0%) changed or added relevant lines in 4 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 100.0%

Totals
Change from base Build 2690589889:	0.0%
Covered Lines:	763
Relevant Lines:	763

💛 - Coveralls

jeswr · 2022-11-21T07:58:23Z

I believe this is ready to go @rubensworks :)

rubensworks

Super, thanks @jeswr!

Will release a new version once you have confirmed my 2 comments.

rubensworks · 2022-11-21T08:12:58Z

lib/IManifest.ts

+      // @see https://github.com/rubensworks/rdf-test-suite.js/pull/78/files#r1026326410
+      ((resource.properties.include.length > 0 || !objectLoader)
+        ? resource
+        : (objectLoader.resources?.[resource.value.slice(0, resource.value.lastIndexOf('.')).replace(/\/manifest$/, '#manifest')] ?? resource)


Could you do a quick manual check to see if this doesn't break anything when running the tests in other places? For example Comunica and https://github.com/rubensworks/jsonld-streaming-parser.js ?

I've now checked both @rubensworks - and it is the same before and after switching to this version.

PS are you aware that the jsonld-streaming-parser is failing 4 spec tests at the moment (this occurred both before and after switching from the npm release to the current branch)

✖ property-scoped contexts which are alias of @nest Nesting terms may have property-scoped contexts defined. Error: Illegal keyword alias in term value, found: 'nest': '{"@id":"@nest"}' at ContextParser.parseInnerContexts (/home/jeswr/github/jsonld-streaming-parser.js/node_modules/jsonld-context-parser/lib/ContextParser.js:550:35) at processTicksAndRejections (node:internal/process/task_queues:96:5) at async ContextParser.parse (/home/jeswr/github/jsonld-streaming-parser.js/node_modules/jsonld-context-parser/lib/ContextParser.js:663:13) at async EntryHandlerKeywordContext.handle (/home/jeswr/github/jsonld-streaming-parser.js/lib/entryhandler/keyword/EntryHandlerKeywordContext.js:33:46) at async JsonLdParser.newOnValueJob (/home/jeswr/github/jsonld-streaming-parser.js/lib/JsonLdParser.js:201:21) at async JsonLdParser.executeBufferedJobs (/home/jeswr/github/jsonld-streaming-parser.js/lib/JsonLdParser.js:407:21) More info: https://w3c.github.io/json-ld-api/tests/toRdf-manifest#tc037 ✖ Bibframe example (poor-mans inferrence) Nesting terms may have property-scoped contexts defined. Error: Illegal keyword alias in term value, found: 'contributionByRole': '{"@id":"@nest"}' at ContextParser.parseInnerContexts (/home/jeswr/github/jsonld-streaming-parser.js/node_modules/jsonld-context-parser/lib/ContextParser.js:550:35) at processTicksAndRejections (node:internal/process/task_queues:96:5) at async ContextParser.parse (/home/jeswr/github/jsonld-streaming-parser.js/node_modules/jsonld-context-parser/lib/ContextParser.js:663:13) at async EntryHandlerKeywordContext.handle (/home/jeswr/github/jsonld-streaming-parser.js/lib/entryhandler/keyword/EntryHandlerKeywordContext.js:33:46) at async JsonLdParser.newOnValueJob (/home/jeswr/github/jsonld-streaming-parser.js/lib/JsonLdParser.js:201:21) at async JsonLdParser.executeBufferedJobs (/home/jeswr/github/jsonld-streaming-parser.js/lib/JsonLdParser.js:407:21) More info: https://w3c.github.io/json-ld-api/tests/toRdf-manifest#tc038 ✖ compact IRI as @vocab Verifies that @vocab defined as a compact IRI expands properly Error: Invalid data parsing Input: { "@context": [ { "@version": 1.1, "ex": { "@id": "http://example.org/", "@prefix": true } }, { "@vocab": "ex:ns/" } ], "foo": "bar" } Expected: [ { "subject": "_:b219_b0", "predicate": "http://example.org/ns/foo", "object": "\"bar\"", "graph": "" } ] Got: [ { "subject": "_:df_221_0", "predicate": "ex:ns/foo", "object": "\"bar\"", "graph": "" } ] More info: https://w3c.github.io/json-ld-api/tests/toRdf-manifest#te124 ✖ term as @vocab Verifies that @vocab defined as a term expands properly Error: Invalid data parsing Input: { "@context": [ { "@version": 1.1, "ex": { "@id": "http://example.org/", "@prefix": true } }, { "@vocab": "ex" } ], "foo": "bar" } Expected: [ { "subject": "_:b220_b0", "predicate": "http://example.org/foo", "object": "\"bar\"", "graph": "" } ] Got: [ { "subject": "_:df_222_0", "predicate": "https://w3c.github.io/json-ld-api/tests/toRdf/exfoo", "object": "\"bar\"", "graph": "" } ] More info: https://w3c.github.io/json-ld-api/tests/toRdf-manifest#te125 ✖ 458 / 462 tests succeeded! (skipped 11)

Alright, perfect, thanks for checking!

PS are you aware that the jsonld-streaming-parser is failing 4 spec tests at the moment

Ah, that's new! Probably because of a recent addition in the test suite.
Will probably run into this very soon when implementing JSON-LD-star there :-)

lib/ManifestLoader.ts

rubensworks · 2022-11-21T09:59:05Z

Released as 1.20.0.

jeswr mentioned this pull request Jul 18, 2022

Running when manifest URL does not match resource URI in document #77

Closed

jeswr added 4 commits July 20, 2022 22:45

chore: document rdf-sparql-star test suites

c21f7e4

feat: allow loading of rdf-sparql-star test suites

9e9d1a4

feat: support RDF-star manifests assuming correct parsing in N3.js

ee09176

WIP

31a3bb9

jeswr force-pushed the feat/rdf-sparql-star branch from b02085d to 31a3bb9 Compare July 20, 2022 12:45

jeswr added 6 commits July 21, 2022 10:15

fix: execute submanifests

67b5d56

WIP: failing manifest test

24d2825

chore: add nested loading test

743be6e

chore: add newline character to sample_manifest.ttl

0ca8276

chore: fix errors with core spec tests not running

3030826

chore: make sure rdf-star submanifests are fetched

846ab6f

jeswr commented Nov 17, 2022

View reviewed changes

chore: add workaround for rdf-star test suite

bfba75b

jeswr marked this pull request as ready for review November 19, 2022 00:36

jeswr requested a review from rubensworks November 19, 2022 05:15

rubensworks approved these changes Nov 21, 2022

View reviewed changes

chore: remove old TODO

434eb94

rubensworks merged commit b131ae8 into rubensworks:master Nov 21, 2022

jeswr deleted the feat/rdf-sparql-star branch November 21, 2022 09:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DRAFT: RDF-Star support #78

DRAFT: RDF-Star support #78

jeswr commented Jul 18, 2022 •

edited

Loading

jeswr Nov 17, 2022

rubensworks Nov 17, 2022

This comment was marked as outdated.

jeswr Nov 18, 2022

jeswr Nov 18, 2022 •

edited

Loading

jeswr Nov 18, 2022

rubensworks Nov 18, 2022

coveralls commented Nov 19, 2022 •

edited

Loading

jeswr commented Nov 21, 2022

rubensworks left a comment

rubensworks Nov 21, 2022

jeswr Nov 21, 2022 •

edited

Loading

rubensworks Nov 21, 2022

rubensworks commented Nov 21, 2022

	resource.properties.include.map((includeList: Resource) => includeList.list
	.map(manifestFromResource.bind(null, testCaseHandlers, options))))),

DRAFT: RDF-Star support #78

DRAFT: RDF-Star support #78

Conversation

jeswr commented Jul 18, 2022 • edited Loading

jeswr Nov 17, 2022

Choose a reason for hiding this comment

rubensworks Nov 17, 2022

Choose a reason for hiding this comment

This comment was marked as outdated.

jeswr Nov 18, 2022

Choose a reason for hiding this comment

jeswr Nov 18, 2022 • edited Loading

Choose a reason for hiding this comment

jeswr Nov 18, 2022

Choose a reason for hiding this comment

rubensworks Nov 18, 2022

Choose a reason for hiding this comment

coveralls commented Nov 19, 2022 • edited Loading

Pull Request Test Coverage Report for Build 3513089150

💛 - Coveralls

jeswr commented Nov 21, 2022

rubensworks left a comment

Choose a reason for hiding this comment

rubensworks Nov 21, 2022

Choose a reason for hiding this comment

jeswr Nov 21, 2022 • edited Loading

Choose a reason for hiding this comment

rubensworks Nov 21, 2022

Choose a reason for hiding this comment

rubensworks commented Nov 21, 2022

jeswr commented Jul 18, 2022 •

edited

Loading

jeswr Nov 18, 2022 •

edited

Loading

coveralls commented Nov 19, 2022 •

edited

Loading

jeswr Nov 21, 2022 •

edited

Loading