From 9d5c29c70db7d06a552271a4d647894c20ec4375 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Wed, 11 Nov 2026 00:42:09 +0000 Subject: [PATCH 1/7] Read property values by index for an engine built with names --- .../flowelements/DeviceDataHashDefault.java | 55 ++++++++++---- .../DeviceDetectionHashEngine.java | 58 +++++++++++++++ .../IPIntelligenceDataHashDefault.java | 74 ++++++++++++++----- .../IPIntelligenceOnPremiseEngine.java | 57 ++++++++++++++ 4 files changed, 214 insertions(+), 30 deletions(-) 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 d78e6b443..3dad6384f 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 @@ -68,6 +68,12 @@ public class DeviceDataHashDefault */ private final List resultsList = new ArrayList<>(); + /** + * The engine which created this instance, used to turn a property name + * into the required property index the engine was built with. + */ + private final DeviceDetectionHashEngine hashEngine; + /** * Constructs a new instance. * @param logger used for logging @@ -83,6 +89,18 @@ public class DeviceDataHashDefault DeviceDetectionHashEngine engine, MissingPropertyService missingPropertyService) { super(logger, flowData, engine, missingPropertyService); + this.hashEngine = engine; + } + + /** + * 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. + * @param propertyName name of the property + * @return required property index, or null + */ + private Integer indexOf(String propertyName) { + return hashEngine == null ? + null : hashEngine.getPropertyIndex(propertyName); } /** @@ -256,8 +274,10 @@ public AspectPropertyValue> getValues(String propertyName) { new AspectPropertyValueDefault<>(); ResultsHashSwig results = getSingleResults(propertyName); if (results != null) { - try (VectorStringValuesSwig value = results.getValues( - Swig.asBytes(propertyName))) { + Integer index = indexOf(propertyName); + try (VectorStringValuesSwig value = index == null ? + results.getValues(Swig.asBytes(propertyName)) : + results.getValues(index)) { if (value.hasValue()) { try (VectorStringSwig vector = value.getValue()) { result.setValue(Collections.unmodifiableList( @@ -277,9 +297,10 @@ protected AspectPropertyValue getValueAsString(String propertyName) { AspectPropertyValue result = new AspectPropertyValueDefault<>(); ResultsHashSwig results = getSingleResults(propertyName); if (results != null) { - - try (StringValueSwig value = results.getValueAsString( - Swig.asBytes(propertyName))) { + Integer index = indexOf(propertyName); + try (StringValueSwig value = index == null ? + results.getValueAsString(Swig.asBytes(propertyName)) : + results.getValueAsString(index)) { if (value.hasValue()) { result.setValue(value.getValue()); } @@ -298,8 +319,10 @@ protected AspectPropertyValue getValueAsJavaScript( new AspectPropertyValueDefault<>(); ResultsHashSwig results = getSingleResults(propertyName); if (results != null) { - try (StringValueSwig value = results.getValueAsString( - Swig.asBytes(propertyName))) { + Integer index = indexOf(propertyName); + try (StringValueSwig value = index == null ? + results.getValueAsString(Swig.asBytes(propertyName)) : + results.getValueAsString(index)) { if (value.hasValue()) { result.setValue(new JavaScript(value.getValue())); } @@ -317,8 +340,10 @@ protected AspectPropertyValue getValueAsInteger( AspectPropertyValue result = new AspectPropertyValueDefault<>(); ResultsHashSwig results = getSingleResults(propertyName); if (results != null) { - try (IntegerValueSwig value = results.getValueAsInteger( - Swig.asBytes(propertyName))) { + Integer index = indexOf(propertyName); + try (IntegerValueSwig value = index == null ? + results.getValueAsInteger(Swig.asBytes(propertyName)) : + results.getValueAsInteger(index)) { if (value.hasValue()) { result.setValue(value.getValue()); } @@ -335,8 +360,10 @@ protected AspectPropertyValue getValueAsBool(String propertyName) { AspectPropertyValue result = new AspectPropertyValueDefault<>(); ResultsHashSwig results = getSingleResults(propertyName); if (results != null) { - try (BoolValueSwig value = results.getValueAsBool( - Swig.asBytes(propertyName))) { + Integer index = indexOf(propertyName); + try (BoolValueSwig value = index == null ? + results.getValueAsBool(Swig.asBytes(propertyName)) : + results.getValueAsBool(index)) { if (value.hasValue()) { result.setValue(value.getValue()); } @@ -353,8 +380,10 @@ protected AspectPropertyValue getValueAsDouble(String propertyName) { AspectPropertyValue result = new AspectPropertyValueDefault<>(); ResultsHashSwig results = getSingleResults(propertyName); if (results != null) { - try (DoubleValueSwig value = results.getValueAsDouble( - Swig.asBytes(propertyName))) { + Integer index = indexOf(propertyName); + try (DoubleValueSwig value = index == null ? + results.getValueAsDouble(Swig.asBytes(propertyName)) : + results.getValueAsDouble(index)) { if (value.hasValue()) { result.setValue(value.getValue()); } 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 64ede4a73..21bb42cf0 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 @@ -62,6 +62,16 @@ public class DeviceDetectionHashEngine private volatile boolean propertiesPopulated = false; private final Random rand = new Random(); + /** + * Case insensitive map from each property name the engine was built with + * to its required property index. Built once, from the names given at + * build, 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. + */ + private volatile Map propertyIndexes = + Collections.emptyMap(); + /** * Construct a new instance of the {@link DeviceDetectionHashEngine}. * @param logger logger instance to use for logging @@ -113,6 +123,52 @@ public MetaDataSwig getMetaData() { return engine.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. + * @param propertyName name of the property + * @return required property index, or null + */ + Integer getPropertyIndex(String propertyName) { + return propertyIndexes.get(propertyName); + } + + /** + * Build the map from the names the engine was built with to their + * required property indexes, using the name the native engine reports at + * each index. Called once, after the native engine is first made, and + * never again, because the whole point of the index is that it keeps + * naming the same property across a reload. + */ + private void setPropertyIndexes() { + Map indexes = + new TreeMap<>(String.CASE_INSENSITIVE_ORDER); + try { + VectorStringSwig names = propertiesConfigSwig.getProperties(); + int count = names.size(); + names.delete(); + if (count > 0) { + try (EvidenceDeviceDetectionSwig evidence = + new EvidenceDeviceDetectionSwig(); + ResultsHashSwig results = engine.process(evidence)) { + for (int i = 0; i < count; i++) { + String name = results.getPropertyName(i); + if (name != null && name.isEmpty() == false) { + indexes.put(name, i); + } + } + } + } + } catch (Exception e) { + logger.error( + "Exception occurred while mapping property names to indexes. " + + "Values will be read by name.", + e); + indexes.clear(); + } + propertyIndexes = Collections.unmodifiableMap(indexes); + } + @Override public List getProperties() { if (isFalse(propertiesPopulated)) { @@ -286,6 +342,7 @@ public void refreshData(String dataFileIdentifier) { throw new IllegalStateException("Failed to create SwigEngine after " + tries + " retries, last exception was " + lastMessage); } + setPropertyIndexes(); } else { engine.refreshData(); } @@ -296,6 +353,7 @@ public void refreshData(String dataFileIdentifier) { public void refreshData(String dataFileIdentifier, byte[] data) { if (engine == null) { engine = new EngineHashSwig(data, config, propertiesConfigSwig); + setPropertyIndexes(); } else { engine.refreshData(data); } 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 6baf8af0e..a93045407 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 @@ -68,6 +68,12 @@ public class IPIntelligenceDataHashDefault */ private final List resultsList = new ArrayList<>(); + /** + * The engine which created this instance, used to turn a property name + * into the required property index the engine was built with. + */ + private final IPIntelligenceOnPremiseEngine ipiEngine; + /** * Constructs a new instance. * @param logger used for logging @@ -83,6 +89,18 @@ public class IPIntelligenceDataHashDefault IPIntelligenceOnPremiseEngine engine, MissingPropertyService missingPropertyService) { super(logger, flowData, engine, missingPropertyService); + this.ipiEngine = engine; + } + + /** + * 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. + * @param propertyName name of the property + * @return required property index, or null + */ + private Integer indexOf(String propertyName) { + return ipiEngine == null ? + null : ipiEngine.getPropertyIndex(propertyName); } /** @@ -184,10 +202,13 @@ public AspectPropertyValue> getValues(String propertyName, return null; } Class valueType = parametrizedTypes[2]; + Integer index = indexOf(propertyName); if (valueType.equals(Boolean.class)) { populateWeightedValues( result, - () -> results.getValuesAsWeightedBoolList(propertyName), + () -> index == null ? + results.getValuesAsWeightedBoolList(propertyName) : + results.getValuesAsWeightedBoolList(index), WeightedBoolListValueSwig::hasValue, WeightedBoolListValueSwig::getNoValueMessage, WeightedBoolListValueSwig::getValue, @@ -196,7 +217,9 @@ public AspectPropertyValue> getValues(String propertyName, } else if (valueType.equals(Double.class)) { populateWeightedValues( result, - () -> results.getValuesAsWeightedDoubleList(propertyName), + () -> index == null ? + results.getValuesAsWeightedDoubleList(propertyName) : + results.getValuesAsWeightedDoubleList(index), WeightedDoubleListValueSwig::hasValue, WeightedDoubleListValueSwig::getNoValueMessage, WeightedDoubleListValueSwig::getValue, @@ -205,7 +228,9 @@ public AspectPropertyValue> getValues(String propertyName, } else if (valueType.equals(Float.class)) { populateWeightedValues( result, - () -> results.getValuesAsWeightedDoubleList(propertyName), + () -> index == null ? + results.getValuesAsWeightedDoubleList(propertyName) : + results.getValuesAsWeightedDoubleList(index), WeightedDoubleListValueSwig::hasValue, WeightedDoubleListValueSwig::getNoValueMessage, WeightedDoubleListValueSwig::getValue, @@ -214,7 +239,9 @@ public AspectPropertyValue> getValues(String propertyName, } else if (valueType.equals(Integer.class)) { populateWeightedValues( result, - () -> results.getValuesAsWeightedIntegerList(propertyName), + () -> index == null ? + results.getValuesAsWeightedIntegerList(propertyName) : + results.getValuesAsWeightedIntegerList(index), WeightedIntListValueSwig::hasValue, WeightedIntListValueSwig::getNoValueMessage, WeightedIntListValueSwig::getValue, @@ -225,7 +252,9 @@ public AspectPropertyValue> getValues(String propertyName, || valueType.equals(Inet6Address.class)) { populateWeightedValues( result, - () -> results.getValuesAsWeightedStringList(propertyName), + () -> index == null ? + results.getValuesAsWeightedStringList(propertyName) : + results.getValuesAsWeightedStringList(index), WeightedStringListValueSwig::hasValue, WeightedStringListValueSwig::getNoValueMessage, WeightedStringListValueSwig::getValue, @@ -240,7 +269,9 @@ public AspectPropertyValue> getValues(String propertyName, } else if (valueType.equals(String.class)) { populateWeightedValues( result, - () -> results.getValuesAsWeightedUTF8StringList(propertyName), + () -> index == null ? + results.getValuesAsWeightedUTF8StringList(propertyName) : + results.getValuesAsWeightedUTF8StringList(index), WeightedUTF8StringListValueSwig::hasValue, WeightedUTF8StringListValueSwig::getNoValueMessage, WeightedUTF8StringListValueSwig::getValue, @@ -267,9 +298,10 @@ protected AspectPropertyValue getValueAsString(String propertyName) { AspectPropertyValue result = new AspectPropertyValueDefault<>(); ResultsIpiSwig results = getSingleResults(propertyName); if (results != null) { - - try (StringValueSwig value = results.getValueAsString( - propertyName)) { + Integer index = indexOf(propertyName); + try (StringValueSwig value = index == null ? + results.getValueAsString(propertyName) : + results.getValueAsString(index)) { if (value.hasValue()) { result.setValue(value.getValue()); } @@ -288,8 +320,10 @@ protected AspectPropertyValue getValueAsJavaScript( new AspectPropertyValueDefault<>(); ResultsIpiSwig results = getSingleResults(propertyName); if (results != null) { - try (StringValueSwig value = results.getValueAsString( - propertyName)) { + Integer index = indexOf(propertyName); + try (StringValueSwig value = index == null ? + results.getValueAsString(propertyName) : + results.getValueAsString(index)) { if (value.hasValue()) { result.setValue(new JavaScript(value.getValue())); } @@ -307,8 +341,10 @@ protected AspectPropertyValue getValueAsInteger( AspectPropertyValue result = new AspectPropertyValueDefault<>(); ResultsIpiSwig results = getSingleResults(propertyName); if (results != null) { - try (IntegerValueSwig value = results.getValueAsInteger( - propertyName)) { + Integer index = indexOf(propertyName); + try (IntegerValueSwig value = index == null ? + results.getValueAsInteger(propertyName) : + results.getValueAsInteger(index)) { if (value.hasValue()) { result.setValue(value.getValue()); } @@ -325,8 +361,10 @@ protected AspectPropertyValue getValueAsBool(String propertyName) { AspectPropertyValue result = new AspectPropertyValueDefault<>(); ResultsIpiSwig results = getSingleResults(propertyName); if (results != null) { - try (BoolValueSwig value = results.getValueAsBool( - propertyName)) { + Integer index = indexOf(propertyName); + try (BoolValueSwig value = index == null ? + results.getValueAsBool(propertyName) : + results.getValueAsBool(index)) { if (value.hasValue()) { result.setValue(value.getValue()); } @@ -343,8 +381,10 @@ protected AspectPropertyValue getValueAsDouble(String propertyName) { AspectPropertyValue result = new AspectPropertyValueDefault<>(); ResultsIpiSwig results = getSingleResults(propertyName); if (results != null) { - try (DoubleValueSwig value = results.getValueAsDouble( - propertyName)) { + Integer index = indexOf(propertyName); + try (DoubleValueSwig value = index == null ? + results.getValueAsDouble(propertyName) : + results.getValueAsDouble(index)) { if (value.hasValue()) { result.setValue(value.getValue()); } 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 a0779d7dc..3a6907a8c 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 @@ -62,6 +62,16 @@ public class IPIntelligenceOnPremiseEngine private volatile boolean propertiesPopulated = false; private final Random rand = new Random(); + /** + * Case insensitive map from each property name the engine was built with + * to its required property index. Built once, from the names given at + * build, 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. + */ + private volatile Map propertyIndexes = + Collections.emptyMap(); + /** * Construct a new instance of the {@link IPIntelligenceOnPremiseEngine}. * @param logger logger instance to use for logging @@ -113,6 +123,51 @@ public MetaDataSwig getMetaData() { return engine.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. + * @param propertyName name of the property + * @return required property index, or null + */ + Integer getPropertyIndex(String propertyName) { + return propertyIndexes.get(propertyName); + } + + /** + * Build the map from the names the engine was built with to their + * required property indexes, using the name the native engine reports at + * each index. Called once, after the native engine is first made, and + * never again, because the whole point of the index is that it keeps + * naming the same property across a reload. + */ + private void setPropertyIndexes() { + Map indexes = + new TreeMap<>(String.CASE_INSENSITIVE_ORDER); + try { + VectorStringSwig names = propertiesConfigSwig.getProperties(); + int count = names.size(); + names.delete(); + if (count > 0) { + try (EvidenceIpiSwig evidence = new EvidenceIpiSwig(); + ResultsIpiSwig results = engine.process(evidence)) { + for (int i = 0; i < count; i++) { + String name = results.getPropertyName(i); + if (name != null && name.isEmpty() == false) { + indexes.put(name, i); + } + } + } + } + } catch (Exception e) { + logger.error( + "Exception occurred while mapping property names to indexes. " + + "Values will be read by name.", + e); + indexes.clear(); + } + propertyIndexes = Collections.unmodifiableMap(indexes); + } + @Override public List getProperties() { if (isFalse(propertiesPopulated)) { @@ -276,6 +331,7 @@ public void refreshData(String dataFileIdentifier) { throw new IllegalStateException("Failed to create SwigEngine after " + tries + " retries, last exception was " + lastMessage); } + setPropertyIndexes(); } else { engine.refreshData(); } @@ -286,6 +342,7 @@ public void refreshData(String dataFileIdentifier) { public void refreshData(String dataFileIdentifier, byte[] data) { if (engine == null) { engine = new EngineIpiSwig(data, config, propertiesConfigSwig); + setPropertyIndexes(); } else { engine.refreshData(data); } From f3be9a43ffcbb07d96df84be6eca6a5c39079e70 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Wed, 11 Nov 2026 00:43:25 +0000 Subject: [PATCH 2/7] Test the property index rule on both on-premise engines --- .../flowelements/PropertyIndexTests.java | 245 ++++++++++++++++++ .../flowelements/PropertyIndexTests.java | 147 +++++++++++ 2 files changed, 392 insertions(+) create mode 100644 device-detection/device-detection.hash.engine.on-premise/src/test/java/fiftyone/devicedetection/hash/engine/onpremise/flowelements/PropertyIndexTests.java create mode 100644 ip-intelligence/ip-intelligence.engine.on-premise/src/test/java/fiftyone/ipintelligence/engine/onpremise/flowelements/PropertyIndexTests.java 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 new file mode 100644 index 000000000..f1b525ec7 --- /dev/null +++ b/device-detection/device-detection.hash.engine.on-premise/src/test/java/fiftyone/devicedetection/hash/engine/onpremise/flowelements/PropertyIndexTests.java @@ -0,0 +1,245 @@ +/* ********************************************************************* + * 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.devicedetection.hash.engine.onpremise.flowelements; + +import fiftyone.devicedetection.shared.DeviceData; +import fiftyone.pipeline.core.data.FlowData; +import fiftyone.pipeline.core.flowelements.Pipeline; +import fiftyone.pipeline.core.flowelements.PipelineBuilder; +import fiftyone.pipeline.engines.Constants; +import fiftyone.pipeline.engines.data.AspectPropertyValue; +import org.junit.Test; +import org.slf4j.ILoggerFactory; +import org.slf4j.LoggerFactory; + +import java.io.File; +import java.nio.file.Files; +import java.util.Arrays; +import java.util.List; + +import static fiftyone.devicedetection.shared.testhelpers.FileUtils.LITE_HASH_DATA_FILE_PATH; +import static fiftyone.devicedetection.shared.testhelpers.FileUtils.TAC_HASH_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.Assume.assumeTrue; + +/** + * Tests for the property index rule, which says that a property's index is + * its place in the list of names the engine was built with, that the index is + * fixed at build, and that every read by name goes through that index. + */ +public class PropertyIndexTests { + + private static final ILoggerFactory loggerFactory = + LoggerFactory.getILoggerFactory(); + + /** + * Names in an order that is deliberately not the order the data file + * holds them in, so an index taken from the data file would be wrong. + */ + private static final List NAMES = Arrays.asList( + "ScreenPixelsHeight", + "IsMobile", + "PlatformName", + "BrowserName"); + + private static File dataFile(String name) { + try { + return getFilePath(name); + } + catch (IllegalArgumentException e) { + assumeTrue(e.getMessage(), false); + return null; + } + } + + private static DeviceDetectionHashEngine build( + File file, List names) throws Exception { + DeviceDetectionHashEngineBuilder builder = + new DeviceDetectionHashEngineBuilder(loggerFactory, null) + .setPerformanceProfile( + Constants.PerformanceProfiles.HighPerformance) + .setAutoUpdate(false) + .setDataFileSystemWatcher(false); + if (names != null) { + builder.setProperties(names); + } + return builder.build(file.toString(), false); + } + + /** + * Every name the engine was built with has an index, and the index is the + * place of the name in the build time list, not its place in the data + * file. This is the test that goes red without the change, because + * reading by name alone gives the right answer today. + */ + @Test + public void PropertyIndex_FromBuildTimeNames() throws Exception { + File file = dataFile(LITE_HASH_DATA_FILE_PATH); + try (DeviceDetectionHashEngine engine = build(file, NAMES)) { + for (int i = 0; i < NAMES.size(); i++) { + Integer index = engine.getPropertyIndex(NAMES.get(i)); + assertNotNull( + "Expected an index for '" + NAMES.get(i) + "'.", + index); + assertEquals( + "Expected '" + NAMES.get(i) + "' at index " + i + ".", + Integer.valueOf(i), + index); + } + } + } + + /** + * The lookup is case insensitive, because a caller may ask for a property + * in any casing. + */ + @Test + public void PropertyIndex_CaseInsensitive() throws Exception { + File file = dataFile(LITE_HASH_DATA_FILE_PATH); + try (DeviceDetectionHashEngine engine = build(file, NAMES)) { + assertEquals( + engine.getPropertyIndex("IsMobile"), + engine.getPropertyIndex("ismobile")); + } + } + + /** + * A name the engine was not built with has no index, so the read falls + * back to the name and gives the answer it gives today. + */ + @Test + public void PropertyIndex_UnknownNameHasNoIndex() throws Exception { + File file = dataFile(LITE_HASH_DATA_FILE_PATH); + try (DeviceDetectionHashEngine engine = build(file, NAMES)) { + assertNull(engine.getPropertyIndex("HardwareVendor")); + assertNull(engine.getPropertyIndex("NotAPropertyAtAll")); + } + } + + /** + * An engine built with no names reads by name, which is the path left + * unchanged until the native layer keeps the first load's names across a + * reload. + */ + @Test + public void PropertyIndex_NoNamesReadsByName() throws Exception { + File file = dataFile(LITE_HASH_DATA_FILE_PATH); + try (DeviceDetectionHashEngine engine = build(file, null)) { + assertNull(engine.getPropertyIndex("IsMobile")); + } + } + + /** + * A read by name and the typed read give the same value, because the + * typed accessor goes through the same path. + */ + @Test + public void PropertyIndex_ByNameMatchesTyped() throws Exception { + File file = dataFile(LITE_HASH_DATA_FILE_PATH); + DeviceDetectionHashEngine engine = build(file, NAMES); + try (Pipeline pipeline = new PipelineBuilder(loggerFactory) + .addFlowElement(engine) + .build(); + FlowData data = pipeline.createFlowData()) { + data.addEvidence( + "header.user-agent", + "Mozilla/5.0 (iPhone; CPU iPhone OS 15_0 like Mac OS X) " + + "AppleWebKit/605.1.15 (KHTML, like Gecko) " + + "Version/15.0 Mobile/15E148 Safari/604.1"); + data.process(); + DeviceData device = data.get(DeviceData.class); + AspectPropertyValue typed = device.getIsMobile(); + AspectPropertyValue byName = + (AspectPropertyValue) device.get("IsMobile"); + assertEquals(typed.hasValue(), byName.hasValue()); + if (typed.hasValue()) { + assertEquals(typed.getValue(), byName.getValue()); + } + } + } + + /** + * After a reload from a data file whose properties are a superset and in + * a different order, each index still names the property it named at + * build, so every value read by name is still the right one. + */ + @Test + public void PropertyIndex_SurvivesReload() throws Exception { + File lite = dataFile(LITE_HASH_DATA_FILE_PATH); + File bigger = dataFile(TAC_HASH_DATA_FILE_NAME); + + String userAgent = + "Mozilla/5.0 (iPhone; CPU iPhone OS 15_0 like Mac OS X) " + + "AppleWebKit/605.1.15 (KHTML, like Gecko) " + + "Version/15.0 Mobile/15E148 Safari/604.1"; + + DeviceDetectionHashEngine engine = build(lite, NAMES); + try (Pipeline pipeline = new PipelineBuilder(loggerFactory) + .addFlowElement(engine) + .build()) { + + // The indexes the engine was built with. + for (int i = 0; i < NAMES.size(); i++) { + assertEquals( + Integer.valueOf(i), + engine.getPropertyIndex(NAMES.get(i))); + } + + // Reload from the larger data file, whose property order differs. + engine.refreshData( + engine.getDataFiles().get(0).getIdentifier(), + Files.readAllBytes(bigger.toPath())); + + // The indexes have not moved. + for (int i = 0; i < NAMES.size(); i++) { + assertEquals( + "Index of '" + NAMES.get(i) + "' moved on reload.", + Integer.valueOf(i), + engine.getPropertyIndex(NAMES.get(i))); + } + + // Every name still reads its own property, and the typed read + // agrees with the read by name. + try (FlowData data = pipeline.createFlowData()) { + data.addEvidence("header.user-agent", userAgent); + data.process(); + DeviceData device = data.get(DeviceData.class); + AspectPropertyValue typed = device.getIsMobile(); + AspectPropertyValue byName = + (AspectPropertyValue) device.get("IsMobile"); + assertEquals(typed.hasValue(), byName.hasValue()); + if (typed.hasValue()) { + assertEquals(typed.getValue(), byName.getValue()); + } + for (String name : NAMES) { + assertNotNull( + "Expected a result for '" + name + "'.", + device.get(name)); + } + } + } + } +} 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 new file mode 100644 index 000000000..f2b05832b --- /dev/null +++ b/ip-intelligence/ip-intelligence.engine.on-premise/src/test/java/fiftyone/ipintelligence/engine/onpremise/flowelements/PropertyIndexTests.java @@ -0,0 +1,147 @@ +/* ********************************************************************* + * 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.ipintelligence.engine.onpremise.flowelements; + +import fiftyone.pipeline.engines.Constants; +import org.junit.Test; +import org.slf4j.ILoggerFactory; +import org.slf4j.LoggerFactory; + +import java.io.File; +import java.util.Arrays; +import java.util.List; + +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.Assume.assumeTrue; + +/** + * Tests for the property index rule on the IP Intelligence on-premise + * engine, which says that a property's index is its place in the list of + * names the engine was built with, that the index is fixed at build, and that + * every read by name goes through that index. + */ +public class PropertyIndexTests { + + private static final ILoggerFactory loggerFactory = + LoggerFactory.getILoggerFactory(); + + /** + * Names in an order that is deliberately not the order the data file + * holds them in, so an index taken from the data file would be wrong. + */ + private static final List NAMES = Arrays.asList( + "RegisteredCountry", + "AccuracyRadiusMin", + "RegisteredName", + "Latitude"); + + private static File dataFile(String name) { + try { + return getFilePath(name); + } + catch (IllegalArgumentException e) { + assumeTrue(e.getMessage(), false); + return null; + } + } + + private static IPIntelligenceOnPremiseEngine build( + File file, List names) throws Exception { + IPIntelligenceOnPremiseEngineBuilder builder = + new IPIntelligenceOnPremiseEngineBuilder(loggerFactory, null) + .setPerformanceProfile( + Constants.PerformanceProfiles.MaxPerformance) + .setAutoUpdate(false) + .setDataFileSystemWatcher(false); + if (names != null) { + builder.setProperties(names); + } + return builder.build(file.toString(), false); + } + + /** + * Every name the engine was built with has an index, and the index is the + * place of the name in the build time list, not its place in the data + * file. This is the test that goes red without the change, because + * reading by name alone gives the right answer today. + */ + @Test + public void PropertyIndex_FromBuildTimeNames() throws Exception { + File file = dataFile(LITE_IPI_DATA_FILE_NAME); + try (IPIntelligenceOnPremiseEngine engine = build(file, NAMES)) { + for (int i = 0; i < NAMES.size(); i++) { + Integer index = engine.getPropertyIndex(NAMES.get(i)); + assertNotNull( + "Expected an index for '" + NAMES.get(i) + "'.", + index); + assertEquals( + "Expected '" + NAMES.get(i) + "' at index " + i + ".", + Integer.valueOf(i), + index); + } + } + } + + /** + * The lookup is case insensitive, because a caller may ask for a property + * in any casing. + */ + @Test + public void PropertyIndex_CaseInsensitive() throws Exception { + File file = dataFile(LITE_IPI_DATA_FILE_NAME); + try (IPIntelligenceOnPremiseEngine engine = build(file, NAMES)) { + assertEquals( + engine.getPropertyIndex("RegisteredName"), + engine.getPropertyIndex("registeredname")); + } + } + + /** + * A name the engine was not built with has no index, so the read falls + * back to the name and gives the answer it gives today. + */ + @Test + public void PropertyIndex_UnknownNameHasNoIndex() throws Exception { + File file = dataFile(LITE_IPI_DATA_FILE_NAME); + try (IPIntelligenceOnPremiseEngine engine = build(file, NAMES)) { + assertNull(engine.getPropertyIndex("NotAPropertyAtAll")); + } + } + + /** + * An engine built with no names reads by name, which is the path left + * unchanged until the native layer keeps the first load's names across a + * reload. + */ + @Test + public void PropertyIndex_NoNamesReadsByName() throws Exception { + File file = dataFile(LITE_IPI_DATA_FILE_NAME); + try (IPIntelligenceOnPremiseEngine engine = build(file, null)) { + assertNull(engine.getPropertyIndex("RegisteredName")); + } + } +} From db1d837a02c79ab621b8145f92da3218ce1d923f Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Wed, 11 Nov 2026 07:48:10 +0000 Subject: [PATCH 3/7] Test that a property name given twice has the index of its first place C gives a property the place of its name among the names given, the first where the name is given more than once, and compares names without regard to case. The builder drops only an exact repeat, so IsMobile and ismobile both reach C. The engines' map keeps the last index for such a name, so the new tests fail on it. The test comments no longer say which test goes red or describe the change. --- .../flowelements/PropertyIndexTests.java | 30 ++++++++++++++----- .../flowelements/PropertyIndexTests.java | 30 ++++++++++++++----- 2 files changed, 46 insertions(+), 14 deletions(-) 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 f1b525ec7..f3157342c 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 @@ -92,8 +92,7 @@ private static DeviceDetectionHashEngine build( /** * Every name the engine was built with has an index, and the index is the * place of the name in the build time list, not its place in the data - * file. This is the test that goes red without the change, because - * reading by name alone gives the right answer today. + * file. */ @Test public void PropertyIndex_FromBuildTimeNames() throws Exception { @@ -127,8 +126,26 @@ public void PropertyIndex_CaseInsensitive() throws Exception { } /** - * A name the engine was not built with has no index, so the read falls - * back to the name and gives the answer it gives today. + * A name given twice, in any letter case, has the index of its first + * place, as the native engine gives it. + */ + @Test + public void PropertyIndex_NameGivenTwiceHasFirstIndex() throws Exception { + File file = dataFile(LITE_HASH_DATA_FILE_PATH); + try (DeviceDetectionHashEngine engine = build(file, + Arrays.asList("IsMobile", "BrowserName", "ismobile"))) { + assertEquals( + Integer.valueOf(0), + engine.getPropertyIndex("IsMobile")); + assertEquals( + Integer.valueOf(1), + engine.getPropertyIndex("BrowserName")); + } + } + + /** + * A name the engine was not built with has no index, so the read goes by + * name. */ @Test public void PropertyIndex_UnknownNameHasNoIndex() throws Exception { @@ -140,9 +157,8 @@ public void PropertyIndex_UnknownNameHasNoIndex() throws Exception { } /** - * An engine built with no names reads by name, which is the path left - * unchanged until the native layer keeps the first load's names across a - * reload. + * An engine built with no names has no indexes, so every read goes by + * name. */ @Test public void PropertyIndex_NoNamesReadsByName() throws Exception { 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 f2b05832b..1aac78acd 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 @@ -86,8 +86,7 @@ private static IPIntelligenceOnPremiseEngine build( /** * Every name the engine was built with has an index, and the index is the * place of the name in the build time list, not its place in the data - * file. This is the test that goes red without the change, because - * reading by name alone gives the right answer today. + * file. */ @Test public void PropertyIndex_FromBuildTimeNames() throws Exception { @@ -121,8 +120,26 @@ public void PropertyIndex_CaseInsensitive() throws Exception { } /** - * A name the engine was not built with has no index, so the read falls - * back to the name and gives the answer it gives today. + * A name given twice, in any letter case, has the index of its first + * place, as the native engine gives it. + */ + @Test + public void PropertyIndex_NameGivenTwiceHasFirstIndex() throws Exception { + File file = dataFile(LITE_IPI_DATA_FILE_NAME); + try (IPIntelligenceOnPremiseEngine engine = build(file, + Arrays.asList("RegisteredName", "Latitude", "registeredname"))) { + assertEquals( + Integer.valueOf(0), + engine.getPropertyIndex("RegisteredName")); + assertEquals( + Integer.valueOf(1), + engine.getPropertyIndex("Latitude")); + } + } + + /** + * A name the engine was not built with has no index, so the read goes by + * name. */ @Test public void PropertyIndex_UnknownNameHasNoIndex() throws Exception { @@ -133,9 +150,8 @@ public void PropertyIndex_UnknownNameHasNoIndex() throws Exception { } /** - * An engine built with no names reads by name, which is the path left - * unchanged until the native layer keeps the first load's names across a - * reload. + * An engine built with no names has no indexes, so every read goes by + * name. */ @Test public void PropertyIndex_NoNamesReadsByName() throws Exception { From 691ce2e204d178c7e041073873fb8219f899cc66 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Wed, 11 Nov 2026 07:50:47 +0000 Subject: [PATCH 4/7] Take the property indexes from the names the builder gives, keeping the first Each engine now makes its map from the list of names its builder passes to the native engine, in the constructor, before any data is loaded. That is the list C takes the required property indexes from, one place for each entry in the order given, so the map holds the same indexes without running a detection on empty Evidence to get a results object, which for an engine built with names walked every component's graph at build. A name given more than once keeps its first place, as C keeps it. The map is final and never rebuilt, and the native engine's required properties configuration is no longer read back. --- .../DeviceDetectionHashEngine.java | 58 ++++++++----------- .../DeviceDetectionHashEngineBuilder.java | 1 + .../IPIntelligenceOnPremiseEngine.java | 57 ++++++++---------- .../IPIntelligenceOnPremiseEngineBuilder.java | 1 + 4 files changed, 48 insertions(+), 69 deletions(-) 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 21bb42cf0..2da75263e 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 @@ -64,13 +64,14 @@ public class DeviceDetectionHashEngine /** * Case insensitive map from each property name the engine was built with - * to its required property index. Built once, from the names given at - * build, 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. + * to its required property index, which is the place of the name in that + * 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. */ - private volatile Map propertyIndexes = - Collections.emptyMap(); + private final Map propertyIndexes; /** * Construct a new instance of the {@link DeviceDetectionHashEngine}. @@ -80,6 +81,8 @@ public class DeviceDetectionHashEngine * @param properties native required properties configuration which define * the properties which the engine should be initialised * with + * @param propertyNames the property names {@code properties} was made + * from, in the same order * @param deviceDataFactory the factory to use when creating a * {@link DeviceDataHash} instance * @param tempDataFileDir the file where a temporary data file copy @@ -90,11 +93,13 @@ public class DeviceDetectionHashEngine AspectEngineDataFile dataFile, ConfigHashSwig config, RequiredPropertiesConfigSwig properties, + List propertyNames, ElementDataFactory deviceDataFactory, String tempDataFileDir) { super(logger, deviceDataFactory, tempDataFileDir); this.config = config; this.propertiesConfigSwig = properties; + this.propertyIndexes = indexesOf(propertyNames); addDataFile(dataFile); } @@ -134,39 +139,24 @@ Integer getPropertyIndex(String propertyName) { } /** - * Build the map from the names the engine was built with to their - * required property indexes, using the name the native engine reports at - * each index. Called once, after the native engine is first made, and - * never again, because the whole point of the index is that it keeps - * naming the same property across a reload. + * Map each name to its place in the list, keeping the first place of a + * name given more than once in any letter case. + * @param names the property names in the order the engine is built with, + * or null + * @return unmodifiable case insensitive map from name to index */ - private void setPropertyIndexes() { + private static Map indexesOf(List names) { Map indexes = new TreeMap<>(String.CASE_INSENSITIVE_ORDER); - try { - VectorStringSwig names = propertiesConfigSwig.getProperties(); - int count = names.size(); - names.delete(); - if (count > 0) { - try (EvidenceDeviceDetectionSwig evidence = - new EvidenceDeviceDetectionSwig(); - ResultsHashSwig results = engine.process(evidence)) { - for (int i = 0; i < count; i++) { - String name = results.getPropertyName(i); - if (name != null && name.isEmpty() == false) { - indexes.put(name, i); - } - } + if (names != null) { + for (int i = 0; i < names.size(); i++) { + String name = names.get(i); + if (name != null && name.isEmpty() == false) { + indexes.putIfAbsent(name, i); } } - } catch (Exception e) { - logger.error( - "Exception occurred while mapping property names to indexes. " + - "Values will be read by name.", - e); - indexes.clear(); } - propertyIndexes = Collections.unmodifiableMap(indexes); + return Collections.unmodifiableMap(indexes); } @Override @@ -342,7 +332,6 @@ public void refreshData(String dataFileIdentifier) { throw new IllegalStateException("Failed to create SwigEngine after " + tries + " retries, last exception was " + lastMessage); } - setPropertyIndexes(); } else { engine.refreshData(); } @@ -353,7 +342,6 @@ public void refreshData(String dataFileIdentifier) { public void refreshData(String dataFileIdentifier, byte[] data) { if (engine == null) { engine = new EngineHashSwig(data, config, propertiesConfigSwig); - setPropertyIndexes(); } else { engine.refreshData(data); } diff --git a/device-detection/device-detection.hash.engine.on-premise/src/main/java/fiftyone/devicedetection/hash/engine/onpremise/flowelements/DeviceDetectionHashEngineBuilder.java b/device-detection/device-detection.hash.engine.on-premise/src/main/java/fiftyone/devicedetection/hash/engine/onpremise/flowelements/DeviceDetectionHashEngineBuilder.java index 774e1b574..1de52bba8 100644 --- a/device-detection/device-detection.hash.engine.on-premise/src/main/java/fiftyone/devicedetection/hash/engine/onpremise/flowelements/DeviceDetectionHashEngineBuilder.java +++ b/device-detection/device-detection.hash.engine.on-premise/src/main/java/fiftyone/devicedetection/hash/engine/onpremise/flowelements/DeviceDetectionHashEngineBuilder.java @@ -341,6 +341,7 @@ protected DeviceDetectionHashEngine newEngine(List properties) { dataFile, config, requiredProperties, + properties, new HashDataFactory(loggerFactory), tempDir); } 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 3a6907a8c..053521d41 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 @@ -64,13 +64,14 @@ public class IPIntelligenceOnPremiseEngine /** * Case insensitive map from each property name the engine was built with - * to its required property index. Built once, from the names given at - * build, 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. + * to its required property index, which is the place of the name in that + * 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. */ - private volatile Map propertyIndexes = - Collections.emptyMap(); + private final Map propertyIndexes; /** * Construct a new instance of the {@link IPIntelligenceOnPremiseEngine}. @@ -80,6 +81,8 @@ public class IPIntelligenceOnPremiseEngine * @param properties native required properties configuration which define * the properties which the engine should be initialised * with + * @param propertyNames the property names {@code properties} was made + * from, in the same order * @param ipiDataFactory the factory to use when creating a * {@link IPIntelligenceDataHash} instance * @param tempDataFileDir the file where a temporary data file copy @@ -90,11 +93,13 @@ public class IPIntelligenceOnPremiseEngine AspectEngineDataFile dataFile, ConfigIpiSwig config, RequiredPropertiesConfigSwig properties, + List propertyNames, ElementDataFactory ipiDataFactory, String tempDataFileDir) { super(logger, ipiDataFactory, tempDataFileDir); this.config = config; this.propertiesConfigSwig = properties; + this.propertyIndexes = indexesOf(propertyNames); addDataFile(dataFile); } @@ -134,38 +139,24 @@ Integer getPropertyIndex(String propertyName) { } /** - * Build the map from the names the engine was built with to their - * required property indexes, using the name the native engine reports at - * each index. Called once, after the native engine is first made, and - * never again, because the whole point of the index is that it keeps - * naming the same property across a reload. + * Map each name to its place in the list, keeping the first place of a + * name given more than once in any letter case. + * @param names the property names in the order the engine is built with, + * or null + * @return unmodifiable case insensitive map from name to index */ - private void setPropertyIndexes() { + private static Map indexesOf(List names) { Map indexes = new TreeMap<>(String.CASE_INSENSITIVE_ORDER); - try { - VectorStringSwig names = propertiesConfigSwig.getProperties(); - int count = names.size(); - names.delete(); - if (count > 0) { - try (EvidenceIpiSwig evidence = new EvidenceIpiSwig(); - ResultsIpiSwig results = engine.process(evidence)) { - for (int i = 0; i < count; i++) { - String name = results.getPropertyName(i); - if (name != null && name.isEmpty() == false) { - indexes.put(name, i); - } - } + if (names != null) { + for (int i = 0; i < names.size(); i++) { + String name = names.get(i); + if (name != null && name.isEmpty() == false) { + indexes.putIfAbsent(name, i); } } - } catch (Exception e) { - logger.error( - "Exception occurred while mapping property names to indexes. " + - "Values will be read by name.", - e); - indexes.clear(); } - propertyIndexes = Collections.unmodifiableMap(indexes); + return Collections.unmodifiableMap(indexes); } @Override @@ -331,7 +322,6 @@ public void refreshData(String dataFileIdentifier) { throw new IllegalStateException("Failed to create SwigEngine after " + tries + " retries, last exception was " + lastMessage); } - setPropertyIndexes(); } else { engine.refreshData(); } @@ -342,7 +332,6 @@ public void refreshData(String dataFileIdentifier) { public void refreshData(String dataFileIdentifier, byte[] data) { if (engine == null) { engine = new EngineIpiSwig(data, config, propertiesConfigSwig); - setPropertyIndexes(); } else { engine.refreshData(data); } diff --git a/ip-intelligence/ip-intelligence.engine.on-premise/src/main/java/fiftyone/ipintelligence/engine/onpremise/flowelements/IPIntelligenceOnPremiseEngineBuilder.java b/ip-intelligence/ip-intelligence.engine.on-premise/src/main/java/fiftyone/ipintelligence/engine/onpremise/flowelements/IPIntelligenceOnPremiseEngineBuilder.java index c403aee93..cf0b071fe 100644 --- a/ip-intelligence/ip-intelligence.engine.on-premise/src/main/java/fiftyone/ipintelligence/engine/onpremise/flowelements/IPIntelligenceOnPremiseEngineBuilder.java +++ b/ip-intelligence/ip-intelligence.engine.on-premise/src/main/java/fiftyone/ipintelligence/engine/onpremise/flowelements/IPIntelligenceOnPremiseEngineBuilder.java @@ -257,6 +257,7 @@ protected IPIntelligenceOnPremiseEngine newEngine(List properties) { dataFile, config, requiredProperties, + properties, new IpiDataFactory(loggerFactory), tempDir); } From 83c0839772ad9c66d822e2aa51c0f49bdc7a49ef Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Wed, 11 Nov 2026 09:03:23 +0000 Subject: [PATCH 5/7] Time a read of IsMobile by name against a read by index PropertyIndex_ReadByIndexAgainstByName builds the native engine with the same names, reads IsMobile 20,000 times by name and 20,000 times by index from the same native results, and 20,000 times through the typed accessor, in five rounds after a warm-up. It requires every read to give the same value and prints the lowest, median and highest time of one read, so each build reports the figures for its platform. --- .../flowelements/PropertyIndexTests.java | 121 ++++++++++++++++++ 1 file changed, 121 insertions(+) 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 f3157342c..faf3446f2 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 @@ -22,6 +22,12 @@ package fiftyone.devicedetection.hash.engine.onpremise.flowelements; +import fiftyone.devicedetection.hash.engine.onpremise.interop.swig.BoolValueSwig; +import fiftyone.devicedetection.hash.engine.onpremise.interop.swig.ConfigHashSwig; +import fiftyone.devicedetection.hash.engine.onpremise.interop.swig.EngineHashSwig; +import fiftyone.devicedetection.hash.engine.onpremise.interop.swig.RequiredPropertiesConfigSwig; +import fiftyone.devicedetection.hash.engine.onpremise.interop.swig.ResultsHashSwig; +import fiftyone.devicedetection.hash.engine.onpremise.interop.swig.VectorStringSwig; import fiftyone.devicedetection.shared.DeviceData; import fiftyone.pipeline.core.data.FlowData; import fiftyone.pipeline.core.flowelements.Pipeline; @@ -34,7 +40,9 @@ import java.io.File; import java.nio.file.Files; +import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.List; import static fiftyone.devicedetection.shared.testhelpers.FileUtils.LITE_HASH_DATA_FILE_PATH; @@ -65,6 +73,15 @@ public class PropertyIndexTests { "PlatformName", "BrowserName"); + private static final String MOBILE = + "Mozilla/5.0 (iPhone; CPU iPhone OS 15_0 like Mac OS X) " + + "AppleWebKit/605.1.15 (KHTML, like Gecko) " + + "Version/15.0 Mobile/15E148 Safari/604.1"; + + private static final int READS = 20000; + + private static final int ROUNDS = 5; + private static File dataFile(String name) { try { return getFilePath(name); @@ -258,4 +275,108 @@ public void PropertyIndex_SurvivesReload() throws Exception { } } } + + /** + * Reads IsMobile {@link #READS} times by name and {@link #READS} times by + * 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. + */ + @Test + public void PropertyIndex_ReadByIndexAgainstByName() throws Exception { + File file = dataFile(LITE_HASH_DATA_FILE_PATH); + DeviceDetectionHashEngine engine = build(file, NAMES); + int index = engine.getPropertyIndex("IsMobile"); + ConfigHashSwig config = new ConfigHashSwig(); + config.setHighPerformance(); + RequiredPropertiesConfigSwig required; + try (VectorStringSwig names = new VectorStringSwig()) { + names.addAll(NAMES); + required = new RequiredPropertiesConfigSwig(names); + } + EngineHashSwig nativeEngine = + new EngineHashSwig(file.getAbsolutePath(), config, required); + try (Pipeline pipeline = new PipelineBuilder(loggerFactory) + .addFlowElement(engine) + .build(); + FlowData data = pipeline.createFlowData(); + ResultsHashSwig results = nativeEngine.process(MOBILE)) { + data.addEvidence("header.user-agent", MOBILE); + data.process(); + DeviceData device = data.get(DeviceData.class); + + boolean expected = readByName(results); + assertEquals(expected, readByIndex(results, index)); + assertEquals(expected, (boolean) device.getIsMobile().getValue()); + + List byName = new ArrayList<>(); + List byIndex = new ArrayList<>(); + List typed = new ArrayList<>(); + int different = 0; + for (int round = 0; round <= ROUNDS; round++) { + long start = System.nanoTime(); + for (int i = 0; i < READS; i++) { + if (readByName(results) != expected) { + different++; + } + } + long nameNanos = System.nanoTime() - start; + start = System.nanoTime(); + for (int i = 0; i < READS; i++) { + if (readByIndex(results, index) != expected) { + different++; + } + } + long indexNanos = System.nanoTime() - start; + start = System.nanoTime(); + for (int i = 0; i < READS; i++) { + if (device.getIsMobile().getValue() != expected) { + different++; + } + } + long typedNanos = System.nanoTime() - start; + if (round > 0) { + byName.add(nameNanos / READS); + byIndex.add(indexNanos / READS); + typed.add(typedNanos / READS); + } + } + 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", + READS, ROUNDS, spread(byName), spread(byIndex), + spread(typed), System.getProperty("java.version"), + System.getProperty("os.name"))); + } + finally { + nativeEngine.delete(); + required.delete(); + config.delete(); + } + } + + private static boolean readByName(ResultsHashSwig results) { + try (BoolValueSwig value = results.getValueAsBool("IsMobile")) { + return value.getValue(); + } + } + + private static boolean readByIndex(ResultsHashSwig results, int index) { + try (BoolValueSwig value = results.getValueAsBool(index)) { + return value.getValue(); + } + } + + /** + * The lowest, median and highest, in that order. + */ + private static String spread(List values) { + List sorted = new ArrayList<>(values); + Collections.sort(sorted); + return sorted.get(0) + "/" + sorted.get(sorted.size() / 2) + "/" + + sorted.get(sorted.size() - 1); + } } From 567d225e2abae27a341bb215d712ccde1de7b524 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Wed, 11 Nov 2026 09:51:24 +0000 Subject: [PATCH 6/7] Time the read by name through the overload the reads use A read by name with no index goes through getValueAsBool(Swig.asBytes( name)), which is also how every read by name went on rc, so the timing test now reads by name that way rather than through the String overload. --- .../engine/onpremise/flowelements/PropertyIndexTests.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) 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 faf3446f2..b60248560 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 @@ -22,6 +22,7 @@ package fiftyone.devicedetection.hash.engine.onpremise.flowelements; +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; import fiftyone.devicedetection.hash.engine.onpremise.interop.swig.EngineHashSwig; @@ -358,8 +359,13 @@ READS, ROUNDS, spread(byName), spread(byIndex), } } + /** + * 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. + */ private static boolean readByName(ResultsHashSwig results) { - try (BoolValueSwig value = results.getValueAsBool("IsMobile")) { + try (BoolValueSwig value = + results.getValueAsBool(Swig.asBytes("IsMobile"))) { return value.getValue(); } } From 52bcfd1c2761c851743737cf4827aa9730dc63bb Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Wed, 11 Nov 2026 10:16:48 +0000 Subject: [PATCH 7/7] Test a reload from a data file that lacks a property the engine was named with The engine is built on the TAC file with a name the Lite file lacks, then reloaded from the Lite file. The name keeps its index, a read of it by index gives no value rather than an error, as a read by name does, and IsMobile still reads its own value. C maps a named index to the data set before it reads, and a name the data set lacks maps to -1, so the read reports no value. The test pins that, so it has no red. --- .../flowelements/PropertyIndexTests.java | 79 +++++++++++++++++++ 1 file changed, 79 insertions(+) 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 b60248560..4c222ce41 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 @@ -22,6 +22,7 @@ package fiftyone.devicedetection.hash.engine.onpremise.flowelements; +import fiftyone.devicedetection.hash.engine.onpremise.data.DeviceDataHash; 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; @@ -35,6 +36,7 @@ import fiftyone.pipeline.core.flowelements.PipelineBuilder; import fiftyone.pipeline.engines.Constants; import fiftyone.pipeline.engines.data.AspectPropertyValue; +import fiftyone.pipeline.engines.fiftyone.data.FiftyOneAspectPropertyMetaData; import org.junit.Test; import org.slf4j.ILoggerFactory; import org.slf4j.LoggerFactory; @@ -50,8 +52,10 @@ import static fiftyone.devicedetection.shared.testhelpers.FileUtils.TAC_HASH_DATA_FILE_NAME; import static fiftyone.pipeline.util.FileFinder.getFilePath; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; import static org.junit.Assume.assumeTrue; /** @@ -277,6 +281,81 @@ public void PropertyIndex_SurvivesReload() throws Exception { } } + /** + * A name the engine was built with keeps its index after a reload from a + * data file that lacks the property. A read by index then gives no value, + * as a read by name does, rather than an error, and the other names still + * read their own properties. + */ + @Test + public void PropertyIndex_NameTheReloadedFileLacks() throws Exception { + File lite = dataFile(LITE_HASH_DATA_FILE_PATH); + File bigger = dataFile(TAC_HASH_DATA_FILE_NAME); + String missing = nameOnlyIn(bigger, lite); + assumeTrue("The TAC file has no property the Lite file lacks.", + missing != null); + List names = Arrays.asList(missing, "IsMobile"); + + DeviceDetectionHashEngine engine = build(bigger, names); + try (Pipeline pipeline = new PipelineBuilder(loggerFactory) + .addFlowElement(engine) + .build()) { + engine.refreshData( + engine.getDataFiles().get(0).getIdentifier(), + Files.readAllBytes(lite.toPath())); + + assertEquals( + "Index of '" + missing + "' moved on reload.", + Integer.valueOf(0), + engine.getPropertyIndex(missing)); + assertEquals( + Integer.valueOf(1), + engine.getPropertyIndex("IsMobile")); + + try (FlowData data = pipeline.createFlowData()) { + data.addEvidence("header.user-agent", MOBILE); + data.process(); + DeviceDataHash device = + (DeviceDataHash) data.get(DeviceData.class); + AspectPropertyValue> values = + device.getValues(missing); + assertFalse( + "'" + missing + "' has a value from a file without it.", + values.hasValue()); + assertNotNull(values.getNoValueMessage()); + assertTrue("IsMobile has no value after the reload.", + device.getIsMobile().hasValue()); + assertEquals(Boolean.TRUE, device.getIsMobile().getValue()); + } + } + } + + /** + * The first property name, in name order, that one data file has and the + * other lacks, or null if there is none. + */ + private static String nameOnlyIn(File has, File lacks) throws Exception { + List lacking = propertyNames(lacks); + for (String name : propertyNames(has)) { + if (lacking.contains(name) == false) { + return name; + } + } + return null; + } + + private static List propertyNames(File file) throws Exception { + List names = new ArrayList<>(); + try (DeviceDetectionHashEngine engine = build(file, null)) { + for (FiftyOneAspectPropertyMetaData property : + engine.getProperties()) { + names.add(property.getName()); + } + } + Collections.sort(names); + return names; + } + /** * Reads IsMobile {@link #READS} times by name and {@link #READS} times by * index from the same native results, and {@link #READS} times through