From 276dd895c816ad42161fa98a15bc2cf68419d1b1 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Mon, 26 Oct 2026 23:32:50 +0000 Subject: [PATCH 1/2] Test that a failed update on start-up leaves the engine on its data file --- .../tests/update_on_startup.rs | 101 ++++++++++++++++ .../tests/update_on_startup.rs | 88 ++++++++++++++ pipeline-engines/tests/update_on_startup.rs | 109 ++++++++++++++++++ 3 files changed, 298 insertions(+) create mode 100644 device-detection-onpremise/tests/update_on_startup.rs create mode 100644 ip-intelligence-onpremise/tests/update_on_startup.rs create mode 100644 pipeline-engines/tests/update_on_startup.rs diff --git a/device-detection-onpremise/tests/update_on_startup.rs b/device-detection-onpremise/tests/update_on_startup.rs new file mode 100644 index 0000000..c6608fa --- /dev/null +++ b/device-detection-onpremise/tests/update_on_startup.rs @@ -0,0 +1,101 @@ +/* ********************************************************************* + * 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. + * ********************************************************************* */ + +//! Update on start-up when the update cannot be fetched. The test needs the +//! Lite file `51DEGREES_DD_PATH` names, and skips with a note without it. + +use std::net::TcpListener; +use std::path::PathBuf; +use std::sync::Arc; + +use fiftyone_device_detection_onpremise::DeviceDetectionOnPremiseEngineBuilder; +use fiftyone_device_detection_shared::{DeviceData, DEVICE_DATA_KEY}; +use fiftyone_native::PerformanceProfile; +use fiftyone_pipeline_core::{Evidence, FlowElement, Pipeline}; +use fiftyone_pipeline_engines::{DataFileConfiguration, DataUpdateService}; + +/// A mobile Safari on iPhone User-Agent. +const MOBILE_USER_AGENT: &str = "Mozilla/5.0 (iPhone; CPU iPhone OS 17_0 like Mac OS X) \ + AppleWebKit/605.1.15 (KHTML, like Gecko) Version/17.0 Mobile/15E148 Safari/604.1"; + +/// With update on start-up on, an engine builds from the data file it has when +/// the update cannot be fetched, as the specification says, and detects with +/// it. The data file stays registered for later updates. +#[test] +fn an_engine_builds_from_its_data_file_when_the_update_on_start_up_fails() { + let Some(lite) = std::env::var("51DEGREES_DD_PATH") + .ok() + .map(PathBuf::from) + .filter(|path| path.is_file()) + else { + eprintln!("skipping update on start-up: set 51DEGREES_DD_PATH to the Lite file"); + return; + }; + let dir = std::env::temp_dir().join(format!("fiftyone-dd-startup-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + let path = dir.join("51Degrees.hash"); + std::fs::copy(&lite, &path).unwrap(); + // Nothing listens on a port once its listener is dropped. + let listener = TcpListener::bind("127.0.0.1:0").unwrap(); + let url = format!("http://{}/download", listener.local_addr().unwrap()); + drop(listener); + + let service = Arc::new(DataUpdateService::new()); + let config = DataFileConfiguration::builder(&path) + .data_update_url(url) + .data_update_license_keys(["KEY"]) + .update_on_startup(true) + .automatic_updates_enabled(false) + .file_system_watcher_enabled(false) + .build(); + let engine = DeviceDetectionOnPremiseEngineBuilder::new(&path) + .performance_profile(PerformanceProfile::InMemory) + .auto_update(config, Some(Arc::clone(&service))) + .build() + .expect("the engine uses the data file it has"); + + assert_eq!(service.registered_count(), 1); + let element: Arc = engine.clone(); + let pipeline = Pipeline::builder().add_element(element).build().unwrap(); + let mut data = pipeline.create_flow_data_with( + Evidence::builder() + .add("header.user-agent", MOBILE_USER_AGENT) + .build(), + ); + data.process().unwrap(); + assert_eq!( + data.get(DEVICE_DATA_KEY) + .unwrap() + .is_mobile() + .value() + .ok() + .copied(), + Some(true) + ); + + drop(data); + drop(pipeline); + drop(engine); + drop(service); + let _ = std::fs::remove_dir_all(&dir); +} diff --git a/ip-intelligence-onpremise/tests/update_on_startup.rs b/ip-intelligence-onpremise/tests/update_on_startup.rs new file mode 100644 index 0000000..e58c8fe --- /dev/null +++ b/ip-intelligence-onpremise/tests/update_on_startup.rs @@ -0,0 +1,88 @@ +/* ********************************************************************* + * 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. + * ********************************************************************* */ + +//! Update on start-up when the update cannot be fetched. The test needs the +//! ASN file `51DEGREES_IPI_PATH` names, and skips with a note without it. + +use std::net::TcpListener; +use std::path::PathBuf; +use std::sync::Arc; + +use fiftyone_ip_intelligence_onpremise::{IpIntelligenceOnPremiseEngine, IP_DATA_KEY}; +use fiftyone_native::PerformanceProfile; +use fiftyone_pipeline_core::{Evidence, FlowElement, Pipeline}; +use fiftyone_pipeline_engines::DataUpdateService; + +/// With update on start-up on, an engine builds from the data file it has when +/// the update cannot be fetched, as the specification says, and looks up with +/// it. The data file stays registered for later updates. +#[test] +fn an_engine_builds_from_its_data_file_when_the_update_on_start_up_fails() { + let Some(asn) = std::env::var("51DEGREES_IPI_PATH") + .ok() + .map(PathBuf::from) + .filter(|path| path.is_file()) + else { + eprintln!("skipping update on start-up: set 51DEGREES_IPI_PATH to the ASN file"); + return; + }; + let dir = std::env::temp_dir().join(format!("fiftyone-ipi-startup-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + let path = dir.join("51Degrees.ipi"); + std::fs::copy(&asn, &path).unwrap(); + // Nothing listens on a port once its listener is dropped. + let listener = TcpListener::bind("127.0.0.1:0").unwrap(); + let url = format!("http://{}/download", listener.local_addr().unwrap()); + drop(listener); + + let service = Arc::new(DataUpdateService::new()); + let engine = IpIntelligenceOnPremiseEngine::builder(&path) + .performance_profile(PerformanceProfile::InMemory) + .properties(["Asn"]) + .auto_update(false) + .file_system_watcher(false) + .update_on_startup(true) + .data_update_url(url) + .data_update_license_keys(["KEY"]) + .data_update_service(Arc::clone(&service)) + .build_shared() + .expect("the engine uses the data file it has"); + + assert_eq!(service.registered_count(), 1); + let element: Arc = engine.clone(); + let pipeline = Pipeline::builder().add_element(element).build().unwrap(); + let mut data = pipeline.create_flow_data_with( + Evidence::builder() + .add("server.client-ip", "1.1.1.1") + .build(), + ); + data.process().unwrap(); + let asn_value = data.get(IP_DATA_KEY).unwrap().string("Asn"); + assert!(asn_value.value().is_ok(), "{asn_value:?}"); + + drop(data); + drop(pipeline); + drop(engine); + drop(service); + let _ = std::fs::remove_dir_all(&dir); +} diff --git a/pipeline-engines/tests/update_on_startup.rs b/pipeline-engines/tests/update_on_startup.rs new file mode 100644 index 0000000..fdd10d0 --- /dev/null +++ b/pipeline-engines/tests/update_on_startup.rs @@ -0,0 +1,109 @@ +/* ********************************************************************* + * 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. + * ********************************************************************* */ + +//! Tests of update on start-up when the update cannot be fetched. +#![cfg(feature = "data-update")] + +use std::net::TcpListener; +use std::sync::Arc; + +use fiftyone_pipeline_core::{ + EvidenceKeyFilter, EvidenceKeyFilterWhitelist, FlowData, FlowElement, PropertyMetaData, Result, +}; +use fiftyone_pipeline_engines::{ + AspectEngine, AspectEngineDataFile, AspectPropertyMetaData, AutoUpdateStatus, + DataFileConfiguration, DataUpdateService, OnPremiseAspectEngine, +}; + +/// An engine with data loaded, which a refresh leaves as it is. +struct LoadedEngine { + filter: EvidenceKeyFilterWhitelist, + data_files: Vec>, +} + +impl FlowElement for LoadedEngine { + fn process(&self, _data: &mut FlowData) -> Result<()> { + Ok(()) + } + fn data_key(&self) -> &str { + "loaded" + } + fn evidence_key_filter(&self) -> &dyn EvidenceKeyFilter { + &self.filter + } + fn properties(&self) -> &[PropertyMetaData] { + &[] + } +} + +impl AspectEngine for LoadedEngine { + fn data_source_tier(&self) -> &str { + "Lite" + } + fn aspect_properties(&self) -> &[AspectPropertyMetaData] { + &[] + } +} + +impl OnPremiseAspectEngine for LoadedEngine { + fn data_files(&self) -> &[Arc] { + &self.data_files + } + fn refresh(&self, _identifier: Option<&str>) -> Result<()> { + Ok(()) + } +} + +/// A failed update on start-up leaves the engine on the data it loaded, as the +/// specification says, so the data file is still registered for the updates +/// that follow, and the failure is reported. +#[test] +fn a_failed_update_on_start_up_still_registers_the_data_file() { + // Nothing listens on a port once its listener is dropped. + let listener = TcpListener::bind("127.0.0.1:0").unwrap(); + let url = format!("http://{}/datafile", listener.local_addr().unwrap()); + drop(listener); + let dir = std::env::temp_dir().join(format!("fiftyone-startup-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let path = dir.join("device.dat"); + std::fs::write(&path, b"the data loaded").unwrap(); + let config = DataFileConfiguration::builder(&path) + .data_update_url(url) + .update_on_startup(true) + .automatic_updates_enabled(true) + .file_system_watcher_enabled(false) + .build(); + let data_file = Arc::new(AspectEngineDataFile::new(config)); + let engine: Arc = Arc::new(LoadedEngine { + filter: EvidenceKeyFilterWhitelist::new(Vec::::new()), + data_files: vec![Arc::clone(&data_file)], + }); + let service = DataUpdateService::new(); + + let error = service + .register(engine, data_file) + .expect_err("nothing is listening"); + + assert_eq!(error.status, AutoUpdateStatus::HttpsError); + assert_eq!(service.registered_count(), 1); + let _ = std::fs::remove_dir_all(&dir); +} From e97ff3240b501fb4b14ae877703d486971f364d2 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Mon, 26 Oct 2026 23:33:12 +0000 Subject: [PATCH 2/2] Keep the local data file when the update on start-up fails, and register it all the same The specification says a failed update on start-up falls back to the local file, and fails only when there is none. The service returned before it registered the data file, so the file was never polled or watched, and both engine builders turned the error into a failed build, so an application could not start while the update server was unreachable or refusing requests. The service now logs the failure and registers the file all the same, and the builders return the engine, which has already loaded the local file. --- device-detection-onpremise/src/builder.rs | 10 ++++++---- ip-intelligence-onpremise/src/builder.rs | 10 ++++++---- pipeline-engines/src/data_update_service.rs | 22 ++++++++++++++++----- 3 files changed, 29 insertions(+), 13 deletions(-) diff --git a/device-detection-onpremise/src/builder.rs b/device-detection-onpremise/src/builder.rs index 721957f..0e19571 100644 --- a/device-detection-onpremise/src/builder.rs +++ b/device-detection-onpremise/src/builder.rs @@ -201,7 +201,9 @@ impl DeviceDetectionOnPremiseEngineBuilder { /// no licence key, as .NET does. When automatic updates /// were configured with a service, the engine's data file is registered /// with that service before the engine is returned, so a startup poll (if - /// requested) and the polling schedule are in effect. + /// requested) and the polling schedule are in effect. A startup poll that + /// fails leaves the engine on the data file it loaded, as the + /// specification says, and the service logs the failure. pub fn build(self) -> Result> { // The data-file configuration the engine carries. When auto-update was // requested, use that configuration; otherwise build a static one with @@ -244,9 +246,9 @@ impl DeviceDetectionOnPremiseEngineBuilder { .cloned() .ok_or_else(|| Error::configuration("engine has no data file to register"))?; let as_engine: Arc = engine.clone(); - service - .register(as_engine, data_file) - .map_err(|e| Error::configuration(e.to_string()))?; + // A failed update on start-up leaves the engine on the data file it + // loaded, as the specification says, and the service logs it. + let _ = service.register(as_engine, data_file); } Ok(engine) diff --git a/ip-intelligence-onpremise/src/builder.rs b/ip-intelligence-onpremise/src/builder.rs index aa8512e..527f114 100644 --- a/ip-intelligence-onpremise/src/builder.rs +++ b/ip-intelligence-onpremise/src/builder.rs @@ -243,7 +243,9 @@ impl IpIntelligenceOnPremiseEngineBuilder { /// /// Registration starts the file-system watcher and schedules the polls the /// options configure, and polls once first when update on startup is on. - /// Refuses, before opening the file, when automatic updates or update on + /// A startup poll that fails leaves the engine on the data file it loaded, + /// as the specification says, and the service logs the failure. Refuses, + /// before opening the file, when automatic updates or update on /// startup are on and no licence key is set. pub fn build_shared(mut self) -> Result> { let service = self.data_update_service.take(); @@ -255,9 +257,9 @@ impl IpIntelligenceOnPremiseEngineBuilder { .cloned() .ok_or_else(|| Error::configuration("engine has no data file to register"))?; let as_engine: Arc = engine.clone(); - service - .register(as_engine, data_file) - .map_err(|e| Error::configuration(e.to_string()))?; + // A failed update on start-up leaves the engine on the data file it + // loaded, as the specification says, and the service logs it. + let _ = service.register(as_engine, data_file); } Ok(engine) } diff --git a/pipeline-engines/src/data_update_service.rs b/pipeline-engines/src/data_update_service.rs index 6c6e83d..2c30643 100644 --- a/pipeline-engines/src/data_update_service.rs +++ b/pipeline-engines/src/data_update_service.rs @@ -147,7 +147,10 @@ impl DataUpdateService { /// interval (when `automatic_updates_enabled`). /// /// Returns the status of the startup poll, or [`AutoUpdateStatus::NotNeeded`] - /// when no startup poll was requested. + /// when no startup poll was requested. A failed startup poll leaves the + /// engine on the data it loaded, as the specification says. It is logged + /// and returned as an error, and the file is registered all the same, so + /// the updates that follow still run. pub fn register( &self, engine: Arc, @@ -164,11 +167,20 @@ impl DataUpdateService { watcher: Mutex::new(None), }); - let mut startup_status = AutoUpdateStatus::NotNeeded; + let mut startup = Ok(AutoUpdateStatus::NotNeeded); - // Update on startup: poll once, synchronously. + // Update on startup: poll once, synchronously. A failure leaves the + // engine on the data it loaded, so the file is still registered for + // the updates that follow. if config.update_on_startup && !already_registered { - startup_status = self.check_for_update_from_url(®istration)?; + startup = self.check_for_update_from_url(®istration); + if let Err(error) = &startup { + log::warn!( + "An error occurred while updating the data file for {} on start-up. \ + The data it has is used. Error detail: {error}", + registration.engine.data_key() + ); + } } // Install the file-system watcher if requested. @@ -196,7 +208,7 @@ impl DataUpdateService { self.ensure_scheduler(); self.inner.wake.notify_all(); - Ok(startup_status) + startup } /// Trigger an immediate update check for the engine's data file matching