diff --git a/CHANGELOG.md b/CHANGELOG.md index cf46f94a..092d31d0 100644 --- 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 deleted file mode 100644 index 83ee700b..00000000 --- a/changelog.d/389.misc +++ /dev/null @@ -1 +0,0 @@ -Move dev tools from install_requires to extras_require. \ No newline at end of file diff --git a/changelog.d/394.misc b/changelog.d/394.misc deleted file mode 100644 index 7d894491..00000000 --- a/changelog.d/394.misc +++ /dev/null @@ -1 +0,0 @@ -Run background jobs in `run` rather than in Sydent's constructor. diff --git a/scripts/casefold_db.py b/scripts/casefold_db.py index 7734d3ce..d627213a 100755 --- 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,7 +268,6 @@ 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 @@ -271,18 +275,17 @@ def update_global_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 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 index 54499df3..5a8e0983 100644 --- 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 index 73db5b24..68d5cee9 100644 --- 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