search

digitalbazaar / jsonld.js

A JSON-LD Processor and API implementation in JavaScript
https://json-ld.org/
Other
1.66k stars 195 forks source link

Fix for: Framing does not respect specified value #300 #374

Closed michaelcpuckett closed 4 years ago

michaelcpuckett commented 4 years ago

This fixes #300 to match behavior in RDF Distiller.

Expected Behaviour: Given a document with 2 persons "Jane" and "John" and a frame for "givenName": "John" I expect the result to contain only John, but not Jane.

Actual Behaviour: The result contains both persons, just that Janes givenName is set to null

This moves the initial check on the node to see if it has any properties so that value matches can be checked.

I added a test based on the issue.

(Originally opened #373 but spotted an issue after checking the test-suites -- Now all tests are passing locally.)

michaelcpuckett commented 4 years ago

This is passing the new test, but a more complicated test is failing locally. I'll leave this open while I look further.

davidlehn commented 4 years ago

A test like this should be added to the main test suite too.

michaelcpuckett commented 4 years ago

OK, it's working now.

I moved the 3 tests to here:

https://github.com/w3c/json-ld-framing/pull/100

I emailed the group regarding adding the tests.

gkellogg commented 4 years ago

You can remove the following lines from test-common:

        /frame-manifest.jsonld#tin01$/,
        /frame-manifest.jsonld#tin02$/,

This still leaves 2 list tests and one included test, which might have a related fix.

gkellogg commented 4 years ago

It ended up that tin03 was failing because framing was order dependent. I updated the test to simplify, so tin03 passes too and the following can be removed from test-common.js:

        // included
        /frame-manifest.jsonld#tin01$/,
        /frame-manifest.jsonld#tin02$/,
        /frame-manifest.jsonld#tin03$/,
davidlehn commented 4 years ago

Thanks!