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

Allow readOnly + required properties #143

Closed
jpraet opened this issue Sep 28, 2023 · 6 comments
Closed

Allow readOnly + required properties #143

jpraet opened this issue Sep 28, 2023 · 6 comments
Assignees

Comments

@jpraet
Copy link
Contributor

jpraet commented Sep 28, 2023

https://www.belgif.be/specification/rest/api-guide/#rule-oas-rdonly states that

Properties marked as readOnly being true SHOULD NOT be in the required list of the defined schema.

But the spec supports this: https://spec.openapis.org/oas/v3.0.3#fixed-fields-19

If the property is marked as readOnly being true and is in the required list, the required will take effect on the response only.

@pvdbosch
Copy link
Contributor

We didn't include this bc of two reasons:

@kernixski
Copy link

I am not convinced that json-schema-validation (still in draft) is OK. I mean: it partially removes the usefulness of eiter "readOnly"/"writeOnly" or "required". OpenAPI 3.0 spec has more sense. It will force us to create specific objects for the request and the response if we want the contract to be clear and complete, and that's really annoying.

@pvdbosch
Copy link
Contributor

pvdbosch commented Oct 5, 2023

JSON schema seems to be perpetually draft status :)

I comprehend that JSON Schema is a standard that's unaware of request or response concepts; it only defines how a schema to validate a JSON document. It's comparable to XML Schema or Jakarta Bean Validation standards that can't express "read only" in this sense either.

It might be possible for the OpenAPI standard at some point to add an extension keyword like "requiredInResponse", but it can't override "required" from JSON Schema because that would lead to interoperability problems between the two standards; which is what we currently see in OpenAPI 3.0 and JSON Schema draft-wright-00.

@pvdbosch
Copy link
Contributor

Did some tests; handling of readOnly and writeOnly seems to have been improved in recent openapi-generator (7.0.1), at least for spring and JAX-RS.
Getters/setters are always generated regardless of readOnly/writeOnly; and @NotNull only if property is required and not readOnly.

@Schema(name = "readOnlyRequiredProperty", accessMode = Schema.AccessMode.READ_ONLY, requiredMode = Schema.RequiredMode.REQUIRED)
 @JsonProperty("readOnlyRequiredProperty")
 public String getReadOnlyRequiredProperty() {
   return readOnlyRequiredProperty;
 }

 public void setReadOnlyRequiredProperty(String readOnlyRequiredProperty) {
   this.readOnlyRequiredProperty = readOnlyRequiredProperty;
 }

 @Schema(name = "writeOnlyProperty", requiredMode = Schema.RequiredMode.NOT_REQUIRED)
 @JsonProperty("writeOnlyProperty")
 public String getWriteOnlyProperty() {
   return writeOnlyProperty;
 }

 public void setWriteOnlyProperty(String writeOnlyProperty) {
   this.writeOnlyProperty = writeOnlyProperty;
 }

 @NotNull 
 @Schema(name = "requiredProperty", requiredMode = Schema.RequiredMode.REQUIRED)
 @JsonProperty("requiredProperty")
 public String getRequiredProperty() {
   return requiredProperty;
 }

 public void setRequiredProperty(String requiredProperty) {
   this.requiredProperty = requiredProperty;
 }

I'd still avoid setting readOnly on required properties, to be forward-compatible with OpenAPI 3.1.

@pvdbosch
Copy link
Contributor

discussed on WG:

  • add explanation for restriction in REST guide (forwards-compatibility with OAS 3.1)
  • rule can be verified in styleguide-validator tool

@pvdbosch
Copy link
Contributor

pvdbosch commented Apr 5, 2024

PR has been merged and will be published next release.

@pvdbosch pvdbosch closed this as completed Apr 5, 2024
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