sentry-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From kal...@apache.org
Subject [sentry] branch master updated: SENTRY-2493: Sentry store api's for path mapping should handle empty/null paths.(Kalyan Kumar Kalvagadda reviewed by Arjun Mishra)
Date Thu, 07 Feb 2019 13:55:08 GMT
This is an automated email from the ASF dual-hosted git repository.

kalyan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sentry.git


The following commit(s) were added to refs/heads/master by this push:
     new 8701ac1  SENTRY-2493: Sentry store api's for path mapping should handle empty/null
paths.(Kalyan Kumar Kalvagadda reviewed by Arjun Mishra)
8701ac1 is described below

commit 8701ac1d256505d8bc67ac3f8d4aaa5766bc2cd3
Author: Kalyan Kumar Kalvagadda <kkalyan@cloudera.com>
AuthorDate: Thu Feb 7 07:54:41 2019 -0600

    SENTRY-2493: Sentry store api's for path mapping should handle empty/null paths.(Kalyan
Kumar Kalvagadda reviewed by Arjun Mishra)
    
    Change-Id: I4e6143878febb429864331dcb5219e46a66f4491
---
 .../db/service/model/MAuthzPathsMapping.java       | 15 ++++---
 .../service/persistent/NotificationProcessor.java  | 12 +++++-
 .../db/service/persistent/QueryParamBuilder.java   |  5 ++-
 .../db/service/persistent/SentryStore.java         |  5 ++-
 .../service/persistent/SentryStoreInterface.java   |  2 +-
 .../db/service/persistent/TestSentryStore.java     | 46 ++++++++++++++++++++++
 6 files changed, 73 insertions(+), 12 deletions(-)

