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

FeatureRequest: WARN log on additional property #828

Open
manuelwaltschek opened this issue Jun 20, 2023 · 4 comments
Open

FeatureRequest: WARN log on additional property #828

manuelwaltschek opened this issue Jun 20, 2023 · 4 comments

Comments

@manuelwaltschek
Copy link

manuelwaltschek commented Jun 20, 2023

Is it possible to add warnings when a property is not included in the schema?
My use case:
Building a data modeling tool built on json-schema, having strict schemas and also using schema composition extensively.
As seen in This issue unevaluatedProperties validates recursively, but additionalProperties does not. When using "allOf" I have to use unevaluatedProperties, but when it is a self-contained schema, we are using additionalProperties=false. This does not apply to any nested object definitions and logging it would help find any instances where a client might use the wrong property name.

Edit: what do you think about a "strict mode" that is configurable so that it fails on addtionalProperties recursively?

@fdutton
Copy link
Contributor

fdutton commented Jun 20, 2023

@manuelwaltschek The JSON Schema Team introduced unevaluatedProperties specifically for this use-case. Apparently, there was a lot of demand for this recursive behavior but they did not want to redefine the semantics of additionalProperties.

If I understand your need correctly, it seems to me that simply adding required would do what you want.

@manuelwaltschek
Copy link
Author

manuelwaltschek commented Jun 20, 2023

I think it will not quite solve my issue.

For example, this is a snippet of my schema:

"regularHours": {
          "type": "array",
          "items": {
            "type": "object",
            "properties": {
              "weekday": {
                "type": "string",
                "enum": [
                  "MONDAY",
                  "TUESDAY",
                  "WEDNESDAY",
                  "THURSDAY",
                  "FRIDAY",
                  "SATURDAY",
                  "SUNDAY"
                ]
              },
              "periodBegin": {
                "type": "string",
                "pattern": "^([0-1][0-9]|2[0-3]):[0-5][0-9]$"
              },
              "periodEnd": {
                "type": "string",
                "pattern": "^([0-1][0-9]|2[0-3]):[0-5][0-9]$"
              }
            }
          }
        }

When the client sends the field "period_begin" validation does not fail since it does not match the property name, I would have to add "additionalProperties=false" to the subschema, but this is inconvenient in my opinion, when there are a lot of sub-schemas, I found it to be error prone, when you just forget it ;)

@kool79
Copy link

kool79 commented Oct 11, 2023

@manuelwaltschek this library just implement a standard. The standard required to define "additionalProperties=false" on every level, any inheritance is absend here. The WARN log which you propose maybe useful for your edgecase project, but definetly will spam some others. Especially for deep jsons where we have a limited defined set of properties only for topmost level(s)

@manuelwaltschek
Copy link
Author

I agree with you @kool79 but this is why I asked for it to be configurable. I've had this problem multiple times which was also hard to cover with tests, as the schema just accepted anything and clients using these schemas on our endpoints failed to map properties to the right name of the property not getting any errors at all.
We would need to add an additional level of validation, which regards the data and schema. We even added a routine that recursively added additionalProperties / unevaluatedProperties to any sub-schemas but this led to other issues.

So adding additional validation "on-top" of the schema-validation makes me wonder why I even use that in the first place. Maybe it just isn't the right tool for the job.

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

3 participants