amadan

oficina / docs Public

Seed sample documents per person, on person creation, not per instance

Documents are owner-scoped and a boot-time seed has no person to own
anything, so seeding moves from an instance-wide, boot-time event
(the -seed flag, still gated to -env development for local dev) to a
per-person one, hooked into Store.EnsurePerson's create branch only.

Tying it to "this Person row was just created" rather than "this
person's document list is empty" matters: a person who deletes every
sample document keeps their Person row, so their next sign-in takes
EnsurePerson's found branch and gets nothing resurrected. Opt-in via
$DOCS_SAMPLE_DOCUMENTS (default off, parsed permissively rather than
fail-closed — this only gates cosmetic content, not who can sign in).

Seeding runs after the person row's INSERT has already committed, not
inside any transaction around it: it calls into WebAssembly per
document and must never hold locks against, or roll back, the
person's own row. A seeding failure is logged and swallowed there,
never surfaced as an EnsurePerson error — a person who can't sign in
because sample content failed to generate is worse than one with an
empty list. EnsurePerson's existing duplicate-key race handling is
unchanged, so only one concurrent first sign-in ever seeds.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011F5CivVm4CmL9ByfSSAaXA
Paul Campbell pushed by paul@keymail.dev 854648b2c22d491fdd6d5c9c1f9dae208de356db parent 43ca2e2
7 files changed, +580 −21
  • cmd/docs/main.go +26 −5
  • internal/docs/app.go +55 −1
  • internal/docs/sample_documents_internal_test.go +203 −0
  • internal/docs/sample_documents_test.go +189 −0
  • internal/docs/seed.go +18 −9
  • internal/docs/seed_test.go +3 −3
  • internal/docs/store.go +86 −3

Intent

full ledger: samples-for-new-people

