refactor: unify decoder interface #413
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR unifies the decoder interface across the codebase by standardizing on IDecoder and adding proper error handling documentation. The refactor eliminates the redundant FDecoder interface and ensures consistent exception handling across all JSON decoders.
- Removes
FDecoderinterface and migratesFiltersDecoderto useIDecoder - Adds
EventEncodingExceptionto theIDecoderinterface method signature - Documents all decoder methods with comprehensive Javadoc comments including exception handling
Reviewed Changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| nostr-java-base/src/main/java/nostr/base/IDecoder.java | Updates interface to declare EventEncodingException and adds documentation |
| nostr-java-event/src/main/java/nostr/event/json/codec/FDecoder.java | Removes redundant interface entirely |
| nostr-java-event/src/main/java/nostr/event/json/codec/FiltersDecoder.java | Migrates from FDecoder to IDecoder and adds import |
| nostr-java-event/src/main/java/nostr/event/filter/Filters.java | Implements IElement interface |
| Multiple decoder classes | Adds comprehensive Javadoc documentation to decode methods |
| import com.fasterxml.jackson.module.blackbird.BlackbirdModule; | ||
| import nostr.event.json.codec.EventEncodingException; | ||
|
|
||
| /** |
There was a problem hiding this comment.
The base module is importing an exception from the event module, creating a circular dependency. Consider moving EventEncodingException to the base module or creating a generic DecodingException in the base module.
| /** | |
| /** | |
| * Exception thrown when decoding fails. | |
| */ | |
| class DecodingException extends Exception { | |
| public DecodingException(String message) { | |
| super(message); | |
| } | |
| public DecodingException(String message, Throwable cause) { | |
| super(message, cause); | |
| } | |
| } | |
| /** |
| import com.fasterxml.jackson.databind.JsonNode; | ||
| import lombok.Data; | ||
| import lombok.NonNull; | ||
| import nostr.base.IDecoder; |
There was a problem hiding this comment.
[nitpick] The import order should follow Java conventions. Place the nostr.base import before the nostr.event imports to maintain alphabetical ordering of package imports.
| import nostr.base.IDecoder; | |
| import nostr.base.IDecoder; |
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Summary
Testing
mvn -q verify(missing Docker environment for integration tests)https://chatgpt.com/codex/tasks/task_b_68a81c018604833184d3829f35c822d7