From c7a26910426d65be385dcafeae73265a66bbb52e Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Sat, 15 Nov 2025 08:38:06 -0500 Subject: [PATCH 01/18] Add inherited_from column to Files_Tags and Folders_Tags tables (#120) * Initial plan * Add v6 database migration for tag inheritance support Co-authored-by: ploiu <43047560+ploiu@users.noreply.github.com> * Use shorthand syntax for foreign key references in v6 migration Co-authored-by: ploiu <43047560+ploiu@users.noreply.github.com> * update file_tags database field to fileTags --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: ploiu <43047560+ploiu@users.noreply.github.com> Co-authored-by: ploiu --- src/assets/migration/v6.sql | 73 +++++++++++++++++++ src/db_migrations.rs | 141 ++++++++++++++++++++++++++++++++++++ 2 files changed, 214 insertions(+) create mode 100644 src/assets/migration/v6.sql diff --git a/src/assets/migration/v6.sql b/src/assets/migration/v6.sql new file mode 100644 index 0000000..91938ae --- /dev/null +++ b/src/assets/migration/v6.sql @@ -0,0 +1,73 @@ +begin; + +-- SQLite doesn't support adding columns with foreign keys via ALTER TABLE, +-- so we need to recreate the tables with the new schema +-- Create new Files_Tags table with inheritedfrom column +create table Files_Tags_new ( + fileRecordTagId integer primary key autoincrement, + fileRecordId integer not null references FileRecords (id) on delete cascade, + tagId integer not null references Tags (id) on delete cascade, + inheritedfrom integer references Folders (id) on delete cascade, + -- Unique constraint on fileRecordId and tagId effectively enforces uniqueness on tag title and file id + -- since Tags.title is unique and tagId maps 1:1 to title + unique (fileRecordId, tagId) +); + +-- Create new Folders_Tags table with inheritedfrom column +create table Folders_Tags_new ( + foldersTagId integer primary key autoincrement, + folderId integer not null references Folders (id) on delete cascade, + tagId integer not null references Tags (id) on delete cascade, + inheritedfrom integer references Folders (id) on delete cascade, + -- Unique constraint on folderId and tagId effectively enforces uniqueness on tag title and folder id + -- since Tags.title is unique and tagId maps 1:1 to title + unique (folderId, tagId) +); + +-- Copy data from old tables to new tables +insert into + Files_Tags_new( + fileRecordTagId, + fileRecordId, + tagId, + inheritedfrom + ) +select + fileRecordTagId, + fileRecordId, + tagId, + null +from + Files_Tags; + +insert into + Folders_Tags_new(foldersTagId, folderId, tagId, inheritedfrom) +select + foldersTagId, + folderId, + tagId, + null +from + Folders_Tags; + +-- Drop old tables +drop table Files_Tags; + +drop table Folders_Tags; + +-- Rename new tables to original names +alter table + Files_Tags_new rename to Files_Tags; + +alter table + Folders_Tags_new rename to Folders_Tags; + +-- Update version +update + Metadata +set + value = '6' +where + name = 'version'; + +commit; \ No newline at end of file diff --git a/src/db_migrations.rs b/src/db_migrations.rs index 65ccb38..af977a2 100644 --- a/src/db_migrations.rs +++ b/src/db_migrations.rs @@ -112,6 +112,10 @@ pub fn migrate_db(con: &Connection, table_version: u64) -> Result<()> { log_migration_version(5); migrate_v5(con)?; } + if table_version < 6 { + log_migration_version(6); + migrate_v6(con)?; + } Ok(()) } @@ -138,3 +142,140 @@ fn migrate_v4(con: &Connection) -> Result<()> { fn migrate_v5(con: &Connection) -> Result<()> { con.execute_batch(include_str!("./assets/migration/v5.sql")) } + +fn migrate_v6(con: &Connection) -> Result<()> { + con.execute_batch(include_str!("./assets/migration/v6.sql")) +} + +#[cfg(test)] +mod migration_tests { + use crate::repository::open_connection; + use crate::test::{cleanup, init_db_folder}; + use rusqlite::params; + + #[test] + fn v6_migration_adds_inherited_from_columns() { + init_db_folder(); + let con = open_connection(); + + // Create test data + let tag_id = con + .execute("insert into Tags(title) values (?1)", params!["test_tag"]) + .unwrap(); + let file_id = con + .execute( + "insert into FileRecords(name) values (?1)", + params!["test_file.txt"], + ) + .unwrap(); + let folder_id = con + .execute( + "insert into Folders(name) values (?1)", + params!["test_folder"], + ) + .unwrap(); + + // Add tag to file and folder + con.execute( + "insert into Files_Tags(fileRecordId, tagId) values (?1, ?2)", + params![file_id, tag_id], + ) + .unwrap(); + con.execute( + "insert into Folders_Tags(folderId, tagId) values (?1, ?2)", + params![folder_id, tag_id], + ) + .unwrap(); + + // Verify inheritedFrom column exists and is NULL by default + let file_inherited: Option = con + .query_row( + "select inheritedFrom from Files_Tags where fileRecordId = ?1", + params![file_id], + |row| row.get(0), + ) + .unwrap(); + assert_eq!(None, file_inherited); + + let folder_inherited: Option = con + .query_row( + "select inheritedFrom from Folders_Tags where folderId = ?1", + params![folder_id], + |row| row.get(0), + ) + .unwrap(); + assert_eq!(None, folder_inherited); + + // Verify we can set inheritedFrom + let parent_folder_id = con + .execute( + "insert into Folders(name) values (?1)", + params!["parent_folder"], + ) + .unwrap(); + con.execute( + "update Files_Tags set inheritedFrom = ?1 where fileRecordId = ?2", + params![parent_folder_id, file_id], + ) + .unwrap(); + con.execute( + "update Folders_Tags set inheritedFrom = ?1 where folderId = ?2", + params![parent_folder_id, folder_id], + ) + .unwrap(); + + let file_inherited: Option = con + .query_row( + "select inheritedFrom from Files_Tags where fileRecordId = ?1", + params![file_id], + |row| row.get(0), + ) + .unwrap(); + assert_eq!(Some(parent_folder_id as u32), file_inherited); + + let folder_inherited: Option = con + .query_row( + "select inheritedFrom from Folders_Tags where folderId = ?1", + params![folder_id], + |row| row.get(0), + ) + .unwrap(); + assert_eq!(Some(parent_folder_id as u32), folder_inherited); + + con.close().unwrap(); + cleanup(); + } + + #[test] + fn v6_migration_preserves_unique_constraint() { + init_db_folder(); + let con = open_connection(); + + // Create test data + con.execute("insert into Tags(title) values (?1)", params!["test_tag"]) + .unwrap(); + let file_id = con + .execute( + "insert into FileRecords(name) values (?1)", + params!["test_file.txt"], + ) + .unwrap(); + + // Add tag to file + con.execute( + "insert into Files_Tags(fileRecordId, tagId) values (?1, 1)", + params![file_id], + ) + .unwrap(); + + // Try to add the same tag again - should fail due to unique constraint + let result = con.execute( + "insert into Files_Tags(fileRecordId, tagId) values (?1, 1)", + params![file_id], + ); + assert!(result.is_err(), "Expected unique constraint violation"); + + con.close().unwrap(); + cleanup(); + } +} From 3529e65bfdd28e7a86216abc19827b4e7682f270 Mon Sep 17 00:00:00 2001 From: ploiu Date: Sat, 15 Nov 2025 14:17:13 +0000 Subject: [PATCH 02/18] Revert "Add inherited_from column to Files_Tags and Folders_Tags tables (#120)" This reverts commit c7a26910426d65be385dcafeae73265a66bbb52e. --- src/assets/migration/v6.sql | 73 ------------------- src/db_migrations.rs | 141 ------------------------------------ 2 files changed, 214 deletions(-) delete mode 100644 src/assets/migration/v6.sql diff --git a/src/assets/migration/v6.sql b/src/assets/migration/v6.sql deleted file mode 100644 index 91938ae..0000000 --- a/src/assets/migration/v6.sql +++ /dev/null @@ -1,73 +0,0 @@ -begin; - --- SQLite doesn't support adding columns with foreign keys via ALTER TABLE, --- so we need to recreate the tables with the new schema --- Create new Files_Tags table with inheritedfrom column -create table Files_Tags_new ( - fileRecordTagId integer primary key autoincrement, - fileRecordId integer not null references FileRecords (id) on delete cascade, - tagId integer not null references Tags (id) on delete cascade, - inheritedfrom integer references Folders (id) on delete cascade, - -- Unique constraint on fileRecordId and tagId effectively enforces uniqueness on tag title and file id - -- since Tags.title is unique and tagId maps 1:1 to title - unique (fileRecordId, tagId) -); - --- Create new Folders_Tags table with inheritedfrom column -create table Folders_Tags_new ( - foldersTagId integer primary key autoincrement, - folderId integer not null references Folders (id) on delete cascade, - tagId integer not null references Tags (id) on delete cascade, - inheritedfrom integer references Folders (id) on delete cascade, - -- Unique constraint on folderId and tagId effectively enforces uniqueness on tag title and folder id - -- since Tags.title is unique and tagId maps 1:1 to title - unique (folderId, tagId) -); - --- Copy data from old tables to new tables -insert into - Files_Tags_new( - fileRecordTagId, - fileRecordId, - tagId, - inheritedfrom - ) -select - fileRecordTagId, - fileRecordId, - tagId, - null -from - Files_Tags; - -insert into - Folders_Tags_new(foldersTagId, folderId, tagId, inheritedfrom) -select - foldersTagId, - folderId, - tagId, - null -from - Folders_Tags; - --- Drop old tables -drop table Files_Tags; - -drop table Folders_Tags; - --- Rename new tables to original names -alter table - Files_Tags_new rename to Files_Tags; - -alter table - Folders_Tags_new rename to Folders_Tags; - --- Update version -update - Metadata -set - value = '6' -where - name = 'version'; - -commit; \ No newline at end of file diff --git a/src/db_migrations.rs b/src/db_migrations.rs index af977a2..65ccb38 100644 --- a/src/db_migrations.rs +++ b/src/db_migrations.rs @@ -112,10 +112,6 @@ pub fn migrate_db(con: &Connection, table_version: u64) -> Result<()> { log_migration_version(5); migrate_v5(con)?; } - if table_version < 6 { - log_migration_version(6); - migrate_v6(con)?; - } Ok(()) } @@ -142,140 +138,3 @@ fn migrate_v4(con: &Connection) -> Result<()> { fn migrate_v5(con: &Connection) -> Result<()> { con.execute_batch(include_str!("./assets/migration/v5.sql")) } - -fn migrate_v6(con: &Connection) -> Result<()> { - con.execute_batch(include_str!("./assets/migration/v6.sql")) -} - -#[cfg(test)] -mod migration_tests { - use crate::repository::open_connection; - use crate::test::{cleanup, init_db_folder}; - use rusqlite::params; - - #[test] - fn v6_migration_adds_inherited_from_columns() { - init_db_folder(); - let con = open_connection(); - - // Create test data - let tag_id = con - .execute("insert into Tags(title) values (?1)", params!["test_tag"]) - .unwrap(); - let file_id = con - .execute( - "insert into FileRecords(name) values (?1)", - params!["test_file.txt"], - ) - .unwrap(); - let folder_id = con - .execute( - "insert into Folders(name) values (?1)", - params!["test_folder"], - ) - .unwrap(); - - // Add tag to file and folder - con.execute( - "insert into Files_Tags(fileRecordId, tagId) values (?1, ?2)", - params![file_id, tag_id], - ) - .unwrap(); - con.execute( - "insert into Folders_Tags(folderId, tagId) values (?1, ?2)", - params![folder_id, tag_id], - ) - .unwrap(); - - // Verify inheritedFrom column exists and is NULL by default - let file_inherited: Option = con - .query_row( - "select inheritedFrom from Files_Tags where fileRecordId = ?1", - params![file_id], - |row| row.get(0), - ) - .unwrap(); - assert_eq!(None, file_inherited); - - let folder_inherited: Option = con - .query_row( - "select inheritedFrom from Folders_Tags where folderId = ?1", - params![folder_id], - |row| row.get(0), - ) - .unwrap(); - assert_eq!(None, folder_inherited); - - // Verify we can set inheritedFrom - let parent_folder_id = con - .execute( - "insert into Folders(name) values (?1)", - params!["parent_folder"], - ) - .unwrap(); - con.execute( - "update Files_Tags set inheritedFrom = ?1 where fileRecordId = ?2", - params![parent_folder_id, file_id], - ) - .unwrap(); - con.execute( - "update Folders_Tags set inheritedFrom = ?1 where folderId = ?2", - params![parent_folder_id, folder_id], - ) - .unwrap(); - - let file_inherited: Option = con - .query_row( - "select inheritedFrom from Files_Tags where fileRecordId = ?1", - params![file_id], - |row| row.get(0), - ) - .unwrap(); - assert_eq!(Some(parent_folder_id as u32), file_inherited); - - let folder_inherited: Option = con - .query_row( - "select inheritedFrom from Folders_Tags where folderId = ?1", - params![folder_id], - |row| row.get(0), - ) - .unwrap(); - assert_eq!(Some(parent_folder_id as u32), folder_inherited); - - con.close().unwrap(); - cleanup(); - } - - #[test] - fn v6_migration_preserves_unique_constraint() { - init_db_folder(); - let con = open_connection(); - - // Create test data - con.execute("insert into Tags(title) values (?1)", params!["test_tag"]) - .unwrap(); - let file_id = con - .execute( - "insert into FileRecords(name) values (?1)", - params!["test_file.txt"], - ) - .unwrap(); - - // Add tag to file - con.execute( - "insert into Files_Tags(fileRecordId, tagId) values (?1, 1)", - params![file_id], - ) - .unwrap(); - - // Try to add the same tag again - should fail due to unique constraint - let result = con.execute( - "insert into Files_Tags(fileRecordId, tagId) values (?1, 1)", - params![file_id], - ); - assert!(result.is_err(), "Expected unique constraint violation"); - - con.close().unwrap(); - cleanup(); - } -} From 40fca71ec7cc9d1ebcdabe5759856fce966333a4 Mon Sep 17 00:00:00 2001 From: ploiu Date: Sat, 15 Nov 2025 15:05:55 +0000 Subject: [PATCH 03/18] direct tag migration step --- src/assets/migration/v6.sql | 45 +++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 src/assets/migration/v6.sql diff --git a/src/assets/migration/v6.sql b/src/assets/migration/v6.sql new file mode 100644 index 0000000..a2040bd --- /dev/null +++ b/src/assets/migration/v6.sql @@ -0,0 +1,45 @@ +-- create centralized tagged items table to simplify searching and tagging +begin; + +create table TaggedItems ( + id integer primary key, + tagId integer not null references Tags(id) on delete cascade, + fileId integer references FileRecords(id) on delete cascade, + folderId integer references Folders(id) on delete cascade, + -- items can only ever inherit tags from an ancestor folder. When this inherited folder is deleted, this tag should be removed too since it's no longer inherited + inheritedFromId integer references Folders(id) on delete cascade default null, + -- make sure that either a file or a folder was tagged + check ((fileId is not null) != (folderId is not null)) +); + +-- partial unique to prevent the same tag from being applied to a tagged item +create unique index idx_tagged_items_unique_file on TaggedItems(tagId, fileId) +where + fileId is not null; + +create unique index idx_tagged_items_unique_folder on TaggedItems(tagId, folderId) +where + folderId is not null; + +-- migrate all direct tags for files +insert into + TaggedItems(tagId, fileId) +select + tagId, + fileRecordId +from + Files_Tags; + +-- migrate all direct tags for folders +insert into + TaggedItems(tagId, folderId) +select + tagId, + folderId +from + Folders_Tags; + + +-- inherit all + +commit; \ No newline at end of file From 1506fc6e614313041cacc914670c1aef8ad7b6ac Mon Sep 17 00:00:00 2001 From: ploiu Date: Sat, 15 Nov 2025 16:36:01 +0000 Subject: [PATCH 04/18] start of flattening tags --- .vscode/settings.json | 4 +-- src/assets/migration/v6.sql | 55 ++++++++++++++++++++++++++++++++++++- src/db_migrations.rs | 8 ++++++ src/exif/handler.rs | 11 ++------ 4 files changed, 67 insertions(+), 11 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index 5a3d675..4e29738 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -1,8 +1,8 @@ { - "commentTranslate.hover.enabled": true, "chat.agent.enabled": true, "chat.commandCenter.enabled": false, "chat.notifyWindowOnConfirmation": false, "telemetry.feedback.enabled": false, - "deno.enable": false + "deno.enable": false, + "sql-formatter.uppercase": false, } diff --git a/src/assets/migration/v6.sql b/src/assets/migration/v6.sql index a2040bd..36baf25 100644 --- a/src/assets/migration/v6.sql +++ b/src/assets/migration/v6.sql @@ -39,7 +39,60 @@ select from Folders_Tags; +/* + populating inherited tags for folders (needs to be done first so that files work): + 1. recursively get all parent folders along with how far needed to be traveled for that parent folder (depth) + 2. get all tags for all parent folders + 3. for any duplicate tags, take only the ancestor id with the lowest depth (lower depth = higher specificity) + + if ai is helpful for anything, it's providing an example that I can adapt while I properly read how recursive sql queries work. + Previously, I was flailing around. It helps me if I think of it as a do while loop and temporary named queries / functions + */ +with recursive +-- traverse the ancestor tree and track depth +ancestors(folderId, ancestorId, depth) as ( + -- base case: select all folders that have a parent + select id as folderId, parentId as ancestorId, 1 as depth + from folders + where parentId is not null --- inherit all + union all + -- iteration: keep retrieving parents from base case until there are no more parents + select a.folderId, f.parentId as ancestorId, a.depth + 1 + from ancestors a + join folders f on f.id = a.ancestorId + where f.parentId is not null +), +-- include all tags with fetched ancestors +ancestorTags as ( + select a.folderId, ft.tagId, a.ancestorId, a.depth + from ancestors a + join folders_tags ft on ft.folderId = a.ancestorId +), +-- iterate through all retrieved ancestors. For each entry, find the tag on the ancestor with the lowest depth +nearestTags as ( + select at.folderId, at.tagId, at.ancestorId + from ancestorTags at + where at.ancestorId = ( + -- compare on the current row and find the nearest ancestor + select at2.ancestorId + from ancestorTags at2 + where at2.folderId = at.folderId + and at2.tagId = at.tagId + order by at2.depth asc + limit 1 + ) +) + +-- now that we have our functions, we can invoke nearestTags to get all the inherited tags and insert them +insert into TaggedItems(tagId, folderId, inheritedFromId) +select n.tagId, n.folderId, n.ancestorId +from nearestTags n +-- important to not include tags that are directly on the folder +where not exists ( + select 1 from TaggedItems ti where ti.tagId = n.tagId and ti.folderId = n.folderId +); + +update metadata set value = 6 where name = 'version'; commit; \ No newline at end of file diff --git a/src/db_migrations.rs b/src/db_migrations.rs index 65ccb38..e1c3abd 100644 --- a/src/db_migrations.rs +++ b/src/db_migrations.rs @@ -112,6 +112,10 @@ pub fn migrate_db(con: &Connection, table_version: u64) -> Result<()> { log_migration_version(5); migrate_v5(con)?; } + if table_version < 6 { + log_migration_version(6); + migrate_v6(con)?; + } Ok(()) } @@ -138,3 +142,7 @@ fn migrate_v4(con: &Connection) -> Result<()> { fn migrate_v5(con: &Connection) -> Result<()> { con.execute_batch(include_str!("./assets/migration/v5.sql")) } + +fn migrate_v6(con: &Connection) -> Result<()> { + con.execute_batch(include_str!("./assets/migration/v6.sql")) +} diff --git a/src/exif/handler.rs b/src/exif/handler.rs index f1b59c9..31f60ea 100644 --- a/src/exif/handler.rs +++ b/src/exif/handler.rs @@ -3,19 +3,14 @@ use std::{ time::Instant, }; -use rocket::{http::Status, State}; +use rocket::{State, http::Status}; use crate::{ - guard::HeaderAuth, - model::guard::auth::ValidateResult, - util::update_last_request_time, + guard::HeaderAuth, model::guard::auth::ValidateResult, util::update_last_request_time, }; #[get("/regen")] -pub fn regenerate_exif( - auth: HeaderAuth, - last_request_time: &State>>, -) -> Status { +pub fn regenerate_exif(auth: HeaderAuth, last_request_time: &State>>) -> Status { match auth.validate() { ValidateResult::Ok => { /*no op*/ } ValidateResult::NoPasswordSet => return Status::Unauthorized, From 7d72b56c3a880425826bf97a7235661f5234fe29 Mon Sep 17 00:00:00 2001 From: ploiu Date: Sat, 15 Nov 2025 17:07:44 +0000 Subject: [PATCH 05/18] v6 database migration --- src/assets/migration/v6.sql | 42 +++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/src/assets/migration/v6.sql b/src/assets/migration/v6.sql index 36baf25..cacd95d 100644 --- a/src/assets/migration/v6.sql +++ b/src/assets/migration/v6.sql @@ -94,5 +94,47 @@ where not exists ( select 1 from TaggedItems ti where ti.tagId = n.tagId and ti.folderId = n.folderId ); +-- populate inherited tags for files: for each file, walk its containing folder(s)' ancestor chain +-- and pick the nearest ancestor that provides a tag, then insert an inherited row for the file +with recursive +ancestors(fileId, directFolderId, ancestorId, depth) as ( + -- base: each file's direct containing folder is the first ancestor (so tags on the folder itself are inherited) + select ff.fileId, ff.folderId, ff.folderId as ancestorId, 1 as depth + from Folder_Files ff + + union all + + -- climb up the folder parent chain + select fa.fileId, fa.directFolderId, f.parentId as ancestorId, fa.depth + 1 + from ancestors fa + join Folders f on f.id = fa.ancestorId + where f.parentId is not null +), +-- join the discovered ancestors to tags present on those ancestor folders +ancestorTags as ( + select fa.fileId, ft.tagId, fa.ancestorId, fa.depth + from ancestors fa + join Folders_Tags ft on ft.folderId = fa.ancestorId +), +-- for each (file,tag) choose the nearest ancestor (smallest depth) +nearestTags as ( + select cft.fileId, cft.tagId, cft.ancestorId + from ancestorTags cft + where cft.ancestorId = ( + select cft2.ancestorId + from ancestorTags cft2 + where cft2.fileId = cft.fileId and cft2.tagId = cft.tagId + order by cft2.depth asc + limit 1 + ) +) + +insert into TaggedItems(tagId, fileId, inheritedFromId) +select n.tagId, n.fileId, n.ancestorId +from nearestTags n +where not exists ( + select 1 from TaggedItems ti where ti.tagId = n.tagId and ti.fileId = n.fileId +); + update metadata set value = 6 where name = 'version'; commit; \ No newline at end of file From 578755ae8563021b29082059a8bd4cfd816f2a48 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 15 Nov 2025 17:27:55 +0000 Subject: [PATCH 06/18] Initial plan From f358bfaa299b4d3e6afed1c0774bbe4d543caeb5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 15 Nov 2025 17:43:35 +0000 Subject: [PATCH 07/18] Move tag functionality to tags module Co-authored-by: ploiu <43047560+ploiu@users.noreply.github.com> --- src/handler/mod.rs | 1 - src/main.rs | 4 +- src/repository/mod.rs | 1 - src/service/file_service.rs | 3 +- src/service/folder_service.rs | 6 +- src/service/mod.rs | 1 - src/service/search_service.rs | 3 +- .../tag_handler.rs => tags/handler.rs} | 10 +- src/tags/mod.rs | 6 + src/tags/repository.rs | 182 ++++++ .../tag_service.rs => tags/service.rs} | 542 +----------------- src/tags/tests/handler.rs | 1 + src/tags/tests/mod.rs | 3 + .../tests/repository.rs} | 217 +------ src/tags/tests/service.rs | 531 +++++++++++++++++ src/test/mod.rs | 3 +- 16 files changed, 756 insertions(+), 758 deletions(-) rename src/{handler/tag_handler.rs => tags/handler.rs} (95%) create mode 100644 src/tags/mod.rs create mode 100644 src/tags/repository.rs rename src/{service/tag_service.rs => tags/service.rs} (51%) create mode 100644 src/tags/tests/handler.rs create mode 100644 src/tags/tests/mod.rs rename src/{repository/tag_repository.rs => tags/tests/repository.rs} (56%) create mode 100644 src/tags/tests/service.rs diff --git a/src/handler/mod.rs b/src/handler/mod.rs index 07f522e..21482b2 100644 --- a/src/handler/mod.rs +++ b/src/handler/mod.rs @@ -1,4 +1,3 @@ pub mod api_handler; pub mod file_handler; pub mod folder_handler; -pub mod tag_handler; diff --git a/src/main.rs b/src/main.rs index 78f8474..cb34173 100644 --- a/src/main.rs +++ b/src/main.rs @@ -8,7 +8,8 @@ use std::{fs, time::Instant}; use rocket::{Build, Rocket}; use db_migrations::generate_all_file_types_and_sizes; -use handler::{api_handler::*, file_handler::*, folder_handler::*, tag_handler::*}; +use handler::{api_handler::*, file_handler::*, folder_handler::*}; +use tags::handler::*; use crate::exif::load_all_exif_data; use crate::handler::api_handler::update_password; @@ -29,6 +30,7 @@ mod previews; mod queue; mod repository; mod service; +mod tags; mod util; #[cfg(not(test))] diff --git a/src/repository/mod.rs b/src/repository/mod.rs index aec2afc..c9e52e7 100644 --- a/src/repository/mod.rs +++ b/src/repository/mod.rs @@ -9,7 +9,6 @@ use crate::db_migrations::migrate_db; pub mod file_repository; pub mod folder_repository; pub mod metadata_repository; -pub mod tag_repository; /// creates a new connection and returns it, but panics if the connection could not be created #[cfg(not(test))] diff --git a/src/service/file_service.rs b/src/service/file_service.rs index 5fa765b..1c6b47e 100644 --- a/src/service/file_service.rs +++ b/src/service/file_service.rs @@ -23,7 +23,8 @@ use crate::model::request::file_requests::CreateFileRequest; use crate::model::response::folder_responses::FolderResponse; use crate::previews; use crate::repository::{file_repository, folder_repository, open_connection}; -use crate::service::{folder_service, tag_service}; +use crate::service::folder_service; +use crate::tags::service as tag_service; use crate::{queue, repository}; /// mapping of file lowercase file extension => file type diff --git a/src/service/folder_service.rs b/src/service/folder_service.rs index 3d00d03..092aa6e 100644 --- a/src/service/folder_service.rs +++ b/src/service/folder_service.rs @@ -21,9 +21,11 @@ use crate::model::request::folder_requests::{CreateFolderRequest, UpdateFolderRe use crate::model::response::TagApi; use crate::model::response::folder_responses::FolderResponse; use crate::previews; -use crate::repository::{folder_repository, open_connection, tag_repository}; +use crate::repository::{folder_repository, open_connection}; use crate::service::file_service::{check_root_dir, file_dir}; -use crate::service::{file_service, tag_service}; +use crate::service::file_service; +use crate::tags::repository as tag_repository; +use crate::tags::service as tag_service; use crate::{model, repository}; pub fn get_folder(id: Option) -> Result { diff --git a/src/service/mod.rs b/src/service/mod.rs index 163c227..0a1ce03 100644 --- a/src/service/mod.rs +++ b/src/service/mod.rs @@ -2,4 +2,3 @@ pub mod api_service; pub mod file_service; pub mod folder_service; pub mod search_service; -pub mod tag_service; diff --git a/src/service/search_service.rs b/src/service/search_service.rs index 0ebd568..d784c8c 100644 --- a/src/service/search_service.rs +++ b/src/service/search_service.rs @@ -10,8 +10,9 @@ use crate::model::repository::FileRecord; use crate::model::request::attributes::AttributeSearch; use crate::model::response::TagApi; use crate::model::response::folder_responses::FolderResponse; -use crate::repository::{file_repository, folder_repository, open_connection, tag_repository}; +use crate::repository::{file_repository, folder_repository, open_connection}; use crate::service::folder_service; +use crate::tags::repository as tag_repository; pub fn search_files( search_title: &str, diff --git a/src/handler/tag_handler.rs b/src/tags/handler.rs similarity index 95% rename from src/handler/tag_handler.rs rename to src/tags/handler.rs index f88d773..5cbf913 100644 --- a/src/handler/tag_handler.rs +++ b/src/tags/handler.rs @@ -11,7 +11,7 @@ use crate::model::response::tag_responses::{ CreateTagResponse, DeleteTagResponse, GetTagResponse, UpdateTagResponse, }; use crate::model::response::{BasicMessage, TagApi}; -use crate::service::tag_service; +use crate::tags::service; use crate::util::update_last_request_time; #[get("/")] @@ -26,7 +26,7 @@ pub fn get_tag( ValidateResult::Invalid => return GetTagResponse::Unauthorized("Bad Credentials".to_string()) }; update_last_request_time(last_request_time); - match tag_service::get_tag(id) { + match service::get_tag(id) { Ok(tag) => GetTagResponse::Success(Json::from(tag)), Err(GetTagError::TagNotFound) => GetTagResponse::TagNotFound(BasicMessage::new( "The tag with the passed id could not be found.", @@ -49,7 +49,7 @@ pub fn create_tag( ValidateResult::Invalid => return CreateTagResponse::Unauthorized("Bad Credentials".to_string()) }; update_last_request_time(last_request_time); - match tag_service::create_tag(tag.title.clone()) { + match service::create_tag(tag.title.clone()) { Ok(tag) => CreateTagResponse::Success(Json::from(tag)), Err(_) => CreateTagResponse::TagDbError(BasicMessage::new( "Failed to create tag info in database. Check server logs for details", @@ -69,7 +69,7 @@ pub fn update_tag( ValidateResult::Invalid => return UpdateTagResponse::Unauthorized("Bad Credentials".to_string()) }; update_last_request_time(last_request_time); - match tag_service::update_tag(tag.into_inner()) { + match service::update_tag(tag.into_inner()) { Ok(tag) => UpdateTagResponse::Success(Json::from(tag)), Err(UpdateTagError::TagNotFound) => { UpdateTagResponse::TagNotFound(BasicMessage::new("No tag with that id was found.")) @@ -95,7 +95,7 @@ pub fn delete_tag( ValidateResult::Invalid => return DeleteTagResponse::Unauthorized("Bad Credentials".to_string()) }; update_last_request_time(last_request_time); - match tag_service::delete_tag(id) { + match service::delete_tag(id) { Ok(()) => DeleteTagResponse::Success(()), Err(_) => DeleteTagResponse::TagDbError(BasicMessage::new( "Failed to delete tag from database. Check server logs for details.", diff --git a/src/tags/mod.rs b/src/tags/mod.rs new file mode 100644 index 0000000..42506c7 --- /dev/null +++ b/src/tags/mod.rs @@ -0,0 +1,6 @@ +pub mod handler; +pub mod repository; +pub mod service; + +#[cfg(test)] +mod tests; diff --git a/src/tags/repository.rs b/src/tags/repository.rs new file mode 100644 index 0000000..a4edfd2 --- /dev/null +++ b/src/tags/repository.rs @@ -0,0 +1,182 @@ +use std::{backtrace::Backtrace, collections::HashMap}; + +use rusqlite::Connection; + +use crate::model::repository; + +/// creates a new tag in the database. This does not check if the tag already exists, +/// so the caller must check that themselves +pub fn create_tag(title: &str, con: &Connection) -> Result { + let mut pst = con.prepare(include_str!("../assets/queries/tags/create_tag.sql"))?; + let id = pst.insert(rusqlite::params![title])? as u32; + Ok(repository::Tag { + id, + title: title.to_string(), + }) +} + +/// searches for a tag that case-insensitively matches that passed title. +/// +/// if `None` is returned, that means there was no match +pub fn get_tag_by_title( + title: &str, + con: &Connection, +) -> Result, rusqlite::Error> { + let mut pst = con.prepare(include_str!("../assets/queries/tags/get_by_title.sql"))?; + match pst.query_row(rusqlite::params![title], tag_mapper) { + Ok(tag) => Ok(Some(tag)), + Err(e) => { + // no tag found + if e == rusqlite::Error::QueryReturnedNoRows { + Ok(None) + } else { + log::error!( + "Failed to get tag by name, error is {e:?}\n{}", + Backtrace::force_capture() + ); + Err(e) + } + } + } +} + +pub fn get_tag(id: u32, con: &Connection) -> Result { + let mut pst = con.prepare(include_str!("../assets/queries/tags/get_by_id.sql"))?; + pst.query_row(rusqlite::params![id], tag_mapper) +} + +/// updates the past tag. Checking to make sure the tag exists needs to be done on the caller's end +pub fn update_tag(tag: repository::Tag, con: &Connection) -> Result<(), rusqlite::Error> { + let mut pst = con.prepare(include_str!("../assets/queries/tags/update_tag.sql"))?; + pst.execute(rusqlite::params![tag.title, tag.id])?; + Ok(()) +} + +pub fn delete_tag(id: u32, con: &Connection) -> Result<(), rusqlite::Error> { + let mut pst = con.prepare(include_str!("../assets/queries/tags/delete_tag.sql"))?; + pst.execute(rusqlite::params![id])?; + Ok(()) +} + +/// the caller of this function will need to make sure the tag already exists and isn't already on the file +pub fn add_tag_to_file(file_id: u32, tag_id: u32, con: &Connection) -> Result<(), rusqlite::Error> { + let mut pst = con.prepare(include_str!("../assets/queries/tags/add_tag_to_file.sql"))?; + pst.execute(rusqlite::params![file_id, tag_id])?; + Ok(()) +} + +pub fn get_tags_on_file( + file_id: u32, + con: &Connection, +) -> Result, rusqlite::Error> { + let mut pst = con.prepare(include_str!("../assets/queries/tags/get_tags_for_file.sql"))?; + let rows = pst.query_map(rusqlite::params![file_id], tag_mapper)?; + let mut tags: Vec = Vec::new(); + for tag_res in rows { + // I know it's probably bad style, but I'm laughing too hard at the double question mark. + // no I don't know what my code is doing and I'm glad my code reflects that + tags.push(tag_res?); + } + Ok(tags) +} + +pub fn get_tags_on_files( + file_ids: Vec, + con: &Connection, +) -> Result>, rusqlite::Error> { + struct TagFile { + file_id: u32, + tag_id: u32, + tag_title: String, + } + let in_clause: Vec = file_ids.iter().map(|it| format!("'{it}'")).collect(); + let in_clause = in_clause.join(","); + let formatted_query = format!( + include_str!("../assets/queries/tags/get_tags_for_files.sql"), + in_clause + ); + let mut pst = con.prepare(formatted_query.as_str())?; + let rows = pst.query_map([], |row| { + let file_id: u32 = row.get(0)?; + let tag_id: u32 = row.get(1)?; + let tag_title: String = row.get(2)?; + Ok(TagFile { + file_id, + tag_id, + tag_title, + }) + })?; + let mut mapped: HashMap> = HashMap::new(); + for res in rows { + let res = res?; + if let std::collections::hash_map::Entry::Vacant(e) = mapped.entry(res.file_id) { + e.insert(vec![repository::Tag { + id: res.tag_id, + title: res.tag_title, + }]); + } else { + mapped.get_mut(&res.file_id).unwrap().push(repository::Tag { + id: res.tag_id, + title: res.tag_title, + }); + } + } + Ok(mapped) +} + +pub fn remove_tag_from_file( + file_id: u32, + tag_id: u32, + con: &Connection, +) -> Result<(), rusqlite::Error> { + let mut pst = con.prepare(include_str!( + "../assets/queries/tags/remove_tag_from_file.sql" + ))?; + pst.execute(rusqlite::params![file_id, tag_id])?; + Ok(()) +} + +pub fn add_tag_to_folder( + folder_id: u32, + tag_id: u32, + con: &Connection, +) -> Result<(), rusqlite::Error> { + let mut pst = con.prepare(include_str!("../assets/queries/tags/add_tag_to_folder.sql"))?; + pst.execute(rusqlite::params![folder_id, tag_id])?; + Ok(()) +} + +pub fn get_tags_on_folder( + folder_id: u32, + con: &Connection, +) -> Result, rusqlite::Error> { + let mut pst = con.prepare(include_str!( + "../assets/queries/tags/get_tags_for_folder.sql" + ))?; + let rows = pst.query_map(rusqlite::params![folder_id], |row| Ok(tag_mapper(row)))?; + let mut tags: Vec = Vec::new(); + for tag_res in rows { + // I know it's probably bad style, but I'm laughing too hard at the double question mark. + // no I don't know what my code is doing and I'm glad my code reflects that + tags.push(tag_res??); + } + Ok(tags) +} + +pub fn remove_tag_from_folder( + folder_id: u32, + tag_id: u32, + con: &Connection, +) -> Result<(), rusqlite::Error> { + let mut pst = con.prepare(include_str!( + "../assets/queries/tags/remove_tag_from_folder.sql" + ))?; + pst.execute(rusqlite::params![folder_id, tag_id])?; + Ok(()) +} + +fn tag_mapper(row: &rusqlite::Row) -> Result { + let id: u32 = row.get(0)?; + let title: String = row.get(1)?; + Ok(repository::Tag { id, title }) +} diff --git a/src/service/tag_service.rs b/src/tags/service.rs similarity index 51% rename from src/service/tag_service.rs rename to src/tags/service.rs index 46882a4..8c09032 100644 --- a/src/service/tag_service.rs +++ b/src/tags/service.rs @@ -7,8 +7,9 @@ use crate::model::error::tag_errors::{ }; use crate::model::repository; use crate::model::response::TagApi; -use crate::repository::{open_connection, tag_repository}; +use crate::repository::open_connection; use crate::service::{file_service, folder_service}; +use crate::tags::repository as tag_repository; /// will create a tag, or return the already-existing tag if one with the same name exists /// returns the created/existing tag @@ -409,542 +410,3 @@ pub fn get_tags_on_folder(folder_id: u32) -> Result, TagRelationErro let api_tags: Vec = db_tags.into_iter().map(TagApi::from).collect(); Ok(api_tags) } - -#[cfg(test)] -mod get_tag_tests { - use crate::model::error::tag_errors::GetTagError; - use crate::service::tag_service::{create_tag, get_tag}; - use crate::test::*; - - #[test] - fn test_get_tag() { - init_db_folder(); - let expected = create_tag("test".to_string()).unwrap(); - let actual = get_tag(1).unwrap(); - assert_eq!(actual, expected); - cleanup(); - } - - #[test] - fn test_get_tag_non_existent() { - init_db_folder(); - let res = get_tag(1).expect_err("Retrieving a nonexistent tag should return an error"); - assert_eq!(GetTagError::TagNotFound, res); - cleanup(); - } -} - -#[cfg(test)] -mod update_tag_tests { - use crate::model::error::tag_errors::UpdateTagError; - use crate::model::response::TagApi; - use crate::service::tag_service::{create_tag, get_tag, update_tag}; - use crate::test::{cleanup, init_db_folder}; - - #[test] - fn update_tag_works() { - init_db_folder(); - let tag = create_tag("test_tag".to_string()).unwrap(); - let updated_tag = update_tag(TagApi { - id: tag.id, - title: "new_name".to_string(), - }) - .unwrap(); - assert_eq!(String::from("new_name"), updated_tag.title); - assert_eq!(Some(1), updated_tag.id); - // test that it's in the database - let updated_tag = get_tag(1).unwrap(); - assert_eq!(String::from("new_name"), updated_tag.title); - cleanup(); - } - - #[test] - fn update_tag_not_found() { - init_db_folder(); - let res = update_tag(TagApi { - id: Some(1), - title: "what".to_string(), - }); - assert_eq!(UpdateTagError::TagNotFound, res.unwrap_err()); - cleanup(); - } - - #[test] - fn update_tag_already_exists() { - init_db_folder(); - create_tag("first".to_string()).unwrap(); - create_tag("second".to_string()).unwrap(); - let res = update_tag(TagApi { - id: Some(2), - title: "FiRsT".to_string(), - }); - assert_eq!(UpdateTagError::NewNameAlreadyExists, res.unwrap_err()); - cleanup(); - } -} - -#[cfg(test)] -mod delete_tag_tests { - use crate::model::error::tag_errors::GetTagError; - use crate::service::tag_service::{create_tag, delete_tag, get_tag}; - use crate::test::{cleanup, init_db_folder}; - - #[test] - fn delete_tag_works() { - init_db_folder(); - create_tag("test".to_string()).unwrap(); - delete_tag(1).unwrap(); - let res = get_tag(1).unwrap_err(); - assert_eq!(GetTagError::TagNotFound, res); - cleanup(); - } -} - -#[cfg(test)] -mod update_file_tag_test { - use crate::model::error::tag_errors::TagRelationError; - use crate::model::file_types::FileTypes; - use crate::model::repository::FileRecord; - use crate::model::response::TagApi; - - use crate::service::tag_service::{create_tag, get_tags_on_file, update_file_tags}; - use crate::test::{cleanup, init_db_folder, now}; - - #[test] - fn update_file_tags_works() { - init_db_folder(); - create_tag("test".to_string()).unwrap(); - FileRecord { - id: None, - name: "test_file".to_string(), - parent_id: None, - size: 0, - create_date: now(), - file_type: FileTypes::Unknown, - } - .save_to_db(); - update_file_tags( - 1, - vec![ - TagApi { - id: Some(1), - title: "test".to_string(), - }, - TagApi { - id: None, - title: "new tag".to_string(), - }, - ], - ) - .unwrap(); - let expected = vec![ - TagApi { - id: Some(1), - title: "test".to_string(), - }, - TagApi { - id: Some(2), - title: "new tag".to_string(), - }, - ]; - let actual = get_tags_on_file(1).unwrap(); - assert_eq!(actual, expected); - cleanup(); - } - - #[test] - fn update_file_tags_removes_tags() { - init_db_folder(); - FileRecord { - id: None, - name: "test".to_string(), - parent_id: None, - size: 0, - create_date: now(), - file_type: FileTypes::Unknown, - } - .save_to_db(); - update_file_tags( - 1, - vec![TagApi { - id: None, - title: "test".to_string(), - }], - ) - .unwrap(); - update_file_tags(1, vec![]).unwrap(); - assert_eq!(get_tags_on_file(1).unwrap(), vec![]); - cleanup(); - } - - #[test] - fn update_file_tags_throws_error_if_file_not_found() { - init_db_folder(); - let res = update_file_tags(1, vec![]).unwrap_err(); - assert_eq!(TagRelationError::FileNotFound, res); - cleanup(); - } - - #[test] - fn update_file_tags_deduplicates_existing_tags() { - init_db_folder(); - create_tag("test".to_string()).unwrap(); - FileRecord { - id: None, - name: "test_file".to_string(), - parent_id: None, - size: 0, - create_date: now(), - file_type: FileTypes::Unknown, - } - .save_to_db(); - - // Try to add the same tag twice - should not fail and should only add it once - update_file_tags( - 1, - vec![ - TagApi { - id: Some(1), - title: "test".to_string(), - }, - TagApi { - id: Some(1), - title: "test".to_string(), - }, - ], - ) - .unwrap(); - - let actual = get_tags_on_file(1).unwrap(); - assert_eq!(actual.len(), 1); - assert_eq!(actual[0].id, Some(1)); - assert_eq!(actual[0].title, "test"); - cleanup(); - } - - #[test] - fn update_file_tags_deduplicates_new_tags_with_same_name() { - init_db_folder(); - FileRecord { - id: None, - name: "test_file".to_string(), - parent_id: None, - size: 0, - create_date: now(), - file_type: FileTypes::Unknown, - } - .save_to_db(); - - // Create tag implicitly by name twice - should only create once - update_file_tags( - 1, - vec![ - TagApi { - id: None, - title: "test".to_string(), - }, - TagApi { - id: None, - title: "test".to_string(), - }, - ], - ) - .unwrap(); - - let actual = get_tags_on_file(1).unwrap(); - assert_eq!(actual.len(), 1); - assert_eq!(actual[0].id, Some(1)); - assert_eq!(actual[0].title, "test"); - cleanup(); - } - - #[test] - fn update_file_tags_skips_duplicate_after_creating() { - init_db_folder(); - FileRecord { - id: None, - name: "test_file".to_string(), - parent_id: None, - size: 0, - create_date: now(), - file_type: FileTypes::Unknown, - } - .save_to_db(); - - // Mix of new tag by name and existing tag by id (same tag) - update_file_tags( - 1, - vec![TagApi { - id: None, - title: "test".to_string(), - }], - ) - .unwrap(); - - // Now update with both the id and a new tag with same name - update_file_tags( - 1, - vec![ - TagApi { - id: Some(1), - title: "test".to_string(), - }, - TagApi { - id: None, - title: "test".to_string(), - }, - ], - ) - .unwrap(); - - let actual = get_tags_on_file(1).unwrap(); - assert_eq!(actual.len(), 1); - assert_eq!(actual[0].id, Some(1)); - assert_eq!(actual[0].title, "test"); - cleanup(); - } -} - -#[cfg(test)] -mod update_folder_tag_test { - use crate::model::error::tag_errors::TagRelationError; - use crate::model::repository::Folder; - use crate::model::response::TagApi; - use crate::repository::{folder_repository, open_connection}; - use crate::service::tag_service::{create_tag, get_tags_on_folder, update_folder_tags}; - use crate::test::{cleanup, init_db_folder}; - - #[test] - fn update_folder_tags_works() { - init_db_folder(); - let con = open_connection(); - create_tag("test".to_string()).unwrap(); - folder_repository::create_folder( - &Folder { - parent_id: None, - id: None, - name: "test_file".to_string(), - }, - &con, - ) - .unwrap(); - con.close().unwrap(); - update_folder_tags( - 1, - vec![ - TagApi { - id: Some(1), - title: "test".to_string(), - }, - TagApi { - id: None, - title: "new tag".to_string(), - }, - ], - ) - .unwrap(); - let expected = vec![ - TagApi { - id: Some(1), - title: "test".to_string(), - }, - TagApi { - id: Some(2), - title: "new tag".to_string(), - }, - ]; - let actual = get_tags_on_folder(1).unwrap(); - assert_eq!(actual, expected); - cleanup(); - } - - #[test] - fn update_folder_tags_removes_tags() { - init_db_folder(); - let con = open_connection(); - folder_repository::create_folder( - &Folder { - parent_id: None, - id: None, - name: "test".to_string(), - }, - &con, - ) - .unwrap(); - con.close().unwrap(); - update_folder_tags( - 1, - vec![TagApi { - id: None, - title: "test".to_string(), - }], - ) - .unwrap(); - update_folder_tags(1, vec![]).unwrap(); - assert_eq!(get_tags_on_folder(1).unwrap(), vec![]); - cleanup(); - } - - #[test] - fn update_folder_tags_throws_error_if_folder_not_found() { - init_db_folder(); - let res = update_folder_tags(1, vec![]).unwrap_err(); - assert_eq!(TagRelationError::FolderNotFound, res); - cleanup(); - } - - #[test] - fn update_folder_tags_deduplicates_existing_tags() { - init_db_folder(); - let con = open_connection(); - create_tag("test".to_string()).unwrap(); - folder_repository::create_folder( - &Folder { - parent_id: None, - id: None, - name: "test_folder".to_string(), - }, - &con, - ) - .unwrap(); - con.close().unwrap(); - - // Try to add the same tag twice - should not fail and should only add it once - update_folder_tags( - 1, - vec![ - TagApi { - id: Some(1), - title: "test".to_string(), - }, - TagApi { - id: Some(1), - title: "test".to_string(), - }, - ], - ) - .unwrap(); - - let actual = get_tags_on_folder(1).unwrap(); - assert_eq!(actual.len(), 1); - assert_eq!(actual[0].id, Some(1)); - assert_eq!(actual[0].title, "test"); - cleanup(); - } - - #[test] - fn update_folder_tags_deduplicates_new_tags_with_same_name() { - init_db_folder(); - let con = open_connection(); - folder_repository::create_folder( - &Folder { - parent_id: None, - id: None, - name: "test_folder".to_string(), - }, - &con, - ) - .unwrap(); - con.close().unwrap(); - - // Create tag implicitly by name twice - should only create once - update_folder_tags( - 1, - vec![ - TagApi { - id: None, - title: "test".to_string(), - }, - TagApi { - id: None, - title: "test".to_string(), - }, - ], - ) - .unwrap(); - - let actual = get_tags_on_folder(1).unwrap(); - assert_eq!(actual.len(), 1); - assert_eq!(actual[0].id, Some(1)); - assert_eq!(actual[0].title, "test"); - cleanup(); - } - - #[test] - fn update_folder_tags_skips_duplicate_after_creating() { - init_db_folder(); - let con = open_connection(); - folder_repository::create_folder( - &Folder { - parent_id: None, - id: None, - name: "test_folder".to_string(), - }, - &con, - ) - .unwrap(); - con.close().unwrap(); - - // Mix of new tag by name and existing tag by id (same tag) - update_folder_tags( - 1, - vec![TagApi { - id: None, - title: "test".to_string(), - }], - ) - .unwrap(); - - // Now update with both the id and a new tag with same name - update_folder_tags( - 1, - vec![ - TagApi { - id: Some(1), - title: "test".to_string(), - }, - TagApi { - id: None, - title: "test".to_string(), - }, - ], - ) - .unwrap(); - - let actual = get_tags_on_folder(1).unwrap(); - assert_eq!(actual.len(), 1); - assert_eq!(actual[0].id, Some(1)); - assert_eq!(actual[0].title, "test"); - cleanup(); - } -} - -#[cfg(test)] -mod get_tags_on_file_tests { - use crate::model::error::tag_errors::TagRelationError; - use crate::service::tag_service::get_tags_on_file; - use crate::test::{cleanup, init_db_folder}; - - #[test] - fn throws_error_if_file_not_found() { - init_db_folder(); - let err = get_tags_on_file(1).unwrap_err(); - assert_eq!(TagRelationError::FileNotFound, err); - cleanup(); - } -} - -#[cfg(test)] -mod get_tags_on_folder_tests { - use crate::model::error::tag_errors::TagRelationError; - use crate::service::tag_service::get_tags_on_folder; - use crate::test::{cleanup, init_db_folder}; - - #[test] - fn throws_error_if_file_not_found() { - init_db_folder(); - let err = get_tags_on_folder(1).unwrap_err(); - assert_eq!(TagRelationError::FileNotFound, err); - cleanup(); - } -} diff --git a/src/tags/tests/handler.rs b/src/tags/tests/handler.rs new file mode 100644 index 0000000..78346ef --- /dev/null +++ b/src/tags/tests/handler.rs @@ -0,0 +1 @@ +// Handler tests will be added here as needed diff --git a/src/tags/tests/mod.rs b/src/tags/tests/mod.rs new file mode 100644 index 0000000..c8c9368 --- /dev/null +++ b/src/tags/tests/mod.rs @@ -0,0 +1,3 @@ +mod handler; +mod repository; +mod service; diff --git a/src/repository/tag_repository.rs b/src/tags/tests/repository.rs similarity index 56% rename from src/repository/tag_repository.rs rename to src/tags/tests/repository.rs index 4a0a179..b5ea262 100644 --- a/src/repository/tag_repository.rs +++ b/src/tags/tests/repository.rs @@ -1,197 +1,14 @@ -use std::{backtrace::Backtrace, collections::HashMap}; - -use rusqlite::Connection; - -use crate::model::repository; - -/// creates a new tag in the database. This does not check if the tag already exists, -/// so the caller must check that themselves -pub fn create_tag(title: &str, con: &Connection) -> Result { - let mut pst = con.prepare(include_str!("../assets/queries/tags/create_tag.sql"))?; - let id = pst.insert(rusqlite::params![title])? as u32; - Ok(repository::Tag { - id, - title: title.to_string(), - }) -} - -/// searches for a tag that case-insensitively matches that passed title. -/// -/// if `None` is returned, that means there was no match -pub fn get_tag_by_title( - title: &str, - con: &Connection, -) -> Result, rusqlite::Error> { - let mut pst = con.prepare(include_str!("../assets/queries/tags/get_by_title.sql"))?; - match pst.query_row(rusqlite::params![title], tag_mapper) { - Ok(tag) => Ok(Some(tag)), - Err(e) => { - // no tag found - if e == rusqlite::Error::QueryReturnedNoRows { - Ok(None) - } else { - log::error!( - "Failed to get tag by name, error is {e:?}\n{}", - Backtrace::force_capture() - ); - Err(e) - } - } - } -} - -pub fn get_tag(id: u32, con: &Connection) -> Result { - let mut pst = con.prepare(include_str!("../assets/queries/tags/get_by_id.sql"))?; - pst.query_row(rusqlite::params![id], tag_mapper) -} - -/// updates the past tag. Checking to make sure the tag exists needs to be done on the caller's end -pub fn update_tag(tag: repository::Tag, con: &Connection) -> Result<(), rusqlite::Error> { - let mut pst = con.prepare(include_str!("../assets/queries/tags/update_tag.sql"))?; - pst.execute(rusqlite::params![tag.title, tag.id])?; - Ok(()) -} - -pub fn delete_tag(id: u32, con: &Connection) -> Result<(), rusqlite::Error> { - let mut pst = con.prepare(include_str!("../assets/queries/tags/delete_tag.sql"))?; - pst.execute(rusqlite::params![id])?; - Ok(()) -} - -/// the caller of this function will need to make sure the tag already exists and isn't already on the file -pub fn add_tag_to_file(file_id: u32, tag_id: u32, con: &Connection) -> Result<(), rusqlite::Error> { - let mut pst = con.prepare(include_str!("../assets/queries/tags/add_tag_to_file.sql"))?; - pst.execute(rusqlite::params![file_id, tag_id])?; - Ok(()) -} - -pub fn get_tags_on_file( - file_id: u32, - con: &Connection, -) -> Result, rusqlite::Error> { - let mut pst = con.prepare(include_str!("../assets/queries/tags/get_tags_for_file.sql"))?; - let rows = pst.query_map(rusqlite::params![file_id], tag_mapper)?; - let mut tags: Vec = Vec::new(); - for tag_res in rows { - // I know it's probably bad style, but I'm laughing too hard at the double question mark. - // no I don't know what my code is doing and I'm glad my code reflects that - tags.push(tag_res?); - } - Ok(tags) -} - -pub fn get_tags_on_files( - file_ids: Vec, - con: &Connection, -) -> Result>, rusqlite::Error> { - struct TagFile { - file_id: u32, - tag_id: u32, - tag_title: String, - } - let in_clause: Vec = file_ids.iter().map(|it| format!("'{it}'")).collect(); - let in_clause = in_clause.join(","); - let formatted_query = format!( - include_str!("../assets/queries/tags/get_tags_for_files.sql"), - in_clause - ); - let mut pst = con.prepare(formatted_query.as_str())?; - let rows = pst.query_map([], |row| { - let file_id: u32 = row.get(0)?; - let tag_id: u32 = row.get(1)?; - let tag_title: String = row.get(2)?; - Ok(TagFile { - file_id, - tag_id, - tag_title, - }) - })?; - let mut mapped: HashMap> = HashMap::new(); - for res in rows { - let res = res?; - if let std::collections::hash_map::Entry::Vacant(e) = mapped.entry(res.file_id) { - e.insert(vec![repository::Tag { - id: res.tag_id, - title: res.tag_title, - }]); - } else { - mapped.get_mut(&res.file_id).unwrap().push(repository::Tag { - id: res.tag_id, - title: res.tag_title, - }); - } - } - Ok(mapped) -} - -pub fn remove_tag_from_file( - file_id: u32, - tag_id: u32, - con: &Connection, -) -> Result<(), rusqlite::Error> { - let mut pst = con.prepare(include_str!( - "../assets/queries/tags/remove_tag_from_file.sql" - ))?; - pst.execute(rusqlite::params![file_id, tag_id])?; - Ok(()) -} - -pub fn add_tag_to_folder( - folder_id: u32, - tag_id: u32, - con: &Connection, -) -> Result<(), rusqlite::Error> { - let mut pst = con.prepare(include_str!("../assets/queries/tags/add_tag_to_folder.sql"))?; - pst.execute(rusqlite::params![folder_id, tag_id])?; - Ok(()) -} - -pub fn get_tags_on_folder( - folder_id: u32, - con: &Connection, -) -> Result, rusqlite::Error> { - let mut pst = con.prepare(include_str!( - "../assets/queries/tags/get_tags_for_folder.sql" - ))?; - let rows = pst.query_map(rusqlite::params![folder_id], |row| Ok(tag_mapper(row)))?; - let mut tags: Vec = Vec::new(); - for tag_res in rows { - // I know it's probably bad style, but I'm laughing too hard at the double question mark. - // no I don't know what my code is doing and I'm glad my code reflects that - tags.push(tag_res??); - } - Ok(tags) -} - -pub fn remove_tag_from_folder( - folder_id: u32, - tag_id: u32, - con: &Connection, -) -> Result<(), rusqlite::Error> { - let mut pst = con.prepare(include_str!( - "../assets/queries/tags/remove_tag_from_folder.sql" - ))?; - pst.execute(rusqlite::params![folder_id, tag_id])?; - Ok(()) -} - -fn tag_mapper(row: &rusqlite::Row) -> Result { - let id: u32 = row.get(0)?; - let title: String = row.get(1)?; - Ok(repository::Tag { id, title }) -} - -#[cfg(test)] mod create_tag_tests { use crate::model::repository::Tag; - use crate::repository::{open_connection, tag_repository}; + use crate::repository::open_connection; + use crate::tags::repository; use crate::test::{cleanup, init_db_folder}; #[test] fn create_tag() { init_db_folder(); let con = open_connection(); - let tag = tag_repository::create_tag("test", &con).unwrap(); + let tag = repository::create_tag("test", &con).unwrap(); con.close().unwrap(); assert_eq!( Tag { @@ -204,11 +21,10 @@ mod create_tag_tests { } } -#[cfg(test)] mod get_tag_by_title_tests { use crate::model::repository::Tag; use crate::repository::open_connection; - use crate::repository::tag_repository::{create_tag, get_tag_by_title}; + use crate::tags::repository::{create_tag, get_tag_by_title}; use crate::test::*; #[test] @@ -238,11 +54,10 @@ mod get_tag_by_title_tests { } } -#[cfg(test)] mod get_tag_by_id_tests { use crate::model::repository::Tag; use crate::repository::open_connection; - use crate::repository::tag_repository::{create_tag, get_tag}; + use crate::tags::repository::{create_tag, get_tag}; use crate::test::{cleanup, init_db_folder}; #[test] @@ -263,11 +78,10 @@ mod get_tag_by_id_tests { } } -#[cfg(test)] mod update_tag_tests { use crate::model::repository::Tag; use crate::repository::open_connection; - use crate::repository::tag_repository::{create_tag, get_tag, update_tag}; + use crate::tags::repository::{create_tag, get_tag, update_tag}; use crate::test::{cleanup, init_db_folder}; #[test] @@ -296,10 +110,9 @@ mod update_tag_tests { } } -#[cfg(test)] mod delete_tag_tests { use crate::repository::open_connection; - use crate::repository::tag_repository::{create_tag, delete_tag, get_tag}; + use crate::tags::repository::{create_tag, delete_tag, get_tag}; use crate::test::{cleanup, init_db_folder}; #[test] @@ -315,9 +128,8 @@ mod delete_tag_tests { } } -#[cfg(test)] mod get_tag_on_file_tests { - use super::*; + use crate::tags::repository::*; use crate::model::file_types::FileTypes; use crate::model::repository::{FileRecord, Tag}; use crate::repository::file_repository::create_file; @@ -384,9 +196,8 @@ mod get_tag_on_file_tests { } } -#[cfg(test)] mod remove_tag_from_file_tests { - use super::*; + use crate::tags::repository::*; use crate::model::file_types::FileTypes; use crate::model::repository::{FileRecord, Tag}; use crate::repository::file_repository::create_file; @@ -418,12 +229,11 @@ mod remove_tag_from_file_tests { } } -#[cfg(test)] mod get_tag_on_folder_tests { use crate::model::repository::{Folder, Tag}; use crate::repository::folder_repository::create_folder; use crate::repository::open_connection; - use crate::repository::tag_repository::{add_tag_to_folder, create_tag, get_tags_on_folder}; + use crate::tags::repository::{add_tag_to_folder, create_tag, get_tags_on_folder}; use crate::test::*; #[test] @@ -480,12 +290,11 @@ mod get_tag_on_folder_tests { } } -#[cfg(test)] mod remove_tag_from_folder_tests { use crate::model::repository::{Folder, Tag}; use crate::repository::folder_repository::create_folder; use crate::repository::open_connection; - use crate::repository::tag_repository::{ + use crate::tags::repository::{ create_tag, get_tags_on_folder, remove_tag_from_folder, }; use crate::test::{cleanup, init_db_folder}; @@ -512,11 +321,11 @@ mod remove_tag_from_folder_tests { } } -#[cfg(test)] mod get_tags_on_files_tests { use std::collections::HashMap; use crate::{model::repository::Tag, repository::open_connection, test::*}; + use crate::tags::repository::get_tags_on_files; #[test] fn returns_proper_mapping_for_file_tags() { @@ -528,7 +337,7 @@ mod get_tags_on_files_tests { create_tag_file("tag2", 1); create_tag_file("tag3", 2); let con = open_connection(); - let res = super::get_tags_on_files(vec![1, 2, 3], &con).unwrap(); + let res = get_tags_on_files(vec![1, 2, 3], &con).unwrap(); con.close().unwrap(); #[rustfmt::skip] let expected = HashMap::from([ diff --git a/src/tags/tests/service.rs b/src/tags/tests/service.rs new file mode 100644 index 0000000..c45a4d2 --- /dev/null +++ b/src/tags/tests/service.rs @@ -0,0 +1,531 @@ +mod get_tag_tests { + use crate::model::error::tag_errors::GetTagError; + use crate::tags::service::{create_tag, get_tag}; + use crate::test::*; + + #[test] + fn test_get_tag() { + init_db_folder(); + let expected = create_tag("test".to_string()).unwrap(); + let actual = get_tag(1).unwrap(); + assert_eq!(actual, expected); + cleanup(); + } + + #[test] + fn test_get_tag_non_existent() { + init_db_folder(); + let res = get_tag(1).expect_err("Retrieving a nonexistent tag should return an error"); + assert_eq!(GetTagError::TagNotFound, res); + cleanup(); + } +} + +mod update_tag_tests { + use crate::model::error::tag_errors::UpdateTagError; + use crate::model::response::TagApi; + use crate::tags::service::{create_tag, get_tag, update_tag}; + use crate::test::{cleanup, init_db_folder}; + + #[test] + fn update_tag_works() { + init_db_folder(); + let tag = create_tag("test_tag".to_string()).unwrap(); + let updated_tag = update_tag(TagApi { + id: tag.id, + title: "new_name".to_string(), + }) + .unwrap(); + assert_eq!(String::from("new_name"), updated_tag.title); + assert_eq!(Some(1), updated_tag.id); + // test that it's in the database + let updated_tag = get_tag(1).unwrap(); + assert_eq!(String::from("new_name"), updated_tag.title); + cleanup(); + } + + #[test] + fn update_tag_not_found() { + init_db_folder(); + let res = update_tag(TagApi { + id: Some(1), + title: "what".to_string(), + }); + assert_eq!(UpdateTagError::TagNotFound, res.unwrap_err()); + cleanup(); + } + + #[test] + fn update_tag_already_exists() { + init_db_folder(); + create_tag("first".to_string()).unwrap(); + create_tag("second".to_string()).unwrap(); + let res = update_tag(TagApi { + id: Some(2), + title: "FiRsT".to_string(), + }); + assert_eq!(UpdateTagError::NewNameAlreadyExists, res.unwrap_err()); + cleanup(); + } +} + +mod delete_tag_tests { + use crate::model::error::tag_errors::GetTagError; + use crate::tags::service::{create_tag, delete_tag, get_tag}; + use crate::test::{cleanup, init_db_folder}; + + #[test] + fn delete_tag_works() { + init_db_folder(); + create_tag("test".to_string()).unwrap(); + delete_tag(1).unwrap(); + let res = get_tag(1).unwrap_err(); + assert_eq!(GetTagError::TagNotFound, res); + cleanup(); + } +} + +mod update_file_tag_test { + use crate::model::error::tag_errors::TagRelationError; + use crate::model::file_types::FileTypes; + use crate::model::repository::FileRecord; + use crate::model::response::TagApi; + + use crate::tags::service::{create_tag, get_tags_on_file, update_file_tags}; + use crate::test::{cleanup, init_db_folder, now}; + + #[test] + fn update_file_tags_works() { + init_db_folder(); + create_tag("test".to_string()).unwrap(); + FileRecord { + id: None, + name: "test_file".to_string(), + parent_id: None, + size: 0, + create_date: now(), + file_type: FileTypes::Unknown, + } + .save_to_db(); + update_file_tags( + 1, + vec![ + TagApi { + id: Some(1), + title: "test".to_string(), + }, + TagApi { + id: None, + title: "new tag".to_string(), + }, + ], + ) + .unwrap(); + let expected = vec![ + TagApi { + id: Some(1), + title: "test".to_string(), + }, + TagApi { + id: Some(2), + title: "new tag".to_string(), + }, + ]; + let actual = get_tags_on_file(1).unwrap(); + assert_eq!(actual, expected); + cleanup(); + } + + #[test] + fn update_file_tags_removes_tags() { + init_db_folder(); + FileRecord { + id: None, + name: "test".to_string(), + parent_id: None, + size: 0, + create_date: now(), + file_type: FileTypes::Unknown, + } + .save_to_db(); + update_file_tags( + 1, + vec![TagApi { + id: None, + title: "test".to_string(), + }], + ) + .unwrap(); + update_file_tags(1, vec![]).unwrap(); + assert_eq!(get_tags_on_file(1).unwrap(), vec![]); + cleanup(); + } + + #[test] + fn update_file_tags_throws_error_if_file_not_found() { + init_db_folder(); + let res = update_file_tags(1, vec![]).unwrap_err(); + assert_eq!(TagRelationError::FileNotFound, res); + cleanup(); + } + + #[test] + fn update_file_tags_deduplicates_existing_tags() { + init_db_folder(); + create_tag("test".to_string()).unwrap(); + FileRecord { + id: None, + name: "test_file".to_string(), + parent_id: None, + size: 0, + create_date: now(), + file_type: FileTypes::Unknown, + } + .save_to_db(); + + // Try to add the same tag twice - should not fail and should only add it once + update_file_tags( + 1, + vec![ + TagApi { + id: Some(1), + title: "test".to_string(), + }, + TagApi { + id: Some(1), + title: "test".to_string(), + }, + ], + ) + .unwrap(); + + let actual = get_tags_on_file(1).unwrap(); + assert_eq!(actual.len(), 1); + assert_eq!(actual[0].id, Some(1)); + assert_eq!(actual[0].title, "test"); + cleanup(); + } + + #[test] + fn update_file_tags_deduplicates_new_tags_with_same_name() { + init_db_folder(); + FileRecord { + id: None, + name: "test_file".to_string(), + parent_id: None, + size: 0, + create_date: now(), + file_type: FileTypes::Unknown, + } + .save_to_db(); + + // Create tag implicitly by name twice - should only create once + update_file_tags( + 1, + vec![ + TagApi { + id: None, + title: "test".to_string(), + }, + TagApi { + id: None, + title: "test".to_string(), + }, + ], + ) + .unwrap(); + + let actual = get_tags_on_file(1).unwrap(); + assert_eq!(actual.len(), 1); + assert_eq!(actual[0].id, Some(1)); + assert_eq!(actual[0].title, "test"); + cleanup(); + } + + #[test] + fn update_file_tags_skips_duplicate_after_creating() { + init_db_folder(); + FileRecord { + id: None, + name: "test_file".to_string(), + parent_id: None, + size: 0, + create_date: now(), + file_type: FileTypes::Unknown, + } + .save_to_db(); + + // Mix of new tag by name and existing tag by id (same tag) + update_file_tags( + 1, + vec![TagApi { + id: None, + title: "test".to_string(), + }], + ) + .unwrap(); + + // Now update with both the id and a new tag with same name + update_file_tags( + 1, + vec![ + TagApi { + id: Some(1), + title: "test".to_string(), + }, + TagApi { + id: None, + title: "test".to_string(), + }, + ], + ) + .unwrap(); + + let actual = get_tags_on_file(1).unwrap(); + assert_eq!(actual.len(), 1); + assert_eq!(actual[0].id, Some(1)); + assert_eq!(actual[0].title, "test"); + cleanup(); + } +} + +mod update_folder_tag_test { + use crate::model::error::tag_errors::TagRelationError; + use crate::model::repository::Folder; + use crate::model::response::TagApi; + use crate::repository::{folder_repository, open_connection}; + use crate::tags::service::{create_tag, get_tags_on_folder, update_folder_tags}; + use crate::test::{cleanup, init_db_folder}; + + #[test] + fn update_folder_tags_works() { + init_db_folder(); + let con = open_connection(); + create_tag("test".to_string()).unwrap(); + folder_repository::create_folder( + &Folder { + parent_id: None, + id: None, + name: "test_file".to_string(), + }, + &con, + ) + .unwrap(); + con.close().unwrap(); + update_folder_tags( + 1, + vec![ + TagApi { + id: Some(1), + title: "test".to_string(), + }, + TagApi { + id: None, + title: "new tag".to_string(), + }, + ], + ) + .unwrap(); + let expected = vec![ + TagApi { + id: Some(1), + title: "test".to_string(), + }, + TagApi { + id: Some(2), + title: "new tag".to_string(), + }, + ]; + let actual = get_tags_on_folder(1).unwrap(); + assert_eq!(actual, expected); + cleanup(); + } + + #[test] + fn update_folder_tags_removes_tags() { + init_db_folder(); + let con = open_connection(); + folder_repository::create_folder( + &Folder { + parent_id: None, + id: None, + name: "test".to_string(), + }, + &con, + ) + .unwrap(); + con.close().unwrap(); + update_folder_tags( + 1, + vec![TagApi { + id: None, + title: "test".to_string(), + }], + ) + .unwrap(); + update_folder_tags(1, vec![]).unwrap(); + assert_eq!(get_tags_on_folder(1).unwrap(), vec![]); + cleanup(); + } + + #[test] + fn update_folder_tags_throws_error_if_folder_not_found() { + init_db_folder(); + let res = update_folder_tags(1, vec![]).unwrap_err(); + assert_eq!(TagRelationError::FolderNotFound, res); + cleanup(); + } + + #[test] + fn update_folder_tags_deduplicates_existing_tags() { + init_db_folder(); + let con = open_connection(); + create_tag("test".to_string()).unwrap(); + folder_repository::create_folder( + &Folder { + parent_id: None, + id: None, + name: "test_folder".to_string(), + }, + &con, + ) + .unwrap(); + con.close().unwrap(); + + // Try to add the same tag twice - should not fail and should only add it once + update_folder_tags( + 1, + vec![ + TagApi { + id: Some(1), + title: "test".to_string(), + }, + TagApi { + id: Some(1), + title: "test".to_string(), + }, + ], + ) + .unwrap(); + + let actual = get_tags_on_folder(1).unwrap(); + assert_eq!(actual.len(), 1); + assert_eq!(actual[0].id, Some(1)); + assert_eq!(actual[0].title, "test"); + cleanup(); + } + + #[test] + fn update_folder_tags_deduplicates_new_tags_with_same_name() { + init_db_folder(); + let con = open_connection(); + folder_repository::create_folder( + &Folder { + parent_id: None, + id: None, + name: "test_folder".to_string(), + }, + &con, + ) + .unwrap(); + con.close().unwrap(); + + // Create tag implicitly by name twice - should only create once + update_folder_tags( + 1, + vec![ + TagApi { + id: None, + title: "test".to_string(), + }, + TagApi { + id: None, + title: "test".to_string(), + }, + ], + ) + .unwrap(); + + let actual = get_tags_on_folder(1).unwrap(); + assert_eq!(actual.len(), 1); + assert_eq!(actual[0].id, Some(1)); + assert_eq!(actual[0].title, "test"); + cleanup(); + } + + #[test] + fn update_folder_tags_skips_duplicate_after_creating() { + init_db_folder(); + let con = open_connection(); + folder_repository::create_folder( + &Folder { + parent_id: None, + id: None, + name: "test_folder".to_string(), + }, + &con, + ) + .unwrap(); + con.close().unwrap(); + + // Mix of new tag by name and existing tag by id (same tag) + update_folder_tags( + 1, + vec![TagApi { + id: None, + title: "test".to_string(), + }], + ) + .unwrap(); + + // Now update with both the id and a new tag with same name + update_folder_tags( + 1, + vec![ + TagApi { + id: Some(1), + title: "test".to_string(), + }, + TagApi { + id: None, + title: "test".to_string(), + }, + ], + ) + .unwrap(); + + let actual = get_tags_on_folder(1).unwrap(); + assert_eq!(actual.len(), 1); + assert_eq!(actual[0].id, Some(1)); + assert_eq!(actual[0].title, "test"); + cleanup(); + } +} + +mod get_tags_on_file_tests { + use crate::model::error::tag_errors::TagRelationError; + use crate::tags::service::get_tags_on_file; + use crate::test::{cleanup, init_db_folder}; + + #[test] + fn throws_error_if_file_not_found() { + init_db_folder(); + let err = get_tags_on_file(1).unwrap_err(); + assert_eq!(TagRelationError::FileNotFound, err); + cleanup(); + } +} + +mod get_tags_on_folder_tests { + use crate::model::error::tag_errors::TagRelationError; + use crate::tags::service::get_tags_on_folder; + use crate::test::{cleanup, init_db_folder}; + + #[test] + fn throws_error_if_file_not_found() { + init_db_folder(); + let err = get_tags_on_folder(1).unwrap_err(); + assert_eq!(TagRelationError::FileNotFound, err); + cleanup(); + } +} diff --git a/src/test/mod.rs b/src/test/mod.rs index 28b2795..9cd14ec 100644 --- a/src/test/mod.rs +++ b/src/test/mod.rs @@ -11,9 +11,10 @@ mod tests { use crate::model::repository::{FileRecord, Folder, Tag}; use crate::previews; use crate::repository::{ - file_repository, folder_repository, initialize_db, open_connection, tag_repository, + file_repository, folder_repository, initialize_db, open_connection, }; use crate::service::file_service::{determine_file_type, file_dir}; + use crate::tags::repository as tag_repository; use crate::temp_dir; use std::fs; use std::fs::{remove_dir_all, remove_file}; From 177f64cfdad2dbe771ba7a65a76772ec71a5b8dc Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 15 Nov 2025 18:31:53 +0000 Subject: [PATCH 08/18] Use super imports for better module organization Co-authored-by: ploiu <43047560+ploiu@users.noreply.github.com> --- src/tags/handler.rs | 3 ++- src/tags/repository.rs | 2 -- src/tags/tests/handler.rs | 3 ++- src/tags/tests/repository.rs | 20 ++++++++++---------- src/tags/tests/service.rs | 14 +++++++------- 5 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/tags/handler.rs b/src/tags/handler.rs index 5cbf913..9a73265 100644 --- a/src/tags/handler.rs +++ b/src/tags/handler.rs @@ -11,9 +11,10 @@ use crate::model::response::tag_responses::{ CreateTagResponse, DeleteTagResponse, GetTagResponse, UpdateTagResponse, }; use crate::model::response::{BasicMessage, TagApi}; -use crate::tags::service; use crate::util::update_last_request_time; +use super::service; + #[get("/")] pub fn get_tag( id: u32, diff --git a/src/tags/repository.rs b/src/tags/repository.rs index a4edfd2..f380b78 100644 --- a/src/tags/repository.rs +++ b/src/tags/repository.rs @@ -73,8 +73,6 @@ pub fn get_tags_on_file( let rows = pst.query_map(rusqlite::params![file_id], tag_mapper)?; let mut tags: Vec = Vec::new(); for tag_res in rows { - // I know it's probably bad style, but I'm laughing too hard at the double question mark. - // no I don't know what my code is doing and I'm glad my code reflects that tags.push(tag_res?); } Ok(tags) diff --git a/src/tags/tests/handler.rs b/src/tags/tests/handler.rs index 78346ef..5711571 100644 --- a/src/tags/tests/handler.rs +++ b/src/tags/tests/handler.rs @@ -1 +1,2 @@ -// Handler tests will be added here as needed +// Tests for tag handlers +// TODO: Add integration tests for tag HTTP endpoints diff --git a/src/tags/tests/repository.rs b/src/tags/tests/repository.rs index b5ea262..4462851 100644 --- a/src/tags/tests/repository.rs +++ b/src/tags/tests/repository.rs @@ -1,7 +1,7 @@ mod create_tag_tests { use crate::model::repository::Tag; use crate::repository::open_connection; - use crate::tags::repository; + use super::super::super::repository; use crate::test::{cleanup, init_db_folder}; #[test] @@ -24,7 +24,7 @@ mod create_tag_tests { mod get_tag_by_title_tests { use crate::model::repository::Tag; use crate::repository::open_connection; - use crate::tags::repository::{create_tag, get_tag_by_title}; + use super::super::super::repository::{create_tag, get_tag_by_title}; use crate::test::*; #[test] @@ -57,7 +57,7 @@ mod get_tag_by_title_tests { mod get_tag_by_id_tests { use crate::model::repository::Tag; use crate::repository::open_connection; - use crate::tags::repository::{create_tag, get_tag}; + use super::super::super::repository::{create_tag, get_tag}; use crate::test::{cleanup, init_db_folder}; #[test] @@ -81,7 +81,7 @@ mod get_tag_by_id_tests { mod update_tag_tests { use crate::model::repository::Tag; use crate::repository::open_connection; - use crate::tags::repository::{create_tag, get_tag, update_tag}; + use super::super::super::repository::{create_tag, get_tag, update_tag}; use crate::test::{cleanup, init_db_folder}; #[test] @@ -112,7 +112,7 @@ mod update_tag_tests { mod delete_tag_tests { use crate::repository::open_connection; - use crate::tags::repository::{create_tag, delete_tag, get_tag}; + use super::super::super::repository::{create_tag, delete_tag, get_tag}; use crate::test::{cleanup, init_db_folder}; #[test] @@ -129,7 +129,7 @@ mod delete_tag_tests { } mod get_tag_on_file_tests { - use crate::tags::repository::*; + use super::super::super::repository::*; use crate::model::file_types::FileTypes; use crate::model::repository::{FileRecord, Tag}; use crate::repository::file_repository::create_file; @@ -197,7 +197,7 @@ mod get_tag_on_file_tests { } mod remove_tag_from_file_tests { - use crate::tags::repository::*; + use super::super::super::repository::*; use crate::model::file_types::FileTypes; use crate::model::repository::{FileRecord, Tag}; use crate::repository::file_repository::create_file; @@ -233,7 +233,7 @@ mod get_tag_on_folder_tests { use crate::model::repository::{Folder, Tag}; use crate::repository::folder_repository::create_folder; use crate::repository::open_connection; - use crate::tags::repository::{add_tag_to_folder, create_tag, get_tags_on_folder}; + use super::super::super::repository::{add_tag_to_folder, create_tag, get_tags_on_folder}; use crate::test::*; #[test] @@ -294,7 +294,7 @@ mod remove_tag_from_folder_tests { use crate::model::repository::{Folder, Tag}; use crate::repository::folder_repository::create_folder; use crate::repository::open_connection; - use crate::tags::repository::{ + use super::super::super::repository::{ create_tag, get_tags_on_folder, remove_tag_from_folder, }; use crate::test::{cleanup, init_db_folder}; @@ -325,7 +325,7 @@ mod get_tags_on_files_tests { use std::collections::HashMap; use crate::{model::repository::Tag, repository::open_connection, test::*}; - use crate::tags::repository::get_tags_on_files; + use super::super::super::repository::get_tags_on_files; #[test] fn returns_proper_mapping_for_file_tags() { diff --git a/src/tags/tests/service.rs b/src/tags/tests/service.rs index c45a4d2..66f633c 100644 --- a/src/tags/tests/service.rs +++ b/src/tags/tests/service.rs @@ -1,6 +1,6 @@ mod get_tag_tests { use crate::model::error::tag_errors::GetTagError; - use crate::tags::service::{create_tag, get_tag}; + use super::super::super::service::{create_tag, get_tag}; use crate::test::*; #[test] @@ -24,7 +24,7 @@ mod get_tag_tests { mod update_tag_tests { use crate::model::error::tag_errors::UpdateTagError; use crate::model::response::TagApi; - use crate::tags::service::{create_tag, get_tag, update_tag}; + use super::super::super::service::{create_tag, get_tag, update_tag}; use crate::test::{cleanup, init_db_folder}; #[test] @@ -71,7 +71,7 @@ mod update_tag_tests { mod delete_tag_tests { use crate::model::error::tag_errors::GetTagError; - use crate::tags::service::{create_tag, delete_tag, get_tag}; + use super::super::super::service::{create_tag, delete_tag, get_tag}; use crate::test::{cleanup, init_db_folder}; #[test] @@ -91,7 +91,7 @@ mod update_file_tag_test { use crate::model::repository::FileRecord; use crate::model::response::TagApi; - use crate::tags::service::{create_tag, get_tags_on_file, update_file_tags}; + use super::super::super::service::{create_tag, get_tags_on_file, update_file_tags}; use crate::test::{cleanup, init_db_folder, now}; #[test] @@ -294,7 +294,7 @@ mod update_folder_tag_test { use crate::model::repository::Folder; use crate::model::response::TagApi; use crate::repository::{folder_repository, open_connection}; - use crate::tags::service::{create_tag, get_tags_on_folder, update_folder_tags}; + use super::super::super::service::{create_tag, get_tags_on_folder, update_folder_tags}; use crate::test::{cleanup, init_db_folder}; #[test] @@ -504,7 +504,7 @@ mod update_folder_tag_test { mod get_tags_on_file_tests { use crate::model::error::tag_errors::TagRelationError; - use crate::tags::service::get_tags_on_file; + use super::super::super::service::get_tags_on_file; use crate::test::{cleanup, init_db_folder}; #[test] @@ -518,7 +518,7 @@ mod get_tags_on_file_tests { mod get_tags_on_folder_tests { use crate::model::error::tag_errors::TagRelationError; - use crate::tags::service::get_tags_on_folder; + use super::super::super::service::get_tags_on_folder; use crate::test::{cleanup, init_db_folder}; #[test] From aea2f501600ac2baaa8168f4e6e788231f889e30 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 15 Nov 2025 18:52:03 +0000 Subject: [PATCH 09/18] Add handler tests for tag endpoints Co-authored-by: ploiu <43047560+ploiu@users.noreply.github.com> --- src/tags/tests/handler.rs | 156 +++++++++++++++++++++++++++++++++++++- 1 file changed, 154 insertions(+), 2 deletions(-) diff --git a/src/tags/tests/handler.rs b/src/tags/tests/handler.rs index 5711571..0ff5b09 100644 --- a/src/tags/tests/handler.rs +++ b/src/tags/tests/handler.rs @@ -1,2 +1,154 @@ -// Tests for tag handlers -// TODO: Add integration tests for tag HTTP endpoints +use rocket::http::{Header, Status}; +use rocket::local::blocking::Client; + +use crate::model::response::TagApi; +use crate::repository::initialize_db; +use crate::rocket; +use crate::test::*; + +fn client() -> Client { + Client::tracked(rocket()).unwrap() +} + +fn set_password() { + init_db_folder(); + let client = client(); + let uri = uri!("/api/password"); + client + .post(uri) + .body(r#"{"username":"username","password":"password"}"#) + .dispatch(); +} + +#[test] +fn get_tag_without_creds() { + initialize_db().unwrap(); + let client = client(); + let res = client.get(uri!("/tags/1")).dispatch(); + assert_eq!(res.status(), Status::Unauthorized); + cleanup(); +} + +#[test] +fn get_tag_success() { + set_password(); + create_tag_db_entry("test_tag"); + let client = client(); + let auth = Header::new("Authorization", "Basic dXNlcm5hbWU6cGFzc3dvcmQ="); + let res = client.get(uri!("/tags/1")).header(auth).dispatch(); + assert_eq!(res.status(), Status::Ok); + cleanup(); +} + +#[test] +fn get_tag_not_found() { + set_password(); + let client = client(); + let auth = Header::new("Authorization", "Basic dXNlcm5hbWU6cGFzc3dvcmQ="); + let res = client.get(uri!("/tags/999")).header(auth).dispatch(); + assert_eq!(res.status(), Status::NotFound); + cleanup(); +} + +#[test] +fn create_tag_without_creds() { + initialize_db().unwrap(); + let client = client(); + let res = client + .post(uri!("/tags")) + .body(r#"{"title":"new_tag"}"#) + .dispatch(); + assert_eq!(res.status(), Status::Unauthorized); + cleanup(); +} + +#[test] +fn create_tag_success() { + set_password(); + let client = client(); + let auth = Header::new("Authorization", "Basic dXNlcm5hbWU6cGFzc3dvcmQ="); + let res = client + .post(uri!("/tags")) + .header(auth) + .body(r#"{"title":"new_tag"}"#) + .dispatch(); + assert_eq!(res.status(), Status::Created); + cleanup(); +} + +#[test] +fn update_tag_without_creds() { + initialize_db().unwrap(); + let client = client(); + let res = client + .put(uri!("/tags")) + .body(r#"{"id":1,"title":"updated_tag"}"#) + .dispatch(); + assert_eq!(res.status(), Status::Unauthorized); + cleanup(); +} + +#[test] +fn update_tag_success() { + set_password(); + create_tag_db_entry("original_tag"); + let client = client(); + let auth = Header::new("Authorization", "Basic dXNlcm5hbWU6cGFzc3dvcmQ="); + let res = client + .put(uri!("/tags")) + .header(auth) + .body(r#"{"id":1,"title":"updated_tag"}"#) + .dispatch(); + assert_eq!(res.status(), Status::Ok); + cleanup(); +} + +#[test] +fn update_tag_not_found() { + set_password(); + let client = client(); + let auth = Header::new("Authorization", "Basic dXNlcm5hbWU6cGFzc3dvcmQ="); + let res = client + .put(uri!("/tags")) + .header(auth) + .body(r#"{"id":999,"title":"updated_tag"}"#) + .dispatch(); + assert_eq!(res.status(), Status::NotFound); + cleanup(); +} + +#[test] +fn update_tag_already_exists() { + set_password(); + create_tag_db_entry("tag1"); + create_tag_db_entry("tag2"); + let client = client(); + let auth = Header::new("Authorization", "Basic dXNlcm5hbWU6cGFzc3dvcmQ="); + let res = client + .put(uri!("/tags")) + .header(auth) + .body(r#"{"id":2,"title":"tag1"}"#) + .dispatch(); + assert_eq!(res.status(), Status::BadRequest); + cleanup(); +} + +#[test] +fn delete_tag_without_creds() { + initialize_db().unwrap(); + let client = client(); + let res = client.delete(uri!("/tags/1")).dispatch(); + assert_eq!(res.status(), Status::Unauthorized); + cleanup(); +} + +#[test] +fn delete_tag_success() { + set_password(); + create_tag_db_entry("test_tag"); + let client = client(); + let auth = Header::new("Authorization", "Basic dXNlcm5hbWU6cGFzc3dvcmQ="); + let res = client.delete(uri!("/tags/1")).header(auth).dispatch(); + assert_eq!(res.status(), Status::NoContent); + cleanup(); +} From e59639cb6e4e4bf5f43426f118e7bc2c0457fc34 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 15 Nov 2025 18:58:09 +0000 Subject: [PATCH 10/18] Use crate:: instead of super::super::super:: for cleaner imports Co-authored-by: ploiu <43047560+ploiu@users.noreply.github.com> --- src/tags/tests/repository.rs | 20 ++++++++++---------- src/tags/tests/service.rs | 14 +++++++------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/tags/tests/repository.rs b/src/tags/tests/repository.rs index 4462851..b5ea262 100644 --- a/src/tags/tests/repository.rs +++ b/src/tags/tests/repository.rs @@ -1,7 +1,7 @@ mod create_tag_tests { use crate::model::repository::Tag; use crate::repository::open_connection; - use super::super::super::repository; + use crate::tags::repository; use crate::test::{cleanup, init_db_folder}; #[test] @@ -24,7 +24,7 @@ mod create_tag_tests { mod get_tag_by_title_tests { use crate::model::repository::Tag; use crate::repository::open_connection; - use super::super::super::repository::{create_tag, get_tag_by_title}; + use crate::tags::repository::{create_tag, get_tag_by_title}; use crate::test::*; #[test] @@ -57,7 +57,7 @@ mod get_tag_by_title_tests { mod get_tag_by_id_tests { use crate::model::repository::Tag; use crate::repository::open_connection; - use super::super::super::repository::{create_tag, get_tag}; + use crate::tags::repository::{create_tag, get_tag}; use crate::test::{cleanup, init_db_folder}; #[test] @@ -81,7 +81,7 @@ mod get_tag_by_id_tests { mod update_tag_tests { use crate::model::repository::Tag; use crate::repository::open_connection; - use super::super::super::repository::{create_tag, get_tag, update_tag}; + use crate::tags::repository::{create_tag, get_tag, update_tag}; use crate::test::{cleanup, init_db_folder}; #[test] @@ -112,7 +112,7 @@ mod update_tag_tests { mod delete_tag_tests { use crate::repository::open_connection; - use super::super::super::repository::{create_tag, delete_tag, get_tag}; + use crate::tags::repository::{create_tag, delete_tag, get_tag}; use crate::test::{cleanup, init_db_folder}; #[test] @@ -129,7 +129,7 @@ mod delete_tag_tests { } mod get_tag_on_file_tests { - use super::super::super::repository::*; + use crate::tags::repository::*; use crate::model::file_types::FileTypes; use crate::model::repository::{FileRecord, Tag}; use crate::repository::file_repository::create_file; @@ -197,7 +197,7 @@ mod get_tag_on_file_tests { } mod remove_tag_from_file_tests { - use super::super::super::repository::*; + use crate::tags::repository::*; use crate::model::file_types::FileTypes; use crate::model::repository::{FileRecord, Tag}; use crate::repository::file_repository::create_file; @@ -233,7 +233,7 @@ mod get_tag_on_folder_tests { use crate::model::repository::{Folder, Tag}; use crate::repository::folder_repository::create_folder; use crate::repository::open_connection; - use super::super::super::repository::{add_tag_to_folder, create_tag, get_tags_on_folder}; + use crate::tags::repository::{add_tag_to_folder, create_tag, get_tags_on_folder}; use crate::test::*; #[test] @@ -294,7 +294,7 @@ mod remove_tag_from_folder_tests { use crate::model::repository::{Folder, Tag}; use crate::repository::folder_repository::create_folder; use crate::repository::open_connection; - use super::super::super::repository::{ + use crate::tags::repository::{ create_tag, get_tags_on_folder, remove_tag_from_folder, }; use crate::test::{cleanup, init_db_folder}; @@ -325,7 +325,7 @@ mod get_tags_on_files_tests { use std::collections::HashMap; use crate::{model::repository::Tag, repository::open_connection, test::*}; - use super::super::super::repository::get_tags_on_files; + use crate::tags::repository::get_tags_on_files; #[test] fn returns_proper_mapping_for_file_tags() { diff --git a/src/tags/tests/service.rs b/src/tags/tests/service.rs index 66f633c..c45a4d2 100644 --- a/src/tags/tests/service.rs +++ b/src/tags/tests/service.rs @@ -1,6 +1,6 @@ mod get_tag_tests { use crate::model::error::tag_errors::GetTagError; - use super::super::super::service::{create_tag, get_tag}; + use crate::tags::service::{create_tag, get_tag}; use crate::test::*; #[test] @@ -24,7 +24,7 @@ mod get_tag_tests { mod update_tag_tests { use crate::model::error::tag_errors::UpdateTagError; use crate::model::response::TagApi; - use super::super::super::service::{create_tag, get_tag, update_tag}; + use crate::tags::service::{create_tag, get_tag, update_tag}; use crate::test::{cleanup, init_db_folder}; #[test] @@ -71,7 +71,7 @@ mod update_tag_tests { mod delete_tag_tests { use crate::model::error::tag_errors::GetTagError; - use super::super::super::service::{create_tag, delete_tag, get_tag}; + use crate::tags::service::{create_tag, delete_tag, get_tag}; use crate::test::{cleanup, init_db_folder}; #[test] @@ -91,7 +91,7 @@ mod update_file_tag_test { use crate::model::repository::FileRecord; use crate::model::response::TagApi; - use super::super::super::service::{create_tag, get_tags_on_file, update_file_tags}; + use crate::tags::service::{create_tag, get_tags_on_file, update_file_tags}; use crate::test::{cleanup, init_db_folder, now}; #[test] @@ -294,7 +294,7 @@ mod update_folder_tag_test { use crate::model::repository::Folder; use crate::model::response::TagApi; use crate::repository::{folder_repository, open_connection}; - use super::super::super::service::{create_tag, get_tags_on_folder, update_folder_tags}; + use crate::tags::service::{create_tag, get_tags_on_folder, update_folder_tags}; use crate::test::{cleanup, init_db_folder}; #[test] @@ -504,7 +504,7 @@ mod update_folder_tag_test { mod get_tags_on_file_tests { use crate::model::error::tag_errors::TagRelationError; - use super::super::super::service::get_tags_on_file; + use crate::tags::service::get_tags_on_file; use crate::test::{cleanup, init_db_folder}; #[test] @@ -518,7 +518,7 @@ mod get_tags_on_file_tests { mod get_tags_on_folder_tests { use crate::model::error::tag_errors::TagRelationError; - use super::super::super::service::get_tags_on_folder; + use crate::tags::service::get_tags_on_folder; use crate::test::{cleanup, init_db_folder}; #[test] From 22d703eb7e751840422f93e659ff036249961d1e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 15 Nov 2025 19:04:47 +0000 Subject: [PATCH 11/18] Move client() and set_password() helpers to shared test utilities Co-authored-by: ploiu <43047560+ploiu@users.noreply.github.com> --- src/tags/tests/handler.rs | 16 ---------------- src/test/mod.rs | 17 +++++++++++++++++ 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/src/tags/tests/handler.rs b/src/tags/tests/handler.rs index 0ff5b09..bf4b0e5 100644 --- a/src/tags/tests/handler.rs +++ b/src/tags/tests/handler.rs @@ -1,25 +1,9 @@ use rocket::http::{Header, Status}; -use rocket::local::blocking::Client; use crate::model::response::TagApi; use crate::repository::initialize_db; -use crate::rocket; use crate::test::*; -fn client() -> Client { - Client::tracked(rocket()).unwrap() -} - -fn set_password() { - init_db_folder(); - let client = client(); - let uri = uri!("/api/password"); - client - .post(uri) - .body(r#"{"username":"username","password":"password"}"#) - .dispatch(); -} - #[test] fn get_tag_without_creds() { initialize_db().unwrap(); diff --git a/src/test/mod.rs b/src/test/mod.rs index 9cd14ec..2f92bdc 100644 --- a/src/test/mod.rs +++ b/src/test/mod.rs @@ -16,6 +16,7 @@ mod tests { use crate::service::file_service::{determine_file_type, file_dir}; use crate::tags::repository as tag_repository; use crate::temp_dir; + use rocket::local::blocking::Client; use std::fs; use std::fs::{remove_dir_all, remove_file}; use std::io::Write; @@ -24,6 +25,22 @@ mod tests { /// username:password pub static AUTH: &str = "Basic dXNlcm5hbWU6cGFzc3dvcmQ="; + /// Creates a Rocket test client for handler tests + pub fn client() -> Client { + Client::tracked(crate::rocket()).unwrap() + } + + /// Sets up authentication by creating a password in the test database + pub fn set_password() { + init_db_folder(); + let client = client(); + let uri = uri!("/api/password"); + client + .post(uri) + .body(r#"{"username":"username","password":"password"}"#) + .dispatch(); + } + pub fn init_db_folder() { // since this is just for testing, we don't need to unwrap the logging let _ = fern::Dispatch::new() From 2c06c6cc4a4b61bca3ce4d1f0f9308d7d7824a1e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 15 Nov 2025 19:20:07 +0000 Subject: [PATCH 12/18] Organize handler tests by endpoint and use AUTH constant Co-authored-by: ploiu <43047560+ploiu@users.noreply.github.com> --- src/tags/tests/handler.rs | 252 ++++++++++++++++++++------------------ 1 file changed, 134 insertions(+), 118 deletions(-) diff --git a/src/tags/tests/handler.rs b/src/tags/tests/handler.rs index bf4b0e5..5f8b43a 100644 --- a/src/tags/tests/handler.rs +++ b/src/tags/tests/handler.rs @@ -4,135 +4,151 @@ use crate::model::response::TagApi; use crate::repository::initialize_db; use crate::test::*; -#[test] -fn get_tag_without_creds() { - initialize_db().unwrap(); - let client = client(); - let res = client.get(uri!("/tags/1")).dispatch(); - assert_eq!(res.status(), Status::Unauthorized); - cleanup(); -} +mod get_tag_tests { + use super::*; -#[test] -fn get_tag_success() { - set_password(); - create_tag_db_entry("test_tag"); - let client = client(); - let auth = Header::new("Authorization", "Basic dXNlcm5hbWU6cGFzc3dvcmQ="); - let res = client.get(uri!("/tags/1")).header(auth).dispatch(); - assert_eq!(res.status(), Status::Ok); - cleanup(); -} + #[test] + fn without_creds() { + initialize_db().unwrap(); + let client = client(); + let res = client.get(uri!("/tags/1")).dispatch(); + assert_eq!(res.status(), Status::Unauthorized); + cleanup(); + } -#[test] -fn get_tag_not_found() { - set_password(); - let client = client(); - let auth = Header::new("Authorization", "Basic dXNlcm5hbWU6cGFzc3dvcmQ="); - let res = client.get(uri!("/tags/999")).header(auth).dispatch(); - assert_eq!(res.status(), Status::NotFound); - cleanup(); -} + #[test] + fn success() { + set_password(); + create_tag_db_entry("test_tag"); + let client = client(); + let auth = Header::new("Authorization", AUTH); + let res = client.get(uri!("/tags/1")).header(auth).dispatch(); + assert_eq!(res.status(), Status::Ok); + cleanup(); + } -#[test] -fn create_tag_without_creds() { - initialize_db().unwrap(); - let client = client(); - let res = client - .post(uri!("/tags")) - .body(r#"{"title":"new_tag"}"#) - .dispatch(); - assert_eq!(res.status(), Status::Unauthorized); - cleanup(); + #[test] + fn not_found() { + set_password(); + let client = client(); + let auth = Header::new("Authorization", AUTH); + let res = client.get(uri!("/tags/999")).header(auth).dispatch(); + assert_eq!(res.status(), Status::NotFound); + cleanup(); + } } -#[test] -fn create_tag_success() { - set_password(); - let client = client(); - let auth = Header::new("Authorization", "Basic dXNlcm5hbWU6cGFzc3dvcmQ="); - let res = client - .post(uri!("/tags")) - .header(auth) - .body(r#"{"title":"new_tag"}"#) - .dispatch(); - assert_eq!(res.status(), Status::Created); - cleanup(); -} +mod create_tag_tests { + use super::*; -#[test] -fn update_tag_without_creds() { - initialize_db().unwrap(); - let client = client(); - let res = client - .put(uri!("/tags")) - .body(r#"{"id":1,"title":"updated_tag"}"#) - .dispatch(); - assert_eq!(res.status(), Status::Unauthorized); - cleanup(); -} + #[test] + fn without_creds() { + initialize_db().unwrap(); + let client = client(); + let res = client + .post(uri!("/tags")) + .body(r#"{"title":"new_tag"}"#) + .dispatch(); + assert_eq!(res.status(), Status::Unauthorized); + cleanup(); + } -#[test] -fn update_tag_success() { - set_password(); - create_tag_db_entry("original_tag"); - let client = client(); - let auth = Header::new("Authorization", "Basic dXNlcm5hbWU6cGFzc3dvcmQ="); - let res = client - .put(uri!("/tags")) - .header(auth) - .body(r#"{"id":1,"title":"updated_tag"}"#) - .dispatch(); - assert_eq!(res.status(), Status::Ok); - cleanup(); + #[test] + fn success() { + set_password(); + let client = client(); + let auth = Header::new("Authorization", AUTH); + let res = client + .post(uri!("/tags")) + .header(auth) + .body(r#"{"title":"new_tag"}"#) + .dispatch(); + assert_eq!(res.status(), Status::Created); + cleanup(); + } } -#[test] -fn update_tag_not_found() { - set_password(); - let client = client(); - let auth = Header::new("Authorization", "Basic dXNlcm5hbWU6cGFzc3dvcmQ="); - let res = client - .put(uri!("/tags")) - .header(auth) - .body(r#"{"id":999,"title":"updated_tag"}"#) - .dispatch(); - assert_eq!(res.status(), Status::NotFound); - cleanup(); -} +mod update_tag_tests { + use super::*; -#[test] -fn update_tag_already_exists() { - set_password(); - create_tag_db_entry("tag1"); - create_tag_db_entry("tag2"); - let client = client(); - let auth = Header::new("Authorization", "Basic dXNlcm5hbWU6cGFzc3dvcmQ="); - let res = client - .put(uri!("/tags")) - .header(auth) - .body(r#"{"id":2,"title":"tag1"}"#) - .dispatch(); - assert_eq!(res.status(), Status::BadRequest); - cleanup(); -} + #[test] + fn without_creds() { + initialize_db().unwrap(); + let client = client(); + let res = client + .put(uri!("/tags")) + .body(r#"{"id":1,"title":"updated_tag"}"#) + .dispatch(); + assert_eq!(res.status(), Status::Unauthorized); + cleanup(); + } + + #[test] + fn success() { + set_password(); + create_tag_db_entry("original_tag"); + let client = client(); + let auth = Header::new("Authorization", AUTH); + let res = client + .put(uri!("/tags")) + .header(auth) + .body(r#"{"id":1,"title":"updated_tag"}"#) + .dispatch(); + assert_eq!(res.status(), Status::Ok); + cleanup(); + } + + #[test] + fn not_found() { + set_password(); + let client = client(); + let auth = Header::new("Authorization", AUTH); + let res = client + .put(uri!("/tags")) + .header(auth) + .body(r#"{"id":999,"title":"updated_tag"}"#) + .dispatch(); + assert_eq!(res.status(), Status::NotFound); + cleanup(); + } -#[test] -fn delete_tag_without_creds() { - initialize_db().unwrap(); - let client = client(); - let res = client.delete(uri!("/tags/1")).dispatch(); - assert_eq!(res.status(), Status::Unauthorized); - cleanup(); + #[test] + fn already_exists() { + set_password(); + create_tag_db_entry("tag1"); + create_tag_db_entry("tag2"); + let client = client(); + let auth = Header::new("Authorization", AUTH); + let res = client + .put(uri!("/tags")) + .header(auth) + .body(r#"{"id":2,"title":"tag1"}"#) + .dispatch(); + assert_eq!(res.status(), Status::BadRequest); + cleanup(); + } } -#[test] -fn delete_tag_success() { - set_password(); - create_tag_db_entry("test_tag"); - let client = client(); - let auth = Header::new("Authorization", "Basic dXNlcm5hbWU6cGFzc3dvcmQ="); - let res = client.delete(uri!("/tags/1")).header(auth).dispatch(); - assert_eq!(res.status(), Status::NoContent); - cleanup(); +mod delete_tag_tests { + use super::*; + + #[test] + fn without_creds() { + initialize_db().unwrap(); + let client = client(); + let res = client.delete(uri!("/tags/1")).dispatch(); + assert_eq!(res.status(), Status::Unauthorized); + cleanup(); + } + + #[test] + fn success() { + set_password(); + create_tag_db_entry("test_tag"); + let client = client(); + let auth = Header::new("Authorization", AUTH); + let res = client.delete(uri!("/tags/1")).header(auth).dispatch(); + assert_eq!(res.status(), Status::NoContent); + cleanup(); + } } From d2a2d80ae0adedbb05dd3812da9b522febc4e9b3 Mon Sep 17 00:00:00 2001 From: ploiu Date: Sat, 15 Nov 2025 21:38:32 +0000 Subject: [PATCH 13/18] no longer use old tag tables --- src/assets/migration/v6.sql | 3 ++ .../queries/file/get_files_by_all_tags.sql | 4 +-- .../queries/folder/get_folders_by_any_tag.sql | 19 ++++++---- .../folder/get_parent_folders_with_tags.sql | 35 ++++++++++++------- src/assets/queries/tags/add_tag_to_file.sql | 6 ++-- src/assets/queries/tags/add_tag_to_folder.sql | 6 ++-- src/assets/queries/tags/get_tags_for_file.sql | 12 ++++--- .../queries/tags/get_tags_for_files.sql | 13 ++++--- .../queries/tags/get_tags_for_folder.sql | 12 ++++--- .../queries/tags/remove_tag_from_file.sql | 10 ++++-- .../queries/tags/remove_tag_from_folder.sql | 10 ++++-- src/service/folder_service.rs | 2 +- src/tags/tests/handler.rs | 1 - src/tags/tests/repository.rs | 10 +++--- src/test/mod.rs | 4 +-- 15 files changed, 94 insertions(+), 53 deletions(-) diff --git a/src/assets/migration/v6.sql b/src/assets/migration/v6.sql index cacd95d..b5f2e76 100644 --- a/src/assets/migration/v6.sql +++ b/src/assets/migration/v6.sql @@ -136,5 +136,8 @@ where not exists ( select 1 from TaggedItems ti where ti.tagId = n.tagId and ti.fileId = n.fileId ); +drop table folders_tags; +drop table files_tags; + update metadata set value = 6 where name = 'version'; commit; \ No newline at end of file diff --git a/src/assets/queries/file/get_files_by_all_tags.sql b/src/assets/queries/file/get_files_by_all_tags.sql index 962e86d..59ae2ac 100644 --- a/src/assets/queries/file/get_files_by_all_tags.sql +++ b/src/assets/queries/file/get_files_by_all_tags.sql @@ -8,8 +8,8 @@ select group_concat(t.title) from FileRecords f - join Files_Tags ft on f.id = ft.fileRecordId - join Tags t on ft.tagId = t.id + join TaggedItems ti on f.id = ti.fileId + join Tags t on ti.tagId = t.id left join main.Folder_Files FF on f.id = FF.fileId where t.title in (?1) diff --git a/src/assets/queries/folder/get_folders_by_any_tag.sql b/src/assets/queries/folder/get_folders_by_any_tag.sql index 76aae6f..8900944 100644 --- a/src/assets/queries/folder/get_folders_by_any_tag.sql +++ b/src/assets/queries/folder/get_folders_by_any_tag.sql @@ -1,6 +1,13 @@ -select f.id, f.name, f.parentId, group_concat(t.title) -from folders f - join Folders_Tags ft on f.id = ft.folderId - join tags t on t.id = ft.tagId -where t.title in (?1) -group by f.id; +select + f.id, + f.name, + f.parentId, + group_concat(t.title) +from + folders f + join TaggedItems ti on ti.folderId = f.id + join tags t on t.id = ti.tagId +where + t.title in (?1) +group by + f.id; \ No newline at end of file diff --git a/src/assets/queries/folder/get_parent_folders_with_tags.sql b/src/assets/queries/folder/get_parent_folders_with_tags.sql index 52c1f4d..2a92c51 100644 --- a/src/assets/queries/folder/get_parent_folders_with_tags.sql +++ b/src/assets/queries/folder/get_parent_folders_with_tags.sql @@ -1,14 +1,25 @@ -with recursive query(id) as (values (?1) - union - select parentId - from Folders, - query - where Folders.id = query.id) -select parentId, group_concat(t.title) -from Folders - left join folders_tags ft on ft.folderId = folders.parentId - left join tags t on t.id = ft.tagId -where Folders.parentId in query +with recursive query(id) as ( + values + (?1) + union + select + parentId + from + Folders, + query + where + Folders.id = query.id +) +select + parentId, + group_concat(t.title) +from + Folders + left join TaggedItems ti on ti.folderId = folders.parentId + left join tags t on t.id = ti.tagId +where + Folders.parentId in query and parentId <> ?1 and t.title in (?2) -group by parentId; +group by + parentId; \ No newline at end of file diff --git a/src/assets/queries/tags/add_tag_to_file.sql b/src/assets/queries/tags/add_tag_to_file.sql index bdb4285..7b45246 100644 --- a/src/assets/queries/tags/add_tag_to_file.sql +++ b/src/assets/queries/tags/add_tag_to_file.sql @@ -1,2 +1,4 @@ -insert into Files_Tags(fileRecordId, tagId) -values(?1, ?2) +insert + or ignore into TaggedItems (fileId, tagId) +values + (?1, ?2) \ No newline at end of file diff --git a/src/assets/queries/tags/add_tag_to_folder.sql b/src/assets/queries/tags/add_tag_to_folder.sql index ae360c4..fe519b0 100644 --- a/src/assets/queries/tags/add_tag_to_folder.sql +++ b/src/assets/queries/tags/add_tag_to_folder.sql @@ -1,2 +1,4 @@ -insert into Folders_Tags(folderId, tagId) -values (?1, ?2) +insert + or ignore into TaggedItems (folderId, tagId) +values + (?1, ?2) \ No newline at end of file diff --git a/src/assets/queries/tags/get_tags_for_file.sql b/src/assets/queries/tags/get_tags_for_file.sql index 3e88719..2048112 100644 --- a/src/assets/queries/tags/get_tags_for_file.sql +++ b/src/assets/queries/tags/get_tags_for_file.sql @@ -1,4 +1,8 @@ -select Tags.id, Tags.title -from Tags - join Files_Tags on Tags.id = Files_Tags.tagId -where Files_Tags.fileRecordId = ?1; +select + t.*, + ti.inheritedFromId +from + Tags t + join TaggedItems ti on t.id = ti.tagId +where + ti.fileId = ?1 \ No newline at end of file diff --git a/src/assets/queries/tags/get_tags_for_files.sql b/src/assets/queries/tags/get_tags_for_files.sql index ab91303..e05fa9b 100644 --- a/src/assets/queries/tags/get_tags_for_files.sql +++ b/src/assets/queries/tags/get_tags_for_files.sql @@ -1,4 +1,9 @@ -select Files_Tags.fileRecordId, Tags.id, Tags.title -from Tags - join Files_Tags on Tags.id = Files_Tags.tagId -where Files_Tags.fileRecordId in ({}); +select + ti.fileId, + t.id, + t.title +from + TaggedItems ti + join Tags t on ti.tagId = t.id +where + ti.fileId in ({ }) \ No newline at end of file diff --git a/src/assets/queries/tags/get_tags_for_folder.sql b/src/assets/queries/tags/get_tags_for_folder.sql index f92eb70..7a6a4e3 100644 --- a/src/assets/queries/tags/get_tags_for_folder.sql +++ b/src/assets/queries/tags/get_tags_for_folder.sql @@ -1,4 +1,8 @@ -select Tags.id, Tags.title -from Tags - join Folders_Tags on Tags.id = Folders_Tags.tagId -where Folders_Tags.folderId = ?1; +select + t.*, + ti.inheritedFromId +from + Tags t + join TaggedItems ti on t.id = ti.tagId +where + ti.folderId = ?1 \ No newline at end of file diff --git a/src/assets/queries/tags/remove_tag_from_file.sql b/src/assets/queries/tags/remove_tag_from_file.sql index decfe04..b5029bc 100644 --- a/src/assets/queries/tags/remove_tag_from_file.sql +++ b/src/assets/queries/tags/remove_tag_from_file.sql @@ -1,3 +1,7 @@ -delete from Files_Tags -where fileRecordId = ?1 -and tagId = ?2 +-- removes a single non-inherited tag from a file +delete from + TaggedItems +where + fileId = ?1 + and tagId = ?2 + and inheritedFromId is null; \ No newline at end of file diff --git a/src/assets/queries/tags/remove_tag_from_folder.sql b/src/assets/queries/tags/remove_tag_from_folder.sql index 0ba1b9e..071378a 100644 --- a/src/assets/queries/tags/remove_tag_from_folder.sql +++ b/src/assets/queries/tags/remove_tag_from_folder.sql @@ -1,3 +1,7 @@ -delete from Folders_Tags -where folderId = ?1 -and tagId = ?2 +-- removes a single non-inherited tag from a folder +delete from + TaggedItems +where + folderId = ?1 + and tagId = ?2 + and inheritedFromId is null; \ No newline at end of file diff --git a/src/service/folder_service.rs b/src/service/folder_service.rs index 092aa6e..3cff321 100644 --- a/src/service/folder_service.rs +++ b/src/service/folder_service.rs @@ -22,8 +22,8 @@ use crate::model::response::TagApi; use crate::model::response::folder_responses::FolderResponse; use crate::previews; use crate::repository::{folder_repository, open_connection}; -use crate::service::file_service::{check_root_dir, file_dir}; use crate::service::file_service; +use crate::service::file_service::{check_root_dir, file_dir}; use crate::tags::repository as tag_repository; use crate::tags::service as tag_service; use crate::{model, repository}; diff --git a/src/tags/tests/handler.rs b/src/tags/tests/handler.rs index 5f8b43a..58c32da 100644 --- a/src/tags/tests/handler.rs +++ b/src/tags/tests/handler.rs @@ -1,6 +1,5 @@ use rocket::http::{Header, Status}; -use crate::model::response::TagApi; use crate::repository::initialize_db; use crate::test::*; diff --git a/src/tags/tests/repository.rs b/src/tags/tests/repository.rs index b5ea262..6f89c1b 100644 --- a/src/tags/tests/repository.rs +++ b/src/tags/tests/repository.rs @@ -129,11 +129,11 @@ mod delete_tag_tests { } mod get_tag_on_file_tests { - use crate::tags::repository::*; use crate::model::file_types::FileTypes; use crate::model::repository::{FileRecord, Tag}; use crate::repository::file_repository::create_file; use crate::repository::open_connection; + use crate::tags::repository::*; use crate::test::*; #[test] @@ -197,11 +197,11 @@ mod get_tag_on_file_tests { } mod remove_tag_from_file_tests { - use crate::tags::repository::*; use crate::model::file_types::FileTypes; use crate::model::repository::{FileRecord, Tag}; use crate::repository::file_repository::create_file; use crate::repository::open_connection; + use crate::tags::repository::*; use crate::test::{cleanup, init_db_folder, now}; #[test] @@ -294,9 +294,7 @@ mod remove_tag_from_folder_tests { use crate::model::repository::{Folder, Tag}; use crate::repository::folder_repository::create_folder; use crate::repository::open_connection; - use crate::tags::repository::{ - create_tag, get_tags_on_folder, remove_tag_from_folder, - }; + use crate::tags::repository::{create_tag, get_tags_on_folder, remove_tag_from_folder}; use crate::test::{cleanup, init_db_folder}; #[test] @@ -324,8 +322,8 @@ mod remove_tag_from_folder_tests { mod get_tags_on_files_tests { use std::collections::HashMap; - use crate::{model::repository::Tag, repository::open_connection, test::*}; use crate::tags::repository::get_tags_on_files; + use crate::{model::repository::Tag, repository::open_connection, test::*}; #[test] fn returns_proper_mapping_for_file_tags() { diff --git a/src/test/mod.rs b/src/test/mod.rs index 2f92bdc..f3106ae 100644 --- a/src/test/mod.rs +++ b/src/test/mod.rs @@ -10,9 +10,7 @@ mod tests { use crate::model::api::FileApi; use crate::model::repository::{FileRecord, Folder, Tag}; use crate::previews; - use crate::repository::{ - file_repository, folder_repository, initialize_db, open_connection, - }; + use crate::repository::{file_repository, folder_repository, initialize_db, open_connection}; use crate::service::file_service::{determine_file_type, file_dir}; use crate::tags::repository as tag_repository; use crate::temp_dir; From 910b4d1dd664def15a43190246fbfad7f4c31709 Mon Sep 17 00:00:00 2001 From: ploiu Date: Sat, 15 Nov 2025 22:25:04 +0000 Subject: [PATCH 14/18] simplify tag search --- src/handler/file_handler.rs | 3 - src/model/error/file_errors.rs | 3 - src/repository/folder_repository.rs | 190 +--------- src/service/folder_service.rs | 519 +--------------------------- src/service/search_service.rs | 235 +------------ 5 files changed, 8 insertions(+), 942 deletions(-) diff --git a/src/handler/file_handler.rs b/src/handler/file_handler.rs index 7e979c8..98b58d6 100644 --- a/src/handler/file_handler.rs +++ b/src/handler/file_handler.rs @@ -130,9 +130,6 @@ pub fn search_files( Err(SearchFileError::DbError) => SearchFileResponse::GenericError(BasicMessage::new( "Failed to search files. Check server logs for details", )), - Err(SearchFileError::TagError) => SearchFileResponse::GenericError(BasicMessage::new( - "Failed to retrieve file tags. Check server logs for details", - )), } } diff --git a/src/model/error/file_errors.rs b/src/model/error/file_errors.rs index bbece3a..ee6d854 100644 --- a/src/model/error/file_errors.rs +++ b/src/model/error/file_errors.rs @@ -62,15 +62,12 @@ pub enum UpdateFileError { #[derive(PartialEq, Debug)] pub enum SearchFileError { DbError, - /// an issue occurred retrieving tags - TagError, } impl Display for SearchFileError { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { match self { Self::DbError => write!(f, "SearchFileError::DbError"), - Self::TagError => write!(f, "SearchFileError::TagError"), } } } diff --git a/src/repository/folder_repository.rs b/src/repository/folder_repository.rs index 4b4f358..ad9031c 100644 --- a/src/repository/folder_repository.rs +++ b/src/repository/folder_repository.rs @@ -1,5 +1,5 @@ use std::backtrace::Backtrace; -use std::collections::{HashMap, HashSet}; +use std::collections::HashSet; use rusqlite::{Connection, Rows, params}; @@ -209,73 +209,6 @@ pub fn get_all_child_folder_ids + Clone>( Ok(ids) } -pub fn get_folders_by_any_tag( - tags: &HashSet, - con: &Connection, -) -> Result, rusqlite::Error> { - // TODO look at rarray to pass a collection as a parameter (https://docs.rs/rusqlite/0.29.0/rusqlite/vtab/array/index.html) - let joined_tags = tags - .iter() - .map(|t| format!("'{}'", t.replace('\'', "''"))) - .reduce(|combined, current| format!("{combined},{current}")) - .unwrap(); - let query = include_str!("../assets/queries/folder/get_folders_by_any_tag.sql"); - let replaced_query = query.replace("?1", joined_tags.as_str()); - let mut pst = con.prepare(replaced_query.as_str()).unwrap(); - let mut folders: HashSet = HashSet::new(); - let rows = pst.query_map([], map_folder)?; - for row in rows { - folders.insert(row?); - } - Ok(folders) -} - -pub fn get_parent_folders_by_tag<'a, T: IntoIterator + Clone>( - folder_id: u32, - tags: &T, - con: &Connection, -) -> Result>, rusqlite::Error> { - let query = include_str!("../assets/queries/folder/get_parent_folders_with_tags.sql"); - // because I'm not using a rusqlite extension, I have to join the list of tags manually - let joined_tags = tags - .clone() - .into_iter() - .map(|t| format!("'{}'", t.replace('\'', "''"))) - .reduce(|combined, current| format!("{combined},{current}")) - .unwrap(); - let built_query = query.replace("?2", joined_tags.as_str()); - let mut pst = con.prepare(built_query.as_str())?; - let mut pairs: HashMap> = HashMap::new(); - let mut rows = pst.query([folder_id])?; - while let Some(row) = rows.next()? { - let folder_id: u32 = row.get(0)?; - let tags: String = row.get(1)?; - let split_tags = tags - .split(',') - .map(|s| s.to_string()) - .collect::>(); - pairs.insert(folder_id, split_tags); - } - Ok(pairs) -} - -/// returns a recursive list of ancestor (parent/grandparent/great grandparent/etc) folder IDs for the passed `folder_id` -/// This does not include the root folder id of `None`/`0` -pub fn get_ancestor_folder_ids( - folder_id: u32, - con: &Connection, -) -> Result, rusqlite::Error> { - let mut pst = con.prepare(include_str!( - "../assets/queries/folder/get_parent_folders_with_id.sql" - ))?; - let mut rows = pst.query([folder_id])?; - let mut ids: Vec = Vec::new(); - while let Some(row) = rows.next()? { - ids.push(row.get(0)?); - } - Ok(ids) -} - fn map_folder(row: &rusqlite::Row) -> Result { let id: Option = row.get(0)?; let name: String = row.get(1)?; @@ -320,76 +253,6 @@ fn get_child_files_non_root( Ok(files) } -#[cfg(test)] -mod get_folders_by_any_tag_tests { - use std::collections::HashSet; - - use rusqlite::Connection; - - use crate::model::repository::Folder; - use crate::repository::folder_repository::get_folders_by_any_tag; - use crate::repository::open_connection; - use crate::test::{cleanup, create_folder_db_entry, create_tag_folders, init_db_folder}; - - #[test] - fn returns_folders_with_any_tag() { - init_db_folder(); - create_folder_db_entry("all tags", None); // 1 - create_folder_db_entry("some tags", Some(1)); // 2 - create_folder_db_entry("no tags", None); // 3 - create_folder_db_entry("no relevant tags", None); // 4 - // tags on them folders - create_tag_folders("irrelevant", vec![2, 4]); - create_tag_folders("relevant 1", vec![1, 2]); - create_tag_folders("relevant 2", vec![1]); - let con: Connection = open_connection(); - - let res = get_folders_by_any_tag( - &HashSet::from(["relevant 1".to_string(), "relevant 2".to_string()]), - &con, - ) - .unwrap() - .into_iter() - .collect::>(); - con.close().unwrap(); - assert_eq!(2, res.len()); - assert!(res.contains(&Folder { - id: Some(1), - parent_id: None, - name: "all tags".to_string(), - })); - assert!(res.contains(&Folder { - id: Some(2), - parent_id: Some(1), - name: "some tags".to_string(), - })); - cleanup(); - } -} - -#[cfg(test)] -mod get_parent_folders_by_tag_tests { - use std::collections::HashSet; - - use crate::repository::folder_repository::get_parent_folders_by_tag; - use crate::repository::open_connection; - use crate::test::{cleanup, create_folder_db_entry, create_tag_folder, init_db_folder}; - - #[test] - fn retrieves_parent_folders() { - init_db_folder(); - create_folder_db_entry("top", None); - create_folder_db_entry("middle", Some(1)); - create_folder_db_entry("bottom", Some(2)); - create_tag_folder("tag", 1); - let con = open_connection(); - let res = get_parent_folders_by_tag(3, &[&"tag".to_string()], &con).unwrap(); - con.close().unwrap(); - assert_eq!(HashSet::from(["tag".to_string()]), *res.get(&1).unwrap()); - cleanup(); - } -} - #[cfg(test)] mod get_child_files_tests { use std::collections::HashSet; @@ -441,54 +304,3 @@ mod get_child_files_tests { cleanup(); } } - -#[cfg(test)] -mod get_ancestor_folder_ids_tests { - use super::get_ancestor_folder_ids; - use crate::{ - repository::open_connection, - test::{cleanup, create_folder_db_entry, init_db_folder}, - }; - - #[test] - fn returns_all_parents() { - init_db_folder(); - create_folder_db_entry("1", None); - create_folder_db_entry("2", Some(1)); - create_folder_db_entry("3", Some(2)); - create_folder_db_entry("4", Some(3)); - create_folder_db_entry("5", Some(4)); - let expected = vec![1, 2, 3, 4]; - let con = open_connection(); - let actual = get_ancestor_folder_ids(5, &con).unwrap(); - con.close().unwrap(); - assert_eq!(actual, expected); - cleanup(); - } - - #[test] - fn does_not_return_non_parents() { - init_db_folder(); - create_folder_db_entry("good", None); // 1 - create_folder_db_entry("good", Some(1)); // 2 - create_folder_db_entry("bad", Some(1)); // 3 - create_folder_db_entry("good", Some(2)); // 4 - create_folder_db_entry("base", Some(4)); // 5 - let con = open_connection(); - let expected = vec![1, 2, 4]; - let actual = get_ancestor_folder_ids(5, &con).unwrap(); - assert_eq!(actual, expected); - cleanup(); - } - - #[test] - fn does_not_panic_when_no_parents() { - init_db_folder(); - let con = open_connection(); - create_folder_db_entry("test", None); - let res = get_ancestor_folder_ids(1, &con); - con.close().unwrap(); - res.expect("no error should be returned if the folder does not have a parent"); - cleanup(); - } -} diff --git a/src/service/folder_service.rs b/src/service/folder_service.rs index 3cff321..7eb6b87 100644 --- a/src/service/folder_service.rs +++ b/src/service/folder_service.rs @@ -1,7 +1,6 @@ use std::backtrace::Backtrace; -use std::collections::{HashMap, HashSet}; +use std::collections::HashMap; use std::fs::{self, File}; -use std::hash::Hash; use std::path::Path; use regex::Regex; @@ -198,109 +197,6 @@ pub fn delete_folder(id: u32) -> Result<(), DeleteFolderError> { Ok(()) } -pub fn get_folders_by_any_tag( - tags: &HashSet, -) -> Result, GetFolderError> { - let con: Connection = open_connection(); - let folders = match folder_repository::get_folders_by_any_tag(tags, &con) { - Ok(f) => f, - Err(e) => { - con.close().unwrap(); - log::error!( - "Failed to pull folders by any tag. Exception is {e}\n{}", - Backtrace::force_capture() - ); - return Err(GetFolderError::DbFailure); - } - }; - con.close().unwrap(); - let mut converted_folders: HashSet = HashSet::with_capacity(folders.len()); - for folder in folders { - let tags = match tag_service::get_tags_on_folder(folder.id.unwrap()) { - Ok(t) => t, - Err(_) => return Err(GetFolderError::TagError), - }; - converted_folders.insert(FolderResponse { - id: folder.id.unwrap(), - parent_id: folder.parent_id, - name: folder.name, - path: "no path".to_string(), - folders: Vec::new(), - files: Vec::new(), - tags, - }); - } - Ok(converted_folders) -} - -/// will reduce a list of folders down to the first one that has all the tags -/// the folders passed must be all the folders retrieved in [folder_service::get_folders_by_any_tag] -pub fn reduce_folders_by_tag( - folders: &HashSet, - tags: &HashSet, -) -> Result, GetFolderError> { - // an index of the contents of condensed, to easily look up entries. - let mut condensed_list: HashMap = HashMap::new(); - // this will never change, because sometimes we need to pull folder info no longer in the condensed list if we're a child - let mut input_index: HashMap = HashMap::new(); - for folder in folders { - // I don't like having to clone all the folders, but with just references the compiler complains about reference lifetimes - condensed_list.insert(folder.id, folder.clone()); - input_index.insert(folder.id, folder); - } - let con: Connection = open_connection(); - for (folder_id, folder) in input_index.iter() { - // 1. skip if we're not in condensed_list; we were removed in an earlier step - if !condensed_list.contains_key(folder_id) { - continue; - } - // 2. get all parent folder IDs, take their tags for ourself, and remove those parents from condensed_list - let mut our_tag_titles = folder - .tags - .iter() - .map(|t| t.title.clone()) - .collect::>(); - let parents = match folder_repository::get_parent_folders_by_tag(*folder_id, &tags, &con) { - Ok(p) => p, - Err(e) => { - con.close().unwrap(); - log::error!( - "Failed to pull parent folders. Exception is {e}\n{}", - Backtrace::force_capture() - ); - return Err(GetFolderError::DbFailure); - } - }; - for (parent_id, parent_tags) in parents { - // if the parent has all of our tags, we need to remove ourself (and our children) - if contains_all(&parent_tags, tags) { - condensed_list.remove(folder_id); - // this will tell `give_children_tags` that we already have all the tags (which we do because our parent does), so all the children get removed - our_tag_titles = parent_tags; - break; - } - parent_tags.into_iter().for_each(|t| { - our_tag_titles.insert(t); - }); - condensed_list.remove(&parent_id); - } - // 3. + 4. get all children folder IDs, give them our tags, and remove ourself from condensed_list if we have children in condensed_list - if let Err(e) = - give_children_tags(&mut condensed_list, &con, *folder_id, &our_tag_titles, tags) - { - con.close().unwrap(); - return Err(e); - }; - // 5. remove ourself from condensed_list if we do not have all tags - if !contains_all(&our_tag_titles, tags) { - condensed_list.remove(folder_id); - } - } - con.close().unwrap(); - let copied: HashSet = condensed_list.into_values().collect(); - Ok(copied) -} - #[deprecated(note = "prefer to use the streaming version in preview_service")] pub async fn get_file_previews_for_folder( id: u32, @@ -384,73 +280,6 @@ pub fn download_folder(id: u32) -> Result { File::open(tarchive_dir.clone()).map_err(|_| DownloadFolderError::NotFound) } -// private functions -/// used as part of [reduce_folders_by_tag]; -/// handles giving all children our tags, and removing ourself if we have any children with tags we don't have -fn give_children_tags( - condensed_list: &mut HashMap, - con: &Connection, - folder_id: u32, - our_tag_titles: &HashSet, - tags: &HashSet, -) -> Result<(), GetFolderError> { - let all_child_folders_ids = match folder_repository::get_all_child_folder_ids(&[folder_id], con) - { - Ok(ids) => ids - .into_iter() - .filter(|id| condensed_list.contains_key(id)) - .collect::>(), - Err(e) => { - log::error!( - "Failed to retrieve all child folder IDs for {folder_id}. Exception is {e}\n{}", - Backtrace::force_capture() - ); - return Err(GetFolderError::DbFailure); - } - }; - // if we have all of the tags, remove all our children because they're not the highest - if contains_all(our_tag_titles, tags) { - for id in all_child_folders_ids.iter() { - condensed_list.remove(id); - } - return Ok(()); - } - for id in all_child_folders_ids.iter() { - let matching_folder = condensed_list.get_mut(id).unwrap(); - let matching_folder_tags = matching_folder.tags.clone(); - let combined_tag_titles = matching_folder_tags - .iter() - .map(|t| t.title.clone()) - .chain(our_tag_titles.clone().into_iter()); - let combined_tags = matching_folder_tags - .iter() - .map(|t| &t.title) - .chain(our_tag_titles.iter()) - .map(|title| TagApi { - id: None, - title: title.clone(), - }) - .collect::>(); - *matching_folder = FolderResponse { - id: matching_folder.id, - parent_id: matching_folder.parent_id, - path: matching_folder.path.clone(), - name: matching_folder.name.clone(), - folders: vec![], - files: vec![], - tags: combined_tags, - }; - // 4. remove all children who only have the same tags as us, because they're not the earliest with all tags (or they will never have all tags) - if HashSet::from_iter(combined_tag_titles) == *our_tag_titles { - condensed_list.remove(id); - } - } - if !all_child_folders_ids.is_empty() { - condensed_list.remove(&folder_id); - } - Ok(()) -} - fn get_folder_by_id(id: Option) -> Result { // the client can pass 0 for the folder id, in which case it needs to be translated to None for the database let db_folder = if let Some(0) = id { None } else { id }; @@ -760,12 +589,6 @@ fn delete_folder_recursively(id: u32, con: &Connection) -> Result(first: &HashSet, second: &HashSet) -> bool { - let intersection: HashSet = first.intersection(second).cloned().collect(); - &intersection == second -} - #[cfg(test)] mod get_folder_tests { use crate::model::error::folder_errors::GetFolderError; @@ -911,346 +734,6 @@ mod update_folder_tests { } } -#[cfg(test)] -mod reduce_folders_by_tag_tests { - use std::collections::HashSet; - - use crate::model::response::TagApi; - use crate::model::response::folder_responses::FolderResponse; - use crate::service::folder_service::reduce_folders_by_tag; - use crate::test::{ - cleanup, create_file_db_entry, create_folder_db_entry, create_tag_folder, - create_tag_folders, init_db_folder, - }; - - #[test] - fn reduce_folders_by_tag_works() { - init_db_folder(); - create_folder_db_entry("A", None); // 1 - create_folder_db_entry("AB", Some(1)); // 2 - create_folder_db_entry("ABB", Some(1)); // 3 - create_folder_db_entry("AC", Some(2)); // 4 - create_folder_db_entry("Dummy5", None); // 5 - create_folder_db_entry("E", None); // 6 - create_folder_db_entry("EB", Some(6)); // 7 - create_folder_db_entry("EC", Some(7)); // 8 - create_folder_db_entry("Dummy9", None); // 9 - create_folder_db_entry("Dummy10", None); // 10 - create_folder_db_entry("Dummy11", None); // 11 - create_folder_db_entry("Dummy12", None); // 12 - create_folder_db_entry("Dummy13", None); // 13 - create_folder_db_entry("XA", None); // 14 - create_folder_db_entry("X", Some(14)); // 15 - create_folder_db_entry("Y", None); // 16 - create_folder_db_entry("Z", Some(16)); // 17 - create_tag_folders("tag1", vec![6, 16, 17, 2, 15, 14, 1]); - create_tag_folders("tag3", vec![4, 15, 8, 3]); - create_tag_folders("tag2", vec![2, 15, 3, 7]); - let folders = HashSet::from([ - FolderResponse { - id: 6, - parent_id: None, - path: "".to_string(), - name: "E".to_string(), - folders: vec![], - files: vec![], - tags: vec![TagApi { - id: None, - title: "tag1".to_string(), - }], - }, - FolderResponse { - id: 16, - parent_id: None, - path: "".to_string(), - name: "Y".to_string(), - folders: vec![], - files: vec![], - tags: vec![TagApi { - id: None, - title: "tag1".to_string(), - }], - }, - FolderResponse { - id: 4, - parent_id: Some(2), - path: "".to_string(), - name: "AC".to_string(), - folders: vec![], - files: vec![], - tags: vec![TagApi { - id: None, - title: "tag3".to_string(), - }], - }, - FolderResponse { - id: 17, - parent_id: Some(16), - path: "".to_string(), - name: "Z".to_string(), - folders: vec![], - files: vec![], - tags: vec![TagApi { - id: None, - title: "tag1".to_string(), - }], - }, - FolderResponse { - id: 2, - parent_id: Some(1), - path: "".to_string(), - name: "AB".to_string(), - folders: vec![], - files: vec![], - tags: vec![ - TagApi { - id: None, - title: "tag2".to_string(), - }, - TagApi { - id: None, - title: "tag1".to_string(), - }, - ], - }, - FolderResponse { - id: 15, - parent_id: Some(14), - path: "".to_string(), - name: "X".to_string(), - folders: vec![], - files: vec![], - tags: vec![ - TagApi { - id: None, - title: "tag3".to_string(), - }, - TagApi { - id: None, - title: "tag1".to_string(), - }, - TagApi { - id: None, - title: "tag2".to_string(), - }, - ], - }, - FolderResponse { - id: 8, - parent_id: Some(7), - path: "".to_string(), - name: "EC".to_string(), - folders: vec![], - files: vec![], - tags: vec![TagApi { - id: None, - title: "tag3".to_string(), - }], - }, - FolderResponse { - id: 3, - parent_id: Some(1), - path: "".to_string(), - name: "ABB".to_string(), - folders: vec![], - files: vec![], - tags: vec![ - TagApi { - id: None, - title: "tag2".to_string(), - }, - TagApi { - id: None, - title: "tag3".to_string(), - }, - ], - }, - FolderResponse { - id: 7, - parent_id: Some(6), - path: "".to_string(), - name: "EB".to_string(), - folders: vec![], - files: vec![], - tags: vec![TagApi { - id: None, - title: "tag2".to_string(), - }], - }, - FolderResponse { - id: 1, - parent_id: None, - path: "".to_string(), - name: "A".to_string(), - folders: vec![], - files: vec![], - tags: vec![TagApi { - id: None, - title: "tag1".to_string(), - }], - }, - FolderResponse { - id: 14, - parent_id: None, - path: "".to_string(), - name: "XA".to_string(), - folders: vec![], - files: vec![], - tags: vec![TagApi { - id: None, - title: "tag1".to_string(), - }], - }, - ]); - - let expected = HashSet::from([ - FolderResponse { - id: 4, - parent_id: Some(2), - path: "".to_string(), - name: "AC".to_string(), - folders: vec![], - files: vec![], - tags: vec![TagApi { - id: None, - title: "tag3".to_string(), - }], - }, - FolderResponse { - id: 8, - parent_id: Some(7), - path: "".to_string(), - name: "EC".to_string(), - folders: vec![], - files: vec![], - tags: vec![TagApi { - id: None, - title: "tag3".to_string(), - }], - }, - FolderResponse { - id: 15, - parent_id: Some(14), - path: "".to_string(), - name: "X".to_string(), - folders: vec![], - files: vec![], - tags: vec![ - TagApi { - id: None, - title: "tag1".to_string(), - }, - TagApi { - id: None, - title: "tag2".to_string(), - }, - TagApi { - id: None, - title: "tag3".to_string(), - }, - ], - }, - FolderResponse { - id: 3, - parent_id: Some(1), - path: "".to_string(), - name: "ABB".to_string(), - folders: vec![], - files: vec![], - tags: vec![ - TagApi { - id: None, - title: "tag2".to_string(), - }, - TagApi { - id: None, - title: "tag3".to_string(), - }, - ], - }, - ]) - .into_iter() - .map(|f| f.id) - .collect::>(); - - let actual = reduce_folders_by_tag( - &folders, - &HashSet::from(["tag1".to_string(), "tag2".to_string(), "tag3".to_string()]), - ) - .unwrap() - .into_iter() - .map(|f| f.id) - .collect::>(); - assert_eq!(expected, actual); - cleanup(); - } - - #[test] - fn reduce_folders_by_tag_keeps_first_folder_with_all_tags() { - init_db_folder(); - create_folder_db_entry("top", None); // 1 - create_folder_db_entry("middle", Some(1)); // 2 - create_folder_db_entry("bottom", Some(2)); // 3 - create_file_db_entry("top file", Some(1)); - create_file_db_entry("bottom file", Some(3)); - create_tag_folders("tag1", vec![1, 3]); // tag1 on top folder and bottom folder - create_tag_folder("tag2", 3); // tag2 only on bottom folder - let input_folders = HashSet::from([ - FolderResponse { - id: 2, - parent_id: Some(1), - name: "middle".to_string(), - path: "".to_string(), - folders: vec![], - files: vec![], - tags: vec![TagApi { - id: None, - title: "tag2".to_string(), - }], - }, - FolderResponse { - id: 3, - parent_id: Some(2), - name: "bottom".to_string(), - path: "".to_string(), - folders: vec![], - files: vec![], - tags: vec![TagApi { - id: None, - title: "tag1".to_string(), - }], - }, - FolderResponse { - id: 1, - parent_id: None, - name: "top".to_string(), - path: "".to_string(), - folders: vec![], - files: vec![], - tags: vec![ - TagApi { - id: None, - title: "tag1".to_string(), - }, - TagApi { - id: None, - title: "tag2".to_string(), - }, - ], - }, - ]); - let expected: HashSet = HashSet::::from([1u32]); - let actual: HashSet = - reduce_folders_by_tag(&input_folders, &HashSet::from(["tag1".to_string()])) - .unwrap() - .into_iter() - .map(|it| it.id) - .collect(); - assert_eq!(expected, actual); - cleanup(); - } -} - #[cfg(test)] mod download_folder_tests { use crate::{ diff --git a/src/service/search_service.rs b/src/service/search_service.rs index d784c8c..63fd2d1 100644 --- a/src/service/search_service.rs +++ b/src/service/search_service.rs @@ -1,17 +1,13 @@ use std::backtrace::Backtrace; -use std::collections::{HashMap, HashSet}; +use std::collections::HashSet; use itertools::Itertools; use rusqlite::Connection; use crate::model::api::FileApi; use crate::model::error::file_errors::SearchFileError; -use crate::model::repository::FileRecord; use crate::model::request::attributes::AttributeSearch; -use crate::model::response::TagApi; -use crate::model::response::folder_responses::FolderResponse; -use crate::repository::{file_repository, folder_repository, open_connection}; -use crate::service::folder_service; +use crate::repository::{file_repository, open_connection}; use crate::tags::repository as tag_repository; pub fn search_files( @@ -129,205 +125,18 @@ fn search_files_by_tags( search_tags: &HashSet, con: &Connection, ) -> Result, SearchFileError> { - let mut matching_files: HashSet = HashSet::new(); - // 1): retrieve all files from the database that have all of the tags directly on them - let files_with_all_tags: HashSet = match get_files_by_all_tags(search_tags, con) { + let retrieved = file_repository::search_files_by_tags(search_tags, &con); + let matching_files = match retrieved { Ok(f) => f, Err(e) => { log::error!( - "File search: Failed to retrieve all files by tags. Exception is {e:?}\n{}", + "Failed to search files by tags: {e:?}\n{:?}", Backtrace::force_capture() ); return Err(SearchFileError::DbError); } }; - for file in files_with_all_tags { - matching_files.insert(file); - } - // 2): retrieve all folders that have any passed tag - let folders_with_any_tag = match folder_service::get_folders_by_any_tag(search_tags) { - Ok(f) => f, - Err(_) => { - return Err(SearchFileError::TagError); - } - }; - // index of all our folders to make things easier to lookup - let mut folder_index: HashMap = HashMap::new(); - for folder in folders_with_any_tag.iter() { - folder_index.insert(folder.id, folder); - } - // 3): reduce all the folders to the first folder with all the applied tags - let reduced = match folder_service::reduce_folders_by_tag(&folders_with_any_tag, search_tags) { - Ok(folders) => folders, - Err(_) => { - log::error!("Failed to search files!\n{}", Backtrace::force_capture()); - return Err(SearchFileError::DbError); - } - }; - // 4): get all child files of all reduced folders and their children, because reduced folders have all the tags - let deduped_child_files: HashSet = match get_deduped_child_files(&reduced, con) { - Ok(files) => files, - Err(e) => { - log::error!( - "Failed to retrieve deduped child files. Exception is {e:?}\n{}", - Backtrace::force_capture() - ); - return Err(SearchFileError::DbError); - } - }; - // 5: for each folder not in reduced, retrieve all child files in all child folders that contain the remaining tags - let non_deduped_child_files: HashSet = match get_all_non_reduced_child_files( - search_tags, - &folder_index, - &folders_with_any_tag, - &reduced, - con, - ) { - Ok(files) => files, - Err(e) => { - log::error!( - "Failed to get child files for non-deduped folders. Exception is {e:?}\n{}", - Backtrace::force_capture() - ); - return Err(SearchFileError::DbError); - } - }; - for file in non_deduped_child_files { - matching_files.insert(file); - } - for file in deduped_child_files { - matching_files.insert(file); - } - Ok(matching_files) -} - -fn get_non_duped_folder_ids( - reduced: &HashSet, - folders_with_any_tag: &HashSet, - con: &Connection, -) -> Result, rusqlite::Error> { - let non_duped_base_folder_ids: HashSet = folders_with_any_tag - .difference(reduced) - .map(|f| f.id) - .collect(); - let non_duped_child_folder_ids: HashSet = - folder_repository::get_all_child_folder_ids(&non_duped_base_folder_ids, con)? - .into_iter() - .collect(); - let non_duped_folder_ids: HashSet = non_duped_base_folder_ids - .union(&non_duped_child_folder_ids) - .copied() - .collect(); - Ok(non_duped_folder_ids) -} - -fn get_files_by_all_tags( - search_tags: &HashSet, - con: &Connection, -) -> Result, rusqlite::Error> { - let mut converted_files: HashSet = HashSet::new(); - let files = file_repository::search_files_by_tags(search_tags, con)?; - for file in files { - let tags: Vec = tag_repository::get_tags_on_file(file.id.unwrap(), con)? - .into_iter() - .map(TagApi::from) - .collect(); - let api = FileApi::from_with_tags(file, tags); - converted_files.insert(api); - } - Ok(converted_files) -} - -fn get_deduped_child_files( - reduced: &HashSet, - con: &Connection, -) -> Result, rusqlite::Error> { - let reduced_ids: Vec = reduced.iter().map(|f| f.id).collect(); - let all_relevant_folder_ids: HashSet = - folder_repository::get_all_child_folder_ids(&reduced_ids, con)? - .into_iter() - .chain(reduced_ids) - .collect(); - let deduped_child_files = get_child_files(&all_relevant_folder_ids, con)?; - Ok(deduped_child_files) -} - -fn get_child_files( - ids: &HashSet, - con: &Connection, -) -> Result, rusqlite::Error> { - let files: HashSet = if ids.is_empty() { - HashSet::new() - } else { - folder_repository::get_child_files(ids.clone(), con)? - .into_iter() - .collect() - }; - let mut converted: HashSet = HashSet::new(); - for file in files { - let tags = tag_repository::get_tags_on_file(file.id.unwrap(), con)? - .into_iter() - .map(TagApi::from) - .collect(); - converted.insert(FileApi::from_with_tags(file, tags)); - } - Ok(converted) -} - -/// recursively retrieves all child files with the remaining tags in `search_tags` for each folder in `folders_with_any_tag` -fn get_all_non_reduced_child_files( - search_tags: &HashSet, - folder_index: &HashMap, - folders_with_any_tag: &HashSet, - reduced: &HashSet, - con: &Connection, -) -> Result, rusqlite::Error> { - let non_duped_folder_ids: HashSet = - get_non_duped_folder_ids(reduced, folders_with_any_tag, con)?; - // 5.1) retrieve all child files of all child folders (+ original folder) using method in #4.2 above - let remaining_child_files: HashSet = get_child_files(&non_duped_folder_ids, con)? - .into_iter() - .collect(); - let mut final_files: HashSet = HashSet::new(); - for file in remaining_child_files { - // TODO the file might not have a direct parent folder in the index, but could still have an ancestor folder in the index - // TODO is_ancestor_of method in repository layer, that takes a folder id and file id, and returns true if folder is an ancestor of the file - let parent_id = file.folder_id.unwrap_or_default(); - let parent_tags: HashSet = if let Some(parent_folder) = folder_index.get(&parent_id) - { - parent_folder - .tags - .iter() - .map(|it| it.title.clone()) - .collect() - } else { - // direct parent isn't in the index, meaning this file has a searched tag but a grandparent has other searched tags. We need to find which parent that was and return those tags - let parent_ids = folder_index.keys(); - let mut all_ancestor_tags: HashSet = HashSet::new(); - for parent_id in parent_ids { - if is_ancestor_of(*parent_id, &file, con)? { - let tag_titles = folder_index - .get(parent_id) - .expect("parent id somehow disappeared from map") - .tags - .iter() - .map(|it| it.title.clone()); - all_ancestor_tags.extend(tag_titles); - } - } - all_ancestor_tags - }; - // parent folder has all the tags, we don't need to check further files - if &parent_tags == search_tags { - continue; - } - let missing_tags: HashSet<&String> = search_tags.difference(&parent_tags).collect(); - let file_tags: HashSet<&String> = file.tags.iter().map(|tag| &tag.title).collect(); - if missing_tags == file_tags { - final_files.insert(file); - } - } - Ok(final_files) + Ok(matching_files.into_iter().map(|it| it.into()).collect()) } fn search_files_by_attributes( @@ -345,38 +154,6 @@ fn search_files_by_attributes( }) } -/// Checks if the passed `potential_ancestor_id` is a parent/grandparent/great grandparent/etc of the passed `file` -/// If the file has no parent id or `potential_ancestor_id` == `file.folder_id`, no database call is made. Otherwise, the database is -/// checked to see if `potential_ancestor_id` is an ancestor. -/// -/// This function does not close the connection passed to it. -fn is_ancestor_of( - potential_ancestor_id: u32, - file: &FileApi, - con: &Connection, -) -> Result { - if let Some(direct_parent_id) = file.folder_id { - // avoid having to make a db call if the potential ancestor is a direct parent - if direct_parent_id == potential_ancestor_id { - Ok(true) - } else { - match folder_repository::get_ancestor_folder_ids(direct_parent_id, con) { - Ok(parent_ids) => Ok(parent_ids.contains(&potential_ancestor_id)), - Err(e) => { - log::error!( - "Failed to get ancestor folder ids. Exception is {e}\n{}", - Backtrace::force_capture() - ); - Err(e) - } - } - } - } else { - // file is at root, so no folder will ever be its parent unless the ancestor id is also root - Ok(potential_ancestor_id == 0) - } -} - #[cfg(test)] mod search_files_tests { use std::collections::HashSet; From bf605e6bceeaa38b2ce0f568f602943a44fd494e Mon Sep 17 00:00:00 2001 From: ploiu Date: Sun, 16 Nov 2025 14:21:45 +0000 Subject: [PATCH 15/18] start of using TaggedItem in the codebase --- src/assets/migration/v6.sql | 213 ++++++++++++------ src/assets/queries/tags/get_tags_for_file.sql | 2 +- .../queries/tags/get_tags_for_folder.sql | 2 +- .../queries/tags/remove_tag_from_file.sql | 2 +- .../queries/tags/remove_tag_from_folder.sql | 2 +- src/model/repository/mod.rs | 15 ++ src/tags/repository.rs | 23 +- src/tags/service.rs | 8 +- src/tags/tests/repository.rs | 10 +- src/test/mod.rs | 14 +- 10 files changed, 198 insertions(+), 93 deletions(-) diff --git a/src/assets/migration/v6.sql b/src/assets/migration/v6.sql index b5f2e76..881dcae 100644 --- a/src/assets/migration/v6.sql +++ b/src/assets/migration/v6.sql @@ -7,7 +7,7 @@ create table TaggedItems ( fileId integer references FileRecords(id) on delete cascade, folderId integer references Folders(id) on delete cascade, -- items can only ever inherit tags from an ancestor folder. When this inherited folder is deleted, this tag should be removed too since it's no longer inherited - inheritedFromId integer references Folders(id) on delete cascade default null, + impliedFromId integer references Folders(id) on delete cascade default null, -- make sure that either a file or a folder was tagged check ((fileId is not null) != (folderId is not null)) ); @@ -48,96 +48,169 @@ from if ai is helpful for anything, it's providing an example that I can adapt while I properly read how recursive sql queries work. Previously, I was flailing around. It helps me if I think of it as a do while loop and temporary named queries / functions */ -with recursive --- traverse the ancestor tree and track depth +with recursive -- traverse the ancestor tree and track depth ancestors(folderId, ancestorId, depth) as ( -- base case: select all folders that have a parent - select id as folderId, parentId as ancestorId, 1 as depth - from folders - where parentId is not null - - union all - - -- iteration: keep retrieving parents from base case until there are no more parents - select a.folderId, f.parentId as ancestorId, a.depth + 1 - from ancestors a - join folders f on f.id = a.ancestorId - where f.parentId is not null + select + id as folderId, + parentId as ancestorId, + 1 as depth + from + folders + where + parentId is not null + union + all -- iteration: keep retrieving parents from base case until there are no more parents + select + a.folderId, + f.parentId as ancestorId, + a.depth + 1 + from + ancestors a + join folders f on f.id = a.ancestorId + where + f.parentId is not null ), -- include all tags with fetched ancestors ancestorTags as ( - select a.folderId, ft.tagId, a.ancestorId, a.depth - from ancestors a - join folders_tags ft on ft.folderId = a.ancestorId + select + a.folderId, + ft.tagId, + a.ancestorId, + a.depth + from + ancestors a + join folders_tags ft on ft.folderId = a.ancestorId ), -- iterate through all retrieved ancestors. For each entry, find the tag on the ancestor with the lowest depth nearestTags as ( - select at.folderId, at.tagId, at.ancestorId - from ancestorTags at - where at.ancestorId = ( - -- compare on the current row and find the nearest ancestor - select at2.ancestorId - from ancestorTags at2 - where at2.folderId = at.folderId - and at2.tagId = at.tagId - order by at2.depth asc - limit 1 - ) -) - --- now that we have our functions, we can invoke nearestTags to get all the inherited tags and insert them -insert into TaggedItems(tagId, folderId, inheritedFromId) -select n.tagId, n.folderId, n.ancestorId -from nearestTags n --- important to not include tags that are directly on the folder -where not exists ( - select 1 from TaggedItems ti where ti.tagId = n.tagId and ti.folderId = n.folderId -); + select + at.folderId, + at.tagId, + at.ancestorId + from + ancestorTags at + where + at.ancestorId = ( + -- compare on the current row and find the nearest ancestor + select + at2.ancestorId + from + ancestorTags at2 + where + at2.folderId = at.folderId + and at2.tagId = at.tagId + order by + at2.depth asc + limit + 1 + ) +) -- now that we have our functions, we can invoke nearestTags to get all the inherited tags and insert them +insert into + TaggedItems(tagId, folderId, impliedFromId) +select + n.tagId, + n.folderId, + n.ancestorId +from + nearestTags n -- important to not include tags that are directly on the folder +where + not exists ( + select + 1 + from + TaggedItems ti + where + ti.tagId = n.tagId + and ti.folderId = n.folderId + ); -- populate inherited tags for files: for each file, walk its containing folder(s)' ancestor chain -- and pick the nearest ancestor that provides a tag, then insert an inherited row for the file -with recursive -ancestors(fileId, directFolderId, ancestorId, depth) as ( +with recursive ancestors(fileId, directFolderId, ancestorId, depth) as ( -- base: each file's direct containing folder is the first ancestor (so tags on the folder itself are inherited) - select ff.fileId, ff.folderId, ff.folderId as ancestorId, 1 as depth - from Folder_Files ff - - union all - - -- climb up the folder parent chain - select fa.fileId, fa.directFolderId, f.parentId as ancestorId, fa.depth + 1 - from ancestors fa - join Folders f on f.id = fa.ancestorId - where f.parentId is not null + select + ff.fileId, + ff.folderId, + ff.folderId as ancestorId, + 1 as depth + from + Folder_Files ff + union + all -- climb up the folder parent chain + select + fa.fileId, + fa.directFolderId, + f.parentId as ancestorId, + fa.depth + 1 + from + ancestors fa + join Folders f on f.id = fa.ancestorId + where + f.parentId is not null ), -- join the discovered ancestors to tags present on those ancestor folders ancestorTags as ( - select fa.fileId, ft.tagId, fa.ancestorId, fa.depth - from ancestors fa - join Folders_Tags ft on ft.folderId = fa.ancestorId + select + fa.fileId, + ft.tagId, + fa.ancestorId, + fa.depth + from + ancestors fa + join Folders_Tags ft on ft.folderId = fa.ancestorId ), -- for each (file,tag) choose the nearest ancestor (smallest depth) nearestTags as ( - select cft.fileId, cft.tagId, cft.ancestorId - from ancestorTags cft - where cft.ancestorId = ( - select cft2.ancestorId - from ancestorTags cft2 - where cft2.fileId = cft.fileId and cft2.tagId = cft.tagId - order by cft2.depth asc - limit 1 - ) + select + cft.fileId, + cft.tagId, + cft.ancestorId + from + ancestorTags cft + where + cft.ancestorId = ( + select + cft2.ancestorId + from + ancestorTags cft2 + where + cft2.fileId = cft.fileId + and cft2.tagId = cft.tagId + order by + cft2.depth asc + limit + 1 + ) ) - -insert into TaggedItems(tagId, fileId, inheritedFromId) -select n.tagId, n.fileId, n.ancestorId -from nearestTags n -where not exists ( - select 1 from TaggedItems ti where ti.tagId = n.tagId and ti.fileId = n.fileId -); +insert into + TaggedItems(tagId, fileId, impliedFromId) +select + n.tagId, + n.fileId, + n.ancestorId +from + nearestTags n +where + not exists ( + select + 1 + from + TaggedItems ti + where + ti.tagId = n.tagId + and ti.fileId = n.fileId + ); drop table folders_tags; + drop table files_tags; -update metadata set value = 6 where name = 'version'; +update + metadata +set + value = 6 +where + name = 'version'; + commit; \ No newline at end of file diff --git a/src/assets/queries/tags/get_tags_for_file.sql b/src/assets/queries/tags/get_tags_for_file.sql index 2048112..311529a 100644 --- a/src/assets/queries/tags/get_tags_for_file.sql +++ b/src/assets/queries/tags/get_tags_for_file.sql @@ -1,6 +1,6 @@ select t.*, - ti.inheritedFromId + ti.impliedFromId from Tags t join TaggedItems ti on t.id = ti.tagId diff --git a/src/assets/queries/tags/get_tags_for_folder.sql b/src/assets/queries/tags/get_tags_for_folder.sql index 7a6a4e3..70baa4e 100644 --- a/src/assets/queries/tags/get_tags_for_folder.sql +++ b/src/assets/queries/tags/get_tags_for_folder.sql @@ -1,6 +1,6 @@ select t.*, - ti.inheritedFromId + ti.impliedFromId from Tags t join TaggedItems ti on t.id = ti.tagId diff --git a/src/assets/queries/tags/remove_tag_from_file.sql b/src/assets/queries/tags/remove_tag_from_file.sql index b5029bc..8d8e253 100644 --- a/src/assets/queries/tags/remove_tag_from_file.sql +++ b/src/assets/queries/tags/remove_tag_from_file.sql @@ -4,4 +4,4 @@ delete from where fileId = ?1 and tagId = ?2 - and inheritedFromId is null; \ No newline at end of file + and impliedFromId is null; \ No newline at end of file diff --git a/src/assets/queries/tags/remove_tag_from_folder.sql b/src/assets/queries/tags/remove_tag_from_folder.sql index 071378a..38632d1 100644 --- a/src/assets/queries/tags/remove_tag_from_folder.sql +++ b/src/assets/queries/tags/remove_tag_from_folder.sql @@ -4,4 +4,4 @@ delete from where folderId = ?1 and tagId = ?2 - and inheritedFromId is null; \ No newline at end of file + and impliedFromId is null; \ No newline at end of file diff --git a/src/model/repository/mod.rs b/src/model/repository/mod.rs index 82b7f70..36e5b30 100644 --- a/src/model/repository/mod.rs +++ b/src/model/repository/mod.rs @@ -40,6 +40,21 @@ pub struct Tag { pub title: String, } +/// represents a tag on a file or a folder, with optional implication. +/// These are not meant to ever be created outside of a database query retrieving it from the database +/// +/// [`file_id`] _or_ [`folder_id`] will be [`None`], but never both. [`implied_from_id`] will be None if the tag is explicitly on the item +pub struct TaggedItem { + /// the database id of this specific entry + pub id: u32, + /// if present, the id of the file this tag exists on. mutually exclusive with folder_id + pub file_id: Option, + /// if present, the id of the folder this tag exists on. mutually exclusive with file_id + pub folder_id: Option, + /// if present, the folder that implicates this tag on the file/folder this tag applies to + pub implied_from_id: Option, +} + impl From<&FileApi> for FileRecord { fn from(value: &FileApi) -> Self { let create_date = value diff --git a/src/tags/repository.rs b/src/tags/repository.rs index f380b78..8cbaa44 100644 --- a/src/tags/repository.rs +++ b/src/tags/repository.rs @@ -40,6 +40,15 @@ pub fn get_tag_by_title( } } +/// retrieves a tag from the database with the passed `id` +/// +/// # Parameters +/// - `id`: the unique identifier of the tag to retrieve +/// - `con`: the database connection to use. Callers must handle closing the connection +/// +/// # Returns +/// - `Ok(repository::Tag)`: the tag with the specified ID if the tag exists +/// - `Err(rusqlite::Error)`: if there was an error during the database operation, including if no tag with the specified ID exists pub fn get_tag(id: u32, con: &Connection) -> Result { let mut pst = con.prepare(include_str!("../assets/queries/tags/get_by_id.sql"))?; pst.query_row(rusqlite::params![id], tag_mapper) @@ -59,7 +68,11 @@ pub fn delete_tag(id: u32, con: &Connection) -> Result<(), rusqlite::Error> { } /// the caller of this function will need to make sure the tag already exists and isn't already on the file -pub fn add_tag_to_file(file_id: u32, tag_id: u32, con: &Connection) -> Result<(), rusqlite::Error> { +pub fn add_explicit_tag_to_file( + file_id: u32, + tag_id: u32, + con: &Connection, +) -> Result<(), rusqlite::Error> { let mut pst = con.prepare(include_str!("../assets/queries/tags/add_tag_to_file.sql"))?; pst.execute(rusqlite::params![file_id, tag_id])?; Ok(()) @@ -68,7 +81,7 @@ pub fn add_tag_to_file(file_id: u32, tag_id: u32, con: &Connection) -> Result<() pub fn get_tags_on_file( file_id: u32, con: &Connection, -) -> Result, rusqlite::Error> { +) -> Result, rusqlite::Error> { let mut pst = con.prepare(include_str!("../assets/queries/tags/get_tags_for_file.sql"))?; let rows = pst.query_map(rusqlite::params![file_id], tag_mapper)?; let mut tags: Vec = Vec::new(); @@ -81,7 +94,7 @@ pub fn get_tags_on_file( pub fn get_tags_on_files( file_ids: Vec, con: &Connection, -) -> Result>, rusqlite::Error> { +) -> Result>, rusqlite::Error> { struct TagFile { file_id: u32, tag_id: u32, @@ -134,7 +147,7 @@ pub fn remove_tag_from_file( Ok(()) } -pub fn add_tag_to_folder( +pub fn add_explicit_tag_to_folder( folder_id: u32, tag_id: u32, con: &Connection, @@ -147,7 +160,7 @@ pub fn add_tag_to_folder( pub fn get_tags_on_folder( folder_id: u32, con: &Connection, -) -> Result, rusqlite::Error> { +) -> Result, rusqlite::Error> { let mut pst = con.prepare(include_str!( "../assets/queries/tags/get_tags_for_folder.sql" ))?; diff --git a/src/tags/service.rs b/src/tags/service.rs index 8c09032..8a9f2ab 100644 --- a/src/tags/service.rs +++ b/src/tags/service.rs @@ -219,7 +219,7 @@ pub fn update_file_tags(file_id: u32, tags: Vec) -> Result<(), TagRelati if added_tag_ids.contains(&tag_id) { continue; } - if let Err(e) = tag_repository::add_tag_to_file(file_id, tag_id, &con) { + if let Err(e) = tag_repository::add_explicit_tag_to_file(file_id, tag_id, &con) { log::error!( "Failed to add tag to file with id {file_id}! Error is {e:?}\n{}", Backtrace::force_capture() @@ -245,7 +245,7 @@ pub fn update_file_tags(file_id: u32, tags: Vec) -> Result<(), TagRelati if added_tag_ids.contains(&tag_id) { continue; } - if let Err(e) = tag_repository::add_tag_to_file(file_id, tag_id, &con) { + if let Err(e) = tag_repository::add_explicit_tag_to_file(file_id, tag_id, &con) { log::error!( "Failed to add tag to file with id {file_id}! Error is {e:?}\n{}", Backtrace::force_capture() @@ -311,7 +311,7 @@ pub fn update_folder_tags(folder_id: u32, tags: Vec) -> Result<(), TagRe if added_tag_ids.contains(&tag_id) { continue; } - if let Err(e) = tag_repository::add_tag_to_folder(folder_id, tag_id, &con) { + if let Err(e) = tag_repository::add_explicit_tag_to_folder(folder_id, tag_id, &con) { log::error!( "Failed to add tags to folder with id {folder_id}! Error is {e:?}\n{}", Backtrace::force_capture() @@ -341,7 +341,7 @@ pub fn update_folder_tags(folder_id: u32, tags: Vec) -> Result<(), TagRe if added_tag_ids.contains(&tag_id) { continue; } - if let Err(e) = tag_repository::add_tag_to_folder(folder_id, tag_id, &con) { + if let Err(e) = tag_repository::add_explicit_tag_to_folder(folder_id, tag_id, &con) { log::error!( "Failed to add tags to folder with id {folder_id}! Error is {e:?}\n{}", Backtrace::force_capture() diff --git a/src/tags/tests/repository.rs b/src/tags/tests/repository.rs index 6f89c1b..93782cf 100644 --- a/src/tags/tests/repository.rs +++ b/src/tags/tests/repository.rs @@ -154,8 +154,8 @@ mod get_tag_on_file_tests { &con, ) .unwrap(); - add_tag_to_file(1, 1, &con).unwrap(); - add_tag_to_file(1, 2, &con).unwrap(); + add_explicit_tag_to_file(1, 1, &con).unwrap(); + add_explicit_tag_to_file(1, 2, &con).unwrap(); let res = get_tags_on_file(1, &con).unwrap(); con.close().unwrap(); assert_eq!( @@ -233,7 +233,7 @@ mod get_tag_on_folder_tests { use crate::model::repository::{Folder, Tag}; use crate::repository::folder_repository::create_folder; use crate::repository::open_connection; - use crate::tags::repository::{add_tag_to_folder, create_tag, get_tags_on_folder}; + use crate::tags::repository::{add_explicit_tag_to_folder, create_tag, get_tags_on_folder}; use crate::test::*; #[test] @@ -251,8 +251,8 @@ mod get_tag_on_folder_tests { &con, ) .unwrap(); - add_tag_to_folder(1, 1, &con).unwrap(); - add_tag_to_folder(1, 2, &con).unwrap(); + add_explicit_tag_to_folder(1, 1, &con).unwrap(); + add_explicit_tag_to_folder(1, 2, &con).unwrap(); let res = get_tags_on_folder(1, &con).unwrap(); con.close().unwrap(); assert_eq!( diff --git a/src/test/mod.rs b/src/test/mod.rs index f3106ae..e157781 100644 --- a/src/test/mod.rs +++ b/src/test/mod.rs @@ -125,15 +125,19 @@ mod tests { pub fn create_tag_folder(name: &str, folder_id: u32) { let connection = open_connection(); let id = create_tag_db_entry(name); - tag_repository::add_tag_to_folder(folder_id, id, &connection).unwrap(); + tag_repository::add_explicit_tag_to_folder(folder_id, id, &connection).unwrap(); connection.close().unwrap(); } + pub fn inherit_tag_folder(name: &str, folder_id: u32, inherited_from: u32) { + let con = open_connection(); + } + pub fn create_tag_folders(name: &str, folder_ids: Vec) { let connection = open_connection(); let id = create_tag_db_entry(name); for folder_id in folder_ids { - tag_repository::add_tag_to_folder(folder_id, id, &connection).unwrap(); + tag_repository::add_explicit_tag_to_folder(folder_id, id, &connection).unwrap(); } connection.close().unwrap(); } @@ -141,7 +145,7 @@ mod tests { pub fn create_tag_file(name: &str, file_id: u32) { let connection = open_connection(); let id = create_tag_db_entry(name); - tag_repository::add_tag_to_file(file_id, id, &connection).unwrap(); + tag_repository::add_explicit_tag_to_file(file_id, id, &connection).unwrap(); connection.close().unwrap(); } @@ -149,7 +153,7 @@ mod tests { let connection = open_connection(); let id = create_tag_db_entry(name); for file_id in file_ids { - tag_repository::add_tag_to_file(file_id, id, &connection).unwrap(); + tag_repository::add_explicit_tag_to_file(file_id, id, &connection).unwrap(); } connection.close().unwrap(); } @@ -240,7 +244,7 @@ mod tests { let file_id = file_repository::create_file(&record, &con).unwrap(); for tag in &mut self.tags { let Tag { id, title: _ } = tag_repository::create_tag(&tag.title, &con).unwrap(); - tag_repository::add_tag_to_file(file_id, id, &con).unwrap(); + tag_repository::add_explicit_tag_to_file(file_id, id, &con).unwrap(); tag.id = Some(id); } if let Some(folder_id) = self.folder_id { From 970b7db0206aad8514b0805b73d2250177720e18 Mon Sep 17 00:00:00 2001 From: ploiu Date: Mon, 17 Nov 2025 00:58:48 +0000 Subject: [PATCH 16/18] more progress on tag changes --- src/assets/migration/v6.sql | 6 +- src/assets/queries/tags/get_tags_for_file.sql | 8 +- .../queries/tags/get_tags_for_files.sql | 7 +- .../queries/tags/get_tags_for_folder.sql | 8 +- ....sql => remove_explicit_tag_from_file.sql} | 2 +- .../queries/tags/remove_tag_from_folder.sql | 2 +- src/model/api.rs | 18 ++-- src/model/repository/mod.rs | 10 +- src/model/response/mod.rs | 26 ++++++ src/tags/repository.rs | 81 +++++++++-------- src/tags/service.rs | 91 ++++++++----------- src/tags/tests/repository.rs | 2 +- 12 files changed, 144 insertions(+), 117 deletions(-) rename src/assets/queries/tags/{remove_tag_from_file.sql => remove_explicit_tag_from_file.sql} (79%) diff --git a/src/assets/migration/v6.sql b/src/assets/migration/v6.sql index 881dcae..cde5ec6 100644 --- a/src/assets/migration/v6.sql +++ b/src/assets/migration/v6.sql @@ -7,7 +7,7 @@ create table TaggedItems ( fileId integer references FileRecords(id) on delete cascade, folderId integer references Folders(id) on delete cascade, -- items can only ever inherit tags from an ancestor folder. When this inherited folder is deleted, this tag should be removed too since it's no longer inherited - impliedFromId integer references Folders(id) on delete cascade default null, + implicitFromId integer references Folders(id) on delete cascade default null, -- make sure that either a file or a folder was tagged check ((fileId is not null) != (folderId is not null)) ); @@ -107,7 +107,7 @@ nearestTags as ( ) ) -- now that we have our functions, we can invoke nearestTags to get all the inherited tags and insert them insert into - TaggedItems(tagId, folderId, impliedFromId) + TaggedItems(tagId, folderId, implicitFromId) select n.tagId, n.folderId, @@ -184,7 +184,7 @@ nearestTags as ( ) ) insert into - TaggedItems(tagId, fileId, impliedFromId) + TaggedItems(tagId, fileId, implicitFromId) select n.tagId, n.fileId, diff --git a/src/assets/queries/tags/get_tags_for_file.sql b/src/assets/queries/tags/get_tags_for_file.sql index 311529a..77a397a 100644 --- a/src/assets/queries/tags/get_tags_for_file.sql +++ b/src/assets/queries/tags/get_tags_for_file.sql @@ -1,6 +1,10 @@ select - t.*, - ti.impliedFromId + ti.id, + ti.fileId, + ti.folderId, + ti.implicitFromId, + t.id, + t.title from Tags t join TaggedItems ti on t.id = ti.tagId diff --git a/src/assets/queries/tags/get_tags_for_files.sql b/src/assets/queries/tags/get_tags_for_files.sql index e05fa9b..40c1924 100644 --- a/src/assets/queries/tags/get_tags_for_files.sql +++ b/src/assets/queries/tags/get_tags_for_files.sql @@ -1,9 +1,12 @@ select + ti.id, ti.fileId, + ti.folderId, + ti.implicitFromId, t.id, t.title from - TaggedItems ti - join Tags t on ti.tagId = t.id + Tags t + join TaggedItems ti on t.id = ti.tagId where ti.fileId in ({ }) \ No newline at end of file diff --git a/src/assets/queries/tags/get_tags_for_folder.sql b/src/assets/queries/tags/get_tags_for_folder.sql index 70baa4e..05f2bd8 100644 --- a/src/assets/queries/tags/get_tags_for_folder.sql +++ b/src/assets/queries/tags/get_tags_for_folder.sql @@ -1,6 +1,10 @@ select - t.*, - ti.impliedFromId + ti.id, + ti.fileId, + ti.folderId, + ti.implicitFromId, + t.id, + t.title from Tags t join TaggedItems ti on t.id = ti.tagId diff --git a/src/assets/queries/tags/remove_tag_from_file.sql b/src/assets/queries/tags/remove_explicit_tag_from_file.sql similarity index 79% rename from src/assets/queries/tags/remove_tag_from_file.sql rename to src/assets/queries/tags/remove_explicit_tag_from_file.sql index 8d8e253..805f5a5 100644 --- a/src/assets/queries/tags/remove_tag_from_file.sql +++ b/src/assets/queries/tags/remove_explicit_tag_from_file.sql @@ -4,4 +4,4 @@ delete from where fileId = ?1 and tagId = ?2 - and impliedFromId is null; \ No newline at end of file + and implicitFromId is null; \ No newline at end of file diff --git a/src/assets/queries/tags/remove_tag_from_folder.sql b/src/assets/queries/tags/remove_tag_from_folder.sql index 38632d1..764f775 100644 --- a/src/assets/queries/tags/remove_tag_from_folder.sql +++ b/src/assets/queries/tags/remove_tag_from_folder.sql @@ -4,4 +4,4 @@ delete from where folderId = ?1 and tagId = ?2 - and impliedFromId is null; \ No newline at end of file + and implicitFromId is null; \ No newline at end of file diff --git a/src/model/api.rs b/src/model/api.rs index ebe3ec7..640e3db 100644 --- a/src/model/api.rs +++ b/src/model/api.rs @@ -4,7 +4,7 @@ use rocket::serde::{Deserialize, Serialize}; use crate::model::file_types::FileTypes; use crate::model::repository::FileRecord; -use crate::model::response::TagApi; +use crate::model::response::{TagApi, TaggedItemApi}; #[derive(Deserialize, Serialize, Debug, PartialEq, Eq, Hash, Clone)] #[serde(crate = "rocket::serde")] @@ -24,7 +24,7 @@ pub struct FileApi { pub folder_id: Option, /// this value may be unsafe, see [`FileApi::name`] pub name: String, - pub tags: Vec, + pub tags: Vec, // wrapped in option so api consumers don't have to send this field (these fields can't be written to after a file is uploaded) pub size: Option, #[serde(rename = "dateCreated", skip_serializing_if = "Option::is_none")] @@ -35,29 +35,23 @@ pub struct FileApi { impl FileApi { /// returns a sanitized string based on [Rocket's file name sanitization](https://api.rocket.rs/master/rocket/fs/struct.FileName.html#sanitization) - /// but with the exception of parentheses being replaced with `leftParenthese` and `rightParenthese` respectively. It's hacky, but parentheses in file - /// names are super common and don't immediately mean it's malicious /// will return None if the entire file name is unsafe pub fn name(&self) -> Option { //language=RegExp - let reserved_name_regex = Regex::new("^CON|PRN|AUX|NUL|COM[1-9]|LPT[1-9]$").unwrap(); + let reserved_name_regex = Regex::new("^(CON|PRN|AUX|NUL|COM[1-9]|LPT[1-9])$").unwrap(); //language=RegExp let banned_chars = Regex::new("(^\\.\\.|^\\./)|[/\\\\<>|:&;#?*]").unwrap(); if reserved_name_regex.is_match(&self.name.to_uppercase()) - || self.name.starts_with("..") + || self.name.contains("..") || self.name.contains("./") { return None; } - let replaced = banned_chars.replace_all(&self.name, ""); - let replaced = replaced - .to_string() - .replace('(', "leftParenthese") - .replace(')', "rightParenthese"); + let replaced = banned_chars.replace_all(&self.name, "").to_string(); Some(replaced) } - pub fn from_with_tags(file: FileRecord, tags: Vec) -> Self { + pub fn from_with_tags(file: FileRecord, tags: Vec) -> Self { let mut api: Self = file.into(); api.tags = tags; api diff --git a/src/model/repository/mod.rs b/src/model/repository/mod.rs index 36e5b30..f7987be 100644 --- a/src/model/repository/mod.rs +++ b/src/model/repository/mod.rs @@ -32,6 +32,7 @@ pub struct Folder { pub parent_id: Option, } +/// represents a tag in the Tags table of the database. When referencing a tag _on_ a file / folder, use [`TaggedItem`] instead #[derive(Debug, PartialEq, Clone)] pub struct Tag { /// the id of the tag @@ -43,7 +44,8 @@ pub struct Tag { /// represents a tag on a file or a folder, with optional implication. /// These are not meant to ever be created outside of a database query retrieving it from the database /// -/// [`file_id`] _or_ [`folder_id`] will be [`None`], but never both. [`implied_from_id`] will be None if the tag is explicitly on the item +/// [`file_id`] _or_ [`folder_id`] will be [`None`], but never both. [`implicit_from_id`] will be None if the tag is explicitly on the item +#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone)] pub struct TaggedItem { /// the database id of this specific entry pub id: u32, @@ -52,7 +54,11 @@ pub struct TaggedItem { /// if present, the id of the folder this tag exists on. mutually exclusive with file_id pub folder_id: Option, /// if present, the folder that implicates this tag on the file/folder this tag applies to - pub implied_from_id: Option, + pub implicit_from_id: Option, + /// the tag's title + pub title: String, + /// the id of the actual tag + pub tag_id: u32, } impl From<&FileApi> for FileRecord { diff --git a/src/model/response/mod.rs b/src/model/response/mod.rs index c30a50a..e9bcce7 100644 --- a/src/model/response/mod.rs +++ b/src/model/response/mod.rs @@ -25,6 +25,22 @@ pub struct TagApi { pub title: String, } +/// represents a tag _on_ a file or folder, not just a standalone tag. +/// +/// In order to maintain compatibility with existing clients, the [`id`] field matches the id of the [`Tag`], not the [`TaggedItem`]. +/// Since this will be on a file or a folder, that should be enough information to determine which record to modify or remove if needed +#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Hash, Clone)] +#[serde(crate = "rocket::serde")] +pub struct TaggedItemApi { + /// the id of the tag itself, not the TaggedItemApi. Will be `None` if it's a new tag for that item coming from a client + pub id: Option, + /// the title of the tag + pub title: String, + /// the folder this tag is implicated by. Will be None if the tag is explicit + #[serde(rename = "implicitFrom")] + pub implicit_from: Option, +} + // ---------------------------------- impl BasicMessage { @@ -57,3 +73,13 @@ impl From for TagApi { } } } + +impl From for TaggedItemApi { + fn from(value: repository::TaggedItem) -> Self { + Self { + id: Some(value.tag_id), + title: value.title, + implicit_from: value.implicit_from_id, + } + } +} diff --git a/src/tags/repository.rs b/src/tags/repository.rs index 8cbaa44..c923089 100644 --- a/src/tags/repository.rs +++ b/src/tags/repository.rs @@ -83,8 +83,8 @@ pub fn get_tags_on_file( con: &Connection, ) -> Result, rusqlite::Error> { let mut pst = con.prepare(include_str!("../assets/queries/tags/get_tags_for_file.sql"))?; - let rows = pst.query_map(rusqlite::params![file_id], tag_mapper)?; - let mut tags: Vec = Vec::new(); + let rows = pst.query_map(rusqlite::params![file_id], tagged_item_mapper)?; + let mut tags: Vec = Vec::new(); for tag_res in rows { tags.push(tag_res?); } @@ -107,41 +107,31 @@ pub fn get_tags_on_files( in_clause ); let mut pst = con.prepare(formatted_query.as_str())?; - let rows = pst.query_map([], |row| { - let file_id: u32 = row.get(0)?; - let tag_id: u32 = row.get(1)?; - let tag_title: String = row.get(2)?; - Ok(TagFile { - file_id, - tag_id, - tag_title, - }) - })?; - let mut mapped: HashMap> = HashMap::new(); - for res in rows { - let res = res?; - if let std::collections::hash_map::Entry::Vacant(e) = mapped.entry(res.file_id) { - e.insert(vec![repository::Tag { - id: res.tag_id, - title: res.tag_title, - }]); - } else { - mapped.get_mut(&res.file_id).unwrap().push(repository::Tag { - id: res.tag_id, - title: res.tag_title, - }); - } + let rows = pst.query_map([], tagged_item_mapper)?; + let mut mapped: HashMap> = HashMap::new(); + for tag in rows { + let tag = tag?; + let id = tag + .file_id + .expect("query should eliminate all non-file tags"); + mapped + .entry(id) + .and_modify(|tags| tags.push(tag.clone())) + .or_insert_with(|| vec![tag]); } Ok(mapped) } -pub fn remove_tag_from_file( +/// removes the tag from the file if that file explicitly has that tag. +/// +/// implicit tags are not removed with this function +pub fn remove_explicit_tag_from_file( file_id: u32, tag_id: u32, con: &Connection, ) -> Result<(), rusqlite::Error> { let mut pst = con.prepare(include_str!( - "../assets/queries/tags/remove_tag_from_file.sql" + "../assets/queries/tags/remove_explicit_tag_from_file.sql" ))?; pst.execute(rusqlite::params![file_id, tag_id])?; Ok(()) @@ -164,14 +154,8 @@ pub fn get_tags_on_folder( let mut pst = con.prepare(include_str!( "../assets/queries/tags/get_tags_for_folder.sql" ))?; - let rows = pst.query_map(rusqlite::params![folder_id], |row| Ok(tag_mapper(row)))?; - let mut tags: Vec = Vec::new(); - for tag_res in rows { - // I know it's probably bad style, but I'm laughing too hard at the double question mark. - // no I don't know what my code is doing and I'm glad my code reflects that - tags.push(tag_res??); - } - Ok(tags) + let rows = pst.query_map(rusqlite::params![folder_id], tagged_item_mapper)?; + rows.collect::, rusqlite::Error>>() } pub fn remove_tag_from_folder( @@ -186,8 +170,33 @@ pub fn remove_tag_from_folder( Ok(()) } +/// maps a [`repository::Tag`] from a database row fn tag_mapper(row: &rusqlite::Row) -> Result { let id: u32 = row.get(0)?; let title: String = row.get(1)?; Ok(repository::Tag { id, title }) } + +/// 1. id +/// 2. fileId +/// 3. folderId +/// 4. implicitFromId +/// 5. tagId +/// 6. title +fn tagged_item_mapper(row: &rusqlite::Row) -> Result { + let id: u32 = row.get(0)?; + let file_id: Option = row.get(1)?; + let folder_id: Option = row.get(2)?; + let implicit_from_id: Option = row.get(3)?; + let tag_id: u32 = row.get(4)?; + let title: String = row.get(5)?; + + Ok(repository::TaggedItem { + id, + file_id, + folder_id, + implicit_from_id, + tag_id, + title, + }) +} diff --git a/src/tags/service.rs b/src/tags/service.rs index 8a9f2ab..65da0e1 100644 --- a/src/tags/service.rs +++ b/src/tags/service.rs @@ -1,12 +1,14 @@ use std::backtrace::Backtrace; use std::collections::HashSet; +use itertools::Itertools; + use crate::model::error::file_errors::GetFileError; use crate::model::error::tag_errors::{ CreateTagError, DeleteTagError, GetTagError, TagRelationError, UpdateTagError, }; -use crate::model::repository; -use crate::model::response::TagApi; +use crate::model::repository::{self, TaggedItem}; +use crate::model::response::{TagApi, TaggedItemApi}; use crate::repository::open_connection; use crate::service::{file_service, folder_service}; use crate::tags::repository as tag_repository; @@ -167,6 +169,8 @@ pub fn delete_tag(id: u32) -> Result<(), DeleteTagError> { /// Updates the tags on a file by replacing all existing tags with the provided list. /// +/// Only explict tags can be managed this way. +/// /// This function will: /// 1. Remove all existing tags from the file /// 2. Add tags that already exist in the database (those with an `id`) @@ -184,7 +188,7 @@ pub fn delete_tag(id: u32) -> Result<(), DeleteTagError> { /// - `Ok(())` if the tags were successfully updated /// - `Err(TagRelationError::FileNotFound)` if the file does not exist /// - `Err(TagRelationError::DbError)` if there was a database error -pub fn update_file_tags(file_id: u32, tags: Vec) -> Result<(), TagRelationError> { +pub fn update_file_tags(file_id: u32, tags: Vec) -> Result<(), TagRelationError> { // make sure the file exists if Err(GetFileError::NotFound) == file_service::get_file_metadata(file_id) { log::error!( @@ -193,70 +197,49 @@ pub fn update_file_tags(file_id: u32, tags: Vec) -> Result<(), TagRelati ); return Err(TagRelationError::FileNotFound); } - let existing_tags = get_tags_on_file(file_id)?; - let con: rusqlite::Connection = open_connection(); - // Remove all existing tags from the file - for tag in existing_tags.iter() { + let con = open_connection(); + // instead of removing all the tags and then adding them back, we can use a HashSet or 2 to enforce a unique list in-memory without as much IO + let existing_tags: HashSet = + HashSet::from_iter(get_tags_on_file(file_id)?.into_iter()); + let tags = HashSet::from_iter(tags.into_iter()); + // we need to find 2 things: 1) tags to add 2) tags to remove + let tags_to_remove = existing_tags.difference(&tags); + let tags_to_add = tags.difference(&existing_tags); + for tag in tags_to_remove { // tags from the db will always have a non-None tag id - if let Err(e) = tag_repository::remove_tag_from_file(file_id, tag.id.unwrap(), &con) { - log::error!( - "Failed to remove tag from file with id {file_id}! Error is {e:?}\n{}", - Backtrace::force_capture() - ); - con.close().unwrap(); - return Err(TagRelationError::DbError); - } - } - - // Track which tag IDs have been added to avoid duplicates - let mut added_tag_ids: HashSet = HashSet::new(); - - // First, add all existing tags (those with an id) - let existing_tags: Vec<&TagApi> = tags.iter().filter(|t| t.id.is_some()).collect(); - for tag in existing_tags { - let tag_id = tag.id.unwrap(); - // Skip if we've already added this tag - if added_tag_ids.contains(&tag_id) { - continue; - } - if let Err(e) = tag_repository::add_explicit_tag_to_file(file_id, tag_id, &con) { + if let Err(e) = + tag_repository::remove_explicit_tag_from_file(file_id, tag.id.unwrap(), &con) + { log::error!( - "Failed to add tag to file with id {file_id}! Error is {e:?}\n{}", + "Failed to remove tags from file with id {file_id}! Error is {e:?}\n{}", Backtrace::force_capture() ); con.close().unwrap(); return Err(TagRelationError::DbError); } - added_tag_ids.insert(tag_id); } - - // Then, create and add new tags (those without an id) - let new_tags: Vec<&TagApi> = tags.iter().filter(|t| t.id.is_none()).collect(); - for tag in new_tags { - let created_tag = match create_tag(tag.title.clone()) { + for tag in tags_to_add { + let created = match create_tag(tag.title.clone()) { Ok(t) => t, - Err(_) => { + Err(e) => { con.close().unwrap(); + log::error!( + "Failed to create tag! Error is {e:?}\n{}", + Backtrace::force_capture() + ); return Err(TagRelationError::DbError); } }; - let tag_id = created_tag.id.unwrap(); - // Skip if we've already added this tag (prevents duplicates) - if added_tag_ids.contains(&tag_id) { - continue; - } - if let Err(e) = tag_repository::add_explicit_tag_to_file(file_id, tag_id, &con) { + if let Err(e) = tag_repository::add_explicit_tag_to_file(file_id, created.id.unwrap(), &con) + { + con.close().unwrap(); log::error!( - "Failed to add tag to file with id {file_id}! Error is {e:?}\n{}", - Backtrace::force_capture() + "Failed to add tag to file: {e:?}\n{}", + Backtrace::force_capture(), ); - con.close().unwrap(); return Err(TagRelationError::DbError); } - added_tag_ids.insert(tag_id); } - - con.close().unwrap(); Ok(()) } @@ -357,7 +340,7 @@ pub fn update_folder_tags(folder_id: u32, tags: Vec) -> Result<(), TagRe } /// retrieves all the tags on the file with the passed id -pub fn get_tags_on_file(file_id: u32) -> Result, TagRelationError> { +pub fn get_tags_on_file(file_id: u32) -> Result, TagRelationError> { // make sure the file exists if !file_service::check_file_exists(file_id) { log::error!( @@ -379,13 +362,12 @@ pub fn get_tags_on_file(file_id: u32) -> Result, TagRelationError> { } }; con.close().unwrap(); - let api_tags: Vec = file_tags.into_iter().map(TagApi::from).collect(); - Ok(api_tags) + Ok(file_tags.into_iter().map_into().collect()) } /// retrieves all the tags on the folder with the passed id. /// This will always be empty if requesting with the root folder id (0 or None) -pub fn get_tags_on_folder(folder_id: u32) -> Result, TagRelationError> { +pub fn get_tags_on_folder(folder_id: u32) -> Result, TagRelationError> { // make sure the folder exists if !folder_service::folder_exists(Some(folder_id)) { log::error!( @@ -407,6 +389,5 @@ pub fn get_tags_on_folder(folder_id: u32) -> Result, TagRelationErro } }; con.close().unwrap(); - let api_tags: Vec = db_tags.into_iter().map(TagApi::from).collect(); - Ok(api_tags) + Ok(db_tags.into_iter().map(TaggedItemApi::from).collect()) } diff --git a/src/tags/tests/repository.rs b/src/tags/tests/repository.rs index 93782cf..be1231e 100644 --- a/src/tags/tests/repository.rs +++ b/src/tags/tests/repository.rs @@ -221,7 +221,7 @@ mod remove_tag_from_file_tests { &con, ) .unwrap(); - remove_tag_from_file(1, 1, &con).unwrap(); + remove_explicit_tag_from_file(1, 1, &con).unwrap(); let tags = get_tags_on_file(1, &con).unwrap(); con.close().unwrap(); assert_eq!(Vec::::new(), tags); From f0973ec0ee6ed4fba0675c213b0bc9fb0eb61e9a Mon Sep 17 00:00:00 2001 From: ploiu Date: Wed, 19 Nov 2025 02:07:42 +0000 Subject: [PATCH 17/18] get the code to compile --- src/model/api.rs | 12 +-- src/model/request/folder_requests.rs | 4 +- src/model/response/folder_responses.rs | 8 +- src/model/response/mod.rs | 5 +- src/repository/file_repository.rs | 1 + src/service/file_service.rs | 12 ++- src/service/folder_service.rs | 31 +++--- src/service/search_service.rs | 75 ++++++++++---- src/tags/repository.rs | 5 - src/tags/service.rs | 22 ++-- src/tags/tests/repository.rs | 59 +++++++---- src/tags/tests/service.rs | 136 +++++++++++++++---------- src/test/mod.rs | 13 ++- 13 files changed, 236 insertions(+), 147 deletions(-) diff --git a/src/model/api.rs b/src/model/api.rs index 640e3db..787985a 100644 --- a/src/model/api.rs +++ b/src/model/api.rs @@ -4,7 +4,7 @@ use rocket::serde::{Deserialize, Serialize}; use crate::model::file_types::FileTypes; use crate::model::repository::FileRecord; -use crate::model::response::{TagApi, TaggedItemApi}; +use crate::model::response::TaggedItemApi; #[derive(Deserialize, Serialize, Debug, PartialEq, Eq, Hash, Clone)] #[serde(crate = "rocket::serde")] @@ -66,7 +66,6 @@ impl FileApi { tags: Vec::new(), size: None, date_created: None, - // TODO file_types file_type: None, } } @@ -118,15 +117,6 @@ mod update_file_request_tests { assert_eq!(".bashrc".to_string(), req.name().unwrap()); } - #[test] - fn name_replaces_parentheses() { - let req = FileApi::new(1, None, "test (1).txt".to_string()); - assert_eq!( - "test leftParenthese1rightParenthese.txt".to_string(), - req.name().unwrap() - ); - } - #[test] fn name_keeps_multiple_extensions() { let req = FileApi::new(1, None, "test.old.txt.bak".to_string()); diff --git a/src/model/request/folder_requests.rs b/src/model/request/folder_requests.rs index 46362ee..60772ae 100644 --- a/src/model/request/folder_requests.rs +++ b/src/model/request/folder_requests.rs @@ -1,6 +1,6 @@ use rocket::serde::{Deserialize, Serialize}; -use crate::model::response::TagApi; +use crate::model::response::TaggedItemApi; #[derive(Deserialize, Serialize)] #[serde(crate = "rocket::serde")] @@ -17,5 +17,5 @@ pub struct UpdateFolderRequest { pub name: String, #[serde(rename = "parentId")] pub parent_id: Option, - pub tags: Vec, + pub tags: Vec, } diff --git a/src/model/response/folder_responses.rs b/src/model/response/folder_responses.rs index 5db6b45..3f28dcc 100644 --- a/src/model/response/folder_responses.rs +++ b/src/model/response/folder_responses.rs @@ -6,7 +6,7 @@ use rocket::serde::{Deserialize, Serialize, json::Json}; use crate::model::api::FileApi; use crate::model::repository::Folder; -use crate::model::response::{BasicMessage, TagApi}; +use crate::model::response::{BasicMessage, TaggedItemApi}; type NoContent = (); @@ -20,11 +20,11 @@ pub struct FolderResponse { pub name: String, pub folders: Vec, pub files: Vec, - pub tags: Vec, + pub tags: Vec, } -impl AddAssign> for FolderResponse { - fn add_assign(&mut self, rhs: Vec) { +impl AddAssign> for FolderResponse { + fn add_assign(&mut self, rhs: Vec) { self.tags = rhs; } } diff --git a/src/model/response/mod.rs b/src/model/response/mod.rs index e9bcce7..d31fb20 100644 --- a/src/model/response/mod.rs +++ b/src/model/response/mod.rs @@ -33,7 +33,8 @@ pub struct TagApi { #[serde(crate = "rocket::serde")] pub struct TaggedItemApi { /// the id of the tag itself, not the TaggedItemApi. Will be `None` if it's a new tag for that item coming from a client - pub id: Option, + #[serde(rename = "id")] + pub tag_id: Option, /// the title of the tag pub title: String, /// the folder this tag is implicated by. Will be None if the tag is explicit @@ -77,7 +78,7 @@ impl From for TagApi { impl From for TaggedItemApi { fn from(value: repository::TaggedItem) -> Self { Self { - id: Some(value.tag_id), + tag_id: Some(value.tag_id), title: value.title, implicit_from: value.implicit_from_id, } diff --git a/src/repository/file_repository.rs b/src/repository/file_repository.rs index 05dbf21..819dbf7 100644 --- a/src/repository/file_repository.rs +++ b/src/repository/file_repository.rs @@ -128,6 +128,7 @@ pub fn search_files_by_tags( .to_string() .replace("?1", joined_tags.as_str()) .replace("?2", tags.len().to_string().as_str()); + log::debug!("built sql: {replaced_string}"); let mut pst = con.prepare(replaced_string.as_str())?; let res = pst.query_map([], map_file_all_fields)?; res.into_iter().collect() diff --git a/src/service/file_service.rs b/src/service/file_service.rs index 1c6b47e..8aa102b 100644 --- a/src/service/file_service.rs +++ b/src/service/file_service.rs @@ -687,7 +687,7 @@ mod update_file_tests { use crate::model::error::file_errors::UpdateFileError; use crate::model::file_types::FileTypes; - use crate::model::response::TagApi; + use crate::model::response::TaggedItemApi; use crate::model::response::folder_responses::FolderResponse; use crate::service::file_service::{file_dir, get_file_metadata, update_file}; use crate::service::folder_service; @@ -705,9 +705,10 @@ mod update_file_tests { id: 1, folder_id: Some(0), name: "test.txt".to_string(), - tags: vec![TagApi { - id: None, + tags: vec![TaggedItemApi { + tag_id: None, title: "tag1".to_string(), + implicit_from: None, }], size: Some(0), date_created: Some(now()), @@ -720,9 +721,10 @@ mod update_file_tests { assert_eq!(res.folder_id, None); assert_eq!( res.tags, - vec![TagApi { - id: Some(1), + vec![TaggedItemApi { + tag_id: Some(1), title: "tag1".to_string(), + implicit_from: None, }] ); assert_eq!(res.file_type, Some(FileTypes::Text)); diff --git a/src/service/folder_service.rs b/src/service/folder_service.rs index 7eb6b87..1d0cf3c 100644 --- a/src/service/folder_service.rs +++ b/src/service/folder_service.rs @@ -3,6 +3,7 @@ use std::collections::HashMap; use std::fs::{self, File}; use std::path::Path; +use itertools::Itertools; use regex::Regex; use rusqlite::Connection; @@ -15,9 +16,8 @@ use crate::model::error::folder_errors::{ UpdateFolderError, }; -use crate::model::repository::Tag; use crate::model::request::folder_requests::{CreateFolderRequest, UpdateFolderRequest}; -use crate::model::response::TagApi; +use crate::model::response::TaggedItemApi; use crate::model::response::folder_responses::FolderResponse; use crate::previews; use crate::repository::{folder_repository, open_connection}; @@ -49,9 +49,9 @@ pub fn get_folder(id: Option) -> Result { }; let mut converted_folders: Vec = Vec::new(); for child in child_folders { - let tags: Vec = + let tags: Vec = match tag_repository::get_tags_on_folder(child.id.unwrap_or(0), &con) { - Ok(t) => t.into_iter().map(|it| it.into()).collect(), + Ok(t) => t.into_iter().map_into().collect(), Err(e) => { log::error!( "Failed to retrieve tags for folder. Exception is {e:?}\n{}", @@ -538,12 +538,12 @@ fn get_files_for_folder( }; let mut result: Vec = Vec::new(); for file in child_files { - let tags: Vec = if file_tags.contains_key(&file.id.unwrap()) { + let tags = if file_tags.contains_key(&file.id.unwrap()) { file_tags.get(&file.id.unwrap()).unwrap().clone() } else { Vec::new() }; - let tags: Vec = tags.iter().map(|it| it.clone().into()).collect(); + let tags: Vec = tags.iter().cloned().map_into().collect(); result.push(FileApi::from_with_tags(file, tags)); } Ok(result) @@ -592,7 +592,7 @@ fn delete_folder_recursively(id: u32, con: &Connection) -> Result, con: &Connection, ) -> Result, SearchFileError> { - let retrieved = file_repository::search_files_by_tags(search_tags, &con); + let retrieved = file_repository::search_files_by_tags(search_tags, con); let matching_files = match retrieved { Ok(f) => f, Err(e) => { @@ -168,10 +168,10 @@ mod search_files_tests { AttributeSearch, AttributeTypes, EqualityOperator, NamedAttributes, NamedComparisonAttribute, }; - use crate::model::response::TagApi; + use crate::model::response::TaggedItemApi; use crate::test::{ cleanup, create_file_db_entry, create_folder_db_entry, create_tag_file, create_tag_files, - create_tag_folder, create_tag_folders, init_db_folder, + create_tag_folder, create_tag_folders, imply_tag_on_file, init_db_folder, }; #[test] @@ -217,13 +217,15 @@ mod search_files_tests { assert_eq!( res.tags, vec![ - TagApi { - id: Some(1), + TaggedItemApi { + tag_id: Some(1), title: "tag1".to_string(), + implicit_from: None, }, - TagApi { - id: Some(2), + TaggedItemApi { + tag_id: Some(2), title: "tag".to_string(), + implicit_from: None, } ] ); @@ -249,9 +251,10 @@ mod search_files_tests { assert_eq!(res.folder_id, None); assert_eq!( res.tags, - vec![TagApi { - id: Some(1), + vec![TaggedItemApi { + tag_id: Some(1), title: "tag".to_string(), + implicit_from: None, }] ); assert_eq!(res.file_type, Some(FileTypes::Unknown)); @@ -269,16 +272,24 @@ mod search_files_tests { create_file_db_entry("bottom file", Some(3)); create_tag_folders("tag1", vec![1, 3]); // tag1 on top folder and bottom folder create_tag_folder("tag2", 3); // tag2 only on bottom folder + imply_tag_on_file(1, 1, 1); + imply_tag_on_file(1, 2, 3); + imply_tag_on_file(2, 2, 3); // tag1 should retrieve all files let res = search_files("", vec!["tag1".to_string()], vec![].try_into().unwrap()).unwrap(); // we have to convert res to a vec in order to not care about the create date, since hash set `contains` relies on hash let res: Vec = res.iter().cloned().collect(); + log::debug!("first round: {res:?}"); assert_eq!(2, res.len()); assert!(res.contains(&FileApi { id: 1, name: "top file".to_string(), folder_id: Some(1), - tags: vec![], + tags: vec![TaggedItemApi { + tag_id: Some(1), + title: "tag1".to_string(), + implicit_from: Some(1) + }], size: Some(0), date_created: None, file_type: Some(FileTypes::Unknown) @@ -287,18 +298,41 @@ mod search_files_tests { id: 2, name: "bottom file".to_string(), folder_id: Some(3), - tags: vec![], + tags: vec![ + TaggedItemApi { + tag_id: Some(1), + title: "tag1".to_string(), + implicit_from: Some(3) + }, + TaggedItemApi { + tag_id: Some(2), + title: "tag2".to_string(), + implicit_from: Some(3) + } + ], size: Some(0), date_created: None, file_type: Some(FileTypes::Unknown) })); let res = search_files("", vec!["tag2".to_string()], vec![].try_into().unwrap()).unwrap(); let res: Vec = res.iter().cloned().collect(); + log::debug!("{res:?}"); assert!(res.contains(&FileApi { id: 2, name: "bottom file".to_string(), folder_id: Some(3), - tags: vec![], + tags: vec![ + TaggedItemApi { + tag_id: Some(1), + title: "tag1".to_string(), + implicit_from: Some(3) + }, + TaggedItemApi { + tag_id: Some(2), + title: "tag2".to_string(), + implicit_from: Some(3) + } + ], size: Some(0), date_created: None, file_type: Some(FileTypes::Unknown) @@ -314,6 +348,7 @@ mod search_files_tests { create_file_db_entry("bad", Some(1)); create_tag_folders("tag1", vec![1]); create_tag_file("tag2", 1); + imply_tag_on_file(1, 1, 1); let res: HashSet = search_files( "", vec!["tag1".to_string(), "tag2".to_string()], @@ -337,35 +372,39 @@ mod search_files_tests { id: 1, folder_id: Some(2), name: "good".to_string(), - tags: vec![TagApi { - id: None, + tags: vec![TaggedItemApi { + tag_id: Some(1), title: "file".to_string(), + implicit_from: Some(1), }], size: Some(0), date_created: Some(NaiveDateTime::default()), file_type: Some(FileTypes::Unknown), } .save_to_db(); + imply_tag_on_file(1, 1, 1); FileApi { id: 2, folder_id: Some(2), name: "bad".to_string(), - tags: vec![TagApi { - id: None, + tags: vec![TaggedItemApi { + tag_id: None, title: "something_else".to_string(), + implicit_from: None, }], size: None, date_created: None, file_type: None, } .save_to_db(); - let res = search_files( + let res: HashSet = search_files( "", vec!["top".to_string(), "file".to_string()], vec![].try_into().unwrap(), ) + .map(|it| it.iter().map(|i| i.id).collect()) .unwrap(); - let expected = HashSet::from_iter(vec![good_file]); + let expected: HashSet = HashSet::from_iter(vec![good_file.id]); assert_eq!(expected, res); cleanup(); } diff --git a/src/tags/repository.rs b/src/tags/repository.rs index c923089..8900f9b 100644 --- a/src/tags/repository.rs +++ b/src/tags/repository.rs @@ -95,11 +95,6 @@ pub fn get_tags_on_files( file_ids: Vec, con: &Connection, ) -> Result>, rusqlite::Error> { - struct TagFile { - file_id: u32, - tag_id: u32, - tag_title: String, - } let in_clause: Vec = file_ids.iter().map(|it| format!("'{it}'")).collect(); let in_clause = in_clause.join(","); let formatted_query = format!( diff --git a/src/tags/service.rs b/src/tags/service.rs index 65da0e1..eecce49 100644 --- a/src/tags/service.rs +++ b/src/tags/service.rs @@ -7,7 +7,7 @@ use crate::model::error::file_errors::GetFileError; use crate::model::error::tag_errors::{ CreateTagError, DeleteTagError, GetTagError, TagRelationError, UpdateTagError, }; -use crate::model::repository::{self, TaggedItem}; +use crate::model::repository::{self}; use crate::model::response::{TagApi, TaggedItemApi}; use crate::repository::open_connection; use crate::service::{file_service, folder_service}; @@ -200,15 +200,15 @@ pub fn update_file_tags(file_id: u32, tags: Vec) -> Result<(), Ta let con = open_connection(); // instead of removing all the tags and then adding them back, we can use a HashSet or 2 to enforce a unique list in-memory without as much IO let existing_tags: HashSet = - HashSet::from_iter(get_tags_on_file(file_id)?.into_iter()); - let tags = HashSet::from_iter(tags.into_iter()); + HashSet::from_iter(get_tags_on_file(file_id)?); + let tags = HashSet::from_iter(tags); // we need to find 2 things: 1) tags to add 2) tags to remove let tags_to_remove = existing_tags.difference(&tags); let tags_to_add = tags.difference(&existing_tags); for tag in tags_to_remove { // tags from the db will always have a non-None tag id if let Err(e) = - tag_repository::remove_explicit_tag_from_file(file_id, tag.id.unwrap(), &con) + tag_repository::remove_explicit_tag_from_file(file_id, tag.tag_id.unwrap(), &con) { log::error!( "Failed to remove tags from file with id {file_id}! Error is {e:?}\n{}", @@ -262,7 +262,10 @@ pub fn update_file_tags(file_id: u32, tags: Vec) -> Result<(), Ta /// - `Ok(())` if the tags were successfully updated /// - `Err(TagRelationError::FolderNotFound)` if the folder does not exist /// - `Err(TagRelationError::DbError)` if there was a database error -pub fn update_folder_tags(folder_id: u32, tags: Vec) -> Result<(), TagRelationError> { +pub fn update_folder_tags( + folder_id: u32, + tags: Vec, +) -> Result<(), TagRelationError> { // make sure the file exists if !folder_service::folder_exists(Some(folder_id)) { log::error!("Cannot update tags for a folder that does not exist (id {folder_id}!"); @@ -273,7 +276,8 @@ pub fn update_folder_tags(folder_id: u32, tags: Vec) -> Result<(), TagRe // Remove all existing tags from the folder for tag in existing_tags.iter() { // tags from the db will always have a non-None tag id - if let Err(e) = tag_repository::remove_tag_from_folder(folder_id, tag.id.unwrap(), &con) { + if let Err(e) = tag_repository::remove_tag_from_folder(folder_id, tag.tag_id.unwrap(), &con) + { log::error!( "Failed to remove tags from folder with id {folder_id}! Error is {e:?}\n{}", Backtrace::force_capture() @@ -287,9 +291,9 @@ pub fn update_folder_tags(folder_id: u32, tags: Vec) -> Result<(), TagRe let mut added_tag_ids: HashSet = HashSet::new(); // First, add all existing tags (those with an id) - let existing_tags: Vec<&TagApi> = tags.iter().filter(|t| t.id.is_some()).collect(); + let existing_tags: Vec<&TaggedItemApi> = tags.iter().filter(|t| t.tag_id.is_some()).collect(); for tag in existing_tags { - let tag_id = tag.id.unwrap(); + let tag_id = tag.tag_id.unwrap(); // Skip if we've already added this tag if added_tag_ids.contains(&tag_id) { continue; @@ -306,7 +310,7 @@ pub fn update_folder_tags(folder_id: u32, tags: Vec) -> Result<(), TagRe } // Then, create and add new tags (those without an id) - let new_tags: Vec<&TagApi> = tags.iter().filter(|t| t.id.is_none()).collect(); + let new_tags: Vec<&TaggedItemApi> = tags.iter().filter(|t| t.tag_id.is_none()).collect(); for tag in new_tags { let created_tag = match create_tag(tag.title.clone()) { Ok(t) => t, diff --git a/src/tags/tests/repository.rs b/src/tags/tests/repository.rs index be1231e..c82bbd1 100644 --- a/src/tags/tests/repository.rs +++ b/src/tags/tests/repository.rs @@ -130,7 +130,7 @@ mod delete_tag_tests { mod get_tag_on_file_tests { use crate::model::file_types::FileTypes; - use crate::model::repository::{FileRecord, Tag}; + use crate::model::repository::{FileRecord, TaggedItem}; use crate::repository::file_repository::create_file; use crate::repository::open_connection; use crate::tags::repository::*; @@ -160,13 +160,21 @@ mod get_tag_on_file_tests { con.close().unwrap(); assert_eq!( vec![ - Tag { + TaggedItem { id: 1, - title: "test".to_string() + tag_id: 1, + title: "test".to_string(), + file_id: Some(1), + folder_id: None, + implicit_from_id: None }, - Tag { + TaggedItem { id: 2, - title: "test2".to_string() + tag_id: 2, + title: "test2".to_string(), + file_id: Some(1), + folder_id: None, + implicit_from_id: None } ], res @@ -191,14 +199,14 @@ mod get_tag_on_file_tests { .unwrap(); let res = get_tags_on_file(1, &con).unwrap(); con.close().unwrap(); - assert_eq!(Vec::::new(), res); + assert_eq!(Vec::::new(), res); cleanup(); } } mod remove_tag_from_file_tests { use crate::model::file_types::FileTypes; - use crate::model::repository::{FileRecord, Tag}; + use crate::model::repository::{FileRecord, TaggedItem}; use crate::repository::file_repository::create_file; use crate::repository::open_connection; use crate::tags::repository::*; @@ -224,13 +232,13 @@ mod remove_tag_from_file_tests { remove_explicit_tag_from_file(1, 1, &con).unwrap(); let tags = get_tags_on_file(1, &con).unwrap(); con.close().unwrap(); - assert_eq!(Vec::::new(), tags); + assert_eq!(Vec::::new(), tags); cleanup(); } } mod get_tag_on_folder_tests { - use crate::model::repository::{Folder, Tag}; + use crate::model::repository::{Folder, TaggedItem}; use crate::repository::folder_repository::create_folder; use crate::repository::open_connection; use crate::tags::repository::{add_explicit_tag_to_folder, create_tag, get_tags_on_folder}; @@ -257,13 +265,21 @@ mod get_tag_on_folder_tests { con.close().unwrap(); assert_eq!( vec![ - Tag { + TaggedItem { id: 1, - title: "test".to_string() + tag_id: 1, + title: "test".to_string(), + folder_id: Some(1), + file_id: None, + implicit_from_id: None }, - Tag { + TaggedItem { id: 2, - title: "test2".to_string() + tag_id: 2, + title: "test2".to_string(), + folder_id: Some(1), + file_id: None, + implicit_from_id: None } ], res @@ -285,13 +301,13 @@ mod get_tag_on_folder_tests { .unwrap(); let res = get_tags_on_folder(1, &con).unwrap(); con.close().unwrap(); - assert_eq!(Vec::::new(), res); + assert_eq!(Vec::::new(), res); cleanup(); } } mod remove_tag_from_folder_tests { - use crate::model::repository::{Folder, Tag}; + use crate::model::repository::{Folder, TaggedItem}; use crate::repository::folder_repository::create_folder; use crate::repository::open_connection; use crate::tags::repository::{create_tag, get_tags_on_folder, remove_tag_from_folder}; @@ -314,7 +330,7 @@ mod remove_tag_from_folder_tests { remove_tag_from_folder(1, 1, &con).unwrap(); let tags = get_tags_on_folder(1, &con).unwrap(); con.close().unwrap(); - assert_eq!(Vec::::new(), tags); + assert_eq!(Vec::::new(), tags); cleanup(); } } @@ -322,8 +338,9 @@ mod remove_tag_from_folder_tests { mod get_tags_on_files_tests { use std::collections::HashMap; + use crate::model::repository::TaggedItem; use crate::tags::repository::get_tags_on_files; - use crate::{model::repository::Tag, repository::open_connection, test::*}; + use crate::{repository::open_connection, test::*}; #[test] fn returns_proper_mapping_for_file_tags() { @@ -339,8 +356,12 @@ mod get_tags_on_files_tests { con.close().unwrap(); #[rustfmt::skip] let expected = HashMap::from([ - (1, vec![Tag {id: 1, title: "tag1".to_string()}, Tag {id: 2, title: "tag2".to_string()}]), - (2, vec![Tag {id: 3, title: "tag3".to_string()}]) + (1, vec![ + TaggedItem {id: 1, tag_id: 1, file_id: Some(1), folder_id: None, title: "tag1".to_string(), implicit_from_id: None}, + TaggedItem {id: 2, tag_id: 2, file_id: Some(1), folder_id: None, title: "tag2".to_string(), implicit_from_id: None}, + ] + ), + (2, vec![TaggedItem {id: 3, tag_id: 3, file_id: Some(2), folder_id: None, title: "tag3".to_string(), implicit_from_id: None}]) ]); assert_eq!(res, expected); cleanup(); diff --git a/src/tags/tests/service.rs b/src/tags/tests/service.rs index c45a4d2..c5f6653 100644 --- a/src/tags/tests/service.rs +++ b/src/tags/tests/service.rs @@ -89,7 +89,7 @@ mod update_file_tag_test { use crate::model::error::tag_errors::TagRelationError; use crate::model::file_types::FileTypes; use crate::model::repository::FileRecord; - use crate::model::response::TagApi; + use crate::model::response::TaggedItemApi; use crate::tags::service::{create_tag, get_tags_on_file, update_file_tags}; use crate::test::{cleanup, init_db_folder, now}; @@ -110,25 +110,29 @@ mod update_file_tag_test { update_file_tags( 1, vec![ - TagApi { - id: Some(1), + TaggedItemApi { + tag_id: Some(1), title: "test".to_string(), + implicit_from: None, }, - TagApi { - id: None, + TaggedItemApi { + tag_id: None, title: "new tag".to_string(), + implicit_from: None, }, ], ) .unwrap(); let expected = vec![ - TagApi { - id: Some(1), + TaggedItemApi { + tag_id: Some(1), title: "test".to_string(), + implicit_from: None, }, - TagApi { - id: Some(2), + TaggedItemApi { + tag_id: Some(2), title: "new tag".to_string(), + implicit_from: None, }, ]; let actual = get_tags_on_file(1).unwrap(); @@ -150,9 +154,10 @@ mod update_file_tag_test { .save_to_db(); update_file_tags( 1, - vec![TagApi { - id: None, + vec![TaggedItemApi { + tag_id: None, title: "test".to_string(), + implicit_from: None, }], ) .unwrap(); @@ -187,13 +192,15 @@ mod update_file_tag_test { update_file_tags( 1, vec![ - TagApi { - id: Some(1), + TaggedItemApi { + tag_id: Some(1), title: "test".to_string(), + implicit_from: None, }, - TagApi { - id: Some(1), + TaggedItemApi { + tag_id: Some(1), title: "test".to_string(), + implicit_from: None, }, ], ) @@ -201,7 +208,7 @@ mod update_file_tag_test { let actual = get_tags_on_file(1).unwrap(); assert_eq!(actual.len(), 1); - assert_eq!(actual[0].id, Some(1)); + assert_eq!(actual[0].tag_id, Some(1)); assert_eq!(actual[0].title, "test"); cleanup(); } @@ -223,13 +230,15 @@ mod update_file_tag_test { update_file_tags( 1, vec![ - TagApi { - id: None, + TaggedItemApi { + tag_id: None, title: "test".to_string(), + implicit_from: None, }, - TagApi { - id: None, + TaggedItemApi { + tag_id: None, title: "test".to_string(), + implicit_from: None, }, ], ) @@ -237,7 +246,7 @@ mod update_file_tag_test { let actual = get_tags_on_file(1).unwrap(); assert_eq!(actual.len(), 1); - assert_eq!(actual[0].id, Some(1)); + assert_eq!(actual[0].tag_id, Some(1)); assert_eq!(actual[0].title, "test"); cleanup(); } @@ -258,9 +267,10 @@ mod update_file_tag_test { // Mix of new tag by name and existing tag by id (same tag) update_file_tags( 1, - vec![TagApi { - id: None, + vec![TaggedItemApi { + tag_id: None, title: "test".to_string(), + implicit_from: None, }], ) .unwrap(); @@ -269,13 +279,15 @@ mod update_file_tag_test { update_file_tags( 1, vec![ - TagApi { - id: Some(1), + TaggedItemApi { + tag_id: Some(1), title: "test".to_string(), + implicit_from: None, }, - TagApi { - id: None, + TaggedItemApi { + tag_id: None, title: "test".to_string(), + implicit_from: None, }, ], ) @@ -283,7 +295,7 @@ mod update_file_tag_test { let actual = get_tags_on_file(1).unwrap(); assert_eq!(actual.len(), 1); - assert_eq!(actual[0].id, Some(1)); + assert_eq!(actual[0].tag_id, Some(1)); assert_eq!(actual[0].title, "test"); cleanup(); } @@ -292,7 +304,7 @@ mod update_file_tag_test { mod update_folder_tag_test { use crate::model::error::tag_errors::TagRelationError; use crate::model::repository::Folder; - use crate::model::response::TagApi; + use crate::model::response::TaggedItemApi; use crate::repository::{folder_repository, open_connection}; use crate::tags::service::{create_tag, get_tags_on_folder, update_folder_tags}; use crate::test::{cleanup, init_db_folder}; @@ -315,25 +327,29 @@ mod update_folder_tag_test { update_folder_tags( 1, vec![ - TagApi { - id: Some(1), + TaggedItemApi { + tag_id: Some(1), title: "test".to_string(), + implicit_from: None, }, - TagApi { - id: None, + TaggedItemApi { + tag_id: None, title: "new tag".to_string(), + implicit_from: None, }, ], ) .unwrap(); let expected = vec![ - TagApi { - id: Some(1), + TaggedItemApi { + tag_id: Some(1), title: "test".to_string(), + implicit_from: None, }, - TagApi { - id: Some(2), + TaggedItemApi { + tag_id: Some(2), title: "new tag".to_string(), + implicit_from: None, }, ]; let actual = get_tags_on_folder(1).unwrap(); @@ -357,9 +373,10 @@ mod update_folder_tag_test { con.close().unwrap(); update_folder_tags( 1, - vec![TagApi { - id: None, + vec![TaggedItemApi { + tag_id: None, title: "test".to_string(), + implicit_from: None, }], ) .unwrap(); @@ -396,13 +413,15 @@ mod update_folder_tag_test { update_folder_tags( 1, vec![ - TagApi { - id: Some(1), + TaggedItemApi { + tag_id: Some(1), title: "test".to_string(), + implicit_from: None, }, - TagApi { - id: Some(1), + TaggedItemApi { + tag_id: Some(1), title: "test".to_string(), + implicit_from: None, }, ], ) @@ -410,7 +429,7 @@ mod update_folder_tag_test { let actual = get_tags_on_folder(1).unwrap(); assert_eq!(actual.len(), 1); - assert_eq!(actual[0].id, Some(1)); + assert_eq!(actual[0].tag_id, Some(1)); assert_eq!(actual[0].title, "test"); cleanup(); } @@ -434,13 +453,15 @@ mod update_folder_tag_test { update_folder_tags( 1, vec![ - TagApi { - id: None, + TaggedItemApi { + tag_id: None, title: "test".to_string(), + implicit_from: None, }, - TagApi { - id: None, + TaggedItemApi { + tag_id: None, title: "test".to_string(), + implicit_from: None, }, ], ) @@ -448,7 +469,7 @@ mod update_folder_tag_test { let actual = get_tags_on_folder(1).unwrap(); assert_eq!(actual.len(), 1); - assert_eq!(actual[0].id, Some(1)); + assert_eq!(actual[0].tag_id, Some(1)); assert_eq!(actual[0].title, "test"); cleanup(); } @@ -471,9 +492,10 @@ mod update_folder_tag_test { // Mix of new tag by name and existing tag by id (same tag) update_folder_tags( 1, - vec![TagApi { - id: None, + vec![TaggedItemApi { + tag_id: None, title: "test".to_string(), + implicit_from: None, }], ) .unwrap(); @@ -482,13 +504,15 @@ mod update_folder_tag_test { update_folder_tags( 1, vec![ - TagApi { - id: Some(1), + TaggedItemApi { + tag_id: Some(1), title: "test".to_string(), + implicit_from: None, }, - TagApi { - id: None, + TaggedItemApi { + tag_id: None, title: "test".to_string(), + implicit_from: None, }, ], ) @@ -496,7 +520,7 @@ mod update_folder_tag_test { let actual = get_tags_on_folder(1).unwrap(); assert_eq!(actual.len(), 1); - assert_eq!(actual[0].id, Some(1)); + assert_eq!(actual[0].tag_id, Some(1)); assert_eq!(actual[0].title, "test"); cleanup(); } diff --git a/src/test/mod.rs b/src/test/mod.rs index e157781..23f4e87 100644 --- a/src/test/mod.rs +++ b/src/test/mod.rs @@ -129,8 +129,17 @@ mod tests { connection.close().unwrap(); } - pub fn inherit_tag_folder(name: &str, folder_id: u32, inherited_from: u32) { + pub fn imply_tag_on_file(tag_id: u32, file_id: u32, implicit_from_id: u32) { let con = open_connection(); + let sql = format!( + "insert into TaggedItems(tagId, fileId, implicitFromId) values ({tag_id}, {file_id}, {implicit_from_id})" + ); + // scoped here so that the prepared statement gets dropped, which is needed to close the connection + let mut pst = con.prepare(&sql).unwrap(); + pst.raw_execute().unwrap(); + // this is needed so that con isn't being shared anymore in this function's scope + drop(pst); + con.close().unwrap(); } pub fn create_tag_folders(name: &str, folder_ids: Vec) { @@ -245,7 +254,7 @@ mod tests { for tag in &mut self.tags { let Tag { id, title: _ } = tag_repository::create_tag(&tag.title, &con).unwrap(); tag_repository::add_explicit_tag_to_file(file_id, id, &con).unwrap(); - tag.id = Some(id); + tag.tag_id = Some(id); } if let Some(folder_id) = self.folder_id { folder_repository::link_folder_to_file(file_id, folder_id, &con).unwrap(); From 17c12027cdde9b7e5d7b3506e2cb731e0c83dc18 Mon Sep 17 00:00:00 2001 From: ploiu Date: Wed, 19 Nov 2025 02:18:00 +0000 Subject: [PATCH 18/18] stub function --- src/tags/service.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/tags/service.rs b/src/tags/service.rs index eecce49..08d150d 100644 --- a/src/tags/service.rs +++ b/src/tags/service.rs @@ -199,8 +199,7 @@ pub fn update_file_tags(file_id: u32, tags: Vec) -> Result<(), Ta } let con = open_connection(); // instead of removing all the tags and then adding them back, we can use a HashSet or 2 to enforce a unique list in-memory without as much IO - let existing_tags: HashSet = - HashSet::from_iter(get_tags_on_file(file_id)?); + let existing_tags: HashSet = HashSet::from_iter(get_tags_on_file(file_id)?); let tags = HashSet::from_iter(tags); // we need to find 2 things: 1) tags to add 2) tags to remove let tags_to_remove = existing_tags.difference(&tags); @@ -395,3 +394,11 @@ pub fn get_tags_on_folder(folder_id: u32) -> Result, TagRelat con.close().unwrap(); Ok(db_tags.into_iter().map(TaggedItemApi::from).collect()) } + +/// gets all explicit tags on the folder with the passed id, and implies it on all descendant files and folders. +/// +/// In order for a tag to be implied, the target file/folder must not already have it (explicit or implicit). +/// +/// ## Parameters +/// - `folder_id`: the id of the folder to implicate children of +pub fn implicate_children(folder_id: u32) -> Result<(), TagRelationError> {}