Skip to content

fix: throw error if route segment is empty or null#962

Merged
Strift merged 2 commits intomainfrom
fix-delete-document
Apr 22, 2026
Merged

fix: throw error if route segment is empty or null#962
Strift merged 2 commits intomainfrom
fix-delete-document

Conversation

@StephaneRob
Copy link
Copy Markdown
Contributor

@StephaneRob StephaneRob commented Apr 21, 2026

Pull Request

Related issue

Fixes #961

What does this PR do?

  • check route segment before appending to url and throw error if null or empty

Summary by CodeRabbit

  • Bug Fixes

    • Improved validation of URL path segments to reject null or empty values; prevents creation of invalid paths and returns clearer error messages when segments are missing.
  • Tests

    • Added unit test coverage for invalid input handling to ensure the new validation and error messaging remain enforced and stable.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 21, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3e803f5e-e2a4-47ad-922b-05076c0cdfc4

📥 Commits

Reviewing files that changed from the base of the PR and between 311f94e and df8a49a.

📒 Files selected for processing (1)
  • src/test/java/com/meilisearch/sdk/http/URLBuilderTest.java
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/test/java/com/meilisearch/sdk/http/URLBuilderTest.java

📝 Walkthrough

Walkthrough

Input 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

Cohort / File(s) Summary
Input Validation
src/main/java/com/meilisearch/sdk/http/URLBuilder.java
Added null/empty check in addSubroute(String route) and throw IllegalArgumentException with message "route segment must not be null or empty".
Test Coverage
src/test/java/com/meilisearch/sdk/http/URLBuilderTest.java
Added addSubrouteWithNullOrEmptyRoute test using assertThrows to verify IllegalArgumentException and exact message for "" and null inputs.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Poem

🐰 I hop through routes both short and long,
I sniff each segment — right from wrong.
No empty trails shall pass my test,
I guard the paths and do my best. 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the main change: adding validation to throw an error when route segments are null or empty.
Linked Issues check ✅ Passed The PR fully addresses issue #961 by adding validation in URLBuilder.addSubroute() to reject null or empty route segments, preventing accidental deletion of all documents.
Out of Scope Changes check ✅ Passed All changes are within scope: URLBuilder validation logic and corresponding test cases directly address the linked issue with no unrelated modifications.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-delete-document

Comment @coderabbitai help to get the list of available commands and usage tips.

@StephaneRob StephaneRob force-pushed the fix-delete-document branch from f03b327 to 311f94e Compare April 21, 2026 05:41
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 74dc904 and f03b327.

📒 Files selected for processing (2)
  • src/main/java/com/meilisearch/sdk/http/URLBuilder.java
  • src/test/java/com/meilisearch/sdk/http/URLBuilderTest.java

Comment thread src/main/java/com/meilisearch/sdk/http/URLBuilder.java
Comment thread src/test/java/com/meilisearch/sdk/http/URLBuilderTest.java
Comment thread src/test/java/com/meilisearch/sdk/http/URLBuilderTest.java
@Strift Strift added the bug Something isn't working label Apr 22, 2026
@Strift Strift added this pull request to the merge queue Apr 22, 2026
Merged via the queue into main with commit 9919b11 Apr 22, 2026
3 checks passed
@Strift Strift deleted the fix-delete-document branch April 22, 2026 03:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Delete documents without identifier delete all documents

2 participants