From e225659acc9a33e12483e0011380e341114bc932 Mon Sep 17 00:00:00 2001 From: Austin Butler Date: Wed, 9 Sep 2026 16:17:09 -0700 Subject: [PATCH 1/2] fix: migrate flag listing to REST API 20240415 with pagination --- CHANGELOG.md | 4 + internal/ldclient/flags.go | 28 +++- internal/ldclient/flags_test.go | 235 ++++++++++++++++++++++++++++++++ 3 files changed, 265 insertions(+), 2 deletions(-) create mode 100644 internal/ldclient/flags_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 12ee4966..2e400433 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ ### Fixed +- Update flag listing from deprecated REST API version `20220603` to the current version, + [`20240415`](https://launchdarkly.com/docs/guides/api/api-migration-guide), including pagination support + for active and archived flags + ## 2.3.0 ### Added diff --git a/internal/ldclient/flags.go b/internal/ldclient/flags.go index 12a5ab0b..e2a04ff6 100644 --- a/internal/ldclient/flags.go +++ b/internal/ldclient/flags.go @@ -5,6 +5,7 @@ import ( "fmt" "net/http" "net/url" + "strconv" ldapi "github.com/launchdarkly/api-client-go/v15" lcr "github.com/launchdarkly/find-code-references-in-pull-request/config" @@ -39,15 +40,38 @@ func GetAllFlags(config *lcr.Config) ([]ldapi.FeatureFlag, error) { } func getFlags(config *lcr.Config, params url.Values) ([]ldapi.FeatureFlag, error) { - url := fmt.Sprintf("%s/api/v2/flags/%s", config.LdInstance, config.LdProject) + pageParams := make(url.Values, len(params)+2) + for key, values := range params { + pageParams[key] = append([]string(nil), values...) + } + pageParams.Set("limit", "100") + client := &http.Client{} + flags := []ldapi.FeatureFlag{} + for offset := 0; ; { + pageParams.Set("offset", strconv.Itoa(offset)) + page, err := getFlagPage(client, config, pageParams) + if err != nil { + return []ldapi.FeatureFlag{}, err + } + // A short page does not guarantee that there are no more flags. + if len(page) == 0 { + return flags, nil + } + flags = append(flags, page...) + offset += len(page) + } +} + +func getFlagPage(client *http.Client, config *lcr.Config, params url.Values) ([]ldapi.FeatureFlag, error) { + url := fmt.Sprintf("%s/api/v2/flags/%s", config.LdInstance, config.LdProject) req, err := http.NewRequest(http.MethodGet, url, nil) if err != nil { return []ldapi.FeatureFlag{}, err } req.URL.RawQuery = params.Encode() req.Header.Add("Authorization", config.ApiToken) - req.Header.Add("LD-API-Version", "20220603") + req.Header.Add("LD-API-Version", "20240415") req.Header.Add("User-Agent", fmt.Sprintf("find-code-references-pr/%s", version.Version)) resp, err := client.Do(req) diff --git a/internal/ldclient/flags_test.go b/internal/ldclient/flags_test.go new file mode 100644 index 00000000..45593a3a --- /dev/null +++ b/internal/ldclient/flags_test.go @@ -0,0 +1,235 @@ +package ldapi + +import ( + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "net/url" + "strconv" + "sync/atomic" + "testing" + + ldapi "github.com/launchdarkly/api-client-go/v15" + lcr "github.com/launchdarkly/find-code-references-in-pull-request/config" + "github.com/launchdarkly/find-code-references-in-pull-request/internal/version" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestGetAllFlagsPagination(t *testing.T) { + tests := []struct { + name string + activePages []int + archivedPages []int + }{ + {name: "empty", activePages: []int{0}}, + {name: "one flag", activePages: []int{1, 0}}, + {name: "exactly one full page", activePages: []int{100, 0}}, + {name: "multiple active pages", activePages: []int{100, 1, 0}}, + {name: "multiple active and archived pages", activePages: []int{100, 1, 0}, archivedPages: []int{100, 2, 0}}, + {name: "empty active collection", activePages: []int{0}, archivedPages: []int{2, 0}}, + {name: "empty archived collection", activePages: []int{1, 0}, archivedPages: []int{0}}, + {name: "short pages", activePages: []int{2, 1, 0}, archivedPages: []int{1, 0}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var pages []flagPageResponse + wantKeys := []string{} + for collection, sizes := range [][]int{tt.activePages, tt.archivedPages} { + filter, prefix := "", "active" + if collection == 1 { + filter, prefix = "state:archived", "archived" + } + offset := 0 + for _, count := range sizes { + items := make([]map[string]string, count) + for i := range items { + key := fmt.Sprintf("%s-%d", prefix, offset+i) + items[i] = map[string]string{"key": key} + wantKeys = append(wantKeys, key) + } + body, err := json.Marshal(map[string]any{"items": items}) + require.NoError(t, err) + pages = append(pages, flagPageResponse{offset: offset, filter: filter, body: string(body)}) + offset += count + } + } + config := serveFlagPages(t, tt.archivedPages != nil, pages) + + flags, err := GetAllFlags(config) + + require.NoError(t, err) + gotKeys := make([]string, len(flags)) + for i, flag := range flags { + gotKeys[i] = flag.Key + } + assert.Equal(t, wantKeys, gotKeys) + }) + } +} + +func TestGetAllFlagsPreservesMetadata(t *testing.T) { + config := serveFlagPages(t, false, []flagPageResponse{ + {body: `{"items":[{"key":"checkout","name":"Checkout","kind":"boolean","tags":["billing"], + "environments":{"production":{"on":true,"_environmentName":"Production", + "_site":{"href":"/test-project/production/features/checkout","type":"text/html"}}}}]}`}, + {offset: 1, body: `{"items":[]}`}, + }) + + flags, err := GetAllFlags(config) + + require.NoError(t, err) + require.Len(t, flags, 1) + assert.Equal(t, ldapi.FeatureFlag{ + Key: "checkout", + Name: "Checkout", + Kind: "boolean", + Tags: []string{"billing"}, + Environments: map[string]ldapi.FeatureFlagConfig{ + "production": { + On: true, + EnvironmentName: "Production", + Site: ldapi.Link{ + Href: ldapi.PtrString("/test-project/production/features/checkout"), + Type: ldapi.PtrString("text/html"), + }, + }, + }, + }, flags[0]) +} + +func TestGetFlagsPreservesQueryParameters(t *testing.T) { + config := serveFlagPages(t, true, []flagPageResponse{ + {filter: "state:archived", body: `{"items":[{"key":"archived"}]}`}, + {offset: 1, filter: "state:archived", body: `{"items":[]}`}, + }) + params := url.Values{ + "env": {"production"}, + "filter": {"state:archived"}, + "limit": {"20"}, + "offset": {"42"}, + } + want := url.Values{ + "env": {"production"}, + "filter": {"state:archived"}, + "limit": {"20"}, + "offset": {"42"}, + } + + flags, err := getFlags(config, params) + + require.NoError(t, err) + assert.Len(t, flags, 1) + assert.Equal(t, want, params) +} + +func TestGetAllFlagsErrors(t *testing.T) { + tests := []struct { + name string + status int + body string + wantError string + }{ + { + name: "unauthorized", status: http.StatusUnauthorized, + body: `{"message":"unauthorized"}`, wantError: "401", + }, + { + name: "server error", status: http.StatusInternalServerError, + body: `{"message":"unavailable"}`, wantError: "500", + }, + { + name: "non-JSON error", status: http.StatusBadGateway, + body: "bad gateway", wantError: "502. unable to parse response", + }, + { + name: "invalid success JSON", status: http.StatusOK, + body: "not JSON", wantError: "invalid character", + }, + } + for _, tt := range tests { + for _, archived := range []bool{false, true} { + for _, laterPage := range []bool{false, true} { + name := fmt.Sprintf("%s/archived=%t/laterPage=%t", tt.name, archived, laterPage) + t.Run(name, func(t *testing.T) { + var pages []flagPageResponse + filter := "" + if archived { + pages = append(pages, + flagPageResponse{body: `{"items":[{"key":"active"}]}`}, + flagPageResponse{offset: 1, body: `{"items":[]}`}, + ) + filter = "state:archived" + } + offset := 0 + if laterPage { + pages = append(pages, flagPageResponse{filter: filter, body: `{"items":[{"key":"first"}]}`}) + offset = 1 + } + pages = append(pages, flagPageResponse{offset: offset, filter: filter, status: tt.status, body: tt.body}) + config := serveFlagPages(t, archived, pages) + + flags, err := GetAllFlags(config) + + require.ErrorContains(t, err, tt.wantError) + assert.Empty(t, flags) + }) + } + } + } +} + +type flagPageResponse struct { + offset int + filter string + status int + body string +} + +func serveFlagPages(t *testing.T, includeArchived bool, pages []flagPageResponse) *lcr.Config { + t.Helper() + var requests atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + index := int(requests.Add(1)) - 1 + if !assert.Less(t, index, len(pages), "unexpected flag-list request") { + http.Error(w, "unexpected flag-list request", http.StatusInternalServerError) + return + } + page := pages[index] + assert.Equal(t, http.MethodGet, r.Method) + assert.Equal(t, "/api/v2/flags/test-project", r.URL.Path) + assert.Equal(t, "test-api-token", r.Header.Get("Authorization")) + assert.Equal(t, "20240415", r.Header.Get("LD-API-Version")) + assert.Equal(t, "find-code-references-pr/"+version.Version, r.Header.Get("User-Agent")) + query := url.Values{ + "env": {"production"}, + "limit": {"100"}, + "offset": {strconv.Itoa(page.offset)}, + } + if page.filter != "" { + query.Set("filter", page.filter) + } + assert.Equal(t, query, r.URL.Query()) + w.Header().Set("Content-Type", "application/json") + status := page.status + if status == 0 { + status = http.StatusOK + } + w.WriteHeader(status) + _, err := w.Write([]byte(page.body)) + assert.NoError(t, err) + })) + t.Cleanup(func() { + server.Close() + assert.Equal(t, len(pages), int(requests.Load()), "flag-list request count") + }) + return &lcr.Config{ + LdInstance: server.URL, + LdProject: "test-project", + LdEnvironment: "production", + ApiToken: "test-api-token", + IncludeArchivedFlags: includeArchived, + } +} From 9749c1c820dffac2b30ef9cc60188715ea1c6f0b Mon Sep 17 00:00:00 2001 From: Austin Butler Date: Thu, 10 Sep 2026 16:45:55 -0700 Subject: [PATCH 2/2] fix: advance flag page offsets by request limit --- internal/ldclient/flags.go | 8 ++++---- internal/ldclient/flags_test.go | 18 ++++++++++-------- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/internal/ldclient/flags.go b/internal/ldclient/flags.go index e2a04ff6..4ea77561 100644 --- a/internal/ldclient/flags.go +++ b/internal/ldclient/flags.go @@ -44,22 +44,22 @@ func getFlags(config *lcr.Config, params url.Values) ([]ldapi.FeatureFlag, error for key, values := range params { pageParams[key] = append([]string(nil), values...) } - pageParams.Set("limit", "100") + const pageSize = 100 + pageParams.Set("limit", strconv.Itoa(pageSize)) client := &http.Client{} flags := []ldapi.FeatureFlag{} - for offset := 0; ; { + for offset := 0; ; offset += pageSize { pageParams.Set("offset", strconv.Itoa(offset)) page, err := getFlagPage(client, config, pageParams) if err != nil { return []ldapi.FeatureFlag{}, err } - // A short page does not guarantee that there are no more flags. + // The migration guide uses an empty page to signal completion. if len(page) == 0 { return flags, nil } flags = append(flags, page...) - offset += len(page) } } diff --git a/internal/ldclient/flags_test.go b/internal/ldclient/flags_test.go index 45593a3a..841e78b3 100644 --- a/internal/ldclient/flags_test.go +++ b/internal/ldclient/flags_test.go @@ -42,18 +42,20 @@ func TestGetAllFlagsPagination(t *testing.T) { if collection == 1 { filter, prefix = "state:archived", "archived" } - offset := 0 + keyOffset := 0 + requestOffset := 0 for _, count := range sizes { items := make([]map[string]string, count) for i := range items { - key := fmt.Sprintf("%s-%d", prefix, offset+i) + key := fmt.Sprintf("%s-%d", prefix, keyOffset+i) items[i] = map[string]string{"key": key} wantKeys = append(wantKeys, key) } body, err := json.Marshal(map[string]any{"items": items}) require.NoError(t, err) - pages = append(pages, flagPageResponse{offset: offset, filter: filter, body: string(body)}) - offset += count + pages = append(pages, flagPageResponse{offset: requestOffset, filter: filter, body: string(body)}) + keyOffset += count + requestOffset += 100 } } config := serveFlagPages(t, tt.archivedPages != nil, pages) @@ -75,7 +77,7 @@ func TestGetAllFlagsPreservesMetadata(t *testing.T) { {body: `{"items":[{"key":"checkout","name":"Checkout","kind":"boolean","tags":["billing"], "environments":{"production":{"on":true,"_environmentName":"Production", "_site":{"href":"/test-project/production/features/checkout","type":"text/html"}}}}]}`}, - {offset: 1, body: `{"items":[]}`}, + {offset: 100, body: `{"items":[]}`}, }) flags, err := GetAllFlags(config) @@ -103,7 +105,7 @@ func TestGetAllFlagsPreservesMetadata(t *testing.T) { func TestGetFlagsPreservesQueryParameters(t *testing.T) { config := serveFlagPages(t, true, []flagPageResponse{ {filter: "state:archived", body: `{"items":[{"key":"archived"}]}`}, - {offset: 1, filter: "state:archived", body: `{"items":[]}`}, + {offset: 100, filter: "state:archived", body: `{"items":[]}`}, }) params := url.Values{ "env": {"production"}, @@ -159,14 +161,14 @@ func TestGetAllFlagsErrors(t *testing.T) { if archived { pages = append(pages, flagPageResponse{body: `{"items":[{"key":"active"}]}`}, - flagPageResponse{offset: 1, body: `{"items":[]}`}, + flagPageResponse{offset: 100, body: `{"items":[]}`}, ) filter = "state:archived" } offset := 0 if laterPage { pages = append(pages, flagPageResponse{filter: filter, body: `{"items":[{"key":"first"}]}`}) - offset = 1 + offset = 100 } pages = append(pages, flagPageResponse{offset: offset, filter: filter, status: tt.status, body: tt.body}) config := serveFlagPages(t, archived, pages)