diff --git a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java
b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java
index 95f4b4b..a4190f5 100644
--- a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java
+++ b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java
@@ -58,9 +58,11 @@ public class MAuthzPathsMapping {
     this.authzObjectId = authzObjectId;
     this.authzObjName = MSentryUtil.safeIntern(authzObjName);
     this.pathsPersisted = Collections.EMPTY_SET;
-    this.pathsToPersist = new HashSet<>(paths.size());
-    for(String path : paths) {
-      this.pathsToPersist.add(path);
+    if(paths != null && !paths.isEmpty()) {
+      this.pathsToPersist = new HashSet<>(paths.size());
+      for (String path : paths) {
+        this.pathsToPersist.add(path);
+      }
     }
     this.createTimeMs = System.currentTimeMillis();
   }
@@ -90,7 +92,7 @@ public class MAuthzPathsMapping {
   }
 
   public void addPathToPersist(Collection<String> paths) {
-    if(paths == null || paths.size() == 0) {
+    if(paths == null || paths.isEmpty()) {
       return;
     }
     if(pathsToPersist == null) {
@@ -176,6 +178,9 @@ public class MAuthzPathsMapping {
    */
   public void makePersistent(PersistenceManager pm) {
     pm.makePersistent(this);
+    if(pathsToPersist == null || pathsToPersist.isEmpty()) {
+      return;
+    }
     for (String path : pathsToPersist) {
       pm.makePersistent(new MPathToPersist(authzObjectId, path));
     }
@@ -187,7 +192,7 @@ public class MAuthzPathsMapping {
    * @param pm Persistence manager instance
    * @param paths paths to be removed.
    */
-  public void deletePersistent(PersistenceManager pm, Iterable<String> paths) {
+  public void deletePersistent(PersistenceManager pm, Collection<String> paths) {
     Query query = pm.newQuery(MPathToPersist.class);
     QueryParamBuilder paramBuilder = QueryParamBuilder.addPathFilter(null, paths).addObject("authzObjectId",
         authzObjectId);
diff --git a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/NotificationProcessor.java
b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/NotificationProcessor.java
index 3a61b52..377c673 100644
--- a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/NotificationProcessor.java
+++ b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/NotificationProcessor.java
@@ -573,7 +573,11 @@ final class NotificationProcessor {
         paths.add(pathTree);
       }
     }
-    sentryStore.addAuthzPathsMapping(authzObj, paths, update);
+    if(!paths.isEmpty()) {
+      sentryStore.addAuthzPathsMapping(authzObj, paths, update);
+    } else {
+      LOGGER.info("Received empty paths for {}, not updating sentry store", authzObj);
+    }
   }
 
   /**
@@ -609,7 +613,11 @@ final class NotificationProcessor {
         paths.add(pathTree);
       }
     }
-    sentryStore.deleteAuthzPathsMapping(authzObj, paths, update);
+    if(!paths.isEmpty()) {
+      sentryStore.deleteAuthzPathsMapping(authzObj, paths, update);
+    } else {
+      LOGGER.info("Received empty paths for {}, not updating sentry store", authzObj);
+    }
   }
 
   /**
diff --git a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/QueryParamBuilder.java
b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/QueryParamBuilder.java
index 11c208d..240120c 100644
--- a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/QueryParamBuilder.java
+++ b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/QueryParamBuilder.java
@@ -31,6 +31,7 @@ import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.Collection;
 import java.util.concurrent.atomic.AtomicLong;
 
 /**
@@ -352,11 +353,11 @@ public class QueryParamBuilder {
    * @return paramBuilder supplied or a new one if the supplied one is null.
    */
   public static QueryParamBuilder addPathFilter(QueryParamBuilder paramBuilder,
-                                                 Iterable<String> paths) {
+                                                 Collection<String> paths) {
     if (paramBuilder == null) {
       paramBuilder = new QueryParamBuilder();
     }
-    if (paths == null) {
+    if (paths == null || paths.isEmpty()) {
       return paramBuilder;
     }
     paramBuilder.newChild().addSet("this.path == ", paths, false);
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 e031ed4..0b17867 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
@@ -624,6 +624,7 @@ public class SentryStore implements SentryStoreInterface {
    * Removes all the information related to HMS Objects from sentry store.
    */
   public void clearHmsPathInformation() throws Exception {
+    LOGGER.info("Clearing all the Path information");
     tm.executeTransactionWithRetry(
             pm -> {
               // Data in MAuthzPathsSnapshotId.class is not cleared intentionally.
@@ -3448,7 +3449,7 @@ public class SentryStore implements SentryStoreInterface {
    * @param paths a set of paths need to be deleted from the authzObj -> [Paths] mapping
    * @param update the corresponding path delta update
    */
-  public void deleteAuthzPathsMapping(final String authzObj, final Iterable<String>
paths,
+  public void deleteAuthzPathsMapping(final String authzObj, final Collection<String>
paths,
       final UniquePathsUpdate update) throws Exception {
     execute(update, pm -> {
       pm.setDetachAllOnCommit(false); // No need to detach objects
@@ -3466,7 +3467,7 @@ public class SentryStore implements SentryStoreInterface {
    * @throws SentryNoSuchObjectException if cannot find the existing authzObj or path.
    */
   private void deleteAuthzPathsMappingCore(PersistenceManager pm, String authzObj,
-                                           Iterable<String> paths) {
+                                           Collection<String> paths) {
     long currentSnapshotID = getCurrentAuthzPathsSnapshotID(pm);
     if (currentSnapshotID <= EMPTY_PATHS_SNAPSHOT_ID) {
       LOGGER.error("No paths snapshot ID is found. Cannot delete authzoObj: {}", authzObj);
diff --git a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreInterface.java
b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreInterface.java
index 84cb868..77cafa9 100644
--- a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreInterface.java
+++ b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreInterface.java
@@ -723,7 +723,7 @@ public interface SentryStoreInterface {
    * @param update the corresponding path delta update
    */
   void deleteAuthzPathsMapping(final String authzObj,
-                               final Iterable<String> paths,
+                               final Collection<String> paths,
                                final UniquePathsUpdate update) throws Exception;
 
   /**
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 62d6ea8..38b4c87 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
@@ -2703,6 +2703,34 @@ public class TestSentryStore extends org.junit.Assert {
     assertEquals(paths.size(), 4);
     assertTrue(paths.stream().anyMatch(x -> x.getPath().equalsIgnoreCase("/hive/db1/tb1/par2")));
     assertTrue(paths.stream().anyMatch(x -> x.getPath().equalsIgnoreCase("/hive/db1/tb1/par3")));
+
+    //Add null paths to an existing mapping and verify that there are no exceptions
+    try {
+      sentryStore.addAuthzPathsMapping("db1.tb1", null, null);
+    } catch (Exception e) {
+      fail("Exception occurred while adding mapping with null paths");
+    }
+
+    //Add new mapping with null paths and verify that there are no exceptions
+    try {
+      sentryStore.addAuthzPathsMapping("db1.tb5", null, null);
+    } catch (Exception e) {
+      fail("Exception occurred while adding mapping with null paths");
+    }
+
+    //Add empty paths collection to existing mapping verify that there are no exceptions
+    try {
+      sentryStore.addAuthzPathsMapping("db1.tb6", Collections.EMPTY_SET, null);
+    } catch (Exception e) {
+      fail("Exception occurred while adding mapping with empty path collection");
+    }
+
+    //Add new mapping with empty paths collection and verify that there are no exceptions
+    try {
+      sentryStore.addAuthzPathsMapping("db1.tb1", Collections.EMPTY_SET, null);
+    } catch (Exception e) {
+      fail("Exception occurred while adding mapping with empty path collection");
+    }
   }
 
   @Test
@@ -2920,6 +2948,24 @@ public class TestSentryStore extends org.junit.Assert {
     sentryStore.deleteAllAuthzPathsMapping("db2.table", null);
     // Verify the Path count
     assertEquals(0, sentryStore.getCount(MPath.class).intValue());
+
+    // Add a mapping to two paths
+    sentryStore.addAuthzPathsMapping("db1.table",
+            Sets.newHashSet("db1/tbl1", "db1/tbl2"), null);
+    // deleting the mapping with paths as null and verifying that all the paths are deleted.
+    sentryStore.deleteAuthzPathsMapping("db1.table", null, null);
+    // Verify the Path count
+    assertEquals(0, sentryStore.getCount(MPath.class).intValue());
+
+    // Add a mapping to two paths
+    sentryStore.addAuthzPathsMapping("db1.table",
+            Sets.newHashSet("db1/tbl1", "db1/tbl2"), null);
+    // deleting the mapping with paths as empty set and verifying that all the paths are
deleted.
+    sentryStore.deleteAuthzPathsMapping("db1.table", Collections.EMPTY_SET, null);
+    // Verify the Path count
+    assertEquals(0, sentryStore.getCount(MPath.class).intValue());
+
+
   }
 
   @Test


Mime
View raw message