From 52e2a1668ca9572e0b5ec638d82c4148bedc80fa Mon Sep 17 00:00:00 2001
From: Eric Chiang <eric.chiang@coreos.com>
Date: Mon, 31 Oct 2016 23:00:55 -0700
Subject: [PATCH] storage/sql: use isolation level "serializable" for
 transactions

---
 storage/sql/config_test.go | 27 +++++++++++++++++++++------
 storage/sql/sql.go         | 25 ++++++++++++++++++++++++-
 2 files changed, 45 insertions(+), 7 deletions(-)

diff --git a/storage/sql/config_test.go b/storage/sql/config_test.go
index 169e89ae..1afcf520 100644
--- a/storage/sql/config_test.go
+++ b/storage/sql/config_test.go
@@ -36,6 +36,7 @@ func cleanDB(c *conn) error {
 		delete from auth_code;
 		delete from refresh_token;
 		delete from keys;
+		delete from password;
 	`)
 	return err
 }
@@ -48,6 +49,7 @@ func TestSQLite3(t *testing.T) {
 		s := &SQLite3{":memory:"}
 		conn, err := s.open()
 		if err != nil {
+			fmt.Fprintln(os.Stdout, err)
 			t.Fatal(err)
 		}
 		return conn
@@ -58,15 +60,25 @@ func TestSQLite3(t *testing.T) {
 	})
 }
 
+func getenv(key, defaultVal string) string {
+	if val := os.Getenv(key); val != "" {
+		return val
+	}
+	return defaultVal
+}
+
+const testPostgresEnv = "DEX_POSTGRES_HOST"
+
 func TestPostgres(t *testing.T) {
-	if os.Getenv("DEX_POSTGRES_HOST") == "" {
-		t.Skip("postgres envs not set, skipping tests")
+	host := os.Getenv(testPostgresEnv)
+	if host == "" {
+		t.Skipf("test environment variable %q not set, skipping", testPostgresEnv)
 	}
 	p := Postgres{
-		Database: os.Getenv("DEX_POSTGRES_DATABASE"),
-		User:     os.Getenv("DEX_POSTGRES_USER"),
-		Password: os.Getenv("DEX_POSTGRES_PASSWORD"),
-		Host:     os.Getenv("DEX_POSTGRES_HOST"),
+		Database: getenv("DEX_POSTGRES_DATABASE", "postgres"),
+		User:     getenv("DEX_POSTGRES_USER", "postgres"),
+		Password: getenv("DEX_POSTGRES_PASSWORD", "postgres"),
+		Host:     host,
 		SSL: PostgresSSL{
 			Mode: sslDisable, // Postgres container doesn't support SSL.
 		},
@@ -92,4 +104,7 @@ func TestPostgres(t *testing.T) {
 	withTimeout(time.Minute*1, func() {
 		conformance.RunTests(t, newStorage)
 	})
+	withTimeout(time.Minute*1, func() {
+		conformance.RunTransactionTests(t, newStorage)
+	})
 }
diff --git a/storage/sql/sql.go b/storage/sql/sql.go
index 113fa972..02cc4f4a 100644
--- a/storage/sql/sql.go
+++ b/storage/sql/sql.go
@@ -45,7 +45,30 @@ func matchLiteral(s string) *regexp.Regexp {
 var (
 	// The "github.com/lib/pq" driver is the default flavor. All others are
 	// translations of this.
-	flavorPostgres = flavor{}
+	flavorPostgres = flavor{
+		// The default behavior for Postgres transactions is consistent reads, not consistent writes.
+		// For each transaction opened, ensure it has the correct isolation level.
+		//
+		// See: https://www.postgresql.org/docs/9.3/static/sql-set-transaction.html
+		//
+		// NOTE(ericchiang): For some reason using `SET SESSION CHARACTERISTICS AS TRANSACTION` at a
+		// session level didn't work for some edge cases. Might be something worth exploring.
+		executeTx: func(db *sql.DB, fn func(sqlTx *sql.Tx) error) error {
+			tx, err := db.Begin()
+			if err != nil {
+				return err
+			}
+			defer tx.Rollback()
+
+			if _, err := tx.Exec(`SET TRANSACTION ISOLATION LEVEL SERIALIZABLE;`); err != nil {
+				return err
+			}
+			if err := fn(tx); err != nil {
+				return err
+			}
+			return tx.Commit()
+		},
+	}
 
 	flavorSQLite3 = flavor{
 		queryReplacers: []replacer{