From e8115914c653995769a6aee4005d451c0ff50df8 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 13 May 2026 13:48:39 +1200 Subject: [PATCH 01/18] Add media_edit_context table and DbTable variant --- .../0014-create-media-edit-context-table.sql | 56 ++++++++++++++++++ wp_mobile_cache/src/db_types.rs | 1 + wp_mobile_cache/src/db_types/media/edit.rs | 57 +++++++++++++++++++ wp_mobile_cache/src/db_types/media/mod.rs | 4 ++ wp_mobile_cache/src/lib.rs | 7 ++- 5 files changed, 124 insertions(+), 1 deletion(-) create mode 100644 wp_mobile_cache/migrations/0014-create-media-edit-context-table.sql create mode 100644 wp_mobile_cache/src/db_types/media/edit.rs create mode 100644 wp_mobile_cache/src/db_types/media/mod.rs diff --git a/wp_mobile_cache/migrations/0014-create-media-edit-context-table.sql b/wp_mobile_cache/migrations/0014-create-media-edit-context-table.sql new file mode 100644 index 000000000..dcc4b2ad6 --- /dev/null +++ b/wp_mobile_cache/migrations/0014-create-media-edit-context-table.sql @@ -0,0 +1,56 @@ +CREATE TABLE `media_edit_context` ( + `rowid` INTEGER PRIMARY KEY AUTOINCREMENT, + `db_site_id` INTEGER NOT NULL REFERENCES db_sites(id) ON DELETE CASCADE, + + -- Top-level fields (mirror MediaWithEditContext) + `id` INTEGER NOT NULL, + `date` TEXT NOT NULL, + `date_gmt` TEXT NOT NULL, + `link` TEXT NOT NULL, + `modified` TEXT NOT NULL, + `modified_gmt` TEXT NOT NULL, + `slug` TEXT NOT NULL, + `status` TEXT NOT NULL, + `post_type` TEXT NOT NULL, + `password` TEXT, + `permalink_template` TEXT NOT NULL, + `generated_slug` TEXT NOT NULL, + `author` INTEGER NOT NULL, + `comment_status` TEXT NOT NULL, + `ping_status` TEXT NOT NULL, + `template` TEXT NOT NULL, + `alt_text` TEXT NOT NULL, + `media_type` TEXT NOT NULL, + `mime_type` TEXT NOT NULL, + `source_url` TEXT NOT NULL, + `post_id` INTEGER, + + -- Required list field (stored as JSON to keep migrations simple) + `missing_image_sizes` TEXT NOT NULL, + + -- Nested guid (raw is optional, rendered is required in edit context) + `guid_raw` TEXT, + `guid_rendered` TEXT NOT NULL, + + -- Nested title (rendered is required, raw is optional) + `title_raw` TEXT, + `title_rendered` TEXT NOT NULL, + + -- Nested caption / description (both inner fields are non-optional Strings in edit context) + `caption_raw` TEXT NOT NULL, + `caption_rendered` TEXT NOT NULL, + `description_raw` TEXT NOT NULL, + `description_rendered` TEXT NOT NULL, + + -- Opaque media_details payload, stored as raw JSON, parsed lazily on read. + `media_details` TEXT NOT NULL, + + -- Cache metadata + `last_fetched_at` TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')), + `additional_fields` TEXT, + + FOREIGN KEY (db_site_id) REFERENCES db_sites(id) ON DELETE CASCADE +) STRICT; + +CREATE UNIQUE INDEX idx_media_edit_context_unique_db_site_id_and_id ON media_edit_context(db_site_id, id); +CREATE INDEX idx_media_edit_context_db_site_id ON media_edit_context(db_site_id); diff --git a/wp_mobile_cache/src/db_types.rs b/wp_mobile_cache/src/db_types.rs index de2d2d63c..1b23dbd42 100644 --- a/wp_mobile_cache/src/db_types.rs +++ b/wp_mobile_cache/src/db_types.rs @@ -2,6 +2,7 @@ pub mod db_list_metadata; pub mod db_site; pub mod db_term_relationship; pub mod helpers; +pub mod media; pub mod post_types; pub mod posts; pub mod row_ext; diff --git a/wp_mobile_cache/src/db_types/media/edit.rs b/wp_mobile_cache/src/db_types/media/edit.rs new file mode 100644 index 000000000..19539147b --- /dev/null +++ b/wp_mobile_cache/src/db_types/media/edit.rs @@ -0,0 +1,57 @@ +use crate::{RowId, db_types::row_ext::ColumnIndex}; +use wp_api::media::MediaWithEditContext; + +/// Column indexes for media_edit_context table. +/// These must match the order of columns in the CREATE TABLE statement. +#[repr(usize)] +#[derive(Debug, Clone, Copy)] +pub(crate) enum MediaEditContextColumn { + Rowid = 0, + DbSiteId = 1, + Id = 2, + Date = 3, + DateGmt = 4, + Link = 5, + Modified = 6, + ModifiedGmt = 7, + Slug = 8, + Status = 9, + PostType = 10, + Password = 11, + PermalinkTemplate = 12, + GeneratedSlug = 13, + Author = 14, + CommentStatus = 15, + PingStatus = 16, + Template = 17, + AltText = 18, + MediaType = 19, + MimeType = 20, + SourceUrl = 21, + PostId = 22, + MissingImageSizes = 23, + GuidRaw = 24, + GuidRendered = 25, + TitleRaw = 26, + TitleRendered = 27, + CaptionRaw = 28, + CaptionRendered = 29, + DescriptionRaw = 30, + DescriptionRendered = 31, + MediaDetails = 32, + LastFetchedAt = 33, + AdditionalFields = 34, +} + +impl ColumnIndex for MediaEditContextColumn { + fn as_index(&self) -> usize { + *self as usize + } +} + +pub struct DbMediaWithEditContext { + pub row_id: RowId, + pub db_site_id: RowId, + pub media: MediaWithEditContext, + pub last_fetched_at: String, +} diff --git a/wp_mobile_cache/src/db_types/media/mod.rs b/wp_mobile_cache/src/db_types/media/mod.rs new file mode 100644 index 000000000..f74189f93 --- /dev/null +++ b/wp_mobile_cache/src/db_types/media/mod.rs @@ -0,0 +1,4 @@ +mod edit; + +pub use edit::DbMediaWithEditContext; +pub(crate) use edit::MediaEditContextColumn; diff --git a/wp_mobile_cache/src/lib.rs b/wp_mobile_cache/src/lib.rs index 121a72874..84e094d7e 100644 --- a/wp_mobile_cache/src/lib.rs +++ b/wp_mobile_cache/src/lib.rs @@ -85,6 +85,8 @@ pub enum DbTable { PostsEmbedContext, /// Post types with edit context (post type configuration data) PostTypesEditContext, + /// Media with edit context (full media data for editing) + MediaEditContext, /// Self-hosted WordPress sites SelfHostedSites, /// Database sites mapping table @@ -114,6 +116,7 @@ impl DbTable { DbTable::PostsViewContext => "posts_view_context", DbTable::PostsEmbedContext => "posts_embed_context", DbTable::PostTypesEditContext => "post_types_edit_context", + DbTable::MediaEditContext => "media_edit_context", DbTable::SelfHostedSites => "self_hosted_sites", DbTable::DbSites => "db_sites", DbTable::TermRelationships => "term_relationships", @@ -148,6 +151,7 @@ impl TryFrom<&str> for DbTable { "posts_view_context" => Ok(DbTable::PostsViewContext), "posts_embed_context" => Ok(DbTable::PostsEmbedContext), "post_types_edit_context" => Ok(DbTable::PostTypesEditContext), + "media_edit_context" => Ok(DbTable::MediaEditContext), "self_hosted_sites" => Ok(DbTable::SelfHostedSites), "db_sites" => Ok(DbTable::DbSites), "term_relationships" => Ok(DbTable::TermRelationships), @@ -444,7 +448,7 @@ impl TryFrom for WpApiCache { } } -static MIGRATION_QUERIES: [&str; 13] = [ +static MIGRATION_QUERIES: [&str; 14] = [ include_str!("../migrations/0001-create-sites-table.sql"), include_str!("../migrations/0002-create-posts-table.sql"), include_str!("../migrations/0003-create-term-relationships.sql"), @@ -458,6 +462,7 @@ static MIGRATION_QUERIES: [&str; 13] = [ include_str!("../migrations/0011-add-additional-fields-to-posts-tables.sql"), include_str!("../migrations/0012-invalidate-post-entity-states.sql"), include_str!("../migrations/0013-invalidate-post-entity-states-for-meta.sql"), + include_str!("../migrations/0014-create-media-edit-context-table.sql"), ]; pub struct MigrationManager<'a> { From 139b8c85f6a0d06d5f4ae20900ad01de535e73a9 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 13 May 2026 13:54:12 +1200 Subject: [PATCH 02/18] Add MediaEditContext EntityType variant --- wp_mobile_cache/src/repository/entity_state.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/wp_mobile_cache/src/repository/entity_state.rs b/wp_mobile_cache/src/repository/entity_state.rs index ff6888a56..1521335c8 100644 --- a/wp_mobile_cache/src/repository/entity_state.rs +++ b/wp_mobile_cache/src/repository/entity_state.rs @@ -19,6 +19,8 @@ use std::collections::HashMap; pub enum EntityType { /// Posts with edit context (table: posts_edit_context) PostsEditContext = 0, + /// Media with edit context (table: media_edit_context) + MediaEditContext = 1, } impl EntityType { @@ -29,6 +31,7 @@ impl EntityType { pub fn table_name(self) -> &'static str { match self { EntityType::PostsEditContext => "posts_edit_context", + EntityType::MediaEditContext => "media_edit_context", } } } @@ -43,6 +46,7 @@ impl FromSql for EntityType { fn column_result(value: rusqlite::types::ValueRef<'_>) -> FromSqlResult { i64::column_result(value).and_then(|i| match i { 0 => Ok(EntityType::PostsEditContext), + 1 => Ok(EntityType::MediaEditContext), _ => Err(FromSqlError::OutOfRange(i)), }) } @@ -475,6 +479,20 @@ mod tests { assert_eq!(EntityType::PostsEditContext as i64, 0); } + #[test] + fn test_entity_type_media_edit_context_sql_round_trip() { + let conn = Connection::open_in_memory().unwrap(); + // discriminant guard: ensure on-disk value doesn't drift + assert_eq!(EntityType::MediaEditContext as i64, 1); + + // SQL round-trip: i64 -> EntityType + let value: i64 = 1; + let result: EntityType = conn + .query_row("SELECT ?", [value], |row| row.get(0)) + .unwrap(); + assert_eq!(result, EntityType::MediaEditContext); + } + #[test] fn test_filter_fetchable_logic() { let (conn, db_site) = setup_test_db(); From 46f12498e2096aef8870b21df85096a40eb87022 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 13 May 2026 14:03:46 +1200 Subject: [PATCH 03/18] Add MediaRepository with edit-context upsert/select/delete --- wp_mobile_cache/src/repository/media.rs | 665 ++++++++++++++++++++++++ wp_mobile_cache/src/repository/mod.rs | 1 + 2 files changed, 666 insertions(+) create mode 100644 wp_mobile_cache/src/repository/media.rs diff --git a/wp_mobile_cache/src/repository/media.rs b/wp_mobile_cache/src/repository/media.rs new file mode 100644 index 000000000..f08d727e0 --- /dev/null +++ b/wp_mobile_cache/src/repository/media.rs @@ -0,0 +1,665 @@ +use crate::{ + DbTable, RowId, SqliteDbError, + context::{EditContext, IsContext}, + db_types::{ + db_site::DbSite, + helpers::{ + deserialize_json_value, get_id, get_optional_id, parse_datetime, parse_enum, + serialize_value_to_json, + }, + media::{DbMediaWithEditContext, MediaEditContextColumn}, + row_ext::RowExt, + }, + entity::{EntityId, FullEntity}, + repository::{QueryExecutor, TransactionManager}, +}; +use rusqlite::{OptionalExtension, Row}; +use std::{collections::HashMap, marker::PhantomData, sync::Arc}; +use wp_api::{ + media::{ + MediaCaptionWithEditContext, MediaDescriptionWithEditContext, MediaId, MediaWithEditContext, + }, + posts::{PostGuidWithEditContext, PostTitleWithEditContext}, + prelude::WpGmtDateTime, +}; + +/// Entity-specific context trait for Media. +/// +/// Mirrors `PostContext` but omits the term-relationship preload: media has no +/// categories or tags, so `from_row` takes only `&Row`. +pub trait MediaContext: IsContext { + type Media; + type DbMedia; + + fn table() -> DbTable; + fn from_row(row: &Row) -> Result; + fn rowid(db: &Self::DbMedia) -> RowId; +} + +/// Repository for managing media in the database. +/// +/// Generic over `MediaContext`. Phase 0 only implements `EditContext`. +pub struct MediaRepository { + _phantom: PhantomData, +} + +impl Default for MediaRepository { + fn default() -> Self { + Self::new() + } +} + +impl MediaRepository { + pub fn new() -> Self { + Self { + _phantom: PhantomData, + } + } + + pub fn table_name() -> &'static str { + C::table().table_name() + } + + /// Select a media row by its EntityId. + /// + /// Returns an error if the EntityId's table doesn't match this repository's context. + pub fn select_by_entity_id( + &self, + executor: &impl QueryExecutor, + entity_id: &EntityId, + ) -> Result>, SqliteDbError> { + entity_id.validate_table(C::table())?; + + let sql = format!( + "SELECT * FROM {} WHERE db_site_id = ? AND rowid = ?", + Self::table_name() + ); + let mut stmt = executor.prepare(&sql)?; + let db_media = stmt + .query_row([entity_id.db_site.row_id, entity_id.rowid], |row| { + C::from_row(row).map_err(|e| rusqlite::Error::ToSqlConversionFailure(Box::new(e))) + }) + .optional() + .map_err(SqliteDbError::from)?; + + Ok(db_media.map(|db_media| { + let entity_id = Arc::new(*entity_id); + FullEntity::new(entity_id, db_media) + })) + } + + /// Select all media rows for a given site. + /// + /// Unlike posts, media has no equivalent of status-filtered queries at the repository + /// level (filtering happens at the API layer), so this is implemented directly without + /// a `select_by_filter` indirection. + pub fn select_all( + &self, + executor: &impl QueryExecutor, + site: &DbSite, + ) -> Result>, SqliteDbError> { + let sql = format!("SELECT * FROM {} WHERE db_site_id = ?", Self::table_name()); + let mut stmt = executor.prepare(&sql)?; + let rows = stmt + .query_map([site.row_id], |row| { + C::from_row(row).map_err(|e| rusqlite::Error::ToSqlConversionFailure(Box::new(e))) + })? + .collect::, _>>() + .map_err(SqliteDbError::from)?; + + Ok(rows + .into_iter() + .map(|db_media| { + let rowid = C::rowid(&db_media); + let entity_id = Arc::new(EntityId::new(*site, C::table(), rowid)); + FullEntity::new(entity_id, db_media) + }) + .collect()) + } + + /// Select a media row by its WordPress media ID for a given site. + pub fn select_by_media_id( + &self, + executor: &impl QueryExecutor, + site: &DbSite, + media_id: MediaId, + ) -> Result>, SqliteDbError> { + let sql = format!( + "SELECT * FROM {} WHERE db_site_id = ? AND id = ?", + Self::table_name() + ); + let mut stmt = executor.prepare(&sql)?; + let db_media = stmt + .query_row(rusqlite::params![site.row_id, media_id.0], |row| { + C::from_row(row).map_err(|e| rusqlite::Error::ToSqlConversionFailure(Box::new(e))) + }) + .optional() + .map_err(SqliteDbError::from)?; + + Ok(db_media.map(|db_media| { + let rowid = C::rowid(&db_media); + let entity_id = Arc::new(EntityId::new(*site, C::table(), rowid)); + FullEntity::new(entity_id, db_media) + })) + } + + /// Select `modified_gmt` timestamps for multiple media items by their WordPress media IDs. + /// + /// Lightweight query used for staleness detection; media not present in the cache are + /// omitted from the result. + pub fn select_modified_gmt_by_ids( + &self, + executor: &impl QueryExecutor, + site: &DbSite, + media_ids: &[MediaId], + ) -> Result, SqliteDbError> { + if media_ids.is_empty() { + return Ok(HashMap::new()); + } + + let ids_str = media_ids + .iter() + .map(|id| id.0.to_string()) + .collect::>() + .join(", "); + + let sql = format!( + "SELECT id, modified_gmt FROM {} WHERE db_site_id = ? AND id IN ({})", + Self::table_name(), + ids_str + ); + + let mut stmt = executor.prepare(&sql)?; + let rows = stmt.query_map([site.row_id], |row| { + let id: i64 = row.get(0)?; + let modified_gmt_str: String = row.get(1)?; + Ok((id, modified_gmt_str)) + })?; + + Ok(rows + .filter_map(|row_result| { + row_result.ok().and_then(|(id, modified_gmt_str)| { + modified_gmt_str + .parse::() + .ok() + .map(|modified_gmt| (MediaId(id), modified_gmt)) + }) + }) + .collect()) + } + + /// Delete a media row by its EntityId. + /// + /// Returns the number of rows deleted (0 or 1). Unlike posts, no term relationships + /// are involved. + pub fn delete_by_entity_id( + &self, + executor: &impl QueryExecutor, + entity_id: &EntityId, + ) -> Result { + entity_id.validate_table(C::table())?; + + let sql = format!( + "SELECT id FROM {} WHERE db_site_id = ? AND rowid = ?", + Self::table_name() + ); + let mut stmt = executor.prepare(&sql)?; + let media_id = stmt + .query_row([entity_id.db_site.row_id, entity_id.rowid], |row| { + row.get::<_, i64>(0) + }) + .optional() + .map_err(SqliteDbError::from)?; + + match media_id { + Some(id) => self.delete_by_media_id(executor, &entity_id.db_site, MediaId(id)), + None => Ok(0), + } + } + + /// Delete a media row by its WordPress media ID for a given site. + pub fn delete_by_media_id( + &self, + executor: &impl QueryExecutor, + site: &DbSite, + media_id: MediaId, + ) -> Result { + let sql = format!( + "DELETE FROM {} WHERE db_site_id = ? AND id = ?", + Self::table_name() + ); + executor.execute(&sql, rusqlite::params![site.row_id, media_id.0]) + } + + /// Get the total count of media rows for a given site. + pub fn count( + &self, + executor: &impl QueryExecutor, + site: &DbSite, + ) -> Result { + let sql = format!( + "SELECT COUNT(*) FROM {} WHERE db_site_id = ?", + Self::table_name() + ); + let mut stmt = executor.prepare(&sql)?; + stmt.query_row([site.row_id], |row| row.get(0)) + .map_err(SqliteDbError::from) + } +} + +impl MediaContext for EditContext { + type Media = MediaWithEditContext; + type DbMedia = DbMediaWithEditContext; + + fn table() -> DbTable { + DbTable::MediaEditContext + } + + fn from_row(row: &Row) -> Result { + use MediaEditContextColumn::*; + + let row_id: RowId = row.get_column(Rowid)?; + let db_site_id: RowId = row.get_column(MediaEditContextColumn::DbSiteId)?; + + // media_details is stored as raw JSON text and reconstructed into a `Box`. + // Parsing here keeps the on-disk payload byte-for-byte identical to what was written, + // while still validating that the stored value is syntactically valid JSON. + let media_details_json: String = row.get_column(MediaDetails)?; + let media_details_payload: Box = + serde_json::from_str(&media_details_json).map_err(|e| { + SqliteDbError::SqliteError(format!("Failed to parse media_details JSON: {}", e)) + })?; + let media_details = Arc::new(wp_api::media::MediaDetails { + payload: media_details_payload, + }); + + // missing_image_sizes is a non-optional Vec, persisted as a JSON array. + let missing_image_sizes_json: String = row.get_column(MissingImageSizes)?; + let missing_image_sizes: Vec = serde_json::from_str(&missing_image_sizes_json) + .map_err(|e| { + SqliteDbError::SqliteError(format!( + "Failed to parse missing_image_sizes JSON: {}", + e + )) + })?; + + let media = MediaWithEditContext { + id: get_id(row, Id)?, + date: row.get_column(Date)?, + date_gmt: parse_datetime(row, DateGmt)?, + guid: PostGuidWithEditContext { + raw: row.get_column(GuidRaw)?, + rendered: row.get_column(GuidRendered)?, + }, + link: row.get_column(Link)?, + modified: row.get_column(Modified)?, + modified_gmt: parse_datetime(row, ModifiedGmt)?, + slug: row.get_column(Slug)?, + status: parse_enum(row, Status)?, + post_type: row.get_column(PostType)?, + password: row.get_column(Password)?, + permalink_template: row.get_column(PermalinkTemplate)?, + generated_slug: row.get_column(GeneratedSlug)?, + title: PostTitleWithEditContext { + raw: row.get_column(TitleRaw)?, + rendered: row.get_column(TitleRendered)?, + }, + author: get_id(row, Author)?, + comment_status: parse_enum(row, CommentStatus)?, + ping_status: parse_enum(row, PingStatus)?, + template: row.get_column(Template)?, + alt_text: row.get_column(AltText)?, + caption: MediaCaptionWithEditContext { + raw: row.get_column(CaptionRaw)?, + rendered: row.get_column(CaptionRendered)?, + }, + description: MediaDescriptionWithEditContext { + raw: row.get_column(DescriptionRaw)?, + rendered: row.get_column(DescriptionRendered)?, + }, + media_type: parse_enum(row, MediaType)?, + mime_type: row.get_column(MimeType)?, + media_details, + post_id: get_optional_id(row, PostId)?, + source_url: row.get_column(SourceUrl)?, + missing_image_sizes, + }; + + // additional_fields is part of the schema for future use but is not currently + // exposed on MediaWithEditContext. We still read+discard it (via the migration + // column) so writes that include it round-trip cleanly via the upsert column list. + let _additional_fields: Option = + deserialize_json_value(row.get_column(AdditionalFields)?)?; + + Ok(DbMediaWithEditContext { + row_id, + db_site_id, + media, + last_fetched_at: row.get_column(LastFetchedAt)?, + }) + } + + fn rowid(db: &Self::DbMedia) -> RowId { + db.row_id + } +} + +impl MediaRepository { + /// Upsert a media row with edit context (atomic transaction). + /// + /// Uses a transaction even though only one table is touched, matching the posts + /// pattern and leaving room for future composability (e.g. associated metadata + /// tables) without changing the public signature. + pub fn upsert( + &self, + transaction_manager: &mut impl TransactionManager, + site: &DbSite, + media: &MediaWithEditContext, + ) -> Result { + let tx = transaction_manager.transaction()?; + + let missing_image_sizes_json = + serde_json::to_string(&media.missing_image_sizes).map_err(|e| { + SqliteDbError::SqliteError(format!( + "Failed to serialize missing_image_sizes: {}", + e + )) + })?; + + let upsert_sql = format!( + r#" + INSERT INTO {} ( + db_site_id, id, date, date_gmt, link, modified, modified_gmt, slug, status, post_type, + password, permalink_template, generated_slug, author, comment_status, ping_status, + template, alt_text, media_type, mime_type, source_url, post_id, missing_image_sizes, + guid_raw, guid_rendered, title_raw, title_rendered, + caption_raw, caption_rendered, description_raw, description_rendered, + media_details, additional_fields + ) VALUES ( + :db_site_id, :id, :date, :date_gmt, :link, :modified, :modified_gmt, :slug, :status, :post_type, + :password, :permalink_template, :generated_slug, :author, :comment_status, :ping_status, + :template, :alt_text, :media_type, :mime_type, :source_url, :post_id, :missing_image_sizes, + :guid_raw, :guid_rendered, :title_raw, :title_rendered, + :caption_raw, :caption_rendered, :description_raw, :description_rendered, + :media_details, :additional_fields + ) + ON CONFLICT(db_site_id, id) DO UPDATE SET + date = excluded.date, + date_gmt = excluded.date_gmt, + link = excluded.link, + modified = excluded.modified, + modified_gmt = excluded.modified_gmt, + slug = excluded.slug, + status = excluded.status, + post_type = excluded.post_type, + password = excluded.password, + permalink_template = excluded.permalink_template, + generated_slug = excluded.generated_slug, + author = excluded.author, + comment_status = excluded.comment_status, + ping_status = excluded.ping_status, + template = excluded.template, + alt_text = excluded.alt_text, + media_type = excluded.media_type, + mime_type = excluded.mime_type, + source_url = excluded.source_url, + post_id = excluded.post_id, + missing_image_sizes = excluded.missing_image_sizes, + guid_raw = excluded.guid_raw, + guid_rendered = excluded.guid_rendered, + title_raw = excluded.title_raw, + title_rendered = excluded.title_rendered, + caption_raw = excluded.caption_raw, + caption_rendered = excluded.caption_rendered, + description_raw = excluded.description_raw, + description_rendered = excluded.description_rendered, + media_details = excluded.media_details, + additional_fields = excluded.additional_fields, + last_fetched_at = strftime('%Y-%m-%dT%H:%M:%fZ', 'now') + RETURNING rowid + "#, + Self::table_name() + ); + + let no_additional_fields: Option = None; + + let media_rowid: i64 = tx + .query_row( + &upsert_sql, + rusqlite::named_params! { + ":db_site_id": site.row_id, + ":id": media.id.0, + ":date": media.date, + ":date_gmt": media.date_gmt.to_string(), + ":link": media.link, + ":modified": media.modified, + ":modified_gmt": media.modified_gmt.to_string(), + ":slug": media.slug, + ":status": media.status.to_string(), + ":post_type": media.post_type, + ":password": media.password.clone(), + ":permalink_template": media.permalink_template, + ":generated_slug": media.generated_slug, + ":author": media.author.0, + ":comment_status": media.comment_status.to_string(), + ":ping_status": media.ping_status.to_string(), + ":template": media.template, + ":alt_text": media.alt_text, + ":media_type": media.media_type.to_string(), + ":mime_type": media.mime_type, + ":source_url": media.source_url, + ":post_id": media.post_id.map(|p| p.0), + ":missing_image_sizes": missing_image_sizes_json, + ":guid_raw": media.guid.raw, + ":guid_rendered": media.guid.rendered, + ":title_raw": media.title.raw, + ":title_rendered": media.title.rendered, + ":caption_raw": media.caption.raw, + ":caption_rendered": media.caption.rendered, + ":description_raw": media.description.raw, + ":description_rendered": media.description.rendered, + ":media_details": media.media_details.payload.get(), + ":additional_fields": serialize_value_to_json(&no_additional_fields)?, + }, + |row| row.get(0), + ) + .map_err(SqliteDbError::from)?; + let media_rowid = RowId(media_rowid); + + tx.commit().map_err(SqliteDbError::from)?; + Ok(EntityId::new(*site, EditContext::table(), media_rowid)) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::{ + MigrationManager, db_types::self_hosted_site::SelfHostedSite, + repository::sites::SiteRepository, + }; + use rusqlite::Connection; + use wp_api::{ + media::{ + MediaCaptionWithEditContext, MediaDescriptionWithEditContext, MediaDetails, MediaId, + MediaStatus, MediaType, MediaWithEditContext, + }, + posts::{ + PostCommentStatus, PostGuidWithEditContext, PostPingStatus, PostTitleWithEditContext, + }, + users::UserId, + }; + + fn setup_db() -> (Connection, DbSite) { + let mut conn = Connection::open_in_memory().expect("open in-memory db"); + let mut mgr = MigrationManager::new(&conn).expect("migration manager"); + mgr.perform_migrations().expect("migrations"); + let db_site = SiteRepository + .upsert_self_hosted_site( + &mut conn, + &SelfHostedSite { + url: "https://test.local".into(), + api_root: "https://test.local/wp-json".into(), + }, + ) + .expect("upsert site") + .db_site; + (conn, db_site) + } + + fn make_media(id: i64) -> MediaWithEditContext { + MediaWithEditContext { + id: MediaId(id), + date: "2026-01-01T00:00:00".into(), + date_gmt: "2026-01-01T00:00:00Z".parse().unwrap(), + guid: PostGuidWithEditContext { + raw: None, + rendered: format!("https://example.com/?p={id}"), + }, + link: format!("https://example.com/{id}"), + modified: "2026-01-01T00:00:00".into(), + modified_gmt: "2026-01-01T00:00:00Z".parse().unwrap(), + slug: format!("media-{id}"), + status: MediaStatus::Inherit, + post_type: "attachment".into(), + password: None, + permalink_template: format!("https://example.com/?attachment_id={id}"), + generated_slug: format!("media-{id}"), + title: PostTitleWithEditContext { + raw: None, + rendered: format!("Media {id}"), + }, + author: UserId(1), + comment_status: PostCommentStatus::Open, + ping_status: PostPingStatus::Open, + template: String::new(), + alt_text: String::new(), + caption: MediaCaptionWithEditContext { + raw: String::new(), + rendered: String::new(), + }, + description: MediaDescriptionWithEditContext { + raw: String::new(), + rendered: String::new(), + }, + media_type: MediaType::File, + mime_type: "application/octet-stream".into(), + media_details: Arc::new(MediaDetails { + payload: serde_json::value::RawValue::from_string("{}".into()).unwrap(), + }), + post_id: None, + source_url: format!("https://example.com/media-{id}.bin"), + missing_image_sizes: Vec::new(), + } + } + + #[test] + fn select_by_media_id_returns_none_when_empty() { + let (conn, site) = setup_db(); + let repo = MediaRepository::::new(); + let result = repo + .select_by_media_id(&conn, &site, MediaId(42)) + .expect("select returns ok"); + assert!(result.is_none(), "expected None on empty table"); + } + + #[test] + fn upsert_then_select_by_media_id_round_trips_fields() { + let (mut conn, site) = setup_db(); + let repo = MediaRepository::::new(); + let media = make_media(42); + + repo.upsert(&mut conn, &site, &media).expect("upsert"); + let retrieved = repo + .select_by_media_id(&conn, &site, MediaId(42)) + .expect("select") + .expect("row should exist"); + + assert_eq!(retrieved.data.media.id, MediaId(42)); + assert_eq!(retrieved.data.media.slug, "media-42"); + assert_eq!(retrieved.data.media.status, MediaStatus::Inherit); + } + + #[test] + fn upsert_twice_with_same_id_updates_in_place_no_duplicate_rows() { + let (mut conn, site) = setup_db(); + let repo = MediaRepository::::new(); + + let first = make_media(7); + repo.upsert(&mut conn, &site, &first).expect("first upsert"); + + let mut second = make_media(7); + second.title = PostTitleWithEditContext { + raw: Some("Updated raw".into()), + rendered: "Updated rendered".into(), + }; + repo.upsert(&mut conn, &site, &second) + .expect("second upsert"); + + assert_eq!(repo.count(&conn, &site).expect("count"), 1); + let retrieved = repo + .select_by_media_id(&conn, &site, MediaId(7)) + .expect("select") + .expect("row should exist"); + assert_eq!(retrieved.data.media.title.rendered, "Updated rendered"); + assert_eq!( + retrieved.data.media.title.raw, + Some("Updated raw".to_string()) + ); + } + + #[test] + fn count_returns_number_of_rows_for_site() { + let (mut conn, site) = setup_db(); + let repo = MediaRepository::::new(); + for id in 1..=3 { + repo.upsert(&mut conn, &site, &make_media(id)) + .expect("upsert"); + } + assert_eq!(repo.count(&conn, &site).expect("count"), 3); + } + + #[test] + fn delete_by_media_id_returns_one_and_removes_row() { + let (mut conn, site) = setup_db(); + let repo = MediaRepository::::new(); + repo.upsert(&mut conn, &site, &make_media(1)) + .expect("upsert"); + + let deleted = repo + .delete_by_media_id(&conn, &site, MediaId(1)) + .expect("delete"); + assert_eq!(deleted, 1); + assert!( + repo.select_by_media_id(&conn, &site, MediaId(1)) + .expect("select") + .is_none() + ); + } + + #[test] + fn delete_by_media_id_non_existent_returns_zero() { + let (conn, site) = setup_db(); + let repo = MediaRepository::::new(); + let deleted = repo + .delete_by_media_id(&conn, &site, MediaId(999)) + .expect("delete"); + assert_eq!(deleted, 0); + } + + #[test] + fn select_by_entity_id_rejects_wrong_table_name() { + let (conn, site) = setup_db(); + let repo = MediaRepository::::new(); + let bad_entity_id = EntityId::new(site, DbTable::PostsEditContext, RowId(1)); + let result = repo.select_by_entity_id(&conn, &bad_entity_id); + match result { + Err(SqliteDbError::TableNameMismatch { expected, actual }) => { + assert_eq!(expected, DbTable::MediaEditContext); + assert_eq!(actual, DbTable::PostsEditContext); + } + Err(other) => panic!("expected TableNameMismatch error, got error {:?}", other), + Ok(_) => panic!("expected TableNameMismatch error, got Ok(_)"), + } + } +} diff --git a/wp_mobile_cache/src/repository/mod.rs b/wp_mobile_cache/src/repository/mod.rs index 1d21410c8..3f2fa4914 100644 --- a/wp_mobile_cache/src/repository/mod.rs +++ b/wp_mobile_cache/src/repository/mod.rs @@ -3,6 +3,7 @@ use rusqlite::Connection; pub mod entity_state; pub mod list_metadata; +pub mod media; pub mod post_types; pub mod posts; pub mod sites; From 9f65dcb0e77ffec4a596a60941572b48cf797cbd Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 13 May 2026 14:12:17 +1200 Subject: [PATCH 04/18] Add MediaBuilder test fixture and use it from MediaRepository tests --- wp_mobile_cache/src/repository/media.rs | 75 ++------ wp_mobile_cache/src/test_fixtures.rs | 1 + wp_mobile_cache/src/test_fixtures/media.rs | 206 +++++++++++++++++++++ 3 files changed, 220 insertions(+), 62 deletions(-) create mode 100644 wp_mobile_cache/src/test_fixtures/media.rs diff --git a/wp_mobile_cache/src/repository/media.rs b/wp_mobile_cache/src/repository/media.rs index f08d727e0..5f9767dff 100644 --- a/wp_mobile_cache/src/repository/media.rs +++ b/wp_mobile_cache/src/repository/media.rs @@ -476,18 +476,12 @@ mod tests { use super::*; use crate::{ MigrationManager, db_types::self_hosted_site::SelfHostedSite, - repository::sites::SiteRepository, + repository::sites::SiteRepository, test_fixtures::media::MediaBuilder, }; use rusqlite::Connection; use wp_api::{ - media::{ - MediaCaptionWithEditContext, MediaDescriptionWithEditContext, MediaDetails, MediaId, - MediaStatus, MediaType, MediaWithEditContext, - }, - posts::{ - PostCommentStatus, PostGuidWithEditContext, PostPingStatus, PostTitleWithEditContext, - }, - users::UserId, + media::{MediaId, MediaStatus}, + posts::PostTitleWithEditContext, }; fn setup_db() -> (Connection, DbSite) { @@ -507,52 +501,6 @@ mod tests { (conn, db_site) } - fn make_media(id: i64) -> MediaWithEditContext { - MediaWithEditContext { - id: MediaId(id), - date: "2026-01-01T00:00:00".into(), - date_gmt: "2026-01-01T00:00:00Z".parse().unwrap(), - guid: PostGuidWithEditContext { - raw: None, - rendered: format!("https://example.com/?p={id}"), - }, - link: format!("https://example.com/{id}"), - modified: "2026-01-01T00:00:00".into(), - modified_gmt: "2026-01-01T00:00:00Z".parse().unwrap(), - slug: format!("media-{id}"), - status: MediaStatus::Inherit, - post_type: "attachment".into(), - password: None, - permalink_template: format!("https://example.com/?attachment_id={id}"), - generated_slug: format!("media-{id}"), - title: PostTitleWithEditContext { - raw: None, - rendered: format!("Media {id}"), - }, - author: UserId(1), - comment_status: PostCommentStatus::Open, - ping_status: PostPingStatus::Open, - template: String::new(), - alt_text: String::new(), - caption: MediaCaptionWithEditContext { - raw: String::new(), - rendered: String::new(), - }, - description: MediaDescriptionWithEditContext { - raw: String::new(), - rendered: String::new(), - }, - media_type: MediaType::File, - mime_type: "application/octet-stream".into(), - media_details: Arc::new(MediaDetails { - payload: serde_json::value::RawValue::from_string("{}".into()).unwrap(), - }), - post_id: None, - source_url: format!("https://example.com/media-{id}.bin"), - missing_image_sizes: Vec::new(), - } - } - #[test] fn select_by_media_id_returns_none_when_empty() { let (conn, site) = setup_db(); @@ -567,7 +515,10 @@ mod tests { fn upsert_then_select_by_media_id_round_trips_fields() { let (mut conn, site) = setup_db(); let repo = MediaRepository::::new(); - let media = make_media(42); + let media = MediaBuilder::minimal() + .with_id(42) + .with_slug("media-42") + .build(); repo.upsert(&mut conn, &site, &media).expect("upsert"); let retrieved = repo @@ -585,10 +536,10 @@ mod tests { let (mut conn, site) = setup_db(); let repo = MediaRepository::::new(); - let first = make_media(7); + let first = MediaBuilder::minimal().with_id(7).build(); repo.upsert(&mut conn, &site, &first).expect("first upsert"); - let mut second = make_media(7); + let mut second = MediaBuilder::minimal().with_id(7).build(); second.title = PostTitleWithEditContext { raw: Some("Updated raw".into()), rendered: "Updated rendered".into(), @@ -613,8 +564,8 @@ mod tests { let (mut conn, site) = setup_db(); let repo = MediaRepository::::new(); for id in 1..=3 { - repo.upsert(&mut conn, &site, &make_media(id)) - .expect("upsert"); + let media = MediaBuilder::minimal().with_id(id).build(); + repo.upsert(&mut conn, &site, &media).expect("upsert"); } assert_eq!(repo.count(&conn, &site).expect("count"), 3); } @@ -623,8 +574,8 @@ mod tests { fn delete_by_media_id_returns_one_and_removes_row() { let (mut conn, site) = setup_db(); let repo = MediaRepository::::new(); - repo.upsert(&mut conn, &site, &make_media(1)) - .expect("upsert"); + let media = MediaBuilder::minimal().with_id(1).build(); + repo.upsert(&mut conn, &site, &media).expect("upsert"); let deleted = repo .delete_by_media_id(&conn, &site, MediaId(1)) diff --git a/wp_mobile_cache/src/test_fixtures.rs b/wp_mobile_cache/src/test_fixtures.rs index c8ab76c8e..95612a56d 100644 --- a/wp_mobile_cache/src/test_fixtures.rs +++ b/wp_mobile_cache/src/test_fixtures.rs @@ -13,6 +13,7 @@ use rstest::*; use rusqlite::Connection; use std::sync::atomic::{AtomicU32, Ordering}; +pub mod media; pub mod post_types; pub mod posts; diff --git a/wp_mobile_cache/src/test_fixtures/media.rs b/wp_mobile_cache/src/test_fixtures/media.rs new file mode 100644 index 000000000..e58f7105c --- /dev/null +++ b/wp_mobile_cache/src/test_fixtures/media.rs @@ -0,0 +1,206 @@ +use std::sync::Arc; +use std::sync::atomic::{AtomicI64, Ordering}; +use wp_api::{ + media::{ + MediaCaptionWithEditContext, MediaDescriptionWithEditContext, MediaDetails, MediaId, + MediaStatus, MediaType, MediaWithEditContext, + }, + posts::{ + PostCommentStatus, PostGuidWithEditContext, PostId, PostPingStatus, + PostTitleWithEditContext, + }, + users::UserId, +}; + +/// Initial state for MediaBuilder - determines which field values are populated. +pub enum MediaBuilderInitialState { + /// Minimal valid media with only required fields populated + Minimal, + /// Fully populated media with all optional fields set + Full, +} + +/// Builder for creating test media entities with automatic ID management. +/// +/// Use `MediaBuilder::minimal()` for media with only required fields, +/// or `MediaBuilder::full()` for media with all fields populated. +/// +/// Reduces boilerplate and prevents ID collisions in tests by auto-incrementing IDs. +pub struct MediaBuilder { + media: MediaWithEditContext, +} + +impl MediaBuilder { + /// Create a new builder with auto-incremented ID starting from 2000. + /// + /// Uses thread-safe atomic counter to ensure unique IDs across tests. + /// IDs start at 2000 to make them distinguishable from PostBuilder IDs (1000+). + pub fn new(initial_state: MediaBuilderInitialState) -> Self { + static COUNTER: AtomicI64 = AtomicI64::new(2000); + let id = COUNTER.fetch_add(1, Ordering::SeqCst); + + let mut media = match initial_state { + MediaBuilderInitialState::Minimal => create_minimal_media(), + MediaBuilderInitialState::Full => create_full_media(), + }; + media.id = MediaId(id); + Self { media } + } + + /// Create a minimal media builder with only required fields populated. + pub fn minimal() -> Self { + Self::new(MediaBuilderInitialState::Minimal) + } + + /// Create a full media builder with all optional fields populated. + pub fn full() -> Self { + Self::new(MediaBuilderInitialState::Full) + } + + /// Set a specific media ID (overrides auto-increment). + pub fn with_id(mut self, id: i64) -> Self { + self.media.id = MediaId(id); + self + } + + /// Set a specific media ID (overrides auto-increment). + pub fn with_media_id(mut self, media_id: MediaId) -> Self { + self.media.id = media_id; + self + } + + /// Set the media slug. + pub fn with_slug(mut self, slug: &str) -> Self { + self.media.slug = slug.into(); + self + } + + /// Set the media status. + pub fn with_status(mut self, status: MediaStatus) -> Self { + self.media.status = status; + self + } + + /// Set the media title. + pub fn with_title(mut self, title: &str) -> Self { + self.media.title = PostTitleWithEditContext { + raw: Some(title.into()), + rendered: title.into(), + }; + self + } + + /// Set the media author. + pub fn with_author(mut self, author: UserId) -> Self { + self.media.author = author; + self + } + + /// Set the attached post id. + pub fn with_post_id(mut self, post_id: PostId) -> Self { + self.media.post_id = Some(post_id); + self + } + + /// Set the media type. + pub fn with_media_type(mut self, media_type: MediaType) -> Self { + self.media.media_type = media_type; + self + } + + /// Set the MIME type. + pub fn with_mime_type(mut self, mime: &str) -> Self { + self.media.mime_type = mime.into(); + self + } + + /// Build the final MediaWithEditContext. + pub fn build(self) -> MediaWithEditContext { + self.media + } +} + +impl Default for MediaBuilder { + fn default() -> Self { + Self::minimal() + } +} + +fn create_minimal_media() -> MediaWithEditContext { + MediaWithEditContext { + id: MediaId(0), + date: "2026-01-01T00:00:00".into(), + date_gmt: "2026-01-01T00:00:00Z".parse().unwrap(), + guid: PostGuidWithEditContext { + raw: None, + rendered: "https://example.com/?p=0".into(), + }, + link: "https://example.com/0".into(), + modified: "2026-01-01T00:00:00".into(), + modified_gmt: "2026-01-01T00:00:00Z".parse().unwrap(), + slug: "media-0".into(), + status: MediaStatus::Inherit, + post_type: "attachment".into(), + password: None, + permalink_template: "https://example.com/?attachment_id=0".into(), + generated_slug: "media-0".into(), + title: PostTitleWithEditContext { + raw: None, + rendered: "Media 0".into(), + }, + author: UserId(1), + comment_status: PostCommentStatus::Open, + ping_status: PostPingStatus::Open, + template: String::new(), + alt_text: String::new(), + caption: MediaCaptionWithEditContext { + raw: String::new(), + rendered: String::new(), + }, + description: MediaDescriptionWithEditContext { + raw: String::new(), + rendered: String::new(), + }, + media_type: MediaType::File, + mime_type: "application/octet-stream".into(), + media_details: Arc::new(MediaDetails { + payload: serde_json::value::RawValue::from_string("{}".into()).unwrap(), + }), + post_id: None, + source_url: "https://example.com/media-0.bin".into(), + missing_image_sizes: Vec::new(), + } +} + +fn create_full_media() -> MediaWithEditContext { + let mut m = create_minimal_media(); + m.password = Some("secret".into()); + m.post_id = Some(PostId(100)); + m.alt_text = "alt text".into(); + m.media_type = MediaType::Image; + m.mime_type = "image/jpeg".into(); + m.caption = MediaCaptionWithEditContext { + raw: "caption raw".into(), + rendered: "

caption rendered

".into(), + }; + m.description = MediaDescriptionWithEditContext { + raw: "description raw".into(), + rendered: "

description rendered

".into(), + }; + m.missing_image_sizes = vec!["thumbnail".into(), "medium".into()]; + m.media_details = Arc::new(MediaDetails { + payload: serde_json::value::RawValue::from_string( + r#"{"filesize":12345,"width":1024,"height":768}"#.into(), + ) + .unwrap(), + }); + m.title = PostTitleWithEditContext { + raw: Some("Full Media Title".into()), + rendered: "Full Media Title".into(), + }; + m.guid = PostGuidWithEditContext { + raw: Some("https://example.com/?p=999".into()), + rendered: "https://example.com/?p=999".into(), + }; + m +} From a97db2cf16108523385ddc167af61be673f8ee62 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 13 May 2026 14:21:28 +1200 Subject: [PATCH 05/18] Add MediaListFilter and media_list_filter_cache_key --- wp_mobile/src/cache_key.rs | 74 ++++++++++- wp_mobile/src/filters/media_list_filter.rs | 147 +++++++++++++++++++++ wp_mobile/src/filters/mod.rs | 2 + 3 files changed, 220 insertions(+), 3 deletions(-) create mode 100644 wp_mobile/src/filters/media_list_filter.rs diff --git a/wp_mobile/src/cache_key.rs b/wp_mobile/src/cache_key.rs index c889bf3cb..a782842a2 100644 --- a/wp_mobile/src/cache_key.rs +++ b/wp_mobile/src/cache_key.rs @@ -6,11 +6,11 @@ use url::Url; use wp_api::{ - posts::PostListParamsField, request::endpoint::posts_endpoint::PostEndpointType, - url_query::QueryPairsExtension, + media::MediaListParamsField, posts::PostListParamsField, + request::endpoint::posts_endpoint::PostEndpointType, url_query::QueryPairsExtension, }; -use crate::filters::PostListFilter; +use crate::filters::{MediaListFilter, PostListFilter}; /// Generates a cache key segment from a `PostEndpointType`. /// @@ -103,6 +103,41 @@ pub fn post_list_filter_cache_key(filter: &PostListFilter) -> String { url.query().unwrap_or("").to_string() } +/// Generates a deterministic cache key from `MediaListFilter`. +/// +/// All fields in `MediaListFilter` are included in the cache key since it only +/// contains filter-relevant fields (pagination, instance-specific, and date +/// range fields are excluded by design in `MediaListFilter`). +// TODO: Remove `allow(dead_code)` once `MediaService` (added in a follow-up task) +// consumes this function. +#[allow(dead_code)] +pub fn media_list_filter_cache_key(filter: &MediaListFilter) -> String { + let mut url = Url::parse("https://cache-key-generator.local").expect("valid base URL"); + + { + let mut q = url.query_pairs_mut(); + + // Alphabetically ordered for determinism. + q.append_vec_query_value_pair(MediaListParamsField::Author, &filter.author); + q.append_vec_query_value_pair(MediaListParamsField::AuthorExclude, &filter.author_exclude); + q.append_option_query_value_pair( + MediaListParamsField::MediaType, + filter.media_type.as_ref(), + ); + q.append_option_query_value_pair(MediaListParamsField::MimeType, filter.mime_type.as_ref()); + q.append_option_query_value_pair(MediaListParamsField::Order, filter.order.as_ref()); + q.append_option_query_value_pair(MediaListParamsField::Orderby, filter.orderby.as_ref()); + q.append_vec_query_value_pair(MediaListParamsField::Parent, &filter.parent); + q.append_vec_query_value_pair(MediaListParamsField::ParentExclude, &filter.parent_exclude); + q.append_option_query_value_pair(MediaListParamsField::Search, filter.search.as_ref()); + q.append_vec_query_value_pair(MediaListParamsField::SearchColumns, &filter.search_columns); + q.append_vec_query_value_pair(MediaListParamsField::Slug, &filter.slug); + q.append_vec_query_value_pair(MediaListParamsField::Status, &filter.status); + } + + url.query().unwrap_or("").to_string() +} + #[cfg(test)] mod tests { use super::*; @@ -167,4 +202,37 @@ mod tests { let key = endpoint_type_cache_key(&PostEndpointType::Custom("products".to_string())); assert_eq!(key, "post_type_custom_products"); } + + #[test] + fn media_empty_filter_produces_empty_key() { + let filter = MediaListFilter::default(); + let key = media_list_filter_cache_key(&filter); + assert_eq!(key, ""); + } + + #[test] + fn media_status_filter() { + use wp_api::media::MediaStatus; + let filter = MediaListFilter { + status: vec![MediaStatus::Inherit], + ..Default::default() + }; + let key = media_list_filter_cache_key(&filter); + assert_eq!(key, "status=inherit"); + } + + #[test] + fn media_multi_field_sorted() { + use wp_api::media::{MediaStatus, MediaTypeParam}; + use wp_api::users::UserId; + let filter = MediaListFilter { + status: vec![MediaStatus::Inherit], + author: vec![UserId(5)], + media_type: Some(MediaTypeParam::Image), + ..Default::default() + }; + let key = media_list_filter_cache_key(&filter); + // Fields in alphabetical order: author, media_type, status + assert_eq!(key, "author=5&media_type=image&status=inherit"); + } } diff --git a/wp_mobile/src/filters/media_list_filter.rs b/wp_mobile/src/filters/media_list_filter.rs new file mode 100644 index 000000000..36742727b --- /dev/null +++ b/wp_mobile/src/filters/media_list_filter.rs @@ -0,0 +1,147 @@ +//! Filter type for media metadata collections. +//! +//! This module provides `MediaListFilter`, a subset of `MediaListParams` containing +//! only fields appropriate for metadata collection filtering. + +use wp_api::{ + WpApiParamOrder, + media::{MediaListParams, MediaStatus, MediaTypeParam}, + posts::{PostId, WpApiParamPostsOrderBy, WpApiParamPostsSearchColumn}, + users::UserId, +}; + +/// Filter parameters for media metadata collections. +/// +/// This type exposes only the filter-relevant fields from `MediaListParams`, +/// excluding pagination, instance-specific lookup (`include`/`exclude`), and +/// date-range fields. Those are inappropriate for metadata-collection use +/// cases for the same reasons documented on `PostListFilter`. +#[derive(Debug, Default, Clone, PartialEq, Eq, uniffi::Record)] +pub struct MediaListFilter { + /// Limit results to those matching a string. + #[uniffi(default = None)] + pub search: Option, + + /// Array of column names to be searched. + #[uniffi(default = [])] + pub search_columns: Vec, + + /// Limit result set to media assigned to specific authors. + #[uniffi(default = [])] + pub author: Vec, + + /// Ensure result set excludes media assigned to specific authors. + #[uniffi(default = [])] + pub author_exclude: Vec, + + /// Order sort attribute ascending or descending. + /// Default: desc + #[uniffi(default = None)] + pub order: Option, + + /// Sort collection by media attribute. + /// Default: date + #[uniffi(default = None)] + pub orderby: Option, + + /// Limit result set to media with one or more specific slugs. + #[uniffi(default = [])] + pub slug: Vec, + + /// Limit result set to media assigned one or more statuses. + /// Default: inherit + #[uniffi(default = [])] + pub status: Vec, + + /// Limit result set to media attached to one of these posts. + #[uniffi(default = [])] + pub parent: Vec, + + /// Exclude media attached to any of these posts. + #[uniffi(default = [])] + pub parent_exclude: Vec, + + /// Limit result set to attachments of a particular media type. + #[uniffi(default = None)] + pub media_type: Option, + + /// Limit result set to attachments of a particular MIME type. + #[uniffi(default = None)] + pub mime_type: Option, +} + +impl MediaListFilter { + /// Convert this filter into a `MediaListParams` ready for an API call. + /// + /// Pagination is provided by the caller (the service layer) since + /// `MediaListFilter` deliberately omits page/per_page/offset. + pub fn to_list_params(&self, page: u32, per_page: u32) -> MediaListParams { + MediaListParams { + page: Some(page), + per_page: Some(per_page), + search: self.search.clone(), + search_columns: self.search_columns.clone(), + author: self.author.clone(), + author_exclude: self.author_exclude.clone(), + order: self.order, + orderby: self.orderby, + slug: self.slug.clone(), + status: self.status.clone(), + parent: self.parent.clone(), + parent_exclude: self.parent_exclude.clone(), + media_type: self.media_type.clone(), + mime_type: self.mime_type.clone(), + // Fields intentionally excluded from MediaListFilter. + offset: None, + include: Vec::new(), + exclude: Vec::new(), + after: None, + modified_after: None, + before: None, + modified_before: None, + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn to_list_params_sets_page_and_per_page() { + let filter = MediaListFilter::default(); + let params = filter.to_list_params(3, 25); + assert_eq!(params.page, Some(3)); + assert_eq!(params.per_page, Some(25)); + } + + #[test] + fn to_list_params_clones_search_columns_and_status() { + let filter = MediaListFilter { + search_columns: vec![WpApiParamPostsSearchColumn::PostTitle], + status: vec![MediaStatus::Inherit, MediaStatus::Private], + ..Default::default() + }; + let params = filter.to_list_params(1, 10); + assert_eq!( + params.search_columns, + vec![WpApiParamPostsSearchColumn::PostTitle] + ); + assert_eq!( + params.status, + vec![MediaStatus::Inherit, MediaStatus::Private] + ); + } + + #[test] + fn to_list_params_passes_media_type_and_mime_type_through() { + let filter = MediaListFilter { + media_type: Some(MediaTypeParam::Image), + mime_type: Some("image/jpeg".to_string()), + ..Default::default() + }; + let params = filter.to_list_params(1, 10); + assert_eq!(params.media_type, Some(MediaTypeParam::Image)); + assert_eq!(params.mime_type, Some("image/jpeg".to_string())); + } +} diff --git a/wp_mobile/src/filters/mod.rs b/wp_mobile/src/filters/mod.rs index 3474949ad..bef363a0f 100644 --- a/wp_mobile/src/filters/mod.rs +++ b/wp_mobile/src/filters/mod.rs @@ -1,7 +1,9 @@ +mod media_list_filter; mod post_filter; mod post_list_filter; mod post_type_filter; +pub use media_list_filter::MediaListFilter; pub use post_filter::AnyPostFilter; pub use post_list_filter::PostListFilter; pub(crate) use post_list_filter::compare_posts_by_order; From 82727a88a1b7fafc7354bc63e19b8582a8ef38fc Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 13 May 2026 14:35:54 +1200 Subject: [PATCH 06/18] Add MediaService with sync, fetch, and delete APIs --- wp_mobile/src/cache_key.rs | 3 - wp_mobile/src/lib.rs | 5 + wp_mobile/src/service/media.rs | 955 +++++++++++++++++++++++++++++++++ wp_mobile/src/service/mod.rs | 17 +- 4 files changed, 976 insertions(+), 4 deletions(-) create mode 100644 wp_mobile/src/service/media.rs diff --git a/wp_mobile/src/cache_key.rs b/wp_mobile/src/cache_key.rs index a782842a2..8a7dfae4c 100644 --- a/wp_mobile/src/cache_key.rs +++ b/wp_mobile/src/cache_key.rs @@ -108,9 +108,6 @@ pub fn post_list_filter_cache_key(filter: &PostListFilter) -> String { /// All fields in `MediaListFilter` are included in the cache key since it only /// contains filter-relevant fields (pagination, instance-specific, and date /// range fields are excluded by design in `MediaListFilter`). -// TODO: Remove `allow(dead_code)` once `MediaService` (added in a follow-up task) -// consumes this function. -#[allow(dead_code)] pub fn media_list_filter_cache_key(filter: &MediaListFilter) -> String { let mut url = Url::parse("https://cache-key-generator.local").expect("valid base URL"); diff --git a/wp_mobile/src/lib.rs b/wp_mobile/src/lib.rs index df4b7415f..1381906b4 100644 --- a/wp_mobile/src/lib.rs +++ b/wp_mobile/src/lib.rs @@ -21,6 +21,11 @@ wp_mobile_entity!( wp_api::posts::AnyPostWithEditContext ); +wp_mobile_entity!( + EntityMediaWithEditContext, + wp_api::media::MediaWithEditContext +); + wp_mobile_entity!( EntityPostTypeDetailsWithEditContext, wp_api::post_types::PostTypeDetailsWithEditContext diff --git a/wp_mobile/src/service/media.rs b/wp_mobile/src/service/media.rs new file mode 100644 index 000000000..cbe098b82 --- /dev/null +++ b/wp_mobile/src/service/media.rs @@ -0,0 +1,955 @@ +use crate::{ + EntityMediaWithEditContext, + cache_key::media_list_filter_cache_key, + collection::{FetchError, FetchResult, MetadataCollectionCore}, + filters::MediaListFilter, + service::{ + entity_state_service::{EntityStateReader, EntityStateReaderImpl, EntityStateService}, + metadata::MetadataService, + }, + sync::{DbEntityState, EntityMetadata, MetadataFetchResult, SyncResult, SyncStrategy}, +}; +use std::{collections::HashSet, sync::Arc}; +use wp_api::{ + api_client::WpApiClient, + media::{ + MediaDeleteResponse, MediaId, MediaListParams, MediaWithEditContext, + SparseMediaFieldWithEditContext, + }, +}; +use wp_mobile_cache::{ + DbTable, WpApiCache, + context::EditContext, + db_types::db_site::DbSite, + entity::{Entity, EntityId, FullEntity}, + list_metadata::ListKey, + repository::{entity_state::EntityType, media::MediaRepository}, +}; + +/// Maximum number of media items to fetch in a single batch request +const BATCH_FETCH_SIZE: usize = 100; + +// Internal types + +/// Result from loading media by IDs. +pub struct LoadByIdsResult { + /// Entity IDs of successfully loaded media + pub entity_ids: Vec, + /// Number of media items that were requested but failed to load + pub failed_count: usize, +} + +/// Statistics from fetching missing and stale media. +pub(crate) struct FetchStats { + /// Number of media items that needed fetching (Missing or Stale state) + pub(crate) fetched_count: usize, + /// Number of media items that failed to fetch + pub(crate) failed_count: usize, +} + +/// Stub for `MediaMetadataCollectionWithEditContext`. +/// +/// TODO: replaced by Task 7. This placeholder lets `MediaService` expose the +/// collection factory now; the real type lands with the collection module. +#[derive(uniffi::Object)] +pub struct MediaMetadataCollectionWithEditContext { + _core: MetadataCollectionCore, + _service: Arc, + _filter: MediaListFilter, +} + +impl MediaMetadataCollectionWithEditContext { + pub fn new( + core: MetadataCollectionCore, + service: Arc, + filter: MediaListFilter, + ) -> Self { + Self { + _core: core, + _service: service, + _filter: filter, + } + } +} + +/// Service layer for media operations +/// +/// Provides a bridge between clients and the underlying network/cache layers. +/// Handles fetching and deleting media. Mutations that iOS performs directly +/// against the API client (create/update/upload) are deliberately not exposed here. +/// +/// # Metadata Sync Infrastructure +/// +/// The service provides access to metadata-first sync infrastructure: +/// - Entity state tracking via `EntityStateStore` associated functions +/// - Database-backed list metadata via `metadata_service` +/// +/// Collections get read-only access via reader methods. This ensures cross-collection +/// consistency when multiple collections share the same underlying entities. +#[derive(uniffi::Object)] +pub struct MediaService { + db_site: Arc, + api_client: Arc, + cache: Arc, + + /// Database-backed list metadata service. + /// Persists list structure across app restarts. + pub(crate) metadata_service: Arc, +} + +impl MediaService { + pub fn new(api_client: Arc, db_site: Arc, cache: Arc) -> Self { + let metadata_service = Arc::new(MetadataService::new(db_site.clone(), cache.clone())); + + Self { + api_client, + db_site, + cache, + metadata_service, + } + } + + /// Sync a page of media items from network to cache. + /// + /// Fetches full media data from the API and saves it to the database: + /// 1. Converts filter to API parameters + /// 2. Makes network request via WpApiClient + /// 3. Upserts media to database via repository + /// 4. Returns entity IDs and pagination info + pub async fn sync_media_page( + &self, + filter: &MediaListFilter, + page: u32, + per_page: u32, + ) -> Result { + let params = filter.to_list_params(page, per_page); + + let response = self + .api_client + .media() + .list_with_edit_context(¶ms) + .await?; + + let entity_ids = self.cache.execute(|conn| { + let repo = MediaRepository::::new(); + response + .data + .iter() + .map(|media| { + repo.upsert(conn, &self.db_site, media) + .map_err(|e| FetchError::Database { + err_message: e.to_string(), + }) + }) + .collect::, FetchError>>() + })?; + + Ok(FetchResult { + entity_ids, + total_items: response.header_map.wp_total().map(|n| n as i64), + total_pages: response.header_map.wp_total_pages(), + current_page: page, + }) + } + + /// Fetch lightweight metadata (id, modified_gmt, post_id) for a page of media. + /// + /// Returns only the minimal fields needed to determine list structure and staleness. + /// Does not fetch or save full media content. + pub(crate) async fn fetch_media_metadata( + &self, + filter: &MediaListFilter, + page: u32, + per_page: u32, + ) -> Result { + let request_params = filter.to_list_params(page, per_page); + + let response = self + .api_client + .media() + .filter_list_with_edit_context( + &request_params, + &[ + SparseMediaFieldWithEditContext::Id, + SparseMediaFieldWithEditContext::ModifiedGmt, + SparseMediaFieldWithEditContext::PostId, + ], + ) + .await?; + + // `post_id` (media's "attached post") serves the parent slot in `EntityMetadata`. + // `menu_order` is always `None` for media. + let metadata: Vec = response + .data + .into_iter() + .filter_map(|sparse| { + Some(EntityMetadata::new( + sparse.id?.0, + sparse.modified_gmt, + sparse.post_id.map(|p| p.0), + None, + )) + }) + .collect(); + + Ok(MetadataFetchResult::new( + metadata, + response.header_map.wp_total().map(|n| n as i64), + response.header_map.wp_total_pages(), + page, + )) + } + + /// Find stale media items by comparing fetched metadata timestamps with cached DB values. + /// + /// A media item is considered stale if: + /// 1. It's currently in `Fresh` state in the state store + /// 2. Its fetched `modified_gmt` differs from the cached `modified_gmt` in the database + pub(crate) fn find_stale_media_by_timestamp( + &self, + metadata: &[EntityMetadata], + state_reader: &dyn EntityStateReader, + ) -> Vec { + let cached_ids: Vec = metadata + .iter() + .filter(|m| matches!(state_reader.get(m.id), DbEntityState::Fresh)) + .map(|m| MediaId(m.id)) + .collect(); + + if cached_ids.is_empty() { + return Vec::new(); + } + + let cached_timestamps = self + .cache + .execute(|conn| { + let repo = MediaRepository::::new(); + repo.select_modified_gmt_by_ids(conn, &self.db_site, &cached_ids) + }) + .unwrap_or_else(|e| { + log::warn!( + "Failed to query cached timestamps for staleness check: {}", + e + ); + Default::default() + }); + + metadata + .iter() + .filter_map(|m| { + if let Some(fetched_modified) = &m.modified_gmt + && let Some(cached_modified) = cached_timestamps.get(&MediaId(m.id)) + && fetched_modified != cached_modified + { + Some(m.id) + } else { + None + } + }) + .collect() + } + + /// Sync a media list using the default full sync strategy. + pub async fn sync_list( + &self, + key: &ListKey, + filter: &MediaListFilter, + per_page: u32, + is_refresh: bool, + ) -> Result { + self.sync_list_with_strategy(key, filter, per_page, is_refresh, SyncStrategy::Full) + .await + } + + /// Sync a media list with explicit strategy control. + /// + /// Mirrors `PostService::sync_list_with_strategy`: fetches list metadata, stores + /// it, detects stale items via modified_gmt comparison, and (for `Full`) selectively + /// fetches missing/stale entities. + pub async fn sync_list_with_strategy( + &self, + key: &ListKey, + filter: &MediaListFilter, + per_page: u32, + is_refresh: bool, + strategy: SyncStrategy, + ) -> Result { + // 1. Fetch and store metadata + let metadata_result = if is_refresh { + self.metadata_service + .refresh(key, per_page, |page, per_page| { + self.fetch_media_metadata(filter, page, per_page) + }) + .await? + } else { + self.metadata_service + .load_more(key, |page, per_page| { + self.fetch_media_metadata(filter, page, per_page) + }) + .await? + }; + + // 2. Detect and mark stale media (always done - doesn't fetch) + let stale_ids = self.find_stale_media_by_timestamp( + &metadata_result.metadata, + self.state_reader_with_edit_context().as_ref(), + ); + + if !stale_ids.is_empty() { + log::debug!( + "Found {} stale media item(s) via modified_gmt comparison", + stale_ids.len() + ); + EntityStateService::save_batch( + &self.cache, + &self.db_site, + EntityType::MediaEditContext, + &stale_ids, + DbEntityState::Stale, + ); + } + + // 3. Fetch missing/stale media (only for Full strategy) + let stats = match strategy { + SyncStrategy::MetadataOnly => FetchStats { + fetched_count: 0, + failed_count: 0, + }, + SyncStrategy::Full => { + self.fetch_missing_and_stale_media(&metadata_result.metadata) + .await + } + }; + + let total_items = self + .metadata_service + .get_entity_ids(key) + .map(|ids| ids.len()) + .unwrap_or(0); + + let pagination = self.metadata_service.get_pagination(key).ok().flatten(); + let current_page = pagination.as_ref().and_then(|p| p.current_page); + let has_more_pages = pagination.as_ref().and_then(|p| { + current_page.and_then(|current| p.total_pages.map(|total| current < total)) + }); + + Ok(SyncResult::new( + total_items, + stats.fetched_count, + stats.failed_count, + has_more_pages, + current_page, + metadata_result.total_pages, + )) + } + + /// Fetch media items that are missing or stale based on current state. + pub(crate) async fn fetch_missing_and_stale_media( + &self, + metadata: &[EntityMetadata], + ) -> FetchStats { + let ids_to_fetch: Vec = metadata + .iter() + .filter(|m| { + let state = EntityStateService::get( + &self.cache, + &self.db_site, + EntityType::MediaEditContext, + m.id, + ); + state.needs_fetch() + }) + .map(|m| MediaId(m.id)) + .collect(); + + let fetched_count = ids_to_fetch.len(); + let mut failed_count = 0; + + if !ids_to_fetch.is_empty() { + for chunk in ids_to_fetch.chunks(BATCH_FETCH_SIZE) { + match self.load_media_by_ids(chunk.to_vec()).await { + Ok(result) => { + failed_count += result.failed_count; + } + Err(e) => { + log::warn!( + "Failed to load {} media item(s) (IDs: {:?}): {}", + chunk.len(), + chunk, + e + ); + failed_count += chunk.len(); + } + } + } + } + + FetchStats { + fetched_count, + failed_count, + } + } + + /// Load media items by IDs from network to cache with state tracking. + /// + /// Mirrors `PostService::load_posts_by_ids`. Unlike posts, `MediaListParams` + /// has no `status` field defaulting to "any+trash"; the API default applies. + pub async fn load_media_by_ids( + &self, + ids: Vec, + ) -> Result { + if ids.is_empty() { + return Ok(LoadByIdsResult { + entity_ids: Vec::new(), + failed_count: 0, + }); + } + + let raw_ids: Vec = ids.iter().map(|id| id.0).collect(); + let fetchable = EntityStateService::filter_fetchable( + &self.cache, + &self.db_site, + EntityType::MediaEditContext, + &raw_ids, + ); + + if fetchable.is_empty() { + return Ok(LoadByIdsResult { + entity_ids: Vec::new(), + failed_count: 0, + }); + } + + EntityStateService::save_batch( + &self.cache, + &self.db_site, + EntityType::MediaEditContext, + &fetchable, + DbEntityState::Fetching, + ); + + let media_ids: Vec = fetchable.iter().map(|&id| MediaId(id)).collect(); + + let params = MediaListParams { + include: media_ids, + per_page: Some(BATCH_FETCH_SIZE as u32), + ..Default::default() + }; + + match self + .api_client + .media() + .list_with_edit_context(¶ms) + .await + { + Ok(response) => { + let entity_ids = match self.cache.execute(|conn| { + let repo = MediaRepository::::new(); + response + .data + .iter() + .map(|media| { + repo.upsert(conn, &self.db_site, media).map_err(|e| { + FetchError::Database { + err_message: e.to_string(), + } + }) + }) + .collect::, FetchError>>() + }) { + Ok(ids) => ids, + Err(e) => { + EntityStateService::save_batch( + &self.cache, + &self.db_site, + EntityType::MediaEditContext, + &fetchable, + DbEntityState::failed(e.to_string()), + ); + return Err(e); + } + }; + + let fetched_ids: Vec = response.data.iter().map(|m| m.id.0).collect(); + EntityStateService::save_batch( + &self.cache, + &self.db_site, + EntityType::MediaEditContext, + &fetched_ids, + DbEntityState::Fresh, + ); + + let fetched_set: HashSet = fetched_ids.iter().copied().collect(); + let failed_ids: Vec = fetchable + .iter() + .filter(|id| !fetched_set.contains(id)) + .copied() + .collect(); + let failed_count = failed_ids.len(); + if !failed_ids.is_empty() { + EntityStateService::save_batch( + &self.cache, + &self.db_site, + EntityType::MediaEditContext, + &failed_ids, + DbEntityState::failed("Not found"), + ); + } + + Ok(LoadByIdsResult { + entity_ids, + failed_count, + }) + } + Err(e) => { + EntityStateService::save_batch( + &self.cache, + &self.db_site, + EntityType::MediaEditContext, + &fetchable, + DbEntityState::failed(e.to_string()), + ); + Err(e.into()) + } + } + } + + /// Get read-only access to the entity state reader for edit context. + pub fn state_reader_with_edit_context(&self) -> Arc { + Arc::new(EntityStateReaderImpl::new( + self.cache.clone(), + *self.db_site, + EntityType::MediaEditContext, + )) + } + + /// Get read-only access to the persistent metadata service. + pub fn persistent_metadata_reader(&self) -> Arc { + self.metadata_service.clone() + } + + /// Read media items by IDs from the database cache. + pub fn read_media_by_ids_from_db( + &self, + ids: &[i64], + ) -> Result>, wp_mobile_cache::SqliteDbError> { + if ids.is_empty() { + return Ok(Vec::new()); + } + + let repo = MediaRepository::::new(); + + // TODO: query database for all IDs in one call instead of iterating? + self.cache.execute(|connection| { + ids.iter() + .map(|&id| repo.select_by_media_id(connection, &self.db_site, MediaId(id))) + .collect::, _>>() + .map(|items| { + items + .into_iter() + .flatten() + .map(|db_media| FullEntity::new(db_media.entity_id, db_media.data.media)) + .collect() + }) + }) + } +} + +#[uniffi::export] +impl MediaService { + /// Get an entity handle using an EntityId + /// + /// Returns an entity that can be used to read media data with full edit context. + /// The entity is lightweight - it doesn't fetch data until you call load_data() on it. + pub fn get_entity_with_edit_context(&self, entity_id: EntityId) -> EntityMediaWithEditContext { + let cache = self.cache.clone(); + + Entity::::new( + entity_id, + Box::new(move || { + let repo = MediaRepository::::new(); + + cache + .execute(|connection| repo.select_by_entity_id(connection, &entity_id)) + .map(|opt| { + opt.map(|db_media_full_entity| { + FullEntity::new( + db_media_full_entity.entity_id, + db_media_full_entity.data.media, + ) + }) + }) + }), + ) + .into() + } + + /// Get the total count of media for this site + pub fn count_edit_context(&self) -> Result { + let repo = MediaRepository::::new(); + self.cache + .execute(|connection| repo.count(connection, &self.db_site)) + } + + /// Delete a media item by its EntityId + pub fn delete_by_entity_id( + &self, + entity_id: &EntityId, + ) -> Result { + let repo = MediaRepository::::new(); + self.cache.execute(|connection| { + repo.delete_by_entity_id(connection, entity_id) + .map(|n| n as u64) + }) + } + + /// Delete a media item by its WordPress media ID + pub fn delete_by_media_id( + &self, + media_id: MediaId, + ) -> Result { + let repo = MediaRepository::::new(); + self.cache.execute(|connection| { + repo.delete_by_media_id(connection, &self.db_site, media_id) + .map(|n| n as u64) + }) + } + + /// Permanently delete a media item via the REST API and remove it from the local cache. + /// + /// REST DELETE on media always passes `force=true` (the server rejects `force=false`), + /// so there is no separate "trash" path. + pub async fn delete_media_permanently( + self: &Arc, + media_id: &MediaId, + ) -> Result { + let response = self.api_client.media().delete(media_id).await?.data; + + self.delete_by_media_id(*media_id) + .map_err(|e| FetchError::Database { + err_message: e.to_string(), + })?; + + Ok(response) + } + + /// Create a metadata-first media collection with edit context + /// + /// Returns a collection that uses a two-phase sync strategy: + /// 1. Fetch lightweight metadata (id + modified_gmt) to define list structure + /// 2. Selectively fetch full data for missing or stale items + pub fn create_media_metadata_collection_with_edit_context( + self: &Arc, + filter: MediaListFilter, + per_page: u32, + ) -> Arc { + let cache_key = media_list_filter_cache_key(&filter); + let key: ListKey = + format!("site_{:?}:edit:media:{}", self.db_site.row_id, cache_key).into(); + + let core = MetadataCollectionCore::new( + key, + self.persistent_metadata_reader(), + self.state_reader_with_edit_context(), + vec![DbTable::MediaEditContext, DbTable::ListMetadataItems], + per_page, + ); + + Arc::new(MediaMetadataCollectionWithEditContext::new( + core, + self.clone(), + filter, + )) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::testing::{EmptyAppNotifier, MockExecutor, mock_api_client}; + use rstest::*; + use rusqlite::Connection; + use wp_api::{media::MediaId, prelude::*}; + use wp_mobile_cache::{ + MigrationManager, WpApiCache, + db_types::self_hosted_site::SelfHostedSite, + repository::{media::MediaRepository, sites::SiteRepository}, + test_fixtures::media::MediaBuilder, + }; + + /// Test context bundling MediaService with database and site setup + pub struct MediaServiceTestContext { + pub media_service: Arc, + pub db_site: Arc, + pub cache: Arc, + } + + /// Test helper that encapsulates a test media item with its assertion logic + struct TestMedia { + id: MediaId, + title: String, + slug: String, + } + + impl TestMedia { + fn assert_matches(&self, media: &MediaWithEditContext) { + assert_eq!(media.id, self.id); + assert_eq!(media.title.rendered, self.title); + assert_eq!(media.slug, self.slug); + } + } + + /// Helper function to insert a test media item into the cache. + fn insert_test_media(ctx: &MediaServiceTestContext) -> TestMedia { + let test_media = TestMedia { + id: MediaId(4242), + title: "Test Media".to_string(), + slug: "test-media".to_string(), + }; + + let media = MediaBuilder::minimal() + .with_id(test_media.id.0) + .with_title(&test_media.title) + .with_slug(&test_media.slug) + .build(); + + ctx.cache + .execute(|conn| { + let repo = MediaRepository::::new(); + repo.upsert(conn, &ctx.db_site, &media) + }) + .expect("Media insert should succeed"); + + test_media + } + + #[rstest] + fn test_get_entity_load_data_returns_cached_media(ctx: MediaServiceTestContext) { + let test_media = insert_test_media(&ctx); + + let entity_id = ctx + .cache + .execute(|conn| { + let repo = MediaRepository::::new(); + repo.select_by_media_id(conn, &ctx.db_site, test_media.id) + .map(|opt| opt.map(|full_entity| *full_entity.entity_id)) + }) + .expect("Database read should succeed") + .expect("Media should exist"); + + let entity = ctx.media_service.get_entity_with_edit_context(entity_id); + let result = entity.0.load_data().expect("Database read should succeed"); + + let full_entity = result.expect("Media should be found in cache"); + test_media.assert_matches(&full_entity.data); + } + + #[rstest] + fn test_delete_by_entity_id(ctx: MediaServiceTestContext) { + let test_media = insert_test_media(&ctx); + let entity_id = ctx + .cache + .execute(|conn| { + let repo = MediaRepository::::new(); + repo.select_by_media_id(conn, &ctx.db_site, test_media.id) + .map(|opt| opt.map(|full_entity| *full_entity.entity_id)) + }) + .expect("Database read should succeed") + .expect("Media should exist"); + + let deleted = ctx + .media_service + .delete_by_entity_id(&entity_id) + .expect("Delete should succeed"); + + assert_eq!(deleted, 1, "Should delete 1 media item"); + + let result = ctx.cache.execute(|conn| { + let repo = MediaRepository::::new(); + repo.select_by_entity_id(conn, &entity_id) + }); + assert!( + result.unwrap().is_none(), + "Media should not exist after deletion" + ); + } + + #[rstest] + fn test_delete_by_media_id(ctx: MediaServiceTestContext) { + let test_media = insert_test_media(&ctx); + + let deleted = ctx + .media_service + .delete_by_media_id(test_media.id) + .expect("Delete should succeed"); + + assert_eq!(deleted, 1, "Should delete 1 media item"); + + let result = ctx.cache.execute(|conn| { + let repo = MediaRepository::::new(); + repo.select_by_media_id(conn, &ctx.db_site, test_media.id) + }); + assert!( + result.unwrap().is_none(), + "Media should not exist after deletion" + ); + } + + #[rstest] + fn test_delete_by_entity_id_non_existent_returns_zero(ctx: MediaServiceTestContext) { + let test_media = insert_test_media(&ctx); + let entity_id = ctx + .cache + .execute(|conn| { + let repo = MediaRepository::::new(); + repo.select_by_media_id(conn, &ctx.db_site, test_media.id) + .map(|opt| opt.map(|full_entity| *full_entity.entity_id)) + }) + .expect("Database read should succeed") + .expect("Media should exist"); + + ctx.media_service + .delete_by_entity_id(&entity_id) + .expect("First delete should succeed"); + + let deleted = ctx + .media_service + .delete_by_entity_id(&entity_id) + .expect("Delete should not error"); + + assert_eq!(deleted, 0, "Should return 0 for non-existent media"); + } + + #[rstest] + fn test_delete_by_media_id_non_existent_returns_zero(ctx: MediaServiceTestContext) { + let deleted = ctx + .media_service + .delete_by_media_id(MediaId(99999)) + .expect("Delete should not error"); + + assert_eq!(deleted, 0, "Should return 0 for non-existent media"); + } + + /// Helper to create a MediaService whose network requests always fail. + fn service_with_network_error() -> Arc { + let mock_executor = Arc::new(MockExecutor::with_execute_fn(|request| { + Err(RequestExecutionError::RequestExecutionFailed { + status_code: None, + redirects: None, + reason: RequestExecutionErrorReason::GenericError { + error_message: "Network timeout".to_string(), + }, + request_url: request.url().0, + request_method: request.method(), + }) + })); + + let api_root_url = + Arc::new(ParsedUrl::parse("https://test.local/wp-json").expect("Parse URL")); + let api_client = Arc::new(WpApiClient::new( + Arc::new(WpOrgSiteApiUrlResolver::new(api_root_url)), + WpApiClientDelegate { + auth_provider: Arc::new(WpAuthenticationProvider::none()), + request_executor: mock_executor, + middleware_pipeline: Arc::new(WpApiMiddlewarePipeline::default()), + app_notifier: Arc::new(EmptyAppNotifier), + }, + )); + + let mut conn = Connection::open_in_memory().expect("Create in-memory database"); + let mut migration_manager = MigrationManager::new(&conn).expect("Create migration manager"); + migration_manager + .perform_migrations() + .expect("Migrations succeed"); + + let site_repo = SiteRepository; + let self_hosted_site = SelfHostedSite { + url: "https://test.local".to_string(), + api_root: "https://test.local/wp-json".to_string(), + }; + let db_site = site_repo + .upsert_self_hosted_site(&mut conn, &self_hosted_site) + .expect("Site creation") + .db_site; + + let cache = Arc::new(WpApiCache::try_from(conn).expect("Cache creation should succeed")); + let db_site_arc = Arc::new(db_site); + Arc::new(MediaService::new(api_client, db_site_arc, cache)) + } + + #[tokio::test] + async fn test_load_media_by_ids_marks_all_as_failed_on_network_error() { + let service = service_with_network_error(); + + let result = service + .load_media_by_ids(vec![MediaId(1), MediaId(2)]) + .await; + + assert!(result.is_err(), "Network error should return Err"); + + let state1 = EntityStateService::get( + &service.cache, + &service.db_site, + EntityType::MediaEditContext, + 1, + ); + let state2 = EntityStateService::get( + &service.cache, + &service.db_site, + EntityType::MediaEditContext, + 2, + ); + assert!( + matches!(state1, crate::sync::DbEntityState::Failed { .. }), + "Media 1 should be marked as Failed on network error" + ); + assert!( + matches!(state2, crate::sync::DbEntityState::Failed { .. }), + "Media 2 should be marked as Failed on network error" + ); + } + + #[rstest] + fn test_create_media_metadata_collection_with_edit_context_returns_arc( + ctx: MediaServiceTestContext, + ) { + // Sanity check: the factory wires the stub collection without panicking. + let _collection = ctx + .media_service + .create_media_metadata_collection_with_edit_context(MediaListFilter::default(), 20); + } + + #[fixture] + fn ctx(mock_api_client: Arc) -> MediaServiceTestContext { + let mut conn = Connection::open_in_memory().expect("Failed to create in-memory database"); + let mut migration_manager = + MigrationManager::new(&conn).expect("Failed to create migration manager"); + migration_manager + .perform_migrations() + .expect("Migrations should succeed"); + + let site_repo = SiteRepository; + let self_hosted_site = SelfHostedSite { + url: "https://test.local".to_string(), + api_root: "https://test.local/wp-json".to_string(), + }; + let db_site = site_repo + .upsert_self_hosted_site(&mut conn, &self_hosted_site) + .expect("Site creation should succeed") + .db_site; + + let cache = Arc::new(WpApiCache::try_from(conn).expect("Cache creation should succeed")); + let db_site_arc = Arc::new(db_site); + let media_service = Arc::new(MediaService::new( + mock_api_client, + db_site_arc.clone(), + cache.clone(), + )); + + MediaServiceTestContext { + media_service, + db_site: db_site_arc, + cache, + } + } +} diff --git a/wp_mobile/src/service/mod.rs b/wp_mobile/src/service/mod.rs index d4205d683..0d0c62b25 100644 --- a/wp_mobile/src/service/mod.rs +++ b/wp_mobile/src/service/mod.rs @@ -1,10 +1,13 @@ use crate::service::sites::SiteInfo; -use crate::service::{post_types::PostTypeService, posts::PostService, sites::SiteService}; +use crate::service::{ + media::MediaService, post_types::PostTypeService, posts::PostService, sites::SiteService, +}; use std::sync::Arc; use wp_api::prelude::{ApiUrlResolver, WpApiClient, WpApiClientDelegate}; use wp_mobile_cache::{WpApiCache, db_types::db_site::DbSite}; pub mod entity_state_service; +pub mod media; pub mod metadata; pub mod mock_post_service; pub mod post_types; @@ -38,6 +41,7 @@ impl From for WpServiceError { /// domain-specific services like PostService, PostTypeService, etc. #[derive(uniffi::Object)] pub struct WpService { + media: Arc, posts: Arc, post_types: Arc, sites: Arc, @@ -59,9 +63,15 @@ impl WpService { db_site_arc.clone(), cache.clone(), )); + let media = Arc::new(MediaService::new( + api_client.clone(), + db_site_arc.clone(), + cache.clone(), + )); let post_types = Arc::new(PostTypeService::new(api_client, db_site_arc, cache)); Self { + media, posts, post_types, sites: site_service, @@ -111,6 +121,11 @@ impl WpService { self.post_types.clone() } + /// Get the media service for this WordPress site + pub fn media(&self) -> Arc { + self.media.clone() + } + /// Get the site service for this WordPress site pub fn sites(&self) -> Arc { self.sites.clone() From 9e25a66a92b1028853f20748217ad9c18ec928d6 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 13 May 2026 14:48:53 +1200 Subject: [PATCH 07/18] Order service accessors alphabetically in WpService --- wp_mobile/src/service/mod.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/wp_mobile/src/service/mod.rs b/wp_mobile/src/service/mod.rs index 0d0c62b25..e94635a9f 100644 --- a/wp_mobile/src/service/mod.rs +++ b/wp_mobile/src/service/mod.rs @@ -42,8 +42,8 @@ impl From for WpServiceError { #[derive(uniffi::Object)] pub struct WpService { media: Arc, - posts: Arc, post_types: Arc, + posts: Arc, sites: Arc, } @@ -58,12 +58,12 @@ impl WpService { let api_client = Arc::new(WpApiClient::new(api_url_resolver, delegate)); let db_site_arc = Arc::new(db_site); - let posts = Arc::new(PostService::new( + let media = Arc::new(MediaService::new( api_client.clone(), db_site_arc.clone(), cache.clone(), )); - let media = Arc::new(MediaService::new( + let posts = Arc::new(PostService::new( api_client.clone(), db_site_arc.clone(), cache.clone(), @@ -72,8 +72,8 @@ impl WpService { Self { media, - posts, post_types, + posts, sites: site_service, } } @@ -111,9 +111,9 @@ impl WpService { )) } - /// Get the post service for this WordPress site - pub fn posts(&self) -> Arc { - self.posts.clone() + /// Get the media service for this WordPress site + pub fn media(&self) -> Arc { + self.media.clone() } /// Get the post type service for this WordPress site @@ -121,9 +121,9 @@ impl WpService { self.post_types.clone() } - /// Get the media service for this WordPress site - pub fn media(&self) -> Arc { - self.media.clone() + /// Get the post service for this WordPress site + pub fn posts(&self) -> Arc { + self.posts.clone() } /// Get the site service for this WordPress site From 54d7752aa152cece5b6b0aee2e512c2d3a18de56 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 13 May 2026 14:54:26 +1200 Subject: [PATCH 08/18] Add MediaMetadataCollectionWithEditContext and wire factory on MediaService --- .../collection/media_metadata_collection.rs | 332 ++++++++++++++++++ wp_mobile/src/collection/mod.rs | 4 + wp_mobile/src/service/media.rs | 31 +- 3 files changed, 340 insertions(+), 27 deletions(-) create mode 100644 wp_mobile/src/collection/media_metadata_collection.rs diff --git a/wp_mobile/src/collection/media_metadata_collection.rs b/wp_mobile/src/collection/media_metadata_collection.rs new file mode 100644 index 000000000..54d7f3093 --- /dev/null +++ b/wp_mobile/src/collection/media_metadata_collection.rs @@ -0,0 +1,332 @@ +//! Media-specific metadata collection for efficient list syncing. + +use std::{collections::HashMap, sync::Arc}; + +use wp_api::media::MediaWithEditContext; +use wp_mobile_cache::{UpdateHook, entity::FullEntity}; + +use crate::{ + collection::{CollectionError, FetchError, MetadataCollectionCore}, + filters::MediaListFilter, + service::media::MediaService, + sync::{ListInfo, SyncResult}, +}; + +// Generate MediaItemState enum, MediaMetadataCollectionItem struct, and From trait +// implementations using the shared macro. This mirrors the post metadata collection +// without duplicating the boilerplate. +crate::wp_mobile_metadata_item!( + MediaMetadataCollectionItem, + MediaItemState, + crate::FullEntityMediaWithEditContext +); + +/// Metadata-first collection for media with edit context. +/// +/// This collection uses a two-phase sync strategy: +/// 1. Fetch lightweight metadata (id + modified_gmt) to define list structure +/// 2. Selectively fetch full data for missing or stale items +/// +/// Mirrors `PostMetadataCollectionWithEditContext` but without the membership-update +/// path, since `MediaService` does not expose mutation methods that would +/// notify collections of changes. +#[derive(uniffi::Object)] +pub struct MediaMetadataCollectionWithEditContext { + /// Core collection infrastructure (shared query logic) + core: MetadataCollectionCore, + + /// Reference to service for sync operations and loading entity data + service: Arc, + + /// Filter parameters for the media list + filter: MediaListFilter, +} + +impl MediaMetadataCollectionWithEditContext { + pub fn new( + core: MetadataCollectionCore, + service: Arc, + filter: MediaListFilter, + ) -> Self { + Self { + core, + service, + filter, + } + } +} + +#[uniffi::export] +impl MediaMetadataCollectionWithEditContext { + /// Load all items with their current states and data. + /// + /// Returns items in list order with type-safe state representation. + /// Each item's `state` is a [`MediaItemState`] variant that encodes both + /// the sync status and data availability. + /// + /// This is the primary method for getting collection contents to display. + /// + /// # Note + /// Data availability is independent of the internal `DbEntityState`. After an app + /// restart, items may have internal state `Missing` but still have cached data + /// available. This method will return `FetchingWithData`, `Stale`, or `FailedWithData` + /// variants appropriately when cached data exists. + /// + /// This async function is exported to client platforms (Kotlin/Swift) where it + /// will be executed on a background thread. The underlying Rust implementation + /// is synchronous as rusqlite doesn't support async operations. + pub async fn load_items(&self) -> Result, CollectionError> { + let Some(items) = self.core.items() else { + // No metadata loaded yet - return empty list + return Ok(Vec::new()); + }; + + // Load ALL media from cache - data availability is independent of DbEntityState. + // After app restart, DbEntityState resets to Missing but data may still be cached. + let all_ids: Vec = items.iter().map(|item| item.id()).collect(); + + let cached_media = if all_ids.is_empty() { + Vec::new() + } else { + self.service + .read_media_by_ids_from_db(&all_ids) + .map_err(|e| CollectionError::DatabaseError { + err_message: e.to_string(), + })? + }; + + // Build a map for quick lookup (using remove to take ownership) + let mut cached_map: HashMap> = + cached_media.into_iter().map(|m| (m.data.id.0, m)).collect(); + + // Convert CollectionItem + cached data → MediaMetadataCollectionItem using From trait + Ok(items + .into_iter() + .map(|item| { + let id = item.id(); + let cached_data = cached_map.remove(&id).map(Into::into); + MediaMetadataCollectionItem::from((item, cached_data)) + }) + .collect()) + } + + /// Refresh the collection (fetch page 1, replace metadata). + /// + /// This: + /// 1. Fetches metadata from the network (page 1) + /// 2. Replaces existing metadata in the store + /// 3. Fetches missing/stale entities + /// + /// Returns sync statistics including counts and pagination info. + pub async fn refresh(&self) -> Result { + log::debug!("MediaMetadataCollection: Refreshing collection"); + + // If the per_page configuration changed since this list was created, + // delete the stale list so it gets recreated with the correct per_page. + // This is safe because refresh replaces all list content anyway. + self.service + .metadata_service + .delete_list_if_per_page_changed(self.core.key(), self.core.per_page())?; + + let result = self + .service + .sync_list(self.core.key(), &self.filter, self.core.per_page(), true) + .await?; + + log::debug!( + "MediaMetadataCollection: Refreshed {} items, page 1 of {}, fetched {}, failed {}", + result.total_items, + result + .total_pages + .map(|p| p.to_string()) + .unwrap_or_else(|| "?".to_string()), + result.fetched_count, + result.failed_count + ); + + Ok(result) + } + + /// Load the next page of items. + /// + /// This: + /// 1. Fetches metadata for the next page + /// 2. Appends to existing metadata in the store + /// 3. Fetches missing/stale entities from the new page + /// + /// Returns `SyncResult::no_op()` if already on the last page. + pub async fn load_next_page(&self) -> Result { + // Delegate pagination logic to core orchestrator + self.core + .load_next_page_with(|| async { + let result = self + .service + .sync_list(self.core.key(), &self.filter, self.core.per_page(), false) + .await?; + + log::debug!( + "MediaMetadataCollection: Loaded page {} of {}: {} items total, fetched {}, failed {}", + result + .current_page + .map(|p| p.to_string()) + .unwrap_or_else(|| "?".to_string()), + result + .total_pages + .map(|p| p.to_string()) + .unwrap_or_else(|| "?".to_string()), + result.total_items, + result.fetched_count, + result.failed_count + ); + + Ok(result) + }) + .await + } + + /// Get combined list info (pagination + sync state) in a single query. + /// + /// Returns `None` if the list hasn't been created yet. + /// Use this instead of calling `current_page()`, `total_pages()`, `sync_state()` + /// separately to avoid multiple database queries. + pub fn list_info(&self) -> Option { + self.core.list_info() + } + + /// Check if there are more pages to load. + /// + /// Returns: + /// - `None` - Unknown (no metadata loaded or total_pages not provided by API) + /// - `Some(true)` - More pages available + /// - `Some(false)` - On last page + pub fn has_more_pages(&self) -> Option { + self.core.has_more_pages() + } + + /// Get the current page number. + /// + /// Returns: + /// - `None` - No metadata loaded yet + /// - `Some(n)` - Currently on page n + pub fn current_page(&self) -> Option { + self.core.current_page() + } + + /// Get the total number of pages, if known. + pub fn total_pages(&self) -> Option { + self.core.total_pages() + } + + /// 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. Observe state changes + /// via `is_relevant_state_update`. + /// + /// # Note + /// This async function is exported to client platforms (Kotlin/Swift) where it + /// will be executed on a background thread. The underlying Rust implementation + /// is synchronous as rusqlite doesn't support async operations. + pub async fn sync_state(&self) -> wp_mobile_cache::list_metadata::ListState { + self.core.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.core.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 (MediaEditContext) + /// - 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.core.is_relevant_data_update(hook) + } + + /// Check if a database update affects this collection's list info (pagination + state). + /// + /// Returns `true` if the update is to: + /// - `ListMetadata` table (pagination info changed) + /// - `ListMetadataState` table (sync state changed) + /// + /// Use this for listInfo observers that should update pagination display and loading indicators. + pub fn is_relevant_list_info_update(&self, hook: &UpdateHook) -> bool { + self.core.is_relevant_list_info_update(hook) + } + + /// Get the filter parameters for this collection. + pub fn filter(&self) -> MediaListFilter { + self.filter.clone() + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::service::media::MediaService; + use crate::testing::mock_api_client; + use rstest::*; + use rusqlite::Connection; + use std::sync::Arc; + use wp_api::api_client::WpApiClient; + use wp_mobile_cache::{ + MigrationManager, WpApiCache, db_types::self_hosted_site::SelfHostedSite, + repository::sites::SiteRepository, + }; + + /// Build a MediaService backed by an in-memory cache, mirroring the + /// fixture pattern used by `service::media` tests. + fn make_service(api_client: Arc) -> Arc { + let mut conn = Connection::open_in_memory().expect("Failed to create in-memory database"); + let mut migration_manager = + MigrationManager::new(&conn).expect("Failed to create migration manager"); + migration_manager + .perform_migrations() + .expect("Migrations should succeed"); + + let site_repo = SiteRepository; + let self_hosted_site = SelfHostedSite { + url: "https://test.local".to_string(), + api_root: "https://test.local/wp-json".to_string(), + }; + let db_site = site_repo + .upsert_self_hosted_site(&mut conn, &self_hosted_site) + .expect("Site creation should succeed") + .db_site; + + let cache = Arc::new(WpApiCache::try_from(conn).expect("Cache creation should succeed")); + Arc::new(MediaService::new(api_client, Arc::new(db_site), cache)) + } + + /// Smoke test: load_items returns empty on a fresh cache and filter() returns + /// the constructor-bound filter. Full sync/refresh flows are covered by the + /// integration test in Task 8. + #[rstest] + #[tokio::test] + async fn test_load_items_empty_and_filter_round_trip(mock_api_client: Arc) { + let service = make_service(mock_api_client); + let collection = service + .create_media_metadata_collection_with_edit_context(MediaListFilter::default(), 10); + + let items = collection + .load_items() + .await + .expect("load_items should succeed on fresh cache"); + assert!(items.is_empty(), "Fresh cache should yield no items"); + + let filter = collection.filter(); + assert_eq!(filter, MediaListFilter::default()); + } +} diff --git a/wp_mobile/src/collection/mod.rs b/wp_mobile/src/collection/mod.rs index ac17c31a3..669c81fc4 100644 --- a/wp_mobile/src/collection/mod.rs +++ b/wp_mobile/src/collection/mod.rs @@ -2,6 +2,7 @@ mod collection_error; mod core; mod fetch_error; mod fetch_result; +pub(crate) mod media_metadata_collection; pub(crate) mod post_collection; pub(crate) mod post_metadata_collection; pub(crate) mod post_type_collection; @@ -11,6 +12,9 @@ pub use collection_error::CollectionError; pub use core::MetadataCollectionCore; pub use fetch_error::FetchError; pub use fetch_result::FetchResult; +pub use media_metadata_collection::{ + MediaItemState, MediaMetadataCollectionItem, MediaMetadataCollectionWithEditContext, +}; pub use post_metadata_collection::{ PostItemState, PostMetadataCollectionItem, PostMetadataCollectionWithEditContext, }; diff --git a/wp_mobile/src/service/media.rs b/wp_mobile/src/service/media.rs index cbe098b82..0cbb92deb 100644 --- a/wp_mobile/src/service/media.rs +++ b/wp_mobile/src/service/media.rs @@ -1,7 +1,9 @@ use crate::{ EntityMediaWithEditContext, cache_key::media_list_filter_cache_key, - collection::{FetchError, FetchResult, MetadataCollectionCore}, + collection::{ + FetchError, FetchResult, MediaMetadataCollectionWithEditContext, MetadataCollectionCore, + }, filters::MediaListFilter, service::{ entity_state_service::{EntityStateReader, EntityStateReaderImpl, EntityStateService}, @@ -47,31 +49,6 @@ pub(crate) struct FetchStats { pub(crate) failed_count: usize, } -/// Stub for `MediaMetadataCollectionWithEditContext`. -/// -/// TODO: replaced by Task 7. This placeholder lets `MediaService` expose the -/// collection factory now; the real type lands with the collection module. -#[derive(uniffi::Object)] -pub struct MediaMetadataCollectionWithEditContext { - _core: MetadataCollectionCore, - _service: Arc, - _filter: MediaListFilter, -} - -impl MediaMetadataCollectionWithEditContext { - pub fn new( - core: MetadataCollectionCore, - service: Arc, - filter: MediaListFilter, - ) -> Self { - Self { - _core: core, - _service: service, - _filter: filter, - } - } -} - /// Service layer for media operations /// /// Provides a bridge between clients and the underlying network/cache layers. @@ -913,7 +890,7 @@ mod tests { fn test_create_media_metadata_collection_with_edit_context_returns_arc( ctx: MediaServiceTestContext, ) { - // Sanity check: the factory wires the stub collection without panicking. + // Sanity check: the factory wires the collection without panicking. let _collection = ctx .media_service .create_media_metadata_collection_with_edit_context(MediaListFilter::default(), 20); From 5757897c489e293ac2110bb4147d258ad68739c6 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 13 May 2026 15:41:26 +1200 Subject: [PATCH 09/18] Add integration test for MediaMetadataCollectionWithEditContext --- .../tests/test_media_collection.rs | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 wp_mobile_integration_tests/tests/test_media_collection.rs diff --git a/wp_mobile_integration_tests/tests/test_media_collection.rs b/wp_mobile_integration_tests/tests/test_media_collection.rs new file mode 100644 index 000000000..718d2bd84 --- /dev/null +++ b/wp_mobile_integration_tests/tests/test_media_collection.rs @@ -0,0 +1,44 @@ +use wp_mobile::collection::MediaItemState; +use wp_mobile::filters::MediaListFilter; +use wp_mobile_integration_tests::*; + +#[tokio::test] +#[parallel] +async fn test_refresh_loads_media_items() { + let ctx = create_test_context(); + + let collection = ctx + .service + .media() + .create_media_metadata_collection_with_edit_context(MediaListFilter::default(), 5); + + let result = collection + .refresh() + .await + .expect("refresh should succeed against the test server"); + + assert!( + result.total_items > 0, + "expected the test instance to have at least one media item; refresh returned {} total", + result.total_items + ); + + let items = collection + .load_items() + .await + .expect("load_items should succeed after refresh"); + + assert_eq!( + items.len(), + (result.total_items as usize).min(5), + "loaded items should match the first page size or total, whichever is smaller" + ); + + for item in &items { + assert!( + matches!(item.state, MediaItemState::Fresh { .. }), + "all items should be Fresh after refresh, got {:?}", + item.state + ); + } +} From 92e963bd0aaa853fc60e7f689bfc84197007075d Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 13 May 2026 16:06:34 +1200 Subject: [PATCH 10/18] Add failing test: load_media_by_ids should send all attachment statuses --- wp_mobile/src/service/media.rs | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/wp_mobile/src/service/media.rs b/wp_mobile/src/service/media.rs index 0cbb92deb..138ac9305 100644 --- a/wp_mobile/src/service/media.rs +++ b/wp_mobile/src/service/media.rs @@ -854,6 +854,35 @@ mod tests { Arc::new(MediaService::new(api_client, db_site_arc, cache)) } + #[tokio::test] + async fn test_load_media_by_ids_includes_all_attachment_statuses_in_request() { + // The metadata pass can use `MediaListFilter.status = [Private]`, so the + // hydration follow-up via `include` must explicitly pass every core + // attachment status to bypass the REST controller's `status=inherit` + // default. Otherwise the included IDs get filtered back out and end up + // marked Failed("Not found"). + let service = service_with_network_error(); + + let result = service + .load_media_by_ids(vec![MediaId(1), MediaId(2)]) + .await; + + let request_url = match result { + Err(FetchError::Api(WpApiError::RequestExecutionFailed { request_url, .. })) => { + request_url + } + Err(other) => panic!("Expected RequestExecutionFailed, got: {:?}", other), + Ok(_) => panic!("Expected network error, got Ok"), + }; + + // URL-encoded comma is %2C + assert!( + request_url.contains("status=inherit%2Cprivate%2Ctrash"), + "expected request URL to include status=inherit,private,trash; got {}", + request_url + ); + } + #[tokio::test] async fn test_load_media_by_ids_marks_all_as_failed_on_network_error() { let service = service_with_network_error(); From dd713c77926a8f7b75ac3d17bdf56a3731a8bae3 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 13 May 2026 16:07:27 +1200 Subject: [PATCH 11/18] Fix load_media_by_ids: send all attachment statuses to bypass REST default --- wp_mobile/src/service/media.rs | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/wp_mobile/src/service/media.rs b/wp_mobile/src/service/media.rs index 138ac9305..1436944a2 100644 --- a/wp_mobile/src/service/media.rs +++ b/wp_mobile/src/service/media.rs @@ -15,7 +15,7 @@ use std::{collections::HashSet, sync::Arc}; use wp_api::{ api_client::WpApiClient, media::{ - MediaDeleteResponse, MediaId, MediaListParams, MediaWithEditContext, + MediaDeleteResponse, MediaId, MediaListParams, MediaStatus, MediaWithEditContext, SparseMediaFieldWithEditContext, }, }; @@ -31,6 +31,15 @@ use wp_mobile_cache::{ /// Maximum number of media items to fetch in a single batch request const BATCH_FETCH_SIZE: usize = 100; +/// All core WordPress attachment statuses. Used by `load_media_by_ids` to +/// bypass the REST default of `status=inherit` so we can hydrate items the +/// metadata pass returned via a status filter on the user's `MediaListFilter`. +const ALL_ATTACHMENT_STATUSES: &[MediaStatus] = &[ + MediaStatus::Inherit, + MediaStatus::Private, + MediaStatus::Trash, +]; + // Internal types /// Result from loading media by IDs. @@ -369,8 +378,10 @@ impl MediaService { /// Load media items by IDs from network to cache with state tracking. /// - /// Mirrors `PostService::load_posts_by_ids`. Unlike posts, `MediaListParams` - /// has no `status` field defaulting to "any+trash"; the API default applies. + /// Mirrors `PostService::load_posts_by_ids`. Passes `ALL_ATTACHMENT_STATUSES` + /// explicitly so the REST controller's `status=inherit` default doesn't filter + /// out IDs the metadata pass surfaced via a `status` filter (e.g. `Private` or + /// `Trash`). pub async fn load_media_by_ids( &self, ids: Vec, @@ -410,6 +421,7 @@ impl MediaService { let params = MediaListParams { include: media_ids, per_page: Some(BATCH_FETCH_SIZE as u32), + status: ALL_ATTACHMENT_STATUSES.to_vec(), ..Default::default() }; From 0c42025e07ffba71bd7619eb7d6272e375681b7f Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 13 May 2026 16:09:10 +1200 Subject: [PATCH 12/18] Add failing test: delete_media_permanently should scrub list metadata --- wp_mobile/src/service/media.rs | 88 ++++++++++++++++++++++++++++++++++ 1 file changed, 88 insertions(+) diff --git a/wp_mobile/src/service/media.rs b/wp_mobile/src/service/media.rs index 1436944a2..ad702b185 100644 --- a/wp_mobile/src/service/media.rs +++ b/wp_mobile/src/service/media.rs @@ -937,6 +937,94 @@ mod tests { .create_media_metadata_collection_with_edit_context(MediaListFilter::default(), 20); } + /// Tests the cleanup helper directly (approach b in the bug-fix spec). + /// + /// Going through `delete_media_permanently` would require mocking a valid + /// `MediaDeleteResponse` JSON which adds a lot of brittle setup, so this + /// test covers the cleanup helper that `delete_media_permanently` calls. + #[rstest] + fn test_remove_entity_from_lists_with_key_prefix_only_removes_from_matching_keys( + ctx: MediaServiceTestContext, + ) { + use wp_mobile_cache::repository::list_metadata::{ + ListMetadataItemInput, ListMetadataRepository, + }; + + let media_key: ListKey = + format!("site_{:?}:edit:media:filter=fake", ctx.db_site.row_id).into(); + let posts_key: ListKey = format!("site_{:?}:edit:posts:foo", ctx.db_site.row_id).into(); + let entity_id: i64 = 42; + + // Seed two list_metadata rows (one media-prefixed, one posts-prefixed) + // and put `entity_id` into both. + ctx.cache + .execute(|conn| { + let item = ListMetadataItemInput { + entity_id, + modified_gmt: None, + parent: None, + menu_order: None, + }; + ListMetadataRepository::set_items_by_list_key( + conn, + &ctx.db_site, + &media_key, + 25, + std::slice::from_ref(&item), + )?; + ListMetadataRepository::set_items_by_list_key( + conn, + &ctx.db_site, + &posts_key, + 25, + std::slice::from_ref(&item), + ) + }) + .expect("Seeding list metadata should succeed"); + + // Sanity: both lists reference the entity before cleanup. + assert!( + ctx.media_service + .metadata_service + .list_contains_entity(&media_key, entity_id) + .expect("contains check"), + "media list should contain the entity before cleanup" + ); + assert!( + ctx.media_service + .metadata_service + .list_contains_entity(&posts_key, entity_id) + .expect("contains check"), + "posts list should contain the entity before cleanup" + ); + + // Act: scrub the entity from media-prefixed lists only. + let media_list_prefix = format!("site_{:?}:edit:media:", ctx.db_site.row_id); + let removed = ctx + .media_service + .metadata_service + .remove_entity_from_lists_with_key_prefix(&media_list_prefix, entity_id) + .expect("cleanup should succeed"); + assert_eq!(removed, 1, "should remove exactly one row"); + + // Media list no longer references 42. + assert!( + !ctx.media_service + .metadata_service + .list_contains_entity(&media_key, entity_id) + .expect("contains check"), + "media list should NOT contain the entity after cleanup" + ); + // Posts list still references 42 (no over-match on the prefix). + assert!( + ctx.media_service + .metadata_service + .list_contains_entity(&posts_key, entity_id) + .expect("contains check"), + "posts list should still contain the entity (prefix-scoped delete must not over-match)" + ); + } + #[fixture] fn ctx(mock_api_client: Arc) -> MediaServiceTestContext { let mut conn = Connection::open_in_memory().expect("Failed to create in-memory database"); From 7c483fb319c55f0d78de289b26652e58a552c2d1 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 13 May 2026 16:11:41 +1200 Subject: [PATCH 13/18] Scrub deleted media from list metadata in delete_media_permanently --- wp_mobile/src/service/media.rs | 17 +++ wp_mobile/src/service/metadata.rs | 24 ++++ .../src/repository/list_metadata.rs | 107 ++++++++++++++++++ 3 files changed, 148 insertions(+) diff --git a/wp_mobile/src/service/media.rs b/wp_mobile/src/service/media.rs index ad702b185..39a938657 100644 --- a/wp_mobile/src/service/media.rs +++ b/wp_mobile/src/service/media.rs @@ -619,6 +619,23 @@ impl MediaService { err_message: e.to_string(), })?; + // Scrub the deleted media from every cached media list. Without this, + // collection load_items would return a phantom row that converts to + // `Missing`/`Stale` until the next full refresh. Failure here is + // logged but not propagated: the REST delete and local row delete + // already succeeded. + let media_list_prefix = format!("site_{:?}:edit:media:", self.db_site.row_id); + if let Err(e) = self + .metadata_service + .remove_entity_from_lists_with_key_prefix(&media_list_prefix, media_id.0) + { + log::warn!( + "Failed to remove deleted media id {} from list metadata: {}", + media_id.0, + e + ); + } + Ok(response) } diff --git a/wp_mobile/src/service/metadata.rs b/wp_mobile/src/service/metadata.rs index 5351129f8..ead8e1037 100644 --- a/wp_mobile/src/service/metadata.rs +++ b/wp_mobile/src/service/metadata.rs @@ -140,6 +140,30 @@ impl MetadataService { })?) } + /// Remove `entity_id` from every list whose key starts with `key_prefix` + /// for this site. + /// + /// Used by service-level deletes (e.g. `MediaService::delete_media_permanently`) + /// to scrub the deleted entity from every cached list immediately, so observers + /// see the removal without waiting for a full refresh. + /// + /// Returns the number of rows removed across all lists. Returns `Ok(0)` if the + /// entity wasn't referenced in any matching list. + pub fn remove_entity_from_lists_with_key_prefix( + &self, + key_prefix: &str, + entity_id: i64, + ) -> Result { + Ok(self.cache.execute(|conn| { + ListMetadataRepository::remove_entity_from_lists_with_key_prefix( + conn, + &self.db_site, + key_prefix, + entity_id, + ) + })?) + } + /// Get list metadata as EntityMetadata structs (for ListMetadataReader trait). /// /// Converts database items to the format expected by MetadataCollection. diff --git a/wp_mobile_cache/src/repository/list_metadata.rs b/wp_mobile_cache/src/repository/list_metadata.rs index ea0b2813c..bfd95288d 100644 --- a/wp_mobile_cache/src/repository/list_metadata.rs +++ b/wp_mobile_cache/src/repository/list_metadata.rs @@ -510,6 +510,47 @@ impl ListMetadataRepository { Ok(()) } + /// Remove a single entity ID from every list metadata items row whose + /// containing list_metadata row matches the given site and key prefix. + /// + /// Used by service-level deletes to scrub a deleted entity from every + /// collection's stored list (so the collection doesn't return a phantom + /// `Missing` row until the next full refresh). + /// + /// Returns the number of rows deleted. + pub fn remove_entity_from_lists_with_key_prefix( + executor: &impl QueryExecutor, + site: &DbSite, + key_prefix: &str, + entity_id: i64, + ) -> Result { + // SQLite's `LIKE` treats `_` as "any single character" and `%` as + // "any sequence". The expected prefix has no SQL wildcards by design + // (e.g. `site_RowId(1):edit:media:`), but escape defensively so an + // unexpected caller can't widen the match. + let escaped_prefix = key_prefix + .replace('\\', r"\\") + .replace('%', r"\%") + .replace('_', r"\_"); + let like_pattern = format!("{}%", escaped_prefix); + + let sql = format!( + "DELETE FROM {items} \ + WHERE entity_id = ? \ + AND list_metadata_id IN (\ + SELECT rowid FROM {header} \ + WHERE db_site_id = ? AND key LIKE ? ESCAPE '\\'\ + )", + items = Self::items_table().table_name(), + header = Self::header_table().table_name(), + ); + + executor.execute( + &sql, + rusqlite::params![entity_id, site.row_id, like_pattern], + ) + } + /// Delete all data for a list (header, items, and state). pub fn delete_list( executor: &impl QueryExecutor, @@ -1272,6 +1313,72 @@ mod tests { ); } + #[rstest] + fn test_remove_entity_from_lists_with_key_prefix_scopes_by_prefix(test_ctx: TestContext) { + // The `_` character in a LIKE pattern matches any single character by + // default. Without an ESCAPE clause, the prefix `site_1:edit:media:` + // would also match `siteX:edit:media:` (or anything where `_` is some + // other char). This test guards both correct prefix matching and the + // escape behaviour. + let media_key = ListKey::from("site_1:edit:media:filter=fake"); + let posts_key = ListKey::from("site_1:edit:posts:foo"); + let entity_id: i64 = 42; + + let item = ListMetadataItemInput { + entity_id, + modified_gmt: None, + parent: None, + menu_order: None, + }; + + ListMetadataRepository::set_items_by_list_key( + &test_ctx.conn, + &test_ctx.site, + &media_key, + TEST_PER_PAGE, + std::slice::from_ref(&item), + ) + .expect("should succeed"); + ListMetadataRepository::set_items_by_list_key( + &test_ctx.conn, + &test_ctx.site, + &posts_key, + TEST_PER_PAGE, + std::slice::from_ref(&item), + ) + .expect("should succeed"); + + let removed = ListMetadataRepository::remove_entity_from_lists_with_key_prefix( + &test_ctx.conn, + &test_ctx.site, + "site_1:edit:media:", + entity_id, + ) + .expect("should succeed"); + assert_eq!(removed, 1, "should remove exactly the media row"); + + // Media list no longer references the entity. + assert!( + !ListMetadataRepository::contains_entity( + &test_ctx.conn, + &test_ctx.site, + &media_key, + entity_id, + ) + .expect("should succeed") + ); + // Posts list still references the entity (prefix did not over-match). + assert!( + ListMetadataRepository::contains_entity( + &test_ctx.conn, + &test_ctx.site, + &posts_key, + entity_id, + ) + .expect("should succeed") + ); + } + #[rstest] fn test_items_preserve_order(test_ctx: TestContext) { let key = ListKey::from("edit:posts:ordered"); From 2cb922898695ba5776229ab9f247c7fa5daf6cce Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 13 May 2026 16:18:51 +1200 Subject: [PATCH 14/18] Rename ALL_ATTACHMENT_STATUSES to ALL_CORE_ATTACHMENT_STATUSES and document custom-status gap --- wp_mobile/src/service/media.rs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/wp_mobile/src/service/media.rs b/wp_mobile/src/service/media.rs index 39a938657..6e820a21c 100644 --- a/wp_mobile/src/service/media.rs +++ b/wp_mobile/src/service/media.rs @@ -31,10 +31,16 @@ use wp_mobile_cache::{ /// Maximum number of media items to fetch in a single batch request const BATCH_FETCH_SIZE: usize = 100; -/// All core WordPress attachment statuses. Used by `load_media_by_ids` to +/// All WordPress *core* attachment statuses. Used by `load_media_by_ids` to /// bypass the REST default of `status=inherit` so we can hydrate items the /// metadata pass returned via a status filter on the user's `MediaListFilter`. -const ALL_ATTACHMENT_STATUSES: &[MediaStatus] = &[ +/// +/// Limitation: plugin-defined custom attachment statuses (`MediaStatus::Custom`) +/// are not enumerated here, so a metadata-pass hit on a custom-status item +/// would still fall through to `Failed("Not found")` on hydration. Threading +/// the caller's filter statuses through `load_media_by_ids` would close that +/// gap; not worth the signature change until a real consumer requires it. +const ALL_CORE_ATTACHMENT_STATUSES: &[MediaStatus] = &[ MediaStatus::Inherit, MediaStatus::Private, MediaStatus::Trash, @@ -378,7 +384,7 @@ impl MediaService { /// Load media items by IDs from network to cache with state tracking. /// - /// Mirrors `PostService::load_posts_by_ids`. Passes `ALL_ATTACHMENT_STATUSES` + /// Mirrors `PostService::load_posts_by_ids`. Passes `ALL_CORE_ATTACHMENT_STATUSES` /// explicitly so the REST controller's `status=inherit` default doesn't filter /// out IDs the metadata pass surfaced via a `status` filter (e.g. `Private` or /// `Trash`). @@ -421,7 +427,7 @@ impl MediaService { let params = MediaListParams { include: media_ids, per_page: Some(BATCH_FETCH_SIZE as u32), - status: ALL_ATTACHMENT_STATUSES.to_vec(), + status: ALL_CORE_ATTACHMENT_STATUSES.to_vec(), ..Default::default() }; From 82e59a671e45845d1958b40b78efb6458959c181 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 13 May 2026 17:18:55 +1200 Subject: [PATCH 15/18] Add failing test: load_media_by_ids should include custom statuses from filter --- wp_mobile/src/service/media.rs | 39 ++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/wp_mobile/src/service/media.rs b/wp_mobile/src/service/media.rs index 6e820a21c..551021a3a 100644 --- a/wp_mobile/src/service/media.rs +++ b/wp_mobile/src/service/media.rs @@ -918,6 +918,45 @@ mod tests { ); } + #[tokio::test] + async fn test_load_media_by_ids_includes_custom_status_from_filter() { + // Regression test for code-review-2 Finding 2: custom attachment statuses + // from the caller's filter must be included in the hydration request so + // custom-status items don't fall through to Failed("Not found"). + let service = service_with_network_error(); + let custom_status = wp_api::media::MediaStatus::Custom("workflow".to_string()); + let result = service + .load_media_by_ids(vec![wp_api::media::MediaId(1)], &[custom_status]) + .await; + + let url = match result { + Err(FetchError::Api(WpApiError::RequestExecutionFailed { request_url, .. })) => { + request_url + } + Err(other) => panic!("expected RequestExecutionFailed; got {:?}", other), + Ok(_) => panic!("Expected network error, got Ok"), + }; + + // URL-encoded comma is %2C. The hydration request must include the custom + // status in addition to the core defaults. + assert!( + url.contains("status="), + "expected request URL to include status= param; got {}", + url + ); + assert!( + url.contains("workflow"), + "expected request URL to include the custom status `workflow`; got {}", + url + ); + // Core statuses must still be present so this doesn't regress Finding 1. + assert!( + url.contains("inherit") && url.contains("private") && url.contains("trash"), + "expected request URL to include all core statuses; got {}", + url + ); + } + #[tokio::test] async fn test_load_media_by_ids_marks_all_as_failed_on_network_error() { let service = service_with_network_error(); From c121d7b90b47149594a1b3b1164c33c827029fae Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 13 May 2026 17:20:13 +1200 Subject: [PATCH 16/18] Thread caller filter statuses through load_media_by_ids --- wp_mobile/src/service/media.rs | 41 +++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/wp_mobile/src/service/media.rs b/wp_mobile/src/service/media.rs index 551021a3a..fd07a207d 100644 --- a/wp_mobile/src/service/media.rs +++ b/wp_mobile/src/service/media.rs @@ -31,15 +31,10 @@ use wp_mobile_cache::{ /// Maximum number of media items to fetch in a single batch request const BATCH_FETCH_SIZE: usize = 100; -/// All WordPress *core* attachment statuses. Used by `load_media_by_ids` to -/// bypass the REST default of `status=inherit` so we can hydrate items the -/// metadata pass returned via a status filter on the user's `MediaListFilter`. -/// -/// Limitation: plugin-defined custom attachment statuses (`MediaStatus::Custom`) -/// are not enumerated here, so a metadata-pass hit on a custom-status item -/// would still fall through to `Failed("Not found")` on hydration. Threading -/// the caller's filter statuses through `load_media_by_ids` would close that -/// gap; not worth the signature change until a real consumer requires it. +/// Core WordPress attachment statuses. Used by `load_media_by_ids` as the +/// baseline for hydration requests; the caller's filter statuses are unioned +/// in so a `MediaListFilter` containing `MediaStatus::Custom(...)` still +/// produces a hydration request that matches what the metadata pass returned. const ALL_CORE_ATTACHMENT_STATUSES: &[MediaStatus] = &[ MediaStatus::Inherit, MediaStatus::Private, @@ -308,7 +303,7 @@ impl MediaService { failed_count: 0, }, SyncStrategy::Full => { - self.fetch_missing_and_stale_media(&metadata_result.metadata) + self.fetch_missing_and_stale_media(&metadata_result.metadata, filter) .await } }; @@ -339,6 +334,7 @@ impl MediaService { pub(crate) async fn fetch_missing_and_stale_media( &self, metadata: &[EntityMetadata], + filter: &MediaListFilter, ) -> FetchStats { let ids_to_fetch: Vec = metadata .iter() @@ -359,7 +355,7 @@ impl MediaService { if !ids_to_fetch.is_empty() { for chunk in ids_to_fetch.chunks(BATCH_FETCH_SIZE) { - match self.load_media_by_ids(chunk.to_vec()).await { + match self.load_media_by_ids(chunk.to_vec(), &filter.status).await { Ok(result) => { failed_count += result.failed_count; } @@ -384,13 +380,15 @@ impl MediaService { /// Load media items by IDs from network to cache with state tracking. /// - /// Mirrors `PostService::load_posts_by_ids`. Passes `ALL_CORE_ATTACHMENT_STATUSES` - /// explicitly so the REST controller's `status=inherit` default doesn't filter - /// out IDs the metadata pass surfaced via a `status` filter (e.g. `Private` or - /// `Trash`). + /// Mirrors `PostService::load_posts_by_ids`. The request's `status` query + /// param is the union of `ALL_CORE_ATTACHMENT_STATUSES` and + /// `additional_statuses`, so the hydration request matches whatever the + /// metadata pass returned (including plugin-defined `MediaStatus::Custom` + /// values from the caller's filter). pub async fn load_media_by_ids( &self, ids: Vec, + additional_statuses: &[MediaStatus], ) -> Result { if ids.is_empty() { return Ok(LoadByIdsResult { @@ -424,10 +422,17 @@ impl MediaService { let media_ids: Vec = fetchable.iter().map(|&id| MediaId(id)).collect(); + let mut statuses: Vec = ALL_CORE_ATTACHMENT_STATUSES.to_vec(); + for status in additional_statuses { + if !statuses.contains(status) { + statuses.push(status.clone()); + } + } + let params = MediaListParams { include: media_ids, per_page: Some(BATCH_FETCH_SIZE as u32), - status: ALL_CORE_ATTACHMENT_STATUSES.to_vec(), + status: statuses, ..Default::default() }; @@ -899,7 +904,7 @@ mod tests { let service = service_with_network_error(); let result = service - .load_media_by_ids(vec![MediaId(1), MediaId(2)]) + .load_media_by_ids(vec![MediaId(1), MediaId(2)], &[]) .await; let request_url = match result { @@ -962,7 +967,7 @@ mod tests { let service = service_with_network_error(); let result = service - .load_media_by_ids(vec![MediaId(1), MediaId(2)]) + .load_media_by_ids(vec![MediaId(1), MediaId(2)], &[]) .await; assert!(result.is_err(), "Network error should return Err"); From 369abb8b6535e08e0fc0066017c394ab673fb5d5 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Thu, 14 May 2026 09:25:44 +1200 Subject: [PATCH 17/18] Derive Hashable on MediaDetails and MediaWithEditContext for Swift bindings SparseMedia opted out of PartialEq via #[WpContextualDontDerivePartialEq] because MediaDetails.payload (Box) can't auto-derive PartialEq. That suppression cascaded to MediaWithEditContext and broke Swift's auto-synthesis for FullEntityMediaWithEditContext. Hand-implement PartialEq/Eq/Hash on MediaDetails by raw-JSON-string comparison, mark it exported to Swift via #[uniffi::export(Eq, Hash)], drop the contextual opt-out so the contextual media types derive the traits like the post side, and extend the Swift bindings patch script to add Hashable extensions for the contextual media types (MediaDetails is a reference type so uniffi can't auto-synthesize the conformance on the Record). --- scripts/swift-bindings.sh | 6 ++++++ wp_api/src/media.rs | 16 +++++++++++++++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/scripts/swift-bindings.sh b/scripts/swift-bindings.sh index 2b4563774..e4f33bbd8 100755 --- a/scripts/swift-bindings.sh +++ b/scripts/swift-bindings.sh @@ -80,6 +80,12 @@ import SQLite3\ extension AnyPostWithEditContext: Hashable {} extension AnyPostWithEmbedContext: Hashable {} extension AnyPostWithViewContext: Hashable {} + +// MediaWith*Context types contain `MediaDetails` (a reference type) which +// prevents automatic Hashable synthesis. Add the conformance manually. +extension MediaWithEditContext: Hashable {} +extension MediaWithEmbedContext: Hashable {} +extension MediaWithViewContext: Hashable {} PATCH } diff --git a/wp_api/src/media.rs b/wp_api/src/media.rs index 3c236a8c5..bafd85a50 100644 --- a/wp_api/src/media.rs +++ b/wp_api/src/media.rs @@ -359,7 +359,6 @@ impl From for HashMap { } #[derive(Debug, Serialize, Deserialize, uniffi::Record, WpContextual)] -#[WpContextualDontDerivePartialEq] pub struct SparseMedia { #[WpContext(edit, embed, view)] pub id: Option, @@ -428,6 +427,7 @@ pub struct SparseMedia { #[derive(Debug, Serialize, Deserialize, uniffi::Object)] #[serde(transparent)] +#[uniffi::export(Eq, Hash)] pub struct MediaDetails { pub payload: Box, } @@ -457,6 +457,20 @@ impl MediaDetails { } } +impl PartialEq for MediaDetails { + fn eq(&self, other: &Self) -> bool { + self.payload.get() == other.payload.get() + } +} + +impl Eq for MediaDetails {} + +impl std::hash::Hash for MediaDetails { + fn hash(&self, state: &mut H) { + self.payload.get().hash(state); + } +} + #[derive(Debug, uniffi::Enum)] pub enum MediaDetailsPayload { Audio(AudioMediaDetails), From 3dc56314c47721f53c2ec1004f43c4e6075c18ea Mon Sep 17 00:00:00 2001 From: Tony Li Date: Thu, 14 May 2026 09:55:38 +1200 Subject: [PATCH 18/18] Add change logs --- CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c5408bb0..d5ee308a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - [Global Styles](https://developer.wordpress.org/rest-api/reference/wp_global_styles/) endpoint - [Pattern Directory Items](https://developer.wordpress.org/rest-api/reference/pattern-directory-items/) endpoint - [Rendered Blocks](https://developer.wordpress.org/rest-api/reference/rendered-blocks/) endpoint +- `MediaService` on `WpService` (sync, fetch, state tracking, `delete_media_permanently`) and `MediaMetadataCollectionWithEditContext`, mirroring the existing `PostService` / `PostMetadataCollectionWithEditContext` pattern for a cached, paginated, observable media list +- `MediaListFilter`, the subset of `MediaListParams` that backs `MediaService.create_media_metadata_collection_with_edit_context` (excludes pagination, include/exclude, and date ranges) +- `wp_mobile_cache` storage for media: `media_edit_context` table (migration 0014), `DbTable::MediaEditContext`, `EntityType::MediaEditContext`, and a `MediaRepository` mirroring `PostRepository` minus term relationships +- `MetadataService::remove_entity_from_lists_with_key_prefix` so service-level deletes can scrub a deleted entity from every cached list for a site without waiting for a refresh + +### Changed + +- `MediaDetails` now derives `Eq + Hash` (raw-JSON-string comparison, FIXME left in place) and is exported via `#[uniffi::export(Eq, Hash)]`; `SparseMedia`'s `#[WpContextualDontDerivePartialEq]` opt-out is removed so `MediaWithEditContext` and the generated `FullEntityMediaWithEditContext` Swift wrapper synthesize `Equatable + Hashable` ### Removed