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

bug in frame expansion algorithm? #532

Closed
eroux opened this issue Aug 21, 2017 · 8 comments
Closed

bug in frame expansion algorithm? #532

eroux opened this issue Aug 21, 2017 · 8 comments

Comments

@eroux
Copy link

eroux commented Aug 21, 2017

I have a frame that just specifies an id:

{
  "@id": "http://purl.bdrc.io/resource/W30020"
}

as I just want this ID to be framed. It sounds quite legit to me, but when expanding the frame, the result of the expansion is null because of 13.2. Maybe this could be true only if frame expansion flag is not present?

@eroux
Copy link
Author

eroux commented Aug 21, 2017

In fact, the spec (if I interpret it correctly) here is in contradiction with the Framing Test Test 0022: Match on @id which has

input:

{
  "@context": {"ex": "http://example.org/"},
  "@graph": [{
    "@id": "ex:Sub1",
    "@type": "ex:Type1"
  }, {
    "@id": "ex:Sub2",
    "@type": "ex:Type2"
  }]
}

frame:

{
  "@context": {"ex": "http://example.org/"},
  "@id": "ex:Sub1"
}

output:

{
  "@context": {"ex": "http://example.org/"},
  "@graph": [{
    "@id": "ex:Sub1",
    "@type": "ex:Type1"
  }]
}

Here the frame should expand to null according to the spec, while in this test it clearly isn't the case.

@eroux
Copy link
Author

eroux commented Aug 21, 2017

Note that the test is bogus anyway: with the latest frame matching algorithm I cannot find any reason why ex:Type2 wouldn't match.

@dlongley
Copy link
Member

Yes, I think it's a bug to expand the frame to null when there are no properties other than @id when the frame expansion flag is set.

Regarding why ex:Type2 won't match -- it's because the frame has an @id and the @id for the node that has an @type of ex:Type2 does not have a matching @id.

The framing algorithm text should be more clear that when the frame has an @id, it is used exclusively to match, i.e. if a node doesn't have that @id, it will not match.

@eroux
Copy link
Author

eroux commented Aug 21, 2017

Ok, then indeed the text should be completed/amended. Thank you!

@gkellogg
Copy link
Member

Yes, there's a missing clause in 13.2 which should allow an object with just @id when framing.

Note that the test is bogus anyway: with the latest frame matching algorithm I cannot find any reason why ex:Type2 wouldn't match.

No, this is a specific feature in 1.1 to match on @id. See step 1 in Frame Matching:

Node matches if it has an @id property including any IRI or blank node in the @id property in frame.

My code is here.

@gkellogg gkellogg self-assigned this Aug 21, 2017
@gkellogg gkellogg added this to the JSON-LD 1.1 milestone Aug 21, 2017
@eroux
Copy link
Author

eroux commented Aug 21, 2017

I've read the Frame matching, and I understand the intent, but the wording is not precise enough because it doesn't say that if the frame contains @ids, then nodes that don't match one of the values should not match. It only says that nodes that have these @ids match. So my initial reaction was not to decide at this point and to go to point 2, but it's not the intended behavior. So I think a wording like

If the frame contains at least one @id property, then the node matches if it has an @id property matching any IRI or blank node in the @id property of the frame, and the node does not match if it has no such property.

To express that in JavaScript-ish, the current wording says:

// 1.
if (node.id in frame.ids)
    return true;
// 2.
etc.

while the intent is

// 1.
if (frame.ids)
    return node.id in frame.ids;
// 2.
etc.

@gkellogg
Copy link
Member

Yes, that's a good point. If the frame has an @id to use for matching, and a given object has an @id that does not match, any attempt to match must be aborted. I'll tag this for update.

@gkellogg
Copy link
Member

@eroux There is a missing clause in the Expansion Algorithm 13.2. I'm amending as follows:

Otherwise, if result is a dictionary whose only key is @id, set result to null. When the frame expansion flag is set, a dictionary containing only the @id key is retained.

This style matches that used elsewhere in the algorithm. Alternatively, it could be something like the following:

Otherwise, if result is a dictionary whose only key is @id, and the frame expansion flag is not set, set result to null.

Regarding node matching in the framing algorithm, how about amending the first step as follows:

Node matches if it has an @id property including any IRI or blank node in the @id property in frame. Otherwise, if frame has a non-empty @id property which is not an empty dictionary, node does not match.

The Ruby code for this is the following:

return ids.include?(subject['@id']) if !ids.empty? && ids != [{}]

gkellogg added a commit that referenced this issue Dec 12, 2017
…en framing.

Fix frame matching algorithm to not match a node if frame has a non-empty `@id`, which is not an empty dictionary.
Fixes #532.
gkellogg added a commit that referenced this issue Dec 12, 2017
)

* Fix expansion algorithm to not reject a node containing only `@id` when framing.
Fix frame matching algorithm to not match a node if frame has a non-empty `@id`, which is not an empty dictionary.
Fixes #532.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants