Bugfix/submission tek rolling period (#752)

* Replaced possible variable rolling period with constant value * Add tests for flexible rolling period * apply sugested review changes * Add javaDoc comments for changed getExpiryDateTime * Fix midnight check in SubmissionPayload Validator Co-authored-by: Michael Burwig <michael.burwig@sap.com>
corona-warn-app · Sep 11, 2020 · aa0c624 · aa0c624
1 parent a4d90dd
commit aa0c624
Show file tree

Hide file tree

Showing 4 changed files with 59 additions and 2 deletions.
diff --git a/...ronawarn/server/services/distribution/assembly/diagnosiskeys/ProdDiagnosisKeyBundler.java b/...ronawarn/server/services/distribution/assembly/diagnosiskeys/ProdDiagnosisKeyBundler.java
@@ -98,11 +98,12 @@ private static Optional<LocalDateTime> getEarliestDistributableTimestamp(
 
   /**
    * Returns the end of the rolling time window that a {@link DiagnosisKey} was active for as a {@link LocalDateTime}.
+   * The ".plusDays(1L)" is used as there can be now diagnosis keys with rollingPeriod set to less than 1 day.
    */
   private LocalDateTime getExpiryDateTime(DiagnosisKey diagnosisKey) {
     return LocalDateTime
         .ofEpochSecond(diagnosisKey.getRollingStartIntervalNumber() * TEN_MINUTES_INTERVAL_SECONDS, 0, UTC)
-        .plusMinutes(diagnosisKey.getRollingPeriod() * 10L);
+        .plusDays(1L);
   }
 
   /**

diff --git a/...services/distribution/assembly/diagnosiskeys/ProdDiagnosisKeyBundlerExpiryPolicyTest.java b/...services/distribution/assembly/diagnosiskeys/ProdDiagnosisKeyBundlerExpiryPolicyTest.java
@@ -85,4 +85,47 @@ void testLastPeriodOfHourAndSubmissionGreaterDistributionDateTime() {
     bundler.setDiagnosisKeys(diagnosisKeys, LocalDateTime.of(1970, 1, 5, 0, 0));
     assertThat(bundler.getDiagnosisKeysForHour(LocalDateTime.of(1970, 1, 2, 4, 0, 0))).hasSize(10);
   }
+
+  @ParameterizedTest
+  @ValueSource(longs = {0L, 24L, 24L + 2L})
+  void testLastPeriodOfHourAndSubmissionLessThanDistributionDateTimeWithFlexibleRollingPeriod(
+      long submissionTimestamp) {
+    List<DiagnosisKey> diagnosisKeys = Helpers.buildDiagnosisKeysWithFlexibleRollingPeriod(5, submissionTimestamp, 5, 44);
+    diagnosisKeys.addAll(Helpers.buildDiagnosisKeysWithFlexibleRollingPeriod(5, submissionTimestamp, 5, 100));
+    bundler.setDiagnosisKeys(diagnosisKeys, LocalDateTime.of(1970, 1, 5, 0, 0));
+    assertThat(bundler.getDiagnosisKeysForHour(LocalDateTime.of(1970, 1, 2, 3, 0, 0))).hasSize(10);
+  }
+
+  @Test
+  void testLastPeriodOfHourAndSubmissionEqualsDistributionDateTimeWithFlexibleRollingPeriod() {
+    List<DiagnosisKey> diagnosisKeys = Helpers.buildDiagnosisKeysWithFlexibleRollingPeriod(5, 24L + 3L, 5, 44);
+    diagnosisKeys.addAll(Helpers.buildDiagnosisKeysWithFlexibleRollingPeriod(5, 24L + 3L, 5, 100));
+    bundler.setDiagnosisKeys(diagnosisKeys, LocalDateTime.of(1970, 1, 5, 0, 0));
+    assertThat(bundler.getDiagnosisKeysForHour(LocalDateTime.of(1970, 1, 2, 3, 0, 0))).hasSize(10);
+  }
+
+  @ParameterizedTest
+  @ValueSource(longs = {0L, 24L, 24L + 2L, 24L + 3L})
+  void testFirstPeriodOfHourAndSubmissionLessThanDistributionDateTimeWithFlexibleRollingPeriod(long submissionTimestamp) {
+    List<DiagnosisKey> diagnosisKeys = Helpers.buildDiagnosisKeysWithFlexibleRollingPeriod(6, submissionTimestamp, 5, 44);
+    diagnosisKeys.addAll(Helpers.buildDiagnosisKeysWithFlexibleRollingPeriod(6, submissionTimestamp, 5, 100));
+    bundler.setDiagnosisKeys(diagnosisKeys, LocalDateTime.of(1970, 1, 5, 0, 0));
+    assertThat(bundler.getDiagnosisKeysForHour(LocalDateTime.of(1970, 1, 2, 4, 0, 0))).hasSize(10);
+  }
+
+  @Test
+  void testFirstPeriodOfHourAndSubmissionEqualsDistributionDateTimeWithFlexibleRollingPeriod() {
+    List<DiagnosisKey> diagnosisKeys = Helpers.buildDiagnosisKeysWithFlexibleRollingPeriod(6, 24L + 4L, 5, 44);
+    diagnosisKeys.addAll(Helpers.buildDiagnosisKeysWithFlexibleRollingPeriod(6, 24L + 4L, 5, 100));
+    bundler.setDiagnosisKeys(diagnosisKeys, LocalDateTime.of(1970, 1, 5, 0, 0));
+    assertThat(bundler.getDiagnosisKeysForHour(LocalDateTime.of(1970, 1, 2, 4, 0, 0))).hasSize(10);
+  }
+
+  @Test
+  void testLastPeriodOfHourAndSubmissionGreaterDistributionDateTimeWithFlexibleRollingPeriod() {
+    List<DiagnosisKey> diagnosisKeys = Helpers.buildDiagnosisKeysWithFlexibleRollingPeriod(5, 24L + 4L, 5, 44);
+    diagnosisKeys.addAll(Helpers.buildDiagnosisKeysWithFlexibleRollingPeriod(5, 24L + 4L, 5, 80));
+    bundler.setDiagnosisKeys(diagnosisKeys, LocalDateTime.of(1970, 1, 5, 0, 0));
+    assertThat(bundler.getDiagnosisKeysForHour(LocalDateTime.of(1970, 1, 2, 4, 0, 0))).hasSize(10);
+  }
 }
diff --git a/...istribution/src/test/java/app/coronawarn/server/services/distribution/common/Helpers.java b/...istribution/src/test/java/app/coronawarn/server/services/distribution/common/Helpers.java
@@ -76,6 +76,18 @@ public static List<DiagnosisKey> buildDiagnosisKeys(int startIntervalNumber, lon
         .collect(Collectors.toList());
   }
 
+  public static List<DiagnosisKey> buildDiagnosisKeysWithFlexibleRollingPeriod(
+      int startIntervalNumber, long submissionTimestamp, int number, int rollingPeriod) {
+    return IntStream.range(0, number)
+        .mapToObj(ignoredValue -> DiagnosisKey.builder()
+            .withKeyData(new byte[16])
+            .withRollingStartIntervalNumber(startIntervalNumber)
+            .withTransmissionRiskLevel(2)
+            .withSubmissionTimestamp(submissionTimestamp)
+            .withRollingPeriod(rollingPeriod).build())
+        .collect(Collectors.toList());
+  }
+
   public static Set<String> getFilePaths(java.io.File root, String basePath) {
     Set<String> files = Arrays.stream(Objects.requireNonNull(root.listFiles()))
         .filter(File::isFile)

diff --git a/...ain/java/app/coronawarn/server/services/submission/validation/ValidSubmissionPayload.java b/...ain/java/app/coronawarn/server/services/submission/validation/ValidSubmissionPayload.java
@@ -151,8 +151,9 @@ private boolean keysHaveFlexibleRollingPeriod(List<TemporaryExposureKey> exposur
 
     private boolean checkStartIntervalNumberIsAtMidNight(List<TemporaryExposureKey> exposureKeys,
         ConstraintValidatorContext validatorContext) {
+      // check if any start interval number is not set to midnight by performing modulo 24 hrs in minutes/10
       boolean isNotMidNight00Utc = exposureKeys.stream()
-          .anyMatch(exposureKey -> exposureKey.getRollingStartIntervalNumber() % maxRollingPeriod > 0);
+          .anyMatch(exposureKey -> exposureKey.getRollingStartIntervalNumber() % 144 > 0);
 
       if (isNotMidNight00Utc) {
         addViolation(validatorContext, "Start Interval Number must be at midnight ( 00:00 UTC )");