fix(config): honor OVERRIDE_IF_ABSENT for nested properties by bajiaoshan · Pull Request #16514 · apache/dubbo · GitHub
Skip to content

fix(config): honor OVERRIDE_IF_ABSENT for nested properties - #16514

Open
bajiaoshan wants to merge 2 commits into
apache:3.3from
bajiaoshan:codex/fix-nested-config-refresh
Open

bajiaoshan wants to merge 2 commits into
apache:3.3from
bajiaoshan:codex/fix-nested-config-refresh

Conversation

@bajiaoshan

@bajiaoshan bajiaoshan commented Oct 18, 2026 •

Copy link
Copy Markdown

What is the purpose of the change?

Refreshing a configuration in OVERRIDE_IF_ABSENT mode currently preserves top-level properties but overwrites already configured nested properties. For example, an explicitly disabled metrics aggregation becomes enabled when an external configuration sets dubbo.metrics.aggregation.enabled=true.

Reuse an existing nested configuration instance during refresh and check each property's current value against its actual owning object. Instantiate a nested configuration only when it is missing. This preserves configured nested values while continuing to fill absent values and apply external overrides in the other tested modes.

Add nine parameterized regression cases using real metrics configuration classes, including two nesting levels, an explicitly configured false, and previously absent nested objects. Also verify that a setter-only nested configuration remains supported when no getter exists. Before the fix, the two OVERRIDE_IF_ABSENT cases fail; after the fix, all ten cases pass.

Fixes #16513.

Validation

  • Copying the same ten-case regression test into an isolated worktree at base commit ae809de2fc81b408194240965eb20bf27e6971df, with production sources unchanged, produces two assertion failures, both in OVERRIDE_IF_ABSENT mode. All ten pass with the fix.
  • The latest local JaCoCo run passes 81 selected configuration-related tests in dubbo-common, including the ten regression/compatibility cases. The 48 selected dubbo-config-api tests and the 51-module dependency reactor previously passed; the follow-up commit changes tests only.
  • Checkstyle reports zero violations; Spotless and git diff --check pass. The Unix line-ending check is skipped on Windows.
  • The selected unit tests run with -DenableEmbeddedZookeeper=false; no external registry is required. Full-project and external-service integration tests have not been run locally.
  • Both the initial upstream run and the latest upstream GitHub Actions run completed successfully for the submitted commits.

Coverage

The original Codecov partial line was the conditional lookup of the nested property's getter. The added setter-only compatibility test exercises the missing-getter path and verifies that the initialized nested object receives its external configuration value. Local JaCoCo confirms that all six executable lines changed in AbstractConfig.java are now fully covered, including both branches of that conditional (previously five of six, or 83.33%). The latest Codecov report confirms that all modified and coverable lines are covered by tests; the unit-test reports show 100% patch coverage.

Checklist

  • Link the corresponding GitHub issue.
  • Explain the problem, root cause, and resulting behavior.
  • Add regression tests and verify that they fail before the production fix and pass afterward.
  • Verify the upstream GitHub Actions results after submission.

@codecov-commenter

codecov-commenter commented Oct 18, 2026 •

Copy link
Copy Markdown

@LI123456mo LI123456mo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reusing the existing nested instance in assignProperties and passing obj into isPropertySet addresses both causes described in the issue, and the added tests fail before the change and pass after.

LGTM @EarthChen @RainYuY

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Nested configuration properties ignore OVERRIDE_IF_ABSENT during refresh

3 participants