Simplify collection setters

Author: swallez

java-client/src/main/java/co/elastic/clients/util/ApiTypeHelper.java

@@ -22,7 +22,6 @@
 import javax.annotation.Nullable;
 import java.util.AbstractList;
 import java.util.AbstractMap;
-import java.util.Collection;
 import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
@@ -100,7 +99,6 @@ public Iterator<Object> iterator() {
     };
 
     static final List<Object> UNDEFINED_LIST = new EmptyList();
-    static final List<Object> RESET_LIST = new EmptyList();
 
     /**
      * Returns an empty list that is undefined from a JSON perspective. It will not be serialized
@@ -111,19 +109,6 @@ public static <T> List<T> undefinedList() {
         return (List<T>)UNDEFINED_LIST;
     }
 
-    /**
-     * Returns a "magic list" that will reset list properties of API types to {@code null}.
-     * <p>
-     * List setters on API type builders add items to the property using {@link List#addAll(Collection)} and do not accept
-     * a {@code null} value which is considered as a bug. In some rare occasions it may however be necessary to reset
-     * the builder's property, and this is what the result of this method is meant for: setting a builder's list property
-     * to the value returned by this method will reset that property to {@code null}.
-     */
-    @SuppressWarnings("unchecked")
-    public static <T> List<T> resetList() {
-        return (List<T>)RESET_LIST;
-    }
-
     /**
      * Is {@code list} defined according to JSON/JavaScript semantics?
      *
@@ -167,7 +152,6 @@ public Set<Entry<Object, Object>> entrySet() {
     }
 
     static final Map<Object, Object> UNDEFINED_MAP = new EmptyMap();
-    static final Map<Object, Object> RESET_MAP = new EmptyMap();
 
     /**
      * Returns an empty list that is undefined from a JSON perspective. It will not be serialized
@@ -178,19 +162,6 @@ public static <K, V> Map<K, V> undefinedMap() {
         return (Map<K, V>)UNDEFINED_MAP;
     }
 
-    /**
-     * Returns a "magic map" that will reset map properties of API types to {@code null}.
-     * <p>
-     * Map setters on API type builders add entries to the property using {@link Map#putAll(Map)} and do not accept
-     * a {@code null} value which is considered as a bug. In some rare occasions it may however be necessary to reset
-     * the builder's property, and this is what the result of this method is meant for: setting a builder's map property
-     * to the value returned by this method will reset that property to {@code null}.
-     */
-    @SuppressWarnings("unchecked")
-    public static <K, V> Map<K, V> resetMap() {
-        return (Map<K, V>)RESET_MAP;
-    }
-
     /**
      * Is {@code map} defined according to JSON/JavaScript semantics?
      *

java-client/src/main/java/co/elastic/clients/util/ObjectBuilderBase.java

@@ -25,6 +25,7 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 
 public class ObjectBuilderBase {
     private boolean _used = false;
@@ -76,9 +77,7 @@ protected static <T> List<T> _listAddAll(List<T> list, List<T> values) {
         if (list == null) {
             // Keep the original list to avoid an unnecessary copy.
             // It will be copied if we add more values.
-            return values;
-        } else if (values == ApiTypeHelper.RESET_LIST) {
-            return null;
+            return Objects.requireNonNull(values);
         } else {
             list = _mutableList(list);
             list.addAll(values);
@@ -119,11 +118,14 @@ protected static <K, V> Map<K, V> _mapPut(Map<K, V> map, K key, V value) {
 
     /** Add all elements of a list to a (possibly {@code null}) map */
     protected static <K, V> Map<K, V> _mapPutAll(Map<K, V> map, Map<K, V> entries) {
-        if (entries == ApiTypeHelper.RESET_MAP) {
-            return null;
+        if (map == null) {
+            // Keep the original map to avoid an unnecessary copy.
+            // It will be copied if we add more entries.
+            return Objects.requireNonNull(entries);
+        } else {
+            map = _mutableMap(map);
+            map.putAll(entries);
+            return map;
         }
-        map = _mutableMap(map);
-        map.putAll(entries);
-        return map;
     }
 }

java-client/src/test/java/co/elastic/clients/elasticsearch/model/ClassStructureTest.java

@@ -214,19 +214,13 @@ public void testListSetters() {
 
 
         {
-            // Reset builder property
-            SearchRequest search = SearchRequest.of(b -> b
-                .storedFields(fields)
-                .storedFields(ApiTypeHelper.resetList())
-            );
-            assertFalse(ApiTypeHelper.isDefined(search.storedFields()));
-
-            search = SearchRequest.of(b -> b
-                .storedFields(fields)
-                .storedFields(ApiTypeHelper.resetList())
-                .storedFields("d", "e")
-            );
-            assertEquals(Arrays.asList("d", "e"), search.storedFields());
+            // List cannot be null
+            List<String> nullFields = null;
+            assertThrows(NullPointerException.class, () -> {
+                SearchRequest.of(b -> b
+                    .storedFields(nullFields)
+                );
+            });
         }
 
         {
@@ -255,7 +249,6 @@ public void testMapSetters() {
 
         {
             // Appending doesn't modify the original collection
-
             SearchRequest search = SearchRequest.of(b -> b
                 .aggregations(aggs)
                 .aggregations("aggC", countC._toAggregation())
@@ -273,20 +266,11 @@ public void testMapSetters() {
         }
 
         {
-            // Reset builder property
-            SearchRequest search = SearchRequest.of(b -> b
-                .aggregations(aggs)
-                .aggregations(ApiTypeHelper.resetMap())
-            );
-            assertFalse(ApiTypeHelper.isDefined(search.storedFields()));
-
-            search = SearchRequest.of(b -> b
-                .aggregations(aggs)
-                .aggregations(ApiTypeHelper.resetMap())
-                .aggregations("aggC", countC._toAggregation())
-            );
-            assertEquals(1, search.aggregations().size());
-            assertEquals("c", search.aggregations().get("aggC").valueCount().field());
+            // Map cannot be null
+            assertThrows(NullPointerException.class, () -> {
+                Map<String, Aggregation> nullMap = null;
+                SearchRequest.of(b -> b.aggregations(nullMap));
+            });
         }
     }