From 93e699101e297244802590365e96da638404b579 Mon Sep 17 00:00:00 2001 From: Eugene Blikh Date: Sat, 15 Aug 2026 12:09:58 +0300 Subject: [PATCH] storage: create databases empty so the first push needs no --force MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every automatic creation path wrote an "Initialize data repository" commit through WriteEmptyRepo, and that commit is history. dolt decides fast-forward on the client (actions.CanFastForward over the remotesapi), so the server cannot forgive the collision: pushing a database that has a root commit of its own — a beads tracker, anything grown locally — was rejected as a non-fast-forward and could only land with --force. That is the whole reason the companion-database recipe starts with a forced push. push-to-create already provisioned an empty store for this exact reason. Give the other two paths the same default: /internal/repos, whose caller is git.sr.ht's post-update hook and therefore fires before its user has ever pushed, now always provisions empty, and the web form does unless its new "initialize with an empty commit" checkbox is ticked. The checkbox buys what an empty store cannot offer — a database that can be cloned before anything is pushed to it, since dolt refuses a store with no commits as "contains no Dolt data". Which is also why the overview of a database with no branches now teaches push rather than clone: the clone box there quoted a command that could not work. A store that fails to open is deliberately not treated as empty — an unreadable database must not be advertised as a fresh one. --- cmd/doltsrht/main.go | 4 ++ cmd/doltsrht/main_test.go | 4 ++ docs/DESIGN.md | 12 +++- storage/init.go | 7 +++ web/deps.go | 12 ++++ web/handlers_internal.go | 34 +++------- web/handlers_internal_test.go | 18 ++++-- web/handlers_repo.go | 55 +++++++++++++--- web/templates/create.html | 14 +++++ web/templates/overview.html | 21 +++++++ web/web_test.go | 114 +++++++++++++++++++++++++++++++--- 11 files changed, 244 insertions(+), 51 deletions(-) diff --git a/cmd/doltsrht/main.go b/cmd/doltsrht/main.go index b95df4cfd21cdd0cbefcf01ced6e0b549181524d..f235b521f9a4865be867a85b02e45a303fa4c20b 100644 --- a/cmd/doltsrht/main.go +++ b/cmd/doltsrht/main.go @@ -76,6 +76,10 @@ func (m *storeManager) InitStore(ctx context.Context, absPath, ownerName, ownerE return storage.InitStore(ctx, absPath, ownerName, ownerEmail) } +func (m *storeManager) InitEmptyStore(ctx context.Context, absPath string) error { + return storage.InitEmptyStore(ctx, absPath) +} + func (m *storeManager) DeleteStore(ctx context.Context, root, absPath string) error { return storage.DeleteStore(ctx, root, absPath) } diff --git a/cmd/doltsrht/main_test.go b/cmd/doltsrht/main_test.go index bf4c8dab69d8a202e9ffab8cb064da03fbb98472..56c1d0512d45f24493d6ceac03ce8365abbe9b62 100644 --- a/cmd/doltsrht/main_test.go +++ b/cmd/doltsrht/main_test.go @@ -255,6 +255,10 @@ func (noStores) InitStore(context.Context, string, string, string) error { panic("cmd/doltsrht: the boot tests must not touch the on-disk stores") } +func (noStores) InitEmptyStore(context.Context, string) error { + panic("cmd/doltsrht: the boot tests must not touch the on-disk stores") +} + func (noStores) DeleteStore(context.Context, string, string) error { panic("cmd/doltsrht: the boot tests must not touch the on-disk stores") } diff --git a/docs/DESIGN.md b/docs/DESIGN.md index 35b014dd6e59021c7ab92adeffead5e17b3bb7db..a3f2b3f357a9cce1aa85e1e854bdd3abd3790f12 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -68,7 +68,15 @@ Key verified facts that shape the design: = authorization), web UI owns the rest. `getScheme()` honors `x-forwarded-proto` — nginx must set it on grpc_pass. - Empty-db init primitive: `doltdb.LoadDoltDB(ctx, types.Format_Default, fileURL, fs)` - + `ddb.WriteEmptyRepo(ctx, "main", name, email)`. + + `ddb.WriteEmptyRepo(ctx, "main", name, email)`. **This is the opt-in path, not + the default one**: dolt decides fast-forward on the client, so the initial commit + it writes makes the first push from any database with a history of its own a + non-fast-forward the server cannot forgive. Every creation path (web form, + `/internal/repos`, push-to-create) therefore defaults to `InitEmptyStore` — a bare + NBS store with no commits and no branches — and only the create form's + "initialize with an empty commit" checkbox reaches `WriteEmptyRepo`. The cost of an + empty store is that it cannot be cloned (`ErrNoDataAtRemote`), which is why the + overview of a database with no branches teaches push instead of clone. - Bare stores **cannot** be opened by the sqle engine / embedded driver (they expect working sets). Browse UI uses low-level read-only APIs instead (see §Browse). - dolt CLI v2.1.10 installed at `/opt/homebrew/bin/dolt` (integration tests). @@ -88,7 +96,7 @@ core/ # PURE domain: names.go (validate, ParseRepoPath), access.go (matrix db/ # postgres: repos.go, access.go, keys.go (dolt_key CRUD) authn/ # ctx.go (Caller), cookie.go (optional unified-login middleware), # token.go (Basic: PAT trio + 60s cache), jwt.go (Bearer: EdDSA verify) -storage/ # init.go (InitStore/DeleteStore via WriteEmptyRepo), dbcache.go (remotesrv.DBCache) +storage/ # init.go (InitEmptyStore default / InitStore opt-in / Delete/MoveStore), dbcache.go (remotesrv.DBCache) remoteapi/ # server.go (remotesrv assembly), interceptors.go, credsvc.go (WhoAmI grpc server) browse/ # open.go, log.go, tables.go, diff.go — read-only doltdb over bare stores web/ # router.go, handlers_*.go, templates.go, templates/*.html diff --git a/storage/init.go b/storage/init.go index de293260525c2fed6d0ffb7cc81d58a4124bd914..a8372c415c75332b8934518bcef12ae148aa6b53 100644 --- a/storage/init.go +++ b/storage/init.go @@ -46,6 +46,13 @@ func RepoDiskPath(root, owner, name string) string { // into it with a single "main" branch and an initial commit authored by // ownerName/ownerEmail. // +// It is the OPT-IN half of database creation — the create form's "initialize +// with an empty commit" checkbox — and not what any automatic path uses. The +// initial commit it writes is history, so a client pushing a database that has +// its own history is pushing a non-fast-forward and must --force. What it buys +// in exchange is a database that can be cloned before anything is pushed to it, +// which InitEmptyStore's result cannot. +// // absPath must be absolute. On any failure after the directory is created, // InitStore removes absPath so a failed creation never leaves a partial store // behind. Idempotence is NOT provided: calling InitStore on an existing store diff --git a/web/deps.go b/web/deps.go index 5731336fb41513fe559aba4901c02222c769f314..0fd9d26700668471b3238c1919f8829bd3a47739 100644 --- a/web/deps.go +++ b/web/deps.go @@ -83,7 +83,19 @@ type Config struct { type StoreManager interface { // InitStore creates a bare store at absPath and writes an empty repo authored // by ownerName/ownerEmail. On any failure it must leave no partial store. + // That empty repo is an "Initialize data repository" commit, so it is the + // opt-in half of creation: see InitEmptyStore for why it is not the default. InitStore(ctx context.Context, absPath, ownerName, ownerEmail string) error + // InitEmptyStore creates a bare store at absPath with NO commits and NO + // branches, so a client's first push lands as the initial history instead of + // being rejected as a non-fast-forward. dolt decides fast-forward on the + // client (actions.CanFastForward over the remotesapi), so an initial commit + // on our side cannot be forgiven by the server — it can only be not written. + // The cost is that a store with no commits cannot be dolt-cloned at all + // ("remote at that url contains no Dolt data"), which is why the empty + // overview page teaches push rather than clone. On any failure it must leave + // no partial store. + InitEmptyStore(ctx context.Context, absPath string) error // DeleteStore removes the store at absPath, refusing anything outside root. DeleteStore(ctx context.Context, root, absPath string) error // Evict closes and drops any memoized served handle for diskPath, so a diff --git a/web/handlers_internal.go b/web/handlers_internal.go index cba12881f365f9ce6660ba4976b5f6bb85355f40..cf88560e5c2394ecc04261662ce8c5bb32390ba4 100644 --- a/web/handlers_internal.go +++ b/web/handlers_internal.go @@ -7,10 +7,6 @@ import ( "net/http" "strings" - "sourcecraft.dev/bigbes/sr-ht-core/config" - - "sourcecraft.dev/bigbes/sr-ht-ecore/instconf" - "sourcecraft.dev/bigbes/sr-ht-dolt/core" "sourcecraft.dev/bigbes/sr-ht-dolt/db" ) @@ -134,26 +130,16 @@ func (a *app) handleInternalCreate(w http.ResponseWriter, r *http.Request) { return } - // The initial empty commit's author is cosmetic (real pushes overwrite - // history); mirror handleCreate and author it as the instance owner, - // falling back to the database owner. - authorName, authorEmail := config.GetOwner(a.cfg.Conf) - if authorName == "" { - authorName = caller.Username - } - if authorEmail == "" { - // The authority and not the bare host: this is the domain half of an - // address that identifies *this* deployment, and two instances behind - // one hostname on two ports are two of them. An origin that names no - // host at all yields "", and then there is no domain to synthesize — - // leave the address empty rather than emit "alice@". - if authority := instconf.OriginAuthority(a.chrome.SelfOrigin()); authority != "" { - authorEmail = caller.Username + "@" + authority - } - } - if err := a.cfg.Stores.InitStore(ctx, diskPath, authorName, authorEmail); err != nil { - // InitStore self-cleans its directory; undo the metadata row too so a - // failed provision leaves nothing behind and a retry can start clean. + // The companion is provisioned EMPTY — no branches, no "Initialize data + // repository" commit. This endpoint fires before its user has ever pushed, + // and whatever they push first (a beads tracker, a database built locally) + // has a history of its own. dolt decides fast-forward on the client, so an + // initial commit here would make every such first push a non-fast-forward + // the server cannot forgive — the user's only way in would be --force. An + // empty store lets that first push land as the database's initial history. + if err := a.cfg.Stores.InitEmptyStore(ctx, diskPath); err != nil { + // InitEmptyStore self-cleans its directory; undo the metadata row too so + // a failed provision leaves nothing behind and a retry can start clean. _ = a.cfg.Repos.DeleteRepo(ctx, created.ID) http.Error(w, "initialize database store", http.StatusInternalServerError) return diff --git a/web/handlers_internal_test.go b/web/handlers_internal_test.go index 6c92bf7a04653b0c0891084ff2b577079dc898fa..6821a4d773c92e400229d899e0a6a2735f6eaa07 100644 --- a/web/handlers_internal_test.go +++ b/web/handlers_internal_test.go @@ -73,10 +73,15 @@ func TestInternalCreateProvisions(t *testing.T) { if got.Visibility != core.VisibilityPrivate { t.Fatalf("default visibility: got %q, want PRIVATE", got.Visibility) } - // On-disk store initialized at the mapped path. - if len(h.stores.initCalls) != 1 || !strings.HasSuffix(h.stores.initCalls[0], "/~alice/widgets") { - t.Fatalf("expected InitStore at ~alice/widgets, got %v", h.stores.initCalls) - } + // On-disk store initialized at the mapped path — and initialized EMPTY. The + // caller is git.sr.ht's post-update hook, which fires before its user has + // ever pushed to the companion: an "Initialize data repository" commit here + // would make that first push a non-fast-forward and force the user to + // --force their own history in. + if len(h.stores.initEmptyCalls) != 1 || !strings.HasSuffix(h.stores.initEmptyCalls[0], "/~alice/widgets") { + t.Fatalf("expected InitEmptyStore at ~alice/widgets, got %v", h.stores.initEmptyCalls) + } + assert.Empty(t, h.stores.initCalls, "an auto-provisioned companion must carry no initial commit") } func TestInternalCreateIdempotent(t *testing.T) { @@ -92,8 +97,9 @@ func TestInternalCreateIdempotent(t *testing.T) { t.Fatalf("expected created:false, got %s", rec.Body.String()) } // Must NOT touch disk when the row already exists. - if len(h.stores.initCalls) != 0 { - t.Fatalf("InitStore must not run for an existing companion, got %v", h.stores.initCalls) + if len(h.stores.initCalls)+len(h.stores.initEmptyCalls) != 0 { + t.Fatalf("no store init may run for an existing companion, got %v / %v", + h.stores.initCalls, h.stores.initEmptyCalls) } } diff --git a/web/handlers_repo.go b/web/handlers_repo.go index d748c2d57ae00204b935f1202c1550cedfe7d9ba..3dd11d512cbc68b130565a64a60297f13e842a44 100644 --- a/web/handlers_repo.go +++ b/web/handlers_repo.go @@ -1,6 +1,7 @@ package web import ( + "context" "errors" "log/slog" "net/http" @@ -56,6 +57,12 @@ type createForm struct { Name string Description string Visibility string + // Initialize asks for an "Initialize data repository" commit in the new + // store. It defaults to OFF: an initial commit makes the first push from a + // database with a history of its own a non-fast-forward (dolt decides that + // on the client), so it would have to be forced. On costs the opposite — + // the database is clonable immediately, which an empty store is not. + Initialize bool } func (a *app) renderCreate(w http.ResponseWriter, r *http.Request, status int, form createForm, errMsg string) { @@ -90,6 +97,9 @@ func (a *app) handleCreate(w http.ResponseWriter, r *http.Request) { Name: strings.TrimSpace(values.Get("name")), Description: strings.TrimSpace(values.Get("description")), Visibility: values.Get("visibility"), + // An unchecked checkbox is simply absent from the submission, so any + // value at all means checked. + Initialize: values.Get("initialize") != "", } visibility, ok := parseVisibility(form.Visibility) @@ -103,14 +113,6 @@ func (a *app) handleCreate(w http.ResponseWriter, r *http.Request) { } owner := ac.Username - ownerName, ownerEmail := config.GetOwner(a.cfg.Conf) - if ownerEmail == "" { - ownerEmail = ac.Email - } - if ownerName == "" { - ownerName = owner - } - diskPath := a.cfg.RepoDiskPath(owner, form.Name) repo := &core.Repo{ Name: form.Name, @@ -135,8 +137,13 @@ func (a *app) handleCreate(w http.ResponseWriter, r *http.Request) { return } - if err := a.cfg.Stores.InitStore(r.Context(), diskPath, ownerName, ownerEmail); err != nil { - // InitStore self-cleans its directory; undo the metadata row too. + // Empty by default, so the first push from a database that already has a + // history lands as this one's initial history rather than being rejected as + // a non-fast-forward. The checkbox buys the opposite trade: an initial + // commit, and with it a database that can be cloned before anything is + // pushed to it. + if err := a.initStore(r.Context(), diskPath, ac, form.Initialize); err != nil { + // Both init paths self-clean their directory; undo the metadata row too. _ = a.cfg.Repos.DeleteRepo(r.Context(), created.ID) http.Error(w, "failed to initialize database store", http.StatusInternalServerError) return @@ -145,6 +152,26 @@ func (a *app) handleCreate(w http.ResponseWriter, r *http.Request) { http.Redirect(w, r, "/~"+owner+"/"+form.Name, http.StatusSeeOther) } +// initStore materializes the on-disk store for a newly created database. With +// initialize false — the default — it writes a store with no commits at all, so +// the owner's first push is a fast-forward from empty. With initialize true it +// writes the "Initialize data repository" commit, whose author is cosmetic +// (real pushes overwrite it): the instance owner from the config, falling back +// to the creating user's own name and address. +func (a *app) initStore(ctx context.Context, diskPath string, ac *authContext, initialize bool) error { + if !initialize { + return a.cfg.Stores.InitEmptyStore(ctx, diskPath) + } + ownerName, ownerEmail := config.GetOwner(a.cfg.Conf) + if ownerName == "" { + ownerName = ac.Username + } + if ownerEmail == "" { + ownerEmail = ac.Email + } + return a.cfg.Stores.InitStore(ctx, diskPath, ownerName, ownerEmail) +} + // handleUser renders a single user's visible databases (~user listing). func (a *app) handleUser(w http.ResponseWriter, r *http.Request) { owner := chi.URLParam(r, "user") @@ -249,6 +276,7 @@ func (a *app) handleOverview(w http.ResponseWriter, r *http.Request) { Views []View CloneURL string BrowseFailed bool + Empty bool }{ Page: a.page(r, repo.OwnerName+"/"+repo.Name+" — "+serviceName), Repo: repo, @@ -258,6 +286,13 @@ func (a *app) handleOverview(w http.ResponseWriter, r *http.Request) { Views: views, CloneURL: a.cloneURL(repo), BrowseFailed: browseFailed, + // A database nothing has been pushed to yet: it has no branches and the + // store read fine, so the emptiness is the answer rather than a symptom. + // The page then teaches push instead of clone — a store with no commits + // cannot be cloned at all, dolt refuses it as "contains no Dolt data". + // A browse failure is deliberately NOT empty: an unreadable store must + // not be advertised as a fresh one waiting for its first push. + Empty: !browseFailed && len(branches) == 0, } a.render(w, http.StatusOK, "overview", view) } diff --git a/web/templates/create.html b/web/templates/create.html index 421e193872ed9c30acbb6ba8c7980836dbfe34fb..0fb611da993f1d8c3c49dc4ea2f7d583468d4aae 100644 --- a/web/templates/create.html +++ b/web/templates/create.html @@ -28,6 +28,20 @@ +
+ + + Leave this off to push a database you already have: the new database starts + with no commits, so your first push lands as its history and needs no + --force. Turn it on to get a main branch with an + empty initial commit, which makes the database clonable right away — at the + cost of forcing that first push from anywhere else. + +
{{- end}} diff --git a/web/templates/overview.html b/web/templates/overview.html index b7abd8db54b83a5ab08eec213b989c62c630826f..4bec731293f6415b1ae5495e7e28323a660ef3eb 100644 --- a/web/templates/overview.html +++ b/web/templates/overview.html @@ -10,6 +10,26 @@ {{template "viewtabs" (dict "Repo" .Repo "Views" .Views "Current" "overview" "Ref" .DefaultBranch)}} +{{if .Empty}} +
+

