Repository: incubator-sentry Updated Branches: refs/heads/master 36125fddc -> a962e2425
SENTRY-1065: Make SentryNoSuchObjectException exception error message consistent across all files (Anne Yu via Hao Hao). Project: http://git-wip-us.apache.org/repos/asf/incubator-sentry/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-sentry/commit/a962e242 Tree: http://git-wip-us.apache.org/repos/asf/incubator-sentry/tree/a962e242 Diff: http://git-wip-us.apache.org/repos/asf/incubator-sentry/diff/a962e242 Branch: refs/heads/master Commit: a962e24254cc0ca31d18e36b18891390183f9630 Parents: 36125fd Author: Anne Yu <[email protected]> Authored: Thu Mar 3 11:55:59 2016 -0800 Committer: Anne Yu <[email protected]> Committed: Thu Mar 3 11:55:59 2016 -0800 ---------------------------------------------------------------------- .../hdfs/SentryAuthorizationProvider.java | 2 +- .../service/persistent/DelegateSentryStore.java | 6 ++-- .../thrift/SentryGenericPolicyProcessor.java | 14 ++++++--- .../db/service/persistent/SentryStore.java | 16 +++++----- .../thrift/SentryPolicyStoreProcessor.java | 8 ++--- .../persistent/TestDelegateSentryStore.java | 4 +-- .../TestSentryGenericPolicyProcessor.java | 32 ++++++++++++-------- .../hive/hiveserver/UnmanagedHiveServer.java | 2 +- .../e2e/metastore/TestMetastoreEndToEnd.java | 2 +- .../tests/e2e/sqoop/TestRoleOperation.java | 2 +- 10 files changed, 49 insertions(+), 39 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/a962e242/sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationProvider.java ---------------------------------------------------------------------- diff --git a/sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationProvider.java b/sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationProvider.java index cf85fa5..c701723 100644 --- a/sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationProvider.java +++ b/sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationProvider.java @@ -399,7 +399,7 @@ public class SentryAuthorizationProvider defaultAuthzProvider.removeAclFeature(node); } else { if (warn) { - LOG.warn("### removeAclFeature is requested on {}, but it doesn't " + + LOG.warn("### removeAclFeature is requested on {}, but it does not " + "have any acl.", node); } } http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/a962e242/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 1e497a0..74c52fa 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 @@ -122,7 +122,7 @@ public class DelegateSentryStore implements SentryStoreLayer { query.setUnique(true); MSentryRole sentryRole = (MSentryRole) query.execute(role); if (sentryRole == null) { - throw new SentryNoSuchObjectException("Role " + role); + throw new SentryNoSuchObjectException("Role: " + role + " doesn't exist"); } else { pm.retrieve(sentryRole); sentryRole.removeGMPrivileges(); @@ -168,7 +168,7 @@ public class DelegateSentryStore implements SentryStoreLayer { pm = openTransaction(); MSentryRole mRole = getRole(role, pm); if (mRole == null) { - throw new SentryNoSuchObjectException("role:" + role + " isn't exist"); + throw new SentryNoSuchObjectException("Role: " + role + " doesn't exist"); } /** * check with grant option @@ -199,7 +199,7 @@ public class DelegateSentryStore implements SentryStoreLayer { pm = openTransaction(); MSentryRole mRole = getRole(role, pm); if (mRole == null) { - throw new SentryNoSuchObjectException("role:" + role + " isn't exist"); + throw new SentryNoSuchObjectException("Role: " + role + " doesn't exist"); } /** * check with grant option http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/a962e242/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java index 613f10f..97c2e71 100644 --- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java +++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java @@ -198,20 +198,24 @@ public class SentryGenericPolicyProcessor implements SentryGenericPolicyService. try { response = handler.handle(); } catch (SentryAccessDeniedException e) { - LOGGER.error(e.getMessage(), e); + String msg = "Sentry access denied: " + e.getMessage(); + LOGGER.error(msg, e); response.status = Status.AccessDenied(e.getMessage(), e); } catch (SentryAlreadyExistsException e) { - LOGGER.error(e.getMessage(), e); + String msg = "Sentry object already exists: " + e.getMessage(); + LOGGER.error(msg, e); response.status = Status.AlreadyExists(e.getMessage(), e); } catch (SentryNoSuchObjectException e) { - LOGGER.error(e.getMessage(), e); + String msg = "Sentry object doesn't exist: " + e.getMessage(); + LOGGER.error(msg, e); response.status = Status.NoSuchObject(e.getMessage(), e); } catch (SentryInvalidInputException e) { - String msg = "Invalid input privilege object"; + String msg = "Invalid input privilege object: " + e.getMessage(); LOGGER.error(msg, e); response.status = Status.InvalidInput(msg, e); } catch (SentryThriftAPIMismatchException e) { - LOGGER.error(e.getMessage(), e); + String msg = "Sentry thrift API mismatch error: " + e.getMessage(); + LOGGER.error(msg, e); response.status = Status.THRIFT_VERSION_MISMATCH(e.getMessage(), e); } catch (Exception e) { String msg = "Unknown error:" + e.getMessage(); http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/a962e242/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 9cebe1e..c5c5ffb 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 @@ -132,7 +132,7 @@ public class SentryStore { prop.putAll(ServerConfig.SENTRY_STORE_DEFAULTS); String jdbcUrl = conf.get(ServerConfig.SENTRY_STORE_JDBC_URL, "").trim(); Preconditions.checkArgument(!jdbcUrl.isEmpty(), "Required parameter " + - ServerConfig.SENTRY_STORE_JDBC_URL + " missing"); + ServerConfig.SENTRY_STORE_JDBC_URL + " is missed"); String user = conf.get(ServerConfig.SENTRY_STORE_JDBC_USER, ServerConfig. SENTRY_STORE_JDBC_USER_DEFAULT).trim(); //Password will be read from Credential provider specified using property @@ -446,7 +446,7 @@ public class SentryStore { MSentryPrivilege mPrivilege = null; MSentryRole mRole = getMSentryRole(pm, roleName); if (mRole == null) { - throw new SentryNoSuchObjectException("Role: " + roleName); + throw new SentryNoSuchObjectException("Role: " + roleName + " doesn't exist"); } else { if (!isNULL(privilege.getColumnName()) || !isNULL(privilege.getTableName()) @@ -537,7 +537,7 @@ public class SentryStore { query.setUnique(true); MSentryRole mRole = (MSentryRole) query.execute(roleName); if (mRole == null) { - throw new SentryNoSuchObjectException("Role: " + roleName); + throw new SentryNoSuchObjectException("Role: " + roleName + " doesn't exist"); } else { query = pm.newQuery(MSentryPrivilege.class); MSentryPrivilege mPrivilege = getMSentryPrivilege(tPrivilege, pm); @@ -801,7 +801,7 @@ public class SentryStore { query.setUnique(true); MSentryRole sentryRole = (MSentryRole) query.execute(lRoleName); if (sentryRole == null) { - throw new SentryNoSuchObjectException("Role " + lRoleName); + throw new SentryNoSuchObjectException("Role: " + lRoleName + " doesn't exist"); } else { pm.retrieve(sentryRole); int numPrivs = sentryRole.getPrivileges().size(); @@ -840,7 +840,7 @@ public class SentryStore { query.setUnique(true); MSentryRole role = (MSentryRole) query.execute(lRoleName); if (role == null) { - throw new SentryNoSuchObjectException("Role: " + lRoleName); + throw new SentryNoSuchObjectException("Role: " + lRoleName + " doesn't exist"); } else { query = pm.newQuery(MSentryGroup.class); query.setFilter("this.groupName == t"); @@ -874,7 +874,7 @@ public class SentryStore { query.setUnique(true); MSentryRole role = (MSentryRole) query.execute(roleName); if (role == null) { - throw new SentryNoSuchObjectException("Role: " + roleName); + throw new SentryNoSuchObjectException("Role: " + roleName + " doesn't exist"); } else { query = pm.newQuery(MSentryGroup.class); query.setFilter("this.groupName == t"); @@ -915,7 +915,7 @@ public class SentryStore { query.setUnique(true); MSentryRole sentryRole = (MSentryRole) query.execute(roleName); if (sentryRole == null) { - throw new SentryNoSuchObjectException("Role " + roleName); + throw new SentryNoSuchObjectException("Role: " + roleName + " doesn't exist"); } else { pm.retrieve(sentryRole); } @@ -1165,7 +1165,7 @@ public class SentryStore { query.setUnique(true); sentryGroup = (MSentryGroup) query.execute(groupName); if (sentryGroup == null) { - throw new SentryNoSuchObjectException("Group " + groupName); + throw new SentryNoSuchObjectException("Group: " + groupName + " doesn't exist"); } else { pm.retrieve(sentryGroup); } http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/a962e242/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 82bfca5..8881d82 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 @@ -292,7 +292,7 @@ public class SentryPolicyStoreProcessor implements SentryPolicyService.Iface { plugin.onAlterSentryRoleGrantPrivilege(request); } } catch (SentryNoSuchObjectException e) { - String msg = "Role: " + request.getRoleName() + " doesn't exist."; + String msg = "Role: " + request.getRoleName() + " doesn't exist"; LOGGER.error(msg, e); response.setStatus(Status.NoSuchObject(msg, e)); } catch (SentryInvalidInputException e) { @@ -420,7 +420,7 @@ public class SentryPolicyStoreProcessor implements SentryPolicyService.Iface { plugin.onDropSentryRole(request); } } catch (SentryNoSuchObjectException e) { - String msg = "Role :" + request + " does not exist."; + String msg = "Role :" + request + " doesn't exist"; LOGGER.error(msg, e); response.setStatus(Status.NoSuchObject(msg, e)); } catch (SentryAccessDeniedException e) { @@ -466,7 +466,7 @@ public class SentryPolicyStoreProcessor implements SentryPolicyService.Iface { plugin.onAlterSentryRoleAddGroups(request); } } catch (SentryNoSuchObjectException e) { - String msg = "Role: " + request + " does not exist."; + String msg = "Role: " + request + " doesn't exist"; LOGGER.error(msg, e); response.setStatus(Status.NoSuchObject(msg, e)); } catch (SentryAccessDeniedException e) { @@ -571,7 +571,7 @@ public class SentryPolicyStoreProcessor implements SentryPolicyService.Iface { response.setStatus(Status.OK()); } catch (SentryNoSuchObjectException e) { response.setRoles(roleSet); - String msg = "Role: " + request + " couldn't be retrieved."; + String msg = "Request: " + request + " couldn't be completed, message: " + e.getMessage(); LOGGER.error(msg, e); response.setStatus(Status.NoSuchObject(msg, e)); } catch (SentryAccessDeniedException e) { http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/a962e242/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestDelegateSentryStore.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestDelegateSentryStore.java b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestDelegateSentryStore.java index 751bc3f..b3822fc 100644 --- a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestDelegateSentryStore.java +++ b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestDelegateSentryStore.java @@ -59,7 +59,7 @@ public class TestDelegateSentryStore extends SentryStoreIntegrationBase{ sentryStore.createRole(SEARCH, roleName1, grantor); try { sentryStore.createRole(SEARCH, roleName2, grantor); - fail("SentryAlreadyExistsException should have been thrown"); + fail("Fail to throw SentryAlreadyExistsException"); } catch (SentryAlreadyExistsException e) { //ignore the exception } @@ -67,7 +67,7 @@ public class TestDelegateSentryStore extends SentryStoreIntegrationBase{ try { sentryStore.dropRole(SEARCH, roleName2, grantor); } catch (SentryNoSuchObjectException e) { - fail("SentryNoSuchObjectException shouldn't have been thrown"); + fail("Shouldn't throw SentryNoSuchObjectException"); } } http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/a962e242/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 6821cf9..48c45ce 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 @@ -165,26 +165,32 @@ public class TestSentryGenericPolicyProcessor { @Test public void testOperationWithException() throws Exception { - when(mockStore.createRole(anyString(), anyString(), anyString())) - .thenThrow(new SentryAlreadyExistsException("role already exists")); + String roleName = anyString(); + when(mockStore.createRole(anyString(), roleName, anyString())) + .thenThrow(new SentryAlreadyExistsException("Role: " + roleName + " already exists")); - when(mockStore.dropRole(anyString(), anyString(), anyString())) - .thenThrow(new SentryNoSuchObjectException("role isn't exist")); + roleName = anyString(); + when(mockStore.dropRole(anyString(), roleName, anyString())) + .thenThrow(new SentryNoSuchObjectException("Role: " + roleName + " doesn't exist")); - when(mockStore.alterRoleAddGroups(anyString(), anyString(), anySetOf(String.class),anyString())) - .thenThrow(new SentryNoSuchObjectException("role isn't exist")); + roleName = anyString(); + when(mockStore.alterRoleAddGroups(anyString(), roleName, anySetOf(String.class),anyString())) + .thenThrow(new SentryNoSuchObjectException("Role: " + roleName + " doesn't exist")); - when(mockStore.alterRoleDeleteGroups(anyString(), anyString(),anySetOf(String.class), anyString())) - .thenThrow(new SentryNoSuchObjectException("role isn't exist")); + roleName = anyString(); + when(mockStore.alterRoleDeleteGroups(anyString(), roleName, anySetOf(String.class), anyString())) + .thenThrow(new SentryNoSuchObjectException("Role: " + roleName + " doesn't exist")); - when(mockStore.alterRoleGrantPrivilege(anyString(), anyString(), any(PrivilegeObject.class), anyString())) - .thenThrow(new SentryGrantDeniedException("has no grant")); + roleName = anyString(); + when(mockStore.alterRoleGrantPrivilege(anyString(), roleName, any(PrivilegeObject.class), anyString())) + .thenThrow(new SentryGrantDeniedException("Role: " + roleName + " is not allowed to do grant")); - when(mockStore.alterRoleRevokePrivilege(anyString(), anyString(),any(PrivilegeObject.class), anyString())) - .thenThrow(new SentryGrantDeniedException("has no grant")); + roleName = anyString(); + when(mockStore.alterRoleRevokePrivilege(anyString(), roleName, any(PrivilegeObject.class), anyString())) + .thenThrow(new SentryGrantDeniedException("Role: " + roleName + " is not allowed to do grant")); when(mockStore.dropPrivilege(anyString(), any(PrivilegeObject.class), anyString())) - .thenThrow(new SentryInvalidInputException("nvalid input privilege object")); + .thenThrow(new SentryInvalidInputException("Invalid input privilege object")); when(mockStore.renamePrivilege(anyString(), anyString(), anyListOf(Authorizable.class), anyListOf(Authorizable.class), anyString())) http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/a962e242/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/UnmanagedHiveServer.java ---------------------------------------------------------------------- diff --git a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/UnmanagedHiveServer.java b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/UnmanagedHiveServer.java index e8b3a2a..beae8e8 100644 --- a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/UnmanagedHiveServer.java +++ b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/UnmanagedHiveServer.java @@ -60,7 +60,7 @@ public class UnmanagedHiveServer implements HiveServer { }else { val = System.getProperty(hiveVar, defaultVal); } - Preconditions.checkNotNull(val, "Required system property missing: Provide it using -D"+ hiveVar); + Preconditions.checkNotNull(val, "Required system property is missed: Provide it using -D"+ hiveVar); LOGGER.info("Using from system property" + hiveVar + " = " + val ); }else { LOGGER.info("Using from hive-site.xml" + hiveVar + " = " + val ); http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/a962e242/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestMetastoreEndToEnd.java ---------------------------------------------------------------------- diff --git a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestMetastoreEndToEnd.java b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestMetastoreEndToEnd.java index 1fac8b0..1b3240f 100644 --- a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestMetastoreEndToEnd.java +++ b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestMetastoreEndToEnd.java @@ -584,7 +584,7 @@ public class TestMetastoreEndToEnd extends execHiveSQL("ALTER TABLE " + dbName + "." + tabName1 + " ADD PARTITION (part_col ='" + partVal2 + "') location '" + tabDir1 + "'", USER2_1); - fail("alter table should have failed due to missing URI privilege"); + fail("alter table should have failed due to URI privilege missed"); } catch (IOException e) { // Expected error } http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/a962e242/sentry-tests/sentry-tests-sqoop/src/test/java/org/apache/sentry/tests/e2e/sqoop/TestRoleOperation.java ---------------------------------------------------------------------- diff --git a/sentry-tests/sentry-tests-sqoop/src/test/java/org/apache/sentry/tests/e2e/sqoop/TestRoleOperation.java b/sentry-tests/sentry-tests-sqoop/src/test/java/org/apache/sentry/tests/e2e/sqoop/TestRoleOperation.java index 1a6ca02..d47f0ad 100644 --- a/sentry-tests/sentry-tests-sqoop/src/test/java/org/apache/sentry/tests/e2e/sqoop/TestRoleOperation.java +++ b/sentry-tests/sentry-tests-sqoop/src/test/java/org/apache/sentry/tests/e2e/sqoop/TestRoleOperation.java @@ -80,7 +80,7 @@ public class TestRoleOperation extends AbstractSqoopSentryTestBase { SqoopClient client = sqoopServerRunner.getSqoopClient(ADMIN_USER); try { client.dropRole(new MRole("drop_noexisted_role_1")); - fail("expected SentryNoSuchObjectException happend"); + fail("expect SentryNoSuchObjectException to throw"); } catch (Exception e) { assertCausedMessage(e, "SentryNoSuchObjectException"); }
