disable modifiable collection transformer for Maps (#554)

* disable modifiable collection transformer for Maps * test updates
linkedin · Mar 13, 2024 · 46c92a9 · 46c92a9
1 parent 2fd93ac
commit 46c92a9
Show file tree

Hide file tree

Showing 17 changed files with 124 additions and 378 deletions.
diff --git a/avro-builder/tests/codegen-110/src/main/avro/vs110/TestCollections.avsc b/avro-builder/tests/codegen-110/src/main/avro/vs110/TestCollections.avsc
@@ -78,6 +78,16 @@
         "type": "array",
         "items": "int"
       }
+    },
+    {
+      "name": "unionOfIntMap",
+      "type": [
+        "null",
+        {
+          "type": "map",
+          "values": "int"
+        }
+      ]
     }
   ],
   "type": "record"

diff --git a/avro-builder/tests/codegen-111/src/main/avro/vs111/TestCollections.avsc b/avro-builder/tests/codegen-111/src/main/avro/vs111/TestCollections.avsc
@@ -79,6 +79,16 @@
         "type": "array",
         "items": "int"
       }
+    },
+    {
+      "name": "unionOfIntMap",
+      "type": [
+        "null",
+        {
+          "type": "map",
+          "values": "int"
+        }
+      ]
     }
   ],
   "type": "record"

diff --git a/avro-builder/tests/codegen-14/src/main/avro/vs14/TestCollections.avsc b/avro-builder/tests/codegen-14/src/main/avro/vs14/TestCollections.avsc
@@ -79,6 +79,16 @@
         "type": "array",
         "items": "int"
       }
+    },
+    {
+      "name": "unionOfIntMap",
+      "type": [
+        "null",
+        {
+          "type": "map",
+          "values": "int"
+        }
+      ]
     }
   ],
   "type": "record"

diff --git a/avro-builder/tests/codegen-15/src/main/avro/vs15/TestCollections.avsc b/avro-builder/tests/codegen-15/src/main/avro/vs15/TestCollections.avsc
@@ -79,6 +79,16 @@
         "type": "array",
         "items": "int"
       }
+    },
+    {
+      "name": "unionOfIntMap",
+      "type": [
+        "null",
+        {
+          "type": "map",
+          "values": "int"
+        }
+      ]
     }
   ],
   "type": "record"

diff --git a/avro-builder/tests/codegen-16/src/main/avro/vs16/TestCollections.avsc b/avro-builder/tests/codegen-16/src/main/avro/vs16/TestCollections.avsc
@@ -79,6 +79,16 @@
         "type": "array",
         "items": "int"
       }
+    },
+    {
+      "name": "unionOfIntMap",
+      "type": [
+        "null",
+        {
+          "type": "map",
+          "values": "int"
+        }
+      ]
     }
   ],
   "type": "record"

diff --git a/avro-builder/tests/codegen-17/src/main/avro/vs17/TestCollections.avsc b/avro-builder/tests/codegen-17/src/main/avro/vs17/TestCollections.avsc
@@ -79,6 +79,16 @@
         "type": "array",
         "items": "int"
       }
+    },
+    {
+      "name": "unionOfIntMap",
+      "type": [
+        "null",
+        {
+          "type": "map",
+          "values": "int"
+        }
+      ]
     }
   ],
   "type": "record"

diff --git a/avro-builder/tests/codegen-18/src/main/avro/vs18/TestCollections.avsc b/avro-builder/tests/codegen-18/src/main/avro/vs18/TestCollections.avsc
@@ -79,6 +79,16 @@
         "type": "array",
         "items": "int"
       }
+    },
+    {
+      "name": "unionOfIntMap",
+      "type": [
+        "null",
+        {
+          "type": "map",
+          "values": "int"
+        }
+      ]
     }
   ],
   "type": "record"

diff --git a/avro-builder/tests/codegen-19/src/main/avro/vs19/TestCollections.avsc b/avro-builder/tests/codegen-19/src/main/avro/vs19/TestCollections.avsc
@@ -79,6 +79,16 @@
         "type": "array",
         "items": "int"
       }
+    },
+    {
+      "name": "unionOfIntMap",
+      "type": [
+        "null",
+        {
+          "type": "map",
+          "values": "int"
+        }
+      ]
     }
   ],
   "type": "record"

diff --git a/avro-builder/tests/codegen-charseq-method/src/main/avro/charseqmethod/TestCollections.avsc b/avro-builder/tests/codegen-charseq-method/src/main/avro/charseqmethod/TestCollections.avsc
@@ -79,6 +79,16 @@
         "type": "array",
         "items": "int"
       }
+    },
+    {
+      "name": "unionOfIntMap",
+      "type": [
+        "null",
+        {
+          "type": "map",
+          "values": "int"
+        }
+      ]
     }
   ],
   "type": "record"

diff --git a/...lder/tests/codegen-no-utf8-in-putbyindex/src/main/avro/charseqmethod/TestCollections.avsc b/...lder/tests/codegen-no-utf8-in-putbyindex/src/main/avro/charseqmethod/TestCollections.avsc
@@ -79,6 +79,16 @@
         "type": "array",
         "items": "int"
       }
