From 55d08e77ed6e3177849569e5ccf1cf18b688cbca Mon Sep 17 00:00:00 2001 From: Worph Date: Sun, 2 Aug 2026 21:35:39 +0200 Subject: [PATCH] improvement on user management --- internal/api/udl.go | 10 ++++- internal/storage/udl.go | 28 ++++++++++--- internal/storage/udl_test.go | 79 ++++++++++++++++++++++++++++++++++++ 3 files changed, 111 insertions(+), 6 deletions(-) create mode 100644 internal/storage/udl_test.go diff --git a/internal/api/udl.go b/internal/api/udl.go index d8dd0b1..7232ab9 100644 --- a/internal/api/udl.go +++ b/internal/api/udl.go @@ -27,6 +27,14 @@ type UDLPutRequest struct { Ts int64 `json:"ts"` Record string `json:"record"` // base64 CBOR, opaque + signed Value *json.RawMessage `json:"value,omitempty"` + // Tombstone marks this write as a deletion of (uid, cid, key). The record + // itself is opaque here — meta-core never decodes the CBOR, and a private- + // tier value and a tombstone are equally "no plaintext value" — so the + // writer has to say. Without it the cid is never removed from + // udl:idx:user::key:, which then grows for the life of the + // account and is returned in full on every My List / Continue Watching + // read. Omitted by older writers, which keeps the previous behaviour. + Tombstone bool `json:"tombstone,omitempty"` } // UDLPutResponse is the response of PUT /api/udl/record. @@ -104,7 +112,7 @@ func (s *Server) handleUDLRecordPut(w http.ResponseWriter, r *http.Request) { hasValue = true } - accepted, err := s.storage.UDLUpsertIfNewer(req.Uid, req.Cid, req.Key, req.Version, req.Ts, req.Record, publicValue, hasValue) + accepted, err := s.storage.UDLUpsertIfNewer(req.Uid, req.Cid, req.Key, req.Version, req.Ts, req.Record, publicValue, hasValue, req.Tombstone) if err != nil { writeError(w, http.StatusInternalServerError, err.Error()) return diff --git a/internal/storage/udl.go b/internal/storage/udl.go index f402fa7..5e20294 100644 --- a/internal/storage/udl.go +++ b/internal/storage/udl.go @@ -72,7 +72,15 @@ func udlIdxCidUsers(cid string) string { return fmt.Sprintf("udl:idx:cid:%s // // KEYS: 1=cell 2=idx:user:key 3=idx:user:cid 4=idx:cid:key 5=idx:cid:users // ARGV: 1=version 2=ts 3=record 4=value 5=hasValue("1"/"0") -// 6=cid 7=key 8=uid +// 6=cid 7=key 8=uid 9=tombstone("1"/"0") +// +// A tombstone still writes the cell — that is the CRDT state, and dropping it +// would let an older write resurrect the value — but it SREMs the cid from the +// user+key index. That index is what `active_for_user_key` returns, so leaving +// deleted cids in it means every My List / Continue Watching read carries every +// title ever un-liked or finished, forever. The other three indexes are left +// alone: they answer "who has an opinion about this cid", where a tombstone is +// still an answer. var udlUpsertScript = redis.NewScript(` local stored = redis.call('HGET', KEYS[1], 'version') if stored and tonumber(stored) >= tonumber(ARGV[1]) then @@ -84,7 +92,11 @@ if ARGV[5] == '1' then else redis.call('HDEL', KEYS[1], 'value') end -redis.call('SADD', KEYS[2], ARGV[6]) +if ARGV[9] == '1' then + redis.call('SREM', KEYS[2], ARGV[6]) +else + redis.call('SADD', KEYS[2], ARGV[6]) +end redis.call('SADD', KEYS[3], ARGV[7]) redis.call('SADD', KEYS[4], ARGV[8]) redis.call('SADD', KEYS[5], ARGV[8]) @@ -94,8 +106,10 @@ return 1 // UDLUpsertIfNewer stores a record cell iff version strictly exceeds the stored // one. publicValue/hasValue carry the plaintext JSON scalar for public keys // (used by aggregation); pass hasValue=false for private-tier keys and -// tombstones. Returns accepted=false when the write is stale. -func (c *Client) UDLUpsertIfNewer(uid, cid, key string, version, ts int64, record, publicValue string, hasValue bool) (bool, error) { +// tombstones. Set tombstone=true when the record deletes the cell, which also +// prunes the cid from the user+key index. Returns accepted=false when the write +// is stale. +func (c *Client) UDLUpsertIfNewer(uid, cid, key string, version, ts int64, record, publicValue string, hasValue, tombstone bool) (bool, error) { c.mu.Lock() defer c.mu.Unlock() @@ -117,7 +131,11 @@ func (c *Client) UDLUpsertIfNewer(uid, cid, key string, version, ts int64, recor if hasValue { hv = "1" } - res, err := udlUpsertScript.Run(ctx, c.client, keys, version, ts, record, publicValue, hv, cid, key, uid).Result() + tomb := "0" + if tombstone { + tomb = "1" + } + res, err := udlUpsertScript.Run(ctx, c.client, keys, version, ts, record, publicValue, hv, cid, key, uid, tomb).Result() if err != nil { return false, fmt.Errorf("udl upsert failed: %w", err) } diff --git a/internal/storage/udl_test.go b/internal/storage/udl_test.go new file mode 100644 index 0000000..6a9a952 --- /dev/null +++ b/internal/storage/udl_test.go @@ -0,0 +1,79 @@ +package storage + +import ( + "testing" +) + +// A tombstone must leave the cell in place but drop the cid from the user+key +// index. +// +// The cell has to stay: it *is* the CRDT state, and deleting it would let an +// older write resurrect the value the tombstone retracted. The index entry has +// to go: `active_for_user_key` returns every member of that set, so a set that +// only ever grows means each My List / Continue Watching read carries every +// title the user has ever un-liked or finished — with its full record blob — +// forever. The reader filters them out again, so the cost is invisible until +// the account is old. +func TestUDLUpsert_TombstonePrunesUserKeyIndexButKeepsCell(t *testing.T) { + c, _ := newTestClient(t, "") + + const ( + uid = "zAlice" + cid = "bagcsaaalocator" + key = "seek" + ) + + // A live position: indexed, readable. + if ok, err := c.UDLUpsertIfNewer(uid, cid, key, 1, 100, "rec-v1", "", false, false); err != nil || !ok { + t.Fatalf("live upsert: ok=%v err=%v", ok, err) + } + entries, err := c.UDLListUserKey(uid, key) + if err != nil { + t.Fatalf("active_for_user_key: %v", err) + } + if len(entries) != 1 || entries[0].Cid != cid { + t.Fatalf("live position should be indexed, got %+v", entries) + } + + // Finishing it tombstones the seek. + if ok, err := c.UDLUpsertIfNewer(uid, cid, key, 2, 200, "rec-v2-tombstone", "", false, true); err != nil || !ok { + t.Fatalf("tombstone upsert: ok=%v err=%v", ok, err) + } + + entries, err = c.UDLListUserKey(uid, key) + if err != nil { + t.Fatalf("active_for_user_key after tombstone: %v", err) + } + if len(entries) != 0 { + t.Fatalf("tombstoned cid must leave the user+key index, got %+v", entries) + } + + // ...but the cell itself survives, at the tombstone's version. + cell, exists, err := c.UDLGetCell(uid, cid, key) + if err != nil { + t.Fatalf("get cell: %v", err) + } + if !exists { + t.Fatal("tombstone must not delete the cell — an older write could then resurrect the value") + } + if cell.Version != 2 || cell.Record != "rec-v2-tombstone" { + t.Fatalf("cell should hold the tombstone record at v2, got %+v", cell) + } + + // And the high-water mark still rejects the superseded write. + if ok, err := c.UDLUpsertIfNewer(uid, cid, key, 1, 300, "rec-v1-replay", "", false, false); err != nil || ok { + t.Fatalf("stale replay must be rejected: ok=%v err=%v", ok, err) + } + + // Re-watching re-establishes the cell and puts it back in the index. + if ok, err := c.UDLUpsertIfNewer(uid, cid, key, 3, 400, "rec-v3", "", false, false); err != nil || !ok { + t.Fatalf("re-establish upsert: ok=%v err=%v", ok, err) + } + entries, err = c.UDLListUserKey(uid, key) + if err != nil { + t.Fatalf("active_for_user_key after re-establish: %v", err) + } + if len(entries) != 1 || entries[0].Cid != cid { + t.Fatalf("re-established cid must return to the index, got %+v", entries) + } +}