micronaut-projects / micronaut-openapi

Generates OpenAPI / Swagger Documentation for Micronaut projects
https://micronaut-projects.github.io/micronaut-openapi/latest/guide/index.html
Apache License 2.0
80 stars 95 forks source link

OpenAPI Polimorfizm issue in controller #1794

Closed alapierre closed 1 month ago

alapierre commented 1 month ago

Expected Behavior

I have OpenApi model with inheritance and polimorfizm like:

BaseInvoiceDto:

    BaseInvoiceDto:
      type: object
      required:
        - sellerVatId
        - docType
      properties:
        docType:
          $ref: '#/components/schemas/DocType'
        sellerVatId:
          type: string
          maxLength: 10
...
      discriminator:
        propertyName: docType
        mapping:
          FS: "#/components/schemas/SalesInvoiceCreateDto"
          FW: "#/components/schemas/CurrencyInvoiceCreateDto"

and two child classes:

    SalesInvoiceCreateDto:
      allOf:
        - $ref: '#/components/schemas/BaseInvoiceDto'
    CurrencyInvoiceCreateDto:
      allOf:
        - $ref: '#/components/schemas/SalesInvoiceCreateDto'
        - type: object
          properties:
            currency:
              type: string
              description: "Currency of the invoice"
              default: "PLN"
            exchangeRate:
              type: number
              description: "Exchange rate for the currency"
            exchangeDate:
              type: string
              format: date
          required:
            - currency
            - exchangeRate
            - exchangeDate

almost all works fine, expect controller part. Generator create interface CreateInvoiceRequest and makes it parameter of controller method:

  /api/invoice:
    post:
      tags:
        - Invoice
      operationId: createInvoice
      summary: Creates new sale invoice
      requestBody:
        required: true
        content:
          application/json:
            schema:
              oneOf:
                - $ref: '#/components/schemas/SalesInvoiceCreateDto'
                - $ref: '#/components/schemas/CurrencyInvoiceCreateDto'
    @Operation(
        operationId = "createInvoice",
        summary = "Creates new sale invoice",
        responses = {
            @ApiResponse(responseCode = "201", description = "Invoice created correctly", content = @Content(mediaType = "application/json", schema = @Schema(implementation = InvoiceIdDto.class))),
            @ApiResponse(responseCode = "403", description = "No access to cost center with Manager rights", content = @Content(mediaType = "application/json", schema = @Schema(implementation = ErrorDTO.class)))
        },
        security = @SecurityRequirement(name = "bearerAuth")
    )
    @Post("/api/invoice")
    @Secured(SecurityRule.IS_AUTHENTICATED)
    HttpResponse<@Valid InvoiceIdDto> createInvoice(
        @Body @NotNull @Valid CreateInvoiceRequest createInvoiceRequest
    );
public class SalesInvoiceCreateDto extends BaseInvoiceDto implements CreateInvoiceRequest 

and

public class CurrencyInvoiceCreateDto extends SalesInvoiceCreateDto implements CreateInvoiceRequest

when I call this controller I got error:

Failed to convert argument [invoice] for value [null] due to: Unable to deserialize type [interface dev.itrust.fk.api.model.CreateInvoiceRequest]: No default constructor exists"

but if I manually change parameter type to BaseInvoiceDto all works fine. What is propose of creating CreateInvoiceRequest interface and why it is controller parametr?

Actual Behaviour

controller not works

Steps To Reproduce

No response

Environment Information

Example Application

No response

Version

4.6.1, 4.6.2

altro3 commented 1 month ago

Could you create a simple reproducer application for it? I need full information about the project settings to quickly understand what the problem is. The information in the issue is not enough

alapierre commented 1 month ago

here is sample project: https://github.com/alapierre/micronaut-sample/tree/openapi (branch openapi)

I now understand propose of CreateInvoiceRequest - it is to limit objects cause by oneOf element in OpenAPI. This is completely understandable. Byt why Micronaut can't deserialize JSON request base on this interface?

altro3 commented 1 month ago

Ok, i'll check it tomorrow

alapierre commented 1 month ago

workaround is to make controller in this way:

  /api/invoice:
    post:
      tags:
        - Invoice
      operationId: createInvoice
      summary: Creates new sale invoice
      requestBody:
        required: true
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/BaseInvoiceDto'
#              oneOf:
#                - $ref: '#/components/schemas/SalesInvoiceCreateDto'
#                - $ref: '#/components/schemas/CurrencyInvoiceCreateDto'
      responses:
        201:
          description: Invoice created correctly
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/InvoiceIdDto'
        403:
          description: No access to cost center with Manager rights
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/ErrorDTO'

