build(portal): enable OpenAPI generator and Java 8 compatibility#5468
Conversation
WalkthroughAdds OpenAPI-driven code generation to the apollo-portal module: introduces Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant Maven as Maven
participant OAG as OpenAPI-Generator
participant FS as Filesystem
participant Compiler as Java Compiler
Dev->>Maven: mvn generate-sources / mvn compile
note right of Maven #DDEBF7: Reads `apollo.openapi.spec.url` and plugin config\n(openapi-generator configured in POM)
Maven->>OAG: invoke openapi-generator (spring) with spec URL & options
OAG-->>FS: write generated Java sources to generated-sources/openapi
Maven->>Maven: build-helper-maven-plugin adds generated dir to sources
Maven->>Compiler: compile main + generated sources
Compiler-->>Maven: compiled classes
Maven-->>Dev: produced artifact
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🧪 Early access (Sonnet 4.5): enabledWe are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience. Note:
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apollo-portal/pom.xml(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build (8)
apollo-portal/pom.xml
Outdated
| <apollo.openapi.spec.url>https://raw.githubusercontent.com/tacklequestions/apollo-openapi/main/apollo-openapi.yaml</apollo.openapi.spec.url> | ||
| <github.path>${project.artifactId}</github.path> | ||
| <maven.build.timestamp.format>yyyyMMddHHmmss</maven.build.timestamp.format> |
There was a problem hiding this comment.
Avoid remote OpenAPI spec dependency in builds
Pointing <apollo.openapi.spec.url> to a personal GitHub raw URL makes every Maven build depend on third-party network availability and whatever happens to be on that branch at build time. That breaks reproducibility, fails in locked-down/offline build environments, and leaves us exposed if the external repo disappears or drifts. Please vendor the spec into this repo (or another controlled artifact) and default the property to that local path, keeping the remote URL only as an opt-in override.
🤖 Prompt for AI Agents
In apollo-portal/pom.xml around lines 30-32, the apollo.openapi.spec.url is
pointing to a remote raw GitHub URL which makes builds network-dependent; vendor
the OpenAPI spec into this repository (for example add apollo-openapi.yaml to
the repo root or src/main/resources), change the default
<apollo.openapi.spec.url> property to reference that local file path (e.g.
${project.basedir}/apollo-openapi.yaml or classpath:/apollo-openapi.yaml), and
keep the original remote URL only as an optional override via a separate
property or profile so consumers can opt in to using the remote location if
desired. Ensure any build/code that reads the property can handle file://,
classpath:, or relative paths consistently.
apollo-portal/pom.xml
Outdated
| <plugin> | ||
| <groupId>org.openapitools</groupId> | ||
| <artifactId>openapi-generator-maven-plugin</artifactId> | ||
| <version>7.15.0</version> |
There was a problem hiding this comment.
Due to the introduction of different versions of OpenAPI generator Maven plugin, it is necessary to ensure consistency in the generated code, especially in terms of parameter order
There was a problem hiding this comment.
Already fixed those issues, thanks for reviewing
apollo-portal/pom.xml
Outdated
| <artifactId>apollo-portal</artifactId> | ||
| <name>Apollo Portal</name> | ||
| <properties> | ||
| <apollo.openapi.spec.url>https://raw.githubusercontent.com/tacklequestions/apollo-openapi/main/apollo-openapi.yaml</apollo.openapi.spec.url> |
There was a problem hiding this comment.
Avoid associating personal code repositories with YAML.
There was a problem hiding this comment.
Already fixed those issues, thanks for reviewing
…y openapi-generator plugin version to avoid inconsistencies in generated code
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apollo-portal/pom.xml (2)
162-167: RevisitskipValidateSpec=true.Skipping validation can let broken specs pass and generate uncompilable sources. Recommend turning this off for local builds or adding a separate
validateexecution in CI as shown above.
140-172: Clarify OpenAPI Generator plugin executionMaven will merge the
<executions>from<pluginManagement>into your<build><plugins>declaration, so thegenerate-openapi-sourcesgoal will run as intended. For clearer intent, you may relocate the<executions>block directly under<build><plugins>and reserve<pluginManagement>solely for version/config defaults. Also consider adding avalidateexecution (e.g.<goal>validate</goal>in thevalidatephase) in your CI to fail fast on spec issues.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apollo-portal/pom.xml(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build (8)
🔇 Additional comments (2)
apollo-portal/pom.xml (2)
257-277: LGTM: generated sources are correctly added to compilation.Path matches the generator’s default layout under the chosen output dir. This will keep IDEs happy.
30-33: Avoid remote OpenAPI spec in default builds; vendor locally.Defaulting
apollo.openapi.spec.urlto a GitHub raw URL makes builds network‑dependent and non‑reproducible. Please point the default to a repo‑controlled file and leave the remote URL as an opt‑in override (e.g.,-Dapollo.openapi.spec.url=...). This also addresses the “personal repo YAML” concern noted by others.Apply this minimal change:
- <apollo.openapi.spec.url>https://raw.githubusercontent.com/apolloconfig/apollo-openapi/refs/heads/main/apollo-openapi.yaml</apollo.openapi.spec.url> + <!-- Default to a vendored spec; allow override via -Dapollo.openapi.spec.url --> + <apollo.openapi.spec.url>${project.basedir}/src/main/openapi/apollo-openapi.yaml</apollo.openapi.spec.url>Optionally add a profile for remote opt‑in:
+ <profiles> + <profile> + <id>use-remote-openapi</id> + <properties> + <apollo.openapi.spec.url>https://raw.githubusercontent.com/apolloconfig/apollo-openapi/refs/heads/main/apollo-openapi.yaml</apollo.openapi.spec.url> + </properties> + </profile> + </profiles>
| <dependency> | ||
| <groupId>org.openapitools</groupId> | ||
| <artifactId>jackson-databind-nullable</artifactId> | ||
| <version>0.2.6</version> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>io.swagger.core.v3</groupId> | ||
| <artifactId>swagger-annotations-jakarta</artifactId> | ||
| <version>2.2.22</version> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>io.swagger.core.v3</groupId> | ||
| <artifactId>swagger-models-jakarta</artifactId> | ||
| <version>2.2.22</version> | ||
| </dependency> |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Check jakarta vs javax alignment and narrow scopes if possible.
You added swagger-annotations-jakarta and swagger-models-jakarta. If the portal is still on Spring Boot 2.x (javax), prefer the non‑jakarta variants to avoid classpath mismatches. Also, if these are only needed to compile generated interfaces/models and not at runtime, mark them provided to keep the runtime surface small. Otherwise, keep as is.
Verification script:
If Boot 2.x / javax, switch deps:
- <artifactId>swagger-annotations-jakarta</artifactId>
+ <artifactId>swagger-annotations</artifactId>
...
- <artifactId>swagger-models-jakarta</artifactId>
+ <artifactId>swagger-models</artifactId>If runtime not required, restrict scope:
- </version>
+ </version>
+ <scope>provided</scope>🏁 Script executed:
#!/bin/bash
# Detect Spring Boot major and javax/jakarta usage
rg -nP -C2 '<artifactId>spring-boot-dependencies</artifactId>\s*<version>[^<]+' pom.xml **/pom.xml
rg -nP -C2 '\bimport\s+javax\.' --type=java
rg -nP -C2 '\bimport\s+jakarta\.' --type=javaLength of output: 107204
🏁 Script executed:
#!/bin/bash
# Confirm Spring Boot parent/BOM in apollo-portal/pom.xml
rg -nP -C3 '<parent>' apollo-portal/pom.xml
rg -nP '<artifactId>spring-boot-starter-parent</artifactId>' -C2 apollo-portal/pom.xml
rg -nP '<artifactId>spring-boot-dependencies</artifactId>' -C2 apollo-portal/pom.xml
rg -nP '<groupId>org.springframework.boot</groupId>' -C2 apollo-portal/pom.xmlLength of output: 1051
Align Swagger dependencies with javax-based Spring Boot 2.x and scope compile-only artifacts as provided
Replace swagger-annotations-jakarta/swagger-models-jakarta with the non-Jakarta variants (io.swagger.core.v3:swagger-annotations and io.swagger.core.v3:swagger-models, version 2.2.22) to match your Spring Boot 2.x (javax) classpath, and if these are only needed at compile time (for generated interfaces/models), add <scope>provided</scope>.
🤖 Prompt for AI Agents
In apollo-portal/pom.xml around lines 48 to 62, the PR adds Jakarta variants of
Swagger artifacts which conflict with the project’s javax (Spring Boot 2.x)
classpath; replace io.swagger.core.v3:swagger-annotations-jakarta and
io.swagger.core.v3:swagger-models-jakarta with
io.swagger.core.v3:swagger-annotations and io.swagger.core.v3:swagger-models
(version 2.2.22) respectively, and if those dependencies are only needed at
compile time for generated interfaces/models mark them with
<scope>provided</scope> so they are not bundled at runtime.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
apollo-portal/pom.xml
Outdated
| <dependency> | ||
| <groupId>org.openapitools</groupId> | ||
| <artifactId>jackson-databind-nullable</artifactId> | ||
| <version>0.2.6</version> |
There was a problem hiding this comment.
Manage the dependency version in the parent pom's dependencyManagement section.
There was a problem hiding this comment.
Already move that version management to parent's pom, thanks for reviewing!
| <dependency> | ||
| <groupId>org.openapitools</groupId> | ||
| <artifactId>jackson-databind-nullable</artifactId> | ||
| <version>0.2.6</version> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>io.swagger.core.v3</groupId> | ||
| <artifactId>swagger-annotations-jakarta</artifactId> | ||
| <version>2.2.22</version> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>io.swagger.core.v3</groupId> | ||
| <artifactId>swagger-models-jakarta</artifactId> | ||
| <version>2.2.22</version> | ||
| </dependency> |
apollo-portal/pom.xml
Outdated
|
|
||
| </dependencies> | ||
| <build> | ||
| <!-- openapi-generator插件配置 --> |
There was a problem hiding this comment.
already fixed that issue, thanks for reviewing!
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
apollo-portal/pom.xml (1)
30-30: Remote OpenAPI spec URL already flagged by previous reviewers.This has been identified in past reviews as breaking build reproducibility and creating a network dependency. The maintainer requested vendoring the spec into the repository.
🧹 Nitpick comments (1)
apollo-portal/pom.xml (1)
137-169: Consider enabling OpenAPI spec validation.The configuration sets
<skipValidateSpec>true</skipValidateSpec>, which disables validation of the OpenAPI specification. While this may speed up builds, it can allow malformed or inconsistent specs to slip through, leading to runtime issues or unexpected generated code.If the spec is well-formed, apply this diff to enable validation:
- <skipValidateSpec>true</skipValidateSpec> + <skipValidateSpec>false</skipValidateSpec>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apollo-portal/pom.xml(4 hunks)pom.xml(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build (8)
🔇 Additional comments (3)
apollo-portal/pom.xml (2)
248-272: LGTM! OpenAPI generator and build-helper integration looks correct.The plugin declarations properly wire OpenAPI code generation into the Maven lifecycle and add generated sources to the compile path.
48-59: Scope OpenAPI dependencies as provided if only compile-timeNo references to
JsonNullableor Swagger v3 annotations/models were found in your custom code underapollo-portal/src. If these artifacts are only needed to compile the generated sources and not at runtime, add<scope>provided</scope>to each of the
jackson-databind-nullable,swagger-annotations, andswagger-modelsdependencies inapollo-portal/pom.xml. Confirm that your runtime environment supplies these libraries and that none of the generated code requires them on the packaged classpath.pom.xml (1)
208-222: LGTM! Dependency management properly centralizes OpenAPI/Swagger versions.The parent POM correctly defines versions for jackson-databind-nullable, swagger-annotations, and swagger-models in dependencyManagement, allowing child modules to inherit them without duplication.
…anagement in parent POM Co-authored-by: arrow2020 <[email protected]>
bd631d0 to
c2b7b9c
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apollo-portal/pom.xml (1)
30-30: Remote OpenAPI spec URL still breaks build reproducibility.This property still points to a remote GitHub URL, making every build network-dependent and non-reproducible. This was flagged in previous reviews, and while you acknowledged fixing it, the remote URL remains. Vendor the OpenAPI spec into this repository (e.g.,
src/main/resources/apollo-openapi.yaml) and change the default to a local path like${project.basedir}/src/main/resources/apollo-openapi.yaml.Apply this approach:
- <apollo.openapi.spec.url>https://raw.githubusercontent.com/apolloconfig/apollo-openapi/refs/heads/main/apollo-openapi.yaml</apollo.openapi.spec.url> + <apollo.openapi.spec.url>${project.basedir}/src/main/resources/apollo-openapi.yaml</apollo.openapi.spec.url>Then add the
apollo-openapi.yamlfile toapollo-portal/src/main/resources/in the repository.
🧹 Nitpick comments (1)
apollo-portal/pom.xml (1)
254-272: Consider managing build-helper-maven-plugin version in parent POM.The
build-helper-maven-pluginversion 3.4.0 is hard-coded here. For consistency with other plugins in the Apollo project and to facilitate version upgrades, consider moving this version declaration to the parent POM's<pluginManagement>section.Example structure for parent
pom.xml:<pluginManagement> <plugins> <plugin> <groupId>org.codehaus.mojo</groupId> <artifactId>build-helper-maven-plugin</artifactId> <version>3.4.0</version> </plugin> </plugins> </pluginManagement>Then remove the version from this file:
<plugin> <groupId>org.codehaus.mojo</groupId> <artifactId>build-helper-maven-plugin</artifactId> - <version>3.4.0</version> <executions>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apollo-portal/pom.xml(4 hunks)pom.xml(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- pom.xml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build (8)
🔇 Additional comments (2)
apollo-portal/pom.xml (2)
137-169: Java version profiles missing; spec validation skipped.No
<profile>definitions forjava8/java11were found in eitherapollo-portal/pom.xmlor the root POM, despite the PR stating separate profiles for plugin versions 6.6.0 (java8) and 7.15.0 (java11plus). Additionally,<skipValidateSpec>true</skipValidateSpec>bypasses OpenAPI spec validation and could allow invalid specs to generate broken code. Please confirm where the Java version profiles are defined or add them here, and reconsider settingskipValidateSpectofalseto catch spec issues early.
48-59: Dependency versions properly managed in parent POM.The three new dependencies are declared under
<dependencyManagement>in the parentpom.xmlwith corresponding version properties; no further action required.
What's the purpose of this PR
Enable OpenAPI code generation for portal (interfaces/models) and set up Java 8/11 compatibility so subsequent OpenAPI-related refactors can build against generated types without touching runtime logic.
Which issue(s) this PR fixes:
Fixes #5462
Brief changelog
Summary by CodeRabbit