Repository: ambari Updated Branches: refs/heads/branch-feature-AMBARI-14714-blueprintv2 60169350a -> 87a7d61fe
AMBARI-22325 Improve exception handling (benyoka) Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/87a7d61f Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/87a7d61f Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/87a7d61f Branch: refs/heads/branch-feature-AMBARI-14714-blueprintv2 Commit: 87a7d61fe9f67b7102bbc955c3a8950cbe57fecc Parents: 6016935 Author: Balazs Bence Sari <beny...@apache.org> Authored: Tue Nov 28 14:32:19 2017 +0100 Committer: Balazs Bence Sari <beny...@apache.org> Committed: Tue Nov 28 14:34:59 2017 +0100 ---------------------------------------------------------------------- .../ambari/server/ObjectNotFoundException.java | 2 +- .../server/ParentObjectNotFoundException.java | 2 +- .../ambari/server/StackAccessException.java | 18 +++- .../server/api/services/AmbariMetaInfo.java | 89 +++++++++++--------- .../AmbariManagementControllerImpl.java | 32 +++++-- .../server/controller/StackV2Factory.java | 8 +- .../server/topology/BlueprintV2Factory.java | 5 +- .../ambari/server/StackAccessExceptionTest.java | 57 +++++++++++++ 8 files changed, 159 insertions(+), 54 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/87a7d61f/ambari-server/src/main/java/org/apache/ambari/server/ObjectNotFoundException.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/ObjectNotFoundException.java b/ambari-server/src/main/java/org/apache/ambari/server/ObjectNotFoundException.java index 75c9f3b..62fa788 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/ObjectNotFoundException.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/ObjectNotFoundException.java @@ -28,7 +28,7 @@ public class ObjectNotFoundException extends AmbariException { * @param cause the root cause */ public ObjectNotFoundException(String msg, ObjectNotFoundException cause) { - super(msg + ". " + cause.getMessage(), cause); + super(msg + ". " + cause.getMessage(), cause); } /** http://git-wip-us.apache.org/repos/asf/ambari/blob/87a7d61f/ambari-server/src/main/java/org/apache/ambari/server/ParentObjectNotFoundException.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/ParentObjectNotFoundException.java b/ambari-server/src/main/java/org/apache/ambari/server/ParentObjectNotFoundException.java index 15bd7cb..1613b84 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/ParentObjectNotFoundException.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/ParentObjectNotFoundException.java @@ -30,7 +30,7 @@ public class ParentObjectNotFoundException extends ObjectNotFoundException { * @param cause the root cause */ public ParentObjectNotFoundException(String msg, ObjectNotFoundException cause) { - super(msg + ". " + cause.getMessage(), cause); + super(msg, cause); } /** http://git-wip-us.apache.org/repos/asf/ambari/blob/87a7d61f/ambari-server/src/main/java/org/apache/ambari/server/StackAccessException.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/StackAccessException.java b/ambari-server/src/main/java/org/apache/ambari/server/StackAccessException.java index b8bfff3..924617f 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/StackAccessException.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/StackAccessException.java @@ -22,6 +22,22 @@ package org.apache.ambari.server; public class StackAccessException extends ObjectNotFoundException { public StackAccessException(String message) { - super("Stack data, " + message); + super(message); } + + public StackAccessException(String... messageAttributes) { + super(buildMessage(messageAttributes)); + } + + private static String buildMessage(String... messageAttributes) { + StringBuilder bld = new StringBuilder("Stack data"); + boolean even = true; + for (String messageAttribute: messageAttributes) { + bld.append(even ? ", " : "="); + bld.append(messageAttribute); + even = !even; + } + return bld.toString(); + } + } http://git-wip-us.apache.org/repos/asf/ambari/blob/87a7d61f/ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariMetaInfo.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariMetaInfo.java b/ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariMetaInfo.java index c410ce4..7cf27ba 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariMetaInfo.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariMetaInfo.java @@ -348,10 +348,11 @@ public class AmbariMetaInfo { ComponentInfo component = getService(stackName, version, stackServiceName).getComponentByName(componentName); if (component == null) { - throw new StackAccessException("stackName=" + stackName - + ", stackVersion=" + version - + ", stackServiceName=" + stackServiceName - + ", componentName=" + componentName); + throw new StackAccessException( + "stackName", stackName, + "stackVersion", version, + "stackServiceName", stackServiceName, + "componentName", componentName); } return component; } @@ -405,11 +406,12 @@ public class AmbariMetaInfo { } } if (foundDependency == null) { - throw new StackAccessException("stackName=" + stackName - + ", stackVersion= " + version - + ", stackService=" + service - + ", stackComponent= " + component - + ", dependency=" + dependencyName); + throw new StackAccessException( + "stackName", stackName, + "stackVersion", version, + "stackService", service, + "stackComponent", component, + "dependency", dependencyName); } return foundDependency; @@ -458,10 +460,11 @@ public class AmbariMetaInfo { List<RepositoryInfo> repositories = getRepositories(stackName, version, osType); if (repositories.size() == 0) { - throw new StackAccessException("stackName=" + stackName - + ", stackVersion=" + version - + ", osType=" + osType - + ", repoId=" + repoId); + throw new StackAccessException( + "stackName", stackName, + "stackVersion", version, + "osType", osType, + "repoId", repoId); } RepositoryInfo repoResult = null; @@ -592,8 +595,10 @@ public class AmbariMetaInfo { ServiceInfo service = getStack(stackName, version).getService(stackServiceName); if (service == null) { - throw new StackAccessException("stackName=" + stackName + ", stackVersion=" + - version + ", stackServiceName=" + stackServiceName); + throw new StackAccessException( + "stackName", stackName, + "stackVersion", version, + "stackServiceName", stackServiceName); } return service; @@ -700,7 +705,7 @@ public class AmbariMetaInfo { Collection<StackInfo> stacks = stackManager.getStacks(stackName); if (stacks.isEmpty()) { - throw new StackAccessException("stackName=" + stackName); + throw new StackAccessException("stackName", stackName); } return stacks; @@ -750,7 +755,7 @@ public class AmbariMetaInfo { Collection<ExtensionInfo> extensions = stackManager.getExtensions(extensionName); if (extensions.isEmpty()) { - throw new StackAccessException("extensionName=" + extensionName); + throw new StackAccessException("extensionName", extensionName); } return extensions; @@ -786,10 +791,11 @@ public class AmbariMetaInfo { : getServiceProperties(stackName, version, stackServiceName); if (properties.size() == 0) { - throw new StackAccessException("stackName=" + stackName - + ", stackVersion=" + version - + ", stackServiceName=" + stackServiceName - + ", propertyName=" + propertyName); + throw new StackAccessException( + "stackName", stackName, + "stackVersion", version, + "stackServiceName", stackServiceName, + "propertyName", propertyName); } Set<PropertyInfo> propertyResult = new HashSet<>(); @@ -854,9 +860,10 @@ public class AmbariMetaInfo { throws AmbariException { Set<PropertyInfo> settings = getStackSettings(stackName, version); if (settings.size() == 0) { - throw new StackAccessException("stackName=" + stackName - + ", stackVersion=" + version - + ", settingName=" + settingName); + throw new StackAccessException( + "stackName", stackName, + "stackVersion", version, + "settingName", settingName); } Set<PropertyInfo> settingResult = new HashSet<>(); @@ -868,9 +875,10 @@ public class AmbariMetaInfo { } if (settingResult.isEmpty()) { - throw new StackAccessException("stackName=" + stackName - + ", stackVersion=" + version - + ", settingName=" + settingName); + throw new StackAccessException( + "stackName",stackName, + "stackVersion", version, + "settingName=", settingName); } return settingResult; @@ -881,9 +889,10 @@ public class AmbariMetaInfo { Set<PropertyInfo> properties = getStackProperties(stackName, version); if (properties.size() == 0) { - throw new StackAccessException("stackName=" + stackName - + ", stackVersion=" + version - + ", propertyName=" + propertyName); + throw new StackAccessException( + "stackName", stackName, + "stackVersion", version, + "propertyName",propertyName); } Set<PropertyInfo> propertyResult = new HashSet<>(); @@ -895,9 +904,10 @@ public class AmbariMetaInfo { } if (propertyResult.isEmpty()) { - throw new StackAccessException("stackName=" + stackName - + ", stackVersion=" + version - + ", propertyName=" + propertyName); + throw new StackAccessException( + "stackName", stackName, + "stackVersion", version, + "propertyName", propertyName); } return propertyResult; @@ -925,9 +935,10 @@ public class AmbariMetaInfo { Set<OperatingSystemInfo> operatingSystems = getOperatingSystems(stackName, version); if (operatingSystems.size() == 0) { - throw new StackAccessException("stackName=" + stackName - + ", stackVersion=" + version - + ", osType=" + osType); + throw new StackAccessException( + "stackName",stackName, + "stackVersion", version, + "osType", osType); } OperatingSystemInfo resultOperatingSystem = null; @@ -940,9 +951,9 @@ public class AmbariMetaInfo { } if (resultOperatingSystem == null) { - throw new StackAccessException("stackName=" + stackName - + ", stackVersion=" + version - + ", osType=" + osType); + throw new StackAccessException("stackName" + stackName + + "stackVersion=" + version + + "osType=" + osType); } return resultOperatingSystem; http://git-wip-us.apache.org/repos/asf/ambari/blob/87a7d61f/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java index 6896a9b..c150f5b 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java @@ -471,7 +471,9 @@ public class AmbariManagementControllerImpl implements AmbariManagementControlle stackId.getStackVersion()); if (stackInfo == null) { - throw new StackAccessException("stackName=" + stackId.getStackName() + ", stackVersion=" + stackId.getStackVersion()); + throw new StackAccessException( + "stackName", stackId.getStackName(), + "stackVersion=", stackId.getStackVersion()); } // FIXME add support for desired configs at cluster level @@ -5819,13 +5821,17 @@ public class AmbariManagementControllerImpl implements AmbariManagementControlle StackInfo stackInfo = ambariMetaInfo.getStack(linkEntity.getStack().getStackName(), linkEntity.getStack().getStackVersion()); if (stackInfo == null) { - throw new StackAccessException("stackName=" + linkEntity.getStack().getStackName() + ", stackVersion=" + linkEntity.getStack().getStackVersion()); + throw new StackAccessException( + "stackName", linkEntity.getStack().getStackName(), + "stackVersion", linkEntity.getStack().getStackVersion()); } ExtensionInfo extensionInfo = ambariMetaInfo.getExtension(linkEntity.getExtension().getExtensionName(), linkEntity.getExtension().getExtensionVersion()); if (extensionInfo == null) { - throw new StackAccessException("extensionName=" + linkEntity.getExtension().getExtensionName() + ", extensionVersion=" + linkEntity.getExtension().getExtensionVersion()); + throw new StackAccessException( + "extensionName", linkEntity.getExtension().getExtensionName(), + "extensionVersion", linkEntity.getExtension().getExtensionVersion()); } ExtensionHelper.validateDeleteLink(getClusters(), stackInfo, extensionInfo); @@ -5863,13 +5869,17 @@ public class AmbariManagementControllerImpl implements AmbariManagementControlle StackInfo stackInfo = ambariMetaInfo.getStack(request.getStackName(), request.getStackVersion()); if (stackInfo == null) { - throw new StackAccessException("stackName=" + request.getStackName() + ", stackVersion=" + request.getStackVersion()); + throw new StackAccessException( + "stackName", request.getStackName(), + "stackVersion", request.getStackVersion()); } ExtensionInfo extensionInfo = ambariMetaInfo.getExtension(request.getExtensionName(), request.getExtensionVersion()); if (extensionInfo == null) { - throw new StackAccessException("extensionName=" + request.getExtensionName() + ", extensionVersion=" + request.getExtensionVersion()); + throw new StackAccessException( + "extensionName", request.getExtensionName(), + "extensionVersion", request.getExtensionVersion()); } helper.createExtensionLink(ambariMetaInfo.getStackManager(), stackInfo, extensionInfo); @@ -5905,7 +5915,9 @@ public class AmbariManagementControllerImpl implements AmbariManagementControlle StackInfo stackInfo = ambariMetaInfo.getStack(oldLinkEntity.getStack().getStackName(), oldLinkEntity.getStack().getStackVersion()); if (stackInfo == null) { - throw new StackAccessException(String.format("stackName=%s, stackVersion=%s", oldLinkEntity.getStack().getStackName(), oldLinkEntity.getStack().getStackVersion())); + throw new StackAccessException( + "stackName", oldLinkEntity.getStack().getStackName(), + "stackVersion", oldLinkEntity.getStack().getStackVersion()); } if (newLinkRequest.getExtensionName() == null || newLinkRequest.getExtensionVersion() == null) { @@ -5922,10 +5934,14 @@ public class AmbariManagementControllerImpl implements AmbariManagementControlle ExtensionInfo newExtensionInfo = ambariMetaInfo.getExtension(newLinkRequest.getExtensionName(), newLinkRequest.getExtensionVersion()); if (oldExtensionInfo == null) { - throw new StackAccessException(String.format("Old extensionName=%s, extensionVersion=%s", oldLinkEntity.getExtension().getExtensionName(), oldLinkEntity.getExtension().getExtensionVersion())); + throw new StackAccessException( + "Old extensionName", oldLinkEntity.getStack().getStackName(), + "extensionVersion", oldLinkEntity.getExtension().getExtensionVersion()); } if (newExtensionInfo == null) { - throw new StackAccessException(String.format("New extensionName=%s, extensionVersion=%s", newLinkRequest.getExtensionName(), newLinkRequest.getExtensionVersion())); + throw new StackAccessException( + "New extensionName", newLinkRequest.getExtensionName(), + "extensionVersion", newLinkRequest.getExtensionVersion()); } helper.updateExtensionLink(ambariMetaInfo.getStackManager(), oldLinkEntity, stackInfo, oldExtensionInfo, newExtensionInfo); http://git-wip-us.apache.org/repos/asf/ambari/blob/87a7d61f/ambari-server/src/main/java/org/apache/ambari/server/controller/StackV2Factory.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/StackV2Factory.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/StackV2Factory.java index 201fcb1..546c99a 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/StackV2Factory.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/StackV2Factory.java @@ -26,6 +26,7 @@ import java.util.Map; import java.util.Set; import org.apache.ambari.server.AmbariException; +import org.apache.ambari.server.ObjectNotFoundException; import org.apache.ambari.server.orm.dao.RepositoryVersionDAO; import org.apache.ambari.server.orm.entities.RepositoryVersionEntity; import org.apache.ambari.server.orm.entities.StackEntity; @@ -36,8 +37,6 @@ import org.apache.ambari.server.state.StackId; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import com.google.common.base.Preconditions; - public class StackV2Factory { private final static Logger LOG = LoggerFactory.getLogger(StackV2Factory.class); @@ -89,7 +88,10 @@ public class StackV2Factory { StackId stackId = new StackId(stackData.stackName, stackData.stackVersion); RepositoryVersionEntity entity = repositoryVersionDAO.findByStackAndVersion(stackId, stackData.repoVersion); - Preconditions.checkNotNull(entity, "Repo version %s not found for stack %s", stackData.repoVersion, stackId); + if (null == entity) { + throw new ObjectNotFoundException( + String.format("Repo version %s not found for stack %s", stackData.repoVersion, stackId)); + } } private void getComponentInfos(StackData stackData) { http://git-wip-us.apache.org/repos/asf/ambari/blob/87a7d61f/ambari-server/src/main/java/org/apache/ambari/server/topology/BlueprintV2Factory.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/topology/BlueprintV2Factory.java b/ambari-server/src/main/java/org/apache/ambari/server/topology/BlueprintV2Factory.java index 6ce27ef..99fbd87 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/topology/BlueprintV2Factory.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/topology/BlueprintV2Factory.java @@ -129,7 +129,10 @@ public class BlueprintV2Factory { return stackFactory.create(stackId, repositoryVersion); } catch (AmbariException e) { throw new IllegalArgumentException( - String.format("Unable to parse stack. name=%s, version=%s", stackId.getStackName(), stackId.getStackVersion()), + String.format("Unable to parse stack. name=%s, version=%s, cause: %s", + stackId.getStackName(), + stackId.getStackVersion(), + e.getMessage()), e); } } http://git-wip-us.apache.org/repos/asf/ambari/blob/87a7d61f/ambari-server/src/test/java/org/apache/ambari/server/StackAccessExceptionTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/StackAccessExceptionTest.java b/ambari-server/src/test/java/org/apache/ambari/server/StackAccessExceptionTest.java new file mode 100644 index 0000000..452a2ca --- /dev/null +++ b/ambari-server/src/test/java/org/apache/ambari/server/StackAccessExceptionTest.java @@ -0,0 +1,57 @@ +/* + * 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.ambari.server; + +import static org.junit.Assert.assertEquals; + +import org.junit.Test; + +public class StackAccessExceptionTest { + + @Test + public void testConstructor() { + StackAccessException ex = new StackAccessException( + "stackName", "HDP", + "stackVersion", "3.0.0", + "repoVersion", "3.0.0.0-467"); + assertEquals("Stack data, stackName=HDP, stackVersion=3.0.0, repoVersion=3.0.0.0-467", ex.getMessage()); + } + + /** + * These is an erroneous usage. However, it should be safe (throw no exception). + */ + @Test + public void testConstructorOddArgsShouldNotThrowException() { + StackAccessException ex = new StackAccessException( + "stackName", "HDP", + "stackVersion", "3.0.0", + "repoVersion"); + assertEquals("Stack data, stackName=HDP, stackVersion=3.0.0, repoVersion", ex.getMessage()); + } + + /** + * These is an erroneous usage. However, it should be safe (throw no exception). + */ + @Test + public void testConstructorNoArgsShouldNotThrowException() { + StackAccessException ex = new StackAccessException(); + assertEquals("Stack data", ex.getMessage()); + } + + +} \ No newline at end of file
