From c84937b85292c31411eb2329570fdbf1903582e7 Mon Sep 17 00:00:00 2001 From: Till <2353100+S7evinK@users.noreply.github.com> Date: Tue, 5 Apr 2022 15:32:30 +0200 Subject: [PATCH 1/5] Add Are We Synapse Yet to GHA (#2321) * Add Are We Synapse Yet to GHA * Better output & add comments --- .github/workflows/dendrite.yml | 9 ++++++++- are-we-synapse-yet.py | 9 +++++---- show-expected-fail-tests.sh | 16 ++++++++-------- 3 files changed, 21 insertions(+), 13 deletions(-) diff --git a/.github/workflows/dendrite.yml b/.github/workflows/dendrite.yml index 99747f798..c80a82d1d 100644 --- a/.github/workflows/dendrite.yml +++ b/.github/workflows/dendrite.yml @@ -233,7 +233,14 @@ jobs: - name: Summarise results.tap if: ${{ always() }} run: /sytest/scripts/tap_to_gha.pl /logs/results.tap - + - name: Sytest List Maintenance + if: ${{ always() }} + run: /src/show-expected-fail-tests.sh /logs/results.tap /src/sytest-whitelist /src/sytest-blacklist + continue-on-error: true # not fatal + - name: Are We Synapse Yet? + if: ${{ always() }} + run: /src/are-we-synapse-yet.py /logs/results.tap -v + continue-on-error: true # not fatal - name: Upload Sytest logs uses: actions/upload-artifact@v2 if: ${{ always() }} diff --git a/are-we-synapse-yet.py b/are-we-synapse-yet.py index 10b1be28a..8d551575f 100755 --- a/are-we-synapse-yet.py +++ b/are-we-synapse-yet.py @@ -3,7 +3,7 @@ from __future__ import division import argparse import re -import sys +import os # Usage: $ ./are-we-synapse-yet.py [-v] results.tap # This script scans a results.tap file from Dendrite's CI process and spits out @@ -156,6 +156,7 @@ def parse_test_line(line): # ✓ POST /register downcases capitals in usernames # ... def print_stats(header_name, gid_to_tests, gid_to_name, verbose): + ci = os.getenv("CI") # When running from GHA, this groups the subsections subsections = [] # Registration: 100% (13/13 tests) subsection_test_names = {} # 'subsection name': ["✓ Test 1", "✓ Test 2", "× Test 3"] total_passing = 0 @@ -169,7 +170,7 @@ def print_stats(header_name, gid_to_tests, gid_to_name, verbose): for name, passing in tests.items(): if passing: group_passing += 1 - test_names_and_marks.append(f"{'✓' if passing else '×'} {name}") + test_names_and_marks.append(f"{'✅' if passing else '❌'} {name}") total_tests += group_total total_passing += group_passing @@ -186,11 +187,11 @@ def print_stats(header_name, gid_to_tests, gid_to_name, verbose): print("%s: %s (%d/%d tests)" % (header_name, pct, total_passing, total_tests)) print("-" * (len(header_name)+1)) for line in subsections: - print(" %s" % (line,)) + print("%s%s" % ("::group::" if ci and verbose else "", line,)) if verbose: for test_name_and_pass_mark in subsection_test_names[line]: print(" %s" % (test_name_and_pass_mark,)) - print("") + print("%s" % ("::endgroup::" if ci else "")) print("") def main(results_tap_path, verbose): diff --git a/show-expected-fail-tests.sh b/show-expected-fail-tests.sh index 320d4ebd3..3ed937a0f 100755 --- a/show-expected-fail-tests.sh +++ b/show-expected-fail-tests.sh @@ -89,17 +89,17 @@ if [ -n "${tests_to_add}" ] && [ -n "${already_in_whitelist}" ]; then fi if [ -n "${tests_to_add}" ]; then - echo "**ERROR**: The following tests passed but are not present in \`$2\`. Please append them to the file:" - echo "\`\`\`" - echo -e "${tests_to_add}" - echo "\`\`\`" + echo "::error::The following tests passed but are not present in \`$2\`. Please append them to the file:" + echo "::group::Passing tests" + echo -e "${tests_to_add}" + echo "::endgroup::" fi if [ -n "${already_in_whitelist}" ]; then - echo "**WARN**: Tests in the whitelist still marked as **expected fail**:" - echo "\`\`\`" - echo -e "${already_in_whitelist}" - echo "\`\`\`" + echo "::warning::Tests in the whitelist still marked as **expected fail**:" + echo "::group::Still marked as expected fail" + echo -e "${already_in_whitelist}" + echo "::endgroup::" fi exit ${fail_build} From 4d9d9cc9b105ec5a4cd64dbaca000d0592c8c226 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Tue, 5 Apr 2022 14:43:44 +0100 Subject: [PATCH 2/5] Update to matrix-org/gomatrixserverlib#300 --- federationapi/internal/perform.go | 10 +++++----- go.mod | 3 +-- go.sum | 8 ++++---- roomserver/internal/input/input_missing.go | 7 ++++--- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/federationapi/internal/perform.go b/federationapi/internal/perform.go index b888b3654..8cd944346 100644 --- a/federationapi/internal/perform.go +++ b/federationapi/internal/perform.go @@ -392,17 +392,17 @@ func (r *FederationInternalAPI) performOutboundPeekUsingServer( // we have the peek state now so let's process regardless of whether upstream gives up ctx = context.Background() - respState := respPeek.ToRespState() - authEvents := respState.AuthEvents.UntrustedEvents(respPeek.RoomVersion) + // authenticate the state returned (check its auth events etc) // the equivalent of CheckSendJoinResponse() + authEvents, _, err := respState.Check(ctx, respPeek.RoomVersion, r.keyRing, federatedAuthProvider(ctx, r.federation, r.keyRing, serverName)) + if err != nil { + return fmt.Errorf("error checking state returned from peeking: %w", err) + } if err = sanityCheckAuthChain(authEvents); err != nil { return fmt.Errorf("sanityCheckAuthChain: %w", err) } - if err = respState.Check(ctx, respPeek.RoomVersion, r.keyRing, federatedAuthProvider(ctx, r.federation, r.keyRing, serverName)); err != nil { - return fmt.Errorf("error checking state returned from peeking: %w", err) - } // If we've got this far, the remote server is peeking. if renewing { diff --git a/go.mod b/go.mod index f287fb9f4..a34f8a578 100644 --- a/go.mod +++ b/go.mod @@ -38,7 +38,7 @@ require ( github.com/matrix-org/go-http-js-libp2p v0.0.0-20200518170932-783164aeeda4 github.com/matrix-org/go-sqlite3-js v0.0.0-20210709140738-b0d1ba599a6d github.com/matrix-org/gomatrix v0.0.0-20210324163249-be2af5ef2e16 - github.com/matrix-org/gomatrixserverlib v0.0.0-20220404174134-970e11ad2142 + github.com/matrix-org/gomatrixserverlib v0.0.0-20220405134050-301e340659d5 github.com/matrix-org/pinecone v0.0.0-20220404141326-e526fa82f79d github.com/matrix-org/util v0.0.0-20200807132607-55161520e1d4 github.com/mattn/go-sqlite3 v1.14.10 @@ -64,7 +64,6 @@ require ( golang.org/x/image v0.0.0-20211028202545-6944b10bf410 golang.org/x/mobile v0.0.0-20220325161704-447654d348e3 golang.org/x/net v0.0.0-20220127200216-cd36cc0744dd - golang.org/x/sys v0.0.0-20220227234510-4e6760a101f9 // indirect golang.org/x/term v0.0.0-20210927222741-03fcf44c2211 gopkg.in/h2non/bimg.v1 v1.1.5 gopkg.in/yaml.v2 v2.4.0 diff --git a/go.sum b/go.sum index bbe109ff3..c76f9c55d 100644 --- a/go.sum +++ b/go.sum @@ -977,8 +977,8 @@ github.com/matrix-org/go-sqlite3-js v0.0.0-20210709140738-b0d1ba599a6d/go.mod h1 github.com/matrix-org/gomatrix v0.0.0-20190528120928-7df988a63f26/go.mod h1:3fxX6gUjWyI/2Bt7J1OLhpCzOfO/bB3AiX0cJtEKud0= github.com/matrix-org/gomatrix v0.0.0-20210324163249-be2af5ef2e16 h1:ZtO5uywdd5dLDCud4r0r55eP4j9FuUNpl60Gmntcop4= github.com/matrix-org/gomatrix v0.0.0-20210324163249-be2af5ef2e16/go.mod h1:/gBX06Kw0exX1HrwmoBibFA98yBk/jxKpGVeyQbff+s= -github.com/matrix-org/gomatrixserverlib v0.0.0-20220404174134-970e11ad2142 h1:kkFKjbPn9oySI07bA3vVInFMjTRdMxASgwJXmABli4o= -github.com/matrix-org/gomatrixserverlib v0.0.0-20220404174134-970e11ad2142/go.mod h1:+WF5InseAMgi1fTnU46JH39IDpEvLep0fDzx9LDf2Bo= +github.com/matrix-org/gomatrixserverlib v0.0.0-20220405134050-301e340659d5 h1:Fkennny7+Z/5pygrhjFMZbz1j++P2hhhLoT7NO3p8DQ= +github.com/matrix-org/gomatrixserverlib v0.0.0-20220405134050-301e340659d5/go.mod h1:V5eO8rn/C3rcxig37A/BCeKerLFS+9Avg/77FIeTZ48= github.com/matrix-org/pinecone v0.0.0-20220404141326-e526fa82f79d h1:1+T4eOPRsf6cr0lMPW4oO2k8TTHm4mqIh65kpEID5Rk= github.com/matrix-org/pinecone v0.0.0-20220404141326-e526fa82f79d/go.mod h1:ulJzsVOTssIVp1j/m5eI//4VpAGDkMt5NrRuAVX7wpc= github.com/matrix-org/util v0.0.0-20190711121626-527ce5ddefc7/go.mod h1:vVQlW/emklohkZnOPwD3LrZUBqdfsbiyO3p1lNV8F6U= @@ -1727,8 +1727,8 @@ golang.org/x/sys v0.0.0-20211102192858-4dd72447c267/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20211216021012-1d35b9e2eb4e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220111092808-5a964db01320/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220114195835-da31bd327af9/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.0.0-20220227234510-4e6760a101f9 h1:nhht2DYV/Sn3qOayu8lM+cU1ii9sTLUeBQwQQfUHtrs= -golang.org/x/sys v0.0.0-20220227234510-4e6760a101f9/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20220405052023-b1e9470b6e64 h1:D1v9ucDTYBtbz5vNuBbAhIMAGhQhJ6Ym5ah3maMVNX4= +golang.org/x/sys v0.0.0-20220405052023-b1e9470b6e64/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211 h1:JGgROgKl9N8DuW20oFS5gxc+lE67/N3FcwmBPMe7ArY= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= diff --git a/roomserver/internal/input/input_missing.go b/roomserver/internal/input/input_missing.go index a7da9b06d..2c958335d 100644 --- a/roomserver/internal/input/input_missing.go +++ b/roomserver/internal/input/input_missing.go @@ -613,12 +613,13 @@ func (t *missingStateReq) lookupMissingStateViaState( return nil, err } // Check that the returned state is valid. - if err := state.Check(ctx, roomVersion, t.keys, nil); err != nil { + authEvents, stateEvents, err := state.Check(ctx, roomVersion, t.keys, nil) + if err != nil { return nil, err } parsedState := &parsedRespState{ - AuthEvents: make([]*gomatrixserverlib.Event, len(state.AuthEvents)), - StateEvents: make([]*gomatrixserverlib.Event, len(state.StateEvents)), + AuthEvents: authEvents, + StateEvents: stateEvents, } // Cache the results of this state lookup and deduplicate anything we already // have in the cache, freeing up memory. From b49a6757c8a082f8d3048b53f1c160c73943fcee Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Tue, 5 Apr 2022 16:43:29 +0100 Subject: [PATCH 3/5] Update the "Are We Synapse Yet?" list --- are-we-synapse-yet.list | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/are-we-synapse-yet.list b/are-we-synapse-yet.list index c2c726e57..4281171ab 100644 --- a/are-we-synapse-yet.list +++ b/are-we-synapse-yet.list @@ -106,10 +106,13 @@ rst Users cannot set notifications powerlevel higher than their own (2 subtests) rst Both GET and PUT work rct POST /rooms/:room_id/receipt can create receipts red POST /rooms/:room_id/read_markers can create read marker +med POST /media/v3/upload can create an upload med POST /media/r0/upload can create an upload +med GET /media/v3/download can fetch the value again med GET /media/r0/download can fetch the value again cap GET /capabilities is present and well formed for registered user cap GET /r0/capabilities is not public +cap GET /v3/capabilities is not public reg Register with a recaptcha reg registration is idempotent, without username specified reg registration is idempotent, with username specified @@ -174,7 +177,9 @@ eph Ephemeral messages received from clients are correctly expired ali Room aliases can contain Unicode f,ali Remote room alias queries can handle Unicode ali Canonical alias can be set +ali Canonical alias can be set (3 subtests) ali Canonical alias can include alt_aliases +ali Canonical alias can include alt_aliases (4 subtests) ali Regular users can add and delete aliases in the default room configuration ali Regular users can add and delete aliases when m.room.aliases is restricted ali Deleting a non-existent alias should return a 404 @@ -478,6 +483,30 @@ rmv Inbound federation rejects invites which include invalid JSON for room versi rmv Outbound federation rejects invite response which include invalid JSON for room version 6 rmv Inbound federation rejects invite rejections which include invalid JSON for room version 6 rmv Server rejects invalid JSON in a version 6 room +rmv User can create and send/receive messages in a room with version 7 (2 subtests) +rmv local user can join room with version 7 +rmv User can invite local user to room with version 7 +rmv remote user can join room with version 7 +rmv User can invite remote user to room with version 7 +rmv Remote user can backfill in a room with version 7 +rmv Can reject invites over federation for rooms with version 7 +rmv Can receive redactions from regular users over federation in room version 7 +rmv User can create and send/receive messages in a room with version 8 (2 subtests) +rmv local user can join room with version 8 +rmv User can invite local user to room with version 8 +rmv remote user can join room with version 8 +rmv User can invite remote user to room with version 8 +rmv Remote user can backfill in a room with version 8 +rmv Can reject invites over federation for rooms with version 8 +rmv Can receive redactions from regular users over federation in room version 8 +rmv User can create and send/receive messages in a room with version 9 (2 subtests) +rmv local user can join room with version 9 +rmv User can invite local user to room with version 9 +rmv remote user can join room with version 9 +rmv User can invite remote user to room with version 9 +rmv Remote user can backfill in a room with version 9 +rmv Can reject invites over federation for rooms with version 9 +rmv Can receive redactions from regular users over federation in room version 9 pre Presence changes are reported to local room members f,pre Presence changes are also reported to remote room members pre Presence changes to UNAVAILABLE are reported to local room members @@ -772,12 +801,15 @@ app AS can make room aliases app Regular users cannot create room aliases within the AS namespace app AS-ghosted users can use rooms via AS app AS-ghosted users can use rooms themselves +app AS-ghosted users can use rooms via AS (2 subtests) +app AS-ghosted users can use rooms themselves (3 subtests) app Ghost user must register before joining room app AS can set avatar for ghosted users app AS can set displayname for ghosted users app AS can't set displayname for random users app Inviting an AS-hosted user asks the AS server app Accesing an AS-hosted room alias asks the AS server +app Accesing an AS-hosted room alias asks the AS server (2 subtests) app Events in rooms with AS-hosted room aliases are sent to AS server app AS user (not ghost) can join room without registering app AS user (not ghost) can join room without registering, with user_id query param @@ -789,6 +821,8 @@ app AS can publish rooms in their own list app AS and main public room lists are separate app AS can deactivate a user psh Test that a message is pushed +psh Test that a message is pushed (6 subtests) +psh Test that rejected pushers are removed. (4 subtests) psh Invites are pushed psh Rooms with names are correctly named in pushed psh Rooms with canonical alias are correctly named in pushed @@ -857,9 +891,12 @@ pre Left room members do not cause problems for presence crm Rooms can be created with an initial invite list (SYN-205) (1 subtests) typ Typing notifications don't leak ban Non-present room members cannot ban others +ban Non-present room members cannot ban others (3 subtests) psh Getting push rules doesn't corrupt the cache SYN-390 +psh Getting push rules doesn't corrupt the cache SYN-390 (3 subtests) inv Test that we can be reinvited to a room we created syn Multiple calls to /sync should not cause 500 errors +syn Multiple calls to /sync should not cause 500 errors (6 subtests) gst Guest user can call /events on another world_readable room (SYN-606) gst Real user can call /events on another world_readable room (SYN-606) gst Events come down the correct room From f7109de500d675855f9d63c1959f244d34096a4b Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Tue, 5 Apr 2022 16:45:01 +0100 Subject: [PATCH 4/5] Nuke a couple of tests for now --- sytest-blacklist | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/sytest-blacklist b/sytest-blacklist index 6a3b88390..a4bdf372d 100644 --- a/sytest-blacklist +++ b/sytest-blacklist @@ -1,35 +1,46 @@ # Blacklisted until matrix-org/dendrite#862 is reverted due to Riot bug + Latest account data appears in v2 /sync # Blacklisted because we don't support ignores yet + Ignore invite in incremental sync # Relies on a rejected PL event which will never be accepted into the DAG -# Caused by https://github.com/matrix-org/sytest/pull/911 + +# Caused by + Outbound federation requests missing prev_events and then asks for /state_ids and resolves the state -# We don't implement lazy membership loading yet. +# We don't implement lazy membership loading yet + The only membership state included in a gapped incremental sync is for senders in the timeline # Blacklisted out of flakiness after #1479 + Invited user can reject local invite after originator leaves Invited user can reject invite for empty room If user leaves room, remote user changes device and rejoins we see update in /sync and /keys/changes # Blacklisted due to flakiness + Forgotten room messages cannot be paginated # Blacklisted due to flakiness after #1774 + Local device key changes get to remote servers with correct prev_id # Flakey + Local device key changes appear in /keys/changes # we don't support groups + Remove group category Remove group role # Flakey + AS-ghosted users can use rooms themselves AS-ghosted users can use rooms via AS Events in rooms with AS-hosted room aliases are sent to AS server @@ -37,6 +48,12 @@ Inviting an AS-hosted user asks the AS server Accesing an AS-hosted room alias asks the AS server # Flakey, need additional investigation + Messages that notify from another user increment notification_count Messages that highlight from another user increment unread highlight count Notifications can be viewed with GET /notifications + +# More flakey + +If remote user leaves room we no longer receive device updates +Local device key changes get to remote servers From 16e2d243fc8f3d433a9d7f428e6f782065dc5e89 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Wed, 6 Apr 2022 10:43:54 +0100 Subject: [PATCH 5/5] Reduce the number of allocations made by `localRoomMembers` when consuming stream events for push notifications (#2324) --- roomserver/api/query.go | 2 ++ roomserver/internal/query/query.go | 2 +- userapi/consumers/syncapi_streamevent.go | 1 + 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/roomserver/api/query.go b/roomserver/api/query.go index 66e85f2f3..8f84edcb5 100644 --- a/roomserver/api/query.go +++ b/roomserver/api/query.go @@ -128,6 +128,8 @@ type QueryMembershipForUserResponse struct { type QueryMembershipsForRoomRequest struct { // If true, only returns the membership events of "join" membership JoinedOnly bool `json:"joined_only"` + // If true, only returns the membership events of local users + LocalOnly bool `json:"local_only"` // ID of the room to fetch memberships from RoomID string `json:"room_id"` // Optional - ID of the user sending the request, for checking if the diff --git a/roomserver/internal/query/query.go b/roomserver/internal/query/query.go index 471c6fb42..7e4d56684 100644 --- a/roomserver/internal/query/query.go +++ b/roomserver/internal/query/query.go @@ -220,7 +220,7 @@ func (r *Queryer) QueryMembershipsForRoom( if request.Sender == "" { var events []types.Event var eventNIDs []types.EventNID - eventNIDs, err = r.DB.GetMembershipEventNIDsForRoom(ctx, info.RoomNID, request.JoinedOnly, false) + eventNIDs, err = r.DB.GetMembershipEventNIDsForRoom(ctx, info.RoomNID, request.JoinedOnly, request.LocalOnly) if err != nil { return fmt.Errorf("r.DB.GetMembershipEventNIDsForRoom: %w", err) } diff --git a/userapi/consumers/syncapi_streamevent.go b/userapi/consumers/syncapi_streamevent.go index da3cd3937..34a67d2ea 100644 --- a/userapi/consumers/syncapi_streamevent.go +++ b/userapi/consumers/syncapi_streamevent.go @@ -184,6 +184,7 @@ func (s *OutputStreamEventConsumer) localRoomMembers(ctx context.Context, roomID req := &rsapi.QueryMembershipsForRoomRequest{ RoomID: roomID, JoinedOnly: true, + LocalOnly: true, } var res rsapi.QueryMembershipsForRoomResponse