Repository: supabase/cli
Pull request #1259 (2023-06-28 10:42:34)

fix: db reset should apply initial schema before service restart
## What kind of change does this PR introduce?

Bug fix https://github.com/supabase/cli/issues/1255

## What is the current behavior?

Please link any relevant issues here.

## What is the new behavior?

Feel free to include screenshots if it includes visual changes.

## Additional context

Add any other context or screenshots.


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

* **15** of **21**   **(71.43%)**  changed or added relevant lines in **1** file are covered.
* **12** unchanged lines in **2** files lost coverage.
* Overall coverage decreased (**-0.2%**) to **61.68%**

---

|  Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
| :-----|--------------|--------|---: |
| [internal/db/reset/reset.go](https://coveralls.io/builds/61012526/source?filename=internal%2Fdb%2Freset%2Freset.go#L67) | 15 | 21 | 71.43%
<!-- | **Total:** | **15** | **21** | **71.43%** | -->

|  Files with Coverage Reduction | New Missed Lines | % |
| :-----|--------------|--: |
| [internal/db/reset/reset.go](https://coveralls.io/builds/61012526/source?filename=internal%2Fdb%2Freset%2Freset.go#L69) | 5 | 87.88% |
| [internal/gen/keys/keys.go](https://coveralls.io/builds/61012526/source?filename=internal%2Fgen%2Fkeys%2Fkeys.go#L94) | 7 | 7.94% |
<!-- | **Total:** | **12** |  | -->

|  Totals | [![Coverage Status](https://coveralls.io/builds/61012526/badge)](https://coveralls.io/builds/61012526) |
| :-- | --: |
| Change from base [Build 5387961954](https://coveralls.io/builds/60981981): |  -0.2% |
| Covered Lines: | 4750 |
| Relevant Lines: | 7701 |

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


Patch:
diff --git a/internal/db/reset/reset.go b/internal/db/reset/reset.go
index 41375cc1..63226e35 100644
--- a/internal/db/reset/reset.go
+++ b/internal/db/reset/reset.go
@@ -64,25 +64,25 @@ func resetDatabase(ctx context.Context, fsys afero.Fs, options ...func(*pgx.Conn
 	if err := RecreateDatabase(ctx, options...); err != nil {
 		return err
 	}
-	defer RestartDatabase(context.Background(), os.Stderr)
-	return initDatabase(ctx, fsys, options...)
-}
-
-func initDatabase(ctx context.Context, fsys afero.Fs, options ...func(*pgx.ConnConfig)) error {
-	url := fmt.Sprintf("postgresql://supabase_admin:postgres@localhost:%d/postgres?connect_timeout=2", utils.Config.Db.Port)
-	conn, err := utils.ConnectByUrl(ctx, url, options...)
-	if err != nil {
+	if err := initDatabase(ctx, options...); err != nil {
 		return err
 	}
-	defer conn.Close(context.Background())
-	fmt.Fprintln(os.Stderr, "Initializing schema...")
-	if err := apply.BatchExecDDL(ctx, conn, strings.NewReader(utils.InitialSchemaSql)); err != nil {
+	RestartDatabase(context.Background(), os.Stderr)
+	conn, err := utils.ConnectLocalPostgres(ctx, pgconn.Config{}, options...)
+	if err != nil {
 		return err
 	}
-	if _, err := conn.Exec(ctx, SET_POSTGRES_ROLE); err != nil {
+	defer conn.Close(context.Background())
+	return InitialiseDatabase(ctx, conn, fsys)
+}
+
+func initDatabase(ctx context.Context, options ...func(*pgx.ConnConfig)) error {
+	conn, err := utils.ConnectLocalPostgres(ctx, pgconn.Config{User: "supabase_admin"}, options...)
+	if err != nil {
 		return err
 	}
-	return InitialiseDatabase(ctx, conn, fsys)
+	defer conn.Close(context.Background())
+	return apply.BatchExecDDL(ctx, conn, strings.NewReader(utils.InitialSchemaSql))
 }
 
 func InitialiseDatabase(ctx context.Context, conn *pgx.Conn, fsys afero.Fs) error {
@@ -94,8 +94,7 @@ func InitialiseDatabase(ctx context.Context, conn *pgx.Conn, fsys afero.Fs) erro
 
 // Recreate postgres database by connecting to template1
 func RecreateDatabase(ctx context.Context, options ...func(*pgx.ConnConfig)) error {
-	url := fmt.Sprintf("postgresql://supabase_admin:postgres@localhost:%d/template1?connect_timeout=2", utils.Config.Db.Port)
-	conn, err := utils.ConnectByUrl(ctx, url, options...)
+	conn, err := utils.ConnectLocalPostgres(ctx, pgconn.Config{User: "supabase_admin", Database: "template1"}, options...)
 	if err != nil {
 		return err
 	}
@@ -103,12 +102,14 @@ func RecreateDatabase(ctx context.Context, options ...func(*pgx.ConnConfig)) err
 	if err := DisconnectClients(ctx, conn); err != nil {
 		return err
 	}
-	drop := "DROP DATABASE IF EXISTS postgres WITH (FORCE);"
-	if _, err := conn.Exec(ctx, drop); err != nil {
-		return err
+	// We are not dropping roles here because they are cluster level entities. Use stop && start instead.
+	sql := repair.MigrationFile{
+		Lines: []string{
+			"DROP DATABASE IF EXISTS postgres WITH (FORCE)",
+			"CREATE DATABASE postgres WITH OWNER postgres",
+		},
 	}
-	_, err = conn.Exec(ctx, "CREATE DATABASE postgres WITH OWNER postgres;")
-	return err
+	return sql.ExecBatch(ctx, conn)
 }
 
 func SeedDatabase(ctx context.Context, conn *pgx.Conn, fsys afero.Fs) error {
@@ -133,10 +134,8 @@ func DisconnectClients(ctx context.Context, conn *pgx.Conn) error {
 		}
 	}
 	term := fmt.Sprintf(utils.TerminateDbSqlFmt, "postgres")
-	if _, err := conn.Exec(ctx, term); err != nil {
-		return err
-	}
-	return nil
+	_, err := conn.Exec(ctx, term)
+	return err
 }
 
 func RestartDatabase(ctx context.Context, w io.Writer) {
@@ -229,7 +228,7 @@ func ListSchemas(ctx context.Context, conn *pgx.Conn, exclude ...string) ([]stri
 		}
 		schemas = append(schemas, name)
 	}
-	return schemas, nil
+	return schemas, rows.Err()
 }
 
 func likeEscapeSchema(schemas []string) (result []string) {


Test patch:
diff --git a/internal/db/reset/reset_test.go b/internal/db/reset/reset_test.go
index 5f7c1b4f..798c07dd 100644
--- a/internal/db/reset/reset_test.go
+++ b/internal/db/reset/reset_test.go
@@ -75,25 +75,19 @@ func TestInitDatabase(t *testing.T) {
 	t.Run("initialises postgres database", func(t *testing.T) {
 		utils.Config.Db.Port = 54322
 		utils.InitialSchemaSql = "CREATE SCHEMA public"
-		// Setup in-memory fs
-		fsys := afero.NewMemMapFs()
 		// Setup mock postgres
 		conn := pgtest.NewConn()
 		defer conn.Close(t)
 		conn.Query(utils.InitialSchemaSql).
-			Reply("CREATE SCHEMA").
-			Query(SET_POSTGRES_ROLE).
-			Reply("SET ROLE")
+			Reply("CREATE SCHEMA")
 		// Run test
-		assert.NoError(t, initDatabase(context.Background(), fsys, conn.Intercept))
+		assert.NoError(t, initDatabase(context.Background(), conn.Intercept))
 	})
 
 	t.Run("throws error on connect failure", func(t *testing.T) {
 		utils.Config.Db.Port = 0
-		// Setup in-memory fs
-		fsys := afero.NewMemMapFs()
 		// Run test
-		err := initDatabase(context.Background(), fsys)
+		err := initDatabase(context.Background())
 		// Check error
 		assert.ErrorContains(t, err, "invalid port (outside range)")
 	})
@@ -101,55 +95,16 @@ func TestInitDatabase(t *testing.T) {
 	t.Run("throws error on duplicate schema", func(t *testing.T) {
 		utils.Config.Db.Port = 54322
 		utils.InitialSchemaSql = "CREATE SCHEMA public"
-		// Setup in-memory fs
-		fsys := afero.NewMemMapFs()
 		// Setup mock postgres
 		conn := pgtest.NewConn()
 		defer conn.Close(t)
 		conn.Query(utils.InitialSchemaSql).
 			ReplyError(pgerrcode.DuplicateSchema, `schema "public" already exists`)
 		// Run test
-		err := initDatabase(context.Background(), fsys, conn.Intercept)
+		err := initDatabase(context.Background(), conn.Intercept)
 		// Check error
 		assert.ErrorContains(t, err, `ERROR: schema "public" already exists (SQLSTATE 42P06)`)
 	})
-
-	t.Run("throws error on failure to switch role", func(t *testing.T) {
-		utils.Config.Db.Port = 54322
-		utils.InitialSchemaSql = "CREATE SCHEMA public"
-		// Setup in-memory fs
-		fsys := afero.NewMemMapFs()
-		// Setup mock postgres
-		conn := pgtest.NewConn()
-		defer conn.Close(t)
-		conn.Query(utils.InitialSchemaSql).
-			Reply("CREATE SCHEMA").
-			Query(SET_POSTGRES_ROLE).
-			Reply("SET ROLE").
-			ReplyError(pgerrcode.InvalidParameterValue, `role "test" does not exist`)
-		// Run test
-		err := initDatabase(context.Background(), fsys, conn.Intercept)
-		// Check error
-		assert.ErrorContains(t, err, `ERROR: role "test" does not exist (SQLSTATE 22023)`)
-	})
-
-	t.Run("throws error on migration failure", func(t *testing.T) {
-		utils.Config.Db.Port = 54322
-		utils.InitialSchemaSql = "CREATE SCHEMA public"
-		// Setup in-memory fs
-		fsys := &fstest.OpenErrorFs{DenyPath: utils.MigrationsDir}
-		// Setup mock postgres
-		conn := pgtest.NewConn()
-		defer conn.Close(t)
-		conn.Query(utils.InitialSchemaSql).
-			Reply("CREATE SCHEMA").
-			Query(SET_POSTGRES_ROLE).
-			Reply("SET ROLE")
-		// Run test
-		err := initDatabase(context.Background(), fsys, conn.Intercept)
-		// Check error
-		assert.ErrorIs(t, err, os.ErrPermission)
-	})
 }
 
 func TestSeedDatabase(t *testing.T) {
@@ -219,9 +174,9 @@ func TestRecreateDatabase(t *testing.T) {
 			Reply("ALTER DATABASE").
 			Query(fmt.Sprintf(utils.TerminateDbSqlFmt, "postgres")).
 			Reply("DO").
-			Query("DROP DATABASE IF EXISTS postgres WITH (FORCE);").
+			Query("DROP DATABASE IF EXISTS postgres WITH (FORCE)").
 			Reply("DROP DATABASE").
-			Query("CREATE DATABASE postgres WITH OWNER postgres;").
+			Query("CREATE DATABASE postgres WITH OWNER postgres").
 			Reply("CREATE DATABASE")
 		// Run test
 		assert.NoError(t, RecreateDatabase(context.Background(), conn.Intercept))
@@ -269,8 +224,9 @@ func TestRecreateDatabase(t *testing.T) {
 			Reply("ALTER DATABASE").
 			Query(fmt.Sprintf(utils.TerminateDbSqlFmt, "postgres")).
 			Reply("DO").
-			Query("DROP DATABASE IF EXISTS postgres WITH (FORCE);").
-			ReplyError(pgerrcode.ObjectInUse, `database "postgres" is used by an active logical replication slot`)
+			Query("DROP DATABASE IF EXISTS postgres WITH (FORCE)").
+			ReplyError(pgerrcode.ObjectInUse, `database "postgres" is used by an active logical replication slot`).
+			Query("CREATE DATABASE postgres WITH OWNER postgres")
 		// Run test
 		err := RecreateDatabase(context.Background(), conn.Intercept)
 		// Check error