You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ambari.apache.org by jl...@apache.org on 2015/12/28 21:49:32 UTC

ambari git commit: AMBARI-14452: It should be possible to delete service components if all related host components are stopped (Ajit Kumar via jluniya)

Repository: ambari
Updated Branches:
  refs/heads/trunk 7d6caf6cd -> b328f6dbe


AMBARI-14452: It should be possible to delete service components if all related host components are stopped (Ajit Kumar via jluniya)


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

Branch: refs/heads/trunk
Commit: b328f6dbe3ed94df662858962aad37b4e889bcd9
Parents: 7d6caf6
Author: Jayush Luniya <jl...@hortonworks.com>
Authored: Mon Dec 28 12:49:51 2015 -0800
Committer: Jayush Luniya <jl...@hortonworks.com>
Committed: Mon Dec 28 12:49:51 2015 -0800

----------------------------------------------------------------------
 .../controller/ServiceComponentRequest.java     |   7 +-
 .../internal/ComponentResourceProvider.java     | 604 +++++++------------
 .../internal/ComponentResourceProviderTest.java | 213 +------
 3 files changed, 234 insertions(+), 590 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/ambari/blob/b328f6db/ambari-server/src/main/java/org/apache/ambari/server/controller/ServiceComponentRequest.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/ServiceComponentRequest.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/ServiceComponentRequest.java
