From ec0a523ac338bab1eb23a6b21227b8f7402cc2d4 Mon Sep 17 00:00:00 2001
From: Erik Johnston <erik@matrix.org>
Date: Tue, 10 Jan 2017 18:37:18 +0000
Subject: Split out static state methods from StateHandler

---
 synapse/state.py | 144 ++++++++++++++++++++++++++++---------------------------
 1 file changed, 74 insertions(+), 70 deletions(-)

(limited to 'synapse/state.py')

diff --git a/synapse/state.py b/synapse/state.py
index b9d5627a82..c75499c3e0 100644
--- a/synapse/state.py
+++ b/synapse/state.py
@@ -16,6 +16,7 @@
 
 from twisted.internet import defer
 
+from synapse import event_auth
 from synapse.util.logutils import log_function
 from synapse.util.caches.expiringcache import ExpiringCache
 from synapse.util.metrics import Measure
@@ -335,9 +336,10 @@ class StateHandler(object):
                     [state_map[e_id] for key, e_id in st.items() if e_id in state_map]
                     for st in state_groups_ids.values()
                 ]
-                new_state, _ = self._resolve_events(
-                    state_sets, event_type, state_key
-                )
+                with Measure(self.clock, "state._resolve_events"):
+                    new_state, _ = Resolver.resolve_events(
+                        state_sets, event_type, state_key
+                    )
                 new_state = {
                     key: e.event_id for key, e in new_state.items()
                 }
@@ -388,68 +390,78 @@ class StateHandler(object):
         logger.info(
             "Resolving state for %s with %d groups", event.room_id, len(state_sets)
         )
-        if event.is_state():
-            return self._resolve_events(
-                state_sets, event.type, event.state_key
-            )
-        else:
-            return self._resolve_events(state_sets)
+        with Measure(self.clock, "state._resolve_events"):
+            if event.is_state():
+                return Resolver.resolve_events(
+                    state_sets, event.type, event.state_key
+                )
+            else:
+                return Resolver.resolve_events(state_sets)
+
 
-    def _resolve_events(self, state_sets, event_type=None, state_key=""):
+def _ordered_events(events):
+    def key_func(e):
+        return -int(e.depth), hashlib.sha1(e.event_id).hexdigest()
+
+    return sorted(events, key=key_func)
+
+
+class Resolver(object):
+    @staticmethod
+    def resolve_events(state_sets, event_type=None, state_key=""):
         """
         Returns
             (dict[(str, str), synapse.events.FrozenEvent], list[str]): a tuple
             (new_state, prev_states). new_state is a map from (type, state_key)
             to event. prev_states is a list of event_ids.
         """
-        with Measure(self.clock, "state._resolve_events"):
-            state = {}
-            for st in state_sets:
-                for e in st:
-                    state.setdefault(
-                        (e.type, e.state_key),
-                        {}
-                    )[e.event_id] = e
-
-            unconflicted_state = {
-                k: v.values()[0] for k, v in state.items()
-                if len(v.values()) == 1
-            }
+        state = {}
+        for st in state_sets:
+            for e in st:
+                state.setdefault(
+                    (e.type, e.state_key),
+                    {}
+                )[e.event_id] = e
+
+        unconflicted_state = {
+            k: v.values()[0] for k, v in state.items()
+            if len(v.values()) == 1
+        }
 
-            conflicted_state = {
-                k: v.values()
-                for k, v in state.items()
-                if len(v.values()) > 1
-            }
+        conflicted_state = {
+            k: v.values()
+            for k, v in state.items()
+            if len(v.values()) > 1
+        }
 
-            if event_type:
-                prev_states_events = conflicted_state.get(
-                    (event_type, state_key), []
-                )
-                prev_states = [s.event_id for s in prev_states_events]
-            else:
-                prev_states = []
+        if event_type:
+            prev_states_events = conflicted_state.get(
+                (event_type, state_key), []
+            )
+            prev_states = [s.event_id for s in prev_states_events]
+        else:
+            prev_states = []
 
-            auth_events = {
-                k: e for k, e in unconflicted_state.items()
-                if k[0] in AuthEventTypes
-            }
+        auth_events = {
+            k: e for k, e in unconflicted_state.items()
+            if k[0] in AuthEventTypes
+        }
 
-            try:
-                resolved_state = self._resolve_state_events(
-                    conflicted_state, auth_events
-                )
-            except:
-                logger.exception("Failed to resolve state")
-                raise
+        try:
+            resolved_state = Resolver._resolve_state_events(
+                conflicted_state, auth_events
+            )
+        except:
+            logger.exception("Failed to resolve state")
+            raise
 
-            new_state = unconflicted_state
-            new_state.update(resolved_state)
+        new_state = unconflicted_state
+        new_state.update(resolved_state)
 
         return new_state, prev_states
 
