Skip to content

Improved overflow support: Internal event handlers can dispatch new events - #21

Merged
sungshik merged 5 commits into
improved-overflow-support-mainfrom
improved-overflow-support/event-handlers-with-active-watch
Feb 24, 2025
Merged

Improved overflow support: Internal event handlers can dispatch new events#21
sungshik merged 5 commits into
improved-overflow-support-mainfrom
improved-overflow-support/event-handlers-with-active-watch

Conversation

@sungshik

@sungshiksungshik commented Feb 24, 2025

Copy link
Copy Markdown
Contributor

What

This PR generalizes the signature of internal event handlers from Consumer<WatchEvent> to BiConsumer<EventHandlingWatch, WatchEvent>. Thus, when internal event handlers are called, they also get access to the watch that dispatched the event.

This change affects only the internals of the library (currently only the JDK...Watch classes, but later also the macOS code). The end-user API still expects Consumer<WatchEvent> (or WatchEventListener).

Why

Imagine an overflow event happens, so the directory needs to be rescanned. Before this PR, the event handler of the watch could look something like this (simplified code, just to make the point):

classFooWatchimplementsEventHandlingWatch {
...
@overridepublicvoidhandleEvent(WatchEventevent) {
if (event.getKind() == WatchEvent.Kind.OVERFLOW) {
varevents = rescanDirectory(); // Returns a collection of rescanning events (CREATED or MODIFIED)for (vare : events) {
// Recursively call this method for each of the rescanning eventshandleEvent(e);
}
}
// Run the user-defined event handlerthis.eventHandler.accept(event);
}
}

The point is that, to auto-handle overflow events, the auto-handler must access a watch to dispatch rescanning events (i.e., call the watch's handleEvent method). There are several alternatives to achieve this, including:

  1. Define auto-handlers inside the watch class -- This is essentially what currently happens in JDKRecursiveDirectoryWatch
  2. Define auto-handlers outside the watch class; pass a watch class reference when the auto-handler is constructed
  3. Define auto-handlers outside the watch class; pass a watch class reference when the auto-handler is called

The problem with alternative 1 is that each auto-handler class should be usable with multiple watch classes (otherwise, we're potentially duplicating a lot of code). The problem with alternative 2 is that each auto-handler instance should be usable with multiple watch instances (otherwise, we're potentially duplicating a lot of handler objects that do essentially the same thing).

Thus, this PR implements alternative 3. It enables:

  • Defining auto-handler code completely independent of the watch classes (as long as they implement the EventHandlingWatch interface, which has method handleEvent)
  • Construct a single auto-handler that can be (re)used by multiple watches

(Usages of these features are demonstrated in the next PR.)

@sungshiksungshik mentioned this pull request Feb 24, 2025
7 tasks
Base automatically changed from improved-overflow-support/richer-active-watch to improved-overflow-support-mainFebruary 24, 2025 11:20
@codecov

codecovBot commented Feb 24, 2025

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.6%. Comparing base (8d4dd06) to head (d52cfeb).
Report is 6 commits behind head on improved-overflow-support-main.

Additional details and impacted files
@@ Coverage Diff @@## improved-overflow-support-main #21 +/- ##
================================================================
Coverage 80.6% 80.6% - Complexity 95 97 +2 
================================================================
Files 12 12 Lines 428 428 Branches 41 41 ================================================================
Hits 345 345 Misses 61 61 Partials 22 22 

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sungshik
sungshik marked this pull request as ready for review February 24, 2025 12:16

@DavyLandmanDavyLandman left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I generally dislike BiConsumers (as they start to hint at the fact it should have been a class/interface), but I'm okay with this.

@sungshik
sungshik merged commit a712a3a into improved-overflow-support-mainFeb 24, 2025
@sungshik
sungshik deleted the improved-overflow-support/event-handlers-with-active-watch branch February 24, 2025 13:10
Sign up for freeto 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

@sungshik@DavyLandman