Skip to content

Commit 441eaea

Browse files
edsonmichaqueTyk Bot
authored and
Tyk Bot
committed
[TT-7815] ensure path params are migrated to OAS (#6966)
### **User description** <details open> <summary><a href="https://tyktech.atlassian.net/browse/TT-7815" title="TT-7815" target="_blank">TT-7815</a></summary> <br /> <table> <tr> <th>Summary</th> <td>Cannot migrate API with endpoints containing path parameter </td> </tr> <tr> <th>Type</th> <td> <img alt="Bug" src="https://tyktech.atlassian.net/rest/api/2/universal_avatar/view/type/issuetype/avatar/10303?size=medium" /> Bug </td> </tr> <tr> <th>Status</th> <td>In Dev</td> </tr> <tr> <th>Points</th> <td>N/A</td> </tr> <tr> <th>Labels</th> <td><a href="https://tyktech.atlassian.net/issues?jql=project%20%3D%20TT%20AND%20labels%20%3D%20DoD_Fail%20ORDER%20BY%20created%20DESC" title="DoD_Fail">DoD_Fail</a>, <a href="https://tyktech.atlassian.net/issues?jql=project%20%3D%20TT%20AND%20labels%20%3D%20QA_Fail%20ORDER%20BY%20created%20DESC" title="QA_Fail">QA_Fail</a>, <a href="https://tyktech.atlassian.net/issues?jql=project%20%3D%20TT%20AND%20labels%20%3D%20Re_open%20ORDER%20BY%20created%20DESC" title="Re_open">Re_open</a></td> </tr> </table> </details> <!-- do not remove this marker as it will break jira-lint's functionality. added_by_jira_lint --> --- <!-- Provide a general summary of your changes in the Title above --> ## Description <!-- Describe your changes in detail --> This PR makes sure that path params are successfully migrated from Classic to OAS ## Related Issue <!-- This project only accepts pull requests related to open issues. --> <!-- If suggesting a new feature or change, please discuss it in an issue first. --> <!-- If fixing a bug, there should be an issue describing it with steps to reproduce. --> <!-- OSS: Please link to the issue here. Tyk: please create/link the JIRA ticket. --> https://tyktech.atlassian.net/browse/TT-7815 ## Motivation and Context <!-- Why is this change required? What problem does it solve? --> ## How This Has Been Tested <!-- Please describe in detail how you tested your changes --> <!-- Include details of your testing environment, and the tests --> <!-- you ran to see how your change affects other areas of the code, etc. --> <!-- This information is helpful for reviewers and QA. --> ## Screenshots (if appropriate) ## Types of changes <!-- What types of changes does your code introduce? Put an `x` in all the boxes that apply: --> - [ ] 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 <!-- Go over all the following points, and put an `x` in all the boxes that apply --> <!-- If there are no documentation updates required, mark the item as checked. --> <!-- Raise up any additional concerns not covered by the 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** - Bug fix - Enhancement - Tests ___ ### **Description** - Refactored path splitting logic for OAS conversion. - Introduced helper functions for regex and mux template parsing. - Added unit tests covering various path parameter scenarios. - Provided test fixtures for classic to OAS migration. ___ ### **Changes walkthrough** 📝 <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td><strong>Enhancement</strong></td><td><table> <tr> <td> <details> <summary><strong>operation.go</strong><dd><code>Enhance path parameter migration in OAS operations.</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></summary> <hr> apidef/oas/operation.go <li>Added import for regexp and httputil.<br> <li> Refactored splitPath with empty path check.<br> <li> Introduced parsePathSegment, parseMuxTemplate, and isIdentifier.<br> <li> Improved regex detection and parameter naming. </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6966/files#diff-6d92d2d5b09a5fa7129609bb7cd0d383d015250ec07062b6a93a83257be51fb5">+49/-18</a>&nbsp; </td> </tr> </table></td></tr><tr><td><strong>Tests</strong></td><td><table> <tr> <td> <details> <summary><strong>operation_test.go</strong><dd><code>Add unit tests for splitPath functionality.</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></summary> <hr> apidef/oas/operation_test.go <li>Added TestSplitPath covering diverse scenarios.<br> <li> Verified correct parsing for simple, regex, and mux templates.<br> <li> Ensured empty and root paths are handled. </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6966/files#diff-cd234db716d6d2edc97c135ef546021c9ab4fa9282d63964bd155d41635cf964">+72/-0</a>&nbsp; &nbsp; </td> </tr> <tr> <td> <details> <summary><strong>path_params.yml</strong><dd><code>Add path params test fixture for OAS migration.</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></summary> <hr> apidef/oas/testdata/fixtures/path_params.yml <li>Created YAML fixtures for classic path parameter migration.<br> <li> Defined multiple test cases with varied input patterns.<br> <li> Mapped expected outputs for both simple and regex parameters. </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6966/files#diff-0368200f5970a6c4e9bbfa2bb67a2af7568412926cf37d42a65579ef9bea4570">+144/-0</a>&nbsp; </td> </tr> </table></td></tr></tr></tbody></table> ___ > <details> <summary> Need help?</summary><li>Type <code>/help how to ...</code> in the comments thread for any questions about PR-Agent usage.</li><li>Check out the <a href="https://qodo-merge-docs.qodo.ai/usage-guide/">documentation</a> for more information.</li></details> (cherry picked from commit 2deca99)
1 parent 27480a8 commit 441eaea

File tree

3 files changed

+313
-18
lines changed

3 files changed

+313
-18
lines changed

apidef/oas/operation.go

+47-18
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212

1313
"github.com/TykTechnologies/tyk/apidef"
1414
"github.com/TykTechnologies/tyk/internal/oasutil"
15+
"github.com/TykTechnologies/tyk/regexp"
1516
)
1617

