[codex] Complete portal OpenAPI migration#5618
Open
nobodyiam wants to merge 22 commits into
Open
Conversation
dosubot
Bot
added
the
size:XXL
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughCompletes Portal UI management OpenAPI migration: bump OpenAPI spec to v0.3.4, add PortalManagementController and PortalUserController, enrich OpenAppDTOs with owner display names, migrate frontend calls to /openapi/v1, enhance URL-collection tooling, and update tests/docs. ChangesPortal OpenAPI v1 Migration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
Completes the Apollo Portal OpenAPI migration by moving the remaining Portal UI management endpoints and frontend call sites to /openapi/v1/..., updating the portal to consume the released apollo-openapi spec v0.3.4, and enhancing tooling/docs to track the migration status.
Changes:
- Adds Portal-session-only OpenAPI v1 controllers for remaining Portal management surfaces (users/consumers/audit/favorites/search/system/server-config/import/export, etc.).
- Migrates remaining frontend JS services/controllers/directives to
/openapi/v1/...routes and updates the generated URL inventory to include direct$http/download/select2 calls outsidestatic/scripts/services. - Updates OpenAPI enrichment for App owner display name and bumps
apollo-portalOpenAPI spec URL tov0.3.4, along with refreshed migration tracking docs.
Reviewed changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/openapi/collect_portal_frontend_urls.py | Extends URL inventory extraction to cover direct API calls and variable-based URL expressions; updates summary labeling to “Source”. |
| scripts/openapi/collect_portal_frontend_urls_test.py | Adds test coverage for scanning non-service JS files (controllers/directives) and direct download/select2 calls. |
| docs/zh/contribution/apollo-portal-openapi-migration-tracking.md | Updates migration status and contract target to v0.3.4 and reflects full frontend OpenAPI coverage. |
| docs/zh/contribution/apollo-portal-openapi-frontend-url-inventory.md | Refreshes generated URL inventory (now “Source”), reflecting 131/131 OpenAPI entries and additional scanned files. |
| docs/en/contribution/apollo-portal-openapi-migration-tracking.md | English migration tracking update to v0.3.4 and full frontend OpenAPI adoption. |
| docs/en/contribution/apollo-portal-openapi-frontend-url-inventory.md | English URL inventory refresh and inclusion of additional sources outside service files. |
| apollo-portal/src/test/java/com/ctrip/framework/apollo/openapi/v1/controller/PortalUserControllerTest.java | Adds unit tests for new Portal user management OpenAPI controller behavior and auth-type gating. |
| apollo-portal/src/test/java/com/ctrip/framework/apollo/openapi/v1/controller/PortalManagementControllerTest.java | Adds unit tests for selected Portal management OpenAPI endpoints (page settings, commit visibility, export). |
| apollo-portal/src/test/java/com/ctrip/framework/apollo/openapi/server/service/ServerAppOpenApiServiceTest.java | Updates service test to cover/enforce owner display name enrichment via AdditionalUserInfoEnrichService. |
| apollo-portal/src/main/resources/static/scripts/services/UserService.js | Migrates user APIs to /openapi/v1/user and /openapi/v1/users.... |
| apollo-portal/src/main/resources/static/scripts/services/SystemInfoService.js | Migrates system info/health calls to /openapi/v1/system-info.... |
| apollo-portal/src/main/resources/static/scripts/services/ServerConfigService.js | Migrates server config endpoints to /openapi/v1/server/.... |
| apollo-portal/src/main/resources/static/scripts/services/ReleaseHistoryService.js | Migrates release history endpoint to /openapi/v1/.../releases/histories. |
| apollo-portal/src/main/resources/static/scripts/services/GlobalSearchValueService.js | Migrates global search item-info endpoint to /openapi/v1/global-search/.... |
| apollo-portal/src/main/resources/static/scripts/services/FavoriteService.js | Migrates favorites CRUD/reorder endpoints to /openapi/v1/favorites.... |
| apollo-portal/src/main/resources/static/scripts/services/ExportService.js | Migrates config import endpoint to /openapi/v1/import. |
| apollo-portal/src/main/resources/static/scripts/services/ConsumerService.js | Migrates consumer/token/role APIs to /openapi/v1/.... |
| apollo-portal/src/main/resources/static/scripts/services/CommonService.js | Migrates page-settings endpoint to /openapi/v1/page-settings. |
| apollo-portal/src/main/resources/static/scripts/services/CommitService.js | Migrates commit history endpoint to /openapi/v1/.../commits. |
| apollo-portal/src/main/resources/static/scripts/services/AuditLogService.js | Migrates audit log endpoints to /openapi/v1/apollo/audit/.... |
| apollo-portal/src/main/resources/static/scripts/services/AppService.js | Migrates app CRUD/roles/system-role checks to OpenAPI routes and adapts create payload shape to OpenAPI contract. |
| apollo-portal/src/main/resources/static/scripts/directive/namespace-panel-directive.js | Migrates namespace item export download URL to /openapi/v1/.../items/export. |
| apollo-portal/src/main/resources/static/scripts/directive/import-namespace-modal-directive.js | Migrates namespace item import upload URL to /openapi/v1/.../items/import. |
| apollo-portal/src/main/resources/static/scripts/directive/directive.js | Migrates app search and user selector select2 AJAX URLs to /openapi/v1/.... |
| apollo-portal/src/main/resources/static/scripts/controller/ConfigExportController.js | Migrates config export/import and app export/import URLs to /openapi/v1/.... |
| apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/enricher/adapter/OpenAppDtoUserInfoEnrichedAdapter.java | Adds adapter to enrich OpenAppDTO owner display names via shared enrichment pipeline. |
| apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/v1/controller/PortalUserController.java | Introduces Portal-session-only OpenAPI controller for current user, search, create/update, enable/disable. |
| apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/v1/controller/PortalManagementController.java | Adds Portal-session-only OpenAPI controller for remaining UI management surfaces (audit, favorites, server config, import/export, app search, etc.). |
| apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/util/OpenApiModelConverters.java | Adds converters for OpenUserDTO→UserPO and list-based UserInfo→OpenUserInfoDTO. |
| apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/server/service/ServerAppOpenApiService.java | Enriches returned OpenAPI app DTOs with owner display names for Portal UI compatibility. |
| apollo-portal/pom.xml | Pins apollo.openapi.spec.url to released apollo-openapi v0.3.4. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
apollo-portal/src/main/resources/static/scripts/services/AppService.js (1)
162-169: ⚡ Quick winPreserve backend create response when available.
Line 169 always resolves the original
app, which drops any server-normalized/generated fields returned bycreate_app. Prefer returningresultwith a fallback.Proposed fix
- }, function () { - d.resolve(app); + }, function (result) { + d.resolve(result || app); }, function (result) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apollo-portal/src/main/resources/static/scripts/services/AppService.js` around lines 162 - 169, The code currently always resolves the original app object (d.resolve(app)) inside the success callback of app_resource.create_app, discarding any server-normalized/generated fields; update the success callback for app_resource.create_app to accept the server response (e.g., result) and resolve with the server result if present, falling back to the original app (e.g., d.resolve(result || app)); locate the callback passed to app_resource.create_app in the block using appPayload and admins and replace the fixed d.resolve(app) with a resolution that prefers the server response.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/v1/controller/PortalManagementController.java`:
- Around line 228-230: In PortalManagementController, validate the incoming env
string before calling Env.valueOf(...) or Env.transformEnv(...) (e.g., in calls
around commitService.find and commitService.findByKey) by using the same helper
used elsewhere to convert and reject unknown environments; if the conversion
yields Env.UNKNOWN or is invalid, return a 400 Bad Request instead of proceeding
(do this for all listed call sites including the ones around commitService.find,
findByKey and other handlers referenced in this file). Ensure each method checks
the validated Env result and only passes a valid Env into commitService.find,
commitService.findByKey, or any other service call, mirroring the existing
helper usage and error response pattern in this class.
- Around line 362-369: The method searchItemInfoByKeyOrValue currently calls
key.isEmpty() and value.isEmpty() which throws NPE if either query param is
missing; change the guard to treat null or empty the same (e.g., use
Objects.isNull(key) || key.isEmpty(), Strings.isNullOrEmpty(key) or
org.apache.commons.lang3.StringUtils.isEmpty(key)) and do the same for value,
then throw the existing BadRequestException when both are null/empty; leave the
call to globalSearchService.getAllEnvItemInfoBySearch(key, value, 0,
portalConfig.getPerEnvSearchMaxResults()) unchanged.
In
`@apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/v1/controller/PortalUserController.java`:
- Around line 85-89: Replace the inappropriate UnsupportedOperationException
with an access-control exception when permission is denied: in the conditional
that uses unifiedPermissionValidator.isSuperAdmin(), user.getUsername(),
userInfoHolder.getUser().getUserId(), user.getEnabled() and USER_ENABLED, throw
a proper access-denied exception (e.g.
org.springframework.security.access.AccessDeniedException) with a clear message
instead of UnsupportedOperationException so the framework returns an
authorization failure response.
In
`@apollo-portal/src/test/java/com/ctrip/framework/apollo/openapi/v1/controller/PortalManagementControllerTest.java`:
- Around line 19-24: The test class PortalManagementControllerTest still uses
JUnit 4 APIs and lacks class Javadoc; update it to JUnit 5 by replacing
org.junit.Assert static imports with org.junit.jupiter.api.Assertions (e.g.,
assertEquals, assertNotNull, assertTrue) and replace
`@RunWith`(MockitoJUnitRunner.class) with `@ExtendWith`(MockitoExtension.class),
convert any `@Test`(expected=...) usages to assertThrows in test methods, adjust
imports for org.junit.jupiter.api.Test, and ensure Mockito static imports (when,
verify, verifyNoInteractions) remain; finally add a short class-level Javadoc
describing the test purpose above the PortalManagementControllerTest class
declaration.
In
`@apollo-portal/src/test/java/com/ctrip/framework/apollo/openapi/v1/controller/PortalUserControllerTest.java`:
- Around line 19-20: The test class PortalUserControllerTest uses JUnit4;
migrate it to JUnit5 by replacing org.junit.Before and org.junit.After with
org.junit.jupiter.api.BeforeEach and AfterEach on the corresponding
setup/teardown methods, replace org.junit.Test imports with
org.junit.jupiter.api.Test, remove `@RunWith`(MockitoJUnitRunner.class) and
annotate the class with
`@ExtendWith`(org.mockito.junit.jupiter.MockitoExtension.class), update static
assert imports to org.junit.jupiter.api.Assertions (use
assertEquals/assertNotNull from JUnit Jupiter), and convert any
`@Test`(expected=...) usages to assertThrows(...) calls (wrap the code that should
throw inside a lambda and assert the thrown exception type) while keeping
existing Mockito mocks and method names intact (e.g., retain
PortalUserControllerTest class name and its setup/teardown method names).
In `@scripts/openapi/collect_portal_frontend_urls.py`:
- Line 323: Replace the fullwidth colon in the f-string "-
前端文件数：{service_count}" with a standard ASCII colon to resolve Ruff RUF001;
locate the f-string containing the symbol sequence "- 前端文件数：{service_count}" and
change the punctuation to "- 前端文件数: {service_count}" (keeping the same
interpolation of service_count) so the string no longer contains the ambiguous
Unicode colon.
- Around line 211-214: The branch handling collect_resource_base that calls
collect_resource_expression and sets method = "RESOURCE_BASE" must also resolve
variable references inside the extracted $resource(...) expression before
extracting paths; update the logic in that branch to detect when the expression
contains a variable name (e.g., `$resource(baseUrl, ...)`), look up and
substitute the variable from the existing variables/context map used elsewhere
in this script (same place where other variable resolution happens), then run
the path extraction on the resolved string and proceed with method
"RESOURCE_BASE". Ensure you use the same helper/resolution mechanism used by
other parts of the file so variable lookups are consistent with functions like
collect_resource_expression.
---
Nitpick comments:
In `@apollo-portal/src/main/resources/static/scripts/services/AppService.js`:
- Around line 162-169: The code currently always resolves the original app
object (d.resolve(app)) inside the success callback of app_resource.create_app,
discarding any server-normalized/generated fields; update the success callback
for app_resource.create_app to accept the server response (e.g., result) and
resolve with the server result if present, falling back to the original app
(e.g., d.resolve(result || app)); locate the callback passed to
app_resource.create_app in the block using appPayload and admins and replace the
fixed d.resolve(app) with a resolution that prefers the server response.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b8273947-ab75-47dd-bda6-51cbcf147f35
📒 Files selected for processing (32)
CHANGES.mdapollo-portal/pom.xmlapollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/server/service/ServerAppOpenApiService.javaapollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/util/OpenApiModelConverters.javaapollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/v1/controller/PortalManagementController.javaapollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/v1/controller/PortalUserController.javaapollo-portal/src/main/java/com/ctrip/framework/apollo/portal/enricher/adapter/OpenAppDtoUserInfoEnrichedAdapter.javaapollo-portal/src/main/resources/static/scripts/controller/ConfigExportController.jsapollo-portal/src/main/resources/static/scripts/directive/directive.jsapollo-portal/src/main/resources/static/scripts/directive/import-namespace-modal-directive.jsapollo-portal/src/main/resources/static/scripts/directive/namespace-panel-directive.jsapollo-portal/src/main/resources/static/scripts/services/AppService.jsapollo-portal/src/main/resources/static/scripts/services/AuditLogService.jsapollo-portal/src/main/resources/static/scripts/services/CommitService.jsapollo-portal/src/main/resources/static/scripts/services/CommonService.jsapollo-portal/src/main/resources/static/scripts/services/ConsumerService.jsapollo-portal/src/main/resources/static/scripts/services/ExportService.jsapollo-portal/src/main/resources/static/scripts/services/FavoriteService.jsapollo-portal/src/main/resources/static/scripts/services/GlobalSearchValueService.jsapollo-portal/src/main/resources/static/scripts/services/ReleaseHistoryService.jsapollo-portal/src/main/resources/static/scripts/services/ServerConfigService.jsapollo-portal/src/main/resources/static/scripts/services/SystemInfoService.jsapollo-portal/src/main/resources/static/scripts/services/UserService.jsapollo-portal/src/test/java/com/ctrip/framework/apollo/openapi/server/service/ServerAppOpenApiServiceTest.javaapollo-portal/src/test/java/com/ctrip/framework/apollo/openapi/v1/controller/PortalManagementControllerTest.javaapollo-portal/src/test/java/com/ctrip/framework/apollo/openapi/v1/controller/PortalUserControllerTest.javadocs/en/contribution/apollo-portal-openapi-frontend-url-inventory.mddocs/en/contribution/apollo-portal-openapi-migration-tracking.mddocs/zh/contribution/apollo-portal-openapi-frontend-url-inventory.mddocs/zh/contribution/apollo-portal-openapi-migration-tracking.mdscripts/openapi/collect_portal_frontend_urls.pyscripts/openapi/collect_portal_frontend_urls_test.py
Member
Author
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e61a83b9ad
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 84cc767ad5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Member
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Hooray! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
nobodyiam
force-pushed
the
codex/portal-openapi-final-migration
branch
from
f3ecda8 to
7852220
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7852220096
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8f7a871f47
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ef0c5a6edf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 804bc06968
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Member
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What's the purpose of this PR
Complete the remaining Apollo Portal OpenAPI migration by moving the final Portal UI management surfaces to generated OpenAPI contracts and routes. This PR also updates
apollo-portalto consume the releasedapollo-openapiv0.3.4spec.Which issue(s) this PR fixes:
N/A.
Brief changelog
/openapi/v1/..., keeping UI-local response adapters where current Portal screens require legacy shapes.apollo-portalto the releasedapollo-openapiv0.3.4tag.Follow this checklist to help us incorporate your contribution quickly and easily:
mvn clean testto make sure this pull request doesn't break anything.mvn spotless:applyto format your code.CHANGESlog.Tests
./mvnw clean test(BUILD SUCCESS, 02:50)./mvnw -B -pl apollo-assembly -am -DskipTests package(BUILD SUCCESS, 36.004 s)PORTAL_USERNAME=apollo PORTAL_PASSWORD=admin ./e2e/portal-e2e/scripts/wait-for-ready.sh(cd e2e/portal-e2e && npm ci && npx playwright install chromium && BASE_URL=http://127.0.0.1:8070 PLAYWRIGHT_WORKERS=2 npm run test:e2e:ci)(19 passed, 1.3m)./mvnw spotless:applygit diff --checkOpenAPI contract
Summary by CodeRabbit
New Features
Improvements
Refactor
Tests
Chores