From 741c145d293f124bd55d5b6c4239fd84481037f3 Mon Sep 17 00:00:00 2001 From: Matthias Bertschy Date: Thu, 19 Sep 2024 15:20:20 +0200 Subject: [PATCH 1/4] use single char for dynamic, add comparison func Signed-off-by: Matthias Bertschy --- .../file/dynamicpathdetector/analyzer.go | 31 +++++++ .../tests/analyze_endpoints_test.go | 20 ++-- .../tests/analyze_opens_test.go | 10 +- .../tests/benchmark_test.go | 9 ++ .../tests/coverage_test.go | 91 +++++++++++++++++-- .../file/dynamicpathdetector/types.go | 2 +- 6 files changed, 140 insertions(+), 23 deletions(-) diff --git a/pkg/registry/file/dynamicpathdetector/analyzer.go b/pkg/registry/file/dynamicpathdetector/analyzer.go index 114649c6..0a886768 100644 --- a/pkg/registry/file/dynamicpathdetector/analyzer.go +++ b/pkg/registry/file/dynamicpathdetector/analyzer.go @@ -136,3 +136,34 @@ func shallowChildrenCopy(src, dst *SegmentNode) { } } } + +func CompareDynamic(dynamicPath, regularPath string) bool { + dynamicIndex, regularIndex := 0, 0 + dynamicLen, regularLen := len(dynamicPath), len(regularPath) + + for dynamicIndex < dynamicLen && regularIndex < regularLen { + // Find the next segment in dynamicPath + dynamicSegmentStart := dynamicIndex + for dynamicIndex < dynamicLen && dynamicPath[dynamicIndex] != '/' { + dynamicIndex++ + } + dynamicSegment := dynamicPath[dynamicSegmentStart:dynamicIndex] + + // Find the next segment in regularPath + regularSegmentStart := regularIndex + for regularIndex < regularLen && regularPath[regularIndex] != '/' { + regularIndex++ + } + regularSegment := regularPath[regularSegmentStart:regularIndex] + + if dynamicSegment != DynamicIdentifier && dynamicSegment != regularSegment { + return false + } + + // Move to the next segment + dynamicIndex++ + regularIndex++ + } + + return true +} diff --git a/pkg/registry/file/dynamicpathdetector/tests/analyze_endpoints_test.go b/pkg/registry/file/dynamicpathdetector/tests/analyze_endpoints_test.go index 2225c882..38eea1f8 100644 --- a/pkg/registry/file/dynamicpathdetector/tests/analyze_endpoints_test.go +++ b/pkg/registry/file/dynamicpathdetector/tests/analyze_endpoints_test.go @@ -38,7 +38,7 @@ func TestAnalyzeEndpoints(t *testing.T) { name: "Test with multiple endpoints", input: []types.HTTPEndpoint{ { - Endpoint: ":80/users/", + Endpoint: ":80/users/\u22ef", Methods: []string{"GET"}, }, { @@ -48,7 +48,7 @@ func TestAnalyzeEndpoints(t *testing.T) { }, expected: []types.HTTPEndpoint{ { - Endpoint: ":80/users/", + Endpoint: ":80/users/\u22ef", Methods: []string{"GET", "POST"}, }, }, @@ -57,17 +57,17 @@ func TestAnalyzeEndpoints(t *testing.T) { name: "Test with dynamic segments", input: []types.HTTPEndpoint{ { - Endpoint: ":80/users/123/posts/", + Endpoint: ":80/users/123/posts/\u22ef", Methods: []string{"GET"}, }, { - Endpoint: ":80/users//posts/101", + Endpoint: ":80/users/\u22ef/posts/101", Methods: []string{"POST"}, }, }, expected: []types.HTTPEndpoint{ { - Endpoint: ":80/users//posts/", + Endpoint: ":80/users/\u22ef/posts/\u22ef", Methods: []string{"GET", "POST"}, }, }, @@ -111,19 +111,19 @@ func TestAnalyzeEndpoints(t *testing.T) { name: "Test with dynamic segments and different headers", input: []types.HTTPEndpoint{ { - Endpoint: ":80/x/123/posts/", + Endpoint: ":80/x/123/posts/\u22ef", Methods: []string{"GET"}, Headers: json.RawMessage(`{"Content-Type": ["application/json"], "X-API-Key": ["key1"]}`), }, { - Endpoint: ":80/x//posts/101", + Endpoint: ":80/x/\u22ef/posts/101", Methods: []string{"POST"}, Headers: json.RawMessage(`{"Content-Type": ["application/xml"], "Authorization": ["Bearer token"]}`), }, }, expected: []types.HTTPEndpoint{ { - Endpoint: ":80/x//posts/", + Endpoint: ":80/x/\u22ef/posts/\u22ef", Methods: []string{"GET", "POST"}, Headers: json.RawMessage(`{"Authorization":["Bearer token"],"Content-Type":["<>","application/json","application/xml"],"X-API-Key":["key1"]}`), }, @@ -158,7 +158,7 @@ func TestAnalyzeEndpointsWithThreshold(t *testing.T) { expected := []types.HTTPEndpoint{ { - Endpoint: ":80/users/", + Endpoint: ":80/users/\u22ef", Methods: []string{"GET"}, }, } @@ -197,7 +197,7 @@ func TestAnalyzeEndpointsWithExactThreshold(t *testing.T) { // Check that all endpoints are now merged into one dynamic endpoint expected := []types.HTTPEndpoint{ { - Endpoint: ":80/users/", + Endpoint: ":80/users/\u22ef", Methods: []string{"GET"}, }, } diff --git a/pkg/registry/file/dynamicpathdetector/tests/analyze_opens_test.go b/pkg/registry/file/dynamicpathdetector/tests/analyze_opens_test.go index c4c4288f..a68e0104 100644 --- a/pkg/registry/file/dynamicpathdetector/tests/analyze_opens_test.go +++ b/pkg/registry/file/dynamicpathdetector/tests/analyze_opens_test.go @@ -21,7 +21,7 @@ func TestAnalyzeOpensWithThreshold(t *testing.T) { expected := []types.OpenCalls{ { - Path: "/home//file.txt", + Path: "/home/\u22ef/file.txt", Flags: []string{}, }, } @@ -46,7 +46,7 @@ func TestAnalyzeOpensWithFlagMergingAndThreshold(t *testing.T) { {Path: "/home/user4/file.txt", Flags: []string{"READ", "WRITE"}}, }, expected: []types.OpenCalls{ - {Path: "/home//file.txt", Flags: []string{"APPEND", "READ", "WRITE"}}, + {Path: "/home/\u22ef/file.txt", Flags: []string{"APPEND", "READ", "WRITE"}}, }, }, { @@ -71,7 +71,7 @@ func TestAnalyzeOpensWithFlagMergingAndThreshold(t *testing.T) { {Path: "/var/log/app2.log", Flags: []string{"WRITE"}}, }, expected: []types.OpenCalls{ - {Path: "/home//common.txt", Flags: []string{"APPEND", "READ", "WRITE"}}, + {Path: "/home/\u22ef/common.txt", Flags: []string{"APPEND", "READ", "WRITE"}}, {Path: "/var/log/app1.log", Flags: []string{"READ"}}, {Path: "/var/log/app2.log", Flags: []string{"WRITE"}}, }, @@ -89,8 +89,8 @@ func TestAnalyzeOpensWithFlagMergingAndThreshold(t *testing.T) { {Path: "/home/user4/file2.txt", Flags: []string{"READ", "WRITE"}}, }, expected: []types.OpenCalls{ - {Path: "/home//file1.txt", Flags: []string{"APPEND", "READ", "WRITE"}}, - {Path: "/home//file2.txt", Flags: []string{"APPEND", "READ", "WRITE"}}, + {Path: "/home/\u22ef/file1.txt", Flags: []string{"APPEND", "READ", "WRITE"}}, + {Path: "/home/\u22ef/file2.txt", Flags: []string{"APPEND", "READ", "WRITE"}}, }, }, } diff --git a/pkg/registry/file/dynamicpathdetector/tests/benchmark_test.go b/pkg/registry/file/dynamicpathdetector/tests/benchmark_test.go index 9532dc83..68e6ec7f 100644 --- a/pkg/registry/file/dynamicpathdetector/tests/benchmark_test.go +++ b/pkg/registry/file/dynamicpathdetector/tests/benchmark_test.go @@ -71,6 +71,15 @@ func BenchmarkAnalyzeOpensVsDeflateStringer(b *testing.B) { }) } +func BenchmarkCompareDynamic(b *testing.B) { + dynamicPath := "/api/\u22ef/\u22ef" + regularPath := "/api/users/123" + for i := 0; i < b.N; i++ { + _ = dynamicpathdetector.CompareDynamic(dynamicPath, regularPath) + } + b.ReportAllocs() +} + func generateMixedPaths(count int, fixedLength int) []string { paths := make([]string, count) staticSegments := []string{"users", "profile", "settings", "api", "v1", "posts", "organizations", "departments", "employees", "projects", "tasks", "categories", "subcategories", "items", "articles"} diff --git a/pkg/registry/file/dynamicpathdetector/tests/coverage_test.go b/pkg/registry/file/dynamicpathdetector/tests/coverage_test.go index fd6f7483..8d21159e 100644 --- a/pkg/registry/file/dynamicpathdetector/tests/coverage_test.go +++ b/pkg/registry/file/dynamicpathdetector/tests/coverage_test.go @@ -53,7 +53,7 @@ func TestDynamicSegments(t *testing.T) { if err != nil { t.Errorf("AnalyzePath() returned an error: %v", err) } - expected := "/api/users/" + expected := "/api/users/\u22ef" assert.Equal(t, expected, result) // Test with one of the original IDs to ensure it's also marked as dynamic @@ -77,7 +77,7 @@ func TestMultipleDynamicSegments(t *testing.T) { // Test with the 100th unique user and post IDs (should trigger dynamic segments) result, err := analyzer.AnalyzePath("/api/users/101/posts/1031", "api") assert.NoError(t, err) - expected := "/api/users//posts/" + expected := "/api/users/\u22ef/posts/\u22ef" assert.Equal(t, expected, result) } @@ -96,7 +96,7 @@ func TestMixedStaticAndDynamicSegments(t *testing.T) { // Test with the 100th unique user ID but same 'posts' segment (should trigger dynamic segment for users) result, err := analyzer.AnalyzePath("/api/users/99/posts", "api") assert.NoError(t, err) - expected := "/api/users//posts" + expected := "/api/users/\u22ef/posts" assert.Equal(t, expected, result) } @@ -124,7 +124,7 @@ func TestDynamicThreshold(t *testing.T) { } result, _ := analyzer.AnalyzePath("/api/users/991", "api") - assert.Equal(t, "/api/users/", result) + assert.Equal(t, "/api/users/\u22ef", result) } func TestEdgeCases(t *testing.T) { @@ -154,14 +154,91 @@ func TestDynamicInsertion(t *testing.T) { analyzer := dynamicpathdetector.NewPathAnalyzer(100) // Insert a new path with a different identifier - result, err := analyzer.AnalyzePath("/api/users/", "api") + result, err := analyzer.AnalyzePath("/api/users/\u22ef", "api") assert.NoError(t, err) - expected := "/api/users/" + expected := "/api/users/\u22ef" assert.Equal(t, expected, result) // Insert a new path with the same identifier result, err = analyzer.AnalyzePath("/api/users/102", "api") assert.NoError(t, err) - expected = "/api/users/" + expected = "/api/users/\u22ef" assert.Equal(t, expected, result) } + +func TestCompareDynamic(t *testing.T) { + tests := []struct { + name string + dynamicPath string + regularPath string + want bool + }{ + { + name: "Equal paths", + dynamicPath: "/api/users/123", + regularPath: "/api/users/123", + want: true, + }, + { + name: "Different paths", + dynamicPath: "/api/users/123", + regularPath: "/api/users/456", + want: false, + }, + { + name: "Dynamic segment at the end", + dynamicPath: "/api/users/\u22ef", + regularPath: "/api/users/123", + want: true, + }, + { + name: "Dynamic segment at the end, no match", + dynamicPath: "/api/users/\u22ef", + regularPath: "/api/apps/123", + want: false, + }, + { + name: "Dynamic segment in the middle", + dynamicPath: "/api/\u22ef/123", + regularPath: "/api/users/123", + want: true, + }, + { + name: "Dynamic segment in the middle, no match", + dynamicPath: "/api/\u22ef/123", + regularPath: "/api/users/456", + want: false, + }, + { + name: "2 dynamic segments", + dynamicPath: "/api/\u22ef/\u22ef", + regularPath: "/api/users/123", + want: true, + }, + { + name: "2 dynamic segments, no match", + dynamicPath: "/api/\u22ef/\u22ef", + regularPath: "/papi/users/456", + want: false, + }, + { + name: "2 other dynamic segments", + dynamicPath: "/\u22ef/users/\u22ef", + regularPath: "/api/users/123", + want: true, + }, + { + name: "2 other dynamic segments, no match", + dynamicPath: "/\u22ef/users/\u22ef", + regularPath: "/api/apps/456", + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := dynamicpathdetector.CompareDynamic(tt.dynamicPath, tt.regularPath); got != tt.want { + t.Errorf("CompareDynamic() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/pkg/registry/file/dynamicpathdetector/types.go b/pkg/registry/file/dynamicpathdetector/types.go index 3c5ff9da..1bd2d52f 100644 --- a/pkg/registry/file/dynamicpathdetector/types.go +++ b/pkg/registry/file/dynamicpathdetector/types.go @@ -1,6 +1,6 @@ package dynamicpathdetector -const DynamicIdentifier string = "" +const DynamicIdentifier string = "\u22ef" type SegmentNode struct { SegmentName string From 05b3fa941aaa6caa6c41e2aa791cb053b16e51e5 Mon Sep 17 00:00:00 2001 From: Matthias Bertschy Date: Thu, 19 Sep 2024 15:46:43 +0200 Subject: [PATCH 2/4] rewrite processSegments with the same logic Signed-off-by: Matthias Bertschy --- .../file/dynamicpathdetector/analyzer.go | 24 +++++++++---------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/pkg/registry/file/dynamicpathdetector/analyzer.go b/pkg/registry/file/dynamicpathdetector/analyzer.go index 0a886768..a6f349f8 100644 --- a/pkg/registry/file/dynamicpathdetector/analyzer.go +++ b/pkg/registry/file/dynamicpathdetector/analyzer.go @@ -29,23 +29,21 @@ func (ua *PathAnalyzer) AnalyzePath(p, identifier string) (string, error) { func (ua *PathAnalyzer) processSegments(node *SegmentNode, p string) string { var result strings.Builder currentNode := node - start := 0 - for i := range p { - if p[i] == '/' { - segment := p[start:i] - currentNode = ua.processSegment(currentNode, segment) - ua.updateNodeStats(currentNode) - result.WriteString(currentNode.SegmentName) - result.WriteByte('/') - start = i + 1 + i := 0 + for { + start := i + for i < len(p) && p[i] != '/' { + i++ } - } - // Process the last segment - if start < len(p) { - segment := p[start:] + segment := p[start:i] currentNode = ua.processSegment(currentNode, segment) ua.updateNodeStats(currentNode) result.WriteString(currentNode.SegmentName) + i++ + if len(p) < i { + break + } + result.WriteByte('/') } return result.String() } From 57e263c741eb98a184ad5cc0461ec5ddac3fb107 Mon Sep 17 00:00:00 2001 From: Matthias Bertschy Date: Thu, 19 Sep 2024 16:29:47 +0200 Subject: [PATCH 3/4] fix CompareDynamic when one path is shorter Signed-off-by: Matthias Bertschy --- pkg/registry/file/dynamicpathdetector/analyzer.go | 2 +- .../file/dynamicpathdetector/tests/coverage_test.go | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/pkg/registry/file/dynamicpathdetector/analyzer.go b/pkg/registry/file/dynamicpathdetector/analyzer.go index a6f349f8..4506b49f 100644 --- a/pkg/registry/file/dynamicpathdetector/analyzer.go +++ b/pkg/registry/file/dynamicpathdetector/analyzer.go @@ -163,5 +163,5 @@ func CompareDynamic(dynamicPath, regularPath string) bool { regularIndex++ } - return true + return dynamicIndex > dynamicLen && regularIndex > regularLen } diff --git a/pkg/registry/file/dynamicpathdetector/tests/coverage_test.go b/pkg/registry/file/dynamicpathdetector/tests/coverage_test.go index 8d21159e..8f05b960 100644 --- a/pkg/registry/file/dynamicpathdetector/tests/coverage_test.go +++ b/pkg/registry/file/dynamicpathdetector/tests/coverage_test.go @@ -191,6 +191,12 @@ func TestCompareDynamic(t *testing.T) { regularPath: "/api/users/123", want: true, }, + { + name: "Dynamic segment at the end", + dynamicPath: "/api/users/\u22ef", + regularPath: "/api/users/123/posts", + want: false, + }, { name: "Dynamic segment at the end, no match", dynamicPath: "/api/users/\u22ef", From 1ec96eacdef6b8d55dafda3f2163d784180c0378 Mon Sep 17 00:00:00 2001 From: Afek Berger Date: Thu, 19 Sep 2024 17:56:08 +0300 Subject: [PATCH 4/4] Fixed analyzer bug --- .../file/dynamicpathdetector/analyzer.go | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/pkg/registry/file/dynamicpathdetector/analyzer.go b/pkg/registry/file/dynamicpathdetector/analyzer.go index 4506b49f..1f17d80a 100644 --- a/pkg/registry/file/dynamicpathdetector/analyzer.go +++ b/pkg/registry/file/dynamicpathdetector/analyzer.go @@ -51,18 +51,17 @@ func (ua *PathAnalyzer) processSegments(node *SegmentNode, p string) string { func (ua *PathAnalyzer) processSegment(node *SegmentNode, segment string) *SegmentNode { if segment == DynamicIdentifier { return ua.handleDynamicSegment(node) - } else if child, exists := node.Children[segment]; exists || node.IsNextDynamic() { - return ua.handleExistingSegment(node, child, exists) - } else { - return ua.handleNewSegment(node, segment) - } -} - -func (ua *PathAnalyzer) handleExistingSegment(node *SegmentNode, child *SegmentNode, exists bool) *SegmentNode { - if exists { + } else if node.IsNextDynamic() { + if len(node.Children) > 1 { + temp := node.Children[DynamicIdentifier] + node.Children = map[string]*SegmentNode{} + node.Children[DynamicIdentifier] = temp + } + return node.Children[DynamicIdentifier] + } else if child, exists := node.Children[segment]; exists { return child } else { - return node.Children[DynamicIdentifier] + return ua.handleNewSegment(node, segment) } }