Painless: Cleanup Cache (#33963)

* Adds equals/hashcode methods to the Painless* objects within the lookup package. * Changes the caches to use the Painless* objects as keys as well as values. This forces future changes to taken into account the appropriate values for caching. * Deletes the existing caching objects in favor of Painless* objects. This removes a pair of bugs that were not taking into account subtle corner cases related to augmented methods and caching. * Uses the Painless* objects to check for equivalency in existing Painless* objects that may have already been added to a PainlessClass. This removes a bug related to return not being appropriately checked when adding methods. * Cleans up several error messages.
elastic · Oct 30, 2018 · 8d99925 · 8d99925
1 parent 6ff6875
commit 8d99925
Show file tree

Hide file tree

Showing 8 changed files with 336 additions and 322 deletions.
diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessCast.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessCast.java
@@ -19,38 +19,59 @@
 
 package org.elasticsearch.painless.lookup;
 
+import java.util.Objects;
+
 public class PainlessCast {
 
     /** Create a standard cast with no boxing/unboxing. */
     public static PainlessCast originalTypetoTargetType(Class<?> originalType, Class<?> targetType, boolean explicitCast) {
+        Objects.requireNonNull(originalType);
+        Objects.requireNonNull(targetType);
+
         return new PainlessCast(originalType, targetType, explicitCast, null, null, null, null);
     }
 
     /** Create a cast where the original type will be unboxed, and then the cast will be performed. */
     public static PainlessCast unboxOriginalType(
             Class<?> originalType, Class<?> targetType, boolean explicitCast, Class<?> unboxOriginalType) {
 
+        Objects.requireNonNull(originalType);
+        Objects.requireNonNull(targetType);
+        Objects.requireNonNull(unboxOriginalType);
+
         return new PainlessCast(originalType, targetType, explicitCast, unboxOriginalType, null, null, null);
     }
 
     /** Create a cast where the target type will be unboxed, and then the cast will be performed. */
     public static PainlessCast unboxTargetType(
             Class<?> originalType, Class<?> targetType, boolean explicitCast, Class<?> unboxTargetType) {
 
+        Objects.requireNonNull(originalType);
+        Objects.requireNonNull(targetType);
+        Objects.requireNonNull(unboxTargetType);
+
         return new PainlessCast(originalType, targetType, explicitCast, null, unboxTargetType, null, null);
     }
 
     /** Create a cast where the original type will be boxed, and then the cast will be performed. */
     public static PainlessCast boxOriginalType(
             Class<?> originalType, Class<?> targetType, boolean explicitCast, Class<?> boxOriginalType) {
 
+        Objects.requireNonNull(originalType);
+        Objects.requireNonNull(targetType);
+        Objects.requireNonNull(boxOriginalType);
+
         return new PainlessCast(originalType, targetType, explicitCast, null, null, boxOriginalType, null);
     }
 
     /** Create a cast where the target type will be boxed, and then the cast will be performed. */
     public static PainlessCast boxTargetType(
             Class<?> originalType, Class<?> targetType, boolean explicitCast, Class<?> boxTargetType) {
 
+        Objects.requireNonNull(originalType);
+        Objects.requireNonNull(targetType);
+        Objects.requireNonNull(boxTargetType);
+
         return new PainlessCast(originalType, targetType, explicitCast, null, null, null, boxTargetType);
     }
 
@@ -73,4 +94,30 @@ private PainlessCast(Class<?> originalType, Class<?> targetType, boolean explici
         this.boxOriginalType = boxOriginalType;
         this.boxTargetType = boxTargetType;
     }
+
+    @Override
+    public boolean equals(Object object) {
+        if (this == object) {
+            return true;
+        }
+
+        if (object == null || getClass() != object.getClass()) {
+            return false;
+        }
+
+        PainlessCast that = (PainlessCast)object;
+
+        return explicitCast == that.explicitCast &&
+                Objects.equals(originalType, that.originalType) &&
+                Objects.equals(targetType, that.targetType) &&
+                Objects.equals(unboxOriginalType, that.unboxOriginalType) &&
+                Objects.equals(unboxTargetType, that.unboxTargetType) &&
+                Objects.equals(boxOriginalType, that.boxOriginalType) &&
+                Objects.equals(boxTargetType, that.boxTargetType);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(originalType, targetType, explicitCast, unboxOriginalType, unboxTargetType, boxOriginalType, boxTargetType);
+    }
 }
diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessClass.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessClass.java
@@ -22,6 +22,7 @@
 import java.lang.invoke.MethodHandle;
 import java.util.Collections;
 import java.util.Map;
+import java.util.Objects;
 
 public final class PainlessClass {
 
@@ -57,4 +58,29 @@ public final class PainlessClass {
 
         this.functionalInterfaceMethod = functionalInterfaceMethod;
     }
+
+    @Override
+    public boolean equals(Object object) {
+        if (this == object) {
+            return true;
+        }
+
+        if (object == null || getClass() != object.getClass()) {
+            return false;
+        }
+
+        PainlessClass that = (PainlessClass)object;
+
+        return Objects.equals(constructors, that.constructors) &&
+                Objects.equals(staticMethods, that.staticMethods) &&
+                Objects.equals(methods, that.methods) &&
+                Objects.equals(staticFields, that.staticFields) &&
+                Objects.equals(fields, that.fields) &&
+                Objects.equals(functionalInterfaceMethod, that.functionalInterfaceMethod);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(constructors, staticMethods, methods, staticFields, fields, functionalInterfaceMethod);
+    }
 }
diff --git a/...s/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessClassBinding.java b/...s/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessClassBinding.java
@@ -22,6 +22,7 @@
 import java.lang.reflect.Constructor;
 import java.lang.reflect.Method;
 import java.util.List;
+import java.util.Objects;
 
 public class PainlessClassBinding {
 
@@ -38,4 +39,28 @@ public class PainlessClassBinding {
         this.returnType = returnType;
         this.typeParameters = typeParameters;
     }
+
+    @Override
+    public boolean equals(Object object) {
+        if (this == object) {
+            return true;
+        }
+
+        if (object == null || getClass() != object.getClass()) {
+            return false;
+        }
+
+        PainlessClassBinding that = (PainlessClassBinding)object;
+
+        return Objects.equals(javaConstructor, that.javaConstructor) &&
+                Objects.equals(javaMethod, that.javaMethod) &&
+                Objects.equals(returnType, that.returnType) &&
+                Objects.equals(typeParameters, that.typeParameters);
+    }
+
+    @Override
+    public int hashCode() {
+
+        return Objects.hash(javaConstructor, javaMethod, returnType, typeParameters);
+    }
 }
diff --git a/...s/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessClassBuilder.java b/...s/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessClassBuilder.java
@@ -22,6 +22,7 @@
 import java.lang.invoke.MethodHandle;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.Objects;
 
 final class PainlessClassBuilder {
 
@@ -57,4 +58,29 @@ PainlessClass build() {
         return new PainlessClass(constructors, staticMethods, methods, staticFields, fields,
                 getterMethodHandles, setterMethodHandles, functionalInterfaceMethod);
     }
+
+    @Override
+    public boolean equals(Object object) {
+        if (this == object) {
+            return true;
+        }
+
+        if (object == null || getClass() != object.getClass()) {
+            return false;
+        }
+
+        PainlessClassBuilder that = (PainlessClassBuilder)object;
+
+        return Objects.equals(constructors, that.constructors) &&
+                Objects.equals(staticMethods, that.staticMethods) &&
+                Objects.equals(methods, that.methods) &&
+                Objects.equals(staticFields, that.staticFields) &&
+                Objects.equals(fields, that.fields) &&
+                Objects.equals(functionalInterfaceMethod, that.functionalInterfaceMethod);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(constructors, staticMethods, methods, staticFields, fields, functionalInterfaceMethod);
+    }
 }
diff --git a/...es/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessConstructor.java b/...es/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessConstructor.java
@@ -23,6 +23,7 @@
 import java.lang.invoke.MethodType;
 import java.lang.reflect.Constructor;
 import java.util.List;
+import java.util.Objects;
 
 public class PainlessConstructor {
 
@@ -37,4 +38,26 @@ public class PainlessConstructor {
         this.methodHandle = methodHandle;
         this.methodType = methodType;
     }
+
+    @Override
+    public boolean equals(Object object) {
+        if (this == object) {
+            return true;
+        }
+
+        if (object == null || getClass() != object.getClass()) {
+            return false;
+        }
+
+        PainlessConstructor that = (PainlessConstructor)object;
+
+        return Objects.equals(javaConstructor, that.javaConstructor) &&
+                Objects.equals(typeParameters, that.typeParameters) &&
+                Objects.equals(methodType, that.methodType);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(javaConstructor, typeParameters, methodType);
+    }
 }
diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessField.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessField.java
@@ -21,6 +21,7 @@
 
 import java.lang.invoke.MethodHandle;
 import java.lang.reflect.Field;
+import java.util.Objects;
 
 public final class PainlessField {
 
@@ -37,4 +38,25 @@ public final class PainlessField {
         this.getterMethodHandle = getterMethodHandle;
         this.setterMethodHandle = setterMethodHandle;
     }
+
+    @Override
+    public boolean equals(Object object) {
+        if (this == object) {
+            return true;
+        }
+
+        if (object == null || getClass() != object.getClass()) {
+            return false;
+        }
+
+        PainlessField that = (PainlessField)object;
+
+        return Objects.equals(javaField, that.javaField) &&
+                Objects.equals(typeParameter, that.typeParameter);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(javaField, typeParameter);
+    }
 }