Revert "AMBARI-7218. rename host group deletes configs (dlysnichenko)"

This reverts commit 9ce44c953de1154883887538c5979e5d9036a30c.


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

Branch: refs/heads/branch-alerts-dev
Commit: 22b1df06454e32753864ea3a622d9cd5b8d921fa
Parents: 9cf27bf
Author: Lisnichenko Dmitro <dlysniche...@hortonworks.com>
Authored: Wed Sep 10 13:13:46 2014 +0300
Committer: Lisnichenko Dmitro <dlysniche...@hortonworks.com>
Committed: Wed Sep 10 13:13:46 2014 +0300

----------------------------------------------------------------------
 .../internal/ConfigGroupResourceProvider.java   | 136 +++++++------------
 .../ConfigGroupResourceProviderTest.java        | 130 ------------------
 2 files changed, 49 insertions(+), 217 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/ambari/blob/22b1df06/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ConfigGroupResourceProvider.java
----------------------------------------------------------------------
diff --git 
a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ConfigGroupResourceProvider.java
 
b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ConfigGroupResourceProvider.java
index 52df317..6e13d9c 100644
--- 
a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ConfigGroupResourceProvider.java
+++ 
b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ConfigGroupResourceProvider.java
@@ -383,36 +383,6 @@ public class ConfigGroupResourceProvider extends
     cluster.deleteConfigGroup(request.getId());
   }
 
