Skip to content

Conversation

@arrow1991
Copy link
Contributor

@arrow1991 arrow1991 commented Dec 29, 2025

What's the purpose of this PR

Remove redundant code

Follow this checklist to help us incorporate your contribution quickly and easily:

  • [√] Read the Contributing Guide before making this pull request.
  • [√] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • [√] Write necessary unit tests to verify the code.
  • [√] Run mvn clean test to make sure this pull request doesn't break anything.
  • [√] Run mvn spotless:apply to format your code.
  • [√] Update the CHANGES log.

Summary by CodeRabbit

  • Refactor

    • Simplified null and empty collection checks
    • Replaced lambda expressions with method references for readability
    • Streamlined variable initializations
  • Chores

    • Standardized logging to parameterized formatting
    • Removed redundant checked exceptions from several method signatures

✏️ Tip: You can customize this high-level summary in your review settings.

@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Dec 29, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 29, 2025

📝 Walkthrough

Walkthrough

Multiple non-functional code cleanups across the Apollo codebase: removed redundant null initializations, dropped unnecessary checked exceptions from method signatures, modernized collection checks, converted lambdas to method references, and updated logging to parameterized SLF4J. No behavioral logic changes reported.

Changes

Cohort / File(s) Summary
Variable Declaration Cleanup
apollo-adminservice/src/main/java/com/ctrip/framework/apollo/adminservice/controller/AppController.java, apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/v1/controller/ClusterController.java, apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/ItemService.java
Removed explicit null initializations from local variable declarations; variables are assigned on all control paths before use.
Checked Exception Removal
apollo-audit/apollo-audit-impl/src/main/java/com/ctrip/framework/apollo/audit/context/ApolloAuditScopeManager.java, apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/component/RestTemplateFactory.java, apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/controller/UserInfoController.java
Removed throws declarations for checked exceptions (e.g., IOException, UnsupportedEncodingException) and related imports; method bodies unchanged.
Collection Emptiness Checks
apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/service/ReleaseService.java, apollo-common/src/main/java/com/ctrip/framework/apollo/common/dto/PageDTO.java, apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/NamespaceService.java
Replaced size() comparisons with isEmpty()/!isEmpty() checks for readability; behavior preserved.
Null/Empty Guard Simplification
apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/component/config/PortalConfig.java, apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/FavoriteService.java
Simplified null/empty guarding conditions (removed redundant empty-array checks or cross-checks) while keeping existing control flow invariants.
Stream / Method Reference Refactor
apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/component/txtresolver/PropertyResolver.java, apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/ItemService.java, apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/NamespaceService.java
Replaced lambda expressions with method references (e.g., this::isCommentItem, ItemDTO::getKey) and method-reference for forEach calls; functional equivalence retained.
Logging Modernization
apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/component/AdminServiceAddressLocator.java, apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/environment/PortalMetaDomainService.java, apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/ConfigsExportService.java, apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/spi/ldap/FilterLdapByGroupUserSearch.java
Replaced String.format() and string concatenation with parameterized SLF4J logging using {} placeholders and passing the exception as a separate parameter.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested labels

lgtm

Suggested reviewers

  • hezhangjian

Poem

🐰 I nibble code with gentle paws and cheer,
Nulls tuck away, the lines are clear and near.
Lambdas hop to references so spry,
Logs wear tidy braces, neat and dry.
A rabbit claps—refactor delight!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 7.69% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main refactoring objective: removing redundant null initializations, exception declarations, and string checks with idiomatic alternatives, which aligns with all the changes across multiple files.
✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a7b0dd1 and a3b2211.

