TT-7306: Migrate Mock Response from Classic API Definition to OAS API Definition

[edsonmichaque]

User description

TT-7306

Summary	[OAS:migration] Migrate Mock Response from Classic API Definition to OAS API Definition
Type	Bug
Status	In Dev
Points	N/A
Labels	-

---

Description

This PR adds support for converting mock responses between Tyk's classic API format and OAS format.

Related Issue

https://tyktech.atlassian.net/browse/TT-7306

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 support for migrating mock responses between Classic and OAS API definitions.

• Implemented methods to fill and extract mock responses in OAS middleware.

• Enhanced test coverage for mock response handling, including edge cases.

• Introduced YAML fixtures for mock response scenarios in OAS and Classic formats.

---

Changes walkthrough 📝

	Relevant files
Enhancement	middleware.go Enhance middleware to support mock responses                          apidef/oas/middleware.go Added initialization for Operations in the Fill method. Enhanced Fill method to handle mock responses. +4/-0      operation.go Add mock response handling in OAS operations                          apidef/oas/operation.go Added logic to handle mock response paths and operations. Implemented methods to fill and extract mock responses. Introduced generateOperationID utility for mock responses. +139/-0
Tests	middleware_test.go Add tests for middleware mock response handling                    apidef/oas/middleware_test.go Added test case for mock response extraction and filling. Verified mock response consistency during migration. +26/-0    operation_test.go Add tests for OAS mock response operations                              apidef/oas/operation_test.go Added comprehensive tests for mock response extraction and filling. Covered scenarios for basic, multiple, and disabled mock responses. +345/-0  mock_response.yml Add YAML fixtures for mock response testing                            apidef/oas/testdata/fixtures/mock_response.yml Added YAML fixtures for mock response scenarios. Included cases for OAS to Classic and vice versa. Covered enabled and disabled mock responses. +121/-0

---

Need help?

Type /help how to ... in the comments thread for any questions about PR-Agent usage.

Check out the documentation for more information.

[buger]

This PR is too huge for one to review 💔

Additions	1292 🙅‍♀️
Expected	⬇️ 800

Consider breaking it down into multiple small PRs.

Check out this guide to learn more about PR best-practices.

[buger]

Let's make that PR title a 💯 shall we? 💪

Your PR title and story title look slightly different. Just checking in to know if it was intentional!

Story Title	[OAS:migration] Migrate Mock Response from Classic API Definition to OAS API Definition
PR Title	wip: extract and fill mock responses from/to oas/classic

Check out this guide to learn more about PR best-practices.

[github-actions]

API Changes

--- prev.txt	2025-02-25 12:55:37.129731102 +0000
+++ current.txt	2025-02-25 12:55:32.968699417 +0000
@@ -3300,6 +3300,11 @@
 }
     MockResponse configures the mock responses.
 
+func (m *MockResponse) ExtractTo(meta *apidef.MockResponseMeta)
+
+func (m *MockResponse) Fill(op apidef.MockResponseMeta)
+    Fill populates the MockResponse fields from a classic API MockResponseMeta.
+
 func (m *MockResponse) Import(enabled bool)
     Import populates *MockResponse with enabled argument for FromOASExamples.
 

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit cb56906)

