Enumerate the subtree: batched node listing, snapshot walks, one progress vocabulary #12065

[rymsha]

Follow-up to #12247. It began as the two gaps that PR left open — a listing that had to be held whole, and an audit log vacuumed through the search index — and grew into the walk that both of those wanted: enumerate the branch entries, decide from what the entries already carry, and complete nodes from version blobs only where a node is really needed. Every whole-subtree operation in the repository now works that way, and every one of them reports its progress in the same words.

1. list and enumerate are two methods, not one with a mode

The listing API had grown a batch size and a cursor, which turned one method into two contracts wearing one name. They are now separate:

// the whole subtree, path-ordered, filtered by what the caller may read
final ContentIds ids = nodeService.list( ListNodesParams.create().parentPath( parent ).build() )
    .map( entry -> ContentId.from( entry.nodeId() ) )
    .collect( ContentIds.collector() );

// the whole subtree, one bounded batch at a time
String cursor = null;
do {
    final EnumerateNodesResult batch = nodeService.enumerate(
        EnumerateNodesParams.create().parentPath( parent ).cursor( cursor ).build() );
    // consume batch.getEntries()
    cursor = batch.getCursor();
} while ( cursor != null );

• list streams. It is used to collect ids more often than anything else, so it answers with a Stream<NodeListEntry> that maps straight into whatever the caller collects, without the intermediate result object ListNodesResult used to be. That class is gone; so is its recursive flag. The branch index carries no parent field, so the flag never bought a cheaper scan — the non-recursive form was the same scan with a filter on top. The three system listings that do want the children of a folder (virtual applications, the application repo, scheduled jobs) now carry that filter themselves, where the cost of it is visible.
• enumerate is always batched, and asks for the administrator role. It serves dump, export, clean-up and synchronization, all of which need every node the subtree holds — so instead of filtering each entry against the caller's permissions, it refuses a caller without the role up front. Entries therefore cost no ACL resolution at all, and a batch is exactly as large as it says.
• The cursor is a position, not a page. An unbounded enumeration positions on the node id, which never changes: a node moved between batches is still observed exactly once, and deletions behind the cursor cost nothing. Both are pinned by itests that move and delete entries mid-enumeration.
• A batch may be empty before the enumeration is over, so the contract is to continue until a batch answers with no cursor.
• The batch ceiling is not API. 10 000 is the most the index will answer in one request; it is the default and the maximum, and it is private — a caller sets a smaller batch only where fewer entries settle the question.

An enumeration can be bounded, and then it means something

enumerate takes modifiedBefore. A bounded enumeration holds only the nodes whose timestamp falls before the bound, and it arrives oldest first — a consumer working through a backlog gets it in the order the backlog accumulated, for nothing. The cursor is then (timestamp, id) rather than the id alone, and a cursor that never came from a bounded batch is refused with a plain message rather than a raw parse error.

Every entry names the version it was observed at

NodeEnumerationEntry carries a NodeVersionId. A walker that reads by it — getByIdAndVersionId, getVersion — works on the snapshot its enumeration described: the path and timestamp of the entry are those of what is read, whatever the node has become since. The export and the dump both do this now, so neither can write a node under a path it no longer has, or a timestamp it no longer carries.

A batch says how much is left

EnumerateNodesResult.getRemaining() is the number of entries this batch holds plus everything after it. It costs nothing (the index counts it while answering) and, since an enumeration filters nothing away, it is the real number rather than a guess. Consumers add it to what they have consumed already, so the first batch states the size of the whole walk and later batches correct it if the subtree changes. It is an int, saturated where the index's own long crosses into the API — the range a node count can use, and the range a script can hold.

2. The audit log vacuum stops using find — and stops refreshing

CleanUpAuditLogCommand was the worst find-and-delete loop left: query the search index, delete 10 000 records one at a time, refresh the search index, query again. It now enumerates the log from storage, bounded by the age threshold, and deletes what it is handed. There is no refresh anywhere in it: the cursor only moves forward over ground the deletions leave behind, and a record too fresh to be visible without a refresh is far too fresh to be expired.

One deliberate behavior change: a record is aged by its node timestamp — when it was written — where the search filtered the record's own time field, which a caller of AuditLogService.log may set freely. The log is add-only and written once, so the two coincide for every record the system writes; a backdated record now lives until its write time passes the threshold.

The clean-up also reports what it is doing, in the words the rest of the family uses. CleanUpAuditLogListener gained resolved, so the vacuum states how many records it has to delete — computed from the first batch's remaining count, not raised one batch at a time — and processed() became recordsDeleted(int), the count-carrying, domain-named method every other listener has. start(int batchSize) and finished() are deprecated: resolved already announces the work before the first delete, the batch size they published is the ceiling this API keeps private, and the clean-up is over when it returns. AuditLogCleanupTaskHandler logs those two moments around its own call and feeds the running count plus the total into the task's ProgressReporter — on a report interval of its own, with a final report when the clean-up returns, so a run that ends mid-interval still lands on its exact count.

