From e2b8a90897e137fd118768a3bf35b70642916eb7 Mon Sep 17 00:00:00 2001 From: Jonathan de Jong Date: Mon, 5 Apr 2021 15:10:18 +0200 Subject: Update mypy configuration: `no_implicit_optional = True` (#9742) --- synapse/http/client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'synapse/http') diff --git a/synapse/http/client.py b/synapse/http/client.py index a0caba84e4..e691ba6d88 100644 --- a/synapse/http/client.py +++ b/synapse/http/client.py @@ -590,7 +590,7 @@ class SimpleHttpClient: uri: str, json_body: Any, args: Optional[QueryParams] = None, - headers: RawHeaders = None, + headers: Optional[RawHeaders] = None, ) -> Any: """Puts some json to the given URI. -- cgit 1.5.1 From 48d44ab1425ffac721b7d407823c2315cda1929a Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 8 Apr 2021 08:01:14 -0400 Subject: Record more information into structured logs. (#9654) Records additional request information into the structured logs, e.g. the requester, IP address, etc. --- changelog.d/9654.feature | 1 + synapse/http/site.py | 112 ++++++++++++++++++++------ synapse/logging/context.py | 70 +++++++++++++--- synapse/metrics/background_process_metrics.py | 18 +++-- synapse/replication/tcp/protocol.py | 5 +- tests/crypto/test_keyring.py | 23 +++--- tests/logging/test_terse_json.py | 70 +++++++++++++++- tests/unittest.py | 2 +- tests/util/caches/test_descriptors.py | 7 +- tests/util/test_logcontext.py | 35 +++----- 10 files changed, 255 insertions(+), 88 deletions(-) create mode 100644 changelog.d/9654.feature (limited to 'synapse/http') diff --git a/changelog.d/9654.feature b/changelog.d/9654.feature new file mode 100644 index 0000000000..a54c96cf19 --- /dev/null +++ b/changelog.d/9654.feature @@ -0,0 +1 @@ +Include request information in structured logging output. diff --git a/synapse/http/site.py b/synapse/http/site.py index 47754aff43..c0c873ce32 100644 --- a/synapse/http/site.py +++ b/synapse/http/site.py @@ -14,7 +14,7 @@ import contextlib import logging import time -from typing import Optional, Type, Union +from typing import Optional, Tuple, Type, Union import attr from zope.interface import implementer @@ -26,7 +26,11 @@ from twisted.web.server import Request, Site from synapse.config.server import ListenerConfig from synapse.http import get_request_user_agent, redact_uri from synapse.http.request_metrics import RequestMetrics, requests_counter -from synapse.logging.context import LoggingContext, PreserveLoggingContext +from synapse.logging.context import ( + ContextRequest, + LoggingContext, + PreserveLoggingContext, +) from synapse.types import Requester logger = logging.getLogger(__name__) @@ -63,7 +67,7 @@ class SynapseRequest(Request): # The requester, if authenticated. For federation requests this is the # server name, for client requests this is the Requester object. - self.requester = None # type: Optional[Union[Requester, str]] + self._requester = None # type: Optional[Union[Requester, str]] # we can't yet create the logcontext, as we don't know the method. self.logcontext = None # type: Optional[LoggingContext] @@ -93,6 +97,31 @@ class SynapseRequest(Request): self.site.site_tag, ) + @property + def requester(self) -> Optional[Union[Requester, str]]: + return self._requester + + @requester.setter + def requester(self, value: Union[Requester, str]) -> None: + # Store the requester, and update some properties based on it. + + # This should only be called once. + assert self._requester is None + + self._requester = value + + # A logging context should exist by now (and have a ContextRequest). + assert self.logcontext is not None + assert self.logcontext.request is not None + + ( + requester, + authenticated_entity, + ) = self.get_authenticated_entity() + self.logcontext.request.requester = requester + # If there's no authenticated entity, it was the requester. + self.logcontext.request.authenticated_entity = authenticated_entity or requester + def get_request_id(self): return "%s-%i" % (self.get_method(), self.request_seq) @@ -126,13 +155,60 @@ class SynapseRequest(Request): return self.method.decode("ascii") return method + def get_authenticated_entity(self) -> Tuple[Optional[str], Optional[str]]: + """ + Get the "authenticated" entity of the request, which might be the user + performing the action, or a user being puppeted by a server admin. + + Returns: + A tuple: + The first item is a string representing the user making the request. + + The second item is a string or None representing the user who + authenticated when making this request. See + Requester.authenticated_entity. + """ + # Convert the requester into a string that we can log + if isinstance(self._requester, str): + return self._requester, None + elif isinstance(self._requester, Requester): + requester = self._requester.user.to_string() + authenticated_entity = self._requester.authenticated_entity + + # If this is a request where the target user doesn't match the user who + # authenticated (e.g. and admin is puppetting a user) then we return both. + if self._requester.user.to_string() != authenticated_entity: + return requester, authenticated_entity + + return requester, None + elif self._requester is not None: + # This shouldn't happen, but we log it so we don't lose information + # and can see that we're doing something wrong. + return repr(self._requester), None # type: ignore[unreachable] + + return None, None + def render(self, resrc): # this is called once a Resource has been found to serve the request; in our # case the Resource in question will normally be a JsonResource. # create a LogContext for this request request_id = self.get_request_id() - self.logcontext = LoggingContext(request_id, request=request_id) + self.logcontext = LoggingContext( + request_id, + request=ContextRequest( + request_id=request_id, + ip_address=self.getClientIP(), + site_tag=self.site.site_tag, + # The requester is going to be unknown at this point. + requester=None, + authenticated_entity=None, + method=self.get_method(), + url=self.get_redacted_uri(), + protocol=self.clientproto.decode("ascii", errors="replace"), + user_agent=get_request_user_agent(self), + ), + ) # override the Server header which is set by twisted self.setHeader("Server", self.site.server_version_string) @@ -277,25 +353,6 @@ class SynapseRequest(Request): # to the client (nb may be negative) response_send_time = self.finish_time - self._processing_finished_time - # Convert the requester into a string that we can log - authenticated_entity = None - if isinstance(self.requester, str): - authenticated_entity = self.requester - elif isinstance(self.requester, Requester): - authenticated_entity = self.requester.authenticated_entity - - # If this is a request where the target user doesn't match the user who - # authenticated (e.g. and admin is puppetting a user) then we log both. - if self.requester.user.to_string() != authenticated_entity: - authenticated_entity = "{},{}".format( - authenticated_entity, - self.requester.user.to_string(), - ) - elif self.requester is not None: - # This shouldn't happen, but we log it so we don't lose information - # and can see that we're doing something wrong. - authenticated_entity = repr(self.requester) # type: ignore[unreachable] - user_agent = get_request_user_agent(self, "-") code = str(self.code) @@ -305,6 +362,13 @@ class SynapseRequest(Request): code += "!" log_level = logging.INFO if self._should_log_request() else logging.DEBUG + + # If this is a request where the target user doesn't match the user who + # authenticated (e.g. and admin is puppetting a user) then we log both. + requester, authenticated_entity = self.get_authenticated_entity() + if authenticated_entity: + requester = "{}.{}".format(authenticated_entity, requester) + self.site.access_logger.log( log_level, "%s - %s - {%s}" @@ -312,7 +376,7 @@ class SynapseRequest(Request): ' %sB %s "%s %s %s" "%s" [%d dbevts]', self.getClientIP(), self.site.site_tag, - authenticated_entity, + requester, processing_time, response_send_time, usage.ru_utime, diff --git a/synapse/logging/context.py b/synapse/logging/context.py index 03cf3c2b8e..e78343f554 100644 --- a/synapse/logging/context.py +++ b/synapse/logging/context.py @@ -22,7 +22,6 @@ them. See doc/log_contexts.rst for details on how this works. """ - import inspect import logging import threading @@ -30,6 +29,7 @@ import types import warnings from typing import TYPE_CHECKING, Optional, Tuple, TypeVar, Union +import attr from typing_extensions import Literal from twisted.internet import defer, threads @@ -181,6 +181,29 @@ class ContextResourceUsage: return res +@attr.s(slots=True) +class ContextRequest: + """ + A bundle of attributes from the SynapseRequest object. + + This exists to: + + * Avoid a cycle between LoggingContext and SynapseRequest. + * Be a single variable that can be passed from parent LoggingContexts to + their children. + """ + + request_id = attr.ib(type=str) + ip_address = attr.ib(type=str) + site_tag = attr.ib(type=str) + requester = attr.ib(type=Optional[str]) + authenticated_entity = attr.ib(type=Optional[str]) + method = attr.ib(type=str) + url = attr.ib(type=str) + protocol = attr.ib(type=str) + user_agent = attr.ib(type=str) + + LoggingContextOrSentinel = Union["LoggingContext", "_Sentinel"] @@ -256,7 +279,7 @@ class LoggingContext: self, name: Optional[str] = None, parent_context: "Optional[LoggingContext]" = None, - request: Optional[str] = None, + request: Optional[ContextRequest] = None, ) -> None: self.previous_context = current_context() self.name = name @@ -281,7 +304,11 @@ class LoggingContext: self.parent_context = parent_context if self.parent_context is not None: - self.parent_context.copy_to(self) + # we track the current request_id + self.request = self.parent_context.request + + # we also track the current scope: + self.scope = self.parent_context.scope if request is not None: # the request param overrides the request from the parent context @@ -289,7 +316,7 @@ class LoggingContext: def __str__(self) -> str: if self.request: - return str(self.request) + return self.request.request_id return "%s@%x" % (self.name, id(self)) @classmethod @@ -556,8 +583,23 @@ class LoggingContextFilter(logging.Filter): # we end up in a death spiral of infinite loops, so let's check, for # robustness' sake. if context is not None: - # Logging is interested in the request. - record.request = context.request # type: ignore + # Logging is interested in the request ID. Note that for backwards + # compatibility this is stored as the "request" on the record. + record.request = str(context) # type: ignore + + # Add some data from the HTTP request. + request = context.request + if request is None: + return True + + record.ip_address = request.ip_address # type: ignore + record.site_tag = request.site_tag # type: ignore + record.requester = request.requester # type: ignore + record.authenticated_entity = request.authenticated_entity # type: ignore + record.method = request.method # type: ignore + record.url = request.url # type: ignore + record.protocol = request.protocol # type: ignore + record.user_agent = request.user_agent # type: ignore return True @@ -630,8 +672,8 @@ def set_current_context(context: LoggingContextOrSentinel) -> LoggingContextOrSe def nested_logging_context(suffix: str) -> LoggingContext: """Creates a new logging context as a child of another. - The nested logging context will have a 'request' made up of the parent context's - request, plus the given suffix. + The nested logging context will have a 'name' made up of the parent context's + name, plus the given suffix. CPU/db usage stats will be added to the parent context's on exit. @@ -641,7 +683,7 @@ def nested_logging_context(suffix: str) -> LoggingContext: # ... do stuff Args: - suffix: suffix to add to the parent context's 'request'. + suffix: suffix to add to the parent context's 'name'. Returns: LoggingContext: new logging context. @@ -653,11 +695,17 @@ def nested_logging_context(suffix: str) -> LoggingContext: ) parent_context = None prefix = "" + request = None else: assert isinstance(curr_context, LoggingContext) parent_context = curr_context - prefix = str(parent_context.request) - return LoggingContext(parent_context=parent_context, request=prefix + "-" + suffix) + prefix = str(parent_context.name) + request = parent_context.request + return LoggingContext( + prefix + "-" + suffix, + parent_context=parent_context, + request=request, + ) def preserve_fn(f): diff --git a/synapse/metrics/background_process_metrics.py b/synapse/metrics/background_process_metrics.py index b56986d8e7..e8a9096c03 100644 --- a/synapse/metrics/background_process_metrics.py +++ b/synapse/metrics/background_process_metrics.py @@ -16,7 +16,7 @@ import logging import threading from functools import wraps -from typing import TYPE_CHECKING, Dict, Optional, Set +from typing import TYPE_CHECKING, Dict, Optional, Set, Union from prometheus_client.core import REGISTRY, Counter, Gauge @@ -199,11 +199,11 @@ def run_as_background_process(desc: str, func, *args, bg_start_span=True, **kwar _background_process_start_count.labels(desc).inc() _background_process_in_flight_count.labels(desc).inc() - with BackgroundProcessLoggingContext(desc, "%s-%i" % (desc, count)) as context: + with BackgroundProcessLoggingContext(desc, count) as context: try: ctx = noop_context_manager() if bg_start_span: - ctx = start_active_span(desc, tags={"request_id": context.request}) + ctx = start_active_span(desc, tags={"request_id": str(context)}) with ctx: return await maybe_awaitable(func(*args, **kwargs)) except Exception: @@ -242,13 +242,19 @@ class BackgroundProcessLoggingContext(LoggingContext): processes. """ - __slots__ = ["_proc"] + __slots__ = ["_id", "_proc"] - def __init__(self, name: str, request: Optional[str] = None): - super().__init__(name, request=request) + def __init__(self, name: str, id: Optional[Union[int, str]] = None): + super().__init__(name) + self._id = id self._proc = _BackgroundProcess(name, self) + def __str__(self) -> str: + if self._id is not None: + return "%s-%s" % (self.name, self._id) + return "%s@%x" % (self.name, id(self)) + def start(self, rusage: "Optional[resource._RUsage]"): """Log context has started running (again).""" diff --git a/synapse/replication/tcp/protocol.py b/synapse/replication/tcp/protocol.py index e829add257..d10d574246 100644 --- a/synapse/replication/tcp/protocol.py +++ b/synapse/replication/tcp/protocol.py @@ -184,8 +184,9 @@ class BaseReplicationStreamProtocol(LineOnlyReceiver): # a logcontext which we use for processing incoming commands. We declare it as a # background process so that the CPU stats get reported to prometheus. - ctx_name = "replication-conn-%s" % self.conn_id - self._logging_context = BackgroundProcessLoggingContext(ctx_name, ctx_name) + self._logging_context = BackgroundProcessLoggingContext( + "replication-conn", self.conn_id + ) def connectionMade(self): logger.info("[%s] Connection established", self.id()) diff --git a/tests/crypto/test_keyring.py b/tests/crypto/test_keyring.py index 30fcc4c1bf..946482b7e7 100644 --- a/tests/crypto/test_keyring.py +++ b/tests/crypto/test_keyring.py @@ -16,6 +16,7 @@ import time from mock import Mock +import attr import canonicaljson import signedjson.key import signedjson.sign @@ -68,6 +69,11 @@ class MockPerspectiveServer: signedjson.sign.sign_json(res, self.server_name, self.key) +@attr.s(slots=True) +class FakeRequest: + id = attr.ib() + + @logcontext_clean class KeyringTestCase(unittest.HomeserverTestCase): def check_context(self, val, expected): @@ -89,7 +95,7 @@ class KeyringTestCase(unittest.HomeserverTestCase): first_lookup_deferred = Deferred() async def first_lookup_fetch(keys_to_fetch): - self.assertEquals(current_context().request, "context_11") + self.assertEquals(current_context().request.id, "context_11") self.assertEqual(keys_to_fetch, {"server10": {get_key_id(key1): 0}}) await make_deferred_yieldable(first_lookup_deferred) @@ -102,9 +108,7 @@ class KeyringTestCase(unittest.HomeserverTestCase): mock_fetcher.get_keys.side_effect = first_lookup_fetch async def first_lookup(): - with LoggingContext("context_11") as context_11: - context_11.request = "context_11" - + with LoggingContext("context_11", request=FakeRequest("context_11")): res_deferreds = kr.verify_json_objects_for_server( [("server10", json1, 0, "test10"), ("server11", {}, 0, "test11")] ) @@ -130,7 +134,7 @@ class KeyringTestCase(unittest.HomeserverTestCase): # should block rather than start a second call async def second_lookup_fetch(keys_to_fetch): - self.assertEquals(current_context().request, "context_12") + self.assertEquals(current_context().request.id, "context_12") return { "server10": { get_key_id(key1): FetchKeyResult(get_verify_key(key1), 100) @@ -142,9 +146,7 @@ class KeyringTestCase(unittest.HomeserverTestCase): second_lookup_state = [0] async def second_lookup(): - with LoggingContext("context_12") as context_12: - context_12.request = "context_12" - + with LoggingContext("context_12", request=FakeRequest("context_12")): res_deferreds_2 = kr.verify_json_objects_for_server( [("server10", json1, 0, "test")] ) @@ -589,10 +591,7 @@ def get_key_id(key): @defer.inlineCallbacks def run_in_context(f, *args, **kwargs): - with LoggingContext("testctx") as ctx: - # we set the "request" prop to make it easier to follow what's going on in the - # logs. - ctx.request = "testctx" + with LoggingContext("testctx"): rv = yield f(*args, **kwargs) return rv diff --git a/tests/logging/test_terse_json.py b/tests/logging/test_terse_json.py index 48a74e2eee..bfe0d11c93 100644 --- a/tests/logging/test_terse_json.py +++ b/tests/logging/test_terse_json.py @@ -12,15 +12,20 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. - import json import logging -from io import StringIO +from io import BytesIO, StringIO + +from mock import Mock, patch + +from twisted.web.server import Request +from synapse.http.site import SynapseRequest from synapse.logging._terse_json import JsonFormatter, TerseJsonFormatter from synapse.logging.context import LoggingContext, LoggingContextFilter from tests.logging import LoggerCleanupMixin +from tests.server import FakeChannel from tests.unittest import TestCase @@ -120,7 +125,7 @@ class TerseJsonTestCase(LoggerCleanupMixin, TestCase): handler.addFilter(LoggingContextFilter()) logger = self.get_logger(handler) - with LoggingContext(request="test"): + with LoggingContext("name"): logger.info("Hello there, %s!", "wally") log = self.get_log_line() @@ -134,4 +139,61 @@ class TerseJsonTestCase(LoggerCleanupMixin, TestCase): ] self.assertCountEqual(log.keys(), expected_log_keys) self.assertEqual(log["log"], "Hello there, wally!") - self.assertEqual(log["request"], "test") + self.assertTrue(log["request"].startswith("name@")) + + def test_with_request_context(self): + """ + Information from the logging context request should be added to the JSON response. + """ + handler = logging.StreamHandler(self.output) + handler.setFormatter(JsonFormatter()) + handler.addFilter(LoggingContextFilter()) + logger = self.get_logger(handler) + + # A full request isn't needed here. + site = Mock(spec=["site_tag", "server_version_string", "getResourceFor"]) + site.site_tag = "test-site" + site.server_version_string = "Server v1" + request = SynapseRequest(FakeChannel(site, None)) + # Call requestReceived to finish instantiating the object. + request.content = BytesIO() + # Partially skip some of the internal processing of SynapseRequest. + request._started_processing = Mock() + request.request_metrics = Mock(spec=["name"]) + with patch.object(Request, "render"): + request.requestReceived(b"POST", b"/_matrix/client/versions", b"1.1") + + # Also set the requester to ensure the processing works. + request.requester = "@foo:test" + + with LoggingContext(parent_context=request.logcontext): + logger.info("Hello there, %s!", "wally") + + log = self.get_log_line() + + # The terse logger includes additional request information, if possible. + expected_log_keys = [ + "log", + "level", + "namespace", + "request", + "ip_address", + "site_tag", + "requester", + "authenticated_entity", + "method", + "url", + "protocol", + "user_agent", + ] + self.assertCountEqual(log.keys(), expected_log_keys) + self.assertEqual(log["log"], "Hello there, wally!") + self.assertTrue(log["request"].startswith("POST-")) + self.assertEqual(log["ip_address"], "127.0.0.1") + self.assertEqual(log["site_tag"], "test-site") + self.assertEqual(log["requester"], "@foo:test") + self.assertEqual(log["authenticated_entity"], "@foo:test") + self.assertEqual(log["method"], "POST") + self.assertEqual(log["url"], "/_matrix/client/versions") + self.assertEqual(log["protocol"], "1.1") + self.assertEqual(log["user_agent"], "") diff --git a/tests/unittest.py b/tests/unittest.py index 58a4daa1ec..57b6a395c7 100644 --- a/tests/unittest.py +++ b/tests/unittest.py @@ -471,7 +471,7 @@ class HomeserverTestCase(TestCase): kwargs["config"] = config_obj async def run_bg_updates(): - with LoggingContext("run_bg_updates", request="run_bg_updates-1"): + with LoggingContext("run_bg_updates"): while not await stor.db_pool.updates.has_completed_background_updates(): await stor.db_pool.updates.do_next_background_update(1) diff --git a/tests/util/caches/test_descriptors.py b/tests/util/caches/test_descriptors.py index afb11b9caf..e434e21aee 100644 --- a/tests/util/caches/test_descriptors.py +++ b/tests/util/caches/test_descriptors.py @@ -661,14 +661,13 @@ class CachedListDescriptorTestCase(unittest.TestCase): @descriptors.cachedList("fn", "args1") async def list_fn(self, args1, arg2): - assert current_context().request == "c1" + assert current_context().name == "c1" # we want this to behave like an asynchronous function await run_on_reactor() - assert current_context().request == "c1" + assert current_context().name == "c1" return self.mock(args1, arg2) - with LoggingContext() as c1: - c1.request = "c1" + with LoggingContext("c1") as c1: obj = Cls() obj.mock.return_value = {10: "fish", 20: "chips"} d1 = obj.list_fn([10, 20], 2) diff --git a/tests/util/test_logcontext.py b/tests/util/test_logcontext.py index 58ee918f65..5d9c4665aa 100644 --- a/tests/util/test_logcontext.py +++ b/tests/util/test_logcontext.py @@ -17,11 +17,10 @@ from .. import unittest class LoggingContextTestCase(unittest.TestCase): def _check_test_key(self, value): - self.assertEquals(current_context().request, value) + self.assertEquals(current_context().name, value) def test_with_context(self): - with LoggingContext() as context_one: - context_one.request = "test" + with LoggingContext("test"): self._check_test_key("test") @defer.inlineCallbacks @@ -30,15 +29,13 @@ class LoggingContextTestCase(unittest.TestCase): @defer.inlineCallbacks def competing_callback(): - with LoggingContext() as competing_context: - competing_context.request = "competing" + with LoggingContext("competing"): yield clock.sleep(0) self._check_test_key("competing") reactor.callLater(0, competing_callback) - with LoggingContext() as context_one: - context_one.request = "one" + with LoggingContext("one"): yield clock.sleep(0) self._check_test_key("one") @@ -47,9 +44,7 @@ class LoggingContextTestCase(unittest.TestCase): callback_completed = [False] - with LoggingContext() as context_one: - context_one.request = "one" - + with LoggingContext("one"): # fire off function, but don't wait on it. d2 = run_in_background(function) @@ -133,9 +128,7 @@ class LoggingContextTestCase(unittest.TestCase): sentinel_context = current_context() - with LoggingContext() as context_one: - context_one.request = "one" - + with LoggingContext("one"): d1 = make_deferred_yieldable(blocking_function()) # make sure that the context was reset by make_deferred_yieldable self.assertIs(current_context(), sentinel_context) @@ -149,9 +142,7 @@ class LoggingContextTestCase(unittest.TestCase): def test_make_deferred_yieldable_with_chained_deferreds(self): sentinel_context = current_context() - with LoggingContext() as context_one: - context_one.request = "one" - + with LoggingContext("one"): d1 = make_deferred_yieldable(_chained_deferred_function()) # make sure that the context was reset by make_deferred_yieldable self.assertIs(current_context(), sentinel_context) @@ -166,9 +157,7 @@ class LoggingContextTestCase(unittest.TestCase): """Check that make_deferred_yieldable does the right thing when its argument isn't actually a deferred""" - with LoggingContext() as context_one: - context_one.request = "one" - + with LoggingContext("one"): d1 = make_deferred_yieldable("bum") self._check_test_key("one") @@ -177,9 +166,9 @@ class LoggingContextTestCase(unittest.TestCase): self._check_test_key("one") def test_nested_logging_context(self): - with LoggingContext(request="foo"): + with LoggingContext("foo"): nested_context = nested_logging_context(suffix="bar") - self.assertEqual(nested_context.request, "foo-bar") + self.assertEqual(nested_context.name, "foo-bar") @defer.inlineCallbacks def test_make_deferred_yieldable_with_await(self): @@ -193,9 +182,7 @@ class LoggingContextTestCase(unittest.TestCase): sentinel_context = current_context() - with LoggingContext() as context_one: - context_one.request = "one" - + with LoggingContext("one"): d1 = make_deferred_yieldable(blocking_function()) # make sure that the context was reset by make_deferred_yieldable self.assertIs(current_context(), sentinel_context) -- cgit 1.5.1 From 2ca4e349e9d0c606d802ae15c06089080fa4f27e Mon Sep 17 00:00:00 2001 From: Jonathan de Jong Date: Thu, 8 Apr 2021 23:38:54 +0200 Subject: Bugbear: Add Mutable Parameter fixes (#9682) Part of #9366 Adds in fixes for B006 and B008, both relating to mutable parameter lint errors. Signed-off-by: Jonathan de Jong --- changelog.d/9682.misc | 1 + contrib/cmdclient/console.py | 5 ++++- contrib/cmdclient/http.py | 24 +++++++++++++++----- setup.cfg | 4 ++-- synapse/appservice/scheduler.py | 6 ++--- synapse/config/ratelimiting.py | 6 +++-- synapse/events/__init__.py | 14 ++++++++---- synapse/federation/units.py | 5 +++-- synapse/handlers/appservice.py | 4 ++-- synapse/handlers/federation.py | 2 +- synapse/handlers/message.py | 11 ++++++--- synapse/handlers/register.py | 4 +++- synapse/handlers/sync.py | 8 +++---- synapse/http/client.py | 4 ++-- synapse/http/proxyagent.py | 6 +++-- synapse/logging/opentracing.py | 3 ++- synapse/module_api/__init__.py | 12 +++++----- synapse/notifier.py | 19 +++++++++------- synapse/storage/database.py | 20 ++++++++++++----- synapse/storage/databases/main/events.py | 7 ++++-- synapse/storage/databases/main/group_server.py | 4 +++- synapse/storage/databases/main/state.py | 6 +++-- synapse/storage/databases/state/bg_updates.py | 5 ++++- synapse/storage/databases/state/store.py | 5 +++-- synapse/storage/state.py | 26 +++++++++++++--------- synapse/storage/util/id_generators.py | 11 +++++++-- synapse/util/caches/lrucache.py | 14 +++++++----- .../federation/test_matrix_federation_agent.py | 15 +++++++++---- tests/replication/_base.py | 4 ++-- tests/replication/slave/storage/test_events.py | 16 ++++++++----- tests/rest/client/v1/test_rooms.py | 5 ++++- tests/rest/client/v1/utils.py | 14 ++++++++---- tests/rest/client/v2_alpha/test_relations.py | 5 +++-- tests/storage/test_id_generators.py | 14 +++++++----- tests/storage/test_redaction.py | 10 +++++++-- tests/test_state.py | 5 +++-- tests/test_visibility.py | 7 ++++-- tests/util/test_ratelimitutils.py | 6 +++-- 38 files changed, 224 insertions(+), 113 deletions(-) create mode 100644 changelog.d/9682.misc (limited to 'synapse/http') diff --git a/changelog.d/9682.misc b/changelog.d/9682.misc new file mode 100644 index 0000000000..428a466fac --- /dev/null +++ b/changelog.d/9682.misc @@ -0,0 +1 @@ +Introduce flake8-bugbear to the test suite and fix some of its lint violations. diff --git a/contrib/cmdclient/console.py b/contrib/cmdclient/console.py index 67e032244e..856dd437db 100755 --- a/contrib/cmdclient/console.py +++ b/contrib/cmdclient/console.py @@ -24,6 +24,7 @@ import sys import time import urllib from http import TwistedHttpClient +from typing import Optional import nacl.encoding import nacl.signing @@ -718,7 +719,7 @@ class SynapseCmd(cmd.Cmd): method, path, data=None, - query_params={"access_token": None}, + query_params: Optional[dict] = None, alt_text=None, ): """Runs an HTTP request and pretty prints the output. @@ -729,6 +730,8 @@ class SynapseCmd(cmd.Cmd): data: Raw JSON data if any query_params: dict of query parameters to add to the url """ + query_params = query_params or {"access_token": None} + url = self._url() + path if "access_token" in query_params: query_params["access_token"] = self._tok() diff --git a/contrib/cmdclient/http.py b/contrib/cmdclient/http.py index 851e80c25b..1cf913756e 100644 --- a/contrib/cmdclient/http.py +++ b/contrib/cmdclient/http.py @@ -16,6 +16,7 @@ import json import urllib from pprint import pformat +from typing import Optional from twisted.internet import defer, reactor from twisted.web.client import Agent, readBody @@ -85,8 +86,9 @@ class TwistedHttpClient(HttpClient): body = yield readBody(response) defer.returnValue(json.loads(body)) - def _create_put_request(self, url, json_data, headers_dict={}): + def _create_put_request(self, url, json_data, headers_dict: Optional[dict] = None): """Wrapper of _create_request to issue a PUT request""" + headers_dict = headers_dict or {} if "Content-Type" not in headers_dict: raise defer.error(RuntimeError("Must include Content-Type header for PUTs")) @@ -95,14 +97,22 @@ class TwistedHttpClient(HttpClient): "PUT", url, producer=_JsonProducer(json_data), headers_dict=headers_dict ) - def _create_get_request(self, url, headers_dict={}): + def _create_get_request(self, url, headers_dict: Optional[dict] = None): """Wrapper of _create_request to issue a GET request""" - return self._create_request("GET", url, headers_dict=headers_dict) + return self._create_request("GET", url, headers_dict=headers_dict or {}) @defer.inlineCallbacks def do_request( - self, method, url, data=None, qparams=None, jsonreq=True, headers={} + self, + method, + url, + data=None, + qparams=None, + jsonreq=True, + headers: Optional[dict] = None, ): + headers = headers or {} + if qparams: url = "%s?%s" % (url, urllib.urlencode(qparams, True)) @@ -123,8 +133,12 @@ class TwistedHttpClient(HttpClient): defer.returnValue(json.loads(body)) @defer.inlineCallbacks - def _create_request(self, method, url, producer=None, headers_dict={}): + def _create_request( + self, method, url, producer=None, headers_dict: Optional[dict] = None + ): """Creates and sends a request to the given url""" + headers_dict = headers_dict or {} + headers_dict["User-Agent"] = ["Synapse Cmd Client"] retries_left = 5 diff --git a/setup.cfg b/setup.cfg index 7329eed213..5fdb51ac73 100644 --- a/setup.cfg +++ b/setup.cfg @@ -18,8 +18,8 @@ ignore = # E203: whitespace before ':' (which is contrary to pep8?) # E731: do not assign a lambda expression, use a def # E501: Line too long (black enforces this for us) -# B00*: Subsection of the bugbear suite (TODO: add in remaining fixes) -ignore=W503,W504,E203,E731,E501,B006,B007,B008 +# B007: Subsection of the bugbear suite (TODO: add in remaining fixes) +ignore=W503,W504,E203,E731,E501,B007 [isort] line_length = 88 diff --git a/synapse/appservice/scheduler.py b/synapse/appservice/scheduler.py index 366c476f80..5203ffe90f 100644 --- a/synapse/appservice/scheduler.py +++ b/synapse/appservice/scheduler.py @@ -49,7 +49,7 @@ This is all tied together by the AppServiceScheduler which DIs the required components. """ import logging -from typing import List +from typing import List, Optional from synapse.appservice import ApplicationService, ApplicationServiceState from synapse.events import EventBase @@ -191,11 +191,11 @@ class _TransactionController: self, service: ApplicationService, events: List[EventBase], - ephemeral: List[JsonDict] = [], + ephemeral: Optional[List[JsonDict]] = None, ): try: txn = await self.store.create_appservice_txn( - service=service, events=events, ephemeral=ephemeral + service=service, events=events, ephemeral=ephemeral or [] ) service_is_up = await self._is_service_up(service) if service_is_up: diff --git a/synapse/config/ratelimiting.py b/synapse/config/ratelimiting.py index 3f3997f4e5..7a8d5851c4 100644 --- a/synapse/config/ratelimiting.py +++ b/synapse/config/ratelimiting.py @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -from typing import Dict +from typing import Dict, Optional from ._base import Config @@ -21,8 +21,10 @@ class RateLimitConfig: def __init__( self, config: Dict[str, float], - defaults={"per_second": 0.17, "burst_count": 3.0}, + defaults: Optional[Dict[str, float]] = None, ): + defaults = defaults or {"per_second": 0.17, "burst_count": 3.0} + self.per_second = config.get("per_second", defaults["per_second"]) self.burst_count = int(config.get("burst_count", defaults["burst_count"])) diff --git a/synapse/events/__init__.py b/synapse/events/__init__.py index 8f6b955d17..f9032e3697 100644 --- a/synapse/events/__init__.py +++ b/synapse/events/__init__.py @@ -330,9 +330,11 @@ class FrozenEvent(EventBase): self, event_dict: JsonDict, room_version: RoomVersion, - internal_metadata_dict: JsonDict = {}, + internal_metadata_dict: Optional[JsonDict] = None, rejected_reason: Optional[str] = None, ): + internal_metadata_dict = internal_metadata_dict or {} + event_dict = dict(event_dict) # Signatures is a dict of dicts, and this is faster than doing a @@ -386,9 +388,11 @@ class FrozenEventV2(EventBase): self, event_dict: JsonDict, room_version: RoomVersion, - internal_metadata_dict: JsonDict = {}, + internal_metadata_dict: Optional[JsonDict] = None, rejected_reason: Optional[str] = None, ): + internal_metadata_dict = internal_metadata_dict or {} + event_dict = dict(event_dict) # Signatures is a dict of dicts, and this is faster than doing a @@ -507,9 +511,11 @@ def _event_type_from_format_version(format_version: int) -> Type[EventBase]: def make_event_from_dict( event_dict: JsonDict, room_version: RoomVersion = RoomVersions.V1, - internal_metadata_dict: JsonDict = {}, + internal_metadata_dict: Optional[JsonDict] = None, rejected_reason: Optional[str] = None, ) -> EventBase: """Construct an EventBase from the given event dict""" event_type = _event_type_from_format_version(room_version.event_format) - return event_type(event_dict, room_version, internal_metadata_dict, rejected_reason) + return event_type( + event_dict, room_version, internal_metadata_dict or {}, rejected_reason + ) diff --git a/synapse/federation/units.py b/synapse/federation/units.py index b662c42621..0f8bf000ac 100644 --- a/synapse/federation/units.py +++ b/synapse/federation/units.py @@ -18,6 +18,7 @@ server protocol. """ import logging +from typing import Optional import attr @@ -98,7 +99,7 @@ class Transaction(JsonEncodedObject): "pdus", ] - def __init__(self, transaction_id=None, pdus=[], **kwargs): + def __init__(self, transaction_id=None, pdus: Optional[list] = None, **kwargs): """If we include a list of pdus then we decode then as PDU's automatically. """ @@ -107,7 +108,7 @@ class Transaction(JsonEncodedObject): if "edus" in kwargs and not kwargs["edus"]: del kwargs["edus"] - super().__init__(transaction_id=transaction_id, pdus=pdus, **kwargs) + super().__init__(transaction_id=transaction_id, pdus=pdus or [], **kwargs) @staticmethod def create_new(pdus, **kwargs): diff --git a/synapse/handlers/appservice.py b/synapse/handlers/appservice.py index 996f9e5deb..9fb7ee335d 100644 --- a/synapse/handlers/appservice.py +++ b/synapse/handlers/appservice.py @@ -182,7 +182,7 @@ class ApplicationServicesHandler: self, stream_key: str, new_token: Optional[int], - users: Collection[Union[str, UserID]] = [], + users: Optional[Collection[Union[str, UserID]]] = None, ): """This is called by the notifier in the background when a ephemeral event handled by the homeserver. @@ -215,7 +215,7 @@ class ApplicationServicesHandler: # We only start a new background process if necessary rather than # optimistically (to cut down on overhead). self._notify_interested_services_ephemeral( - services, stream_key, new_token, users + services, stream_key, new_token, users or [] ) @wrap_as_background_process("notify_interested_services_ephemeral") diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index 5ea8a7b603..67888898ff 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -1790,7 +1790,7 @@ class FederationHandler(BaseHandler): room_id: str, user_id: str, membership: str, - content: JsonDict = {}, + content: JsonDict, params: Optional[Dict[str, Union[str, Iterable[str]]]] = None, ) -> Tuple[str, EventBase, RoomVersion]: ( diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index 6069968f7f..125dae6d25 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -137,7 +137,7 @@ class MessageHandler: self, user_id: str, room_id: str, - state_filter: StateFilter = StateFilter.all(), + state_filter: Optional[StateFilter] = None, at_token: Optional[StreamToken] = None, is_guest: bool = False, ) -> List[dict]: @@ -164,6 +164,8 @@ class MessageHandler: AuthError (403) if the user doesn't have permission to view members of this room. """ + state_filter = state_filter or StateFilter.all() + if at_token: # FIXME this claims to get the state at a stream position, but # get_recent_events_for_room operates by topo ordering. This therefore @@ -874,7 +876,7 @@ class EventCreationHandler: event: EventBase, context: EventContext, ratelimit: bool = True, - extra_users: List[UserID] = [], + extra_users: Optional[List[UserID]] = None, ignore_shadow_ban: bool = False, ) -> EventBase: """Processes a new event. @@ -902,6 +904,7 @@ class EventCreationHandler: Raises: ShadowBanError if the requester has been shadow-banned. """ + extra_users = extra_users or [] # we don't apply shadow-banning to membership events here. Invites are blocked # higher up the stack, and we allow shadow-banned users to send join and leave @@ -1071,7 +1074,7 @@ class EventCreationHandler: event: EventBase, context: EventContext, ratelimit: bool = True, - extra_users: List[UserID] = [], + extra_users: Optional[List[UserID]] = None, ) -> EventBase: """Called when we have fully built the event, have already calculated the push actions for the event, and checked auth. @@ -1083,6 +1086,8 @@ class EventCreationHandler: it was de-duplicated (e.g. because we had already persisted an event with the same transaction ID.) """ + extra_users = extra_users or [] + assert self.storage.persistence is not None assert self._events_shard_config.should_handle( self._instance_name, event.room_id diff --git a/synapse/handlers/register.py b/synapse/handlers/register.py index 9701b76d0f..3b6660c873 100644 --- a/synapse/handlers/register.py +++ b/synapse/handlers/register.py @@ -169,7 +169,7 @@ class RegistrationHandler(BaseHandler): user_type: Optional[str] = None, default_display_name: Optional[str] = None, address: Optional[str] = None, - bind_emails: Iterable[str] = [], + bind_emails: Optional[Iterable[str]] = None, by_admin: bool = False, user_agent_ips: Optional[List[Tuple[str, str]]] = None, auth_provider_id: Optional[str] = None, @@ -204,6 +204,8 @@ class RegistrationHandler(BaseHandler): Raises: SynapseError if there was a problem registering. """ + bind_emails = bind_emails or [] + await self.check_registration_ratelimit(address) result = await self.spam_checker.check_registration_for_spam( diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index ff11266c67..f8d88ef77b 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -548,7 +548,7 @@ class SyncHandler: ) async def get_state_after_event( - self, event: EventBase, state_filter: StateFilter = StateFilter.all() + self, event: EventBase, state_filter: Optional[StateFilter] = None ) -> StateMap[str]: """ Get the room state after the given event @@ -558,7 +558,7 @@ class SyncHandler: state_filter: The state filter used to fetch state from the database. """ state_ids = await self.state_store.get_state_ids_for_event( - event.event_id, state_filter=state_filter + event.event_id, state_filter=state_filter or StateFilter.all() ) if event.is_state(): state_ids = dict(state_ids) @@ -569,7 +569,7 @@ class SyncHandler: self, room_id: str, stream_position: StreamToken, - state_filter: StateFilter = StateFilter.all(), + state_filter: Optional[StateFilter] = None, ) -> StateMap[str]: """Get the room state at a particular stream position @@ -589,7 +589,7 @@ class SyncHandler: if last_events: last_event = last_events[-1] state = await self.get_state_after_event( - last_event, state_filter=state_filter + last_event, state_filter=state_filter or StateFilter.all() ) else: diff --git a/synapse/http/client.py b/synapse/http/client.py index e691ba6d88..f7a07f0466 100644 --- a/synapse/http/client.py +++ b/synapse/http/client.py @@ -297,7 +297,7 @@ class SimpleHttpClient: def __init__( self, hs: "HomeServer", - treq_args: Dict[str, Any] = {}, + treq_args: Optional[Dict[str, Any]] = None, ip_whitelist: Optional[IPSet] = None, ip_blacklist: Optional[IPSet] = None, use_proxy: bool = False, @@ -317,7 +317,7 @@ class SimpleHttpClient: self._ip_whitelist = ip_whitelist self._ip_blacklist = ip_blacklist - self._extra_treq_args = treq_args + self._extra_treq_args = treq_args or {} self.user_agent = hs.version_string self.clock = hs.get_clock() diff --git a/synapse/http/proxyagent.py b/synapse/http/proxyagent.py index 16ec850064..ea5ad14cb0 100644 --- a/synapse/http/proxyagent.py +++ b/synapse/http/proxyagent.py @@ -27,7 +27,7 @@ from twisted.python.failure import Failure from twisted.web.client import URI, BrowserLikePolicyForHTTPS, _AgentBase from twisted.web.error import SchemeNotSupported from twisted.web.http_headers import Headers -from twisted.web.iweb import IAgent +from twisted.web.iweb import IAgent, IPolicyForHTTPS from synapse.http.connectproxyclient import HTTPConnectProxyEndpoint @@ -88,12 +88,14 @@ class ProxyAgent(_AgentBase): self, reactor, proxy_reactor=None, - contextFactory=BrowserLikePolicyForHTTPS(), + contextFactory: Optional[IPolicyForHTTPS] = None, connectTimeout=None, bindAddress=None, pool=None, use_proxy=False, ): + contextFactory = contextFactory or BrowserLikePolicyForHTTPS() + _AgentBase.__init__(self, reactor, pool) if proxy_reactor is None: diff --git a/synapse/logging/opentracing.py b/synapse/logging/opentracing.py index b8081f197e..bfe9136fd8 100644 --- a/synapse/logging/opentracing.py +++ b/synapse/logging/opentracing.py @@ -486,7 +486,7 @@ def start_active_span_from_request( def start_active_span_from_edu( edu_content, operation_name, - references=[], + references: Optional[list] = None, tags=None, start_time=None, ignore_active_span=False, @@ -501,6 +501,7 @@ def start_active_span_from_edu( For the other args see opentracing.tracer """ + references = references or [] if opentracing is None: return noop_context_manager() diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index 3ecd46c038..ca1bd4cdc9 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -14,7 +14,7 @@ # See the License for the specific language governing permissions and # limitations under the License. import logging -from typing import TYPE_CHECKING, Any, Generator, Iterable, Optional, Tuple +from typing import TYPE_CHECKING, Any, Generator, Iterable, List, Optional, Tuple from twisted.internet import defer @@ -127,7 +127,7 @@ class ModuleApi: return defer.ensureDeferred(self._auth_handler.check_user_exists(user_id)) @defer.inlineCallbacks - def register(self, localpart, displayname=None, emails=[]): + def register(self, localpart, displayname=None, emails: Optional[List[str]] = None): """Registers a new user with given localpart and optional displayname, emails. Also returns an access token for the new user. @@ -147,11 +147,13 @@ class ModuleApi: logger.warning( "Using deprecated ModuleApi.register which creates a dummy user device." ) - user_id = yield self.register_user(localpart, displayname, emails) + user_id = yield self.register_user(localpart, displayname, emails or []) _, access_token = yield self.register_device(user_id) return user_id, access_token - def register_user(self, localpart, displayname=None, emails=[]): + def register_user( + self, localpart, displayname=None, emails: Optional[List[str]] = None + ): """Registers a new user with given localpart and optional displayname, emails. Args: @@ -170,7 +172,7 @@ class ModuleApi: self._hs.get_registration_handler().register_user( localpart=localpart, default_display_name=displayname, - bind_emails=emails, + bind_emails=emails or [], ) ) diff --git a/synapse/notifier.py b/synapse/notifier.py index c178db57e3..7ce34380af 100644 --- a/synapse/notifier.py +++ b/synapse/notifier.py @@ -276,7 +276,7 @@ class Notifier: event: EventBase, event_pos: PersistedEventPosition, max_room_stream_token: RoomStreamToken, - extra_users: Collection[UserID] = [], + extra_users: Optional[Collection[UserID]] = None, ): """Unwraps event and calls `on_new_room_event_args`.""" self.on_new_room_event_args( @@ -286,7 +286,7 @@ class Notifier: state_key=event.get("state_key"), membership=event.content.get("membership"), max_room_stream_token=max_room_stream_token, - extra_users=extra_users, + extra_users=extra_users or [], ) def on_new_room_event_args( @@ -297,7 +297,7 @@ class Notifier: membership: Optional[str], event_pos: PersistedEventPosition, max_room_stream_token: RoomStreamToken, - extra_users: Collection[UserID] = [], + extra_users: Optional[Collection[UserID]] = None, ): """Used by handlers to inform the notifier something has happened in the room, room event wise. @@ -313,7 +313,7 @@ class Notifier: self.pending_new_room_events.append( _PendingRoomEventEntry( event_pos=event_pos, - extra_users=extra_users, + extra_users=extra_users or [], room_id=room_id, type=event_type, state_key=state_key, @@ -382,14 +382,14 @@ class Notifier: self, stream_key: str, new_token: Union[int, RoomStreamToken], - users: Collection[Union[str, UserID]] = [], + users: Optional[Collection[Union[str, UserID]]] = None, ): try: stream_token = None if isinstance(new_token, int): stream_token = new_token self.appservice_handler.notify_interested_services_ephemeral( - stream_key, stream_token, users + stream_key, stream_token, users or [] ) except Exception: logger.exception("Error notifying application services of event") @@ -404,13 +404,16 @@ class Notifier: self, stream_key: str, new_token: Union[int, RoomStreamToken], - users: Collection[Union[str, UserID]] = [], - rooms: Collection[str] = [], + users: Optional[Collection[Union[str, UserID]]] = None, + rooms: Optional[Collection[str]] = None, ): """Used to inform listeners that something has happened event wise. Will wake up all listeners for the given users and rooms. """ + users = users or [] + rooms = rooms or [] + with Measure(self.clock, "on_new_event"): user_streams = set() diff --git a/synapse/storage/database.py b/synapse/storage/database.py index b302cd5786..fa15b0ce5b 100644 --- a/synapse/storage/database.py +++ b/synapse/storage/database.py @@ -900,7 +900,7 @@ class DatabasePool: table: str, keyvalues: Dict[str, Any], values: Dict[str, Any], - insertion_values: Dict[str, Any] = {}, + insertion_values: Optional[Dict[str, Any]] = None, desc: str = "simple_upsert", lock: bool = True, ) -> Optional[bool]: @@ -927,6 +927,8 @@ class DatabasePool: Native upserts always return None. Emulated upserts return True if a new entry was created, False if an existing one was updated. """ + insertion_values = insertion_values or {} + attempts = 0 while True: try: @@ -964,7 +966,7 @@ class DatabasePool: table: str, keyvalues: Dict[str, Any], values: Dict[str, Any], - insertion_values: Dict[str, Any] = {}, + insertion_values: Optional[Dict[str, Any]] = None, lock: bool = True, ) -> Optional[bool]: """ @@ -982,6 +984,8 @@ class DatabasePool: Native upserts always return None. Emulated upserts return True if a new entry was created, False if an existing one was updated. """ + insertion_values = insertion_values or {} + if self.engine.can_native_upsert and table not in self._unsafe_to_upsert_tables: self.simple_upsert_txn_native_upsert( txn, table, keyvalues, values, insertion_values=insertion_values @@ -1003,7 +1007,7 @@ class DatabasePool: table: str, keyvalues: Dict[str, Any], values: Dict[str, Any], - insertion_values: Dict[str, Any] = {}, + insertion_values: Optional[Dict[str, Any]] = None, lock: bool = True, ) -> bool: """ @@ -1017,6 +1021,8 @@ class DatabasePool: Returns True if a new entry was created, False if an existing one was updated. """ + insertion_values = insertion_values or {} + # We need to lock the table :(, unless we're *really* careful if lock: self.engine.lock_table(txn, table) @@ -1077,7 +1083,7 @@ class DatabasePool: table: str, keyvalues: Dict[str, Any], values: Dict[str, Any], - insertion_values: Dict[str, Any] = {}, + insertion_values: Optional[Dict[str, Any]] = None, ) -> None: """ Use the native UPSERT functionality in recent PostgreSQL versions. @@ -1090,7 +1096,7 @@ class DatabasePool: """ allvalues = {} # type: Dict[str, Any] allvalues.update(keyvalues) - allvalues.update(insertion_values) + allvalues.update(insertion_values or {}) if not values: latter = "NOTHING" @@ -1513,7 +1519,7 @@ class DatabasePool: column: str, iterable: Iterable[Any], retcols: Iterable[str], - keyvalues: Dict[str, Any] = {}, + keyvalues: Optional[Dict[str, Any]] = None, desc: str = "simple_select_many_batch", batch_size: int = 100, ) -> List[Any]: @@ -1531,6 +1537,8 @@ class DatabasePool: desc: description of the transaction, for logging and metrics batch_size: the number of rows for each select query """ + keyvalues = keyvalues or {} + results = [] # type: List[Dict[str, Any]] if not iterable: diff --git a/synapse/storage/databases/main/events.py b/synapse/storage/databases/main/events.py index 98dac19a95..ad17123915 100644 --- a/synapse/storage/databases/main/events.py +++ b/synapse/storage/databases/main/events.py @@ -320,8 +320,8 @@ class PersistEventsStore: txn: LoggingTransaction, events_and_contexts: List[Tuple[EventBase, EventContext]], backfilled: bool, - state_delta_for_room: Dict[str, DeltaState] = {}, - new_forward_extremeties: Dict[str, List[str]] = {}, + state_delta_for_room: Optional[Dict[str, DeltaState]] = None, + new_forward_extremeties: Optional[Dict[str, List[str]]] = None, ): """Insert some number of room events into the necessary database tables. @@ -342,6 +342,9 @@ class PersistEventsStore: extremities. """ + state_delta_for_room = state_delta_for_room or {} + new_forward_extremeties = new_forward_extremeties or {} + all_events_and_contexts = events_and_contexts min_stream_order = events_and_contexts[0][0].internal_metadata.stream_ordering diff --git a/synapse/storage/databases/main/group_server.py b/synapse/storage/databases/main/group_server.py index 8f462dfc31..bd7826f4e9 100644 --- a/synapse/storage/databases/main/group_server.py +++ b/synapse/storage/databases/main/group_server.py @@ -1171,7 +1171,7 @@ class GroupServerStore(GroupServerWorkerStore): user_id: str, membership: str, is_admin: bool = False, - content: JsonDict = {}, + content: Optional[JsonDict] = None, local_attestation: Optional[dict] = None, remote_attestation: Optional[dict] = None, is_publicised: bool = False, @@ -1192,6 +1192,8 @@ class GroupServerStore(GroupServerWorkerStore): is_publicised: Whether this should be publicised. """ + content = content or {} + def _register_user_group_membership_txn(txn, next_id): # TODO: Upsert? self.db_pool.simple_delete_txn( diff --git a/synapse/storage/databases/main/state.py b/synapse/storage/databases/main/state.py index a7f371732f..93431efe00 100644 --- a/synapse/storage/databases/main/state.py +++ b/synapse/storage/databases/main/state.py @@ -190,7 +190,7 @@ class StateGroupWorkerStore(EventsWorkerStore, SQLBaseStore): # FIXME: how should this be cached? async def get_filtered_current_state_ids( - self, room_id: str, state_filter: StateFilter = StateFilter.all() + self, room_id: str, state_filter: Optional[StateFilter] = None ) -> StateMap[str]: """Get the current state event of a given type for a room based on the current_state_events table. This may not be as up-to-date as the result @@ -205,7 +205,9 @@ class StateGroupWorkerStore(EventsWorkerStore, SQLBaseStore): Map from type/state_key to event ID. """ - where_clause, where_args = state_filter.make_sql_filter_clause() + where_clause, where_args = ( + state_filter or StateFilter.all() + ).make_sql_filter_clause() if not where_clause: # We delegate to the cached version diff --git a/synapse/storage/databases/state/bg_updates.py b/synapse/storage/databases/state/bg_updates.py index 1fd333b707..75c09b3687 100644 --- a/synapse/storage/databases/state/bg_updates.py +++ b/synapse/storage/databases/state/bg_updates.py @@ -14,6 +14,7 @@ # limitations under the License. import logging +from typing import Optional from synapse.storage._base import SQLBaseStore from synapse.storage.database import DatabasePool @@ -73,8 +74,10 @@ class StateGroupBackgroundUpdateStore(SQLBaseStore): return count def _get_state_groups_from_groups_txn( - self, txn, groups, state_filter=StateFilter.all() + self, txn, groups, state_filter: Optional[StateFilter] = None ): + state_filter = state_filter or StateFilter.all() + results = {group: {} for group in groups} where_clause, where_args = state_filter.make_sql_filter_clause() diff --git a/synapse/storage/databases/state/store.py b/synapse/storage/databases/state/store.py index 97ec65f757..dfcf89d91c 100644 --- a/synapse/storage/databases/state/store.py +++ b/synapse/storage/databases/state/store.py @@ -15,7 +15,7 @@ import logging from collections import namedtuple -from typing import Dict, Iterable, List, Set, Tuple +from typing import Dict, Iterable, List, Optional, Set, Tuple from synapse.api.constants import EventTypes from synapse.storage._base import SQLBaseStore @@ -210,7 +210,7 @@ class StateGroupDataStore(StateBackgroundUpdateStore, SQLBaseStore): return state_filter.filter_state(state_dict_ids), not missing_types async def _get_state_for_groups( - self, groups: Iterable[int], state_filter: StateFilter = StateFilter.all() + self, groups: Iterable[int], state_filter: Optional[StateFilter] = None ) -> Dict[int, MutableStateMap[str]]: """Gets the state at each of a list of state groups, optionally filtering by type/state_key @@ -223,6 +223,7 @@ class StateGroupDataStore(StateBackgroundUpdateStore, SQLBaseStore): Returns: Dict of state group to state map. """ + state_filter = state_filter or StateFilter.all() member_filter, non_member_filter = state_filter.get_member_split() diff --git a/synapse/storage/state.py b/synapse/storage/state.py index 2e277a21c4..c1c147c62a 100644 --- a/synapse/storage/state.py +++ b/synapse/storage/state.py @@ -449,7 +449,7 @@ class StateGroupStorage: return self.stores.state._get_state_groups_from_groups(groups, state_filter) async def get_state_for_events( - self, event_ids: Iterable[str], state_filter: StateFilter = StateFilter.all() + self, event_ids: Iterable[str], state_filter: Optional[StateFilter] = None ) -> Dict[str, StateMap[EventBase]]: """Given a list of event_ids and type tuples, return a list of state dicts for each event. @@ -465,7 +465,7 @@ class StateGroupStorage: groups = set(event_to_groups.values()) group_to_state = await self.stores.state._get_state_for_groups( - groups, state_filter + groups, state_filter or StateFilter.all() ) state_event_map = await self.stores.main.get_events( @@ -485,7 +485,7 @@ class StateGroupStorage: return {event: event_to_state[event] for event in event_ids} async def get_state_ids_for_events( - self, event_ids: Iterable[str], state_filter: StateFilter = StateFilter.all() + self, event_ids: Iterable[str], state_filter: Optional[StateFilter] = None ) -> Dict[str, StateMap[str]]: """ Get the state dicts corresponding to a list of events, containing the event_ids @@ -502,7 +502,7 @@ class StateGroupStorage: groups = set(event_to_groups.values()) group_to_state = await self.stores.state._get_state_for_groups( - groups, state_filter + groups, state_filter or StateFilter.all() ) event_to_state = { @@ -513,7 +513,7 @@ class StateGroupStorage: return {event: event_to_state[event] for event in event_ids} async def get_state_for_event( - self, event_id: str, state_filter: StateFilter = StateFilter.all() + self, event_id: str, state_filter: Optional[StateFilter] = None ) -> StateMap[EventBase]: """ Get the state dict corresponding to a particular event @@ -525,11 +525,13 @@ class StateGroupStorage: Returns: A dict from (type, state_key) -> state_event """ - state_map = await self.get_state_for_events([event_id], state_filter) + state_map = await self.get_state_for_events( + [event_id], state_filter or StateFilter.all() + ) return state_map[event_id] async def get_state_ids_for_event( - self, event_id: str, state_filter: StateFilter = StateFilter.all() + self, event_id: str, state_filter: Optional[StateFilter] = None ) -> StateMap[str]: """ Get the state dict corresponding to a particular event @@ -541,11 +543,13 @@ class StateGroupStorage: Returns: A dict from (type, state_key) -> state_event """ - state_map = await self.get_state_ids_for_events([event_id], state_filter) + state_map = await self.get_state_ids_for_events( + [event_id], state_filter or StateFilter.all() + ) return state_map[event_id] def _get_state_for_groups( - self, groups: Iterable[int], state_filter: StateFilter = StateFilter.all() + self, groups: Iterable[int], state_filter: Optional[StateFilter] = None ) -> Awaitable[Dict[int, MutableStateMap[str]]]: """Gets the state at each of a list of state groups, optionally filtering by type/state_key @@ -558,7 +562,9 @@ class StateGroupStorage: Returns: Dict of state group to state map. """ - return self.stores.state._get_state_for_groups(groups, state_filter) + return self.stores.state._get_state_for_groups( + groups, state_filter or StateFilter.all() + ) async def store_state_group( self, diff --git a/synapse/storage/util/id_generators.py b/synapse/storage/util/id_generators.py index d4643c4fdf..32d6cc16b9 100644 --- a/synapse/storage/util/id_generators.py +++ b/synapse/storage/util/id_generators.py @@ -17,7 +17,7 @@ import logging import threading from collections import OrderedDict from contextlib import contextmanager -from typing import Dict, List, Optional, Set, Tuple, Union +from typing import Dict, Iterable, List, Optional, Set, Tuple, Union import attr @@ -91,7 +91,14 @@ class StreamIdGenerator: # ... persist event ... """ - def __init__(self, db_conn, table, column, extra_tables=[], step=1): + def __init__( + self, + db_conn, + table, + column, + extra_tables: Iterable[Tuple[str, str]] = (), + step=1, + ): assert step != 0 self._lock = threading.Lock() self._step = step diff --git a/synapse/util/caches/lrucache.py b/synapse/util/caches/lrucache.py index 60bb6ff642..20c8e2d9f5 100644 --- a/synapse/util/caches/lrucache.py +++ b/synapse/util/caches/lrucache.py @@ -57,12 +57,14 @@ def enumerate_leaves(node, depth): class _Node: __slots__ = ["prev_node", "next_node", "key", "value", "callbacks"] - def __init__(self, prev_node, next_node, key, value, callbacks=set()): + def __init__( + self, prev_node, next_node, key, value, callbacks: Optional[set] = None + ): self.prev_node = prev_node self.next_node = next_node self.key = key self.value = value - self.callbacks = callbacks + self.callbacks = callbacks or set() class LruCache(Generic[KT, VT]): @@ -176,10 +178,10 @@ class LruCache(Generic[KT, VT]): self.len = synchronized(cache_len) - def add_node(key, value, callbacks=set()): + def add_node(key, value, callbacks: Optional[set] = None): prev_node = list_root next_node = prev_node.next_node - node = _Node(prev_node, next_node, key, value, callbacks) + node = _Node(prev_node, next_node, key, value, callbacks or set()) prev_node.next_node = node next_node.prev_node = node cache[key] = node @@ -237,7 +239,7 @@ class LruCache(Generic[KT, VT]): def cache_get( key: KT, default: Optional[T] = None, - callbacks: Iterable[Callable[[], None]] = [], + callbacks: Iterable[Callable[[], None]] = (), update_metrics: bool = True, ): node = cache.get(key, None) @@ -253,7 +255,7 @@ class LruCache(Generic[KT, VT]): return default @synchronized - def cache_set(key: KT, value: VT, callbacks: Iterable[Callable[[], None]] = []): + def cache_set(key: KT, value: VT, callbacks: Iterable[Callable[[], None]] = ()): node = cache.get(key, None) if node is not None: # We sometimes store large objects, e.g. dicts, which cause diff --git a/tests/http/federation/test_matrix_federation_agent.py b/tests/http/federation/test_matrix_federation_agent.py index 4c56253da5..73e12ea6c3 100644 --- a/tests/http/federation/test_matrix_federation_agent.py +++ b/tests/http/federation/test_matrix_federation_agent.py @@ -13,6 +13,7 @@ # See the License for the specific language governing permissions and # limitations under the License. import logging +from typing import Optional from mock import Mock @@ -180,7 +181,11 @@ class MatrixFederationAgentTests(unittest.TestCase): _check_logcontext(context) def _handle_well_known_connection( - self, client_factory, expected_sni, content, response_headers={} + self, + client_factory, + expected_sni, + content, + response_headers: Optional[dict] = None, ): """Handle an outgoing HTTPs connection: wire it up to a server, check that the request is for a .well-known, and send the response. @@ -202,10 +207,12 @@ class MatrixFederationAgentTests(unittest.TestCase): self.assertEqual( request.requestHeaders.getRawHeaders(b"user-agent"), [b"test-agent"] ) - self._send_well_known_response(request, content, headers=response_headers) + self._send_well_known_response(request, content, headers=response_headers or {}) return well_known_server - def _send_well_known_response(self, request, content, headers={}): + def _send_well_known_response( + self, request, content, headers: Optional[dict] = None + ): """Check that an incoming request looks like a valid .well-known request, and send back the response. """ @@ -213,7 +220,7 @@ class MatrixFederationAgentTests(unittest.TestCase): self.assertEqual(request.path, b"/.well-known/matrix/server") self.assertEqual(request.requestHeaders.getRawHeaders(b"host"), [b"testserv"]) # send back a response - for k, v in headers.items(): + for k, v in (headers or {}).items(): request.setHeader(k, v) request.write(content) request.finish() diff --git a/tests/replication/_base.py b/tests/replication/_base.py index 1d4a592862..aff19d9fb3 100644 --- a/tests/replication/_base.py +++ b/tests/replication/_base.py @@ -266,7 +266,7 @@ class BaseMultiWorkerStreamTestCase(unittest.HomeserverTestCase): return resource def make_worker_hs( - self, worker_app: str, extra_config: dict = {}, **kwargs + self, worker_app: str, extra_config: Optional[dict] = None, **kwargs ) -> HomeServer: """Make a new worker HS instance, correctly connecting replcation stream to the master HS. @@ -283,7 +283,7 @@ class BaseMultiWorkerStreamTestCase(unittest.HomeserverTestCase): config = self._get_worker_hs_config() config["worker_app"] = worker_app - config.update(extra_config) + config.update(extra_config or {}) worker_hs = self.setup_test_homeserver( homeserver_to_use=GenericWorkerServer, diff --git a/tests/replication/slave/storage/test_events.py b/tests/replication/slave/storage/test_events.py index 0ceb0f935c..333374b183 100644 --- a/tests/replication/slave/storage/test_events.py +++ b/tests/replication/slave/storage/test_events.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. import logging +from typing import Iterable, Optional from canonicaljson import encode_canonical_json @@ -332,15 +333,18 @@ class SlavedEventStoreTestCase(BaseSlavedStoreTestCase): room_id=ROOM_ID, type="m.room.message", key=None, - internal={}, + internal: Optional[dict] = None, depth=None, - prev_events=[], - auth_events=[], - prev_state=[], + prev_events: Optional[list] = None, + auth_events: Optional[list] = None, + prev_state: Optional[list] = None, redacts=None, - push_actions=[], + push_actions: Iterable = frozenset(), **content ): + prev_events = prev_events or [] + auth_events = auth_events or [] + prev_state = prev_state or [] if depth is None: depth = self.event_id @@ -369,7 +373,7 @@ class SlavedEventStoreTestCase(BaseSlavedStoreTestCase): if redacts is not None: event_dict["redacts"] = redacts - event = make_event_from_dict(event_dict, internal_metadata_dict=internal) + event = make_event_from_dict(event_dict, internal_metadata_dict=internal or {}) self.event_id += 1 state_handler = self.hs.get_state_handler() diff --git a/tests/rest/client/v1/test_rooms.py b/tests/rest/client/v1/test_rooms.py index ed65f645fc..715414a310 100644 --- a/tests/rest/client/v1/test_rooms.py +++ b/tests/rest/client/v1/test_rooms.py @@ -19,6 +19,7 @@ """Tests REST events for /rooms paths.""" import json +from typing import Iterable from urllib import parse as urlparse from mock import Mock @@ -207,7 +208,9 @@ class RoomPermissionsTestCase(RoomBase): ) self.assertEquals(403, channel.code, msg=channel.result["body"]) - def _test_get_membership(self, room=None, members=[], expect_code=None): + def _test_get_membership( + self, room=None, members: Iterable = frozenset(), expect_code=None + ): for member in members: path = "/rooms/%s/state/m.room.member/%s" % (room, member) channel = self.make_request("GET", path) diff --git a/tests/rest/client/v1/utils.py b/tests/rest/client/v1/utils.py index 946740aa5d..8a4dddae2b 100644 --- a/tests/rest/client/v1/utils.py +++ b/tests/rest/client/v1/utils.py @@ -132,7 +132,7 @@ class RestHelper: src: str, targ: str, membership: str, - extra_data: dict = {}, + extra_data: Optional[dict] = None, tok: Optional[str] = None, expect_code: int = 200, ) -> None: @@ -156,7 +156,7 @@ class RestHelper: path = path + "?access_token=%s" % tok data = {"membership": membership} - data.update(extra_data) + data.update(extra_data or {}) channel = make_request( self.hs.get_reactor(), @@ -187,7 +187,13 @@ class RestHelper: ) def send_event( - self, room_id, type, content={}, txn_id=None, tok=None, expect_code=200 + self, + room_id, + type, + content: Optional[dict] = None, + txn_id=None, + tok=None, + expect_code=200, ): if txn_id is None: txn_id = "m%s" % (str(time.time())) @@ -201,7 +207,7 @@ class RestHelper: self.site, "PUT", path, - json.dumps(content).encode("utf8"), + json.dumps(content or {}).encode("utf8"), ) assert ( diff --git a/tests/rest/client/v2_alpha/test_relations.py b/tests/rest/client/v2_alpha/test_relations.py index e7bb5583fc..21ee436b91 100644 --- a/tests/rest/client/v2_alpha/test_relations.py +++ b/tests/rest/client/v2_alpha/test_relations.py @@ -16,6 +16,7 @@ import itertools import json import urllib +from typing import Optional from synapse.api.constants import EventTypes, RelationTypes from synapse.rest import admin @@ -681,7 +682,7 @@ class RelationsTestCase(unittest.HomeserverTestCase): relation_type, event_type, key=None, - content={}, + content: Optional[dict] = None, access_token=None, parent_id=None, ): @@ -713,7 +714,7 @@ class RelationsTestCase(unittest.HomeserverTestCase): "POST", "/_matrix/client/unstable/rooms/%s/send_relation/%s/%s/%s%s" % (self.room, original_id, relation_type, event_type, query), - json.dumps(content).encode("utf-8"), + json.dumps(content or {}).encode("utf-8"), access_token=access_token, ) return channel diff --git a/tests/storage/test_id_generators.py b/tests/storage/test_id_generators.py index aad6bc907e..6c389fe9ac 100644 --- a/tests/storage/test_id_generators.py +++ b/tests/storage/test_id_generators.py @@ -12,6 +12,8 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +from typing import List, Optional + from synapse.storage.database import DatabasePool from synapse.storage.engines import IncorrectDatabaseSetup from synapse.storage.util.id_generators import MultiWriterIdGenerator @@ -43,7 +45,7 @@ class MultiWriterIdGeneratorTestCase(HomeserverTestCase): ) def _create_id_generator( - self, instance_name="master", writers=["master"] + self, instance_name="master", writers: Optional[List[str]] = None ) -> MultiWriterIdGenerator: def _create(conn): return MultiWriterIdGenerator( @@ -53,7 +55,7 @@ class MultiWriterIdGeneratorTestCase(HomeserverTestCase): instance_name=instance_name, tables=[("foobar", "instance_name", "stream_id")], sequence_name="foobar_seq", - writers=writers, + writers=writers or ["master"], ) return self.get_success_or_raise(self.db_pool.runWithConnection(_create)) @@ -476,7 +478,7 @@ class BackwardsMultiWriterIdGeneratorTestCase(HomeserverTestCase): ) def _create_id_generator( - self, instance_name="master", writers=["master"] + self, instance_name="master", writers: Optional[List[str]] = None ) -> MultiWriterIdGenerator: def _create(conn): return MultiWriterIdGenerator( @@ -486,7 +488,7 @@ class BackwardsMultiWriterIdGeneratorTestCase(HomeserverTestCase): instance_name=instance_name, tables=[("foobar", "instance_name", "stream_id")], sequence_name="foobar_seq", - writers=writers, + writers=writers or ["master"], positive=False, ) @@ -612,7 +614,7 @@ class MultiTableMultiWriterIdGeneratorTestCase(HomeserverTestCase): ) def _create_id_generator( - self, instance_name="master", writers=["master"] + self, instance_name="master", writers: Optional[List[str]] = None ) -> MultiWriterIdGenerator: def _create(conn): return MultiWriterIdGenerator( @@ -625,7 +627,7 @@ class MultiTableMultiWriterIdGeneratorTestCase(HomeserverTestCase): ("foobar2", "instance_name", "stream_id"), ], sequence_name="foobar_seq", - writers=writers, + writers=writers or ["master"], ) return self.get_success_or_raise(self.db_pool.runWithConnection(_create)) diff --git a/tests/storage/test_redaction.py b/tests/storage/test_redaction.py index 2622207639..2d2f58903c 100644 --- a/tests/storage/test_redaction.py +++ b/tests/storage/test_redaction.py @@ -12,6 +12,7 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +from typing import Optional from canonicaljson import json @@ -47,10 +48,15 @@ class RedactionTestCase(unittest.HomeserverTestCase): self.depth = 1 def inject_room_member( - self, room, user, membership, replaces_state=None, extra_content={} + self, + room, + user, + membership, + replaces_state=None, + extra_content: Optional[dict] = None, ): content = {"membership": membership} - content.update(extra_content) + content.update(extra_content or {}) builder = self.event_builder_factory.for_room_version( RoomVersions.V1, { diff --git a/tests/test_state.py b/tests/test_state.py index 6227a3ba95..1d2019699d 100644 --- a/tests/test_state.py +++ b/tests/test_state.py @@ -12,6 +12,7 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +from typing import List, Optional from mock import Mock @@ -37,7 +38,7 @@ def create_event( state_key=None, depth=2, event_id=None, - prev_events=[], + prev_events: Optional[List[str]] = None, **kwargs ): global _next_event_id @@ -58,7 +59,7 @@ def create_event( "sender": "@user_id:example.com", "room_id": "!room_id:example.com", "depth": depth, - "prev_events": prev_events, + "prev_events": prev_events or [], } if state_key is not None: diff --git a/tests/test_visibility.py b/tests/test_visibility.py index 510b630114..1b4dd47a82 100644 --- a/tests/test_visibility.py +++ b/tests/test_visibility.py @@ -13,6 +13,7 @@ # See the License for the specific language governing permissions and # limitations under the License. import logging +from typing import Optional from mock import Mock @@ -147,9 +148,11 @@ class FilterEventsForServerTestCase(tests.unittest.TestCase): return event @defer.inlineCallbacks - def inject_room_member(self, user_id, membership="join", extra_content={}): + def inject_room_member( + self, user_id, membership="join", extra_content: Optional[dict] = None + ): content = {"membership": membership} - content.update(extra_content) + content.update(extra_content or {}) builder = self.event_builder_factory.for_room_version( RoomVersions.V1, { diff --git a/tests/util/test_ratelimitutils.py b/tests/util/test_ratelimitutils.py index 4d1aee91d5..3fed55090a 100644 --- a/tests/util/test_ratelimitutils.py +++ b/tests/util/test_ratelimitutils.py @@ -12,6 +12,8 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +from typing import Optional + from synapse.config.homeserver import HomeServerConfig from synapse.util.ratelimitutils import FederationRateLimiter @@ -89,9 +91,9 @@ def _await_resolution(reactor, d): return (reactor.seconds() - start_time) * 1000 -def build_rc_config(settings={}): +def build_rc_config(settings: Optional[dict] = None): config_dict = default_config("test") - config_dict.update(settings) + config_dict.update(settings or {}) config = HomeServerConfig() config.parse_config_dict(config_dict, "", "") return config.rc_federation -- cgit 1.5.1 From 1d5f0e3529ec5acd889037c8ebcca2820ad003d5 Mon Sep 17 00:00:00 2001 From: Dan Callahan Date: Tue, 13 Apr 2021 10:41:34 +0100 Subject: Bump black configuration to target py36 (#9781) Signed-off-by: Dan Callahan --- changelog.d/9781.misc | 1 + pyproject.toml | 2 +- synapse/config/tls.py | 2 +- synapse/handlers/presence.py | 2 +- synapse/http/matrixfederationclient.py | 2 +- synapse/http/site.py | 2 +- synapse/storage/database.py | 8 ++++---- tests/replication/slave/storage/test_events.py | 2 +- tests/test_state.py | 2 +- tests/test_utils/event_injection.py | 6 +++--- tests/utils.py | 2 +- 11 files changed, 16 insertions(+), 15 deletions(-) create mode 100644 changelog.d/9781.misc (limited to 'synapse/http') diff --git a/changelog.d/9781.misc b/changelog.d/9781.misc new file mode 100644 index 0000000000..d1c73fc741 --- /dev/null +++ b/changelog.d/9781.misc @@ -0,0 +1 @@ +Update Black configuration to target Python 3.6. diff --git a/pyproject.toml b/pyproject.toml index cd880d4e39..8bca1fa4ef 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -35,7 +35,7 @@ showcontent = true [tool.black] -target-version = ['py35'] +target-version = ['py36'] exclude = ''' ( diff --git a/synapse/config/tls.py b/synapse/config/tls.py index ad37b93c02..85b5db4c40 100644 --- a/synapse/config/tls.py +++ b/synapse/config/tls.py @@ -270,7 +270,7 @@ class TlsConfig(Config): tls_certificate_path, tls_private_key_path, acme_domain, - **kwargs + **kwargs, ): """If the acme_domain is specified acme will be enabled. If the TLS paths are not specified the default will be certs in the diff --git a/synapse/handlers/presence.py b/synapse/handlers/presence.py index c817f2952d..0047907cd9 100644 --- a/synapse/handlers/presence.py +++ b/synapse/handlers/presence.py @@ -1071,7 +1071,7 @@ class PresenceEventSource: room_ids=None, include_offline=True, explicit_room_id=None, - **kwargs + **kwargs, ) -> Tuple[List[UserPresenceState], int]: # The process for getting presence events are: # 1. Get the rooms the user is in. diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py index 5f01ebd3d4..ab47dec8f2 100644 --- a/synapse/http/matrixfederationclient.py +++ b/synapse/http/matrixfederationclient.py @@ -272,7 +272,7 @@ class MatrixFederationHttpClient: self, request: MatrixFederationRequest, try_trailing_slash_on_400: bool = False, - **send_request_args + **send_request_args, ) -> IResponse: """Wrapper for _send_request which can optionally retry the request upon receiving a combination of a 400 HTTP response code and a diff --git a/synapse/http/site.py b/synapse/http/site.py index c0c873ce32..32b5e19c09 100644 --- a/synapse/http/site.py +++ b/synapse/http/site.py @@ -497,7 +497,7 @@ class SynapseSite(Site): resource, server_version_string, *args, - **kwargs + **kwargs, ): Site.__init__(self, resource, *args, **kwargs) diff --git a/synapse/storage/database.py b/synapse/storage/database.py index fa15b0ce5b..77ef29ec71 100644 --- a/synapse/storage/database.py +++ b/synapse/storage/database.py @@ -488,7 +488,7 @@ class DatabasePool: exception_callbacks: List[_CallbackListEntry], func: "Callable[..., R]", *args: Any, - **kwargs: Any + **kwargs: Any, ) -> R: """Start a new database transaction with the given connection. @@ -622,7 +622,7 @@ class DatabasePool: func: "Callable[..., R]", *args: Any, db_autocommit: bool = False, - **kwargs: Any + **kwargs: Any, ) -> R: """Starts a transaction on the database and runs a given function @@ -682,7 +682,7 @@ class DatabasePool: func: "Callable[..., R]", *args: Any, db_autocommit: bool = False, - **kwargs: Any + **kwargs: Any, ) -> R: """Wraps the .runWithConnection() method on the underlying db_pool. @@ -775,7 +775,7 @@ class DatabasePool: desc: str, decoder: Optional[Callable[[Cursor], R]], query: str, - *args: Any + *args: Any, ) -> R: """Runs a single query for a result set. diff --git a/tests/replication/slave/storage/test_events.py b/tests/replication/slave/storage/test_events.py index 333374b183..db80a0bdbd 100644 --- a/tests/replication/slave/storage/test_events.py +++ b/tests/replication/slave/storage/test_events.py @@ -340,7 +340,7 @@ class SlavedEventStoreTestCase(BaseSlavedStoreTestCase): prev_state: Optional[list] = None, redacts=None, push_actions: Iterable = frozenset(), - **content + **content, ): prev_events = prev_events or [] auth_events = auth_events or [] diff --git a/tests/test_state.py b/tests/test_state.py index 83383d8872..0d626f49f6 100644 --- a/tests/test_state.py +++ b/tests/test_state.py @@ -38,7 +38,7 @@ def create_event( depth=2, event_id=None, prev_events: Optional[List[str]] = None, - **kwargs + **kwargs, ): global _next_event_id diff --git a/tests/test_utils/event_injection.py b/tests/test_utils/event_injection.py index c3c4a93e1f..3dfbf8f8a9 100644 --- a/tests/test_utils/event_injection.py +++ b/tests/test_utils/event_injection.py @@ -33,7 +33,7 @@ async def inject_member_event( membership: str, target: Optional[str] = None, extra_content: Optional[dict] = None, - **kwargs + **kwargs, ) -> EventBase: """Inject a membership event into a room.""" if target is None: @@ -58,7 +58,7 @@ async def inject_event( hs: synapse.server.HomeServer, room_version: Optional[str] = None, prev_event_ids: Optional[List[str]] = None, - **kwargs + **kwargs, ) -> EventBase: """Inject a generic event into a room @@ -83,7 +83,7 @@ async def create_event( hs: synapse.server.HomeServer, room_version: Optional[str] = None, prev_event_ids: Optional[List[str]] = None, - **kwargs + **kwargs, ) -> Tuple[EventBase, EventContext]: if room_version is None: room_version = await hs.get_datastore().get_room_version_id(kwargs["room_id"]) diff --git a/tests/utils.py b/tests/utils.py index 2e34fad11c..c78d3e5ba7 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -190,7 +190,7 @@ def setup_test_homeserver( config=None, reactor=None, homeserver_to_use: Type[HomeServer] = TestHomeServer, - **kwargs + **kwargs, ): """ Setup a homeserver suitable for running tests against. Keyword arguments -- cgit 1.5.1