From f9dac2caef563fc3b6ad0875b19c902da5874e01 Mon Sep 17 00:00:00 2001 From: Vasiliy Mikhailov Date: Fri, 26 Jun 2026 20:44:51 +0300 Subject: [PATCH 1/3] Do not mutate the input map in SwitchTransformer.switchTransformer switchTransformer(Map) extracted the default with map.remove(null), which removes the null key from the caller list map as a side effect. Use map.get(null) so the factory method leaves the input map unchanged. --- .../functors/SwitchTransformer.java | 12 ++-- .../SwitchTransformerMapMutatesTest.java | 58 +++++++++++++++++++ 2 files changed, 65 insertions(+), 5 deletions(-) create mode 100644 src/test/java/org/apache/commons/collections4/functors/SwitchTransformerMapMutatesTest.java diff --git a/src/main/java/org/apache/commons/collections4/functors/SwitchTransformer.java b/src/main/java/org/apache/commons/collections4/functors/SwitchTransformer.java index 213d1d5b95..a1dd1b6239 100644 --- a/src/main/java/org/apache/commons/collections4/functors/SwitchTransformer.java +++ b/src/main/java/org/apache/commons/collections4/functors/SwitchTransformer.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; @@ -63,9 +64,10 @@ public static Transformer switchTransformer( if (map.isEmpty()) { return ConstantTransformer.nullTransformer(); } - // convert to array like this to guarantee iterator() ordering - final Transformer defaultTransformer = map.remove(null); - final int size = map.size(); + // copy so the caller's map is not mutated; LinkedHashMap preserves iterator() ordering + final Map, Transformer> entries = new LinkedHashMap<>(map); + final Transformer defaultTransformer = entries.remove(null); + final int size = entries.size(); if (size == 0) { return (Transformer) (defaultTransformer == null ? ConstantTransformer.nullTransformer() : defaultTransformer); @@ -73,8 +75,8 @@ public static Transformer switchTransformer( final Transformer[] transformers = new Transformer[size]; final Predicate[] preds = new Predicate[size]; int i = 0; - for (final Map.Entry, - ? extends Transformer> entry : map.entrySet()) { + for (final Map.Entry, + Transformer> entry : entries.entrySet()) { preds[i] = entry.getKey(); transformers[i] = entry.getValue(); i++; diff --git a/src/test/java/org/apache/commons/collections4/functors/SwitchTransformerMapMutatesTest.java b/src/test/java/org/apache/commons/collections4/functors/SwitchTransformerMapMutatesTest.java new file mode 100644 index 0000000000..fda2fde223 --- /dev/null +++ b/src/test/java/org/apache/commons/collections4/functors/SwitchTransformerMapMutatesTest.java @@ -0,0 +1,58 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.commons.collections4.functors; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.util.LinkedHashMap; +import java.util.Map; + +import org.apache.commons.collections4.Predicate; +import org.apache.commons.collections4.Transformer; +import org.junit.jupiter.api.Test; + +/** + * Tests that SwitchTransformer.switchTransformer(Map) does not mutate the input map. + */ +class SwitchTransformerMapMutatesTest { + + /** + * Tests that switchTransformer(Map) does NOT mutate the input map by removing the null key. + */ + @Test + void testSwitchTransformerMapDoesNotMutateInput() { + final Transformer defaultTransformer = ConstantTransformer.constantTransformer("default"); + final Transformer transformer = ConstantTransformer.constantTransformer("value"); + final Predicate predicate = NullPredicate.INSTANCE; + + @SuppressWarnings("unchecked") + final Map, Transformer> map = new LinkedHashMap<>(); + map.put(null, defaultTransformer); + map.put(predicate, transformer); + + final int sizeBefore = map.size(); + assertEquals(2, sizeBefore, "map should have 2 entries before call"); + + // Call the factory method + 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 " + sizeBefore + " but got " + sizeAfter); + } +} From aa92c6ec89a55d3aa378ab777798443ace41bf19 Mon Sep 17 00:00:00 2001 From: Gary Gregory Date: Sun, 28 Jun 2026 08:06:38 -0400 Subject: [PATCH 2/3] Declutter new test. --- .../functors/SwitchTransformerMapMutatesTest.java | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) 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 fda2fde223..5c247cf5d1 100644 --- a/src/test/java/org/apache/commons/collections4/functors/SwitchTransformerMapMutatesTest.java +++ b/src/test/java/org/apache/commons/collections4/functors/SwitchTransformerMapMutatesTest.java @@ -38,21 +38,16 @@ void testSwitchTransformerMapDoesNotMutateInput() { final Transformer defaultTransformer = ConstantTransformer.constantTransformer("default"); final Transformer transformer = ConstantTransformer.constantTransformer("value"); final Predicate predicate = NullPredicate.INSTANCE; - @SuppressWarnings("unchecked") final Map, Transformer> map = new LinkedHashMap<>(); map.put(null, defaultTransformer); map.put(predicate, transformer); - final int sizeBefore = map.size(); - assertEquals(2, sizeBefore, "map should have 2 entries before call"); - + assertEquals(2, sizeBefore); // Call the factory method 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 " + sizeBefore + " but got " + sizeAfter); + assertEquals(sizeBefore, sizeAfter, "switchTransformer must not mutate the input map; expected size"); } } From 1b44759bdce8c7a7ae68241f490b2d824e2a7762 Mon Sep 17 00:00:00 2001 From: Gary Gregory Date: Sun, 28 Jun 2026 08:10:12 -0400 Subject: [PATCH 3/3] Fix mess in test. --- .../collections4/functors/SwitchTransformerMapMutatesTest.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) 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 5c247cf5d1..3cde120689 100644 --- a/src/test/java/org/apache/commons/collections4/functors/SwitchTransformerMapMutatesTest.java +++ b/src/test/java/org/apache/commons/collections4/functors/SwitchTransformerMapMutatesTest.java @@ -37,8 +37,7 @@ class SwitchTransformerMapMutatesTest { void testSwitchTransformerMapDoesNotMutateInput() { final Transformer defaultTransformer = ConstantTransformer.constantTransformer("default"); final Transformer transformer = ConstantTransformer.constantTransformer("value"); - final Predicate predicate = NullPredicate.INSTANCE; - @SuppressWarnings("unchecked") + final Predicate predicate = NullPredicate.nullPredicate(); final Map, Transformer> map = new LinkedHashMap<>(); map.put(null, defaultTransformer); map.put(predicate, transformer);