Skip to content

Commit 59e3188

Browse files
aulmeaulme
committed
Address feedback
1 parent d380310 commit 59e3188

File tree

2 files changed

+51
-54
lines changed

2 files changed

+51
-54
lines changed

‎src/main/java/com/uid2/admin/secret/SaltRotation.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
public class SaltRotation {
2020
private final IKeyGenerator keyGenerator;
21+
private final long THIRTY_DAYS_IN_MS = Duration.ofDays(30).toMillis();
2122

2223
public SaltRotation(IKeyGenerator keyGenerator) {
2324
this.keyGenerator = keyGenerator;
@@ -76,10 +77,9 @@ private SaltEntry updateSalt(SaltEntry oldSalt, boolean shouldRotate, long nextE
7677
}
7778

7879
private long calculateRefreshFrom(long lastUpdated, long nextEffective) {
79-
long thirtyDaysMillis = 2592000000L;
8080
long age = nextEffective - lastUpdated;
81-
long multiplier = age / thirtyDaysMillis + 1;
82-
return lastUpdated + (multiplier * thirtyDaysMillis);
81+
long multiplier = age / THIRTY_DAYS_IN_MS + 1;
82+
return lastUpdated + (multiplier * THIRTY_DAYS_IN_MS);
8383
}
8484

8585
private List<Integer> pickSaltIndexesToRotate(

‎src/test/java/com/uid2/admin/secret/SaltRotationTest.java‎

Lines changed: 48 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -23,18 +23,16 @@ public class SaltRotationTest {
2323
@Mock private IKeyGenerator keyGenerator;
2424
private SaltRotation saltRotation;
2525

26-
private static final LocalDate targetDate = LocalDate.of(2025, 1, 1);
27-
private static final Instant targetDateAsInstant = targetDate.atStartOfDay().toInstant(ZoneOffset.UTC);
28-
29-
private static final Instant oneDayEarlier = targetDateAsInstant.minus(1, DAYS);
30-
private static final Instant twoDaysEarlier = targetDateAsInstant.minus(2, DAYS);
31-
private static final Instant threeDaysEarlier = targetDateAsInstant.minus(3, DAYS);
32-
private static final Instant fourDaysEarlier = targetDateAsInstant.minus(4, DAYS);
33-
private static final Instant fiveDaysEarlier = targetDateAsInstant.minus(5, DAYS);
34-
private static final Instant sixDaysEarlier = targetDateAsInstant.minus(6, DAYS);
35-
private static final Instant tenDaysEarlier = targetDateAsInstant.minus(10, DAYS);
36-
private static final Instant sixDaysLater = targetDateAsInstant.plus(6, DAYS);
37-
private static final Instant sevenDaysLater = targetDateAsInstant.plus(7, DAYS);
26+
private final LocalDate targetDate = LocalDate.of(2025, 1, 1);
27+
private final Instant targetDateAsInstant = targetDate.atStartOfDay().toInstant(ZoneOffset.UTC);
28+
29+
private Instant daysEarlier(int days) {
30+
return targetDateAsInstant.minus(days, DAYS);
31+
}
32+
33+
private Instant daysLater(int days) {
34+
return targetDateAsInstant.plus(days, DAYS);
35+
}
3836

3937
@BeforeEach
4038
void setup() {
@@ -80,7 +78,7 @@ void rotateSaltsLastSnapshotIsUpToDate() throws Exception {
8078
};
8179
var lastSnapshot = SnapshotBuilder.start()
8280
.withEntries(10, targetDateAsInstant)
83-
.build(targetDateAsInstant, sevenDaysLater);
81+
.build(targetDateAsInstant, daysLater(7));
8482

8583
var result1 = saltRotation.rotateSalts(lastSnapshot, minAges, 0.2, targetDate);
8684
assertFalse(result1.hasSnapshot());
@@ -97,7 +95,7 @@ void rotateSaltsAllSaltsUpToDate() throws Exception {
9795

9896
var lastSnapshot = SnapshotBuilder.start()
9997
.withEntries(10, targetDateAsInstant)
100-
.build(oneDayEarlier, sixDaysLater);
98+
.build(daysEarlier(1), daysLater(6));
10199

102100
var result = saltRotation.rotateSalts(lastSnapshot, minAges, 0.2, targetDate);
103101
assertFalse(result.hasSnapshot());
@@ -112,15 +110,15 @@ void rotateSaltsAllSaltsOld() throws Exception {
112110
};
113111

114112
var lastSnapshot = SnapshotBuilder.start()
115-
.withEntries(10, tenDaysEarlier)
116-
.build(oneDayEarlier, sixDaysLater);
113+
.withEntries(10, daysEarlier(10))
114+
.build(daysEarlier(1), daysLater(6));
117115

118116
var result = saltRotation.rotateSalts(lastSnapshot, minAges, 0.2, targetDate);
119117
assertTrue(result.hasSnapshot());
120118
assertEquals(2, countEntriesWithLastUpdated(result.getSnapshot().getAllRotatingSalts(), result.getSnapshot().getEffective()));
121-
assertEquals(8, countEntriesWithLastUpdated(result.getSnapshot().getAllRotatingSalts(), tenDaysEarlier));
119+
assertEquals(8, countEntriesWithLastUpdated(result.getSnapshot().getAllRotatingSalts(), daysEarlier(10)));
122120
assertEquals(targetDateAsInstant, result.getSnapshot().getEffective());
123-
assertEquals(sevenDaysLater, result.getSnapshot().getExpires());
121+
assertEquals(daysLater(7), result.getSnapshot().getExpires());
124122
verify(keyGenerator, times(2)).generateRandomKeyString(anyInt());
125123
}
126124

@@ -132,21 +130,20 @@ void rotateSaltsRotateSaltsFromOldestBucketOnly() throws Exception {
132130
};
133131

134132
var lastSnapshot = SnapshotBuilder.start()
135-
.withEntries(3, sixDaysEarlier)
136-
.withEntries(5, fiveDaysEarlier)
137-
.withEntries(2, fourDaysEarlier)
138-
.build(targetDateAsInstant.minus(1, DAYS),
139-
targetDateAsInstant.plus(6, DAYS));
133+
.withEntries(3, daysEarlier(6))
134+
.withEntries(5, daysEarlier(5))
135+
.withEntries(2, daysEarlier(4))
136+
.build(daysEarlier(1), daysLater(6));
140137

141138
var result = saltRotation.rotateSalts(lastSnapshot, minAges, 0.2, targetDate);
142139
assertTrue(result.hasSnapshot());
143140
var salts = result.getSnapshot().getAllRotatingSalts();
144141
assertEquals(2, countEntriesWithLastUpdated(salts, result.getSnapshot().getEffective()));
145-
assertEquals(1, countEntriesWithLastUpdated(salts, sixDaysEarlier));
146-
assertEquals(5, countEntriesWithLastUpdated(salts, fiveDaysEarlier));
147-
assertEquals(2, countEntriesWithLastUpdated(salts, fourDaysEarlier));
142+
assertEquals(1, countEntriesWithLastUpdated(salts, daysEarlier(6)));
143+
assertEquals(5, countEntriesWithLastUpdated(salts, daysEarlier(5)));
144+
assertEquals(2, countEntriesWithLastUpdated(salts, daysEarlier(4)));
148145
assertEquals(targetDateAsInstant, result.getSnapshot().getEffective());
149-
assertEquals(sevenDaysLater, result.getSnapshot().getExpires());
146+
assertEquals(daysLater(7), result.getSnapshot().getExpires());
150147
verify(keyGenerator, times(2)).generateRandomKeyString(anyInt());
151148
}
152149

@@ -158,18 +155,18 @@ void rotateSaltsRotateSaltsFromNewerBucketOnly() throws Exception {
158155
};
159156

160157
var lastSnapshot = SnapshotBuilder.start()
161-
.withEntries(3, fourDaysEarlier)
162-
.withEntries(7, threeDaysEarlier)
163-
.build(oneDayEarlier, sixDaysLater);
158+
.withEntries(3, daysEarlier(4))
159+
.withEntries(7, daysEarlier(3))
160+
.build(daysEarlier(1), daysLater(6));
164161

165162
var result = saltRotation.rotateSalts(lastSnapshot, minAges, 0.2, targetDate);
166163
assertTrue(result.hasSnapshot());
167164
var salts = result.getSnapshot().getAllRotatingSalts();
168165
assertEquals(2, countEntriesWithLastUpdated(salts, result.getSnapshot().getEffective()));
169-
assertEquals(1, countEntriesWithLastUpdated(salts, fourDaysEarlier));
170-
assertEquals(7, countEntriesWithLastUpdated(salts, threeDaysEarlier));
166+
assertEquals(1, countEntriesWithLastUpdated(salts, daysEarlier(4)));
167+
assertEquals(7, countEntriesWithLastUpdated(salts, daysEarlier(3)));
171168
assertEquals(targetDateAsInstant, result.getSnapshot().getEffective());
172-
assertEquals(sevenDaysLater, result.getSnapshot().getExpires());
169+
assertEquals(daysLater(7), result.getSnapshot().getExpires());
173170
verify(keyGenerator, times(2)).generateRandomKeyString(anyInt());
174171
}
175172

@@ -181,20 +178,20 @@ void rotateSaltsRotateSaltsFromMultipleBuckets() throws Exception {
181178
};
182179

183180
var lastSnapshot = SnapshotBuilder.start()
184-
.withEntries(3, sixDaysEarlier)
185-
.withEntries(5, fiveDaysEarlier)
186-
.withEntries(2, fourDaysEarlier)
187-
.build(oneDayEarlier, sixDaysLater);
181+
.withEntries(3, daysEarlier(6))
182+
.withEntries(5, daysEarlier(5))
183+
.withEntries(2, daysEarlier(4))
184+
.build(daysEarlier(1), daysLater(6));
188185

189186
var result = saltRotation.rotateSalts(lastSnapshot, minAges, 0.45, targetDate);
190187
assertTrue(result.hasSnapshot());
191188
var salts = result.getSnapshot().getAllRotatingSalts();
192189
assertEquals(5, countEntriesWithLastUpdated(salts, result.getSnapshot().getEffective()));
193-
assertEquals(0, countEntriesWithLastUpdated(salts, sixDaysEarlier));
194-
assertEquals(3, countEntriesWithLastUpdated(salts, fiveDaysEarlier));
195-
assertEquals(2, countEntriesWithLastUpdated(salts, fourDaysEarlier));
190+
assertEquals(0, countEntriesWithLastUpdated(salts, daysEarlier(6)));
191+
assertEquals(3, countEntriesWithLastUpdated(salts, daysEarlier(5)));
192+
assertEquals(2, countEntriesWithLastUpdated(salts, daysEarlier(4)));
196193
assertEquals(targetDateAsInstant, result.getSnapshot().getEffective());
197-
assertEquals(sevenDaysLater, result.getSnapshot().getExpires());
194+
assertEquals(daysLater(7), result.getSnapshot().getExpires());
198195
verify(keyGenerator, times(5)).generateRandomKeyString(anyInt());
199196
}
200197

@@ -206,20 +203,20 @@ void rotateSaltsRotateSaltsInsufficientOutdatedSalts() throws Exception {
206203
};
207204

208205
var lastSnapshot = SnapshotBuilder.start()
209-
.withEntries(1, fiveDaysEarlier)
210-
.withEntries(2, fourDaysEarlier)
211-
.withEntries(7, twoDaysEarlier)
212-
.build(oneDayEarlier, sixDaysLater);
206+
.withEntries(1, daysEarlier(5))
207+
.withEntries(2, daysEarlier(4))
208+
.withEntries(7, daysEarlier(2))
209+
.build(daysEarlier(1), daysLater(6));
213210

214211
var result = saltRotation.rotateSalts(lastSnapshot, minAges, 0.45, targetDate);
215212
assertTrue(result.hasSnapshot());
216213
var salts = result.getSnapshot().getAllRotatingSalts();
217214
assertEquals(3, countEntriesWithLastUpdated(salts, result.getSnapshot().getEffective()));
218-
assertEquals(0, countEntriesWithLastUpdated(salts, fiveDaysEarlier));
219-
assertEquals(0, countEntriesWithLastUpdated(salts, fourDaysEarlier));
220-
assertEquals(7, countEntriesWithLastUpdated(salts, twoDaysEarlier));
215+
assertEquals(0, countEntriesWithLastUpdated(salts, daysEarlier(5)));
216+
assertEquals(0, countEntriesWithLastUpdated(salts, daysEarlier(4)));
217+
assertEquals(7, countEntriesWithLastUpdated(salts, daysEarlier(2)));
221218
assertEquals(targetDateAsInstant, result.getSnapshot().getEffective());
222-
assertEquals(sevenDaysLater, result.getSnapshot().getExpires());
219+
assertEquals(daysLater(7), result.getSnapshot().getExpires());
223220
verify(keyGenerator, times(3)).generateRandomKeyString(anyInt());
224221
}
225222

@@ -230,10 +227,10 @@ void rotateSaltsRotateSaltsInsufficientOutdatedSalts() throws Exception {
230227
"60, 90", // Exactly at multiple of 30 days post rotation, use next increment of 30 days
231228
})
232229
void testRefreshFromCalculation(int lastRotationDaysAgo, int refreshFromDaysFromRotation) throws Exception {
233-
var lastRotation = targetDateAsInstant.minus(lastRotationDaysAgo, DAYS);
230+
var lastRotation = daysEarlier(lastRotationDaysAgo);
234231
var lastSnapshot = SnapshotBuilder.start()
235232
.withEntries(new SaltEntry(1, "1", lastRotation.toEpochMilli(), "salt1", 100L, null, null, null))
236-
.build(oneDayEarlier, sixDaysLater);
233+
.build(daysEarlier(1), daysLater(6));
237234

238235
var result = saltRotation.rotateSalts(lastSnapshot, new Duration[]{ Duration.ofDays(1) }, 0.45, targetDate);
239236
var actual = result.getSnapshot().getAllRotatingSalts()[0];

0 commit comments

Comments
 (0)