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

[BUG][SPRING] x-field-extra-annotation missing in 6.5.0 #15408

Closed
4 of 6 tasks
tofi86 opened this issue May 3, 2023 · 17 comments
Closed
4 of 6 tasks

[BUG][SPRING] x-field-extra-annotation missing in 6.5.0 #15408

tofi86 opened this issue May 3, 2023 · 17 comments

Comments

@tofi86
Copy link

tofi86 commented May 3, 2023

Bug Report Checklist

  • Have you provided a full/minimal spec to reproduce the issue?
  • Have you validated the input using an OpenAPI validator (example)?
  • Have you tested with the latest master to confirm the issue still exists?
  • Have you searched for related issues/PRs?
  • What's the actual output vs expected output?
  • [Optional] Sponsorship to speed up the bug fix or feature request (example)
Description
openapi-generator version

regression in 6.5.0

OpenAPI declaration file content or url

This OA spec snippet

    CompanyDto:
      type: object
      description: ....
      properties:
        priceCategory:
          description: The price category
          nullable: true
          x-field-extra-annotation: '@IgnoreForRoles("MEDIA_ADMIN")'
          allOf:
            - $ref: '#/components/schemas/SamplingPriceCategoryEnum'
Expected Output

... produced the following output in v6.4.0 (using openApiNullable = true):

  @JsonProperty("priceCategory")
  @IgnoreForRoles("MEDIA_ADMIN")
  private JsonNullable<SamplingPriceCategoryEnum> priceCategory = JsonNullable.undefined();
Actual Output

... but produces the following in 6.5.0:

  private JsonNullable<SamplingPriceCategoryEnum> priceCategory = JsonNullable.undefined();
@borsch
Copy link
Member

borsch commented May 4, 2023

@tofi86 could you please share plugin configuration/CLI command that you are using

@jkt628
Copy link

jkt628 commented May 4, 2023

i'm seeing this from gradle plugin org.openapi.generator configured with

tasks.register('generateEventRouter', GenerateTask) {
	generatorName.set("spring")
	inputSpec.set(eventRouter)
	outputDir.set("$buildDir/generated/openapi")
	apiPackage.set("eventRouter.rest")
	modelPackage.set("eventRouter.model")
	configOptions.set([
		interfaceOnly: "true"
	])
}

i tried versions back to 6.2.0 with no joy.

@borsch
Copy link
Member

borsch commented May 5, 2023

@jkt628 can you create minimal reproducible example? I've tried latest master, but it works as expected

@tofi86
Copy link
Author

tofi86 commented May 5, 2023

@borsch sure, this is my maven configuration:

<configuration>
    <inputSpec>${project.basedir}/src/main/resources/test-15408.yaml</inputSpec>

    <generatorName>spring</generatorName>
    <library>spring-boot</library>

    <apiPackage>org.example.backend.api.generated</apiPackage>
    <invokerPackage>org.example.backend.api.generated.handler</invokerPackage>
    <modelPackage>org.example.backend.data.generated</modelPackage>

    <globalProperties>
        <!--
            Don't skip form model (multipart/form-data) generation as this has a bug as described in:
            https://github.com/OpenAPITools/openapi-generator/issues/12036
        -->
        <skipFormModel>false</skipFormModel><!-- default: true -->
    </globalProperties>

    <configOptions>
        <basePackage>org.example.backend.api</basePackage>
        <configPackage>org.example.backend.api.v1.config</configPackage>

        <!-- Use interfaceOnly pattern... -->
        <interfaceOnly>true</interfaceOnly><!-- default: false -->

        <!-- Use Java 8 LocalDate/LocalDateTime classes for dates -->
        <dateLibrary>java8-localdatetime</dateLibrary><!-- default: java8 -->

        <!-- Enable OpenAPI Jackson Nullable library for models -->
        <openApiNullable>true</openApiNullable><!-- default: false -->

        <!-- Use Bean Validation Impl. to perform BeanValidation -->
        <useBeanValidation>true</useBeanValidation><!-- default: true -->
        <performBeanValidation>true</performBeanValidation><!-- default: false -->

        <!-- Use Optional<> classes for non-required request params -->
        <useOptional>true</useOptional><!-- default: false -->

        <!-- Hides the generation timestamp when files are generated -->
        <hideGenerationTimestamp>true</hideGenerationTimestamp><!-- default: false -->

        <!-- Do not generate springdoc/springfox docs -->
        <documentationProvider>none</documentationProvider><!-- default: springdoc -->
    </configOptions>
    <typeMappings>
        <typeMapping>time=java.time.LocalTime</typeMapping>
    </typeMappings>
