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/ingresscache_test.go b/pkg/ingresscache/ingresscache_test.go index 663238bf..bcc4b3eb 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.go b/pkg/ingresscache/safetrie.go index 79248201..2a07031f 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" @@ -41,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") } @@ -57,28 +56,32 @@ 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 the function already exists at this path, skip adding it to prevent duplicates + if pathFunctionNames.Contains(function) { + // 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 } - 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 } -// 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 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() @@ -88,27 +91,30 @@ 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 } -// 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") @@ -127,9 +133,9 @@ 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 @@ -146,25 +152,75 @@ 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 string + +func (s SingleTarget) Contains(functionName string) bool { + return string(s) == 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{string(s), functionName}}, nil +} + +func (s SingleTarget) ToSliceString() []string { + return []string{string(s)} +} + +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(c.functionNames[1]), nil + } + + if c.functionNames[1] == functionName { + return SingleTarget(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/safetrie_test.go b/pkg/ingresscache/safetrie_test.go index 1fcf0e04..bb7420d4 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,34 +158,34 @@ 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() { 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) @@ -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,16 +334,16 @@ 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"), }, }, } { 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) @@ -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 } @@ -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) } @@ -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("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("test-function1"), + }, + } + + for _, testCase := range testCases { + suite.Run(testCase.name, func() { + testSingleFunctionName := SingleTarget("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("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("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(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("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("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)) +} diff --git a/pkg/ingresscache/types.go b/pkg/ingresscache/types.go index 6f66340f..50980e91 100644 --- a/pkg/ingresscache/types.go +++ b/pkg/ingresscache/types.go @@ -32,15 +32,33 @@ 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 } + +// 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 +}