but still, it should work with oneOf

altro3 commented 1 month ago

You have a mistake in your sspec: need to write this:

  /api/invoice:
    post:
      tags:
        - Invoice
      operationId: createInvoice
      summary: Creates new sale invoice
      requestBody:
        required: true
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/BaseInvoiceDto'

But yes, models were generated with error. I fixed it here: https://github.com/micronaut-projects/micronaut-openapi/pull/1795

alapierre commented 1 month ago

but still, this should work too:

              oneOf:
                - $ref: '#/components/schemas/SalesInvoiceCreateDto'
                - $ref: '#/components/schemas/CurrencyInvoiceCreateDto'
altro3 commented 1 month ago

I don't understand how this is supposed to work. You describe subtypes and discriminator in another schema, but here you describe a new schema that knows nothing about discriminator and subtypes. That's where the problem comes from.

alapierre commented 1 month ago

class hierary looks like that:

    BaseInvoiceDto
            |                                                                                        
 SalesInvoiceCreateDto 
            |
CurrencyInvoiceCreateDto    

BaseInvoiceDto has discriminator definition, and two types inherit it form the base.

It is standard OpenAPI pattern - you can use oneOf to describe expected objects in request or response, eg:

  /api/invoice/{id}:
    get:
      tags:
        - Invoice
      parameters:
        - in: path
          name: id
          schema:
            type: string
            format: uuid
      operationId: loadInvoice
      summary: load invoice
      responses:
        200:
          description: Invoice
          content:
            application/json:
              schema:
                oneOf:
                  - $ref: '#/components/schemas/SalesInvoiceCreateDto'
                  - $ref: '#/components/schemas/CurrencyInvoiceCreateDto'
        403:
          description: No access to cost center with Manager rights
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/ErrorDTO'

It is convenient to describe only part of class hierarchy and to describe in details what will be returned. If I return BaseDto how API client will know about extra fields in child types?

In case when polymorphic object is in response, Mircornaut works almost OK:

GET http://localhost:8080/api/invoice/ce7140fb-eded-4d01-a3ac-bc0742a551bd
Accept: application/json
HTTP/1.1 200 OK
date: Fri, 4 Oct 2024 05:38:42 GMT
content-type: application/json
content-length: 136

{
  "": "CurrencyInvoiceCreateDto",
  "currency": "EUR",
  "docType": "FW",
  "exchangeRate": 4.75,
  "sellerVatId": "12345678",
  "exchangeDate": "2024-10-04"
}

so, why it can't works if this pattern is in POST request, like that:

  /api/invoice:
    post:
      tags:
        - Invoice
      operationId: createInvoice
      summary: Creates new sale invoice
      requestBody:
        required: true
        content:
          application/json:
            schema:
              oneOf:
                - $ref: '#/components/schemas/SalesInvoiceCreateDto'
                - $ref: '#/components/schemas/CurrencyInvoiceCreateDto'
      responses:
        201:
          description: Invoice created correctly
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/InvoiceIdDto'
        403:
          description: No access to cost center with Manager rights
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/ErrorDTO'

What is not OK with GET response?

  1. it returns attribute with empty name "" with Java class name - it is not necessary, because type is describe by discriminator field docType
  2. it not understand discriminator value - I have to set it manually in controller. Why?
@Controller
public class InvoiceResource implements InvoiceApi {
    @Override
    public HttpResponse<@Valid InvoiceIdDto> createInvoice(CreateInvoiceRequest createInvoiceRequest) {
        return HttpResponse.created(new InvoiceIdDto().docNumber("123").id(UUID.randomUUID()));
    }

    @Override
    public CreateInvoiceRequest loadInvoice(UUID id) {

        val invoice = new CurrencyInvoiceCreateDto("EUR", BigDecimal.valueOf(4.75), LocalDate.now(), "12345678");
        invoice.setDocType(DocType.FW);

        return invoice;
    }

}
alapierre commented 1 month ago

A more practical example of class inheritance usage in response is in sample projekt: https://github.com/alapierre/micronaut-sample/tree/openapi

