Repository navigation
feat: count a parent's child workflows, optionally by status - #45
Merged
Merged
Conversation
Add WorkflowsService.countChildWorkflows(parentWorkflowId[, statuses]), backed by a new WorkflowStore.countChildWorkflows implemented for MySQL and SQLite. The status overload rejects an empty list instead of reading it as "any status". parent_workflow_id had no index, so the count would scan every instance of the owner. Add an (owner, parent_workflow_id, status) index through a MySQL migration (V7) and its SQLite counterpart (V3); it covers both query shapes. The store method is a default that throws UnsupportedOperationException, like findWorkflows, so out-of-tree WorkflowStore implementations keep compiling.
Diff coverage on the PR was 68% against the 90% gate. Cover the SQLException branch in both stores by pointing a store at a table prefix with no tables, and cover WorkflowStore's default method directly. Drop the unreachable empty-result branch: COUNT(*) without GROUP BY always yields exactly one row.
rgamba
commented
Oct 26, 2026
| * exist). | ||
| */ | ||
| default long countChildWorkflows( | ||
| String parentWorkflowId, List<WorkflowInstance.Status> statuses) { |
Member
Author
There was a problem hiding this comment.
statuses should be a set not a list
Member
Author
There was a problem hiding this comment.
Done in 3ed06fc — statuses is now a java.util.Set<WorkflowInstance.Status> (an empty set still means any status). I made the client-facing WorkflowsService.countChildWorkflows(parentWorkflowId, statusViews) overload take a Set too, so both layers agree; it still rejects an empty set.
- Ricardo's agent
rgamba
commented
Oct 26, 2026
|
|
||
| override fun countChildWorkflows( | ||
| parentWorkflowId: String, | ||
| statuses: VavrList<WorkflowInstance.Status>, |
Member
Author
There was a problem hiding this comment.
Done in 3ed06fc — both the MySQL and SQLite implementations (and the testutils tracing delegate) take a plain Kotlin Set now, with no vavr types in the signature. Placeholders are built with joinToString and bound with forEachIndexed.
- Ricardo's agent
Review feedback: statuses are a filter, so a set is the right type, and the store signature shouldn't expose vavr. WorkflowStore, both store implementations and the WorkflowsService overload now take a Set; the service still rejects an empty one.
rgamba
marked this pull request as ready for review
October 26, 2026 18:59
sndre
approved these changes
Oct 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
A client-facing way to ask how many child workflows a parent has, optionally only those in a given set of statuses:
Design
WorkflowsService, next tocountWorkflowsByStatus. It reads the store directly, so its result changes between replays. The KDoc says to call it from client code or from inside an action, never directly in a workflow method.java.util.Set. The status overload rejects an empty set instead of reading it as "any status", so a caller whose computed set ends up empty gets an error rather than a count of every child.WorkflowStore.countChildWorkflows, implemented for MySQL and SQLite as oneCOUNT(*)scoped to the owner. It is a default method that throwsUnsupportedOperationException, asfindWorkflowsdoes, soWorkflowStoreimplementations outside this repo keep compiling. The testutils tracing store delegates to it.parent_workflow_idhad no index, so the count would have scanned every instance of the owner. This adds(owner, parent_workflow_id, status)through a MySQL migration (V7) and its SQLite counterpart (V3). It covers both query shapes, so the count never reads table rows.Trade-offs
workflow_instancestable it still takes time and I/O, so deployments that apply migrations by hand should schedule V7 accordingly.Testing
BaseWorkflowStoreTest, runs against both MySQL and SQLite). It covers status filtering, multiple statuses, a sibling parent's children, top-level workflows, the same parent ID under another owner, and a parent that doesn't exist. A second shared test points a store at a table prefix with no tables and checks that the realSQLExceptioncomes back wrapped inInternalError. Both pass on SQLite (45/45 inSqliteWorkflowStoreTest). The MySQL run happens in CI; embedded MariaDB can't install on my machine.BaseWorkflowIntegTest). It starts two real child workflows through the factory and counts them throughWorkflowsService: 2 in total, 2COMPLETED, 0RUNNING, and 0 for an unrelated parent. It passes.WorkflowsServiceTest) cover both overloads and the empty-set rejection.WorkflowStoreTestcovers the interface default for stores that don't override it.EXPLAIN QUERY PLAN:spotlessCheckis clean.