Add Strings utility library.

Update tests
ultrabrew · Dec 8, 2020 · faec88b · faec88b
1 parent da347b1
commit faec88b
Show file tree

Hide file tree

Showing 7 changed files with 109 additions and 17 deletions.
diff --git a/core/src/main/java/io/ultrabrew/metrics/util/Strings.java b/core/src/main/java/io/ultrabrew/metrics/util/Strings.java
@@ -0,0 +1,22 @@
+// Copyright 2020, Oath Inc.
+// Licensed under the terms of the Apache License 2.0 license. See LICENSE file in Ultrabrew Metrics
+// for terms.
+package io.ultrabrew.metrics.util;
+
+public class Strings {
+
+  ///CLOVER:OFF
+  private Strings() {
+    // static class
+  }
+  ///CLOVER:ON
+
+  /**
+   * Whether or not the string is empty or null.
+   * @param s A string to test.
+   * @return True if the string is empty or null, false if not.
+   */
+  public static boolean isNullOrEmpty(String s) {
+    return s == null || s.isEmpty();
+  }
+}
diff --git a/core/src/test/java/io/ultrabrew/metrics/util/StringsTest.java b/core/src/test/java/io/ultrabrew/metrics/util/StringsTest.java
@@ -0,0 +1,19 @@
+// Copyright 2020, Oath Inc.
+// Licensed under the terms of the Apache License 2.0 license. See LICENSE file in Ultrabrew Metrics
+// for terms.
+package io.ultrabrew.metrics.util;
+
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+
+import org.junit.jupiter.api.Test;
+
+public class StringsTest {
+
+  @Test
+  public void testIsNullOrEmpty() {
+    assertTrue(Strings.isNullOrEmpty(null));
+    assertTrue(Strings.isNullOrEmpty(""));
+    assertFalse(Strings.isNullOrEmpty("foo"));
+  }
+}
diff --git a/reporter-influxdb/src/main/java/io/ultrabrew/metrics/reporters/influxdb/InfluxDBClient.java b/reporter-influxdb/src/main/java/io/ultrabrew/metrics/reporters/influxdb/InfluxDBClient.java
@@ -4,6 +4,7 @@
 
 package io.ultrabrew.metrics.reporters.influxdb;
 
+import io.ultrabrew.metrics.util.Strings;
 import java.util.Arrays;
 import java.io.IOException;
 import java.net.URI;
