From c2e0a56fbc349716b1b763c125b18a5f798d6442 Mon Sep 17 00:00:00 2001 From: Haoran Meng Date: Fri, 4 Sep 2020 16:20:34 +0800 Subject: [PATCH] Fixes persist local configurations to config center when overwrite=false (#7247) * Fixes persist local configurations to config center * Fixes persist local configurations to config center --- .../governance/core/config/ConfigCenter.java | 25 +++++++-------- .../core/config/ConfigCenterTest.java | 32 +++++++++---------- 2 files changed, 28 insertions(+), 29 deletions(-) diff --git a/shardingsphere-governance/shardingsphere-governance-core/shardingsphere-governance-core-config/src/main/java/org/apache/shardingsphere/governance/core/config/ConfigCenter.java b/shardingsphere-governance/shardingsphere-governance-core/shardingsphere-governance-core-config/src/main/java/org/apache/shardingsphere/governance/core/config/ConfigCenter.java index 3ef1c2d0b9..029edfb9b9 100644 --- a/shardingsphere-governance/shardingsphere-governance-core/shardingsphere-governance-core-config/src/main/java/org/apache/shardingsphere/governance/core/config/ConfigCenter.java +++ b/shardingsphere-governance/shardingsphere-governance-core/shardingsphere-governance-core-config/src/main/java/org/apache/shardingsphere/governance/core/config/ConfigCenter.java @@ -84,7 +84,7 @@ public final class ConfigCenter { persistDataSourceConfigurations(schemaName, dataSourceConfigs, isOverwrite); persistRuleConfigurations(schemaName, ruleConfigurations, isOverwrite); // TODO Consider removing the following one. - persistSchemaName(schemaName, isOverwrite); + persistSchemaName(schemaName); } /** @@ -129,10 +129,9 @@ public final class ConfigCenter { } private void persistDataSourceConfigurations(final String schemaName, final Map dataSourceConfigurations, final boolean isOverwrite) { - if (dataSourceConfigurations.isEmpty() || !isOverwrite) { - return; + if (!dataSourceConfigurations.isEmpty() && (isOverwrite || !hasDataSourceConfiguration(schemaName))) { + persistDataSourceConfigurations(schemaName, dataSourceConfigurations); } - persistDataSourceConfigurations(schemaName, dataSourceConfigurations); } private void persistDataSourceConfigurations(final String schemaName, final Map dataSourceConfigurations) { @@ -143,10 +142,9 @@ public final class ConfigCenter { } private void persistRuleConfigurations(final String schemaName, final Collection ruleConfigurations, final boolean isOverwrite) { - if (ruleConfigurations.isEmpty() || !isOverwrite) { - return; + if (!ruleConfigurations.isEmpty() && (isOverwrite || !hasRuleConfiguration(schemaName))) { + persistRuleConfigurations(schemaName, ruleConfigurations); } - persistRuleConfigurations(schemaName, ruleConfigurations); } private void persistRuleConfigurations(final String schemaName, final Collection ruleConfigurations) { @@ -200,21 +198,22 @@ public final class ConfigCenter { } private void persistAuthentication(final Authentication authentication, final boolean isOverwrite) { - if (null != authentication && isOverwrite) { + if (null != authentication && (isOverwrite || !hasAuthentication())) { repository.persist(node.getAuthenticationPath(), YamlEngine.marshal(new AuthenticationYamlSwapper().swapToYamlConfiguration(authentication))); } } private void persistProperties(final Properties props, final boolean isOverwrite) { - if (!props.isEmpty() && isOverwrite) { + if (!props.isEmpty() && (isOverwrite || !hasProperties())) { repository.persist(node.getPropsPath(), YamlEngine.marshal(props)); } } - private void persistSchemaName(final String schemaName, final boolean isOverwrite) { - if (!isOverwrite) { - return; - } + private boolean hasProperties() { + return !Strings.isNullOrEmpty(repository.get(node.getPropsPath())); + } + + private void persistSchemaName(final String schemaName) { String schemaNames = repository.get(node.getSchemaPath()); if (Strings.isNullOrEmpty(schemaNames)) { repository.persist(node.getSchemaPath(), schemaName); diff --git a/shardingsphere-governance/shardingsphere-governance-core/shardingsphere-governance-core-config/src/test/java/org/apache/shardingsphere/governance/core/config/ConfigCenterTest.java b/shardingsphere-governance/shardingsphere-governance-core/shardingsphere-governance-core-config/src/test/java/org/apache/shardingsphere/governance/core/config/ConfigCenterTest.java index 0d50e353af..2da58c116c 100644 --- a/shardingsphere-governance/shardingsphere-governance-core/shardingsphere-governance-core-config/src/test/java/org/apache/shardingsphere/governance/core/config/ConfigCenterTest.java +++ b/shardingsphere-governance/shardingsphere-governance-core/shardingsphere-governance-core-config/src/test/java/org/apache/shardingsphere/governance/core/config/ConfigCenterTest.java @@ -92,8 +92,8 @@ public final class ConfigCenterTest { public void assertPersistConfigurationForShardingRuleWithoutAuthenticationAndIsNotOverwriteAndConfigurationIsExisted() { ConfigCenter configurationService = new ConfigCenter(configurationRepository); configurationService.persistConfigurations("sharding_db", createDataSourceConfigurations(), createRuleConfigurations(), false); - verify(configurationRepository, times(0)).persist(eq("/config/schema/sharding_db/datasource"), ArgumentMatchers.any()); - verify(configurationRepository, times(0)).persist("/config/schema/sharding_db/rule", readYAML(SHARDING_RULE_YAML)); + verify(configurationRepository).persist(eq("/config/schema/sharding_db/datasource"), ArgumentMatchers.any()); + verify(configurationRepository).persist(eq("/config/schema/sharding_db/rule"), ArgumentMatchers.any()); } @Test @@ -114,8 +114,8 @@ public final class ConfigCenterTest { public void assertPersistConfigurationForShardingRuleWithoutAuthenticationAndIsNotOverwriteAndConfigurationIsNotExisted() { ConfigCenter configurationService = new ConfigCenter(configurationRepository); configurationService.persistConfigurations("sharding_db", createDataSourceConfigurations(), createRuleConfigurations(), false); - verify(configurationRepository, times(0)).persist(eq("/config/schema/sharding_db/datasource"), ArgumentMatchers.any()); - verify(configurationRepository, times(0)).persist("/config/schema/sharding_db/rule", readYAML(SHARDING_RULE_YAML)); + verify(configurationRepository).persist(eq("/config/schema/sharding_db/datasource"), ArgumentMatchers.any()); + verify(configurationRepository).persist(eq("/config/schema/sharding_db/rule"), ArgumentMatchers.any()); } @@ -131,16 +131,16 @@ public final class ConfigCenterTest { public void assertPersistConfigurationForMasterSlaveRuleWithoutAuthenticationAndIsNotOverwriteAndConfigurationIsExisted() { ConfigCenter configurationService = new ConfigCenter(configurationRepository); configurationService.persistConfigurations("sharding_db", createDataSourceConfigurations(), createMasterSlaveRuleConfiguration(), false); - verify(configurationRepository, times(0)).persist(eq("/config/schema/sharding_db/datasource"), ArgumentMatchers.any()); - verify(configurationRepository, times(0)).persist("/config/schema/sharding_db/rule", readYAML(MASTER_SLAVE_RULE_YAML)); + verify(configurationRepository).persist(eq("/config/schema/sharding_db/datasource"), ArgumentMatchers.any()); + verify(configurationRepository).persist(eq("/config/schema/sharding_db/rule"), ArgumentMatchers.any()); } @Test public void assertPersistConfigurationForMasterSlaveRuleWithoutAuthenticationAndIsNotOverwriteAndConfigurationIsNotExisted() { ConfigCenter configurationService = new ConfigCenter(configurationRepository); configurationService.persistConfigurations("sharding_db", createDataSourceConfigurations(), createMasterSlaveRuleConfiguration(), false); - verify(configurationRepository, times(0)).persist(eq("/config/schema/sharding_db/datasource"), ArgumentMatchers.any()); - verify(configurationRepository, times(0)).persist("/config/schema/sharding_db/rule", readYAML(MASTER_SLAVE_RULE_YAML)); + verify(configurationRepository).persist(eq("/config/schema/sharding_db/datasource"), ArgumentMatchers.any()); + verify(configurationRepository).persist(eq("/config/schema/sharding_db/rule"), ArgumentMatchers.any()); } @Test @@ -155,16 +155,16 @@ public final class ConfigCenterTest { public void assertPersistConfigurationForShardingRuleWithAuthenticationAndIsNotOverwriteAndConfigurationIsExisted() { ConfigCenter configurationService = new ConfigCenter(configurationRepository); configurationService.persistConfigurations("sharding_db", createDataSourceConfigurations(), createRuleConfigurations(), false); - verify(configurationRepository, times(0)).persist(eq("/config/schema/sharding_db/datasource"), ArgumentMatchers.any()); - verify(configurationRepository, times(0)).persist("/config/schema/sharding_db/rule", readYAML(SHARDING_RULE_YAML)); + verify(configurationRepository).persist(eq("/config/schema/sharding_db/datasource"), ArgumentMatchers.any()); + verify(configurationRepository).persist(eq("/config/schema/sharding_db/rule"), ArgumentMatchers.any()); } @Test public void assertPersistConfigurationForShardingRuleWithAuthenticationAndIsNotOverwriteAndConfigurationIsNotExisted() { ConfigCenter configurationService = new ConfigCenter(configurationRepository); configurationService.persistConfigurations("sharding_db", createDataSourceConfigurations(), createRuleConfigurations(), false); - verify(configurationRepository, times(0)).persist(eq("/config/schema/sharding_db/datasource"), ArgumentMatchers.any()); - verify(configurationRepository, times(0)).persist("/config/schema/sharding_db/rule", readYAML(SHARDING_RULE_YAML)); + verify(configurationRepository).persist(eq("/config/schema/sharding_db/datasource"), ArgumentMatchers.any()); + verify(configurationRepository).persist(eq("/config/schema/sharding_db/rule"), ArgumentMatchers.any()); } @Test @@ -179,8 +179,8 @@ public final class ConfigCenterTest { public void assertPersistConfigurationForMasterSlaveRuleWithAuthenticationAndIsNotOverwriteAndConfigurationIsExisted() { ConfigCenter configurationService = new ConfigCenter(configurationRepository); configurationService.persistConfigurations("sharding_db", createDataSourceConfigurations(), createMasterSlaveRuleConfiguration(), false); - verify(configurationRepository, times(0)).persist(eq("/config/schema/sharding_db/datasource"), ArgumentMatchers.any()); - verify(configurationRepository, times(0)).persist("/config/schema/sharding_db/rule", readYAML(MASTER_SLAVE_RULE_YAML)); + verify(configurationRepository).persist(eq("/config/schema/sharding_db/datasource"), ArgumentMatchers.any()); + verify(configurationRepository).persist(eq("/config/schema/sharding_db/rule"), ArgumentMatchers.any()); } @Test @@ -188,8 +188,8 @@ public final class ConfigCenterTest { ConfigCenter configurationService = new ConfigCenter(configurationRepository); configurationService.persistConfigurations("sharding_db", createDataSourceConfigurations(), createMasterSlaveRuleConfiguration(), false); - verify(configurationRepository, times(0)).persist(eq("/config/schema/sharding_db/datasource"), ArgumentMatchers.any()); - verify(configurationRepository, times(0)).persist("/config/schema/sharding_db/rule", readYAML(MASTER_SLAVE_RULE_YAML)); + verify(configurationRepository).persist(eq("/config/schema/sharding_db/datasource"), ArgumentMatchers.any()); + verify(configurationRepository).persist(eq("/config/schema/sharding_db/rule"), ArgumentMatchers.any()); } @Test -- GitLab