From 7ae4f7236a0873e490c7f5dc5d69f3b922818cb4 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 3 Mar 2023 07:13:03 -0500 Subject: [PATCH 1/5] Configure ruff to automatically fix issues. (#15194) --- changelog.d/15194.misc | 1 + scripts-dev/lint.sh | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) create mode 100644 changelog.d/15194.misc diff --git a/changelog.d/15194.misc b/changelog.d/15194.misc new file mode 100644 index 0000000000..931bf5448a --- /dev/null +++ b/changelog.d/15194.misc @@ -0,0 +1 @@ +Automatically fix errors with `ruff`. diff --git a/scripts-dev/lint.sh b/scripts-dev/lint.sh index 392c509a8a..9e4ed3246e 100755 --- a/scripts-dev/lint.sh +++ b/scripts-dev/lint.sh @@ -112,7 +112,7 @@ python3 -m black "${files[@]}" # Catch any common programming mistakes in Python code. # --quiet suppresses the update check. -ruff --quiet "${files[@]}" +ruff --quiet --fix "${files[@]}" # Catch any common programming mistakes in Rust code. # From 848f7e3d5ff38ca28e56e6143e584974da1eec42 Mon Sep 17 00:00:00 2001 From: Quentin Gliech Date: Fri, 3 Mar 2023 13:22:49 +0100 Subject: [PATCH 2/5] Remove unspecced and buggy `PUT` method on the unstable `/rooms//batch_send` endpoint. (#15199) --- changelog.d/15199.misc | 1 + synapse/rest/client/room_batch.py | 16 +--------------- 2 files changed, 2 insertions(+), 15 deletions(-) create mode 100644 changelog.d/15199.misc diff --git a/changelog.d/15199.misc b/changelog.d/15199.misc new file mode 100644 index 0000000000..145b03fe16 --- /dev/null +++ b/changelog.d/15199.misc @@ -0,0 +1 @@ +Remove unspecced and buggy `PUT` method on the unstable `/rooms//batch_send` endpoint. diff --git a/synapse/rest/client/room_batch.py b/synapse/rest/client/room_batch.py index 10be4a781b..ef284ecc11 100644 --- a/synapse/rest/client/room_batch.py +++ b/synapse/rest/client/room_batch.py @@ -15,9 +15,7 @@ import logging import re from http import HTTPStatus -from typing import TYPE_CHECKING, Awaitable, Tuple - -from twisted.web.server import Request +from typing import TYPE_CHECKING, Tuple from synapse.api.constants import EventContentFields from synapse.api.errors import AuthError, Codes, SynapseError @@ -30,7 +28,6 @@ from synapse.http.servlet import ( parse_strings_from_args, ) from synapse.http.site import SynapseRequest -from synapse.rest.client.transactions import HttpTransactionCache from synapse.types import JsonDict if TYPE_CHECKING: @@ -79,7 +76,6 @@ class RoomBatchSendEventRestServlet(RestServlet): self.event_creation_handler = hs.get_event_creation_handler() self.auth = hs.get_auth() self.room_batch_handler = hs.get_room_batch_handler() - self.txns = HttpTransactionCache(hs) async def on_POST( self, request: SynapseRequest, room_id: str @@ -249,16 +245,6 @@ class RoomBatchSendEventRestServlet(RestServlet): return HTTPStatus.OK, response_dict - def on_GET(self, request: Request, room_id: str) -> Tuple[int, str]: - return HTTPStatus.NOT_IMPLEMENTED, "Not implemented" - - def on_PUT( - self, request: SynapseRequest, room_id: str - ) -> Awaitable[Tuple[int, JsonDict]]: - return self.txns.fetch_or_execute_request( - request, self.on_POST, request, room_id - ) - def register_servlets(hs: "HomeServer", http_server: HttpServer) -> None: msc2716_enabled = hs.config.experimental.msc2716_enabled From 02f74f3a997a4356b5bda957ebc51a829dad15f9 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 3 Mar 2023 08:13:37 -0500 Subject: [PATCH 3/5] Combine AbstractStreamIdTracker and AbstractStreamIdGenerator. (#15192) AbstractStreamIdTracker (now) has only a single sub-class: AbstractStreamIdGenerator, combine them to simplify some code and remove any direct references to AbstractStreamIdTracker. --- changelog.d/15192.misc | 1 + synapse/storage/databases/main/devices.py | 7 ++----- synapse/storage/databases/main/events_worker.py | 5 ++--- synapse/storage/databases/main/push_rule.py | 3 +-- synapse/storage/databases/main/pusher.py | 3 +-- synapse/storage/databases/main/receipts.py | 6 +++--- synapse/storage/util/id_generators.py | 17 +++++------------ 7 files changed, 15 insertions(+), 27 deletions(-) create mode 100644 changelog.d/15192.misc diff --git a/changelog.d/15192.misc b/changelog.d/15192.misc new file mode 100644 index 0000000000..1076686875 --- /dev/null +++ b/changelog.d/15192.misc @@ -0,0 +1 @@ +Combine `AbstractStreamIdTracker` and `AbstractStreamIdGenerator`. diff --git a/synapse/storage/databases/main/devices.py b/synapse/storage/databases/main/devices.py index 0dd15f16ff..5503621ad6 100644 --- a/synapse/storage/databases/main/devices.py +++ b/synapse/storage/databases/main/devices.py @@ -52,7 +52,6 @@ from synapse.storage.databases.main.roommember import RoomMemberWorkerStore from synapse.storage.types import Cursor from synapse.storage.util.id_generators import ( AbstractStreamIdGenerator, - AbstractStreamIdTracker, StreamIdGenerator, ) from synapse.types import JsonDict, StrCollection, get_verify_key_from_cross_signing_key @@ -91,7 +90,7 @@ class DeviceWorkerStore(RoomMemberWorkerStore, EndToEndKeyWorkerStore): # In the worker store this is an ID tracker which we overwrite in the non-worker # class below that is used on the main process. - self._device_list_id_gen: AbstractStreamIdTracker = StreamIdGenerator( + self._device_list_id_gen = StreamIdGenerator( db_conn, hs.get_replication_notifier(), "device_lists_stream", @@ -712,9 +711,7 @@ class DeviceWorkerStore(RoomMemberWorkerStore, EndToEndKeyWorkerStore): The new stream ID. """ - # TODO: this looks like it's _writing_. Should this be on DeviceStore rather - # than DeviceWorkerStore? - async with self._device_list_id_gen.get_next() as stream_id: # type: ignore[attr-defined] + async with self._device_list_id_gen.get_next() as stream_id: await self.db_pool.runInteraction( "add_user_sig_change_to_streams", self._add_user_signature_change_txn, diff --git a/synapse/storage/databases/main/events_worker.py b/synapse/storage/databases/main/events_worker.py index b7e7498125..20b7a68362 100644 --- a/synapse/storage/databases/main/events_worker.py +++ b/synapse/storage/databases/main/events_worker.py @@ -72,7 +72,6 @@ from synapse.storage.engines import PostgresEngine from synapse.storage.types import Cursor from synapse.storage.util.id_generators import ( AbstractStreamIdGenerator, - AbstractStreamIdTracker, MultiWriterIdGenerator, StreamIdGenerator, ) @@ -187,8 +186,8 @@ class EventsWorkerStore(SQLBaseStore): ): super().__init__(database, db_conn, hs) - self._stream_id_gen: AbstractStreamIdTracker - self._backfill_id_gen: AbstractStreamIdTracker + self._stream_id_gen: AbstractStreamIdGenerator + self._backfill_id_gen: AbstractStreamIdGenerator if isinstance(database.engine, PostgresEngine): # If we're using Postgres than we can use `MultiWriterIdGenerator` # regardless of whether this process writes to the streams or not. diff --git a/synapse/storage/databases/main/push_rule.py b/synapse/storage/databases/main/push_rule.py index 9b2bbe060d..9f862f00c1 100644 --- a/synapse/storage/databases/main/push_rule.py +++ b/synapse/storage/databases/main/push_rule.py @@ -46,7 +46,6 @@ from synapse.storage.engines import PostgresEngine, Sqlite3Engine from synapse.storage.push_rule import InconsistentRuleException, RuleNotFoundException from synapse.storage.util.id_generators import ( AbstractStreamIdGenerator, - AbstractStreamIdTracker, IdGenerator, StreamIdGenerator, ) @@ -118,7 +117,7 @@ class PushRulesWorkerStore( # In the worker store this is an ID tracker which we overwrite in the non-worker # class below that is used on the main process. - self._push_rules_stream_id_gen: AbstractStreamIdTracker = StreamIdGenerator( + self._push_rules_stream_id_gen = StreamIdGenerator( db_conn, hs.get_replication_notifier(), "push_rules_stream", diff --git a/synapse/storage/databases/main/pusher.py b/synapse/storage/databases/main/pusher.py index fddbc07afa..9a24f7a655 100644 --- a/synapse/storage/databases/main/pusher.py +++ b/synapse/storage/databases/main/pusher.py @@ -36,7 +36,6 @@ from synapse.storage.database import ( ) from synapse.storage.util.id_generators import ( AbstractStreamIdGenerator, - AbstractStreamIdTracker, StreamIdGenerator, ) from synapse.types import JsonDict @@ -60,7 +59,7 @@ class PusherWorkerStore(SQLBaseStore): # In the worker store this is an ID tracker which we overwrite in the non-worker # class below that is used on the main process. - self._pushers_id_gen: AbstractStreamIdTracker = StreamIdGenerator( + self._pushers_id_gen = StreamIdGenerator( db_conn, hs.get_replication_notifier(), "pushers", diff --git a/synapse/storage/databases/main/receipts.py b/synapse/storage/databases/main/receipts.py index 92a82240ab..074942b167 100644 --- a/synapse/storage/databases/main/receipts.py +++ b/synapse/storage/databases/main/receipts.py @@ -39,7 +39,7 @@ from synapse.storage.database import ( from synapse.storage.engines import PostgresEngine from synapse.storage.engines._base import IsolationLevel from synapse.storage.util.id_generators import ( - AbstractStreamIdTracker, + AbstractStreamIdGenerator, MultiWriterIdGenerator, StreamIdGenerator, ) @@ -65,7 +65,7 @@ class ReceiptsWorkerStore(SQLBaseStore): # In the worker store this is an ID tracker which we overwrite in the non-worker # class below that is used on the main process. - self._receipts_id_gen: AbstractStreamIdTracker + self._receipts_id_gen: AbstractStreamIdGenerator if isinstance(database.engine, PostgresEngine): self._can_write_to_receipts = ( @@ -768,7 +768,7 @@ class ReceiptsWorkerStore(SQLBaseStore): "insert_receipt_conv", self._graph_to_linear, room_id, event_ids ) - async with self._receipts_id_gen.get_next() as stream_id: # type: ignore[attr-defined] + async with self._receipts_id_gen.get_next() as stream_id: event_ts = await self.db_pool.runInteraction( "insert_linearized_receipt", self._insert_linearized_receipt_txn, diff --git a/synapse/storage/util/id_generators.py b/synapse/storage/util/id_generators.py index 334d3d718b..d2c874b9a8 100644 --- a/synapse/storage/util/id_generators.py +++ b/synapse/storage/util/id_generators.py @@ -93,8 +93,11 @@ def _load_current_id( return res -class AbstractStreamIdTracker(metaclass=abc.ABCMeta): - """Tracks the "current" stream ID of a stream that may have multiple writers. +class AbstractStreamIdGenerator(metaclass=abc.ABCMeta): + """Generates or tracks stream IDs for a stream that may have multiple writers. + + Each stream ID represents a write transaction, whose completion is tracked + so that the "current" stream ID of the stream can be determined. Stream IDs are monotonically increasing or decreasing integers representing write transactions. The "current" stream ID is the stream ID such that all transactions @@ -130,16 +133,6 @@ class AbstractStreamIdTracker(metaclass=abc.ABCMeta): """ raise NotImplementedError() - -class AbstractStreamIdGenerator(AbstractStreamIdTracker): - """Generates stream IDs for a stream that may have multiple writers. - - Each stream ID represents a write transaction, whose completion is tracked - so that the "current" stream ID of the stream can be determined. - - See `AbstractStreamIdTracker` for more details. - """ - @abc.abstractmethod def get_next(self) -> AsyncContextManager[int]: """ From 6b6e91e6106e84c391c9635fc830aa6081119c71 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Fri, 3 Mar 2023 15:22:06 +0100 Subject: [PATCH 4/5] Fix ICU tests on alpine / macOS. (#15177) The word boundary behaviour is slightly different, consider it acceptable for the tests. --- changelog.d/15177.bugfix | 1 + tests/storage/test_user_directory.py | 2 ++ 2 files changed, 3 insertions(+) create mode 100644 changelog.d/15177.bugfix diff --git a/changelog.d/15177.bugfix b/changelog.d/15177.bugfix new file mode 100644 index 0000000000..b9764947eb --- /dev/null +++ b/changelog.d/15177.bugfix @@ -0,0 +1 @@ +Fix test_icu_word_boundary_punctuation for alpine / macos installed ICU versions. diff --git a/tests/storage/test_user_directory.py b/tests/storage/test_user_directory.py index 43b724c4dd..8c72aa1722 100644 --- a/tests/storage/test_user_directory.py +++ b/tests/storage/test_user_directory.py @@ -696,6 +696,8 @@ class UserDirectoryICUTestCase(HomeserverTestCase): ["lazy'fox", "jumped", "over", "the", "dog"], # ICU 70 on Ubuntu 22.04 ["lazy'fox", "jumped:over", "the.dog"], + # pyicu 2.10.2 on Alpine edge / macOS + ["lazy'fox", "jumped", "over", "the.dog"], ), ) From 242d2a27ce18e682106854f5280566f4ced98c34 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 3 Mar 2023 14:26:14 +0000 Subject: [PATCH 5/5] Use nightly rustfmt in CI (#15188) As we use some nightly only options, e.g. to group and sort imports consistently. --- .github/workflows/tests.yml | 3 ++- changelog.d/15188.misc | 1 + rust/benches/evaluator.rs | 1 + rust/src/push/evaluator.rs | 2 +- 4 files changed, 5 insertions(+), 2 deletions(-) create mode 100644 changelog.d/15188.misc diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 48a33c2f49..806bd2bfa4 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -156,7 +156,8 @@ jobs: # We pin to a specific commit for paranoia's sake. uses: dtolnay/rust-toolchain@e12eda571dc9a5ee5d58eecf4738ec291c66f295 with: - toolchain: 1.58.1 + # We use nightly so that it correctly groups together imports + toolchain: nightly-2022-12-01 components: rustfmt - uses: Swatinem/rust-cache@v2 diff --git a/changelog.d/15188.misc b/changelog.d/15188.misc new file mode 100644 index 0000000000..e4e9472f01 --- /dev/null +++ b/changelog.d/15188.misc @@ -0,0 +1 @@ +Use nightly rustfmt in CI. diff --git a/rust/benches/evaluator.rs b/rust/benches/evaluator.rs index 7c987d4948..44477e63f7 100644 --- a/rust/benches/evaluator.rs +++ b/rust/benches/evaluator.rs @@ -14,6 +14,7 @@ #![feature(test)] use std::collections::BTreeSet; + use synapse::push::{ evaluator::PushRuleEvaluator, Condition, EventMatchCondition, FilteredPushRules, JsonValue, PushRules, SimpleJsonValue, diff --git a/rust/src/push/evaluator.rs b/rust/src/push/evaluator.rs index 55846627cc..1c2a05ad9a 100644 --- a/rust/src/push/evaluator.rs +++ b/rust/src/push/evaluator.rs @@ -15,7 +15,6 @@ use std::borrow::Cow; use std::collections::BTreeMap; -use crate::push::{EventMatchPatternType, JsonValue}; use anyhow::{Context, Error}; use lazy_static::lazy_static; use log::warn; @@ -27,6 +26,7 @@ use super::{ Action, Condition, ExactEventMatchCondition, FilteredPushRules, KnownCondition, SimpleJsonValue, }; +use crate::push::{EventMatchPatternType, JsonValue}; lazy_static! { /// Used to parse the `is` clause in the room member count condition.