You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ambari.apache.org by sw...@apache.org on 2014/08/11 23:04:44 UTC

git commit: AMBARI-6822. Privilege removal API should correctly process arrays.

Repository: ambari
Updated Branches:
  refs/heads/trunk bf49c0c2f -> b7f9612b5


AMBARI-6822. Privilege removal API should correctly process arrays.


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

Branch: refs/heads/trunk
Commit: b7f9612b5556d4f14906b54caebefd118a548e73
Parents: bf49c0c
Author: Siddharth Wagle <sw...@hortonworks.com>
Authored: Mon Aug 11 14:03:20 2014 -0700
Committer: Siddharth Wagle <sw...@hortonworks.com>
Committed: Mon Aug 11 14:04:31 2014 -0700

----------------------------------------------------------------------
 .../server/api/services/PrivilegeService.java   |  22 +++-
 .../AbstractControllerResourceProvider.java     |  30 -----
 .../internal/AbstractResourceProvider.java      |  31 ++++-
 .../AmbariPrivilegeResourceProvider.java        |   6 +
 .../ClusterPrivilegeResourceProvider.java       |   8 ++
 .../internal/PrivilegeResourceProvider.java     | 119 ++++++++++++++++---
 .../internal/ViewPrivilegeResourceProvider.java |  23 +++-
 .../ambari/server/orm/dao/PrivilegeDAO.java     |  38 ++++--
 .../server/orm/entities/PrivilegeEntity.java    |   2 +-
 .../api/services/PrivilegeServiceTest.java      |  12 ++
 .../AbstractControllerResourceProviderTest.java |  45 +------
 .../internal/AbstractResourceProviderTest.java  |  37 ++++++
 .../AmbariPrivilegeResourceProviderTest.java    |  75 +++++++++---
 .../ClusterPrivilegeResourceProviderTest.java   |   8 +-
 .../ViewPrivilegeResourceProviderTest.java      |  12 +-
 15 files changed, 323 insertions(+), 145 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/ambari/blob/b7f9612b/ambari-server/src/main/java/org/apache/ambari/server/api/services/PrivilegeService.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/api/services/PrivilegeService.java b/ambari-server/src/main/java/org/apache/ambari/server/api/services/PrivilegeService.java
