From 064365fb4feb9ffc470d65c5b4c542c9d5d04e1f Mon Sep 17 00:00:00 2001 From: Jef Roosens Date: Tue, 4 Mar 2025 08:46:49 +0100 Subject: [PATCH] refactor: decide to not create separate table for subscriptions --- migrations/2025-02-23-095541_initial/up.sql | 19 +--- src/db/mod.rs | 2 +- ...subscription.rs => device_subscription.rs} | 100 +++++++++--------- src/db/models/episode_action.rs | 8 +- src/db/models/mod.rs | 2 +- src/db/repository/subscription.rs | 88 +++++++-------- src/db/schema.rs | 29 +++-- 7 files changed, 119 insertions(+), 129 deletions(-) rename src/db/models/{subscription.rs => device_subscription.rs} (65%) diff --git a/migrations/2025-02-23-095541_initial/up.sql b/migrations/2025-02-23-095541_initial/up.sql index a1b1773..e2dc295 100644 --- a/migrations/2025-02-23-095541_initial/up.sql +++ b/migrations/2025-02-23-095541_initial/up.sql @@ -29,41 +29,32 @@ create table devices ( unique (user_id, device_id) ); -create table subscriptions ( - id integer primary key not null, - - url text unique not null -); - create table device_subscriptions ( id integer primary key not null, device_id bigint not null references devices (id) on delete cascade, - subscription_id bigint not null - references subscriptions (id) - on delete cascade + + podcast_url text not null, time_changed bigint not null default 0, deleted boolean not null default false, - unique (device_id, subscription_id) + unique (device_id, podcast_url) ); create table episode_actions ( id integer primary key not null, - subscription_id bigint not null - references subscriptions (id) - on delete set null, - -- Can be null, as the device is not always provided device_id bigint references devices (id) on delete set null, + podcast_url text not null, episode_url text not null, + timestamp bigint, action text not null, started integer, diff --git a/src/db/mod.rs b/src/db/mod.rs index e09f4be..5a610e4 100644 --- a/src/db/mod.rs +++ b/src/db/mod.rs @@ -5,7 +5,7 @@ mod schema; pub use models::device::{Device, DeviceType, NewDevice}; pub use models::episode_action::{ActionType, EpisodeAction, NewEpisodeAction}; pub use models::session::Session; -pub use models::subscription::{NewSubscription, Subscription}; +pub use models::device_subscription::{NewDeviceSubscription, DeviceSubscription}; pub use models::user::{NewUser, User}; pub use repository::SqliteRepository; diff --git a/src/db/models/subscription.rs b/src/db/models/device_subscription.rs similarity index 65% rename from src/db/models/subscription.rs rename to src/db/models/device_subscription.rs index d0bb1be..338be97 100644 --- a/src/db/models/subscription.rs +++ b/src/db/models/device_subscription.rs @@ -6,39 +6,39 @@ use serde::{Deserialize, Serialize}; use crate::db::{schema::*, DbPool, DbResult}; #[derive(Serialize, Deserialize, Clone, Queryable, Selectable)] -#[diesel(table_name = subscriptions)] +#[diesel(table_name = device_subscriptions)] #[diesel(check_for_backend(diesel::sqlite::Sqlite))] -pub struct Subscription { +pub struct DeviceSubscription { pub id: i64, pub device_id: i64, - pub url: String, + pub podcast_url: String, pub time_changed: i64, pub deleted: bool, } #[derive(Deserialize, Insertable)] -#[diesel(table_name = subscriptions)] +#[diesel(table_name = device_subscriptions)] #[diesel(check_for_backend(diesel::sqlite::Sqlite))] -pub struct NewSubscription { +pub struct NewDeviceSubscription { pub device_id: i64, - pub url: String, + pub podcast_url: String, pub time_changed: i64, pub deleted: bool, } -impl Subscription { +impl DeviceSubscription { pub fn for_device(pool: &DbPool, device_id: i64) -> DbResult> { - Ok(subscriptions::table - .select(subscriptions::url) - .filter(subscriptions::device_id.eq(device_id)) + Ok(device_subscriptions::table + .select(device_subscriptions::podcast_url) + .filter(device_subscriptions::device_id.eq(device_id)) .get_results(&mut pool.get()?)?) } pub fn for_user(pool: &DbPool, user_id: i64) -> DbResult> { - Ok(subscriptions::table + Ok(device_subscriptions::table .inner_join(devices::table) .filter(devices::user_id.eq(user_id)) - .select(subscriptions::url) + .select(device_subscriptions::podcast_url) .distinct() .get_results(&mut pool.get()?)?) } @@ -55,9 +55,9 @@ impl Subscription { // on conflict. Therefore, we instead calculate which URLs should be inserted and which // updated, so we avoid conflicts. let urls: HashSet = urls.into_iter().collect(); - let urls_in_db: HashSet = subscriptions::table - .select(subscriptions::url) - .filter(subscriptions::device_id.eq(device_id)) + let urls_in_db: HashSet = device_subscriptions::table + .select(device_subscriptions::podcast_url) + .filter(device_subscriptions::device_id.eq(device_id)) .get_results(&mut pool.get()?)? .into_iter() .collect(); @@ -75,41 +75,41 @@ impl Subscription { // Mark the URLs to delete as properly deleted diesel::update( - subscriptions::table.filter( - subscriptions::device_id + device_subscriptions::table.filter( + device_subscriptions::device_id .eq(device_id) - .and(subscriptions::url.eq_any(urls_to_delete)), + .and(device_subscriptions::podcast_url.eq_any(urls_to_delete)), ), ) .set(( - subscriptions::deleted.eq(true), - subscriptions::time_changed.eq(timestamp), + device_subscriptions::deleted.eq(true), + device_subscriptions::time_changed.eq(timestamp), )) .execute(conn)?; // Update the existing deleted URLs that are reinserted as no longer deleted diesel::update( - subscriptions::table.filter( - subscriptions::device_id + device_subscriptions::table.filter( + device_subscriptions::device_id .eq(device_id) - .and(subscriptions::url.eq_any(urls_to_update)) - .and(subscriptions::deleted.eq(true)), + .and(device_subscriptions::podcast_url.eq_any(urls_to_update)) + .and(device_subscriptions::deleted.eq(true)), ), ) .set(( - subscriptions::deleted.eq(false), - subscriptions::time_changed.eq(timestamp), + device_subscriptions::deleted.eq(false), + device_subscriptions::time_changed.eq(timestamp), )) .execute(conn)?; // Insert the new values into the database - diesel::insert_into(subscriptions::table) + diesel::insert_into(device_subscriptions::table) .values( urls_to_insert .into_iter() - .map(|url| NewSubscription { + .map(|url| NewDeviceSubscription { device_id, - url: url.to_string(), + podcast_url: url.to_string(), deleted: false, time_changed: timestamp, }) @@ -134,9 +134,9 @@ impl Subscription { let removed: HashSet<_> = removed.into_iter().collect(); pool.get()?.transaction(|conn| { - let urls_in_db: HashSet = subscriptions::table - .select(subscriptions::url) - .filter(subscriptions::device_id.eq(device_id)) + let urls_in_db: HashSet = device_subscriptions::table + .select(device_subscriptions::podcast_url) + .filter(device_subscriptions::device_id.eq(device_id)) .get_results(&mut pool.get()?)? .into_iter() .collect(); @@ -148,16 +148,16 @@ impl Subscription { let urls_to_delete = removed.intersection(&urls_in_db); diesel::update( - subscriptions::table.filter( - subscriptions::device_id + device_subscriptions::table.filter( + device_subscriptions::device_id .eq(device_id) - .and(subscriptions::url.eq_any(urls_to_delete)) - .and(subscriptions::deleted.eq(false)), + .and(device_subscriptions::podcast_url.eq_any(urls_to_delete)) + .and(device_subscriptions::deleted.eq(false)), ), ) .set(( - subscriptions::deleted.eq(true), - subscriptions::time_changed.eq(timestamp), + device_subscriptions::deleted.eq(true), + device_subscriptions::time_changed.eq(timestamp), )) .execute(conn)?; @@ -166,16 +166,16 @@ impl Subscription { let urls_to_update = added.intersection(&urls_in_db); diesel::update( - subscriptions::table.filter( - subscriptions::device_id + device_subscriptions::table.filter( + device_subscriptions::device_id .eq(device_id) - .and(subscriptions::url.eq_any(urls_to_update)) - .and(subscriptions::deleted.eq(true)), + .and(device_subscriptions::podcast_url.eq_any(urls_to_update)) + .and(device_subscriptions::deleted.eq(true)), ), ) .set(( - subscriptions::deleted.eq(false), - subscriptions::time_changed.eq(timestamp), + device_subscriptions::deleted.eq(false), + device_subscriptions::time_changed.eq(timestamp), )) .execute(conn)?; @@ -183,13 +183,13 @@ impl Subscription { // added list let urls_to_insert = added.difference(&urls_in_db); - diesel::insert_into(subscriptions::table) + diesel::insert_into(device_subscriptions::table) .values( urls_to_insert .into_iter() - .map(|url| NewSubscription { + .map(|url| NewDeviceSubscription { device_id, - url: url.to_string(), + podcast_url: url.to_string(), deleted: false, time_changed: timestamp, }) @@ -206,12 +206,12 @@ impl Subscription { device_id: i64, timestamp: i64, ) -> DbResult> { - Ok(subscriptions::table + Ok(device_subscriptions::table .select(Self::as_select()) .filter( - subscriptions::device_id + device_subscriptions::device_id .eq(device_id) - .and(subscriptions::time_changed.ge(timestamp)), + .and(device_subscriptions::time_changed.ge(timestamp)), ) .get_results(&mut pool.get()?)?) } diff --git a/src/db/models/episode_action.rs b/src/db/models/episode_action.rs index 3305742..2ab7907 100644 --- a/src/db/models/episode_action.rs +++ b/src/db/models/episode_action.rs @@ -19,9 +19,9 @@ use crate::db::{schema::*, DbPool, DbResult}; #[diesel(check_for_backend(diesel::sqlite::Sqlite))] pub struct EpisodeAction { id: i64, - subscription_id: i64, device_id: Option, - episode: String, + podcast_url: String, + episode_url: String, timestamp: Option, action: ActionType, started: Option, @@ -33,9 +33,9 @@ pub struct EpisodeAction { #[diesel(table_name = episode_actions)] #[diesel(check_for_backend(diesel::sqlite::Sqlite))] pub struct NewEpisodeAction { - subscription_id: i64, device_id: Option, - episode: String, + podcast_url: String, + episode_url: String, timestamp: Option, action: ActionType, started: Option, diff --git a/src/db/models/mod.rs b/src/db/models/mod.rs index 6e36b0f..440060b 100644 --- a/src/db/models/mod.rs +++ b/src/db/models/mod.rs @@ -1,5 +1,5 @@ pub mod device; +pub mod device_subscription; pub mod episode_action; pub mod session; -pub mod subscription; pub mod user; diff --git a/src/db/repository/subscription.rs b/src/db/repository/subscription.rs index 82d9e39..b72c8ac 100644 --- a/src/db/repository/subscription.rs +++ b/src/db/repository/subscription.rs @@ -13,10 +13,10 @@ impl gpodder::SubscriptionRepository for SqliteRepository { &self, user: &gpodder::User, ) -> Result, gpodder::AuthErr> { - Ok(subscriptions::table + Ok(device_subscriptions::table .inner_join(devices::table) .filter(devices::user_id.eq(user.id)) - .select(subscriptions::url) + .select(device_subscriptions::podcast_url) .distinct() .get_results(&mut self.pool.get()?)?) } @@ -26,14 +26,14 @@ impl gpodder::SubscriptionRepository for SqliteRepository { user: &gpodder::User, device_id: &str, ) -> Result, gpodder::AuthErr> { - Ok(subscriptions::table + Ok(device_subscriptions::table .inner_join(devices::table) .filter( devices::user_id .eq(user.id) .and(devices::device_id.eq(device_id)), ) - .select(subscriptions::url) + .select(device_subscriptions::podcast_url) .get_results(&mut self.pool.get()?)?) } @@ -61,9 +61,9 @@ impl gpodder::SubscriptionRepository for SqliteRepository { // on conflict. Therefore, we instead calculate which URLs should be inserted and which // updated, so we avoid conflicts. let urls: HashSet = urls.into_iter().collect(); - let urls_in_db: HashSet = subscriptions::table - .select(subscriptions::url) - .filter(subscriptions::device_id.eq(device.id)) + let urls_in_db: HashSet = device_subscriptions::table + .select(device_subscriptions::podcast_url) + .filter(device_subscriptions::device_id.eq(device.id)) .get_results(conn)? .into_iter() .collect(); @@ -81,41 +81,41 @@ impl gpodder::SubscriptionRepository for SqliteRepository { // Mark the URLs to delete as properly deleted diesel::update( - subscriptions::table.filter( - subscriptions::device_id + device_subscriptions::table.filter( + device_subscriptions::device_id .eq(device.id) - .and(subscriptions::url.eq_any(urls_to_delete)), + .and(device_subscriptions::podcast_url.eq_any(urls_to_delete)), ), ) .set(( - subscriptions::deleted.eq(true), - subscriptions::time_changed.eq(timestamp), + device_subscriptions::deleted.eq(true), + device_subscriptions::time_changed.eq(timestamp), )) .execute(conn)?; // Update the existing deleted URLs that are reinserted as no longer deleted diesel::update( - subscriptions::table.filter( - subscriptions::device_id + device_subscriptions::table.filter( + device_subscriptions::device_id .eq(device.id) - .and(subscriptions::url.eq_any(urls_to_update)) - .and(subscriptions::deleted.eq(true)), + .and(device_subscriptions::podcast_url.eq_any(urls_to_update)) + .and(device_subscriptions::deleted.eq(true)), ), ) .set(( - subscriptions::deleted.eq(false), - subscriptions::time_changed.eq(timestamp), + device_subscriptions::deleted.eq(false), + device_subscriptions::time_changed.eq(timestamp), )) .execute(conn)?; // Insert the new values into the database - diesel::insert_into(subscriptions::table) + diesel::insert_into(device_subscriptions::table) .values( urls_to_insert .into_iter() - .map(|url| db::NewSubscription { + .map(|url| db::NewDeviceSubscription { device_id: device.id, - url: url.to_string(), + podcast_url: url.to_string(), deleted: false, time_changed: timestamp, }) @@ -154,9 +154,9 @@ impl gpodder::SubscriptionRepository for SqliteRepository { ) .get_result(conn)?; - let urls_in_db: HashSet = subscriptions::table - .select(subscriptions::url) - .filter(subscriptions::device_id.eq(device.id)) + let urls_in_db: HashSet = device_subscriptions::table + .select(device_subscriptions::podcast_url) + .filter(device_subscriptions::device_id.eq(device.id)) .get_results(conn)? .into_iter() .collect(); @@ -168,16 +168,16 @@ impl gpodder::SubscriptionRepository for SqliteRepository { let urls_to_delete = remove.intersection(&urls_in_db); diesel::update( - subscriptions::table.filter( - subscriptions::device_id + device_subscriptions::table.filter( + device_subscriptions::device_id .eq(device.id) - .and(subscriptions::url.eq_any(urls_to_delete)) - .and(subscriptions::deleted.eq(false)), + .and(device_subscriptions::podcast_url.eq_any(urls_to_delete)) + .and(device_subscriptions::deleted.eq(false)), ), ) .set(( - subscriptions::deleted.eq(true), - subscriptions::time_changed.eq(timestamp), + device_subscriptions::deleted.eq(true), + device_subscriptions::time_changed.eq(timestamp), )) .execute(conn)?; @@ -186,16 +186,16 @@ impl gpodder::SubscriptionRepository for SqliteRepository { let urls_to_update = add.intersection(&urls_in_db); diesel::update( - subscriptions::table.filter( - subscriptions::device_id + device_subscriptions::table.filter( + device_subscriptions::device_id .eq(device.id) - .and(subscriptions::url.eq_any(urls_to_update)) - .and(subscriptions::deleted.eq(true)), + .and(device_subscriptions::podcast_url.eq_any(urls_to_update)) + .and(device_subscriptions::deleted.eq(true)), ), ) .set(( - subscriptions::deleted.eq(false), - subscriptions::time_changed.eq(timestamp), + device_subscriptions::deleted.eq(false), + device_subscriptions::time_changed.eq(timestamp), )) .execute(conn)?; @@ -203,13 +203,13 @@ impl gpodder::SubscriptionRepository for SqliteRepository { // added list let urls_to_insert = add.difference(&urls_in_db); - diesel::insert_into(subscriptions::table) + diesel::insert_into(device_subscriptions::table) .values( urls_to_insert .into_iter() - .map(|url| db::NewSubscription { + .map(|url| db::NewDeviceSubscription { device_id: device.id, - url: url.to_string(), + podcast_url: url.to_string(), deleted: false, time_changed: timestamp, }) @@ -231,23 +231,23 @@ impl gpodder::SubscriptionRepository for SqliteRepository { ) -> Result<(i64, Vec, Vec), gpodder::AuthErr> { let (mut timestamp, mut added, mut removed) = (0, Vec::new(), Vec::new()); - let query = subscriptions::table + let query = device_subscriptions::table .inner_join(devices::table) .filter( devices::user_id .eq(user.id) .and(devices::device_id.eq(device_id)) - .and(subscriptions::time_changed.ge(since)), + .and(device_subscriptions::time_changed.ge(since)), ) - .select(db::Subscription::as_select()); + .select(db::DeviceSubscription::as_select()); for sub in query.load_iter(&mut self.pool.get()?)? { let sub = sub?; if sub.deleted { - removed.push(sub.url); + removed.push(sub.podcast_url); } else { - added.push(sub.url); + added.push(sub.podcast_url); } timestamp = timestamp.max(sub.time_changed); diff --git a/src/db/schema.rs b/src/db/schema.rs index a7740af..e5ff905 100644 --- a/src/db/schema.rs +++ b/src/db/schema.rs @@ -1,5 +1,15 @@ // @generated automatically by Diesel CLI. +diesel::table! { + device_subscriptions (id) { + id -> BigInt, + device_id -> BigInt, + podcast_url -> Text, + time_changed -> BigInt, + deleted -> Bool, + } +} + diesel::table! { devices (id) { id -> BigInt, @@ -14,9 +24,9 @@ diesel::table! { diesel::table! { episode_actions (id) { id -> BigInt, - subscription_id -> BigInt, device_id -> Nullable, - episode -> Text, + podcast_url -> Text, + episode_url -> Text, timestamp -> Nullable, action -> Text, started -> Nullable, @@ -32,16 +42,6 @@ diesel::table! { } } -diesel::table! { - subscriptions (id) { - id -> BigInt, - device_id -> BigInt, - url -> Text, - time_changed -> BigInt, - deleted -> Bool, - } -} - diesel::table! { users (id) { id -> BigInt, @@ -50,16 +50,15 @@ diesel::table! { } } +diesel::joinable!(device_subscriptions -> devices (device_id)); diesel::joinable!(devices -> users (user_id)); diesel::joinable!(episode_actions -> devices (device_id)); -diesel::joinable!(episode_actions -> subscriptions (subscription_id)); diesel::joinable!(sessions -> users (user_id)); -diesel::joinable!(subscriptions -> devices (device_id)); diesel::allow_tables_to_appear_in_same_query!( + device_subscriptions, devices, episode_actions, sessions, - subscriptions, users, );