Share a repeated evidence set at most once in each interval

pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/trackers/ShareUsageTracker.java

@@ -38,7 +38,9 @@
 
 /**
  * A tracker used by share usage to attempt to avoid repeatedly sending data
- * relating to the same user session.
+ * relating to the same user session. Evidence that matches an entry is shared
+ * again once the interval has passed since it was last shared, so a repeat is
+ * shared at most once in each interval.
  * @see <a href="https://github.com/51Degrees/specifications/blob/main/pipeline-specification/pipeline-elements/usage-sharing-element.md#session-tracking">Specification</a>
  */
 public class ShareUsageTracker extends TrackerBase<Date> {
@@ -64,7 +66,8 @@ public class ShareUsageTracker extends TrackerBase<Date> {
      * Construct a new instance.
      * @param configuration the cache configuration to use when building the
      *                      internal cache used by this tracker
-     * @param intervalMillis interval between matched in millis
+     * @param intervalMillis how long after evidence is shared a repeat of it is
+     *                       not shared, in milliseconds
      * @param filter the {@link EvidenceKeyFilter} that defines the evidence
      *               values to use when creating a key from an {@link FlowData}
      */
@@ -83,14 +86,22 @@ public ShareUsageTracker(
 
     @Override
     protected Date newValue(FlowData data) {
-        // Stored meta-data values for this tracker are always true.
+        // The time the new evidence is shared.
         return new Date();
     }
 
     @Override
     protected boolean match(FlowData data, Date value) {
-        Date now = new Date();
-        return value.getTime() <= (now.getTime() - interval);
+        // Sharing a repeat stores the time in the cached value, so the next
+        // interval starts from this share.
+        long now = System.currentTimeMillis();
+        synchronized (value) {
+            if (value.getTime() <= now - interval) {
+                value.setTime(now);
+                return true;
+            }
+            return false;
+        }
     }
 
     @Override

pipeline/pipeline.engines.fiftyone/src/test/java/fiftyone/pipeline/engines/fiftyone/trackers/ShareUsageTrackerTests.java

@@ -40,12 +40,29 @@
 import static fiftyone.pipeline.core.Constants.*;
 import static fiftyone.pipeline.engines.Constants.DEFAULT_SESSION_COOKIE_NAME;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.Mockito.any;
 import static org.mockito.Mockito.when;
 
 public class ShareUsageTrackerTests {
 
-    private long interval;
+    /**
+     * An interval no run of these tests comes near, for the tests that do
+     * not test expiry, so a slow runner cannot make a repeat count as new.
+     */
+    private static final long NO_EXPIRY_INTERVAL_MILLIS = 60 * 60 * 1000;
+
+    /**
+     * A short interval, for a test that sleeps past it to test expiry.
+     */
+    private static final long EXPIRY_INTERVAL_MILLIS = 50;
+
+    /**
+     * Added to the wait for an interval to pass, so that a sleep that returns
+     * early or a slow clock cannot leave a repeat inside the interval.
+     */
+    private static final long EXPIRY_MARGIN_MILLIS = 250;
 
     private EvidenceKeyFilter evidenceKeyFilter;
 
@@ -62,7 +79,6 @@ public void Init() {
         includedQueryStringParameters = new ArrayList<>();
         sessionCookieName = DEFAULT_SESSION_COOKIE_NAME;
 
-        interval = 50;
         evidenceKeyFilter = new EvidenceKeyFilterShareUsage(
             blockedHttpHeaders,
             includedQueryStringParameters,
@@ -73,7 +89,7 @@ public void Init() {
             1000);
         shareUsageTracker = new ShareUsageTracker(
             cacheConfig,
-            interval,
+            NO_EXPIRY_INTERVAL_MILLIS,
             evidenceKeyFilter);
 
         evidenceData = new HashMap<>();
@@ -102,26 +118,56 @@ public void ShareUsageTracker_RepeatEvidence_BeforeSessionTimeout() {
 
     @Test
     public void ShareUsageTracker_RepeatEvidence_AfterSessionTimeout() throws InterruptedException {
+        ShareUsageTracker shareUsageTracker = new ShareUsageTracker(
+            new CacheConfiguration(new LruPutCache.Builder(), 1000),
+            EXPIRY_INTERVAL_MILLIS,
+            evidenceKeyFilter);
         int trackedEvents = 0;
         for (int i = 0; i < 2; i++) {
             if (shareUsageTracker.track(data)) {
                 trackedEvents++;
             }
 
-            // Wait some time equal to the interval to elapse.
-            Thread.sleep(interval);
+            // Wait for the interval to pass, with a margin.
+            Thread.sleep(EXPIRY_INTERVAL_MILLIS + EXPIRY_MARGIN_MILLIS);
         }
 
         assertEquals(2, trackedEvents);
     }
 
+    /**
+     * A repeat is shared at most once in each interval. Sharing a repeat after
+     * the interval starts a new interval, so a repeat straight after it is not
+     * shared.
+     */
     @Test
-    public void ShareUsageTracker_Session_Track() throws InterruptedException {
-        shareUsageTracker = new ShareUsageTracker(
+    public void ShareUsageTracker_RepeatEvidence_SharedOncePerInterval()
+        throws InterruptedException {
+        long interval = 500;
+        ShareUsageTracker shareUsageTracker = new ShareUsageTracker(
             new CacheConfiguration(new LruPutCache.Builder(), 1000),
             interval,
             evidenceKeyFilter);
 
+        assertTrue(shareUsageTracker.track(data),
+            "New evidence was not shared.");
+        Thread.sleep(interval + EXPIRY_MARGIN_MILLIS);
+        assertTrue(shareUsageTracker.track(data),
+            "A repeat after the interval was not shared.");
+        assertFalse(shareUsageTracker.track(data),
+            "A repeat straight after a repeat was shared was also shared, " +
+                "so the time it was shared was not stored.");
+    }
+
+    @Test
+    public void ShareUsageTracker_Session_Track() {
+        // The interval does not expire, so only the session cookie keeps the
+        // two requests apart.
+        shareUsageTracker = new ShareUsageTracker(
+            new CacheConfiguration(new LruPutCache.Builder(), 1000),
+            NO_EXPIRY_INTERVAL_MILLIS,
+            evidenceKeyFilter);
+
         int trackedEvents = 0;
         for (int i = 0; i < 2; i++) {
             final Map<String, Object> evidenceData = new HashMap<>();
@@ -138,7 +184,6 @@ public DataKey answer(InvocationOnMock invocationOnMock) throws Throwable {
                     }
                 }
             );
-            Thread.sleep(interval);
             if (shareUsageTracker.track(data)) {
                 trackedEvents++;
             }
@@ -156,7 +201,7 @@ public void ShareUsageTracker_Session_DoNotTrack() {
             sessionCookieName);
         ShareUsageTracker shareUsageTracker = new ShareUsageTracker(
             new CacheConfiguration(new LruPutCache.Builder(), 1000),
-            interval,
+            NO_EXPIRY_INTERVAL_MILLIS,
             evidenceKeyFilter);
 
         int trackedEvents = 0;