Skip to content

Fix JSON literal value version 1 check. #346

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 4 commits into from
Dec 10, 2019

Conversation

dlongley
Copy link
Member

So this fixes a bug where the JSON literal version 1 check was being performed on the unexpanded input instead of on the expanded output. IOW, it was looking for @value on the unexpanded input and then doing some version 1 check on it if @type was not @json. If the input uses an alias for @type (like type) this check will be erroneously triggered and throw an error. Example input:

{
  "@context": {
    "type": "@type"
  },
  "ex:foo": {
    "type": "@json",
    "@value": {test: 1}
  }
}

The conditional should have been checking for @type on the expanded output (expandedParent) instead -- a similar problem would have occurred if @value had been aliased.

However, in fixing this bug we start to fail some test:

#te029 Verifies that an exception is raised in Expansion when an invalid value object value is found

In looking at the input for that test, however, it seems like the test is wrong. The input is:

{
  "@type": true
}

Which is not a value object (there is no @value ... it is a top-level input). So I'm not quite sure what should be done here as I don't know what that test was suppose to test originally.

@davidlehn
Copy link
Member

@dlongley That input you mention is for e028-in.jsonld, not e029-in.jsonld.

@dlongley
Copy link
Member Author

dlongley commented Dec 10, 2019

@dlongley That input you mention is for e028-in.jsonld, not e029-in.jsonld.

Oh, well, something different is amiss I guess.

@davidlehn
Copy link
Member

Not fine, test still fails. The expandedParent change isn't quite right. The @value checks need to be on the unexpanded data. Can fix by reverting the expandedParent change back to element except for that one that accesses @type. (Tested that with the new type and value alias tests.) Is that more correct?

@dlongley
Copy link
Member Author

@davidlehn -- it seems like the check would just get skipped if you looked for @value on the unexpanded data and value alias were used.

Capture unexpanded value for later checks. Handles issue with
determining value when keyword aliases may have been used.
Copy link
Collaborator

@gkellogg gkellogg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems fine, but if you like, I'll take this over when I get on to JSON literals, in general.

Special keywords are handled before this point.
@davidlehn davidlehn force-pushed the fix-json-literal-value-check branch from 0388444 to 350f2f4 Compare December 10, 2019 22:51
@davidlehn davidlehn merged commit d0a64bc into master Dec 10, 2019
@davidlehn davidlehn deleted the fix-json-literal-value-check branch December 10, 2019 23:04
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