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

ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ConfigGroupResourceProvider.java

@@ -383,6 +383,36 @@ 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()
@@ -504,81 +534,89 @@ 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(
-          "Attempted to add a config group to a cluster which doesn't exist", e);
+          String.format(
+            "The cluster %s does not exist, can not update a config group",
+          request.getClusterName()), e);
       }
 
-      if (request.getId() == null) {
-        throw new AmbariException("Config group Id is a required parameter.");
-      }
-
-      validateRequest(request);
-
       // Find config group
-      ConfigGroup configGroup = cluster.getConfigGroups().get(request.getId());
+      Map<Long, ConfigGroup> configGroups = cluster.getConfigGroups();
+      ConfigGroup configGroup = configGroups.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;
-      }
 
-      // 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);
+      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;
         }
-      }
-
-      verifyHostList(cluster, hosts, request);
 
-      configGroup.setHosts(hosts);
+        // 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 Configs
-      configGroup.setConfigurations(request.getConfigs());
+        verifyHostList(cluster, hosts, request);
 
-      // Save
-      configGroup.setName(request.getGroupName());
-      configGroup.setDescription(request.getDescription());
-      configGroup.setTag(request.getTag());
+        configGroup.setHosts(hosts);
 
-      configLogger.info("Persisting updated Config group"
-        + ", clusterName = " + configGroup.getClusterName()
-        + ", id = " + configGroup.getId()
-        + ", tag = " + configGroup.getTag()
-        + ", user = " + getManagementController().getAuthName());
+        // Update Configs
+        configGroup.setConfigurations(request.getConfigs());
+        // Save
+        configGroup.setName(request.getGroupName());
+        configGroup.setDescription(request.getDescription());
+        configGroup.setTag(request.getTag());
 
-      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());
+        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());
+        }
       }
     }
-
     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();
+  }
+
   @SuppressWarnings("unchecked")
   ConfigGroupRequest getConfigGroupRequest(Map<String, Object> properties) {
     Object groupIdObj = properties.get(CONFIGGROUP_ID_PROPERTY_ID);

ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ConfigGroupResourceProviderTest.java

@@ -49,6 +49,7 @@ 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;
@@ -56,6 +57,7 @@ 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 {
@@ -305,6 +307,134 @@ 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 {