w3c / json-ld-syntax

JSON-LD 1.1 Specification
https://w3c.github.io/json-ld-syntax/
Other
116 stars 23 forks source link

Clarification needed for expansion of terms with `@container` set to `@id` and a local context with a `@base` #412

Open niklasl opened 1 year ago

niklasl commented 1 year ago

When expanding this compact data:

{
  "@context": {
    "rdf": "http://www.w3.org/1999/02/22-rdf-syntax-ns/",
    "@vocab": "https://example.org/ns#",
    "typeDescribed": {
      "@id": "rdf:type",
      "@context": {"@base": "https://example.org/ns/"},
      "@container": "@id"
    }
  },
  "@id": "/item/1",
  "typeDescribed": {
    "Item": {"label": "Item"}
  }
}

I cannot tell from the spec (step 13.8.3.7.4 of the expansion algorithm) whether the key Item will be resolved against the local context or not. Actually, reading the spec I see nothing that sets the active context to the local context of the typeDescribed before reaching 13.8.3.7.4, although I would expect that this is the intention. Both the JSON-LD Playground and the Ruby RDF Distiller do expand the above to this:

[
  {
    "@id": "/test/item/1",
    "http://www.w3.org/1999/02/22-rdf-syntax-ns#type": [
      {
        "https://example.org/ns/label": [
          {"@value": "Item"}
        ],
        "@id": "https://example.org/ns/Item"
      }
    ]
  }
]

In the Ruby code corresponding to step 13.8.2 and onwards, IIRC, a container_context is defined and from thereon used.

Unless I've missed something in the algorithm that clarifies that something like this is required, I believe we should clarify this exhaustively in the spec, and add this test case (unless I've overlooked an existing one for this).

I've made a similar adjustment to TRLD to reflect this presumably desired behaviour.

gkellogg commented 1 year ago

I think it's described using the following steps:

I think the algorithm is correct, but there are definitely a lot of moving parts. If you had passed the test suite other than this newly defined test, then that would indicate that the test coverage should be improved. Please go ahead and add a PR for the new expansion test (and corresponding toRdf test).

niklasl commented 1 year ago

I thought so too, but as my implementation is basically a transcription of the algorithm into Python (with some minor optimizations and refactored parts), I wondered why it didn't work.

Apparently, step 13.8.3.4 occurs for expansion of the index (i.e. the index key, see 13.8.3) before the active property becomes, in my example, typeDescribed. The current context is still the parent context, as is evident in step 13.5:

Initialize container mapping to key's container mapping in active context.

The key here is, with the above example, set to typeDescribed, and it is not, here, used as the active property. Thus the active context under which the index is expanded as an IRI is not the property scoped context. It is true that step 3 and 8 has been invoked by this time, but via step 13.8.3.6, within the recursive call, i.e. within its own scope. So for the index value the active context is determined from typeDescribed, and the local context becomes active. But step 13.8.3.7.4 is not invoked there, since we're processing the index value. Once the recursive call exits, the algoritm proceeds with 13.8.3.7, so when we reach 13.8.3.7.4 to expand the index, as specified, the active context is not the property scoped one.

I believe this is why your (and now also my) implementation uses a container context (which is a "sibling" to the container mapping of the key term definition in the active context). It appears to be an optimization, but it also introduces and uses this context, determined from the key, which is not spelled out to be used in the algorithm.

gkellogg commented 1 year ago

Yes, my analysis started at the wrong point:

You're expecting that 13.8.3.4 would set expanded index using the property-scoped context which would result in "https://example.org/ns/Item" instead of "https://example.org/ns#Item". Indeed, my code creates container context at about 13.8.2 using the following Ruby code:

# While processing index keys, if container includes @type, clear type-scoped term definitions
container_context = if container.include?('@type') && context.previous_context
  context.previous_context
elsif container.include?('@id') && context.term_definitions[key]
  id_context = context.term_definitions[key].context if context.term_definitions[key]
  if id_context.nil?
    context
  else
    log_debug("expand", depth: log_depth.to_i) {"id_context: #{id_context.inspect}"}
    context.parse(id_context, base: @options[:base], propagate: false)
  end
else
  context
end

At this point, it seems odd that I'd have such relatively complex code without something corresponding in the algorithm text. Definitely seems a substantive Erratum is in order.