Split collection observers for data vs state updates

oguzkocer · oguzkocer · commit d64142fbea3e · 2025-12-18T10:48:37.000-05:00
Phase 4 of MetadataService implementation: Enable granular observer control
so UI can show loading indicators separately from data refreshes.

Changes:
- Split `is_relevant_update` into `is_relevant_data_update` and
  `is_relevant_state_update` in `MetadataCollection`
- Add relevance checking methods to `ListMetadataReader` trait:
  - `get_list_metadata_id()` - get DB rowid for a key
  - `is_item_row_for_key()` - check if item row belongs to key
  - `is_state_row_for_list()` - check if state row belongs to list
  - `get_sync_state()` - get current ListState for a key
- Add `sync_state()` method to query current sync state (Idle,
  FetchingFirstPage, FetchingNextPage, Error)
- Add repository methods for relevance checking in `wp_mobile_cache`
- Implement all trait methods in `MetadataService`

The original `is_relevant_update()` still works (combines both checks)
for backwards compatibility.

Phase 4.2 (Kotlin wrapper split observers) not included - requires
platform-specific updates.
diff --git a/wp_mobile/src/collection/post_metadata_collection.rs b/wp_mobile/src/collection/post_metadata_collection.rs
@@ -186,14 +186,49 @@ impl PostMetadataCollectionWithEditContext {
         self.collection.total_pages()
     }
 
-    /// Check if a database update is relevant to this collection.
+    /// Get the current sync state for this collection.
     ///
-    /// Returns `true` if the update is to a table this collection monitors.
-    /// Platform layers use this to determine when to notify observers.
+    /// Returns the current `ListState`:
+    /// - `Idle` - No sync in progress
+    /// - `FetchingFirstPage` - Refresh in progress
+    /// - `FetchingNextPage` - Load more in progress
+    /// - `Error` - Last sync failed
+    ///
+    /// Use this to show loading indicators in the UI. Observe state changes
+    /// via `is_relevant_state_update`.
+    pub fn sync_state(&self) -> wp_mobile_cache::list_metadata::ListState {
+        self.collection.sync_state()
+    }
+
+    /// Check if a database update is relevant to this collection (either data or state).
+    ///
+    /// Returns `true` if the update affects either data or state.
+    /// For more granular control, use `is_relevant_data_update` or `is_relevant_state_update`.
     pub fn is_relevant_update(&self, hook: &UpdateHook) -> bool {
         self.collection.is_relevant_update(hook)
     }
 
+    /// Check if a database update affects this collection's data.
+    ///
+    /// Returns `true` if the update is to:
+    /// - An entity table this collection monitors (PostsEditContext, TermRelationships)
+    /// - The ListMetadataItems table for this collection's key
+    ///
+    /// Use this for data observers that should refresh list contents.
+    pub fn is_relevant_data_update(&self, hook: &UpdateHook) -> bool {
+        self.collection.is_relevant_data_update(hook)
+    }
+
+    /// Check if a database update affects this collection's sync state.
+    ///
+    /// Returns `true` if the update is to the ListMetadataState table
+    /// for this collection's specific list.
+    ///
+    /// Use this for state observers that should update loading indicators.
+    pub fn is_relevant_state_update(&self, hook: &UpdateHook) -> bool {
+        self.collection.is_relevant_state_update(hook)
+    }
+
     /// Get the filter for this collection.
     pub fn filter(&self) -> AnyPostFilter {
         self.filter.clone()
diff --git a/wp_mobile/src/service/metadata.rs b/wp_mobile/src/service/metadata.rs
@@ -302,16 +302,87 @@ impl MetadataService {
         })?;
         Ok(())
     }