Push to get started

+

This database has no commits yet, so there is nothing to clone — push + something and that push becomes its history. A database you already have works + as well as a fresh one: its own commits land here unchanged, with no + --force anywhere.

+ 
dolt init   # skip in a database you already have
+dolt remote add origin {{.CloneURL}}
+dolt push origin main
+

Authenticate with a meta.sr.ht personal access token (Basic auth):

+ 
export DOLT_REMOTE_PASSWORD=<your meta access token>
+dolt push --user {{.Repo.OwnerName}} origin main
+

Or, associate a dolt key once and push with no + credentials (like a git SSH key):

+ 
dolt creds new
+dolt login --auth-endpoint {{doltHost .SelfOrigin}} --login-url {{.SelfOrigin}}/settings/keys
+dolt push origin main
+
+{{else}}

Clone

With a meta.sr.ht personal access token (Basic auth):

@@ -67,4 +87,5 @@ dolt clone {{.CloneURL}} {{end}}
+{{end}} {{- end}} diff --git a/web/web_test.go b/web/web_test.go index 31aace8ead67460affec018f4fc4fcce0e2ddffb..421eed877da35a4a29ade0dba1bf5f2ec2e70e01 100644 --- a/web/web_test.go +++ b/web/web_test.go @@ -246,18 +246,26 @@ func (f *fakeStore) DeleteKey(_ context.Context, id, userID int) error { } type fakeStoreManager struct { - initErr error - deleteErr error - evictErr error - initCalls []string - deleteCalls []string - evictCalls []string + initErr error + deleteErr error + evictErr error + // initCalls records InitStore (with an initial commit); initEmptyCalls + // records InitEmptyStore. They are separate so a test can say which of the + // two creation paths ran, not merely that a store was created. + initCalls []string + initEmptyCalls []string + deleteCalls []string + evictCalls []string } func (m *fakeStoreManager) InitStore(_ context.Context, absPath, _, _ string) error { m.initCalls = append(m.initCalls, absPath) return m.initErr } +func (m *fakeStoreManager) InitEmptyStore(_ context.Context, absPath string) error { + m.initEmptyCalls = append(m.initEmptyCalls, absPath) + return m.initErr +} func (m *fakeStoreManager) DeleteStore(_ context.Context, _, absPath string) error { m.deleteCalls = append(m.deleteCalls, absPath) return m.deleteErr @@ -609,6 +617,39 @@ func TestOverviewDoesNotPrintTheBrowseError(t *testing.T) { }) } +// A database created the new way — an empty store, nothing pushed yet — has no +// branches at all. Its overview must teach push rather than clone: dolt refuses +// to clone a store with no commits ("contains no Dolt data"), so a clone box +// there hands the reader a command that cannot work. +func TestOverviewOfAnEmptyDatabaseTeachesPush(t *testing.T) { + h := newHarness(t) + h.store.add(&core.Repo{Name: "db", OwnerID: 1, OwnerName: "alice", + Path: "/var/lib/dolt/~alice/db", Visibility: core.VisibilityPublic}) + h.browse.sess = &fakeSession{} + + rec := h.do("GET", "/~alice/db", nil, nil) + require.Equal(t, http.StatusOK, rec.Code) + body := rec.Body.String() + + assert.Contains(t, body, "Push to get started") + assert.Contains(t, body, "dolt remote add origin") + assert.Contains(t, body, "dolt push origin main") + assert.NotContains(t, body, "dolt clone", "an empty store cannot be cloned") + assert.NotContains(t, body, "Could not read history", + "emptiness is a state, not a browse failure") + + // A store that cannot be READ is not an empty one: the push instructions + // would be a lie about a database that may well have history. + broken := newHarness(t) + broken.store.add(&core.Repo{Name: "db", OwnerID: 1, OwnerName: "alice", + Path: "/var/lib/dolt/~alice/db", Visibility: core.VisibilityPublic}) + broken.browse.errByPath = map[string]error{"/var/lib/dolt/~alice/db": errors.New(browseDetail)} + + rec = broken.do("GET", "/~alice/db", nil, nil) + require.Equal(t, http.StatusOK, rec.Code) + assert.NotContains(t, rec.Body.String(), "Push to get started") +} + func TestDashboardLists(t *testing.T) { h := newHarness(t) h.store.add(&core.Repo{Name: "mine", OwnerID: 3, OwnerName: "bob", Path: "/m", Visibility: core.VisibilityPrivate}) @@ -654,20 +695,75 @@ func TestCreateValidationAndSuccess(t *testing.T) { if got := ok.Header().Get("Location"); got != "/~carol/gooddb" { t.Fatalf("create redirect: got %q", got) } - if len(h.stores.initCalls) != 1 || h.stores.initCalls[0] != "/var/lib/dolt/~carol/gooddb" { - t.Fatalf("InitStore not called correctly: %v", h.stores.initCalls) + // The form was submitted without the "initialize" checkbox, so the store is + // created EMPTY: no initial commit for the owner's first push to collide + // with. The commit-writing path must not have run at all. + if len(h.stores.initEmptyCalls) != 1 || h.stores.initEmptyCalls[0] != "/var/lib/dolt/~carol/gooddb" { + t.Fatalf("InitEmptyStore not called correctly: %v", h.stores.initEmptyCalls) + } + if len(h.stores.initCalls) != 0 { + t.Fatalf("InitStore must not run without the initialize checkbox: %v", h.stores.initCalls) } if _, err := h.store.GetRepoByOwnerAndName(context.Background(), "carol", "gooddb"); err != nil { t.Fatalf("repo row not created: %v", err) } } +// TestCreateWithInitializeCheckbox is the other half of TestCreateValidationAndSuccess: +// the checkbox is the only way to get an "Initialize data repository" commit, +// and it must reach InitStore rather than the empty path. +func TestCreateWithInitializeCheckbox(t *testing.T) { + h := newHarness(t) + caller := testCaller(5, "carol") + + rec := h.do("POST", "/create", caller, url.Values{ + "name": {"seeded"}, + "visibility": {"PUBLIC"}, + "initialize": {"on"}, + }) + require.Equal(t, http.StatusSeeOther, rec.Code, rec.Body.String()) + assert.Equal(t, []string{"/var/lib/dolt/~carol/seeded"}, h.stores.initCalls) + assert.Empty(t, h.stores.initEmptyCalls, "the checkbox selects the commit-writing path exclusively") +} + +// TestCreateFormOffersTheInitializeCheckbox pins the control itself: the +// handler's default is only reachable from a browser if the form renders an +// unchecked "initialize" box. +func TestCreateFormOffersTheInitializeCheckbox(t *testing.T) { + h := newHarness(t) + + rec := h.do("GET", "/create", testCaller(5, "carol"), nil) + require.Equal(t, http.StatusOK, rec.Code) + body := rec.Body.String() + assert.Contains(t, body, `name="initialize"`) + assert.NotContains(t, body, "checked", "the initialize checkbox defaults to off") +} + +// TestCreateEmptyStoreFailureRollsBackRow is TestCreateStoreFailureRollsBackRow +// for the default (empty) path: a failed InitEmptyStore must leave no metadata +// row behind either. +func TestCreateEmptyStoreFailureRollsBackRow(t *testing.T) { + h := newHarness(t) + h.stores.initErr = errors.New("disk full") + + rec := h.do("POST", "/create", testCaller(5, "carol"), + url.Values{"name": {"gooddb"}, "visibility": {"PUBLIC"}}) + require.Equal(t, http.StatusInternalServerError, rec.Code) + _, err := h.store.GetRepoByOwnerAndName(context.Background(), "carol", "gooddb") + assert.ErrorIs(t, err, db.ErrNotFound, "orphan repo row survived an empty-store failure") + assert.Len(t, h.store.deletedRepos, 1) +} + func TestCreateStoreFailureRollsBackRow(t *testing.T) { h := newHarness(t) h.stores.initErr = errors.New("disk full") caller := testCaller(5, "carol") - rec := h.do("POST", "/create", caller, url.Values{"name": {"gooddb"}, "visibility": {"PUBLIC"}}) + // The checkbox path, so this covers InitStore's rollback specifically; the + // default (empty) path is TestCreateEmptyStoreFailureRollsBackRow's. + rec := h.do("POST", "/create", caller, url.Values{ + "name": {"gooddb"}, "visibility": {"PUBLIC"}, "initialize": {"on"}, + }) if rec.Code != http.StatusInternalServerError { t.Fatalf("create with store failure: got %d, want 500", rec.Code) }