Release a watched engine with its service, run the watcher only with automatic updates on, and watch only the data file

README.md

@@ -115,10 +115,12 @@ path, so cloud-only users and most of CI build without a C compiler.
   `.data_update_license_keys(..)`, and that refreshes the engine when the data
   file changes on disk. The Distributor serves nothing without a licence key,
   so a build with polling on and no key fails with an error that says so, as
-  it does in .NET, and a deployment of the free Lite file turns polling off
-  with `.auto_update(false)`. `.file_system_watcher(false)` turns the watcher
-  off, and `pipeline.resource::<DataUpdateService>()` reaches the service for
-  a check on demand.
+  it does in .NET, and a deployment of the free Lite file turns automatic
+  updates off with `.auto_update(false)`, which stops the polling and the file
+  watcher together, as the specification's master switch does.
+  `.file_system_watcher(false)` turns only the watcher off, and
+  `pipeline.resource::<DataUpdateService>()` reaches the service for a check
+  on demand.
 - JavaScript minification in `fiftyone-javascript-builder` is opt-in behind the
   `minify` feature and off by default. See ARCHITECTURE.md for why.
 

device-detection/src/lib.rs

@@ -90,10 +90,11 @@
 //! Distributor serves nothing without the licence keys set with
 //! `.data_update_license_keys(..)`, so a build with polling on and no key
 //! fails with an error that says so, as it does in .NET, and a deployment of
-//! the free Lite file turns polling off with `.auto_update(false)`.
-//! `.file_system_watcher(false)` turns the watcher off, and
-//! `pipeline.resource::<DataUpdateService>()` reaches the service for a check
-//! on demand.
+//! the free Lite file turns automatic updates off with `.auto_update(false)`,
+//! which stops the polling and the file watcher together, as the
+//! specification's master switch does. `.file_system_watcher(false)` turns
+//! only the watcher off, and `pipeline.resource::<DataUpdateService>()`
+//! reaches the service for a check on demand.
 //!
 //! ## Examples
 //!
