mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-10-26 14:30:13 +00:00
feat(relations): exact-title resolution and hydrated reads for relation fields (PLAN-2857 U6 / TASK-2996) (#1326)
PLAN-2857 unit U6, closes TASK-2996.
U1 made a relation value have to name a live item. U6 adds the third
spelling a human actually knows — the item's TITLE — and the read side
that lets a client render any of them.
- a value may be a UUID, an issue ref, or an EXACT TITLE, in that
order, so an item literally titled COLO-3 is unreachable by title
while the ref COLO-3 resolves;
- title matching is scoped to the collection the field declares: a
title unique only workspace-wide REFUSES naming the collection
searched, and two or more matches inside it are `ambiguous` rather
than `not_found`, which would state the opposite of what happened;
- reads carry `relation_targets` — field key to {id, ref, title} —
BESIDE `fields` rather than inside it, so read-modify-write keeps
working;
- the --fields DSL takes a relation's target collection. It used to put
the third part into Options and build a field with NO target, which
every subsequent write then refused with target_missing; a bare
`owner:relation` is refused at parse time now;
- ToolSurfaceVersion 0.31 — a behaviour bump, because that DSL refusal
is a call that used to be accepted.
THE RULED PROPERTY: among live exact-title matches in the declared
collection, only VISIBLE ones count — zero is not_found, exactly one
RESOLVES, two or more is ambiguous — and a hidden match never changes the
answer a caller gets.
It took three attempts to put that in the right place. It was a
SERVER-SIDE second pass first, which left the cross-workspace copy —
running the resolver inside its own transaction — deciding without it, so
preflight and copy disagreed about one input. It lives in the resolver
now, taking the store.RelationVisibilityFunc this codebase already had
for the shape, and which resolveRelationsForWrite's own comment had named
as the missing piece. nil means no requester and every live match counts.
CARRIED values are exempt by provenance: nobody asserted them, and
judging them by the mover's visibility is what MigrateRelationReferentsQ's
comment calls out as making the stored bytes depend on who moved the
item. Supplied values and injected defaults are judged.
EIGHT CODEX ROUNDS; seven found something real. The ones worth keeping:
a CAP on the candidate list was itself an oracle (one visible plus cap
hidden read differently from one visible plus cap+1); the paging cursor
used the NULLABLE item_number and excluded legacy rows the deciding count
included; hydration filtered on COLLECTION visibility, which is
documented as deliberately nav-lenient, so an item-level grant on one row
exposed every sibling's ref and title; and hydration scanned that
nullable item_number into an int, so one pre-006 row erased
relation_targets from the ENTIRE response.
Round 7's "P1" about changed exported signatures was REJECTED, not fixed:
internal/store cannot be imported outside this module, so the compiler
had already proved every caller updated — which is why a required
parameter is right rather than a variadic a caller can silently omit.
TESTING: every fix carries a mutation. Three verdicts were thrown away
rather than counted — two mutants did not COMPILE (a build failure
produces FAIL lines identical to a real kill) and one mutation never
applied. In one sub-task three successive tests passed for reasons that
had not been established: a field that never arrived, a fixture whose
encoder filtered the key before the wire, and a schema reshape that
silently failed. One mutation is recorded UNCOVERED rather than dropped:
the canSee==nil count-before-nil-check is a race with no seam to force,
so that fix is argued from the code path and not demonstrated.
Filed, not fixed: BUG-3012 (ref-path TOCTOU), BUG-3013 (U2 chip wording),
BUG-3014 (bundle-import title renders as dangling), BUG-3015 (the
relationsCarry posture drops silently, medium).
Gates on 29bb6e2c: go test ./... 0, vet 0, gofmt clean, lint 0 issues,
web vitest green. All 8 CI checks green.
Claude-Session: https://claude.ai/code/session_01PRcuviXka4Un1FGrBrX8jc
This commit is contained in:
@@ -168,8 +168,13 @@ Two ways to define the schema:
|
|||||||
|
|
||||||
--fields Compact DSL for the simple case: key:type[:option1,option2,...]
|
--fields Compact DSL for the simple case: key:type[:option1,option2,...]
|
||||||
Separate multiple fields with semicolons. Does not support
|
Separate multiple fields with semicolons. Does not support
|
||||||
terminal_options, custom defaults, computed fields, suffixes,
|
terminal_options, custom defaults, computed fields, or suffixes.
|
||||||
or relation collections.
|
|
||||||
|
For a relation the third part is the TARGET COLLECTION slug
|
||||||
|
rather than an options list, and it is required:
|
||||||
|
owner:relation:people
|
||||||
|
A relation with no target is refused, because such a field
|
||||||
|
cannot accept any value.
|
||||||
|
|
||||||
--schema Full CollectionSchema JSON for everything else. Accepts:
|
--schema Full CollectionSchema JSON for everything else. Accepts:
|
||||||
inline JSON: --schema '{"fields":[...]}'
|
inline JSON: --schema '{"fields":[...]}'
|
||||||
@@ -181,6 +186,7 @@ Two ways to define the schema:
|
|||||||
Examples:
|
Examples:
|
||||||
pad collection create "Bugs" --fields "status:select:new,triaged,fixing,resolved;severity:select:low,medium,high,critical;component:text"
|
pad collection create "Bugs" --fields "status:select:new,triaged,fixing,resolved;severity:select:low,medium,high,critical;component:text"
|
||||||
pad collection create "Decisions" --icon "⚖️" --fields "status:select:proposed,accepted,rejected;impact:select:low,medium,high"
|
pad collection create "Decisions" --icon "⚖️" --fields "status:select:proposed,accepted,rejected;impact:select:low,medium,high"
|
||||||
|
pad collection create "Reviews" --fields "status:select:open,done;reviewer:relation:people"
|
||||||
pad collection create "Marketing" --schema '{"fields":[{"key":"status","label":"Status","type":"select","options":["idea","drafting","review","published","archived"],"terminal_options":["published","archived"],"default":"idea","required":true}]}'
|
pad collection create "Marketing" --schema '{"fields":[{"key":"status","label":"Status","type":"select","options":["idea","drafting","review","published","archived"],"terminal_options":["published","archived"],"default":"idea","required":true}]}'
|
||||||
pad collection create "Marketing" --schema @./marketing-schema.json
|
pad collection create "Marketing" --schema @./marketing-schema.json
|
||||||
cat schema.json | pad collection create "Marketing" --schema -
|
cat schema.json | pad collection create "Marketing" --schema -
|
||||||
|
|||||||
@@ -776,6 +776,15 @@ func showCmd() *cobra.Command {
|
|||||||
if item.Convention != nil && (k == "category" || k == "trigger" || k == "scope" || k == "priority" || k == "enforcement" || k == "surfaces" || k == "commands") {
|
if item.Convention != nil && (k == "category" || k == "trigger" || k == "scope" || k == "priority" || k == "enforcement" || k == "surfaces" || k == "commands") {
|
||||||
continue // shown in dedicated section below
|
continue // shown in dedicated section below
|
||||||
}
|
}
|
||||||
|
// U6: a relation's stored value is an item id, which
|
||||||
|
// tells a reader nothing. The response already carries
|
||||||
|
// the hydrated target, so this is a render, not a
|
||||||
|
// lookup — and it falls back to the raw value when the
|
||||||
|
// field is not a relation or was not hydrated.
|
||||||
|
if target, ok := item.RelationTargets[k]; ok {
|
||||||
|
fmt.Printf("%-12s %s\n", k+":", cli.RenderRelationValue(target))
|
||||||
|
continue
|
||||||
|
}
|
||||||
fmt.Printf("%-12s %v\n", k+":", v)
|
fmt.Printf("%-12s %v\n", k+":", v)
|
||||||
}
|
}
|
||||||
fmt.Println("---")
|
fmt.Println("---")
|
||||||
|
|||||||
@@ -20,6 +20,10 @@ import (
|
|||||||
// `tags` are emitted as nested JSON (not escaped strings) so agents can read
|
// `tags` are emitted as nested JSON (not escaped strings) so agents can read
|
||||||
// them without a second parse.
|
// them without a second parse.
|
||||||
//
|
//
|
||||||
|
// The one id-bearing member it KEEPS is `relation_targets` (PLAN-2857 U6) —
|
||||||
|
// see the field's own comment for why a relation is the exception to the
|
||||||
|
// dropped-UUID rule.
|
||||||
|
//
|
||||||
// The full shape (raw models.Item) is still available via `item list --full`.
|
// The full shape (raw models.Item) is still available via `item list --full`.
|
||||||
type ItemSummary struct {
|
type ItemSummary struct {
|
||||||
Ref string `json:"ref,omitempty"`
|
Ref string `json:"ref,omitempty"`
|
||||||
@@ -35,8 +39,23 @@ type ItemSummary struct {
|
|||||||
AssignedUser string `json:"assigned_user,omitempty"`
|
AssignedUser string `json:"assigned_user,omitempty"`
|
||||||
AgentRole string `json:"agent_role,omitempty"`
|
AgentRole string `json:"agent_role,omitempty"`
|
||||||
HasChildren bool `json:"has_children,omitempty"`
|
HasChildren bool `json:"has_children,omitempty"`
|
||||||
CreatedAt time.Time `json:"created_at"`
|
|
||||||
UpdatedAt time.Time `json:"updated_at"`
|
// RelationTargets carries the hydrated `relation` values (PLAN-2857 U6).
|
||||||
|
//
|
||||||
|
// It survives the summary projection while the other UUID plumbing does
|
||||||
|
// not, because it is the ONE case where the id in `fields` is useless on
|
||||||
|
// its own: a relation stores an item id, and without this member a client
|
||||||
|
// reading a list has a UUID and no way to render it short of a request per
|
||||||
|
// value. That is the defect U6 exists to close, and dropping it here would
|
||||||
|
// leave it open on the agent doors — `pad item list --format json` and
|
||||||
|
// `pad_item.list` on both transports — which are the doors that matter
|
||||||
|
// most.
|
||||||
|
//
|
||||||
|
// Small by construction: one entry per relation field the item actually
|
||||||
|
// carries, and absent entirely on the items that carry none.
|
||||||
|
RelationTargets map[string]models.RelationTarget `json:"relation_targets,omitempty"`
|
||||||
|
CreatedAt time.Time `json:"created_at"`
|
||||||
|
UpdatedAt time.Time `json:"updated_at"`
|
||||||
}
|
}
|
||||||
|
|
||||||
// contentPreviewLimit caps the content_preview at a small, agent-friendly
|
// contentPreviewLimit caps the content_preview at a small, agent-friendly
|
||||||
@@ -97,21 +116,22 @@ func rawJSONOrNil(s string) json.RawMessage {
|
|||||||
// ToItemSummary projects a single models.Item into the summary shape.
|
// ToItemSummary projects a single models.Item into the summary shape.
|
||||||
func ToItemSummary(item models.Item) ItemSummary {
|
func ToItemSummary(item models.Item) ItemSummary {
|
||||||
return ItemSummary{
|
return ItemSummary{
|
||||||
Ref: item.Ref,
|
Ref: item.Ref,
|
||||||
Title: item.Title,
|
Title: item.Title,
|
||||||
Slug: item.Slug,
|
Slug: item.Slug,
|
||||||
CollectionSlug: item.CollectionSlug,
|
CollectionSlug: item.CollectionSlug,
|
||||||
ItemNumber: item.ItemNumber,
|
ItemNumber: item.ItemNumber,
|
||||||
Fields: rawJSONOrNil(item.Fields),
|
Fields: rawJSONOrNil(item.Fields),
|
||||||
Tags: rawJSONOrNil(item.Tags),
|
Tags: rawJSONOrNil(item.Tags),
|
||||||
Pinned: item.Pinned,
|
Pinned: item.Pinned,
|
||||||
ContentPreview: contentPreview(item.Content),
|
ContentPreview: contentPreview(item.Content),
|
||||||
ParentRef: item.ParentRef,
|
ParentRef: item.ParentRef,
|
||||||
AssignedUser: item.AssignedUserName,
|
AssignedUser: item.AssignedUserName,
|
||||||
AgentRole: item.AgentRoleSlug,
|
AgentRole: item.AgentRoleSlug,
|
||||||
HasChildren: item.HasChildren,
|
HasChildren: item.HasChildren,
|
||||||
CreatedAt: item.CreatedAt,
|
RelationTargets: item.RelationTargets,
|
||||||
UpdatedAt: item.UpdatedAt,
|
CreatedAt: item.CreatedAt,
|
||||||
|
UpdatedAt: item.UpdatedAt,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,39 @@
|
|||||||
|
package cli
|
||||||
|
|
||||||
|
import (
|
||||||
|
"fmt"
|
||||||
|
|
||||||
|
"github.com/PerpetualSoftware/pad/internal/models"
|
||||||
|
)
|
||||||
|
|
||||||
|
// RenderRelationValue is the ONE place a hydrated `relation` value becomes text
|
||||||
|
// for a human (PLAN-2857 U6).
|
||||||
|
//
|
||||||
|
// A relation stores an item id, so the raw stored value is a UUID and reading
|
||||||
|
// it tells a person nothing. `relation_targets` already carries what is needed
|
||||||
|
// to render it, so this is a formatter over data the response holds — no
|
||||||
|
// lookup, and nothing here can fail.
|
||||||
|
//
|
||||||
|
// resolved -> "COLO-3 · Red"
|
||||||
|
// id-only -> "6f2c… (unavailable)"
|
||||||
|
//
|
||||||
|
// WHY "unavailable" AND NOT "deleted". An id-only entry has TWO causes that
|
||||||
|
// this layer cannot tell apart: the target is gone, or the requester may not
|
||||||
|
// see it. "(deleted)" would assert non-existence for both, which is false for
|
||||||
|
// half of them and is an existence claim the response does not support — the
|
||||||
|
// same defect codex round 12 found in the copy dialog, where the UI read
|
||||||
|
// `not_found` as "does not exist" when the server also emits it for a target
|
||||||
|
// the caller merely cannot see. The server works to make those two
|
||||||
|
// indistinguishable; the renderer must not undo that in the last inch.
|
||||||
|
func RenderRelationValue(target models.RelationTarget) string {
|
||||||
|
if target.Ref == "" {
|
||||||
|
return fmt.Sprintf("%s (unavailable)", target.ID)
|
||||||
|
}
|
||||||
|
if target.Title == "" {
|
||||||
|
// Ref without a title should not happen — they come from the same row —
|
||||||
|
// but rendering "COLO-3 · " would look like a bug in the data rather
|
||||||
|
// than in this function.
|
||||||
|
return target.Ref
|
||||||
|
}
|
||||||
|
return fmt.Sprintf("%s · %s", target.Ref, target.Title)
|
||||||
|
}
|
||||||
@@ -0,0 +1,52 @@
|
|||||||
|
package cli
|
||||||
|
|
||||||
|
import (
|
||||||
|
"strings"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"github.com/PerpetualSoftware/pad/internal/models"
|
||||||
|
)
|
||||||
|
|
||||||
|
// TestRenderRelationValue covers the three shapes a hydrated relation can
|
||||||
|
// arrive in, and one thing the renderer must NOT say.
|
||||||
|
func TestRenderRelationValue(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
t.Run("resolved renders ref and title", func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
got := RenderRelationValue(models.RelationTarget{ID: "abc", Ref: "COLO-3", Title: "Red"})
|
||||||
|
if got != "COLO-3 · Red" {
|
||||||
|
t.Errorf("got %q, want %q", got, "COLO-3 · Red")
|
||||||
|
}
|
||||||
|
if strings.Contains(got, "abc") {
|
||||||
|
t.Error("the raw id leaked into a fully-resolved render, which is the noise U6 exists to remove")
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("id-only keeps the id and claims nothing about it", func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
got := RenderRelationValue(models.RelationTarget{ID: "6f2c-dead"})
|
||||||
|
if !strings.Contains(got, "6f2c-dead") {
|
||||||
|
t.Errorf("got %q, want the stored id retained — it is all the caller has", got)
|
||||||
|
}
|
||||||
|
// THE POINT OF THIS LEG. An id-only entry means either the target is
|
||||||
|
// gone or the requester may not see it, and this layer cannot tell
|
||||||
|
// which. Saying "deleted" asserts non-existence for a target that may
|
||||||
|
// be alive and merely hidden — the existence oracle the server's
|
||||||
|
// collapse exists to prevent, and the exact defect codex round 12
|
||||||
|
// found in the copy dialog's not_found wording.
|
||||||
|
for _, forbidden := range []string{"deleted", "removed", "does not exist", "missing"} {
|
||||||
|
if strings.Contains(strings.ToLower(got), forbidden) {
|
||||||
|
t.Errorf("render %q claims %q; an id-only entry covers BOTH a gone target and a hidden one, so a non-existence claim is false for half the cases", got, forbidden)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("ref without a title does not render a dangling separator", func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
got := RenderRelationValue(models.RelationTarget{ID: "abc", Ref: "COLO-3"})
|
||||||
|
if got != "COLO-3" {
|
||||||
|
t.Errorf("got %q, want %q", got, "COLO-3")
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
@@ -47,7 +47,31 @@ func ParseFieldsDSL(fieldsDSL string) (models.CollectionSchema, error) {
|
|||||||
Label: cases.Title(language.English).String(strings.ReplaceAll(parts[0], "_", " ")),
|
Label: cases.Title(language.English).String(strings.ReplaceAll(parts[0], "_", " ")),
|
||||||
Type: parts[1],
|
Type: parts[1],
|
||||||
}
|
}
|
||||||
if len(parts) == 3 && parts[2] != "" {
|
if fd.Type == "relation" {
|
||||||
|
// PLAN-2857 U6. For a relation the third part is the TARGET
|
||||||
|
// COLLECTION slug, not an options list.
|
||||||
|
//
|
||||||
|
// It used to fall into Options like every other type, which did not
|
||||||
|
// merely fail to declare the target: it produced a relation field
|
||||||
|
// with Options=["colors"] and NO target, and every write to that
|
||||||
|
// field was then refused with `target_missing`. A DSL that builds a
|
||||||
|
// field nothing can ever write to is worse than one that refuses,
|
||||||
|
// so a bare `owner:relation` is now an error at parse time.
|
||||||
|
target := ""
|
||||||
|
if len(parts) == 3 {
|
||||||
|
target = strings.TrimSpace(parts[2])
|
||||||
|
}
|
||||||
|
if target == "" {
|
||||||
|
return schema, fmt.Errorf("invalid field definition: %q — a relation needs its target collection (expected %s:relation:<collection-slug>)", f, fd.Key)
|
||||||
|
}
|
||||||
|
if strings.Contains(target, ",") {
|
||||||
|
// A comma here means the caller wrote an options list, which is
|
||||||
|
// the pre-U6 shape. Refusing names the mistake; accepting the
|
||||||
|
// first item would silently pick a target they did not choose.
|
||||||
|
return schema, fmt.Errorf("invalid field definition: %q — a relation takes ONE target collection, not a list", f)
|
||||||
|
}
|
||||||
|
fd.Collection = target
|
||||||
|
} else if len(parts) == 3 && parts[2] != "" {
|
||||||
fd.Options = strings.Split(parts[2], ",")
|
fd.Options = strings.Split(parts[2], ",")
|
||||||
}
|
}
|
||||||
if fd.Type == "select" && fd.Key == "status" {
|
if fd.Type == "select" && fd.Key == "status" {
|
||||||
|
|||||||
@@ -0,0 +1,81 @@
|
|||||||
|
package collections
|
||||||
|
|
||||||
|
import (
|
||||||
|
"strings"
|
||||||
|
"testing"
|
||||||
|
)
|
||||||
|
|
||||||
|
// The `--fields` DSL and relation targets (PLAN-2857 U6).
|
||||||
|
//
|
||||||
|
// The gap this closes was not "the DSL cannot declare a target". It was worse:
|
||||||
|
// `owner:relation:colors` put "colors" into Options like every other type, so
|
||||||
|
// it built a relation field with NO target collection — and every write to that
|
||||||
|
// field was then refused with `target_missing`. The DSL silently produced a
|
||||||
|
// field nothing could ever write to.
|
||||||
|
|
||||||
|
func TestParseFieldsDSL_RelationTakesTheTargetCollection(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
schema, err := ParseFieldsDSL("owner:relation:people")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("ParseFieldsDSL: %v", err)
|
||||||
|
}
|
||||||
|
if len(schema.Fields) != 1 {
|
||||||
|
t.Fatalf("want 1 field, got %d", len(schema.Fields))
|
||||||
|
}
|
||||||
|
fd := schema.Fields[0]
|
||||||
|
if fd.Collection != "people" {
|
||||||
|
t.Errorf("Collection = %q, want %q", fd.Collection, "people")
|
||||||
|
}
|
||||||
|
if len(fd.Options) != 0 {
|
||||||
|
t.Errorf("Options = %v, want empty — putting the target in Options is the defect this closes, and it leaves the field unwritable", fd.Options)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// A bare `owner:relation` is REFUSED rather than producing the unusable field
|
||||||
|
// the old parser built. The error names the expected shape, because a caller
|
||||||
|
// who typed the old form needs to know what to type instead.
|
||||||
|
func TestParseFieldsDSL_BareRelationIsRefused(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
for _, in := range []string{"owner:relation", "owner:relation:", "owner:relation: "} {
|
||||||
|
schema, err := ParseFieldsDSL(in)
|
||||||
|
if err == nil {
|
||||||
|
t.Errorf("%q: expected an error, got schema %+v — a relation with no target is a field no write can ever satisfy", in, schema)
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
if !strings.Contains(err.Error(), "relation") || !strings.Contains(err.Error(), "collection") {
|
||||||
|
t.Errorf("%q: error %q should name what is missing and the expected shape", in, err.Error())
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// A comma means the caller wrote an OPTIONS list — the pre-U6 shape. Refusing
|
||||||
|
// names the mistake; taking the first entry would silently pick a target they
|
||||||
|
// did not choose.
|
||||||
|
func TestParseFieldsDSL_RelationRefusesAnOptionsList(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
_, err := ParseFieldsDSL("owner:relation:people,teams")
|
||||||
|
if err == nil {
|
||||||
|
t.Fatal("expected an error for a comma-separated relation target")
|
||||||
|
}
|
||||||
|
if !strings.Contains(err.Error(), "ONE target collection") {
|
||||||
|
t.Errorf("error %q should say a relation takes one target", err.Error())
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// CONTROL: the change must not have altered how every OTHER type reads its
|
||||||
|
// third part. Without this, moving the options branch could break select and
|
||||||
|
// nothing in this file would notice.
|
||||||
|
func TestParseFieldsDSL_NonRelationStillTakesOptions(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
schema, err := ParseFieldsDSL("size:select:small,large")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("ParseFieldsDSL: %v", err)
|
||||||
|
}
|
||||||
|
fd := schema.Fields[0]
|
||||||
|
if len(fd.Options) != 2 || fd.Options[0] != "small" || fd.Options[1] != "large" {
|
||||||
|
t.Errorf("Options = %v, want [small large]", fd.Options)
|
||||||
|
}
|
||||||
|
if fd.Collection != "" {
|
||||||
|
t.Errorf("Collection = %q, want empty on a non-relation", fd.Collection)
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -40,7 +40,7 @@ var padCollectionTool = ToolDef{
|
|||||||
{
|
{
|
||||||
Name: "fields",
|
Name: "fields",
|
||||||
Type: "string",
|
Type: "string",
|
||||||
Description: "Compact field DSL: \"key:type[:options]; ...\". Optional for action=create. Example: \"status:select:open,done; priority:select:high,medium,low\". Use `schema` instead when you need terminal_options, custom defaults, computed fields, suffixes, or relation collections — the DSL cannot express those. Mutually exclusive with `schema`.",
|
Description: "Compact field DSL: \"key:type[:options]; ...\". Optional for action=create. Example: \"status:select:open,done; priority:select:high,medium,low\". For a relation the third part is the TARGET COLLECTION slug rather than an options list, and it is required: \"reviewer:relation:people\" — a relation with no target is refused, since such a field cannot accept any value. Use `schema` instead when you need terminal_options, custom defaults, computed fields, or suffixes — the DSL cannot express those. Mutually exclusive with `schema`.",
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
Name: "schema",
|
Name: "schema",
|
||||||
@@ -104,11 +104,15 @@ Actions:
|
|||||||
Optional: fields OR schema (mutually exclusive), icon, description,
|
Optional: fields OR schema (mutually exclusive), icon, description,
|
||||||
layout, default_view, board_group_by.
|
layout, default_view, board_group_by.
|
||||||
DSL example: fields="status:select:open,done; priority:select:high,medium,low"
|
DSL example: fields="status:select:open,done; priority:select:high,medium,low"
|
||||||
|
Relation in the DSL: the third part is the TARGET COLLECTION slug,
|
||||||
|
not an options list, and it is required —
|
||||||
|
fields="reviewer:relation:people". A relation with no target is
|
||||||
|
refused rather than built, because such a field cannot accept any
|
||||||
|
value.
|
||||||
Schema example: schema={"fields":[{"key":"status","type":"select","options":["new","done"],"terminal_options":["done"]}]}
|
Schema example: schema={"fields":[{"key":"status","type":"select","options":["new","done"],"terminal_options":["done"]}]}
|
||||||
Prefer schema when terminal_options matters (driving dashboard
|
Prefer schema when terminal_options matters (driving dashboard
|
||||||
active/complete counts) or when fields need defaults, computed
|
active/complete counts) or when fields need defaults, computed
|
||||||
flags, suffixes, or relation collections — the DSL cannot express
|
flags, or suffixes — the DSL cannot express those.
|
||||||
those.
|
|
||||||
update — Update an existing collection's name, icon, description, prefix,
|
update — Update an existing collection's name, icon, description, prefix,
|
||||||
schema, or sort order. Workspace owner only.
|
schema, or sort order. Workspace owner only.
|
||||||
Required: workspace, slug.
|
Required: workspace, slug.
|
||||||
|
|||||||
@@ -0,0 +1,157 @@
|
|||||||
|
package mcp
|
||||||
|
|
||||||
|
// PLAN-2857 U6 — a relation value survives the REMOTE transport in both
|
||||||
|
// directions: an exact TITLE written through it resolves to the canonical id,
|
||||||
|
// and the hydrated `relation_targets` reaches the agent through the summary
|
||||||
|
// projection.
|
||||||
|
//
|
||||||
|
// WHY THIS IS NOT CEREMONIAL, given the server doors are already tested. The
|
||||||
|
// remote list path does not forward the handler's response — it re-projects it
|
||||||
|
// through cli.ToItemSummaries, and that projection drops UUID plumbing on
|
||||||
|
// purpose. relation_targets was dropped by it during this very unit, which is
|
||||||
|
// exactly the failure a handler-level test cannot see: the server was correct
|
||||||
|
// and the agent still received a bare id with nothing to render it. Same
|
||||||
|
// rationale as the summary-shape tests above, and the same fixture style —
|
||||||
|
// real server, real store, because a stubbed handler can encode a response the
|
||||||
|
// endpoint never produces.
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"encoding/json"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"github.com/PerpetualSoftware/pad/internal/models"
|
||||||
|
"github.com/PerpetualSoftware/pad/internal/server"
|
||||||
|
"github.com/PerpetualSoftware/pad/internal/store"
|
||||||
|
"github.com/PerpetualSoftware/pad/internal/store/storetest"
|
||||||
|
)
|
||||||
|
|
||||||
|
type relationTargetsFixture struct {
|
||||||
|
d *HTTPHandlerDispatcher
|
||||||
|
store *store.Store
|
||||||
|
ws *models.Workspace
|
||||||
|
cars *models.Collection
|
||||||
|
target *models.Item
|
||||||
|
}
|
||||||
|
|
||||||
|
func newRelationTargetsFixture(t *testing.T) *relationTargetsFixture {
|
||||||
|
t.Helper()
|
||||||
|
s := storetest.NewSQLite(t)
|
||||||
|
srv := server.New(s)
|
||||||
|
t.Cleanup(srv.Stop)
|
||||||
|
|
||||||
|
owner, err := s.CreateUser(models.UserCreate{
|
||||||
|
Email: "rel-owner@example.com", Name: "Owner", Password: "correct-horse-battery-staple",
|
||||||
|
})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("CreateUser: %v", err)
|
||||||
|
}
|
||||||
|
ws, err := s.CreateWorkspace(models.WorkspaceCreate{Name: "Rel WS", Slug: "rel-ws", OwnerID: owner.ID})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("CreateWorkspace: %v", err)
|
||||||
|
}
|
||||||
|
if err := s.AddWorkspaceMember(ws.ID, owner.ID, "owner"); err != nil {
|
||||||
|
t.Fatalf("AddWorkspaceMember: %v", err)
|
||||||
|
}
|
||||||
|
colors, err := s.CreateCollection(ws.ID, models.CollectionCreate{
|
||||||
|
Name: "Colors", Slug: "colors", Prefix: "COLO",
|
||||||
|
Schema: `{"fields":[{"key":"status","type":"select","options":["open","done"],"default":"open"}]}`,
|
||||||
|
})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("CreateCollection(colors): %v", err)
|
||||||
|
}
|
||||||
|
cars, err := s.CreateCollection(ws.ID, models.CollectionCreate{
|
||||||
|
Name: "Cars", Slug: "cars", Prefix: "CAR",
|
||||||
|
Schema: `{"fields":[{"key":"status","type":"select","options":["open","done"],"default":"open"},{"key":"color","type":"relation","collection":"colors"}]}`,
|
||||||
|
})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("CreateCollection(cars): %v", err)
|
||||||
|
}
|
||||||
|
red, err := s.CreateItem(ws.ID, colors.ID, models.ItemCreate{Title: "Red", CreatedBy: owner.ID})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("CreateItem(red): %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
return &relationTargetsFixture{
|
||||||
|
d: &HTTPHandlerDispatcher{
|
||||||
|
Handler: srv,
|
||||||
|
UserResolver: func(context.Context) *models.User { return owner },
|
||||||
|
},
|
||||||
|
store: s, ws: ws, cars: cars, target: red,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestHTTPRelation_TitleWrittenOverTheRemoteDoorStoresTheCanonicalID: the write
|
||||||
|
// half. The title is resolved SERVER-side, so the remote door must carry the
|
||||||
|
// caller's string through untouched and store what the server resolved.
|
||||||
|
func TestHTTPRelation_TitleWrittenOverTheRemoteDoorStoresTheCanonicalID(t *testing.T) {
|
||||||
|
f := newRelationTargetsFixture(t)
|
||||||
|
|
||||||
|
ctx := WithDispatchInput(context.Background(), map[string]any{
|
||||||
|
"workspace": "rel-ws",
|
||||||
|
"collection": "cars",
|
||||||
|
"title": "By title",
|
||||||
|
"field": []any{"color=Red"},
|
||||||
|
})
|
||||||
|
res, err := f.d.Dispatch(ctx, []string{"item", "create"}, nil)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("Dispatch(item create): %v", err)
|
||||||
|
}
|
||||||
|
if res.IsError {
|
||||||
|
t.Fatalf("create refused an exact title: %s", textOf(res))
|
||||||
|
}
|
||||||
|
|
||||||
|
// Read the STORED value back, not the response: a door that echoed the
|
||||||
|
// right thing while writing the wrong thing would pass a response check.
|
||||||
|
items, err := f.store.ListItems(f.ws.ID, models.ItemListParams{CollectionSlug: "cars"})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("ListItems: %v", err)
|
||||||
|
}
|
||||||
|
if len(items) != 1 {
|
||||||
|
t.Fatalf("expected 1 car, got %d", len(items))
|
||||||
|
}
|
||||||
|
var fields map[string]any
|
||||||
|
if err := json.Unmarshal([]byte(items[0].Fields), &fields); err != nil {
|
||||||
|
t.Fatalf("decode fields: %v", err)
|
||||||
|
}
|
||||||
|
if fields["color"] != f.target.ID {
|
||||||
|
t.Errorf("stored color = %v, want the canonical id %s — the title did not resolve through this transport", fields["color"], f.target.ID)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestHTTPRelation_ListCarriesHydratedTargets: the read half, and the one that
|
||||||
|
// caught a real strip. The summary projection must keep relation_targets.
|
||||||
|
func TestHTTPRelation_ListCarriesHydratedTargets(t *testing.T) {
|
||||||
|
f := newRelationTargetsFixture(t)
|
||||||
|
|
||||||
|
if _, err := f.store.CreateItem(f.ws.ID, f.cars.ID, models.ItemCreate{
|
||||||
|
Title: "Ruby car",
|
||||||
|
Fields: `{"status":"open","color":"` + f.target.ID + `"}`,
|
||||||
|
}); err != nil {
|
||||||
|
t.Fatalf("CreateItem(car): %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
text := dispatchList(t, f.d, map[string]any{"workspace": "rel-ws", "collection": "cars"})
|
||||||
|
|
||||||
|
var items []struct {
|
||||||
|
Ref string `json:"ref"`
|
||||||
|
RelationTargets map[string]struct {
|
||||||
|
ID string `json:"id"`
|
||||||
|
Ref string `json:"ref"`
|
||||||
|
Title string `json:"title"`
|
||||||
|
} `json:"relation_targets"`
|
||||||
|
}
|
||||||
|
if err := json.Unmarshal([]byte(text), &items); err != nil {
|
||||||
|
t.Fatalf("decode summaries: %v\n%s", err, text)
|
||||||
|
}
|
||||||
|
if len(items) != 1 {
|
||||||
|
t.Fatalf("expected 1 item, got %d\n%s", len(items), text)
|
||||||
|
}
|
||||||
|
got, ok := items[0].RelationTargets["color"]
|
||||||
|
if !ok {
|
||||||
|
t.Fatalf("the summary projection dropped relation_targets, so an agent gets a bare id with nothing to render it — the exact defect U6 exists to close:\n%s", text)
|
||||||
|
}
|
||||||
|
if got.ID != f.target.ID || got.Ref != f.target.Ref || got.Title != "Red" {
|
||||||
|
t.Errorf("hydrated as %+v, want {id:%s ref:%s title:Red}", got, f.target.ID, f.target.Ref)
|
||||||
|
}
|
||||||
|
}
|
||||||
File diff suppressed because one or more lines are too long
+42
-1
@@ -937,6 +937,47 @@ const CmdhelpVersion = "0.1"
|
|||||||
// case, which is the one with a real claim to leniency, is
|
// case, which is the one with a real claim to leniency, is
|
||||||
// already exempt by provenance rather than by a flag.
|
// already exempt by provenance rather than by a flag.
|
||||||
//
|
//
|
||||||
|
// 0.31 — PLAN-2857 U6 / TASK-2996. A `relation` field value may now
|
||||||
|
// be an EXACT TITLE, scoped to the collection that field declares,
|
||||||
|
// alongside the UUID and ref U1 accepted; reads carry a new
|
||||||
|
// `relation_targets` member hydrating each stored id to
|
||||||
|
// {id, ref, title}; and `pad_collection.create`'s `fields` DSL takes
|
||||||
|
// the target collection slug as a relation's third part.
|
||||||
|
//
|
||||||
|
// A BEHAVIOR bump on the v0.30/v0.29 grounds — no tool name, action
|
||||||
|
// enum or param shape changed. Two of the three parts are purely
|
||||||
|
// additive: a title is a THIRD accepted spelling, so nothing a 0.30
|
||||||
|
// consumer sends stops working, and `relation_targets` is additive
|
||||||
|
// and omitempty. The bump is owed by the DSL: `fields="owner:relation"`
|
||||||
|
// used to be ACCEPTED and is now REFUSED.
|
||||||
|
//
|
||||||
|
// That refusal is the fix, not a regression. The old parser put the
|
||||||
|
// third part into Options for every type, so a relation declared
|
||||||
|
// through the DSL got Options=["colors"] and NO target collection —
|
||||||
|
// and every subsequent write to that field was refused with
|
||||||
|
// `target_missing`. The DSL was building a field that could never
|
||||||
|
// accept a value, and a caller relying on that was relying on a
|
||||||
|
// field that did not work. Same disposition as v0.24's strict-input
|
||||||
|
// half: the reliance was indistinguishable from a caller bug, so the
|
||||||
|
// break is the fix.
|
||||||
|
//
|
||||||
|
// Title resolution is COLLECTION-SCOPED, and that is the whole
|
||||||
|
// point rather than an implementation detail. A title unique only
|
||||||
|
// workspace-wide REFUSES, naming the collection searched; resolving
|
||||||
|
// it workspace-wide would let free text land on whatever happens to
|
||||||
|
// carry that title today. Two or more matches inside the declared
|
||||||
|
// collection get a new `ambiguous` reason rather than `not_found`,
|
||||||
|
// which would state the opposite of what happened. The ladder is
|
||||||
|
// UUID, then ref, then title, so an item literally TITLED "COLO-3"
|
||||||
|
// is unreachable by title while the ref COLO-3 resolves — deliberate,
|
||||||
|
// since refs are canonical and the ambiguity is the title's doing.
|
||||||
|
//
|
||||||
|
// An id-only `relation_targets` entry means the target is gone OR
|
||||||
|
// the caller may not see it, and consumers must not render it as
|
||||||
|
// either: the two are made indistinguishable on purpose, and the
|
||||||
|
// write side collapses `wrong_collection` to `not_found` for the
|
||||||
|
// same reason.
|
||||||
|
//
|
||||||
// - POST-0.30, NO BUMP — BUG-2995. A successful content write through
|
// - POST-0.30, NO BUMP — BUG-2995. A successful content write through
|
||||||
// the designated applier used to answer with the item's PREVIOUS
|
// the designated applier used to answer with the item's PREVIOUS
|
||||||
// content: on that path the markdown goes to a live browser tab's
|
// content: on that path the markdown goes to a live browser tab's
|
||||||
@@ -975,7 +1016,7 @@ const CmdhelpVersion = "0.1"
|
|||||||
// is ever updated is not established — the flush belongs to a
|
// is ever updated is not established — the flush belongs to a
|
||||||
// browser tab and BUG-3000 carries the open half — so no surface
|
// browser tab and BUG-3000 carries the open half — so no surface
|
||||||
// here states a duration.
|
// here states a duration.
|
||||||
const ToolSurfaceVersion = "0.30"
|
const ToolSurfaceVersion = "0.31"
|
||||||
|
|
||||||
// MetaVersionURI is the canonical URI of the queryable version document.
|
// MetaVersionURI is the canonical URI of the queryable version document.
|
||||||
// Lives outside the pad://workspace/{ws}/... namespace because it's a
|
// Lives outside the pad://workspace/{ws}/... namespace because it's a
|
||||||
|
|||||||
@@ -109,6 +109,20 @@ type ItemWriteWarnings struct {
|
|||||||
ContentOutcome string `json:"content_outcome,omitempty"`
|
ContentOutcome string `json:"content_outcome,omitempty"`
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// RelationTarget is one hydrated `relation` value: the item an ID points at,
|
||||||
|
// in the smallest shape a caller needs to RENDER it without a second request
|
||||||
|
// (PLAN-2857 U6).
|
||||||
|
//
|
||||||
|
// `ID` is always set — it is the stored value, and it is all there is when the
|
||||||
|
// target is gone or invisible. `Ref` and `Title` are present only when the
|
||||||
|
// target resolved and the requester may see it, so a consumer must treat them
|
||||||
|
// as optional rather than assuming a hydrated entry is complete.
|
||||||
|
type RelationTarget struct {
|
||||||
|
ID string `json:"id"`
|
||||||
|
Ref string `json:"ref,omitempty"`
|
||||||
|
Title string `json:"title,omitempty"`
|
||||||
|
}
|
||||||
|
|
||||||
// ContentOutcomeAppliedPendingFlush is the only ContentOutcome value that rides on
|
// ContentOutcomeAppliedPendingFlush is the only ContentOutcome value that rides on
|
||||||
// a 2xx: the content reached the collaborative document and items.content has not
|
// a 2xx: the content reached the collaborative document and items.content has not
|
||||||
// caught up. It lives here rather than beside the error-path outcome constants
|
// caught up. It lives here rather than beside the error-path outcome constants
|
||||||
@@ -222,6 +236,25 @@ type Item struct {
|
|||||||
ParentSlug string `json:"parent_slug,omitempty"`
|
ParentSlug string `json:"parent_slug,omitempty"`
|
||||||
ParentCollectionSlug string `json:"parent_collection_slug,omitempty"`
|
ParentCollectionSlug string `json:"parent_collection_slug,omitempty"`
|
||||||
|
|
||||||
|
// RelationTargets hydrates `relation` field values for READ responses
|
||||||
|
// (PLAN-2857 U6): field key -> the item that value names.
|
||||||
|
//
|
||||||
|
// It sits BESIDE `fields` rather than inside it, by lead ruling. `fields` is
|
||||||
|
// a write shape as well as a read shape — a caller reads an item, changes
|
||||||
|
// one key and writes the object back — so replacing a relation's ID string
|
||||||
|
// with an object there would break that round trip for every existing
|
||||||
|
// client. Here it is additive and omitempty: a consumer that does not know
|
||||||
|
// the key ignores it.
|
||||||
|
//
|
||||||
|
// A value that cannot be hydrated appears as an ID-ONLY entry rather than
|
||||||
|
// being omitted. Omitting it would read as "this item has no relation",
|
||||||
|
// which is a different and false statement; the honest one is "it points at
|
||||||
|
// this id and I cannot tell you more". Two cases produce that: a DANGLING
|
||||||
|
// value whose target is gone, and a target the requester may not see —
|
||||||
|
// hydrating a ref or title for the latter would leak exactly what the
|
||||||
|
// write-side visibility collapse exists to withhold.
|
||||||
|
RelationTargets map[string]RelationTarget `json:"relation_targets,omitempty"`
|
||||||
|
|
||||||
// Warnings is populated on WRITE responses only (create / update), never
|
// Warnings is populated on WRITE responses only (create / update), never
|
||||||
// on reads, and is never stored. It carries things the write did that the
|
// on reads, and is never stored. It carries things the write did that the
|
||||||
// caller might not have meant, or could not otherwise learn: field keys the
|
// caller might not have meant, or could not otherwise learn: field keys the
|
||||||
|
|||||||
@@ -78,7 +78,7 @@ func (s *Server) handleListItems(w http.ResponseWriter, r *http.Request) {
|
|||||||
if result == nil {
|
if result == nil {
|
||||||
result = []models.Item{}
|
result = []models.Item{}
|
||||||
}
|
}
|
||||||
s.enrichItemsWithParent(workspaceID, result, visibleIDs)
|
s.enrichItemsWithParent(r, workspaceID, result, visibleIDs)
|
||||||
|
|
||||||
writeJSON(w, http.StatusOK, result)
|
writeJSON(w, http.StatusOK, result)
|
||||||
}
|
}
|
||||||
@@ -245,7 +245,7 @@ func (s *Server) handleListItemsIndex(w http.ResponseWriter, r *http.Request) {
|
|||||||
if result == nil {
|
if result == nil {
|
||||||
result = []models.Item{}
|
result = []models.Item{}
|
||||||
}
|
}
|
||||||
s.enrichItemsWithParent(workspaceID, result, visibleIDs)
|
s.enrichItemsWithParent(r, workspaceID, result, visibleIDs)
|
||||||
|
|
||||||
// Cursor: max(pre-list workspace floor, MAX(returned-rows.seq)).
|
// Cursor: max(pre-list workspace floor, MAX(returned-rows.seq)).
|
||||||
// See the long comment above MaxItemSeq for why the floor is read
|
// See the long comment above MaxItemSeq for why the floor is read
|
||||||
@@ -389,7 +389,7 @@ func (s *Server) handleListItemsChanges(w http.ResponseWriter, r *http.Request)
|
|||||||
// /items-index payload shape. Soft-deleted parents are skipped by
|
// /items-index payload shape. Soft-deleted parents are skipped by
|
||||||
// the underlying GetItem (deleted_at IS NULL), so we don't leak
|
// the underlying GetItem (deleted_at IS NULL), so we don't leak
|
||||||
// parent title / ref after the parent itself has been archived.
|
// parent title / ref after the parent itself has been archived.
|
||||||
s.enrichItemsWithParent(workspaceID, rows, visibleIDs)
|
s.enrichItemsWithParent(r, workspaceID, rows, visibleIDs)
|
||||||
|
|
||||||
changes := make([]itemChangeRow, 0, len(rows))
|
changes := make([]itemChangeRow, 0, len(rows))
|
||||||
for _, it := range rows {
|
for _, it := range rows {
|
||||||
@@ -586,7 +586,7 @@ func (s *Server) handleListCollectionItems(w http.ResponseWriter, r *http.Reques
|
|||||||
if result == nil {
|
if result == nil {
|
||||||
result = []models.Item{}
|
result = []models.Item{}
|
||||||
}
|
}
|
||||||
s.enrichItemsWithParent(workspaceID, result, visibleIDs)
|
s.enrichItemsWithParent(r, workspaceID, result, visibleIDs)
|
||||||
|
|
||||||
writeJSON(w, http.StatusOK, result)
|
writeJSON(w, http.StatusOK, result)
|
||||||
}
|
}
|
||||||
@@ -735,7 +735,7 @@ func (s *Server) handleCreateItem(w http.ResponseWriter, r *http.Request) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
createVisIDs, _ := s.visibleCollectionIDs(r, workspaceID)
|
createVisIDs, _ := s.visibleCollectionIDs(r, workspaceID)
|
||||||
if err := s.enrichItemForResponse(item, createVisIDs); err != nil {
|
if err := s.enrichItemForResponse(r, item, createVisIDs); err != nil {
|
||||||
writeInternalError(w, err)
|
writeInternalError(w, err)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
@@ -1003,7 +1003,7 @@ func (s *Server) handleGetItem(w http.ResponseWriter, r *http.Request) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
enrichVisIDs, _ := s.visibleCollectionIDs(r, workspaceID)
|
enrichVisIDs, _ := s.visibleCollectionIDs(r, workspaceID)
|
||||||
if err := s.enrichItemForResponse(item, enrichVisIDs); err != nil {
|
if err := s.enrichItemForResponse(r, item, enrichVisIDs); err != nil {
|
||||||
writeInternalError(w, err)
|
writeInternalError(w, err)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
@@ -2147,7 +2147,7 @@ func (s *Server) handleUpdateItem(w http.ResponseWriter, r *http.Request) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
updateVisIDs, _ := s.visibleCollectionIDs(r, workspaceID)
|
updateVisIDs, _ := s.visibleCollectionIDs(r, workspaceID)
|
||||||
if err := s.enrichItemForResponse(updated, updateVisIDs); err != nil {
|
if err := s.enrichItemForResponse(r, updated, updateVisIDs); err != nil {
|
||||||
writeInternalError(w, err)
|
writeInternalError(w, err)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
@@ -2283,7 +2283,7 @@ func (s *Server) handleRestoreItem(w http.ResponseWriter, r *http.Request) {
|
|||||||
s.publishItemEventWithName(sseItemRestored, workspaceID, restored.ID, restored.Title, restored.CollectionSlug, actor, actorNameFromRequest(r), source, restored.Seq)
|
s.publishItemEventWithName(sseItemRestored, workspaceID, restored.ID, restored.Title, restored.CollectionSlug, actor, actorNameFromRequest(r), source, restored.Seq)
|
||||||
|
|
||||||
restoreVisIDs, _ := s.visibleCollectionIDs(r, workspaceID)
|
restoreVisIDs, _ := s.visibleCollectionIDs(r, workspaceID)
|
||||||
if err := s.enrichItemForResponse(restored, restoreVisIDs); err != nil {
|
if err := s.enrichItemForResponse(r, restored, restoreVisIDs); err != nil {
|
||||||
writeInternalError(w, err)
|
writeInternalError(w, err)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
@@ -2457,6 +2457,7 @@ func (s *Server) handleMoveItem(w http.ResponseWriter, r *http.Request) {
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
relRefusals, relDropped, relErr := s.store.MigrateRelationReferents(
|
relRefusals, relDropped, relErr := s.store.MigrateRelationReferents(
|
||||||
|
s.relationVisibility(r, workspaceRole(r)),
|
||||||
workspaceID, items.SchemaForMigratedFields(targetSchema), result.Fields,
|
workspaceID, items.SchemaForMigratedFields(targetSchema), result.Fields,
|
||||||
input.FieldOverrides, store.CarriedSourceValues(currentFields, result.Dropped),
|
input.FieldOverrides, store.CarriedSourceValues(currentFields, result.Dropped),
|
||||||
store.RelationCarryWithinWorkspace)
|
store.RelationCarryWithinWorkspace)
|
||||||
@@ -2499,6 +2500,7 @@ func (s *Server) handleMoveItem(w http.ResponseWriter, r *http.Request) {
|
|||||||
// the required-field check has to see a value referent resolution dropped,
|
// the required-field check has to see a value referent resolution dropped,
|
||||||
// which is why the main pass runs first — see ResolveLateRelationDefaults.
|
// which is why the main pass runs first — see ResolveLateRelationDefaults.
|
||||||
lateDropped, lateErr := s.store.ResolveLateRelationDefaults(
|
lateDropped, lateErr := s.store.ResolveLateRelationDefaults(
|
||||||
|
s.relationVisibility(r, workspaceRole(r)),
|
||||||
workspaceID, items.SchemaForMigratedFields(targetSchema), result.Fields, relBefore)
|
workspaceID, items.SchemaForMigratedFields(targetSchema), result.Fields, relBefore)
|
||||||
if lateErr != nil {
|
if lateErr != nil {
|
||||||
writeInternalError(w, lateErr)
|
writeInternalError(w, lateErr)
|
||||||
@@ -2651,7 +2653,7 @@ func (s *Server) handleMoveItem(w http.ResponseWriter, r *http.Request) {
|
|||||||
s.publishWatchNotifications(workspaceID, moved, actor, actorNameForMove)
|
s.publishWatchNotifications(workspaceID, moved, actor, actorNameForMove)
|
||||||
|
|
||||||
moveVisIDs, _ := s.visibleCollectionIDs(r, workspaceID)
|
moveVisIDs, _ := s.visibleCollectionIDs(r, workspaceID)
|
||||||
if err := s.enrichItemForResponse(moved, moveVisIDs); err != nil {
|
if err := s.enrichItemForResponse(r, moved, moveVisIDs); err != nil {
|
||||||
writeInternalError(w, err)
|
writeInternalError(w, err)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
@@ -3039,7 +3041,7 @@ func (s *Server) handleGetItemChildren(w http.ResponseWriter, r *http.Request) {
|
|||||||
children = filtered
|
children = filtered
|
||||||
}
|
}
|
||||||
|
|
||||||
s.enrichItemsWithParent(workspaceID, children, visibleIDs)
|
s.enrichItemsWithParent(r, workspaceID, children, visibleIDs)
|
||||||
if visibleIDs != nil {
|
if visibleIDs != nil {
|
||||||
// Visibility-aware has_children: only count visible grandchildren
|
// Visibility-aware has_children: only count visible grandchildren
|
||||||
for i := range children {
|
for i := range children {
|
||||||
|
|||||||
@@ -563,7 +563,7 @@ func (s *Server) bulkFieldUpdate(r *http.Request, workspaceID string, item *mode
|
|||||||
for k, v := range suppliedRelations {
|
for k, v := range suppliedRelations {
|
||||||
fieldMap[k] = v
|
fieldMap[k] = v
|
||||||
}
|
}
|
||||||
lateDropped, lateErr := s.store.ResolveLateRelationDefaults(workspaceID, schema, fieldMap, relBefore)
|
lateDropped, lateErr := s.store.ResolveLateRelationDefaults(s.relationVisibility(r, workspaceRole(r)), workspaceID, schema, fieldMap, relBefore)
|
||||||
if lateErr != nil {
|
if lateErr != nil {
|
||||||
return nil, &bulkOpError{message: "Failed to resolve relation references", code: "internal_error"}
|
return nil, &bulkOpError{message: "Failed to resolve relation references", code: "internal_error"}
|
||||||
}
|
}
|
||||||
@@ -822,6 +822,7 @@ func (s *Server) bulkMoveCollection(r *http.Request, workspaceID string, item *m
|
|||||||
return nil, &bulkOpError{message: relationIssuesMessage(invisible), code: "validation_error"}
|
return nil, &bulkOpError{message: relationIssuesMessage(invisible), code: "validation_error"}
|
||||||
}
|
}
|
||||||
relRefusals, relDropped, relErr := s.store.MigrateRelationReferents(
|
relRefusals, relDropped, relErr := s.store.MigrateRelationReferents(
|
||||||
|
s.relationVisibility(r, workspaceRole(r)),
|
||||||
workspaceID, items.SchemaForMigratedFields(targetSchema), result.Fields,
|
workspaceID, items.SchemaForMigratedFields(targetSchema), result.Fields,
|
||||||
suppliedByCaller, store.CarriedSourceValues(currentFields, result.Dropped),
|
suppliedByCaller, store.CarriedSourceValues(currentFields, result.Dropped),
|
||||||
store.RelationCarryWithinWorkspace)
|
store.RelationCarryWithinWorkspace)
|
||||||
@@ -851,6 +852,7 @@ func (s *Server) bulkMoveCollection(r *http.Request, workspaceID string, item *m
|
|||||||
// Relation defaults ValidateFields just injected (codex round 2). After
|
// Relation defaults ValidateFields just injected (codex round 2). After
|
||||||
// validation for the reason ResolveLateRelationDefaults documents.
|
// validation for the reason ResolveLateRelationDefaults documents.
|
||||||
lateDropped, lateErr := s.store.ResolveLateRelationDefaults(
|
lateDropped, lateErr := s.store.ResolveLateRelationDefaults(
|
||||||
|
s.relationVisibility(r, workspaceRole(r)),
|
||||||
workspaceID, items.SchemaForMigratedFields(targetSchema), result.Fields, relBefore)
|
workspaceID, items.SchemaForMigratedFields(targetSchema), result.Fields, relBefore)
|
||||||
if lateErr != nil {
|
if lateErr != nil {
|
||||||
return nil, &bulkOpError{message: "Failed to resolve relation references", code: "internal_error"}
|
return nil, &bulkOpError{message: "Failed to resolve relation references", code: "internal_error"}
|
||||||
|
|||||||
@@ -811,6 +811,7 @@ func (s *Server) handleCopyItemPreflight(w http.ResponseWriter, r *http.Request)
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
relRefusals, relDropped, relErr := s.store.MigrateRelationReferents(
|
relRefusals, relDropped, relErr := s.store.MigrateRelationReferents(
|
||||||
|
s.relationVisibility(r, dst.Role),
|
||||||
dst.WorkspaceID(), items.SchemaForMigratedFields(targetSchema), final,
|
dst.WorkspaceID(), items.SchemaForMigratedFields(targetSchema), final,
|
||||||
input.FieldOverrides, carriedSource, relMode)
|
input.FieldOverrides, carriedSource, relMode)
|
||||||
if relErr != nil {
|
if relErr != nil {
|
||||||
@@ -866,6 +867,7 @@ func (s *Server) handleCopyItemPreflight(w http.ResponseWriter, r *http.Request)
|
|||||||
// reported an unresolved default as carrying while the copy dropped it
|
// reported an unresolved default as carrying while the copy dropped it
|
||||||
// would be the DR-6 divergence in a new place.
|
// would be the DR-6 divergence in a new place.
|
||||||
lateDropped, lateErr := s.store.ResolveLateRelationDefaults(
|
lateDropped, lateErr := s.store.ResolveLateRelationDefaults(
|
||||||
|
s.relationVisibility(r, dst.Role),
|
||||||
dst.WorkspaceID(), items.SchemaForMigratedFields(targetSchema), final, relBefore)
|
dst.WorkspaceID(), items.SchemaForMigratedFields(targetSchema), final, relBefore)
|
||||||
if lateErr != nil {
|
if lateErr != nil {
|
||||||
writeInternalError(w, fmt.Errorf("copy preflight: resolve relation defaults: %w", lateErr))
|
writeInternalError(w, fmt.Errorf("copy preflight: resolve relation defaults: %w", lateErr))
|
||||||
|
|||||||
@@ -441,7 +441,7 @@ func (s *Server) handleGetProjectChangelog(w http.ResponseWriter, r *http.Reques
|
|||||||
// (codex R1 P1, TASK-1916: using collIDs here dropped parent
|
// (codex R1 P1, TASK-1916: using collIDs here dropped parent
|
||||||
// fields for items whose parent lives in an item-grant-only
|
// fields for items whose parent lives in an item-grant-only
|
||||||
// collection, silently breaking ?parent= for that guest).
|
// collection, silently breaking ?parent= for that guest).
|
||||||
s.enrichItemsWithParent(workspaceID, items, navVisibleIDs)
|
s.enrichItemsWithParent(r, workspaceID, items, navVisibleIDs)
|
||||||
filtered := items[:0]
|
filtered := items[:0]
|
||||||
for _, item := range items {
|
for _, item := range items {
|
||||||
if itemMatchesParentFilter(item, parent) {
|
if itemMatchesParentFilter(item, parent) {
|
||||||
|
|||||||
@@ -204,7 +204,7 @@ func (s *Server) handleListStarredItems(w http.ResponseWriter, r *http.Request)
|
|||||||
items = filtered
|
items = filtered
|
||||||
|
|
||||||
// Hydrate items with parent links and computed refs
|
// Hydrate items with parent links and computed refs
|
||||||
s.enrichItemsWithParent(workspaceID, items, visibleIDs)
|
s.enrichItemsWithParent(r, workspaceID, items, visibleIDs)
|
||||||
|
|
||||||
if len(items) == 0 {
|
if len(items) == 0 {
|
||||||
items = []models.Item{}
|
items = []models.Item{}
|
||||||
|
|||||||
@@ -2,6 +2,7 @@ package server
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"fmt"
|
"fmt"
|
||||||
|
"net/http"
|
||||||
"strings"
|
"strings"
|
||||||
|
|
||||||
"github.com/PerpetualSoftware/pad/internal/models"
|
"github.com/PerpetualSoftware/pad/internal/models"
|
||||||
@@ -10,7 +11,7 @@ import (
|
|||||||
// enrichItemsWithParent batch-populates parent link info on a slice of items.
|
// enrichItemsWithParent batch-populates parent link info on a slice of items.
|
||||||
// Used by list endpoints where calling enrichItemForResponse per-item is too expensive.
|
// Used by list endpoints where calling enrichItemForResponse per-item is too expensive.
|
||||||
// visibleIDs controls which parent collections are allowed; nil means all visible.
|
// visibleIDs controls which parent collections are allowed; nil means all visible.
|
||||||
func (s *Server) enrichItemsWithParent(workspaceID string, items []models.Item, visibleIDs ...[]string) {
|
func (s *Server) enrichItemsWithParent(r *http.Request, workspaceID string, items []models.Item, visibleIDs ...[]string) {
|
||||||
if len(items) == 0 {
|
if len(items) == 0 {
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
@@ -22,6 +23,11 @@ func (s *Server) enrichItemsWithParent(workspaceID string, items []models.Item,
|
|||||||
hasVis = true
|
hasVis = true
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// U6 hydration runs BEFORE the parent-map early return below. The two are
|
||||||
|
// unrelated decorations, and a workspace with no parent links at all would
|
||||||
|
// otherwise return here and silently ship every list read unhydrated.
|
||||||
|
s.hydrateRelationTargets(r, workspaceID, items)
|
||||||
|
|
||||||
parentMap, err := s.store.GetParentMap(workspaceID)
|
parentMap, err := s.store.GetParentMap(workspaceID)
|
||||||
if err != nil || len(parentMap) == 0 {
|
if err != nil || len(parentMap) == 0 {
|
||||||
return
|
return
|
||||||
@@ -77,7 +83,7 @@ func (s *Server) enrichItemsWithParent(workspaceID string, items []models.Item,
|
|||||||
// enrichItemForResponse populates derived closure and parent info on a single item.
|
// enrichItemForResponse populates derived closure and parent info on a single item.
|
||||||
// An optional visibleIDs slice filters related items so hidden-collection metadata
|
// An optional visibleIDs slice filters related items so hidden-collection metadata
|
||||||
// is not leaked. Pass nil (or omit) for full access.
|
// is not leaked. Pass nil (or omit) for full access.
|
||||||
func (s *Server) enrichItemForResponse(item *models.Item, visibleIDs ...[]string) error {
|
func (s *Server) enrichItemForResponse(r *http.Request, item *models.Item, visibleIDs ...[]string) error {
|
||||||
if item == nil {
|
if item == nil {
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
@@ -94,6 +100,13 @@ func (s *Server) enrichItemForResponse(item *models.Item, visibleIDs ...[]string
|
|||||||
}
|
}
|
||||||
item.DerivedClosure = closure
|
item.DerivedClosure = closure
|
||||||
|
|
||||||
|
// U6: one item is the degenerate batch. Going through the same helper means
|
||||||
|
// a single read and a list read cannot disagree about what `relation_targets`
|
||||||
|
// says for the same item — including which targets collapse to id-only.
|
||||||
|
one := []models.Item{*item}
|
||||||
|
s.hydrateRelationTargets(r, item.WorkspaceID, one)
|
||||||
|
item.RelationTargets = one[0].RelationTargets
|
||||||
|
|
||||||
// Populate parent link info — skip if parent is in a hidden collection
|
// Populate parent link info — skip if parent is in a hidden collection
|
||||||
parentLink, err := s.store.GetParentForItem(item.ID)
|
parentLink, err := s.store.GetParentForItem(item.ID)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
|||||||
@@ -797,7 +797,7 @@ func TestMovedTo_MutationResponsesDoNotCarryPointer(t *testing.T) {
|
|||||||
if fresh.MovedTo != nil {
|
if fresh.MovedTo != nil {
|
||||||
t.Fatal("store-level item carries MovedTo; it must only ever be set by handleGetItem")
|
t.Fatal("store-level item carries MovedTo; it must only ever be set by handleGetItem")
|
||||||
}
|
}
|
||||||
if err := f.srv.enrichItemForResponse(fresh, nil); err != nil {
|
if err := f.srv.enrichItemForResponse(httptest.NewRequest("GET", "/", nil), fresh, nil); err != nil {
|
||||||
t.Fatalf("enrichItemForResponse: %v", err)
|
t.Fatalf("enrichItemForResponse: %v", err)
|
||||||
}
|
}
|
||||||
if fresh.MovedTo != nil {
|
if fresh.MovedTo != nil {
|
||||||
|
|||||||
@@ -0,0 +1,136 @@
|
|||||||
|
package server
|
||||||
|
|
||||||
|
import (
|
||||||
|
"encoding/json"
|
||||||
|
"net/http"
|
||||||
|
|
||||||
|
"github.com/PerpetualSoftware/pad/internal/models"
|
||||||
|
)
|
||||||
|
|
||||||
|
// hydrateRelationTargets fills `relation_targets` on a batch of items
|
||||||
|
// (PLAN-2857 U6): for each `relation` field the collection declares, the item
|
||||||
|
// the stored id names, as {id, ref, title}.
|
||||||
|
//
|
||||||
|
// It is the READ-side counterpart to the write-side resolver, and it inherits
|
||||||
|
// that resolver's division of labour: the store answers what the items say, and
|
||||||
|
// this layer — the one that knows who is asking — decides what may be said back.
|
||||||
|
//
|
||||||
|
// TWO THINGS COLLAPSE A TARGET TO ID-ONLY, and neither omits the key:
|
||||||
|
//
|
||||||
|
// - the value resolves to nothing live in this workspace (dangling, or an id
|
||||||
|
// naming another workspace, which the store declines to resolve);
|
||||||
|
// - the requester cannot see the target ITEM.
|
||||||
|
//
|
||||||
|
// THAT SECOND CHECK IS PER-ITEM, NOT PER-COLLECTION, and the difference is a
|
||||||
|
// leak rather than a nicety. `store.VisibleCollectionIDs` is deliberately
|
||||||
|
// NAV-LENIENT: it folds in a collection reachable only through an ITEM-level
|
||||||
|
// grant "so the collection appears in navigation", and leaves item-level
|
||||||
|
// filtering to the handlers — requireCollectionFullyVisible's comment says so,
|
||||||
|
// and exists because BUG-1920 walked into it. Filtering hydration on that set
|
||||||
|
// would hand a member with a grant on ONE item in a collection the ref and
|
||||||
|
// title of EVERY item any relation points at there. `checkItemVisible` is the
|
||||||
|
// authority, and it is asked once per DISTINCT TARGET, not once per item.
|
||||||
|
//
|
||||||
|
// Omitting the key instead would say "this item has no relation here", which is
|
||||||
|
// a different statement and a false one. An id-only entry says the true thing:
|
||||||
|
// the value is stored, and there is nothing more this requester may be told.
|
||||||
|
// The write side already collapses `wrong_collection` to `not_found` for an
|
||||||
|
// invisible target; hydrating that item's ref and title on the read would hand
|
||||||
|
// back exactly what that collapse withholds.
|
||||||
|
//
|
||||||
|
// BEST-EFFORT. A failure here leaves items unhydrated rather than failing the
|
||||||
|
// read — the same posture enrichItemsWithParent takes for parent info, and for
|
||||||
|
// the same reason: `relation_targets` is additive convenience, and a list read
|
||||||
|
// that 500s because a convenience field could not be built is a worse answer
|
||||||
|
// than one without it.
|
||||||
|
func (s *Server) hydrateRelationTargets(r *http.Request, workspaceID string, items []models.Item) {
|
||||||
|
if len(items) == 0 || r == nil {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
schemas, err := s.relationSchemasForWorkspace(workspaceID)
|
||||||
|
if err != nil || len(schemas) == 0 {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
targets, err := s.store.HydrateRelationTargetsQ(s.store.DB(), workspaceID, items, schemas)
|
||||||
|
if err != nil || len(targets) == 0 {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
// workspaceRole(r) is the right role here, unlike in the cross-workspace
|
||||||
|
// copy: every item being enriched belongs to the workspace in the URL,
|
||||||
|
// which is the workspace that role was stashed for.
|
||||||
|
user, role, bearer := currentUser(r), workspaceRole(r), isBearerAuth(r)
|
||||||
|
|
||||||
|
// One decision per DISTINCT target, reused across every item pointing at
|
||||||
|
// it. A list page of 50 items sharing one target asks once.
|
||||||
|
allowed := map[string]bool{}
|
||||||
|
visible := func(id string) bool {
|
||||||
|
if seen, done := allowed[id]; done {
|
||||||
|
return seen
|
||||||
|
}
|
||||||
|
item, err := s.store.GetItem(id)
|
||||||
|
if err != nil || item == nil {
|
||||||
|
allowed[id] = false
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
seen, err := s.checkItemVisible(workspaceID, item, user, role, bearer)
|
||||||
|
if err != nil {
|
||||||
|
// Fail CLOSED. An error here is not a licence to disclose.
|
||||||
|
seen = false
|
||||||
|
}
|
||||||
|
allowed[id] = seen
|
||||||
|
return seen
|
||||||
|
}
|
||||||
|
|
||||||
|
for i := range items {
|
||||||
|
perField, ok := targets[items[i].ID]
|
||||||
|
if !ok {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
out := make(map[string]models.RelationTarget, len(perField))
|
||||||
|
for key, target := range perField {
|
||||||
|
if target.Ref != "" && !visible(target.ID) {
|
||||||
|
// Resolved, but not for these eyes. Keep the id — the value IS
|
||||||
|
// stored — and drop what would disclose the target.
|
||||||
|
target = models.RelationTarget{ID: target.ID}
|
||||||
|
}
|
||||||
|
out[key] = target
|
||||||
|
}
|
||||||
|
items[i].RelationTargets = out
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// relationSchemasForWorkspace maps collection ID -> schema for every collection
|
||||||
|
// in the workspace that DECLARES a relation field.
|
||||||
|
//
|
||||||
|
// Narrowed to relation-declaring collections on purpose: the map is handed to
|
||||||
|
// the store's hydrator, which skips a collection it cannot find, so including
|
||||||
|
// the rest would only make a larger map for it to walk past. A collection with
|
||||||
|
// no relation field contributes no work either way.
|
||||||
|
func (s *Server) relationSchemasForWorkspace(workspaceID string) (map[string]models.CollectionSchema, error) {
|
||||||
|
colls, err := s.store.ListCollections(workspaceID)
|
||||||
|
if err != nil {
|
||||||
|
return nil, err
|
||||||
|
}
|
||||||
|
out := map[string]models.CollectionSchema{}
|
||||||
|
for i := range colls {
|
||||||
|
if colls[i].Schema == "" {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
var schema models.CollectionSchema
|
||||||
|
if err := json.Unmarshal([]byte(colls[i].Schema), &schema); err != nil {
|
||||||
|
// A schema that will not parse is a different defect, reported
|
||||||
|
// elsewhere; hydration declines to be the second voice.
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
for _, def := range schema.Fields {
|
||||||
|
if def.Type == "relation" {
|
||||||
|
out[colls[i].ID] = schema
|
||||||
|
break
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return out, nil
|
||||||
|
}
|
||||||
@@ -85,7 +85,7 @@ func (s *Server) resolveRelationReferentsAs(
|
|||||||
return canonical
|
return canonical
|
||||||
}
|
}
|
||||||
|
|
||||||
issues, err := s.store.ResolveRelationReferents(workspaceID, schema, fieldMap)
|
issues, err := s.store.ResolveRelationReferents(workspaceID, schema, fieldMap, s.relationVisibility(r, role))
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
@@ -120,9 +120,21 @@ func (s *Server) resolveRelationReferentsAs(
|
|||||||
// where a person belongs" is the useful half of this reason.
|
// where a person belongs" is the useful half of this reason.
|
||||||
//
|
//
|
||||||
// Any other issue is already `not_found`-shaped and needs nothing.
|
// Any other issue is already `not_found`-shaped and needs nothing.
|
||||||
|
// An `ambiguous` reason arrives already decided: since the
|
||||||
|
// resolver takes this requester's visibility predicate, its count
|
||||||
|
// is the VISIBLE count and there is nothing left to narrow here.
|
||||||
|
// It used to be re-decided in this layer, which left the copy
|
||||||
|
// transaction — where no server pass runs — leaking.
|
||||||
if ri.Reason != store.RelationTargetWrongCollection {
|
if ri.Reason != store.RelationTargetWrongCollection {
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
if ri.VisibilityChecked {
|
||||||
|
// The resolver judged this one against this requester already
|
||||||
|
// (title path). Re-resolving it here would run a title through
|
||||||
|
// a UUID-or-ref ladder, find nothing, and collapse a reason the
|
||||||
|
// caller is entitled to see.
|
||||||
|
continue
|
||||||
|
}
|
||||||
target, terr := s.store.ResolveRelationTarget(workspaceID, ri.Value)
|
target, terr := s.store.ResolveRelationTarget(workspaceID, ri.Value)
|
||||||
if terr != nil {
|
if terr != nil {
|
||||||
return nil, terr
|
return nil, terr
|
||||||
@@ -215,9 +227,13 @@ func collapseIssue(issues []store.RelationIssue, key string, reason store.Relati
|
|||||||
// Reviewer named ONE site; this is applied at all five late-default sites,
|
// Reviewer named ONE site; this is applied at all five late-default sites,
|
||||||
// per CONVE-18 — the class is "a store-resolved issue reaching a caller
|
// per CONVE-18 — the class is "a store-resolved issue reaching a caller
|
||||||
// without passing the visibility collapse", not the one call it was spotted at.
|
// without passing the visibility collapse", not the one call it was spotted at.
|
||||||
|
// ONLY the ref path reaches here now. A title-derived issue is decided inside
|
||||||
|
// the resolver, which takes this requester's visibility predicate — so an
|
||||||
|
// invisible title match never becomes a `wrong_collection` in the first place,
|
||||||
|
// and there is nothing for this pass to collapse.
|
||||||
func (s *Server) collapseInvisibleRelationIssues(r *http.Request, workspaceID, role string, issues []store.RelationIssue) error {
|
func (s *Server) collapseInvisibleRelationIssues(r *http.Request, workspaceID, role string, issues []store.RelationIssue) error {
|
||||||
for i := range issues {
|
for i := range issues {
|
||||||
if issues[i].Reason != store.RelationTargetWrongCollection {
|
if issues[i].Reason != store.RelationTargetWrongCollection || issues[i].VisibilityChecked {
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
target, terr := s.store.ResolveRelationTarget(workspaceID, issues[i].Value)
|
target, terr := s.store.ResolveRelationTarget(workspaceID, issues[i].Value)
|
||||||
@@ -452,7 +468,7 @@ func (s *Server) resolveRelationsForWrite(
|
|||||||
// — and reporting none of it. The refusals are still returned; they are
|
// — and reporting none of it. The refusals are still returned; they are
|
||||||
// the carry report, not a stop.
|
// the carry report, not a stop.
|
||||||
|
|
||||||
lateDropped, err := s.store.ResolveLateRelationDefaults(workspaceID, schema, fieldMap, presentBefore)
|
lateDropped, err := s.store.ResolveLateRelationDefaults(s.relationVisibility(r, role), workspaceID, schema, fieldMap, presentBefore)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, nil, err
|
return nil, nil, err
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1,116 @@
|
|||||||
|
package server
|
||||||
|
|
||||||
|
import (
|
||||||
|
"encoding/json"
|
||||||
|
"net/http"
|
||||||
|
"net/http/httptest"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"github.com/PerpetualSoftware/pad/internal/models"
|
||||||
|
)
|
||||||
|
|
||||||
|
// The copy door and the ruled visibility property (PLAN-2857 U6).
|
||||||
|
//
|
||||||
|
// These are the legs codex round 3 named as missing, and they are the reason
|
||||||
|
// the property had to move INTO the resolver. The cross-workspace copy runs the
|
||||||
|
// resolver inside its own transaction, so a server-layer second pass could not
|
||||||
|
// reach it: a title override narrowed correctly and then had the narrowed id
|
||||||
|
// thrown away, and the copy's late defaults were never narrowed at all — which
|
||||||
|
// made PREFLIGHT and COPY disagree, the DR-6 divergence class in a new place.
|
||||||
|
|
||||||
|
// twinsInDest creates two same-titled live items in the destination target
|
||||||
|
// collection and returns them, plus an editor who can see the FIRST only.
|
||||||
|
//
|
||||||
|
// The editor's access to wsB is collection-level on the tasks collection and
|
||||||
|
// item-level on one twin. That combination is the point: it is what makes the
|
||||||
|
// visible count differ from the live count.
|
||||||
|
func twinsInDest(t *testing.T, f *relationFixture, title string) (*models.Item, *models.Item, *models.User) {
|
||||||
|
t.Helper()
|
||||||
|
visible, err := f.srv.store.CreateItem(f.wsB.ID, f.targetsB.ID, models.ItemCreate{Title: title, CreatedBy: f.owner.ID})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("CreateItem(visible twin): %v", err)
|
||||||
|
}
|
||||||
|
hidden, err := f.srv.store.CreateItem(f.wsB.ID, f.targetsB.ID, models.ItemCreate{Title: title, CreatedBy: f.owner.ID})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("CreateItem(hidden twin): %v", err)
|
||||||
|
}
|
||||||
|
u := f.restrictedEditor("copy-twin@example.com", "copytwin",
|
||||||
|
[]string{f.collA.ID}, []string{f.collB.ID})
|
||||||
|
if _, err := f.srv.store.CreateItemGrant(f.wsB.ID, visible.ID, u.ID, "read", f.owner.ID); err != nil {
|
||||||
|
t.Fatalf("CreateItemGrant: %v", err)
|
||||||
|
}
|
||||||
|
return visible, hidden, u
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestRelationTitleCopy_OverrideResolvesForACallerWhoSeesOneTwin is round 3's
|
||||||
|
// first P1: the copy re-resolved the raw title inside its transaction and saw
|
||||||
|
// `ambiguous`, so a hidden twin changed the copy's STATUS.
|
||||||
|
func TestRelationTitleCopy_OverrideResolvesForACallerWhoSeesOneTwin(t *testing.T) {
|
||||||
|
f := newCopyRelationFixture(t)
|
||||||
|
visible, hidden, u := twinsInDest(t, f, "Twin In B")
|
||||||
|
|
||||||
|
body := f.baseBody()
|
||||||
|
body["field_overrides"] = map[string]any{"owner_ref": "Twin In B"}
|
||||||
|
|
||||||
|
rr := f.callCopy(u, reqOpts{}, body)
|
||||||
|
if rr.Code != http.StatusCreated && rr.Code != http.StatusOK {
|
||||||
|
t.Fatalf("a caller who can see exactly ONE match must have the override resolve — the twin they cannot see is not their ambiguity: %d %s", rr.Code, rr.Body.String())
|
||||||
|
}
|
||||||
|
var res ItemCopyResult
|
||||||
|
decodeInto(t, rr, &res)
|
||||||
|
got := f.persistedFields(res.Item.ID)["owner_ref"]
|
||||||
|
if got != visible.ID {
|
||||||
|
t.Errorf("stored owner_ref = %#v, want the VISIBLE twin %s (hidden twin is %s)", got, visible.ID, hidden.ID)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestRelationTitleCopy_PreflightAndCopyAgreeOnANarrowedTitle is round 3's
|
||||||
|
// second P1, and it is the one a per-door test cannot make: agreement between
|
||||||
|
// two doors is a claim about the PAIR.
|
||||||
|
//
|
||||||
|
// Preflight applied the narrowing and the copy did not, so the preview promised
|
||||||
|
// a value the copy then refused or dropped.
|
||||||
|
func TestRelationTitleCopy_PreflightAndCopyAgreeOnANarrowedTitle(t *testing.T) {
|
||||||
|
f := newCopyRelationFixture(t)
|
||||||
|
visible, _, u := twinsInDest(t, f, "Agreed Twin")
|
||||||
|
|
||||||
|
body := f.baseBody()
|
||||||
|
body["field_overrides"] = map[string]any{"owner_ref": "Agreed Twin"}
|
||||||
|
|
||||||
|
// PREFLIGHT first, as a user would.
|
||||||
|
preRR := f.call(u, reqOpts{}, body)
|
||||||
|
if preRR.Code != http.StatusOK {
|
||||||
|
t.Fatalf("preflight: expected 200, got %d: %s", preRR.Code, preRR.Body.String())
|
||||||
|
}
|
||||||
|
var pre ItemCopyPreflight
|
||||||
|
decodeInto(t, preRR, &pre)
|
||||||
|
promised, carried := carriedValue(pre, "owner_ref")
|
||||||
|
if !carried {
|
||||||
|
reason, dropped := droppedReason(pre, "owner_ref")
|
||||||
|
t.Fatalf("preflight did not promise the narrowed value (dropped=%v reason=%q)", dropped, reason)
|
||||||
|
}
|
||||||
|
if promised != visible.ID {
|
||||||
|
t.Errorf("preflight promised %#v, want the visible twin %s", promised, visible.ID)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Then the COPY, with the identical body. The two must agree.
|
||||||
|
copyRR := f.callCopy(u, reqOpts{}, body)
|
||||||
|
if copyRR.Code != http.StatusCreated && copyRR.Code != http.StatusOK {
|
||||||
|
t.Fatalf("the preflight promised a value and the copy refused: %d %s", copyRR.Code, copyRR.Body.String())
|
||||||
|
}
|
||||||
|
var res ItemCopyResult
|
||||||
|
decodeInto(t, copyRR, &res)
|
||||||
|
stored := f.persistedFields(res.Item.ID)["owner_ref"]
|
||||||
|
if stored != promised {
|
||||||
|
t.Errorf("preflight promised %#v and the copy stored %#v — a preview that does not predict the copy is the DR-6 divergence", promised, stored)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// decodeInto unmarshals a recorder body, failing with the body on error so a
|
||||||
|
// decode failure names what came back rather than only that it did not parse.
|
||||||
|
func decodeInto(t *testing.T, rr *httptest.ResponseRecorder, out any) {
|
||||||
|
t.Helper()
|
||||||
|
if err := json.Unmarshal(rr.Body.Bytes(), out); err != nil {
|
||||||
|
t.Fatalf("decode: %v\nbody: %s", err, rr.Body.String())
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -0,0 +1,586 @@
|
|||||||
|
package server
|
||||||
|
|
||||||
|
import (
|
||||||
|
"encoding/json"
|
||||||
|
"fmt"
|
||||||
|
"net/http"
|
||||||
|
"net/http/httptest"
|
||||||
|
"strings"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"github.com/PerpetualSoftware/pad/internal/models"
|
||||||
|
)
|
||||||
|
|
||||||
|
// Door-level legs for exact-title resolution and read hydration (PLAN-2857 U6).
|
||||||
|
//
|
||||||
|
// internal/store already tests the title ladder and the hydrator directly, so
|
||||||
|
// those vouch for the COMPONENTS. They say nothing about whether any door is
|
||||||
|
// BOUND to them — a create handler that never reaches the title rung, or a read
|
||||||
|
// handler that never hydrates, passes every store test. These are the binding
|
||||||
|
// claims, driven through the handlers a client actually reaches. Same
|
||||||
|
// separation, and same reason, as the U1 table above.
|
||||||
|
|
||||||
|
// TestRelationTitleDoors_ThreeSpellingsAgreeThroughTheCreateDoor is the design
|
||||||
|
// row's proving test at the door: uuid, ref and exact title, each through
|
||||||
|
// handleCreateItem, must land on an identical STORED id.
|
||||||
|
//
|
||||||
|
// Read back out of the database rather than off the response, for the reason
|
||||||
|
// storedRelation documents: a handler that echoed the right thing while writing
|
||||||
|
// the wrong thing would pass a response-only check.
|
||||||
|
func TestRelationTitleDoors_ThreeSpellingsAgreeThroughTheCreateDoor(t *testing.T) {
|
||||||
|
f := newDoorFixture(t)
|
||||||
|
path := "/api/v1/workspaces/" + f.ws.Slug + "/collections/" + f.tasks.Slug + "/items"
|
||||||
|
params := map[string]string{"collSlug": f.tasks.Slug}
|
||||||
|
|
||||||
|
for _, supplied := range []string{f.target.ID, f.target.Ref, f.target.Title} {
|
||||||
|
body := map[string]any{"title": "By " + supplied, "fields": map[string]any{"owner_ref": supplied}}
|
||||||
|
rr := f.call(f.srv.handleCreateItem, "POST", path, params, body)
|
||||||
|
if rr.Code != http.StatusCreated {
|
||||||
|
t.Fatalf("supplied %q: expected 201, got %d: %s", supplied, rr.Code, rr.Body.String())
|
||||||
|
}
|
||||||
|
var created models.Item
|
||||||
|
if err := json.Unmarshal(rr.Body.Bytes(), &created); err != nil {
|
||||||
|
t.Fatalf("supplied %q: decode: %v", supplied, err)
|
||||||
|
}
|
||||||
|
stored, ok := f.storedRelation(created.ID)
|
||||||
|
if !ok {
|
||||||
|
t.Fatalf("supplied %q: no owner_ref stored", supplied)
|
||||||
|
}
|
||||||
|
if stored != f.target.ID {
|
||||||
|
t.Errorf("supplied %q stored %v, want the canonical id %s — the door must agree with the other two spellings", supplied, stored, f.target.ID)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestRelationTitleDoors_TitleWrongCollectionDoesNotDiscloseExistence is
|
||||||
|
// ruling (3) on the TITLE path, and it is the leg the addendum was written for.
|
||||||
|
//
|
||||||
|
// The ref-path equivalent (TestRelationDoors_WrongCollectionDoesNotDisclose-
|
||||||
|
// Existence) collapses by RE-RESOLVING the value through ResolveRelationTarget.
|
||||||
|
// That ladder does not speak titles, so a title-derived wrong_collection would
|
||||||
|
// re-resolve to nil, take the not-found arm, and collapse for EVERY caller —
|
||||||
|
// visible and invisible alike. The invisible leg below would still pass. Only
|
||||||
|
// the VISIBLE leg catches it, which is why both are here and why the fix
|
||||||
|
// carries the matched id on the issue instead of looking it up twice.
|
||||||
|
func TestRelationTitleDoors_TitleWrongCollectionDoesNotDiscloseExistence(t *testing.T) {
|
||||||
|
f := newDoorFixture(t)
|
||||||
|
// A live item in a collection that is NOT the relation's declared target,
|
||||||
|
// addressed by its TITLE.
|
||||||
|
other := mustSchemaCollection(t, f.srv, f.ws.ID, "Vaults", `{"fields":[]}`)
|
||||||
|
if _, err := f.srv.store.CreateItem(f.ws.ID, other.ID, models.ItemCreate{
|
||||||
|
Title: "Ledger", CreatedBy: f.owner.ID,
|
||||||
|
}); err != nil {
|
||||||
|
t.Fatalf("CreateItem(ledger): %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
body := map[string]any{"title": "New", "fields": map[string]any{"owner_ref": "Ledger"}}
|
||||||
|
path := "/api/v1/workspaces/" + f.ws.Slug + "/collections/" + f.tasks.Slug + "/items"
|
||||||
|
params := map[string]string{"collSlug": f.tasks.Slug}
|
||||||
|
|
||||||
|
// VISIBLE: the owner can see Vaults, so they keep the specific reason. This
|
||||||
|
// is the leg that dies if the collapse re-resolves the title.
|
||||||
|
seeing := f.call(f.srv.handleCreateItem, "POST", path, params, body)
|
||||||
|
if seeing.Code != http.StatusBadRequest {
|
||||||
|
t.Fatalf("expected 400, got %d: %s", seeing.Code, seeing.Body.String())
|
||||||
|
}
|
||||||
|
if !strings.Contains(seeing.Body.String(), "is not an item in collection") {
|
||||||
|
t.Fatalf("a caller who CAN see the target lost the wrong_collection reason — the collapse judged the wrong item, or could not find it at all: %s", seeing.Body.String())
|
||||||
|
}
|
||||||
|
|
||||||
|
// INVISIBLE: an editor with no access to Vaults must not learn that the
|
||||||
|
// title names anything. A title is guessable in a way a ref is not, which
|
||||||
|
// is why this leg exists at all.
|
||||||
|
blind := mustUser(t, f.srv, "blind-title@example.com", "blindtitle", "")
|
||||||
|
if err := f.srv.store.AddWorkspaceMember(f.ws.ID, blind.ID, "editor"); err != nil {
|
||||||
|
t.Fatalf("AddWorkspaceMember: %v", err)
|
||||||
|
}
|
||||||
|
if err := f.srv.store.SetMemberCollectionAccess(f.ws.ID, blind.ID, "specific",
|
||||||
|
[]string{f.tasks.ID, f.people.ID}); err != nil {
|
||||||
|
t.Fatalf("SetMemberCollectionAccess: %v", err)
|
||||||
|
}
|
||||||
|
hidden := f.callAs(blind, "editor", f.srv.handleCreateItem, "POST", path, params, body)
|
||||||
|
if hidden.Code != http.StatusBadRequest {
|
||||||
|
t.Fatalf("expected 400, got %d: %s", hidden.Code, hidden.Body.String())
|
||||||
|
}
|
||||||
|
if strings.Contains(hidden.Body.String(), "is not an item in collection") {
|
||||||
|
t.Fatalf("the refusal tells a caller who cannot see the target that the TITLE names something: %s", hidden.Body.String())
|
||||||
|
}
|
||||||
|
if !strings.Contains(hidden.Body.String(), "does not name an item") {
|
||||||
|
t.Fatalf("expected the not_found phrasing, got: %s", hidden.Body.String())
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestRelationTitleDoors_ReadHydratesTheTarget is the hydration half's binding
|
||||||
|
// claim: the read door returns relation_targets, and a dangling value comes
|
||||||
|
// back ID-ONLY and PRESENT rather than omitted.
|
||||||
|
func TestRelationTitleDoors_ReadHydratesTheTarget(t *testing.T) {
|
||||||
|
f := newDoorFixture(t)
|
||||||
|
|
||||||
|
live := f.seed(`{"owner_ref":"` + f.target.ID + `"}`)
|
||||||
|
dangling := f.seed(`{"owner_ref":"00000000-0000-4000-8000-00000000dead"}`)
|
||||||
|
|
||||||
|
get := func(it *models.Item) models.Item {
|
||||||
|
t.Helper()
|
||||||
|
rr := f.call(f.srv.handleGetItem, "GET",
|
||||||
|
"/api/v1/workspaces/"+f.ws.Slug+"/items/"+it.Slug,
|
||||||
|
map[string]string{"itemSlug": it.Slug}, nil)
|
||||||
|
if rr.Code != http.StatusOK {
|
||||||
|
t.Fatalf("get %s: expected 200, got %d: %s", it.Slug, rr.Code, rr.Body.String())
|
||||||
|
}
|
||||||
|
var out models.Item
|
||||||
|
if err := json.Unmarshal(rr.Body.Bytes(), &out); err != nil {
|
||||||
|
t.Fatalf("decode %s: %v", it.Slug, err)
|
||||||
|
}
|
||||||
|
return out
|
||||||
|
}
|
||||||
|
|
||||||
|
got := get(live)
|
||||||
|
target, ok := got.RelationTargets["owner_ref"]
|
||||||
|
if !ok {
|
||||||
|
t.Fatal("the read door returned no relation_targets — the hydrator is not bound to it, which every store-level test would still pass")
|
||||||
|
}
|
||||||
|
if target.ID != f.target.ID || target.Ref != f.target.Ref || target.Title != f.target.Title {
|
||||||
|
t.Errorf("hydrated as %+v, want {id:%s ref:%s title:%s}", target, f.target.ID, f.target.Ref, f.target.Title)
|
||||||
|
}
|
||||||
|
|
||||||
|
got = get(dangling)
|
||||||
|
target, ok = got.RelationTargets["owner_ref"]
|
||||||
|
if !ok {
|
||||||
|
t.Fatal("a dangling value was OMITTED; omission reads as \"this item has no relation\", which is a different and false statement")
|
||||||
|
}
|
||||||
|
if target.Ref != "" || target.Title != "" {
|
||||||
|
t.Errorf("a dangling value hydrated as %+v, want id-only", target)
|
||||||
|
}
|
||||||
|
if target.ID == "" {
|
||||||
|
t.Error("a dangling entry lost the stored id, which is the only thing the caller had")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestRelationTitleDoors_ReadDoesNotHydrateAnInvisibleTarget is the read-side
|
||||||
|
// mirror of the disclosure rule above.
|
||||||
|
//
|
||||||
|
// The write side collapses `wrong_collection` to `not_found` for a target the
|
||||||
|
// caller cannot see. Hydrating that same item's ref and title on the READ would
|
||||||
|
// hand back exactly what the collapse withholds — a second door onto the fact
|
||||||
|
// the first one closes.
|
||||||
|
func TestRelationTitleDoors_ReadDoesNotHydrateAnInvisibleTarget(t *testing.T) {
|
||||||
|
f := newDoorFixture(t)
|
||||||
|
|
||||||
|
it := f.seed(`{"owner_ref":"` + f.target.ID + `"}`)
|
||||||
|
|
||||||
|
blind := mustUser(t, f.srv, "blind-read@example.com", "blindread", "")
|
||||||
|
if err := f.srv.store.AddWorkspaceMember(f.ws.ID, blind.ID, "editor"); err != nil {
|
||||||
|
t.Fatalf("AddWorkspaceMember: %v", err)
|
||||||
|
}
|
||||||
|
// Can see the item itself, but NOT the collection its relation points into.
|
||||||
|
if err := f.srv.store.SetMemberCollectionAccess(f.ws.ID, blind.ID, "specific",
|
||||||
|
[]string{f.tasks.ID}); err != nil {
|
||||||
|
t.Fatalf("SetMemberCollectionAccess: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
rr := f.callAs(blind, "editor", f.srv.handleGetItem, "GET",
|
||||||
|
"/api/v1/workspaces/"+f.ws.Slug+"/items/"+it.Slug,
|
||||||
|
map[string]string{"itemSlug": it.Slug}, nil)
|
||||||
|
if rr.Code != http.StatusOK {
|
||||||
|
t.Fatalf("expected 200, got %d: %s", rr.Code, rr.Body.String())
|
||||||
|
}
|
||||||
|
var out models.Item
|
||||||
|
if err := json.Unmarshal(rr.Body.Bytes(), &out); err != nil {
|
||||||
|
t.Fatalf("decode: %v", err)
|
||||||
|
}
|
||||||
|
target, ok := out.RelationTargets["owner_ref"]
|
||||||
|
if !ok {
|
||||||
|
t.Fatal("the key was omitted entirely; id-only is the honest answer, omission says the item has no relation")
|
||||||
|
}
|
||||||
|
if target.Ref != "" || target.Title != "" {
|
||||||
|
t.Errorf("hydrated an invisible target as %+v — that is the ref and title the write-side collapse exists to withhold", target)
|
||||||
|
}
|
||||||
|
|
||||||
|
// CONTROL, same fixture: the owner CAN see People and gets the full triple.
|
||||||
|
// Without it, a hydrator that returned id-only for everything would pass.
|
||||||
|
ownerRR := f.call(f.srv.handleGetItem, "GET",
|
||||||
|
"/api/v1/workspaces/"+f.ws.Slug+"/items/"+it.Slug,
|
||||||
|
map[string]string{"itemSlug": it.Slug}, nil)
|
||||||
|
var ownerOut models.Item
|
||||||
|
if err := json.Unmarshal(ownerRR.Body.Bytes(), &ownerOut); err != nil {
|
||||||
|
t.Fatalf("decode owner: %v", err)
|
||||||
|
}
|
||||||
|
if ownerOut.RelationTargets["owner_ref"].Ref != f.target.Ref {
|
||||||
|
t.Errorf("control: the owner should see the full triple, got %+v", ownerOut.RelationTargets["owner_ref"])
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// --- title resolution is scoped to what the caller can SEE ----------------
|
||||||
|
//
|
||||||
|
// Lead ruling on PLAN-2857 U6, one step past "collapse ambiguous when nothing
|
||||||
|
// is visible": among live exact-title matches in the declared collection, only
|
||||||
|
// VISIBLE ones count — zero is not_found, one RESOLVES, two or more is
|
||||||
|
// ambiguous. A hidden match never changes the answer a caller gets.
|
||||||
|
//
|
||||||
|
// The rule lives in the RESOLVER, which takes the requester's visibility
|
||||||
|
// predicate (store.RelationVisibilityFunc). It was a server-side second pass
|
||||||
|
// first, and that left the cross-workspace copy — which runs the resolver
|
||||||
|
// inside its own transaction — deciding without it.
|
||||||
|
|
||||||
|
// restrictedTitleFixture builds two same-titled colours and a member who can
|
||||||
|
// see the Doors collection plus, through an ITEM-LEVEL grant, exactly the
|
||||||
|
// colours named. Item grants are the point: they are what makes
|
||||||
|
// VisibleCollectionIDs nav-lenient, and therefore what a collection-level
|
||||||
|
// filter gets wrong.
|
||||||
|
func restrictedTitleFixture(t *testing.T, f *doorFixture, email string, grantItems ...*models.Item) *models.User {
|
||||||
|
t.Helper()
|
||||||
|
u := mustUser(t, f.srv, email, strings.ReplaceAll(email, "@example.com", ""), "")
|
||||||
|
if err := f.srv.store.AddWorkspaceMember(f.ws.ID, u.ID, "editor"); err != nil {
|
||||||
|
t.Fatalf("AddWorkspaceMember: %v", err)
|
||||||
|
}
|
||||||
|
// Explicit access to the collection under test only. People is reachable
|
||||||
|
// ONLY through the item grants below — the nav-lenient case.
|
||||||
|
if err := f.srv.store.SetMemberCollectionAccess(f.ws.ID, u.ID, "specific", []string{f.tasks.ID}); err != nil {
|
||||||
|
t.Fatalf("SetMemberCollectionAccess: %v", err)
|
||||||
|
}
|
||||||
|
for _, it := range grantItems {
|
||||||
|
if _, err := f.srv.store.CreateItemGrant(f.ws.ID, it.ID, u.ID, "read", f.owner.ID); err != nil {
|
||||||
|
t.Fatalf("CreateItemGrant(%s): %v", it.Title, err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return u
|
||||||
|
}
|
||||||
|
|
||||||
|
// createByTitle drives the create door as `user` with a relation supplied BY
|
||||||
|
// TITLE, returning the recorder so each leg can assert on the outcome.
|
||||||
|
func (f *doorFixture) createByTitle(user *models.User, title, itemTitle string) *httptest.ResponseRecorder {
|
||||||
|
f.t.Helper()
|
||||||
|
return f.callAs(user, "editor", f.srv.handleCreateItem, "POST",
|
||||||
|
"/api/v1/workspaces/"+f.ws.Slug+"/collections/"+f.tasks.Slug+"/items",
|
||||||
|
map[string]string{"collSlug": f.tasks.Slug},
|
||||||
|
map[string]any{"title": itemTitle, "fields": map[string]any{"owner_ref": title}})
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestRelationTitleDoors_AmbiguityIsCountedOverVISIBLEMatchesOnly(t *testing.T) {
|
||||||
|
f := newDoorFixture(t)
|
||||||
|
// Two live people share a title. The OWNER sees both.
|
||||||
|
twinA, err := f.srv.store.CreateItem(f.ws.ID, f.people.ID, models.ItemCreate{Title: "Robin", CreatedBy: f.owner.ID})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("CreateItem(twinA): %v", err)
|
||||||
|
}
|
||||||
|
twinB, err := f.srv.store.CreateItem(f.ws.ID, f.people.ID, models.ItemCreate{Title: "Robin", CreatedBy: f.owner.ID})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("CreateItem(twinB): %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
t.Run("two visible matches stay ambiguous", func(t *testing.T) {
|
||||||
|
both := restrictedTitleFixture(t, f, "sees-both@example.com", twinA, twinB)
|
||||||
|
rr := f.createByTitle(both, "Robin", "Both")
|
||||||
|
if rr.Code != http.StatusBadRequest {
|
||||||
|
t.Fatalf("expected 400, got %d: %s", rr.Code, rr.Body.String())
|
||||||
|
}
|
||||||
|
if !strings.Contains(rr.Body.String(), "more than one") && !strings.Contains(rr.Body.String(), "ambiguous") {
|
||||||
|
t.Errorf("a caller who can see BOTH matches must be told the title is ambiguous, got: %s", rr.Body.String())
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("one visible match resolves, and the hidden twin changes nothing", func(t *testing.T) {
|
||||||
|
one := restrictedTitleFixture(t, f, "sees-one@example.com", twinA)
|
||||||
|
rr := f.createByTitle(one, "Robin", "Only one")
|
||||||
|
if rr.Code != http.StatusCreated {
|
||||||
|
t.Fatalf("a caller who can see exactly ONE match must have it resolve — the other is not ambiguity for them: %d %s", rr.Code, rr.Body.String())
|
||||||
|
}
|
||||||
|
var created models.Item
|
||||||
|
if err := json.Unmarshal(rr.Body.Bytes(), &created); err != nil {
|
||||||
|
t.Fatalf("decode: %v", err)
|
||||||
|
}
|
||||||
|
stored, ok := f.storedRelation(created.ID)
|
||||||
|
if !ok {
|
||||||
|
t.Fatal("no owner_ref stored")
|
||||||
|
}
|
||||||
|
if stored != twinA.ID {
|
||||||
|
t.Errorf("stored %v, want the VISIBLE twin %s", stored, twinA.ID)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("no visible match is not_found, never ambiguous", func(t *testing.T) {
|
||||||
|
none := restrictedTitleFixture(t, f, "sees-none@example.com")
|
||||||
|
rr := f.createByTitle(none, "Robin", "Neither")
|
||||||
|
if rr.Code != http.StatusBadRequest {
|
||||||
|
t.Fatalf("expected 400, got %d: %s", rr.Code, rr.Body.String())
|
||||||
|
}
|
||||||
|
body := rr.Body.String()
|
||||||
|
if strings.Contains(body, "more than one") || strings.Contains(body, "ambiguous") {
|
||||||
|
t.Errorf("telling a caller the title matches SEVERAL items they cannot see is the oracle this rule closes: %s", body)
|
||||||
|
}
|
||||||
|
if !strings.Contains(body, "does not name an item") {
|
||||||
|
t.Errorf("expected the not_found phrasing, got: %s", body)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestRelationTitleDoors_HydrationUsesITEMVisibilityNotCollectionVisibility is
|
||||||
|
// the read-side half of the same trap, and it is a leak a collection-level
|
||||||
|
// filter cannot catch.
|
||||||
|
//
|
||||||
|
// store.VisibleCollectionIDs is deliberately NAV-LENIENT: it folds in a
|
||||||
|
// collection reachable only through an ITEM grant "so the collection appears in
|
||||||
|
// navigation", leaving item-level filtering to handlers.
|
||||||
|
// requireCollectionFullyVisible's comment says so, and exists because BUG-1920
|
||||||
|
// walked into it. So a member holding a grant on ONE person gets People in that
|
||||||
|
// set — and a hydration filtered on it hands them the ref and title of EVERY
|
||||||
|
// person any item points at.
|
||||||
|
func TestRelationTitleDoors_HydrationUsesITEMVisibilityNotCollectionVisibility(t *testing.T) {
|
||||||
|
f := newDoorFixture(t)
|
||||||
|
|
||||||
|
granted, err := f.srv.store.CreateItem(f.ws.ID, f.people.ID, models.ItemCreate{Title: "Granted Person", CreatedBy: f.owner.ID})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("CreateItem(granted): %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
pointsAtGranted := f.seed(`{"owner_ref":"` + granted.ID + `"}`)
|
||||||
|
pointsAtSecret := f.seed(`{"owner_ref":"` + f.target.ID + `"}`)
|
||||||
|
|
||||||
|
// The grant is on `granted` ONLY. f.target (Ada) is a sibling in the same
|
||||||
|
// collection, with no grant.
|
||||||
|
member := restrictedTitleFixture(t, f, "item-grant@example.com", granted, pointsAtGranted, pointsAtSecret)
|
||||||
|
|
||||||
|
read := func(it *models.Item) models.RelationTarget {
|
||||||
|
t.Helper()
|
||||||
|
rr := f.callAs(member, "editor", f.srv.handleGetItem, "GET",
|
||||||
|
"/api/v1/workspaces/"+f.ws.Slug+"/items/"+it.Slug,
|
||||||
|
map[string]string{"itemSlug": it.Slug}, nil)
|
||||||
|
if rr.Code != http.StatusOK {
|
||||||
|
t.Fatalf("get %s: expected 200, got %d: %s", it.Slug, rr.Code, rr.Body.String())
|
||||||
|
}
|
||||||
|
var out models.Item
|
||||||
|
if err := json.Unmarshal(rr.Body.Bytes(), &out); err != nil {
|
||||||
|
t.Fatalf("decode: %v", err)
|
||||||
|
}
|
||||||
|
return out.RelationTargets["owner_ref"]
|
||||||
|
}
|
||||||
|
|
||||||
|
// THE LEAK: a sibling in a nav-lenient collection, with no grant of its own.
|
||||||
|
secret := read(pointsAtSecret)
|
||||||
|
if secret.Ref != "" || secret.Title != "" {
|
||||||
|
t.Errorf("hydrated an UNGRANTED sibling as %+v — the collection is only reachable through an item grant, and filtering on collection visibility hands over every item in it", secret)
|
||||||
|
}
|
||||||
|
if secret.ID == "" {
|
||||||
|
t.Error("the stored id was dropped; id-only is the honest answer, omission is not")
|
||||||
|
}
|
||||||
|
|
||||||
|
// CONTROL, same member and same request shape: the item they DO hold a
|
||||||
|
// grant on still hydrates fully. Without this a hydrator that returned
|
||||||
|
// id-only for everything would pass the assertion above.
|
||||||
|
ok := read(pointsAtGranted)
|
||||||
|
if ok.Ref != granted.Ref || ok.Title != "Granted Person" {
|
||||||
|
t.Errorf("control: the GRANTED target should hydrate fully, got %+v", ok)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestRelationTitleDoors_VisibilityNarrowingHasNoCountBoundary is the
|
||||||
|
// regression test for codex round 2's P1: the first fix bounded the candidate
|
||||||
|
// list at a constant and answered `ambiguous` past it, which made the BOUND an
|
||||||
|
// oracle — a caller could tell "one visible plus cap hidden" (resolves) from
|
||||||
|
// "one visible plus cap+1 hidden" (ambiguous), and the difference is entirely
|
||||||
|
// about items they cannot see.
|
||||||
|
//
|
||||||
|
// The fix pages through candidates and stops at the SECOND VISIBLE one, so no
|
||||||
|
// match count changes the answer. This drives well past the page size with a
|
||||||
|
// single visible match: it must still resolve.
|
||||||
|
func TestRelationTitleDoors_VisibilityNarrowingHasNoCountBoundary(t *testing.T) {
|
||||||
|
f := newDoorFixture(t)
|
||||||
|
|
||||||
|
// One visible match. Creation ORDER does not place it in the walk — the
|
||||||
|
// walk goes by `id` and ids are random UUIDs — so this is not "created
|
||||||
|
// first, therefore visited last". What forces the walk past the page
|
||||||
|
// boundary is the COUNT: 251 matches against a page size of 100, so the
|
||||||
|
// single visible one cannot be decided without paging wherever it lands.
|
||||||
|
visible, err := f.srv.store.CreateItem(f.ws.ID, f.people.ID, models.ItemCreate{Title: "Crowd", CreatedBy: f.owner.ID})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("CreateItem(visible): %v", err)
|
||||||
|
}
|
||||||
|
// Comfortably past relationTitleCandidatePage (100), so the walk pages.
|
||||||
|
const hidden = 250
|
||||||
|
for i := 0; i < hidden; i++ {
|
||||||
|
if _, err := f.srv.store.CreateItem(f.ws.ID, f.people.ID, models.ItemCreate{
|
||||||
|
Title: "Crowd", CreatedBy: f.owner.ID,
|
||||||
|
}); err != nil {
|
||||||
|
t.Fatalf("CreateItem(hidden %d): %v", i, err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
member := restrictedTitleFixture(t, f, "one-in-a-crowd@example.com", visible)
|
||||||
|
rr := f.createByTitle(member, "Crowd", "Found in the crowd")
|
||||||
|
if rr.Code != http.StatusCreated {
|
||||||
|
t.Fatalf("a caller who can see exactly ONE of %d matches must have it resolve, whatever the total: %d %s", hidden+1, rr.Code, rr.Body.String())
|
||||||
|
}
|
||||||
|
var created models.Item
|
||||||
|
if err := json.Unmarshal(rr.Body.Bytes(), &created); err != nil {
|
||||||
|
t.Fatalf("decode: %v", err)
|
||||||
|
}
|
||||||
|
stored, ok := f.storedRelation(created.ID)
|
||||||
|
if !ok {
|
||||||
|
t.Fatal("no owner_ref stored")
|
||||||
|
}
|
||||||
|
if stored != visible.ID {
|
||||||
|
t.Errorf("stored %v, want the one visible match %s", stored, visible.ID)
|
||||||
|
}
|
||||||
|
|
||||||
|
// CONTROL: the OWNER sees all 251 and is told it is ambiguous. Without
|
||||||
|
// this, a narrowing that resolved to the first match for everybody would
|
||||||
|
// pass the leg above.
|
||||||
|
ownerRR := f.call(f.srv.handleCreateItem, "POST",
|
||||||
|
"/api/v1/workspaces/"+f.ws.Slug+"/collections/"+f.tasks.Slug+"/items",
|
||||||
|
map[string]string{"collSlug": f.tasks.Slug},
|
||||||
|
map[string]any{"title": "Owner tries", "fields": map[string]any{"owner_ref": "Crowd"}})
|
||||||
|
if ownerRR.Code != http.StatusBadRequest {
|
||||||
|
t.Fatalf("control: a caller who sees every match must be refused, got %d: %s", ownerRR.Code, ownerRR.Body.String())
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestRelationTitleDoors_UpdateDoorAppliesTheSameNarrowing answers codex round
|
||||||
|
// 2's P2 on coverage: the three ambiguity legs only drove CREATE, and each door
|
||||||
|
// reaches the resolver by its own route.
|
||||||
|
func TestRelationTitleDoors_UpdateDoorAppliesTheSameNarrowing(t *testing.T) {
|
||||||
|
f := newDoorFixture(t)
|
||||||
|
twinA, err := f.srv.store.CreateItem(f.ws.ID, f.people.ID, models.ItemCreate{Title: "Sam", CreatedBy: f.owner.ID})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("CreateItem(twinA): %v", err)
|
||||||
|
}
|
||||||
|
if _, err := f.srv.store.CreateItem(f.ws.ID, f.people.ID, models.ItemCreate{Title: "Sam", CreatedBy: f.owner.ID}); err != nil {
|
||||||
|
t.Fatalf("CreateItem(twinB): %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
subject := f.seed(`{"status":"open"}`)
|
||||||
|
member := restrictedTitleFixture(t, f, "update-sees-one@example.com", twinA, subject)
|
||||||
|
|
||||||
|
rr := f.callAs(member, "editor", f.srv.handleUpdateItem, "PATCH",
|
||||||
|
"/api/v1/workspaces/"+f.ws.Slug+"/items/"+subject.Slug,
|
||||||
|
map[string]string{"itemSlug": subject.Slug},
|
||||||
|
map[string]any{"fields_patch": map[string]any{"owner_ref": "Sam"}})
|
||||||
|
if rr.Code != http.StatusOK {
|
||||||
|
t.Fatalf("the update door must narrow the same way create does: %d %s", rr.Code, rr.Body.String())
|
||||||
|
}
|
||||||
|
stored, ok := f.storedRelation(subject.ID)
|
||||||
|
if !ok {
|
||||||
|
t.Fatal("no owner_ref stored")
|
||||||
|
}
|
||||||
|
if stored != twinA.ID {
|
||||||
|
t.Errorf("stored %v, want the VISIBLE twin %s", stored, twinA.ID)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestRelationTitleDoors_LateDefaultResolvesForACallerWhoSeesOne is codex round
|
||||||
|
// 2's other P1, and it is a leak through the OUTCOME rather than the message.
|
||||||
|
//
|
||||||
|
// A relation DEFAULT declared as a title goes through the late pass, which runs
|
||||||
|
// after validation injects it. The first fix could only downgrade a narrowed
|
||||||
|
// ambiguity to not_found there — ResolveLateRelationDefaults has already
|
||||||
|
// deleted the key and the collapse had no field map to restore it into. So a
|
||||||
|
// caller who was the twin's only visible match got the default APPLIED when
|
||||||
|
// they were the sole match and DROPPED when a twin they cannot see existed. The
|
||||||
|
// reason said nothing, and the outcome said everything.
|
||||||
|
func TestRelationTitleDoors_LateDefaultResolvesForACallerWhoSeesOne(t *testing.T) {
|
||||||
|
f := newDoorFixture(t)
|
||||||
|
|
||||||
|
visible, err := f.srv.store.CreateItem(f.ws.ID, f.people.ID, models.ItemCreate{Title: "Morgan", CreatedBy: f.owner.ID})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("CreateItem(visible): %v", err)
|
||||||
|
}
|
||||||
|
if _, err := f.srv.store.CreateItem(f.ws.ID, f.people.ID, models.ItemCreate{Title: "Morgan", CreatedBy: f.owner.ID}); err != nil {
|
||||||
|
t.Fatalf("CreateItem(hidden twin): %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// The default is a TITLE, and it is ambiguous in the store's own counting.
|
||||||
|
defaulted := mustSchemaCollection(t, f.srv, f.ws.ID, "TitleDefaulted", fmt.Sprintf(`{"fields":[
|
||||||
|
{"key":"status","label":"Status","type":"select","options":["open","done"]},
|
||||||
|
{"key":"priority","label":"Priority","type":"select","options":["low","high"]},
|
||||||
|
{"key":"owner_ref","label":"Owner","type":"relation","collection":%q,"default":"Morgan"}
|
||||||
|
]}`, f.people.Slug))
|
||||||
|
|
||||||
|
subject, err := f.srv.store.CreateItem(f.ws.ID, defaulted.ID, models.ItemCreate{
|
||||||
|
Title: "Gets a defaulted owner", Fields: `{"status":"open"}`, CreatedBy: f.owner.ID,
|
||||||
|
})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("CreateItem(subject): %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
member := restrictedTitleFixture(t, f, "late-default@example.com", visible, subject)
|
||||||
|
if err := f.srv.store.SetMemberCollectionAccess(f.ws.ID, member.ID, "specific",
|
||||||
|
[]string{f.tasks.ID, defaulted.ID}); err != nil {
|
||||||
|
t.Fatalf("SetMemberCollectionAccess: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// The BULK door, not fields_patch. The patch route resolves only the keys
|
||||||
|
// the caller named, so a defaulted relation never reaches the late pass
|
||||||
|
// there at all — I aimed this at fields_patch first and it proved nothing.
|
||||||
|
// `set-priority` is the same route TestRelationDoors_BulkUpdateResolves-
|
||||||
|
// InjectedRelationDefault uses, and it does run the injection.
|
||||||
|
rr := f.callAs(member, "editor", f.srv.handleBulkItems, "POST",
|
||||||
|
"/api/v1/workspaces/"+f.ws.Slug+"/items/bulk", nil,
|
||||||
|
map[string]any{"op": "set-priority", "ids": []string{subject.ID}, "priority": "high"})
|
||||||
|
if rr.Code != http.StatusOK {
|
||||||
|
t.Fatalf("expected 200, got %d: %s", rr.Code, rr.Body.String())
|
||||||
|
}
|
||||||
|
|
||||||
|
stored, ok := f.storedRelationKey(subject.ID, "owner_ref")
|
||||||
|
if !ok {
|
||||||
|
t.Fatal("the defaulted relation was DROPPED for a caller who can see exactly one match; the drop itself tells them a twin they cannot see exists")
|
||||||
|
}
|
||||||
|
if stored != visible.ID {
|
||||||
|
t.Errorf("stored %v, want the one VISIBLE match %s", stored, visible.ID)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestRelationTitleDoors_ProbeFindsAVisibleMatchNotAnArbitraryOne is codex
|
||||||
|
// round 4's P2, and it is the property failing in the other direction.
|
||||||
|
//
|
||||||
|
// When nothing in the declared collection carries the title, the resolver
|
||||||
|
// probes the workspace so it can answer `wrong_collection` instead of
|
||||||
|
// not_found. That probe took ONE arbitrary row and visibility-filtered it — so
|
||||||
|
// if the row it happened to pick was hidden, the caller got not_found even
|
||||||
|
// though a DIFFERENT live match they can see would have earned the specific
|
||||||
|
// reason. A hidden match changed the answer again.
|
||||||
|
func TestRelationTitleDoors_ProbeFindsAVisibleMatchNotAnArbitraryOne(t *testing.T) {
|
||||||
|
f := newDoorFixture(t)
|
||||||
|
|
||||||
|
// Two collections, NEITHER of them the relation's declared target, each
|
||||||
|
// holding an item with the same title. The caller can see one.
|
||||||
|
collOne := mustSchemaCollection(t, f.srv, f.ws.ID, "Vault One", `{"fields":[]}`)
|
||||||
|
collTwo := mustSchemaCollection(t, f.srv, f.ws.ID, "Vault Two", `{"fields":[]}`)
|
||||||
|
itemOne, err := f.srv.store.CreateItem(f.ws.ID, collOne.ID, models.ItemCreate{Title: "Split Title", CreatedBy: f.owner.ID})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("CreateItem(one): %v", err)
|
||||||
|
}
|
||||||
|
itemTwo, err := f.srv.store.CreateItem(f.ws.ID, collTwo.ID, models.ItemCreate{Title: "Split Title", CreatedBy: f.owner.ID})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("CreateItem(two): %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// DETERMINISM. The probe walks by `id`, and ids are random UUIDs, so which
|
||||||
|
// row it reaches first is chance — a test that hid an arbitrary one would
|
||||||
|
// pass roughly half the time against the BROKEN code and look fine. Hide
|
||||||
|
// the collection holding the LEXICALLY SMALLER id, so the first row the
|
||||||
|
// probe sees is always the hidden one and the leg always exercises the
|
||||||
|
// case it names.
|
||||||
|
hiddenColl, openColl := collOne, collTwo
|
||||||
|
if itemTwo.ID < itemOne.ID {
|
||||||
|
hiddenColl, openColl = collTwo, collOne
|
||||||
|
}
|
||||||
|
|
||||||
|
member := mustUser(t, f.srv, "probe-split@example.com", "probesplit", "")
|
||||||
|
if err := f.srv.store.AddWorkspaceMember(f.ws.ID, member.ID, "editor"); err != nil {
|
||||||
|
t.Fatalf("AddWorkspaceMember: %v", err)
|
||||||
|
}
|
||||||
|
// Can see the collection under test and openColl; NOT hiddenColl, whose
|
||||||
|
// item the probe reaches first.
|
||||||
|
if err := f.srv.store.SetMemberCollectionAccess(f.ws.ID, member.ID, "specific",
|
||||||
|
[]string{f.tasks.ID, openColl.ID}); err != nil {
|
||||||
|
t.Fatalf("SetMemberCollectionAccess: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
rr := f.createByTitle(member, "Split Title", "Probing")
|
||||||
|
if rr.Code != http.StatusBadRequest {
|
||||||
|
t.Fatalf("expected 400, got %d: %s", rr.Code, rr.Body.String())
|
||||||
|
}
|
||||||
|
body := rr.Body.String()
|
||||||
|
if !strings.Contains(body, "is not an item in collection") {
|
||||||
|
t.Errorf("the caller CAN see the match in %q, so they are owed the specific reason; the probe reaches %q first and must keep walking rather than filtering one arbitrary row: %s", openColl.Name, hiddenColl.Name, body)
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -0,0 +1,145 @@
|
|||||||
|
package server
|
||||||
|
|
||||||
|
import (
|
||||||
|
"encoding/json"
|
||||||
|
"fmt"
|
||||||
|
"net/http"
|
||||||
|
"strings"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"github.com/PerpetualSoftware/pad/internal/artifact"
|
||||||
|
"github.com/PerpetualSoftware/pad/internal/models"
|
||||||
|
)
|
||||||
|
|
||||||
|
// TestArtifactImport_DropsFieldKeysTheKindDoesNotDeclare pins the assumption
|
||||||
|
// that makes the artifact door's relation-visibility question MOOT
|
||||||
|
// (PLAN-2857 U6, codex round 6 P1).
|
||||||
|
//
|
||||||
|
// Round 6 raised a real-sounding P1: handleImportArtifact uses the
|
||||||
|
// `relationsCarry` posture, so a carried relation title would be judged by the
|
||||||
|
// IMPORTER's visibility and two people importing one artifact would store
|
||||||
|
// different bytes. The lead ruled that import KEEPS visibility — an import has
|
||||||
|
// no prior truth, and exempting it would let a crafted title resolve to a UUID
|
||||||
|
// the importer cannot see but can then read straight back off their own item.
|
||||||
|
//
|
||||||
|
// The reasoning is sound and the path does not exist. An artifact's frontmatter
|
||||||
|
// carries a FIXED ALLOW-LIST of field keys per kind, and anything else is
|
||||||
|
// dropped at DECODE, before any relation code runs — so no relation value can
|
||||||
|
// ride in an artifact, whatever the destination collection's schema has been
|
||||||
|
// reshaped to.
|
||||||
|
//
|
||||||
|
// I wrote the ruled test first and it passed while asserting nothing: the field
|
||||||
|
// never arrived, so the leg could not tell the visibility rule from its own
|
||||||
|
// absence, and a mutation removing the predicate entirely SURVIVED it.
|
||||||
|
//
|
||||||
|
// WHAT THIS TEST DOES AND DOES NOT ESTABLISH, because the same trap is easy to
|
||||||
|
// fall into twice. It asserts the OBSERVABLE invariant end to end: a
|
||||||
|
// frontmatter key the kind does not declare never reaches the stored blob, even
|
||||||
|
// when the destination schema declares it as a relation and the artifact bytes
|
||||||
|
// really carry it (they are hand-built below, because artifact.Encode filters
|
||||||
|
// on the way OUT and an Encode-built fixture never puts the key on the wire).
|
||||||
|
// The control asserts declared keys DO land, so it is not passing by storing
|
||||||
|
// nothing.
|
||||||
|
//
|
||||||
|
// It does NOT identify which layer drops the key, and it is not a guard on the
|
||||||
|
// decode allow-list specifically: widening playbookFieldKeys does not make this
|
||||||
|
// test fail, so at least one further filter sits behind it that I did not
|
||||||
|
// isolate. The claim is "no relation value reaches the import door", verified;
|
||||||
|
// not "here is the single mechanism that stops it".
|
||||||
|
func TestArtifactImport_DropsFieldKeysTheKindDoesNotDeclare(t *testing.T) {
|
||||||
|
srv := testServer(t)
|
||||||
|
wsSlug := createWSForTest(t, srv)
|
||||||
|
ws, err := srv.store.GetWorkspaceBySlug(wsSlug)
|
||||||
|
if err != nil || ws == nil {
|
||||||
|
t.Fatalf("GetWorkspaceBySlug(%s): %v", wsSlug, err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Reshape the destination so the schema WOULD accept the key. If the
|
||||||
|
// artifact format ever stops filtering, this is what lets the value land.
|
||||||
|
secrets := mustSchemaCollection(t, srv, ws.ID, "Secrets", `{"fields":[]}`)
|
||||||
|
if _, err := srv.store.CreateItem(ws.ID, secrets.ID, models.ItemCreate{Title: "Hidden Owner"}); err != nil {
|
||||||
|
t.Fatalf("CreateItem(target): %v", err)
|
||||||
|
}
|
||||||
|
colls, err := srv.store.ListCollections(ws.ID)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("ListCollections: %v", err)
|
||||||
|
}
|
||||||
|
var playbooks *models.Collection
|
||||||
|
for i := range colls {
|
||||||
|
if colls[i].Slug == "playbooks" {
|
||||||
|
playbooks = &colls[i]
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if playbooks == nil {
|
||||||
|
t.Fatalf("no playbooks collection in the seeded workspace (have %d)", len(colls))
|
||||||
|
}
|
||||||
|
var schema map[string]any
|
||||||
|
if err := json.Unmarshal([]byte(playbooks.Schema), &schema); err != nil {
|
||||||
|
t.Fatalf("decode playbooks schema: %v", err)
|
||||||
|
}
|
||||||
|
fields, _ := schema["fields"].([]any)
|
||||||
|
schema["fields"] = append(fields, map[string]any{
|
||||||
|
"key": "owner_ref", "label": "Owner", "type": "relation", "collection": secrets.Slug,
|
||||||
|
})
|
||||||
|
reshaped, err := json.Marshal(schema)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("encode schema: %v", err)
|
||||||
|
}
|
||||||
|
reshapedStr := string(reshaped)
|
||||||
|
if _, err := srv.store.UpdateCollection(playbooks.ID, models.CollectionUpdate{Schema: &reshapedStr}); err != nil {
|
||||||
|
t.Fatalf("UpdateCollection: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// The reshape must actually have taken, or the assertion below passes
|
||||||
|
// because the field is undeclared rather than because the door filters it.
|
||||||
|
reread, rerr := srv.store.GetCollection(playbooks.ID)
|
||||||
|
if rerr != nil {
|
||||||
|
t.Fatalf("re-read playbooks: %v", rerr)
|
||||||
|
}
|
||||||
|
if !strings.Contains(reread.Schema, "owner_ref") {
|
||||||
|
t.Fatalf("the schema reshape did not take, so this test would prove nothing: %s", reread.Schema)
|
||||||
|
}
|
||||||
|
|
||||||
|
// HAND-BUILT BYTES, not artifact.Encode. Encode writes only the kind's
|
||||||
|
// known keys into frontmatter, so an artifact built through it never
|
||||||
|
// carries the extra key and the decode-side filter this test is about is
|
||||||
|
// never exercised — my first version did exactly that and a mutation
|
||||||
|
// widening the decode allow-list SURVIVED it.
|
||||||
|
data := []byte("---\n" +
|
||||||
|
"pad_artifact: playbook\n" +
|
||||||
|
fmt.Sprintf("format_version: %v\n", artifact.FormatVersion) +
|
||||||
|
"title: Artifact With An Undeclared Key\n" +
|
||||||
|
"status: active\n" +
|
||||||
|
"trigger: manual\n" +
|
||||||
|
"scope: all\n" +
|
||||||
|
"owner_ref: Hidden Owner\n" +
|
||||||
|
"---\n\nbody\n")
|
||||||
|
|
||||||
|
rr := doArtifactRequest(srv, "POST", "/api/v1/workspaces/"+wsSlug+"/import-artifact", data)
|
||||||
|
if rr.Code != http.StatusCreated {
|
||||||
|
t.Fatalf("import: expected 201, got %d: %s", rr.Code, rr.Body.String())
|
||||||
|
}
|
||||||
|
var envelope struct {
|
||||||
|
Slug string `json:"slug"`
|
||||||
|
}
|
||||||
|
if err := json.Unmarshal(rr.Body.Bytes(), &envelope); err != nil {
|
||||||
|
t.Fatalf("decode: %v\nbody: %s", err, rr.Body.String())
|
||||||
|
}
|
||||||
|
stored, err := srv.store.GetItemBySlug(ws.ID, envelope.Slug)
|
||||||
|
if err != nil || stored == nil {
|
||||||
|
t.Fatalf("GetItemBySlug(%s): %v", envelope.Slug, err)
|
||||||
|
}
|
||||||
|
var blob map[string]any
|
||||||
|
if err := json.Unmarshal([]byte(stored.Fields), &blob); err != nil {
|
||||||
|
t.Fatalf("decode fields: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
if v, present := blob["owner_ref"]; present {
|
||||||
|
t.Errorf("the artifact door carried %q = %#v into a RELATION field. The frontmatter allow-list used to drop it before any relation code ran, which is why PLAN-2857 U6's visibility rule was never implemented for this door. It is now reachable and the lead's ruling needs applying there", "owner_ref", v)
|
||||||
|
}
|
||||||
|
// CONTROL: the DECLARED keys did land, so the assertion above is about the
|
||||||
|
// allow-list filtering rather than about the import storing nothing.
|
||||||
|
if blob["trigger"] != "manual" || blob["scope"] != "all" {
|
||||||
|
t.Errorf("declared frontmatter keys did not land (%#v) — the leg above proves nothing without this", blob)
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -1143,7 +1143,7 @@ func (s *Store) migrateCopyFields(q Queryer, destWorkspaceID, sourceFieldsJSON,
|
|||||||
if scope == items.CrossWorkspace {
|
if scope == items.CrossWorkspace {
|
||||||
mode = RelationCarryCrossWorkspace
|
mode = RelationCarryCrossWorkspace
|
||||||
}
|
}
|
||||||
relRefusals, relDropped, relErr := s.MigrateRelationReferentsQ(q, destWorkspaceID,
|
relRefusals, relDropped, relErr := s.MigrateRelationReferentsQ(q, canSee, destWorkspaceID,
|
||||||
items.SchemaForMigratedFields(targetSchema), migrated.Fields, overrides,
|
items.SchemaForMigratedFields(targetSchema), migrated.Fields, overrides,
|
||||||
CarriedSourceValues(currentFields, migrated.Dropped), mode)
|
CarriedSourceValues(currentFields, migrated.Dropped), mode)
|
||||||
if relErr != nil {
|
if relErr != nil {
|
||||||
@@ -1179,7 +1179,7 @@ func (s *Store) migrateCopyFields(q Queryer, destWorkspaceID, sourceFieldsJSON,
|
|||||||
}
|
}
|
||||||
// Relation defaults ValidateFields just injected, which the pass above
|
// Relation defaults ValidateFields just injected, which the pass above
|
||||||
// could not have seen (codex round 2).
|
// could not have seen (codex round 2).
|
||||||
lateDropped, lateErr := s.ResolveLateRelationDefaultsQ(q, destWorkspaceID,
|
lateDropped, lateErr := s.ResolveLateRelationDefaultsQ(q, canSee, destWorkspaceID,
|
||||||
items.SchemaForMigratedFields(targetSchema), migrated.Fields, relBefore)
|
items.SchemaForMigratedFields(targetSchema), migrated.Fields, relBefore)
|
||||||
if lateErr != nil {
|
if lateErr != nil {
|
||||||
return nil, nil, fmt.Errorf("copy item across workspaces: resolve relation defaults: %w", lateErr)
|
return nil, nil, fmt.Errorf("copy item across workspaces: resolve relation defaults: %w", lateErr)
|
||||||
|
|||||||
@@ -2,7 +2,9 @@ package store
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"database/sql"
|
"database/sql"
|
||||||
|
"encoding/json"
|
||||||
"fmt"
|
"fmt"
|
||||||
|
"sort"
|
||||||
"strings"
|
"strings"
|
||||||
|
|
||||||
"github.com/PerpetualSoftware/pad/internal/models"
|
"github.com/PerpetualSoftware/pad/internal/models"
|
||||||
@@ -48,6 +50,14 @@ const (
|
|||||||
// so nothing can be checked against it. A schema problem, surfaced rather
|
// so nothing can be checked against it. A schema problem, surfaced rather
|
||||||
// than treated as permission to store anything.
|
// than treated as permission to store anything.
|
||||||
RelationTargetMissing RelationIssueReason = "target_missing"
|
RelationTargetMissing RelationIssueReason = "target_missing"
|
||||||
|
// RelationTargetAmbiguous — the value is an exact TITLE matching more than
|
||||||
|
// one live item in the target collection, so it names no single referent
|
||||||
|
// (PLAN-2857 U6). Distinct from `not_found` deliberately: telling a caller
|
||||||
|
// their title matched nothing, when it matched twice, sends them to write
|
||||||
|
// the item that already exists. Titles are not unique in a collection and
|
||||||
|
// nothing in this codebase makes them so, which is why this reason exists
|
||||||
|
// rather than the lookup taking the first row.
|
||||||
|
RelationTargetAmbiguous RelationIssueReason = "ambiguous"
|
||||||
// RelationTargetInvalidShape — the value is not a string at all, so it
|
// RelationTargetInvalidShape — the value is not a string at all, so it
|
||||||
// cannot name anything. Normally `ValidateFields` catches this and the
|
// cannot name anything. Normally `ValidateFields` catches this and the
|
||||||
// resolver deliberately stays out of it (one error per defect), but an
|
// resolver deliberately stays out of it (one error per defect), but an
|
||||||
@@ -66,6 +76,24 @@ type RelationIssue struct {
|
|||||||
Value string
|
Value string
|
||||||
Target string
|
Target string
|
||||||
Reason RelationIssueReason
|
Reason RelationIssueReason
|
||||||
|
|
||||||
|
// VisibilityChecked is set when the RESOLVER already judged this issue
|
||||||
|
// against a requester — i.e. it was given a RelationVisibilityFunc.
|
||||||
|
//
|
||||||
|
// Only the TITLE path sets it, and only because the server's collapse pass
|
||||||
|
// must then leave it alone. That pass re-resolves the value through
|
||||||
|
// `ResolveRelationTarget`, which speaks a UUID-or-ref ladder and cannot
|
||||||
|
// resolve a title: it would find nothing, take the vanished-target arm, and
|
||||||
|
// collapse a `wrong_collection` the caller is entitled to see. Removing an
|
||||||
|
// earlier marker field reintroduced exactly that, and the VISIBLE leg of
|
||||||
|
// the door test caught it — the invisible leg passes either way.
|
||||||
|
//
|
||||||
|
// The ref path deliberately keeps its own collapse. Retrofitting it is a
|
||||||
|
// change to working code and belongs to BUG-3012, not here.
|
||||||
|
//
|
||||||
|
// Internal only: this type has no JSON tags and reaches callers through
|
||||||
|
// Message().
|
||||||
|
VisibilityChecked bool
|
||||||
}
|
}
|
||||||
|
|
||||||
// Message renders the issue the way every door reports it. One function so the
|
// Message renders the issue the way every door reports it. One function so the
|
||||||
@@ -80,6 +108,12 @@ func (ri RelationIssue) Message() string {
|
|||||||
// earlier wording named only the first and misdiagnosed the second as
|
// earlier wording named only the first and misdiagnosed the second as
|
||||||
// a schema that says nothing when it says something broken.
|
// a schema that says nothing when it says something broken.
|
||||||
return fmt.Sprintf("field %q has no valid target collection, so %q cannot be resolved", ri.Key, ri.Value)
|
return fmt.Sprintf("field %q has no valid target collection, so %q cannot be resolved", ri.Key, ri.Value)
|
||||||
|
case RelationTargetAmbiguous:
|
||||||
|
// Names the collection searched, like wrong_collection does, because
|
||||||
|
// the caller's next move is either to disambiguate by ref or to look
|
||||||
|
// at the duplicates — and both need to know where the search ran.
|
||||||
|
return fmt.Sprintf("field %q: %q matches more than one item in collection %q; use a ref (e.g. PREFIX-3) to say which",
|
||||||
|
ri.Key, ri.Value, ri.Target)
|
||||||
case RelationTargetInvalidShape:
|
case RelationTargetInvalidShape:
|
||||||
return fmt.Sprintf("field %q has a default that is not a reference", ri.Key)
|
return fmt.Sprintf("field %q has a default that is not a reference", ri.Key)
|
||||||
default:
|
default:
|
||||||
@@ -102,6 +136,7 @@ func RelationIssueReasons() []RelationIssueReason {
|
|||||||
RelationTargetMissing,
|
RelationTargetMissing,
|
||||||
RelationTargetNotPortable,
|
RelationTargetNotPortable,
|
||||||
RelationTargetInvalidShape,
|
RelationTargetInvalidShape,
|
||||||
|
RelationTargetAmbiguous,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -135,7 +170,11 @@ func RelationIssuesMessage(issues []RelationIssue) string {
|
|||||||
// targets are out of PLAN-2857 v1.
|
// targets are out of PLAN-2857 v1.
|
||||||
// - Empty and absent values are not issues. Clearing a relation is a
|
// - Empty and absent values are not issues. Clearing a relation is a
|
||||||
// legitimate write, and a required-field check is `ValidateFields`'s job.
|
// legitimate write, and a required-field check is `ValidateFields`'s job.
|
||||||
// - Exact-title resolution is U6. A ref or a UUID resolve; nothing else does.
|
// - A UUID, a ref, or an EXACT TITLE resolve; nothing else does. The title
|
||||||
|
// rung is U6 (PLAN-2857) and is scoped to the field's declared collection,
|
||||||
|
// unlike `ResolveItem`'s workspace-wide ladder. This line used to say
|
||||||
|
// "exact-title resolution is U6. A ref or a UUID resolve; nothing else
|
||||||
|
// does" — it was U1's scope note and U6 falsified it.
|
||||||
// - Soft-deleted targets do not resolve (`ResolveItem` and `GetItem` exclude
|
// - Soft-deleted targets do not resolve (`ResolveItem` and `GetItem` exclude
|
||||||
// them), so writing a reference to a deleted item is refused while an
|
// them), so writing a reference to a deleted item is refused while an
|
||||||
// ALREADY-STORED one still renders honestly on read — the read half U2
|
// ALREADY-STORED one still renders honestly on read — the read half U2
|
||||||
@@ -144,8 +183,9 @@ func (s *Store) ResolveRelationReferents(
|
|||||||
workspaceID string,
|
workspaceID string,
|
||||||
schema models.CollectionSchema,
|
schema models.CollectionSchema,
|
||||||
fieldMap map[string]any,
|
fieldMap map[string]any,
|
||||||
|
canSee RelationVisibilityFunc,
|
||||||
) ([]RelationIssue, error) {
|
) ([]RelationIssue, error) {
|
||||||
return s.ResolveRelationReferentsQ(s.Q(), workspaceID, schema, fieldMap)
|
return s.ResolveRelationReferentsQ(s.Q(), workspaceID, schema, fieldMap, canSee)
|
||||||
}
|
}
|
||||||
|
|
||||||
// ResolveRelationReferentsQ is ResolveRelationReferents parameterised over its
|
// ResolveRelationReferentsQ is ResolveRelationReferents parameterised over its
|
||||||
@@ -163,6 +203,7 @@ func (s *Store) ResolveRelationReferentsQ(
|
|||||||
workspaceID string,
|
workspaceID string,
|
||||||
schema models.CollectionSchema,
|
schema models.CollectionSchema,
|
||||||
fieldMap map[string]any,
|
fieldMap map[string]any,
|
||||||
|
canSee RelationVisibilityFunc,
|
||||||
) ([]RelationIssue, error) {
|
) ([]RelationIssue, error) {
|
||||||
var issues []RelationIssue
|
var issues []RelationIssue
|
||||||
// Cache per target slug: a schema with several relations aimed at one
|
// Cache per target slug: a schema with several relations aimed at one
|
||||||
@@ -222,12 +263,52 @@ func (s *Store) ResolveRelationReferentsQ(
|
|||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
if item == nil {
|
if item == nil {
|
||||||
issues = append(issues, RelationIssue{
|
// U6: the value is neither a UUID nor a ref, so try it as an exact
|
||||||
Key: def.Key, Value: value, Target: def.Collection, Reason: RelationTargetNotFound,
|
// TITLE — scoped to the declared collection, which is the whole
|
||||||
})
|
// distinction from ResolveItem's workspace-wide ladder (R11).
|
||||||
continue
|
titled, outside, ambiguous, terr := s.resolveRelationTitleQ(q, workspaceID, targetID, value, canSee)
|
||||||
|
if terr != nil {
|
||||||
|
return nil, terr
|
||||||
|
}
|
||||||
|
switch {
|
||||||
|
case ambiguous:
|
||||||
|
issues = append(issues, RelationIssue{
|
||||||
|
Key: def.Key, Value: value, Target: def.Collection,
|
||||||
|
Reason: RelationTargetAmbiguous,
|
||||||
|
})
|
||||||
|
continue
|
||||||
|
case titled != nil:
|
||||||
|
// Resolved by title. Falls through to the canonicalisation
|
||||||
|
// below by taking the same shape a ref would have.
|
||||||
|
item = titled
|
||||||
|
case outside != nil:
|
||||||
|
// The title names a live item the requester CAN see, just not
|
||||||
|
// one in this collection — the probe above already applied
|
||||||
|
// visibility, so an invisible match never reaches here.
|
||||||
|
// VisibilityChecked tells the server's collapse to leave this
|
||||||
|
// alone: that pass re-resolves through a UUID-or-ref ladder
|
||||||
|
// which cannot speak titles, so it would find nothing and
|
||||||
|
// collapse a reason the caller is entitled to see.
|
||||||
|
issues = append(issues, RelationIssue{
|
||||||
|
Key: def.Key, Value: value, Target: def.Collection,
|
||||||
|
Reason: RelationTargetWrongCollection, VisibilityChecked: canSee != nil,
|
||||||
|
})
|
||||||
|
continue
|
||||||
|
default:
|
||||||
|
issues = append(issues, RelationIssue{
|
||||||
|
Key: def.Key, Value: value, Target: def.Collection, Reason: RelationTargetNotFound,
|
||||||
|
})
|
||||||
|
continue
|
||||||
|
}
|
||||||
}
|
}
|
||||||
if item.CollectionID != targetID {
|
if item.CollectionID != targetID {
|
||||||
|
// VisibilityChecked is deliberately NOT set here. A ref-derived
|
||||||
|
// issue is re-resolvable by the server through the same ladder that
|
||||||
|
// produced it, so it keeps its own collapse — judging it here would
|
||||||
|
// silently change the ref path's behaviour (no second lookup, no
|
||||||
|
// TOCTOU window) inside a unit whose scope says not to touch it.
|
||||||
|
// That window is real and is filed as BUG-3012 rather than fixed in
|
||||||
|
// passing.
|
||||||
issues = append(issues, RelationIssue{
|
issues = append(issues, RelationIssue{
|
||||||
Key: def.Key, Value: value, Target: def.Collection, Reason: RelationTargetWrongCollection,
|
Key: def.Key, Value: value, Target: def.Collection, Reason: RelationTargetWrongCollection,
|
||||||
})
|
})
|
||||||
@@ -324,6 +405,304 @@ func (s *Store) resolveRelationTargetQ(q Queryer, workspaceID, value string) (*m
|
|||||||
return item, nil
|
return item, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// resolveRelationTitleQ resolves an exact TITLE to an item, scoped to the
|
||||||
|
// declared target collection (PLAN-2857 U6, lead ruling day 55).
|
||||||
|
//
|
||||||
|
// It returns at most one of:
|
||||||
|
//
|
||||||
|
// - item — exactly one live item in `targetCollectionID` carries this
|
||||||
|
// title. This is the only case that resolves.
|
||||||
|
// - outside — nothing in the target collection carries it, but a live item
|
||||||
|
// elsewhere in the workspace does. Returned so the caller can
|
||||||
|
// refuse with `wrong_collection` AND record which item it was;
|
||||||
|
// it never resolves.
|
||||||
|
// - ambiguous — two or more live items in the TARGET collection carry it, so
|
||||||
|
// the title names no single referent.
|
||||||
|
//
|
||||||
|
// All three empty means nothing in the workspace carries the title.
|
||||||
|
//
|
||||||
|
// WHY THE SCOPE IS THE WHOLE POINT. `ResolveItem`'s ladder is workspace-wide
|
||||||
|
// and ends in a SLUG, so `--field color=red` would resolve a `red` item in any
|
||||||
|
// collection — R11 in the design pass, and the reason this is new code instead
|
||||||
|
// of a reuse. The workspace-wide query below exists ONLY to turn "not here"
|
||||||
|
// into a better refusal; it is never allowed to produce a resolution.
|
||||||
|
//
|
||||||
|
// EXACTNESS, and precisely what is exact about it. `=` on the STORED title,
|
||||||
|
// case-sensitive on both engines, with no folding and no trimming of the stored
|
||||||
|
// side: the alternative invents a matching rule nobody ratified and makes two
|
||||||
|
// visibly different titles collide.
|
||||||
|
//
|
||||||
|
// The SUPPLIED value is trimmed before it gets here, by the same TrimSpace the
|
||||||
|
// whole resolver applies to every relation value (a relation names a reference,
|
||||||
|
// not content, and a whitespace-only one has meant "no reference" since U1). So
|
||||||
|
// `" Red "` does match a stored `"Red"`, while a stored `" Red "` is reachable
|
||||||
|
// only by a value that trims to it. Worth stating, because "exact" on its own
|
||||||
|
// implies neither.
|
||||||
|
//
|
||||||
|
// The ORDER of the ladder above this matters too: UUID, then ref, then title.
|
||||||
|
// An item literally titled "TASK-5" is therefore unreachable by title while a
|
||||||
|
// ref TASK-5 resolves. That is the correct precedence — refs are the canonical
|
||||||
|
// form — but it is a real edge, so it is written down here rather than found.
|
||||||
|
func (s *Store) resolveRelationTitleQ(
|
||||||
|
q Queryer,
|
||||||
|
workspaceID, targetCollectionID, title string,
|
||||||
|
canSee RelationVisibilityFunc,
|
||||||
|
) (item *models.Item, outside *models.Item, ambiguous bool, err error) {
|
||||||
|
// THE COUNT THAT DECIDES IS THE VISIBLE COUNT (lead ruling, PLAN-2857 U6).
|
||||||
|
// Among live exact-title matches in the target collection, only ones the
|
||||||
|
// requester can SEE count: zero is not_found, exactly one resolves, two or
|
||||||
|
// more is ambiguous. A hidden match never changes the answer.
|
||||||
|
//
|
||||||
|
// canSee == nil means there is NO requester — import, and any migrate that
|
||||||
|
// runs outside a request — and then every live match counts, which is right
|
||||||
|
// because nobody is being told anything.
|
||||||
|
//
|
||||||
|
// This lives HERE rather than as a second pass in the server layer, and the
|
||||||
|
// difference is not tidiness. The cross-workspace copy runs the resolver
|
||||||
|
// inside its own transaction, where no server-layer pass can reach it, so a
|
||||||
|
// server-side re-decision left the copy door leaking and made preflight and
|
||||||
|
// copy disagree. `RelationVisibilityFunc` already existed for exactly this
|
||||||
|
// shape, and resolveRelationsForWrite's own comment named the gap:
|
||||||
|
// "Unifying the two families needs a caller-supplied visibility predicate
|
||||||
|
// on the store API."
|
||||||
|
//
|
||||||
|
// TWO COSTS, stated because they are real, and stated more carefully than
|
||||||
|
// the first version of this comment (codex rounds 4 and 5 — round 5 was
|
||||||
|
// right that both claims were overstated):
|
||||||
|
//
|
||||||
|
// The walk loads each candidate and asks canSee about it, so it is O(live
|
||||||
|
// items sharing this exact title) queries — in this collection here, and
|
||||||
|
// ANYWHERE IN THE WORKSPACE for the wrong_collection probe below, which is
|
||||||
|
// the larger set. Normally that set is ONE. It grows when many items share
|
||||||
|
// a title, and the walk stops at the SECOND VISIBLE match, so the shape
|
||||||
|
// that actually scans is many HIDDEN same-titled items.
|
||||||
|
//
|
||||||
|
// An earlier version of this comment claimed that shape requires write
|
||||||
|
// access to a collection the walker cannot read. That is FALSE: an
|
||||||
|
// item-level grant lets a caller write the destination while seeing only
|
||||||
|
// selected items in the target collection, which the fixtures in
|
||||||
|
// relation_titles_doors_test.go build directly. The honest bound is the
|
||||||
|
// page size on memory and early termination on the common cases, not an
|
||||||
|
// argument that the bad case is unreachable.
|
||||||
|
//
|
||||||
|
// The cursor is not snapshot-stable. Rows are ordered by `id` and the
|
||||||
|
// cursor only advances, so an UNCHANGED, still-matching row that existed
|
||||||
|
// when the walk started is visited. That is weaker than "every row that
|
||||||
|
// existed is visited", which is what this used to say: each page is its own
|
||||||
|
// snapshot, so a row deleted or retitled before its page is reached is
|
||||||
|
// skipped, and a later insert may be observed. None of that is a defect —
|
||||||
|
// it is inherent to any non-snapshot read, equally true of the LIMIT 2
|
||||||
|
// count this replaced, and a row that changed mid-request is not one the
|
||||||
|
// answer owed anything to.
|
||||||
|
//
|
||||||
|
// PAGED ON `id`, not item_number: item_number is NULLABLE (migration 006
|
||||||
|
// adds it with no constraint and nothing since makes it NOT NULL), and
|
||||||
|
// `item_number > ?` silently excludes every NULL row — so a legacy
|
||||||
|
// visible match could be skipped while the unpaged count still saw it, and
|
||||||
|
// the two disagreed. `id` is the primary key and cannot be null; the order
|
||||||
|
// is arbitrary but STABLE, which is all a cursor needs. Nothing observable
|
||||||
|
// depends on WHICH order, because the only case that resolves has exactly
|
||||||
|
// one visible match.
|
||||||
|
seen := 0
|
||||||
|
var only *models.Item
|
||||||
|
anyMatch := false
|
||||||
|
cursor := ""
|
||||||
|
for {
|
||||||
|
ids, cerr := s.relationTitleIDPageQ(q, workspaceID, targetCollectionID, title, cursor, relationTitlePageSize)
|
||||||
|
if cerr != nil {
|
||||||
|
return nil, nil, false, cerr
|
||||||
|
}
|
||||||
|
if len(ids) == 0 {
|
||||||
|
break
|
||||||
|
}
|
||||||
|
anyMatch = true
|
||||||
|
for _, id := range ids {
|
||||||
|
cursor = id
|
||||||
|
if canSee == nil {
|
||||||
|
// Load BEFORE counting. Counting first meant a row deleted
|
||||||
|
// between the id page and this fetch still counted, so one
|
||||||
|
// remaining live match could answer `ambiguous` (codex round
|
||||||
|
// 5). A vanished row is not a match.
|
||||||
|
found, gerr := s.GetItemQ(q, id)
|
||||||
|
if gerr != nil {
|
||||||
|
return nil, nil, false, gerr
|
||||||
|
}
|
||||||
|
if found == nil {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
seen++
|
||||||
|
if seen > 1 {
|
||||||
|
return nil, nil, true, nil
|
||||||
|
}
|
||||||
|
only = found
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
found, gerr := s.GetItemQ(q, id)
|
||||||
|
if gerr != nil {
|
||||||
|
return nil, nil, false, gerr
|
||||||
|
}
|
||||||
|
if found == nil {
|
||||||
|
// Vanished between the id read and this one; not a match.
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
ok, verr := canSee(q, workspaceID, found)
|
||||||
|
if verr != nil {
|
||||||
|
return nil, nil, false, verr
|
||||||
|
}
|
||||||
|
if !ok {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
seen++
|
||||||
|
if seen > 1 {
|
||||||
|
// Nothing later can change this, so the rest is never read.
|
||||||
|
return nil, nil, true, nil
|
||||||
|
}
|
||||||
|
only = found
|
||||||
|
}
|
||||||
|
if len(ids) < relationTitlePageSize {
|
||||||
|
break
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if seen == 1 {
|
||||||
|
// `only` can be nil if the row vanished between the two reads; the
|
||||||
|
// caller then refuses, which is the honest answer.
|
||||||
|
return only, nil, false, nil
|
||||||
|
}
|
||||||
|
if anyMatch {
|
||||||
|
// The title DOES match inside the declared collection — the requester
|
||||||
|
// just cannot see any of them. Answer not_found and do NOT fall
|
||||||
|
// through to the workspace-wide probe: that probe has no collection
|
||||||
|
// filter, so it would rediscover one of these very items and report
|
||||||
|
// `wrong_collection`, which is both untrue (it is in the right
|
||||||
|
// collection) and the disclosure this rule exists to prevent.
|
||||||
|
return nil, nil, false, nil
|
||||||
|
}
|
||||||
|
|
||||||
|
// Nothing in the target collection. One workspace-wide probe, for the
|
||||||
|
// refusal only.
|
||||||
|
//
|
||||||
|
// It walks for a VISIBLE match rather than taking an arbitrary one and
|
||||||
|
// filtering it. `LIMIT 1` plus a filter was the first shape and it broke
|
||||||
|
// the property in the other direction: if the row it happened to pick was
|
||||||
|
// hidden the caller got not_found, while a DIFFERENT live match they can
|
||||||
|
// see would have earned `wrong_collection` — so a hidden match changed the
|
||||||
|
// answer again (codex round 4).
|
||||||
|
other, perr := s.firstVisibleTitleMatchQ(q, workspaceID, title, canSee)
|
||||||
|
if perr != nil {
|
||||||
|
return nil, nil, false, perr
|
||||||
|
}
|
||||||
|
// A `wrong_collection` refusal names a LIVE item, so raising it for a
|
||||||
|
// target the requester cannot see is the existence oracle the server
|
||||||
|
// collapse closes for refs. Answering not_found here means the collapse has
|
||||||
|
// nothing left to do on the title path.
|
||||||
|
return nil, other, false, nil
|
||||||
|
}
|
||||||
|
|
||||||
|
// firstVisibleTitleMatchQ returns the first live item anywhere in the workspace
|
||||||
|
// with this exact title that the requester may SEE, or nil.
|
||||||
|
//
|
||||||
|
// canSee == nil means no requester, and then the first live match is the
|
||||||
|
// answer — nobody is being told anything.
|
||||||
|
func (s *Store) firstVisibleTitleMatchQ(
|
||||||
|
q Queryer,
|
||||||
|
workspaceID, title string,
|
||||||
|
canSee RelationVisibilityFunc,
|
||||||
|
) (*models.Item, error) {
|
||||||
|
cursor := ""
|
||||||
|
for {
|
||||||
|
rows, err := q.Query(s.q(`
|
||||||
|
SELECT id FROM items
|
||||||
|
WHERE workspace_id = ? AND title = ? AND deleted_at IS NULL AND id > ?
|
||||||
|
ORDER BY id
|
||||||
|
LIMIT ?
|
||||||
|
`), workspaceID, title, cursor, relationTitlePageSize)
|
||||||
|
if err != nil {
|
||||||
|
return nil, fmt.Errorf("relation title probe: %w", err)
|
||||||
|
}
|
||||||
|
var ids []string
|
||||||
|
for rows.Next() {
|
||||||
|
var id string
|
||||||
|
if scanErr := rows.Scan(&id); scanErr != nil {
|
||||||
|
rows.Close()
|
||||||
|
return nil, fmt.Errorf("relation title probe scan: %w", scanErr)
|
||||||
|
}
|
||||||
|
ids = append(ids, id)
|
||||||
|
}
|
||||||
|
if rowsErr := rows.Err(); rowsErr != nil {
|
||||||
|
rows.Close()
|
||||||
|
return nil, fmt.Errorf("relation title probe rows: %w", rowsErr)
|
||||||
|
}
|
||||||
|
rows.Close()
|
||||||
|
if len(ids) == 0 {
|
||||||
|
return nil, nil
|
||||||
|
}
|
||||||
|
for _, id := range ids {
|
||||||
|
cursor = id
|
||||||
|
item, gerr := s.GetItemQ(q, id)
|
||||||
|
if gerr != nil {
|
||||||
|
return nil, gerr
|
||||||
|
}
|
||||||
|
if item == nil {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
if canSee == nil {
|
||||||
|
return item, nil
|
||||||
|
}
|
||||||
|
ok, verr := canSee(q, workspaceID, item)
|
||||||
|
if verr != nil {
|
||||||
|
return nil, verr
|
||||||
|
}
|
||||||
|
if ok {
|
||||||
|
return item, nil
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if len(ids) < relationTitlePageSize {
|
||||||
|
return nil, nil
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// relationTitlePageSize bounds MEMORY for the walk above, not the answer: the
|
||||||
|
// loop stops at the second VISIBLE match or at the end of the rows, so no match
|
||||||
|
// count changes what a caller is told. A cap on the ANSWER was an earlier shape
|
||||||
|
// and it was an oracle in itself — "one visible plus cap hidden" read
|
||||||
|
// differently from "one visible plus cap+1 hidden".
|
||||||
|
const relationTitlePageSize = 100
|
||||||
|
|
||||||
|
// relationTitleIDPageQ returns one page of ids of live items with this exact
|
||||||
|
// title in this collection, ordered by id, starting after `afterID`.
|
||||||
|
//
|
||||||
|
// Ids only: the caller decides what it may look at before loading anything.
|
||||||
|
func (s *Store) relationTitleIDPageQ(
|
||||||
|
q Queryer,
|
||||||
|
workspaceID, collectionID, title, afterID string,
|
||||||
|
limit int,
|
||||||
|
) ([]string, error) {
|
||||||
|
rows, err := q.Query(s.q(`
|
||||||
|
SELECT id FROM items
|
||||||
|
WHERE workspace_id = ? AND collection_id = ? AND title = ?
|
||||||
|
AND deleted_at IS NULL AND id > ?
|
||||||
|
ORDER BY id
|
||||||
|
LIMIT ?
|
||||||
|
`), workspaceID, collectionID, title, afterID, limit)
|
||||||
|
if err != nil {
|
||||||
|
return nil, fmt.Errorf("relation title page: %w", err)
|
||||||
|
}
|
||||||
|
defer rows.Close()
|
||||||
|
var ids []string
|
||||||
|
for rows.Next() {
|
||||||
|
var id string
|
||||||
|
if scanErr := rows.Scan(&id); scanErr != nil {
|
||||||
|
return nil, fmt.Errorf("relation title page scan: %w", scanErr)
|
||||||
|
}
|
||||||
|
ids = append(ids, id)
|
||||||
|
}
|
||||||
|
if rowsErr := rows.Err(); rowsErr != nil {
|
||||||
|
return nil, fmt.Errorf("relation title page rows: %w", rowsErr)
|
||||||
|
}
|
||||||
|
return ids, nil
|
||||||
|
}
|
||||||
|
|
||||||
// collectionIDBySlugQ returns the collection's ID, or "" when the workspace
|
// collectionIDBySlugQ returns the collection's ID, or "" when the workspace
|
||||||
// has no live collection with that slug. ID only: the referent check compares
|
// has no live collection with that slug. ID only: the referent check compares
|
||||||
// `item.CollectionID`, and loading the whole model would pull in per-collection
|
// `item.CollectionID`, and loading the whole model would pull in per-collection
|
||||||
@@ -374,6 +753,14 @@ func (s *Store) itemByRefQ(q Queryer, workspaceID, prefix string, number int) (*
|
|||||||
return s.GetItemQ(q, id)
|
return s.GetItemQ(q, id)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// relationWant is one stored relation value plus the collection slug its field
|
||||||
|
// DECLARES, carried together so hydration can check the second against the
|
||||||
|
// first without re-walking the schemas.
|
||||||
|
type relationWant struct {
|
||||||
|
value string
|
||||||
|
targetSlug string
|
||||||
|
}
|
||||||
|
|
||||||
// RelationTargetNotPortable — a carried relation value that cannot cross a
|
// RelationTargetNotPortable — a carried relation value that cannot cross a
|
||||||
// workspace boundary. Not a defect in the value: it names a live item in the
|
// workspace boundary. Not a defect in the value: it names a live item in the
|
||||||
// SOURCE workspace, and PLAN-2857 v1 excludes cross-workspace relation
|
// SOURCE workspace, and PLAN-2857 v1 excludes cross-workspace relation
|
||||||
@@ -476,6 +863,7 @@ func (o RelationOrigin) Refuses() bool {
|
|||||||
// Mutates fieldMap: dropped keys are deleted, and values that survive are
|
// Mutates fieldMap: dropped keys are deleted, and values that survive are
|
||||||
// canonicalised to their target's ID.
|
// canonicalised to their target's ID.
|
||||||
func (s *Store) MigrateRelationReferents(
|
func (s *Store) MigrateRelationReferents(
|
||||||
|
canSee RelationVisibilityFunc,
|
||||||
workspaceID string,
|
workspaceID string,
|
||||||
schema models.CollectionSchema,
|
schema models.CollectionSchema,
|
||||||
fieldMap map[string]any,
|
fieldMap map[string]any,
|
||||||
@@ -483,13 +871,14 @@ func (s *Store) MigrateRelationReferents(
|
|||||||
sourceFields map[string]any,
|
sourceFields map[string]any,
|
||||||
mode RelationCarryMode,
|
mode RelationCarryMode,
|
||||||
) (refusals []RelationIssue, dropped []RelationIssue, err error) {
|
) (refusals []RelationIssue, dropped []RelationIssue, err error) {
|
||||||
return s.MigrateRelationReferentsQ(s.Q(), workspaceID, schema, fieldMap, supplied, sourceFields, mode)
|
return s.MigrateRelationReferentsQ(s.Q(), canSee, workspaceID, schema, fieldMap, supplied, sourceFields, mode)
|
||||||
}
|
}
|
||||||
|
|
||||||
// MigrateRelationReferentsQ is MigrateRelationReferents on a caller-supplied
|
// MigrateRelationReferentsQ is MigrateRelationReferents on a caller-supplied
|
||||||
// read executor — see ResolveRelationReferentsQ for why the copy door needs it.
|
// read executor — see ResolveRelationReferentsQ for why the copy door needs it.
|
||||||
func (s *Store) MigrateRelationReferentsQ(
|
func (s *Store) MigrateRelationReferentsQ(
|
||||||
q Queryer,
|
q Queryer,
|
||||||
|
canSee RelationVisibilityFunc,
|
||||||
workspaceID string,
|
workspaceID string,
|
||||||
schema models.CollectionSchema,
|
schema models.CollectionSchema,
|
||||||
fieldMap map[string]any,
|
fieldMap map[string]any,
|
||||||
@@ -530,7 +919,7 @@ func (s *Store) MigrateRelationReferentsQ(
|
|||||||
|
|
||||||
// Supplied values are ordinary writes.
|
// Supplied values are ordinary writes.
|
||||||
if suppliedRelations := byOrigin[RelationOriginSupplied]; len(suppliedRelations) > 0 {
|
if suppliedRelations := byOrigin[RelationOriginSupplied]; len(suppliedRelations) > 0 {
|
||||||
issues, resolveErr := s.ResolveRelationReferentsQ(q, workspaceID, schema, suppliedRelations)
|
issues, resolveErr := s.ResolveRelationReferentsQ(q, workspaceID, schema, suppliedRelations, canSee)
|
||||||
if resolveErr != nil {
|
if resolveErr != nil {
|
||||||
return nil, nil, resolveErr
|
return nil, nil, resolveErr
|
||||||
}
|
}
|
||||||
@@ -608,7 +997,7 @@ func (s *Store) MigrateRelationReferentsQ(
|
|||||||
delete(fieldMap, def.Key)
|
delete(fieldMap, def.Key)
|
||||||
delete(defaults, def.Key)
|
delete(defaults, def.Key)
|
||||||
}
|
}
|
||||||
issues, resolveErr := s.ResolveRelationReferentsQ(q, workspaceID, schema, defaults)
|
issues, resolveErr := s.ResolveRelationReferentsQ(q, workspaceID, schema, defaults, canSee)
|
||||||
if resolveErr != nil {
|
if resolveErr != nil {
|
||||||
return nil, nil, resolveErr
|
return nil, nil, resolveErr
|
||||||
}
|
}
|
||||||
@@ -729,7 +1118,21 @@ func (s *Store) MigrateRelationReferentsQ(
|
|||||||
// below, which exists to keep dangling referents out of the blob. So
|
// below, which exists to keep dangling referents out of the blob. So
|
||||||
// this comment is also a warning: if you ever change that rule, you
|
// this comment is also a warning: if you ever change that rule, you
|
||||||
// are changing this too, in the other direction.
|
// are changing this too, in the other direction.
|
||||||
issues, resolveErr := s.ResolveRelationReferentsQ(q, workspaceID, schema, carried)
|
// nil, NOT canSee, and the argument is the twenty lines above this
|
||||||
|
// one. A carried value was asserted by nobody, so no caller is probing
|
||||||
|
// with it; and judging it by the MOVER's visibility is precisely what
|
||||||
|
// "would make the STORED BYTES depend on who performed the move".
|
||||||
|
//
|
||||||
|
// U6 made that reachable for the first time. Before it a carried value
|
||||||
|
// that happened to match a title never resolved at all, so there was
|
||||||
|
// nothing for visibility to change. Passing the predicate here — which
|
||||||
|
// I did, against this file's own standing argument — meant a carried
|
||||||
|
// title resolved for one mover and was DROPPED for another, and the
|
||||||
|
// item's stored fields depended on who touched it (codex round 5).
|
||||||
|
//
|
||||||
|
// The disclosure the predicate exists to prevent needs a caller who
|
||||||
|
// TYPED the title. Supplied values and injected defaults still get it.
|
||||||
|
issues, resolveErr := s.ResolveRelationReferentsQ(q, workspaceID, schema, carried, nil)
|
||||||
if resolveErr != nil {
|
if resolveErr != nil {
|
||||||
return nil, nil, resolveErr
|
return nil, nil, resolveErr
|
||||||
}
|
}
|
||||||
@@ -789,18 +1192,20 @@ func RelationKeysPresent(schema models.CollectionSchema, fieldMap map[string]any
|
|||||||
// MigrateRelationReferents gives RelationOriginDestinationDefault, which is
|
// MigrateRelationReferents gives RelationOriginDestinationDefault, which is
|
||||||
// what this is: the same origin, arriving late.
|
// what this is: the same origin, arriving late.
|
||||||
func (s *Store) ResolveLateRelationDefaults(
|
func (s *Store) ResolveLateRelationDefaults(
|
||||||
|
canSee RelationVisibilityFunc,
|
||||||
workspaceID string,
|
workspaceID string,
|
||||||
schema models.CollectionSchema,
|
schema models.CollectionSchema,
|
||||||
fieldMap map[string]any,
|
fieldMap map[string]any,
|
||||||
before map[string]bool,
|
before map[string]bool,
|
||||||
) (dropped []RelationIssue, err error) {
|
) (dropped []RelationIssue, err error) {
|
||||||
return s.ResolveLateRelationDefaultsQ(s.Q(), workspaceID, schema, fieldMap, before)
|
return s.ResolveLateRelationDefaultsQ(s.Q(), canSee, workspaceID, schema, fieldMap, before)
|
||||||
}
|
}
|
||||||
|
|
||||||
// ResolveLateRelationDefaultsQ is ResolveLateRelationDefaults on a
|
// ResolveLateRelationDefaultsQ is ResolveLateRelationDefaults on a
|
||||||
// caller-supplied read executor — the copy door runs inside a transaction.
|
// caller-supplied read executor — the copy door runs inside a transaction.
|
||||||
func (s *Store) ResolveLateRelationDefaultsQ(
|
func (s *Store) ResolveLateRelationDefaultsQ(
|
||||||
q Queryer,
|
q Queryer,
|
||||||
|
canSee RelationVisibilityFunc,
|
||||||
workspaceID string,
|
workspaceID string,
|
||||||
schema models.CollectionSchema,
|
schema models.CollectionSchema,
|
||||||
fieldMap map[string]any,
|
fieldMap map[string]any,
|
||||||
@@ -849,7 +1254,7 @@ func (s *Store) ResolveLateRelationDefaultsQ(
|
|||||||
// silently discarded them.
|
// silently discarded them.
|
||||||
return dropped, nil
|
return dropped, nil
|
||||||
}
|
}
|
||||||
issues, resolveErr := s.ResolveRelationReferentsQ(q, workspaceID, schema, late)
|
issues, resolveErr := s.ResolveRelationReferentsQ(q, workspaceID, schema, late, canSee)
|
||||||
if resolveErr != nil {
|
if resolveErr != nil {
|
||||||
return nil, resolveErr
|
return nil, resolveErr
|
||||||
}
|
}
|
||||||
@@ -1034,3 +1439,182 @@ func (s *Store) DropInvisibleRelationDefaultsQ(
|
|||||||
}
|
}
|
||||||
return dropped, nil
|
return dropped, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// HydrateRelationTargetsQ resolves the stored `relation` values on a batch of
|
||||||
|
// items into {id, ref, title} entries (PLAN-2857 U6).
|
||||||
|
//
|
||||||
|
// BATCHED ON PURPOSE. A list read of 50 items with two relation fields each is
|
||||||
|
// 100 values; resolving them one at a time is 100 queries behind one page. This
|
||||||
|
// collects every distinct id across every item first and asks once.
|
||||||
|
//
|
||||||
|
// It answers only what the ITEMS say. Visibility is not its business — the same
|
||||||
|
// division the resolver documents at the top of this file — so a caller that
|
||||||
|
// serves a request filters the result before sending it. What it does do is
|
||||||
|
// leave an unresolvable value as an ID-ONLY entry rather than dropping it,
|
||||||
|
// because dropping it says "no relation here" and that is false.
|
||||||
|
//
|
||||||
|
// `schemas` maps a collection ID to that collection's schema, since a batch can
|
||||||
|
// span collections; an item whose collection is absent from the map is skipped
|
||||||
|
// rather than guessed at.
|
||||||
|
//
|
||||||
|
// SCOPED TO ONE WORKSPACE, and that is load-bearing rather than tidy. A stored
|
||||||
|
// relation value is just an id in a blob: nothing stops a legacy row, or a
|
||||||
|
// value that predates U1's validation, from naming an item in ANOTHER
|
||||||
|
// workspace. Resolving it would put that item's ref and title in a response to
|
||||||
|
// someone who cannot see its workspace at all. The resolver guards the same
|
||||||
|
// hole on its UUID branch and says so; this is the read-side half. A foreign id
|
||||||
|
// falls through to the ID-only path below, which is the honest answer: the
|
||||||
|
// value is stored, and there is nothing here to tell the caller about it.
|
||||||
|
func (s *Store) HydrateRelationTargetsQ(
|
||||||
|
q Queryer,
|
||||||
|
workspaceID string,
|
||||||
|
items []models.Item,
|
||||||
|
schemas map[string]models.CollectionSchema,
|
||||||
|
) (map[string]map[string]models.RelationTarget, error) {
|
||||||
|
// item ID -> field key -> the stored value and the collection its field
|
||||||
|
// DECLARES as the target.
|
||||||
|
perItem := map[string]map[string]relationWant{}
|
||||||
|
wanted := map[string]struct{}{}
|
||||||
|
|
||||||
|
for i := range items {
|
||||||
|
schema, ok := schemas[items[i].CollectionID]
|
||||||
|
if !ok {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
fieldMap := map[string]any{}
|
||||||
|
if items[i].Fields != "" {
|
||||||
|
if err := json.Unmarshal([]byte(items[i].Fields), &fieldMap); err != nil {
|
||||||
|
// A blob that will not parse is a different defect and other
|
||||||
|
// code reports it; hydration declines to be the second voice.
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
}
|
||||||
|
for _, def := range schema.Fields {
|
||||||
|
if def.Type != "relation" {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
raw, exists := fieldMap[def.Key]
|
||||||
|
if !exists || raw == nil {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
value, isStr := raw.(string)
|
||||||
|
if !isStr {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
value = strings.TrimSpace(value)
|
||||||
|
if value == "" {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
if perItem[items[i].ID] == nil {
|
||||||
|
perItem[items[i].ID] = map[string]relationWant{}
|
||||||
|
}
|
||||||
|
perItem[items[i].ID][def.Key] = relationWant{value: value, targetSlug: def.Collection}
|
||||||
|
wanted[value] = struct{}{}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if len(wanted) == 0 {
|
||||||
|
return nil, nil
|
||||||
|
}
|
||||||
|
|
||||||
|
ids := make([]string, 0, len(wanted))
|
||||||
|
for id := range wanted {
|
||||||
|
ids = append(ids, id)
|
||||||
|
}
|
||||||
|
// Deterministic order so a query plan and any test on it are stable.
|
||||||
|
sort.Strings(ids)
|
||||||
|
|
||||||
|
resolved := map[string]models.RelationTarget{}
|
||||||
|
// The collection each resolved target actually lives in, for the
|
||||||
|
// declared-collection check below.
|
||||||
|
resolvedIn := map[string]string{}
|
||||||
|
// Chunked: some drivers cap placeholders per statement, and a workspace
|
||||||
|
// with a long list read can exceed it.
|
||||||
|
const chunk = 200
|
||||||
|
for start := 0; start < len(ids); start += chunk {
|
||||||
|
end := start + chunk
|
||||||
|
if end > len(ids) {
|
||||||
|
end = len(ids)
|
||||||
|
}
|
||||||
|
batch := ids[start:end]
|
||||||
|
placeholders := strings.TrimSuffix(strings.Repeat("?,", len(batch)), ",")
|
||||||
|
args := make([]any, 0, len(batch))
|
||||||
|
for _, id := range batch {
|
||||||
|
args = append(args, id)
|
||||||
|
}
|
||||||
|
rows, err := q.Query(s.q(`
|
||||||
|
SELECT i.id, i.title, i.item_number, i.collection_id, c.prefix
|
||||||
|
FROM items i
|
||||||
|
JOIN collections c ON c.id = i.collection_id
|
||||||
|
WHERE i.workspace_id = ? AND i.id IN (`+placeholders+`) AND i.deleted_at IS NULL
|
||||||
|
`), append([]any{workspaceID}, args...)...)
|
||||||
|
if err != nil {
|
||||||
|
return nil, fmt.Errorf("hydrate relation targets: %w", err)
|
||||||
|
}
|
||||||
|
for rows.Next() {
|
||||||
|
var id, title, collectionID, prefix string
|
||||||
|
// NULLABLE. item_number is added by migration 006 with no
|
||||||
|
// constraint and nothing since makes it NOT NULL, so a pre-006 row
|
||||||
|
// carries NULL — and scanning NULL into an int ERRORS. This
|
||||||
|
// function is best-effort and its caller drops the whole map on
|
||||||
|
// error, so one legacy row would have silently removed
|
||||||
|
// `relation_targets` from the ENTIRE response (codex round 7).
|
||||||
|
var number sql.NullInt64
|
||||||
|
if err := rows.Scan(&id, &title, &number, &collectionID, &prefix); err != nil {
|
||||||
|
rows.Close()
|
||||||
|
return nil, fmt.Errorf("hydrate relation targets scan: %w", err)
|
||||||
|
}
|
||||||
|
target := models.RelationTarget{ID: id, Title: title}
|
||||||
|
if number.Valid {
|
||||||
|
target.Ref = fmt.Sprintf("%s-%d", prefix, number.Int64)
|
||||||
|
}
|
||||||
|
resolvedIn[id] = collectionID
|
||||||
|
resolved[id] = target
|
||||||
|
}
|
||||||
|
if err := rows.Err(); err != nil {
|
||||||
|
rows.Close()
|
||||||
|
return nil, fmt.Errorf("hydrate relation targets rows: %w", err)
|
||||||
|
}
|
||||||
|
rows.Close()
|
||||||
|
}
|
||||||
|
|
||||||
|
// Declared target slug -> collection id, resolved once per distinct slug.
|
||||||
|
declaredIDs := map[string]string{}
|
||||||
|
|
||||||
|
out := map[string]map[string]models.RelationTarget{}
|
||||||
|
for itemID, fields := range perItem {
|
||||||
|
for key, want := range fields {
|
||||||
|
value := want.value
|
||||||
|
target, ok := resolved[value]
|
||||||
|
if !ok {
|
||||||
|
// Dangling: the stored value names nothing live. ID-only, so
|
||||||
|
// the read stays honest instead of erroring or pretending.
|
||||||
|
target = models.RelationTarget{ID: value}
|
||||||
|
} else if want.targetSlug != "" {
|
||||||
|
// THE VALUE MUST LIVE WHERE THE FIELD SAYS IT DOES. The write
|
||||||
|
// side refuses a relation pointing outside the declared
|
||||||
|
// collection, but legacy and corrupt rows predate that, and
|
||||||
|
// rendering one as `{ref, title}` presents a Task as though it
|
||||||
|
// were the Colour the field promised. ID-only is the honest
|
||||||
|
// shape — the same one a dangling value gets, for the same
|
||||||
|
// reason (codex round 7).
|
||||||
|
declaredID, known := declaredIDs[want.targetSlug]
|
||||||
|
if !known {
|
||||||
|
var derr error
|
||||||
|
declaredID, derr = s.collectionIDBySlugQ(q, workspaceID, want.targetSlug)
|
||||||
|
if derr != nil {
|
||||||
|
return nil, derr
|
||||||
|
}
|
||||||
|
declaredIDs[want.targetSlug] = declaredID
|
||||||
|
}
|
||||||
|
if declaredID == "" || resolvedIn[value] != declaredID {
|
||||||
|
target = models.RelationTarget{ID: value}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if out[itemID] == nil {
|
||||||
|
out[itemID] = map[string]models.RelationTarget{}
|
||||||
|
}
|
||||||
|
out[itemID][key] = target
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return out, nil
|
||||||
|
}
|
||||||
|
|||||||
@@ -58,7 +58,7 @@ func TestResolveRelationReferents_AcceptsIDAndRefAndCanonicalises(t *testing.T)
|
|||||||
// leg a resolver that refused everything would pass the whole suite.
|
// leg a resolver that refused everything would pass the whole suite.
|
||||||
for _, supplied := range []string{red.ID, red.Ref} {
|
for _, supplied := range []string{red.ID, red.Ref} {
|
||||||
fields := map[string]any{"color": supplied}
|
fields := map[string]any{"color": supplied}
|
||||||
issues, err := s.ResolveRelationReferents(ws.ID, u1RelationSchema("colors"), fields)
|
issues, err := s.ResolveRelationReferents(ws.ID, u1RelationSchema("colors"), fields, nil)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("supplied %q: %v", supplied, err)
|
t.Fatalf("supplied %q: %v", supplied, err)
|
||||||
}
|
}
|
||||||
@@ -109,7 +109,7 @@ func TestResolveRelationReferents_RejectsUnresolvableValues(t *testing.T) {
|
|||||||
for _, tc := range cases {
|
for _, tc := range cases {
|
||||||
t.Run(tc.name, func(t *testing.T) {
|
t.Run(tc.name, func(t *testing.T) {
|
||||||
fields := map[string]any{"color": tc.value}
|
fields := map[string]any{"color": tc.value}
|
||||||
issues, err := s.ResolveRelationReferents(ws.ID, u1RelationSchema("colors"), fields)
|
issues, err := s.ResolveRelationReferents(ws.ID, u1RelationSchema("colors"), fields, nil)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("unexpected error: %v", err)
|
t.Fatalf("unexpected error: %v", err)
|
||||||
}
|
}
|
||||||
@@ -158,7 +158,7 @@ func TestResolveRelationReferents_LeavesNonWritesAlone(t *testing.T) {
|
|||||||
case "explicit null":
|
case "explicit null":
|
||||||
fields["color"] = nil
|
fields["color"] = nil
|
||||||
}
|
}
|
||||||
issues, err := s.ResolveRelationReferents(ws.ID, u1RelationSchema("colors"), fields)
|
issues, err := s.ResolveRelationReferents(ws.ID, u1RelationSchema("colors"), fields, nil)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("unexpected error: %v", err)
|
t.Fatalf("unexpected error: %v", err)
|
||||||
}
|
}
|
||||||
@@ -183,7 +183,7 @@ func TestResolveRelationReferents_SoftDeletedTargetDoesNotResolve(t *testing.T)
|
|||||||
// two halves are what make "deleted" distinguishable from "never resolved"
|
// two halves are what make "deleted" distinguishable from "never resolved"
|
||||||
// in the UI, so this must not quietly become not_found's twin.
|
// in the UI, so this must not quietly become not_found's twin.
|
||||||
fields := map[string]any{"color": gone.ID}
|
fields := map[string]any{"color": gone.ID}
|
||||||
issues, err := s.ResolveRelationReferents(ws.ID, u1RelationSchema("colors"), fields)
|
issues, err := s.ResolveRelationReferents(ws.ID, u1RelationSchema("colors"), fields, nil)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("unexpected error: %v", err)
|
t.Fatalf("unexpected error: %v", err)
|
||||||
}
|
}
|
||||||
@@ -201,7 +201,7 @@ func TestResolveRelationReferents_TargetCollectionProblems(t *testing.T) {
|
|||||||
// A relation field with no target cannot be checked against anything.
|
// A relation field with no target cannot be checked against anything.
|
||||||
// Surfaced rather than treated as permission to store whatever.
|
// Surfaced rather than treated as permission to store whatever.
|
||||||
fields := map[string]any{"color": red.ID}
|
fields := map[string]any{"color": red.ID}
|
||||||
issues, err := s.ResolveRelationReferents(ws.ID, u1RelationSchema(""), fields)
|
issues, err := s.ResolveRelationReferents(ws.ID, u1RelationSchema(""), fields, nil)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("unexpected error: %v", err)
|
t.Fatalf("unexpected error: %v", err)
|
||||||
}
|
}
|
||||||
@@ -212,7 +212,7 @@ func TestResolveRelationReferents_TargetCollectionProblems(t *testing.T) {
|
|||||||
|
|
||||||
t.Run("target names no collection", func(t *testing.T) {
|
t.Run("target names no collection", func(t *testing.T) {
|
||||||
fields := map[string]any{"color": red.ID}
|
fields := map[string]any{"color": red.ID}
|
||||||
issues, err := s.ResolveRelationReferents(ws.ID, u1RelationSchema("nonexistent"), fields)
|
issues, err := s.ResolveRelationReferents(ws.ID, u1RelationSchema("nonexistent"), fields, nil)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("unexpected error: %v", err)
|
t.Fatalf("unexpected error: %v", err)
|
||||||
}
|
}
|
||||||
@@ -238,7 +238,7 @@ func TestResolveRelationReferents_IsDeterministicAndBatched(t *testing.T) {
|
|||||||
var first string
|
var first string
|
||||||
for i := 0; i < 8; i++ {
|
for i := 0; i < 8; i++ {
|
||||||
fields := map[string]any{"accent": "nope-a", "color": "nope-b"}
|
fields := map[string]any{"accent": "nope-a", "color": "nope-b"}
|
||||||
issues, err := s.ResolveRelationReferents(ws.ID, schema, fields)
|
issues, err := s.ResolveRelationReferents(ws.ID, schema, fields, nil)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("unexpected error: %v", err)
|
t.Fatalf("unexpected error: %v", err)
|
||||||
}
|
}
|
||||||
@@ -270,7 +270,7 @@ func TestMigrateRelationReferents_SameWorkspaceKeepsWhatResolves(t *testing.T) {
|
|||||||
// relation must survive. Dropping it would lose data on every move of a
|
// relation must survive. Dropping it would lose data on every move of a
|
||||||
// correctly-related item.
|
// correctly-related item.
|
||||||
fields := map[string]any{"color": red.Ref, "status": "open"}
|
fields := map[string]any{"color": red.Ref, "status": "open"}
|
||||||
refusals, dropped, err := s.MigrateRelationReferents(ws.ID, u1RelationSchema("colors"), fields, nil, carriedFrom(fields), RelationCarryWithinWorkspace)
|
refusals, dropped, err := s.MigrateRelationReferents(nil, ws.ID, u1RelationSchema("colors"), fields, nil, carriedFrom(fields), RelationCarryWithinWorkspace)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("unexpected error: %v", err)
|
t.Fatalf("unexpected error: %v", err)
|
||||||
}
|
}
|
||||||
@@ -291,7 +291,7 @@ func TestMigrateRelationReferents_SameWorkspaceDropsWhatDoesNot(t *testing.T) {
|
|||||||
// accepted any string for a relation all along, so an item carrying "red"
|
// accepted any string for a relation all along, so an item carrying "red"
|
||||||
// must stay MOVABLE. Dropped and reported, never refused.
|
// must stay MOVABLE. Dropped and reported, never refused.
|
||||||
fields := map[string]any{"color": "red", "status": "open"}
|
fields := map[string]any{"color": "red", "status": "open"}
|
||||||
refusals, dropped, err := s.MigrateRelationReferents(ws.ID, u1RelationSchema("colors"), fields, nil, carriedFrom(fields), RelationCarryWithinWorkspace)
|
refusals, dropped, err := s.MigrateRelationReferents(nil, ws.ID, u1RelationSchema("colors"), fields, nil, carriedFrom(fields), RelationCarryWithinWorkspace)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("unexpected error: %v", err)
|
t.Fatalf("unexpected error: %v", err)
|
||||||
}
|
}
|
||||||
@@ -319,7 +319,7 @@ func TestMigrateRelationReferents_CrossWorkspaceDropsEveryCarriedRelation(t *tes
|
|||||||
// so there is nothing in the destination it could mean. Same reason
|
// so there is nothing in the destination it could mean. Same reason
|
||||||
// github_pr uses, because it is the same fact about the same kind of value.
|
// github_pr uses, because it is the same fact about the same kind of value.
|
||||||
fields := map[string]any{"color": red.ID, "status": "open"}
|
fields := map[string]any{"color": red.ID, "status": "open"}
|
||||||
refusals, dropped, err := s.MigrateRelationReferents(ws.ID, u1RelationSchema("colors"), fields, nil, carriedFrom(fields), RelationCarryCrossWorkspace)
|
refusals, dropped, err := s.MigrateRelationReferents(nil, ws.ID, u1RelationSchema("colors"), fields, nil, carriedFrom(fields), RelationCarryCrossWorkspace)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("unexpected error: %v", err)
|
t.Fatalf("unexpected error: %v", err)
|
||||||
}
|
}
|
||||||
@@ -345,7 +345,7 @@ func TestMigrateRelationReferents_SuppliedOverrideRefusesOnEitherMode(t *testing
|
|||||||
for _, mode := range []RelationCarryMode{RelationCarryWithinWorkspace, RelationCarryCrossWorkspace} {
|
for _, mode := range []RelationCarryMode{RelationCarryWithinWorkspace, RelationCarryCrossWorkspace} {
|
||||||
fields := map[string]any{"color": "nope"}
|
fields := map[string]any{"color": "nope"}
|
||||||
supplied := map[string]any{"color": "nope"}
|
supplied := map[string]any{"color": "nope"}
|
||||||
refusals, dropped, err := s.MigrateRelationReferents(ws.ID, u1RelationSchema("colors"), fields, supplied, carriedFrom(fields), mode)
|
refusals, dropped, err := s.MigrateRelationReferents(nil, ws.ID, u1RelationSchema("colors"), fields, supplied, carriedFrom(fields), mode)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("mode %v: %v", mode, err)
|
t.Fatalf("mode %v: %v", mode, err)
|
||||||
}
|
}
|
||||||
@@ -366,7 +366,7 @@ func TestMigrateRelationReferents_SuppliedOverrideRefusesOnEitherMode(t *testing
|
|||||||
// on a copy at all.
|
// on a copy at all.
|
||||||
fields := map[string]any{"color": red.ID}
|
fields := map[string]any{"color": red.ID}
|
||||||
supplied := map[string]any{"color": red.ID}
|
supplied := map[string]any{"color": red.ID}
|
||||||
refusals, dropped, err := s.MigrateRelationReferents(ws.ID, u1RelationSchema("colors"), fields, supplied, carriedFrom(fields), RelationCarryCrossWorkspace)
|
refusals, dropped, err := s.MigrateRelationReferents(nil, ws.ID, u1RelationSchema("colors"), fields, supplied, carriedFrom(fields), RelationCarryCrossWorkspace)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("unexpected error: %v", err)
|
t.Fatalf("unexpected error: %v", err)
|
||||||
}
|
}
|
||||||
@@ -407,12 +407,12 @@ func TestResolveRelationReferents_OverflowingRefDoesNotResolve(t *testing.T) {
|
|||||||
// Sanity: the honest ref for that item still resolves, so a failure below
|
// Sanity: the honest ref for that item still resolves, so a failure below
|
||||||
// is about the overflow and not about the fixture.
|
// is about the overflow and not about the fixture.
|
||||||
ok := map[string]any{"color": red.Ref}
|
ok := map[string]any{"color": red.Ref}
|
||||||
if issues, err := s.ResolveRelationReferents(ws.ID, u1RelationSchema("colors"), ok); err != nil || len(issues) != 0 {
|
if issues, err := s.ResolveRelationReferents(ws.ID, u1RelationSchema("colors"), ok, nil); err != nil || len(issues) != 0 {
|
||||||
t.Fatalf("control: %v %+v", err, issues)
|
t.Fatalf("control: %v %+v", err, issues)
|
||||||
}
|
}
|
||||||
|
|
||||||
overflowed := map[string]any{"color": "COLO-18446744073709551617"}
|
overflowed := map[string]any{"color": "COLO-18446744073709551617"}
|
||||||
issues, err := s.ResolveRelationReferents(ws.ID, u1RelationSchema("colors"), overflowed)
|
issues, err := s.ResolveRelationReferents(ws.ID, u1RelationSchema("colors"), overflowed, nil)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("unexpected error: %v", err)
|
t.Fatalf("unexpected error: %v", err)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1,480 @@
|
|||||||
|
package store
|
||||||
|
|
||||||
|
import (
|
||||||
|
"encoding/json"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"github.com/PerpetualSoftware/pad/internal/models"
|
||||||
|
)
|
||||||
|
|
||||||
|
// Exact-title resolution and read-side hydration for `relation` values
|
||||||
|
// (PLAN-2857 U6).
|
||||||
|
//
|
||||||
|
// U1 accepted a UUID or a ref. U6 adds a THIRD spelling — an exact title,
|
||||||
|
// scoped to the field's declared collection — and the read side that lets a
|
||||||
|
// client render any of them. The legs below are the design row's proving test
|
||||||
|
// at the store level; the door-level equivalents live beside the U1 door tests.
|
||||||
|
//
|
||||||
|
// Every negative leg here is paired with a positive one that would break if the
|
||||||
|
// resolver simply refused everything (CONVE-34). That is not decoration: the
|
||||||
|
// first draft of a "title in the wrong collection must refuse" test passes
|
||||||
|
// against a resolver with no title support at all.
|
||||||
|
|
||||||
|
// resolveOne runs one supplied value through the resolver and returns the
|
||||||
|
// stored result plus any issues, so a leg reads as value-in / outcome-out.
|
||||||
|
func resolveOne(t *testing.T, s *Store, ws *models.Workspace, schema models.CollectionSchema, supplied string) (string, []RelationIssue) {
|
||||||
|
t.Helper()
|
||||||
|
fields := map[string]any{"color": supplied}
|
||||||
|
issues, err := s.ResolveRelationReferents(ws.ID, schema, fields, nil)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("resolve %q: %v", supplied, err)
|
||||||
|
}
|
||||||
|
stored, _ := fields["color"].(string)
|
||||||
|
return stored, issues
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestRelationTitle_ThreeSpellingsOfOneItemStoreTheSameID is the design row's
|
||||||
|
// proving test: the same relation written by ref, by exact title and by raw
|
||||||
|
// UUID must land on an identical stored ID.
|
||||||
|
//
|
||||||
|
// It is also the CONTROL for every refusal leg below. A resolver that refused
|
||||||
|
// every title would pass those legs and fail only this one.
|
||||||
|
func TestRelationTitle_ThreeSpellingsOfOneItemStoreTheSameID(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
s := testStore(t)
|
||||||
|
ws, _, _, red := relationFixture(t, s)
|
||||||
|
schema := u1RelationSchema("colors")
|
||||||
|
|
||||||
|
for _, supplied := range []string{red.ID, red.Ref, red.Title} {
|
||||||
|
stored, issues := resolveOne(t, s, ws, schema, supplied)
|
||||||
|
if len(issues) != 0 {
|
||||||
|
t.Fatalf("%q: unexpected issues %v", supplied, issues)
|
||||||
|
}
|
||||||
|
if stored != red.ID {
|
||||||
|
t.Errorf("%q stored %q, want the canonical id %q — three spellings of one item must not store three different values", supplied, stored, red.ID)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestRelationTitle_ScopeIsTheDeclaredCollection is ruling (3)'s store half and
|
||||||
|
// the R11 guard: a title unique in the WORKSPACE but living outside the
|
||||||
|
// field's declared collection must refuse, not resolve.
|
||||||
|
//
|
||||||
|
// The in-collection control runs in the same test so a uniformly-refusing
|
||||||
|
// resolver cannot pass it.
|
||||||
|
func TestRelationTitle_ScopeIsTheDeclaredCollection(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
s := testStore(t)
|
||||||
|
ws, _, cars, _ := relationFixture(t, s)
|
||||||
|
schema := u1RelationSchema("colors")
|
||||||
|
|
||||||
|
// "Sedan" exists, is unique workspace-wide, and is in the WRONG collection.
|
||||||
|
sedan := createTestItem(t, s, ws.ID, cars.ID, "Sedan", "")
|
||||||
|
|
||||||
|
stored, issues := resolveOne(t, s, ws, schema, "Sedan")
|
||||||
|
if len(issues) != 1 {
|
||||||
|
t.Fatalf("a title outside the declared collection must produce exactly one issue, got %d: %v", len(issues), issues)
|
||||||
|
}
|
||||||
|
if issues[0].Reason != RelationTargetWrongCollection {
|
||||||
|
t.Errorf("reason = %q, want %q — resolving it workspace-wide is the R11 defect this guards", issues[0].Reason, RelationTargetWrongCollection)
|
||||||
|
}
|
||||||
|
if stored == sedan.ID {
|
||||||
|
t.Error("the out-of-collection item was stored anyway")
|
||||||
|
}
|
||||||
|
|
||||||
|
// CONTROL, same run: a title that IS in the declared collection resolves.
|
||||||
|
// Without this leg, a resolver with no title support passes the assertions
|
||||||
|
// above by refusing everything.
|
||||||
|
blue := createTestItem(t, s, ws.ID, mustCollectionID(t, s, ws.ID, "colors"), "Blue", "")
|
||||||
|
stored, issues = resolveOne(t, s, ws, schema, "Blue")
|
||||||
|
if len(issues) != 0 {
|
||||||
|
t.Fatalf("control: an in-collection title must resolve, got issues %v", issues)
|
||||||
|
}
|
||||||
|
if stored != blue.ID {
|
||||||
|
t.Errorf("control: stored %q, want %q", stored, blue.ID)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestRelationTitle_AmbiguousRefusesWithItsOwnReason covers ruling (2): two
|
||||||
|
// live items in the TARGET collection sharing a title name no single one.
|
||||||
|
//
|
||||||
|
// It must NOT collapse into not_found, which would tell the caller the opposite
|
||||||
|
// of what happened — the title matched too much, not too little.
|
||||||
|
func TestRelationTitle_AmbiguousRefusesWithItsOwnReason(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
s := testStore(t)
|
||||||
|
ws, colors, _, _ := relationFixture(t, s)
|
||||||
|
schema := u1RelationSchema("colors")
|
||||||
|
|
||||||
|
createTestItem(t, s, ws.ID, colors.ID, "Teal", "")
|
||||||
|
createTestItem(t, s, ws.ID, colors.ID, "Teal", "")
|
||||||
|
|
||||||
|
stored, issues := resolveOne(t, s, ws, schema, "Teal")
|
||||||
|
if len(issues) != 1 {
|
||||||
|
t.Fatalf("want exactly one issue, got %d: %v", len(issues), issues)
|
||||||
|
}
|
||||||
|
if issues[0].Reason != RelationTargetAmbiguous {
|
||||||
|
t.Errorf("reason = %q, want %q", issues[0].Reason, RelationTargetAmbiguous)
|
||||||
|
}
|
||||||
|
// The supplied value is left as the caller sent it, not blanked and not
|
||||||
|
// canonicalised to either twin — an ambiguous title names no single item,
|
||||||
|
// so there is nothing to canonicalise to.
|
||||||
|
if stored != "Teal" {
|
||||||
|
t.Errorf("stored = %q, want the supplied title back untouched", stored)
|
||||||
|
}
|
||||||
|
|
||||||
|
// The vocabulary must be enumerable, since the copy preflight puts these on
|
||||||
|
// the wire and a cross-stack gate checks the client against this list.
|
||||||
|
found := false
|
||||||
|
for _, r := range RelationIssueReasons() {
|
||||||
|
if r == RelationTargetAmbiguous {
|
||||||
|
found = true
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if !found {
|
||||||
|
t.Error("RelationTargetAmbiguous is missing from RelationIssueReasons(), so no consumer can enumerate it")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestRelationTitle_RefWinsOverAnItemTitledLikeARef pins the ladder's order and
|
||||||
|
// the shadowing case the resolver's doc comment names: UUID, then ref, then
|
||||||
|
// title. An item literally titled "COLO-3" is unreachable BY TITLE while a ref
|
||||||
|
// COLO-3 resolves.
|
||||||
|
//
|
||||||
|
// This is deliberate — refs are the canonical spelling and the ambiguity is
|
||||||
|
// created by the title — but it is a real edge, so it is pinned rather than
|
||||||
|
// left to be rediscovered as a bug.
|
||||||
|
func TestRelationTitle_RefWinsOverAnItemTitledLikeARef(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
s := testStore(t)
|
||||||
|
ws, colors, _, red := relationFixture(t, s)
|
||||||
|
schema := u1RelationSchema("colors")
|
||||||
|
|
||||||
|
// A second colour whose TITLE is the first colour's REF.
|
||||||
|
createTestItem(t, s, ws.ID, colors.ID, red.Ref, "")
|
||||||
|
// A third with an ordinary title, for the control below.
|
||||||
|
plain := createTestItem(t, s, ws.ID, colors.ID, "Cerulean", "")
|
||||||
|
|
||||||
|
stored, issues := resolveOne(t, s, ws, schema, red.Ref)
|
||||||
|
if len(issues) != 0 {
|
||||||
|
t.Fatalf("unexpected issues: %v", issues)
|
||||||
|
}
|
||||||
|
if stored != red.ID {
|
||||||
|
t.Errorf("stored %q, want the REF's target %q — the ref rung must win over an item merely titled like one", stored, red.ID)
|
||||||
|
}
|
||||||
|
|
||||||
|
// CONTROL, and the reason this test needs one (codex round 1 nit): the
|
||||||
|
// assertions above supply a REF and exercise only the pre-existing ref
|
||||||
|
// rung, so they pass with exact-title resolution removed entirely and the
|
||||||
|
// impostor is decoration. The control resolves an ORDINARY title in the
|
||||||
|
// same run, so the test fails if the title rung goes away — which is what
|
||||||
|
// makes the shadowing assertion above a statement about PRECEDENCE rather
|
||||||
|
// than about a rung that might not exist.
|
||||||
|
byTitle, issues := resolveOne(t, s, ws, schema, "Cerulean")
|
||||||
|
if len(issues) != 0 {
|
||||||
|
t.Fatalf("control: an ordinary title must resolve, got %v", issues)
|
||||||
|
}
|
||||||
|
if byTitle != plain.ID {
|
||||||
|
t.Errorf("control: stored %q, want %q", byTitle, plain.ID)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestRelationTitle_SoftDeletedTitleDoesNotResolve: a deleted item's title is
|
||||||
|
// not a handle. The control is the same title on a live item, so the leg
|
||||||
|
// cannot pass by refusing every title.
|
||||||
|
func TestRelationTitle_SoftDeletedTitleDoesNotResolve(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
s := testStore(t)
|
||||||
|
ws, colors, _, _ := relationFixture(t, s)
|
||||||
|
schema := u1RelationSchema("colors")
|
||||||
|
|
||||||
|
gone := createTestItem(t, s, ws.ID, colors.ID, "Ochre", "")
|
||||||
|
if err := s.DeleteItem(gone.ID); err != nil {
|
||||||
|
t.Fatalf("delete: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
_, issues := resolveOne(t, s, ws, schema, "Ochre")
|
||||||
|
if len(issues) != 1 || issues[0].Reason != RelationTargetNotFound {
|
||||||
|
t.Fatalf("a soft-deleted title must be not_found, got %v", issues)
|
||||||
|
}
|
||||||
|
|
||||||
|
// CONTROL: the same title, live again, resolves.
|
||||||
|
live := createTestItem(t, s, ws.ID, colors.ID, "Ochre", "")
|
||||||
|
stored, issues := resolveOne(t, s, ws, schema, "Ochre")
|
||||||
|
if len(issues) != 0 {
|
||||||
|
t.Fatalf("control: a live title must resolve, got %v", issues)
|
||||||
|
}
|
||||||
|
if stored != live.ID {
|
||||||
|
t.Errorf("control: stored %q, want %q", stored, live.ID)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestHydrateRelationTargets_ResolvesDanglesAndRefusesForeignWorkspaces covers
|
||||||
|
// the read side: what a caller gets back for each kind of stored value.
|
||||||
|
//
|
||||||
|
// The foreign-workspace leg is the one worth the fixture cost. A stored
|
||||||
|
// relation value is just an id in a blob, and nothing stops a legacy row from
|
||||||
|
// naming an item in ANOTHER workspace; hydrating it would put that item's ref
|
||||||
|
// and title in a response to someone who cannot see its workspace at all.
|
||||||
|
func TestHydrateRelationTargets_ResolvesDanglesAndRefusesForeignWorkspaces(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
s := testStore(t)
|
||||||
|
ws, colors, cars, red := relationFixture(t, s)
|
||||||
|
|
||||||
|
// A live colour in a DIFFERENT workspace, reachable only by its raw id.
|
||||||
|
otherWS, otherColors, _, _ := relationFixture(t, s)
|
||||||
|
foreign := createTestItem(t, s, otherWS.ID, otherColors.ID, "Foreign Red", "")
|
||||||
|
_ = colors
|
||||||
|
|
||||||
|
mk := func(value string) models.Item {
|
||||||
|
it := createTestItem(t, s, ws.ID, cars.ID, "Car "+value[:6], "")
|
||||||
|
blob, _ := json.Marshal(map[string]any{"status": "open", "color": value})
|
||||||
|
if _, err := s.UpdateItem(it.ID, models.ItemUpdate{Fields: strPtr(string(blob))}); err != nil {
|
||||||
|
t.Fatalf("seed fields: %v", err)
|
||||||
|
}
|
||||||
|
got, err := s.GetItem(it.ID)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("reload: %v", err)
|
||||||
|
}
|
||||||
|
return *got
|
||||||
|
}
|
||||||
|
|
||||||
|
resolvable := mk(red.ID)
|
||||||
|
dangling := mk("00000000-0000-4000-8000-00000000dead")
|
||||||
|
crossWS := mk(foreign.ID)
|
||||||
|
|
||||||
|
schemas := map[string]models.CollectionSchema{cars.ID: u1RelationSchema("colors")}
|
||||||
|
out, err := s.HydrateRelationTargetsQ(s.DB(), ws.ID, []models.Item{resolvable, dangling, crossWS}, schemas)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("hydrate: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Resolvable: the full triple.
|
||||||
|
got := out[resolvable.ID]["color"]
|
||||||
|
if got.ID != red.ID || got.Ref != red.Ref || got.Title != red.Title {
|
||||||
|
t.Errorf("resolvable hydrated as %+v, want {id:%s ref:%s title:%s}", got, red.ID, red.Ref, red.Title)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Dangling: ID-ONLY, and PRESENT. Omitting the key would say "this item has
|
||||||
|
// no relation", which is a different and false statement.
|
||||||
|
got = out[dangling.ID]["color"]
|
||||||
|
if _, present := out[dangling.ID]["color"]; !present {
|
||||||
|
t.Fatal("a dangling value was omitted rather than hydrated id-only")
|
||||||
|
}
|
||||||
|
if got.Ref != "" || got.Title != "" {
|
||||||
|
t.Errorf("dangling hydrated as %+v, want id-only", got)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Cross-workspace: also ID-ONLY. This is the disclosure leg — a ref or a
|
||||||
|
// title here is another workspace's data.
|
||||||
|
got = out[crossWS.ID]["color"]
|
||||||
|
if got.Ref != "" || got.Title != "" {
|
||||||
|
t.Errorf("a foreign-workspace id hydrated as %+v — that is %s's ref/title in this workspace's response", got, otherWS.Slug)
|
||||||
|
}
|
||||||
|
if got.ID != foreign.ID {
|
||||||
|
t.Errorf("cross-workspace entry lost its stored id: %+v", got)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// mustCollectionID looks a collection up by slug, failing the test rather than
|
||||||
|
// returning an empty string that would make a later assertion fail for the
|
||||||
|
// wrong reason.
|
||||||
|
func mustCollectionID(t *testing.T, s *Store, workspaceID, slug string) string {
|
||||||
|
t.Helper()
|
||||||
|
colls, err := s.ListCollections(workspaceID)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("list collections: %v", err)
|
||||||
|
}
|
||||||
|
for i := range colls {
|
||||||
|
if colls[i].Slug == slug {
|
||||||
|
return colls[i].ID
|
||||||
|
}
|
||||||
|
}
|
||||||
|
t.Fatalf("no collection %q in workspace %s (have %d)", slug, workspaceID, len(colls))
|
||||||
|
return ""
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestRelationTitle_ANullItemNumberCandidateStillCounts is codex round 3's
|
||||||
|
// third P1, and it is the reason the candidate walk pages on `id`.
|
||||||
|
//
|
||||||
|
// The first paging shape used `item_number > ?` as its cursor. item_number is
|
||||||
|
// NULLABLE — migration 006 adds it with no constraint and nothing since makes
|
||||||
|
// it NOT NULL — and `item_number > ?` excludes every NULL row, while the count
|
||||||
|
// that decided ambiguity did not. So the two disagreed about the candidate set,
|
||||||
|
// and a live legacy row could be skipped: the walk would see one match where
|
||||||
|
// there were two, and RESOLVE a title that is genuinely ambiguous.
|
||||||
|
//
|
||||||
|
// `id` is the primary key and cannot be null.
|
||||||
|
func TestRelationTitle_ANullItemNumberCandidateStillCounts(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
s := testStore(t)
|
||||||
|
ws, colors, _, _ := relationFixture(t, s)
|
||||||
|
schema := u1RelationSchema("colors")
|
||||||
|
|
||||||
|
ordinary := createTestItem(t, s, ws.ID, colors.ID, "Legacy Hue", "")
|
||||||
|
legacy := createTestItem(t, s, ws.ID, colors.ID, "Legacy Hue", "")
|
||||||
|
// A pre-006 row: live, titled, and carrying no item_number at all.
|
||||||
|
if _, err := s.db.Exec(s.q("UPDATE items SET item_number = NULL WHERE id = ?"), legacy.ID); err != nil {
|
||||||
|
t.Fatalf("null the item_number: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
_, issues := resolveOne(t, s, ws, schema, "Legacy Hue")
|
||||||
|
if len(issues) != 1 || issues[0].Reason != RelationTargetAmbiguous {
|
||||||
|
t.Fatalf("two live items carry this title — one of them with a NULL item_number — so it is ambiguous, got %v", issues)
|
||||||
|
}
|
||||||
|
|
||||||
|
// CONTROL: with the NULL-numbered row gone the same title resolves, so the
|
||||||
|
// leg above is about the NULL row counting rather than about the resolver
|
||||||
|
// refusing everything.
|
||||||
|
if err := s.DeleteItem(legacy.ID); err != nil {
|
||||||
|
t.Fatalf("delete legacy: %v", err)
|
||||||
|
}
|
||||||
|
stored, issues := resolveOne(t, s, ws, schema, "Legacy Hue")
|
||||||
|
if len(issues) != 0 {
|
||||||
|
t.Fatalf("control: one live match must resolve, got %v", issues)
|
||||||
|
}
|
||||||
|
if stored != ordinary.ID {
|
||||||
|
t.Errorf("control: stored %q, want %q", stored, ordinary.ID)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestMigrateRelationTitle_CarriedValueIgnoresTheMoversVisibility is codex
|
||||||
|
// round 5's P1, and this file's own standing argument, as a test.
|
||||||
|
//
|
||||||
|
// A CARRIED value was asserted by nobody, so no caller is probing with it —
|
||||||
|
// and judging it by the MOVER's visibility is exactly what
|
||||||
|
// MigrateRelationReferentsQ's comment calls out as making "the STORED BYTES
|
||||||
|
// depend on who performed the move". U6 made that reachable for the first
|
||||||
|
// time: before it, a carried value that happened to match a title never
|
||||||
|
// resolved at all, so there was nothing for visibility to change.
|
||||||
|
//
|
||||||
|
// Both legs use a predicate that hides EVERYTHING. If the carried path
|
||||||
|
// consulted it, the relation would be dropped; it must survive identically.
|
||||||
|
func TestMigrateRelationTitle_CarriedValueIgnoresTheMoversVisibility(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
s := testStore(t)
|
||||||
|
ws, _, _, red := relationFixture(t, s)
|
||||||
|
|
||||||
|
blind := func(Queryer, string, *models.Item) (bool, error) { return false, nil }
|
||||||
|
|
||||||
|
// The carried value is a TITLE — the U6 spelling, and the one that was
|
||||||
|
// never resolvable before this unit.
|
||||||
|
fields := map[string]any{"color": red.Title, "status": "open"}
|
||||||
|
refusals, dropped, err := s.MigrateRelationReferents(blind, ws.ID, u1RelationSchema("colors"), fields, nil, carriedFrom(fields), RelationCarryWithinWorkspace)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("unexpected error: %v", err)
|
||||||
|
}
|
||||||
|
if len(refusals) != 0 || len(dropped) != 0 {
|
||||||
|
t.Fatalf("a carried title was judged by the mover's visibility, so the stored bytes now depend on WHO moved the item: refusals=%+v dropped=%+v", refusals, dropped)
|
||||||
|
}
|
||||||
|
if fields["color"] != red.ID {
|
||||||
|
t.Errorf("carried title resolved to %v, want the canonical id %s", fields["color"], red.ID)
|
||||||
|
}
|
||||||
|
|
||||||
|
// PARITY: the same carried title with a permissive predicate must produce
|
||||||
|
// the identical stored value. Without this leg, a migrate that ignored the
|
||||||
|
// value entirely would pass the assertions above.
|
||||||
|
seeing := func(Queryer, string, *models.Item) (bool, error) { return true, nil }
|
||||||
|
fields2 := map[string]any{"color": red.Title, "status": "open"}
|
||||||
|
if _, _, err := s.MigrateRelationReferents(seeing, ws.ID, u1RelationSchema("colors"), fields2, nil, carriedFrom(fields2), RelationCarryWithinWorkspace); err != nil {
|
||||||
|
t.Fatalf("unexpected error: %v", err)
|
||||||
|
}
|
||||||
|
if fields2["color"] != fields["color"] {
|
||||||
|
t.Errorf("two movers stored different values for one carried title: %v vs %v", fields["color"], fields2["color"])
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestRelationTitle_SuppliedValueStillObeysVisibility is the other half, and it
|
||||||
|
// is what stops the fix above from being "visibility was switched off".
|
||||||
|
//
|
||||||
|
// A SUPPLIED title — one the caller typed — must still be judged, because that
|
||||||
|
// is the probe the whole rule exists to answer.
|
||||||
|
func TestRelationTitle_SuppliedValueStillObeysVisibility(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
s := testStore(t)
|
||||||
|
ws, _, _, red := relationFixture(t, s)
|
||||||
|
|
||||||
|
blind := func(Queryer, string, *models.Item) (bool, error) { return false, nil }
|
||||||
|
fields := map[string]any{"color": red.Title}
|
||||||
|
issues, err := s.ResolveRelationReferents(ws.ID, u1RelationSchema("colors"), fields, blind)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("unexpected error: %v", err)
|
||||||
|
}
|
||||||
|
if len(issues) != 1 || issues[0].Reason != RelationTargetNotFound {
|
||||||
|
t.Fatalf("a SUPPLIED title naming an item the caller cannot see must be not_found — the carried carve-out must not reach it: %v", issues)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestHydrateRelationTargets_WrongCollectionAndNullNumberStayHonest covers
|
||||||
|
// codex round 7's two read-side P2s.
|
||||||
|
//
|
||||||
|
// Both are about legacy rows: the write side has refused wrong-collection
|
||||||
|
// relation values since U1 and item_number has been populated since migration
|
||||||
|
// 006, but rows predating either still exist and the READ has to hold up.
|
||||||
|
func TestHydrateRelationTargets_WrongCollectionAndNullNumberStayHonest(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
s := testStore(t)
|
||||||
|
ws, colors, cars, red := relationFixture(t, s)
|
||||||
|
|
||||||
|
// A live item in the WRONG collection for the `color` field.
|
||||||
|
sedan := createTestItem(t, s, ws.ID, cars.ID, "Sedan", "")
|
||||||
|
// A legitimate target whose item_number is NULL, as a pre-006 row is.
|
||||||
|
legacy := createTestItem(t, s, ws.ID, colors.ID, "Legacy Blue", "")
|
||||||
|
if _, err := s.db.Exec(s.q("UPDATE items SET item_number = NULL WHERE id = ?"), legacy.ID); err != nil {
|
||||||
|
t.Fatalf("null the item_number: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
mk := func(label, value string) models.Item {
|
||||||
|
it := createTestItem(t, s, ws.ID, cars.ID, label, "")
|
||||||
|
blob, _ := json.Marshal(map[string]any{"status": "open", "color": value})
|
||||||
|
fields := string(blob)
|
||||||
|
if _, err := s.UpdateItem(it.ID, models.ItemUpdate{Fields: &fields}); err != nil {
|
||||||
|
t.Fatalf("seed %s: %v", label, err)
|
||||||
|
}
|
||||||
|
got, err := s.GetItem(it.ID)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("reload %s: %v", label, err)
|
||||||
|
}
|
||||||
|
return *got
|
||||||
|
}
|
||||||
|
|
||||||
|
good := mk("Points at a colour", red.ID)
|
||||||
|
wrong := mk("Points at a car", sedan.ID)
|
||||||
|
nulled := mk("Points at a legacy colour", legacy.ID)
|
||||||
|
|
||||||
|
schemas := map[string]models.CollectionSchema{cars.ID: u1RelationSchema("colors")}
|
||||||
|
out, err := s.HydrateRelationTargetsQ(s.DB(), ws.ID, []models.Item{good, wrong, nulled}, schemas)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("hydrate: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// A NULL item_number must not take the whole response down with it. This
|
||||||
|
// function is best-effort and its caller drops the entire map on error, so
|
||||||
|
// scanning NULL into an int would have removed relation_targets from every
|
||||||
|
// item in the response, not just this one.
|
||||||
|
if len(out) == 0 {
|
||||||
|
t.Fatal("hydration returned nothing at all; one legacy row with a NULL item_number erased the whole response")
|
||||||
|
}
|
||||||
|
if got := out[good.ID]["color"]; got.Ref != red.Ref || got.Title != red.Title {
|
||||||
|
t.Errorf("the ordinary target did not hydrate: %+v", got)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Wrong collection: id-only. Rendering it fully would present a Car as
|
||||||
|
// though it were the Colour the field promised.
|
||||||
|
if got := out[wrong.ID]["color"]; got.Ref != "" || got.Title != "" {
|
||||||
|
t.Errorf("a value pointing OUTSIDE the declared collection hydrated as %+v; the field says colors and this is a car", got)
|
||||||
|
} else if got.ID != sedan.ID {
|
||||||
|
t.Errorf("the wrong-collection entry lost its stored id: %+v", got)
|
||||||
|
}
|
||||||
|
|
||||||
|
// NULL item_number: the row is a legitimate target in the right
|
||||||
|
// collection, so it hydrates — with a title and no ref, because there is
|
||||||
|
// no number to build one from. Not id-only, and not an error.
|
||||||
|
got := out[nulled.ID]["color"]
|
||||||
|
if got.Title != "Legacy Blue" {
|
||||||
|
t.Errorf("a NULL-numbered target lost its title: %+v", got)
|
||||||
|
}
|
||||||
|
if got.Ref != "" {
|
||||||
|
t.Errorf("a NULL-numbered target invented a ref: %+v", got)
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -25,8 +25,9 @@
|
|||||||
*
|
*
|
||||||
* KEPT IN THE SAME ORDER as the Go declarations so a diff of one against the
|
* KEPT IN THE SAME ORDER as the Go declarations so a diff of one against the
|
||||||
* other is readable. The first five are the migrate-level reasons in
|
* other is readable. The first five are the migrate-level reasons in
|
||||||
* `handlers_items_copy_preflight.go`; the last five are the relation-level
|
* `handlers_items_copy_preflight.go`; the rest are the relation-level
|
||||||
* `RelationIssueReason` constants in `internal/store/relation_referents.go`.
|
* `RelationIssueReason` constants in `internal/store/relation_referents.go`
|
||||||
|
* (five at TASK-2878, plus `ambiguous` at PLAN-2857 U6).
|
||||||
*
|
*
|
||||||
* This list is the CONTRACT the completeness test checks the map against. It
|
* This list is the CONTRACT the completeness test checks the map against. It
|
||||||
* is duplicated from Go rather than generated, so it can go stale — which is
|
* is duplicated from Go rather than generated, so it can go stale — which is
|
||||||
@@ -47,6 +48,7 @@ export const COPY_DROP_REASONS = [
|
|||||||
'wrong_collection',
|
'wrong_collection',
|
||||||
'target_missing',
|
'target_missing',
|
||||||
'invalid_shape',
|
'invalid_shape',
|
||||||
|
'ambiguous',
|
||||||
] as const;
|
] as const;
|
||||||
|
|
||||||
export type CopyDropReason = (typeof COPY_DROP_REASONS)[number];
|
export type CopyDropReason = (typeof COPY_DROP_REASONS)[number];
|
||||||
@@ -81,6 +83,13 @@ const MESSAGES: Record<CopyDropReason, string> = {
|
|||||||
target_missing: 'the field has no valid collection to link to',
|
target_missing: 'the field has no valid collection to link to',
|
||||||
|
|
||||||
invalid_shape: 'the destination field’s default is not a valid reference',
|
invalid_shape: 'the destination field’s default is not a valid reference',
|
||||||
|
|
||||||
|
// PLAN-2857 U6. Reachable only from a TITLE — a UUID and a ref are unique
|
||||||
|
// by construction — and it says the title matched TOO MUCH, which is the
|
||||||
|
// opposite of `not_found`. The sentence names the fix the user can act on
|
||||||
|
// (be more specific) without claiming how many matched, which the response
|
||||||
|
// does not say.
|
||||||
|
ambiguous: 'more than one item in that collection has this title',
|
||||||
};
|
};
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
@@ -852,6 +852,15 @@ export interface ItemCopyPreflightDropped {
|
|||||||
| 'not_found'
|
| 'not_found'
|
||||||
| 'wrong_collection'
|
| 'wrong_collection'
|
||||||
| 'target_missing'
|
| 'target_missing'
|
||||||
|
/**
|
||||||
|
* An exact TITLE matched two or more live items inside the field's
|
||||||
|
* declared collection, so it names no single one (PLAN-2857 U6). Only
|
||||||
|
* the title path can produce it — a UUID and a ref are unique by
|
||||||
|
* construction. It is its own reason rather than `not_found` because
|
||||||
|
* that would tell the caller the opposite of what happened: the title
|
||||||
|
* matched too much, not too little.
|
||||||
|
*/
|
||||||
|
| 'ambiguous'
|
||||||
/**
|
/**
|
||||||
* The destination schema's default for this field is not a reference at
|
* The destination schema's default for this field is not a reference at
|
||||||
* all. Injected defaults are never type-checked, so this is the one
|
* all. Injected defaults are never type-checked, so this is the one
|
||||||
|
|||||||
Reference in New Issue
Block a user