3. Dump and export walk enumerations and read version blobs

Both walkers held the whole listing at once and used nothing of it but ids. Both now enumerate:

• NodeExporter enumerates the subtree, states its total from the first batch (1 + consumed + remaining), and reads each node by the version its entry named. It also collects the manual child order of each parent as the walk passes its children rather than reading the children again per ordered parent — the walk had them in hand. ExportNodesParams.batchSize no longer influences anything and is deprecated.
• RepoDumper keeps Map<NodeId, Map<Branch, NodeVersionId>>: the version each branch's scan observed. Version dumping then dumps exactly those versions instead of asking for whatever is active by the time it gets there, and getActiveVersions is gone from the dump path.

4. Move, duplicate, delete and apply-permissions walk what they already hold

These four rebuilt the whole subtree as Nodes before doing anything. They now work from the branch entries and the access-control blobs the entries point at, and fetch version data only for the nodes they actually touch:

• Move and duplicate walk entries in path order — a parent is always handled before its children — and complete nodes from version blobs a stride at a time. Duplicate settles what the caller may copy from the blobs alone, which are almost always cached, so its resolved total is exact from the first report; an unreadable node takes its subtree with it, which path order serves as the contiguous run behind it.
• Apply-permissions streams in strides. It resolves a bounded stride, applies it, reports, and moves on, instead of resolving the whole subtree first — so progress moves while the operation runs and the versions held at once are bounded. The total it reports is projected from the first stride rather than growing stride by stride.
• Version metadata is not fetched where nothing reads it. Only an attribute resolver reads it, so without one the walk skips it entirely.

Companion PR #12282 carries the same benchmark on master (NodeTreeOperationsBenchmark: move, duplicate, applyPermissions, delete over one parent with 10 000 children) so the two runs can be compared. Allocation churn is expected to match — every node is still read, rebuilt and re-indexed — and peak live set is the column this work moves.

5. Every operation listener names its resolved work the same way

There were three vocabularies for one idea: setTotal, nodeResolved/contentResolved, and nothing at all. All of them are now resolved( int ) — a running total that may be called any number of times, corrected in either direction, with -1 for "unknown again":

• Added to MoveNodeListener, DeleteNodeListener, DuplicateNodeListener, PushNodesListener, ArchiveContentListener, RestoreContentListener, DeleteContentListener, DuplicateContentListener, MoveContentListener and CleanUpAuditLogListener, and fired by the commands behind them.
• ApplyNodePermissionsListener.setTotal / ApplyPermissionsListener.setTotal are deprecated: a single upfront total cannot say "unknown", and it obliges the operation to resolve everything before it starts.
• NodeExportListener.nodeResolved, NodeImportListener.nodeResolved, PushContentListener.contentResolved and CleanUpAuditLogListener.processed() are deprecated but still reached: the new method's default forwards to the old one, never the other way round. Producers call the new name only, so inverting that direction would silently stop notifying any consumer that implements the old one — four bridge tests pin the direction, and the javadoc says which way it goes.
• The audit clean-up's own shape was the last one out of line, and is covered in section 2.

6. Layer synchronization stops rescanning the subtree

ParentContentSynchronizer descended level by level, and each level cost another subtree scan. It now takes the subtree once, grouped by parent, and slices it into levels — the descent still syncs a parent before its children. cleanDeletedContents does the same, from one upfront grouped fetch whose snapshot survives its own walk. Everything else in the sync consumes its listings in batches.

The delete guard was renamed after what it decides: hasNoChildren (which examined the whole subtree and inbound references) is now noDescendantSurvives and noInboundReferenceSurvives. The descendant check no longer walks the subtree either — one descendant outside the removal set settles it, so it asks for idsToRemove.size() + 1 entries, which for the clean-up flows is a couple.

Notes for review

• RangeFilter.from/to are inclusive (they set includeLower/includeUpper); the cursors use the exclusive gt/lt forms. The first itest run caught the difference as duplicated entries at every batch boundary.
• Public-API javadoc states the contract only — what the answer holds, what it costs, when a node becomes visible, how far at-most-once reaches, which exceptions come out — and no longer describes how any of it is produced.
• Id providers are listed through a search again: a listing of the providers would have to scan every user and group in the system, where a query narrows on the parent like every other principal lookup there.
• NodeQuery's _source was briefly filtered down to the requested return fields; the commit is reverted in place — complexity without a measurable win, since the whole _source crosses the wire per hit either way.

Verification

core-api (incl. EnumerateNodesParamsTest and the four listener-bridge tests), core-repo, core-content, core-audit, core-export, core-scheduler, core-app, core-security, app-system (incl. the clean-up progress test, which fails if the final report goes missing) and lib-project unit suites; the node, export, dump, audit, apply-permissions, push and content-publish itest packages, including new NodeServiceImplTest_enumerate cases for the admin gate, unfiltered entries, the timestamp bound, cursor semantics under concurrent deletes and moves, and a malformed cursor. The virtual-application and scheduled-job listings are pinned against descendants leaking into them — both assertions were verified to fail without the fix. The only local failures are the three known container-locale/charset cases (import_special_characters, ZipVirtualFileTest, ZipExportWriterTest), which pass in CI.

