Backport: Fix SearchableFields caching and CommitNow race condition (#426, #427)

[Shazwazza]

Summary

Backports two bug fixes from the v4 branch to support/3.x. Both bugs were discovered during Umbraco.Cms.Search compatibility testing.

Fixes

1. SearchableFields caches empty result from initially empty index (#426)

File: src/Examine.Lucene/Search/SearchContext.cs

SearchContext.SearchableFields caches its result after the first read. If accessed before any documents are indexed (e.g., during startup when ExamineMultiFieldQueryParser is constructed), an empty array is cached permanently. After documents are indexed, IsSearcherCurrent() returns true so the SearchContext is never recreated, leaving Search(string) / ManagedQuery() with zero fields to search against.

Fix: Only cache when the result is non-empty.

2. Race condition: IndexCommitted fires before NRT reader refresh (#427)

File: src/Examine.Lucene/Providers/LuceneIndex.cs (nested IndexCommiter class)

In the async commit path (TimerRelease), CommitNow() fires IndexCommitted before WaitForChanges() refreshes the NRT reader. Consumers reacting to the event may search with a stale reader.

Fix: Move WaitForChanges() into CommitNow() before firing the event.

Testing

• All 142 v3 tests pass on net8.0 (the pre-existing ContentDeliveryApiTests.cs build error is unrelated — it uses v4-only APIs)
• Both fixes were validated on v4 with full Umbraco.Cms.Search integration tests (628 passed, 0 failed)

[greptile-apps]

_{2 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

[Shazwazza]

@greptile review

[greptile-apps]

Greptile Summary

This PR backports two targeted bug fixes from the v4 branch to support/3.x, both discovered during Umbraco.Cms.Search integration testing.

• SearchContext.SearchableFields empty-cache fix (SearchContext.cs): Previously, accessing SearchableFields before any documents were indexed would cache an empty string[] permanently — because once IsSearcherCurrent() returned true the SearchContext was never recreated, leaving Search(string) / ManagedQuery() with no fields. The fix correctly skips caching when the result is empty, so fields are re-read on each access until documents arrive and a non-empty result can be safely stored.

• CommitNow() race condition fix (LuceneIndex.cs): In the async commit path (TimerRelease), IndexCommitted was fired before WaitForChanges() refreshed the NRT reader, so event handlers that immediately searched would see stale results. Moving WaitForChanges() inside CommitNow() — before the event — fixes all call sites (sync, async timer, cancellation) in one place, while the redundant post-call WaitForChanges() invocations are correctly removed.

Confidence Score: 4/5

• This PR is safe to merge — both changes are small, well-scoped, and directly address confirmed bugs with a passing test suite.
• Both fixes are minimal and targeted. The SearchableFields change introduces a repeat Acquire/Release on every access while the index is empty, which is acceptable overhead for a startup edge case. The CommitNow refactoring is logically equivalent for all synchronous callers and correctly fixes the ordering bug for the async path. No new public APIs are added and all 142 v3 tests pass.
• No files require special attention — both changes are straightforward and low-risk.

Important Files Changed

Filename	Overview
src/Examine.Lucene/Search/SearchContext.cs	Fixes permanent empty-cache bug in SearchableFields by only caching non-empty results; introduces repeated _searcherManager.Acquire() overhead if the index is persistently field-less (unlikely in practice).
src/Examine.Lucene/Providers/LuceneIndex.cs	Consolidates WaitForChanges() into CommitNow() so the NRT reader is guaranteed to be refreshed before IndexCommitted fires in all code paths (sync, async timer, cancellation).

Sequence Diagram

sequenceDiagram
    participant C as Caller
    participant CI as IndexCommiter
    participant IW as IndexWriter
    participant NRT as NrtReopenThread
    participant H as IndexCommitted Handler

    Note over C,H: BEFORE fix (async / TimerRelease path)
    C->>CI: ScheduleCommit()
    CI->>CI: TimerRelease()
    CI->>IW: Commit()
    CI->>H: IndexCommitted (⚠️ stale NRT reader!)
    CI->>NRT: WaitForChanges() (too late)

    Note over C,H: AFTER fix (all paths)
    C->>CI: CommitNow()
    CI->>IW: Commit()
    CI->>NRT: WaitForChanges() (reader refreshed ✅)
    CI->>H: IndexCommitted (fresh NRT reader ✅)

Loading

_{Last reviewed commit: 25c2036}

[greptile-apps]

_{2 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

Additional Comments (1)

src/Examine.Lucene/Providers/LuceneIndex.cs
Redundant WaitForChanges() calls in !RunAsync path

Now that CommitNow() unconditionally calls WaitForChanges() internally (line 801), the explicit WaitForChanges() calls in the synchronous (!RunAsync) paths are redundant. When CommitNow() returns, the NRT reader has already been refreshed, so the second call is a no-op (the NRT thread has already caught up to _latestGen). This isn't a correctness issue, but it leaves stale comments and may confuse future readers.

The same redundancy also appears in PerformDeleteFromIndexInternal (lines 519–522).

                    {
                        //commit the changes (this will also refresh the NRT reader and fire IndexCommitted)
                        _committer.CommitNow();
                    }

And likewise for the delete path (around line 519):

                    //commit the changes (this will also refresh the NRT reader and fire IndexCommitted)
                    _committer.CommitNow();

[Shazwazza]

@greptile review