docs: add Plan 5 (Object flexible-field values) implementation plan
object.fields jsonb + set_object_fields with registry validation (type + term/authority resolution) + audited per-field diffs; typed FieldError. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,584 @@
|
|||||||
|
# Object Flexible-Field Values Implementation Plan
|
||||||
|
|
||||||
|
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||||
|
|
||||||
|
**Goal:** Where the registry gets used — a `fields jsonb` column on `object`, with `set_object_fields` that validates each value against the field-definition registry (Plan 4) + resolves term/authority references (Plan 2) + audits the per-field changes (Plan 1/3), all in the write transaction.
|
||||||
|
|
||||||
|
**Architecture:** `CatalogueObject` gains a `fields: serde_json::Value` (the raw JSONB map). `db::catalog::set_object_fields` takes the complete desired field map, validates every value (unknown key → error; type mismatch → error; term/authority must resolve), **replaces** the JSONB, and audits the diff against the old map. A typed `FieldError` (using the `db` crate's so-far-unused `thiserror` dep) surfaces validation failures. Required-field *completeness* is NOT enforced here (deferred to the publish gate, Plan 7).
|
||||||
|
|
||||||
|
**Tech Stack:** Rust 2024, sqlx 0.8 (`json`), `serde_json` (values + validation), `thiserror` (FieldError). Tests use `#[sqlx::test]`.
|
||||||
|
|
||||||
|
## Design decisions (approved)
|
||||||
|
- Storage: `object.fields jsonb NOT NULL DEFAULT '{}'`, `field_key → value`.
|
||||||
|
- Value shapes: text→string, localized_text→`{lang: string}`, integer→JSON integer, date→string, boolean→bool, term→term-UUID string (resolves in bound vocab), authority→authority-UUID string (resolves; kind matches if constrained).
|
||||||
|
- `set_object_fields` = **replace** the whole map, **separate** from `update_object`; audits the diff; no-op skips write+audit (consistent with `update_object`).
|
||||||
|
- Required-field enforcement deferred to publish (Plan 7).
|
||||||
|
- Strict per-field rules (date format, min/max, regex) deferred to #11; here `date` validates as a string only.
|
||||||
|
- Spectrum field-set seeding is a separate follow-on (not this plan).
|
||||||
|
|
||||||
|
## Prerequisites
|
||||||
|
- Postgres for tests with CREATE DATABASE rights; pass `DATABASE_URL` inline. Shell env does not persist between commands. Pass transaction connections as `&mut tx` (NOT `&mut *tx`) to avoid clippy `explicit_auto_deref`.
|
||||||
|
|
||||||
|
## File Structure
|
||||||
|
```
|
||||||
|
crates/domain/src/object.rs CatalogueObject gains `fields: serde_json::Value`
|
||||||
|
crates/db/
|
||||||
|
Cargo.toml serde_json also in [dev-dependencies]
|
||||||
|
migrations/0005_object_fields.sql
|
||||||
|
src/catalog.rs + fields in SELECT/map; FieldError; set_object_fields + validation helpers
|
||||||
|
tests/object_fields.rs
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 1: `fields` column + read it back
|
||||||
|
|
||||||
|
**Files:** create `crates/db/migrations/0005_object_fields.sql`; modify `crates/domain/src/object.rs`, `crates/db/src/catalog.rs`, `crates/db/tests/migrate.rs`, `crates/db/Cargo.toml`.
|
||||||
|
|
||||||
|
- [ ] **Step 1: Migration.** Create `crates/db/migrations/0005_object_fields.sql`:
|
||||||
|
```sql
|
||||||
|
-- Flexible field values for a catalogue object, keyed by field-definition key.
|
||||||
|
ALTER TABLE object ADD COLUMN fields JSONB NOT NULL DEFAULT '{}'::jsonb;
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 2: Domain.** In `crates/domain/src/object.rs`, add a `fields` field to `CatalogueObject` (after `visibility`, before the timestamps):
|
||||||
|
```rust
|
||||||
|
pub visibility: Visibility,
|
||||||
|
/// Flexible field values (field key -> value), validated against the registry.
|
||||||
|
pub fields: serde_json::Value,
|
||||||
|
pub created_at: OffsetDateTime,
|
||||||
|
pub updated_at: OffsetDateTime,
|
||||||
|
```
|
||||||
|
Add `use serde_json` if needed (serde_json is already a domain dependency). `CatalogueObject` derives `Debug, Clone, PartialEq` (NOT `Eq`) — `serde_json::Value` is `PartialEq` but not `Eq`, so do not add `Eq`. `to_input()` is unchanged (it maps only the 9 core mutable fields; `fields` is not part of `ObjectInput`).
|
||||||
|
|
||||||
|
- [ ] **Step 3: db reads the column.** In `crates/db/src/catalog.rs`:
|
||||||
|
- Add `fields` to the `OBJECT_COLUMNS` const (append `, fields`).
|
||||||
|
- In `map_object`, add: `fields: row.try_get("fields")?,` (between `visibility` and `created_at`). `sqlx` decodes a `jsonb` column directly into `serde_json::Value`.
|
||||||
|
(`create_object`'s INSERT does NOT list `fields`, so it uses the `'{}'` default — leave the INSERT unchanged.)
|
||||||
|
|
||||||
|
- [ ] **Step 4: dev-dep.** In `crates/db/Cargo.toml`, add `serde_json` to `[dev-dependencies]` (it is a normal dep, but integration tests need it in scope):
|
||||||
|
```toml
|
||||||
|
[dev-dependencies]
|
||||||
|
tokio.workspace = true
|
||||||
|
time.workspace = true
|
||||||
|
serde_json.workspace = true
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 5: Tests.** Append to `crates/db/tests/migrate.rs`:
|
||||||
|
```rust
|
||||||
|
#[sqlx::test]
|
||||||
|
async fn migrate_adds_object_fields_column(pool: PgPool) {
|
||||||
|
let db = Db::from_pool(pool);
|
||||||
|
let exists: Option<bool> = sqlx::query_scalar(
|
||||||
|
"SELECT true FROM information_schema.columns \
|
||||||
|
WHERE table_name = 'object' AND column_name = 'fields'",
|
||||||
|
)
|
||||||
|
.fetch_optional(db.pool())
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
assert_eq!(exists, Some(true));
|
||||||
|
}
|
||||||
|
```
|
||||||
|
And append to `crates/db/tests/catalog.rs`:
|
||||||
|
```rust
|
||||||
|
#[sqlx::test]
|
||||||
|
async fn new_object_has_empty_fields(pool: PgPool) {
|
||||||
|
let db = Db::from_pool(pool);
|
||||||
|
let mut tx = db.pool().begin().await.unwrap();
|
||||||
|
let id = catalog::create_object(&mut tx, AuditActor::System, &sample_input("LM-9")).await.unwrap();
|
||||||
|
tx.commit().await.unwrap();
|
||||||
|
|
||||||
|
let obj = catalog::object_by_id(db.pool(), id).await.unwrap().unwrap();
|
||||||
|
assert_eq!(obj.fields, serde_json::json!({}));
|
||||||
|
}
|
||||||
|
```
|
||||||
|
(`crates/db/tests/catalog.rs` already imports what it needs except possibly `serde_json` — add `use serde_json` if the test uses the `json!` macro and it is not already imported; `serde_json::json!` works fully-qualified as written.)
|
||||||
|
|
||||||
|
- [ ] **Step 6: Run + lint.** `DATABASE_URL=<url> cargo test -p db --test migrate --test catalog` and `cargo test -p domain` → all pass. `cargo +nightly fmt`; `DATABASE_URL=<url> cargo clippy -p db -p domain --all-targets -- -D warnings` → clean.
|
||||||
|
|
||||||
|
- [ ] **Step 7: Commit.**
|
||||||
|
```bash
|
||||||
|
git add crates/domain crates/db
|
||||||
|
git commit -m "feat(db): add object.fields jsonb column, read it into CatalogueObject"
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 2: `set_object_fields` — validate, write, audit
|
||||||
|
|
||||||
|
**Files:** modify `crates/db/src/catalog.rs`; create `crates/db/tests/object_fields.rs`.
|
||||||
|
|
||||||
|
- [ ] **Step 1: Write the failing test** `crates/db/tests/object_fields.rs`:
|
||||||
|
```rust
|
||||||
|
use db::catalog::FieldError;
|
||||||
|
use db::{Db, audit, catalog, fields, vocab};
|
||||||
|
use domain::{
|
||||||
|
AuditAction, AuditActor, FieldType, LocalizedLabel, NewFieldDefinition, ObjectInput, Visibility,
|
||||||
|
};
|
||||||
|
use sqlx::PgPool;
|
||||||
|
|
||||||
|
fn obj_input() -> ObjectInput {
|
||||||
|
ObjectInput {
|
||||||
|
object_number: "LM-1".into(),
|
||||||
|
object_name: "vase".into(),
|
||||||
|
number_of_objects: 1,
|
||||||
|
brief_description: None,
|
||||||
|
current_location: None,
|
||||||
|
current_owner: None,
|
||||||
|
recorder: None,
|
||||||
|
recording_date: None,
|
||||||
|
visibility: Visibility::Draft,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
fn label(text: &str) -> Vec<LocalizedLabel> {
|
||||||
|
vec![LocalizedLabel { lang: "en".into(), label: text.into() }]
|
||||||
|
}
|
||||||
|
|
||||||
|
async fn setup_object(db: &Db) -> domain::ObjectId {
|
||||||
|
let mut tx = db.pool().begin().await.unwrap();
|
||||||
|
let id = catalog::create_object(&mut tx, AuditActor::System, &obj_input()).await.unwrap();
|
||||||
|
tx.commit().await.unwrap();
|
||||||
|
id
|
||||||
|
}
|
||||||
|
|
||||||
|
async fn define(db: &Db, key: &str, field_type: FieldType) {
|
||||||
|
let mut tx = db.pool().begin().await.unwrap();
|
||||||
|
fields::create_field_definition(
|
||||||
|
&mut tx,
|
||||||
|
&NewFieldDefinition { key: key.into(), field_type, required: false, group_key: None, labels: label(key) },
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
tx.commit().await.unwrap();
|
||||||
|
}
|
||||||
|
|
||||||
|
#[sqlx::test]
|
||||||
|
async fn sets_scalar_fields_and_audits(pool: PgPool) {
|
||||||
|
let db = Db::from_pool(pool);
|
||||||
|
let id = setup_object(&db).await;
|
||||||
|
define(&db, "comments", FieldType::Text).await;
|
||||||
|
define(&db, "year", FieldType::Integer).await;
|
||||||
|
define(&db, "on_display", FieldType::Boolean).await;
|
||||||
|
|
||||||
|
let values = serde_json::json!({ "comments": "nice", "year": 1850, "on_display": true });
|
||||||
|
|
||||||
|
let mut tx = db.pool().begin().await.unwrap();
|
||||||
|
catalog::set_object_fields(&mut tx, AuditActor::System, id, values.as_object().unwrap())
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
tx.commit().await.unwrap();
|
||||||
|
|
||||||
|
let obj = catalog::object_by_id(db.pool(), id).await.unwrap().unwrap();
|
||||||
|
assert_eq!(obj.fields["comments"], "nice");
|
||||||
|
assert_eq!(obj.fields["year"], 1850);
|
||||||
|
assert_eq!(obj.fields["on_display"], true);
|
||||||
|
|
||||||
|
let history = audit::history_for(db.pool(), "object", id.to_uuid()).await.unwrap();
|
||||||
|
// created + the field set
|
||||||
|
assert_eq!(history.last().unwrap().action, AuditAction::Updated);
|
||||||
|
let changed: Vec<&str> = history.last().unwrap().changes.iter().map(|c| c.field.as_str()).collect();
|
||||||
|
assert!(changed.contains(&"comments") && changed.contains(&"year") && changed.contains(&"on_display"));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[sqlx::test]
|
||||||
|
async fn term_field_must_resolve_in_its_vocabulary(pool: PgPool) {
|
||||||
|
let db = Db::from_pool(pool);
|
||||||
|
let id = setup_object(&db).await;
|
||||||
|
let material = vocab::create_vocabulary(db.pool(), "material").await.unwrap();
|
||||||
|
define(&db, "material", FieldType::Term { vocabulary_id: material.id }).await;
|
||||||
|
|
||||||
|
// add a real term
|
||||||
|
let mut tx = db.pool().begin().await.unwrap();
|
||||||
|
let wood = vocab::add_term(
|
||||||
|
&mut tx,
|
||||||
|
&domain::NewTerm { vocabulary_id: material.id, external_uri: None, labels: label("wood") },
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
tx.commit().await.unwrap();
|
||||||
|
|
||||||
|
// valid term id resolves
|
||||||
|
let ok = serde_json::json!({ "material": wood.to_string() });
|
||||||
|
let mut tx = db.pool().begin().await.unwrap();
|
||||||
|
catalog::set_object_fields(&mut tx, AuditActor::System, id, ok.as_object().unwrap()).await.unwrap();
|
||||||
|
tx.commit().await.unwrap();
|
||||||
|
|
||||||
|
// random uuid does not resolve
|
||||||
|
let bad = serde_json::json!({ "material": domain::TermId::new().to_string() });
|
||||||
|
let mut tx = db.pool().begin().await.unwrap();
|
||||||
|
let err = catalog::set_object_fields(&mut tx, AuditActor::System, id, bad.as_object().unwrap()).await;
|
||||||
|
assert!(matches!(err, Err(FieldError::Unresolved { .. })));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[sqlx::test]
|
||||||
|
async fn unknown_field_and_type_mismatch_are_rejected(pool: PgPool) {
|
||||||
|
let db = Db::from_pool(pool);
|
||||||
|
let id = setup_object(&db).await;
|
||||||
|
define(&db, "year", FieldType::Integer).await;
|
||||||
|
|
||||||
|
let unknown = serde_json::json!({ "nope": "x" });
|
||||||
|
let mut tx = db.pool().begin().await.unwrap();
|
||||||
|
assert!(matches!(
|
||||||
|
catalog::set_object_fields(&mut tx, AuditActor::System, id, unknown.as_object().unwrap()).await,
|
||||||
|
Err(FieldError::UnknownField(_))
|
||||||
|
));
|
||||||
|
drop(tx);
|
||||||
|
|
||||||
|
let wrong = serde_json::json!({ "year": "not a number" });
|
||||||
|
let mut tx = db.pool().begin().await.unwrap();
|
||||||
|
assert!(matches!(
|
||||||
|
catalog::set_object_fields(&mut tx, AuditActor::System, id, wrong.as_object().unwrap()).await,
|
||||||
|
Err(FieldError::TypeMismatch { .. })
|
||||||
|
));
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 2: Run to verify it fails.** `DATABASE_URL=<url> cargo test -p db --test object_fields` → FAIL.
|
||||||
|
|
||||||
|
- [ ] **Step 3: Implement** — add to the top of `crates/db/src/catalog.rs` the `FieldError` type, and append the function + helpers. Add imports as needed (`use crate::{audit, authority, fields, vocab};` — `audit` is already imported; add `authority, fields, vocab`). Also ensure `use domain::{... TermId, AuthorityId ...}` are available (add to the existing domain import).
|
||||||
|
|
||||||
|
`FieldError`:
|
||||||
|
```rust
|
||||||
|
/// Why setting flexible field values failed.
|
||||||
|
#[derive(Debug, thiserror::Error)]
|
||||||
|
pub enum FieldError {
|
||||||
|
#[error("object not found")]
|
||||||
|
ObjectNotFound,
|
||||||
|
#[error("unknown field: {0}")]
|
||||||
|
UnknownField(String),
|
||||||
|
#[error("field `{field}` expects a {expected} value")]
|
||||||
|
TypeMismatch { field: String, expected: &'static str },
|
||||||
|
#[error("field `{field}`: value does not resolve to an existing {kind}")]
|
||||||
|
Unresolved { field: String, kind: &'static str },
|
||||||
|
#[error(transparent)]
|
||||||
|
Db(#[from] sqlx::Error),
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
`set_object_fields` + helpers:
|
||||||
|
```rust
|
||||||
|
/// Replace an object's flexible field values, validating each against the registry
|
||||||
|
/// (type + term/authority resolution), and audit the per-field diff — all on `conn`.
|
||||||
|
/// A no-op (identical to the current values) writes nothing and records no audit.
|
||||||
|
pub async fn set_object_fields(
|
||||||
|
conn: &mut sqlx::PgConnection,
|
||||||
|
actor: AuditActor,
|
||||||
|
object_id: ObjectId,
|
||||||
|
values: &serde_json::Map<String, Value>,
|
||||||
|
) -> Result<(), FieldError> {
|
||||||
|
let Some(old) = object_by_id(&mut *conn, object_id).await? else {
|
||||||
|
return Err(FieldError::ObjectNotFound);
|
||||||
|
};
|
||||||
|
|
||||||
|
for (key, value) in values {
|
||||||
|
validate_field(&mut *conn, key, value).await?;
|
||||||
|
}
|
||||||
|
|
||||||
|
let new_fields = Value::Object(values.clone());
|
||||||
|
let changes = field_map_changes(&old.fields, &new_fields);
|
||||||
|
if changes.is_empty() {
|
||||||
|
return Ok(());
|
||||||
|
}
|
||||||
|
|
||||||
|
sqlx::query("UPDATE object SET fields = $2, updated_at = now() WHERE id = $1")
|
||||||
|
.bind(object_id.to_uuid())
|
||||||
|
.bind(&new_fields)
|
||||||
|
.execute(&mut *conn)
|
||||||
|
.await?;
|
||||||
|
|
||||||
|
audit::record(
|
||||||
|
&mut *conn,
|
||||||
|
&NewAuditEvent {
|
||||||
|
actor,
|
||||||
|
action: AuditAction::Updated,
|
||||||
|
entity_type: ENTITY_TYPE.to_owned(),
|
||||||
|
entity_id: object_id.to_uuid(),
|
||||||
|
changes,
|
||||||
|
},
|
||||||
|
)
|
||||||
|
.await?;
|
||||||
|
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
|
async fn validate_field(
|
||||||
|
conn: &mut sqlx::PgConnection,
|
||||||
|
key: &str,
|
||||||
|
value: &Value,
|
||||||
|
) -> Result<(), FieldError> {
|
||||||
|
let def = fields::field_definition_by_key(&mut *conn, key)
|
||||||
|
.await?
|
||||||
|
.ok_or_else(|| FieldError::UnknownField(key.to_owned()))?;
|
||||||
|
|
||||||
|
match def.field_type {
|
||||||
|
FieldType::Text => require(value.is_string(), key, "text")?,
|
||||||
|
FieldType::LocalizedText => require(
|
||||||
|
value.as_object().is_some_and(|o| o.values().all(Value::is_string)),
|
||||||
|
key,
|
||||||
|
"localized-text object {lang: string}",
|
||||||
|
)?,
|
||||||
|
FieldType::Integer => require(value.is_i64(), key, "integer")?,
|
||||||
|
FieldType::Date => require(value.is_string(), key, "date string")?,
|
||||||
|
FieldType::Boolean => require(value.is_boolean(), key, "boolean")?,
|
||||||
|
FieldType::Term { vocabulary_id } => {
|
||||||
|
let term_id = parse_uuid(value, key, "term id (uuid string)")?;
|
||||||
|
if vocab::resolve_term(&mut *conn, vocabulary_id, domain::TermId::from_uuid(term_id))
|
||||||
|
.await?
|
||||||
|
.is_none()
|
||||||
|
{
|
||||||
|
return Err(FieldError::Unresolved { field: key.to_owned(), kind: "term" });
|
||||||
|
}
|
||||||
|
}
|
||||||
|
FieldType::Authority { kind } => {
|
||||||
|
let authority_id = parse_uuid(value, key, "authority id (uuid string)")?;
|
||||||
|
match authority::resolve_authority(&mut *conn, domain::AuthorityId::from_uuid(authority_id)).await? {
|
||||||
|
Some(r) if kind.is_none_or(|k| r.kind() == k) => {}
|
||||||
|
_ => return Err(FieldError::Unresolved { field: key.to_owned(), kind: "authority" }),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
|
fn require(ok: bool, field: &str, expected: &'static str) -> Result<(), FieldError> {
|
||||||
|
if ok {
|
||||||
|
Ok(())
|
||||||
|
} else {
|
||||||
|
Err(FieldError::TypeMismatch { field: field.to_owned(), expected })
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
fn parse_uuid(value: &Value, field: &str, expected: &'static str) -> Result<uuid::Uuid, FieldError> {
|
||||||
|
value
|
||||||
|
.as_str()
|
||||||
|
.and_then(|s| s.parse::<uuid::Uuid>().ok())
|
||||||
|
.ok_or_else(|| FieldError::TypeMismatch { field: field.to_owned(), expected })
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Per-key diff between two flexible-field maps. `before`/`after` are `None` when
|
||||||
|
/// the key is absent on that side (so adds and removes are captured).
|
||||||
|
fn field_map_changes(old: &Value, new: &Value) -> Vec<FieldChange> {
|
||||||
|
let empty = serde_json::Map::new();
|
||||||
|
let old_map = old.as_object().unwrap_or(&empty);
|
||||||
|
let new_map = new.as_object().unwrap_or(&empty);
|
||||||
|
|
||||||
|
let keys: std::collections::BTreeSet<&String> = old_map.keys().chain(new_map.keys()).collect();
|
||||||
|
keys.into_iter()
|
||||||
|
.filter_map(|key| {
|
||||||
|
let before = old_map.get(key).cloned();
|
||||||
|
let after = new_map.get(key).cloned();
|
||||||
|
if before != after {
|
||||||
|
Some(FieldChange { field: key.clone(), before, after })
|
||||||
|
} else {
|
||||||
|
None
|
||||||
|
}
|
||||||
|
})
|
||||||
|
.collect()
|
||||||
|
}
|
||||||
|
```
|
||||||
|
Notes:
|
||||||
|
- `db` already depends on `thiserror`, `uuid`, `serde_json`, `domain`, and has the `vocab`/`authority`/`fields`/`audit` sibling modules — no Cargo changes beyond Task 1's dev-dep.
|
||||||
|
- `Value` and `FieldChange` are already imported at the top of `catalog.rs` (`use serde_json::{Value, json};` and `use domain::{..., FieldChange, ...}`). If `FieldChange` is not in the existing domain import, add it.
|
||||||
|
|
||||||
|
- [ ] **Step 4: Run to verify it passes.** `DATABASE_URL=<url> cargo test -p db --test object_fields` → PASS (3 tests).
|
||||||
|
|
||||||
|
- [ ] **Step 5: Lint.** `cargo +nightly fmt`; `DATABASE_URL=<url> cargo clippy -p db --all-targets -- -D warnings` → clean.
|
||||||
|
|
||||||
|
- [ ] **Step 6: Commit.**
|
||||||
|
```bash
|
||||||
|
git add crates/db
|
||||||
|
git commit -m "feat(db): set_object_fields with registry validation and audited diffs"
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 3: validation coverage + replace/no-op semantics
|
||||||
|
|
||||||
|
**Files:** modify `crates/db/tests/object_fields.rs`.
|
||||||
|
|
||||||
|
- [ ] **Step 1: Add tests** to `crates/db/tests/object_fields.rs`:
|
||||||
|
```rust
|
||||||
|
#[sqlx::test]
|
||||||
|
async fn authority_field_enforces_kind(pool: PgPool) {
|
||||||
|
let db = Db::from_pool(pool);
|
||||||
|
let id = setup_object(&db).await;
|
||||||
|
define(&db, "maker", FieldType::Authority { kind: Some(domain::AuthorityKind::Person) }).await;
|
||||||
|
|
||||||
|
let mut tx = db.pool().begin().await.unwrap();
|
||||||
|
let person = db::authority::create_authority(
|
||||||
|
&mut tx,
|
||||||
|
&domain::NewAuthority {
|
||||||
|
kind: domain::AuthorityKind::Person,
|
||||||
|
external_uri: None,
|
||||||
|
labels: label("Carl"),
|
||||||
|
},
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
let place = db::authority::create_authority(
|
||||||
|
&mut tx,
|
||||||
|
&domain::NewAuthority {
|
||||||
|
kind: domain::AuthorityKind::Place,
|
||||||
|
external_uri: None,
|
||||||
|
labels: label("Stockholm"),
|
||||||
|
},
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
tx.commit().await.unwrap();
|
||||||
|
|
||||||
|
// a person resolves
|
||||||
|
let ok = serde_json::json!({ "maker": person.to_string() });
|
||||||
|
let mut tx = db.pool().begin().await.unwrap();
|
||||||
|
catalog::set_object_fields(&mut tx, AuditActor::System, id, ok.as_object().unwrap()).await.unwrap();
|
||||||
|
tx.commit().await.unwrap();
|
||||||
|
|
||||||
|
// a place is the wrong kind
|
||||||
|
let bad = serde_json::json!({ "maker": place.to_string() });
|
||||||
|
let mut tx = db.pool().begin().await.unwrap();
|
||||||
|
assert!(matches!(
|
||||||
|
catalog::set_object_fields(&mut tx, AuditActor::System, id, bad.as_object().unwrap()).await,
|
||||||
|
Err(FieldError::Unresolved { .. })
|
||||||
|
));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[sqlx::test]
|
||||||
|
async fn localized_text_round_trips(pool: PgPool) {
|
||||||
|
let db = Db::from_pool(pool);
|
||||||
|
let id = setup_object(&db).await;
|
||||||
|
define(&db, "title", FieldType::LocalizedText).await;
|
||||||
|
|
||||||
|
let values = serde_json::json!({ "title": { "sv": "Vas", "en": "Vase" } });
|
||||||
|
let mut tx = db.pool().begin().await.unwrap();
|
||||||
|
catalog::set_object_fields(&mut tx, AuditActor::System, id, values.as_object().unwrap()).await.unwrap();
|
||||||
|
tx.commit().await.unwrap();
|
||||||
|
|
||||||
|
let obj = catalog::object_by_id(db.pool(), id).await.unwrap().unwrap();
|
||||||
|
assert_eq!(obj.fields["title"]["sv"], "Vas");
|
||||||
|
assert_eq!(obj.fields["title"]["en"], "Vase");
|
||||||
|
|
||||||
|
// a non-string member is rejected
|
||||||
|
let bad = serde_json::json!({ "title": { "sv": 5 } });
|
||||||
|
let mut tx = db.pool().begin().await.unwrap();
|
||||||
|
assert!(matches!(
|
||||||
|
catalog::set_object_fields(&mut tx, AuditActor::System, id, bad.as_object().unwrap()).await,
|
||||||
|
Err(FieldError::TypeMismatch { .. })
|
||||||
|
));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[sqlx::test]
|
||||||
|
async fn replace_semantics_remove_a_field_and_audit_it(pool: PgPool) {
|
||||||
|
let db = Db::from_pool(pool);
|
||||||
|
let id = setup_object(&db).await;
|
||||||
|
define(&db, "comments", FieldType::Text).await;
|
||||||
|
define(&db, "year", FieldType::Integer).await;
|
||||||
|
|
||||||
|
// set both
|
||||||
|
let mut tx = db.pool().begin().await.unwrap();
|
||||||
|
catalog::set_object_fields(
|
||||||
|
&mut tx,
|
||||||
|
AuditActor::System,
|
||||||
|
id,
|
||||||
|
serde_json::json!({ "comments": "x", "year": 1850 }).as_object().unwrap(),
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
tx.commit().await.unwrap();
|
||||||
|
|
||||||
|
// replace with only `comments` -> `year` removed
|
||||||
|
let mut tx = db.pool().begin().await.unwrap();
|
||||||
|
catalog::set_object_fields(
|
||||||
|
&mut tx,
|
||||||
|
AuditActor::System,
|
||||||
|
id,
|
||||||
|
serde_json::json!({ "comments": "x" }).as_object().unwrap(),
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
tx.commit().await.unwrap();
|
||||||
|
|
||||||
|
let obj = catalog::object_by_id(db.pool(), id).await.unwrap().unwrap();
|
||||||
|
assert!(obj.fields.get("year").is_none());
|
||||||
|
|
||||||
|
let history = audit::history_for(db.pool(), "object", id.to_uuid()).await.unwrap();
|
||||||
|
let last = history.last().unwrap();
|
||||||
|
let year = last.changes.iter().find(|c| c.field == "year").expect("year removal recorded");
|
||||||
|
assert!(year.before.is_some());
|
||||||
|
assert!(year.after.is_none());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[sqlx::test]
|
||||||
|
async fn no_op_set_records_no_audit(pool: PgPool) {
|
||||||
|
let db = Db::from_pool(pool);
|
||||||
|
let id = setup_object(&db).await;
|
||||||
|
define(&db, "comments", FieldType::Text).await;
|
||||||
|
|
||||||
|
let values = serde_json::json!({ "comments": "x" });
|
||||||
|
let mut tx = db.pool().begin().await.unwrap();
|
||||||
|
catalog::set_object_fields(&mut tx, AuditActor::System, id, values.as_object().unwrap()).await.unwrap();
|
||||||
|
tx.commit().await.unwrap();
|
||||||
|
|
||||||
|
let before = audit::history_for(db.pool(), "object", id.to_uuid()).await.unwrap().len();
|
||||||
|
|
||||||
|
// setting the identical map again is a no-op
|
||||||
|
let mut tx = db.pool().begin().await.unwrap();
|
||||||
|
catalog::set_object_fields(&mut tx, AuditActor::System, id, values.as_object().unwrap()).await.unwrap();
|
||||||
|
tx.commit().await.unwrap();
|
||||||
|
|
||||||
|
let after = audit::history_for(db.pool(), "object", id.to_uuid()).await.unwrap().len();
|
||||||
|
assert_eq!(before, after, "a no-op set must not add an audit entry");
|
||||||
|
}
|
||||||
|
|
||||||
|
#[sqlx::test]
|
||||||
|
async fn set_on_missing_object_errors(pool: PgPool) {
|
||||||
|
let db = Db::from_pool(pool);
|
||||||
|
let mut tx = db.pool().begin().await.unwrap();
|
||||||
|
let err = catalog::set_object_fields(
|
||||||
|
&mut tx,
|
||||||
|
AuditActor::System,
|
||||||
|
domain::ObjectId::new(),
|
||||||
|
serde_json::json!({}).as_object().unwrap(),
|
||||||
|
)
|
||||||
|
.await;
|
||||||
|
assert!(matches!(err, Err(FieldError::ObjectNotFound)));
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 2: Run.** `DATABASE_URL=<url> cargo test -p db --test object_fields` → PASS (8 tests total).
|
||||||
|
|
||||||
|
- [ ] **Step 3: Full workspace check.**
|
||||||
|
```bash
|
||||||
|
cargo +nightly fmt --check
|
||||||
|
DATABASE_URL=<url> cargo clippy --workspace --all-targets -- -D warnings
|
||||||
|
DATABASE_URL=<url> cargo test --workspace
|
||||||
|
```
|
||||||
|
Expected: all green.
|
||||||
|
|
||||||
|
- [ ] **Step 4: Commit.**
|
||||||
|
```bash
|
||||||
|
git add crates/db
|
||||||
|
git commit -m "test(db): cover authority-kind, localized text, replace/remove, no-op, missing object"
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Self-Review (completed)
|
||||||
|
|
||||||
|
**Spec coverage (§6.2 values + validation):**
|
||||||
|
- `fields jsonb` on object; value shapes per type → Tasks 1–2. ✓
|
||||||
|
- Validation against registry (type) + term/authority resolution → Task 2 `validate_field`. ✓
|
||||||
|
- Replace semantics; audited per-field diffs (adds/removes/changes); no-op skip → Task 2 `set_object_fields`/`field_map_changes` + Task 3 tests. ✓
|
||||||
|
- Required-completeness deferred to publish (Plan 7); strict date/format rules deferred to #11. ✓ (intentional)
|
||||||
|
- Typed `FieldError` (uses the `db` crate's `thiserror`). ✓
|
||||||
|
- SQL confined to `db`; `domain` I/O-free (only gains a `serde_json::Value` field). ✓
|
||||||
|
|
||||||
|
**Placeholder scan:** none. `<url>` is the documented `DATABASE_URL`.
|
||||||
|
|
||||||
|
**Type consistency:** `set_object_fields(&mut PgConnection, AuditActor, ObjectId, &serde_json::Map<String, Value>) -> Result<(), FieldError>` used identically across impl + all tests. `FieldError` variants matched in tests. `validate_field` matches on `FieldType` variants from `domain` (Plan 4); `vocab::resolve_term`/`authority::resolve_authority` signatures (Plan 2) used as defined. `field_map_changes` produces `FieldChange` (Plan 1) consumed by `audit::record`. `CatalogueObject.fields` added in Task 1 is read by `map_object` and asserted in tests.
|
||||||
|
|
||||||
|
## Notes for follow-on plans
|
||||||
|
- **Spectrum field-set seeding** (a follow-on): use `reference/spectrum-5.0-cataloguing-units-of-information.md` to seed `field_definition`s (key + type + vocabulary binding for "use a standard term source" fields, authority binding for "form of name" fields).
|
||||||
|
- **Required-field completeness** is enforced at the publish gate (Plan 7): when moving to `Visibility::Public`, check all `required` field definitions have a value.
|
||||||
|
- **Strict per-field validation rules** (date format, ranges, regex) — issue #11.
|
||||||
|
- The API layer (Plan 7+/10) will call `update_object` (core) and `set_object_fields` (flexible) within one transaction for a single logical edit, yielding two audit entries; consider whether to coalesce.
|
||||||
Reference in New Issue
Block a user