Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 82 additions & 0 deletions PR.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
Proposed title: fix: Fix CalendarContent addTag duplication; address Qodana findings and add tests

## Summary
This PR fixes a duplication bug in `CalendarContent.addTag`, cleans up Qodana-reported issues (dangling Javadoc, missing Javadoc descriptions, fields that can be final, and safe resource usage), and adds unit tests to validate correct tag handling.

Related issue: #____

## What changed?
- Fix duplication in calendar tag collection
- F:nostr-java-event/src/main/java/nostr/event/entities/CalendarContent.java†L184-L188
- Replace re-put/addAll pattern with `computeIfAbsent(...).add(...)` to append a single element without duplicating the list.
- F:nostr-java-event/src/main/java/nostr/event/entities/CalendarContent.java†L40-L40
- Make `classTypeTagsMap` final.

- Unit tests for calendar tag handling
- F:nostr-java-event/src/test/java/nostr/event/unit/CalendarContentAddTagTest.java†L16-L31
- F:nostr-java-event/src/test/java/nostr/event/unit/CalendarContentAddTagTest.java†L33-L45
- F:nostr-java-event/src/test/java/nostr/event/unit/CalendarContentAddTagTest.java†L47-L64

- Javadoc placement fixes (resolve DanglingJavadoc by placing Javadoc above `@Override`)
- F:nostr-java-api/src/main/java/nostr/api/NostrSpringWebSocketClient.java†L112-L116, L132-L136, L146-L150, L155-L159, L164-L168, L176-L180, L206-L210, L293-L297, L302-L306, L321-L325
- F:nostr-java-event/src/main/java/nostr/event/json/codec/GenericEventDecoder.java†L25-L33
- F:nostr-java-event/src/main/java/nostr/event/json/codec/Nip05ContentDecoder.java†L22-L30
- F:nostr-java-event/src/main/java/nostr/event/json/codec/BaseTagDecoder.java†L22-L30
- F:nostr-java-event/src/main/java/nostr/event/json/codec/BaseMessageDecoder.java†L27-L35
- F:nostr-java-event/src/main/java/nostr/event/json/codec/GenericTagDecoder.java†L26-L34

- Javadoc description additions (fix `@param`, `@return`, `@throws` missing)
- F:nostr-java-crypto/src/main/java/nostr/crypto/schnorr/Schnorr.java†L20-L28, L33-L41
- F:nostr-java-crypto/src/main/java/nostr/crypto/bech32/Bech32.java†L80-L89, L91-L100, L120-L128

- Fields that may be final
- F:nostr-java-api/src/main/java/nostr/api/WebSocketClientHandler.java†L31-L32
- F:nostr-java-event/src/main/java/nostr/event/entities/CalendarContent.java†L40-L40

- Resource inspections: explicitly managed or non-closeable resources
- F:nostr-java-api/src/main/java/nostr/api/WebSocketClientHandler.java†L87-L90, L101-L103
- Suppress false positives for long-lived `SpringWebSocketClient` managed by handler lifecycle.
- F:nostr-java-util/src/main/java/nostr/util/validator/Nip05Validator.java†L95-L96
- Suppress on JDK `HttpClient` which is not AutoCloseable and intended to be reused.

- Remove redundant catch and commented-out code
- F:nostr-java-event/src/main/java/nostr/event/impl/CreateOrUpdateProductEvent.java†L59-L61
- F:nostr-java-event/src/main/java/nostr/event/entities/ZapRequest.java†L12-L19

## BREAKING
None.

## Review focus
- Confirm the intention for `CalendarContent` is to accumulate tags per code without list duplication.
- Sanity-check placement of `@SuppressWarnings("resource")` where resources are explicitly lifecycle-managed.

## Checklist
- [x] Scope ≤ 300 lines (or split/stack)
- [x] Title is verb + object (Conventional Commits: `fix: ...`)
- [x] Description links the issue and answers “why now?”
- [x] BREAKING flagged if needed
- [x] Tests/docs updated (if relevant)

## Testing
- ✅ `mvn -q -DskipTests package`
- Build succeeded for all modules.
- ✅ `mvn -q -Dtest=CalendarContentAddTagTest test` (run in `nostr-java-event`)
- Tests executed successfully. New tests validate:
- Two hashtags produce exactly two items without duplication.
- Single participant `PubKeyTag` stored once with expected key.
- Different tag types tracked independently.
- ⚠️ `mvn -q verify`
- Fails in this sandbox due to Mockito’s inline mock-maker requiring a Byte Buddy agent attach, which is blocked:
- Excerpt:
- `Could not initialize plugin: interface org.mockito.plugins.MockMaker`
- `MockitoInitializationException: Could not initialize inline Byte Buddy mock maker.`
- `Could not self-attach to current VM using external process`
- Local runs in a non-restricted environment should pass once the agent is allowed or Mockito is configured accordingly.

