ambari-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From nc...@apache.org
Subject [31/50] ambari git commit: AMBARI-14040. Update cluster fails with NPE if the request contains null configuration property values (Sebastian Toader via alejandro)
Date Tue, 01 Dec 2015 18:20:21 GMT
AMBARI-14040. Update cluster fails with NPE if the request contains null configuration property
values (Sebastian Toader via alejandro)


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

Branch: refs/heads/branch-dev-patch-upgrade
Commit: a62c4b8aad56fce846603b9ad45ceead0300ae29
Parents: 0ada28a
Author: Alejandro Fernandez <afernandez@hortonworks.com>
Authored: Mon Nov 30 16:01:50 2015 -0800
Committer: Alejandro Fernandez <afernandez@hortonworks.com>
Committed: Mon Nov 30 16:01:50 2015 -0800

----------------------------------------------------------------------
 .../AmbariManagementControllerImpl.java         |  4 +-
 .../internal/StackAdvisorResourceProvider.java  |  8 +-
 .../AmbariManagementControllerImplTest.java     | 90 +++++++++++++++++++-
 .../StackAdvisorResourceProviderTest.java       | 40 +++++++++
 4 files changed, 134 insertions(+), 8 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/ambari/blob/a62c4b8a/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java
b/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java
index 0e3e7b8..c0dc342 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java
@@ -1432,8 +1432,8 @@ public class AmbariManagementControllerImpl implements AmbariManagementControlle
             break;
           } else {
             for (Entry<String, String> property : requestConfigProperties.entrySet())
{
-              if (!StringUtils.equals(property.getValue(),clusterConfigProperties.get(property.getKey())))
{
-                isConfigurationCreationNeeded =true;
+              if (!StringUtils.equals(property.getValue(), clusterConfigProperties.get(property.getKey())))
{
+                isConfigurationCreationNeeded = true;
                 break;
               }
             }

http://git-wip-us.apache.org/repos/asf/ambari/blob/a62c4b8a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/StackAdvisorResourceProvider.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/StackAdvisorResourceProvider.java
b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/StackAdvisorResourceProvider.java
index fe3d006..0ad9126 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/StackAdvisorResourceProvider.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/StackAdvisorResourceProvider.java
@@ -266,8 +266,12 @@ public abstract class StackAdvisorResourceProvider extends ReadOnlyResourceProvi
             siteMap.put(propertiesProperty, propertiesMap);
           }
 
-          String value = properties.get(property).toString();
-          propertiesMap.put(propertyName, value);
+          Object propVal = properties.get(property);
+          if (propVal != null)
+            propertiesMap.put(propertyName, propVal.toString());
+          else
+            LOG.info(String.format("No value specified for configuration property, name =
%s ", property));
+
         } catch (Exception e) {
           LOG.debug(String.format("Error handling configuration property, name = %s", property),
e);
           // do nothing

http://git-wip-us.apache.org/repos/asf/ambari/blob/a62c4b8a/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java
b/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java
index ca3ca36..e2ec5e0 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java
@@ -18,6 +18,8 @@
 
 package org.apache.ambari.server.controller;
 
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Maps;
 import com.google.gson.Gson;
 import com.google.inject.Binder;
 import com.google.inject.Guice;
@@ -52,8 +54,10 @@ import org.apache.ambari.server.security.ldap.LdapBatchDto;
 import org.apache.ambari.server.state.Cluster;
 import org.apache.ambari.server.state.Clusters;
 import org.apache.ambari.server.state.ComponentInfo;
+import org.apache.ambari.server.state.ConfigImpl;
 import org.apache.ambari.server.state.Host;
 import org.apache.ambari.server.state.MaintenanceState;
+import org.apache.ambari.server.state.PropertyInfo;
 import org.apache.ambari.server.state.RepositoryInfo;
 import org.apache.ambari.server.state.SecurityType;
 import org.apache.ambari.server.state.Service;
@@ -85,7 +89,20 @@ import static org.apache.ambari.server.agent.ExecutionCommand.KeyNames.HOST_SYS_
 import static org.apache.ambari.server.agent.ExecutionCommand.KeyNames.JAVA_VERSION;
 import static org.apache.ambari.server.agent.ExecutionCommand.KeyNames.STACK_NAME;
 import static org.apache.ambari.server.agent.ExecutionCommand.KeyNames.STACK_VERSION;
-import static org.easymock.EasyMock.*;
+import static org.easymock.EasyMock.anyBoolean;
+import static org.easymock.EasyMock.anyObject;
+import static org.easymock.EasyMock.capture;
+import static org.easymock.EasyMock.captureBoolean;
+import static org.easymock.EasyMock.createMock;
+import static org.easymock.EasyMock.createMockBuilder;
+import static org.easymock.EasyMock.createNiceMock;
+import static org.easymock.EasyMock.createStrictMock;
+import static org.easymock.EasyMock.eq;
+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;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
@@ -346,7 +363,7 @@ public class AmbariManagementControllerImplTest {
 
     expect(service.getName()).andReturn("service");
     expect(service.getServiceComponent("component")).andThrow(
-        new ServiceComponentNotFoundException("cluster", "service", "component"));
+      new ServiceComponentNotFoundException("cluster", "service", "component"));
     expect(service.getDesiredStackVersion()).andReturn(stackId);
     expect(stackId.getStackName()).andReturn("stack");
     expect(stackId.getStackVersion()).andReturn("1.0");
@@ -356,7 +373,7 @@ public class AmbariManagementControllerImplTest {
     expect(service.getServiceComponents()).andReturn(componentsMap);
     expect(component1.getServiceComponentHosts()).andReturn(Collections.EMPTY_MAP);
     expect(component2.getServiceComponentHosts()).andReturn(
-        Collections.<String, ServiceComponentHost>singletonMap("anyHost", null));
+      Collections.<String, ServiceComponentHost>singletonMap("anyHost", null));
 
     ServiceInfo serviceInfo = createNiceMock(ServiceInfo.class);
     ComponentInfo compInfo = createNiceMock(ComponentInfo.class);
@@ -394,7 +411,7 @@ public class AmbariManagementControllerImplTest {
     expect(service.getServiceComponents()).andReturn(componentsMap);
     expect(component1.getServiceComponentHosts()).andReturn(Collections.EMPTY_MAP);
     expect(component2.getServiceComponentHosts()).andReturn(
-        Collections.<String, ServiceComponentHost>singletonMap("anyHost", null));
+      Collections.<String, ServiceComponentHost>singletonMap("anyHost", null));
 
     ServiceInfo serviceInfo = createNiceMock(ServiceInfo.class);
     expect(serviceInfo.getClientComponent()).andReturn(null);
@@ -566,6 +583,71 @@ public class AmbariManagementControllerImplTest {
   }
 
   /**
+   * Ensure that processing update request does not fail on configuration
+   * properties with no value specified (no value = null reference value)
+   */
+  @Test
+  public void testUpdateClustersWithNullConfigPropertyValues() throws Exception {
+    // member state mocks
+    Capture<AmbariManagementController> controllerCapture = new Capture<AmbariManagementController>();
+    Injector injector = createStrictMock(Injector.class);
+    Cluster cluster = createNiceMock(Cluster.class);
+    ActionManager actionManager = createNiceMock(ActionManager.class);
+    ClusterRequest clusterRequest = createNiceMock(ClusterRequest.class);
+
+    // requests
+    Set<ClusterRequest> setRequests = Collections.singleton(clusterRequest);
+
+    KerberosHelper kerberosHelper = createStrictMock(KerberosHelper.class);
+    // expectations
+    injector.injectMembers(capture(controllerCapture));
+    expect(injector.getInstance(Gson.class)).andReturn(null);
+    expect(injector.getInstance(MaintenanceStateHelper.class)).andReturn(null);
+    expect(injector.getInstance(KerberosHelper.class)).andReturn(kerberosHelper);
+    expect(clusterRequest.getClusterName()).andReturn("clusterNew").anyTimes();
+    expect(clusterRequest.getClusterId()).andReturn(1L).anyTimes();
+
+    ConfigurationRequest configReq = new ConfigurationRequest();
+    final Map<String, String> configReqProps = Maps.newHashMap();
+    configReqProps.put("p1", null);
+    configReq.setProperties(configReqProps);
+
+    expect(clusterRequest.getDesiredConfig()).andReturn(ImmutableList.of(configReq)).anyTimes();
+    expect(clusters.getClusterById(1L)).andReturn(cluster).anyTimes();
+    expect(cluster.getClusterName()).andReturn("clusterOld").anyTimes();
+    expect(cluster.getConfigPropertiesTypes(anyObject(String.class))).andReturn(Maps.<PropertyInfo.PropertyType,
Set<String>>newHashMap()).anyTimes();
+    expect(cluster.getDesiredConfigByType(anyObject(String.class))).andReturn(new ConfigImpl("config-type")
{
+      @Override
+      public Map<String, Map<String, String>> getPropertiesAttributes() {
+        return Maps.newHashMap();
+      }
+
+      @Override
+      public Map<String, String> getProperties() {
+        return configReqProps;
+      }
+
+    }).anyTimes();
+
+    cluster.addSessionAttributes(anyObject(Map.class));
+    expectLastCall().once();
+
+    cluster.setClusterName("clusterNew");
+    expectLastCall();
+
+    // replay mocks
+    replay(actionManager, cluster, clusters, injector, clusterRequest, sessionManager);
+
+    // test
+    AmbariManagementController controller = new AmbariManagementControllerImpl(actionManager,
clusters, injector);
+    controller.updateClusters(setRequests, null);
+
+    // assert and verify
+    assertSame(controller, controllerCapture.getValue());
+    verify(actionManager, cluster, clusters, injector, clusterRequest, sessionManager);
+  }
+
+  /**
    * Ensure that when the cluster is updated KerberosHandler.toggleKerberos is not invoked
unless
    * the security type is altered
    */

http://git-wip-us.apache.org/repos/asf/ambari/blob/a62c4b8a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/StackAdvisorResourceProviderTest.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/StackAdvisorResourceProviderTest.java
b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/StackAdvisorResourceProviderTest.java
index 8c5337b..e3b89b8 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/StackAdvisorResourceProviderTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/StackAdvisorResourceProviderTest.java
@@ -33,6 +33,7 @@ import java.util.Set;
 
 import static org.apache.ambari.server.controller.internal.StackAdvisorResourceProvider.CONFIGURATIONS_PROPERTY_ID;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
@@ -73,4 +74,43 @@ public class StackAdvisorResourceProviderTest {
     assertEquals("string", properties.get("string_prop"));
     assertEquals("[array1, array2]", properties.get("array_prop"));
   }
+
+  @Test
+  public void testCalculateConfigurationsWithNullPropertyValues() throws Exception {
+
+    Map<Resource.Type, String> keyPropertyIds = Collections.emptyMap();
+    Set<String> propertyIds = Collections.emptySet();
+    AmbariManagementController ambariManagementController = mock(AmbariManagementController.class);
+    RecommendationResourceProvider provider = new RecommendationResourceProvider(propertyIds,
+      keyPropertyIds, ambariManagementController);
+
+    Request request = mock(Request.class);
+    Set<Map<String, Object>> propertiesSet = new HashSet<Map<String, Object>>();
+    Map<String, Object> propertiesMap = new HashMap<String, Object>();
+    propertiesMap.put(CONFIGURATIONS_PROPERTY_ID + "site/properties/string_prop", null);
//null value means no value specified for the property
+    List<Object> array = new ArrayList<Object>();
+    array.add("array1");
+    array.add("array2");
+    propertiesMap.put(CONFIGURATIONS_PROPERTY_ID + "site/properties/array_prop", array);
+    propertiesSet.add(propertiesMap);
+
+    doReturn(propertiesSet).when(request).getProperties();
+
+    Map<String, Map<String, Map<String, String>>> calculatedConfigurations
= provider.calculateConfigurations(request);
+
+    assertNotNull(calculatedConfigurations);
+    assertEquals(1, calculatedConfigurations.size());
+    Map<String, Map<String, String>> site = calculatedConfigurations.get("site");
+    assertNotNull(site);
+    assertEquals(1, site.size());
+    Map<String, String> properties = site.get("properties");
+    assertNotNull(properties);
+
+    assertEquals("[array1, array2]", properties.get("array_prop"));
+
+
+    // config properties with null values should be ignored
+    assertFalse(properties.containsKey("string_prop"));
+
+  }
 }


Mime
View raw message