Don't trigger source refresh for TTL-expired entries when doing search()

[cb1kenobi]

When search() iterates over results and encounters a TTL-expired cache entry, transformEntryForSelect() would call ensureLoadedFromSource() and await the result before returning the record.

The fix separates the three cases that were conflated in a single condition:

• onlyIfCached + expired → return the "expired" indicator (unchanged)
• INVALIDATED or EVICTED → block and reload from source (unchanged)
• TTL-expired during search() → return stale cached data without touching the source; get() is the correct place to enforce TTL freshness

Note: I have no idea what I'm doing, but this seems to fix the flakey caching tests.

[kriszyp]

Hmm, this is clearer, but I think there are of couple cases that don't seem quite right:

• If the entry has expired, and context.onlyIfCached is false and checkLoaded is true, it should load from source, but it is not (it will return the stale/expired entry)
• If the entry is invalidated and context.onlyIfCached is true, I think it should return "This entry has expired/invalidated" (the current message is probably a little too terse).

[Ethan-Arrowood]

anyway to test this?

[cb1kenobi]

@Ethan-Arrowood These changes are intended to fix the flakey cached index test. The test performs a get(), search(), and get(), then expects the sourceRequests to be 2, but the small time window of the unit test seems to exhibit an issue where any TTL-expired entry is always loaded from source. We only want to load from source if the data has been invalidated; expired data is ok because yeah the data is stale, but the data is still readable. Remember this is in the context of search() where you could be potentially being fetching thousands of entries. If it had to check the expiration and load from source for every entry, performance would tank.

[kriszyp]

expired data is ok because yeah the data is stale,

The intent of onlyIfCached, is that if it is not set, you can consistently get fresh data across all operations, not fresh data for get and but only fresh data with search with invalidated and fresh data, but not expired data. With the corrected unit test in test/fix-cache-assertions, this branch consistently fails the unit test.

anyway to test this?

Added #265 to track.

[Ethan-Arrowood]

Whats up with the commits here? This seems to be bringing in a lot of other changes now

[cb1kenobi]

Whats up with the commits here? This seems to be bringing in a lot of other changes now

I configured git to rebase instead of merge, so I guess it shows the commits from main.

[Ethan-Arrowood]

okay I guess lets just be sure to squash merge this PR

[dawsontoth]

I configured git to rebase instead of merge

Observationally, it appears that the opposite is happening. I see merge commits.

[cb1kenobi]

Observationally, it appears that the opposite is happening. I see merge commits.

Bah, I did a git merge main instead of git rebase origin/main.