Develop #483
Merged
Develop #483
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
Complete changelog documenting 187 commits across 9 months: - Major features: Streaming subscriptions, BOM migration, docs overhaul - Technical improvements: Refactoring, NIP-05 enhancement, CI/CD - 387 files changed, +18,150/-13,754 lines - Maintained 100% backward API compatibility 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
…mented code, generics)
…dundant casts, imports)
…tags, messages, and event impls
…lify relays Map injection in IT; refactor: use typed tag helpers fallback in NIP04/NIP44/NIP57
refactor: revamp docs, align BOM JUnit, add typed tag helpers, and bump to 0.6.0
…idelines High-priority logging fixes based on comprehensive code review: 1. Fixed empty error message in UserProfile.java - Added descriptive error message and context to exception - Now properly describes Bech32 conversion failure 2. Improved generic warning in GenericEvent.java - Changed from log.warn(ex.getMessage()) to include full context - Added exception stacktrace for better debugging - Message now explains serialization failure context 3. Optimized expensive debug logging in GenericEvent.java - Changed serialized event logging from DEBUG to TRACE level - Added guard with log.isTraceEnabled() to prevent unnecessary String creation - Reduces performance overhead when TRACE is disabled 4. Fixed inappropriate INFO level in GenericTagDecoder.java - Changed log.info to log.debug for routine decoding operation - INFO should be for noteworthy events, not expected operations 5. Added comprehensive LOGGING_REVIEW.md - Documents all logging practices against Clean Code principles - Identifies 8 priority levels of improvements - Overall grade: B+ (will be A- after all high-priority fixes) Compliance with Clean Code chapters 2, 3, 4, 7, 10, 17: - Meaningful error messages (Ch 2: Meaningful Names) - Proper context in logs (Ch 4: Comments) - Better error handling (Ch 7: Error Handling) - Reduced code smells (Ch 17: Smells and Heuristics) Ref: LOGGING_REVIEW.md for complete analysis and remaining action items 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Medium-priority logging improvements per Clean Code Chapter 3 (Functions): 1. Removed constructor logging from PrivateKey class - Removed "Created private key from byte array" debug log - Removed "Created private key from hex string" debug log - Removed "Generated new random private key" debug log - Simplified generateRandomPrivKey() to single return statement 2. Removed constructor logging from PublicKey class - Removed "Created public key from byte array" debug log - Removed "Created public key from hex string" debug log 3. Removed routine operation logging from BaseKey class - Removed "Converted key to Bech32" debug log in toBech32String() - Removed "Converted key to hex string" debug log in toHexString() - Simplified methods to single return statements 4. Enhanced error logging in BaseKey.toBech32String() - Added key type and prefix to error message for better context - Improved exception message to include original error Rationale: - Low-level data container classes should not log object creation - These classes are used frequently, logging creates noise - Constructor logging violates Single Responsibility Principle - If object creation tracking is needed, use profiler/instrumentation - Application layer should handle logging when appropriate Performance impact: - Reduces log overhead for frequently created objects - Eliminates unnecessary string formatting on every key creation Compliance: - Ch 3: Functions do one thing (no logging side effects) - Ch 10: Classes have single responsibility (data, not logging) - Ch 17: Eliminates logging "code smell" in utilities Ref: LOGGING_REVIEW.md sections 2, 5, and 8 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
…ines
Low-priority cleanup per LOGGING_REVIEW.md recommendations:
Removed all log.info("testMethodName") statements from test files across
the entire codebase (89 total removals):
- nostr-java-event: 58 removals
- nostr-java-api: 26 removals
- nostr-java-id: 4 removals
- nostr-java-util: 1 removal
Rationale (Clean Code Ch 17: Code Smells - G15 Selector Arguments):
- Test method names are already visible in JUnit test output
- Logging test names adds noise without value
- JUnit @DisplayName annotation is the proper way to add readable test names
- Reduces unnecessary log output during test execution
Example of proper approach (if needed):
```java
@test
@DisplayName("Event filter encoder should serialize filters correctly")
void testEventFilterEncoder() {
// test code
}
```
Performance impact:
- Eliminates 89 unnecessary log calls during test execution
- Cleaner test output
Compliance:
- Ch 17: Removes code smell (unnecessary logging in tests)
- Ch 4: Tests are self-documenting without log statements
Ref: LOGGING_REVIEW.md section 6 (Code Smells - G15)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
…cketClient Low-priority refactoring per LOGGING_REVIEW.md recommendations: Extracted duplicated recovery logging into reusable helper methods: 1. Added logRecoveryFailure(String operation, int size, IOException ex) - Handles simple recovery failures (send message, subscribe) - Reduces duplication in recover() and recoverSubscription() methods 2. Added logRecoveryFailure(String operation, String command, int size, IOException ex) - Handles recovery failures with command context - Used for BaseMessage recovery methods Benefits: - Reduces code duplication (4 nearly identical log statements → 2 helper methods) - Makes recovery logging consistent across all methods - Easier to maintain and update logging format in one place - Follows DRY (Don't Repeat Yourself) principle Compliance: - Ch 17: Eliminates code smell (G5 - Duplication) - Ch 3: Functions do one thing (separate logging concern) - Ch 10: Single Responsibility (logging extracted to helper) Before: 4 duplicated log.error() calls with similar patterns After: 2 reusable helper methods, 4 one-line calls Ref: LOGGING_REVIEW.md section 6 (Code Smells - G5) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Update version from 0.6.0 to 0.6.1 across all modules: - Root pom.xml (project version and nostr-java.version property) - All 9 module pom.xml files Changes in this release: - Fixed empty error messages and improved log context - Removed constructor logging from low-level key classes - Optimized expensive debug logging with guards - Fixed inappropriate log levels (INFO → DEBUG where needed) - Removed 89 test method name log statements - Extracted duplicated recovery logging Logging compliance improved from B+ to A- per Clean Code guidelines. Ref: LOGGING_REVIEW.md for complete analysis 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
chore/bump-version-0.6.1
…idelines High-priority logging fixes based on comprehensive code review: 1. Fixed empty error message in UserProfile.java - Added descriptive error message and context to exception - Now properly describes Bech32 conversion failure 2. Improved generic warning in GenericEvent.java - Changed from log.warn(ex.getMessage()) to include full context - Added exception stacktrace for better debugging - Message now explains serialization failure context 3. Optimized expensive debug logging in GenericEvent.java - Changed serialized event logging from DEBUG to TRACE level - Added guard with log.isTraceEnabled() to prevent unnecessary String creation - Reduces performance overhead when TRACE is disabled 4. Fixed inappropriate INFO level in GenericTagDecoder.java - Changed log.info to log.debug for routine decoding operation - INFO should be for noteworthy events, not expected operations 5. Added comprehensive LOGGING_REVIEW.md - Documents all logging practices against Clean Code principles - Identifies 8 priority levels of improvements - Overall grade: B+ (will be A- after all high-priority fixes) Compliance with Clean Code chapters 2, 3, 4, 7, 10, 17: - Meaningful error messages (Ch 2: Meaningful Names) - Proper context in logs (Ch 4: Comments) - Better error handling (Ch 7: Error Handling) - Reduced code smells (Ch 17: Smells and Heuristics) Ref: LOGGING_REVIEW.md for complete analysis and remaining action items 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Medium-priority logging improvements per Clean Code Chapter 3 (Functions): 1. Removed constructor logging from PrivateKey class - Removed "Created private key from byte array" debug log - Removed "Created private key from hex string" debug log - Removed "Generated new random private key" debug log - Simplified generateRandomPrivKey() to single return statement 2. Removed constructor logging from PublicKey class - Removed "Created public key from byte array" debug log - Removed "Created public key from hex string" debug log 3. Removed routine operation logging from BaseKey class - Removed "Converted key to Bech32" debug log in toBech32String() - Removed "Converted key to hex string" debug log in toHexString() - Simplified methods to single return statements 4. Enhanced error logging in BaseKey.toBech32String() - Added key type and prefix to error message for better context - Improved exception message to include original error Rationale: - Low-level data container classes should not log object creation - These classes are used frequently, logging creates noise - Constructor logging violates Single Responsibility Principle - If object creation tracking is needed, use profiler/instrumentation - Application layer should handle logging when appropriate Performance impact: - Reduces log overhead for frequently created objects - Eliminates unnecessary string formatting on every key creation Compliance: - Ch 3: Functions do one thing (no logging side effects) - Ch 10: Classes have single responsibility (data, not logging) - Ch 17: Eliminates logging "code smell" in utilities Ref: LOGGING_REVIEW.md sections 2, 5, and 8 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
…ines
Low-priority cleanup per LOGGING_REVIEW.md recommendations:
Removed all log.info("testMethodName") statements from test files across
the entire codebase (89 total removals):
- nostr-java-event: 58 removals
- nostr-java-api: 26 removals
- nostr-java-id: 4 removals
- nostr-java-util: 1 removal
Rationale (Clean Code Ch 17: Code Smells - G15 Selector Arguments):
- Test method names are already visible in JUnit test output
- Logging test names adds noise without value
- JUnit @DisplayName annotation is the proper way to add readable test names
- Reduces unnecessary log output during test execution
Example of proper approach (if needed):
```java
@test
@DisplayName("Event filter encoder should serialize filters correctly")
void testEventFilterEncoder() {
// test code
}
```
Performance impact:
- Eliminates 89 unnecessary log calls during test execution
- Cleaner test output
Compliance:
- Ch 17: Removes code smell (unnecessary logging in tests)
- Ch 4: Tests are self-documenting without log statements
Ref: LOGGING_REVIEW.md section 6 (Code Smells - G15)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
…cketClient Low-priority refactoring per LOGGING_REVIEW.md recommendations: Extracted duplicated recovery logging into reusable helper methods: 1. Added logRecoveryFailure(String operation, int size, IOException ex) - Handles simple recovery failures (send message, subscribe) - Reduces duplication in recover() and recoverSubscription() methods 2. Added logRecoveryFailure(String operation, String command, int size, IOException ex) - Handles recovery failures with command context - Used for BaseMessage recovery methods Benefits: - Reduces code duplication (4 nearly identical log statements → 2 helper methods) - Makes recovery logging consistent across all methods - Easier to maintain and update logging format in one place - Follows DRY (Don't Repeat Yourself) principle Compliance: - Ch 17: Eliminates code smell (G5 - Duplication) - Ch 3: Functions do one thing (separate logging concern) - Ch 10: Single Responsibility (logging extracted to helper) Before: 4 duplicated log.error() calls with similar patterns After: 2 reusable helper methods, 4 one-line calls Ref: LOGGING_REVIEW.md section 6 (Code Smells - G5) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
…adoc links; add missing @return text; adjust throws references to fully qualified types
…qualify types, fix @return, and throws references); examples and client Javadoc fixed
…re system property; avoid ByteBuddy self-attach issues on JDK 21
chore/qodana-final-and-bump-0.6.2
fix: replace generic exception handling
Bumps [actions/checkout](https://github.com/actions/checkout) from 4 to 5. - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](actions/checkout@v4...v5) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: '5' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]>
Bumps [actions/setup-java](https://github.com/actions/setup-java) from 4 to 5. - [Release notes](https://github.com/actions/setup-java/releases) - [Commits](actions/setup-java@v4...v5) --- updated-dependencies: - dependency-name: actions/setup-java dependency-version: '5' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]>
Bumps [org.jacoco:jacoco-maven-plugin](https://github.com/jacoco/jacoco) from 0.8.13 to 0.8.14. - [Release notes](https://github.com/jacoco/jacoco/releases) - [Commits](jacoco/jacoco@v0.8.13...v0.8.14) --- updated-dependencies: - dependency-name: org.jacoco:jacoco-maven-plugin dependency-version: 0.8.14 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]>
…coco-jacoco-maven-plugin-0.8.14 chore(deps): bump org.jacoco:jacoco-maven-plugin from 0.8.13 to 0.8.14
…op/actions/checkout-5 chore(deps): bump actions/checkout from 4 to 5
…op/actions/setup-java-5 chore(deps): bump actions/setup-java from 4 to 5
Update project version from 1.0.2-SNAPSHOT to 1.0.0 for first stable release. Changes: - Update parent POM version to 1.0.0 - Update all 9 module POM versions to reference parent 1.0.0 - Remove .project-management/ from git tracking (add to .gitignore) - Update roadmap script with comprehensive 1.0.0 task tracking All deprecated APIs removed, critical bugs fixed, and comprehensive verification completed. Project is ready for 1.0.0 release. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Remove deprecated method overloads that accept Identity parameter from NIP01 and NIP01EventBuilder: - NIP01.createTextNoteEvent(Identity, String, List<PubKeyTag>) - NIP01EventBuilder.buildTextNote(Identity, String) - NIP01EventBuilder.buildRecipientTextNote(Identity, String, List<PubKeyTag>) These methods are superseded by instance-configured sender pattern where the Identity is set at NIP01 construction time. This simplifies the API and reduces parameter duplication across method calls. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
chore/bump-version-1.0.2-SNAPSHOT
- Revised the troubleshooting section to focus on diagnosing relay send issues. - Removed outdated examples and streamlined the content for clarity.
- Added release automation script and GitHub Actions for CI/CD. - Updated documentation and removed deprecated APIs. - Cleaned up README and reorganized troubleshooting content. BREAKING CHANGE: deprecated APIs are removed in favor of new implementations. Impact: users must update their code to use the new API methods. Migration: refer to MIGRATION.md for guidance on replacements.
- Removed deprecated APIs: `Constants.Kind`, `Encoder.ENCODER_MAPPER_BLACKBIRD`, and NIP01 Identity-based overloads. - Simplified NIP01 to exclusively use instance-configured sender. BREAKING CHANGE: Deprecated APIs are no longer available. Impact: Clients relying on removed APIs will need to update their code. Migration: Refer to the migration guide for replacements and adjustments.
![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.
ℹ️ 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
Comment on lines
389
to
395
| private static boolean amITheRecipient(@NonNull Identity recipient, @NonNull GenericEvent event) { | ||
| var pTag = | ||
| event.getTags().stream() | ||
| .filter(t -> t.getCode().equalsIgnoreCase("p")) | ||
| // Use helper to fetch the p-tag without manual casts | ||
| PubKeyTag pTag = | ||
| Filterable.getTypeSpecificTags(PubKeyTag.class, event).stream() | ||
| .findFirst() | ||
| .orElseThrow(() -> new NoSuchElementException("No matching p-tag found.")); | ||
|
|
There was a problem hiding this comment.
The new decrypt(Identity, GenericEvent) flow adds a fallback that converts generic "p" tags to PubKeyTag so events coming from relays that were deserialized as GenericTag can still be decrypted. However, amITheRecipient() is called immediately afterwards and it re-queries the event with Filterable.getTypeSpecificTags without the same fallback, so any DM whose "p" tag was not already a PubKeyTag still raises NoSuchElementException and decryption never succeeds. Either reuse the already-resolved pTag or allow amITheRecipient() to accept generic tags as well.
Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment.
@codex address this comment, and make sure to add a unit test
There was a problem hiding this comment.
Summary
- Reused the resolved p-tag when determining DM recipients in
NIP04.decrypt, allowing decryption to succeed when relays deliver generic"p"tags. nostr-java-api/src/main/java/nostr/api/NIP04.javaL339-L402 - Added a unit test to confirm that decrypting events with generic
"p"tags works end-to-end. nostr-java-api/src/test/java/nostr/api/unit/NIP04Test.javaL3-L181
Testing
- ❌
mvn -q verify(fails: Non-resolvable import POM xyz.tcheeric:nostr-java-bom:pom:1.1.8 is absent in Maven Central) - ❌
mvn -q -pl nostr-java-api test(fails for the same missing xyz.tcheeric:nostr-java-bom:pom:1.1.8 dependency)
- Added support for optional Maven settings by checking for the presence of a .mvn/settings.xml file. This allows for more flexible Maven configurations during the release process. - Updated the command arguments to include the settings options when running Maven commands.
- Introduced a new settings.xml file to configure Maven repositories. - This includes profiles for central, nostr-java releases, and snapshots.
- Changed the Maven Central repository URL to a more reliable source. - This update ensures better access to dependencies for builds.
fix: handle generic p-tags in NIP04 decrypt recipient check
…ip01eventbuilder fix: restore sender overrides in NIP-01 builder
…ode-comment-in-nip01eventbuilder-dtnalz
…ip01eventbuilder-dtnalz style: simplify generics in NIP01 builder
Chore/bump version 1.0.2 snapshot
…rameter - Removed the sender parameter from multiple event building methods. - This change simplifies the API and ensures a consistent approach to event creation. BREAKING CHANGE: sender parameter is no longer accepted in event building methods. Impact: existing calls to these methods with a sender argument will need to be updated. Migration: remove sender argument from calls to buildTextNote, buildReplaceableEvent, buildEphemeralEvent, and buildAddressableEvent.
- Modify the builder to respect updates to the default sender identity. - Ensure that new events use the updated sender when no override is provided.
- Adjusted Maven commands in CI configuration to specify the custom settings file for consistency across builds. This change ensures that the correct repository configurations are applied during the build process.
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.
Summary
Merge
developintomainto release v1.0.0. This consolidates API cleanup (removal of deprecated entry points), documentation restructuring, and changelog updates. The goal is to stabilize the public surface before post‑1.0 changes.Related issue: #____
What changed?
nostr.config.Constants.Kindfacade; usenostr.base.Kind.nostr.base.Encoder.ENCODER_MAPPER_BLACKBIRD; usenostr.event.json.EventJsonMapper#getMapper().nostr.event.json.EventJsonMapper(Blackbird module).docs/howto/diagnostics.mdand removed maintainer‑only roadmap automation.BREAKING
Constants.Kind→Kind(useKind#valueviagetValue()).Encoder.ENCODER_MAPPER_BLACKBIRD→EventJsonMapper#getMapper().NIP01#createTextNoteEvent(Identity, ...)→ configure sender on instance:new NIP01(identity).createTextNoteEvent(...).See MIGRATION:
MIGRATION.md#nip01-api-changes,MIGRATION.md#objectmapper-usage, andMIGRATION.md#event-kind-constants.Review focus
Checklist
Testing
Commands executed from repo root:
Results:
mvn -q -DskipITs=true -DskipTests compile).Notes:
-Pno-docker.