diff --git a/ci/pmd-resource-ruleset.xml b/ci/pmd-resource-ruleset.xml new file mode 100644 index 000000000..5f99fad8e --- /dev/null +++ b/ci/pmd-resource-ruleset.xml @@ -0,0 +1,99 @@ + + + + + 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 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 + 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 + + + + + + + + + + + + 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/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/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/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/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/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/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..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 @@ -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) { @@ -42,11 +43,13 @@ public int hashProperties(int hash, Wrapper wrapper) { 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)) { @@ -62,50 +65,60 @@ 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; + try (CloseableIterable componentProperties = component.getProperties()) { + for (FiftyOneAspectPropertyMetaData property : componentProperties) { + 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; + try (CloseableIterable profileValues = profile.getValues()) { + for (ValueMetaData value : profileValues) { + 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 0b58bc098..14b30de62 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_PATH)) { 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/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/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/device-detection/pom.xml b/device-detection/pom.xml index ce56dd201..b255d1e53 100644 --- a/device-detection/pom.xml +++ b/device-detection/pom.xml @@ -56,6 +56,7 @@ 3.7.1 6.5.3 0.7.0 + 3.26.0 true false @@ -308,6 +309,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/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 5b6939705..350172405 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,11 +21,13 @@ * ********************************************************************* */ 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; 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; @@ -134,71 +136,92 @@ 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()) { + 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/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 621ef0761..50d44c364 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 @@ -137,9 +137,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/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/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/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 6c66b486f..347c35b60 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/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/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 6e3fa2e8a..9887afc57 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/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 5b7331767..b6addc8d7 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/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/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/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/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/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..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 @@ -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) { @@ -41,11 +42,13 @@ public int hashProperties(int hash, Wrapper wrapper) { 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(); @@ -57,50 +60,60 @@ 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; + try (CloseableIterable componentProperties = component.getProperties()) { + for (FiftyOneAspectPropertyMetaData property : componentProperties) { + 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; + try (CloseableIterable profileValues = profile.getValues()) { + for (ValueMetaData value : profileValues) { + 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/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(); } 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/ip-intelligence/pom.xml b/ip-intelligence/pom.xml index 16a23b1a3..1355b5932 100644 --- a/ip-intelligence/pom.xml +++ b/ip-intelligence/pom.xml @@ -179,6 +179,8 @@ ${maven.multiModuleProjectDirectory}/ci/pmd-ruleset.xml + + ${maven.multiModuleProjectDirectory}/ci/pmd-resource-ruleset.xml true 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/location/pom.xml b/location/pom.xml index 1939c6520..930a853f5 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/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/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/FlowDataTests.java b/pipeline/pipeline.core/src/test/java/fiftyone/pipeline/core/flowelements/FlowDataTests.java index 3e04cf231..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,14 +318,15 @@ public void FlowData_GetNotPresent_Element() { } @Test(expected = IllegalStateException.class) - public void FlowData_PipelineDisposed() { + public void FlowData_PipelineDisposed() throws Exception { 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..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. @@ -416,26 +446,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 +474,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.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)); + } } } } 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; + } +} 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/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 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(); + } + } } 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 a94604104..2178a5f66 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 @@ -72,12 +72,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(); @@ -164,18 +164,19 @@ private List renderWithHosts(String... hosts) throws Exception { new JavaScriptBuilderElementBuilder(factory) .setEndpoint("/json") .build(); - Pipeline pipeline = new PipelineBuilder(factory) + List scripts = new ArrayList<>(); + try (Pipeline pipeline = new PipelineBuilder(factory) .addFlowElement(new SequenceElementBuilder(factory).build()) .addFlowElement(new JsonBuilderElementBuilder(factory).build()) .addFlowElement(jsElement) - .build(); - List scripts = new ArrayList<>(); - for (String host : hosts) { - try (FlowData flowData = pipeline.createFlowData()) { - flowData.addEvidence(Constants.EVIDENCE_HOST_KEY, host); - flowData.process(); - scripts.add( - flowData.getFromElement(jsElement).getJavaScript()); + .build()) { + for (String host : hosts) { + try (FlowData flowData = pipeline.createFlowData()) { + flowData.addEvidence(Constants.EVIDENCE_HOST_KEY, host); + flowData.process(); + scripts.add( + flowData.getFromElement(jsElement).getJavaScript()); + } } } return scripts; 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(); 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(); + } } diff --git a/pipeline/pom.xml b/pipeline/pom.xml index a3c3b5afc..abcd6d8f6 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 + + + +