Skip to content

Additional restriction to @sealed term clearing #136

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

Closed
dlongley opened this issue Feb 21, 2019 · 13 comments
Closed

Additional restriction to @sealed term clearing #136

dlongley opened this issue Feb 21, 2019 · 13 comments

Comments

@dlongley
Copy link
Contributor

dlongley commented Feb 21, 2019

We've completed a preliminary implementation of @sealed terms here:

digitalbazaar/jsonld.js#289

Our implementation:

  • Allows terms to be sealed using "@sealed": true in a term definition (or not sealed with "@sealed": false)
  • Allows setting the default "@sealed" for all terms in an @context via "@sealed": true at the top level
  • Does not affect existing term clearing rules when there are no @sealed terms
  • Allows clearing @sealed terms ONLY via a new property term definition with a scoped @context (note: and the new term does not need to be sealed)

Note that the last item is a slight tweak (additional restriction) on what is in the spec. We believe this approach better matches the use cases and expectations of the target audience for JSON-only application developers. The high-level idea is that a @sealed term is assured to maintain its meaning unless cleared by a new term (with a scoped context) that creates a new branch in the JSON tree.

This approach:

  • Provides sufficient assurance for the meaning of @sealed terms without a need for JSON-LD processing: a JSON-only dev can read a spec and write safe code that traverses JSON-LD data via @sealed terms ... without a need for JSON-LD processing, instead using only validation rules (e.g. json-schema)
  • Allows extensions to the JSON tree to occur safely in an idiomatic way where @sealed terms can only be cleared under new branches in the JSON tree (matches common JSON developer intuitions/expectations)
  • Very closely mirrors what @pchampin articulated so well a while back but has the added benefit of being simpler to implement: there is no need to track the @context from whence @sealed terms came

We'd like to see the spec updated to restrict clearing @sealed terms in the way articulated here and implemented in the above link. This approach seems to fully address the original need for the feature and has a simple implementation.

@gkellogg
Copy link
Member

This looks like a middle ground between keeping track of sealed terms from different contexts and providing some impediment to simply wiping out sealed terms, which only takes effect when you've stepped into a different part of the JSON tree.

@iherman
Copy link
Member

iherman commented Feb 22, 2019

@dlongley, thanks. Just for my understanding, can you propose alternate versions of the examples 42-44? I am. e.g., not sure what you mean by "clearing".

Looking, e.g., in Example 42: does it mean that the second attempt to overwrite a term (namely as part of "knows") is acceptable in contrast to what the example say now? Or is this action permitted only for a term different from "knows", i.e., a "new" term?

My feeling is that you mean the second option. But I am not sure…

@dlongley
Copy link
Contributor Author

dlongley commented Feb 22, 2019

@iherman,

I am. e.g., not sure what you mean by "clearing".

I mean that the active context is reset to the initial active context (a "clean slate" having no term definitions). If there are sealed terms in the active context when an attempt is made to reset it, this will fail unless the attempt that is made is via a term definition with a scoped context and that term is used as a property (as opposed to as a value for @type). More explanation to follow...

does it mean that the second attempt to overwrite a term (namely as part of "knows") is acceptable in contrast to what the example say now? Or is this action permitted only for a term different from "knows", i.e., a "new" term?

Actually, the current examples in the spec don't change at all; they are still correct with this new restriction. So, no, it is still not acceptable and, yes, I mean "the second option" as you suspected. You must create a new term just like Example 44 does. However, this makes it more clear that the only acceptable way is the way Example 44 does it.

For instance, an example of something that would not be acceptable is this:

{
  "@context": [
    {
      "@version": 1.1,
      "@sealed": true,
      "Organization": "http://schema.org/Organization",
      "name": "http://schema.org/name",
      "employee": "http://schema.org/employee"
    }
  ],
  "@type": "Organization",
  "name": "Digital Bazaar",
  "employee" : {
    "@context": [
       // because of "@context": null in an embedded context 
       // and not a scoped context, this `null` will fail and so
       // would the override attempt of name
      null,
      {
        "name": "this_attempt_to_override_name_will_fail"
      }
    ],
    "name": "Manu Sporny"
  }
}

This above example would continue to fail even if employee were not sealed or if a new term were introduced that did not use a scoped context:

