json-ld-api icon indicating copy to clipboard operation
json-ld-api copied to clipboard

Is the possibility of having `@id: null` in an expanded document intended?

Open timothee-haudebourg opened this issue 5 years ago • 8 comments

In the playground implementation, using a keyword-like value as @id causes the node id to explicitly expand to null. I see why it happens since it is the result of the IRI expansion. However this causes the expanded document to be an invalid JSON-LD document, since putting null for @id is forbidden.

Here is an example where the expanded output is an invalid input.

timothee-haudebourg avatar Apr 23 '20 20:04 timothee-haudebourg

This could be addressed by merging steps 13.4.3.1 and 13.4.3.2 in the Expansion algorithm, to check that the result of IRI expanding the value is a string, which it shouldn't be because the @foo value gets ignored and replaced with null. The same might be said of @type, but as this is an odd-case, it might be simpler to add a blanket statement that if keyword-like values are ignored, they should not be used for adding values to results on expansion, without trying to track down every place that would check before and after expansion.

In this case, it's either an error, or no @id property should be added to the result.

gkellogg avatar Apr 23 '20 22:04 gkellogg

This odd case happens in the expansion test 122. The output of this test is not a valid JSON-LD document.

timothee-haudebourg avatar Apr 24 '20 09:04 timothee-haudebourg

This issue was discussed in a meeting.

  • RESOLVED: Defer api #480 until future version, as error case of assigning a non-URI as a URI leads to a further (worse) error case, and the solution would be far reaching
View the transcript Rob Sanderson: https://github.com/w3c/json-ld-api/issues/480
Gregg Kellogg: This is an implication of ignoring keyword-like things.
… The intention is that if you see something that looks like a keyword to be ignired, it would not result in any properties that would be added to the expansion output.
… In this case, this reveals that there may be other cases requiring a more substantial change. So we may have to work with this, and tackle it in the future.
… It would only be a problem for docs using invalid keywords that are ignored in any case. Those cases may result in invalid documents. There is nothing inherrently wrong with this, but we may need a proper solution in the future.
… @json is only valid for @type values, so if someone would use it as a value for a node, there is no logic that would transform it into rdf:json. Since it is a valid keyword, it could result in a URI if you have an @vocab.
… In CBOR, that is not a keyword, the result would be null. Same thing if @type is used in a node object. Neither of those cases would produce anything invalid. It’s a special case. The problem may be entirely contained to @id.
Rob Sanderson: We should defer to future version. This is a an edge case that would result in more weirdness through the algorithm.
Proposed resolution: Defer api #480 until future version, as error case of assigning a non-URI as a URI leads to a further (worse) error case, and the solution would be far reaching (Rob Sanderson)
Gregg Kellogg: We could annotate the test related to this that this results in invalid jsonld.
Gregg Kellogg: +1
Ruben Taelman: +1
Rob Sanderson: +1
Ivan Herman: +
Pierre-Antoine Champin: +1
Benjamin Young: +1
Resolution #6: Defer api #480 until future version, as error case of assigning a non-URI as a URI leads to a further (worse) error case, and the solution would be far reaching

iherman avatar Apr 24 '20 17:04 iherman

In my implementation I ignore ids expanded to null, so the output of the expansion test 122 is the following valid document:

[
  {
    "http://example.org/vocab/foo.bar": [
      {
        "@id": "http://example.org/@foo.bar"
      }
    ],
    "http://example.org/vocab/ignoreme": [
      {}
    ],
    "http://example.org/vocab/at": [
      {
        "@id": "http://example.org/@"
      }
    ]
  }
]

It seems to me to be the right solution, although I'm a bit worried about

leads to a further (worse) error case

I'm not sure to understand why, and if it applies to my solution. So for now I'm stuck with this only failing test, and I'm not comfortable allowing invalid documents to be represented by my datatype model.

timothee-haudebourg avatar Apr 27 '20 17:04 timothee-haudebourg

@timothee-haudebourg This is addressed in #481 by marking expand/0122 non-normative. The issue remains deferred for the future.

gkellogg avatar Apr 27 '20 18:04 gkellogg

This issue also impacts the deserialization test suite. Test #e122 uses the above JSON-LD document as input. I believe the test can only pass using the "invalid" output of the expansion algorithm.

  • If {"@id": "@ignoreMe"} is expanded into {"@id": null} (which is not a valid JSON-LD document) then it matches step 6.1 of the Node Map Generation Algorithm, and no blank node identifier is generated for it. However it will be later discarded by the Deserialize JSON-LD to RDF Algorithm since null is not a well-formed IRI.
  • If it is expanded into {} so to have a valid JSON-LD document (as in my implementation), then it does not match step 6.1, and a new blank node identifier is created for it, and it will not be discarded.

If I can give my opinion, {"@id": null} should not be generated by the expansion algorithm. But even if it is, it should be treated as "there is no @id" in the Node Map Generation Algorithm, and a blank node identifier should be generated anyway.

timothee-haudebourg avatar Sep 02 '21 14:09 timothee-haudebourg

I agree that the expansion algorithm shouldn't generate {"@id": null}, and we'll address this at a later point.

My own implementation, which doesn't follow the spec, also generates a different output:

_:b0 <http://example.org/vocab/foo.bar> <http://example.org/@foo.bar> .
_:b0 <http://example.org/vocab/ignoreme> _:b1 .
_:b0 <http://example.org/vocab/at> <http://example.org/@> .

This is likely because the "@id": null is dropped as being invalid, and the remaining {} creates a new blank node, as there is no @id, but I'd need to look more closely to be sure.

gkellogg avatar Sep 02 '21 21:09 gkellogg

I went full circle today trying to understand why my implementation could not pass the toRdf test #te122, just to find my own previous comment explaining to my present self why. Since expand/0122 is marked as non normative, shouldn't toRdf/e122 also be marked as non normative as well?

timothee-haudebourg avatar Dec 16 '22 13:12 timothee-haudebourg