[TT-12519], added necessary logic for UrlVersioningPattern

andrei-tyk commented 1 month ago

User description

Added necessary logic for UrlVersioningPattern to allow matching of version patterns using regex.

Description

Related Issue

Motivation and Context

How This Has Been Tested

Screenshots (if appropriate)

Types of changes

[ ] Bug fix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to change)
[ ] Refactoring or add test (improvements in base code or adds test coverage to functionality)

Checklist

[ ] I ensured that the documentation is up to date
[ ] I explained why this PR updates go.mod in detail with reasoning why it's required
[ ] I would like a code coverage CI quality gate exception and have explained why

PR Type

Enhancement, Tests

Description

Added UrlVersioningPattern field to VersionDefinition and Versioning structs to support regex-based URL versioning.
Enhanced field documentation in VersionDefinition struct.
Implemented logic in getVersionFromRequest to handle UrlVersioningPattern and added error handling for regex compilation.
Added unit tests for version routing with UrlVersioningPattern.
Updated JSON schema to include urlVersioningPattern field.

Changes walkthrough 📝

Relevant files

Enhancement

api_definitions.go `Add UrlVersioningPattern field to VersionDefinition struct` apidef/api_definitions.go Added `UrlVersioningPattern` field to `VersionDefinition` struct. Enhanced documentation for `VersionDefinition` fields.	+33/-10
root.go `Add UrlVersioningPattern field to Versioning struct` apidef/oas/root.go Added `UrlVersioningPattern` field to `Versioning` struct. Updated `Fill` and `ExtractTo` methods to handle `UrlVersioningPattern`.	+4/-0
api_definition.go `Implement UrlVersioningPattern handling in getVersionFromRequest` gateway/api_definition.go Implemented logic to handle `UrlVersioningPattern` in `getVersionFromRequest`. Added error handling for regex compilation.	+12/-1
x-tyk-api-gateway.json `Add urlVersioningPattern field to JSON schema` apidef/oas/schema/x-tyk-api-gateway.json - Added `urlVersioningPattern` field to the JSON schema.	+3/-0

Tests

proxy.go `Add placeholder package for proxy tests` tests/proxy/proxy.go - Added a placeholder package for proxy tests.	+4/-0
version_routing_test.go `Add unit tests for version routing with UrlVersioningPattern` tests/proxy/version_routing_test.go Added unit tests for version routing with `UrlVersioningPattern`. Tested version matching and fallback behavior.	+97/-0

💡 PR-Agent usage: Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

github-actions[bot] commented 1 month ago

API Changes

--- prev.txt    2024-07-16 16:36:14.869255560 +0000
+++ current.txt 2024-07-16 16:36:11.793268699 +0000
@@ -2021,17 +2021,41 @@
 }

 type VersionDefinition struct {
-   Enabled             bool              `bson:"enabled" json:"enabled"`
-   Name                string            `bson:"name" json:"name"`
-   Default             string            `bson:"default" json:"default"`
-   Location            string            `bson:"location" json:"location"`
-   Key                 string            `bson:"key" json:"key"`
-   StripPath           bool              `bson:"strip_path" json:"strip_path"` // Deprecated. Use StripVersioningData instead.
-   StripVersioningData bool              `bson:"strip_versioning_data" json:"strip_versioning_data"`
-   FallbackToDefault   bool              `bson:"fallback_to_default" json:"fallback_to_default"`
-   Versions            map[string]string `bson:"versions" json:"versions"`
-   BaseID              string            `bson:"base_id" json:"-"` // json tag is `-` because we want this to be hidden to user
+   // Enabled indicates whether this version is enabled or not.
+   Enabled bool `bson:"enabled" json:"enabled"`
+
+   // Name is the name of this version.
+   Name string `bson:"name" json:"name"`
+
+   // Default is the default version to use if no version is specified in the request.
+   Default string `bson:"default" json:"default"`
+
+   // Location is the location in the request where the version information can be found.
+   Location string `bson:"location" json:"location"`
+
+   // Key is the key to use to extract the version information from the specified location.
+   Key string `bson:"key" json:"key"`
+
+   // StripPath is a deprecated field. Use StripVersioningData instead.
+   StripPath bool `bson:"strip_path" json:"strip_path"` // Deprecated. Use StripVersioningData instead.
+
+   // StripVersioningData indicates whether to strip the versioning data from the request.
+   StripVersioningData bool `bson:"strip_versioning_data" json:"strip_versioning_data"`
+
+   // UrlVersioningPattern is the regex pattern to match in the URL for versioning.
+   UrlVersioningPattern string `bson:"url_versioning_pattern" json:"url_versioning_pattern"`
+
+   // FallbackToDefault indicates whether to fallback to the default version if the version in the request does not exist.
+   FallbackToDefault bool `bson:"fallback_to_default" json:"fallback_to_default"`
+
+   // Versions is a map of version names to version ApiIDs.
+   Versions map[string]string `bson:"versions" json:"versions"`
+
+   // BaseID is a hidden field used internally that represents the ApiID of the base API.
+   BaseID string `bson:"base_id" json:"-"` // json tag is `-` because we want this to be hidden to user
 }
+    VersionDefinition is a struct that holds the versioning information for an
+    API.

 type VersionInfo struct {
    Name      string    `bson:"name" json:"name"`
@@ -4719,6 +4743,8 @@
    Versions []VersionToID `bson:"versions" json:"versions"` // required
    // StripVersioningData is a boolean flag, if set to `true`, the API responses will be stripped of versioning data.
    StripVersioningData bool `bson:"stripVersioningData,omitempty" json:"stripVersioningData,omitempty"`
+   // UrlVersioningPattern is a string that contains the pattern that if matched will remove the version from the URL.
+   UrlVersioningPattern string `bson:"urlVersioningPattern,omitempty" json:"urlVersioningPattern,omitempty"`
    // FallbackToDefault controls the behaviour of Tyk when a versioned API is called with a nonexistent version name.
    // If set to `true` then the default API version will be invoked; if set to `false` Tyk will return an HTTP 404
    // `This API version does not seem to exist` error in this scenario.
@@ -12091,6 +12117,10 @@
     manipulate

 func MyPluginReturningError(rw http.ResponseWriter, r *http.Request)
+# Package: ./tests/proxy
+
+package proxy // import "github.com/TykTechnologies/tyk/tests/proxy"
+
 # Package: ./tests/rate

 # Package: ./tests/regression

github-actions[bot] commented 1 month ago

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Key issues to review

**Regex Performance:** Ensure that the regex pattern used in `UrlVersioningPattern` is optimized for performance to avoid potential slowdowns in request processing. **Error Handling:** Consider adding more robust error handling for regex compilation failures in `gateway/api_definition.go`. Currently, it logs an error but does not prevent further processing which might lead to unexpected behaviors.

github-actions[bot] commented 1 month ago

PR Code Suggestions ✨

Category	Suggestion	Score
Performance	Move regex compilation outside the loop to enhance performance ___ To optimize the regex pattern compilation, move the `regexp.Compile` outside the loop to avoid recompilation for each URL part. This change will enhance performance, especially for APIs with multiple URL parts. [gateway/api_definition.go [1733-1737]](https://github.com/TykTechnologies/tyk/pull/6401/files#diff-0cf80174bbafb36f6d4f4308ebbd971b2833b76a936bad568220aa1a4ba0ee8bR1733-R1737) ```diff +// Define the regex outside the loop re, err := regexp.Compile(a.VersionDefinition.UrlVersioningPattern) if err != nil { log.Error("Error compiling versioning pattern: ", err) -} else { - matchesUrlVersioningPattern = re.Match([]byte(part)) + return "" } +for _, part := range strings.Split(uPath, "/") { + if part != "" { + matchesUrlVersioningPattern := re.Match([]byte(part)) ``` Suggestion importance[1-10]: 9 Why: The suggestion correctly identifies a performance improvement by moving the regex compilation outside the loop, which avoids unnecessary recompilation for each URL part. This is a significant optimization for APIs handling multiple URL parts.	9