I hope you will see why I need BaseInvoiceDto

    SalesInvoiceDto:
      allOf:
        - $ref: '#/components/schemas/BaseInvoiceDto'
        - $ref: '#/components/schemas/InvoiceCommonFields'
    CurrencyInvoiceDto:
      allOf:
        - $ref: '#/components/schemas/CurrencyInvoiceCreateDto'
        - $ref: '#/components/schemas/InvoiceCommonFields'
    InvoiceCommonFields:
      properties:
        issuedUser:
          type: string
        issueDate:
          type: string
          format: date
altro3 commented 1 month ago

Let me write you again what is wrong specifically for code generation:

  1. You describe a general scheme and in it a discriminator
    BaseInvoiceDto:
      type: object
      required:
        - sellerVatId
        - docType
      properties:
        docType:
          $ref: '#/components/schemas/DocType'
        sellerVatId:
          type: string
          maxLength: 10
      discriminator:
        propertyName: docType
        mapping:
          FS: "#/components/schemas/SalesInvoiceCreateDto"
          FW: "#/components/schemas/CurrencyInvoiceCreateDto"

Next you describe the sub-schemes that will be derived from this one and jackson will understand, during deserialization, which class it is trying to create based on the discriminator value.

Everything is fine and clear with this - everything is ok. The generator understands everything and adds Jackson annotations for the discriminator:

@JsonTypeInfo
@JsonSubTypes
@JsonSubTypes.Type

Next you describe an ANONYMOUS schema WITHOUT a discriminator and say that the object will be one of two schemas. Since the useOneOfInterfaces mode is enabled by default, the anonymous schema is generated as an interface to somehow link the schemas in the oneOf block.

      schema:
        oneOf:
          - $ref: '#/components/schemas/SalesInvoiceCreateDto'
          - $ref: '#/components/schemas/CurrencyInvoiceCreateDto'

You can try disabling the useOneOfInterfaces setting, then instead of an interface a class will be created and 2 other classes will inherit from it.

So, the code is correct.

But in both cases (useOneOfInterfaces = true and useOneOfInterfaces = false ) Jackson lacks information about the discriminator for correct serialization / deserialization, i.e. it lacks those annotations that were described above. The generator cannot create these annotations, because your anonymous schema does not have any information about the discriminator.

alapierre commented 1 month ago

OK, thank you for explanation. I just wondering, why it works for GET request?

responses:
        200:
          description: Invoice
          content:
            application/json:
              schema:
                oneOf:
                  - $ref: '#/components/schemas/SalesInvoiceCreateDto'
                  - $ref: '#/components/schemas/CurrencyInvoiceCreateDto'
altro3 commented 1 month ago

In my opinion, the difference is obvious. In the first case, deserialization occurs, and Jackson needs to create an object of the correct class, which it must select, depending on the value of the type field. But, as I said earlier, Jackson does not know anything about the discriminator in this case - hence the problem.

In the second case, it is serialization. And then you create an object of the required class. Therefore, Jackson sees the annotations with the help of reflections

@JsonTypeInfo
@JsonSubTypes
@JsonSubTypes.Type

Thus, it can correctly set the value in the type field

alapierre commented 1 day ago

Ok, thank you for this explanation. I was just wondering – if Jackson understands the inheritance hierarchy with BaseInvoiceDto but cannot understand the same with the generated interface – why doesn't this help:

@Generated("io.micronaut.openapi.generator.JavaMicronautServerCodegen")
@JsonIgnoreProperties(
        value = "", // ignore manually set , it will be automatically generated by Jackson during serialization
        allowSetters = true // allows the  to be set during deserialization
)
@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, property = "", visible = true)
@JsonSubTypes({
        @JsonSubTypes.Type(value = SalesInvoiceCreateDto.class, name = "FS"),
        @JsonSubTypes.Type(value = CurrencyInvoiceCreateDto.class, name = "FW")
})
public interface CreateInvoiceRequest {
    public DocType getDocType();
}

I noticed that the Spring Boot generator adds this annotation by default to the generated interface when polimorfizm comes into the picture.

Using oneOf is a much more standard approach in OpenAPI, and it is much more explicit than using a base class. Just take a look at how Swagger GUI presents these two scenarios.

Here is like it looks for oneOf

image

image

One can simply understand how to prepare correct data for this endpoint.

BTW, look at small imperfections in generated by Micronaut code:

@JsonIgnoreProperties(
        value = ""