Address code review comments

Signed-off-by: Olivier Wilkinson (reivilibre) <olivier@librepush.net>
This commit is contained in:
Olivier Wilkinson (reivilibre) 2019-08-28 10:54:22 +01:00
parent dfb22fec48
commit 81aa6d53b0

View file

@ -76,20 +76,18 @@ class StatsHandler(StateDeltasHandler):
# we only want to run this process one-at-a-time, # we only want to run this process one-at-a-time,
# and also, if the initial background updater wants us to keep out, # and also, if the initial background updater wants us to keep out,
# we should respect that. # we should respect that.
try:
run_as_background_process("stats.notify_new_event", process) run_as_background_process("stats.notify_new_event", process)
except: # noqa: E722 re-raised so fine
lock.release()
raise
@defer.inlineCallbacks @defer.inlineCallbacks
def _unsafe_process(self): def _unsafe_process(self):
# If self.pos is None then means we haven't fetched it from DB # If self.pos is None then means we haven't fetched it from DB
# If None is one of the values, then means that the stats regenerator has not (or had not) yet unwedged us
# but note that this might be outdated, so we retrieve the positions again.
if self.pos is None or None in self.pos.values(): if self.pos is None or None in self.pos.values():
self.pos = yield self.store.get_stats_positions() self.pos = yield self.store.get_stats_positions()
# If still None then the initial background update hasn't started yet # If still contains a None position, then the stats regenerator hasn't started yet
if self.pos is None or None in self.pos.values(): if None in self.pos.values():
return None return None
# Loop round handling deltas until we're up to date # Loop round handling deltas until we're up to date
@ -141,7 +139,10 @@ class StatsHandler(StateDeltasHandler):
continue continue
if event_id is None and prev_event_id is None: if event_id is None and prev_event_id is None:
# Errr... logger.error(
"event ID is None and so is the previous event ID. stream_id: %s",
stream_id,
)
continue continue
event_content = {} event_content = {}
@ -153,11 +154,14 @@ class StatsHandler(StateDeltasHandler):
# We use stream_pos here rather than fetch by event_id as event_id # We use stream_pos here rather than fetch by event_id as event_id
# may be None # may be None
now = yield self.store.get_received_ts_by_stream_pos(stream_pos) stream_timestamp = yield self.store.get_received_ts_by_stream_pos(
now = int(now) stream_pos
)
stream_timestamp = int(stream_timestamp)
# All the values in this dict are deltas (RELATIVE changes)
room_stats_delta = {} room_stats_delta = {}
room_stats_complete = False is_newly_created = False
if prev_event_id is None: if prev_event_id is None:
# this state event doesn't overwrite another, # this state event doesn't overwrite another,
@ -198,9 +202,9 @@ class StatsHandler(StateDeltasHandler):
elif prev_membership == Membership.BAN: elif prev_membership == Membership.BAN:
room_stats_delta["banned_members"] = -1 room_stats_delta["banned_members"] = -1
else: else:
err = "%s is not a valid prev_membership" % (repr(prev_membership),) raise ValueError(
logger.error(err) "%r is not a valid prev_membership" % (prev_membership,)
raise ValueError(err) )
if membership == prev_membership: if membership == prev_membership:
pass # noop pass # noop
@ -213,9 +217,7 @@ class StatsHandler(StateDeltasHandler):
elif membership == Membership.BAN: elif membership == Membership.BAN:
room_stats_delta["banned_members"] = +1 room_stats_delta["banned_members"] = +1
else: else:
err = "%s is not a valid membership" % (repr(membership),) raise ValueError("%r is not a valid membership" % (membership,))
logger.error(err)
raise ValueError(err)
user_id = state_key user_id = state_key
if self.is_mine_id(user_id): if self.is_mine_id(user_id):
@ -232,7 +234,7 @@ class StatsHandler(StateDeltasHandler):
delta = +1 if membership == Membership.JOIN else -1 delta = +1 if membership == Membership.JOIN else -1
yield self.store.update_stats_delta( yield self.store.update_stats_delta(
now, "user", user_id, {field: delta} stream_timestamp, "user", user_id, {field: delta}
) )
elif typ == EventTypes.Create: elif typ == EventTypes.Create:
@ -250,7 +252,7 @@ class StatsHandler(StateDeltasHandler):
}, },
) )
room_stats_complete = True is_newly_created = True
elif typ == EventTypes.JoinRules: elif typ == EventTypes.JoinRules:
old_room_state = yield self.store.get_room_state(room_id) old_room_state = yield self.store.get_room_state(room_id)
@ -269,7 +271,9 @@ class StatsHandler(StateDeltasHandler):
prev_event_id, event_id, "join_rule", JoinRules.PUBLIC prev_event_id, event_id, "join_rule", JoinRules.PUBLIC
) )
if is_public is not None: if is_public is not None:
yield self.update_public_room_stats(now, room_id, is_public) yield self.update_public_room_stats(
stream_timestamp, room_id, is_public
)
elif typ == EventTypes.RoomHistoryVisibility: elif typ == EventTypes.RoomHistoryVisibility:
old_room_state = yield self.store.get_room_state(room_id) old_room_state = yield self.store.get_room_state(room_id)
@ -289,7 +293,9 @@ class StatsHandler(StateDeltasHandler):
prev_event_id, event_id, "history_visibility", "world_readable" prev_event_id, event_id, "history_visibility", "world_readable"
) )
if is_public is not None: if is_public is not None:
yield self.update_public_room_stats(now, room_id, is_public) yield self.update_public_room_stats(
stream_timestamp, room_id, is_public
)
elif typ == EventTypes.Encryption: elif typ == EventTypes.Encryption:
yield self.store.update_room_state( yield self.store.update_room_state(
@ -312,9 +318,9 @@ class StatsHandler(StateDeltasHandler):
room_id, {"canonical_alias": event_content.get("alias")} room_id, {"canonical_alias": event_content.get("alias")}
) )
if room_stats_complete: if is_newly_created:
yield self.store.update_stats_delta( yield self.store.update_stats_delta(
now, stream_timestamp,
"room", "room",
room_id, room_id,
room_stats_delta, room_stats_delta,
@ -323,7 +329,7 @@ class StatsHandler(StateDeltasHandler):
elif len(room_stats_delta) > 0: elif len(room_stats_delta) > 0:
yield self.store.update_stats_delta( yield self.store.update_stats_delta(
now, "room", room_id, room_stats_delta stream_timestamp, "room", room_id, room_stats_delta
) )
@defer.inlineCallbacks @defer.inlineCallbacks