index 8a0d200..2caa5d2 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/api/services/PrivilegeService.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/api/services/PrivilegeService.java
@@ -76,6 +76,7 @@ public abstract class PrivilegeService extends BaseService {
    * Handles: POST /privileges
    * Create a privilege.
    *
+   * @param body       request body
    * @param headers    http headers
    * @param ui         uri info
    *
@@ -108,9 +109,26 @@ public abstract class PrivilegeService extends BaseService {
   }
 
   /**
+   * Handles: PUT /privileges
+   * Update a set of privileges for the resource.
+   *
+   * @param body      request body
+   * @param headers   http headers
+   * @param ui        uri info
+   *
+   * @return information regarding the updated privileges
+   */
+  @PUT
+  @Produces("text/plain")
+  public Response updatePrivileges(String body, @Context HttpHeaders headers, @Context UriInfo ui) {
+    return handleRequest(headers, body, ui, Request.Type.PUT, createPrivilegeResource(null));
+  }
+
+  /**
    * Handles: DELETE /privileges
    * Delete privileges.
    *
+   * @param body      request body
    * @param headers   http headers
    * @param ui        uri info
    *
@@ -118,9 +136,9 @@ public abstract class PrivilegeService extends BaseService {
    */
   @DELETE
   @Produces("text/plain")
-  public Response deletePrivileges(@Context HttpHeaders headers, @Context UriInfo ui) {
+  public Response deletePrivileges(String body, @Context HttpHeaders headers, @Context UriInfo ui) {
 
-    return handleRequest(headers, null, ui, Request.Type.DELETE, createPrivilegeResource(null));
+    return handleRequest(headers, body, ui, Request.Type.DELETE, createPrivilegeResource(null));
   }
 
   /**

http://git-wip-us.apache.org/repos/asf/ambari/blob/b7f9612b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AbstractControllerResourceProvider.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AbstractControllerResourceProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AbstractControllerResourceProvider.java
index 1c8948f..71ddc8d 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AbstractControllerResourceProvider.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AbstractControllerResourceProvider.java
@@ -20,9 +20,6 @@ package org.apache.ambari.server.controller.internal;
 
 import org.apache.ambari.server.controller.AmbariManagementController;
 import org.apache.ambari.server.controller.ResourceProviderFactory;
-import org.apache.ambari.server.controller.predicate.ArrayPredicate;
-import org.apache.ambari.server.controller.predicate.EqualsPredicate;
-import org.apache.ambari.server.controller.spi.Predicate;
 import org.apache.ambari.server.controller.spi.Resource;
 import org.apache.ambari.server.controller.spi.ResourceProvider;
 import org.apache.ambari.server.controller.utilities.ClusterControllerHelper;
@@ -173,31 +170,4 @@ public abstract class AbstractControllerResourceProvider extends AbstractResourc
         ensureResourceProvider(type);
   }
 
-  /**
-   * Extracting given query_parameter value from the predicate
-   * @param queryParameterId  query parameter id
-   * @param predicate         predicate
-   * @return the query parameter
-   */
-  protected static Object getQueryParameterValue(String queryParameterId, Predicate predicate) {
-
-    Object result = null;
-
-    if (predicate instanceof EqualsPredicate) {
-      EqualsPredicate equalsPredicate = (EqualsPredicate) predicate;
-      if (queryParameterId.equals(equalsPredicate.getPropertyId())) {
-        return equalsPredicate.getValue();
-      }
-    } else if (predicate instanceof ArrayPredicate) {
-      ArrayPredicate arrayPredicate  = (ArrayPredicate) predicate;
-      for (Predicate predicateItem : arrayPredicate.getPredicates()) {
-        result = getQueryParameterValue(queryParameterId, predicateItem);
-        if (result != null) {
-          return result;
-        }
-      }
-
-    }
-    return result;
-  }
 }

http://git-wip-us.apache.org/repos/asf/ambari/blob/b7f9612b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AbstractResourceProvider.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AbstractResourceProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AbstractResourceProvider.java
index 01c526d..53e683c 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AbstractResourceProvider.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AbstractResourceProvider.java
@@ -32,7 +32,8 @@ import org.apache.ambari.server.ObjectNotFoundException;
 import org.apache.ambari.server.ParentObjectNotFoundException;
 import org.apache.ambari.server.controller.ConfigurationRequest;
 import org.apache.ambari.server.controller.RequestStatusResponse;
-import org.apache.ambari.server.controller.ServiceConfigVersionRequest;
+import org.apache.ambari.server.controller.predicate.ArrayPredicate;
+import org.apache.ambari.server.controller.predicate.EqualsPredicate;
 import org.apache.ambari.server.controller.spi.*;
 import org.apache.ambari.server.controller.utilities.PredicateHelper;
 import org.apache.ambari.server.controller.utilities.PropertyHelper;
@@ -221,6 +222,34 @@ public abstract class AbstractResourceProvider extends BaseProvider implements R
   }
 
   /**
+   * Extracting given query_parameter value from the predicate
+   * @param queryParameterId  query parameter id
+   * @param predicate         predicate
+   * @return the query parameter
+   */
+  protected static Object getQueryParameterValue(String queryParameterId, Predicate predicate) {
+
+    Object result = null;
+
+    if (predicate instanceof EqualsPredicate) {
+      EqualsPredicate equalsPredicate = (EqualsPredicate) predicate;
+      if (queryParameterId.equals(equalsPredicate.getPropertyId())) {
+        return equalsPredicate.getValue();
+      }
+    } else if (predicate instanceof ArrayPredicate) {
+      ArrayPredicate arrayPredicate  = (ArrayPredicate) predicate;
+      for (Predicate predicateItem : arrayPredicate.getPredicates()) {
+        result = getQueryParameterValue(queryParameterId, predicateItem);
+        if (result != null) {
+          return result;
+        }
+      }
+
+    }
+    return result;
+  }
+
+  /**
    * Invoke a command against the Ambari backend to create resources and map
    * any {@link AmbariException} to the types appropriate for the
    * {@link ResourceProvider} interface.

http://git-wip-us.apache.org/repos/asf/ambari/blob/b7f9612b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AmbariPrivilegeResourceProvider.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AmbariPrivilegeResourceProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AmbariPrivilegeResourceProvider.java
index e5fe0e9..b24f994 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AmbariPrivilegeResourceProvider.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AmbariPrivilegeResourceProvider.java
@@ -17,6 +17,7 @@
  */
 package org.apache.ambari.server.controller.internal;
 
+import org.apache.ambari.server.controller.spi.Predicate;
 import org.apache.ambari.server.controller.spi.Resource;
 import org.apache.ambari.server.orm.entities.ResourceEntity;
 
@@ -76,4 +77,9 @@ public class AmbariPrivilegeResourceProvider extends PrivilegeResourceProvider<O
     // the singleton Ambari entity is implied
     return Collections.singletonMap(ResourceEntity.AMBARI_RESOURCE_ID, null);
   }
+
+  @Override
+  public Long getResourceEntityId(Predicate predicate) {
+    return ResourceEntity.AMBARI_RESOURCE_ID;
+  }
 }

http://git-wip-us.apache.org/repos/asf/ambari/blob/b7f9612b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterPrivilegeResourceProvider.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterPrivilegeResourceProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterPrivilegeResourceProvider.java
index 46bfea0..c9e2a83 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterPrivilegeResourceProvider.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterPrivilegeResourceProvider.java
@@ -17,6 +17,7 @@
  */
 package org.apache.ambari.server.controller.internal;
 
+import org.apache.ambari.server.controller.spi.Predicate;
 import org.apache.ambari.server.AmbariException;
 import org.apache.ambari.server.controller.spi.Resource;
 import org.apache.ambari.server.controller.utilities.PropertyHelper;
@@ -135,6 +136,13 @@ public class ClusterPrivilegeResourceProvider extends PrivilegeResourceProvider<
     return Collections.singletonMap(clusterEntity.getResource().getId(), clusterEntity);
   }
 
