mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-09-11 21:39:01 +00:00
feat(copy): the two cross-workspace doors resolve referents, with their pin (TASK-2878)
PLAN-2857 U1, doors seven and eight. `migrateCopyFields` and `handleCopyItemPreflight` now take their relation decision from `store.MigrateRelationReferents` — the same function the four write doors and the two move doors already call, which is the entire reason it exists. The defect this closes: MigrateFields matches on key and TYPE, so a same-named `relation` field carried a SOURCE-workspace item id across the boundary and the preflight reported it as a clean carry. What landed in workspace B was a value naming a row in workspace A — unrenderable, and indistinguishable on read from a legitimate reference. Provenance decides, as at the move doors. A CARRIED value on a cross-workspace copy is dropped without a lookup (no id from A can mean anything in B) and reported through the `dropped_fields` channel BUG-2674 established; a SUPPLIED override is an ordinary write and an unresolvable one is refused, 400 validation_error on both doors, rendered by the same `store.RelationIssuesMessage` so one refusal cannot acquire two phrasings. `internal/server`'s `relationIssuesMessage` now delegates to it: the eighth door refuses from inside `store`, so the sentence had to be reachable there. Two things are threaded rather than re-derived, and both are load-bearing: - The TRANSACTION, not the pool. `migrateCopyFields` becomes a method taking a Queryer, and `copyItemAcrossWorkspacesTx` passes its `tx`. That function has held a transaction since its second statement, so a pool read from inside it can wait for a free connection while every pooled connection is blocked on this transaction's locks — the starvation shape BUG-2409 fixed for the attachment planner and this repo keeps a deterministic test for. This is what the day-70 handoff named as the reason these two doors were not wired with the other six. - The MODE comes from the `scope` MigrateFields was already given, not from a second boundary test. Two independent answers to "is this crossing a workspace" is how one request gets migrated one way and validated the other, and this path also serves a copy whose target IS the source workspace, where relations resolve and survive exactly as on a move. The destination workspace id is the resolution scope: a supplied override is a write into B and must name something that exists there. THE PIN, and why it is not a per-door table. These two doors sit in different PACKAGES and the code at both sites says so is how they drift unnoticed. A table with a row per door can be fully green while the two disagree about one request, which is the defect rather than a gap in coverage of it. So every case sends ONE body to BOTH endpoints: - carried relation — must drop on both, and the preflight must say referent_not_portable rather than the generic no_target_field, which is false here (the destination DOES declare the key, so that answer sends the reader to fix a schema that is fine); - supplied + unresolvable — both refuse, same status, same code, both name the offending field and value, and nothing is written; - supplied + resolvable — the positive control, supplied as a REF so resolution is visible in the result. Without it the first two legs are equally consistent with "relations always fail". Negative controls run, all three mutants BUILD-CHECKED first (a non-compiling mutant produces no `--- FAIL` lines and reads as survived): both doors unwired = DETECTED; preflight unwired alone = DETECTED; store unwired alone = DETECTED. Each single-door mutant failing is the pin's whole claim — neither door can be wired without the other. CONVE-23 sweep: the preflight's LIMITATION comment said this gap belonged in MigrateFields "for both callers at once". That is now false in its prescription as well as its premise — `internal/items` is DB-free by construction and cannot ask whether a string names a live item — so the comment records where the fix actually went and what of it remains open (`computed`, `terminal_options`, `unique_scope`). Gates: internal/server ok 170.0s · internal/store ok 296.1s · internal/items ok · go vet clean · gofmt clean · make lint 0 issues.
This commit is contained in:
@@ -592,13 +592,20 @@ func (s *Server) handleCopyItemPreflight(w http.ResponseWriter, r *http.Request)
|
||||
// existing intra-workspace move path (Codex round 5). Migration
|
||||
// matches on key and type only — plus the option list for a select.
|
||||
// It does not consider a relation field's `collection`, `computed`,
|
||||
// `terminal_options` or `unique_scope`. So a same-named `relation`
|
||||
// field carries a SOURCE-workspace item id into the destination and
|
||||
// is reported as a clean carry, when across workspaces it is a
|
||||
// dangling reference. Reporting that here would require the preflight
|
||||
// to model semantics MigrateFields does not, and the copy would then
|
||||
// disagree with its own preview — the divergence DR-6 exists to
|
||||
// prevent. It belongs in MigrateFields, for both callers at once.
|
||||
// `terminal_options` or `unique_scope`.
|
||||
//
|
||||
// The RELATION half of that is closed as of TASK-2878, and not where
|
||||
// this comment predicted. A same-named `relation` field used to carry a
|
||||
// SOURCE-workspace item id into the destination and be reported as a
|
||||
// clean carry, when across workspaces it is a dangling reference. The fix
|
||||
// did NOT go into MigrateFields: that function is in `internal/items`,
|
||||
// which is DB-free by construction, and deciding whether a string names a
|
||||
// live item in a particular collection is a database question. It went
|
||||
// into `store.MigrateRelationReferents`, called below by this endpoint and
|
||||
// by `migrateCopyFields` — one function, so the preview and the copy still
|
||||
// cannot disagree, which is what the original objection was actually
|
||||
// about. `computed`, `terminal_options` and `unique_scope` remain
|
||||
// unmodelled here.
|
||||
//
|
||||
// MigrateFields computes result.Errors before any override exists, so
|
||||
// those errors are stale the instant an override merges in. They are
|
||||
@@ -649,6 +656,55 @@ func (s *Server) handleCopyItemPreflight(w http.ResponseWriter, r *http.Request)
|
||||
// preflight exists to PREDICT what the copy does, so a coercion on one
|
||||
// side only would make it report a field as failing that the copy accepts.
|
||||
final = items.CoerceFields(final, items.SchemaForMigratedFields(targetSchema))
|
||||
// Relation referents (TASK-2878), through the SAME store function the
|
||||
// mutating copy calls — which is the whole reason that function is in
|
||||
// `store` rather than beside either caller. This endpoint and
|
||||
// `migrateCopyFields` sit in different PACKAGES, and the note above the
|
||||
// CoerceFields call says in as many words that this is how the two drift
|
||||
// unnoticed; a preview that says "carried" while the copy drops is one
|
||||
// request answered two ways, the exact DR-6 divergence this pair exists to
|
||||
// prevent.
|
||||
//
|
||||
// POOL executor (`s.store.MigrateRelationReferents`, not the ...Q form):
|
||||
// the preflight is a read-only dry run holding no transaction. The copy
|
||||
// passes its tx for the reason recorded at that call site.
|
||||
//
|
||||
// Scope computed the same way MigrateFields was given it, a few lines
|
||||
// above — not re-derived, so the two cannot disagree about whether this
|
||||
// request crosses a boundary.
|
||||
relMode := store.RelationCarryWithinWorkspace
|
||||
if items.ScopeFor(item.WorkspaceID, dst.WorkspaceID()) == items.CrossWorkspace {
|
||||
relMode = store.RelationCarryCrossWorkspace
|
||||
}
|
||||
relRefusals, relDropped, relErr := s.store.MigrateRelationReferents(
|
||||
dst.WorkspaceID(), items.SchemaForMigratedFields(targetSchema), final,
|
||||
input.FieldOverrides, relMode)
|
||||
if relErr != nil {
|
||||
writeInternalError(w, fmt.Errorf("copy preflight: resolve relation referents: %w", relErr))
|
||||
return
|
||||
}
|
||||
// An override the caller typed that names nothing is REFUSED here, not
|
||||
// bucketed into needs_value — DR-12's disposition for an override with an
|
||||
// invalid value, which is the branch immediately below. Same 400
|
||||
// validation_error and the same sentence the copy returns, because both
|
||||
// render through store.RelationIssuesMessage.
|
||||
if refuseRelationIssues(w, relRefusals) {
|
||||
return
|
||||
}
|
||||
// Carried values that could not survive join `migrated.Dropped` rather
|
||||
// than a bucket of their own, so `StillDropped` filters them against the
|
||||
// final map exactly as it filters a type-mismatch drop — including the
|
||||
// case where the destination schema's own default re-populates the key,
|
||||
// which makes it a carry again and must not also be reported dropped.
|
||||
// `origin` loses its entry for the same reason: if a default does
|
||||
// re-populate the key, its origin is the destination's default, not the
|
||||
// source value that was just discarded.
|
||||
relationDropReason := make(map[string]string, len(relDropped))
|
||||
for _, ri := range relDropped {
|
||||
migrated.Dropped = append(migrated.Dropped, ri.Key)
|
||||
relationDropReason[ri.Key] = string(ri.Reason)
|
||||
delete(origin, ri.Key)
|
||||
}
|
||||
issues := items.ValidateFieldsDetailed(final, items.SchemaForMigratedFields(targetSchema))
|
||||
|
||||
// DR-12's other half: an override whose VALUE is invalid is rejected,
|
||||
@@ -785,6 +841,21 @@ func (s *Server) handleCopyItemPreflight(w http.ResponseWriter, r *http.Request)
|
||||
for _, key := range sortedDroppedKeys(items.StillDropped(migrated.Dropped, final), sourceSchema.Fields) {
|
||||
reason := "no_target_field"
|
||||
label := key
|
||||
// A relation value whose referent did not survive (TASK-2878). The
|
||||
// reason is the resolver's own — `referent_not_portable` for a carried
|
||||
// value on a cross-workspace copy, or the specific lookup failure
|
||||
// within a workspace — because the generic no_target_field is simply
|
||||
// false here: the destination DOES declare the key, and reporting a
|
||||
// missing field would send the reader to fix a schema that is fine.
|
||||
if relReason, isRelation := relationDropReason[key]; isRelation {
|
||||
if def, exists := targetDefs[key]; exists && def.Label != "" {
|
||||
label = def.Label
|
||||
}
|
||||
resp.Fields.Dropped = append(resp.Fields.Dropped, ItemCopyPreflightDropped{
|
||||
Key: key, Label: label, Kind: "field", Reason: relReason,
|
||||
})
|
||||
continue
|
||||
}
|
||||
// A reserved key in Dropped can only have got there one way: it is
|
||||
// referential and this is a cross-workspace copy. The generic
|
||||
// no_target_field would be actively misleading — no schema declares
|
||||
|
||||
@@ -0,0 +1,272 @@
|
||||
package server
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"net/http"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/PerpetualSoftware/pad/internal/events"
|
||||
"github.com/PerpetualSoftware/pad/internal/models"
|
||||
)
|
||||
|
||||
// Relation referents across the two CROSS-WORKSPACE doors (PLAN-2857 U1 /
|
||||
// TASK-2878).
|
||||
//
|
||||
// THIS IS THE PIN THE COPY PAIR HAD TO LAND WITH, and it is deliberately not a
|
||||
// per-door table. `handleCopyItemPreflight` lives in `internal/server` and
|
||||
// `migrateCopyFields` lives in `internal/store`; the code at both sites carries
|
||||
// a note saying that is exactly how the two drift unnoticed. A table with a
|
||||
// row per door can be fully green while the two doors disagree about the same
|
||||
// request — which is the defect, not a gap in coverage of it. So every case
|
||||
// here sends ONE body to BOTH endpoints and asserts the two answers are the
|
||||
// same answer.
|
||||
//
|
||||
// The three legs are chosen to discriminate rather than to enumerate:
|
||||
//
|
||||
// - carried — the defect being closed. A relation value naming a live item
|
||||
// in the SOURCE workspace used to cross the boundary and be reported as a
|
||||
// clean carry. It must now drop on both doors, and the preflight must say
|
||||
// WHY in a way that is true (referent_not_portable, not no_target_field).
|
||||
// - supplied+unresolvable — the refusal half. Both doors must refuse, with
|
||||
// the same status and code, and nothing may be written.
|
||||
// - supplied+resolvable — the positive control. Without it the first two
|
||||
// legs are equally consistent with "relations always fail", which would
|
||||
// pass a build that simply refused every relation value.
|
||||
|
||||
// relationFixture builds the copy fixture with relation-bearing schemas on
|
||||
// both sides. It reuses copyPreflightFixture so every existing helper —
|
||||
// call / callCopy / ok / copyOK / persistedFields / snapshot — drives these
|
||||
// collections unchanged; only the schemas and the source item differ.
|
||||
type relationFixture struct {
|
||||
*copyPreflightFixture
|
||||
targetsA, targetsB *models.Collection
|
||||
// targetA is a live item in workspace A's target collection: the referent
|
||||
// the source item's carried relation names, so the carried value is VALID
|
||||
// where it starts. A value that was already broken in A would drop for a
|
||||
// reason that has nothing to do with crossing the boundary.
|
||||
targetA *models.Item
|
||||
// targetB is its counterpart in B, the only thing an override can name.
|
||||
targetB *models.Item
|
||||
}
|
||||
|
||||
func newCopyRelationFixture(t *testing.T) *relationFixture {
|
||||
t.Helper()
|
||||
srv := testServer(t)
|
||||
bus := events.New()
|
||||
srv.SetEventBus(bus)
|
||||
t.Cleanup(bus.Close)
|
||||
|
||||
owner := mustUser(t, srv, "rel-owner@example.com", "relowner", "")
|
||||
wsA := mustWorkspace(t, srv, "Rel Source WS", owner.ID)
|
||||
wsB := mustWorkspace(t, srv, "Rel Dest WS", owner.ID)
|
||||
|
||||
// The target collections first: their SLUGS go into the relation
|
||||
// FieldDefs, and a hardcoded guess at how a name slugifies would make
|
||||
// this test's premise depend on CreateCollection's naming rather than on
|
||||
// the behaviour under test.
|
||||
targetsA := mustSchemaCollection(t, srv, wsA.ID, "People A", `{"fields":[]}`)
|
||||
targetsB := mustSchemaCollection(t, srv, wsB.ID, "People B", `{"fields":[]}`)
|
||||
|
||||
relSchema := func(targetSlug string) string {
|
||||
return fmt.Sprintf(`{"fields":[
|
||||
{"key":"status","label":"Status","type":"select","options":["open","done"],"required":true},
|
||||
{"key":"owner_ref","label":"Owner","type":"relation","collection":%q}
|
||||
]}`, targetSlug)
|
||||
}
|
||||
|
||||
collA := mustSchemaCollection(t, srv, wsA.ID, "Rel Tasks A", relSchema(targetsA.Slug))
|
||||
collB := mustSchemaCollection(t, srv, wsB.ID, "Rel Tasks B", relSchema(targetsB.Slug))
|
||||
|
||||
targetA, err := srv.store.CreateItem(wsA.ID, targetsA.ID, models.ItemCreate{
|
||||
Title: "Ada in A", CreatedBy: owner.ID,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("CreateItem(targetA): %v", err)
|
||||
}
|
||||
targetB, err := srv.store.CreateItem(wsB.ID, targetsB.ID, models.ItemCreate{
|
||||
Title: "Grace in B", CreatedBy: owner.ID,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("CreateItem(targetB): %v", err)
|
||||
}
|
||||
|
||||
source, err := srv.store.CreateItem(wsA.ID, collA.ID, models.ItemCreate{
|
||||
Title: "The Related Source",
|
||||
Content: "body",
|
||||
Fields: fmt.Sprintf(`{"status":"open","owner_ref":%q}`, targetA.ID),
|
||||
CreatedBy: owner.ID,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("CreateItem(source): %v", err)
|
||||
}
|
||||
|
||||
return &relationFixture{
|
||||
copyPreflightFixture: ©PreflightFixture{
|
||||
t: t, srv: srv, bus: bus, owner: owner,
|
||||
wsA: wsA, wsB: wsB, collA: collA, collB: collB, hiddenB: targetsB,
|
||||
source: source,
|
||||
},
|
||||
targetsA: targetsA, targetsB: targetsB,
|
||||
targetA: targetA, targetB: targetB,
|
||||
}
|
||||
}
|
||||
|
||||
// droppedReason returns the reason the preflight gave for key, and whether it
|
||||
// reported the key dropped at all.
|
||||
func droppedReason(pre ItemCopyPreflight, key string) (string, bool) {
|
||||
for _, d := range pre.Fields.Dropped {
|
||||
if d.Key == key {
|
||||
return d.Reason, true
|
||||
}
|
||||
}
|
||||
return "", false
|
||||
}
|
||||
|
||||
// carriedValue returns the value the preflight says will land under key.
|
||||
func carriedValue(pre ItemCopyPreflight, key string) (any, bool) {
|
||||
for _, c := range pre.Fields.Carried {
|
||||
if c.Key == key {
|
||||
return c.Value, true
|
||||
}
|
||||
}
|
||||
return nil, false
|
||||
}
|
||||
|
||||
func TestCopyEndpoint_PreflightAndCopyAgreeOnRelationReferents(t *testing.T) {
|
||||
t.Run("a carried relation drops identically on both doors", func(t *testing.T) {
|
||||
f := newCopyRelationFixture(t)
|
||||
body := f.baseBody()
|
||||
|
||||
pre := f.ok(body)
|
||||
|
||||
// The defect, stated positively: the value named a live item in A and
|
||||
// used to arrive in B still naming it. It must not be reported as a
|
||||
// carry.
|
||||
if v, carried := carriedValue(pre, "owner_ref"); carried {
|
||||
t.Fatalf("preflight reports owner_ref CARRYING %#v; a source-workspace "+
|
||||
"referent means nothing in the destination", v)
|
||||
}
|
||||
reason, dropped := droppedReason(pre, "owner_ref")
|
||||
if !dropped {
|
||||
t.Fatalf("preflight reports owner_ref in NEITHER bucket — a field that "+
|
||||
"silently vanishes is worse than one reported wrongly: %+v", pre.Fields)
|
||||
}
|
||||
// The reason is load-bearing. `no_target_field` is what the generic
|
||||
// path would have said and it is false: the destination declares
|
||||
// owner_ref, so that answer sends the reader to fix a schema that is
|
||||
// fine.
|
||||
if reason != "referent_not_portable" {
|
||||
t.Fatalf("preflight dropped owner_ref for reason %q, want referent_not_portable", reason)
|
||||
}
|
||||
|
||||
// The copy's turn — same body, and the agreement assertion is the
|
||||
// point of the pin.
|
||||
res := assertPreflightMatchesCopy(t, f.copyPreflightFixture, "carried relation", body)
|
||||
|
||||
got := f.persistedFields(res.Item.ID)
|
||||
if v, present := got["owner_ref"]; present {
|
||||
t.Fatalf("the copy PERSISTED owner_ref = %#v, a workspace-A item id stored "+
|
||||
"on a workspace-B item", v)
|
||||
}
|
||||
// And the copy's own warnings report it, through the dropped_fields
|
||||
// channel BUG-2674 established — a copy that drops a field silently is
|
||||
// the defect that convention closed, in a new place.
|
||||
if !hasDroppedField(res, "owner_ref") {
|
||||
t.Fatalf("the copy dropped owner_ref without reporting it in warnings.dropped_fields: %+v", res)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("an unresolvable supplied override refuses identically on both doors", func(t *testing.T) {
|
||||
f := newCopyRelationFixture(t)
|
||||
body := f.baseBody()
|
||||
// A ref shaped like a real one and naming nothing. Not a slug and not
|
||||
// free text: those are refused by the resolver for a DIFFERENT reason
|
||||
// (no slug fallback), and a fixture rejectable two ways discriminates
|
||||
// nothing.
|
||||
body["field_overrides"] = map[string]any{"owner_ref": "PEOP-9999"}
|
||||
|
||||
before := f.snapshot()
|
||||
|
||||
preRR := f.call(f.owner, reqOpts{}, body)
|
||||
copyRR := f.callCopy(f.owner, reqOpts{}, body)
|
||||
|
||||
if preRR.Code != http.StatusBadRequest {
|
||||
t.Fatalf("preflight: expected 400, got %d: %s", preRR.Code, preRR.Body.String())
|
||||
}
|
||||
if copyRR.Code != preRR.Code {
|
||||
t.Fatalf("the copy answered %d where the preflight answered %d — the preview "+
|
||||
"lies about whether the request is acceptable:\n preflight: %s\n copy: %s",
|
||||
copyRR.Code, preRR.Code, preRR.Body.String(), copyRR.Body.String())
|
||||
}
|
||||
if pc, cc := errCode(t, preRR), errCode(t, copyRR); pc != cc {
|
||||
t.Fatalf("error codes differ: preflight %q, copy %q", pc, cc)
|
||||
}
|
||||
if got := errCode(t, copyRR); got != "validation_error" {
|
||||
t.Fatalf("copy error code = %q, want validation_error", got)
|
||||
}
|
||||
// Both refusals name the offending value, so a client can fix its
|
||||
// request rather than guess which override was rejected.
|
||||
for _, rr := range []struct {
|
||||
label string
|
||||
body string
|
||||
}{{"preflight", preRR.Body.String()}, {"copy", copyRR.Body.String()}} {
|
||||
if !containsAll(rr.body, "owner_ref", "PEOP-9999") {
|
||||
t.Errorf("%s refusal does not name the field and value: %s", rr.label, rr.body)
|
||||
}
|
||||
}
|
||||
|
||||
// Nothing was written. "Refused" and "created it anyway without the
|
||||
// key" both produce a non-201 if only the status is checked.
|
||||
if after := f.snapshot(); after != before {
|
||||
t.Fatalf("a refused copy mutated state:\n before: %+v\n after: %+v", before, after)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("a resolvable supplied override carries, canonicalised", func(t *testing.T) {
|
||||
f := newCopyRelationFixture(t)
|
||||
body := f.baseBody()
|
||||
// Supplied as a REF, not a UUID: resolving is then visible in the
|
||||
// result, because what lands is the target's ID. A door that accepted
|
||||
// the string without resolving would store "PEOP-1" and pass an
|
||||
// equality check against itself.
|
||||
body["field_overrides"] = map[string]any{"owner_ref": f.targetB.Ref}
|
||||
|
||||
pre := f.ok(body)
|
||||
v, carried := carriedValue(pre, "owner_ref")
|
||||
if !carried {
|
||||
t.Fatalf("preflight does not carry a resolvable override: %+v", pre.Fields)
|
||||
}
|
||||
if v != f.targetB.ID {
|
||||
t.Fatalf("preflight carries owner_ref = %#v, want the resolved id %q — a ref "+
|
||||
"that survives unresolved is a value the destination cannot render", v, f.targetB.ID)
|
||||
}
|
||||
|
||||
res := assertPreflightMatchesCopy(t, f.copyPreflightFixture, "resolvable relation override", body)
|
||||
|
||||
got := f.persistedFields(res.Item.ID)
|
||||
if got["owner_ref"] != f.targetB.ID {
|
||||
t.Fatalf("the copy persisted owner_ref = %#v, want %q", got["owner_ref"], f.targetB.ID)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// hasDroppedField reports whether the copy's own response warned that key was
|
||||
// dropped.
|
||||
func hasDroppedField(res ItemCopyResult, key string) bool {
|
||||
for _, k := range res.Warnings.DroppedFields {
|
||||
if k == key {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
func containsAll(s string, subs ...string) bool {
|
||||
for _, sub := range subs {
|
||||
if !strings.Contains(s, sub) {
|
||||
return false
|
||||
}
|
||||
}
|
||||
return true
|
||||
}
|
||||
@@ -2,7 +2,6 @@ package server
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"strings"
|
||||
|
||||
"github.com/PerpetualSoftware/pad/internal/models"
|
||||
"github.com/PerpetualSoftware/pad/internal/store"
|
||||
@@ -124,10 +123,12 @@ func refuseRelationIssues(w http.ResponseWriter, issues []store.RelationIssue) b
|
||||
// and the two bulk operations (*bulkOpError). Split out rather than duplicated
|
||||
// so a caller cannot accidentally produce a different sentence for the same
|
||||
// refusal depending on which door it came through.
|
||||
//
|
||||
// DELEGATES to store.RelationIssuesMessage rather than joining here (TASK-2878).
|
||||
// The eighth door refuses inside `internal/store` — the cross-workspace copy,
|
||||
// through *FieldValidationError — so the sentence has to be reachable from
|
||||
// there too. Two joins in two packages is how one refusal acquires two
|
||||
// phrasings, which is the drift this unit exists to remove.
|
||||
func relationIssuesMessage(issues []store.RelationIssue) string {
|
||||
parts := make([]string, 0, len(issues))
|
||||
for _, ri := range issues {
|
||||
parts = append(parts, ri.Message())
|
||||
}
|
||||
return strings.Join(parts, "; ")
|
||||
return store.RelationIssuesMessage(issues)
|
||||
}
|
||||
|
||||
@@ -621,7 +621,17 @@ func (s *Store) copyItemAcrossWorkspacesTx(req CrossWorkspaceCopyRequest, source
|
||||
// computes it the same way — a divergence here would have the preview
|
||||
// promising a carry the copy drops, which DR-6 exists to prevent.
|
||||
scope := items.ScopeFor(sourceWorkspaceID, req.TargetWorkspaceID)
|
||||
finalFields, dropped, err := migrateCopyFields(source.Fields, sourceColl.Schema, targetColl.Schema, req.FieldOverrides, scope)
|
||||
// The TRANSACTION is the executor, not the pool. This function now reads
|
||||
// (relation referents, TASK-2878), and `copyItemAcrossWorkspacesTx` has
|
||||
// held a transaction since its second statement — a pool read from inside
|
||||
// it can wait for a free connection while every pooled connection is
|
||||
// itself blocked on this transaction's locks, which is the starvation
|
||||
// shape BUG-2409 fixed for the attachment planner and this repo keeps a
|
||||
// deterministic test for.
|
||||
//
|
||||
// The DESTINATION workspace id, not the source's: a supplied override is
|
||||
// a write into workspace B and has to name something that exists THERE.
|
||||
finalFields, dropped, err := s.migrateCopyFields(tx, req.TargetWorkspaceID, source.Fields, sourceColl.Schema, targetColl.Schema, req.FieldOverrides, scope)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
@@ -1045,7 +1055,7 @@ func (s *Store) getCollectionInWorkspaceTx(tx *sql.Tx, collectionID, workspaceID
|
||||
//
|
||||
// Returns the final field map (the planner's input, pre-rewrite) and the keys
|
||||
// migration dropped.
|
||||
func migrateCopyFields(sourceFieldsJSON, sourceSchemaJSON, targetSchemaJSON string, overrides map[string]any, scope items.MigrateScope) (map[string]any, []string, error) {
|
||||
func (s *Store) migrateCopyFields(q Queryer, destWorkspaceID, sourceFieldsJSON, sourceSchemaJSON, targetSchemaJSON string, overrides map[string]any, scope items.MigrateScope) (map[string]any, []string, error) {
|
||||
var sourceSchema, targetSchema models.CollectionSchema
|
||||
if err := json.Unmarshal([]byte(sourceSchemaJSON), &sourceSchema); err != nil {
|
||||
return nil, nil, fmt.Errorf("copy item across workspaces: parse source schema: %w", err)
|
||||
@@ -1089,6 +1099,46 @@ func migrateCopyFields(sourceFieldsJSON, sourceSchemaJSON, targetSchemaJSON stri
|
||||
// note there; these two live in different PACKAGES, which is exactly how
|
||||
// they would drift unnoticed.
|
||||
migrated.Fields = items.CoerceFields(migrated.Fields, items.SchemaForMigratedFields(targetSchema))
|
||||
// Relation referents (TASK-2878) — the eighth and last coercion door, and
|
||||
// the only one that refuses from inside `store`.
|
||||
//
|
||||
// A migrate door, so PROVENANCE decides rather than the door: a SUPPLIED
|
||||
// override is an ordinary write and an unresolvable one is refused; a
|
||||
// CARRIED value was asserted by nobody, so it is dropped and reported
|
||||
// through the same `dropped_fields` channel BUG-2674 established. The
|
||||
// alternative — refusing carried values — would make every legacy item
|
||||
// uncopyable, and `internal/items` has accepted any string for a relation
|
||||
// all along, so "legacy" is most of them.
|
||||
//
|
||||
// MODE COMES FROM `scope`, the same value MigrateFields was given, rather
|
||||
// than a second flag derived here. Two independent answers to "is this
|
||||
// crossing a workspace boundary" is how one request gets migrated one way
|
||||
// and validated the other; this path also serves a copy whose target IS
|
||||
// the source workspace, where relations resolve and survive exactly as
|
||||
// they do on a move.
|
||||
mode := RelationCarryWithinWorkspace
|
||||
if scope == items.CrossWorkspace {
|
||||
mode = RelationCarryCrossWorkspace
|
||||
}
|
||||
relRefusals, relDropped, relErr := s.MigrateRelationReferentsQ(q, destWorkspaceID,
|
||||
items.SchemaForMigratedFields(targetSchema), migrated.Fields, overrides, mode)
|
||||
if relErr != nil {
|
||||
return nil, nil, fmt.Errorf("copy item across workspaces: resolve relation referents: %w", relErr)
|
||||
}
|
||||
if len(relRefusals) > 0 {
|
||||
// The same 400 validation_error the preflight's refuseRelationIssues
|
||||
// emits, through the channel this function already uses for a failed
|
||||
// destination validation — so the copy and its preview refuse one
|
||||
// request with one code and one sentence.
|
||||
return nil, nil, &FieldValidationError{Err: errors.New(RelationIssuesMessage(relRefusals))}
|
||||
}
|
||||
// Appended to Dropped rather than reported separately: StillDropped below
|
||||
// filters this list against the FINAL map, and MigrateRelationReferentsQ
|
||||
// has already deleted these keys from it, so they survive that filter and
|
||||
// reach `warnings.dropped_fields` exactly as a type-mismatch drop does.
|
||||
for _, ri := range relDropped {
|
||||
migrated.Dropped = append(migrated.Dropped, ri.Key)
|
||||
}
|
||||
if err := items.ValidateFields(migrated.Fields, items.SchemaForMigratedFields(targetSchema)); err != nil {
|
||||
return nil, nil, &FieldValidationError{Err: err}
|
||||
}
|
||||
|
||||
@@ -73,6 +73,21 @@ func (ri RelationIssue) Message() string {
|
||||
}
|
||||
}
|
||||
|
||||
// RelationIssuesMessage renders a set of issues as the ONE sentence every
|
||||
// refusing door reports. It lives here rather than in `internal/server`
|
||||
// because the store's own copy door refuses too (through
|
||||
// `FieldValidationError`), and a second join in a second package is how the
|
||||
// same refusal comes to be phrased two ways — the drift this whole unit
|
||||
// exists to remove. `internal/server`'s `relationIssuesMessage` delegates to
|
||||
// this.
|
||||
func RelationIssuesMessage(issues []RelationIssue) string {
|
||||
parts := make([]string, 0, len(issues))
|
||||
for _, ri := range issues {
|
||||
parts = append(parts, ri.Message())
|
||||
}
|
||||
return strings.Join(parts, "; ")
|
||||
}
|
||||
|
||||
// ResolveRelationReferents canonicalises every `relation` value in fieldMap to
|
||||
// the target item's ID and reports the ones that cannot be resolved.
|
||||
//
|
||||
|
||||
Reference in New Issue
Block a user