Tag writes: bulk endpoint, and fix remove/re-add/duplicate bugs #201
No files matched your search
@@ -727,9 +727,11 @@ GET /video/original/stream?token=... (Range-capable original bytes)
|
||||
|
||||
// Tags
|
||||
GET /image/tags/all (tags + counts, camera_makes, and cameras: make+model bodies)
|
||||
POST /image/tags (add tag to file)
|
||||
DELETE /image/tags (remove tag from file)
|
||||
POST /image/tags/batch (bulk tag updates)
|
||||
POST /image/tags (add tag to file; by name, case-insensitive, created when missing; already tagged = no-op)
|
||||
DELETE /image/tags (remove tag from file: every path sharing its content, matching reads)
|
||||
POST /image/tags/batch (replace ONE file's tag set: { file_name, tag_ids })
|
||||
POST /image/tags/bulk (many files, one transaction: { file_names, add: [names], remove: [ids] }
|
||||
-> { added, removed, tags }; 400 if a tag is in both)
|
||||
|
||||
// Photo versions (#191): AI restorations / upscales / edits shown as ONE photo
|
||||
GET /image/versions?path=...&library=... (path of the original or any version ->
|
||||
|
||||
+668
-128
@@ -33,6 +33,7 @@ where
|
||||
.service(web::resource("image/tags/all").route(web::get().to(get_all_tags::<TagD>)))
|
||||
.service(web::resource("image/tags/batch").route(web::post().to(update_tags::<TagD>)))
|
||||
.service(web::resource("image/tags/lookup").route(web::post().to(lookup_tags_batch::<TagD>)))
|
||||
.service(web::resource("image/tags/bulk").route(web::post().to(bulk_tag::<TagD>)))
|
||||
.service(
|
||||
web::resource("image/tags/{id}")
|
||||
.route(web::put().to(update_tag::<TagD>))
|
||||
@@ -50,32 +51,18 @@ async fn add_tag<D: TagDao>(
|
||||
let tag_name = body.tag_name.clone();
|
||||
let normalized_path = normalize_path(&body.file_name);
|
||||
|
||||
let mut tag_dao = tag_dao.lock().expect("Unable to get TagDao");
|
||||
if tag_name.trim().is_empty() {
|
||||
return Ok(HttpResponse::BadRequest()
|
||||
.json(serde_json::json!({ "error": "Tag name must not be empty" })));
|
||||
}
|
||||
|
||||
let mut tag_dao = tag_dao.lock().expect("Unable to get TagDao");
|
||||
tag_dao
|
||||
.get_all_tags(None)
|
||||
.and_then(|tags| {
|
||||
// Case-insensitive match. With the unique-NOCASE index on
|
||||
// tags.name now in place, a case-sensitive find here would
|
||||
// miss a casing-only collision and let the subsequent
|
||||
// create_tag INSERT crash on the constraint.
|
||||
if let Some((_, tag)) = tags
|
||||
.iter()
|
||||
.find(|t| t.1.name.eq_ignore_ascii_case(&tag_name))
|
||||
{
|
||||
Ok(tag.clone())
|
||||
} else {
|
||||
info!(
|
||||
"Creating missing tag: '{:?}' for file: '{}'",
|
||||
tag_name, &normalized_path
|
||||
);
|
||||
tag_dao.create_tag(tag_name.trim())
|
||||
}
|
||||
})
|
||||
.find_or_create_tag(&tag_name)
|
||||
.and_then(|tag| tag_dao.tag_file(&normalized_path, tag.id))
|
||||
.map(|_| {
|
||||
span.set_status(Status::Ok);
|
||||
HttpResponse::Ok()
|
||||
HttpResponse::Ok().finish()
|
||||
})
|
||||
.into_http_internal_err()
|
||||
}
|
||||
@@ -268,50 +255,131 @@ async fn update_tags<D: TagDao>(
|
||||
let span = tracing::Span::current();
|
||||
let normalized_path = normalize_path(&request.file_name);
|
||||
|
||||
dao.get_tags_for_path(&normalized_path)
|
||||
.and_then(|existing_tags| dao.get_all_tags(None).map(|all| (existing_tags, all)))
|
||||
.map(|(existing_tags, all_tags)| {
|
||||
let tags_to_remove = existing_tags
|
||||
.iter()
|
||||
.filter(|&t| !request.tag_ids.contains(&t.id))
|
||||
.collect::<Vec<&Tag>>();
|
||||
|
||||
for tag in tags_to_remove {
|
||||
info!(
|
||||
"Removing tag {:?} from file: {:?}",
|
||||
tag.name, normalized_path
|
||||
);
|
||||
dao.remove_tag(&tag.name, &normalized_path)
|
||||
.unwrap_or_else(|err| panic!("{:?} Unable to remove tag {:?}", err, &tag.name));
|
||||
}
|
||||
|
||||
let new_tags = all_tags
|
||||
.iter()
|
||||
.filter(|(_, t)| !existing_tags.contains(t) && request.tag_ids.contains(&t.id))
|
||||
.collect::<Vec<&(i64, Tag)>>();
|
||||
|
||||
for (_, new_tag) in new_tags {
|
||||
info!(
|
||||
"Adding tag {:?} to file: {:?}",
|
||||
new_tag.name, normalized_path
|
||||
);
|
||||
|
||||
dao.tag_file(&normalized_path, new_tag.id)
|
||||
.with_context(|| {
|
||||
format!(
|
||||
"Unable to tag file {:?} with tag: {:?}",
|
||||
normalized_path, new_tag.name
|
||||
)
|
||||
})
|
||||
.unwrap();
|
||||
}
|
||||
// Errors propagate instead of panicking: a panic here would poison the
|
||||
// TagDao mutex and fail every later tag request until a restart.
|
||||
let result = (|| -> anyhow::Result<()> {
|
||||
let existing_tags = dao.get_tags_for_path(&normalized_path)?;
|
||||
for tag in existing_tags
|
||||
.iter()
|
||||
.filter(|t| !request.tag_ids.contains(&t.id))
|
||||
{
|
||||
info!(
|
||||
"Removing tag {:?} from file: {:?}",
|
||||
tag.name, normalized_path
|
||||
);
|
||||
dao.remove_tag(&tag.name, &normalized_path)?;
|
||||
}
|
||||
for tag_id in request
|
||||
.tag_ids
|
||||
.iter()
|
||||
.filter(|id| !existing_tags.iter().any(|t| t.id == **id))
|
||||
{
|
||||
info!("Adding tag {} to file: {:?}", tag_id, normalized_path);
|
||||
dao.tag_file(&normalized_path, *tag_id)?;
|
||||
}
|
||||
Ok(())
|
||||
})();
|
||||
|
||||
result
|
||||
.map(|_| {
|
||||
span.set_status(Status::Ok);
|
||||
HttpResponse::Ok()
|
||||
HttpResponse::Ok().finish()
|
||||
})
|
||||
.into_http_internal_err()
|
||||
}
|
||||
|
||||
/// Most photos one `POST /image/tags/bulk` may name. A select-all over a
|
||||
/// large folder stays well under it; the transaction holds SQLite's write
|
||||
/// lock for the whole batch.
|
||||
const BULK_TAG_MAX_FILES: usize = 5000;
|
||||
|
||||
#[derive(Deserialize, Debug)]
|
||||
pub struct BulkTagRequest {
|
||||
pub file_names: Vec<String>,
|
||||
/// Tag names to add, created when missing (case-insensitive match).
|
||||
#[serde(default)]
|
||||
pub add: Vec<String>,
|
||||
/// Tag ids to remove.
|
||||
#[serde(default)]
|
||||
pub remove: Vec<i32>,
|
||||
}
|
||||
|
||||
#[derive(Serialize, Debug)]
|
||||
pub struct BulkTagResponse {
|
||||
/// `tagged_photo` rows written (photos that already had a tag don't count).
|
||||
pub added: usize,
|
||||
/// `tagged_photo` rows deleted, across every copy of each photo.
|
||||
pub removed: usize,
|
||||
/// The resolved `add` tags, so a client learns the ids of new ones.
|
||||
pub tags: Vec<Tag>,
|
||||
}
|
||||
|
||||
/// Add and remove tags on many photos in one transaction: all of it applies
|
||||
/// or none does. Tags are library-agnostic (they follow the bytes), so this
|
||||
/// takes no `library`. Removal follows the photo's content like reads do.
|
||||
#[tracing::instrument(name = "bulk_tag", skip_all)]
|
||||
async fn bulk_tag<D: TagDao>(
|
||||
_: Claims,
|
||||
body: web::Json<BulkTagRequest>,
|
||||
tag_dao: web::Data<Mutex<D>>,
|
||||
) -> impl Responder {
|
||||
let span = tracing::Span::current();
|
||||
let bad = |msg: &str| HttpResponse::BadRequest().json(serde_json::json!({ "error": msg }));
|
||||
|
||||
let mut paths: Vec<String> = Vec::with_capacity(body.file_names.len());
|
||||
for p in body.file_names.iter().map(|p| normalize_path(p)) {
|
||||
if !paths.contains(&p) {
|
||||
paths.push(p);
|
||||
}
|
||||
}
|
||||
if paths.is_empty() {
|
||||
return bad("file_names must not be empty");
|
||||
}
|
||||
if paths.len() > BULK_TAG_MAX_FILES {
|
||||
return bad(&format!("At most {} files per request", BULK_TAG_MAX_FILES));
|
||||
}
|
||||
|
||||
let mut add: Vec<String> = Vec::with_capacity(body.add.len());
|
||||
for name in body.add.iter().map(|n| n.trim()) {
|
||||
if name.is_empty() {
|
||||
return bad("Tag name must not be empty");
|
||||
}
|
||||
if !add.iter().any(|a| a.eq_ignore_ascii_case(name)) {
|
||||
add.push(name.to_string());
|
||||
}
|
||||
}
|
||||
let mut remove = body.remove.clone();
|
||||
remove.sort_unstable();
|
||||
remove.dedup();
|
||||
if add.is_empty() && remove.is_empty() {
|
||||
return bad("Nothing to add or remove");
|
||||
}
|
||||
|
||||
span.set_attribute("path_count", paths.len() as i64);
|
||||
let mut dao = tag_dao.lock().expect("Unable to get TagDao");
|
||||
match dao.bulk_tag(&paths, &add, &remove) {
|
||||
Ok(BulkTagOutcome::Applied {
|
||||
added,
|
||||
removed,
|
||||
tags,
|
||||
}) => {
|
||||
span.set_status(Status::Ok);
|
||||
HttpResponse::Ok().json(BulkTagResponse {
|
||||
added,
|
||||
removed,
|
||||
tags,
|
||||
})
|
||||
}
|
||||
Ok(BulkTagOutcome::Conflict { tag }) => HttpResponse::BadRequest()
|
||||
.json(serde_json::json!({ "error": "A tag is both added and removed", "tag": tag })),
|
||||
Err(e) => {
|
||||
log::error!("bulk_tag failed: {:?}", e);
|
||||
HttpResponse::InternalServerError()
|
||||
.json(serde_json::json!({ "error": "Bulk tag failed" }))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Deserialize, Debug)]
|
||||
pub struct LookupTagsBatchRequest {
|
||||
pub paths: Vec<String>,
|
||||
@@ -552,6 +620,20 @@ pub enum UpdateTagOutcome {
|
||||
Conflict { existing: Tag },
|
||||
}
|
||||
|
||||
/// Result of [`TagDao::bulk_tag`].
|
||||
#[derive(Debug, PartialEq)]
|
||||
pub enum BulkTagOutcome {
|
||||
/// `tags` are the resolved `add` tags (existing or just created);
|
||||
/// `added` / `removed` count `tagged_photo` rows written / deleted.
|
||||
Applied {
|
||||
added: usize,
|
||||
removed: usize,
|
||||
tags: Vec<Tag>,
|
||||
},
|
||||
/// A tag named in `add` is also in `remove`. Nothing was written.
|
||||
Conflict { tag: Tag },
|
||||
}
|
||||
|
||||
pub trait TagDao: Send + Sync {
|
||||
fn get_all_tags(&mut self, path: Option<String>) -> anyhow::Result<Vec<(i64, Tag)>>;
|
||||
fn get_tags_for_path(&mut self, path: &str) -> anyhow::Result<Vec<Tag>>;
|
||||
@@ -567,8 +649,27 @@ pub trait TagDao: Send + Sync {
|
||||
paths: &[String],
|
||||
) -> anyhow::Result<std::collections::HashMap<String, Vec<Tag>>>;
|
||||
fn create_tag(&mut self, name: &str) -> anyhow::Result<Tag>;
|
||||
/// The tag named `name` (case-insensitive, like `idx_tags_name_nocase`),
|
||||
/// created when there is none. Unlike `get_all_tags`, this sees tags
|
||||
/// that no photo carries any more.
|
||||
fn find_or_create_tag(&mut self, name: &str) -> anyhow::Result<Tag>;
|
||||
/// Remove the tag named `tag_name` (case-insensitive) from the photo at
|
||||
/// `path`: from every path sharing its content and, for a photo version,
|
||||
/// from its original, matching what reads show. `None` when no tag has
|
||||
/// that name.
|
||||
fn remove_tag(&mut self, tag_name: &str, path: &str) -> anyhow::Result<Option<()>>;
|
||||
/// Tag the photo at `path` (a version's tag goes on its original).
|
||||
/// Tagging a photo that already carries the tag is a no-op.
|
||||
fn tag_file(&mut self, path: &str, tag_id: i32) -> anyhow::Result<TaggedPhoto>;
|
||||
/// Add the tags named `add` to every photo in `paths` and remove the tags
|
||||
/// with ids `remove` from them, in one transaction. Names follow
|
||||
/// [`TagDao::find_or_create_tag`], removal follows [`TagDao::remove_tag`].
|
||||
fn bulk_tag(
|
||||
&mut self,
|
||||
paths: &[String],
|
||||
add: &[String],
|
||||
remove: &[i32],
|
||||
) -> anyhow::Result<BulkTagOutcome>;
|
||||
fn get_files_with_all_tag_ids(
|
||||
&mut self,
|
||||
tag_ids: Vec<i32>,
|
||||
@@ -614,6 +715,102 @@ fn canonical_path(conn: &mut SqliteConnection, path: &str) -> String {
|
||||
.unwrap_or_else(|| path.to_string())
|
||||
}
|
||||
|
||||
/// The tag named `name`, matched case-insensitively like the
|
||||
/// `idx_tags_name_nocase` index (SQLite's NOCASE folds ASCII only, as does
|
||||
/// `eq_ignore_ascii_case`). The tags table is small; this mirrors the
|
||||
/// conflict check in `update_tag_name`.
|
||||
fn find_tag_by_name(conn: &mut SqliteConnection, name: &str) -> anyhow::Result<Option<Tag>> {
|
||||
Ok(tags::table
|
||||
.select((tags::id, tags::name, tags::created_time))
|
||||
.get_results::<Tag>(conn)
|
||||
.with_context(|| "Unable to list tags")?
|
||||
.into_iter()
|
||||
.find(|t| t.name.eq_ignore_ascii_case(name)))
|
||||
}
|
||||
|
||||
fn find_or_create_tag_in(conn: &mut SqliteConnection, name: &str) -> anyhow::Result<Tag> {
|
||||
let name = name.trim();
|
||||
if let Some(tag) = find_tag_by_name(conn, name)? {
|
||||
return Ok(tag);
|
||||
}
|
||||
diesel::insert_into(tags::table)
|
||||
.values(InsertTag {
|
||||
name: name.to_string(),
|
||||
created_time: Utc::now().timestamp(),
|
||||
})
|
||||
.execute(conn)
|
||||
.with_context(|| format!("Unable to insert tag {:?}", name))?;
|
||||
info!("Created tag: {:?}", name);
|
||||
find_tag_by_name(conn, name)?.with_context(|| format!("Tag {:?} vanished after insert", name))
|
||||
}
|
||||
|
||||
/// Insert the `(path, tag_id)` row unless it exists (`UNIQUE(rel_path,
|
||||
/// tag_id)`), on the canonical path. Returns that path and the number of rows
|
||||
/// inserted (0 or 1).
|
||||
fn insert_tag_row(
|
||||
conn: &mut SqliteConnection,
|
||||
path: &str,
|
||||
tag_id: i32,
|
||||
) -> anyhow::Result<(String, usize)> {
|
||||
// Tagging a photo version tags its original (photo versions, #191):
|
||||
// the tag binds to the original's bytes and path.
|
||||
let path = canonical_path(conn, path);
|
||||
|
||||
// Eagerly populate content_hash so this tag follows the bytes, not the
|
||||
// path (see CLAUDE.md "Multi-library data model"). None is fine: the
|
||||
// reconciliation pass backfills once image_exif has a hash for this
|
||||
// file. Library-agnostic by design: any matching image_exif row's hash
|
||||
// is acceptable.
|
||||
let content_hash: Option<String> = {
|
||||
use crate::database::schema::image_exif as ie;
|
||||
ie::table
|
||||
.filter(ie::rel_path.eq(&path))
|
||||
.filter(ie::content_hash.is_not_null())
|
||||
.select(ie::content_hash)
|
||||
.first::<Option<String>>(conn)
|
||||
.ok()
|
||||
.flatten()
|
||||
};
|
||||
|
||||
let inserted = diesel::insert_or_ignore_into(tagged_photo::table)
|
||||
.values(InsertTaggedPhoto {
|
||||
tag_id,
|
||||
photo_name: path.clone(),
|
||||
created_time: Utc::now().timestamp(),
|
||||
content_hash,
|
||||
})
|
||||
.execute(conn)
|
||||
.with_context(|| format!("Unable to tag file {:?}", path))?;
|
||||
if inserted > 0 {
|
||||
info!("Inserted tagged photo: {:#} -> {:?}", tag_id, path);
|
||||
}
|
||||
Ok((path, inserted))
|
||||
}
|
||||
|
||||
/// Delete tag `tag_id` from the photo at `path` the way reads see it: the
|
||||
/// canonical path, every `tagged_photo` row on a path holding the same
|
||||
/// content (`image_exif.content_hash`), and every row already keyed on that
|
||||
/// hash. Library-agnostic, like the tag reads in `get_tags` and
|
||||
/// `lookup_tags_batch`. Returns the number of rows deleted.
|
||||
fn remove_tag_rows(conn: &mut SqliteConnection, path: &str, tag_id: i32) -> anyhow::Result<usize> {
|
||||
let path = canonical_path(conn, path);
|
||||
diesel::sql_query(
|
||||
"DELETE FROM tagged_photo WHERE tag_id = ?1 AND (
|
||||
rel_path = ?2
|
||||
OR content_hash IN (
|
||||
SELECT e.content_hash FROM image_exif e
|
||||
WHERE e.rel_path = ?2 AND e.content_hash IS NOT NULL)
|
||||
OR rel_path IN (
|
||||
SELECT s.rel_path FROM image_exif s WHERE s.content_hash IN (
|
||||
SELECT e.content_hash FROM image_exif e
|
||||
WHERE e.rel_path = ?2 AND e.content_hash IS NOT NULL)))",
|
||||
)
|
||||
.bind::<Integer, _>(tag_id)
|
||||
.bind::<Text, _>(&path)
|
||||
.execute(conn)
|
||||
.with_context(|| format!("Unable to remove tag {} from {:?}", tag_id, path))
|
||||
}
|
||||
|
||||
pub struct SqliteTagDao {
|
||||
connection: Arc<Mutex<SqliteConnection>>,
|
||||
}
|
||||
@@ -864,6 +1061,17 @@ impl TagDao for SqliteTagDao {
|
||||
})
|
||||
}
|
||||
|
||||
fn find_or_create_tag(&mut self, name: &str) -> anyhow::Result<Tag> {
|
||||
let mut conn = self
|
||||
.connection
|
||||
.lock()
|
||||
.expect("Unable to lock SqliteTagDao connection");
|
||||
trace_db_call("insert", "find_or_create_tag", |span| {
|
||||
span.set_attribute("name", name.to_string());
|
||||
find_or_create_tag_in(conn.deref_mut(), name)
|
||||
})
|
||||
}
|
||||
|
||||
fn remove_tag(&mut self, tag_name: &str, path: &str) -> anyhow::Result<Option<()>> {
|
||||
let mut conn = self
|
||||
.connection
|
||||
@@ -873,27 +1081,16 @@ impl TagDao for SqliteTagDao {
|
||||
span.set_attribute("tag_name", tag_name.to_string());
|
||||
span.set_attribute("path", path.to_string());
|
||||
|
||||
let path = &canonical_path(conn.deref_mut(), path);
|
||||
tags::table
|
||||
.filter(tags::name.eq(tag_name))
|
||||
.get_result::<Tag>(conn.deref_mut())
|
||||
.optional()
|
||||
.with_context(|| format!("Unable to get tag '{}'", tag_name))
|
||||
.and_then(|tag| {
|
||||
if let Some(tag) = tag {
|
||||
diesel::delete(
|
||||
tagged_photo::table
|
||||
.filter(tagged_photo::tag_id.eq(tag.id))
|
||||
.filter(tagged_photo::rel_path.eq(path)),
|
||||
)
|
||||
.execute(conn.deref_mut())
|
||||
.with_context(|| format!("Unable to delete tag: '{}'", &tag.name))
|
||||
.map(|_| Some(()))
|
||||
} else {
|
||||
info!("No tag found with name '{}'", tag_name);
|
||||
Ok(None)
|
||||
}
|
||||
})
|
||||
let Some(tag) = find_tag_by_name(conn.deref_mut(), tag_name.trim())? else {
|
||||
info!("No tag found with name '{}'", tag_name);
|
||||
return Ok(None);
|
||||
};
|
||||
let removed = remove_tag_rows(conn.deref_mut(), path, tag.id)?;
|
||||
debug!(
|
||||
"Removed {} rows of tag {:?} for {:?}",
|
||||
removed, tag.name, path
|
||||
);
|
||||
Ok(Some(()))
|
||||
})
|
||||
}
|
||||
|
||||
@@ -906,58 +1103,64 @@ impl TagDao for SqliteTagDao {
|
||||
span.set_attribute("path", path.to_string());
|
||||
span.set_attribute("tag_id", tag_id.to_string());
|
||||
|
||||
// Tagging a photo version tags its original (photo versions,
|
||||
// #191): the tag binds to the original's bytes and path.
|
||||
let path = &canonical_path(conn.deref_mut(), path);
|
||||
let (path, _) = insert_tag_row(conn.deref_mut(), path, tag_id)?;
|
||||
tagged_photo::table
|
||||
.filter(tagged_photo::rel_path.eq(&path))
|
||||
.filter(tagged_photo::tag_id.eq(tag_id))
|
||||
.first(conn.deref_mut())
|
||||
.with_context(|| format!("Error reading tag {} on {:?}", tag_id, path))
|
||||
})
|
||||
}
|
||||
|
||||
// Eagerly populate content_hash so this tag follows the bytes,
|
||||
// not the path (see CLAUDE.md "Multi-library data model").
|
||||
// None is fine — the reconciliation pass will backfill once
|
||||
// image_exif has a hash for this file. We deliberately don't
|
||||
// require library_id here: the tag handler is library-
|
||||
// agnostic by design, and any matching image_exif row's hash
|
||||
// is acceptable. If the path resolves to different bytes in
|
||||
// different libraries, reconciliation per-library refines.
|
||||
let content_hash: Option<String> = {
|
||||
use crate::database::schema::image_exif as ie;
|
||||
ie::table
|
||||
.filter(ie::rel_path.eq(path))
|
||||
.filter(ie::content_hash.is_not_null())
|
||||
.select(ie::content_hash)
|
||||
.first::<Option<String>>(conn.deref_mut())
|
||||
.ok()
|
||||
.flatten()
|
||||
};
|
||||
fn bulk_tag(
|
||||
&mut self,
|
||||
paths: &[String],
|
||||
add: &[String],
|
||||
remove: &[i32],
|
||||
) -> anyhow::Result<BulkTagOutcome> {
|
||||
let mut conn = self
|
||||
.connection
|
||||
.lock()
|
||||
.expect("Unable to lock SqliteTagDao connection");
|
||||
trace_db_call("update", "bulk_tag", |span| {
|
||||
span.set_attribute("path_count", paths.len() as i64);
|
||||
span.set_attribute("add_count", add.len() as i64);
|
||||
span.set_attribute("remove_count", remove.len() as i64);
|
||||
|
||||
diesel::insert_into(tagged_photo::table)
|
||||
.values(InsertTaggedPhoto {
|
||||
tag_id,
|
||||
photo_name: path.to_string(),
|
||||
created_time: Utc::now().timestamp(),
|
||||
content_hash,
|
||||
})
|
||||
.execute(conn.deref_mut())
|
||||
.with_context(|| format!("Unable to tag file {:?} in sqlite", path))
|
||||
.and_then(|_| {
|
||||
info!("Inserted tagged photo: {:#} -> {:?}", tag_id, path);
|
||||
define_sql_function! {
|
||||
fn last_insert_rowid() -> diesel::sql_types::Integer;
|
||||
conn.immediate_transaction::<BulkTagOutcome, anyhow::Error, _>(|conn| {
|
||||
let mut tags = Vec::with_capacity(add.len());
|
||||
for name in add {
|
||||
// A name already in `remove` must not be created first.
|
||||
if let Some(tag) = find_tag_by_name(conn, name.trim())?
|
||||
&& remove.contains(&tag.id)
|
||||
{
|
||||
return Ok(BulkTagOutcome::Conflict { tag });
|
||||
}
|
||||
diesel::select(last_insert_rowid())
|
||||
.get_result::<i32>(conn.deref_mut())
|
||||
.with_context(|| "Unable to get last inserted tag from Sqlite")
|
||||
})
|
||||
.and_then(|tagged_id| {
|
||||
tagged_photo::table
|
||||
.find(tagged_id)
|
||||
.first(conn.deref_mut())
|
||||
.with_context(|| {
|
||||
format!(
|
||||
"Error getting inserted tagged photo with id: {:?}",
|
||||
tagged_id
|
||||
)
|
||||
})
|
||||
}
|
||||
for name in add {
|
||||
tags.push(find_or_create_tag_in(conn, name)?);
|
||||
}
|
||||
let (mut added, mut removed) = (0, 0);
|
||||
for path in paths {
|
||||
for tag_id in remove {
|
||||
removed += remove_tag_rows(conn, path, *tag_id)?;
|
||||
}
|
||||
for tag in &tags {
|
||||
added += insert_tag_row(conn, path, tag.id)?.1;
|
||||
}
|
||||
}
|
||||
info!(
|
||||
"Bulk tag: {} photos, +{} rows, -{} rows",
|
||||
paths.len(),
|
||||
added,
|
||||
removed
|
||||
);
|
||||
Ok(BulkTagOutcome::Applied {
|
||||
added,
|
||||
removed,
|
||||
tags,
|
||||
})
|
||||
})
|
||||
})
|
||||
}
|
||||
|
||||
@@ -1270,6 +1473,28 @@ mod tests {
|
||||
Ok(tag)
|
||||
}
|
||||
|
||||
fn find_or_create_tag(&mut self, name: &str) -> anyhow::Result<Tag> {
|
||||
let existing = self
|
||||
.tags
|
||||
.borrow()
|
||||
.iter()
|
||||
.find(|t| t.name.eq_ignore_ascii_case(name.trim()))
|
||||
.cloned();
|
||||
match existing {
|
||||
Some(tag) => Ok(tag),
|
||||
None => self.create_tag(name.trim()),
|
||||
}
|
||||
}
|
||||
|
||||
fn bulk_tag(
|
||||
&mut self,
|
||||
_paths: &[String],
|
||||
_add: &[String],
|
||||
_remove: &[i32],
|
||||
) -> anyhow::Result<BulkTagOutcome> {
|
||||
unimplemented!("bulk_tag is tested against SQLite")
|
||||
}
|
||||
|
||||
fn remove_tag(&mut self, tag_name: &str, path: &str) -> anyhow::Result<Option<()>> {
|
||||
let mut clone = {
|
||||
let photo_tags = &self.tagged_photos.borrow()[path];
|
||||
@@ -1666,3 +1891,318 @@ pub struct FileWithTagCount {
|
||||
#[diesel(sql_type = BigInt)]
|
||||
pub(crate) tag_count: i64,
|
||||
}
|
||||
|
||||
/// Tag writes against a real SQLite schema (`UNIQUE(rel_path, tag_id)`,
|
||||
/// `idx_tags_name_nocase`, content-hash siblings), which the `TestTagDao`
|
||||
/// mock can't model.
|
||||
#[cfg(test)]
|
||||
mod sqlite_tests {
|
||||
use super::*;
|
||||
use crate::database::test::in_memory_db_connection;
|
||||
use actix_web::body::MessageBody;
|
||||
use actix_web::http::StatusCode;
|
||||
|
||||
struct Fx {
|
||||
conn: Arc<Mutex<SqliteConnection>>,
|
||||
data: web::Data<Mutex<SqliteTagDao>>,
|
||||
}
|
||||
|
||||
impl Fx {
|
||||
fn new() -> Self {
|
||||
let conn = Arc::new(Mutex::new(in_memory_db_connection()));
|
||||
let data = web::Data::new(Mutex::new(SqliteTagDao::new(conn.clone())));
|
||||
Fx { conn, data }
|
||||
}
|
||||
fn exec(&self, sql: &str) {
|
||||
diesel::sql_query(sql)
|
||||
.execute(self.conn.lock().unwrap().deref_mut())
|
||||
.unwrap();
|
||||
}
|
||||
fn count(&self, sql: &str) -> i64 {
|
||||
#[derive(QueryableByName)]
|
||||
struct N {
|
||||
#[diesel(sql_type = BigInt)]
|
||||
n: i64,
|
||||
}
|
||||
diesel::sql_query(sql)
|
||||
.get_result::<N>(self.conn.lock().unwrap().deref_mut())
|
||||
.unwrap()
|
||||
.n
|
||||
}
|
||||
/// An indexed file in `library` holding bytes `hash`.
|
||||
fn photo(&self, library: i32, path: &str, hash: &str) {
|
||||
self.exec(&format!(
|
||||
"INSERT INTO image_exif (library_id, rel_path, created_time, last_modified, content_hash) \
|
||||
VALUES ({library}, '{path}', 0, 0, '{hash}')"
|
||||
));
|
||||
}
|
||||
fn tag_names(&self, path: &str) -> Vec<String> {
|
||||
let mut names: Vec<String> = self
|
||||
.data
|
||||
.lock()
|
||||
.unwrap()
|
||||
.get_tags_for_path(path)
|
||||
.unwrap()
|
||||
.into_iter()
|
||||
.map(|t| t.name)
|
||||
.collect();
|
||||
names.sort();
|
||||
names
|
||||
}
|
||||
async fn add(&self, file: &str, tag: &str) -> StatusCode {
|
||||
let body = AddTagRequest {
|
||||
file_name: file.into(),
|
||||
tag_name: tag.into(),
|
||||
};
|
||||
status(add_tag(claims(), web::Json(body), self.data.clone()).await)
|
||||
}
|
||||
async fn remove(&self, file: &str, tag: &str) -> StatusCode {
|
||||
let body = AddTagRequest {
|
||||
file_name: file.into(),
|
||||
tag_name: tag.into(),
|
||||
};
|
||||
status(remove_tagged_photo(claims(), web::Json(body), self.data.clone()).await)
|
||||
}
|
||||
async fn bulk(
|
||||
&self,
|
||||
files: &[&str],
|
||||
add: &[&str],
|
||||
remove: &[i32],
|
||||
) -> (StatusCode, serde_json::Value) {
|
||||
let body = BulkTagRequest {
|
||||
file_names: files.iter().map(|f| f.to_string()).collect(),
|
||||
add: add.iter().map(|a| a.to_string()).collect(),
|
||||
remove: remove.to_vec(),
|
||||
};
|
||||
let resp = bulk_tag(claims(), web::Json(body), self.data.clone())
|
||||
.await
|
||||
.respond_to(&actix_web::test::TestRequest::default().to_http_request());
|
||||
let status = resp.status();
|
||||
let bytes = resp.into_body().try_into_bytes().unwrap_or_default();
|
||||
(status, serde_json::from_slice(&bytes).unwrap_or_default())
|
||||
}
|
||||
fn tag_id(&self, name: &str) -> i32 {
|
||||
self.data
|
||||
.lock()
|
||||
.unwrap()
|
||||
.find_or_create_tag(name)
|
||||
.unwrap()
|
||||
.id
|
||||
}
|
||||
}
|
||||
|
||||
fn claims() -> Claims {
|
||||
Claims::valid_user("1".into())
|
||||
}
|
||||
|
||||
fn status(r: impl Responder) -> StatusCode {
|
||||
r.respond_to(&actix_web::test::TestRequest::default().to_http_request())
|
||||
.status()
|
||||
}
|
||||
|
||||
#[actix_rt::test]
|
||||
async fn adding_a_tag_the_photo_already_has_is_a_no_op() {
|
||||
let fx = Fx::new();
|
||||
assert_eq!(fx.add("a.jpg", "beach").await, StatusCode::OK);
|
||||
assert_eq!(fx.add("a.jpg", "beach").await, StatusCode::OK);
|
||||
assert_eq!(fx.add("a.jpg", "Beach").await, StatusCode::OK);
|
||||
assert_eq!(fx.count("SELECT COUNT(*) AS n FROM tagged_photo"), 1);
|
||||
assert_eq!(fx.count("SELECT COUNT(*) AS n FROM tags"), 1);
|
||||
}
|
||||
|
||||
#[actix_rt::test]
|
||||
async fn a_tag_no_photo_carries_can_be_added_again() {
|
||||
let fx = Fx::new();
|
||||
fx.add("a.jpg", "beach").await;
|
||||
assert_eq!(fx.remove("a.jpg", "beach").await, StatusCode::OK);
|
||||
assert_eq!(fx.add("b.jpg", "BEACH").await, StatusCode::OK);
|
||||
assert_eq!(
|
||||
fx.count("SELECT COUNT(*) AS n FROM tags"),
|
||||
1,
|
||||
"reused, not duplicated"
|
||||
);
|
||||
assert_eq!(fx.tag_names("b.jpg"), vec!["beach"]);
|
||||
}
|
||||
|
||||
#[actix_rt::test]
|
||||
async fn an_empty_tag_name_is_rejected() {
|
||||
let fx = Fx::new();
|
||||
assert_eq!(fx.add("a.jpg", " ").await, StatusCode::BAD_REQUEST);
|
||||
assert_eq!(fx.count("SELECT COUNT(*) AS n FROM tags"), 0);
|
||||
}
|
||||
|
||||
#[actix_rt::test]
|
||||
async fn removal_ignores_case_and_reaches_every_copy_of_the_photo() {
|
||||
let fx = Fx::new();
|
||||
fx.photo(1, "2020/a.jpg", "h-a");
|
||||
fx.photo(2, "archive/2020/a.jpg", "h-a");
|
||||
fx.photo(1, "2020/b.jpg", "h-b");
|
||||
// The tag was written while viewing the archive copy, and once more on
|
||||
// a path that no longer exists but kept the hash.
|
||||
fx.add("archive/2020/a.jpg", "beach").await;
|
||||
let id = fx.tag_id("beach");
|
||||
fx.exec(&format!(
|
||||
"INSERT INTO tagged_photo (rel_path, tag_id, created_time, content_hash) \
|
||||
VALUES ('old/a.jpg', {id}, 0, 'h-a')"
|
||||
));
|
||||
fx.add("2020/b.jpg", "beach").await;
|
||||
|
||||
assert_eq!(fx.remove("2020/a.jpg", "BEACH").await, StatusCode::OK);
|
||||
|
||||
assert_eq!(
|
||||
fx.count(&format!(
|
||||
"SELECT COUNT(*) AS n FROM tagged_photo WHERE tag_id = {id}"
|
||||
)),
|
||||
1,
|
||||
"only the other photo keeps it"
|
||||
);
|
||||
assert_eq!(fx.tag_names("2020/b.jpg"), vec!["beach"]);
|
||||
}
|
||||
|
||||
#[actix_rt::test]
|
||||
async fn removing_an_unknown_tag_is_404() {
|
||||
let fx = Fx::new();
|
||||
assert_eq!(fx.remove("a.jpg", "nope").await, StatusCode::NOT_FOUND);
|
||||
}
|
||||
|
||||
#[actix_rt::test]
|
||||
async fn replacing_a_tag_set_can_add_a_tag_no_photo_carries() {
|
||||
let fx = Fx::new();
|
||||
let unused = fx.tag_id("unused");
|
||||
fx.add("a.jpg", "beach").await;
|
||||
let body = AddTagsRequest {
|
||||
file_name: "a.jpg".into(),
|
||||
tag_ids: vec![unused],
|
||||
};
|
||||
assert_eq!(
|
||||
status(update_tags(claims(), fx.data.clone(), web::Json(body)).await),
|
||||
StatusCode::OK
|
||||
);
|
||||
assert_eq!(fx.tag_names("a.jpg"), vec!["unused"]);
|
||||
}
|
||||
|
||||
#[actix_rt::test]
|
||||
async fn a_failing_tag_set_replace_answers_500_and_tagging_keeps_working() {
|
||||
let fx = Fx::new();
|
||||
fx.exec(
|
||||
"CREATE TRIGGER boom BEFORE INSERT ON tagged_photo WHEN NEW.rel_path = 'boom.jpg' \
|
||||
BEGIN SELECT RAISE(ABORT, 'boom'); END",
|
||||
);
|
||||
let id = fx.tag_id("beach");
|
||||
let body = AddTagsRequest {
|
||||
file_name: "boom.jpg".into(),
|
||||
tag_ids: vec![id],
|
||||
};
|
||||
assert_eq!(
|
||||
status(update_tags(claims(), fx.data.clone(), web::Json(body)).await),
|
||||
StatusCode::INTERNAL_SERVER_ERROR
|
||||
);
|
||||
assert!(!fx.data.is_poisoned(), "the TagDao lock must survive");
|
||||
assert_eq!(fx.add("a.jpg", "beach").await, StatusCode::OK);
|
||||
}
|
||||
|
||||
#[actix_rt::test]
|
||||
async fn bulk_adds_existing_and_new_tags_to_every_photo() {
|
||||
let fx = Fx::new();
|
||||
fx.add("a.jpg", "beach").await;
|
||||
let (st, body) = fx
|
||||
.bulk(
|
||||
&["a.jpg", "x/b.jpg", "x\\b.jpg"],
|
||||
&["Beach", "Road trip", "road TRIP"],
|
||||
&[],
|
||||
)
|
||||
.await;
|
||||
assert_eq!(st, StatusCode::OK);
|
||||
// a.jpg already had beach: 3 new rows, not 4.
|
||||
assert_eq!(body["added"], 3);
|
||||
assert_eq!(body["removed"], 0);
|
||||
let names: Vec<&str> = body["tags"]
|
||||
.as_array()
|
||||
.unwrap()
|
||||
.iter()
|
||||
.map(|t| t["name"].as_str().unwrap())
|
||||
.collect();
|
||||
assert_eq!(names, vec!["beach", "Road trip"]);
|
||||
assert_eq!(fx.tag_names("a.jpg"), vec!["Road trip", "beach"]);
|
||||
assert_eq!(fx.tag_names("x/b.jpg"), vec!["Road trip", "beach"]);
|
||||
}
|
||||
|
||||
#[actix_rt::test]
|
||||
async fn bulk_removes_from_every_copy() {
|
||||
let fx = Fx::new();
|
||||
fx.photo(1, "a.jpg", "h-a");
|
||||
fx.photo(2, "archive/a.jpg", "h-a");
|
||||
fx.add("archive/a.jpg", "beach").await;
|
||||
fx.add("b.jpg", "beach").await;
|
||||
fx.add("c.jpg", "beach").await;
|
||||
let id = fx.tag_id("beach");
|
||||
|
||||
let (st, body) = fx.bulk(&["a.jpg", "b.jpg"], &[], &[id]).await;
|
||||
assert_eq!(st, StatusCode::OK);
|
||||
assert_eq!(body["removed"], 2);
|
||||
assert!(fx.tag_names("archive/a.jpg").is_empty());
|
||||
assert!(fx.tag_names("b.jpg").is_empty());
|
||||
assert_eq!(
|
||||
fx.tag_names("c.jpg"),
|
||||
vec!["beach"],
|
||||
"unselected photos keep it"
|
||||
);
|
||||
}
|
||||
|
||||
#[actix_rt::test]
|
||||
async fn bulk_writes_nothing_when_any_row_fails() {
|
||||
let fx = Fx::new();
|
||||
fx.exec(
|
||||
"CREATE TRIGGER boom BEFORE INSERT ON tagged_photo WHEN NEW.rel_path = 'boom.jpg' \
|
||||
BEGIN SELECT RAISE(ABORT, 'boom'); END",
|
||||
);
|
||||
let (st, _) = fx.bulk(&["a.jpg", "boom.jpg"], &["brand new"], &[]).await;
|
||||
assert_eq!(st, StatusCode::INTERNAL_SERVER_ERROR);
|
||||
assert_eq!(fx.count("SELECT COUNT(*) AS n FROM tagged_photo"), 0);
|
||||
assert_eq!(
|
||||
fx.count("SELECT COUNT(*) AS n FROM tags"),
|
||||
0,
|
||||
"the new tag rolled back too"
|
||||
);
|
||||
assert!(!fx.data.is_poisoned());
|
||||
}
|
||||
|
||||
#[actix_rt::test]
|
||||
async fn bulk_rejects_a_tag_both_added_and_removed() {
|
||||
let fx = Fx::new();
|
||||
let id = fx.tag_id("beach");
|
||||
let (st, body) = fx.bulk(&["a.jpg"], &["BEACH", "new"], &[id]).await;
|
||||
assert_eq!(st, StatusCode::BAD_REQUEST);
|
||||
assert_eq!(body["tag"]["id"], id);
|
||||
assert_eq!(
|
||||
fx.count("SELECT COUNT(*) AS n FROM tags"),
|
||||
1,
|
||||
"'new' not created"
|
||||
);
|
||||
}
|
||||
|
||||
#[actix_rt::test]
|
||||
async fn bulk_rejects_empty_requests() {
|
||||
let fx = Fx::new();
|
||||
assert_eq!(
|
||||
fx.bulk(&[], &["beach"], &[]).await.0,
|
||||
StatusCode::BAD_REQUEST
|
||||
);
|
||||
assert_eq!(
|
||||
fx.bulk(&["a.jpg"], &[], &[]).await.0,
|
||||
StatusCode::BAD_REQUEST
|
||||
);
|
||||
assert_eq!(
|
||||
fx.bulk(&["a.jpg"], &[" "], &[]).await.0,
|
||||
StatusCode::BAD_REQUEST
|
||||
);
|
||||
let many: Vec<String> = (0..=BULK_TAG_MAX_FILES)
|
||||
.map(|i| format!("{i}.jpg"))
|
||||
.collect();
|
||||
let many: Vec<&str> = many.iter().map(String::as_str).collect();
|
||||
assert_eq!(
|
||||
fx.bulk(&many, &["beach"], &[]).await.0,
|
||||
StatusCode::BAD_REQUEST
|
||||
);
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user