SENTRY-2441: When MAuthzPathsMapping is deleted all associated MPaths should be deleted automatically.. (Kalyan Kumar Kalvagadda reviewed by Arjun Mishra and Na Li)
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>
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 @@
* @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 @@
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 #{}",
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.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 @@
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 @@
}
@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();