{
  "@context": [
    {
      "@version": 1.1,
      "@sealed": true,
      "Organization": "http://schema.org/Organization",
      "name": "http://schema.org/name",
      "employee": "http://schema.org/employee"
    },
    {
      "newButNoScopedContext": "http://example.com/newButNoScopedContext"
    }
  ],
  "@type": "Organization",
  "name": "Digital Bazaar",
  "newButNoScopedContext" : {
    "@context": [
       // because of "@context": null in an embedded context 
       // and not a scoped context, this `null` will fail and so
       // would the override attempt of name
      null,
      {
        "name": "this_attempt_to_override_name_will_fail"
      }
    ],
    "name": "Manu Sporny"
  }
}

Similarly, this would fail:

{
  "@context": [
    {
      "@version": 1.1,
      "@sealed": true,
      "Organization": "http://schema.org/Organization",
      "name": "http://schema.org/name",
      "employee": "http://schema.org/employee",
      "SpecialOrganization": {
        "@id": "http://example.com/SpecialOrganization",
        // if the term for this definition is used as an "@type",
        // then this attempt to clear "@sealed" terms will fail
        "@context": [
          null,
          {
            "name": "this_attempt_to_override_name_will_fail",
            "employee": "this_attempt_to_override_name_will_fail"
          }
        ]
      }
    }
  ],
  // because of "@context": null in a scoped context for
  // a term used as an "@type" rather than as a property, its
  // term definition will fail
  "@type": ["Organization", "SpecialOrganization"],
  "name": "Digital Bazaar",
  "employee" : {
    "name": "Manu Sporny"
  }
}

@iherman
Copy link
Member

iherman commented Feb 22, 2019

@dlongley still trying to understand... If I did the following (sorry for the ugly code, but it is legal...)

{
  "@context": [
    {
      "@version": 1.1,
      "@sealed": true,
      "Organization": "http://schema.org/Organization",
      "name": "http://schema.org/name",
      "employee": "http://schema.org/employee",
      "SpecialOrganization": {
        "@id": "http://example.com/SpecialOrganization",
        // if the term for this definition is used as an "@type",
        // then this attempt to clear "@sealed" terms will fail
        "@context": [
          null,
          {
            "name": "this_attempt_to_override_name_will_fail",
            "employee": "this_attempt_to_override_name_will_fail"
          }
        ]
      }
    }
  ],
  // because of "@context": null in a scoped context for
  // a term used as an "@type" rather than as a property, its
  // term definition will fail
  "@type": ["Organization", "SpecialOrganization"],
  "name": "Digital Bazaar",
  "employee" : {
    "name": "Manu Sporny"
  },
  "SpecialOrganization" : {
      "name" : "somebody"
  }
}

then is it so that the second occurence of "SpecialOrganization" will not fail because I used "SpecialOrganization" as a term this time?

@pchampin
Copy link
Contributor

I like this new proposal.

If we are to accept it, I would update the spec text, because the paragraph before Example 44 would not be correct anymore. It would have to make it clear why Example 44 works, and in which cases "@context": null would fail...

@dlongley
Copy link
Contributor Author

@iherman,

then is it so that the second occurence of "SpecialOrganization" will not fail because I used "SpecialOrganization" as a term this time?

Yes, it would be legal there. Of course, since SpecialOrganization still appears in @type in your example, a JSON-LD processor configured to throw errors would have already thrown an error for that occurrence.

@iherman
Copy link
Member

iherman commented Feb 22, 2019

Would that be an error, like stop processing, or a warning, like ignoring the changes?

Anyway, that is another matter, I think I understand what you are getting at. (Until I am lost again, that is:-) I agree with @pchampin that the text in the spec should be updated asap, to see if the full @seal @protected feature can be explained clearly...

@dlongley
Copy link
Contributor Author

Would that be an error, like stop processing, or a warning, like ignoring the changes?

Anyway, that is another matter, I think I understand what you are getting at.

I agree that's another issue. I suspect we might want processors to support two modes: a default mode that raises an error (stops processing) and an optional mode to instead issue a warning that the attempted changes were invalid and thus ignored.

@azaroth42
Copy link
Contributor

I disagree that the definition provided matches the examples. You can have scoped contexts based on @type, so the SpecialOrganization example should NOT fail. Otherwise you're introducing even more special cases by allowing property scoped contexts to override @protected but type scoped contexts cannot.

This needs more work.

@dlongley
Copy link
Contributor Author

dlongley commented Mar 1, 2019

You can have scoped contexts based on @type, so the SpecialOrganization example should NOT fail.

No, this is not permitted as it would break the protection rules.