+
+    // ============================================
+    // Relevance checking for update hooks
+    // ============================================
+
+    /// Get the list_metadata_id (rowid) for a given key.
+    ///
+    /// Returns None if no list exists for this key yet.
+    /// Used by collections to cache the ID for state update matching.
+    pub fn get_list_metadata_id(&self, key: &str) -> Option<i64> {
+        self.cache
+            .execute(|conn| self.repo.get_list_metadata_id(conn, &self.db_site, key))
+            .ok()
+            .flatten()
+            .map(i64::from) // Convert RowId to i64 for trait interface
+    }
+
+    /// Check if a list_metadata_state row belongs to a specific list_metadata_id.
+    ///
+    /// Given a rowid from the list_metadata_state table (from an UpdateHook),
+    /// returns true if that state row belongs to the given list_metadata_id.
+    pub fn is_state_row_for_list(&self, state_row_id: i64, list_metadata_id: i64) -> bool {
+        use wp_mobile_cache::RowId;
+
+        self.cache
+            .execute(|conn| {
+                self.repo
+                    .get_list_metadata_id_for_state_row(conn, RowId::from(state_row_id))
+            })
+            .ok()
+            .flatten()
+            .is_some_and(|id| i64::from(id) == list_metadata_id)
+    }
+
+    /// Check if a list_metadata_items row belongs to a specific key.
+    ///
+    /// Given a rowid from the list_metadata_items table (from an UpdateHook),
+    /// returns true if that item row belongs to this service's site and the given key.
+    pub fn is_item_row_for_key(&self, item_row_id: i64, key: &str) -> bool {
+        use wp_mobile_cache::RowId;
+
+        self.cache
+            .execute(|conn| {
+                self.repo
+                    .is_item_row_for_key(conn, &self.db_site, key, RowId::from(item_row_id))
+            })
+            .unwrap_or(false)
+    }
 }
 
 /// Implement ListMetadataReader for database-backed metadata.
 ///
 /// This allows MetadataCollection to read list structure from the database
 /// through the same trait interface it uses for in-memory stores.
+///
+/// Unlike the in-memory implementation, this also supports relevance checking
+/// methods for split observers (data vs state updates).
 impl ListMetadataReader for MetadataService {
     fn get(&self, key: &str) -> Option<Vec<EntityMetadata>> {
         self.get_metadata(key).ok().flatten()
     }
+
+    fn get_list_metadata_id(&self, key: &str) -> Option<i64> {
+        // Delegate to our existing method
+        MetadataService::get_list_metadata_id(self, key)
+    }
+
+    fn is_item_row_for_key(&self, item_row_id: i64, key: &str) -> bool {
+        // Delegate to our existing method
+        MetadataService::is_item_row_for_key(self, item_row_id, key)
+    }
+
+    fn is_state_row_for_list(&self, state_row_id: i64, list_metadata_id: i64) -> bool {
+        // Delegate to our existing method
+        MetadataService::is_state_row_for_list(self, state_row_id, list_metadata_id)
+    }
+
+    fn get_sync_state(&self, key: &str) -> wp_mobile_cache::list_metadata::ListState {
+        // Delegate to our existing method, default to Idle on error
+        self.get_state(key).unwrap_or_default()
+    }
 }
 
 /// Pagination info for a list.
diff --git a/wp_mobile/src/sync/list_metadata_store.rs b/wp_mobile/src/sync/list_metadata_store.rs
@@ -7,11 +7,60 @@ use super::EntityMetadata;
 ///
 /// This trait allows components (like `MetadataCollection`) to read list structure
 /// without being able to modify it. Only the service layer should write metadata.
