This is an automated email from the ASF dual-hosted git repository. imaxon pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/asterixdb.git
commit d5de9eb65fa935076d0bb82094d3fdf5357ea704 Author: Hussain Towaileb <[email protected]> AuthorDate: Tue May 11 17:09:05 2021 +0300 [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 <[email protected]> Tested-by: Jenkins <[email protected]> Reviewed-by: Michael Blow <[email protected]> --- .../java/org/apache/hyracks/util/StorageUtil.java | 53 ++++++--------- .../org/apache/hyracks/util/StorageUnitTest.java | 79 ++++++++++++++++++++++ 2 files changed, 99 insertions(+), 33 deletions(-) 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 @@ package org.apache.hyracks.util; 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 @@ public class StorageUtil { * @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); + + // Default to bytes or find provided unit + StorageUnit unitType = !unit.isEmpty() ? StorageUnit.lookupBySuffix(unit) : StorageUnit.BYTE; + if (unitType == null) { + throw invalidFormatException(s); + } - // Return the bytes. - return unitType.toBytes(Double.parseDouble(sFinalVal)); + 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")); + } + } +}
