Skip to content

Sealed contexts (plus restrict context nullification to term definitions) #289

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

Merged
merged 10 commits into from
Apr 17, 2019

Conversation

cwebber
Copy link
Contributor

@cwebber cwebber commented Feb 19, 2019

Feature works, and also adds restriction of context nullification to term definitions. I'm guessing @dlongley will be explaining why this is needed for the extension story to make sense on a call or something soon so I won't bother here.

(Note that I haven't bothered to do tests yet. I will soon but for the moment I'll just paste some test examples into a comment below.)

@cwebber
Copy link
Contributor Author

cwebber commented Feb 19, 2019

Oh yeah and this also BREAKS a test, because, well, it should until that test is removed (since we're adding a new restriction on what's permitted for context nullification).

EDIT: My understanding was wrong, but @dlongley and I discussed it below and it should be handled right now.

@davidlehn
Copy link
Member

Add @sealed to context.js isKeyword(). (I would have thought that was needed to pass other tests, but I guess not?)

@cwebber
Copy link
Contributor Author

cwebber commented Feb 19, 2019

@davidlehn okay I added that one

@cwebber
Copy link
Contributor Author

cwebber commented Feb 19, 2019

I'm trying to figure out what @dlongley wants me to do about these type context nullification rules but my brain is struggling to catch up. In the meanwhile, how about I paste the manual tests I've been using of what should and shouldn't be working/allowed:

var jsonld = require('./lib/jsonld.js');
var contextLib = require('./lib/context.js');

function printResults(err, expanded) {
    if (err) {
        console.log('error:\n'); console.log(err);
    } else {
        console.log(JSON.stringify(expanded, null, 2));
    }}


// ok
jsonld.expand(
    {'@context': {'foo': {'@id': 'https://example.org/#foo',
                          '@sealed': true},
                  '@version': 1.1,
                  'bar': 'https://example.org/#bar'},
     'foo': 'bar',
     'bar': {'foo': 'baz'}},
    printResults)

// error
jsonld.expand(
    {'@context': {'foo': {'@id': 'https://example.org/#foo',
                          '@sealed': true},
                  '@version': 1.1,
                  'bar': 'https://example.org/#bar'},
     'foo': 'bar',
     'bar': {'@context': {'foo': 'https://schema.org/#oof'},
             'foo': 'baz'}},
    printResults)

// error
jsonld.expand(
    {'@context': {'foo': {'@id': 'https://example.org/#foo'},
                  '@version': 1.1,
                  '@sealed': true,
                  'bar': 'https://example.org/#bar'},
     'foo': 'bar',
     'bar': {'@context': {'foo': 'https://schema.org/#oof'},
             'foo': 'baz'}},
    printResults)

// ok
jsonld.expand(
    {'@context': {'foo': {'@id': 'https://example.org/#foo',
                          '@sealed': false},
                  '@version': 1.1,
                  '@sealed': true,
                  'bar': 'https://example.org/#bar'},
     'foo': 'bar',
     'bar': {'@context': {'foo': 'https://schema.org/#oof'},
             'foo': 'baz'}},
    printResults)

// ok
jsonld.expand(
    {'@context': {'foo': {'@id': 'https://example.org/#foo'},
                  '@version': 1.1,
                  '@sealed': true,
                  'bar': {'@id': 'https://example.org/#bar',
                          '@context': [null]}},
     'foo': 'bar',
     'bar': {'@context': {'foo': 'https://schema.org/#oof'},
             'foo': 'baz'}},
    printResults)

@cwebber
Copy link
Contributor Author

cwebber commented Feb 19, 2019

Two more things that are not ok

// not ok
jsonld.expand(
    {'@context': {'foo': {'@id': 'https://example.org/#foo'},
                  '@version': 1.1,
                  '@sealed': true,
                  'bar': 'https://example.org/#bar',
                  'Baz': {'@id': 'https://example.org/#Baz',
                          '@context': [null]}},
     'foo': 'bar',
     'bar': {'@type': 'Baz',
             '@context': {'foo': 'https://schema.org/#oof'},
             'foo': 'baz'}},
    printResults)

// not ok
jsonld.expand(
    {'@context': {'foo': {'@id': 'https://example.org/#foo'},
                  '@version': 1.1,
                  '@sealed': true,
                  'bar': 'https://example.org/#bar',
                  'Baz': {'@id': 'https://example.org/#Baz',
                          '@context': [null,
                                       {'foo': 'https://example.org/#oof'}]}},
     '@type': 'Baz',
     'foo': 'baz'},
    printResults)

@davidlehn
Copy link
Member

@cwebber: Feel free to add local mocha js tests. Can add more test files in tests/test.js, and look at misc.js or others as examples. Or just add to misc.js. Those are independent files too so easy to run.
Sometimes easier to test new features that way before tests are moved to the main test suite.

@dlongley
Copy link
Member

Yay! Looks like this is doing the right thing now and it was even easier than I thought. I suspect we'll need to add the "propertyTermDefinition" flag to the active context to record its "origin" so that the cache will work properly -- and add some tests that toggle back and forth using the same context to see that it works next.

Looking great, almost entirely done with this feature. I'll need to talk with @gkellogg about it soon.

@dlongley
Copy link
Member

Do we need to set propertyTermDefinition: true here:

jsonld.js/lib/compact.js

Lines 102 to 106 in 1de987c

// use any scoped context on activeProperty
const ctx = _getContextValue(activeCtx, activeProperty, '@context');
if(!_isUndefined(ctx)) {
activeCtx = _processContext({activeCtx, localCtx: ctx, options});
}

And add some compaction tests for it?

cwebber and others added 10 commits April 17, 2019 11:09
- Use `undefined` vs `null` for getContextValue(ctx, key, '@context').
- Improve `@protected` support.
- Allow options to be passed through various functions.
- Add `protectedMode` support.
  - Temporarily just logging to console in warn mode.
- Allow redefinition via property term scoped contexts.
- Rename property term scoped context flag and use in `options`.
- Pass property term scoped context flag in compaction algorithm.
- Fix context caching/processing for arrays of contexts.
@dlongley dlongley merged commit 82791e0 into master Apr 17, 2019
@dlongley dlongley deleted the sealed-contexts branch April 17, 2019 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants