diff --git a/Cargo.lock b/Cargo.lock index 9bc0f9629..166c3907a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5306,6 +5306,7 @@ name = "wp_mobile_cache" version = "0.1.0" dependencies = [ "chrono", + "integration_test_credentials", "rstest", "rusqlite", "serde", diff --git a/native/kotlin/api/kotlin/src/integrationTest/kotlin/WordPressApiCacheTest.kt b/native/kotlin/api/kotlin/src/integrationTest/kotlin/WordPressApiCacheTest.kt index bc88e842e..7ccfe48a0 100644 --- a/native/kotlin/api/kotlin/src/integrationTest/kotlin/WordPressApiCacheTest.kt +++ b/native/kotlin/api/kotlin/src/integrationTest/kotlin/WordPressApiCacheTest.kt @@ -11,7 +11,7 @@ class WordPressApiCacheTest { @Test fun testThatMigrationsWork() = runTest { - assertEquals(5, WordPressApiCache().performMigrations()) + assertEquals(6, WordPressApiCache().performMigrations()) } @Test diff --git a/native/swift/Tests/wordpress-api-cache/WordPressApiCacheTests.swift b/native/swift/Tests/wordpress-api-cache/WordPressApiCacheTests.swift index 6b471f1c4..706635d37 100644 --- a/native/swift/Tests/wordpress-api-cache/WordPressApiCacheTests.swift +++ b/native/swift/Tests/wordpress-api-cache/WordPressApiCacheTests.swift @@ -13,7 +13,7 @@ actor Test { @Test func testMigrationsWork() async throws { let migrationsPerformed = try await self.cache.performMigrations() - #expect(migrationsPerformed == 5) + #expect(migrationsPerformed == 6) } #if !os(Linux) diff --git a/wp_mobile_cache/Cargo.toml b/wp_mobile_cache/Cargo.toml index ed7e1323a..9816a14ca 100644 --- a/wp_mobile_cache/Cargo.toml +++ b/wp_mobile_cache/Cargo.toml @@ -16,4 +16,5 @@ wp_api = { path = "../wp_api" } [dev-dependencies] chrono = { workspace = true } +integration_test_credentials = { path = "../integration_test_credentials" } rstest = "0.24" diff --git a/wp_mobile_cache/migrations/0001-create-sites-table.sql b/wp_mobile_cache/migrations/0001-create-sites-table.sql index f1a2dc510..f4db11496 100644 --- a/wp_mobile_cache/migrations/0001-create-sites-table.sql +++ b/wp_mobile_cache/migrations/0001-create-sites-table.sql @@ -1,4 +1,14 @@ -CREATE TABLE `sites` ( +CREATE TABLE `db_sites` ( -- Internal DB field (auto-incrementing) - `id` INTEGER PRIMARY KEY AUTOINCREMENT + `id` INTEGER PRIMARY KEY AUTOINCREMENT, + + -- Type of site (0 = SelfHosted, 1 = WordPressCom) + `site_type` INTEGER NOT NULL, + + -- Reference to type-specific table (self_hosted_sites.id or wordpress_com_sites.id) + -- Note: Not a foreign key constraint since it can point to different tables + `mapped_site_id` INTEGER NOT NULL ) STRICT; + +-- Unique constraint to prevent duplicate site mappings +CREATE UNIQUE INDEX idx_db_sites_unique_site_type_and_mapped_site_id ON db_sites(site_type, mapped_site_id); diff --git a/wp_mobile_cache/migrations/0002-create-posts-table.sql b/wp_mobile_cache/migrations/0002-create-posts-table.sql index 31c1cc53d..3d916eddb 100644 --- a/wp_mobile_cache/migrations/0002-create-posts-table.sql +++ b/wp_mobile_cache/migrations/0002-create-posts-table.sql @@ -2,8 +2,8 @@ CREATE TABLE `posts_edit_context` ( -- Internal DB field (auto-incrementing) `rowid` INTEGER PRIMARY KEY AUTOINCREMENT, - -- Site identifier (foreign key to sites table) - `db_site_id` INTEGER NOT NULL REFERENCES sites(id) ON DELETE CASCADE, + -- Site identifier (foreign key to db_sites table) + `db_site_id` INTEGER NOT NULL REFERENCES db_sites(id) ON DELETE CASCADE, -- Top-level non-nullable fields `id` INTEGER NOT NULL, @@ -57,7 +57,7 @@ CREATE TABLE `posts_edit_context` ( -- Client-side cache metadata: when this post was last fetched from the WordPress API `last_fetched_at` TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')), - FOREIGN KEY (db_site_id) REFERENCES sites(id) ON DELETE CASCADE + FOREIGN KEY (db_site_id) REFERENCES db_sites(id) ON DELETE CASCADE ) STRICT; CREATE UNIQUE INDEX idx_posts_edit_context_unique_db_site_id_and_id ON posts_edit_context(db_site_id, id); diff --git a/wp_mobile_cache/migrations/0003-create-term-relationships.sql b/wp_mobile_cache/migrations/0003-create-term-relationships.sql index 9c1c3f5a1..6eedb24c6 100644 --- a/wp_mobile_cache/migrations/0003-create-term-relationships.sql +++ b/wp_mobile_cache/migrations/0003-create-term-relationships.sql @@ -2,7 +2,7 @@ CREATE TABLE `term_relationships` ( -- Internal DB field (auto-incrementing) `rowid` INTEGER PRIMARY KEY AUTOINCREMENT, - -- Site identifier (foreign key to sites table) + -- Site identifier (foreign key to db_sites table) `db_site_id` INTEGER NOT NULL, -- Object identifier (rowid of post/page/nav_menu_item/etc) @@ -15,7 +15,7 @@ CREATE TABLE `term_relationships` ( -- Taxonomy type ('category', 'post_tag', or custom taxonomy) `taxonomy_type` TEXT NOT NULL, - FOREIGN KEY (db_site_id) REFERENCES sites(id) ON DELETE CASCADE + FOREIGN KEY (db_site_id) REFERENCES db_sites(id) ON DELETE CASCADE ) STRICT; -- Prevent duplicate associations (same object can't have same term twice in same taxonomy) diff --git a/wp_mobile_cache/migrations/0004-create-posts-view-context-table.sql b/wp_mobile_cache/migrations/0004-create-posts-view-context-table.sql index 3d47612e8..ce544b98b 100644 --- a/wp_mobile_cache/migrations/0004-create-posts-view-context-table.sql +++ b/wp_mobile_cache/migrations/0004-create-posts-view-context-table.sql @@ -2,8 +2,8 @@ CREATE TABLE `posts_view_context` ( -- Internal DB field (auto-incrementing) `rowid` INTEGER PRIMARY KEY AUTOINCREMENT, - -- Site identifier (foreign key to sites table) - `db_site_id` INTEGER NOT NULL REFERENCES sites(id) ON DELETE CASCADE, + -- Site identifier (foreign key to db_sites table) + `db_site_id` INTEGER NOT NULL REFERENCES db_sites(id) ON DELETE CASCADE, -- Top-level non-nullable fields `id` INTEGER NOT NULL, @@ -50,7 +50,7 @@ CREATE TABLE `posts_view_context` ( -- Client-side cache metadata: when this post was last fetched from the WordPress API `last_fetched_at` TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')), - FOREIGN KEY (db_site_id) REFERENCES sites(id) ON DELETE CASCADE + FOREIGN KEY (db_site_id) REFERENCES db_sites(id) ON DELETE CASCADE ) STRICT; CREATE UNIQUE INDEX idx_posts_view_context_unique_db_site_id_and_id ON posts_view_context(db_site_id, id); diff --git a/wp_mobile_cache/migrations/0005-create-posts-embed-context-table.sql b/wp_mobile_cache/migrations/0005-create-posts-embed-context-table.sql index e2c67b78e..5434b1729 100644 --- a/wp_mobile_cache/migrations/0005-create-posts-embed-context-table.sql +++ b/wp_mobile_cache/migrations/0005-create-posts-embed-context-table.sql @@ -2,8 +2,8 @@ CREATE TABLE `posts_embed_context` ( -- Internal DB field (auto-incrementing) `rowid` INTEGER PRIMARY KEY AUTOINCREMENT, - -- Site identifier (foreign key to sites table) - `db_site_id` INTEGER NOT NULL REFERENCES sites(id) ON DELETE CASCADE, + -- Site identifier (foreign key to db_sites table) + `db_site_id` INTEGER NOT NULL REFERENCES db_sites(id) ON DELETE CASCADE, -- Top-level non-nullable fields (minimal set for embed context) `id` INTEGER NOT NULL, @@ -29,7 +29,7 @@ CREATE TABLE `posts_embed_context` ( -- Client-side cache metadata: when this post was last fetched from the WordPress API `last_fetched_at` TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')), - FOREIGN KEY (db_site_id) REFERENCES sites(id) ON DELETE CASCADE + FOREIGN KEY (db_site_id) REFERENCES db_sites(id) ON DELETE CASCADE ) STRICT; CREATE UNIQUE INDEX idx_posts_embed_context_unique_db_site_id_and_id ON posts_embed_context(db_site_id, id); diff --git a/wp_mobile_cache/migrations/0006-create-self-hosted-sites-table.sql b/wp_mobile_cache/migrations/0006-create-self-hosted-sites-table.sql new file mode 100644 index 000000000..c57e605d0 --- /dev/null +++ b/wp_mobile_cache/migrations/0006-create-self-hosted-sites-table.sql @@ -0,0 +1,12 @@ +CREATE TABLE `self_hosted_sites` ( + -- Internal DB field (auto-incrementing) + `rowid` INTEGER PRIMARY KEY AUTOINCREMENT, + + -- Site URL (unique constraint for upsert logic) + `url` TEXT NOT NULL UNIQUE, + + -- WordPress REST API root URL + `api_root` TEXT NOT NULL +) STRICT; + +CREATE INDEX idx_self_hosted_sites_url ON self_hosted_sites(url); diff --git a/wp_mobile_cache/src/db_types.rs b/wp_mobile_cache/src/db_types.rs index 83d81e78b..0a3874799 100644 --- a/wp_mobile_cache/src/db_types.rs +++ b/wp_mobile_cache/src/db_types.rs @@ -1 +1,6 @@ +pub mod db_site; +pub mod db_term_relationship; +pub mod helpers; pub mod posts; +pub mod row_ext; +pub mod self_hosted_site; diff --git a/wp_mobile_cache/src/db_types/db_site.rs b/wp_mobile_cache/src/db_types/db_site.rs new file mode 100644 index 000000000..3eb5e4857 --- /dev/null +++ b/wp_mobile_cache/src/db_types/db_site.rs @@ -0,0 +1,54 @@ +use crate::RowId; +use rusqlite::types::{FromSql, FromSqlResult, ToSql, ToSqlOutput}; + +/// Type of WordPress site stored in the database. +/// +/// Uses integer representation in the database for performance. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, serde::Serialize, serde::Deserialize)] +#[repr(i64)] +pub enum DbSiteType { + SelfHosted = 0, + WordPressCom = 1, +} + +impl ToSql for DbSiteType { + fn to_sql(&self) -> rusqlite::Result> { + Ok(ToSqlOutput::from(*self as i64)) + } +} + +impl FromSql for DbSiteType { + fn column_result(value: rusqlite::types::ValueRef<'_>) -> FromSqlResult { + match i64::column_result(value)? { + 0 => Ok(DbSiteType::SelfHosted), + 1 => Ok(DbSiteType::WordPressCom), + other => Err(rusqlite::types::FromSqlError::OutOfRange(other)), + } + } +} + +/// Represents a cached WordPress site in the database. +/// +/// # Design Rationale +/// +/// This is intentionally a database-specific type (hence the `Db` prefix) rather than +/// a domain type representing a WordPress site. This design choice prevents confusion: +/// +/// - **Not a WordPress.com site ID**: The `row_id` has no relationship to WordPress.com site IDs +/// - **Not a self-hosted site identifier**: Self-hosted sites don't have numeric IDs +/// - **Internal cache identifier only**: This ID exists only for our local database's multi-site support +/// +/// # Site Type Mapping +/// +/// The `site_type` field indicates which type-specific table contains additional data: +/// - `DbSiteType::SelfHosted` → `mapped_site_id` references `self_hosted_sites` table +/// - `DbSiteType::WordPressCom` → `mapped_site_id` references `wordpress_com_sites` table (future) +/// +/// Note: `mapped_site_id` is a reference column, not a foreign key constraint, since it can +/// point to different tables based on `site_type`. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, serde::Serialize, serde::Deserialize)] +pub struct DbSite { + pub row_id: RowId, + pub site_type: DbSiteType, + pub mapped_site_id: RowId, +} diff --git a/wp_mobile_cache/src/mappings/term_relationships.rs b/wp_mobile_cache/src/db_types/db_term_relationship.rs similarity index 85% rename from wp_mobile_cache/src/mappings/term_relationships.rs rename to wp_mobile_cache/src/db_types/db_term_relationship.rs index 22b390ea2..c5a3275b0 100644 --- a/wp_mobile_cache/src/mappings/term_relationships.rs +++ b/wp_mobile_cache/src/db_types/db_term_relationship.rs @@ -1,6 +1,6 @@ use crate::{ - DbSite, SqliteDbError, - mappings::{ColumnIndex, RowExt}, + SqliteDbError, + db_types::row_ext::{ColumnIndex, RowExt}, term_relationships::DbTermRelationship, }; use rusqlite::Row; @@ -31,9 +31,7 @@ impl DbTermRelationship { Ok(Self { row_id: row.get_column(Col::Rowid)?, - site: DbSite { - row_id: row.get_column(Col::DbSiteId)?, - }, + db_site_id: row.get_column(Col::DbSiteId)?, object_id: row.get_column(Col::ObjectId)?, term_id: TermId(row.get_column(Col::TermId)?), taxonomy_type: row.get_column::(Col::TaxonomyType)?.into(), diff --git a/wp_mobile_cache/src/mappings/helpers.rs b/wp_mobile_cache/src/db_types/helpers.rs similarity index 99% rename from wp_mobile_cache/src/mappings/helpers.rs rename to wp_mobile_cache/src/db_types/helpers.rs index 690d2ea27..6d2beb3d5 100644 --- a/wp_mobile_cache/src/mappings/helpers.rs +++ b/wp_mobile_cache/src/db_types/helpers.rs @@ -1,6 +1,6 @@ use crate::{ SqliteDbError, - mappings::{ColumnIndex, RowExt}, + db_types::row_ext::{ColumnIndex, RowExt}, }; use rusqlite::Row; use serde::{Deserialize, Serialize}; diff --git a/wp_mobile_cache/src/db_types/posts/edit.rs b/wp_mobile_cache/src/db_types/posts/edit.rs index 21adf7adc..41fa5e197 100644 --- a/wp_mobile_cache/src/db_types/posts/edit.rs +++ b/wp_mobile_cache/src/db_types/posts/edit.rs @@ -1,4 +1,4 @@ -use crate::{DbSite, RowId, mappings::ColumnIndex}; +use crate::{RowId, db_types::row_ext::ColumnIndex}; use wp_api::posts::AnyPostWithEditContext; /// Column indexes for posts_edit_context table. @@ -7,7 +7,7 @@ use wp_api::posts::AnyPostWithEditContext; #[derive(Debug, Clone, Copy)] pub(crate) enum PostEditContextColumn { Rowid = 0, - SiteId = 1, + DbSiteId = 1, Id = 2, Date = 3, DateGmt = 4, @@ -52,7 +52,7 @@ impl ColumnIndex for PostEditContextColumn { pub struct DbAnyPostWithEditContext { pub row_id: RowId, - pub site: DbSite, + pub db_site_id: RowId, pub post: AnyPostWithEditContext, pub last_fetched_at: String, } diff --git a/wp_mobile_cache/src/db_types/posts/embed.rs b/wp_mobile_cache/src/db_types/posts/embed.rs index 9f12d2e11..c10cc9f61 100644 --- a/wp_mobile_cache/src/db_types/posts/embed.rs +++ b/wp_mobile_cache/src/db_types/posts/embed.rs @@ -1,4 +1,4 @@ -use crate::{DbSite, RowId, mappings::ColumnIndex}; +use crate::{RowId, db_types::row_ext::ColumnIndex}; use wp_api::posts::AnyPostWithEmbedContext; /// Column indexes for posts_embed_context table. @@ -7,7 +7,7 @@ use wp_api::posts::AnyPostWithEmbedContext; #[derive(Debug, Clone, Copy)] pub(crate) enum PostEmbedContextColumn { Rowid = 0, - SiteId = 1, + DbSiteId = 1, Id = 2, Date = 3, Link = 4, @@ -30,7 +30,7 @@ impl ColumnIndex for PostEmbedContextColumn { pub struct DbAnyPostWithEmbedContext { pub row_id: RowId, - pub site: DbSite, + pub db_site_id: RowId, pub post: AnyPostWithEmbedContext, pub last_fetched_at: String, } diff --git a/wp_mobile_cache/src/db_types/posts/view.rs b/wp_mobile_cache/src/db_types/posts/view.rs index 441650e63..024ee353f 100644 --- a/wp_mobile_cache/src/db_types/posts/view.rs +++ b/wp_mobile_cache/src/db_types/posts/view.rs @@ -1,4 +1,4 @@ -use crate::{DbSite, RowId, mappings::ColumnIndex}; +use crate::{RowId, db_types::row_ext::ColumnIndex}; use wp_api::posts::AnyPostWithViewContext; /// Column indexes for posts_view_context table. @@ -7,7 +7,7 @@ use wp_api::posts::AnyPostWithViewContext; #[derive(Debug, Clone, Copy)] pub(crate) enum PostViewContextColumn { Rowid = 0, - SiteId = 1, + DbSiteId = 1, Id = 2, Date = 3, DateGmt = 4, @@ -45,7 +45,7 @@ impl ColumnIndex for PostViewContextColumn { pub struct DbAnyPostWithViewContext { pub row_id: RowId, - pub site: DbSite, + pub db_site_id: RowId, pub post: AnyPostWithViewContext, pub last_fetched_at: String, } diff --git a/wp_mobile_cache/src/mappings.rs b/wp_mobile_cache/src/db_types/row_ext.rs similarity index 93% rename from wp_mobile_cache/src/mappings.rs rename to wp_mobile_cache/src/db_types/row_ext.rs index eb98ff0d2..477354b10 100644 --- a/wp_mobile_cache/src/mappings.rs +++ b/wp_mobile_cache/src/db_types/row_ext.rs @@ -1,8 +1,5 @@ use rusqlite::Row; -pub mod helpers; -pub mod term_relationships; - /// Trait for types that can be used as column indexes. /// Implemented by column enum types to provide type-safe column access. pub trait ColumnIndex { diff --git a/wp_mobile_cache/src/db_types/self_hosted_site.rs b/wp_mobile_cache/src/db_types/self_hosted_site.rs new file mode 100644 index 000000000..88c8c5559 --- /dev/null +++ b/wp_mobile_cache/src/db_types/self_hosted_site.rs @@ -0,0 +1,37 @@ +use crate::{RowId, db_types::row_ext::ColumnIndex}; + +/// Column indexes for self_hosted_sites table. +/// These must match the order of columns in the CREATE TABLE statement. +#[repr(usize)] +#[derive(Debug, Clone, Copy)] +pub(crate) enum DbSelfHostedSiteColumn { + Rowid = 0, + Url = 1, + ApiRoot = 2, +} + +impl ColumnIndex for DbSelfHostedSiteColumn { + fn as_index(&self) -> usize { + *self as usize + } +} + +/// Represents a self-hosted WordPress site (domain model). +/// +/// This type contains the site data without database metadata. +/// Use this when creating or updating sites. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct SelfHostedSite { + pub url: String, + pub api_root: String, +} + +/// Represents a self-hosted WordPress site in the database. +/// +/// This type includes the database rowid along with the site data. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct DbSelfHostedSite { + pub row_id: RowId, + pub url: String, + pub api_root: String, +} diff --git a/wp_mobile_cache/src/lib.rs b/wp_mobile_cache/src/lib.rs index 88934056f..f43ebbd84 100644 --- a/wp_mobile_cache/src/lib.rs +++ b/wp_mobile_cache/src/lib.rs @@ -5,7 +5,6 @@ use std::sync::{Arc, Mutex}; pub mod context; pub mod db_types; -pub mod mappings; pub mod repository; pub mod term_relationships; @@ -85,34 +84,6 @@ impl From for i64 { } } -/// Represents a cached WordPress site in the database. -/// -/// # Design Rationale -/// -/// This is intentionally a database-specific type (hence the `Db` prefix) rather than -/// a domain type representing a WordPress site. This design choice prevents confusion: -/// -/// - **Not a WordPress.com site ID**: The `row_id` has no relationship to WordPress.com site IDs -/// - **Not a self-hosted site identifier**: Self-hosted sites don't have numeric IDs -/// - **Internal cache identifier only**: This ID exists only for our local database's multi-site support -/// -/// # Future Extension -/// -/// When site type tables are added (e.g., `self_hosted_sites`, `wordpress_com_sites`), this -/// struct will gain additional fields: -/// -/// ```ignore -/// pub struct DbSite { -/// pub row_id: RowId, -/// pub site_type: SiteType, // SelfHosted | WordPressCom -/// pub mapped_site_id: RowId, // Foreign key to specific site type table -/// } -/// ``` -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, serde::Serialize, serde::Deserialize)] -pub struct DbSite { - pub row_id: RowId, -} - /// Get the SQLite version string from the database. /// /// This function queries the database for its SQLite version using `SELECT sqlite_version()`. @@ -219,12 +190,13 @@ impl WpApiCache { } } -static MIGRATION_QUERIES: [&str; 5] = [ +static MIGRATION_QUERIES: [&str; 6] = [ include_str!("../migrations/0001-create-sites-table.sql"), include_str!("../migrations/0002-create-posts-table.sql"), include_str!("../migrations/0003-create-term-relationships.sql"), include_str!("../migrations/0004-create-posts-view-context-table.sql"), include_str!("../migrations/0005-create-posts-embed-context-table.sql"), + include_str!("../migrations/0006-create-self-hosted-sites-table.sql"), ]; pub struct MigrationManager<'a> { diff --git a/wp_mobile_cache/src/repository/mod.rs b/wp_mobile_cache/src/repository/mod.rs index 791a10a3b..b2d4f9122 100644 --- a/wp_mobile_cache/src/repository/mod.rs +++ b/wp_mobile_cache/src/repository/mod.rs @@ -2,6 +2,7 @@ use crate::{RowId, SqliteDbError}; use rusqlite::Connection; pub mod posts; +pub mod sites; pub mod term_relationships; #[cfg(test)] diff --git a/wp_mobile_cache/src/repository/posts.rs b/wp_mobile_cache/src/repository/posts.rs index b7c714404..c9d4cc95c 100644 --- a/wp_mobile_cache/src/repository/posts.rs +++ b/wp_mobile_cache/src/repository/posts.rs @@ -1,16 +1,17 @@ use crate::{ - DbSite, RowId, SqliteDbError, + RowId, SqliteDbError, context::{EditContext, EmbedContext, IsContext, ViewContext}, - db_types::posts::{ - DbAnyPostWithEditContext, DbAnyPostWithEmbedContext, DbAnyPostWithViewContext, - PostEditContextColumn, PostEmbedContextColumn, PostViewContextColumn, - }, - mappings::{ - RowExt, + db_types::{ + db_site::DbSite, helpers::{ bool_to_integer, deserialize_json_value, get_id, get_optional_id, integer_to_bool, parse_datetime, parse_enum, parse_optional_enum, serialize_value_to_json, }, + posts::{ + DbAnyPostWithEditContext, DbAnyPostWithEmbedContext, DbAnyPostWithViewContext, + PostEditContextColumn, PostEmbedContextColumn, PostViewContextColumn, + }, + row_ext::RowExt, }, repository::{ QueryExecutor, TransactionManager, term_relationships::TermRelationshipRepository, @@ -271,9 +272,7 @@ impl PostContext for EditContext { use PostEditContextColumn::*; let row_id: RowId = row.get_column(Rowid)?; - let site = DbSite { - row_id: row.get_column(PostEditContextColumn::SiteId)?, - }; + let db_site_id: RowId = row.get_column(PostEditContextColumn::DbSiteId)?; // EditContext uses term relationships (categories and tags) let term_relationships = fetch_terms()?; @@ -338,7 +337,7 @@ impl PostContext for EditContext { Ok(DbAnyPostWithEditContext { row_id, - site, + db_site_id, post, last_fetched_at: row.get_column(LastFetchedAt)?, }) @@ -356,9 +355,7 @@ impl PostContext for ViewContext { use PostViewContextColumn::*; let row_id: RowId = row.get_column(Rowid)?; - let site = DbSite { - row_id: row.get_column(PostViewContextColumn::SiteId)?, - }; + let db_site_id: RowId = row.get_column(PostViewContextColumn::DbSiteId)?; // ViewContext uses term relationships (categories and tags) let term_relationships = fetch_terms()?; @@ -416,7 +413,7 @@ impl PostContext for ViewContext { Ok(DbAnyPostWithViewContext { row_id, - site, + db_site_id, post, last_fetched_at: row.get_column(LastFetchedAt)?, }) @@ -434,9 +431,7 @@ impl PostContext for EmbedContext { use PostEmbedContextColumn::*; let row_id: RowId = row.get_column(Rowid)?; - let site = DbSite { - row_id: row.get_column(PostEmbedContextColumn::SiteId)?, - }; + let db_site_id: RowId = row.get_column(PostEmbedContextColumn::DbSiteId)?; // EmbedContext does not use term relationships (no categories/tags in embed context) // The fetch_terms closure is never called, avoiding unnecessary database queries @@ -468,7 +463,7 @@ impl PostContext for EmbedContext { Ok(DbAnyPostWithEmbedContext { row_id, - site, + db_site_id, post, last_fetched_at: row.get_column(LastFetchedAt)?, }) @@ -847,7 +842,7 @@ mod tests { use crate::db_types::posts::{ PostEditContextColumn, PostEmbedContextColumn, PostViewContextColumn, }; - use crate::mappings::ColumnIndex; + use crate::db_types::row_ext::ColumnIndex; use crate::test_fixtures::{ TestContext, assert_recent_timestamp, get_table_column_names, posts::PostBuilder, test_ctx, }; @@ -864,7 +859,7 @@ mod tests { // Verify each enum value maps to the correct column name assert_eq!(columns[Rowid.as_index()], "rowid"); - assert_eq!(columns[SiteId.as_index()], "db_site_id"); + assert_eq!(columns[DbSiteId.as_index()], "db_site_id"); assert_eq!(columns[Id.as_index()], "id"); assert_eq!(columns[Date.as_index()], "date"); assert_eq!(columns[DateGmt.as_index()], "date_gmt"); @@ -916,7 +911,7 @@ mod tests { let columns = get_table_column_names(&test_ctx.conn, "posts_view_context"); assert_eq!(columns[Rowid.as_index()], "rowid"); - assert_eq!(columns[SiteId.as_index()], "db_site_id"); + assert_eq!(columns[DbSiteId.as_index()], "db_site_id"); assert_eq!(columns[Id.as_index()], "id"); assert_eq!(columns[Date.as_index()], "date"); assert_eq!(columns[DateGmt.as_index()], "date_gmt"); @@ -957,7 +952,7 @@ mod tests { let columns = get_table_column_names(&test_ctx.conn, "posts_embed_context"); assert_eq!(columns[Rowid.as_index()], "rowid"); - assert_eq!(columns[SiteId.as_index()], "db_site_id"); + assert_eq!(columns[DbSiteId.as_index()], "db_site_id"); assert_eq!(columns[Id.as_index()], "id"); assert_eq!(columns[Date.as_index()], "date"); assert_eq!(columns[Link.as_index()], "link"); @@ -993,7 +988,7 @@ mod tests { // Verify round-trip assert_eq!(retrieved.row_id, rowid); - assert_eq!(retrieved.site, test_ctx.site); + assert_eq!(retrieved.db_site_id, test_ctx.site.row_id); assert_recent_timestamp(&retrieved.last_fetched_at); assert_eq!(retrieved.post, original_post); } @@ -1065,7 +1060,7 @@ mod tests { .expect("Post should exist"); assert_eq!(retrieved.row_id, rowid); - assert_eq!(retrieved.site, test_ctx.site); + assert_eq!(retrieved.db_site_id, test_ctx.site.row_id); assert_eq!(retrieved.post, post); } @@ -1087,7 +1082,7 @@ mod tests { .expect("Post should exist"); assert_eq!(retrieved.post.id, PostId(42)); - assert_eq!(retrieved.site, test_ctx.site); + assert_eq!(retrieved.db_site_id, test_ctx.site.row_id); assert_eq!(retrieved.post, post); } @@ -1275,7 +1270,7 @@ mod tests { .expect("Failed to select post by post_id") .expect("Post should exist after insert"); assert_eq!(retrieved.row_id, rowid); - assert_eq!(retrieved.site, test_ctx.site); + assert_eq!(retrieved.db_site_id, test_ctx.site.row_id); assert_eq!(retrieved.post.status, PostStatus::Draft); } diff --git a/wp_mobile_cache/src/repository/posts_constraint_tests.rs b/wp_mobile_cache/src/repository/posts_constraint_tests.rs index 5dfbab78b..6bb1ceccc 100644 --- a/wp_mobile_cache/src/repository/posts_constraint_tests.rs +++ b/wp_mobile_cache/src/repository/posts_constraint_tests.rs @@ -4,7 +4,8 @@ //! and that error cases are handled appropriately. use crate::{ - DbSite, RowId, + RowId, + db_types::db_site::{DbSite, DbSiteType}, test_fixtures::{TestContext, posts::PostBuilder, test_ctx}, }; use rstest::*; @@ -57,7 +58,12 @@ fn test_duplicate_post_id_in_same_site_updates_on_upsert(mut test_ctx: TestConte #[rstest] fn test_invalid_site_id_fails_foreign_key_constraint(mut test_ctx: TestContext) { - let non_existent_site = DbSite { row_id: RowId(999) }; // Site doesn't exist + // Site doesn't exist in database - intentionally invalid for testing error handling + let non_existent_site = DbSite { + row_id: RowId(999), + site_type: DbSiteType::SelfHosted, + mapped_site_id: RowId(999), + }; let post = PostBuilder::minimal().build(); let result = test_ctx diff --git a/wp_mobile_cache/src/repository/posts_multi_site_tests.rs b/wp_mobile_cache/src/repository/posts_multi_site_tests.rs index 7526e16aa..468934fb8 100644 --- a/wp_mobile_cache/src/repository/posts_multi_site_tests.rs +++ b/wp_mobile_cache/src/repository/posts_multi_site_tests.rs @@ -1,12 +1,12 @@ //! Multi-site isolation tests for PostRepository. -use crate::test_fixtures::{TestContext, create_test_site, posts::PostBuilder, test_ctx}; +use crate::test_fixtures::{TestContext, create_random_test_site, posts::PostBuilder, test_ctx}; use rstest::*; use wp_api::posts::PostId; #[rstest] fn test_posts_in_site_1_invisible_to_site_2(mut test_ctx: TestContext) { - let site2 = create_test_site(&test_ctx.conn, 2); + let site2 = create_random_test_site(&mut test_ctx.conn); // Insert post in site 1 let post = PostBuilder::minimal().with_id(100).build(); @@ -28,7 +28,7 @@ fn test_posts_in_site_1_invisible_to_site_2(mut test_ctx: TestContext) { #[rstest] fn test_same_post_id_can_exist_in_different_sites(mut test_ctx: TestContext) { - let site2 = create_test_site(&test_ctx.conn, 2); + let site2 = create_random_test_site(&mut test_ctx.conn); // Create post with same ID in both sites let post_id = PostId(42); @@ -69,7 +69,7 @@ fn test_same_post_id_can_exist_in_different_sites(mut test_ctx: TestContext) { #[rstest] fn test_select_all_only_returns_posts_for_requested_site(mut test_ctx: TestContext) { - let site2 = create_test_site(&test_ctx.conn, 2); + let site2 = create_random_test_site(&mut test_ctx.conn); // Insert posts in site 1 test_ctx @@ -119,7 +119,7 @@ fn test_select_all_only_returns_posts_for_requested_site(mut test_ctx: TestConte #[rstest] fn test_count_only_counts_posts_for_requested_site(mut test_ctx: TestContext) { - let site2 = create_test_site(&test_ctx.conn, 2); + let site2 = create_random_test_site(&mut test_ctx.conn); // Insert posts in both sites test_ctx @@ -156,7 +156,7 @@ fn test_count_only_counts_posts_for_requested_site(mut test_ctx: TestContext) { #[rstest] fn test_delete_by_post_id_only_deletes_from_specified_site(mut test_ctx: TestContext) { - let site2 = create_test_site(&test_ctx.conn, 2); + let site2 = create_random_test_site(&mut test_ctx.conn); let post_id = PostId(999); diff --git a/wp_mobile_cache/src/repository/posts_transaction_tests.rs b/wp_mobile_cache/src/repository/posts_transaction_tests.rs index 737fd3009..3cf3fe260 100644 --- a/wp_mobile_cache/src/repository/posts_transaction_tests.rs +++ b/wp_mobile_cache/src/repository/posts_transaction_tests.rs @@ -4,7 +4,8 @@ //! ensuring proper rollback on errors without leaving partial writes or corrupted data. use crate::{ - DbSite, RowId, + RowId, + db_types::db_site::{DbSite, DbSiteType}, test_fixtures::{TestContext, posts::PostBuilder, test_ctx}, }; use rstest::*; @@ -75,7 +76,12 @@ fn test_upsert_batch_handles_duplicate_ids_by_updating(mut test_ctx: TestContext #[rstest] fn test_upsert_batch_fails_on_foreign_key_violation(mut test_ctx: TestContext) { - let invalid_site = DbSite { row_id: RowId(999) }; + // Site doesn't exist in database - intentionally invalid for testing error handling + let invalid_site = DbSite { + row_id: RowId(999), + site_type: DbSiteType::SelfHosted, + mapped_site_id: RowId(999), + }; let post1 = PostBuilder::minimal().build(); let post2 = PostBuilder::minimal().build(); diff --git a/wp_mobile_cache/src/repository/sites.rs b/wp_mobile_cache/src/repository/sites.rs new file mode 100644 index 000000000..2904202e7 --- /dev/null +++ b/wp_mobile_cache/src/repository/sites.rs @@ -0,0 +1,641 @@ +use crate::{ + RowId, SqliteDbError, + db_types::{ + db_site::{DbSite, DbSiteType}, + row_ext::RowExt, + self_hosted_site::{DbSelfHostedSite, DbSelfHostedSiteColumn, SelfHostedSite}, + }, + repository::{QueryExecutor, TransactionManager}, +}; +use rusqlite::OptionalExtension; + +pub struct SiteRepository; + +impl SiteRepository { + const SELF_HOSTED_SITES_TABLE: &'static str = "self_hosted_sites"; + const DB_SITES_TABLE: &'static str = "db_sites"; + + /// Upsert a self-hosted site and return both the DbSite and DbSelfHostedSite (atomic transaction). + /// + /// If a site with the given URL already exists, updates it. Otherwise creates a new one. + /// Uses SQLite's RETURNING clause to get the inserted/updated rowid. + pub fn upsert_self_hosted_site( + &self, + transaction_manager: &mut impl TransactionManager, + site: &SelfHostedSite, + ) -> Result<(DbSite, DbSelfHostedSite), SqliteDbError> { + let tx = transaction_manager.transaction()?; + + // Upsert into self_hosted_sites and get the rowid + let self_hosted_site_id: RowId = { + let sql = format!( + "INSERT INTO {} (url, api_root) VALUES (?, ?) + ON CONFLICT(url) DO UPDATE SET api_root = excluded.api_root + RETURNING rowid", + Self::SELF_HOSTED_SITES_TABLE + ); + + let mut stmt = tx.prepare(&sql)?; + stmt.query_row((&site.url, &site.api_root), |row| row.get(0)) + .map_err(SqliteDbError::from)? + }; + + // Upsert into sites table + // If site_type + mapped_site_id already exists, reuse that entry + // Note: DO UPDATE SET is required for RETURNING to work on conflict (DO NOTHING returns no rows) + let site_id: RowId = { + let sql = format!( + "INSERT INTO {} (site_type, mapped_site_id) VALUES (?, ?) + ON CONFLICT(site_type, mapped_site_id) DO UPDATE SET + site_type = excluded.site_type + RETURNING rowid", + Self::DB_SITES_TABLE + ); + + let mut stmt = tx.prepare(&sql)?; + stmt.query_row((DbSiteType::SelfHosted, self_hosted_site_id), |row| { + row.get(0) + }) + .map_err(SqliteDbError::from)? + }; + + let db_site = DbSite { + row_id: site_id, + site_type: DbSiteType::SelfHosted, + mapped_site_id: self_hosted_site_id, + }; + + let db_self_hosted_site = DbSelfHostedSite { + row_id: self_hosted_site_id, + url: site.url.clone(), + api_root: site.api_root.clone(), + }; + + tx.commit().map_err(SqliteDbError::from)?; + Ok((db_site, db_self_hosted_site)) + } + + /// Select a self-hosted site by its DbSite reference. + /// + /// Returns None if the site doesn't exist or isn't a self-hosted site. + pub fn select_self_hosted_site( + &self, + executor: &impl QueryExecutor, + site: &DbSite, + ) -> Result, SqliteDbError> { + if site.site_type != DbSiteType::SelfHosted { + return Ok(None); + } + + let sql = format!( + "SELECT * FROM {} WHERE rowid = ?", + Self::SELF_HOSTED_SITES_TABLE + ); + let mut stmt = executor.prepare(&sql)?; + + stmt.query_row([site.mapped_site_id], |row| { + Ok(DbSelfHostedSite { + row_id: row.get_column(DbSelfHostedSiteColumn::Rowid)?, + url: row.get_column(DbSelfHostedSiteColumn::Url)?, + api_root: row.get_column(DbSelfHostedSiteColumn::ApiRoot)?, + }) + }) + .optional() + .map_err(SqliteDbError::from) + } + + /// Select a self-hosted site by URL. + /// + /// Returns both the DbSite and DbSelfHostedSite if found. + pub fn select_self_hosted_site_by_url( + &self, + executor: &impl QueryExecutor, + url: &str, + ) -> Result, SqliteDbError> { + // First get the self_hosted_site + let sql = format!( + "SELECT * FROM {} WHERE url = ?", + Self::SELF_HOSTED_SITES_TABLE + ); + let mut stmt = executor.prepare(&sql)?; + + let self_hosted_site: Option = stmt + .query_row([url], |row| { + Ok(DbSelfHostedSite { + row_id: row.get_column(DbSelfHostedSiteColumn::Rowid)?, + url: row.get_column(DbSelfHostedSiteColumn::Url)?, + api_root: row.get_column(DbSelfHostedSiteColumn::ApiRoot)?, + }) + }) + .optional() + .map_err(SqliteDbError::from)?; + + let Some(self_hosted_site) = self_hosted_site else { + return Ok(None); + }; + + // Then find the corresponding site + let sql = format!( + "SELECT rowid, site_type, mapped_site_id FROM {} + WHERE site_type = ? AND mapped_site_id = ?", + Self::DB_SITES_TABLE + ); + let mut stmt = executor.prepare(&sql)?; + + let db_site: Option = stmt + .query_row((DbSiteType::SelfHosted, self_hosted_site.row_id), |row| { + Ok(DbSite { + row_id: row.get(0)?, + site_type: row.get(1)?, + mapped_site_id: row.get(2)?, + }) + }) + .optional() + .map_err(SqliteDbError::from)?; + + Ok(db_site.map(|site| (site, self_hosted_site))) + } + + /// Count all sites in the database. + /// + /// This is primarily useful for testing to verify database state. + pub fn count_all_db_sites( + &self, + executor: &impl QueryExecutor, + ) -> Result { + let sql = format!("SELECT COUNT(*) FROM {}", Self::DB_SITES_TABLE); + let mut stmt = executor.prepare(&sql)?; + let count: i64 = stmt.query_row([], |row| row.get(0))?; + Ok(count as usize) + } + + /// Count all self-hosted sites in the database. + /// + /// This is primarily useful for testing to verify database state. + pub fn count_all_self_hosted_sites( + &self, + executor: &impl QueryExecutor, + ) -> Result { + let sql = format!("SELECT COUNT(*) FROM {}", Self::SELF_HOSTED_SITES_TABLE); + let mut stmt = executor.prepare(&sql)?; + let count: i64 = stmt.query_row([], |row| row.get(0))?; + Ok(count as usize) + } + + /// Delete a site and its type-specific data (atomic transaction). + /// + /// Deletes both the site entry and its corresponding type-specific entry + /// (e.g., from self_hosted_sites or wordpress_com_sites). This ensures proper + /// cleanup since foreign key constraints cannot be used with polymorphic references. + /// + /// Returns `true` if a site was deleted, `false` if the site didn't exist. + pub fn delete_site( + &self, + transaction_manager: &mut impl TransactionManager, + site: &DbSite, + ) -> Result { + let tx = transaction_manager.transaction()?; + + // Delete from type-specific table based on site_type + match site.site_type { + DbSiteType::SelfHosted => { + let sql = format!( + "DELETE FROM {} WHERE rowid = ?", + Self::SELF_HOSTED_SITES_TABLE + ); + tx.execute(&sql, [site.mapped_site_id])?; + } + DbSiteType::WordPressCom => { + panic!("WordPress.com site deletion is not yet implemented") + } + }; + + // Delete from sites table + let sites_deleted = { + let sql = format!("DELETE FROM {} WHERE rowid = ?", Self::DB_SITES_TABLE); + tx.execute(&sql, [site.row_id])? + }; + + tx.commit().map_err(SqliteDbError::from)?; + Ok(sites_deleted > 0) + } + + /// Delete a self-hosted site by URL (convenience wrapper). + /// + /// Returns `true` if a site was deleted, `false` if no site with that URL exists. + pub fn delete_self_hosted_site_by_url( + &self, + transaction_manager: &mut impl TransactionManager, + url: &str, + ) -> Result { + let site_data = self.select_self_hosted_site_by_url(transaction_manager, url)?; + + match site_data { + Some((db_site, _)) => self.delete_site(transaction_manager, &db_site), + None => Ok(false), + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::test_fixtures::get_table_column_names; + use crate::{MigrationManager, db_types::row_ext::ColumnIndex}; + use rstest::*; + use rusqlite::Connection; + + #[fixture] + fn test_conn() -> Connection { + let conn = Connection::open_in_memory().expect("Failed to open in-memory database"); + let mut migration_manager = + MigrationManager::new(&conn).expect("Failed to create MigrationManager"); + migration_manager + .perform_migrations() + .expect("All migrations should succeed"); + conn + } + + /// Verify that DbSelfHostedSiteColumn enum values match the actual database schema. + /// This test protects against column reordering in migrations breaking the positional index mapping. + #[rstest] + fn test_self_hosted_site_column_enum_matches_schema(test_conn: Connection) { + use DbSelfHostedSiteColumn::*; + + let columns = get_table_column_names(&test_conn, "self_hosted_sites"); + + // Verify each enum value maps to the correct column name + assert_eq!(columns[Rowid.as_index()], "rowid"); + assert_eq!(columns[Url.as_index()], "url"); + assert_eq!(columns[ApiRoot.as_index()], "api_root"); + + // Verify total column count matches + assert_eq!(columns.len(), ApiRoot.as_index() + 1); + } + + #[rstest] + fn test_upsert_inserts_new_site(mut test_conn: Connection) { + let repo = SiteRepository; + let site = SelfHostedSite { + url: "https://example.com".to_string(), + api_root: "https://example.com/wp-json".to_string(), + }; + + let (db_site, db_self_hosted_site) = repo + .upsert_self_hosted_site(&mut test_conn, &site) + .expect("Failed to upsert site"); + + // Verify self_hosted_sites entry + assert_eq!(db_self_hosted_site.url, site.url); + assert_eq!(db_self_hosted_site.api_root, site.api_root); + + // Verify sites entry + assert_eq!(db_site.site_type, DbSiteType::SelfHosted); + assert_eq!(db_site.mapped_site_id, db_self_hosted_site.row_id); + } + + #[rstest] + fn test_upsert_updates_existing_site_url(mut test_conn: Connection) { + let repo = SiteRepository; + let url = "https://example.com"; + + // First insert + let site1 = SelfHostedSite { + url: url.to_string(), + api_root: "https://example.com/wp-json".to_string(), + }; + let (db_site1, db_self_hosted_site1) = repo + .upsert_self_hosted_site(&mut test_conn, &site1) + .expect("Failed to upsert site"); + + // Second upsert with same URL but different api_root + let site2 = SelfHostedSite { + url: url.to_string(), + api_root: "https://example.com/wordpress/wp-json".to_string(), + }; + let (db_site2, db_self_hosted_site2) = repo + .upsert_self_hosted_site(&mut test_conn, &site2) + .expect("Failed to upsert site"); + + // Verify it updated the same row (same rowid) - this proves update, not insert + assert_eq!(db_self_hosted_site1.row_id, db_self_hosted_site2.row_id); + assert_eq!(db_site1.row_id, db_site2.row_id); + + // Verify api_root was updated + assert_eq!(db_self_hosted_site2.api_root, site2.api_root); + } + + #[rstest] + fn test_upsert_does_not_create_duplicate_sites_entries(mut test_conn: Connection) { + let repo = SiteRepository; + + // Insert same site multiple times + let site = SelfHostedSite { + url: "https://example.com".to_string(), + api_root: "https://example.com/wp-json".to_string(), + }; + + let (db_site1, _) = repo + .upsert_self_hosted_site(&mut test_conn, &site) + .expect("First upsert failed"); + let (db_site2, _) = repo + .upsert_self_hosted_site(&mut test_conn, &site) + .expect("Second upsert failed"); + let (db_site3, _) = repo + .upsert_self_hosted_site(&mut test_conn, &site) + .expect("Third upsert failed"); + + // All should return the same DbSite + assert_eq!(db_site1.row_id, db_site2.row_id); + assert_eq!(db_site2.row_id, db_site3.row_id); + + // Verify only one entry exists in sites table (ensures bug fix works) + let count = repo + .count_all_db_sites(&test_conn) + .expect("Failed to count db_sites"); + assert_eq!( + count, 1, + "Multiple upserts should not create duplicate sites table entries" + ); + } + + #[rstest] + fn test_select_self_hosted_site_by_db_site(mut test_conn: Connection) { + let repo = SiteRepository; + let site = SelfHostedSite { + url: "https://example.com".to_string(), + api_root: "https://example.com/wp-json".to_string(), + }; + + let (db_site, original_db_self_hosted_site) = repo + .upsert_self_hosted_site(&mut test_conn, &site) + .expect("Failed to upsert site"); + + // Select using DbSite reference + let retrieved = repo + .select_self_hosted_site(&test_conn, &db_site) + .expect("Failed to select site") + .expect("Site should exist"); + + assert_eq!(retrieved, original_db_self_hosted_site); + } + + #[rstest] + fn test_select_self_hosted_site_returns_none_for_wrong_site_type(test_conn: Connection) { + let repo = SiteRepository; + + // Create a DbSite with WordPressCom type (not inserted, just for testing) + let non_self_hosted_site = DbSite { + row_id: RowId(999), + site_type: DbSiteType::WordPressCom, + mapped_site_id: RowId(999), + }; + + let result = repo + .select_self_hosted_site(&test_conn, &non_self_hosted_site) + .expect("Query should succeed"); + + assert_eq!( + result, None, + "Should return None for non-SelfHosted site type" + ); + } + + #[rstest] + fn test_select_self_hosted_site_returns_none_for_non_existent_site(test_conn: Connection) { + let repo = SiteRepository; + + let non_existent_site = DbSite { + row_id: RowId(999), + site_type: DbSiteType::SelfHosted, + mapped_site_id: RowId(999), + }; + + let result = repo + .select_self_hosted_site(&test_conn, &non_existent_site) + .expect("Query should succeed"); + + assert_eq!(result, None, "Should return None for non-existent site"); + } + + #[rstest] + fn test_select_self_hosted_site_by_url(mut test_conn: Connection) { + let repo = SiteRepository; + let site = SelfHostedSite { + url: "https://example.com".to_string(), + api_root: "https://example.com/wp-json".to_string(), + }; + + let (original_db_site, original_db_self_hosted_site) = repo + .upsert_self_hosted_site(&mut test_conn, &site) + .expect("Failed to upsert site"); + + // Select by URL + let (retrieved_db_site, retrieved_db_self_hosted_site) = repo + .select_self_hosted_site_by_url(&test_conn, &site.url) + .expect("Failed to select site") + .expect("Site should exist"); + + // Verify both structs match + assert_eq!(retrieved_db_site, original_db_site); + assert_eq!(retrieved_db_self_hosted_site, original_db_self_hosted_site); + } + + #[rstest] + fn test_select_self_hosted_site_by_url_returns_none_for_non_existent_url( + test_conn: Connection, + ) { + let repo = SiteRepository; + + let result = repo + .select_self_hosted_site_by_url(&test_conn, "https://non-existent.com") + .expect("Query should succeed"); + + assert_eq!(result, None, "Should return None for non-existent URL"); + } + + #[rstest] + fn test_multiple_different_sites_can_coexist(mut test_conn: Connection) { + let repo = SiteRepository; + + let site1 = SelfHostedSite { + url: "https://example1.com".to_string(), + api_root: "https://example1.com/wp-json".to_string(), + }; + let site2 = SelfHostedSite { + url: "https://example2.com".to_string(), + api_root: "https://example2.com/wp-json".to_string(), + }; + + let (db_site1, _) = repo + .upsert_self_hosted_site(&mut test_conn, &site1) + .expect("Failed to upsert site1"); + let (db_site2, _) = repo + .upsert_self_hosted_site(&mut test_conn, &site2) + .expect("Failed to upsert site2"); + + // Verify different sites get different IDs + assert_ne!(db_site1.row_id, db_site2.row_id); + + // Verify both can be retrieved by URL + let retrieved1 = repo + .select_self_hosted_site_by_url(&test_conn, &site1.url) + .expect("Failed to select site by URL"); + let retrieved2 = repo + .select_self_hosted_site_by_url(&test_conn, &site2.url) + .expect("Failed to select site by URL"); + + assert!(retrieved1.is_some()); + assert!(retrieved2.is_some()); + } + + #[rstest] + fn test_delete_site_removes_both_tables(mut test_conn: Connection) { + let repo = SiteRepository; + let site = SelfHostedSite { + url: "https://example.com".to_string(), + api_root: "https://example.com/wp-json".to_string(), + }; + + // Create site + let (db_site, _) = repo + .upsert_self_hosted_site(&mut test_conn, &site) + .expect("Failed to upsert site"); + + // Verify site exists in both tables + let count_sites = repo + .count_all_db_sites(&test_conn) + .expect("Failed to count db_sites"); + let count_self_hosted = repo + .count_all_self_hosted_sites(&test_conn) + .expect("Failed to count self_hosted_sites"); + assert_eq!(count_sites, 1); + assert_eq!(count_self_hosted, 1); + + // Delete site + let deleted = repo + .delete_site(&mut test_conn, &db_site) + .expect("Failed to delete site"); + assert!(deleted, "Should return true when site is deleted"); + + // Verify site is removed from both tables + let count_sites_after = repo + .count_all_db_sites(&test_conn) + .expect("Failed to count db_sites after delete"); + let count_self_hosted_after = repo + .count_all_self_hosted_sites(&test_conn) + .expect("Failed to count self_hosted_sites after delete"); + assert_eq!( + count_sites_after, 0, + "Site should be deleted from sites table" + ); + assert_eq!( + count_self_hosted_after, 0, + "Site should be deleted from self_hosted_sites table" + ); + } + + #[rstest] + fn test_delete_site_returns_false_for_non_existent_site(mut test_conn: Connection) { + let repo = SiteRepository; + + let non_existent_site = DbSite { + row_id: RowId(999), + site_type: DbSiteType::SelfHosted, + mapped_site_id: RowId(888), + }; + + let deleted = repo + .delete_site(&mut test_conn, &non_existent_site) + .expect("Failed to delete site"); + assert!(!deleted, "Should return false when site doesn't exist"); + } + + #[rstest] + fn test_delete_site_only_deletes_specified_site(mut test_conn: Connection) { + let repo = SiteRepository; + + // Create two sites + let site1 = SelfHostedSite { + url: "https://example1.com".to_string(), + api_root: "https://example1.com/wp-json".to_string(), + }; + let site2 = SelfHostedSite { + url: "https://example2.com".to_string(), + api_root: "https://example2.com/wp-json".to_string(), + }; + + let (db_site1, _) = repo + .upsert_self_hosted_site(&mut test_conn, &site1) + .expect("Failed to upsert site1"); + let (db_site2, _) = repo + .upsert_self_hosted_site(&mut test_conn, &site2) + .expect("Failed to upsert site2"); + + // Delete site1 + let deleted = repo + .delete_site(&mut test_conn, &db_site1) + .expect("Failed to delete site1"); + assert!(deleted); + + // Verify site1 is gone + let retrieved1 = repo + .select_self_hosted_site_by_url(&test_conn, &site1.url) + .expect("Failed to select site by URL"); + assert_eq!(retrieved1, None, "Site1 should be deleted"); + + // Verify site2 still exists + let retrieved2 = repo + .select_self_hosted_site_by_url(&test_conn, &site2.url) + .expect("Failed to select site by URL"); + assert!(retrieved2.is_some(), "Site2 should still exist"); + assert_eq!( + retrieved2.expect("Site2 should exist").0.row_id, + db_site2.row_id + ); + } + + #[rstest] + fn test_delete_self_hosted_site_by_url_deletes_site(mut test_conn: Connection) { + let repo = SiteRepository; + let url = "https://example.com"; + let site = SelfHostedSite { + url: url.to_string(), + api_root: "https://example.com/wp-json".to_string(), + }; + + // Create site + repo.upsert_self_hosted_site(&mut test_conn, &site) + .expect("Failed to upsert site"); + + // Verify site exists + let before_delete = repo + .select_self_hosted_site_by_url(&test_conn, url) + .expect("Failed to select site by URL"); + assert!(before_delete.is_some()); + + // Delete by URL + let deleted = repo + .delete_self_hosted_site_by_url(&mut test_conn, url) + .expect("Failed to delete site by URL"); + assert!(deleted, "Should return true when site is deleted"); + + // Verify site is gone + let after_delete = repo + .select_self_hosted_site_by_url(&test_conn, url) + .expect("Failed to select site by URL"); + assert_eq!(after_delete, None, "Site should be deleted"); + } + + #[rstest] + fn test_delete_self_hosted_site_by_url_returns_false_for_non_existent( + mut test_conn: Connection, + ) { + let repo = SiteRepository; + + let deleted = repo + .delete_self_hosted_site_by_url(&mut test_conn, "https://non-existent.com") + .expect("Failed to delete site by URL"); + assert!(!deleted, "Should return false when site doesn't exist"); + } +} diff --git a/wp_mobile_cache/src/repository/term_relationships.rs b/wp_mobile_cache/src/repository/term_relationships.rs index 0557f5851..c9fbb7acf 100644 --- a/wp_mobile_cache/src/repository/term_relationships.rs +++ b/wp_mobile_cache/src/repository/term_relationships.rs @@ -1,5 +1,6 @@ use crate::{ - DbSite, SqliteDbError, + SqliteDbError, + db_types::db_site::DbSite, repository::{InTransaction, QueryExecutor}, term_relationships::DbTermRelationship, }; diff --git a/wp_mobile_cache/src/repository/term_relationships_multi_site_tests.rs b/wp_mobile_cache/src/repository/term_relationships_multi_site_tests.rs index 73e52669f..1108a5f82 100644 --- a/wp_mobile_cache/src/repository/term_relationships_multi_site_tests.rs +++ b/wp_mobile_cache/src/repository/term_relationships_multi_site_tests.rs @@ -2,13 +2,13 @@ //! //! These tests verify that term relationships are correctly isolated between sites. -use crate::test_fixtures::{TestContext, create_test_site, posts::PostBuilder, test_ctx}; +use crate::test_fixtures::{TestContext, create_random_test_site, posts::PostBuilder, test_ctx}; use rstest::*; use wp_api::{taxonomies::TaxonomyType, terms::TermId}; #[rstest] fn test_term_relationships_isolated_by_site(mut test_ctx: TestContext) { - let site2 = create_test_site(&test_ctx.conn, 2); + let site2 = create_random_test_site(&mut test_ctx.conn); // Insert post in site 1 with categories let post1 = PostBuilder::minimal() diff --git a/wp_mobile_cache/src/term_relationships.rs b/wp_mobile_cache/src/term_relationships.rs index 36ea0d116..264630348 100644 --- a/wp_mobile_cache/src/term_relationships.rs +++ b/wp_mobile_cache/src/term_relationships.rs @@ -1,4 +1,4 @@ -use crate::{DbSite, RowId}; +use crate::RowId; use wp_api::taxonomies::TaxonomyType; use wp_api::terms::TermId; @@ -10,8 +10,8 @@ use wp_api::terms::TermId; pub struct DbTermRelationship { /// SQLite rowid of this relationship pub row_id: RowId, - /// Site this relationship belongs to - pub site: DbSite, + /// Database site ID (rowid from sites table) this relationship belongs to + pub db_site_id: RowId, /// Row ID of the object (post, page, etc.) in its respective table pub object_id: RowId, /// WordPress term ID diff --git a/wp_mobile_cache/src/test_fixtures.rs b/wp_mobile_cache/src/test_fixtures.rs index 0fd104ee0..53f7d4c67 100644 --- a/wp_mobile_cache/src/test_fixtures.rs +++ b/wp_mobile_cache/src/test_fixtures.rs @@ -1,11 +1,17 @@ use crate::{ - DbSite, MigrationManager, RowId, + MigrationManager, context::EditContext, - repository::{posts::PostRepository, term_relationships::TermRelationshipRepository}, + db_types::{db_site::DbSite, self_hosted_site::SelfHostedSite}, + repository::{ + posts::PostRepository, sites::SiteRepository, + term_relationships::TermRelationshipRepository, + }, }; use chrono::{DateTime, Utc}; +use integration_test_credentials::TestCredentials; use rstest::*; use rusqlite::Connection; +use std::sync::atomic::{AtomicU32, Ordering}; pub mod posts; @@ -17,7 +23,7 @@ pub mod posts; /// /// ```rust /// #[rstest] -/// fn test_something(test_ctx: TestContext) { +/// fn test_something(mut test_ctx: TestContext) { /// let post = PostBuilder::new().build(); /// test_ctx.post_repo.upsert(&mut test_ctx.conn, &test_ctx.site, &post).unwrap(); /// } @@ -31,38 +37,68 @@ pub struct TestContext { #[fixture] pub fn test_ctx() -> TestContext { + let (conn, site) = test_db(); TestContext { - conn: test_db(), - site: DbSite { row_id: RowId(1) }, + conn, + site, post_repo: PostRepository::new(), term_repo: TermRelationshipRepository, } } -fn test_db() -> Connection { - let conn = Connection::open_in_memory().unwrap(); +fn test_db() -> (Connection, DbSite) { + let mut conn = Connection::open_in_memory().unwrap(); let mut migration_manager = MigrationManager::new(&conn).unwrap(); migration_manager .perform_migrations() .expect("All migrations should succeed"); - // Insert default test site (id = 1) - conn.execute("INSERT INTO sites (id) VALUES (1)", []) - .expect("Failed to insert test site"); + // Insert default test site using real test credentials + let test_creds = TestCredentials::instance(); + let self_hosted_site = SelfHostedSite { + url: test_creds.site_url.to_string(), + api_root: format!("{}/wp-json", test_creds.site_url), + }; - conn + let db_site = create_test_site(&mut conn, &self_hosted_site); + + (conn, db_site) } -/// Helper to create an additional test site with a specific ID. +/// Helper to create a test site. /// -/// Useful when you need more than 2 sites or specific site IDs. -pub fn create_test_site(conn: &Connection, id: i64) -> DbSite { - conn.execute("INSERT INTO sites (id) VALUES (?)", [id]) - .expect("Failed to create test site"); - DbSite { - row_id: RowId(id as u64), - } +/// Uses `SiteRepository` to insert the site into the database. +pub fn create_test_site(conn: &mut Connection, site: &SelfHostedSite) -> DbSite { + let site_repo = SiteRepository; + let (db_site, _) = site_repo + .upsert_self_hosted_site(conn, site) + .expect("Failed to upsert test site"); + + db_site +} + +static RANDOM_TEST_SITE_COUNTER: AtomicU32 = AtomicU32::new(1); + +/// Helper to create a test site with auto-generated URL. +/// +/// Uses an internal counter to generate unique URLs for each call. +/// Useful for tests that need multiple sites but don't care about specific URLs. +/// +/// # Example +/// +/// ```rust +/// let site1 = create_random_test_site(&mut conn); +/// let site2 = create_random_test_site(&mut conn); +/// // site1 and site2 will have different URLs +/// ``` +pub fn create_random_test_site(conn: &mut Connection) -> DbSite { + let counter = RANDOM_TEST_SITE_COUNTER.fetch_add(1, Ordering::Relaxed); + let site = SelfHostedSite { + url: format!("https://test-site-{}.local", counter), + api_root: format!("https://test-site-{}.local/wp-json", counter), + }; + create_test_site(conn, &site) } /// Validates that a timestamp is a recent, valid ISO 8601 UTC timestamp.