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 <[email protected]>
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