+  @Override
+  public Long getResourceEntityId(Predicate predicate) {
+    final String clusterName = getQueryParameterValue(PRIVILEGE_CLUSTER_NAME_PROPERTY_ID, predicate).toString();
+    final ClusterEntity clusterEntity = clusterDAO.findByName(clusterName);
+    return clusterEntity.getResource().getId();
+  }
+
 
   // ----- helper methods ----------------------------------------------------
 

http://git-wip-us.apache.org/repos/asf/ambari/blob/b7f9612b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/PrivilegeResourceProvider.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/PrivilegeResourceProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/PrivilegeResourceProvider.java
index 42ab21d..60d6396 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/PrivilegeResourceProvider.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/PrivilegeResourceProvider.java
@@ -18,6 +18,15 @@
 
 package org.apache.ambari.server.controller.internal;
 
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
 import org.apache.ambari.server.AmbariException;
 import org.apache.ambari.server.DuplicateResourceException;
 import org.apache.ambari.server.controller.spi.NoSuchParentResourceException;
@@ -29,7 +38,6 @@ import org.apache.ambari.server.controller.spi.Resource;
 import org.apache.ambari.server.controller.spi.ResourceAlreadyExistsException;
 import org.apache.ambari.server.controller.spi.SystemException;
 import org.apache.ambari.server.controller.spi.UnsupportedPropertyException;
-import org.apache.ambari.server.controller.utilities.PropertyHelper;
 import org.apache.ambari.server.orm.dao.GroupDAO;
 import org.apache.ambari.server.orm.dao.PermissionDAO;
 import org.apache.ambari.server.orm.dao.PrincipalDAO;
@@ -44,14 +52,6 @@ import org.apache.ambari.server.orm.entities.PrivilegeEntity;
 import org.apache.ambari.server.orm.entities.ResourceEntity;
 import org.apache.ambari.server.orm.entities.UserEntity;
 
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.LinkedList;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-
 /**
  * Abstract resource provider for privilege resources.
  */
@@ -139,12 +139,21 @@ public abstract class PrivilegeResourceProvider<T> extends AbstractResourceProvi
   /**
    * Get the entities for the owning resources from the given properties.
    *
-   * @param properties  the set of properties
+   * @param properties the set of properties
    *
    * @return the entities
    */
   public abstract Map<Long, T> getResourceEntities(Map<String, Object> properties);
 
+  /**
+   * Get the id for the resource specified by predicate.
+   *
+   * @param predicate predicate
+   *
+   * @return the resource id
+   */
+  public abstract Long getResourceEntityId(Predicate predicate);
+
 
   // ----- ResourceProvider --------------------------------------------------
 
@@ -219,14 +228,16 @@ public abstract class PrivilegeResourceProvider<T> extends AbstractResourceProvi
   @Override
   public RequestStatus updateResources(Request request, Predicate predicate)
       throws SystemException, UnsupportedPropertyException, NoSuchResourceException, NoSuchParentResourceException {
-    throw new UnsupportedOperationException("Not supported.");
+    modifyResources(getUpdateCommand(request, predicate));
+    notifyUpdate(resourceType, request, predicate);
+    return getRequestStatus(null);
   }
 
   @Override
   public RequestStatus deleteResources(Predicate predicate)
       throws SystemException, UnsupportedPropertyException, NoSuchResourceException, NoSuchParentResourceException {
     modifyResources(getDeleteCommand(predicate));
-    notifyDelete(Resource.Type.ViewInstance, predicate);
+    notifyDelete(resourceType, predicate);
     return getRequestStatus(null);
   }
 
@@ -331,6 +342,11 @@ public abstract class PrivilegeResourceProvider<T> extends AbstractResourceProvi
       if (userEntity != null) {
         entity.setPrincipal(principalDAO.findById(userEntity.getPrincipal().getId()));
       }
+    } else {
+      throw new AmbariException("Unknown principal type " + principalType);
+    }
+    if (entity.getPrincipal() == null) {
+      throw new AmbariException("Could not find " + principalType + " named " + principalName);
     }
     return entity;
   }
@@ -378,13 +394,16 @@ public abstract class PrivilegeResourceProvider<T> extends AbstractResourceProvi
       @Override
       public Void invoke() throws AmbariException {
         try {
-          Set<Resource> resources = getResources(PropertyHelper.getReadRequest(), predicate);
-          for (Resource resource : resources) {
-
-            PrivilegeEntity entity =
-                privilegeDAO.findById((Integer) resource.getPropertyValue(PRIVILEGE_ID_PROPERTY_ID));
-
+          for (Map<String, Object> resource : getPropertyMaps(predicate)) {
+            if (resource.get(PRIVILEGE_ID_PROPERTY_ID) == null) {
+              throw new AmbariException("Privilege ID should be provided for this request");
+            }
+            PrivilegeEntity entity = privilegeDAO.findById(Integer.valueOf(resource.get(PRIVILEGE_ID_PROPERTY_ID).toString()));
             if (entity != null) {
+              if (!checkResourceTypes(entity)) {
+                throw new AmbariException("Can't remove " + entity.getPermission().getResourceType().getName() +
+                    " permission from a " + entity.getResource().getResourceType().getName() + " resource.");
+              }
               privilegeDAO.remove(entity);
             }
           }
@@ -395,5 +414,69 @@ public abstract class PrivilegeResourceProvider<T> extends AbstractResourceProvi
       }
     };
   }
