This is an automated email from the ASF dual-hosted git repository.
dhemery pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/develop by this push:
new 6ed908b GEODE-7568: Pass persistence service to config mgr
constructors (#4462)
6ed908b is described below
commit 6ed908bdbc63c5e4d6de12089f87b5e594cb4f30
Author: Dale Emery <[email protected]>
AuthorDate: Wed Dec 11 09:34:04 2019 -0800
GEODE-7568: Pass persistence service to config mgr constructors (#4462)
* GEODE-7568: Pass persistence service to config mgr constructors
Pass the persistence service to each manager on construction, rather
than passing it to each method.
Co-authored-by: Dale Emery <[email protected]>
* Remove spurious commented code
---
.../RegionConfigMutatorIntegrationTest.java | 2 +-
.../api/LocatorClusterManagementService.java | 16 ++++-----
.../mutators/CacheConfigurationManager.java | 40 ++++++++++++----------
.../mutators/ConfigurationManager.java | 12 +++----
.../configuration/mutators/DeploymentManager.java | 20 ++++++-----
.../mutators/GatewayReceiverConfigManager.java | 8 +++--
.../configuration/mutators/IndexConfigManager.java | 5 +++
.../configuration/mutators/PdxManager.java | 5 +++
.../mutators/RegionConfigManager.java | 5 +++
.../api/LocatorClusterManagementServiceTest.java | 16 ++++-----
.../mutators/IndexConfigManagerTest.java | 2 +-
.../configuration/mutators/PdxManagerTest.java | 5 ++-
.../mutators/RegionConfigManagerTest.java | 2 +-
.../validators/MemberValidatorTest.java | 2 +-
14 files changed, 82 insertions(+), 58 deletions(-)
diff --git
a/geode-core/src/integrationTest/java/org/apache/geode/management/internal/configuration/mutators/RegionConfigMutatorIntegrationTest.java
b/geode-core/src/integrationTest/java/org/apache/geode/management/internal/configuration/mutators/RegionConfigMutatorIntegrationTest.java
index ff44cb5..969404e 100644
---
a/geode-core/src/integrationTest/java/org/apache/geode/management/internal/configuration/mutators/RegionConfigMutatorIntegrationTest.java
+++
b/geode-core/src/integrationTest/java/org/apache/geode/management/internal/configuration/mutators/RegionConfigMutatorIntegrationTest.java
@@ -37,7 +37,7 @@ public class RegionConfigMutatorIntegrationTest {
@Before
public void before() throws Exception {
config = new Region();
- mutator = new RegionConfigManager();
+ mutator = new RegionConfigManager(null);
}
@Test
diff --git
a/geode-core/src/main/java/org/apache/geode/management/internal/api/LocatorClusterManagementService.java
b/geode-core/src/main/java/org/apache/geode/management/internal/api/LocatorClusterManagementService.java
index 771323b..9516150 100644
---
a/geode-core/src/main/java/org/apache/geode/management/internal/api/LocatorClusterManagementService.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/api/LocatorClusterManagementService.java
@@ -106,11 +106,11 @@ public class LocatorClusterManagementService implements
ClusterManagementService
new MemberValidator(cache, persistenceService), new
CommonConfigurationValidator(),
new OperationManager(cache, new OperationHistoryManager()));
// initialize the list of managers
- managers.put(Region.class, new RegionConfigManager());
- managers.put(Pdx.class, new PdxManager());
- managers.put(GatewayReceiver.class, new GatewayReceiverConfigManager());
- managers.put(Index.class, new IndexConfigManager());
- managers.put(Deployment.class, new DeploymentManager());
+ managers.put(Region.class, new RegionConfigManager(persistenceService));
+ managers.put(Pdx.class, new PdxManager(persistenceService));
+ managers.put(GatewayReceiver.class, new
GatewayReceiverConfigManager(persistenceService));
+ managers.put(Index.class, new IndexConfigManager(persistenceService));
+ managers.put(Deployment.class, new DeploymentManager(persistenceService));
// initialize the list of validators
validators.put(Region.class, new RegionConfigValidator(cache));
@@ -183,7 +183,7 @@ public class LocatorClusterManagementService implements
ClusterManagementService
}
// persist configuration in cache config
- boolean success = configurationManager.add(persistenceService, config,
groupName);
+ boolean success = configurationManager.add(config, groupName);
if (success) {
result.setStatus(StatusCode.OK,
"Successfully updated configuration for " + groupName + ".");
@@ -249,7 +249,7 @@ public class LocatorClusterManagementService implements
ClusterManagementService
List<String> updatedGroups = new ArrayList<>();
List<String> failedGroups = new ArrayList<>();
for (String finalGroup : groupsWithThisElement) {
- boolean success = configurationManager.delete(persistenceService,
config, finalGroup);
+ boolean success = configurationManager.delete(config, finalGroup);
if (success) {
updatedGroups.add(finalGroup);
} else {
@@ -298,7 +298,7 @@ public class LocatorClusterManagementService implements
ClusterManagementService
}
for (String group : groups) {
- List<T> list = manager.list(persistenceService, filter, group);
+ List<T> list = manager.list(filter, group);
if (!AbstractConfiguration.isCluster(group)) {
list.forEach(t -> {
if (t instanceof GroupableConfiguration) {
diff --git
a/geode-core/src/main/java/org/apache/geode/management/internal/configuration/mutators/CacheConfigurationManager.java
b/geode-core/src/main/java/org/apache/geode/management/internal/configuration/mutators/CacheConfigurationManager.java
index e5c6cd9..a00a911 100644
---
a/geode-core/src/main/java/org/apache/geode/management/internal/configuration/mutators/CacheConfigurationManager.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/configuration/mutators/CacheConfigurationManager.java
@@ -25,7 +25,7 @@ import org.apache.logging.log4j.Logger;
import org.apache.geode.annotations.Experimental;
import org.apache.geode.cache.configuration.CacheConfig;
-import
org.apache.geode.distributed.internal.InternalConfigurationPersistenceService;
+import org.apache.geode.distributed.ConfigurationPersistenceService;
import org.apache.geode.logging.internal.log4j.api.LogService;
import org.apache.geode.management.configuration.AbstractConfiguration;
@@ -36,6 +36,11 @@ import
org.apache.geode.management.configuration.AbstractConfiguration;
public abstract class CacheConfigurationManager<T extends
AbstractConfiguration>
implements ConfigurationManager<T> {
private static final Logger logger = LogService.getLogger();
+ private final ConfigurationPersistenceService persistenceService;
+
+ CacheConfigurationManager(ConfigurationPersistenceService
persistenceService) {
+ this.persistenceService = persistenceService;
+ }
/**
* specify how to add the config to the existing cache config. Note at this
point, the config
@@ -60,38 +65,37 @@ public abstract class CacheConfigurationManager<T extends
AbstractConfiguration>
*
* Note: incoming and existing should have the same ID already
*/
- public void checkCompatibility(T incoming, String group, T existing) {};
+ public void checkCompatibility(T incoming, String group, T existing) {}
- public final boolean add(InternalConfigurationPersistenceService service, T
config,
- String groupName) {
- return updateCacheConfig(service, config, groupName, this::add);
+ @Override
+ public final boolean add(T config, String groupName) {
+ return updateCacheConfig(config, groupName, this::add);
}
- public final boolean delete(InternalConfigurationPersistenceService service,
T config,
- String groupName) {
- return updateCacheConfig(service, config, groupName, this::delete);
+ @Override
+ public final boolean delete(T config, String groupName) {
+ return updateCacheConfig(config, groupName, this::delete);
}
- public final boolean update(InternalConfigurationPersistenceService service,
T config,
- String groupName) {
- return updateCacheConfig(service, config, groupName, this::update);
+ @Override
+ public final boolean update(T config, String groupName) {
+ return updateCacheConfig(config, groupName, this::update);
}
- public final List<T> list(InternalConfigurationPersistenceService service, T
filterConfig,
- String groupName) {
+ @Override
+ public final List<T> list(T filterConfig, String groupName) {
CacheConfig currentPersistedConfig =
- service.getCacheConfig(
+ persistenceService.getCacheConfig(
AbstractConfiguration.isCluster(groupName) ?
AbstractConfiguration.CLUSTER : groupName,
true);
return list(filterConfig, currentPersistedConfig);
}
- boolean updateCacheConfig(InternalConfigurationPersistenceService service, T
config,
- String groupName, BiConsumer<T, CacheConfig> consumer) {
+ boolean updateCacheConfig(T config, String groupName, BiConsumer<T,
CacheConfig> updater) {
AtomicBoolean success = new AtomicBoolean(true);
- service.updateCacheConfig(groupName, cacheConfigForGroup -> {
+ persistenceService.updateCacheConfig(groupName, cacheConfigForGroup -> {
try {
- consumer.accept(config, cacheConfigForGroup);
+ updater.accept(config, cacheConfigForGroup);
} catch (Exception e) {
String message = "Failed to update cluster configuration for " +
groupName + ".";
logger.error(message, e);
diff --git
a/geode-core/src/main/java/org/apache/geode/management/internal/configuration/mutators/ConfigurationManager.java
b/geode-core/src/main/java/org/apache/geode/management/internal/configuration/mutators/ConfigurationManager.java
index b5dc604..7bc46ab 100644
---
a/geode-core/src/main/java/org/apache/geode/management/internal/configuration/mutators/ConfigurationManager.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/configuration/mutators/ConfigurationManager.java
@@ -19,7 +19,6 @@ package
org.apache.geode.management.internal.configuration.mutators;
import java.util.List;
import org.apache.geode.annotations.Experimental;
-import
org.apache.geode.distributed.internal.InternalConfigurationPersistenceService;
import org.apache.geode.management.configuration.AbstractConfiguration;
/**
@@ -28,14 +27,11 @@ import
org.apache.geode.management.configuration.AbstractConfiguration;
@Experimental
public interface ConfigurationManager<T extends AbstractConfiguration> {
- /**
- * specify how to add the config to the existing group of cluster
configuration
- */
- boolean add(InternalConfigurationPersistenceService service, T config,
String groupName);
+ boolean add(T config, String groupName);
- boolean delete(InternalConfigurationPersistenceService service, T config,
String groupName);
+ boolean delete(T config, String groupName);
- boolean update(InternalConfigurationPersistenceService service, T config,
String groupName);
+ boolean update(T config, String groupName);
- List<T> list(InternalConfigurationPersistenceService service, T
filterConfig, String groupName);
+ List<T> list(T filterConfig, String groupName);
}
diff --git
a/geode-core/src/main/java/org/apache/geode/management/internal/configuration/mutators/DeploymentManager.java
b/geode-core/src/main/java/org/apache/geode/management/internal/configuration/mutators/DeploymentManager.java
index 0604ed2..a6593ca 100644
---
a/geode-core/src/main/java/org/apache/geode/management/internal/configuration/mutators/DeploymentManager.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/configuration/mutators/DeploymentManager.java
@@ -24,28 +24,30 @@ import org.apache.geode.management.configuration.Deployment;
import org.apache.geode.management.internal.configuration.domain.Configuration;
public class DeploymentManager implements ConfigurationManager<Deployment> {
+ private final InternalConfigurationPersistenceService persistenceService;
+
+ public DeploymentManager(InternalConfigurationPersistenceService
persistenceService) {
+ this.persistenceService = persistenceService;
+ }
+
@Override
- public boolean add(InternalConfigurationPersistenceService service,
Deployment config,
- String groupName) {
+ public boolean add(Deployment config, String groupName) {
throw new IllegalStateException("Not implemented");
}
@Override
- public boolean delete(InternalConfigurationPersistenceService service,
Deployment config,
- String groupName) {
+ public boolean delete(Deployment config, String groupName) {
throw new IllegalStateException("Not implemented");
}
@Override
- public boolean update(InternalConfigurationPersistenceService service,
Deployment config,
- String groupName) {
+ public boolean update(Deployment config, String groupName) {
throw new IllegalStateException("Not implemented");
}
@Override
- public List<Deployment> list(InternalConfigurationPersistenceService service,
- Deployment filterConfig, String groupName) {
- Configuration existing = service.getConfiguration(groupName);
+ public List<Deployment> list(Deployment filterConfig, String groupName) {
+ Configuration existing = persistenceService.getConfiguration(groupName);
Stream<String> stream = existing.getJarNames().stream();
if (filterConfig.getJarFileName() != null) {
stream = stream.filter(x -> x.equals(filterConfig.getJarFileName()));
diff --git
a/geode-core/src/main/java/org/apache/geode/management/internal/configuration/mutators/GatewayReceiverConfigManager.java
b/geode-core/src/main/java/org/apache/geode/management/internal/configuration/mutators/GatewayReceiverConfigManager.java
index a29f087..7840ce4 100644
---
a/geode-core/src/main/java/org/apache/geode/management/internal/configuration/mutators/GatewayReceiverConfigManager.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/configuration/mutators/GatewayReceiverConfigManager.java
@@ -20,14 +20,18 @@ import java.util.List;
import org.apache.geode.cache.configuration.CacheConfig;
import org.apache.geode.cache.configuration.GatewayReceiverConfig;
+import org.apache.geode.distributed.ConfigurationPersistenceService;
import org.apache.geode.management.configuration.GatewayReceiver;
import
org.apache.geode.management.internal.configuration.converters.GatewayReceiverConverter;
-public class GatewayReceiverConfigManager
- extends CacheConfigurationManager<GatewayReceiver> {
+public class GatewayReceiverConfigManager extends
CacheConfigurationManager<GatewayReceiver> {
private final GatewayReceiverConverter converter = new
GatewayReceiverConverter();
+ public GatewayReceiverConfigManager(ConfigurationPersistenceService
persistenceService) {
+ super(persistenceService);
+ }
+
@Override
public void add(GatewayReceiver config, CacheConfig existing) {
existing.setGatewayReceiver(converter.fromConfigObject(config));
diff --git
a/geode-core/src/main/java/org/apache/geode/management/internal/configuration/mutators/IndexConfigManager.java
b/geode-core/src/main/java/org/apache/geode/management/internal/configuration/mutators/IndexConfigManager.java
index 09c3db9..52d729d 100644
---
a/geode-core/src/main/java/org/apache/geode/management/internal/configuration/mutators/IndexConfigManager.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/configuration/mutators/IndexConfigManager.java
@@ -22,12 +22,17 @@ import org.apache.commons.lang3.NotImplementedException;
import org.apache.geode.cache.configuration.CacheConfig;
import org.apache.geode.cache.configuration.RegionConfig;
+import org.apache.geode.distributed.ConfigurationPersistenceService;
import org.apache.geode.management.configuration.Index;
import
org.apache.geode.management.internal.configuration.converters.IndexConverter;
public class IndexConfigManager extends CacheConfigurationManager<Index> {
private final IndexConverter converter = new IndexConverter();
+ public IndexConfigManager(ConfigurationPersistenceService
persistenceService) {
+ super(persistenceService);
+ }
+
@Override
public void add(Index config, CacheConfig existing) {
throw new NotImplementedException("Not implemented yet");
diff --git
a/geode-core/src/main/java/org/apache/geode/management/internal/configuration/mutators/PdxManager.java
b/geode-core/src/main/java/org/apache/geode/management/internal/configuration/mutators/PdxManager.java
index 5425dd6..bb7f0c5 100644
---
a/geode-core/src/main/java/org/apache/geode/management/internal/configuration/mutators/PdxManager.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/configuration/mutators/PdxManager.java
@@ -19,12 +19,17 @@ import java.util.Collections;
import java.util.List;
import org.apache.geode.cache.configuration.CacheConfig;
+import org.apache.geode.distributed.ConfigurationPersistenceService;
import org.apache.geode.management.configuration.Pdx;
import
org.apache.geode.management.internal.configuration.converters.PdxConverter;
public class PdxManager extends CacheConfigurationManager<Pdx> {
private final PdxConverter pdxConverter = new PdxConverter();
+ public PdxManager(ConfigurationPersistenceService service) {
+ super(service);
+ }
+
@Override
public void add(Pdx config, CacheConfig existing) {
existing.setPdx(pdxConverter.fromConfigObject((config)));
diff --git
a/geode-core/src/main/java/org/apache/geode/management/internal/configuration/mutators/RegionConfigManager.java
b/geode-core/src/main/java/org/apache/geode/management/internal/configuration/mutators/RegionConfigManager.java
index 70c049a..b7bb973 100644
---
a/geode-core/src/main/java/org/apache/geode/management/internal/configuration/mutators/RegionConfigManager.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/configuration/mutators/RegionConfigManager.java
@@ -27,6 +27,7 @@ import org.apache.commons.lang3.StringUtils;
import org.apache.geode.cache.configuration.CacheConfig;
import org.apache.geode.cache.configuration.RegionConfig;
+import org.apache.geode.distributed.ConfigurationPersistenceService;
import org.apache.geode.lang.Identifiable;
import org.apache.geode.management.configuration.Region;
import
org.apache.geode.management.internal.configuration.converters.RegionConverter;
@@ -35,6 +36,10 @@ public class RegionConfigManager extends
CacheConfigurationManager<Region> {
private final RegionConverter converter = new RegionConverter();
+ public RegionConfigManager(ConfigurationPersistenceService service) {
+ super(service);
+ }
+
@Override
public void add(Region configElement, CacheConfig existingConfig) {
existingConfig.getRegions().add(converter.fromConfigObject(configElement));
diff --git
a/geode-core/src/test/java/org/apache/geode/management/internal/api/LocatorClusterManagementServiceTest.java
b/geode-core/src/test/java/org/apache/geode/management/internal/api/LocatorClusterManagementServiceTest.java
index 32a7950..8c56caa 100644
---
a/geode-core/src/test/java/org/apache/geode/management/internal/api/LocatorClusterManagementServiceTest.java
+++
b/geode-core/src/test/java/org/apache/geode/management/internal/api/LocatorClusterManagementServiceTest.java
@@ -95,20 +95,20 @@ public class LocatorClusterManagementServiceTest {
@Before
public void before() throws Exception {
+ persistenceService = spy(new InternalConfigurationPersistenceService(
+ JAXBService.create(CacheConfig.class)));
+
cache = mock(InternalCache.class);
regionValidator = mock(RegionConfigValidator.class);
doCallRealMethod().when(regionValidator).validate(eq(CacheElementOperation.DELETE),
any());
- regionManager = spy(RegionConfigManager.class);
+ regionManager = spy(new RegionConfigManager(persistenceService));
cacheElementValidator = spy(CommonConfigurationValidator.class);
validators.put(Region.class, regionValidator);
managers.put(Region.class, regionManager);
- managers.put(GatewayReceiverConfig.class, new
GatewayReceiverConfigManager());
+ managers.put(GatewayReceiverConfig.class, new
GatewayReceiverConfigManager(null));
memberValidator = mock(MemberValidator.class);
- persistenceService = spy(new InternalConfigurationPersistenceService(
- JAXBService.create(CacheConfig.class)));
-
Set<String> groups = new HashSet<>();
groups.add("cluster");
doReturn(groups).when(persistenceService).getGroups();
@@ -209,7 +209,7 @@ public class LocatorClusterManagementServiceTest {
service.list(regionConfig);
verify(persistenceService).getCacheConfig("cluster", true);
- verify(regionManager).list(any(), any());
+ verify(regionManager).list(any(Region.class), any(CacheConfig.class));
}
@Test
@@ -219,7 +219,7 @@ public class LocatorClusterManagementServiceTest {
service.list(regionConfig);
verify(persistenceService).getCacheConfig("cluster", true);
- verify(regionManager).list(any(), any());
+ verify(regionManager).list(any(Region.class), any(CacheConfig.class));
}
@Test
@@ -342,7 +342,7 @@ public class LocatorClusterManagementServiceTest {
doReturn(mockRegion).when(persistenceService).getConfigurationRegion();
ClusterManagementRealizationResult result = service.delete(regionConfig);
- verify(regionManager).delete(eq(regionConfig), any());
+ verify(regionManager).delete(eq(regionConfig), any(CacheConfig.class));
assertThat(result.isSuccessful()).isTrue();
assertThat(result.getMemberStatuses()).hasSize(0);
assertThat(result.getStatusMessage())
diff --git
a/geode-core/src/test/java/org/apache/geode/management/internal/configuration/mutators/IndexConfigManagerTest.java
b/geode-core/src/test/java/org/apache/geode/management/internal/configuration/mutators/IndexConfigManagerTest.java
index 91a1f78..6a676c9 100644
---
a/geode-core/src/test/java/org/apache/geode/management/internal/configuration/mutators/IndexConfigManagerTest.java
+++
b/geode-core/src/test/java/org/apache/geode/management/internal/configuration/mutators/IndexConfigManagerTest.java
@@ -36,7 +36,7 @@ public class IndexConfigManagerTest {
public void before() throws Exception {
cacheConfig = new CacheConfig();
index = new Index();
- manager = new IndexConfigManager();
+ manager = new IndexConfigManager(null);
}
@Test
diff --git
a/geode-core/src/test/java/org/apache/geode/management/internal/configuration/mutators/PdxManagerTest.java
b/geode-core/src/test/java/org/apache/geode/management/internal/configuration/mutators/PdxManagerTest.java
index 30f6e41..caeb632 100644
---
a/geode-core/src/test/java/org/apache/geode/management/internal/configuration/mutators/PdxManagerTest.java
+++
b/geode-core/src/test/java/org/apache/geode/management/internal/configuration/mutators/PdxManagerTest.java
@@ -22,6 +22,7 @@ import org.junit.Before;
import org.junit.Test;
import org.apache.geode.cache.configuration.CacheConfig;
+import org.apache.geode.distributed.ConfigurationPersistenceService;
public class PdxManagerTest {
private PdxManager manager;
@@ -29,7 +30,9 @@ public class PdxManagerTest {
@Before
public void before() throws Exception {
- manager = new PdxManager();
+ ConfigurationPersistenceService persistenceService =
+ mock(ConfigurationPersistenceService.class);
+ manager = new PdxManager(persistenceService);
config = mock(CacheConfig.class);
}
diff --git
a/geode-core/src/test/java/org/apache/geode/management/internal/configuration/mutators/RegionConfigManagerTest.java
b/geode-core/src/test/java/org/apache/geode/management/internal/configuration/mutators/RegionConfigManagerTest.java
index 5229795..d33a7b8 100644
---
a/geode-core/src/test/java/org/apache/geode/management/internal/configuration/mutators/RegionConfigManagerTest.java
+++
b/geode-core/src/test/java/org/apache/geode/management/internal/configuration/mutators/RegionConfigManagerTest.java
@@ -31,7 +31,7 @@ public class RegionConfigManagerTest {
@Before
public void before() throws Exception {
- manager = spy(new RegionConfigManager());
+ manager = spy(new RegionConfigManager(null));
config1 = new Region();
config1.setName("test");
config2 = new Region();
diff --git
a/geode-core/src/test/java/org/apache/geode/management/internal/configuration/validators/MemberValidatorTest.java
b/geode-core/src/test/java/org/apache/geode/management/internal/configuration/validators/MemberValidatorTest.java
index 4df1ed2..5eea8f2 100644
---
a/geode-core/src/test/java/org/apache/geode/management/internal/configuration/validators/MemberValidatorTest.java
+++
b/geode-core/src/test/java/org/apache/geode/management/internal/configuration/validators/MemberValidatorTest.java
@@ -53,7 +53,7 @@ public class MemberValidatorTest {
public void before() throws Exception {
cache = mock(InternalCache.class);
service = mock(ConfigurationPersistenceService.class);
- regionManager = new RegionConfigManager();
+ regionManager = new RegionConfigManager(null);
validator = spy(new MemberValidator(cache, service));
DistributedMember member1 = mock(DistributedMember.class);
