diff --git a/pkg/hold/gc/predecessor_test.go b/pkg/hold/gc/predecessor_test.go new file mode 100644 index 0000000..5d5dc65 --- /dev/null +++ b/pkg/hold/gc/predecessor_test.go @@ -0,0 +1,136 @@ +package gc + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "testing" + + "atcr.io/pkg/atproto" +) + +// captainResponse renders the getRecord envelope a hold returns for its own +// captain record, with the successor field set to whatever the caller wants. +func captainResponse(t *testing.T, successor string) string { + t.Helper() + captain := atproto.CaptainRecord{ + Type: atproto.CaptainCollection, + Owner: "did:plc:owner", + Public: true, + Successor: successor, + } + value, err := json.Marshal(captain) + if err != nil { + t.Fatalf("marshal captain: %v", err) + } + envelope, err := json.Marshal(map[string]json.RawMessage{"value": value}) + if err != nil { + t.Fatalf("marshal envelope: %v", err) + } + return string(envelope) +} + +// TestCheckPredecessorAt_InconclusiveOnTransportFailure pins the distinction +// 95d4f7c introduced: a hold that could not be asked is not a hold that +// answered "no". Every one of these paths used to return a bare false, which +// isPredecessorHold recorded in a process-lifetime cache — so a single blip +// dropped that hold's manifests out of the referenced set and deleted blobs +// this hold is still serving on the predecessor's behalf. +// +// definitive must be false for all of them. isPredecessor is false too, but it +// is the definitive flag that decides whether the answer is allowed to be +// cached, and therefore the flag that carries the blob loss. +func TestCheckPredecessorAt_InconclusiveOnTransportFailure(t *testing.T) { + tests := []struct { + name string + // holdURL returns the URL to probe. Each case gets its own server so a + // closed listener cannot disturb the others. + holdURL func(t *testing.T) string + }{ + { + name: "hold answers 500", + holdURL: func(t *testing.T) string { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + })) + t.Cleanup(srv.Close) + return srv.URL + }, + }, + { + name: "listener closed, connection refused", + holdURL: func(t *testing.T) string { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})) + url := srv.URL + // Close before probing: the port is real but nothing is on it, + // which is what a stopped hold looks like from here. + srv.Close() + return url + }, + }, + { + name: "200 with a body that is not JSON", + holdURL: func(t *testing.T) string { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprint(w, "502 Bad Gateway") + })) + t.Cleanup(srv.Close) + return srv.URL + }, + }, + { + name: "200 with a valid envelope wrapping a garbage record", + holdURL: func(t *testing.T) string { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprint(w, `{"value":"not-an-object"}`) + })) + t.Cleanup(srv.Close) + return srv.URL + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gc := &GarbageCollector{logger: newTestLogger()} + + isPredecessor, definitive := gc.checkPredecessorAt( + context.Background(), "did:web:predecessor.example.com", tt.holdURL(t)) + + if definitive { + t.Errorf("definitive = true for an unanswered probe: the negative gets cached "+ + "for the life of the process and this hold's blobs are deleted (isPredecessor=%v)", + isPredecessor) + } + if isPredecessor { + t.Errorf("isPredecessor = true, want false on a failed probe") + } + }) + } +} + +// TestAnalyzeRecordsClearsPredecessorUnresolved pins the reset documented on +// the field itself. predecessorUnresolved exists so one unreachable hold costs +// a single timeout per run rather than one per manifest. If the clear is ever +// dropped, the first outage becomes permanent: every later run short-circuits +// on the stale entry, treats that hold's manifests as referenced forever, and +// GC quietly stops reclaiming anything those manifests touch. +func TestAnalyzeRecordsClearsPredecessorUnresolved(t *testing.T) { + gc, _, ctx := newRegressionGC(t) + + const staleHold = "did:web:was-down-last-night.example.com" + gc.predecessorUnresolved = map[string]bool{staleHold: true} + + // An empty hold discovers no users, so this returns almost immediately. The + // clear happens at the top of the function, before any of that work. + if _, err := gc.analyzeRecords(ctx); err != nil { + t.Fatalf("analyzeRecords: %v", err) + } + + if gc.predecessorUnresolved[staleHold] { + t.Error("predecessorUnresolved survived analyzeRecords: a hold that was down once " + + "is never re-checked, and GC stops reclaiming its manifests' blobs") + } +}