Readonly Flag in openapi and validation

[ahoeing]

Hello @SDA-SE/sda-commons-maintainer ,

we had a discussion some times ago about resource models with read/write models. @christopher-cudennec , you should remember at least ;)

We currently favour the usage of the readonly flag in openapi specifications for such attributes that cannot be set by consumers instead of creating read and write model

          id:
            type: "string"
            description: "Technical identifier of the CommunicationAddress which will\
              \ be generated if not provided"
            readOnly: true
            example: "c0399bc8-81c9-11eb-8dcd-0242ac13000"

This flag can be created with openapi annotations in java:

  @Schema(
      description =
          "Technical identifier of the partner which will be generated if not " + "provided",
      example = "fabe4baa-81c8-11eb-8dcd-0242ac130003",
      accessMode = Schema.AccessMode.READ_ONLY,
      required = true)

But this annotation does not cause a real validation, that the field is not set in writing operations.
Validation can be implemented with a custom validator

public class ReadOnlyValidator implements ConstraintValidator<ReadOnly, Object> {
  @Override
  public boolean isValid(Object value, ConstraintValidatorContext context) {
    return value == null;
  }
}

and used e.g. that way

  @ReadOnly(groups = CheckReadOnly.class)
  private String id;

with activation of this validation only for operations writing the resource

  Response write(
          @NotNull
          @Valid
          @Validated(CheckReadOnly.class)
          Resource resource);

I think this feature can also be helpful for others and I would like to contribute that instead of creating my own library.

André

[JoergSiebahn]

You may use @JsonProperty(access = JsonProperty.Access.READ_ONLY) to avoid getting the property filled from a consumer.
If you do validation on read only properties, you break the tolerant reader pattern regarding „ignore unknown fields“.

[JoergSiebahn]

In a matching API specification for the POST body resource representation I would not mention the property … so for me it is unknown.

[ahoeing]