## Network Access
- No external network calls required by these changes.
- No blocked domains observed. Test failures are unrelated to network and stem from sandbox agent-attach restrictions.

## Notes
- `CalendarContent.addTag` previously reinserted the list and added all elements again, causing duplication. The fix uses `computeIfAbsent` and appends exactly one element.
- I intentionally placed `@SuppressWarnings("resource")` where objects are long-lived or non-`AutoCloseable` (e.g., Java `HttpClient`) to silence false positives noted by Qodana.
55 changes: 55 additions & 0 deletions PR_BOM_MIGRATION.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
title: feat: migrate to nostr-java-bom for centralized version management

## Summary
Related issue: #____
Migrate nostr-java to use `nostr-java-bom` for centralized dependency version management across the Nostr Java ecosystem. This eliminates duplicate version properties and ensures consistent dependency versions.

## What changed?
- **Version bump**: `0.4.0` → `0.5.0`
- **BOM updated**: nostr-java-bom `1.0.0` → `1.1.0` (now includes Spring Boot dependencies)
- Remove Spring Boot parent POM dependency
- Replace 30+ version properties with single `nostr-java-bom.version` property (F:pom.xml†L77)
- Import `nostr-java-bom:1.1.0` in `dependencyManagement` (F:pom.xml†L87-L93)
- Remove version tags from all dependencies across modules:
- `nostr-java-crypto`: removed bcprov-jdk18on version (F:nostr-java-crypto/pom.xml†L37)
- `nostr-java-util`: removed commons-lang3 version (F:nostr-java-util/pom.xml†L28)
- `nostr-java-client`: removed Spring Boot versions, added compile scope for awaitility (F:nostr-java-client/pom.xml†L56)
- `nostr-java-api`: removed Spring Boot versions
- Simplify plugin management - versions now inherited from BOM (F:pom.xml†L100-L168)
- Update nostr-java-bom to import Spring Boot dependencies BOM

## BOM Architecture Changes
```
nostr-java-bom 1.1.0 (updated)
├─ imports spring-boot-dependencies (NEW)
├─ defines nostr-java modules (updated to 0.5.0)
└─ defines shared dependencies (BouncyCastle, Jackson, Lombok, test deps)
```

## Benefits
- **Single source of truth**: All Nostr Java dependency versions managed in one place
- **Consistency**: Identical dependency versions across all Nostr projects
- **Simplified updates**: Bump dependency versions once in BOM, all projects inherit it
- **Reduced duplication**: From 30+ version properties to 1
- **Spring Boot integration**: Now imports Spring Boot BOM for Spring dependencies

## BREAKING
None. Internal build configuration change only; no API or runtime behavior changes.

## Protocol Compliance
- No change to NIP (Nostr Implementation Possibilities) compliance
- Behavior remains compliant with Nostr protocol specifications

## Testing
- ✅ `mvn clean install -DskipTests -U` - BUILD SUCCESS
- All modules compile successfully with BOM-managed versions
- Plugin version warnings are non-blocking

## Checklist
- [x] Title uses `type: description`
- [x] File citations included
- [x] Version bumped to 0.5.0
- [x] nostr-java-bom updated to 1.1.0 with Spring Boot support
- [x] Build verified with BOM
- [x] No functional changes; protocol compliance unchanged
- [x] BOM deployed to https://maven.398ja.xyz/releases/xyz/tcheeric/nostr-java-bom/1.1.0/
26 changes: 13 additions & 13 deletions nostr-java-api/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
<parent>
<groupId>xyz.tcheeric</groupId>
<artifactId>nostr-java</artifactId>
<version>0.4.0</version>
<version>0.5.0</version>
<relativePath>../pom.xml</relativePath>
</parent>

Expand All @@ -26,42 +26,42 @@
<dependency>
<groupId>org.apache.commons</groupId>
<artifactId>commons-text</artifactId>
<version>${commons-text.version}</version>

</dependency>
<dependency>
<groupId>${project.groupId}</groupId>
<artifactId>nostr-java-client</artifactId>
<version>${nostr-java.version}</version>

</dependency>
<dependency>
<groupId>${project.groupId}</groupId>
<artifactId>nostr-java-encryption</artifactId>
<version>${nostr-java.version}</version>

</dependency>
<dependency>
<groupId>${project.groupId}</groupId>
<artifactId>nostr-java-id</artifactId>
<version>${nostr-java.version}</version>

</dependency>
<dependency>
<groupId>${project.groupId}</groupId>
<artifactId>nostr-java-event</artifactId>
<version>${nostr-java.version}</version>

</dependency>
<dependency>
<groupId>${project.groupId}</groupId>
<artifactId>nostr-java-util</artifactId>
<version>${nostr-java.version}</version>

</dependency>
<dependency>
<groupId>${project.groupId}</groupId>
<artifactId>nostr-java-crypto</artifactId>
<version>${nostr-java.version}</version>

