From 2e952e99ff01d6c19a0af168fdb4909b32386f65 Mon Sep 17 00:00:00 2001 From: weilerN Date: Sun, 22 Jun 2025 12:54:56 +0300 Subject: [PATCH 1/6] Introducing FunctionTarget interface and implementations --- pkg/ingresscache/safetrie.go | 75 ++++++++++++++++++++++++++++++++++++ pkg/ingresscache/types.go | 18 +++++++++ 2 files changed, 93 insertions(+) diff --git a/pkg/ingresscache/safetrie.go b/pkg/ingresscache/safetrie.go index 79248201..773174b3 100644 --- a/pkg/ingresscache/safetrie.go +++ b/pkg/ingresscache/safetrie.go @@ -168,3 +168,78 @@ func excludeElemFromSlice(slice []string, elem string) []string { return slice } } + +// ----- implementations for FunctionTarget interface ----- + +type SingleTarget struct { + functionName string +} + +func (s *SingleTarget) Contains(functionName string) bool { + return s.functionName == functionName +} + +func (s *SingleTarget) Delete(functionName string) (FunctionTarget, error) { + if !s.Contains(functionName) { + // if the function name is not found, return the original SingleTarget + return s, nil + } + + // this should never be called for SingleTarget + return nil, errors.New("cannot remove function name from SingleTarget, it only contains one function name") +} + +func (s *SingleTarget) Add(functionName string) (FunctionTarget, error) { + if s.Contains(functionName) { + return s, nil + } + + return &CanaryTarget{functionNames: [2]string{s.functionName, functionName}}, nil +} + +func (s *SingleTarget) ToSliceString() []string { + return []string{s.functionName} +} + +func (s *SingleTarget) IsSingle() bool { + return true +} + +type CanaryTarget struct { + functionNames [2]string +} + +func (c *CanaryTarget) Contains(functionName string) bool { + return c.functionNames[0] == functionName || c.functionNames[1] == functionName +} + +func (c *CanaryTarget) Delete(functionName string) (FunctionTarget, error) { + if c.functionNames[0] == functionName { + return &SingleTarget{functionName: c.functionNames[1]}, nil + } + + if c.functionNames[1] == functionName { + return &SingleTarget{functionName: c.functionNames[0]}, nil + } + + // if reached here, it means CanaryTarget does not contain the function name + return c, nil +} + +func (c *CanaryTarget) Add(functionName string) (FunctionTarget, error) { + if c.Contains(functionName) { + // If the function already exists, return the original CanaryTarget + return c, nil + } + + // This should never be called for CanaryTarget since it should always contain exactly two function names + return c, errors.New("cannot add function name to CanaryTarget, it already contains two function names") +} + +func (c *CanaryTarget) ToSliceString() []string { + return []string{c.functionNames[0], c.functionNames[1]} +} + +func (c *CanaryTarget) IsSingle() bool { + return false +} diff --git a/pkg/ingresscache/types.go b/pkg/ingresscache/types.go index 6f66340f..10af3072 100644 --- a/pkg/ingresscache/types.go +++ b/pkg/ingresscache/types.go @@ -44,3 +44,21 @@ type IngressHostsTree interface { // IsEmpty checks if the tree is empty IsEmpty() bool } + +// FunctionTarget defines the trie.PathTrie value +type FunctionTarget interface { + // Contains checks if the function name is present + Contains(functionName string) bool + + // Delete returns an updated FunctionTarget after removing the function name + Delete(functionName string) (FunctionTarget, error) + + // Add returns an updated FunctionTarget after adding the function name + Add(functionName string) (FunctionTarget, error) + + // ToSliceString returns a slice of function names + ToSliceString() []string + + // IsSingle returns true if the struct type is SingleTarget + IsSingle() bool +} From dff469f2446c149fd6c1f013a5d3fd5c98e7fef5 Mon Sep 17 00:00:00 2001 From: weilerN Date: Sun, 22 Jun 2025 13:05:27 +0300 Subject: [PATCH 2/6] Adopt FunctionTarget into the safeTrie --- pkg/ingresscache/safetrie.go | 64 +++++++++++++----------------------- 1 file changed, 23 insertions(+), 41 deletions(-) diff --git a/pkg/ingresscache/safetrie.go b/pkg/ingresscache/safetrie.go index 773174b3..b6ac3923 100644 --- a/pkg/ingresscache/safetrie.go +++ b/pkg/ingresscache/safetrie.go @@ -21,7 +21,6 @@ such restriction. package ingresscache import ( - "slices" "sync" "github.com/dghubble/trie" @@ -57,22 +56,25 @@ func (st *SafeTrie) SetFunctionName(path string, function string) error { // get the exact path value in order to avoid creating a new path if it already exists pathValue := st.pathTrie.Get(path) if pathValue == nil { - st.pathTrie.Put(path, []string{function}) + st.pathTrie.Put(path, &SingleTarget{function}) return nil } - pathFunctionNames, ok := pathValue.([]string) + pathFunctionNames, ok := pathValue.(FunctionTarget) if !ok { - return errors.Errorf("path value should be []string, got %T", pathValue) + return errors.Errorf("path value should be FunctionTarget, got %T", pathValue) } - if slices.Contains(pathFunctionNames, function) { + if pathFunctionNames.Contains(function) { // If the function already exists at this path, skip adding it to prevent duplicates return nil } - pathFunctionNames = append(pathFunctionNames, function) - st.pathTrie.Put(path, pathFunctionNames) + functionNames, err := pathFunctionNames.Add(function) + if err != nil { + return errors.Wrapf(err, "failed to set function name to path. path: %s, function: %s", path, function) + } + st.pathTrie.Put(path, functionNames) return nil } @@ -88,22 +90,25 @@ func (st *SafeTrie) DeleteFunctionName(path string, function string) error { return nil } - pathFunctionNames, ok := pathValue.([]string) + pathFunctionNames, ok := pathValue.(FunctionTarget) if !ok { - return errors.Errorf("path value should be []string, got %T", pathValue) + return errors.Errorf("path value should be FunctionTarget, got %T", pathValue) } - // If the function to delete matches the current function name and it's the only value, delete the path - if len(pathFunctionNames) == 1 { - if pathFunctionNames[0] == function { + // If the function to delete matches the current function name, and it's the only value, delete the path + if pathFunctionNames.IsSingle() { + if pathFunctionNames.Contains(function) { st.pathTrie.Delete(path) } return nil } - // TODO - will be removed once moving into efficient pathFunctionNames implementation (i.e. not using slices) - pathFunctionNames = excludeElemFromSlice(pathFunctionNames, function) - st.pathTrie.Put(path, pathFunctionNames) + // update the path with the new function names after deletion + functionNames, err := pathFunctionNames.Delete(function) + if err != nil { + return errors.Wrapf(err, "failed to remove function name from path. function: %s, path: %s", function, path) + } + st.pathTrie.Put(path, functionNames) return nil } @@ -127,12 +132,12 @@ func (st *SafeTrie) GetFunctionNames(path string) ([]string, error) { return nil, errors.Errorf("no value found for path: %s", path) } - functionNames, ok := walkPathResult.([]string) + functionNames, ok := walkPathResult.(FunctionTarget) if !ok { - return nil, errors.Errorf("walkPathResult value should be []string, got %v", walkPathResult) + return nil, errors.Errorf("walkPathResult value should be FunctionTarget, got %v", walkPathResult) } - return functionNames, nil + return functionNames.ToSliceString(), nil } // IsEmpty return true if the SafeTrie is empty @@ -146,29 +151,6 @@ func (st *SafeTrie) IsEmpty() bool { return walkResult == nil } -// TODO - will be removed once moving into efficient pathFunctionNames implementation (i.e. not using slices) -func excludeElemFromSlice(slice []string, elem string) []string { - // Assuming len(slice) <= 2 based on the ingress validations of: https://github.com/nuclio/nuclio/pkg/platform/kube/platform.go - switch len(slice) { - case 1: - if slice[0] == elem { - return []string{} - } - return slice - case 2: - if slice[0] == elem { - return []string{slice[1]} - } - if slice[1] == elem { - return []string{slice[0]} - } - // elem not found, return original slice - return slice - default: - return slice - } -} - // ----- implementations for FunctionTarget interface ----- type SingleTarget struct { From 786fca4c9c6bcc5adf32a516bd648f8430441bd5 Mon Sep 17 00:00:00 2001 From: weilerN Date: Sun, 22 Jun 2025 15:49:26 +0300 Subject: [PATCH 3/6] Unit test for FunctionTarget implementations and adopt changes in current unit tests --- pkg/ingresscache/ingresscache_test.go | 44 +-- pkg/ingresscache/safetrie_test.go | 393 +++++++++++++++++++++++--- 2 files changed, 372 insertions(+), 65 deletions(-) diff --git a/pkg/ingresscache/ingresscache_test.go b/pkg/ingresscache/ingresscache_test.go index 663238bf..b54fee1d 100644 --- a/pkg/ingresscache/ingresscache_test.go +++ b/pkg/ingresscache/ingresscache_test.go @@ -123,13 +123,13 @@ func (suite *IngressCacheTestSuite) TestSet() { args testIngressCacheArgs expectError bool errorMessage string - expectedResult map[string]map[string][]string + expectedResult map[string]map[string]FunctionTarget }{ { name: "Set new host", args: testIngressCacheArgs{"example.com", "/test/path", "test-function-name-1"}, - expectedResult: map[string]map[string][]string{ - "example.com": {"/test/path": {"test-function-name-1"}}, + expectedResult: map[string]map[string]FunctionTarget{ + "example.com": {"/test/path": &SingleTarget{"test-function-name-1"}}, }, }, { name: "Set unique functionName that will be added to existing host and path", @@ -137,8 +137,8 @@ func (suite *IngressCacheTestSuite) TestSet() { initialState: []ingressCacheTestInitialState{ {"example.com", "/test/path", "test-function-name-1"}, }, - expectedResult: map[string]map[string][]string{ - "example.com": {"/test/path": {"test-function-name-1", "test-function-name-2"}}, + expectedResult: map[string]map[string]FunctionTarget{ + "example.com": {"/test/path": &CanaryTarget{[2]string{"test-function-name-1", "test-function-name-2"}}}, }, }, { name: "Set existing functionName for existing host and path", @@ -146,8 +146,8 @@ func (suite *IngressCacheTestSuite) TestSet() { initialState: []ingressCacheTestInitialState{ {"example.com", "/test/path", "test-function-name-1"}, }, - expectedResult: map[string]map[string][]string{ - "example.com": {"/test/path": {"test-function-name-1"}}, + expectedResult: map[string]map[string]FunctionTarget{ + "example.com": {"/test/path": &SingleTarget{"test-function-name-1"}}, }, }, { name: "Set another host and path", @@ -155,9 +155,9 @@ func (suite *IngressCacheTestSuite) TestSet() { initialState: []ingressCacheTestInitialState{ {"example.com", "/test/path", "test-function-name-1"}, }, - expectedResult: map[string]map[string][]string{ - "google.com": {"/test/path": {"test-function-name-1"}}, - "example.com": {"/test/path": {"test-function-name-1"}}, + expectedResult: map[string]map[string]FunctionTarget{ + "google.com": {"/test/path": &SingleTarget{"test-function-name-1"}}, + "example.com": {"/test/path": &SingleTarget{"test-function-name-1"}}, }, }, } { @@ -188,20 +188,20 @@ func (suite *IngressCacheTestSuite) TestDelete() { expectError bool errorMessage string initialState []ingressCacheTestInitialState - expectedResult map[string]map[string][]string + expectedResult map[string]map[string]FunctionTarget }{ { name: "Delete when cache is empty", args: testIngressCacheArgs{"example.com", "/test/path", "test-function-name-1"}, - expectedResult: map[string]map[string][]string{}, + expectedResult: map[string]map[string]FunctionTarget{}, }, { name: "Delete not existed host", args: testIngressCacheArgs{"google.com", "/test/path", "test-function-name-1"}, initialState: []ingressCacheTestInitialState{ {"example.com", "/test/path", "test-function-name-1"}, }, - expectedResult: map[string]map[string][]string{ - "example.com": {"/test/path": {"test-function-name-1"}}, + expectedResult: map[string]map[string]FunctionTarget{ + "example.com": {"/test/path": &SingleTarget{"test-function-name-1"}}, }, }, { name: "Delete last function in host, validate host deletion", @@ -210,8 +210,8 @@ func (suite *IngressCacheTestSuite) TestDelete() { {"example.com", "/test/path", "test-function-name-1"}, {"google.com", "/test/path", "test-function-name-1"}, }, - expectedResult: map[string]map[string][]string{ - "google.com": {"/test/path": {"test-function-name-1"}}, + expectedResult: map[string]map[string]FunctionTarget{ + "google.com": {"/test/path": &SingleTarget{"test-function-name-1"}}, }, }, { name: "Delete not existing url and validate host wasn't deleted", @@ -219,8 +219,8 @@ func (suite *IngressCacheTestSuite) TestDelete() { initialState: []ingressCacheTestInitialState{ {"example.com", "/test/path", "test-function-name-1"}, }, - expectedResult: map[string]map[string][]string{ - "example.com": {"/test/path": {"test-function-name-1"}}, + expectedResult: map[string]map[string]FunctionTarget{ + "example.com": {"/test/path": &SingleTarget{"test-function-name-1"}}, }, }, { name: "Delete not last function in path and validate host wasn't deleted", @@ -229,8 +229,8 @@ func (suite *IngressCacheTestSuite) TestDelete() { {"example.com", "/test/path", "test-function-name-1"}, {"example.com", "/test/path", "test-function-name-2"}, }, - expectedResult: map[string]map[string][]string{ - "example.com": {"/test/path": {"test-function-name-1"}}, + expectedResult: map[string]map[string]FunctionTarget{ + "example.com": {"/test/path": &SingleTarget{"test-function-name-1"}}, }, }, } { @@ -270,8 +270,8 @@ func (suite *IngressCacheTestSuite) getTestIngressCache(initialState []ingressCa } // flattenIngressCache flattens the IngressCache's syncMap into a map for easier comparison in tests -func (suite *IngressCacheTestSuite) flattenIngressCache(testIngressCache *IngressCache) map[string]map[string][]string { - output := make(map[string]map[string][]string) +func (suite *IngressCacheTestSuite) flattenIngressCache(testIngressCache *IngressCache) map[string]map[string]FunctionTarget { + output := make(map[string]map[string]FunctionTarget) testIngressCache.syncMap.Range(func(key, value interface{}) bool { safeTrie, ok := value.(*SafeTrie) suite.Require().True(ok) diff --git a/pkg/ingresscache/safetrie_test.go b/pkg/ingresscache/safetrie_test.go index 1fcf0e04..3d2fda04 100644 --- a/pkg/ingresscache/safetrie_test.go +++ b/pkg/ingresscache/safetrie_test.go @@ -40,7 +40,7 @@ func (suite *SafeTrieTestSuite) TestPathTreeSet() { for _, testCase := range []struct { name string args []safeTrieFunctionArgs - expectedResult map[string][]string + expectedResult map[string]FunctionTarget expectError bool errorMessage string }{ @@ -52,7 +52,7 @@ func (suite *SafeTrieTestSuite) TestPathTreeSet() { function: "test-function", }, }, - expectedResult: map[string][]string{"/path/to/function": {"test-function"}}, + expectedResult: map[string]FunctionTarget{"/path/to/function": &SingleTarget{"test-function"}}, }, { name: "idempotent test", args: []safeTrieFunctionArgs{ @@ -64,7 +64,7 @@ func (suite *SafeTrieTestSuite) TestPathTreeSet() { function: "test-function", }, }, - expectedResult: map[string][]string{"/path/to/function": {"test-function"}}, + expectedResult: map[string]FunctionTarget{"/path/to/function": &SingleTarget{"test-function"}}, }, { name: "set twice the same path with a different function", args: []safeTrieFunctionArgs{ @@ -76,7 +76,7 @@ func (suite *SafeTrieTestSuite) TestPathTreeSet() { function: "test-function2", }, }, - expectedResult: map[string][]string{"/path/to/function": {"test-function", "test-function2"}}, + expectedResult: map[string]FunctionTarget{"/path/to/function": &CanaryTarget{[2]string{"test-function", "test-function2"}}}, }, { name: "set nested paths and different functions", args: []safeTrieFunctionArgs{ @@ -88,9 +88,9 @@ func (suite *SafeTrieTestSuite) TestPathTreeSet() { function: "test-function2", }, }, - expectedResult: map[string][]string{ - "/path/to/function": {"test-function"}, - "/path/to/function/nested": {"test-function2"}, + expectedResult: map[string]FunctionTarget{ + "/path/to/function": &SingleTarget{"test-function"}, + "/path/to/function/nested": &SingleTarget{"test-function2"}, }, }, { name: "set different paths and different functions", @@ -103,9 +103,9 @@ func (suite *SafeTrieTestSuite) TestPathTreeSet() { function: "test-function2", }, }, - expectedResult: map[string][]string{ - "/path/to/function": {"test-function"}, - "/another/path/to/function/": {"test-function2"}, + expectedResult: map[string]FunctionTarget{ + "/path/to/function": &SingleTarget{"test-function"}, + "/another/path/to/function/": &SingleTarget{"test-function2"}, }, }, { name: "empty function name", @@ -115,7 +115,7 @@ func (suite *SafeTrieTestSuite) TestPathTreeSet() { function: "", }, }, - expectedResult: map[string][]string{}, + expectedResult: map[string]FunctionTarget{}, expectError: true, errorMessage: "function is empty", }, { @@ -126,7 +126,7 @@ func (suite *SafeTrieTestSuite) TestPathTreeSet() { function: "test-function", }, }, - expectedResult: map[string][]string{}, + expectedResult: map[string]FunctionTarget{}, expectError: true, errorMessage: "path is empty", }, { @@ -137,8 +137,8 @@ func (suite *SafeTrieTestSuite) TestPathTreeSet() { function: "test-function", }, }, - expectedResult: map[string][]string{ - "///path/to/function": {"test-function"}, + expectedResult: map[string]FunctionTarget{ + "///path/to/function": &SingleTarget{"test-function"}, }, }, { name: "path starts without slash", @@ -148,8 +148,8 @@ func (suite *SafeTrieTestSuite) TestPathTreeSet() { function: "test-function", }, }, - expectedResult: map[string][]string{ - "path/to/function": {"test-function"}, + expectedResult: map[string]FunctionTarget{ + "path/to/function": &SingleTarget{"test-function"}, }, }, { name: "lots of paths and functions", @@ -158,28 +158,28 @@ func (suite *SafeTrieTestSuite) TestPathTreeSet() { }, { name: "path ends with slash", args: []safeTrieFunctionArgs{{path: "/path/to/function/", function: "test-function"}}, - expectedResult: map[string][]string{"/path/to/function/": {"test-function"}}, + expectedResult: map[string]FunctionTarget{"/path/to/function/": &SingleTarget{"test-function"}}, }, { name: "path with dots", args: []safeTrieFunctionArgs{{path: "/path/./to/./function/", function: "test-function"}}, - expectedResult: map[string][]string{"/path/./to/./function/": {"test-function"}}, + expectedResult: map[string]FunctionTarget{"/path/./to/./function/": &SingleTarget{"test-function"}}, }, { name: "upper case path", args: []safeTrieFunctionArgs{{path: "/PATH/TO/function", function: "test-function"}}, - expectedResult: map[string][]string{"/PATH/TO/function": {"test-function"}}, + expectedResult: map[string]FunctionTarget{"/PATH/TO/function": &SingleTarget{"test-function"}}, }, { name: "upper case function name", args: []safeTrieFunctionArgs{ {path: "/path/to/function", function: "test-function"}, {path: "/path/to/function", function: "test-FUNCTION"}, }, - expectedResult: map[string][]string{"/path/to/function": {"test-function", "test-FUNCTION"}}, + expectedResult: map[string]FunctionTarget{"/path/to/function": &CanaryTarget{[2]string{"test-function", "test-FUNCTION"}}}, }, { name: "path with numbers and hyphens", args: []safeTrieFunctionArgs{ {path: "/api/v1/user-data/123", function: "test-function"}, }, - expectedResult: map[string][]string{"/api/v1/user-data/123": {"test-function"}}, + expectedResult: map[string]FunctionTarget{"/api/v1/user-data/123": &SingleTarget{"test-function"}}, }, } { suite.Run(testCase.name, func() { @@ -275,7 +275,7 @@ func (suite *SafeTrieTestSuite) TestPathTreeDelete() { initialState []safeTrieFunctionArgs // initial state of the path tree before delete name string deleteArgs safeTrieFunctionArgs - expectedResult map[string][]string + expectedResult map[string]FunctionTarget expectError bool errorMessage string }{ @@ -286,8 +286,8 @@ func (suite *SafeTrieTestSuite) TestPathTreeDelete() { {"/path/to/function1/nested", "test-function2"}, }, deleteArgs: safeTrieFunctionArgs{"/path/to/function1", "test-function1"}, - expectedResult: map[string][]string{ - "/path/to/function1/nested": {"test-function2"}, + expectedResult: map[string]FunctionTarget{ + "/path/to/function1/nested": &SingleTarget{"test-function2"}, }, }, { name: "delete a function from multiple values and validate that the other function is still there", @@ -296,8 +296,8 @@ func (suite *SafeTrieTestSuite) TestPathTreeDelete() { {"/path/to/multiple/functions", "test-function2"}, }, deleteArgs: safeTrieFunctionArgs{"/path/to/multiple/functions", "test-function1"}, - expectedResult: map[string][]string{ - "/path/to/multiple/functions": {"test-function2"}, + expectedResult: map[string]FunctionTarget{ + "/path/to/multiple/functions": &SingleTarget{"test-function2"}, }, }, { name: "delete function that does not exist in the path", @@ -305,8 +305,8 @@ func (suite *SafeTrieTestSuite) TestPathTreeDelete() { {"/path/to/function1", "test-function1"}, }, deleteArgs: safeTrieFunctionArgs{"/path/to/function1", "test-function2"}, - expectedResult: map[string][]string{ - "/path/to/function1": {"test-function1"}, + expectedResult: map[string]FunctionTarget{ + "/path/to/function1": &SingleTarget{"test-function1"}, }, }, { name: "delete function that does not exist in multiple value path", @@ -315,8 +315,8 @@ func (suite *SafeTrieTestSuite) TestPathTreeDelete() { {"/path/to/functions", "test-function2"}, }, deleteArgs: safeTrieFunctionArgs{"/path/to/functions", "test-not-existing-function"}, - expectedResult: map[string][]string{ - "/path/to/functions": {"test-function1", "test-function2"}, + expectedResult: map[string]FunctionTarget{ + "/path/to/functions": &CanaryTarget{[2]string{"test-function1", "test-function2"}}, }, }, { name: "delete not exist path", @@ -324,8 +324,8 @@ func (suite *SafeTrieTestSuite) TestPathTreeDelete() { {"/path/to/function1", "test-function1"}, }, deleteArgs: safeTrieFunctionArgs{"/path/to/function1/nested", "test-function2"}, - expectedResult: map[string][]string{ - "/path/to/function1": {"test-function1"}, + expectedResult: map[string]FunctionTarget{ + "/path/to/function1": &SingleTarget{"test-function1"}, }, }, { name: "delete path with suffix that does not exist", @@ -334,9 +334,9 @@ func (suite *SafeTrieTestSuite) TestPathTreeDelete() { {"/path/to/function1/nested", "test-function2"}, }, deleteArgs: safeTrieFunctionArgs{"/path/to/function1/path/suffix", "test-function1"}, - expectedResult: map[string][]string{ - "/path/to/function1": {"test-function1"}, - "/path/to/function1/nested": {"test-function2"}, + expectedResult: map[string]FunctionTarget{ + "/path/to/function1": &SingleTarget{"test-function1"}, + "/path/to/function1/nested": &SingleTarget{"test-function2"}, }, }, } { @@ -387,19 +387,19 @@ func (suite *SafeTrieTestSuite) TestPathTreeIsEmpty() { // --- SafeTrieTestSuite suite methods --- -// flattenSafeTrie converts a PathTrie into a map[string][]string +// flattenSafeTrie converts a PathTrie into a map[string]FunctionTarget // This function is not part of the SafeTrieTestSuite because it is also in use in IngressCacheTestSuite -func flattenSafeTrie(st *SafeTrie) (map[string][]string, error) { - resultMap := make(map[string][]string) +func flattenSafeTrie(st *SafeTrie) (map[string]FunctionTarget, error) { + resultMap := make(map[string]FunctionTarget) err := st.pathTrie.Walk(func(key string, value interface{}) error { // The Walk function iterates over all nodes. // Only store key-value pairs where a non-nil value has been explicitly 'Put'. // If a node exists as an internal prefix (e.g., "/a" for "/a/b"), its 'value' will be nil. // We only care about the values that were actually stored. if value != nil { - convertedValue, ok := value.([]string) + convertedValue, ok := value.(FunctionTarget) if !ok { - return fmt.Errorf("path value should be []string") + return fmt.Errorf("path value should be FunctionTarget") } resultMap[key] = convertedValue } @@ -421,11 +421,11 @@ func (suite *SafeTrieTestSuite) generatePathsAndFunctions(num int) []safeTrieFun return args } -func (suite *SafeTrieTestSuite) generateExpectedResultMap(num int) map[string][]string { - expectedResult := make(map[string][]string) +func (suite *SafeTrieTestSuite) generateExpectedResultMap(num int) map[string]FunctionTarget { + expectedResult := make(map[string]FunctionTarget) args := suite.generatePathsAndFunctions(num) for i := 0; i < num; i++ { - expectedResult[args[i].path] = []string{args[i].function} + expectedResult[args[i].path] = &SingleTarget{args[i].function} } return expectedResult } @@ -447,3 +447,310 @@ func (suite *SafeTrieTestSuite) generateSafeTrieForTest(initialSafeTrieState []s func TestSafeTrie(t *testing.T) { suite.Run(t, new(SafeTrieTestSuite)) } + +// --- SingleTargetTestSuite --- +type SingleTargetTestSuite struct { + suite.Suite +} + +func (suite *SingleTargetTestSuite) TestContains() { + testCases := []struct { + name string + functionName string + expectedResult bool + }{ + { + name: "Contains exact match", + functionName: "myFunction", + expectedResult: true, + }, { + name: "Contains no match", + functionName: "otherFunction", + expectedResult: false, + }, { + name: "Contains empty function name no match", + functionName: "", + expectedResult: false, + }, { + name: "Contains case sensitive", + functionName: "MYFUNCTION", + expectedResult: false, + }, + } + + for _, testCase := range testCases { + suite.Run(testCase.name, func() { + testSingleFunctionName := &SingleTarget{functionName: "myFunction"} + result := testSingleFunctionName.Contains(testCase.functionName) + suite.Equal(testCase.expectedResult, result) + }) + } +} + +func (suite *SingleTargetTestSuite) TestRemoveFunctionName() { + testCases := []struct { + name string + functionName string + expectedResult FunctionTarget + expectError bool + errorMessage string + }{ + { + name: "RemoveExistingFunction", + functionName: "test-function1", + expectedResult: nil, + expectError: true, + errorMessage: "cannot remove function name from SingleTarget, it only contains one function name", + }, + { + name: "RemoveNonExistingFunction", + functionName: "otherFunction", + expectedResult: &SingleTarget{functionName: "test-function1"}, + }, + } + + for _, testCase := range testCases { + suite.Run(testCase.name, func() { + testSingleFunctionName := &SingleTarget{functionName: "test-function1"} + result, err := testSingleFunctionName.Delete(testCase.functionName) + if testCase.expectError { + suite.Require().Error(err) + suite.Require().ErrorContains(err, testCase.errorMessage) + suite.Nil(result) + } else { + suite.Equal(testCase.expectedResult, result) + } + }) + } +} + +func (suite *SingleTargetTestSuite) TestAddFunctionName() { + testCases := []struct { + name string + functionName string + expectedResult FunctionTarget + }{ + { + name: "Add same function name", + functionName: "test-function1", + expectedResult: &SingleTarget{functionName: "test-function1"}, + }, { + name: "Add function name", + functionName: "test-function2", + expectedResult: &CanaryTarget{[2]string{"test-function1", "test-function2"}}, + }, + } + + for _, testCase := range testCases { + suite.Run(testCase.name, func() { + testSingleFunctionName := &SingleTarget{functionName: "test-function1"} + result, err := testSingleFunctionName.Add(testCase.functionName) + suite.Require().NoError(err) + suite.Require().NotNil(result) + suite.Equal(testCase.expectedResult, result) + }) + } +} + +func (suite *SingleTargetTestSuite) TestToSliceString() { + testCases := []struct { + name string + singleFunctionName string + expectedResult []string + }{ + { + name: "ToSliceStringWithFunction", + singleFunctionName: "toSliceStringFunction", + expectedResult: []string{"toSliceStringFunction"}, + }, { + name: "ToSliceStringWithSpecialChars", + singleFunctionName: "my-function_123", + expectedResult: []string{"my-function_123"}, + }, + } + + for _, testCase := range testCases { + suite.Run(testCase.name, func() { + testSingleFunctionName := &SingleTarget{functionName: testCase.singleFunctionName} + result := testSingleFunctionName.ToSliceString() + suite.Equal(testCase.expectedResult, result) + suite.Len(result, 1) + }) + } +} + +func (suite *SingleTargetTestSuite) TestIsSingleFunctionName() { + testCases := []struct { + name string + singleFunctionName *SingleTarget + }{ + { + name: "IsSingleFunctionNameTrue", + singleFunctionName: &SingleTarget{functionName: "isSingleFunctionNameFunction"}, + }, + } + + for _, testCase := range testCases { + suite.Run(testCase.name, func() { + result := testCase.singleFunctionName.IsSingle() + suite.Require().True(result) + }) + } +} + +// TestSingleFunctionNameTestSuite runs the test suite +func TestSingleFunctionNameTestSuite(t *testing.T) { + suite.Run(t, new(SingleTargetTestSuite)) +} + +// --- CanaryTargetTestSuite --- +type CanaryTargetTestSuite struct { + suite.Suite +} + +func (suite *CanaryTargetTestSuite) TestContains() { + testCases := []struct { + name string + functionName string + expectedResult bool + }{ + { + name: "Contains match", + functionName: "test-function1", + expectedResult: true, + }, { + name: "Contains no match", + functionName: "test-function3", + expectedResult: false, + }, { + name: "Contains empty function name", + functionName: "", + expectedResult: false, + }, { + name: "Contains case sensitive", + functionName: "TEST-function1", + expectedResult: false, + }, + } + + for _, testCase := range testCases { + suite.Run(testCase.name, func() { + testCanaryFunctionNames := &CanaryTarget{[2]string{"test-function1", "test-function2"}} + result := testCanaryFunctionNames.Contains(testCase.functionName) + suite.Equal(testCase.expectedResult, result) + }) + } +} + +func (suite *CanaryTargetTestSuite) TestRemoveFunctionName() { + testCases := []struct { + name string + functionName string + expectedResult FunctionTarget + }{ + { + name: "RemoveExistingFunction", + functionName: "test-function1", + expectedResult: &SingleTarget{functionName: "test-function2"}, + }, { + name: "RemoveNotExistingFunction", + functionName: "test-function3", + expectedResult: &CanaryTarget{[2]string{"test-function1", "test-function2"}}, + }, + } + + for _, testCase := range testCases { + suite.Run(testCase.name, func() { + testCanaryFunctionNames := &CanaryTarget{[2]string{"test-function1", "test-function2"}} + result, err := testCanaryFunctionNames.Delete(testCase.functionName) + suite.Require().NoError(err) + suite.Require().Equal(testCase.expectedResult, result) + }) + } +} + +func (suite *CanaryTargetTestSuite) TestAddFunctionName() { + testCases := []struct { + name string + functionName string + expectedResult FunctionTarget + expectError bool + errorMessage string + }{ + { + name: "Add same function name", + functionName: "test-function1", + expectedResult: &CanaryTarget{[2]string{"test-function1", "test-function2"}}, + }, { + name: "Add distinct function name to a CanaryTarget", + functionName: "test-function3", + expectedResult: &CanaryTarget{[2]string{"test-function1", "test-function2"}}, + expectError: true, + errorMessage: "cannot add function name to CanaryTarget, it already contains two function names", + }, + } + + for _, testCase := range testCases { + suite.Run(testCase.name, func() { + testCanaryFunctionNames := &CanaryTarget{[2]string{"test-function1", "test-function2"}} + result, err := testCanaryFunctionNames.Add(testCase.functionName) + if testCase.expectError { + suite.Require().Error(err) + suite.Require().ErrorContains(err, testCase.errorMessage) + suite.Equal(testCase.expectedResult, result) + } else { + suite.Require().NoError(err) + suite.Equal(testCase.expectedResult, result) + } + }) + } +} + +func (suite *CanaryTargetTestSuite) TestToSliceString() { + testCases := []struct { + name string + canaryTarget [2]string + expectedResult []string + }{ + { + name: "ToSliceStringWithFunction", + canaryTarget: [2]string{"test-function1", "test-function2"}, + expectedResult: []string{"test-function1", "test-function2"}, + }, { + name: "ToSliceStringWithSpecialChars", + canaryTarget: [2]string{"my-function_123", "test-function2"}, + expectedResult: []string{"my-function_123", "test-function2"}, + }, + } + + for _, testCase := range testCases { + suite.Run(testCase.name, func() { + testCanaryFunctionNames := &CanaryTarget{testCase.canaryTarget} + result := testCanaryFunctionNames.ToSliceString() + suite.Equal(testCase.expectedResult, result) + }) + } +} + +func (suite *CanaryTargetTestSuite) TestIsSingleFunctionName() { + testCases := []struct { + name string + }{ + { + name: "IsSingleFunctionNameTrue", + }, + } + + for _, testCase := range testCases { + suite.Run(testCase.name, func() { + testCanaryFunctionNames := &CanaryTarget{[2]string{"test-function1", "test-function2"}} + result := testCanaryFunctionNames.IsSingle() + suite.Require().False(result) + }) + } +} + +// TestCanaryFunctionNamesTestSuite runs the test suite +func TestCanaryFunctionNamesTestSuite(t *testing.T) { + suite.Run(t, new(CanaryTargetTestSuite)) +} From e168a5f27318be47ec118c6cd7697a5e41cacbdc Mon Sep 17 00:00:00 2001 From: weilerN Date: Sun, 22 Jun 2025 16:38:10 +0300 Subject: [PATCH 4/6] propagate the FunctionTarget to the cache level and align interfaces functions declarations --- pkg/ingresscache/ingresscache.go | 8 +++--- pkg/ingresscache/safetrie.go | 14 +++++----- pkg/ingresscache/safetrie_test.go | 46 +++++++++++++++---------------- pkg/ingresscache/types.go | 12 ++++---- 4 files changed, 40 insertions(+), 40 deletions(-) diff --git a/pkg/ingresscache/ingresscache.go b/pkg/ingresscache/ingresscache.go index 92fe39a5..b18cafe5 100644 --- a/pkg/ingresscache/ingresscache.go +++ b/pkg/ingresscache/ingresscache.go @@ -50,7 +50,7 @@ func (ic *IngressCache) Set(host, path, function string) error { return errors.Errorf("cache set failed: invalid path tree value: got: %t", urlTree) } - if err := ingressHostsTree.SetFunctionName(path, function); err != nil { + if err := ingressHostsTree.Set(path, function); err != nil { return errors.Wrap(err, "failed to set function name in the ingress host tree") } @@ -72,7 +72,7 @@ func (ic *IngressCache) Delete(host, path, function string) error { return errors.Errorf("cache delete failed: invalid path tree value: got: %t", urlTree) } - if err := ingressHostsTree.DeleteFunctionName(path, function); err != nil { + if err := ingressHostsTree.Delete(path, function); err != nil { return errors.Wrap(err, "failed to delete function name from the ingress host tree") } @@ -97,10 +97,10 @@ func (ic *IngressCache) Get(host, path string) ([]string, error) { return nil, errors.Errorf("cache get failed: invalid path tree value: got: %t", urlTree) } - result, err := ingressHostsTree.GetFunctionNames(path) + result, err := ingressHostsTree.Get(path) if err != nil { return nil, errors.Wrap(err, "failed to get the function name from the ingress host tree") } - return result, nil + return result.ToSliceString(), nil } diff --git a/pkg/ingresscache/safetrie.go b/pkg/ingresscache/safetrie.go index b6ac3923..a7ca46af 100644 --- a/pkg/ingresscache/safetrie.go +++ b/pkg/ingresscache/safetrie.go @@ -40,8 +40,8 @@ func NewSafeTrie() *SafeTrie { } } -// SetFunctionName sets a function for a given path. If the path does not exist, it creates it -func (st *SafeTrie) SetFunctionName(path string, function string) error { +// Set adds a function for a given path. If the path does not exist, it creates it +func (st *SafeTrie) Set(path string, function string) error { if path == "" { return errors.New("path is empty") } @@ -79,8 +79,8 @@ func (st *SafeTrie) SetFunctionName(path string, function string) error { return nil } -// DeleteFunctionName removes a function from a path and also deletes the path if the function is the only one associated with that path -func (st *SafeTrie) DeleteFunctionName(path string, function string) error { +// Delete removes a function from a path and also deletes the path if the function is the only one associated with that path +func (st *SafeTrie) Delete(path string, function string) error { st.rwMutex.Lock() defer st.rwMutex.Unlock() @@ -112,8 +112,8 @@ func (st *SafeTrie) DeleteFunctionName(path string, function string) error { return nil } -// GetFunctionNames retrieve the closest prefix matching the path and returns the associated functions -func (st *SafeTrie) GetFunctionNames(path string) ([]string, error) { +// Get retrieve the closest prefix matching the path and returns the associated functions +func (st *SafeTrie) Get(path string) (FunctionTarget, error) { var walkPathResult interface{} if path == "" { return nil, errors.New("path is empty") @@ -137,7 +137,7 @@ func (st *SafeTrie) GetFunctionNames(path string) ([]string, error) { return nil, errors.Errorf("walkPathResult value should be FunctionTarget, got %v", walkPathResult) } - return functionNames.ToSliceString(), nil + return functionNames, nil } // IsEmpty return true if the SafeTrie is empty diff --git a/pkg/ingresscache/safetrie_test.go b/pkg/ingresscache/safetrie_test.go index 3d2fda04..1af871a3 100644 --- a/pkg/ingresscache/safetrie_test.go +++ b/pkg/ingresscache/safetrie_test.go @@ -185,7 +185,7 @@ func (suite *SafeTrieTestSuite) TestPathTreeSet() { suite.Run(testCase.name, func() { testSafeTrie := suite.generateSafeTrieForTest([]safeTrieFunctionArgs{}) for _, setArgs := range testCase.args { - err := testSafeTrie.SetFunctionName(setArgs.path, setArgs.function) + err := testSafeTrie.Set(setArgs.path, setArgs.function) if testCase.expectError { suite.Require().Error(err) suite.Require().ErrorContains(err, testCase.errorMessage) @@ -212,53 +212,53 @@ func (suite *SafeTrieTestSuite) TestPathTreeGet() { } for _, testCase := range []struct { name string - arg string - expectedResult []string + path string + expectedResult FunctionTarget expectError bool errorMessage string }{ { name: "get root path", - arg: "/", - expectedResult: []string{"test-function"}, + path: "/", + expectedResult: &SingleTarget{"test-function"}, }, { name: "get regular path", - arg: "/path/to/function1", - expectedResult: []string{"test-function1"}, + path: "/path/to/function1", + expectedResult: &SingleTarget{"test-function1"}, }, { name: "get nested path", - arg: "/path/to/function1/nested", - expectedResult: []string{"test-function2"}, + path: "/path/to/function1/nested", + expectedResult: &SingleTarget{"test-function2"}, }, { name: "get closest match", - arg: "/path/to/function1/nested/extra", - expectedResult: []string{"test-function2"}, + path: "/path/to/function1/nested/extra", + expectedResult: &SingleTarget{"test-function2"}, }, { name: "get empty path", - arg: "", + path: "", expectError: true, errorMessage: "path is empty", }, { name: "get closest match with different suffix", - arg: "/path/to/function1/something/else", - expectedResult: []string{"test-function1"}, + path: "/path/to/function1/something/else", + expectedResult: &SingleTarget{"test-function1"}, }, { name: "get path with dots", - arg: "/path/./to/./function/", - expectedResult: []string{"test-function1"}, + path: "/path/./to/./function/", + expectedResult: &SingleTarget{"test-function1"}, }, { name: "get path with slash", - arg: "path//to//function/", - expectedResult: []string{"test-function1"}, + path: "path//to//function/", + expectedResult: &SingleTarget{"test-function1"}, }, { name: "get multiple functions for the same path", - arg: "/path/to/multiple/functions", - expectedResult: []string{"test-function1", "test-function2"}, + path: "/path/to/multiple/functions", + expectedResult: &CanaryTarget{[2]string{"test-function1", "test-function2"}}, }, } { suite.Run(testCase.name, func() { testSafeTrie := suite.generateSafeTrieForTest(initialStateGetTest) - result, err := testSafeTrie.GetFunctionNames(testCase.arg) + result, err := testSafeTrie.Get(testCase.path) if testCase.expectError { suite.Require().Error(err) suite.Require().ErrorContains(err, testCase.errorMessage) @@ -343,7 +343,7 @@ func (suite *SafeTrieTestSuite) TestPathTreeDelete() { suite.Run(testCase.name, func() { testSafeTrie := suite.generateSafeTrieForTest(testCase.initialState) - err := testSafeTrie.DeleteFunctionName(testCase.deleteArgs.path, testCase.deleteArgs.function) + err := testSafeTrie.Delete(testCase.deleteArgs.path, testCase.deleteArgs.function) if testCase.expectError { suite.Require().Error(err) suite.Require().ErrorContains(err, testCase.errorMessage) @@ -437,7 +437,7 @@ func (suite *SafeTrieTestSuite) generateSafeTrieForTest(initialSafeTrieState []s // set path tree with the provided required state for _, args := range initialSafeTrieState { - err = safeTrie.SetFunctionName(args.path, args.function) + err = safeTrie.Set(args.path, args.function) suite.Require().NoError(err) } diff --git a/pkg/ingresscache/types.go b/pkg/ingresscache/types.go index 10af3072..50980e91 100644 --- a/pkg/ingresscache/types.go +++ b/pkg/ingresscache/types.go @@ -32,14 +32,14 @@ type IngressHostCache interface { } type IngressHostsTree interface { - // SetFunctionName sets a function for a given path. Will overwrite existing values if the path already exists - SetFunctionName(path string, function string) error + // Set sets a function for a given path. Will overwrite existing values if the path already exists + Set(path string, function string) error - // DeleteFunctionName removes the function from the given path and deletes the deepest suffix used only by that function; does nothing if the path or function doesn't exist. - DeleteFunctionName(path string, function string) error + // Delete removes the function from the given path and deletes the deepest suffix used only by that function; does nothing if the path or function doesn't exist. + Delete(path string, function string) error - // GetFunctionNames retrieves the best matching function names for a given path based on longest prefix match - GetFunctionNames(path string) ([]string, error) + // Get retrieves the best matching function names for a given path based on longest prefix match + Get(path string) (FunctionTarget, error) // IsEmpty checks if the tree is empty IsEmpty() bool From 0f1c127d223ae5a00841d38cf910af6cb3e18904 Mon Sep 17 00:00:00 2001 From: weilerN Date: Mon, 23 Jun 2025 17:15:04 +0300 Subject: [PATCH 5/6] CR comment- rephrase comments --- pkg/ingresscache/safetrie.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pkg/ingresscache/safetrie.go b/pkg/ingresscache/safetrie.go index a7ca46af..c509468e 100644 --- a/pkg/ingresscache/safetrie.go +++ b/pkg/ingresscache/safetrie.go @@ -66,7 +66,8 @@ func (st *SafeTrie) Set(path string, function string) error { } if pathFunctionNames.Contains(function) { - // If the function already exists at this path, skip adding it to prevent duplicates + // Although Add() checks if the function exists and returns the same value, it still performs a trie walk that ends with no changes when values are identical. + // This validation avoids that unnecessary walk return nil } @@ -74,12 +75,12 @@ func (st *SafeTrie) Set(path string, function string) error { if err != nil { return errors.Wrapf(err, "failed to set function name to path. path: %s, function: %s", path, function) } - st.pathTrie.Put(path, functionNames) + st.pathTrie.Put(path, functionNames) return nil } -// Delete removes a function from a path and also deletes the path if the function is the only one associated with that path +// Delete removes a function from a path and cleans up the longest suffix of the path only used by that function func (st *SafeTrie) Delete(path string, function string) error { st.rwMutex.Lock() defer st.rwMutex.Unlock() From 5a0a141f73778234bc15ed1d9a3f29300067e939 Mon Sep 17 00:00:00 2001 From: weilerN Date: Tue, 24 Jun 2025 14:50:55 +0300 Subject: [PATCH 6/6] CR comment-change singleTarget from struct to a string wrapper --- pkg/ingresscache/ingresscache_test.go | 16 +++--- pkg/ingresscache/safetrie.go | 26 +++++----- pkg/ingresscache/safetrie_test.go | 70 +++++++++++++-------------- 3 files changed, 55 insertions(+), 57 deletions(-) diff --git a/pkg/ingresscache/ingresscache_test.go b/pkg/ingresscache/ingresscache_test.go index b54fee1d..bcc4b3eb 100644 --- a/pkg/ingresscache/ingresscache_test.go +++ b/pkg/ingresscache/ingresscache_test.go @@ -129,7 +129,7 @@ func (suite *IngressCacheTestSuite) TestSet() { name: "Set new host", args: testIngressCacheArgs{"example.com", "/test/path", "test-function-name-1"}, expectedResult: map[string]map[string]FunctionTarget{ - "example.com": {"/test/path": &SingleTarget{"test-function-name-1"}}, + "example.com": {"/test/path": SingleTarget("test-function-name-1")}, }, }, { name: "Set unique functionName that will be added to existing host and path", @@ -147,7 +147,7 @@ func (suite *IngressCacheTestSuite) TestSet() { {"example.com", "/test/path", "test-function-name-1"}, }, expectedResult: map[string]map[string]FunctionTarget{ - "example.com": {"/test/path": &SingleTarget{"test-function-name-1"}}, + "example.com": {"/test/path": SingleTarget("test-function-name-1")}, }, }, { name: "Set another host and path", @@ -156,8 +156,8 @@ func (suite *IngressCacheTestSuite) TestSet() { {"example.com", "/test/path", "test-function-name-1"}, }, expectedResult: map[string]map[string]FunctionTarget{ - "google.com": {"/test/path": &SingleTarget{"test-function-name-1"}}, - "example.com": {"/test/path": &SingleTarget{"test-function-name-1"}}, + "google.com": {"/test/path": SingleTarget("test-function-name-1")}, + "example.com": {"/test/path": SingleTarget("test-function-name-1")}, }, }, } { @@ -201,7 +201,7 @@ func (suite *IngressCacheTestSuite) TestDelete() { {"example.com", "/test/path", "test-function-name-1"}, }, expectedResult: map[string]map[string]FunctionTarget{ - "example.com": {"/test/path": &SingleTarget{"test-function-name-1"}}, + "example.com": {"/test/path": SingleTarget("test-function-name-1")}, }, }, { name: "Delete last function in host, validate host deletion", @@ -211,7 +211,7 @@ func (suite *IngressCacheTestSuite) TestDelete() { {"google.com", "/test/path", "test-function-name-1"}, }, expectedResult: map[string]map[string]FunctionTarget{ - "google.com": {"/test/path": &SingleTarget{"test-function-name-1"}}, + "google.com": {"/test/path": SingleTarget("test-function-name-1")}, }, }, { name: "Delete not existing url and validate host wasn't deleted", @@ -220,7 +220,7 @@ func (suite *IngressCacheTestSuite) TestDelete() { {"example.com", "/test/path", "test-function-name-1"}, }, expectedResult: map[string]map[string]FunctionTarget{ - "example.com": {"/test/path": &SingleTarget{"test-function-name-1"}}, + "example.com": {"/test/path": SingleTarget("test-function-name-1")}, }, }, { name: "Delete not last function in path and validate host wasn't deleted", @@ -230,7 +230,7 @@ func (suite *IngressCacheTestSuite) TestDelete() { {"example.com", "/test/path", "test-function-name-2"}, }, expectedResult: map[string]map[string]FunctionTarget{ - "example.com": {"/test/path": &SingleTarget{"test-function-name-1"}}, + "example.com": {"/test/path": SingleTarget("test-function-name-1")}, }, }, } { diff --git a/pkg/ingresscache/safetrie.go b/pkg/ingresscache/safetrie.go index c509468e..2a07031f 100644 --- a/pkg/ingresscache/safetrie.go +++ b/pkg/ingresscache/safetrie.go @@ -56,7 +56,7 @@ func (st *SafeTrie) Set(path string, function string) error { // get the exact path value in order to avoid creating a new path if it already exists pathValue := st.pathTrie.Get(path) if pathValue == nil { - st.pathTrie.Put(path, &SingleTarget{function}) + st.pathTrie.Put(path, SingleTarget(function)) return nil } @@ -154,15 +154,13 @@ func (st *SafeTrie) IsEmpty() bool { // ----- implementations for FunctionTarget interface ----- -type SingleTarget struct { - functionName string -} +type SingleTarget string -func (s *SingleTarget) Contains(functionName string) bool { - return s.functionName == functionName +func (s SingleTarget) Contains(functionName string) bool { + return string(s) == functionName } -func (s *SingleTarget) Delete(functionName string) (FunctionTarget, error) { +func (s SingleTarget) Delete(functionName string) (FunctionTarget, error) { if !s.Contains(functionName) { // if the function name is not found, return the original SingleTarget return s, nil @@ -172,19 +170,19 @@ func (s *SingleTarget) Delete(functionName string) (FunctionTarget, error) { return nil, errors.New("cannot remove function name from SingleTarget, it only contains one function name") } -func (s *SingleTarget) Add(functionName string) (FunctionTarget, error) { +func (s SingleTarget) Add(functionName string) (FunctionTarget, error) { if s.Contains(functionName) { return s, nil } - return &CanaryTarget{functionNames: [2]string{s.functionName, functionName}}, nil + return &CanaryTarget{functionNames: [2]string{string(s), functionName}}, nil } -func (s *SingleTarget) ToSliceString() []string { - return []string{s.functionName} +func (s SingleTarget) ToSliceString() []string { + return []string{string(s)} } -func (s *SingleTarget) IsSingle() bool { +func (s SingleTarget) IsSingle() bool { return true } @@ -198,11 +196,11 @@ func (c *CanaryTarget) Contains(functionName string) bool { func (c *CanaryTarget) Delete(functionName string) (FunctionTarget, error) { if c.functionNames[0] == functionName { - return &SingleTarget{functionName: c.functionNames[1]}, nil + return SingleTarget(c.functionNames[1]), nil } if c.functionNames[1] == functionName { - return &SingleTarget{functionName: c.functionNames[0]}, nil + return SingleTarget(c.functionNames[0]), nil } // if reached here, it means CanaryTarget does not contain the function name diff --git a/pkg/ingresscache/safetrie_test.go b/pkg/ingresscache/safetrie_test.go index 1af871a3..bb7420d4 100644 --- a/pkg/ingresscache/safetrie_test.go +++ b/pkg/ingresscache/safetrie_test.go @@ -52,7 +52,7 @@ func (suite *SafeTrieTestSuite) TestPathTreeSet() { function: "test-function", }, }, - expectedResult: map[string]FunctionTarget{"/path/to/function": &SingleTarget{"test-function"}}, + expectedResult: map[string]FunctionTarget{"/path/to/function": SingleTarget("test-function")}, }, { name: "idempotent test", args: []safeTrieFunctionArgs{ @@ -64,7 +64,7 @@ func (suite *SafeTrieTestSuite) TestPathTreeSet() { function: "test-function", }, }, - expectedResult: map[string]FunctionTarget{"/path/to/function": &SingleTarget{"test-function"}}, + expectedResult: map[string]FunctionTarget{"/path/to/function": SingleTarget("test-function")}, }, { name: "set twice the same path with a different function", args: []safeTrieFunctionArgs{ @@ -89,8 +89,8 @@ func (suite *SafeTrieTestSuite) TestPathTreeSet() { }, }, expectedResult: map[string]FunctionTarget{ - "/path/to/function": &SingleTarget{"test-function"}, - "/path/to/function/nested": &SingleTarget{"test-function2"}, + "/path/to/function": SingleTarget("test-function"), + "/path/to/function/nested": SingleTarget("test-function2"), }, }, { name: "set different paths and different functions", @@ -104,8 +104,8 @@ func (suite *SafeTrieTestSuite) TestPathTreeSet() { }, }, expectedResult: map[string]FunctionTarget{ - "/path/to/function": &SingleTarget{"test-function"}, - "/another/path/to/function/": &SingleTarget{"test-function2"}, + "/path/to/function": SingleTarget("test-function"), + "/another/path/to/function/": SingleTarget("test-function2"), }, }, { name: "empty function name", @@ -138,7 +138,7 @@ func (suite *SafeTrieTestSuite) TestPathTreeSet() { }, }, expectedResult: map[string]FunctionTarget{ - "///path/to/function": &SingleTarget{"test-function"}, + "///path/to/function": SingleTarget("test-function"), }, }, { name: "path starts without slash", @@ -149,7 +149,7 @@ func (suite *SafeTrieTestSuite) TestPathTreeSet() { }, }, expectedResult: map[string]FunctionTarget{ - "path/to/function": &SingleTarget{"test-function"}, + "path/to/function": SingleTarget("test-function"), }, }, { name: "lots of paths and functions", @@ -158,15 +158,15 @@ func (suite *SafeTrieTestSuite) TestPathTreeSet() { }, { name: "path ends with slash", args: []safeTrieFunctionArgs{{path: "/path/to/function/", function: "test-function"}}, - expectedResult: map[string]FunctionTarget{"/path/to/function/": &SingleTarget{"test-function"}}, + expectedResult: map[string]FunctionTarget{"/path/to/function/": SingleTarget("test-function")}, }, { name: "path with dots", args: []safeTrieFunctionArgs{{path: "/path/./to/./function/", function: "test-function"}}, - expectedResult: map[string]FunctionTarget{"/path/./to/./function/": &SingleTarget{"test-function"}}, + expectedResult: map[string]FunctionTarget{"/path/./to/./function/": SingleTarget("test-function")}, }, { name: "upper case path", args: []safeTrieFunctionArgs{{path: "/PATH/TO/function", function: "test-function"}}, - expectedResult: map[string]FunctionTarget{"/PATH/TO/function": &SingleTarget{"test-function"}}, + expectedResult: map[string]FunctionTarget{"/PATH/TO/function": SingleTarget("test-function")}, }, { name: "upper case function name", args: []safeTrieFunctionArgs{ @@ -179,7 +179,7 @@ func (suite *SafeTrieTestSuite) TestPathTreeSet() { args: []safeTrieFunctionArgs{ {path: "/api/v1/user-data/123", function: "test-function"}, }, - expectedResult: map[string]FunctionTarget{"/api/v1/user-data/123": &SingleTarget{"test-function"}}, + expectedResult: map[string]FunctionTarget{"/api/v1/user-data/123": SingleTarget("test-function")}, }, } { suite.Run(testCase.name, func() { @@ -220,19 +220,19 @@ func (suite *SafeTrieTestSuite) TestPathTreeGet() { { name: "get root path", path: "/", - expectedResult: &SingleTarget{"test-function"}, + expectedResult: SingleTarget("test-function"), }, { name: "get regular path", path: "/path/to/function1", - expectedResult: &SingleTarget{"test-function1"}, + expectedResult: SingleTarget("test-function1"), }, { name: "get nested path", path: "/path/to/function1/nested", - expectedResult: &SingleTarget{"test-function2"}, + expectedResult: SingleTarget("test-function2"), }, { name: "get closest match", path: "/path/to/function1/nested/extra", - expectedResult: &SingleTarget{"test-function2"}, + expectedResult: SingleTarget("test-function2"), }, { name: "get empty path", path: "", @@ -241,15 +241,15 @@ func (suite *SafeTrieTestSuite) TestPathTreeGet() { }, { name: "get closest match with different suffix", path: "/path/to/function1/something/else", - expectedResult: &SingleTarget{"test-function1"}, + expectedResult: SingleTarget("test-function1"), }, { name: "get path with dots", path: "/path/./to/./function/", - expectedResult: &SingleTarget{"test-function1"}, + expectedResult: SingleTarget("test-function1"), }, { name: "get path with slash", path: "path//to//function/", - expectedResult: &SingleTarget{"test-function1"}, + expectedResult: SingleTarget("test-function1"), }, { name: "get multiple functions for the same path", path: "/path/to/multiple/functions", @@ -287,7 +287,7 @@ func (suite *SafeTrieTestSuite) TestPathTreeDelete() { }, deleteArgs: safeTrieFunctionArgs{"/path/to/function1", "test-function1"}, expectedResult: map[string]FunctionTarget{ - "/path/to/function1/nested": &SingleTarget{"test-function2"}, + "/path/to/function1/nested": SingleTarget("test-function2"), }, }, { name: "delete a function from multiple values and validate that the other function is still there", @@ -297,7 +297,7 @@ func (suite *SafeTrieTestSuite) TestPathTreeDelete() { }, deleteArgs: safeTrieFunctionArgs{"/path/to/multiple/functions", "test-function1"}, expectedResult: map[string]FunctionTarget{ - "/path/to/multiple/functions": &SingleTarget{"test-function2"}, + "/path/to/multiple/functions": SingleTarget("test-function2"), }, }, { name: "delete function that does not exist in the path", @@ -306,7 +306,7 @@ func (suite *SafeTrieTestSuite) TestPathTreeDelete() { }, deleteArgs: safeTrieFunctionArgs{"/path/to/function1", "test-function2"}, expectedResult: map[string]FunctionTarget{ - "/path/to/function1": &SingleTarget{"test-function1"}, + "/path/to/function1": SingleTarget("test-function1"), }, }, { name: "delete function that does not exist in multiple value path", @@ -325,7 +325,7 @@ func (suite *SafeTrieTestSuite) TestPathTreeDelete() { }, deleteArgs: safeTrieFunctionArgs{"/path/to/function1/nested", "test-function2"}, expectedResult: map[string]FunctionTarget{ - "/path/to/function1": &SingleTarget{"test-function1"}, + "/path/to/function1": SingleTarget("test-function1"), }, }, { name: "delete path with suffix that does not exist", @@ -335,8 +335,8 @@ func (suite *SafeTrieTestSuite) TestPathTreeDelete() { }, deleteArgs: safeTrieFunctionArgs{"/path/to/function1/path/suffix", "test-function1"}, expectedResult: map[string]FunctionTarget{ - "/path/to/function1": &SingleTarget{"test-function1"}, - "/path/to/function1/nested": &SingleTarget{"test-function2"}, + "/path/to/function1": SingleTarget("test-function1"), + "/path/to/function1/nested": SingleTarget("test-function2"), }, }, } { @@ -425,7 +425,7 @@ func (suite *SafeTrieTestSuite) generateExpectedResultMap(num int) map[string]Fu expectedResult := make(map[string]FunctionTarget) args := suite.generatePathsAndFunctions(num) for i := 0; i < num; i++ { - expectedResult[args[i].path] = &SingleTarget{args[i].function} + expectedResult[args[i].path] = SingleTarget(args[i].function) } return expectedResult } @@ -480,7 +480,7 @@ func (suite *SingleTargetTestSuite) TestContains() { for _, testCase := range testCases { suite.Run(testCase.name, func() { - testSingleFunctionName := &SingleTarget{functionName: "myFunction"} + testSingleFunctionName := SingleTarget("myFunction") result := testSingleFunctionName.Contains(testCase.functionName) suite.Equal(testCase.expectedResult, result) }) @@ -505,13 +505,13 @@ func (suite *SingleTargetTestSuite) TestRemoveFunctionName() { { name: "RemoveNonExistingFunction", functionName: "otherFunction", - expectedResult: &SingleTarget{functionName: "test-function1"}, + expectedResult: SingleTarget("test-function1"), }, } for _, testCase := range testCases { suite.Run(testCase.name, func() { - testSingleFunctionName := &SingleTarget{functionName: "test-function1"} + testSingleFunctionName := SingleTarget("test-function1") result, err := testSingleFunctionName.Delete(testCase.functionName) if testCase.expectError { suite.Require().Error(err) @@ -533,7 +533,7 @@ func (suite *SingleTargetTestSuite) TestAddFunctionName() { { name: "Add same function name", functionName: "test-function1", - expectedResult: &SingleTarget{functionName: "test-function1"}, + expectedResult: SingleTarget("test-function1"), }, { name: "Add function name", functionName: "test-function2", @@ -543,7 +543,7 @@ func (suite *SingleTargetTestSuite) TestAddFunctionName() { for _, testCase := range testCases { suite.Run(testCase.name, func() { - testSingleFunctionName := &SingleTarget{functionName: "test-function1"} + testSingleFunctionName := SingleTarget("test-function1") result, err := testSingleFunctionName.Add(testCase.functionName) suite.Require().NoError(err) suite.Require().NotNil(result) @@ -571,7 +571,7 @@ func (suite *SingleTargetTestSuite) TestToSliceString() { for _, testCase := range testCases { suite.Run(testCase.name, func() { - testSingleFunctionName := &SingleTarget{functionName: testCase.singleFunctionName} + testSingleFunctionName := SingleTarget(testCase.singleFunctionName) result := testSingleFunctionName.ToSliceString() suite.Equal(testCase.expectedResult, result) suite.Len(result, 1) @@ -582,11 +582,11 @@ func (suite *SingleTargetTestSuite) TestToSliceString() { func (suite *SingleTargetTestSuite) TestIsSingleFunctionName() { testCases := []struct { name string - singleFunctionName *SingleTarget + singleFunctionName SingleTarget }{ { name: "IsSingleFunctionNameTrue", - singleFunctionName: &SingleTarget{functionName: "isSingleFunctionNameFunction"}, + singleFunctionName: SingleTarget("isSingleFunctionNameFunction"), }, } @@ -651,7 +651,7 @@ func (suite *CanaryTargetTestSuite) TestRemoveFunctionName() { { name: "RemoveExistingFunction", functionName: "test-function1", - expectedResult: &SingleTarget{functionName: "test-function2"}, + expectedResult: SingleTarget("test-function2"), }, { name: "RemoveNotExistingFunction", functionName: "test-function3",