From 21784a62bac0e0b7ba61ef7c5d587b8ac74b257e Mon Sep 17 00:00:00 2001 From: khanhtc1202 Date: Fri, 11 Sep 2026 16:19:50 +0900 Subject: [PATCH 1/6] Filter out soft-deleted applications from list application result Signed-off-by: khanhtc1202 --- pkg/datastore/applicationstore.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/datastore/applicationstore.go b/pkg/datastore/applicationstore.go index 8d47c6fbb1..fe85de296e 100644 --- a/pkg/datastore/applicationstore.go +++ b/pkg/datastore/applicationstore.go @@ -104,6 +104,10 @@ func (s *applicationStore) List(ctx context.Context, opts ListOptions) ([]*model if err != nil { return nil, "", err } + // Soft-deleted applications should never be returned from list queries. + if app.Deleted { + continue + } apps = append(apps, &app) } From 61cfccb8212fa0b1e56178282f4c1697b96c160b Mon Sep 17 00:00:00 2001 From: khanhtc1202 Date: Fri, 11 Sep 2026 16:34:28 +0900 Subject: [PATCH 2/6] Add test Signed-off-by: khanhtc1202 --- pkg/datastore/applicationstore_test.go | 61 +++++++++++++++++++++++--- 1 file changed, 54 insertions(+), 7 deletions(-) diff --git a/pkg/datastore/applicationstore_test.go b/pkg/datastore/applicationstore_test.go index 998797a4cc..6fac154faf 100644 --- a/pkg/datastore/applicationstore_test.go +++ b/pkg/datastore/applicationstore_test.go @@ -126,10 +126,11 @@ func TestListApplications(t *testing.T) { defer ctrl.Finish() testcases := []struct { - name string - opts ListOptions - ds DataStore - wantErr bool + name string + opts ListOptions + ds DataStore + wantErr bool + wantAppIDs []string }{ { name: "iterator done", @@ -146,7 +147,8 @@ func TestListApplications(t *testing.T) { Return(it, nil) return ds }(), - wantErr: false, + wantErr: false, + wantAppIDs: nil, }, { name: "unexpected error occurred", @@ -163,15 +165,60 @@ func TestListApplications(t *testing.T) { Return(it, nil) return ds }(), - wantErr: true, + wantErr: true, + wantAppIDs: nil, + }, + { + name: "deleted applications are filtered out", + opts: ListOptions{}, + ds: func() DataStore { + apps := []*model.Application{ + {Id: "app-1", Name: "active-app", Deleted: false}, + {Id: "app-2", Name: "deleted-app", Deleted: true}, + {Id: "app-3", Name: "another-active-app", Deleted: false}, + } + callCount := 0 + + it := NewMockIterator(ctrl) + it.EXPECT(). + Next(gomock.Any()). + DoAndReturn(func(dst interface{}) error { + if callCount >= len(apps) { + return ErrIteratorDone + } + app := dst.(*model.Application) + *app = *apps[callCount] + callCount++ + return nil + }). + Times(len(apps) + 1) + it.EXPECT(). + Cursor(). + Return("cursor", nil) + + ds := NewMockDataStore(ctrl) + ds.EXPECT(). + Find(gomock.Any(), gomock.Any(), ListOptions{}). + Return(it, nil) + return ds + }(), + wantErr: false, + wantAppIDs: []string{"app-1", "app-3"}, }, } for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { s := NewApplicationStore(tc.ds) - _, _, err := s.List(context.Background(), tc.opts) + apps, _, err := s.List(context.Background(), tc.opts) assert.Equal(t, tc.wantErr, err != nil) + if tc.wantAppIDs != nil { + gotIDs := make([]string, len(apps)) + for i, app := range apps { + gotIDs[i] = app.Id + } + assert.ElementsMatch(t, tc.wantAppIDs, gotIDs) + } }) } } From 48d4f98373abbf8e5312fa81d5fdffdbe9c3f16b Mon Sep 17 00:00:00 2001 From: khanhtc1202 Date: Fri, 11 Sep 2026 16:55:12 +0900 Subject: [PATCH 3/6] Move filter logic to handler layer Signed-off-by: khanhtc1202 --- pkg/app/server/grpcapi/api.go | 35 ++++++++------- pkg/app/server/grpcapi/web_api.go | 20 ++++----- pkg/datastore/applicationstore.go | 4 -- pkg/datastore/applicationstore_test.go | 61 +++----------------------- 4 files changed, 37 insertions(+), 83 deletions(-) diff --git a/pkg/app/server/grpcapi/api.go b/pkg/app/server/grpcapi/api.go index e590c3c2de..1224535e19 100644 --- a/pkg/app/server/grpcapi/api.go +++ b/pkg/app/server/grpcapi/api.go @@ -305,22 +305,27 @@ func (a *API) ListApplications(ctx context.Context, req *apiservice.ListApplicat return nil, gRPCStoreError(err, "failed to list applications") } + // Filter applications based on labels and deleted status. + // NOTE: Filtering is done application-side to avoid requiring new composite indexes. labels := req.Labels - if len(req.Labels) == 0 { - return &apiservice.ListApplicationsResponse{ - Applications: apps, - Cursor: cursor, - }, nil + filterApp := func(app *model.Application) bool { + if app.Deleted { + return false + } + if len(labels) > 0 && !app.ContainLabels(labels) { + return false + } + return true } - // NOTE: Filtering by labels is done by the application-side because we need to create composite indexes for every combination in the filter. filtered := make([]*model.Application, 0, len(apps)) - for _, a := range apps { - if a.ContainLabels(req.Labels) { - filtered = append(filtered, a) + for _, app := range apps { + if filterApp(app) { + filtered = append(filtered, app) } } - // Stop running additional queries for more data, and return filtered deployments immediately with + + // Stop running additional queries for more data, and return filtered applications immediately with // current cursor if the size before filtering is already less than the page size. if len(apps) < limit { return &apiservice.ListApplicationsResponse{ @@ -328,8 +333,8 @@ func (a *API) ListApplications(ctx context.Context, req *apiservice.ListApplicat Cursor: cursor, }, nil } - // Repeat the query until the number of filtered deployments reaches the page size, - // or until it finishes scanning to page_min_updated_at. + // Repeat the query until the number of filtered applications reaches the page size, + // or until it finishes scanning all pages. for len(filtered) < limit { options.Cursor = cursor apps, cursor, err = a.applicationStore.List(ctx, options) @@ -340,9 +345,9 @@ func (a *API) ListApplications(ctx context.Context, req *apiservice.ListApplicat if len(apps) == 0 { break } - for _, d := range apps { - if d.ContainLabels(labels) { - filtered = append(filtered, d) + for _, app := range apps { + if filterApp(app) { + filtered = append(filtered, app) } } } diff --git a/pkg/app/server/grpcapi/web_api.go b/pkg/app/server/grpcapi/web_api.go index 4fdbedbe71..b2227614af 100644 --- a/pkg/app/server/grpcapi/web_api.go +++ b/pkg/app/server/grpcapi/web_api.go @@ -685,18 +685,18 @@ func (a *WebAPI) ListApplications(ctx context.Context, req *webservice.ListAppli return nil, gRPCStoreError(err, "list applications") } - if len(req.Options.Labels) == 0 { - return &webservice.ListApplicationsResponse{ - Applications: apps, - }, nil - } - - // NOTE: Filtering by labels is done by the application-side because we need to create composite indexes for every combination in the filter. + // Filter applications based on labels and deleted status. + // NOTE: Filtering is done application-side to avoid requiring new composite indexes. + labels := req.Options.Labels filtered := make([]*model.Application, 0, len(apps)) - for _, a := range apps { - if a.ContainLabels(req.Options.Labels) { - filtered = append(filtered, a) + for _, app := range apps { + if app.Deleted { + continue + } + if len(labels) > 0 && !app.ContainLabels(labels) { + continue } + filtered = append(filtered, app) } return &webservice.ListApplicationsResponse{ Applications: filtered, diff --git a/pkg/datastore/applicationstore.go b/pkg/datastore/applicationstore.go index fe85de296e..8d47c6fbb1 100644 --- a/pkg/datastore/applicationstore.go +++ b/pkg/datastore/applicationstore.go @@ -104,10 +104,6 @@ func (s *applicationStore) List(ctx context.Context, opts ListOptions) ([]*model if err != nil { return nil, "", err } - // Soft-deleted applications should never be returned from list queries. - if app.Deleted { - continue - } apps = append(apps, &app) } diff --git a/pkg/datastore/applicationstore_test.go b/pkg/datastore/applicationstore_test.go index 6fac154faf..998797a4cc 100644 --- a/pkg/datastore/applicationstore_test.go +++ b/pkg/datastore/applicationstore_test.go @@ -126,11 +126,10 @@ func TestListApplications(t *testing.T) { defer ctrl.Finish() testcases := []struct { - name string - opts ListOptions - ds DataStore - wantErr bool - wantAppIDs []string + name string + opts ListOptions + ds DataStore + wantErr bool }{ { name: "iterator done", @@ -147,8 +146,7 @@ func TestListApplications(t *testing.T) { Return(it, nil) return ds }(), - wantErr: false, - wantAppIDs: nil, + wantErr: false, }, { name: "unexpected error occurred", @@ -165,60 +163,15 @@ func TestListApplications(t *testing.T) { Return(it, nil) return ds }(), - wantErr: true, - wantAppIDs: nil, - }, - { - name: "deleted applications are filtered out", - opts: ListOptions{}, - ds: func() DataStore { - apps := []*model.Application{ - {Id: "app-1", Name: "active-app", Deleted: false}, - {Id: "app-2", Name: "deleted-app", Deleted: true}, - {Id: "app-3", Name: "another-active-app", Deleted: false}, - } - callCount := 0 - - it := NewMockIterator(ctrl) - it.EXPECT(). - Next(gomock.Any()). - DoAndReturn(func(dst interface{}) error { - if callCount >= len(apps) { - return ErrIteratorDone - } - app := dst.(*model.Application) - *app = *apps[callCount] - callCount++ - return nil - }). - Times(len(apps) + 1) - it.EXPECT(). - Cursor(). - Return("cursor", nil) - - ds := NewMockDataStore(ctrl) - ds.EXPECT(). - Find(gomock.Any(), gomock.Any(), ListOptions{}). - Return(it, nil) - return ds - }(), - wantErr: false, - wantAppIDs: []string{"app-1", "app-3"}, + wantErr: true, }, } for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { s := NewApplicationStore(tc.ds) - apps, _, err := s.List(context.Background(), tc.opts) + _, _, err := s.List(context.Background(), tc.opts) assert.Equal(t, tc.wantErr, err != nil) - if tc.wantAppIDs != nil { - gotIDs := make([]string, len(apps)) - for i, app := range apps { - gotIDs[i] = app.Id - } - assert.ElementsMatch(t, tc.wantAppIDs, gotIDs) - } }) } } From 63c7a0fb6035d5b238e0ca6bc14694f5a3b95afe Mon Sep 17 00:00:00 2001 From: khanhtc1202 Date: Fri, 11 Sep 2026 18:45:53 +0900 Subject: [PATCH 4/6] Add test Signed-off-by: khanhtc1202 --- pkg/app/server/grpcapi/api_test.go | 86 ++++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/pkg/app/server/grpcapi/api_test.go b/pkg/app/server/grpcapi/api_test.go index e45016e851..1bd2029375 100644 --- a/pkg/app/server/grpcapi/api_test.go +++ b/pkg/app/server/grpcapi/api_test.go @@ -166,3 +166,89 @@ func TestListApplicationsCursor(t *testing.T) { }) } } + +func TestListApplicationsFilterDeleted(t *testing.T) { + testcases := []struct { + name string + req *apiservice.ListApplicationsRequest + pages [][]*model.Application + expectedAppIDs []string + }{ + { + name: "deleted applications are filtered out", + req: &apiservice.ListApplicationsRequest{Limit: 10}, + pages: [][]*model.Application{ + { + {Id: "app-1", ProjectId: "project-id", Deleted: false}, + {Id: "app-2", ProjectId: "project-id", Deleted: true}, + {Id: "app-3", ProjectId: "project-id", Deleted: false}, + }, + }, + expectedAppIDs: []string{"app-1", "app-3"}, + }, + { + name: "pagination continues when page contains only deleted apps", + req: &apiservice.ListApplicationsRequest{Limit: 2}, + pages: [][]*model.Application{ + // First page: all deleted + { + {Id: "app-1", ProjectId: "project-id", Deleted: true}, + {Id: "app-2", ProjectId: "project-id", Deleted: true}, + }, + // Second page: mix of deleted and non-deleted + { + {Id: "app-3", ProjectId: "project-id", Deleted: false}, + {Id: "app-4", ProjectId: "project-id", Deleted: true}, + }, + // Third page: non-deleted + { + {Id: "app-5", ProjectId: "project-id", Deleted: false}, + }, + }, + expectedAppIDs: []string{"app-3", "app-5"}, + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + pageIndex := 0 + store := datastoretest.NewMockApplicationStore(ctrl) + store.EXPECT(). + List(gomock.Any(), gomock.Any()). + DoAndReturn(func(_ context.Context, opts datastore.ListOptions) ([]*model.Application, string, error) { + if pageIndex >= len(tc.pages) { + return nil, "", nil + } + apps := tc.pages[pageIndex] + pageIndex++ + cursor := "" + if pageIndex < len(tc.pages) { + cursor = "next-cursor" + } + return apps, cursor, nil + }). + AnyTimes() + + api := &API{ + applicationStore: store, + logger: zap.NewNop(), + } + ctx := rpcauth.ContextWithAPIKey(context.TODO(), &model.APIKey{ + ProjectId: "project-id", + Role: model.APIKey_READ_ONLY, + }) + + resp, err := api.ListApplications(ctx, tc.req) + assert.NoError(t, err) + + gotIDs := make([]string, len(resp.Applications)) + for i, app := range resp.Applications { + gotIDs[i] = app.Id + } + assert.ElementsMatch(t, tc.expectedAppIDs, gotIDs) + }) + } +} From 0ebeb2af396e23dc310f0c1c0d147f1b9580ace1 Mon Sep 17 00:00:00 2001 From: khanhtc1202 Date: Mon, 14 Sep 2026 18:38:36 +0900 Subject: [PATCH 5/6] Update logic to ensure only return number of applications under limit Signed-off-by: khanhtc1202 --- pkg/app/server/grpcapi/api.go | 2 ++ pkg/app/server/grpcapi/api_test.go | 23 +++++++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/pkg/app/server/grpcapi/api.go b/pkg/app/server/grpcapi/api.go index 1224535e19..43eb14dd26 100644 --- a/pkg/app/server/grpcapi/api.go +++ b/pkg/app/server/grpcapi/api.go @@ -337,6 +337,8 @@ func (a *API) ListApplications(ctx context.Context, req *apiservice.ListApplicat // or until it finishes scanning all pages. for len(filtered) < limit { options.Cursor = cursor + // Fetch only the remaining number of applications to keep the response within the requested limit. + options.Limit = limit - len(filtered) apps, cursor, err = a.applicationStore.List(ctx, options) if err != nil { a.logger.Error("failed to get applications", zap.Error(err)) diff --git a/pkg/app/server/grpcapi/api_test.go b/pkg/app/server/grpcapi/api_test.go index 1bd2029375..31288f02dc 100644 --- a/pkg/app/server/grpcapi/api_test.go +++ b/pkg/app/server/grpcapi/api_test.go @@ -207,6 +207,25 @@ func TestListApplicationsFilterDeleted(t *testing.T) { }, expectedAppIDs: []string{"app-3", "app-5"}, }, + { + name: "the number of returned applications does not exceed the limit", + req: &apiservice.ListApplicationsRequest{Limit: 3}, + pages: [][]*model.Application{ + // First page: one of them is deleted, so an additional query is needed. + { + {Id: "app-1", ProjectId: "project-id", Deleted: false}, + {Id: "app-2", ProjectId: "project-id", Deleted: true}, + {Id: "app-3", ProjectId: "project-id", Deleted: false}, + }, + // Second page: only the remaining one should be taken. + { + {Id: "app-4", ProjectId: "project-id", Deleted: false}, + {Id: "app-5", ProjectId: "project-id", Deleted: false}, + {Id: "app-6", ProjectId: "project-id", Deleted: false}, + }, + }, + expectedAppIDs: []string{"app-1", "app-3", "app-4"}, + }, } for _, tc := range testcases { @@ -224,6 +243,10 @@ func TestListApplicationsFilterDeleted(t *testing.T) { } apps := tc.pages[pageIndex] pageIndex++ + // Mimic the datastore behavior of returning at most the requested number of rows. + if opts.Limit > 0 && len(apps) > opts.Limit { + apps = apps[:opts.Limit] + } cursor := "" if pageIndex < len(tc.pages) { cursor = "next-cursor" From 89828904b94327ab3ff0fb2b1b5a031079ebecfc Mon Sep 17 00:00:00 2001 From: khanhtc1202 Date: Mon, 14 Sep 2026 18:54:28 +0900 Subject: [PATCH 6/6] Fix potential nil derefer Signed-off-by: khanhtc1202 --- pkg/app/server/grpcapi/web_api.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pkg/app/server/grpcapi/web_api.go b/pkg/app/server/grpcapi/web_api.go index b2227614af..0ac75e7c4b 100644 --- a/pkg/app/server/grpcapi/web_api.go +++ b/pkg/app/server/grpcapi/web_api.go @@ -687,7 +687,10 @@ func (a *WebAPI) ListApplications(ctx context.Context, req *webservice.ListAppli // Filter applications based on labels and deleted status. // NOTE: Filtering is done application-side to avoid requiring new composite indexes. - labels := req.Options.Labels + var labels map[string]string + if o := req.Options; o != nil { + labels = o.Labels + } filtered := make([]*model.Application, 0, len(apps)) for _, app := range apps { if app.Deleted {