</configuration>

And this is the test spec I was using for the example:

openapi: 3.0.3

info:
  title: "API"
  description: "REST API"
  version: 0.0.1
  license:
    name: "GNU General Public License v3.0"
    url: "https://choosealicense.com/licenses/gpl-3.0/"
  contact:
    name: "Team"
    email: "[email protected]"
    url: "https://www.example.org.de"


servers:
  - url: http://localhost:8080
    description: Local development server


components:

  securitySchemes:
    BearerAuth:
      description: JWT authorization header using the Bearer scheme.
      type: http
      scheme: bearer
      bearerFormat: JWT

  schemas:
    CompanyDto:
      type: object
      properties:
        priceCategory:
          description: The price category
          nullable: true
          x-field-extra-annotation: '@IgnoreForRoles("MEDIA_ADMIN")'
          allOf:
            - $ref: '#/components/schemas/SamplingPriceCategoryEnum'

    SamplingPriceCategoryEnum:
      description: The price category of a sampling action
      type: string
      enum:
        - FREE
        - PRICE_TIER_1
        - PRICE_TIER_2


security:
  - bearerAuth: [ ]


paths:

  /company:
    summary: Represents a Company

Generated Java code looks like this:

public class CompanyDto {

  private JsonNullable<SamplingPriceCategoryEnum> priceCategory = JsonNullable.undefined();

  public CompanyDto priceCategory(SamplingPriceCategoryEnum priceCategory) {
    this.priceCategory = JsonNullable.of(priceCategory);
    return this;
  }

  /**
   * Get priceCategory
   * @return priceCategory
  */
  @Valid 
  @JsonProperty("priceCategory")
  public JsonNullable<SamplingPriceCategoryEnum> getPriceCategory() {
    return priceCategory;
  }

  public void setPriceCategory(JsonNullable<SamplingPriceCategoryEnum> priceCategory) {
    this.priceCategory = priceCategory;
  }

 ...

@tofi86 tofi86 changed the title [BUG][JAVA] x-field-extra-annotation missing in 6.5.0 [BUG][SPRING] x-field-extra-annotation missing in 6.5.0 May 5, 2023
@jkt628
Copy link

jkt628 commented May 5, 2023

@jkt628
Copy link

jkt628 commented May 9, 2023

i found an ugly way to at least mark the first field with @Id through https://github.com/jkt628/x-field-extra-annotation/pull/1/commits/a040d18b2352c1465944d0a7ef63d54f2382fb7b. going through pojo.mushtache it looks like vendorExtensions.x-field-extra-annotation is not set for the field which will bomb a bunch of generators.

@borsch
Copy link
Member

borsch commented May 10, 2023

i found an ugly way to at least mark the first field with @Id through jkt628/x-field-extra-annotation@a040d18. going through pojo.mushtache it looks like vendorExtensions.x-field-extra-annotation is not set for the field which will bomb a bunch of generators.

Unfortunately you can't use both $ref & x-field-extra-annotation at same time. This would be the correct approach