+///
+/// # Relevance Checking
+///
+/// The trait also provides methods for checking if database update hooks are relevant
+/// to a specific collection. These are used to implement split observers for data vs
+/// state updates.
+///
+/// Default implementations return `false` (safe for in-memory stores that don't support
+/// these checks). Database-backed implementations override with actual checks.
 pub trait ListMetadataReader: Send + Sync {
     /// Get the metadata list for a filter key.
     ///
     /// Returns `None` if no metadata has been stored for this key.
     fn get(&self, key: &str) -> Option<Vec<EntityMetadata>>;
+
+    /// Get the list_metadata_id (database rowid) for a given key.
+    ///
+    /// Returns `None` if no list exists for this key yet, or if this is an
+    /// in-memory implementation that doesn't support this operation.
+    ///
+    /// Used by collections to cache the ID for efficient state update matching.
+    fn get_list_metadata_id(&self, _key: &str) -> Option<i64> {
+        None
+    }
+
+    /// Check if a list_metadata_items row belongs to a specific key.
+    ///
+    /// Given a rowid from the list_metadata_items table (from an UpdateHook),
+    /// returns true if that item row belongs to the given key.
+    ///
+    /// Default implementation returns `false` (in-memory stores don't track row IDs).
+    fn is_item_row_for_key(&self, _item_row_id: i64, _key: &str) -> bool {
+        false
+    }
+
+    /// Check if a list_metadata_state row belongs to a specific list_metadata_id.
+    ///
+    /// Given a rowid from the list_metadata_state table (from an UpdateHook),
+    /// returns true if that state row belongs to the given list_metadata_id.
+    ///
+    /// Default implementation returns `false` (in-memory stores don't track row IDs).
+    fn is_state_row_for_list(&self, _state_row_id: i64, _list_metadata_id: i64) -> bool {
+        false
+    }
+
+    /// Get the current sync state for a list.
+    ///
+    /// Returns the current `ListState` (Idle, FetchingFirstPage, FetchingNextPage, Error).
+    /// Used by UI to show loading indicators or error states.
+    ///
+    /// Default implementation returns `Idle` (in-memory stores don't track state).
+    fn get_sync_state(&self, _key: &str) -> wp_mobile_cache::list_metadata::ListState {
+        wp_mobile_cache::list_metadata::ListState::Idle
+    }
 }
 
 /// Store for list metadata (entity IDs + modified timestamps per filter).
diff --git a/wp_mobile/src/sync/metadata_collection.rs b/wp_mobile/src/sync/metadata_collection.rs
@@ -1,6 +1,6 @@
 use std::sync::{Arc, RwLock};
 
-use wp_mobile_cache::UpdateHook;
+use wp_mobile_cache::{DbTable, UpdateHook};
 
 use crate::collection::FetchError;
 
@@ -14,6 +14,13 @@ struct PaginationState {
     per_page: u32,
 }
 
+/// Cached state for relevance checking.
+#[derive(Debug, Default)]
+struct RelevanceCache {
+    /// Cached list_metadata_id (populated lazily on first state relevance check)
+    list_metadata_id: Option<i64>,
+}
+
 /// Collection that uses metadata-first fetching strategy.
 ///
 /// This collection type:
@@ -72,11 +79,14 @@ where
     /// Fetcher for metadata and full entities
     fetcher: F,
 
-    /// Tables to monitor for relevant updates
-    relevant_tables: Vec<wp_mobile_cache::DbTable>,
+    /// Tables to monitor for data updates (entity tables like PostsEditContext)
+    relevant_data_tables: Vec<DbTable>,
 
     /// Pagination state (uses interior mutability for UniFFI compatibility)
     pagination: RwLock<PaginationState>,
+
+    /// Cached state for relevance checking
+    relevance_cache: RwLock<RelevanceCache>,
 }
 
 impl<F> MetadataCollection<F>
@@ -90,25 +100,26 @@ where
     /// * `metadata_reader` - Read-only access to list metadata store
     /// * `state_reader` - Read-only access to entity state store
     /// * `fetcher` - Implementation for fetching metadata and entities
