This is an automated email from the ASF dual-hosted git repository.
garydgregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-collections.git
The following commit(s) were added to refs/heads/master by this push:
new dbf599d62 do not mutate the input map in switch closure/transformer
factories (#700)
dbf599d62 is described below
commit dbf599d62866a28f7d6ca7d054575b68beaaf55c
Author: Dexter.k <[email protected]>
AuthorDate: Thu Jul 2 12:21:09 2026 +0000
do not mutate the input map in switch closure/transformer factories (#700)
* do not mutate input map in switch closure/transformer factories
* assert built switch result is valid in map-copy tests
Signed-off-by: Naveed Khan <[email protected]>
---------
Signed-off-by: Naveed Khan <[email protected]>
---
src/changes/changes.xml | 1 +
.../apache/commons/collections4/ClosureUtils.java | 9 ++++---
.../commons/collections4/TransformerUtils.java | 9 ++++---
.../collections4/functors/SwitchClosure.java | 10 ++++----
.../commons/collections4/ClosureUtilsTest.java | 28 ++++++++++++++++++++++
.../commons/collections4/TransformerUtilsTest.java | 12 ++++++++++
.../functors/SwitchTransformerMapMutatesTest.java | 5 +++-
7 files changed, 63 insertions(+), 11 deletions(-)
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index eb1d1315d..ad12f961a 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -64,6 +64,7 @@
<action type="fix" dev="ggregory" due-to="Vasiliy Mikhailov, Gary
Gregory">Fix integer overflow in SetOperations.cosineSimilarity (#693).</action>
<action type="fix" dev="ggregory" due-to="Vasiliy Mikhailov, Gary
Gregory">Validate deserialized size in CircularFifoQueue.readObject
(#678).</action>
<action type="fix" dev="ggregory" due-to="Vasiliy Mikhailov, Gary
Gregory">Do not mutate the input map in SwitchTransformer.switchTransformer
(#696).</action>
+ <action type="fix" dev="ggregory" due-to="Naveed Khan, Gary Gregory">Do
not mutate the input map in SwitchClosure.switchClosure,
ClosureUtils.switchMapClosure and
TransformerUtils.switchMapTransformer.</action>
<!-- ADD -->
<action type="add" dev="ggregory" due-to="Gary Gregory">Add generics to
UnmodifiableIterator for the wrapped type.</action>
<action type="add" dev="ggregory" due-to="Gary Gregory">Add a Maven
benchmark profile for JMH.</action>
diff --git a/src/main/java/org/apache/commons/collections4/ClosureUtils.java
b/src/main/java/org/apache/commons/collections4/ClosureUtils.java
index 4ad525c4d..b90584a57 100644
--- a/src/main/java/org/apache/commons/collections4/ClosureUtils.java
+++ b/src/main/java/org/apache/commons/collections4/ClosureUtils.java
@@ -17,6 +17,7 @@
package org.apache.commons.collections4;
import java.util.Collection;
+import java.util.LinkedHashMap;
import java.util.Map;
import java.util.Objects;
@@ -324,12 +325,14 @@ public class ClosureUtils {
@SuppressWarnings("unchecked")
public static <E> Closure<E> switchMapClosure(final Map<? extends E,
Closure<E>> objectsAndClosures) {
Objects.requireNonNull(objectsAndClosures, "objectsAndClosures");
- final Closure<? super E> def = objectsAndClosures.remove(null);
- final int size = objectsAndClosures.size();
+ // copy so the caller's map is not mutated
+ final Map<? extends E, Closure<E>> objects = new
LinkedHashMap<>(objectsAndClosures);
+ final Closure<? super E> def = objects.remove(null);
+ final int size = objects.size();
final Closure<? super E>[] trs = new Closure[size];
final Predicate<E>[] preds = new Predicate[size];
int i = 0;
- for (final Map.Entry<? extends E, Closure<E>> entry :
objectsAndClosures.entrySet()) {
+ for (final Map.Entry<? extends E, Closure<E>> entry :
objects.entrySet()) {
preds[i] = EqualPredicate.<E>equalPredicate(entry.getKey());
trs[i] = entry.getValue();
i++;
diff --git
a/src/main/java/org/apache/commons/collections4/TransformerUtils.java
b/src/main/java/org/apache/commons/collections4/TransformerUtils.java
index 3b1bdceab..8376b78c2 100644
--- a/src/main/java/org/apache/commons/collections4/TransformerUtils.java
+++ b/src/main/java/org/apache/commons/collections4/TransformerUtils.java
@@ -17,6 +17,7 @@
package org.apache.commons.collections4;
import java.util.Collection;
+import java.util.LinkedHashMap;
import java.util.Map;
import java.util.Objects;
@@ -372,12 +373,14 @@ public class TransformerUtils {
final Map<I, Transformer<I, O>> objectsAndTransformers) {
Objects.requireNonNull(objectsAndTransformers,
"objectsAndTransformers");
- final Transformer<? super I, ? extends O> def =
objectsAndTransformers.remove(null);
- final int size = objectsAndTransformers.size();
+ // copy so the caller's map is not mutated
+ final Map<I, Transformer<I, O>> objects = new
LinkedHashMap<>(objectsAndTransformers);
+ final Transformer<? super I, ? extends O> def = objects.remove(null);
+ final int size = objects.size();
final Transformer<? super I, ? extends O>[] trs = new
Transformer[size];
final Predicate<I>[] preds = new Predicate[size];
int i = 0;
- for (final Map.Entry<I, Transformer<I, O>> entry :
objectsAndTransformers.entrySet()) {
+ for (final Map.Entry<I, Transformer<I, O>> entry : objects.entrySet())
{
preds[i] = EqualPredicate.<I>equalPredicate(entry.getKey());
trs[i++] = entry.getValue();
}
diff --git
a/src/main/java/org/apache/commons/collections4/functors/SwitchClosure.java
b/src/main/java/org/apache/commons/collections4/functors/SwitchClosure.java
index d3aff1985..12329473b 100644
--- a/src/main/java/org/apache/commons/collections4/functors/SwitchClosure.java
+++ b/src/main/java/org/apache/commons/collections4/functors/SwitchClosure.java
@@ -17,6 +17,7 @@
package org.apache.commons.collections4.functors;
import java.io.Serializable;
+import java.util.LinkedHashMap;
import java.util.Map;
import java.util.Objects;
@@ -57,16 +58,17 @@ public class SwitchClosure<T> implements Closure<T>,
Serializable {
@SuppressWarnings("unchecked")
public static <E> Closure<E> switchClosure(final Map<Predicate<E>,
Closure<E>> predicatesAndClosures) {
Objects.requireNonNull(predicatesAndClosures, "predicatesAndClosures");
- // convert to array like this to guarantee iterator() ordering
- final Closure<? super E> defaultClosure =
predicatesAndClosures.remove(null);
- final int size = predicatesAndClosures.size();
+ // copy so the caller's map is not mutated; LinkedHashMap preserves
iterator() ordering
+ final Map<Predicate<E>, Closure<E>> entries = new
LinkedHashMap<>(predicatesAndClosures);
+ final Closure<? super E> defaultClosure = entries.remove(null);
+ final int size = entries.size();
if (size == 0) {
return (Closure<E>) (defaultClosure == null ?
NOPClosure.<E>nopClosure() : defaultClosure);
}
final Closure<E>[] closures = new Closure[size];
final Predicate<E>[] preds = new Predicate[size];
int i = 0;
- for (final Map.Entry<Predicate<E>, Closure<E>> entry :
predicatesAndClosures.entrySet()) {
+ for (final Map.Entry<Predicate<E>, Closure<E>> entry :
entries.entrySet()) {
preds[i] = entry.getKey();
closures[i] = entry.getValue();
i++;
diff --git
a/src/test/java/org/apache/commons/collections4/ClosureUtilsTest.java
b/src/test/java/org/apache/commons/collections4/ClosureUtilsTest.java
index 546b2e0dd..4a7ea5f19 100644
--- a/src/test/java/org/apache/commons/collections4/ClosureUtilsTest.java
+++ b/src/test/java/org/apache/commons/collections4/ClosureUtilsTest.java
@@ -20,6 +20,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertSame;
import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import java.io.IOException;
import java.util.ArrayList;
@@ -309,6 +310,33 @@ class ClosureUtilsTest {
assertThrows(NullPointerException.class, () ->
ClosureUtils.switchMapClosure(null));
}
+ @Test
+ void testSwitchClosureDoesNotMutateInputMap() {
+ final MockClosure<String> def = new MockClosure<>();
+ final MockClosure<String> match = new MockClosure<>();
+ final Map<Predicate<String>, Closure<String>> predicateMap = new
HashMap<>();
+ predicateMap.put(null, def);
+ predicateMap.put(EqualPredicate.equalPredicate("HELLO"), match);
+ final Closure<String> closure =
ClosureUtils.switchClosure(predicateMap);
+ assertTrue(predicateMap.containsKey(null));
+ closure.execute("HELLO");
+ closure.execute("WORLD");
+ assertEquals(1, match.count);
+ assertEquals(1, def.count);
+
+ final MockClosure<String> mapDef = new MockClosure<>();
+ final MockClosure<String> mapMatch = new MockClosure<>();
+ final Map<String, Closure<String>> objectMap = new HashMap<>();
+ objectMap.put(null, mapDef);
+ objectMap.put("HELLO", mapMatch);
+ final Closure<String> mapClosure =
ClosureUtils.switchMapClosure(objectMap);
+ assertTrue(objectMap.containsKey(null));
+ mapClosure.execute("HELLO");
+ mapClosure.execute("WORLD");
+ assertEquals(1, mapMatch.count);
+ assertEquals(1, mapDef.count);
+ }
+
@Test
void testTransformerClosure() {
final MockTransformer<Object> mock = new MockTransformer<>();
diff --git
a/src/test/java/org/apache/commons/collections4/TransformerUtilsTest.java
b/src/test/java/org/apache/commons/collections4/TransformerUtilsTest.java
index 5029f2e60..9be5e3c78 100644
--- a/src/test/java/org/apache/commons/collections4/TransformerUtilsTest.java
+++ b/src/test/java/org/apache/commons/collections4/TransformerUtilsTest.java
@@ -21,6 +21,7 @@ import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertSame;
import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import java.io.IOException;
import java.util.ArrayList;
@@ -284,6 +285,17 @@ class TransformerUtilsTest {
assertThrows(NullPointerException.class, () ->
TransformerUtils.switchMapTransformer(null));
}
+ @Test
+ void testSwitchMapTransformerDoesNotMutateInputMap() {
+ final Map<String, Transformer<String, String>> map = new HashMap<>();
+ map.put(null, TransformerUtils.constantTransformer("default"));
+ map.put("HELLO", TransformerUtils.constantTransformer("A"));
+ final Transformer<String, String> transformer =
TransformerUtils.switchMapTransformer(map);
+ assertTrue(map.containsKey(null));
+ assertEquals("A", transformer.transform("HELLO"));
+ assertEquals("default", transformer.transform("WORLD"));
+ }
+
@Test
@SuppressWarnings("unchecked")
void testSwitchTransformer() {
diff --git
a/src/test/java/org/apache/commons/collections4/functors/SwitchTransformerMapMutatesTest.java
b/src/test/java/org/apache/commons/collections4/functors/SwitchTransformerMapMutatesTest.java
index 3cde12068..1bad1dbe4 100644
---
a/src/test/java/org/apache/commons/collections4/functors/SwitchTransformerMapMutatesTest.java
+++
b/src/test/java/org/apache/commons/collections4/functors/SwitchTransformerMapMutatesTest.java
@@ -44,9 +44,12 @@ class SwitchTransformerMapMutatesTest {
final int sizeBefore = map.size();
assertEquals(2, sizeBefore);
// Call the factory method
- SwitchTransformer.switchTransformer(map);
+ final Transformer<String, String> result =
SwitchTransformer.switchTransformer(map);
// The map should NOT have been mutated - null key must still be
present
final int sizeAfter = map.size();
assertEquals(sizeBefore, sizeAfter, "switchTransformer must not mutate
the input map; expected size");
+ // The returned transformer must still route via the copied map
+ assertEquals("value", result.transform(null));
+ assertEquals("default", result.transform("other"));
}
}
