From 7caa5f2a082fd6c6b5f46a5297ea86ee56219353 Mon Sep 17 00:00:00 2001 From: ThetaDev Date: Sun, 17 Sep 2023 04:17:45 +0200 Subject: [PATCH 1/3] feat: create graph from playlist changes --- crates/db/src/models/change_operation.rs | 16 ++++++++ crates/db/src/models/playlist.rs | 45 +++++++++++++++++++++ crates/db/src/models/playlist_change.rs | 50 ++++++++++++++++++++++++ crates/db/src/testutil.rs | 16 +++++++- 4 files changed, 126 insertions(+), 1 deletion(-) diff --git a/crates/db/src/models/change_operation.rs b/crates/db/src/models/change_operation.rs index 08f9b02..51f91cf 100644 --- a/crates/db/src/models/change_operation.rs +++ b/crates/db/src/models/change_operation.rs @@ -99,6 +99,22 @@ impl ChangeOperation { } } +impl ToString for ChangeOperation { + fn to_string(&self) -> String { + match self { + ChangeOperation::Ins { pos, val } => { + let mut s = format!("INS {pos}: "); + for v in val { + s += &v.to_string(); + } + s + } + ChangeOperation::Del { pos, n } => format!("DEL {pos} ({n})"), + ChangeOperation::Mov { pos, n, to } => format!("MOV {pos} ({n}) -> {to}"), + } + } +} + #[cfg(test)] mod tests { use crate::models::MusicService; diff --git a/crates/db/src/models/playlist.rs b/crates/db/src/models/playlist.rs index b25e59d..0b40eb0 100644 --- a/crates/db/src/models/playlist.rs +++ b/crates/db/src/models/playlist.rs @@ -835,4 +835,49 @@ mod tests { "got: {entries:#?}" ); } + + #[sqlx_database_tester::test(pool(variable = "pool"))] + async fn add_changes_1() { + testutil::run_sql("base.sql", &pool).await; + + let mut tx = pool.begin().await.unwrap(); + Playlist::add_change( + Some(ids::PLAYLIST_CHANGE_HEAD), + ids::PLAYLIST_ID_TEST1, + vec![ChangeOperation::Ins { + pos: 3, + val: vec![SrcIdOwned("2txScm52-QI".to_owned(), MusicService::YouTube)], + }], + &mut tx, + ) + .await + .unwrap(); + tx.commit().await.unwrap(); + + let mut tx = pool.begin().await.unwrap(); + Playlist::add_change( + Some(ids::PLAYLIST_CHANGE_I3), + ids::PLAYLIST_ID_TEST1, + vec![ChangeOperation::Del { pos: 1, n: 1 }], + &mut tx, + ) + .await + .unwrap(); + tx.commit().await.unwrap(); + + let pl = Playlist::get(ids::PLAYLIST_TEST1, &pool).await.unwrap(); + let entries = pl.get_entries(&pool).await.unwrap(); + + assert!( + testutil::compare_pl_entries( + &entries, + &[ + SrcId("WSBUeFdXiSs", MusicService::YouTube), + SrcId("6485PhOtHzY", MusicService::YouTube), + SrcId("2txScm52-QI", MusicService::YouTube) + ], + ), + "got: {entries:#?}" + ); + } } diff --git a/crates/db/src/models/playlist_change.rs b/crates/db/src/models/playlist_change.rs index a3f22e6..3bfcd90 100644 --- a/crates/db/src/models/playlist_change.rs +++ b/crates/db/src/models/playlist_change.rs @@ -1,3 +1,5 @@ +use std::fmt::Debug; + use nonempty_collections::{nev, NonEmptyIterator}; use otvec::Operation; use serde::Serialize; @@ -273,6 +275,54 @@ parent1_id, parent2_id, created_at from playlist_changes where playlist_id=$1 or ncm.insert_with_id(merge_ids.merge, exec).await?; Ok(merge_ids.merge) } + + /// Create a Graphviz graph from playlist changes + pub fn change_graph(changes: &[PlaylistChange]) -> String { + let mut g = String::from("digraph {\n node [shape=box];\n"); + + let line_start = " \""; + let line_end = "\"\n"; + let arrow = "\"->\""; + + for change in changes { + let cid = change.id.to_string(); + + g += line_start; + g += &cid; + g += "\" [label=<#"; + g += &change.seq.to_string(); + g += "
"; + g += &cid; + g += "
"; + + for op in change.operations.0.iter().take(5) { + g += "
"; + g += &op.to_string(); + } + if change.operations.0.len() > 5 { + g += &format!("
({} more ops)", change.operations.0.len() - 5); + } + g += " >]\n"; + + if let Some(p1) = change.parent1_id { + g += line_start; + g += &p1.to_string(); + g += arrow; + g += &cid; + g += line_end; + } + if let Some(p2) = change.parent2_id { + g += line_start; + g += &p2.to_string(); + g += arrow; + g += &cid; + g += line_end; + } + } + + g += "}\n"; + g + } } impl PlaylistChangeNew { diff --git a/crates/db/src/testutil.rs b/crates/db/src/testutil.rs index ab82e6c..e3902aa 100644 --- a/crates/db/src/testutil.rs +++ b/crates/db/src/testutil.rs @@ -3,7 +3,7 @@ use std::path::PathBuf; use once_cell::sync::Lazy; use path_macro::path; -use crate::models::{PlaylistEntry, SrcId}; +use crate::models::{PlaylistChange, PlaylistEntry, SrcId}; pub static TESTDATA: Lazy = Lazy::new(|| path!(env!("CARGO_MANIFEST_DIR") / "testdata")); @@ -64,3 +64,17 @@ pub fn compare_pl_entries(entries: &[PlaylistEntry], ids: &[SrcId]) -> bool { } entries.iter().zip(ids).all(|(a, b)| a.id == *b) } + +/// Print the change graph of a playlist +/// +/// Use Graphviz to visualize: +pub async fn print_change_graph<'a, E>(playlist_id: i32, exec: E) +where + E: sqlx::Executor<'a, Database = sqlx::Postgres>, +{ + let changes = PlaylistChange::get_all_changes(playlist_id, exec) + .await + .unwrap(); + let graph = PlaylistChange::change_graph(&changes); + println!("{graph}"); +} From b01bf69f97f976eb2f29323a384383c6234a838e Mon Sep 17 00:00:00 2001 From: ThetaDev Date: Sun, 17 Sep 2023 19:10:04 +0200 Subject: [PATCH 2/3] tests: add more tests for playlist vcs, remove playlist cache from model --- Cargo.lock | 128 ++++++++++-------- crates/db/src/models/change_operation.rs | 53 ++++++-- crates/db/src/models/mod.rs | 2 + crates/db/src/models/playlist.rs | 126 +++++++++++++---- crates/db/src/models/playlist_change.rs | 112 ++++++++++++++- ...__models__playlist__tests__crud_empty.snap | 2 - ...odels__playlist__tests__crud_inserted.snap | 2 - crates/db/src/models/types.rs | 3 +- 8 files changed, 317 insertions(+), 111 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5326cf1..03b6739 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -73,9 +73,9 @@ checksum = "9e1b586273c5702936fe7b7d6896644d8be71e6314cfe09d3167c95f712589e8" [[package]] name = "base64" -version = "0.21.3" +version = "0.21.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "414dcefbc63d77c526a76b3afcf6fbb9b5e2791c19c3aa2297733208750c6e53" +checksum = "9ba43ea6f343b788c8764558649e08df62f86c6ef251fdaeb1ffd010a9ae50a2" [[package]] name = "base64ct" @@ -109,9 +109,9 @@ dependencies = [ [[package]] name = "bumpalo" -version = "3.13.0" +version = "3.14.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a3e2c3daef883ecc1b5d58c15adae93470a91d425f3532ba1695849656af3fc1" +checksum = "7f30e7476521f6f8af1a1c4c0b8cc94f0bee37d91763d0ca2665f299b6cd8aec" [[package]] name = "byteorder" @@ -121,9 +121,9 @@ checksum = "14c189c53d098945499cdfa7ecc63567cf3886b3332b312a5b4585d8d3a6a610" [[package]] name = "bytes" -version = "1.4.0" +version = "1.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "89b2fd2a0dcf38d7971e2194b6b6eebab45ae01067456a7fd93d5547a61b70be" +checksum = "a2bd12c1caf447e69cd4528f47f94d203fd2582878ecb9e9465484c4148a8223" [[package]] name = "cc" @@ -349,6 +349,12 @@ version = "2.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6999dc1837253364c2ebb0704ba97994bd874e8f195d665c50b7548f6ea92764" +[[package]] +name = "finl_unicode" +version = "1.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8fcfdc7a0362c9f4444381a9e697c79d435fe65b52a37466fc2c1184cee9edc6" + [[package]] name = "flume" version = "0.10.14" @@ -443,7 +449,7 @@ checksum = "89ca545a94061b6365f2c7355b4b32bd20df3ff95f02da9329b34ccc3bd6ee72" dependencies = [ "proc-macro2", "quote", - "syn 2.0.29", + "syn 2.0.36", ] [[package]] @@ -615,9 +621,9 @@ dependencies = [ [[package]] name = "itertools" -version = "0.10.5" +version = "0.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b0fd2260e829bddf4cb6ea802289de2f86d6a7a690192fbe91b3f46e0f2c8473" +checksum = "b1c173a5686ce8bfa551b3563d0c2170bf24ca44da99c7ca4bfdab5418c3fe57" dependencies = [ "either", ] @@ -648,9 +654,9 @@ dependencies = [ [[package]] name = "libc" -version = "0.2.147" +version = "0.2.148" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b4668fb0ea861c1df094127ac5f1da3409a82116a4ba74fca2e58ef927159bb3" +checksum = "9cdc71e17332e86d2e1d38c1f99edcb6288ee11b815fb1a4b049eaa2114d369b" [[package]] name = "libm" @@ -677,9 +683,9 @@ checksum = "0717cef1bc8b636c6e1c1bbdefc09e6322da8a9321966e8928ef80d20f7f770f" [[package]] name = "linux-raw-sys" -version = "0.4.5" +version = "0.4.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "57bcfdad1b858c2db7c38303a6d2ad4dfaf5eb53dfeb0910128b2c26d6158503" +checksum = "1a9bad9f94746442c783ca431b22403b519cd7fbeed0533fdd6328b2f2212128" [[package]] name = "lock_api" @@ -708,9 +714,9 @@ dependencies = [ [[package]] name = "memchr" -version = "2.6.0" +version = "2.6.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "76fc44e2588d5b436dbc3c6cf62aef290f90dab6235744a93dfe1cc18f451e2c" +checksum = "8f232d6ef707e1956a43342693d2a31e72989554d58299d7a88738cc95b0d35c" [[package]] name = "minimal-lexical" @@ -817,9 +823,9 @@ dependencies = [ [[package]] name = "object" -version = "0.32.0" +version = "0.32.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "77ac5bbd07aea88c60a577a1ce218075ffd59208b2d7ca97adf9bfc5aeb21ebe" +checksum = "9cf5f9dd3933bd50a9e1f149ec995f39ae2c496d31fd772c1fd45ebc27e902b0" dependencies = [ "memchr", ] @@ -890,19 +896,20 @@ checksum = "9b2a4787296e9989611394c33f193f676704af1686e70b8f8033ab5ba9a35a94" [[package]] name = "pest" -version = "2.7.2" +version = "2.7.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1acb4a4365a13f749a93f1a094a7805e5cfa0955373a9de860d962eaa3a5fe5a" +checksum = "d7a4d085fd991ac8d5b05a147b437791b4260b76326baf0fc60cf7c9c27ecd33" dependencies = [ + "memchr", "thiserror", "ucd-trie", ] [[package]] name = "pest_derive" -version = "2.7.2" +version = "2.7.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "666d00490d4ac815001da55838c500eafb0320019bbaa44444137c48b443a853" +checksum = "a2bee7be22ce7918f641a33f08e3f43388c7656772244e2bbb2477f44cc9021a" dependencies = [ "pest", "pest_generator", @@ -910,22 +917,22 @@ dependencies = [ [[package]] name = "pest_generator" -version = "2.7.2" +version = "2.7.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "68ca01446f50dbda87c1786af8770d535423fa8a53aec03b8f4e3d7eb10e0929" +checksum = "d1511785c5e98d79a05e8a6bc34b4ac2168a0e3e92161862030ad84daa223141" dependencies = [ "pest", "pest_meta", "proc-macro2", "quote", - "syn 2.0.29", + "syn 2.0.36", ] [[package]] name = "pest_meta" -version = "2.7.2" +version = "2.7.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "56af0a30af74d0445c0bf6d9d051c979b516a1a5af790d251daee76005420a48" +checksum = "b42f0394d3123e33353ca5e1e89092e533d2cc490389f2bd6131c43c634ebc5f" dependencies = [ "once_cell", "pest", @@ -949,7 +956,7 @@ checksum = "4359fd9c9171ec6e8c62926d6faaf553a8dc3f64e1507e76da7911b4f6a04405" dependencies = [ "proc-macro2", "quote", - "syn 2.0.29", + "syn 2.0.36", ] [[package]] @@ -999,9 +1006,9 @@ checksum = "5b40af805b3121feab8a3c29f04d8ad262fa8e0561883e7653e024ae4479e6de" [[package]] name = "proc-macro2" -version = "1.0.66" +version = "1.0.67" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "18fb31db3f9bddb2ea821cde30a9f70117e3f119938b5ee630b7403aa6e2ead9" +checksum = "3d433d9f1a3e8c1263d9456598b16fec66f4acc9a74dacffd35c7bb09b3a1328" dependencies = [ "unicode-ident", ] @@ -1116,9 +1123,9 @@ checksum = "d626bb9dae77e28219937af045c257c28bfd3f69333c512553507f5f9798cb76" [[package]] name = "rustix" -version = "0.38.10" +version = "0.38.13" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ed6248e1caa625eb708e266e06159f135e8c26f2bb7ceb72dc4b2766d0340964" +checksum = "d7db8590df6dfcd144d22afd1b83b36c21a18d7cbc1dc4bb5295a8712e9eb662" dependencies = [ "bitflags 2.4.0", "errno", @@ -1144,14 +1151,14 @@ version = "1.0.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2d3987094b1d07b653b7dfdc3f70ce9a1da9c51ac18c1b06b662e4f9a0e9f4b2" dependencies = [ - "base64 0.21.3", + "base64 0.21.4", ] [[package]] name = "rustls-webpki" -version = "0.101.4" +version = "0.101.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7d93931baf2d282fff8d3a532bbfd7653f734643161b87e3e01e59a04439bf0d" +checksum = "45a27e3b59326c16e23d30aeb7a36a24cc0d29e71d68ff611cdfb4a01d013bed" dependencies = [ "ring", "untrusted", @@ -1196,14 +1203,14 @@ checksum = "4eca7ac642d82aa35b60049a6eccb4be6be75e599bd2e9adb5f875a737654af2" dependencies = [ "proc-macro2", "quote", - "syn 2.0.29", + "syn 2.0.36", ] [[package]] name = "serde_json" -version = "1.0.105" +version = "1.0.107" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "693151e1ac27563d6dbcec9dee9fbd5da8539b20fa14ad3752b2e6d363ace360" +checksum = "6b420ce6e3d8bd882e9b243c6eed35dbc9a6110c9769e74b584e0d68d1f20c65" dependencies = [ "itoa", "ryu", @@ -1280,9 +1287,9 @@ checksum = "62bb4feee49fdd9f707ef802e22365a35de4b7b299de4763d44bfea899442ff9" [[package]] name = "socket2" -version = "0.5.3" +version = "0.5.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2538b18701741680e0322a2302176d3253a35388e2e62f172f64f4f16605f877" +checksum = "4031e820eb552adee9295814c0ced9e5cf38ddf1e8b7d566d6de8e2538ea989e" dependencies = [ "libc", "windows-sys 0.48.0", @@ -1315,9 +1322,9 @@ dependencies = [ [[package]] name = "sqlformat" -version = "0.2.1" +version = "0.2.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0c12bc9199d1db8234678b7051747c07f517cdcf019262d1847b94ec8b1aee3e" +checksum = "6b7b278788e7be4d0d29c0f39497a0eef3fba6bbc8e70d8bf7fde46edeaa9e85" dependencies = [ "itertools", "nom", @@ -1451,7 +1458,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8ca69bf415b93b60b80dc8fda3cb4ef52b2336614d8da2de5456cc942a110482" dependencies = [ "atoi", - "base64 0.21.3", + "base64 0.21.4", "bitflags 2.4.0", "byteorder", "bytes", @@ -1495,7 +1502,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a0db2df1b8731c3651e204629dd55e52adbae0462fa1bdcbed56a2302c18181e" dependencies = [ "atoi", - "base64 0.21.3", + "base64 0.21.4", "bitflags 2.4.0", "byteorder", "crc", @@ -1555,10 +1562,11 @@ dependencies = [ [[package]] name = "stringprep" -version = "0.1.3" +version = "0.1.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "db3737bde7edce97102e0e2b15365bf7a20bfdb5f60f4f9e8d7004258a51a8da" +checksum = "bb41d74e231a107a1b4ee36bd1214b11285b77768d2e3824aedafa988fd36ee6" dependencies = [ + "finl_unicode", "unicode-bidi", "unicode-normalization", ] @@ -1588,9 +1596,9 @@ dependencies = [ [[package]] name = "syn" -version = "2.0.29" +version = "2.0.36" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c324c494eba9d92503e6f1ef2e6df781e78f6a7705a0202d9801b198807d518a" +checksum = "91e02e55d62894af2a08aca894c6577281f76769ba47c94d5756bec8ac6e7373" dependencies = [ "proc-macro2", "quote", @@ -1612,22 +1620,22 @@ dependencies = [ [[package]] name = "thiserror" -version = "1.0.47" +version = "1.0.48" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "97a802ec30afc17eee47b2855fc72e0c4cd62be9b4efe6591edde0ec5bd68d8f" +checksum = "9d6d7a740b8a666a7e828dd00da9c0dc290dff53154ea77ac109281de90589b7" dependencies = [ "thiserror-impl", ] [[package]] name = "thiserror-impl" -version = "1.0.47" +version = "1.0.48" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6bb623b56e39ab7dcd4b1b98bb6c8f8d907ed255b18de254088016b27a8ee19b" +checksum = "49922ecae66cc8a249b77e68d1d0623c1b2c514f0060c27cdc68bd62a1219d35" dependencies = [ "proc-macro2", "quote", - "syn 2.0.29", + "syn 2.0.36", ] [[package]] @@ -1720,7 +1728,7 @@ checksum = "630bdcf245f78637c13ec01ffae6187cca34625e8c63150d424b59e55af2675e" dependencies = [ "proc-macro2", "quote", - "syn 2.0.29", + "syn 2.0.36", ] [[package]] @@ -1755,7 +1763,7 @@ checksum = "5f4f31f56159e98206da9efd823404b79b6ef3143b4a7ab76e67b1751b25a4ab" dependencies = [ "proc-macro2", "quote", - "syn 2.0.29", + "syn 2.0.36", ] [[package]] @@ -1769,9 +1777,9 @@ dependencies = [ [[package]] name = "typenum" -version = "1.16.0" +version = "1.17.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "497961ef93d974e23eb6f433eb5fe1b7930b659f06d12dec6fc44a8f554c0bba" +checksum = "42ff0bf0c66b8238c6f3b578df37d0b7848e55df8577b3f74f92a69acceeb825" [[package]] name = "ucd-trie" @@ -1787,9 +1795,9 @@ checksum = "92888ba5573ff080736b3648696b70cafad7d250551175acbaa4e0385b3e1460" [[package]] name = "unicode-ident" -version = "1.0.11" +version = "1.0.12" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "301abaae475aa91687eb82514b328ab47a211a533026cb25fc3e519b86adfc3c" +checksum = "3354b9ac3fae1ff6755cb6db53683adb661634f67557942dea4facebec0fee4b" [[package]] name = "unicode-normalization" @@ -1878,7 +1886,7 @@ dependencies = [ "once_cell", "proc-macro2", "quote", - "syn 2.0.29", + "syn 2.0.36", "wasm-bindgen-shared", ] @@ -1900,7 +1908,7 @@ checksum = "54681b18a46765f095758388f2d0cf16eb8d4169b639ab575a8f5693af210c7b" dependencies = [ "proc-macro2", "quote", - "syn 2.0.29", + "syn 2.0.36", "wasm-bindgen-backend", "wasm-bindgen-shared", ] diff --git a/crates/db/src/models/change_operation.rs b/crates/db/src/models/change_operation.rs index 51f91cf..6a92a49 100644 --- a/crates/db/src/models/change_operation.rs +++ b/crates/db/src/models/change_operation.rs @@ -5,7 +5,7 @@ use time::PrimitiveDateTime; use super::{DatabaseError, PlaylistChange, PlaylistEntry, SrcIdOwned}; /// Change operation stored in the database -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "SCREAMING_SNAKE_CASE", tag = "typ")] pub enum ChangeOperation { /// Insert @@ -99,18 +99,28 @@ impl ChangeOperation { } } -impl ToString for ChangeOperation { - fn to_string(&self) -> String { +impl std::fmt::Display for ChangeOperation { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { ChangeOperation::Ins { pos, val } => { - let mut s = format!("INS {pos}: "); - for v in val { - s += &v.to_string(); + f.write_fmt(format_args!("INS {pos}: "))?; + let show_max = 5; + + for i in 0..(val.len().min(show_max)) { + if i != 0 { + f.write_str(", ")?; + } + val[i].fmt(f)?; } - s + if val.len() > show_max { + f.write_fmt(format_args!(" (+{})", val.len() - show_max))?; + } + Ok(()) + } + ChangeOperation::Del { pos, n } => f.write_fmt(format_args!("DEL {pos} ({n})")), + ChangeOperation::Mov { pos, n, to } => { + f.write_fmt(format_args!("MOV {pos} ({n}) -> {to}")) } - ChangeOperation::Del { pos, n } => format!("DEL {pos} ({n})"), - ChangeOperation::Mov { pos, n, to } => format!("MOV {pos} ({n}) -> {to}"), } } } @@ -122,7 +132,7 @@ mod tests { use super::*; #[test] - pub fn to_json() { + fn to_json() { let ins = ChangeOperation::Ins { pos: 1, val: vec![SrcIdOwned("Zeerrnuli5E".to_owned(), MusicService::YouTube)], @@ -138,14 +148,33 @@ mod tests { r#"{"typ":"DEL","pos":1,"n":2}"# ); - let del = ChangeOperation::Mov { + let mov = ChangeOperation::Mov { pos: 1, n: 2, to: 3, }; assert_eq!( - serde_json::to_string(&del).unwrap(), + serde_json::to_string(&mov).unwrap(), r#"{"typ":"MOV","pos":1,"n":2,"to":3}"# ); } + + #[test] + fn to_string() { + let ins = ChangeOperation::Ins { + pos: 1, + val: vec![SrcIdOwned("Zeerrnuli5E".to_owned(), MusicService::YouTube)], + }; + assert_eq!(ins.to_string(), r#"INS 1: yt:Zeerrnuli5E"#); + + let del = ChangeOperation::Del { pos: 1, n: 2 }; + assert_eq!(del.to_string(), r#"DEL 1 (2)"#); + + let mov = ChangeOperation::Mov { + pos: 1, + n: 2, + to: 3, + }; + assert_eq!(mov.to_string(), r#"MOV 1 (2) -> 3"#); + } } diff --git a/crates/db/src/models/mod.rs b/crates/db/src/models/mod.rs index 8d002c3..09189ac 100644 --- a/crates/db/src/models/mod.rs +++ b/crates/db/src/models/mod.rs @@ -35,6 +35,8 @@ pub enum DatabaseError { Json(#[from] serde_json::Error), #[error("Item {0} not found")] NotFound(IdOwned), + #[error("Conflict while inserting {typ}: ID {id}")] + Conflict { typ: &'static str, id: String }, #[error("Playlist VCS error: {0}")] PlaylistVcs(Cow<'static, str>), #[error("DB error: {0}")] diff --git a/crates/db/src/models/playlist.rs b/crates/db/src/models/playlist.rs index 0b40eb0..2148aca 100644 --- a/crates/db/src/models/playlist.rs +++ b/crates/db/src/models/playlist.rs @@ -28,11 +28,10 @@ pub struct Playlist { pub updated_at: PrimitiveDateTime, pub last_sync_at: Option, pub last_sync_data: Option>, - pub cache_version: Option, - pub cache_data: Option>>, } /// Data for creating a playlist +#[derive(Default)] pub struct PlaylistNew { pub src_id: Option, pub service: Option, @@ -110,8 +109,7 @@ impl Playlist { Self, r#"select id, src_id, service as "service: _", name, description, owner_name, owner_url, playlist_type as "playlist_type: _", image_url, image_hash, - image_type as "image_type: _", created_at, updated_at, last_sync_at, last_sync_data as "last_sync_data: _", - cache_version, cache_data as "cache_data: _" + image_type as "image_type: _", created_at, updated_at, last_sync_at, last_sync_data as "last_sync_data: _" from playlists where id=$1"#, id ) @@ -123,8 +121,7 @@ from playlists where id=$1"#, Self, r#"select id, src_id, service as "service: _", name, description, owner_name, owner_url, playlist_type as "playlist_type: _", image_url, image_hash, - image_type as "image_type: _", created_at, updated_at, last_sync_at, last_sync_data as "last_sync_data: _", - cache_version, cache_data as "cache_data: _" + image_type as "image_type: _", created_at, updated_at, last_sync_at, last_sync_data as "last_sync_data: _" from playlists where src_id=$1 and service=$2"#, src_id, srv as MusicService @@ -210,6 +207,23 @@ from playlists where src_id=$1 and service=$2"#, Ok(()) } + /// Get the cached playlist entries for the given version if available + async fn get_cache<'a, E>( + id: i32, + exec: E, + ) -> Result)>, DatabaseError> + where + E: sqlx::Executor<'a, Database = sqlx::Postgres>, + { + let res = sqlx::query!( + r#"select cache_version, cache_data as "cache_data: Json>" from playlists where id=$1"#, + id, + ).fetch_optional(exec).await?; + + Ok(res.and_then(|res| res.cache_version.zip(res.cache_data.map(|d| d.0)))) + } + + /// Update the cached playlist entries async fn set_cache<'a, E>( id: i32, cache_version: Uuid, @@ -289,17 +303,17 @@ order by pc.created_at"#, /// Get a list of playlist entries from the current revision /// /// Merges heads and updates the cache if necessary. - pub async fn get_entries<'a, E>(&self, exec: E) -> Result, DatabaseError> + pub async fn get_entries<'a, E>(id: i32, exec: E) -> Result, DatabaseError> where E: sqlx::Executor<'a, Database = sqlx::Postgres> + Copy, { - if let Some(head) = Self::get_single_head(self.id, exec).await? { + if let Some(head) = Self::get_single_head(id, exec).await? { // Get list of items and changes to apply - let (mut items, changes) = if let (Some(cache_version), Some(Json(cache_data))) = - (self.cache_version, self.cache_data.clone()) - { + let cache_data = Self::get_cache(id, exec).await?; + + let (mut items, changes) = if let Some((cache_version, entries)) = cache_data { ( - cache_data, + entries, if head != cache_version { PlaylistChange::get_changes(Some(cache_version), head, exec).await? } else { @@ -320,7 +334,7 @@ order by pc.created_at"#, } // Update cache - Self::set_cache(self.id, head, &items, exec).await?; + Self::set_cache(id, head, &items, exec).await?; } Ok(items) @@ -347,11 +361,14 @@ order by pc.created_at"#, Ok(items) } - pub async fn get_tracks<'a, E>(&self, exec: E) -> Result, DatabaseError> + pub async fn get_tracks<'a, E>( + id: i32, + exec: E, + ) -> Result, DatabaseError> where E: sqlx::Executor<'a, Database = sqlx::Postgres> + Copy, { - let entries = self.get_entries(exec).await?; + let entries = Self::get_entries(id, exec).await?; Self::tracks_from_entries(&entries, exec).await } @@ -765,14 +782,18 @@ mod tests { async fn get_tracks() { testutil::run_sql("base.sql", &pool).await; - let pl = Playlist::get(ids::PLAYLIST_TEST1, &pool).await.unwrap(); - let tracks = pl.get_tracks(&pool).await.unwrap(); + let tracks = Playlist::get_tracks(ids::PLAYLIST_ID_TEST1, &pool) + .await + .unwrap(); insta::assert_ron_snapshot!("get_tracks", tracks); // Check cache - let pl = Playlist::get(ids::PLAYLIST_TEST1, &pool).await.unwrap(); - assert_eq!(pl.cache_version.unwrap(), ids::PLAYLIST_CHANGE_HEAD); - for (i, entry) in pl.cache_data.unwrap().0.iter().enumerate() { + let (cache_version, cache_entries) = Playlist::get_cache(ids::PLAYLIST_ID_TEST1, &pool) + .await + .unwrap() + .unwrap(); + assert_eq!(cache_version, ids::PLAYLIST_CHANGE_HEAD); + for (i, entry) in cache_entries.iter().enumerate() { let t = &tracks[i]; let track = t.track.as_ref().unwrap(); assert_eq!(t.n, u32::try_from(i).unwrap()); @@ -819,9 +840,9 @@ mod tests { .unwrap(); tx.commit().await.unwrap(); - let pl = Playlist::get(ids::PLAYLIST_TEST1, &pool).await.unwrap(); - let entries = pl.get_entries(&pool).await.unwrap(); - + let entries = Playlist::get_entries(ids::PLAYLIST_ID_TEST1, &pool) + .await + .unwrap(); assert!( testutil::compare_pl_entries( &entries, @@ -829,7 +850,7 @@ mod tests { SrcId("WSBUeFdXiSs", MusicService::YouTube), SrcId("OCgE2GSL1Pk", MusicService::YouTube), SrcId("6485PhOtHzY", MusicService::YouTube), - SrcId("2txScm52-QI", MusicService::YouTube) + SrcId("2txScm52-QI", MusicService::YouTube), ], ), "got: {entries:#?}" @@ -865,19 +886,68 @@ mod tests { .unwrap(); tx.commit().await.unwrap(); - let pl = Playlist::get(ids::PLAYLIST_TEST1, &pool).await.unwrap(); - let entries = pl.get_entries(&pool).await.unwrap(); - + let entries = Playlist::get_entries(ids::PLAYLIST_ID_TEST1, &pool) + .await + .unwrap(); assert!( testutil::compare_pl_entries( &entries, &[ SrcId("WSBUeFdXiSs", MusicService::YouTube), SrcId("6485PhOtHzY", MusicService::YouTube), - SrcId("2txScm52-QI", MusicService::YouTube) + SrcId("2txScm52-QI", MusicService::YouTube), ], ), "got: {entries:#?}" ); } + + /// Add a change to the playlist and run the merge operation concurrently to make + /// sure it does not create any ghost revisions. + #[sqlx_database_tester::test(pool(variable = "pool"))] + async fn merge_concurrent() { + testutil::run_sql("base.sql", &pool).await; + + let mut tx = pool.begin().await.unwrap(); + Playlist::add_change( + Some(ids::PLAYLIST_CHANGE_I3), + ids::PLAYLIST_ID_TEST1, + vec![ChangeOperation::Ins { + pos: 3, + val: vec![SrcIdOwned("2txScm52-QI".to_owned(), MusicService::YouTube)], + }], + &mut tx, + ) + .await + .unwrap(); + tx.commit().await.unwrap(); + + stream::iter(0..8) + .for_each_concurrent(8, |_| { + let pool = pool.clone(); + async move { + let entries = Playlist::get_entries(ids::PLAYLIST_ID_TEST1, &pool) + .await + .unwrap(); + assert!( + testutil::compare_pl_entries( + &entries, + &[ + SrcId("WSBUeFdXiSs", MusicService::YouTube), + SrcId("OCgE2GSL1Pk", MusicService::YouTube), + SrcId("6485PhOtHzY", MusicService::YouTube), + SrcId("2txScm52-QI", MusicService::YouTube), + ], + ), + "got: {entries:#?}" + ); + } + }) + .await; + + let revs = PlaylistChange::get_all_changes(ids::PLAYLIST_ID_TEST1, &pool) + .await + .unwrap(); + assert_eq!(revs.len(), 12); + } } diff --git a/crates/db/src/models/playlist_change.rs b/crates/db/src/models/playlist_change.rs index 3bfcd90..74167e1 100644 --- a/crates/db/src/models/playlist_change.rs +++ b/crates/db/src/models/playlist_change.rs @@ -347,12 +347,11 @@ values ($1,$2,$3,$4,$5,$6) returning id"#, pub async fn insert_with_id<'a, 'b, E>(&'b self, id: Uuid, exec: E) -> Result<(), DatabaseError> where - E: sqlx::Executor<'a, Database = sqlx::Postgres>, + E: sqlx::Executor<'a, Database = sqlx::Postgres> + Copy, { - sqlx::query!( + let res = sqlx::query!( r#"insert into playlist_changes (id,seq,operations,playlist_id,parent1_id,parent2_id,created_at) -values ($1,$2,$3,$4,$5,$6,$7) -on conflict (id) do nothing"#, +values ($1,$2,$3,$4,$5,$6,$7)"#, id, self.seq, serde_json::to_value(&self.operations)?, @@ -360,16 +359,42 @@ on conflict (id) do nothing"#, self.parent1_id, self.parent2_id, self.created_at.unwrap_or_else(|| util::primitive_now()), - ).execute(exec).await?; + ).execute(exec).await; + + if let Err(e) = res { + match &e { + sqlx::Error::Database(dbe) => { + if dbe.is_unique_violation() { + let conflicting = PlaylistChange::get(id, exec).await?; + if self.seq != conflicting.seq + || self.playlist_id != conflicting.playlist_id + || self.operations != conflicting.operations.0 + || self.parent1_id != conflicting.parent1_id + || self.parent2_id != conflicting.parent2_id + { + return Err(DatabaseError::Conflict { + typ: "playlist_change", + id: id.to_string(), + }); + } + } else { + return Err(DatabaseError::Database(e)); + } + } + _ => return Err(DatabaseError::Database(e)), + } + } Ok(()) } } #[cfg(test)] mod tests { + use time::macros::datetime; + use super::*; use crate::{ - models::Playlist, + models::{MusicService, Playlist, SrcIdOwned}, testutil::{self, ids}, }; @@ -403,6 +428,81 @@ mod tests { insta::assert_ron_snapshot!(res); } + #[sqlx_database_tester::test(pool(variable = "pool"))] + async fn insert_change() { + testutil::run_sql("base.sql", &pool).await; + + let nc = PlaylistChangeNew { + seq: 7, + playlist_id: ids::PLAYLIST_ID_TEST1, + operations: vec![ChangeOperation::Ins { + pos: 1, + val: vec![SrcIdOwned("abcd".to_string(), MusicService::Tiraya)], + }], + parent1_id: Some(ids::PLAYLIST_CHANGE_HEAD), + parent2_id: None, + ..Default::default() + }; + let id = nc.insert(&pool).await.unwrap(); + + let got = PlaylistChange::get(id, &pool).await.unwrap(); + assert_eq!(got.id, id); + assert_eq!(got.seq, nc.seq); + assert_eq!(got.operations.0, nc.operations); + assert_eq!(got.parent1_id, nc.parent1_id); + assert_eq!(got.parent2_id, nc.parent2_id); + } + + /// Trying to insert changes with same ID and content as already existing ones + /// should result in no action + #[sqlx_database_tester::test(pool(variable = "pool"))] + async fn insert_with_id_duplicate() { + testutil::run_sql("base.sql", &pool).await; + + let nc = PlaylistChangeNew { + seq: 6, + playlist_id: ids::PLAYLIST_ID_TEST1, + operations: Vec::new(), + parent1_id: Some(ids::PLAYLIST_CHANGE_BR1T), + parent2_id: Some(ids::PLAYLIST_CHANGE_BR2T), + ..Default::default() + }; + nc.insert_with_id(ids::PLAYLIST_CHANGE_HEAD, &pool) + .await + .unwrap(); + + let change = PlaylistChange::get(ids::PLAYLIST_CHANGE_HEAD, &pool) + .await + .unwrap(); + assert_eq!(change.created_at, datetime!(2023-09-06 23:50:59.397384)); + } + + /// Trying to insert changes with same ID and different content as already existing ones + /// should result in an error + #[sqlx_database_tester::test(pool(variable = "pool"))] + async fn insert_with_id_conflict() { + testutil::run_sql("base.sql", &pool).await; + + let nc = PlaylistChangeNew { + seq: 6, + playlist_id: ids::PLAYLIST_ID_TEST1, + operations: Vec::new(), + parent1_id: Some(ids::PLAYLIST_CHANGE_BR1T), + parent2_id: None, + ..Default::default() + }; + let err = nc + .insert_with_id(ids::PLAYLIST_CHANGE_HEAD, &pool) + .await + .unwrap_err(); + if let DatabaseError::Conflict { typ, id } = err { + assert_eq!(typ, "playlist_change"); + assert_eq!(id, ids::PLAYLIST_CHANGE_HEAD.to_string()); + } else { + panic!("unexpected err: {err}") + } + } + #[sqlx_database_tester::test(pool(variable = "pool"))] async fn paths_to_lca() { testutil::run_sql("base.sql", &pool).await; diff --git a/crates/db/src/models/snapshots/tiraya_db__models__playlist__tests__crud_empty.snap b/crates/db/src/models/snapshots/tiraya_db__models__playlist__tests__crud_empty.snap index c776b18..859c4de 100644 --- a/crates/db/src/models/snapshots/tiraya_db__models__playlist__tests__crud_empty.snap +++ b/crates/db/src/models/snapshots/tiraya_db__models__playlist__tests__crud_empty.snap @@ -18,6 +18,4 @@ Playlist( updated_at: "[date]", last_sync_at: None, last_sync_data: None, - cache_version: None, - cache_data: None, ) diff --git a/crates/db/src/models/snapshots/tiraya_db__models__playlist__tests__crud_inserted.snap b/crates/db/src/models/snapshots/tiraya_db__models__playlist__tests__crud_inserted.snap index e712139..7d9cb4f 100644 --- a/crates/db/src/models/snapshots/tiraya_db__models__playlist__tests__crud_inserted.snap +++ b/crates/db/src/models/snapshots/tiraya_db__models__playlist__tests__crud_inserted.snap @@ -18,6 +18,4 @@ Playlist( updated_at: "[date]", last_sync_at: None, last_sync_data: None, - cache_version: None, - cache_data: None, ) diff --git a/crates/db/src/models/types.rs b/crates/db/src/models/types.rs index 1489599..6eda648 100644 --- a/crates/db/src/models/types.rs +++ b/crates/db/src/models/types.rs @@ -39,11 +39,12 @@ pub enum DatePrecision { Year, } -#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash, Serialize, Deserialize, sqlx::Type)] +#[derive(Default, Debug, Copy, Clone, PartialEq, Eq, Hash, Serialize, Deserialize, sqlx::Type)] #[serde(rename_all = "snake_case")] #[sqlx(type_name = "playlist_type", rename_all = "snake_case")] pub enum PlaylistType { /// Local playlist created by a Tiraya user + #[default] Local, /// Remote playlist hosted by another service Remote, From 520949c07ade79a764dae2dbfb8091839a5dc121 Mon Sep 17 00:00:00 2001 From: ThetaDev Date: Mon, 18 Sep 2023 00:03:14 +0200 Subject: [PATCH 3/3] feat: add support for moving multiple tracks --- crates/db/src/models/change_operation.rs | 255 +++++++++++++++++++---- crates/db/src/models/mod.rs | 6 - crates/db/src/models/playlist.rs | 5 +- crates/db/src/models/playlist_change.rs | 14 +- 4 files changed, 217 insertions(+), 63 deletions(-) diff --git a/crates/db/src/models/change_operation.rs b/crates/db/src/models/change_operation.rs index 6a92a49..b435c42 100644 --- a/crates/db/src/models/change_operation.rs +++ b/crates/db/src/models/change_operation.rs @@ -9,50 +9,41 @@ use super::{DatabaseError, PlaylistChange, PlaylistEntry, SrcIdOwned}; #[serde(rename_all = "SCREAMING_SNAKE_CASE", tag = "typ")] pub enum ChangeOperation { /// Insert - Ins { pos: u32, val: Vec }, + Ins { pos: usize, val: Vec }, /// Delete - Del { pos: u32, n: u32 }, + Del { pos: usize, n: usize }, /// Move - Mov { pos: u32, n: u32, to: u32 }, + Mov { pos: usize, n: usize, to: usize }, } impl ChangeOperation { - pub fn to_op(&self, dt: PrimitiveDateTime) -> Result, DatabaseError> { + pub fn into_op(self, dt: PrimitiveDateTime) -> Result, DatabaseError> { match self { ChangeOperation::Ins { pos, val } => Ok(Operation::Ins { - pos: (*pos).try_into()?, - val: val - .iter() - .map(|id| PlaylistEntry { - id: id.to_owned(), - dt, - }) - .collect(), - }), - ChangeOperation::Del { pos, n } => Ok(Operation::Del { - pos: (*pos).try_into()?, - n: (*n).try_into()?, + pos, + val: val.into_iter().map(|id| PlaylistEntry { id, dt }).collect(), }), + ChangeOperation::Del { pos, n } => Ok(Operation::Del { pos, n }), ChangeOperation::Mov { pos, n, to } => { - if *n == 1 { - Ok(Operation::Mov { - pos: (*pos).try_into()?, - to: (*to).try_into()?, - }) + if n == 1 { + Ok(Operation::Mov { pos, to }) } else { - /* - let start = usize::try_from(pos)?; - let end = usize::try_from(pos + n)?; - let to = usize::try_from(to)?; + let end = pos + n; + Ok(Operation::Seq { - ops: (start..end) - .rev() - .enumerate() - .map(|(i, pos)| Operation::Mov { pos, to }) - .collect(), + ops: if to > pos { + // Move down + (pos..end) + .map(|pos| Operation::Mov { pos, to: to + n }) + .collect() + } else { + // Move up + (pos..end) + .rev() + .map(|pos| Operation::Mov { pos, to }) + .collect() + }, }) - */ - todo!("figure out moving ranges") } } } @@ -64,7 +55,9 @@ impl ChangeOperation { let changes = changes.into_iter(); let mut ops = Vec::with_capacity(changes.size_hint().0); for c in changes { - ops.append(&mut c.operations()?); + for cop in &c.operations.0 { + ops.push(cop.clone().into_op(c.created_at)?); + } } Ok(Operation::Seq { ops }) } @@ -76,18 +69,11 @@ impl ChangeOperation { let changeop = match op { Operation::Nop => return Ok(()), Operation::Ins { pos, val } => ChangeOperation::Ins { - pos: pos.try_into()?, + pos, val: val.into_iter().map(|itm| itm.id).collect(), }, - Operation::Del { pos, n } => ChangeOperation::Del { - pos: pos.try_into()?, - n: n.try_into()?, - }, - Operation::Mov { pos, to } => ChangeOperation::Mov { - pos: pos.try_into()?, - n: 1, - to: to.try_into()?, - }, + Operation::Del { pos, n } => ChangeOperation::Del { pos, n }, + Operation::Mov { pos, to } => ChangeOperation::Mov { pos, n: 1, to }, Operation::Seq { ops } => { return ops .into_iter() @@ -97,6 +83,46 @@ impl ChangeOperation { list.push(changeop); Ok(()) } + + pub fn apply( + self, + items: &mut Vec, + dt: PrimitiveDateTime, + ) -> Result<(), DatabaseError> { + let assert_pos = |pos: usize| { + if pos > items.len() { + Err(DatabaseError::PlaylistVcs( + format!("index {} out of range (len: {})", pos, items.len()).into(), + )) + } else { + Ok(()) + } + }; + + match self { + ChangeOperation::Ins { pos, val } => { + let new_items = val.into_iter().map(|id| PlaylistEntry { id, dt }); + if pos < items.len() { + items.splice(pos..pos, new_items); + } else { + items.extend(new_items); + } + } + ChangeOperation::Del { pos, n } => { + let end = pos + n; + assert_pos(end)?; + items.splice(pos..end, None); + } + ChangeOperation::Mov { pos, n, to } => { + let end = pos + n; + assert_pos(end)?; + assert_pos(to)?; + let moved = items.splice(pos..end, None).collect::>(); + items.splice(to..to, moved); + } + } + Ok(()) + } } impl std::fmt::Display for ChangeOperation { @@ -127,6 +153,8 @@ impl std::fmt::Display for ChangeOperation { #[cfg(test)] mod tests { + use time::macros::datetime; + use crate::models::MusicService; use super::*; @@ -177,4 +205,145 @@ mod tests { }; assert_eq!(mov.to_string(), r#"MOV 1 (2) -> 3"#); } + + const TESTDATE: time::PrimitiveDateTime = datetime!(2023-09-06 8:00:00); + + #[test] + fn apply_ins() { + let mut entries = vec![ + PlaylistEntry { + id: SrcIdOwned("1".to_owned(), MusicService::Tiraya), + dt: TESTDATE, + }, + PlaylistEntry { + id: SrcIdOwned("4".to_owned(), MusicService::Tiraya), + dt: TESTDATE, + }, + ]; + let ins = ChangeOperation::Ins { + pos: 1, + val: vec![ + SrcIdOwned("2".to_owned(), MusicService::Tiraya), + SrcIdOwned("3".to_owned(), MusicService::Tiraya), + ], + }; + ins.apply(&mut entries, TESTDATE).unwrap(); + + assert_eq!(entries.len(), 4); + for (i, e) in entries.iter().enumerate() { + assert_eq!(e.id.0, (i + 1).to_string()); + } + } + + #[test] + fn apply_del() { + let mut entries = vec![ + PlaylistEntry { + id: SrcIdOwned("1".to_owned(), MusicService::Tiraya), + dt: TESTDATE, + }, + PlaylistEntry { + id: SrcIdOwned("2".to_owned(), MusicService::Tiraya), + dt: TESTDATE, + }, + PlaylistEntry { + id: SrcIdOwned("X".to_owned(), MusicService::Tiraya), + dt: TESTDATE, + }, + PlaylistEntry { + id: SrcIdOwned("X".to_owned(), MusicService::Tiraya), + dt: TESTDATE, + }, + ]; + let del = ChangeOperation::Del { pos: 2, n: 2 }; + del.apply(&mut entries, TESTDATE).unwrap(); + + assert_eq!(entries.len(), 2); + for (i, e) in entries.iter().enumerate() { + assert_eq!(e.id.0, (i + 1).to_string()); + } + } + + #[test] + fn apply_mov_up() { + let mut entries = vec![ + PlaylistEntry { + id: SrcIdOwned("3".to_owned(), MusicService::Tiraya), + dt: TESTDATE, + }, + PlaylistEntry { + id: SrcIdOwned("4".to_owned(), MusicService::Tiraya), + dt: TESTDATE, + }, + PlaylistEntry { + id: SrcIdOwned("1".to_owned(), MusicService::Tiraya), + dt: TESTDATE, + }, + PlaylistEntry { + id: SrcIdOwned("2".to_owned(), MusicService::Tiraya), + dt: TESTDATE, + }, + ]; + let mov = ChangeOperation::Mov { + pos: 2, + n: 2, + to: 0, + }; + mov.apply(&mut entries, TESTDATE).unwrap(); + + assert_eq!(entries.len(), 4); + for (i, e) in entries.iter().enumerate() { + assert_eq!(e.id.0, (i + 1).to_string()); + } + } + + #[test] + fn apply_mov_down() { + let mut entries = vec![ + PlaylistEntry { + id: SrcIdOwned("3".to_owned(), MusicService::Tiraya), + dt: TESTDATE, + }, + PlaylistEntry { + id: SrcIdOwned("4".to_owned(), MusicService::Tiraya), + dt: TESTDATE, + }, + PlaylistEntry { + id: SrcIdOwned("1".to_owned(), MusicService::Tiraya), + dt: TESTDATE, + }, + PlaylistEntry { + id: SrcIdOwned("2".to_owned(), MusicService::Tiraya), + dt: TESTDATE, + }, + ]; + let mov = ChangeOperation::Mov { + pos: 0, + n: 2, + to: 2, + }; + mov.apply(&mut entries, TESTDATE).unwrap(); + + assert_eq!(entries.len(), 4); + for (i, e) in entries.iter().enumerate() { + assert_eq!(e.id.0, (i + 1).to_string()); + } + } + + #[test] + fn apply_del_out_of_range() { + let mut entries = vec![ + PlaylistEntry { + id: SrcIdOwned("1".to_owned(), MusicService::Tiraya), + dt: TESTDATE, + }, + PlaylistEntry { + id: SrcIdOwned("2".to_owned(), MusicService::Tiraya), + dt: TESTDATE, + }, + ]; + let del = ChangeOperation::Del { pos: 1, n: 2 }; + let err = del.apply(&mut entries, TESTDATE).unwrap_err(); + assert!(matches!(err, DatabaseError::PlaylistVcs(_))); + } } diff --git a/crates/db/src/models/mod.rs b/crates/db/src/models/mod.rs index 09189ac..464fb89 100644 --- a/crates/db/src/models/mod.rs +++ b/crates/db/src/models/mod.rs @@ -43,12 +43,6 @@ pub enum DatabaseError { Other(Cow<'static, str>), } -impl From for DatabaseError { - fn from(value: std::num::TryFromIntError) -> Self { - Self::Other(value.to_string().into()) - } -} - #[derive(Debug, Copy, Clone, PartialEq, Eq)] pub enum Id<'a> { Db(i32), diff --git a/crates/db/src/models/playlist.rs b/crates/db/src/models/playlist.rs index 2148aca..1c860b2 100644 --- a/crates/db/src/models/playlist.rs +++ b/crates/db/src/models/playlist.rs @@ -329,7 +329,7 @@ order by pc.created_at"#, if !changes.is_empty() { // Apply changes - for c in changes.iter().rev() { + for c in changes.into_iter().rev() { c.apply(&mut items)?; } @@ -354,10 +354,9 @@ order by pc.created_at"#, let changes = PlaylistChange::get_changes(None, head, exec).await?; // Apply changes - for c in changes.iter().rev() { + for c in changes.into_iter().rev() { c.apply(&mut items)?; } - Ok(items) } diff --git a/crates/db/src/models/playlist_change.rs b/crates/db/src/models/playlist_change.rs index 74167e1..b3b7e08 100644 --- a/crates/db/src/models/playlist_change.rs +++ b/crates/db/src/models/playlist_change.rs @@ -1,7 +1,6 @@ use std::fmt::Debug; use nonempty_collections::{nev, NonEmptyIterator}; -use otvec::Operation; use serde::Serialize; use sqlx::types::Json; use time::PrimitiveDateTime; @@ -117,16 +116,9 @@ parent1_id, parent2_id, created_at from playlist_changes where playlist_id=$1 or ).fetch_all(exec).await } - pub fn operations(&self) -> Result>, DatabaseError> { - self.operations - .iter() - .map(|op| op.to_op(self.created_at)) - .collect::, _>>() - } - - pub fn apply(&self, items: &mut Vec) -> Result<(), DatabaseError> { - for op in &self.operations.0 { - op.to_op(self.created_at)?.apply(items); + pub fn apply(self, items: &mut Vec) -> Result<(), DatabaseError> { + for op in self.operations.0 { + op.apply(items, self.created_at)?; } Ok(()) }