Skip to content

Conversation

tcheeric
Copy link
Owner

No description provided.

erict875 added 30 commits May 18, 2025 05:28
…age to enhance tag handling and validation
…CashuProof, CashuQuote, CashuToken, and CashuWallet, updating package structure and enhancing type references
…Event to enhance tag handling and update constructors, and move ChannelProfile to a new package
…age structure, enhance tag handling, and update constructors
…llEvent, and CustomerOrderEvent to enhance type handling, improve constructor parameters, and update validation logic
…ove type handling, and update package structure
erict875 added 21 commits May 18, 2025 05:45
…or validation, simplify tag handling, and add utility methods for recipient and sender retrieval
…, and ZapRequestEvent classes: enhance constructor parameters, add no-args constructors, and improve tag validation
… tag creation, and improve test assertions
…ent handling, and simplify method signatures
…t creation methods, and improve handling of UserProfile and MentionsEvent
…r: update event handling by replacing IEvent with GenericEvent and improve method structure
@tcheeric tcheeric requested a review from Copilot May 18, 2025 22:47
Copy link
Contributor

@Copilot 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 upgrades the library to v0.7 by replacing specialized factories with GenericEventFactory and BaseTagFactory, unifying event and tag creation, and adding validation logic for channel and reaction events.

  • Standardized on GenericEventFactory for event creation and BaseTagFactory for tags
  • Removed legacy NIPxxImpl factories and adjusted method signatures and return types
  • Added validation checks and helper overloads for channel, reaction, and deletion flows
  • Bumped project version to 0.7-SNAPSHOT in Maven and Gradle configs

Reviewed Changes

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

Show a summary per file
File Description
NIP31.java Switched createAltTag to return BaseTag via BaseTagFactory
NIP30.java Renamed to createEmojiTag, now returns BaseTag from BaseTagFactory
NIP28.java Refactored channel create/message/metadata/hide/mute events to use GenericEventFactory, added validation and tagging
NIP25.java Unified reaction events with GenericEventFactory, improved tag logic, added website reactions
NIP23.java Refactored long-form note/draft and tag methods to use GenericEventFactory and BaseTagFactory
NIP20.java Simplified createOkMessage to use GenericEvent.getId()
NIP15.java Refactored marketplace events to GenericEventFactory, added direct-tagging helpers
NIP14.java Converted createSubjectTag to use BaseTagFactory
NIP12.java Replaced geohash/hashtag/reference factories with BaseTagFactory
NIP09.java Consolidated deletion event flow with GenericEventFactory, streamlining tag extraction
NIP08.java Updated mentions event to use GenericEventFactory
NIP05.java Refactored metadata event with JSON validation and GenericEventFactory
NIP04.java Refactored direct message encryption and event creation to use GenericEventFactory
NIP03.java Replaced OtsEventFactory with GenericEventFactory and unified ALT/event tag creation
NIP02.java Simplified contact list event to GenericEventFactory, strengthened tag validation
NIP01.java Major overhaul: replaced all NIP01 factories with GenericEventFactory and BaseTagFactory
EventNostr.java Removed generic parameter on the class, simplified updateEvent and addTag
pom.xml Bumped parent version to 0.7-SNAPSHOT
gradle.properties Updated nostr-java.version to 0.7-SNAPSHOT
buildSrc/build.gradle Updated buildSrc version to 0.7-SNAPSHOT
Comments suppressed due to low confidence (1)

nostr-java-api/src/main/java/nostr/api/NIP23.java:31

  • [nitpick] Method name is misspelled as creatLongFormTextNoteEvent; consider renaming to createLongFormTextNoteEvent for consistency.
public NIP23 creatLongFormTextNoteEvent(@NonNull String content) {

Comment on lines +64 to +65
//.filter(tag -> "a".equals(tag.getCode()))
//.filter(tag -> tag instanceof AddressTag)
Copy link
Preview

Copilot AI May 18, 2025

Choose a reason for hiding this comment

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

The filters for AddressTag instances are commented out, so every tag will be cast to AddressTag and may cause ClassCastException. Re-enable filtering or add an instanceof check before casting.

Suggested change
//.filter(tag -> "a".equals(tag.getCode()))
//.filter(tag -> tag instanceof AddressTag)
.filter(tag -> "a".equals(tag.getCode()))
.filter(tag -> tag instanceof AddressTag)

Copilot uses AI. Check for mistakes.


NIP23 createLongFormDraftEvent(@NonNull String content) {
Copy link
Preview

Copilot AI May 18, 2025

Choose a reason for hiding this comment

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

[nitpick] The createLongFormDraftEvent method has package-private visibility; consider adding public to match other NIPxx methods.

Suggested change
NIP23 createLongFormDraftEvent(@NonNull String content) {
public NIP23 createLongFormDraftEvent(@NonNull String content) {

Copilot uses AI. Check for mistakes.

@tcheeric tcheeric requested a review from avlo May 18, 2025 22:56
@avlo
Copy link
Collaborator

avlo commented May 18, 2025

awesome! checking shortly and will keep you posted

@avlo
Copy link
Collaborator

avlo commented May 20, 2025

@tcheeric hi, eric. i've noticed many/(all?) impl/<XYZ>Event.java classes using @NoArgsConstructor as well (see above comment), which similarly may/will yield downstream ambiguities- and therefore, undesirable- and furthermore, preventable- complexity.

i'm currently updating your branch with all @NoArgsConstructor's removed variant and submit changes directly to nj-0.7_upgrade branch shortly.

otherwise pls lmk what your motivation is for their inclusions and i/we can come up with a suitable and safe design variant.

@avlo avlo merged commit 4bc30cc into develop May 21, 2025
@tcheeric
Copy link
Owner Author

Good catch. There might have been a reason in the past, but I don't think they're needed at this point. Thanks for the cleanup.

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