1718
// Operations holds Operation definitions.
@@ -577,27 +578,25 @@ func isRegex(value string) bool {
577578

578579
// splitPath splits URL into folder parts, detecting regex patterns.
579580
func splitPath(inPath string) ([]pathPart, bool) {
580-
// Each URL fragment can contain a regex, but the whole
581-
// URL isn't just a regex (`/a/.*/foot` => `/a/{param1}/foot`)
582-
parts := strings.Split(strings.Trim(inPath, "/"), "/")
581+
trimmedPath := strings.Trim(inPath, "/")
582+
583+
if trimmedPath == "" {
584+
return []pathPart{}, false
585+
}
586+
587+
parts := strings.Split(trimmedPath, "/")
583588
result := make([]pathPart, len(parts))
584-
found := 0
585-
586-
for k, value := range parts {
587-
name := value
588-
isRegex := isRegex(value)
589-
if isRegex {
590-
found++
591-
name = fmt.Sprintf("customRegex%d", found)
592-
}
593-
result[k] = pathPart{
594-
name: name,
595-
value: value,
596-
isRegex: isRegex,
597-
}
589+
590+
regexCount := 0
591+
hasRegex := false
592+
593+
for i, segment := range parts {
594+
var part pathPart
595+
part, regexCount, hasRegex = parsePathSegment(segment, regexCount, hasRegex)
596+
result[i] = part
598597
}
599598

600-
return result, found > 0
599+
return result, hasRegex
601600
}
602601

603602
// buildPath converts the URL paths with regex to named parameters
@@ -1050,3 +1049,33 @@ func hasMockResponse(methodActions map[string]apidef.EndpointMethodMeta) bool {
10501049

10511050
return false
10521051
}
1052+
1053+
// parsePathSegment parses a single path segment and determines if it contains a regex pattern.
1054+
func parsePathSegment(segment string, regexCount int, hasRegex bool) (pathPart, int, bool) {
1055+
if strings.HasPrefix(segment, "{") && strings.HasSuffix(segment, "}") {
1056+
return parseMuxTemplate(segment, regexCount)
1057+
} else if isIdentifier(segment) {
1058+
return pathPart{name: segment, value: segment, isRegex: false}, regexCount, hasRegex
1059+
} else {
1060+
regexCount++
1061+
return pathPart{name: fmt.Sprintf("customRegex%d", regexCount), value: segment, isRegex: true}, regexCount, true
1062+
}
1063+
}
1064+
1065+
// parseMuxTemplate parses a segment that is a mux template and extracts the name or assigns a custom regex name.
1066+
func parseMuxTemplate(segment string, regexCount int) (pathPart, int, bool) {
1067+
segment = strings.Trim(segment, "{}")
1068+
1069+
name, _, ok := strings.Cut(segment, ":")
1070+
if ok || isIdentifier(segment) {
1071+
return pathPart{name: name, isRegex: true}, regexCount, true
1072+
}
1073+
1074+
regexCount++
1075+
return pathPart{name: fmt.Sprintf("customRegex%d", regexCount), isRegex: true}, regexCount, true
1076+
}
1077+
1078+
func isIdentifier(value string) bool {
1079+
matched, _ := regexp.MatchString(`^[a-zA-Z0-9._-]+$`, value) //nolint
1080+
return matched
1081+
}

apidef/oas/operation_test.go

+89
Original file line numberDiff line numberDiff line change
@@ -1219,3 +1219,92 @@ func TestOAS_fillAllowance(t *testing.T) {
12191219
assert.False(t, operation.Allow.Enabled)
12201220
})
12211221
}
1222+
1223+
func TestSplitPath(t *testing.T) {
1224+
tests := map[string]struct {
1225+
input string
1226+
wantParts []pathPart
1227+
wantRegex bool
1228+
}{
1229+
"simple path": {
1230+
input: "/test/path",
1231+
wantParts: []pathPart{
1232+
{name: "test", value: "test", isRegex: false},
1233+
{name: "path", value: "path", isRegex: false},
1234+
},
1235+
wantRegex: false,
1236+
},
1237+
"path with regex": {
1238+
input: "/test/.*/end",
1239+
wantParts: []pathPart{
1240+
{name: "test", value: "test", isRegex: false},
1241+
{name: "customRegex1", value: ".*", isRegex: true},
1242+
{name: "end", value: "end", isRegex: false},
1243+
},
1244+
wantRegex: true,
1245+
},
1246+
"path with curly braces": {
1247+
input: "/users/{id}/profile",
1248+
wantParts: []pathPart{
1249+
{name: "users", value: "users", isRegex: false},
1250+
{name: "id", isRegex: true},
1251+
{name: "profile", value: "profile", isRegex: false},
1252+
},
1253+
wantRegex: true,
1254+
},
1255+
"path with named regex": {
1256+
input: "/users/{userId:[0-9]+}/posts",
1257+
wantParts: []pathPart{
1258+
{name: "users", value: "users", isRegex: false},
1259+
{name: "userId", isRegex: true},
1260+
{name: "posts", value: "posts", isRegex: false},
1261+
},
1262+
wantRegex: true,
1263+
},
1264+
"path with named direct regex": {
1265+
input: "/users/[0-9]+/posts",
1266+
wantParts: []pathPart{
1267+
{name: "users", value: "users", isRegex: false},
1268+
{name: "customRegex1", value: "[0-9]+", isRegex: true},
1269+
{name: "posts", value: "posts", isRegex: false},
1270+
},
1271+
wantRegex: true,
1272+
},
1273+
"empty path": {
1274+
input: "",
1275+
wantParts: []pathPart{},
1276+
wantRegex: false,
1277+
},
1278+
"root path": {
1279+
input: "/",
1280+
wantParts: []pathPart{},
1281+
wantRegex: false,
1282+
},
1283+
"path with multiple regexes": {
1284+
input: "/users/{userId:[0-9]+}/posts/{postId:[a-z]+}/[a-z]+/{[0-9]{2}}/[a-z]{10}/abc/{id}/def/[0-9]+",
1285+
wantParts: []pathPart{
1286+
{name: "users", value: "users", isRegex: false},
1287+
{name: "userId", isRegex: true},
1288+
{name: "posts", value: "posts", isRegex: false},
1289+
{name: "postId", isRegex: true},
1290+
{name: "customRegex1", value: "[a-z]+", isRegex: true},
1291+
{name: "customRegex2", isRegex: true},
1292+
{name: "customRegex3", value: "[a-z]{10}", isRegex: true},
1293+
{name: "abc", value: "abc", isRegex: false},
1294+
{name: "id", isRegex: true},
1295+
{name: "def", value: "def", isRegex: false},
1296+
{name: "customRegex4", value: "[0-9]+", isRegex: true},
1297+
},
1298+
wantRegex: true,
1299+
},
1300+
}
1301+
1302+
for name, tc := range tests {
1303+
t.Run(name, func(t *testing.T) {
1304+
gotParts, gotRegex := splitPath(tc.input)
1305+
1306+
assert.Equal(t, tc.wantRegex, gotRegex, "regex detection mismatch")
1307+
assert.Equal(t, tc.wantParts, gotParts, "parts mismatch")
1308+
})
1309+
}
1310+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,177 @@
1+
---
2+
name: "Path Params"
3+
tests:
4+
- source: "classic"
5+
input:
6+
version_data:
7+
versions:
8+
"":
9+
extended_paths:
10+
white_list:
11+
- path: "/test/{id}"
12+
method: ""
13+
ignore_case: false
14+
disabled: false
15+
method_actions:
16+
GET:
17+
action: "reply"
18+
code: 200
19+
headers:
20+
Content-Type: "application/json"
21+
data: '{"message": "success"}'
22+
output:
23+
paths:
24+
/test/{id}:
25+
parameters:
26+
- name: id
27+
in: path
28+
required: true
29+
schema:
30+
type: string
31+
pattern: <nil>
32+
- source: "classic"
33+
input:
34+
version_data:
35+
versions:
36+
"":
37+
extended_paths:
38+
white_list:
39+
- path: "/test/{testId:[0-9]+}"
40+
method: ""
41+
ignore_case: false
42+
disabled: false
43+
method_actions:
44+
GET:
45+
action: "reply"
46+
code: 200
47+
headers:
48+
Content-Type: "application/json"
49+
data: '{"message": "success"}'
50+
output:
51+
paths:
52+
/test/{testId}:
53+
parameters:
54+
- name: testId
55+
in: path
56+
required: true
57+
schema:
58+
type: string
59+
pattern: "<nil>"
60+
- source: "classic"
61+
input:
62+
version_data:
63+
versions:
64+
"":
65+
extended_paths:
66+
white_list:
67+
- path: "/test/[0-9]+"
68+
method: ""
69+
ignore_case: false
70+
disabled: false
71+
method_actions:
72+
GET:
73+
action: "reply"
74+
code: 200
75+
headers:
76+
Content-Type: "application/json"
77+
data: '{"message": "success"}'
78+
output:
79+
paths:
80+
/test/{customRegex1}:
81+
parameters:
82+
- name: customRegex1
83+
in: path
84+
required: true
85+
schema:
86+
type: string
87+
pattern: "[0-9]+"
88+
- source: "classic"
89+
input:
90+
version_data:
91+
versions:
92+
"":
93+
extended_paths:
94+
white_list:
95+
- path: "/test/testId"
96+
method: ""
97+
ignore_case: false
98+
disabled: false
99+
method_actions:
100+
GET:
101+
action: "reply"
102+
code: 200
103+
headers:
104+
Content-Type: "application/json"
105+
data: '{"message": "success"}'
106+
output:
107+
paths:
108+
/test/testId:
109+
parameters: <nil>
110+
- source: "classic"
111+
input:
112+
version_data:
113+
versions:
114+
"":
115+
extended_paths:
116+
white_list:
117+
- path: "/test/{[0-9]+}/{id}"
118+
method: ""
119+
ignore_case: false
120+
disabled: false
121+
method_actions:
122+
GET:
123+
action: "reply"
124+
code: 200
125+
headers:
126+
Content-Type: "application/json"
127+
data: '{"message": "success"}'
128+
output:
129+
paths:
130+
/test/{customRegex1}/{id}:
131+
parameters:
132+
- name: customRegex1
133+
in: path
134+
required: true
135+
schema:
136+
type: string
137+
pattern: <nil>
138+
- name: id
139+
in: path
140+
required: true
141+
schema:
142+
type: string
143+
pattern: <nil>
144+
- source: "classic"
145+
input:
146+
version_data:
147+
versions:
148+
"":
149+
extended_paths:
150+
white_list:
151+
- path: "/abc/{id}/def/[0-9]+"
152+
method: ""
153+
ignore_case: false
154+
disabled: false
155+
method_actions:
156+
GET:
157+
action: "reply"
158+
code: 200
159+
headers:
160+
Content-Type: "application/json"
161+
data: '{"message": "success"}'
162+
output:
163+
paths:
164+
/abc/{id}/def/{customRegex1}:
165+
parameters:
166+
- name: id
167+
in: path
168+
required: true
169+
schema:
170+
type: string
171+
pattern: <nil>
172+
- name: customRegex1
173+
in: path
174+
required: true
175+
schema:
176+
type: string
177+
pattern: "[0-9]+"

0 commit comments

Comments
 (0)