Here are some key observations to aid the review process:

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

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Possible Issue The fillMockResponsePaths function assumes that the Content-Type header exists in the mockResponse.Headers map. While it defaults to "text/plain" if not found, there might be edge cases where this assumption could lead to unexpected behavior. Ensure this logic is robust and handles all edge cases. func (s *OAS) fillMockResponsePaths(paths openapi3.Paths, ep apidef.ExtendedPathsSet) { if paths == nil { paths = make(openapi3.Paths) } for _, mockResponse := range ep.MockResponse { path := mockResponse.Path method := mockResponse.Method if paths[path] == nil { paths[path] = &openapi3.PathItem{} } statusCode := strconv.Itoa(mockResponse.Code) contentType := "text/plain" if ct, exists := mockResponse.Headers["Content-Type"]; exists { contentType = ct } op := &openapi3.Operation{ OperationID: generateOperationID(mockResponse), Responses: openapi3.Responses{ statusCode: &openapi3.ResponseRef{ Value: &openapi3.Response{ Description: &[]string{"OK"}[0], Content: openapi3.Content{ contentType: &openapi3.MediaType{ Schema: &openapi3.SchemaRef{ Value: &openapi3.Schema{ Type: "string", Example: mockResponse.Body, }, }, }, }, }, }, }, } // Set the operation based on method switch method { case http.MethodGet: paths[path].Get = op case http.MethodPost: paths[path].Post = op case http.MethodPut: paths[path].Put = op case http.MethodDelete: paths[path].Delete = op case http.MethodPatch: paths[path].Patch = op } // Set up the mock response in Tyk extension operationID := generateOperationID(mockResponse) operation := s.GetTykExtension().getOperation(operationID) if operation.MockResponse == nil { operation.MockResponse = &MockResponse{} } if operation.IgnoreAuthentication == nil { // Enable ignore authentication for this endpoint to maintain compatibility with Classic APIs // In Classic APIs, mock responses are processed before authentication // In OAS, mock responses are processed at the end of the chain, so we need to explicitly // bypass authentication to maintain the same behavior during migration operation.IgnoreAuthentication = &Allowance{ Enabled: true, } } operation.MockResponse.Fill(mockResponse) } Code Duplication The fillMockResponsePaths function contains repetitive code for setting up operations for different HTTP methods. Consider refactoring to reduce duplication and improve maintainability. switch method { case http.MethodGet: paths[path].Get = op case http.MethodPost: paths[path].Post = op case http.MethodPut: paths[path].Put = op case http.MethodDelete: paths[path].Delete = op case http.MethodPatch: paths[path].Patch = op } Test Coverage The test cases for fillMockResponsePaths do not seem to cover scenarios where the MockResponse object has invalid or unexpected data. Adding such tests would improve robustness. func TestOAS_MockResponse_fillMockResponsePaths(t *testing.T) { t.Parallel() // Test case 1: Basic mock response t.Run("basic mock response", func(t *testing.T) { ep := apidef.ExtendedPathsSet{ MockResponse: []apidef.MockResponseMeta{ { Path: "/test", Method: "GET", Code: 200, Body: `{"message": "success"}`, Headers: map[string]string{ "Content-Type": "application/json", }, }, }, } spec := &OAS{ T: openapi3.T{ Paths: openapi3.Paths{}, }, } spec.fillMockResponsePaths(spec.Paths, ep) // Verify the mock response was correctly added to paths assert.Len(t, spec.Paths, 1) pathItem := spec.Paths["/test"] assert.NotNil(t, pathItem) assert.NotNil(t, pathItem.Get) assert.Equal(t, "testGET", pathItem.Get.OperationID) assert.Nil(t, pathItem.Post) assert.Nil(t, pathItem.Put) assert.Nil(t, pathItem.Patch) assert.Nil(t, pathItem.Delete) // Verify response responses := pathItem.Get.Responses assert.NotNil(t, responses) response := responses["200"] assert.NotNil(t, response.Value) // Verify content content := response.Value.Content assert.NotNil(t, content["application/json"]) mediaType := content["application/json"] assert.Equal(t, `{"message": "success"}`, mediaType.Example) assert.Equal(t, `{"message": "success"}`, mediaType.Examples["default"].Value.Value) // Verify headers headers := response.Value.Headers assert.NotNil(t, headers["Content-Type"]) assert.Equal(t, "application/json", headers["Content-Type"].Value.Schema.Value.Example) }) // Test case 2: Multiple methods t.Run("multiple methods", func(t *testing.T) { ep := apidef.ExtendedPathsSet{ MockResponse: []apidef.MockResponseMeta{ { Path: "/test", Method: "GET", Code: 200, Body: `{"status": "ok"}`, Headers: map[string]string{ "Content-Type": "application/json", }, }, { Path: "/test", Method: "POST", Code: 201, Body: `{"id": "123"}`, Headers: map[string]string{ "Content-Type": "application/json", "Location": "/test/123", }, }, }, } spec := &OAS{ T: openapi3.T{ Paths: openapi3.Paths{}, }, } spec.fillMockResponsePaths(spec.Paths, ep) // Verify path item assert.Len(t, spec.Paths, 1) pathItem := spec.Paths["/test"] assert.NotNil(t, pathItem) assert.NotNil(t, pathItem.Get) assert.NotNil(t, pathItem.Post) // Verify GET operation assert.Equal(t, "testGET", pathItem.Get.OperationID) getResponse := pathItem.Get.Responses["200"].Value assert.Equal(t, `{"status": "ok"}`, getResponse.Content["application/json"].Example) assert.Equal(t, "application/json", getResponse.Headers["Content-Type"].Value.Schema.Value.Example) // Verify POST operation assert.Equal(t, "testPOST", pathItem.Post.OperationID) postResponse := pathItem.Post.Responses["201"].Value assert.Equal(t, `{"id": "123"}`, postResponse.Content["application/json"].Example) assert.Equal(t, "application/json", postResponse.Headers["Content-Type"].Value.Schema.Value.Example) assert.Equal(t, "/test/123", postResponse.Headers["Location"].Value.Schema.Value.Example) }) // Test case 3: No content type header t.Run("no content type header", func(t *testing.T) { ep := apidef.ExtendedPathsSet{ MockResponse: []apidef.MockResponseMeta{ { Path: "/test", Method: "GET", Code: 204, Body: "", }, }, } spec := &OAS{ T: openapi3.T{ Paths: openapi3.Paths{}, }, } spec.fillMockResponsePaths(spec.Paths, ep) // Verify response pathItem := spec.Paths["/test"] assert.NotNil(t, pathItem) response := pathItem.Get.Responses["204"].Value // When no content type is specified, it should default to text/plain assert.NotNil(t, response.Content["text/plain"]) assert.Equal(t, "", response.Content["text/plain"].Example) assert.Empty(t, response.Headers) })

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to cb56906
Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Validate HTTP method before usage Add a check to ensure that the method variable in fillMockResponsePaths is valid (e.g., GET, POST, etc.) before using it in the switch statement to avoid unexpected behavior or runtime errors. apidef/oas/operation.go [211-221] -switch method { -case http.MethodGet: - paths[path].Get = op -case http.MethodPost: - paths[path].Post = op -case http.MethodPut: - paths[path].Put = op -case http.MethodDelete: - paths[path].Delete = op -case http.MethodPatch: - paths[path].Patch = op +if isValidHTTPMethod(method) { + switch method { + case http.MethodGet: + paths[path].Get = op + case http.MethodPost: + paths[path].Post = op + case http.MethodPut: + paths[path].Put = op + case http.MethodDelete: + paths[path].Delete = op + case http.MethodPatch: + paths[path].Patch = op + } +} else { + log.Errorf("Invalid HTTP method: %s", method) } Suggestion importance[1-10]: 8 __ Why: Adding a validation check for the HTTP method ensures robustness and prevents runtime errors caused by invalid or unexpected method values. This is a significant improvement in terms of error handling and code safety.	Medium
	Validate header content type existence Ensure that mockResponse.Headers["Content-Type"] is checked for nil or empty values before assigning it to contentType to avoid potential nil pointer dereferences. apidef/oas/operation.go [185-186] -if ct, exists := mockResponse.Headers["Content-Type"]; exists { +if ct, exists := mockResponse.Headers["Content-Type"]; exists && ct != "" { contentType = ct } Suggestion importance[1-10]: 7 __ Why: Checking for nil or empty values in the "Content-Type" header before assigning it to contentType prevents potential nil pointer dereferences, enhancing code reliability and avoiding runtime errors.	Medium
General	Handle empty operation ID generation Add a fallback mechanism in generateOperationID to handle cases where mockResponse.Path or mockResponse.Method is empty to avoid generating invalid operation IDs. apidef/oas/operation.go [1005-1006] +if mockResponse.Path == "" || mockResponse.Method == "" { + return "defaultOperationID" +} return strings.TrimPrefix(mockResponse.Path, "/") + mockResponse.Method Suggestion importance[1-10]: 6 __ Why: Adding a fallback mechanism for empty mockResponse.Path or mockResponse.Method ensures that a valid operation ID is always generated, improving the robustness of the code. However, the fallback value "defaultOperationID" might not be ideal and could lead to conflicts if not handled carefully.	Low
	Ensure proper initialization of operations Add a check to ensure m.Operations is properly initialized with default values to avoid potential nil pointer dereferences when accessing its fields. apidef/oas/middleware.go [33-34] if m.Operations == nil { - m.Operations = Operations{} + m.Operations = Operations{ + // Initialize with default values if necessary + } } Suggestion importance[1-10]: 5 __ Why: While the suggestion to initialize m.Operations with default values adds clarity and reduces potential nil pointer dereferences, the current implementation already initializes it to an empty Operations struct, which is sufficient in most cases. The improvement is minor.	Low

---

Previous suggestions

Suggestions up to commit 845f0bc

Category	Suggestion	Impact
Possible issue	Prevent nil pointer dereference Add a check to ensure that mainVersion.ExtendedPaths is not nil before accessing or modifying its MockResponse field in the Operations.ExtractTo method to prevent potential nil pointer dereferences. apidef/oas/operation.go [54-64] extendedPaths := mainVersion.ExtendedPaths -... +if extendedPaths == nil { + extendedPaths = apidef.ExtendedPathsSet{} +} extendedPaths.MockResponse = append(extendedPaths.MockResponse, mockMeta) Suggestion importance[1-10]: 9 __ Why: The suggestion adds a necessary nil check for mainVersion.ExtendedPaths before accessing or modifying its MockResponse field, which is crucial to prevent potential runtime errors due to nil pointer dereferences. This is a significant improvement in ensuring code robustness.	High
	Add nil check for safety Add a nil check for m.Global before calling m.Global.ExtractTo(api) in the Middleware.ExtractTo method to prevent runtime errors if m.Global is nil. apidef/oas/middleware.go [52-53] -m.Global.ExtractTo(api) +if m.Global != nil { + m.Global.ExtractTo(api) +} Suggestion importance[1-10]: 9 __ Why: Adding a nil check for m.Global before calling m.Global.ExtractTo(api) is a critical safeguard against runtime errors. This change significantly improves the robustness of the Middleware.ExtractTo method.	High
General	Validate input before processing paths Validate that mockResponse.Path and mockResponse.Method are not empty before using them in fillMockResponsePaths to avoid generating invalid OpenAPI paths or operations. apidef/oas/operation.go [267-341] path := mockResponse.Path method := mockResponse.Method +if path == "" || method == "" { + continue +} ... paths[path].Get = oasOperation Suggestion importance[1-10]: 8 __ Why: The suggestion introduces validation for mockResponse.Path and mockResponse.Method to ensure they are not empty before processing. This prevents invalid OpenAPI paths or operations, enhancing the reliability and correctness of the code.	Medium
	Avoid unnecessary map reinitialization Ensure the Headers map in MockResponse.ExtractTo is initialized only when it is nil to avoid unnecessary reinitialization and potential data loss. apidef/oas/operation.go [964-969] -meta.Headers = make(map[string]string) +if meta.Headers == nil { + meta.Headers = make(map[string]string) +} for _, h := range m.Headers { meta.Headers[h.Name] = h.Value } Suggestion importance[1-10]: 7 __ Why: The suggestion ensures that the Headers map in MockResponse.ExtractTo is only initialized when it is nil, avoiding redundant reinitialization and potential data loss. While this is a minor optimization, it improves code efficiency and clarity.	Medium

[github-actions]

Persistent review updated to latest commit cb56906

[titpetric]

Bit of a code smell, headers have canonical formatting and we're checking two casings.
https://pkg.go.dev/net/http#CanonicalHeaderKey

[edsonmichaque]

Thanks, let me correct it

[edsonmichaque]

Fixed it, PTAL

[titpetric]

Suggested change

	var js interface{}
	var js interface{}

Could this be json.RawMessage?

[edsonmichaque]

Let fix that, thanks

[edsonmichaque]

Fixed it, PTAL

[jeffy-mathew]

this would also satisfy a simple string - ref

How about we use a map to unmarshal? - ref

[edsonmichaque]

You're right, thanks @jeffy-mathew.
A quick question, do you think we should also unmarshal to []interface{}, to cover arrays?

[edsonmichaque]

PTAL again @titpetric and @jeffy-mathew

[sonarqubecloud]

Quality Gate passed

Issues
3 New issues
0 Accepted issues

Measures
0 Security Hotspots
89.5% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[andrei-tyk]

the function body is way too long, we should create helper functions whenever possible to increase readability and reduce complexity

[andrei-tyk]

"oasRequiredDummyValue" sounds more indicative than "CHANGE ME"

[andrei-tyk]

no need to create a slice of variants, its easier to read if you check for both

[andrei-tyk]

if you split the method into multiple helpers. you will be able to just return from the underlying helper instead of breaking the for loop

[andrei-tyk]

extract this into a helper to not increase the body of the extractPathsAndOperations function

[andrei-tyk]

use either assert or require

[andrei-tyk]

use either only assert or only require

[andrei-tyk]

function in function looks concise but it's hard to review in a web editor