Skip to content

Commit 31c339e

Browse files
russellbrussellb
committed
fix(persistence): return typed Conflict for absent-row conditional writes
A version-matched put_if that matched no row reported an absent row as a stringy Database error while a version mismatch was a typed Conflict. Callers that need to distinguish 'condition not met' from a real error had to match backend-dependent error text. Report both cases as Conflict: an absent row carries current_resource_version: None, a version mismatch carries the current version. Applied to both the SQLite and Postgres backends. Signed-off-by: Russell Bryant <[email protected]>
1 parent bf68e0b commit 31c339e

2 files changed

Lines changed: 18 additions & 19 deletions

File tree

‎crates/openshell-server/src/persistence/postgres.rs‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -160,17 +160,16 @@ RETURNING resource_version, created_at_ms, updated_at_ms
160160
updated_at_ms: row.get("updated_at_ms"),
161161
})
162162
} else {
163-
// Check if object exists to distinguish NotFound from Conflict
163+
// The version-matched UPDATE matched no row. Distinguish a
164+
// version mismatch (row present, different version) from an
165+
// absent row (deleted / never existed). Both are CAS
166+
// precondition failures, so report them as typed `Conflict`
167+
// rather than a backend-dependent error string: absent rows
168+
// carry `current_resource_version: None`.
164169
let existing = self.get(object_type, id).await?;
165-
if let Some(record) = existing {
166-
Err(PersistenceError::Conflict {
167-
current_resource_version: Some(record.resource_version),
168-
})
169-
} else {
170-
Err(PersistenceError::Database(format!(
171-
"object not found: {object_type}/{id}"
172-
)))
173-
}
170+
Err(PersistenceError::Conflict {
171+
current_resource_version: existing.map(|record| record.resource_version),
172+
})
174173
}
175174
}
176175
WriteCondition::Unconditional => {

‎crates/openshell-server/src/persistence/sqlite.rs‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -168,16 +168,16 @@ WHERE "object_type" = ?1 AND "id" = ?2 AND "resource_version" = ?3
168168
.map_err(|e| map_db_error(&e))?;
169169

170170
if result.rows_affected() == 0 {
171-
// Check if object exists to distinguish NotFound from Conflict
171+
// The version-matched UPDATE matched no row. Distinguish a
172+
// version mismatch (row present, different version) from an
173+
// absent row (deleted / never existed). Both are CAS
174+
// precondition failures, so report them as typed `Conflict`
175+
// rather than a backend-dependent error string: absent rows
176+
// carry `current_resource_version: None`.
172177
let existing = self.get(object_type, id).await?;
173-
if let Some(record) = existing {
174-
return Err(PersistenceError::Conflict {
175-
current_resource_version: Some(record.resource_version),
176-
});
177-
}
178-
return Err(PersistenceError::Database(format!(
179-
"object not found: {object_type}/{id}"
180-
)));
178+
return Err(PersistenceError::Conflict {
179+
current_resource_version: existing.map(|record| record.resource_version),
180+
});
181181
}
182182

183183
// Fetch the updated record to get the new resource_version

0 commit comments

Comments
 (0)