Merge pull request #894 from matrix-org/dbkr/push_room_naming

Use similar naming we use in email notifs for push

synapse/push/httppusher.py

@@ -38,6 +38,7 @@ def __init__(self, hs, pusherdict):
         self.hs = hs
         self.store = self.hs.get_datastore()
         self.clock = self.hs.get_clock()
+        self.state_handler = self.hs.get_state_handler()
         self.user_id = pusherdict['user_name']
         self.app_id = pusherdict['app_id']
         self.app_display_name = pusherdict['app_display_name']
@@ -237,7 +238,9 @@ def _process_one(self, push_action):
 
     @defer.inlineCallbacks
     def _build_notification_dict(self, event, tweaks, badge):
-        ctx = yield push_tools.get_context_for_event(self.hs.get_datastore(), event)
+        ctx = yield push_tools.get_context_for_event(
+            self.state_handler, event, self.user_id
+        )
 
         d = {
             'notification': {
@@ -269,8 +272,8 @@ def _build_notification_dict(self, event, tweaks, badge):
         if 'content' in event:
             d['notification']['content'] = event.content
 
-        if len(ctx['aliases']):
-            d['notification']['room_alias'] = ctx['aliases'][0]
+        # We no longer send aliases separately, instead, we send the human
+        # readable name of the room, which may be an alias.
         if 'sender_display_name' in ctx and len(ctx['sender_display_name']) > 0:
             d['notification']['sender_display_name'] = ctx['sender_display_name']
         if 'name' in ctx and len(ctx['name']) > 0:

synapse/push/push_tools.py

@@ -14,6 +14,9 @@
 # limitations under the License.
 
 from twisted.internet import defer
+from synapse.util.presentable_names import (
+    calculate_room_name, name_from_member_event
+)
 
 
 @defer.inlineCallbacks
@@ -45,24 +48,21 @@ def get_badge_count(store, user_id):
 
 
 @defer.inlineCallbacks
-def get_context_for_event(store, ev):
-    name_aliases = yield store.get_room_name_and_aliases(
-        ev.room_id
-    )
+def get_context_for_event(state_handler, ev, user_id):
+    ctx = {}
 
-    ctx = {'aliases': name_aliases[1]}
-    if name_aliases[0] is not None:
-        ctx['name'] = name_aliases[0]
+    room_state = yield state_handler.get_current_state(ev.room_id)
 
-    their_member_events_for_room = yield store.get_current_state(
-        room_id=ev.room_id,
-        event_type='m.room.member',
-        state_key=ev.user_id
+    # we no longer bother setting room_alias, and make room_name the
+    # human-readable name instead, be that m.room.namer, an alias or
+    # a list of people in the room
+    name = calculate_room_name(
+        room_state, user_id, fallback_to_single_member=False
     )
-    for mev in their_member_events_for_room:
-        if mev.content['membership'] == 'join' and 'displayname' in mev.content:
-            dn = mev.content['displayname']
-            if dn is not None:
-                ctx['sender_display_name'] = dn
+    if name:
+        ctx['name'] = name
+
+    sender_state_event = room_state[("m.room.member", ev.sender)]
+    ctx['sender_display_name'] = name_from_member_event(sender_state_event)
 
     defer.returnValue(ctx)

synapse/replication/slave/storage/events.py

@@ -18,7 +18,6 @@
 from synapse.api.constants import EventTypes
 from synapse.events import FrozenEvent
 from synapse.storage import DataStore
-from synapse.storage.room import RoomStore
 from synapse.storage.roommember import RoomMemberStore
 from synapse.storage.event_federation import EventFederationStore
 from synapse.storage.event_push_actions import EventPushActionsStore
@@ -64,7 +63,6 @@ def __init__(self, db_conn, hs):
 
     # Cached functions can't be accessed through a class instance so we need
     # to reach inside the __dict__ to extract them.
-    get_room_name_and_aliases = RoomStore.__dict__["get_room_name_and_aliases"]
     get_rooms_for_user = RoomMemberStore.__dict__["get_rooms_for_user"]
     get_users_in_room = RoomMemberStore.__dict__["get_users_in_room"]
     get_latest_event_ids_in_room = EventFederationStore.__dict__[
@@ -202,7 +200,6 @@ def invalidate_caches_for_event(self, event, backfilled, reset_state):
             self.get_rooms_for_user.invalidate_all()
             self.get_users_in_room.invalidate((event.room_id,))
             # self.get_joined_hosts_for_room.invalidate((event.room_id,))
-            self.get_room_name_and_aliases.invalidate((event.room_id,))
 
         self._invalidate_get_event_cache(event.event_id)
 
@@ -246,9 +243,3 @@ def invalidate_caches_for_event(self, event, backfilled, reset_state):
         self._get_current_state_for_key.invalidate((
             event.room_id, event.type, event.state_key
         ))
-
-        if event.type in [EventTypes.Name, EventTypes.Aliases]:
-            self.get_room_name_and_aliases.invalidate(
-                (event.room_id,)
-            )
-            pass

synapse/storage/events.py

@@ -355,7 +355,6 @@ def _persist_event_txn(self, txn, event, context, current_state, backfilled=Fals
             txn.call_after(self.get_rooms_for_user.invalidate_all)
             txn.call_after(self.get_users_in_room.invalidate, (event.room_id,))
             txn.call_after(self.get_joined_hosts_for_room.invalidate, (event.room_id,))
-            txn.call_after(self.get_room_name_and_aliases.invalidate, (event.room_id,))
 
             # Add an entry to the current_state_resets table to record the point
             # where we clobbered the current state
@@ -666,12 +665,6 @@ def event_dict(event):
                 (event.room_id, event.type, event.state_key,)
             )
 
-            if event.type in [EventTypes.Name, EventTypes.Aliases]:
-                txn.call_after(
-                    self.get_room_name_and_aliases.invalidate,
-                    (event.room_id,)
-                )
-
             self._simple_upsert_txn(
                 txn,
                 "current_state_events",

synapse/storage/room.py

@@ -18,7 +18,6 @@
 from synapse.api.errors import StoreError
 
 from ._base import SQLBaseStore
-from synapse.util.caches.descriptors import cachedInlineCallbacks
 from .engines import PostgresEngine, Sqlite3Engine
 
 import collections
@@ -192,49 +191,6 @@ def _store_event_search_txn(self, txn, event, key, value):
             # This should be unreachable.
             raise Exception("Unrecognized database engine")
 
-    @cachedInlineCallbacks()
-    def get_room_name_and_aliases(self, room_id):
-        def get_room_name(txn):
-            sql = (
-                "SELECT name FROM room_names"
-                " INNER JOIN current_state_events USING (room_id, event_id)"
-                " WHERE room_id = ?"
-                " LIMIT 1"
-            )
-
-            txn.execute(sql, (room_id,))
-            rows = txn.fetchall()
-            if rows:
-                return rows[0][0]
-            else:
-                return None
-
-            return [row[0] for row in txn.fetchall()]
-
-        def get_room_aliases(txn):
-            sql = (
-                "SELECT content FROM current_state_events"
-                " INNER JOIN events USING (room_id, event_id)"
-                " WHERE room_id = ?"
-            )
-            txn.execute(sql, (room_id,))
-            return [row[0] for row in txn.fetchall()]
-
-        name = yield self.runInteraction("get_room_name", get_room_name)
-        alias_contents = yield self.runInteraction("get_room_aliases", get_room_aliases)
-
-        aliases = []
-
-        for c in alias_contents:
-            try:
-                content = json.loads(c)
-            except:
-                continue
-
-            aliases.extend(content.get('aliases', []))
-
-        defer.returnValue((name, aliases))
-
     def add_event_report(self, room_id, event_id, user_id, reason, content,
                          received_ts):
         next_id = self._event_reports_id_gen.get_next()

synapse/util/presentable_names.py

@@ -25,7 +25,8 @@
 ALL_ALONE = "Empty Room"
 
 
-def calculate_room_name(room_state, user_id, fallback_to_members=True):
+def calculate_room_name(room_state, user_id, fallback_to_members=True,
+                        fallback_to_single_member=True):
     """
     Works out a user-facing name for the given room as per Matrix
     spec recommendations.
@@ -129,6 +130,8 @@ def calculate_room_name(room_state, user_id, fallback_to_members=True):
                 return name_from_member_event(all_members[0])
         else:
             return ALL_ALONE
+    elif len(other_members) == 1 and not fallback_to_single_member:
+        return None
     else:
         return descriptor_from_member_events(other_members)
 

tests/replication/slave/storage/test_events.py

@@ -58,47 +58,6 @@ def setUp(self):
     def tearDown(self):
         [unpatch() for unpatch in self.unpatches]
 
-    @defer.inlineCallbacks
-    def test_room_name_and_aliases(self):
-        create = yield self.persist(type="m.room.create", key="", creator=USER_ID)
-        yield self.persist(type="m.room.member", key=USER_ID, membership="join")
-        yield self.persist(type="m.room.name", key="", name="name1")
-        yield self.persist(
-            type="m.room.aliases", key="blue", aliases=["#1:blue"]
-        )
-        yield self.replicate()
-        yield self.check(
-            "get_room_name_and_aliases", (ROOM_ID,), ("name1", ["#1:blue"])
-        )
-
-        # Set the room name.
-        yield self.persist(type="m.room.name", key="", name="name2")
-        yield self.replicate()
-        yield self.check(
-            "get_room_name_and_aliases", (ROOM_ID,), ("name2", ["#1:blue"])
-        )
-
-        # Set the room aliases.
-        yield self.persist(
-            type="m.room.aliases", key="blue", aliases=["#2:blue"]
-        )
-        yield self.replicate()
-        yield self.check(
-            "get_room_name_and_aliases", (ROOM_ID,), ("name2", ["#2:blue"])
-        )
-
-        # Leave and join the room clobbering the state.
-        yield self.persist(type="m.room.member", key=USER_ID, membership="leave")
-        yield self.persist(
-            type="m.room.member", key=USER_ID, membership="join",
-            reset_state=[create]
-        )
-        yield self.replicate()
-
-        yield self.check(
-            "get_room_name_and_aliases", (ROOM_ID,), (None, [])
-        )
-
     @defer.inlineCallbacks
     def test_room_members(self):
         create = yield self.persist(type="m.room.create", key="", creator=USER_ID)