[incubator-pinot] branch master updated: In TimeUtils.timeUnitFromString(), return null if input is null or empty (#4088)

[ja...@apache.org]

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

jackie pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git


The following commit(s) were added to refs/heads/master by this push:
     new e115bc9  In TimeUtils.timeUnitFromString(), return null if input is null or empty (#4088)
e115bc9 is described below

commit e115bc9f34f78fdd2d4e519273fbd82a3e1c35bd
Author: Xiaotian (Jackie) Jiang <17...@users.noreply.github.com>
AuthorDate: Sun Apr 7 15:07:59 2019 -0700

    In TimeUtils.timeUnitFromString(), return null if input is null or empty (#4088)
    
    For backward compatible
---
 .../common/config/SegmentsValidationAndRetentionConfig.java   |  2 +-
 .../java/org/apache/pinot/common/utils/time/TimeUtils.java    | 11 +++++++++--
 .../test/java/org/apache/pinot/common/utils/UtilsTest.java    | 10 ++++++++++
 .../apache/pinot/core/segment/index/SegmentMetadataImpl.java  |  1 +
 4 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/pinot-common/src/main/java/org/apache/pinot/common/config/SegmentsValidationAndRetentionConfig.java b/pinot-common/src/main/java/org/apache/pinot/common/config/SegmentsValidationAndRetentionConfig.java
index 9d244c6..e9eeef0 100644
--- a/pinot-common/src/main/java/org/apache/pinot/common/config/SegmentsValidationAndRetentionConfig.java
+++ b/pinot-common/src/main/java/org/apache/pinot/common/config/SegmentsValidationAndRetentionConfig.java
@@ -93,7 +93,7 @@ public class SegmentsValidationAndRetentionConfig {
   }
 
   public void setTimeType(String timeType) {
-    _timeType = timeType != null ? TimeUtils.timeUnitFromString(timeType) : null;
+    _timeType = TimeUtils.timeUnitFromString(timeType);
   }
 
   public String getRetentionTimeUnit() {
diff --git a/pinot-common/src/main/java/org/apache/pinot/common/utils/time/TimeUtils.java b/pinot-common/src/main/java/org/apache/pinot/common/utils/time/TimeUtils.java
index 6a89292..f693d65 100644
--- a/pinot-common/src/main/java/org/apache/pinot/common/utils/time/TimeUtils.java
+++ b/pinot-common/src/main/java/org/apache/pinot/common/utils/time/TimeUtils.java
@@ -21,6 +21,7 @@ package org.apache.pinot.common.utils.time;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.concurrent.TimeUnit;
+import javax.annotation.Nullable;
 import org.joda.time.DateTime;
 import org.joda.time.DateTimeZone;
 import org.joda.time.Duration;
@@ -47,7 +48,8 @@ public class TimeUtils {
   private static final long VALID_MAX_TIME_MILLIS = new DateTime(2071, 1, 1, 0, 0, 0, 0, DateTimeZone.UTC).getMillis();
 
   /**
-   * Converts a time unit string into {@link TimeUnit}, ignoring case.
+   * Converts a time unit string into {@link TimeUnit}, ignoring case. For {@code null} or empty time unit string,
+   * returns {@code null}.
    * <p>Supports the following legacy time unit strings:
    * <ul>
    *   <li>"daysSinceEpoch" -> DAYS</li>
@@ -59,7 +61,12 @@ public class TimeUtils {
    * @param timeUnitString The time unit string to convert, e.g. "DAYS" or "SECONDS"
    * @return The corresponding {@link TimeUnit}
    */
-  public static TimeUnit timeUnitFromString(String timeUnitString) {
+  @Nullable
+  public static TimeUnit timeUnitFromString(@Nullable String timeUnitString) {
+    // NOTE: for backward-compatibility, return null if time unit string is null or empty
+    if (timeUnitString == null || timeUnitString.isEmpty()) {
+      return null;
+    }
     String upperCaseTimeUnitString = timeUnitString.toUpperCase();
     TimeUnit timeUnit = TIME_UNIT_MAP.get(upperCaseTimeUnitString);
     if (timeUnit != null) {
diff --git a/pinot-common/src/test/java/org/apache/pinot/common/utils/UtilsTest.java b/pinot-common/src/test/java/org/apache/pinot/common/utils/UtilsTest.java
index 8b4ce21..10b4cff 100644
--- a/pinot-common/src/test/java/org/apache/pinot/common/utils/UtilsTest.java
+++ b/pinot-common/src/test/java/org/apache/pinot/common/utils/UtilsTest.java
@@ -25,6 +25,8 @@ import org.testng.Assert;
 import org.testng.annotations.Test;
 
 import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertNull;
+import static org.testng.Assert.fail;
 
 
 /**
@@ -49,6 +51,14 @@ public class UtilsTest {
     assertEquals(TimeUtils.timeUnitFromString("HOURSSINCEEPOCH"), TimeUnit.HOURS);
     assertEquals(TimeUtils.timeUnitFromString("MinutesSinceEpoch"), TimeUnit.MINUTES);
     assertEquals(TimeUtils.timeUnitFromString("SeCoNdSsInCeEpOcH"), TimeUnit.SECONDS);
+    assertNull(TimeUtils.timeUnitFromString(null));
+    assertNull(TimeUtils.timeUnitFromString(""));
+    try {
+      TimeUtils.timeUnitFromString("unknown");
+      fail();
+    } catch (Exception e) {
+      // Expected
+    }
   }
 
   @Test
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/segment/index/SegmentMetadataImpl.java b/pinot-core/src/main/java/org/apache/pinot/core/segment/index/SegmentMetadataImpl.java
index 71e39b7..d731e2f 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/segment/index/SegmentMetadataImpl.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/segment/index/SegmentMetadataImpl.java
@@ -180,6 +180,7 @@ public class SegmentMetadataImpl implements SegmentMetadata {
         .containsKey(SEGMENT_END_TIME) && segmentMetadataPropertiesConfiguration.containsKey(TIME_UNIT)) {
       try {
         _timeUnit = TimeUtils.timeUnitFromString(segmentMetadataPropertiesConfiguration.getString(TIME_UNIT));
+        assert _timeUnit != null;
         _timeGranularity = new Duration(_timeUnit.toMillis(1));
         String startTimeString = segmentMetadataPropertiesConfiguration.getString(SEGMENT_START_TIME);
         String endTimeString = segmentMetadataPropertiesConfiguration.getString(SEGMENT_END_TIME);


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org