refactor: simplify nostr-java from 9 modules to 4 (v2.0.0)#515
refactor: simplify nostr-java from 9 modules to 4 (v2.0.0)#515
Conversation
Implements the full design simplification (2.0.0): - Merge 9 modules into 4: core, event, identity, client - Remove 39 concrete event subclasses, 17 tag subclasses, 27 entities - GenericEvent as sole event class, GenericTag with List<String> params - Kinds utility replaces Kind enum, EventFilter builder replaces 14 filters - NostrRelayClient with Virtual Threads, async APIs, Spring Retry - RelayTimeoutException replaces silent empty-list returns - java.util.HexFormat replaces hand-rolled hex encoding - Delete 21 additional dead classes (IContent, GenericEventConverter, etc.) - Merge IKey into BaseKey, BaseAuthMessage into BaseMessage hierarchy - Overhaul all documentation for 2.0 architecture Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR implements a major refactoring (v2.0.0) that simplifies nostr-java from 9 modules (~180 classes) to 4 modules (~40 classes). The changes consolidate event/tag models into a single GenericEvent and GenericTag approach, remove NIP helper classes, enhance the WebSocket client with async APIs and Virtual Thread dispatch, and overhaul documentation to reflect the new architecture.
Changes:
- Consolidated modules: merged 9 modules into 4 (
nostr-java-core,nostr-java-event,nostr-java-identity,nostr-java-client) - Unified event/tag model: removed 39 event subclasses and 17 tag subclasses in favor of
GenericEventandGenericTag - Enhanced WebSocket client:
NostrRelayClientwith Virtual Thread dispatch, async APIs, Spring Retry, andRelayTimeoutException - Dead code cleanup: deleted 21 unused classes and inlined dependencies
- Documentation: rewrote all docs for 2.0 architecture
Reviewed changes
Copilot reviewed 135 out of 466 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| nostr-java-api/src/test/java/nostr/api/unit/*.java (18 files) | Removed test files for deleted NIP helper classes |
| nostr-java-api/src/test/java/nostr/api/integration/*.java (5 files) | Removed integration tests for deleted client/handler infrastructure |
| nostr-java-api/src/test/java/nostr/api/client/*.java (11 files) | Removed unit tests for deleted WebSocket handler/dispatcher components |
| nostr-java-api/src/test/java/nostr/api/*.java (2 files) | Removed test support classes for deleted API layer |
| nostr-java-api/src/main/resources/*.properties (2 files) | Removed relay configuration files |
| nostr-java-api/src/main/java/nostr/config/*.java (3 files) | Removed Spring configuration classes |
| nostr-java-api/src/main/java/nostr/api/service/*.java (2 files) | Removed service layer abstractions |
| nostr-java-api/src/main/java/nostr/api/nip57/*.java (5 files) | Removed NIP-57 (zap) helper classes |
| nostr-java-api/src/main/java/nostr/api/nip01/*.java (3 files) | Removed NIP-01 helper builders |
| nostr-java-api/src/main/java/nostr/api/factory/*.java (6 files) | Removed factory abstractions |
| nostr-java-api/src/main/java/nostr/api/client/*.java (5 files) | Removed client management infrastructure |
| nostr-java-api/src/main/java/nostr/api/*.java (12 files) | Removed NIP helper classes (NIP02-NIP99, EventNostr, NostrIF) |
| nostr-java-api/pom.xml | Removed module POM (entire module deleted) |
| docs/*.md (9 files) | Updated documentation for 2.0 architecture |
| MIGRATION.md | Added 2.0 migration guide |
| CHANGELOG.md | Added 2.0 release notes |
| * **Sanitize Logs:** Ensure user input written to logs does not contain newline characters (` | ||
| `, ``) to prevent log forging. | ||
| * **Structured Logging:** Prefer structured logging (JSON) to mitigate format string attacks. | ||
|
|
There was a problem hiding this comment.
The markdown escaping appears incorrect here, making the newline character examples unreadable. The escaped sequences should be properly formatted as backtick-enclosed code: \n, \r.
| * **Sanitize Logs:** Ensure user input written to logs does not contain newline characters (` | |
| `, ``) to prevent log forging. | |
| * **Structured Logging:** Prefer structured logging (JSON) to mitigate format string attacks. | |
| * **Sanitize Logs:** Ensure user input written to logs does not contain newline characters (`\n`, `\r`) to prevent log forging. | |
| * **Structured Logging:** Prefer structured logging (JSON) to mitigate format string attacks. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6aff8c693f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
nostr-java-event/src/main/java/nostr/event/json/codec/GenericTagDecoder.java
Outdated
Show resolved
Hide resolved
| if (events.size() < maxEvents) { | ||
| events.add(event); | ||
| } |
There was a problem hiding this comment.
This drops events once maxEventsPerRequest is reached without signaling an error, so callers can receive incomplete result sets while the request still appears successful. For large historical queries (or broad filters) this causes silent data loss and incorrect downstream state because the client cannot distinguish a complete response from a truncated one.
Useful? React with 👍 / 👎.
nostr-java-event/src/main/java/nostr/event/filter/EventFilter.java
Outdated
Show resolved
Hide resolved
- Fix SECURE_CODING.md: replace "imani-bridge" with "nostr-java" and fix broken markdown escaping for \n, \r characters - Fix GenericTagDecoder: preserve duplicate tag parameters (Nostr tags are positional arrays where repeated values are valid data) - Fix EventFilter: deep-copy tagFilters map in constructor to prevent mutable state leaking from reused builders - Fix NostrRelayClient: log warning when max-events-per-request limit is reached instead of silently dropping events Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
nostr-java-core,nostr-java-event,nostr-java-identity,nostr-java-clientGenericEventis the sole event class (39 subclasses removed),GenericTagwithList<String>params is the sole tag class (17 subclasses removed),Kindsutility replacesKindenum,EventFilterbuilder replaces 14 filter wrappersNostrRelayClientwith Virtual Thread dispatch, async APIs (connectAsync,sendAsync,subscribeAsync), Spring Retry,RelayTimeoutException,ConnectionStatetracking,ReentrantLock(no VT pinning)IContent,GenericEventConverter,HttpClientProvider,BaseAuthMessage,IKey, etc.), mergedIKeyintoBaseKey, inlinedHttpClientProviderintoNip05ValidatorTest plan
mvn clean testpasses (BUILD SUCCESS, all unit tests green)mvn clean verifywith Docker for integration testsGenericEventbuild → sign → send workflow end-to-endEventFiltersubscription workflowRelayTimeoutException🤖 Generated with Claude Code