Otherwise you're introducing even more special cases by allowing property scoped contexts to override @protected but type scoped contexts cannot.

I disagree. There are not several special cases. There is only one way to override @protected and it is through property term scoped contexts.

This is chosen because it is the only safe way to do it -- and it matches expectations and intuitions for what "protection" means. The reason @protection is added to a term is to enable users to safely consume JSON-LD as regular JSON (i.e. idiomatically and without having to use JSON-LD processing).

Since JSON developers rely on the structure of the JSON tree to understand meaning, it makes sense that adding and removing @protection depends on this. If @protection can be removed, it must only be done in a way that would not break this understanding. Therefore, protection can be safely removed by introducing a new section of the JSON tree via a scoped @context -- no other way. This approach mirrors how idiomatic JSON extensions are done: by extending the JSON tree.

@azaroth42
Copy link
Contributor

Since JSON developers rely on the structure of the JSON tree to understand meaning,

Not necessarily. A perfectly comprehensible JSON pattern would be to instantiate the JSON objects as objects, and then just understand the meaning based on the classes. This would imply that an @type based pattern is just as useful as a predicate based pattern.

The special cases are:

  • @context: null doesn't work as one might expect
  • scoped contexts by @type don't work as one might expect, and would cause errors when the class is referred to by a predicate scoped context (I believe)

@dlongley
Copy link
Contributor Author

dlongley commented Apr 9, 2019

I think an @type based pattern is fine, however, when there are conflicts with existing @protected terms, processing will fail. You cannot mix types with existing meaning, just like you'd have trouble doing multiple inheritance (or it is prohibited in many languages). So you can't erase existing @protected terms using @type scoped term definitions.

@iherman
Copy link
Member

iherman commented Apr 15, 2019

This issue was discussed in a meeting.

  • RESOLVED: Allow @protected terms to be changed only via property scoped contexts, and not via setting the active context to null
  • RESOLVED: Any attempt to change or clear a @protected term results in an error being raised, other than as provided for already
