From 576e34319ab17a6135030b4a5a1a9dc97b12f43e Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Wed, 4 Nov 2026 12:17:25 +0000 Subject: [PATCH 1/9] Close the FlowData and metadata collections the library opens on every path The servlet filter closed a request's FlowData only when the rest of the filter chain returned normally, and the result service lost the FlowData when processing it failed. The Spring interceptor closed it in afterCompletion, which Spring does not call when preHandle returns false, so the FlowData of every request for the JavaScript or JSON was never closed. Java never frees native results by itself, so each of those requests kept its native results, and through them the engine's data set, until the process ended. ProfileMetaData.getName iterated the native values collection without closing it, which held the data set until the garbage collector finalized the collection. --- ci/pmd-resource-ruleset.xml | 74 +++++++++++++++++++ .../onpremise/data/ProfileMetaDataHash.java | 16 +++- .../onpremise/data/ProfileMetaDataIPI.java | 16 +++- .../mvc/components/FiftyOneInterceptor.java | 25 +++++-- .../web/mvc/FiftyOneInterceptorTests.java | 15 ++++ .../fiftyone/pipeline/web/PipelineFilter.java | 41 +++++----- .../services/PipelineResultServiceCore.java | 23 +++++- .../pipeline/web/PipelineFilterTests.java | 41 +++++++++- 8 files changed, 209 insertions(+), 42 deletions(-) create mode 100644 ci/pmd-resource-ruleset.xml diff --git a/ci/pmd-resource-ruleset.xml b/ci/pmd-resource-ruleset.xml new file mode 100644 index 000000000..bc1193153 --- /dev/null +++ b/ci/pmd-resource-ruleset.xml @@ -0,0 +1,74 @@ + + + + + Fails the build when a FlowData, a Pipeline or a collection of + metadata is not closed on every path. The native results in a FlowData + and the native metadata collections each hold the engine's data set, + so if one is left open, closing the Pipeline cannot free the data set. + Java never frees the native results by itself. + + Where something is closed by its owner elsewhere, such as the FlowData + the web integration closes at the end of a request, the code says so + with @SuppressWarnings and a comment naming the owner. + + + + + Every FlowData created is closed by a try-with-resources statement, + unless it is returned to the caller or kept in a field. + + 3 + + + + + + + + + + + + Every CloseableIterable a method returns, such as the profiles or + the components of an engine, is closed by a try-with-resources + statement, unless it is returned to the caller. + + 3 + + + + + + + + + + + + + + + + + diff --git a/device-detection/device-detection.hash.engine.on-premise/src/main/java/fiftyone/devicedetection/hash/engine/onpremise/data/ProfileMetaDataHash.java b/device-detection/device-detection.hash.engine.on-premise/src/main/java/fiftyone/devicedetection/hash/engine/onpremise/data/ProfileMetaDataHash.java index 7603773ae..afadcd5b2 100644 --- a/device-detection/device-detection.hash.engine.on-premise/src/main/java/fiftyone/devicedetection/hash/engine/onpremise/data/ProfileMetaDataHash.java +++ b/device-detection/device-detection.hash.engine.on-premise/src/main/java/fiftyone/devicedetection/hash/engine/onpremise/data/ProfileMetaDataHash.java @@ -114,11 +114,19 @@ public ComponentMetaData getComponent() { @Override public String getName() { List values = new ArrayList<>(); - for (ValueMetaData value : getValues()) { - if (value.getProperty().getDisplayOrder() > 0 && - value.getName().equalsIgnoreCase("N/A") == false) { - values.add(value); + // The native collection holds the data set until it is closed. The + // values taken from it are separate objects, so they outlive it. + try (CloseableIterable profileValues = getValues()) { + for (ValueMetaData value : profileValues) { + if (value.getProperty().getDisplayOrder() > 0 && + value.getName().equalsIgnoreCase("N/A") == false) { + values.add(value); + } } + } catch (RuntimeException e) { + throw e; + } catch (Exception e) { + throw new IllegalStateException(e); } ValueMetaData[] valuesArray = new ValueMetaData[0]; valuesArray = values.toArray(valuesArray); diff --git a/ip-intelligence/ip-intelligence.engine.on-premise/src/main/java/fiftyone/ipintelligence/engine/onpremise/data/ProfileMetaDataIPI.java b/ip-intelligence/ip-intelligence.engine.on-premise/src/main/java/fiftyone/ipintelligence/engine/onpremise/data/ProfileMetaDataIPI.java index b9ee322df..3f2645723 100644 --- a/ip-intelligence/ip-intelligence.engine.on-premise/src/main/java/fiftyone/ipintelligence/engine/onpremise/data/ProfileMetaDataIPI.java +++ b/ip-intelligence/ip-intelligence.engine.on-premise/src/main/java/fiftyone/ipintelligence/engine/onpremise/data/ProfileMetaDataIPI.java @@ -114,11 +114,19 @@ public ComponentMetaData getComponent() { @Override public String getName() { List values = new ArrayList<>(); - for (ValueMetaData value : getValues()) { - if (value.getProperty().getDisplayOrder() > 0 && - value.getName().equalsIgnoreCase("N/A") == false) { - values.add(value); + // The native collection holds the data set until it is closed. The + // values taken from it are separate objects, so they outlive it. + try (CloseableIterable profileValues = getValues()) { + for (ValueMetaData value : profileValues) { + if (value.getProperty().getDisplayOrder() > 0 && + value.getName().equalsIgnoreCase("N/A") == false) { + values.add(value); + } } + } catch (RuntimeException e) { + throw e; + } catch (Exception e) { + throw new IllegalStateException(e); } ValueMetaData[] valuesArray = new ValueMetaData[0]; valuesArray = values.toArray(valuesArray); diff --git a/pipeline/pipeline.web.packages/pipeline.web.mvc/src/main/java/fiftyone/pipeline/web/mvc/components/FiftyOneInterceptor.java b/pipeline/pipeline.web.packages/pipeline.web.mvc/src/main/java/fiftyone/pipeline/web/mvc/components/FiftyOneInterceptor.java index 351b3b5da..309b7669b 100644 --- a/pipeline/pipeline.web.packages/pipeline.web.mvc/src/main/java/fiftyone/pipeline/web/mvc/components/FiftyOneInterceptor.java +++ b/pipeline/pipeline.web.packages/pipeline.web.mvc/src/main/java/fiftyone/pipeline/web/mvc/components/FiftyOneInterceptor.java @@ -127,15 +127,24 @@ public boolean preHandle( HttpServletResponse response, Object handler) throws Exception { resultService.process(request); - - // Set the Client Hints request headers - uachService.setResponseHeaders(request, response); - - if (fiftyOneJsService.serveJS(request, response) == false && - fiftyOneJsService.serveJson(request, response) == false) { - return super.preHandle(request, response, handler); + + boolean proceed = false; + try { + // Set the Client Hints request headers + uachService.setResponseHeaders(request, response); + + proceed = fiftyOneJsService.serveJS(request, response) == false && + fiftyOneJsService.serveJson(request, response) == false && + super.preHandle(request, response, handler); + return proceed; + } finally { + // Spring calls afterCompletion, which closes the FlowData, only + // when preHandle returns true. In every other case, serving the + // JavaScript or JSON included, the FlowData is closed here. + if (proceed == false) { + flowDataProvider.getFlowData(request).close(); + } } - return false; } @Override diff --git a/pipeline/pipeline.web.packages/pipeline.web.mvc/src/test/java/fiftyone/pipeline/web/mvc/FiftyOneInterceptorTests.java b/pipeline/pipeline.web.packages/pipeline.web.mvc/src/test/java/fiftyone/pipeline/web/mvc/FiftyOneInterceptorTests.java index 2b9562ada..077a88b46 100644 --- a/pipeline/pipeline.web.packages/pipeline.web.mvc/src/test/java/fiftyone/pipeline/web/mvc/FiftyOneInterceptorTests.java +++ b/pipeline/pipeline.web.packages/pipeline.web.mvc/src/test/java/fiftyone/pipeline/web/mvc/FiftyOneInterceptorTests.java @@ -22,6 +22,7 @@ package fiftyone.pipeline.web.mvc; +import static org.junit.Assert.assertFalse; import static org.mockito.ArgumentMatchers.*; import static org.mockito.Mockito.*; @@ -123,4 +124,18 @@ public void FiftyOneInterceptor_AfterCompletion() throws Exception { verify(flowData, times(1)) .close(); } + + /** + * Check that the FlowData is closed when preHandle serves the JavaScript + * itself, as Spring does not call afterCompletion when preHandle returns + * false. + * @throws Exception + */ + @Test + public void FiftyOneInterceptor_PreHandle_ServesJavaScript_ClosesFlowData() + throws Exception { + assertFalse(interceptor.preHandle(request, response, handler)); + verify(flowData, times(1)) + .close(); + } } diff --git a/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/PipelineFilter.java b/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/PipelineFilter.java index 1834f5a56..098da0926 100644 --- a/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/PipelineFilter.java +++ b/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/PipelineFilter.java @@ -122,27 +122,30 @@ public void doFilter( // Populate the request properties and store against the // HttpContext. resultService.process((HttpServletRequest)request); - - // Get FlowData from the request. - FlowData flowData = flowDataProviderCore.getFlowData((HttpServletRequest)request); - - // Set UACH response headers. - uachServiceCore.setResponseHeaders( - flowData, (HttpServletResponse)response); - - // If 51Degrees JavaScript or JSON is being requested then serve it. - // Otherwise continue down the filter Pipeline. - if (jsService.serveJS((HttpServletRequest)request, (HttpServletResponse) response) == false && - jsService.serveJson((HttpServletRequest)request, (HttpServletResponse) response) == false) { - chain.doFilter(request, response);//sends request to next resource - } - // The rest of the filters in the chain have now been called. So it is - // time to dispose of the FlowData instance. try { - flowDataProviderCore.getFlowData((HttpServletRequest) request).close(); - } catch (Exception e) { - throw new ServletException("FlowData could not be disposed of.", e); + // Get FlowData from the request. + FlowData flowData = flowDataProviderCore.getFlowData((HttpServletRequest)request); + + // Set UACH response headers. + uachServiceCore.setResponseHeaders( + flowData, (HttpServletResponse)response); + + // If 51Degrees JavaScript or JSON is being requested then serve it. + // Otherwise continue down the filter Pipeline. + if (jsService.serveJS((HttpServletRequest)request, (HttpServletResponse) response) == false && + jsService.serveJson((HttpServletRequest)request, (HttpServletResponse) response) == false) { + chain.doFilter(request, response);//sends request to next resource + } + } finally { + // The rest of the filters in the chain have now been called, or + // one of them failed. Either way it is time to dispose of the + // FlowData instance. + try { + flowDataProviderCore.getFlowData((HttpServletRequest) request).close(); + } catch (Exception e) { + throw new ServletException("FlowData could not be disposed of.", e); + } } } diff --git a/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/services/PipelineResultServiceCore.java b/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/services/PipelineResultServiceCore.java index 9bc21609a..289c90ff9 100644 --- a/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/services/PipelineResultServiceCore.java +++ b/pipeline/pipeline.web.packages/pipeline.web/src/main/java/fiftyone/pipeline/web/services/PipelineResultServiceCore.java @@ -71,14 +71,29 @@ public Default( this.pipeline = pipeline; } + /** + * The FlowData is kept in the request, and whatever handles the end + * of the request closes it. If processing fails it never reaches the + * request, so it is closed here instead. + */ @Override + @SuppressWarnings("PMD.FlowDataInTryWithResources") public void process(HttpServletRequest request) { // Create the flowData FlowData flowData = pipeline.createFlowData(); - // Extract the required pieces of evidence from the request - evidenceService.addEvidenceFromRequest(flowData, request); - // Start processing the data - flowData.process(); + try { + // Extract the required pieces of evidence from the request + evidenceService.addEvidenceFromRequest(flowData, request); + // Start processing the data + flowData.process(); + } catch (RuntimeException | Error e) { + try { + flowData.close(); + } catch (Exception closeException) { + e.addSuppressed(closeException); + } + throw e; + } // Store the FlowData in the request request.setAttribute(HTTPCONTEXT_FLOWDATA, flowData); } diff --git a/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/PipelineFilterTests.java b/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/PipelineFilterTests.java index e4362e3ae..e0f7b7508 100644 --- a/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/PipelineFilterTests.java +++ b/pipeline/pipeline.web.packages/pipeline.web/src/test/java/fiftyone/pipeline/web/PipelineFilterTests.java @@ -25,6 +25,7 @@ import fiftyone.pipeline.core.data.ElementData; import fiftyone.pipeline.core.data.FlowData; +import org.junit.After; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -41,7 +42,9 @@ import static fiftyone.pipeline.engines.fiftyone.data.SetHeadersData.*; import static fiftyone.pipeline.engines.fiftyone.flowelements.SetHeadersElement.*; import static fiftyone.pipeline.web.Constants.*; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.fail; import static org.mockito.ArgumentMatchers.*; import static org.mockito.Mockito.*; @@ -54,6 +57,7 @@ public class PipelineFilterTests { private FilterChain chain; private FlowData flowData; private ElementData elementData; + private PipelineFilter filter; @Before public void init() throws IOException, ServletException { @@ -102,13 +106,23 @@ public void init() throws IOException, ServletException { }).when(request).setAttribute(eq(HTTPCONTEXT_FLOWDATA), any(Object.class)); } + /** + * Close the pipeline each filter built. + */ + @After + public void cleanUp() { + if (filter != null) { + filter.destroy(); + } + } + /** * Check that the filter can be initialised. * @throws ServletException */ @Test public void PipelineFilter_Initialise() throws ServletException { - PipelineFilter filter = new PipelineFilter(); + filter = new PipelineFilter(); filter.init(config); } @@ -121,7 +135,7 @@ public void PipelineFilter_Initialise() throws ServletException { */ @Test public void PipelineFilter_Process() throws ServletException, IOException { - PipelineFilter filter = new PipelineFilter(); + filter = new PipelineFilter(); filter.init(config); filter.doFilter(request, response, chain); @@ -142,10 +156,31 @@ public void PipelineFilter_Process() throws ServletException, IOException { */ @Test public void PipelineFilter_CloseFlowData() throws Exception { - PipelineFilter filter = new PipelineFilter(); + filter = new PipelineFilter(); filter.init(config); filter.doFilter(request, response, chain); verify(flowData, times(1)).close(); } + + /** + * Check that the filter disposes of the FlowData when a later filter in + * the chain fails. + * @throws Exception + */ + @Test + public void PipelineFilter_CloseFlowData_ChainFails() throws Exception { + doThrow(new ServletException("chain failed")) + .when(chain).doFilter(any(ServletRequest.class), any(ServletResponse.class)); + filter = new PipelineFilter(); + + filter.init(config); + try { + filter.doFilter(request, response, chain); + fail("Expected the exception from the chain"); + } catch (ServletException e) { + assertEquals("chain failed", e.getMessage()); + } + verify(flowData, times(1)).close(); + } } From 1ec40c32ea028a243d3bd4fa0811dd4aaad95c2b Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Wed, 4 Nov 2026 12:17:29 +0000 Subject: [PATCH 2/9] Close every FlowData, Pipeline, engine and metadata collection in tests and examples The pipeline builder tests built 38 on-premise pipelines, each with its data file loaded, and closed none of them. Other tests left FlowData, pipelines, share usage elements and their sending threads, and metadata collections holding the data set, open on some or all paths. Several examples built engines into a pipeline that was not asked to close them, or closed the pipeline outside a finally block. --- .../hash/engine/onpremise/WrapperHash.java | 7 +- .../onpremise/data/DataValidatorHash.java | 13 +- .../onpremise/data/MetaDataHasherHash.java | 61 +- .../onpremise/flowelements/EngineTests.java | 7 +- .../shared/testhelpers/Wrapper.java | 7 +- .../testhelpers/data/MetaDataHasher.java | 8 +- .../shared/testhelpers/data/ValueTests.java | 7 +- ...DetectionOnPremisePipelineBuilderTest.java | 211 +++--- .../examples/console/MetadataOnPrem.java | 36 +- .../examples/console/ShareUsageExample.java | 5 +- .../console/GettingStartedCloudMixed.java | 3 + .../examples/web/GettingStartedApi.java | 15 +- .../engine/onpremise/WrapperHash.java | 7 +- .../onpremise/data/MetaDataHasherHash.java | 61 +- .../onpremise/flowelements/EngineTests.java | 7 +- .../shared/testhelpers/Wrapper.java | 7 +- .../testhelpers/data/MetaDataHasher.java | 8 +- ...elligenceOnPremisePipelineBuilderTest.java | 148 ++-- .../examples/CombiningServices.java | 18 +- .../examples/cloud/GettingStarted.java | 13 +- .../CloudAspectEngineBaseTests.java | 33 +- .../flowelements/CloudRequestEngineTests.java | 71 +- .../core/flowelements/FlowDataTests.java | 7 +- .../flowelements/ParallelElementsTests.java | 55 +- .../flowelements/PipelineBuilderTests.java | 119 +-- .../core/flowelements/PipelineTests.java | 45 +- .../developerexamples/cloudengine/Main.java | 49 +- .../developerexamples/flowelement/Main.java | 31 +- .../onpremiseengine/Main.java | 32 +- .../developerexamples/usagesharing/Main.java | 40 +- .../flowelements/SetHeadersElementTests.java | 96 +-- .../flowelements/ShareUsageElementTests.java | 685 +++++++++--------- .../performance/ShareUsageOverheadTests.java | 103 +-- 33 files changed, 1103 insertions(+), 912 deletions(-) diff --git a/device-detection/device-detection.hash.engine.on-premise/src/test/java/fiftyone/devicedetection/hash/engine/onpremise/WrapperHash.java b/device-detection/device-detection.hash.engine.on-premise/src/test/java/fiftyone/devicedetection/hash/engine/onpremise/WrapperHash.java index 2d034f6ff..21bdb6fb7 100644 --- a/device-detection/device-detection.hash.engine.on-premise/src/test/java/fiftyone/devicedetection/hash/engine/onpremise/WrapperHash.java +++ b/device-detection/device-detection.hash.engine.on-premise/src/test/java/fiftyone/devicedetection/hash/engine/onpremise/WrapperHash.java @@ -28,6 +28,7 @@ import fiftyone.pipeline.core.flowelements.Pipeline; import fiftyone.pipeline.core.flowelements.PipelineBuilder; import fiftyone.pipeline.engines.Constants; +import fiftyone.pipeline.engines.fiftyone.data.CloseableIterable; import fiftyone.pipeline.engines.fiftyone.data.ComponentMetaData; import fiftyone.pipeline.engines.fiftyone.data.FiftyOneAspectPropertyMetaData; import fiftyone.pipeline.engines.fiftyone.data.ProfileMetaData; @@ -70,15 +71,15 @@ public Iterable getProperties() { } @Override - public Iterable getValues() { + public CloseableIterable getValues() { return engine.getValues(); } - public Iterable getProfiles() { + public CloseableIterable getProfiles() { return engine.getProfiles(); } - public Iterable getComponents() { + public CloseableIterable getComponents() { return engine.getComponents(); } diff --git a/device-detection/device-detection.hash.engine.on-premise/src/test/java/fiftyone/devicedetection/hash/engine/onpremise/data/DataValidatorHash.java b/device-detection/device-detection.hash.engine.on-premise/src/test/java/fiftyone/devicedetection/hash/engine/onpremise/data/DataValidatorHash.java index 336a52141..ddd55d599 100644 --- a/device-detection/device-detection.hash.engine.on-premise/src/test/java/fiftyone/devicedetection/hash/engine/onpremise/data/DataValidatorHash.java +++ b/device-detection/device-detection.hash.engine.on-premise/src/test/java/fiftyone/devicedetection/hash/engine/onpremise/data/DataValidatorHash.java @@ -29,6 +29,7 @@ import fiftyone.pipeline.core.data.FlowData; import fiftyone.pipeline.engines.data.AspectPropertyValue; import fiftyone.pipeline.engines.exceptions.NoValueException; +import fiftyone.pipeline.engines.fiftyone.data.CloseableIterable; import fiftyone.pipeline.engines.fiftyone.data.ComponentMetaData; import fiftyone.pipeline.engines.fiftyone.data.FiftyOneAspectPropertyMetaData; @@ -91,8 +92,16 @@ public void validateData(FlowData data, boolean validEvidence) throws NoValueExc // rather than a fixed value, and that valid evidence produces at least // one result. int componentCount = 0; - for (ComponentMetaData component : engine.getComponents()) { - componentCount++; + // The collection holds the engine's data set until it is closed. + try (CloseableIterable components = + engine.getComponents()) { + for (ComponentMetaData component : components) { + componentCount++; + } + } catch (RuntimeException e) { + throw e; + } catch (Exception e) { + throw new IllegalStateException(e); } int matchedUserAgents = elementData.getUserAgents().getValue().size(); assertTrue( diff --git a/device-detection/device-detection.hash.engine.on-premise/src/test/java/fiftyone/devicedetection/hash/engine/onpremise/data/MetaDataHasherHash.java b/device-detection/device-detection.hash.engine.on-premise/src/test/java/fiftyone/devicedetection/hash/engine/onpremise/data/MetaDataHasherHash.java index f793a46fc..293c790d8 100644 --- a/device-detection/device-detection.hash.engine.on-premise/src/test/java/fiftyone/devicedetection/hash/engine/onpremise/data/MetaDataHasherHash.java +++ b/device-detection/device-detection.hash.engine.on-premise/src/test/java/fiftyone/devicedetection/hash/engine/onpremise/data/MetaDataHasherHash.java @@ -25,6 +25,7 @@ import fiftyone.devicedetection.hash.engine.onpremise.flowelements.Constants; import fiftyone.devicedetection.shared.testhelpers.Wrapper; import fiftyone.devicedetection.shared.testhelpers.data.MetaDataHasher; +import fiftyone.pipeline.engines.fiftyone.data.CloseableIterable; import fiftyone.pipeline.engines.fiftyone.data.ComponentMetaData; import fiftyone.pipeline.engines.fiftyone.data.FiftyOneAspectPropertyMetaData; import fiftyone.pipeline.engines.fiftyone.data.ProfileMetaData; @@ -34,7 +35,7 @@ public class MetaDataHasherHash implements MetaDataHasher { @Override - public int hashProperties(int hash, Wrapper wrapper) { + public int hashProperties(int hash, Wrapper wrapper) throws Exception { int i = 0; Iterable properties = wrapper.getProperties(); for (FiftyOneAspectPropertyMetaData property : properties) { @@ -62,50 +63,56 @@ public int hashProperties(int hash, Wrapper wrapper) { } @Override - public int hashValues(int hash, Wrapper wrapper) { + public int hashValues(int hash, Wrapper wrapper) throws Exception { int i = 0; - for (ValueMetaData value : wrapper.getValues()) { - if (i % 100 == 0) { - hash ^= value.hashCode(); - hash ^= value.getProperty().hashCode(); + try (CloseableIterable values = wrapper.getValues()) { + for (ValueMetaData value : values) { + if (i % 100 == 0) { + hash ^= value.hashCode(); + hash ^= value.getProperty().hashCode(); + } + i++; } - i++; } return hash; } @Override - public int hashComponents(int hash, Wrapper wrapper) { - for (ComponentMetaData component : wrapper.getComponents()) { - hash ^= component.hashCode(); - int i = 0; - for (FiftyOneAspectPropertyMetaData property : component.getProperties()) { - if (i % 10 == 0) { - hash ^= property.hashCode(); + public int hashComponents(int hash, Wrapper wrapper) throws Exception { + try (CloseableIterable components = wrapper.getComponents()) { + for (ComponentMetaData component : components) { + hash ^= component.hashCode(); + int i = 0; + for (FiftyOneAspectPropertyMetaData property : component.getProperties()) { + if (i % 10 == 0) { + hash ^= property.hashCode(); + } + i++; } - i++; + hash ^= component.getDefaultProfile().hashCode(); } - hash ^= component.getDefaultProfile().hashCode(); } return hash; } @Override - public int hashProfiles(int hash, Wrapper wrapper) { + public int hashProfiles(int hash, Wrapper wrapper) throws Exception { int i = 0; - for (ProfileMetaData profile : wrapper.getProfiles()) { - if (i % 100 == 0) { - hash ^= profile.hashCode(); - hash ^= profile.getComponent().hashCode(); - int j = 0; - for (ValueMetaData value : profile.getValues()) { - if (j % 10 == 0) { - hash ^= value.hashCode(); + try (CloseableIterable profiles = wrapper.getProfiles()) { + for (ProfileMetaData profile : profiles) { + if (i % 100 == 0) { + hash ^= profile.hashCode(); + hash ^= profile.getComponent().hashCode(); + int j = 0; + for (ValueMetaData value : profile.getValues()) { + if (j % 10 == 0) { + hash ^= value.hashCode(); + } + j++; } - j++; } + i++; } - i++; } return hash; } diff --git a/device-detection/device-detection.hash.engine.on-premise/src/test/java/fiftyone/devicedetection/hash/engine/onpremise/flowelements/EngineTests.java b/device-detection/device-detection.hash.engine.on-premise/src/test/java/fiftyone/devicedetection/hash/engine/onpremise/flowelements/EngineTests.java index b73fbd83c..3f1a7a2cd 100644 --- a/device-detection/device-detection.hash.engine.on-premise/src/test/java/fiftyone/devicedetection/hash/engine/onpremise/flowelements/EngineTests.java +++ b/device-detection/device-detection.hash.engine.on-premise/src/test/java/fiftyone/devicedetection/hash/engine/onpremise/flowelements/EngineTests.java @@ -62,9 +62,10 @@ private void testDataSourceTier(String fileName) throws Exception { assumeTrue(e.getMessage(), false); } - DeviceDetectionHashEngine engine = createEngine(dataFile); - String tier = engine.getDataSourceTier(); - engine.close(); + String tier; + try (DeviceDetectionHashEngine engine = createEngine(dataFile)) { + tier = engine.getDataSourceTier(); + } if (fileName.equals(LITE_HASH_DATA_FILE_NAME)) { assertEquals("Lite", tier); diff --git a/device-detection/device-detection.shared/src/test/java/fiftyone/devicedetection/shared/testhelpers/Wrapper.java b/device-detection/device-detection.shared/src/test/java/fiftyone/devicedetection/shared/testhelpers/Wrapper.java index 45fc54abe..82a7b6454 100644 --- a/device-detection/device-detection.shared/src/test/java/fiftyone/devicedetection/shared/testhelpers/Wrapper.java +++ b/device-detection/device-detection.shared/src/test/java/fiftyone/devicedetection/shared/testhelpers/Wrapper.java @@ -24,6 +24,7 @@ import fiftyone.pipeline.core.flowelements.Pipeline; import fiftyone.pipeline.engines.data.AspectData; +import fiftyone.pipeline.engines.fiftyone.data.CloseableIterable; import fiftyone.pipeline.engines.fiftyone.data.ComponentMetaData; import fiftyone.pipeline.engines.fiftyone.data.FiftyOneAspectPropertyMetaData; import fiftyone.pipeline.engines.fiftyone.data.ProfileMetaData; @@ -38,11 +39,11 @@ public interface Wrapper extends Closeable { Iterable getProperties(); - Iterable getValues(); + CloseableIterable getValues(); - Iterable getProfiles(); + CloseableIterable getProfiles(); - Iterable getComponents(); + CloseableIterable getComponents(); FiftyOneAspectEngine getEngine(); } \ No newline at end of file diff --git a/device-detection/device-detection.shared/src/test/java/fiftyone/devicedetection/shared/testhelpers/data/MetaDataHasher.java b/device-detection/device-detection.shared/src/test/java/fiftyone/devicedetection/shared/testhelpers/data/MetaDataHasher.java index 49240aecd..17fd7796e 100644 --- a/device-detection/device-detection.shared/src/test/java/fiftyone/devicedetection/shared/testhelpers/data/MetaDataHasher.java +++ b/device-detection/device-detection.shared/src/test/java/fiftyone/devicedetection/shared/testhelpers/data/MetaDataHasher.java @@ -25,11 +25,11 @@ import fiftyone.devicedetection.shared.testhelpers.Wrapper; public interface MetaDataHasher { - int hashProperties(int hash, Wrapper wrapper); + int hashProperties(int hash, Wrapper wrapper) throws Exception; - int hashValues(int hash, Wrapper wrapper); + int hashValues(int hash, Wrapper wrapper) throws Exception; - int hashComponents(int hash, Wrapper wrapper); + int hashComponents(int hash, Wrapper wrapper) throws Exception; - int hashProfiles(int hash, Wrapper wrapper); + int hashProfiles(int hash, Wrapper wrapper) throws Exception; } diff --git a/device-detection/device-detection.shared/src/test/java/fiftyone/devicedetection/shared/testhelpers/data/ValueTests.java b/device-detection/device-detection.shared/src/test/java/fiftyone/devicedetection/shared/testhelpers/data/ValueTests.java index 148e3a6e0..6826fb4d2 100644 --- a/device-detection/device-detection.shared/src/test/java/fiftyone/devicedetection/shared/testhelpers/data/ValueTests.java +++ b/device-detection/device-detection.shared/src/test/java/fiftyone/devicedetection/shared/testhelpers/data/ValueTests.java @@ -30,6 +30,7 @@ import fiftyone.pipeline.core.data.FlowData; import fiftyone.pipeline.engines.data.AspectPropertyValue; import fiftyone.pipeline.engines.exceptions.PropertyMissingException; +import fiftyone.pipeline.engines.fiftyone.data.CloseableIterable; import fiftyone.pipeline.engines.fiftyone.data.ComponentMetaData; import fiftyone.pipeline.engines.fiftyone.data.FiftyOneAspectPropertyMetaData; @@ -67,8 +68,10 @@ public static void matchedUserAgents(Wrapper wrapper) throws Exception { // depends on the data file, so assert the bound here and validate // every matched substring below. int componentCount = 0; - for (ComponentMetaData component : wrapper.getComponents()) { - componentCount++; + try (CloseableIterable components = wrapper.getComponents()) { + for (ComponentMetaData component : components) { + componentCount++; + } } int matchedCount = device.getUserAgents().getValue().size(); assertTrue( diff --git a/device-detection/device-detection/src/test/java/fiftyone/devicedetection/DeviceDetectionOnPremisePipelineBuilderTest.java b/device-detection/device-detection/src/test/java/fiftyone/devicedetection/DeviceDetectionOnPremisePipelineBuilderTest.java index c9430a527..21b9e5bd4 100644 --- a/device-detection/device-detection/src/test/java/fiftyone/devicedetection/DeviceDetectionOnPremisePipelineBuilderTest.java +++ b/device-detection/device-detection/src/test/java/fiftyone/devicedetection/DeviceDetectionOnPremisePipelineBuilderTest.java @@ -87,12 +87,13 @@ private ConfigHashSwig getConfigHashSwig(Pipeline pipeline) throws NoSuchFieldEx @Test public void setFilename_PropertyPropagatedToDataFileConfiguration() throws Exception { - Pipeline pipeline = builder + try (Pipeline pipeline = builder .setFilename(HASH_DATA_FILE_NAME, true) - .build(); - DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline); - assertEquals(HASH_DATA_FILE_NAME, dataFileConfiguration.getDataFilePath()); - assertTrue(dataFileConfiguration.getCreateTempDataCopy()); + .build()) { + DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline); + assertEquals(HASH_DATA_FILE_NAME, dataFileConfiguration.getDataFilePath()); + assertTrue(dataFileConfiguration.getCreateTempDataCopy()); + } } @Test @@ -118,210 +119,232 @@ public void setFilename_OtherExtension_ExceptionThrown() { @Test public void setEngineData_PropertyPropagatedToDataFileConfiguration() throws Exception { byte[] data = Files.readAllBytes(datafile.toPath()); - Pipeline pipeline = new DeviceDetectionPipelineBuilder(loggerFactory) + try (Pipeline pipeline = new DeviceDetectionPipelineBuilder(loggerFactory) .useOnPremise(new byte[]{}, Enums.DeviceDetectionAlgorithm.Hash) .setEngineData(data) .setShareUsage(false) - .build(); - DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline); - assertEquals(data, dataFileConfiguration.getData()); + .build()) { + DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline); + assertEquals(data, dataFileConfiguration.getData()); + } } @Test public void setEngineData_FilenameIsSet_PropertyNotPropagated() throws Exception { byte[] data = Files.readAllBytes(datafile.toPath()); - Pipeline pipeline = builder + try (Pipeline pipeline = builder .setEngineData(data) - .build(); - DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline); - assertNull(dataFileConfiguration.getData()); + .build()) { + DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline); + assertNull(dataFileConfiguration.getData()); + } } @Test public void setShareUsage_False_SharedFlowElementNotAdded() throws Exception { - Pipeline pipeline = new DeviceDetectionPipelineBuilder(loggerFactory) + try (Pipeline pipeline = new DeviceDetectionPipelineBuilder(loggerFactory) .useOnPremise(datafile.getAbsolutePath(), false) .setShareUsage(false) - .build(); - List flowElements = pipeline.getFlowElements(); - assertEquals(1, flowElements.size()); - assertFalse(flowElements.get(0) instanceof ShareUsageElement); + .build()) { + List flowElements = pipeline.getFlowElements(); + assertEquals(1, flowElements.size()); + assertFalse(flowElements.get(0) instanceof ShareUsageElement); + } } @Test public void setShareUsage_True_SharedFlowElementAdded() throws Exception { - Pipeline pipeline = new DeviceDetectionPipelineBuilder(loggerFactory) + try (Pipeline pipeline = new DeviceDetectionPipelineBuilder(loggerFactory) .useOnPremise(datafile.getAbsolutePath(), false) .setShareUsage(true) - .build(); - List flowElements = pipeline.getFlowElements(); - assertEquals(2, flowElements.size()); - assertTrue(flowElements.get(0) instanceof ShareUsageElement); + .build()) { + List flowElements = pipeline.getFlowElements(); + assertEquals(2, flowElements.size()); + assertTrue(flowElements.get(0) instanceof ShareUsageElement); + } } @Test public void setAutoUpdate_PropertyPropagatedToDataFileConfiguration() throws Exception { - Pipeline pipeline = builder + try (Pipeline pipeline = builder .setAutoUpdate(true) - .build(); - DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline); - assertTrue(dataFileConfiguration.getAutomaticUpdatesEnabled()); + .build()) { + DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline); + assertTrue(dataFileConfiguration.getAutomaticUpdatesEnabled()); + } } @Test public void setDataFileSystemWatcher_PropertyPropagatedToDataFileConfiguration() throws Exception { - Pipeline pipeline = builder + try (Pipeline pipeline = builder .setDataFileSystemWatcher(true) - .build(); - DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline); - assertTrue(dataFileConfiguration.getFileSystemWatcherEnabled()); + .build()) { + DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline); + assertTrue(dataFileConfiguration.getFileSystemWatcherEnabled()); + } } @Test public void setDataUpdateService_RegisterDataFileTriggered() throws Exception { DataUpdateServiceDefault dataUpdateServiceDefault = mock(DataUpdateServiceDefault.class); - Pipeline pipeline = builder + try (Pipeline ignored = builder .setAutoUpdate(true) .setDataUpdateService(dataUpdateServiceDefault) - .build(); - verify(dataUpdateServiceDefault).registerDataFile(any()); + .build()) { + verify(dataUpdateServiceDefault).registerDataFile(any()); + } } @Test public void setDataUpdateOnStartup_PropertyPropagatedToDataFileConfiguration() throws Exception { - Pipeline pipeline = builder + try (Pipeline pipeline = builder .setDataUpdateOnStartup(true) - .build(); - DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline); - assertTrue(dataFileConfiguration.getUpdateOnStartup()); + .build()) { + DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline); + assertTrue(dataFileConfiguration.getUpdateOnStartup()); + } } @Test public void setUpdatePollingInterval_PropertyPropagatedToDataFileConfiguration() throws Exception { - Pipeline pipeline = builder + try (Pipeline pipeline = builder .setUpdatePollingInterval(10) - .build(); - DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline); - assertEquals(10, dataFileConfiguration.getPollingIntervalSeconds()); + .build()) { + DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline); + assertEquals(10, dataFileConfiguration.getPollingIntervalSeconds()); + } } @Test public void setUpdatePollingIntervalMillis_PropertyPropagatedToDataFileConfiguration() throws Exception { - Pipeline pipeline = builder + try (Pipeline pipeline = builder .setUpdatePollingIntervalMillis(10000) - .build(); - DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline); - assertEquals(10, dataFileConfiguration.getPollingIntervalSeconds()); + .build()) { + DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline); + assertEquals(10, dataFileConfiguration.getPollingIntervalSeconds()); + } } @Test public void setUpdateRandomisationMax_PropertyPropagatedToDataFileConfiguration() throws Exception { - Pipeline pipeline = builder + try (Pipeline pipeline = builder .setUpdateRandomisationMax(123) - .build(); - DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline); - assertEquals(123, dataFileConfiguration.getMaxRandomisationSeconds()); + .build()) { + DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline); + assertEquals(123, dataFileConfiguration.getMaxRandomisationSeconds()); + } } @Test public void setUpdateRandomisationMaxMillis_PropertyPropagatedToDataFileConfiguration() throws Exception { - Pipeline pipeline = builder + try (Pipeline pipeline = builder .setUpdateRandomisationMaxMillis(123000) - .build(); - DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline); - assertEquals(123, dataFileConfiguration.getMaxRandomisationSeconds()); + .build()) { + DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline); + assertEquals(123, dataFileConfiguration.getMaxRandomisationSeconds()); + } } @Test public void setDataUpdateLicenseKey_PropertyPropagatedToDataFileConfiguration() throws Exception { String key = "licence_key"; - Pipeline pipeline = builder + try (Pipeline pipeline = builder .setDataUpdateLicenseKey(key) - .build(); - DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline); - assertTrue(dataFileConfiguration.getDataUpdateLicenseKeys().contains(key)); + .build()) { + DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline); + assertTrue(dataFileConfiguration.getDataUpdateLicenseKeys().contains(key)); + } } @Test public void setConcurrency_PropertyPropagatedSwigConfiguration() throws Exception { - Pipeline pipeline = builder + try (Pipeline pipeline = builder .setConcurrency(4) - .build(); - ConfigHashSwig swig = getConfigHashSwig(pipeline); - assertEquals(4, swig.getConcurrency()); + .build()) { + ConfigHashSwig swig = getConfigHashSwig(pipeline); + assertEquals(4, swig.getConcurrency()); + } } @Test public void setDifference_PropertyPropagatedSwigConfiguration() throws Exception { - Pipeline pipeline = builder + try (Pipeline pipeline = builder .setDifference(123) - .build(); - ConfigHashSwig swig = getConfigHashSwig(pipeline); - assertEquals(123, swig.getDifference()); + .build()) { + ConfigHashSwig swig = getConfigHashSwig(pipeline); + assertEquals(123, swig.getDifference()); + } } @Test public void setAllowUnmatched_PropertyPropagatedSwigConfiguration() throws Exception { - Pipeline pipeline = builder + try (Pipeline pipeline = builder .setAllowUnmatched(true) - .build(); - ConfigHashSwig swig = getConfigHashSwig(pipeline); - assertTrue(swig.getAllowUnmatched()); + .build()) { + ConfigHashSwig swig = getConfigHashSwig(pipeline); + assertTrue(swig.getAllowUnmatched()); + } } @Test public void setDrift_PropertyPropagatedSwigConfiguration() throws Exception { - Pipeline pipeline = builder + try (Pipeline pipeline = builder .setDrift(123) - .build(); - ConfigHashSwig swig = getConfigHashSwig(pipeline); - assertEquals(123, swig.getDrift()); + .build()) { + ConfigHashSwig swig = getConfigHashSwig(pipeline); + assertEquals(123, swig.getDrift()); + } } @Test public void setUsePerformanceGraph_PropertyPropagatedSwigConfiguration() throws Exception { - Pipeline pipeline = builder + try (Pipeline pipeline = builder .setUsePerformanceGraph(true) - .build(); - ConfigHashSwig swig = getConfigHashSwig(pipeline); - assertTrue(swig.getUsePerformanceGraph()); + .build()) { + ConfigHashSwig swig = getConfigHashSwig(pipeline); + assertTrue(swig.getUsePerformanceGraph()); + } } @Test public void setUsePredictiveGraph_PropertyPropagatedSwigConfiguration() throws Exception { - Pipeline pipeline = builder + try (Pipeline pipeline = builder .setUsePredictiveGraph(true) - .build(); - ConfigHashSwig swig = getConfigHashSwig(pipeline); - assertTrue(swig.getUsePredictiveGraph()); + .build()) { + ConfigHashSwig swig = getConfigHashSwig(pipeline); + assertTrue(swig.getUsePredictiveGraph()); + } } @Test public void setDataUpdateUrl_PropertyPropagatedToDataFileConfiguration() throws Exception { String url = "https://update.me.com"; - Pipeline pipeline = builder + try (Pipeline pipeline = builder .setDataUpdateUrl(url) - .build(); - DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline); - assertEquals(url, dataFileConfiguration.getDataUpdateUrl()); + .build()) { + DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline); + assertEquals(url, dataFileConfiguration.getDataUpdateUrl()); + } } @Test public void setDataUpdateVerifyMd5_PropertyPropagatedToDataFileConfiguration() throws Exception { - Pipeline pipeline = builder + try (Pipeline pipeline = builder .setDataUpdateVerifyMd5(true) - .build(); - DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline); - assertTrue(dataFileConfiguration.getVerifyMd5()); + .build()) { + DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline); + assertTrue(dataFileConfiguration.getVerifyMd5()); + } } @Test public void setDataUpdateUrlFormatter_PropertyPropagatedToDataFileConfiguration() throws Exception { DataUpdateUrlFormatter urlFormatter = mock(DataUpdateUrlFormatter.class); - Pipeline pipeline = builder + try (Pipeline pipeline = builder .setDataUpdateUrlFormatter(urlFormatter) - .build(); - DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline); - assertEquals(urlFormatter, dataFileConfiguration.getUrlFormatter()); + .build()) { + DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline); + assertEquals(urlFormatter, dataFileConfiguration.getUrlFormatter()); + } } } \ No newline at end of file diff --git a/examples/device-detection/console/src/main/java/fiftyone/devicedetection/examples/console/MetadataOnPrem.java b/examples/device-detection/console/src/main/java/fiftyone/devicedetection/examples/console/MetadataOnPrem.java index a58e739ab..51b696286 100644 --- a/examples/device-detection/console/src/main/java/fiftyone/devicedetection/examples/console/MetadataOnPrem.java +++ b/examples/device-detection/console/src/main/java/fiftyone/devicedetection/examples/console/MetadataOnPrem.java @@ -26,6 +26,7 @@ import fiftyone.devicedetection.hash.engine.onpremise.flowelements.DeviceDetectionHashEngineBuilder; import fiftyone.pipeline.core.data.EvidenceKeyFilterWhitelist; import fiftyone.pipeline.engines.Constants; +import fiftyone.pipeline.engines.fiftyone.data.CloseableIterable; import fiftyone.pipeline.engines.fiftyone.data.ComponentMetaData; import fiftyone.pipeline.engines.fiftyone.data.ProfileMetaData; import fiftyone.pipeline.engines.fiftyone.data.ValueMetaData; @@ -132,23 +133,36 @@ private static void outputEvidenceKeyDetails(DeviceDetectionHashEngine ddEngine, } private static void outputProfileDetails(DeviceDetectionHashEngine ddEngine, - PrintWriter output) { - // Group the profiles by component and then output the number of profiles + PrintWriter output) throws Exception { + // Group the profiles by component and then output the number of profiles // for each component. - Map> groups = - StreamSupport.stream(ddEngine.getProfiles().spliterator(), false) - .collect(Collectors.groupingBy(p -> p.getComponent().getName())); - groups.forEach((k,v)->output.format("%s Profiles: %d\n", k , v.size())); + // + // A try-with-resource block MUST be used for the collections of + // metadata returned by the engine. Each one holds a reference to the + // engine's data set until it is closed, so if it is left open the + // data set cannot be freed when the engine is closed. + Map groups; + try (CloseableIterable profiles = ddEngine.getProfiles()) { + groups = StreamSupport.stream(profiles.spliterator(), false) + .collect(Collectors.groupingBy( + p -> p.getComponent().getName(), + Collectors.counting())); + } + groups.forEach((k,v)->output.format("%s Profiles: %d\n", k , v)); } // Output the component name as well as a list of all the associated properties. // If we're outputting to console then we also add some formatting to make it // more readable. - private static void outputComponents(DeviceDetectionHashEngine ddEngine, PrintWriter output){ - ddEngine.getComponents().forEach(c -> { - output.println("Component - "+ c.getName()); - outputProperties(c, output); - }); + private static void outputComponents(DeviceDetectionHashEngine ddEngine, PrintWriter output) + throws Exception { + // See outputProfileDetails for why this is a try-with-resource block. + try (CloseableIterable components = ddEngine.getComponents()) { + components.forEach(c -> { + output.println("Component - "+ c.getName()); + outputProperties(c, output); + }); + } } private static void outputProperties(ComponentMetaData component, PrintWriter output) { diff --git a/examples/device-detection/console/src/main/java/fiftyone/devicedetection/examples/console/ShareUsageExample.java b/examples/device-detection/console/src/main/java/fiftyone/devicedetection/examples/console/ShareUsageExample.java index 9e9ba2320..93ceecad0 100644 --- a/examples/device-detection/console/src/main/java/fiftyone/devicedetection/examples/console/ShareUsageExample.java +++ b/examples/device-detection/console/src/main/java/fiftyone/devicedetection/examples/console/ShareUsageExample.java @@ -135,9 +135,12 @@ public static void run(InputStream is) throws Exception { // .setIncludedQueryStringParameter("campaign") .build(); - // Build a pipeline with only the ShareUsageElement + // Build a pipeline with only the ShareUsageElement. The pipeline is + // asked to close the element, which sends any data still queued, as + // it is not closed anywhere else. try (Pipeline pipeline = new PipelineBuilder() .addFlowElement(shareUsageElement) + .setAutoCloseElements(true) .build()) { logger.info("Pipeline built successfully with ShareUsageElement"); diff --git a/examples/ip-intelligence/console/src/main/java/fiftyone/ipintelligence/examples/console/GettingStartedCloudMixed.java b/examples/ip-intelligence/console/src/main/java/fiftyone/ipintelligence/examples/console/GettingStartedCloudMixed.java index e5bf11069..efca03fdc 100644 --- a/examples/ip-intelligence/console/src/main/java/fiftyone/ipintelligence/examples/console/GettingStartedCloudMixed.java +++ b/examples/ip-intelligence/console/src/main/java/fiftyone/ipintelligence/examples/console/GettingStartedCloudMixed.java @@ -144,10 +144,13 @@ public static void run(String resourceKey, IPIntelligenceCloudEngine ipIntelligenceEngine = new IPIntelligenceCloudEngineBuilder(loggerFactory).build(); + // The engines are not closed anywhere else, so the pipeline is asked + // to close them when it is closed. try (Pipeline pipeline = new PipelineBuilder(loggerFactory) .addFlowElement(cloudRequestEngine) .addFlowElement(deviceDetectionEngine) .addFlowElement(ipIntelligenceEngine) + .setAutoCloseElements(true) .build()) { // carry out some sample detections diff --git a/examples/ip-intelligence/web/getting-started.api/src/main/java/fiftyone/ipintelligence/examples/web/GettingStartedApi.java b/examples/ip-intelligence/web/getting-started.api/src/main/java/fiftyone/ipintelligence/examples/web/GettingStartedApi.java index 2de381969..d98752dd6 100644 --- a/examples/ip-intelligence/web/getting-started.api/src/main/java/fiftyone/ipintelligence/examples/web/GettingStartedApi.java +++ b/examples/ip-intelligence/web/getting-started.api/src/main/java/fiftyone/ipintelligence/examples/web/GettingStartedApi.java @@ -127,16 +127,29 @@ public void init() throws ServletException { .setAutoUpdate(false) .build(dataFileLocation, false); // The SequenceElement provides the sequence evidence required by - // the JsonBuilderElement. + // the JsonBuilderElement. The pipeline closes all three elements, + // the engine and its data file included, when destroy closes it. pipeline = new PipelineBuilder(LoggerFactory.getILoggerFactory()) .addFlowElement(new SequenceElementBuilder( LoggerFactory.getILoggerFactory()).build()) .addFlowElement(engine) .addFlowElement(new JsonBuilderElementBuilder( LoggerFactory.getILoggerFactory()).build()) + .setAutoCloseElements(true) .build(); DataFileHelper.logDataFileInfo(engine); } catch (Exception e) { + // A servlet whose init fails is never destroyed, so whatever was + // built is closed here. + try { + if (pipeline != null) { + pipeline.close(); + } else if (engine != null) { + engine.close(); + } + } catch (Exception closeException) { + e.addSuppressed(closeException); + } throw new ServletException("Failed to create pipeline from data file '" + dataFileLocation + "'", e); } diff --git a/ip-intelligence/ip-intelligence.engine.on-premise/src/test/java/fiftyone/ipintelligence/engine/onpremise/WrapperHash.java b/ip-intelligence/ip-intelligence.engine.on-premise/src/test/java/fiftyone/ipintelligence/engine/onpremise/WrapperHash.java index d9e861c0f..a001f6e34 100644 --- a/ip-intelligence/ip-intelligence.engine.on-premise/src/test/java/fiftyone/ipintelligence/engine/onpremise/WrapperHash.java +++ b/ip-intelligence/ip-intelligence.engine.on-premise/src/test/java/fiftyone/ipintelligence/engine/onpremise/WrapperHash.java @@ -28,6 +28,7 @@ import fiftyone.pipeline.core.flowelements.Pipeline; import fiftyone.pipeline.core.flowelements.PipelineBuilder; import fiftyone.pipeline.engines.Constants; +import fiftyone.pipeline.engines.fiftyone.data.CloseableIterable; import fiftyone.pipeline.engines.fiftyone.data.ComponentMetaData; import fiftyone.pipeline.engines.fiftyone.data.FiftyOneAspectPropertyMetaData; import fiftyone.pipeline.engines.fiftyone.data.ProfileMetaData; @@ -69,15 +70,15 @@ public Iterable getProperties() { } @Override - public Iterable getValues() { + public CloseableIterable getValues() { return engine.getValues(); } - public Iterable getProfiles() { + public CloseableIterable getProfiles() { return engine.getProfiles(); } - public Iterable getComponents() { + public CloseableIterable getComponents() { return engine.getComponents(); } diff --git a/ip-intelligence/ip-intelligence.engine.on-premise/src/test/java/fiftyone/ipintelligence/engine/onpremise/data/MetaDataHasherHash.java b/ip-intelligence/ip-intelligence.engine.on-premise/src/test/java/fiftyone/ipintelligence/engine/onpremise/data/MetaDataHasherHash.java index 4683157a5..d79b90e23 100644 --- a/ip-intelligence/ip-intelligence.engine.on-premise/src/test/java/fiftyone/ipintelligence/engine/onpremise/data/MetaDataHasherHash.java +++ b/ip-intelligence/ip-intelligence.engine.on-premise/src/test/java/fiftyone/ipintelligence/engine/onpremise/data/MetaDataHasherHash.java @@ -24,6 +24,7 @@ import fiftyone.ipintelligence.shared.testhelpers.Wrapper; import fiftyone.ipintelligence.shared.testhelpers.data.MetaDataHasher; +import fiftyone.pipeline.engines.fiftyone.data.CloseableIterable; import fiftyone.pipeline.engines.fiftyone.data.ComponentMetaData; import fiftyone.pipeline.engines.fiftyone.data.FiftyOneAspectPropertyMetaData; import fiftyone.pipeline.engines.fiftyone.data.ProfileMetaData; @@ -33,7 +34,7 @@ public class MetaDataHasherHash implements MetaDataHasher { @Override - public int hashProperties(int hash, Wrapper wrapper) { + public int hashProperties(int hash, Wrapper wrapper) throws Exception { int i = 0; Iterable properties = wrapper.getProperties(); for (FiftyOneAspectPropertyMetaData property : properties) { @@ -57,50 +58,56 @@ public int hashProperties(int hash, Wrapper wrapper) { } @Override - public int hashValues(int hash, Wrapper wrapper) { + public int hashValues(int hash, Wrapper wrapper) throws Exception { int i = 0; - for (ValueMetaData value : wrapper.getValues()) { - if (i % 100 == 0) { - hash ^= value.hashCode(); - hash ^= value.getProperty().hashCode(); + try (CloseableIterable values = wrapper.getValues()) { + for (ValueMetaData value : values) { + if (i % 100 == 0) { + hash ^= value.hashCode(); + hash ^= value.getProperty().hashCode(); + } + i++; } - i++; } return hash; } @Override - public int hashComponents(int hash, Wrapper wrapper) { - for (ComponentMetaData component : wrapper.getComponents()) { - hash ^= component.hashCode(); - int i = 0; - for (FiftyOneAspectPropertyMetaData property : component.getProperties()) { - if (i % 10 == 0) { - hash ^= property.hashCode(); + public int hashComponents(int hash, Wrapper wrapper) throws Exception { + try (CloseableIterable components = wrapper.getComponents()) { + for (ComponentMetaData component : components) { + hash ^= component.hashCode(); + int i = 0; + for (FiftyOneAspectPropertyMetaData property : component.getProperties()) { + if (i % 10 == 0) { + hash ^= property.hashCode(); + } + i++; } - i++; + hash ^= component.getDefaultProfile().hashCode(); } - hash ^= component.getDefaultProfile().hashCode(); } return hash; } @Override - public int hashProfiles(int hash, Wrapper wrapper) { + public int hashProfiles(int hash, Wrapper wrapper) throws Exception { int i = 0; - for (ProfileMetaData profile : wrapper.getProfiles()) { - if (i % 100 == 0) { - hash ^= profile.hashCode(); - hash ^= profile.getComponent().hashCode(); - int j = 0; - for (ValueMetaData value : profile.getValues()) { - if (j % 10 == 0) { - hash ^= value.hashCode(); + try (CloseableIterable profiles = wrapper.getProfiles()) { + for (ProfileMetaData profile : profiles) { + if (i % 100 == 0) { + hash ^= profile.hashCode(); + hash ^= profile.getComponent().hashCode(); + int j = 0; + for (ValueMetaData value : profile.getValues()) { + if (j % 10 == 0) { + hash ^= value.hashCode(); + } + j++; } - j++; } + i++; } - i++; } return hash; } diff --git a/ip-intelligence/ip-intelligence.engine.on-premise/src/test/java/fiftyone/ipintelligence/engine/onpremise/flowelements/EngineTests.java b/ip-intelligence/ip-intelligence.engine.on-premise/src/test/java/fiftyone/ipintelligence/engine/onpremise/flowelements/EngineTests.java index 4f0d9005b..e12f6cc10 100644 --- a/ip-intelligence/ip-intelligence.engine.on-premise/src/test/java/fiftyone/ipintelligence/engine/onpremise/flowelements/EngineTests.java +++ b/ip-intelligence/ip-intelligence.engine.on-premise/src/test/java/fiftyone/ipintelligence/engine/onpremise/flowelements/EngineTests.java @@ -62,9 +62,10 @@ private void testDataSourceTier(String fileName) throws Exception { assumeTrue(e.getMessage(), false); } - IPIntelligenceOnPremiseEngine engine = createEngine(dataFile); - String tier = engine.getDataSourceTier(); - engine.close(); + String tier; + try (IPIntelligenceOnPremiseEngine engine = createEngine(dataFile)) { + tier = engine.getDataSourceTier(); + } if (fileName.equals(LITE_IPI_DATA_FILE_NAME)) { assertEquals("Lite", tier); diff --git a/ip-intelligence/ip-intelligence.shared/src/test/java/fiftyone/ipintelligence/shared/testhelpers/Wrapper.java b/ip-intelligence/ip-intelligence.shared/src/test/java/fiftyone/ipintelligence/shared/testhelpers/Wrapper.java index 39378f9fa..d74e56ff7 100644 --- a/ip-intelligence/ip-intelligence.shared/src/test/java/fiftyone/ipintelligence/shared/testhelpers/Wrapper.java +++ b/ip-intelligence/ip-intelligence.shared/src/test/java/fiftyone/ipintelligence/shared/testhelpers/Wrapper.java @@ -24,6 +24,7 @@ import fiftyone.pipeline.core.flowelements.Pipeline; import fiftyone.pipeline.engines.data.AspectData; +import fiftyone.pipeline.engines.fiftyone.data.CloseableIterable; import fiftyone.pipeline.engines.fiftyone.data.ComponentMetaData; import fiftyone.pipeline.engines.fiftyone.data.FiftyOneAspectPropertyMetaData; import fiftyone.pipeline.engines.fiftyone.data.ProfileMetaData; @@ -38,11 +39,11 @@ public interface Wrapper extends Closeable { Iterable getProperties(); - Iterable getValues(); + CloseableIterable getValues(); - Iterable getProfiles(); + CloseableIterable getProfiles(); - Iterable getComponents(); + CloseableIterable getComponents(); FiftyOneAspectEngine getEngine(); } \ No newline at end of file diff --git a/ip-intelligence/ip-intelligence.shared/src/test/java/fiftyone/ipintelligence/shared/testhelpers/data/MetaDataHasher.java b/ip-intelligence/ip-intelligence.shared/src/test/java/fiftyone/ipintelligence/shared/testhelpers/data/MetaDataHasher.java index 43ffbbfa7..da6e9b7bd 100644 --- a/ip-intelligence/ip-intelligence.shared/src/test/java/fiftyone/ipintelligence/shared/testhelpers/data/MetaDataHasher.java +++ b/ip-intelligence/ip-intelligence.shared/src/test/java/fiftyone/ipintelligence/shared/testhelpers/data/MetaDataHasher.java @@ -25,11 +25,11 @@ import fiftyone.ipintelligence.shared.testhelpers.Wrapper; public interface MetaDataHasher { - int hashProperties(int hash, Wrapper wrapper); + int hashProperties(int hash, Wrapper wrapper) throws Exception; - int hashValues(int hash, Wrapper wrapper); + int hashValues(int hash, Wrapper wrapper) throws Exception; - int hashComponents(int hash, Wrapper wrapper); + int hashComponents(int hash, Wrapper wrapper) throws Exception; - int hashProfiles(int hash, Wrapper wrapper); + int hashProfiles(int hash, Wrapper wrapper) throws Exception; } diff --git a/ip-intelligence/ip-intelligence/src/test/java/fiftyone/ipintelligence/IPIntelligenceOnPremisePipelineBuilderTest.java b/ip-intelligence/ip-intelligence/src/test/java/fiftyone/ipintelligence/IPIntelligenceOnPremisePipelineBuilderTest.java index 98b386e97..d220eb776 100644 --- a/ip-intelligence/ip-intelligence/src/test/java/fiftyone/ipintelligence/IPIntelligenceOnPremisePipelineBuilderTest.java +++ b/ip-intelligence/ip-intelligence/src/test/java/fiftyone/ipintelligence/IPIntelligenceOnPremisePipelineBuilderTest.java @@ -88,12 +88,13 @@ private ConfigIpiSwig getConfigIpiSwig(Pipeline pipeline) throws NoSuchFieldExce @Test public void setFilename_PropertyPropagatedToDataFileConfiguration() throws Exception { - Pipeline pipeline = builder + try (Pipeline pipeline = builder .setFilename(IPI_DATA_FILE_NAME, true) - .build(); - DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline); - assertEquals(IPI_DATA_FILE_NAME, dataFileConfiguration.getDataFilePath()); - assertTrue(dataFileConfiguration.getCreateTempDataCopy()); + .build()) { + DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline); + assertEquals(IPI_DATA_FILE_NAME, dataFileConfiguration.getDataFilePath()); + assertTrue(dataFileConfiguration.getCreateTempDataCopy()); + } } @Test @@ -118,144 +119,159 @@ public void setFilename_OtherExtension_ExceptionThrown() { @Test public void setShareUsage_False_SharedFlowElementNotAdded() throws Exception { - Pipeline pipeline = new IPIntelligencePipelineBuilder(loggerFactory) + try (Pipeline pipeline = new IPIntelligencePipelineBuilder(loggerFactory) .useOnPremise(datafile.getAbsolutePath(), false) .setShareUsage(false) - .build(); - List flowElements = pipeline.getFlowElements(); - assertEquals(1, flowElements.size()); - assertFalse(flowElements.get(0) instanceof ShareUsageElement); + .build()) { + List flowElements = pipeline.getFlowElements(); + assertEquals(1, flowElements.size()); + assertFalse(flowElements.get(0) instanceof ShareUsageElement); + } } @Test public void setShareUsage_True_SharedFlowElementAdded() throws Exception { - Pipeline pipeline = new IPIntelligencePipelineBuilder(loggerFactory) + try (Pipeline pipeline = new IPIntelligencePipelineBuilder(loggerFactory) .useOnPremise(datafile.getAbsolutePath(), false) .setShareUsage(true) - .build(); - List flowElements = pipeline.getFlowElements(); - assertEquals(2, flowElements.size()); - assertTrue(flowElements.get(0) instanceof ShareUsageElement); + .build()) { + List flowElements = pipeline.getFlowElements(); + assertEquals(2, flowElements.size()); + assertTrue(flowElements.get(0) instanceof ShareUsageElement); + } } @Test public void setAutoUpdate_PropertyPropagatedToDataFileConfiguration() throws Exception { - Pipeline pipeline = builder + try (Pipeline pipeline = builder .setAutoUpdate(true) - .build(); - DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline); - assertTrue(dataFileConfiguration.getAutomaticUpdatesEnabled()); + .build()) { + DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline); + assertTrue(dataFileConfiguration.getAutomaticUpdatesEnabled()); + } } @Test public void setDataFileSystemWatcher_PropertyPropagatedToDataFileConfiguration() throws Exception { - Pipeline pipeline = builder + try (Pipeline pipeline = builder .setDataFileSystemWatcher(true) - .build(); - DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline); - assertTrue(dataFileConfiguration.getFileSystemWatcherEnabled()); + .build()) { + DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline); + assertTrue(dataFileConfiguration.getFileSystemWatcherEnabled()); + } } @Test public void setDataUpdateService_RegisterDataFileTriggered() throws Exception { DataUpdateServiceDefault dataUpdateServiceDefault = mock(DataUpdateServiceDefault.class); - builder + try (Pipeline ignored = builder .setAutoUpdate(true) .setDataUpdateService(dataUpdateServiceDefault) - .build(); - verify(dataUpdateServiceDefault).registerDataFile(any()); + .build()) { + verify(dataUpdateServiceDefault).registerDataFile(any()); + } } @Test public void setDataUpdateOnStartup_PropertyPropagatedToDataFileConfiguration() throws Exception { - Pipeline pipeline = builder + try (Pipeline pipeline = builder .setDataUpdateOnStartup(true) - .build(); - DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline); - assertTrue(dataFileConfiguration.getUpdateOnStartup()); + .build()) { + DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline); + assertTrue(dataFileConfiguration.getUpdateOnStartup()); + } } @Test public void setUpdatePollingInterval_PropertyPropagatedToDataFileConfiguration() throws Exception { - Pipeline pipeline = builder + try (Pipeline pipeline = builder .setUpdatePollingInterval(10) - .build(); - DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline); - assertEquals(10, dataFileConfiguration.getPollingIntervalSeconds()); + .build()) { + DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline); + assertEquals(10, dataFileConfiguration.getPollingIntervalSeconds()); + } } @Test public void setUpdatePollingIntervalMillis_PropertyPropagatedToDataFileConfiguration() throws Exception { - Pipeline pipeline = builder + try (Pipeline pipeline = builder .setUpdatePollingIntervalMillis(10000) - .build(); - DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline); - assertEquals(10, dataFileConfiguration.getPollingIntervalSeconds()); + .build()) { + DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline); + assertEquals(10, dataFileConfiguration.getPollingIntervalSeconds()); + } } @Test public void setUpdateRandomisationMax_PropertyPropagatedToDataFileConfiguration() throws Exception { - Pipeline pipeline = builder + try (Pipeline pipeline = builder .setUpdateRandomisationMax(123) - .build(); - DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline); - assertEquals(123, dataFileConfiguration.getMaxRandomisationSeconds()); + .build()) { + DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline); + assertEquals(123, dataFileConfiguration.getMaxRandomisationSeconds()); + } } @Test public void setUpdateRandomisationMaxMillis_PropertyPropagatedToDataFileConfiguration() throws Exception { - Pipeline pipeline = builder + try (Pipeline pipeline = builder .setUpdateRandomisationMaxMillis(123000) - .build(); - DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline); - assertEquals(123, dataFileConfiguration.getMaxRandomisationSeconds()); + .build()) { + DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline); + assertEquals(123, dataFileConfiguration.getMaxRandomisationSeconds()); + } } @Test public void setDataUpdateLicenseKey_PropertyPropagatedToDataFileConfiguration() throws Exception { String key = "licence_key"; - Pipeline pipeline = builder + try (Pipeline pipeline = builder .setDataUpdateLicenseKey(key) - .build(); - DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline); - assertTrue(dataFileConfiguration.getDataUpdateLicenseKeys().contains(key)); + .build()) { + DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline); + assertTrue(dataFileConfiguration.getDataUpdateLicenseKeys().contains(key)); + } } @Test public void setConcurrency_PropertyPropagatedSwigConfiguration() throws Exception { - Pipeline pipeline = builder + try (Pipeline pipeline = builder .setConcurrency(4) - .build(); - ConfigIpiSwig swig = getConfigIpiSwig(pipeline); - assertEquals(4, swig.getConcurrency()); + .build()) { + ConfigIpiSwig swig = getConfigIpiSwig(pipeline); + assertEquals(4, swig.getConcurrency()); + } } @Test public void setDataUpdateUrl_PropertyPropagatedToDataFileConfiguration() throws Exception { String url = "https://update.me.com"; - Pipeline pipeline = builder + try (Pipeline pipeline = builder .setDataUpdateUrl(url) - .build(); - DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline); - assertEquals(url, dataFileConfiguration.getDataUpdateUrl()); + .build()) { + DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline); + assertEquals(url, dataFileConfiguration.getDataUpdateUrl()); + } } @Test public void setDataUpdateVerifyMd5_PropertyPropagatedToDataFileConfiguration() throws Exception { - Pipeline pipeline = builder + try (Pipeline pipeline = builder .setDataUpdateVerifyMd5(true) - .build(); - DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline); - assertTrue(dataFileConfiguration.getVerifyMd5()); + .build()) { + DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline); + assertTrue(dataFileConfiguration.getVerifyMd5()); + } } @Test public void setDataUpdateUrlFormatter_PropertyPropagatedToDataFileConfiguration() throws Exception { DataUpdateUrlFormatter urlFormatter = mock(DataUpdateUrlFormatter.class); - Pipeline pipeline = builder + try (Pipeline pipeline = builder .setDataUpdateUrlFormatter(urlFormatter) - .build(); - DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline); - assertEquals(urlFormatter, dataFileConfiguration.getUrlFormatter()); + .build()) { + DataFileConfiguration dataFileConfiguration = getDataFileConfiguration(pipeline); + assertEquals(urlFormatter, dataFileConfiguration.getUrlFormatter()); + } } } \ No newline at end of file diff --git a/location/geo-location.shell.examples/src/main/java/fiftyone/geolocation/examples/CombiningServices.java b/location/geo-location.shell.examples/src/main/java/fiftyone/geolocation/examples/CombiningServices.java index 748499f05..c3088a99f 100644 --- a/location/geo-location.shell.examples/src/main/java/fiftyone/geolocation/examples/CombiningServices.java +++ b/location/geo-location.shell.examples/src/main/java/fiftyone/geolocation/examples/CombiningServices.java @@ -74,12 +74,14 @@ * .build(Enums.GeoLocationProvider.FiftyOneDegrees); * ``` * - * 2. Build a new Pipeline containing the engines which were just created. + * 2. Build a new Pipeline containing the engines which were just created, + * and ask it to close them when it is closed. * ``` * Pipeline pipeline = new PipelineBuilder(loggerFactory) * .addFlowElement(cloudRequestEngine) * .addFlowElement(deviceDetectionEngine) * .addFlowElement(locationEngine) + * .setAutoCloseElements(true) * .build(); * ``` * @@ -92,9 +94,10 @@ * .build(); * ``` * 3. Create a new FlowData instance ready to be populated with evidence for - * the Pipeline. + * the Pipeline, in a try-with-resources block so that it is closed when it + * is finished with. * ``` - * FlowData data = pipeline.createFlowData(); + * try (FlowData data = pipeline.createFlowData()) { * ``` * * 4. Process a longitude, latitude and User-Agent to retrieve the values @@ -148,14 +151,16 @@ public static void main(String[] args) throws Exception { new GeoLocationCloudEngineBuilder(loggerFactory) .build(Enums.GeoLocationProvider.FiftyOneDegrees); - Pipeline pipeline = + // The engines are not closed anywhere else, so the pipeline is + // asked to close them when it is closed. + try (Pipeline pipeline = new PipelineBuilder(loggerFactory) .addFlowElement(cloudRequestEngine) .addFlowElement(deviceDetectionEngine) .addFlowElement(locationEngine) + .setAutoCloseElements(true) .build(); - - try (FlowData flowData = pipeline.createFlowData()) { + FlowData flowData = pipeline.createFlowData()) { flowData .addEvidence(EVIDENCE_HEADER_USERAGENT_KEY, mobileUserAgent) .addEvidence(EVIDENCE_GEO_LAT_PARAM_KEY, "51.458048") @@ -173,7 +178,6 @@ public static void main(String[] args) throws Exception { System.out.println("Country: " + country.toString()); System.out.println("IsMobile: " + isMobile.toString()); } - pipeline.close(); } } diff --git a/location/geo-location.shell.examples/src/main/java/fiftyone/geolocation/examples/cloud/GettingStarted.java b/location/geo-location.shell.examples/src/main/java/fiftyone/geolocation/examples/cloud/GettingStarted.java index 553517c6a..8c86dfe01 100644 --- a/location/geo-location.shell.examples/src/main/java/fiftyone/geolocation/examples/cloud/GettingStarted.java +++ b/location/geo-location.shell.examples/src/main/java/fiftyone/geolocation/examples/cloud/GettingStarted.java @@ -57,9 +57,10 @@ * ``` * * 2. Create a new FlowData instance ready to be populated with evidence for - * the Pipeline. + * the Pipeline, in a try-with-resources block so that it is closed when it + * is finished with. * ``` - * FlowData data = pipeline.createFlowData(); + * try (FlowData data = pipeline.createFlowData()) { * ``` * * 3. Process a longitude and latitude to retrieve the values associated with @@ -92,14 +93,13 @@ public static void main(String[] args) throws Exception { "property as it is used by this example."); } else { - Pipeline pipeline = new GeoLocationPipelineBuilder(loggerFactory) + try (Pipeline pipeline = new GeoLocationPipelineBuilder(loggerFactory) .useCloud(resourceKey, Enums.GeoLocationProvider.FiftyOneDegrees) .setEndPoint("https://cloud.51degrees.com/api/v4") .useLazyLoading(1000) .setAutoCloseElements(true) .build(); - - try (FlowData flowData = pipeline.createFlowData()) { + FlowData flowData = pipeline.createFlowData()) { flowData.addEvidence(EVIDENCE_GEO_LAT_PARAM_KEY, "51.458048") .addEvidence(EVIDENCE_GEO_LON_PARAM_KEY, "-0.9822207999999999") .process(); @@ -115,8 +115,7 @@ public static void main(String[] args) throws Exception { System.out.println("Country: " + country.toString()); System.out.println(flowData.get(GeoData.class).getRegion()); - } - pipeline.close(); + } } } diff --git a/pipeline/pipeline.cloudrequestengine/src/test/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudAspectEngineBaseTests.java b/pipeline/pipeline.cloudrequestengine/src/test/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudAspectEngineBaseTests.java index 54b2c8f96..514a9369e 100644 --- a/pipeline/pipeline.cloudrequestengine/src/test/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudAspectEngineBaseTests.java +++ b/pipeline/pipeline.cloudrequestengine/src/test/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudAspectEngineBaseTests.java @@ -181,9 +181,15 @@ public void init() { } @AfterEach - public void cleanUp() { - loggerFactory.assertMaxWarnings(maxWarnings); - loggerFactory.assertMaxErrors(maxErrors); + public void cleanUp() throws Exception { + try { + loggerFactory.assertMaxWarnings(maxWarnings); + loggerFactory.assertMaxErrors(maxErrors); + } finally { + if (pipeline != null) { + pipeline.close(); + } + } } private boolean propertiesContainName( @@ -370,9 +376,10 @@ public Object answer(InvocationOnMock invocationOnMock) throws Throwable { .addFlowElement(mockRequestEngine) .addFlowElement(mockTestInstance) .build(); - FlowData flowData = pipeline.createFlowData(); - flowData.addEvidence("query.user-agent", "iPhone"); - flowData.process(); + try (FlowData flowData = pipeline.createFlowData()) { + flowData.addEvidence("query.user-agent", "iPhone"); + flowData.process(); + } } catch (Exception e) { fail("An exception has occurred here."); @@ -422,9 +429,10 @@ public Object answer(InvocationOnMock invocationOnMock) throws Throwable { .addFlowElement(mockRequestEngine) .addFlowElement(mockTestInstance) .build(); - FlowData flowData = pipeline.createFlowData(); - flowData.addEvidence("query.user-agent", "iPhone"); - flowData.process(); + try (FlowData flowData = pipeline.createFlowData()) { + flowData.addEvidence("query.user-agent", "iPhone"); + flowData.process(); + } } catch (Exception e) { fail("An exception has occurred here."); @@ -480,9 +488,10 @@ public Object answer(InvocationOnMock invocationOnMock) throws Throwable { .addFlowElement(mockTestInstance) .addFlowElement(mockRequestEngine) .build(); - FlowData flowData = pipeline.createFlowData(); - flowData.addEvidence("query.user-agent", "iPhone"); - flowData.process(); + try (FlowData flowData = pipeline.createFlowData()) { + flowData.addEvidence("query.user-agent", "iPhone"); + flowData.process(); + } fail("Expected exception was not thrown"); } catch (Exception ex) { diff --git a/pipeline/pipeline.cloudrequestengine/src/test/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineTests.java b/pipeline/pipeline.cloudrequestengine/src/test/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineTests.java index c82645790..01b82a180 100644 --- a/pipeline/pipeline.cloudrequestengine/src/test/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineTests.java +++ b/pipeline/pipeline.cloudrequestengine/src/test/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineTests.java @@ -75,29 +75,30 @@ public void Process() throws Exception { .build(); - Pipeline pipeline = new PipelineBuilder(loggerFactory) + try (Pipeline pipeline = new PipelineBuilder(loggerFactory) .addFlowElement(engine).build(); - FlowData data = pipeline.createFlowData(); - data.addEvidence("query.User-Agent", userAgent); - data.process(); + FlowData data = pipeline.createFlowData()) { + data.addEvidence("query.User-Agent", userAgent); + data.process(); - String result = data.getFromElement(engine).getJsonResponse(); - assertEquals("{'device':{'value':'1'}}", result); + String result = data.getFromElement(engine).getJsonResponse(); + assertEquals("{'device':{'value':'1'}}", result); - JSONObject jsonObj = new JSONObject(result); - assertEquals(1, jsonObj.getJSONObject("device").getInt("value")); + JSONObject jsonObj = new JSONObject(result); + assertEquals(1, jsonObj.getJSONObject("device").getInt("value")); - verify(httpClient, times(1)) // we expected a single external POST request - .postData( - argThat(c -> { - return c.getURL().equals(expectedUrl); // to this uri - }), - ArgumentMatchers.anyMap(), - argThat(bytes -> { - String string = new String(bytes); - return string.contains("resource=" + resourceKey) && // content contains resource key - string.contains("User-Agent=" + userAgent); // content contains user agent - })); + verify(httpClient, times(1)) // we expected a single external POST request + .postData( + argThat(c -> { + return c.getURL().equals(expectedUrl); // to this uri + }), + ArgumentMatchers.anyMap(), + argThat(bytes -> { + String string = new String(bytes); + return string.contains("resource=" + resourceKey) && // content contains resource key + string.contains("User-Agent=" + userAgent); // content contains user agent + })); + } } /** @@ -113,19 +114,20 @@ public void Process_SuppressProcessExceptions_Exceptions_Added_To_Errors() throw .setResourceKey(resourceKey) .build(); - Pipeline pipeline = new PipelineBuilder(loggerFactory) + try (Pipeline pipeline = new PipelineBuilder(loggerFactory) .setSuppressProcessException(true) .addFlowElement(engine) .build(); - FlowData data = pipeline.createFlowData(); + FlowData data = pipeline.createFlowData()) { - data.addEvidence("query.User-Agent", userAgent); - data.process(); + data.addEvidence("query.User-Agent", userAgent); + data.process(); - assertFalse(data.getErrors().isEmpty()); - ArrayList errors = (ArrayList) data.getErrors(); - Throwable throwable = errors.get(0).getThrowable(); - assertInstanceOf(IOException.class, throwable); + assertFalse(data.getErrors().isEmpty()); + ArrayList errors = (ArrayList) data.getErrors(); + Throwable throwable = errors.get(0).getThrowable(); + assertInstanceOf(IOException.class, throwable); + } } /** @@ -330,11 +332,10 @@ public void evidencePrecidence(String name, boolean shouldWarn, String evidence1 .setResourceKey("resourcekey") .build(); - Pipeline pipeline = new PipelineBuilder(loggerFactory) + try (Pipeline pipeline = new PipelineBuilder(loggerFactory) .addFlowElement(engine) .build(); - - try (FlowData flowData = pipeline.createFlowData()) { + FlowData flowData = pipeline.createFlowData()) { flowData.addEvidence(evidence1Parts[0], evidence1Parts[1]); flowData.addEvidence(evidence2Parts[0], evidence2Parts[1]); @@ -450,11 +451,10 @@ public void getFormData(String name, Map evidence, String expect .setResourceKey("resourcekey") .build(); - Pipeline pipeline = new PipelineBuilder(loggerFactory) + try (Pipeline pipeline = new PipelineBuilder(loggerFactory) .addFlowElement(engine) .build(); - - try (FlowData data = pipeline.createFlowData()) { + FlowData data = pipeline.createFlowData()) { for (Map.Entry entry : evidence.entrySet()) { data.addEvidence(entry.getKey(), entry.getValue()); } @@ -479,11 +479,10 @@ public void getFormData_KeepsDotsInEvidenceName() throws Exception { .setResourceKey("resourcekey") .build(); - Pipeline pipeline = new PipelineBuilder(loggerFactory) + try (Pipeline pipeline = new PipelineBuilder(loggerFactory) .addFlowElement(engine) .build(); - - try (FlowData data = pipeline.createFlowData()) { + FlowData data = pipeline.createFlowData()) { data.addEvidence("query.id.usage", "non-marketing"); data.addEvidence("header.id.email", "someone@example.com"); diff --git a/pipeline/pipeline.core/src/test/java/fiftyone/pipeline/core/flowelements/FlowDataTests.java b/pipeline/pipeline.core/src/test/java/fiftyone/pipeline/core/flowelements/FlowDataTests.java index 3e04cf231..2dd08130b 100644 --- a/pipeline/pipeline.core/src/test/java/fiftyone/pipeline/core/flowelements/FlowDataTests.java +++ b/pipeline/pipeline.core/src/test/java/fiftyone/pipeline/core/flowelements/FlowDataTests.java @@ -320,12 +320,13 @@ public void FlowData_GetNotPresent_Element() { @Test(expected = IllegalStateException.class) public void FlowData_PipelineDisposed() { when(pipeline.isClosed()).thenReturn(true); - FlowData flowData = new FlowDataDefault( + try (FlowData flowData = new FlowDataDefault( mock(Logger.class), pipeline, - new EvidenceDefault(mock(Logger.class))); + new EvidenceDefault(mock(Logger.class)))) { - flowData.process(); + flowData.process(); + } } @Test diff --git a/pipeline/pipeline.core/src/test/java/fiftyone/pipeline/core/flowelements/ParallelElementsTests.java b/pipeline/pipeline.core/src/test/java/fiftyone/pipeline/core/flowelements/ParallelElementsTests.java index 8ddaa0435..617089b24 100644 --- a/pipeline/pipeline.core/src/test/java/fiftyone/pipeline/core/flowelements/ParallelElementsTests.java +++ b/pipeline/pipeline.core/src/test/java/fiftyone/pipeline/core/flowelements/ParallelElementsTests.java @@ -126,35 +126,36 @@ public TestElementData create(FlowData flowData) { Arrays.asList(element1, element2, element3)); - FlowData data = new FlowDataDefault( + try (FlowData data = new FlowDataDefault( mock(Logger.class), pipeline, - new EvidenceDefault(mock(Logger.class))); - data.process(); - - // Act - parallelElements.process(data); - - List startTimes = new ArrayList<>(); - startTimes.add((Date) data.get("element1").get("start")); - startTimes.add((Date) data.get("element2").get("start")); - startTimes.add((Date) data.get("element3").get("start")); - List endTimes = new ArrayList<>(); - endTimes.add((Date) data.get("element1").get("end")); - endTimes.add((Date) data.get("element2").get("end")); - endTimes.add((Date) data.get("element3").get("end")); - - // Assert - assertTrue("Expected no errors", - data.getErrors() == null || data.getErrors().size() == 0); - for (Date start : startTimes) { - for (Date end : endTimes) { - assertTrue( - "The elements were not processed sequentially. Times were: " + - startTimes.get(0) + " => " + endTimes.get(0) + "\n" + - startTimes.get(1) + " => " + endTimes.get(1) + "\n" + - startTimes.get(2) + " => " + endTimes.get(2), - start.before(end)); + new EvidenceDefault(mock(Logger.class)))) { + data.process(); + + // Act + parallelElements.process(data); + + List startTimes = new ArrayList<>(); + startTimes.add((Date) data.get("element1").get("start")); + startTimes.add((Date) data.get("element2").get("start")); + startTimes.add((Date) data.get("element3").get("start")); + List endTimes = new ArrayList<>(); + endTimes.add((Date) data.get("element1").get("end")); + endTimes.add((Date) data.get("element2").get("end")); + endTimes.add((Date) data.get("element3").get("end")); + + // Assert + assertTrue("Expected no errors", + data.getErrors() == null || data.getErrors().size() == 0); + for (Date start : startTimes) { + for (Date end : endTimes) { + assertTrue( + "The elements were not processed sequentially. Times were: " + + startTimes.get(0) + " => " + endTimes.get(0) + "\n" + + startTimes.get(1) + " => " + endTimes.get(1) + "\n" + + startTimes.get(2) + " => " + endTimes.get(2), + start.before(end)); + } } } } diff --git a/pipeline/pipeline.core/src/test/java/fiftyone/pipeline/core/flowelements/PipelineBuilderTests.java b/pipeline/pipeline.core/src/test/java/fiftyone/pipeline/core/flowelements/PipelineBuilderTests.java index c28bf4438..3bfb105ef 100644 --- a/pipeline/pipeline.core/src/test/java/fiftyone/pipeline/core/flowelements/PipelineBuilderTests.java +++ b/pipeline/pipeline.core/src/test/java/fiftyone/pipeline/core/flowelements/PipelineBuilderTests.java @@ -416,26 +416,27 @@ public void PipelineBuilder_BuildFromConfiguration_ParallelElements() throws Exc options.elements.add(elOpts); // Pass the configuration to the builder to create the pipeline. - Pipeline pipeline = builder.buildFromConfiguration(options); - // Get the elements - ListSplitterElement splitterElement = pipeline.getElement(ListSplitterElement.class); - MultiplyByElement multiplyByElement = pipeline.getElement(MultiplyByElement.class); - - // Create, populate and process flow data. - try (FlowData flowData = pipeline.createFlowData()) { - flowData - .addEvidence(splitterElement.evidenceKeys.get(0), "1,2,abc") - .addEvidence(multiplyByElement.evidenceKeys.get(0), 25) - .process(); + try (Pipeline pipeline = builder.buildFromConfiguration(options)) { + // Get the elements + ListSplitterElement splitterElement = pipeline.getElement(ListSplitterElement.class); + MultiplyByElement multiplyByElement = pipeline.getElement(MultiplyByElement.class); + + // Create, populate and process flow data. + try (FlowData flowData = pipeline.createFlowData()) { + flowData + .addEvidence(splitterElement.evidenceKeys.get(0), "1,2,abc") + .addEvidence(multiplyByElement.evidenceKeys.get(0), 25) + .process(); - // Get the results and verify them. - ListSplitterElementData splitterData = flowData.getFromElement(splitterElement); - TestElementData multiplyByData = flowData.getFromElement(multiplyByElement); + // Get the results and verify them. + ListSplitterElementData splitterData = flowData.getFromElement(splitterElement); + TestElementData multiplyByData = flowData.getFromElement(multiplyByElement); - assertEquals("1", splitterData.getResult().get(0)); - assertEquals("2", splitterData.getResult().get(1)); - assertEquals("abc", splitterData.getResult().get(2)); - assertEquals(75, multiplyByData.getResult()); + assertEquals("1", splitterData.getResult().get(0)); + assertEquals("2", splitterData.getResult().get(1)); + assertEquals("abc", splitterData.getResult().get(2)); + assertEquals(75, multiplyByData.getResult()); + } } } @@ -443,58 +444,60 @@ private void VerifyListSplitterElementPipeline( PipelineOptions options, SplitOption splitOn) throws Exception { // Pass the configuration to the builder to create the pipeline. - Pipeline pipeline = builder.buildFromConfiguration(options); + try (Pipeline pipeline = builder.buildFromConfiguration(options)) { - ListSplitterElement element = pipeline.getElement(ListSplitterElement.class); - // Check we've got the expected number of evidence keys. - assertEquals(1, element.evidenceKeys.size()); + ListSplitterElement element = pipeline.getElement(ListSplitterElement.class); + // Check we've got the expected number of evidence keys. + assertEquals(1, element.evidenceKeys.size()); - // Create, populate and process flow data. - try (FlowData flowData = pipeline.createFlowData()) { - flowData.addEvidence(element.evidenceKeys.get(0), "123,456|789,0") - .process(); + // Create, populate and process flow data. + try (FlowData flowData = pipeline.createFlowData()) { + flowData.addEvidence(element.evidenceKeys.get(0), "123,456|789,0") + .process(); - // Get the result and verify it. - ListSplitterElementData elementData = flowData.getFromElement(element); - switch (splitOn) { - case Comma: - assertEquals("123", elementData.getResult().get(0)); - assertEquals("456|789", elementData.getResult().get(1)); - assertEquals("0", elementData.getResult().get(2)); - break; - case Pipe: - assertEquals("123,456", elementData.getResult().get(0)); - assertEquals("789,0", elementData.getResult().get(1)); - break; - case CommaMaxLengthThree: - assertEquals("123", elementData.getResult().get(0)); - assertEquals("456", elementData.getResult().get(1)); - assertEquals("|78", elementData.getResult().get(2)); - assertEquals("9", elementData.getResult().get(3)); - assertEquals("0", elementData.getResult().get(4)); - break; - default: - break; + // Get the result and verify it. + ListSplitterElementData elementData = flowData.getFromElement(element); + switch (splitOn) { + case Comma: + assertEquals("123", elementData.getResult().get(0)); + assertEquals("456|789", elementData.getResult().get(1)); + assertEquals("0", elementData.getResult().get(2)); + break; + case Pipe: + assertEquals("123,456", elementData.getResult().get(0)); + assertEquals("789,0", elementData.getResult().get(1)); + break; + case CommaMaxLengthThree: + assertEquals("123", elementData.getResult().get(0)); + assertEquals("456", elementData.getResult().get(1)); + assertEquals("|78", elementData.getResult().get(2)); + assertEquals("9", elementData.getResult().get(3)); + assertEquals("0", elementData.getResult().get(4)); + break; + default: + break; + } } } } private void VerifyMultiplyByElementPipeline(PipelineOptions options) throws Exception { // Pass the configuration to the builder to create the pipeline. - Pipeline pipeline = builder.buildFromConfiguration(options); - MultiplyByElement element = pipeline.getElement(MultiplyByElement.class); + try (Pipeline pipeline = builder.buildFromConfiguration(options)) { + MultiplyByElement element = pipeline.getElement(MultiplyByElement.class); - // Check we've got the expected number of evidence keys. - assertEquals(1, element.evidenceKeys.size()); + // Check we've got the expected number of evidence keys. + assertEquals(1, element.evidenceKeys.size()); - // Create, populate and process flow data. - try (FlowData flowData = pipeline.createFlowData()) { - flowData.addEvidence(element.evidenceKeys.get(0), 5) - .process(); + // Create, populate and process flow data. + try (FlowData flowData = pipeline.createFlowData()) { + flowData.addEvidence(element.evidenceKeys.get(0), 5) + .process(); - // Get the result and verify it. - TestElementData elementData = flowData.getFromElement(element); - assertEquals(40, elementData.getResult()); + // Get the result and verify it. + TestElementData elementData = flowData.getFromElement(element); + assertEquals(40, elementData.getResult()); + } } } diff --git a/pipeline/pipeline.core/src/test/java/fiftyone/pipeline/core/flowelements/PipelineTests.java b/pipeline/pipeline.core/src/test/java/fiftyone/pipeline/core/flowelements/PipelineTests.java index c688da1ee..e7d0f1d7b 100644 --- a/pipeline/pipeline.core/src/test/java/fiftyone/pipeline/core/flowelements/PipelineTests.java +++ b/pipeline/pipeline.core/src/test/java/fiftyone/pipeline/core/flowelements/PipelineTests.java @@ -102,30 +102,31 @@ public TestElementData create(FlowData flowData) { new FlowElement[]{element1, element2}); // Don't create the flow data via the pipeline as we just want // to test Process. - FlowData data = new FlowDataDefault( + try (FlowData data = new FlowDataDefault( mock(Logger.class), pipeline, - new EvidenceDefault(mock(Logger.class))); - - // Act - data.process(); - - // Assert - assertTrue("Expected no errors", - data.getErrors() == null || data.getErrors().size() == 0); - // Check that the resulting data has the expected values - assertTrue("data from element 1 is missing in the result", - data.getDataKeys().contains("element1")); - assertTrue("data from element 2 is missing in the result", data.getDataKeys().contains("element2")); - assertEquals("done", data.get("element1").get("key").toString()); - assertEquals("done", data.get("element2").get("key").toString()); - // Check that element 1 was called before element 2. - verify(element2, times(1)).process(argThat(new ArgumentMatcher() { - @Override - public boolean matches(FlowData flowData) { - return flowData.getDataKeys().contains("element1"); - } - })); + new EvidenceDefault(mock(Logger.class)))) { + + // Act + data.process(); + + // Assert + assertTrue("Expected no errors", + data.getErrors() == null || data.getErrors().size() == 0); + // Check that the resulting data has the expected values + assertTrue("data from element 1 is missing in the result", + data.getDataKeys().contains("element1")); + assertTrue("data from element 2 is missing in the result", data.getDataKeys().contains("element2")); + assertEquals("done", data.get("element1").get("key").toString()); + assertEquals("done", data.get("element2").get("key").toString()); + // Check that element 1 was called before element 2. + verify(element2, times(1)).process(argThat(new ArgumentMatcher() { + @Override + public boolean matches(FlowData flowData) { + return flowData.getDataKeys().contains("element1"); + } + })); + } } @Test diff --git a/pipeline/pipeline.developer-examples/pipeline.developer-examples.cloud-engine/src/main/java/pipeline/developerexamples/cloudengine/Main.java b/pipeline/pipeline.developer-examples/pipeline.developer-examples.cloud-engine/src/main/java/pipeline/developerexamples/cloudengine/Main.java index 819bcee46..5b7c1b325 100644 --- a/pipeline/pipeline.developer-examples/pipeline.developer-examples.cloud-engine/src/main/java/pipeline/developerexamples/cloudengine/Main.java +++ b/pipeline/pipeline.developer-examples/pipeline.developer-examples.cloud-engine/src/main/java/pipeline/developerexamples/cloudengine/Main.java @@ -105,33 +105,36 @@ public void run() throws Exception { loggerFactory) .build(); - Pipeline pipeline = new PipelineBuilder(loggerFactory) + // The pipeline closes the engines when it is closed. + try (Pipeline pipeline = new PipelineBuilder(loggerFactory) .addFlowElement(cloudRequestEngine) .addFlowElement(ageElement) - .build(); + .setAutoCloseElements(true) + .build()) { - String dob = "18/12/1992"; + String dob = "18/12/1992"; - try (FlowData flowData = pipeline.createFlowData()) { - flowData - .addEvidence("cookie.date-of-birth", dob) - .process(); - - String starSign = - flowData.getFromElement(ageElement).getStarSign(); - if (starSign == null) { - // The service answered but offers no star sign - // product, which is what the 51Degrees cloud says, - // because star signs are only served by the example - // service. The connection, the property and evidence - // key negotiation and the request itself all worked. - System.out.println("The cloud service at " + endpoint() - + " does not offer the star sign product, so no " - + "star sign is available for a date of birth of " - + dob + "."); - } else { - System.out.println("With a date of birth of " + dob - + ", your star sign is " + starSign + "."); + try (FlowData flowData = pipeline.createFlowData()) { + flowData + .addEvidence("cookie.date-of-birth", dob) + .process(); + + String starSign = + flowData.getFromElement(ageElement).getStarSign(); + if (starSign == null) { + // The service answered but offers no star sign + // product, which is what the 51Degrees cloud says, + // because star signs are only served by the example + // service. The connection, the property and evidence + // key negotiation and the request itself all worked. + System.out.println("The cloud service at " + endpoint() + + " does not offer the star sign product, so no " + + "star sign is available for a date of birth of " + + dob + "."); + } else { + System.out.println("With a date of birth of " + dob + + ", your star sign is " + starSign + "."); + } } } //! [usage] diff --git a/pipeline/pipeline.developer-examples/pipeline.developer-examples.flowelement/src/main/java/pipeline/developerexamples/flowelement/Main.java b/pipeline/pipeline.developer-examples/pipeline.developer-examples.flowelement/src/main/java/pipeline/developerexamples/flowelement/Main.java index 8406ac0aa..76d7e4473 100644 --- a/pipeline/pipeline.developer-examples/pipeline.developer-examples.flowelement/src/main/java/pipeline/developerexamples/flowelement/Main.java +++ b/pipeline/pipeline.developer-examples/pipeline.developer-examples.flowelement/src/main/java/pipeline/developerexamples/flowelement/Main.java @@ -45,22 +45,25 @@ public void run() throws Exception { new SimpleFlowElementBuilder(loggerFactory) .build(); - Pipeline pipeline = new PipelineBuilder(loggerFactory) + // The pipeline closes the element when it is closed. + try (Pipeline pipeline = new PipelineBuilder(loggerFactory) .addFlowElement(starSignElement) - .build(); - Calendar dob = Calendar.getInstance(); - dob.set(1992, Calendar.DECEMBER, 18); + .setAutoCloseElements(true) + .build()) { + Calendar dob = Calendar.getInstance(); + dob.set(1992, Calendar.DECEMBER, 18); - try (FlowData flowData = pipeline.createFlowData()) { - flowData - .addEvidence("date-of-birth", dob.getTime()) - .process(); - - System.out.println("With a date of birth of " + - new SimpleDateFormat("yyyy/MM/dd").format(dob.getTime()) + - ", your star sign is " + - flowData.getFromElement(starSignElement).getStarSign() + - "."); + try (FlowData flowData = pipeline.createFlowData()) { + flowData + .addEvidence("date-of-birth", dob.getTime()) + .process(); + + System.out.println("With a date of birth of " + + new SimpleDateFormat("yyyy/MM/dd").format(dob.getTime()) + + ", your star sign is " + + flowData.getFromElement(starSignElement).getStarSign() + + "."); + } } //! [usage] } diff --git a/pipeline/pipeline.developer-examples/pipeline.developer-examples.onpremise-engine/src/main/java/pipeline/developerexamples/onpremiseengine/Main.java b/pipeline/pipeline.developer-examples/pipeline.developer-examples.onpremise-engine/src/main/java/pipeline/developerexamples/onpremiseengine/Main.java index 594cdbd5b..8d803d244 100644 --- a/pipeline/pipeline.developer-examples/pipeline.developer-examples.onpremise-engine/src/main/java/pipeline/developerexamples/onpremiseengine/Main.java +++ b/pipeline/pipeline.developer-examples/pipeline.developer-examples.onpremise-engine/src/main/java/pipeline/developerexamples/onpremiseengine/Main.java @@ -47,22 +47,26 @@ public void run() throws Exception { Main.class.getClassLoader().getResource("starsigns.csv").getPath(), false); - Pipeline pipeline = new PipelineBuilder(loggerFactory) + // The pipeline closes the engine, and with it the data file, + // when it is closed. + try (Pipeline pipeline = new PipelineBuilder(loggerFactory) .addFlowElement(starSignEngine) - .build(); - Calendar dob = Calendar.getInstance(); - dob.set(1992, Calendar.DECEMBER, 18); + .setAutoCloseElements(true) + .build()) { + Calendar dob = Calendar.getInstance(); + dob.set(1992, Calendar.DECEMBER, 18); - try (FlowData flowData = pipeline.createFlowData()) { - flowData - .addEvidence("date-of-birth", dob.getTime()) - .process(); - - System.out.println("With a date of birth of " + - new SimpleDateFormat("yyyy/MM/dd").format(dob.getTime()) + - ", your star sign is " + - flowData.getFromElement(starSignEngine).getStarSign() + - "."); + try (FlowData flowData = pipeline.createFlowData()) { + flowData + .addEvidence("date-of-birth", dob.getTime()) + .process(); + + System.out.println("With a date of birth of " + + new SimpleDateFormat("yyyy/MM/dd").format(dob.getTime()) + + ", your star sign is " + + flowData.getFromElement(starSignEngine).getStarSign() + + "."); + } } //! [usage] } diff --git a/pipeline/pipeline.developer-examples/pipeline.developer-examples.usage-sharing/src/main/java/pipeline/developerexamples/usagesharing/Main.java b/pipeline/pipeline.developer-examples/pipeline.developer-examples.usage-sharing/src/main/java/pipeline/developerexamples/usagesharing/Main.java index 407759b4d..f6da01434 100644 --- a/pipeline/pipeline.developer-examples/pipeline.developer-examples.usage-sharing/src/main/java/pipeline/developerexamples/usagesharing/Main.java +++ b/pipeline/pipeline.developer-examples/pipeline.developer-examples.usage-sharing/src/main/java/pipeline/developerexamples/usagesharing/Main.java @@ -88,26 +88,30 @@ public void run() throws Exception { new File(Objects.requireNonNull(getClass().getClassLoader(). getResource("51d.xml")).getFile())); - // Build a new Pipeline from the configuration. - Pipeline pipeline = new PipelineBuilder() + // Build a new Pipeline from the configuration. Closing the + // pipeline closes the ShareUsageElement it created, which sends + // any usage data still queued. + try (Pipeline pipeline = new PipelineBuilder() .addService(http) - .buildFromConfiguration(options); - - // Alternatively, the commented code below shows how to - // configure the ShareUsageElement in code, rather than - // using a configuration file. - //ShareUsageElement usageElement = new ShareUsageBuilder(loggerFactory, http) - // .setSharePercentage(0.01) - // .setMinimumEntriesPerMessage(2500) - // .build(); - //Pipeline pipeline = new PipelineBuilder() - // .addFlowElement(usageElement) - // .build(); + .buildFromConfiguration(options)) { - System.out.println( - "Pipeline created with share usage element. Evidence processed " + - "with this pipeline will now be periodically shared with " + - "51Degrees using the specified configuration."); + // Alternatively, the commented code below shows how to + // configure the ShareUsageElement in code, rather than + // using a configuration file. + //ShareUsageElement usageElement = new ShareUsageBuilder(loggerFactory, http) + // .setSharePercentage(0.01) + // .setMinimumEntriesPerMessage(2500) + // .build(); + //Pipeline pipeline = new PipelineBuilder() + // .addFlowElement(usageElement) + // .setAutoCloseElements(true) + // .build(); + + System.out.println( + "Pipeline created with share usage element. Evidence processed " + + "with this pipeline will now be periodically shared with " + + "51Degrees using the specified configuration."); + } } } diff --git a/pipeline/pipeline.engines.fiftyone/src/test/java/fiftyone/pipeline/engines/fiftyone/flowelements/SetHeadersElementTests.java b/pipeline/pipeline.engines.fiftyone/src/test/java/fiftyone/pipeline/engines/fiftyone/flowelements/SetHeadersElementTests.java index 66a433a25..cd25d63ac 100644 --- a/pipeline/pipeline.engines.fiftyone/src/test/java/fiftyone/pipeline/engines/fiftyone/flowelements/SetHeadersElementTests.java +++ b/pipeline/pipeline.engines.fiftyone/src/test/java/fiftyone/pipeline/engines/fiftyone/flowelements/SetHeadersElementTests.java @@ -33,6 +33,7 @@ import fiftyone.pipeline.engines.data.AspectPropertyValueDefault; import fiftyone.pipeline.engines.fiftyone.data.SetHeadersData; import fiftyone.pipeline.exceptions.AggregateException; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; @@ -66,6 +67,13 @@ public SetHeadersElementTests() { when(internalLogger.getLogger(anyString())).thenReturn(mock(Logger.class)); loggerFactory = new TestLoggerFactory(internalLogger); } + + @AfterEach + public void cleanUp() throws Exception { + if (pipeline != null) { + pipeline.close(); + } + } /** * Check that SetHeaderElement handles both 'String' and @@ -84,14 +92,15 @@ public void setHeadersElement(boolean valueIsAPV) throws Exception { new HashMap(); propertyNameValues.put("SetHeaderBrowserAccept-CH", value); createPipeline(propertyNameValues); - FlowData data = pipeline.createFlowData(); - data.process(); + try (FlowData data = pipeline.createFlowData()) { + data.process(); - // Verify the output - SetHeadersData typedOutput = getFromFlowData(data); - assertEquals(1, typedOutput.getResponseHeaderDictionary().size()); - assertTrue(typedOutput.getResponseHeaderDictionary().containsKey("Accept-CH")); - assertEquals("UA-Platform", typedOutput.getResponseHeaderDictionary().get("Accept-CH")); + // Verify the output + SetHeadersData typedOutput = getFromFlowData(data); + assertEquals(1, typedOutput.getResponseHeaderDictionary().size()); + assertTrue(typedOutput.getResponseHeaderDictionary().containsKey("Accept-CH")); + assertEquals("UA-Platform", typedOutput.getResponseHeaderDictionary().get("Accept-CH")); + } } /** @@ -104,12 +113,13 @@ public void setHeadersElement_InvalidPropertyValues(Object sourcePropertyValue) Map propertyNameValues = new HashMap(); propertyNameValues.put("SetHeaderBrowserAccept-CH", sourcePropertyValue); createPipeline(propertyNameValues); - FlowData data = pipeline.createFlowData(); - data.process(); + try (FlowData data = pipeline.createFlowData()) { + data.process(); - // Verify the output - SetHeadersData typedOutput = getFromFlowData(data); - assertEquals(0, typedOutput.getResponseHeaderDictionary().size()); + // Verify the output + SetHeadersData typedOutput = getFromFlowData(data); + assertEquals(0, typedOutput.getResponseHeaderDictionary().size()); + } } /** @@ -139,12 +149,13 @@ public void setHeadersElement_APV_InvalidPropertyValues( Map propertyNameValues = new HashMap(); propertyNameValues.put("SetHeaderBrowserAccept-CH", value); createPipeline(propertyNameValues); - FlowData data = pipeline.createFlowData(); - data.process(); + try (FlowData data = pipeline.createFlowData()) { + data.process(); - // Verify the output - SetHeadersData typedOutput = getFromFlowData(data); - assertEquals(0, typedOutput.getResponseHeaderDictionary().size()); + // Verify the output + SetHeadersData typedOutput = getFromFlowData(data); + assertEquals(0, typedOutput.getResponseHeaderDictionary().size()); + } } /** @@ -167,10 +178,11 @@ public void setHeadersElement_InvalidPropertyNames( Map propertyNameValues = new HashMap(); propertyNameValues.put(sourcePropertyName, "TEST"); createPipeline(propertyNameValues); - FlowData data = pipeline.createFlowData(); - assertThrows(AggregateException.class, () -> { - data.process(); - }); + try (FlowData data = pipeline.createFlowData()) { + assertThrows(AggregateException.class, () -> { + data.process(); + }); + } } /** @@ -184,17 +196,18 @@ public void setHeadersElement_MultipleProperties() throws Exception { propertyNameValues.put("SetHeaderHardwareCritical-CH", "Sec-CH-UA-Model,Sec-CH-UA-Mobile"); createPipeline(propertyNameValues); - FlowData data = pipeline.createFlowData(); - data.process(); + try (FlowData data = pipeline.createFlowData()) { + data.process(); - // Verify the output - SetHeadersData typedOutput = getFromFlowData(data); - assertEquals(2, typedOutput.getResponseHeaderDictionary().size()); - assertTrue(typedOutput.getResponseHeaderDictionary().containsKey("Accept-CH")); - assertTrue(typedOutput.getResponseHeaderDictionary().containsKey("Critical-CH")); - assertEquals("Sec-CH-UA", typedOutput.getResponseHeaderDictionary().get("Accept-CH")); - assertEquals("Sec-CH-UA-Model,Sec-CH-UA-Mobile", - typedOutput.getResponseHeaderDictionary().get("Critical-CH")); + // Verify the output + SetHeadersData typedOutput = getFromFlowData(data); + assertEquals(2, typedOutput.getResponseHeaderDictionary().size()); + assertTrue(typedOutput.getResponseHeaderDictionary().containsKey("Accept-CH")); + assertTrue(typedOutput.getResponseHeaderDictionary().containsKey("Critical-CH")); + assertEquals("Sec-CH-UA", typedOutput.getResponseHeaderDictionary().get("Accept-CH")); + assertEquals("Sec-CH-UA-Model,Sec-CH-UA-Mobile", + typedOutput.getResponseHeaderDictionary().get("Critical-CH")); + } } /** @@ -207,17 +220,18 @@ public void setHeadersElement_MultipleProperties_SameHeader() throws Exception { propertyNameValues.put("SetHeaderBrowserAccept-CH", "Sec-CH-UA"); propertyNameValues.put("SetHeaderHardwareAccept-CH", "Sec-CH-UA-Model,Sec-CH-UA-Mobile"); createPipeline(propertyNameValues); - FlowData data = pipeline.createFlowData(); - data.process(); + try (FlowData data = pipeline.createFlowData()) { + data.process(); - // Verify the output - SetHeadersData typedOutput = getFromFlowData(data); - assertEquals(1, typedOutput.getResponseHeaderDictionary().size()); - assertTrue(typedOutput.getResponseHeaderDictionary().containsKey("Accept-CH")); - String acceptCHValue = typedOutput.getResponseHeaderDictionary().get("Accept-CH"); - assertTrue(acceptCHValue.contains("Sec-CH-UA")); - assertTrue(acceptCHValue.contains("Sec-CH-UA-Model,Sec-CH-UA-Mobile")); - assertTrue(acceptCHValue.matches("^Sec-CH-[a-zA-Z-]+,Sec-CH-[a-zA-Z-]+,Sec-CH-[a-zA-Z-]+$")); + // Verify the output + SetHeadersData typedOutput = getFromFlowData(data); + assertEquals(1, typedOutput.getResponseHeaderDictionary().size()); + assertTrue(typedOutput.getResponseHeaderDictionary().containsKey("Accept-CH")); + String acceptCHValue = typedOutput.getResponseHeaderDictionary().get("Accept-CH"); + assertTrue(acceptCHValue.contains("Sec-CH-UA")); + assertTrue(acceptCHValue.contains("Sec-CH-UA-Model,Sec-CH-UA-Mobile")); + assertTrue(acceptCHValue.matches("^Sec-CH-[a-zA-Z-]+,Sec-CH-[a-zA-Z-]+,Sec-CH-[a-zA-Z-]+$")); + } } private void createPipeline( diff --git a/pipeline/pipeline.engines.fiftyone/src/test/java/fiftyone/pipeline/engines/fiftyone/flowelements/ShareUsageElementTests.java b/pipeline/pipeline.engines.fiftyone/src/test/java/fiftyone/pipeline/engines/fiftyone/flowelements/ShareUsageElementTests.java index cf699e55b..176a2a156 100644 --- a/pipeline/pipeline.engines.fiftyone/src/test/java/fiftyone/pipeline/engines/fiftyone/flowelements/ShareUsageElementTests.java +++ b/pipeline/pipeline.engines.fiftyone/src/test/java/fiftyone/pipeline/engines/fiftyone/flowelements/ShareUsageElementTests.java @@ -32,6 +32,7 @@ import fiftyone.pipeline.engines.services.HttpClient; import fiftyone.pipeline.engines.trackers.Tracker; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Disabled; @@ -185,6 +186,18 @@ public void Init() throws Exception { tracker = new TestTracker(); } + /** + * The element runs its own sending thread, which closing it stops. + * Closing an element a test has already closed does nothing more. + */ + @AfterEach + public void cleanUp() throws Exception { + if (shareUsageElement != null) { + shareUsageElement.close(); + } + pipeline.close(); + } + private void createShareUsage( double sharePercentage, int minimumEntriesPerMessage, @@ -250,40 +263,41 @@ public void ShareUsageElement_SingleEvent_ClientIPAndHeader() throws Exception { evidenceData.put(EVIDENCE_COOKIE_PREFIX + EVIDENCE_SEPERATOR + FIFTYONE_COOKIE_PREFIX + "Profile", "123456"); evidenceData.put(EVIDENCE_COOKIE_PREFIX + EVIDENCE_SEPERATOR + "RemoveMe", "123456"); - FlowData data = pipeline.createFlowData(); - data.addEvidence(evidenceData); - // Act - shareUsageElement.process(data); - // Wait for the consumer task to finish. - assertNotNull(shareUsageElement.getSendDataFuture()); - shareUsageElement.getSendDataFuture().get(); + try (FlowData data = pipeline.createFlowData()) { + data.addEvidence(evidenceData); + // Act + shareUsageElement.process(data); + // Wait for the consumer task to finish. + assertNotNull(shareUsageElement.getSendDataFuture()); + shareUsageElement.getSendDataFuture().get(); - ByteArrayOutputStream baos = connector.getBaos(); - byte[] output = baos.toByteArray(); + ByteArrayOutputStream baos = connector.getBaos(); + byte[] output = baos.toByteArray(); - DocumentBuilder builder = docBuilderFactory.newDocumentBuilder(); - Document doc = builder.parse(new ByteArrayInputStream(output)); + DocumentBuilder builder = docBuilderFactory.newDocumentBuilder(); + Document doc = builder.parse(new ByteArrayInputStream(output)); - NodeList devices = (NodeList) xpath.compile("//Devices/Device").evaluate(doc, XPathConstants.NODESET); - assertEquals(1, devices.getLength(), "One device expected"); + NodeList devices = (NodeList) xpath.compile("//Devices/Device").evaluate(doc, XPathConstants.NODESET); + assertEquals(1, devices.getLength(), "One device expected"); - NodeList headers = (NodeList) xpath.compile(EVIDENCE_HTTPHEADER_PREFIX).evaluate(devices.item(0), XPathConstants.NODESET); - assertEquals(2, headers.getLength(), "2 headers expected"); - NodeList header1 = (NodeList) xpath.compile(EVIDENCE_HTTPHEADER_PREFIX+"[@Name='x-forwarded-for']").evaluate(devices.item(0), XPathConstants.NODESET); - assertEquals("5.6.7.8",header1.item(0).getTextContent()); - NodeList header2 = (NodeList) xpath.compile(EVIDENCE_HTTPHEADER_PREFIX+"[@Name='forwarded-for']").evaluate(devices.item(0), XPathConstants.NODESET); - assertEquals("2001::",header2.item(0).getTextContent()); + NodeList headers = (NodeList) xpath.compile(EVIDENCE_HTTPHEADER_PREFIX).evaluate(devices.item(0), XPathConstants.NODESET); + assertEquals(2, headers.getLength(), "2 headers expected"); + NodeList header1 = (NodeList) xpath.compile(EVIDENCE_HTTPHEADER_PREFIX+"[@Name='x-forwarded-for']").evaluate(devices.item(0), XPathConstants.NODESET); + assertEquals("5.6.7.8",header1.item(0).getTextContent()); + NodeList header2 = (NodeList) xpath.compile(EVIDENCE_HTTPHEADER_PREFIX+"[@Name='forwarded-for']").evaluate(devices.item(0), XPathConstants.NODESET); + assertEquals("2001::",header2.item(0).getTextContent()); - NodeList cookies = (NodeList) xpath.compile(EVIDENCE_COOKIE_PREFIX).evaluate(devices.item(0), XPathConstants.NODESET); - assertEquals(1, cookies.getLength(), "1 cookie expected"); - assertEquals("51d_Profile", cookies.item(0).getAttributes().getNamedItem("Name").getTextContent()); - assertEquals("123456", cookies.item(0).getFirstChild().getTextContent()); + NodeList cookies = (NodeList) xpath.compile(EVIDENCE_COOKIE_PREFIX).evaluate(devices.item(0), XPathConstants.NODESET); + assertEquals(1, cookies.getLength(), "1 cookie expected"); + assertEquals("51d_Profile", cookies.item(0).getAttributes().getNamedItem("Name").getTextContent()); + assertEquals("123456", cookies.item(0).getFirstChild().getTextContent()); - NodeList clientIp = (NodeList) xpath.compile("ClientIP").evaluate(devices.item(0), XPathConstants.NODESET); - assertEquals(1, clientIp.getLength(), "1 client IP expected"); - assertEquals("1.2.3.4", clientIp.item(0).getTextContent()); + NodeList clientIp = (NodeList) xpath.compile("ClientIP").evaluate(devices.item(0), XPathConstants.NODESET); + assertEquals(1, clientIp.getLength(), "1 client IP expected"); + assertEquals("1.2.3.4", clientIp.item(0).getTextContent()); + } } /** @@ -321,43 +335,44 @@ public void ShareUsageElement_SingleEvent_ClientIPAndHeader_HttpClient() throws evidenceData.put(EVIDENCE_COOKIE_PREFIX + EVIDENCE_SEPERATOR + FIFTYONE_COOKIE_PREFIX + "Profile", "123456"); evidenceData.put(EVIDENCE_COOKIE_PREFIX + EVIDENCE_SEPERATOR + "RemoveMe", "123456"); - FlowData data = pipeline.createFlowData(); - data.addEvidence(evidenceData); - // Act - shareUsageElement.process(data); - // Wait for the consumer task to finish. - assertNotNull(shareUsageElement.getSendDataFuture()); - shareUsageElement.getSendDataFuture().get(); + try (FlowData data = pipeline.createFlowData()) { + data.addEvidence(evidenceData); + // Act + shareUsageElement.process(data); + // Wait for the consumer task to finish. + assertNotNull(shareUsageElement.getSendDataFuture()); + shareUsageElement.getSendDataFuture().get(); - ByteArrayOutputStream baos = new ByteArrayOutputStream(); - byte[] zip = ((TestHttpClient)shareUsageElement.httpClient).getBuffer(); - // unzip the buffer - copy(new GZIPInputStream(new ByteArrayInputStream(zip)), baos); - byte[] output = baos.toByteArray(); + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + byte[] zip = ((TestHttpClient)shareUsageElement.httpClient).getBuffer(); + // unzip the buffer + copy(new GZIPInputStream(new ByteArrayInputStream(zip)), baos); + byte[] output = baos.toByteArray(); - DocumentBuilder builder = docBuilderFactory.newDocumentBuilder(); - Document doc = builder.parse(new ByteArrayInputStream(output)); + DocumentBuilder builder = docBuilderFactory.newDocumentBuilder(); + Document doc = builder.parse(new ByteArrayInputStream(output)); - NodeList devices = (NodeList) xpath.compile("//Devices/Device").evaluate(doc, XPathConstants.NODESET); - assertEquals(1, devices.getLength(), "One device expected"); + NodeList devices = (NodeList) xpath.compile("//Devices/Device").evaluate(doc, XPathConstants.NODESET); + assertEquals(1, devices.getLength(), "One device expected"); - NodeList headers = (NodeList) xpath.compile(EVIDENCE_HTTPHEADER_PREFIX).evaluate(devices.item(0), XPathConstants.NODESET); - assertEquals(2, headers.getLength(), "2 headers expected"); - NodeList header1 = (NodeList) xpath.compile(EVIDENCE_HTTPHEADER_PREFIX+"[@Name='x-forwarded-for']").evaluate(devices.item(0), XPathConstants.NODESET); - assertEquals("5.6.7.8",header1.item(0).getTextContent()); - NodeList header2 = (NodeList) xpath.compile(EVIDENCE_HTTPHEADER_PREFIX+"[@Name='forwarded-for']").evaluate(devices.item(0), XPathConstants.NODESET); - assertEquals("2001::",header2.item(0).getTextContent()); + NodeList headers = (NodeList) xpath.compile(EVIDENCE_HTTPHEADER_PREFIX).evaluate(devices.item(0), XPathConstants.NODESET); + assertEquals(2, headers.getLength(), "2 headers expected"); + NodeList header1 = (NodeList) xpath.compile(EVIDENCE_HTTPHEADER_PREFIX+"[@Name='x-forwarded-for']").evaluate(devices.item(0), XPathConstants.NODESET); + assertEquals("5.6.7.8",header1.item(0).getTextContent()); + NodeList header2 = (NodeList) xpath.compile(EVIDENCE_HTTPHEADER_PREFIX+"[@Name='forwarded-for']").evaluate(devices.item(0), XPathConstants.NODESET); + assertEquals("2001::",header2.item(0).getTextContent()); - NodeList cookies = (NodeList) xpath.compile(EVIDENCE_COOKIE_PREFIX).evaluate(devices.item(0), XPathConstants.NODESET); - assertEquals(1, cookies.getLength(), "1 cookie expected"); - assertEquals("51d_Profile", cookies.item(0).getAttributes().getNamedItem("Name").getTextContent()); - assertEquals("123456", cookies.item(0).getFirstChild().getTextContent()); + NodeList cookies = (NodeList) xpath.compile(EVIDENCE_COOKIE_PREFIX).evaluate(devices.item(0), XPathConstants.NODESET); + assertEquals(1, cookies.getLength(), "1 cookie expected"); + assertEquals("51d_Profile", cookies.item(0).getAttributes().getNamedItem("Name").getTextContent()); + assertEquals("123456", cookies.item(0).getFirstChild().getTextContent()); - NodeList clientIp = (NodeList) xpath.compile("ClientIP").evaluate(devices.item(0), XPathConstants.NODESET); - assertEquals(1, clientIp.getLength(), "1 client IP expected"); - assertEquals("1.2.3.4", clientIp.item(0).getTextContent()); + NodeList clientIp = (NodeList) xpath.compile("ClientIP").evaluate(devices.item(0), XPathConstants.NODESET); + assertEquals(1, clientIp.getLength(), "1 client IP expected"); + assertEquals("1.2.3.4", clientIp.item(0).getTextContent()); + } } @Test @@ -373,14 +388,15 @@ public void ShareUsageElement_TwoEvents_FirstEvent() throws Exception { Map evidenceData = new HashMap<>(); evidenceData.put(EVIDENCE_CLIENTIP_KEY, "1.2.3.4"); - FlowData data = pipeline.createFlowData(); - data.addEvidence(evidenceData); + try (FlowData data = pipeline.createFlowData()) { + data.addEvidence(evidenceData); - // Act - shareUsageElement.process(data); + // Act + shareUsageElement.process(data); - // sending of data never gets triggered - assertNull(shareUsageElement.getSendDataFuture()); + // sending of data never gets triggered + assertNull(shareUsageElement.getSendDataFuture()); + } } @Test @@ -396,30 +412,31 @@ public void ShareUsageElement_TwoEvents_SecondEvent() throws Exception { Map evidenceData = new HashMap<>(); evidenceData.put(EVIDENCE_CLIENTIP_KEY, "1.2.3.4"); - FlowData data = pipeline.createFlowData(); - data.addEvidence(evidenceData); + try (FlowData data = pipeline.createFlowData()) { + data.addEvidence(evidenceData); - // Act - shareUsageElement.process(data); - shareUsageElement.process(data); + // Act + shareUsageElement.process(data); + shareUsageElement.process(data); - // Wait for the consumer task to finish. - assertNotNull(shareUsageElement.getSendDataFuture()); - shareUsageElement.getSendDataFuture().get(); + // Wait for the consumer task to finish. + assertNotNull(shareUsageElement.getSendDataFuture()); + shareUsageElement.getSendDataFuture().get(); - ByteArrayOutputStream baos = connector.getBaos(); - byte[] output = baos.toByteArray(); + ByteArrayOutputStream baos = connector.getBaos(); + byte[] output = baos.toByteArray(); - DocumentBuilder builder = docBuilderFactory.newDocumentBuilder(); - Document doc = builder.parse(new ByteArrayInputStream(output)); + DocumentBuilder builder = docBuilderFactory.newDocumentBuilder(); + Document doc = builder.parse(new ByteArrayInputStream(output)); - // Verify that there is only one devices element - NodeList devices = (NodeList) xpath.compile("//Devices").evaluate(doc, XPathConstants.NODESET); - assertEquals(1, devices.getLength(), "1 devices node expected"); + // Verify that there is only one devices element + NodeList devices = (NodeList) xpath.compile("//Devices").evaluate(doc, XPathConstants.NODESET); + assertEquals(1, devices.getLength(), "1 devices node expected"); - // Make sure there are 2 'Device' nodes - NodeList device = (NodeList) xpath.compile("//Device").evaluate(devices.item(0), XPathConstants.NODESET); - assertEquals(2, device.getLength(), "2 device nodes expected"); + // Make sure there are 2 'Device' nodes + NodeList device = (NodeList) xpath.compile("//Device").evaluate(devices.item(0), XPathConstants.NODESET); + assertEquals(2, device.getLength(), "2 device nodes expected"); + } } @Test @@ -440,33 +457,34 @@ public void ShareUsageElement_RestrictedHeaders() throws Exception { evidenceData.put(EVIDENCE_HTTPHEADER_PREFIX + EVIDENCE_SEPERATOR + "forwarded-for", "2001::"); evidenceData.put(EVIDENCE_HTTPHEADER_PREFIX + EVIDENCE_SEPERATOR + "user-agent", useragent); - FlowData data = pipeline.createFlowData(); - data.addEvidence(evidenceData); - // Act - shareUsageElement.process(data); - // Wait for the consumer task to finish. - assertNotNull(shareUsageElement.getSendDataFuture()); - shareUsageElement.getSendDataFuture().get(); - - ByteArrayOutputStream baos = connector.getBaos(); - byte[] output = baos.toByteArray(); - - DocumentBuilder builder = docBuilderFactory.newDocumentBuilder(); - Document doc = builder.parse(new ByteArrayInputStream(output)); - - // Verify that there is only one clientIP element - NodeList clientIP = (NodeList) xpath.compile("//ClientIP").evaluate(doc, XPathConstants.NODESET); - assertEquals(1, clientIP.getLength(), "1 ClientIP node expected"); - - // Make sure there is a header node - NodeList device = (NodeList) xpath.compile("//header[@Name='user-agent']").evaluate(doc, XPathConstants.NODESET); - assertEquals(1, device.getLength(), "1 user-agent header expected"); - // Make sure no x-forwarded-for - NodeList xf = (NodeList) xpath.compile("//header[@Name='x-forwarded-for']").evaluate(doc, XPathConstants.NODESET); - assertEquals(0, xf.getLength(), "0 x-forwarded-for header expected"); - // Make sure no forwarded-for - NodeList f = (NodeList) xpath.compile("//header[@Name='forwarded-for']").evaluate(doc, XPathConstants.NODESET); - assertEquals(0, f.getLength(), "0 forwarded-for header expected"); + try (FlowData data = pipeline.createFlowData()) { + data.addEvidence(evidenceData); + // Act + shareUsageElement.process(data); + // Wait for the consumer task to finish. + assertNotNull(shareUsageElement.getSendDataFuture()); + shareUsageElement.getSendDataFuture().get(); + + ByteArrayOutputStream baos = connector.getBaos(); + byte[] output = baos.toByteArray(); + + DocumentBuilder builder = docBuilderFactory.newDocumentBuilder(); + Document doc = builder.parse(new ByteArrayInputStream(output)); + + // Verify that there is only one clientIP element + NodeList clientIP = (NodeList) xpath.compile("//ClientIP").evaluate(doc, XPathConstants.NODESET); + assertEquals(1, clientIP.getLength(), "1 ClientIP node expected"); + + // Make sure there is a header node + NodeList device = (NodeList) xpath.compile("//header[@Name='user-agent']").evaluate(doc, XPathConstants.NODESET); + assertEquals(1, device.getLength(), "1 user-agent header expected"); + // Make sure no x-forwarded-for + NodeList xf = (NodeList) xpath.compile("//header[@Name='x-forwarded-for']").evaluate(doc, XPathConstants.NODESET); + assertEquals(0, xf.getLength(), "0 x-forwarded-for header expected"); + // Make sure no forwarded-for + NodeList f = (NodeList) xpath.compile("//header[@Name='forwarded-for']").evaluate(doc, XPathConstants.NODESET); + assertEquals(0, f.getLength(), "0 forwarded-for header expected"); + } } @Test @@ -483,32 +501,33 @@ public void ShareUsageElement_LowPercentage() throws Exception { Map evidenceData = new HashMap<>(); evidenceData.put(EVIDENCE_CLIENTIP_KEY, "1.2.3.4"); - FlowData data = pipeline.createFlowData(); - data.addEvidence(evidenceData); + try (FlowData data = pipeline.createFlowData()) { + data.addEvidence(evidenceData); - // Act - int requiredEvents = 0; - while (connector.getBaos().size()==0 && requiredEvents <= 1000000) { - shareUsageElement.process(data); - requiredEvents++; + // Act + int requiredEvents = 0; + while (connector.getBaos().size()==0 && requiredEvents <= 1000000) { + shareUsageElement.process(data); + requiredEvents++; + } + // Wait for the consumer task to finish. + assertNotNull(shareUsageElement.getSendDataFuture()); + shareUsageElement.getSendDataFuture().get(); + + logger.info("Required events was {}", requiredEvents); + + // Assert + // On average, the number of required events should be around + // 100,000. However, as it's chance based it can vary + // significantly. We only want to catch any gross errors so just + // make sure the value is of the expected order of magnitude. + assertTrue(requiredEvents > 10000, + "Expected the number of required events to be at least " + + "10,000, but was actually '" + requiredEvents + "'"); + assertTrue(requiredEvents < 1000000, + "Expected the number of required events to be less than " + + "1,000,000, but was actually '" + requiredEvents + "'"); } - // Wait for the consumer task to finish. - assertNotNull(shareUsageElement.getSendDataFuture()); - shareUsageElement.getSendDataFuture().get(); - - logger.info("Required events was {}", requiredEvents); - - // Assert - // On average, the number of required events should be around - // 100,000. However, as it's chance based it can vary - // significantly. We only want to catch any gross errors so just - // make sure the value is of the expected order of magnitude. - assertTrue(requiredEvents > 10000, - "Expected the number of required events to be at least " + - "10,000, but was actually '" + requiredEvents + "'"); - assertTrue(requiredEvents < 1000000, - "Expected the number of required events to be less than " + - "1,000,000, but was actually '" + requiredEvents + "'"); } @Test public void ShareUsageElement_SendOnCleanup() throws Exception { @@ -524,33 +543,34 @@ public void ShareUsageElement_SendOnCleanup() throws Exception { Map evidenceData = new HashMap<>(); evidenceData.put(EVIDENCE_CLIENTIP_KEY, "1.2.3.4"); - FlowData data = pipeline.createFlowData(); - data.addEvidence(evidenceData); + try (FlowData data = pipeline.createFlowData()) { + data.addEvidence(evidenceData); - // Act - shareUsageElement.process(data); + // Act + shareUsageElement.process(data); - // No data should be being sending yet. - assertNull(shareUsageElement.getSendDataFuture()); + // No data should be being sending yet. + assertNull(shareUsageElement.getSendDataFuture()); - // Dispose of the element. - shareUsageElement.close(); + // Dispose of the element. + shareUsageElement.close(); - // Assert - // data should have been sent - assertNotNull(shareUsageElement.getSendDataFuture()); - assertTrue(shareUsageElement.isClosed()); + // Assert + // data should have been sent + assertNotNull(shareUsageElement.getSendDataFuture()); + assertTrue(shareUsageElement.isClosed()); - ByteArrayOutputStream baos = connector.getBaos(); - byte[] output = baos.toByteArray(); + ByteArrayOutputStream baos = connector.getBaos(); + byte[] output = baos.toByteArray(); - DocumentBuilder builder = docBuilderFactory.newDocumentBuilder(); - Document doc = builder.parse(new ByteArrayInputStream(output)); + DocumentBuilder builder = docBuilderFactory.newDocumentBuilder(); + Document doc = builder.parse(new ByteArrayInputStream(output)); - // Check that the expected values are populated. - NodeList f = (NodeList) xpath.compile("//Device").evaluate(doc, XPathConstants.NODESET); - assertEquals(1, f.getLength(), "1 device expected"); + // Check that the expected values are populated. + NodeList f = (NodeList) xpath.compile("//Device").evaluate(doc, XPathConstants.NODESET); + assertEquals(1, f.getLength(), "1 device expected"); + } } @Test public void ShareUsageElement_CancelOnServerError() throws Exception { @@ -568,17 +588,18 @@ public void ShareUsageElement_CancelOnServerError() throws Exception { ((TestConnector)shareUsageElement.dataUploader).setResponseCode(500); Map evidenceData = new HashMap<>(); evidenceData.put(EVIDENCE_CLIENTIP_KEY, "1.2.3.4"); - FlowData data = pipeline.createFlowData(); - data.addEvidence(evidenceData); + try (FlowData data = pipeline.createFlowData()) { + data.addEvidence(evidenceData); - shareUsageElement.process(data); - // Wait for the consumer task to finish. - assertNotNull(shareUsageElement.getSendDataFuture()); - shareUsageElement.getSendDataFuture().get(); + shareUsageElement.process(data); + // Wait for the consumer task to finish. + assertNotNull(shareUsageElement.getSendDataFuture()); + shareUsageElement.getSendDataFuture().get(); - // Assert - assertFalse(shareUsageElement.isCanceled()); - assertEquals(0, shareUsageElement.evidenceCollection.size()); + // Assert + assertFalse(shareUsageElement.isCanceled()); + assertEquals(0, shareUsageElement.evidenceCollection.size()); + } } @Test @@ -598,15 +619,16 @@ public void ShareUsageElement_IgnoreOnEvidence() throws Exception { evidenceData.put(EVIDENCE_CLIENTIP_KEY, "1.2.3.4"); evidenceData.put("header.User-Agent", "Azure Traffic Manager Endpoint Monitor"); - FlowData data = pipeline.createFlowData(); - data.addEvidence(evidenceData); + try (FlowData data = pipeline.createFlowData()) { + data.addEvidence(evidenceData); - // Act - shareUsageElement.process(data); - // Check that the consumer task did not start. - assertNull(shareUsageElement.getSendDataFuture()); + // Act + shareUsageElement.process(data); + // Check that the consumer task did not start. + assertNull(shareUsageElement.getSendDataFuture()); - assertEquals(0, connector.getBaos().size()); + assertEquals(0, connector.getBaos().size()); + } } @Test @Disabled // we now throw exceptions for invalid data @@ -672,28 +694,29 @@ public void ShareUsageElement_BadSchema() throws Exception { // (0x0018) - Cancel control character evidenceData.put(EVIDENCE_HEADER_USERAGENT_KEY, ""); - FlowData data = pipeline.createFlowData(); - data.addEvidence(evidenceData); + try (FlowData data = pipeline.createFlowData()) { + data.addEvidence(evidenceData); - // Act - shareUsageElement.process(data); - // Wait for the consumer task to finish. - assertNotNull(shareUsageElement.getSendDataFuture()); - shareUsageElement.getSendDataFuture().get(); - - ByteArrayOutputStream baos = connector.getBaos(); - byte[] output = baos.toByteArray(); - - DocumentBuilder builder = docBuilderFactory.newDocumentBuilder(); - Document doc = builder.parse(new ByteArrayInputStream(output)); - - // Check that the expected values are populated. - NodeList f = (NodeList) xpath.compile("//header[@Name='user-agent']").evaluate(doc, XPathConstants.NODESET); - assertEquals(1, f.getLength(), "1 user-agent header expected"); - assertEquals("", f.item(0).getTextContent()); - assertEquals("true", f.item(0).getAttributes().getNamedItem("replaced").getTextContent()); - NodeList b = (NodeList) xpath.compile("//BadSchema").evaluate(doc, XPathConstants.NODESET); - assertEquals(0, b.getLength()); + // Act + shareUsageElement.process(data); + // Wait for the consumer task to finish. + assertNotNull(shareUsageElement.getSendDataFuture()); + shareUsageElement.getSendDataFuture().get(); + + ByteArrayOutputStream baos = connector.getBaos(); + byte[] output = baos.toByteArray(); + + DocumentBuilder builder = docBuilderFactory.newDocumentBuilder(); + Document doc = builder.parse(new ByteArrayInputStream(output)); + + // Check that the expected values are populated. + NodeList f = (NodeList) xpath.compile("//header[@Name='user-agent']").evaluate(doc, XPathConstants.NODESET); + assertEquals(1, f.getLength(), "1 user-agent header expected"); + assertEquals("", f.item(0).getTextContent()); + assertEquals("true", f.item(0).getAttributes().getNamedItem("replaced").getTextContent()); + NodeList b = (NodeList) xpath.compile("//BadSchema").evaluate(doc, XPathConstants.NODESET); + assertEquals(0, b.getLength()); + } } /** @@ -715,27 +738,28 @@ public void ShareUsageElement_SessionIdAndSequence_None() throws Exception { Map evidenceData = new HashMap<>(); evidenceData.put(EVIDENCE_CLIENTIP_KEY, "1.2.3.4"); - FlowData data = pipeline.createFlowData(); - data.addEvidence(evidenceData); + try (FlowData data = pipeline.createFlowData()) { + data.addEvidence(evidenceData); - // Act - sequenceElement.process(data); - shareUsageElement.process(data); + // Act + sequenceElement.process(data); + shareUsageElement.process(data); - // Wait for the consumer task to finish. - assertNotNull(shareUsageElement.getSendDataFuture()); - shareUsageElement.getSendDataFuture().get(); + // Wait for the consumer task to finish. + assertNotNull(shareUsageElement.getSendDataFuture()); + shareUsageElement.getSendDataFuture().get(); - ByteArrayOutputStream baos = connector.getBaos(); - byte[] output = baos.toByteArray(); + ByteArrayOutputStream baos = connector.getBaos(); + byte[] output = baos.toByteArray(); - DocumentBuilder builder = docBuilderFactory.newDocumentBuilder(); - Document doc = builder.parse(new ByteArrayInputStream(output)); + DocumentBuilder builder = docBuilderFactory.newDocumentBuilder(); + Document doc = builder.parse(new ByteArrayInputStream(output)); - NodeList f = (NodeList) xpath.compile("//SessionId").evaluate(doc, XPathConstants.NODESET); - assertEquals(1, f.getLength(), "Expecting 1 session ID"); - NodeList ff = (NodeList) xpath.compile("//Sequence").evaluate(doc, XPathConstants.NODESET); - assertEquals("1", ff.item(0).getTextContent(), "Expecting Sequence ID 1"); + NodeList f = (NodeList) xpath.compile("//SessionId").evaluate(doc, XPathConstants.NODESET); + assertEquals(1, f.getLength(), "Expecting 1 session ID"); + NodeList ff = (NodeList) xpath.compile("//Sequence").evaluate(doc, XPathConstants.NODESET); + assertEquals("1", ff.item(0).getTextContent(), "Expecting Sequence ID 1"); + } } /** @@ -760,28 +784,29 @@ public void ShareUsageElement_SessionIdAndSequence_Existing() throws Exception { evidenceData.put(Constants.EVIDENCE_SESSIONID, "abcdefg-hijklmn-opqrst-uvwxyz"); evidenceData.put(Constants.EVIDENCE_SEQUENCE, 2); - FlowData data = pipeline.createFlowData(); - data.addEvidence(evidenceData); + try (FlowData data = pipeline.createFlowData()) { + data.addEvidence(evidenceData); - // Act - sequenceElement.process(data); - shareUsageElement.process(data); + // Act + sequenceElement.process(data); + shareUsageElement.process(data); - // Wait for the consumer task to finish. - assertNotNull(shareUsageElement.getSendDataFuture()); - shareUsageElement.getSendDataFuture().get(); + // Wait for the consumer task to finish. + assertNotNull(shareUsageElement.getSendDataFuture()); + shareUsageElement.getSendDataFuture().get(); - ByteArrayOutputStream baos = connector.getBaos(); - byte[] output = baos.toByteArray(); + ByteArrayOutputStream baos = connector.getBaos(); + byte[] output = baos.toByteArray(); - DocumentBuilder builder = docBuilderFactory.newDocumentBuilder(); - Document doc = builder.parse(new ByteArrayInputStream(output)); + DocumentBuilder builder = docBuilderFactory.newDocumentBuilder(); + Document doc = builder.parse(new ByteArrayInputStream(output)); - NodeList f = (NodeList) xpath.compile("//SessionId").evaluate(doc, XPathConstants.NODESET); - assertEquals(1, f.getLength(), "Expecting 1 session ID"); - assertEquals("abcdefg-hijklmn-opqrst-uvwxyz", f.item(0).getTextContent(), "Expecting session ID to be ..."); - NodeList ff = (NodeList) xpath.compile("//Sequence").evaluate(doc, XPathConstants.NODESET); - assertEquals("3", ff.item(0).getTextContent(), "Expecting Sequence ID 1"); + NodeList f = (NodeList) xpath.compile("//SessionId").evaluate(doc, XPathConstants.NODESET); + assertEquals(1, f.getLength(), "Expecting 1 session ID"); + assertEquals("abcdefg-hijklmn-opqrst-uvwxyz", f.item(0).getTextContent(), "Expecting session ID to be ..."); + NodeList ff = (NodeList) xpath.compile("//Sequence").evaluate(doc, XPathConstants.NODESET); + assertEquals("3", ff.item(0).getTextContent(), "Expecting Sequence ID 1"); + } } /** @@ -808,28 +833,29 @@ public void CheckForOffByOneError() throws Exception { Map evidenceData = new HashMap<>(); evidenceData.put(EVIDENCE_CLIENTIP_KEY, "1.2.3.4"); - FlowData data = pipeline.createFlowData(); - data.addEvidence(evidenceData); + try (FlowData data = pipeline.createFlowData()) { + data.addEvidence(evidenceData); - // collect multiple documents - connector.getBaos().write("".getBytes(StandardCharsets.UTF_8)); - // Act - for (int i=0; i < count; i++) { - shareUsageElement.process(data); - } - // Wait for the consumer task to finish. - shareUsageElement.close(); + // collect multiple documents + connector.getBaos().write("".getBytes(StandardCharsets.UTF_8)); + // Act + for (int i=0; i < count; i++) { + shareUsageElement.process(data); + } + // Wait for the consumer task to finish. + shareUsageElement.close(); - ByteArrayOutputStream baos = connector.getBaos(); - baos.write("".getBytes(StandardCharsets.UTF_8)); - byte[] output = baos.toByteArray(); + ByteArrayOutputStream baos = connector.getBaos(); + baos.write("".getBytes(StandardCharsets.UTF_8)); + byte[] output = baos.toByteArray(); - DocumentBuilder builder = docBuilderFactory.newDocumentBuilder(); - Document doc = builder.parse(new ByteArrayInputStream(output)); + DocumentBuilder builder = docBuilderFactory.newDocumentBuilder(); + Document doc = builder.parse(new ByteArrayInputStream(output)); - NodeList f = (NodeList) xpath.compile("//Device").evaluate(doc, XPathConstants.NODESET); - assertEquals(count, f.getLength(), "Expecting devices"); + NodeList f = (NodeList) xpath.compile("//Device").evaluate(doc, XPathConstants.NODESET); + assertEquals(count, f.getLength(), "Expecting devices"); + } } /** * A consumer that has finished its drain loop with fewer entries left than @@ -890,54 +916,55 @@ protected void sendUsageData() { Map evidenceData = new HashMap<>(); evidenceData.put(EVIDENCE_CLIENTIP_KEY, "1.2.3.4"); - FlowData data = pipeline.createFlowData(); - data.addEvidence(evidenceData); + try (FlowData data = pipeline.createFlowData()) { + data.addEvidence(evidenceData); - // collect multiple documents - connector.getBaos().write("".getBytes(StandardCharsets.UTF_8)); + // collect multiple documents + connector.getBaos().write("".getBytes(StandardCharsets.UTF_8)); - // close in a finally as well, so that an assertion failure before the - // Act step below still releases the lingering consumer. - try { - // Send one full batch and wait for the consumer to drain it. - for (int i = 0; i < minimumEntriesPerMessage; i++) { - shareUsageElement.process(data); - } - assertTrue(consumerHasDrainedQueue.await(LINGER_TIMEOUT_MILLIS, TimeUnit.MILLISECONDS), - "The consumer should have drained the first batch"); + // close in a finally as well, so that an assertion failure before the + // Act step below still releases the lingering consumer. + try { + // Send one full batch and wait for the consumer to drain it. + for (int i = 0; i < minimumEntriesPerMessage; i++) { + shareUsageElement.process(data); + } + assertTrue(consumerHasDrainedQueue.await(LINGER_TIMEOUT_MILLIS, TimeUnit.MILLISECONDS), + "The consumer should have drained the first batch"); - // Queue too few entries to start a consumer of their own, while the - // previous consumer's task has not completed yet. - for (int i = 0; i < remainder; i++) { - shareUsageElement.process(data); - } - assertEquals(remainder, shareUsageElement.evidenceCollection.size()); - assertTrue(shareUsageElement.isRunning(), - "The first consumer should still be reported as running"); + // Queue too few entries to start a consumer of their own, while the + // previous consumer's task has not completed yet. + for (int i = 0; i < remainder; i++) { + shareUsageElement.process(data); + } + assertEquals(remainder, shareUsageElement.evidenceCollection.size()); + assertTrue(shareUsageElement.isRunning(), + "The first consumer should still be reported as running"); - // Act - close must flush the remainder. - shareUsageElement.close(); - assertEquals(0, shareUsageElement.evidenceCollection.size(), - "close should leave nothing queued"); - } finally { - // Swallowed so that a failure above is still the reported one - the - // close in the Act step is the one under test. - try { + // Act - close must flush the remainder. shareUsageElement.close(); - } catch (Exception exception) { - logger.warn("Exception closing the element under test", exception); + assertEquals(0, shareUsageElement.evidenceCollection.size(), + "close should leave nothing queued"); + } finally { + // Swallowed so that a failure above is still the reported one - the + // close in the Act step is the one under test. + try { + shareUsageElement.close(); + } catch (Exception exception) { + logger.warn("Exception closing the element under test", exception); + } } - } - ByteArrayOutputStream baos = connector.getBaos(); - baos.write("".getBytes(StandardCharsets.UTF_8)); - byte[] output = baos.toByteArray(); + ByteArrayOutputStream baos = connector.getBaos(); + baos.write("".getBytes(StandardCharsets.UTF_8)); + byte[] output = baos.toByteArray(); - DocumentBuilder builder = docBuilderFactory.newDocumentBuilder(); - Document doc = builder.parse(new ByteArrayInputStream(output)); + DocumentBuilder builder = docBuilderFactory.newDocumentBuilder(); + Document doc = builder.parse(new ByteArrayInputStream(output)); - NodeList f = (NodeList) xpath.compile("//Device").evaluate(doc, XPathConstants.NODESET); - assertEquals(minimumEntriesPerMessage + remainder, f.getLength(), "Expecting devices"); + NodeList f = (NodeList) xpath.compile("//Device").evaluate(doc, XPathConstants.NODESET); + assertEquals(minimumEntriesPerMessage + remainder, f.getLength(), "Expecting devices"); + } } @Test @@ -962,29 +989,30 @@ public void CheckForDrainQueueEvenIfError() throws Exception { Map evidenceData = new HashMap<>(); evidenceData.put(EVIDENCE_CLIENTIP_KEY, "1.2.3.4"); - FlowData data = pipeline.createFlowData(); - data.addEvidence(evidenceData); + try (FlowData data = pipeline.createFlowData()) { + data.addEvidence(evidenceData); - // collect multiple documents - connector.getBaos().write("".getBytes(StandardCharsets.UTF_8)); - // Act - for (int i=0; i < count; i++) { - shareUsageElement.process(data); - } - // Wait for the consumer task to finish. - shareUsageElement.close(); + // collect multiple documents + connector.getBaos().write("".getBytes(StandardCharsets.UTF_8)); + // Act + for (int i=0; i < count; i++) { + shareUsageElement.process(data); + } + // Wait for the consumer task to finish. + shareUsageElement.close(); - ByteArrayOutputStream baos = connector.getBaos(); - baos.write("".getBytes(StandardCharsets.UTF_8)); - byte[] output = baos.toByteArray(); + ByteArrayOutputStream baos = connector.getBaos(); + baos.write("".getBytes(StandardCharsets.UTF_8)); + byte[] output = baos.toByteArray(); - DocumentBuilder builder = docBuilderFactory.newDocumentBuilder(); - Document doc = builder.parse(new ByteArrayInputStream(output)); + DocumentBuilder builder = docBuilderFactory.newDocumentBuilder(); + Document doc = builder.parse(new ByteArrayInputStream(output)); - NodeList f = (NodeList) xpath.compile("//Device").evaluate(doc, XPathConstants.NODESET); - // this shows that sending is ignoring the failures - assertEquals(count, f.getLength(), "Expecting devices"); + NodeList f = (NodeList) xpath.compile("//Device").evaluate(doc, XPathConstants.NODESET); + // this shows that sending is ignoring the failures + assertEquals(count, f.getLength(), "Expecting devices"); + } } @Test public void CheckForContinueAndResumeWhenQueueFull() throws Exception { @@ -1003,48 +1031,49 @@ public void CheckForContinueAndResumeWhenQueueFull() throws Exception { Map evidenceData = new HashMap<>(); evidenceData.put(EVIDENCE_CLIENTIP_KEY, "1.2.3.4"); - FlowData data = pipeline.createFlowData(); - data.addEvidence(evidenceData); + try (FlowData data = pipeline.createFlowData()) { + data.addEvidence(evidenceData); - // the output document will collect multiple documents, - // so we need to wrap them in a root element - connector.getBaos().write("".getBytes(StandardCharsets.UTF_8)); + // the output document will collect multiple documents, + // so we need to wrap them in a root element + connector.getBaos().write("".getBytes(StandardCharsets.UTF_8)); - // Overwhelm the queue - int count = 55; - for (int i=0; i < count; i++) { - shareUsageElement.process(data); - } - long amountOfLostData; - assertTrue((amountOfLostData = shareUsageElement.lostData) > 0); - // wait for queue to drain - logger.info("Sleeping"); - Thread.sleep(5000); - - // send a new element which should reset the flag - shareUsageElement.process(data); - assertEquals(amountOfLostData, shareUsageElement.lostData); - - // overwhelm the queue again - for (int i=0; i < count; i++) { + // Overwhelm the queue + int count = 55; + for (int i=0; i < count; i++) { + shareUsageElement.process(data); + } + long amountOfLostData; + assertTrue((amountOfLostData = shareUsageElement.lostData) > 0); + // wait for queue to drain + logger.info("Sleeping"); + Thread.sleep(5000); + + // send a new element which should reset the flag shareUsageElement.process(data); - } - assertTrue(amountOfLostData < shareUsageElement.lostData); + assertEquals(amountOfLostData, shareUsageElement.lostData); + + // overwhelm the queue again + for (int i=0; i < count; i++) { + shareUsageElement.process(data); + } + assertTrue(amountOfLostData < shareUsageElement.lostData); - // Wait for the consumer task to finish. - shareUsageElement.close(); + // Wait for the consumer task to finish. + shareUsageElement.close(); - // get the data output - ByteArrayOutputStream baos = connector.getBaos(); - baos.write("".getBytes(StandardCharsets.UTF_8)); - byte[] output = baos.toByteArray(); + // get the data output + ByteArrayOutputStream baos = connector.getBaos(); + baos.write("".getBytes(StandardCharsets.UTF_8)); + byte[] output = baos.toByteArray(); - DocumentBuilder builder = docBuilderFactory.newDocumentBuilder(); - Document doc = builder.parse(new ByteArrayInputStream(output)); + DocumentBuilder builder = docBuilderFactory.newDocumentBuilder(); + Document doc = builder.parse(new ByteArrayInputStream(output)); - NodeList f = (NodeList) xpath.compile("//Device").evaluate(doc, XPathConstants.NODESET); - // the count is what we put in minus what we have recorded as lost - assertEquals(2 * count + 1 - shareUsageElement.lostData, f.getLength(), "Expecting devices"); + NodeList f = (NodeList) xpath.compile("//Device").evaluate(doc, XPathConstants.NODESET); + // the count is what we put in minus what we have recorded as lost + assertEquals(2 * count + 1 - shareUsageElement.lostData, f.getLength(), "Expecting devices"); + } } } diff --git a/pipeline/pipeline.engines.fiftyone/src/test/java/fiftyone/pipeline/engines/fiftyone/performance/ShareUsageOverheadTests.java b/pipeline/pipeline.engines.fiftyone/src/test/java/fiftyone/pipeline/engines/fiftyone/performance/ShareUsageOverheadTests.java index be67bd7e4..9583850a7 100644 --- a/pipeline/pipeline.engines.fiftyone/src/test/java/fiftyone/pipeline/engines/fiftyone/performance/ShareUsageOverheadTests.java +++ b/pipeline/pipeline.engines.fiftyone/src/test/java/fiftyone/pipeline/engines/fiftyone/performance/ShareUsageOverheadTests.java @@ -30,10 +30,12 @@ import fiftyone.pipeline.engines.fiftyone.flowelements.ShareUsageBuilder; import fiftyone.pipeline.engines.fiftyone.flowelements.ShareUsageElement; import fiftyone.pipeline.engines.flowelements.AspectEngine; +import fiftyone.pipeline.engines.services.DataUploader; import fiftyone.pipeline.engines.services.HttpClient; import fiftyone.pipeline.engines.testhelpers.flowelements.EmptyEngineBuilder; import org.json.JSONObject; import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -95,43 +97,50 @@ public void Initialise() throws Exception { .setProcessCost(0) .build(); + // Closing the element sends what is still queued, so it is given an + // uploader that sends nothing anywhere. ShareUsageElement shareUsage = - new ShareUsageBuilder(loggerFactory) + new ShareUsageBuilder(loggerFactory, mock(DataUploader.class)) .build(); pipeline = new PipelineBuilder(loggerFactory) .addFlowElement(engine) .addFlowElement(shareUsage) + .setAutoCloseElements(true) .build(); } + @AfterEach + public void cleanUp() throws Exception { + pipeline.close(); + } + + // The FlowData are all processed before any is closed, so that closing + // them is not part of the time measured. They are closed together in a + // finally block rather than one by one with try-with-resources. @Test - public void ShareUsageOverhead_SingleEvidence() { + @SuppressWarnings("PMD.FlowDataInTryWithResources") + public void ShareUsageOverhead_SingleEvidence() throws Exception { int iterations = 10_000; double maxOverheadPerCall = 0.1; List data = new ArrayList<>(); - for (int i = 0; i < iterations; i++) { - FlowData flowData = pipeline.createFlowData(); - flowData.addEvidence("header.user-agent", "Mozilla/5.0 (iPad; U; CPU OS 3_2_1 like Mac OS X; en-us) AppleWebKit/531.21.10 (KHTML, like Gecko) Mobile/7B405"); - data.add(flowData); - } + long start; + long end; + try { + for (int i = 0; i < iterations; i++) { + FlowData flowData = pipeline.createFlowData(); + data.add(flowData); + flowData.addEvidence("header.user-agent", "Mozilla/5.0 (iPad; U; CPU OS 3_2_1 like Mac OS X; en-us) AppleWebKit/531.21.10 (KHTML, like Gecko) Mobile/7B405"); + } - long start = System.currentTimeMillis(); - for (FlowData entry : data) { - entry.process(); - } - long end = System.currentTimeMillis(); - - // It is not efficient to use try-with-resources - // in this scenario, so loop through and perform - // manual close on each FlowData - data.forEach((f) -> { - try { - f.close(); - } catch (Exception e) { - e.printStackTrace(); + start = System.currentTimeMillis(); + for (FlowData entry : data) { + entry.process(); } - }); + end = System.currentTimeMillis(); + } finally { + closeAll(data); + } double msOverheadPerCall = (double)(end - start) / iterations; overheadMillis.put("ShareUsageOverhead_SingleEvidence_ms", msOverheadPerCall); @@ -142,36 +151,34 @@ public void ShareUsageOverhead_SingleEvidence() { msOverheadPerCall + "ms. Maximum permitted is " + maxOverheadPerCall); } + // See ShareUsageOverhead_SingleEvidence for why the FlowData are closed + // in a finally block. @Test - public void ShareUsageOverhead_ThousandEvidence() { + @SuppressWarnings("PMD.FlowDataInTryWithResources") + public void ShareUsageOverhead_ThousandEvidence() throws Exception { int iterations = 1_000; int evidenceCount = 1_000; int maxOverheadPerCallMillis = 10; List data = new ArrayList<>(); - for (int i = 0; i < iterations; i++) { - FlowData flowData = pipeline.createFlowData(); - for (int j = 0; j < evidenceCount; j++) { - flowData.addEvidence("header." + j, j); + long start; + long end; + try { + for (int i = 0; i < iterations; i++) { + FlowData flowData = pipeline.createFlowData(); + data.add(flowData); + for (int j = 0; j < evidenceCount; j++) { + flowData.addEvidence("header." + j, j); + } + flowData.addEvidence("header.user-agent", "Mozilla/5.0 (iPad; U; CPU OS 3_2_1 like Mac OS X; en-us) AppleWebKit/531.21.10 (KHTML, like Gecko) Mobile/7B405"); } - flowData.addEvidence("header.user-agent", "Mozilla/5.0 (iPad; U; CPU OS 3_2_1 like Mac OS X; en-us) AppleWebKit/531.21.10 (KHTML, like Gecko) Mobile/7B405"); - data.add(flowData); - } - long start = System.currentTimeMillis(); - for (FlowData entry : data) { - entry.process(); - } - long end = System.currentTimeMillis(); - - // It is not efficient to use try-with-resources - // in this scenario, so loop through and perform - // manual close on each FlowData - data.forEach((f) -> { - try { - f.close(); - } catch (Exception e) { - e.printStackTrace(); + start = System.currentTimeMillis(); + for (FlowData entry : data) { + entry.process(); } - }); + end = System.currentTimeMillis(); + } finally { + closeAll(data); + } double msOverheadPerCall = (double)(end - start) / iterations; overheadMillis.put("ShareUsageOverhead_HundredEvidence_ms", msOverheadPerCall); @@ -181,4 +188,10 @@ public void ShareUsageOverhead_ThousandEvidence() { "Pipeline with share usage overhead per Process call was " + msOverheadPerCall + "ms. Maximum permitted is " + maxOverheadPerCallMillis); } + + private static void closeAll(List data) throws Exception { + for (FlowData flowData : data) { + flowData.close(); + } + } } From b6c5f35d6c9df8e323e2763d682275da969d2ae5 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Wed, 4 Nov 2026 12:17:26 +0000 Subject: [PATCH 3/9] Fail the build when a FlowData, Pipeline or metadata collection is left open ci/pmd-resource-ruleset.xml holds three PMD rules, run in every tree after the tests compile. A FlowData must be created in a try-with-resources statement, a metadata collection an engine returns must be used in one, and a Pipeline a builder has built must be closed in one or in a finally block, unless each is returned to the caller. --- ci/pmd-resource-ruleset.xml | 35 ++++++++++++++++++++++++++----- device-detection/pom.xml | 35 +++++++++++++++++++++++++++++++ examples/device-detection/pom.xml | 35 +++++++++++++++++++++++++++++++ examples/ip-intelligence/pom.xml | 35 +++++++++++++++++++++++++++++++ ip-intelligence/pom.xml | 2 ++ location/pom.xml | 35 +++++++++++++++++++++++++++++++ pipeline/pom.xml | 35 +++++++++++++++++++++++++++++++ 7 files changed, 207 insertions(+), 5 deletions(-) diff --git a/ci/pmd-resource-ruleset.xml b/ci/pmd-resource-ruleset.xml index bc1193153..5f99fad8e 100644 --- a/ci/pmd-resource-ruleset.xml +++ b/ci/pmd-resource-ruleset.xml @@ -8,7 +8,7 @@ Fails the build when a FlowData, a Pipeline or a collection of metadata is not closed on every path. The native results in a FlowData and the native metadata collections each hold the engine's data set, - so if one is left open, closing the Pipeline cannot free the data set. + so while one is open, closing the Pipeline cannot free the data set. Java never frees the native results by itself. Where something is closed by its owner elsewhere, such as the FlowData @@ -63,11 +63,36 @@ - - + + + A local variable holding a Pipeline that a builder has just built + is a try-with-resources resource, is closed in a finally block of + the same method, or is returned to the caller. + + 3 - + + + + + diff --git a/device-detection/pom.xml b/device-detection/pom.xml index 8aef20aea..0514aa354 100644 --- a/device-detection/pom.xml +++ b/device-detection/pom.xml @@ -52,6 +52,7 @@ 3.7.1 6.5.3 0.7.0 + 3.26.0 true false @@ -304,6 +305,40 @@ + + + org.apache.maven.plugins + maven-pmd-plugin + ${maven-pmd-plugin.version} + + + + ${maven.multiModuleProjectDirectory}/ci/pmd-resource-ruleset.xml + + true + true + true + false + + **/interop/swig/*.java + + + + + check-resources-closed + process-test-classes + + check + + + + diff --git a/examples/device-detection/pom.xml b/examples/device-detection/pom.xml index d7617e6e6..fa55e5534 100644 --- a/examples/device-detection/pom.xml +++ b/examples/device-detection/pom.xml @@ -62,6 +62,7 @@ 2.0.20 1.3.16 + 3.26.0 @@ -165,6 +166,40 @@ + + + org.apache.maven.plugins + maven-pmd-plugin + ${maven-pmd-plugin.version} + + + + ${maven.multiModuleProjectDirectory}/ci/pmd-resource-ruleset.xml + + true + true + true + false + + **/interop/swig/*.java + + + + + check-resources-closed + process-test-classes + + check + + + + diff --git a/examples/ip-intelligence/pom.xml b/examples/ip-intelligence/pom.xml index 945f34223..2e3624a91 100644 --- a/examples/ip-intelligence/pom.xml +++ b/examples/ip-intelligence/pom.xml @@ -63,6 +63,7 @@ 2.0.20 1.3.16 + 3.26.0 @@ -165,6 +166,40 @@ + + + org.apache.maven.plugins + maven-pmd-plugin + ${maven-pmd-plugin.version} + + + + ${maven.multiModuleProjectDirectory}/ci/pmd-resource-ruleset.xml + + true + true + true + false + + **/interop/swig/*.java + + + + + check-resources-closed + process-test-classes + + check + + + + diff --git a/ip-intelligence/pom.xml b/ip-intelligence/pom.xml index 4327c14b2..63690428e 100644 --- a/ip-intelligence/pom.xml +++ b/ip-intelligence/pom.xml @@ -175,6 +175,8 @@ ${maven.multiModuleProjectDirectory}/ci/pmd-ruleset.xml + + ${maven.multiModuleProjectDirectory}/ci/pmd-resource-ruleset.xml true diff --git a/location/pom.xml b/location/pom.xml index e3e332f3f..4ececfe93 100644 --- a/location/pom.xml +++ b/location/pom.xml @@ -78,6 +78,7 @@ 3.0.0 3.0.1 0.7.0 + 3.26.0 true false @@ -300,6 +301,40 @@ + + + org.apache.maven.plugins + maven-pmd-plugin + ${maven-pmd-plugin.version} + + + + ${maven.multiModuleProjectDirectory}/ci/pmd-resource-ruleset.xml + + true + true + true + false + + **/interop/swig/*.java + + + + + check-resources-closed + process-test-classes + + check + + + + diff --git a/pipeline/pom.xml b/pipeline/pom.xml index 868a93a44..fc510636c 100644 --- a/pipeline/pom.xml +++ b/pipeline/pom.xml @@ -95,6 +95,7 @@ 2.6 3.7 0.7.0 + 3.26.0 true false @@ -351,6 +352,40 @@ + + + org.apache.maven.plugins + maven-pmd-plugin + ${maven-pmd-plugin.version} + + + + ${maven.multiModuleProjectDirectory}/ci/pmd-resource-ruleset.xml + + true + true + true + false + + **/interop/swig/*.java + + + + + check-resources-closed + process-test-classes + + check + + + + From 629c39b5c8b29b72485c16b53f27517ffa2f9c3d Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Wed, 4 Nov 2026 12:22:47 +0000 Subject: [PATCH 4/9] Declare the exception closing the FlowData can throw in the disposed pipeline test --- .../java/fiftyone/pipeline/core/flowelements/FlowDataTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pipeline/pipeline.core/src/test/java/fiftyone/pipeline/core/flowelements/FlowDataTests.java b/pipeline/pipeline.core/src/test/java/fiftyone/pipeline/core/flowelements/FlowDataTests.java index 2dd08130b..613033fa2 100644 --- a/pipeline/pipeline.core/src/test/java/fiftyone/pipeline/core/flowelements/FlowDataTests.java +++ b/pipeline/pipeline.core/src/test/java/fiftyone/pipeline/core/flowelements/FlowDataTests.java @@ -318,7 +318,7 @@ public void FlowData_GetNotPresent_Element() { } @Test(expected = IllegalStateException.class) - public void FlowData_PipelineDisposed() { + public void FlowData_PipelineDisposed() throws Exception { when(pipeline.isClosed()).thenReturn(true); try (FlowData flowData = new FlowDataDefault( mock(Logger.class), From e0f90df40478a1d8f056db63cbeccc6bb559db07 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Wed, 4 Nov 2026 12:27:37 +0000 Subject: [PATCH 5/9] Close the pipelines in the pipeline integration tests --- .../integration/PipelineIntegrationTests.java | 34 +++++++++---------- 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/pipeline/pipeline.core/src/test/java/fiftyone/pipeline/core/integration/PipelineIntegrationTests.java b/pipeline/pipeline.core/src/test/java/fiftyone/pipeline/core/integration/PipelineIntegrationTests.java index c891b7181..1abdcd94a 100644 --- a/pipeline/pipeline.core/src/test/java/fiftyone/pipeline/core/integration/PipelineIntegrationTests.java +++ b/pipeline/pipeline.core/src/test/java/fiftyone/pipeline/core/integration/PipelineIntegrationTests.java @@ -54,11 +54,10 @@ public static void setup() { @Test public void PipelineIntegration_SingleElement() throws Exception { MultiplyByFiveElement fiveElement = new MultiplyByFiveElement(mock(Logger.class)); - Pipeline pipeline = new PipelineBuilder(loggerFactory) + try (Pipeline pipeline = new PipelineBuilder(loggerFactory) .addFlowElement(fiveElement) .build(); - - try (FlowData flowData = pipeline.createFlowData()) { + FlowData flowData = pipeline.createFlowData()) { flowData.addEvidence(fiveElement.evidenceKeys.get(0), 2); flowData.process(); @@ -71,12 +70,11 @@ public void PipelineIntegration_SingleElement() throws Exception { public void PipelineIntegration_MultipleElements() throws Exception { MultiplyByFiveElement fiveElement = new MultiplyByFiveElement(mock(Logger.class)); MultiplyByTenElement tenElement = new MultiplyByTenElement(mock(Logger.class)); - Pipeline pipeline = new PipelineBuilder(loggerFactory) + try (Pipeline pipeline = new PipelineBuilder(loggerFactory) .addFlowElement(fiveElement) .addFlowElement(tenElement) .build(); - - try (FlowData flowData = pipeline.createFlowData()) { + FlowData flowData = pipeline.createFlowData()) { flowData.addEvidence(fiveElement.evidenceKeys.get(0), 2); flowData.process(); @@ -91,11 +89,10 @@ public void PipelineIntegration_MultipleElements() throws Exception { public void PipelineIntegration_MultipleElementsParallel() throws Exception { MultiplyByFiveElement fiveElement = new MultiplyByFiveElement(mock(Logger.class)); MultiplyByTenElement tenElement = new MultiplyByTenElement(mock(Logger.class)); - Pipeline pipeline = new PipelineBuilder(loggerFactory) + try (Pipeline pipeline = new PipelineBuilder(loggerFactory) .addFlowElementsParallel(new FlowElement[]{fiveElement, tenElement}) .build(); - - try (FlowData flowData = pipeline.createFlowData()) { + FlowData flowData = pipeline.createFlowData()) { flowData.addEvidence(fiveElement.evidenceKeys.get(0), 2); flowData.process(); @@ -113,19 +110,20 @@ public void PipelineIntegration_StopFlag() throws Exception { FlowElement testElement = mock(FlowElement.class); when(testElement.getElementDataKey()).thenReturn("test"); when(testElement.getProperties()).thenReturn(Collections.emptyList()); - Pipeline pipeline = new PipelineBuilder(loggerFactory) + try (Pipeline pipeline = new PipelineBuilder(loggerFactory) .addFlowElement(stopElement) .addFlowElement(testElement) - .build(); + .build()) { - // Create and process flow data - try (FlowData flowData = pipeline.createFlowData()) { - flowData.process(); + // Create and process flow data + try (FlowData flowData = pipeline.createFlowData()) { + flowData.process(); - // Check that the stop flag is set - assertTrue(flowData.isStopped()); - // Check that the second element was never processed - verify(testElement, never()).process(any(FlowData.class)); + // Check that the stop flag is set + assertTrue(flowData.isStopped()); + // Check that the second element was never processed + verify(testElement, never()).process(any(FlowData.class)); + } } } } From aef12cacac766a441d9b86ad49620685ae2a6f2e Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Wed, 4 Nov 2026 12:31:53 +0000 Subject: [PATCH 6/9] Close the remaining pipelines the resource check reported, and declare the exceptions closing can throw --- .../engine/onpremise/data/MetaDataHashTests.java | 8 ++++---- .../engine/onpremise/data/MetaDataHashTests.java | 8 ++++---- .../pipeline/javascriptbuilder/CookieTests.java | 15 +++++++-------- .../javascriptbuilder/JavaScriptUrlTests.java | 4 ++-- .../pipeline/jsonbuilder/JsonBuilderTests.java | 8 +++++--- 5 files changed, 22 insertions(+), 21 deletions(-) diff --git a/device-detection/device-detection.hash.engine.on-premise/src/test/java/fiftyone/devicedetection/hash/engine/onpremise/data/MetaDataHashTests.java b/device-detection/device-detection.hash.engine.on-premise/src/test/java/fiftyone/devicedetection/hash/engine/onpremise/data/MetaDataHashTests.java index 4d0a9e220..8e4fa034a 100644 --- a/device-detection/device-detection.hash.engine.on-premise/src/test/java/fiftyone/devicedetection/hash/engine/onpremise/data/MetaDataHashTests.java +++ b/device-detection/device-detection.hash.engine.on-premise/src/test/java/fiftyone/devicedetection/hash/engine/onpremise/data/MetaDataHashTests.java @@ -65,7 +65,7 @@ public void MetaData_Hash_Reload() throws ExecutionException, InterruptedExcepti * assumption rather than failing. */ @Test - public void MetaData_Performance_Properties() { + public void MetaData_Performance_Properties() throws Exception { MetaDataHasherHash hasher = new MetaDataHasherHash(); long start = System.currentTimeMillis(); hasher.hashProperties(0, getWrapper()); @@ -83,7 +83,7 @@ public void MetaData_Performance_Properties() { * assumption rather than failing. */ @Test - public void MetaData_Performance_Components() { + public void MetaData_Performance_Components() throws Exception { MetaDataHasherHash hasher = new MetaDataHasherHash(); long start = System.currentTimeMillis(); hasher.hashComponents(0, getWrapper()); @@ -101,7 +101,7 @@ public void MetaData_Performance_Components() { * assumption rather than failing. */ @Test - public void MetaData_Performance_Values() { + public void MetaData_Performance_Values() throws Exception { MetaDataHasherHash hasher = new MetaDataHasherHash(); long start = System.currentTimeMillis(); hasher.hashValues(0, getWrapper()); @@ -119,7 +119,7 @@ public void MetaData_Performance_Values() { * assumption rather than failing. */ @Test - public void MetaData_Performance_Profiles() { + public void MetaData_Performance_Profiles() throws Exception { MetaDataHasherHash hasher = new MetaDataHasherHash(); long start = System.currentTimeMillis(); hasher.hashProfiles(0, getWrapper()); diff --git a/ip-intelligence/ip-intelligence.engine.on-premise/src/test/java/fiftyone/ipintelligence/engine/onpremise/data/MetaDataHashTests.java b/ip-intelligence/ip-intelligence.engine.on-premise/src/test/java/fiftyone/ipintelligence/engine/onpremise/data/MetaDataHashTests.java index d229af08b..f9f9a03eb 100644 --- a/ip-intelligence/ip-intelligence.engine.on-premise/src/test/java/fiftyone/ipintelligence/engine/onpremise/data/MetaDataHashTests.java +++ b/ip-intelligence/ip-intelligence.engine.on-premise/src/test/java/fiftyone/ipintelligence/engine/onpremise/data/MetaDataHashTests.java @@ -67,7 +67,7 @@ public void MetaData_Hash_Reload() throws ExecutionException, InterruptedExcepti * assumption rather than failing. */ @Test - public void MetaData_Performance_Properties() { + public void MetaData_Performance_Properties() throws Exception { MetaDataHasherHash hasher = new MetaDataHasherHash(); long start = System.currentTimeMillis(); hasher.hashProperties(0, getWrapper()); @@ -85,7 +85,7 @@ public void MetaData_Performance_Properties() { * assumption rather than failing. */ @Test - public void MetaData_Performance_Components() { + public void MetaData_Performance_Components() throws Exception { MetaDataHasherHash hasher = new MetaDataHasherHash(); long start = System.currentTimeMillis(); hasher.hashComponents(0, getWrapper()); @@ -103,7 +103,7 @@ public void MetaData_Performance_Components() { * assumption rather than failing. */ @Test - public void MetaData_Performance_Values() { + public void MetaData_Performance_Values() throws Exception { MetaDataHasherHash hasher = new MetaDataHasherHash(); long start = System.currentTimeMillis(); hasher.hashValues(0, getWrapper()); @@ -121,7 +121,7 @@ public void MetaData_Performance_Values() { * assumption rather than failing. */ @Test - public void MetaData_Performance_Profiles() { + public void MetaData_Performance_Profiles() throws Exception { MetaDataHasherHash hasher = new MetaDataHasherHash(); long start = System.currentTimeMillis(); hasher.hashProfiles(0, getWrapper()); diff --git a/pipeline/pipeline.javascriptbuilder/src/test/java/fiftyone/pipeline/javascriptbuilder/CookieTests.java b/pipeline/pipeline.javascriptbuilder/src/test/java/fiftyone/pipeline/javascriptbuilder/CookieTests.java index be3bbaada..5aed919b3 100644 --- a/pipeline/pipeline.javascriptbuilder/src/test/java/fiftyone/pipeline/javascriptbuilder/CookieTests.java +++ b/pipeline/pipeline.javascriptbuilder/src/test/java/fiftyone/pipeline/javascriptbuilder/CookieTests.java @@ -138,16 +138,15 @@ public void TestJavaScriptCookies( JavaScriptBuilderElement jsElement = new JavaScriptBuilderElementBuilder(loggerFactory) .setEnableCookies(enableInConfig) .build(); - Pipeline pipeline = new PipelineBuilder(loggerFactory) - .addFlowElement(cookieElement) - .addFlowElement(sequenceElement) - .addFlowElement(jsonElement) - .addFlowElement(jsElement) - .build(); - // Act String javaScript = null; - try (FlowData flowData = pipeline.createFlowData()) + try (Pipeline pipeline = new PipelineBuilder(loggerFactory) + .addFlowElement(cookieElement) + .addFlowElement(sequenceElement) + .addFlowElement(jsonElement) + .addFlowElement(jsElement) + .build(); + FlowData flowData = pipeline.createFlowData()) { flowData.addEvidence( Constants.EVIDENCE_ENABLE_COOKIES, diff --git a/pipeline/pipeline.javascriptbuilder/src/test/java/fiftyone/pipeline/javascriptbuilder/JavaScriptUrlTests.java b/pipeline/pipeline.javascriptbuilder/src/test/java/fiftyone/pipeline/javascriptbuilder/JavaScriptUrlTests.java index 4f3fc0dea..b11d9639c 100644 --- a/pipeline/pipeline.javascriptbuilder/src/test/java/fiftyone/pipeline/javascriptbuilder/JavaScriptUrlTests.java +++ b/pipeline/pipeline.javascriptbuilder/src/test/java/fiftyone/pipeline/javascriptbuilder/JavaScriptUrlTests.java @@ -71,12 +71,12 @@ private String render(String contextRoot) throws Exception { builder.setContextRoot(contextRoot); } JavaScriptBuilderElement jsElement = builder.build(); - Pipeline pipeline = new PipelineBuilder(loggerFactory) + try (Pipeline pipeline = new PipelineBuilder(loggerFactory) .addFlowElement(new SequenceElementBuilder(loggerFactory).build()) .addFlowElement(new JsonBuilderElementBuilder(loggerFactory).build()) .addFlowElement(jsElement) .build(); - try (FlowData flowData = pipeline.createFlowData()) { + FlowData flowData = pipeline.createFlowData()) { flowData.addEvidence("query.user-agent", "iPhone"); flowData.addEvidence("query.id.usage", "non-marketing"); flowData.process(); diff --git a/pipeline/pipeline.jsonbuilder/src/test/java/fiftyone/pipeline/jsonbuilder/JsonBuilderTests.java b/pipeline/pipeline.jsonbuilder/src/test/java/fiftyone/pipeline/jsonbuilder/JsonBuilderTests.java index 9c9d9f935..965526329 100644 --- a/pipeline/pipeline.jsonbuilder/src/test/java/fiftyone/pipeline/jsonbuilder/JsonBuilderTests.java +++ b/pipeline/pipeline.jsonbuilder/src/test/java/fiftyone/pipeline/jsonbuilder/JsonBuilderTests.java @@ -460,13 +460,15 @@ public void JsonBuilder_LazyLoading() throws Exception { .build(); SequenceElement sequenceElement = new SequenceElementBuilder(loggerFactory) .build(); - Pipeline pipeline = new PipelineBuilder(loggerFactory) + // The pipeline closes the elements, the lazy loading engine's + // threads included. + try (Pipeline pipeline = new PipelineBuilder(loggerFactory) .addFlowElement(sequenceElement) .addFlowElement(engine) .addFlowElement(jsonBuilder) + .setAutoCloseElements(true) .build(); - - try (FlowData flowData = pipeline.createFlowData()) { + FlowData flowData = pipeline.createFlowData()) { flowData.process(); From dec5761ff1019a56474eb98947e0db7ca7380f6c Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Wed, 4 Nov 2026 14:50:45 +0000 Subject: [PATCH 7/9] Close the pipelines in the cloud request origin tests --- .../fiftyone/devicedetection/CloudRequestOriginTests.java | 5 ++--- .../fiftyone/ipintelligence/CloudRequestOriginTests.java | 5 ++--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/device-detection/device-detection/src/test/java/fiftyone/devicedetection/CloudRequestOriginTests.java b/device-detection/device-detection/src/test/java/fiftyone/devicedetection/CloudRequestOriginTests.java index 72e4e2335..bb311b1b3 100644 --- a/device-detection/device-detection/src/test/java/fiftyone/devicedetection/CloudRequestOriginTests.java +++ b/device-detection/device-detection/src/test/java/fiftyone/devicedetection/CloudRequestOriginTests.java @@ -86,12 +86,11 @@ public void ResourceKeyWithOrigin() throws Exception { when(loggerFactory.getLogger(anyString())).thenReturn(logger); try { - Pipeline pipeline = new DeviceDetectionPipelineBuilder(loggerFactory, http) + try (Pipeline pipeline = new DeviceDetectionPipelineBuilder(loggerFactory, http) .useCloud(resourceKey) .setCloudRequestOrigin(this.origin) .build(); - - try(FlowData data = pipeline.createFlowData()) { + FlowData data = pipeline.createFlowData()) { data.addEvidence("query.user-agent", "Mozilla/5.0 (Linux; Android 8.0.0; Pixel 2 XL " + "Build/OPD1.170816.004) AppleWebKit/537.36 " + diff --git a/ip-intelligence/ip-intelligence/src/test/java/fiftyone/ipintelligence/CloudRequestOriginTests.java b/ip-intelligence/ip-intelligence/src/test/java/fiftyone/ipintelligence/CloudRequestOriginTests.java index c4d13d293..54b262730 100644 --- a/ip-intelligence/ip-intelligence/src/test/java/fiftyone/ipintelligence/CloudRequestOriginTests.java +++ b/ip-intelligence/ip-intelligence/src/test/java/fiftyone/ipintelligence/CloudRequestOriginTests.java @@ -88,12 +88,11 @@ public void ResourceKeyWithOrigin() throws Exception { when(loggerFactory.getLogger(anyString())).thenReturn(logger); try { - Pipeline pipeline = new IPIntelligencePipelineBuilder(loggerFactory, http) + try (Pipeline pipeline = new IPIntelligencePipelineBuilder(loggerFactory, http) .useCloud(resourceKey) .setCloudRequestOrigin(this.origin) .build(); - - try(FlowData data = pipeline.createFlowData()) { + FlowData data = pipeline.createFlowData()) { data.addEvidence("query.client-ip", "8.8.8.8") .process(); } From d3056c7ea4e85f4e265fe4db5a1fefe0e5781899 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Wed, 4 Nov 2026 16:38:21 +0000 Subject: [PATCH 8/9] Close the elements buildFromConfiguration has built when it fails When an element failed to build, or the pipeline could not be built from the elements, the elements already built were left open, holding their data files and threads. They are now closed before the failure is passed on. The original exception is the one thrown, and any failure to close an element is added to it as suppressed. --- .../core/flowelements/PipelineBuilder.java | 63 ++++++++++++++----- .../flowelements/PipelineBuilderTests.java | 30 +++++++++ .../flowelements/RecordingElementBuilder.java | 47 ++++++++++++++ 3 files changed, 126 insertions(+), 14 deletions(-) create mode 100644 pipeline/pipeline.core/src/test/java/fiftyone/pipeline/core/testclasses/flowelements/RecordingElementBuilder.java diff --git a/pipeline/pipeline.core/src/main/java/fiftyone/pipeline/core/flowelements/PipelineBuilder.java b/pipeline/pipeline.core/src/main/java/fiftyone/pipeline/core/flowelements/PipelineBuilder.java index 20c846576..fbebda96e 100644 --- a/pipeline/pipeline.core/src/main/java/fiftyone/pipeline/core/flowelements/PipelineBuilder.java +++ b/pipeline/pipeline.core/src/main/java/fiftyone/pipeline/core/flowelements/PipelineBuilder.java @@ -122,9 +122,12 @@ public Pipeline buildFromConfiguration(PipelineOptions options) "The following builder parameters could not be processed: " + stringJoin(builderParams, ",")); } - } catch (PipelineConfigurationException ex) { - logger.debug("Problem with pipeline configuration, " + - "failed to create pipeline.", ex); + } catch (Exception | Error ex) { + if (ex instanceof PipelineConfigurationException) { + logger.debug("Problem with pipeline configuration, " + + "failed to create pipeline.", ex); + } + closeAfterFailure(flowElements, ex); throw ex; } // As the elements are all created within the builder, the user @@ -134,7 +137,32 @@ public Pipeline buildFromConfiguration(PipelineOptions options) // Build and return the pipeline using the list of flow elements // that have been created from the configuration options. - return build(); + try { + return build(); + } catch (Exception | Error ex) { + closeAfterFailure(flowElements, ex); + throw ex; + } + } + + /** + * Closes the elements built so far when building from configuration + * fails, as no pipeline exists to close them. A failure to close one is + * added to the original failure, which is the one reported. + * @param elements the elements built so far, emptied once closed + * @param failure the reason building failed + */ + private static void closeAfterFailure( + List elements, + Throwable failure) { + for (FlowElement element : elements) { + try { + element.close(); + } catch (Exception e) { + failure.addSuppressed(e); + } + } + elements.clear(); } /** @@ -377,17 +405,24 @@ private void addParallelElementsToList( // Iterate through the sub elements, creating them and // adding them to the list. int subCounter = 0; - for (ElementOptions subElement : elementOptions.subElements) { - if (subElement.subElements != null && - subElement.subElements.size() > 0) { - throw new PipelineConfigurationException( - "ElementOptions " + elementIndex + " contains nested " + - "sub elements. This is not supported."); - } else { - addElementToList(parallelElements, subElement, - "element " + subCounter + " in element " + elementIndex); + try { + for (ElementOptions subElement : elementOptions.subElements) { + if (subElement.subElements != null && + subElement.subElements.size() > 0) { + throw new PipelineConfigurationException( + "ElementOptions " + elementIndex + " contains nested " + + "sub elements. This is not supported."); + } else { + addElementToList(parallelElements, subElement, + "element " + subCounter + " in element " + elementIndex); + } + subCounter++; } - subCounter++; + } catch (RuntimeException | Error ex) { + // These elements are not in the builder's list yet, so they are + // closed here. + closeAfterFailure(parallelElements, ex); + throw ex; } // Now we've created all the elements, create the // ParallelElements instance and add it to the pipeline's diff --git a/pipeline/pipeline.core/src/test/java/fiftyone/pipeline/core/flowelements/PipelineBuilderTests.java b/pipeline/pipeline.core/src/test/java/fiftyone/pipeline/core/flowelements/PipelineBuilderTests.java index 3bfb105ef..f8e1c968b 100644 --- a/pipeline/pipeline.core/src/test/java/fiftyone/pipeline/core/flowelements/PipelineBuilderTests.java +++ b/pipeline/pipeline.core/src/test/java/fiftyone/pipeline/core/flowelements/PipelineBuilderTests.java @@ -42,6 +42,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -233,6 +234,35 @@ public void PipelineBuilder_BuildFromConfiguration_MandatoryParameterWrongType() builder.buildFromConfiguration(opts); } + /** + * When an element fails to build, the elements built before it are + * closed, and the failure reported is the original one. + */ + @Test + public void PipelineBuilder_BuildFromConfiguration_SecondElementFails_FirstClosed() throws Exception { + RecordingElementBuilder.BUILT.clear(); + PipelineOptions opts = new PipelineOptions(); + ElementOptions first = new ElementOptions(); + first.builderName = "RecordingElementBuilder"; + opts.elements.add(first); + // The mandatory 'multiple' parameter is not set, so this one fails. + ElementOptions second = new ElementOptions(); + second.builderName = "MultiplyByElementBuilder"; + opts.elements.add(second); + + maxErrors = 1; + + try { + builder.buildFromConfiguration(opts); + fail("Expected the second element to fail to build"); + } catch (PipelineConfigurationException e) { + assertEquals(0, e.getSuppressed().length); + } + assertEquals(1, RecordingElementBuilder.BUILT.size()); + assertTrue("The first element should have been closed", + RecordingElementBuilder.BUILT.get(0).isClosed()); + } + @Test(expected = PipelineConfigurationException.class) public void PipelineBuilder_BuildFromConfiguration_NoBuilder() throws Exception { // Create the configuration object. diff --git a/pipeline/pipeline.core/src/test/java/fiftyone/pipeline/core/testclasses/flowelements/RecordingElementBuilder.java b/pipeline/pipeline.core/src/test/java/fiftyone/pipeline/core/testclasses/flowelements/RecordingElementBuilder.java new file mode 100644 index 000000000..6ab3d4221 --- /dev/null +++ b/pipeline/pipeline.core/src/test/java/fiftyone/pipeline/core/testclasses/flowelements/RecordingElementBuilder.java @@ -0,0 +1,47 @@ +/* ********************************************************************* + * 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.core.testclasses.flowelements; + +import fiftyone.pipeline.annotations.ElementBuilder; +import fiftyone.pipeline.core.flowelements.FlowElement; + +import java.util.ArrayList; +import java.util.List; + +/** + * Builds list splitter elements and keeps each one, so that a test can check + * what happened to the elements a pipeline builder made. + */ +@ElementBuilder +public class RecordingElementBuilder { + + @SuppressWarnings("rawtypes") + public static final List BUILT = new ArrayList<>(); + + @SuppressWarnings("rawtypes") + public FlowElement build() { + FlowElement element = new ListSplitterElement(",", 100000); + BUILT.add(element); + return element; + } +} From 69b631ab52afdba86c272c5dce195ec828a39496 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Wed, 4 Nov 2026 16:42:24 +0000 Subject: [PATCH 9/9] Declare the closeable collections the metadata returns, and close the FlowData once in the web examples ComponentMetaData.getProperties(), ProfileMetaData.getValues(), ProfileMetaData.getValues(String) and FiftyOneAspectPropertyMetaData.getValues() declared Iterable, while the on-premise engines return native collections from them, two of which hold the data set until closed. They now declare CloseableIterable, so a caller can close the collection without a cast, and the callers here close it. The IP Intelligence web examples closed the FlowData that the filter owns and closes at the end of the request. They no longer close it, and their comments say that the filter does. --- .../onpremise/data/ComponentMetaDataHash.java | 3 +- .../onpremise/data/PropertyMetaDataHash.java | 3 +- .../onpremise/data/MetaDataHasherHash.java | 30 +++-- .../examples/console/MetadataOnPrem.java | 103 ++++++++++-------- .../examples/console/MetadataOnPrem.java | 103 ++++++++++-------- .../examples/console/MetricsOnPrem.java | 10 +- .../web/GettingStartedWebCloudMixed.java | 12 +- .../examples/web/GettingStartedWebCloud.java | 16 +-- .../examples/web/GettingStartedWebMixed.java | 12 +- .../examples/web/GettingStartedWebOnPrem.java | 16 +-- .../onpremise/data/ComponentMetaDataIPI.java | 3 +- .../onpremise/data/PropertyMetaDataIPI.java | 3 +- .../onpremise/data/MetaDataHasherHash.java | 30 +++-- .../fiftyone/data/ComponentMetaData.java | 3 +- .../data/FiftyOneAspectPropertyMetaData.java | 3 +- ...FiftyOneAspectPropertyMetaDataDefault.java | 6 +- .../fiftyone/data/ProfileMetaData.java | 6 +- 17 files changed, 189 insertions(+), 173 deletions(-) diff --git a/device-detection/device-detection.hash.engine.on-premise/src/main/java/fiftyone/devicedetection/hash/engine/onpremise/data/ComponentMetaDataHash.java b/device-detection/device-detection.hash.engine.on-premise/src/main/java/fiftyone/devicedetection/hash/engine/onpremise/data/ComponentMetaDataHash.java index 30d18118b..63cfdbf6f 100644 --- a/device-detection/device-detection.hash.engine.on-premise/src/main/java/fiftyone/devicedetection/hash/engine/onpremise/data/ComponentMetaDataHash.java +++ b/device-detection/device-detection.hash.engine.on-premise/src/main/java/fiftyone/devicedetection/hash/engine/onpremise/data/ComponentMetaDataHash.java @@ -22,6 +22,7 @@ package fiftyone.devicedetection.hash.engine.onpremise.data; +import fiftyone.pipeline.engines.fiftyone.data.CloseableIterable; import fiftyone.devicedetection.hash.engine.onpremise.flowelements.DeviceDetectionHashEngine; import fiftyone.devicedetection.hash.engine.onpremise.interop.PropertyIterable; import fiftyone.devicedetection.hash.engine.onpremise.interop.swig.ComponentMetaDataSwig; @@ -77,7 +78,7 @@ public ProfileMetaData getDefaultProfile() { } @Override - public Iterable getProperties() { + public CloseableIterable getProperties() { return new PropertyIterable( engine, properties, diff --git a/device-detection/device-detection.hash.engine.on-premise/src/main/java/fiftyone/devicedetection/hash/engine/onpremise/data/PropertyMetaDataHash.java b/device-detection/device-detection.hash.engine.on-premise/src/main/java/fiftyone/devicedetection/hash/engine/onpremise/data/PropertyMetaDataHash.java index 3dbf2b5d5..6a6b5a92a 100644 --- a/device-detection/device-detection.hash.engine.on-premise/src/main/java/fiftyone/devicedetection/hash/engine/onpremise/data/PropertyMetaDataHash.java +++ b/device-detection/device-detection.hash.engine.on-premise/src/main/java/fiftyone/devicedetection/hash/engine/onpremise/data/PropertyMetaDataHash.java @@ -22,6 +22,7 @@ package fiftyone.devicedetection.hash.engine.onpremise.data; +import fiftyone.pipeline.engines.fiftyone.data.CloseableIterable; import fiftyone.devicedetection.hash.engine.onpremise.flowelements.DeviceDetectionHashEngine; import fiftyone.devicedetection.hash.engine.onpremise.interop.Swig; import fiftyone.devicedetection.hash.engine.onpremise.interop.ValueIterable; @@ -136,7 +137,7 @@ public ComponentMetaData getComponent() { } @Override - public Iterable getValues() { + public CloseableIterable getValues() { return new ValueIterable( engine, engine.getMetaData().getValuesForProperty(source)); diff --git a/device-detection/device-detection.hash.engine.on-premise/src/test/java/fiftyone/devicedetection/hash/engine/onpremise/data/MetaDataHasherHash.java b/device-detection/device-detection.hash.engine.on-premise/src/test/java/fiftyone/devicedetection/hash/engine/onpremise/data/MetaDataHasherHash.java index 293c790d8..aa5a619ba 100644 --- a/device-detection/device-detection.hash.engine.on-premise/src/test/java/fiftyone/devicedetection/hash/engine/onpremise/data/MetaDataHasherHash.java +++ b/device-detection/device-detection.hash.engine.on-premise/src/test/java/fiftyone/devicedetection/hash/engine/onpremise/data/MetaDataHasherHash.java @@ -43,11 +43,13 @@ public int hashProperties(int hash, Wrapper wrapper) throws Exception { hash ^= property.hashCode(); int j = 0; - for (ValueMetaData value : property.getValues()) { - if (j % 10 == 0) { - hash ^= value.hashCode(); + try (CloseableIterable propertyValues = property.getValues()) { + for (ValueMetaData value : propertyValues) { + if (j % 10 == 0) { + hash ^= value.hashCode(); + } + j++; } - j++; } // Match metric property does not have component so don't include if (!property.getCategory().equals(Constants.MatchMetrics.CATEGORY)) { @@ -83,11 +85,13 @@ public int hashComponents(int hash, Wrapper wrapper) throws Exception { for (ComponentMetaData component : components) { hash ^= component.hashCode(); int i = 0; - for (FiftyOneAspectPropertyMetaData property : component.getProperties()) { - if (i % 10 == 0) { - hash ^= property.hashCode(); + try (CloseableIterable componentProperties = component.getProperties()) { + for (FiftyOneAspectPropertyMetaData property : componentProperties) { + if (i % 10 == 0) { + hash ^= property.hashCode(); + } + i++; } - i++; } hash ^= component.getDefaultProfile().hashCode(); } @@ -104,11 +108,13 @@ public int hashProfiles(int hash, Wrapper wrapper) throws Exception { hash ^= profile.hashCode(); hash ^= profile.getComponent().hashCode(); int j = 0; - for (ValueMetaData value : profile.getValues()) { - if (j % 10 == 0) { - hash ^= value.hashCode(); + try (CloseableIterable profileValues = profile.getValues()) { + for (ValueMetaData value : profileValues) { + if (j % 10 == 0) { + hash ^= value.hashCode(); + } + j++; } - j++; } } i++; diff --git a/examples/device-detection/console/src/main/java/fiftyone/devicedetection/examples/console/MetadataOnPrem.java b/examples/device-detection/console/src/main/java/fiftyone/devicedetection/examples/console/MetadataOnPrem.java index 51b696286..40c834d74 100644 --- a/examples/device-detection/console/src/main/java/fiftyone/devicedetection/examples/console/MetadataOnPrem.java +++ b/examples/device-detection/console/src/main/java/fiftyone/devicedetection/examples/console/MetadataOnPrem.java @@ -21,6 +21,7 @@ * ********************************************************************* */ package fiftyone.devicedetection.examples.console; +import fiftyone.pipeline.engines.fiftyone.data.FiftyOneAspectPropertyMetaData; import fiftyone.devicedetection.examples.shared.DataFileHelper; import fiftyone.devicedetection.hash.engine.onpremise.flowelements.DeviceDetectionHashEngine; import fiftyone.devicedetection.hash.engine.onpremise.flowelements.DeviceDetectionHashEngineBuilder; @@ -158,59 +159,67 @@ private static void outputComponents(DeviceDetectionHashEngine ddEngine, PrintWr throws Exception { // See outputProfileDetails for why this is a try-with-resource block. try (CloseableIterable components = ddEngine.getComponents()) { - components.forEach(c -> { + for (ComponentMetaData c : components) { output.println("Component - "+ c.getName()); outputProperties(c, output); - }); + } } } - private static void outputProperties(ComponentMetaData component, PrintWriter output) { - if (component.getProperties().iterator().hasNext() == false) { - output.println(" ... no properties"); - return; + private static void outputProperties(ComponentMetaData component, PrintWriter output) + throws Exception { + // The properties of a component, and the values of a property, are + // collections that are closed in the same way. + try (CloseableIterable properties = + component.getProperties()) { + if (properties.iterator().hasNext() == false) { + output.println(" ... no properties"); + return; + } + for (FiftyOneAspectPropertyMetaData property : properties) { + // Output some details about the property. + // If we're outputting to console then we also add some formatting to make it + // more readable. + output.format(" Property - %s [Category: %s] (%s)\n " + + "Description: %s\n", + property.getName(), + property.getCategory(), + property.getType().getName(), + property.getDescription()); + + // Next, output a list of the possible values this property can have. + // Most properties in the Device Metrics category do not have defined + // values so exclude them. + if (property.getCategory().equals("Device Metrics")==false) { + StringBuilder values = new StringBuilder(" Possible " + + "values: "); + try (CloseableIterable propertyValues = + property.getValues()) { + Spliterator spliterator2 = + propertyValues.spliterator(); + StreamSupport.stream(spliterator2, false) + .limit(20) + .forEach(value -> { + // add value + values.append(truncateToNl(value.getName())); + // add description if exists + String d = value.getDescription(); + if (Objects.nonNull(d) && d.isEmpty() == false) { + values.append("(") + .append(d) + .append(")"); + } + values.append(","); + }); + + if (spliterator2.estimateSize() > 20) { + values.append(" + more ..."); + } + } + output.println(values); + } + } } - component.getProperties() - .forEach(property-> { - // Output some details about the property. - // If we're outputting to console then we also add some formatting to make it - // more readable. - output.format(" Property - %s [Category: %s] (%s)\n " + - "Description: %s\n", - property.getName(), - property.getCategory(), - property.getType().getName(), - property.getDescription()); - - // Next, output a list of the possible values this property can have. - // Most properties in the Device Metrics category do not have defined - // values so exclude them. - if (property.getCategory().equals("Device Metrics")==false) { - StringBuilder values = new StringBuilder(" Possible " + - "values: "); - Spliterator spliterator2 = - property.getValues().spliterator(); - StreamSupport.stream(spliterator2, false) - .limit(20) - .forEach(value -> { - // add value - values.append(truncateToNl(value.getName())); - // add description if exists - String d = value.getDescription(); - if (Objects.nonNull(d) && d.isEmpty() == false) { - values.append("(") - .append(d) - .append(")"); - } - values.append(","); - }); - - if (spliterator2.estimateSize() > 20) { - values.append(" + more ..."); - } - output.println(values); - } - }); } diff --git a/examples/ip-intelligence/console/src/main/java/fiftyone/ipintelligence/examples/console/MetadataOnPrem.java b/examples/ip-intelligence/console/src/main/java/fiftyone/ipintelligence/examples/console/MetadataOnPrem.java index 7a96b562f..6e127722d 100644 --- a/examples/ip-intelligence/console/src/main/java/fiftyone/ipintelligence/examples/console/MetadataOnPrem.java +++ b/examples/ip-intelligence/console/src/main/java/fiftyone/ipintelligence/examples/console/MetadataOnPrem.java @@ -21,6 +21,7 @@ * ********************************************************************* */ package fiftyone.ipintelligence.examples.console; +import fiftyone.pipeline.engines.fiftyone.data.FiftyOneAspectPropertyMetaData; import fiftyone.ipintelligence.examples.shared.DataFileHelper; import fiftyone.ipintelligence.engine.onpremise.flowelements.IPIntelligenceOnPremiseEngine; import fiftyone.ipintelligence.engine.onpremise.flowelements.IPIntelligenceOnPremiseEngineBuilder; @@ -175,59 +176,67 @@ private static void outputComponents(IPIntelligenceOnPremiseEngine engine, Print throws Exception { // See outputProfileDetails for why this is a try-with-resource block. try (CloseableIterable components = engine.getComponents()) { - components.forEach(c -> { + for (ComponentMetaData c : components) { output.println("Component - "+ c.getName()); outputProperties(c, output); - }); + } } } - private static void outputProperties(ComponentMetaData component, PrintWriter output) { - if (component.getProperties().iterator().hasNext() == false) { - output.println(" ... no properties"); - return; + private static void outputProperties(ComponentMetaData component, PrintWriter output) + throws Exception { + // The properties of a component, and the values of a property, are + // collections that are closed in the same way. + try (CloseableIterable properties = + component.getProperties()) { + if (properties.iterator().hasNext() == false) { + output.println(" ... no properties"); + return; + } + for (FiftyOneAspectPropertyMetaData property : properties) { + // Output some details about the property. + // If we're outputting to console then we also add some formatting to make it + // more readable. + output.format(" Property - %s [Category: %s] (%s)\n " + + "Description: %s\n", + property.getName(), + property.getCategory(), + property.getType().getName(), + property.getDescription()); + + // Next, output a list of the possible values this property can have. + // Most properties in the Metrics category do not have defined + // values so exclude them. + if (property.getCategory().equals("Metrics")==false) { + StringBuilder values = new StringBuilder(" Possible " + + "values: "); + try (CloseableIterable propertyValues = + property.getValues()) { + Spliterator spliterator2 = + propertyValues.spliterator(); + StreamSupport.stream(spliterator2, false) + .limit(20) + .forEach(value -> { + // add value + values.append(truncateToNl(value.getName())); + // add description if exists + String d = value.getDescription(); + if (Objects.nonNull(d) && d.isEmpty() == false) { + values.append("(") + .append(d) + .append(")"); + } + values.append(","); + }); + + if (spliterator2.estimateSize() > 20) { + values.append(" + more ..."); + } + } + output.println(values); + } + } } - component.getProperties() - .forEach(property-> { - // Output some details about the property. - // If we're outputting to console then we also add some formatting to make it - // more readable. - output.format(" Property - %s [Category: %s] (%s)\n " + - "Description: %s\n", - property.getName(), - property.getCategory(), - property.getType().getName(), - property.getDescription()); - - // Next, output a list of the possible values this property can have. - // Most properties in the Metrics category do not have defined - // values so exclude them. - if (property.getCategory().equals("Metrics")==false) { - StringBuilder values = new StringBuilder(" Possible " + - "values: "); - Spliterator spliterator2 = - property.getValues().spliterator(); - StreamSupport.stream(spliterator2, false) - .limit(20) - .forEach(value -> { - // add value - values.append(truncateToNl(value.getName())); - // add description if exists - String d = value.getDescription(); - if (Objects.nonNull(d) && d.isEmpty() == false) { - values.append("(") - .append(d) - .append(")"); - } - values.append(","); - }); - - if (spliterator2.estimateSize() > 20) { - values.append(" + more ..."); - } - output.println(values); - } - }); } diff --git a/examples/ip-intelligence/console/src/main/java/fiftyone/ipintelligence/examples/console/MetricsOnPrem.java b/examples/ip-intelligence/console/src/main/java/fiftyone/ipintelligence/examples/console/MetricsOnPrem.java index aaf6ddac4..0ce713554 100644 --- a/examples/ip-intelligence/console/src/main/java/fiftyone/ipintelligence/examples/console/MetricsOnPrem.java +++ b/examples/ip-intelligence/console/src/main/java/fiftyone/ipintelligence/examples/console/MetricsOnPrem.java @@ -507,11 +507,13 @@ private static String getValue(ProfileMetaData profile, String name) String result = null; // Every value is closed, not only the first, as each one wraps a // native object of its own. - for (ValueMetaData value : profile.getValues(name)) { - if (result == null) { - result = value.getName(); + try (CloseableIterable values = profile.getValues(name)) { + for (ValueMetaData value : values) { + if (result == null) { + result = value.getName(); + } + value.close(); } - value.close(); } return result; } diff --git a/examples/ip-intelligence/web/getting-started.cloud.mixed/src/main/java/fiftyone/ipintelligence/examples/web/GettingStartedWebCloudMixed.java b/examples/ip-intelligence/web/getting-started.cloud.mixed/src/main/java/fiftyone/ipintelligence/examples/web/GettingStartedWebCloudMixed.java index 4090fc110..977e3b3fa 100644 --- a/examples/ip-intelligence/web/getting-started.cloud.mixed/src/main/java/fiftyone/ipintelligence/examples/web/GettingStartedWebCloudMixed.java +++ b/examples/ip-intelligence/web/getting-started.cloud.mixed/src/main/java/fiftyone/ipintelligence/examples/web/GettingStartedWebCloudMixed.java @@ -118,7 +118,9 @@ protected void processRequest(HttpServletRequest request, HttpServletResponse re // Get the IP address parameter from the request for custom lookup String inputIpAddress = request.getParameter("client-ip"); - // The detection has already been carried out by the PipelineFilter + // The detection has already been carried out by the PipelineFilter, + // which closes the FlowData at the end of the request, so it is not + // closed here. FlowData flowData = flowDataProvider.getFlowData(request); // Determine target IP for display. @@ -149,14 +151,6 @@ protected void processRequest(HttpServletRequest request, HttpServletResponse re String htmlTemplate = loadTemplate(resourceBase + "/WEB-INF/html/index.html"); String processedHtml = substituteTemplateValues(htmlTemplate, deviceData, ipiData, targetIp, flowData); out.println(processedHtml); - } finally { - if (flowData != null) { - try { - flowData.close(); - } catch (Exception e) { - logger.warn("Error closing flow data", e); - } - } } } diff --git a/examples/ip-intelligence/web/getting-started.cloud/src/main/java/fiftyone/ipintelligence/examples/web/GettingStartedWebCloud.java b/examples/ip-intelligence/web/getting-started.cloud/src/main/java/fiftyone/ipintelligence/examples/web/GettingStartedWebCloud.java index dbcb50e7f..2e0fbb58f 100644 --- a/examples/ip-intelligence/web/getting-started.cloud/src/main/java/fiftyone/ipintelligence/examples/web/GettingStartedWebCloud.java +++ b/examples/ip-intelligence/web/getting-started.cloud/src/main/java/fiftyone/ipintelligence/examples/web/GettingStartedWebCloud.java @@ -47,8 +47,9 @@ * The `PipelineFilter` is used to intercept requests and perform IP Intelligence. The results * will be stored in the HttpServletRequest object. * - * The results of the lookup can be accessed by using a FlowDataProvider which - * is responsible for managing the lifecycle of the flowData - do NOT dispose + * The results of the lookup can be accessed by using a FlowDataProvider. The + * PipelineFilter closes the FlowData at the end of the request, so it is not + * closed by the example. * ```{java} * FlowData flowData = flowDataProvider.getFlowData(request); * IPIntelligenceData ipData = flowData.get(IPIntelligenceData.class); @@ -137,7 +138,8 @@ protected void processRequest(HttpServletRequest request, HttpServletResponse re String inputIpAddress = request.getParameter("client-ip"); // The lookup has already been carried out by the PipelineFilter - // which is responsible for the lifecycle of the flowData - do NOT dispose + // which closes the FlowData at the end of the request, so it is not + // closed here. FlowData flowData = flowDataProvider.getFlowData(request); // Determine target IP for display. @@ -165,14 +167,6 @@ protected void processRequest(HttpServletRequest request, HttpServletResponse re String htmlTemplate = loadTemplate(resourceBase + "/WEB-INF/html/index.html"); String processedHtml = substituteTemplateValues(htmlTemplate, ipiData, targetIp, flowData); out.println(processedHtml); - } finally { - if (flowData != null) { - try { - flowData.close(); - } catch (Exception e) { - logger.warn("Error closing flow data", e); - } - } } } diff --git a/examples/ip-intelligence/web/getting-started.mixed/src/main/java/fiftyone/ipintelligence/examples/web/GettingStartedWebMixed.java b/examples/ip-intelligence/web/getting-started.mixed/src/main/java/fiftyone/ipintelligence/examples/web/GettingStartedWebMixed.java index 9969e3262..45330c910 100644 --- a/examples/ip-intelligence/web/getting-started.mixed/src/main/java/fiftyone/ipintelligence/examples/web/GettingStartedWebMixed.java +++ b/examples/ip-intelligence/web/getting-started.mixed/src/main/java/fiftyone/ipintelligence/examples/web/GettingStartedWebMixed.java @@ -140,7 +140,9 @@ protected void processRequest(HttpServletRequest request, HttpServletResponse re // Get the IP address parameter from the request for custom lookup String inputIpAddress = request.getParameter("client-ip"); - // The detection has already been carried out by the PipelineFilter + // The detection has already been carried out by the PipelineFilter, + // which closes the FlowData at the end of the request, so it is not + // closed here. FlowData flowData = flowDataProvider.getFlowData(request); // Determine target IP for display. @@ -171,14 +173,6 @@ protected void processRequest(HttpServletRequest request, HttpServletResponse re String htmlTemplate = loadTemplate(resourceBase + "/WEB-INF/html/index.html"); String processedHtml = substituteTemplateValues(htmlTemplate, deviceData, ipiData, targetIp, flowData); out.println(processedHtml); - } finally { - if (flowData != null) { - try { - flowData.close(); - } catch (Exception e) { - logger.warn("Error closing flow data", e); - } - } } } diff --git a/examples/ip-intelligence/web/getting-started.onprem/src/main/java/fiftyone/ipintelligence/examples/web/GettingStartedWebOnPrem.java b/examples/ip-intelligence/web/getting-started.onprem/src/main/java/fiftyone/ipintelligence/examples/web/GettingStartedWebOnPrem.java index d73bafdf8..ec48e2df9 100644 --- a/examples/ip-intelligence/web/getting-started.onprem/src/main/java/fiftyone/ipintelligence/examples/web/GettingStartedWebOnPrem.java +++ b/examples/ip-intelligence/web/getting-started.onprem/src/main/java/fiftyone/ipintelligence/examples/web/GettingStartedWebOnPrem.java @@ -45,8 +45,9 @@ * The `PipelineFilter` is used to intercept requests and perform IP Intelligence. The results * will be stored in the HttpServletRequest object. * - * The results of the lookup can be accessed by using a FlowDataProvider which - * is responsible for managing the lifecycle of the flowData - do NOT dispose + * The results of the lookup can be accessed by using a FlowDataProvider. The + * PipelineFilter closes the FlowData at the end of the request, so it is not + * closed by the example. * ```{java} * FlowData flowData = flowDataProvider.getFlowData(request); * IPIntelligenceData ipData = flowData.get(IPIntelligenceData.class); @@ -155,7 +156,8 @@ protected void processRequest(HttpServletRequest request, HttpServletResponse re String inputIpAddress = request.getParameter("client-ip"); // The lookup has already been carried out by the PipelineFilter - // which is responsible for the lifecycle of the flowData - do NOT dispose + // which closes the FlowData at the end of the request, so it is not + // closed here. FlowData flowData = flowDataProvider.getFlowData(request); // Determine target IP for display. @@ -183,14 +185,6 @@ protected void processRequest(HttpServletRequest request, HttpServletResponse re String htmlTemplate = loadTemplate(resourceBase + "/WEB-INF/html/index.html"); String processedHtml = substituteTemplateValues(htmlTemplate, ipiData, targetIp, flowData); out.println(processedHtml); - } finally { - if (flowData != null) { - try { - flowData.close(); - } catch (Exception e) { - logger.warn("Error closing flow data", e); - } - } } } diff --git a/ip-intelligence/ip-intelligence.engine.on-premise/src/main/java/fiftyone/ipintelligence/engine/onpremise/data/ComponentMetaDataIPI.java b/ip-intelligence/ip-intelligence.engine.on-premise/src/main/java/fiftyone/ipintelligence/engine/onpremise/data/ComponentMetaDataIPI.java index 7ee5100d7..528e1d737 100644 --- a/ip-intelligence/ip-intelligence.engine.on-premise/src/main/java/fiftyone/ipintelligence/engine/onpremise/data/ComponentMetaDataIPI.java +++ b/ip-intelligence/ip-intelligence.engine.on-premise/src/main/java/fiftyone/ipintelligence/engine/onpremise/data/ComponentMetaDataIPI.java @@ -22,6 +22,7 @@ package fiftyone.ipintelligence.engine.onpremise.data; +import fiftyone.pipeline.engines.fiftyone.data.CloseableIterable; import fiftyone.ipintelligence.engine.onpremise.flowelements.IPIntelligenceOnPremiseEngine; import fiftyone.ipintelligence.engine.onpremise.interop.PropertyIterable; import fiftyone.ipintelligence.engine.onpremise.interop.swig.ComponentMetaDataSwig; @@ -77,7 +78,7 @@ public ProfileMetaData getDefaultProfile() { } @Override - public Iterable getProperties() { + public CloseableIterable getProperties() { return new PropertyIterable( engine, properties, diff --git a/ip-intelligence/ip-intelligence.engine.on-premise/src/main/java/fiftyone/ipintelligence/engine/onpremise/data/PropertyMetaDataIPI.java b/ip-intelligence/ip-intelligence.engine.on-premise/src/main/java/fiftyone/ipintelligence/engine/onpremise/data/PropertyMetaDataIPI.java index 527df5b74..eabb2aecb 100644 --- a/ip-intelligence/ip-intelligence.engine.on-premise/src/main/java/fiftyone/ipintelligence/engine/onpremise/data/PropertyMetaDataIPI.java +++ b/ip-intelligence/ip-intelligence.engine.on-premise/src/main/java/fiftyone/ipintelligence/engine/onpremise/data/PropertyMetaDataIPI.java @@ -22,6 +22,7 @@ package fiftyone.ipintelligence.engine.onpremise.data; +import fiftyone.pipeline.engines.fiftyone.data.CloseableIterable; import fiftyone.ipintelligence.engine.onpremise.flowelements.IPIntelligenceOnPremiseEngine; import fiftyone.ipintelligence.engine.onpremise.interop.Swig; import fiftyone.ipintelligence.engine.onpremise.interop.ValueIterable; @@ -136,7 +137,7 @@ public ComponentMetaData getComponent() { } @Override - public Iterable getValues() { + public CloseableIterable getValues() { return new ValueIterable( engine, engine.getMetaData().getValuesForProperty(source)); diff --git a/ip-intelligence/ip-intelligence.engine.on-premise/src/test/java/fiftyone/ipintelligence/engine/onpremise/data/MetaDataHasherHash.java b/ip-intelligence/ip-intelligence.engine.on-premise/src/test/java/fiftyone/ipintelligence/engine/onpremise/data/MetaDataHasherHash.java index d79b90e23..2bd50f8b6 100644 --- a/ip-intelligence/ip-intelligence.engine.on-premise/src/test/java/fiftyone/ipintelligence/engine/onpremise/data/MetaDataHasherHash.java +++ b/ip-intelligence/ip-intelligence.engine.on-premise/src/test/java/fiftyone/ipintelligence/engine/onpremise/data/MetaDataHasherHash.java @@ -42,11 +42,13 @@ public int hashProperties(int hash, Wrapper wrapper) throws Exception { hash ^= property.hashCode(); int j = 0; - for (ValueMetaData value : property.getValues()) { - if (j % 10 == 0) { - hash ^= value.hashCode(); + try (CloseableIterable propertyValues = property.getValues()) { + for (ValueMetaData value : propertyValues) { + if (j % 10 == 0) { + hash ^= value.hashCode(); + } + j++; } - j++; } if (property.getDefaultValue() != null) { hash ^= property.getDefaultValue().hashCode(); @@ -78,11 +80,13 @@ public int hashComponents(int hash, Wrapper wrapper) throws Exception { for (ComponentMetaData component : components) { hash ^= component.hashCode(); int i = 0; - for (FiftyOneAspectPropertyMetaData property : component.getProperties()) { - if (i % 10 == 0) { - hash ^= property.hashCode(); + try (CloseableIterable componentProperties = component.getProperties()) { + for (FiftyOneAspectPropertyMetaData property : componentProperties) { + if (i % 10 == 0) { + hash ^= property.hashCode(); + } + i++; } - i++; } hash ^= component.getDefaultProfile().hashCode(); } @@ -99,11 +103,13 @@ public int hashProfiles(int hash, Wrapper wrapper) throws Exception { hash ^= profile.hashCode(); hash ^= profile.getComponent().hashCode(); int j = 0; - for (ValueMetaData value : profile.getValues()) { - if (j % 10 == 0) { - hash ^= value.hashCode(); + try (CloseableIterable profileValues = profile.getValues()) { + for (ValueMetaData value : profileValues) { + if (j % 10 == 0) { + hash ^= value.hashCode(); + } + j++; } - j++; } } i++; diff --git a/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/data/ComponentMetaData.java b/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/data/ComponentMetaData.java index 2c3b0ac4f..830f8d8ef 100644 --- a/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/data/ComponentMetaData.java +++ b/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/data/ComponentMetaData.java @@ -50,9 +50,10 @@ public interface ComponentMetaData extends Closeable { /** * List of the properties which come under the umbrella of this component. + * The caller closes the collection, which can hold native memory. * @return list of properties */ - Iterable getProperties(); + CloseableIterable getProperties(); /** * Get the property from the component which has the name provided. Null is diff --git a/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/data/FiftyOneAspectPropertyMetaData.java b/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/data/FiftyOneAspectPropertyMetaData.java index 2d6357f34..da0c127dc 100644 --- a/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/data/FiftyOneAspectPropertyMetaData.java +++ b/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/data/FiftyOneAspectPropertyMetaData.java @@ -91,9 +91,10 @@ public interface FiftyOneAspectPropertyMetaData /** * Get the values which relate to the property i.e. the values which can be * returned for this property. + * The caller closes the collection, which can hold the engine's data set. * @return values which relate to the property */ - Iterable getValues(); + CloseableIterable getValues(); /** * Get the default value for the property. diff --git a/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/data/FiftyOneAspectPropertyMetaDataDefault.java b/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/data/FiftyOneAspectPropertyMetaDataDefault.java index cd80a737f..a460d8443 100644 --- a/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/data/FiftyOneAspectPropertyMetaDataDefault.java +++ b/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/data/FiftyOneAspectPropertyMetaDataDefault.java @@ -130,8 +130,8 @@ public ComponentMetaData getComponent() { } @Override - public Iterable getValues() { - return () -> { + public CloseableIterable getValues() { + return new CloseableIterableDefault(() -> { Iterator innerValues = values.iterator(); return new Iterator() { @@ -145,7 +145,7 @@ public ValueMetaData next() { return innerValues.next(); } }; - }; + }); } @Override diff --git a/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/data/ProfileMetaData.java b/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/data/ProfileMetaData.java index bf0ddfabc..94ea3d982 100644 --- a/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/data/ProfileMetaData.java +++ b/pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/data/ProfileMetaData.java @@ -39,16 +39,18 @@ public interface ProfileMetaData extends Closeable { /** * Get the values which are defined in the profile (for some Engines * multiple profiles are required to build a full set of results). + * The caller closes the collection, which can hold the engine's data set. * @return values defined in the profile */ - Iterable getValues(); + CloseableIterable getValues(); /** * Gets the values associated with the profile and the property name. * @param propertyName to get the values for + * The caller closes the collection, which can hold native memory. * @return values matching the property */ - Iterable getValues(String propertyName); + CloseableIterable getValues(String propertyName); /** * If there is a value for the profile with the property name and value then