Fix PollingTime parsing to support comma in RSQL (#2791)

The PollingTime now supports all RSQL filters that doesn't contain '->'
For duration HH:mm:ss and ISO-8601 is supported
For deviation 0-99% are suppported (as before)

Signed-off-by: Avgustin Marinov <Avgustin.Marinov@bosch.com>

hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/tenancy/configuration/ControllerPollProperties.java

@@ -28,24 +28,28 @@ public class ControllerPollProperties implements Serializable {
     private static final long serialVersionUID = 1L;
 
     /**
-     * Maximum polling time that can be configured system-wide and by tenant in HH:mm:ss notation.
+     * Minimum polling time that can be configured by a tenant in HH:mm:ss or ISO-8601 notation.
-     */
-    private String maxPollingTime = "23:59:59";
-
-    /**
-     * Minimum polling time that can be configured by a tenant in HH:mm:ss notation.
      */
     private String minPollingTime = "00:00:30";
 
     /**
-     * Controller polling time that can be configured system-wide and by tenant in HH:mm:ss(~\d{1,2}%)? notation, plus
+     * Maximum polling time that can be configured system-wide and by tenant in HH:mm:ss or ISO-8601 notation.
+     */
+    private String maxPollingTime = "23:59:59";
+
+    /**
+     * Controller polling time that can be configured system-wide and by tenant in (HH:mm:ss|ISO-8601)(~\d{1,2}%)? notation, plus
      * followed (optionally and ordered) by a comma separated @lt;QL filter@gt; -@gt; polling time that overrides the
      * default polling time for the targets that match the filter.
+     * <p/>
+     * If the computed polling time returned by DDI to device (i.e. poling time + deviation) is not less then a day  then it will be
+     * sent to device in ISO-8601 format. This may brake backward compatibility for devices expecting polling time in HH:mm:ss format.
+     * In order to prevent this, for legacy devices, keep the maxPollingTime less than a day.
      */
     private String pollingTime = "00:05:00";
 
     /**
-     * Controller polling overdue time that can be configured system-wide and by tenant in HH:mm:ss notation.
+     * Controller polling overdue time that can be configured system-wide and by tenant in HH:mm:ss or ISO-8601 notation.
      */
     private String pollingOverdueTime = "00:05:00";
 

hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/tenancy/configuration/DurationHelper.java

@@ -18,39 +18,36 @@ import lombok.NoArgsConstructor;
 
 /**
  * This class is a helper for converting a duration into a string and for the other way. The string is in the format expected
- * in configuration and database - in {@link Duration} default format or in custom format like "01:00:00" or "01:01:50:50"
+ * in configuration and database - in {@link Duration} default format or in custom format like "01:00:00" or standard ISO-8601.
- * (starting with seconds, minutes, hours, days from the end).
  */
 @NoArgsConstructor(access = lombok.AccessLevel.PRIVATE)
 public final class DurationHelper {
 
     private static final DateTimeFormatter DURATION_FORMATER = DateTimeFormatter.ofPattern("HH:mm:ss");
-    private static final long SECONDS_PER_DAY = 24 * 60 * 60L; // 24 hours * 60 minutes * 60 seconds
     private static final Duration DAY = Duration.ofDays(1);
 
     /**
      * Converts a Duration into a formatted String
      *
      * @param duration duration, which will be converted into a formatted String
-     * @return String in the duration format, specified as HH:mm:ss or d+:HH:mm:ss
+     * @return String in the duration format, specified as HH:mm:ss (of possible, i.e. less than a day) or ISO-8601 format
      */
     public static String toString(final Duration duration) {
         if (duration == null) {
             return null;
         }
 
-        if (duration.compareTo(DAY) < 0) { // backward compatible HH:mm:ss
+        if (duration.compareTo(DAY) < 0) { // backward compatible HH:mm:ss (if possible, could be sent to devices via DDI)
             return LocalTime.ofSecondOfDay(duration.toSeconds()).format(DURATION_FORMATER);
-        } else { // custom format d+:HH:mm:ss
+        } else { // ISO-8601
-            return duration.toDays() + ":" + LocalTime.ofSecondOfDay(duration.toSeconds() % SECONDS_PER_DAY).format(DURATION_FORMATER);
+            return duration.toString();
         }
     }
 
     /**
      * Converts a formatted String into a Duration object.
      *
-     * @param durationStr String in {@link Duration} default format or in custom format like "01:00:00" or "01:01:50:50"
+     * @param durationStr String in {@link Duration} default format or in custom format like "01:00:00" or standard ISO-8601
-     *                    (starting with seconds, minutes, hours, days from the end)
      * @return duration as a {@link Duration} object
      * @throws DateTimeParseException when String is in wrong format
      */
@@ -76,12 +73,6 @@ public final class DurationHelper {
                         .ofHours(Long.parseLong(split[0]))
                         .plusMinutes(Long.parseLong(split[1]))
                         .plusSeconds(Long.parseLong(split[2]));
-            } else if (split.length == 4) { // d:HH:mm:ss
-                return Duration
-                        .ofDays(Long.parseLong(split[0]))
-                        .plusHours(Long.parseLong(split[1]))
-                        .plusMinutes(Long.parseLong(split[2]))
-                        .plusSeconds(Long.parseLong(split[3]));
             } else {
                 throw new IllegalArgumentException("No more then 4 chunks (split by ':') are allowed in duration");
             }

hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/tenancy/configuration/PollingTime.java

@@ -10,9 +10,7 @@
 package org.eclipse.hawkbit.tenancy.configuration;
 
 import java.time.Duration;
-import java.time.format.DateTimeParseException;
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
 import java.util.Random;
@@ -25,31 +23,35 @@ import org.eclipse.hawkbit.repository.exception.TenantConfigurationValidatorExce
 @Value
 public class PollingTime {
 
-    private static final Pattern OVERRIDE_PATTERN = Pattern.compile(
-            "\\s{0,5},\\s{0,5}(?<qlStr>[^,]*)\\s{0,5}->\\s{0,5}(?<pollInterval>" + PollingInterval.POLLING_INTERVALE_REGEX + ")\\s{0,5}");
-
     PollingInterval pollingInterval;
     List<Override> overrides;
 
+    @SuppressWarnings("java:S127")
     public PollingTime(final String pollingTime) {
         final int indexOfComma = pollingTime.indexOf(',');
         if (indexOfComma == -1) { // no overrides
             pollingInterval = new PollingInterval(pollingTime);
-            overrides = Collections.emptyList();
+            overrides = List.of();
         } else {
             // Extract the main polling interval and overrides
             final String pollingIntervalStr = pollingTime.substring(0, indexOfComma);
             pollingInterval = new PollingInterval(pollingIntervalStr);
             overrides = new ArrayList<>();
-            final String overridesStr = pollingTime.substring(indexOfComma).trim(); // with initial comma
+            final String overridesStr = pollingTime.substring(indexOfComma + 1).trim();
-            final Matcher overridesMatcher = OVERRIDE_PATTERN.matcher(overridesStr);
+            for (int start = 0; ; ) {
-            for (int start = 0; start < overridesStr.length(); start = overridesMatcher.end()) {
+                final int separatorIndex = overridesStr.indexOf("->", start);
-                if (overridesMatcher.find(start)) {
+                if (separatorIndex == -1) {
-                    overrides.add(new Override(
+                    throw new TenantConfigurationValidatorException("Invalid pollingTime override: '" + overridesStr.substring(start) + "'");
-                            overridesMatcher.group("qlStr").trim(),
-                            new PollingInterval(overridesMatcher.group("pollInterval").trim())));
                 } else {
-                    throw new TenantConfigurationValidatorException("Invalid pollingTime overrides: " + overridesStr);
+                    final String ql = overridesStr.substring(start, separatorIndex).trim();
+                    final int nextCommaIndex = overridesStr.indexOf(',', separatorIndex);
+                    if (nextCommaIndex == -1) { // last override
+                        overrides.add(new Override(ql, new PollingInterval(overridesStr.substring(separatorIndex + 2).trim())));
+                        break;
+                    } else {
+                        overrides.add(new Override(ql, new PollingInterval(overridesStr.substring(separatorIndex + 2, nextCommaIndex).trim())));
+                        start = nextCommaIndex + 1;
+                    }
                 }
             }
         }
@@ -61,7 +63,7 @@ public class PollingTime {
         @SuppressWarnings("java:S1068") // used for random delay only, no need of secure random
         private static final Random RANDOM = new Random();
 
-        public static final String POLLING_INTERVALE_REGEX = "\\s{0,5}(?<pollingInterval>\\d{2}:[0-5]\\d:[0-5]\\d)\\s{0,5}(~(?<deviationPercent>\\d{1,2})%)?\\s{0,5}";
+        private static final String POLLING_INTERVALE_REGEX = "(?<pollingInterval>[^~]+)(~(?<deviationPercent>\\d{1,2})%)?\\s{0,5}";
         private static final Pattern POLLING_INTERVAL_PATTERN = Pattern.compile(POLLING_INTERVALE_REGEX);
 
         Duration interval;
@@ -69,16 +71,17 @@ public class PollingTime {
 
         public PollingInterval(final String pollingInterval) {
             final Matcher matcher = POLLING_INTERVAL_PATTERN.matcher(pollingInterval);
-            if (matcher.matches()) {
+            try {
-                try {
+                if (matcher.matches()) {
-                    this.interval = DurationHelper.fromString(matcher.group("pollingInterval"));
+                    interval = DurationHelper.fromString(matcher.group("pollingInterval").trim());
-                } catch (final DateTimeParseException ex) {
+                    deviationPercent = Optional.ofNullable(matcher.group("deviationPercent"))
-                    throw new TenantConfigurationValidatorException(
+                            .map(String::trim).map(Integer::parseInt).orElse(0);
-                            "The given configuration value is expected as a string in the format HH:mm:ss(~\\d{1,2})?.");
+                } else {
+                    throw new IllegalArgumentException();
                 }
-                this.deviationPercent = Optional.ofNullable(matcher.group("deviationPercent")).map(Integer::parseInt).orElse(0);
+            } catch (final Exception e) {
-            } else {
+                throw new TenantConfigurationValidatorException(
-                throw new TenantConfigurationValidatorException("Invalid pollingInterval: " + pollingInterval);
+                        "Invalid pollingInterval: '" + pollingInterval + "', expecting: (HH:mm:ss|ISO-8601)(~\\d{1,2}%)?");
             }
         }
 

hawkbit-repository/hawkbit-repository-api/src/test/java/org/eclipse/hawkbit/repository/PollingTimeTest.java

@@ -41,6 +41,7 @@ class PollingTimeTest {
     @Test
     void testComplexWithOverrides() {
         assertExpectedComplexWithOverrides("01:00:00~10%, group == 'eu' -> 00:02:00~15%, status != in_sync -> 00:05:00");
+        assertExpectedComplexWithOverrides("PT1H~10%, group == 'eu' -> PT2M~15%, status != in_sync -> PT5M");
     }
 
     @Test
@@ -48,16 +49,36 @@ class PollingTimeTest {
         assertExpectedComplexWithOverrides("01:00:00~10%, group == 'eu'  -> 00:02:00~15%, status != in_sync ->00:05:00");
         assertExpectedComplexWithOverrides(" 01:00:00~10%, group == 'eu'  -> 00:02:00~15%, status != in_sync ->00:05:00  ");
         assertExpectedComplexWithOverrides(" 01:00:00~10% , group == 'eu'  -> 00:02:00 ~15%, status != in_sync ->00:05:00  ");
+        assertExpectedComplexWithOverrides("PT1H~10%, group == 'eu'  -> PT2M~15%, status != in_sync ->PT5M");
+        assertExpectedComplexWithOverrides(" PT1H~10%, group == 'eu'  -> PT2M~15%, status != in_sync ->PT5M");
+        assertExpectedComplexWithOverrides(" PT1H~10% , group == 'eu'  -> PT2M~15%, status != in_sync ->PT5M");
+    }
+
+    @Test
+    void testComplexWithOverridesWithCommaInQl() {
+        assertExpectedComplexWithOverrides(
+                "01:00:00~10%, group == 'eu' -> 00:02:00~15%, status =in= (in_sync,pending) -> 00:05:00",
+                "group == 'eu'", "status =in= (in_sync,pending)");
+        assertExpectedComplexWithOverrides(
+                "PT1H~10%, group == 'eu' -> PT2M~15%, status =in= (in_sync,pending) -> PT5M",
+                "group == 'eu'", "status =in= (in_sync,pending)");
+        assertExpectedComplexWithOverrides(
+                "01:00:00~10%, group == 'eu' -> 00:02:00~15%, status =in= (in_sync,pending) -> 00:05:00, region == us -> 00:10:00",
+                "group == 'eu'", "status =in= (in_sync,pending)", "region == us");
     }
 
     private static void assertExpectedComplexWithOverrides(final String pollingTimeStr) {
+        assertExpectedComplexWithOverrides(pollingTimeStr, "group == 'eu'", "status != in_sync");
+    }
+
+    private static void assertExpectedComplexWithOverrides(final String pollingTimeStr, final String... expectedQls) {
         final PollingTime pollingTime = new PollingTime(pollingTimeStr);
         assertThat(pollingTime.getPollingInterval().getInterval()).hasToString("PT1H");
         assertThat(pollingTime.getPollingInterval().getDeviationPercent()).isEqualTo(10);
-        assertThat(pollingTime.getOverrides().get(0).qlStr()).isEqualTo("group == 'eu'");
+        assertThat(pollingTime.getOverrides().get(0).qlStr()).isEqualTo(expectedQls[0]);
         assertThat(pollingTime.getOverrides().get(0).pollingInterval().getInterval()).hasToString("PT2M");
         assertThat(pollingTime.getOverrides().get(0).pollingInterval().getDeviationPercent()).isEqualTo(15);
-        assertThat(pollingTime.getOverrides().get(1).qlStr()).isEqualTo("status != in_sync");
+        assertThat(pollingTime.getOverrides().get(1).qlStr()).isEqualTo(expectedQls[1]);
         assertThat(pollingTime.getOverrides().get(1).pollingInterval().getInterval()).hasToString("PT5M");
         assertThat(pollingTime.getOverrides().get(1).pollingInterval().getDeviationPercent()).isZero();
     }