Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Next Next commit
wip: migrate nostr-java to nostr-java-bom - needs BOM update for Spri…
…ng Boot
  • Loading branch information
erict875 committed Oct 5, 2025
commit df880acbf9555207ce1a321a5585bd4c32270e6e
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.
24 changes: 12 additions & 12 deletions nostr-java-api/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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
10 changes: 5 additions & 5 deletions nostr-java-base/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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
12 changes: 6 additions & 6 deletions nostr-java-client/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>

</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
6 changes: 3 additions & 3 deletions nostr-java-crypto/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,14 @@
<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 +61,7 @@
<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter</artifactId>
<version>${junit-jupiter.version}</version>

<scope>test</scope>
</dependency>
</dependencies>
Expand Down
8 changes: 4 additions & 4 deletions nostr-java-encryption/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -30,27 +30,27 @@
<dependency>
<groupId>${project.groupId}</groupId>
<artifactId>nostr-java-crypto</artifactId>
<version>${nostr-java.version}</version>

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

</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>
</dependencies>
Expand Down
12 changes: 6 additions & 6 deletions nostr-java-event/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,17 @@
<dependency>
<groupId>${project.groupId}</groupId>
<artifactId>nostr-java-base</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>${project.groupId}</groupId>
<artifactId>nostr-java-util</artifactId>
<version>${nostr-java.version}</version>

</dependency>
<dependency>
<groupId>com.fasterxml.jackson.core</groupId>
Expand All @@ -46,12 +46,12 @@
<dependency>
<groupId>com.fasterxml.jackson.module</groupId>
<artifactId>jackson-module-blackbird</artifactId>
<version>${jackson-module-blackbird.version}</version>

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

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

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