Make collection clones of IMap immutable (#12198)

hazelcast-client/src/main/java/com/hazelcast/client/proxy/ClientMapProxy.java

@@ -114,6 +114,7 @@
 import com.hazelcast.internal.journal.EventJournalReader;
 import com.hazelcast.internal.serialization.InternalSerializationService;
 import com.hazelcast.internal.util.SimpleCompletedFuture;
+import com.hazelcast.internal.util.collection.ImmutableInflatableSet;
 import com.hazelcast.map.EntryBackupProcessor;
 import com.hazelcast.map.EntryProcessor;
 import com.hazelcast.map.MapInterceptor;
@@ -154,7 +155,6 @@
 import com.hazelcast.util.CollectionUtil;
 import com.hazelcast.util.IterationType;
 import com.hazelcast.util.Preconditions;
-import com.hazelcast.util.collection.InflatableSet;
 
 import java.util.AbstractMap;
 import java.util.ArrayList;
@@ -1171,7 +1171,8 @@ public Set<K> keySet() {
         ClientMessage response = invoke(request);
         MapKeySetCodec.ResponseParameters resultParameters = MapKeySetCodec.decodeResponse(response);
 
-        InflatableSet.Builder<K> setBuilder = InflatableSet.newBuilder(resultParameters.response.size());
+        ImmutableInflatableSet.ImmutableSetBuilder<K> setBuilder =
+                ImmutableInflatableSet.newImmutableSetBuilder(resultParameters.response.size());
         for (Data data : resultParameters.response) {
             K key = toObject(data);
             setBuilder.add(key);
@@ -1182,7 +1183,8 @@ public Set<K> keySet() {
     @Override
     public Map<K, V> getAll(Set<K> keys) {
         if (CollectionUtil.isEmpty(keys)) {
-            return emptyMap();
+            // Wrap emptyMap() into unmodifiableMap to make sure put/putAll methods throw UnsupportedOperationException
+            return Collections.unmodifiableMap(emptyMap());
         }
 
         int keysSize = keys.size();
@@ -1196,7 +1198,7 @@ public Map<K, V> getAll(Set<K> keys) {
             V value = toObject(resultingKeyValuePairs.get(i++));
             result.put(key, value);
         }
-        return result;
+        return Collections.unmodifiableMap(result);
     }
 
     protected void getAllInternal(Set<K> keys, Map<Integer, List<Data>> partitionToKeyData, List<Object> resultingKeyValuePairs) {
@@ -1262,7 +1264,8 @@ public Set<Entry<K, V>> entrySet() {
         ClientMessage response = invoke(request);
         MapEntrySetCodec.ResponseParameters resultParameters = MapEntrySetCodec.decodeResponse(response);
 
-        InflatableSet.Builder<Entry<K, V>> setBuilder = InflatableSet.newBuilder(resultParameters.response.size());
+        ImmutableInflatableSet.ImmutableSetBuilder<Entry<K, V>> setBuilder =
+                ImmutableInflatableSet.newImmutableSetBuilder(resultParameters.response.size());
         InternalSerializationService serializationService = getContext().getSerializationService();
         for (Entry<Data, Data> row : resultParameters.response) {
             LazyMapEntry<K, V> entry = new LazyMapEntry<K, V>(row.getKey(), row.getValue(), serializationService);
@@ -1282,7 +1285,8 @@ public Set<K> keySet(Predicate predicate) {
         ClientMessage response = invokeWithPredicate(request, predicate);
         MapKeySetWithPredicateCodec.ResponseParameters resultParameters = MapKeySetWithPredicateCodec.decodeResponse(response);
 
-        InflatableSet.Builder<K> setBuilder = InflatableSet.newBuilder(resultParameters.response.size());
+        ImmutableInflatableSet.ImmutableSetBuilder<K> setBuilder =
+                ImmutableInflatableSet.newImmutableSetBuilder(resultParameters.response.size());
         for (Data data : resultParameters.response) {
             K key = toObject(data);
             setBuilder.add(key);
@@ -1319,7 +1323,8 @@ public Set<Entry<K, V>> entrySet(Predicate predicate) {
         ClientMessage response = invokeWithPredicate(request, predicate);
         MapEntriesWithPredicateCodec.ResponseParameters resultParameters = MapEntriesWithPredicateCodec.decodeResponse(response);
 
-        InflatableSet.Builder<Entry<K, V>> setBuilder = InflatableSet.newBuilder(resultParameters.response.size());
+        ImmutableInflatableSet.ImmutableSetBuilder<Entry<K, V>> setBuilder =
+                ImmutableInflatableSet.newImmutableSetBuilder(resultParameters.response.size());
         InternalSerializationService serializationService = getContext().getSerializationService();
         for (Entry<Data, Data> row : resultParameters.response) {
             LazyMapEntry<K, V> entry = new LazyMapEntry<K, V>(row.getKey(), row.getValue(), serializationService);

hazelcast-client/src/test/java/com/hazelcast/client/map/ClientMapBasicTest.java

@@ -21,6 +21,7 @@
 import com.hazelcast.core.EntryView;
 import com.hazelcast.core.IMap;
 import com.hazelcast.core.MapEvent;
+import com.hazelcast.map.BasicMapTest;
 import com.hazelcast.map.MapInterceptor;
 import com.hazelcast.map.listener.EntryEvictedListener;
 import com.hazelcast.monitor.LocalMapStats;
@@ -930,6 +931,11 @@ public void testPutAll() {
         }
     }
 
+    @Test
+    public void testMapClonedCollectionsImmutable() {
+        BasicMapTest.testMapClonedCollectionsImmutable(client, false);
+    }
+
     @Test
     public void testGetAll() {
         int max = 100;

hazelcast/src/main/java/com/hazelcast/client/impl/protocol/task/map/MapPublisherCreateMessageTask.java

@@ -23,6 +23,7 @@
 import com.hazelcast.instance.MemberImpl;
 import com.hazelcast.instance.Node;
 import com.hazelcast.internal.cluster.ClusterService;
+import com.hazelcast.internal.util.collection.InflatableSet;
 import com.hazelcast.map.impl.MapService;
 import com.hazelcast.map.impl.query.QueryResult;
 import com.hazelcast.map.impl.query.QueryResultRow;
@@ -35,7 +36,6 @@
 import com.hazelcast.spi.InvocationBuilder;
 import com.hazelcast.spi.impl.operationservice.InternalOperationService;
 import com.hazelcast.util.ExceptionUtil;
-import com.hazelcast.util.collection.InflatableSet;
 
 import java.security.Permission;
 import java.util.ArrayList;

hazelcast/src/main/java/com/hazelcast/client/impl/protocol/task/map/MapPublisherCreateWithValueMessageTask.java

@@ -23,6 +23,7 @@
 import com.hazelcast.instance.MemberImpl;
 import com.hazelcast.instance.Node;
 import com.hazelcast.internal.cluster.ClusterService;
+import com.hazelcast.internal.util.collection.InflatableSet;
 import com.hazelcast.map.impl.MapService;
 import com.hazelcast.map.impl.query.QueryResult;
 import com.hazelcast.map.impl.query.QueryResultRow;
@@ -35,7 +36,6 @@
 import com.hazelcast.spi.InvocationBuilder;
 import com.hazelcast.spi.impl.operationservice.InternalOperationService;
 import com.hazelcast.util.ExceptionUtil;
-import com.hazelcast.util.collection.InflatableSet;
 
 import java.security.Permission;
 import java.util.AbstractMap;

hazelcast/src/main/java/com/hazelcast/core/IMap.java

@@ -59,9 +59,9 @@
  * <li>The {@code get} method returns a clone of original values, so modifying the returned value does not change
  * the actual value in the map. You should put the modified value back to make changes visible to all nodes.
  * For additional info, see {@link IMap#get(Object)}.</li>
- * <li>Methods, including but not limited to {@code keySet}, {@code values}, {@code entrySet}, return a collection
- * clone of the values. The collection is <b>NOT</b> backed by the map, so changes to the map are <b>NOT</b> reflected
- * in the collection, and vice-versa.</li>
+ * <li>Methods, including but not limited to {@code keySet}, {@code values}, {@code entrySet}, return an <b>immutable</b>
+ * collection clone of the values. The collection is <b>NOT</b> backed by the map, so changes to the map are <b>NOT</b>
+ * reflected in the collection.</li>
  * <li>Since Hazelcast is compiled with Java 1.6, we can't override default methods introduced in later Java versions,
  * nor can we add documentation to them. Methods, including but not limited to {@code computeIfPresent}, may behave
  * incorrectly if the value passed to the update function is modified in-place and returned as a result of the invocation.
@@ -470,13 +470,13 @@ public interface IMap<K, V> extends ConcurrentMap<K, V>, BaseMap<K, V> {
     void flush();
 
     /**
-     * Returns the entries for the given keys.
+     * Returns an immutable map of entries for the given keys.
      * <p>
      * <b>Warning 1:</b>
      * <p>
      * The returned map is <b>NOT</b> backed by the original map, so
      * changes to the original map are <b>NOT</b> reflected in the
-     * returned map, and vice-versa.
+     * returned map.
      * <p>
      * <b>Warning 2:</b>
      * <p>
@@ -493,7 +493,7 @@ public interface IMap<K, V> extends ConcurrentMap<K, V>, BaseMap<K, V> {
      * and are propagated to the caller.
      *
      * @param keys keys to get (keys inside the collection cannot be null)
-     * @return map of entries
+     * @return an immutable map of entries
      * @throws NullPointerException if any of the specified keys are null
      */
     Map<K, V> getAll(Set<K> keys);
@@ -2252,69 +2252,69 @@ boolean tryLock(K key, long time, TimeUnit timeunit, long leaseTime, TimeUnit le
     void evictAll();
 
     /**
-     * Returns a set clone of the keys contained in this map.
+     * Returns an immutable set clone of the keys contained in this map.
      * <p>
      * <b>Warning:</b>
      * <p>
      * The set is <b>NOT</b> backed by the map,
-     * so changes to the map are <b>NOT</b> reflected in the set, and vice-versa.
+     * so changes to the map are <b>NOT</b> reflected in the set.
      * <p>
      * This method is always executed by a distributed query,
      * so it may throw a {@link QueryResultSizeExceededException}
      * if {@link GroupProperty#QUERY_RESULT_SIZE_LIMIT} is configured.
      *
-     * @return a set clone of the keys contained in this map
+     * @return an immutable set clone of the keys contained in this map
      * @throws QueryResultSizeExceededException if query result size limit is exceeded
      * @see GroupProperty#QUERY_RESULT_SIZE_LIMIT
      */
     Set<K> keySet();
 
     /**
-     * Returns a collection clone of the values contained in this map.
+     * Returns an immutable collection clone of the values contained in this map.
      * <p>
      * <b>Warning:</b>
      * <p>
      * The collection is <b>NOT</b> backed by the map,
-     * so changes to the map are <b>NOT</b> reflected in the collection, and vice-versa.
+     * so changes to the map are <b>NOT</b> reflected in the collection.
      * <p>
      * This method is always executed by a distributed query,
      * so it may throw a {@link QueryResultSizeExceededException}
      * if {@link GroupProperty#QUERY_RESULT_SIZE_LIMIT} is configured.
      *
-     * @return a collection clone of the values contained in this map
+     * @return an immutable collection clone of the values contained in this map
      * @throws QueryResultSizeExceededException if query result size limit is exceeded
      * @see GroupProperty#QUERY_RESULT_SIZE_LIMIT
      */
     Collection<V> values();
 
     /**
-     * Returns a {@link Set} clone of the mappings contained in this map.
+     * Returns an immutable {@link Set} clone of the mappings contained in this map.
      * <p>
      * <b>Warning:</b>
      * <p>
      * The set is <b>NOT</b> backed by the map,
-     * so changes to the map are <b>NOT</b> reflected in the set, and vice-versa.
+     * so changes to the map are <b>NOT</b> reflected in the set.
      * <p>
      * This method is always executed by a distributed query,
      * so it may throw a {@link QueryResultSizeExceededException}
      * if {@link GroupProperty#QUERY_RESULT_SIZE_LIMIT} is configured.
      *
-     * @return a set clone of the keys mappings in this map
+     * @return an immutable set clone of the keys mappings in this map
      * @throws QueryResultSizeExceededException if query result size limit is exceeded
      * @see GroupProperty#QUERY_RESULT_SIZE_LIMIT
      */
     Set<Map.Entry<K, V>> entrySet();
 
     /**
      * Queries the map based on the specified predicate and
-     * returns the keys of matching entries.
+     * returns an immutable {@link Set} clone of the keys of matching entries.
      * <p>
      * Specified predicate runs on all members in parallel.
      * <p>
      * <b>Warning:</b>
      * <p>
      * The set is <b>NOT</b> backed by the map,
-     * so changes to the map are <b>NOT</b> reflected in the set, and vice-versa.
+     * so changes to the map are <b>NOT</b> reflected in the set.
      * <p>
      * This method is always executed by a distributed query,
      * so it may throw a {@link QueryResultSizeExceededException}
@@ -2329,14 +2329,14 @@ boolean tryLock(K key, long time, TimeUnit timeunit, long leaseTime, TimeUnit le
     Set<K> keySet(Predicate predicate);
 
     /**
-     * Queries the map based on the specified predicate and returns the matching entries.
+     * Queries the map based on the specified predicate and returns an immutable set of the matching entries.
      * <p>
      * Specified predicate runs on all members in parallel.
      * <p>
      * <b>Warning:</b>
      * <p>
      * The set is <b>NOT</b> backed by the map,
-     * so changes to the map are <b>NOT</b> reflected in the set, and vice-versa.
+     * so changes to the map are <b>NOT</b> reflected in the set.
      * <p>
      * This method is always executed by a distributed query,
      * so it may throw a {@link QueryResultSizeExceededException}
@@ -2351,14 +2351,15 @@ boolean tryLock(K key, long time, TimeUnit timeunit, long leaseTime, TimeUnit le
     Set<Map.Entry<K, V>> entrySet(Predicate predicate);
 
     /**
-     * Queries the map based on the specified predicate and returns the values of matching entries.
+     * Queries the map based on the specified predicate and returns an immutable collection of the values
+     * of matching entries.
      * <p>
      * Specified predicate runs on all members in parallel.
      * <p>
      * <b>Warning:</b>
      * <p>
      * The collection is <b>NOT</b> backed by the map,
-     * so changes to the map are <b>NOT</b> reflected in the collection, and vice-versa.
+     * so changes to the map are <b>NOT</b> reflected in the collection.
      * <p>
      * This method is always executed by a distributed query,
      * so it may throw a {@link QueryResultSizeExceededException}
@@ -2373,7 +2374,7 @@ boolean tryLock(K key, long time, TimeUnit timeunit, long leaseTime, TimeUnit le
     Collection<V> values(Predicate predicate);
 
     /**
-     * Returns the locally owned set of keys.
+     * Returns the locally owned immutable set of keys.
      * <p>
      * Each key in this map is owned and managed by a specific
      * member in the cluster.
@@ -2385,20 +2386,20 @@ boolean tryLock(K key, long time, TimeUnit timeunit, long leaseTime, TimeUnit le
      * <b>Warning:</b>
      * <p>
      * The set is <b>NOT</b> backed by the map,
-     * so changes to the map are <b>NOT</b> reflected in the set, and vice-versa.
+     * so changes to the map are <b>NOT</b> reflected in the set.
      * <p>
      * This method is always executed by a distributed query,
      * so it may throw a {@link QueryResultSizeExceededException}
      * if {@link GroupProperty#QUERY_RESULT_SIZE_LIMIT} is configured.
      *
-     * @return locally owned keys
+     * @return locally owned immutable set of keys
      * @throws QueryResultSizeExceededException if query result size limit is exceeded
      * @see GroupProperty#QUERY_RESULT_SIZE_LIMIT
      */
     Set<K> localKeySet();
 
     /**
-     * Returns the keys of matching locally owned entries.
+     * Returns an immutable set of the keys of matching locally owned entries.
      * <p>
      * Each key in this map is owned and managed by a specific
      * member in the cluster.
@@ -2410,14 +2411,14 @@ boolean tryLock(K key, long time, TimeUnit timeunit, long leaseTime, TimeUnit le
      * <b>Warning:</b>
      * <p>
      * The set is <b>NOT</b> backed by the map,
-     * so changes to the map are <b>NOT</b> reflected in the set, and vice-versa.
+     * so changes to the map are <b>NOT</b> reflected in the set.
      * <p>
      * This method is always executed by a distributed query,
      * so it may throw a {@link QueryResultSizeExceededException}
      * if {@link GroupProperty#QUERY_RESULT_SIZE_LIMIT} is configured.
      *
      * @param predicate specified query criteria
-     * @return keys of matching locally owned entries
+     * @return an immutable set of the keys of matching locally owned entries
      * @throws QueryResultSizeExceededException if query result size limit is exceeded
      * @see GroupProperty#QUERY_RESULT_SIZE_LIMIT
      */

hazelcast/src/main/java/com/hazelcast/internal/nearcache/impl/preloader/NearCachePreloader.java

@@ -20,14 +20,14 @@
 import com.hazelcast.internal.adapter.DataStructureAdapter;
 import com.hazelcast.internal.serialization.impl.HeapData;
 import com.hazelcast.internal.util.BufferingInputStream;
+import com.hazelcast.internal.util.collection.InflatableSet;
+import com.hazelcast.internal.util.collection.InflatableSet.Builder;
 import com.hazelcast.logging.ILogger;
 import com.hazelcast.logging.Logger;
 import com.hazelcast.memory.MemoryUnit;
 import com.hazelcast.monitor.impl.NearCacheStatsImpl;
 import com.hazelcast.nio.serialization.Data;
 import com.hazelcast.spi.serialization.SerializationService;
-import com.hazelcast.util.collection.InflatableSet;
-import com.hazelcast.util.collection.InflatableSet.Builder;
 
 import java.io.File;
 import java.io.FileInputStream;

hazelcast/src/main/java/com/hazelcast/internal/util/ResultSet.java

@@ -19,10 +19,12 @@
 import com.hazelcast.util.IterationType;
 
 import java.util.AbstractSet;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.function.Predicate;
 
 /**
  *
@@ -88,4 +90,33 @@ public void remove() {
             throw new UnsupportedOperationException();
         }
     }
+
+    @Override
+    public boolean addAll(Collection c) {
+        throw new UnsupportedOperationException();
+    }
+
+    @Override
+    public boolean remove(Object o) {
+        throw new UnsupportedOperationException();
+    }
+
+    @Override
+    public boolean removeAll(Collection<?> c) {
+        throw new UnsupportedOperationException();
+    }
+
+    @Override
+    public boolean removeIf(Predicate filter) {
+        throw new UnsupportedOperationException();
+    }
+
+    public boolean retainAll(Collection<?> coll) {
+        throw new UnsupportedOperationException();
+    }
+
+    @Override
+    public void clear() {
+        throw new UnsupportedOperationException();
+    }
 }