+
+  private Command<Void> getUpdateCommand(final Request request, final Predicate predicate) {
+    return new Command<Void>() {
+      @Override
+      public Void invoke() throws AmbariException {
+        Long resource = null;
+        final List<PrivilegeEntity> requiredEntities = new ArrayList<PrivilegeEntity>();
+        for (Map<String, Object> properties: request.getProperties()) {
+          Set<Long> resourceIds = getResourceEntities(properties).keySet();
+          Long      resourceId  = resourceIds.iterator().next();
+
+          if (resource != null && resourceId != resource) {
+            throw new AmbariException("Can't update privileges of multiple resources in one request");
+          }
+          resource = resourceId;
+
+          PrivilegeEntity entity = toEntity(properties, resourceId);
+          requiredEntities.add(entity);
+        }
+        if (resource == null) {
+          // request body is empty, use predicate instead
+          resource = getResourceEntityId(predicate);
+        }
+        final List<PrivilegeEntity> currentPrivileges = privilegeDAO.findByResourceId(resource);
+
+        for (PrivilegeEntity requiredPrivilege: requiredEntities) {
+          boolean isInBothLists = false;
+          for (PrivilegeEntity currentPrivilege: currentPrivileges) {
+            if (requiredPrivilege.getPermission().getPermissionName().equals(currentPrivilege.getPermission().getPermissionName()) &&
+                    requiredPrivilege.getPrincipal().getId().equals(currentPrivilege.getPrincipal().getId())) {
+              isInBothLists = true;
+              break;
+            }
+          }
+          if (!isInBothLists) {
+            if (!checkResourceTypes(requiredPrivilege)) {
+              throw new AmbariException("Can't grant " + requiredPrivilege.getPermission().getResourceType().getName() +
+                  " permission on a " + requiredPrivilege.getResource().getResourceType().getName() + " resource.");
+            }
+
+            privilegeDAO.create(requiredPrivilege);
+          }
+        }
+        for (PrivilegeEntity currentPrivilege: currentPrivileges) {
+          boolean isInBothLists = false;
+          for (PrivilegeEntity requiredPrivilege: requiredEntities) {
+            if (requiredPrivilege.getPermission().getPermissionName().equals(currentPrivilege.getPermission().getPermissionName()) &&
+                    requiredPrivilege.getPrincipal().getId().equals(currentPrivilege.getPrincipal().getId())) {
+              isInBothLists = true;
+              break;
+            }
+          }
+          if (!isInBothLists) {
+            if (!checkResourceTypes(currentPrivilege)) {
+              throw new AmbariException("Can't remove " + currentPrivilege.getPermission().getResourceType().getName() +
+                  " permission from a " + currentPrivilege.getResource().getResourceType().getName() + " resource.");
+            }
+            privilegeDAO.remove(currentPrivilege);
+          }
+        }
+        return null;
+      }
+    };
+  }
 }
 

http://git-wip-us.apache.org/repos/asf/ambari/blob/b7f9612b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ViewPrivilegeResourceProvider.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ViewPrivilegeResourceProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ViewPrivilegeResourceProvider.java
index 6d10797..57eb28e 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ViewPrivilegeResourceProvider.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ViewPrivilegeResourceProvider.java
@@ -17,7 +17,14 @@
  */
 package org.apache.ambari.server.controller.internal;
 
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
 import org.apache.ambari.server.AmbariException;
+import org.apache.ambari.server.controller.spi.Predicate;
 import org.apache.ambari.server.controller.spi.Resource;
 import org.apache.ambari.server.orm.entities.GroupEntity;
 import org.apache.ambari.server.orm.entities.PermissionEntity;
@@ -29,12 +36,6 @@ import org.apache.ambari.server.orm.entities.ViewEntity;
 import org.apache.ambari.server.orm.entities.ViewInstanceEntity;
 import org.apache.ambari.server.view.ViewRegistry;
 
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.Map;
-import java.util.Set;
-
 /**
  * Resource provider for view privilege resources.
  */
@@ -139,6 +140,16 @@ public class ViewPrivilegeResourceProvider extends PrivilegeResourceProvider<Vie
     return resourceEntities;
   }
 
+  @Override
+  public Long getResourceEntityId(Predicate predicate) {
+    final ViewRegistry viewRegistry = ViewRegistry.getInstance();
+
+    final String viewName     = getQueryParameterValue(PRIVILEGE_VIEW_NAME_PROPERTY_ID, predicate).toString();
+    final String viewVersion  = getQueryParameterValue(PRIVILEGE_VIEW_VERSION_PROPERTY_ID, predicate).toString();
+    final String instanceName = getQueryParameterValue(PRIVILEGE_INSTANCE_NAME_PROPERTY_ID, predicate).toString();
+    final ViewInstanceEntity viewInstanceEntity = viewRegistry.getInstanceDefinition(viewName, viewVersion, instanceName);
+    return viewInstanceEntity.getResource().getId();
+  }
 
   // ----- helper methods ----------------------------------------------------
 

