From 14f5de38fe6e2fd2e0ec686b56b2839fdad9c72a Mon Sep 17 00:00:00 2001 From: Matheus Pimenta Date: Mon, 13 Apr 2026 13:53:20 +0100 Subject: [PATCH 1/2] Support limit/skip pagination on changes endpoints Signed-off-by: Matheus Pimenta --- client.go | 6 +-- list_measurement_changes.go | 5 ++ list_measurement_changes_test.go | 6 +++ list_rank_changes.go | 11 +++++ list_rank_changes_test.go | 6 +++ list_rikishi_changes.go | 8 +++ list_shikona_changes.go | 5 ++ list_shikona_changes_test.go | 6 +++ tests/integration/list_rank_changes_test.go | 55 ++++++++++++++------- 9 files changed, 88 insertions(+), 20 deletions(-) diff --git a/client.go b/client.go index 54eac62..35333a4 100644 --- a/client.go +++ b/client.go @@ -30,7 +30,7 @@ type Client interface { // Error represents an error returned by the Sumo API. type Error struct { StatusCode int - Body []byte + Body string ReadBodyErr error } @@ -41,7 +41,7 @@ func (e *Error) Error() string { case len(e.Body) == 0: return fmt.Sprintf("sumoapi: received HTTP %d response with empty body", e.StatusCode) default: - return fmt.Sprintf("sumoapi: received HTTP %d response: %s", e.StatusCode, string(e.Body)) + return fmt.Sprintf("sumoapi: received HTTP %d response: %s", e.StatusCode, e.Body) } } @@ -104,7 +104,7 @@ func (c *client) doRequest(ctx context.Context, method, path string, query url.V b, readErr := io.ReadAll(resp.Body) return nil, &Error{ StatusCode: status, - Body: b, + Body: string(b), ReadBodyErr: readErr, } } diff --git a/list_measurement_changes.go b/list_measurement_changes.go index af76197..75c78ef 100644 --- a/list_measurement_changes.go +++ b/list_measurement_changes.go @@ -5,6 +5,11 @@ import "context" // ListMeasurementChangesAPI defines the methods available for listing rikishi measurement changes across bashos. type ListMeasurementChangesAPI interface { // ListMeasurementChanges calls the GET /api/measurements endpoint. + // + // Documented bugs: + // - When filtering by bashoId, the API rejects the sortOrder query + // parameter with 400 INVALID_QUERY_PARAMS, so callers cannot force a + // stable order on basho-scoped queries. ListMeasurementChanges(ctx context.Context, req ListRikishiChangesRequest) ([]Measurement, error) } diff --git a/list_measurement_changes_test.go b/list_measurement_changes_test.go index da60f97..9776ae6 100644 --- a/list_measurement_changes_test.go +++ b/list_measurement_changes_test.go @@ -174,6 +174,8 @@ func TestClient_ListMeasurementChanges(t *testing.T) { g.Expect(query.Get("rikishiId")).To(Equal("123")) g.Expect(query.Get("bashoId")).To(Equal("202501")) g.Expect(query.Get("sortOrder")).To(Equal("asc")) + g.Expect(query.Get("limit")).To(Equal("50")) + g.Expect(query.Get("skip")).To(Equal("10")) return nil }, response: &http.Response{ @@ -187,6 +189,8 @@ func TestClient_ListMeasurementChanges(t *testing.T) { RikishiID: 123, BashoID: &bashoID, SortOrder: "asc", + Limit: 50, + Skip: 10, }) g.Expect(err).ToNot(HaveOccurred()) @@ -208,6 +212,8 @@ func TestClient_ListMeasurementChanges(t *testing.T) { g.Expect(query.Has("rikishiId")).To(BeFalse()) g.Expect(query.Has("bashoId")).To(BeFalse()) g.Expect(query.Has("sortOrder")).To(BeFalse()) + g.Expect(query.Has("limit")).To(BeFalse()) + g.Expect(query.Has("skip")).To(BeFalse()) return nil }, response: &http.Response{ diff --git a/list_rank_changes.go b/list_rank_changes.go index 9aeafcc..a2e5d16 100644 --- a/list_rank_changes.go +++ b/list_rank_changes.go @@ -5,6 +5,17 @@ import "context" // ListRankChangesAPI defines the methods available for listing rikishi rank changes across bashos. type ListRankChangesAPI interface { // ListRankChanges calls the GET /api/ranks endpoint. + // + // Documented bugs: + // - When filtering by bashoId, the API does not return results in any + // deterministic order: the first record changes depending on the + // limit value (e.g. limit=3 returns rikishiId 8850 first, while + // limit>=50 returns rikishiId 19 first). Pagination via skip still + // yields the full set of unique records, but the order in which they + // arrive is not stable. + // - When filtering by bashoId, the API rejects the sortOrder query + // parameter with 400 INVALID_QUERY_PARAMS, so callers cannot force a + // stable order on basho-scoped queries. ListRankChanges(ctx context.Context, req ListRikishiChangesRequest) ([]Rank, error) } diff --git a/list_rank_changes_test.go b/list_rank_changes_test.go index c611804..011778b 100644 --- a/list_rank_changes_test.go +++ b/list_rank_changes_test.go @@ -167,6 +167,8 @@ func TestClient_ListRankChanges(t *testing.T) { g.Expect(query.Get("rikishiId")).To(Equal("123")) g.Expect(query.Get("bashoId")).To(Equal("202501")) g.Expect(query.Get("sortOrder")).To(Equal("asc")) + g.Expect(query.Get("limit")).To(Equal("50")) + g.Expect(query.Get("skip")).To(Equal("10")) return nil }, response: &http.Response{ @@ -180,6 +182,8 @@ func TestClient_ListRankChanges(t *testing.T) { RikishiID: 123, BashoID: &bashoID, SortOrder: "asc", + Limit: 50, + Skip: 10, }) g.Expect(err).ToNot(HaveOccurred()) @@ -200,6 +204,8 @@ func TestClient_ListRankChanges(t *testing.T) { g.Expect(query.Has("rikishiId")).To(BeFalse()) g.Expect(query.Has("bashoId")).To(BeFalse()) g.Expect(query.Has("sortOrder")).To(BeFalse()) + g.Expect(query.Has("limit")).To(BeFalse()) + g.Expect(query.Has("skip")).To(BeFalse()) return nil }, response: &http.Response{ diff --git a/list_rikishi_changes.go b/list_rikishi_changes.go index 4d044e3..19c9634 100644 --- a/list_rikishi_changes.go +++ b/list_rikishi_changes.go @@ -11,6 +11,8 @@ type ListRikishiChangesRequest struct { RikishiID int `json:"rikishiId,omitempty" jsonschema:"The ID of the rikishi (sumo wrestler) whose changes are to be listed. Cannot be used together with bashoId."` BashoID *BashoID `json:"bashoId,omitempty" jsonschema:"The ID of the basho (sumo tournament) for which rikishi (sumo wrestler) changes are to be listed. Cannot be used together with rikishiId."` SortOrder string `json:"sortOrder,omitempty" jsonschema:"The order in which to sort the results by basho (sumo tournament). Valid values are 'asc' for ascending and 'desc' for descending. Default is 'desc'."` + Limit int `json:"limit,omitempty" jsonschema:"The maximum number of results to return."` + Skip int `json:"skip,omitempty" jsonschema:"The number of results to skip over for pagination."` } func listRikishiChanges[obj any](ctx context.Context, c *client, path string, req ListRikishiChangesRequest) ([]obj, error) { @@ -24,5 +26,11 @@ func listRikishiChanges[obj any](ctx context.Context, c *client, path string, re if order := getSortOrder(req.SortOrder); order != "" { query.Set("sortOrder", order) } + if req.Limit > 0 { + query.Set("limit", fmt.Sprint(req.Limit)) + } + if req.Skip > 0 { + query.Set("skip", fmt.Sprint(req.Skip)) + } return listObjects[obj](ctx, c, path, query) } diff --git a/list_shikona_changes.go b/list_shikona_changes.go index 84e574e..4957bc4 100644 --- a/list_shikona_changes.go +++ b/list_shikona_changes.go @@ -5,6 +5,11 @@ import "context" // ListShikonaChangesAPI defines the methods available for listing rikishi shikona changes across bashos. type ListShikonaChangesAPI interface { // ListShikonaChanges calls the GET /api/shikonas endpoint. + // + // Documented bugs: + // - When filtering by bashoId, the API rejects the sortOrder query + // parameter with 400 INVALID_QUERY_PARAMS, so callers cannot force a + // stable order on basho-scoped queries. ListShikonaChanges(ctx context.Context, req ListRikishiChangesRequest) ([]Shikona, error) } diff --git a/list_shikona_changes_test.go b/list_shikona_changes_test.go index 0646658..2d4ce1a 100644 --- a/list_shikona_changes_test.go +++ b/list_shikona_changes_test.go @@ -172,6 +172,8 @@ func TestClient_ListShikonaChanges(t *testing.T) { g.Expect(query.Get("rikishiId")).To(Equal("123")) g.Expect(query.Get("bashoId")).To(Equal("202501")) g.Expect(query.Get("sortOrder")).To(Equal("asc")) + g.Expect(query.Get("limit")).To(Equal("50")) + g.Expect(query.Get("skip")).To(Equal("10")) return nil }, response: &http.Response{ @@ -185,6 +187,8 @@ func TestClient_ListShikonaChanges(t *testing.T) { RikishiID: 123, BashoID: &bashoID, SortOrder: "asc", + Limit: 50, + Skip: 10, }) g.Expect(err).ToNot(HaveOccurred()) @@ -206,6 +210,8 @@ func TestClient_ListShikonaChanges(t *testing.T) { g.Expect(query.Has("rikishiId")).To(BeFalse()) g.Expect(query.Has("bashoId")).To(BeFalse()) g.Expect(query.Has("sortOrder")).To(BeFalse()) + g.Expect(query.Has("limit")).To(BeFalse()) + g.Expect(query.Has("skip")).To(BeFalse()) return nil }, response: &http.Response{ diff --git a/tests/integration/list_rank_changes_test.go b/tests/integration/list_rank_changes_test.go index f48ba90..9b6f959 100644 --- a/tests/integration/list_rank_changes_test.go +++ b/tests/integration/list_rank_changes_test.go @@ -12,15 +12,31 @@ import ( func TestIntegration_ListRankChanges(t *testing.T) { client := sumoapi.New() + // Use a non-default limit to exercise the Limit input as well. + const pageLimit = 99 + + listAllRanks := func(g *WithT, req sumoapi.ListRikishiChangesRequest) []sumoapi.Rank { + req.Limit = pageLimit + var all []sumoapi.Rank + for { + req.Skip = len(all) + page, err := client.ListRankChanges(context.Background(), req) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(page).ToNot(BeNil()) + all = append(all, page...) + if len(page) < pageLimit { + return all + } + } + } + t.Run("for rikishi", func(t *testing.T) { g := NewWithT(t) - resp, err := client.ListRankChanges(context.Background(), sumoapi.ListRikishiChangesRequest{ + resp := listAllRanks(g, sumoapi.ListRikishiChangesRequest{ RikishiID: 3081, // Hakuho }) - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(resp).ToNot(BeNil()) g.Expect(resp).To(HaveLen(122)) expectedBashoID := sumoapi.BashoID{Year: 2021, Month: 9} @@ -48,24 +64,29 @@ func TestIntegration_ListRankChanges(t *testing.T) { Month: 9, } - resp, err := client.ListRankChanges(context.Background(), sumoapi.ListRikishiChangesRequest{ + resp := listAllRanks(g, sumoapi.ListRikishiChangesRequest{ BashoID: &bashoID, }) - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(resp).ToNot(BeNil()) g.Expect(resp).To(HaveLen(611)) - g.Expect(resp[0].ID).To(Equal(sumoapi.RikishiChangeID{BashoID: bashoID, RikishiID: 8850})) - g.Expect(resp[0].BashoID).To(Equal(bashoID)) - g.Expect(resp[0].RikishiID).To(Equal(8850)) - g.Expect(resp[0].HumanReadableName).To(Equal("Yokozuna 1 East")) - g.Expect(resp[0].NumericName).To(Equal(101)) - - g.Expect(resp[610].ID).To(Equal(sumoapi.RikishiChangeID{BashoID: bashoID, RikishiID: 9101})) - g.Expect(resp[610].BashoID).To(Equal(bashoID)) - g.Expect(resp[610].RikishiID).To(Equal(9101)) - g.Expect(resp[610].HumanReadableName).To(Equal("Jonokuchi 26 East")) - g.Expect(resp[610].NumericName).To(Equal(1026)) + // The /ranks endpoint does not return basho-scoped results in any + // deterministic order (the first record changes depending on the + // limit value), so assert membership rather than position. Pagination + // still yields all 611 unique records. + g.Expect(resp).To(ContainElement(sumoapi.Rank{ + ID: sumoapi.RikishiChangeID{BashoID: bashoID, RikishiID: 8850}, + BashoID: bashoID, + RikishiID: 8850, + HumanReadableName: "Yokozuna 1 East", + NumericName: 101, + })) + g.Expect(resp).To(ContainElement(sumoapi.Rank{ + ID: sumoapi.RikishiChangeID{BashoID: bashoID, RikishiID: 9101}, + BashoID: bashoID, + RikishiID: 9101, + HumanReadableName: "Jonokuchi 26 East", + NumericName: 1026, + })) }) } From d5443a87740e35c40c9cc0b5e27f1fbdaade524d Mon Sep 17 00:00:00 2001 From: Matheus Pimenta Date: Mon, 20 Apr 2026 08:10:44 +0100 Subject: [PATCH 2/2] Address review comments Signed-off-by: Matheus Pimenta --- list_measurement_changes.go | 5 ----- list_rank_changes.go | 12 ++---------- list_shikona_changes.go | 5 ----- tests/integration/list_rank_changes_test.go | 19 ++++--------------- 4 files changed, 6 insertions(+), 35 deletions(-) diff --git a/list_measurement_changes.go b/list_measurement_changes.go index 75c78ef..af76197 100644 --- a/list_measurement_changes.go +++ b/list_measurement_changes.go @@ -5,11 +5,6 @@ import "context" // ListMeasurementChangesAPI defines the methods available for listing rikishi measurement changes across bashos. type ListMeasurementChangesAPI interface { // ListMeasurementChanges calls the GET /api/measurements endpoint. - // - // Documented bugs: - // - When filtering by bashoId, the API rejects the sortOrder query - // parameter with 400 INVALID_QUERY_PARAMS, so callers cannot force a - // stable order on basho-scoped queries. ListMeasurementChanges(ctx context.Context, req ListRikishiChangesRequest) ([]Measurement, error) } diff --git a/list_rank_changes.go b/list_rank_changes.go index a2e5d16..92e50c3 100644 --- a/list_rank_changes.go +++ b/list_rank_changes.go @@ -6,16 +6,8 @@ import "context" type ListRankChangesAPI interface { // ListRankChanges calls the GET /api/ranks endpoint. // - // Documented bugs: - // - When filtering by bashoId, the API does not return results in any - // deterministic order: the first record changes depending on the - // limit value (e.g. limit=3 returns rikishiId 8850 first, while - // limit>=50 returns rikishiId 19 first). Pagination via skip still - // yields the full set of unique records, but the order in which they - // arrive is not stable. - // - When filtering by bashoId, the API rejects the sortOrder query - // parameter with 400 INVALID_QUERY_PARAMS, so callers cannot force a - // stable order on basho-scoped queries. + // Ordering: results are sorted by rank (always ascending) when filtering + // by bashoId, and by bashoId when filtering by rikishiId. ListRankChanges(ctx context.Context, req ListRikishiChangesRequest) ([]Rank, error) } diff --git a/list_shikona_changes.go b/list_shikona_changes.go index 4957bc4..84e574e 100644 --- a/list_shikona_changes.go +++ b/list_shikona_changes.go @@ -5,11 +5,6 @@ import "context" // ListShikonaChangesAPI defines the methods available for listing rikishi shikona changes across bashos. type ListShikonaChangesAPI interface { // ListShikonaChanges calls the GET /api/shikonas endpoint. - // - // Documented bugs: - // - When filtering by bashoId, the API rejects the sortOrder query - // parameter with 400 INVALID_QUERY_PARAMS, so callers cannot force a - // stable order on basho-scoped queries. ListShikonaChanges(ctx context.Context, req ListRikishiChangesRequest) ([]Shikona, error) } diff --git a/tests/integration/list_rank_changes_test.go b/tests/integration/list_rank_changes_test.go index 9b6f959..ca87703 100644 --- a/tests/integration/list_rank_changes_test.go +++ b/tests/integration/list_rank_changes_test.go @@ -70,23 +70,12 @@ func TestIntegration_ListRankChanges(t *testing.T) { g.Expect(resp).To(HaveLen(611)) - // The /ranks endpoint does not return basho-scoped results in any - // deterministic order (the first record changes depending on the - // limit value), so assert membership rather than position. Pagination - // still yields all 611 unique records. - g.Expect(resp).To(ContainElement(sumoapi.Rank{ - ID: sumoapi.RikishiChangeID{BashoID: bashoID, RikishiID: 8850}, + g.Expect(resp[0]).To(Equal(sumoapi.Rank{ + ID: sumoapi.RikishiChangeID{BashoID: bashoID, RikishiID: 19}, BashoID: bashoID, - RikishiID: 8850, - HumanReadableName: "Yokozuna 1 East", + RikishiID: 19, + HumanReadableName: "Yokozuna 1 West", NumericName: 101, })) - g.Expect(resp).To(ContainElement(sumoapi.Rank{ - ID: sumoapi.RikishiChangeID{BashoID: bashoID, RikishiID: 9101}, - BashoID: bashoID, - RikishiID: 9101, - HumanReadableName: "Jonokuchi 26 East", - NumericName: 1026, - })) }) }