Address comments from review

Author: felixbarny

server/src/main/java/org/elasticsearch/index/mapper/ESInetAddressPoint.java

@@ -15,7 +15,7 @@
 import org.apache.lucene.util.BytesRef;
 import org.elasticsearch.common.network.CIDRUtils;
 import org.elasticsearch.common.network.InetAddresses;
-import org.elasticsearch.core.SuppressForbidden;
+import org.elasticsearch.common.network.NetworkAddress;
 import org.elasticsearch.xcontent.XContentString;
 
 import java.net.InetAddress;
@@ -38,17 +38,46 @@ class ESInetAddressPoint extends Field {
     private final XContentString ipString;
     private final InetAddress inetAddress;
 
-    protected ESInetAddressPoint(String name, XContentString ipString) {
+    /**
+     * Creates a new ESInetAddressPoint, indexing the provided address.
+     * <p>
+     * This is the difference compared to {@link #ESInetAddressPoint(String, InetAddress)}
+     * and {@link InetAddressPoint#InetAddressPoint(String, InetAddress)}
+     * is that this constructor uses a more efficient way to parse the IP address that avoids the need to create
+     * a {@link String} and an {@link InetAddress} object for the IP address.
+     *
+     * @param name the name of the field
+     * @param value the IP address as a string
+     * @throws IllegalArgumentException if the field name or value is null or if the IP address is invalid
+     */
+    protected ESInetAddressPoint(String name, XContentString value) {
         super(name, TYPE);
-        this.fieldsData = new BytesRef(InetAddresses.encodeAsIpv6(ipString));
-        this.ipString = ipString;
+        if (value == null) {
+            throw new IllegalArgumentException("point must not be null");
+        }
+        this.fieldsData = new BytesRef(InetAddresses.encodeAsIpv6(value));
+        this.ipString = value;
         this.inetAddress = null;
     }
 
-    protected ESInetAddressPoint(String name, InetAddress inetAddress) {
+    /**
+     * Creates a new ESInetAddressPoint, indexing the provided address.
+     * <p>
+     * This constructor is similar to Lucene's InetAddressPoint.
+     * For performance reasons, it is recommended to use the constructor that accepts
+     * an {@link XContentString} representation of the IP address instead.
+     *
+     * @param name field name
+     * @param value InetAddress value
+     * @throws IllegalArgumentException if the field name or value is null.
+     */
+    protected ESInetAddressPoint(String name, InetAddress value) {
         super(name, TYPE);
-        this.fieldsData = new BytesRef(CIDRUtils.encode(inetAddress.getAddress()));
-        this.inetAddress = inetAddress;
+        if (value == null) {
+            throw new IllegalArgumentException("point must not be null");
+        }
+        this.fieldsData = new BytesRef(CIDRUtils.encode(value.getAddress()));
+        this.inetAddress = value;
         this.ipString = null;
     }
 
@@ -63,10 +92,6 @@ public InetAddress getInetAddress() {
     }
 
     @Override
-    @SuppressForbidden(
-        reason = "Calling InetAddress#getHostAddress to mimic what InetAddressPoint does. "
-            + "Some tests depend on the exact string representation."
-    )
     public String toString() {
         StringBuilder result = new StringBuilder();
         result.append(getClass().getSimpleName());
@@ -79,10 +104,10 @@ public String toString() {
         InetAddress address = InetAddressPoint.decode(BytesRef.deepCopyOf(bytes).bytes);
         if (address.getAddress().length == 16) {
             result.append('[');
-            result.append(address.getHostAddress());
+            result.append(NetworkAddress.format(address));
             result.append(']');
         } else {
-            result.append(address.getHostAddress());
+            result.append(NetworkAddress.format(address));
         }
 
         result.append('>');

server/src/test/java/org/elasticsearch/index/mapper/IpScriptMapperTests.java

@@ -75,16 +75,16 @@ protected IpFieldScript.Factory multipleValuesScript() {
     @Override
     protected void assertMultipleValues(List<IndexableField> fields) {
         assertEquals(4, fields.size());
-        assertEquals("ESInetAddressPoint <field:[0:0:0:0:0:0:0:1]>", fields.get(0).toString());
+        assertEquals("ESInetAddressPoint <field:[::1]>", fields.get(0).toString());
         assertEquals("docValuesType=SORTED_SET<field:[0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 1]>", fields.get(1).toString());
-        assertEquals("ESInetAddressPoint <field:[0:0:0:0:0:0:0:2]>", fields.get(2).toString());
+        assertEquals("ESInetAddressPoint <field:[::2]>", fields.get(2).toString());
         assertEquals("docValuesType=SORTED_SET<field:[0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 2]>", fields.get(3).toString());
     }
 
     @Override
     protected void assertDocValuesDisabled(List<IndexableField> fields) {
         assertEquals(1, fields.size());
-        assertEquals("ESInetAddressPoint <field:[0:0:0:0:0:0:0:1]>", fields.get(0).toString());
+        assertEquals("ESInetAddressPoint <field:[::1]>", fields.get(0).toString());
     }
 
     @Override