Merge branch 'release-v2.4'

matrix-org · Sep 17, 2021 · 5f90f62 · 5f90f62
2 parents 7857b06 + 8270d08
commit 5f90f62
Show file tree

Hide file tree

Showing 6 changed files with 75 additions and 32 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,28 @@
+Sydent 2.4.3 (2021-09-14)
+=========================
+
+Bugfixes
+--------
+
+- Fix a bug introduced in v2.4.0 that caused association unbindings to fail with an Internal Server Error. ([\#397](https://github.com/matrix-org/sydent/issues/397))
+
+
+Sydent 2.4.2 (2021-09-13)
+=========================
+
+Bugfixes
+--------
+
+- Fix a bug causing the email address migration script to take a lot of time to run due to inefficient database queries. ([\#396](https://github.com/matrix-org/sydent/issues/396))
+
+
+Internal Changes
+----------------
+
+- Move dev tools from `install_requires` to `extras_require`. ([\#389](https://github.com/matrix-org/sydent/issues/389))
+- Run background jobs in `run` rather than in Sydent's constructor. ([\#394](https://github.com/matrix-org/sydent/issues/394))
+
+
 Sydent 2.4.1 (2021-09-10)
 =========================
 

diff --git a/changelog.d/389.misc b/changelog.d/389.misc
diff --git a/changelog.d/394.misc b/changelog.d/394.misc
diff --git a/scripts/casefold_db.py b/scripts/casefold_db.py
@@ -15,6 +15,7 @@
 
 import argparse
 import json
+import logging
 import os
 import sqlite3
 import sys
@@ -31,6 +32,8 @@
 from sydent.util.hash import sha256_and_url_safe_base64
 from tests.utils import ResolvingMemoryReactorClock
 
+logger = logging.getLogger("casefold_db")
+
 # Maximum number of attempts to send an email.
 MAX_ATTEMPTS_FOR_EMAIL = 5
 
@@ -88,7 +91,7 @@ def sendEmailWithBackoff(
     """Send an email with exponential backoff - that way we don't stop sending halfway
     through if the SMTP server rejects our email (e.g. because of rate limiting).
 
-    Setting test to True disables the logging.
+    Setting test to True disables the backoff.
 
     Raises a CantSendEmailException if no email could be sent after MAX_ATTEMPTS_FOR_EMAIL
     attempts.
@@ -111,16 +114,14 @@ def sendEmailWithBackoff(
                 {"mxid": mxid},
                 log_send_errors=False,
             )
-            if not test:
-                print("Sent email to %s" % address)
+            logger.info("Sent email to %s" % address)
 
             return
         except EmailSendException:
-            if not test:
-                print(
-                    "Failed to send email to %s (attempt %d/%d)"
-                    % (address, i + 1, MAX_ATTEMPTS_FOR_EMAIL)
-                )
+            logger.info(
+                "Failed to send email to %s (attempt %d/%d)"
+                % (address, i + 1, MAX_ATTEMPTS_FOR_EMAIL)
+            )
 
             time.sleep(backoff)
             backoff *= 2
@@ -142,10 +143,13 @@ def update_local_associations(
     Setting dry_run to True means that the script is being run in dry-run mode
     by the user, i.e. it will run but will not send any email nor update the database.
     Setting test to True means that the function is being called as part of an automated
-    test, and therefore we should neither backoff when sending emails or log.
+    test, and therefore we shouldn't backoff when sending emails.
 
     :return: None
     """
+
+    logger.info("Processing rows in local_threepid_associations")
+
     res = db.execute(
         "SELECT address, mxid FROM local_threepid_associations WHERE medium = 'email'"
         "ORDER BY ts DESC"
@@ -154,6 +158,8 @@ def update_local_associations(
     # a dict that associates an email address with correspoinding mxids and lookup hashes
     associations: Dict[str, List[Tuple[str, str, str]]] = {}
 
+    logger.info("Computing new hashes and signatures for local_threepid_associations")
+
     # iterate through selected associations, casefold email, rehash it, and add to
     # associations dict
     for address, mxid in res.fetchall():
@@ -197,8 +203,7 @@ def update_local_associations(
                     )
                 )
 
-    if not test:
-        print(f"{len(deltas)} rows to update in local_threepid_associations")
+    logger.info(f"{len(deltas)} rows to update in local_threepid_associations")
 
     # Apply the deltas
     for casefolded_address, delta in deltas.items():
@@ -208,7 +213,7 @@ def update_local_associations(
                 log_msg += (
                     f" and deleting {len(delta.to_delete)} rows associated with it"
                 )
-            print(log_msg)
+            logger.info(log_msg)
 
         try:
             # Delete each association, and send an email mentioning the affected MXID.
@@ -230,7 +235,7 @@ def update_local_associations(
                     if not dry_run:
                         cur = db.cursor()
                         cur.execute(
-                            "DELETE FROM local_threepid_associations WHERE address = ?",
+                            "DELETE FROM local_threepid_associations WHERE medium = 'email' AND address = ?",
                             (to_delete.address,),
                         )
                         db.commit()
@@ -239,7 +244,7 @@ def update_local_associations(
             if not dry_run:
                 cur = db.cursor()
                 cur.execute(
-                    "UPDATE local_threepid_associations SET address = ?, lookup_hash = ? WHERE address = ? AND mxid = ?",
+                    "UPDATE local_threepid_associations SET address = ?, lookup_hash = ? WHERE medium = 'email' AND address = ? AND mxid = ?",
                     (
                         casefolded_address,
                         delta.to_update.lookup_hash,
@@ -263,26 +268,24 @@ def update_global_associations(
     sydent: Sydent,
     db: sqlite3.Connection,
     dry_run: bool,
-    test: bool = False,
 ) -> None:
     """Update the DB table global_threepid_associations so that all stored
     emails are casefolded, the signed association is re-signed and any duplicate
     mxid's associated with the given email are deleted.
 
     Setting dry_run to True means that the script is being run in dry-run mode
     by the user, i.e. it will run but will not send any email nor update the database.
-    Setting test to True means that the function is being called as part of an automated
-    test, and therefore we should suppress logs.
 
     :return: None
     """
 
+    logger.info("Processing rows in global_threepid_associations")
+
     # get every row where the local server is origin server and medium is email
     origin_server = sydent.config.general.server_name
     medium = "email"
 
-    cur = db.cursor()
-    res = cur.execute(
+    res = db.execute(
         "SELECT address, mxid, sgAssoc FROM global_threepid_associations WHERE medium = ?"
         "AND originServer = ? ORDER BY ts DESC",
         (medium, origin_server),
@@ -292,6 +295,8 @@ def update_global_associations(
     # signed association
     associations: Dict[str, List[Tuple[str, str, str, str]]] = {}
 
+    logger.info("Computing new hashes and signatures for global_threepid_associations")
+
     # iterate through selected associations, casefold email, rehash it, re-sign the
     # associations and add to associations dict
     for address, mxid, sg_assoc in res.fetchall():
@@ -344,23 +349,32 @@ def update_global_associations(
             for address, mxid, _, _ in assoc_tuples[1:]:
                 to_delete.append((address,))
 
-    if not test:
-        print(
-            f"{len(to_delete)} rows to delete, {len(db_update_args)} rows to update in global_threepid_associations"
-        )
-
+    logger.info(
+        f"{len(to_delete)} rows to delete, {len(db_update_args)} rows to update in global_threepid_associations"
+    )
     if not dry_run:
+        cur = db.cursor()
+
         if len(to_delete) > 0:
             cur.executemany(
-                "DELETE FROM global_threepid_associations WHERE address = ?", to_delete
+                "DELETE FROM global_threepid_associations WHERE medium = 'email' AND address = ?",
+                to_delete,
+            )
+
+            logger.info(
+                f"{len(to_delete)} rows deleted from global_threepid_associations"
             )
 
         if len(db_update_args) > 0:
             cur.executemany(
-                "UPDATE global_threepid_associations SET address = ?, lookup_hash = ?, sgAssoc = ? WHERE address = ? AND mxid = ?",
+                "UPDATE global_threepid_associations SET address = ?, lookup_hash = ?, sgAssoc = ? WHERE medium = 'email' AND address = ? AND mxid = ?",
                 db_update_args,
             )
 
+            logger.info(
+                f"{len(db_update_args)} rows updated in global_threepid_associations"
+            )
+
         db.commit()
 
 
@@ -379,9 +393,17 @@ def update_global_associations(
 
     args = parser.parse_args()
 
+    # Set up logging.
+    log_format = "%(asctime)s - %(name)s - %(lineno)d - %(levelname)s" " - %(message)s"
+    formatter = logging.Formatter(log_format)
+    handler = logging.StreamHandler()
+    handler.setFormatter(formatter)
+    logger.setLevel(logging.INFO)
+    logger.addHandler(handler)
+
     # if the path the user gives us doesn't work, find it for them
     if not os.path.exists(args.config_path):
-        print(f"The config file '{args.config_path}' does not exist.")
+        logger.error(f"The config file '{args.config_path}' does not exist.")
         sys.exit(1)
 
     sydent_config = SydentConfig()

diff --git a/sydent/__init__.py b/sydent/__init__.py
@@ -1 +1 @@
-__version__ = "2.4.1"
+__version__ = "2.4.3"
diff --git a/tests/test_casefold_migration.py b/tests/test_casefold_migration.py
@@ -247,7 +247,6 @@ def test_global_db_migration(self):
             self.sydent,
             self.sydent.db,
             dry_run=False,
-            test=True,
         )
 
         cur = self.sydent.db.cursor()
@@ -299,7 +298,6 @@ def test_dry_run_does_nothing(self):
                 self.sydent,
                 self.sydent.db,
                 dry_run=True,
-                test=True,
             )
 
         # test no emails were sent