</dependency>
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter</artifactId>
<version>${spring-boot.version}</version>

</dependency>
<dependency>
<groupId>com.fasterxml.jackson.core</groupId>
Expand All @@ -70,7 +70,7 @@
<dependency>
<groupId>org.projectlombok</groupId>
<artifactId>lombok</artifactId>
<version>${lombok.version}</version>

<scope>provided</scope>
</dependency>
<dependency>
Expand All @@ -85,19 +85,19 @@
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-test</artifactId>
<version>${spring-boot.version}</version>

<scope>test</scope>
</dependency>
<dependency>
<groupId>com.google.guava</groupId>
<artifactId>guava</artifactId>
<version>${guava.version}</version>

<scope>test</scope>
</dependency>
<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter</artifactId>
<version>${junit-jupiter.version}</version>

<scope>test</scope>
</dependency>
</dependencies>
Expand Down
12 changes: 6 additions & 6 deletions nostr-java-base/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
<parent>
<groupId>xyz.tcheeric</groupId>
<artifactId>nostr-java</artifactId>
<version>0.4.0</version>
<version>0.5.0</version>
<relativePath>../pom.xml</relativePath>
</parent>

Expand All @@ -27,12 +27,12 @@
<dependency>
<groupId>${project.groupId}</groupId>
<artifactId>nostr-java-util</artifactId>
<version>${nostr-java.version}</version>

</dependency>
<dependency>
<groupId>${project.groupId}</groupId>
<artifactId>nostr-java-crypto</artifactId>
<version>${nostr-java.version}</version>

</dependency>

<!-- JSON Processing -->
Expand All @@ -43,14 +43,14 @@
<dependency>
<groupId>com.fasterxml.jackson.module</groupId>
<artifactId>jackson-module-blackbird</artifactId>
<version>${jackson-module-blackbird.version}</version>

</dependency>

<!-- Lombok and Logging -->
<dependency>
<groupId>org.projectlombok</groupId>
<artifactId>lombok</artifactId>
<version>${lombok.version}</version>

<scope>provided</scope>
</dependency>
<dependency>
Expand All @@ -62,7 +62,7 @@
<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter</artifactId>
<version>${junit-jupiter.version}</version>

<scope>test</scope>
</dependency>
</dependencies>
Expand Down
14 changes: 7 additions & 7 deletions nostr-java-client/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
<parent>
<groupId>xyz.tcheeric</groupId>
<artifactId>nostr-java</artifactId>
<version>0.4.0</version>
<version>0.5.0</version>
<relativePath>../pom.xml</relativePath>
</parent>

Expand All @@ -27,14 +27,14 @@
<dependency>
<groupId>${project.groupId}</groupId>
<artifactId>nostr-java-id</artifactId>
<version>${nostr-java.version}</version>

</dependency>

<!-- Spring Framework -->
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-websocket</artifactId>
<version>${spring-boot.version}</version>

</dependency>
<dependency>
<groupId>org.springframework</groupId>
Expand All @@ -53,28 +53,28 @@
<dependency>
<groupId>org.awaitility</groupId>
<artifactId>awaitility</artifactId>
<version>${awaitility.version}</version>
<scope>compile</scope>
</dependency>

<!-- Lombok -->
<dependency>
<groupId>org.projectlombok</groupId>
<artifactId>lombok</artifactId>
<version>${lombok.version}</version>

<scope>provided</scope>
</dependency>

<!-- Testing -->
<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter</artifactId>
<version>${junit-jupiter.version}</version>

<scope>test</scope>
</dependency>
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-test</artifactId>
<version>${spring-boot.version}</version>

<scope>test</scope>
</dependency>
<dependency>
Expand Down
9 changes: 4 additions & 5 deletions nostr-java-crypto/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
<parent>
<groupId>xyz.tcheeric</groupId>
<artifactId>nostr-java</artifactId>
<version>0.4.0</version>
<version>0.5.0</version>
<relativePath>../pom.xml</relativePath>
</parent>

Expand Down Expand Up @@ -35,21 +35,20 @@
<dependency>
<groupId>org.bouncycastle</groupId>
<artifactId>bcprov-jdk18on</artifactId>
<version>${bcprov-jdk18on.version}</version>
</dependency>

<!-- Project Modules -->
<dependency>
<groupId>${project.groupId}</groupId>
<artifactId>nostr-java-util</artifactId>
<version>${nostr-java.version}</version>

</dependency>

<!-- Lombok and Logging -->
<dependency>
<groupId>org.projectlombok</groupId>
<artifactId>lombok</artifactId>
<version>${lombok.version}</version>

<scope>provided</scope>
</dependency>
<dependency>
Expand All @@ -61,7 +60,7 @@
<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter</artifactId>
<version>${junit-jupiter.version}</version>

<scope>test</scope>
</dependency>
</dependencies>
Expand Down
Loading
Loading