Handle caching expressions with different expression parameters

When calling .ToString() on expression, strip the expression parameter, arrow and parameter from the expression, e.g.

Expression<Func<Project, object>> expression = p => p.Name; would be p => p.Name in string form, but strip p => p. to leave just Name. We can generate the same hashcode for two expressions for the same type and expression that use different expression parameters.

Implement GetHashCode similarly in PropertyName as in Field
Implement _simple_ Equality in Field and PropertyName
Removed the superfluous cache buster tests and moved others onto FieldResolverCacheTests

Author: russcam

docs/asciidoc/query-dsl/specialized/script/script-query-usage.asciidoc

@@ -18,7 +18,6 @@ See the Elasticsearch documentation on {ref_current}/query-dsl-script-query.html
 q
 .Script(sn => sn
     .Name("named_query")
-    .Boost(1.1)
     .Inline(_templateString)
     .Params(p => p.Add("param1", 50))
 )
@@ -31,7 +30,6 @@ q
 new ScriptQuery
 {
     Name = "named_query",
-    Boost = 1.1,
     Inline = _templateString,
     Params = new Dictionary<string, object>
     {
@@ -46,7 +44,6 @@ new ScriptQuery
 {
   "script": {
     "_name": "named_query",
-    "boost": 1.1,
     "script": {
       "inline": "doc['numberOfCommits'].value > param1",
       "params": {

src/Nest/CommonAbstractions/Extensions/ExpressionExtensions.cs

@@ -1,5 +1,7 @@
 using System;
+using System.Linq;
 using System.Linq.Expressions;
+using System.Text.RegularExpressions;
 
 namespace Nest
 {
@@ -44,5 +46,26 @@ protected override Expression VisitUnary(UnaryExpression node)
 				return node;
 			}
 		}
+
+		private static readonly Regex ExpressionRegex = new Regex(@"^\s*(.*)\s*\=\>\s*\1\.");
+		private static readonly Regex MemberExpressionRegex = new Regex(@"^[^\.]*\.");
+
+		internal static object ComparisonValueFromExpression(this Expression expression, out Type type)
+		{
+			type = null;
+
+			if (expression == null) return null;
+
+			var lambda = expression as LambdaExpression;
+			if (lambda == null)
+				return ExpressionRegex.Replace(expression.ToString(), string.Empty);
+
+			type = lambda.Parameters.FirstOrDefault()?.Type;
+
+			var memberExpression = lambda.Body as MemberExpression;
+			return memberExpression != null
+				? MemberExpressionRegex.Replace(memberExpression.ToString(), string.Empty)
+				: ExpressionRegex.Replace(expression.ToString(), string.Empty);
+		}
 	}
-}
+}

src/Nest/CommonAbstractions/Infer/Field/Field.cs

@@ -13,8 +13,8 @@ public class Field : IEquatable<Field>, IUrlParameter
 		private string _name;
 		private Expression _expression;
 		private PropertyInfo _property;
-
-		private Type Type { get; set; }
+		private Type _type;
+		private object _comparisonValue;
 
 		public string Name
 		{
@@ -35,8 +35,8 @@ public Expression Expression
 			{
 				_expression = value;
 				Type type;
-				var comparisonValue = ComparisonValueFromExpression(value, out type);
-				Type = type;
+				var comparisonValue = value.ComparisonValueFromExpression(out type);
+				_type = type;
 				SetComparisonValue(comparisonValue);
 				CacheableExpression = !new HasConstantExpressionVisitor(value).Found;
 			}
@@ -49,14 +49,12 @@ public PropertyInfo Property
 			{
 				_property = value;
 				SetComparisonValue(value);
-				Type = value.DeclaringType;
+				_type = value.DeclaringType;
 			}
 		}
 
 		public double? Boost { get; set; }
 
-		private object ComparisonValue { get; set; }
-
 		public bool CacheableExpression { get; private set; }
 
 		public Fields And<T>(Expression<Func<T, object>> field) where T : class =>
@@ -77,6 +75,8 @@ public static Field Create(string name, double? boost = null)
 
 		public static Field Create(Expression expression, double? boost = null)
 		{
+			if (expression == null) return null;
+
 			Field field = expression;
 			field.Boost = boost;
 			return field;
@@ -96,22 +96,6 @@ private static string ParseFieldName(string name, out double? boost)
 			return name;
 		}
 
-		private static object ComparisonValueFromExpression(Expression expression, out Type type)
-		{
-			type = null;
-
-			if (expression == null) return null;
-
-			var lambda = expression as LambdaExpression;
-			if (lambda == null)
-				return expression.ToString();
-
-			type = lambda.Parameters.FirstOrDefault()?.Type;
-
-			var memberExpression = lambda.Body as MemberExpression;
-			return memberExpression?.ToString() ?? expression.ToString();
-		}
-
 		public static implicit operator Field(string name)
 		{
 			return name.IsNullOrEmpty() ? null : new Field
@@ -138,19 +122,37 @@ public static implicit operator Field(PropertyInfo property)
 
 		public override int GetHashCode()
 		{
-			var hashCode = ComparisonValue?.GetHashCode() ?? 0;
-			hashCode = (hashCode * 397) ^ (Type?.GetHashCode() ?? 0);
-			return hashCode;
+			unchecked
+			{
+				var hashCode = _comparisonValue?.GetHashCode() ?? 0;
+				hashCode = (hashCode * 397) ^ (_type?.GetHashCode() ?? 0);
+				return hashCode;
+			}
 		}
 
-		bool IEquatable<Field>.Equals(Field other) => Equals(other);
+		bool IEquatable<Field>.Equals(Field other)
+		{
+			return _type != null
+				? other != null && _type == other._type && _comparisonValue.Equals(other._comparisonValue)
+				: other != null && _comparisonValue.Equals(other._comparisonValue);
+		}
 
 		public override bool Equals(object obj)
 		{
-			var other = obj as Field;
-			if (other == null)
-				return false;
-			return ComparisonValue.Equals(other.ComparisonValue);
+			if (ReferenceEquals(null, obj)) return false;
+			if (ReferenceEquals(this, obj)) return true;
+			if (obj.GetType() != GetType()) return false;
+			return ((IEquatable<Field>)this).Equals(obj as Field);
+		}
+
+		public static bool operator ==(Field x, Field y)
+		{
+			return Equals(x, y);
+		}
+
+		public static bool operator !=(Field x, Field y)
+		{
+			return !(x == y);
 		}
 
 		string IUrlParameter.GetString(IConnectionConfigurationValues settings)
@@ -164,10 +166,10 @@ string IUrlParameter.GetString(IConnectionConfigurationValues settings)
 
 		private void SetComparisonValue(object value)
 		{
-			if (ComparisonValue != null && value != null)
-				throw new InvalidOperationException($"{nameof(ComparisonValue)} already has a value");
+			if (_comparisonValue != null && value != null)
+				throw new InvalidOperationException($"{nameof(_comparisonValue)} already has a value");
 
-			ComparisonValue = value;
+			_comparisonValue = value;
 		}
 	}
 }

src/Nest/CommonAbstractions/Infer/Field/FieldExpressionVisitor.cs

@@ -64,9 +64,9 @@ protected override Expression VisitConstant(ConstantExpression node)
 
 	internal class FieldExpressionVisitor : ExpressionVisitor
     {
-		private Stack<string> _stack = new Stack<string>();
+		private readonly Stack<string> _stack = new Stack<string>();
 
-		private IConnectionSettingsValues _settings;
+		private readonly IConnectionSettingsValues _settings;
 
 		public FieldExpressionVisitor(IConnectionSettingsValues settings)
 		{

src/Nest/CommonAbstractions/Infer/Field/FieldResolver.cs

@@ -42,24 +42,29 @@ private string ResolveFieldName(Field field)
 				return this.Resolve(field.Expression, field.Property);
 			}
 
-			string f;
-			if (this.Fields.TryGetValue(field, out f))
-				return f;
+			string fieldName;
+			if (this.Fields.TryGetValue(field, out fieldName))
+				return fieldName;
 
-			f = this.Resolve(field.Expression, field.Property);
-			this.Fields.TryAdd(field, f);
-			return f;
+			fieldName = this.Resolve(field.Expression, field.Property);
+			this.Fields.TryAdd(field, fieldName);
+			return fieldName;
 		}
 
 		public string Resolve(PropertyName property)
 		{
 			if (property.IsConditionless()) return null;
-			if (!property.Name.IsNullOrEmpty())
-				return property.Name;
+			if (!property.Name.IsNullOrEmpty()) return property.Name;
+
+			if (property.Expression != null && !property.CacheableExpression)
+			{
+				return this.Resolve(property.Expression, property.Property);
+			}
 
 			string propertyName;
 			if (this.Properties.TryGetValue(property, out propertyName))
 				return propertyName;
+
 			propertyName = this.Resolve(property.Expression, property.Property, true);
 			this.Properties.TryAdd(property, propertyName);
 			return propertyName;

src/Nest/CommonAbstractions/Infer/PropertyName/PropertyName.cs

@@ -1,34 +1,43 @@
 using System;
+using System.Linq;
 using System.Linq.Expressions;
 using System.Reflection;
+using System.Text.RegularExpressions;
 using Elasticsearch.Net;
 
 namespace Nest
 {
 	[ContractJsonConverter(typeof(PropertyNameJsonConverter))]
 	public class PropertyName : IEquatable<PropertyName>, IUrlParameter
 	{
+		private object _comparisonValue;
+		private Type _type;
+
 		public string Name { get; set; }
 		public Expression Expression { get; set; }
 		public PropertyInfo Property { get; set; }
-
-		private object ComparisonValue;
+		public bool CacheableExpression { get; private set; }
 
 		public static implicit operator PropertyName(string name)
 		{
 			return name == null ? null : new PropertyName
 			{
 				Name = name,
-				ComparisonValue = name
+				_comparisonValue = name
 			};
 		}
 
 		public static implicit operator PropertyName(Expression expression)
 		{
-			return expression == null ? null : new PropertyName
+			if (expression == null) return null;
+
+			Type type;
+			return new PropertyName
 			{
 				Expression = expression,
-				ComparisonValue = ((expression as LambdaExpression)?.Body as MemberExpression)?.Member.Name ?? expression.ToString()
+				CacheableExpression = !new HasConstantExpressionVisitor(expression).Found,
+				_comparisonValue = expression.ComparisonValueFromExpression(out type),
+				_type = type
 			};
 		}
 
@@ -37,34 +46,50 @@ public static implicit operator PropertyName(PropertyInfo property)
 			return property == null ? null : new PropertyName
 			{
 				Property = property,
-				ComparisonValue = property
+				_comparisonValue = property
 			};
 		}
 
 		public override int GetHashCode()
 		{
-			return ComparisonValue?.GetHashCode() ?? 0;	
+			unchecked
+			{
+				var hashCode = _comparisonValue?.GetHashCode() ?? 0;
+				hashCode = (hashCode * 397) ^ (_type?.GetHashCode() ?? 0);
+				return hashCode;
+			}
 		}
 
 		bool IEquatable<PropertyName>.Equals(PropertyName other)
 		{
-			return Equals(other);
+			return _type != null
+				? other != null && _type == other._type && _comparisonValue.Equals(other._comparisonValue)
+				: other != null && _comparisonValue.Equals(other._comparisonValue);
 		}
 
 		public override bool Equals(object obj)
 		{
-			var other = obj as PropertyName;
-			if (other == null)
-				return false;
+			if (ReferenceEquals(null, obj)) return false;
+			if (ReferenceEquals(this, obj)) return true;
+			if (obj.GetType() != GetType()) return false;
+			return ((IEquatable<PropertyName>)this).Equals(obj as PropertyName);
+		}
 
-			return ComparisonValue.Equals(other.ComparisonValue);
+		public static bool operator ==(PropertyName x, PropertyName y)
+		{
+			return Equals(x, y);
+		}
+
+		public static bool operator !=(PropertyName x, PropertyName y)
+		{
+			return !(x == y);
 		}
 
 		string IUrlParameter.GetString(IConnectionConfigurationValues settings)
 		{
 			var nestSettings = settings as IConnectionSettingsValues;
 			if (nestSettings == null)
-				throw new Exception("Tried to pass field name on querysting but it could not be resolved because no nest settings are available");
+				throw new Exception("Tried to pass field name on querystring but it could not be resolved because no nest settings are available");
 			var infer = new Inferrer(nestSettings);
 			return infer.PropertyName(this);
 		}

src/Nest/QueryDsl/Query.cs

@@ -161,8 +161,11 @@ public static QueryContainer SpanTerm(Func<SpanTermQueryDescriptor<T>, ISpanTerm
 		public static QueryContainer SpanWithin(Func<SpanWithinQueryDescriptor<T>, ISpanWithinQuery> selector) =>
 			new QueryContainerDescriptor<T>().SpanWithin(selector);
 
+#pragma warning disable 618
+		[Obsolete("Scheduled to be removed in 5.0.  Setting Strict() at the container level does is a noop and must be set on each individual query.")]
 		public static QueryContainerDescriptor<T> Strict(bool strict = true) =>
 			new QueryContainerDescriptor<T>().Strict(strict);
+#pragma warning restore 618
 
 		public static QueryContainer Template(Func<TemplateQueryDescriptor<T>, ITemplateQuery> selector) =>
 			new QueryContainerDescriptor<T>().Template(selector);