[HOLD] Stop storing actor identity in ate Redis's value

cmd/ateapi/internal/store/ateredis/ateredis.go

@@ -307,6 +307,27 @@ func (s *Persistence) DebugClearAll(ctx context.Context) error {
 	return err
 }
 
+// marshalStoredActor serializes dbActor for storage, clearing the identity
+// fields as they are stored in the key instead of the value. It mutates
+// dbActor: callers pass a clone they own.
+func marshalStoredActor(dbActor *ateapipb.Actor) ([]byte, error) {
+	dbActor.Atespace = ""
+	dbActor.ActorId = ""
+	return protojson.Marshal(dbActor)
+}
+
+// unmarshalStoredActor parses a stored actor value and backfills the identity
+// from the key components.
+func unmarshalStoredActor(data []byte, atespace, id string) (*ateapipb.Actor, error) {
+	actor := &ateapipb.Actor{}
+	if err := protojson.Unmarshal(data, actor); err != nil {
+		return nil, err
+	}
+	actor.Atespace = atespace
+	actor.ActorId = id
+	return actor, nil
+}
+
 func (s *Persistence) GetActor(ctx context.Context, atespace, id string) (*ateapipb.Actor, error) {
 	dbKey := actorDBKey(atespace, id)
 
@@ -318,15 +339,11 @@ func (s *Persistence) GetActor(ctx context.Context, atespace, id string) (*ateap
 		return nil, fmt.Errorf("while getting actor key %q: %w", dbKey, err)
 	}
 
-	actor := &ateapipb.Actor{}
-	if err := protojson.Unmarshal(dbActorBytes, actor); err != nil {
+	actor, err := unmarshalStoredActor(dbActorBytes, atespace, id)
+	if err != nil {
 		return nil, fmt.Errorf("while unmarshaling actor: %w", err)
 	}
 
-	if actor.GetActorId() != id || actor.GetAtespace() != atespace {
-		return nil, fmt.Errorf("(impossible) mismatch between stored id/atespace and key")
-	}
-
 	return actor, nil
 }
 
@@ -338,9 +355,9 @@ func (s *Persistence) CreateActor(ctx context.Context, actor *ateapipb.Actor) er
 	dbActor := proto.Clone(actor).(*ateapipb.Actor)
 	dbActor.Version = 1
 
-	dbActorBytes, err := protojson.Marshal(dbActor)
+	dbActorBytes, err := marshalStoredActor(dbActor)
 	if err != nil {
-		return fmt.Errorf("in protojson.Marshal: %w", err)
+		return fmt.Errorf("while marshaling actor: %w", err)
 	}
 
 	ok, err := s.rdb.SetNX(ctx, dbKey, dbActorBytes, 0).Result()
@@ -538,22 +555,16 @@ func (s *Persistence) UpdateActor(ctx context.Context, actor *ateapipb.Actor, ex
 			return store.ErrPersistenceRetry
 		}
 		dbActor.Version = currentActor.GetVersion() + 1
-		if currentActor.GetActorId() != dbActor.GetActorId() {
-			return fmt.Errorf("actor_id is immutable")
-		}
-		if currentActor.GetAtespace() != dbActor.GetAtespace() {
-			return fmt.Errorf("atespace is immutable")
-		}
 		if currentActor.GetActorTemplateNamespace() != dbActor.GetActorTemplateNamespace() {
 			return fmt.Errorf("actor_template_namespace is immutable")
 		}
 		if currentActor.GetActorTemplateName() != dbActor.GetActorTemplateName() {
 			return fmt.Errorf("actor_template_name is immutable")
 		}
 
-		newVal, err := protojson.Marshal(dbActor)
+		newVal, err := marshalStoredActor(dbActor)
 		if err != nil {
-			return fmt.Errorf("in protojson.Marshal: %w", err)
+			return fmt.Errorf("while marshaling actor: %w", err)
 		}
 
 		_, err = tx.TxPipelined(ctx, func(pipe redis.Pipeliner) error {
@@ -768,7 +779,7 @@ func (s *Persistence) fetchActors(ctx context.Context, master *redis.Client, key
 	}
 
 	var actors []*ateapipb.Actor
-	for _, cmd := range cmds {
+	for i, cmd := range cmds {
 		getCmd, ok := cmd.(*redis.StringCmd)
 		if !ok {
 			continue
@@ -780,9 +791,13 @@ func (s *Persistence) fetchActors(ctx context.Context, master *redis.Client, key
 			return nil, fmt.Errorf("while getting actor: %w", getCmd.Err())
 		}
 
-		actor := &ateapipb.Actor{}
-		if err := protojson.Unmarshal([]byte(getCmd.Val()), actor); err != nil {
-			return nil, fmt.Errorf("in protojson.Unmarshal: %w", err)
+		parts := strings.Split(keys[i], ":")
+		if len(parts) != 3 {
+			return nil, fmt.Errorf("bad key format %q", keys[i])
+		}
+		actor, err := unmarshalStoredActor([]byte(getCmd.Val()), parts[1], parts[2])
+		if err != nil {
+			return nil, fmt.Errorf("while unmarshaling actor: %w", err)
 		}
 		actors = append(actors, actor)
 	}

cmd/ateapi/internal/store/ateredis/ateredis_test.go

@@ -19,6 +19,7 @@ import (
 	"errors"
 	"fmt"
 	"sort"
+	"strings"
 	"sync"
 	"testing"
 	"time"
@@ -1070,6 +1071,56 @@ func TestDeleteAtespace_EmptyAfterActorsRemoved(t *testing.T) {
 	}
 }
 
+func TestActorValue_OmitsIdentity(t *testing.T) {
+	mr, s, ctx := setupTest(t)
+	defer mr.Close()
+
+	actor := &ateapipb.Actor{
+		ActorId:                "a1",
+		Atespace:               "team-a",
+		ActorTemplateNamespace: "default",
+		ActorTemplateName:      "tmpl",
+		Status:                 ateapipb.Actor_STATUS_SUSPENDED,
+	}
+	if err := s.CreateActor(ctx, actor); err != nil {
+		t.Fatalf("CreateActor failed: %v", err)
+	}
+
+	assertNoIdentity := func(operation string) {
+		t.Helper()
+		raw, err := mr.Get("actor:team-a:a1")
+		if err != nil {
+			t.Fatalf("raw get after %s failed: %v", operation, err)
+		}
+		if strings.Contains(raw, `"actorId"`) || strings.Contains(raw, `"atespace"`) {
+			t.Errorf("value after %s duplicates key identity: %s", operation, raw)
+		}
+	}
+	assertNoIdentity("create")
+
+	got, err := s.GetActor(ctx, "team-a", "a1")
+	if err != nil {
+		t.Fatalf("GetActor failed: %v", err)
+	}
+	if got.GetAtespace() != "team-a" || got.GetActorId() != "a1" {
+		t.Errorf("GetActor identity = (%q, %q), want (team-a, a1)", got.GetAtespace(), got.GetActorId())
+	}
+
+	got.Status = ateapipb.Actor_STATUS_RUNNING
+	if err := s.UpdateActor(ctx, got, got.GetVersion()); err != nil {
+		t.Fatalf("UpdateActor failed: %v", err)
+	}
+	assertNoIdentity("update")
+
+	actors, _, err := s.ListActors(ctx, "team-a", 10, "")
+	if err != nil {
+		t.Fatalf("ListActors failed: %v", err)
+	}
+	if len(actors) != 1 || actors[0].GetAtespace() != "team-a" || actors[0].GetActorId() != "a1" {
+		t.Errorf("ListActors did not re-populate identity: %v", actors)
+	}
+}
+
 func TestDeleteAtespace_EmptyWhileOtherAtespaceNonEmpty(t *testing.T) {
 	mr, s, ctx := setupTest(t)
 	defer mr.Close()