amadan

carlos / platform Public

layout: AppendLayoutIntent returns the stamped request (review fix round 1)

AppendLayoutIntent returned only an error, so the console door in plan 5
would need a second read just to learn the id and timestamp it had
recorded. Change the signature to return (LayoutIntent, error), the
stamped request on success and the zero value on every error path,
mirroring AppendRestartScope's own return.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
claude 25245ff01dd05e35081669c619ff9745c7ca598c parent b5bab43
2 files changed, +31 −17
  • internal/release/layoutintent.go +14 −12
  • internal/release/layoutintent_test.go +17 −5

CI passed — run details

diff --git a/internal/release/layoutintent.go b/internal/release/layoutintent.go
index 3f33ba3a7..acacaeae0 100644
--- a/internal/release/layoutintent.go
+++ b/internal/release/layoutintent.go
@@ -106,9 +106,11 @@ func LoadLayoutIntent(ctx context.Context, s Store, boxLabel string) (*LayoutInt
}
// AppendLayoutIntent records one request, oldest-first, after every request
-// already in the log. ID and RequestedAt are stamped here when the caller
-// leaves them empty, so a caller that only cares about Mode and Scope never
-// has to mint either itself.
+// already in the log, and returns the stamped request it wrote. ID and
+// RequestedAt are stamped here when the caller leaves them empty, so a
+// caller that only cares about Mode and Scope never has to mint either
+// itself, and returning the stamped value means the console door (plan 5)
+// never has to read the object back just to learn the id it recorded.
//
// A CAS conflict is retried, the same shape and for the same reason as
// AppendRestartScope: another writer's request names a different mode or
@@ -116,14 +118,14 @@ func LoadLayoutIntent(ctx context.Context, s Store, boxLabel string) (*LayoutInt
// will never happen. The retry budget is small on purpose; this is a
// human-paced verb, and a contended object means something is wrong that
// another lap will not fix.
-func AppendLayoutIntent(ctx context.Context, s Store, in LayoutIntent) error {
+func AppendLayoutIntent(ctx context.Context, s Store, in LayoutIntent) (LayoutIntent, error) {
if !ValidLayoutMode(in.Mode) {
- return fmt.Errorf("layout intent: mode %q is not flat or nested", in.Mode)
+ return LayoutIntent{}, fmt.Errorf("layout intent: mode %q is not flat or nested", in.Mode)
}
if in.ID == "" {
id := make([]byte, 16)
if _, err := io.ReadFull(rand.Reader, id); err != nil {
- return err
+ return LayoutIntent{}, err
}
in.ID = hex.EncodeToString(id)
}
@@ -135,14 +137,14 @@ func AppendLayoutIntent(ctx context.Context, s Store, in LayoutIntent) error {
for attempt := 0; attempt < 3; attempt++ {
raw, etag, err := s.Get(ctx, key)
if err != nil && !errors.Is(err, os.ErrNotExist) {
- return err
+ return LayoutIntent{}, err
}
g := &LayoutIntents{Schema: 1}
if err == nil {
if g, err = parseLayoutIntents(key, raw); err != nil {
// Refusing beats overwriting: a log this binary cannot read is
// one whose entries it would silently drop on the next write.
- return err
+ return LayoutIntent{}, err
}
} else {
etag = "" // create-only
@@ -151,16 +153,16 @@ func AppendLayoutIntent(ctx context.Context, s Store, in LayoutIntent) error {
g.Requests = append(g.Requests, in)
body, merr := json.Marshal(g)
if merr != nil {
- return merr
+ return LayoutIntent{}, merr
}
if _, err := s.Put(ctx, key, body, etag); err != nil {
if errors.Is(err, ErrCASConflict) {
lastErr = err
continue
}
- return err
+ return LayoutIntent{}, err
}
- return nil
+ return in, nil
}
- return fmt.Errorf("layout intent: %s is being written by someone else; nothing was requested; try again: %w", key, lastErr)
+ return LayoutIntent{}, fmt.Errorf("layout intent: %s is being written by someone else; nothing was requested; try again: %w", key, lastErr)
}
diff --git a/internal/release/layoutintent_test.go b/internal/release/layoutintent_test.go
index 37563ee4d..f5c9b5834 100644
--- a/internal/release/layoutintent_test.go
+++ b/internal/release/layoutintent_test.go
@@ -17,16 +17,20 @@ func TestLayoutIntentRoundTripAndScope(t *testing.T) {
t.Fatalf("absent object: found=%v err=%v", found, err)
}
- if err := AppendLayoutIntent(ctx, s, LayoutIntent{Mode: LayoutModeNested, Scope: LayoutScope{Box: "kass-1"}}); err != nil {
+ stamped, err := AppendLayoutIntent(ctx, s, LayoutIntent{Mode: LayoutModeNested, Scope: LayoutScope{Box: "kass-1"}})
+ if err != nil {
t.Fatalf("append: %v", err)
}
+ if stamped.ID == "" || stamped.RequestedAt == "" {
+ t.Fatalf("append did not return the stamped id and timestamp: %+v", stamped)
+ }
got, found, err := LoadLayoutIntent(ctx, s, "kass-1")
if err != nil || !found || got.Mode != LayoutModeNested {
t.Fatalf("round trip: %+v %v %v", got, found, err)
}
- if got.ID == "" || got.RequestedAt == "" {
- t.Fatalf("append did not stamp an id and a timestamp: %+v", got)
+ if got.ID != stamped.ID || got.RequestedAt != stamped.RequestedAt {
+ t.Fatalf("stored request = %+v, want the returned stamped values %+v", got, stamped)
}
if !got.Applies("kass-1") || got.Applies("kass-2") {
t.Error("box scope must match only the named box")
@@ -37,9 +41,13 @@ func TestLayoutIntentRoundTripAndScope(t *testing.T) {
// A later fleet-wide request (empty scope) applies everywhere, including
// kass-1, and it is newer, so it is the one that wins there too.
- if err := AppendLayoutIntent(ctx, s, LayoutIntent{Mode: LayoutModeFlat}); err != nil {
+ second, err := AppendLayoutIntent(ctx, s, LayoutIntent{Mode: LayoutModeFlat})
+ if err != nil {
t.Fatalf("second append: %v", err)
}
+ if second.ID == stamped.ID {
+ t.Fatalf("second append reused the first request's id %q", second.ID)
+ }
got, found, err = LoadLayoutIntent(ctx, s, "kass-1")
if err != nil || !found || got.Mode != LayoutModeFlat {
t.Fatalf("the newest applying request must win: %+v %v %v", got, found, err)
@@ -77,9 +85,13 @@ func TestValidLayoutMode(t *testing.T) {
func TestAppendLayoutIntentRefusesAnInvalidMode(t *testing.T) {
s := NewDirStore(t.TempDir())
- if err := AppendLayoutIntent(context.Background(), s, LayoutIntent{Mode: "bogus"}); err == nil {
+ stamped, err := AppendLayoutIntent(context.Background(), s, LayoutIntent{Mode: "bogus"})
+ if err == nil {
t.Fatal("an invalid mode must be refused, not written")
}
+ if stamped != (LayoutIntent{}) {
+ t.Fatalf("a refused append must return the zero value, got %+v", stamped)
+ }
if _, found, err := LoadLayoutIntent(context.Background(), s, "any"); err != nil || found {
t.Fatalf("a refused append must leave the object absent: found=%v err=%v", found, err)
}