View the transcript Additional restriction to @Sealed term clearing
Ivan Herman: link: #136
Benjamin Young: Protected Term Definition - https://w3c.github.io/json-ld-syntax/#protected-term-definitions
Benjamin Young: we’re discussing 136. There are some already-merged PRs and there are some tests
… this topic is our exclusive focus for today
… because this would really help VCWG
… this piece of work is valuable to support them NOT taking JSON-LD out of the spec
… let’s start with a summary of the editorial work
Gregg Kellogg: I think there are some PRs that have been completed on the syntax side
Pierre-Antoine Champin: all related PRs have been merged
Gregg Kellogg: I think we still need to add the mechanism that would disallow unsealing via @context:null
Dave Longley: we’re just looking exactly what gkellogg said
… what’s currently there would allow for the protection to be removed merely via @context:null anywhere in the doc
… which doesn’t do what we actually need from the feature
… the point is to be able to call out terms so that people using the context can be minimal with processing on those terms
… with but one little change, we can accomplish that
… just by restricting to a scoped context
… that will provide the needed assurance
Pierre-Antoine Champin: to be clear, the @context:null could be on any protected definition?
Dave Longley: correct
… should not complicate implementations
Rob Sanderson: if a @context:null is encountered elsewhere than in a property-scoped context, it changes from wiping out everything to just wiping out non-protected terms?
Dave Longley: no, that should be an error under our change
… because that would be trying to remove protection illegally
Rob Sanderson: that seems backwards-incompatible
… it changes the meaning of @context:null
Dave Longley: well, no, because there were never protected terms before
Rob Sanderson: 2nd opinion would be desirable
Gregg Kellogg: what we had done before was not throw errors but issue warnings
David I. Lehn: protectedMode and tests here: w3c/json-ld-api#69
Gregg Kellogg: dlehn has an issue for a “protected mode” flag which would switch between errors and warnings
… are you putting those together or something totally new?
Dave Longley: we’ve implemented both, and we’re okay with either, but we think that throwing an error would be cleaner, but we can default to the warning
… and in that case, the protected terms would just not be redefined
… we have tests for either case
… but we thought that defaulting to error would be cleaner
Pierre-Antoine Champin: just wanted to point out that property-scoped context aren’t in 1.0
@context:null in a property-scoped definitions simply doesn’t exist now,
… so we can’t break extant JSON-LD
… [then changes his mind]
Gregg Kellogg: the only way to provoke this is via a protected term, which didn’t exist in 1.0
… so you couldn’t produce this error that way
… I think that adding flags to the API is waffling
… we should choose a party to go to
… in my own processor I run testing in a “validation” mode which is strict
… this might be such a thing
… but for conformance, maybe we should just pick a road here
Rob Sanderson: Here’s when it breaks 1.0: { @context: [ "1.1-context-with-protected", "existing-´1.0-context-with-null"] ... }
Rob Sanderson: if the intent is that extant 1.0 contexts work as written
… then adding “protected” that can’t be reset via @context:null
… won’t work if a 1.0 context began with @context:null but is used at a point where protected terms are in scope
Gregg Kellogg: it used to be that we run in 1.0 until we see a marker for 1.1, but we now start in 1.1
Benjamin Young: current processing model triggering definition https://w3c.github.io/json-ld-syntax/#dfn-processing-mode
Rob Sanderson: so 1.0 contexts still work as expected and danbri won’t come gunning for us?
… or not?
… are we going to get objections from Google or MS ?
… if we can put this into the set of allowable incompatibilities
… I won’t stand in the way
Ivan Herman: to prepare for a possible objection, is it worth to add a note to the document making that clear?
Rob Sanderson: +1 from me to calling it out specifically
Gregg Kellogg: to say that this is a 1.1 feature?
Ivan Herman: question may come up, and we can preempt them in this case
… either in the spec or the best practices doc
Benjamin Young: if it risks a formal objection we should put it in the docs
Gregg Kellogg: it’s not necessary to qualify generating an error if the attempt to clear out the context with a protected-mode term in scope for 1.1.
Adam Soroka: [and I lost the end of the thought]
Gregg Kellogg: that might be a point at which we could add a note
… that this error cannot be generated from a proc running in 1.0 mode
David Newbury: when we’re talking about 1.0 v 1.1 mode for compatibility
Adam Soroka: .. we’re talking about the doc being processed
David Newbury: not the context of the context
Gregg Kellogg: documents aren’t 1.0 or 1.1
… the processing is 1.0 or 1.1
… contriolled by @Version in the context
Benjamin Young: the @Version definition https://w3c.github.io/json-ld-syntax/#h-note-1
Gregg Kellogg: but that can happen in any context
David Newbury: right, but that doesn’t change the context, right, just its interpretation in the context (different use of the word) in which it is being processed?
Rob Sanderson: I understand the pattern of restricting to property-scoped contexts
… but is that the limit?
Dave Longley: it’s acceptable to put protected type-scoped definition
… but you can’t clear terms there
… that would lead to multiple inheritance
… think of protected terms as a “base class” for anything in the doc
… appending a type to any node would change the semantics
… which misses the whole point, which is to make processing easier
Rob Sanderson: we can’t limit the types assigned to a resource
… and type-scoped defns in this way could lead to collisions.
Gregg Kellogg: I implied something like this in an earlier version
… what would happen is
… when you expand based on a property which is a protected term
… that is when you look for embedded context associated with that term
… . that is when you could see @contxt:null
… you detect that by calling expansion with some option that includes the term at hand
Dave Longley: we ran into no gotchas
… we did it with both warning and error
… it was simple, the hardest part was the warning
… tracking when you can clear protection or not was actually fairly simple
Gregg Kellogg: ready with a Pull Request?
Dave Longley: not quite, but quite close
Benjamin Young: w3c/json-ld-api#69
Benjamin Young: Add @Protected tests and a protectedMode flag.
Dave Longley: yeah, that looks like the right PR
David I. Lehn: the related is implementation PR is here: digitalbazaar/jsonld.js#289
Pierre-Antoine Champin: I can edit the current section on “protected” in the syntax document, to reflect those changes
Dave Longley: that PR should cover all the bases
… clearing things when you should be bale to, when you shouldn’t…
Gregg Kellogg: some updates look to be needed
… some test are failing and it’s coming from Digital Bazaar’s repo
Dave Longley: we can move the PR, np
Benjamin Young: this PR has the protected mode warning/error flag on the API
… we can massage that independent of the PR
Ivan Herman: firstly we should formally resolve the issue in a very clear way
… but also we need a statement or something from this WG making it clear that this is a feature that is frozen and will not be removed
Dave Longley: yes, we need that reassurance
Ivan Herman: we have to produce that
… a similar situation (JSON-LD being behind) can happen with WoT WG as well
… they will need the same kind of things
… we could produce a blog
… saying that all features in the document are stable in the sense that
… we don’t expect to remove them
… would that work for you?
Dave Longley: my understanding is that it would
Gregg Kellogg: when we talked about freezing before, I said then that we need to get out a WD
… I don’t think we can point at an editor’s draft for such an assurance
… so what’s the timing for that?
Ivan Herman: for a CR, for which you are running right now
… having a fresh WD from JSON-LD WG should be enough
… but getting to Rec would mean that isn’t enough
… we have a publishing process through which to go, and it could take 2-3 weeks
Dave Longley: We’re already in CR
… 2-3 weeks doesn’t change anything
… we prefer sooner rather than later
… this will help us get through a number of issues
Ivan Herman: and helps you with the discussion you are having with commenters
Rob Sanderson: let’s say we have a WD within 1-2 weeks
… and get the feedback
… then write the blog post saying “No more new features”
… would be 3-4 weeks
Dave Longley: yes, but TBC all we need is “these features won’t be taken out”, not “no new features”
Rob Sanderson: if we do hit timeline issues, we could offer something more than a blog post for VCWG in particular
Dave Longley: a simple statement would be acceptable
Gregg Kellogg: we do need to resolve “warnings vs. errors”
Rob Sanderson: we need to discuss that beforehand
Dave Longley: I don’t want to prevent people from writing nonstandard implementations that use warnings,
… I was concerned by not having this behavior be configurable
Proposed resolution: attempts to override @Protected terms will throw an error during processing in 1.1 mode (Benjamin Young)
Adam Soroka: +1
Adam Soroka: me oh, good point
Adam Soroka: [general discussion about how to advance the wording of this propsoal]
Dave Longley: don’t want to confuse people with wording about when you can use @context:null
Gregg Kellogg: also, terms aren’t really changed by that action, they are cleared
@context:null doesn’t change terms, it clears them
Rob Sanderson: I thought you didn’t need to set @context:null to alter a term
Gregg Kellogg: what if you set a term defintion to null?
Rob Sanderson: we can put that off until later
Rob Sanderson: {"@context": {"protected": {"@id": "something", "@Protected": True}, "extension": {"@context": {"protected": {"@id": "something else"}}}
Rob Sanderson: I believe that given our current definitions, that ^^^ would be acceptable
… you should be able to set a term directly to something else
Dave Longley: as long as you are property-scoped, you can wipe out everything, but yes, you could go at only a single term
Gregg Kellogg: and 1.0 does allow setting a single term to null
Proposed resolution: Allow @Protected terms to be changed only via property scoped contexts, and not via setting the active context to null (Benjamin Young)
Dave Longley: +1 (of course, setting the context to null within a property scoped context is also allowed)
Gregg Kellogg: +1
Rob Sanderson: +1
Rob Sanderson: we could make that a bit clearer as pointed out by dlongely
Jeff Mixter: +1
Benjamin Young: +1
David Newbury: +1
Ivan Herman: +1
Pierre-Antoine Champin: +1
Tim Cole: +1
Ruben Taelman: +1
David I. Lehn: +1
Adam Soroka: +1
Resolution #2: Allow @Protected terms to be changed only via property scoped contexts, and not via setting the active context to null
Adam Soroka: the proposal, just the text; I am on board with the idea
Gregg Kellogg: other than as previously provided
Dave Longley: +1 to Gregg
Dave Longley: +1 to the proposal
Benjamin Young: what we’re saying is that it is indeed an error, not a warning
Ivan Herman: +1
Proposed resolution: Any attempt to change or clear a @Protected term results in an error being raised, other than as provided for already (Rob Sanderson)
David Newbury: +1
Gregg Kellogg: +1
Jeff Mixter: +1
Rob Sanderson: +1
Adam Soroka: +1
Dave Longley: +1
Ivan Herman: +1
Tim Cole: +1
Pierre-Antoine Champin: +1
Ruben Taelman: +1
Benjamin Young: +1
David I. Lehn: +1
Resolution #3: Any attempt to change or clear a @Protected term results in an error being raised, other than as provided for already
Gregg Kellogg: for the purpose of best time use we should look at issues on the GitHub management console to find what we need to do to get to a new WD
Ivan Herman: different topic?
Benjamin Young: yeah, I have no other issues

pchampin added a commit that referenced this issue Apr 15, 2019
NB: marked example 44 as ignored, because it is not supported by the current implementation
gkellogg pushed a commit that referenced this issue Apr 15, 2019
NB: marked example 44 as ignored, because it is not supported by the current implementation
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

5 participants