List token based pagination (#1041)

* WIP * WIP WIP * wip tests * Fix flag pagination test * WIP tests * Add ordering ability for tests * Fix other queries * Just use offset in token * Update default/max limits * Bump golangci-lint to fix local linter; simplify * deprecated linter config * Bump actions linter to match local * Fix mysql tests * Update export to use nextPageToken * try to fix lint action * add server tests * Add deprecated to protos * WIP more offset * rm console log * Reset Flags.vue * Rm max/defaults * Rm unused constants * more flag pagination tests * more tests; todo more rule pagination tests * rm default limit * Add offset server tests * Json changes * Update changelog
flipt-io · Oct 5, 2022 · 301ee60 · 301ee60
1 parent f94d704
commit 301ee60
Show file tree

Hide file tree

Showing 33 changed files with 1,776 additions and 637 deletions.
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -38,6 +38,9 @@ jobs:
         with:
           # Required: the version of golangci-lint is required and must be specified without patch version: we always use the latest patch version.
           version: v1.49
+          skip-pkg-cache: true
+          skip-build-cache: true
+          args: --timeout=10m
 
   test:
     name: Test

diff --git a/.golangci.yml b/.golangci.yml
@@ -14,9 +14,7 @@ run:
     - "bin"
     - "_tools"
     - "dist"
-    - "docs"
     - "rpc"
-    - "site"
     - "swagger"
     - "ui"
 
@@ -28,7 +26,6 @@ run:
 
 linters:
   enable:
-    - deadcode
     - depguard
     - errcheck
     - goconst
@@ -41,11 +38,9 @@ linters:
     - megacheck
     - misspell
     - staticcheck
-    - structcheck
     - stylecheck
     - unconvert
     - unparam
-    - varcheck
   disable:
     - contextcheck
     - exhaustive

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,6 +3,21 @@
 This format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## Unreleased
+
+### Added
+
+- Page token based pagination for `list` methods for forward compatibility with
+  future versions of the API [#936](https://github.com/flipt-io/flipt/issues/936)
+
+### Changed
+
+- Validation for `list` methods now requires a `limit` if requesting with an `offset` or `page_token`
+
+### Deprecated
+
+- Deprecated `offset` in `list` methods in favor of `page_token` [#936](https://github.com/flipt-io/flipt/issues/936)
+
 ## [v1.12.1](https://github.com/markphelps/flipt/releases/tag/v1.12.1) - 2022-09-30
 
 ### Fixed

diff --git a/Taskfile.yml b/Taskfile.yml
@@ -137,7 +137,7 @@ tasks:
   test:
     desc: Run all the tests
     cmds:
-      - go test {{.TEST_OPTS}} -covermode=atomic -count=1 -coverprofile={{.COVERAGE_FILE}} {{.SOURCE_FILES}} -run={{.TEST_PATTERN}} -timeout=30s
+      - go test {{.TEST_OPTS}} -covermode=atomic -count=1 -coverprofile={{.COVERAGE_FILE}} {{.SOURCE_FILES}} -run={{.TEST_PATTERN}} -timeout=60s
     vars:
       TEST_PATTERN: '{{.TEST_PATTERN | default "."}}'
       TEST_OPTS: '{{.TEST_OPTS | default "-race"}}'

diff --git a/cmd/flipt/export.go b/cmd/flipt/export.go
@@ -44,11 +44,11 @@ func runExport(ctx context.Context, logger *zap.Logger) error {
 
 	switch driver {
 	case sql.SQLite:
-		store = sqlite.NewStore(db)
+		store = sqlite.NewStore(db, logger)
 	case sql.Postgres:
-		store = postgres.NewStore(db)
+		store = postgres.NewStore(db, logger)
 	case sql.MySQL:
-		store = mysql.NewStore(db)
+		store = mysql.NewStore(db, logger)
 	}
 
 	// default to stdout

diff --git a/cmd/flipt/import.go b/cmd/flipt/import.go
@@ -48,11 +48,11 @@ func runImport(ctx context.Context, logger *zap.Logger, args []string) error {
 
 	switch driver {
 	case sql.SQLite:
-		store = sqlite.NewStore(db)
+		store = sqlite.NewStore(db, logger)
 	case sql.Postgres:
-		store = postgres.NewStore(db)
+		store = postgres.NewStore(db, logger)
 	case sql.MySQL:
-		store = mysql.NewStore(db)
+		store = mysql.NewStore(db, logger)
 	}
 
 	var in io.ReadCloser = os.Stdin

diff --git a/cmd/flipt/main.go b/cmd/flipt/main.go
@@ -426,11 +426,11 @@ func run(ctx context.Context, logger *zap.Logger) error {
 
 		switch driver {
 		case sql.SQLite:
-			store = sqlite.NewStore(db)
+			store = sqlite.NewStore(db, logger)
 		case sql.Postgres:
-			store = postgres.NewStore(db)
+			store = postgres.NewStore(db, logger)
 		case sql.MySQL:
-			store = mysql.NewStore(db)
+			store = mysql.NewStore(db, logger)
 		}
 
 		logger.Debug("store enabled", zap.Stringer("driver", store))

diff --git a/internal/ext/exporter.go b/internal/ext/exporter.go
@@ -14,9 +14,9 @@ import (
 const defaultBatchSize = 25
 
 type lister interface {
-	ListFlags(ctx context.Context, opts ...storage.QueryOption) ([]*flipt.Flag, error)
-	ListSegments(ctx context.Context, opts ...storage.QueryOption) ([]*flipt.Segment, error)
-	ListRules(ctx context.Context, flagKey string, opts ...storage.QueryOption) ([]*flipt.Rule, error)
+	ListFlags(ctx context.Context, opts ...storage.QueryOption) (storage.ResultSet[*flipt.Flag], error)
+	ListSegments(ctx context.Context, opts ...storage.QueryOption) (storage.ResultSet[*flipt.Segment], error)
+	ListRules(ctx context.Context, flagKey string, opts ...storage.QueryOption) (storage.ResultSet[*flipt.Rule], error)
 }
 
 type Exporter struct {
@@ -40,16 +40,21 @@ func (e *Exporter) Export(ctx context.Context, w io.Writer) error {
 
 	defer enc.Close()
 
-	var remaining = true
+	var (
+		remaining = true
+		nextPage  string
+	)
 
 	// export flags/variants in batches
 	for batch := uint64(0); remaining; batch++ {
-		flags, err := e.store.ListFlags(ctx, storage.WithOffset(batch*batchSize), storage.WithLimit(batchSize))
+		resp, err := e.store.ListFlags(ctx, storage.WithPageToken(nextPage), storage.WithLimit(batchSize))
 		if err != nil {
 			return fmt.Errorf("getting flags: %w", err)
 		}
 
-		remaining = uint64(len(flags)) == batchSize
+		flags := resp.Results
+		nextPage = resp.NextPageToken
+		remaining = nextPage != ""
 
 		for _, f := range flags {
 			flag := &Flag{
@@ -82,11 +87,12 @@ func (e *Exporter) Export(ctx context.Context, w io.Writer) error {
 			}
 
 			// export rules for flag
-			rules, err := e.store.ListRules(ctx, flag.Key)
+			resp, err := e.store.ListRules(ctx, flag.Key)
 			if err != nil {
 				return fmt.Errorf("getting rules for flag %q: %w", flag.Key, err)
 			}
 
+			rules := resp.Results
 			for _, r := range rules {
 				rule := &Rule{
 					SegmentKey: r.SegmentKey,
@@ -108,15 +114,18 @@ func (e *Exporter) Export(ctx context.Context, w io.Writer) error {
 	}
 
 	remaining = true
+	nextPage = ""
 
 	// export segments/constraints in batches
 	for batch := uint64(0); remaining; batch++ {
-		segments, err := e.store.ListSegments(ctx, storage.WithOffset(batch*batchSize), storage.WithLimit(batchSize))
+		resp, err := e.store.ListSegments(ctx, storage.WithPageToken(nextPage), storage.WithLimit(batchSize))
 		if err != nil {
 			return fmt.Errorf("getting segments: %w", err)
 		}
 
-		remaining = uint64(len(segments)) == batchSize
+		segments := resp.Results
+		nextPage = resp.NextPageToken
+		remaining = nextPage != ""
 
 		for _, s := range segments {
 			segment := &Segment{

diff --git a/internal/ext/exporter_test.go b/internal/ext/exporter_test.go
@@ -22,16 +22,22 @@ type mockLister struct {
 	ruleErr error
 }
 
-func (m mockLister) ListFlags(ctx context.Context, opts ...storage.QueryOption) ([]*flipt.Flag, error) {
-	return m.flags, m.flagErr
+func (m mockLister) ListFlags(ctx context.Context, opts ...storage.QueryOption) (storage.ResultSet[*flipt.Flag], error) {
+	return storage.ResultSet[*flipt.Flag]{
+		Results: m.flags,
+	}, m.flagErr
 }
 
-func (m mockLister) ListSegments(ctx context.Context, opts ...storage.QueryOption) ([]*flipt.Segment, error) {
-	return m.segments, m.segmentErr
+func (m mockLister) ListSegments(ctx context.Context, opts ...storage.QueryOption) (storage.ResultSet[*flipt.Segment], error) {
+	return storage.ResultSet[*flipt.Segment]{
+		Results: m.segments,
+	}, m.segmentErr
 }
 
-func (m mockLister) ListRules(ctx context.Context, flagKey string, opts ...storage.QueryOption) ([]*flipt.Rule, error) {
-	return m.rules, m.ruleErr
+func (m mockLister) ListRules(ctx context.Context, flagKey string, opts ...storage.QueryOption) (storage.ResultSet[*flipt.Rule], error) {
+	return storage.ResultSet[*flipt.Rule]{
+		Results: m.rules,
+	}, m.ruleErr
 }
 
 func TestExport(t *testing.T) {