    EventRouterMessage:
      type: object
      x-class-extra-annotation: |-
        @javax.persistence.Entity
        @javax.persistence.Table(name="event_router_requests")
      properties:
        guid:
          x-field-extra-annotation: |-
            @javax.persistence.Id
            @javax.persistence.Column(name = "id")
          type: string                  <--- this is the change. use direct configuration here instead of $ref
          pattern: '^[0-9A-Fa-f]{8}-(?:[0-9A-Fa-f]{4}-){3}[0-9A-Fa-f]{12}$'
        accounting:
          $ref: '#/components/schemas/Accounting'
        event:
          $ref: '#/components/schemas/Event'

@borsch
Copy link
Member

borsch commented May 10, 2023

@tofi86 same issue with yours spec - swagger can't handle both $ref/$allOf/$onOff & extensions or other field. In your case correct schema would be

    CompanyDto:
      type: object
      properties:
        priceCategory:
          description: The price category
          nullable: true
          x-field-extra-annotation: '@IgnoreForRoles("MEDIA_ADMIN")'
          type: string     <---- this is the change. Move enum here instead of allOf + $ref
          enum:
          - FREE
          - PRICE_TIER_1
          - PRICE_TIER_2
          title: SamplingPriceCategoryEnum

@tofi86
Copy link
Author

tofi86 commented May 10, 2023

@borsch Sure, this could be a valid solution for this issue.

But on the other hand

  • the enum wouldn't be reusable anymore (no enum class SamplingPriceCategoryEnum is generated).
  • it worked just fine in 6.4 🤷‍♂️

@tofi86
Copy link
Author

tofi86 commented May 10, 2023

the enum wouldn't be reusable anymore (no enum class SamplingPriceCategoryEnum is generated).

Oh, now I see the title: SamplingPriceCategoryEnum. Seems like I wasn't aware of this feature to generate this enum class with a certain class name.

@borsch
Copy link
Member

borsch commented May 10, 2023

@borsch Sure, this could be a valid solution for this issue.

But on the other hand

* the enum wouldn't be reusable anymore (no enum class `SamplingPriceCategoryEnum` is generated).

* it worked just fine in 6.4 🤷‍♂️

I'll check 6.4.0 version

@jkt628
Copy link

jkt628 commented May 10, 2023

unfortunately, i'm coming around to recognize openapi-generator is just not flexible enough for what i want to do without a lot of customization. in the long run it will be more maintainable to find another way to meet my needs.

@borsch borsch closed this as completed May 17, 2023
@tofi86
Copy link
Author

tofi86 commented May 17, 2023

@borsch why closed? You wanted to check v6.4.0 where this worked 🤷

@borsch borsch reopened this May 18, 2023
@borsch
Copy link
Member

borsch commented May 24, 2023

@tofi86 could you please create minimal project which generate expected code with 6.4.0, but not with 6.5.0.

I've tried this project, but getting same code with both versions

minimal(ish) example: https://github.com/jkt628/x-field-extra-annotation

Also based on diff v6.4.0...v6.5.0 I also haven't found anything suspicious yet

@tofi86
Copy link
Author

tofi86 commented May 24, 2023

@borsch cannot speak for the repo you mentioned as it isn't mine. Did you try with my example code from further above? -> #15408 (comment)

If this doesn't help or work for you I can try to set up a small project later today or tomorrow...

@borsch
Copy link
Member

borsch commented May 24, 2023

@tofi86 I've finally found what the issue here. This regression was introduced after #14882 where special handling was added for allOf with single schema. I'll create PR with fix

@jkt628 BTW. This fix should also work for you if use simple change $ref to allOf + single $ref. So in you case it would be

EventRouterMessage:
      type: object
      x-class-extra-annotation: |-
        @javax.persistence.Entity
        @javax.persistence.Table(name="event_router_requests")
      properties:
        guid:
          x-field-extra-annotation: |-
            @javax.persistence.Id
          allOf:                        <-- this is the change from $ref to allOf+$ref
            - $ref: '#/components/schemas/GUID'

@borsch
Copy link
Member

borsch commented May 25, 2023

@tofi86 @jkt628 please try 7.0.0-SNAPSHOT build once build from 26 of May would be available here

https://oss.sonatype.org/content/repositories/snapshots/org/openapitools/openapi-generator-cli/7.0.0-SNAPSHOT/

@borsch borsch closed this as completed May 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants