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

Don't validate readOnly properties when writing, and writeOnly when reading #104

Open
bertramakers opened this issue Feb 22, 2022 · 4 comments

Comments

@bertramakers
Copy link

Given a schema like:

{
  "type": "object",
  "properties": {
    "readAndWrite": {"type": "string"},
    "readOnly": {"type": "string", "readOnly": true},
    "writeOnly": {"type": "string", "writeOnly": true}
  },
  "required": ["readAndWrite", "readOnly", "writeOnly"]
}

And data like this (for example on an incoming HTTP request, so when "writing"):

{
  "readAndWrite": "...",
  "writeOnly": "..."
}

Opis\JsonSchema\Validator::validate() will return a validation result with an error like:

The required properties (readOnly) are missing

Is it possible to set some kind of "read" or "write" context on the validator, to ignore either writeOnly or readOnly properties in some situations?

We have two reasons to mark some read-only properties as required in our schemas:

  • We do not only use JSON schema to validate incoming data, but we also want to validate the JSON that we return in response bodies, so we can pro-actively find implementation bugs. And some read-only properties must always be included in the response/read context.
  • So integrators can see what properties will always be included in a read context.

Additionally even if we don't make any read-only property required, we also just don't want to validate their type and value if they happen to be included in a write context. Because an API client might fetch a resource, alter some (write-able) properties, and send the data back including the read-only properties which might be stale by the time that they send their data. Or they might always include a read-only property in their request by accident. Either way we ignore those properties in write contexts, so we also want to ignore them in the validation. Which seems to be a valid implementation according to:

If "readOnly" has a value of boolean true, it indicates that the value of the instance is managed exclusively by the owning authority, and attempts by an application to modify the value of this property are expected to be ignored or rejected by that owning authority.

https://json-schema.org/draft/2020-12/json-schema-validation.html#rfc.section.9.4

@sorinsarca
Copy link
Member

From what I understand, you have 2-in-1 schema, and based on some context there should be a "split". Now, the problem is
that you cannot make this split easy. Sure, you can rewrite some keywords (properties, required) and it might just work for simple schema examples,
but as soon as you hit a $ref keyword or even an if keyword the problems start to arise.

The below example shows how the name property can be marked as readOnly depending on the locked property.
In order to "split" you have to evaluate the schema first, but you cannot correctly evaluate the schema because it is a 2-in-1 schema.

{
  "type": "object",
  "properties": {
      "locked": {"type": "boolean"},
      "name": {"type": "string"}
  },
  "required": ["locked"],

  "if": {
      "properties": {
          "locked": {"const": true}
      }
  },
  "then" {
     "properties": {
       "name": {"readOnly": true}
     }
  }
}

readOnly and writeOnly are just some meta keywords, they are useful to generate documentation, or a simple form-builder might use that info to mark a field as read only.

@bertramakers
Copy link
Author

Thanks for the thorough explanation @sorinsarca!

How would you suggest we tackle this issue if it cannot be fixed in the validator?

By using 2 duplicate schemas that are 99% the same but with different required properties?

Or by using 1 schema without any required fields and an allOf schema for reads and an allOf schema for writes with different required arrays? But sadly a lot of tooling do not support allOf that well. I know that's an issue with the tooling but I do see it mentioned in a lot of (Open)API guidelines from companies/organizations that allOf/oneOf/anyOf should be avoided. For example https://github.com/paypal/api-standards/blob/master/http-api-style-guide.md#schema-composition-advanced-syntax

The only solution I see without allOf to have 2 schemas with different required arrays depending on the read/write context then, would be to duplicate every property in 2 schemas (read/write) but with a $ref for each property to keep duplication to a minimum. But we have a lot of properties on the schema so even with a $ref for each property it would still be a lot of duplication and a bit error-prone (a property can accidentally be omitted in one schema.)

Am I overlooking another solution?

@sorinsarca
Copy link
Member

Just to be clear: if in 99% of cases we have to detect the readOnly/writeOnly properties and patch the required than this can be done using a custom validator.
I'll try to create a working example by the end of this week, and we'll see if this approach is good enough.

@bertramakers
Copy link
Author

I think that would solve most cases yes. It's also an issue that people are experiencing with Spectral stoplightio/spectral#1274

Someone linked to the Swagger (OpenAPI) docs there, which state:

If a readOnly or writeOnly property is included in the required list, required affects just the relevant scope – responses only or requests only. That is, read-only required properties apply to responses only, and write-only required properties – to requests only.

Of course that is documentation for Swagger/OpenAPI and not JSON schema. But looking at the OpenAPI 3.0 docs, it says:

OpenAPI schemas can also use the following keywords that are not part of JSON Schema:

  • ...
  • readOnly
  • writeOnly

https://swagger.io/docs/specification/data-models/data-types/

Those docs are probably outdated since readOnly and writeOnly are also part of JSON schema now. But if OpenAPI had them first, I'd assume that JSON schema adopted them with their original behaviour. Which would make the part in the OpenAPI docs about the required scope correct for JSON schema as well.

I'm no expert though, just trying to find as much relevant info about this as possible. So feel free to correct me if I overlooked or misinterpreted something here.

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