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][JAVA] Uses spring pageable without any config indicating to do so #13052

Closed
4 tasks done
downloadpizza opened this issue Aug 1, 2022 · 5 comments · Fixed by #15113
Closed
4 tasks done

[BUG][JAVA] Uses spring pageable without any config indicating to do so #13052

downloadpizza opened this issue Aug 1, 2022 · 5 comments · Fixed by #15113

Comments

@downloadpizza
Copy link

downloadpizza commented Aug 1, 2022

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 searched for related issues/PRs?
  • What's the actual output vs expected output?
Description

Using any schema called Pageable will instead use springs pageable (in a spring project)

openapi-generator version

6.0.1

OpenAPI declaration file content or url

https://github.com/downloadpizza/spring-openapi-pageable-poc

Generation Details

refer to pom.xml in my project

Steps to reproduce
  1. Clone my project
  2. Run maven compile
  3. Look in generated PagedObject class and check which Pageable is used.
Related issues/PRs

#8360

Suggest a fix

redo the entire Pageable and springfox thing. #8192 (comment) as this user has said (which I agree with) it is a dumb thing for a mature library. Its a random very intrusive change that benefits very few people.

@downloadpizza
Copy link
Author

Bump, this is not a big issue but it is documented nowhere and randomly breaks stuff, not very funny.

@Mintas
Copy link
Contributor

Mintas commented Nov 8, 2022

@downloadpizza it looks like you can try this workaround with little tweak in your schema (replaced ref with type for Pageable):
typeMappings = ['pageable': 'xyz.model.Pageable']

components:
  schemas:
    Pageable:
      type: object
      properties:
        definetly_not_pageable:
          type: string
    PagedObject:
      type: object
      properties:
        value:
          type: string
        pageable:
          type: pageable

btw, I see two possible fixes:
a) https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/SpringCodegen.java#L485 put importMapping for Pageable only if x-spring-paginated enabled

b) make all that importMapping thing customizable, using putIfAbsent(), then you could have used workaround by setting
importMappings = ['Pageable': 'xyz.model.Pageable']

@tofi86
Copy link

tofi86 commented Nov 18, 2022

@Mintas regarding your suggested fix: the importMapping map you're referring to is just a map. It does not actually put the Pageable import in the list of imports to be used in the template. This is actually done behind a guard at https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/SpringCodegen.java#L1089

@tofi86
Copy link

tofi86 commented Nov 18, 2022

@Mintas sorry, my fault. now I understand what you meant!

@downloadpizza
Copy link
Author

@downloadpizza it looks like you can try this workaround with little tweak in your schema (replaced ref with type for Pageable): typeMappings = ['pageable': 'xyz.model.Pageable']

While I am not working on the project where this was an issue anymore. Your solution helped me on something completely unrelated. Thanks!

borsch pushed a commit that referenced this issue May 30, 2023
)

* spring: fix spring pageable used without any config

Spring Pageable is imported whenever we have an API with
a Pageable Json schema, even if x-spring-paginated is not
set.

This commit imports Spring Pageable only if
x-spring-paginated is set to true.

* spring: add unit test for pageable fix

* spring: fix spring pageable feedback from review

remove unrelated example

---------

Co-authored-by: gonzalad <gonzalad@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants