Repository: sentry Updated Branches: refs/heads/master 5f850cc8a -> 6f4a88bd1
SENTRY-1546: Generic Policy provides bad error messages for Sentry exceptions Project: http://git-wip-us.apache.org/repos/asf/sentry/repo Commit: http://git-wip-us.apache.org/repos/asf/sentry/commit/6f4a88bd Tree: http://git-wip-us.apache.org/repos/asf/sentry/tree/6f4a88bd Diff: http://git-wip-us.apache.org/repos/asf/sentry/diff/6f4a88bd Branch: refs/heads/master Commit: 6f4a88bd112260866b6140475676b5f1ab74f7f9 Parents: 5f850cc Author: Alexander Kolbasov <[email protected]> Authored: Mon Mar 6 13:38:40 2017 -0800 Committer: Alexander Kolbasov <[email protected]> Committed: Mon Mar 6 13:39:02 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 +- .../sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java | 2 +- 8 files changed, 17 insertions(+), 14 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/sentry/blob/6f4a88bd/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/6f4a88bd/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/6f4a88bd/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java ---------------------------------------------------------------------- diff --git a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java index 27dbfca..fc84e23 100644 --- a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java +++ b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java @@ -125,7 +125,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 @@ -149,7 +149,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/6f4a88bd/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java ---------------------------------------------------------------------- diff --git a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java index 6d54748..32637d2 100644 --- a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java +++ b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java @@ -1530,7 +1530,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( @@ -2566,7 +2566,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); } /** @@ -2575,7 +2575,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/6f4a88bd/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java ---------------------------------------------------------------------- diff --git a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java index 2ebdf81..e9d9eec 100644 --- a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java +++ b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java @@ -225,7 +225,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/6f4a88bd/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java ---------------------------------------------------------------------- diff --git a/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java b/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java index 3881c11..de66313 100644 --- a/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java +++ b/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java @@ -134,19 +134,19 @@ public class TestSentryGenericPolicyProcessor extends org.junit.Assert { public void testOperationWithException() throws Exception { String roleName = Matchers.anyString(); Mockito.when(mockStore.createRole(Matchers.anyString(), roleName, Matchers.anyString())) - .thenThrow(new SentryAlreadyExistsException("Role: " + roleName + " already exists")); + .thenThrow(new SentryAlreadyExistsException("Role: " + roleName)); roleName = Matchers.anyString(); Mockito.when(mockStore.dropRole(Matchers.anyString(), roleName, Matchers.anyString())) - .thenThrow(new SentryNoSuchObjectException("Role: " + roleName + " doesn't exist")); + .thenThrow(new SentryNoSuchObjectException("Role: " + roleName )); roleName = Matchers.anyString(); Mockito.when(mockStore.alterRoleAddGroups(Matchers.anyString(), roleName, Matchers.anySetOf(String.class), Matchers.anyString())) - .thenThrow(new SentryNoSuchObjectException("Role: " + roleName + " doesn't exist")); + .thenThrow(new SentryNoSuchObjectException("Role: " + roleName)); roleName = Matchers.anyString(); Mockito.when(mockStore.alterRoleDeleteGroups(Matchers.anyString(), roleName, Matchers.anySetOf(String.class), Matchers.anyString())) - .thenThrow(new SentryNoSuchObjectException("Role: " + roleName + " doesn't exist")); + .thenThrow(new SentryNoSuchObjectException("Role: " + roleName)); roleName = Matchers.anyString(); Mockito.when(mockStore.alterRoleGrantPrivilege(Matchers.anyString(), roleName, Matchers.any(PrivilegeObject.class), Matchers.anyString())) http://git-wip-us.apache.org/repos/asf/sentry/blob/6f4a88bd/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 9b986d7..e4acb7e 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 @@ -1790,7 +1790,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()); } http://git-wip-us.apache.org/repos/asf/sentry/blob/6f4a88bd/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java ---------------------------------------------------------------------- diff --git a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java index 0239388..bca9c6f 100644 --- a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java +++ b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java @@ -257,7 +257,7 @@ public abstract class TestHDFSIntegrationBase { // 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()); }
