Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Add the Notifications API #1028

Merged
merged 8 commits into from Aug 22, 2016

Conversation

Projects
None yet
3 participants

dbkr added some commits May 23, 2016

@erikjohnston erikjohnston commented on an outdated diff Aug 18, 2016

synapse/rest/client/v2_alpha/notifications.py
+ notif_events[pa["event_id"]],
+ self.clock.time_msec(),
+ event_format=format_event_for_client_v2_without_room_id,
+ ),
+ }
+
+ if pa["room_id"] not in receipts_by_room:
+ returned_pa["read"] = False
+ else:
+ receipt = receipts_by_room[pa["room_id"]]
+
+ returned_pa["read"] = (
+ receipt["topological_ordering"] >= pa["topological_ordering"] or (
+ receipt["topological_ordering"] == pa["topological_ordering"] and
+ receipt["stream_ordering"] >= pa["stream_ordering"]
+ )
@erikjohnston

erikjohnston Aug 18, 2016

Owner

(5, 3) > (5,2) works as expected

@erikjohnston erikjohnston commented on an outdated diff Aug 18, 2016

synapse/storage/receipts.py
@@ -95,6 +95,31 @@ def get_receipts_for_user(self, user_id, receipt_type):
defer.returnValue({row["room_id"]: row["event_id"] for row in rows})
@defer.inlineCallbacks
+ def get_receipts_for_user_with_orderings(self, user_id, receipt_type):
+ def f(txn):
+ sql = (
+ "SELECT rl.room_id, rl.event_id,"
+ " e.topological_ordering, e.stream_ordering"
+ " FROM receipts_linearized rl,"
+ " events e"
+ " WHERE rl.room_id = e.room_id"
@erikjohnston

erikjohnston Aug 18, 2016

Owner

Generally I prefer to use explicit joins, as it makes it easier to see what the actual conditions are. i.e.:

SELECT rl.room_id, rl.event_id, e.topological_ordering, e.stream_ordering FROM receipts_linearized AS rl
INNER JOIN events AS e USING (room_id, event_id)
WHERE user_id = ?
Owner

erikjohnston commented Aug 18, 2016

This looks OK to me, though am slightly scared by performance. I'm happy to merge this on the condition that we have a rethink if hitting this with @ara4n's account makes the server cry.

dbkr and others added some commits Aug 18, 2016

Use tuple comparison
Hopefully easier to read

@erikjohnston erikjohnston merged commit d143f21 into develop Aug 22, 2016

10 checks passed

Flake8 + Packaging (Commit) Build #1543 origin/dbkr/notifications_api succeeded in 41 sec
Details
Flake8 + Packaging (Merged PR) Build finished.
Details
Sytest Dendron (Commit) Build #633 origin/dbkr/notifications_api succeeded in 9 min 10 sec
Details
Sytest Dendron (Merged PR) Build finished.
Details
Sytest Postgres (Commit) Build #1462 origin/dbkr/notifications_api succeeded in 6 min 50 sec
Details
Sytest Postgres (Merged PR) Build finished.
Details
Sytest SQLite (Commit) Build #1506 origin/dbkr/notifications_api succeeded in 6 min 5 sec
Details
Sytest SQLite (Merged PR) Build finished.
Details
Unit Tests (Commit) Build #1578 origin/dbkr/notifications_api succeeded in 2 min 57 sec
Details
Unit Tests (Merged PR) Build finished.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment