Skip to content

refactor(config): 重构配置类中的常量定义和方法实现#5583

Open
youngzil wants to merge 14 commits intoapolloconfig:masterfrom
youngzil:refactor/config-classes
Open

refactor(config): 重构配置类中的常量定义和方法实现#5583
youngzil wants to merge 14 commits intoapolloconfig:masterfrom
youngzil:refactor/config-classes

Conversation

@youngzil
Copy link
Contributor

@youngzil youngzil commented Mar 19, 2026

What's the purpose of this PR

对 BizConfig、RefreshableConfig、PortalConfig 进行重构,提取常量、合并重复逻辑,提升可读性和可维护性。

Which issue(s) this PR fixes:

Fixes #

Brief changelog

RefreshableConfig

  • 提取 LIST_SEPARATORCONFIG_REFRESH_INTERVALEMPTY_STRING_ARRAY 常量
  • 新增 checkInt() 方法,统一整型配置校验逻辑

BizConfig

  • 新增 INTERVAL_TIME_UNIT 常量,简化 5 个返回 TimeUnit.SECONDS 的方法

PortalConfig

  • 新增 getEnvSetProperty() 私有方法,合并 emailSupportedEnvs()webHookSupportedEnvs()publishTipsSupportedEnvs() 的重复逻辑
  • 提取各类 DEFAULT_* 常量,消除魔法数字

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

    • Consolidated timing units and default timeout/constants across configuration components for consistency.
    • Introduced a shared empty-array constant and moved integer bounds validation into a common configuration helper.
    • Simplified environment-list parsing and tightened empty-value handling to consistently return empty sets.
  • Chores

    • Added a release note entry documenting the refactor.

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

coderabbitai bot commented Mar 19, 2026

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Shared validation/utilities (EMPTY_STRING_ARRAY, checkInt) were moved to RefreshableConfig; BizConfig centralizes interval TimeUnit via INTERVAL_TIME_UNIT and uses the shared empty-array constant; PortalConfig introduces named defaults, consolidates env-list parsing into getEnvSetProperty(), and removes its local checkInt.

Changes

Cohort / File(s) Summary
Shared config base
apollo-common/src/main/java/com/ctrip/framework/apollo/common/config/RefreshableConfig.java
Added protected static final String[] EMPTY_STRING_ARRAY and a public checkInt(int value, int min, int max, int defaultValue) validation helper.
Biz config updates
apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/config/BizConfig.java
Introduced INTERVAL_TIME_UNIT; multiple TimeUnit-returning methods now return it; replaced new String[0] with EMPTY_STRING_ARRAY; removed package-private checkInt(...).
Portal config refactor
apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/component/config/PortalConfig.java
Added named defaults for timeouts/pools/search limits; extracted env-list parsing into getEnvSetProperty() which returns an empty Set for null/zero-length arrays and trims/skips blanks; replaced new String[0] with EMPTY_STRING_ARRAY; removed private checkInt(...).
Docs
CHANGES.md
Added release-note entry about the refactor (PR #5583).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested labels

lgtm

Suggested reviewers

  • nobodyiam

Poem

🐰 I hopped through code with careful paws,
Gathered helpers, trimmed duplicate laws,
TimeUnits sing from a single tree,
Empty arrays shared — neat as can be,
A little rabbit cheers: refactor applause!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 8.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ 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 in Chinese accurately describes the main refactoring work: extracting constants and refactoring method implementations across multiple config classes (BizConfig, RefreshableConfig, PortalConfig).

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with 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.

❤️ Share

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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@apollo-common/src/main/java/com/ctrip/framework/apollo/common/config/RefreshableConfig.java`:
- Around line 145-146: The wrapped logger.warn call in RefreshableConfig (the
logger.warn(...) invocation) is failing Spotless; reformat the statement to
satisfy the project's Java style (or simply run the formatter) by running ./mvnw
spotless:apply before committing so the logger.warn invocation is reformatted
per rules and the Spotless check passes; ensure the modified logger.warn in
RefreshableConfig remains semantically identical (same arguments: value, min,
max, defaultValue).

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e1c2cb7d-6d40-414c-8bd1-955dc7502671

📥 Commits

Reviewing files that changed from the base of the PR and between 4c527a4 and 80ac585.

📒 Files selected for processing (3)
  • apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/config/BizConfig.java
  • apollo-common/src/main/java/com/ctrip/framework/apollo/common/config/RefreshableConfig.java
  • apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/component/config/PortalConfig.java

@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:M This PR changes 30-99 lines, ignoring generated files. labels Mar 19, 2026
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/component/config/PortalConfig.java`:
- Around line 142-152: getEnvSetProperty() and getEnvironments() need to ignore
empty/blank entries produced by getArrayProperty() (which splits on commas), so
trim each string and skip if it is empty before calling Env.valueOf(...) or
Env.addEnvironment(...); update the loops in getEnvSetProperty and
getEnvironments to do a String trimmed = env.trim(); if (trimmed.isEmpty())
continue; then use trimmed for the Env lookup/add to prevent
IllegalArgumentException on empty strings.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a6093ffe-239d-4760-a884-e63c9147502d

📥 Commits

Reviewing files that changed from the base of the PR and between 0d0e8c2 and eab6fdc.

📒 Files selected for processing (2)
  • apollo-common/src/main/java/com/ctrip/framework/apollo/common/config/RefreshableConfig.java
  • apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/component/config/PortalConfig.java
🚧 Files skipped from review as they are similar to previous changes (1)
  • apollo-common/src/main/java/com/ctrip/framework/apollo/common/config/RefreshableConfig.java

@nobodyiam nobodyiam force-pushed the refactor/config-classes branch from 15ec90d to 40528de Compare March 21, 2026 07:57
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.

这次关于环境配置空字符串处理的修复还没有覆盖完整,另外 CHANGES.md 的新增条目位置也需要调整。

请修复以下阻塞项后再更新:

  1. 补齐 portalSupportedEnvs() 对空白/空字符串项的过滤,和 getEnvSetProperty() 保持一致。
  2. 增加一个回归测试,覆盖连续逗号/前导逗号这类 apollo.portal.envs 配置。

另外还有一个小问题请一并调整:
3. CHANGES.md 里的 #5583 条目请加在当前 change list 的末位,不要加在开头。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants