Skip to content

Commit

Permalink
Bugfix: An instance of the subclass of RegistryConfig should be added…
Browse files Browse the repository at this point in the history
… to configsCache as the RegistryConfig class type(apache#15016)
  • Loading branch information
youjie_li committed Dec 24, 2024
1 parent a9e4910 commit 6ac903a
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,10 @@ public final <T extends AbstractConfig> T addConfig(AbstractConfig config) {
config.setScopeModel(scopeModel);
}

Class<? extends AbstractConfig> targetConfigType = getTargetConfigType(config.getClass());

Map<String, AbstractConfig> configsMap =
configsCache.computeIfAbsent(getTagName(config.getClass()), type -> new ConcurrentHashMap<>());
configsCache.computeIfAbsent(getTagName(targetConfigType), type -> new ConcurrentHashMap<>());

// fast check duplicated equivalent config before write lock
if (!(config instanceof ReferenceConfigBase || config instanceof ServiceConfigBase)) {
Expand All @@ -175,17 +177,21 @@ public final <T extends AbstractConfig> T addConfig(AbstractConfig config) {

// lock by config type
synchronized (configsMap) {
return (T) addIfAbsent(config, configsMap);
return (T) addIfAbsent(config, configsMap, targetConfigType);
}
}

protected boolean isSupportConfigType(Class<? extends AbstractConfig> type) {
return getTargetConfigType(type) != null;
}

protected Class<? extends AbstractConfig> getTargetConfigType(Class<? extends AbstractConfig> type) {
for (Class<? extends AbstractConfig> supportedConfigType : supportedConfigTypes) {
if (supportedConfigType.isAssignableFrom(type)) {
return true;
return supportedConfigType;
}
}
return false;
return null;
}

/**
Expand All @@ -196,7 +202,9 @@ protected boolean isSupportConfigType(Class<? extends AbstractConfig> type) {
* @return the existing equivalent config or the new adding config
* @throws IllegalStateException
*/
private <C extends AbstractConfig> C addIfAbsent(C config, Map<String, C> configsMap) throws IllegalStateException {
private <C extends AbstractConfig> C addIfAbsent(
C config, Map<String, C> configsMap, Class<? extends AbstractConfig> targetConfigType)
throws IllegalStateException {

if (config == null || configsMap == null) {
return config;
Expand All @@ -218,7 +226,7 @@ private <C extends AbstractConfig> C addIfAbsent(C config, Map<String, C> config

C existedConfig = configsMap.get(key);
if (existedConfig != null && !isEquals(existedConfig, config)) {
String type = config.getClass().getSimpleName();
String type = targetConfigType.getSimpleName();
logger.warn(
COMMON_UNEXPECTED_EXCEPTION,
"",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,21 @@ void testAddConfig() {
assertFalse(moduleConfigManager.getProviders().isEmpty());
}

@Test
void testAddCustomConfig() {
configManager.addConfig(new CustomRegistryConfig("CustomConfigManagerTest"));

assertTrue(configManager.getRegistry("CustomConfigManagerTest").isPresent());
}

static class CustomRegistryConfig extends RegistryConfig {

CustomRegistryConfig(String id) {
super();
this.setId(id);
}
}

@Test
void testRefreshAll() {
configManager.refreshAll();
Expand Down

0 comments on commit 6ac903a

Please sign in to comment.