Skip to main content

max / audiofiles

Match UNIQUE violations by result code, not error text Replace contains("UNIQUE") at the two sites that turn a uniqueness collision into a domain outcome (VFS name conflict, skipped duplicate import) with a typed predicate on the extended SQLite result codes SQLITE_CONSTRAINT_UNIQUE / SQLITE_CONSTRAINT_PRIMARYKEY. is_unique_violation matches the extended code rather than the ConstraintViolation primary code that resolve.rs uses: FK, NOT NULL and CHECK failures mean the write was malformed, not that the row already exists, and reporting those as a duplicate would swallow real bugs. A test pins that discrimination. The migration crash-recovery paths in db.rs keep their string matching. "duplicate column" and "already exists" raise generic SQLITE_ERROR with no distinguishing extended code, so there is nothing typed to match on. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-07-20 18:25 UTC
Commit: 242a6ec82a4808b799b68244dd0a3d01534e380c
Parent: 6c0d94e
3 files changed, +59 insertions, -3 deletions
@@ -279,7 +279,7 @@ fn import_single_file(
279 279 Err(CoreError::NameConflict(_)) => Ok(ImportFileResult::Duplicate),
280 280 Err(e) => {
281 281 if let CoreError::Db(ref sqlite_err) = e
282 - && sqlite_err.to_string().contains("UNIQUE") {
282 + && audiofiles_core::error::is_unique_violation(sqlite_err) {
283 283 return Ok(ImportFileResult::Duplicate);
284 284 }
285 285 Err(e)
@@ -148,6 +148,24 @@ impl From<crate::db::DbError> for CoreError {
148 148 /// Convenience alias for `std::result::Result<T, CoreError>`.
149 149 pub type Result<T> = std::result::Result<T, CoreError>;
150 150
151 + /// True when `e` is a uniqueness violation — a UNIQUE index or a PRIMARY KEY
152 + /// collision, and nothing else.
153 + ///
154 + /// Callers use this to turn "that name/hash is already taken" into a domain
155 + /// outcome (a name conflict, a skipped duplicate import) instead of an error.
156 + /// It deliberately matches on the *extended* result code rather than the
157 + /// `ConstraintViolation` primary code, because the other constraint kinds — FK,
158 + /// NOT NULL, CHECK — mean the write was malformed, not that the row already
159 + /// exists, and silently reporting those as "duplicate" would swallow real bugs.
160 + pub fn is_unique_violation(e: &rusqlite::Error) -> bool {
161 + matches!(
162 + e,
163 + rusqlite::Error::SqliteFailure(err, _)
164 + if err.extended_code == rusqlite::ffi::SQLITE_CONSTRAINT_UNIQUE
165 + || err.extended_code == rusqlite::ffi::SQLITE_CONSTRAINT_PRIMARYKEY
166 + )
167 + }
168 +
151 169 /// Construct a `CoreError::Io` with path context.
152 170 pub(crate) fn io_err(path: impl Into<PathBuf>, source: std::io::Error) -> CoreError {
153 171 CoreError::Io {
@@ -233,6 +251,44 @@ mod tests {
233 251 }
234 252
235 253 #[test]
254 + fn unique_violation_distinguishes_constraint_kinds() {
255 + let conn = rusqlite::Connection::open_in_memory().unwrap();
256 + conn.execute_batch(
257 + "PRAGMA foreign_keys = ON;
258 + CREATE TABLE parent (id INTEGER PRIMARY KEY);
259 + CREATE TABLE t (
260 + id INTEGER PRIMARY KEY,
261 + uniq TEXT NOT NULL UNIQUE,
262 + needed TEXT NOT NULL,
263 + parent INTEGER REFERENCES parent(id)
264 + );
265 + INSERT INTO t (uniq, needed) VALUES ('taken', 'x');",
266 + )
267 + .unwrap();
268 +
269 + let dup = conn
270 + .execute("INSERT INTO t (uniq, needed) VALUES ('taken', 'y')", [])
271 + .unwrap_err();
272 + assert!(is_unique_violation(&dup), "UNIQUE collision must match");
273 +
274 + // The other constraint kinds share the ConstraintViolation primary code
275 + // but mean the write was malformed — reporting them as "already exists"
276 + // is exactly what this helper exists to prevent.
277 + let not_null = conn
278 + .execute("INSERT INTO t (uniq) VALUES ('fresh')", [])
279 + .unwrap_err();
280 + assert!(!is_unique_violation(&not_null), "NOT NULL must not match");
281 +
282 + let fk = conn
283 + .execute(
284 + "INSERT INTO t (uniq, needed, parent) VALUES ('other', 'z', 999)",
285 + [],
286 + )
287 + .unwrap_err();
288 + assert!(!is_unique_violation(&fk), "FK violation must not match");
289 + }
290 +
291 + #[test]
236 292 fn unix_now_returns_reasonable_timestamp() {
237 293 let ts = unix_now();
238 294 // Should be after 2024-01-01 (1704067200) and before 2100-01-01 (4102444800)
@@ -273,7 +273,7 @@ fn insert_node_or_conflict(
273 273 ) -> Result<NodeId> {
274 274 match db.conn().execute(sql, params) {
275 275 Ok(_) => Ok(NodeId::from(db.conn().last_insert_rowid())),
276 - Err(e) if e.to_string().contains("UNIQUE") => {
276 + Err(e) if crate::error::is_unique_violation(&e) => {
277 277 Err(CoreError::NameConflict(name.to_string()))
278 278 }
279 279 Err(e) => Err(e.into()),
@@ -843,7 +843,7 @@ mod tests {
843 843 dup.is_err(),
844 844 "partial unique index must reject a duplicate root name"
845 845 );
846 - assert!(dup.unwrap_err().to_string().contains("UNIQUE"));
846 + assert!(crate::error::is_unique_violation(&dup.unwrap_err()));
847 847 }
848 848
849 849 #[test]