[NO ISSUE] Fixed storage unit util false positives + added tests

Change-Id: Ic263c4fea0de15803811c88e4d6b01df21e083c8
Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/11303
Integration-Tests: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Tested-by: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Reviewed-by: Michael Blow <mblow@apache.org>
diff --git a/hyracks-fullstack/hyracks/hyracks-util/src/main/java/org/apache/hyracks/util/StorageUtil.java b/hyracks-fullstack/hyracks/hyracks-util/src/main/java/org/apache/hyracks/util/StorageUtil.java
index e4969f0..0456dc7 100644
--- a/hyracks-fullstack/hyracks/hyracks-util/src/main/java/org/apache/hyracks/util/StorageUtil.java
+++ b/hyracks-fullstack/hyracks/hyracks-util/src/main/java/org/apache/hyracks/util/StorageUtil.java
@@ -20,10 +20,13 @@
 
 import java.util.HashMap;
 import java.util.Map;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 public class StorageUtil {
 
     public static final int BASE = 1024;
+    private static final Pattern PATTERN = Pattern.compile("^(-?[.0-9]+)([A-Z]{0,2})$");
 
     public enum StorageUnit {
         BYTE("B", "b", 1),
@@ -96,43 +99,27 @@
      * @throws IllegalArgumentException
      */
     public static double getSizeInBytes(String s) {
-        String sSpaceRemoved = s.replaceAll(" ", "");
-        String sUpper = sSpaceRemoved.toUpperCase();
-
-        // Default type
-        StorageUtil.StorageUnit unitType;
-
-        // If the length is 1, it should only contain a digit number.
-        if (sUpper.length() == 1) {
-            if (Character.isDigit(sUpper.charAt(0))) {
-                unitType = StorageUnit.BYTE;
-            } else {
-                throw invalidFormatException(s);
-            }
-        } else if (sUpper.length() > 1) {
-            String checkStr = sUpper.substring(sUpper.length() - 2);
-            unitType = StorageUnit.lookupBySuffix(checkStr);
-
-            if (unitType == null) {
-                // The last suffix should be at least "B" or a digit to be qualified as byte unit string.
-                char lastChar = sUpper.charAt(sUpper.length() - 1);
-                if (sUpper.substring(sUpper.length() - 1).equals(StorageUnit.BYTE.toString())
-                        || Character.isDigit(lastChar)) {
-                    unitType = StorageUnit.BYTE;
-                } else {
-                    throw invalidFormatException(s);
-                }
-            }
-        } else {
-            // String length is zero. We can't parse this string.
+        String valueAndUnit = s.replace(" ", "").toUpperCase();
+        Matcher matcher = PATTERN.matcher(valueAndUnit);
+        if (!matcher.find()) {
             throw invalidFormatException(s);
         }
 
-        // Strip all unit suffixes such as KB, MB ...
-        String sFinalVal = sUpper.replaceAll("[^-\\.0123456789]", "");
+        String value = matcher.group(1);
+        String unit = matcher.group(2);
 
-        // Return the bytes.
-        return unitType.toBytes(Double.parseDouble(sFinalVal));
+        // Default to bytes or find provided unit
+        StorageUnit unitType = !unit.isEmpty() ? StorageUnit.lookupBySuffix(unit) : StorageUnit.BYTE;
+        if (unitType == null) {
+            throw invalidFormatException(s);
+        }
+
+        try {
+            // Return the bytes.
+            return unitType.toBytes(Double.parseDouble(value));
+        } catch (NumberFormatException ex) {
+            throw invalidFormatException(s);
+        }
     }
 
     private static IllegalArgumentException invalidFormatException(String s) {
diff --git a/hyracks-fullstack/hyracks/hyracks-util/src/test/java/org/apache/hyracks/util/StorageUnitTest.java b/hyracks-fullstack/hyracks/hyracks-util/src/test/java/org/apache/hyracks/util/StorageUnitTest.java
new file mode 100644
index 0000000..445d15f
--- /dev/null
+++ b/hyracks-fullstack/hyracks/hyracks-util/src/test/java/org/apache/hyracks/util/StorageUnitTest.java
@@ -0,0 +1,79 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.hyracks.util;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class StorageUnitTest {
+
+    @Test
+    public void test() {
+        // Valid cases
+        double result1NoUnit = StorageUtil.getSizeInBytes("1"); // Defaults to bytes
+        Assert.assertEquals(1.0, result1NoUnit, 0);
+
+        double result1B = StorageUtil.getSizeInBytes("1B");
+        Assert.assertEquals(1.0, result1B, 0);
+
+        double result1BWithSpaces = StorageUtil.getSizeInBytes("1 B ");
+        Assert.assertEquals(1.0, result1BWithSpaces, 0);
+
+        double result1Kb = StorageUtil.getSizeInBytes("1KB");
+        Assert.assertEquals(1024.0, result1Kb, 0);
+
+        double result1KbWithSpaces = StorageUtil.getSizeInBytes(" 1 K B ");
+        Assert.assertEquals(1024.0, result1KbWithSpaces, 0);
+
+        double resultPoint5KB = StorageUtil.getSizeInBytes(".5KB");
+        Assert.assertEquals(512.0, resultPoint5KB, 0);
+
+        double resultPoint5SmallKB = StorageUtil.getSizeInBytes(".5kB");
+        Assert.assertEquals(512.0, resultPoint5SmallKB, 0);
+
+        double result1Mb = StorageUtil.getSizeInBytes("1MB");
+        Assert.assertEquals(1024.0 * 1024.0, result1Mb, 0);
+
+        double result1Point0Mb = StorageUtil.getSizeInBytes("1.0MB");
+        Assert.assertEquals(1024.0 * 1024.0, result1Point0Mb, 0);
+
+        double result01Point0Mb = StorageUtil.getSizeInBytes("01.0MB");
+        Assert.assertEquals(1024.0 * 1024.0, result01Point0Mb, 0);
+
+        // Invalid cases
+        invalidCase("");
+        invalidCase("99999999999999999999999999999999999999999999999999999999999999999999999999999999999999999");
+        invalidCase("32MB123");
+        invalidCase("1.1.1");
+        invalidCase("12KBMB");
+        invalidCase("MB");
+        invalidCase("1AB");
+        invalidCase("MB1MB");
+        invalidCase("123MBB");
+    }
+
+    private void invalidCase(String value) {
+        try {
+            StorageUtil.getSizeInBytes(value);
+        } catch (Exception ex) {
+            Assert.assertTrue(ex.toString()
+                    .contains("IllegalArgumentException: The given string: " + value + " is not a byte unit string"));
+        }
+    }
+}