Skip to content
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

Missing "type" declarations #5

Open
i8beef opened this issue Jan 4, 2021 · 7 comments
Open

Missing "type" declarations #5

i8beef opened this issue Jan 4, 2021 · 7 comments

Comments

@i8beef
Copy link

i8beef commented Jan 4, 2021

Something I noticed in a few places, and I'm not sure if this is valid JsonSchema or not...

  1. In several traits with multiple forms (OneOf, AnyOf, etc.) the ROOT schema is missing any type declaration ("object" I would expect)... is that on purpose?
  2. On some of the multi-format values that discriminate by enum value (See SensorState), seem to specify their ENUM values without specifying a type for properties... is this valid? I know the property definition carries the type in that instance, but my understanding was an enum node needed to specify a type still...
@proppy
Copy link
Collaborator

proppy commented Jan 12, 2021

  1. We usually nest the actual type inside the oneOf to make it easier to read the sub-schema as an individual unit.
  2. Looking at https://tools.ietf.org/html/draft-handrews-json-schema-validation-01#section-6.1.1 it doesn't look like type nor enum but I agree it could make thing more readable (an enum without a type couldn't be taken as an hint that value of multiple types could be accepted).

@i8beef
Copy link
Author

i8beef commented Jan 12, 2021

Thank you, Im unfamiliar with JSONSchema so I'm not sure what the rules are.

I was looking at the possibility of on the fly transforming your schema validations into a validation layer to get some of the business rules represented therein for free. I was hoping for a shortcut by inheriting your business rules, but I think I'm going to have to consider some other options.

Thanks again for this repo, its a huge help.

@i8beef i8beef closed this as completed Jan 12, 2021
@proppy
Copy link
Collaborator

proppy commented Jan 12, 2021

@i8beef depending on the language you're using there might be validation options already available: https://json-schema.org/implementations.html#validators

@i8beef
Copy link
Author

i8beef commented Jan 12, 2021

Im not validating Google Home messages, I'm validating app config structures that are needed for my bridging app. I tried to keep said config as close to the object structures you use, where each device has attributes, commands and states. The attributes are 1:1 so I load your schema into a validator directly and it works great. I was hoping to do the same for commands and states by transforming your schema into one that would validate my config structures, but I've found that doesn't work that great. oneOf, anyOf, enums, "missing" type declarations (that I can take from other places if I do this differently) kind of shot that in the foot.

Not your issue, I was just looking for a short cut. It'll be better if I parse your schema, and then recursively loop through it to generate a new JSON file, and parse THAT instead for my validations, where I can take an additive approach instead of a subtractive one.

Thanks again!

@proppy
Copy link
Collaborator

proppy commented Jan 12, 2021

@i8beef for applying transformation on JSON schemas you might want to take a look at https://cuelang.org as it provides a nice declarative way to work with schema and data.

I'd be happy to add missing "type" declaration for enum (2) if it makes things easier to consume by downstream tool, I'm just not sure if it wouldn't make things more confusing for top-level anyOf/oneOf (1).

@proppy proppy reopened this Jan 12, 2021
@i8beef
Copy link
Author

i8beef commented Jan 12, 2021

Honestly I'd prefer you just follow whatever the JSONSchema best practices are here. My questions were more just about triggering a review of these if there was something missed, or if it was intentional. What I was trying to do was questionable, and its not surprising I hit issues. You shouldn't pollute your schemas for me here unless there's a legitimate deviation from what the JSON Schema specs say you should do.

Edit: https://json-schema.org/understanding-json-schema/reference/combining.html

They seem to point to your enum usage being valid "factoring out" of common elements (type), so I don't see a big reason to modify.

@proppy
Copy link
Collaborator

proppy commented Jan 12, 2021

We currently only enforce that all sub-schemas with properties also have type defined, but we should probably also apply this logic to top-level schemas and individual non-enum properties.

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

No branches or pull requests

2 participants