You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by ji...@apache.org on 2019/01/31 01:21:05 UTC
[geode] branch develop updated: GEODE-6174: more error handling in
LocatorClusterConfigurationService (#3134)
This is an automated email from the ASF dual-hosted git repository.
jinmeiliao 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 9c35fc7 GEODE-6174: more error handling in LocatorClusterConfigurationService (#3134)
9c35fc7 is described below
commit 9c35fc714ef28074c832158c2673823bc7d08e14
Author: jinmeiliao <ji...@pivotal.io>
AuthorDate: Wed Jan 30 17:20:48 2019 -0800
GEODE-6174: more error handling in LocatorClusterConfigurationService (#3134)
* GEODE-6174: more error handling in LocatorClusterConfigurationService
* fail early if cluster persistence serivce is not enabled
* creating the region twice will result in status code 409
* if region creation is partially failed on members, do not update cluster configuration.
---
.../internal/rest/RegionManagementDunitTest.java | 7 ++
.../internal/DisabledClusterConfigTest.java | 56 ++++++++++
.../internal/HttpServiceIntegrationTest.java | 55 +++++++---
.../internal/api/RegionAPIDUnitTest.java | 13 ---
.../api/LocatorClusterManagementService.java | 64 ++++++-----
.../internal/cli/functions/CliFunctionResult.java | 4 -
.../mutators/RegionConfigMutator.java | 3 +-
.../api/LocatorClusterManagementServiceTest.java | 117 +++++++++++++++++++++
.../geode/test/junit/rules/MemberStarterRule.java | 2 +-
.../controllers/ManagementControllerAdvice.java | 12 ++-
10 files changed, 269 insertions(+), 64 deletions(-)
diff --git a/geode-assembly/src/distributedTest/java/org/apache/geode/management/internal/rest/RegionManagementDunitTest.java b/geode-assembly/src/distributedTest/java/org/apache/geode/management/internal/rest/RegionManagementDunitTest.java
index 43f62dc..628965d 100644
--- a/geode-assembly/src/distributedTest/java/org/apache/geode/management/internal/rest/RegionManagementDunitTest.java
+++ b/geode-assembly/src/distributedTest/java/org/apache/geode/management/internal/rest/RegionManagementDunitTest.java
@@ -94,6 +94,13 @@ public class RegionManagementDunitTest {
// make sure region is persisted
locator.invoke(() -> verifyRegionPersisted("orders", "PARTITION"));
+
+ // create the same region 2nd time
+ result = restClient.doPostAndAssert("/regions", json)
+ .hasStatusCode(409)
+ .getClusterManagementResult();
+ assertThat(result.isSuccessfullyAppliedOnMembers()).isFalse();
+ assertThat(result.isSuccessfullyPersisted()).isFalse();
}
@Test
diff --git a/geode-assembly/src/integrationTest/java/org/apache/geode/management/internal/DisabledClusterConfigTest.java b/geode-assembly/src/integrationTest/java/org/apache/geode/management/internal/DisabledClusterConfigTest.java
new file mode 100644
index 0000000..396c52f
--- /dev/null
+++ b/geode-assembly/src/integrationTest/java/org/apache/geode/management/internal/DisabledClusterConfigTest.java
@@ -0,0 +1,56 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.apache.geode.management.internal;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.Test;
+
+import org.apache.geode.distributed.ConfigurationProperties;
+import org.apache.geode.management.internal.api.ClusterManagementResult;
+import org.apache.geode.test.junit.rules.GeodeDevRestClient;
+import org.apache.geode.test.junit.rules.LocatorStarterRule;
+import org.apache.geode.test.junit.rules.RequiresGeodeHome;
+
+public class DisabledClusterConfigTest {
+ @Rule
+ public LocatorStarterRule locator = new LocatorStarterRule();
+
+ @ClassRule
+ public static RequiresGeodeHome requiresGeodeHome = new RequiresGeodeHome();
+
+ public static GeodeDevRestClient restClient;
+
+ @Test
+ public void disabledClusterConfig() throws Exception {
+ locator.withProperty(ConfigurationProperties.ENABLE_CLUSTER_CONFIGURATION, "false")
+ .withHttpService().startLocator();
+ restClient =
+ new GeodeDevRestClient("/geode-management/v2", "localhost", locator.getHttpPort(), false);
+
+ ClusterManagementResult result =
+ restClient.doPostAndAssert("/regions", "{\"name\":\"test\"}")
+ .hasStatusCode(500)
+ .getClusterManagementResult();
+ assertThat(result.isSuccessful()).isFalse();
+ assertThat(result.isSuccessfullyPersisted()).isFalse();
+ assertThat(result.isSuccessfullyAppliedOnMembers()).isFalse();
+ assertThat(result.getPersistenceStatus().getMessage())
+ .isEqualTo("Cluster configuration service needs to be enabled");
+ }
+}
diff --git a/geode-assembly/src/integrationTest/java/org/apache/geode/management/internal/HttpServiceIntegrationTest.java b/geode-assembly/src/integrationTest/java/org/apache/geode/management/internal/HttpServiceIntegrationTest.java
index 0021a0f..b7ba031 100644
--- a/geode-assembly/src/integrationTest/java/org/apache/geode/management/internal/HttpServiceIntegrationTest.java
+++ b/geode-assembly/src/integrationTest/java/org/apache/geode/management/internal/HttpServiceIntegrationTest.java
@@ -12,44 +12,73 @@
* or implied. See the License for the specific language governing permissions and limitations under
* the License.
*/
-package org.apache.geode.management.internal;
+package org.apache.geode.management.internal;
import org.junit.ClassRule;
+import org.junit.Rule;
import org.junit.Test;
import org.apache.geode.test.junit.rules.GeodeDevRestClient;
import org.apache.geode.test.junit.rules.RequiresGeodeHome;
import org.apache.geode.test.junit.rules.ServerStarterRule;
-
public class HttpServiceIntegrationTest {
-
@ClassRule
public static RequiresGeodeHome requiresGeodeHome = new RequiresGeodeHome();
- @ClassRule
- public static ServerStarterRule server =
- new ServerStarterRule().withRestService().withJMXManager().withAutoStart();
+ @Rule
+ public ServerStarterRule server = new ServerStarterRule();
+
+ private GeodeDevRestClient client;
@Test
- public void devRestIsAvailable() throws Exception {
- GeodeDevRestClient client =
+ public void withDevRestAndJmxManager() throws Exception {
+ server.withRestService().withJMXManager().startServer();
+ client =
new GeodeDevRestClient("/geode/v1", "localhost", server.getHttpPort(), false);
client.doGetAndAssert("/servers").hasStatusCode(200);
+
+ client =
+ new GeodeDevRestClient("/geode-mgmt/v1", "localhost", server.getHttpPort(), false);
+ client.doGetAndAssert("/version").hasStatusCode(200);
+
+ client =
+ new GeodeDevRestClient("/pulse", "localhost", server.getHttpPort(), false);
+ client.doGetAndAssert("/index.html").hasStatusCode(200);
+
}
@Test
- public void adminRestIsAvailable() throws Exception {
- GeodeDevRestClient client =
+ public void withDevRestOnly() throws Exception {
+ server.withRestService().startServer();
+ client =
+ new GeodeDevRestClient("/geode/v1", "localhost", server.getHttpPort(), false);
+ client.doGetAndAssert("/servers").hasStatusCode(200);
+
+ client =
new GeodeDevRestClient("/geode-mgmt/v1", "localhost", server.getHttpPort(), false);
- client.doGetAndAssert("/version").hasStatusCode(200);
+ client.doGetAndAssert("/version").hasStatusCode(404);
+
+ client =
+ new GeodeDevRestClient("/pulse", "localhost", server.getHttpPort(), false);
+ client.doGetAndAssert("/index.html").hasStatusCode(404);
}
@Test
- public void pulseIsAvailable() throws Exception {
- GeodeDevRestClient client =
+ public void withAdminRestOnly() throws Exception {
+ server.withJMXManager().withHttpService().startServer();
+ client =
+ new GeodeDevRestClient("/geode/v1", "localhost", server.getHttpPort(), false);
+ client.doGetAndAssert("/servers").hasStatusCode(404);
+
+ client =
+ new GeodeDevRestClient("/geode-mgmt/v1", "localhost", server.getHttpPort(), false);
+ client.doGetAndAssert("/version").hasStatusCode(200);
+
+ client =
new GeodeDevRestClient("/pulse", "localhost", server.getHttpPort(), false);
client.doGetAndAssert("/index.html").hasStatusCode(200);
}
+
}
diff --git a/geode-core/src/distributedTest/java/org/apache/geode/management/internal/api/RegionAPIDUnitTest.java b/geode-core/src/distributedTest/java/org/apache/geode/management/internal/api/RegionAPIDUnitTest.java
index 70c5800..0c2fec2 100644
--- a/geode-core/src/distributedTest/java/org/apache/geode/management/internal/api/RegionAPIDUnitTest.java
+++ b/geode-core/src/distributedTest/java/org/apache/geode/management/internal/api/RegionAPIDUnitTest.java
@@ -15,7 +15,6 @@
package org.apache.geode.management.internal.api;
import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.assertThatThrownBy;
import org.junit.BeforeClass;
import org.junit.ClassRule;
@@ -100,18 +99,6 @@ public class RegionAPIDUnitTest {
}
@Test
- public void noName() {
- locator.invoke(() -> {
- RegionConfig config = new RegionConfig();
- ClusterManagementService clusterManagementService =
- ClusterStartupRule.getLocator().getClusterManagementService();
- assertThatThrownBy(() -> clusterManagementService.create(config, "cluster"))
- .isInstanceOf(IllegalArgumentException.class)
- .hasMessageContaining("Name of the region has to be specified");
- });
- }
-
- @Test
public void defaultTypeIsPartition() throws Exception {
String regionName = testName.getMethodName();
locator.invoke(() -> {
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 17c5102..3313420 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
@@ -43,7 +43,6 @@ import org.apache.geode.management.internal.configuration.mutators.RegionConfigM
import org.apache.geode.management.internal.configuration.validators.ConfigurationValidator;
import org.apache.geode.management.internal.configuration.validators.RegionConfigValidator;
import org.apache.geode.management.internal.exceptions.EntityExistsException;
-import org.apache.geode.management.internal.exceptions.NoMembersException;
public class LocatorClusterManagementService implements ClusterManagementService {
private static Logger logger = LogService.getLogger();
@@ -77,27 +76,33 @@ public class LocatorClusterManagementService implements ClusterManagementService
group = "cluster";
}
+ if (persistenceService == null) {
+ return new ClusterManagementResult(false,
+ "Cluster configuration service needs to be enabled");
+ }
+
ClusterManagementResult result = new ClusterManagementResult();
ConfigurationMutator configurationMutator = mutators.get(config.getClass());
ConfigurationValidator validator = validators.get(config.getClass());
if (validator != null) {
- validator.validate(config);
+ try {
+ validator.validate(config);
+ } catch (IllegalArgumentException e) {
+ return new ClusterManagementResult(false, e.getMessage());
+ }
}
- final boolean configurationPersistenceEnabled = persistenceService != null;
// exit early if config element already exists in cache config
- if (configurationPersistenceEnabled) {
- CacheConfig currentPersistedConfig = persistenceService.getCacheConfig(group, true);
- if (configurationMutator.exists(config, currentPersistedConfig)) {
- throw new EntityExistsException("cache element " + config.getId() + " already exists.");
- }
+ CacheConfig currentPersistedConfig = persistenceService.getCacheConfig(group, true);
+ if (configurationMutator.exists(config, currentPersistedConfig)) {
+ throw new EntityExistsException("cache element " + config.getId() + " already exists.");
}
// execute function on all members
Set<DistributedMember> targetedMembers = findMembers(null, null);
if (targetedMembers.size() == 0) {
- throw new NoMembersException("no members found to create cache element");
+ return new ClusterManagementResult(false, "no members found to create cache element");
}
List<CliFunctionResult> functionResults = executeAndGetFunctionResult(
@@ -109,25 +114,26 @@ public class LocatorClusterManagementService implements ClusterManagementService
functionResult.isSuccessful(),
functionResult.getStatusMessage()));
- // persist configuration in cache config
- if (configurationPersistenceEnabled) {
- String finalGroup = group;
- persistenceService.updateCacheConfig(finalGroup, cacheConfigForGroup -> {
- try {
- configurationMutator.add(config, cacheConfigForGroup);
- result.setClusterConfigPersisted(true,
- "successfully persisted config for " + finalGroup);
- } catch (Exception e) {
- String message = "failed to update cluster config for " + finalGroup;
- logger.error(message, e);
- result.setClusterConfigPersisted(false, message);
- return null;
- }
-
- return cacheConfigForGroup;
- });
+ if (!result.isSuccessfullyAppliedOnMembers()) {
+ result.setClusterConfigPersisted(false, "Failed to apply the update on all members.");
+ return result;
}
+ // persist configuration in cache config
+ String finalGroup = group;
+ persistenceService.updateCacheConfig(finalGroup, cacheConfigForGroup -> {
+ try {
+ configurationMutator.add(config, cacheConfigForGroup);
+ result.setClusterConfigPersisted(true,
+ "successfully persisted config for " + finalGroup);
+ } catch (Exception e) {
+ String message = "failed to update cluster config for " + finalGroup;
+ logger.error(message, e);
+ result.setClusterConfigPersisted(false, message);
+ return null;
+ }
+ return cacheConfigForGroup;
+ });
return result;
}
@@ -141,11 +147,13 @@ public class LocatorClusterManagementService implements ClusterManagementService
throw new NotImplementedException();
}
- private Set<DistributedMember> findMembers(String[] groups, String[] members) {
+ @VisibleForTesting
+ Set<DistributedMember> findMembers(String[] groups, String[] members) {
return CliUtil.findMembers(groups, members, distributionManager);
}
- private List<CliFunctionResult> executeAndGetFunctionResult(Function function, Object args,
+ @VisibleForTesting
+ List<CliFunctionResult> executeAndGetFunctionResult(Function function, Object args,
Set<DistributedMember> targetMembers) {
ResultCollector rc = CliUtil.executeFunction(function, args, targetMembers);
return CliFunctionResult.cleanResults((List<?>) rc.getResult());
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/CliFunctionResult.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/CliFunctionResult.java
index b649480..424ffc4 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/CliFunctionResult.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/CliFunctionResult.java
@@ -82,10 +82,6 @@ public class CliFunctionResult implements Comparable<CliFunctionResult>, DataSer
this.state = StatusState.OK;
}
- /**
- * @deprecated Use {@code CliFunctionResult(String, StatusState, String)} instead
- */
- @Deprecated
public CliFunctionResult(final String memberIdOrName, final boolean successful,
final String message) {
this(memberIdOrName, successful ? StatusState.OK : StatusState.ERROR, message);
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/configuration/mutators/RegionConfigMutator.java b/geode-core/src/main/java/org/apache/geode/management/internal/configuration/mutators/RegionConfigMutator.java
index 0685505..ec06e8e 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/configuration/mutators/RegionConfigMutator.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/configuration/mutators/RegionConfigMutator.java
@@ -18,6 +18,7 @@
package org.apache.geode.management.internal.configuration.mutators;
import org.apache.geode.cache.configuration.CacheConfig;
+import org.apache.geode.cache.configuration.CacheElement;
import org.apache.geode.cache.configuration.RegionConfig;
public class RegionConfigMutator implements ConfigurationMutator<RegionConfig> {
@@ -31,7 +32,7 @@ public class RegionConfigMutator implements ConfigurationMutator<RegionConfig> {
@Override
public boolean exists(RegionConfig config, CacheConfig existing) {
- return false;
+ return CacheElement.exists(existing.getRegions(), config.getId());
}
@Override
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
new file mode 100644
index 0000000..ad1031f
--- /dev/null
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/api/LocatorClusterManagementServiceTest.java
@@ -0,0 +1,117 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.apache.geode.management.internal.api;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.when;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import org.apache.geode.cache.configuration.CacheConfig;
+import org.apache.geode.cache.configuration.RegionConfig;
+import org.apache.geode.distributed.ConfigurationPersistenceService;
+import org.apache.geode.distributed.DistributedMember;
+import org.apache.geode.distributed.internal.DistributionManager;
+import org.apache.geode.management.internal.cli.functions.CliFunctionResult;
+import org.apache.geode.management.internal.exceptions.EntityExistsException;
+
+public class LocatorClusterManagementServiceTest {
+
+ private LocatorClusterManagementService service;
+ private DistributionManager distributionManager;
+ private ConfigurationPersistenceService persistenceService;
+ private RegionConfig regionConfig;
+ private ClusterManagementResult result;
+
+ @Before
+ public void before() throws Exception {
+ distributionManager = mock(DistributionManager.class);
+ persistenceService = mock(ConfigurationPersistenceService.class);
+ service = spy(new LocatorClusterManagementService(distributionManager, persistenceService));
+ regionConfig = new RegionConfig();
+ }
+
+ @Test
+ public void persistenceIsNull() throws Exception {
+ service = new LocatorClusterManagementService(distributionManager, null);
+ result = service.create(regionConfig, "cluster");
+ assertThat(result.isSuccessful()).isFalse();
+ assertThat(result.getPersistenceStatus().getMessage())
+ .contains("Cluster configuration service needs to be enabled");
+ }
+
+ @Test
+ public void elementAlreadyExist() throws Exception {
+ regionConfig.setName("test");
+ CacheConfig cacheConfig = new CacheConfig();
+ cacheConfig.getRegions().add(regionConfig);
+ when(persistenceService.getCacheConfig("cluster", true)).thenReturn(cacheConfig);
+
+ assertThatThrownBy(() -> service.create(regionConfig, "cluster"))
+ .isInstanceOf(EntityExistsException.class)
+ .hasMessageContaining("cache element test already exists");
+ }
+
+ @Test
+ public void validationFailed() throws Exception {
+ result = service.create(regionConfig, "cluster");
+ assertThat(result.isSuccessful()).isFalse();
+ assertThat(result.getPersistenceStatus().getMessage())
+ .contains("Name of the region has to be specified");
+ }
+
+ @Test
+ public void noMemberFound() throws Exception {
+ regionConfig.setName("test");
+ when(persistenceService.getCacheConfig("cluster", true)).thenReturn(new CacheConfig());
+ doReturn(Collections.emptySet()).when(service).findMembers(any(), any());
+ result = service.create(regionConfig, "cluster");
+ assertThat(result.isSuccessful()).isFalse();
+ assertThat(result.isSuccessfullyAppliedOnMembers()).isFalse();
+ assertThat(result.getPersistenceStatus().getMessage())
+ .contains("no members found to create cache element");
+ }
+
+ @Test
+ public void partialFailureOnMembers() throws Exception {
+ List<CliFunctionResult> functionResults = new ArrayList<>();
+ functionResults.add(new CliFunctionResult("member1", true, "success"));
+ functionResults.add(new CliFunctionResult("member2", false, "failed"));
+ doReturn(functionResults).when(service).executeAndGetFunctionResult(any(), any(), any());
+
+ doReturn(Collections.singleton(mock(DistributedMember.class))).when(service).findMembers(any(),
+ any());
+
+ when(persistenceService.getCacheConfig("cluster", true)).thenReturn(new CacheConfig());
+ regionConfig.setName("test");
+ result = service.create(regionConfig, "cluster");
+ assertThat(result.isSuccessful()).isFalse();
+ assertThat(result.isSuccessfullyAppliedOnMembers()).isFalse();
+ assertThat(result.isSuccessfullyPersisted()).isFalse();
+ assertThat(result.getPersistenceStatus().getMessage())
+ .contains("Failed to apply the update on all members");
+ }
+}
diff --git a/geode-dunit/src/main/java/org/apache/geode/test/junit/rules/MemberStarterRule.java b/geode-dunit/src/main/java/org/apache/geode/test/junit/rules/MemberStarterRule.java
index 18ae982..f878f12 100644
--- a/geode-dunit/src/main/java/org/apache/geode/test/junit/rules/MemberStarterRule.java
+++ b/geode-dunit/src/main/java/org/apache/geode/test/junit/rules/MemberStarterRule.java
@@ -240,7 +240,7 @@ public abstract class MemberStarterRule<T> extends SerializableExternalResource
public T withHttpService(boolean useDefaultPort) {
properties.setProperty(HTTP_SERVICE_BIND_ADDRESS, "localhost");
if (!useDefaultPort) {
- properties.putIfAbsent(HTTP_SERVICE_PORT,
+ properties.put(HTTP_SERVICE_PORT,
portSupplier.getAvailablePort() + "");
this.httpPort = Integer.parseInt(properties.getProperty(HTTP_SERVICE_PORT));
} else {
diff --git a/geode-web-management/src/main/java/org/apache/geode/management/internal/rest/controllers/ManagementControllerAdvice.java b/geode-web-management/src/main/java/org/apache/geode/management/internal/rest/controllers/ManagementControllerAdvice.java
index 1ecd13e..43358ff 100644
--- a/geode-web-management/src/main/java/org/apache/geode/management/internal/rest/controllers/ManagementControllerAdvice.java
+++ b/geode-web-management/src/main/java/org/apache/geode/management/internal/rest/controllers/ManagementControllerAdvice.java
@@ -27,6 +27,7 @@ import org.springframework.web.bind.annotation.ExceptionHandler;
import org.apache.geode.internal.logging.LogService;
import org.apache.geode.management.internal.api.ClusterManagementResult;
+import org.apache.geode.management.internal.exceptions.EntityExistsException;
import org.apache.geode.security.AuthenticationFailedException;
import org.apache.geode.security.NotAuthorizedException;
@@ -41,6 +42,13 @@ public class ManagementControllerAdvice {
HttpStatus.INTERNAL_SERVER_ERROR);
}
+ @ExceptionHandler(EntityExistsException.class)
+ public ResponseEntity<ClusterManagementResult> entityExists(final Exception e) {
+ // no need to log the error stack. User only needs to know the message.
+ return new ResponseEntity<>(new ClusterManagementResult(false, e.getMessage()),
+ HttpStatus.CONFLICT);
+ }
+
@ExceptionHandler({AuthenticationFailedException.class, AuthenticationException.class})
public ResponseEntity<ClusterManagementResult> unauthorized(AuthenticationFailedException e) {
return new ResponseEntity<>(new ClusterManagementResult(false, e.getMessage()),
@@ -49,21 +57,18 @@ public class ManagementControllerAdvice {
@ExceptionHandler({NotAuthorizedException.class, SecurityException.class})
public ResponseEntity<ClusterManagementResult> forbidden(Exception e) {
- logger.info(e.getMessage());
return new ResponseEntity<>(new ClusterManagementResult(false, e.getMessage()),
HttpStatus.FORBIDDEN);
}
@ExceptionHandler(MalformedObjectNameException.class)
public ResponseEntity<ClusterManagementResult> badRequest(final MalformedObjectNameException e) {
- logger.info(e.getMessage(), e);
return new ResponseEntity<>(new ClusterManagementResult(false, e.getMessage()),
HttpStatus.BAD_REQUEST);
}
@ExceptionHandler(InstanceNotFoundException.class)
public ResponseEntity<ClusterManagementResult> notFound(final InstanceNotFoundException e) {
- logger.info(e.getMessage(), e);
return new ResponseEntity<>(new ClusterManagementResult(false, e.getMessage()),
HttpStatus.NOT_FOUND);
}
@@ -79,7 +84,6 @@ public class ManagementControllerAdvice {
@ExceptionHandler(AccessDeniedException.class)
public ResponseEntity<ClusterManagementResult> handleException(
final AccessDeniedException cause) {
- logger.info(cause.getMessage(), cause);
return new ResponseEntity<>(new ClusterManagementResult(false, cause.getMessage()),
HttpStatus.FORBIDDEN);
}