Don't call pre_ping for fresh connection

The pool "pre-ping" feature has been refined to not invoke for a DBAPI
connection that was just opened in the same checkout operation.  pre ping
only applies to a DBAPI connection that's been checked into the pool
and is being checked out again.

Fixes: #4524
Change-Id: Ibe3dfb709dbdc24aa94e96513cfbea456c33b895

1 files changed, 82 insertions, 4 deletions

diff --git a/test/engine/test_reconnect.py b/test/engine/test_reconnect.py
index 481700e70..205c1fb31 100644
--- a/test/engine/test_reconnect.py
+++ b/test/engine/test_reconnect.py
@@ -18,6 +18,7 @@ from sqlalchemy.testing import engines
 from sqlalchemy.testing import eq_
 from sqlalchemy.testing import expect_warnings
 from sqlalchemy.testing import fixtures
+from sqlalchemy.testing import is_
 from sqlalchemy.testing import is_false
 from sqlalchemy.testing import is_true
 from sqlalchemy.testing import mock
@@ -149,7 +150,7 @@ class PrePingMockTest(fixtures.TestBase):
     def setup(self):
         self.dbapi = MockDBAPI()
 
-    def _pool_fixture(self, pre_ping):
+    def _pool_fixture(self, pre_ping, pool_kw=None):
         dialect = url.make_url(
             "postgresql://foo:bar@localhost/test"
         ).get_dialect()()
@@ -158,6 +159,7 @@ class PrePingMockTest(fixtures.TestBase):
             creator=lambda: self.dbapi.connect("foo.db"),
             pre_ping=pre_ping,
             dialect=dialect,
+            **(pool_kw if pool_kw else {})
         )
 
         dialect.is_disconnect = lambda e, conn, cursor: isinstance(
@@ -168,6 +170,66 @@ class PrePingMockTest(fixtures.TestBase):
     def teardown(self):
         self.dbapi.dispose()
 
+    def test_ping_not_on_first_connect(self):
+        pool = self._pool_fixture(
+            pre_ping=True, pool_kw=dict(pool_size=1, max_overflow=0)
+        )
+
+        conn = pool.connect()
+        dbapi_conn = conn.connection
+        eq_(dbapi_conn.mock_calls, [])
+        conn.close()
+
+        # no ping, so no cursor() call.
+        eq_(dbapi_conn.mock_calls, [call.rollback()])
+
+        conn = pool.connect()
+        is_(conn.connection, dbapi_conn)
+
+        # ping, so cursor() call.
+        eq_(dbapi_conn.mock_calls, [call.rollback(), call.cursor()])
+
+        conn.close()
+
+        conn = pool.connect()
+        is_(conn.connection, dbapi_conn)
+
+        # ping, so cursor() call.
+        eq_(
+            dbapi_conn.mock_calls,
+            [call.rollback(), call.cursor(), call.rollback(), call.cursor()],
+        )
+
+        conn.close()
+
+    def test_ping_not_on_reconnect(self):
+        pool = self._pool_fixture(
+            pre_ping=True, pool_kw=dict(pool_size=1, max_overflow=0)
+        )
+
+        conn = pool.connect()
+        dbapi_conn = conn.connection
+        conn_rec = conn._connection_record
+        eq_(dbapi_conn.mock_calls, [])
+        conn.close()
+
+        conn = pool.connect()
+        is_(conn.connection, dbapi_conn)
+        # ping, so cursor() call.
+        eq_(dbapi_conn.mock_calls, [call.rollback(), call.cursor()])
+
+        conn.invalidate()
+
+        is_(conn.connection, None)
+
+        # connect again, make sure we're on the same connection record
+        conn = pool.connect()
+        is_(conn._connection_record, conn_rec)
+
+        # no ping
+        dbapi_conn = conn.connection
+        eq_(dbapi_conn.mock_calls, [])
+
     def test_connect_across_restart(self):
         pool = self._pool_fixture(pre_ping=True)
 
@@ -242,7 +304,17 @@ class PrePingMockTest(fixtures.TestBase):
         old_dbapi_conn = conn.connection
         conn.close()
 
-        eq_(old_dbapi_conn.mock_calls, [call.cursor(), call.rollback()])
+        # no cursor() because no pre ping
+        eq_(old_dbapi_conn.mock_calls, [call.rollback()])
+
+        conn = pool.connect()
+        conn.close()
+
+        # connect again, we see pre-ping
+        eq_(
+            old_dbapi_conn.mock_calls,
+            [call.rollback(), call.cursor(), call.rollback()],
+        )
 
         self.dbapi.shutdown("execute", stop=True)
         self.dbapi.restart()
@@ -253,13 +325,19 @@ class PrePingMockTest(fixtures.TestBase):
         gc_collect()
 
         # new connection was reset on return appropriately
-        eq_(dbapi_conn.mock_calls, [call.cursor(), call.rollback()])
+        eq_(dbapi_conn.mock_calls, [call.rollback()])
 
         # old connection was just closed - did not get an
         # erroneous reset on return
         eq_(
             old_dbapi_conn.mock_calls,
-            [call.cursor(), call.rollback(), call.cursor(), call.close()],
+            [
+                call.rollback(),
+                call.cursor(),
+                call.rollback(),
+                call.cursor(),
+                call.close(),
+            ],
         )