From 2b4fe019d74a340e1816937d219ca70da9f3f6a1 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Thu, 12 Nov 2026 21:50:33 +0000 Subject: [PATCH 1/5] Test that usage sharing finds its values without copying the Evidence --- .../data/ShareUsageEvidenceTests.java | 117 ++++++++++++++++++ 1 file changed, 117 insertions(+) create mode 100644 pipeline/pipeline.engines.fiftyone/src/test/java/fiftyone/pipeline/engines/fiftyone/data/ShareUsageEvidenceTests.java diff --git a/pipeline/pipeline.engines.fiftyone/src/test/java/fiftyone/pipeline/engines/fiftyone/data/ShareUsageEvidenceTests.java b/pipeline/pipeline.engines.fiftyone/src/test/java/fiftyone/pipeline/engines/fiftyone/data/ShareUsageEvidenceTests.java new file mode 100644 index 00000000..fee9b726 --- /dev/null +++ b/pipeline/pipeline.engines.fiftyone/src/test/java/fiftyone/pipeline/engines/fiftyone/data/ShareUsageEvidenceTests.java @@ -0,0 +1,117 @@ +/* ********************************************************************* + * This Original Work is copyright of 51 Degrees Mobile Experts Limited. + * Copyright 2026 51 Degrees Mobile Experts Limited, Davidson House, + * Forbury Square, Reading, Berkshire, United Kingdom RG1 3EU. + * + * This Original Work is licensed under the European Union Public Licence + * (EUPL) v.1.2 and is subject to its terms as set out below. + * + * If a copy of the EUPL was not distributed with this file, You can obtain + * one at https://opensource.org/licenses/EUPL-1.2. + * + * The 'Compatible Licences' set out in the Appendix to the EUPL (as may be + * amended by the European Commission) shall be deemed incompatible for + * the purposes of the Work and the provisions of the compatibility + * clause in Article 5 of the EUPL shall not apply. + * + * If using the Work as, or as part of, a network application, by + * including the attribution notice(s) required under Article 5 of the EUPL + * in the end user terms of the application under an appropriate heading, + * such notice(s) shall fulfill the requirements of that article. + * ********************************************************************* */ + +package fiftyone.pipeline.engines.fiftyone.data; + +import fiftyone.pipeline.core.data.FlowData; +import fiftyone.pipeline.core.flowelements.Pipeline; +import fiftyone.pipeline.core.flowelements.PipelineBuilder; +import fiftyone.pipeline.engines.testhelpers.flowelements.EmptyEngineBuilder; +import org.junit.jupiter.api.Test; +import org.slf4j.LoggerFactory; + +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; + +/** + * The usage sharing element finds the values it shares on every request, so + * with no web request finding them reads the Evidence rather than copying + * every value of it. + */ +public class ShareUsageEvidenceTests { + + private static Pipeline createPipeline() throws Exception { + return new PipelineBuilder(LoggerFactory.getILoggerFactory()) + .addFlowElement(new EmptyEngineBuilder( + LoggerFactory.getILoggerFactory()).build()) + .setAutoCloseElements(true) + .build(); + } + + /** + * With no web request the values found are the Evidence itself, so a + * value added afterwards is among them. A copy would not hold it. + */ + @Test + public void Find_NoWebRequest_ReadsTheEvidenceWithoutCopying() + throws Exception { + try (Pipeline pipeline = createPipeline(); + FlowData data = pipeline.createFlowData()) { + data.addEvidence("header.user-agent", "evidence-ua"); + Map found = + ShareUsageEvidence.find(data.getEvidence()); + data.addEvidence("header.x-later", "later"); + + assertEquals("later", found.get("header.x-later"), + "The Evidence was copied, so every request pays for a copy " + + "of every Evidence value."); + } + } + + /** + * With no web request the values found are still keyed without regard + * to case. + */ + @Test + public void Find_NoWebRequest_KeyedWithoutRegardToCase() + throws Exception { + try (Pipeline pipeline = createPipeline(); + FlowData data = pipeline.createFlowData()) { + data.addEvidence("header.User-Agent", "evidence-ua"); + Map found = + ShareUsageEvidence.find(data.getEvidence()); + + assertEquals("evidence-ua", found.get("HEADER.USER-AGENT")); + assertEquals(1, found.size()); + } + } + + /** + * With a web request its values are added under their own keys, the + * Evidence value is kept for a key in both, and the web request itself + * is not one of the values. + */ + @Test + public void Find_WebRequest_AddsItsValuesAndKeepsTheEvidenceValue() + throws Exception { + Map request = new HashMap<>(); + request.put("header.User-Agent", "request-ua"); + request.put("header.X-Test", "shared"); + try (Pipeline pipeline = createPipeline(); + FlowData data = pipeline.createFlowData()) { + data.addEvidence("header.user-agent", "evidence-ua"); + data.addEvidence(ShareUsageEvidence.HTTP_REQUEST_KEY, + Collections.unmodifiableMap(request)); + Map found = + ShareUsageEvidence.find(data.getEvidence()); + + assertEquals("evidence-ua", found.get("HEADER.USER-AGENT")); + assertEquals("shared", found.get("header.x-test")); + assertFalse(found.containsKey(ShareUsageEvidence.HTTP_REQUEST_KEY)); + assertEquals(2, found.size()); + } + } +} From 0d134f2f1d5beb08fe99545bacdfc79512ceffc1 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Thu, 12 Nov 2026 23:23:12 +0000 Subject: [PATCH 2/5] Count how often usage sharing reads all its values in one call Usage sharing finds the values it shares on every call, and most calls are repeats it does not share. A call that is not shared copies every Evidence value twice, once in the element and once in its tracker, and a call that is shared copies them three times. A web request is copied as often. --- .../ShareUsageEvidenceReadTests.java | 201 ++++++++++++++++++ 1 file changed, 201 insertions(+) create mode 100644 pipeline/pipeline.engines.fiftyone/src/test/java/fiftyone/pipeline/engines/fiftyone/flowelements/ShareUsageEvidenceReadTests.java diff --git a/pipeline/pipeline.engines.fiftyone/src/test/java/fiftyone/pipeline/engines/fiftyone/flowelements/ShareUsageEvidenceReadTests.java b/pipeline/pipeline.engines.fiftyone/src/test/java/fiftyone/pipeline/engines/fiftyone/flowelements/ShareUsageEvidenceReadTests.java new file mode 100644 index 00000000..06ff0643 --- /dev/null +++ b/pipeline/pipeline.engines.fiftyone/src/test/java/fiftyone/pipeline/engines/fiftyone/flowelements/ShareUsageEvidenceReadTests.java @@ -0,0 +1,201 @@ +/* ********************************************************************* + * This Original Work is copyright of 51 Degrees Mobile Experts Limited. + * Copyright 2026 51 Degrees Mobile Experts Limited, Davidson House, + * Forbury Square, Reading, Berkshire, United Kingdom RG1 3EU. + * + * This Original Work is licensed under the European Union Public Licence + * (EUPL) v.1.2 and is subject to its terms as set out below. + * + * If a copy of the EUPL was not distributed with this file, You can obtain + * one at https://opensource.org/licenses/EUPL-1.2. + * + * The 'Compatible Licences' set out in the Appendix to the EUPL (as may be + * amended by the European Commission) shall be deemed incompatible for + * the purposes of the Work and the provisions of the compatibility + * clause in Article 5 of the EUPL shall not apply. + * + * If using the Work as, or as part of, a network application, by + * including the attribution notice(s) required under Article 5 of the EUPL + * in the end user terms of the application under an appropriate heading, + * such notice(s) shall fulfill the requirements of that article. + * ********************************************************************* */ + +package fiftyone.pipeline.engines.fiftyone.flowelements; + +import fiftyone.pipeline.core.data.FlowData; +import fiftyone.pipeline.engines.fiftyone.data.ShareUsageEvidence; +import fiftyone.pipeline.engines.services.DataUploader; +import fiftyone.pipeline.engines.testhelpers.data.MockFlowData; +import org.junit.jupiter.api.Test; +import org.slf4j.LoggerFactory; + +import java.util.AbstractMap; +import java.util.Map; +import java.util.Set; +import java.util.TreeMap; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.Mockito.mock; + +/** + * Usage sharing finds the values it shares on every call, and most calls + * repeat one already shared and are not shared again, so finding them must + * not copy every value more than once. These tests count how often all the + * values of the Evidence and of the web request are read in one call. + */ +public class ShareUsageEvidenceReadTests { + + /** + * Enough values that copying them is most of the cost of a call. + */ + private static final int VALUE_COUNT = 100; + + /** + * A map keyed without regard to case and holding no null value, as + * Evidence and the web request are, that counts how often all its + * values are read. + */ + private static final class CountingMap + extends AbstractMap { + + private final Map values = + new TreeMap<>(String.CASE_INSENSITIVE_ORDER); + + private int reads; + + /** + * @return the reads since this was last called + */ + int takeReads() { + int result = reads; + reads = 0; + return result; + } + + @Override + public Set> entrySet() { + reads++; + return values.entrySet(); + } + + @Override + public Object get(Object key) { + return values.get(key); + } + + @Override + public boolean containsKey(Object key) { + return values.containsKey(key); + } + + @Override + public Object put(String key, Object value) { + return values.put(key, value); + } + + @Override + public int size() { + return values.size(); + } + } + + private static CountingMap headers() { + CountingMap headers = new CountingMap(); + for (int i = 0; i < VALUE_COUNT; i++) { + headers.put("header.x-test-" + i, "value-" + i); + } + return headers; + } + + /** + * An element with the default tracker, sharing every call that is not + * a repeat, and sending nothing until 50 calls are queued. + */ + private static ShareUsageElement createElement() throws Exception { + return new ShareUsageBuilder( + LoggerFactory.getILoggerFactory(), + mock(DataUploader.class)) + .build(); + } + + /** + * With no web request the Evidence is read once to make the session + * tracking key, and a call that is shared reads it once more for the + * values it shares. Neither copies it. + */ + @Test + public void NoWebRequest_EvidenceReadOnceWhenNotShared() + throws Exception { + CountingMap evidence = headers(); + ShareUsageElement element = createElement(); + try { + FlowData data = MockFlowData.createFromEvidence(evidence, false); + evidence.takeReads(); + + element.process(data); + int sharedReads = evidence.takeReads(); + assertEquals(1, element.evidenceCollection.size(), + "The first call was not shared."); + + element.process(data); + int notSharedReads = evidence.takeReads(); + assertEquals(1, element.evidenceCollection.size(), + "The repeat was shared."); + + assertEquals(1, notSharedReads, + "A call that is not shared read all the Evidence more than " + + "once, for the session tracking key."); + assertEquals(2, sharedReads, + "A call that is shared read all the Evidence more than " + + "twice, for the session tracking key and the values " + + "shared."); + } finally { + element.close(); + } + } + + /** + * With a web request the Evidence and the web request are each read + * once, to make the one copy that the session tracking key and the + * values shared both come from. + */ + @Test + public void WebRequest_EachReadOnceWhetherSharedOrNot() throws Exception { + CountingMap request = headers(); + CountingMap evidence = new CountingMap(); + evidence.put("header.user-agent", "evidence-ua"); + evidence.put(ShareUsageEvidence.HTTP_REQUEST_KEY, request); + ShareUsageElement element = createElement(); + try { + FlowData data = MockFlowData.createFromEvidence(evidence, false); + evidence.takeReads(); + request.takeReads(); + + element.process(data); + int sharedEvidenceReads = evidence.takeReads(); + int sharedRequestReads = request.takeReads(); + assertEquals(1, element.evidenceCollection.size(), + "The first call was not shared."); + + element.process(data); + int notSharedEvidenceReads = evidence.takeReads(); + int notSharedRequestReads = request.takeReads(); + assertEquals(1, element.evidenceCollection.size(), + "The repeat was shared."); + + assertEquals(1, notSharedRequestReads, + "A call that is not shared copied the web request more " + + "than once."); + assertEquals(1, notSharedEvidenceReads, + "A call that is not shared copied the Evidence more than " + + "once."); + assertEquals(1, sharedRequestReads, + "A call that is shared copied the web request more than " + + "once."); + assertEquals(1, sharedEvidenceReads, + "A call that is shared copied the Evidence more than once."); + } finally { + element.close(); + } + } +} From db808b006cc4e6bdf786aa433e4ffe04465f6292 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Thu, 12 Nov 2026 23:23:35 +0000 Subject: [PATCH 3/5] Find the usage sharing values once a call, without copying the Evidence Without a web request ShareUsageEvidence.find returns the Evidence's own read-only map, which is already keyed without regard to case and holds no null value, rather than a copy of every value. With a web request the element makes the one copy and gives it to its ShareUsageTracker, which makes its key from it once for both the look up and the store of its cache, rather than finding the values again for each. --- .../fiftyone/data/ShareUsageEvidence.java | 8 ++- .../fiftyone/flowelements/ShareUsageBase.java | 17 +++++- .../fiftyone/trackers/ShareUsageTracker.java | 52 ++++++++++++++++--- 3 files changed, 69 insertions(+), 8 deletions(-) diff --git a/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/data/ShareUsageEvidence.java b/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/data/ShareUsageEvidence.java index a4843828..d6e7d22d 100644 --- a/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/data/ShareUsageEvidence.java +++ b/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/data/ShareUsageEvidence.java @@ -50,11 +50,17 @@ private ShareUsageEvidence() { * Every Evidence value, and every value of the web request when the * Evidence holds it, keyed without regard to case. A key in both keeps * the Evidence value. The web request itself is not one of the values. + * Without a web request the values found are the Evidence's own + * read-only map, as Evidence is already keyed without regard to case and + * holds no null value, so nothing is copied. * @param evidence the Evidence of the request - * @return the values found + * @return the values found, which the caller must not change */ public static Map find(Evidence evidence) { Map values = evidence.asKeyMap(); + if (values.containsKey(HTTP_REQUEST_KEY) == false) { + return values; + } Map found = new TreeMap<>(String.CASE_INSENSITIVE_ORDER); Object request = null; diff --git a/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/flowelements/ShareUsageBase.java b/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/flowelements/ShareUsageBase.java index 9e417dcf..083cc30e 100644 --- a/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/flowelements/ShareUsageBase.java +++ b/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/flowelements/ShareUsageBase.java @@ -795,7 +795,7 @@ private void processData(FlowData data, Map found) { logger.debug("processData()"); if (random.nextDouble() <= sharePercentage) { // Check if the tracker will allow sharing of this data - if (tracker.track(data)) { + if (track(data, found)) { // Extract the data we want from the evidence and add // it to the collection. try { @@ -832,6 +832,21 @@ private void processData(FlowData data, Map found) { } } + /** + * Whether the tracker allows the call to be shared. A + * {@link ShareUsageTracker} is given the values already found, so it does + * not find them again. + * @param data the {@link FlowData} instance that provides the evidence + * @param found the values {@link ShareUsageEvidence#find} found for it + * @return true if the call should be shared + */ + private boolean track(FlowData data, Map found) { + if (tracker instanceof ShareUsageTracker) { + return ((ShareUsageTracker) tracker).track(data, found); + } + return tracker.track(data); + } + /** * Extract the desired data from the values found. * In order to avoid problems with the evidence data being disposed before diff --git a/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/trackers/ShareUsageTracker.java b/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/trackers/ShareUsageTracker.java index 8bfa52d3..44107532 100644 --- a/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/trackers/ShareUsageTracker.java +++ b/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/trackers/ShareUsageTracker.java @@ -53,6 +53,12 @@ public class ShareUsageTracker extends TrackerBase { */ private final long interval; + /** + * The key of the call this thread is tracking, set only while + * {@link #track(FlowData, Map)} runs. + */ + private final ThreadLocal callKey = new ThreadLocal<>(); + /** * Construct a new instance. * @param configuration the cache configuration to use when building the @@ -96,17 +102,51 @@ protected EvidenceKeyFilter getFilter() { return filter; } + @Override + public boolean track(FlowData data) { + return track(data, ShareUsageEvidence.find(data.getEvidence())); + } + + /** + * Tracks a call from the values {@link ShareUsageEvidence#find} has + * already found for it, so they are not found again. The key is made + * once, and the look up and the store of the cache both use it. + * @param data the {@link FlowData} of the call + * @param found the values found for the call + * @return true if the values should be shared + */ + public boolean track(FlowData data, Map found) { + callKey.set(keyOf(found)); + try { + return super.track(data); + } finally { + callKey.remove(); + } + } + /** - * The key is made from the values {@link ShareUsageEvidence#find} finds - * that the filter includes, which take in the web request's values, - * as the session tracking hash is of the values that would be shared. - * The Evidence alone does not hold them. + * The key of the call being tracked on this thread when there is one, + * otherwise a key made from the values found for the data. */ @Override protected DataKey generateKey(FlowData data) { + DataKey key = callKey.get(); + if (key != null) { + return key; + } + return keyOf(ShareUsageEvidence.find(data.getEvidence())); + } + + /** + * The key is made from the values found that the filter includes, which + * take in the web request's values, as the session tracking hash is of + * the values that would be shared. The Evidence alone does not hold them. + * @param found the values {@link ShareUsageEvidence#find} found + * @return the key + */ + private DataKey keyOf(Map found) { DataKeyBuilder builder = new DataKeyBuilderDefault(); - for (Map.Entry entry : - ShareUsageEvidence.find(data.getEvidence()).entrySet()) { + for (Map.Entry entry : found.entrySet()) { if (filter.include(entry.getKey())) { builder.add( filter.order(entry.getKey()), From c736ca5c210d4914be744bf35d9e4e7a1660037d Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Fri, 13 Nov 2026 00:37:21 +0000 Subject: [PATCH 4/5] Count the reads of a call that is not shared The read count tests assert what a call that is not shared costs, which is the call the tracker turns down, as most calls repeat one already shared. A call that is shared reads its values again to share them. --- .../ShareUsageEvidenceReadTests.java | 56 +++++++------------ 1 file changed, 21 insertions(+), 35 deletions(-) diff --git a/pipeline/pipeline.engines.fiftyone/src/test/java/fiftyone/pipeline/engines/fiftyone/flowelements/ShareUsageEvidenceReadTests.java b/pipeline/pipeline.engines.fiftyone/src/test/java/fiftyone/pipeline/engines/fiftyone/flowelements/ShareUsageEvidenceReadTests.java index 06ff0643..d3d919e7 100644 --- a/pipeline/pipeline.engines.fiftyone/src/test/java/fiftyone/pipeline/engines/fiftyone/flowelements/ShareUsageEvidenceReadTests.java +++ b/pipeline/pipeline.engines.fiftyone/src/test/java/fiftyone/pipeline/engines/fiftyone/flowelements/ShareUsageEvidenceReadTests.java @@ -38,10 +38,11 @@ import static org.mockito.Mockito.mock; /** - * Usage sharing finds the values it shares on every call, and most calls - * repeat one already shared and are not shared again, so finding them must - * not copy every value more than once. These tests count how often all the - * values of the Evidence and of the web request are read in one call. + * Usage sharing is called for every request, and most calls repeat one + * already shared, which the tracker does not share again. Such a call must + * not copy every value of the Evidence, or of the web request, more than + * once. These tests count how often all the values of each are read in a + * call that is not shared. */ public class ShareUsageEvidenceReadTests { @@ -119,9 +120,8 @@ private static ShareUsageElement createElement() throws Exception { } /** - * With no web request the Evidence is read once to make the session - * tracking key, and a call that is shared reads it once more for the - * values it shares. Neither copies it. + * With no web request a call that is not shared reads the Evidence once, + * to make the session tracking key, and does not copy it. */ @Test public void NoWebRequest_EvidenceReadOnceWhenNotShared() @@ -130,37 +130,31 @@ public void NoWebRequest_EvidenceReadOnceWhenNotShared() ShareUsageElement element = createElement(); try { FlowData data = MockFlowData.createFromEvidence(evidence, false); - evidence.takeReads(); - element.process(data); - int sharedReads = evidence.takeReads(); assertEquals(1, element.evidenceCollection.size(), "The first call was not shared."); + evidence.takeReads(); element.process(data); - int notSharedReads = evidence.takeReads(); + int reads = evidence.takeReads(); assertEquals(1, element.evidenceCollection.size(), "The repeat was shared."); - assertEquals(1, notSharedReads, + assertEquals(1, reads, "A call that is not shared read all the Evidence more than " + - "once, for the session tracking key."); - assertEquals(2, sharedReads, - "A call that is shared read all the Evidence more than " + - "twice, for the session tracking key and the values " + - "shared."); + "once, when the session tracking key needs it once."); } finally { element.close(); } } /** - * With a web request the Evidence and the web request are each read - * once, to make the one copy that the session tracking key and the - * values shared both come from. + * With a web request a call that is not shared reads the Evidence and + * the web request once each, to make the one copy that the session + * tracking key is made from. */ @Test - public void WebRequest_EachReadOnceWhetherSharedOrNot() throws Exception { + public void WebRequest_EachCopiedOnceWhenNotShared() throws Exception { CountingMap request = headers(); CountingMap evidence = new CountingMap(); evidence.put("header.user-agent", "evidence-ua"); @@ -168,32 +162,24 @@ public void WebRequest_EachReadOnceWhetherSharedOrNot() throws Exception { ShareUsageElement element = createElement(); try { FlowData data = MockFlowData.createFromEvidence(evidence, false); - evidence.takeReads(); - request.takeReads(); - element.process(data); - int sharedEvidenceReads = evidence.takeReads(); - int sharedRequestReads = request.takeReads(); assertEquals(1, element.evidenceCollection.size(), "The first call was not shared."); + evidence.takeReads(); + request.takeReads(); element.process(data); - int notSharedEvidenceReads = evidence.takeReads(); - int notSharedRequestReads = request.takeReads(); + int evidenceReads = evidence.takeReads(); + int requestReads = request.takeReads(); assertEquals(1, element.evidenceCollection.size(), "The repeat was shared."); - assertEquals(1, notSharedRequestReads, + assertEquals(1, requestReads, "A call that is not shared copied the web request more " + "than once."); - assertEquals(1, notSharedEvidenceReads, + assertEquals(1, evidenceReads, "A call that is not shared copied the Evidence more than " + "once."); - assertEquals(1, sharedRequestReads, - "A call that is shared copied the web request more than " + - "once."); - assertEquals(1, sharedEvidenceReads, - "A call that is shared copied the Evidence more than once."); } finally { element.close(); } From ee271a371d76c5bc8e350ddb7eae2f98fb7b6904 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Fri, 13 Nov 2026 00:37:37 +0000 Subject: [PATCH 5/5] Find the usage sharing values only when they are needed The element no longer finds the values on every call. It finds them for the ignore filter when one is set, and otherwise only once the tracker allows the call to be shared. The tracker finds them for its key as it did, which without a web request copies nothing. Giving the element's copy to the tracker for each call measured slower than the copy it saved on a call with one Evidence value, so ShareUsageTracker is as it is on rc. --- .../fiftyone/flowelements/ShareUsageBase.java | 34 +++++------- .../fiftyone/trackers/ShareUsageTracker.java | 52 +++---------------- 2 files changed, 19 insertions(+), 67 deletions(-) diff --git a/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/flowelements/ShareUsageBase.java b/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/flowelements/ShareUsageBase.java index 083cc30e..1fd5bf08 100644 --- a/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/flowelements/ShareUsageBase.java +++ b/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/flowelements/ShareUsageBase.java @@ -645,12 +645,16 @@ public void addPipeline(Pipeline pipeline) { @Override protected void processInternal(FlowData flowData){ boolean ignoreData = false; - Map found = - ShareUsageEvidence.find(flowData.getEvidence()); + // The values are found here only for the ignore filter. Otherwise + // they are found once the tracker allows the call to be shared, as + // most calls repeat one already shared. + Map found = null; logger.debug("processInternal()"); - if (ignoreDataEvidenceFilter != null) { + if (ignoreDataEvidenceFilter != null && + ignoreDataEvidenceFilter.isEmpty() == false) { + found = ShareUsageEvidence.find(flowData.getEvidence()); for (Map.Entry entry : ignoreDataEvidenceFilter) { String key = entry.getKey(); if (found.containsKey(key)) { @@ -789,18 +793,21 @@ private boolean isLocalHost(String address) throws UnknownHostException { /** * Process the supplied request data * @param data the {@link FlowData} instance that provides the evidence - * @param found the values {@link ShareUsageEvidence#find} found for it + * @param found the values {@link ShareUsageEvidence#find} found for it, + * or null if they have not been found yet */ private void processData(FlowData data, Map found) { logger.debug("processData()"); if (random.nextDouble() <= sharePercentage) { // Check if the tracker will allow sharing of this data - if (track(data, found)) { + if (tracker.track(data)) { + Map shared = found != null ? found : + ShareUsageEvidence.find(data.getEvidence()); // Extract the data we want from the evidence and add // it to the collection. try { if (evidenceCollection.offer( - getDataFromEvidence(found), + getDataFromEvidence(shared), addTimeout, TimeUnit.MILLISECONDS)) { queueFullReported = false; @@ -832,21 +839,6 @@ private void processData(FlowData data, Map found) { } } - /** - * Whether the tracker allows the call to be shared. A - * {@link ShareUsageTracker} is given the values already found, so it does - * not find them again. - * @param data the {@link FlowData} instance that provides the evidence - * @param found the values {@link ShareUsageEvidence#find} found for it - * @return true if the call should be shared - */ - private boolean track(FlowData data, Map found) { - if (tracker instanceof ShareUsageTracker) { - return ((ShareUsageTracker) tracker).track(data, found); - } - return tracker.track(data); - } - /** * Extract the desired data from the values found. * In order to avoid problems with the evidence data being disposed before diff --git a/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/trackers/ShareUsageTracker.java b/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/trackers/ShareUsageTracker.java index 44107532..8bfa52d3 100644 --- a/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/trackers/ShareUsageTracker.java +++ b/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/trackers/ShareUsageTracker.java @@ -53,12 +53,6 @@ public class ShareUsageTracker extends TrackerBase { */ private final long interval; - /** - * The key of the call this thread is tracking, set only while - * {@link #track(FlowData, Map)} runs. - */ - private final ThreadLocal callKey = new ThreadLocal<>(); - /** * Construct a new instance. * @param configuration the cache configuration to use when building the @@ -102,51 +96,17 @@ protected EvidenceKeyFilter getFilter() { return filter; } - @Override - public boolean track(FlowData data) { - return track(data, ShareUsageEvidence.find(data.getEvidence())); - } - - /** - * Tracks a call from the values {@link ShareUsageEvidence#find} has - * already found for it, so they are not found again. The key is made - * once, and the look up and the store of the cache both use it. - * @param data the {@link FlowData} of the call - * @param found the values found for the call - * @return true if the values should be shared - */ - public boolean track(FlowData data, Map found) { - callKey.set(keyOf(found)); - try { - return super.track(data); - } finally { - callKey.remove(); - } - } - /** - * The key of the call being tracked on this thread when there is one, - * otherwise a key made from the values found for the data. + * The key is made from the values {@link ShareUsageEvidence#find} finds + * that the filter includes, which take in the web request's values, + * as the session tracking hash is of the values that would be shared. + * The Evidence alone does not hold them. */ @Override protected DataKey generateKey(FlowData data) { - DataKey key = callKey.get(); - if (key != null) { - return key; - } - return keyOf(ShareUsageEvidence.find(data.getEvidence())); - } - - /** - * The key is made from the values found that the filter includes, which - * take in the web request's values, as the session tracking hash is of - * the values that would be shared. The Evidence alone does not hold them. - * @param found the values {@link ShareUsageEvidence#find} found - * @return the key - */ - private DataKey keyOf(Map found) { DataKeyBuilder builder = new DataKeyBuilderDefault(); - for (Map.Entry entry : found.entrySet()) { + for (Map.Entry entry : + ShareUsageEvidence.find(data.getEvidence()).entrySet()) { if (filter.include(entry.getKey())) { builder.add( filter.order(entry.getKey()),