Skip to content

Commit 1d709e7

Browse files
committed
Simplify ListMetadataReader trait with combined ListInfo query
Consolidate 6 separate trait methods into 2: - `get_list_info()` returns pagination + state via single JOIN query - `get_items()` returns entity metadata (renamed from `get()`) This eliminates redundant database queries when accessing multiple pagination fields (e.g., `has_more_pages()` previously made 2 queries). Changes: - Add `ListInfo` struct combining state, pagination fields - Add `DbListHeaderWithState` for database layer - Add `get_header_with_state()` JOIN query to repository - Update `MetadataCollection` to use `list_info()` for all pagination access - Add new tests for `get_list_info()` trait method
1 parent ec970ff commit 1d709e7

File tree

7 files changed

+206
-79
lines changed

7 files changed

+206
-79
lines changed

wp_mobile/src/service/metadata.rs

Lines changed: 58 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use wp_mobile_cache::{
1010
},
1111
};
1212

13-
use crate::sync::{EntityMetadata, ListMetadataReader};
13+
use crate::sync::{EntityMetadata, ListInfo, ListMetadataReader};
1414

1515
use super::WpServiceError;
1616

@@ -313,43 +313,23 @@ impl MetadataService {
313313
/// This allows MetadataCollection to read list structure from the database
314314
/// through the same trait interface it uses for in-memory stores.
315315
impl ListMetadataReader for MetadataService {
316-
fn get(&self, key: &str) -> Option<Vec<EntityMetadata>> {
317-
self.get_metadata(key).ok().flatten()
318-
}
319-
320-
fn get_sync_state(&self, key: &str) -> wp_mobile_cache::list_metadata::ListState {
321-
// Delegate to our existing method, default to Idle on error
322-
self.get_state(key).unwrap_or_default()
323-
}
324-
325-
fn get_current_page(&self, key: &str) -> i64 {
326-
self.get_pagination(key)
327-
.ok()
328-
.flatten()
329-
.map(|p| p.current_page)
330-
.unwrap_or(0)
331-
}
332-
333-
fn get_total_pages(&self, key: &str) -> Option<i64> {
334-
self.get_pagination(key)
335-
.ok()
336-
.flatten()
337-
.and_then(|p| p.total_pages)
338-
}
339-
340-
fn get_total_items(&self, key: &str) -> Option<i64> {
341-
self.get_pagination(key)
316+
fn get_list_info(&self, key: &str) -> Option<ListInfo> {
317+
self.cache
318+
.execute(|conn| self.repo.get_header_with_state(conn, &self.db_site, key))
342319
.ok()
343320
.flatten()
344-
.and_then(|p| p.total_items)
321+
.map(|db| ListInfo {
322+
state: db.state,
323+
error_message: db.error_message,
324+
current_page: db.current_page,
325+
total_pages: db.total_pages,
326+
total_items: db.total_items,
327+
per_page: db.per_page,
328+
})
345329
}
346330

347-
fn get_per_page(&self, key: &str) -> i64 {
348-
self.get_pagination(key)
349-
.ok()
350-
.flatten()
351-
.map(|p| p.per_page)
352-
.unwrap_or(20)
331+
fn get_items(&self, key: &str) -> Option<Vec<EntityMetadata>> {
332+
self.get_metadata(key).ok().flatten()
353333
}
354334
}
355335

@@ -609,7 +589,7 @@ mod tests {
609589
}
610590

611591
#[rstest]
612-
fn test_list_metadata_reader_trait(test_ctx: TestContext) {
592+
fn test_list_metadata_reader_get_items(test_ctx: TestContext) {
613593
let key = "edit:posts:publish";
614594
let metadata = vec![
615595
EntityMetadata::new(100, None, None, None),
@@ -620,16 +600,56 @@ mod tests {
620600

621601
// Access via trait
622602
let reader: &dyn ListMetadataReader = &test_ctx.service;
623-
let result = reader.get(key).unwrap();
603+
let result = reader.get_items(key).unwrap();
624604

625605
assert_eq!(result.len(), 2);
626606
assert_eq!(result[0].id, 100);
627607
assert_eq!(result[1].id, 200);
628608
}
629609

630610
#[rstest]
631-
fn test_list_metadata_reader_returns_none_for_non_existent(test_ctx: TestContext) {
611+
fn test_list_metadata_reader_get_items_returns_none_for_non_existent(test_ctx: TestContext) {
612+
let reader: &dyn ListMetadataReader = &test_ctx.service;
613+
assert!(reader.get_items("nonexistent").is_none());
614+
}
615+
616+
#[rstest]
617+
fn test_list_metadata_reader_get_list_info(test_ctx: TestContext) {
618+
let key = "edit:posts:publish";
619+
620+
// Initially no info
621+
let reader: &dyn ListMetadataReader = &test_ctx.service;
622+
assert!(reader.get_list_info(key).is_none());
623+
624+
// Create header via update_pagination (this creates the list metadata entry)
625+
test_ctx
626+
.service
627+
.update_pagination(key, Some(5), Some(100), 1, 20)
628+
.unwrap();
629+
630+
let info = reader.get_list_info(key).unwrap();
631+
assert_eq!(info.current_page, 1);
632+
assert_eq!(info.per_page, 20);
633+
assert_eq!(info.total_pages, Some(5));
634+
assert_eq!(info.total_items, Some(100));
635+
assert_eq!(info.state, wp_mobile_cache::list_metadata::ListState::Idle);
636+
}
637+
638+
#[rstest]
639+
fn test_list_metadata_reader_get_list_info_with_state(test_ctx: TestContext) {
640+
let key = "edit:posts:publish";
641+
let metadata = vec![EntityMetadata::new(100, None, None, None)];
642+
test_ctx.service.set_items(key, &metadata).unwrap();
643+
644+
// Start a refresh
645+
test_ctx.service.begin_refresh(key).unwrap();
646+
632647
let reader: &dyn ListMetadataReader = &test_ctx.service;
633-
assert!(reader.get("nonexistent").is_none());
648+
let info = reader.get_list_info(key).unwrap();
649+
650+
assert_eq!(
651+
info.state,
652+
wp_mobile_cache::list_metadata::ListState::FetchingFirstPage
653+
);
634654
}
635655
}
Lines changed: 26 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,38 +1,38 @@
11
use super::EntityMetadata;
2+
use wp_mobile_cache::list_metadata::ListState;
3+
4+
/// Combined list information: pagination + sync state.
5+
///
6+
/// Returned by a single JOIN query on `list_metadata` + `list_metadata_state` tables.
7+
#[derive(Debug, Clone, PartialEq, Eq)]
8+
pub struct ListInfo {
9+
/// Current sync state (Idle, FetchingFirstPage, FetchingNextPage, Error)
10+
pub state: ListState,
11+
/// Error message if state is Error
12+
pub error_message: Option<String>,
13+
/// Current page that has been loaded (0 = no pages loaded)
14+
pub current_page: i64,
15+
/// Total number of pages from API response
16+
pub total_pages: Option<i64>,
17+
/// Total number of items from API response
18+
pub total_items: Option<i64>,
19+
/// Items per page
20+
pub per_page: i64,
21+
}
222

323
/// Read-only access to list metadata.
424
///
525
/// This trait allows components (like `MetadataCollection`) to read list structure
626
/// without being able to modify it. Only the service layer should write metadata.
727
pub trait ListMetadataReader: Send + Sync {
8-
/// Get the metadata list for a filter key.
28+
/// Get list info (pagination + state) in a single query.
929
///
1030
/// Returns `None` if no metadata has been stored for this key.
11-
fn get(&self, key: &str) -> Option<Vec<EntityMetadata>>;
12-
13-
/// Get the current sync state for a list.
14-
///
15-
/// Returns the current `ListState` (Idle, FetchingFirstPage, FetchingNextPage, Error).
16-
/// Used by UI to show loading indicators or error states.
17-
fn get_sync_state(&self, key: &str) -> wp_mobile_cache::list_metadata::ListState;
18-
19-
/// Get the current page number for a list.
20-
///
21-
/// Returns 0 if no pages have been fetched yet.
22-
fn get_current_page(&self, key: &str) -> i64;
23-
24-
/// Get the total number of pages for a list.
25-
///
26-
/// Returns `None` if unknown (no fetch has completed yet).
27-
fn get_total_pages(&self, key: &str) -> Option<i64>;
31+
fn get_list_info(&self, key: &str) -> Option<ListInfo>;
2832

29-
/// Get the total number of items for a list.
33+
/// Get the items for a list.
3034
///
31-
/// Returns `None` if unknown (no fetch has completed yet).
32-
fn get_total_items(&self, key: &str) -> Option<i64>;
33-
34-
/// Get the items per page setting for a list.
35-
///
36-
/// Returns the configured per_page value, or a default if not set.
37-
fn get_per_page(&self, key: &str) -> i64;
35+
/// Returns `None` if no metadata has been stored for this key.
36+
/// Returns `Some(vec![])` if the list exists but has no items.
37+
fn get_items(&self, key: &str) -> Option<Vec<EntityMetadata>>;
3838
}

wp_mobile/src/sync/metadata_collection.rs

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@ use wp_mobile_cache::{DbTable, UpdateHook};
44

55
use crate::collection::FetchError;
66

7-
use super::{CollectionItem, EntityStateReader, ListMetadataReader, MetadataFetcher, SyncResult};
7+
use super::{
8+
CollectionItem, EntityStateReader, ListInfo, ListMetadataReader, MetadataFetcher, SyncResult,
9+
};
810

911
/// Collection that uses metadata-first fetching strategy.
1012
///
@@ -114,7 +116,7 @@ where
114116
/// the metadata with the current fetch state.
115117
pub fn items(&self) -> Vec<CollectionItem> {
116118
self.metadata_reader
117-
.get(&self.kv_key)
119+
.get_items(&self.kv_key)
118120
.unwrap_or_default()
119121
.into_iter()
120122
.map(|metadata| {
@@ -123,6 +125,13 @@ where
123125
.collect()
124126
}
125127

128+
/// Get the combined list info (pagination + sync state) in a single query.
129+
///
130+
/// Returns `None` if no metadata has been stored for this key.
131+
pub fn list_info(&self) -> Option<ListInfo> {
132+
self.metadata_reader.get_list_info(&self.kv_key)
133+
}
134+
126135
/// Get the current sync state for this collection.
127136
///
128137
/// Returns the current `ListState`:
@@ -133,7 +142,7 @@ where
133142
///
134143
/// Use this to show loading indicators in the UI.
135144
pub fn sync_state(&self) -> wp_mobile_cache::list_metadata::ListState {
136-
self.metadata_reader.get_sync_state(&self.kv_key)
145+
self.list_info().map(|info| info.state).unwrap_or_default()
137146
}
138147

139148
/// Check if a database update is relevant to this collection (either data or state).
@@ -269,28 +278,28 @@ where
269278

270279
/// Check if there are more pages to load.
271280
pub fn has_more_pages(&self) -> bool {
272-
let current_page = self.current_page();
273-
let total_pages = self.total_pages();
274-
total_pages
275-
.map(|total| current_page < total)
276-
.unwrap_or(true) // Unknown total = assume more pages
281+
self.list_info()
282+
.and_then(|info| info.total_pages.map(|total| info.current_page < total))
283+
.unwrap_or(true) // Unknown total or no info = assume more pages
277284
}
278285

279286
/// Get the current page number (0 = not loaded yet).
280287
pub fn current_page(&self) -> u32 {
281-
self.metadata_reader.get_current_page(&self.kv_key) as u32
288+
self.list_info()
289+
.map(|info| info.current_page as u32)
290+
.unwrap_or(0)
282291
}
283292

284293
/// Get the total number of pages, if known.
285294
pub fn total_pages(&self) -> Option<u32> {
286-
self.metadata_reader
287-
.get_total_pages(&self.kv_key)
295+
self.list_info()
296+
.and_then(|info| info.total_pages)
288297
.map(|p| p as u32)
289298
}
290299

291300
/// Get the total number of items, if known.
292301
pub fn total_items(&self) -> Option<i64> {
293-
self.metadata_reader.get_total_items(&self.kv_key)
302+
self.list_info().and_then(|info| info.total_items)
294303
}
295304

296305
/// Fetch missing and stale items.

wp_mobile/src/sync/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ pub use collection_item::CollectionItem;
4545
pub use entity_metadata::EntityMetadata;
4646
pub use entity_state::EntityState;
4747
pub use entity_state_store::{EntityStateReader, EntityStateStore};
48-
pub use list_metadata_reader::ListMetadataReader;
48+
pub use list_metadata_reader::{ListInfo, ListMetadataReader};
4949
pub use metadata_collection::MetadataCollection;
5050
pub use metadata_fetch_result::MetadataFetchResult;
5151
pub use metadata_fetcher::MetadataFetcher;

wp_mobile_cache/src/db_types/db_list_metadata.rs

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
use crate::{
22
SqliteDbError,
33
db_types::row_ext::{ColumnIndex, RowExt},
4-
list_metadata::{DbListMetadata, DbListMetadataItem, DbListMetadataState, ListState},
4+
list_metadata::{
5+
DbListHeaderWithState, DbListMetadata, DbListMetadataItem, DbListMetadataState, ListState,
6+
},
57
};
68
use rusqlite::Row;
79

@@ -119,3 +121,46 @@ impl DbListMetadataState {
119121
})
120122
}
121123
}
124+
125+
/// Column indexes for the header + state JOIN query.
126+
///
127+
/// Query: SELECT m.total_pages, m.total_items, m.current_page, m.per_page, s.state, s.error_message
128+
/// FROM list_metadata m LEFT JOIN list_metadata_state s ON ...
129+
#[repr(usize)]
130+
#[derive(Debug, Clone, Copy)]
131+
pub enum ListHeaderWithStateColumn {
132+
TotalPages = 0,
133+
TotalItems = 1,
134+
CurrentPage = 2,
135+
PerPage = 3,
136+
State = 4,
137+
ErrorMessage = 5,
138+
}
139+
140+
impl ColumnIndex for ListHeaderWithStateColumn {
141+
fn as_index(&self) -> usize {
142+
*self as usize
143+
}
144+
}
145+
146+
impl DbListHeaderWithState {
147+
/// Construct from a JOIN query row.
148+
///
149+
/// Expects columns in order: total_pages, total_items, current_page, per_page, state, error_message
150+
pub fn from_row(row: &Row) -> Result<Self, SqliteDbError> {
151+
use ListHeaderWithStateColumn as Col;
152+
153+
// state is nullable due to LEFT JOIN - default to "idle"
154+
let state_str: Option<String> = row.get_column(Col::State)?;
155+
let state = state_str.map(ListState::from).unwrap_or(ListState::Idle);
156+
157+
Ok(Self {
158+
state,
159+
error_message: row.get_column(Col::ErrorMessage)?,
160+
current_page: row.get_column(Col::CurrentPage)?,
161+
total_pages: row.get_column(Col::TotalPages)?,
162+
total_items: row.get_column(Col::TotalItems)?,
163+
per_page: row.get_column(Col::PerPage)?,
164+
})
165+
}
166+
}

wp_mobile_cache/src/list_metadata.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,3 +110,22 @@ impl From<String> for ListState {
110110
ListState::from(s.as_str())
111111
}
112112
}
113+
114+
/// Combined header + state from a JOIN query.
115+
///
116+
/// Contains pagination info from `list_metadata` and sync state from `list_metadata_state`.
117+
#[derive(Debug, Clone, PartialEq, Eq)]
118+
pub struct DbListHeaderWithState {
119+
/// Current sync state (defaults to Idle if no state record exists)
120+
pub state: ListState,
121+
/// Error message if state is Error
122+
pub error_message: Option<String>,
123+
/// Current page that has been loaded (0 = no pages loaded)
124+
pub current_page: i64,
125+
/// Total number of pages from API response
126+
pub total_pages: Option<i64>,
127+
/// Total number of items from API response
128+
pub total_items: Option<i64>,
129+
/// Items per page
130+
pub per_page: i64,
131+
}

0 commit comments

Comments
 (0)