M README.md => README.md +14 -0
@@ 276,6 276,20 @@ Once the service is up and you are logged into meta:
dolt config --global --add creds.add_url https://dolt.srht.bigb.es/settings/keys
```
+4. **Rename a database.** `~<you>/<name>` → **settings** → **Rename**. The
+ record and the stored data move together, and no redirect is left behind: the
+ old address stops resolving, so an existing checkout needs its remote
+ replaced (`dolt remote` has no `set-url`).
+
+ ```sh
+ dolt remote remove origin
+ dolt remote add origin https://dolt.srht.bigb.es/~<you>/<new-name>
+ ```
+
+ A database provisioned as the companion of a git.sr.ht repository is
+ re-created under its old name by the next push to that repository — the hook
+ provisions by the *git* repo's name, which renaming here does not change.
+
Anonymous `dolt clone` works for PUBLIC and UNLISTED databases with no
credentials at all; PRIVATE databases return "not found" to unauthorized
callers (their existence is not leaked).
M cmd/doltsrht/main.go => cmd/doltsrht/main.go +4 -0
@@ 84,6 84,10 @@ func (m *storeManager) DeleteStore(ctx context.Context, root, absPath string) er
return storage.DeleteStore(ctx, root, absPath)
}
+func (m *storeManager) MoveStore(ctx context.Context, root, srcPath, dstPath string) error {
+ return storage.MoveStore(ctx, root, srcPath, dstPath)
+}
+
func (m *storeManager) Evict(diskPath string) error {
return m.cache.Evict(diskPath)
}
M cmd/doltsrht/main_test.go => cmd/doltsrht/main_test.go +4 -0
@@ 263,6 263,10 @@ func (noStores) DeleteStore(context.Context, string, string) error {
panic("cmd/doltsrht: the boot tests must not touch the on-disk stores")
}
+func (noStores) MoveStore(context.Context, string, string, string) error {
+ panic("cmd/doltsrht: the boot tests must not touch the on-disk stores")
+}
+
func (noStores) Evict(string) error {
panic("cmd/doltsrht: the boot tests must not touch the on-disk stores")
}
M db/repos.go => db/repos.go +28 -0
@@ 218,6 218,34 @@ WHERE id = $1`
return requireOne(res, "update repo")
}
+// RenameRepo moves a repository to a new name and on-disk path in one
+// statement, and bumps updated. Both columns move together on purpose: path is
+// derived from (owner, name) by storage.RepoDiskPath, and a row whose name and
+// path disagree would be served from the wrong store.
+//
+// The caller is responsible for validating name (core.ValidateName) and for
+// moving the store on disk; this is the metadata half only. A name already
+// taken by the same owner comes back as ErrNameTaken — mapped from both unique
+// indexes the way CreateRepo maps them, because a rename to an existing
+// database trips exactly the same pair — and a missing id as ErrNotFound.
+func (s *Store) RenameRepo(ctx context.Context, id int, name, path string) error {
+ const q = `
+UPDATE repository
+SET name = $2, path = $3, updated = $4
+WHERE id = $1`
+ res, err := s.q.ExecContext(ctx, q, id, name, path, time.Now().UTC())
+ if err != nil {
+ var pqErr *pq.Error
+ if errors.As(err, &pqErr) && pqErr.Code == "23505" &&
+ (pqErr.Constraint == "uq_repo_owner_id_name" ||
+ pqErr.Constraint == "repository_path_key") {
+ return ErrNameTaken
+ }
+ return fmt.Errorf("rename repo %d: %w", id, err)
+ }
+ return requireOne(res, "rename repo")
+}
+
// DeleteRepo removes a repository row (cascading to its access entries). The
// on-disk store removal is the caller's responsibility. Returns ErrNotFound if
// id does not exist.
M db/repos_test.go => db/repos_test.go +65 -0
@@ 72,6 72,71 @@ func TestCreateRepoDuplicateName(t *testing.T) {
}
}
+// A rename moves the name and the on-disk path in one statement: path is
+// derived from (owner, name), and a row whose two halves disagree would be
+// served out of the wrong store.
+func TestRenameRepo(t *testing.T) {
+ s, sqlDB, cleanup := newTestStore(t)
+ defer cleanup()
+ ctx := context.Background()
+
+ insertUser(t, sqlDB, 1, "alice", core.UserTypeUser)
+ repo := mkRepo(t, s, ctx, 1, "alice", "widgets", core.VisibilityPublic)
+
+ require.NoError(t, s.RenameRepo(ctx, repo.ID, "gadgets", "/var/lib/dolt/~alice/gadgets"))
+
+ got, err := s.GetRepoByID(ctx, repo.ID)
+ require.NoError(t, err)
+ assert.Equal(t, "gadgets", got.Name)
+ assert.Equal(t, "/var/lib/dolt/~alice/gadgets", got.Path)
+ assert.Equal(t, core.VisibilityPublic, got.Visibility, "a rename must not touch anything else")
+
+ // The old name stops resolving and the new one answers.
+ _, err = s.GetRepoByOwnerAndName(ctx, "alice", "widgets")
+ assert.ErrorIs(t, err, ErrNotFound)
+ byName, err := s.GetRepoByOwnerAndName(ctx, "alice", "gadgets")
+ require.NoError(t, err)
+ assert.Equal(t, repo.ID, byName.ID)
+
+ assert.ErrorIs(t, s.RenameRepo(ctx, 999, "x", "/var/lib/dolt/~alice/x"), ErrNotFound)
+}
+
+// Renaming onto a name the owner already uses trips the same pair of unique
+// indexes a duplicate create does, and must come back as ErrNameTaken rather
+// than a raw 23505 — the web layer answers 409 on that error alone.
+func TestRenameRepoOntoATakenName(t *testing.T) {
+ for _, tc := range []struct {
+ name string
+ path string
+ }{
+ {"same path (both indexes, path reported)", "/var/lib/dolt/~alice/taken"},
+ {"different path (name index only)", "/srv/dolt/~alice/taken"},
+ } {
+ t.Run(tc.name, func(t *testing.T) {
+ s, sqlDB, cleanup := newTestStore(t)
+ defer cleanup()
+ ctx := context.Background()
+
+ insertUser(t, sqlDB, 1, "alice", core.UserTypeUser)
+ insertUser(t, sqlDB, 2, "bob", core.UserTypeUser)
+ repo := mkRepo(t, s, ctx, 1, "alice", "widgets", core.VisibilityPublic)
+ mkRepo(t, s, ctx, 1, "alice", "taken", core.VisibilityPublic)
+
+ assert.ErrorIs(t, s.RenameRepo(ctx, repo.ID, "taken", tc.path), ErrNameTaken)
+
+ // Nothing moved: the refusal is the whole statement's.
+ got, err := s.GetRepoByID(ctx, repo.ID)
+ require.NoError(t, err)
+ assert.Equal(t, "widgets", got.Name)
+
+ // Uniqueness is per owner — bob's "taken" is no obstacle, and it is
+ // the path index alone that keeps two owners apart on disk.
+ mkRepo(t, s, ctx, 2, "bob", "widgets", core.VisibilityPublic)
+ require.NoError(t, s.RenameRepo(ctx, repo.ID, "gadgets", "/var/lib/dolt/~alice/gadgets"))
+ })
+ }
+}
+
func TestGetRepoNotFound(t *testing.T) {
s, _, cleanup := newTestStore(t)
defer cleanup()
M storage/init.go => storage/init.go +74 -12
@@ 129,29 129,43 @@ func InitEmptyStore(ctx context.Context, absPath string) (err error) {
return nil
}
-// DeleteStore removes the store directory at absPath. It refuses to delete
-// anything that is not strictly contained within root, guarding against a
-// corrupted or attacker-controlled path escaping the configured repos root.
-// Both root and absPath must be absolute.
-func DeleteStore(ctx context.Context, root, absPath string) error {
+// containedPath checks that absPath is an absolute path strictly inside the
+// absolute root and returns both cleaned. It is the guard every destructive
+// path operation in this package runs first, so a corrupted or
+// attacker-controlled path can never escape the configured repos root — and
+// the root itself is never a legal target. op names the caller ("DeleteStore",
+// "MoveStore") so a refusal says which operation was stopped.
+func containedPath(op, root, absPath string) (cleanRoot, cleanPath string, err error) {
if !filepath.IsAbs(root) {
- return fmt.Errorf("storage: DeleteStore requires an absolute root, got %q", root)
+ return "", "", fmt.Errorf("storage: %s requires an absolute root, got %q", op, root)
}
if !filepath.IsAbs(absPath) {
- return fmt.Errorf("storage: DeleteStore requires an absolute path, got %q", absPath)
+ return "", "", fmt.Errorf("storage: %s requires an absolute path, got %q", op, absPath)
}
- cleanRoot := filepath.Clean(root)
- cleanPath := filepath.Clean(absPath)
+ cleanRoot = filepath.Clean(root)
+ cleanPath = filepath.Clean(absPath)
if cleanPath == cleanRoot {
- return fmt.Errorf("storage: refusing to delete the repos root %q", cleanRoot)
+ return "", "", fmt.Errorf("storage: %s refuses to act on the repos root %q", op, cleanRoot)
}
rel, err := filepath.Rel(cleanRoot, cleanPath)
if err != nil {
- return fmt.Errorf("storage: DeleteStore rel(%q, %q): %w", cleanRoot, cleanPath, err)
+ return "", "", fmt.Errorf("storage: %s rel(%q, %q): %w", op, cleanRoot, cleanPath, err)
}
if rel == ".." || strings.HasPrefix(rel, ".."+string(os.PathSeparator)) {
- return fmt.Errorf("storage: refusing to delete %q outside repos root %q", cleanPath, cleanRoot)
+ return "", "", fmt.Errorf("storage: %s refuses %q outside repos root %q", op, cleanPath, cleanRoot)
+ }
+ return cleanRoot, cleanPath, nil
+}
+
+// DeleteStore removes the store directory at absPath. It refuses to delete
+// anything that is not strictly contained within root, guarding against a
+// corrupted or attacker-controlled path escaping the configured repos root.
+// Both root and absPath must be absolute.
+func DeleteStore(ctx context.Context, root, absPath string) error {
+ _, cleanPath, err := containedPath("DeleteStore", root, absPath)
+ if err != nil {
+ return err
}
if err := os.RemoveAll(cleanPath); err != nil {
@@ 159,3 173,51 @@ func DeleteStore(ctx context.Context, root, absPath string) error {
}
return nil
}
+
+// MoveStore relocates the store directory at srcPath to dstPath, the on-disk
+// half of a rename. Both paths must be absolute and strictly inside root.
+//
+// It never overwrites: an existing dstPath is refused before anything is
+// touched, because os.Rename over an empty destination directory would succeed
+// silently and swallow it. A missing srcPath is likewise an error rather than a
+// no-op — a rename whose store never moved would leave the metadata row
+// pointing at nothing.
+//
+// The move itself is one os.Rename, so within a filesystem it is atomic: the
+// store is either wholly at the old path or wholly at the new one, never half
+// copied. Open handles on the old directory survive it (the inodes move, not
+// the files), but they keep resolving new writes against the old path string,
+// so the caller must still evict any memoized handle — see Cache.Evict.
+func MoveStore(ctx context.Context, root, srcPath, dstPath string) error {
+ _, cleanSrc, err := containedPath("MoveStore", root, srcPath)
+ if err != nil {
+ return err
+ }
+ _, cleanDst, err := containedPath("MoveStore", root, dstPath)
+ if err != nil {
+ return err
+ }
+ if cleanSrc == cleanDst {
+ return fmt.Errorf("storage: MoveStore source and destination are the same path %q", cleanSrc)
+ }
+
+ if _, err := os.Stat(cleanSrc); err != nil {
+ return fmt.Errorf("storage: MoveStore source %q: %w", cleanSrc, err)
+ }
+ if _, err := os.Stat(cleanDst); err == nil {
+ return fmt.Errorf("storage: MoveStore refuses to overwrite an existing store at %q", cleanDst)
+ } else if !os.IsNotExist(err) {
+ return fmt.Errorf("storage: MoveStore destination %q: %w", cleanDst, err)
+ }
+
+ // The owner directory ("<root>/~<owner>") already exists for any store that
+ // is being renamed within its owner's namespace, but creating it keeps the
+ // operation correct if a future caller ever moves a store across owners.
+ if err := os.MkdirAll(filepath.Dir(cleanDst), 0o755); err != nil {
+ return fmt.Errorf("storage: create destination dir %q: %w", filepath.Dir(cleanDst), err)
+ }
+ if err := os.Rename(cleanSrc, cleanDst); err != nil {
+ return fmt.Errorf("storage: move store %q -> %q: %w", cleanSrc, cleanDst, err)
+ }
+ return nil
+}
A storage/move_test.go => storage/move_test.go +150 -0
@@ 0,0 1,150 @@
+package storage
+
+import (
+ "context"
+ "os"
+ "path/filepath"
+ "testing"
+
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+)
+
+// A moved store must still be a store: MoveStore relocates the directory whole,
+// so the same chunks open at the new path.
+func TestMoveStoreMovesAServableStore(t *testing.T) {
+ ctx := context.Background()
+ root := t.TempDir()
+ src := RepoDiskPath(root, "alice", "widgets")
+ dst := RepoDiskPath(root, "alice", "gadgets")
+ require.NoError(t, InitStore(ctx, src, "alice", "a@b.test"))
+
+ require.NoError(t, MoveStore(ctx, root, src, dst))
+
+ _, err := os.Stat(src)
+ assert.True(t, os.IsNotExist(err), "the old path must be gone, stat err = %v", err)
+
+ cache := NewCache(func(context.Context, string, string) (string, error) { return dst, nil })
+ defer cache.Close()
+ cs, err := cache.Get(ctx, "~alice/gadgets", nbfVer)
+ require.NoError(t, err, "the moved store must open at its new path")
+ assert.NotNil(t, cs)
+}
+
+// The web rename evicts the memoized handle *after* the directory has moved,
+// so eviction has to survive closing a store whose files are no longer where it
+// opened them. If it did not, every rename of a database that had been served
+// once would end in the eviction-failure branch — a 500 on the routine path
+// instead of on the exceptional one.
+func TestEvictAfterMoveClosesCleanly(t *testing.T) {
+ ctx := context.Background()
+ root := t.TempDir()
+ src := RepoDiskPath(root, "alice", "widgets")
+ dst := RepoDiskPath(root, "alice", "gadgets")
+ require.NoError(t, InitStore(ctx, src, "alice", "a@b.test"))
+
+ cache := NewCache(func(_ context.Context, _, name string) (string, error) {
+ if name == "widgets" {
+ return src, nil
+ }
+ return dst, nil
+ })
+ defer cache.Close()
+
+ _, err := cache.Get(ctx, "~alice/widgets", nbfVer) // memoize a handle on the old path
+ require.NoError(t, err)
+
+ require.NoError(t, MoveStore(ctx, root, src, dst))
+ require.NoError(t, cache.Evict(src), "evicting a handle whose store has moved must not fail")
+
+ // And the moved store is servable again through its new path.
+ _, err = cache.Get(ctx, "~alice/gadgets", nbfVer)
+ require.NoError(t, err)
+}
+
+// os.Rename onto an existing empty directory succeeds and swallows it, so the
+// destination is checked before anything is touched.
+func TestMoveStoreRefusesAnExistingDestination(t *testing.T) {
+ ctx := context.Background()
+ root := t.TempDir()
+ src := RepoDiskPath(root, "alice", "widgets")
+ dst := RepoDiskPath(root, "alice", "gadgets")
+ require.NoError(t, InitStore(ctx, src, "alice", "a@b.test"))
+
+ for _, tc := range []struct {
+ name string
+ setup func(t *testing.T)
+ }{
+ {"empty directory", func(t *testing.T) { require.NoError(t, os.MkdirAll(dst, 0o755)) }},
+ {"another store", func(t *testing.T) { require.NoError(t, InitStore(ctx, dst, "alice", "a@b.test")) }},
+ } {
+ t.Run(tc.name, func(t *testing.T) {
+ require.NoError(t, os.RemoveAll(dst))
+ tc.setup(t)
+
+ require.Error(t, MoveStore(ctx, root, src, dst), "MoveStore must refuse an occupied destination")
+ assert.DirExists(t, src, "a refused move must leave the source where it was")
+ assert.DirExists(t, dst, "a refused move must leave the destination alone")
+ })
+ }
+}
+
+// A source that is not there means the rename would leave the metadata row
+// pointing at nothing, so it is an error rather than a silent success.
+func TestMoveStoreRefusesAMissingSource(t *testing.T) {
+ ctx := context.Background()
+ root := t.TempDir()
+
+ err := MoveStore(ctx, root, RepoDiskPath(root, "alice", "ghost"), RepoDiskPath(root, "alice", "gadgets"))
+ require.Error(t, err)
+ assert.NoDirExists(t, RepoDiskPath(root, "alice", "gadgets"))
+}
+
+func TestMoveStoreRefusesTheSamePath(t *testing.T) {
+ ctx := context.Background()
+ root := t.TempDir()
+ src := RepoDiskPath(root, "alice", "widgets")
+ require.NoError(t, InitStore(ctx, src, "alice", "a@b.test"))
+
+ require.Error(t, MoveStore(ctx, root, src, src))
+ assert.DirExists(t, src)
+}
+
+// Both ends of the move are contained: neither a source dragged in from outside
+// the repos root nor a destination pointed outside it may be acted on.
+func TestMoveStoreRootEscapeGuard(t *testing.T) {
+ ctx := context.Background()
+ root := t.TempDir()
+ outside := t.TempDir()
+
+ canary := filepath.Join(outside, "keep")
+ require.NoError(t, os.WriteFile(canary, []byte("x"), 0o644))
+
+ inside := RepoDiskPath(root, "alice", "widgets")
+ require.NoError(t, InitStore(ctx, inside, "alice", "a@b.test"))
+
+ for name, tc := range map[string]struct{ src, dst string }{
+ "source outside the root": {outside, RepoDiskPath(root, "alice", "gadgets")},
+ "destination outside": {inside, filepath.Join(outside, "stolen")},
+ "traversal destination": {inside, filepath.Join(root, "..", filepath.Base(outside), "stolen")},
+ "the root as a source": {root, RepoDiskPath(root, "alice", "gadgets")},
+ "the root as a destination": {inside, root},
+ "prefix-not-subdir dest": {inside, root + "-evil"},
+ "relative source": {"rel/path", RepoDiskPath(root, "alice", "gadgets")},
+ "relative destination": {inside, "rel/path"},
+ "relative root with abs ends": {inside, RepoDiskPath(root, "alice", "gadgets")},
+ } {
+ t.Run(name, func(t *testing.T) {
+ useRoot := root
+ if name == "relative root with abs ends" {
+ useRoot = "rel/root"
+ }
+ require.Error(t, MoveStore(ctx, useRoot, tc.src, tc.dst),
+ "MoveStore(%q, %q) should have been refused", tc.src, tc.dst)
+ })
+ }
+
+ assert.FileExists(t, canary, "a refused move disturbed something outside the repos root")
+ assert.DirExists(t, inside, "a refused move disturbed the store it named")
+ assert.NoDirExists(t, RepoDiskPath(root, "alice", "gadgets"))
+}
M web/adapters.go => web/adapters.go +4 -0
@@ 51,6 51,10 @@ func (DBAdapter) UpdateRepo(ctx context.Context, id int, description string, vis
return db.FromContext(ctx).UpdateRepo(ctx, id, description, visibility)
}
+func (DBAdapter) RenameRepo(ctx context.Context, id int, name, path string) error {
+ return db.FromContext(ctx).RenameRepo(ctx, id, name, path)
+}
+
func (DBAdapter) DeleteRepo(ctx context.Context, id int) error {
return db.FromContext(ctx).DeleteRepo(ctx, id)
}
M web/deps.go => web/deps.go +7 -0
@@ 98,6 98,10 @@ type StoreManager interface {
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
+ // MoveStore relocates the store at srcPath to dstPath — the on-disk half of
+ // a rename — refusing anything outside root and never overwriting an
+ // existing destination.
+ MoveStore(ctx context.Context, root, srcPath, dstPath string) error
// Evict closes and drops any memoized served handle for diskPath, so a
// recreation at the same path never reuses a stale store.
Evict(diskPath string) error
@@ 122,6 126,9 @@ type RepoStore interface {
ListReposForViewer(ctx context.Context, viewer *core.Caller) ([]*core.Repo, error)
ListReposForDashboard(ctx context.Context, userID int) ([]*core.Repo, error)
UpdateRepo(ctx context.Context, id int, description string, visibility core.Visibility) error
+ // RenameRepo moves the row to a new name and on-disk path together; the
+ // store on disk is moved separately by StoreManager.MoveStore.
+ RenameRepo(ctx context.Context, id int, name, path string) error
DeleteRepo(ctx context.Context, id int) error
EffectiveAccess(ctx context.Context, userID, repoID int) (*core.AccessMode, error)
M web/handlers_settings.go => web/handlers_settings.go +100 -6
@@ 80,20 80,30 @@ func (a *app) renderSettings(w http.ResponseWriter, r *http.Request, status int,
a.render(w, status, "settings", view)
}
-// handleSettings renders the settings page (description/visibility, ACLs, danger
-// zone). Owner only.
+// handleSettings renders the settings page (name, description/visibility, ACLs,
+// danger zone). Owner only.
+//
+// A completed rename lands here by redirect rather than by rendering in place,
+// so the browser's address bar carries the new name; the "renamed" query
+// parameter is how the notice survives that redirect. It is echoed back to the
+// page, so it is name-validated first — the value is a redirect target we wrote
+// ourselves, but nothing stops a reader from hand-editing the URL.
func (a *app) handleSettings(w http.ResponseWriter, r *http.Request) {
repo, _, ok := a.loadRepoForAdmin(w, r)
if !ok {
return
}
- a.renderSettings(w, r, http.StatusOK, repo, "", "")
+ notice := ""
+ if from := r.URL.Query().Get("renamed"); from != "" && core.ValidateName(from) == nil {
+ notice = "Renamed from " + from + "."
+ }
+ a.renderSettings(w, r, http.StatusOK, repo, "", notice)
}
// handleSettingsPost dispatches the settings form on its "action" field:
-// update (description + visibility), acl_add, acl_remove, or delete. Owner
-// only; the same-origin check is the router's (csrf.Require) and has already
-// run.
+// update (description + visibility), rename, acl_add, acl_remove, or delete.
+// Owner only; the same-origin check is the router's (csrf.Require) and has
+// already run.
func (a *app) handleSettingsPost(w http.ResponseWriter, r *http.Request) {
repo, _, ok := a.loadRepoForAdmin(w, r)
if !ok {
@@ 108,6 118,8 @@ func (a *app) handleSettingsPost(w http.ResponseWriter, r *http.Request) {
switch form.Get("action") {
case "update":
a.settingsUpdate(w, r, repo, form)
+ case "rename":
+ a.settingsRename(w, r, repo, form)
case "acl_add":
a.settingsACLAdd(w, r, repo, form)
case "acl_remove":
@@ 136,6 148,88 @@ func (a *app) settingsUpdate(w http.ResponseWriter, r *http.Request, repo *core.
a.renderSettings(w, r, http.StatusOK, repo, "", "Settings saved.")
}
+// settingsRename moves the database to a new name. A database is one metadata
+// row plus one on-disk store directory, and the name is written into both — the
+// row's name column and its path, which storage.RepoDiskPath derives from
+// (owner, name). Both must move, and the served handle memoized under the old
+// path must go with them.
+//
+// The order mirrors creation, which inserts the row before it touches disk: the
+// row moves first, so a name already taken is caught by the unique index while
+// nothing on disk has changed, and once it has moved no request can re-open the
+// store under the old path behind us. A failed store move then rolls the row
+// back, so metadata and disk never disagree about where a database lives.
+//
+// Renaming does not leave a redirect behind: the old address stops resolving,
+// exactly as it does on git.sr.ht, and clones pointing at it must have their
+// remote updated. A companion database provisioned from a git repository will
+// also be re-created under its old name by the next push to that repository —
+// the hook provisions by the git repo's name, which this rename does not touch.
+func (a *app) settingsRename(w http.ResponseWriter, r *http.Request, repo *core.Repo, form url.Values) {
+ newName := strings.TrimSpace(form.Get("name"))
+ if newName == repo.Name {
+ a.renderSettings(w, r, http.StatusOK, repo, "", "That is already the name of this database.")
+ return
+ }
+ if err := core.ValidateName(newName); err != nil {
+ a.renderSettings(w, r, http.StatusBadRequest, repo, err.Error(), "")
+ return
+ }
+
+ oldName, oldPath := repo.Name, repo.Path
+ newPath := a.cfg.RepoDiskPath(repo.OwnerName, newName)
+
+ if err := a.cfg.Repos.RenameRepo(r.Context(), repo.ID, newName, newPath); err != nil {
+ switch {
+ case errors.Is(err, db.ErrNameTaken):
+ a.renderSettings(w, r, http.StatusConflict, repo,
+ "You already have a database named "+newName+".", "")
+ case errors.Is(err, db.ErrNotFound):
+ a.notFound(w, r)
+ default:
+ http.Error(w, "failed to rename database", http.StatusInternalServerError)
+ }
+ return
+ }
+
+ if err := a.cfg.Stores.MoveStore(r.Context(), a.cfg.ReposRoot, oldPath, newPath); err != nil {
+ // The store-layer error names on-disk paths, which this surface never
+ // discloses; the reader gets the fact that matters to them — the rename
+ // did not happen — and the detail goes to the log against the id.
+ slog.Error("moving a database's on-disk store failed; rolling the renamed record back",
+ "component", "web", "database", repo.ID, scribe.Err(err))
+ if rerr := a.cfg.Repos.RenameRepo(r.Context(), repo.ID, oldName, oldPath); rerr != nil {
+ // Both halves failed: the row now names a database whose store is
+ // still at the old path, which no later request can repair on its
+ // own. This is the one outcome worth escalating to a human.
+ slog.Error("rolling a database's renamed record back failed; the record and its store disagree",
+ "component", "web", "database", repo.ID, scribe.Err(rerr))
+ http.Error(w, "The database record was renamed, but its on-disk store could not be moved and the record could not be restored. Contact support.",
+ http.StatusInternalServerError)
+ return
+ }
+ a.renderSettings(w, r, http.StatusInternalServerError, repo,
+ "The database could not be renamed: its on-disk store could not be moved.", "")
+ return
+ }
+
+ repo.Name, repo.Path = newName, newPath
+
+ if err := a.cfg.Stores.Evict(oldPath); err != nil {
+ // The rename itself is done — record and store are both at the new name
+ // — and only the memoized handle for the old path outlived it. Kept
+ // distinct from the failures above for that reason.
+ slog.Error("evicting a database's cached store handle failed after it was renamed",
+ "component", "web", "database", repo.ID, scribe.Err(err))
+ http.Error(w, "The database was renamed, but the cached handle for its old location could not be evicted. Contact support.",
+ http.StatusInternalServerError)
+ return
+ }
+
+ http.Redirect(w, r, "/~"+repo.OwnerName+"/"+newName+"/settings?renamed="+url.QueryEscape(oldName),
+ http.StatusSeeOther)
+}
+
// settingsACLAdd grants (or updates) an ACL entry for a username. The grantee is
// resolved via the user resolver, which mirrors the meta profile on first sight.
func (a *app) settingsACLAdd(w http.ResponseWriter, r *http.Request, repo *core.Repo, form url.Values) {
M web/templates/settings.html => web/templates/settings.html +16 -0
@@ 26,6 26,22 @@
</form>
<div class="header-extension"></div>
+<h4>Rename</h4>
+<p class="text-muted">
+ Renaming moves both this database's record and its stored data. The old
+ address stops working — no redirect is left behind — so anything cloning or
+ pushing to <code>~{{.Repo.OwnerName}}/{{.Repo.Name}}</code> must have its
+ remote updated.
+</p>
+<form method="POST" class="form-inline"
+ onsubmit="return confirm('Rename {{.Repo.Name}}? Existing clones will stop pushing until their remote is updated.')">
+ <input type="hidden" name="action" value="rename">
+ <input type="text" class="form-control mr-2" name="name" value="{{.Repo.Name}}"
+ maxlength="64" autocomplete="off" required>
+ <button type="submit" class="btn btn-primary">Rename</button>
+</form>
+
+<div class="header-extension"></div>
<h4>Access control</h4>
{{if .ACL}}
<table class="table">
M web/web_test.go => web/web_test.go +200 -2
@@ 54,9 54,19 @@ type fakeStore struct {
// answering — the metadata store unreachable rather than the row missing,
// which the pages that resolve one database must not confuse with a
// database that does not exist.
- getErr error
+ getErr error
+ // renameErr, when set, makes RenameRepo fail with it — the metadata half of
+ // a rename refusing, which must leave the store on disk untouched.
+ // renameErrAfter lets that many calls through first, which is how a test
+ // reaches the case where the rename landed and only its rollback fails.
+ renameErr error
+ renameErrAfter int
+
createdCalls []*core.Repo
deletedRepos []int
+ // renameCalls records the new name of every RenameRepo that landed, so a
+ // rollback (a second call back to the old name) is visible to a test.
+ renameCalls []string
}
func newFakeStore() *fakeStore {
@@ 160,6 170,28 @@ func (f *fakeStore) UpdateRepo(_ context.Context, id int, description string, vi
return nil
}
+// RenameRepo mirrors the real store: name and path move together, and the
+// owner's namespace is the uniqueness scope, so a taken name is ErrNameTaken.
+// renameErr forces the metadata half to fail outright.
+func (f *fakeStore) RenameRepo(_ context.Context, id int, name, path string) error {
+ if f.renameErr != nil && len(f.renameCalls) >= f.renameErrAfter {
+ return f.renameErr
+ }
+ r, ok := f.byID[id]
+ if !ok {
+ return db.ErrNotFound
+ }
+ if other, taken := f.repos[r.OwnerName+"/"+name]; taken && other.ID != id {
+ return db.ErrNameTaken
+ }
+ delete(f.repos, r.OwnerName+"/"+r.Name)
+ r.Name = name
+ r.Path = path
+ f.repos[r.OwnerName+"/"+name] = r
+ f.renameCalls = append(f.renameCalls, name)
+ return nil
+}
+
func (f *fakeStore) DeleteRepo(_ context.Context, id int) error {
r, ok := f.byID[id]
if !ok {
@@ 248,6 280,7 @@ func (f *fakeStore) DeleteKey(_ context.Context, id, userID int) error {
type fakeStoreManager struct {
initErr error
deleteErr error
+ moveErr error
evictErr error
// initCalls records InitStore (with an initial commit); initEmptyCalls
// records InitEmptyStore. They are separate so a test can say which of the
@@ 255,7 288,9 @@ type fakeStoreManager struct {
initCalls []string
initEmptyCalls []string
deleteCalls []string
- evictCalls []string
+ // moveCalls records each move as "src -> dst".
+ moveCalls []string
+ evictCalls []string
}
func (m *fakeStoreManager) InitStore(_ context.Context, absPath, _, _ string) error {
@@ 270,6 305,13 @@ func (m *fakeStoreManager) DeleteStore(_ context.Context, _, absPath string) err
m.deleteCalls = append(m.deleteCalls, absPath)
return m.deleteErr
}
+func (m *fakeStoreManager) MoveStore(_ context.Context, _, srcPath, dstPath string) error {
+ if m.moveErr != nil {
+ return m.moveErr
+ }
+ m.moveCalls = append(m.moveCalls, srcPath+" -> "+dstPath)
+ return nil
+}
func (m *fakeStoreManager) Evict(diskPath string) error {
m.evictCalls = append(m.evictCalls, diskPath)
return m.evictErr
@@ 1001,6 1043,162 @@ func TestSettingsDeleteDoesNotPrintTheStoreError(t *testing.T) {
})
}
+// newRenameHarness is one owner with one database at ~owner/db, ready to be
+// renamed.
+func newRenameHarness(t *testing.T) (*harness, *core.Repo, *auth.AuthContext) {
+ t.Helper()
+ h := newHarness(t)
+ repo := h.store.add(&core.Repo{Name: "db", OwnerID: 10, OwnerName: "owner",
+ Path: "/var/lib/dolt/~owner/db", Visibility: core.VisibilityPublic})
+ return h, repo, testCaller(10, "owner")
+}
+
+func rename(h *harness, caller *auth.AuthContext, from, to string) *httptest.ResponseRecorder {
+ return h.do("POST", "/~owner/"+from+"/settings", caller,
+ url.Values{"action": {"rename"}, "name": {to}})
+}
+
+// A rename has to move three things together: the metadata row's name, the
+// row's on-disk path, and the store directory that path names — and then drop
+// the served handle memoized under the old path, or a push would keep landing
+// in a store nothing resolves to any more.
+func TestSettingsRename(t *testing.T) {
+ h, repo, owner := newRenameHarness(t)
+
+ rec := rename(h, owner, "db", "widgets")
+ require.Equal(t, http.StatusSeeOther, rec.Code)
+ assert.Equal(t, "/~owner/widgets/settings?renamed=db", rec.Header().Get("Location"))
+
+ assert.Equal(t, "widgets", repo.Name)
+ assert.Equal(t, "/var/lib/dolt/~owner/widgets", repo.Path,
+ "the row's path must follow the name, or the row would be served from the old store")
+ assert.Equal(t, []string{"/var/lib/dolt/~owner/db -> /var/lib/dolt/~owner/widgets"}, h.stores.moveCalls)
+ assert.Equal(t, []string{"/var/lib/dolt/~owner/db"}, h.stores.evictCalls,
+ "the handle memoized under the old path must be evicted")
+
+ // The old address stops resolving and the new one answers, notice included.
+ assert.Equal(t, http.StatusNotFound, h.do("GET", "/~owner/db", owner, nil).Code)
+ after := h.do("GET", "/~owner/widgets/settings?renamed=db", owner, nil)
+ require.Equal(t, http.StatusOK, after.Code)
+ assert.Contains(t, after.Body.String(), "Renamed from db.")
+}
+
+// Every refusal must be total: nothing on disk moves, and the row keeps both
+// its name and its path.
+func TestSettingsRenameRefusals(t *testing.T) {
+ for _, tc := range []struct {
+ name string
+ to string
+ want int
+ body string
+ }{
+ {"invalid name", "bad name!", http.StatusBadRequest, "must match"},
+ {"traversal", "..", http.StatusBadRequest, "not allowed"},
+ {"empty", "", http.StatusBadRequest, "must not be empty"},
+ {"name already taken", "taken", http.StatusConflict, "You already have a database named taken."},
+ } {
+ t.Run(tc.name, func(t *testing.T) {
+ h, repo, owner := newRenameHarness(t)
+ h.store.add(&core.Repo{Name: "taken", OwnerID: 10, OwnerName: "owner",
+ Path: "/var/lib/dolt/~owner/taken", Visibility: core.VisibilityPublic})
+
+ rec := rename(h, owner, "db", tc.to)
+ require.Equal(t, tc.want, rec.Code)
+ assert.Contains(t, rec.Body.String(), tc.body)
+ assert.Equal(t, "db", repo.Name)
+ assert.Equal(t, "/var/lib/dolt/~owner/db", repo.Path)
+ assert.Empty(t, h.stores.moveCalls, "a refused rename must not touch disk")
+ assert.Empty(t, h.stores.evictCalls)
+ })
+ }
+
+ t.Run("the name it already has", func(t *testing.T) {
+ h, repo, owner := newRenameHarness(t)
+ rec := rename(h, owner, "db", "db")
+ require.Equal(t, http.StatusOK, rec.Code)
+ assert.Contains(t, rec.Body.String(), "That is already the name of this database.")
+ assert.Equal(t, "db", repo.Name)
+ assert.Empty(t, h.store.renameCalls, "a no-op rename must not write the row")
+ assert.Empty(t, h.stores.moveCalls)
+ })
+
+ t.Run("not the owner", func(t *testing.T) {
+ h, repo, _ := newRenameHarness(t)
+ rec := rename(h, testCaller(99, "intruder"), "db", "widgets")
+ require.Equal(t, http.StatusForbidden, rec.Code)
+ assert.Equal(t, "db", repo.Name)
+ assert.Empty(t, h.stores.moveCalls)
+ })
+}
+
+// The row moves before the store does, so a store that will not move leaves a
+// row naming a database whose data is somewhere else. The rename is rolled back
+// rather than left that way — and, as on the delete path, the store layer's own
+// error text (which carries on-disk paths) never reaches the reader.
+func TestSettingsRenameRollsBackWhenTheStoreCannotMove(t *testing.T) {
+ h, repo, owner := newRenameHarness(t)
+ h.stores.moveErr = errors.New(storeDetail)
+
+ rec := rename(h, owner, "db", "widgets")
+ require.Equal(t, http.StatusInternalServerError, rec.Code)
+
+ assert.Equal(t, "db", repo.Name, "the row must be back where it started")
+ assert.Equal(t, "/var/lib/dolt/~owner/db", repo.Path)
+ assert.Equal(t, []string{"widgets", "db"}, h.store.renameCalls,
+ "the rename must have been attempted and then undone")
+ assert.Empty(t, h.stores.evictCalls, "nothing moved, so nothing needs evicting")
+
+ body := rec.Body.String()
+ assert.Contains(t, body, "The database could not be renamed")
+ assert.NotContains(t, body, storeDetail)
+ assert.NotContains(t, body, "/var/lib/dolt", "the store's path must not reach the reader")
+}
+
+// Both halves failing is the one outcome no later request can repair by itself:
+// the row names the new database and the store is still at the old path. It is
+// reported as such instead of as a plain failed rename.
+func TestSettingsRenameRollbackFailure(t *testing.T) {
+ h, _, owner := newRenameHarness(t)
+ h.stores.moveErr = errors.New(storeDetail)
+ h.store.renameErr = errors.New("connection reset by peer")
+ h.store.renameErrAfter = 1 // the rename lands; only its rollback fails
+
+ rec := rename(h, owner, "db", "widgets")
+ require.Equal(t, http.StatusInternalServerError, rec.Code)
+ body := rec.Body.String()
+ assert.Contains(t, body, "the record could not be restored. Contact support.")
+ assert.NotContains(t, body, storeDetail)
+ assert.NotContains(t, body, "connection reset by peer")
+}
+
+// The store moved and the row moved; only the cached handle for the old path
+// outlived them. A distinct fact from the two above, told distinctly.
+func TestSettingsRenameEvictionFailure(t *testing.T) {
+ h, repo, owner := newRenameHarness(t)
+ h.stores.evictErr = errors.New(storeDetail)
+
+ rec := rename(h, owner, "db", "widgets")
+ require.Equal(t, http.StatusInternalServerError, rec.Code)
+ assert.Equal(t, "widgets", repo.Name, "the rename itself did happen")
+ body := rec.Body.String()
+ assert.Contains(t, body, "The database was renamed, but the cached handle")
+ assert.NotContains(t, body, storeDetail)
+ assert.NotContains(t, body, "/var/lib/dolt")
+}
+
+// The notice is built from a query parameter, so it is a reader-controlled
+// string on a page the owner is looking at. Only something that could really be
+// a database name is echoed back.
+func TestSettingsRenamedNoticeIgnoresJunk(t *testing.T) {
+ h, _, owner := newRenameHarness(t)
+
+ rec := h.do("GET", "/~owner/db/settings?renamed="+url.QueryEscape("<script>x</script>"), owner, nil)
+ require.Equal(t, http.StatusOK, rec.Code)
+ body := rec.Body.String()
+ assert.NotContains(t, body, "Renamed from")
+ assert.NotContains(t, body, "<script>x</script>")
+}
+
func TestSettingsACLAddRemove(t *testing.T) {
h := newHarness(t)
repo := h.store.add(&core.Repo{Name: "db", OwnerID: 10, OwnerName: "owner", Path: "/d", Visibility: core.VisibilityPublic})