sentry-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ak...@apache.org
Subject sentry git commit: SENTRY-1625: PrivilegeOperatePersistence can use QueryParamBuilder (Alex Kolbasov, Reviewed by: Hao Hao abd Vadim Spector)
Date Sat, 25 Feb 2017 02:15:52 GMT
Repository: sentry
Updated Branches:
  refs/heads/sentry-ha-redesign b596fe9b8 -> 71f476985


SENTRY-1625: PrivilegeOperatePersistence can use QueryParamBuilder (Alex Kolbasov, Reviewed by: Hao Hao abd Vadim Spector)


Project: http://git-wip-us.apache.org/repos/asf/sentry/repo
Commit: http://git-wip-us.apache.org/repos/asf/sentry/commit/71f47698
Tree: http://git-wip-us.apache.org/repos/asf/sentry/tree/71f47698
Diff: http://git-wip-us.apache.org/repos/asf/sentry/diff/71f47698

Branch: refs/heads/sentry-ha-redesign
Commit: 71f47698586916f2f4e3713346d10cf41485284e
Parents: b596fe9
Author: Alexander Kolbasov <akolb@cloudera.com>
Authored: Fri Feb 24 16:33:22 2017 -0800
Committer: Alexander Kolbasov <akolb@cloudera.com>
Committed: Fri Feb 24 16:33:22 2017 -0800

----------------------------------------------------------------------
 .../persistent/PrivilegeOperatePersistence.java | 195 ++++++---
 .../db/service/model/MSentryGMPrivilege.java    |  69 +---
 .../provider/db/service/model/MSentryRole.java  |   6 +-
 .../service/persistent/QueryParamBuilder.java   | 406 +++++++++++++++++++
 .../db/service/persistent/SentryStore.java      | 377 +++--------------
 .../db/service/persistent/TestSentryStore.java  |  19 +-
 6 files changed, 610 insertions(+), 462 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/sentry/blob/71f47698/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java
index fa9dadf..37484ed 100644
--- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java
+++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java
@@ -19,7 +19,8 @@ package org.apache.sentry.provider.db.generic.service.persistent;
 
 import java.lang.reflect.Constructor;
 import java.util.ArrayList;
-import java.util.LinkedList;
+import java.util.Collections;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -40,19 +41,29 @@ import org.apache.sentry.provider.db.generic.service.persistent.PrivilegeObject.
 import org.apache.sentry.provider.db.service.model.MSentryGMPrivilege;
 import org.apache.sentry.provider.db.service.model.MSentryRole;
 
-import com.google.common.base.Joiner;
 import com.google.common.base.Strings;
 import com.google.common.collect.Maps;
 import com.google.common.collect.Sets;
+import org.apache.sentry.provider.db.service.persistent.QueryParamBuilder;
+import org.apache.sentry.provider.db.service.persistent.SentryStore;
 import org.apache.sentry.service.thrift.ServiceConstants;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import static org.apache.sentry.provider.db.service.persistent.SentryStore.toNULLCol;
