diff --git a/cxx b/cxx index c6d4e4d90..7c4ac436e 160000 --- a/cxx +++ b/cxx @@ -1 +1 @@ -Subproject commit c6d4e4d907e8da83845c095e80374280f8db29c5 +Subproject commit 7c4ac436e47cd15beed304cad2b8151d50a1efd1 diff --git a/device-detection/device-detection.hash.engine.on-premise/src/main/java/fiftyone/devicedetection/hash/engine/onpremise/flowelements/DeviceDataHashDefault.java b/device-detection/device-detection.hash.engine.on-premise/src/main/java/fiftyone/devicedetection/hash/engine/onpremise/flowelements/DeviceDataHashDefault.java index 3dad6384f..691761125 100644 --- a/device-detection/device-detection.hash.engine.on-premise/src/main/java/fiftyone/devicedetection/hash/engine/onpremise/flowelements/DeviceDataHashDefault.java +++ b/device-detection/device-detection.hash.engine.on-premise/src/main/java/fiftyone/devicedetection/hash/engine/onpremise/flowelements/DeviceDataHashDefault.java @@ -94,7 +94,7 @@ public class DeviceDataHashDefault /** * Get the required property index for a property name, or null when the - * engine was not built with that name, so the read falls back to the name. + * engine has no index for that name, so the read falls back to the name. * @param propertyName name of the property * @return required property index, or null */ diff --git a/device-detection/device-detection.hash.engine.on-premise/src/main/java/fiftyone/devicedetection/hash/engine/onpremise/flowelements/DeviceDetectionHashEngine.java b/device-detection/device-detection.hash.engine.on-premise/src/main/java/fiftyone/devicedetection/hash/engine/onpremise/flowelements/DeviceDetectionHashEngine.java index 2da75263e..0b5c93fb1 100644 --- a/device-detection/device-detection.hash.engine.on-premise/src/main/java/fiftyone/devicedetection/hash/engine/onpremise/flowelements/DeviceDetectionHashEngine.java +++ b/device-detection/device-detection.hash.engine.on-premise/src/main/java/fiftyone/devicedetection/hash/engine/onpremise/flowelements/DeviceDetectionHashEngine.java @@ -68,10 +68,20 @@ public class DeviceDetectionHashEngine * list, the first where a name is given more than once, as the native * engine gives it. Made from the names given at build and never from the * data set, so an index keeps naming the same property after a reload - * from a data file whose property order differs. Empty when the engine - * was built with no names, in which case reads go by name. + * from a data file whose property order differs. When the engine was + * built with no names it is made instead from the names the native engine + * reports after the first data file loads, which are the properties of + * that file sorted by name and which a reload does not change. */ - private final Map propertyIndexes; + private volatile Map propertyIndexes; + + /** + * True once {@link #propertyIndexes} holds the names the indexes are + * fixed on, which is at construction when the engine was built with + * names and after the first data file loads when it was not. A reload + * never makes the map again. + */ + private volatile boolean propertyIndexesFixed; /** * Construct a new instance of the {@link DeviceDetectionHashEngine}. @@ -100,6 +110,7 @@ public class DeviceDetectionHashEngine this.config = config; this.propertiesConfigSwig = properties; this.propertyIndexes = indexesOf(propertyNames); + this.propertyIndexesFixed = this.propertyIndexes.isEmpty() == false; addDataFile(dataFile); } @@ -130,7 +141,9 @@ public MetaDataSwig getMetaData() { /** * Get the required property index for a property name, or null when the - * engine was not built with that name and the read has to go by name. + * index list has no such name and the read has to go by name. That covers + * a name the engine was not built with, and a property only a data file + * loaded after the first has. * @param propertyName name of the property * @return required property index, or null */ @@ -138,6 +151,22 @@ Integer getPropertyIndex(String propertyName) { return propertyIndexes.get(propertyName); } + /** + * Fix the indexes on the names the native engine reports, which it does + * only for an engine built with no names, and only after its first data + * file has loaded. Those names are the properties of that file sorted by + * name, and the native engine keeps them and their order through every + * later reload, so the map is made once and a reload never changes it. + */ + private void fixPropertyIndexes() { + if (propertyIndexesFixed == false) { + try (VectorStringSwig names = engine.getRequiredProperties()) { + propertyIndexes = indexesOf(new ArrayList<>(names)); + } + propertyIndexesFixed = true; + } + } + /** * Map each name to its place in the list, keeping the first place of a * name given more than once in any letter case. @@ -395,6 +424,7 @@ protected void unmanagedResourcesCleanup() { } private void setEngineMetaData() { + fixPropertyIndexes(); evidenceKeys = getKeysFromEngine(engine); evidenceKeyFilter = new EvidenceKeyFilterWhitelist( evidenceKeys, diff --git a/device-detection/device-detection.hash.engine.on-premise/src/test/java/fiftyone/devicedetection/hash/engine/onpremise/flowelements/PropertyIndexTests.java b/device-detection/device-detection.hash.engine.on-premise/src/test/java/fiftyone/devicedetection/hash/engine/onpremise/flowelements/PropertyIndexTests.java index 4c222ce41..0302a3a95 100644 --- a/device-detection/device-detection.hash.engine.on-premise/src/test/java/fiftyone/devicedetection/hash/engine/onpremise/flowelements/PropertyIndexTests.java +++ b/device-detection/device-detection.hash.engine.on-premise/src/test/java/fiftyone/devicedetection/hash/engine/onpremise/flowelements/PropertyIndexTests.java @@ -23,6 +23,7 @@ package fiftyone.devicedetection.hash.engine.onpremise.flowelements; import fiftyone.devicedetection.hash.engine.onpremise.data.DeviceDataHash; +import fiftyone.devicedetection.hash.engine.onpremise.data.PropertyMetaDataHash; import fiftyone.devicedetection.hash.engine.onpremise.interop.Swig; import fiftyone.devicedetection.hash.engine.onpremise.interop.swig.BoolValueSwig; import fiftyone.devicedetection.hash.engine.onpremise.interop.swig.ConfigHashSwig; @@ -46,7 +47,11 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; import java.util.List; +import java.util.Map; +import java.util.Set; import static fiftyone.devicedetection.shared.testhelpers.FileUtils.LITE_HASH_DATA_FILE_PATH; import static fiftyone.devicedetection.shared.testhelpers.FileUtils.TAC_HASH_DATA_FILE_NAME; @@ -179,14 +184,108 @@ public void PropertyIndex_UnknownNameHasNoIndex() throws Exception { } /** - * An engine built with no names has no indexes, so every read goes by - * name. + * An engine built with no names has an index for every property of the + * first data file it loads, no two properties share an index, and the + * native results name that property at that index. */ @Test - public void PropertyIndex_NoNamesReadsByName() throws Exception { + public void PropertyIndex_NoNamesFromFirstFile() throws Exception { File file = dataFile(LITE_HASH_DATA_FILE_PATH); - try (DeviceDetectionHashEngine engine = build(file, null)) { - assertNull(engine.getPropertyIndex("IsMobile")); + List names = dataSetNames(file); + try (DeviceDetectionHashEngine engine = build(file, null); + NativeEngine nativeEngine = new NativeEngine(file, null); + ResultsHashSwig results = nativeEngine.engine.process(MOBILE)) { + Set seen = new HashSet<>(); + for (String name : names) { + Integer index = engine.getPropertyIndex(name); + assertNotNull("Expected an index for '" + name + "'.", index); + assertTrue( + "Index " + index + " given to more than one name.", + seen.add(index)); + assertEquals( + "Expected the native results to name '" + name + + "' at index " + index + ".", + name, + results.getPropertyName(index)); + } + } + } + + /** + * An engine built with no names keeps its first data file's indexes + * through a reload from a data file whose properties are a superset in a + * different order. Each property both files have then reads what an + * engine built on the new file reads, and a property only the new file + * has gets no index. + */ + @Test + public void PropertyIndex_NoNamesSurvivesReload() throws Exception { + File lite = dataFile(LITE_HASH_DATA_FILE_PATH); + File bigger = dataFile(TAC_HASH_DATA_FILE_NAME); + List names = dataSetNames(lite); + names.retainAll(dataSetNames(bigger)); + String added = nameOnlyIn(bigger, lite); + + DeviceDetectionHashEngine engine = build(lite, null); + DeviceDetectionHashEngine fresh = build(bigger, null); + try (Pipeline pipeline = new PipelineBuilder(loggerFactory) + .addFlowElement(engine) + .build(); + Pipeline freshPipeline = new PipelineBuilder(loggerFactory) + .addFlowElement(fresh) + .build()) { + + Map before = new HashMap<>(); + for (String name : names) { + Integer index = engine.getPropertyIndex(name); + assertNotNull("Expected an index for '" + name + "'.", index); + before.put(name, index); + } + + engine.refreshData( + engine.getDataFiles().get(0).getIdentifier(), + Files.readAllBytes(bigger.toPath())); + + for (String name : names) { + assertEquals( + "Index of '" + name + "' moved on reload.", + before.get(name), + engine.getPropertyIndex(name)); + } + if (added != null) { + assertNull( + "Expected no index for '" + added + "', which only " + + "the new file has.", + engine.getPropertyIndex(added)); + } + + try (FlowData data = pipeline.createFlowData(); + FlowData expected = freshPipeline.createFlowData()) { + data.addEvidence("header.user-agent", MOBILE); + data.process(); + expected.addEvidence("header.user-agent", MOBILE); + expected.process(); + DeviceData device = data.get(DeviceData.class); + DeviceData want = expected.get(DeviceData.class); + for (String name : names) { + AspectPropertyValue got = + (AspectPropertyValue) device.get(name); + AspectPropertyValue wanted = + (AspectPropertyValue) want.get(name); + assertEquals( + "Expected '" + name + "' to have a value as a " + + "fresh engine gives it.", + wanted.hasValue(), + got.hasValue()); + if (wanted.hasValue()) { + assertEquals( + "Expected '" + name + "' to read as a fresh " + + "engine reads it.", + wanted.getValue(), + got.getValue()); + } + } + } } } @@ -344,6 +443,25 @@ private static String nameOnlyIn(File has, File lacks) throws Exception { return null; } + /** + * The names of the properties the data file holds, sorted, leaving out + * the match metrics the engine adds, which are not in the data file and + * so have no index. + */ + private static List dataSetNames(File file) throws Exception { + List names = new ArrayList<>(); + try (DeviceDetectionHashEngine engine = build(file, null)) { + for (FiftyOneAspectPropertyMetaData property : + engine.getProperties()) { + if (property instanceof PropertyMetaDataHash) { + names.add(property.getName()); + } + } + } + Collections.sort(names); + return names; + } + private static List propertyNames(File file) throws Exception { List names = new ArrayList<>(); try (DeviceDetectionHashEngine engine = build(file, null)) { @@ -361,19 +479,38 @@ private static List propertyNames(File file) throws Exception { * index from the same native results, and {@link #READS} times through * the typed accessor, which is the whole path a caller takes, and prints * the median nanoseconds of one read over {@link #ROUNDS} rounds after a - * warm-up. Every read must give the same value. The times are printed - * rather than asserted, because runners vary. + * warm-up, for an engine built with {@link #NAMES}. Every read must give + * the same value. The times are printed rather than asserted, because + * runners vary. */ @Test public void PropertyIndex_ReadByIndexAgainstByName() throws Exception { + measureReads(NAMES); + } + + /** + * The same measurement for an engine built with no names, which reads by + * the index of the property in its first data file. + */ + @Test + public void PropertyIndex_NoNamesReadByIndexAgainstByName() + throws Exception { + measureReads(null); + } + + private static void measureReads(List built) throws Exception { File file = dataFile(LITE_HASH_DATA_FILE_PATH); - DeviceDetectionHashEngine engine = build(file, NAMES); - int index = engine.getPropertyIndex("IsMobile"); + DeviceDetectionHashEngine engine = build(file, built); + Integer found = engine.getPropertyIndex("IsMobile"); + assertNotNull("Expected an index for 'IsMobile'.", found); + int index = found; ConfigHashSwig config = new ConfigHashSwig(); config.setHighPerformance(); RequiredPropertiesConfigSwig required; try (VectorStringSwig names = new VectorStringSwig()) { - names.addAll(NAMES); + if (built != null) { + names.addAll(built); + } required = new RequiredPropertiesConfigSwig(names); } EngineHashSwig nativeEngine = @@ -425,8 +562,9 @@ public void PropertyIndex_ReadByIndexAgainstByName() throws Exception { } assertEquals("Every read gives the same value", 0, different); System.out.println(String.format( - "READ COST IsMobile reads=%d rounds=%d byNameNs=%s " + - "byIndexNs=%s typedNs=%s java=%s os=%s", + "READ COST IsMobile names=%s reads=%d rounds=%d " + + "byNameNs=%s byIndexNs=%s typedNs=%s java=%s os=%s", + built == null ? "none" : "given", READS, ROUNDS, spread(byName), spread(byIndex), spread(typed), System.getProperty("java.version"), System.getProperty("os.name"))); @@ -438,6 +576,37 @@ READS, ROUNDS, spread(byName), spread(byIndex), } } + /** + * A native engine on its own, so a test can read the native results + * directly. Built with no names when the list is null. + */ + private static class NativeEngine implements AutoCloseable { + private final ConfigHashSwig config = new ConfigHashSwig(); + private final RequiredPropertiesConfigSwig required; + private final EngineHashSwig engine; + + NativeEngine(File file, List names) { + config.setHighPerformance(); + RequiredPropertiesConfigSwig made; + try (VectorStringSwig vector = new VectorStringSwig()) { + if (names != null) { + vector.addAll(names); + } + made = new RequiredPropertiesConfigSwig(vector); + } + required = made; + engine = new EngineHashSwig( + file.getAbsolutePath(), config, required); + } + + @Override + public void close() { + engine.delete(); + required.delete(); + config.delete(); + } + } + /** * Reads IsMobile by name through the overload that takes the name's * bytes, which is how a read goes when the engine has no index for it. diff --git a/ip-intelligence/ip-intelligence.engine.on-premise/src/main/java/fiftyone/ipintelligence/engine/onpremise/flowelements/IPIntelligenceDataHashDefault.java b/ip-intelligence/ip-intelligence.engine.on-premise/src/main/java/fiftyone/ipintelligence/engine/onpremise/flowelements/IPIntelligenceDataHashDefault.java index a93045407..6d1300fd8 100644 --- a/ip-intelligence/ip-intelligence.engine.on-premise/src/main/java/fiftyone/ipintelligence/engine/onpremise/flowelements/IPIntelligenceDataHashDefault.java +++ b/ip-intelligence/ip-intelligence.engine.on-premise/src/main/java/fiftyone/ipintelligence/engine/onpremise/flowelements/IPIntelligenceDataHashDefault.java @@ -94,7 +94,7 @@ public class IPIntelligenceDataHashDefault /** * Get the required property index for a property name, or null when the - * engine was not built with that name, so the read falls back to the name. + * engine has no index for that name, so the read falls back to the name. * @param propertyName name of the property * @return required property index, or null */ diff --git a/ip-intelligence/ip-intelligence.engine.on-premise/src/main/java/fiftyone/ipintelligence/engine/onpremise/flowelements/IPIntelligenceOnPremiseEngine.java b/ip-intelligence/ip-intelligence.engine.on-premise/src/main/java/fiftyone/ipintelligence/engine/onpremise/flowelements/IPIntelligenceOnPremiseEngine.java index 053521d41..6955ef9e9 100644 --- a/ip-intelligence/ip-intelligence.engine.on-premise/src/main/java/fiftyone/ipintelligence/engine/onpremise/flowelements/IPIntelligenceOnPremiseEngine.java +++ b/ip-intelligence/ip-intelligence.engine.on-premise/src/main/java/fiftyone/ipintelligence/engine/onpremise/flowelements/IPIntelligenceOnPremiseEngine.java @@ -68,10 +68,20 @@ public class IPIntelligenceOnPremiseEngine * list, the first where a name is given more than once, as the native * engine gives it. Made from the names given at build and never from the * data set, so an index keeps naming the same property after a reload - * from a data file whose property order differs. Empty when the engine - * was built with no names, in which case reads go by name. + * from a data file whose property order differs. When the engine was + * built with no names it is made instead from the names the native engine + * reports after the first data file loads, which are the properties of + * that file sorted by name and which a reload does not change. */ - private final Map propertyIndexes; + private volatile Map propertyIndexes; + + /** + * True once {@link #propertyIndexes} holds the names the indexes are + * fixed on, which is at construction when the engine was built with + * names and after the first data file loads when it was not. A reload + * never makes the map again. + */ + private volatile boolean propertyIndexesFixed; /** * Construct a new instance of the {@link IPIntelligenceOnPremiseEngine}. @@ -100,6 +110,7 @@ public class IPIntelligenceOnPremiseEngine this.config = config; this.propertiesConfigSwig = properties; this.propertyIndexes = indexesOf(propertyNames); + this.propertyIndexesFixed = this.propertyIndexes.isEmpty() == false; addDataFile(dataFile); } @@ -130,7 +141,9 @@ public MetaDataSwig getMetaData() { /** * Get the required property index for a property name, or null when the - * engine was not built with that name and the read has to go by name. + * index list has no such name and the read has to go by name. That covers + * a name the engine was not built with, and a property only a data file + * loaded after the first has. * @param propertyName name of the property * @return required property index, or null */ @@ -138,6 +151,22 @@ Integer getPropertyIndex(String propertyName) { return propertyIndexes.get(propertyName); } + /** + * Fix the indexes on the names the native engine reports, which it does + * only for an engine built with no names, and only after its first data + * file has loaded. Those names are the properties of that file sorted by + * name, and the native engine keeps them and their order through every + * later reload, so the map is made once and a reload never changes it. + */ + private void fixPropertyIndexes() { + if (propertyIndexesFixed == false) { + try (VectorStringSwig names = engine.getRequiredProperties()) { + propertyIndexes = indexesOf(new ArrayList<>(names)); + } + propertyIndexesFixed = true; + } + } + /** * Map each name to its place in the list, keeping the first place of a * name given more than once in any letter case. @@ -385,6 +414,7 @@ protected void unmanagedResourcesCleanup() { } private void setEngineMetaData() { + fixPropertyIndexes(); evidenceKeys = getKeysFromEngine(engine); evidenceKeyFilter = new EvidenceKeyFilterWhitelist( evidenceKeys, diff --git a/ip-intelligence/ip-intelligence.engine.on-premise/src/main/java/fiftyone/ipintelligence/engine/onpremise/interop/swig/ConfigIpiSwig.java b/ip-intelligence/ip-intelligence.engine.on-premise/src/main/java/fiftyone/ipintelligence/engine/onpremise/interop/swig/ConfigIpiSwig.java index e86fa537f..522943998 100644 --- a/ip-intelligence/ip-intelligence.engine.on-premise/src/main/java/fiftyone/ipintelligence/engine/onpremise/interop/swig/ConfigIpiSwig.java +++ b/ip-intelligence/ip-intelligence.engine.on-premise/src/main/java/fiftyone/ipintelligence/engine/onpremise/interop/swig/ConfigIpiSwig.java @@ -81,7 +81,7 @@ public CollectionConfigSwig getStrings() { } public CollectionConfigSwig getComponents() { - return new CollectionConfigSwig(IpIntelligenceOnPremiseEngineModuleJNI.ConfigIpiSwig_getComponents(swigCPtr, this), true); + return new CollectionConfigSwig(IpIntelligenceOnPremiseEngineModuleJNI.ConfigIpiSwig_getComponents(swigCPtr, this), false); } public CollectionConfigSwig getMaps() { @@ -89,15 +89,15 @@ public CollectionConfigSwig getMaps() { } public CollectionConfigSwig getProperties() { - return new CollectionConfigSwig(IpIntelligenceOnPremiseEngineModuleJNI.ConfigIpiSwig_getProperties(swigCPtr, this), true); + return new CollectionConfigSwig(IpIntelligenceOnPremiseEngineModuleJNI.ConfigIpiSwig_getProperties(swigCPtr, this), false); } public CollectionConfigSwig getValues() { - return new CollectionConfigSwig(IpIntelligenceOnPremiseEngineModuleJNI.ConfigIpiSwig_getValues(swigCPtr, this), true); + return new CollectionConfigSwig(IpIntelligenceOnPremiseEngineModuleJNI.ConfigIpiSwig_getValues(swigCPtr, this), false); } public CollectionConfigSwig getProfiles() { - return new CollectionConfigSwig(IpIntelligenceOnPremiseEngineModuleJNI.ConfigIpiSwig_getProfiles(swigCPtr, this), true); + return new CollectionConfigSwig(IpIntelligenceOnPremiseEngineModuleJNI.ConfigIpiSwig_getProfiles(swigCPtr, this), false); } public CollectionConfigSwig getGraphs() { diff --git a/ip-intelligence/ip-intelligence.engine.on-premise/src/test/java/fiftyone/ipintelligence/engine/onpremise/flowelements/BuilderTests.java b/ip-intelligence/ip-intelligence.engine.on-premise/src/test/java/fiftyone/ipintelligence/engine/onpremise/flowelements/BuilderTests.java index 30b9e0025..405a4a227 100644 --- a/ip-intelligence/ip-intelligence.engine.on-premise/src/test/java/fiftyone/ipintelligence/engine/onpremise/flowelements/BuilderTests.java +++ b/ip-intelligence/ip-intelligence.engine.on-premise/src/test/java/fiftyone/ipintelligence/engine/onpremise/flowelements/BuilderTests.java @@ -92,6 +92,43 @@ public void Builder_MaxPerformance_ReplacesBalanced() throws Exception { collections(PerformanceProfiles.Balanced), inMemory)); } + /** + * Check that every collection the configuration hands out is borrowed. + * The collections belong to the configuration, so a proxy that owned + * one would free it a second time when the proxy is deleted. + */ + @Test + public void Config_CollectionsAreBorrowed() throws Exception { + ConfigIpiSwig config = new ConfigIpiSwig(); + try { + config.setBalanced(); + long[] before = collections(config); + CollectionConfigSwig[] all = { + config.getStrings(), + config.getComponents(), + config.getMaps(), + config.getProperties(), + config.getValues(), + config.getProfiles(), + config.getGraphs(), + config.getProfileGroups(), + config.getProfileOffsets(), + config.getPropertyTypes(), + config.getGraph() + }; + Field own = CollectionConfigSwig.class + .getDeclaredField("swigCMemOwn"); + own.setAccessible(true); + for (CollectionConfigSwig collection : all) { + assertFalse(own.getBoolean(collection)); + collection.delete(); + } + assertArrayEquals(before, collections(config)); + } finally { + config.delete(); + } + } + /** * The native configuration the builder hands to the engine it builds. */ @@ -132,17 +169,17 @@ private static long[] collections(PerformanceProfiles profile) { */ private static long[] collections(ConfigIpiSwig config) { CollectionConfigSwig[] all = { - borrowed(config.getStrings()), - borrowed(config.getComponents()), - borrowed(config.getMaps()), - borrowed(config.getProperties()), - borrowed(config.getValues()), - borrowed(config.getProfiles()), - borrowed(config.getGraphs()), - borrowed(config.getProfileGroups()), - borrowed(config.getProfileOffsets()), - borrowed(config.getPropertyTypes()), - borrowed(config.getGraph()) + config.getStrings(), + config.getComponents(), + config.getMaps(), + config.getProperties(), + config.getValues(), + config.getProfiles(), + config.getGraphs(), + config.getProfileGroups(), + config.getProfileOffsets(), + config.getPropertyTypes(), + config.getGraph() }; long[] settings = new long[all.length * 2]; for (int i = 0; i < all.length; i++) { @@ -151,20 +188,4 @@ private static long[] collections(ConfigIpiSwig config) { } return settings; } - - /** - * The generated wrappers mark four ConfigIpiSwig getters as owned until - * they are regenerated with the C library's corrected MetaData.i. - */ - private static CollectionConfigSwig borrowed(CollectionConfigSwig proxy) { - try { - Field own = CollectionConfigSwig.class - .getDeclaredField("swigCMemOwn"); - own.setAccessible(true); - own.setBoolean(proxy, false); - return proxy; - } catch (ReflectiveOperationException e) { - throw new IllegalStateException(e); - } - } } diff --git a/ip-intelligence/ip-intelligence.engine.on-premise/src/test/java/fiftyone/ipintelligence/engine/onpremise/flowelements/PropertyIndexTests.java b/ip-intelligence/ip-intelligence.engine.on-premise/src/test/java/fiftyone/ipintelligence/engine/onpremise/flowelements/PropertyIndexTests.java index 1aac78acd..9ae3290c2 100644 --- a/ip-intelligence/ip-intelligence.engine.on-premise/src/test/java/fiftyone/ipintelligence/engine/onpremise/flowelements/PropertyIndexTests.java +++ b/ip-intelligence/ip-intelligence.engine.on-premise/src/test/java/fiftyone/ipintelligence/engine/onpremise/flowelements/PropertyIndexTests.java @@ -22,20 +22,32 @@ package fiftyone.ipintelligence.engine.onpremise.flowelements; +import fiftyone.ipintelligence.engine.onpremise.interop.swig.ConfigIpiSwig; +import fiftyone.ipintelligence.engine.onpremise.interop.swig.EngineIpiSwig; +import fiftyone.ipintelligence.engine.onpremise.interop.swig.RequiredPropertiesConfigSwig; +import fiftyone.ipintelligence.engine.onpremise.interop.swig.ResultsIpiSwig; +import fiftyone.ipintelligence.engine.onpremise.interop.swig.VectorStringSwig; import fiftyone.pipeline.engines.Constants; +import fiftyone.pipeline.engines.fiftyone.data.FiftyOneAspectPropertyMetaData; import org.junit.Test; import org.slf4j.ILoggerFactory; import org.slf4j.LoggerFactory; import java.io.File; +import java.util.ArrayList; import java.util.Arrays; +import java.util.HashMap; +import java.util.HashSet; import java.util.List; +import java.util.Map; +import java.util.Set; import static fiftyone.ipintelligence.shared.testhelpers.FileUtils.LITE_IPI_DATA_FILE_NAME; import static fiftyone.pipeline.util.FileFinder.getFilePath; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; import static org.junit.Assume.assumeTrue; /** @@ -59,6 +71,8 @@ public class PropertyIndexTests { "RegisteredName", "Latitude"); + private static final String ADDRESS = "185.28.167.77"; + private static File dataFile(String name) { try { return getFilePath(name); @@ -150,14 +164,95 @@ public void PropertyIndex_UnknownNameHasNoIndex() throws Exception { } /** - * An engine built with no names has no indexes, so every read goes by - * name. + * An engine built with no names has an index for every property of the + * first data file it loads, no two properties share an index, and the + * native results name that property at that index. */ @Test - public void PropertyIndex_NoNamesReadsByName() throws Exception { + public void PropertyIndex_NoNamesFromFirstFile() throws Exception { + File file = dataFile(LITE_IPI_DATA_FILE_NAME); + try (IPIntelligenceOnPremiseEngine engine = build(file, null); + NativeEngine nativeEngine = new NativeEngine(file); + ResultsIpiSwig results = nativeEngine.engine.process(ADDRESS)) { + List names = propertyNames(engine); + assertTrue("Expected the data file to have properties.", + names.isEmpty() == false); + Set seen = new HashSet<>(); + for (String name : names) { + Integer index = engine.getPropertyIndex(name); + assertNotNull("Expected an index for '" + name + "'.", index); + assertTrue( + "Index " + index + " given to more than one name.", + seen.add(index)); + assertEquals( + "Expected the native results to name '" + name + + "' at index " + index + ".", + name, + results.getPropertyName(index)); + } + } + } + + /** + * An engine built with no names keeps the indexes it took from its first + * data file through a reload, because they are made once and not from + * the reloaded data set. + */ + @Test + public void PropertyIndex_NoNamesSurvivesReload() throws Exception { File file = dataFile(LITE_IPI_DATA_FILE_NAME); try (IPIntelligenceOnPremiseEngine engine = build(file, null)) { - assertNull(engine.getPropertyIndex("RegisteredName")); + Map before = new HashMap<>(); + for (String name : propertyNames(engine)) { + Integer index = engine.getPropertyIndex(name); + assertNotNull("Expected an index for '" + name + "'.", index); + before.put(name, index); + } + engine.refreshData(engine.getDataFiles().get(0).getIdentifier()); + for (Map.Entry entry : before.entrySet()) { + assertEquals( + "Index of '" + entry.getKey() + "' moved on reload.", + entry.getValue(), + engine.getPropertyIndex(entry.getKey())); + } + } + } + + private static List propertyNames( + IPIntelligenceOnPremiseEngine engine) { + List names = new ArrayList<>(); + for (FiftyOneAspectPropertyMetaData property : + engine.getProperties()) { + names.add(property.getName()); + } + return names; + } + + /** + * A native engine on its own with no names, so a test can read the + * native results directly. + */ + private static class NativeEngine implements AutoCloseable { + private final ConfigIpiSwig config = new ConfigIpiSwig(); + private final RequiredPropertiesConfigSwig required; + private final EngineIpiSwig engine; + + NativeEngine(File file) { + config.setMaxPerformance(); + RequiredPropertiesConfigSwig made; + try (VectorStringSwig vector = new VectorStringSwig()) { + made = new RequiredPropertiesConfigSwig(vector); + } + required = made; + engine = new EngineIpiSwig( + file.getAbsolutePath(), config, required); + } + + @Override + public void close() { + engine.delete(); + required.delete(); + config.delete(); } } } diff --git a/ip-intelligence/ip-intelligence/src/test/java/fiftyone/ipintelligence/OnPremisePerformanceProfileTests.java b/ip-intelligence/ip-intelligence/src/test/java/fiftyone/ipintelligence/OnPremisePerformanceProfileTests.java index 3af3ff389..01664c283 100644 --- a/ip-intelligence/ip-intelligence/src/test/java/fiftyone/ipintelligence/OnPremisePerformanceProfileTests.java +++ b/ip-intelligence/ip-intelligence/src/test/java/fiftyone/ipintelligence/OnPremisePerformanceProfileTests.java @@ -171,17 +171,17 @@ private static long[] collections(PerformanceProfiles profile) { */ private static long[] collections(ConfigIpiSwig config) { CollectionConfigSwig[] all = { - borrowed(config.getStrings()), - borrowed(config.getComponents()), - borrowed(config.getMaps()), - borrowed(config.getProperties()), - borrowed(config.getValues()), - borrowed(config.getProfiles()), - borrowed(config.getGraphs()), - borrowed(config.getProfileGroups()), - borrowed(config.getProfileOffsets()), - borrowed(config.getPropertyTypes()), - borrowed(config.getGraph()) + config.getStrings(), + config.getComponents(), + config.getMaps(), + config.getProperties(), + config.getValues(), + config.getProfiles(), + config.getGraphs(), + config.getProfileGroups(), + config.getProfileOffsets(), + config.getPropertyTypes(), + config.getGraph() }; long[] settings = new long[all.length * 2]; for (int i = 0; i < all.length; i++) { @@ -190,18 +190,4 @@ private static long[] collections(ConfigIpiSwig config) { } return settings; } - - /** - * The generated wrappers mark four ConfigIpiSwig getters as owned until - * they are regenerated with the C library's corrected MetaData.i. - */ - private static CollectionConfigSwig borrowed(CollectionConfigSwig proxy) { - try { - field(CollectionConfigSwig.class, "swigCMemOwn") - .setBoolean(proxy, false); - return proxy; - } catch (ReflectiveOperationException e) { - throw new IllegalStateException(e); - } - } }