rancher/steve
1
0
Fork 0
mirror of https://github.com/rancher/steve.git synced 2025-04-27 19:05:09 +00:00
Code Issues Projects Releases Wiki Activity

Sort-indirect PR broken into smaller parts: part 3/6 - pass listOptions by reference (#612)

Browse Source 
* Move types related to list options and sql queries into their own package.

The problem having these in the informer package is that eventually code
in other packages will need to import `informer` only for constants or types,
but some members of the informer package may already depend on those. Best to
move type definitions into their own simpler package.

* Fix the ListOptions sort field.

Instead of making it a single array-ish field, convert it into a
true array of Sort Directives.  Easier to read, less bending backwards.

* Pass the listOptions struct by reference to avoid copying.

We never update the ListOptions struct once it's created so there's
no need to pass it by value.

This might be a near-useless optimization...
This commit is contained in:
Eric Promislow 2025-04-25 11:19:34 -07:00 committed by GitHub
parent 89268ba86b
commit 57ce685118
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
13 changed files with 28 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
pkg/sqlcache/informer/informer.go
View File

@ -30,7 +30,7 @@ type Informer struct {
}
type ByOptionsLister interface {
ListByOptions(ctx context.Context, lo sqltypes.ListOptions, partitions []partition.Partition, namespace string) (*unstructured.UnstructuredList, int, string, error)
ListByOptions(ctx context.Context, lo *sqltypes.ListOptions, partitions []partition.Partition, namespace string) (*unstructured.UnstructuredList, int, string, error)
}
// this is set to a var so that it can be overridden by test code for mocking purposes
@ -66,7 +66,7 @@ func NewInformer(ctx context.Context, client dynamic.ResourceInterface, fields [
// copy of the known state of all objects (in an Indexer).
// The resync period option here is passed from Informer to Reflector to periodically (re)-push all known
// objects to the DeltaFIFO. That causes the periodic (re-)firing all registered handlers.
// sqltypes.In this case we are not registering any handlers to this particular informer, so re-syncing is a no-op.
// In this case we are not registering any handlers to this particular informer, so re-syncing is a no-op.
// We therefore just disable it right away.
resyncPeriod := time.Duration(0)
@ -103,7 +103,7 @@ func NewInformer(ctx context.Context, client dynamic.ResourceInterface, fields [
// - the total number of resources (returned list might be a subset depending on pagination options in lo)
// - a continue token, if there are more pages after the returned one
// - an error instead of all of the above if anything went wrong
func (i *Informer) ListByOptions(ctx context.Context, lo sqltypes.ListOptions, partitions []partition.Partition, namespace string) (*unstructured.UnstructuredList, int, string, error) {
func (i *Informer) ListByOptions(ctx context.Context, lo *sqltypes.ListOptions, partitions []partition.Partition, namespace string) (*unstructured.UnstructuredList, int, string, error) {
return i.ByOptionsLister.ListByOptions(ctx, lo, partitions, namespace)
}

2
pkg/sqlcache/informer/informer_mocks_test.go
View File

@ -43,7 +43,7 @@ func (m *MockByOptionsLister) EXPECT() *MockByOptionsListerMockRecorder {
}
// ListByOptions mocks base method.
func (m *MockByOptionsLister) ListByOptions(arg0 context.Context, arg1 sqltypes.ListOptions, arg2 []partition.Partition, arg3 string) (*unstructured.UnstructuredList, int, string, error) {
func (m *MockByOptionsLister) ListByOptions(arg0 context.Context, arg1 *sqltypes.ListOptions, arg2 []partition.Partition, arg3 string) (*unstructured.UnstructuredList, int, string, error) {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "ListByOptions", arg0, arg1, arg2, arg3)
ret0, _ := ret[0].(*unstructured.UnstructuredList)

8
pkg/sqlcache/informer/informer_test.go
View File

@ -325,8 +325,8 @@ func TestInformerListByOptions(t *testing.T) {
}
expectedTotal := len(expectedList.Items)
expectedContinueToken := "123"
indexer.EXPECT().ListByOptions(context.Background(), lo, partitions, ns).Return(expectedList, expectedTotal, expectedContinueToken, nil)
list, total, continueToken, err := informer.ListByOptions(context.Background(), lo, partitions, ns)
indexer.EXPECT().ListByOptions(context.Background(), &lo, partitions, ns).Return(expectedList, expectedTotal, expectedContinueToken, nil)
list, total, continueToken, err := informer.ListByOptions(context.Background(), &lo, partitions, ns)
assert.Nil(t, err)
assert.Equal(t, expectedList, list)
assert.Equal(t, len(expectedList.Items), total)
@ -340,8 +340,8 @@ func TestInformerListByOptions(t *testing.T) {
lo := sqltypes.ListOptions{}
var partitions []partition.Partition
ns := "somens"
indexer.EXPECT().ListByOptions(context.Background(), lo, partitions, ns).Return(nil, 0, "", fmt.Errorf("error"))
_, _, _, err := informer.ListByOptions(context.Background(), lo, partitions, ns)
indexer.EXPECT().ListByOptions(context.Background(), &lo, partitions, ns).Return(nil, 0, "", fmt.Errorf("error"))
_, _, _, err := informer.ListByOptions(context.Background(), &lo, partitions, ns)
assert.NotNil(t, err)
}})
t.Parallel()

6
pkg/sqlcache/informer/listoption_indexer.go
View File

@ -248,7 +248,7 @@ func (l *ListOptionIndexer) deleteLabels(key string, tx transaction.Client) erro
// - the total number of resources (returned list might be a subset depending on pagination options in lo)
// - a continue token, if there are more pages after the returned one
// - an error instead of all of the above if anything went wrong
func (l *ListOptionIndexer) ListByOptions(ctx context.Context, lo sqltypes.ListOptions, partitions []partition.Partition, namespace string) (*unstructured.UnstructuredList, int, string, error) {
func (l *ListOptionIndexer) ListByOptions(ctx context.Context, lo *sqltypes.ListOptions, partitions []partition.Partition, namespace string) (*unstructured.UnstructuredList, int, string, error) {
queryInfo, err := l.constructQuery(lo, partitions, namespace, db.Sanitize(l.GetName()))
if err != nil {
return nil, 0, "", err
@ -267,8 +267,8 @@ type QueryInfo struct {
offset int
}
func (l *ListOptionIndexer) constructQuery(lo sqltypes.ListOptions, partitions []partition.Partition, namespace string, dbName string) (*QueryInfo, error) {
ensureSortLabelsAreSelected(&lo)
func (l *ListOptionIndexer) constructQuery(lo *sqltypes.ListOptions, partitions []partition.Partition, namespace string, dbName string) (*QueryInfo, error) {
ensureSortLabelsAreSelected(lo)
queryInfo := &QueryInfo{}
queryUsesLabels := hasLabelFilter(lo.Filters)
joinTableIndexByLabelName := make(map[string]int)

4
pkg/sqlcache/informer/listoption_indexer_test.go
View File

@ -978,7 +978,7 @@ func TestListByOptions(t *testing.T) {
if len(test.extraIndexedFields) > 0 {
lii.indexedFields = append(lii.indexedFields, test.extraIndexedFields...)
}
queryInfo, err := lii.constructQuery(test.listOptions, test.partitions, test.ns, "something")
queryInfo, err := lii.constructQuery(&test.listOptions, test.partitions, test.ns, "something")
if test.expectedErr != nil {
assert.Equal(t, test.expectedErr, err)
return
@ -1667,7 +1667,7 @@ func TestConstructQuery(t *testing.T) {
Indexer: i,
indexedFields: []string{"metadata.queryField1", "status.queryField2"},
}
queryInfo, err := lii.constructQuery(test.listOptions, test.partitions, test.ns, "something")
queryInfo, err := lii.constructQuery(&test.listOptions, test.partitions, test.ns, "something")
if test.expectedErr != nil {
assert.Equal(t, test.expectedErr, err)
return

4
pkg/sqlcache/integration_test.go
View File

@ -284,7 +284,7 @@ func (i *IntegrationSuite) TestSQLCacheFilters() {
partitions := []partition.Partition{defaultPartition}
ctx, cancel := context.WithTimeout(context.Background(), time.Second*5)
defer cancel()
cfgMaps, total, continueToken, err := cache.ListByOptions(ctx, options, partitions, testNamespace)
cfgMaps, total, continueToken, err := cache.ListByOptions(ctx, &options, partitions, testNamespace)
i.Require().NoError(err)
// since there's no additional pages, the continue token should be empty
i.Require().Equal("", continueToken)
@ -338,7 +338,7 @@ func (i *IntegrationSuite) waitForCacheReady(readyResourceNames []string, namesp
partitions := []partition.Partition{defaultPartition}
cacheCtx, cacheCancel := context.WithTimeout(ctx, time.Second*5)
defer cacheCancel()
currentResources, total, _, err := cache.ListByOptions(cacheCtx, options, partitions, namespace)
currentResources, total, _, err := cache.ListByOptions(cacheCtx, &options, partitions, namespace)
if err != nil {
// note that we don't return the error since that would stop the polling
return false, nil

4
pkg/stores/sqlpartition/listprocessor/processor.go
View File

@ -57,7 +57,7 @@ type Cache interface {
// - the total number of resources (returned list might be a subset depending on pagination options in lo)
// - a continue token, if there are more pages after the returned one
// - an error instead of all of the above if anything went wrong
ListByOptions(ctx context.Context, lo sqltypes.ListOptions, partitions []partition.Partition, namespace string) (*unstructured.UnstructuredList, int, string, error)
ListByOptions(ctx context.Context, lo *sqltypes.ListOptions, partitions []partition.Partition, namespace string) (*unstructured.UnstructuredList, int, string, error)
}
func k8sOpToRancherOp(k8sOp selection.Operator) (sqltypes.Op, bool, error) {
@ -202,7 +202,7 @@ func splitQuery(query string) []string {
func parseNamespaceOrProjectFilters(ctx context.Context, projOrNS string, op sqltypes.Op, namespaceInformer Cache) ([]sqltypes.Filter, error) {
var filters []sqltypes.Filter
for _, pn := range strings.Split(projOrNS, ",") {
uList, _, _, err := namespaceInformer.ListByOptions(ctx, sqltypes.ListOptions{
uList, _, _, err := namespaceInformer.ListByOptions(ctx, &sqltypes.ListOptions{
Filters: []sqltypes.OrFilter{
{
Filters: []sqltypes.Filter{

6
pkg/stores/sqlpartition/listprocessor/processor_test.go
View File

@ -82,7 +82,7 @@ func TestParseQuery(t *testing.T) {
},
}
nsc := NewMockCache(gomock.NewController(t))
nsc.EXPECT().ListByOptions(context.Background(), sqltypes.ListOptions{
nsc.EXPECT().ListByOptions(context.Background(), &sqltypes.ListOptions{
Filters: []sqltypes.OrFilter{
{
Filters: []sqltypes.Filter{
@ -132,7 +132,7 @@ func TestParseQuery(t *testing.T) {
errExpected: true,
setupNSCache: func() Cache {
nsi := NewMockCache(gomock.NewController(t))
nsi.EXPECT().ListByOptions(context.Background(), sqltypes.ListOptions{
nsi.EXPECT().ListByOptions(context.Background(), &sqltypes.ListOptions{
Filters: []sqltypes.OrFilter{
{
Filters: []sqltypes.Filter{
@ -185,7 +185,7 @@ func TestParseQuery(t *testing.T) {
Items: []unstructured.Unstructured{},
}
nsi := NewMockCache(gomock.NewController(t))
nsi.EXPECT().ListByOptions(context.Background(), sqltypes.ListOptions{
nsi.EXPECT().ListByOptions(context.Background(), &sqltypes.ListOptions{
Filters: []sqltypes.OrFilter{
{
Filters: []sqltypes.Filter{

2
pkg/stores/sqlpartition/listprocessor/proxy_mocks_test.go
View File

@ -43,7 +43,7 @@ func (m *MockCache) EXPECT() *MockCacheMockRecorder {
}
// ListByOptions mocks base method.
func (m *MockCache) ListByOptions(arg0 context.Context, arg1 sqltypes.ListOptions, arg2 []partition.Partition, arg3 string) (*unstructured.UnstructuredList, int, string, error) {
func (m *MockCache) ListByOptions(arg0 context.Context, arg1 *sqltypes.ListOptions, arg2 []partition.Partition, arg3 string) (*unstructured.UnstructuredList, int, string, error) {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "ListByOptions", arg0, arg1, arg2, arg3)
ret0, _ := ret[0].(*unstructured.UnstructuredList)

2
pkg/stores/sqlproxy/proxy_mocks_test.go
View File

@ -51,7 +51,7 @@ func (m *MockCache) EXPECT() *MockCacheMockRecorder {
}
// ListByOptions mocks base method.
func (m *MockCache) ListByOptions(arg0 context.Context, arg1 sqltypes.ListOptions, arg2 []partition.Partition, arg3 string) (*unstructured.UnstructuredList, int, string, error) {
func (m *MockCache) ListByOptions(arg0 context.Context, arg1 *sqltypes.ListOptions, arg2 []partition.Partition, arg3 string) (*unstructured.UnstructuredList, int, string, error) {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "ListByOptions", arg0, arg1, arg2, arg3)
ret0, _ := ret[0].(*unstructured.UnstructuredList)

4
pkg/stores/sqlproxy/proxy_store.go
View File

@ -217,7 +217,7 @@ type Cache interface {
// - the total number of resources (returned list might be a subset depending on pagination options in lo)
// - a continue token, if there are more pages after the returned one
// - an error instead of all of the above if anything went wrong
ListByOptions(ctx context.Context, lo sqltypes.ListOptions, partitions []partition.Partition, namespace string) (*unstructured.UnstructuredList, int, string, error)
ListByOptions(ctx context.Context, lo *sqltypes.ListOptions, partitions []partition.Partition, namespace string) (*unstructured.UnstructuredList, int, string, error)
}
// WarningBuffer holds warnings that may be returned from the kubernetes api
@ -784,7 +784,7 @@ func (s *Store) ListByPartitions(apiOp *types.APIRequest, schema *types.APISchem
return nil, 0, "", err
}
list, total, continueToken, err := inf.ListByOptions(apiOp.Context(), opts, partitions, apiOp.Namespace)
list, total, continueToken, err := inf.ListByOptions(apiOp.Context(), &opts, partitions, apiOp.Namespace)
if err != nil {
if errors.Is(err, informer.ErrInvalidColumn) {
return nil, 0, "", apierror.NewAPIError(validation.InvalidBodyContent, err.Error())

6
pkg/stores/sqlproxy/proxy_store_test.go
View File

@ -244,7 +244,7 @@ func TestListByPartitions(t *testing.T) {
// This tests that fields are being extracted from schema columns and the type specific fields map
cf.EXPECT().CacheFor(context.Background(), [][]string{{"some", "field"}, {`id`}, {`metadata`, `state`, `name`}, {"gvk", "specific", "fields"}}, gomock.Any(), &tablelistconvert.Client{ResourceInterface: ri}, attributes.GVK(schema), attributes.Namespaced(schema), true).Return(c, nil)
tb.EXPECT().GetTransformFunc(attributes.GVK(schema)).Return(func(obj interface{}) (interface{}, error) { return obj, nil })
bloi.EXPECT().ListByOptions(req.Context(), opts, partitions, req.Namespace).Return(listToReturn, len(listToReturn.Items), "", nil)
bloi.EXPECT().ListByOptions(req.Context(), &opts, partitions, req.Namespace).Return(listToReturn, len(listToReturn.Items), "", nil)
list, total, contToken, err := s.ListByPartitions(req, schema, partitions)
assert.Nil(t, err)
assert.Equal(t, expectedItems, list)
@ -461,7 +461,7 @@ func TestListByPartitions(t *testing.T) {
cf.EXPECT().CacheFor(context.Background(), [][]string{{"some", "field"}, {`id`}, {`metadata`, `state`, `name`}, {"gvk", "specific", "fields"}}, gomock.Any(), &tablelistconvert.Client{ResourceInterface: ri}, attributes.GVK(schema), attributes.Namespaced(schema), false).Return(c, nil)
tb.EXPECT().GetTransformFunc(attributes.GVK(schema)).Return(func(obj interface{}) (interface{}, error) { return obj, nil })
bloi.EXPECT().ListByOptions(req.Context(), opts, partitions, req.Namespace).Return(listToReturn, len(listToReturn.Items), "", nil)
bloi.EXPECT().ListByOptions(req.Context(), &opts, partitions, req.Namespace).Return(listToReturn, len(listToReturn.Items), "", nil)
list, total, contToken, err := s.ListByPartitions(req, schema, partitions)
assert.Nil(t, err)
assert.Equal(t, expectedItems, list)
@ -610,7 +610,7 @@ func TestListByPartitions(t *testing.T) {
cg.EXPECT().TableAdminClient(req, schema, "", &WarningBuffer{}).Return(ri, nil)
// This tests that fields are being extracted from schema columns and the type specific fields map
cf.EXPECT().CacheFor(context.Background(), [][]string{{"some", "field"}, {`id`}, {`metadata`, `state`, `name`}, {"gvk", "specific", "fields"}}, gomock.Any(), &tablelistconvert.Client{ResourceInterface: ri}, attributes.GVK(schema), attributes.Namespaced(schema), true).Return(c, nil)
bloi.EXPECT().ListByOptions(req.Context(), opts, partitions, req.Namespace).Return(nil, 0, "", fmt.Errorf("error"))
bloi.EXPECT().ListByOptions(req.Context(), &opts, partitions, req.Namespace).Return(nil, 0, "", fmt.Errorf("error"))
tb.EXPECT().GetTransformFunc(attributes.GVK(schema)).Return(func(obj interface{}) (interface{}, error) { return obj, nil })
_, _, _, err = s.ListByPartitions(req, schema, partitions)

2
pkg/stores/sqlproxy/sql_informer_mocks_test.go
View File

@ -43,7 +43,7 @@ func (m *MockByOptionsLister) EXPECT() *MockByOptionsListerMockRecorder {
}
// ListByOptions mocks base method.
func (m *MockByOptionsLister) ListByOptions(arg0 context.Context, arg1 sqltypes.ListOptions, arg2 []partition.Partition, arg3 string) (*unstructured.UnstructuredList, int, string, error) {
func (m *MockByOptionsLister) ListByOptions(arg0 context.Context, arg1 *sqltypes.ListOptions, arg2 []partition.Partition, arg3 string) (*unstructured.UnstructuredList, int, string, error) {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "ListByOptions", arg0, arg1, arg2, arg3)
ret0, _ := ret[0].(*unstructured.UnstructuredList)