Script: Metadata validateMetadata optimization

docs/changelog/93333.yaml

@@ -0,0 +1,5 @@
+pr: 93333
+summary: "Script: Metadata `validateMetadata` optimization"
+area: Infra/Scripting
+type: enhancement
+issues: []

server/src/main/java/org/elasticsearch/script/Metadata.java

@@ -55,26 +55,50 @@ public class Metadata {
     public static FieldProperty<Number> LongField = new FieldProperty<>(Number.class).withValidation(FieldProperty.LONGABLE_NUMBER);
 
     protected final Map<String, Object> map;
-    protected final Map<String, FieldProperty<?>> properties;
+    private final Map<String, FieldProperty<?>> properties;
     protected static final FieldProperty<?> BAD_KEY = new FieldProperty<>(null, false, false, null);
 
-    public Metadata(Map<String, Object> map, Map<String, FieldProperty<?>> properties) {
+    /**
+     * Constructs a new Metadata object represented by the given map and properties.
+     * <p>
+     * The passed-in map is used directly -- subsequent modifications to it outside the methods of this class may result in
+     * undefined behavior.
+     * <p>
+     * The properties map is used directly as well, but we verify at runtime that it <b>must</b> be an immutable map (i.e. constructed
+     * via a call to {@link Map#of()} (or similar) in production, or via {@link Map#copyOf(Map)}} in tests). Since it must be an
+     * immutable map, subsequent modifications are not possible.
+     *
+     * @param map the backing map for this metadata instance
+     * @param properties the immutable map of defined properties for the type of metadata represented by this instance
+     */
+    protected Metadata(Map<String, Object> map, Map<String, FieldProperty<?>> properties) {
         this.map = map;
-        this.properties = Collections.unmodifiableMap(properties);
+        // we can't tell the compiler that properties must be a java.util.ImmutableCollections.AbstractImmutableMap, but
+        // we can use this copyOf + assert to verify that at runtime.
+        this.properties = Map.copyOf(properties);
+        assert this.properties == properties : "properties map must be constructed via Map.of(...) or Map.copyOf(...)";
         validateMetadata();
     }
 
+    // a 'not found' sentinel value for use in validateMetadata below
+    private static final Object NOT_FOUND = new Object();
+
     /**
      * Check that all metadata map contains only valid metadata and no extraneous keys
      */
     protected void validateMetadata() {
         int numMetadata = 0;
         for (Map.Entry<String, FieldProperty<?>> entry : properties.entrySet()) {
             String key = entry.getKey();
-            if (map.containsKey(key)) {
+            Object value = map.getOrDefault(key, NOT_FOUND);
+            if (value != NOT_FOUND) {
                 numMetadata++;
+                // check whether it's permissible to have the value for the property
+                entry.getValue().check(MapOperation.INIT, key, value);
+            } else {
+                // check whether it's permissible to *not* have a value for the property
+                entry.getValue().check(MapOperation.INIT, key, null);
             }
-            entry.getValue().check(MapOperation.INIT, key, map.get(key));
         }
         if (numMetadata < map.size()) {
             Set<String> keys = new HashSet<>(map.keySet());
@@ -239,7 +263,7 @@ public int size() {
 
     @Override
     public Metadata clone() {
-        // properties is an UnmodifiableMap, no need to create a copy
+        // properties is an unmodifiable map, no need to create a copy here
         return new Metadata(new HashMap<>(map), properties);
     }
 

server/src/test/java/org/elasticsearch/script/CtxMapTests.java

@@ -25,7 +25,7 @@ public class CtxMapTests extends ESTestCase {
     @Before
     public void setUp() throws Exception {
         super.setUp();
-        map = new CtxMap<>(new HashMap<>(), new Metadata(new HashMap<>(), new HashMap<>()));
+        map = new CtxMap<>(new HashMap<>(), new Metadata(Map.of(), Map.of()));
     }
 
     public void testAddingJunkToCtx() {

server/src/test/java/org/elasticsearch/script/MetadataTests.java

@@ -58,7 +58,7 @@ private static Map<String, Metadata.FieldProperty<?>> allowAllValidators(String.
         for (String key : keys) {
             validators.put(key, Metadata.FieldProperty.ALLOW_ALL);
         }
-        return validators;
+        return Map.copyOf(validators);
     }
 
     public void testValidateMetadata() {

test/framework/src/main/java/org/elasticsearch/ingest/TestIngestCtxMetadata.java

@@ -15,7 +15,7 @@
 
 public class TestIngestCtxMetadata extends IngestDocMetadata {
     public TestIngestCtxMetadata(Map<String, Object> map, Map<String, FieldProperty<?>> properties) {
-        super(map, properties, null);
+        super(map, Map.copyOf(properties), null);
     }
 
     public static TestIngestCtxMetadata withNullableVersion(Map<String, Object> map) {