Repository: sentry Updated Branches: refs/heads/master 12429f125 -> 374d55a62
SENTRY-2441: When MAuthzPathsMapping is deleted all associated MPaths should be deleted automatically.. (Kalyan Kumar Kalvagadda reviewed by Arjun Mishra and Na Li) Project: http://git-wip-us.apache.org/repos/asf/sentry/repo Commit: http://git-wip-us.apache.org/repos/asf/sentry/commit/374d55a6 Tree: http://git-wip-us.apache.org/repos/asf/sentry/tree/374d55a6 Diff: http://git-wip-us.apache.org/repos/asf/sentry/diff/374d55a6 Branch: refs/heads/master Commit: 374d55a62f402c375779163ad3fe0aff0753fe83 Parents: 12429f1 Author: Kalyan Kumar Kalvagadda <[email protected]> Authored: Wed Nov 7 11:13:28 2018 -0600 Committer: Kalyan Kumar Kalvagadda <[email protected]> Committed: Wed Nov 7 11:13:28 2018 -0600 ---------------------------------------------------------------------- .../provider/db/service/model/package.jdo | 5 ++- .../db/service/persistent/SentryStore.java | 7 ++-- .../db/service/persistent/TestSentryStore.java | 37 ++++++++++++++++++++ 3 files changed, 43 insertions(+), 6 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/sentry/blob/374d55a6/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo ---------------------------------------------------------------------- diff --git a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo index 20ec0de..e3ae24b 100644 --- a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo +++ b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo @@ -274,7 +274,10 @@ <column name="CREATE_TIME_MS" jdbc-type="BIGINT"/> </field> <field name = "paths"> - <collection element-type="org.apache.sentry.provider.db.service.model.MPath"/> + <!-- Setting attribute dependent-element to true enables JDO cascading operations. in this case we need it to + cascade delete. + --> + <collection element-type="org.apache.sentry.provider.db.service.model.MPath" dependent-element="true"/> <element> <column name="AUTHZ_OBJ_ID"/> </element> http://git-wip-us.apache.org/repos/asf/sentry/blob/374d55a6/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 33c4061..e2d6c85 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 @@ -430,7 +430,8 @@ public class SentryStore implements SentryStoreInterface { * @param <T> Class type * @return count of objects or -1 in case of error */ - private <T> Long getCount(final Class<T> tClass) { + @VisibleForTesting + <T> Long getCount(final Class<T> tClass) { try { return tm.executeTransaction( pm -> { @@ -3500,10 +3501,6 @@ public class SentryStore implements SentryStoreInterface { MAuthzPathsMapping mAuthzPathsMapping = getMAuthzPathsMappingCore(pm, currentSnapshotID, authzObj); if (mAuthzPathsMapping != null) { - for (MPath mPath : mAuthzPathsMapping.getPaths()) { - mAuthzPathsMapping.removePath(mPath); - pm.deletePersistent(mPath); - } pm.deletePersistent(mAuthzPathsMapping); } else { LOGGER.error("nonexistent authzObj: {} on current paths snapshot ID #{}", http://git-wip-us.apache.org/repos/asf/sentry/blob/374d55a6/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java ---------------------------------------------------------------------- diff --git a/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java b/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java index 66db6ae..ca8c416 100644 --- a/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java +++ b/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java @@ -67,6 +67,7 @@ import org.apache.sentry.provider.db.service.model.MSentryPermChange; import org.apache.sentry.provider.db.service.model.MSentryPathChange; import org.apache.sentry.provider.db.service.model.MSentryPrivilege; import org.apache.sentry.provider.db.service.model.MSentryRole; +import org.apache.sentry.provider.db.service.model.MPath; import org.apache.sentry.api.service.thrift.TSentryActiveRoleSet; import org.apache.sentry.api.service.thrift.TSentryAuthorizable; import org.apache.sentry.api.service.thrift.TSentryGrantOption; @@ -2646,6 +2647,7 @@ public class TestSentryStore extends org.junit.Assert { assertEquals(1, nodeMap.size());//Tree size assertEquals(0, pathImage.size()); assertEquals(0, sentryStore.getMPaths().size()); + assertEquals(0, sentryStore.getCount(MPath.class).intValue()); // Query the persisted path change and ensure it equals to the original one lastChangeID = sentryStore.getLastProcessedPathChangeID(); @@ -2658,6 +2660,41 @@ public class TestSentryStore extends org.junit.Assert { } @Test + public void testDeleteAuthzPathsMapping() throws Exception { + + long notificationID = 0; + + // Persist an empty image so that we can add paths to it. + sentryStore.persistFullPathsImage(new HashMap<String, Collection<String>>(), notificationID); + + // Add a mapping to two paths + sentryStore.addAuthzPathsMapping("db1.table", + Sets.newHashSet("db1/tbl1", "db1/tbl2"), null); + + // Verify the Path count + assertEquals(2, sentryStore.getCount(MPath.class).intValue()); + + // Add a mapping to three paths + sentryStore.addAuthzPathsMapping("db2.table", + Sets.newHashSet("db2/tbl1", "db2/tbl2", "db2/tbl3"), null); + + + // Verify the Path count + assertEquals(5, sentryStore.getCount(MPath.class).intValue()); + + // deleting the mapping and verifying the MPath count + sentryStore.deleteAllAuthzPathsMapping("db1.table", null); + // Verify the Path count + assertEquals(3, sentryStore.getCount(MPath.class).intValue()); + + + // deleting the mapping and verifying the MPath count + sentryStore.deleteAllAuthzPathsMapping("db2.table", null); + // Verify the Path count + assertEquals(0, sentryStore.getCount(MPath.class).intValue()); + } + + @Test public void testRenameUpdateAuthzPathsMapping() throws Exception { Map<String, Collection<String>> authzPaths = new HashMap<>(); Long lastNotificationId = sentryStore.getLastProcessedNotificationID();
