Add Bottin integration with NIP-05 support and JDK 21 migration #4
Conversation
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
Implements Service Provider Interface (SPI) pattern to allow external projects like bottin to provide persistent implementations of NIP-05 and account management. Changes: - Add Nip05ManagerProvider and AccountManagerProvider interfaces - Add DefaultNip05ManagerProvider and DefaultAccountManagerProvider (priority 0) - Enhance Nip05Manager with persistence methods (findByNip05, findByDomain, etc.) - Enhance AccountManager with lookup methods (findAccount, exists, etc.) - Add helper methods to Nip05Record (getRelays, getUsername, getDomain, fromBunkerKey) - Add NIP-05 configuration properties to Spring Boot starter - Update auto-configuration for provider selection by priority - Add nsecbunker-account dependency to Spring Boot starter - Create integration documentation for bottin 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Add detailed documentation explaining how nsecbunker-java leverages bottin's NIP-05 functionalities through the SPI pattern: - Auto-configuration flow diagram showing provider injection - Provider selection logic with priority-based auto-selection - NIP-05 operations table with method descriptions - Transparent usage example demonstrating database-backed operations - Troubleshooting section for verifying bottin integration is active - Domain verification requirements for setupNip05() 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Add org.testcontainers:postgresql dependency to nsecbunker-e2e module for Bottin e2e tests that require a PostgreSQL database. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Add comprehensive end-to-end tests for Bottin integration using Testcontainers. Tests cover: - Domain management (create, list, get, delete) - NIP-05 record CRUD operations - Well-known endpoint NIP-05 spec compliance - Error handling (duplicate records, invalid pubkey) Components added: - BottinContainer: Testcontainer for docker.398ja.xyz/bottin image - BottinTestClient: HTTP client for Bottin REST API - BottinE2ETest: 18 test scenarios with ordered execution Requires Bottin to support BOTTIN_VERIFICATION_SKIP=true for auto-verifying domains in test mode. Run with: mvn -Pe2e test -Dtest=BottinE2ETest 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Add Maven property to control Bottin domain verification skip mode: - Default: true (skip verification in tests) - Override with: -Dbottin.verification.skip=false Property is passed to tests via Surefire systemPropertyVariables and read by BottinContainer at initialization. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Update all module versions from 0.1.0-SNAPSHOT to 0.1.0 for the initial release. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Update BottinContainer to use docker.398ja.xyz/bottin-web:latest instead of docker.398ja.xyz/bottin:latest. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Add required environment variables for Bottin to use PostgreSQL: - SPRING_DATASOURCE_DRIVER_CLASS_NAME=org.postgresql.Driver - SPRING_JPA_DATABASE_PLATFORM=org.hibernate.dialect.PostgreSQLDialect - SPRING_FLYWAY_ENABLED=false (use Hibernate DDL auto instead) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Bottin's REST API requires authentication. Updated: - BottinTestClient: Added constructor with username/password for Basic Auth, adds Authorization header to API requests - BottinContainer: Added admin credentials configuration via BOTTIN_ADMIN_USERNAME and BOTTIN_ADMIN_PASSWORD env vars - BottinE2ETest: Use authenticated client with admin credentials Public endpoints (health, well-known) remain unauthenticated. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Enable Flyway migrations (SPRING_FLYWAY_ENABLED=true) instead of Hibernate DDL auto to properly create database schema - Add forceVerifyDomain() helper that directly updates PostgreSQL to mark domains as verified (bypasses normal verification flow) - Update shouldRegisterNewDomain test to force-verify domain after creation, enabling NIP-05 record tests to proceed Bottin requires verified domains before creating NIP-05 records. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Add @tag("bottin") to BottinE2ETest and create e2e-bottin profile that filters tests by this tag. Run Bottin tests only: mvn -Pe2e-bottin test -pl nsecbunker-tests/nsecbunker-e2e 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
There was a problem hiding this comment.
Pull request overview
This pull request introduces Bottin integration for persistent NIP-05 identity management, implements a Service Provider Interface (SPI) pattern for pluggable account and NIP-05 managers, migrates the codebase from JDK 17 to JDK 21, and releases version 0.1.0. The changes establish a foundation for extensible identity management while maintaining backward compatibility through sensible defaults.
Key changes:
- Implementation of SPI pattern with AccountManagerProvider and Nip05ManagerProvider interfaces allowing pluggable storage backends
- Comprehensive e2e test suite for Bottin integration using Testcontainers with PostgreSQL
- Enhanced domain models (Nip05Record, AccountManager, Nip05Manager) with additional query and management methods
- Spring Boot auto-configuration enhancements for provider selection based on priority
- Version bump to 0.1.0 across all Maven modules
Reviewed changes
Copilot reviewed 34 out of 34 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| pom.xml (multiple) | Version update from 0.1.0-SNAPSHOT to 0.1.0 release across all modules |
| BottinContainer.java | New Testcontainer implementation for Bottin service with PostgreSQL configuration |
| BottinTestClient.java | HTTP client for e2e tests providing domain and NIP-05 record management operations |
| BottinE2ETest.java | Comprehensive e2e test suite verifying domain management, NIP-05 CRUD operations, and well-known endpoint compliance |
| AccountManagerProvider.java | New SPI for pluggable account storage backends with priority-based selection |
| Nip05ManagerProvider.java | New SPI for pluggable NIP-05 storage backends with priority-based selection |
| DefaultAccountManagerProvider.java | Default in-memory implementation of AccountManagerProvider |
| DefaultNip05ManagerProvider.java | Default in-memory implementation of Nip05ManagerProvider |
| Nip05Record.java | Enhanced with helper methods for relay parsing, username/domain extraction, and factory methods |
| AccountManager.java | Extended interface with query methods (findAccount, findByUsername, findByDomain, exists, deleteAccount, count) |
| Nip05Manager.java | Extended interface with query methods (findByNip05, findByDomain, findByPubkey, deleteNip05, updateRelays, count) |
| NsecBunkerAutoConfiguration.java | Added bean creation for AccountManager and Nip05Manager with provider selection logic |
| NsecBunkerProperties.java | Added Nip05 nested configuration class for NIP-05 settings |
| bottin.md | New comprehensive integration guide for Bottin with usage examples and troubleshooting |
| README.md | Updated CI badge URL and JDK version from 17 to 21 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...-boot-starter/src/main/java/xyz/tcheeric/nsecbunker/starter/NsecBunkerAutoConfiguration.java
Show resolved
Hide resolved
Comment on lines
61
to
72
| /** | ||
| * Finds an account by username. | ||
| * | ||
| * <p>Default implementation returns empty. Persistent implementations | ||
| * should override this method to provide username-based lookup.</p> | ||
| * | ||
| * @param username username to search for | ||
| * @return the account if found, empty otherwise | ||
| */ | ||
| default CompletableFuture<Optional<AccountRegistrationResult>> findByUsername(String username) { | ||
| return CompletableFuture.completedFuture(Optional.empty()); | ||
| } |
There was a problem hiding this comment.
The default implementation of findByUsername searches by username alone, but usernames are not unique across domains. A user "alice" on "example.com" and "alice" on "another.com" are different accounts. This method could return ambiguous results when the same username exists on multiple domains. Consider either requiring both username and domain parameters, or documenting that this returns the first match and may be ambiguous in multi-domain scenarios.
Comment on lines
+409
to
+410
| // Assert | ||
| assertThat(response.statusCode()).isEqualTo(200); |
There was a problem hiding this comment.
The toggle logic in this test is problematic. The comment says "ensure record is enabled" but then toggles without checking the current state. This could leave the record in either state (enabled or disabled) depending on what happened in previous tests. This will cause test flakiness.
Instead, should verify the current state first and only toggle if needed to ensure the record is enabled before testing the well-known endpoint.
Comment on lines
64
to
68
| try { | ||
| return DEFAULT_MAPPER.readValue(relaysJson, STRING_LIST_TYPE); | ||
| } catch (JsonProcessingException e) { | ||
| return List.of(); | ||
| } |
There was a problem hiding this comment.
The getRelays method silently swallows JsonProcessingException and returns an empty list. This could hide data corruption or invalid JSON in the relaysJson field. Consider logging the exception at least at debug level to aid troubleshooting, especially since this is returning parsed data that callers may rely on.
nsecbunker-account/src/main/java/xyz/tcheeric/nsecbunker/account/nip05/Nip05Record.java
Outdated
Show resolved
Hide resolved
| # nsecbunker-java | ||
|
|
||
| [](https://github.com/tcheeric/nsecbunker-java/actions/workflows/ci.yml) | ||
| [](https://github.com/398ja/nsecbunker-java/actions/workflows/ci.yml) |
There was a problem hiding this comment.
The GitHub repository URL in the CI badge has changed from 'tcheeric' to '398ja'. Ensure this repository actually exists at the new URL and that the CI workflow is properly configured there. If this is a typo or temporary URL, it should be corrected before release.
Suggested change
| [](https://github.com/398ja/nsecbunker-java/actions/workflows/ci.yml) | |
| [](https://github.com/tcheeric/nsecbunker-java/actions/workflows/ci.yml) |
nsecbunker-tests/nsecbunker-e2e/src/test/java/xyz/tcheeric/nsecbunker/e2e/BottinE2ETest.java
Show resolved
Hide resolved
Comment on lines
+77
to
+96
| public String getUsername() { | ||
| if (nip05 == null || !nip05.contains("@")) { | ||
| return null; | ||
| } | ||
| return nip05.split("@")[0]; | ||
| } | ||
|
|
||
| /** | ||
| * Extracts the domain portion of the NIP-05 identifier. | ||
| * | ||
| * @return the domain (e.g., "example.com" from "[email protected]") | ||
| */ | ||
| @JsonIgnore | ||
| public String getDomain() { | ||
| if (nip05 == null || !nip05.contains("@")) { | ||
| return null; | ||
| } | ||
| String[] parts = nip05.split("@"); | ||
| return parts.length > 1 ? parts[1] : null; | ||
| } |
There was a problem hiding this comment.
The getUsername and getDomain methods use String.split("@") without proper handling of multiple @ symbols. If nip05 contains multiple @ characters (e.g., "alice@[email protected]"), this will incorrectly parse the username and domain. Consider using indexOf and substring for more robust parsing, or validating the NIP-05 format to ensure it contains exactly one @ symbol.
...sts/nsecbunker-e2e/src/test/java/xyz/tcheeric/nsecbunker/e2e/containers/BottinContainer.java
Outdated
Show resolved
Hide resolved
Comment on lines
32
to
33
| public DefaultNip05ManagerProvider() { | ||
| this(new DefaultAccountManager(), new ObjectMapper()); |
There was a problem hiding this comment.
The no-arg constructor creates a new DefaultAccountManager instance which is then potentially replaced in the full constructor (line 52). This means two AccountManager instances may be created unnecessarily. Consider having the no-arg constructor delegate directly to the full constructor with null values, letting the null check happen once.
Qodana for JVM23 new problems were found
☁️ View the detailed Qodana report Detected 74 dependenciesThird-party software listThis page lists the third-party software dependencies used in project
Contact Qodana teamContact us at [email protected]
|
Code quality fixes: - Make RelayConnection and RelayPool final (CT_CONSTRUCTOR_THROW) - Make AdminConnectionListener static inner class - Fix similar log messages in RelayPool, alerting classes - Fix MismatchedCollectionQueryUpdate in WebhookAlertDelivery, RelayPool - Add @serial annotations to exception classes (MissingSerialAnnotation) - Remove dead local store in RelayHealthMonitor (DLS_DEAD_LOCAL_STORE) - Fix SQL injection pattern warning in BottinE2ETest - Fix duplicated JSON serialization in Nip05Record - Remove unused imports and code across modules Dependency updates for CVE fixes: - Spring Boot 3.2.4 -> 3.5.9 - JUnit 5.10.2 -> 5.12.2 - Logback 1.5.3 -> 1.5.15 - Micrometer 1.12.4 -> 1.14.3 - maven-pmd-plugin 3.21.2 -> 3.26.0 (Java 21 support) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 107 out of 107 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
This pull request introduces several updates and features, including:
Related Issue
Fixes #
Type of Change
Checklist
Testing
Test coverage includes both unit and comprehensive end-to-end tests for Bottin integration, verifying domain verification, NIP-05 record management, and well-known endpoint compliance. The updates have been verified using Testcontainers and Maven configurations for e2e profiles.
Screenshots (if applicable)
Additional Notes
This PR represents an essential milestone with initial version release (0.1.0) and significant groundwork for robust Bottin integration. Further enhancements and additional features may follow in subsequent releases.