diff --git a/domains/amazon/mock.go b/domains/amazon/mock.go index 3705ec1..283ea7e 100644 --- a/domains/amazon/mock.go +++ b/domains/amazon/mock.go @@ -27,6 +27,13 @@ type ( // touching any row. notifyRetryAfter time.Duration + // pollInterval is the fallback cadence ProcessEvents' main loop + // checks for unprocessed/retry-due events on its own, independent + // of Postgres NOTIFY - a safety net for events that end up in + // mock.shop_amazon_events without ever going through the + // mock.raw_shop_events insert+trigger path that fires NOTIFY. + pollInterval time.Duration + ready chan struct{} readyOnce sync.Once } @@ -48,6 +55,7 @@ const ( eventChannelName = "mock_shop_amazon_event_inserted" defaultNotifyRetryAfter = 30 * time.Second + defaultPollInterval = time.Minute ) func NewMocks(log *logging.Logger, db *pgxpool.Pool) *Mocks { @@ -55,6 +63,7 @@ func NewMocks(log *logging.Logger, db *pgxpool.Pool) *Mocks { log: log, db: db, notifyRetryAfter: defaultNotifyRetryAfter, + pollInterval: defaultPollInterval, ready: make(chan struct{}), } } @@ -72,6 +81,14 @@ func (m *Mocks) WithNotifyRetryAfter(d time.Duration) *Mocks { return m } +// WithPollInterval overrides how often ProcessEvents checks for +// unprocessed/retry-due events on its own, independent of NOTIFY. Mainly +// useful for tests that don't want to wait out the default. +func (m *Mocks) WithPollInterval(d time.Duration) *Mocks { + m.pollInterval = d + return m +} + // Ready returns a channel that's closed once ProcessEvents has registered // its Postgres LISTEN and is actively watching for notifications. Callers // that need to know the reactive path is live - tests in particular - @@ -101,7 +118,9 @@ func (m *Mocks) ProcessEvents(ctx context.Context) error { if !ok { return <-errCh } - case <-time.After(time.Minute): + m.log.Debug("woke up: notification received") + case <-time.After(m.pollInterval): + m.log.Debug("woke up: poll interval elapsed") } } } diff --git a/domains/amazon/mock_test.go b/domains/amazon/mock_test.go index dfd9b1c..6d1b405 100644 --- a/domains/amazon/mock_test.go +++ b/domains/amazon/mock_test.go @@ -288,6 +288,60 @@ func TestProcessUnprocessedEvents_RetriesUnackedNotification(t *testing.T) { } } +// TestProcessEvents_PollFallbackPicksUpRetryDueEvents proves the poll +// branch of ProcessEvents' select actually causes reprocessing, decoupled +// from NOTIFY entirely: after the one real insert (which does fire NOTIFY, +// same as any other test here), nothing ever triggers another notification +// for the rest of the test. The event becomes retry-due almost immediately +// (notifyRetryAfter is tiny), so the *only* way it can be dispatched a +// second time is the poll timer in the select firing on its own. +func TestProcessEvents_PollFallbackPicksUpRetryDueEvents(t *testing.T) { + pool := testdb.Pool(t) + spy := newNotifySpy() + spy.setAutoAck(false) + m := NewMocks(testdb.Logger(), pool). + SetListener(spy). + WithNotifyRetryAfter(30 * time.Millisecond). + WithPollInterval(60 * time.Millisecond) + + shopID := "test-shop-" + uuid.NewString() + cleanupShop(t, pool, shopID) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + errCh := make(chan error, 1) + go func() { + errCh <- m.ProcessEvents(ctx) + }() + + select { + case <-m.Ready(): + case <-time.After(5 * time.Second): + t.Fatal("ProcessEvents() did not become ready (LISTEN registered) within 5s") + } + + insertRawAmazonEvent(t, pool, shopID, "evt-1") + + // first dispatch, via the real NOTIFY. + spy.waitForCount(t, 1, 2*time.Second) + + // second dispatch: nothing will notify again from here on, so this can + // only come from the poll branch of the select waking the loop up on + // its own and finding the event retry-due. + spy.waitForCount(t, 2, 3*time.Second) + + cancel() + select { + case err := <-errCh: + if err != nil { + t.Fatalf("ProcessEvents() returned error = %v after context cancellation, want nil", err) + } + case <-time.After(5 * time.Second): + t.Fatal("ProcessEvents() did not return within 5s of context cancellation") + } +} + // TestProcessEvents_ReactsToNotification drives the actual long-running // loop: LISTEN registration, a real Postgres NOTIFY fired by the DB trigger // on insert, WaitForNotification waking the loop, and the listener callback diff --git a/work-summaries/work-summary-Claude-2026-08-05-2101.md b/work-summaries/work-summary-Claude-2026-08-05-2101.md new file mode 100644 index 0000000..93b6299 --- /dev/null +++ b/work-summaries/work-summary-Claude-2026-08-05-2101.md @@ -0,0 +1,27 @@ +# Work Summary — 2026-08-05 21:01 + +## Task +Insight #3 from the `domains/amazon` design review: the 1-minute poll fallback in `ProcessEvents` was a bare `time.Minute` literal inline in a `select` - untestable without waiting 60+ seconds or refactoring, not tunable without a code change, and with no logging distinguishing whether a wake-up came from a real NOTIFY or the poll timer. + +## Design confirmed with user before implementing +- Deploy-time configurable, not live/API-adjustable - matches how `notifyRetryAfter` (insight #2) already works: a field set once at `Mocks` construction, changed by editing the call site and restarting, not a runtime toggle. No stated need for a live knob, and adding one (mutex-guarded field, endpoint, auth, validation) would be real complexity for a need nobody has. +- Go-level field + builder only, not wired through `.env`/`config.Config` - consistent with `notifyRetryAfter`, which also isn't env-configurable today. + +## Changes +`domains/amazon/mock.go`: +- `Mocks` gained a `pollInterval time.Duration` field (default `time.Minute`, via new `defaultPollInterval` const) and `WithPollInterval(d)` builder, mirroring `WithNotifyRetryAfter`. +- `ProcessEvents`'s `select` now uses `m.pollInterval` instead of the `time.Minute` literal. +- Added a `Debug` log line on each of the two meaningful wake-up branches ("woke up: notification received" / "woke up: poll interval elapsed"), so it's now observable in practice which path is actually firing - previously both looked identical afterward. + +`domains/amazon/mock_test.go`: +- New `TestProcessEvents_PollFallbackPicksUpRetryDueEvents`. Proving the poll branch actually works without waiting 60s - or without any trigger/NOTIFY manipulation that would be unsafe to run against the shared dev DB - needed a bit of care: a direct insert into `mock.shop_amazon_events` isn't possible (FK to `mock.raw_shop_events`), and any insert into `raw_shop_events` for `platform='amazon'` unconditionally fires the trigger's `pg_notify`, so there's no clean way to insert an event that's guaranteed to never notify. Instead, the test reuses insight #2's retry mechanism: one real event is inserted (fires NOTIFY normally, dispatched once), the listener never acks it, and with `notifyRetryAfter` set smaller than `pollInterval`, the event becomes retry-due almost immediately - so the *only* thing that can cause a second dispatch, since nothing else ever notifies again for the rest of the test, is the poll timer in the `select` firing on its own. Confirms the mechanism cleanly and safely (no shared state touched beyond the test's own rows). + +## Verification +- `go build ./...` / `go vet ./...` clean. +- Manually confirmed the new log lines actually fire as expected: a small standalone program (not part of the repo, written to the scratchpad and deleted after) run against the test DB with `WithPollInterval(60ms)` printed `"woke up: poll interval elapsed"` on a steady ~60ms cadence. +- `go test ./domains/amazon/... -v -race`: all 6 tests pass (5 existing + 1 new). +- 10x repeated runs (`-count=1 -race`) with no flakes, ~1.4-1.5s each. +- `make test` / `make test-against-dev-db`: full suite green both ways; zero leftover rows in the dev DB afterward. + +## Follow-ups / not done here +- Insight #4 (a LISTEN-connection error is fatal to the *entire application*, not just this processor - the most consequential one, given eleven more platforms already share this trigger shape in the migrations) remains open, next in line per the user's one-at-a-time request.