From ae6ad4cf4190dbd2dadd72fc8131e4c139f8eb4a Mon Sep 17 00:00:00 2001
From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
Date: Thu, 27 Sep 2018 11:22:25 +0100
Subject: docstrings and unittests for storage.state (#3958)

I spent ages trying to figure out how I was going mad...
---
 tests/storage/test_state.py | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

(limited to 'tests')

diff --git a/tests/storage/test_state.py b/tests/storage/test_state.py
index b910965932..b9c5b39d59 100644
--- a/tests/storage/test_state.py
+++ b/tests/storage/test_state.py
@@ -74,6 +74,45 @@ class StateStoreTestCase(tests.unittest.TestCase):
             self.assertEqual(s1[t].event_id, s2[t].event_id)
         self.assertEqual(len(s1), len(s2))
 
+    @defer.inlineCallbacks
+    def test_get_state_groups_ids(self):
+        e1 = yield self.inject_state_event(
+            self.room, self.u_alice, EventTypes.Create, '', {}
+        )
+        e2 = yield self.inject_state_event(
+            self.room, self.u_alice, EventTypes.Name, '', {"name": "test room"}
+        )
+
+        state_group_map = yield self.store.get_state_groups_ids(self.room, [e2.event_id])
+        self.assertEqual(len(state_group_map), 1)
+        state_map = list(state_group_map.values())[0]
+        self.assertDictEqual(
+            state_map,
+            {
+                (EventTypes.Create, ''): e1.event_id,
+                (EventTypes.Name, ''): e2.event_id,
+            },
+        )
+
+    @defer.inlineCallbacks
+    def test_get_state_groups(self):
+        e1 = yield self.inject_state_event(
+            self.room, self.u_alice, EventTypes.Create, '', {}
+        )
+        e2 = yield self.inject_state_event(
+            self.room, self.u_alice, EventTypes.Name, '', {"name": "test room"}
+        )
+
+        state_group_map = yield self.store.get_state_groups(
+            self.room, [e2.event_id])
+        self.assertEqual(len(state_group_map), 1)
+        state_list = list(state_group_map.values())[0]
+
+        self.assertEqual(
+            {ev.event_id for ev in state_list},
+            {e1.event_id, e2.event_id},
+        )
+
     @defer.inlineCallbacks
     def test_get_state_for_event(self):
 
-- 
cgit 1.5.1


From 4a15a3e4d539dcea9a4a57e7cd800a926f2a17c3 Mon Sep 17 00:00:00 2001
From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
Date: Thu, 27 Sep 2018 11:25:34 +0100
Subject: Include eventid in log lines when processing incoming federation
 transactions (#3959)

when processing incoming transactions, it can be hard to see what's going on,
because we process a bunch of stuff in parallel, and because we may end up
recursively working our way through a chain of three or four events.

This commit creates a way to use logcontexts to add the relevant event ids to
the log lines.
---
 changelog.d/3959.feature                |  1 +
 synapse/federation/federation_server.py | 32 ++++++++--------
 synapse/handlers/federation.py          | 65 ++++++++++++++++++++-------------
 synapse/util/logcontext.py              | 41 +++++++++++++++++++--
 tests/test_federation.py                | 28 ++++++++------
 tests/util/test_logcontext.py           |  5 +++
 6 files changed, 115 insertions(+), 57 deletions(-)
 create mode 100644 changelog.d/3959.feature

(limited to 'tests')

diff --git a/changelog.d/3959.feature b/changelog.d/3959.feature
new file mode 100644
index 0000000000..b3a4f37a8d
--- /dev/null
+++ b/changelog.d/3959.feature
@@ -0,0 +1 @@
+Include eventid in log lines when processing incoming federation transactions
\ No newline at end of file
diff --git a/synapse/federation/federation_server.py b/synapse/federation/federation_server.py
index 9a571e4fc7..819e8f7331 100644
--- a/synapse/federation/federation_server.py
+++ b/synapse/federation/federation_server.py
@@ -46,6 +46,7 @@ from synapse.replication.http.federation import (
 from synapse.types import get_domain_from_id
 from synapse.util.async_helpers import Linearizer, concurrently_execute
 from synapse.util.caches.response_cache import ResponseCache
+from synapse.util.logcontext import nested_logging_context
 from synapse.util.logutils import log_function
 
 # when processing incoming transactions, we try to handle multiple rooms in
@@ -187,21 +188,22 @@ class FederationServer(FederationBase):
 
             for pdu in pdus_by_room[room_id]:
                 event_id = pdu.event_id
-                try:
-                    yield self._handle_received_pdu(
-                        origin, pdu
-                    )
-                    pdu_results[event_id] = {}
-                except FederationError as e:
-                    logger.warn("Error handling PDU %s: %s", event_id, e)
-                    pdu_results[event_id] = {"error": str(e)}
-                except Exception as e:
-                    f = failure.Failure()
-                    pdu_results[event_id] = {"error": str(e)}
-                    logger.error(
-                        "Failed to handle PDU %s: %s",
-                        event_id, f.getTraceback().rstrip(),
-                    )
+                with nested_logging_context(event_id):
+                    try:
+                        yield self._handle_received_pdu(
+                            origin, pdu
+                        )
+                        pdu_results[event_id] = {}
+                    except FederationError as e:
+                        logger.warn("Error handling PDU %s: %s", event_id, e)
+                        pdu_results[event_id] = {"error": str(e)}
+                    except Exception as e:
+                        f = failure.Failure()
+                        pdu_results[event_id] = {"error": str(e)}
+                        logger.error(
+                            "Failed to handle PDU %s: %s",
+                            event_id, f.getTraceback().rstrip(),
+                        )
 
         yield concurrently_execute(
             process_pdus_for_room, pdus_by_room.keys(),
diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py
index 2ccdc3bfa7..993546387c 100644
--- a/synapse/handlers/federation.py
+++ b/synapse/handlers/federation.py
@@ -339,14 +339,18 @@ class FederationHandler(BaseHandler):
                             "[%s %s] Requesting state at missing prev_event %s",
                             room_id, event_id, p,
                         )
-                        state, got_auth_chain = (
-                            yield self.federation_client.get_state_for_room(
-                                origin, room_id, p,
+
+                        with logcontext.nested_logging_context(p):
+                            state, got_auth_chain = (
+                                yield self.federation_client.get_state_for_room(
+                                    origin, room_id, p,
+                                )
                             )
-                        )
-                        auth_chains.update(got_auth_chain)
-                        state_group = {(x.type, x.state_key): x.event_id for x in state}
-                        state_groups.append(state_group)
+                            auth_chains.update(got_auth_chain)
+                            state_group = {
+                                (x.type, x.state_key): x.event_id for x in state
+                            }
+                            state_groups.append(state_group)
 
                     # Resolve any conflicting state
                     def fetch(ev_ids):
@@ -483,20 +487,21 @@ class FederationHandler(BaseHandler):
                 "[%s %s] Handling received prev_event %s",
                 room_id, event_id, ev.event_id,
             )
-            try:
-                yield self.on_receive_pdu(
-                    origin,
-                    ev,
-                    sent_to_us_directly=False,
-                )
-            except FederationError as e:
-                if e.code == 403:
-                    logger.warn(
-                        "[%s %s] Received prev_event %s failed history check.",
-                        room_id, event_id, ev.event_id,
+            with logcontext.nested_logging_context(ev.event_id):
+                try:
+                    yield self.on_receive_pdu(
+                        origin,
+                        ev,
+                        sent_to_us_directly=False,
                     )
-                else:
-                    raise
+                except FederationError as e:
+                    if e.code == 403:
+                        logger.warn(
+                            "[%s %s] Received prev_event %s failed history check.",
+                            room_id, event_id, ev.event_id,
+                        )
+                    else:
+                        raise
 
     @defer.inlineCallbacks
     def _process_received_pdu(self, origin, event, state, auth_chain):
@@ -1135,7 +1140,8 @@ class FederationHandler(BaseHandler):
             try:
                 logger.info("Processing queued PDU %s which was received "
                             "while we were joining %s", p.event_id, p.room_id)
-                yield self.on_receive_pdu(origin, p, sent_to_us_directly=True)
+                with logcontext.nested_logging_context(p.event_id):
+                    yield self.on_receive_pdu(origin, p, sent_to_us_directly=True)
             except Exception as e:
                 logger.warn(
                     "Error handling queued PDU %s from %s: %s",
@@ -1581,15 +1587,22 @@ class FederationHandler(BaseHandler):
 
         Notifies about the events where appropriate.
         """
-        contexts = yield logcontext.make_deferred_yieldable(defer.gatherResults(
-            [
-                logcontext.run_in_background(
-                    self._prep_event,
+
+        @defer.inlineCallbacks
+        def prep(ev_info):
+            event = ev_info["event"]
+            with logcontext.nested_logging_context(suffix=event.event_id):
+                res = yield self._prep_event(
                     origin,
-                    ev_info["event"],
+                    event,
                     state=ev_info.get("state"),
                     auth_events=ev_info.get("auth_events"),
                 )
+            defer.returnValue(res)
+
+        contexts = yield logcontext.make_deferred_yieldable(defer.gatherResults(
+            [
+                logcontext.run_in_background(prep, ev_info)
                 for ev_info in event_infos
             ], consumeErrors=True,
         ))
diff --git a/synapse/util/logcontext.py b/synapse/util/logcontext.py
index a0c2d37610..89224b26cc 100644
--- a/synapse/util/logcontext.py
+++ b/synapse/util/logcontext.py
@@ -200,7 +200,7 @@ class LoggingContext(object):
 
     sentinel = Sentinel()
 
-    def __init__(self, name=None, parent_context=None):
+    def __init__(self, name=None, parent_context=None, request=None):
         self.previous_context = LoggingContext.current_context()
         self.name = name
 
@@ -218,6 +218,13 @@ class LoggingContext(object):
 
         self.parent_context = parent_context
 
+        if self.parent_context is not None:
+            self.parent_context.copy_to(self)
+
+        if request is not None:
+            # the request param overrides the request from the parent context
+            self.request = request
+
     def __str__(self):
         return "%s@%x" % (self.name, id(self))
 
@@ -256,9 +263,6 @@ class LoggingContext(object):
             )
         self.alive = True
 
-        if self.parent_context is not None:
-            self.parent_context.copy_to(self)
-
         return self
 
     def __exit__(self, type, value, traceback):
@@ -439,6 +443,35 @@ class PreserveLoggingContext(object):
                 )
 
 
+def nested_logging_context(suffix, parent_context=None):
+    """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.
+
+    CPU/db usage stats will be added to the parent context's on exit.
+
+    Normal usage looks like:
+
+        with nested_logging_context(suffix):
+            # ... do stuff
+
+    Args:
+        suffix (str): suffix to add to the parent context's 'request'.
+        parent_context (LoggingContext|None): parent context. Will use the current context
+            if None.
+
+    Returns:
+        LoggingContext: new logging context.
+    """
+    if parent_context is None:
+        parent_context = LoggingContext.current_context()
+    return LoggingContext(
+        parent_context=parent_context,
+        request=parent_context.request + "-" + suffix,
+    )
+
+
 def preserve_fn(f):
     """Function decorator which wraps the function with run_in_background"""
     def g(*args, **kwargs):
diff --git a/tests/test_federation.py b/tests/test_federation.py
index 2540604fcc..ff55c7a627 100644
--- a/tests/test_federation.py
+++ b/tests/test_federation.py
@@ -6,6 +6,7 @@ from twisted.internet.defer import maybeDeferred, succeed
 from synapse.events import FrozenEvent
 from synapse.types import Requester, UserID
 from synapse.util import Clock
+from synapse.util.logcontext import LoggingContext
 
 from tests import unittest
 from tests.server import ThreadedMemoryReactorClock, setup_test_homeserver
@@ -117,9 +118,10 @@ class MessageAcceptTests(unittest.TestCase):
             }
         )
 
-        d = self.handler.on_receive_pdu(
-            "test.serv", lying_event, sent_to_us_directly=True
-        )
+        with LoggingContext(request="lying_event"):
+            d = self.handler.on_receive_pdu(
+                "test.serv", lying_event, sent_to_us_directly=True
+            )
 
         # Step the reactor, so the database fetches come back
         self.reactor.advance(1)
@@ -209,11 +211,12 @@ class MessageAcceptTests(unittest.TestCase):
             }
         )
 
-        d = self.handler.on_receive_pdu(
-            "test.serv", good_event, sent_to_us_directly=True
-        )
-        self.reactor.advance(1)
-        self.assertEqual(self.successResultOf(d), None)
+        with LoggingContext(request="good_event"):
+            d = self.handler.on_receive_pdu(
+                "test.serv", good_event, sent_to_us_directly=True
+            )
+            self.reactor.advance(1)
+            self.assertEqual(self.successResultOf(d), None)
 
         bad_event = FrozenEvent(
             {
@@ -230,10 +233,11 @@ class MessageAcceptTests(unittest.TestCase):
             }
         )
 
-        d = self.handler.on_receive_pdu(
-            "test.serv", bad_event, sent_to_us_directly=True
-        )
-        self.reactor.advance(1)
+        with LoggingContext(request="bad_event"):
+            d = self.handler.on_receive_pdu(
+                "test.serv", bad_event, sent_to_us_directly=True
+            )
+            self.reactor.advance(1)
 
         extrem = maybeDeferred(
             self.homeserver.datastore.get_latest_event_ids_in_room, self.room_id
diff --git a/tests/util/test_logcontext.py b/tests/util/test_logcontext.py
index 4633db77b3..8adaee3c8d 100644
--- a/tests/util/test_logcontext.py
+++ b/tests/util/test_logcontext.py
@@ -159,6 +159,11 @@ class LoggingContextTestCase(unittest.TestCase):
             self.assertEqual(r, "bum")
             self._check_test_key("one")
 
+    def test_nested_logging_context(self):
+        with LoggingContext(request="foo"):
+            nested_context = logcontext.nested_logging_context(suffix="bar")
+            self.assertEqual(nested_context.request, "foo-bar")
+
 
 # a function which returns a deferred which has been "called", but
 # which had a function which returned another incomplete deferred on
-- 
cgit 1.5.1


From dc5db01ff25dac6ec74ceea9b4d815a8c43cd7dd Mon Sep 17 00:00:00 2001
From: Schnuffle <schnuffle@gotec-cafe.de>
Date: Thu, 27 Sep 2018 13:38:50 +0200
Subject: Replaced all occurences of e.message with str(e) Signed-off-by:
 Schnuffle  <schnuffle@github.com>

---
 scripts-dev/dump_macaroon.py     | 2 +-
 synapse/api/filtering.py         | 2 +-
 synapse/app/__init__.py          | 2 +-
 synapse/app/appservice.py        | 2 +-
 synapse/app/client_reader.py     | 2 +-
 synapse/app/event_creator.py     | 2 +-
 synapse/app/federation_reader.py | 2 +-
 synapse/app/federation_sender.py | 2 +-
 synapse/app/frontend_proxy.py    | 2 +-
 synapse/app/homeserver.py        | 4 ++--
 synapse/app/media_repository.py  | 2 +-
 synapse/app/pusher.py            | 2 +-
 synapse/app/synchrotron.py       | 2 +-
 synapse/app/user_dir.py          | 2 +-
 synapse/config/__main__.py       | 2 +-
 synapse/handlers/e2e_keys.py     | 2 +-
 synapse/handlers/profile.py      | 2 +-
 tests/unittest.py                | 2 +-
 18 files changed, 19 insertions(+), 19 deletions(-)

(limited to 'tests')

diff --git a/scripts-dev/dump_macaroon.py b/scripts-dev/dump_macaroon.py
index 6e45be75d6..fcc5568835 100755
--- a/scripts-dev/dump_macaroon.py
+++ b/scripts-dev/dump_macaroon.py
@@ -21,4 +21,4 @@ try:
     verifier.verify(macaroon, key)
     print "Signature is correct"
 except Exception as e:
-    print e.message
+    print str(e)
diff --git a/synapse/api/filtering.py b/synapse/api/filtering.py
index a31a9a17e0..eed8c67e6a 100644
--- a/synapse/api/filtering.py
+++ b/synapse/api/filtering.py
@@ -226,7 +226,7 @@ class Filtering(object):
             jsonschema.validate(user_filter_json, USER_FILTER_SCHEMA,
                                 format_checker=FormatChecker())
         except jsonschema.ValidationError as e:
-            raise SynapseError(400, e.message)
+            raise SynapseError(400, str(e))
 
 
 class FilterCollection(object):
diff --git a/synapse/app/__init__.py b/synapse/app/__init__.py
index 3b6b9368b8..c3afcc573b 100644
--- a/synapse/app/__init__.py
+++ b/synapse/app/__init__.py
@@ -24,7 +24,7 @@ try:
     python_dependencies.check_requirements()
 except python_dependencies.MissingRequirementError as e:
     message = "\n".join([
-        "Missing Requirement: %s" % (e.message,),
+        "Missing Requirement: %s" % (str(e),),
         "To install run:",
         "    pip install --upgrade --force \"%s\"" % (e.dependency,),
         "",
diff --git a/synapse/app/appservice.py b/synapse/app/appservice.py
index 02039f7e79..8559e141af 100644
--- a/synapse/app/appservice.py
+++ b/synapse/app/appservice.py
@@ -136,7 +136,7 @@ def start(config_options):
             "Synapse appservice", config_options
         )
     except ConfigError as e:
-        sys.stderr.write("\n" + e.message + "\n")
+        sys.stderr.write("\n" + str(e) + "\n")
         sys.exit(1)
 
     assert config.worker_app == "synapse.app.appservice"
diff --git a/synapse/app/client_reader.py b/synapse/app/client_reader.py
index 4c73c637bb..76aed8c60a 100644
--- a/synapse/app/client_reader.py
+++ b/synapse/app/client_reader.py
@@ -153,7 +153,7 @@ def start(config_options):
             "Synapse client reader", config_options
         )
     except ConfigError as e:
-        sys.stderr.write("\n" + e.message + "\n")
+        sys.stderr.write("\n" + str(e) + "\n")
         sys.exit(1)
 
     assert config.worker_app == "synapse.app.client_reader"
diff --git a/synapse/app/event_creator.py b/synapse/app/event_creator.py
index bc82197b2a..9060ab14f6 100644
--- a/synapse/app/event_creator.py
+++ b/synapse/app/event_creator.py
@@ -169,7 +169,7 @@ def start(config_options):
             "Synapse event creator", config_options
         )
     except ConfigError as e:
-        sys.stderr.write("\n" + e.message + "\n")
+        sys.stderr.write("\n" + str(e) + "\n")
         sys.exit(1)
 
     assert config.worker_app == "synapse.app.event_creator"
diff --git a/synapse/app/federation_reader.py b/synapse/app/federation_reader.py
index 18ca71ef99..228a297fb8 100644
--- a/synapse/app/federation_reader.py
+++ b/synapse/app/federation_reader.py
@@ -140,7 +140,7 @@ def start(config_options):
             "Synapse federation reader", config_options
         )
     except ConfigError as e:
-        sys.stderr.write("\n" + e.message + "\n")
+        sys.stderr.write("\n" + str(e) + "\n")
         sys.exit(1)
 
     assert config.worker_app == "synapse.app.federation_reader"
diff --git a/synapse/app/federation_sender.py b/synapse/app/federation_sender.py
index 6501c57792..e9a99d76e1 100644
--- a/synapse/app/federation_sender.py
+++ b/synapse/app/federation_sender.py
@@ -160,7 +160,7 @@ def start(config_options):
             "Synapse federation sender", config_options
         )
     except ConfigError as e:
-        sys.stderr.write("\n" + e.message + "\n")
+        sys.stderr.write("\n" + str(e) + "\n")
         sys.exit(1)
 
     assert config.worker_app == "synapse.app.federation_sender"
diff --git a/synapse/app/frontend_proxy.py b/synapse/app/frontend_proxy.py
index b076fbe522..fc4b25de1c 100644
--- a/synapse/app/frontend_proxy.py
+++ b/synapse/app/frontend_proxy.py
@@ -228,7 +228,7 @@ def start(config_options):
             "Synapse frontend proxy", config_options
         )
     except ConfigError as e:
-        sys.stderr.write("\n" + e.message + "\n")
+        sys.stderr.write("\n" + str(e) + "\n")
         sys.exit(1)
 
     assert config.worker_app == "synapse.app.frontend_proxy"
diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py
index 8c5d858b0b..a98fdbd210 100755
--- a/synapse/app/homeserver.py
+++ b/synapse/app/homeserver.py
@@ -301,7 +301,7 @@ class SynapseHomeServer(HomeServer):
         try:
             database_engine.check_database(db_conn.cursor())
         except IncorrectDatabaseSetup as e:
-            quit_with_error(e.message)
+            quit_with_error(str(e))
 
 
 # Gauges to expose monthly active user control metrics
@@ -328,7 +328,7 @@ def setup(config_options):
             config_options,
         )
     except ConfigError as e:
-        sys.stderr.write("\n" + e.message + "\n")
+        sys.stderr.write("\n" + str(e) + "\n")
         sys.exit(1)
 
     if not config:
diff --git a/synapse/app/media_repository.py b/synapse/app/media_repository.py
index 992d182dba..acc0487adc 100644
--- a/synapse/app/media_repository.py
+++ b/synapse/app/media_repository.py
@@ -133,7 +133,7 @@ def start(config_options):
             "Synapse media repository", config_options
         )
     except ConfigError as e:
-        sys.stderr.write("\n" + e.message + "\n")
+        sys.stderr.write("\n" + str(e) + "\n")
         sys.exit(1)
 
     assert config.worker_app == "synapse.app.media_repository"
diff --git a/synapse/app/pusher.py b/synapse/app/pusher.py
index 2ec4c7defb..630dcda478 100644
--- a/synapse/app/pusher.py
+++ b/synapse/app/pusher.py
@@ -191,7 +191,7 @@ def start(config_options):
             "Synapse pusher", config_options
         )
     except ConfigError as e:
-        sys.stderr.write("\n" + e.message + "\n")
+        sys.stderr.write("\n" + str(e) + "\n")
         sys.exit(1)
 
     assert config.worker_app == "synapse.app.pusher"
diff --git a/synapse/app/synchrotron.py b/synapse/app/synchrotron.py
index df81b7bcbe..9a7fc6ee9d 100644
--- a/synapse/app/synchrotron.py
+++ b/synapse/app/synchrotron.py
@@ -410,7 +410,7 @@ def start(config_options):
             "Synapse synchrotron", config_options
         )
     except ConfigError as e:
-        sys.stderr.write("\n" + e.message + "\n")
+        sys.stderr.write("\n" + str(e) + "\n")
         sys.exit(1)
 
     assert config.worker_app == "synapse.app.synchrotron"
diff --git a/synapse/app/user_dir.py b/synapse/app/user_dir.py
index b383e79c1c..0a5f62b509 100644
--- a/synapse/app/user_dir.py
+++ b/synapse/app/user_dir.py
@@ -188,7 +188,7 @@ def start(config_options):
             "Synapse user directory", config_options
         )
     except ConfigError as e:
-        sys.stderr.write("\n" + e.message + "\n")
+        sys.stderr.write("\n" + str(e) + "\n")
         sys.exit(1)
 
     assert config.worker_app == "synapse.app.user_dir"
diff --git a/synapse/config/__main__.py b/synapse/config/__main__.py
index 58c97a70af..8fccf573ee 100644
--- a/synapse/config/__main__.py
+++ b/synapse/config/__main__.py
@@ -25,7 +25,7 @@ if __name__ == "__main__":
         try:
             config = HomeServerConfig.load_config("", sys.argv[3:])
         except ConfigError as e:
-            sys.stderr.write("\n" + e.message + "\n")
+            sys.stderr.write("\n" + str(e) + "\n")
             sys.exit(1)
 
         print (getattr(config, key))
diff --git a/synapse/handlers/e2e_keys.py b/synapse/handlers/e2e_keys.py
index 578e9250fb..9dc46aa15f 100644
--- a/synapse/handlers/e2e_keys.py
+++ b/synapse/handlers/e2e_keys.py
@@ -341,7 +341,7 @@ class E2eKeysHandler(object):
 def _exception_to_failure(e):
     if isinstance(e, CodeMessageException):
         return {
-            "status": e.code, "message": e.message,
+            "status": e.code, "message": str(e),
         }
 
     if isinstance(e, NotRetryingDestination):
diff --git a/synapse/handlers/profile.py b/synapse/handlers/profile.py
index 75b8b7ce6a..f284d5a385 100644
--- a/synapse/handlers/profile.py
+++ b/synapse/handlers/profile.py
@@ -278,7 +278,7 @@ class BaseProfileHandler(BaseHandler):
             except Exception as e:
                 logger.warn(
                     "Failed to update join event for room %s - %s",
-                    room_id, str(e.message)
+                    room_id, str(e)
                 )
 
 
diff --git a/tests/unittest.py b/tests/unittest.py
index ef905e6389..043710afaf 100644
--- a/tests/unittest.py
+++ b/tests/unittest.py
@@ -121,7 +121,7 @@ class TestCase(unittest.TestCase):
             try:
                 self.assertEquals(attrs[key], getattr(obj, key))
             except AssertionError as e:
-                raise (type(e))(e.message + " for '.%s'" % key)
+                raise (type(e))(str(e) + " for '.%s'" % key)
 
     def assert_dict(self, required, actual):
         """Does a partial assert of a dict.
-- 
cgit 1.5.1


From 484a9b8c81a2315a05a00c7094b7f5fb4a9d5794 Mon Sep 17 00:00:00 2001
From: Richard van der Hoff <richard@matrix.org>
Date: Thu, 27 Sep 2018 12:48:06 +0100
Subject: Remove redundant, failing, test

This test didn't do what it claimed to do, and what it claimed to do was the
same as test_cant_hide_direct_ancestors anyway.

This stuff is tested by sytest anyway.
---
 tests/test_federation.py | 106 -----------------------------------------------
 1 file changed, 106 deletions(-)

(limited to 'tests')

diff --git a/tests/test_federation.py b/tests/test_federation.py
index ff55c7a627..952a0a7b51 100644
--- a/tests/test_federation.py
+++ b/tests/test_federation.py
@@ -141,109 +141,3 @@ class MessageAcceptTests(unittest.TestCase):
             self.homeserver.datastore.get_latest_event_ids_in_room, self.room_id
         )
         self.assertEqual(self.successResultOf(extrem)[0], "$join:test.serv")
-
-    def test_cant_hide_past_history(self):
-        """
-        If you send a message, you must be able to provide the direct
-        prev_events that said event references.
-        """
-
-        def post_json(destination, path, data, headers=None, timeout=0):
-            if path.startswith("/_matrix/federation/v1/get_missing_events/"):
-                return {
-                    "events": [
-                        {
-                            "room_id": self.room_id,
-                            "sender": "@baduser:test.serv",
-                            "event_id": "three:test.serv",
-                            "depth": 1000,
-                            "origin_server_ts": 1,
-                            "type": "m.room.message",
-                            "origin": "test.serv",
-                            "content": "hewwo?",
-                            "auth_events": [],
-                            "prev_events": [("four:test.serv", {})],
-                        }
-                    ]
-                }
-
-        self.http_client.post_json = post_json
-
-        def get_json(destination, path, args, headers=None):
-            if path.startswith("/_matrix/federation/v1/state_ids/"):
-                d = self.successResultOf(
-                    self.homeserver.datastore.get_state_ids_for_event("one:test.serv")
-                )
-
-                return succeed(
-                    {
-                        "pdu_ids": [
-                            y
-                            for x, y in d.items()
-                            if x == ("m.room.member", "@us:test")
-                        ],
-                        "auth_chain_ids": list(d.values()),
-                    }
-                )
-
-        self.http_client.get_json = get_json
-
-        # Figure out what the most recent event is
-        most_recent = self.successResultOf(
-            maybeDeferred(
-                self.homeserver.datastore.get_latest_event_ids_in_room, self.room_id
-            )
-        )[0]
-
-        # Make a good event
-        good_event = FrozenEvent(
-            {
-                "room_id": self.room_id,
-                "sender": "@baduser:test.serv",
-                "event_id": "one:test.serv",
-                "depth": 1000,
-                "origin_server_ts": 1,
-                "type": "m.room.message",
-                "origin": "test.serv",
-                "content": "hewwo?",
-                "auth_events": [],
-                "prev_events": [(most_recent, {})],
-            }
-        )
-
-        with LoggingContext(request="good_event"):
-            d = self.handler.on_receive_pdu(
-                "test.serv", good_event, sent_to_us_directly=True
-            )
-            self.reactor.advance(1)
-            self.assertEqual(self.successResultOf(d), None)
-
-        bad_event = FrozenEvent(
-            {
-                "room_id": self.room_id,
-                "sender": "@baduser:test.serv",
-                "event_id": "two:test.serv",
-                "depth": 1000,
-                "origin_server_ts": 1,
-                "type": "m.room.message",
-                "origin": "test.serv",
-                "content": "hewwo?",
-                "auth_events": [],
-                "prev_events": [("one:test.serv", {}), ("three:test.serv", {})],
-            }
-        )
-
-        with LoggingContext(request="bad_event"):
-            d = self.handler.on_receive_pdu(
-                "test.serv", bad_event, sent_to_us_directly=True
-            )
-            self.reactor.advance(1)
-
-        extrem = maybeDeferred(
-            self.homeserver.datastore.get_latest_event_ids_in_room, self.room_id
-        )
-        self.assertEqual(self.successResultOf(extrem)[0], "two:test.serv")
-
-        state = self.homeserver.get_state_handler().get_current_state_ids(self.room_id)
-        self.reactor.advance(1)
-        self.assertIn(("m.room.member", "@us:test"), self.successResultOf(state).keys())
-- 
cgit 1.5.1


From 6e05fd032c670080f9e9f99f2e2e8b8eccf24c7d Mon Sep 17 00:00:00 2001
From: Amber Brown <hawkowl@atleastfornow.net>
Date: Tue, 2 Oct 2018 00:11:58 +1000
Subject: Fix userconsent on Python 3 (#3938)

---
 changelog.d/3938.bugfix              |   1 +
 synapse/api/urls.py                  |   2 +-
 tests/server_notices/test_consent.py | 100 +++++++++++++++++++++++++++++++++
 tests/storage/test_client_ips.py     |  48 +---------------
 tests/unittest.py                    |  80 +++++++++++++++++++++++++-
 tests/utils.py                       | 105 +++++++++++++++++++----------------
 6 files changed, 240 insertions(+), 96 deletions(-)
 create mode 100644 changelog.d/3938.bugfix
 create mode 100644 tests/server_notices/test_consent.py

(limited to 'tests')

diff --git a/changelog.d/3938.bugfix b/changelog.d/3938.bugfix
new file mode 100644
index 0000000000..01ccca21a7
--- /dev/null
+++ b/changelog.d/3938.bugfix
@@ -0,0 +1 @@
+Sending server notices regarding user consent now works on Python 3.
diff --git a/synapse/api/urls.py b/synapse/api/urls.py
index 71347912f1..6d9f1ca0ef 100644
--- a/synapse/api/urls.py
+++ b/synapse/api/urls.py
@@ -64,7 +64,7 @@ class ConsentURIBuilder(object):
         """
         mac = hmac.new(
             key=self._hmac_secret,
-            msg=user_id,
+            msg=user_id.encode('ascii'),
             digestmod=sha256,
         ).hexdigest()
         consent_uri = "%s_matrix/consent?%s" % (
diff --git a/tests/server_notices/test_consent.py b/tests/server_notices/test_consent.py
new file mode 100644
index 0000000000..95badc985e
--- /dev/null
+++ b/tests/server_notices/test_consent.py
@@ -0,0 +1,100 @@
+# -*- coding: utf-8 -*-
+# Copyright 2018 New Vector Ltd
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# 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 synapse.rest.client.v1 import admin, login, room
+from synapse.rest.client.v2_alpha import sync
+
+from tests import unittest
+
+
+class ConsentNoticesTests(unittest.HomeserverTestCase):
+
+    servlets = [
+        sync.register_servlets,
+        admin.register_servlets,
+        login.register_servlets,
+        room.register_servlets,
+    ]
+
+    def make_homeserver(self, reactor, clock):
+
+        self.consent_notice_message = "consent %(consent_uri)s"
+        config = self.default_config()
+        config.user_consent_version = "1"
+        config.user_consent_server_notice_content = {
+            "msgtype": "m.text",
+            "body": self.consent_notice_message,
+        }
+        config.public_baseurl = "https://example.com/"
+        config.form_secret = "123abc"
+
+        config.server_notices_mxid = "@notices:test"
+        config.server_notices_mxid_display_name = "test display name"
+        config.server_notices_mxid_avatar_url = None
+        config.server_notices_room_name = "Server Notices"
+
+        hs = self.setup_test_homeserver(config=config)
+
+        return hs
+
+    def prepare(self, reactor, clock, hs):
+        self.user_id = self.register_user("bob", "abc123")
+        self.access_token = self.login("bob", "abc123")
+
+    def test_get_sync_message(self):
+        """
+        When user consent server notices are enabled, a sync will cause a notice
+        to fire (in a room which the user is invited to). The notice contains
+        the notice URL + an authentication code.
+        """
+        # Initial sync, to get the user consent room invite
+        request, channel = self.make_request(
+            "GET", "/_matrix/client/r0/sync", access_token=self.access_token
+        )
+        self.render(request)
+        self.assertEqual(channel.code, 200)
+
+        # Get the Room ID to join
+        room_id = list(channel.json_body["rooms"]["invite"].keys())[0]
+
+        # Join the room
+        request, channel = self.make_request(
+            "POST",
+            "/_matrix/client/r0/rooms/" + room_id + "/join",
+            access_token=self.access_token,
+        )
+        self.render(request)
+        self.assertEqual(channel.code, 200)
+
+        # Sync again, to get the message in the room
+        request, channel = self.make_request(
+            "GET", "/_matrix/client/r0/sync", access_token=self.access_token
+        )
+        self.render(request)
+        self.assertEqual(channel.code, 200)
+
+        # Get the message
+        room = channel.json_body["rooms"]["join"][room_id]
+        messages = [
+            x for x in room["timeline"]["events"] if x["type"] == "m.room.message"
+        ]
+
+        # One message, with the consent URL
+        self.assertEqual(len(messages), 1)
+        self.assertTrue(
+            messages[0]["content"]["body"].startswith(
+                "consent https://example.com/_matrix/consent"
+            )
+        )
diff --git a/tests/storage/test_client_ips.py b/tests/storage/test_client_ips.py
index 2ffbb9f14f..4577e9422b 100644
--- a/tests/storage/test_client_ips.py
+++ b/tests/storage/test_client_ips.py
@@ -14,10 +14,6 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import hashlib
-import hmac
-import json
-
 from mock import Mock
 
 from twisted.internet import defer
@@ -145,34 +141,8 @@ class ClientIpAuthTestCase(unittest.HomeserverTestCase):
         return hs
 
     def prepare(self, hs, reactor, clock):
-        self.hs.config.registration_shared_secret = u"shared"
         self.store = self.hs.get_datastore()
-
-        # Create the user
-        request, channel = self.make_request("GET", "/_matrix/client/r0/admin/register")
-        self.render(request)
-        nonce = channel.json_body["nonce"]
-
-        want_mac = hmac.new(key=b"shared", digestmod=hashlib.sha1)
-        want_mac.update(nonce.encode('ascii') + b"\x00bob\x00abc123\x00admin")
-        want_mac = want_mac.hexdigest()
-
-        body = json.dumps(
-            {
-                "nonce": nonce,
-                "username": "bob",
-                "password": "abc123",
-                "admin": True,
-                "mac": want_mac,
-            }
-        )
-        request, channel = self.make_request(
-            "POST", "/_matrix/client/r0/admin/register", body.encode('utf8')
-        )
-        self.render(request)
-
-        self.assertEqual(channel.code, 200)
-        self.user_id = channel.json_body["user_id"]
+        self.user_id = self.register_user("bob", "abc123", True)
 
     def test_request_with_xforwarded(self):
         """
@@ -194,20 +164,7 @@ class ClientIpAuthTestCase(unittest.HomeserverTestCase):
     def _runtest(self, headers, expected_ip, make_request_args):
         device_id = "bleb"
 
-        body = json.dumps(
-            {
-                "type": "m.login.password",
-                "user": "bob",
-                "password": "abc123",
-                "device_id": device_id,
-            }
-        )
-        request, channel = self.make_request(
-            "POST", "/_matrix/client/r0/login", body.encode('utf8'), **make_request_args
-        )
-        self.render(request)
-        self.assertEqual(channel.code, 200)
-        access_token = channel.json_body["access_token"].encode('ascii')
+        access_token = self.login("bob", "abc123", device_id=device_id)
 
         # Advance to a known time
         self.reactor.advance(123456 - self.reactor.seconds())
@@ -215,7 +172,6 @@ class ClientIpAuthTestCase(unittest.HomeserverTestCase):
         request, channel = self.make_request(
             "GET",
             "/_matrix/client/r0/admin/users/" + self.user_id,
-            body.encode('utf8'),
             access_token=access_token,
             **make_request_args
         )
diff --git a/tests/unittest.py b/tests/unittest.py
index 043710afaf..a59291cc60 100644
--- a/tests/unittest.py
+++ b/tests/unittest.py
@@ -14,6 +14,8 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import hashlib
+import hmac
 import logging
 
 from mock import Mock
@@ -32,6 +34,7 @@ from synapse.types import UserID, create_requester
 from synapse.util.logcontext import LoggingContextFilter
 
 from tests.server import get_clock, make_request, render, setup_test_homeserver
+from tests.utils import default_config
 
 # Set up putting Synapse's logs into Trial's.
 rootLogger = logging.getLogger()
@@ -121,7 +124,7 @@ class TestCase(unittest.TestCase):
             try:
                 self.assertEquals(attrs[key], getattr(obj, key))
             except AssertionError as e:
-                raise (type(e))(str(e) + " for '.%s'" % key)
+                raise (type(e))(e.message + " for '.%s'" % key)
 
     def assert_dict(self, required, actual):
         """Does a partial assert of a dict.
@@ -223,6 +226,15 @@ class HomeserverTestCase(TestCase):
         hs = self.setup_test_homeserver()
         return hs
 
+    def default_config(self, name="test"):
+        """
+        Get a default HomeServer config object.
+
+        Args:
+            name (str): The homeserver name/domain.
+        """
+        return default_config(name)
+
     def prepare(self, reactor, clock, homeserver):
         """
         Prepare for the test.  This involves things like mocking out parts of
@@ -297,3 +309,69 @@ class HomeserverTestCase(TestCase):
             return d
         self.pump()
         return self.successResultOf(d)
+
+    def register_user(self, username, password, admin=False):
+        """
+        Register a user. Requires the Admin API be registered.
+
+        Args:
+            username (bytes/unicode): The user part of the new user.
+            password (bytes/unicode): The password of the new user.
+            admin (bool): Whether the user should be created as an admin
+            or not.
+
+        Returns:
+            The MXID of the new user (unicode).
+        """
+        self.hs.config.registration_shared_secret = u"shared"
+
+        # Create the user
+        request, channel = self.make_request("GET", "/_matrix/client/r0/admin/register")
+        self.render(request)
+        nonce = channel.json_body["nonce"]
+
+        want_mac = hmac.new(key=b"shared", digestmod=hashlib.sha1)
+        nonce_str = b"\x00".join([username.encode('utf8'), password.encode('utf8')])
+        if admin:
+            nonce_str += b"\x00admin"
+        else:
+            nonce_str += b"\x00notadmin"
+        want_mac.update(nonce.encode('ascii') + b"\x00" + nonce_str)
+        want_mac = want_mac.hexdigest()
+
+        body = json.dumps(
+            {
+                "nonce": nonce,
+                "username": username,
+                "password": password,
+                "admin": admin,
+                "mac": want_mac,
+            }
+        )
+        request, channel = self.make_request(
+            "POST", "/_matrix/client/r0/admin/register", body.encode('utf8')
+        )
+        self.render(request)
+        self.assertEqual(channel.code, 200)
+
+        user_id = channel.json_body["user_id"]
+        return user_id
+
+    def login(self, username, password, device_id=None):
+        """
+        Log in a user, and get an access token. Requires the Login API be
+        registered.
+
+        """
+        body = {"type": "m.login.password", "user": username, "password": password}
+        if device_id:
+            body["device_id"] = device_id
+
+        request, channel = self.make_request(
+            "POST", "/_matrix/client/r0/login", json.dumps(body).encode('utf8')
+        )
+        self.render(request)
+        self.assertEqual(channel.code, 200)
+
+        access_token = channel.json_body["access_token"].encode('ascii')
+        return access_token
diff --git a/tests/utils.py b/tests/utils.py
index aaed1149c3..1ef80e7b79 100644
--- a/tests/utils.py
+++ b/tests/utils.py
@@ -96,6 +96,62 @@ def setupdb():
         atexit.register(_cleanup)
 
 
+def default_config(name):
+    """
+    Create a reasonable test config.
+    """
+    config = Mock()
+    config.signing_key = [MockKey()]
+    config.event_cache_size = 1
+    config.enable_registration = True
+    config.macaroon_secret_key = "not even a little secret"
+    config.expire_access_token = False
+    config.server_name = name
+    config.trusted_third_party_id_servers = []
+    config.room_invite_state_types = []
+    config.password_providers = []
+    config.worker_replication_url = ""
+    config.worker_app = None
+    config.email_enable_notifs = False
+    config.block_non_admin_invites = False
+    config.federation_domain_whitelist = None
+    config.federation_rc_reject_limit = 10
+    config.federation_rc_sleep_limit = 10
+    config.federation_rc_sleep_delay = 100
+    config.federation_rc_concurrent = 10
+    config.filter_timeline_limit = 5000
+    config.user_directory_search_all_users = False
+    config.user_consent_server_notice_content = None
+    config.block_events_without_consent_error = None
+    config.media_storage_providers = []
+    config.auto_join_rooms = []
+    config.limit_usage_by_mau = False
+    config.hs_disabled = False
+    config.hs_disabled_message = ""
+    config.hs_disabled_limit_type = ""
+    config.max_mau_value = 50
+    config.mau_trial_days = 0
+    config.mau_limits_reserved_threepids = []
+    config.admin_contact = None
+    config.rc_messages_per_second = 10000
+    config.rc_message_burst_count = 10000
+
+    # we need a sane default_room_version, otherwise attempts to create rooms will
+    # fail.
+    config.default_room_version = "1"
+
+    # disable user directory updates, because they get done in the
+    # background, which upsets the test runner.
+    config.update_user_directory = False
+
+    def is_threepid_reserved(threepid):
+        return ServerConfig.is_threepid_reserved(config, threepid)
+
+    config.is_threepid_reserved.side_effect = is_threepid_reserved
+
+    return config
+
+
 class TestHomeServer(HomeServer):
     DATASTORE_CLASS = DataStore
 
@@ -124,54 +180,7 @@ def setup_test_homeserver(
         from twisted.internet import reactor
 
     if config is None:
-        config = Mock()
-        config.signing_key = [MockKey()]
-        config.event_cache_size = 1
-        config.enable_registration = True
-        config.macaroon_secret_key = "not even a little secret"
-        config.expire_access_token = False
-        config.server_name = name
-        config.trusted_third_party_id_servers = []
-        config.room_invite_state_types = []
-        config.password_providers = []
-        config.worker_replication_url = ""
-        config.worker_app = None
-        config.email_enable_notifs = False
-        config.block_non_admin_invites = False
-        config.federation_domain_whitelist = None
-        config.federation_rc_reject_limit = 10
-        config.federation_rc_sleep_limit = 10
-        config.federation_rc_sleep_delay = 100
-        config.federation_rc_concurrent = 10
-        config.filter_timeline_limit = 5000
-        config.user_directory_search_all_users = False
-        config.user_consent_server_notice_content = None
-        config.block_events_without_consent_error = None
-        config.media_storage_providers = []
-        config.auto_join_rooms = []
-        config.limit_usage_by_mau = False
-        config.hs_disabled = False
-        config.hs_disabled_message = ""
-        config.hs_disabled_limit_type = ""
-        config.max_mau_value = 50
-        config.mau_trial_days = 0
-        config.mau_limits_reserved_threepids = []
-        config.admin_contact = None
-        config.rc_messages_per_second = 10000
-        config.rc_message_burst_count = 10000
-
-        # we need a sane default_room_version, otherwise attempts to create rooms will
-        # fail.
-        config.default_room_version = "1"
-
-        # disable user directory updates, because they get done in the
-        # background, which upsets the test runner.
-        config.update_user_directory = False
-
-        def is_threepid_reserved(threepid):
-            return ServerConfig.is_threepid_reserved(config, threepid)
-
-        config.is_threepid_reserved.side_effect = is_threepid_reserved
+        config = default_config(name)
 
     config.use_frozen_dicts = True
     config.ldap_enabled = False
-- 
cgit 1.5.1