Recording refreshFrom, remove unnecessary interface #461
Conversation
|
|
||
| private final LocalDate targetDate = LocalDate.of(2025, 1, 1); | ||
| private final Instant targetDateAsInstant = targetDate.atStartOfDay().toInstant(ZoneOffset.UTC); | ||
| private static final LocalDate targetDate = LocalDate.of(2025, 1, 1); |
There was a problem hiding this comment.
By convention, static final should be all uppercase
i.e. TARGET_DATE, ONE_DAY_EARLIER, etc.
There was a problem hiding this comment.
I'd rather make them non-static 😓
There was a problem hiding this comment.
With @sophia-chen-ttd 's suggestion no longer need those.
| var result1 = saltRotation.rotateSalts(lastSnapshot, minAges, 0.2, targetDate); | ||
| assertFalse(result1.hasSnapshot()); | ||
| final ISaltRotation.Result result2 = saltRotation.rotateSalts(lastSnapshot, minAges, 0.2, targetDate.minusDays(1)); | ||
| var result2 = saltRotation.rotateSalts(lastSnapshot, minAges, 0.2, targetDate.minusDays(1)); |
There was a problem hiding this comment.
Same with some other parts, but you already have the oneDayEarlier variable set so you can just use that directly
Also wondering if we should just parametrize this test
There was a problem hiding this comment.
The one day earlier variable is an Instant, while here we need a LocalDate, converting would be even longer.
Parametrization - probably will be riddled with if statements, maybe an exercise for another day.
| .withEntries(3, sixDaysEarlier) | ||
| .withEntries(5, fiveDaysEarlier) | ||
| .withEntries(2, fourDaysEarlier) | ||
| .build(targetDateAsInstant.minus(1, DAYS), |
There was a problem hiding this comment.
Same here regarding the day variables you created previously
There was a problem hiding this comment.
Missed this one, fixed
| } | ||
|
|
||
| private long calculateRefreshFrom(long lastUpdated, long nextEffective) { | ||
| long thirtyDaysMillis = 2592000000L; |
There was a problem hiding this comment.
You can also use Duration.ofDays(30).toMillis();
There was a problem hiding this comment.
This is done 1 million times in a loop so I wanted to reduce logic to minimum. Perhaps could move it to class level so it doesn't get recomputed every time.
| private static final Instant sixDaysEarlier = targetDateAsInstant.minus(6, DAYS); | ||
| private static final Instant tenDaysEarlier = targetDateAsInstant.minus(10, DAYS); | ||
| private static final Instant sixDaysLater = targetDateAsInstant.plus(6, DAYS); | ||
| private static final Instant sevenDaysLater = targetDateAsInstant.plus(7, DAYS); |
There was a problem hiding this comment.
would it be worth having a small function where we can put in the days to we want to add / subtract so this can be more flexible? When we start testing for rotations on intervals of 30s, these values will need to change anyway
There was a problem hiding this comment.
Good point, we already have those.
No description provided.