From 912361f7a88b83c941ac250be6da065bdd9e9454 Mon Sep 17 00:00:00 2001 From: Eric Promislow Date: Wed, 13 Nov 2024 18:08:17 -0800 Subject: [PATCH] Get the unit tests to pass. --- pkg/cache/sql/informer/listoption_indexer.go | 51 ++++++++++--------- .../sql/informer/listoption_indexer_test.go | 21 ++++++-- 2 files changed, 44 insertions(+), 28 deletions(-) diff --git a/pkg/cache/sql/informer/listoption_indexer.go b/pkg/cache/sql/informer/listoption_indexer.go index 0491394a..5b66fab3 100644 --- a/pkg/cache/sql/informer/listoption_indexer.go +++ b/pkg/cache/sql/informer/listoption_indexer.go @@ -46,9 +46,10 @@ var ( ) const ( - matchFmt = `%%%s%%` - strictMatchFmt = `%s` - createFieldsTableFmt = `CREATE TABLE "%s_fields" ( + matchFmt = `%%%s%%` + strictMatchFmt = `%s` + escapeBackslashDirective = ` ESCAPE '\'` // The leading space is crucial for unit tests only + createFieldsTableFmt = `CREATE TABLE "%s_fields" ( key TEXT NOT NULL PRIMARY KEY, %s )` @@ -424,8 +425,8 @@ func (l *ListOptionIndexer) ListByOptions(ctx context.Context, lo ListOptions, p // assemble and log the final query query += limitClause query += offsetClause - logrus.Infof("ListOptionIndexer prepared statement: %v", query) - logrus.Infof("Params: %v", params) + logrus.Debugf("ListOptionIndexer prepared statement: %v", query) + logrus.Debugf("Params: %v", params) // execute stmt := l.Prepare(query) @@ -511,6 +512,7 @@ func (l *ListOptionIndexer) buildORClauseFromFilters(orFilters OrFilter) (string func (l *ListOptionIndexer) getFieldFilter(filter Filter) (string, []any, error) { opString := "" + escapeString := "" columnName := toColumnName(filter.Field) if err := l.validateColumn(columnName); err != nil { return "", nil, err @@ -519,21 +521,23 @@ func (l *ListOptionIndexer) getFieldFilter(filter Filter) (string, []any, error) case Eq: if filter.Partial { opString = "LIKE" + escapeString = escapeBackslashDirective } else { opString = "=" } - clause := fmt.Sprintf((`f."%s" %s ?`), columnName, opString) + clause := fmt.Sprintf(`f."%s" %s ?%s`, columnName, opString, escapeString) return clause, []any{formatMatchTarget(filter)}, nil case NotEq: if filter.Partial { opString = "NOT LIKE" + escapeString = escapeBackslashDirective } else { opString = "!=" } - clause := fmt.Sprintf(`(f."%s" %s ?)`, columnName, opString) + clause := fmt.Sprintf(`f."%s" %s ?%s`, columnName, opString, escapeString) return clause, []any{formatMatchTarget(filter)}, nil case Exists: - clause := fmt.Sprintf(`(f."%s" IS NOT NULL)`, columnName) + clause := fmt.Sprintf(`f."%s" IS NOT NULL`, columnName) return clause, []any{}, nil case In: fallthrough @@ -546,7 +550,7 @@ func (l *ListOptionIndexer) getFieldFilter(filter Filter) (string, []any, error) if filter.Op == "notin" { opString = "NOT IN" } - clause := fmt.Sprintf(`(f."%s" %s IN %s)`, columnName, opString, target) + clause := fmt.Sprintf(`f."%s" %s IN %s`, columnName, opString, target) matches := make([]any, len(filter.Matches)) for i, match := range filter.Matches { matches[i] = match @@ -559,38 +563,38 @@ func (l *ListOptionIndexer) getFieldFilter(filter Filter) (string, []any, error) func (l *ListOptionIndexer) getLabelFilter(filter Filter) (string, []any, error) { opString := "" - columnName := toColumnName(filter.Field) - if err := l.validateColumn(columnName); err != nil { - return "", nil, err - } + escapeString := "" if len(filter.Field) < 3 || filter.Field[0] != "metadata" || filter.Field[1] != "labels" { - return "", nil, fmt.Errorf("expecting a metadata.labels field, got '%s'", toColumnName(filter.Field)) + return "", nil, fmt.Errorf("expecting a metadata.labels field, got '%s'", strings.Join(filter.Field, ".")) } matchFmtToUse := strictMatchFmt + labelName := filter.Field[2] switch filter.Op { case Eq: if filter.Partial { opString = "LIKE" + escapeString = escapeBackslashDirective matchFmtToUse = matchFmt } else { opString = "=" } - clause := fmt.Sprintf(`(lt.label = ? AND lt.value %s ?)`, opString) - return clause, []any{formatMatchTargetWithFormatter(filter.Field[2], matchFmtToUse), formatMatchTargetWithFormatter(filter.Matches[0], matchFmtToUse)}, nil + clause := fmt.Sprintf(`lt.label = ? AND lt.value %s ?%s`, opString, escapeString) + return clause, []any{labelName, formatMatchTargetWithFormatter(filter.Matches[0], matchFmtToUse)}, nil case NotEq: if filter.Partial { opString = "NOT LIKE" + escapeString = escapeBackslashDirective matchFmtToUse = matchFmt } else { opString = "!=" } - clause := fmt.Sprintf(`(lt.label = ? AND lt.value %s ?)`, opString) - return clause, []any{formatMatchTargetWithFormatter(filter.Field[2], matchFmtToUse), formatMatchTargetWithFormatter(filter.Matches[0], matchFmtToUse)}, nil + clause := fmt.Sprintf(`lt.label = ? AND lt.value %s ?%s`, opString, escapeString) + return clause, []any{labelName, formatMatchTargetWithFormatter(filter.Matches[0], matchFmtToUse)}, nil case Exists: - clause := fmt.Sprintf(`(lt.label = ?)`) - return clause, []any{formatMatchTargetWithFormatter(filter.Field[2], strictMatchFmt)}, nil + clause := fmt.Sprintf(`lt.label = ?`) + return clause, []any{labelName}, nil case In: fallthrough @@ -600,10 +604,11 @@ func (l *ListOptionIndexer) getLabelFilter(filter Filter) (string, []any, error) if filter.Op == NotIn { opString = "NOT IN" } - clause := fmt.Sprintf(`(lt.label = ? AND lt.value %s %s)`, opString, target) - matches := make([]any, len(filter.Matches)) + clause := fmt.Sprintf(`lt.label = ? AND lt.value %s %s`, opString, target) + matches := make([]any, len(filter.Matches)+1) + matches[0] = labelName for i, match := range filter.Matches { - matches[i] = match + matches[i+1] = match } return clause, matches, nil } diff --git a/pkg/cache/sql/informer/listoption_indexer_test.go b/pkg/cache/sql/informer/listoption_indexer_test.go index b201ce94..b2812bcd 100644 --- a/pkg/cache/sql/informer/listoption_indexer_test.go +++ b/pkg/cache/sql/informer/listoption_indexer_test.go @@ -11,6 +11,7 @@ import ( "database/sql" "fmt" "reflect" + "strings" "testing" "github.com/rancher/lasso/pkg/cache/sql/partition" @@ -304,6 +305,7 @@ func TestListByOptions(t *testing.T) { Field: []string{"metadata", "somefield"}, Matches: []string{"somevalue"}, Op: Eq, + Partial: true, }, }, }, @@ -317,7 +319,7 @@ func TestListByOptions(t *testing.T) { (f."metadata.somefield" LIKE ? ESCAPE '\') AND (FALSE) ORDER BY f."metadata.name" ASC `, - expectedStmtArgs: []any{"somevalue"}, + expectedStmtArgs: []any{"%somevalue%"}, returnList: []any{&unstructured.Unstructured{Object: unstrTestObjectMap}, &unstructured.Unstructured{Object: unstrTestObjectMap}}, expectedList: &unstructured.UnstructuredList{Object: map[string]interface{}{"items": []map[string]interface{}{unstrTestObjectMap, unstrTestObjectMap}}, Items: []unstructured.Unstructured{{Object: unstrTestObjectMap}, {Object: unstrTestObjectMap}}}, expectedContToken: "", @@ -332,6 +334,7 @@ func TestListByOptions(t *testing.T) { Field: []string{"metadata", "somefield"}, Matches: []string{"somevalue"}, Op: NotEq, + Partial: true, }, }, }, @@ -345,7 +348,7 @@ func TestListByOptions(t *testing.T) { (f."metadata.somefield" NOT LIKE ? ESCAPE '\') AND (FALSE) ORDER BY f."metadata.name" ASC `, - expectedStmtArgs: []any{"somevalue"}, + expectedStmtArgs: []any{"%somevalue%"}, returnList: []any{&unstructured.Unstructured{Object: unstrTestObjectMap}, &unstructured.Unstructured{Object: unstrTestObjectMap}}, expectedList: &unstructured.UnstructuredList{Object: map[string]interface{}{"items": []map[string]interface{}{unstrTestObjectMap, unstrTestObjectMap}}, Items: []unstructured.Unstructured{{Object: unstrTestObjectMap}, {Object: unstrTestObjectMap}}}, expectedContToken: "", @@ -395,11 +398,13 @@ func TestListByOptions(t *testing.T) { Field: []string{"metadata", "somefield"}, Matches: []string{"someothervalue"}, Op: Eq, + Partial: true, }, { Field: []string{"metadata", "somefield"}, Matches: []string{"somethirdvalue"}, Op: NotEq, + Partial: true, }, }, }, @@ -413,7 +418,7 @@ func TestListByOptions(t *testing.T) { (f."metadata.somefield" LIKE ? ESCAPE '\' OR f."metadata.somefield" LIKE ? ESCAPE '\' OR f."metadata.somefield" NOT LIKE ? ESCAPE '\') AND (FALSE) ORDER BY f."metadata.name" ASC `, - expectedStmtArgs: []any{"%somevalue%", "someothervalue", "somethirdvalue"}, + expectedStmtArgs: []any{"%somevalue%", "%someothervalue%", "%somethirdvalue%"}, returnList: []any{&unstructured.Unstructured{Object: unstrTestObjectMap}, &unstructured.Unstructured{Object: unstrTestObjectMap}}, expectedList: &unstructured.UnstructuredList{Object: map[string]interface{}{"items": []map[string]interface{}{unstrTestObjectMap, unstrTestObjectMap}}, Items: []unstructured.Unstructured{{Object: unstrTestObjectMap}, {Object: unstrTestObjectMap}}}, expectedContToken: "", @@ -434,6 +439,7 @@ func TestListByOptions(t *testing.T) { Field: []string{"status", "someotherfield"}, Matches: []string{"someothervalue"}, Op: NotEq, + Partial: true, }, }, }, @@ -443,6 +449,7 @@ func TestListByOptions(t *testing.T) { Field: []string{"metadata", "somefield"}, Matches: []string{"somethirdvalue"}, Op: Eq, + Partial: true, }, }, }, @@ -458,7 +465,7 @@ func TestListByOptions(t *testing.T) { (f."metadata.namespace" = ?) AND (FALSE) ORDER BY f."metadata.name" ASC `, - expectedStmtArgs: []any{"%somevalue%", "someothervalue", "somethirdvalue", "test4"}, + expectedStmtArgs: []any{"%somevalue%", "%someothervalue%", "%somethirdvalue%", "test4"}, returnList: []any{&unstructured.Unstructured{Object: unstrTestObjectMap}, &unstructured.Unstructured{Object: unstrTestObjectMap}}, expectedList: &unstructured.UnstructuredList{Object: map[string]interface{}{"items": []map[string]interface{}{unstrTestObjectMap, unstrTestObjectMap}}, Items: []unstructured.Unstructured{{Object: unstrTestObjectMap}, {Object: unstrTestObjectMap}}}, expectedContToken: "", @@ -796,8 +803,12 @@ func TestListByOptions(t *testing.T) { store.EXPECT().ReadInt(rows).Return(len(test.expectedList.Items), nil) store.EXPECT().CloseStmt(stmt).Return(nil) } + if strings.Contains(test.description, "should select the label") { + fmt.Printf("stop here") + } - list, total, contToken, err := lii.ListByOptions(context.TODO(), test.listOptions, test.partitions, test.ns) + contextForDebugging := context.TODO() + list, total, contToken, err := lii.ListByOptions(contextForDebugging, test.listOptions, test.partitions, test.ns) if test.expectedErr == nil { assert.Nil(t, err) } else {