[TT-12762]respect response plugins contract over responsePlugin.plugins

[jeffy-mathew]

User description

Description

Fix a bug where response plugins are not loaded when using OAS APIs.

Related Issue

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

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

Bug fix, Tests

---

Description

• Fixed a bug where response plugins were not loaded correctly when using OAS APIs by adding a return statement after extracting response plugins.
• Added a new test case to verify the correct extraction of response plugins and ensure the regression issue TT-12762 is resolved.

---

Changes walkthrough 📝

	Relevant files
Bug fix	middleware.go Fix response plugins extraction logic                                        apidef/oas/middleware.go Added a return statement after extracting response plugins to ensure correct behavior. Initialized ResponsePlugin if it is nil. +1/-0
Tests	middleware_test.go Add test for response plugins extraction                                  apidef/oas/middleware_test.go Added a new test case to verify the correct extraction of response plugins. Ensured the test case checks for the regression issue TT-12762. +56/-0

---

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

[github-actions]

API Changes

no api changes detected

[github-actions]

PR Reviewer Guide 🔍

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

🧪 PR contains tests

🔒 No security concerns identified

⚡ Key issues to review Early Return The addition of return at line 417 might cause unintended early termination of the extractResponsePluginsTo function, skipping any logic that follows the conditional block. This needs careful review to ensure that subsequent logic is not necessary when g.ResponsePlugins is not nil.

[github-actions]

PR Code Suggestions ✨

Category	Suggestion	Score
Possible bug	Modify the return logic to ensure all necessary branches are evaluated The addition of the return statement after processing ResponsePlugins may prematurely terminate the function, potentially skipping important logic that follows, especially if ResponsePlugin is not nil and needs processing. Consider restructuring the logic to ensure all necessary conditions and branches are evaluated. apidef/oas/middleware.go [414-417] if g.ResponsePlugins != nil { api.CustomMiddleware.Response = make([]apidef.MiddlewareDefinition, len(g.ResponsePlugins)) g.ResponsePlugins.ExtractTo(api.CustomMiddleware.Response) - return + if g.ResponsePlugin == nil { + return + } } Suggestion importance[1-10]: 9 Why: The suggestion correctly identifies a potential issue where the function may return prematurely, skipping important logic. The proposed change ensures that all necessary conditions are evaluated, improving the function's robustness.	9
Maintainability	Refactor test setup into a separate helper function for clarity The test case for "response plugins" includes a large block of setup code which can be refactored into a helper function to improve readability and reusability. This will make the test easier to understand and maintain. apidef/oas/middleware_test.go [76-110] t.Run("response plugins", func(t *testing.T) { - customPlugins := CustomPlugins{ - CustomPlugin{ - Enabled: true, - FunctionName: "func1", - Path: "/path1", - }, - } - responsePlugins := CustomPlugins{ - CustomPlugin{ - Enabled: true, - FunctionName: "func2", - Path: "/path2", - }, - CustomPlugin{ - Enabled: true, - FunctionName: "func3", - Path: "/path3", - }, - } - var pluginMW = Middleware{ - Global: &Global{ - ResponsePlugin: &ResponsePlugin{ - Plugins: customPlugins, - }, - ResponsePlugins: responsePlugins, - }, - } + customPlugins, responsePlugins, pluginMW := setupResponsePluginsTest() var convertedAPI apidef.APIDefinition convertedAPI.SetDisabledFlags() pluginMW.ExtractTo(&convertedAPI) }) +// Define setupResponsePluginsTest in a separate function Suggestion importance[1-10]: 7 Why: The suggestion improves code readability and maintainability by refactoring the setup code into a helper function. This makes the test easier to understand and maintain, though it does not address any critical issues.	7

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarCloud

[jeffy-mathew]

/release to release-5.5

[jeffy-mathew]

/release to release-5.5.0

[jeffy-mathew]

/release to release-5.3

[tykbot]

Working on it! Note that it can take a few minutes.

[tykbot]

Working on it! Note that it can take a few minutes.

[tykbot]

Working on it! Note that it can take a few minutes.

[tykbot]

@jeffy-mathew Seems like there is conflict and it require manual merge.

[tykbot]

Still working...

[tykbot]

@jeffy-mathew Succesfully merged PR

[tykbot]

@jeffy-mathew Succesfully merged PR