Skip to content

Avoid nesting in event processor, do not add coupling#782

Merged
SHAcollision merged 1 commit intopubky:feat/universal-tags-app-specific-pathsfrom
ok300:ok300-universal-tags-event-handling-simplification
Apr 3, 2026
Merged

Avoid nesting in event processor, do not add coupling#782
SHAcollision merged 1 commit intopubky:feat/universal-tags-app-specific-pathsfrom
ok300:ok300-universal-tags-event-handling-simplification

Conversation

@ok300
Copy link
Copy Markdown
Contributor

@ok300 ok300 commented Mar 31, 2026

This PR proposes a way to address this review thread: #777 (comment) . This is a proposed change to #777 , so it points to that PR's branch.

More specifically, it avoids introducing (7-layer) nesting in the main event processor logic, while also avoiding the coupling counter-argument raised in the same thread.


AI detailed description below

The counter-argument rests on a false dilemma: that incorporating logic into Event::parse_event necessarily means teaching nexus-common about app-specific paths. It doesn't.

The root cause of the nesting is that Event::parse_event (nexus-common/src/models/event/mod.rs:70-72) treats a ParsedUri::try_from failure as InvalidEventLine:

  let parsed_uri = ParsedUri::try_from(uri.as_str()).map_err(|e| {
      EventProcessorError::InvalidEventLine(format!("Cannot parse event URI: {e}"))
  })?;

But pubky://user/pub/mapky/tags/123 is not an invalid event line — it's a valid event that pubky-app-specs doesn't recognize. By conflating "unrecognized path" with "invalid line," parse_event throws away the already-parsed event_type and uri, forcing the universal tag handler to re-parse the raw line from scratch via parse_event_line (which duplicates the split + event type validation).

The fix: Give parse_event a richer return type that distinguishes "parsed," "skipped," and "unrecognized URI":

  pub enum ParseResult {
      Parsed(Event),
      Skipped,
      UnrecognizedUri { event_type: EventType, uri: String, reason: String },
  }

nexus-common gains zero knowledge of universal tags or app-specific paths.

What this achieves:

  1. Nesting drops from 7 levels to 2-3 (flat match arms)
  2. parse_event_line is eliminated — no more duplicated line splitting
  3. nexus-common stays free of app-specific logic
  4. All universal tag business logic (try_parse_app_tag_path, handle_put, handle_del) stays unchanged in nexus-watcher
  5. The retry error handling can be extracted into a small helper method mirroring the existing handle_event pattern

@ok300 ok300 requested review from SHAcollision and tipogi March 31, 2026 09:47
@ok300 ok300 self-assigned this Mar 31, 2026
@ok300 ok300 added the 👀 watcher Nexus indexer related operations label Mar 31, 2026
@ok300 ok300 added this to the 2026-Q1 milestone Mar 31, 2026
Copy link
Copy Markdown
Collaborator

@tipogi tipogi left a comment

Choose a reason for hiding this comment

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

LGTM

@SHAcollision SHAcollision merged commit d606680 into pubky:feat/universal-tags-app-specific-paths Apr 3, 2026
@ok300 ok300 deleted the ok300-universal-tags-event-handling-simplification branch April 3, 2026 18:26
SHAcollision added a commit that referenced this pull request Apr 11, 2026
* feat: universal tags with app-specific paths

* Add TODO for future query batching

* chore: remove empty wrapper put_index_sorted_set_static

* chore: remove superfluous stream conversions

* chore: remove empty wrapper put_score_index_sorted_set_static

* Avoid nesting in event processor, do not add coupling (#782)

* PR fixes

* fix clippy

* infinite loop fix

* fixes

* fix: use new traced_join! to integrate tracing into
tag::put_sync_resource

* chore: add URI length validation in raw URI lookup API

* chore: removed unused fields in try_handle_universal_tag

* chore: extract, re-use ResourceDetails::get_by_id(resource_id)

* chore: use `Option<&[String]>` as tags type to simplify type conversions

* chore: simplify build_index_key return type to minimize type conversions

---------

Co-authored-by: ok300 <106775972+ok300@users.noreply.github.com>
aintnostressin pushed a commit that referenced this pull request Apr 20, 2026
* feat: universal tags with app-specific paths

* Add TODO for future query batching

* chore: remove empty wrapper put_index_sorted_set_static

* chore: remove superfluous stream conversions

* chore: remove empty wrapper put_score_index_sorted_set_static

* Avoid nesting in event processor, do not add coupling (#782)

* PR fixes

* fix clippy

* infinite loop fix

* fixes

* fix: use new traced_join! to integrate tracing into
tag::put_sync_resource

* chore: add URI length validation in raw URI lookup API

* chore: removed unused fields in try_handle_universal_tag

* chore: extract, re-use ResourceDetails::get_by_id(resource_id)

* chore: use `Option<&[String]>` as tags type to simplify type conversions

* chore: simplify build_index_key return type to minimize type conversions

---------

Co-authored-by: ok300 <106775972+ok300@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

👀 watcher Nexus indexer related operations

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants