pboettch / json-schema-validator

JSON schema validator for JSON for Modern C++
Other
466 stars 134 forks source link

📚️ raise error when trying to change a read only value #213

Closed opsocket closed 1 year ago

opsocket commented 1 year ago

Relates to #212

This pull request honors "readOnly" keyword from section 10.03 from the specification by raising an error 🐢

pboettch commented 1 year ago

Are you sure that this is how read-only shall work? If the value is not equal to the default value (if any) it shall fail?

Isn't read-only only for run-time usage of an schema-validated instance? So out-of-scope of this library?

pboettch commented 1 year ago

In any case, we need a test before integrating changes. I think in this case the tests will also show the limitations of this approach.

opsocket commented 1 year ago

Since this library allows defaults parsing (which is awesome), it should reflect the referenced section of the specification in the process

pboettch commented 1 year ago

readOnly has nothing to do with default-values. I don't understand. I read the section and I strongly think that readOnly is something to be ignore by the validator. I could be exposed as the validator compiles the schema but the validation-process itself should ignore it.

It has to be handled by the owning authority.

opsocket commented 1 year ago

Here's how i understand the section:

If "readOnly" has a value of boolean true, it indicates that the value of the instance is managed exclusively by the owning authority

The owning authority is the validator since it owns the instances that make up the schema tree.

and attempts by an application to modify the value of this property are expected to be ignored or rejected by that owning authority

It is expected that attempts to change the value of this key, in the patch resulting from the validation process at runtime, will be ignored or rejected.

opsocket commented 1 year ago

Do you think it should also reject attempts when the key has no default ?

pboettch commented 1 year ago

The owning authority is the validator since it owns the instances that make up the schema tree.

False, the schema validator gets the to-be-validated instance as a const. How can it own something which can't be modified? The whole instance is read-only from the validator's point of view.

If you're still in doubt about that, please check with the json-schema-project to clarify. https://github.com/json-schema-org/community/discussions

However, what the validator could do is to provided a list of readOnly JSON-paths which you can used in your application to check when the user changes something in the instance.

pboettch commented 1 year ago

Closing for the moment. Please re-open once you have a feedback showing that I'm wrong.

opsocket commented 1 year ago

Let's begin the journey to "Why are you wrong?"

the schema validator gets the to-be-validated instance as a const.

This is absolutely true, we both agree! The user-defined input should stay read only 😉

How can it own something which can't be modified? The whole instance is read-only from the validator's point of view.

There is obviously a misunderstanding here since we agree again, so I will do my very best to clarify.

Users are writing applications including the library.

  1. They initialize a validator with a root schema that they crafted themselves.
  2. They use this validator to validate a user-defined instance against the root schema embedded by the validator.
  3. They merge a copy of the default instance of the root schema with the user-defined instance if it passes validation.

Questions:

With all this, if you still don't see why this is the way to go, I can't do anything more for this library

If you're still in doubt about that, please check with the json-schema-project to clarify. https://github.com/json-schema-org/community/discussions

I'll leave it as is, hopefully if your opinion evolves or if you find tests that fail, tag me 🍻

However, what the validator could do is to provided a list of readOnly JSON-paths which you can used in your application to check when the user changes something in the instance.

I thought about a vector of json pointer, it's an attractive idea too but it adds an unnecessary burden to the user from my point of view