From 1d4ecd3e4c078e67a6a04c1ffda48d635cf66fb7 Mon Sep 17 00:00:00 2001 From: Andy LoPresto Date: Thu, 29 Nov 2018 20:17:58 -0800 Subject: [PATCH 1/9] NIFI-5854 Added skeleton logic to convert decimal time units. Added helper methods. Added unit tests. --- .../org/apache/nifi/util/FormatUtils.java | 222 ++++++++++- .../processor/TestFormatUtilsGroovy.groovy | 130 ------- .../nifi/util/TestFormatUtilsGroovy.groovy | 351 ++++++++++++++++++ 3 files changed, 566 insertions(+), 137 deletions(-) delete mode 100644 nifi-commons/nifi-utils/src/test/groovy/org/apache/nifi/processor/TestFormatUtilsGroovy.groovy create mode 100644 nifi-commons/nifi-utils/src/test/groovy/org/apache/nifi/util/TestFormatUtilsGroovy.groovy diff --git a/nifi-commons/nifi-utils/src/main/java/org/apache/nifi/util/FormatUtils.java b/nifi-commons/nifi-utils/src/main/java/org/apache/nifi/util/FormatUtils.java index 1c9140b7d97d..18658ce70f2a 100644 --- a/nifi-commons/nifi-utils/src/main/java/org/apache/nifi/util/FormatUtils.java +++ b/nifi-commons/nifi-utils/src/main/java/org/apache/nifi/util/FormatUtils.java @@ -17,12 +17,13 @@ package org.apache.nifi.util; import java.text.NumberFormat; +import java.util.Arrays; +import java.util.List; import java.util.concurrent.TimeUnit; import java.util.regex.Matcher; import java.util.regex.Pattern; public class FormatUtils { - private static final String UNION = "|"; // for Data Sizes @@ -41,7 +42,7 @@ public class FormatUtils { private static final String WEEKS = join(UNION, "w", "wk", "wks", "week", "weeks"); private static final String VALID_TIME_UNITS = join(UNION, NANOS, MILLIS, SECS, MINS, HOURS, DAYS, WEEKS); - public static final String TIME_DURATION_REGEX = "(\\d+)\\s*(" + VALID_TIME_UNITS + ")"; + public static final String TIME_DURATION_REGEX = "([\\d.]+)\\s*(" + VALID_TIME_UNITS + ")"; public static final Pattern TIME_DURATION_PATTERN = Pattern.compile(TIME_DURATION_REGEX); /** @@ -58,7 +59,7 @@ public static String formatCount(final long count) { * Formats the specified duration in 'mm:ss.SSS' format. * * @param sourceDuration the duration to format - * @param sourceUnit the unit to interpret the duration + * @param sourceUnit the unit to interpret the duration * @return representation of the given time data in minutes/seconds */ public static String formatMinutesSeconds(final long sourceDuration, final TimeUnit sourceUnit) { @@ -79,7 +80,7 @@ public static String formatMinutesSeconds(final long sourceDuration, final TimeU * Formats the specified duration in 'HH:mm:ss.SSS' format. * * @param sourceDuration the duration to format - * @param sourceUnit the unit to interpret the duration + * @param sourceUnit the unit to interpret the duration * @return representation of the given time data in hours/minutes/seconds */ public static String formatHoursMinutesSeconds(final long sourceDuration, final TimeUnit sourceUnit) { @@ -142,7 +143,7 @@ public static String formatDataSize(final double dataSize) { public static long getTimeDuration(final String value, final TimeUnit desiredUnit) { final Matcher matcher = TIME_DURATION_PATTERN.matcher(value.toLowerCase()); if (!matcher.matches()) { - throw new IllegalArgumentException("Value '" + value + "' is not a valid Time Duration"); + throw new IllegalArgumentException("Value '" + value + "' is not a valid time duration"); } final String duration = matcher.group(1); @@ -193,13 +194,220 @@ public static long getTimeDuration(final String value, final TimeUnit desiredUni case "week": case "weeks": final long durationVal = Long.parseLong(duration); - return desiredUnit.convert(durationVal, TimeUnit.DAYS)*7; + return desiredUnit.convert(durationVal, TimeUnit.DAYS) * 7; } final long durationVal = Long.parseLong(duration); return desiredUnit.convert(durationVal, specifiedTimeUnit); } + /** + * Returns the parsed and converted input in the requested units. + *

+ * If the value is {@code 0 <= x < 1} in the provided units, the units will first be converted to a smaller unit to get a value >= 1 (i.e. 0.5 seconds -> 500 milliseconds). + * This is because the underlying unit conversion cannot handle decimal values. + *

+ * If the value is {@code x >= 1} but x is not a whole number, the units will first be converted to a smaller unit to attempt to get a whole number value (i.e. 1.5 seconds -> 1500 milliseconds). + *

+ * This method handles decimal values over {@code 1 ns}, but {@code < 1 ns} will return {@code 0} in any other unit. + *

+ * Examples: + *

+ * "10 seconds", {@code TimeUnit.MILLISECONDS} -> 10_000.0 + * "0.010 s", {@code TimeUnit.MILLISECONDS} -> 10.0 + * "0.010 s", {@code TimeUnit.SECONDS} -> 0.010 + * "0.010 ns", {@code TimeUnit.NANOSECONDS} -> 0.010 + * "0.010 ns", {@code TimeUnit.MICROSECONDS} -> 0 + * + * @param value the {@code String} input + * @param desiredUnit the desired output {@link TimeUnit} + * @return the parsed and converted amount (without a unit) + */ + public static double getPreciseTimeDuration(final String value, final TimeUnit desiredUnit) { + final Matcher matcher = TIME_DURATION_PATTERN.matcher(value.toLowerCase()); + if (!matcher.matches()) { + throw new IllegalArgumentException("Value '" + value + "' is not a valid time duration"); + } + + final String duration = matcher.group(1); + final String units = matcher.group(2); + + final double durationVal = Double.parseDouble(duration); + long durationLong = 0; + if (durationVal == Math.rint(durationVal)) { + durationLong = Math.round(durationVal); + } else { + // Try reducing the size of the units to make + } + + // The TimeUnit enum doesn't have a value for WEEKS, so handle this case independently + if (isWeek(units)) { + // Do custom week logic + return desiredUnit.convert(durationLong, TimeUnit.DAYS) * 7; + } else { + TimeUnit specifiedTimeUnit = determineTimeUnit(units); + return desiredUnit.convert(durationLong, specifiedTimeUnit); + } + } + + /** + * Converts the provided time duration value to one that can be represented as a whole number. + * Returns a {@code List} containing the new value as a {@code long} at index 0 and the + * {@link TimeUnit} at index 1. If the incoming value is already whole, it is returned as is. + * If the incoming value cannot be made whole, a whole approximation is returned. For values + * {@code >= 1 TimeUnit.NANOSECONDS}, the value is rounded (i.e. 123.4 ns -> 123 ns). + * For values {@code < 1 TimeUnit.NANOSECONDS}, the constant [1L, {@code TimeUnit.NANOSECONDS}] is returned as the smallest measurable unit of time. + *

+ * Examples: + *

+ * 1, {@code TimeUnit.SECONDS} -> [1, {@code TimeUnit.SECONDS}] + * 1.1, {@code TimeUnit.SECONDS} -> [1100, {@code TimeUnit.MILLISECONDS}] + * 0.1, {@code TimeUnit.SECONDS} -> [1000, {@code TimeUnit.MILLISECONDS}] + * 0.1, {@code TimeUnit.NANOSECONDS} -> [1, {@code TimeUnit.NANOSECONDS}] + * + * @param decimal the time duration as a decimal + * @param timeUnit the current time unit + * @return the time duration as a whole number ({@code long}) and the smaller time unit used + */ + protected static List makeWholeNumberTime(double decimal, TimeUnit timeUnit) { + // If the value is already a whole number, return it and the current time unit + if (decimal == Math.rint(decimal)) { + return Arrays.asList(new Object[]{(long) decimal, timeUnit}); + } else if (TimeUnit.NANOSECONDS == timeUnit) { + // The time unit is as small as possible + if (decimal < 1.0) { + decimal = 1; + } else { + decimal = Math.rint(decimal); + } + return Arrays.asList(new Object[]{(long) decimal, timeUnit}); + } else { + // TODO: Implement + return Arrays.asList(new Object[]{1L, TimeUnit.NANOSECONDS}); + // double newDecimal = 0; + // if (decimal < 1.0) { + // newDecimal = 1; + // } else { + // newDecimal = Math.rint(decimal); + // } + // TimeUnit smallerTimeUnit = getSmallerTimeUnit(timeUnit); + + + // int multiplier = calculateMultiplier(timeUnit, smallerTimeUnit); + // smallerTimeUnit.convert(decimal, timeUnit); + // return makeWholeNumber(decimal, timeUnit); + } + } + + protected static long calculateMultiplier(TimeUnit originalTimeUnit, TimeUnit newTimeUnit) { + if (originalTimeUnit == newTimeUnit) { + return 1; + } else if (originalTimeUnit.ordinal() < newTimeUnit.ordinal()) { + throw new IllegalArgumentException("The original time unit '" + originalTimeUnit + "' must be larger than the new time unit '" + newTimeUnit + "'"); + } else { + // TODO: Refactor to constant + final List MULTIPLIERS = Arrays.asList(1000L, 1000L, 1000L, 60L, 60L, 24L); + + int originalOrd = originalTimeUnit.ordinal(); + int newOrd = newTimeUnit.ordinal(); + + List unitMultipliers = MULTIPLIERS.subList(newOrd, originalOrd); + return unitMultipliers.stream().reduce(1L, (a, b) -> (long) a * b); + + // // Check the unit ordinals (*seconds (0-3) are all 1000x multipliers) + // if (originalOrd < 4) { + // if (newOrd < 4) { + // // Both old and new units are some multiplier of seconds, so just subtract the difference in unit prefixes and raise 1000 to that power + // return (long) Math.pow(1000, (originalOrd - newOrd)); + // } + // + // } + } + } + + /** + * Returns the next smallest {@link TimeUnit} (i.e. {@code TimeUnit.DAYS -> TimeUnit.HOURS}). If the parameter is {@code null} or {@code TimeUnit.NANOSECONDS}, an {@link IllegalArgumentException} is thrown because there is no valid smaller TimeUnit. + * + * @param originalUnit the TimeUnit + * @return the next smaller TimeUnit + */ + protected static TimeUnit getSmallerTimeUnit(TimeUnit originalUnit) { + if (originalUnit == null || TimeUnit.NANOSECONDS == originalUnit) { + throw new IllegalArgumentException("Cannot determine a smaller time unit than '" + originalUnit + "'"); + } else { + return TimeUnit.values()[originalUnit.ordinal() - 1]; + } + } + + /** + * Returns {@code true} if this raw unit {@code String} is parsed as representing "weeks", which does not have a value in the {@link TimeUnit} enum. + * + * @param rawUnit the String containing the desired unit + * @return true if the unit is "weeks"; false otherwise + */ + protected static boolean isWeek(final String rawUnit) { + switch (rawUnit) { + case "w": + case "wk": + case "wks": + case "week": + case "weeks": + return true; + default: + return false; + } + } + + /** + * Returns the {@link TimeUnit} enum that maps to the provided raw {@code String} input. The highest time unit is {@code TimeUnit.DAYS}. Any input that cannot be parsed will result in an {@link IllegalArgumentException}. + * + * @param rawUnit the String to parse + * @return the TimeUnit + */ + protected static TimeUnit determineTimeUnit(String rawUnit) { + switch (rawUnit.toLowerCase()) { + case "ns": + case "nano": + case "nanos": + case "nanoseconds": + return TimeUnit.NANOSECONDS; + case "µs": + case "micro": + case "micros": + case "microseconds": + return TimeUnit.MICROSECONDS; + case "ms": + case "milli": + case "millis": + case "milliseconds": + return TimeUnit.MILLISECONDS; + case "s": + case "sec": + case "secs": + case "second": + case "seconds": + return TimeUnit.SECONDS; + case "m": + case "min": + case "mins": + case "minute": + case "minutes": + return TimeUnit.MINUTES; + case "h": + case "hr": + case "hrs": + case "hour": + case "hours": + return TimeUnit.HOURS; + case "d": + case "day": + case "days": + return TimeUnit.DAYS; + default: + throw new IllegalArgumentException("Could not parse '" + rawUnit + "' to TimeUnit"); + } + } + public static String formatUtilization(final double utilization) { return utilization + "%"; } @@ -225,7 +433,7 @@ private static String join(final String delimiter, final String... values) { * 3 seconds, 8 millis, 3 nanos - if includeTotalNanos = false, * 3 seconds, 8 millis, 3 nanos (3008000003 nanos) - if includeTotalNanos = true * - * @param nanos the number of nanoseconds to format + * @param nanos the number of nanoseconds to format * @param includeTotalNanos whether or not to include the total number of nanoseconds in parentheses in the returned value * @return a human-readable String that is a formatted representation of the given number of nanoseconds. */ diff --git a/nifi-commons/nifi-utils/src/test/groovy/org/apache/nifi/processor/TestFormatUtilsGroovy.groovy b/nifi-commons/nifi-utils/src/test/groovy/org/apache/nifi/processor/TestFormatUtilsGroovy.groovy deleted file mode 100644 index f3e4f4640226..000000000000 --- a/nifi-commons/nifi-utils/src/test/groovy/org/apache/nifi/processor/TestFormatUtilsGroovy.groovy +++ /dev/null @@ -1,130 +0,0 @@ -/* - * 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.nifi.processor - -import org.apache.nifi.util.FormatUtils -import org.junit.After -import org.junit.Before -import org.junit.BeforeClass -import org.junit.Test -import org.junit.runner.RunWith -import org.junit.runners.JUnit4 -import org.slf4j.Logger -import org.slf4j.LoggerFactory - -import java.util.concurrent.TimeUnit - -@RunWith(JUnit4.class) -class TestFormatUtilsGroovy extends GroovyTestCase { - private static final Logger logger = LoggerFactory.getLogger(TestFormatUtilsGroovy.class) - - @BeforeClass - public static void setUpOnce() throws Exception { - logger.metaClass.methodMissing = { String name, args -> - logger.info("[${name?.toUpperCase()}] ${(args as List).join(" ")}") - } - } - - @Before - public void setUp() throws Exception { - - } - - @After - public void tearDown() throws Exception { - - } - - /** - * New feature test - */ - @Test - void testShouldConvertWeeks() { - // Arrange - final List WEEKS = ["1 week", "1 wk", "1 w", "1 wks", "1 weeks"] - final long EXPECTED_DAYS = 7L - - // Act - List days = WEEKS.collect { String week -> - FormatUtils.getTimeDuration(week, TimeUnit.DAYS) - } - logger.converted(days) - - // Assert - assert days.every { it == EXPECTED_DAYS } - } - - - - @Test - void testShouldHandleNegativeWeeks() { - // Arrange - final List WEEKS = ["-1 week", "-1 wk", "-1 w", "-1 weeks", "- 1 week"] - - // Act - List msgs = WEEKS.collect { String week -> - shouldFail(IllegalArgumentException) { - FormatUtils.getTimeDuration(week, TimeUnit.DAYS) - } - } - - // Assert - assert msgs.every { it =~ /Value '.*' is not a valid Time Duration/ } - } - - - - /** - * Regression test - */ - @Test - void testShouldHandleInvalidAbbreviations() { - // Arrange - final List WEEKS = ["1 work", "1 wek", "1 k"] - - // Act - List msgs = WEEKS.collect { String week -> - shouldFail(IllegalArgumentException) { - FormatUtils.getTimeDuration(week, TimeUnit.DAYS) - } - } - - // Assert - assert msgs.every { it =~ /Value '.*' is not a valid Time Duration/ } - - } - - - /** - * New feature test - */ - @Test - void testShouldHandleNoSpaceInInput() { - // Arrange - final List WEEKS = ["1week", "1wk", "1w", "1wks", "1weeks"] - final long EXPECTED_DAYS = 7L - - // Act - List days = WEEKS.collect { String week -> - FormatUtils.getTimeDuration(week, TimeUnit.DAYS) - } - logger.converted(days) - - // Assert - assert days.every { it == EXPECTED_DAYS } - } -} \ No newline at end of file diff --git a/nifi-commons/nifi-utils/src/test/groovy/org/apache/nifi/util/TestFormatUtilsGroovy.groovy b/nifi-commons/nifi-utils/src/test/groovy/org/apache/nifi/util/TestFormatUtilsGroovy.groovy new file mode 100644 index 000000000000..058aa2e42b85 --- /dev/null +++ b/nifi-commons/nifi-utils/src/test/groovy/org/apache/nifi/util/TestFormatUtilsGroovy.groovy @@ -0,0 +1,351 @@ +/* + * 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.nifi.util + + +import org.junit.After +import org.junit.Before +import org.junit.BeforeClass +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.JUnit4 +import org.slf4j.Logger +import org.slf4j.LoggerFactory + +import java.util.concurrent.TimeUnit + +@RunWith(JUnit4.class) +class TestFormatUtilsGroovy extends GroovyTestCase { + private static final Logger logger = LoggerFactory.getLogger(TestFormatUtilsGroovy.class) + + @BeforeClass + static void setUpOnce() throws Exception { + logger.metaClass.methodMissing = { String name, args -> + logger.info("[${name?.toUpperCase()}] ${(args as List).join(" ")}") + } + } + + @Before + void setUp() throws Exception { + + } + + @After + void tearDown() throws Exception { + + } + + /** + * New feature test + */ + @Test + void testShouldConvertWeeks() { + // Arrange + final List WEEKS = ["1 week", "1 wk", "1 w", "1 wks", "1 weeks"] + final long EXPECTED_DAYS = 7L + + // Act + List days = WEEKS.collect { String week -> + FormatUtils.getTimeDuration(week, TimeUnit.DAYS) + } + logger.converted(days) + + // Assert + assert days.every { it == EXPECTED_DAYS } + } + + + @Test + void testShouldHandleNegativeWeeks() { + // Arrange + final List WEEKS = ["-1 week", "-1 wk", "-1 w", "-1 weeks", "- 1 week"] + + // Act + List msgs = WEEKS.collect { String week -> + shouldFail(IllegalArgumentException) { + FormatUtils.getTimeDuration(week, TimeUnit.DAYS) + } + } + + // Assert + assert msgs.every { it =~ /Value '.*' is not a valid Time Duration/ } + } + + /** + * Regression test + */ + @Test + void testShouldHandleInvalidAbbreviations() { + // Arrange + final List WEEKS = ["1 work", "1 wek", "1 k"] + + // Act + List msgs = WEEKS.collect { String week -> + shouldFail(IllegalArgumentException) { + FormatUtils.getTimeDuration(week, TimeUnit.DAYS) + } + } + + // Assert + assert msgs.every { it =~ /Value '.*' is not a valid Time Duration/ } + + } + + /** + * New feature test + */ + @Test + void testShouldHandleNoSpaceInInput() { + // Arrange + final List WEEKS = ["1week", "1wk", "1w", "1wks", "1weeks"] + final long EXPECTED_DAYS = 7L + + // Act + List days = WEEKS.collect { String week -> + FormatUtils.getTimeDuration(week, TimeUnit.DAYS) + } + logger.converted(days) + + // Assert + assert days.every { it == EXPECTED_DAYS } + } + + /** + * New feature test + */ + @Test + void testShouldHandleDecimalValues() { + // Arrange + final List WHOLE_NUMBERS = ["10 ms", "10 millis", "10 milliseconds"] + final List DECIMAL_NUMBERS = ["0.010 s", "0.010 seconds"] + final long EXPECTED_MILLIS = 10 + + // Act + List parsedWholeMillis = WHOLE_NUMBERS.collect { String whole -> + FormatUtils.getTimeDuration(whole, TimeUnit.MILLISECONDS) + } + logger.converted(parsedWholeMillis) + + List parsedDecimalMillis = DECIMAL_NUMBERS.collect { String decimal -> + FormatUtils.getTimeDuration(decimal, TimeUnit.MILLISECONDS) + } + logger.converted(parsedDecimalMillis) + + // Assert + assert parsedWholeMillis.every { it == EXPECTED_MILLIS } + assert parsedDecimalMillis.every { it == EXPECTED_MILLIS } + } + + /** + * New feature test + */ + @Test + void testGetPreciseTimeDurationShouldHandleDecimalValues() { + // Arrange + final List WHOLE_NUMBERS = ["10 ms", "10 millis", "10 milliseconds"] + final List DECIMAL_NUMBERS = ["0.010 s", "0.010 seconds"] + final float EXPECTED_MILLIS = 10.0 + + // Act + List parsedWholeMillis = WHOLE_NUMBERS.collect { String whole -> + FormatUtils.getPreciseTimeDuration(whole, TimeUnit.MILLISECONDS) + } + logger.converted(parsedWholeMillis) + + List parsedDecimalMillis = DECIMAL_NUMBERS.collect { String decimal -> + FormatUtils.getPreciseTimeDuration(decimal, TimeUnit.MILLISECONDS) + } + logger.converted(parsedDecimalMillis) + + // Assert + assert parsedWholeMillis.every { it == EXPECTED_MILLIS } + assert parsedDecimalMillis.every { it == EXPECTED_MILLIS } + } + + /** + * Positive flow test for decimal inputs that can be converted (all equal values) + */ + @Test + void testShouldMakeWholeNumberTime() { + // Arrange + final List DECIMAL_TIMES = [ + [0.000_000_010, TimeUnit.SECONDS], + [0.000_010, TimeUnit.MILLISECONDS], + [0.010, TimeUnit.MICROSECONDS] + ] + final long EXPECTED_NANOS = 10L + + // Act + List parsedWholeNanos = DECIMAL_TIMES.collect { List it -> + FormatUtils.makeWholeNumberTime(it[0] as float, it[1] as TimeUnit) + } + logger.converted(parsedWholeNanos) + + // Assert + assert parsedWholeNanos.every { it == [EXPECTED_NANOS, TimeUnit.NANOSECONDS] } + } + + // TODO: Non-metric units (i.e. days to hours, hours to minutes, minutes to seconds) + + /** + * Positive flow test for whole inputs + */ + @Test + void testMakeWholeNumberTimeShouldHandleWholeNumbers() { + // Arrange + final List WHOLE_TIMES = [ + [10.0, TimeUnit.DAYS], + [10.0, TimeUnit.HOURS], + [10.0, TimeUnit.MINUTES], + [10.0, TimeUnit.SECONDS], + [10.0, TimeUnit.MILLISECONDS], + [10.0, TimeUnit.MICROSECONDS], + [10.0, TimeUnit.NANOSECONDS], + ] + + // Act + List parsedWholeTimes = WHOLE_TIMES.collect { List it -> + FormatUtils.makeWholeNumberTime(it[0] as float, it[1] as TimeUnit) + } + logger.converted(parsedWholeTimes) + + // Assert + parsedWholeTimes.eachWithIndex { List elements, int i -> + assert elements[0] instanceof Long + assert elements[0] == 10L + assert elements[1] == WHOLE_TIMES[i][1] + } + } + + /** + * Negative flow test for nanosecond inputs (regardless of value, the unit cannot be converted) + */ + @Test + void testMakeWholeNumberTimeShouldHandleNanoseconds() { + // Arrange + final List WHOLE_TIMES = [ + [1100.0, TimeUnit.NANOSECONDS], + [2.1, TimeUnit.NANOSECONDS], + [1.0, TimeUnit.NANOSECONDS], + [0.1, TimeUnit.NANOSECONDS], + ] + + final List EXPECTED_TIMES = [ + [1100L, TimeUnit.NANOSECONDS], + [2L, TimeUnit.NANOSECONDS], + [1L, TimeUnit.NANOSECONDS], + [1L, TimeUnit.NANOSECONDS], + ] + + // Act + List parsedWholeTimes = WHOLE_TIMES.collect { List it -> + FormatUtils.makeWholeNumberTime(it[0] as float, it[1] as TimeUnit) + } + logger.converted(parsedWholeTimes) + + // Assert + assert parsedWholeTimes == EXPECTED_TIMES + } + + /** + * Positive flow test for whole inputs + */ + @Test + void testShouldGetSmallerTimeUnit() { + // Arrange + final List UNITS = TimeUnit.values() as List + + // Act + def nullMsg = shouldFail(IllegalArgumentException) { + FormatUtils.getSmallerTimeUnit(null) + } + logger.expected(nullMsg) + + def nanosMsg = shouldFail(IllegalArgumentException) { + FormatUtils.getSmallerTimeUnit(TimeUnit.NANOSECONDS) + } + logger.expected(nanosMsg) + + List smallerTimeUnits = UNITS[1..-1].collect { TimeUnit unit -> + FormatUtils.getSmallerTimeUnit(unit) + } + logger.converted(smallerTimeUnits) + + // Assert + assert nullMsg == "Cannot determine a smaller time unit than 'null'" + assert nanosMsg == "Cannot determine a smaller time unit than 'NANOSECONDS'" + assert smallerTimeUnits == UNITS[0..<-1] + } + + /** + * Positive flow test for multipliers based on valid time units + */ + @Test + void testShouldCalculateMultiplier() { + // Arrange + final Map SCENARIOS = [ + "allUnits" : [original: TimeUnit.DAYS, destination: TimeUnit.NANOSECONDS, expectedMultiplier: (long) 24 * 60 * 60 * 1_000_000_000], + "microsToNanos" : [original: TimeUnit.MICROSECONDS, destination: TimeUnit.NANOSECONDS, expectedMultiplier: 1_000], + "millisToNanos" : [original: TimeUnit.MILLISECONDS, destination: TimeUnit.NANOSECONDS, expectedMultiplier: 1_000_000], + "millisToMicros": [original: TimeUnit.MILLISECONDS, destination: TimeUnit.MICROSECONDS, expectedMultiplier: 1_000], + "daysToHours" : [original: TimeUnit.DAYS, destination: TimeUnit.HOURS, expectedMultiplier: 24], + "daysToSeconds" : [original: TimeUnit.DAYS, destination: TimeUnit.SECONDS, expectedMultiplier: 24 * 60 * 60], + ] + + // Act + Map results = SCENARIOS.collectEntries { String k, Map values -> + logger.debug("Evaluating ${k}: ${values}") + [k, FormatUtils.calculateMultiplier(values.original, values.destination)] + } + logger.converted(results) + + // Assert + results.every { String key, long value -> + assert value == SCENARIOS[key].expectedMultiplier + } + } + + /** + * Negative flow test for multipliers based on incorrectly-ordered time units + */ + @Test + void testCalculateMultiplierShouldHandleIncorrectUnits() { + // Arrange + final Map SCENARIOS = [ + "allUnits" : [original: TimeUnit.NANOSECONDS, destination: TimeUnit.DAYS], + "nanosToMicros" : [original: TimeUnit.NANOSECONDS, destination: TimeUnit.MICROSECONDS], + "hoursToDays" : [original: TimeUnit.HOURS, destination: TimeUnit.DAYS], + ] + + // Act + Map results = SCENARIOS.collectEntries { String k, Map values -> + logger.debug("Evaluating ${k}: ${values}") + def msg = shouldFail(IllegalArgumentException) { + FormatUtils.calculateMultiplier(values.original, values.destination) + } + logger.expected(msg) + [k, msg] + } + + // Assert + results.every { String key, String value -> + assert value =~ "The original time unit '.*' must be larger than the new time unit '.*'" + } + } + + // TODO: Microsecond parsing +} \ No newline at end of file From 50b7b6436454b3b0d7e36fd1f2139dd1f10b9067 Mon Sep 17 00:00:00 2001 From: Andy LoPresto Date: Thu, 29 Nov 2018 20:25:04 -0800 Subject: [PATCH 2/9] NIFI-5854 [WIP] Cleaned up logic. Resolved failing unit tests due to error message change. --- .../org/apache/nifi/util/FormatUtils.java | 26 +++++++++---------- .../nifi/util/TestFormatUtilsGroovy.groovy | 4 +-- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/nifi-commons/nifi-utils/src/main/java/org/apache/nifi/util/FormatUtils.java b/nifi-commons/nifi-utils/src/main/java/org/apache/nifi/util/FormatUtils.java index 18658ce70f2a..4b791cbb5251 100644 --- a/nifi-commons/nifi-utils/src/main/java/org/apache/nifi/util/FormatUtils.java +++ b/nifi-commons/nifi-utils/src/main/java/org/apache/nifi/util/FormatUtils.java @@ -44,6 +44,7 @@ public class FormatUtils { private static final String VALID_TIME_UNITS = join(UNION, NANOS, MILLIS, SECS, MINS, HOURS, DAYS, WEEKS); public static final String TIME_DURATION_REGEX = "([\\d.]+)\\s*(" + VALID_TIME_UNITS + ")"; public static final Pattern TIME_DURATION_PATTERN = Pattern.compile(TIME_DURATION_REGEX); + private static final List TIME_UNIT_MULTIPLIERS = Arrays.asList(1000L, 1000L, 1000L, 60L, 60L, 24L); /** * Formats the specified count by adding commas. @@ -299,29 +300,28 @@ protected static List makeWholeNumberTime(double decimal, TimeUnit timeU } } + /** + * Returns the numerical multiplier to convert a value from {@code originalTimeUnit} to + * {@code newTimeUnit} (i.e. for {@code TimeUnit.DAYS -> TimeUnit.MINUTES} would return + * 24 * 60 = 720). If the original and new units are the same, returns 1. If the new unit + * is larger than the original (i.e. the result would be less than 1), throws an + * {@link IllegalArgumentException}. + * + * @param originalTimeUnit the source time unit + * @param newTimeUnit the destination time unit + * @return the numerical multiplier between the units + */ protected static long calculateMultiplier(TimeUnit originalTimeUnit, TimeUnit newTimeUnit) { if (originalTimeUnit == newTimeUnit) { return 1; } else if (originalTimeUnit.ordinal() < newTimeUnit.ordinal()) { throw new IllegalArgumentException("The original time unit '" + originalTimeUnit + "' must be larger than the new time unit '" + newTimeUnit + "'"); } else { - // TODO: Refactor to constant - final List MULTIPLIERS = Arrays.asList(1000L, 1000L, 1000L, 60L, 60L, 24L); - int originalOrd = originalTimeUnit.ordinal(); int newOrd = newTimeUnit.ordinal(); - List unitMultipliers = MULTIPLIERS.subList(newOrd, originalOrd); + List unitMultipliers = TIME_UNIT_MULTIPLIERS.subList(newOrd, originalOrd); return unitMultipliers.stream().reduce(1L, (a, b) -> (long) a * b); - - // // Check the unit ordinals (*seconds (0-3) are all 1000x multipliers) - // if (originalOrd < 4) { - // if (newOrd < 4) { - // // Both old and new units are some multiplier of seconds, so just subtract the difference in unit prefixes and raise 1000 to that power - // return (long) Math.pow(1000, (originalOrd - newOrd)); - // } - // - // } } } diff --git a/nifi-commons/nifi-utils/src/test/groovy/org/apache/nifi/util/TestFormatUtilsGroovy.groovy b/nifi-commons/nifi-utils/src/test/groovy/org/apache/nifi/util/TestFormatUtilsGroovy.groovy index 058aa2e42b85..9792c1860127 100644 --- a/nifi-commons/nifi-utils/src/test/groovy/org/apache/nifi/util/TestFormatUtilsGroovy.groovy +++ b/nifi-commons/nifi-utils/src/test/groovy/org/apache/nifi/util/TestFormatUtilsGroovy.groovy @@ -82,7 +82,7 @@ class TestFormatUtilsGroovy extends GroovyTestCase { } // Assert - assert msgs.every { it =~ /Value '.*' is not a valid Time Duration/ } + assert msgs.every { it =~ /Value '.*' is not a valid time duration/ } } /** @@ -101,7 +101,7 @@ class TestFormatUtilsGroovy extends GroovyTestCase { } // Assert - assert msgs.every { it =~ /Value '.*' is not a valid Time Duration/ } + assert msgs.every { it =~ /Value '.*' is not a valid time duration/ } } From fc16cf37439e1bee5afe58adbfe61c31bc77df3f Mon Sep 17 00:00:00 2001 From: Andy LoPresto Date: Thu, 29 Nov 2018 20:48:21 -0800 Subject: [PATCH 3/9] NIFI-5854 [WIP] All helper method unit tests pass. --- .../org/apache/nifi/util/FormatUtils.java | 20 ++---- .../nifi/util/TestFormatUtilsGroovy.groovy | 62 +++++++++++++++++-- 2 files changed, 63 insertions(+), 19 deletions(-) diff --git a/nifi-commons/nifi-utils/src/main/java/org/apache/nifi/util/FormatUtils.java b/nifi-commons/nifi-utils/src/main/java/org/apache/nifi/util/FormatUtils.java index 4b791cbb5251..faa36e0b595c 100644 --- a/nifi-commons/nifi-utils/src/main/java/org/apache/nifi/util/FormatUtils.java +++ b/nifi-commons/nifi-utils/src/main/java/org/apache/nifi/util/FormatUtils.java @@ -283,20 +283,12 @@ protected static List makeWholeNumberTime(double decimal, TimeUnit timeU } return Arrays.asList(new Object[]{(long) decimal, timeUnit}); } else { - // TODO: Implement - return Arrays.asList(new Object[]{1L, TimeUnit.NANOSECONDS}); - // double newDecimal = 0; - // if (decimal < 1.0) { - // newDecimal = 1; - // } else { - // newDecimal = Math.rint(decimal); - // } - // TimeUnit smallerTimeUnit = getSmallerTimeUnit(timeUnit); - - - // int multiplier = calculateMultiplier(timeUnit, smallerTimeUnit); - // smallerTimeUnit.convert(decimal, timeUnit); - // return makeWholeNumber(decimal, timeUnit); + // Determine the next time unit and the respective multiplier + TimeUnit smallerTimeUnit = getSmallerTimeUnit(timeUnit); + long multiplier = calculateMultiplier(timeUnit, smallerTimeUnit); + + // Recurse with the original number converted to the smaller unit + return makeWholeNumberTime(decimal * multiplier, smallerTimeUnit); } } diff --git a/nifi-commons/nifi-utils/src/test/groovy/org/apache/nifi/util/TestFormatUtilsGroovy.groovy b/nifi-commons/nifi-utils/src/test/groovy/org/apache/nifi/util/TestFormatUtilsGroovy.groovy index 9792c1860127..5d691ba36e4a 100644 --- a/nifi-commons/nifi-utils/src/test/groovy/org/apache/nifi/util/TestFormatUtilsGroovy.groovy +++ b/nifi-commons/nifi-utils/src/test/groovy/org/apache/nifi/util/TestFormatUtilsGroovy.groovy @@ -180,7 +180,7 @@ class TestFormatUtilsGroovy extends GroovyTestCase { * Positive flow test for decimal inputs that can be converted (all equal values) */ @Test - void testShouldMakeWholeNumberTime() { + void testMakeWholeNumberTimeShouldHandleDecimals() { // Arrange final List DECIMAL_TIMES = [ [0.000_000_010, TimeUnit.SECONDS], @@ -199,7 +199,59 @@ class TestFormatUtilsGroovy extends GroovyTestCase { assert parsedWholeNanos.every { it == [EXPECTED_NANOS, TimeUnit.NANOSECONDS] } } - // TODO: Non-metric units (i.e. days to hours, hours to minutes, minutes to seconds) + /** + * Positive flow test for decimal inputs that can be converted (metric values) + */ + @Test + void testMakeWholeNumberTimeShouldHandleMetricConversions() { + // Arrange + final Map SCENARIOS = [ + "secondsToMillis" : [originalUnits: TimeUnit.SECONDS, expectedUnits: TimeUnit.MILLISECONDS, expectedValue: 123_400, originalValue: 123.4], + "secondsToMicros" : [originalUnits: TimeUnit.SECONDS, expectedUnits: TimeUnit.MICROSECONDS, originalValue: 1.000_345, expectedValue: 1_000_345], + "millisToNanos" : [originalUnits: TimeUnit.MILLISECONDS, expectedUnits: TimeUnit.NANOSECONDS, originalValue: 0.75, expectedValue: 750_000], + "nanosToNanosGE1" : [originalUnits: TimeUnit.NANOSECONDS, expectedUnits: TimeUnit.NANOSECONDS, originalValue: 123.4, expectedValue: 123], + "nanosToNanosLE1" : [originalUnits: TimeUnit.NANOSECONDS, expectedUnits: TimeUnit.NANOSECONDS, originalValue: 0.123, expectedValue: 1], + ] + + // Act + Map results = SCENARIOS.collectEntries { String k, Map values -> + logger.debug("Evaluating ${k}: ${values}") + [k, FormatUtils.makeWholeNumberTime(values.originalValue, values.originalUnits)] + } + logger.info(results) + + // Assert + results.every { String key, List values -> + assert values.first() == SCENARIOS[key].expectedValue + assert values.last() == SCENARIOS[key].expectedUnits + } + } + + /** + * Positive flow test for decimal inputs that can be converted (non-metric values) + */ + @Test + void testMakeWholeNumberTimeShouldHandleNonMetricConversions() { + // Arrange + final Map SCENARIOS = [ + "daysToHours" : [originalUnits: TimeUnit.DAYS, expectedUnits: TimeUnit.HOURS, expectedValue: 36, originalValue: 1.5], + "hoursToMinutes" : [originalUnits: TimeUnit.HOURS, expectedUnits: TimeUnit.MINUTES, originalValue: 1.5, expectedValue: 90], + "hoursToMinutes2" : [originalUnits: TimeUnit.HOURS, expectedUnits: TimeUnit.MINUTES, originalValue: 0.75, expectedValue: 45], + ] + + // Act + Map results = SCENARIOS.collectEntries { String k, Map values -> + logger.debug("Evaluating ${k}: ${values}") + [k, FormatUtils.makeWholeNumberTime(values.originalValue, values.originalUnits)] + } + logger.info(results) + + // Assert + results.every { String key, List values -> + assert values.first() == SCENARIOS[key].expectedValue + assert values.last() == SCENARIOS[key].expectedUnits + } + } /** * Positive flow test for whole inputs @@ -326,9 +378,9 @@ class TestFormatUtilsGroovy extends GroovyTestCase { void testCalculateMultiplierShouldHandleIncorrectUnits() { // Arrange final Map SCENARIOS = [ - "allUnits" : [original: TimeUnit.NANOSECONDS, destination: TimeUnit.DAYS], - "nanosToMicros" : [original: TimeUnit.NANOSECONDS, destination: TimeUnit.MICROSECONDS], - "hoursToDays" : [original: TimeUnit.HOURS, destination: TimeUnit.DAYS], + "allUnits" : [original: TimeUnit.NANOSECONDS, destination: TimeUnit.DAYS], + "nanosToMicros": [original: TimeUnit.NANOSECONDS, destination: TimeUnit.MICROSECONDS], + "hoursToDays" : [original: TimeUnit.HOURS, destination: TimeUnit.DAYS], ] // Act From a0865667f03ad9e1a0ad46277045fa364720cd98 Mon Sep 17 00:00:00 2001 From: Andy LoPresto Date: Thu, 29 Nov 2018 21:15:27 -0800 Subject: [PATCH 4/9] NIFI-5854 [WIP] FormatUtils#getPreciseTimeDuration() now handles all tested inputs correctly. Added unit tests. --- .../org/apache/nifi/util/FormatUtils.java | 31 ++++++---- .../nifi/util/TestFormatUtilsGroovy.groovy | 60 ++++++++++++++++++- 2 files changed, 78 insertions(+), 13 deletions(-) diff --git a/nifi-commons/nifi-utils/src/main/java/org/apache/nifi/util/FormatUtils.java b/nifi-commons/nifi-utils/src/main/java/org/apache/nifi/util/FormatUtils.java index faa36e0b595c..a6f2d9235644 100644 --- a/nifi-commons/nifi-utils/src/main/java/org/apache/nifi/util/FormatUtils.java +++ b/nifi-commons/nifi-utils/src/main/java/org/apache/nifi/util/FormatUtils.java @@ -233,22 +233,29 @@ public static double getPreciseTimeDuration(final String value, final TimeUnit d final String duration = matcher.group(1); final String units = matcher.group(2); - final double durationVal = Double.parseDouble(duration); - long durationLong = 0; - if (durationVal == Math.rint(durationVal)) { - durationLong = Math.round(durationVal); - } else { - // Try reducing the size of the units to make - } + double durationVal = Double.parseDouble(duration); + TimeUnit specifiedTimeUnit; // The TimeUnit enum doesn't have a value for WEEKS, so handle this case independently if (isWeek(units)) { - // Do custom week logic - return desiredUnit.convert(durationLong, TimeUnit.DAYS) * 7; + specifiedTimeUnit = TimeUnit.DAYS; + durationVal *= 7; + } else { + specifiedTimeUnit = determineTimeUnit(units); + } + + // The units are now guaranteed to be in DAYS or smaller + long durationLong; + if (durationVal == Math.rint(durationVal)) { + durationLong = Math.round(durationVal); } else { - TimeUnit specifiedTimeUnit = determineTimeUnit(units); - return desiredUnit.convert(durationLong, specifiedTimeUnit); + // Try reducing the size of the units to make the input a long + List wholeResults = makeWholeNumberTime(durationVal, specifiedTimeUnit); + durationLong = (long) wholeResults.get(0); + specifiedTimeUnit = (TimeUnit) wholeResults.get(1); } + + return desiredUnit.convert(durationLong, specifiedTimeUnit); } /** @@ -300,7 +307,7 @@ protected static List makeWholeNumberTime(double decimal, TimeUnit timeU * {@link IllegalArgumentException}. * * @param originalTimeUnit the source time unit - * @param newTimeUnit the destination time unit + * @param newTimeUnit the destination time unit * @return the numerical multiplier between the units */ protected static long calculateMultiplier(TimeUnit originalTimeUnit, TimeUnit newTimeUnit) { diff --git a/nifi-commons/nifi-utils/src/test/groovy/org/apache/nifi/util/TestFormatUtilsGroovy.groovy b/nifi-commons/nifi-utils/src/test/groovy/org/apache/nifi/util/TestFormatUtilsGroovy.groovy index 5d691ba36e4a..77bb82e21ded 100644 --- a/nifi-commons/nifi-utils/src/test/groovy/org/apache/nifi/util/TestFormatUtilsGroovy.groovy +++ b/nifi-commons/nifi-utils/src/test/groovy/org/apache/nifi/util/TestFormatUtilsGroovy.groovy @@ -151,7 +151,65 @@ class TestFormatUtilsGroovy extends GroovyTestCase { } /** - * New feature test + * Regression test for custom week logic + */ + @Test + void testGetPreciseTimeDurationShouldHandleWeeks() { + // Arrange + final String ONE_WEEK = "1 week" + final Map ONE_WEEK_IN_OTHER_UNITS = [ + (TimeUnit.DAYS): 7, + (TimeUnit.HOURS): 7 * 24, + (TimeUnit.MINUTES): 7 * 24 * 60, + (TimeUnit.SECONDS): (long) 7 * 24 * 60 * 60, + (TimeUnit.MILLISECONDS): (long) 7 * 24 * 60 * 60 * 1000, + (TimeUnit.MICROSECONDS): (long) 7 * 24 * 60 * 60 * 1000 * 1000, + (TimeUnit.NANOSECONDS): (long) 7 * 24 * 60 * 60 * 1000 * 1000 * 1000, + ] + + // Act + Map oneWeekInOtherUnits = TimeUnit.values()[0..<-1].collectEntries { TimeUnit destinationUnit -> + [destinationUnit, FormatUtils.getPreciseTimeDuration(ONE_WEEK, destinationUnit)] + } + logger.converted(oneWeekInOtherUnits) + + // Assert + oneWeekInOtherUnits.each {TimeUnit k, double value -> + assert value == ONE_WEEK_IN_OTHER_UNITS[k] + } + } + + /** + * Positive flow test for custom week logic with decimal value + */ + @Test + void testGetPreciseTimeDurationShouldHandleDecimalWeeks() { + // Arrange + final String ONE_AND_A_HALF_WEEKS = "1.5 week" + final Map ONE_POINT_FIVE_WEEKS_IN_OTHER_UNITS = [ + (TimeUnit.DAYS): 7, + (TimeUnit.HOURS): 7 * 24, + (TimeUnit.MINUTES): 7 * 24 * 60, + (TimeUnit.SECONDS): (long) 7 * 24 * 60 * 60, + (TimeUnit.MILLISECONDS): (long) 7 * 24 * 60 * 60 * 1000, + (TimeUnit.MICROSECONDS): (long) 7 * 24 * 60 * 60 * 1000 * 1000, + (TimeUnit.NANOSECONDS): (long) 7 * 24 * 60 * 60 * 1000 * 1000 * 1000, + ].collectEntries { k, v -> [k, v * 1.5] } + + // Act + Map onePointFiveWeeksInOtherUnits = TimeUnit.values()[0..<-1].collectEntries { TimeUnit destinationUnit -> + [destinationUnit, FormatUtils.getPreciseTimeDuration(ONE_AND_A_HALF_WEEKS, destinationUnit)] + } + logger.converted(onePointFiveWeeksInOtherUnits) + + // Assert + onePointFiveWeeksInOtherUnits.each {TimeUnit k, double value -> + assert value == ONE_POINT_FIVE_WEEKS_IN_OTHER_UNITS[k] + } + } + + /** + * Positive flow test for decimal time inputs */ @Test void testGetPreciseTimeDurationShouldHandleDecimalValues() { From d2031d5a06713b69f1323215afe87efc8d93428f Mon Sep 17 00:00:00 2001 From: Andy LoPresto Date: Fri, 30 Nov 2018 11:39:34 -0800 Subject: [PATCH 5/9] NIFI-5854 [WIP] FormatUtils#getTimeDuration() still using long. Added unit tests. Renamed existing unit tests to reflect method under test. --- .../org/apache/nifi/util/FormatUtils.java | 4 +- .../nifi/util/TestFormatUtilsGroovy.groovy | 75 +++++++++++++------ 2 files changed, 54 insertions(+), 25 deletions(-) diff --git a/nifi-commons/nifi-utils/src/main/java/org/apache/nifi/util/FormatUtils.java b/nifi-commons/nifi-utils/src/main/java/org/apache/nifi/util/FormatUtils.java index a6f2d9235644..b63ab2c1ea0b 100644 --- a/nifi-commons/nifi-utils/src/main/java/org/apache/nifi/util/FormatUtils.java +++ b/nifi-commons/nifi-utils/src/main/java/org/apache/nifi/util/FormatUtils.java @@ -210,6 +210,8 @@ public static long getTimeDuration(final String value, final TimeUnit desiredUni *

* If the value is {@code x >= 1} but x is not a whole number, the units will first be converted to a smaller unit to attempt to get a whole number value (i.e. 1.5 seconds -> 1500 milliseconds). *

+ * If the value is {@code x < 1000} and the units are {@code TimeUnit.NANOSECONDS}, the result will be a whole number of nanoseconds, rounded (i.e. 123.4 ns -> 123 ns). + *

* This method handles decimal values over {@code 1 ns}, but {@code < 1 ns} will return {@code 0} in any other unit. *

* Examples: @@ -217,7 +219,7 @@ public static long getTimeDuration(final String value, final TimeUnit desiredUni * "10 seconds", {@code TimeUnit.MILLISECONDS} -> 10_000.0 * "0.010 s", {@code TimeUnit.MILLISECONDS} -> 10.0 * "0.010 s", {@code TimeUnit.SECONDS} -> 0.010 - * "0.010 ns", {@code TimeUnit.NANOSECONDS} -> 0.010 + * "0.010 ns", {@code TimeUnit.NANOSECONDS} -> 1 * "0.010 ns", {@code TimeUnit.MICROSECONDS} -> 0 * * @param value the {@code String} input diff --git a/nifi-commons/nifi-utils/src/test/groovy/org/apache/nifi/util/TestFormatUtilsGroovy.groovy b/nifi-commons/nifi-utils/src/test/groovy/org/apache/nifi/util/TestFormatUtilsGroovy.groovy index 77bb82e21ded..04a9b436c331 100644 --- a/nifi-commons/nifi-utils/src/test/groovy/org/apache/nifi/util/TestFormatUtilsGroovy.groovy +++ b/nifi-commons/nifi-utils/src/test/groovy/org/apache/nifi/util/TestFormatUtilsGroovy.groovy @@ -53,7 +53,7 @@ class TestFormatUtilsGroovy extends GroovyTestCase { * New feature test */ @Test - void testShouldConvertWeeks() { + void testGetTimeDurationShouldConvertWeeks() { // Arrange final List WEEKS = ["1 week", "1 wk", "1 w", "1 wks", "1 weeks"] final long EXPECTED_DAYS = 7L @@ -70,7 +70,7 @@ class TestFormatUtilsGroovy extends GroovyTestCase { @Test - void testShouldHandleNegativeWeeks() { + void testGetTimeDurationShouldHandleNegativeWeeks() { // Arrange final List WEEKS = ["-1 week", "-1 wk", "-1 w", "-1 weeks", "- 1 week"] @@ -89,7 +89,7 @@ class TestFormatUtilsGroovy extends GroovyTestCase { * Regression test */ @Test - void testShouldHandleInvalidAbbreviations() { + void testGetTimeDurationShouldHandleInvalidAbbreviations() { // Arrange final List WEEKS = ["1 work", "1 wek", "1 k"] @@ -109,7 +109,7 @@ class TestFormatUtilsGroovy extends GroovyTestCase { * New feature test */ @Test - void testShouldHandleNoSpaceInInput() { + void testGetTimeDurationShouldHandleNoSpaceInInput() { // Arrange final List WEEKS = ["1week", "1wk", "1w", "1wks", "1weeks"] final long EXPECTED_DAYS = 7L @@ -128,7 +128,7 @@ class TestFormatUtilsGroovy extends GroovyTestCase { * New feature test */ @Test - void testShouldHandleDecimalValues() { + void testGetTimeDurationShouldHandleDecimalValues() { // Arrange final List WHOLE_NUMBERS = ["10 ms", "10 millis", "10 milliseconds"] final List DECIMAL_NUMBERS = ["0.010 s", "0.010 seconds"] @@ -158,13 +158,13 @@ class TestFormatUtilsGroovy extends GroovyTestCase { // Arrange final String ONE_WEEK = "1 week" final Map ONE_WEEK_IN_OTHER_UNITS = [ - (TimeUnit.DAYS): 7, - (TimeUnit.HOURS): 7 * 24, - (TimeUnit.MINUTES): 7 * 24 * 60, - (TimeUnit.SECONDS): (long) 7 * 24 * 60 * 60, + (TimeUnit.DAYS) : 7, + (TimeUnit.HOURS) : 7 * 24, + (TimeUnit.MINUTES) : 7 * 24 * 60, + (TimeUnit.SECONDS) : (long) 7 * 24 * 60 * 60, (TimeUnit.MILLISECONDS): (long) 7 * 24 * 60 * 60 * 1000, (TimeUnit.MICROSECONDS): (long) 7 * 24 * 60 * 60 * 1000 * 1000, - (TimeUnit.NANOSECONDS): (long) 7 * 24 * 60 * 60 * 1000 * 1000 * 1000, + (TimeUnit.NANOSECONDS) : (long) 7 * 24 * 60 * 60 * 1000 * 1000 * 1000, ] // Act @@ -174,7 +174,7 @@ class TestFormatUtilsGroovy extends GroovyTestCase { logger.converted(oneWeekInOtherUnits) // Assert - oneWeekInOtherUnits.each {TimeUnit k, double value -> + oneWeekInOtherUnits.each { TimeUnit k, double value -> assert value == ONE_WEEK_IN_OTHER_UNITS[k] } } @@ -187,13 +187,13 @@ class TestFormatUtilsGroovy extends GroovyTestCase { // Arrange final String ONE_AND_A_HALF_WEEKS = "1.5 week" final Map ONE_POINT_FIVE_WEEKS_IN_OTHER_UNITS = [ - (TimeUnit.DAYS): 7, - (TimeUnit.HOURS): 7 * 24, - (TimeUnit.MINUTES): 7 * 24 * 60, - (TimeUnit.SECONDS): (long) 7 * 24 * 60 * 60, + (TimeUnit.DAYS) : 7, + (TimeUnit.HOURS) : 7 * 24, + (TimeUnit.MINUTES) : 7 * 24 * 60, + (TimeUnit.SECONDS) : (long) 7 * 24 * 60 * 60, (TimeUnit.MILLISECONDS): (long) 7 * 24 * 60 * 60 * 1000, (TimeUnit.MICROSECONDS): (long) 7 * 24 * 60 * 60 * 1000 * 1000, - (TimeUnit.NANOSECONDS): (long) 7 * 24 * 60 * 60 * 1000 * 1000 * 1000, + (TimeUnit.NANOSECONDS) : (long) 7 * 24 * 60 * 60 * 1000 * 1000 * 1000, ].collectEntries { k, v -> [k, v * 1.5] } // Act @@ -203,7 +203,7 @@ class TestFormatUtilsGroovy extends GroovyTestCase { logger.converted(onePointFiveWeeksInOtherUnits) // Assert - onePointFiveWeeksInOtherUnits.each {TimeUnit k, double value -> + onePointFiveWeeksInOtherUnits.each { TimeUnit k, double value -> assert value == ONE_POINT_FIVE_WEEKS_IN_OTHER_UNITS[k] } } @@ -234,6 +234,33 @@ class TestFormatUtilsGroovy extends GroovyTestCase { assert parsedDecimalMillis.every { it == EXPECTED_MILLIS } } + /** + * Positive flow test for decimal inputs that are extremely small + */ + @Test + void testGetPreciseTimeDurationShouldHandleSmallDecimalValues() { + // Arrange + final Map SCENARIOS = [ + "decimalNanos" : [originalUnits: TimeUnit.NANOSECONDS, expectedUnits: TimeUnit.NANOSECONDS, originalValue: 123.4, expectedValue: 123.0], + "lessThanOneNano" : [originalUnits: TimeUnit.NANOSECONDS, expectedUnits: TimeUnit.NANOSECONDS, originalValue: 0.9, expectedValue: 1], + "lessThanOneNanoToMillis": [originalUnits: TimeUnit.NANOSECONDS, expectedUnits: TimeUnit.MILLISECONDS, originalValue: 0.9, expectedValue: 0], + "decimalMillisToNanos" : [originalUnits: TimeUnit.MILLISECONDS, expectedUnits: TimeUnit.NANOSECONDS, originalValue: 123.4, expectedValue: 123_400_000], + ] + + // Act + Map results = SCENARIOS.collectEntries { String k, Map values -> + logger.debug("Evaluating ${k}: ${values}") + String input = "${values.originalValue} ${values.originalUnits.name()}" + [k, FormatUtils.getPreciseTimeDuration(input, values.expectedUnits)] + } + logger.info(results) + + // Assert + results.every { String key, double value -> + assert value == SCENARIOS[key].expectedValue + } + } + /** * Positive flow test for decimal inputs that can be converted (all equal values) */ @@ -264,11 +291,11 @@ class TestFormatUtilsGroovy extends GroovyTestCase { void testMakeWholeNumberTimeShouldHandleMetricConversions() { // Arrange final Map SCENARIOS = [ - "secondsToMillis" : [originalUnits: TimeUnit.SECONDS, expectedUnits: TimeUnit.MILLISECONDS, expectedValue: 123_400, originalValue: 123.4], - "secondsToMicros" : [originalUnits: TimeUnit.SECONDS, expectedUnits: TimeUnit.MICROSECONDS, originalValue: 1.000_345, expectedValue: 1_000_345], - "millisToNanos" : [originalUnits: TimeUnit.MILLISECONDS, expectedUnits: TimeUnit.NANOSECONDS, originalValue: 0.75, expectedValue: 750_000], - "nanosToNanosGE1" : [originalUnits: TimeUnit.NANOSECONDS, expectedUnits: TimeUnit.NANOSECONDS, originalValue: 123.4, expectedValue: 123], - "nanosToNanosLE1" : [originalUnits: TimeUnit.NANOSECONDS, expectedUnits: TimeUnit.NANOSECONDS, originalValue: 0.123, expectedValue: 1], + "secondsToMillis": [originalUnits: TimeUnit.SECONDS, expectedUnits: TimeUnit.MILLISECONDS, expectedValue: 123_400, originalValue: 123.4], + "secondsToMicros": [originalUnits: TimeUnit.SECONDS, expectedUnits: TimeUnit.MICROSECONDS, originalValue: 1.000_345, expectedValue: 1_000_345], + "millisToNanos" : [originalUnits: TimeUnit.MILLISECONDS, expectedUnits: TimeUnit.NANOSECONDS, originalValue: 0.75, expectedValue: 750_000], + "nanosToNanosGE1": [originalUnits: TimeUnit.NANOSECONDS, expectedUnits: TimeUnit.NANOSECONDS, originalValue: 123.4, expectedValue: 123], + "nanosToNanosLE1": [originalUnits: TimeUnit.NANOSECONDS, expectedUnits: TimeUnit.NANOSECONDS, originalValue: 0.123, expectedValue: 1], ] // Act @@ -292,9 +319,9 @@ class TestFormatUtilsGroovy extends GroovyTestCase { void testMakeWholeNumberTimeShouldHandleNonMetricConversions() { // Arrange final Map SCENARIOS = [ - "daysToHours" : [originalUnits: TimeUnit.DAYS, expectedUnits: TimeUnit.HOURS, expectedValue: 36, originalValue: 1.5], + "daysToHours" : [originalUnits: TimeUnit.DAYS, expectedUnits: TimeUnit.HOURS, expectedValue: 36, originalValue: 1.5], "hoursToMinutes" : [originalUnits: TimeUnit.HOURS, expectedUnits: TimeUnit.MINUTES, originalValue: 1.5, expectedValue: 90], - "hoursToMinutes2" : [originalUnits: TimeUnit.HOURS, expectedUnits: TimeUnit.MINUTES, originalValue: 0.75, expectedValue: 45], + "hoursToMinutes2": [originalUnits: TimeUnit.HOURS, expectedUnits: TimeUnit.MINUTES, originalValue: 0.75, expectedValue: 45], ] // Act From ed51f48a3bee3750864b1415b5e85b42422ecaaf Mon Sep 17 00:00:00 2001 From: Andy LoPresto Date: Fri, 30 Nov 2018 12:13:57 -0800 Subject: [PATCH 6/9] NIFI-5854 FormatUtils#getTimeDuration() returns long but now accepts decimal inputs. Added @Deprecation warnings (will update callers where possible). All unit tests pass. --- .../org/apache/nifi/util/FormatUtils.java | 68 +++---------------- 1 file changed, 10 insertions(+), 58 deletions(-) diff --git a/nifi-commons/nifi-utils/src/main/java/org/apache/nifi/util/FormatUtils.java b/nifi-commons/nifi-utils/src/main/java/org/apache/nifi/util/FormatUtils.java index b63ab2c1ea0b..ed3a8f9e71b3 100644 --- a/nifi-commons/nifi-utils/src/main/java/org/apache/nifi/util/FormatUtils.java +++ b/nifi-commons/nifi-utils/src/main/java/org/apache/nifi/util/FormatUtils.java @@ -141,65 +141,17 @@ public static String formatDataSize(final double dataSize) { return format.format(dataSize) + " bytes"; } + /** + * Returns a time duration in the requested {@link TimeUnit} after parsing the {@code String} input. If the resulting value is a decimal (i.e. {@code 25 hours -> TimeUnit.DAYS = 1.04}), the value is rounded. + * + * @param value the raw String input (i.e. "28 minutes") + * @param desiredUnit the requested output {@link TimeUnit} + * @return the whole number value of this duration in the requested units + * @deprecated As of Apache NiFi 1.9.0, because this method only returns whole numbers, use {@link #getPreciseTimeDuration(String, TimeUnit)} when possible. + */ + @Deprecated public static long getTimeDuration(final String value, final TimeUnit desiredUnit) { - final Matcher matcher = TIME_DURATION_PATTERN.matcher(value.toLowerCase()); - if (!matcher.matches()) { - throw new IllegalArgumentException("Value '" + value + "' is not a valid time duration"); - } - - final String duration = matcher.group(1); - final String units = matcher.group(2); - TimeUnit specifiedTimeUnit = null; - switch (units.toLowerCase()) { - case "ns": - case "nano": - case "nanos": - case "nanoseconds": - specifiedTimeUnit = TimeUnit.NANOSECONDS; - break; - case "ms": - case "milli": - case "millis": - case "milliseconds": - specifiedTimeUnit = TimeUnit.MILLISECONDS; - break; - case "s": - case "sec": - case "secs": - case "second": - case "seconds": - specifiedTimeUnit = TimeUnit.SECONDS; - break; - case "m": - case "min": - case "mins": - case "minute": - case "minutes": - specifiedTimeUnit = TimeUnit.MINUTES; - break; - case "h": - case "hr": - case "hrs": - case "hour": - case "hours": - specifiedTimeUnit = TimeUnit.HOURS; - break; - case "d": - case "day": - case "days": - specifiedTimeUnit = TimeUnit.DAYS; - break; - case "w": - case "wk": - case "wks": - case "week": - case "weeks": - final long durationVal = Long.parseLong(duration); - return desiredUnit.convert(durationVal, TimeUnit.DAYS) * 7; - } - - final long durationVal = Long.parseLong(duration); - return desiredUnit.convert(durationVal, specifiedTimeUnit); + return Math.round(getPreciseTimeDuration(value, desiredUnit)); } /** From 867234c1e99dc4308b56d4fd0f6f677c8a2c9408 Mon Sep 17 00:00:00 2001 From: Andy LoPresto Date: Fri, 30 Nov 2018 14:27:21 -0800 Subject: [PATCH 7/9] NIFI-5854 Fixed unit tests (ran in IDE but not Maven) due to int overflows. Fixed checkstyle issues. --- .../main/java/org/apache/nifi/util/FormatUtils.java | 12 +++++++++--- .../apache/nifi/util/TestFormatUtilsGroovy.groovy | 10 +++++----- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/nifi-commons/nifi-utils/src/main/java/org/apache/nifi/util/FormatUtils.java b/nifi-commons/nifi-utils/src/main/java/org/apache/nifi/util/FormatUtils.java index ed3a8f9e71b3..ed42c1434e38 100644 --- a/nifi-commons/nifi-utils/src/main/java/org/apache/nifi/util/FormatUtils.java +++ b/nifi-commons/nifi-utils/src/main/java/org/apache/nifi/util/FormatUtils.java @@ -142,7 +142,9 @@ public static String formatDataSize(final double dataSize) { } /** - * Returns a time duration in the requested {@link TimeUnit} after parsing the {@code String} input. If the resulting value is a decimal (i.e. {@code 25 hours -> TimeUnit.DAYS = 1.04}), the value is rounded. + * Returns a time duration in the requested {@link TimeUnit} after parsing the {@code String} + * input. If the resulting value is a decimal (i.e. + * {@code 25 hours -> TimeUnit.DAYS = 1.04}), the value is rounded. * * @param value the raw String input (i.e. "28 minutes") * @param desiredUnit the requested output {@link TimeUnit} @@ -279,7 +281,9 @@ protected static long calculateMultiplier(TimeUnit originalTimeUnit, TimeUnit ne } /** - * Returns the next smallest {@link TimeUnit} (i.e. {@code TimeUnit.DAYS -> TimeUnit.HOURS}). If the parameter is {@code null} or {@code TimeUnit.NANOSECONDS}, an {@link IllegalArgumentException} is thrown because there is no valid smaller TimeUnit. + * Returns the next smallest {@link TimeUnit} (i.e. {@code TimeUnit.DAYS -> TimeUnit.HOURS}). + * If the parameter is {@code null} or {@code TimeUnit.NANOSECONDS}, an + * {@link IllegalArgumentException} is thrown because there is no valid smaller TimeUnit. * * @param originalUnit the TimeUnit * @return the next smaller TimeUnit @@ -312,7 +316,9 @@ protected static boolean isWeek(final String rawUnit) { } /** - * Returns the {@link TimeUnit} enum that maps to the provided raw {@code String} input. The highest time unit is {@code TimeUnit.DAYS}. Any input that cannot be parsed will result in an {@link IllegalArgumentException}. + * Returns the {@link TimeUnit} enum that maps to the provided raw {@code String} input. The + * highest time unit is {@code TimeUnit.DAYS}. Any input that cannot be parsed will result in + * an {@link IllegalArgumentException}. * * @param rawUnit the String to parse * @return the TimeUnit diff --git a/nifi-commons/nifi-utils/src/test/groovy/org/apache/nifi/util/TestFormatUtilsGroovy.groovy b/nifi-commons/nifi-utils/src/test/groovy/org/apache/nifi/util/TestFormatUtilsGroovy.groovy index 04a9b436c331..8fc9d0c1937d 100644 --- a/nifi-commons/nifi-utils/src/test/groovy/org/apache/nifi/util/TestFormatUtilsGroovy.groovy +++ b/nifi-commons/nifi-utils/src/test/groovy/org/apache/nifi/util/TestFormatUtilsGroovy.groovy @@ -163,8 +163,8 @@ class TestFormatUtilsGroovy extends GroovyTestCase { (TimeUnit.MINUTES) : 7 * 24 * 60, (TimeUnit.SECONDS) : (long) 7 * 24 * 60 * 60, (TimeUnit.MILLISECONDS): (long) 7 * 24 * 60 * 60 * 1000, - (TimeUnit.MICROSECONDS): (long) 7 * 24 * 60 * 60 * 1000 * 1000, - (TimeUnit.NANOSECONDS) : (long) 7 * 24 * 60 * 60 * 1000 * 1000 * 1000, + (TimeUnit.MICROSECONDS): (long) 7 * 24 * 60 * 60 * ((long) 1000 * 1000), + (TimeUnit.NANOSECONDS) : (long) 7 * 24 * 60 * 60 * ((long) 1000 * 1000 * 1000), ] // Act @@ -192,8 +192,8 @@ class TestFormatUtilsGroovy extends GroovyTestCase { (TimeUnit.MINUTES) : 7 * 24 * 60, (TimeUnit.SECONDS) : (long) 7 * 24 * 60 * 60, (TimeUnit.MILLISECONDS): (long) 7 * 24 * 60 * 60 * 1000, - (TimeUnit.MICROSECONDS): (long) 7 * 24 * 60 * 60 * 1000 * 1000, - (TimeUnit.NANOSECONDS) : (long) 7 * 24 * 60 * 60 * 1000 * 1000 * 1000, + (TimeUnit.MICROSECONDS): (long) 7 * 24 * 60 * 60 * ((long) 1000 * 1000), + (TimeUnit.NANOSECONDS) : (long) 7 * 24 * 60 * 60 * ((long) 1000 * 1000 * 1000), ].collectEntries { k, v -> [k, v * 1.5] } // Act @@ -435,7 +435,7 @@ class TestFormatUtilsGroovy extends GroovyTestCase { void testShouldCalculateMultiplier() { // Arrange final Map SCENARIOS = [ - "allUnits" : [original: TimeUnit.DAYS, destination: TimeUnit.NANOSECONDS, expectedMultiplier: (long) 24 * 60 * 60 * 1_000_000_000], + "allUnits" : [original: TimeUnit.DAYS, destination: TimeUnit.NANOSECONDS, expectedMultiplier: (long) 24 * 60 * 60 * (long) 1_000_000_000], "microsToNanos" : [original: TimeUnit.MICROSECONDS, destination: TimeUnit.NANOSECONDS, expectedMultiplier: 1_000], "millisToNanos" : [original: TimeUnit.MILLISECONDS, destination: TimeUnit.NANOSECONDS, expectedMultiplier: 1_000_000], "millisToMicros": [original: TimeUnit.MILLISECONDS, destination: TimeUnit.MICROSECONDS, expectedMultiplier: 1_000], From f11c852580e9203b4117c803bfe76be7dfd7abbc Mon Sep 17 00:00:00 2001 From: Andy LoPresto Date: Fri, 30 Nov 2018 14:30:47 -0800 Subject: [PATCH 8/9] NIFI-5854 Fixed typo in Javadoc. --- .../src/main/java/org/apache/nifi/util/FormatUtils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nifi-commons/nifi-utils/src/main/java/org/apache/nifi/util/FormatUtils.java b/nifi-commons/nifi-utils/src/main/java/org/apache/nifi/util/FormatUtils.java index ed42c1434e38..b90bfb404a12 100644 --- a/nifi-commons/nifi-utils/src/main/java/org/apache/nifi/util/FormatUtils.java +++ b/nifi-commons/nifi-utils/src/main/java/org/apache/nifi/util/FormatUtils.java @@ -226,7 +226,7 @@ public static double getPreciseTimeDuration(final String value, final TimeUnit d *

* 1, {@code TimeUnit.SECONDS} -> [1, {@code TimeUnit.SECONDS}] * 1.1, {@code TimeUnit.SECONDS} -> [1100, {@code TimeUnit.MILLISECONDS}] - * 0.1, {@code TimeUnit.SECONDS} -> [1000, {@code TimeUnit.MILLISECONDS}] + * 0.1, {@code TimeUnit.SECONDS} -> [100, {@code TimeUnit.MILLISECONDS}] * 0.1, {@code TimeUnit.NANOSECONDS} -> [1, {@code TimeUnit.NANOSECONDS}] * * @param decimal the time duration as a decimal From 9c267c91f637c50f1d178364ef4d9f6578df275f Mon Sep 17 00:00:00 2001 From: Andy LoPresto Date: Fri, 30 Nov 2018 14:32:29 -0800 Subject: [PATCH 9/9] NIFI-5854 Fixed typo in Javadoc. --- .../src/main/java/org/apache/nifi/util/FormatUtils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nifi-commons/nifi-utils/src/main/java/org/apache/nifi/util/FormatUtils.java b/nifi-commons/nifi-utils/src/main/java/org/apache/nifi/util/FormatUtils.java index b90bfb404a12..7d2992f34a3d 100644 --- a/nifi-commons/nifi-utils/src/main/java/org/apache/nifi/util/FormatUtils.java +++ b/nifi-commons/nifi-utils/src/main/java/org/apache/nifi/util/FormatUtils.java @@ -258,7 +258,7 @@ protected static List makeWholeNumberTime(double decimal, TimeUnit timeU /** * Returns the numerical multiplier to convert a value from {@code originalTimeUnit} to * {@code newTimeUnit} (i.e. for {@code TimeUnit.DAYS -> TimeUnit.MINUTES} would return - * 24 * 60 = 720). If the original and new units are the same, returns 1. If the new unit + * 24 * 60 = 1440). If the original and new units are the same, returns 1. If the new unit * is larger than the original (i.e. the result would be less than 1), throws an * {@link IllegalArgumentException}. *