From 42e21a40c5e42a49fd795f453d7ef13a03ce8198 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 3 Nov 2023 11:02:09 -0400 Subject: [PATCH 01/15] Basic upsert tests. --- tests/storage/test_base.py | 60 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/tests/storage/test_base.py b/tests/storage/test_base.py index e4a52c301ed2..fe040b1eac5e 100644 --- a/tests/storage/test_base.py +++ b/tests/storage/test_base.py @@ -209,3 +209,63 @@ def test_delete_one(self) -> Generator["defer.Deferred[object]", object, None]: self.mock_txn.execute.assert_called_with( "DELETE FROM tablename WHERE keycol = ?", ["Go away"] ) + + @defer.inlineCallbacks + def test_upsert(self) -> Generator["defer.Deferred[object]", object, None]: + self.mock_txn.rowcount = 1 + + result = yield defer.ensureDeferred( + self.datastore.db_pool.simple_upsert( + table="tablename", + keyvalues={"columnname": "oldvalue"}, + values={"othercol": "newvalue"}, + ) + ) + + self.mock_txn.execute.assert_called_with( + "INSERT INTO tablename (columnname, othercol) VALUES (?, ?) ON CONFLICT (columnname) DO UPDATE SET othercol=EXCLUDED.othercol", + ["oldvalue", "newvalue"], + ) + self.assertTrue(result) + + @defer.inlineCallbacks + def test_upsert_with_insert( + self, + ) -> Generator["defer.Deferred[object]", object, None]: + self.mock_txn.rowcount = 1 + + result = yield defer.ensureDeferred( + self.datastore.db_pool.simple_upsert( + table="tablename", + keyvalues={"columnname": "oldvalue"}, + values={"othercol": "newvalue"}, + insertion_values={"thirdcol": "insertionval"}, + ) + ) + + self.mock_txn.execute.assert_called_with( + "INSERT INTO tablename (columnname, thirdcol, othercol) VALUES (?, ?, ?) ON CONFLICT (columnname) DO UPDATE SET othercol=EXCLUDED.othercol", + ["oldvalue", "insertionval", "newvalue"], + ) + self.assertTrue(result) + + @defer.inlineCallbacks + def test_upsert_with_where( + self, + ) -> Generator["defer.Deferred[object]", object, None]: + self.mock_txn.rowcount = 1 + + result = yield defer.ensureDeferred( + self.datastore.db_pool.simple_upsert( + table="tablename", + keyvalues={"columnname": "oldvalue"}, + values={"othercol": "newvalue"}, + where_clause="thirdcol IS NULL", + ) + ) + + self.mock_txn.execute.assert_called_with( + "INSERT INTO tablename (columnname, othercol) VALUES (?, ?) ON CONFLICT (columnname) WHERE thirdcol IS NULL DO UPDATE SET othercol=EXCLUDED.othercol", + ["oldvalue", "newvalue"], + ) + self.assertTrue(result) From 3da39af91dd8daf726e91f0edaaf6a318e425a6f Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 3 Nov 2023 11:39:43 -0400 Subject: [PATCH 02/15] Add basic upsert_many tests. --- tests/storage/test_base.py | 42 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/tests/storage/test_base.py b/tests/storage/test_base.py index fe040b1eac5e..7a673a495c05 100644 --- a/tests/storage/test_base.py +++ b/tests/storage/test_base.py @@ -269,3 +269,45 @@ def test_upsert_with_where( ["oldvalue", "newvalue"], ) self.assertTrue(result) + + @defer.inlineCallbacks + def test_upsert_many(self) -> Generator["defer.Deferred[object]", object, None]: + yield defer.ensureDeferred( + self.datastore.db_pool.simple_upsert_many( + table="tablename", + key_names=["columnname"], + key_values=[["oldvalue"]], + value_names=["othercol"], + value_values=[["newvalue"]], + desc="", + ) + ) + + # TODO Test postgres variant. + + self.mock_txn.executemany.assert_called_with( + "INSERT INTO tablename (columnname, othercol) VALUES (?, ?) ON CONFLICT (columnname) DO UPDATE SET othercol=EXCLUDED.othercol", + [("oldvalue", "newvalue")], + ) + + @defer.inlineCallbacks + def test_upsert_many_no_values( + self, + ) -> Generator["defer.Deferred[object]", object, None]: + yield defer.ensureDeferred( + self.datastore.db_pool.simple_upsert_many( + table="tablename", + key_names=["columnname"], + key_values=[["oldvalue"]], + value_names=[], + value_values=[], + desc="", + ) + ) + + # TODO Test postgres variant. + + self.mock_txn.executemany.assert_called_with( + "INSERT INTO tablename (columnname) VALUES (?) ON CONFLICT (columnname) DO NOTHING", + [("oldvalue",)], + ) From a5348d56fe38f356e9c617231c1cb244d3004f5c Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 3 Nov 2023 12:04:34 -0400 Subject: [PATCH 03/15] Test insert_many and update_many. --- synapse/storage/database.py | 4 +-- tests/storage/test_base.py | 60 +++++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 3 deletions(-) diff --git a/synapse/storage/database.py b/synapse/storage/database.py index 6d54bb0eb234..9843e25e1011 100644 --- a/synapse/storage/database.py +++ b/synapse/storage/database.py @@ -2062,9 +2062,7 @@ def simple_update_many_txn( where_clause = "" # UPDATE mytable SET col1 = ?, col2 = ? WHERE col3 = ? AND col4 = ? - sql = f""" - UPDATE {table} SET {set_clause} {where_clause} - """ + sql = f"UPDATE {table} SET {set_clause} {where_clause}" txn.execute_batch(sql, args) diff --git a/tests/storage/test_base.py b/tests/storage/test_base.py index 7a673a495c05..0b923f3db5eb 100644 --- a/tests/storage/test_base.py +++ b/tests/storage/test_base.py @@ -55,6 +55,8 @@ def runWithConnection(func, *args, **kwargs): # type: ignore[no-untyped-def] engine = create_engine(sqlite_config) fake_engine = Mock(wraps=engine) fake_engine.in_transaction.return_value = False + fake_engine.module.OperationalError = engine.module.OperationalError + fake_engine.module.DatabaseError = engine.module.DatabaseError db = DatabasePool(Mock(), Mock(config=sqlite_config), fake_engine) db._db_pool = self.db_pool @@ -91,6 +93,33 @@ def test_insert_3cols(self) -> Generator["defer.Deferred[object]", object, None] "INSERT INTO tablename (colA, colB, colC) VALUES(?, ?, ?)", (1, 2, 3) ) + @defer.inlineCallbacks + def test_insert_many(self) -> Generator["defer.Deferred[object]", object, None]: + yield defer.ensureDeferred( + self.datastore.db_pool.simple_insert_many( + table="tablename", + keys=( + "col1", + "col2", + ), + values=[ + ( + "val1", + "val2", + ), + ("val3", "val4"), + ], + desc="", + ) + ) + + # TODO Test postgres variant. + + self.mock_txn.executemany.assert_called_with( + "INSERT INTO tablename (col1, col2) VALUES(?, ?)", + [("val1", "val2"), ("val3", "val4")], + ) + @defer.inlineCallbacks def test_select_one_1col(self) -> Generator["defer.Deferred[object]", object, None]: self.mock_txn.rowcount = 1 @@ -196,6 +225,37 @@ def test_update_one_4cols( [3, 4, 1, 2], ) + @defer.inlineCallbacks + def test_update_many(self) -> Generator["defer.Deferred[object]", object, None]: + yield defer.ensureDeferred( + self.datastore.db_pool.simple_update_many( + table="tablename", + key_names=("col1", "col2"), + key_values=[("val1", "val2")], + value_names=("col3",), + value_values=[("val3",)], + desc="", + ) + ) + + self.mock_txn.executemany.assert_called_with( + "UPDATE tablename SET col3 = ? WHERE col1 = ? AND col2 = ?", + [("val3", "val1", "val2"), ("val3", "val1", "val2")], + ) + + # key_values and value_values must be the same length. + with self.assertRaises(ValueError): + yield defer.ensureDeferred( + self.datastore.db_pool.simple_update_many( + table="tablename", + key_names=("col1", "col2"), + key_values=[("val1", "val2")], + value_names=("col3",), + value_values=[], + desc="", + ) + ) + @defer.inlineCallbacks def test_delete_one(self) -> Generator["defer.Deferred[object]", object, None]: self.mock_txn.rowcount = 1 From 0edc67cf5ff5c14123cc72bec369b99f7ef14e9d Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 3 Nov 2023 12:10:57 -0400 Subject: [PATCH 04/15] Add tests for delete_many. --- synapse/storage/database.py | 5 +---- tests/storage/test_base.py | 41 +++++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/synapse/storage/database.py b/synapse/storage/database.py index 9843e25e1011..479320b646f8 100644 --- a/synapse/storage/database.py +++ b/synapse/storage/database.py @@ -2281,8 +2281,6 @@ def simple_delete_many_txn( if not values: return 0 - sql = "DELETE FROM %s" % table - clause, values = make_in_list_sql_clause(txn.database_engine, column, values) clauses = [clause] @@ -2290,8 +2288,7 @@ def simple_delete_many_txn( clauses.append("%s = ?" % (key,)) values.append(value) - if clauses: - sql = "%s WHERE %s" % (sql, " AND ".join(clauses)) + sql = "DELETE FROM %s WHERE %s" % (table, " AND ".join(clauses)) txn.execute(sql, values) return txn.rowcount diff --git a/tests/storage/test_base.py b/tests/storage/test_base.py index 0b923f3db5eb..347bef631e26 100644 --- a/tests/storage/test_base.py +++ b/tests/storage/test_base.py @@ -270,6 +270,47 @@ def test_delete_one(self) -> Generator["defer.Deferred[object]", object, None]: "DELETE FROM tablename WHERE keycol = ?", ["Go away"] ) + @defer.inlineCallbacks + def test_delete_many(self) -> Generator["defer.Deferred[object]", object, None]: + self.mock_txn.rowcount = 2 + + result = yield defer.ensureDeferred( + self.datastore.db_pool.simple_delete_many( + table="tablename", + column="col1", + iterable=("val1", "val2"), + keyvalues={"col2": "val3"}, + desc="", + ) + ) + + self.mock_txn.execute.assert_called_with( + "DELETE FROM tablename WHERE col1 = ANY(?) AND col2 = ?", + [["val1", "val2"], "val3"], + ) + self.assertEqual(result, 2) + + @defer.inlineCallbacks + def test_delete_many_no_keyvalues( + self, + ) -> Generator["defer.Deferred[object]", object, None]: + self.mock_txn.rowcount = 2 + + result = yield defer.ensureDeferred( + self.datastore.db_pool.simple_delete_many( + table="tablename", + column="col1", + iterable=("val1", "val2"), + keyvalues={}, + desc="", + ) + ) + + self.mock_txn.execute.assert_called_with( + "DELETE FROM tablename WHERE col1 = ANY(?)", [["val1", "val2"]] + ) + self.assertEqual(result, 2) + @defer.inlineCallbacks def test_upsert(self) -> Generator["defer.Deferred[object]", object, None]: self.mock_txn.rowcount = 1 From 6252b310d9c2bfd975fbcd93890ab86fbf3c0b75 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 3 Nov 2023 12:30:50 -0400 Subject: [PATCH 05/15] Test select_many_batch. --- tests/storage/test_base.py | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/tests/storage/test_base.py b/tests/storage/test_base.py index 347bef631e26..af29c9e1b35f 100644 --- a/tests/storage/test_base.py +++ b/tests/storage/test_base.py @@ -14,7 +14,7 @@ from collections import OrderedDict from typing import Generator -from unittest.mock import Mock +from unittest.mock import Mock, call from twisted.internet import defer @@ -189,6 +189,38 @@ def test_select_list(self) -> Generator["defer.Deferred[object]", object, None]: "SELECT colA FROM tablename WHERE keycol = ?", ["A set"] ) + @defer.inlineCallbacks + def test_select_many_batch( + self, + ) -> Generator["defer.Deferred[object]", object, None]: + self.mock_txn.rowcount = 3 + self.mock_txn.fetchall.side_effect = [[(1,), (2,)], [(3,)]] + + ret = yield defer.ensureDeferred( + self.datastore.db_pool.simple_select_many_batch( + table="tablename", + column="col1", + iterable=("val1", "val2", "val3"), + retcols=("col2",), + keyvalues={"col3": "val4"}, + batch_size=2, + ) + ) + + self.mock_txn.execute.assert_has_calls( + [ + call( + "SELECT col2 FROM tablename WHERE col1 = ANY(?) AND col3 = ?", + [["val1", "val2"], "val4"], + ), + call( + "SELECT col2 FROM tablename WHERE col1 = ANY(?) AND col3 = ?", + [["val3"], "val4"], + ), + ], + ) + self.assertEqual([(1,), (2,), (3,)], ret) + @defer.inlineCallbacks def test_update_one_1col(self) -> Generator["defer.Deferred[object]", object, None]: self.mock_txn.rowcount = 1 From 926780c194e68eb5cacd837ba3fd1cb1142158ca Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 3 Nov 2023 12:34:32 -0400 Subject: [PATCH 06/15] Add tests for not having iterables. --- tests/storage/test_base.py | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/tests/storage/test_base.py b/tests/storage/test_base.py index af29c9e1b35f..c2af55dc963c 100644 --- a/tests/storage/test_base.py +++ b/tests/storage/test_base.py @@ -221,6 +221,22 @@ def test_select_many_batch( ) self.assertEqual([(1,), (2,), (3,)], ret) + def test_select_many_no_iterable(self) -> None: + self.mock_txn.rowcount = 3 + self.mock_txn.fetchall.side_effect = [(1,), (2,)] + + ret = self.datastore.db_pool.simple_select_many_txn( + self.mock_txn, + table="tablename", + column="col1", + iterable=(), + retcols=("col2",), + keyvalues={"col3": "val4"}, + ) + + self.mock_txn.execute.assert_not_called() + self.assertEqual([], ret) + @defer.inlineCallbacks def test_update_one_1col(self) -> Generator["defer.Deferred[object]", object, None]: self.mock_txn.rowcount = 1 @@ -322,6 +338,23 @@ def test_delete_many(self) -> Generator["defer.Deferred[object]", object, None]: ) self.assertEqual(result, 2) + @defer.inlineCallbacks + def test_delete_many_no_iterable( + self, + ) -> Generator["defer.Deferred[object]", object, None]: + result = yield defer.ensureDeferred( + self.datastore.db_pool.simple_delete_many( + table="tablename", + column="col1", + iterable=(), + keyvalues={"col2": "val3"}, + desc="", + ) + ) + + self.mock_txn.execute.assert_not_called() + self.assertEqual(result, 0) + @defer.inlineCallbacks def test_delete_many_no_keyvalues( self, From 7ac489914f1aba17b706015e0fe5f7f8c1082724 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 3 Nov 2023 12:46:26 -0400 Subject: [PATCH 07/15] Newsfragment --- changelog.d/16596.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/16596.misc diff --git a/changelog.d/16596.misc b/changelog.d/16596.misc new file mode 100644 index 000000000000..fa457b12e5e0 --- /dev/null +++ b/changelog.d/16596.misc @@ -0,0 +1 @@ +Improve tests of the SQL generator. From ff58a471cad4fe1d401dcb49c8d42569e32d1bcc Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 6 Nov 2023 11:19:53 -0500 Subject: [PATCH 08/15] Test simple_upsert_many with multiple rows. --- tests/storage/test_base.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/storage/test_base.py b/tests/storage/test_base.py index c2af55dc963c..12dee76e6e08 100644 --- a/tests/storage/test_base.py +++ b/tests/storage/test_base.py @@ -441,10 +441,10 @@ def test_upsert_many(self) -> Generator["defer.Deferred[object]", object, None]: yield defer.ensureDeferred( self.datastore.db_pool.simple_upsert_many( table="tablename", - key_names=["columnname"], - key_values=[["oldvalue"]], - value_names=["othercol"], - value_values=[["newvalue"]], + key_names=["keycol1", "keycol2"], + key_values=[["keyval1", "keyval2"], ["keyval3", "keyval4"]], + value_names=["valuecol3"], + value_values=[["val5"], ["val6"]], desc="", ) ) @@ -452,8 +452,8 @@ def test_upsert_many(self) -> Generator["defer.Deferred[object]", object, None]: # TODO Test postgres variant. self.mock_txn.executemany.assert_called_with( - "INSERT INTO tablename (columnname, othercol) VALUES (?, ?) ON CONFLICT (columnname) DO UPDATE SET othercol=EXCLUDED.othercol", - [("oldvalue", "newvalue")], + "INSERT INTO tablename (keycol1, keycol2, valuecol3) VALUES (?, ?, ?) ON CONFLICT (keycol1, keycol2) DO UPDATE SET valuecol3=EXCLUDED.valuecol3", + [("keyval1", "keyval2", "val5"), ("keyval3", "keyval4", "val6")], ) @defer.inlineCallbacks From 80a7f75e4dc4d8612c61f02faa8b17bcc8af54d0 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 6 Nov 2023 11:23:33 -0500 Subject: [PATCH 09/15] Avoid double space. --- synapse/storage/database.py | 4 ++-- tests/storage/test_base.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/synapse/storage/database.py b/synapse/storage/database.py index 479320b646f8..abc7d8a5d268 100644 --- a/synapse/storage/database.py +++ b/synapse/storage/database.py @@ -1401,12 +1401,12 @@ def simple_upsert_txn_native_upsert( allvalues.update(values) latter = "UPDATE SET " + ", ".join(k + "=EXCLUDED." + k for k in values) - sql = "INSERT INTO %s (%s) VALUES (%s) ON CONFLICT (%s) %s DO %s" % ( + sql = "INSERT INTO %s (%s) VALUES (%s) ON CONFLICT (%s) %sDO %s" % ( table, ", ".join(k for k in allvalues), ", ".join("?" for _ in allvalues), ", ".join(k for k in keyvalues), - f"WHERE {where_clause}" if where_clause else "", + f"WHERE {where_clause} " if where_clause else "", latter, ) txn.execute(sql, list(allvalues.values())) diff --git a/tests/storage/test_base.py b/tests/storage/test_base.py index 12dee76e6e08..738e83aa8eb3 100644 --- a/tests/storage/test_base.py +++ b/tests/storage/test_base.py @@ -389,7 +389,7 @@ def test_upsert(self) -> Generator["defer.Deferred[object]", object, None]: ) self.mock_txn.execute.assert_called_with( - "INSERT INTO tablename (columnname, othercol) VALUES (?, ?) ON CONFLICT (columnname) DO UPDATE SET othercol=EXCLUDED.othercol", + "INSERT INTO tablename (columnname, othercol) VALUES (?, ?) ON CONFLICT (columnname) DO UPDATE SET othercol=EXCLUDED.othercol", ["oldvalue", "newvalue"], ) self.assertTrue(result) @@ -410,7 +410,7 @@ def test_upsert_with_insert( ) self.mock_txn.execute.assert_called_with( - "INSERT INTO tablename (columnname, thirdcol, othercol) VALUES (?, ?, ?) ON CONFLICT (columnname) DO UPDATE SET othercol=EXCLUDED.othercol", + "INSERT INTO tablename (columnname, thirdcol, othercol) VALUES (?, ?, ?) ON CONFLICT (columnname) DO UPDATE SET othercol=EXCLUDED.othercol", ["oldvalue", "insertionval", "newvalue"], ) self.assertTrue(result) From 9f3ba0683d90eaf3027df60f044b3108ba1370ed Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 6 Nov 2023 11:32:13 -0500 Subject: [PATCH 10/15] Clarify variable name. --- tests/storage/test_base.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/storage/test_base.py b/tests/storage/test_base.py index 738e83aa8eb3..9caf3960442c 100644 --- a/tests/storage/test_base.py +++ b/tests/storage/test_base.py @@ -31,7 +31,8 @@ class SQLBaseStoreTestCase(unittest.TestCase): """Test the "simple" SQL generating methods in SQLBaseStore.""" def setUp(self) -> None: - self.db_pool = Mock(spec=["runInteraction"]) + # This is the Twisted connection pool. + conn_pool = Mock(spec=["runInteraction", "runWithConnection"]) self.mock_txn = Mock() self.mock_conn = Mock(spec_set=["cursor", "rollback", "commit"]) self.mock_conn.cursor.return_value = self.mock_txn @@ -41,12 +42,12 @@ def setUp(self) -> None: def runInteraction(func, *args, **kwargs) -> defer.Deferred: # type: ignore[no-untyped-def] return defer.succeed(func(self.mock_txn, *args, **kwargs)) - self.db_pool.runInteraction = runInteraction + conn_pool.runInteraction = runInteraction def runWithConnection(func, *args, **kwargs): # type: ignore[no-untyped-def] return defer.succeed(func(self.mock_conn, *args, **kwargs)) - self.db_pool.runWithConnection = runWithConnection + conn_pool.runWithConnection = runWithConnection config = default_config(name="test", parse=True) hs = TestHomeServer("test", config=config) @@ -59,7 +60,7 @@ def runWithConnection(func, *args, **kwargs): # type: ignore[no-untyped-def] fake_engine.module.DatabaseError = engine.module.DatabaseError db = DatabasePool(Mock(), Mock(config=sqlite_config), fake_engine) - db._db_pool = self.db_pool + db._db_pool = conn_pool self.datastore = SQLBaseStore(db, None, hs) # type: ignore[arg-type] From 98b28b4ca07822d4e1934adc63efb6251db2dc9b Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 6 Nov 2023 12:13:46 -0500 Subject: [PATCH 11/15] Add tests for emulated upserts. --- tests/storage/test_base.py | 175 ++++++++++++++++++++++++++++++++++++- 1 file changed, 174 insertions(+), 1 deletion(-) diff --git a/tests/storage/test_base.py b/tests/storage/test_base.py index 9caf3960442c..4f742b6d2f2d 100644 --- a/tests/storage/test_base.py +++ b/tests/storage/test_base.py @@ -58,6 +58,7 @@ def runWithConnection(func, *args, **kwargs): # type: ignore[no-untyped-def] fake_engine.in_transaction.return_value = False fake_engine.module.OperationalError = engine.module.OperationalError fake_engine.module.DatabaseError = engine.module.DatabaseError + fake_engine.module.IntegrityError = engine.module.IntegrityError db = DatabasePool(Mock(), Mock(config=sqlite_config), fake_engine) db._db_pool = conn_pool @@ -396,7 +397,28 @@ def test_upsert(self) -> Generator["defer.Deferred[object]", object, None]: self.assertTrue(result) @defer.inlineCallbacks - def test_upsert_with_insert( + def test_upsert_no_values( + self, + ) -> Generator["defer.Deferred[object]", object, None]: + self.mock_txn.rowcount = 1 + + result = yield defer.ensureDeferred( + self.datastore.db_pool.simple_upsert( + table="tablename", + keyvalues={"columnname": "value"}, + values={}, + insertion_values={"columnname": "value"}, + ) + ) + + self.mock_txn.execute.assert_called_with( + "INSERT INTO tablename (columnname) VALUES (?) ON CONFLICT (columnname) DO NOTHING", + ["value"], + ) + self.assertTrue(result) + + @defer.inlineCallbacks + def test_upsert_with_insertion( self, ) -> Generator["defer.Deferred[object]", object, None]: self.mock_txn.rowcount = 1 @@ -478,3 +500,154 @@ def test_upsert_many_no_values( "INSERT INTO tablename (columnname) VALUES (?) ON CONFLICT (columnname) DO NOTHING", [("oldvalue",)], ) + + @defer.inlineCallbacks + def test_upsert_emulated_no_values_exists( + self, + ) -> Generator["defer.Deferred[object]", object, None]: + self.datastore.db_pool._unsafe_to_upsert_tables.add("tablename") + + self.mock_txn.fetchall.return_value = [(1,)] + + result = yield defer.ensureDeferred( + self.datastore.db_pool.simple_upsert( + table="tablename", + keyvalues={"columnname": "value"}, + values={}, + insertion_values={"columnname": "value"}, + ) + ) + + self.mock_txn.execute.assert_called_with( + "SELECT 1 FROM tablename WHERE columnname = ?", ["value"] + ) + self.assertFalse(result) + + @defer.inlineCallbacks + def test_upsert_emulated_no_values_not_exists( + self, + ) -> Generator["defer.Deferred[object]", object, None]: + self.datastore.db_pool._unsafe_to_upsert_tables.add("tablename") + + self.mock_txn.fetchall.return_value = [] + self.mock_txn.rowcount = 1 + + result = yield defer.ensureDeferred( + self.datastore.db_pool.simple_upsert( + table="tablename", + keyvalues={"columnname": "value"}, + values={}, + insertion_values={"columnname": "value"}, + ) + ) + + self.mock_txn.execute.assert_has_calls( + [ + call( + "SELECT 1 FROM tablename WHERE columnname = ?", + ["value"], + ), + call("INSERT INTO tablename (columnname) VALUES (?)", ["value"]), + ], + ) + self.assertTrue(result) + + @defer.inlineCallbacks + def test_upsert_emulated_with_insertion_exists( + self, + ) -> Generator["defer.Deferred[object]", object, None]: + self.datastore.db_pool._unsafe_to_upsert_tables.add("tablename") + + self.mock_txn.rowcount = 1 + + result = yield defer.ensureDeferred( + self.datastore.db_pool.simple_upsert( + table="tablename", + keyvalues={"columnname": "oldvalue"}, + values={"othercol": "newvalue"}, + insertion_values={"thirdcol": "insertionval"}, + ) + ) + + self.mock_txn.execute.assert_called_with( + "UPDATE tablename SET othercol = ? WHERE columnname = ?", + ["newvalue", "oldvalue"], + ) + self.assertTrue(result) + + @defer.inlineCallbacks + def test_upsert_emulated_with_insertion_not_exists( + self, + ) -> Generator["defer.Deferred[object]", object, None]: + self.datastore.db_pool._unsafe_to_upsert_tables.add("tablename") + + self.mock_txn.rowcount = 0 + + result = yield defer.ensureDeferred( + self.datastore.db_pool.simple_upsert( + table="tablename", + keyvalues={"columnname": "oldvalue"}, + values={"othercol": "newvalue"}, + insertion_values={"thirdcol": "insertionval"}, + ) + ) + + self.mock_txn.execute.assert_has_calls( + [ + call( + "UPDATE tablename SET othercol = ? WHERE columnname = ?", + ["newvalue", "oldvalue"], + ), + call( + "INSERT INTO tablename (columnname, othercol, thirdcol) VALUES (?, ?, ?)", + ["oldvalue", "newvalue", "insertionval"], + ), + ] + ) + self.assertTrue(result) + + @defer.inlineCallbacks + def test_upsert_emulated_with_where( + self, + ) -> Generator["defer.Deferred[object]", object, None]: + self.datastore.db_pool._unsafe_to_upsert_tables.add("tablename") + + self.mock_txn.rowcount = 1 + + result = yield defer.ensureDeferred( + self.datastore.db_pool.simple_upsert( + table="tablename", + keyvalues={"columnname": "oldvalue"}, + values={"othercol": "newvalue"}, + where_clause="thirdcol IS NULL", + ) + ) + + self.mock_txn.execute.assert_called_with( + "UPDATE tablename SET othercol = ? WHERE columnname = ? AND thirdcol IS NULL", + ["newvalue", "oldvalue"], + ) + self.assertTrue(result) + + @defer.inlineCallbacks + def test_upsert_emulated_with_where_no_values( + self, + ) -> Generator["defer.Deferred[object]", object, None]: + self.datastore.db_pool._unsafe_to_upsert_tables.add("tablename") + + self.mock_txn.rowcount = 1 + + result = yield defer.ensureDeferred( + self.datastore.db_pool.simple_upsert( + table="tablename", + keyvalues={"columnname": "oldvalue"}, + values={}, + where_clause="thirdcol IS NULL", + ) + ) + + self.mock_txn.execute.assert_called_with( + "SELECT 1 FROM tablename WHERE columnname = ? AND thirdcol IS NULL", + ["oldvalue"], + ) + self.assertFalse(result) From 85928cf895a8af9bb26e7d66490fdea82e376ea3 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 6 Nov 2023 14:31:29 -0500 Subject: [PATCH 12/15] Add postgres tests. --- tests/storage/test_base.py | 127 +++++++++++++++++++++++++++++-------- 1 file changed, 99 insertions(+), 28 deletions(-) diff --git a/tests/storage/test_base.py b/tests/storage/test_base.py index 4f742b6d2f2d..daee83442d2c 100644 --- a/tests/storage/test_base.py +++ b/tests/storage/test_base.py @@ -14,7 +14,7 @@ from collections import OrderedDict from typing import Generator -from unittest.mock import Mock, call +from unittest.mock import Mock, call, patch from twisted.internet import defer @@ -24,7 +24,7 @@ from tests import unittest from tests.server import TestHomeServer -from tests.utils import default_config +from tests.utils import default_config, USE_POSTGRES_FOR_TESTS class SQLBaseStoreTestCase(unittest.TestCase): @@ -34,8 +34,38 @@ def setUp(self) -> None: # This is the Twisted connection pool. conn_pool = Mock(spec=["runInteraction", "runWithConnection"]) self.mock_txn = Mock() - self.mock_conn = Mock(spec_set=["cursor", "rollback", "commit"]) + if USE_POSTGRES_FOR_TESTS: + # To avoid testing psycopg2 itself, patch execute_batch/execute_values + # to assert how it is called. + from psycopg2 import extras + + self.mock_execute_batch = Mock() + self.execute_batch_patcher = patch.object( + extras, "execute_batch", new=self.mock_execute_batch + ) + self.execute_batch_patcher.start() + self.mock_execute_values = Mock() + self.execute_values_patcher = patch.object( + extras, "execute_values", new=self.mock_execute_values + ) + self.execute_values_patcher.start() + + self.mock_conn = Mock( + spec_set=[ + "cursor", + "rollback", + "commit", + "closed", + "reconnect", + "set_session", + "encoding", + ] + ) + self.mock_conn.encoding = "UNICODE" + else: + self.mock_conn = Mock(spec_set=["cursor", "rollback", "commit"]) self.mock_conn.cursor.return_value = self.mock_txn + self.mock_txn.connection = self.mock_conn self.mock_conn.rollback.return_value = None # Our fake runInteraction just runs synchronously inline @@ -52,19 +82,32 @@ def runWithConnection(func, *args, **kwargs): # type: ignore[no-untyped-def] config = default_config(name="test", parse=True) hs = TestHomeServer("test", config=config) - sqlite_config = {"name": "sqlite3"} - engine = create_engine(sqlite_config) + if USE_POSTGRES_FOR_TESTS: + config = {"name": "psycopg2", "args": {}} + else: + config = {"name": "sqlite3"} + engine = create_engine(config) + fake_engine = Mock(wraps=engine) fake_engine.in_transaction.return_value = False fake_engine.module.OperationalError = engine.module.OperationalError fake_engine.module.DatabaseError = engine.module.DatabaseError fake_engine.module.IntegrityError = engine.module.IntegrityError + # Don't convert param style to make assertions easier. + fake_engine.convert_param_style = lambda sql: sql + # To fix isinstance(...) checks. + fake_engine.__class__ = engine.__class__ - db = DatabasePool(Mock(), Mock(config=sqlite_config), fake_engine) + db = DatabasePool(Mock(), Mock(config=config), fake_engine) db._db_pool = conn_pool self.datastore = SQLBaseStore(db, None, hs) # type: ignore[arg-type] + def tearDown(self) -> None: + if USE_POSTGRES_FOR_TESTS: + self.execute_batch_patcher.stop() + self.execute_values_patcher.stop() + @defer.inlineCallbacks def test_insert_1col(self) -> Generator["defer.Deferred[object]", object, None]: self.mock_txn.rowcount = 1 @@ -115,12 +158,19 @@ def test_insert_many(self) -> Generator["defer.Deferred[object]", object, None]: ) ) - # TODO Test postgres variant. - - self.mock_txn.executemany.assert_called_with( - "INSERT INTO tablename (col1, col2) VALUES(?, ?)", - [("val1", "val2"), ("val3", "val4")], - ) + if USE_POSTGRES_FOR_TESTS: + self.mock_execute_values.assert_called_with( + self.mock_txn, + "INSERT INTO tablename (col1, col2) VALUES ?", + [("val1", "val2"), ("val3", "val4")], + template=None, + fetch=False, + ) + else: + self.mock_txn.executemany.assert_called_with( + "INSERT INTO tablename (col1, col2) VALUES(?, ?)", + [("val1", "val2"), ("val3", "val4")], + ) @defer.inlineCallbacks def test_select_one_1col(self) -> Generator["defer.Deferred[object]", object, None]: @@ -288,10 +338,17 @@ def test_update_many(self) -> Generator["defer.Deferred[object]", object, None]: ) ) - self.mock_txn.executemany.assert_called_with( - "UPDATE tablename SET col3 = ? WHERE col1 = ? AND col2 = ?", - [("val3", "val1", "val2"), ("val3", "val1", "val2")], - ) + if USE_POSTGRES_FOR_TESTS: + self.mock_execute_batch.assert_called_with( + self.mock_txn, + "UPDATE tablename SET col3 = ? WHERE col1 = ? AND col2 = ?", + [("val3", "val1", "val2"), ("val3", "val1", "val2")], + ) + else: + self.mock_txn.executemany.assert_called_with( + "UPDATE tablename SET col3 = ? WHERE col1 = ? AND col2 = ?", + [("val3", "val1", "val2"), ("val3", "val1", "val2")], + ) # key_values and value_values must be the same length. with self.assertRaises(ValueError): @@ -472,12 +529,19 @@ def test_upsert_many(self) -> Generator["defer.Deferred[object]", object, None]: ) ) - # TODO Test postgres variant. - - self.mock_txn.executemany.assert_called_with( - "INSERT INTO tablename (keycol1, keycol2, valuecol3) VALUES (?, ?, ?) ON CONFLICT (keycol1, keycol2) DO UPDATE SET valuecol3=EXCLUDED.valuecol3", - [("keyval1", "keyval2", "val5"), ("keyval3", "keyval4", "val6")], - ) + if USE_POSTGRES_FOR_TESTS: + self.mock_execute_values.assert_called_with( + self.mock_txn, + "INSERT INTO tablename (keycol1, keycol2, valuecol3) VALUES ? ON CONFLICT (keycol1, keycol2) DO UPDATE SET valuecol3=EXCLUDED.valuecol3", + [("keyval1", "keyval2", "val5"), ("keyval3", "keyval4", "val6")], + template=None, + fetch=False, + ) + else: + self.mock_txn.executemany.assert_called_with( + "INSERT INTO tablename (keycol1, keycol2, valuecol3) VALUES (?, ?, ?) ON CONFLICT (keycol1, keycol2) DO UPDATE SET valuecol3=EXCLUDED.valuecol3", + [("keyval1", "keyval2", "val5"), ("keyval3", "keyval4", "val6")], + ) @defer.inlineCallbacks def test_upsert_many_no_values( @@ -494,12 +558,19 @@ def test_upsert_many_no_values( ) ) - # TODO Test postgres variant. - - self.mock_txn.executemany.assert_called_with( - "INSERT INTO tablename (columnname) VALUES (?) ON CONFLICT (columnname) DO NOTHING", - [("oldvalue",)], - ) + if USE_POSTGRES_FOR_TESTS: + self.mock_execute_values.assert_called_with( + self.mock_txn, + "INSERT INTO tablename (columnname) VALUES ? ON CONFLICT (columnname) DO NOTHING", + [("oldvalue",)], + template=None, + fetch=False, + ) + else: + self.mock_txn.executemany.assert_called_with( + "INSERT INTO tablename (columnname) VALUES (?) ON CONFLICT (columnname) DO NOTHING", + [("oldvalue",)], + ) @defer.inlineCallbacks def test_upsert_emulated_no_values_exists( From 1fa8fcc9fb3e4133acfb0df6b2c3c2f00d76a807 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 6 Nov 2023 14:50:32 -0500 Subject: [PATCH 13/15] Lint --- tests/storage/test_base.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/storage/test_base.py b/tests/storage/test_base.py index daee83442d2c..3238c30d7b69 100644 --- a/tests/storage/test_base.py +++ b/tests/storage/test_base.py @@ -24,7 +24,7 @@ from tests import unittest from tests.server import TestHomeServer -from tests.utils import default_config, USE_POSTGRES_FOR_TESTS +from tests.utils import USE_POSTGRES_FOR_TESTS, default_config class SQLBaseStoreTestCase(unittest.TestCase): @@ -83,10 +83,10 @@ def runWithConnection(func, *args, **kwargs): # type: ignore[no-untyped-def] hs = TestHomeServer("test", config=config) if USE_POSTGRES_FOR_TESTS: - config = {"name": "psycopg2", "args": {}} + db_config = {"name": "psycopg2", "args": {}} else: - config = {"name": "sqlite3"} - engine = create_engine(config) + db_config = {"name": "sqlite3"} + engine = create_engine(db_config) fake_engine = Mock(wraps=engine) fake_engine.in_transaction.return_value = False @@ -96,9 +96,9 @@ def runWithConnection(func, *args, **kwargs): # type: ignore[no-untyped-def] # Don't convert param style to make assertions easier. fake_engine.convert_param_style = lambda sql: sql # To fix isinstance(...) checks. - fake_engine.__class__ = engine.__class__ + fake_engine.__class__ = engine.__class__ # type: ignore[assignment] - db = DatabasePool(Mock(), Mock(config=config), fake_engine) + db = DatabasePool(Mock(), Mock(config=db_config), fake_engine) db._db_pool = conn_pool self.datastore = SQLBaseStore(db, None, hs) # type: ignore[arg-type] From 5172d8b115bf052f98844102482f1cf15aa4afb9 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 7 Nov 2023 08:44:40 -0500 Subject: [PATCH 14/15] Fix-up asserts. --- tests/storage/test_base.py | 115 +++++++++++++++++++++++++------------ 1 file changed, 78 insertions(+), 37 deletions(-) diff --git a/tests/storage/test_base.py b/tests/storage/test_base.py index 3238c30d7b69..1de22201af23 100644 --- a/tests/storage/test_base.py +++ b/tests/storage/test_base.py @@ -118,7 +118,7 @@ def test_insert_1col(self) -> Generator["defer.Deferred[object]", object, None]: ) ) - self.mock_txn.execute.assert_called_with( + self.mock_txn.execute.assert_called_once_with( "INSERT INTO tablename (columname) VALUES(?)", ("Value",) ) @@ -134,7 +134,7 @@ def test_insert_3cols(self) -> Generator["defer.Deferred[object]", object, None] ) ) - self.mock_txn.execute.assert_called_with( + self.mock_txn.execute.assert_called_once_with( "INSERT INTO tablename (colA, colB, colC) VALUES(?, ?, ?)", (1, 2, 3) ) @@ -159,7 +159,7 @@ def test_insert_many(self) -> Generator["defer.Deferred[object]", object, None]: ) if USE_POSTGRES_FOR_TESTS: - self.mock_execute_values.assert_called_with( + self.mock_execute_values.assert_called_once_with( self.mock_txn, "INSERT INTO tablename (col1, col2) VALUES ?", [("val1", "val2"), ("val3", "val4")], @@ -167,7 +167,7 @@ def test_insert_many(self) -> Generator["defer.Deferred[object]", object, None]: fetch=False, ) else: - self.mock_txn.executemany.assert_called_with( + self.mock_txn.executemany.assert_called_once_with( "INSERT INTO tablename (col1, col2) VALUES(?, ?)", [("val1", "val2"), ("val3", "val4")], ) @@ -184,7 +184,7 @@ def test_select_one_1col(self) -> Generator["defer.Deferred[object]", object, No ) self.assertEqual("Value", value) - self.mock_txn.execute.assert_called_with( + self.mock_txn.execute.assert_called_once_with( "SELECT retcol FROM tablename WHERE keycol = ?", ["TheKey"] ) @@ -202,7 +202,7 @@ def test_select_one_3col(self) -> Generator["defer.Deferred[object]", object, No ) self.assertEqual({"colA": 1, "colB": 2, "colC": 3}, ret) - self.mock_txn.execute.assert_called_with( + self.mock_txn.execute.assert_called_once_with( "SELECT colA, colB, colC FROM tablename WHERE keycol = ?", ["TheKey"] ) @@ -237,7 +237,7 @@ def test_select_list(self) -> Generator["defer.Deferred[object]", object, None]: ) self.assertEqual([(1,), (2,), (3,)], ret) - self.mock_txn.execute.assert_called_with( + self.mock_txn.execute.assert_called_once_with( "SELECT colA FROM tablename WHERE keycol = ?", ["A set"] ) @@ -301,7 +301,7 @@ def test_update_one_1col(self) -> Generator["defer.Deferred[object]", object, No ) ) - self.mock_txn.execute.assert_called_with( + self.mock_txn.execute.assert_called_once_with( "UPDATE tablename SET columnname = ? WHERE keycol = ?", ["New Value", "TheKey"], ) @@ -320,7 +320,7 @@ def test_update_one_4cols( ) ) - self.mock_txn.execute.assert_called_with( + self.mock_txn.execute.assert_called_once_with( "UPDATE tablename SET colC = ?, colD = ? WHERE" " colA = ? AND colB = ?", [3, 4, 1, 2], ) @@ -339,13 +339,13 @@ def test_update_many(self) -> Generator["defer.Deferred[object]", object, None]: ) if USE_POSTGRES_FOR_TESTS: - self.mock_execute_batch.assert_called_with( + self.mock_execute_batch.assert_called_once_with( self.mock_txn, "UPDATE tablename SET col3 = ? WHERE col1 = ? AND col2 = ?", [("val3", "val1", "val2"), ("val3", "val1", "val2")], ) else: - self.mock_txn.executemany.assert_called_with( + self.mock_txn.executemany.assert_called_once_with( "UPDATE tablename SET col3 = ? WHERE col1 = ? AND col2 = ?", [("val3", "val1", "val2"), ("val3", "val1", "val2")], ) @@ -373,7 +373,7 @@ def test_delete_one(self) -> Generator["defer.Deferred[object]", object, None]: ) ) - self.mock_txn.execute.assert_called_with( + self.mock_txn.execute.assert_called_once_with( "DELETE FROM tablename WHERE keycol = ?", ["Go away"] ) @@ -391,7 +391,7 @@ def test_delete_many(self) -> Generator["defer.Deferred[object]", object, None]: ) ) - self.mock_txn.execute.assert_called_with( + self.mock_txn.execute.assert_called_once_with( "DELETE FROM tablename WHERE col1 = ANY(?) AND col2 = ?", [["val1", "val2"], "val3"], ) @@ -430,7 +430,7 @@ def test_delete_many_no_keyvalues( ) ) - self.mock_txn.execute.assert_called_with( + self.mock_txn.execute.assert_called_once_with( "DELETE FROM tablename WHERE col1 = ANY(?)", [["val1", "val2"]] ) self.assertEqual(result, 2) @@ -447,7 +447,7 @@ def test_upsert(self) -> Generator["defer.Deferred[object]", object, None]: ) ) - self.mock_txn.execute.assert_called_with( + self.mock_txn.execute.assert_called_once_with( "INSERT INTO tablename (columnname, othercol) VALUES (?, ?) ON CONFLICT (columnname) DO UPDATE SET othercol=EXCLUDED.othercol", ["oldvalue", "newvalue"], ) @@ -468,7 +468,7 @@ def test_upsert_no_values( ) ) - self.mock_txn.execute.assert_called_with( + self.mock_txn.execute.assert_called_once_with( "INSERT INTO tablename (columnname) VALUES (?) ON CONFLICT (columnname) DO NOTHING", ["value"], ) @@ -489,7 +489,7 @@ def test_upsert_with_insertion( ) ) - self.mock_txn.execute.assert_called_with( + self.mock_txn.execute.assert_called_once_with( "INSERT INTO tablename (columnname, thirdcol, othercol) VALUES (?, ?, ?) ON CONFLICT (columnname) DO UPDATE SET othercol=EXCLUDED.othercol", ["oldvalue", "insertionval", "newvalue"], ) @@ -510,7 +510,7 @@ def test_upsert_with_where( ) ) - self.mock_txn.execute.assert_called_with( + self.mock_txn.execute.assert_called_once_with( "INSERT INTO tablename (columnname, othercol) VALUES (?, ?) ON CONFLICT (columnname) WHERE thirdcol IS NULL DO UPDATE SET othercol=EXCLUDED.othercol", ["oldvalue", "newvalue"], ) @@ -530,7 +530,7 @@ def test_upsert_many(self) -> Generator["defer.Deferred[object]", object, None]: ) if USE_POSTGRES_FOR_TESTS: - self.mock_execute_values.assert_called_with( + self.mock_execute_values.assert_called_once_with( self.mock_txn, "INSERT INTO tablename (keycol1, keycol2, valuecol3) VALUES ? ON CONFLICT (keycol1, keycol2) DO UPDATE SET valuecol3=EXCLUDED.valuecol3", [("keyval1", "keyval2", "val5"), ("keyval3", "keyval4", "val6")], @@ -538,7 +538,7 @@ def test_upsert_many(self) -> Generator["defer.Deferred[object]", object, None]: fetch=False, ) else: - self.mock_txn.executemany.assert_called_with( + self.mock_txn.executemany.assert_called_once_with( "INSERT INTO tablename (keycol1, keycol2, valuecol3) VALUES (?, ?, ?) ON CONFLICT (keycol1, keycol2) DO UPDATE SET valuecol3=EXCLUDED.valuecol3", [("keyval1", "keyval2", "val5"), ("keyval3", "keyval4", "val6")], ) @@ -559,7 +559,7 @@ def test_upsert_many_no_values( ) if USE_POSTGRES_FOR_TESTS: - self.mock_execute_values.assert_called_with( + self.mock_execute_values.assert_called_once_with( self.mock_txn, "INSERT INTO tablename (columnname) VALUES ? ON CONFLICT (columnname) DO NOTHING", [("oldvalue",)], @@ -567,7 +567,7 @@ def test_upsert_many_no_values( fetch=False, ) else: - self.mock_txn.executemany.assert_called_with( + self.mock_txn.executemany.assert_called_once_with( "INSERT INTO tablename (columnname) VALUES (?) ON CONFLICT (columnname) DO NOTHING", [("oldvalue",)], ) @@ -589,9 +589,17 @@ def test_upsert_emulated_no_values_exists( ) ) - self.mock_txn.execute.assert_called_with( - "SELECT 1 FROM tablename WHERE columnname = ?", ["value"] - ) + if USE_POSTGRES_FOR_TESTS: + self.mock_txn.execute.assert_has_calls( + [ + call("LOCK TABLE tablename in EXCLUSIVE MODE", ()), + call("SELECT 1 FROM tablename WHERE columnname = ?", ["value"]), + ] + ) + else: + self.mock_txn.execute.assert_called_once_with( + "SELECT 1 FROM tablename WHERE columnname = ?", ["value"] + ) self.assertFalse(result) @defer.inlineCallbacks @@ -640,10 +648,21 @@ def test_upsert_emulated_with_insertion_exists( ) ) - self.mock_txn.execute.assert_called_with( - "UPDATE tablename SET othercol = ? WHERE columnname = ?", - ["newvalue", "oldvalue"], - ) + if USE_POSTGRES_FOR_TESTS: + self.mock_txn.execute.assert_has_calls( + [ + call("LOCK TABLE tablename in EXCLUSIVE MODE", ()), + call( + "UPDATE tablename SET othercol = ? WHERE columnname = ?", + ["newvalue", "oldvalue"], + ), + ] + ) + else: + self.mock_txn.execute.assert_called_once_with( + "UPDATE tablename SET othercol = ? WHERE columnname = ?", + ["newvalue", "oldvalue"], + ) self.assertTrue(result) @defer.inlineCallbacks @@ -694,10 +713,21 @@ def test_upsert_emulated_with_where( ) ) - self.mock_txn.execute.assert_called_with( - "UPDATE tablename SET othercol = ? WHERE columnname = ? AND thirdcol IS NULL", - ["newvalue", "oldvalue"], - ) + if USE_POSTGRES_FOR_TESTS: + self.mock_txn.execute.assert_has_calls( + [ + call("LOCK TABLE tablename in EXCLUSIVE MODE", ()), + call( + "UPDATE tablename SET othercol = ? WHERE columnname = ? AND thirdcol IS NULL", + ["newvalue", "oldvalue"], + ), + ] + ) + else: + self.mock_txn.execute.assert_called_once_with( + "UPDATE tablename SET othercol = ? WHERE columnname = ? AND thirdcol IS NULL", + ["newvalue", "oldvalue"], + ) self.assertTrue(result) @defer.inlineCallbacks @@ -717,8 +747,19 @@ def test_upsert_emulated_with_where_no_values( ) ) - self.mock_txn.execute.assert_called_with( - "SELECT 1 FROM tablename WHERE columnname = ? AND thirdcol IS NULL", - ["oldvalue"], - ) + if USE_POSTGRES_FOR_TESTS: + self.mock_txn.execute.assert_has_calls( + [ + call("LOCK TABLE tablename in EXCLUSIVE MODE", ()), + call( + "SELECT 1 FROM tablename WHERE columnname = ? AND thirdcol IS NULL", + ["oldvalue"], + ), + ] + ) + else: + self.mock_txn.execute.assert_called_once_with( + "SELECT 1 FROM tablename WHERE columnname = ? AND thirdcol IS NULL", + ["oldvalue"], + ) self.assertFalse(result) From e4957d64e0d124a2c1c04a362955283daeffe8ef Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 7 Nov 2023 08:53:48 -0500 Subject: [PATCH 15/15] Add tests for no values. --- tests/storage/test_base.py | 56 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/tests/storage/test_base.py b/tests/storage/test_base.py index 1de22201af23..b4c490b568f2 100644 --- a/tests/storage/test_base.py +++ b/tests/storage/test_base.py @@ -172,6 +172,35 @@ def test_insert_many(self) -> Generator["defer.Deferred[object]", object, None]: [("val1", "val2"), ("val3", "val4")], ) + @defer.inlineCallbacks + def test_insert_many_no_iterable( + self, + ) -> Generator["defer.Deferred[object]", object, None]: + yield defer.ensureDeferred( + self.datastore.db_pool.simple_insert_many( + table="tablename", + keys=( + "col1", + "col2", + ), + values=[], + desc="", + ) + ) + + if USE_POSTGRES_FOR_TESTS: + self.mock_execute_values.assert_called_once_with( + self.mock_txn, + "INSERT INTO tablename (col1, col2) VALUES ?", + [], + template=None, + fetch=False, + ) + else: + self.mock_txn.executemany.assert_called_once_with( + "INSERT INTO tablename (col1, col2) VALUES(?, ?)", [] + ) + @defer.inlineCallbacks def test_select_one_1col(self) -> Generator["defer.Deferred[object]", object, None]: self.mock_txn.rowcount = 1 @@ -363,6 +392,33 @@ def test_update_many(self) -> Generator["defer.Deferred[object]", object, None]: ) ) + @defer.inlineCallbacks + def test_update_many_no_values( + self, + ) -> Generator["defer.Deferred[object]", object, None]: + yield defer.ensureDeferred( + self.datastore.db_pool.simple_update_many( + table="tablename", + key_names=("col1", "col2"), + key_values=[], + value_names=("col3",), + value_values=[], + desc="", + ) + ) + + if USE_POSTGRES_FOR_TESTS: + self.mock_execute_batch.assert_called_once_with( + self.mock_txn, + "UPDATE tablename SET col3 = ? WHERE col1 = ? AND col2 = ?", + [], + ) + else: + self.mock_txn.executemany.assert_called_once_with( + "UPDATE tablename SET col3 = ? WHERE col1 = ? AND col2 = ?", + [], + ) + @defer.inlineCallbacks def test_delete_one(self) -> Generator["defer.Deferred[object]", object, None]: self.mock_txn.rowcount = 1