s8sankalp commented on PR #694: URL: https://github.com/apache/commons-collections/pull/694#issuecomment-4826531303
> 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. 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. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
