From 05bf53329f4fa1d96c95690464fc51da795f7e38 Mon Sep 17 00:00:00 2001 From: Jessie Morris Date: Thu, 23 Jun 2022 20:42:43 -0600 Subject: [PATCH 1/4] Add test that replicates parallel problem in issue 285 --- sqlmock_test.go | 127 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 127 insertions(+) diff --git a/sqlmock_test.go b/sqlmock_test.go index ee6b516..bac72c3 100644 --- a/sqlmock_test.go +++ b/sqlmock_test.go @@ -394,6 +394,50 @@ func TestUnorderedPreparedQueryExecutions(t *testing.T) { } } +func TestParallelPreparedQueryExecutions(t *testing.T) { + db, mock, err := New() + if err != nil { + t.Errorf("an error '%s' was not expected when opening a stub database connection", err) + } + mock.MatchExpectationsInOrder(false) + + mock.ExpectPrepare("INSERT INTO authors \\((.+)\\) VALUES \\((.+)\\)"). + ExpectExec(). + WithArgs(1, "Jane Doe"). + WillReturnResult(NewResult(1, 1)) + + mock.ExpectPrepare("INSERT INTO authors \\((.+)\\) VALUES \\((.+)\\)"). + ExpectExec(). + WithArgs(0, "John Doe"). + WillReturnResult(NewResult(0, 1)) + + t.Run("Parallel1", func(t *testing.T) { + t.Parallel() + + stmt, err := db.Prepare("INSERT INTO authors (id, name) VALUES (?, ?)") + if err != nil { + t.Errorf("error '%s' was not expected while creating a prepared statement", err) + } else { + _, err = stmt.Exec(0, "John Doe") + } + }) + + t.Run("Parallel2", func(t *testing.T) { + t.Parallel() + + stmt, err := db.Prepare("INSERT INTO authors (id, name) VALUES (?, ?)") + if err != nil { + t.Errorf("error '%s' was not expected while creating a prepared statement", err) + } else { + _, err = stmt.Exec(1, "Jane Doe") + } + }) + + t.Cleanup(func() { + db.Close() + }) +} + func TestUnexpectedOperations(t *testing.T) { t.Parallel() db, mock, err := New() @@ -632,6 +676,89 @@ func TestGoroutineExecutionWithUnorderedExpectationMatching(t *testing.T) { // note this line is important for unordered expectation matching mock.MatchExpectationsInOrder(false) + data := []interface{}{ + 1, + "John Doe", + 2, + "Jane Doe", + } + rows := NewRows([]string{"id", "name"}) + rows.AddRow(data[0], data[1]) + rows.AddRow(data[2], data[3]) + + mock.ExpectExec("DROP TABLE IF EXISTS author").WillReturnResult(NewResult(0, 0)) + mock.ExpectExec("TRUNCATE TABLE").WillReturnResult(NewResult(0, 0)) + + mock.ExpectExec("CREATE TABLE IF NOT EXISTS author").WillReturnResult(NewResult(0, 0)) + + mock.ExpectQuery("SELECT").WillReturnRows(rows).WithArgs() + + mock.ExpectPrepare("INSERT INTO"). + ExpectExec(). + WithArgs( + data[0], + data[1], + data[2], + data[3], + ). + WillReturnResult(NewResult(0, 2)) + + var wg sync.WaitGroup + queries := []func() error{ + func() error { + _, err := db.Exec("CREATE TABLE IF NOT EXISTS author (a varchar(255)") + return err + }, + func() error { + _, err := db.Exec("TRUNCATE TABLE author") + return err + }, + func() error { + stmt, err := db.Prepare("INSERT INTO author (id,name) VALUES (?,?),(?,?)") + if err != nil { + return err + } + _, err = stmt.Exec(1, "John Doe", 2, "Jane Doe") + return err + }, + func() error { + _, err := db.Query("SELECT * FROM author") + return err + }, + func() error { + _, err := db.Exec("DROP TABLE IF EXISTS author") + return err + }, + } + + wg.Add(len(queries)) + for _, f := range queries { + go func(f func() error) { + if err := f(); err != nil { + t.Errorf("error was not expected: %s", err) + } + wg.Done() + }(f) + } + + wg.Wait() + + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("there were unfulfilled expectations: %s", err) + } +} + +func TestGoroutineExecutionMultiTypes(t *testing.T) { + t.Parallel() + db, mock, err := New() + if err != nil { + t.Errorf("an error '%s' was not expected when opening a stub database connection", err) + } + defer db.Close() + + // note this line is important for unordered expectation matching + mock.MatchExpectationsInOrder(false) + result := NewResult(1, 1) mock.ExpectExec("^UPDATE one").WithArgs("one").WillReturnResult(result) From 46af0dedfe29aaa3ac40aec461c88b1c958c602e Mon Sep 17 00:00:00 2001 From: Rajshree Sanjayam Date: Wed, 16 Aug 2023 13:53:50 +0530 Subject: [PATCH 2/4] fix for parallel calls with prepared stmts --- driver.go | 2 +- sqlmock.go | 37 ++++++++++++++++++++++++++++++++++++- sqlmock_test.go | 4 +++- 3 files changed, 40 insertions(+), 3 deletions(-) diff --git a/driver.go b/driver.go index 802f8fb..d336253 100644 --- a/driver.go +++ b/driver.go @@ -45,7 +45,7 @@ func New(options ...func(*sqlmock) error) (*sql.DB, Sqlmock, error) { dsn := fmt.Sprintf("sqlmock_db_%d", pool.counter) pool.counter++ - smock := &sqlmock{dsn: dsn, drv: pool, ordered: true} + smock := &sqlmock{dsn: dsn, drv: pool, ordered: true, expectedPrepareOnce: false} pool.conns[dsn] = smock pool.Unlock() diff --git a/sqlmock.go b/sqlmock.go index d074266..88489be 100644 --- a/sqlmock.go +++ b/sqlmock.go @@ -84,6 +84,16 @@ type SqlmockCommon interface { // sql driver.Value slice or from the CSV string and // to be used as sql driver.Rows. NewRows(columns []string) *Rows + + // ExpectPreparedQueryOnce gives an option to control addition + // and match of queries passed via prepared statements in expectations. + // By default this is set to false. + // + // If you use goroutines to parallelize your query execution using + // prepared stmts, this option helps to avoid building multiple expectations + // for same query. Otherwise when set to true, already matched prepared or + // executed query may lead to generation of unexpected query matches. + ExpectPreparedQueryOnce(bool) } type sqlmock struct { @@ -95,7 +105,8 @@ type sqlmock struct { queryMatcher QueryMatcher monitorPings bool - expected []expectation + expected []expectation + expectedPrepareOnce bool } func (c *sqlmock) open(options []func(*sqlmock) error) (*sql.DB, Sqlmock, error) { @@ -137,6 +148,10 @@ func (c *sqlmock) MatchExpectationsInOrder(b bool) { c.ordered = b } +func (c *sqlmock) ExpectPreparedQueryOnce(b bool) { + c.expectedPrepareOnce = b +} + // Close a mock database driver connection. It may or may not // be called depending on the circumstances, but if it is called // there must be an *ExpectedClose expectation satisfied. @@ -296,6 +311,16 @@ func (c *sqlmock) prepare(query string) (*ExpectedPrepare, error) { if next.fulfilled() { next.Unlock() fulfilled++ + + if c.expectedPrepareOnce { + if pr, ok := next.(*ExpectedPrepare); ok { + if err := c.queryMatcher.Match(pr.expectSQL, query); err == nil { + expected = pr + next.Lock() + break + } + } + } continue } @@ -334,6 +359,16 @@ func (c *sqlmock) prepare(query string) (*ExpectedPrepare, error) { } func (c *sqlmock) ExpectPrepare(expectedSQL string) *ExpectedPrepare { + if c.expectedPrepareOnce { + for _, e := range c.expected { + if ep, ok := e.(*ExpectedPrepare); ok { + if ep.expectSQL == expectedSQL { + return ep + } + } + } + } + e := &ExpectedPrepare{expectSQL: expectedSQL, mock: c} c.expected = append(c.expected, e) return e diff --git a/sqlmock_test.go b/sqlmock_test.go index bac72c3..aa23cb5 100644 --- a/sqlmock_test.go +++ b/sqlmock_test.go @@ -672,10 +672,12 @@ func TestGoroutineExecutionWithUnorderedExpectationMatching(t *testing.T) { t.Errorf("an error '%s' was not expected when opening a stub database connection", err) } defer db.Close() - // note this line is important for unordered expectation matching mock.MatchExpectationsInOrder(false) + // note this line is needed for parallel query expectation matching. + mock.ExpectPreparedQueryOnce(true) + data := []interface{}{ 1, "John Doe", From 84cfc9adec7cb07f7aec91ae5ccb6c409819c10e Mon Sep 17 00:00:00 2001 From: Rajshree Sanjayam Date: Wed, 16 Aug 2023 14:04:15 +0530 Subject: [PATCH 3/4] fix for parallel calls with prepared stmts --- driver.go | 2 +- sqlmock.go | 39 ++++++++++----------------------------- sqlmock_test.go | 3 --- 3 files changed, 11 insertions(+), 33 deletions(-) diff --git a/driver.go b/driver.go index d336253..802f8fb 100644 --- a/driver.go +++ b/driver.go @@ -45,7 +45,7 @@ func New(options ...func(*sqlmock) error) (*sql.DB, Sqlmock, error) { dsn := fmt.Sprintf("sqlmock_db_%d", pool.counter) pool.counter++ - smock := &sqlmock{dsn: dsn, drv: pool, ordered: true, expectedPrepareOnce: false} + smock := &sqlmock{dsn: dsn, drv: pool, ordered: true} pool.conns[dsn] = smock pool.Unlock() diff --git a/sqlmock.go b/sqlmock.go index 88489be..d1ed601 100644 --- a/sqlmock.go +++ b/sqlmock.go @@ -84,16 +84,6 @@ type SqlmockCommon interface { // sql driver.Value slice or from the CSV string and // to be used as sql driver.Rows. NewRows(columns []string) *Rows - - // ExpectPreparedQueryOnce gives an option to control addition - // and match of queries passed via prepared statements in expectations. - // By default this is set to false. - // - // If you use goroutines to parallelize your query execution using - // prepared stmts, this option helps to avoid building multiple expectations - // for same query. Otherwise when set to true, already matched prepared or - // executed query may lead to generation of unexpected query matches. - ExpectPreparedQueryOnce(bool) } type sqlmock struct { @@ -105,8 +95,7 @@ type sqlmock struct { queryMatcher QueryMatcher monitorPings bool - expected []expectation - expectedPrepareOnce bool + expected []expectation } func (c *sqlmock) open(options []func(*sqlmock) error) (*sql.DB, Sqlmock, error) { @@ -148,10 +137,6 @@ func (c *sqlmock) MatchExpectationsInOrder(b bool) { c.ordered = b } -func (c *sqlmock) ExpectPreparedQueryOnce(b bool) { - c.expectedPrepareOnce = b -} - // Close a mock database driver connection. It may or may not // be called depending on the circumstances, but if it is called // there must be an *ExpectedClose expectation satisfied. @@ -312,13 +297,11 @@ func (c *sqlmock) prepare(query string) (*ExpectedPrepare, error) { next.Unlock() fulfilled++ - if c.expectedPrepareOnce { - if pr, ok := next.(*ExpectedPrepare); ok { - if err := c.queryMatcher.Match(pr.expectSQL, query); err == nil { - expected = pr - next.Lock() - break - } + if pr, ok := next.(*ExpectedPrepare); ok { + if err := c.queryMatcher.Match(pr.expectSQL, query); err == nil { + expected = pr + next.Lock() + break } } continue @@ -359,12 +342,10 @@ func (c *sqlmock) prepare(query string) (*ExpectedPrepare, error) { } func (c *sqlmock) ExpectPrepare(expectedSQL string) *ExpectedPrepare { - if c.expectedPrepareOnce { - for _, e := range c.expected { - if ep, ok := e.(*ExpectedPrepare); ok { - if ep.expectSQL == expectedSQL { - return ep - } + for _, e := range c.expected { + if ep, ok := e.(*ExpectedPrepare); ok { + if ep.expectSQL == expectedSQL { + return ep } } } diff --git a/sqlmock_test.go b/sqlmock_test.go index aa23cb5..3573b63 100644 --- a/sqlmock_test.go +++ b/sqlmock_test.go @@ -675,9 +675,6 @@ func TestGoroutineExecutionWithUnorderedExpectationMatching(t *testing.T) { // note this line is important for unordered expectation matching mock.MatchExpectationsInOrder(false) - // note this line is needed for parallel query expectation matching. - mock.ExpectPreparedQueryOnce(true) - data := []interface{}{ 1, "John Doe", From d23f51393585929f6773cf0146ec1a85c0bae549 Mon Sep 17 00:00:00 2001 From: Rajshree Sanjayam Date: Wed, 16 Aug 2023 14:11:05 +0530 Subject: [PATCH 4/4] fix for parallel calls with prepared stmts --- sqlmock_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/sqlmock_test.go b/sqlmock_test.go index 3573b63..bac72c3 100644 --- a/sqlmock_test.go +++ b/sqlmock_test.go @@ -672,6 +672,7 @@ func TestGoroutineExecutionWithUnorderedExpectationMatching(t *testing.T) { t.Errorf("an error '%s' was not expected when opening a stub database connection", err) } defer db.Close() + // note this line is important for unordered expectation matching mock.MatchExpectationsInOrder(false)