📒 Files selected for processing (16)
  • apollo-adminservice/src/main/java/com/ctrip/framework/apollo/adminservice/controller/AppController.java
  • apollo-audit/apollo-audit-impl/src/main/java/com/ctrip/framework/apollo/audit/context/ApolloAuditScopeManager.java
  • apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/service/ReleaseService.java
  • apollo-common/src/main/java/com/ctrip/framework/apollo/common/dto/PageDTO.java
  • apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/v1/controller/ClusterController.java
  • apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/component/AdminServiceAddressLocator.java
  • apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/component/RestTemplateFactory.java
  • apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/component/config/PortalConfig.java
  • apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/component/txtresolver/PropertyResolver.java
  • apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/controller/UserInfoController.java
  • apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/environment/PortalMetaDomainService.java
  • apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/ConfigsExportService.java
  • apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/FavoriteService.java
  • apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/ItemService.java
  • apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/NamespaceService.java
  • apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/spi/ldap/FilterLdapByGroupUserSearch.java
🚧 Files skipped from review as they are similar to previous changes (11)
  • apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/component/txtresolver/PropertyResolver.java
  • apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/environment/PortalMetaDomainService.java
  • apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/NamespaceService.java
  • apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/ConfigsExportService.java
  • apollo-audit/apollo-audit-impl/src/main/java/com/ctrip/framework/apollo/audit/context/ApolloAuditScopeManager.java
  • apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/component/AdminServiceAddressLocator.java
  • apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/service/ReleaseService.java
  • apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/component/config/PortalConfig.java
  • apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/ItemService.java
  • apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/v1/controller/ClusterController.java
  • apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/FavoriteService.java
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-29T07:37:59.647Z
Learnt from: tacklequestions
Repo: apolloconfig/apollo PR: 5499
File: apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/v1/controller/AppController.java:82-104
Timestamp: 2025-12-29T07:37:59.647Z
Learning: In apollo-portal AppController.createApp (Java), the operator is intentionally derived from the request body fields (dataChangeCreatedBy or ownerName) for all authentication types, unlike other methods in the same controller (updateApp, createAppInEnv, deleteApp) which use conditional operator handling based on auth type. This design maintains historical consistency in the user experience.

Applied to files:

  • apollo-adminservice/src/main/java/com/ctrip/framework/apollo/adminservice/controller/AppController.java
🔇 Additional comments (5)
apollo-adminservice/src/main/java/com/ctrip/framework/apollo/adminservice/controller/AppController.java (1)

83-93: LGTM! Clean removal of redundant null initialization.

The explicit = null initialization was unnecessary since both branches of the if-else (lines 88 and 90) guarantee that app is assigned before use on line 92. Java's definite assignment analysis confirms this is safe.

apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/component/RestTemplateFactory.java (1)

66-66: LGTM! Clean removal of unnecessary exception declaration.

The method body doesn't throw UnsupportedEncodingException, so removing it from the signature is correct and improves code clarity.

apollo-common/src/main/java/com/ctrip/framework/apollo/common/dto/PageDTO.java (1)

56-58: LGTM! Good refactoring to use isEmpty().

The change from size() > 0 to !isEmpty() is more idiomatic and clearly expresses the intent. The null check is properly preserved.

apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/spi/ldap/FilterLdapByGroupUserSearch.java (1)

77-79: LGTM! Parameterized logging is more idiomatic and efficient.

The change from string concatenation to parameterized logging follows SLF4J best practices and avoids unnecessary string construction.

apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/controller/UserInfoController.java (1)

108-110: No changes needed. The removal of throws IOException is correct. The LogoutHandler interface and all its implementations (DefaultLogoutHandler and OidcLogoutHandler) do not declare IOException in their method signatures, confirming the exception declaration was redundant in the controller method.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@arrow1991 arrow1991 changed the title * Remove redundant code * Refactor: Replace null initializations, exception declarations, and string checks with idiomatic alternatives Dec 29, 2025
Copy link
Member

@nobodyiam nobodyiam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jan 1, 2026
@nobodyiam nobodyiam merged commit ab71dc8 into apolloconfig:master Jan 1, 2026
9 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jan 1, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

lgtm This PR has been approved by a maintainer size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants