belgif / rest-guide

REST Guidelines of Belgian government institutions
https://www.belgif.be/specification/rest/api-guide/
Apache License 2.0
24 stars 4 forks source link

Allow readOnly + required properties #143

Closed jpraet closed 4 months ago

jpraet commented 11 months ago

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 commented 11 months ago

We didn't include this bc of two reasons:

kernixski commented 11 months ago

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 commented 10 months ago

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 commented 9 months ago

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 commented 9 months ago

discussed on WG:

pvdbosch commented 4 months ago

PR has been merged and will be published next release.