revert back the rate format to ISO-8601

Author: jeetchoudhary

dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/RateLimit.java

@@ -1,32 +1,32 @@
 package com.salesforce.dockerfileimageupdate.utils;
 
 import java.time.Duration;
+import java.time.format.DateTimeParseException;
 import java.util.UnknownFormatConversionException;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
 import org.apache.commons.lang3.StringUtils;
-import org.apache.commons.lang3.math.NumberUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
- * Creating this as inner class as existence of this class outside
- * RateLimiter class will make a little sense. This class is solely
- * meant to encapsulate the primitives needed for the RateLimiter
- * and logic for populating those after tokenizing the input
+ * Class meant to tokenize input and encapsulate the primitives needed for
+ * the RateLimiter
+ *
+ * @see RateLimiter
  */
 public class RateLimit {
     public static final String ERROR_MESSAGE = "Unexpected format or unit encountered, valid input is " +
-            "<integer>-per-<time-unit> where time-init is one of 's', 'm', or 'h'. " +
-            "Example: 500-per-h";
-    private static final Pattern eventPattern =
-            Pattern.compile("(^[1-9]\\d{0,18})(-per-)?([1-9]\\d{0,18})*([smh]?)", Pattern.CASE_INSENSITIVE);
+            "<integer>-<ISO-8601_formatted_time> Example: 500-PT1S means(500 per second) \n" +
+            "500-PT1M means(500 per Minute)";
+    private static final Logger log = LoggerFactory.getLogger(RateLimit.class);
     private final long rate; //Maximum count of PRs to be sent per rLimitDuration
     private final Duration duration; //Rate Limit duration
     private final Duration tokenAddingRate; //Rate at which tokens are added in the bucket
 
     /**
      * Initialize RateLimit object with based on parameters passed
-     * @param rate Maximum count of PRs to be sent per rLimitDuration
-     * @param duration Rate Limit duration
+     *
+     * @param rate            Maximum count of PRs to be sent per rLimitDuration
+     * @param duration        Rate Limit duration
      * @param tokenAddingRate Rate at which tokens are added in the bucket
      */
     public RateLimit(long rate, Duration duration, Duration tokenAddingRate) {
@@ -43,68 +43,50 @@ public RateLimit() {
                 Constants.DEFAULT_TOKEN_ADDING_RATE);
     }
 
-    public static RateLimit tokenizeAndGetRateLimit(String input) {
-
-        if (input == null) {
+    /**
+     * This method will accept an input string in format <integer>-<ISO-8601_formatted_time> and
+     * will tokenize it to create a RateLimit object.
+     * <pre>
+     * Examples of tokenization
+     * input : 500-PT1S -> {rate : 500, duration 1 second (PT1S), token adding rate: every .002 seconds (PT0.002S)}
+     *         60-PT1M -> {rate : 60, duration 1 minute (PT1M), token adding rate: every 1 seconds (PT1S)}
+     * </pre>
+     *
+     * @param input string in format <integer>-<ISO-8601 formatted_time]> example 500-PT1S , 500-PT60S , 500-PT1H
+     * @return RateLimit based on the input value
+     * @throws UnknownFormatConversionException if input is not a string with expected format <integer>-<ISO-8601_formatted_time>
+     */
+    public static RateLimit tokenizeAndGetRateLimit(String input) throws UnknownFormatConversionException {
+        if (StringUtils.isEmpty(input) || !input.contains("-")) {
             throw new UnknownFormatConversionException(ERROR_MESSAGE);
         }
 
-        long rLimit;
-        Duration rLimitDuration;
-        Duration tokAddingRate;
-        String literalConstant;
-        String rateDuration;
-        String rateDurationUnitChar;
+        int delimiterIndex = input.indexOf('-');
 
-        Matcher matcher = eventPattern.matcher(input);
+        try {
+            long inputRate = Long.parseLong(input.substring(0, delimiterIndex));
+//            "PT20.345S" -- parses as "20.345 seconds"
+//            "PT15M"     -- parses as "15 minutes" (where a minute is 60 seconds)
+//            "PT10H"     -- parses as "10 hours" (where an hour is 3600 seconds)
+//            "P2D"       -- parses as "2 days" (where a day is 24 hours or 86400
+            Duration inputDuration = Duration.parse(input.substring(delimiterIndex + 1));
 
-        if (!matcher.matches()) {
-            throw new UnknownFormatConversionException(ERROR_MESSAGE);
-        } else {
-            /**
-             * eventPattern regex is divided into 5 groups
-             * 0 group being the whole expression
-             * 1 group is numeric rate limit value and will always
-             *                          be present if exp matched
-             * 2 is literal constant '-per-' and is optional
-             * 3 is numeric rate duration and is optional
-             * 4 is a char(s/m/h) and is optional
-             */
-            rLimit = Long.parseLong(matcher.group(1)); // will always have a numeric value that fits in Long
-            literalConstant = matcher.group(2); // can be null or empty or '-per-'
-            rateDuration = matcher.group(3);// can be null or empty or numeric value
-            rateDurationUnitChar = matcher.group(4); //can be empty or char s,m,h
-        }
+//            Keeping token adding rate based on the lowest supported rate i.e. nanoseconds.
+//            means new tokens can be added to the rate limiter every nanosecond based on
+//            the input passed.
+//            Duration is auto casted to upper unit of time. so input of 600-PT60M
+//            will be set inputTokAddingRate to PT6S means every 6 seconds one token will be
+//            added to the pool to be consumed
+//
+            Duration inputTokAddingRate = Duration.ofNanos(inputDuration.toNanos() / inputRate);
 
-        if (StringUtils.isEmpty(literalConstant)) {
-            //return rate with default duration and token adding rate
-            return new RateLimit(rLimit, Constants.DEFAULT_RATE_LIMIT_DURATION, Constants.DEFAULT_TOKEN_ADDING_RATE);
+            log.info("constructing rate limit object with rate: {}, duration: {}, and token adding rate: {}",
+                    inputRate, inputDuration, inputTokAddingRate);
+            return new RateLimit(inputRate, inputDuration, inputTokAddingRate);
+        } catch (NumberFormatException | DateTimeParseException ex) {
+            throw new UnknownFormatConversionException(ERROR_MESSAGE);
         }
 
-        // value is either going to be a valid number or null. defaulting to 1 unit(every hour/every min and so on)
-        long duration = NumberUtils.isParsable(rateDuration) ? Long.parseLong(rateDuration) : 1;
-
-        switch (rateDurationUnitChar) {
-            case "s":
-            case "S":
-                rLimitDuration = Duration.ofSeconds(duration);
-                tokAddingRate = Duration.ofSeconds(rLimit / duration);
-                break;
-            case "m":
-            case "M":
-                rLimitDuration = Duration.ofMinutes(duration);
-                tokAddingRate = Duration.ofMinutes(rLimit / duration);
-                break;
-            case "h":
-            case "H":
-                rLimitDuration = Duration.ofHours(duration);
-                tokAddingRate = Duration.ofHours(rLimit / duration);
-                break;
-            default:
-                //should not reach here are regex will enforce char, keeping it for any unexpected use case.
-                throw new UnknownFormatConversionException(ERROR_MESSAGE);
-        }
-        return new RateLimit(rLimit, rLimitDuration, tokAddingRate);
     }
 
     public long getRate() {

dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/utils/RateLimitTest.java

@@ -4,18 +4,20 @@
 import java.util.UnknownFormatConversionException;
 import org.testng.annotations.DataProvider;
 import org.testng.annotations.Test;
-import static org.testng.Assert.*;
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertThrows;
 
 public class RateLimitTest {
     @DataProvider(name = "rateLimitInputSuccessData")
     public static Object[][] rateLimitInputSuccessData() {
         return new Object[][]{
-                {"500", 500, Constants.DEFAULT_RATE_LIMIT_DURATION, Constants.DEFAULT_TOKEN_ADDING_RATE},
-                {"500-per-s", 500, Duration.ofSeconds(1), Duration.ofSeconds(500)},
-                {"500-per-60s", 500, Duration.ofSeconds(60), Duration.ofSeconds(500 / 60)},
-                {"500-per-1m", 500, Duration.ofMinutes(1), Duration.ofMinutes(500)},
-                {"500-per-1h", 500, Duration.ofHours(1), Duration.ofHours(500)},
-                {"1234-per-2h", 1234, Duration.ofHours(2), Duration.ofHours(1234 / 2)},
+                {"500-PT1S", 500, "PT1S", "PT0.002S"},
+                {"500-PT60S", 500, "PT1M", "PT0.12S"},
+                {"500-PT1M", 500, "PT1M", "PT0.12S"},
+                {"500-PT1H", 500, "PT1H", "PT7.2S"},
+                {"500-PT6H", 500, "PT6H", "PT43.2S"},
+                {"600-PT1H", 600, "PT1H", "PT6S"},
+                {"86400-PT24H", 86400, "PT24H", "PT1S"},
         };
     }
 
@@ -31,6 +33,8 @@ public static Object[][] rateLimitInputFailureData() {
                 {null, UnknownFormatConversionException.class},
                 {"2dse2", UnknownFormatConversionException.class},
                 {"null", UnknownFormatConversionException.class},
+                {"500-per-s", UnknownFormatConversionException.class},
+                {"500-PT1H-PT1H", UnknownFormatConversionException.class},
 
         };
     }
@@ -41,10 +45,10 @@ public void testingFailureTokenizingOfInputString(String rateLimitInputStr, Clas
     }
 
     @Test(dataProvider = "rateLimitInputSuccessData")
-    public void testingSuccessTokenizingOfInputString(String rateLimitInputStr, int rateLimitInputArg, Duration rateLimitDurationInputArg, Duration tokenAddingRateInputArg) {
+    public void testingSuccessTokenizingOfInputString(String rateLimitInputStr, long rateLimitInputArg, String rlDuration, String rlTokAddingRate) {
         RateLimit rateLimit = RateLimit.tokenizeAndGetRateLimit(rateLimitInputStr);
         assertEquals(rateLimit.getRate(), rateLimitInputArg);
-        assertEquals(rateLimit.getDuration(), rateLimitDurationInputArg);
-        assertEquals(rateLimit.getTokenAddingRate(), tokenAddingRateInputArg);
+        assertEquals(rateLimit.getDuration(), Duration.parse(rlDuration));
+        assertEquals(rateLimit.getTokenAddingRate(), Duration.parse(rlTokAddingRate));
     }
 }

dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/utils/RateLimiterTest.java

@@ -23,14 +23,15 @@ public static Object[][] getRateLimitingData() {
     @DataProvider(name = "dataForTestingRateLimitingEventCreation")
     public static Object[][] getDataForTestingRateLimitingEventCreation() {
         return new Object[][]{
-                {"1234-per-2h", RateLimiter.class, false},
-                {"500", RateLimiter.class, false},
-                {"500-per-s", RateLimiter.class, false},
-                {"500-per-60s", RateLimiter.class, false},
+                {"500-PT1S", RateLimiter.class, false},
+                {"500-PT60S", RateLimiter.class, false},
+                {"500-PT1M", RateLimiter.class, false},
+                {"500-PT1H", RateLimiter.class, false},
                 {"image", null, true},
                 {"500-random", null, true},
                 {"", null, true},
-                {"null", null, true}
+                {"null", null, true},
+                {"500", null, true}
         };
     }