diff --git a/api/src/main/java/org/apache/iceberg/util/CharSequenceMap.java b/api/src/main/java/org/apache/iceberg/util/CharSequenceMap.java index e9595b45d559..e2d517242c1d 100644 --- a/api/src/main/java/org/apache/iceberg/util/CharSequenceMap.java +++ b/api/src/main/java/org/apache/iceberg/util/CharSequenceMap.java @@ -20,6 +20,7 @@ import java.io.Serializable; import java.util.Collection; +import java.util.Collections; import java.util.Map; import java.util.Objects; import java.util.Set; @@ -132,12 +133,12 @@ public Set keySet() { keySet.add(wrapper.get()); } - return keySet; + return Collections.unmodifiableSet(keySet); } @Override public Collection values() { - return wrapperMap.values(); + return Collections.unmodifiableCollection(wrapperMap.values()); } @Override @@ -148,7 +149,7 @@ public Set> entrySet() { entrySet.add(new CharSequenceEntry<>(entry)); } - return entrySet; + return Collections.unmodifiableSet(entrySet); } public V computeIfAbsent(CharSequence key, Supplier valueSupplier) { diff --git a/api/src/test/java/org/apache/iceberg/util/TestCharSequenceMap.java b/api/src/test/java/org/apache/iceberg/util/TestCharSequenceMap.java index 0d74aa5bf2f2..bd8a95c30043 100644 --- a/api/src/test/java/org/apache/iceberg/util/TestCharSequenceMap.java +++ b/api/src/test/java/org/apache/iceberg/util/TestCharSequenceMap.java @@ -19,7 +19,11 @@ package org.apache.iceberg.util; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import java.util.Collection; +import java.util.Map; +import java.util.Set; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; @@ -88,12 +92,26 @@ public void testClear() { assertThat(map).isEmpty(); } + @Test + public void testKeySet() { + CharSequenceMap map = CharSequenceMap.create(); + map.put("key1", "value1"); + map.put("key2", "value2"); + Set keySet = map.keySet(); + assertThat(keySet).containsAll(ImmutableList.of("key1", "key2")); + assertThatThrownBy(() -> keySet.remove("key1")) + .isInstanceOf(UnsupportedOperationException.class); + } + @Test public void testValues() { CharSequenceMap map = CharSequenceMap.create(); map.put("key1", "value1"); map.put("key2", "value2"); - assertThat(map.values()).containsAll(ImmutableList.of("value1", "value2")); + Collection values = map.values(); + assertThat(values).containsAll(ImmutableList.of("value1", "value2")); + assertThatThrownBy(() -> values.remove("value1")) + .isInstanceOf(UnsupportedOperationException.class); } @Test @@ -101,7 +119,11 @@ public void testEntrySet() { CharSequenceMap map = CharSequenceMap.create(); map.put("key1", "value1"); map.put(new StringBuilder("key2"), "value2"); - assertThat(map.entrySet()).hasSize(2); + Set> entrySet = map.entrySet(); + assertThat(entrySet).hasSize(2); + Map.Entry entryToRemove = entrySet.iterator().next(); + assertThatThrownBy(() -> entrySet.remove(entryToRemove)) + .isInstanceOf(UnsupportedOperationException.class); } @Test diff --git a/core/src/main/java/org/apache/iceberg/util/StructLikeMap.java b/core/src/main/java/org/apache/iceberg/util/StructLikeMap.java index 2bb5fa1c9d40..637d891ce5e1 100644 --- a/core/src/main/java/org/apache/iceberg/util/StructLikeMap.java +++ b/core/src/main/java/org/apache/iceberg/util/StructLikeMap.java @@ -20,6 +20,7 @@ import java.util.AbstractMap; import java.util.Collection; +import java.util.Collections; import java.util.Map; import java.util.Set; import java.util.function.Function; @@ -108,12 +109,12 @@ public Set keySet() { for (StructLikeWrapper wrapper : wrapperMap.keySet()) { keySet.add(wrapper.get()); } - return keySet; + return Collections.unmodifiableSet(keySet); } @Override public Collection values() { - return wrapperMap.values(); + return Collections.unmodifiableCollection(wrapperMap.values()); } @Override @@ -122,7 +123,7 @@ public Set> entrySet() { for (Entry entry : wrapperMap.entrySet()) { entrySet.add(new StructLikeEntry<>(entry)); } - return entrySet; + return Collections.unmodifiableSet(entrySet); } private static class StructLikeEntry implements Entry { diff --git a/core/src/test/java/org/apache/iceberg/util/TestStructLikeMap.java b/core/src/test/java/org/apache/iceberg/util/TestStructLikeMap.java index f18c48eaa344..865de081e5b2 100644 --- a/core/src/test/java/org/apache/iceberg/util/TestStructLikeMap.java +++ b/core/src/test/java/org/apache/iceberg/util/TestStructLikeMap.java @@ -19,6 +19,7 @@ package org.apache.iceberg.util; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import java.util.Collection; import java.util.Map; @@ -63,6 +64,16 @@ public void testSingleRecord() { assertThat(entry.getValue()).isEqualTo("1-aaa"); break; } + + Map.Entry entryToRemove = entrySet.iterator().next(); + assertThatThrownBy(() -> entrySet.remove(entryToRemove)) + .isInstanceOf(UnsupportedOperationException.class); + + assertThatThrownBy(() -> keySet.remove(record1)) + .isInstanceOf(UnsupportedOperationException.class); + + assertThatThrownBy(() -> values.remove("1-aaa")) + .isInstanceOf(UnsupportedOperationException.class); } @Test