Github suggestions: Use static factory methods as constructors for Rule and Selector classes

Author: mtoffl01

internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java

@@ -25,13 +25,7 @@ public Rule(List<Selector> selectors, Map<String, Object> configuration) {
     this.configuration = configuration;
   }
 
-  public Rule(Object yaml) {
-    if (!(yaml instanceof Map)) {
-      throw new StableConfigMappingException(
-          "Rule must be a map, but got: " + yaml.getClass().getSimpleName());
-    }
-    Map map = (Map) yaml;
-
+  public static Rule from(Map<?, ?> map) {
     Object selectorsObj = map.get("selectors");
     if (selectorsObj == null) {
       throw new StableConfigMappingException("Missing 'selectors' in rule: " + map);
@@ -40,10 +34,20 @@ public Rule(Object yaml) {
       throw new StableConfigMappingException(
           "'selectors' must be a list, but got: " + selectorsObj.getClass().getSimpleName());
     }
-    selectors =
+    List<Selector> selectors =
         unmodifiableList(
-            ((List<Object>) selectorsObj)
-                .stream().filter(Objects::nonNull).map(Selector::new).collect(toList()));
+            ((List<?>) selectorsObj)
+                .stream()
+                    .filter(Objects::nonNull)
+                    .map(
+                        s -> {
+                          if (!(s instanceof Map)) {
+                            throw new StableConfigMappingException(
+                                "Selector must be a map, but got: " + s.getClass().getSimpleName());
+                          }
+                          return Selector.from((Map<?, ?>) s);
+                        })
+                    .collect(toList()));
 
     Object configObj = map.get("configuration");
     if (configObj == null) {
@@ -53,7 +57,9 @@ public Rule(Object yaml) {
       throw new StableConfigMappingException(
           "'configuration' must be a map, but got: " + configObj.getClass().getSimpleName());
     }
-    configuration = unmodifiableMap((Map<String, Object>) configObj);
+    Map<String, Object> configuration = (Map<String, Object>) configObj;
+
+    return new Rule(selectors, configuration);
   }
 
   public List<Selector> getSelectors() {

internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Selector.java

@@ -17,13 +17,7 @@ public Selector(String origin, String key, List<String> matches, String operator
     this.operator = operator;
   }
 
-  public Selector(Object yaml) {
-    if (!(yaml instanceof Map)) {
-      throw new StableConfigMappingException(
-          "Selector must be a map, but got: " + yaml.getClass().getSimpleName());
-    }
-    Map map = (Map) yaml;
-
+  public static Selector from(Map<?, ?> map) {
     Object originObj = map.get("origin");
     if (originObj == null) {
       throw new StableConfigMappingException("Missing 'origin' in selector: " + map);
@@ -32,18 +26,18 @@ public Selector(Object yaml) {
       throw new StableConfigMappingException(
           "'origin' must be a string, but got: " + originObj.getClass().getSimpleName());
     }
-    origin = (String) originObj;
+    String origin = (String) originObj;
 
     Object keyObj = map.get("key");
-    key = (keyObj instanceof String) ? (String) keyObj : null;
+    String key = (keyObj instanceof String) ? (String) keyObj : null;
 
     Object matchesObj = map.get("matches");
     if (matchesObj != null && !(matchesObj instanceof List)) {
       throw new StableConfigMappingException(
           "'matches' must be a list, but got: " + matchesObj.getClass().getSimpleName());
     }
     List<String> rawMatches = (List<String>) matchesObj;
-    matches =
+    List<String> matches =
         rawMatches != null ? Collections.unmodifiableList(rawMatches) : Collections.emptyList();
 
     Object operatorObj = map.get("operator");
@@ -54,7 +48,9 @@ public Selector(Object yaml) {
       throw new StableConfigMappingException(
           "'operator' must be a string, but got: " + operatorObj.getClass().getSimpleName());
     }
-    operator = (String) operatorObj;
+    String operator = (String) operatorObj;
+
+    return new Selector(origin, key, matches, operator);
   }
 
   public String getOrigin() {

internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfig.java

@@ -4,7 +4,6 @@
 import static java.util.Collections.emptyMap;
 import static java.util.Collections.unmodifiableList;
 import static java.util.Collections.unmodifiableMap;
-import static java.util.stream.Collectors.toList;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -21,10 +20,7 @@ public StableConfig(Object yaml) {
     this.apmConfigurationDefault =
         unmodifiableMap(
             (Map<String, Object>) map.getOrDefault("apm_configuration_default", emptyMap()));
-    this.apmConfigurationRules =
-        unmodifiableList(
-            ((List<Object>) map.getOrDefault("apm_configuration_rules", emptyList()))
-                .stream().map(Rule::new).collect(toList()));
+    this.apmConfigurationRules = parseRules(map);
   }
 
   // test only
@@ -45,4 +41,25 @@ public Map<String, Object> getApmConfigurationDefault() {
   public List<Rule> getApmConfigurationRules() {
     return apmConfigurationRules;
   }
+
+  private List<Rule> parseRules(Map<?, ?> map) {
+    Object rulesObj = map.get("apm_configuration_rules");
+    if (rulesObj instanceof List) {
+      List<?> rulesList = (List<?>) rulesObj;
+      List<Rule> rules = new ArrayList<>();
+      for (Object ruleObj : rulesList) {
+        if (ruleObj instanceof Map) {
+          rules.add(Rule.from((Map<?, ?>) ruleObj));
+        } else {
+          // Log or throw with a detailed message
+          throw new StableConfigMappingException(
+              "Rule must be a map, but got: "
+                  + (ruleObj == null ? "null" : ruleObj.getClass().getSimpleName()));
+        }
+      }
+      return unmodifiableList(rules);
+    } else {
+      return emptyList();
+    }
+  }
 }