[COLLECTIONS-776] Fix expiration bypass in PassiveExpiringMap when wrapped in SynchronizedMap

[s8sankalp]

Description

When PassiveExpiringMap is decorated with Collections.synchronizedMap(), calling view methods like entrySet(), keySet(), or values() returns cached view decorators managed by SynchronizedMap. This bypasses PassiveExpiringMap's view methods and avoids triggering the passive eviction logic removeAllExpired() during iteration, resulting in expired entries remaining in the map indefinitely.

This PR introduces custom collection views (EntrySet, KeySet, ValuesCollection) and their respective iterators to:

1. Ensure removeAllExpired() is triggered on all read and write collection view operations.
2. Propagate iterator remove() actions to the map's internal expirationMap.
3. Preserve the O(1) complexity of remove() and removeAll() for the set views.

Verification

• Added JUnit regression test testCollectionsSynchronizedMapExpiration() verifying correct passive eviction when iterating/accessing collection views of a synchronized PassiveExpiringMap.
• Added JUnit regression test testCollectionViewRemoval() verifying correct cleanup of expirationMap when elements are removed via view iterators.
• All unit tests pass successfully.

[garydgregory]

-1 since applying only the test side of the patch doesn't cause any tests to fail. TDD is calling...

[garydgregory]

-1, see my comment.

[garydgregory]

Hello @s8sankalp

Thank you for the PR.

There is untested code in this PR. To see what's missing:

• Run: mvn clean verify site
• Open the site in: target/site/index.html
• Navigate to the JaCoCo report

You'll see in red what's missing, for example, null inputs on the removeAll() and retainAll() methods in the new classes.

In unit tests, this is an anti-pattern unless you are specifically testing for size() implementation instead of state:

assertEquals(0, entrySet.size());

Update or add:

assertTrue(entrySet.isEmpty());

You don't need blanks lines all over the place since you nicely document the steps with // comments.

[garydgregory]

Hello @s8sankalp

Why does PassiveExpiringMap.EntrySet.iterator() call PassiveExpiringMap.removeAllExpired(long) and PassiveExpiringMap.KeySet.iterator() does not?

Isn't that call either superfluous in the former or missing in the later?

Commenting out the call of PassiveExpiringMap.removeAllExpired(long) in PassiveExpiringMap.EntrySet.iterator() doesn't cause any test to fail.

This tells me at lease one test is missing.

Please check.

[s8sankalp]

Hello @s8sankalp

Why does PassiveExpiringMap.EntrySet.iterator() call PassiveExpiringMap.removeAllExpired(long) and PassiveExpiringMap.KeySet.iterator() does not?

Isn't that call either superfluous in the former or missing in the later?

Commenting out the call of PassiveExpiringMap.removeAllExpired(long) in PassiveExpiringMap.EntrySet.iterator() doesn't cause any test to fail.

This tells me at lease one test is missing.

Please check.

hi @garydgregory

The call to removeAllExpired() is not needed in KeySet.iterator() because both the key set and values iterators delegate directly to entrySet().iterator(), which already triggers the eviction check. Conversely, the eviction call in EntrySet.iterator() is necessary to support cached views. If a client caches a collection view (like the entry set) and waits for entries to expire before obtaining an iterator, the iterator must run the eviction check to avoid returning those expired elements. To verify this behavior and prevent future regressions, I have added a new unit test asserting that iterators created from cached views after expiration do not return any elements; this test now correctly fails if the eviction call in EntrySet.iterator() is removed.