fix: throw error if route segment is empty or null#962
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughInput validation was added to URLBuilder.addSubroute(String) to throw IllegalArgumentException for null or empty route segments. A unit test was added to assert the exception type and exact message. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
f03b327 to
311f94e
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/java/com/meilisearch/sdk/http/URLBuilder.java`:
- Around line 24-26: In URLBuilder where you currently validate "route" (the
block that throws IllegalArgumentException for null or empty), tighten
validation to reject blank-only and slash-only segments: trim the route and
reject if trimmed is empty, and also reject if removing all slashes from the
trimmed value yields an empty string (e.g., "/" or "///"). After validation,
normalize the segment by stripping leading and trailing slashes before appending
so you don't introduce double slashes; update the validation/normalization logic
in the URLBuilder method that handles the "route" parameter accordingly.
In `@src/test/java/com/meilisearch/sdk/http/URLBuilderTest.java`:
- Around line 36-41: The test addSubrouteWithNullOrEmptyRoute currently only
asserts the empty-string case; add a second assertion that calls
classToTest.addSubroute(null) wrapped in
assertThrows(IllegalArgumentException.class) and assert that its getMessage()
equals "route segment must not be null or empty" so both null and empty branches
of URLBuilder.addSubroute are covered (refer to addSubroute and classToTest in
the test).
- Around line 35-41: The test URLBuilderTest.addSubrouteWithNullOrEmptyRoute
uses assertThrows but the static import is missing; fix by either adding the
static import for org.junit.jupiter.api.Assertions.assertThrows at the top of
the test file or by qualifying the call as Assertions.assertThrows(..) when
invoking assertThrows around classToTest.addSubroute(""); keep the rest of the
assertion intact so the IllegalArgumentException message check still uses
getMessage() and is(equalTo(...)).
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ae2ee2cf-c873-488a-b05d-9c45c7600dfe
📒 Files selected for processing (2)
src/main/java/com/meilisearch/sdk/http/URLBuilder.javasrc/test/java/com/meilisearch/sdk/http/URLBuilderTest.java
Pull Request
Related issue
Fixes #961
What does this PR do?
Summary by CodeRabbit
Bug Fixes
Tests