fix: ConfigService.getConfig(appId, namespace) returns wrong app's config

[Shawyeok]

Problem

ConfigService.getConfig(appId, "some-namespace.yml") silently returns config belonging to the default application (app.id in app.properties) instead of the requested appId, with no error or warning.

The same bug affects any non-properties namespace format (YAML, YML, JSON, XML, TXT).

Root cause

DefaultConfigFactory.create(appId, namespace) correctly dispatches to createPropertiesCompatibleFileConfigRepository(appId, namespace, format) for non-properties namespaces, but that method then calls the two-arg ConfigService.getConfigFile(namespace, format) — silently dropping appId:

// DefaultConfigFactory.java (before fix)
PropertiesCompatibleFileConfigRepository createPropertiesCompatibleFileConfigRepository(
    String appId, String namespace, ConfigFileFormat format) {
  String actualNamespaceName = trimNamespaceFormat(namespace, format);
  PropertiesCompatibleConfigFile configFile = (PropertiesCompatibleConfigFile) ConfigService
      .getConfigFile(actualNamespaceName, format);   // ← appId silently dropped
  return new PropertiesCompatibleFileConfigRepository(configFile);
}

ConfigManager already had a correct three-arg getConfigFile(appId, namespace, format) overload, but it was not exposed on ConfigService.

Fix

ConfigService.java — add the missing overload that mirrors the existing getConfig(appId, namespace) pattern:

public static ConfigFile getConfigFile(String appId, String namespace,
    ConfigFileFormat configFileFormat) {
  return s_instance.getManager().getConfigFile(appId, namespace, configFileFormat);
}

DefaultConfigFactory.java — use the new overload so appId is preserved:

configFile = (PropertiesCompatibleConfigFile) ConfigService
    .getConfigFile(appId, actualNamespaceName, format);   // ← appId preserved

Tests

| Test | What it verifies |
|------|-----------------|| ConfigServiceTest.testGetConfigWithCustomAppId | ConfigService.getConfig(appId, namespace) returns a Config whose property values reflect the requested appId, not the default |
| ConfigServiceTest.testGetConfigFileWithCustomAppId | ConfigService.getConfigFile(appId, ns, format) returns a ConfigFile whose getAppId() matches the requested appId |

Affected versions

Reproducible on apollo-client 2.4.0 and 2.5.0. ConfigFileFormat.Properties namespaces are unaffected (they go through createConfigRepository, not createPropertiesCompatibleFileConfigRepository).

Summary by CodeRabbit

• New Features

  • Public API added to retrieve configuration files by specifying a custom application ID, namespace, and file format; config-file resolution now respects the provided app ID.

• Tests

  • Added tests validating config and config-file retrieval with a custom application ID.
  • Improved integration test stability by adding a wait-for-namespace helper and updating assertions to wait for namespace availability.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds a public ConfigService.getConfigFile(appId, namespace, configFileFormat) overload, updates DefaultConfigFactory to pass appId when resolving config files, adds tests validating custom appId for Config and ConfigFile lookups, and adds a probe helper to wait for namespaces.

Changes

AppId-aware ConfigFile retrieval

Layer / File(s)	Summary
ConfigService getConfigFile overload with appId apollo-client/src/main/java/com/ctrip/framework/apollo/ConfigService.java	Adds a new public static method that accepts appId, namespace, and configFileFormat parameters and delegates to the manager for ConfigFile retrieval.
Factory wiring, tests, and probe wait helper apollo-client/src/main/java/com/ctrip/framework/apollo/spi/DefaultConfigFactory.java, apollo-client/src/test/java/com/ctrip/framework/apollo/ConfigServiceTest.java, apollo-compat-tests/apollo-spring-compat-it/src/test/java/com/ctrip/framework/apollo/compat/spring/SpringApolloEventListenerProbe.java, apollo-compat-tests/apollo-spring-compat-it/src/test/java/com/ctrip/framework/apollo/compat/spring/SpringAnnotationCompatibilityTest.java	DefaultConfigFactory.createPropertiesCompatibleFileConfigRepository now passes appId into ConfigService.getConfigFile. Two new tests verify ConfigService.getConfig(customAppId, namespace) and getConfigFile(customAppId, namespace, format) behavior. SpringApolloEventListenerProbe adds waitForNamespace(...) and a test replaces a direct namespace equality assertion with a boolean wait-and-assert.

Sequence Diagram

sequenceDiagram
  participant DefaultConfigFactory
  participant ConfigService
  participant ConfigManager
  DefaultConfigFactory->>ConfigService: getConfigFile(appId, namespace, format)
  ConfigService->>ConfigManager: getConfigFile(appId, namespace, format)
  ConfigManager-->>ConfigService: ConfigFile
  ConfigService-->>DefaultConfigFactory: ConfigFile

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• apolloconfig/apollo-java#70: Introduces the multi-appId API refactor that overlaps with adding the ConfigService.getConfigFile(appId, namespace, format) overload and related call-site updates.

Suggested reviewers

• nobodyiam

Poem

