From fe8939c07eea0e4f0727d569aa48341ed59b79f5 Mon Sep 17 00:00:00 2001 From: Jef Roosens Date: Wed, 19 Mar 2025 13:22:58 +0100 Subject: [PATCH 1/3] test(gpodder_sqlite): add remove old sessions test --- gpodder_sqlite/tests/auth_test.rs | 40 +++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/gpodder_sqlite/tests/auth_test.rs b/gpodder_sqlite/tests/auth_test.rs index 2fe0945..15117ed 100644 --- a/gpodder_sqlite/tests/auth_test.rs +++ b/gpodder_sqlite/tests/auth_test.rs @@ -102,3 +102,43 @@ fn test_refresh_session() { Some(new_session) ); } + +#[test] +fn remove_old_sessions() { + let (store, users) = common::setup(); + + let now = chrono::Utc::now().trunc_subsecs(0); + let timestamps = [ + now, + now - TimeDelta::seconds(1), + now - TimeDelta::seconds(2), + ]; + + for (i, ts) in timestamps.into_iter().enumerate() { + store + .insert_session(&Session { + id: i as i64, + last_seen: ts, + user: users[0].clone(), + }) + .expect("insert shouldn't fail"); + } + + assert_eq!( + store + .remove_old_sessions(now) + .expect("remove old sessions shouldn't fail"), + 2 + ); + + assert!(matches!(store.get_session(1), Ok(None))); + assert!(matches!(store.get_session(2), Ok(None))); + assert_eq!( + store.get_session(0).expect("get session shouldn't fail"), + Some(Session { + id: 0, + last_seen: now, + user: users[0].clone(), + }) + ); +} From 22016fe0e9fb4288c47be081720a99aa8fee0486 Mon Sep 17 00:00:00 2001 From: Jef Roosens Date: Wed, 19 Mar 2025 14:58:04 +0100 Subject: [PATCH 2/3] fix(gpodder_sqlite): force in-memory database to consist of only one connection --- gpodder_sqlite/src/lib.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/gpodder_sqlite/src/lib.rs b/gpodder_sqlite/src/lib.rs index f2f0082..10de4e4 100644 --- a/gpodder_sqlite/src/lib.rs +++ b/gpodder_sqlite/src/lib.rs @@ -98,6 +98,10 @@ pub fn initialize_db(path: impl AsRef, run_migrations: bool) -> Result Result { let manager = ConnectionManager::::new(":memory:"); let pool = Pool::builder() + // An in-memory database is recreated for each opened connection, so we try to force only a + // single connection to stay alive + .min_idle(Some(1)) + .max_size(1) .connection_customizer(Box::new(AddQueryDebugLogs)) .build(manager)?; From 73988d6264bd626c40f164c63f770bce0e7c24f6 Mon Sep 17 00:00:00 2001 From: Jef Roosens Date: Wed, 19 Mar 2025 15:00:00 +0100 Subject: [PATCH 3/3] test(gpodder_sqlite): start device tests --- gpodder_sqlite/.env | 2 +- gpodder_sqlite/.gitignore | 1 + gpodder_sqlite/diesel.toml | 2 +- gpodder_sqlite/src/repository/device.rs | 10 +++--- gpodder_sqlite/tests/auth_test.rs | 2 +- gpodder_sqlite/tests/device_test.rs | 41 +++++++++++++++++++++++++ 6 files changed, 51 insertions(+), 7 deletions(-) create mode 100644 gpodder_sqlite/.gitignore create mode 100644 gpodder_sqlite/tests/device_test.rs diff --git a/gpodder_sqlite/.env b/gpodder_sqlite/.env index 2b8d56c..79c8a77 100644 --- a/gpodder_sqlite/.env +++ b/gpodder_sqlite/.env @@ -1 +1 @@ -DATABASE_URL=data/otter.sqlite3 +DATABASE_URL=otter.sqlite3 diff --git a/gpodder_sqlite/.gitignore b/gpodder_sqlite/.gitignore new file mode 100644 index 0000000..a047466 --- /dev/null +++ b/gpodder_sqlite/.gitignore @@ -0,0 +1 @@ +otter.sqlite3 diff --git a/gpodder_sqlite/diesel.toml b/gpodder_sqlite/diesel.toml index f348d3c..79a9129 100644 --- a/gpodder_sqlite/diesel.toml +++ b/gpodder_sqlite/diesel.toml @@ -2,7 +2,7 @@ # see https://diesel.rs/guides/configuring-diesel-cli [print_schema] -file = "src/db/schema.rs" +file = "src/schema.rs" custom_type_derives = ["diesel::query_builder::QueryId", "Clone"] sqlite_integer_primary_key_is_bigint = true diff --git a/gpodder_sqlite/src/repository/device.rs b/gpodder_sqlite/src/repository/device.rs index d69caeb..4b91a06 100644 --- a/gpodder_sqlite/src/repository/device.rs +++ b/gpodder_sqlite/src/repository/device.rs @@ -1,7 +1,7 @@ use std::collections::HashSet; use chrono::{DateTime, Utc}; -use diesel::{alias, dsl::not, prelude::*}; +use diesel::{alias, dsl::not, prelude::*, sqlite::Sqlite}; use gpodder::AuthErr; use super::SqliteRepository; @@ -70,6 +70,8 @@ impl gpodder::DeviceRepository for SqliteRepository { patch: gpodder::DevicePatch, ) -> Result<(), gpodder::AuthErr> { (|| { + let conn = &mut self.pool.get()?; + if let Some(mut device) = devices::table .select(Device::as_select()) .filter( @@ -77,7 +79,7 @@ impl gpodder::DeviceRepository for SqliteRepository { .eq(user.id) .and(devices::device_id.eq(device_id)), ) - .get_result(&mut self.pool.get()?) + .get_result(conn) .optional()? { if let Some(caption) = patch.caption { @@ -93,7 +95,7 @@ impl gpodder::DeviceRepository for SqliteRepository { devices::caption.eq(&device.caption), devices::type_.eq(&device.type_), )) - .execute(&mut self.pool.get()?)?; + .execute(conn)?; } else { let device = NewDevice { device_id: device_id.to_string(), @@ -104,7 +106,7 @@ impl gpodder::DeviceRepository for SqliteRepository { diesel::insert_into(devices::table) .values(device) - .execute(&mut self.pool.get()?)?; + .execute(conn)?; } Ok::<_, DbError>(()) diff --git a/gpodder_sqlite/tests/auth_test.rs b/gpodder_sqlite/tests/auth_test.rs index 15117ed..7eadfe5 100644 --- a/gpodder_sqlite/tests/auth_test.rs +++ b/gpodder_sqlite/tests/auth_test.rs @@ -104,7 +104,7 @@ fn test_refresh_session() { } #[test] -fn remove_old_sessions() { +fn test_remove_old_sessions() { let (store, users) = common::setup(); let now = chrono::Utc::now().trunc_subsecs(0); diff --git a/gpodder_sqlite/tests/device_test.rs b/gpodder_sqlite/tests/device_test.rs new file mode 100644 index 0000000..6cf5520 --- /dev/null +++ b/gpodder_sqlite/tests/device_test.rs @@ -0,0 +1,41 @@ +mod common; + +use gpodder::{DevicePatch, DeviceRepository, DeviceType}; + +#[test] +fn test_insert_devices() { + let (store, users) = common::setup(); + + store.devices_for_user(&users[0]).unwrap(); + + store + .update_device_info( + &users[0], + "device1", + DevicePatch { + caption: Some("caption1".to_string()), + r#type: Some(DeviceType::Other), + }, + ) + .expect("update info shouldn't fail"); + + store + .update_device_info( + &users[1], + "device2", + DevicePatch { + caption: Some("caption2".to_string()), + r#type: Some(DeviceType::Laptop), + }, + ) + .expect("update info shouldn't fail"); + + let devices = store.devices_for_user(&users[0]); + assert!(matches!(devices, Ok(_))); + + let devices = devices.unwrap(); + assert_eq!(devices.len(), 1); + + assert_eq!(devices[0].caption, "caption1"); + assert_eq!(devices[0].r#type, DeviceType::Other); +}