Skip to content

ROX-28697: remove GetAll generator #14947

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
janisz merged 2 commits into from
Apr 10, 2025
Merged

ROX-28697: remove GetAll generator #14947

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6719e19
ROX-28697: remove GetAll generator
janisz Apr 10, 2025
047b9ce
fix
janisz Apr 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion central/authprovider/datastore/internal/store/postgres/gen.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
package postgres

//go:generate pg-table-bindings-wrapper --type=storage.AuthProvider --search-category AUTH_PROVIDERS --get-all-func
//go:generate pg-table-bindings-wrapper --type=storage.AuthProvider --search-category AUTH_PROVIDERS
1 change: 0 additions & 1 deletion central/authprovider/datastore/internal/store/postgres/store.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions central/authprovider/datastore/internal/store/postgres/store_test.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions migrator/migrations/postgreshelper/schema/gen.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package schema

//go:generate pg-schema-migration-helper --type=storage.TestSingleKeyStruct --search-category SEARCH_UNSET --get-all-func
//go:generate pg-schema-migration-helper --type=storage.TestSingleUUIDKeyStruct --search-category SEARCH_UNSET --get-all-func
//go:generate pg-schema-migration-helper --type=storage.TestSingleKeyStruct --search-category SEARCH_UNSET
//go:generate pg-schema-migration-helper --type=storage.TestSingleUUIDKeyStruct --search-category SEARCH_UNSET
//go:generate pg-schema-migration-helper --type=storage.TestStruct --search-category SEARCH_UNSET
//go:generate pg-schema-migration-helper --type=storage.TestParent4 --search-category 72 --references storage.TestGrandparent --search-scope 61,74
//go:generate pg-schema-migration-helper --type=storage.TestParent3 --search-category 69 --references storage.TestGrandparent
Expand Down
15 changes: 0 additions & 15 deletions pkg/search/postgres/store.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ type Store[T any, PT pgutils.Unmarshaler[T]] interface {
Search(ctx context.Context, q *v1.Query) ([]search.Result, error)
Walk(ctx context.Context, fn func(obj PT) error) error
WalkByQuery(ctx context.Context, q *v1.Query, fn func(obj PT) error) error
GetAll(ctx context.Context) ([]PT, error)
Get(ctx context.Context, id string) (PT, bool, error)
GetByQuery(ctx context.Context, query *v1.Query) ([]*T, error)
GetIDs(ctx context.Context) ([]string, error)
Expand Down Expand Up @@ -205,20 +204,6 @@ func (s *genericStore[T, PT]) WalkByQuery(ctx context.Context, query *v1.Query,
return s.walkByQuery(ctx, query, fn)
}

// GetAll retrieves all objects from the store.
//
// Deprecated: This can be dangerous on high cardinality stores consider Walk instead.
func (s *genericStore[T, PT]) GetAll(ctx context.Context) ([]PT, error) {
defer s.setPostgresOperationDurationTime(time.Now(), ops.GetAll)

var objs []PT
err := s.Walk(ctx, func(obj PT) error {
objs = append(objs, obj)
return nil
})
return objs, err
}

// Get returns the object, if it exists from the store.
func (s *genericStore[T, PT]) Get(ctx context.Context, id string) (PT, bool, error) {
defer s.setPostgresOperationDurationTime(time.Now(), ops.Get)
Expand Down
18 changes: 0 additions & 18 deletions pkg/search/postgres/store_cache.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -408,24 +408,6 @@ func (c *cachedStore[T, PT]) GetIDsByQuery(ctx context.Context, query *v1.Query)
return c.underlyingStore.GetIDsByQuery(ctx, query)
}

// GetAll retrieves all objects from the store.
//
// Deprecated: This can be dangerous on high cardinality stores consider Walk instead.
func (c *cachedStore[T, PT]) GetAll(ctx context.Context) ([]PT, error) {
defer c.setCacheOperationDurationTime(time.Now(), ops.GetAll)
c.cacheLock.RLock()
defer c.cacheLock.RUnlock()
result := make([]PT, 0, len(c.cache))
err := c.walkCacheNoLock(ctx, func(obj PT) error {
result = append(result, obj.CloneVT())
return nil
})
if err != nil {
return nil, err
}
return result, nil
}

func (c *cachedStore[T, PT]) walkCacheNoLock(ctx context.Context, fn func(obj PT) error) error {
for _, obj := range c.cache {
if err := ctx.Err(); err != nil {
Expand Down
13 changes: 0 additions & 13 deletions pkg/search/postgres/store_cache_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,19 +322,6 @@ func TestCachedWalkByQueryContextCancelation(t *testing.T) {
assert.ErrorIs(t, err, context.Canceled)
}

func TestCachedGetAll(t *testing.T) {
testDB := pgtest.ForT(t)
store := newCachedStore(testDB)
require.NotNil(t, store)

testObjects := sampleCachedTestSingleKeyStructArray("GetAll")
assert.NoError(t, store.UpsertMany(cachedStoreCtx, testObjects))

fetchedObjects, err := store.GetAll(cachedStoreCtx)
assert.NoError(t, err)
protoassert.ElementsMatch(t, fetchedObjects, testObjects)
}

func TestCachedGetIDs(t *testing.T) {
testDB := pgtest.ForT(t)
store := newCachedStore(testDB)
Expand Down
13 changes: 0 additions & 13 deletions pkg/search/postgres/store_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -265,19 +265,6 @@ func TestWalkByQuery(t *testing.T) {
protoassert.ElementsMatch(t, expectedObjects, walkedObjects)
}

func TestGetAll(t *testing.T) {
testDB := pgtest.ForT(t)
store := newStore(testDB)
require.NotNil(t, store)

testObjects := sampleTestSingleKeyStructArray("GetAll")
assert.NoError(t, store.UpsertMany(ctx, testObjects))

fetchedObjects, err := store.GetAll(ctx)
assert.NoError(t, err)
protoassert.ElementsMatch(t, fetchedObjects, testObjects)
}

func TestGetIDs(t *testing.T) {
testDB := pgtest.ForT(t)
store := newStore(testDB)
Expand Down
5 changes: 0 additions & 5 deletions tools/generate-helpers/pg-table-bindings/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,6 @@ type properties struct {
// Indicates the directory in which the generated schema file must go.
SchemaDirectory string

// Indicates that we want to generate a GetAll function. Defaults to false because this can be dangerous on high cardinality stores.
GetAll bool

// Indicates that we should just generate the singleton store.
SingletonStore bool

Expand Down Expand Up @@ -135,7 +132,6 @@ func main() {
c.Flags().BoolVar(&props.JoinTable, "read-only-store", false, "if set to true, creates read-only store")
c.Flags().BoolVar(&props.NoCopyFrom, "no-copy-from", false, "if true, indicates that the store should not use Postgres copyFrom operation")
c.Flags().BoolVar(&props.SchemaOnly, "schema-only", false, "if true, generates only the schema and not store and index")
c.Flags().BoolVar(&props.GetAll, "get-all-func", false, "if true, generates a GetAll function (can be dangerous on high cardinality stores, use with care)")
c.Flags().StringVar(&props.SchemaDirectory, "schema-directory", "", "the directory in which to generate the schema")
c.Flags().BoolVar(&props.SingletonStore, "singleton", false, "indicates that we should just generate the singleton store")
c.Flags().StringSliceVar(&props.SearchScope, "search-scope", []string{}, "if set, the search is scoped to specified search categories. comma seperated of search categories")
Expand Down Expand Up @@ -202,7 +198,6 @@ func main() {
"SearchCategory": searchCategory,
"JoinTable": props.JoinTable,
"PermissionChecker": props.PermissionChecker,
"GetAll": props.GetAll,
"Obj": object{
storageType: props.Type,
permissionCheckerEnabled: permissionCheckerEnabled,
Expand Down
3 changes: 0 additions & 3 deletions tools/generate-helpers/pg-table-bindings/store.go.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,6 @@ type Store interface {
{{- end }}
GetMany(ctx context.Context, identifiers []{{$primaryKeyType}}) ([]*storeType, []int, error)
GetIDs(ctx context.Context) ([]{{$primaryKeyType}}, error)
{{- if .GetAll }}
GetAll(ctx context.Context) ([]*storeType, error)
{{- end }}

Walk(ctx context.Context, fn func(obj *storeType) error) error
WalkByQuery(ctx context.Context, query *v1.Query, fn func(obj *storeType) error) error
Expand Down
6 changes: 0 additions & 6 deletions tools/generate-helpers/pg-table-bindings/store_test.go.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,12 +128,6 @@ func (s *{{$namePrefix}}StoreSuite) TestStore() {

s.NoError(store.UpsertMany(ctx, {{.TrimmedType|lowerCamelCase}}s))

{{- if .GetAll }}
all{{.TrimmedType|upperCamelCase}}, err := store.GetAll(ctx)
s.NoError(err)
protoassert.ElementsMatch(s.T(), {{$name}}s, all{{.TrimmedType|upperCamelCase}})
{{- end }}

{{.TrimmedType|lowerCamelCase}}Count, err = store.Count(ctx, search.EmptyQuery())
s.NoError(err)
s.Equal(200, {{.TrimmedType|lowerCamelCase}}Count)
Expand Down
2 changes: 1 addition & 1 deletion tools/generate-helpers/pg-table-bindings/test/postgres/gen.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
package postgres

//go:generate pg-table-bindings-wrapper --type=storage.TestSingleKeyStruct --search-category 100 --get-all-func
//go:generate pg-table-bindings-wrapper --type=storage.TestSingleKeyStruct --search-category 100
1 change: 0 additions & 1 deletion tools/generate-helpers/pg-table-bindings/test/postgres/store.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions tools/generate-helpers/pg-table-bindings/test/postgres/store_test.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion tools/generate-helpers/pg-table-bindings/testuuidkey/postgres/gen.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
package postgres

//go:generate pg-table-bindings-wrapper --type=storage.TestSingleUUIDKeyStruct --search-category 115 --get-all-func
//go:generate pg-table-bindings-wrapper --type=storage.TestSingleUUIDKeyStruct --search-category 115
1 change: 0 additions & 1 deletion tools/generate-helpers/pg-table-bindings/testuuidkey/postgres/store.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions tools/generate-helpers/pg-table-bindings/testuuidkey/postgres/store_test.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading