This is an automated email from the ASF dual-hosted git repository.

adoroszlai pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/ambari.git


The following commit(s) were added to refs/heads/trunk by this push:
     new 883d95f  AMBARI-25061. Refactor AddServiceInfo to use a builder (#2756)
883d95f is described below

commit 883d95f7a93ed53e637b617acd86cd2bf9b5c3fd
Author: Doroszlai, Attila <6454655+adorosz...@users.noreply.github.com>
AuthorDate: Mon Jan 7 20:48:20 2019 +0100

    AMBARI-25061. Refactor AddServiceInfo to use a builder (#2756)
---
 .../server/topology/addservice/AddServiceInfo.java | 87 ++++++++++++++++++----
 .../topology/addservice/RequestValidator.java      | 13 +++-
 .../ProvisionActionPredicateBuilderTest.java       |  8 +-
 .../addservice/StackAdvisorAdapterTest.java        | 61 ++++++++++++---
 4 files changed, 136 insertions(+), 33 deletions(-)

diff --git 
a/ambari-server/src/main/java/org/apache/ambari/server/topology/addservice/AddServiceInfo.java
 
b/ambari-server/src/main/java/org/apache/ambari/server/topology/addservice/AddServiceInfo.java
index 1d675e5..0c0c267 100644
--- 
a/ambari-server/src/main/java/org/apache/ambari/server/topology/addservice/AddServiceInfo.java
+++ 
b/ambari-server/src/main/java/org/apache/ambari/server/topology/addservice/AddServiceInfo.java
@@ -42,18 +42,7 @@ public final class AddServiceInfo {
   private final Configuration config;
   private final LayoutRecommendationInfo recommendationInfo;
 
-  public AddServiceInfo(
-    AddServiceRequest request,
-    String clusterName,
-    RequestStageContainer stages,
-    Stack stack,
-    Configuration config,
-    Map<String, Map<String, Set<String>>> newServices
-  ) {
-    this(request, clusterName, stages, stack, config, newServices, null, null);
-  }
-
-  AddServiceInfo(
+  private AddServiceInfo(
     AddServiceRequest request,
     String clusterName,
     RequestStageContainer stages,
@@ -73,13 +62,26 @@ public final class AddServiceInfo {
     this.recommendationInfo = recommendationInfo;
   }
 
+  public Builder toBuilder() {
+    return new Builder()
+      .setRequest(request)
+      .setClusterName(clusterName)
+      .setStack(stack)
+      .setKerberosDescriptor(kerberosDescriptor)
+      .setNewServices(newServices)
+      .setStages(stages)
+      .setConfig(config)
+      .setRecommendationInfo(recommendationInfo)
+      ;
+  }
+
   public AddServiceInfo withLayoutRecommendation(Map<String, Map<String, 
Set<String>>> services,
                                                  LayoutRecommendationInfo 
recommendation) {
-    return new AddServiceInfo(request, clusterName, stages, stack, config, 
services, kerberosDescriptor, recommendation);
+    return 
toBuilder().setNewServices(services).setRecommendationInfo(recommendation).build();
   }
 
   public AddServiceInfo withConfig(Configuration newConfig) {
-    return new AddServiceInfo(request, clusterName, stages, stack, newConfig, 
newServices, kerberosDescriptor, recommendationInfo);
+    return toBuilder().setConfig(newConfig).build();
   }
 
   @Override
@@ -141,4 +143,61 @@ public final class AddServiceInfo {
     return !request.getServices().isEmpty();
   }
 
+  public static class Builder {
+
+    private AddServiceRequest request;
+    private String clusterName;
+    private Stack stack;
+    private KerberosDescriptor kerberosDescriptor;
+    private Map<String, Map<String, Set<String>>> newServices;
+    private RequestStageContainer stages;
+    private Configuration config;
+    private LayoutRecommendationInfo recommendationInfo;
+
+    public AddServiceInfo build() {
+      return new AddServiceInfo(request, clusterName, stages, stack, config, 
newServices, kerberosDescriptor, recommendationInfo);
+    }
+
+    public Builder setRequest(AddServiceRequest request) {
+      this.request = request;
+      return this;
+    }
+
+    public Builder setClusterName(String clusterName) {
+      this.clusterName = clusterName;
+      return this;
+    }
+
+    public Builder setStages(RequestStageContainer stages) {
+      this.stages = stages;
+      return this;
+    }
+
+    public Builder setStack(Stack stack) {
+      this.stack = stack;
+      return this;
+    }
+
+    public Builder setConfig(Configuration config) {
+      this.config = config;
+      return this;
+    }
+
+    public Builder setNewServices(Map<String, Map<String, Set<String>>> 
newServices) {
+      this.newServices = newServices;
+      return this;
+    }
+
+    public Builder setKerberosDescriptor(KerberosDescriptor 
kerberosDescriptor) {
+      this.kerberosDescriptor = kerberosDescriptor;
+      return this;
+    }
+
+    public Builder setRecommendationInfo(LayoutRecommendationInfo 
recommendationInfo) {
+      this.recommendationInfo = recommendationInfo;
+      return this;
+    }
+
+  }
+
 }
diff --git 
a/ambari-server/src/main/java/org/apache/ambari/server/topology/addservice/RequestValidator.java
 
b/ambari-server/src/main/java/org/apache/ambari/server/topology/addservice/RequestValidator.java
index 59302d1..5ccd3b6 100644
--- 
a/ambari-server/src/main/java/org/apache/ambari/server/topology/addservice/RequestValidator.java
+++ 
b/ambari-server/src/main/java/org/apache/ambari/server/topology/addservice/RequestValidator.java
@@ -121,10 +121,15 @@ public class RequestValidator {
     CHECK.checkState(!serviceInfoCreated.getAndSet(true), "Can create only one 
instance for each validated add service request");
 
     RequestStageContainer stages = new 
RequestStageContainer(actionManager.getNextRequestId(), null, requestFactory, 
actionManager);
-    AddServiceInfo validatedRequest = new AddServiceInfo(request, 
cluster.getClusterName(), stages,
-      state.getStack(), state.getConfig(), state.getNewServices(), 
state.getKerberosDescriptor(),
-      null
-    );
+    AddServiceInfo validatedRequest = new AddServiceInfo.Builder()
+      .setRequest(request)
+      .setClusterName(cluster.getClusterName())
+      .setStages(stages)
+      .setStack(state.getStack())
+      .setConfig(state.getConfig())
+      .setNewServices(state.getNewServices())
+      .setKerberosDescriptor(state.getKerberosDescriptor())
+      .build();
     stages.setRequestContext(validatedRequest.describe());
     return validatedRequest;
   }
diff --git 
a/ambari-server/src/test/java/org/apache/ambari/server/topology/addservice/ProvisionActionPredicateBuilderTest.java
 
b/ambari-server/src/test/java/org/apache/ambari/server/topology/addservice/ProvisionActionPredicateBuilderTest.java
index 9a3bab6..c99cbd3 100644
--- 
a/ambari-server/src/test/java/org/apache/ambari/server/topology/addservice/ProvisionActionPredicateBuilderTest.java
+++ 
b/ambari-server/src/test/java/org/apache/ambari/server/topology/addservice/ProvisionActionPredicateBuilderTest.java
@@ -57,11 +57,13 @@ public class ProvisionActionPredicateBuilderTest {
     )
   );
   private static final RequestStageContainer STAGES = new 
RequestStageContainer(42L, null, null, null, null);
+  private static final AddServiceInfo.Builder ADD_SERVICE_INFO_BUILDER = new 
AddServiceInfo.Builder()
+    
.setClusterName(CLUSTER_NAME).setStages(STAGES).setNewServices(NEW_SERVICES);
 
   @Test
   public void noCustomProvisionAction() {
     AddServiceRequest request = createRequest(null, null, null);
-    AddServiceInfo info = new AddServiceInfo(request, CLUSTER_NAME, STAGES, 
null, null, NEW_SERVICES);
+    AddServiceInfo info = ADD_SERVICE_INFO_BUILDER.setRequest(request).build();
     ProvisionActionPredicateBuilder builder = new 
ProvisionActionPredicateBuilder(info);
 
     assertTrue(builder.getPredicate(ProvisionStep.INSTALL).isPresent());
@@ -81,7 +83,7 @@ public class ProvisionActionPredicateBuilderTest {
   @Test
   public void requestLevelStartOnly() {
     AddServiceRequest request = createRequest(ProvisionAction.START_ONLY, 
null, null);
-    AddServiceInfo info = new AddServiceInfo(request, CLUSTER_NAME, STAGES, 
null, null, NEW_SERVICES);
+    AddServiceInfo info = ADD_SERVICE_INFO_BUILDER.setRequest(request).build();
     ProvisionActionPredicateBuilder builder = new 
ProvisionActionPredicateBuilder(info);
 
     assertEquals(Optional.empty(), 
builder.getPredicate(ProvisionStep.INSTALL));
@@ -115,7 +117,7 @@ public class ProvisionActionPredicateBuilderTest {
         Component.of("METRICS_MONITOR", ProvisionAction.INSTALL_ONLY, "c7404", 
"c7405") // overrides service-level
       )
     );
-    AddServiceInfo info = new AddServiceInfo(request, CLUSTER_NAME, STAGES, 
null, null, NEW_SERVICES);
+    AddServiceInfo info = ADD_SERVICE_INFO_BUILDER.setRequest(request).build();
     ProvisionActionPredicateBuilder builder = new 
ProvisionActionPredicateBuilder(info);
 
     assertTrue(builder.getPredicate(ProvisionStep.INSTALL).isPresent());
diff --git 
a/ambari-server/src/test/java/org/apache/ambari/server/topology/addservice/StackAdvisorAdapterTest.java
 
b/ambari-server/src/test/java/org/apache/ambari/server/topology/addservice/StackAdvisorAdapterTest.java
index 5219acb..bd80bda 100644
--- 
a/ambari-server/src/test/java/org/apache/ambari/server/topology/addservice/StackAdvisorAdapterTest.java
+++ 
b/ambari-server/src/test/java/org/apache/ambari/server/topology/addservice/StackAdvisorAdapterTest.java
@@ -117,6 +117,9 @@ public class StackAdvisorAdapterTest {
     .put("c7406", ImmutableSet.of("DATANODE", "HDFS_CLIENT", 
"ZOOKEEPER_CLIENT"))
     .build();
 
+  private static final AddServiceInfo.Builder ADD_SERVICE_INFO_BUILDER = new 
AddServiceInfo.Builder()
+    .setClusterName("c1");
+
   @Test
   public void getHostComponentMap() {
     assertEquals(HOST_COMPONENT_MAP, 
StackAdvisorAdapter.getHostComponentMap(COMPONENT_HOST_MAP));
@@ -178,7 +181,12 @@ public class StackAdvisorAdapterTest {
         "OOZIE_CLIENT", ImmutableSet.of("c7403", "c7404")));
 
     AddServiceRequest request = 
request(ConfigRecommendationStrategy.ALWAYS_APPLY);
-    AddServiceInfo info = new AddServiceInfo(request, "c1", null, stack, 
Configuration.newEmpty(), newServices); // No LayoutReommendationInfo -> needs 
to be calculated
+    AddServiceInfo info = ADD_SERVICE_INFO_BUILDER
+      .setRequest(request)
+      .setStack(stack)
+      .setConfig(Configuration.newEmpty())
+      .setNewServices(newServices)
+      .build(); // No LayoutReommendationInfo -> needs to be calculated
 
     LayoutRecommendationInfo layoutRecommendationInfo = 
adapter.getLayoutRecommendationInfo(info);
     layoutRecommendationInfo.getAllServiceLayouts();
@@ -360,7 +368,11 @@ public class StackAdvisorAdapterTest {
       "KAFKA",
       ImmutableMap.of("KAFKA_BROKER", emptySet()));
 
-    AddServiceInfo info = new AddServiceInfo(null, "c1", null, stack, 
org.apache.ambari.server.topology.Configuration.newEmpty(), newServices);
+    AddServiceInfo info = ADD_SERVICE_INFO_BUILDER
+      .setStack(stack)
+      .setConfig(Configuration.newEmpty())
+      .setNewServices(newServices)
+      .build();
     AddServiceInfo infoWithRecommendations = adapter.recommendLayout(info);
 
     Map<String, Map<String, Set<String>>> expectedNewLayout = ImmutableMap.of(
@@ -392,7 +404,12 @@ public class StackAdvisorAdapterTest {
     clusterConfig.setParentConfiguration(stackConfig);
 
     AddServiceRequest request = 
request(ConfigRecommendationStrategy.ALWAYS_APPLY);
-    AddServiceInfo info = new AddServiceInfo(request, "c1", null, stack, 
userConfig, newServices); // No LayoutRecommendationInfo
+    AddServiceInfo info = ADD_SERVICE_INFO_BUILDER
+      .setRequest(request)
+      .setStack(stack)
+      .setConfig(userConfig)
+      .setNewServices(newServices)
+      .build(); // No LayoutRecommendationInfo
     AddServiceInfo infoWithConfig = adapter.recommendConfigurations(info);
 
     Configuration recommendedConfig = infoWithConfig.getConfig();
@@ -442,8 +459,13 @@ public class StackAdvisorAdapterTest {
 
     LayoutRecommendationInfo layoutRecommendationInfo = new 
LayoutRecommendationInfo(new HashMap<>(), new HashMap<>()); // contents doesn't 
matter for the test
     AddServiceRequest request = 
request(ConfigRecommendationStrategy.ALWAYS_APPLY);
-    AddServiceInfo info = new AddServiceInfo(request, "c1", null, stack, 
userConfig, newServices)
-      .withLayoutRecommendation(newServices, layoutRecommendationInfo);
+    AddServiceInfo info = ADD_SERVICE_INFO_BUILDER
+      .setRequest(request)
+      .setStack(stack)
+      .setConfig(userConfig)
+      .setNewServices(newServices)
+      .setRecommendationInfo(layoutRecommendationInfo)
+      .build();
     AddServiceInfo infoWithConfig = adapter.recommendConfigurations(info);
 
     Configuration recommendedConfig = infoWithConfig.getConfig();
@@ -493,8 +515,13 @@ public class StackAdvisorAdapterTest {
 
     LayoutRecommendationInfo layoutRecommendationInfo = new 
LayoutRecommendationInfo(new HashMap<>(), new HashMap<>()); // contents doesn't 
matter for the test
     AddServiceRequest request = 
request(ConfigRecommendationStrategy.ALWAYS_APPLY_DONT_OVERRIDE_CUSTOM_VALUES);
-    AddServiceInfo info = new AddServiceInfo(request, "c1", null, stack, 
userConfig, newServices)
-      .withLayoutRecommendation(newServices, layoutRecommendationInfo);
+    AddServiceInfo info = ADD_SERVICE_INFO_BUILDER
+      .setRequest(request)
+      .setStack(stack)
+      .setConfig(userConfig)
+      .setNewServices(newServices)
+      .setRecommendationInfo(layoutRecommendationInfo)
+      .build();
     AddServiceInfo infoWithConfig = adapter.recommendConfigurations(info);
 
     assertSame(userConfig, infoWithConfig.getConfig()); // user config stays 
top priority
@@ -549,8 +576,13 @@ public class StackAdvisorAdapterTest {
 
     LayoutRecommendationInfo layoutRecommendationInfo = new 
LayoutRecommendationInfo(new HashMap<>(), new HashMap<>()); // contents doesn't 
matter for the test
     AddServiceRequest request = 
request(ConfigRecommendationStrategy.NEVER_APPLY);
-    AddServiceInfo info = new AddServiceInfo(request, "c1", null, stack, 
userConfig, newServices)
-      .withLayoutRecommendation(newServices, layoutRecommendationInfo);
+    AddServiceInfo info = ADD_SERVICE_INFO_BUILDER
+      .setRequest(request)
+      .setStack(stack)
+      .setConfig(userConfig)
+      .setNewServices(newServices)
+      .setRecommendationInfo(layoutRecommendationInfo)
+      .build();
     AddServiceInfo infoWithConfig = adapter.recommendConfigurations(info);
 
     // No recommended config, no stack config
@@ -591,8 +623,13 @@ public class StackAdvisorAdapterTest {
 
     LayoutRecommendationInfo layoutRecommendationInfo = new 
LayoutRecommendationInfo(new HashMap<>(), new HashMap<>()); // contents doesn't 
matter for the test
     AddServiceRequest request = 
request(ConfigRecommendationStrategy.ONLY_STACK_DEFAULTS_APPLY);
-    AddServiceInfo info = new AddServiceInfo(request, "c1", null, stack, 
userConfig, newServices)
-      .withLayoutRecommendation(newServices, layoutRecommendationInfo);
+    AddServiceInfo info = ADD_SERVICE_INFO_BUILDER
+      .setRequest(request)
+      .setStack(stack)
+      .setConfig(userConfig)
+      .setNewServices(newServices)
+      .setRecommendationInfo(layoutRecommendationInfo)
+      .build();
     AddServiceInfo infoWithConfig = adapter.recommendConfigurations(info);
     Configuration recommendedConfig = 
infoWithConfig.getConfig().getParentConfiguration();
 
@@ -663,4 +700,4 @@ public class StackAdvisorAdapterTest {
     return new AddServiceRequest(null, strategy, null, null, null, null, null, 
null, null, null, null);
   }
 
-}
\ No newline at end of file
+}

Reply via email to