From 0a273b9a81e06720197db037e161a50a18dc3398 Mon Sep 17 00:00:00 2001 From: huangjunwei Date: Tue, 16 Dec 2025 14:34:06 +0800 Subject: [PATCH] fix as copilot suggest Signed-off-by: huangjunwei --- pkg/storage/driver/cfgmaps.go | 6 +++--- pkg/storage/driver/cfgmaps_test.go | 8 ++++---- pkg/storage/driver/driver.go | 4 ++-- pkg/storage/driver/memory.go | 7 ++----- pkg/storage/driver/memory_test.go | 8 ++++---- pkg/storage/driver/mock_test.go | 18 ++++++++++++++++-- pkg/storage/driver/secrets.go | 6 +++--- pkg/storage/driver/secrets_test.go | 8 ++++---- pkg/storage/driver/sql.go | 20 ++++++++++++-------- pkg/storage/storage_test.go | 4 ++-- 10 files changed, 52 insertions(+), 37 deletions(-) diff --git a/pkg/storage/driver/cfgmaps.go b/pkg/storage/driver/cfgmaps.go index 7d1c53194..0b389027c 100644 --- a/pkg/storage/driver/cfgmaps.go +++ b/pkg/storage/driver/cfgmaps.go @@ -90,7 +90,7 @@ func (cfgmaps *ConfigMaps) Get(key string) (release.Releaser, error) { } // listPages is a common method to list release with pagination -func (cfgmaps *ConfigMaps) listPages(f func(page []release.Releaser, lastPage bool) (end bool), opts metav1.ListOptions, filter func(release.Releaser) bool) (err error) { +func (cfgmaps *ConfigMaps) listPages(f func(page []release.Releaser, remaining bool) (end bool), opts metav1.ListOptions, filter func(release.Releaser) bool) (err error) { if opts.Limit == 0 { opts.Limit = DefaultPaginationLimit } @@ -163,7 +163,7 @@ func (cfgmaps *ConfigMaps) List(filter func(release.Releaser) bool) ([]release.R } // ListPages same as List, but with pagination -func (cfgmaps *ConfigMaps) ListPages(f func(page []release.Releaser, lastPage bool) (end bool), limit int64, filter func(release.Releaser) bool) error { +func (cfgmaps *ConfigMaps) ListPages(f func(page []release.Releaser, remaining bool) (end bool), limit int64, filter func(release.Releaser) bool) error { lsel := kblabels.Set{"owner": owner}.AsSelector() opts := metav1.ListOptions{Limit: limit, LabelSelector: lsel.String()} @@ -207,7 +207,7 @@ func (cfgmaps *ConfigMaps) Query(labels map[string]string) ([]release.Releaser, } // QueryPages same as Query, but with pagination -func (cfgmaps *ConfigMaps) QueryPages(f func(page []release.Releaser, lastPage bool) (end bool), limit int64, labels map[string]string) error { +func (cfgmaps *ConfigMaps) QueryPages(f func(page []release.Releaser, remaining bool) (end bool), limit int64, labels map[string]string) error { ls := kblabels.Set{} for k, v := range labels { if errs := validation.IsValidLabelValue(v); len(errs) != 0 { diff --git a/pkg/storage/driver/cfgmaps_test.go b/pkg/storage/driver/cfgmaps_test.go index 353ff39ed..9778cef3e 100644 --- a/pkg/storage/driver/cfgmaps_test.go +++ b/pkg/storage/driver/cfgmaps_test.go @@ -112,13 +112,13 @@ func TestConfigMapList(t *testing.T) { }...) // list releases with pagination - err := cfgmaps.ListPages(func(page []release.Releaser, lastPage bool) (end bool) { + err := cfgmaps.ListPages(func(page []release.Releaser, remaining bool) (end bool) { // check if len(page) != 2 { t.Errorf("Expected 2 cfgmaps, got %d:\n%v\n", len(page), page) } - return !lastPage + return !remaining }, 2, func(rel release.Releaser) bool { return true }) // check if err != nil { @@ -199,12 +199,12 @@ func TestConfigMapQuery(t *testing.T) { } // query cfgmaps with pagination - err = cfgmaps.QueryPages(func(page []release.Releaser, lastPage bool) (err bool) { + err = cfgmaps.QueryPages(func(page []release.Releaser, remaining bool) (err bool) { if len(page) != 2 { t.Fatalf("Expected 2 results, actual %d", len(page)) } - return !lastPage + return !remaining }, 2, map[string]string{"status": "deployed"}) if err != nil { t.Fatalf("Failed to query: %s", err) diff --git a/pkg/storage/driver/driver.go b/pkg/storage/driver/driver.go index 0a2f02af6..d9445e344 100644 --- a/pkg/storage/driver/driver.go +++ b/pkg/storage/driver/driver.go @@ -96,9 +96,9 @@ type Deletor interface { type Queryor interface { Get(key string) (release.Releaser, error) List(filter func(release.Releaser) bool) ([]release.Releaser, error) - ListPages(f func(page []release.Releaser, lastPage bool) (end bool), limit int64, filter func(release.Releaser) bool) error + ListPages(f func(page []release.Releaser, remaining bool) (end bool), limit int64, filter func(release.Releaser) bool) error Query(labels map[string]string) ([]release.Releaser, error) - QueryPages(f func(page []release.Releaser, lastPage bool) (end bool), limit int64, labels map[string]string) error + QueryPages(f func(page []release.Releaser, remaining bool) (end bool), limit int64, labels map[string]string) error } // Driver is the interface composed of Creator, Updator, Deletor, and Queryor diff --git a/pkg/storage/driver/memory.go b/pkg/storage/driver/memory.go index fdc5e6dec..0158f8fbc 100644 --- a/pkg/storage/driver/memory.go +++ b/pkg/storage/driver/memory.go @@ -17,7 +17,6 @@ limitations under the License. package driver import ( - "fmt" "log/slog" "strconv" "strings" @@ -116,7 +115,7 @@ func (mem *Memory) List(filter func(release.Releaser) bool) ([]release.Releaser, } // ListPages is a common method to list release with pagination -func (mem *Memory) ListPages(f func(page []release.Releaser, lastPage bool) (end bool), limit int64, filter func(release.Releaser) bool) (err error) { +func (mem *Memory) ListPages(f func(page []release.Releaser, remaining bool) (end bool), limit int64, filter func(release.Releaser) bool) (err error) { defer unlock(mem.rlock()) if limit == 0 { @@ -223,7 +222,7 @@ func (mem *Memory) Query(keyvals map[string]string) ([]release.Releaser, error) } // QueryPages same as Query, but with pagination -func (mem *Memory) QueryPages(f func(page []release.Releaser, lastPage bool) (end bool), limit int64, keyvals map[string]string) error { +func (mem *Memory) QueryPages(f func(page []release.Releaser, remaining bool) (end bool), limit int64, keyvals map[string]string) error { defer unlock(mem.rlock()) if limit == 0 { @@ -235,8 +234,6 @@ func (mem *Memory) QueryPages(f func(page []release.Releaser, lastPage bool) (en lbs.init() lbs.fromMap(keyvals) - fmt.Printf("namespace: %s\n", mem.namespace) - ls := make([]release.Releaser, 0, int(limit)) if mem.namespace != "" { for _, recs := range mem.cache[mem.namespace] { diff --git a/pkg/storage/driver/memory_test.go b/pkg/storage/driver/memory_test.go index 61e4e1429..00af3ed5c 100644 --- a/pkg/storage/driver/memory_test.go +++ b/pkg/storage/driver/memory_test.go @@ -108,13 +108,13 @@ func TestMemoryList(t *testing.T) { ts.SetNamespace("default") // list releases with pagination - err := ts.ListPages(func(page []release.Releaser, lastPage bool) (end bool) { + err := ts.ListPages(func(page []release.Releaser, remaining bool) (end bool) { // check if len(page) != 2 { t.Errorf("Expected 2 releases, got %d:\n%v\n", len(page), page) } - return !lastPage + return !remaining }, 2, func(rel release.Releaser) bool { return true }) // check if err != nil { @@ -271,12 +271,12 @@ func TestMemoryDelete(t *testing.T) { ts.SetNamespace("") // query cfgmaps with pagination - err := ts.QueryPages(func(page []release.Releaser, lastPage bool) (err bool) { + err := ts.QueryPages(func(page []release.Releaser, remaining bool) (err bool) { if len(page) != 1 { t.Fatalf("Expected 1 results, actual %d", len(page)) } - return !lastPage + return !remaining }, 1, map[string]string{"status": "deployed"}) if err != nil { t.Fatalf("Failed to query: %s", err) diff --git a/pkg/storage/driver/mock_test.go b/pkg/storage/driver/mock_test.go index c426c8937..6adc41f2f 100644 --- a/pkg/storage/driver/mock_test.go +++ b/pkg/storage/driver/mock_test.go @@ -142,12 +142,19 @@ func (mock *MockConfigMapsInterface) List(_ context.Context, opts metav1.ListOpt } if opts.Continue != "" { - i, _ := strconv.ParseInt(opts.Continue, 10, 64) + i, err := strconv.ParseInt(opts.Continue, 10, 64) + if err != nil { + return nil, fmt.Errorf("invalid continue value %q: %w", opts.Continue, err) + } objects = objects[int(i):] + list.Continue = "" } if opts.Limit > 0 && opts.Limit <= int64(len(objects)) { objects = objects[:int(opts.Limit)] + if opts.Limit < int64(len(objects)) { + list.Continue = strconv.FormatInt(opts.Limit, 10) + } } for _, cfgmap := range objects { @@ -244,12 +251,19 @@ func (mock *MockSecretsInterface) List(_ context.Context, opts metav1.ListOption } if opts.Continue != "" { - i, _ := strconv.ParseInt(opts.Continue, 10, 64) + i, err := strconv.ParseInt(opts.Continue, 10, 64) + if err != nil { + return nil, fmt.Errorf("invalid continue value: %v", err) + } objects = objects[int(i):] + list.Continue = "" } if opts.Limit > 0 && opts.Limit <= int64(len(objects)) { objects = objects[:int(opts.Limit)] + if opts.Limit < int64(len(objects)) { + list.Continue = strconv.FormatInt(opts.Limit, 10) + } } for _, secret := range objects { diff --git a/pkg/storage/driver/secrets.go b/pkg/storage/driver/secrets.go index b0f8cc851..e8af475a4 100644 --- a/pkg/storage/driver/secrets.go +++ b/pkg/storage/driver/secrets.go @@ -85,7 +85,7 @@ func (secrets *Secrets) Get(key string) (release.Releaser, error) { } // listPages is a common method to list release with pagination -func (secrets *Secrets) listPages(f func(page []release.Releaser, lastPage bool) (end bool), opts metav1.ListOptions, filter func(release.Releaser) bool) (err error) { +func (secrets *Secrets) listPages(f func(page []release.Releaser, remaining bool) (end bool), opts metav1.ListOptions, filter func(release.Releaser) bool) (err error) { if opts.Limit == 0 { opts.Limit = DefaultPaginationLimit } @@ -157,7 +157,7 @@ func (secrets *Secrets) List(filter func(release.Releaser) bool) ([]release.Rele } // ListPages same as List, but with pagination -func (secrets *Secrets) ListPages(f func(page []release.Releaser, lastPage bool) (end bool), limit int64, filter func(release.Releaser) bool) error { +func (secrets *Secrets) ListPages(f func(page []release.Releaser, remaining bool) (end bool), limit int64, filter func(release.Releaser) bool) error { lsel := kblabels.Set{"owner": owner}.AsSelector() opts := metav1.ListOptions{Limit: limit, LabelSelector: lsel.String()} @@ -200,7 +200,7 @@ func (secrets *Secrets) Query(labels map[string]string) ([]release.Releaser, err } // QueryPages same as Query, but with pagination -func (secrets *Secrets) QueryPages(f func(page []release.Releaser, lastPage bool) (end bool), limit int64, labels map[string]string) error { +func (secrets *Secrets) QueryPages(f func(page []release.Releaser, remaining bool) (end bool), limit int64, labels map[string]string) error { ls := kblabels.Set{} for k, v := range labels { if errs := validation.IsValidLabelValue(v); len(errs) != 0 { diff --git a/pkg/storage/driver/secrets_test.go b/pkg/storage/driver/secrets_test.go index 24e989654..117186086 100644 --- a/pkg/storage/driver/secrets_test.go +++ b/pkg/storage/driver/secrets_test.go @@ -97,13 +97,13 @@ func TestSecretList(t *testing.T) { }...) // list releases with pagination - err := secrets.ListPages(func(page []release.Releaser, lastPage bool) (end bool) { + err := secrets.ListPages(func(page []release.Releaser, remaining bool) (end bool) { // check if len(page) != 2 { t.Errorf("Expected 2 secrets, got %d:\n%v\n", len(page), page) } - return !lastPage + return !remaining }, 2, func(rel release.Releaser) bool { return true }) // check if err != nil { @@ -184,12 +184,12 @@ func TestSecretQuery(t *testing.T) { } // query releases with pagination - err = secrets.QueryPages(func(page []release.Releaser, lastPage bool) (err bool) { + err = secrets.QueryPages(func(page []release.Releaser, remaining bool) (err bool) { if len(page) != 2 { t.Fatalf("Expected 2 results, actual %d", len(page)) } - return !lastPage + return !remaining }, 2, map[string]string{"status": "deployed"}) if err != nil { t.Fatalf("Failed to query: %s", err) diff --git a/pkg/storage/driver/sql.go b/pkg/storage/driver/sql.go index bd099fb0c..9f4d7c53e 100644 --- a/pkg/storage/driver/sql.go +++ b/pkg/storage/driver/sql.go @@ -384,7 +384,7 @@ func (s *SQL) List(filter func(release.Releaser) bool) ([]release.Releaser, erro } // ListPages is a common method to list release with pagination -func (s *SQL) ListPages(f func(page []release.Releaser, lastPage bool) (end bool), limit int64, filter func(release.Releaser) bool) (err error) { +func (s *SQL) ListPages(f func(page []release.Releaser, remaining bool) (end bool), limit int64, filter func(release.Releaser) bool) (err error) { if limit == 0 { limit = DefaultPaginationLimit } @@ -399,8 +399,10 @@ func (s *SQL) ListPages(f func(page []release.Releaser, lastPage bool) (end bool sb = sb.Where(sq.Eq{sqlReleaseTableNamespaceColumn: s.namespace}) } + var page, offset uint64 for { - query, args, err := sb.Limit(uint64(limit)).ToSql() + offset = (page - 1) * uint64(limit) + query, args, err := sb.Offset(offset).Limit(uint64(limit)).ToSql() if err != nil { s.Logger().Debug("failed to build query", slog.Any("error", err)) return err @@ -434,6 +436,8 @@ func (s *SQL) ListPages(f func(page []release.Releaser, lastPage bool) (end bool if f(releases, int64(len(records)) < limit) { break } + + page++ } return nil @@ -505,7 +509,7 @@ func (s *SQL) Query(labels map[string]string) ([]release.Releaser, error) { } // QueryPages same as Query, but with pagination -func (s *SQL) QueryPages(f func(page []release.Releaser, lastPage bool) (end bool), limit int64, labels map[string]string) error { +func (s *SQL) QueryPages(f func(page []release.Releaser, remaining bool) (end bool), limit int64, labels map[string]string) error { sb := s.statementBuilder. Select(sqlReleaseTableKeyColumn, sqlReleaseTableNamespaceColumn, sqlReleaseTableBodyColumn). From(sqlReleaseTableName) @@ -529,9 +533,11 @@ func (s *SQL) QueryPages(f func(page []release.Releaser, lastPage bool) (end boo sb = sb.Where(sq.Eq{sqlReleaseTableNamespaceColumn: s.namespace}) } + var page, offset uint64 for { // Build our query - query, args, err := sb.Limit(uint64(limit)).ToSql() + offset = (page - 1) * uint64(limit) + query, args, err := sb.Offset(offset).Limit(uint64(limit)).ToSql() if err != nil { s.Logger().Debug("failed to build query", slog.Any("error", err)) return err @@ -543,10 +549,6 @@ func (s *SQL) QueryPages(f func(page []release.Releaser, lastPage bool) (end boo return err } - if len(records) == 0 { - return ErrReleaseNotFound - } - var releases []release.Releaser for _, record := range records { release, err := decodeRelease(record.Body) @@ -566,6 +568,8 @@ func (s *SQL) QueryPages(f func(page []release.Releaser, lastPage bool) (end boo if f(releases, int64(len(records)) < limit) { break } + + page++ } return nil diff --git a/pkg/storage/storage_test.go b/pkg/storage/storage_test.go index 63bd2431f..452574922 100644 --- a/pkg/storage/storage_test.go +++ b/pkg/storage/storage_test.go @@ -314,13 +314,13 @@ func (d *MaxHistoryMockDriver) Get(key string) (release.Releaser, error) { func (d *MaxHistoryMockDriver) List(filter func(release.Releaser) bool) ([]release.Releaser, error) { return d.Driver.List(filter) } -func (d *MaxHistoryMockDriver) ListPages(f func(page []release.Releaser, lastPage bool) (end bool), limit int64, filter func(release.Releaser) bool) error { +func (d *MaxHistoryMockDriver) ListPages(f func(page []release.Releaser, remaining bool) (end bool), limit int64, filter func(release.Releaser) bool) error { return d.Driver.ListPages(f, limit, filter) } func (d *MaxHistoryMockDriver) Query(labels map[string]string) ([]release.Releaser, error) { return d.Driver.Query(labels) } -func (d *MaxHistoryMockDriver) QueryPages(f func(page []release.Releaser, lastPage bool) (end bool), limit int64, labels map[string]string) error { +func (d *MaxHistoryMockDriver) QueryPages(f func(page []release.Releaser, remaining bool) (end bool), limit int64, labels map[string]string) error { return d.Driver.QueryPages(f, limit, labels) } func (d *MaxHistoryMockDriver) Name() string {