diff --git a/src/main/java/org/jenkinsci/plugins/github/admin/GitHubDuplicateEventsMonitor.java b/src/main/java/org/jenkinsci/plugins/github/admin/GitHubDuplicateEventsMonitor.java index e24713cf6..794f3db04 100644 --- a/src/main/java/org/jenkinsci/plugins/github/admin/GitHubDuplicateEventsMonitor.java +++ b/src/main/java/org/jenkinsci/plugins/github/admin/GitHubDuplicateEventsMonitor.java @@ -12,6 +12,7 @@ import com.google.common.annotations.VisibleForTesting; import hudson.Extension; +import hudson.ExtensionList; import hudson.model.AdministrativeMonitor; import hudson.model.Item; import jenkins.model.Jenkins; @@ -57,7 +58,7 @@ String getLastDuplicateUrl() { @Override public boolean isActivated() { - return DuplicateEventsSubscriber.isDuplicateEventSeen(); + return ExtensionList.lookupSingleton(DuplicateEventsSubscriber.class).isDuplicateEventSeen(); } @Override @@ -75,7 +76,7 @@ public void checkRequiredPermission() { public HttpResponse doGetLastDuplicatePayload() { Jenkins.get().checkPermission(Jenkins.SYSTEM_READ); JSONObject data; - var lastDuplicate = DuplicateEventsSubscriber.getLastDuplicate(); + var lastDuplicate = ExtensionList.lookupSingleton(DuplicateEventsSubscriber.class).getLastDuplicate(); if (lastDuplicate != null) { data = JSONObject.fromObject(lastDuplicate.ghSubscriberEvent().getPayload()); } else { @@ -102,7 +103,7 @@ public static final class DuplicateEventsSubscriber extends GHEventsSubscriber { private static final Logger LOGGER = Logger.getLogger(DuplicateEventsSubscriber.class.getName()); - private static Ticker ticker = Ticker.systemTicker(); + private Ticker ticker = Ticker.systemTicker(); /** * Caches GitHub event GUIDs for 10 minutes to track recent events to detect duplicates. *

@@ -114,21 +115,21 @@ public static final class DuplicateEventsSubscriber extends GHEventsSubscriber { * timestamp (assuming caffeine internally keeps long) takes 8 bytes; total of 44 bytes * per entry. So the maximum memory consumed by this cache is 24k * 44 = 1056k = 1.056 MB. */ - private static final Cache EVENT_TRACKER = Caffeine.newBuilder() - .maximumSize(24_000L) - .expireAfterWrite(Duration.ofMinutes(10)) - .ticker(() -> ticker.read()) - .build(); + private final Cache eventTracker = Caffeine.newBuilder() + .maximumSize(24_000L) + .expireAfterWrite(Duration.ofMinutes(10)) + .ticker(() -> ticker.read()) + .build(); private static final Object DUMMY = new Object(); - private static volatile TrackedDuplicateEvent lastDuplicate; + private volatile TrackedDuplicateEvent lastDuplicate; public record TrackedDuplicateEvent( String eventGuid, Instant lastUpdated, GHSubscriberEvent ghSubscriberEvent) { } private static final Duration TWENTY_FOUR_HOURS = Duration.ofHours(24); @VisibleForTesting @Restricted(NoExternalUse.class) - static void setTicker(Ticker testTicker) { + void setTicker(Ticker testTicker) { ticker = testTicker; } @@ -174,10 +175,10 @@ protected void onEvent(final GHSubscriberEvent event) { if (eventGuid == null) { return; } - if (EVENT_TRACKER.getIfPresent(eventGuid) != null) { + if (eventTracker.getIfPresent(eventGuid) != null) { lastDuplicate = new TrackedDuplicateEvent(eventGuid, getNow(), event); } - EVENT_TRACKER.put(eventGuid, DUMMY); + eventTracker.put(eventGuid, DUMMY); } /** @@ -187,16 +188,16 @@ protected void onEvent(final GHSubscriberEvent event) { * * @return {@code true} if a duplicate was seen in the last 24 hours, {@code false} otherwise. */ - public static boolean isDuplicateEventSeen() { + public boolean isDuplicateEventSeen() { return lastDuplicate != null && Duration.between(lastDuplicate.lastUpdated(), getNow()).compareTo(TWENTY_FOUR_HOURS) < 0; } - private static Instant getNow() { + private Instant getNow() { return Instant.ofEpochSecond(0L, ticker.read()); } - public static TrackedDuplicateEvent getLastDuplicate() { + public TrackedDuplicateEvent getLastDuplicate() { return lastDuplicate; } @@ -206,10 +207,10 @@ public static TrackedDuplicateEvent getLastDuplicate() { */ @VisibleForTesting @Restricted(NoExternalUse.class) - static Set getEventCountsTracker() { - return EVENT_TRACKER.asMap().keySet().stream() - .filter(key -> EVENT_TRACKER.getIfPresent(key) != null) - .collect(Collectors.toSet()); + Set getPresentEventKeys() { + return eventTracker.asMap().keySet().stream() + .filter(key -> eventTracker.getIfPresent(key) != null) + .collect(Collectors.toSet()); } } } diff --git a/src/test/java/org/jenkinsci/plugins/github/admin/GitHubDuplicateEventsMonitorUnitTest.java b/src/test/java/org/jenkinsci/plugins/github/admin/GitHubDuplicateEventsMonitorUnitTest.java index 7f92fd1f8..fd8195ac4 100644 --- a/src/test/java/org/jenkinsci/plugins/github/admin/GitHubDuplicateEventsMonitorUnitTest.java +++ b/src/test/java/org/jenkinsci/plugins/github/admin/GitHubDuplicateEventsMonitorUnitTest.java @@ -3,9 +3,6 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.nullValue; -import static org.jenkinsci.plugins.github.admin.GitHubDuplicateEventsMonitor.DuplicateEventsSubscriber.getEventCountsTracker; -import static org.jenkinsci.plugins.github.admin.GitHubDuplicateEventsMonitor.DuplicateEventsSubscriber.getLastDuplicate; -import static org.jenkinsci.plugins.github.admin.GitHubDuplicateEventsMonitor.DuplicateEventsSubscriber.isDuplicateEventSeen; import java.time.Duration; import java.time.Instant; @@ -21,81 +18,82 @@ @For(GitHubDuplicateEventsMonitor.DuplicateEventsSubscriber.class) public class GitHubDuplicateEventsMonitorUnitTest { - private final GitHubDuplicateEventsMonitor.DuplicateEventsSubscriber subscriber - = new GitHubDuplicateEventsMonitor.DuplicateEventsSubscriber(); - @Test public void onEventShouldTrackEventAndKeepTrackOfLastDuplicate() { + var subscriber = new GitHubDuplicateEventsMonitor.DuplicateEventsSubscriber(); + var now = Instant.parse("2025-02-05T03:00:00Z"); var after1Sec = Instant.parse("2025-02-05T03:00:01Z"); var after2Sec = Instant.parse("2025-02-05T03:00:02Z"); FakeTicker fakeTicker = new FakeTicker(now); - GitHubDuplicateEventsMonitor.DuplicateEventsSubscriber.setTicker(fakeTicker); + subscriber.setTicker(fakeTicker); - assertThat("lastDuplicate is null at first", getLastDuplicate(), is(nullValue())); - assertThat("should not throw NPE", isDuplicateEventSeen(), is(false)); + assertThat("lastDuplicate is null at first", subscriber.getLastDuplicate(), is(nullValue())); + assertThat("should not throw NPE", subscriber.isDuplicateEventSeen(), is(false)); // send a null event subscriber.onEvent(new GHSubscriberEvent(null, "origin", GHEvent.PUSH, "payload")); - assertThat("null event is not tracked", getEventCountsTracker().size(), is(0)); - assertThat("lastDuplicate is still null", getLastDuplicate(), is(nullValue())); + assertThat("null event is not tracked", subscriber.getPresentEventKeys().size(), is(0)); + assertThat("lastDuplicate is still null", subscriber.getLastDuplicate(), is(nullValue())); // at present subscriber.onEvent(new GHSubscriberEvent("1", "origin", GHEvent.PUSH, "payload")); - assertThat(getEventCountsTracker(), is(Set.of("1"))); - assertThat(getLastDuplicate(), is(nullValue())); - assertThat(isDuplicateEventSeen(), is(false)); + assertThat(subscriber.getPresentEventKeys(), is(Set.of("1"))); + assertThat(subscriber.getLastDuplicate(), is(nullValue())); + assertThat(subscriber.isDuplicateEventSeen(), is(false)); subscriber.onEvent(new GHSubscriberEvent("2", "origin", GHEvent.PUSH, "payload")); - assertThat(getLastDuplicate(), is(nullValue())); - assertThat(isDuplicateEventSeen(), is(false)); - assertThat(getEventCountsTracker(), is(Set.of("1", "2"))); + assertThat(subscriber.getLastDuplicate(), is(nullValue())); + assertThat(subscriber.isDuplicateEventSeen(), is(false)); + assertThat(subscriber.getPresentEventKeys(), is(Set.of("1", "2"))); subscriber.onEvent(new GHSubscriberEvent(null, "origin", GHEvent.PUSH, "payload")); - assertThat(getLastDuplicate(), is(nullValue())); - assertThat(getEventCountsTracker(), is(Set.of("1", "2"))); - assertThat(isDuplicateEventSeen(), is(false)); + assertThat(subscriber.getLastDuplicate(), is(nullValue())); + assertThat(subscriber.getPresentEventKeys(), is(Set.of("1", "2"))); + assertThat(subscriber.isDuplicateEventSeen(), is(false)); // after a second fakeTicker.advance(Duration.ofSeconds(1)); subscriber.onEvent(new GHSubscriberEvent("1", "origin", GHEvent.PUSH, "payload")); - assertThat(getLastDuplicate().eventGuid(), is("1")); - assertThat(getLastDuplicate().lastUpdated(), is(after1Sec)); - assertThat(getEventCountsTracker(), is(Set.of("1", "2"))); - assertThat(isDuplicateEventSeen(), is(true)); + assertThat(subscriber.getLastDuplicate().eventGuid(), is("1")); + assertThat(subscriber.getLastDuplicate().lastUpdated(), is(after1Sec)); + assertThat(subscriber.getPresentEventKeys(), is(Set.of("1", "2"))); + assertThat(subscriber.isDuplicateEventSeen(), is(true)); // second occurrence for another event after 2 seconds fakeTicker.advance(Duration.ofSeconds(1)); subscriber.onEvent(new GHSubscriberEvent("2", "origin", GHEvent.PUSH, "payload")); - assertThat(getLastDuplicate().eventGuid(), is("2")); - assertThat(getLastDuplicate().lastUpdated(), is(after2Sec)); - assertThat(getEventCountsTracker(), is(Set.of("1", "2"))); - assertThat(isDuplicateEventSeen(), is(true)); + assertThat(subscriber.getLastDuplicate().eventGuid(), is("2")); + assertThat(subscriber.getLastDuplicate().lastUpdated(), is(after2Sec)); + assertThat(subscriber.getPresentEventKeys(), is(Set.of("1", "2"))); + assertThat(subscriber.isDuplicateEventSeen(), is(true)); // 24 hours has passed; note we already added 2 seconds/ so effectively 24h 2sec now. fakeTicker.advance(Duration.ofHours(24)); - assertThat(isDuplicateEventSeen(), is(false)); + assertThat(subscriber.isDuplicateEventSeen(), is(false)); } @Test public void checkOldEntriesAreExpiredAfter10Minutes() { + var subscriber = new GitHubDuplicateEventsMonitor.DuplicateEventsSubscriber(); + var now = Instant.parse("2025-02-05T03:00:00Z"); FakeTicker fakeTicker = new FakeTicker(now); - GitHubDuplicateEventsMonitor.DuplicateEventsSubscriber.setTicker(fakeTicker); + subscriber.setTicker(fakeTicker); // at present subscriber.onEvent(new GHSubscriberEvent("1", "origin", GHEvent.PUSH, "payload")); subscriber.onEvent(new GHSubscriberEvent("2", "origin", GHEvent.PUSH, "payload")); - assertThat(getEventCountsTracker(), is(Set.of("1", "2"))); + assertThat(subscriber.getPresentEventKeys(), is(Set.of("1", "2"))); // after 2 minutes fakeTicker.advance(Duration.ofMinutes(2)); subscriber.onEvent(new GHSubscriberEvent("3", "origin", GHEvent.PUSH, "payload")); subscriber.onEvent(new GHSubscriberEvent("4", "origin", GHEvent.PUSH, "payload")); - assertThat(getEventCountsTracker(), is(Set.of("1", "2", "3", "4"))); - assertThat(getEventCountsTracker().size(), is(4)); + assertThat(subscriber.getPresentEventKeys(), is(Set.of("1", "2", "3", "4"))); + assertThat(subscriber.getPresentEventKeys().size(), is(4)); // 10 minutes 1 second later fakeTicker.advance(Duration.ofMinutes(8).plusSeconds(1)); - assertThat(getEventCountsTracker(), is(Set.of("3", "4"))); - assertThat(getEventCountsTracker().size(), is(2)); + assertThat(subscriber.getPresentEventKeys(), is(Set.of("3", "4"))); + assertThat(subscriber.getPresentEventKeys().size(), is(2)); } private static class FakeTicker implements Ticker {