diff --git a/backend/plugins/tempo/api/remote.go b/backend/plugins/tempo/api/remote.go index d8dd4ee2520..40b2f53b76b 100644 --- a/backend/plugins/tempo/api/remote.go +++ b/backend/plugins/tempo/api/remote.go @@ -58,12 +58,14 @@ func listTempoRemoteTeams( return nil, nil, errors.Default.Wrap(err, "failed to get teams from Tempo API") } + // Tempo v4 PageableMetadata has count/limit/offset/next/previous but no + // total: "next" links the next page and is absent on the last one. var response struct { Metadata struct { - Count int `json:"count"` - Limit int `json:"limit"` - Offset int `json:"offset"` - Total int `json:"total"` + Count int `json:"count"` + Limit int `json:"limit"` + Offset int `json:"offset"` + Next string `json:"next"` } `json:"metadata"` Results []models.TempoTeamResponse `json:"results"` } @@ -83,7 +85,7 @@ func listTempoRemoteTeams( }) } - if page.Offset+page.Limit < response.Metadata.Total { + if response.Metadata.Next != "" { nextPage = &TempoRemotePagination{ Limit: page.Limit, Offset: page.Offset + page.Limit, diff --git a/backend/plugins/tempo/api/remote_test.go b/backend/plugins/tempo/api/remote_test.go new file mode 100644 index 00000000000..c0aee40316b --- /dev/null +++ b/backend/plugins/tempo/api/remote_test.go @@ -0,0 +1,94 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with +this work for additional information regarding copyright ownership. +The ASF licenses this file to You under the Apache License, Version 2.0 +(the "License"); you may not use this file except in compliance with +the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package api + +import ( + "io" + "net/http" + "net/url" + "strings" + "testing" + + "github.com/apache/devlake/core/errors" + "github.com/apache/devlake/core/plugin" + "github.com/apache/devlake/plugins/tempo/models" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// fakeTempoClient answers every GET with the same body and records the query. +type fakeTempoClient struct { + body string + query url.Values +} + +func (c *fakeTempoClient) Get(_ string, query url.Values, _ http.Header) (*http.Response, errors.Error) { + c.query = query + return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(strings.NewReader(c.body))}, nil +} + +func (c *fakeTempoClient) Post(string, url.Values, interface{}, http.Header) (*http.Response, errors.Error) { + return nil, errors.Default.New("not implemented") +} +func (c *fakeTempoClient) SetData(string, interface{}) {} +func (c *fakeTempoClient) GetData(string) interface{} { return nil } +func (c *fakeTempoClient) SetHeaders(map[string]string) {} +func (c *fakeTempoClient) SetBeforeFunction(plugin.ApiClientBeforeRequest) {} +func (c *fakeTempoClient) GetBeforeFunction() plugin.ApiClientBeforeRequest { return nil } +func (c *fakeTempoClient) SetAfterFunction(plugin.ApiClientAfterResponse) {} +func (c *fakeTempoClient) GetAfterFunction() plugin.ApiClientAfterResponse { return nil } + +func TestListTempoRemoteTeamsPagination(t *testing.T) { + connection := &models.TempoConnection{} + connection.ID = 1 + + t.Run("follows metadata.next, which Tempo v4 returns instead of a total", func(t *testing.T) { + // shape of GET /4/teams (PageableTeam): no "total" in metadata + client := &fakeTempoClient{body: `{ + "self": "https://api.tempo.io/4/teams?offset=0&limit=2", + "metadata": {"count": 2, "offset": 0, "limit": 2, + "next": "https://api.tempo.io/4/teams?offset=2&limit=2"}, + "results": [ + {"id": 17, "name": "Backend", "summary": "", "key": ""}, + {"id": 71, "name": "Frontend", "summary": "", "key": ""} + ] + }`} + children, nextPage, err := listTempoRemoteTeams(connection, client, "", TempoRemotePagination{Limit: 2}) + require.NoError(t, err) + require.Len(t, children, 2) + assert.Equal(t, "17", children[0].Id) + assert.Equal(t, "0", client.query.Get("offset")) + assert.Equal(t, "2", client.query.Get("limit")) + require.NotNil(t, nextPage) + assert.Equal(t, TempoRemotePagination{Limit: 2, Offset: 2}, *nextPage) + }) + + t.Run("stops on the last page, which has no next link", func(t *testing.T) { + client := &fakeTempoClient{body: `{ + "self": "https://api.tempo.io/4/teams?offset=2&limit=2", + "metadata": {"count": 1, "offset": 2, "limit": 2, + "previous": "https://api.tempo.io/4/teams?offset=0&limit=2"}, + "results": [{"id": 90, "name": "QA", "summary": "", "key": ""}] + }`} + children, nextPage, err := listTempoRemoteTeams(connection, client, "", TempoRemotePagination{Limit: 2, Offset: 2}) + require.NoError(t, err) + require.Len(t, children, 1) + assert.Equal(t, "2", client.query.Get("offset")) + assert.Nil(t, nextPage) + }) +} diff --git a/backend/plugins/tempo/tasks/team_collector.go b/backend/plugins/tempo/tasks/team_collector.go index ab65383bcec..4c6d0a1d096 100644 --- a/backend/plugins/tempo/tasks/team_collector.go +++ b/backend/plugins/tempo/tasks/team_collector.go @@ -64,21 +64,8 @@ func CollectTeams(taskCtx plugin.SubTaskContext) errors.Error { ApiClient: data.ApiClient, UrlTemplate: "teams", PageSize: 50, - GetTotalPages: func(res *http.Response, args *api.ApiCollectorArgs) (int, errors.Error) { - var response struct { - Metadata struct { - Count int `json:"count"` - Limit int `json:"limit"` - Total int `json:"total"` - Offset int `json:"offset"` - } `json:"metadata"` - } - if err := api.UnmarshalResponse(res, &response); err != nil { - return 0, err - } - totalPages := (response.Metadata.Total + args.PageSize - 1) / args.PageSize - return totalPages, nil - }, + // No GetTotalPages: Tempo v4 pagination metadata (PageableMetadata) + // has no total, so pages are fetched until one comes back short. Query: func(reqData *api.RequestData) (url.Values, errors.Error) { query := url.Values{} pager := reqData.Pager