From ee6aafac4ad444e743ca3238a95bfd7b48b5b554 Mon Sep 17 00:00:00 2001 From: Yike Xiao Date: Tue, 9 Jun 2026 16:06:46 +0800 Subject: [PATCH 1/2] fix: appId dropped when creating PropertiesCompatibleFileConfigRepository for non-default appId MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DefaultConfigFactory.createPropertiesCompatibleFileConfigRepository() received an appId parameter but called ConfigService.getConfigFile(namespace, format) — the two-arg overload that ignores appId and resolves against the default app.id from app.properties. Fixes the bug by: 1. Adding ConfigService.getConfigFile(appId, namespace, format) that delegates to the already-correct ConfigManager.getConfigFile(appId, namespace, format). 2. Updating DefaultConfigFactory to call the new three-arg overload so the caller-specified appId is preserved. Adds tests: - DefaultConfigFactoryTest.testCreatePropertiesCompatibleFileConfigRepositoryForwardsCustomAppId: verifies ConfigManager is invoked with the supplied appId, never the default. - ConfigServiceTest.testGetConfigFileWithCustomAppId: verifies the new ConfigService.getConfigFile(appId, ns, format) overload returns a ConfigFile whose getAppId() equals the requested appId. Co-Authored-By: Claude Sonnet 4.6 --- .../ctrip/framework/apollo/ConfigService.java | 13 +++++++++ .../apollo/spi/DefaultConfigFactory.java | 2 +- .../framework/apollo/ConfigServiceTest.java | 28 +++++++++++++++++++ 3 files changed, 42 insertions(+), 1 deletion(-) diff --git a/apollo-client/src/main/java/com/ctrip/framework/apollo/ConfigService.java b/apollo-client/src/main/java/com/ctrip/framework/apollo/ConfigService.java index 2886823b..97127f43 100644 --- a/apollo-client/src/main/java/com/ctrip/framework/apollo/ConfigService.java +++ b/apollo-client/src/main/java/com/ctrip/framework/apollo/ConfigService.java @@ -101,6 +101,19 @@ public static ConfigFile getConfigFile(String namespace, ConfigFileFormat config return s_instance.getManager().getConfigFile(namespace, configFileFormat); } + /** + * Get the config file instance for the appId and namespace. + * + * @param appId the appId of the config + * @param namespace the namespace of the config without file extension, e.g. "application" + * @param configFileFormat the config file format + * @return config file instance + */ + public static ConfigFile getConfigFile(String appId, String namespace, + ConfigFileFormat configFileFormat) { + return s_instance.getManager().getConfigFile(appId, namespace, configFileFormat); + } + public static ConfigMonitor getConfigMonitor(){ return s_instance.getMonitor(); } diff --git a/apollo-client/src/main/java/com/ctrip/framework/apollo/spi/DefaultConfigFactory.java b/apollo-client/src/main/java/com/ctrip/framework/apollo/spi/DefaultConfigFactory.java index 6f896892..9e392810 100644 --- a/apollo-client/src/main/java/com/ctrip/framework/apollo/spi/DefaultConfigFactory.java +++ b/apollo-client/src/main/java/com/ctrip/framework/apollo/spi/DefaultConfigFactory.java @@ -163,7 +163,7 @@ PropertiesCompatibleFileConfigRepository createPropertiesCompatibleFileConfigRep String appId, String namespace, ConfigFileFormat format) { String actualNamespaceName = trimNamespaceFormat(namespace, format); PropertiesCompatibleConfigFile configFile = (PropertiesCompatibleConfigFile) ConfigService - .getConfigFile(actualNamespaceName, format); + .getConfigFile(appId, actualNamespaceName, format); return new PropertiesCompatibleFileConfigRepository(configFile); } diff --git a/apollo-client/src/test/java/com/ctrip/framework/apollo/ConfigServiceTest.java b/apollo-client/src/test/java/com/ctrip/framework/apollo/ConfigServiceTest.java index 65f0c2f4..a5a4d70d 100644 --- a/apollo-client/src/test/java/com/ctrip/framework/apollo/ConfigServiceTest.java +++ b/apollo-client/src/test/java/com/ctrip/framework/apollo/ConfigServiceTest.java @@ -103,6 +103,34 @@ public void testMockConfigFactoryForConfigFile() throws Exception { assertEquals(someNamespaceFileName + ":" + someConfigFileFormat.getValue(), configFile.getContent()); } + @Test + public void testGetConfigWithCustomAppId() throws Exception { + String customAppId = "customAppId"; + String someNamespace = "mock"; + String someKey = "someKey"; + MockInjector.setInstance(ConfigFactory.class, someNamespace, new MockConfigFactory()); + + Config config = ConfigService.getConfig(customAppId, someNamespace); + + assertEquals(customAppId + ConfigConsts.CLUSTER_NAMESPACE_SEPARATOR + someNamespace + ":" + someKey, + config.getProperty(someKey, null)); + } + + @Test + public void testGetConfigFileWithCustomAppId() throws Exception { + String customAppId = "customAppId"; + String someNamespace = "mock"; + ConfigFileFormat someConfigFileFormat = ConfigFileFormat.YML; + String someNamespaceFileName = + String.format("%s.%s", someNamespace, someConfigFileFormat.getValue()); + MockInjector.setInstance(ConfigFactory.class, someNamespaceFileName, new MockConfigFactory()); + + ConfigFile configFile = ConfigService.getConfigFile(customAppId, someNamespace, someConfigFileFormat); + + assertEquals(customAppId, configFile.getAppId()); + assertEquals(someNamespaceFileName, configFile.getNamespace()); + } + private static class MockConfig extends AbstractConfig { private final String m_appId; private final String m_namespace; From 25b8bde7070b66d4e501d6967d07ea7de968cba7 Mon Sep 17 00:00:00 2001 From: Yike Xiao Date: Sun, 21 Jun 2026 11:24:09 +0800 Subject: [PATCH 2/2] test: add regression test for custom appId on properties-compatible namespace Add ConfigServiceTest.testGetConfigWithCustomAppIdForPropertiesCompatibleNamespace, which drives the real DefaultConfigFactory path (create -> createPropertiesCompatibleFileConfigRepository -> ConfigService.getConfigFile(appId, namespace, format)) for a .yml namespace. The existing custom-appId tests either used a properties namespace or called the new getConfigFile overload directly, so neither would catch DefaultConfigFactory.createPropertiesCompatibleFileConfigRepository dropping the custom appId again. The new test stubs only createConfigFile and echoes the received appId into the resulting Config, so it fails if the appId is dropped. Co-Authored-By: Claude Opus 4.8 --- .../framework/apollo/ConfigServiceTest.java | 89 +++++++++++++++++++ 1 file changed, 89 insertions(+) diff --git a/apollo-client/src/test/java/com/ctrip/framework/apollo/ConfigServiceTest.java b/apollo-client/src/test/java/com/ctrip/framework/apollo/ConfigServiceTest.java index a5a4d70d..09c3e4a8 100644 --- a/apollo-client/src/test/java/com/ctrip/framework/apollo/ConfigServiceTest.java +++ b/apollo-client/src/test/java/com/ctrip/framework/apollo/ConfigServiceTest.java @@ -20,6 +20,8 @@ import com.ctrip.framework.apollo.core.MetaDomainConsts; import com.ctrip.framework.apollo.enums.ConfigSourceType; +import com.ctrip.framework.apollo.spi.DefaultConfigFactory; +import java.util.Properties; import java.util.Set; import org.junit.After; @@ -131,6 +133,32 @@ public void testGetConfigFileWithCustomAppId() throws Exception { assertEquals(someNamespaceFileName, configFile.getNamespace()); } + @Test + public void testGetConfigWithCustomAppIdForPropertiesCompatibleNamespace() throws Exception { + String customAppId = "customAppId"; + String someNamespace = "mock"; + ConfigFileFormat someConfigFileFormat = ConfigFileFormat.YML; + String someNamespaceFileName = + String.format("%s.%s", someNamespace, someConfigFileFormat.getValue()); + + // Exercise the real DefaultConfigFactory path for a non-properties namespace, i.e. + // create(appId, "mock.yml") -> createPropertiesCompatibleFileConfigRepository(...) -> + // ConfigService.getConfigFile(appId, "mock", YML). Only createConfigFile is stubbed (to avoid + // hitting a remote repository); it echoes the appId it receives into the resulting properties so + // the assertion below fails if DefaultConfigFactory ever drops the custom appId again. + MockInjector.setInstance(ConfigFactory.class, someNamespaceFileName, new DefaultConfigFactory() { + @Override + public ConfigFile createConfigFile(String appId, String namespace, + ConfigFileFormat configFileFormat) { + return new MockPropertiesCompatibleConfigFile(appId, namespace, configFileFormat); + } + }); + + Config config = ConfigService.getConfig(customAppId, someNamespaceFileName); + + assertEquals(customAppId, config.getProperty("appId", null)); + } + private static class MockConfig extends AbstractConfig { private final String m_appId; private final String m_namespace; @@ -241,6 +269,67 @@ public ConfigFile createConfigFile(String appId, String namespace, ConfigFileFor } } + private static class MockPropertiesCompatibleConfigFile implements PropertiesCompatibleConfigFile { + private final String m_appId; + private final String m_namespace; + private final ConfigFileFormat m_configFileFormat; + + public MockPropertiesCompatibleConfigFile(String appId, String namespace, + ConfigFileFormat configFileFormat) { + m_appId = appId; + m_namespace = namespace; + m_configFileFormat = configFileFormat; + } + + @Override + public Properties asProperties() { + Properties properties = new Properties(); + // echo the appId so it is observable through the resulting Config + properties.setProperty("appId", m_appId); + return properties; + } + + @Override + public String getContent() { + return null; + } + + @Override + public boolean hasContent() { + return true; + } + + @Override + public String getAppId() { + return m_appId; + } + + @Override + public String getNamespace() { + return m_namespace; + } + + @Override + public ConfigFileFormat getConfigFileFormat() { + return m_configFileFormat; + } + + @Override + public void addChangeListener(ConfigFileChangeListener listener) { + + } + + @Override + public boolean removeChangeListener(ConfigFileChangeListener listener) { + return false; + } + + @Override + public ConfigSourceType getSourceType() { + return ConfigSourceType.REMOTE; + } + } + public static class MockConfigUtil extends ConfigUtil { @Override public String getAppId() {