AMBARI-15200. Unusable configs after creating override. (stoader)

ambari-server/src/main/java/org/apache/ambari/server/controller/ServiceConfigVersionResponse.java

@@ -20,17 +20,21 @@ package org.apache.ambari.server.controller;
 
 
 import java.util.List;
+import java.util.Objects;
 
-import com.google.inject.Inject;
 import org.apache.ambari.server.StaticallyInject;
 import org.apache.ambari.server.orm.dao.HostDAO;
 import org.apache.ambari.server.orm.entities.ClusterEntity;
 import org.apache.ambari.server.orm.entities.ServiceConfigEntity;
 import org.apache.ambari.server.orm.entities.StackEntity;
 import org.apache.ambari.server.state.StackId;
+import org.apache.commons.lang.builder.EqualsBuilder;
+import org.apache.commons.lang.builder.HashCodeBuilder;
 import org.codehaus.jackson.annotate.JsonProperty;
 import org.codehaus.jackson.map.annotate.JsonSerialize;
 
+import com.google.inject.Inject;
+
 @StaticallyInject
 public class ServiceConfigVersionResponse {
   @JsonProperty("cluster_name")
@@ -176,5 +180,49 @@ public class ServiceConfigVersionResponse {
   public Boolean isCompatibleWithCurrentStack() {
     return isCompatibleWithCurrentStack;
   }
+
+  @Override
+  public final boolean equals(Object o) {
+    if (this == o) return true;
+
+    if (!(o instanceof ServiceConfigVersionResponse)) return false;
+
+    ServiceConfigVersionResponse that = (ServiceConfigVersionResponse) o;
+
+    return new EqualsBuilder()
+      .append(clusterName, that.clusterName)
+      .append(serviceName, that.serviceName)
+      .append(version, that.version)
+      .append(createTime, that.createTime)
+      .append(groupId, that.groupId)
+      .append(groupName, that.groupName)
+      .append(userName, that.userName)
+      .append(note, that.note)
+      .append(stackId, that.stackId)
+      .append(isCurrent, that.isCurrent)
+      .append(isCompatibleWithCurrentStack, that.isCompatibleWithCurrentStack)
+      .append(configurations, that.configurations)
+      .append(hosts, that.hosts)
+      .isEquals();
+  }
+
+  @Override
+  public final int hashCode() {
+    return new HashCodeBuilder(17, 37)
+      .append(clusterName)
+      .append(serviceName)
+      .append(version)
+      .append(createTime)
+      .append(groupId)
+      .append(groupName)
+      .append(userName)
+      .append(note)
+      .append(stackId)
+      .append(isCurrent)
+      .append(isCompatibleWithCurrentStack)
+      .append(configurations)
+      .append(hosts)
+      .toHashCode();
+  }
 }
 

ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java

@@ -2347,7 +2347,7 @@ public class ClusterImpl implements Cluster {
       configGroup == null ? "-1" : configGroup.getId(),
       serviceConfigEntity.getVersion());
 
-    String configGroupName = configGroup != null ? configGroup.getName() : null;
+    String configGroupName = configGroup != null ? configGroup.getName() : "default";
 
     ServiceConfigVersionResponse response = new ServiceConfigVersionResponse(
         serviceConfigEntity, configGroupName);
@@ -2434,26 +2434,40 @@ public class ClusterImpl implements Cluster {
       List<ServiceConfigVersionResponse> serviceConfigVersionResponses = new ArrayList<ServiceConfigVersionResponse>();
 
       List<ServiceConfigEntity> serviceConfigs = serviceConfigDAO.getServiceConfigs(getClusterId());
-      Map<String, ServiceConfigVersionResponse> activeServiceConfigResponses = new HashMap<>();
+
+      // Gather for each service in each config group the active service config response  as we
+      // iterate through all service config responses
+      Map<String, Map<String, ServiceConfigVersionResponse>> activeServiceConfigResponses = new HashMap<>();
 
       for (ServiceConfigEntity serviceConfigEntity : serviceConfigs) {
         ServiceConfigVersionResponse serviceConfigVersionResponse = convertToServiceConfigVersionResponse(serviceConfigEntity);
 
-        ServiceConfigVersionResponse activeServiceConfigResponse = activeServiceConfigResponses.get(serviceConfigVersionResponse.getServiceName());
+        Map<String, ServiceConfigVersionResponse> activeServiceConfigResponseGroups = activeServiceConfigResponses.get(serviceConfigVersionResponse.getServiceName());
+
+        if (activeServiceConfigResponseGroups == null) {
+          Map<String, ServiceConfigVersionResponse> serviceConfigGroups = new HashMap<>();
+          activeServiceConfigResponses.put(serviceConfigVersionResponse.getServiceName(), serviceConfigGroups);
+
+          activeServiceConfigResponseGroups = serviceConfigGroups;
+        }
+
+        // the active config within a group
+        ServiceConfigVersionResponse activeServiceConfigResponse = activeServiceConfigResponseGroups.get(serviceConfigVersionResponse.getGroupName());
+
         if (activeServiceConfigResponse == null) {
+          activeServiceConfigResponseGroups.put(serviceConfigVersionResponse.getGroupName(), serviceConfigVersionResponse);
           activeServiceConfigResponse = serviceConfigVersionResponse;
-          activeServiceConfigResponses.put(serviceConfigVersionResponse.getServiceName(), serviceConfigVersionResponse);
         }
 
         serviceConfigVersionResponse.setConfigurations(new ArrayList<ConfigurationResponse>());
 
         if (serviceConfigEntity.getGroupId() == null) {
           if (serviceConfigVersionResponse.getCreateTime() > activeServiceConfigResponse.getCreateTime())
-            activeServiceConfigResponses.put(serviceConfigVersionResponse.getServiceName(), serviceConfigVersionResponse);
+            activeServiceConfigResponseGroups.put(serviceConfigVersionResponse.getGroupName(), serviceConfigVersionResponse);
         }
         else if (clusterConfigGroups != null && clusterConfigGroups.containsKey(serviceConfigEntity.getGroupId())){
           if (serviceConfigVersionResponse.getVersion() > activeServiceConfigResponse.getVersion())
-            activeServiceConfigResponses.put(serviceConfigVersionResponse.getServiceName(), serviceConfigVersionResponse);
+            activeServiceConfigResponseGroups.put(serviceConfigVersionResponse.getGroupName(), serviceConfigVersionResponse);
         }
 
         serviceConfigVersionResponse.setIsCurrent(false);
@@ -2472,8 +2486,10 @@ public class ClusterImpl implements Cluster {
         serviceConfigVersionResponses.add(serviceConfigVersionResponse);
       }
 
-      for (ServiceConfigVersionResponse serviceConfigVersionResponse: activeServiceConfigResponses.values()) {
-        serviceConfigVersionResponse.setIsCurrent(true);
+      for (Map<String, ServiceConfigVersionResponse> serviceConfigVersionResponseGroup: activeServiceConfigResponses.values()) {
+        for (ServiceConfigVersionResponse serviceConfigVersionResponse : serviceConfigVersionResponseGroup.values()) {
+          serviceConfigVersionResponse.setIsCurrent(true);
+        }
       }
 
       return serviceConfigVersionResponses;

ambari-server/src/test/java/org/apache/ambari/server/controller/ServiceConfigVersionResponseTest.java

@@ -0,0 +1,34 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ambari.server.controller;
+
+import org.junit.Test;
+
+import nl.jqno.equalsverifier.EqualsVerifier;
+import nl.jqno.equalsverifier.Warning;
+
+public class ServiceConfigVersionResponseTest {
+  @Test
+  public void testEquals() throws Exception {
+    EqualsVerifier
+      .forClass(ServiceConfigVersionResponse.class)
+      .suppress(Warning.NONFINAL_FIELDS)
+      .verify();
+  }
+}

ambari-server/src/test/java/org/apache/ambari/server/state/cluster/ClusterTest.java

@@ -18,19 +18,12 @@
 
 package org.apache.ambari.server.state.cluster;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.when;
-
 import java.lang.reflect.Field;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.Comparator;
 import java.util.ConcurrentModificationException;
 import java.util.EnumSet;
 import java.util.HashMap;
@@ -50,6 +43,7 @@ import org.apache.ambari.server.agent.DiskInfo;
 import org.apache.ambari.server.agent.HostInfo;
 import org.apache.ambari.server.api.services.AmbariMetaInfo;
 import org.apache.ambari.server.controller.ClusterResponse;
+import org.apache.ambari.server.controller.ConfigurationResponse;
 import org.apache.ambari.server.controller.ServiceConfigVersionResponse;
 import org.apache.ambari.server.orm.GuiceJpaInitializer;
 import org.apache.ambari.server.orm.InMemoryDefaultTestModule;
@@ -111,6 +105,8 @@ import org.junit.Ignore;
 import org.junit.Test;
 import org.mockito.ArgumentCaptor;
 
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
 import com.google.gson.Gson;
 import com.google.inject.AbstractModule;
 import com.google.inject.Guice;
@@ -123,6 +119,16 @@ import com.google.inject.util.Modules;
 
 import junit.framework.Assert;
 
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
 public class ClusterTest {
 
   private static final EnumSet<RepositoryVersionState> TERMINAL_VERSION_STATES =
@@ -1334,6 +1340,70 @@ public class ClusterTest {
     assertEquals("Five total scvs", 5, c1.getServiceConfigVersions().size());
   }
 
+  @Test
+  public void testAllServiceConfigVersionsWithConfigGroups() throws Exception {
+    // Given
+    createDefaultCluster();
+
+    Config hdfsSiteConfigV1 = configFactory.createNew(c1, "hdfs-site", ImmutableMap.of("p1", "v1"), ImmutableMap.<String, Map<String,String>>of());
+    hdfsSiteConfigV1.setTag("version1");
+    hdfsSiteConfigV1.persist();
+
+    c1.addConfig(hdfsSiteConfigV1);
+
+
+    ServiceConfigVersionResponse hdfsSiteConfigResponseV1 = c1.addDesiredConfig("admin", Collections.singleton(hdfsSiteConfigV1));
+    List<ConfigurationResponse> configResponsesDefaultGroup =  Collections.singletonList(
+      new ConfigurationResponse(c1.getClusterName(), hdfsSiteConfigV1.getStackId(),
+        hdfsSiteConfigV1.getType(), hdfsSiteConfigV1.getTag(), hdfsSiteConfigV1.getVersion(),
+        hdfsSiteConfigV1.getProperties(), hdfsSiteConfigV1.getPropertiesAttributes(), hdfsSiteConfigV1.getPropertiesTypes())
+    );
+
+    hdfsSiteConfigResponseV1.setConfigurations(configResponsesDefaultGroup);
+
+    Config hdfsSiteConfigV2 = configFactory.createNew(c1, "hdfs-site", ImmutableMap.of("p1", "v2"), ImmutableMap.<String, Map<String,String>>of());
+    hdfsSiteConfigV2.setTag("version2");
+
+    ConfigGroup configGroup = configGroupFactory.createNew(c1, "configGroup1", "version1", "test description", ImmutableMap.of(hdfsSiteConfigV2.getType(), hdfsSiteConfigV2), ImmutableMap.<Long, Host>of());
+    configGroup.persist();
+
+    c1.addConfigGroup(configGroup);
+    ServiceConfigVersionResponse hdfsSiteConfigResponseV2 = c1.createServiceConfigVersion("HDFS", "admin", "test note", configGroup);
+    hdfsSiteConfigResponseV2.setConfigurations(Collections.singletonList(
+      new ConfigurationResponse(c1.getClusterName(), hdfsSiteConfigV2.getStackId(),
+        hdfsSiteConfigV2.getType(), hdfsSiteConfigV2.getTag(), hdfsSiteConfigV2.getVersion(),
+        hdfsSiteConfigV2.getProperties(), hdfsSiteConfigV2.getPropertiesAttributes(), hdfsSiteConfigV2.getPropertiesTypes())
+    ));
+    hdfsSiteConfigResponseV2.setIsCurrent(true); // this is the active config in 'configGroup1' config group as it's the solely service config
+
+    // hdfs config v3
+    ServiceConfigVersionResponse hdfsSiteConfigResponseV3 = c1.createServiceConfigVersion("HDFS", "admin", "new config in default group", null);
+    hdfsSiteConfigResponseV3.setConfigurations(configResponsesDefaultGroup);
+    hdfsSiteConfigResponseV3.setIsCurrent(true); // this is the active config in default config group as it's more recent than V1
+
+
+
+    // When
+    List<ServiceConfigVersionResponse> expectedServiceConfigResponses = ImmutableList.of(hdfsSiteConfigResponseV1, hdfsSiteConfigResponseV2, hdfsSiteConfigResponseV3);
+    List<ServiceConfigVersionResponse> allServiceConfigResponses = c1.getServiceConfigVersions();
+
+
+    Collections.sort(
+      allServiceConfigResponses,
+      new Comparator<ServiceConfigVersionResponse>() {
+        @Override
+        public int compare(ServiceConfigVersionResponse o1, ServiceConfigVersionResponse o2) {
+          return o1.getVersion().compareTo(o2.getVersion());
+        }
+      }
+    );
+
+    // Then
+    assertThat(
+      allServiceConfigResponses,
+      is(expectedServiceConfigResponses));
+  }
+
   @Test
   public void testTransitionClusterVersion() throws Exception {
     createDefaultCluster();