sql: search existing LoggedError before inserting (#55807)

tests/conftest.py

@@ -1,5 +1,6 @@
 import configparser
 import os
+from unittest import mock
 
 import pytest
 
@@ -79,3 +80,38 @@ def nocache(settings):
             'BACKEND': 'django.core.cache.backends.dummy.DummyCache',
         }
     }
+
+
+@pytest.fixture
+def sql_queries(monkeypatch):
+    import psycopg2
+
+    import wcs.sql
+
+    queries = []
+    wcs.sql.cleanup_connection()
+    old_connect = psycopg2.connect
+
+    def connect(*args, **kwargs):
+        conn = old_connect(*args, **kwargs)
+        mocked_conn = mock.Mock(wraps=conn)
+        old_cursor = conn.cursor
+
+        def cursor(*args, **kwargs):
+            cur = old_cursor(*args, **kwargs)
+            mocked_cur = mock.Mock(wraps=cur)
+            old_execute = cur.execute
+
+            def execute(*args, **kwargs):
+                queries.append(args[0])
+                return old_execute(*args, **kwargs)
+
+            mocked_cur.execute = execute
+            return mocked_cur
+
+        mocked_conn.cursor = cursor
+        return mocked_conn
+
+    monkeypatch.setattr(psycopg2, 'connect', connect)
+    yield queries
+    wcs.sql.cleanup_connection()

tests/test_sql.py

@@ -2061,3 +2061,17 @@ def test_migration_40_user_is_active():
     assert sql.SqlUser.count() == 2
     assert sql.SqlUser.get(id=user.id).is_active is False
     assert sql.SqlUser.get(id=user2.id).is_active is True
+
+
+def test_logged_error_store_without_integrity_error(sql_queries):
+    sql.LoggedError.record('there was an error')
+
+    assert len(sql_queries) == 2
+    assert 'SELECT' in sql_queries[0]
+    assert 'INSERT' in sql_queries[1]
+    sql_queries.clear()
+
+    sql.LoggedError.record('there was an error')
+    assert len(sql_queries) == 2
+    assert 'SELECT' in sql_queries[0]
+    assert 'UPDATE' in sql_queries[1]

wcs/sql.py

@@ -3041,27 +3041,28 @@ class LoggedError(SqlMixin, wcs.logged_errors.LoggedError):
 
         conn, cur = get_connection_and_cursor()
         if not self.id:
-            column_names = [x for x in sql_dict.keys() if x != 'id']
-            sql_statement = '''INSERT INTO %s (%s)
-                               VALUES (%s)
-                               RETURNING id''' % (
-                self._table_name,
-                ', '.join(column_names),
-                ', '.join(['%%(%s)s' % x for x in column_names]),
-            )
-            try:
-                cur.execute(sql_statement, sql_dict)
-            except psycopg2.IntegrityError:
-                # tech_id already used ?
-                conn.rollback()
-                existing_errors = list(self.get_with_indexed_value('tech_id', self.tech_id))
-                if not existing_errors:
-                    raise
+            existing_errors = list(self.get_with_indexed_value('tech_id', self.tech_id))
+            if not existing_errors:
+                column_names = [x for x in sql_dict.keys() if x != 'id']
+                sql_statement = '''INSERT INTO %s (%s)
+                                   VALUES (%s)
+                                   RETURNING id''' % (
+                    self._table_name,
+                    ', '.join(column_names),
+                    ', '.join(['%%(%s)s' % x for x in column_names]),
+                )
+                try:
+                    cur.execute(sql_statement, sql_dict)
+                    self.id = cur.fetchone()[0]
+                except psycopg2.IntegrityError:
+                    # tech_id already used ?
+                    conn.rollback()
+                    existing_errors = list(self.get_with_indexed_value('tech_id', self.tech_id))
+            if existing_errors:
                 error = existing_errors[0]
                 error.occurences_count += 1
                 error.latest_occurence_timestamp = self.latest_occurence_timestamp
                 return error.store()
-            self.id = cur.fetchone()[0]
         else:
             column_names = sql_dict.keys()
             sql_statement = '''UPDATE %s SET %s WHERE id = %%(id)s RETURNING id''' % (
@@ -3069,9 +3070,7 @@ class LoggedError(SqlMixin, wcs.logged_errors.LoggedError):
                 ', '.join(['%s = %%(%s)s' % (x, x) for x in column_names]),
             )
             cur.execute(sql_statement, sql_dict)
-            if cur.fetchone() is None:
-                raise AssertionError()
-
+            assert cur.fetchone() is not None, 'LoggedError id not found'
         conn.commit()
         cur.close()