Make /sync return heroes if room name or canonical alias are empty #5089

k80w · 2019-04-22T17:08:20Z

This closes issue #4194

Fixes matrix-org#4194 Signed-off-by: Katie Wolfe <katie@dnaf.moe>

Signed-off-by: Katie Wolfe <katie@dnaf.moe>

Fixes matrix-org#4194 Signed-off-by: Katie Wolfe <katie@dnaf.moe>

Signed-off-by: Katie Wolfe <katie@dnaf.moe>

I probably should've just run autopep8 in the first place... Signed-off-by: Katie Wolfe <katie@dnaf.moe>

richvdh · 2019-05-10T10:25:27Z

Hi, and thanks very much for your work on this. However, I think you've stumbled into a hornet's nest, which is the lack of specification for this feature (see matrix-org/matrix-spec-proposals#1868).

I feel quite strongly that if this is a thing that's important enough to change, then it ought to be specced and tested to make sure that clients agree on it being the right thing to do, and future server implementations do the same thing.

In other words: I don't think we can progress this until we have a spec for m.heroes, and sytests to validate that it is followed.

richvdh · 2019-05-10T10:27:21Z

[a bunch of the CI has failed here due to a problem which is fixed in latest develop.]

codecov · 2019-06-05T13:09:36Z

Codecov Report

❗ No coverage uploaded for pull request base (develop@2615c6b). Click here to learn what that means.
The diff coverage is 0%.

@@            Coverage Diff             @@
##             develop    #5089   +/-   ##
==========================================
  Coverage           ?   63.03%           
==========================================
  Files              ?      341           
  Lines              ?    35629           
  Branches           ?     5834           
==========================================
  Hits               ?    22458           
  Misses             ?    11600           
  Partials           ?     1571

codecov · 2019-06-05T13:09:37Z

Codecov Report

❗ No coverage uploaded for pull request base (develop@2615c6b). Click here to learn what that means.
The diff coverage is 0%.

@@            Coverage Diff             @@
##             develop    #5089   +/-   ##
==========================================
  Coverage           ?   63.03%           
==========================================
  Files              ?      341           
  Lines              ?    35629           
  Branches           ?     5834           
==========================================
  Hits               ?    22458           
  Misses             ?    11602           
  Partials           ?     1569

babolivier · 2019-06-05T13:15:43Z

Given we're looking at some lazy loading bugs in the context of the upcoming Synapse 1.0, I've done some updates on this PR in the hope to get it merged. What I've done is merging develop in to unwedge the CI, and removing [event].content from the condition since the spec forbids the content of m.room.name and m.room.canonical_alias events from being either null or an empty dict (and [event].content.get covers the latter anyway).

synapse/handlers/sync.py

babolivier · 2019-06-06T10:18:41Z

One more checked off the list, thanks for your work @dnaf! 🙂

@dnaf

Synapse 1.0.0rc1 (2019-06-07) ============================= Features -------- - Synapse now more efficiently collates room statistics. ([\#4338](#4338), [\#5260](#5260), [\#5324](#5324)) - Add experimental support for relations (aka reactions and edits). ([\#5220](#5220)) - Ability to configure default room version. ([\#5223](#5223), [\#5249](#5249)) - Allow configuring a range for the account validity startup job. ([\#5276](#5276)) - CAS login will now hit the r0 API, not the deprecated v1 one. ([\#5286](#5286)) - Validate federation server TLS certificates by default (implements [MSC1711](https://github.com/matrix-org/matrix-doc/blob/master/proposals/1711-x509-for-federation.md)). ([\#5359](#5359)) - Update /_matrix/client/versions to reference support for r0.5.0. ([\#5360](#5360)) - Add a script to generate new signing-key files. ([\#5361](#5361)) - Update upgrade and installation guides ahead of 1.0. ([\#5371](#5371)) - Replace the `perspectives` configuration section with `trusted_key_servers`, and make validating the signatures on responses optional (since TLS will do this job for us). ([\#5374](#5374)) - Add ability to perform password reset via email without trusting the identity server. ([\#5377](#5377)) - Set default room version to v4. ([\#5379](#5379)) Bugfixes -------- - Fixes client-server API not sending "m.heroes" to lazy-load /sync requests when a rooms name or its canonical alias are empty. Thanks to @dnaf for this work! ([\#5089](#5089)) - Prevent federation device list updates breaking when processing multiple updates at once. ([\#5156](#5156)) - Fix worker registration bug caused by ClientReaderSlavedStore being unable to see get_profileinfo. ([\#5200](#5200)) - Fix race when backfilling in rooms with worker mode. ([\#5221](#5221)) - Fix appservice timestamp massaging. ([\#5233](#5233)) - Ensure that server_keys fetched via a notary server are correctly signed. ([\#5251](#5251)) - Show the correct error when logging out and access token is missing. ([\#5256](#5256)) - Fix error code when there is an invalid parameter on /_matrix/client/r0/publicRooms ([\#5257](#5257)) - Fix error when downloading thumbnail with missing width/height parameter. ([\#5258](#5258)) - Fix schema update for account validity. ([\#5268](#5268)) - Fix bug where we leaked extremities when we soft failed events, leading to performance degradation. ([\#5274](#5274), [\#5278](#5278), [\#5291](#5291)) - Fix "db txn 'update_presence' from sentinel context" log messages. ([\#5275](#5275)) - Fix dropped logcontexts during high outbound traffic. ([\#5277](#5277)) - Fix a bug where it is not possible to get events in the federation format with the request `GET /_matrix/client/r0/rooms/{roomId}/messages`. ([\#5293](#5293)) - Fix performance problems with the rooms stats background update. ([\#5294](#5294)) - Fix noisy 'no key for server' logs. ([\#5300](#5300)) - Fix bug where a notary server would sometimes forget old keys. ([\#5307](#5307)) - Prevent users from setting huge displaynames and avatar URLs. ([\#5309](#5309)) - Fix handling of failures when processing incoming events where calling `/event_auth` on remote server fails. ([\#5317](#5317)) - Ensure that we have an up-to-date copy of the signing key when validating incoming federation requests. ([\#5321](#5321)) - Fix various problems which made the signing-key notary server time out for some requests. ([\#5333](#5333)) - Fix bug which would make certain operations (such as room joins) block for 20 minutes while attemoting to fetch verification keys. ([\#5334](#5334)) - Fix a bug where we could rapidly mark a server as unreachable even though it was only down for a few minutes. ([\#5335](#5335), [\#5340](#5340)) - Fix a bug where account validity renewal emails could only be sent when email notifs were enabled. ([\#5341](#5341)) - Fix failure when fetching batches of events during backfill, etc. ([\#5342](#5342)) - Add a new room version where the timestamps on events are checked against the validity periods on signing keys. ([\#5348](#5348), [\#5354](#5354)) - Fix room stats and presence background updates to correctly handle missing events. ([\#5352](#5352)) - Include left members in room summaries' heroes. ([\#5355](#5355)) - Fix `federation_custom_ca_list` configuration option. ([\#5362](#5362)) - Fix missing logcontext warnings on shutdown. ([\#5369](#5369)) Improved Documentation ---------------------- - Fix docs on resetting the user directory. ([\#5282](#5282)) - Fix notes about ACME in the MSC1711 faq. ([\#5357](#5357)) Internal Changes ---------------- - Synapse will now serve the experimental "room complexity" API endpoint. ([\#5216](#5216)) - The base classes for the v1 and v2_alpha REST APIs have been unified. ([\#5226](#5226), [\#5328](#5328)) - Simplifications and comments in do_auth. ([\#5227](#5227)) - Remove urllib3 pin as requests 2.22.0 has been released supporting urllib3 1.25.2. ([\#5230](#5230)) - Preparatory work for key-validity features. ([\#5232](#5232), [\#5234](#5234), [\#5235](#5235), [\#5236](#5236), [\#5237](#5237), [\#5244](#5244), [\#5250](#5250), [\#5296](#5296), [\#5299](#5299), [\#5343](#5343), [\#5347](#5347), [\#5356](#5356)) - Specify the type of reCAPTCHA key to use. ([\#5283](#5283)) - Improve sample config for monthly active user blocking. ([\#5284](#5284)) - Remove spurious debug from MatrixFederationHttpClient.get_json. ([\#5287](#5287)) - Improve logging for logcontext leaks. ([\#5288](#5288)) - Clarify that the admin change password API logs the user out. ([\#5303](#5303)) - New installs will now use the v54 full schema, rather than the full schema v14 and applying incremental updates to v54. ([\#5320](#5320)) - Improve docstrings on MatrixFederationClient. ([\#5332](#5332)) - Clean up FederationClient.get_events for clarity. ([\#5344](#5344)) - Various improvements to debug logging. ([\#5353](#5353)) - Don't run CI build checks until sample config check has passed. ([\#5370](#5370)) - Automatically retry buildkite builds (max twice) when an agent is lost. ([\#5380](#5380))

k80w force-pushed the m-heroes-empty-room-name branch from 7047ba1 to adcb204 Compare April 24, 2019 16:18

k80w added 5 commits April 24, 2019 12:26

Show heroes if room name or canonical alias are empty

7f025eb

Fixes matrix-org#4194 Signed-off-by: Katie Wolfe <katie@dnaf.moe>

Add changelog.d/5084.bugfix

0a4c135

Signed-off-by: Katie Wolfe <katie@dnaf.moe>

Add full stop to 5084.bugfix

60041ea

Signed-off-by: Katie Wolfe <katie@dnaf.moe>

Show heroes if room name or canonical alias are empty

5d3ed79

Fixes matrix-org#4194 Signed-off-by: Katie Wolfe <katie@dnaf.moe>

Clean up code

b3e5db4

Signed-off-by: Katie Wolfe <katie@dnaf.moe>

k80w force-pushed the m-heroes-empty-room-name branch from adcb204 to b3e5db4 Compare April 24, 2019 16:27

Lint

7e07dc4

I probably should've just run autopep8 in the first place... Signed-off-by: Katie Wolfe <katie@dnaf.moe>

richvdh requested a review from a team May 9, 2019 22:23

richvdh mentioned this pull request May 10, 2019

Always calculate heroes for rooms without avatars #3898

Closed

richvdh removed the request for review from a team May 10, 2019 10:54

babolivier added 2 commits June 5, 2019 13:38

Merge branch 'develop' into m-heroes-empty-room-name

40596ae

Simplify condition

0a2f522

babolivier requested a review from a team June 5, 2019 13:16

babolivier added the Synapse v1.0 label Jun 5, 2019

erikjohnston approved these changes Jun 6, 2019

View reviewed changes

erikjohnston reviewed Jun 6, 2019

View reviewed changes

synapse/handlers/sync.py Outdated Show resolved Hide resolved

babolivier added 2 commits June 6, 2019 10:34

Simplify condition

64fa928

Add credit in the changelog

7898a1a

babolivier merged commit 8f06344 into matrix-org:develop Jun 6, 2019

babolivier mentioned this pull request Jun 6, 2019

Lazy loading: the CS API should provide heroes if room name is "" #4194

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make /sync return heroes if room name or canonical alias are empty #5089

Make /sync return heroes if room name or canonical alias are empty #5089

k80w commented Apr 22, 2019

richvdh commented May 10, 2019

richvdh commented May 10, 2019

codecov bot commented Jun 5, 2019

codecov bot commented Jun 5, 2019 •

edited

Loading

babolivier commented Jun 5, 2019 •

edited

Loading

babolivier commented Jun 6, 2019

Make /sync return heroes if room name or canonical alias are empty #5089

Make /sync return heroes if room name or canonical alias are empty #5089

Conversation

k80w commented Apr 22, 2019

richvdh commented May 10, 2019

richvdh commented May 10, 2019

codecov bot commented Jun 5, 2019

Codecov Report

codecov bot commented Jun 5, 2019 • edited Loading

Codecov Report

babolivier commented Jun 5, 2019 • edited Loading

babolivier commented Jun 6, 2019

codecov bot commented Jun 5, 2019 •

edited

Loading

babolivier commented Jun 5, 2019 •

edited

Loading