mirror of
https://github.com/ModernRelay/omnigraph.git
synced 2026-06-09 01:35:18 +02:00
test: baseline schema-apply coverage for enum-add and metadata-only apply
Pins two foundations for enum-migration work: - Planner + integration coverage that adding a *nullable* enum property is supported (AddProperty), backfills existing rows as NULL, and that the enum value-set is enforced on subsequent writes. Closes the gap where this path was exercised only implicitly via generic nullable-property logic. - A regression test for the metadata-only apply path (UpdateTypeMetadata from an added @description): the manifest version does not advance, yet the schema contract is persisted, `applied` is true, and a reopen re-plans the same source as a no-op. Enum *widen* will ride exactly this path, so the contract is now nailed down before building on it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
d6d92a32a0
commit
550ab8b3d1
2 changed files with 223 additions and 0 deletions
|
|
@ -897,6 +897,63 @@ node Person {
|
|||
}));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn plan_supports_adding_a_nullable_enum_property() {
|
||||
// An enum is just a String-typed PropType carrying `enum_values`,
|
||||
// so adding a *nullable* one rides the same additive path as any
|
||||
// other nullable property: it must emit an AddProperty step (not
|
||||
// UnsupportedChange), with the enum value-set preserved on the
|
||||
// step's PropType. The required-enum case is rejected exactly like
|
||||
// any required addition — see `plan_rejects_required_property_addition`
|
||||
// (the OG-MF-103 backfill path is type-agnostic).
|
||||
let accepted = build_schema_ir(
|
||||
&parse_schema(
|
||||
r#"
|
||||
node Person {
|
||||
name: String @key
|
||||
}
|
||||
"#,
|
||||
)
|
||||
.unwrap(),
|
||||
)
|
||||
.unwrap();
|
||||
let desired = build_schema_ir(
|
||||
&parse_schema(
|
||||
r#"
|
||||
node Person {
|
||||
name: String @key
|
||||
role: enum(admin, guest, member)?
|
||||
}
|
||||
"#,
|
||||
)
|
||||
.unwrap(),
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
let plan = plan_schema_migration(&accepted, &desired).unwrap();
|
||||
assert!(plan.supported, "additive nullable enum must be supported: {plan:?}");
|
||||
assert!(
|
||||
plan.steps.contains(&AddProperty {
|
||||
type_kind: SchemaTypeKind::Node,
|
||||
type_name: "Person".to_string(),
|
||||
property_name: "role".to_string(),
|
||||
property_type: PropType::enum_type(
|
||||
vec!["admin".to_string(), "guest".to_string(), "member".to_string()],
|
||||
true,
|
||||
),
|
||||
}),
|
||||
"expected AddProperty with the enum value-set preserved: {plan:?}",
|
||||
);
|
||||
// Negative: an enum add must not be mistaken for a type change.
|
||||
assert!(
|
||||
!plan
|
||||
.steps
|
||||
.iter()
|
||||
.any(|step| matches!(step, UnsupportedChange { .. })),
|
||||
"nullable enum add must not emit UnsupportedChange: {plan:?}",
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn plan_supports_explicit_type_and_property_rename() {
|
||||
let accepted = build_schema_ir(
|
||||
|
|
|
|||
|
|
@ -60,6 +60,61 @@ async fn apply_schema_noop_returns_not_applied() {
|
|||
assert!(result.steps.is_empty());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn apply_schema_metadata_only_change_persists_and_reports_applied() {
|
||||
// A plan whose only step is metadata-class (here `UpdateTypeMetadata`
|
||||
// from an added `@description`) contributes nothing to the manifest's
|
||||
// table set, so `manifest_changes` is empty and the Lance manifest
|
||||
// version does not advance. The migration is still durable: the new
|
||||
// `_schema.pg` / `_schema.ir.json` / `__schema_state.json` are written
|
||||
// and renamed into place, `applied` is true, and a reopen sees the new
|
||||
// schema as the accepted contract (re-planning the same schema is a
|
||||
// no-op). Enum *widen* rides exactly this path, so pin it here.
|
||||
//
|
||||
// Note: `commit_changes_with_actor` still appends a commit-graph node at
|
||||
// the unchanged manifest version for any metadata-only apply. That is
|
||||
// pre-existing behavior shared by every `@description`-style change and
|
||||
// is out of scope here — not introduced by enum migration.
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let mut db = init_and_load(&dir).await;
|
||||
let people_before = count_rows(&db, "node:Person").await;
|
||||
|
||||
let desired = TEST_SCHEMA.replace(
|
||||
"node Person {",
|
||||
"node Person @description(\"people in the graph\") {",
|
||||
);
|
||||
|
||||
let plan = db.plan_schema(&desired).await.unwrap();
|
||||
assert!(plan.supported, "metadata-only plan must be supported: {plan:?}");
|
||||
assert!(
|
||||
plan.steps.iter().any(|step| matches!(
|
||||
step,
|
||||
SchemaMigrationStep::UpdateTypeMetadata {
|
||||
type_kind: SchemaTypeKind::Node,
|
||||
name,
|
||||
..
|
||||
} if name == "Person"
|
||||
)),
|
||||
"expected UpdateTypeMetadata for Person: {plan:?}",
|
||||
);
|
||||
|
||||
let result = db.apply_schema(&desired).await.unwrap();
|
||||
assert!(result.supported && result.applied, "metadata-only apply must report applied");
|
||||
assert_eq!(count_rows(&db, "node:Person").await, people_before);
|
||||
|
||||
// Reopen: the new schema is the accepted contract (passes the
|
||||
// schema-state validity check) and re-planning the same source is a
|
||||
// no-op, proving the contract files were persisted.
|
||||
let uri = dir.path().to_str().unwrap().to_string();
|
||||
drop(db);
|
||||
let reopened = Omnigraph::open(&uri).await.unwrap();
|
||||
let replan = reopened.plan_schema(&desired).await.unwrap();
|
||||
assert!(
|
||||
replan.supported && replan.steps.is_empty(),
|
||||
"after reopen, re-planning the persisted schema must be a no-op: {replan:?}",
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn apply_schema_rejects_when_non_main_branch_exists() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
|
|
@ -418,6 +473,117 @@ async fn apply_schema_rejects_adding_a_required_property_without_backfill() {
|
|||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn apply_schema_adds_a_nullable_enum_property_and_enforces_value_set() {
|
||||
// The additive-enum contract end-to-end:
|
||||
// 1. Adding a nullable enum property plans as AddProperty (supported).
|
||||
// 2. Apply advances the manifest and backfills existing rows with NULL
|
||||
// for the new column (the stage_overwrite rewrite path).
|
||||
// 3. The enum value-set declared by the migration is enforced on
|
||||
// subsequent writes — out-of-set rejected, in-set accepted — proving
|
||||
// the added enum is a real enum, not just an unconstrained String.
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let mut db = init_and_load(&dir).await;
|
||||
let people_before = count_rows(&db, "node:Person").await;
|
||||
assert!(people_before > 0, "fixture should seed Person rows");
|
||||
let before_version = db
|
||||
.snapshot_of(ReadTarget::branch("main"))
|
||||
.await
|
||||
.unwrap()
|
||||
.version();
|
||||
|
||||
// Add `role: enum(admin, guest, member)?` to Person (nullable → no backfill).
|
||||
let desired = TEST_SCHEMA.replace(
|
||||
" age: I32?\n}",
|
||||
" age: I32?\n role: enum(admin, guest, member)?\n}",
|
||||
);
|
||||
|
||||
// (1) Plan: supported, emits AddProperty for Person.role, no UnsupportedChange.
|
||||
let plan = db.plan_schema(&desired).await.unwrap();
|
||||
assert!(plan.supported, "nullable enum add must be supported: {plan:?}");
|
||||
assert!(
|
||||
plan.steps.iter().any(|step| matches!(
|
||||
step,
|
||||
SchemaMigrationStep::AddProperty {
|
||||
type_kind: SchemaTypeKind::Node,
|
||||
type_name,
|
||||
property_name,
|
||||
..
|
||||
} if type_name == "Person" && property_name == "role"
|
||||
)),
|
||||
"expected AddProperty for Person.role: {plan:?}",
|
||||
);
|
||||
assert!(
|
||||
!plan
|
||||
.steps
|
||||
.iter()
|
||||
.any(|step| matches!(step, SchemaMigrationStep::UnsupportedChange { .. })),
|
||||
"nullable enum add must not emit UnsupportedChange: {plan:?}",
|
||||
);
|
||||
|
||||
let result = db.apply_schema(&desired).await.unwrap();
|
||||
assert!(result.supported && result.applied);
|
||||
|
||||
// (2) Manifest advanced; row count unchanged; `role` column present and
|
||||
// NULL for every pre-existing row.
|
||||
let after_version = db
|
||||
.snapshot_of(ReadTarget::branch("main"))
|
||||
.await
|
||||
.unwrap()
|
||||
.version();
|
||||
assert!(after_version > before_version, "manifest must advance after add");
|
||||
assert_eq!(count_rows(&db, "node:Person").await, people_before);
|
||||
|
||||
let person_batches = read_table(&db, "node:Person").await;
|
||||
assert!(
|
||||
person_batches
|
||||
.iter()
|
||||
.any(|b| b.schema().column_with_name("role").is_some()),
|
||||
"Person dataset schema must include the new 'role' column after add",
|
||||
);
|
||||
assert!(
|
||||
collect_column_strings(&person_batches, "role").is_empty(),
|
||||
"every pre-existing Person row must read NULL for the added enum column",
|
||||
);
|
||||
|
||||
// (3) Enum value-set enforced on writes against the migrated schema.
|
||||
const SET_ROLE: &str = r#"
|
||||
query set_role($name: String, $role: String) {
|
||||
update Person set { role: $role } where name = $name
|
||||
}
|
||||
"#;
|
||||
|
||||
// Out-of-set value rejected (same surface as validators.rs).
|
||||
let err = mutate_main(
|
||||
&mut db,
|
||||
SET_ROLE,
|
||||
"set_role",
|
||||
¶ms(&[("$name", "Alice"), ("$role", "superadmin")]),
|
||||
)
|
||||
.await
|
||||
.unwrap_err();
|
||||
assert!(
|
||||
err.to_string().contains("invalid enum value 'superadmin'"),
|
||||
"added enum must reject out-of-set values; got: {err}",
|
||||
);
|
||||
|
||||
// In-set value accepted and persisted.
|
||||
mutate_main(
|
||||
&mut db,
|
||||
SET_ROLE,
|
||||
"set_role",
|
||||
¶ms(&[("$name", "Alice"), ("$role", "admin")]),
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
let roles_after = collect_column_strings(&read_table(&db, "node:Person").await, "role");
|
||||
assert_eq!(
|
||||
roles_after,
|
||||
vec!["admin".to_string()],
|
||||
"only Alice's role should be set, to the accepted enum value",
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn plan_schema_for_property_type_narrowing_is_not_supported() {
|
||||
// Symmetric companion to `apply_schema_unsupported_plan_does_not_advance_manifest`,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue