Add cucumber tests for API Product context length validation - #14271
Add cucumber tests for API Product context length validation#14271manodyaSenevirathne wants to merge 3 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughChangesAPI product context length handling
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant CucumberScenario
participant Utils
participant Names
CucumberScenario->>Utils: resolvePayloadPlaceholders(payload)
Utils->>Names: ofLength(length)
Names-->>Utils: exact-length value
Utils-->>CucumberScenario: resolved payload
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@all-in-one-apim/modules/integration-v2/tests-integration/cucumber-tests/src/test/java/org/wso2/am/integration/cucumbertests/utils/Names.java`:
- Around line 66-73: Update Names.ofLength(int length) to validate negative
inputs before building or truncating the value. When length is less than zero,
throw IllegalArgumentException with a clear message; preserve the existing
generation behavior for zero and positive lengths.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b6646cff-b83a-42e5-8c18-8105856bf453
📒 Files selected for processing (3)
all-in-one-apim/modules/integration-v2/tests-integration/cucumber-tests/src/test/java/org/wso2/am/integration/cucumbertests/utils/Names.javaall-in-one-apim/modules/integration-v2/tests-integration/cucumber-tests/src/test/java/org/wso2/am/integration/cucumbertests/utils/Utils.javaall-in-one-apim/modules/integration-v2/tests-integration/cucumber-tests/src/test/resources/features/publisher/api_products.feature
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #14271 +/- ##
=============================================
+ Coverage 19.94% 63.87% +43.92%
+ Complexity 1523 166 -1357
=============================================
Files 365 25 -340
Lines 17875 454 -17421
Branches 1913 11 -1902
=============================================
- Hits 3566 290 -3276
+ Misses 14266 158 -14108
+ Partials 43 6 -37
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
all-in-one-apim/modules/integration-v2/tests-integration/cucumber-tests/src/test/java/org/wso2/am/integration/cucumbertests/utils/Utils.java (3)
346-355: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winExclude the URL fragment from query parsing.
For a URL such as
...?code=abc#fragment, this method returnsabc#fragmentforcode. The fragment is not part of the query value and can corrupt redirect or authorization-code handling.Stop the query substring at the first raw
#before splitting it into parameters.Suggested fix
- for (String pair : url.substring(q + 1).split("&")) { + String query = url.substring(q + 1); + int fragment = query.indexOf('#'); + if (fragment >= 0) { + query = query.substring(0, fragment); + } + for (String pair : query.split("&")) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@all-in-one-apim/modules/integration-v2/tests-integration/cucumber-tests/src/test/java/org/wso2/am/integration/cucumbertests/utils/Utils.java` around lines 346 - 355, Update the query parsing logic in the URL parameter helper around the visible q and substring handling to locate the first raw '#' after the query marker and exclude it from the query substring before splitting parameters. Preserve existing parameter decoding and handling for URLs without fragments.
1042-1070: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winKeep lookup failures distinct from “resource not found.”
This helper returns
nullfor failed reads and malformed bodies, but its documented callers treat everynullas absent and retry the create path. A 4xx/5xx response, bad JSON, or missinglistcan therefore cause duplicate resource creation.Return
nullonly for a valid successful body where no matching entry exists. Propagate lookup failures or return an explicit error result.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@all-in-one-apim/modules/integration-v2/tests-integration/cucumber-tests/src/test/java/org/wso2/am/integration/cucumbertests/utils/Utils.java` around lines 1042 - 1070, Update findIdByNameInListResponse so null is returned only after a valid 200 response with a parseable body and list containing no matching name. Treat non-2xx responses, missing or malformed list data, and HTTP/JSON failures as lookup errors by propagating them or using an explicit error result, and update the method contract/callers as needed so failures are not interpreted as “not found.”Sources: Coding guidelines, Learnings
111-117: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winCap the inter-poll sleep at the deadline.
pollWithinsleeps after an unsuccessful attempt only viapollPause, soSystem.currentTimeMillis() < deadlinedoes not limit that sleep. When an attempt finishes neardeadline, this sleeps for the full tiered interval before returning null and overruns the documented retry window.Use a deadline-aware pause helper and pass the remaining time to
pollPausePauseUntil(...)asMath.min(interval, remaining). Also update external callers that pass their own deadlines, such asGatewayRestArtifactsSteps.iRetrieveGatewayArtifactUntilAvailableand the stepdefs with directpollPausewaits.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@all-in-one-apim/modules/integration-v2/tests-integration/cucumber-tests/src/test/java/org/wso2/am/integration/cucumbertests/utils/Utils.java` around lines 111 - 117, The pollPause retry sleep can exceed the caller’s deadline. Update pollPause and its callers to use a deadline-aware pause helper, passing the remaining duration and capping each tiered interval with Math.min(interval, remaining); update pollWithin, GatewayRestArtifactsSteps.iRetrieveGatewayArtifactUntilAvailable, and step definitions with direct pollPause waits while preserving existing polling behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@all-in-one-apim/modules/integration-v2/tests-integration/cucumber-tests/src/test/java/org/wso2/am/integration/cucumbertests/utils/Utils.java`:
- Around line 346-355: Update the query parsing logic in the URL parameter
helper around the visible q and substring handling to locate the first raw '#'
after the query marker and exclude it from the query substring before splitting
parameters. Preserve existing parameter decoding and handling for URLs without
fragments.
- Around line 1042-1070: Update findIdByNameInListResponse so null is returned
only after a valid 200 response with a parseable body and list containing no
matching name. Treat non-2xx responses, missing or malformed list data, and
HTTP/JSON failures as lookup errors by propagating them or using an explicit
error result, and update the method contract/callers as needed so failures are
not interpreted as “not found.”
- Around line 111-117: The pollPause retry sleep can exceed the caller’s
deadline. Update pollPause and its callers to use a deadline-aware pause helper,
passing the remaining duration and capping each tiered interval with
Math.min(interval, remaining); update pollWithin,
GatewayRestArtifactsSteps.iRetrieveGatewayArtifactUntilAvailable, and step
definitions with direct pollPause waits while preserving existing polling
behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a7e91d3e-9e35-4f4c-974d-305ce9c3fd73
📒 Files selected for processing (1)
all-in-one-apim/modules/integration-v2/tests-integration/cucumber-tests/src/test/java/org/wso2/am/integration/cucumbertests/utils/Utils.java
5ac8c9d to
99c11d2
Compare
b6f9324 to
ca06b32
Compare
ec0b6cd to
5024c00
Compare
Adds two scenarios to publisher/api_products.feature validating the
maximum allowed length for API Product contexts: one within the limit
(150 chars, expects 201) and one exceeding it (233 chars, expects 400
mentioning the limit). Introduces a ${LENGTH:n} payload placeholder
(Utils.resolvePayloadPlaceholders / Names.ofLength) for generating a
context of an exact character count that stays unique across reruns,
since neither a literal string nor the existing ${UNIQUE:base} pattern
could satisfy both an exact length and rerun-safety at once.
Note: these two new scenarios currently fail against this repo's
present carbon-apimgt dependency, which still enforces the old 60-char
limit rather than 232. They are correctly detecting that the
underlying fix has not yet been forward-ported here.
Related Issue
- wso2-enterprise/wso2-apim-internal#17921
…ber-tests/src/test/java/org/wso2/am/integration/cucumbertests/utils/Names.java Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…cenarios Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
5024c00 to
b7128da
Compare
1 participant
Purpose
Adds two new Cucumber scenarios to
publisher/api_products.featurevalidating the maximum allowed length for API Product contexts:Introduces a
${LENGTH:n}payload placeholder (Utils.resolvePayloadPlaceholders/Names.ofLength) for generating a context of an exact character count that also stays unique across reruns — neither a literal string nor the existing${UNIQUE:base}pattern could satisfy both requirements at once.Note: these two scenarios currently fail against this repo's present carbon-apimgt dependency, which still enforces the old 60-char limit rather than 232 (confirmed via
context: size must be between 1 and 60in the actual response). They are correctly detecting that the underlying fix has not yet been forward-ported here — that is a separate, follow-up piece of work.Related Issue
Merge after