This is an automated email from the ASF dual-hosted git repository.
davsclaus pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/camel.git
The following commit(s) were added to refs/heads/main by this push:
new 62eed4b38acc CAMEL-26437: camel-util - FilterIterator: remove after a
failed next throws IllegalStateException (#28784)
62eed4b38acc is described below
commit 62eed4b38acc47acbcc83139b9f06ba9a136ed65
Author: allthingssecurity <[email protected]>
AuthorDate: Sun Nov 1 13:50:15 2026 +0530
CAMEL-26437: camel-util - FilterIterator: remove after a failed next throws
IllegalStateException (#28784)
Co-Authored-By: Claude Opus 5.5 <[email protected]>
---
.../java/org/apache/camel/util/FilterIterator.java | 10 ++-
.../camel/util/FilterIteratorRemoveTest.java | 76 +++++++++++++++++++++-
.../ROOT/pages/camel-4x-upgrade-guide-4_24.adoc | 6 ++
3 files changed, 88 insertions(+), 4 deletions(-)
diff --git
a/core/camel-util/src/main/java/org/apache/camel/util/FilterIterator.java
b/core/camel-util/src/main/java/org/apache/camel/util/FilterIterator.java
index 5482f7188eff..dc720c2dca1e 100644
--- a/core/camel-util/src/main/java/org/apache/camel/util/FilterIterator.java
+++ b/core/camel-util/src/main/java/org/apache/camel/util/FilterIterator.java
@@ -81,9 +81,10 @@ public class FilterIterator<T> implements Iterator<T>,
Closeable {
/**
* Removes the last element returned by {@link #next()} from the
underlying iterator.
*
- * @throws IllegalStateException if {@link #next()} has not been called,
{@link #remove()} has already been called
- * after the last call to {@link #next()},
or {@link #hasNext()} has been called after
- * it (the look-ahead has advanced the
underlying iterator past that element)
+ * @throws IllegalStateException if {@link #next()} has not been called,
if {@link #remove()} has already been
+ * called after the last call to {@link
#next()}, if {@link #hasNext()} was called
+ * after it, or if a later {@link #next()}
failed after it looked at further elements
+ * (they were all rejected or null, or the
filter or the underlying iterator threw)
*/
@Override
public void remove() {
@@ -96,6 +97,9 @@ public class FilterIterator<T> implements Iterator<T>,
Closeable {
protected T checkNext() {
while (!closed && it.hasNext()) {
+ // moving the underlying iterator past the element returned by
next() means it can no longer be removed,
+ // also when no accepted element follows (next() then throws
NoSuchElementException) or the filter throws
+ canRemove = false;
T ep = it.next();
if (ep != null && (filter == null || filter.test(ep))) {
return ep;
diff --git
a/core/camel-util/src/test/java/org/apache/camel/util/FilterIteratorRemoveTest.java
b/core/camel-util/src/test/java/org/apache/camel/util/FilterIteratorRemoveTest.java
index 71c2f189a1fb..c2257d888923 100644
---
a/core/camel-util/src/test/java/org/apache/camel/util/FilterIteratorRemoveTest.java
+++
b/core/camel-util/src/test/java/org/apache/camel/util/FilterIteratorRemoveTest.java
@@ -17,7 +17,9 @@
package org.apache.camel.util;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.List;
+import java.util.NoSuchElementException;
import org.junit.jupiter.api.Test;
@@ -28,7 +30,8 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
/**
* remove() removes the element last returned by next(); hasNext() looks ahead
in the underlying iterator, so after it
- * that element can no longer be removed and remove() must fail instead of
removing the element looked at.
+ * that element can no longer be removed and remove() must fail instead of
removing the element looked at. The same
+ * holds for a next() that fails after it looked at further elements (they
were all rejected, or the filter threw).
*/
class FilterIteratorRemoveTest {
@@ -94,4 +97,75 @@ class FilterIteratorRemoveTest {
it.remove();
assertEquals(List.of("a", "b"), list);
}
+
+ @Test
+ void removeAfterNextFoundOnlyRejectedElementsFailsAndRemovesNothing() {
+ List<String> list = new ArrayList<>(List.of("a", "b", "c"));
+ FilterIterator<String> it = new FilterIterator<>(list.iterator(),
"a"::equals);
+
+ assertEquals("a", it.next());
+ assertThrows(NoSuchElementException.class, it::next);
+ assertThrows(IllegalStateException.class, it::remove);
+ assertEquals(List.of("a", "b", "c"), list);
+ assertFalse(it.hasNext());
+ }
+
+ @Test
+ void removeAfterTheFilterThrowsFailsAndRemovesNothing() {
+ List<String> list = new ArrayList<>(List.of("a", "b", "c"));
+ FilterIterator<String> it = new FilterIterator<>(list.iterator(), s ->
{
+ if ("b".equals(s)) {
+ throw new IllegalArgumentException("cannot filter b");
+ }
+ return true;
+ });
+
+ assertEquals("a", it.next());
+ assertThrows(IllegalArgumentException.class, it::next);
+ assertThrows(IllegalStateException.class, it::remove);
+ assertEquals(List.of("a", "b", "c"), list);
+
+ // the iterator goes on after the element the filter failed on, and
the next element can be removed
+ assertEquals("c", it.next());
+ it.remove();
+ assertEquals(List.of("a", "b"), list);
+ }
+
+ @Test
+ void removeAfterNextSkippedANullElementFailsAndRemovesNothing() {
+ List<String> list = new ArrayList<>(Arrays.asList("a", null));
+ FilterIterator<String> it = new FilterIterator<>(list.iterator());
+
+ assertEquals("a", it.next());
+ assertThrows(NoSuchElementException.class, it::next);
+ assertThrows(IllegalStateException.class, it::remove);
+ assertEquals(Arrays.asList("a", null), list);
+ }
+
+ // control: at the end of the underlying iterator next() fails without
moving it, so the element returned last can
+ // still be removed (as with the iterator of a list)
+ @Test
+ void removeAfterNextAtTheEndRemovesTheLastReturnedElement() {
+ List<String> list = new ArrayList<>(List.of("a", "b"));
+ FilterIterator<String> it = new FilterIterator<>(list.iterator());
+
+ assertEquals("a", it.next());
+ assertEquals("b", it.next());
+ assertThrows(NoSuchElementException.class, it::next);
+ it.remove();
+ assertEquals(List.of("a"), list);
+ }
+
+ // control: hasNext() looked at the rejected elements before next() failed
+ @Test
+ void removeAfterHasNextFoundOnlyRejectedElementsFails() {
+ List<String> list = new ArrayList<>(List.of("a", "b"));
+ FilterIterator<String> it = new FilterIterator<>(list.iterator(),
"a"::equals);
+
+ assertEquals("a", it.next());
+ assertFalse(it.hasNext());
+ assertThrows(NoSuchElementException.class, it::next);
+ assertThrows(IllegalStateException.class, it::remove);
+ assertEquals(List.of("a", "b"), list);
+ }
}
diff --git
a/docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_24.adoc
b/docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_24.adoc
index c919c687b9a1..a827b2f9e442 100644
--- a/docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_24.adoc
+++ b/docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_24.adoc
@@ -279,6 +279,12 @@ removed the element after the one `next()` returned, and a
`remove()` before any
`remove()` now throws `IllegalStateException` unless it directly follows
`next()`. Code that called `hasNext()`
between `next()` and `remove()` must call `remove()` before `hasNext()`.
+The same applies to a `remove()` after a `next()` that failed after it looked
at further elements: a
+`NoSuchElementException` because the filter rejected all the elements that
were left (or they were null), or an
+exception thrown by the filter or by the iterator it wraps. Such a `remove()`
could remove an element that `next()`
+never returned; it now throws `IllegalStateException` and removes nothing. A
`next()` that fails at the end of the
+wrapped iterator, without looking at any element, still leaves the element
returned before it removable.
+
=== camel-util - CaseInsensitiveMap merges keys that differ only in the case
of a supplementary letter
`CaseInsensitiveMap`, which holds the message headers, now treats two keys
that differ only in the case of a letter