(TK-316) Create subclasses for ClientTimer

src/java/com/puppetlabs/http/client/impl/metrics/ClientMetricFilter.java

@@ -2,8 +2,11 @@ package com.puppetlabs.http.client.impl.metrics;
 
 import com.codahale.metrics.MetricFilter;
 import com.codahale.metrics.Metric;
+import com.puppetlabs.http.client.metrics.MetricIdClientTimer;
 import com.puppetlabs.http.client.metrics.Metrics;
 import com.puppetlabs.http.client.metrics.ClientTimer;
+import com.puppetlabs.http.client.metrics.UrlAndMethodClientTimer;
+import com.puppetlabs.http.client.metrics.UrlClientTimer;
 
 import java.util.ArrayList;
 
@@ -14,6 +17,8 @@ public class ClientMetricFilter implements MetricFilter{
     private ArrayList<String> metricId;
     private Metrics.MetricType metricType;
 
+    // TODO: break this class up into two or more filter classes; it's combining a lot of logic at the moment
+
     public ClientMetricFilter(String category, Metrics.MetricType metricType) {
         this.category = category;
         this.metricType = metricType;
@@ -32,20 +37,40 @@ public class ClientMetricFilter implements MetricFilter{
         if ( metric.getMetricType().equals(metricType) ) {
             if ( category != null ) {
                 switch (category) {
+                    // TODO: we should be able to break this up into multiple methods that accept the more
+                    // concrete types in their signatures
                     case Metrics.ID_NAMESPACE:
-                        return metric.getMetricId() != null;
+                        return metric instanceof MetricIdClientTimer;
                     case Metrics.URL_METHOD_NAMESPACE:
-                        return  metric.getMethod() != null;
+                        return metric instanceof UrlAndMethodClientTimer;
                     case Metrics.URL_NAMESPACE:
-                        return metric.getUrl() != null && metric.getMethod() == null;
+                        return (metric instanceof UrlClientTimer) &&
+                                !(metric instanceof UrlAndMethodClientTimer);
                 }
             } else {
                 if ( method != null ) {
-                    return url.equals(metric.getUrl()) && method.equals(metric.getMethod()) ;
+                    // TODO: we should be able to break this up into multiple methods that accept the more
+                    if (metric instanceof UrlAndMethodClientTimer) {
+                        UrlAndMethodClientTimer urlAndMethodClientTimer = (UrlAndMethodClientTimer) metric;
+                        return url.equals(urlAndMethodClientTimer.getUrl()) && method.equals(urlAndMethodClientTimer.getMethod());
+                    } else {
+                        return false;
+                    }
                 } else if ( url != null ) {
-                    return url.equals(metric.getUrl()) && metric.getMethod() == null;
+                    if ((metric instanceof UrlClientTimer) &&
+                        !(metric instanceof UrlAndMethodClientTimer)) {
+                        UrlClientTimer urlClientTimer = (UrlClientTimer) metric;
+                        return url.equals(urlClientTimer.getUrl());
+                    } else {
+                        return false;
+                    }
                 } else {
-                    return metricId.equals(metric.getMetricId());
+                    if (metric instanceof MetricIdClientTimer) {
+                        MetricIdClientTimer metricIdClientTimer = (MetricIdClientTimer) metric;
+                        return metricId.equals(metricIdClientTimer.getMetricId());
+                    } else {
+                        return false;
+                    }
                 }
             }
         }

src/java/com/puppetlabs/http/client/impl/metrics/TimerUtils.java

@@ -4,7 +4,10 @@ import com.codahale.metrics.Metric;
 import com.codahale.metrics.MetricRegistry;
 import com.codahale.metrics.Timer;
 import com.puppetlabs.http.client.metrics.ClientTimer;
+import com.puppetlabs.http.client.metrics.MetricIdClientTimer;
 import com.puppetlabs.http.client.metrics.Metrics;
+import com.puppetlabs.http.client.metrics.UrlAndMethodClientTimer;
+import com.puppetlabs.http.client.metrics.UrlClientTimer;
 import org.apache.http.HttpRequest;
 import org.apache.http.RequestLine;
 import org.slf4j.Logger;
@@ -53,7 +56,7 @@ public class TimerUtils {
             String metric_name = MetricRegistry.name(Metrics.METRIC_NAMESPACE,
                     currentIdWithNamespace.toArray(new String[currentIdWithNamespace.size()]));
 
-            ClientTimer timer = new ClientTimer(metric_name, currentId, Metrics.MetricType.FULL_RESPONSE);
+            ClientTimer timer = new MetricIdClientTimer(metric_name, currentId, Metrics.MetricType.FULL_RESPONSE);
             timerContexts.add(getOrAddTimer(registry, metric_name, timer).time());
         }
         return timerContexts;
@@ -72,10 +75,10 @@ public class TimerUtils {
             final String urlAndMethodName = MetricRegistry.name(Metrics.METRIC_NAMESPACE, Metrics.URL_METHOD_NAMESPACE,
                     strippedUrl, method, Metrics.FULL_RESPONSE_STRING);
 
-            ClientTimer urlTimer = new ClientTimer(urlName, strippedUrl, Metrics.MetricType.FULL_RESPONSE);
+            ClientTimer urlTimer = new UrlClientTimer(urlName, strippedUrl, Metrics.MetricType.FULL_RESPONSE);
             timerContexts.add(getOrAddTimer(registry, urlName, urlTimer).time());
 
-            ClientTimer urlMethodTimer = new ClientTimer(urlAndMethodName, strippedUrl,
+            ClientTimer urlMethodTimer = new UrlAndMethodClientTimer(urlAndMethodName, strippedUrl,
                     method, Metrics.MetricType.FULL_RESPONSE);
             timerContexts.add(getOrAddTimer(registry, urlAndMethodName, urlMethodTimer).time());
         } catch (URISyntaxException e) {

src/java/com/puppetlabs/http/client/metrics/ClientMetricData.java

@@ -1,6 +1,6 @@
 package com.puppetlabs.http.client.metrics;
 
-import java.util.ArrayList;
+import java.util.List;
 
 public class ClientMetricData {
     private String metricName;
@@ -9,10 +9,10 @@ public class ClientMetricData {
     private Long aggregate;
     private String url;
     private String method;
-    private ArrayList<String> metricId;
+    private List<String> metricId;
 
     ClientMetricData(String metricName, Long count, Long mean, Long aggregate,
-                     String url, String method, ArrayList<String> metricId) {
+                     String url, String method, List<String> metricId) {
         this.metricName = metricName;
         this.count = count;
         this.mean = mean;
@@ -46,7 +46,7 @@ public class ClientMetricData {
         return method;
     }
 
-    public ArrayList<String> getMetricId() {
+    public List<String> getMetricId() {
         return metricId;
     }
 }

src/java/com/puppetlabs/http/client/metrics/ClientTimer.java

@@ -2,37 +2,14 @@ package com.puppetlabs.http.client.metrics;
 
 import com.codahale.metrics.Timer;
 
-import java.util.ArrayList;
+public abstract class ClientTimer  extends Timer {
+    private final String metricName;
 
-public class ClientTimer  extends Timer {
+    private final Metrics.MetricType metricType;
-    private String metricName;
-    private String url;
-    private String method;
-    private ArrayList<String> metricId;
 
-    private Metrics.MetricType metricType;
+    ClientTimer(String metricName, Metrics.MetricType metricType) {
-
-    public ClientTimer(String metricName, String url, Metrics.MetricType metricType) {
         super();
         this.metricName = metricName;
-        this.url = url;
-        this.metricType = metricType;
-    }
-
-    public ClientTimer(String metricName, String url, String method,
-                       Metrics.MetricType metricType) {
-        super();
-        this.metricName = metricName;
-        this.url = url;
-        this.method = method;
-        this.metricType = metricType;
-    }
-
-    public ClientTimer(String metricName, ArrayList<String> metricId,
-                       Metrics.MetricType metricType) {
-        super();
-        this.metricName = metricName;
-        this.metricId = metricId;
         this.metricType = metricType;
     }
 
@@ -40,18 +17,6 @@ public class ClientTimer  extends Timer {
         return metricName;
     }
 
-    public String getUrl() {
-        return url;
-    }
-
-    public String getMethod() {
-        return method;
-    }
-
-    public ArrayList<String> getMetricId() {
-        return metricId;
-    }
-
     public Metrics.MetricType getMetricType() {
         return metricType;
     }

src/java/com/puppetlabs/http/client/metrics/MetricIdClientTimer.java

@@ -0,0 +1,19 @@
+package com.puppetlabs.http.client.metrics;
+
+import java.util.ArrayList;
+import java.util.List;
+
+public class MetricIdClientTimer extends ClientTimer {
+
+    private final List<String> metricId;
+
+    public MetricIdClientTimer(String metricName, List<String> metricId,
+                       Metrics.MetricType metricType) {
+        super(metricName, metricType);
+        this.metricId = metricId;
+    }
+
+    public List<String> getMetricId() {
+        return metricId;
+    }
+}

src/java/com/puppetlabs/http/client/metrics/Metrics.java

@@ -9,6 +9,7 @@ import java.net.URISyntaxException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.concurrent.TimeUnit;
 
@@ -27,8 +28,8 @@ public class Metrics {
         return convertedUri.toString();
     }
 
-    public static ArrayList<ClientTimer> getClientTimerArray(Map<String, Timer> timers){
+    public static List<ClientTimer> getClientTimerArray(Map<String, Timer> timers){
-        ArrayList<ClientTimer> timerArray = new ArrayList<>();
+        List<ClientTimer> timerArray = new ArrayList<>();
         for (Map.Entry<String, Timer> entry : timers.entrySet()) {
             ClientTimer timer = (ClientTimer)entry.getValue();
             timerArray.add(timer);
@@ -36,9 +37,9 @@ public class Metrics {
         return timerArray;
     }
 
-    public static Map<String, ArrayList<ClientTimer>> getClientMetrics(MetricRegistry metricRegistry){
+    public static Map<String, List<ClientTimer>> getClientMetrics(MetricRegistry metricRegistry){
         if (metricRegistry != null) {
-            Map<String, ArrayList<ClientTimer>> timers = new HashMap<>();
+            Map<String, List<ClientTimer>> timers = new HashMap<>();
             timers.put("url", getClientTimerArray(metricRegistry.getTimers(
                     new ClientMetricFilter(URL_NAMESPACE, MetricType.FULL_RESPONSE))));
             timers.put("url-and-method", getClientTimerArray(metricRegistry.getTimers(
@@ -51,7 +52,7 @@ public class Metrics {
         }
     }
 
-    public static ArrayList<ClientTimer> getClientMetricsByUrl(MetricRegistry metricRegistry,
+    public static List<ClientTimer> getClientMetricsByUrl(MetricRegistry metricRegistry,
                                                                final String url){
         if (metricRegistry != null) {
             Map<String, Timer> timers = metricRegistry.getTimers(
@@ -62,7 +63,7 @@ public class Metrics {
         }
     }
 
-    public static ArrayList<ClientTimer> getClientMetricsByUrlAndMethod(MetricRegistry metricRegistry,
+    public static List<ClientTimer> getClientMetricsByUrlAndMethod(MetricRegistry metricRegistry,
                                                                         final String url,
                                                                         final String method){
         if (metricRegistry != null) {
@@ -74,7 +75,7 @@ public class Metrics {
         }
     }
 
-    public static ArrayList<ClientTimer> getClientMetricsByMetricId(MetricRegistry metricRegistry,
+    public static List<ClientTimer> getClientMetricsByMetricId(MetricRegistry metricRegistry,
                                                                     final String[] metricId){
         if (metricRegistry != null) {
             if (metricId.length == 0) {
@@ -93,18 +94,20 @@ public class Metrics {
         }
     }
 
-    public static ArrayList<ClientMetricData> computeClientMetricsData(ArrayList<ClientTimer> timers){
+    public static List<ClientMetricData> computeClientMetricsData(List<ClientTimer> timers){
         if (timers != null) {
-            ArrayList<ClientMetricData> metricsData = new ArrayList<>();
+            List<ClientMetricData> metricsData = new ArrayList<>();
             for (ClientTimer timer: timers) {
                 Double mean = timer.getSnapshot().getMean();
                 Long meanMillis = TimeUnit.NANOSECONDS.toMillis(mean.longValue());
                 Long count = timer.getCount();
                 Long aggregate = count * meanMillis;
                 String metricName = timer.getMetricName();
-                String url = timer.getUrl();
+                // TODO: create subclasses of ClientMetricData to prevent null values from being necessary,
-                String method = timer.getMethod();
+                // refactor into methods with types in signatures to get rid of instanceof stuff.
-                ArrayList<String> metricId = timer.getMetricId();
+                String url = timer instanceof UrlClientTimer ? ((UrlClientTimer)(timer)).getUrl() : null;
+                String method = timer instanceof UrlAndMethodClientTimer ? ((UrlAndMethodClientTimer)(timer)).getMethod() : null;
+                List<String> metricId = timer instanceof MetricIdClientTimer ? ((MetricIdClientTimer)(timer)).getMetricId() : null;
 
                 ClientMetricData data = new ClientMetricData(metricName, count, meanMillis,
                         aggregate, url, method, metricId);
@@ -116,10 +119,10 @@ public class Metrics {
         }
     }
 
-    public static Map<String, ArrayList<ClientMetricData>> getClientMetricsData(MetricRegistry metricRegistry){
+    public static Map<String, List<ClientMetricData>> getClientMetricsData(MetricRegistry metricRegistry){
         if ( metricRegistry != null ) {
-            Map<String, ArrayList<ClientTimer>> timers = getClientMetrics(metricRegistry);
+            Map<String, List<ClientTimer>> timers = getClientMetrics(metricRegistry);
-            Map<String, ArrayList<ClientMetricData>> data = new HashMap<>();
+            Map<String, List<ClientMetricData>> data = new HashMap<>();
             data.put("url", computeClientMetricsData(timers.get("url")));
             data.put("url-and-method", computeClientMetricsData(timers.get("url-and-method")));
             data.put("metric-id", computeClientMetricsData(timers.get("metric-id")));
@@ -129,22 +132,22 @@ public class Metrics {
         }
     }
 
-    public static ArrayList<ClientMetricData> getClientMetricsDataByUrl(MetricRegistry metricRegistry,
+    public static List<ClientMetricData> getClientMetricsDataByUrl(MetricRegistry metricRegistry,
                                                                         String url){
-        ArrayList<ClientTimer> timers = getClientMetricsByUrl(metricRegistry, url);
+        List<ClientTimer> timers = getClientMetricsByUrl(metricRegistry, url);
         return computeClientMetricsData(timers);
     }
 
-    public static ArrayList<ClientMetricData> getClientMetricsDataByUrlAndMethod(MetricRegistry metricRegistry,
+    public static List<ClientMetricData> getClientMetricsDataByUrlAndMethod(MetricRegistry metricRegistry,
                                                                                  String url,
                                                                                  String method){
-        ArrayList<ClientTimer> timers = getClientMetricsByUrlAndMethod(metricRegistry, url, method);
+        List<ClientTimer> timers = getClientMetricsByUrlAndMethod(metricRegistry, url, method);
         return computeClientMetricsData(timers);
     }
 
-    public static ArrayList<ClientMetricData> getClientMetricsDataByMetricId(MetricRegistry metricRegistry,
+    public static List<ClientMetricData> getClientMetricsDataByMetricId(MetricRegistry metricRegistry,
                                                                              String[] metricId){
-        ArrayList<ClientTimer> timers = getClientMetricsByMetricId(metricRegistry, metricId);
+        List<ClientTimer> timers = getClientMetricsByMetricId(metricRegistry, metricId);
         return computeClientMetricsData(timers);
     }
 }

src/java/com/puppetlabs/http/client/metrics/UrlAndMethodClientTimer.java

@@ -0,0 +1,15 @@
+package com.puppetlabs.http.client.metrics;
+
+public class UrlAndMethodClientTimer extends UrlClientTimer {
+    private final String method;
+
+    public UrlAndMethodClientTimer(String metricName, String url, String method,
+                                   Metrics.MetricType metricType) {
+        super(metricName, url, metricType);
+        this.method = method;
+    }
+
+    public String getMethod() {
+        return method;
+    }
+}

src/java/com/puppetlabs/http/client/metrics/UrlClientTimer.java

@@ -0,0 +1,14 @@
+package com.puppetlabs.http.client.metrics;
+
+public class UrlClientTimer extends ClientTimer {
+    private final String url;
+
+    public UrlClientTimer(String metricName, String url, Metrics.MetricType metricType) {
+        super(metricName, metricType);
+        this.url = url;
+    }
+
+    public String getUrl() {
+        return url;
+    }
+}