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 d78e6b44..3dad6384 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 64ede4a7..2da75263 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,17 @@ 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, 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 final Map propertyIndexes; + /** * Construct a new instance of the {@link DeviceDetectionHashEngine}. * @param logger logger instance to use for logging @@ -70,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 @@ -80,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); } @@ -113,6 +128,37 @@ 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); + } + + /** + * 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 static Map indexesOf(List names) { + Map indexes = + new TreeMap<>(String.CASE_INSENSITIVE_ORDER); + 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); + } + } + } + return Collections.unmodifiableMap(indexes); + } + @Override public List getProperties() { if (isFalse(propertiesPopulated)) { 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 774e1b57..1de52bba 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/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 00000000..4c222ce4 --- /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,467 @@ +/* ********************************************************************* + * 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.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; +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; +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; + +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; +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; + +/** + * 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 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); + } + 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. + */ + @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 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 { + 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 has no indexes, so every read goes by + * name. + */ + @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)); + } + } + } + } + + /** + * 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 + * 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(); + } + } + + /** + * 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(Swig.asBytes("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); + } +} 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 6baf8af0..a9304540 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 a0779d7d..053521d4 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,17 @@ 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, 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 final Map propertyIndexes; + /** * Construct a new instance of the {@link IPIntelligenceOnPremiseEngine}. * @param logger logger instance to use for logging @@ -70,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 @@ -80,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); } @@ -113,6 +128,37 @@ 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); + } + + /** + * 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 static Map indexesOf(List names) { + Map indexes = + new TreeMap<>(String.CASE_INSENSITIVE_ORDER); + 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); + } + } + } + return Collections.unmodifiableMap(indexes); + } + @Override public List getProperties() { if (isFalse(propertiesPopulated)) { 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 c403aee9..cf0b071f 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); } 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 00000000..1aac78ac --- /dev/null +++ b/ip-intelligence/ip-intelligence.engine.on-premise/src/test/java/fiftyone/ipintelligence/engine/onpremise/flowelements/PropertyIndexTests.java @@ -0,0 +1,163 @@ +/* ********************************************************************* + * 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. + */ + @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 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 { + File file = dataFile(LITE_IPI_DATA_FILE_NAME); + try (IPIntelligenceOnPremiseEngine engine = build(file, NAMES)) { + assertNull(engine.getPropertyIndex("NotAPropertyAtAll")); + } + } + + /** + * An engine built with no names has no indexes, so every read goes by + * name. + */ + @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")); + } + } +}