🤖 Generated with Claude Code

https://claude.ai/code/session_01PT3t5hDXj91Sk6idKRQwBs

[codacy-production]

Codacy's Analysis Summary

0 new issue (≤ 0 issue)
0 new security issue
130 complexity
More details

AI Reviewer: run a review on demand. To trigger the first review automatically, go to your organization or repository integration settings. AI can make mistakes. Always validate suggestions.

_{TIP This summary will be updated as you push new changes. Give us feedback}

[Copilot]

Pull request overview

This PR is a follow-up to #12247 that (1) adds cursor-based batching to NodeService.list(...) to avoid building huge in-memory listings, and (2) rewrites the audit log vacuum to enumerate records via batched listing rather than repeated search+refresh cycles.

Changes:

• Add batched listing support (batchSize + cursor) to NodeService.list, including API docs and integration tests covering empty batches, mixed-case cursors, and delete-while-scanning.
• Update node-branch listing internals to support cursor continuation via an exclusive range bound (gt/lt) on the path field.
• Rework audit log cleanup to scan entries in batches from storage and delete those older than the threshold, updating unit tests accordingly.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
modules/itest/itest-core/src/test/java/com/enonic/xp/core/node/NodeServiceImplTest_list.java	Adds integration coverage for cursor batching semantics (empty batches, mixed-case cursor continuation, deletion during scan, permission-filter advancement).
modules/core/core-repo/src/main/java/com/enonic/xp/repo/impl/node/NodeServiceImpl.java	Switches list(...) implementation to use the new batched execution path and propagate cursor into ListNodesResult.
modules/core/core-repo/src/main/java/com/enonic/xp/repo/impl/node/FindNodeBranchEntriesByParentCommand.java	Implements executeBatch() with size limiting and cursor continuation via a path range filter; keeps execute() for unbatched callers.
modules/core/core-audit/src/test/java/com/enonic/xp/core/impl/audit/AuditLogServiceImplTest.java	Updates audit cleanup tests to mock nodeService.list(...) batches instead of findByQuery(...), and adds a “keeps newer than threshold” case.
modules/core/core-audit/src/main/java/com/enonic/xp/core/impl/audit/CleanUpAuditLogCommand.java	Replaces the find-and-refresh loop with batched storage enumeration + timestamp check, preserving listener contract.
modules/core/core-api/src/main/java/com/enonic/xp/node/NodeService.java	Documents the new batching contract for list(...) (cursor-based continuation until cursor is null).
modules/core/core-api/src/main/java/com/enonic/xp/node/ListNodesResult.java	Adds nullable cursor to represent continuation for batched listings and updates result semantics/docs for empty batches.
modules/core/core-api/src/main/java/com/enonic/xp/node/ListNodesParams.java	Adds batchSize + cursor parameters and builder-time validation for invalid combinations.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[codecov]

Codecov Report

❌ Patch coverage is 93.15068% with 40 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.10%. Comparing base (b0341bd) to head (4526108).
⚠️ Report is 9 commits behind head on master.

Files with missing lines	Patch %	Lines
...a/com/enonic/xp/core/impl/export/NodeExporter.java	81.53%	8 Missing and 4 partials ⚠️
...c/xp/core/impl/app/ApplicationRepoServiceImpl.java	0.00%	6 Missing ⚠️
...nic/xp/core/impl/security/SecurityServiceImpl.java	0.00%	5 Missing ⚠️
...xp/repo/impl/node/ApplyNodePermissionsCommand.java	96.20%	0 Missing and 3 partials ⚠️
.../xp/core/impl/content/DeletedEventSyncCommand.java	81.81%	0 Missing and 2 partials ⚠️
...ic/xp/lib/project/SetProjectPublicReadHandler.java	0.00%	2 Missing ⚠️
...ava/com/enonic/xp/content/PushContentListener.java	66.66%	1 Missing ⚠️
.../java/com/enonic/xp/export/NodeExportListener.java	66.66%	1 Missing ⚠️
.../java/com/enonic/xp/export/NodeImportListener.java	66.66%	1 Missing ⚠️
...nic/xp/core/impl/content/LayersContentService.java	96.42%	0 Missing and 1 partial ⚠️
... and 6 more

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #12281      +/-   ##
============================================
+ Coverage     87.01%   87.10%   +0.08%     
- Complexity    20754    20842      +88     
============================================
  Files          2593     2610      +17     
  Lines         69055    69349     +294     
  Branches       5723     5749      +26     
============================================
+ Hits          60091    60406     +315     
+ Misses         6290     6270      -20     
+ Partials       2674     2673       -1     

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.