WFLY-12014 Distributed session manager should not assume unmodifiable…

… sets are immutable
wildfly · Apr 25, 2019 · c1c756f · c1c756f
1 parent 51a1021
commit c1c756f
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 14 deletions.
diff --git a/...ring/ee/spi/src/main/java/org/wildfly/clustering/ee/immutable/CollectionImmutability.java b/...ring/ee/spi/src/main/java/org/wildfly/clustering/ee/immutable/CollectionImmutability.java
@@ -22,12 +22,12 @@
 
 package org.wildfly.clustering.ee.immutable;
 
+import java.util.AbstractMap;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
 
 import org.wildfly.clustering.ee.Immutability;
 
@@ -42,16 +42,18 @@ public class CollectionImmutability implements Immutability {
     private final List<Class<?>> unmodifiableCollectionClasses = Arrays.asList(
                 Collections.singleton(null).getClass(),
                 Collections.singletonList(null).getClass(),
-                Collections.singletonMap(null, null).getClass(),
                 Collections.unmodifiableCollection(Collections.emptyList()).getClass(),
                 Collections.unmodifiableList(Collections.emptyList()).getClass(),
-                Collections.unmodifiableMap(Collections.emptyMap()).getClass(),
-                Collections.unmodifiableNavigableMap(Collections.emptyNavigableMap()).getClass(),
                 Collections.unmodifiableNavigableSet(Collections.emptyNavigableSet()).getClass(),
                 Collections.unmodifiableSet(Collections.emptySet()).getClass(),
-                Collections.unmodifiableSortedMap(Collections.emptySortedMap()).getClass(),
                 Collections.unmodifiableSortedSet(Collections.emptySortedSet()).getClass());
 
+    private final List<Class<?>> unmodifiableMapClasses = Arrays.asList(
+                Collections.singletonMap(null, null).getClass(),
+                Collections.unmodifiableMap(Collections.emptyMap()).getClass(),
+                Collections.unmodifiableNavigableMap(Collections.emptyNavigableMap()).getClass(),
+                Collections.unmodifiableSortedMap(Collections.emptySortedMap()).getClass());
+
     private final Immutability elementImmutability;
 
     public CollectionImmutability(Immutability elementImmutability) {
@@ -62,17 +64,30 @@ public CollectionImmutability(Immutability elementImmutability) {
     public boolean test(Object object) {
         for (Class<?> unmodifiableCollectionClass : this.unmodifiableCollectionClasses) {
             if (unmodifiableCollectionClass.isInstance(object)) {
-                // An unmodifiable set should be immutable.
-                if (object instanceof Set) return true;
                 // An unmodifiable collection is immutable if its members are immutable.
-                // An unmodifiable map should be immutable if its values are immutable.
-                Collection<?> collection = (object instanceof Map) ? ((Map<?, ?>) object).values() : (Collection<?>) object;
-                for (Object element : collection) {
+                for (Object element : (Collection<?>) object) {
                     if (!this.elementImmutability.test(element)) return false;
                 }
                 return true;
             }
         }
+        for (Class<?> unmodifiableMapClass : this.unmodifiableMapClasses) {
+            if (unmodifiableMapClass.isInstance(object)) {
+                // An unmodifiable map is immutable if its entries are immutable.
+                for (Map.Entry<?, ?> entry : ((Map<?, ?>) object).entrySet()) {
+                    if (!this.test(entry)) return false;
+                }
+                return true;
+            }
+        }
+        if (object instanceof AbstractMap.SimpleImmutableEntry) {
+            return this.test((Map.Entry<?, ?>) object);
+        }
         return false;
     }
+
+    // An unmodifiable map entry is immutable if its key and value are immutable.
+    private boolean test(Map.Entry<?, ?> entry) {
+        return this.elementImmutability.test(entry.getKey()) && this.elementImmutability.test(entry.getValue());
+    }
 }
diff --git a/...tering/ee/spi/src/test/java/org/wildfly/clustering/ee/immutable/ImmutabilityTestCase.java b/...tering/ee/spi/src/test/java/org/wildfly/clustering/ee/immutable/ImmutabilityTestCase.java
@@ -53,6 +53,7 @@
 import java.time.zone.ZoneOffsetTransitionRule;
 import java.time.zone.ZoneOffsetTransitionRule.TimeDefinition;
 import java.time.zone.ZoneRules;
+import java.util.AbstractMap;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.Currency;
@@ -152,6 +153,7 @@ protected void test(Immutability immutability) throws Exception {
         assertTrue(immutability.test(Collections.singleton(new JCIPImmutableObject())));
         assertTrue(immutability.test(Collections.singletonList(new JCIPImmutableObject())));
         assertTrue(immutability.test(Collections.singletonMap("1", new JCIPImmutableObject())));
+        assertTrue(immutability.test(new AbstractMap.SimpleImmutableEntry<>("1", new JCIPImmutableObject())));
 
         assertTrue(immutability.test(Collections.unmodifiableCollection(Arrays.asList("1", "2"))));
         assertTrue(immutability.test(Collections.unmodifiableList(Arrays.asList("1", "2"))));
@@ -165,6 +167,7 @@ protected void test(Immutability immutability) throws Exception {
         Object mutableObject = new AtomicInteger();
         assertFalse(immutability.test(Collections.singletonList(mutableObject)));
         assertFalse(immutability.test(Collections.singletonMap("1", mutableObject)));
+        assertFalse(immutability.test(new AbstractMap.SimpleImmutableEntry<>("1", mutableObject)));
     }
 
     @net.jcip.annotations.Immutable

diff --git a/...rc/test/java/org/wildfly/clustering/web/session/SessionAttributeImmutabilityTestCase.java b/...rc/test/java/org/wildfly/clustering/web/session/SessionAttributeImmutabilityTestCase.java
@@ -24,12 +24,15 @@
 
 import static org.junit.Assert.assertTrue;
 
+import java.util.AbstractMap;
 import java.util.Collections;
 import java.util.EnumSet;
 
 import org.junit.Test;
+import org.wildfly.clustering.ee.CompositeIterable;
 import org.wildfly.clustering.ee.Immutability;
 import org.wildfly.clustering.ee.immutable.CompositeImmutability;
+import org.wildfly.clustering.ee.immutable.DefaultImmutability;
 import org.wildfly.clustering.ee.immutable.ImmutabilityTestCase;
 import org.wildfly.clustering.web.annotation.Immutable;
 
@@ -41,15 +44,18 @@ public class SessionAttributeImmutabilityTestCase extends ImmutabilityTestCase {
     @Override
     @Test
     public void test() throws Exception {
-        this.test(new CompositeImmutability(EnumSet.allOf(SessionAttributeImmutability.class)));
+        this.test(new CompositeImmutability(new CompositeIterable<>(EnumSet.allOf(DefaultImmutability.class), EnumSet.allOf(SessionAttributeImmutability.class))));
     }
 
     @Override
     protected void test(Immutability immutability) throws Exception {
+        ImmutableObject immutableObject = new ImmutableObject();
+
         assertTrue(immutability.test(new ImmutableObject()));
-        assertTrue(immutability.test(Collections.singleton(new ImmutableObject())));
-        assertTrue(immutability.test(Collections.singletonList(new ImmutableObject())));
-        assertTrue(immutability.test(Collections.singletonMap("1", new ImmutableObject())));
+        assertTrue(immutability.test(Collections.singleton(immutableObject)));
+        assertTrue(immutability.test(Collections.singletonList(immutableObject)));
+        assertTrue(immutability.test(Collections.singletonMap("1", immutableObject)));
+        assertTrue(immutability.test(new AbstractMap.SimpleImmutableEntry<>("1", immutableObject)));
     }
 
     @Immutable