Skip to content

Add immutable builder-based SDK configuration#22

Merged
rammrain merged 5 commits into
mainfrom
feature/sdk-configuration
Apr 10, 2026
Merged

Add immutable builder-based SDK configuration#22
rammrain merged 5 commits into
mainfrom
feature/sdk-configuration

Conversation

@rammrain
Copy link
Copy Markdown
Member

@rammrain rammrain commented Apr 10, 2026

Summary

  • Expand MontonioSdkConfiguration into an immutable, builder-constructed configuration object with credentials (accessKey, secretKey), HTTP timeouts (connectTimeout, requestTimeout), token expiration, and environment URL constants
  • Validate required fields (accessKey, secretKey) at build time via MontonioValidationException
  • Replace mutable @Getter/@Setter POJO with Lombok @Builder + @Getter (immutable after construction)

Closes #10

Test plan

  • Builder with only required fields applies correct defaults (sandbox URL, 10s connect, 30s request, 5min token)
  • Builder with all fields overridden returns custom values
  • URL constants have correct values
  • Null accessKey/secretKey throws MontonioValidationException
  • Blank accessKey/secretKey throws MontonioValidationException
  • All existing tests still pass (28 total)

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • SDK configuration is now immutable and created via a builder for safer, predictable setup.
    • Added a production base URL constant with a sandbox default for easy environment switching.
    • Configurable HTTP timeouts and token lifetime with sensible defaults.
  • Bug Fixes / Validation

    • Credentials and timing fields are validated at build time with clear errors for missing/invalid values.
  • Tests

    • Unit tests cover defaults, overrides, URL constants and validation failure cases.
  • Documentation

    • New design doc with usage examples for minimal, production and multi-instance configurations.

Expand MontonioSdkConfiguration with credentials, timeouts, and token
expiration. The class is now immutable and constructed via Lombok @builder
with validation that accessKey and secretKey are required.

Closes #10

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 10, 2026

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

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: 397be554-7fd2-4a4f-bf54-87eac16c0f84

📥 Commits

Reviewing files that changed from the base of the PR and between 4c8d6ba and 6b71c42.

📒 Files selected for processing (2)
  • src/main/java/ee/bitweb/montonio/sdk/MontonioSdkConfiguration.java
  • src/test/java/ee/bitweb/montonio/sdk/MontonioSdkConfigurationTest.java
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/main/java/ee/bitweb/montonio/sdk/MontonioSdkConfiguration.java

📝 Walkthrough

Walkthrough

Immutable Lombok @Builder configuration MontonioSdkConfiguration added/updated: explicit accessKey/secretKey, sandbox/production URL constants, Duration-based connectTimeout/requestTimeout/tokenExpirationTime, validation that fails fast for missing/blank credentials, a new design doc, and expanded unit tests. (50 words)

Changes

Cohort / File(s) Summary
Design Specification
docs/plans/2026-04-10-sdk-configuration-design.md
Added design doc describing immutable builder-based SDK configuration, default values, validation rules, SANDBOX_BASE_URL / PRODUCTION_BASE_URL constants, usage examples, and a test plan.
Configuration Implementation
src/main/java/ee/bitweb/montonio/sdk/MontonioSdkConfiguration.java
Refactored to @Getter + @Builder immutable class. Introduced private final fields: accessKey, secretKey, baseUrl, connectTimeout, requestTimeout, tokenExpirationTime; added PRODUCTION_BASE_URL; constructor validates non-null/non-blank credentials and non-null/non-negative Durations; defaults applied via builder.
Test Coverage
src/test/java/ee/bitweb/montonio/sdk/MontonioSdkConfigurationTest.java
Expanded tests to assert builder defaults, overridden values, URL constant literals, and validation failures for null/blank credentials, null baseUrl/Duration fields, and negative duration values; checks MontonioValidationException field mappings.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~40 minutes

Poem

🐰✨ I nibbled through defaults and keys,
Builder hops quick on sandbox breeze;
Timeouts set and URLs align,
Credentials checked — each field fine.
Hop forth, merchants — configure with glee!

