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

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

Open
timothee-haudebourg opened this issue Apr 23, 2020 · 8 comments
Labels
defer-future-version Defer this issue until a future version of JSON-LD ErratumRaised wr:spec-updated

Comments

@timothee-haudebourg
Copy link

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 timothee-haudebourg changed the title Is the possibility of having @id: null in a expanded document intended? Is the possibility of having @id: null in an expanded document intended? Apr 23, 2020
@gkellogg
Copy link
Member

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 gkellogg added defer-future-version Defer this issue until a future version of JSON-LD wr:open labels Apr 23, 2020
@azaroth42 azaroth42 added this to Discuss-Call in JSON-LD Management Apr 23, 2020
@timothee-haudebourg
Copy link
Author

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

@iherman
Copy link
Member

iherman commented Apr 24, 2020

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: #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

@timothee-haudebourg
Copy link
Author

timothee-haudebourg commented Apr 27, 2020

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.

gkellogg added a commit that referenced this issue Apr 27, 2020
…esult.

Note: toRdf/e122 does not include a triple for this, so it remains normative.

For #480.
@gkellogg
Copy link
Member

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

@gkellogg gkellogg moved this from Discuss-Call to Future Work in JSON-LD Management Apr 28, 2020
gkellogg added a commit that referenced this issue Apr 29, 2020
…esult.

Note: toRdf/e122 does not include a triple for this, so it remains normative.

For #480.
@timothee-haudebourg
Copy link
Author

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.

@gkellogg
Copy link
Member

gkellogg commented Sep 2, 2021

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.

@timothee-haudebourg
Copy link
Author

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
defer-future-version Defer this issue until a future version of JSON-LD ErratumRaised wr:spec-updated
Projects
Development

No branches or pull requests

3 participants