Repository: supabase/cli
Pull request #330 (2022-08-04 13:15:17)

fix: branch creation failure after init
## What kind of change does this PR introduce?

part 1 for fixing https://github.com/supabase/cli/issues/321

## What is the current behavior?

branch create fails if `supabase start` is not run.

## What is the new behavior?

- allow `supabase db branch create` to succeed by using `MkdirAll`.

## Additional context

Add any other context or screenshots.


coveralls: ## Pull Request Test Coverage Report for [Build 2800832853](https://coveralls.io/builds/51437341)

* **41** of **51**   **(80.39%)**  changed or added relevant lines in **3** files are covered.
* **53** unchanged lines in **3** files lost coverage.
* Overall coverage increased (+**0.2%**) to **21.689%**

---

|  Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
| :-----|--------------|--------|---: |
| [internal/utils/misc.go](https://coveralls.io/builds/51437341/source?filename=internal%2Futils%2Fmisc.go#L124) | 1 | 3 | 33.33%
| [internal/db/branch/create/create.go](https://coveralls.io/builds/51437341/source?filename=internal%2Fdb%2Fbranch%2Fcreate%2Fcreate.go#L34) | 39 | 47 | 82.98%
<!-- | **Total:** | **41** | **51** | **80.39%** | -->

|  Files with Coverage Reduction | New Missed Lines | % |
| :-----|--------------|--: |
| [internal/debug/postgres.go](https://coveralls.io/builds/51437341/source?filename=internal%2Fdebug%2Fpostgres.go#L105) | 3 | 64.86% |
| [internal/utils/docker.go](https://coveralls.io/builds/51437341/source?filename=internal%2Futils%2Fdocker.go#L38) | 10 | 12.36% |
| [internal/utils/misc.go](https://coveralls.io/builds/51437341/source?filename=internal%2Futils%2Fmisc.go#L141) | 40 | 34.4% |
<!-- | **Total:** | **53** |  | -->

|  Totals | [![Coverage Status](https://coveralls.io/builds/51437341/badge)](https://coveralls.io/builds/51437341) |
| :-- | --: |
| Change from base [Build 2794992472](https://coveralls.io/builds/51408625): |  0.2% |
| Covered Lines: | 1112 |
| Relevant Lines: | 5127 |

---
##### 💛  - [Coveralls](https://coveralls.io)


Patch:
diff --git a/cmd/db.go b/cmd/db.go
index bb9c5b6a..d5a4dea7 100644
--- a/cmd/db.go
+++ b/cmd/db.go
@@ -33,7 +33,7 @@ var (
 		Short: "Create a branch.",
 		Args:  cobra.ExactArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			return create.Run(args[0])
+			return create.Run(args[0], afero.NewOsFs())
 		},
 	}
 
diff --git a/internal/db/branch/create/create.go b/internal/db/branch/create/create.go
index 3c5ef651..fafbf1a9 100644
--- a/internal/db/branch/create/create.go
+++ b/internal/db/branch/create/create.go
@@ -3,34 +3,62 @@ package create
 import (
 	"bytes"
 	"context"
+	_ "embed"
 	"errors"
 	"fmt"
 	"io"
 	"os"
-	"regexp"
+	"path/filepath"
 
+	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/pkg/stdcopy"
+	"github.com/spf13/afero"
 	"github.com/supabase/cli/internal/utils"
 )
 
-var ctx = context.Background()
+var (
+	//go:embed templates/clone.sh
+	cloneScript string
+)
+
+func Run(branch string, fsys afero.Fs) error {
+	if err := utils.LoadConfigFS(fsys); err != nil {
+		return err
+	}
+	if err := utils.AssertSupabaseDbIsRunning(); err != nil {
+		return err
+	}
+
+	branchPath := filepath.Join(filepath.Dir(utils.CurrBranchPath), branch)
+	if err := assertNewBranchIsValid(branchPath, fsys); err != nil {
+		return nil
+	}
 
-func Run(branch string) error {
-	if err := utils.AssertSupabaseStartIsRunning(); err != nil {
+	var ctx = context.Background()
+	if err := createBranch(ctx, branch); err != nil {
 		return err
 	}
 
+	if err := fsys.MkdirAll(branchPath, 0755); err != nil {
+		return err
+	}
+
+	fmt.Println("Created branch " + utils.Aqua(branch) + ".")
+	return nil
+}
+
+func assertNewBranchIsValid(branchPath string, fsys afero.Fs) error {
+	branch := filepath.Base(branchPath)
+
 	if utils.IsBranchNameReserved(branch) {
 		return errors.New("Cannot create branch " + utils.Aqua(branch) + ": branch name is reserved.")
 	}
 
-	if valid, err := regexp.MatchString(`[[:word:]-]+`, branch); err != nil {
-		return err
-	} else if !valid {
+	if !utils.BranchNamePattern.MatchString(branch) {
 		return errors.New("Branch name " + utils.Aqua(branch) + " is invalid. Must match [0-9A-Za-z_-]+.")
 	}
 
-	if _, err := os.ReadDir("supabase/.branches/" + branch); errors.Is(err, os.ErrNotExist) {
+	if _, err := afero.ReadDir(fsys, branchPath); errors.Is(err, os.ErrNotExist) {
 		// skip
 	} else if err != nil {
 		return err
@@ -38,59 +66,37 @@ func Run(branch string) error {
 		return errors.New("Branch " + utils.Aqua(branch) + " already exists.")
 	}
 
-	var dumpBuf bytes.Buffer
-	if err := func() error {
-		out, err := utils.DockerExec(ctx, utils.DbId, []string{
-			"pg_dump", "postgresql://postgres:postgres@localhost/postgres",
-		})
-		if err != nil {
-			return err
-		}
-
-		var errBuf bytes.Buffer
-		if _, err := stdcopy.StdCopy(&dumpBuf, &errBuf, out); err != nil {
-			return err
-		}
-		if errBuf.Len() > 0 {
-			return errors.New(errBuf.String())
-		}
+	return nil
+}
 
-		return nil
-	}(); err != nil {
-		return fmt.Errorf("Error creating branch: %w", err)
+func createBranch(ctx context.Context, branch string) error {
+	exec, err := utils.Docker.ContainerExecCreate(ctx, utils.DbId, types.ExecConfig{
+		Cmd:          []string{"/bin/bash", "-c", cloneScript},
+		Env:          []string{"DB_NAME=" + branch},
+		AttachStderr: true,
+		AttachStdout: true,
+	})
+	if err != nil {
+		return err
 	}
-
-	if err := func() error {
-		out, err := utils.DockerExec(ctx, utils.DbId, []string{
-			"sh", "-c", `psql --set ON_ERROR_STOP=on postgresql://postgres:postgres@localhost/postgres <<'EOSQL'
-CREATE DATABASE "` + branch + `";
-\connect ` + branch + `
-BEGIN;
-` + dumpBuf.String() + `
-COMMIT;
-EOSQL
-`,
-		})
-		if err != nil {
-			return err
-		}
-		var errBuf bytes.Buffer
-		if _, err := stdcopy.StdCopy(io.Discard, &errBuf, out); err != nil {
-			return err
-		}
-		if errBuf.Len() > 0 {
-			return errors.New(errBuf.String())
-		}
-
-		return nil
-	}(); err != nil {
-		return fmt.Errorf("Error creating branch %s: %w", utils.Aqua(branch), err)
+	// Read exec output
+	resp, err := utils.Docker.ContainerExecAttach(ctx, exec.ID, types.ExecStartCheck{})
+	if err != nil {
+		return err
 	}
-
-	if err := os.Mkdir("supabase/.branches/"+branch, 0755); err != nil {
+	defer resp.Close()
+	// Capture error details
+	var errBuf bytes.Buffer
+	if _, err := stdcopy.StdCopy(io.Discard, &errBuf, resp.Reader); err != nil {
 		return err
 	}
-
-	fmt.Println("Created branch " + utils.Aqua(branch) + ".")
+	// Get the exit code
+	iresp, err := utils.Docker.ContainerExecInspect(ctx, exec.ID)
+	if err != nil {
+		return err
+	}
+	if iresp.ExitCode > 0 {
+		return errors.New("Error creating branch: " + errBuf.String())
+	}
 	return nil
 }
diff --git a/internal/db/branch/create/templates/clone.sh b/internal/db/branch/create/templates/clone.sh
new file mode 100755
index 00000000..ac8d6cca
--- /dev/null
+++ b/internal/db/branch/create/templates/clone.sh
@@ -0,0 +1,5 @@
+#!/bin/bash
+set -euo pipefail
+
+createdb --username postgres --host 127.0.0.1 "$DB_NAME"
+pg_dump --username postgres --host 127.0.0.1 postgres | psql --username postgres --host 127.0.0.1 "$DB_NAME"
diff --git a/internal/utils/misc.go b/internal/utils/misc.go
index 61922f78..b876dd8d 100644
--- a/internal/utils/misc.go
+++ b/internal/utils/misc.go
@@ -64,6 +64,7 @@ var (
 	ProjectRefPattern  = regexp.MustCompile(`^[a-z]{20}$`)
 	PostgresUrlPattern = regexp.MustCompile(`^postgres(?:ql)?://postgres:(.+)@(.+?)(:\d+)?/postgres$`)
 	MigrateFilePattern = regexp.MustCompile(`([0-9]+)_.*\.sql`)
+	BranchNamePattern  = regexp.MustCompile(`[[:word:]-]+`)
 )
 
 func GetCurrentTimestamp() string {
@@ -119,6 +120,10 @@ func AssertSupabaseStartIsRunning() error {
 		return err
 	}
 
+	return AssertSupabaseDbIsRunning()
+}
+
+func AssertSupabaseDbIsRunning() error {
 	if _, err := Docker.ContainerInspect(context.Background(), DbId); err != nil {
 		return errors.New(Aqua("supabase start") + " is not running.")
 	}


Test patch:
diff --git a/internal/db/branch/create/create_test.go b/internal/db/branch/create/create_test.go
new file mode 100644
index 00000000..e57f217b
--- /dev/null
+++ b/internal/db/branch/create/create_test.go
@@ -0,0 +1,113 @@
+package create
+
+import (
+	"context"
+	"net/http"
+	"testing"
+
+	"github.com/docker/docker/api/types"
+	"github.com/docker/docker/client"
+	"github.com/spf13/afero"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"github.com/supabase/cli/internal/utils"
+	"gopkg.in/h2non/gock.v1"
+)
+
+func TestBranchValidation(t *testing.T) {
+	t.Run("branch name is valid", func(t *testing.T) {
+		assert.NoError(t, assertNewBranchIsValid("test-branch", afero.NewMemMapFs()))
+	})
+
+	t.Run("branch name is reserved", func(t *testing.T) {
+		assert.Error(t, assertNewBranchIsValid("main", afero.NewMemMapFs()))
+	})
+
+	t.Run("branch name is invalid", func(t *testing.T) {
+		assert.Error(t, assertNewBranchIsValid("@", afero.NewMemMapFs()))
+	})
+
+	t.Run("branch not a directory", func(t *testing.T) {
+		// Setup in-memory fs
+		fsys := afero.NewMemMapFs()
+		path := "/supabase/.branches/test-branch"
+		_, err := fsys.Create(path)
+		require.NoError(t, err)
+		// Run test
+		assert.Error(t, assertNewBranchIsValid(path, fsys))
+	})
+
+	t.Run("branch already exists", func(t *testing.T) {
+		// Setup in-memory fs
+		fsys := afero.NewMemMapFs()
+		path := "/supabase/.branches/test-branch"
+		require.NoError(t, fsys.MkdirAll(path, 0755))
+		// Run test
+		assert.Error(t, assertNewBranchIsValid(path, fsys))
+	})
+}
+
+func TestBranchCreation(t *testing.T) {
+	const version = "1.41"
+	utils.DbId = "test-db"
+
+	t.Run("docker exec failure", func(t *testing.T) {
+		// Setup mock docker
+		require.NoError(t, client.WithHTTPClient(http.DefaultClient)(utils.Docker))
+		defer gock.Off()
+		gock.New("http:///var/run/docker.sock").
+			Head("/_ping").
+			Reply(http.StatusOK).
+			SetHeader("API-Version", version).
+			SetHeader("OSType", "linux")
+		gock.New("http:///var/run/docker.sock").
+			Post("/v" + version + "/containers/" + utils.DbId + "/exec").
+			Reply(http.StatusServiceUnavailable)
+		// Run test
+		assert.Error(t, createBranch(context.Background(), "test-branch"))
+	})
+
+	t.Run("docker attach failure", func(t *testing.T) {
+		// Setup mock docker
+		require.NoError(t, client.WithHTTPClient(http.DefaultClient)(utils.Docker))
+		defer gock.Off()
+		gock.New("http:///var/run/docker.sock").
+			Head("/_ping").
+			Reply(http.StatusOK).
+			SetHeader("API-Version", version).
+			SetHeader("OSType", "linux")
+		gock.New("http:///var/run/docker.sock").
+			Post("/v" + version + "/containers/" + utils.DbId + "/exec").
+			Reply(http.StatusCreated).
+			JSON(types.ContainerJSON{})
+		// Run test
+		assert.Error(t, createBranch(context.Background(), "test-branch"))
+	})
+}
+
+func TestCreateCommand(t *testing.T) {
+	const (
+		version = "v1.41"
+		branch  = "test-branch"
+	)
+
+	t.Run("throws error on missing config", func(t *testing.T) {
+		assert.Error(t, Run(branch, afero.NewMemMapFs()))
+	})
+
+	t.Run("throws error on stopped db", func(t *testing.T) {
+		// Setup in-memory fs
+		fsys := &afero.MemMapFs{}
+		require.NoError(t, utils.WriteConfig(fsys, false))
+		// Setup mock docker
+		require.NoError(t, client.WithHTTPClient(http.DefaultClient)(utils.Docker))
+		defer gock.Off()
+		gock.New("http:///var/run/docker.sock").
+			Head("/_ping").
+			Reply(http.StatusOK).
+			SetHeader("API-Version", version).
+			SetHeader("OSType", "linux")
+		// Run test
+		assert.Error(t, Run(branch, fsys))
+	})
+}
diff --git a/test/branch_test.go b/test/branch_test.go
index 1d01fdf7..ad71167c 100644
--- a/test/branch_test.go
+++ b/test/branch_test.go
@@ -1,8 +1,10 @@
 package integration
 
 import (
+	"encoding/json"
 	"os"
 
+	"github.com/docker/docker/api/types"
 	"github.com/stretchr/testify/require"
 )
 
@@ -29,10 +31,9 @@ func (suite *DBTestSuite) TestBranchCreate() {
 	require.ElementsMatch(suite.T(), suite.params, ids)
 
 	// check commands in exec calls
-	require.ElementsMatch(suite.T(), suite.bodies, []string{
-		"{\"User\":\"\",\"Privileged\":false,\"Tty\":false,\"AttachStdin\":false,\"AttachStderr\":true,\"AttachStdout\":true,\"Detach\":false,\"DetachKeys\":\"\",\"Env\":null,\"WorkingDir\":\"\",\"Cmd\":[\"pg_dump\",\"postgresql://postgres:postgres@localhost/postgres\"]}\n",
-		"{\"Detach\":false,\"Tty\":false}\n",
-		"{\"User\":\"\",\"Privileged\":false,\"Tty\":false,\"AttachStdin\":false,\"AttachStderr\":true,\"AttachStdout\":true,\"Detach\":false,\"DetachKeys\":\"\",\"Env\":null,\"WorkingDir\":\"\",\"Cmd\":[\"sh\",\"-c\",\"psql --set ON_ERROR_STOP=on postgresql://postgres:postgres@localhost/postgres \\u003c\\u003c'EOSQL'\\nCREATE DATABASE \\\"" + branch + "\\\";\\n\\\\connect " + branch + "\\nBEGIN;\\nexit code 0\\nCOMMIT;\\nEOSQL\\n\"]}\n",
-		"{\"Detach\":false,\"Tty\":false}\n",
-	})
+	require.Equal(suite.T(), 2, len(suite.bodies))
+	var execBody types.ExecConfig
+	require.NoError(suite.T(), json.Unmarshal([]byte(suite.bodies[0]), &execBody))
+	var startBody types.ExecStartCheck
+	require.NoError(suite.T(), json.Unmarshal([]byte(suite.bodies[1]), &startBody))
 }
diff --git a/test/mocks/docker/server.go b/test/mocks/docker/server.go
index 7fffb2ec..bb028e38 100644
--- a/test/mocks/docker/server.go
+++ b/test/mocks/docker/server.go
@@ -49,6 +49,7 @@ func (s *Server) NewRouter() *gin.Engine {
 
 	exec := router.Group("/exec")
 	exec.POST("/:id/start", s.startExec)
+	exec.GET("/:id/json", s.inspectContainer)
 
 	return root
 }