-    /// * `relevant_tables` - DB tables to monitor for updates
+    /// * `relevant_data_tables` - DB tables to monitor for data updates (entity tables)
     pub fn new(
         kv_key: String,
         metadata_reader: Arc<dyn ListMetadataReader>,
         state_reader: Arc<dyn EntityStateReader>,
         fetcher: F,
-        relevant_tables: Vec<wp_mobile_cache::DbTable>,
+        relevant_data_tables: Vec<DbTable>,
     ) -> Self {
         Self {
             kv_key,
             metadata_reader,
             state_reader,
             fetcher,
-            relevant_tables,
+            relevant_data_tables,
             pagination: RwLock::new(PaginationState {
                 current_page: 0,
                 total_pages: None,
                 per_page: 20,
             }),
+            relevance_cache: RwLock::new(RelevanceCache::default()),
         }
     }
 
@@ -135,12 +146,86 @@ where
             .collect()
     }
 
-    /// Check if a database update is relevant to this collection.
+    /// Get the current sync state for this collection.
+    ///
+    /// Returns the current `ListState`:
+    /// - `Idle` - No sync in progress
+    /// - `FetchingFirstPage` - Refresh in progress
+    /// - `FetchingNextPage` - Load more in progress
+    /// - `Error` - Last sync failed
+    ///
+    /// Use this to show loading indicators in the UI.
+    pub fn sync_state(&self) -> wp_mobile_cache::list_metadata::ListState {
+        self.metadata_reader.get_sync_state(&self.kv_key)
+    }
+
+    /// Check if a database update is relevant to this collection (either data or state).
     ///
-    /// Returns `true` if the update is to a table this collection monitors.
-    /// Platform layers use this to determine when to notify observers.
+    /// Returns `true` if the update affects either data or state.
+    /// For more granular control, use `is_relevant_data_update` or `is_relevant_state_update`.
     pub fn is_relevant_update(&self, hook: &UpdateHook) -> bool {
-        self.relevant_tables.contains(&hook.table)
+        self.is_relevant_data_update(hook) || self.is_relevant_state_update(hook)
+    }
+
+    /// Check if a database update affects this collection's data.
+    ///
+    /// Returns `true` if the update is to:
+    /// - An entity table this collection monitors (e.g., PostsEditContext, TermRelationships)
+    /// - The ListMetadataItems table for this collection's key
+    ///
+    /// Use this for data observers that should refresh list contents.
+    pub fn is_relevant_data_update(&self, hook: &UpdateHook) -> bool {
+        // Check entity tables
+        if self.relevant_data_tables.contains(&hook.table) {
+            return true;
+        }
+
+        // Check ListMetadataItems for this specific key
+        if hook.table == DbTable::ListMetadataItems {
+            return self
+                .metadata_reader
+                .is_item_row_for_key(hook.row_id, &self.kv_key);
+        }
+
+        false
+    }
+
+    /// Check if a database update affects this collection's sync state.
+    ///
+    /// Returns `true` if the update is to the ListMetadataState table
+    /// for this collection's specific list.
+    ///
+    /// Use this for state observers that should update loading indicators.
+    pub fn is_relevant_state_update(&self, hook: &UpdateHook) -> bool {
+        if hook.table != DbTable::ListMetadataState {
+            return false;
+        }
+
+        // Get or cache the list_metadata_id
+        let list_metadata_id = {
+            let cache = self.relevance_cache.read().unwrap();
+            cache.list_metadata_id
+        };
+
+        let list_metadata_id = match list_metadata_id {
+            Some(id) => id,
+            None => {
+                // Try to get from database
+                let id = self.metadata_reader.get_list_metadata_id(&self.kv_key);
+                if let Some(id) = id {
+                    // Cache for next time
+                    self.relevance_cache.write().unwrap().list_metadata_id = Some(id);
+                    id
+                } else {
+                    // List doesn't exist yet, so this state update isn't for us
+                    return false;
+                }
+            }
+        };
+
+        // Check if the state row belongs to our list
+        self.metadata_reader
+            .is_state_row_for_list(hook.row_id, list_metadata_id)
     }
 
     /// Refresh the collection (fetch page 1, replace metadata).
diff --git a/wp_mobile_cache/src/repository/list_metadata.rs b/wp_mobile_cache/src/repository/list_metadata.rs
