Skip to content

Conversation

@tcheeric
Copy link
Owner

Summary

  • Remove redundant catch-and-rethrow blocks flagged by static analysis.
  • Reuse a single HttpClient instance in Nip05Validator instead of per-call creation.
  • Chore: bump version to 0.3.0 (x.y.z).
  • Deprecations: replace StringEscapeUtils import, Retryable.value, and JsonNode.fields().

Changes

  • F:nostr-java-event/src/main/java/nostr/event/impl/CreateOrUpdateStallEvent.java†L49-L62: remove catch (AssertionError e) { throw e; }; preserve wrapping of non-assertion exceptions.
  • F:nostr-java-event/src/main/java/nostr/event/impl/MerchantEvent.java†L43-L56: remove catch (AssertionError e) { throw e; }; preserve wrapping of non-assertion exceptions.
  • F:nostr-java-util/src/main/java/nostr/util/validator/Nip05Validator.java†L32-L49,L82: add cached HttpClient and client() accessor; replace per-call HttpClient.newHttpClient() with reuse.
  • F:nostr-java-api/src/main/java/nostr/api/NIP57.java†L20: switch to org.apache.commons.text.StringEscapeUtils.
  • F:nostr-java-api/src/main/java/nostr/api/NIP28.java†L25: switch to org.apache.commons.text.StringEscapeUtils.
  • F:nostr-java-client/src/main/java/nostr/client/springwebsocket/NostrRetryable.java†L14-L18: use include = IOException.class instead of deprecated value.
  • F:nostr-java-event/src/main/java/nostr/event/json/deserializer/*Deserializer.java: use fieldNames() + get(name) instead of deprecated fields().
  • F:nostr-java-event/src/main/java/nostr/event/json/serializer/AbstractTagSerializer.java†L23-L31: use fieldNames() + get(name) instead of deprecated fields().
  • F:pom.xml†L74: add commons-text.version property.
  • F:nostr-java-api/pom.xml†L21-L26: add org.apache.commons:commons-text dependency.
  • F:pom.xml†L6,L77: bump project + property version to 0.3.0.
  • F:nostr-java-*/pom.xml: bump module versions to 0.3.0.

Testing

  • mvn -q -DskipITs=false verify
    • Passed locally. Notable logs (tests ran, no failures):
      • Spring WebSocket client tests executed with retries and expected exceptions in tests.
      • Testcontainers pulled and started scsibug/nostr-rs-relay:latest containers successfully.

Network Access

  • No blocked domains encountered. Maven dependencies and Testcontainers images resolved successfully.

Notes

  • SLF4J no-provider warnings are informational and unchanged by this PR.
  • Mockito agent warnings are also informational and unrelated to these changes.

Protocol Compliance

  • No event schema or protocol behavior changed. Validations remain aligned with NIP-15 content expectations (stall and merchant entity checks remain intact).

erict875 added 7 commits August 31, 2025 16:26
…field extraction

- Updated deserializer classes to use a while loop for extracting field values
  instead of forEachRemaining. This change improves readability and consistency
  across the deserialization logic.
…ckage

- Changed the import statement for StringEscapeUtils from
  org.apache.commons.lang3 to org.apache.commons.text to align
  with the latest library updates and improve compatibility.
- Updated the parent version to 0.3.0 for the project.
- Added commons-text as a new dependency for text manipulation utilities.
- Ensure both `name` and `currency` fields are validated correctly.
- Throw an assertion error with a clear message if either field is missing.
…tion

- Simplified the error handling logic for merchant entity validation.
- Consolidated checks for null or empty `id` field into a single block.
- Improved readability and maintainability of the code.
- Refactored nip05 validation logic to enhance readability and maintainability.
- Improved error messages for invalid local-part and domain syntax.
- Streamlined HTTP client usage by introducing a default HttpClientProvider.
- Changed the retryable annotation to include IOException instead of
  just value. This improves the flexibility of the retry mechanism
  for handling IO exceptions.
