Repository: sentry Updated Branches: refs/heads/sentry-ha-redesign afdfd86b0 -> 731a5d15e
SENTRY-1546: Generic Policy provides bad error messages for Sentry exceptions (Kalyan Kumar Kalvagadda, Reviewed by: Alex Kolbasov) Project: http://git-wip-us.apache.org/repos/asf/sentry/repo Commit: http://git-wip-us.apache.org/repos/asf/sentry/commit/731a5d15 Tree: http://git-wip-us.apache.org/repos/asf/sentry/tree/731a5d15 Diff: http://git-wip-us.apache.org/repos/asf/sentry/diff/731a5d15 Branch: refs/heads/sentry-ha-redesign Commit: 731a5d15eb78178e6ffdcd1f7444f8c4172bf663 Parents: afdfd86 Author: Alexander Kolbasov <[email protected]> Authored: Mon Mar 6 17:31:48 2017 -0800 Committer: Alexander Kolbasov <[email protected]> Committed: Mon Mar 6 17:31:48 2017 -0800 ---------------------------------------------------------------------- .../core/common/exception/SentryAlreadyExistsException.java | 3 ++- .../core/common/exception/SentryNoSuchObjectException.java | 3 ++- .../db/generic/service/persistent/DelegateSentryStore.java | 4 ++-- .../sentry/provider/db/service/persistent/SentryStore.java | 7 ++++--- .../db/service/thrift/SentryPolicyStoreProcessor.java | 2 +- .../service/thrift/TestSentryGenericPolicyProcessor.java | 8 ++++---- .../apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java | 2 +- 7 files changed, 16 insertions(+), 13 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/sentry/blob/731a5d15/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/SentryAlreadyExistsException.java ---------------------------------------------------------------------- diff --git a/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/SentryAlreadyExistsException.java b/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/SentryAlreadyExistsException.java index f29c42f..15fab4d 100644 --- a/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/SentryAlreadyExistsException.java +++ b/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/SentryAlreadyExistsException.java @@ -19,8 +19,9 @@ package org.apache.sentry.core.common.exception; public class SentryAlreadyExistsException extends SentryUserException { private static final long serialVersionUID = 1298632655835L; + private static final String ExceptionSpecificMsg = " already exists"; public SentryAlreadyExistsException(String msg) { - super(msg); + super(msg+ExceptionSpecificMsg); } public SentryAlreadyExistsException(String msg, String reason) { super(msg, reason); http://git-wip-us.apache.org/repos/asf/sentry/blob/731a5d15/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/SentryNoSuchObjectException.java ---------------------------------------------------------------------- diff --git a/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/SentryNoSuchObjectException.java b/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/SentryNoSuchObjectException.java index 70719e4..fb7d8b1 100644 --- a/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/SentryNoSuchObjectException.java +++ b/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/SentryNoSuchObjectException.java @@ -19,8 +19,9 @@ package org.apache.sentry.core.common.exception; public class SentryNoSuchObjectException extends SentryUserException { private static final long serialVersionUID = 2962080655835L; + private static final String ExceptionSpecificMsg = " doesn't exist"; public SentryNoSuchObjectException(String msg) { - super(msg); + super(msg+ExceptionSpecificMsg); } public SentryNoSuchObjectException(String msg, String reason) { super(msg, reason); http://git-wip-us.apache.org/repos/asf/sentry/blob/731a5d15/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java index 0c5d019..ed47441 100644 --- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java +++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java @@ -123,7 +123,7 @@ public class DelegateSentryStore implements SentryStoreLayer { String trimmedRole = toTrimmedLower(role); MSentryRole mRole = getRole(trimmedRole, pm); if (mRole == null) { - throw new SentryNoSuchObjectException("Role: " + trimmedRole + " doesn't exist"); + throw new SentryNoSuchObjectException("Role: " + trimmedRole); } // check with grant option @@ -146,7 +146,7 @@ public class DelegateSentryStore implements SentryStoreLayer { String trimmedRole = toTrimmedLower(role); MSentryRole mRole = getRole(trimmedRole, pm); if (mRole == null) { - throw new SentryNoSuchObjectException("Role: " + trimmedRole + " doesn't exist"); + throw new SentryNoSuchObjectException("Role: " + trimmedRole); } // check with grant option http://git-wip-us.apache.org/repos/asf/sentry/blob/731a5d15/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java index 38f68cd..a5a835e 100644 --- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java +++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java @@ -1882,7 +1882,7 @@ public class SentryStore { .execute(); pm.retrieveAll(mSentryVersions); if (mSentryVersions.isEmpty()) { - throw new SentryNoSuchObjectException("No matching version found"); + throw new SentryNoSuchObjectException("Matching Version"); } if (mSentryVersions.size() > 1) { throw new SentryAccessDeniedException( @@ -3082,7 +3082,7 @@ public class SentryStore { * @return SentryNoSuchObjectException with appropriate message */ private static SentryNoSuchObjectException noSuchRole(String roleName) { - return new SentryNoSuchObjectException("nonexistent role " + roleName); + return new SentryNoSuchObjectException("Role " + roleName); } /** @@ -3091,7 +3091,8 @@ public class SentryStore { * @return SentryNoSuchObjectException with appropriate message */ private static SentryNoSuchObjectException noSuchGroup(String groupName) { - return new SentryNoSuchObjectException("nonexistent group + " + groupName); + return new SentryNoSuchObjectException("Group " + groupName); + } /** http://git-wip-us.apache.org/repos/asf/sentry/blob/731a5d15/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java index 2ba3d38..ee2a466 100644 --- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java +++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java @@ -205,7 +205,7 @@ public class SentryPolicyStoreProcessor implements SentryPolicyService.Iface { } catch (SentryAlreadyExistsException e) { String msg = "Role: " + request + " already exists."; LOGGER.error(msg, e); - response.setStatus(Status.AlreadyExists(msg, e)); + response.setStatus(Status.AlreadyExists(e.getMessage(), e)); } catch (SentryAccessDeniedException e) { LOGGER.error(e.getMessage(), e); response.setStatus(Status.AccessDenied(e.getMessage(), e)); http://git-wip-us.apache.org/repos/asf/sentry/blob/731a5d15/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java index 34302b3..d4bf435 100644 --- a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java +++ b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java @@ -138,19 +138,19 @@ public class TestSentryGenericPolicyProcessor extends org.junit.Assert { public void testOperationWithException() throws Exception { String roleName = anyString(); Mockito.when(mockStore.createRole(anyString(), roleName, anyString())) - .thenThrow(new SentryAlreadyExistsException("Role: " + roleName + " already exists")); + .thenThrow(new SentryAlreadyExistsException("Role: " + roleName)); roleName = anyString(); Mockito.when(mockStore.dropRole(anyString(), roleName, anyString())) - .thenThrow(new SentryNoSuchObjectException("Role: " + roleName + " doesn't exist")); + .thenThrow(new SentryNoSuchObjectException("Role: " + roleName )); roleName = anyString(); Mockito.when(mockStore.alterRoleAddGroups(anyString(), roleName, anySetOf(String.class),anyString())) - .thenThrow(new SentryNoSuchObjectException("Role: " + roleName + " doesn't exist")); + .thenThrow(new SentryNoSuchObjectException("Role: " + roleName)); roleName = anyString(); Mockito.when(mockStore.alterRoleDeleteGroups(anyString(), roleName, anySetOf(String.class), anyString())) - .thenThrow(new SentryNoSuchObjectException("Role: " + roleName + " doesn't exist")); + .thenThrow(new SentryNoSuchObjectException("Role: " + roleName)); roleName = anyString(); Mockito.when(mockStore.alterRoleGrantPrivilege(anyString(), roleName, any(PrivilegeObject.class), anyString())) http://git-wip-us.apache.org/repos/asf/sentry/blob/731a5d15/sentry-tests/sentry-tests-hive-v2/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java ---------------------------------------------------------------------- diff --git a/sentry-tests/sentry-tests-hive-v2/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java b/sentry-tests/sentry-tests-hive-v2/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java index 4d0c4b5..667e02e 100644 --- a/sentry-tests/sentry-tests-hive-v2/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java +++ b/sentry-tests/sentry-tests-hive-v2/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java @@ -1793,7 +1793,7 @@ public class TestHDFSIntegration { // In case of duplicate acl exist, exception should be thrown. if (acls.containsKey(ent.getName())) { - throw new SentryAlreadyExistsException("The acl " + ent.getName() + " already exists.\n"); + throw new SentryAlreadyExistsException("The acl " + ent.getName()); } else { acls.put(ent.getName(), ent.getPermission()); }