@@ -314,14 +315,17 @@ impl DeviceDetectionOnPremisePipelineBuilder {
     /// with each new file. The 51Degrees Distributor serves nothing without a
     /// licence key, so [`Self::build`] refuses with this on and no key set, as
     /// .NET does, and a deployment of the Lite file turns this off or sets
-    /// [`Self::data_update_license_keys`].
+    /// [`Self::data_update_license_keys`]. This is the master switch for
+    /// automatic updates, so with it off the file-system watcher does not run
+    /// either.
     pub fn auto_update(mut self, enabled: bool) -> Self {
         self.auto_update = enabled;
         self
     }
 
     /// Enable or disable the file-system watcher, which refreshes the engine
-    /// when the data file changes on disk. On by default.
+    /// when the data file changes on disk. On by default, and it runs only
+    /// while [`Self::auto_update`] is on.
     pub fn file_system_watcher(mut self, enabled: bool) -> Self {
         self.file_system_watcher = enabled;
         self

examples/device-detection-examples/src/bin/dd-onprem-update-data-file.rs

@@ -76,9 +76,11 @@ pub fn run(options: ExampleOptions) -> Result<()> {
     let config = DataFileConfiguration::builder(&options.data_file)
         // Off so `build` does not block; see the startup section in the docs.
         .update_on_startup(false)
-        // Watch the file on disk and reload when it is replaced.
+        // Watch the file on disk and reload when it is replaced, while
+        // automatic updates are on.
         .file_system_watcher_enabled(true)
-        // Poll the distributor for a newer file on an interval.
+        // Poll the distributor for a newer file on an interval. Automatic
+        // updates are the master switch for polling and the watcher alike.
         .automatic_updates_enabled(options.license_key.is_some())
         // A short interval purely so the example does not appear idle; a real
         // deployment uses around 30 minutes.
@@ -164,9 +166,11 @@ fn describe_update_mechanisms(license_key: Option<&str>) {
          an Enterprise license key and a writable file location."
     );
     println!(
-        "2. File-system watcher. Set file_system_watcher_enabled(true). The \
-         service watches the data file on disk and reloads the engine when the \
-         file is replaced, which is how a manually-downloaded file is picked up."
+        "2. File-system watcher. Set file_system_watcher_enabled(true), with \
+         automatic_updates_enabled(true), the master switch for automatic \
+         updates. The service watches the data file on disk and reloads the \
+         engine when the file is replaced, which is how a manually-downloaded \
+         file is picked up."
     );
     println!(
         "3. Remote-update polling. Set automatic_updates_enabled(true), a \
@@ -266,10 +270,10 @@ mod tests {
  *   configuration, building the engine blocks while the service polls the
  *   51Degrees distributor once, so the engine starts with the most recent data.
  *   This needs an Enterprise license key and a writable file location.
- * - File-system watcher. With `file_system_watcher_enabled(true)`, the service
- *   watches the data file on disk and reloads the engine whenever the file is
- *   replaced. This is how a file you download and drop into place yourself is
- *   picked up automatically.
+ * - File-system watcher. With `file_system_watcher_enabled(true)` and
+ *   automatic updates on, the service watches the data file on disk and
+ *   reloads the engine whenever the file is replaced. This is how a file you
+ *   download and drop into place yourself is picked up automatically.
  * - Remote-update polling. With `automatic_updates_enabled(true)`, a polling
  *   interval (around 30 minutes in production) and a randomisation window (around
  *   10 minutes, to stagger many instances), the service periodically polls the

ip-intelligence-onpremise/src/builder.rs

@@ -156,15 +156,17 @@ impl IpIntelligenceOnPremiseEngineBuilder {
     ///
     /// When enabled, the [`DataUpdateService`] the engine is registered with
     /// polls the update URL on the polling interval and applies a downloaded
-    /// update by calling the engine's `refresh`.
+    /// update by calling the engine's `refresh`. This is the master switch for
+    /// automatic updates, so with it off the file-system watcher does not run
+    /// either.
     pub fn auto_update(mut self, enabled: bool) -> Self {
         self.auto_update_enabled = enabled;
         self
     }
 
     /// Enable or disable the file-system watcher for the data file. On by
-    /// default. When enabled, the update service refreshes the engine when the
-    /// file changes on disk.
+    /// default. When enabled, and [`Self::auto_update`] is on, the update
+    /// service refreshes the engine when the file changes on disk.
     pub fn file_system_watcher(mut self, enabled: bool) -> Self {
         self.file_system_watcher_enabled = enabled;
         self

ip-intelligence/src/lib.rs

@@ -65,10 +65,11 @@
 //! Distributor serves nothing without the licence keys set with
 //! `.data_update_license_keys(..)`, so a build with polling on and no key
 //! fails with an error that says so, as it does in .NET, and a deployment
-//! without a key turns polling off with `.auto_update(false)`.
-//! `.file_system_watcher(false)` turns the watcher off, and
-//! `pipeline.resource::<DataUpdateService>()` reaches the service for a check
-//! on demand.
+//! without a key turns automatic updates off with `.auto_update(false)`, which
+//! stops the polling and the file watcher together, as the specification's
+//! master switch does. `.file_system_watcher(false)` turns only the watcher
+//! off, and `pipeline.resource::<DataUpdateService>()` reaches the service for
+//! a check on demand.
 //!
 //! # Features
 //!

ip-intelligence/src/on_premise.rs

@@ -128,14 +128,17 @@ impl OnPremiseIpIntelligencePipelineBuilder {
     /// with each new file. The 51Degrees Distributor serves nothing without a
     /// licence key, so [`Self::build`] refuses with this on and no key set, as
     /// .NET does, and a deployment without one turns this off or sets
-    /// [`Self::data_update_license_keys`].
+    /// [`Self::data_update_license_keys`]. This is the master switch for
+    /// automatic updates, so with it off the file-system watcher does not run
+    /// either.
     pub fn auto_update(mut self, enabled: bool) -> Self {
         self.auto_update = enabled;
         self
     }
 
     /// Enable or disable the file-system watcher, which refreshes the engine
-    /// when the data file changes on disk. On by default.
+    /// when the data file changes on disk. On by default, and it runs only
+    /// while [`Self::auto_update`] is on.
     pub fn file_system_watcher(mut self, enabled: bool) -> Self {
         self.file_system_watcher = enabled;
         self

pipeline-engines/src/data_file.rs

@@ -104,9 +104,12 @@ pub struct DataFileConfiguration {
     /// polling.
     pub data_update_url: Option<String>,
     /// Whether the service should periodically poll [`Self::data_update_url`].
+    /// This is the master switch for automatic updates, so with it off there
+    /// is no file-system watcher either.
     pub automatic_updates_enabled: bool,
     /// Whether a file-system watcher should refresh the engine when the file at
-    /// [`Self::data_file_path`] changes on disk.
+    /// [`Self::data_file_path`] changes on disk. The watcher runs only when
+    /// [`Self::automatic_updates_enabled`] is on as well.
     pub file_system_watcher_enabled: bool,
     /// Whether the service should poll for an update once, synchronously, when
     /// the data file is first registered.
@@ -231,13 +234,15 @@ impl DataFileConfigurationBuilder {
         self
     }
 
-    /// Enable or disable automatic remote polling.
+    /// Enable or disable automatic updates, the master switch for remote
+    /// polling and the file-system watcher.
     pub fn automatic_updates_enabled(mut self, enabled: bool) -> Self {
         self.config.automatic_updates_enabled = enabled;
         self
     }
 
-    /// Enable or disable the file-system watcher.
+    /// Enable or disable the file-system watcher, which runs only while
+    /// automatic updates are on.
     pub fn file_system_watcher_enabled(mut self, enabled: bool) -> Self {
         self.config.file_system_watcher_enabled = enabled;
         self

pipeline-engines/src/data_update_service.rs

@@ -34,7 +34,9 @@
 //!   stagger many instances), sending an `If-Modified-Since` header so the
 //!   server can answer `304 Not Modified` cheaply.
 //! - **Automatic update from file.** A [`notify`] file-system watcher refreshes
-//!   the engine when the data file changes on disk.
+//!   the engine when the data file changes on disk. Like polling, it runs only
+//!   while automatic updates are on, which the specification makes the master
+//!   switch for both.
 //! - **Programmatic update.** [`DataUpdateService::check_for_update`] triggers an
 //!   immediate check on demand.
 //!
@@ -141,8 +143,10 @@ impl DataUpdateService {
     /// - If the configuration sets `update_on_startup`, a synchronous remote
     ///   poll runs now and its result is returned. A failed startup update is
     ///   returned as an error so the caller can decide whether to proceed.
-    /// - If `file_system_watcher_enabled` and the file has a path, a watcher is
-    ///   installed.
+    /// - If `automatic_updates_enabled` and `file_system_watcher_enabled` are
+    ///   both on and the file has a path, a watcher is installed. Automatic
+    ///   updates are the master switch for the watcher, as the specification
+    ///   says.
     /// - The file is added to the scheduler's list so it is polled on its
     ///   interval (when `automatic_updates_enabled`).
     ///
@@ -171,8 +175,12 @@ impl DataUpdateService {
             startup_status = self.check_for_update_from_url(&registration)?;
         }
 
-        // Install the file-system watcher if requested.
-        if config.file_system_watcher_enabled && data_file.data_file_path().is_some() {
+        // Install the file-system watcher if requested and automatic updates,
+        // its master switch, are on.
+        if config.automatic_updates_enabled
+            && config.file_system_watcher_enabled
+            && data_file.data_file_path().is_some()
+        {
             self.install_watcher(&registration);
         }
 
@@ -331,31 +339,40 @@ impl DataUpdateService {
 
     /// Install a file-system watcher that refreshes the engine when the data
     /// file changes on disk.
+    ///
+    /// The registration owns the watcher, so the watcher's callback holds the
+    /// registration weakly. A strong reference would keep the registration,
+    /// its engine and the watcher's thread alive after the service is dropped.
     fn install_watcher(&self, registration: &Arc<Registration>) {
         use notify::{RecursiveMode, Watcher};
 
         let path = match registration.data_file.data_file_path() {
             Some(p) => p.to_path_buf(),
             None => return,
         };
+        let file_name = path.file_name().map(ToOwned::to_owned);
 
-        let reg = Arc::clone(registration);
-        // The service is held alive by the caller for as long as updates are
-        // wanted, so the closure captures only the registration (which owns the
-        // engine and data file) plus a clone of the HTTP client is not needed
-        // here, the watcher only refreshes from the file on disk.
-        let client = self.client.clone();
-        let service = ServiceHandle {
-            client,
-            inner: Arc::clone(&self.inner),
-        };
+        // The file as it is now is the one the engine loaded, so only a later
+        // change refreshes it.
+        if registration.data_file.last_applied_modified().is_none() {
+            if let Some(modified) = file_modified_time(&path) {
+                registration.data_file.set_last_applied_modified(modified);
+            }
+        }
 
+        let reg = Arc::downgrade(registration);
         let watcher = notify::recommended_watcher(move |res: notify::Result<notify::Event>| {
-            if let Ok(event) = res {
-                if event.kind.is_modify() || event.kind.is_create() {
-                    // Debounce duplicate events by comparing the file's
-                    // last-modified time with the last we applied.
-                    service.on_file_changed(&reg);
+            let Ok(event) = res else {
+                return;
+            };
+            // The watch is on the folder, so skip events for other files in it.
+            let names_the_file = event
+                .paths
+                .iter()
+                .any(|changed| changed.file_name() == file_name.as_deref());
+            if names_the_file && (event.kind.is_modify() || event.kind.is_create()) {
+                if let Some(reg) = reg.upgrade() {
+                    ServiceHandle::on_file_changed(&reg);
                 }
             }
         });
@@ -535,7 +552,7 @@ impl ServiceHandle {
 
     /// Handle a file-system-watcher event by refreshing the engine from disk if
     /// the file is genuinely newer than the one already applied.
-    fn on_file_changed(&self, registration: &Arc<Registration>) {
+    fn on_file_changed(registration: &Registration) {
         let path = match registration.data_file.data_file_path() {
             Some(p) => p.to_path_buf(),
             None => return,