Mapping to a custom domain object produces uncompilable code when combined with bean validations generated from openapi spec

[tedberg]

Given openapi.yaml:

  /movies/{movieId}:
    parameters:
      - $ref: '#/components/parameters/movieId'
 
...

components:
  parameters:
    movieId:
      name: movieId
      in: path
      description: Identifier of a movie
      required: true
      schema:
        type: string
        minLength: 5
        maxLength: 15
        pattern: someRegex expression

Generating normally works, producing an API with @Size, @Pattern and the String movieId parameter. Then add the mapping.yaml config to use a preexisting MovieId record from our domain that already contains validation:

openapi-processor-mapping: v13

...

map:
    paths:
        /movies/{movieId}:
          parameters:
            - name: movieId => com.example.domain.MovieId

Now the generated code swaps out the String movieId for a MovieId reference, but we cannot compile because @Pattern and @Size are not compatible with any class other than String (or CharacterSequence). If I go back to openapi.yaml and remove:

        minLength: 5
        maxLength: 15
        pattern: someRegex expression

Then it will generate and compile successfully. This is not great though as then I have worsened my contract, generated documentation and generated clients.

Proposed solutions:

1. Add mapping options to remove annotations. Using syntax of:

map:
    paths:
        /movies/{movieId}:
          parameters:
            - name: movieId => com.example.domain.MovieId
            - drop: movieId @ Jakarta.validation.constraints.Size
            - drop: movieId @ Jakarta.validation.constraints.Pattern

We could tell the generator to build out an annotation removal list, which it could then use to omit these annotation instances.

2. Store the bean validator annotation to type compatibility matrix in memory, consult this after resolving type mappings, before rendering out the code. When the type replacements produce an incompatible pairing, as with movieId, use this intelligence to omit generation of these bean validation annotations. Ideally, the mapping logs will detect this omission and log a warning that by contract, we are obligated to perform these validations and to be sure that our manual code achieves this.

3. Best solution, add each of the above enhancements. The second one to automatically resolve generating invalid code and the first to allow for resolving any other edge case scenarios that occur.

For a test case, I'd refer to your medium article about mapping an integer to a year. If after type integer you simply add:

    type: integer
    format: year
    minimum: 1970
    maximum: 2099

This will fail with the same problem.

[hauner]

Interesting case, who would have thought of that ;-)

the drop variant fits to the drop of a full parameter, which would be nice.

Since we need an "annotation to allowed types" map, it makes sense to have that in the mapping so it can be easily extended and an explicit drop would not be necessary (?).

Something like:

bean-validation-types:
  javax/jakarta.validation.constraints.DecimalMin:
    - java.lang.BigDecimal
    - java.lang.BigInteger
    - java.lang.CharSequence
    - java.lang.Byte
    - java.lang.Short
    - java.lang.Integer
    - java.lang.Long
    - byte
    - short
    - int
    - long

The processor could initialize that internally to avoid having the full list in the mapping but still having a way to extend it.

Thanks for reporting!

[tedberg]

That looks good. DecimalMin and DecimalMax share the datatypes, so perhaps there is a way to codify this without repetition. Either way, if the library includes the standard mapping, but consuming projects can extend would be best.

The drop option could be a separate enhancement. Just a work around for edge cases. For example, in the docs is a global parameter annotation applied:

parameters:
- type: Foo @ io.openapiprocessor.samples.validations.Sum(value = 42)

When working with legacy code, you might find that 9 of 10 times, adding the @sum to Foo is perfect, but in the 10th case, a sizable refactor is needed to get that endpoint to be compliant. drop would let one patch that edge case until they could fix it properly.

[hauner]

Having a default allowed mappings does not work as nice as I thought.

DecimalMin allows, for example java.lang.CharSequence. The processor would have to check if the target type implements the interface. That is not possible for all classes because a class that implements it may not be on the classpath of the processor. It is not, if it is a project specific class.

It could check the implements for standard java classes, but it would be necessary to explicitly list all project specific classes. Considering your example I guess that would be most of the time.

So maybe it is just the drop then :-)

[tedberg]

Considering opt-in (add) vs opt-out (drop) for these cases, I'd think the vast majority of the time a custom mapping is added, it won't be compatible with the bean validator. The exception would be with time, where @past works with a long list, such as Year. However for this case, all the reasonable values are in java.util and java.time packages. (Obviously, @NotNull, @Valid and @null are exempt)

java.util.Date
java.util.Calendar
java.time.Instant
java.time.LocalDate
java.time.LocalDateTime
java.time.LocalTime
java.time.MonthDay
java.time.OffsetDateTime
java.time.OffsetTime
java.time.Year
java.time.YearMonth
java.time.ZonedDateTime
java.time.chrono.HijrahDate
java.time.chrono.JapaneseDate
java.time.chrono.MinguoDate
java.time.chrono.ThaiBuddhistDate

It looks like the java.time classes are all final.

So for the DecimalMin, I think if someone maps a custom child of one of:

    - java.lang.BigDecimal
    - java.lang.BigInteger
    - java.lang.CharSequence

Then forcing them to opt-in via adding the annotation would be cleaner than forcing an opt-out for all of the other cases. Though, I don't know if that mapping to add the annotation could be smart enough to get the value defined in openApi.