-    @log_function
-    def _resolve_state_events(self, conflicted_state, auth_events):
+    @staticmethod
+    def _resolve_state_events(conflicted_state, auth_events):
         """ This is where we actually decide which of the conflicted state to
         use.
 
@@ -464,7 +476,7 @@ class StateHandler(object):
         if power_key in conflicted_state:
             events = conflicted_state[power_key]
             logger.debug("Resolving conflicted power levels %r", events)
-            resolved_state[power_key] = self._resolve_auth_events(
+            resolved_state[power_key] = Resolver._resolve_auth_events(
                 events, auth_events)
 
         auth_events.update(resolved_state)
@@ -472,7 +484,7 @@ class StateHandler(object):
         for key, events in conflicted_state.items():
             if key[0] == EventTypes.JoinRules:
                 logger.debug("Resolving conflicted join rules %r", events)
-                resolved_state[key] = self._resolve_auth_events(
+                resolved_state[key] = Resolver._resolve_auth_events(
                     events,
                     auth_events
                 )
@@ -482,7 +494,7 @@ class StateHandler(object):
         for key, events in conflicted_state.items():
             if key[0] == EventTypes.Member:
                 logger.debug("Resolving conflicted member lists %r", events)
-                resolved_state[key] = self._resolve_auth_events(
+                resolved_state[key] = Resolver._resolve_auth_events(
                     events,
                     auth_events
                 )
@@ -492,14 +504,15 @@ class StateHandler(object):
         for key, events in conflicted_state.items():
             if key not in resolved_state:
                 logger.debug("Resolving conflicted state %r:%r", key, events)
-                resolved_state[key] = self._resolve_normal_events(
+                resolved_state[key] = Resolver._resolve_normal_events(
                     events, auth_events
                 )
 
         return resolved_state
 
-    def _resolve_auth_events(self, events, auth_events):
-        reverse = [i for i in reversed(self._ordered_events(events))]
+    @staticmethod
+    def _resolve_auth_events(events, auth_events):
+        reverse = [i for i in reversed(_ordered_events(events))]
 
         auth_events = dict(auth_events)
 
@@ -507,23 +520,20 @@ class StateHandler(object):
         for event in reverse[1:]:
             auth_events[(prev_event.type, prev_event.state_key)] = prev_event
             try:
-                # FIXME: hs.get_auth() is bad style, but we need to do it to
-                # get around circular deps.
                 # The signatures have already been checked at this point
-                self.hs.get_auth().check(event, auth_events, do_sig_check=False)
+                event_auth.check(event, auth_events, do_sig_check=False)
                 prev_event = event
             except AuthError:
                 return prev_event
 
         return event
 
-    def _resolve_normal_events(self, events, auth_events):
-        for event in self._ordered_events(events):
+    @staticmethod
+    def _resolve_normal_events(events, auth_events):
+        for event in _ordered_events(events):
             try:
-                # FIXME: hs.get_auth() is bad style, but we need to do it to
-                # get around circular deps.
                 # The signatures have already been checked at this point
-                self.hs.get_auth().check(event, auth_events, do_sig_check=False)
+                event_auth.check(event, auth_events, do_sig_check=False)
                 return event
             except AuthError:
                 pass
@@ -531,9 +541,3 @@ class StateHandler(object):
         # Use the last event (the one with the least depth) if they all fail
         # the auth check.
         return event
-
-    def _ordered_events(self, events):
-        def key_func(e):
-            return -int(e.depth), hashlib.sha1(e.event_id).hexdigest()
-
-        return sorted(events, key=key_func)
-- 
cgit 1.5.1


From 46aebbbcbf94eb78ae45d3bb3bf3ffeabb44dd4f Mon Sep 17 00:00:00 2001
From: Erik Johnston <erik@matrix.org>
Date: Mon, 16 Jan 2017 13:48:04 +0000
Subject: Add support for 'iterable' to ExpiringCache

---
 synapse/state.py                     |  6 +++++-
 synapse/util/caches/expiringcache.py | 26 +++++++++++++++++---------
 2 files changed, 22 insertions(+), 10 deletions(-)

(limited to 'synapse/state.py')

diff --git a/synapse/state.py b/synapse/state.py
index b9d5627a82..461e82acdf 100644
--- a/synapse/state.py
+++ b/synapse/state.py
@@ -41,7 +41,7 @@ KeyStateTuple = namedtuple("KeyStateTuple", ("context", "type", "state_key"))
 CACHE_SIZE_FACTOR = float(os.environ.get("SYNAPSE_CACHE_FACTOR", 0.1))
 
 
-SIZE_OF_CACHE = int(1000 * CACHE_SIZE_FACTOR)
+SIZE_OF_CACHE = int(10000 * CACHE_SIZE_FACTOR)
 EVICTION_TIMEOUT_SECONDS = 60 * 60
 
 
@@ -77,6 +77,9 @@ class _StateCacheEntry(object):
         else:
             self.state_id = _gen_state_id()
 
+    def __len__(self):
+        return len(self.state)
+
 
 class StateHandler(object):
     """ Responsible for doing state conflict resolution.
@@ -99,6 +102,7 @@ class StateHandler(object):
             clock=self.clock,
             max_len=SIZE_OF_CACHE,
             expiry_ms=EVICTION_TIMEOUT_SECONDS * 1000,
+            iterable=True,
             reset_expiry_on_get=True,
         )
 
diff --git a/synapse/util/caches/expiringcache.py b/synapse/util/caches/expiringcache.py
index 080388958f..9b44b3fab3 100644
--- a/synapse/util/caches/expiringcache.py
+++ b/synapse/util/caches/expiringcache.py
@@ -23,7 +23,7 @@ logger = logging.getLogger(__name__)
 
 class ExpiringCache(object):
     def __init__(self, cache_name, clock, max_len=0, expiry_ms=0,
-                 reset_expiry_on_get=False):
+                 reset_expiry_on_get=False, iterable=False):
         """
         Args:
             cache_name (str): Name of this cache, used for logging.
@@ -36,6 +36,8 @@ class ExpiringCache(object):
                 evicted based on time.
             reset_expiry_on_get (bool): If true, will reset the expiry time for
                 an item on access. Defaults to False.
+            iterable (bool): If true, the size is calculated by summing the
+                sizes of all entries, rather than the number of entries.
 
         """
         self._cache_name = cache_name
@@ -49,7 +51,9 @@ class ExpiringCache(object):
 
         self._cache = {}
 
-        self.metrics = register_cache(cache_name, self._cache)
+        self.metrics = register_cache(cache_name, self)
+
+        self.iterable = iterable
 
     def start(self):
         if not self._expiry_ms:
@@ -66,14 +70,15 @@ class ExpiringCache(object):
         self._cache[key] = _CacheEntry(now, value)
 
         # Evict if there are now too many items
-        if self._max_len and len(self._cache.keys()) > self._max_len:
+        if self._max_len and len(self) > self._max_len:
             sorted_entries = sorted(
-                self._cache.items(),
+                self._cache.keys(),
                 key=lambda item: item[1].time,
             )
 
-            for k, _ in sorted_entries[self._max_len:]:
-                self._cache.pop(k)
+            while len(self) > self._max_len and sorted_entries:
+                key = sorted_entries.pop()
+                self._cache.pop(key)
 
     def __getitem__(self, key):
         try:
@@ -99,7 +104,7 @@ class ExpiringCache(object):
             # zero expiry time means don't expire. This should never get called
             # since we have this check in start too.
             return
-        begin_length = len(self._cache)
+        begin_length = len(self)
 
         now = self._clock.time_msec()
 
@@ -114,11 +119,14 @@ class ExpiringCache(object):
 
         logger.debug(
             "[%s] _prune_cache before: %d, after len: %d",
-            self._cache_name, begin_length, len(self._cache)
+            self._cache_name, begin_length, len(self)
         )
 
     def __len__(self):
-        return len(self._cache)
+        if self.iterable:
+            return sum(len(value.value) for value in self._cache.itervalues())
+        else:
+            return len(self._cache)
 
 
 class _CacheEntry(object):
-- 
cgit 1.5.1


From beda469bc6e96a0b776c3d6742cf97950819b2f0 Mon Sep 17 00:00:00 2001
From: Erik Johnston <erik@matrix.org>
Date: Mon, 16 Jan 2017 15:05:24 +0000
Subject: Put staticmethods at module level

---
 synapse/state.py | 244 +++++++++++++++++++++++++++----------------------------
 1 file changed, 121 insertions(+), 123 deletions(-)

(limited to 'synapse/state.py')

diff --git a/synapse/state.py b/synapse/state.py
index c75499c3e0..90b14e758c 100644
--- a/synapse/state.py
+++ b/synapse/state.py
@@ -337,7 +337,7 @@ class StateHandler(object):
                     for st in state_groups_ids.values()
                 ]
                 with Measure(self.clock, "state._resolve_events"):