-  private void basicRequestValidation(ConfigGroupRequest request) {
-    if (request.getId() == null
-            || request.getClusterName() == null
-            || request.getClusterName().isEmpty()
-            || request.getGroupName() == null
-            || request.getGroupName().isEmpty()) {
-      LOG.debug("Received a config group request with request id = " +
-              request.getId() + ", cluster name = " +
-              request.getClusterName() + ", group name = " + 
request.getGroupName());
-      throw new IllegalArgumentException("Request id, " +
-              "cluster name and " +
-              "group name have to be provided.");
-    }
-  }
-
-  private void validateRenameRequest(ConfigGroupRequest request) {
-    if (request.getTag() != null
-            || (request.getHosts() != null && ! request.getHosts().isEmpty())
-            || request.getDescription() != null
-            || request.getServiceConfigVersionNote() != null
-            || (request.getConfigs()!=null && ! 
request.getConfigs().isEmpty())) {
-      throw new IllegalArgumentException("Request with id " +
-              request.getId() +
-              " seems to be a config group rename request. " +
-              "Renaming config group can not be combined with other " +
-              "operations, so hosts, configs, description, service config 
version note " +
-              "request fields should not be populated.");
-    }
-  }
-
   private void validateRequest(ConfigGroupRequest request) {
     if (request.getClusterName() == null
       || request.getClusterName().isEmpty()
@@ -534,87 +504,79 @@ public class ConfigGroupResourceProvider extends
     Clusters clusters = getManagementController().getClusters();
 
     for (ConfigGroupRequest request : requests) {
-      basicRequestValidation(request);
 
       Cluster cluster;
       try {
         cluster = clusters.getCluster(request.getClusterName());
       } catch (ClusterNotFoundException e) {
         throw new ParentObjectNotFoundException(
-          String.format(
-            "The cluster %s does not exist, can not update a config group",
-          request.getClusterName()), e);
+          "Attempted to add a config group to a cluster which doesn't exist", 
e);
       }
 
+      if (request.getId() == null) {
+        throw new AmbariException("Config group Id is a required parameter.");
+      }
+
+      validateRequest(request);
+
       // Find config group
-      Map<Long, ConfigGroup> configGroups = cluster.getConfigGroups();
-      ConfigGroup configGroup = configGroups.get(request.getId());
+      ConfigGroup configGroup = cluster.getConfigGroups().get(request.getId());
       if (configGroup == null) {
         throw new AmbariException("Config group not found"
                                  + ", clusterName = " + 
request.getClusterName()
                                  + ", groupId = " + request.getId());
       }
+      String serviceName = configGroup.getServiceName();
+      String requestServiceName = 
cluster.getServiceForConfigTypes(request.getConfigs().keySet());
+      if (serviceName != null && requestServiceName !=null && 
!StringUtils.equals(serviceName, requestServiceName)) {
+        throw new IllegalArgumentException("Config group " + 
configGroup.getId() +
+            " is mapped to service " + serviceName + ", " +
+            "but request contain configs from service " + requestServiceName);
+      } else if (serviceName == null && requestServiceName != null) {
+        configGroup.setServiceName(requestServiceName);
+        serviceName = requestServiceName;
+      }
 
-      if (! configGroup.getName().equals(request.getGroupName())) {
-        // Looks like user tries to rename a config group
-        validateRenameRequest(request);
-        configGroup.setName(request.getGroupName());
-        persist(configGroup);
-      } else { // Any other action
-        validateRequest(request);
-        String serviceName = configGroup.getServiceName();
-        String requestServiceName = 
cluster.getServiceForConfigTypes(request.getConfigs().keySet());
-        if (serviceName != null && requestServiceName !=null && 
!StringUtils.equals(serviceName, requestServiceName)) {
-          throw new IllegalArgumentException("Config group " + 
configGroup.getId() +
-              " is mapped to service " + serviceName + ", " +
-              "but request contain configs from service " + 
requestServiceName);
-        } else if (serviceName == null && requestServiceName != null) {
-          configGroup.setServiceName(requestServiceName);
-          serviceName = requestServiceName;
-        }
-
-        // Update hosts
-        Map<String, Host> hosts = new HashMap<String, Host>();
-        if (request.getHosts() != null && !request.getHosts().isEmpty()) {
-          for (String hostname : request.getHosts()) {
-            Host host = clusters.getHost(hostname);
-            if (host == null) {
-              throw new HostNotFoundException(hostname);
-            }
-            hosts.put(hostname, host);
+      // Update hosts
+      Map<String, Host> hosts = new HashMap<String, Host>();
+      if (request.getHosts() != null && !request.getHosts().isEmpty()) {
+        for (String hostname : request.getHosts()) {
+          Host host = clusters.getHost(hostname);
+          if (host == null) {
+            throw new HostNotFoundException(hostname);
           }
+          hosts.put(hostname, host);
         }
+      }
+
+      verifyHostList(cluster, hosts, request);
 
-        verifyHostList(cluster, hosts, request);
+      configGroup.setHosts(hosts);
 
-        configGroup.setHosts(hosts);
+      // Update Configs
+      configGroup.setConfigurations(request.getConfigs());
 
-        // Update Configs
-        configGroup.setConfigurations(request.getConfigs());
-        // Save
-        configGroup.setName(request.getGroupName());
-        configGroup.setDescription(request.getDescription());
-        configGroup.setTag(request.getTag());
+      // Save
+      configGroup.setName(request.getGroupName());
+      configGroup.setDescription(request.getDescription());
+      configGroup.setTag(request.getTag());
 
-        persist(configGroup);
-        if (serviceName != null) {
-          cluster.createServiceConfigVersion(serviceName, 
getManagementController().getAuthName(), null, configGroup);
-        } else {
-          LOG.warn("Could not determine service name for config group {}, 
service config version not created",
-                  configGroup.getId());
-        }
+      configLogger.info("Persisting updated Config group"
+        + ", clusterName = " + configGroup.getClusterName()
+        + ", id = " + configGroup.getId()
+        + ", tag = " + configGroup.getTag()
+        + ", user = " + getManagementController().getAuthName());
+
+      configGroup.persist();
+      if (serviceName != null) {
+        cluster.createServiceConfigVersion(serviceName, 
getManagementController().getAuthName(), null, configGroup);
+      } else {
+        LOG.warn("Could not determine service name for config group {}, 
service config version not created",
+            configGroup.getId());
       }
     }
-    getManagementController().getConfigHelper().invalidateStaleConfigsCache();
-  }
 
-  private void persist(ConfigGroup configGroup) {
-    configLogger.info("Persisting updated Config group"
-            + ", clusterName = " + configGroup.getClusterName()
-            + ", id = " + configGroup.getId()
-            + ", tag = " + configGroup.getTag()
-            + ", user = " + getManagementController().getAuthName());
-    configGroup.persist();
+    getManagementController().getConfigHelper().invalidateStaleConfigsCache();
   }
 
   @SuppressWarnings("unchecked")

http://git-wip-us.apache.org/repos/asf/ambari/blob/22b1df06/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ConfigGroupResourceProviderTest.java
----------------------------------------------------------------------
diff --git 
a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ConfigGroupResourceProviderTest.java
 
b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ConfigGroupResourceProviderTest.java
index 116e435..4194979 100644
--- 
a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ConfigGroupResourceProviderTest.java
+++ 
b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ConfigGroupResourceProviderTest.java
@@ -49,7 +49,6 @@ import java.util.Set;
 import static junit.framework.Assert.assertEquals;
 import static junit.framework.Assert.assertNotNull;
 import static junit.framework.Assert.assertTrue;
-import static junit.framework.Assert.fail;
 import static org.easymock.EasyMock.anyObject;
 import static org.easymock.EasyMock.capture;
 import static org.easymock.EasyMock.createMock;
@@ -57,7 +56,6 @@ import static org.easymock.EasyMock.createNiceMock;
 import static org.easymock.EasyMock.expect;
 import static org.easymock.EasyMock.expectLastCall;
 import static org.easymock.EasyMock.replay;
-import static org.easymock.EasyMock.reset;
 import static org.easymock.EasyMock.verify;
 
 public class ConfigGroupResourceProviderTest {
@@ -307,134 +305,6 @@ public class ConfigGroupResourceProviderTest {
       configGroup, response, configGroupResponse, configHelper);
   }
 
-  @Test
-  public void testUpdateConfigGroup_renameGroup() throws Exception {
-    AmbariManagementController managementController = 
createMock(AmbariManagementController.class);
-    RequestStatusResponse response = 
createNiceMock(RequestStatusResponse.class);
-    ConfigHelper configHelper = createNiceMock(ConfigHelper.class);
-    Clusters clusters = createNiceMock(Clusters.class);
-    Cluster cluster = createNiceMock(Cluster.class);
-    Host h1 = createNiceMock(Host.class);
-    Host h2 = createNiceMock(Host.class);
-    final ConfigGroup configGroup = createNiceMock(ConfigGroup.class);
-    ConfigGroupResponse configGroupResponse = createNiceMock
-            (ConfigGroupResponse.class);
-
-    expect(managementController.getClusters()).andReturn(clusters).anyTimes();
-    expect(managementController.getAuthName()).andReturn("admin").anyTimes();
-    expect(clusters.getCluster("Cluster100")).andReturn(cluster).anyTimes();
-
-    expect(configGroup.getName()).andReturn("originalGroupName").anyTimes();
-    expect(configGroup.getId()).andReturn(25L).anyTimes();
-    
expect(configGroup.convertToResponse()).andReturn(configGroupResponse).anyTimes();
-    
expect(configGroupResponse.getClusterName()).andReturn("Cluster100").anyTimes();
-    expect(configGroupResponse.getId()).andReturn(25L).anyTimes();
-
-    replay(configGroup);
-
-    expect(cluster.getConfigGroups()).andStubAnswer(new IAnswer<Map<Long, 
ConfigGroup>>() {
-      @Override
-      public Map<Long, ConfigGroup> answer() throws Throwable {
-        Map<Long, ConfigGroup> configGroupMap = new HashMap<Long, 
ConfigGroup>();
-        configGroupMap.put(configGroup.getId(), configGroup);
-        return configGroupMap;
-      }
-    });
-
-
-    
expect(managementController.getConfigHelper()).andReturn(configHelper).once();
-    configHelper.invalidateStaleConfigsCache();
-    expectLastCall().once();
-
-    replay(managementController, clusters, cluster,
-            response, configGroupResponse, configHelper);
-
-    ResourceProvider provider = getConfigGroupResourceProvider
-            (managementController);
-
-    String newGroupName = "newGroupName";
-    /*
-     * Check rename request validation
-     */
-    Map<String, Object> properties = new LinkedHashMap<String, Object>();
-
-    Set<Map<String, Object>> hostSet = new HashSet<Map<String, Object>>();
-    Map<String, Object> host1 = new HashMap<String, Object>();
-    host1.put(ConfigGroupResourceProvider.CONFIGGROUP_HOSTNAME_PROPERTY_ID, 
"h1");
-    hostSet.add(host1);
-    Map<String, Object> host2 = new HashMap<String, Object>();
-    host2.put(ConfigGroupResourceProvider.CONFIGGROUP_HOSTNAME_PROPERTY_ID, 
"h2");
-    hostSet.add(host2);
-
-    Set<Map<String, Object>> configSet = new HashSet<Map<String, Object>>();
-    Map<String, String> configMap = new HashMap<String, String>();
-    Map<String, Object> configs = new HashMap<String, Object>();
-    configs.put("type", "core-site");
-    configs.put("tag", "version100");
-    configMap.put("key1", "value1");
-    configs.put("properties", configMap);
-    configSet.add(configs);
-
-    properties.put(ConfigGroupResourceProvider
-            .CONFIGGROUP_CLUSTER_NAME_PROPERTY_ID, "Cluster100");
-    properties.put(ConfigGroupResourceProvider.CONFIGGROUP_NAME_PROPERTY_ID,
-            newGroupName);
-    properties.put(ConfigGroupResourceProvider.CONFIGGROUP_TAG_PROPERTY_ID,
-            "tag-1");
-    properties.put(ConfigGroupResourceProvider.CONFIGGROUP_HOSTS_PROPERTY_ID,
-            hostSet );
-    properties.put(ConfigGroupResourceProvider.CONFIGGROUP_CONFIGS_PROPERTY_ID,
-            configSet);
-
-    Map<String, String> mapRequestProps = new HashMap<String, String>();
-    mapRequestProps.put("context", "Called from a test");
-
-    Request request = PropertyHelper.getUpdateRequest(properties, 
mapRequestProps);
-
-    Predicate predicate = new PredicateBuilder().property
-            
(ConfigGroupResourceProvider.CONFIGGROUP_CLUSTER_NAME_PROPERTY_ID).equals
-            ("Cluster100").and().
-            
property(ConfigGroupResourceProvider.CONFIGGROUP_ID_PROPERTY_ID).equals
-            (25L).toPredicate();
-
-    try {
-      provider.updateResources(request, predicate);
-      fail("Should throw IllegalArgumentException");
-    } catch (IllegalArgumentException e) {
-      // expected
-    }
-
-    verify(configGroup);
-
-    /*
-     * Check execution of  rename request
-     */
-    reset(configGroup);
-    expect(configGroup.getName()).andReturn("originalGroupName").anyTimes();
-    expect(configGroup.getId()).andReturn(25L).anyTimes();
-
-    
expect(configGroup.convertToResponse()).andReturn(configGroupResponse).anyTimes();
-
-    configGroup.setName(newGroupName);
-    expectLastCall().once();
-    replay(configGroup);
-
-    reset(cluster);
-    Map<Long, ConfigGroup> configGroupMap = new HashMap<Long, ConfigGroup>();
-    configGroupMap.put(configGroup.getId(), configGroup);
-    expect(cluster.getConfigGroups()).andReturn(configGroupMap).anyTimes();
-    replay(cluster);
-
-    properties = new LinkedHashMap<String, Object>();
-    properties.put(ConfigGroupResourceProvider
-            .CONFIGGROUP_CLUSTER_NAME_PROPERTY_ID, "Cluster100");
-    properties.put(ConfigGroupResourceProvider.CONFIGGROUP_NAME_PROPERTY_ID,
-            newGroupName);
-    request = PropertyHelper.getUpdateRequest(properties, mapRequestProps);
-    provider.updateResources(request, predicate);
-    verify(configGroup);
-  }
-
   @SuppressWarnings("unchecked")
   @Test
   public void testGetConfigGroup() throws Exception {

Reply via email to