Merge pull request #491 from IABTechLab/gdm-UID2-5557-refreshfrom-hotfix

Added day truncation for refresh from

Author: aulme

src/main/java/com/uid2/admin/salt/SaltRotation.java

@@ -16,7 +16,7 @@
 import java.util.stream.Collectors;
 
 public class SaltRotation {
-    private final static long THIRTY_DAYS_IN_MS = Duration.ofDays(30).toMillis();
+    private static final long THIRTY_DAYS_IN_MS = Duration.ofDays(30).toMillis();
 
     private final IKeyGenerator keyGenerator;
     private final boolean isRefreshFromEnabled;
@@ -31,8 +31,7 @@ public Result rotateSalts(
             SaltSnapshot lastSnapshot,
             Duration[] minAges,
             double fraction,
-            TargetDate targetDate
-    ) throws Exception {
+            TargetDate targetDate) throws Exception {
         var preRotationSalts = lastSnapshot.getAllRotatingSalts();
         var nextEffective = targetDate.asInstant();
         var nextExpires = nextEffective.plus(7, ChronoUnit.DAYS);
@@ -79,7 +78,7 @@ private Set<SaltEntry> findRefreshableSalts(SaltEntry[] preRotationSalts, Target
 
     private boolean isRefreshable(TargetDate targetDate, SaltEntry salt) {
         if (this.isRefreshFromEnabled) {
-            return salt.refreshFrom().equals(targetDate.asEpochMs());
+            return Instant.ofEpochMilli(salt.refreshFrom()).truncatedTo(ChronoUnit.DAYS).equals(targetDate.asInstant());
         }
 
         return true;
@@ -116,7 +115,7 @@ private SaltEntry updateSalt(SaltEntry oldSalt, TargetDate targetDate, boolean s
 
     private long calculateRefreshFrom(SaltEntry salt, TargetDate targetDate) {
         long multiplier = targetDate.saltAgeInDays(salt) / 30 + 1;
-        return salt.lastUpdated() + (multiplier * THIRTY_DAYS_IN_MS);
+        return Instant.ofEpochMilli(salt.lastUpdated()).truncatedTo(ChronoUnit.DAYS).toEpochMilli() + (multiplier * THIRTY_DAYS_IN_MS);
     }
 
     private String calculatePreviousSalt(SaltEntry salt, boolean shouldRotate, TargetDate targetDate) {
@@ -133,8 +132,7 @@ private List<SaltEntry> pickSaltsToRotate(
             Set<SaltEntry> refreshableSalts,
             TargetDate targetDate,
             Duration[] minAges,
-            int numSaltsToRotate
-    ) {
+            int numSaltsToRotate) {
         var thresholds = Arrays.stream(minAges)
                 .map(minAge -> targetDate.asInstant().minusSeconds(minAge.getSeconds()))
                 .sorted()
@@ -162,8 +160,7 @@ private List<SaltEntry> pickSaltsToRotateInTimeWindow(
             Set<SaltEntry> refreshableSalts,
             int maxIndexes,
             long minLastUpdated,
-            long maxLastUpdated
-    ) {
+            long maxLastUpdated) {
         ArrayList<SaltEntry> candidateSalts = refreshableSalts.stream()
                 .filter(salt -> minLastUpdated <= salt.lastUpdated() && salt.lastUpdated() < maxLastUpdated)
                 .collect(Collectors.toCollection(ArrayList::new));
@@ -195,7 +192,7 @@ private void logSaltAges(String saltCountType, TargetDate targetDate, Collection
     }
 
     @Getter
-    public static class Result {
+    public static final class Result {
         private final SaltSnapshot snapshot; // can be null if new snapshot is not needed
         private final String reason; // why you are not getting a new snapshot
 

src/main/java/com/uid2/admin/salt/TargetDate.java

@@ -4,10 +4,11 @@
 
 import java.time.*;
 import java.time.format.DateTimeFormatter;
+import java.time.temporal.ChronoUnit;
 import java.util.Objects;
 
 public class TargetDate {
-    private final static long DAY_IN_MS = Duration.ofDays(1).toMillis();
+    private static final long DAY_IN_MS = Duration.ofDays(1).toMillis();
 
     private final LocalDate date;
     private final long epochMs;
@@ -39,7 +40,7 @@ public Instant asInstant() {
 
     // relative to this date
     public long saltAgeInDays(SaltEntry salt) {
-        return (this.asEpochMs() - salt.lastUpdated()) / DAY_IN_MS;
+        return (this.asEpochMs() - Instant.ofEpochMilli(salt.lastUpdated()).truncatedTo(ChronoUnit.DAYS).toEpochMilli()) / DAY_IN_MS;
     }
 
     public TargetDate plusDays(int days) {

src/test/java/com/uid2/admin/salt/SaltRotationTest.java

@@ -29,7 +29,7 @@
 import ch.qos.logback.classic.Logger;
 import org.slf4j.LoggerFactory;
 
-public class SaltRotationTest {
+class SaltRotationTest {
     @Mock
     private IKeyGenerator keyGenerator;
     private SaltRotation saltRotation;
@@ -50,7 +50,7 @@ void setup() {
     }
 
     @AfterEach
-    void tearDown() throws Exception {
+    void teardown() throws Exception {
         appender.stop();
         mocks.close();
     }
@@ -209,13 +209,16 @@ void rotateSaltsRotateSaltsInsufficientOutdatedSalts() throws Exception {
 
     @ParameterizedTest
     @CsvSource({
-            "5, 30", // Soon after rotation, use 30 days post rotation
-            "40, 60", // >30 days after rotation use the next increment of 30 days
-            "60, 90", // Exactly at multiple of 30 days post rotation, use next increment of 30 days
+            "5, 0, 30", // Soon after rotation, use 30 days post rotation
+            "5, 100, 30", // Soon after rotation, use 30 days post rotation with some offset
+            "40, 0, 60", // >30 days after rotation use the next increment of 30 days
+            "40, 100, 60", // >30 days after rotation use the next increment of 30 days with some offset
+            "60, 0, 90", // Exactly at multiple of 30 days post rotation, use next increment of 30 days
+            "60, 100, 90" // Exactly at multiple of 30 days post rotation, use next increment of 30 days with some offset
     })
-    void testRefreshFromCalculation(int lastRotationDaysAgo, int refreshFromDaysFromRotation) throws Exception {
+    void testRefreshFromCalculation(int lastRotationDaysAgo, int lastRotationMsOffset, int refreshFromDaysFromRotation) throws Exception {
         var lastRotation = daysEarlier(lastRotationDaysAgo);
-        SaltBuilder saltBuilder = SaltBuilder.start().lastUpdated(lastRotation);
+        SaltBuilder saltBuilder = SaltBuilder.start().lastUpdated(lastRotation.asInstant().plusMillis(lastRotationMsOffset));
         var lastSnapshot = SaltSnapshotBuilder.start()
                 .entries(saltBuilder)
                 .build();

src/test/java/com/uid2/admin/salt/helper/SaltBuilder.java

@@ -32,11 +32,21 @@ public SaltBuilder lastUpdated(TargetDate lastUpdated) {
         return this;
     }
 
+    public SaltBuilder lastUpdated(Instant lastUpdated) {
+        this.lastUpdated = lastUpdated;
+        return this;
+    }
+
     public SaltBuilder refreshFrom(TargetDate refreshFrom) {
         this.refreshFrom = refreshFrom.asInstant();
         return this;
     }
 
+    public SaltBuilder refreshFrom(Instant refreshFrom) {
+        this.refreshFrom = refreshFrom;
+        return this;
+    }
+
     public SaltBuilder currentSalt(String currentSalt) {
         this.currentSalt = currentSalt;
         return this;

src/test/java/com/uid2/admin/salt/helper/TargetDateUtil.java

@@ -2,9 +2,12 @@
 
 import com.uid2.admin.salt.TargetDate;
 
-public class TargetDateUtil {
+public final class TargetDateUtil {
     private static final TargetDate TARGET_DATE = TargetDate.of(2025, 1, 1);
 
+    private TargetDateUtil() {
+    }
+
     public static TargetDate daysEarlier(int days) {
         return TARGET_DATE.minusDays(days);
     }