-                    new_state, _ = Resolver.resolve_events(
+                    new_state, _ = resolve_events(
                         state_sets, event_type, state_key
                     )
                 new_state = {
@@ -392,11 +392,11 @@ class StateHandler(object):
         )
         with Measure(self.clock, "state._resolve_events"):
             if event.is_state():
-                return Resolver.resolve_events(
+                return resolve_events(
                     state_sets, event.type, event.state_key
                 )
             else:
-                return Resolver.resolve_events(state_sets)
+                return resolve_events(state_sets)
 
 
 def _ordered_events(events):
@@ -406,138 +406,136 @@ def _ordered_events(events):
     return sorted(events, key=key_func)
 
 
-class Resolver(object):
-    @staticmethod
-    def resolve_events(state_sets, event_type=None, state_key=""):
-        """
-        Returns
-            (dict[(str, str), synapse.events.FrozenEvent], list[str]): a tuple
-            (new_state, prev_states). new_state is a map from (type, state_key)
-            to event. prev_states is a list of event_ids.
-        """
-        state = {}
-        for st in state_sets:
-            for e in st:
-                state.setdefault(
-                    (e.type, e.state_key),
-                    {}
-                )[e.event_id] = e
-
-        unconflicted_state = {
-            k: v.values()[0] for k, v in state.items()
-            if len(v.values()) == 1
-        }
+def resolve_events(state_sets, event_type=None, state_key=""):
+    """
+    Returns
+        (dict[(str, str), synapse.events.FrozenEvent], list[str]): a tuple
+        (new_state, prev_states). new_state is a map from (type, state_key)
+        to event. prev_states is a list of event_ids.
+    """
+    state = {}
+    for st in state_sets:
+        for e in st:
+            state.setdefault(
+                (e.type, e.state_key),
+                {}
+            )[e.event_id] = e
+
+    unconflicted_state = {
+        k: v.values()[0] for k, v in state.items()
+        if len(v.values()) == 1
+    }
+
+    conflicted_state = {
+        k: v.values()
+        for k, v in state.items()
+        if len(v.values()) > 1
+    }
+
+    if event_type:
+        prev_states_events = conflicted_state.get(
+            (event_type, state_key), []
+        )
+        prev_states = [s.event_id for s in prev_states_events]
+    else:
+        prev_states = []
+
+    auth_events = {
+        k: e for k, e in unconflicted_state.items()
+        if k[0] in AuthEventTypes
+    }
+
+    try:
+        resolved_state = _resolve_state_events(
+            conflicted_state, auth_events
+        )
+    except:
+        logger.exception("Failed to resolve state")
+        raise
 
-        conflicted_state = {
-            k: v.values()
-            for k, v in state.items()
-            if len(v.values()) > 1
-        }
+    new_state = unconflicted_state
+    new_state.update(resolved_state)
 
-        if event_type:
-            prev_states_events = conflicted_state.get(
-                (event_type, state_key), []
+    return new_state, prev_states
+
+
+def _resolve_state_events(conflicted_state, auth_events):
+    """ This is where we actually decide which of the conflicted state to
+    use.
+
+    We resolve conflicts in the following order:
+        1. power levels
+        2. join rules
+        3. memberships
+        4. other events.
+    """
+    resolved_state = {}
+    power_key = (EventTypes.PowerLevels, "")
+    if power_key in conflicted_state:
+        events = conflicted_state[power_key]
+        logger.debug("Resolving conflicted power levels %r", events)
+        resolved_state[power_key] = _resolve_auth_events(
+            events, auth_events)
+
+    auth_events.update(resolved_state)
+
+    for key, events in conflicted_state.items():
+        if key[0] == EventTypes.JoinRules:
+            logger.debug("Resolving conflicted join rules %r", events)
+            resolved_state[key] = _resolve_auth_events(
+                events,
+                auth_events
             )
-            prev_states = [s.event_id for s in prev_states_events]
-        else:
-            prev_states = []
 
-        auth_events = {
-            k: e for k, e in unconflicted_state.items()
-            if k[0] in AuthEventTypes
-        }
+    auth_events.update(resolved_state)
 
-        try:
-            resolved_state = Resolver._resolve_state_events(
-                conflicted_state, auth_events
+    for key, events in conflicted_state.items():
+        if key[0] == EventTypes.Member:
+            logger.debug("Resolving conflicted member lists %r", events)
+            resolved_state[key] = _resolve_auth_events(
+                events,
+                auth_events
             )
-        except:
-            logger.exception("Failed to resolve state")
-            raise
 
-        new_state = unconflicted_state
-        new_state.update(resolved_state)
+    auth_events.update(resolved_state)
 
-        return new_state, prev_states
+    for key, events in conflicted_state.items():
+        if key not in resolved_state:
+            logger.debug("Resolving conflicted state %r:%r", key, events)
+            resolved_state[key] = _resolve_normal_events(
+                events, auth_events
+            )
 
-    @staticmethod
-    def _resolve_state_events(conflicted_state, auth_events):
-        """ This is where we actually decide which of the conflicted state to
-        use.
+    return resolved_state
 
-        We resolve conflicts in the following order:
-            1. power levels
-            2. join rules
-            3. memberships
-            4. other events.
-        """
-        resolved_state = {}
-        power_key = (EventTypes.PowerLevels, "")
-        if power_key in conflicted_state:
-            events = conflicted_state[power_key]
-            logger.debug("Resolving conflicted power levels %r", events)
-            resolved_state[power_key] = Resolver._resolve_auth_events(
-                events, auth_events)
-
-        auth_events.update(resolved_state)
-
-        for key, events in conflicted_state.items():
-            if key[0] == EventTypes.JoinRules:
-                logger.debug("Resolving conflicted join rules %r", events)
-                resolved_state[key] = Resolver._resolve_auth_events(
-                    events,
-                    auth_events
-                )
 
-        auth_events.update(resolved_state)
+def _resolve_auth_events(events, auth_events):
+    reverse = [i for i in reversed(_ordered_events(events))]
 
-        for key, events in conflicted_state.items():
-            if key[0] == EventTypes.Member:
-                logger.debug("Resolving conflicted member lists %r", events)
-                resolved_state[key] = Resolver._resolve_auth_events(
-                    events,
-                    auth_events
-                )
+    auth_events = dict(auth_events)
+
+    prev_event = reverse[0]
+    for event in reverse[1:]:
+        auth_events[(prev_event.type, prev_event.state_key)] = prev_event
+        try:
+            # The signatures have already been checked at this point
+            event_auth.check(event, auth_events, do_sig_check=False)
+            prev_event = event
+        except AuthError:
+            return prev_event
 
-        auth_events.update(resolved_state)
+    return event
 
-        for key, events in conflicted_state.items():
-            if key not in resolved_state:
-                logger.debug("Resolving conflicted state %r:%r", key, events)
-                resolved_state[key] = Resolver._resolve_normal_events(
-                    events, auth_events
-                )
 
-        return resolved_state
-
-    @staticmethod
-    def _resolve_auth_events(events, auth_events):
-        reverse = [i for i in reversed(_ordered_events(events))]
-
-        auth_events = dict(auth_events)
-
-        prev_event = reverse[0]
-        for event in reverse[1:]:
-            auth_events[(prev_event.type, prev_event.state_key)] = prev_event
-            try:
-                # The signatures have already been checked at this point
-                event_auth.check(event, auth_events, do_sig_check=False)
-                prev_event = event
-            except AuthError:
-                return prev_event
-
-        return event
-
-    @staticmethod
-    def _resolve_normal_events(events, auth_events):
-        for event in _ordered_events(events):
-            try:
-                # The signatures have already been checked at this point
-                event_auth.check(event, auth_events, do_sig_check=False)
-                return event
-            except AuthError:
-                pass
-
-        # Use the last event (the one with the least depth) if they all fail
-        # the auth check.
-        return event
+def _resolve_normal_events(events, auth_events):
+    for event in _ordered_events(events):
+        try:
+            # The signatures have already been checked at this point
+            event_auth.check(event, auth_events, do_sig_check=False)
+            return event
+        except AuthError:
+            pass
+
+    # Use the last event (the one with the least depth) if they all fail
+    # the auth check.
+    return event
-- 
cgit 1.5.1


From 897f8752da3c9f7b2d214fe91e8356be5db545c3 Mon Sep 17 00:00:00 2001
From: Erik Johnston <erik@matrix.org>
Date: Mon, 16 Jan 2017 15:08:17 +0000
Subject: Up cache max entries for state

---
 synapse/state.py              | 2 +-
 synapse/storage/roommember.py | 2 +-
 synapse/storage/state.py      | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

(limited to 'synapse/state.py')

diff --git a/synapse/state.py b/synapse/state.py
index 461e82acdf..66e1a685e8 100644
--- a/synapse/state.py
+++ b/synapse/state.py
@@ -41,7 +41,7 @@ KeyStateTuple = namedtuple("KeyStateTuple", ("context", "type", "state_key"))
 CACHE_SIZE_FACTOR = float(os.environ.get("SYNAPSE_CACHE_FACTOR", 0.1))
 
 
-SIZE_OF_CACHE = int(10000 * CACHE_SIZE_FACTOR)
+SIZE_OF_CACHE = int(100000 * CACHE_SIZE_FACTOR)
 EVICTION_TIMEOUT_SECONDS = 60 * 60
 
 
diff --git a/synapse/storage/roommember.py b/synapse/storage/roommember.py
index 8dce89073d..768e0a4451 100644
--- a/synapse/storage/roommember.py
+++ b/synapse/storage/roommember.py
@@ -391,7 +391,7 @@ class RoomMemberStore(SQLBaseStore):
         )
 
     @cachedInlineCallbacks(num_args=2, cache_context=True, iterable=True,
-                           max_entries=50000)
+                           max_entries=100000)
     def _get_joined_users_from_context(self, room_id, state_group, current_state_ids,
                                        cache_context, event=None):
         # We don't use `state_group`, it's there so that we can cache based
diff --git a/synapse/storage/state.py b/synapse/storage/state.py
index fe942ecad7..7d34dd03bf 100644
--- a/synapse/storage/state.py
+++ b/synapse/storage/state.py
@@ -284,7 +284,7 @@ class StateStore(SQLBaseStore):
             return [r[0] for r in results]
         return self.runInteraction("get_current_state_for_key", f)
 
-    @cached(num_args=2, max_entries=50000, iterable=True)
+    @cached(num_args=2, max_entries=100000, iterable=True)
     def _get_state_group_from_group(self, group, types):
         raise NotImplementedError()
 
-- 
cgit 1.5.1


From 5d6bad1b3c325897db81f84ebfc67ca687d851c0 Mon Sep 17 00:00:00 2001
From: Erik Johnston <erik@matrix.org>
Date: Fri, 13 Jan 2017 13:16:54 +0000
Subject: Optimise state resolution

---
 synapse/event_auth.py          |  49 ++++++++--
 synapse/events/__init__.py     |   8 +-
 synapse/events/builder.py      |   6 +-
 synapse/handlers/federation.py |   2 +-
 synapse/state.py               | 209 +++++++++++++++++++++++++++++------------
 tests/api/test_filtering.py    |   5 +-
 tests/events/test_utils.py     |  22 ++++-
 7 files changed, 229 insertions(+), 72 deletions(-)

(limited to 'synapse/state.py')

diff --git a/synapse/event_auth.py b/synapse/event_auth.py
index 983d8e9a85..3b7726a526 100644
--- a/synapse/event_auth.py
+++ b/synapse/event_auth.py
@@ -27,7 +27,7 @@ from synapse.types import UserID, get_domain_from_id
 logger = logging.getLogger(__name__)
 
 
-def check(event, auth_events, do_sig_check=True):
+def check(event, auth_events, do_sig_check=True, do_size_check=True):
     """ Checks if this event is correctly authed.
 
     Args:
@@ -38,7 +38,8 @@ def check(event, auth_events, do_sig_check=True):
     Returns:
         True if the auth checks pass.
     """
-    _check_size_limits(event)
+    if do_size_check:
+        _check_size_limits(event)
 
     if not hasattr(event, "room_id"):
         raise AuthError(500, "Event has no room_id: %s" % event)
@@ -119,10 +120,11 @@ def check(event, auth_events, do_sig_check=True):
             )
         return True
 
-    logger.debug(
-        "Auth events: %s",
-        [a.event_id for a in auth_events.values()]
-    )
+    if logger.isEnabledFor(logging.DEBUG):
+        logger.debug(
+            "Auth events: %s",
+            [a.event_id for a in auth_events.values()]
+        )
 
     if event.type == EventTypes.Member:
         allowed = _is_membership_change_allowed(
@@ -639,3 +641,38 @@ def get_public_keys(invite_event):
         public_keys.append(o)
     public_keys.extend(invite_event.content.get("public_keys", []))
     return public_keys
+
+
+def auth_types_for_event(event):
+    """Given an event, return a list of (EventType, StateKey) that may be
+    needed to auth the event. The returned list may be a superset of what
+    would actually be required depending on the full state of the room.
+
+    Used to limit the number of events to fetch from the database to
+    actually auth the event.
+    """
+    if event.type == EventTypes.Create:
+        return []
+
+    auth_types = []
+
+    auth_types.append((EventTypes.PowerLevels, "", ))
+    auth_types.append((EventTypes.Member, event.user_id, ))
+    auth_types.append((EventTypes.Create, "", ))
+
+    if event.type == EventTypes.Member:
+        e_type = event.content["membership"]
+        if e_type in [Membership.JOIN, Membership.INVITE]:
+            auth_types.append((EventTypes.JoinRules, "", ))
+
+        auth_types.append((EventTypes.Member, event.state_key, ))
+
+        if e_type == Membership.INVITE:
+            if "third_party_invite" in event.content:
+                key = (
+                    EventTypes.ThirdPartyInvite,
+                    event.content["third_party_invite"]["signed"]["token"]
+                )
+                auth_types.append(key)
+
+    return auth_types
diff --git a/synapse/events/__init__.py b/synapse/events/__init__.py
index da9f3ad436..e673e96cc0 100644
--- a/synapse/events/__init__.py
+++ b/synapse/events/__init__.py
@@ -79,7 +79,6 @@ class EventBase(object):
     auth_events = _event_dict_property("auth_events")
     depth = _event_dict_property("depth")
     content = _event_dict_property("content")
-    event_id = _event_dict_property("event_id")
     hashes = _event_dict_property("hashes")
     origin = _event_dict_property("origin")
     origin_server_ts = _event_dict_property("origin_server_ts")
@@ -88,8 +87,6 @@ class EventBase(object):
     redacts = _event_dict_property("redacts")
     room_id = _event_dict_property("room_id")
     sender = _event_dict_property("sender")
-    state_key = _event_dict_property("state_key")
-    type = _event_dict_property("type")
     user_id = _event_dict_property("sender")
 
     @property
@@ -162,6 +159,11 @@ class FrozenEvent(EventBase):
         else:
             frozen_dict = event_dict
 
+        self.event_id = event_dict["event_id"]
+        self.type = event_dict["type"]
+        if "state_key" in event_dict:
+            self.state_key = event_dict["state_key"]
+
         super(FrozenEvent, self).__init__(
             frozen_dict,
             signatures=signatures,
diff --git a/synapse/events/builder.py b/synapse/events/builder.py
index 7369d70980..365fd96bd2 100644
--- a/synapse/events/builder.py
+++ b/synapse/events/builder.py
@@ -13,7 +13,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-from . import EventBase, FrozenEvent
+from . import EventBase, FrozenEvent, _event_dict_property
 
 from synapse.types import EventID
 
@@ -34,6 +34,10 @@ class EventBuilder(EventBase):
             internal_metadata_dict=internal_metadata_dict,
         )
 
+    event_id = _event_dict_property("event_id")
+    state_key = _event_dict_property("state_key")
+    type = _event_dict_property("type")
+
     def build(self):
         return FrozenEvent.from_event(self)
 
diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py
index 1021bcc405..ea89e0cf2d 100644
--- a/synapse/handlers/federation.py
+++ b/synapse/handlers/federation.py
@@ -1530,7 +1530,7 @@ class FederationHandler(BaseHandler):
                     (d.type, d.state_key): d for d in different_events if d
                 })
 
