CARVIEW |
Navigation Menu
-
Notifications
You must be signed in to change notification settings - Fork 66
fix: align JSON Schema with JSON-LD @context format #2080
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
Conversation
TD Call 20 Feb:
|
"contains": { | ||
"$ref": "#/definitions/thing-context-td-uri-v1.1" | ||
"allOf": [ | ||
{ | ||
"$ref": "#/definitions/thing-context-td-uri-v1.1" | ||
}, | ||
{ | ||
"$ref": "#/definitions/thing-context-td-uri-v1" | ||
} | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@egekorkan So, without this change, an invalid context definition is actually not being properly detected for some reason, see here: https://www.jsonschemavalidator.net/s/qp7EUNgs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This also includes contexts that only use the new TD context URL: https://www.jsonschemavalidator.net/s/hfTQX44x
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there is still a problem. The first entry can be any url if the second one is https://www.w3.org/2022/wot/td/v1.1
I don't think we want that to happen...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the updated schema from this PR, that is actually being picked up by the validator: https://www.jsonschemavalidator.net/s/C6X5uTuI
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right. I used the wrong schema. I think I found another problem, but it gets complex and might be difficult to fix. One can flip the context URLs and it is still valid.
https://www.jsonschemavalidator.net/s/h7KdeCHp
I am fine if the schema does not catch this issue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, that is a good point,I wasn't sure if that is actually a bug or if a version 1.1 TD could also just add the 1.0 URL to its context 🤔 Maybe the assertion could even be clarified by an erratum, but then again, I am also not sure if the problem is that severe.
The changes relating to the restriction of non-string values in |
By the way, https://json-schema.hyperjump.io/ can be used to test multiple json values per schemas. I am working on another branch in the meantime to be able to have a record of all this |
Ok this took longer than I expected/wanted :D Here is my summary:
|
TD Call:
|
eclipse-thingweb/playground#626 is working as expected with all new schemas and no previous tests failing. Once we merge this PR, I will also add more tests there to be able to see failures. |
Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
WG Resolution to merge this: https://www.w3.org/2025/03/26-wot-minutes.html#d30b |
The changes will be reflected at https://github.com/w3c/wot-resources/tree/main/td/v1.1 |
This PR fixes #2022 by specifying an explicit type of
string
for theadditionalProperties
in a TD's@context
definition.While resolving the issue, I noticed a small bug in the schema related to
@context
s that contain both the old and the new TD context URL. To resolve it, I adjusted thecontains
definition that is present in the corresponding "subschema".With the changes from this PR, the following invalid TDs are now rejected by a JSON Schema validator, as the specification intends it.
TD with new @context URL only
TD with old @context URL