+    },
+    {
+      "name": "unionOfIntMap",
+      "type": [
+        "null",
+        {
+          "type": "map",
+          "values": "int"
+        }
+      ]
     }
   ],
   "type": "record"

diff --git a/.../tests/tests-allavro/src/test/java/com/linkedin/avroutil1/builder/SpecificRecordTest.java b/.../tests/tests-allavro/src/test/java/com/linkedin/avroutil1/builder/SpecificRecordTest.java
@@ -1374,6 +1374,7 @@ private Object[][] testStringTypeParamsProvider() {
       put("arOfUnionOfStr", "java.util.List<java.lang.String>");
       put("arOfMapOfUnionOfArray", "java.util.List<java.util.Map<java.lang.String, java.util.List<java.lang.String>>>");
       put("intAr", "java.util.List<java.lang.Integer>");
+      put("unionOfIntMap", "java.util.Map<java.lang.String, java.lang.Integer>");
     }};
 
     Map<String, String> vs14TestCollectionsCharSeqFieldToType = new LinkedHashMap<String, String>() {{
@@ -1386,6 +1387,7 @@ private Object[][] testStringTypeParamsProvider() {
       put("arOfUnionOfStr", "java.util.List<java.lang.CharSequence>");
       put("arOfMapOfUnionOfArray", "java.util.List<java.util.Map<java.lang.CharSequence, java.util.List<java.lang.CharSequence>>>");
       put("intAr", "java.util.List<java.lang.Integer>");
+      put("unionOfIntMap", "java.util.Map<java.lang.CharSequence, java.lang.Integer>");
     }};
 
     return new Object[][]{
@@ -1493,6 +1495,11 @@ public void testRecordWithCharSeqStringTypeForMethods() throws Exception {
       put("key1", Arrays.asList("val1", "val2"));
       put("key2", Arrays.asList("val10", "val20"));
     }};
+
+    Map<CharSequence, Integer> mapOfInt = new HashMap<CharSequence, Integer>() {{
+      put("key1", 1);
+      put("key2", 2);
+    }};
     charseqmethod.TestCollections.Builder testCollectionsBuilder = charseqmethod.TestCollections.newBuilder()
         .setStr(str)
         .setStrAr(Arrays.asList(str))
@@ -1501,7 +1508,8 @@ public void testRecordWithCharSeqStringTypeForMethods() throws Exception {
         .setArOfMap(Arrays.asList(mapCharSeq))
         .setUnionOfMap(mapCharSeq)
         .setArOfUnionOfStr(Arrays.asList(str))
-        .setArOfMapOfUnionOfArray(Arrays.asList(mapOfList)).setIntAr(Arrays.asList(1, 2, 3));
+        .setArOfMapOfUnionOfArray(Arrays.asList(mapOfList)).setIntAr(Arrays.asList(1, 2, 3))
+        .setUnionOfIntMap(mapOfInt);
 
     charseqmethod.TestCollections testCollections = testCollectionsBuilder.build();
 
@@ -1840,7 +1848,8 @@ public void testNewBuilder() throws Exception {
         .setArOfMap(instance.getArOfMap())
         .setUnionOfMap(instance.getUnionOfMap())
         .setArOfUnionOfStr(instance.getArOfUnionOfStr())
-        .setArOfMapOfUnionOfArray(instance.getArOfMapOfUnionOfArray());
+        .setArOfMapOfUnionOfArray(instance.getArOfMapOfUnionOfArray())
+        .setUnionOfIntMap(instance.getUnionOfIntMap());
 
     TestCollections.newBuilder(builder);
 
@@ -1870,9 +1879,7 @@ public void modifiablePrimitiveCollectionTest() {
 
 
     // Union (null, Map<String, String>)
-    instance.getUnionOfMap().put("key1", tba);
-    Assert.assertEquals(tba, instance.getUnionOfMap().get("key1"));
-    Assert.assertEquals(new Utf8(tba), instance.unionOfMap.get(new Utf8("key1")));
+    Assert.assertThrows(UnsupportedOperationException.class, () -> instance.getUnionOfMap().put("key1", tba));
 
     instance.getIntAr().add(Integer.MAX_VALUE);
     Assert.assertEquals((int) instance.getIntAr().get(instance.getIntAr().size() - 1), Integer.MAX_VALUE);
@@ -1902,9 +1909,7 @@ public void modifiablePrimitiveCollectionTestForCharSeq() {
 
 
     // Union (null, Map<String, String>)
-    instance.getUnionOfMap().put("key1", tba);
-    Assert.assertEquals(tba, instance.getUnionOfMap().get("key1"));
-    Assert.assertEquals(new Utf8(tba), instance.unionOfMap.get(new Utf8("key1")));
+    Assert.assertThrows(UnsupportedOperationException.class, () -> instance.getUnionOfMap().put("key1", tba));
 
     instance.getIntAr().add(Integer.MAX_VALUE);
     Assert.assertEquals((int) instance.getIntAr().get(instance.getIntAr().size() - 1), Integer.MAX_VALUE);
@@ -1934,9 +1939,7 @@ public void testCharSeqAccessorForNoUtf8() {
 
 
     // Union (null, Map<String, String>)
-    instance.getUnionOfMap().put("key1", tba);
-    Assert.assertEquals(tba, instance.getUnionOfMap().get("key1"));
-    Assert.assertEquals(new Utf8(tba), instance.unionOfMap.get(new Utf8("key1")));
+    Assert.assertThrows(UnsupportedOperationException.class, () -> instance.getUnionOfMap().put("key1", tba));
 
     instance.getIntAr().add(Integer.MAX_VALUE);
     Assert.assertEquals((int) instance.getIntAr().get(instance.getIntAr().size() - 1), Integer.MAX_VALUE);

diff --git a/...t/java/com/linkedin/avroutil1/compatibility/collectiontransformer/CollectionViewTest.java b/...t/java/com/linkedin/avroutil1/compatibility/collectiontransformer/CollectionViewTest.java
@@ -7,9 +7,7 @@
 
 import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.HashMap;
 import java.util.List;
-import java.util.Map;
 import org.apache.avro.util.Utf8;
 import org.testng.Assert;
 import org.testng.annotations.Test;
@@ -126,108 +124,4 @@ public void testCharSequenceListView() {
       Assert.assertTrue(utf8List.contains(u));
     }
   }
-
-  @Test
-  public void testStringMapView() {
-    Map<Utf8, Utf8> utf8Map = new HashMap<>();
-    List<String> keys = Arrays.asList("key1", "key2", "key3");
-    String val = "value";
-
-    Map<String, String> map = CollectionTransformerUtil.createStringMapView(utf8Map);
-
-    // utf8 map is empty
-    Assert.assertEquals(utf8Map.size(), 0);
-    // view should be empty
-    Assert.assertEquals(map.size(), 0);
-
-    // insert in view
-    for (String key : keys) {
-      map.put(key, key + val);
-    }
-
-    // view should have 3 elements
-    for (String key : keys) {
-      Assert.assertTrue(map.containsKey(key));
-    }
-    // utf8 map should contain the same 3 elements
-    for (String key : keys) {
-      Assert.assertTrue(utf8Map.containsKey(new Utf8(key)));
-    }
-
-    // remove from map
-    for (String key : keys) {
-      map.remove(key);
-    }
-    // view should be empty
-    Assert.assertEquals(map.size(), 0);
-  }
-
-  @Test
-  public void testUtf8MapView() {
-    Map<Utf8, Utf8> utf8Map = new HashMap<>();
-    List<Utf8> keys = Arrays.asList(new Utf8("key1"), new Utf8("key2"), new Utf8("key3"));
-    Utf8 val = new Utf8("value");
-
-    Map<Utf8, Utf8> map = CollectionTransformerUtil.createUtf8MapView(utf8Map);
-    // utf8 map is empty
-    Assert.assertEquals(utf8Map.size(), 0);
-    // view should be empty
-    Assert.assertEquals(map.size(), 0);
-
-    // insert in view
-    for (Utf8 key : keys) {
-      map.put(key, val);
-    }
-
-    // view should have 3 elements
-    for (Utf8 key : keys) {
-      Assert.assertTrue(map.containsKey(key));
-    }
-    // utf8 map should contain the same 3 elements
-    for (Utf8 key : keys) {
-      Assert.assertTrue(utf8Map.containsKey(key));
-    }
-
-    // remove from view
-    for (Utf8 key : keys) {
-      map.remove(key);
-    }
-    // view should be empty
-    Assert.assertEquals(map.size(), 0);
-  }
-
-  @Test
-  public void testCharSequenceMapView() {
-    Map<Utf8, Utf8> utf8Map = new HashMap<>();
-    List<CharSequence> keys = Arrays.asList("key1", "key2", "key3");
-    String val = "value";
-
-    Map<CharSequence, CharSequence> map = CollectionTransformerUtil.createCharSequenceMapView(utf8Map);
-
-    // utf8 map is empty
-    Assert.assertEquals(utf8Map.size(), 0);
-    // view should be empty
-    Assert.assertEquals(map.size(), 0);
-
-    // insert in view
-    for (CharSequence key : keys) {
-      map.put(key, key + val);
-    }
-
-    // view should have 3 elements
-    for (CharSequence key : keys) {
-      Assert.assertTrue(map.containsKey(key));
-    }
-    // utf8 map should contain the same 3 elements
-    for (CharSequence key : keys) {
-      Assert.assertTrue(utf8Map.containsKey(new Utf8(String.valueOf(key))));
-    }
-
-    // remove from view
-    for (CharSequence key : keys) {
-      map.remove(key);
-    }
-    // view should be empty
-    Assert.assertEquals(map.size(), 0);
-  }
 }