-                new_state, prev_state = self.state_handler.resolve_events(
+                new_state = self.state_handler.resolve_events(
                     [local_view.values(), remote_view.values()],
                     event
                 )
diff --git a/synapse/state.py b/synapse/state.py
index 90b14e758c..294e0c2081 100644
--- a/synapse/state.py
+++ b/synapse/state.py
@@ -22,11 +22,10 @@ from synapse.util.caches.expiringcache import ExpiringCache
 from synapse.util.metrics import Measure
 from synapse.api.constants import EventTypes
 from synapse.api.errors import AuthError
-from synapse.api.auth import AuthEventTypes
 from synapse.events.snapshot import EventContext
 from synapse.util.async import Linearizer
 
-from collections import namedtuple
+from collections import namedtuple, defaultdict
 from frozendict import frozendict
 
 import logging
@@ -48,6 +47,8 @@ EVICTION_TIMEOUT_SECONDS = 60 * 60
 
 _NEXT_STATE_ID = 1
 
+POWER_KEY = (EventTypes.PowerLevels, "")
+
 
 def _gen_state_id():
     global _NEXT_STATE_ID
@@ -328,21 +329,13 @@ class StateHandler(object):
 
             if conflicted_state:
                 logger.info("Resolving conflicted state for %r", room_id)
-                state_map = yield self.store.get_events(
-                    [e_id for st in state_groups_ids.values() for e_id in st.values()],
-                    get_prev_content=False
-                )
-                state_sets = [
-                    [state_map[e_id] for key, e_id in st.items() if e_id in state_map]
-                    for st in state_groups_ids.values()
-                ]
                 with Measure(self.clock, "state._resolve_events"):
-                    new_state, _ = resolve_events(
-                        state_sets, event_type, state_key
+                    new_state = yield resolve_events(
+                        state_groups_ids.values(),
+                        state_map_factory=lambda ev_ids: self.store.get_events(
+                            ev_ids, get_prev_content=False
+                        ),
                     )
-                new_state = {
-                    key: e.event_id for key, e in new_state.items()
-                }
             else:
                 new_state = {
                     key: e_ids.pop() for key, e_ids in state.items()
@@ -390,13 +383,25 @@ class StateHandler(object):
         logger.info(
             "Resolving state for %s with %d groups", event.room_id, len(state_sets)
         )
+        state_set_ids = [{
+            (ev.type, ev.state_key): ev.event_id
+            for ev in st
+        } for st in state_sets]
+
+        state_map = {
+            ev.event_id: ev
+            for st in state_sets
+            for ev in st
+        }
+
         with Measure(self.clock, "state._resolve_events"):
-            if event.is_state():
-                return resolve_events(
-                    state_sets, event.type, event.state_key
-                )
-            else:
-                return resolve_events(state_sets)
+            new_state = resolve_events(state_set_ids, state_map)
+
+        new_state = {
+            key: state_map[ev_id] for key, ev_id in new_state.items()
+        }
+
+        return new_state
 
 
 def _ordered_events(events):
@@ -406,43 +411,117 @@ def _ordered_events(events):
     return sorted(events, key=key_func)
 
 
-def resolve_events(state_sets, event_type=None, state_key=""):
+def resolve_events(state_sets, state_map_factory):
     """
+    Args:
+        state_sets(list): List of dicts of (type, state_key) -> event_id,
+            which are the different state groups to resolve.
+        state_map_factory(dict|callable): If callable, then will be called
+            with a list of event_ids that are needed, and should return with
+            a Deferred of dict of event_id to event. Otherwise, should be
+            a dict from event_id to event of all events in state_sets.
+
     Returns
-        (dict[(str, str), synapse.events.FrozenEvent], list[str]): a tuple
-        (new_state, prev_states). new_state is a map from (type, state_key)
-        to event. prev_states is a list of event_ids.
+        dict[(str, str), synapse.events.FrozenEvent] is a map from
+        (type, state_key) to event.
     """
-    state = {}
-    for st in state_sets:
-        for e in st:
-            state.setdefault(
-                (e.type, e.state_key),
-                {}
-            )[e.event_id] = e
-
-    unconflicted_state = {
-        k: v.values()[0] for k, v in state.items()
-        if len(v.values()) == 1
-    }
+    unconflicted_state, conflicted_state = _seperate(
+        state_sets,
+    )
 
+    if callable(state_map_factory):
+        return _resolve_with_state_fac(
+            unconflicted_state, conflicted_state, state_map_factory
+        )
+
+    state_map = state_map_factory
+
+    auth_events = _create_auth_events_from_maps(
+        unconflicted_state, conflicted_state, state_map
+    )
+
+    return _resolve_with_state(
+        unconflicted_state, conflicted_state, auth_events, state_map
+    )
+
+
+def _seperate(state_sets):
+    """Takes the state_sets and figures out which keys are conflicted and
+    which aren't. i.e., which have multiple different event_ids associated
+    with them in different state sets.
+    """
+    unconflicted_state = dict(state_sets[0])
+    conflicted_state = {}
+
+    full_states = defaultdict(
+        set,
+        {k: set((v,)) for k, v in state_sets[0].iteritems()}
+    )
+
+    for state_set in state_sets[1:]:
+        for key, value in state_set.iteritems():
+            ls = full_states[key]
+            if not ls:
+                ls.add(value)
+                unconflicted_state[key] = value
+            elif value not in ls:
+                ls.add(value)
+                if len(ls) == 2:
+                    conflicted_state[key] = ls
+                    unconflicted_state.pop(key, None)
+
+    return unconflicted_state, conflicted_state
+
+
+@defer.inlineCallbacks
+def _resolve_with_state_fac(unconflicted_state, conflicted_state,
+                            state_map_factory):
+    needed_events = set(
+        event_id
+        for event_ids in conflicted_state.itervalues()
+        for event_id in event_ids
+    )
+
+    state_map = yield state_map_factory(needed_events)
+
+    auth_events = _create_auth_events_from_maps(
+        unconflicted_state, conflicted_state, state_map
+    )
+
+    new_needed_events = set(auth_events.itervalues())
+    new_needed_events -= needed_events
+
+    state_map_new = yield state_map_factory(new_needed_events)
+    state_map.update(state_map_new)
+
+    defer.returnValue(_resolve_with_state(
+        unconflicted_state, conflicted_state, auth_events, state_map
+    ))
+
+
+def _create_auth_events_from_maps(unconflicted_state, conflicted_state, state_map):
+    auth_events = {}
+    for event_ids in conflicted_state.itervalues():
+        for event_id in event_ids:
+            keys = event_auth.auth_types_for_event(state_map[event_id])
+            for key in keys:
+                if key not in auth_events:
+                    event_id = unconflicted_state.get(key, None)
+                    if event_id:
+                        auth_events[key] = event_id
+    return auth_events
+
+
+def _resolve_with_state(unconflicted_state, conflicted_state, auth_events,
+                        state_map):
     conflicted_state = {
-        k: v.values()
-        for k, v in state.items()
-        if len(v.values()) > 1
+        key: [state_map[ev_id] for ev_id in event_ids]
+        for key, event_ids in conflicted_state.items()
     }
 
-    if event_type:
-        prev_states_events = conflicted_state.get(
-            (event_type, state_key), []
-        )
-        prev_states = [s.event_id for s in prev_states_events]
-    else:
-        prev_states = []
-
     auth_events = {
-        k: e for k, e in unconflicted_state.items()
-        if k[0] in AuthEventTypes
+        key: state_map[ev_id]
+        for key, ev_id in auth_events.items()
     }
 
     try:
@@ -454,9 +533,10 @@ def resolve_events(state_sets, event_type=None, state_key=""):
         raise
 
     new_state = unconflicted_state
-    new_state.update(resolved_state)
+    for key, event in resolved_state.iteritems():
+        new_state[key] = event.event_id
 
-    return new_state, prev_states
+    return new_state
 
 
 def _resolve_state_events(conflicted_state, auth_events):
@@ -470,11 +550,10 @@ def _resolve_state_events(conflicted_state, auth_events):
         4. other events.
     """
     resolved_state = {}
-    power_key = (EventTypes.PowerLevels, "")
-    if power_key in conflicted_state:
-        events = conflicted_state[power_key]
+    if POWER_KEY in conflicted_state:
+        events = conflicted_state[POWER_KEY]
         logger.debug("Resolving conflicted power levels %r", events)
-        resolved_state[power_key] = _resolve_auth_events(
+        resolved_state[POWER_KEY] = _resolve_auth_events(
             events, auth_events)
 
     auth_events.update(resolved_state)
@@ -512,14 +591,26 @@ def _resolve_state_events(conflicted_state, auth_events):
 def _resolve_auth_events(events, auth_events):
     reverse = [i for i in reversed(_ordered_events(events))]
 
-    auth_events = dict(auth_events)
+    auth_keys = set(
+        key
+        for event in events
+        for key in event_auth.auth_types_for_event(event)
+    )
+
+    new_auth_events = {}
+    for key in auth_keys:
+        auth_event = auth_events.get(key, None)
+        if auth_event:
+            new_auth_events[key] = auth_event
+
+    auth_events = new_auth_events
 
     prev_event = reverse[0]
     for event in reverse[1:]:
         auth_events[(prev_event.type, prev_event.state_key)] = prev_event
         try:
             # The signatures have already been checked at this point
-            event_auth.check(event, auth_events, do_sig_check=False)
+            event_auth.check(event, auth_events, do_sig_check=False, do_size_check=False)
             prev_event = event
         except AuthError:
             return prev_event
@@ -531,7 +622,7 @@ def _resolve_normal_events(events, auth_events):
     for event in _ordered_events(events):
         try:
             # The signatures have already been checked at this point
-            event_auth.check(event, auth_events, do_sig_check=False)
+            event_auth.check(event, auth_events, do_sig_check=False, do_size_check=False)
             return event
         except AuthError:
             pass
diff --git a/tests/api/test_filtering.py b/tests/api/test_filtering.py
index dcb6c5bc31..50e8607c14 100644
--- a/tests/api/test_filtering.py
+++ b/tests/api/test_filtering.py
@@ -25,10 +25,13 @@ from synapse.api.filtering import Filter
 from synapse.events import FrozenEvent
 
 user_localpart = "test_user"
-# MockEvent = namedtuple("MockEvent", "sender type room_id")
 
 
 def MockEvent(**kwargs):
+    if "event_id" not in kwargs:
+        kwargs["event_id"] = "fake_event_id"
+    if "type" not in kwargs:
+        kwargs["type"] = "fake_type"
     return FrozenEvent(kwargs)
 
 
diff --git a/tests/events/test_utils.py b/tests/events/test_utils.py
index 29f068d1f1..dfc870066e 100644
--- a/tests/events/test_utils.py
+++ b/tests/events/test_utils.py
@@ -21,6 +21,10 @@ from synapse.events.utils import prune_event, serialize_event
 
 
 def MockEvent(**kwargs):
+    if "event_id" not in kwargs:
+        kwargs["event_id"] = "fake_event_id"
+    if "type" not in kwargs:
+        kwargs["type"] = "fake_type"
     return FrozenEvent(kwargs)
 
 
@@ -35,9 +39,13 @@ class PruneEventTestCase(unittest.TestCase):
 
     def test_minimal(self):
         self.run_test(
-            {'type': 'A'},
             {
                 'type': 'A',
+                'event_id': '$test:domain',
+            },
+            {
+                'type': 'A',
+                'event_id': '$test:domain',
                 'content': {},
                 'signatures': {},
                 'unsigned': {},
@@ -69,10 +77,12 @@ class PruneEventTestCase(unittest.TestCase):
         self.run_test(
             {
                 'type': 'B',
+                'event_id': '$test:domain',
                 'unsigned': {'age_ts': 20},
             },
             {
                 'type': 'B',
+                'event_id': '$test:domain',
                 'content': {},
                 'signatures': {},
                 'unsigned': {'age_ts': 20},
@@ -82,10 +92,12 @@ class PruneEventTestCase(unittest.TestCase):
         self.run_test(
             {
                 'type': 'B',
+                'event_id': '$test:domain',
                 'unsigned': {'other_key': 'here'},
             },
             {
                 'type': 'B',
+                'event_id': '$test:domain',
                 'content': {},
                 'signatures': {},
                 'unsigned': {},
@@ -96,10 +108,12 @@ class PruneEventTestCase(unittest.TestCase):
         self.run_test(
             {
                 'type': 'C',
+                'event_id': '$test:domain',
                 'content': {'things': 'here'},
             },
             {
                 'type': 'C',
+                'event_id': '$test:domain',
                 'content': {},
                 'signatures': {},
                 'unsigned': {},
@@ -109,10 +123,12 @@ class PruneEventTestCase(unittest.TestCase):
         self.run_test(
             {
                 'type': 'm.room.create',
+                'event_id': '$test:domain',
                 'content': {'creator': '@2:domain', 'other_field': 'here'},
             },
             {
                 'type': 'm.room.create',
+                'event_id': '$test:domain',
                 'content': {'creator': '@2:domain'},
                 'signatures': {},
                 'unsigned': {},
@@ -255,6 +271,8 @@ class SerializeEventTestCase(unittest.TestCase):
         self.assertEquals(
             self.serialize(
                 MockEvent(
+                    type="foo",
+                    event_id="test",
                     room_id="!foo:bar",
                     content={
                         "foo": "bar",
@@ -263,6 +281,8 @@ class SerializeEventTestCase(unittest.TestCase):
                 []
             ),
             {
+                "type": "foo",
+                "event_id": "test",
                 "room_id": "!foo:bar",
                 "content": {
                     "foo": "bar",
-- 
cgit 1.5.1


From 633f97151c6c7fa693b3de4addad641186b4ef02 Mon Sep 17 00:00:00 2001
From: Erik Johnston <erik@matrix.org>
Date: Tue, 17 Jan 2017 13:33:54 +0000
Subject: Check event is in state_map

---
 synapse/state.py | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

(limited to 'synapse/state.py')

diff --git a/synapse/state.py b/synapse/state.py
index 294e0c2081..df9b6b3ccb 100644
--- a/synapse/state.py
+++ b/synapse/state.py
@@ -333,7 +333,7 @@ class StateHandler(object):
                     new_state = yield resolve_events(
                         state_groups_ids.values(),
                         state_map_factory=lambda ev_ids: self.store.get_events(
-                            ev_ids, get_prev_content=False
+                            ev_ids, get_prev_content=False, check_redacted=False,
                         ),
                     )
             else:
@@ -482,6 +482,8 @@ def _resolve_with_state_fac(unconflicted_state, conflicted_state,
         for event_id in event_ids
     )
 
+    logger.info("Asking for %d conflicted events", len(needed_events))
+
     state_map = yield state_map_factory(needed_events)
 
     auth_events = _create_auth_events_from_maps(
@@ -491,6 +493,8 @@ def _resolve_with_state_fac(unconflicted_state, conflicted_state,
     new_needed_events = set(auth_events.itervalues())
     new_needed_events -= needed_events
 
+    logger.info("Asking for %d auth events", len(new_needed_events))
+
     state_map_new = yield state_map_factory(new_needed_events)
     state_map.update(state_map_new)
 
@@ -515,13 +519,14 @@ def _create_auth_events_from_maps(unconflicted_state, conflicted_state, state_ma
 def _resolve_with_state(unconflicted_state, conflicted_state, auth_events,
                         state_map):
     conflicted_state = {
-        key: [state_map[ev_id] for ev_id in event_ids]
+        key: [state_map[ev_id] for ev_id in event_ids if ev_id in state_map]
         for key, event_ids in conflicted_state.items()
     }
 
     auth_events = {
         key: state_map[ev_id]
         for key, ev_id in auth_events.items()
+        if ev_id in state_map
     }
 
     try:
-- 
cgit 1.5.1


From ce59a2faad253409a8047ce9302d3d6c087fe812 Mon Sep 17 00:00:00 2001
From: Erik Johnston <erik@matrix.org>
Date: Tue, 17 Jan 2017 14:18:53 +0000
Subject: Correctly handle case of rejected events in state res

---
 synapse/state.py | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

(limited to 'synapse/state.py')

diff --git a/synapse/state.py b/synapse/state.py
index df9b6b3ccb..d2bd1ad646 100644
--- a/synapse/state.py
+++ b/synapse/state.py
@@ -507,21 +507,27 @@ def _create_auth_events_from_maps(unconflicted_state, conflicted_state, state_ma
     auth_events = {}
     for event_ids in conflicted_state.itervalues():
         for event_id in event_ids:
-            keys = event_auth.auth_types_for_event(state_map[event_id])
-            for key in keys:
-                if key not in auth_events:
-                    event_id = unconflicted_state.get(key, None)
-                    if event_id:
-                        auth_events[key] = event_id
+            if event_id in state_map:
+                keys = event_auth.auth_types_for_event(state_map[event_id])
+                for key in keys:
+                    if key not in auth_events:
+                        event_id = unconflicted_state.get(key, None)
+                        if event_id:
+                            auth_events[key] = event_id
     return auth_events
 
 
 def _resolve_with_state(unconflicted_state, conflicted_state, auth_events,
                         state_map):
-    conflicted_state = {
-        key: [state_map[ev_id] for ev_id in event_ids if ev_id in state_map]
-        for key, event_ids in conflicted_state.items()
-    }
+    new_conflicted_state = {}
+    for key, event_ids in conflicted_state.iteritems():
+        events = [state_map[ev_id] for ev_id in event_ids if ev_id in state_map]
+        if len(events) > 1:
+            new_conflicted_state[key] = events
+        elif len(events) == 1:
+            unconflicted_state[key] = events[0].event_id
+
+    conflicted_state = new_conflicted_state
 
     auth_events = {
         key: state_map[ev_id]
-- 
cgit 1.5.1


From a8594fd19f48a179b263d58ba1f9c5ab2f4cb8d3 Mon Sep 17 00:00:00 2001
From: Erik Johnston <erik@matrix.org>
Date: Tue, 17 Jan 2017 14:59:03 +0000
Subject: Use better names

---
 synapse/state.py | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

(limited to 'synapse/state.py')

diff --git a/synapse/state.py b/synapse/state.py
index d2bd1ad646..6f62876f8c 100644
--- a/synapse/state.py
+++ b/synapse/state.py
@@ -517,21 +517,19 @@ def _create_auth_events_from_maps(unconflicted_state, conflicted_state, state_ma
     return auth_events
 
 
-def _resolve_with_state(unconflicted_state, conflicted_state, auth_events,
+def _resolve_with_state(unconflicted_state_ids, conflicted_state_ds, auth_event_ids,
                         state_map):
-    new_conflicted_state = {}
-    for key, event_ids in conflicted_state.iteritems():
+    conflicted_state = {}
+    for key, event_ids in conflicted_state_ds.iteritems():
         events = [state_map[ev_id] for ev_id in event_ids if ev_id in state_map]
         if len(events) > 1:
-            new_conflicted_state[key] = events
+            conflicted_state[key] = events
         elif len(events) == 1:
-            unconflicted_state[key] = events[0].event_id
-
-    conflicted_state = new_conflicted_state
+            unconflicted_state_ids[key] = events[0].event_id
 
     auth_events = {
         key: state_map[ev_id]
-        for key, ev_id in auth_events.items()
+        for key, ev_id in auth_event_ids.items()
         if ev_id in state_map
     }
 
@@ -543,7 +541,7 @@ def _resolve_with_state(unconflicted_state, conflicted_state, auth_events,
         logger.exception("Failed to resolve state")
         raise
 
-    new_state = unconflicted_state
+    new_state = unconflicted_state_ids
     for key, event in resolved_state.iteritems():
         new_state[key] = event.event_id
 
-- 
cgit 1.5.1


From c6064a7ba6bae6055dc7960e5eef3956131b718d Mon Sep 17 00:00:00 2001
From: Mark Haines <mark.haines@matrix.org>
Date: Tue, 17 Jan 2017 15:23:07 +0000
Subject: Only construct sets when necessary

---
 synapse/state.py | 33 +++++++++++++++++++--------------
 1 file changed, 19 insertions(+), 14 deletions(-)

(limited to 'synapse/state.py')

diff --git a/synapse/state.py b/synapse/state.py
index 6f62876f8c..81c6bae737 100644
--- a/synapse/state.py
+++ b/synapse/state.py
@@ -453,22 +453,27 @@ def _seperate(state_sets):
     unconflicted_state = dict(state_sets[0])
     conflicted_state = {}
 
-    full_states = defaultdict(
-        set,
-        {k: set((v,)) for k, v in state_sets[0].iteritems()}
-    )
-
     for state_set in state_sets[1:]:
         for key, value in state_set.iteritems():
-            ls = full_states[key]
-            if not ls:
-                ls.add(value)
-                unconflicted_state[key] = value
-            elif value not in ls:
-                ls.add(value)
-                if len(ls) == 2:
-                    conflicted_state[key] = ls
-                    unconflicted_state.pop(key, None)
+            # Check if there is an unconflicted entry for the state key.
+            unconflicted_value = unconflicted_state.get(key)
+            if unconflicted_value is None:
+                # There isn't an unconflicted entry so check if there is a
+                # conflicted entry.
+                ls = conflicted_state.get(key)
+                if ls is None:
+                    # There wasn't a conflicted entry so haven't seen this key before.
+                    # Therefore it isn't conflicted yet.
+                    unconflicted_state[key] = value
+                else:
+                    # This key is already conflicted, add our value to the conflict set.
+                    ls.add(value)
+            elif unconflicted_value != value:
+                # If the unconflicted value is not the same as our value then we
+                # have a new conflict. So move the key from the unconflicted_state
+                # to the conflicted state.
+                conflicted_state[key] = {value, unconflicted_value}
+                unconflicted_state.pop(key, None)
 
     return unconflicted_state, conflicted_state
 
-- 
cgit 1.5.1


From ed4d1761525b21989279b99733e415c1c86ed39f Mon Sep 17 00:00:00 2001
From: Erik Johnston <erik@matrix.org>
Date: Tue, 17 Jan 2017 15:27:28 +0000
Subject: PEP8

---
 synapse/state.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'synapse/state.py')

diff --git a/synapse/state.py b/synapse/state.py
index 81c6bae737..15238cd00c 100644
--- a/synapse/state.py
+++ b/synapse/state.py
@@ -25,7 +25,7 @@ from synapse.api.errors import AuthError
 from synapse.events.snapshot import EventContext
 from synapse.util.async import Linearizer
 
-from collections import namedtuple, defaultdict
+from collections import namedtuple
 from frozendict import frozendict
 
 import logging
-- 
cgit 1.5.1


From 5f027d1fc54ab51b420e3deb25d83ac05676fdbf Mon Sep 17 00:00:00 2001
From: Erik Johnston <erik@matrix.org>
Date: Tue, 17 Jan 2017 17:07:15 +0000
Subject: Change resolve_state_groups call site logging to DEBUG

---
 synapse/api/auth.py            | 2 +-
 synapse/handlers/federation.py | 2 +-
 synapse/state.py               | 8 ++++----
 3 files changed, 6 insertions(+), 6 deletions(-)

(limited to 'synapse/state.py')

diff --git a/synapse/api/auth.py b/synapse/api/auth.py
index 280d4c4452..03a215ab1b 100644
--- a/synapse/api/auth.py
+++ b/synapse/api/auth.py
@@ -146,7 +146,7 @@ class Auth(object):
         with Measure(self.clock, "check_host_in_room"):
             latest_event_ids = yield self.store.get_latest_event_ids_in_room(room_id)
 
-            logger.info("calling resolve_state_groups from check_host_in_room")
+            logger.debug("calling resolve_state_groups from check_host_in_room")
             entry = yield self.state.resolve_state_groups(
                 room_id, latest_event_ids
             )
diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py
index 1021bcc405..2e310fed71 100644
--- a/synapse/handlers/federation.py
+++ b/synapse/handlers/federation.py
@@ -591,7 +591,7 @@ class FederationHandler(BaseHandler):
 
         event_ids = list(extremities.keys())
 
-        logger.info("calling resolve_state_groups in _maybe_backfill")
+        logger.debug("calling resolve_state_groups in _maybe_backfill")
         states = yield preserve_context_over_deferred(defer.gatherResults([
             preserve_fn(self.state_handler.resolve_state_groups)(room_id, [e])
             for e in event_ids
diff --git a/synapse/state.py b/synapse/state.py
index 5028b0ac49..1c6e31ac58 100644
--- a/synapse/state.py
+++ b/synapse/state.py
@@ -128,7 +128,7 @@ class StateHandler(object):
         if not latest_event_ids:
             latest_event_ids = yield self.store.get_latest_event_ids_in_room(room_id)
 
-        logger.info("calling resolve_state_groups from get_current_state")
+        logger.debug("calling resolve_state_groups from get_current_state")
         ret = yield self.resolve_state_groups(room_id, latest_event_ids)
         state = ret.state
 
@@ -153,7 +153,7 @@ class StateHandler(object):
         if not latest_event_ids:
             latest_event_ids = yield self.store.get_latest_event_ids_in_room(room_id)
 
-        logger.info("calling resolve_state_groups from get_current_state_ids")
+        logger.debug("calling resolve_state_groups from get_current_state_ids")
         ret = yield self.resolve_state_groups(room_id, latest_event_ids)
         state = ret.state
 
@@ -167,7 +167,7 @@ class StateHandler(object):
     def get_current_user_in_room(self, room_id, latest_event_ids=None):
         if not latest_event_ids:
             latest_event_ids = yield self.store.get_latest_event_ids_in_room(room_id)
-        logger.info("calling resolve_state_groups from get_current_user_in_room")
+        logger.debug("calling resolve_state_groups from get_current_user_in_room")
         entry = yield self.resolve_state_groups(room_id, latest_event_ids)
         joined_users = yield self.store.get_joined_users_from_state(
             room_id, entry.state_id, entry.state
@@ -231,7 +231,7 @@ class StateHandler(object):
             context.prev_state_events = []
             defer.returnValue(context)
 
-        logger.info("calling resolve_state_groups from compute_event_context")
+        logger.debug("calling resolve_state_groups from compute_event_context")
         if event.is_state():
             entry = yield self.resolve_state_groups(
                 event.room_id, [e for e, _ in event.prev_events],
-- 
cgit 1.5.1