DD-2237 Made the PollingTaskExecutor more generic. Now, a task can be represe…#33
Conversation
…nted by multiple records. Also changed the method name of TaskSource to make the distinction clearer between the inputs and the resulting tasks. Bumped the major version, because this is a breaking change.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #33 +/- ##
============================================
+ Coverage 48.64% 48.89% +0.25%
- Complexity 113 117 +4
============================================
Files 31 33 +2
Lines 810 818 +8
Branches 90 90
============================================
+ Hits 394 400 +6
- Misses 378 380 +2
Partials 38 38 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR refactors PollingTaskExecutor to separate “polling for inputs” from “executing scheduled tasks”, and renames the TaskSource method to clarify semantics, with a major version bump for the breaking API change.
Changes:
- Rename
TaskSource.nextTask()toTaskSource.nextInput()and update tests accordingly. - Introduce
TaskSchedulerwithImmediateTaskSchedulerandExecutorServiceTaskScheduler, and route task execution through the scheduler. - Update
PollingTaskExecutorto fetch inputs under@UnitOfWorkvia a dedicated method and schedule tasks instead of running inline.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/java/nl/knaw/dans/lib/util/pollingtaskexec/PollingTaskExecutorTest.java | Updates tests to use nextInput() and validate tick behavior with the default scheduler. |
| src/main/java/nl/knaw/dans/lib/util/pollingtaskexec/TaskSource.java | Renames the API to nextInput() and updates documentation to reflect new semantics. |
| src/main/java/nl/knaw/dans/lib/util/pollingtaskexec/TaskScheduler.java | Adds abstraction for how tasks are scheduled/executed. |
| src/main/java/nl/knaw/dans/lib/util/pollingtaskexec/ImmediateTaskScheduler.java | Provides a default scheduler that runs tasks inline (backwards-compatible behavior). |
| src/main/java/nl/knaw/dans/lib/util/pollingtaskexec/ExecutorServiceTaskScheduler.java | Adds an async scheduler implementation backed by ExecutorService. |
| src/main/java/nl/knaw/dans/lib/util/pollingtaskexec/PollingTaskExecutor.java | Delegates task execution to TaskScheduler and isolates the @UnitOfWork polling call. |
| src/main/java/nl/knaw/dans/lib/util/pollingtaskexec/TaskFactory.java | Minor documentation wording update. |
| pom.xml | Bumps major version to 3.0.0-SNAPSHOT due to breaking API change. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| log.debug("{}: found next task record: {}", name, input.get()); | ||
| Runnable task = taskFactory.create(input.get()); | ||
| taskScheduler.schedule(task); | ||
| } |
There was a problem hiding this comment.
New behavior is introduced by delegating task execution to TaskScheduler, but the existing test suite only covers the default ImmediateTaskScheduler path. Add a unit test that constructs PollingTaskExecutor with a mocked TaskScheduler and asserts schedule(...) is invoked (and that the task is not run inline unless the scheduler does so).
| private final TaskSource<R> taskSource; | ||
| private final TaskFactory<R> taskFactory; | ||
| private final TaskScheduler taskScheduler; |
There was a problem hiding this comment.
PollingTaskExecutor now depends on a TaskScheduler to run tasks (potentially asynchronously), but the public class-level Javadoc still describes it as directly executing tasks. Please update the Javadoc to document the TaskScheduler role and any ordering/concurrency implications.
….java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…rviceTaskScheduler.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…kExecutor.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…kExecutor.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
….java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Fixes DD-2237
Description of changes
Changes to support DD-2237. Introduced a
TaskSchedulerinterface that lets clients configure how tasks picked up byPollingTaskExecutorand created byTaskFactoryare then scheduled (e.g., immediately executed or submitted to an executorservice).Made sure the nextInput is called in its own db-transaction, so that a status update to the underlying record is not lost if the task execution fails.
Notify
@DANS-KNAW/core-systems