index 63af85e..78b9897 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/controller/ServiceComponentRequest.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/ServiceComponentRequest.java
@@ -42,7 +42,6 @@ public class ServiceComponentRequest {
   public ServiceComponentRequest(String clusterName,
                                  String serviceName, String componentName,
                                  String desiredState, String componentCategory) {
-    super();
     this.clusterName = clusterName;
     this.serviceName = serviceName;
     this.componentName = componentName;
@@ -113,4 +112,10 @@ public class ServiceComponentRequest {
   public void setComponentCategory(String componentCategory) {
     this.componentCategory = componentCategory;
   }
+
+  @Override
+  public String toString() {
+    return String.format("[clusterName=%s, serviceName=%s, componentName=%s, desiredState=%s, componentCategory=%s]",
+        clusterName, serviceName, clusterName, desiredState, componentCategory);
+  }
 }

http://git-wip-us.apache.org/repos/asf/ambari/blob/b328f6db/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ComponentResourceProvider.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ComponentResourceProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ComponentResourceProvider.java
index ddbf60a..3ad6e64 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ComponentResourceProvider.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ComponentResourceProvider.java
@@ -65,13 +65,14 @@ import org.apache.ambari.server.state.State;
 import com.google.inject.assistedinject.Assisted;
 import com.google.inject.assistedinject.AssistedInject;
 import com.google.inject.persist.Transactional;
+import org.apache.commons.lang.StringUtils;
+import org.apache.commons.lang.Validate;
 
 /**
  * Resource provider for component resources.
  */
 public class ComponentResourceProvider extends AbstractControllerResourceProvider {
 
-
   // ----- Property ID constants ---------------------------------------------
 
   // Components
@@ -84,12 +85,13 @@ public class ComponentResourceProvider extends AbstractControllerResourceProvide
   protected static final String COMPONENT_STARTED_COUNT_PROPERTY_ID   = "ServiceComponentInfo/started_count";
   protected static final String COMPONENT_INSTALLED_COUNT_PROPERTY_ID = "ServiceComponentInfo/installed_count";
 
+  private static final String TRUE = "true";
+
   //Parameters from the predicate
-  private static final String QUERY_PARAMETERS_RUN_SMOKE_TEST_ID =
-      "params/run_smoke_test";
+  private static final String QUERY_PARAMETERS_RUN_SMOKE_TEST_ID = "params/run_smoke_test";
 
   private static Set<String> pkPropertyIds =
-      new HashSet<String>(Arrays.asList(new String[]{
+      new HashSet<>(Arrays.asList(new String[]{
           COMPONENT_CLUSTER_NAME_PROPERTY_ID,
           COMPONENT_SERVICE_NAME_PROPERTY_ID,
           COMPONENT_COMPONENT_NAME_PROPERTY_ID}));
@@ -99,7 +101,7 @@ public class ComponentResourceProvider extends AbstractControllerResourceProvide
   // ----- Constructors ----------------------------------------------------
 
   /**
-   * Create a  new resource provider for the given management controller.
+   * Create a new resource provider for the given management controller.
    *
    * @param propertyIds           the property ids
    * @param keyPropertyIds        the key property ids
@@ -113,7 +115,6 @@ public class ComponentResourceProvider extends AbstractControllerResourceProvide
     super(propertyIds, keyPropertyIds, managementController);
     this.maintenanceStateHelper = maintenanceStateHelper;
 
-
     setRequiredCreateAuthorizations(EnumSet.of(RoleAuthorization.SERVICE_ADD_DELETE_SERVICES));
     setRequiredDeleteAuthorizations(EnumSet.of(RoleAuthorization.SERVICE_ADD_DELETE_SERVICES));
     setRequiredGetAuthorizations(RoleAuthorization.AUTHORIZATIONS_VIEW_SERVICE);
@@ -126,12 +127,9 @@ public class ComponentResourceProvider extends AbstractControllerResourceProvide
 
   @Override
   protected RequestStatus createResourcesAuthorized(Request request)
-      throws SystemException,
-             UnsupportedPropertyException,
-             ResourceAlreadyExistsException,
-             NoSuchParentResourceException {
+      throws SystemException, UnsupportedPropertyException, ResourceAlreadyExistsException, NoSuchParentResourceException {
 
-    final Set<ServiceComponentRequest> requests = new HashSet<ServiceComponentRequest>();
+    final Set<ServiceComponentRequest> requests = new HashSet<>();
     for (Map<String, Object> propertyMap : request.getProperties()) {
       requests.add(getRequest(propertyMap));
     }
@@ -154,7 +152,7 @@ public class ComponentResourceProvider extends AbstractControllerResourceProvide
   public Set<Resource> getResources(Request request, Predicate predicate)
       throws SystemException, UnsupportedPropertyException, NoSuchResourceException, NoSuchParentResourceException {
 
-    final Set<ServiceComponentRequest> requests = new HashSet<ServiceComponentRequest>();
+    final Set<ServiceComponentRequest> requests = new HashSet<>();
 
     for (Map<String, Object> propertyMap : getPropertyMaps(predicate)) {
       requests.add(getRequest(propertyMap));
@@ -167,16 +165,15 @@ public class ComponentResourceProvider extends AbstractControllerResourceProvide
       }
     });
 
-    Set<String>   requestedIds = getRequestPropertyIds(request, predicate);
-    Set<Resource> resources    = new HashSet<Resource>();
+    Set<String> requestedIds = getRequestPropertyIds(request, predicate);
+    Set<Resource> resources = new HashSet<>();
 
     for (ServiceComponentResponse response : responses) {
       Resource resource = new ResourceImpl(Resource.Type.Component);
       setResourceProperty(resource, COMPONENT_CLUSTER_NAME_PROPERTY_ID, response.getClusterName(), requestedIds);
       setResourceProperty(resource, COMPONENT_SERVICE_NAME_PROPERTY_ID, response.getServiceName(), requestedIds);
       setResourceProperty(resource, COMPONENT_COMPONENT_NAME_PROPERTY_ID, response.getComponentName(), requestedIds);
-      setResourceProperty(resource, COMPONENT_STATE_PROPERTY_ID,
-          response.getDesiredState(), requestedIds);
+      setResourceProperty(resource, COMPONENT_STATE_PROPERTY_ID, response.getDesiredState(), requestedIds);
       setResourceProperty(resource, COMPONENT_CATEGORY_PROPERTY_ID, response.getCategory(), requestedIds);
       setResourceProperty(resource, COMPONENT_TOTAL_COUNT_PROPERTY_ID, response.getTotalCount(), requestedIds);
       setResourceProperty(resource, COMPONENT_STARTED_COUNT_PROPERTY_ID, response.getStartedCount(), requestedIds);
@@ -191,15 +188,13 @@ public class ComponentResourceProvider extends AbstractControllerResourceProvide
   public RequestStatus updateResources(final Request request, Predicate predicate)
       throws SystemException, UnsupportedPropertyException, NoSuchResourceException, NoSuchParentResourceException {
 
-    final Set<ServiceComponentRequest> requests = new HashSet<ServiceComponentRequest>();
+    final Set<ServiceComponentRequest> requests = new HashSet<>();
     for (Map<String, Object> propertyMap : getPropertyMaps(request.getProperties().iterator().next(), predicate)) {
-      ServiceComponentRequest compRequest = getRequest(propertyMap);
-
-      requests.add(compRequest);
+      requests.add(getRequest(propertyMap));
     }
 
     Object queryParameterValue = getQueryParameterValue(QUERY_PARAMETERS_RUN_SMOKE_TEST_ID, predicate);
-    final boolean runSmokeTest = queryParameterValue != null && queryParameterValue.equals("true");
+    final boolean runSmokeTest = TRUE.equals(queryParameterValue);
 
     RequestStatusResponse response = modifyResources(new Command<RequestStatusResponse>() {
       @Override
@@ -217,10 +212,10 @@ public class ComponentResourceProvider extends AbstractControllerResourceProvide
   protected RequestStatus deleteResourcesAuthorized(Predicate predicate)
       throws SystemException, UnsupportedPropertyException, NoSuchResourceException, NoSuchParentResourceException {
 
-    final Set<ServiceComponentRequest> requests = new HashSet<ServiceComponentRequest>();
-      for (Map<String, Object> propertyMap : getPropertyMaps(predicate)) {
-        requests.add(getRequest(propertyMap));
-      }
+    final Set<ServiceComponentRequest> requests = new HashSet<>();
+    for (Map<String, Object> propertyMap : getPropertyMaps(predicate)) {
+      requests.add(getRequest(propertyMap));
+    }
     RequestStatusResponse response = modifyResources(new Command<RequestStatusResponse>() {
       @Override
       public RequestStatusResponse invoke() throws AmbariException, AuthorizationException {
@@ -260,8 +255,8 @@ public class ComponentResourceProvider extends AbstractControllerResourceProvide
   }
 
   // Create the components for the given requests.
-  public synchronized void createComponents(
-      Set<ServiceComponentRequest> requests) throws AmbariException, AuthorizationException {
+  public synchronized void createComponents(Set<ServiceComponentRequest> requests)
+      throws AmbariException, AuthorizationException {
 
     if (requests.isEmpty()) {
       LOG.warn("Received an empty requests set");
@@ -273,106 +268,45 @@ public class ComponentResourceProvider extends AbstractControllerResourceProvide
     ServiceComponentFactory serviceComponentFactory = getManagementController().getServiceComponentFactory();
 
     // do all validation checks
-    Map<String, Map<String, Set<String>>> componentNames =
-        new HashMap<String, Map<String, Set<String>>>();
-    Set<String> duplicates = new HashSet<String>();
+    Map<String, Map<String, Set<String>>> componentNames = new HashMap<>();
+    Set<String> duplicates = new HashSet<>();
 
     for (ServiceComponentRequest request : requests) {
-      if (request.getClusterName() == null
-          || request.getClusterName().isEmpty()
-          || request.getComponentName() == null
-          || request.getComponentName().isEmpty()) {
-        throw new IllegalArgumentException("Invalid arguments"
-            + ", clustername and componentname should be"
-            + " non-null and non-empty when trying to create a"
-            + " component");
-      }
-
-      Cluster cluster;
-      try {
-        cluster = clusters.getCluster(request.getClusterName());
-      } catch (ClusterNotFoundException e) {
-        throw new ParentObjectNotFoundException(
-            "Attempted to add a component to a cluster which doesn't exist:", e);
-      }
-
-      if(!AuthorizationHelper.isAuthorized(ResourceType.CLUSTER, cluster.getResourceId(), RoleAuthorization.SERVICE_ADD_DELETE_SERVICES)) {
-        throw new AuthorizationException("The user is not authorized to create components");
-      }
-
-      if (request.getServiceName() == null
-          || request.getServiceName().isEmpty()) {
-        StackId stackId = cluster.getDesiredStackVersion();
-        String serviceName =
-            ambariMetaInfo.getComponentToService(stackId.getStackName(),
-                stackId.getStackVersion(), request.getComponentName());
-        if (LOG.isDebugEnabled()) {
-          LOG.debug("Looking up service name for component"
-              + ", componentName=" + request.getComponentName()
-              + ", serviceName=" + serviceName);
-        }
+      Validate.notEmpty(request.getComponentName(), "component name should be non-empty");
+      Cluster cluster = getClusterForRequest(request, clusters);
 
-        if (serviceName == null
-            || serviceName.isEmpty()) {
-          throw new AmbariException("Could not find service for component"
-              + ", componentName=" + request.getComponentName()
-              + ", clusterName=" + cluster.getClusterName()
-              + ", stackInfo=" + stackId.getStackId());
-        }
-        request.setServiceName(serviceName);
-      }
+      isAuthorized(cluster, RoleAuthorization.SERVICE_ADD_DELETE_SERVICES);
 
-      if (LOG.isDebugEnabled()) {
-        LOG.debug("Received a createComponent request"
-            + ", clusterName=" + request.getClusterName()
-            + ", serviceName=" + request.getServiceName()
-            + ", componentName=" + request.getComponentName()
-            + ", request=" + request);
-      }
+      setServiceNameIfAbsent(request, cluster, ambariMetaInfo);
+      debug("Received a createComponent request: {}", request);
 
       if (!componentNames.containsKey(request.getClusterName())) {
-        componentNames.put(request.getClusterName(),
-            new HashMap<String, Set<String>>());
+        componentNames.put(request.getClusterName(), new HashMap<String, Set<String>>());
       }
-      if (!componentNames.get(request.getClusterName())
-          .containsKey(request.getServiceName())) {
-        componentNames.get(request.getClusterName()).put(
-            request.getServiceName(), new HashSet<String>());
+      Map<String, Set<String>> serviceComponents = componentNames.get(request.getClusterName());
+      if (!serviceComponents.containsKey(request.getServiceName())) {
+        serviceComponents.put(request.getServiceName(), new HashSet<String>());
       }
-      if (componentNames.get(request.getClusterName())
-          .get(request.getServiceName()).contains(request.getComponentName())) {
+
+      if (serviceComponents.get(request.getServiceName()).contains(request.getComponentName())) {
         // throw error later for dup
-        duplicates.add("[clusterName=" + request.getClusterName() + ", serviceName=" + request.getServiceName() +
-            ", componentName=" + request.getComponentName() + "]");
+        duplicates.add(request.toString());
         continue;
       }
-      componentNames.get(request.getClusterName())
-          .get(request.getServiceName()).add(request.getComponentName());
+      serviceComponents.get(request.getServiceName()).add(request.getComponentName());
 
-      if (request.getDesiredState() != null
-          && !request.getDesiredState().isEmpty()) {
-        State state = State.valueOf(request.getDesiredState());
-        if (!state.isValidDesiredState()
-            || state != State.INIT) {
-          throw new IllegalArgumentException("Invalid desired state"
-              + " only INIT state allowed during creation"
-              + ", providedDesiredState=" + request.getDesiredState());
-        }
+      if (StringUtils.isNotEmpty(request.getDesiredState())) {
+        Validate.isTrue(State.INIT == State.valueOf(request.getDesiredState()),
+            "Invalid desired state only INIT state allowed during creation, providedDesiredState=" + request.getDesiredState());
       }
 
-      Service s;
-      try {
-        s = cluster.getService(request.getServiceName());
-      } catch (ServiceNotFoundException e) {
-        throw new ParentObjectNotFoundException(
-            "Attempted to add a component to a service which doesn't exist:", e);
-      }
+      Service s = getServiceFromCluster(request, cluster);
+
       try {
         ServiceComponent sc = s.getServiceComponent(request.getComponentName());
         if (sc != null) {
           // throw error later for dup
-          duplicates.add("[clusterName=" + request.getClusterName() + ", serviceName=" + request.getServiceName() +
-              ", componentName=" + request.getComponentName() + "]");
+          duplicates.add(request.toString());
           continue;
         }
       } catch (AmbariException e) {
@@ -383,50 +317,30 @@ public class ComponentResourceProvider extends AbstractControllerResourceProvide
       if (!ambariMetaInfo.isValidServiceComponent(stackId.getStackName(),
           stackId.getStackVersion(), s.getName(), request.getComponentName())) {
         throw new IllegalArgumentException("Unsupported or invalid component"
-            + " in stack"
-            + ", clusterName=" + request.getClusterName()
-            + ", serviceName=" + request.getServiceName()
-            + ", componentName=" + request.getComponentName()
-            + ", stackInfo=" + stackId.getStackId());
+            + " in stack stackInfo=" + stackId.getStackId()
+            + " request=" + request.toString());
       }
     }
 
     // ensure only a single cluster update
-    if (componentNames.size() != 1) {
-      throw new IllegalArgumentException("Invalid arguments, updates allowed"
-          + "on only one cluster at a time");
-    }
+    Validate.isTrue(componentNames.size() == 1,
+        "Invalid arguments, updates allowed on only one cluster at a time");
 
     // Validate dups
     if (!duplicates.isEmpty()) {
-      StringBuilder names = new StringBuilder();
-      boolean first = true;
-      for (String cName : duplicates) {
-        if (!first) {
-          names.append(",");
-        }
-        first = false;
-        names.append(cName);
-      }
-      String msg;
-      if (duplicates.size() == 1) {
-        msg = "Attempted to create a component which already exists: ";
-      } else {
-        msg = "Attempted to create components which already exist: ";
-      }
-      throw new DuplicateResourceException(msg + names.toString());
+      //Java8 has StringJoiner library but ambari is not on Java8 yet.
+      throw new DuplicateResourceException("Attempted to create one or more components which already exist:"
+                            + StringUtils.join(duplicates, ","));
     }
 
     // now doing actual work
     for (ServiceComponentRequest request : requests) {
       Cluster cluster = clusters.getCluster(request.getClusterName());
       Service s = cluster.getService(request.getServiceName());
-      ServiceComponent sc = serviceComponentFactory.createNew(s,
-          request.getComponentName());
+      ServiceComponent sc = serviceComponentFactory.createNew(s, request.getComponentName());
       sc.setDesiredStackVersion(s.getDesiredStackVersion());
 
-      if (request.getDesiredState() != null
-          && !request.getDesiredState().isEmpty()) {
+      if (StringUtils.isNotEmpty(request.getDesiredState())) {
         State state = State.valueOf(request.getDesiredState());
         sc.setDesiredState(state);
       } else {
@@ -439,10 +353,8 @@ public class ComponentResourceProvider extends AbstractControllerResourceProvide
   }
 
   // Get the components for the given requests.
-  protected Set<ServiceComponentResponse> getComponents(
-      Set<ServiceComponentRequest> requests) throws AmbariException {
-
-    Set<ServiceComponentResponse> response = new HashSet<ServiceComponentResponse>();
+  protected Set<ServiceComponentResponse> getComponents(Set<ServiceComponentRequest> requests) throws AmbariException {
+    Set<ServiceComponentResponse> response = new HashSet<>();
     for (ServiceComponentRequest request : requests) {
       try {
         response.addAll(getComponents(request));
@@ -458,57 +370,21 @@ public class ComponentResourceProvider extends AbstractControllerResourceProvide
   }
 
   // Get the components for the given request.
-  private Set<ServiceComponentResponse> getComponents(
-      ServiceComponentRequest request) throws AmbariException {
-    if (request.getClusterName() == null
-        || request.getClusterName().isEmpty()) {
-      throw new IllegalArgumentException("Invalid arguments, cluster name"
-          + " should be non-null");
-    }
+  private Set<ServiceComponentResponse> getComponents(ServiceComponentRequest request) throws AmbariException {
 
-    Clusters clusters = getManagementController().getClusters();
-    AmbariMetaInfo ambariMetaInfo = getManagementController().getAmbariMetaInfo();
+    final AmbariMetaInfo ambariMetaInfo = getManagementController().getAmbariMetaInfo();
+    final Clusters clusters = getManagementController().getClusters();
+    final Cluster cluster = getCluster(request, clusters);
 
-    final Cluster cluster;
-    try {
-      cluster = clusters.getCluster(request.getClusterName());
-    } catch (ObjectNotFoundException e) {
-      throw new ParentObjectNotFoundException("Parent Cluster resource doesn't exist", e);
-    }
-
-    Set<ServiceComponentResponse> response =
-        new HashSet<ServiceComponentResponse>();
+    Set<ServiceComponentResponse> response = new HashSet<>();
     String category = null;
 
     StackId stackId = cluster.getDesiredStackVersion();
 
     if (request.getComponentName() != null) {
-      if (request.getServiceName() == null) {
-        String serviceName =
-            ambariMetaInfo.getComponentToService(stackId.getStackName(),
-                stackId.getStackVersion(), request.getComponentName());
-        if (LOG.isDebugEnabled()) {
-          LOG.debug("Looking up service name for component"
-              + ", componentName=" + request.getComponentName()
-              + ", serviceName=" + serviceName);
-        }
-        if (serviceName == null
-            || serviceName.isEmpty()) {
-          throw new ObjectNotFoundException("Could not find service for component"
-              + ", componentName=" + request.getComponentName()
-              + ", clusterName=" + cluster.getClusterName()
-              + ", stackInfo=" + stackId.getStackId());
-        }
-        request.setServiceName(serviceName);
-      }
-
-      final Service s;
-      try {
-        s = cluster.getService(request.getServiceName());
-      } catch (ObjectNotFoundException e) {
-        throw new ParentObjectNotFoundException("Parent Service resource doesn't exist", e);
-      }
+      setServiceNameIfAbsent(request, cluster, ambariMetaInfo);
 
+      final Service s = getServiceFromCluster(request, cluster);
       ServiceComponent sc = s.getServiceComponent(request.getComponentName());
       ServiceComponentResponse serviceComponentResponse = sc.convertToResponse();
 
@@ -527,35 +403,18 @@ public class ComponentResourceProvider extends AbstractControllerResourceProvide
       return response;
     }
 
-    boolean checkDesiredState = false;
-    State desiredStateToCheck = null;
-    if (request.getDesiredState() != null
-        && !request.getDesiredState().isEmpty()) {
-      desiredStateToCheck = State.valueOf(request.getDesiredState());
-      if (!desiredStateToCheck.isValidDesiredState()) {
-        throw new IllegalArgumentException("Invalid arguments, invalid desired"
-            + " state, desiredState=" + desiredStateToCheck);
-      }
-      checkDesiredState = true;
-    }
-
-    Set<Service> services = new HashSet<Service>();
-    if (request.getServiceName() != null
-        && !request.getServiceName().isEmpty()) {
-      try {
-        services.add(cluster.getService(request.getServiceName()));
-      } catch (ObjectNotFoundException e) {
-        throw new ParentObjectNotFoundException("Could not find service", e);
-      }
+    Set<Service> services = new HashSet<>();
+    if (StringUtils.isNotEmpty(request.getServiceName())) {
+      services.add(getServiceFromCluster(request, cluster));
     } else {
       services.addAll(cluster.getServices().values());
     }
 
+    final State desiredStateToCheck = getValidDesiredState(request);
     for (Service s : services) {
       // filter on request.getDesiredState()
       for (ServiceComponent sc : s.getServiceComponents().values()) {
-        if (checkDesiredState
-            && (desiredStateToCheck != sc.getDesiredState())) {
+        if (desiredStateToCheck != null && desiredStateToCheck != sc.getDesiredState()) {
           // skip non matching state
           continue;
         }
@@ -572,8 +431,7 @@ public class ComponentResourceProvider extends AbstractControllerResourceProvide
           // component doesn't exist, nothing to do
         }
         String requestedCategory = request.getComponentCategory();
-        if (requestedCategory != null && !requestedCategory.isEmpty() &&
-            category != null && !requestedCategory.equalsIgnoreCase(category)) {
+        if (StringUtils.isNotEmpty(requestedCategory) && !requestedCategory.equalsIgnoreCase(category)) {
           continue;
         }
 
@@ -585,7 +443,8 @@ public class ComponentResourceProvider extends AbstractControllerResourceProvide
 
   // Update the components for the given requests.
   protected synchronized RequestStatusResponse updateComponents(Set<ServiceComponentRequest> requests,
-                                                             Map<String, String> requestProperties, boolean runSmokeTest)
+                                                                Map<String, String> requestProperties,
+                                                                boolean runSmokeTest)
       throws AmbariException, AuthorizationException {
 
     if (requests.isEmpty()) {
@@ -593,21 +452,16 @@ public class ComponentResourceProvider extends AbstractControllerResourceProvide
       return null;
     }
 
-    AmbariManagementController controller = getManagementController();
-    Clusters clusters = controller.getClusters();
-    AmbariMetaInfo ambariMetaInfo = controller.getAmbariMetaInfo();
+    Clusters clusters = getManagementController().getClusters();
+    AmbariMetaInfo ambariMetaInfo = getManagementController().getAmbariMetaInfo();
 
-    Map<State, List<ServiceComponent>> changedComps =
-        new HashMap<State, List<ServiceComponent>>();
-    Map<String, Map<State, List<ServiceComponentHost>>> changedScHosts =
-        new HashMap<String, Map<State, List<ServiceComponentHost>>>();
-    Collection<ServiceComponentHost> ignoredScHosts =
-        new ArrayList<ServiceComponentHost>();
+    Map<State, List<ServiceComponent>> changedComps = new HashMap<>();
+    Map<String, Map<State, List<ServiceComponentHost>>> changedScHosts = new HashMap<>();
+    Collection<ServiceComponentHost> ignoredScHosts = new ArrayList<>();
 
-    Set<String> clusterNames = new HashSet<String>();
-    Map<String, Map<String, Set<String>>> componentNames =
-        new HashMap<String, Map<String,Set<String>>>();
-    Set<State> seenNewStates = new HashSet<State>();
+    Set<String> clusterNames = new HashSet<>();
+    Map<String, Map<String, Set<String>>> componentNames = new HashMap<>();
+    Set<State> seenNewStates = new HashSet<>();
 
     // Determine operation level
     Resource.Type reqOpLvl;
@@ -615,101 +469,42 @@ public class ComponentResourceProvider extends AbstractControllerResourceProvide
       RequestOperationLevel operationLevel = new RequestOperationLevel(requestProperties);
       reqOpLvl = operationLevel.getLevel();
     } else {
-      String message = "Can not determine request operation level. " +
-              "Operation level property should " +
-              "be specified for this request.";
-      LOG.warn(message);
+      LOG.warn("Can not determine request operation level. Operation level property should be specified for this request.");
       reqOpLvl = Resource.Type.Cluster;
     }
 
     for (ServiceComponentRequest request : requests) {
+      Validate.notEmpty(request.getComponentName(), "component name should be non-empty");
+      final Cluster cluster = getClusterForRequest(request, clusters);
       final String clusterName = request.getClusterName();
-      String componentName = request.getComponentName();
-      if (clusterName == null
-          || clusterName.isEmpty()
-          || componentName == null
-          || componentName.isEmpty()) {
-        throw new IllegalArgumentException("Invalid arguments, cluster name"
-            + ", service name and component name should be provided to"
-            + " update components");
-      }
+      final String componentName = request.getComponentName();
 
-      String serviceName = request.getServiceName();
-      LOG.info("Received a updateComponent request"
-          + ", clusterName=" + clusterName
-          + ", serviceName=" + serviceName
-          + ", componentName=" + componentName
-          + ", request=" + request.toString());
-
-      Cluster cluster = clusters.getCluster(clusterName);
-
-      if (serviceName == null
-          || serviceName.isEmpty()) {
-        StackId stackId = cluster.getDesiredStackVersion();
-        String alternativeServiceName =
-            ambariMetaInfo.getComponentToService(stackId.getStackName(),
-                stackId.getStackVersion(), componentName);
-        if (LOG.isDebugEnabled()) {
-          LOG.debug("Looking up service name for component"
-              + ", componentName=" + componentName
-              + ", serviceName=" + alternativeServiceName);
-        }
+      LOG.info("Received a updateComponent request: {}", request);
 
-        if (alternativeServiceName == null
-            || alternativeServiceName.isEmpty()) {
-          throw new AmbariException("Could not find service for component"
-              + ", componentName=" + componentName
-              + ", clusterName=" + cluster.getClusterName()
-              + ", stackInfo=" + stackId.getStackId());
-        }
-        request.setServiceName(alternativeServiceName);
-        serviceName = alternativeServiceName;
-      }
+      setServiceNameIfAbsent(request, cluster, ambariMetaInfo);
+      final String serviceName = request.getServiceName();
 
-      if (LOG.isDebugEnabled()) {
-        LOG.debug("Received a updateComponent request"
-                + ", clusterName=" + clusterName
-                + ", serviceName=" + serviceName
-                + ", componentName=" + componentName
-                + ", request=" + request);
-      }
+      debug("Received a updateComponent request: {}", request);
 
       clusterNames.add(clusterName);
 
-      if (clusterNames.size() > 1) {
-        // FIXME throw correct error
-        throw new IllegalArgumentException("Updates to multiple clusters is not"
-            + " supported");
-      }
+      Validate.isTrue(clusterNames.size() == 1, "Updates to multiple clusters is not supported");
 
       if (!componentNames.containsKey(clusterName)) {
-        componentNames.put(clusterName,
-            new HashMap<String, Set<String>>());
+        componentNames.put(clusterName, new HashMap<String, Set<String>>());
       }
-      if (!componentNames.get(clusterName)
-          .containsKey(serviceName)) {
-        componentNames.get(clusterName).put(
-                serviceName, new HashSet<String>());
+      if (!componentNames.get(clusterName).containsKey(serviceName)) {
+        componentNames.get(clusterName).put(serviceName, new HashSet<String>());
       }
-      if (componentNames.get(clusterName)
-          .get(serviceName).contains(componentName)){
+      if (componentNames.get(clusterName).get(serviceName).contains(componentName)){
         // throw error later for dup
-        throw new IllegalArgumentException("Invalid request contains duplicate"
-            + " service components");
+        throw new IllegalArgumentException("Invalid request contains duplicate service components");
       }
-      componentNames.get(clusterName)
-          .get(serviceName).add(componentName);
+      componentNames.get(clusterName).get(serviceName).add(componentName);
 
       Service s = cluster.getService(serviceName);
       ServiceComponent sc = s.getServiceComponent(componentName);
-      State newState = null;
-      if (request.getDesiredState() != null) {
-        newState = State.valueOf(request.getDesiredState());
-        if (!newState.isValidDesiredState()) {
-          throw new IllegalArgumentException("Invalid arguments, invalid"
-              + " desired state, desiredState=" + newState.toString());
-        }
-      }
+      State newState = getValidDesiredState(request);
 
       if (! maintenanceStateHelper.isOperationAllowed(reqOpLvl, s)) {
         LOG.info("Operations cannot be applied to component " + componentName
@@ -719,20 +514,12 @@ public class ComponentResourceProvider extends AbstractControllerResourceProvide
       }
 
       if (newState == null) {
-        if (LOG.isDebugEnabled()) {
-          LOG.debug("Nothing to do for new updateServiceComponent request"
-              + ", clusterName=" + clusterName
-              + ", serviceName=" + serviceName
-              + ", componentName=" + componentName
-              + ", newDesiredState=null");
-        }
+        debug("Nothing to do for new updateServiceComponent request, request ={}, newDesiredState=null" + request);
         continue;
       }
 
-      if (sc.isClientComponent() &&
-          !newState.isValidClientComponentState()) {
-        throw new AmbariException("Invalid desired state for a client"
-            + " component");
+      if (sc.isClientComponent() && !newState.isValidClientComponentState()) {
+        throw new AmbariException("Invalid desired state for a client component");
       }
 
       seenNewStates.add(newState);
@@ -740,9 +527,8 @@ public class ComponentResourceProvider extends AbstractControllerResourceProvide
       State oldScState = sc.getDesiredState();
       if (newState != oldScState) {
         // The if user is trying to start or stop the component, ensure authorization
-        if (((newState == State.INSTALLED) || (newState == State.STARTED)) &&
-            !AuthorizationHelper.isAuthorized(ResourceType.CLUSTER, cluster.getResourceId(), RoleAuthorization.SERVICE_START_STOP)) {
-          throw new AuthorizationException("The authenticated user is not authorized to start or stop components of services");
+        if (((newState == State.INSTALLED) || (newState == State.STARTED))) {
+          isAuthorized(cluster, RoleAuthorization.SERVICE_START_STOP);
         }
 
         if (!State.isValidDesiredStateTransition(oldScState, newState)) {
@@ -759,56 +545,50 @@ public class ComponentResourceProvider extends AbstractControllerResourceProvide
         if (!changedComps.containsKey(newState)) {
           changedComps.put(newState, new ArrayList<ServiceComponent>());
         }
-        if (LOG.isDebugEnabled()) {
-          LOG.debug("Handling update to ServiceComponent"
+        debug("Handling update to ServiceComponent"
               + ", clusterName=" + clusterName
               + ", serviceName=" + serviceName
               + ", componentName=" + sc.getName()
               + ", currentDesiredState=" + oldScState
               + ", newDesiredState=" + newState);
-        }
+
         changedComps.get(newState).add(sc);
       }
 
       for (ServiceComponentHost sch : sc.getServiceComponentHosts().values()) {
         State oldSchState = sch.getState();
         if (oldSchState == State.DISABLED || oldSchState == State.UNKNOWN) {
-          if (LOG.isDebugEnabled()) {
-            LOG.debug("Ignoring ServiceComponentHost"
+          debug("Ignoring ServiceComponentHost"
                 + ", clusterName=" + clusterName
                 + ", serviceName=" + serviceName
                 + ", componentName=" + sc.getName()
                 + ", hostname=" + sch.getHostName()
                 + ", currentState=" + oldSchState
                 + ", newDesiredState=" + newState);
-          }
           continue;
         }
 
         if (newState == oldSchState) {
           ignoredScHosts.add(sch);
-          if (LOG.isDebugEnabled()) {
-            LOG.debug("Ignoring ServiceComponentHost"
+          debug("Ignoring ServiceComponentHost"
                 + ", clusterName=" + clusterName
                 + ", serviceName=" + serviceName
                 + ", componentName=" + sc.getName()
                 + ", hostname=" + sch.getHostName()
                 + ", currentState=" + oldSchState
                 + ", newDesiredState=" + newState);
-          }
           continue;
         }
 
         // do not update or alter any HC that is not active
         if (! maintenanceStateHelper.isOperationAllowed(reqOpLvl, sch)) {
           ignoredScHosts.add(sch);
-          if (LOG.isDebugEnabled()) {
-            LOG.debug("Ignoring ServiceComponentHost in maintenance state"
+          debug("Ignoring ServiceComponentHost in maintenance state"
                 + ", clusterName=" + clusterName
                 + ", serviceName=" + serviceName
                 + ", componentName=" + sc.getName()
                 + ", hostname=" + sch.getHostName());
-          }
+
           continue;
         }
 
@@ -825,31 +605,26 @@ public class ComponentResourceProvider extends AbstractControllerResourceProvide
               + ", newDesiredState=" + newState);
         }
         if (!changedScHosts.containsKey(sc.getName())) {
-          changedScHosts.put(sc.getName(),
-              new HashMap<State, List<ServiceComponentHost>>());
+          changedScHosts.put(sc.getName(), new HashMap<State, List<ServiceComponentHost>>());
         }
         if (!changedScHosts.get(sc.getName()).containsKey(newState)) {
-          changedScHosts.get(sc.getName()).put(newState,
-              new ArrayList<ServiceComponentHost>());
+          changedScHosts.get(sc.getName()).put(newState, new ArrayList<ServiceComponentHost>());
         }
-        if (LOG.isDebugEnabled()) {
-          LOG.debug("Handling update to ServiceComponentHost"
+
+        debug("Handling update to ServiceComponentHost"
               + ", clusterName=" + clusterName
               + ", serviceName=" + serviceName
               + ", componentName=" + sc.getName()
               + ", hostname=" + sch.getHostName()
               + ", currentState=" + oldSchState
               + ", newDesiredState=" + newState);
-        }
+
         changedScHosts.get(sc.getName()).get(newState).add(sch);
       }
     }
 
-    if (seenNewStates.size() > 1) {
-      // FIXME should we handle this scenario
-      throw new IllegalArgumentException("Cannot handle different desired"
-          + " state changes for a set of service components at the same time");
-    }
+    Validate.isTrue(seenNewStates.size() <= 1,
+        "Cannot handle different desired state changes for a set of service components at the same time");
 
     // TODO additional validation?
 
@@ -860,87 +635,112 @@ public class ComponentResourceProvider extends AbstractControllerResourceProvide
   }
 
   protected RequestStatusResponse deleteComponents(Set<ServiceComponentRequest> requests) throws AmbariException, AuthorizationException {
-    AmbariManagementController controller = getManagementController();
-    Clusters clusters = controller.getClusters();
-    AmbariMetaInfo ambariMetaInfo = controller.getAmbariMetaInfo();
+    Clusters clusters = getManagementController().getClusters();
+    AmbariMetaInfo ambariMetaInfo = getManagementController().getAmbariMetaInfo();
 
     for (ServiceComponentRequest request : requests) {
+      Validate.notEmpty(request.getComponentName(), "component name should be non-empty");
+      Cluster cluster = getClusterForRequest(request, clusters);
 
-      if (request.getClusterName() == null || request.getClusterName().isEmpty()
-          || request.getComponentName() == null || request.getComponentName().isEmpty()) {
-          throw new IllegalArgumentException("Invalid arguments"
-          + ", clustername and componentname should be"
-          + " non-null and non-empty when trying to create a"
-          + " component");
-      }
+      setServiceNameIfAbsent(request, cluster, ambariMetaInfo);
 
-      Cluster cluster;
-      try {
-        cluster = clusters.getCluster(request.getClusterName());
-      } catch (ClusterNotFoundException e) {
-        throw new ParentObjectNotFoundException(
-              "Attempted to add a component to a cluster which doesn't exist:", e);
+      Service s = getServiceFromCluster(request, cluster);
+
+      ServiceComponent sc = s.getServiceComponent(request.getComponentName());
+
+      if (sc != null) {
+        deleteHostComponentsForServiceComponent(sc, request);
+        sc.setDesiredState(State.DISABLED);
+        s.deleteServiceComponent(request.getComponentName());
       }
+    }
+    return null;
+  }
+
+  private void deleteHostComponentsForServiceComponent(ServiceComponent sc, ServiceComponentRequest request) throws AmbariException {
+    for (ServiceComponentHost sch : sc.getServiceComponentHosts().values()) {
+      if (!sch.getDesiredState().isRemovableState()) {
+        throw new AmbariException("Found non removable host component when trying to delete service component." +
+            " To remove host component, it must be in DISABLED/INIT/INSTALLED/INSTALL_FAILED/UNKNOWN" +
+            "/UNINSTALLED/INSTALLING state."
+            + ", request=" + request.toString()
+            + ", current state=" + sc.getDesiredState() + ".");
 
-      if(!AuthorizationHelper.isAuthorized(ResourceType.CLUSTER, cluster.getResourceId(), RoleAuthorization.SERVICE_ADD_DELETE_SERVICES)) {
-        throw new AuthorizationException("The user is not authorized to delete components");
       }
+    }
 
-      if (request.getServiceName() == null || request.getServiceName().isEmpty()) {
-        StackId stackId = cluster.getDesiredStackVersion();
-        String serviceName = ambariMetaInfo.getComponentToService(stackId.getStackName(),
-                                                                stackId.getStackVersion(), request.getComponentName());
-        if (LOG.isDebugEnabled()) {
-          LOG.debug("Looking up service name for component"
-                    + ", componentName=" + request.getComponentName()
-                    + ", serviceName=" + serviceName);
-        }
+    for (ServiceComponentHost sch : sc.getServiceComponentHosts().values()) {
+      sch.delete();
+    }
+  }
+  private Cluster getClusterForRequest(final ServiceComponentRequest request, final Clusters clusters) throws AmbariException {
+    Validate.notEmpty(request.getClusterName(), "cluster name should be non-empty");
+    try {
+      return clusters.getCluster(request.getClusterName());
+    } catch (ClusterNotFoundException e) {
+      throw new ParentObjectNotFoundException("Attempted to add a component to a cluster which doesn't exist:", e);
+    }
+  }
 
-        if (serviceName == null || serviceName.isEmpty()) {
-            throw new AmbariException("Could not find service for component"
-                         + ", componentName=" + request.getComponentName()
-                         + ", clusterName=" + cluster.getClusterName()
-                         + ", stackInfo=" + stackId.getStackId());
-        }
-        request.setServiceName(serviceName);
-        }
+  private Service getServiceFromCluster(final ServiceComponentRequest request, final Cluster cluster) throws AmbariException {
+    try {
+      return cluster.getService(request.getServiceName());
+    } catch (ServiceNotFoundException e) {
+      throw new ParentObjectNotFoundException("Parent Service resource doesn't exist.", e);
+    }
+  }
 
-      Service s;
-      try {
-        s = cluster.getService(request.getServiceName());
-      } catch (ServiceNotFoundException e) {
-        throw new ParentObjectNotFoundException(
-                    "Attempted to add a component to a service which doesn't exist:", e);
-      }
+  private Cluster getCluster(final ServiceComponentRequest request, final Clusters clusters) throws AmbariException {
+    Validate.notEmpty(request.getClusterName(), "cluster name should be non-empty");
 
-      ServiceComponent sc = s.getServiceComponent(request.getComponentName());
-      if (sc != null) {
-        if (!sc.getDesiredState().isRemovableState()) {
-          throw new AmbariException("Could not delete service component from cluster. To remove service component," +
-                 " it must be in DISABLED/INIT/INSTALLED/INSTALL_FAILED/UNKNOWN/UNINSTALLED/INSTALLING state."
-                 + ", clusterName=" + request.getClusterName()
-                 + ", serviceName=" + request.getServiceName()
-                 + ", componentName=" + request.getComponentName()
-                 + ", current state=" + sc.getDesiredState() + ".");
-          }
+    try {
+      return clusters.getCluster(request.getClusterName());
+    } catch (ObjectNotFoundException e) {
+      throw new ParentObjectNotFoundException("Parent Cluster resource doesn't exist", e);
+    }
 
-        for (ServiceComponentHost sch : sc.getServiceComponentHosts().values()) {
-          if (!sch.getDesiredState().isRemovableState()) {
-            throw new AmbariException("Found non removable host component when trying to delete service component." +
-                      " To remove host component, it must be in DISABLED/INIT/INSTALLED/INSTALL_FAILED/UNKNOWN" +
-                      "/UNINSTALLED/INSTALLING state."
-                      + ", clusterName=" + request.getClusterName()
-                      + ", serviceName=" + request.getServiceName()
-                      + ", componentName=" + request.getComponentName()
-                      + ", current state=" + sc.getDesiredState() + ".");
+  }
 
-          }
-        }
+  private void isAuthorized(final Cluster cluster, final RoleAuthorization roleAuthorization) throws AuthorizationException {
+    if (!AuthorizationHelper.isAuthorized(ResourceType.CLUSTER, cluster.getResourceId(), roleAuthorization)) {
+      throw new AuthorizationException("The user is not authorized to for role " + roleAuthorization.name());
+    }
+  }
 
-        s.deleteServiceComponent(request.getComponentName());
+  private void setServiceNameIfAbsent(final ServiceComponentRequest request,
+                                      final Cluster cluster,
+                                      final AmbariMetaInfo ambariMetaInfo) throws AmbariException {
+    if (StringUtils.isEmpty(request.getServiceName())) {
+      StackId stackId = cluster.getDesiredStackVersion();
+      String componentName = request.getComponentName();
+      String serviceName = ambariMetaInfo.getComponentToService(stackId.getStackName(),
+              stackId.getStackVersion(), componentName);
+      debug("Looking up service name for component, componentName={}, serviceName={}", componentName, serviceName);
+
+      if (StringUtils.isEmpty(serviceName)) {
+        throw new AmbariException("Could not find service for component"
+                + ", componentName=" + request.getComponentName()
+                + ", clusterName=" + cluster.getClusterName()
+                + ", stackInfo=" + stackId.getStackId());
       }
+      request.setServiceName(serviceName);
     }
-    return null;
   }
 
-}
+  private State getValidDesiredState(ServiceComponentRequest request) {
+
+    if (StringUtils.isEmpty(request.getDesiredState())) {
+      return null;
+    }
+    final State desiredStateToCheck = State.valueOf(request.getDesiredState());
+    Validate.isTrue(desiredStateToCheck.isValidDesiredState(),
+          "Invalid arguments, invalid desired state, desiredState=" + desiredStateToCheck);
+    return desiredStateToCheck;
+  }
+
+  private void debug(String format, Object... arguments) {
+    if (LOG.isDebugEnabled()) {
+      LOG.debug(format, arguments);
+    }
+  }
+}
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/ambari/blob/b328f6db/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ComponentResourceProviderTest.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ComponentResourceProviderTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ComponentResourceProviderTest.java
index 6ec27ad..f38fab1 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ComponentResourceProviderTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ComponentResourceProviderTest.java
@@ -58,6 +58,7 @@ import org.apache.ambari.server.controller.spi.Predicate;
 import org.apache.ambari.server.controller.spi.Request;
 import org.apache.ambari.server.controller.spi.Resource;
 import org.apache.ambari.server.controller.spi.ResourceProvider;
+import org.apache.ambari.server.controller.spi.SystemException;
 import org.apache.ambari.server.controller.utilities.PredicateBuilder;
 import org.apache.ambari.server.controller.utilities.PropertyHelper;
 import org.apache.ambari.server.security.TestAuthenticationFactory;
@@ -88,6 +89,10 @@ import org.springframework.security.core.context.SecurityContextHolder;
  */
 public class ComponentResourceProviderTest {
 
+  private static final long CLUSTER_ID = 100;
+  private static final String CLUSTER_NAME = "Cluster100";
+  private static final String SERVICE_NAME = "Service100";
+
   @Before
   public void clearAuthentication() {
     SecurityContextHolder.getContext().setAuthentication(null);
@@ -426,20 +431,26 @@ public class ComponentResourceProviderTest {
 
   @Test
   public void testSuccessDeleteResourcesAsAdministrator() throws Exception {
-    testSuccessDeleteResources(TestAuthenticationFactory.createAdministrator());
+    testSuccessDeleteResources(TestAuthenticationFactory.createAdministrator(), State.INSTALLED);
   }
 
   @Test
   public void testSuccessDeleteResourcesAsClusterAdministrator() throws Exception {
-    testSuccessDeleteResources(TestAuthenticationFactory.createClusterAdministrator());
+    testSuccessDeleteResources(TestAuthenticationFactory.createClusterAdministrator(), State.INSTALLED);
   }
 
+
   @Test(expected = AuthorizationException.class)
-  public void testSuccessDeleteResourcesAsServiceAdministrator() throws Exception {
-    testSuccessDeleteResources(TestAuthenticationFactory.createServiceAdministrator());
+  public void testDeleteResourcesAsServiceAdministrator() throws Exception {
+    testSuccessDeleteResources(TestAuthenticationFactory.createServiceAdministrator(), State.INSTALLED);
   }
 
-  private void testSuccessDeleteResources(Authentication authentication) throws Exception {
+  @Test(expected = SystemException.class)
+  public void testDeleteResourcesWithStartedHostComponentState() throws Exception {
+    testSuccessDeleteResources(TestAuthenticationFactory.createAdministrator(), State.STARTED);
+  }
+
+  private void testSuccessDeleteResources(Authentication authentication, State hostComponentState) throws Exception {
     Resource.Type type = Resource.Type.Component;
 
     AmbariManagementController managementController = createMock(AmbariManagementController.class);
@@ -453,22 +464,22 @@ public class ComponentResourceProviderTest {
     ServiceComponentHost serviceComponentHost = createNiceMock(ServiceComponentHost.class);
     StackId stackId = createNiceMock(StackId.class);
 
-    Map<String, ServiceComponentHost> serviceComponentHosts = new HashMap<String, ServiceComponentHost>();
+    Map<String, ServiceComponentHost> serviceComponentHosts = new HashMap<>();
     serviceComponentHosts.put("", serviceComponentHost);
 
     expect(managementController.getClusters()).andReturn(clusters);
     expect(managementController.getAmbariMetaInfo()).andReturn(ambariMetaInfo);
 
-    expect(clusters.getCluster("Cluster100")).andReturn(cluster);
-    expect(cluster.getService("Service100")).andReturn(service);
-    expect(cluster.getClusterId()).andReturn(2L).anyTimes();
+    expect(clusters.getCluster(CLUSTER_NAME)).andReturn(cluster);
+    expect(cluster.getService(SERVICE_NAME)).andReturn(service);
+    expect(cluster.getClusterId()).andReturn(CLUSTER_ID).anyTimes();
 
     expect(service.getServiceComponent("Component100")).andReturn(serviceComponent);
 
-    expect(serviceComponent.getDesiredState()).andReturn(State.INSTALLED);
-    expect(serviceComponent.getServiceComponentHosts()).andReturn(serviceComponentHosts);
+    expect(serviceComponent.getDesiredState()).andReturn(State.STARTED);
+    expect(serviceComponent.getServiceComponentHosts()).andReturn(serviceComponentHosts).anyTimes();
 
-    expect(serviceComponentHost.getDesiredState()).andReturn(State.INSTALLED);
+    expect(serviceComponentHost.getDesiredState()).andReturn(hostComponentState);
 
 
     service.deleteServiceComponent("Component100");
@@ -579,178 +590,6 @@ public class ComponentResourceProviderTest {
     verify(managementController);
   }
 
-  @Test
-  public void testDeleteResourcesWithServiceComponentStartedAsAdministrator() throws Exception {
-    testDeleteResourcesWithServiceComponentStarted(TestAuthenticationFactory.createAdministrator());
-  }
-
-  @Test
-  public void testDeleteResourcesWithServiceComponentStartedAsClusterAdministrator() throws Exception {
-    testDeleteResourcesWithServiceComponentStarted(TestAuthenticationFactory.createClusterAdministrator());
-  }
-
-  @Test(expected = AuthorizationException.class)
-  public void testDeleteResourcesWithServiceComponentStartedAsServiceAdministrator() throws Exception {
-    testDeleteResourcesWithServiceComponentStarted(TestAuthenticationFactory.createServiceAdministrator());
-  }
-
-  private void testDeleteResourcesWithServiceComponentStarted(Authentication authentication) throws Exception {
-    Resource.Type type = Resource.Type.Component;
-
-    AmbariManagementController managementController = createMock(AmbariManagementController.class);
-    MaintenanceStateHelper maintenanceStateHelper = createNiceMock(MaintenanceStateHelper.class);
-
-    Clusters clusters = createNiceMock(Clusters.class);
-    Cluster cluster = createNiceMock(Cluster.class);
-    Service service = createNiceMock(Service.class);
-    AmbariMetaInfo ambariMetaInfo = createNiceMock(AmbariMetaInfo.class);
-    ServiceComponent serviceComponent = createNiceMock(ServiceComponent.class);
-    ServiceComponentHost serviceComponentHost = createNiceMock(ServiceComponentHost.class);
-    StackId stackId = createNiceMock(StackId.class);
-
-    Map<String, ServiceComponentHost> serviceComponentHosts = new HashMap<String, ServiceComponentHost>();
-    serviceComponentHosts.put("", serviceComponentHost);
-
-    expect(managementController.getClusters()).andReturn(clusters);
-    expect(managementController.getAmbariMetaInfo()).andReturn(ambariMetaInfo);
-
-    expect(clusters.getCluster("Cluster100")).andReturn(cluster);
-    expect(cluster.getService("Service100")).andReturn(service);
-    expect(cluster.getClusterId()).andReturn(2L).anyTimes();
-
-    expect(service.getServiceComponent("Component100")).andReturn(serviceComponent);
-
-    expect(serviceComponent.getDesiredState()).andReturn(State.STARTED);
-    expect(serviceComponent.getServiceComponentHosts()).andReturn(serviceComponentHosts);
-
-    expect(serviceComponentHost.getDesiredState()).andReturn(State.INSTALLED);
-
-
-    // replay
-    replay(managementController, clusters, cluster, service, stackId, ambariMetaInfo,
-           serviceComponent, serviceComponentHost, maintenanceStateHelper);
-
-    SecurityContextHolder.getContext().setAuthentication(authentication);
-
-    ResourceProvider provider = new ComponentResourceProvider(
-                PropertyHelper.getPropertyIds(type),
-                PropertyHelper.getKeyPropertyIds(type),
-                managementController, maintenanceStateHelper);
-
-    AbstractResourceProviderTest.TestObserver observer = new AbstractResourceProviderTest.TestObserver();
-
-    ((ObservableResourceProvider)provider).addObserver(observer);
-
-    Predicate predicate = new PredicateBuilder()
-                .property(ComponentResourceProvider.COMPONENT_CLUSTER_NAME_PROPERTY_ID)
-                .equals("Cluster100")
-                .and()
-                .property(ComponentResourceProvider.COMPONENT_SERVICE_NAME_PROPERTY_ID)
-                .equals("Service100")
-                .and()
-                .property(ComponentResourceProvider.COMPONENT_COMPONENT_NAME_PROPERTY_ID)
-                .equals("Component100").toPredicate();
-
-    try {
-      provider.deleteResources(predicate);
-      Assert.fail("Expected exception.");
-    } catch(Exception e) {
-      if (e instanceof AuthorizationException) {
-        throw e;
-      }
-      //expected
-    }
-
-    // verify
-    verify(managementController);
-  }
-
-  @Test
-  public void testDeleteResourcesWithServiceComponentHostStartedAsAdministrator() throws Exception {
-    testDeleteResourcesWithServiceComponentHostStarted(TestAuthenticationFactory.createAdministrator());
-  }
-
-  @Test
-  public void testDeleteResourcesWithServiceComponentHostStartedAsClusterAdministrator() throws Exception {
-    testDeleteResourcesWithServiceComponentHostStarted(TestAuthenticationFactory.createClusterAdministrator());
-  }
-
-  @Test(expected = AuthorizationException.class)
-  public void testDeleteResourcesWithServiceComponentHostStartedAsServiceAdministrator() throws Exception {
-    testDeleteResourcesWithServiceComponentHostStarted(TestAuthenticationFactory.createServiceAdministrator());
-  }
-
-  private void testDeleteResourcesWithServiceComponentHostStarted(Authentication authentication) throws Exception {
-    Resource.Type type = Resource.Type.Component;
-
-    AmbariManagementController managementController = createMock(AmbariManagementController.class);
-    MaintenanceStateHelper maintenanceStateHelper = createNiceMock(MaintenanceStateHelper.class);
-
-    Clusters clusters = createNiceMock(Clusters.class);
-    Cluster cluster = createNiceMock(Cluster.class);
-    Service service = createNiceMock(Service.class);
-    AmbariMetaInfo ambariMetaInfo = createNiceMock(AmbariMetaInfo.class);
-    ServiceComponent serviceComponent = createNiceMock(ServiceComponent.class);
-    ServiceComponentHost serviceComponentHost = createNiceMock(ServiceComponentHost.class);
-    StackId stackId = createNiceMock(StackId.class);
-
-    Map<String, ServiceComponentHost> serviceComponentHosts = new HashMap<String, ServiceComponentHost>();
-    serviceComponentHosts.put("", serviceComponentHost);
-
-    expect(managementController.getClusters()).andReturn(clusters);
-    expect(managementController.getAmbariMetaInfo()).andReturn(ambariMetaInfo);
-
-    expect(clusters.getCluster("Cluster100")).andReturn(cluster);
-    expect(cluster.getService("Service100")).andReturn(service);
-    expect(cluster.getClusterId()).andReturn(2L).anyTimes();
-
-    expect(service.getServiceComponent("Component100")).andReturn(serviceComponent);
-
-    expect(serviceComponent.getDesiredState()).andReturn(State.INSTALLED);
-    expect(serviceComponent.getServiceComponentHosts()).andReturn(serviceComponentHosts);
-
-    expect(serviceComponentHost.getDesiredState()).andReturn(State.STARTED);
-
-
-    // replay
-    replay(managementController, clusters, cluster, service, stackId, ambariMetaInfo,
-           serviceComponent, serviceComponentHost, maintenanceStateHelper);
-
-    SecurityContextHolder.getContext().setAuthentication(authentication);
-
-    ResourceProvider provider = new ComponentResourceProvider(
-                PropertyHelper.getPropertyIds(type),
-                PropertyHelper.getKeyPropertyIds(type),
-                managementController,maintenanceStateHelper);
-
-    AbstractResourceProviderTest.TestObserver observer = new AbstractResourceProviderTest.TestObserver();
-
-    ((ObservableResourceProvider)provider).addObserver(observer);
-
-    Predicate predicate = new PredicateBuilder()
-                .property(ComponentResourceProvider.COMPONENT_CLUSTER_NAME_PROPERTY_ID)
-                .equals("Cluster100")
-                .and()
-                .property(ComponentResourceProvider.COMPONENT_SERVICE_NAME_PROPERTY_ID)
-                .equals("Service100")
-                .and()
-                .property(ComponentResourceProvider.COMPONENT_COMPONENT_NAME_PROPERTY_ID)
-                .equals("Component100").toPredicate();
-
-    try {
-      provider.deleteResources(predicate);
-      Assert.fail("Expected exception.");
-    } catch(Exception e) {
-      if (e instanceof AuthorizationException) {
-        throw e;
-      }
-      //expected
-    }
-
-    // verify
-    verify(managementController);
-  }
-
 
   @Test
   public void testGetComponents() throws Exception {
@@ -810,7 +649,7 @@ public class ComponentResourceProviderTest {
    * case when an OR predicate is provided in the query.
    */
   @Test
-  public void testGetComponents___OR_Predicate_ServiceComponentNotFoundException() throws Exception {
+  public void testGetComponents_OR_Predicate_ServiceComponentNotFoundException() throws Exception {
     // member state mocks
     AmbariManagementController managementController = createMock(AmbariManagementController.class);
     Clusters clusters = createNiceMock(Clusters.class);
@@ -908,7 +747,7 @@ public class ComponentResourceProviderTest {
    * Ensure that ServiceComponentNotFoundException is propagated in case where there is a single request.
    */
   @Test
-  public void testGetComponents___ServiceComponentNotFoundException() throws Exception {
+  public void testGetComponents_ServiceComponentNotFoundException() throws Exception {
     // member state mocks
     Injector injector = createStrictMock(Injector.class);
     Capture<AmbariManagementController> controllerCapture = EasyMock.newCapture();
@@ -953,7 +792,7 @@ public class ComponentResourceProviderTest {
     assertSame(controller, controllerCapture.getValue());
     verify(injector, clusters, cluster, service);
   }
-  
+
   public static void createComponents(AmbariManagementController controller, Set<ServiceComponentRequest> requests)
       throws AmbariException, AuthorizationException {
     ComponentResourceProvider provider = getComponentResourceProvider(controller);