http://git-wip-us.apache.org/repos/asf/ambari/blob/b7f9612b/ambari-server/src/main/java/org/apache/ambari/server/orm/dao/PrivilegeDAO.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/orm/dao/PrivilegeDAO.java b/ambari-server/src/main/java/org/apache/ambari/server/orm/dao/PrivilegeDAO.java
index 2ad14d2..4fda7bc 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/orm/dao/PrivilegeDAO.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/orm/dao/PrivilegeDAO.java
@@ -18,19 +18,21 @@
 
 package org.apache.ambari.server.orm.dao;
 
-import com.google.inject.Inject;
-import com.google.inject.Provider;
-import com.google.inject.Singleton;
-import com.google.inject.persist.Transactional;
+import java.util.List;
+
+import javax.persistence.EntityManager;
+import javax.persistence.TypedQuery;
+
 import org.apache.ambari.server.orm.entities.PermissionEntity;
 import org.apache.ambari.server.orm.entities.PrincipalEntity;
 import org.apache.ambari.server.orm.entities.PrivilegeEntity;
 import org.apache.ambari.server.orm.entities.ResourceEntity;
 
-import javax.persistence.EntityManager;
-import javax.persistence.TypedQuery;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.Singleton;
+import com.google.inject.persist.Transactional;
 import java.util.Collections;