🚥 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 'Add immutable builder-based SDK configuration' accurately and concisely describes the primary change: converting the mutable MontonioSdkConfiguration to an immutable builder-based implementation.
Linked Issues check ✅ Passed The PR implements most core requirements from issue #10: builder-based construction, configurable timeouts (connectTimeout, requestTimeout), token expiration, and environment URL constants. Multi-merchant credential registry was intentionally omitted with documented YAGNI reasoning.
Out of Scope Changes check ✅ Passed All changes are directly scoped to the linked issue #10 objectives. The design document, configuration class, and tests all focus on immutable builder-based SDK configuration with no extraneous 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 feature/sdk-configuration

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

Copy link
Copy Markdown

@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.

🧹 Nitpick comments (1)
docs/plans/2026-04-10-sdk-configuration-design.md (1)

18-18: Optional: Minor hyphenation style.

The static analysis flagged "rarely-changed" — compound modifiers with adverbs ending in "-ly" typically don't require hyphenation ("rarely changed URL"). This is a trivial style nit and entirely optional to address.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/plans/2026-04-10-sdk-configuration-design.md` at line 18, In the
"Environment selection" table row that currently reads "Simple, flexible; enum
wrapper adds little value for a rarely-changed URL", remove the unnecessary
hyphen and change "rarely-changed URL" to "rarely changed URL" so the phrase is
unhyphenated; update that cell text accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@docs/plans/2026-04-10-sdk-configuration-design.md`:
- Line 18: In the "Environment selection" table row that currently reads
"Simple, flexible; enum wrapper adds little value for a rarely-changed URL",
remove the unnecessary hyphen and change "rarely-changed URL" to "rarely changed
URL" so the phrase is unhyphenated; update that cell text accordingly.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 131166f6-c215-47d7-abfa-95d75c3da02a

📥 Commits

Reviewing files that changed from the base of the PR and between c1befdf and 5f679d4.

📒 Files selected for processing (3)
  • docs/plans/2026-04-10-sdk-configuration-design.md
  • src/main/java/ee/bitweb/montonio/sdk/MontonioSdkConfiguration.java
  • src/test/java/ee/bitweb/montonio/sdk/MontonioSdkConfigurationTest.java

Fix hyphenation style nit in design doc: "rarely-changed" → "rarely changed".

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@rammrain
Copy link
Copy Markdown
Member Author

Re: multi-merchant credential support — This was intentionally scoped out during design. Consumers who need multiple merchants simply create multiple MontonioSdkConfiguration instances. A built-in registry can be added as a later enhancement if there's demand, but YAGNI for now. This is documented in the design doc under "Design Decisions".

Re: docstring coverage — The project currently has no Javadoc on any class (including the exception hierarchy merged in #21). Adding docstrings is out of scope for this configuration PR. Happy to address project-wide in a separate issue if desired.

rammrain and others added 2 commits April 10, 2026 06:28
Remove custom builder class — let Lombok's generated build() handle
@Builder.Default values. Validation moves to a package-private all-args
constructor that Lombok's builder calls directly.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Reject explicit nulls for baseUrl, connectTimeout, requestTimeout, and
tokenExpirationTime — prevents callers from overriding @Builder.Default
values with null.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@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: 1

🤖 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/MontonioSdkConfiguration.java`:
- Around line 48-55: In MontonioSdkConfiguration, after the existing null checks
for connectTimeout, requestTimeout, and tokenExpirationTime, add validation to
reject negative durations by calling duration.isNegative() on each
(connectTimeout, requestTimeout, tokenExpirationTime) and throw
MontonioValidationException with the same field name and a message like "must
not be negative" when true; ensure these checks are placed immediately after the
corresponding null checks so invalid negative Duration.ofSeconds(-1) values fail
fast during configuration.
🪄 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: 5fe450e8-56e6-4491-890e-2d1788c23124

📥 Commits

Reviewing files that changed from the base of the PR and between 33f8aef and 4c8d6ba.

📒 Files selected for processing (2)
  • src/main/java/ee/bitweb/montonio/sdk/MontonioSdkConfiguration.java
  • src/test/java/ee/bitweb/montonio/sdk/MontonioSdkConfigurationTest.java

Comment thread src/main/java/ee/bitweb/montonio/sdk/MontonioSdkConfiguration.java Outdated
Fail fast on negative connectTimeout, requestTimeout, and
tokenExpirationTime instead of deferring the error to the HTTP/auth layer.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@rammrain rammrain merged commit dbe4b7e into main Apr 10, 2026
5 checks passed
@rammrain rammrain deleted the feature/sdk-configuration branch April 10, 2026 06:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Core configuration and multi-merchant credential support

1 participant