diff --git a/backend/plugins/tempo/api/connection.go b/backend/plugins/tempo/api/connection.go index 12d8e4cd7e7..7f329507bba 100644 --- a/backend/plugins/tempo/api/connection.go +++ b/backend/plugins/tempo/api/connection.go @@ -190,15 +190,9 @@ func GetTeams(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors } // Get teams from Tempo API - res, err := apiClient.Get("teams", nil, nil) + teams, err := listAllTempoTeams(apiClient, nil) if err != nil { - return nil, errors.Default.Wrap(err, "failed to get teams from Tempo API") - } - - var teams []models.TempoTeamResponse - err = api.UnmarshalResponse(res, &teams) - if err != nil { - return nil, errors.Default.Wrap(err, "failed to unmarshal teams response") + return nil, err } // Convert to tool layer models diff --git a/backend/plugins/tempo/api/remote.go b/backend/plugins/tempo/api/remote.go index 40b2f53b76b..93181cc50cc 100644 --- a/backend/plugins/tempo/api/remote.go +++ b/backend/plugins/tempo/api/remote.go @@ -95,6 +95,42 @@ func listTempoRemoteTeams( return children, nextPage, nil } +// listAllTempoTeams fetches every page of GET /4/teams. Tempo v4 pagination +// metadata has no total: "next" links the next page and is absent on the last. +func listAllTempoTeams(apiClient plugin.ApiClient, query url.Values) ([]models.TempoTeamResponse, errors.Error) { + const pageSize = 50 + var teams []models.TempoTeamResponse + for offset := 0; ; offset += pageSize { + pageQuery := url.Values{} + for key, values := range query { + pageQuery[key] = values + } + pageQuery.Set("offset", strconv.Itoa(offset)) + pageQuery.Set("limit", strconv.Itoa(pageSize)) + + res, err := apiClient.Get("teams", pageQuery, nil) + if err != nil { + return nil, errors.Default.Wrap(err, "failed to get teams from Tempo API") + } + + var response struct { + Metadata struct { + Next string `json:"next"` + } `json:"metadata"` + Results []models.TempoTeamResponse `json:"results"` + } + err = api.UnmarshalResponse(res, &response) + if err != nil { + return nil, errors.Default.Wrap(err, "failed to unmarshal teams response") + } + + teams = append(teams, response.Results...) + if response.Metadata.Next == "" || len(response.Results) == 0 { + return teams, nil + } + } +} + // RemoteScopes list all available scopes on the remote server // @Summary list all available scopes on the remote server // @Description list all available scopes on the remote server @@ -125,26 +161,14 @@ func searchTempoRemoteTeams( } } - res, err := apiClient.Get("teams", queryParams, nil) + // all matching teams, not just the first page of the API: the result is + // filtered and paged below + teams, err := listAllTempoTeams(apiClient, queryParams) if err != nil { - return nil, errors.Default.Wrap(err, "failed to get teams from Tempo API") + return nil, err } - var response struct { - Metadata struct { - Count int `json:"count"` - Limit int `json:"limit"` - Offset int `json:"offset"` - Total int `json:"total"` - } `json:"metadata"` - Results []models.TempoTeamResponse `json:"results"` - } - err = api.UnmarshalResponse(res, &response) - if err != nil { - return nil, errors.Default.Wrap(err, "failed to unmarshal teams response") - } - - for _, team := range response.Results { + for _, team := range teams { if params.Search == "" || strings.Contains(strings.ToLower(team.Name), strings.ToLower(params.Search)) { children = append(children, dsmodels.DsRemoteApiScopeListEntry[models.TempoTeam]{ Type: api.RAS_ENTRY_TYPE_SCOPE, diff --git a/backend/plugins/tempo/api/remote_test.go b/backend/plugins/tempo/api/remote_test.go index c0aee40316b..d13a28b0ba5 100644 --- a/backend/plugins/tempo/api/remote_test.go +++ b/backend/plugins/tempo/api/remote_test.go @@ -26,20 +26,34 @@ import ( "github.com/apache/devlake/core/errors" "github.com/apache/devlake/core/plugin" + dsmodels "github.com/apache/devlake/helpers/pluginhelper/api/models" "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. +// fakeTempoClient answers every GET with the same body, or with the body of +// the requested offset (0 by default) when pages is set, and records the +// queries. type fakeTempoClient struct { - body string - query url.Values + body string + pages map[string]string + query url.Values + queries []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 + c.queries = append(c.queries, query) + body := c.body + if c.pages != nil { + offset := query.Get("offset") + if offset == "" { + offset = "0" + } + body = c.pages[offset] + } + return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(strings.NewReader(body))}, nil } func (c *fakeTempoClient) Post(string, url.Values, interface{}, http.Header) (*http.Response, errors.Error) { @@ -92,3 +106,49 @@ func TestListTempoRemoteTeamsPagination(t *testing.T) { assert.Nil(t, nextPage) }) } + +// two pages of GET /4/teams (PageableTeam): the first links the second +var twoTeamPages = map[string]string{ + "0": `{ + "self": "https://api.tempo.io/4/teams?offset=0&limit=50", + "metadata": {"count": 2, "offset": 0, "limit": 50, + "next": "https://api.tempo.io/4/teams?offset=50&limit=50"}, + "results": [ + {"id": 17, "name": "Backend", "summary": "", "key": ""}, + {"id": 71, "name": "Frontend", "summary": "", "key": ""} + ] + }`, + "50": `{ + "self": "https://api.tempo.io/4/teams?offset=50&limit=50", + "metadata": {"count": 1, "offset": 50, "limit": 50, + "previous": "https://api.tempo.io/4/teams?offset=0&limit=50"}, + "results": [{"id": 90, "name": "Backend QA", "summary": "", "key": ""}] + }`, +} + +func TestListAllTempoTeams(t *testing.T) { + client := &fakeTempoClient{pages: twoTeamPages} + teams, err := listAllTempoTeams(client, url.Values{"name": {"x"}}) + require.NoError(t, err) + require.Len(t, teams, 3) + assert.Equal(t, int64(17), teams[0].Id) + assert.Equal(t, int64(90), teams[2].Id) + require.Len(t, client.queries, 2) + assert.Equal(t, "0", client.queries[0].Get("offset")) + assert.Equal(t, "50", client.queries[1].Get("offset")) + assert.Equal(t, "50", client.queries[1].Get("limit")) + assert.Equal(t, "x", client.queries[1].Get("name")) +} + +func TestSearchTempoRemoteTeamsSearchesEveryPage(t *testing.T) { + client := &fakeTempoClient{pages: twoTeamPages} + children, err := searchTempoRemoteTeams(client, &dsmodels.DsRemoteApiScopeSearchParams{ + Search: "backend", Page: 1, PageSize: 50, + }) + require.NoError(t, err) + require.Len(t, children, 2) + assert.Equal(t, "17", children[0].Id) + // on the second page of the Tempo API + assert.Equal(t, "90", children[1].Id) + assert.Equal(t, "backend", client.query.Get("name")) +}