From 503bf6e126052016b35fdc71139261be65338935 Mon Sep 17 00:00:00 2001 From: kegsay <7190048+kegsay@users.noreply.github.com> Date: Fri, 16 Feb 2024 17:26:35 +0000 Subject: [PATCH 1/2] Refactor the match.CheckOff API to use functional options (#711) This means you don't need to tack on `, nil` all the time, and can optionally specify when you want to allow unwanted items, obviating the need for `JSONCheckOffAllowUnwanted`. This composes better and supports adding additional functionality e.g allowing duplicate items. --- match/json.go | 114 ++++++++++++------ tests/csapi/apidoc_search_test.go | 6 +- tests/csapi/room_leave_test.go | 8 +- tests/csapi/room_members_test.go | 10 +- tests/csapi/room_relations_test.go | 18 +-- .../room_messages_relation_filter_test.go | 12 +- ...federation_room_join_partial_state_test.go | 32 +++-- tests/restricted_room_hierarchy_test.go | 2 +- tests/room_hierarchy_test.go | 34 +++--- tests/room_timestamp_to_event_test.go | 4 +- 10 files changed, 140 insertions(+), 100 deletions(-) diff --git a/match/json.go b/match/json.go index 3b1b3829..9ca0cb45 100644 --- a/match/json.go +++ b/match/json.go @@ -113,7 +113,69 @@ func JSONKeyArrayOfSize(wantKey string, wantSize int) JSON { } } -func jsonCheckOffInternal(wantKey string, wantItems []interface{}, allowUnwantedItems bool, mapper func(gjson.Result) interface{}, fn func(interface{}, gjson.Result) error) JSON { +type checkOffOpts struct { + allowUnwantedItems bool + mapper func(gjson.Result) interface{} + forEach func(interface{}, gjson.Result) error +} + +// CheckOffAllowUnwanted allows unwanted items, that is items not in `wantItems`, +// to not fail the check. +func CheckOffAllowUnwanted() func(*checkOffOpts) { + return func(coo *checkOffOpts) { + coo.allowUnwantedItems = true + } +} + +// CheckOffMapper maps each item /before/ continuing the check off process. This +// is useful to convert a gjson.Result to something more domain specific such as +// an event ID. For example, if `r` is a Matrix event, this allows `wantItems` to +// be a slice of event IDs: +// +// CheckOffMapper(func(r gjson.Result) interface{} { +// return r.Get("event_id").Str +// }) +// +// The `mapper` function should map the item to an interface which will be +// comparable via JSONDeepEqual with items in `wantItems`. +func CheckOffMapper(mapper func(gjson.Result) interface{}) func(*checkOffOpts) { + return func(coo *checkOffOpts) { + coo.mapper = mapper + } +} + +// CheckOffForEach does not change the check off logic, but instead passes each item +// to the provided function. If the function returns an error, the check fails. +// It is called with 2 args: the item being checked and the element itself +// (or value if it's an object). +func CheckOffForEach(forEach func(interface{}, gjson.Result) error) func(*checkOffOpts) { + return func(coo *checkOffOpts) { + coo.forEach = forEach + } +} + +// EXPERIMENTAL +// JSONCheckOff returns a matcher which will loop over `wantKey` and ensure that the items +// (which can be array elements or object keys) are present exactly once in `wantItems`. +// This matcher can be used to check off items in an array/object. +// +// This function supports functional options which change the behaviour of the check off +// logic, see match.CheckOff... functions for more information. +// +// Usage: (ensures `events` has these events in any order, with the right event type) +// +// JSONCheckOff("events", []interface{}{"$foo:bar", "$baz:quuz"}, CheckOffMapper(func(r gjson.Result) interface{} { +// return r.Get("event_id").Str +// }), CheckOffForEach(func(eventID interface{}, eventBody gjson.Result) error { +// if eventBody.Get("type").Str != "m.room.message" { +// return fmt.Errorf("expected event to be 'm.room.message'") +// } +// })) +func JSONCheckOff(wantKey string, wantItems []interface{}, opts ...func(*checkOffOpts)) JSON { + var coo checkOffOpts + for _, opt := range opts { + opt(&coo) + } return func(body gjson.Result) error { res := body.Get(wantKey) if !res.Exists() { @@ -128,11 +190,14 @@ func jsonCheckOffInternal(wantKey string, wantItems []interface{}, allowUnwanted if res.IsArray() { itemRes = val } - // convert it to something we can check off - item := mapper(itemRes) - if item == nil { - err = fmt.Errorf("JSONCheckOff(%s): mapper function mapped %v to nil", wantKey, itemRes.Raw) - return false + var item interface{} = itemRes + if coo.mapper != nil { + // convert it to something we can check off + item = coo.mapper(itemRes) + if item == nil { + err = fmt.Errorf("JSONCheckOff(%s): mapper function mapped %v to nil", wantKey, itemRes.Raw) + return false + } } // check off the item @@ -144,7 +209,7 @@ func jsonCheckOffInternal(wantKey string, wantItems []interface{}, allowUnwanted break } } - if !allowUnwantedItems && want == -1 { + if !coo.allowUnwantedItems && want == -1 { err = fmt.Errorf("JSONCheckOff(%s): unexpected item %v (mapped value %v)", wantKey, itemRes.Raw, item) return false } @@ -155,10 +220,10 @@ func jsonCheckOffInternal(wantKey string, wantItems []interface{}, allowUnwanted } // do further checks - if fn != nil { - err = fn(item, val) + if coo.forEach != nil { + err = coo.forEach(item, val) if err != nil { - err = fmt.Errorf("JSONCheckOff(%s): item %v failed checks: %w", wantKey, val, err) + err = fmt.Errorf("JSONCheckOff(%s): forEach function returned an error for item %v: %w", wantKey, val, err) return false } } @@ -175,31 +240,8 @@ func jsonCheckOffInternal(wantKey string, wantItems []interface{}, allowUnwanted } } -// EXPERIMENTAL -// JSONCheckOffAllowUnwanted returns a matcher which will loop over `wantKey` and ensure that the items -// (which can be array elements or object keys) -// are present exactly once in any order in `wantItems`. Allows unexpected items or items -// appear that more than once. This matcher can be used to check off items in -// an array/object. The `mapper` function should map the item to an interface which will be -// comparable via JSONDeepEqual with items in `wantItems`. The optional `fn` callback -// allows more checks to be performed other than checking off the item from the list. It is -// called with 2 args: the result of the `mapper` function and the element itself (or value if -// it's an object). +// DEPRECATED: Prefer JSONCheckOff as this uses functional options which makes params easier to understand. // -// Usage: (ensures `events` has these events in any order, with the right event type) -// -// JSONCheckOffAllowUnwanted("events", []interface{}{"$foo:bar", "$baz:quuz"}, func(r gjson.Result) interface{} { -// return r.Get("event_id").Str -// }, func(eventID interface{}, eventBody gjson.Result) error { -// if eventBody.Get("type").Str != "m.room.message" { -// return fmt.Errorf("expected event to be 'm.room.message'") -// } -// }) -func JSONCheckOffAllowUnwanted(wantKey string, wantItems []interface{}, mapper func(gjson.Result) interface{}, fn func(interface{}, gjson.Result) error) JSON { - return jsonCheckOffInternal(wantKey, wantItems, true, mapper, fn) -} - -// EXPERIMENTAL // JSONCheckOff returns a matcher which will loop over `wantKey` and ensure that the items // (which can be array elements or object keys) // are present exactly once in any order in `wantItems`. If there are unexpected items or items @@ -219,8 +261,8 @@ func JSONCheckOffAllowUnwanted(wantKey string, wantItems []interface{}, mapper f // return fmt.Errorf("expected event to be 'm.room.message'") // } // }) -func JSONCheckOff(wantKey string, wantItems []interface{}, mapper func(gjson.Result) interface{}, fn func(interface{}, gjson.Result) error) JSON { - return jsonCheckOffInternal(wantKey, wantItems, false, mapper, fn) +func JSONCheckOffDeprecated(wantKey string, wantItems []interface{}, mapper func(gjson.Result) interface{}, fn func(interface{}, gjson.Result) error) JSON { + return JSONCheckOff(wantKey, wantItems, CheckOffMapper(mapper), CheckOffForEach(fn)) } // JSONArrayEach returns a matcher which will check that `wantKey` is an array then loops over each diff --git a/tests/csapi/apidoc_search_test.go b/tests/csapi/apidoc_search_test.go index 1640f636..16edb2e7 100644 --- a/tests/csapi/apidoc_search_test.go +++ b/tests/csapi/apidoc_search_test.go @@ -256,9 +256,9 @@ func TestSearch(t *testing.T) { match.JSONKeyArrayOfSize(sce+".results", 2), // the results can be in either order: check that both are there and that the content is as expected - match.JSONCheckOff(sce+".results", []interface{}{eventBeforeUpgrade, eventAfterUpgrade}, func(res gjson.Result) interface{} { + match.JSONCheckOff(sce+".results", []interface{}{eventBeforeUpgrade, eventAfterUpgrade}, match.CheckOffMapper(func(res gjson.Result) interface{} { return res.Get("result.event_id").Str - }, func(eventID interface{}, result gjson.Result) error { + }), match.CheckOffForEach(func(eventID interface{}, result gjson.Result) error { matchers := []match.JSON{ match.JSONKeyEqual("result.type", "m.room.message"), match.JSONKeyEqual("result.content.body", expectedEvents[eventID.(string)]), @@ -269,7 +269,7 @@ func TestSearch(t *testing.T) { } } return nil - }), + })), }, }) }) diff --git a/tests/csapi/room_leave_test.go b/tests/csapi/room_leave_test.go index 00e93395..1ea3e9a5 100644 --- a/tests/csapi/room_leave_test.go +++ b/tests/csapi/room_leave_test.go @@ -166,13 +166,13 @@ func TestLeftRoomFixture(t *testing.T) { []interface{}{ "m.room.member|" + alice.UserID + "|join", "m.room.member|" + bob.UserID + "|leave", - }, func(result gjson.Result) interface{} { + }, match.CheckOffMapper(func(result gjson.Result) interface{} { return strings.Join([]string{ result.Map()["type"].Str, result.Map()["state_key"].Str, result.Get("content.membership").Str, }, "|") - }, nil), + })), }, }) }) @@ -191,13 +191,13 @@ func TestLeftRoomFixture(t *testing.T) { "m.room.message|" + beforeMessageOne + "|", "m.room.message|" + beforeMessageTwo + "|", "m.room.member||" + bob.UserID, - }, func(result gjson.Result) interface{} { + }, match.CheckOffMapper(func(result gjson.Result) interface{} { return strings.Join([]string{ result.Map()["type"].Str, result.Get("content.body").Str, result.Map()["state_key"].Str, }, "|") - }, nil), + })), }, }) }) diff --git a/tests/csapi/room_members_test.go b/tests/csapi/room_members_test.go index bb269f7c..c017b64c 100644 --- a/tests/csapi/room_members_test.go +++ b/tests/csapi/room_members_test.go @@ -52,7 +52,7 @@ func TestGetRoomMembers(t *testing.T) { []interface{}{ "m.room.member|" + alice.UserID, "m.room.member|" + bob.UserID, - }, typeToStateKeyMapper, nil), + }, match.CheckOffMapper(typeToStateKeyMapper)), }, StatusCode: 200, }) @@ -111,7 +111,7 @@ func TestGetRoomMembersAtPoint(t *testing.T) { match.JSONCheckOff("chunk", []interface{}{ "m.room.member|" + alice.UserID, - }, typeToStateKeyMapper, nil), + }, match.CheckOffMapper(typeToStateKeyMapper)), }, StatusCode: 200, @@ -158,7 +158,7 @@ func TestGetFilteredRoomMembers(t *testing.T) { match.JSONCheckOff("chunk", []interface{}{ "m.room.member|" + alice.UserID, - }, typeToStateKeyMapper, nil), + }, match.CheckOffMapper(typeToStateKeyMapper)), }, StatusCode: 200, }) @@ -183,7 +183,7 @@ func TestGetFilteredRoomMembers(t *testing.T) { match.JSONCheckOff("chunk", []interface{}{ "m.room.member|" + bob.UserID, - }, typeToStateKeyMapper, nil), + }, match.CheckOffMapper(typeToStateKeyMapper)), }, StatusCode: 200, }) @@ -208,7 +208,7 @@ func TestGetFilteredRoomMembers(t *testing.T) { match.JSONCheckOff("chunk", []interface{}{ "m.room.member|" + alice.UserID, - }, typeToStateKeyMapper, nil), + }, match.CheckOffMapper(typeToStateKeyMapper)), }, StatusCode: 200, }) diff --git a/tests/csapi/room_relations_test.go b/tests/csapi/room_relations_test.go index 9f83d387..c817b64e 100644 --- a/tests/csapi/room_relations_test.go +++ b/tests/csapi/room_relations_test.go @@ -74,7 +74,7 @@ func TestRelations(t *testing.T) { must.MatchResponse(t, res, match.HTTPResponse{ StatusCode: http.StatusOK, JSON: []match.JSON{ - match.JSONCheckOff("chunk", []interface{}{ + match.JSONCheckOffDeprecated("chunk", []interface{}{ threadEventID, dummyEventID, editEventID, }, func(r gjson.Result) interface{} { return r.Get("event_id").Str @@ -88,7 +88,7 @@ func TestRelations(t *testing.T) { must.MatchResponse(t, res, match.HTTPResponse{ StatusCode: http.StatusOK, JSON: []match.JSON{ - match.JSONCheckOff("chunk", []interface{}{ + match.JSONCheckOffDeprecated("chunk", []interface{}{ threadEventID, dummyEventID, }, func(r gjson.Result) interface{} { return r.Get("event_id").Str @@ -102,7 +102,7 @@ func TestRelations(t *testing.T) { must.MatchResponse(t, res, match.HTTPResponse{ StatusCode: http.StatusOK, JSON: []match.JSON{ - match.JSONCheckOff("chunk", []interface{}{ + match.JSONCheckOffDeprecated("chunk", []interface{}{ threadEventID, }, func(r gjson.Result) interface{} { return r.Get("event_id").Str @@ -152,7 +152,7 @@ func TestRelationsPagination(t *testing.T) { body := must.MatchResponse(t, res, match.HTTPResponse{ StatusCode: http.StatusOK, JSON: []match.JSON{ - match.JSONCheckOff("chunk", []interface{}{ + match.JSONCheckOffDeprecated("chunk", []interface{}{ event_ids[9], event_ids[8], event_ids[7], }, func(r gjson.Result) interface{} { return r.Get("event_id").Str @@ -167,7 +167,7 @@ func TestRelationsPagination(t *testing.T) { must.MatchResponse(t, res, match.HTTPResponse{ StatusCode: http.StatusOK, JSON: []match.JSON{ - match.JSONCheckOff("chunk", []interface{}{ + match.JSONCheckOffDeprecated("chunk", []interface{}{ event_ids[6], event_ids[5], event_ids[4], }, func(r gjson.Result) interface{} { return r.Get("event_id").Str @@ -184,7 +184,7 @@ func TestRelationsPagination(t *testing.T) { body = must.MatchResponse(t, res, match.HTTPResponse{ StatusCode: http.StatusOK, JSON: []match.JSON{ - match.JSONCheckOff("chunk", []interface{}{ + match.JSONCheckOffDeprecated("chunk", []interface{}{ event_ids[0], event_ids[1], event_ids[2], }, func(r gjson.Result) interface{} { return r.Get("event_id").Str @@ -199,7 +199,7 @@ func TestRelationsPagination(t *testing.T) { must.MatchResponse(t, res, match.HTTPResponse{ StatusCode: http.StatusOK, JSON: []match.JSON{ - match.JSONCheckOff("chunk", []interface{}{ + match.JSONCheckOffDeprecated("chunk", []interface{}{ event_ids[3], event_ids[4], event_ids[5], }, func(r gjson.Result) interface{} { return r.Get("event_id").Str @@ -279,7 +279,7 @@ func TestRelationsPaginationSync(t *testing.T) { body := must.MatchResponse(t, res, match.HTTPResponse{ StatusCode: http.StatusOK, JSON: []match.JSON{ - match.JSONCheckOff("chunk", []interface{}{ + match.JSONCheckOffDeprecated("chunk", []interface{}{ event_ids[0], event_ids[1], event_ids[2], }, func(r gjson.Result) interface{} { return r.Get("event_id").Str @@ -294,7 +294,7 @@ func TestRelationsPaginationSync(t *testing.T) { must.MatchResponse(t, res, match.HTTPResponse{ StatusCode: http.StatusOK, JSON: []match.JSON{ - match.JSONCheckOff("chunk", []interface{}{ + match.JSONCheckOffDeprecated("chunk", []interface{}{ event_ids[3], event_ids[4], }, func(r gjson.Result) interface{} { return r.Get("event_id").Str diff --git a/tests/msc3874/room_messages_relation_filter_test.go b/tests/msc3874/room_messages_relation_filter_test.go index e1b55edb..41f9788f 100644 --- a/tests/msc3874/room_messages_relation_filter_test.go +++ b/tests/msc3874/room_messages_relation_filter_test.go @@ -68,7 +68,7 @@ func TestFilterMessagesByRelType(t *testing.T) { must.MatchResponse(t, res, match.HTTPResponse{ StatusCode: http.StatusOK, JSON: []match.JSON{ - match.JSONCheckOff("chunk", []interface{}{ + match.JSONCheckOffDeprecated("chunk", []interface{}{ threadEventID, }, func(r gjson.Result) interface{} { return r.Get("event_id").Str @@ -86,7 +86,7 @@ func TestFilterMessagesByRelType(t *testing.T) { must.MatchResponse(t, res, match.HTTPResponse{ StatusCode: http.StatusOK, JSON: []match.JSON{ - match.JSONCheckOff("chunk", []interface{}{ + match.JSONCheckOffDeprecated("chunk", []interface{}{ referenceEventID, }, func(r gjson.Result) interface{} { return r.Get("event_id").Str @@ -104,7 +104,7 @@ func TestFilterMessagesByRelType(t *testing.T) { must.MatchResponse(t, res, match.HTTPResponse{ StatusCode: http.StatusOK, JSON: []match.JSON{ - match.JSONCheckOff("chunk", []interface{}{ + match.JSONCheckOffDeprecated("chunk", []interface{}{ threadEventID, referenceEventID, }, func(r gjson.Result) interface{} { return r.Get("event_id").Str @@ -122,7 +122,7 @@ func TestFilterMessagesByRelType(t *testing.T) { must.MatchResponse(t, res, match.HTTPResponse{ StatusCode: http.StatusOK, JSON: []match.JSON{ - match.JSONCheckOff("chunk", []interface{}{ + match.JSONCheckOffDeprecated("chunk", []interface{}{ rootEventID, referenceEventID, }, func(r gjson.Result) interface{} { return r.Get("event_id").Str @@ -140,7 +140,7 @@ func TestFilterMessagesByRelType(t *testing.T) { must.MatchResponse(t, res, match.HTTPResponse{ StatusCode: http.StatusOK, JSON: []match.JSON{ - match.JSONCheckOff("chunk", []interface{}{ + match.JSONCheckOffDeprecated("chunk", []interface{}{ rootEventID, threadEventID, }, func(r gjson.Result) interface{} { return r.Get("event_id").Str @@ -158,7 +158,7 @@ func TestFilterMessagesByRelType(t *testing.T) { must.MatchResponse(t, res, match.HTTPResponse{ StatusCode: http.StatusOK, JSON: []match.JSON{ - match.JSONCheckOff("chunk", []interface{}{ + match.JSONCheckOffDeprecated("chunk", []interface{}{ rootEventID, }, func(r gjson.Result) interface{} { return r.Get("event_id").Str diff --git a/tests/msc3902/federation_room_join_partial_state_test.go b/tests/msc3902/federation_room_join_partial_state_test.go index d5264059..1d8159f3 100644 --- a/tests/msc3902/federation_room_join_partial_state_test.go +++ b/tests/msc3902/federation_room_join_partial_state_test.go @@ -383,13 +383,13 @@ func TestPartialStateJoin(t *testing.T) { } // check that the state includes both charlie and derek. - matcher := match.JSONCheckOffAllowUnwanted("state.events", + matcher := match.JSONCheckOff("state.events", []interface{}{ "m.room.member|" + server.UserID("charlie"), "m.room.member|" + server.UserID("derek"), - }, func(result gjson.Result) interface{} { + }, match.CheckOffMapper(func(result gjson.Result) interface{} { return strings.Join([]string{result.Map()["type"].Str, result.Map()["state_key"].Str}, "|") - }, nil, + }), match.CheckOffAllowUnwanted(), ) if err := matcher(roomRes); err != nil { t.Errorf("Did not find expected state events in /sync response: %s", err) @@ -799,8 +799,7 @@ func TestPartialStateJoin(t *testing.T) { matcher := match.JSONCheckOff( "device_lists.changed", []interface{}{derekUserId}, - func(r gjson.Result) interface{} { return r.Str }, - nil, + match.CheckOffMapper(func(r gjson.Result) interface{} { return r.Str }), ) return matcher(res) }, @@ -1203,9 +1202,9 @@ func TestPartialStateJoin(t *testing.T) { "m.room.member|" + alice.UserID, "m.room.member|" + server.UserID("charlie"), "m.room.member|" + server.UserID("derek"), - }, func(result gjson.Result) interface{} { + }, match.CheckOffMapper(func(result gjson.Result) interface{} { return strings.Join([]string{result.Map()["type"].Str, result.Map()["state_key"].Str}, "|") - }, nil), + })), }, }) } @@ -1414,16 +1413,16 @@ func TestPartialStateJoin(t *testing.T) { timelineMatcher := match.JSONCheckOff("timeline.events", []interface{}{lastEventID}, - func(result gjson.Result) interface{} { + match.CheckOffMapper(func(result gjson.Result) interface{} { return result.Map()["event_id"].Str - }, nil, + }), ) - stateMatcher := match.JSONCheckOffAllowUnwanted("state.events", + stateMatcher := match.JSONCheckOff("state.events", []interface{}{ "m.room.member|" + server.UserID("derek"), - }, func(result gjson.Result) interface{} { + }, match.CheckOffMapper(func(result gjson.Result) interface{} { return strings.Join([]string{result.Map()["type"].Str, result.Map()["state_key"].Str}, "|") - }, nil, + }), match.CheckOffAllowUnwanted(), ) if err := timelineMatcher(roomRes); err != nil { t.Errorf("Unexpected timeline events found in gappy /sync response: %s", err) @@ -2897,11 +2896,11 @@ func TestPartialStateJoin(t *testing.T) { must.MatchResponse(t, res, match.HTTPResponse{ StatusCode: 200, JSON: []match.JSON{ - match.JSONCheckOffAllowUnwanted( + match.JSONCheckOff( section, []interface{}{expectedUserID}, - func(r gjson.Result) interface{} { return r.Str }, - nil, + match.CheckOffMapper(func(r gjson.Result) interface{} { return r.Str }), + match.CheckOffAllowUnwanted(), ), }, }) @@ -3488,8 +3487,7 @@ func TestPartialStateJoin(t *testing.T) { match.JSONCheckOff( "servers", []interface{}{"hs1", server.ServerName()}, - func(r gjson.Result) interface{} { return r.Str }, - nil, + match.CheckOffMapper(func(r gjson.Result) interface{} { return r.Str }), ), }, } diff --git a/tests/restricted_room_hierarchy_test.go b/tests/restricted_room_hierarchy_test.go index 438ca14c..fec2e8b5 100644 --- a/tests/restricted_room_hierarchy_test.go +++ b/tests/restricted_room_hierarchy_test.go @@ -22,7 +22,7 @@ func requestAndAssertSummary(t *testing.T, user *client.CSAPI, space string, exp res := user.MustDo(t, "GET", []string{"_matrix", "client", "v1", "rooms", space, "hierarchy"}) must.MatchResponse(t, res, match.HTTPResponse{ JSON: []match.JSON{ - match.JSONCheckOff("rooms", expected_rooms, func(r gjson.Result) interface{} { + match.JSONCheckOffDeprecated("rooms", expected_rooms, func(r gjson.Result) interface{} { return r.Get("room_id").Str }, nil), }, diff --git a/tests/room_hierarchy_test.go b/tests/room_hierarchy_test.go index caf4f126..c9b766fa 100644 --- a/tests/room_hierarchy_test.go +++ b/tests/room_hierarchy_test.go @@ -227,7 +227,7 @@ func TestClientSpacesSummary(t *testing.T) { res := alice.MustDo(t, "GET", []string{"_matrix", "client", "v1", "rooms", root, "hierarchy"}) must.MatchResponse(t, res, match.HTTPResponse{ JSON: []match.JSON{ - match.JSONCheckOff("rooms", []interface{}{ + match.JSONCheckOffDeprecated("rooms", []interface{}{ root, r1, r2, r3, r4, ss1, ss2, }, func(r gjson.Result) interface{} { return r.Get("room_id").Str @@ -248,7 +248,7 @@ func TestClientSpacesSummary(t *testing.T) { return nil }), // Check that the links from Root down to other rooms and spaces exist. - match.JSONCheckOff(`rooms.#(room_type=="m.space")#")`, []interface{}{ + match.JSONCheckOffDeprecated(`rooms.#(room_type=="m.space")#")`, []interface{}{ rootToR1 + ";" + rootToSS1 + ";" + rootToR2, ss1ToSS2, ss2ToR3 + ";" + ss2ToR4, @@ -270,13 +270,13 @@ func TestClientSpacesSummary(t *testing.T) { ) must.MatchResponse(t, res, match.HTTPResponse{ JSON: []match.JSON{ - match.JSONCheckOff("rooms", []interface{}{ + match.JSONCheckOffDeprecated("rooms", []interface{}{ root, r1, r2, ss1, }, func(r gjson.Result) interface{} { return r.Get("room_id").Str }, nil), // All of the links are still there. - match.JSONCheckOff(`rooms.#(room_type=="m.space")#`, []interface{}{ + match.JSONCheckOffDeprecated(`rooms.#(room_type=="m.space")#`, []interface{}{ rootToR1 + ";" + rootToSS1 + ";" + rootToR2, ss1ToSS2, }, roomToChildrenMapper, nil), }, @@ -296,13 +296,13 @@ func TestClientSpacesSummary(t *testing.T) { ) must.MatchResponse(t, res, match.HTTPResponse{ JSON: []match.JSON{ - match.JSONCheckOff("rooms", []interface{}{ + match.JSONCheckOffDeprecated("rooms", []interface{}{ root, r1, r2, }, func(r gjson.Result) interface{} { return r.Get("room_id").Str }, nil), // All of the links are still there. - match.JSONCheckOff(`rooms.#(room_type=="m.space")#`, []interface{}{ + match.JSONCheckOffDeprecated(`rooms.#(room_type=="m.space")#`, []interface{}{ rootToR1 + ";" + rootToR2, }, roomToChildrenMapper, nil), }, @@ -322,7 +322,7 @@ func TestClientSpacesSummary(t *testing.T) { ) body := must.MatchResponse(t, res, match.HTTPResponse{ JSON: []match.JSON{ - match.JSONCheckOff("rooms", []interface{}{ + match.JSONCheckOffDeprecated("rooms", []interface{}{ root, r1, ss1, ss2, }, func(r gjson.Result) interface{} { return r.Get("room_id").Str @@ -341,7 +341,7 @@ func TestClientSpacesSummary(t *testing.T) { ) must.MatchResponse(t, res, match.HTTPResponse{ JSON: []match.JSON{ - match.JSONCheckOff("rooms", []interface{}{ + match.JSONCheckOffDeprecated("rooms", []interface{}{ r3, r4, r2, }, func(r gjson.Result) interface{} { return r.Get("room_id").Str @@ -360,12 +360,12 @@ func TestClientSpacesSummary(t *testing.T) { res := alice.MustDo(t, "GET", []string{"_matrix", "client", "v1", "rooms", root, "hierarchy"}) must.MatchResponse(t, res, match.HTTPResponse{ JSON: []match.JSON{ - match.JSONCheckOff("rooms", []interface{}{ + match.JSONCheckOffDeprecated("rooms", []interface{}{ root, r1, r2, }, func(r gjson.Result) interface{} { return r.Get("room_id").Str }, nil), - match.JSONCheckOff(`rooms.#(room_type=="m.space")#`, []interface{}{ + match.JSONCheckOffDeprecated(`rooms.#(room_type=="m.space")#`, []interface{}{ rootToR1 + ";" + rootToR2, }, roomToChildrenMapper, nil), }, @@ -475,12 +475,12 @@ func TestClientSpacesSummaryJoinRules(t *testing.T) { res := bob.MustDo(t, "GET", []string{"_matrix", "client", "v1", "rooms", root, "hierarchy"}) must.MatchResponse(t, res, match.HTTPResponse{ JSON: []match.JSON{ - match.JSONCheckOff("rooms", []interface{}{ + match.JSONCheckOffDeprecated("rooms", []interface{}{ root, }, func(r gjson.Result) interface{} { return r.Get("room_id").Str }, nil), - match.JSONCheckOff(`rooms.#(room_type=="m.space")#`, []interface{}{ + match.JSONCheckOffDeprecated(`rooms.#(room_type=="m.space")#`, []interface{}{ rootToR1 + ";" + rootToSS1, }, roomToChildrenMapper, nil), }, @@ -493,12 +493,12 @@ func TestClientSpacesSummaryJoinRules(t *testing.T) { res = bob.MustDo(t, "GET", []string{"_matrix", "client", "v1", "rooms", root, "hierarchy"}) must.MatchResponse(t, res, match.HTTPResponse{ JSON: []match.JSON{ - match.JSONCheckOff("rooms", []interface{}{ + match.JSONCheckOffDeprecated("rooms", []interface{}{ root, r1, }, func(r gjson.Result) interface{} { return r.Get("room_id").Str }, nil), - match.JSONCheckOff(`rooms.#(room_type=="m.space")#`, []interface{}{ + match.JSONCheckOffDeprecated(`rooms.#(room_type=="m.space")#`, []interface{}{ rootToR1 + ";" + rootToSS1, }, roomToChildrenMapper, nil), }, @@ -510,12 +510,12 @@ func TestClientSpacesSummaryJoinRules(t *testing.T) { res = bob.MustDo(t, "GET", []string{"_matrix", "client", "v1", "rooms", root, "hierarchy"}) must.MatchResponse(t, res, match.HTTPResponse{ JSON: []match.JSON{ - match.JSONCheckOff("rooms", []interface{}{ + match.JSONCheckOffDeprecated("rooms", []interface{}{ root, r1, ss1, r2, r3, }, func(r gjson.Result) interface{} { return r.Get("room_id").Str }, nil), - match.JSONCheckOff(`rooms.#(room_type=="m.space")#`, []interface{}{ + match.JSONCheckOffDeprecated(`rooms.#(room_type=="m.space")#`, []interface{}{ rootToR1 + ";" + rootToSS1, ss1ToR2 + ";" + ss1ToR3, }, roomToChildrenMapper, nil), }, @@ -641,7 +641,7 @@ func TestFederatedClientSpaces(t *testing.T) { res := alice.MustDo(t, "GET", []string{"_matrix", "client", "v1", "rooms", root, "hierarchy"}) must.MatchResponse(t, res, match.HTTPResponse{ JSON: []match.JSON{ - match.JSONCheckOff("rooms", []interface{}{ + match.JSONCheckOffDeprecated("rooms", []interface{}{ root, r1, r2, r3, r4, ss1, ss2, }, func(r gjson.Result) interface{} { return r.Get("room_id").Str diff --git a/tests/room_timestamp_to_event_test.go b/tests/room_timestamp_to_event_test.go index 7c669847..41bcb57c 100644 --- a/tests/room_timestamp_to_event_test.go +++ b/tests/room_timestamp_to_event_test.go @@ -222,9 +222,9 @@ func TestJumpToDateEndpoint(t *testing.T) { // Make sure both messages are visible must.MatchResponse(t, messagesRes, match.HTTPResponse{ JSON: []match.JSON{ - match.JSONCheckOffAllowUnwanted("chunk", []interface{}{eventA.EventID, eventB.EventID}, func(r gjson.Result) interface{} { + match.JSONCheckOff("chunk", []interface{}{eventA.EventID, eventB.EventID}, match.CheckOffMapper(func(r gjson.Result) interface{} { return r.Get("event_id").Str - }, nil), + }), match.CheckOffAllowUnwanted()), }, }) }) From 0f482b4c6b4994d5524c3c351ff315864ee4c221 Mon Sep 17 00:00:00 2001 From: kegsay <7190048+kegsay@users.noreply.github.com> Date: Tue, 20 Feb 2024 15:06:12 +0000 Subject: [PATCH 2/2] Add tests for MSC4102 (#709) * Add failing test for https://github.com/matrix-org/matrix-spec/issues/1727 * Logs are fun * Update with semantics of MSC4102 * Also test over federation --- tests/csapi/thread_notifications_test.go | 51 ++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/tests/csapi/thread_notifications_test.go b/tests/csapi/thread_notifications_test.go index de440d49..3acfa462 100644 --- a/tests/csapi/thread_notifications_test.go +++ b/tests/csapi/thread_notifications_test.go @@ -301,3 +301,54 @@ func TestThreadedReceipts(t *testing.T) { }), ) } + +// Regression test for https://github.com/matrix-org/matrix-spec/issues/1727 +// Servers should always prefer the unthreaded receipt when there is a clash of receipts +func TestThreadReceiptsInSyncMSC4102(t *testing.T) { + runtime.SkipIf(t, runtime.Dendrite) // not supported + deployment := complement.Deploy(t, 2) + defer deployment.Destroy(t) + + // Create a room with alice and bob. + alice := deployment.Register(t, "hs1", helpers.RegistrationOpts{}) + bob := deployment.Register(t, "hs2", helpers.RegistrationOpts{}) + roomID := alice.MustCreateRoom(t, map[string]interface{}{"preset": "public_chat"}) + bob.MustJoinRoom(t, roomID, []string{"hs1"}) + eventA := alice.SendEventSynced(t, roomID, b.Event{ + Type: "m.room.message", + Content: map[string]interface{}{ + "msgtype": "m.text", + "body": "Hello world!", + }, + }) + eventB := alice.SendEventSynced(t, roomID, b.Event{ + Type: "m.room.message", + Content: map[string]interface{}{ + "msgtype": "m.text", + "body": "Start thread!", + "m.relates_to": map[string]interface{}{ + "event_id": eventA, + "rel_type": "m.thread", + }, + }, + }) + // now send an unthreaded RR for event B and a threaded RR for event B and ensure we see the unthreaded RR + // down /sync. Non-compliant servers will typically send the last one only. + alice.MustDo(t, "POST", []string{"_matrix", "client", "v3", "rooms", roomID, "receipt", "m.read", eventB}, client.WithJSONBody(t, struct{}{})) + alice.MustDo(t, "POST", []string{"_matrix", "client", "v3", "rooms", roomID, "receipt", "m.read", eventB}, client.WithJSONBody(t, map[string]interface{}{"thread_id": eventA})) + + alice.MustSyncUntil( + t, + client.SyncReq{}, + syncHasUnthreadedReadReceipt(roomID, alice.UserID, eventB), + ) + + // bob over federation must also see the same result, to show that the receipt EDUs over + // federation are bundled correctly, or are sent as separate EDUs. + bob.MustSyncUntil( + t, + client.SyncReq{}, + syncHasUnthreadedReadReceipt(roomID, alice.UserID, eventB), + ) + +}