Conversation
Introduce MontonioClient as the top-level SDK entry point with
domain-scoped access: client.orders().get(uuid). OrderService
validates the UUID and delegates GET /orders/{uuid} to the HTTP
client, returning the existing OrderResponse.
Widen MontonioHttpClient three-arg constructor to public so
OrderService (and SDK consumers) can inject test doubles.
Closes #16
Related: #37 (thread safety), #38 (create order operation)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
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 (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR adds GET /orders/{uuid}: a new Changes
Sequence DiagramsequenceDiagram
participant Client
participant MontonioClient
participant OrderService
participant MontonioHttpClient
participant HttpClient
participant Server
Client->>MontonioClient: orders()
activate MontonioClient
MontonioClient->>MontonioClient: lazy instantiate OrderService (cached)
MontonioClient-->>Client: OrderService
deactivate MontonioClient
Client->>OrderService: get(uuid)
activate OrderService
OrderService->>OrderService: validate & trim uuid
OrderService->>MontonioHttpClient: get("/orders/{uuid}", OrderResponse.class)
deactivate OrderService
activate MontonioHttpClient
MontonioHttpClient->>HttpClient: send(HttpRequest)
deactivate MontonioHttpClient
activate HttpClient
HttpClient->>Server: GET /orders/{uuid}
Server-->>HttpClient: 200 JSON / 4xx error
deactivate HttpClient
activate MontonioHttpClient
MontonioHttpClient->>MontonioHttpClient: map/deserialize response (OrderResponse or MontonioApiException)
MontonioHttpClient-->>Client: return or throw mapped result
deactivate MontonioHttpClient
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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)
Warning Review ran into problems🔥 ProblemsTimed out fetching pipeline failures after 30000ms Comment |
Member
Author
|
@coderabbitai review |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@docs/plans/2026-04-10-get-order-status-design.md`:
- Around line 29-33: The fenced code block containing "MontonioClient →
MontonioHttpClient → HttpClient (java.net)" is missing a language identifier;
update the opening triple-backtick to include a language (e.g., ```text) so
markdownlint MD040 is satisfied and the block renders correctly.
In `@src/main/java/ee/bitweb/montonio/sdk/MontonioClient.java`:
- Around line 9-10: The orders() lazy cache in MontonioClient is not thread-safe
(orderService can be instantiated multiple times); make orderService a volatile
field and change orders() to use double-checked locking (check null,
synchronized(this) block, re-check null, then assign) or alternatively make the
accessor synchronized to ensure a single instance; apply the same pattern to the
other lazily-initialized service fields instantiated in the same way elsewhere
in this class (the other service fields initialized like orderService).
In `@src/main/java/ee/bitweb/montonio/sdk/order/OrderService.java`:
- Around line 15-20: The get method in OrderService should normalize the uuid
(trim whitespace) before validation and before concatenating into the request
path to avoid unchecked URI errors; update OrderService.get to first assign a
trimmed version of the input (e.g., normalizedUuid = uuid == null ? null :
uuid.trim()), validate normalizedUuid and throw MontonioValidationException when
null/blank, and then call httpClient.get("/orders/" + normalizedUuid,
OrderResponse.class) so the request path never contains leading/trailing spaces.
🪄 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: e5af1d7b-24ab-46cf-8e2e-13bd64366d95
📒 Files selected for processing (6)
docs/plans/2026-04-10-get-order-status-design.mdsrc/main/java/ee/bitweb/montonio/sdk/MontonioClient.javasrc/main/java/ee/bitweb/montonio/sdk/http/MontonioHttpClient.javasrc/main/java/ee/bitweb/montonio/sdk/order/OrderService.javasrc/test/java/ee/bitweb/montonio/sdk/MontonioClientTest.javasrc/test/java/ee/bitweb/montonio/sdk/order/OrderServiceTest.java
- Add language identifier to fenced code block in design doc (MD040) - Make orders() thread-safe with volatile + double-checked locking - Trim UUID before validation and path construction to prevent whitespace in request URIs - Add test for UUID trimming behavior Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/test/java/ee/bitweb/montonio/sdk/order/OrderServiceTest.java (1)
120-125: Strengthen the empty-UUID test to assert the exception contract, not only the type.For consistency with the null/blank tests, also verify
field == "uuid"(and optionally message content).Suggested patch
`@Test` void getWithEmptyUuidThrowsValidationException() { OrderService service = createServiceWithStub(200, "{}"); - - assertThrows(MontonioValidationException.class, () -> service.get("")); + MontonioValidationException exception = assertThrows( + MontonioValidationException.class, + () -> service.get("") + ); + assertEquals("uuid", exception.getField()); + assertTrue(exception.getMessage().contains("must not be null or blank")); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/ee/bitweb/montonio/sdk/order/OrderServiceTest.java` around lines 120 - 125, The test getWithEmptyUuidThrowsValidationException should assert the exception's validation contract, not just its type: capture the thrown MontonioValidationException from OrderService.get("") (same flow as in getWithNullUuidThrowsValidationException/getWithBlankUuidThrowsValidationException), then assert that exception.getField() equals "uuid" (and optionally assert message content). Update the test method getWithEmptyUuidThrowsValidationException to store the thrown exception and add the field assertion to match the other UUID validation tests.
🤖 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/ee/bitweb/montonio/sdk/MontonioClient.java`:
- Around line 18-20: The package-private constructor
MontonioClient(MontonioHttpClient httpClient) should guard against a null
httpClient to fail fast; update the MontonioClient constructor to validate the
incoming httpClient and throw a clear exception (e.g., NullPointerException or
IllegalArgumentException with a message like "httpClient must not be null") when
null is passed so that faults are caught at construction time instead of later
in orders() or other call paths.
In `@src/main/java/ee/bitweb/montonio/sdk/order/OrderService.java`:
- Around line 11-13: The OrderService constructor should validate the injected
dependency and fail fast: check the constructor parameter httpClient in
OrderService and if it is null throw a clear exception (e.g.,
IllegalArgumentException or NullPointerException) with a descriptive message so
construction fails immediately rather than later in get(...); update the
constructor to perform this null-check before assigning to this.httpClient.
---
Nitpick comments:
In `@src/test/java/ee/bitweb/montonio/sdk/order/OrderServiceTest.java`:
- Around line 120-125: The test getWithEmptyUuidThrowsValidationException should
assert the exception's validation contract, not just its type: capture the
thrown MontonioValidationException from OrderService.get("") (same flow as in
getWithNullUuidThrowsValidationException/getWithBlankUuidThrowsValidationException),
then assert that exception.getField() equals "uuid" (and optionally assert
message content). Update the test method
getWithEmptyUuidThrowsValidationException to store the thrown exception and add
the field assertion to match the other UUID validation tests.
🪄 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: 48b67464-8464-45e1-b23f-6bc54de01a58
📒 Files selected for processing (4)
docs/plans/2026-04-10-get-order-status-design.mdsrc/main/java/ee/bitweb/montonio/sdk/MontonioClient.javasrc/main/java/ee/bitweb/montonio/sdk/order/OrderService.javasrc/test/java/ee/bitweb/montonio/sdk/order/OrderServiceTest.java
✅ Files skipped from review due to trivial changes (1)
- docs/plans/2026-04-10-get-order-status-design.md
- Fail fast on null httpClient in MontonioClient and OrderService constructors - Strengthen empty UUID test to assert field name and message content Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
MontonioClientas the top-level SDK entry point with domain-scoped access:client.orders().get(uuid)OrderService.get(uuid)validates null/blank input and delegatesGET /orders/{uuid}to the HTTP clientorders()createsOrderServiceon first call, reuses it afterMontonioHttpClientthree-arg constructor topublicfor cross-package testabilityNew Issues Created
OrderServiceTest plan
MontonioClientTest— null config rejection,orders()returns non-null, lazy-caching returns same instanceOrderServiceTest— successful retrieval, null/blank/empty UUID validation, 404 error, multiple payment intents, all payment statuses via parameterized test./gradlew build)Closes #16
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests
Documentation