diff --git a/cmd/docs/main.go b/cmd/docs/main.go
index 3780808..7656c6a 100644
--- a/cmd/docs/main.go
+++ b/cmd/docs/main.go
@@ -229,6 +229,17 @@ func main() {
// would otherwise silently mean.
allowedAddresses := os.Getenv("DOCS_ALLOWED_ADDRESSES")
+ // $DOCS_SAMPLE_DOCUMENTS opts a deployed instance into
+ // docs.Config.SampleDocuments — see its own doc comment for the
+ // full reasoning (parsed permissively, default off, deliberately
+ // not fail-closed like the settings just above). This is the
+ // platform-reachable fix for the bug this branch exists to close:
+ // -seed below never runs on a deployed instance (gated to -env
+ // development, which activation argv never sets), so without this
+ // env var a real deployment's sample content stays exactly as
+ // unreachable as it was before.
+ sampleDocuments := os.Getenv("DOCS_SAMPLE_DOCUMENTS")
+
// chooseEnv: -env wins; $DOCS_ENV wins next EXCEPT for
// "development" itself, which chooseEnv never honours from an env
// var (fix round 2 — see its own doc comment); the default with
@@ -258,6 +269,7 @@ func main() {
InstanceKey: instanceKey,
Mailer: mailer,
AllowedAddresses: allowedAddresses,
+ SampleDocuments: sampleDocuments,
Logger: logger,
}
app, err := docs.New(cfg)
@@ -268,21 +280,30 @@ func main() {
defer app.Close()
// Only reachable with both -seed and -env development (the check
- // above) — after New, since docs.Seed needs a real *docs.App with
- // its store and migrated database already in place, and after the
- // InstanceKey/Mailer checks above, so a database that cannot even
- // send a real sign-in link never gets seeded either.
+ // above) — after New, since both EnsurePerson and docs.Seed need a
+ // real app.Store() over an already-migrated database, and after
+ // the InstanceKey/Mailer checks above, so a database that cannot
+ // even send a real sign-in link never gets seeded either.
if seedFlag {
seedAddress := strings.TrimSpace(os.Getenv("DOCS_SEED_ADDRESS"))
if seedAddress == "" {
seedAddress = devSeedAddress
}
+ // EnsurePerson here runs with cfg.SampleDocuments as given on
+ // this invocation: if it's also truthy, EnsurePerson's own
+ // per-person seeding (store.go) already seeds seedAddress the
+ // moment its Person row is first created, before this call
+ // even returns — see its doc comment. The explicit docs.Seed
+ // call just below runs unconditionally regardless, exactly as
+ // it always has: -seed's whole point is "seed this address
+ // right now, guaranteed", not "seed it only if some other
+ // setting happens to agree."
owner, err := app.Store().EnsurePerson(context.Background(), seedAddress)
if err != nil {
logger.Error("docs: seed: resolve person", "address", seedAddress, "err", err)
os.Exit(1)
}
- if err := docs.Seed(context.Background(), app, owner); err != nil {
+ if err := docs.Seed(context.Background(), app.Store(), owner); err != nil {
logger.Error("docs: seed", "address", seedAddress, "err", err)
os.Exit(1)
}
diff --git a/internal/docs/app.go b/internal/docs/app.go
index 5be3dfe..eb2b6da 100644
--- a/internal/docs/app.go
+++ b/internal/docs/app.go
@@ -160,10 +160,64 @@ type Config struct {
// alternative of silently admitting nobody.
AllowedAddresses string
+ // SampleDocuments opts this instance into seeding a brand-new
+ // person's very first sign-in with CARLOS Docs' own sample
+ // documents (seed.go's Seed, six of them) — the fix for "Paul
+ // signed in to a live instance and saw an empty list": the sample
+ // documents docs.Seed already wrote were never reachable on a
+ // deployed instance, because -seed is gated to Env ==
+ // "development" (main.go's own doc comment, unchanged by this
+ // field) and a deployed instance never runs with that flag.
+ // Documents are owner-scoped (Document.OwnerID, owned() in
+ // store.go) and a boot-time seed has no person to own anything, so
+ // this switches seeding from a boot-time, instance-wide event to a
+ // per-person one — see Store.EnsurePerson's own doc comment for
+ // exactly where that happens and why there, not "the list is
+ // empty" (a person who deletes every sample document must not get
+ // them back).
+ //
+ // Sourced from $DOCS_SAMPLE_DOCUMENTS (cmd/docs/main.go), the same
+ // "<APP>_..." convention every other setting on this struct uses.
+ // Parsed by parseSampleDocuments below, which accepts a small,
+ // case-insensitive truthy set ("1", "true", "yes") and treats
+ // everything else — unset, empty, or a typo alike — as off.
+ // Default OFF: a real team instance must never get six invented
+ // documents about a reading room and an allotment committee just
+ // because nobody thought to unset something.
+ //
+ // Deliberately NOT fail-closed the way Origin/InstanceKey/
+ // AllowedAddresses above are: those three guard who can sign in
+ // and what seals a session, where a value New cannot make sense of
+ // silently taking the permissive branch would be a real security
+ // regression, so New refuses to start rather than guess. This
+ // setting only decides whether a new person's document list starts
+ // with six sample documents or zero — cosmetic content, not a
+ // security control — so an unparseable value here is read as "off"
+ // and New boots anyway. A platform whose env var block has a typo
+ // in $DOCS_SAMPLE_DOCUMENTS should get an empty list, silently, the
+ // same outcome as never having set it at all, not a refusal to
+ // serve any traffic over a setting that was never load-bearing for
+ // safety in the first place.
+ SampleDocuments string
+
// Logger defaults to slog.Default().
Logger *slog.Logger
}
+// parseSampleDocuments interprets raw the way Config.SampleDocuments'
+// own doc comment describes: a small, case-insensitive truthy set
+// means on, and everything else means off. See that doc comment for
+// why this is deliberately not fail-closed like resolveInstanceKey/
+// resolveAllowlist (auth.go) just below use for their own settings.
+func parseSampleDocuments(raw string) bool {
+ switch strings.ToLower(strings.TrimSpace(raw)) {
+ case "1", "true", "yes":
+ return true
+ default:
+ return false
+ }
+}
+
// DefaultOrigin is Config.Origin's fallback when empty — see its doc
// comment. Exported so a caller that needs to know what origin this
// app is protecting itself against (NewForTest's callers, notably)
@@ -323,7 +377,7 @@ func New(cfg Config) (*App, error) {
d.Close()
return nil, fmt.Errorf("docs: start core runtime: %w", err)
}
- store := NewStore(d.G, rt)
+ store := NewStore(d.G, rt, parseSampleDocuments(cfg.SampleDocuments), logger)
render, err := newRenderer()
if err != nil {
diff --git a/internal/docs/sample_documents_internal_test.go b/internal/docs/sample_documents_internal_test.go
new file mode 100644
index 0000000..eaa0b9d
--- /dev/null
+++ b/internal/docs/sample_documents_internal_test.go
@@ -0,0 +1,203 @@
+package docs
+
+import (
+ "context"
+ "path/filepath"
+ "sync"
+ "testing"
+ "time"
+
+ "github.com/carlosframework/docs/internal/core"
+)
+
+// newSampleDocumentsInternalTestApp is this file's own copy of
+// sample_documents_test.go's newSampleDocumentsTestApp: that one
+// lives in package docs_test and cannot be reused from here, and the
+// tests below need package-internal access (a *Store's own db handle,
+// to soft-delete rows the way a future "delete document" handler
+// would; a second, deliberately broken *core.Runtime) that only a
+// docs-package test can reach.
+func newSampleDocumentsInternalTestApp(t *testing.T, sampleDocuments string) *App {
+ t.Helper()
+ app, err := New(Config{
+ DBPath: filepath.Join(t.TempDir(), "docs.db"),
+ Env: "development",
+ SampleDocuments: sampleDocuments,
+ })
+ if err != nil {
+ t.Fatalf("New: %v", err)
+ }
+ t.Cleanup(func() {
+ if cerr := app.Close(); cerr != nil {
+ t.Errorf("App.Close: %v", cerr)
+ }
+ })
+ return app
+}
+
+// TestPersonWhoDeletesSampleDocumentsGetsNoneBackOnNextSignIn is the
+// property EnsurePerson's own doc comment calls out by name: seeding
+// is tied to "this Person row was just created", never to "this
+// person's list happens to be empty". There is no delete handler yet
+// (spec's later work), so this reaches into store.db directly to
+// soft-delete every one of the seeded rows exactly the way such a
+// handler would (setting deleted_at, the same column List's own
+// "deleted_at IS NULL" filter checks) — then signs the same address
+// in again and asserts nothing comes back.
+func TestPersonWhoDeletesSampleDocumentsGetsNoneBackOnNextSignIn(t *testing.T) {
+ app := newSampleDocumentsInternalTestApp(t, "1")
+ ctx := context.Background()
+ store := app.Store()
+
+ owner, err := store.EnsurePerson(ctx, "deletes-everything@example.com")
+ if err != nil {
+ t.Fatalf("EnsurePerson (first): %v", err)
+ }
+ first, err := store.List(ctx, owner)
+ if err != nil {
+ t.Fatalf("List (first): %v", err)
+ }
+ if len(first) != 6 {
+ t.Fatalf("got %d seeded documents, want 6", len(first))
+ }
+
+ now := time.Now().UTC()
+ if err := owned(store.db.WithContext(ctx), owner).
+ Model(&Document{}).
+ Update("deleted_at", now).Error; err != nil {
+ t.Fatalf("soft-delete owner's documents: %v", err)
+ }
+
+ afterDelete, err := store.List(ctx, owner)
+ if err != nil {
+ t.Fatalf("List (after delete): %v", err)
+ }
+ if len(afterDelete) != 0 {
+ t.Fatalf("got %d documents after deleting all of them, want 0", len(afterDelete))
+ }
+
+ // The person row already exists, so this hits EnsurePerson's
+ // found branch, not its create branch — seedIfEnabled is never
+ // called, and List must stay at zero.
+ ownerAgain, err := store.EnsurePerson(ctx, "deletes-everything@example.com")
+ if err != nil {
+ t.Fatalf("EnsurePerson (second): %v", err)
+ }
+ if ownerAgain != owner {
+ t.Fatalf("EnsurePerson(again) = %d, want %d", ownerAgain, owner)
+ }
+
+ after, err := store.List(ctx, owner)
+ if err != nil {
+ t.Fatalf("List (after second sign-in): %v", err)
+ }
+ if len(after) != 0 {
+ t.Fatalf("got %d documents back after signing in again, want 0 — sample documents were resurrected", len(after))
+ }
+}
+
+// TestEnsurePersonSurvivesSeedingFailure is the care point the task
+// calls out explicitly: seeding failure must never break sign-in. To
+// force Seed itself to fail without touching the Person row
+// EnsurePerson creates first (which would defeat the point of the
+// test — the person must exist BEFORE seeding is even attempted),
+// this builds a second, deliberately broken *core.Runtime — closed
+// immediately after it compiles — and hands it to a Store sharing the
+// real, working database underneath app's own Store.
+//
+// Confirmed directly (see the probe this test's design was checked
+// against): calling a wazero module method through a Runtime whose
+// underlying wazero runtime has been Close'd returns a clean error
+// ("runtime closed with exit_code(0)"), not a panic — Store.replay's
+// s.core.NewDoc call, the first thing Seed's loop does for its first
+// document's ApplyUpdate, fails this way every time, deterministically,
+// with no flakiness from timing.
+func TestEnsurePersonSurvivesSeedingFailure(t *testing.T) {
+ app := newSampleDocumentsInternalTestApp(t, "1")
+ ctx := context.Background()
+
+ brokenCore, err := core.New(ctx)
+ if err != nil {
+ t.Fatalf("core.New: %v", err)
+ }
+ if err := brokenCore.Close(ctx); err != nil {
+ t.Fatalf("brokenCore.Close: %v", err)
+ }
+
+ broken := NewStore(app.store.db, brokenCore, true, app.logger)
+
+ id, err := broken.EnsurePerson(ctx, "seed-fails@example.com")
+ if err != nil {
+ t.Fatalf("EnsurePerson must succeed even when seeding fails, got: %v", err)
+ }
+ if id <= 0 {
+ t.Fatalf("EnsurePerson returned a non-positive id: %d", id)
+ }
+
+ // The person is durably there: a fresh EnsurePerson call for the
+ // same address finds the same row rather than creating (or
+ // re-seeding) it.
+ id2, err := broken.EnsurePerson(ctx, "seed-fails@example.com")
+ if err != nil {
+ t.Fatalf("EnsurePerson (again): %v", err)
+ }
+ if id2 != id {
+ t.Fatalf("EnsurePerson(again) = %d, want %d (same person, not recreated)", id2, id)
+ }
+
+ // And the same person, looked up through the app's own working
+ // Store, is a real, usable person: List works and returns without
+ // error (whatever partial content Seed managed before it failed —
+ // this asserts "does not break", not "produced zero documents",
+ // since Seed's per-document loop may have left one partial row
+ // behind before its first ApplyUpdate failed).
+ if _, err := app.Store().List(ctx, id); err != nil {
+ t.Fatalf("List for the person seeding failed for: %v", err)
+ }
+}
+
+// TestConcurrentFirstSignInsProduceOnePersonAndOneSampleSet is the
+// concurrency requirement stated explicitly: two simultaneous first
+// sign-ins for the same brand-new address must not produce two
+// people or two sets of documents. EnsurePerson's own duplicate-key
+// race handling (store.go) already guarantees one Person row; this
+// pins that seeding rides along with it — exactly one winner reaches
+// seedIfEnabled, so the address ends up with exactly six documents,
+// never twelve.
+func TestConcurrentFirstSignInsProduceOnePersonAndOneSampleSet(t *testing.T) {
+ app := newSampleDocumentsInternalTestApp(t, "1")
+ store := app.Store()
+ const address = "concurrent-first-signin@example.com"
+
+ const n = 8
+ ids := make([]int64, n)
+ errs := make([]error, n)
+ var wg sync.WaitGroup
+ wg.Add(n)
+ for i := 0; i < n; i++ {
+ go func(i int) {
+ defer wg.Done()
+ ids[i], errs[i] = store.EnsurePerson(context.Background(), address)
+ }(i)
+ }
+ wg.Wait()
+
+ for i, err := range errs {
+ if err != nil {
+ t.Fatalf("EnsurePerson[%d]: %v", i, err)
+ }
+ }
+ for i := 1; i < n; i++ {
+ if ids[i] != ids[0] {
+ t.Fatalf("EnsurePerson returned different ids across concurrent calls: %d and %d — two people were created for one address", ids[0], ids[i])
+ }
+ }
+
+ list, err := store.List(context.Background(), ids[0])
+ if err != nil {
+ t.Fatalf("List: %v", err)
+ }
+ if len(list) != 6 {
+ t.Fatalf("got %d documents after %d concurrent first sign-ins, want exactly 6 (one seeded set, not %d)", len(list), n, n*6)
+ }
+}
diff --git a/internal/docs/sample_documents_test.go b/internal/docs/sample_documents_test.go
new file mode 100644
index 0000000..72f10fd
--- /dev/null
+++ b/internal/docs/sample_documents_test.go
@@ -0,0 +1,189 @@
+package docs_test
+
+import (
+ "context"
+ "path/filepath"
+ "strings"
+ "testing"
+
+ "github.com/carlosframework/docs/internal/docs"
+)
+
+// newSampleDocumentsTestApp builds a development App exactly like
+// docs.NewForTest, except with Config.SampleDocuments set explicitly
+// — the same "build directly via New, package-internal test helper"
+// shape auth_allowlist_test.go's newAllowlistedTestApp uses for the
+// same reason: NewForTest itself never sets this, so any test that
+// wants seeding on (or explicitly off) has to build its own App.
+func newSampleDocumentsTestApp(t *testing.T, sampleDocuments string) *docs.App {
+ t.Helper()
+ app, err := docs.New(docs.Config{
+ DBPath: filepath.Join(t.TempDir(), "docs.db"),
+ Env: "development",
+ SampleDocuments: sampleDocuments,
+ })
+ if err != nil {
+ t.Fatalf("New: %v", err)
+ }
+ t.Cleanup(func() {
+ if cerr := app.Close(); cerr != nil {
+ t.Errorf("App.Close: %v", cerr)
+ }
+ })
+ return app
+}
+
+// TestNewPersonWithSampleDocumentsOnGetsExactlySix pins the design
+// this branch exists to ship: a brand-new person's very first
+// EnsurePerson call, on an instance that opted in via
+// $DOCS_SAMPLE_DOCUMENTS, gets seed.go's own sample set — exactly six
+// documents (len(seedDocs) in seed.go), owned by them, each with real,
+// substantial text read back through the actual CRDT (Store.Text),
+// never a row count standing in for content that was never applied.
+func TestNewPersonWithSampleDocumentsOnGetsExactlySix(t *testing.T) {
+ app := newSampleDocumentsTestApp(t, "1")
+ ctx := context.Background()
+
+ owner, err := app.Store().EnsurePerson(ctx, "brand-new@example.com")
+ if err != nil {
+ t.Fatalf("EnsurePerson: %v", err)
+ }
+
+ list, err := app.Store().List(ctx, owner)
+ if err != nil {
+ t.Fatalf("List: %v", err)
+ }
+ if len(list) != 6 {
+ t.Fatalf("got %d documents, want exactly 6", len(list))
+ }
+
+ for _, d := range list {
+ text, err := app.Store().Text(ctx, d)
+ if err != nil {
+ t.Fatalf("Text(%q): %v", d.Title, err)
+ }
+ if len(strings.TrimSpace(text)) < 200 {
+ t.Errorf("%q has %d characters of text — want substantial, real content", d.Title, len(text))
+ }
+ if d.OwnerID != owner {
+ t.Errorf("%q has OwnerID %d, want %d", d.Title, d.OwnerID, owner)
+ }
+ }
+}
+
+// TestNewPersonWithSampleDocumentsOffGetsNone is the opt-in's default:
+// with $DOCS_SAMPLE_DOCUMENTS unset (SampleDocuments left "" here,
+// exactly what a real deployment gets if nobody has set the variable
+// at all), a brand-new person's list starts empty — the pre-existing,
+// unchanged behaviour every real team instance must keep.
+func TestNewPersonWithSampleDocumentsOffGetsNone(t *testing.T) {
+ app := newSampleDocumentsTestApp(t, "")
+ ctx := context.Background()
+
+ owner, err := app.Store().EnsurePerson(ctx, "brand-new@example.com")
+ if err != nil {
+ t.Fatalf("EnsurePerson: %v", err)
+ }
+
+ list, err := app.Store().List(ctx, owner)
+ if err != nil {
+ t.Fatalf("List: %v", err)
+ }
+ if len(list) != 0 {
+ t.Fatalf("got %d documents with SampleDocuments off, want 0", len(list))
+ }
+}
+
+// TestExistingPersonSigningInAgainGetsNoNewDocuments is the seam this
+// whole design hangs off: EnsurePerson's create branch seeds, its
+// found branch (Person row already exists) never does. Calling
+// EnsurePerson a second time for the same address must not grow the
+// document count at all, whether or not sample content already
+// landed on the first call.
+func TestExistingPersonSigningInAgainGetsNoNewDocuments(t *testing.T) {
+ app := newSampleDocumentsTestApp(t, "true")
+ ctx := context.Background()
+
+ owner, err := app.Store().EnsurePerson(ctx, "returning@example.com")
+ if err != nil {
+ t.Fatalf("EnsurePerson (first): %v", err)
+ }
+ first, err := app.Store().List(ctx, owner)
+ if err != nil {
+ t.Fatalf("List (first): %v", err)
+ }
+ if len(first) != 6 {
+ t.Fatalf("got %d documents after first sign-in, want 6", len(first))
+ }
+
+ ownerAgain, err := app.Store().EnsurePerson(ctx, "returning@example.com")
+ if err != nil {
+ t.Fatalf("EnsurePerson (second): %v", err)
+ }
+ if ownerAgain != owner {
+ t.Fatalf("EnsurePerson(again) = %d, want the same person id %d", ownerAgain, owner)
+ }
+
+ second, err := app.Store().List(ctx, owner)
+ if err != nil {
+ t.Fatalf("List (second): %v", err)
+ }
+ if len(second) != len(first) {
+ t.Fatalf("document count grew from %d to %d on a repeat sign-in", len(first), len(second))
+ }
+}
+
+// TestTwoPeopleEachGetTheirOwnSampleSet checks seeding does not
+// undermine owned() (store.go): two different brand-new people each
+// get their own full set of six, and List for one never includes a
+// row belonging to the other — the same ownership guarantee
+// TestOwnedScopesByOwner (scope_test.go) pins for hand-created
+// documents, exercised here through the seeding path instead.
+func TestTwoPeopleEachGetTheirOwnSampleSet(t *testing.T) {
+ app := newSampleDocumentsTestApp(t, "yes")
+ ctx := context.Background()
+
+ alice, err := app.Store().EnsurePerson(ctx, "alice-sample@example.com")
+ if err != nil {
+ t.Fatalf("EnsurePerson(alice): %v", err)
+ }
+ bob, err := app.Store().EnsurePerson(ctx, "bob-sample@example.com")
+ if err != nil {
+ t.Fatalf("EnsurePerson(bob): %v", err)
+ }
+ if alice == bob {
+ t.Fatalf("alice and bob resolved to the same person id %d", alice)
+ }
+
+ aliceDocs, err := app.Store().List(ctx, alice)
+ if err != nil {
+ t.Fatalf("List(alice): %v", err)
+ }
+ bobDocs, err := app.Store().List(ctx, bob)
+ if err != nil {
+ t.Fatalf("List(bob): %v", err)
+ }
+ if len(aliceDocs) != 6 {
+ t.Fatalf("alice has %d documents, want 6", len(aliceDocs))
+ }
+ if len(bobDocs) != 6 {
+ t.Fatalf("bob has %d documents, want 6", len(bobDocs))
+ }
+
+ for _, d := range aliceDocs {
+ if d.OwnerID != alice {
+ t.Errorf("alice's List returned a document owned by %d, not %d", d.OwnerID, alice)
+ }
+ if _, err := app.Store().Get(ctx, bob, d.Sqid); err == nil {
+ t.Errorf("bob can Get alice's document %q — owned() is not isolating seeded documents", d.Sqid)
+ }
+ }
+ for _, d := range bobDocs {
+ if d.OwnerID != bob {
+ t.Errorf("bob's List returned a document owned by %d, not %d", d.OwnerID, bob)
+ }
+ if _, err := app.Store().Get(ctx, alice, d.Sqid); err == nil {
+ t.Errorf("alice can Get bob's document %q — owned() is not isolating seeded documents", d.Sqid)
+ }
+ }
+}
diff --git a/internal/docs/seed.go b/internal/docs/seed.go
index fd608af..7a4dbc4 100644
--- a/internal/docs/seed.go
+++ b/internal/docs/seed.go
@@ -61,15 +61,24 @@ var seedDocs = []seedDoc{
// exactly the route the wire-compatibility gate's own fixtures
// already use (ReadFixture, testing.go).
//
-// It is meant for a fresh, empty database — cmd/docs/main.go's -seed
-// flag, gated to Env == "development", is the only caller — and does
-// not check for or skip existing documents: calling it twice against
-// the same owner seeds the sample set twice over. That is a feature
-// for reasoning about, not a safety net around; the safety belongs at
-// the caller, not here (main.go's own doc comment on why -seed is
-// gated the way it is).
-func Seed(ctx context.Context, app *App, owner int64) error {
- store := app.Store()
+// It is meant for a fresh, empty owner and does not check for or skip
+// existing documents: calling it twice against the same owner seeds
+// the sample set twice over. That is a feature for reasoning about,
+// not a safety net around; the safety belongs at the caller, not
+// here. There are exactly two callers, each with its own safety
+// story: cmd/docs/main.go's -seed flag, gated to Env == "development"
+// (main.go's own doc comment on why); and Store.EnsurePerson's
+// seedIfEnabled (store.go), called at most once per person, ever, on
+// the one event in a person's history that can only happen once —
+// their row not existing yet (seedIfEnabled's own doc comment).
+//
+// Takes a *Store rather than a *App: this is the same store either
+// caller already has (app.Store(), or the store method's own s), and
+// a *Store has no need for anything else the App wraps — the router,
+// the mailer, the auth plugin. Store.EnsurePerson calling this
+// directly, in-package, is what makes per-person seeding possible at
+// all without a Store depending back on the *App that holds it.
+func Seed(ctx context.Context, store *Store, owner int64) error {
base := time.Now().UTC()
for _, sd := range seedDocs {
diff --git a/internal/docs/seed_test.go b/internal/docs/seed_test.go
index 5c84f04..1c11da6 100644
--- a/internal/docs/seed_test.go
+++ b/internal/docs/seed_test.go
@@ -17,7 +17,7 @@ func TestSeedProducesReadableDocuments(t *testing.T) {
ctx := context.Background()
owner := app.TestPerson(t, "seed@example.com")
- if err := docs.Seed(ctx, app, owner); err != nil {
+ if err := docs.Seed(ctx, app.Store(), owner); err != nil {
t.Fatalf("Seed: %v", err)
}
@@ -54,7 +54,7 @@ func TestSeedVariesTimestamps(t *testing.T) {
ctx := context.Background()
owner := app.TestPerson(t, "seed@example.com")
- if err := docs.Seed(ctx, app, owner); err != nil {
+ if err := docs.Seed(ctx, app.Store(), owner); err != nil {
t.Fatalf("Seed: %v", err)
}
@@ -86,7 +86,7 @@ func TestSeedContentMatchesItsSource(t *testing.T) {
ctx := context.Background()
owner := app.TestPerson(t, "source-check@example.com")
- if err := docs.Seed(ctx, app, owner); err != nil {
+ if err := docs.Seed(ctx, app.Store(), owner); err != nil {
t.Fatalf("Seed: %v", err)
}
diff --git a/internal/docs/store.go b/internal/docs/store.go
index fde79d1..a4a3cff 100644
--- a/internal/docs/store.go
+++ b/internal/docs/store.go
@@ -5,6 +5,7 @@ import (
crand "crypto/rand"
"errors"
"fmt"
+ "log/slog"
mrand "math/rand/v2"
"strings"
"time"
@@ -21,6 +22,17 @@ import (
type Store struct {
db *gorm.DB
core *core.Runtime
+
+ // sampleDocuments mirrors Config.SampleDocuments, already parsed —
+ // see EnsurePerson's own doc comment for what it gates and why the
+ // gate lives here rather than at the HTTP layer.
+ sampleDocuments bool
+
+ // logger is where a sample-seeding failure goes — see
+ // seedIfEnabled's own doc comment for why that failure is logged
+ // and swallowed here rather than ever returned to EnsurePerson's
+ // caller.
+ logger *slog.Logger
}
// NewStore builds a Store over an already-migrated *gorm.DB and a
@@ -28,8 +40,20 @@ type Store struct {
// and is reused for every document (core.New's doc comment); each
// call into it below opens its own per-document instance (a
// *core.Handle) and closes it again.
-func NewStore(db *gorm.DB, rt *core.Runtime) *Store {
- return &Store{db: db, core: rt}
+//
+// sampleDocuments and logger back EnsurePerson's own per-person
+// sample seeding (see its doc comment): sampleDocuments is
+// Config.SampleDocuments already parsed by parseSampleDocuments
+// (app.go), and logger is where a seeding failure is reported rather
+// than propagated. A nil logger defaults to slog.Default(), the same
+// fallback New (app.go) applies to Config.Logger — this Store can be
+// built directly in a test (scope_test.go and others already do) with
+// no logger of its own to pass.
+func NewStore(db *gorm.DB, rt *core.Runtime, sampleDocuments bool, logger *slog.Logger) *Store {
+ if logger == nil {
+ logger = slog.Default()
+ }
+ return &Store{db: db, core: rt, sampleDocuments: sampleDocuments, logger: logger}
}
// sqidAlphabet avoids visually ambiguous characters (0/O, 1/l/I) —
@@ -132,6 +156,17 @@ func owned(g *gorm.DB, owner int64) *gorm.DB {
// a test can mint an owner id without going through HTTP at all — the
// two must and do agree, since TestPerson is this method and nothing
// else.
+//
+// The create branch below — and only the create branch — is also
+// where a brand-new person's sample documents get seeded
+// (seedIfEnabled), when this instance has opted in. That is
+// deliberate and is the whole reason this hangs off "a Person row was
+// just created" rather than off "List came back empty": a person who
+// deletes every one of their documents still has a Person row, so
+// their next sign-in takes the found branch just above and returns
+// early — never reaching seedIfEnabled — and gets nothing back.
+// Seeding is a one-time event in a person's history, tied to the one
+// moment that can only ever happen once: their row not existing yet.
func (s *Store) EnsurePerson(ctx context.Context, email string) (int64, error) {
email = strings.TrimSpace(strings.ToLower(email))
if email == "" {
@@ -152,7 +187,11 @@ func (s *Store) EnsurePerson(ctx context.Context, email string) (int64, error) {
// A concurrent first sign-in from the same brand-new address
// can race two inserts against the unique index on email; the
// loser looks up what the winner just created rather than
- // failing the request over a benign collision.
+ // failing the request over a benign collision. The loser never
+ // seeds: only the goroutine whose INSERT actually won reaches
+ // seedIfEnabled below, so two simultaneous first sign-ins for
+ // the same address still produce exactly one set of sample
+ // documents, not two.
if errors.Is(err, gorm.ErrDuplicatedKey) {
if err2 := s.db.WithContext(ctx).Where("email = ?", email).First(&p).Error; err2 == nil {
return p.ID, nil
@@ -160,9 +199,53 @@ func (s *Store) EnsurePerson(ctx context.Context, email string) (int64, error) {
}
return 0, fmt.Errorf("docs: create person %q: %w", email, err)
}
+
+ s.seedIfEnabled(ctx, p.ID, email)
return p.ID, nil
}
+// seedIfEnabled seeds newOwner's document list with CARLOS Docs' own
+// sample content, when this instance has opted in
+// (Config.SampleDocuments, threaded through as Store.sampleDocuments)
+// — called only from EnsurePerson's just-created-the-row branch above,
+// never from its found-an-existing-row branch, and never called twice
+// for the same person: EnsurePerson's own doc comment has the full
+// reasoning for why that hook point is the one that gives "seed once,
+// per person, ever" for free rather than needing its own bookkeeping.
+//
+// This runs after the Create above has already returned nil — not
+// wrapped in a transaction of its own, and deliberately not folded
+// into a transaction around Create either (there isn't one today:
+// that INSERT already autocommits on its own). Seed's own work below
+// is six Store.Create calls, six Store.ApplyUpdate calls — each of
+// which replays a document's update log and applies a fresh one
+// through internal/core's WebAssembly runtime, not a trivial
+// in-transaction operation — and six backdating UPDATEs. Holding all
+// of that, plus the person row's own INSERT, inside one atomic unit
+// would mean a slow seed holds whatever locks SQLite takes for the
+// duration, and would mean a seed failure could take the newly
+// created person row down with it on rollback. Neither is
+// acceptable: a person's own row must be durable the moment Create
+// above succeeds, regardless of what happens next.
+//
+// So the error boundary sits here, not in Seed and not in
+// EnsurePerson's return: a failure is logged, with the address and
+// the new person id for context, and then dropped on the floor.
+// EnsurePerson still returns (p.ID, nil) no matter what happens in
+// here. A person who cannot sign in because their sample content
+// failed to generate is a far worse outcome than a person who signs
+// in to an empty list — the second is this feature not having worked
+// yet; the first is this feature having broken sign-in, which is the
+// exact failure this whole plan exists to fix, not reintroduce.
+func (s *Store) seedIfEnabled(ctx context.Context, newOwner int64, email string) {
+ if !s.sampleDocuments {
+ return
+ }
+ if err := Seed(ctx, s, newOwner); err != nil {
+ s.logger.Error("docs: seed sample documents for new person", "person", newOwner, "address", email, "err", err)
+ }
+}
+
// Create makes a new document owned by owner: a GUID and a client id
// assigned here by the app (never drawn from yrs — see Document's doc
// comment) and stored, an app-generated sqid, and the given title. It