-import java.util.List;
 
 /**
  * Privilege Data Access Object.
@@ -46,20 +48,20 @@ public class PrivilegeDAO {
   DaoUtils daoUtils;
 
   /**
-   * Find a resource with the given id.
+   * Find a privilege with the given id.
    *
-   * @param id  type id
+   * @param id type id
    *
-   * @return  a matching resource type  or null
+   * @return a matching privilege or null
    */
   public PrivilegeEntity findById(Integer id) {
     return entityManagerProvider.get().find(PrivilegeEntity.class, id);
   }
 
   /**
-   * Find all resources.
+   * Find all privileges.
    *
-   * @return all resources or an empty List
+   * @return all privileges or an empty List
    */
   public List<PrivilegeEntity> findAll() {
     TypedQuery<PrivilegeEntity> query = entityManagerProvider.get().createQuery("SELECT privilege FROM PrivilegeEntity privilege", PrivilegeEntity.class);
@@ -67,6 +69,18 @@ public class PrivilegeDAO {
   }
 
   /**
+   * Find all privileges for given resource.
+   *
+   * @param id ID of the resource
+   * @return all resource privileges or an empty list
+   */
+  public List<PrivilegeEntity> findByResourceId(Long id) {
+    TypedQuery<PrivilegeEntity> query = entityManagerProvider.get().createQuery("SELECT privilege FROM PrivilegeEntity privilege WHERE privilege.resource.id = :resource_id", PrivilegeEntity.class);
+    query.setParameter("resource_id", id);
+    return daoUtils.selectList(query);
+  }
+
+  /**
    * Determine whether or not the given privilege entity exists.
    *
    * @param entity  the privilege entity

http://git-wip-us.apache.org/repos/asf/ambari/blob/b7f9612b/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/PrivilegeEntity.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/PrivilegeEntity.java b/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/PrivilegeEntity.java
index 26802e6..fe97c7d 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/PrivilegeEntity.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/PrivilegeEntity.java
@@ -173,7 +173,7 @@ public class PrivilegeEntity {
 
   @Override
   public int hashCode() {
-    int result = id.hashCode();
+    int result = id != null ? id.hashCode() : 0;
     result = 31 * result + (permission != null ? permission.hashCode() : 0);
     result = 31 * result + (resource != null ? resource.hashCode() : 0);
     result = 31 * result + (principal != null ? principal.hashCode() : 0);

http://git-wip-us.apache.org/repos/asf/ambari/blob/b7f9612b/ambari-server/src/test/java/org/apache/ambari/server/api/services/PrivilegeServiceTest.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/api/services/PrivilegeServiceTest.java b/ambari-server/src/test/java/org/apache/ambari/server/api/services/PrivilegeServiceTest.java
index 516fad9..d9f3e28 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/api/services/PrivilegeServiceTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/api/services/PrivilegeServiceTest.java
@@ -65,12 +65,24 @@ public class PrivilegeServiceTest extends BaseServiceTest {
     args = new Object[] {"body", getHttpHeaders(), getUriInfo(), "id"};
     listInvocations.add(new ServiceTestInvocation(Request.Type.PUT, service, m, args, "body"));
 
+    //updatePrivileges
+    service = new TestPrivilegeService(null);
+    m = service.getClass().getMethod("updatePrivileges", String.class, HttpHeaders.class, UriInfo.class);
+    args = new Object[] {"body", getHttpHeaders(), getUriInfo()};
+    listInvocations.add(new ServiceTestInvocation(Request.Type.PUT, service, m, args, "body"));
+
     //deletePrivilege
     service = new TestPrivilegeService("id");
     m = service.getClass().getMethod("deletePrivilege", HttpHeaders.class, UriInfo.class, String.class);
     args = new Object[] {getHttpHeaders(), getUriInfo(), "id"};
     listInvocations.add(new ServiceTestInvocation(Request.Type.DELETE, service, m, args, null));
 
+    //deletePrivileges
+    service = new TestPrivilegeService(null);
+    m = service.getClass().getMethod("deletePrivileges", String.class, HttpHeaders.class, UriInfo.class);
+    args = new Object[] {"body", getHttpHeaders(), getUriInfo()};
+    listInvocations.add(new ServiceTestInvocation(Request.Type.DELETE, service, m, args, "body"));
+
     return listInvocations;
   }
 

http://git-wip-us.apache.org/repos/asf/ambari/blob/b7f9612b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AbstractControllerResourceProviderTest.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AbstractControllerResourceProviderTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AbstractControllerResourceProviderTest.java
index 8453aad..5414b52 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AbstractControllerResourceProviderTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AbstractControllerResourceProviderTest.java
@@ -22,10 +22,8 @@ import junit.framework.Assert;
 import org.apache.ambari.server.controller.AmbariManagementController;
 import org.apache.ambari.server.controller.MaintenanceStateHelper;
 import org.apache.ambari.server.controller.ResourceProviderFactory;
-import org.apache.ambari.server.controller.spi.Predicate;
 import org.apache.ambari.server.controller.spi.Resource;
 import org.apache.ambari.server.controller.spi.ResourceProvider;
-import org.apache.ambari.server.controller.utilities.PredicateBuilder;
 import org.junit.Test;
 
 import java.util.HashMap;
@@ -56,15 +54,15 @@ public class AbstractControllerResourceProviderTest {
     Map<Resource.Type, String> keyPropertyIds = new HashMap<Resource.Type, String>();
 
     AmbariManagementController managementController = createMock(AmbariManagementController.class);
-    
+
     ResourceProviderFactory factory = createMock(ResourceProviderFactory.class);
 
     MaintenanceStateHelper maintenanceStateHelper = createNiceMock(MaintenanceStateHelper.class);
     ResourceProvider serviceResourceProvider = new ServiceResourceProvider(propertyIds, keyPropertyIds, managementController, maintenanceStateHelper);
     expect(factory.getServiceResourceProvider(propertyIds, keyPropertyIds, managementController)).andReturn(serviceResourceProvider);
-    
+
     AbstractControllerResourceProvider.init(factory);
-    
+
     replay(managementController, factory, maintenanceStateHelper);
 
     AbstractResourceProvider provider =
@@ -77,41 +75,4 @@ public class AbstractControllerResourceProviderTest {
     Assert.assertTrue(provider instanceof ServiceResourceProvider);
   }
 
-  @Test
-  public void testGetQueryParameterValue() {
-
-    String queryParameterId1 = "qp/variable1";
-    String queryParameterValue1 = "value1";
-    String queryParameterId2 = "qp/variable2";
-    String queryParameterValue2 = "value2";
-
-    //Array of predicates
-    Predicate  predicate = new PredicateBuilder().property(queryParameterId1).equals(queryParameterValue1).
-        and().property(queryParameterId2).equals(queryParameterValue2).toPredicate();
-
-    Assert.assertEquals(queryParameterValue1, AbstractControllerResourceProvider.getQueryParameterValue(queryParameterId1, predicate));
-    Assert.assertFalse(queryParameterValue2.equals(AbstractControllerResourceProvider.getQueryParameterValue(queryParameterId1, predicate)));
-    Assert.assertNull(AbstractControllerResourceProvider.getQueryParameterValue("queryParameterIdNotFound", predicate));
-
-    String queryParameterId3 = "qp/variable3";
-    String queryParameterValue3 = "value3";
-
-    // tests ServiceInfo/state=INSTALLED&params/run_smoke_test=true
-    //Array of arrays of predicates
-    predicate = new PredicateBuilder().property(queryParameterId3).equals(queryParameterValue3).
-        and().begin().property(queryParameterId1).equals(queryParameterValue1).
-        and().property(queryParameterId2).equals(queryParameterValue2).end().toPredicate();
-
-    Assert.assertEquals(queryParameterValue1, AbstractControllerResourceProvider.
-        getQueryParameterValue(queryParameterId1, predicate));
-    Assert.assertFalse(queryParameterValue2.equals(AbstractControllerResourceProvider.
-        getQueryParameterValue(queryParameterId1, predicate)));
-    Assert.assertNull(AbstractControllerResourceProvider.
-        getQueryParameterValue("queryParameterIdNotFound", predicate));
-
-    Assert.assertEquals(queryParameterValue3, AbstractControllerResourceProvider.
-        getQueryParameterValue(queryParameterId3, predicate));
-
-  }
-
   }

http://git-wip-us.apache.org/repos/asf/ambari/blob/b7f9612b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AbstractResourceProviderTest.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AbstractResourceProviderTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AbstractResourceProviderTest.java
index 0a56586..c1f4f5b 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AbstractResourceProviderTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AbstractResourceProviderTest.java
@@ -235,6 +235,43 @@ public class AbstractResourceProviderTest {
     }
   }
 
+  @Test
+  public void testGetQueryParameterValue() {
+
+    String queryParameterId1 = "qp/variable1";
+    String queryParameterValue1 = "value1";
+    String queryParameterId2 = "qp/variable2";
+    String queryParameterValue2 = "value2";
+
+    //Array of predicates
+    Predicate  predicate = new PredicateBuilder().property(queryParameterId1).equals(queryParameterValue1).
+        and().property(queryParameterId2).equals(queryParameterValue2).toPredicate();
+
+    Assert.assertEquals(queryParameterValue1, AbstractResourceProvider.getQueryParameterValue(queryParameterId1, predicate));
+    Assert.assertFalse(queryParameterValue2.equals(AbstractResourceProvider.getQueryParameterValue(queryParameterId1, predicate)));
+    Assert.assertNull(AbstractResourceProvider.getQueryParameterValue("queryParameterIdNotFound", predicate));
+
+    String queryParameterId3 = "qp/variable3";
+    String queryParameterValue3 = "value3";
+
+    // tests ServiceInfo/state=INSTALLED&params/run_smoke_test=true
+    //Array of arrays of predicates
+    predicate = new PredicateBuilder().property(queryParameterId3).equals(queryParameterValue3).
+        and().begin().property(queryParameterId1).equals(queryParameterValue1).
+        and().property(queryParameterId2).equals(queryParameterValue2).end().toPredicate();
+
+    Assert.assertEquals(queryParameterValue1, AbstractResourceProvider.
+        getQueryParameterValue(queryParameterId1, predicate));
+    Assert.assertFalse(queryParameterValue2.equals(AbstractResourceProvider.
+        getQueryParameterValue(queryParameterId1, predicate)));
+    Assert.assertNull(AbstractResourceProvider.
+        getQueryParameterValue("queryParameterIdNotFound", predicate));
+
+    Assert.assertEquals(queryParameterValue3, AbstractResourceProvider.
+        getQueryParameterValue(queryParameterId3, predicate));
+
+  }
+
 
   // ----- helper methods ----------------------------------------------------
 

http://git-wip-us.apache.org/repos/asf/ambari/blob/b7f9612b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AmbariPrivilegeResourceProviderTest.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AmbariPrivilegeResourceProviderTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AmbariPrivilegeResourceProviderTest.java
index 44c8810..94fb7dd 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AmbariPrivilegeResourceProviderTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AmbariPrivilegeResourceProviderTest.java
@@ -18,6 +18,21 @@
 
 package org.apache.ambari.server.controller.internal;
 
+import static org.easymock.EasyMock.createNiceMock;
+import static org.easymock.EasyMock.createStrictMock;
+import static org.easymock.EasyMock.expect;
+import static org.easymock.EasyMock.replay;
+import static org.easymock.EasyMock.reset;
+import static org.easymock.EasyMock.verify;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
 import org.apache.ambari.server.controller.spi.Request;
 import org.apache.ambari.server.controller.spi.Resource;
 import org.apache.ambari.server.controller.utilities.PropertyHelper;
@@ -33,24 +48,14 @@ import org.apache.ambari.server.orm.entities.PrincipalEntity;
 import org.apache.ambari.server.orm.entities.PrincipalTypeEntity;
 import org.apache.ambari.server.orm.entities.PrivilegeEntity;
 import org.apache.ambari.server.orm.entities.ResourceEntity;
+import org.apache.ambari.server.orm.entities.ResourceTypeEntity;
 import org.apache.ambari.server.orm.entities.UserEntity;
+import org.easymock.EasyMock;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Test;
 
-import java.util.Collections;
-import java.util.LinkedList;
-import java.util.List;
-import java.util.Set;
-
-import static org.easymock.EasyMock.createNiceMock;
-import static org.easymock.EasyMock.createStrictMock;
-import static org.easymock.EasyMock.expect;
-import static org.easymock.EasyMock.replay;
-import static org.easymock.EasyMock.reset;
-import static org.easymock.EasyMock.verify;
-
 /**
  * AmbariPrivilegeResourceProvider tests.
  */
@@ -129,13 +134,47 @@ public class AmbariPrivilegeResourceProviderTest {
   public void testUpdateResources() throws Exception {
     PrivilegeResourceProvider provider = new AmbariPrivilegeResourceProvider();
 
+    PrivilegeEntity privilegeEntity = createNiceMock(PrivilegeEntity.class);
+    ResourceEntity resourceEntity = createNiceMock(ResourceEntity.class);
+    ResourceTypeEntity resourceTypeEntity = createNiceMock(ResourceTypeEntity.class);
     Request request = createNiceMock(Request.class);
+    PermissionEntity permissionEntity = createNiceMock(PermissionEntity.class);
+    PrincipalEntity principalEntity = createNiceMock(PrincipalEntity.class);
+    UserEntity userEntity = createNiceMock(UserEntity.class);
+
+    expect(privilegeDAO.findByResourceId(1L)).andReturn(Collections.singletonList(privilegeEntity)).anyTimes();
+    privilegeDAO.remove(privilegeEntity);
+    EasyMock.expectLastCall().anyTimes();
+    expect(request.getProperties()).andReturn(new HashSet<Map<String,Object>>() {
+      {
+        add(new HashMap<String, Object>() {
+          {
+           put(PrivilegeResourceProvider.PERMISSION_NAME_PROPERTY_ID, "READ");
+           put(PrivilegeResourceProvider.PRINCIPAL_NAME_PROPERTY_ID, "admin");
+           put(PrivilegeResourceProvider.PRINCIPAL_TYPE_PROPERTY_ID, "user");
+          }
+        });
+      }
+    }).anyTimes();
+    expect(permissionDAO.findPermissionByNameAndType(EasyMock.eq("READ"), EasyMock.<ResourceTypeEntity> anyObject())).andReturn(permissionEntity);
+    expect(resourceDAO.findById(EasyMock.anyLong())).andReturn(resourceEntity);
+    expect(userDAO.findLocalUserByName("admin")).andReturn(userEntity);
+    expect(principalDAO.findById(EasyMock.anyLong())).andReturn(principalEntity);
+    expect(userEntity.getPrincipal()).andReturn(principalEntity).anyTimes();
+    expect(principalEntity.getId()).andReturn(2L).anyTimes();
+    expect(permissionEntity.getPermissionName()).andReturn("READ").anyTimes();
+    expect(privilegeEntity.getPermission()).andReturn(permissionEntity).anyTimes();
+    expect(resourceTypeEntity.getId()).andReturn(3).anyTimes();
+    expect(resourceEntity.getResourceType()).andReturn(resourceTypeEntity).anyTimes();
+    expect(permissionEntity.getResourceType()).andReturn(resourceTypeEntity).anyTimes();
+    expect(privilegeEntity.getPrincipal()).andReturn(principalEntity).anyTimes();
+    privilegeDAO.create(EasyMock.<PrivilegeEntity> anyObject());
+    EasyMock.expectLastCall().anyTimes();
+
+    replay(privilegeEntity, privilegeDAO, request, permissionDAO, permissionEntity, resourceEntity, resourceDAO, principalEntity, principalDAO, userDAO, userEntity, resourceTypeEntity);
+
+    provider.updateResources(request, null);
 
-    try {
-      provider.updateResources(request, null);
-      Assert.fail("expected UnsupportedOperationException");
-    } catch (UnsupportedOperationException e) {
-      // expected
-    }
+    verify(privilegeEntity, privilegeDAO, request, permissionDAO, permissionEntity, resourceEntity, resourceDAO, principalEntity, principalDAO, userDAO, userEntity, resourceTypeEntity);
   }
 }

http://git-wip-us.apache.org/repos/asf/ambari/blob/b7f9612b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterPrivilegeResourceProviderTest.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterPrivilegeResourceProviderTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterPrivilegeResourceProviderTest.java
index 864fc14..148c139 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterPrivilegeResourceProviderTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterPrivilegeResourceProviderTest.java
@@ -142,7 +142,6 @@ public class ClusterPrivilegeResourceProviderTest {
 
   @Test
   public void testUpdateResources() throws Exception {
-
     PermissionEntity permissionEntity = createNiceMock(PermissionEntity.class);
     Request request = createNiceMock(Request.class);
 
@@ -155,9 +154,10 @@ public class ClusterPrivilegeResourceProviderTest {
     PrivilegeResourceProvider provider = new ClusterPrivilegeResourceProvider();
     try {
       provider.updateResources(request, null);
-      Assert.fail("expected UnsupportedOperationException");
-    } catch (UnsupportedOperationException e) {
-      // expected
+    } catch (Exception ex) {
+      // omit the exception, this method is from abstract class and tested in
+      // AmbariPrivilegeResourceProvider#testUpdateResources
+      // just check that permissions are okay
     }
 
     verify(permissionDAO, permissionEntity, request);

http://git-wip-us.apache.org/repos/asf/ambari/blob/b7f9612b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ViewPrivilegeResourceProviderTest.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ViewPrivilegeResourceProviderTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ViewPrivilegeResourceProviderTest.java
index d61a0b6..695ceea 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ViewPrivilegeResourceProviderTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ViewPrivilegeResourceProviderTest.java
@@ -18,7 +18,6 @@
 
 package org.apache.ambari.server.controller.internal;
 
-import org.apache.ambari.server.controller.spi.Request;
 import org.apache.ambari.server.controller.spi.Resource;
 import org.apache.ambari.server.controller.utilities.PropertyHelper;
 import org.apache.ambari.server.orm.dao.GroupDAO;
@@ -154,16 +153,7 @@ public class ViewPrivilegeResourceProviderTest {
 
   @Test
   public void testUpdateResources() throws Exception {
-    PrivilegeResourceProvider provider = new ViewPrivilegeResourceProvider();
-
-    Request request = createNiceMock(Request.class);
-
-    try {
-      provider.updateResources(request, null);
-      Assert.fail("expected UnsupportedOperationException");
-    } catch (UnsupportedOperationException e) {
-      // expected
-    }
+    // see AmbariPrivilegeResourceProvider#testUpdateResources
   }
 }