Merge pull request #2771 from gianm/extractionfn-stuff

Various ExtractionFn null handling fixes.

processing/src/main/java/io/druid/query/extraction/ExtractionFn.java

@@ -59,7 +59,7 @@ public interface ExtractionFn
   public byte[] getCacheKey();
 
   /**
-   * The "extraction" function.  This should map a value into some other String value.
+   * The "extraction" function.  This should map an Object into some String value.
    * <p>
    * In order to maintain the "null and empty string are equivalent" semantics that Druid provides, the
    * empty string is considered invalid output for this method and should instead return null.  This is
@@ -72,8 +72,28 @@ public interface ExtractionFn
    */
   public String apply(Object value);
 
+  /**
+   * The "extraction" function.  This should map a String value into some other String value.
+   * <p>
+   * Like {@link #apply(Object)}, the empty string is considered invalid output for this method and it should
+   * instead return null.
+   *
+   * @param value the original value of the dimension
+   *
+   * @return a value that should be used instead of the original
+   */
   public String apply(String value);
 
+  /**
+   * The "extraction" function.  This should map a long value into some String value.
+   * <p>
+   * Like {@link #apply(Object)}, the empty string is considered invalid output for this method and it should
+   * instead return null.
+   *
+   * @param value the original value of the dimension
+   *
+   * @return a value that should be used instead of the original
+   */
   public String apply(long value);
 
   /**

processing/src/main/java/io/druid/query/extraction/JavaScriptExtractionFn.java

@@ -109,7 +109,7 @@ public class JavaScriptExtractionFn implements ExtractionFn
   @Override
   public String apply(String value)
   {
-    return this.apply((Object) value);
+    return this.apply((Object) Strings.emptyToNull(value));
   }
 
   @Override

processing/src/main/java/io/druid/query/extraction/LowerExtractionFn.java

@@ -31,7 +31,7 @@ import java.nio.ByteBuffer;
 import java.util.Locale;
 
 @JsonTypeName("lower")
-public class LowerExtractionFn implements ExtractionFn
+public class LowerExtractionFn extends DimExtractionFn
 {
   private final Locale locale;
 
@@ -60,12 +60,6 @@ public class LowerExtractionFn implements ExtractionFn
     return key.toLowerCase(locale);
   }
 
-  @Override
-  public String apply(long value)
-  {
-    return apply(String.valueOf(value));
-  }
-
   @Override
   public boolean preservesOrdering()
   {
@@ -88,10 +82,4 @@ public class LowerExtractionFn implements ExtractionFn
                      .put(localeBytes)
                      .array();
   }
-
-  @Override
-  public String apply(Object value)
-  {
-    return apply(String.valueOf(value));
-  }
 }

processing/src/main/java/io/druid/query/extraction/MatchingDimExtractionFn.java

@@ -22,6 +22,7 @@ package io.druid.query.extraction;
 import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.annotation.JsonProperty;
 import com.google.common.base.Preconditions;
+import com.google.common.base.Strings;
 import com.metamx.common.StringUtils;
 
 import java.nio.ByteBuffer;
@@ -59,9 +60,8 @@ public class MatchingDimExtractionFn extends DimExtractionFn
   @Override
   public String apply(String dimValue)
   {
-    dimValue = (dimValue == null) ? "" : dimValue;
+    Matcher matcher = pattern.matcher(Strings.nullToEmpty(dimValue));
-    Matcher matcher = pattern.matcher(dimValue);
+    return matcher.find() ? Strings.emptyToNull(dimValue) : null;
-    return matcher.find() ? dimValue : null;
   }
 
   @JsonProperty("expr")

processing/src/main/java/io/druid/query/extraction/RegexDimExtractionFn.java

@@ -86,11 +86,8 @@ public class RegexDimExtractionFn extends DimExtractionFn
   @Override
   public String apply(String dimValue)
   {
-    if (dimValue == null) {
+    final String retVal;
-      return null;
+    final Matcher matcher = pattern.matcher(Strings.nullToEmpty(dimValue));
-    }
-    String retVal;
-    Matcher matcher = pattern.matcher(dimValue);
     if (matcher.find()) {
       retVal = matcher.group(1);
     } else {

processing/src/main/java/io/druid/query/extraction/SearchQuerySpecDimExtractionFn.java

@@ -22,6 +22,7 @@ package io.druid.query.extraction;
 import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.annotation.JsonProperty;
 import com.google.common.base.Preconditions;
+import com.google.common.base.Strings;
 import io.druid.query.search.search.SearchQuerySpec;
 
 import java.nio.ByteBuffer;
@@ -61,7 +62,7 @@ public class SearchQuerySpecDimExtractionFn extends DimExtractionFn
   @Override
   public String apply(String dimValue)
   {
-    return searchQuerySpec.accept(dimValue) ? dimValue : null;
+    return searchQuerySpec.accept(dimValue) ? Strings.emptyToNull(dimValue) : null;
   }
 
   @Override

processing/src/main/java/io/druid/query/extraction/UpperExtractionFn.java

@@ -31,7 +31,7 @@ import java.nio.ByteBuffer;
 import java.util.Locale;
 
 @JsonTypeName("upper")
-public class UpperExtractionFn implements ExtractionFn
+public class UpperExtractionFn extends DimExtractionFn
 {
   private final Locale locale;
 
@@ -59,12 +59,6 @@ public class UpperExtractionFn implements ExtractionFn
     return key.toUpperCase(locale);
   }
 
-  @Override
-  public String apply(long value)
-  {
-    return apply(String.valueOf(value));
-  }
-
   @Override
   public boolean preservesOrdering()
   {
@@ -87,10 +81,4 @@ public class UpperExtractionFn implements ExtractionFn
                      .put(localeBytes)
                      .array();
   }
-
-  @Override
-  public String apply(Object value)
-  {
-    return apply(String.valueOf(value));
-  }
 }

processing/src/test/java/io/druid/query/extraction/LowerExtractionFnTest.java

@@ -35,6 +35,8 @@ public class LowerExtractionFnTest
     Assert.assertEquals("lower 1 string", extractionFn.apply("lOwER 1 String"));
     Assert.assertEquals(null,  extractionFn.apply(""));
     Assert.assertEquals(null, extractionFn.apply(null));
+    Assert.assertEquals(null, extractionFn.apply((Object)null));
+    Assert.assertEquals("1", extractionFn.apply(1));
   }
 
   @Test

processing/src/test/java/io/druid/query/extraction/MatchingDimExtractionFnTest.java

@@ -67,6 +67,17 @@ public class MatchingDimExtractionFnTest
     }
   }
 
+  @Test
+  public void testNullExtraction()
+  {
+    String regex = "^$";
+    ExtractionFn extractionFn = new MatchingDimExtractionFn(regex);
+
+    Assert.assertNull(extractionFn.apply((Object) null));
+    Assert.assertNull(extractionFn.apply((String) null));
+    Assert.assertNull(extractionFn.apply((String) ""));
+  }
+
   @Test
   public void testSerde() throws Exception
   {

processing/src/test/java/io/druid/query/extraction/RegexDimExtractionFnTest.java

@@ -102,7 +102,6 @@ public class RegexDimExtractionFnTest
     Assert.assertEquals(expected, extracted);
   }
 
-
   @Test
   public void testNullAndEmpty()
   {
@@ -116,6 +115,26 @@ public class RegexDimExtractionFnTest
     Assert.assertEquals(null, extractionFn.apply("/a/b"));
   }
 
+  @Test
+  public void testMissingValueReplacementFromNullAndEmpty()
+  {
+    String regex = "(bob)";
+    ExtractionFn extractionFn = new RegexDimExtractionFn(regex, true, "NO MATCH");
+    Assert.assertEquals("NO MATCH", extractionFn.apply(""));
+    Assert.assertEquals("NO MATCH", extractionFn.apply(null));
+  }
+
+  @Test
+  public void testMissingValueReplacementToEmpty()
+  {
+    String regex = "(bob)";
+    ExtractionFn extractionFn = new RegexDimExtractionFn(regex, true, "");
+    Assert.assertEquals(null, extractionFn.apply(null));
+    Assert.assertEquals(null, extractionFn.apply(""));
+    Assert.assertEquals(null, extractionFn.apply("abc"));
+    Assert.assertEquals(null, extractionFn.apply("123"));
+  }
+
   @Test
   public void testMissingValueReplacement()
   {

processing/src/test/java/io/druid/query/extraction/UpperExtractionFnTest.java

@@ -35,6 +35,8 @@ public class UpperExtractionFnTest
     Assert.assertEquals("UPPER", extractionFn.apply("uPpeR"));
     Assert.assertEquals(null,  extractionFn.apply(""));
     Assert.assertEquals(null, extractionFn.apply(null));
+    Assert.assertEquals(null, extractionFn.apply((Object)null));
+    Assert.assertEquals("1", extractionFn.apply(1));
   }
 
   @Test