Skip to content

Commit

Permalink
Refactor the match.CheckOff API to use functional options (#711)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
kegsay authored Feb 16, 2024
1 parent 6b3745a commit 503bf6e
Show file tree
Hide file tree
Showing 10 changed files with 140 additions and 100 deletions.
114 changes: 78 additions & 36 deletions match/json.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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
Expand All @@ -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
}
Expand All @@ -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
}
}
Expand All @@ -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
Expand All @@ -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
Expand Down
6 changes: 3 additions & 3 deletions tests/csapi/apidoc_search_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)]),
Expand All @@ -269,7 +269,7 @@ func TestSearch(t *testing.T) {
}
}
return nil
}),
})),
},
})
})
Expand Down
8 changes: 4 additions & 4 deletions tests/csapi/room_leave_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
})),
},
})
})
Expand All @@ -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),
})),
},
})
})
Expand Down
10 changes: 5 additions & 5 deletions tests/csapi/room_members_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
})
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
})
Expand All @@ -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,
})
Expand All @@ -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,
})
Expand Down
18 changes: 9 additions & 9 deletions tests/csapi/room_relations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
12 changes: 6 additions & 6 deletions tests/msc3874/room_messages_relation_filter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
Loading

0 comments on commit 503bf6e

Please sign in to comment.