🐰 A rabbit hops through config files with glee,
appId leads the hunt through namespace and tree,
Factory calls out, Service answers the call,
Tests wait and confirm — the lookup finds all,
Little hops, big certainty.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 10.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 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 bug being fixed: that ConfigService.getConfig(appId, namespace) was returning configuration from the wrong application instead of the requested appId.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 71.17%. Comparing base (d4b76f8) to head (f6110d1).
⚠️ Report is 27 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main     #140      +/-   ##
============================================
+ Coverage     68.68%   71.17%   +2.48%     
- Complexity     1503     1640     +137     
============================================
  Files           212      224      +12     
  Lines          6396     6733     +337     
  Branches        647      680      +33     
============================================
+ Hits           4393     4792     +399     
+ Misses         1673     1592      -81     
- Partials        330      349      +19     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[coderabbitai]

Actionable comments posted: 1

🤖 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-client/src/main/java/com/ctrip/framework/apollo/ConfigService.java`:
- Around line 104-115: Update the Javadoc for ConfigService.getConfigFile:
clarify that the namespace parameter should NOT include a file extension (e.g.,
use "application" not "application.yml") because the extension is derived from
the configFileFormat and appended internally by DefaultConfigManager; reference
ConfigService.getConfigFile, the namespace parameter, and ConfigFileFormat (and
DefaultConfigManager behavior) in the comment so callers know to pass just the
base namespace name.

🪄 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: af07b96b-693d-4836-af0e-931a7384e731

📥 Commits

Reviewing files that changed from the base of the PR and between a9a6edc and 2a1096e.

📒 Files selected for processing (3)

• apollo-client/src/main/java/com/ctrip/framework/apollo/ConfigService.java
• apollo-client/src/main/java/com/ctrip/framework/apollo/spi/DefaultConfigFactory.java
• apollo-client/src/test/java/com/ctrip/framework/apollo/ConfigServiceTest.java

[coderabbitai]

🧹 Nitpick comments (1)

apollo-compat-tests/apollo-spring-compat-it/src/test/java/com/ctrip/framework/apollo/compat/spring/SpringApolloEventListenerProbe.java (1)

41-55: Discarded namespaces aren’t a reliability problem in the current test
waitForNamespace(...) consumes the namespaces queue via poll(...) and discards non-matching values, but in this module the SpringApolloEventListenerProbe is only consumed once (waitForNamespace("application", ...) in SpringAnnotationCompatibilityTest), and there are no other usages of the probe (no other waitForNamespace/pollNamespace calls). Discarding won’t affect later assertions here.
If you later reuse the same probe instance to wait for multiple different namespaces, consider a non-destructive approach (e.g., retain unmatched values).

🤖 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-compat-tests/apollo-spring-compat-it/src/test/java/com/ctrip/framework/apollo/compat/spring/SpringApolloEventListenerProbe.java`
around lines 41 - 55, waitForNamespace in SpringApolloEventListenerProbe
currently polls and discards non-matching entries from the namespaces queue,
which may lose events if the probe is reused; change the logic in
waitForNamespace (and any helper like namespaces queue handling) to be
non-destructive: peek the head and only poll when it matches expectedNamespace
(or temporarily buffer unmatched values and requeue them if needed) so unmatched
namespace values are retained for subsequent waits (reference waitForNamespace,
namespaces, and SpringApolloEventListenerProbe to locate the code; consider
SpringAnnotationCompatibilityTest which calls waitForNamespace("application",
...)).

🤖 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.

Nitpick comments:
In
`@apollo-compat-tests/apollo-spring-compat-it/src/test/java/com/ctrip/framework/apollo/compat/spring/SpringApolloEventListenerProbe.java`:
- Around line 41-55: waitForNamespace in SpringApolloEventListenerProbe
currently polls and discards non-matching entries from the namespaces queue,
which may lose events if the probe is reused; change the logic in
waitForNamespace (and any helper like namespaces queue handling) to be
non-destructive: peek the head and only poll when it matches expectedNamespace
(or temporarily buffer unmatched values and requeue them if needed) so unmatched
namespace values are retained for subsequent waits (reference waitForNamespace,
namespaces, and SpringApolloEventListenerProbe to locate the code; consider
SpringAnnotationCompatibilityTest which calls waitForNamespace("application",
...)).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 85763920-8d5b-434f-9070-533887030288

📥 Commits

Reviewing files that changed from the base of the PR and between c7d2dac and 6261c1e.

📒 Files selected for processing (2)

• apollo-compat-tests/apollo-spring-compat-it/src/test/java/com/ctrip/framework/apollo/compat/spring/SpringAnnotationCompatibilityTest.java
• apollo-compat-tests/apollo-spring-compat-it/src/test/java/com/ctrip/framework/apollo/compat/spring/SpringApolloEventListenerProbe.java

[nobodyiam]

Thanks for the fix. I checked the latest head (f6110d1faffe7e382778bbfc8c15156527694e05): the non-properties path now keeps the custom appId by calling ConfigService.getConfigFile(appId, actualNamespaceName, format), and the new overload preserves the existing two-argument API. The added tests cover both custom-app config and config-file lookup, and the namespace Javadoc example now correctly omits the file extension.

I do not see a blocking issue from this pass. A maintainer can do the final approval/merge decision.

Note: this reply was generated and posted automatically by AI for initial triage; a maintainer will follow up if needed.