@@ -61,20 +62,20 @@ private CloseableHttpClient getHttpClient() {
   private void doWrite(final String measurement, final String[] tags, final String[] fields,
       final long timestamp)
       throws IOException {
-    if (measurement == null || measurement.isEmpty()) {
+    if (Strings.isNullOrEmpty(measurement)) {
       LOGGER.warn("Null or empty measurement.");
       return;
     }
     int rollback = byteBuffer.position();
     byteBuffer.put(measurement.getBytes(UTF_8));
     for (int i = 0; i < tags.length; i += 2) {
-      if (tags[i] == null || tags[i].isEmpty()) {
+      if (Strings.isNullOrEmpty(tags[i])) {
         LOGGER.warn("Null or empty tag key in tags array: " 
             + Arrays.toString(tags) + " for measurement " + measurement);
         byteBuffer.position(rollback);
         return;
       }
-      if (tags[i + 1] == null || tags[i + 1].isEmpty()) {
+      if (Strings.isNullOrEmpty(tags[i + 1])) {
         LOGGER.warn("Null or empty tag value in tags array: " 
             + Arrays.toString(tags) + " for measurement " + measurement);
         byteBuffer.position(rollback);
@@ -92,13 +93,13 @@ private void doWrite(final String measurement, final String[] tags, final String
       if (!f) {
         byteBuffer.put(COMMA);
       }
-      if (fields[i] == null || fields[i].isEmpty()) {
+      if (Strings.isNullOrEmpty(fields[i])) {
         LOGGER.warn("Null or empty field name in array: " 
             + Arrays.toString(fields) + " for measurement " + measurement);
         byteBuffer.position(rollback);
         return;
       }
-      if (fields[i + 1] == null || fields[i + 1].isEmpty()) {
+      if (Strings.isNullOrEmpty(fields[i + 1])) {
         LOGGER.warn("Null or empty field value in array: " 
             + Arrays.toString(fields) + " for measurement " + measurement);
         byteBuffer.position(rollback);

diff --git a/...er-influxdb/src/test/java/io/ultrabrew/metrics/reporters/influxdb/InfluxDBClientTest.java b/...er-influxdb/src/test/java/io/ultrabrew/metrics/reporters/influxdb/InfluxDBClientTest.java
@@ -147,6 +147,22 @@ public void testWriteSplitting() throws IOException {
     }};
   }
 
+  @Test
+  public void testNullStrings() throws Exception {
+    new Expectations() {{
+
+    }};
+    client.write(null, new String[] { "host", "web01" }, new String[] { "temp", "80" }, 1534055562000000003L);
+    client.write("cpu_load_short", new String[] { null, "web01" }, new String[] { "temp", "80" }, 1534055562000000003L);
+    client.write("cpu_load_short", new String[] { "host", null }, new String[] { "temp", "80" }, 1534055562000000003L);
+    client.write("cpu_load_short", new String[] { "host", "web01" }, new String[] { null, "80" }, 1534055562000000003L);
+    client.write("cpu_load_short", new String[] { "host", "web01" }, new String[] { "temp", null }, 1534055562000000003L);
+    new Verifications() {{
+      EntityUtils.consumeQuietly((HttpEntity) any);
+      times = 0;
+    }};
+  }
+
   @Test
   public void testTooLargeMeasurement() throws IOException {
     InfluxDBClient c = new InfluxDBClient(URI.create("http://localhost:8086/write?db=test"), 10);

diff --git a/...-influxdb/src/test/java/io/ultrabrew/metrics/reporters/influxdb/InfluxDBReporterTest.java b/...-influxdb/src/test/java/io/ultrabrew/metrics/reporters/influxdb/InfluxDBReporterTest.java
@@ -64,6 +64,20 @@ public void testSetBufferSize() {
     ByteBuffer buffer = Deencapsulation.getField(c, "byteBuffer");
     assertEquals(12765, buffer.capacity());
   }
+
+  @Test
+  public void testSeEndpoint() {
+    InfluxDBReporter r = InfluxDBReporter.builder()
+        .withBaseUri(TEST_URI)
+        .withDatabase("test") // ignored
+        .withEndpoint("/my/change?db=foo")
+        .withBufferSize(12765)
+        .build();
+
+    InfluxDBClient c = Deencapsulation.getField(r, "dbClient");
+    URI uri = Deencapsulation.getField(c, "dbUri");
+    assertEquals(TEST_URI + "/my/change?db=foo", uri.toString());
+  }
 
   @Test
   public void testReporting(@Mocked CloseableHttpClient httpClient,

diff --git a/...er-opentsdb/src/main/java/io/ultrabrew/metrics/reporters/opentsdb/OpenTSDBHttpClient.java b/...er-opentsdb/src/main/java/io/ultrabrew/metrics/reporters/opentsdb/OpenTSDBHttpClient.java
@@ -5,6 +5,7 @@
 package io.ultrabrew.metrics.reporters.opentsdb;
 
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+import io.ultrabrew.metrics.util.Strings;
 import java.util.Arrays;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
@@ -75,31 +76,32 @@ private CloseableHttpClient getHttpClient() {
         .build();
   }
 
-  @SuppressFBWarnings(value="NP", justification="Null check IS performed on tags")
   void write(final String metricName,
       final String[] tags,
       final long timestamp,
       final String value) throws IOException {
     // validation
-    if (metricName == null || metricName.isEmpty()) {
+    if (Strings.isNullOrEmpty(metricName)) {
       LOG.warn("Null or empty metric name.");
       return;
     }
-    if (value == null || value.isEmpty()) {
+    if (Strings.isNullOrEmpty(value)) {
       LOG.warn("Null or empty value.");
       return;
     }
-    if (tags != null) {
-      if (tags.length % 2 != 0) {
-        LOG.warn("Uneven tag count: " + Arrays.toString(tags) + " for metric " + metricName);
+    if (tags == null) {
+      LOG.warn("At least one tag pair must be present.");
+      return;
+    }
+    if (tags.length % 2 != 0) {
+      LOG.warn("Uneven tag count: " + Arrays.toString(tags) + " for metric " + metricName);
+      return;
+    }
+    for (int i = 0; i < tags.length; i++) {
+      if (Strings.isNullOrEmpty(tags[i])) {
+        LOG.warn("Null tag key or value in: " + Arrays.toString(tags) + " for metric " + metricName);
         return;
       }
-      for (int i = 0; i < tags.length; i++) {
-        if (tags[i] == null || tags[i].length() < 1) {
-          LOG.warn("Null tag key or value in: " + Arrays.toString(tags) + " for metric " + metricName);
-          return;
-        }
-      }
     }
 
     if (currentBatchSize++ > 0) {

diff --git a/...pentsdb/src/test/java/io/ultrabrew/metrics/reporters/opentsdb/OpenTSDBHttpClientTest.java b/...pentsdb/src/test/java/io/ultrabrew/metrics/reporters/opentsdb/OpenTSDBHttpClientTest.java
@@ -11,6 +11,7 @@
 import java.nio.charset.StandardCharsets;
 import mockit.Expectations;
 import mockit.Mocked;
+import mockit.Verifications;
 import org.apache.http.StatusLine;
 import org.apache.http.client.methods.CloseableHttpResponse;
 import org.apache.http.client.methods.HttpUriRequest;
@@ -86,6 +87,23 @@ public void testWriteFailsManualFlush() throws Exception {
     assertEquals(0, client.currentBatchSize);
   }
 
+  @Test
+  public void testNullStringsAndTagsValidation() throws Exception {
+    new Expectations() {{
+
+    }};
+    OpenTSDBHttpClient client = new OpenTSDBHttpClient(DUMMY_DB_URI, 64, true);
+    client.write(null, new String[] { "host", "web01" }, 1534055562000000003L, "80");
+    client.write("cpu_load_short.temp", new String[] { null, "web01" }, 1534055562000000003L, "80");
+    client.write("cpu_load_short.temp", new String[] { "host", null }, 1534055562000000003L, "80");
+    client.write("cpu_load_short.temp", new String[] { "host", "web01" }, 1534055562000000003L, null);
+    client.write("cpu_load_short.temp", null, 1534055562000000003L, "80");
+    client.write("cpu_load_short.temp", new String[] { "host" }, 1534055562000000003L, "80");
+    new Verifications() {{
+      times = 0;
+    }};
+  }
+
   @Test
   public void testEscapeString() throws Exception {
     OpenTSDBHttpClient c = new OpenTSDBHttpClient(DUMMY_DB_URI, 64, false);