database/sql: add test for Conn.Validator interface

This addresses comments made by Russ after
https://golang.org/cl/174122 was merged. It addes a test
for the connection validator and renames the interface to just
"Validator".

Change-Id: Iea53e9b250c9be2e86e9b75906e7353e26437c5c
Reviewed-on: https://go-review.googlesource.com/c/go/+/223963
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>

diff --git a/src/database/sql/driver/driver.go b/src/database/sql/driver/driver.go
index a2b844d71f98a00c1c39cd8ba71fa85e12aec6d7..76f1bd3aa13e1be3a19581133527cda8c85db321 100644 (file)
--- a/src/database/sql/driver/driver.go
+++ b/src/database/sql/driver/driver.go
@@ -261,15 +261,15 @@ type SessionResetter interface {
        ResetSession(ctx context.Context) error
 }
 
-// ConnectionValidator may be implemented by Conn to allow drivers to
+// Validator may be implemented by Conn to allow drivers to
 // signal if a connection is valid or if it should be discarded.
 //
 // If implemented, drivers may return the underlying error from queries,
 // even if the connection should be discarded by the connection pool.
-type ConnectionValidator interface {
-       // ValidConnection is called prior to placing the connection into the
+type Validator interface {
+       // IsValid is called prior to placing the connection into the
        // connection pool. The connection will be discarded if false is returned.
-       ValidConnection() bool
+       IsValid() bool
 }
 
 // Result is the result of a query execution.

diff --git a/src/database/sql/fakedb_test.go b/src/database/sql/fakedb_test.go
index 73dab101b73c1607c0847db70d295206c6dc1502..b6e9a5707e9e6b16cbdb7cafc34da30cdd0eab4b 100644 (file)
--- a/src/database/sql/fakedb_test.go
+++ b/src/database/sql/fakedb_test.go
@@ -396,9 +396,9 @@ func (c *fakeConn) ResetSession(ctx context.Context) error {
        return nil
 }
 
-var _ driver.ConnectionValidator = (*fakeConn)(nil)
+var _ driver.Validator = (*fakeConn)(nil)
 
-func (c *fakeConn) ValidConnection() bool {
+func (c *fakeConn) IsValid() bool {
        return !c.isBad()
 }
 

diff --git a/src/database/sql/sql.go b/src/database/sql/sql.go
index 95906b1318aef5868f893b0969f61b39bd61b2e4..4093ffe1bb064559958111c1598e8d4f278fce85 100644 (file)
--- a/src/database/sql/sql.go
+++ b/src/database/sql/sql.go
@@ -512,8 +512,8 @@ func (dc *driverConn) validateConnection(needsReset bool) bool {
        if needsReset {
                dc.needReset = true
        }
-       if cv, ok := dc.ci.(driver.ConnectionValidator); ok {
-               return cv.ValidConnection()
+       if cv, ok := dc.ci.(driver.Validator); ok {
+               return cv.IsValid()
        }
        return true
 }

diff --git a/src/database/sql/sql_test.go b/src/database/sql/sql_test.go
index 0fc994d0a1a1b4e69cdc9f630460cf0811b95b6d..f08eba93b3c51a1705b993a695f06567bb672eaa 100644 (file)
--- a/src/database/sql/sql_test.go
+++ b/src/database/sql/sql_test.go
@@ -1543,6 +1543,37 @@ func TestConnTx(t *testing.T) {
        }
 }
 
+// TestConnIsValid verifies that a database connection that should be discarded,
+// is actually discarded and does not re-enter the connection pool.
+// If the IsValid method from *fakeConn is removed, this test will fail.
+func TestConnIsValid(t *testing.T) {
+       db := newTestDB(t, "people")
+       defer closeDB(t, db)
+
+       db.SetMaxOpenConns(1)
+
+       ctx := context.Background()
+
+       c, err := db.Conn(ctx)
+       if err != nil {
+               t.Fatal(err)
+       }
+
+       err = c.Raw(func(raw interface{}) error {
+               dc := raw.(*fakeConn)
+               dc.stickyBad = true
+               return nil
+       })
+       if err != nil {
+               t.Fatal(err)
+       }
+       c.Close()
+
+       if len(db.freeConn) > 0 && db.freeConn[0].ci.(*fakeConn).stickyBad {
+               t.Fatal("bad connection returned to pool; expected bad connection to be discarded")
+       }
+}
+
 // Tests fix for issue 2542, that we release a lock when querying on
 // a closed connection.
 func TestIssue2542Deadlock(t *testing.T) {