But if you don`t include that in the OpenAPI specification, the consumer will not know that he has to set it, so that deserialization in the service can be successful, when the consumer will not use the Java API directly, or am I missing something?

[JoergSiebahn]

the consumer will not know that he has to set it

Am I getting you wrong? My understanding of your scenario is that the consumer should not set the id property in POST request bodies, e.g. on resource creation, but receives the id on GET response bodies.

I remember a discussion for an implementation detail about using a single class in the backend for GET and POST. In my opinion you are just simplifying code for two different resource representations (MyResourceCreateRepresentation and MyResourceReadRepresentation) with the effect that they are documented as one in the generated OpenAPI. Then the only option regarding the property differences you have is to define them either as read only or as write only. That is what you are facing right now, isn't it? I would say, that has the same meaning as two separately defined representations and therefore needs no validation. But you must make sure, that your code implements the spec you define. And that is what @JsonProperty(access = JsonProperty.Access.READ_ONLY) is doing.

[ahoeing]

No its not right. We are currently merging the Read/Write Classes to one single class and mark the read only attributes in the API as read only. That is able with swagger/swagger annotations, so that an attribute of the resource has the readOnly: true flag in OpenAPI file.

Now we have operations like resource creations (/resources) or even "change" Operations, where we reuse the same java classes to not duplicate to much. We even have an operation, where you can more or less replace the complete resource. There Operations are Write Operations - I assume swagger editor includes readonly attributes only GET operation examples.

So, we have one resource e.g. an Offer that has the id filed marked as readOnly. Now I want to validate that the user does really not fill the ID field, when using a POST /offers operation.
In this case it must be null.

[JoergSiebahn]

We are currently merging the Read/Write Classes to one single class …

I assume, you had no id property in the write model before? Due to the tolerant reader pattern (ignore unknown properties), in that case, it was allowed to send the id and it has been ignored. If you enforce the absence of an id property in the joined model, you will introduce a breaking change, if my assumption is correct. However, it may not affect your consumers, because they did not send the id.

I assume swagger editor includes readonly attributes only GET operation examples.

I assume the same. But it is imho not related to how the server handles the property.

Just as reference, in json-schema both variants (ignore or reject read only properties) are possible. From an SDA perspective - encouraging fault tolerance - I would say, read only properties should be ignored and not rejected. @christopher-cudennec would like to hear your thoughts.

Another remark: To me the setup of the API (with validation groups) feels more complex than needed while just ignoring the id leads to the same result.

[christopher-cudennec]

Do you have a working example for your combination of readOnly + validation?
My assumption was that Jackson will not invoke the setter on deserialization if the attribute is marked as readOnly.

[JoergSiebahn]

Good idea to ask for tests. Here is a self contained passing test that can be used in a sda-dropwizard-service:

TLDR;

When @ReadOnly is defined for the property and the validation is called with the desired group class,

• then, validation is not affected when only @JsonProperty(access = JsonProperty.Access.READ_ONLY) is used (setter not called)
• then, validation is affected when only @Schema(accessMode = Schema.AccessMode.READ_ONLY) is used (id is set)
• then, validation is not affected when @JsonProperty(access = JsonProperty.Access.READ_ONLY) and @Schema(accessMode = Schema.AccessMode.READ_ONLY) are used (setter not called)

Outside the test, I checked the effect on the generated OpenAPI:

• any of @JsonProperty(access = JsonProperty.Access.READ_ONLY) or @Schema(accessMode = Schema.AccessMode.READ_ONLY) will result in readOnly: true in the json-schema.

import static java.lang.annotation.ElementType.ANNOTATION_TYPE;
import static java.lang.annotation.ElementType.CONSTRUCTOR;
import static java.lang.annotation.ElementType.FIELD;
import static java.lang.annotation.ElementType.METHOD;
import static java.lang.annotation.ElementType.PARAMETER;
import static java.lang.annotation.ElementType.TYPE_USE;
import static java.lang.annotation.RetentionPolicy.RUNTIME;
import static org.assertj.core.api.Assertions.assertThat;

import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import io.swagger.v3.oas.annotations.media.Schema;
import java.lang.annotation.Documented;
import java.lang.annotation.Retention;
import java.lang.annotation.Target;
import javax.validation.Constraint;
import javax.validation.ConstraintValidator;
import javax.validation.ConstraintValidatorContext;
import javax.validation.Payload;
import javax.validation.Validation;
import javax.validation.Validator;
import org.junit.jupiter.api.Test;
import org.sdase.commons.server.jackson.ObjectMapperConfigurationUtil;

class ReadOnlyTest {

  static final String JSON_WITH_ID = "{\"id\": \"abc\"}";

  Validator validator = Validation.buildDefaultValidatorFactory().getValidator();

  ObjectMapper objectMapper = ObjectMapperConfigurationUtil.configureMapper().build();

  @Test
  void shouldValidateWithAllAnnotations() throws JsonProcessingException {
    var resource = objectMapper.readValue(JSON_WITH_ID, SchemaPropertyAndValidationDefined.class);
    assertThat(resource).isNotNull().extracting("id").isNull();
    var constraintViolations = validator.validate(resource, CheckReadOnly.class);
    assertThat(constraintViolations).isEmpty();
  }

  @Test
  void shouldValidateWithSchemaAndValidationAnnotations() throws JsonProcessingException {
    var resource = objectMapper.readValue(JSON_WITH_ID, SchemaAndValidationDefined.class);
    assertThat(resource).isNotNull().extracting("id").isEqualTo("abc");
    var constraintViolations = validator.validate(resource, CheckReadOnly.class);
    assertThat(constraintViolations).hasSize(1);
  }

  @Test
  void shouldValidateWithPropertyAndValidationAnnotations() throws JsonProcessingException {
    var resource = objectMapper.readValue(JSON_WITH_ID, PropertyAndValidationDefined.class);
    assertThat(resource).isNotNull().extracting("id").isNull();
    var constraintViolations = validator.validate(resource, CheckReadOnly.class);
    assertThat(constraintViolations).isEmpty();
  }

  @Test
  void shouldValidateWithPropertyAnnotation() throws JsonProcessingException {
    var resource = objectMapper.readValue(JSON_WITH_ID, PropertyDefined.class);
    assertThat(resource).isNotNull().extracting("id").isNull();
    var constraintViolations = validator.validate(resource, CheckReadOnly.class);
    assertThat(constraintViolations).isEmpty();
  }

  @Target({METHOD, FIELD, ANNOTATION_TYPE, CONSTRUCTOR, PARAMETER, TYPE_USE})
  @Retention(RUNTIME)
  @Documented
  @Constraint(validatedBy = {ReadOnlyValidator.class})
  public @interface ReadOnly {
    String message() default "read only";
    Class<?>[] groups() default {};
    Class<? extends Payload>[] payload() default {};
  }

  public static class ReadOnlyValidator implements ConstraintValidator<ReadOnly, Object> {
    @Override
    public boolean isValid(Object value, ConstraintValidatorContext context) {
      return value == null;
    }
  }

  public interface CheckReadOnly {}

  public static class SchemaPropertyAndValidationDefined {

    @Schema(accessMode = Schema.AccessMode.READ_ONLY) // => readOnly in openAPI
    @JsonProperty(access = JsonProperty.Access.READ_ONLY) // => readOnly in openAPI
    @ReadOnly(groups = CheckReadOnly.class)
    private String id;

    public String getId() {
      return id;
    }

    public void setId(String id) {
      this.id = id;
    }
  }

  public static class SchemaAndValidationDefined {

    @Schema(accessMode = Schema.AccessMode.READ_ONLY) // => readOnly in openAPI
    @ReadOnly(groups = CheckReadOnly.class)
    private String id;

    public String getId() {
      return id;
    }

    public void setId(String id) {
      this.id = id;
    }
  }

  public static class PropertyAndValidationDefined {

    @JsonProperty(access = JsonProperty.Access.READ_ONLY) // => readOnly in openAPI
    @ReadOnly(groups = CheckReadOnly.class)
    private String id;

    public String getId() {
      return id;
    }

    public void setId(String id) {
      this.id = id;
    }
  }

  public static class PropertyDefined {

    @JsonProperty(access = JsonProperty.Access.READ_ONLY) // => readOnly in openAPI
    private String id;

    public String getId() {
      return id;
    }

    public void setId(String id) {
      this.id = id;
    }
  }
}