+
 /**
- * This class used do some operations related privilege and make the results
- * persistence
+ * Sentry Generic model privilege persistence support.
+ * <p>
+ * This class is similar to {@link SentryStore} but operates on generic
+ * privileges.
  */
 public class PrivilegeOperatePersistence {
+  private static final String SERVICE_NAME = "serviceName";
+  private static final String COMPONENT_NAME = "componentName";
+  private static final String SCOPE = "scope";
+  private static final String ACTION = "action";
+
   private static final Logger LOGGER = LoggerFactory.getLogger(PrivilegeOperatePersistence.class);
   private static final Map<String, BitFieldActionFactory> actionFactories = Maps.newHashMap();
   static{
@@ -67,31 +78,106 @@ public class PrivilegeOperatePersistence {
     this.conf = conf;
   }
 
-  public boolean checkPrivilegeOption(Set<MSentryRole> roles, PrivilegeObject privilege, PersistenceManager pm) {
-    MSentryGMPrivilege requestPrivilege = convertToPrivilege(privilege);
-    boolean hasGrant = false;
-    //get persistent privileges by roles
-    Query query = pm.newQuery(MSentryGMPrivilege.class);
-    StringBuilder filters = new StringBuilder();
-    if (roles != null && roles.size() > 0) {
-      query.declareVariables("org.apache.sentry.provider.db.service.model.MSentryRole role");
-      List<String> rolesFiler = new LinkedList<String>();
-      for (MSentryRole role : roles) {
-        rolesFiler.add("role.roleName == \"" + role.getRoleName() + "\" ");
+  /**
+   * Return query builder to execute in JDO for search the given privilege
+   * @param privilege Privilege to extract
+   * @return query builder suitable for executing the query
+   */
+  private static QueryParamBuilder toQueryParam(MSentryGMPrivilege privilege) {
+    QueryParamBuilder paramBuilder = QueryParamBuilder.newQueryParamBuilder();
+    paramBuilder.add(SERVICE_NAME, toNULLCol(privilege.getServiceName()), true)
+            .add(COMPONENT_NAME, toNULLCol(privilege.getComponentName()), true)
+            .add(SCOPE, toNULLCol(privilege.getScope()), true)
+            .add(ACTION, toNULLCol(privilege.getAction()), true);
+
+    Boolean grantOption = privilege.getGrantOption();
+    paramBuilder.addObject(SentryStore.GRANT_OPTION, grantOption);
+
+    List<? extends Authorizable> authorizables = privilege.getAuthorizables();
+    int nAuthorizables = authorizables.size();
+    for (int i = 0; i < MSentryGMPrivilege.AUTHORIZABLE_LEVEL; i++) {
+      String resourceName = MSentryGMPrivilege.PREFIX_RESOURCE_NAME + String.valueOf(i);
+      String resourceType = MSentryGMPrivilege.PREFIX_RESOURCE_TYPE + String.valueOf(i);
+
+      if (i >= nAuthorizables) {
+        paramBuilder.addNull(resourceName);
+        paramBuilder.addNull(resourceType);
+      } else {
+        paramBuilder.add(resourceName, authorizables.get(i).getName(), true);
+        paramBuilder.add(resourceType, authorizables.get(i).getTypeName(), true);
       }
-      filters.append("roles.contains(role) " + "&& (" + Joiner.on(" || ").join(rolesFiler) + ")");
     }
-    query.setFilter(filters.toString());
+    return paramBuilder;
+  }
+
+  /**
+   * Create a query template tha includes information from the input privilege:
+   * <ul>
+   *   <li>Service name</li>
+   *   <li>Component name</li>
+   *   <li>Name and type for each authorizable present</li>
+   * </ul>
+   * For exmaple, for Solr may configure the following privileges:
+   * <ul>
+   *   <li>{@code p1:Collection=c1->action=query}</li>
+   *   <li>{@code p2:Collection=c1->Field=f1->action=query}</li>
+   *   <li>{@code p3:Collection=c1->Field=f2->action=query}</li>
+   * </ul>
+   * When the request for privilege revoke has
+   * {@code p4:Collection=c1->action=query}
+   * all privileges matching {@code Collection=c1} should be revoke which means that p1, p2 and p3
+   * should all be revoked.
+   *
+   * @param privilege Source privilege
+   * @return ParamBuilder suitable for executing the query
+   */
+  public static QueryParamBuilder populateIncludePrivilegesParams(MSentryGMPrivilege privilege) {
+    QueryParamBuilder paramBuilder = QueryParamBuilder.newQueryParamBuilder();
+    paramBuilder.add(SERVICE_NAME, toNULLCol(privilege.getServiceName()), true);
+    paramBuilder.add(COMPONENT_NAME, toNULLCol(privilege.getComponentName()), true);
+
+    List<? extends Authorizable> authorizables = privilege.getAuthorizables();
+    int i = 0;
+    for(Authorizable auth: authorizables) {
+      String resourceName = MSentryGMPrivilege.PREFIX_RESOURCE_NAME + String.valueOf(i);
+      String resourceType = MSentryGMPrivilege.PREFIX_RESOURCE_TYPE + String.valueOf(i);
+      paramBuilder.add(resourceName, auth.getName(), true);
+      paramBuilder.add(resourceType, auth.getTypeName(), true);
+      i++;
+    }
+    return paramBuilder;
+  }
+
+  /**
+   * Verify whether specified privilege can be granted
+   * @param roles set of roles for the privilege
+   * @param privilege privilege being checked
+   * @param pm Persistentence manager instance
+   * @return true iff at least one privilege within the role allows for the
+   *   requested privilege
+   */
+  boolean checkPrivilegeOption(Set<MSentryRole> roles, PrivilegeObject privilege, PersistenceManager pm) {
+    MSentryGMPrivilege requestPrivilege = convertToPrivilege(privilege);
+    if (roles.isEmpty()) {
+      return false;
+    }
+    // get persistent privileges by roles
+    // Find all GM privileges for all the input roles
+    Query query = pm.newQuery(MSentryGMPrivilege.class);
+    QueryParamBuilder paramBuilder = QueryParamBuilder.addRolesFilter(query, null,
+            SentryStore.rolesToRoleNames(roles));
+    query.setFilter(paramBuilder.toString());
+    List<MSentryGMPrivilege> tPrivileges =
+            (List<MSentryGMPrivilege>)query.executeWithMap(paramBuilder.getArguments());
 
-    List<MSentryGMPrivilege> tPrivileges = (List<MSentryGMPrivilege>)query.execute();
     for (MSentryGMPrivilege tPrivilege : tPrivileges) {
       if (tPrivilege.getGrantOption() && tPrivilege.implies(requestPrivilege)) {
-        hasGrant = true;
-        break;
+        return true;
       }
     }
-    return hasGrant;
+    return false;
   }
+
   public void grantPrivilege(PrivilegeObject privilege,MSentryRole role, PersistenceManager pm) throws SentryUserException {
     MSentryGMPrivilege mPrivilege = convertToPrivilege(privilege);
     grantRolePartial(mPrivilege, role, pm);
@@ -184,30 +270,21 @@ public class PrivilegeOperatePersistence {
     pm.makePersistent(role);
   }
 
-  /**
-   * Explore Privilege graph and collect privileges that are belong to the specific privilege
-   */
-  @SuppressWarnings("unchecked")
   private Set<MSentryGMPrivilege> populateIncludePrivileges(Set<MSentryRole> roles,
-      MSentryGMPrivilege parent, PersistenceManager pm) {
+                                                            MSentryGMPrivilege parent, PersistenceManager pm) {
     Set<MSentryGMPrivilege> childrens = Sets.newHashSet();
 
     Query query = pm.newQuery(MSentryGMPrivilege.class);
-    StringBuilder filters = new StringBuilder();
-    //add populateIncludePrivilegesQuery
-    filters.append(MSentryGMPrivilege.populateIncludePrivilegesQuery(parent));
+    QueryParamBuilder paramBuilder = populateIncludePrivilegesParams(parent);
+
     // add filter for role names
-    if (roles != null && roles.size() > 0) {
-      query.declareVariables("org.apache.sentry.provider.db.service.model.MSentryRole role");
-      List<String> rolesFiler = new LinkedList<String>();
-      for (MSentryRole role : roles) {
-        rolesFiler.add("role.roleName == \"" + role.getRoleName() + "\" ");
-      }
-      filters.append("&& roles.contains(role) " + "&& (" + Joiner.on(" || ").join(rolesFiler) + ")");
+    if ((roles != null) && !roles.isEmpty()) {
+      QueryParamBuilder.addRolesFilter(query, paramBuilder, SentryStore.rolesToRoleNames(roles));
     }
-    query.setFilter(filters.toString());
+    query.setFilter(paramBuilder.toString());
 
-    List<MSentryGMPrivilege> privileges = (List<MSentryGMPrivilege>)query.execute();
+    List<MSentryGMPrivilege> privileges =
+            (List<MSentryGMPrivilege>)query.executeWithMap(paramBuilder.getArguments());
     childrens.addAll(privileges);
     return childrens;
   }
@@ -315,32 +392,36 @@ public class PrivilegeOperatePersistence {
 
   private MSentryGMPrivilege getPrivilege(MSentryGMPrivilege privilege, PersistenceManager pm) {
     Query query = pm.newQuery(MSentryGMPrivilege.class);
-    query.setFilter(MSentryGMPrivilege.toQuery(privilege));
+    QueryParamBuilder paramBuilder = toQueryParam(privilege);
+    query.setFilter(paramBuilder.toString());
     query.setUnique(true);
-    return (MSentryGMPrivilege)query.execute();
+    MSentryGMPrivilege result = (MSentryGMPrivilege)query.executeWithMap(paramBuilder.getArguments());
+    return result;
   }
 
-  @SuppressWarnings("unchecked")
-  public Set<PrivilegeObject> getPrivilegesByRole(Set<MSentryRole> roles, PersistenceManager pm) {
-    Set<PrivilegeObject> privileges = Sets.newHashSet();
-    if (roles == null || roles.size() == 0) {
-      return privileges;
+  /**
+   * Get all privileges associated with a given roles
+   * @param roles Set of roles
+   * @param pm Persistence manager instance
+   * @return Set (potentially empty) of privileges associated with roles
+   */
+  Set<PrivilegeObject> getPrivilegesByRole(Set<MSentryRole> roles, PersistenceManager pm) {
+    if (roles == null || roles.isEmpty()) {
+      return Collections.emptySet();
     }
+
     Query query = pm.newQuery(MSentryGMPrivilege.class);
-    StringBuilder filters = new StringBuilder();
-    // add filter for role names
-    query.declareVariables("org.apache.sentry.provider.db.service.model.MSentryRole role");
-    List<String> rolesFiler = new LinkedList<String>();
-    for (MSentryRole role : roles) {
-      rolesFiler.add("role.roleName == \"" + role.getRoleName() + "\" ");
+    // Find privileges matching all roles
+    QueryParamBuilder paramBuilder = QueryParamBuilder.addRolesFilter(query, null,
+            SentryStore.rolesToRoleNames(roles));
+    query.setFilter(paramBuilder.toString());
+    List<MSentryGMPrivilege> mPrivileges =
+            (List<MSentryGMPrivilege>)query.executeWithMap(paramBuilder.getArguments());
+    if (mPrivileges.isEmpty()) {
+      return Collections.emptySet();
     }
-    filters.append("roles.contains(role) " + "&& (" + Joiner.on(" || ").join(rolesFiler) + ")");
 
-    query.setFilter(filters.toString());
-    List<MSentryGMPrivilege> mPrivileges = (List<MSentryGMPrivilege>) query.execute();
-    if (mPrivileges == null || mPrivileges.isEmpty()) {
-      return privileges;
-    }
+    Set<PrivilegeObject> privileges = new HashSet<>(mPrivileges.size());
     for (MSentryGMPrivilege mPrivilege : mPrivileges) {
       privileges.add(new Builder()
                                .setComponent(mPrivilege.getComponentName())

http://git-wip-us.apache.org/repos/asf/sentry/blob/71f47698/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryGMPrivilege.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryGMPrivilege.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryGMPrivilege.java
index 55b61ac..749af2a 100644
--- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryGMPrivilege.java
+++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryGMPrivilege.java
@@ -40,11 +40,13 @@ import com.google.common.collect.Lists;
  */
 @PersistenceCapable
 public class MSentryGMPrivilege {
-  private static final String PREFIX_RESOURCE_NAME = "resourceName";
-  private static final String PREFIX_RESOURCE_TYPE = "resourceType";
+  public static final String PREFIX_RESOURCE_NAME = "resourceName";
+  public static final String PREFIX_RESOURCE_TYPE = "resourceType";
+  public static final int AUTHORIZABLE_LEVEL = 4;
+
   private static final String NULL_COL = "__NULL__";
   private static final String SERVICE_SCOPE = "Server";
-  private static final int AUTHORIZABLE_LEVEL = 4;
+
   /**
    * The authorizable List has been stored into resourceName and resourceField columns
    * We assume that the generic model privilege for any component(hive/impala or solr) doesn't exceed four level.
@@ -433,65 +435,4 @@ public class MSentryGMPrivilege {
     }
   }
 
-  /**
-   * return the query to execute in JDO for search the given privilege
-   * @param privilege
-   * @return query
-   */
-  public static String toQuery(MSentryGMPrivilege privilege) {
-    StringBuilder query = new StringBuilder();
-    query.append("serviceName == \"" + toNULLCol(privilege.getServiceName()) + "\" ");
-    query.append("&& componentName == \"" + toNULLCol(privilege.getComponentName()) + "\" ");
-    query.append("&& scope == \"" + toNULLCol(privilege.getScope()) + "\" ");
-    query.append("&& action == \"" + toNULLCol(privilege.getAction()) + "\"");
-    if (privilege.getGrantOption() == null) {
-      query.append("&& this.grantOption == null ");
-    } else if (privilege.getGrantOption()) {
-      query.append("&& grantOption ");
-    } else {
-      query.append("&& !grantOption ");
-    }
-    List<? extends Authorizable> authorizables = privilege.getAuthorizables();
-    for (int i = 0; i < AUTHORIZABLE_LEVEL; i++) {
-      String resourceName = PREFIX_RESOURCE_NAME + String.valueOf(i);
-      String resourceType = PREFIX_RESOURCE_TYPE + String.valueOf(i);
-
-      if (i >= authorizables.size()) {
-        query.append("&& " + resourceName + " == \"" + NULL_COL + "\" ");
-        query.append("&& " + resourceType + " == \"" + NULL_COL + "\" ");
-      } else {
-        query.append("&& " + resourceName + " == \"" + authorizables.get(i).getName() + "\" ");
-        query.append("&& " + resourceType + " == \"" + authorizables.get(i).getTypeName() + "\" ");
-      }
-    }
-    return query.toString();
-  }
-
-  /**
-   * Get the query to execute in the JDO deducing privileges include the scope of according to the given privilege
-   * The query was used in three privilege operations:
-   * 1.revoking privilege
-   * 2.renaming privilege
-   * 3.dropping privilege
-   * Take the Solr for example, if there exists three privileges such as p1:Collection=c1->action=query,
-   * p2:Collection=c1->Field=f1->action=query and p3:Collection=c1->Field=f2->action=query.
-   * When the revoking operation happens, the request privilege is p4:Collection=c1->action=query.
-   * The result is that not only p1 should be revoked, but also p2 and p3 should be revoked together.
-   * So the populateIncludePrivilegesQuery should be Collection=c1
-   * @param privilege
-   * @return query
-   */
-  public static String populateIncludePrivilegesQuery(MSentryGMPrivilege privilege) {
-    StringBuilder query = new StringBuilder();
-    query.append("serviceName == \"" + toNULLCol(privilege.getServiceName()) + "\" ");
-    query.append("&& componentName == \"" + toNULLCol(privilege.getComponentName()) + "\" ");
-    List<? extends Authorizable> authorizables = privilege.getAuthorizables();
-    for (int i= 0 ; i < authorizables.size(); i++) {
-      String resourceName = PREFIX_RESOURCE_NAME + String.valueOf(i);
-      String resourceType = PREFIX_RESOURCE_TYPE + String.valueOf(i);
-      query.append("&& " + resourceName + " == \"" + authorizables.get(i).getName() + "\" ");
-      query.append("&& " + resourceType + " == \"" + authorizables.get(i).getTypeName() + "\" ");
-    }
-    return query.toString();
-  }
 }

http://git-wip-us.apache.org/repos/asf/sentry/blob/71f47698/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryRole.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryRole.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryRole.java
index 6dc6918..f1d7a86 100644
--- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryRole.java
+++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryRole.java
@@ -173,12 +173,14 @@ public class MSentryRole {
     }
   }
 
-  public void removePrivileges() {
+  public Set<MSentryPrivilege> removePrivileges() {
     // copy is required since privilege.removeRole will call remotePrivilege
-    for (MSentryPrivilege privilege : ImmutableSet.copyOf(privileges)) {
+    Set<MSentryPrivilege> copy = ImmutableSet.copyOf(privileges);
+    for (MSentryPrivilege privilege : copy) {
       privilege.removeRole(this);
     }
     Preconditions.checkState(privileges.isEmpty(), "Privileges should be empty: " + privileges);
+    return copy;
   }
 
   @Override

http://git-wip-us.apache.org/repos/asf/sentry/blob/71f47698/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/QueryParamBuilder.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/QueryParamBuilder.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/QueryParamBuilder.java
new file mode 100644
index 0000000..8a77fc1
--- /dev/null
+++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/QueryParamBuilder.java
@@ -0,0 +1,406 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.sentry.provider.db.service.persistent;
+
+import com.google.common.base.Joiner;
+import org.apache.sentry.provider.db.service.model.MSentryRole;
+
+import javax.annotation.concurrent.NotThreadSafe;
+import javax.jdo.Query;
+import java.util.HashMap;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.atomic.AtomicLong;
+
+/**
+ * The QueryParamBuilder provides mechanism for constructing complex JDOQL queries with parameters.
+ * <p>
+ * There are many places where we want to construct a non-trivial parametrized query,
+ * often with sub-queries. A simple query expression is just a list of conditions joined
+ * by operation (either && or ||).  More complicated query may contain sub-expressions which may contain
+ * further sub-expressions, for example {@code (AA && (B | (C && D)))}.
+ * <p>
+ * Query may contain parameters, which usually come from external sources (e.g. Thrift requests). For example,
+ * to search for a record containing specific database name we can use something like
+ * {@code "dbName == " + request.getDbName()}. This opens a possibility JDOQL injection with carefully
+ * constructed requests. To avoid this we need to parameterize the query into something like
+ * {@code "dbName == :dbMame"} and pass {@code dbName} as a query parameter. (The colon in {@code :dbName}
+ * tells Datanucleus to automatically figure out the type of the {@code dbName} parameter). We collect all
+ * such parameters in a map and use {@code executeWithMap()} method of the query to pass all collected
+ * parameters to a query.
+ * <h2>Representing the query and sub-queries</h2>
+ *
+ * A query is represented as
+ * <ul>
+ *   <li>Top-level query operation which is always && or ||</li>
+ *   <li>List of strings that constitute simple subexpressions which are joined with the top level
+ *   operator when the final query string is constructed</li>
+ *   <li>List of sub-expressions. Each sub-expression is another QueryParamBuilder that shares
+ *   the parameter map with the parent. Usually the top-level operation of the sub-expression
+ *   is the inverse of the top-level operation if the parent. Since
+ *   {@code (A && (B && C))} can be simplified as {@code (A && B && C)}, there is no
+ *   need for parent and child to have the same top-level operation.
+ *   Children are added using the {@link #newChild()} method.</li>
+ * </ul>
+ *
+ * <h2>Constructing the string representation of a query</h2>
+ *
+ * Once the query is fully constructed, we can get its string reopresentation suitable for the
+ * {@code setFilter()} method of {@link javax.jdo.Query} by using {@link #toString()} method
+ * which does the following:
+ * <ul>
+ *   <li>Combines all accumulated simple subexpressions using the top-level operator</li>
+ *   <li>Recursively combines children QueryParamBuilder objects using their {@link #toString()}
+ *   methods</li>
+ *   <li>Joins result with the top-level operation</li>
+ * </ul>
+ * <em>NOTE that we do not guarantee specific order of expressions within each level</em>.
+ * <p>
+ * The class also provides useful common methods for checking that some field is or
+ * isn't <em>NULL</em> and method <em>addSet()</em> to add dynamic set of key/values<p>
+ *
+ * Most class methods return <em>this</em> so it is possible to chain calls together.
+ * <p>
+ *
+ * The class is not thread-safe.
+ * <p>
+ * Examples:
+ * <ol>
+ *     <li>
+ * <pre>{@code
+ *   QueryParamBuilder p = newQueryParamBuilder();
+ *   p.add("key1", "val1").add("key2", "val2")
+ *
+ *   // Returns "(this.key1 == :key1 && this.key2 == :key2)"
+ *   String queryStr = p.toString();
+ *
+ *   // Returns map {"key1": "val1", "key2": "val2"}
+ *   Map<String, Object> args = p.getArguments();
+ *
+ *   Query query = pm.newQuery(Foo.class);
+ *   query.setFilter(queryStr);
+ *   query.executeWIthMap(args);
+ *   }</pre>
+ * </li>
+ * <li>
+ * <pre>{@code
+ *   QueryParamBuilder p = newQueryParamBuilder();
+ *   p.add("key1", "val1").add("key2", "val2")
+ *    .newChild() // Inverts logical op from && to ||
+ *      .add("key3", "val3")
+ *      .add("key4", "val4").
+ *
+ *  // Returns "(this.key1 == :val1 && this.key2 == :val2 && \
+ *  //  (this.key3 == :val4 || this.key4 == val4))"
+ *  String queryStr1 = p.toString()
+ * }</pre>
+ * </li>
+ * </ol>
+ *
+ * @see <a href="http://www.datanucleus.org/products/datanucleus/jdo/jdoql.html">Datanucleus JDOQL</a>
+ */
+@NotThreadSafe
+public class QueryParamBuilder {
+
+  /**
+   * Representation of the top-level query operator.
+   * Query is built by joining all parts with the specified Op.
+   */
+  enum Op {
+    AND(" && "),
+    OR(" || ");
+
+    public String toString() {
+      return value;
+    }
+
+    private final String value;
+
+    /** Constructor from string */
+    Op(String val) {
+      this.value = val;
+    }
+  }
+
+  // Query parts that will be joined with Op
+  private final List<String> queryParts = new LinkedList<>();
+  // List of children - allocated lazily when children are added
+  private List<QueryParamBuilder> children;
+  // Query Parameters
+  private final Map<String, Object> arguments;
+  // paramId is used for automatically generating variable names
+  private final AtomicLong paramId;
+  // Join operation
+  private final String operation;
+
+  /**
+   * Create new {@link QueryParamBuilder}
+   * @return the default {@link QueryParamBuilder} with && top-level operation.
+   */
+  public static QueryParamBuilder newQueryParamBuilder() {
+    return new QueryParamBuilder();
+  }
+
+  /**
+   * Create new {@link QueryParamBuilder} with specific top-level operator
+   * @param operation top-level operation for subexpressions
+   * @return {@link QueryParamBuilder} with specified top-level operator
+   */
+  public static QueryParamBuilder newQueryParamBuilder(Op operation) {
+    return new QueryParamBuilder(operation);
+  }
+
+  /**
+   * Create a new child builder and attach to this one. Child's join operation is the
+   * inverse of the parent
+   * @return new child of the QueryBuilder
+   */
+  public QueryParamBuilder newChild() {
+    // Reverse operation of this builder
+    Op operation = this.operation.equals(Op.AND.toString()) ? Op.OR : Op.AND;
+    return this.newChild(operation);
+  }
+
+  /**
+   * Create a new child builder attached to this one
+   * @param operation - join operation
+   * @return new child of the QueryBuilder
+   */
+  private QueryParamBuilder newChild(Op operation) {
+    QueryParamBuilder child = new QueryParamBuilder(this, operation);
+    if (children == null) {
+      children = new LinkedList<>();
+    }
+    children.add(child);
+    return child;
+  }
+
+  /**
+   * Get query arguments
+   * @return query arguments as a map of arg name and arg value suitable for
+   * query.executeWithMap
+   */
+  public Map<String, Object> getArguments() {
+    return arguments;
+  }
+
+  /**
+   * Get query string - reconstructs the query string from all the parts and children.
+   * @return Query string which can be matched with arguments to execute a query.
+   */
+  @Override
+  public String toString() {
+    if (children == null && queryParts.isEmpty()) {
+      return "";
+    }
+    if (children == null) {
+      return "(" + Joiner.on(operation).join(queryParts) + ")";
+    }
+    // Concatenate our query parts with all children
+    List<String> result = new LinkedList<>(queryParts);
+    for (Object child: children) {
+      result.add(child.toString());
+    }
+    return "(" + Joiner.on(operation).join(result) + ")";
+  }
+
+  /**
+   * Add parameter for field fieldName with given value where value is any Object
+   * @param fieldName Field name to query for
+   * @param value Field value (can be any Object)
+   */
+  public QueryParamBuilder addObject(String fieldName, Object value) {
+    return addCustomParam("this." + fieldName + " == :" + fieldName,
+            fieldName, value);
+  }
+
+  /**
+   * Add string parameter for field fieldName
+   * @param fieldName name of the field
+   * @param value String value. Value is normalized - converted to lower case and trimmed
+   * @return this
+   */
+  public QueryParamBuilder add(String fieldName, String value) {
+    return addCommon(fieldName, value, false);
+  }
+
+  /**
+   * Add string parameter to field value with or without normalization
+   * @param fieldName field name of the field
+   * @param value String value, inserted as is if preserveCase is true, normalized otherwise
+   * @param preserveCase if true, trm and lowercase the value.
+   * @return this
+   */
+  public QueryParamBuilder add(String fieldName, String value, boolean preserveCase) {
+    return addCommon(fieldName, value, preserveCase);
+  }
+
+  /**
+   * Add condition that fieldName is not equal to NULL
+   * @param fieldName field name to compare to NULL
+   * @return this
+   */
+  public QueryParamBuilder addNotNull(String fieldName) {
+    queryParts.add(String.format("this.%s != \"%s\"", fieldName, SentryStore.NULL_COL));
+    return this;
+  }
+
+  /**
+   * Add condition that fieldName is equal to NULL
+   * @param fieldName field name to compare to NULL
+   * @return this
+   */
+  public QueryParamBuilder addNull(String fieldName) {
+    queryParts.add(String.format("this.%s == \"%s\"", fieldName, SentryStore.NULL_COL));
+    return this;
+  }
+
+  /**
+   * Add custom string for evaluation together with a single parameter.
+   * This is used in cases where we need expression different from this.name == value
+   * @param expr String expression containing ':&lt paramName&gt' somewhere
+   * @param paramName parameter name
+   * @param value parameter value
+   * @return this
+   */
+  QueryParamBuilder addCustomParam(String expr, String paramName, Object value) {
+    arguments.put(paramName, value);
+    queryParts.add(expr);
+    return this;
+  }
+
+  /**
+   * Add arbitrary query string without parameters
+   * @param expr String expression
+   * @return this
+   */
+  QueryParamBuilder addString(String expr) {
+    queryParts.add(expr);
+    return this;
+  }
+
+  /**
+   * Add common filter for set of Sentry roles. This is used to simplify creating filters for
+   * privileges belonging to the specified set of roles.
+   * @param query Query used for search
+   * @param paramBuilder paramBuilder for parameters
+   * @param roleNames set of role names
+   * @return paramBuilder supplied or a new one if the supplied one is null.
+   */
+  public static QueryParamBuilder addRolesFilter(Query query, QueryParamBuilder paramBuilder,
+                                                 Set<String> roleNames) {
+    query.declareVariables(MSentryRole.class.getName() + " role");
+    if (paramBuilder == null) {
+      paramBuilder = new QueryParamBuilder();
+    }
+    if (roleNames == null || roleNames.isEmpty()) {
+      return paramBuilder;
+    }
+    paramBuilder.newChild().addSet("role.roleName == ", roleNames);
+    paramBuilder.addString("roles.contains(role)");
+    return paramBuilder;
+  }
+
+  /**
+   * Add multiple conditions for set of values.
+   * <p>
+   * Example:
+   * <pre>
+   *  Set<String>names = new HashSet<>();
+   *  names.add("foo");
+   *  names.add("bar");
+   *  names.add("bob");
+   *  paramBuilder.addSet("prefix == ", names);
+   *  // Expect:"(prefix == :var0 && prefix == :var1 && prefix == :var2)"
+   *  paramBuilder.toString());
+   *  // paramBuilder(getArguments()) contains mapping for var0, var1 and var2
+   * </pre>
+   * @param prefix common prefix to use for expression
+   * @param values
+   * @return this
+   */
+  QueryParamBuilder addSet(String prefix, Iterable<String> values) {
+    if (values == null) {
+      return this;
+    }
+
+    // Add expressions of the form 'prefix :var$i'
+    for(String name: values) {
+      // Append index to the varName
+      String vName = "var" + paramId.toString();
+      addCustomParam(prefix + ":" + vName, vName, name.trim().toLowerCase());
+      paramId.incrementAndGet();
+    }
+    return this;
+  }
+
+  /**
+   * Construct a default QueryParamBuilder joining arguments with &&
+   */
+  private QueryParamBuilder() {
+    this(Op.AND);
+  }
+
+  /**
+   * Construct generic QueryParamBuilder
+   * @param operation join operation (AND or OR)
+   */
+  private QueryParamBuilder(Op operation) {
+    this.arguments = new HashMap<>();
+    this.operation = operation.toString();
+    this.paramId = new AtomicLong(0);
+  }
+
+  /**
+   * Internal constructor used for children - reuses arguments from parent
+   * @param parent parent element
+   * @param operation join operation
+   */
+  private QueryParamBuilder(QueryParamBuilder parent, Op operation) {
+    this.arguments = parent.getArguments();
+    this.operation = operation.toString();
+    this.paramId = parent.paramId;
+  }
+
+  /**
+   * common code for adding string values
+   * @param fieldName field name to add
+   * @param value field value
+   * @param preserveCase if true, do not trim and lower
+   * @return this
+   * @throws IllegalArgumentException if fieldName was already added before
+   */
+  private QueryParamBuilder addCommon(String fieldName, String value,
+                                      boolean preserveCase) {
+    Object oldValue;
+    if (preserveCase) {
+      oldValue = arguments.put(fieldName, SentryStore.toNULLCol(SentryStore.safeTrim(value)));
+    } else {
+      oldValue = arguments.put(fieldName, SentryStore.toNULLCol(SentryStore.safeTrimLower(value)));
+    }
+    if (oldValue != null) {
+      // Attempt to insert the same field twice
+      throw new IllegalArgumentException("field " + fieldName + "already exists");
+    }
+    queryParts.add("this." + fieldName + " == :" + fieldName);
+    return this;
+  }
+}

http://git-wip-us.apache.org/repos/asf/sentry/blob/71f47698/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 2ae9f3b..b9272bc 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
@@ -46,11 +46,25 @@ import javax.jdo.Transaction;
 
 import org.apache.commons.lang.StringUtils;
 import org.apache.hadoop.conf.Configuration;
-import org.apache.sentry.core.common.exception.*;
+import org.apache.sentry.core.common.exception.SentryAccessDeniedException;
+import org.apache.sentry.core.common.exception.SentryAlreadyExistsException;
+import org.apache.sentry.core.common.exception.SentryGrantDeniedException;
+import org.apache.sentry.core.common.exception.SentryInvalidInputException;
+import org.apache.sentry.core.common.exception.SentryNoSuchObjectException;
+import org.apache.sentry.core.common.exception.SentrySiteConfigurationException;
+import org.apache.sentry.core.common.exception.SentryUserException;
 import org.apache.sentry.core.common.utils.SentryConstants;
 import org.apache.sentry.core.model.db.AccessConstants;
 import org.apache.sentry.core.model.db.DBModelAuthorizable.AuthorizableType;
-import org.apache.sentry.provider.db.service.model.*;
+import org.apache.sentry.provider.db.service.model.MAuthzPathsMapping;
+import org.apache.sentry.provider.db.service.model.MSentryChange;
+import org.apache.sentry.provider.db.service.model.MSentryGroup;
+import org.apache.sentry.provider.db.service.model.MSentryPathChange;
+import org.apache.sentry.provider.db.service.model.MSentryPermChange;
+import org.apache.sentry.provider.db.service.model.MSentryPrivilege;
+import org.apache.sentry.provider.db.service.model.MSentryUser;
+import org.apache.sentry.provider.db.service.model.MSentryVersion;
+import org.apache.sentry.provider.db.service.model.MSentryRole;
 import org.apache.sentry.provider.db.service.thrift.SentryPolicyStoreProcessor;
 import org.apache.sentry.provider.db.service.thrift.TSentryActiveRoleSet;
 import org.apache.sentry.provider.db.service.thrift.TSentryAuthorizable;
@@ -69,7 +83,6 @@ import org.slf4j.LoggerFactory;
 import com.codahale.metrics.Gauge;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Function;
-import com.google.common.base.Joiner;
 import com.google.common.base.Preconditions;
 import com.google.common.base.Strings;
 import com.google.common.collect.Collections2;
@@ -79,6 +92,7 @@ import com.google.common.collect.Maps;
 import com.google.common.collect.Sets;
 
 import static org.apache.sentry.hdfs.Updateable.Update;
+import static org.apache.sentry.provider.db.service.persistent.QueryParamBuilder.newQueryParamBuilder;
 
 /**
  * SentryStore is the data access object for Sentry data. Strings
@@ -904,7 +918,7 @@ public class SentryStore {
     }
 
     Query query = pm.newQuery(MSentryPrivilege.class);
-    QueryParamBuilder paramBuilder = addRolesFilter(query, null, roleNames)
+    QueryParamBuilder paramBuilder = QueryParamBuilder.addRolesFilter(query, null, roleNames)
             .add(SERVER_NAME, parent.getServerName());
 
     if (!isNULL(parent.getDbName())) {
@@ -948,7 +962,7 @@ public class SentryStore {
   @SuppressWarnings("unchecked")
   private List<MSentryPrivilege> getMSentryPrivileges(TSentryPrivilege tPriv, PersistenceManager pm) {
     Query query = pm.newQuery(MSentryPrivilege.class);
-    QueryParamBuilder paramBuilder = new QueryParamBuilder();
+    QueryParamBuilder paramBuilder = newQueryParamBuilder();
     paramBuilder
             .add(SERVER_NAME, tPriv.getServerName())
             .add("action", tPriv.getAction());
@@ -978,7 +992,7 @@ public class SentryStore {
       grantOption = false;
     }
 
-    QueryParamBuilder paramBuilder = new QueryParamBuilder();
+    QueryParamBuilder paramBuilder = newQueryParamBuilder();
     paramBuilder.add(SERVER_NAME, tPriv.getServerName())
             .add(DB_NAME, tPriv.getDbName())
             .add(TABLE_NAME, tPriv.getTableName())
@@ -1271,7 +1285,7 @@ public class SentryStore {
       new TransactionBlock<Boolean>() {
         public Boolean execute(PersistenceManager pm) throws Exception {
           Query query = pm.newQuery(MSentryPrivilege.class);
-          QueryParamBuilder paramBuilder = addRolesFilter(query,null, roleNames);
+          QueryParamBuilder paramBuilder = QueryParamBuilder.addRolesFilter(query,null, roleNames);
           paramBuilder.add(SERVER_NAME, serverName);
           query.setFilter(paramBuilder.toString());
           query.setResult("count(this)");
@@ -1294,7 +1308,7 @@ public class SentryStore {
         public List<MSentryPrivilege> execute(PersistenceManager pm)
                 throws Exception {
           Query query = pm.newQuery(MSentryPrivilege.class);
-          QueryParamBuilder paramBuilder = addRolesFilter(query, null, roleNames);
+          QueryParamBuilder paramBuilder = QueryParamBuilder.addRolesFilter(query, null, roleNames);
 
           if (authHierarchy != null && authHierarchy.getServer() != null) {
             paramBuilder.add(SERVER_NAME, authHierarchy.getServer());
@@ -1349,11 +1363,11 @@ public class SentryStore {
         new TransactionBlock<List<MSentryPrivilege>>() {
           public List<MSentryPrivilege> execute(PersistenceManager pm) throws Exception {
             Query query = pm.newQuery(MSentryPrivilege.class);
-            QueryParamBuilder paramBuilder = new QueryParamBuilder();
+            QueryParamBuilder paramBuilder = newQueryParamBuilder();
             if (roleNames == null || roleNames.isEmpty()) {
               paramBuilder.addString("!roles.isEmpty()");
             } else {
-              addRolesFilter(query, paramBuilder, roleNames);
+              QueryParamBuilder.addRolesFilter(query, paramBuilder, roleNames);
             }
             if (authHierarchy.getServer() != null) {
               paramBuilder.add(SERVER_NAME, authHierarchy.getServer());
@@ -1785,13 +1799,15 @@ public class SentryStore {
     }
     return mSentryPrivilege;
   }
-  private static String safeTrim(String s) {
+
+  static String safeTrim(String s) {
     if (s == null) {
       return null;
     }
     return s.trim();
   }
-  private static String safeTrimLower(String s) {
+
+  static String safeTrimLower(String s) {
     if (s == null) {
       return null;
     }
@@ -2223,7 +2239,7 @@ public class SentryStore {
                 throws Exception {
           Map<String, HashMap<String, String>> retVal = new HashMap<>();
           Query query = pm.newQuery(MSentryPrivilege.class);
-          QueryParamBuilder paramBuilder = new QueryParamBuilder();
+          QueryParamBuilder paramBuilder = newQueryParamBuilder();
           paramBuilder
                   .addNotNull(SERVER_NAME)
                   .addNotNull(DB_NAME)
@@ -2554,7 +2570,7 @@ public class SentryStore {
             if (roleNames == null || roleNames.isEmpty()) {
               mSentryRoles = (List<MSentryRole>)query.execute();
             } else {
-              QueryParamBuilder paramBuilder = new QueryParamBuilder(QueryParamBuilder.Op.OR);
+              QueryParamBuilder paramBuilder = newQueryParamBuilder(QueryParamBuilder.Op.OR);
               paramBuilder.addSet("roleName == ", roleNames);
               query.setFilter(paramBuilder.toString());
               mSentryRoles =
@@ -2629,7 +2645,7 @@ public class SentryStore {
         public Map<String, Set<TSentryPrivilege>> execute(PersistenceManager pm)
                 throws Exception {
           Query query = pm.newQuery(MSentryPrivilege.class);
-          QueryParamBuilder paramBuilder = new QueryParamBuilder();
+          QueryParamBuilder paramBuilder = newQueryParamBuilder();
 
           if (!StringUtils.isEmpty(dbName)) {
               paramBuilder.add(DB_NAME, dbName);
@@ -2694,11 +2710,7 @@ public class SentryStore {
       return Collections.emptySet();
     }
 
-    Set<String> roleNames = new HashSet<>(mSentryRoles.size());
-    for (MSentryRole mSentryRole : mSentryRoles) {
-      roleNames.add(mSentryRole.getRoleName());
-    }
-    return roleNames;
+    return rolesToRoleNames(mSentryRoles);
   }
 
   /**
@@ -3006,11 +3018,24 @@ public class SentryStore {
   }
 
   /**
+   * Return set of rolenames from a collection of roles
+   * @param roles - collection of roles
+   * @return set of role names for each role in collection
+   */
+  public static Set<String> rolesToRoleNames(final Iterable<MSentryRole> roles) {
+    Set<String> roleNames = new HashSet<>();
+    for (MSentryRole mSentryRole : roles) {
+      roleNames.add(mSentryRole.getRoleName());
+    }
+    return roleNames;
+  }
+
+  /**
    * Return exception for nonexistent role
    * @param roleName Role name
    * @return SentryNoSuchObjectException with appropriate message
    */
-  private SentryNoSuchObjectException noSuchRole(String roleName) {
+  private static SentryNoSuchObjectException noSuchRole(String roleName) {
     return new SentryNoSuchObjectException("nonexistent role " + roleName);
   }
 
@@ -3019,7 +3044,7 @@ public class SentryStore {
    * @param groupName Group name
    * @return SentryNoSuchObjectException with appropriate message
    */
-  private SentryNoSuchObjectException noSuchGroup(String groupName) {
+  private static SentryNoSuchObjectException noSuchGroup(String groupName) {
     return new SentryNoSuchObjectException("nonexistent group + " + groupName);
   }
 
@@ -3033,314 +3058,6 @@ public class SentryStore {
   }
 
   /**
-   * Add common filter for set of roles
-   * @param query Query used for search
-   * @param paramBuilder paramBuilder for parameters
-   * @param roleNames set of role names
-   * @return paramBuilder supplied or a new one
-   */
-  private QueryParamBuilder addRolesFilter(Query query, QueryParamBuilder paramBuilder,
-                                           Set<String> roleNames) {
-    query.declareVariables(MSentryRole.class.getName() + " role");
-    if (paramBuilder == null) {
-      paramBuilder = new QueryParamBuilder();
-    }
-    if (roleNames == null || roleNames.isEmpty()) {
-        return paramBuilder;
-    }
-    paramBuilder.newChild().addSet("role.roleName == ", roleNames);
-    paramBuilder.addString("roles.contains(role)");
-    return paramBuilder;
-  }
-
-  /**
-   * QueryParamBuilder is a helper class assisting in creating parameters for
-   * JDOQL queries.
-   * <p>
-   * There are many places where we want to construct a non-trivial parametrized query,
-   * often with sub-queries. A simple query expression is just a list of conditions joined
-   * by operation (either AND or OR). For the whole expression we keep a dictionary of
-   * parameter names to parameter values that is passed to <em>query.executeWithMap()</em>.
-   * <p>
-   * For sub-expressions we create a child <em>QueryParamBuilder</em> which shares the
-   * dictionary with the parent. Then when we construct the final query string using
-   * <em>toString()</em> operator we recursively convert each child to a string and add each
-   * of the resulting strings to the top-level expression list.
-   * <p>
-   * The class also provides useful common methods for checking that some field is or
-   * isn't <em>NULL</em> and method <em>addSet()</em> to add dynamic set of key/values
-   * <p>
-   * Most class methods return <em>this</em> so it is possible to chain calls together.
-   * <p>
-   * Examples:
-   * <ol>
-   *     <li>
-   * <pre>
-   *   QueryParamBuilder p
-   *      .add("key1", "val1")
-   *      .add("key2", "val2")
-   *
-   *   // Returns "(this.key1 == :key1 && this.key2 == :key2)"
-   *   String queryStr = p.toString();
-   *
-   *   // Returns map {"key1": "val1", "key2": "val2"}
-   *   Map<String, Object> args = p.getArguments();
-   *
-   *   Query query = pm.newQuery(Foo.class);
-   *   query.setFilter(queryStr);
-   *   query.executeWIthMap(args);
-   * </pre></li>
-   * <li>
-   * <pre>
-   *   QueryParamBuilder p = new QueryParamBuilder();
-   *   p.add("key1", "val1")
-   *    .add("key2", "val2")
-   *    .newChild()
-   *      .add("key3", "val3")
-   *      .add("key4", "val4").
-   *
-   *  // Returns "(this.key1 == :val1 && this.key2 == :val2 && \
-   *  //  (this.key3 == :val4 || this.key4 == val4))"
-   *  String queryStr1 = p.toString()
-   * </pre></li>
-   * </ol>
-   */
-  static class QueryParamBuilder {
-
-    // Query is built by joining all parts with the specified Op
-    enum Op {
-      AND(" && "),
-      OR(" || ");
-
-      public String toString() {
-        return value;
-      }
-
-      private final String value;
-      Op(String val) {
-        this.value = val;
-      }
-    };
-
-    // Query parts that will be joined with Op
-    private final List<String> queryParts = new LinkedList<>();
-    // List of children - allocated lazily when children are added
-    private List<QueryParamBuilder> children;
-    // Query Parameters
-    private final Map<String, Object> arguments;
-    // Join operation
-    private final String operation;
-    private final String THIS = "this.";
-
-    /**
-     * Construct a default QueryParamBuilder joining arguments with AND
-     */
-    QueryParamBuilder() {
-      this(Op.AND);
-    }
-
-    /**
-     * Construct generic QueryParamBuilder
-     * @param operation join operation (AND or OR)
-     */
-    QueryParamBuilder(Op operation) {
-      this.arguments = new HashMap<>();
-      this.operation = operation.toString();
-    }
-
-    /**
-     * Internal constructor used for children - reuses arguments from parent
-     * @param parent parent element
-     * @param operation join operation
-     */
-    private QueryParamBuilder(QueryParamBuilder parent, Op operation) {
-      this.arguments = parent.getArguments();
-      this.operation = operation.toString();
-    }
-
-    /**
-     * Create a new child builder attached to this one
-     * @param operation - join operation
-     * @return new child of the QueryBuilder
-     */
-    QueryParamBuilder newChild(Op operation) {
-      QueryParamBuilder child = new QueryParamBuilder(this, operation);
-      if (children == null) {
-        children = new LinkedList<>();
-      }
-      children.add(child);
-      return child;
-    }
-
-    /**
-     * Create a new child builder and attach to this one. Child's join operation is the
-     * inverse of the parent
-     * @return new child of the QueryBuilder
-     */
-    QueryParamBuilder newChild() {
-      // Reverse operation of this builder
-      Op operation = this.operation.equals(Op.AND.toString()) ? Op.OR : Op.AND;
-      return this.newChild(operation);
-    }
-
-    /**
-     * Get query arguments
-     * @return query arguments as a map of arg name and arg value suitable for
-     * query.executeWithMap
-     */
-    public Map<String, Object> getArguments() {
-      return arguments;
-    }
-
-    /**
-     * Get query string - reconstructs the query string from all the parts and children.
-     * @return Query string which can be matched with arguments to execute a query.
-     */
-    @Override
-    public String toString() {
-      if (children == null && queryParts.isEmpty()) {
-        return "";
-      }
-      if (children == null) {
-        return "(" + Joiner.on(operation).join(queryParts) + ")";
-      }
-      // Concatenate our query parts with all children
-      List<String> result = new LinkedList<>(queryParts);
-      for (Object child: children) {
-        result.add(child.toString());
-      }
-      return "(" + Joiner.on(operation).join(result) + ")";
-    }
-
-    /**
-     * Add parameter for field fieldName with given value where value is any Object
-     * @param fieldName Field name to query for
-     * @param value Field value (can be any Object)
-     */
-    public QueryParamBuilder addObject(String fieldName, Object value) {
-      return addCustomParam(THIS + fieldName + " == :" + fieldName,
-              fieldName, value);
-    }
-
-    /**
-     * Add string parameter for field fieldName
-     * @param fieldName name of the field
-     * @param value String value. Value is normalized - converted to lower case and trimmed
-     * @return this
-     */
-    public QueryParamBuilder add(String fieldName, String value) {
-      return addCommon(fieldName, value, false);
-    }
-
-    /**
-     * Add string parameter to field value with or without normalization
-     * @param fieldName field name of the field
-     * @param value String value, inserted as is if preserveCase is true, normalized otherwise
-     * @param preserveCase if true, trm and lowercase the value.
-     * @return this
-     */
-    public QueryParamBuilder add(String fieldName, String value, boolean preserveCase) {
-      return addCommon(fieldName, value, preserveCase);
-    }
-
-    /**
-     * Add condition that fieldName is not equal to NULL
-     * @param fieldName field name to compare to NULL
-     * @return this
-     */
-    public QueryParamBuilder addNotNull(String fieldName) {
-      queryParts.add(String.format("this.%s != \"%s\"", fieldName, NULL_COL));
-      return this;
-    }
-
-    /**
-     * Add condition that fieldName is equal to NULL
-     * @param fieldName field name to compare to NULL
-     * @return this
-     */
-    public QueryParamBuilder addNull(String fieldName) {
-      queryParts.add(String.format("this.%s == \"%s\"", fieldName, NULL_COL));
-      return this;
-    }
-
-    /**
-     * Add custom string for evaluation together with a single parameter.
-     * This is used in cases where we need expression different from this.name == value
-     * @param expr String expression containing ':&lt paramName&gt' somewhere
-     * @param paramName parameter name
-     * @param value parameter value
-     * @return this
-     */
-    public QueryParamBuilder addCustomParam(String expr, String paramName, Object value) {
-      arguments.put(paramName, value);
-      queryParts.add(expr);
-      return this;
-    }
-
-    /**
-     * Add arbitrary query string without parameters
-     * @param expr String expression
-     * @return this
-     */
-    public QueryParamBuilder addString(String expr) {
-      queryParts.add(expr);
-      return this;
-    }
-
-    /**
-     * Add multiple conditions for set of values.
-     * <p>
-     * Example:
-     * <pre>
-     *  Set<String>names = new HashSet<>();
-     *  names.add("foo");
-     *  names.add("bar");
-     *  names.add("bob");
-     *  paramBuilder.addSet("prefix == ", names);
-     *  // Expect:"(prefix == :var0 && prefix == :var1 && prefix == :var2)"
-     *  paramBuilder.toString());
-     *  // paramBuilder(getArguments()) contains mapping for var0, var1 and var2
-     * </pre>
-     * @param prefix common prefix to use for expression
-     * @param values
-     * @return this
-     */
-    public QueryParamBuilder addSet(String prefix, Set<String> values) {
-      if (values == null) {
-        return this;
-      }
-      int index = 0;
-
-      // Add expressions of the form 'prefix :var$i'
-      for(String name: values) {
-        // Append index to the varName
-        String vName = String.format("var%d", index);
-        addCustomParam(prefix + ":" + vName, vName, name.trim().toLowerCase());
-        index++;
-      }
-      return this;
-    }
-
-    /**
-     * common code for adding string values
-     * @param fieldName field name to add
-     * @param value field value
-     * @param preserveCase if true, do not trim and lower
-     * @return this
-     */
-    private QueryParamBuilder addCommon(String fieldName, String value,
-                           boolean preserveCase) {
-      if (preserveCase) {
-        arguments.put(fieldName, toNULLCol(safeTrim(value)));
-      } else {
-        arguments.put(fieldName, toNULLCol(safeTrimLower(value)));
-      }
-      queryParts.add("this." + fieldName + " == :" + fieldName);
-      return this;
-    }
-  }
-
-  /**
    * Get the last processed change ID for perm/path delta changes.
    *
    * @param pm the PersistenceManager

http://git-wip-us.apache.org/repos/asf/sentry/blob/71f47698/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
index d9fce69..0e22755 100644
--- a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
+++ b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
@@ -36,6 +36,7 @@ import org.apache.sentry.core.common.exception.SentryAlreadyExistsException;
 import org.apache.sentry.core.common.exception.SentryGrantDeniedException;
 import org.apache.sentry.core.common.exception.SentryNoSuchObjectException;
 import org.apache.sentry.hdfs.PermissionsUpdate;
+import org.apache.sentry.hdfs.Updateable;
 import org.apache.sentry.hdfs.service.thrift.TPrivilegeChanges;
 import org.apache.sentry.hdfs.service.thrift.TRoleChanges;
 import org.apache.sentry.provider.db.service.model.MSentryPermChange;
@@ -61,7 +62,7 @@ import com.google.common.collect.Maps;
 import com.google.common.collect.Sets;
 import com.google.common.io.Files;
 
-import static org.apache.sentry.hdfs.Updateable.Update;
+import static org.apache.sentry.provider.db.service.persistent.QueryParamBuilder.newQueryParamBuilder;
 
 public class TestSentryStore extends org.junit.Assert {
 
@@ -2201,8 +2202,8 @@ public class TestSentryStore extends org.junit.Assert {
   }
 
   public void testQueryParamBuilder() {
-    SentryStore.QueryParamBuilder paramBuilder;
-    paramBuilder = new SentryStore.QueryParamBuilder();
+    QueryParamBuilder paramBuilder;
+    paramBuilder = newQueryParamBuilder();
     // Try single parameter
     paramBuilder.add("key", "val");
     assertEquals("(this.key == :key)", paramBuilder.toString());
@@ -2215,7 +2216,7 @@ public class TestSentryStore extends org.junit.Assert {
     assertEquals("val", params.get("key"));
     assertEquals("Val1", params.get("key1"));
 
-    paramBuilder = new SentryStore.QueryParamBuilder(SentryStore.QueryParamBuilder.Op.OR);
+    paramBuilder = newQueryParamBuilder(QueryParamBuilder.Op.OR);
     paramBuilder.add("key", " Val ", true);
     paramBuilder.addNotNull("notNullField");
     paramBuilder.addNull("nullField");
@@ -2224,14 +2225,14 @@ public class TestSentryStore extends org.junit.Assert {
     params = paramBuilder.getArguments();
     assertEquals("Val", params.get("key"));
 
-    paramBuilder = new SentryStore.QueryParamBuilder()
+    paramBuilder = newQueryParamBuilder()
             .addNull("var1")
             .addNotNull("var2");
     assertEquals("(this.var1 == \"__NULL__\" && this.var2 != \"__NULL__\")",
             paramBuilder.toString());
 
     // Test newChild()
-    paramBuilder = new SentryStore.QueryParamBuilder();
+    paramBuilder = newQueryParamBuilder();
     paramBuilder
             .addString("e1")
             .addString("e2")
@@ -2250,7 +2251,7 @@ public class TestSentryStore extends org.junit.Assert {
     assertEquals("e4", params.get("v4"));
 
     // Test addSet
-    paramBuilder = new SentryStore.QueryParamBuilder();
+    paramBuilder = newQueryParamBuilder();
     Set<String>names = new HashSet<>();
     names.add("foo");
     names.add("bar");
@@ -2293,7 +2294,7 @@ public class TestSentryStore extends org.junit.Assert {
         roleName, privilege.getAction().toUpperCase());
 
     // Grant the privilege to role test-privilege and verify it has been persisted.
-    Map<TSentryPrivilege, Update> addPrivilegesUpdateMap = Maps.newHashMap();
+    Map<TSentryPrivilege, Updateable.Update> addPrivilegesUpdateMap = Maps.newHashMap();
     addPrivilegesUpdateMap.put(privilege, addUpdate);
     sentryStore.alterSentryRoleGrantPrivileges(grantor, roleName, Sets.newHashSet(privilege), addPrivilegesUpdateMap);
     MSentryRole role = sentryStore.getMSentryRoleByName(roleName);
@@ -2311,7 +2312,7 @@ public class TestSentryStore extends org.junit.Assert {
         roleName, privilege.getAction().toUpperCase());
 
     // Revoke the same privilege and verify it has been removed.
-    Map<TSentryPrivilege, Update> delPrivilegesUpdateMap = Maps.newHashMap();
+    Map<TSentryPrivilege, Updateable.Update> delPrivilegesUpdateMap = Maps.newHashMap();
     delPrivilegesUpdateMap.put(privilege, delUpdate);
     sentryStore.alterSentryRoleRevokePrivileges(grantor, roleName,
         Sets.newHashSet(privilege), delPrivilegesUpdateMap);


Mime
View raw message