@tcheeric tcheeric requested a review from Copilot August 31, 2025 15:33
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the codebase to remove redundant error handling patterns, improve performance through HttpClient reuse, update deprecated API usage, and bump the project version to 0.3.0. The changes address static analysis findings and modernize the code to use current best practices.

Key changes include:

  • Elimination of redundant catch-and-rethrow blocks in event validation classes
  • Migration from per-call HttpClient creation to a reusable instance pattern in Nip05Validator
  • Update of deprecated Jackson and Spring Retry API usage to modern alternatives

Reviewed Changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
pom.xml Version bump to 0.3.0 and addition of commons-text dependency property
nostr-java-util/src/main/java/nostr/util/validator/Nip05Validator.java Major refactor to use HttpClientProvider pattern and improve validation logic
nostr-java-event/src/main/java/nostr/event/json/serializer/AbstractTagSerializer.java Replace deprecated fields() with fieldNames() iteration
nostr-java-event/src/main/java/nostr/event/json/deserializer/*Deserializer.java Replace deprecated fields() with fieldNames() iteration across multiple deserializers
nostr-java-event/src/main/java/nostr/event/impl/*.java Remove redundant catch blocks and fix extra closing braces
nostr-java-client/src/main/java/nostr/client/springwebsocket/NostrRetryable.java Replace deprecated value with include parameter
nostr-java-api/src/main/java/nostr/api/*.java Switch from deprecated commons-lang3 to commons-text StringEscapeUtils
*/pom.xml Update module versions to 0.3.0

Comment on lines +43 to +44
@Builder.Default @JsonIgnore
private final HttpClientProvider httpClientProvider = new DefaultHttpClientProvider();
Copy link

Copilot AI Aug 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code references HttpClientProvider and DefaultHttpClientProvider classes that are not visible in the imports or defined in this file. These classes appear to be missing from the codebase, which would cause compilation errors.

Copilot uses AI. Check for mistakes.
Comment on lines +40 to +41
@Builder.Default @JsonIgnore private final Duration connectTimeout = Duration.ofSeconds(5);
@Builder.Default @JsonIgnore private final Duration requestTimeout = Duration.ofSeconds(5);
Copy link

Copilot AI Aug 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Duration and @JsonIgnore imports are missing. Add import java.time.Duration; and import com.fasterxml.jackson.annotation.JsonIgnore; to the imports section.

Copilot uses AI. Check for mistakes.
@github-actions
Copy link

Qodana Community for JVM

166 new problems were found

Inspection name Severity Problems
Unchecked warning 🔶 Warning 61
Link specified as plain text 🔶 Warning 24
Javadoc declaration problems 🔶 Warning 22
Dangling Javadoc comment 🔶 Warning 20
Redundant local variable 🔶 Warning 11
Field may be 'final' 🔶 Warning 3
Redundant type cast 🔶 Warning 3
Deprecated API usage 🔶 Warning 2
'size() == 0' can be replaced with 'isEmpty()' 🔶 Warning 2
AutoCloseable used without 'try'-with-resources 🔶 Warning 1
Caught exception is immediately rethrown 🔶 Warning 1
Default annotation parameter value 🔶 Warning 1
Deprecated member is still used 🔶 Warning 1
Lombok @Getter may be used 🔶 Warning 1
@NotNull/@Nullable problems 🔶 Warning 1
Redundant type arguments 🔶 Warning 1
Redundant character escape 🔶 Warning 1
Suspicious variable/parameter name combination 🔶 Warning 1
Type parameter hides visible type 🔶 Warning 1
Commented out code ◽️ Notice 5
Multiple occurrences of the same expression ◽️ Notice 2
@NotNull/@Nullable problems ◽️ Notice 1

☁️ View the detailed Qodana report

Contact Qodana team

Contact us at qodana-support@jetbrains.com

@tcheeric tcheeric merged commit 7135a2e into develop Aug 31, 2025
2 of 3 checks passed
@tcheeric tcheeric deleted the refactor/rethrow-and-httpclient-0.3.0 branch August 31, 2025 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants