Skip to content

Commit a5d3398

Browse files
gmarzgmarz
committed
Port of FieldResolver memory leak fix from master
Closes #2048
1 parent 7c92c5d commit a5d3398

File tree

7 files changed

+439
-24
lines changed

7 files changed

+439
-24
lines changed

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

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
using System.Linq.Expressions;
44
using System.Reflection;
55
using Elasticsearch.Net;
6+
using System.Linq;
67

78
namespace Nest
89
{
@@ -13,6 +14,8 @@ public class Field : IEquatable<Field>, IUrlParameter
1314
private Expression _expression;
1415
private PropertyInfo _property;
1516

17+
private Type Type { get; set; }
18+
1619
public string Name
1720
{
1821
get { return _name; }
@@ -31,8 +34,11 @@ public Expression Expression
3134
set
3235
{
3336
_expression = value;
34-
var comparisonValue = ComparisonValueFromExpression(value);
37+
Type type;
38+
var comparisonValue = ComparisonValueFromExpression(value, out type);
39+
Type = type;
3540
SetComparisonValue(comparisonValue);
41+
CacheableExpression = !new HasConstantExpressionVisitor(value).Found;
3642
}
3743
}
3844

@@ -43,13 +49,16 @@ public PropertyInfo Property
4349
{
4450
_property = value;
4551
SetComparisonValue(value);
52+
Type = value.DeclaringType;
4653
}
4754
}
4855

4956
public double? Boost { get; set; }
5057

5158
private object ComparisonValue { get; set; }
5259

60+
public bool CacheableExpression { get; private set; }
61+
5362
public Fields And<T>(Expression<Func<T, object>> field) where T : class =>
5463
new Fields(new [] { this, field });
5564

@@ -82,31 +91,32 @@ private static string ParseFieldName(string name, out double? boost)
8291
if (parts.Length > 1)
8392
{
8493
name = parts[0];
85-
boost = Double.Parse(parts[1], CultureInfo.InvariantCulture);
94+
boost = double.Parse(parts[1], CultureInfo.InvariantCulture);
8695
}
8796
return name;
8897
}
8998

90-
private static object ComparisonValueFromExpression(Expression expression)
99+
private static object ComparisonValueFromExpression(Expression expression, out Type type)
91100
{
101+
type = null;
102+
92103
if (expression == null) return null;
93104

94105
var lambda = expression as LambdaExpression;
95106
if (lambda == null)
96107
return expression.ToString();
97108

98-
var memberExpression = lambda.Body as MemberExpression;
99-
if (memberExpression == null)
100-
return expression.ToString();
109+
type = lambda.Parameters.FirstOrDefault()?.Type;
101110

102-
return memberExpression;
111+
var memberExpression = lambda.Body as MemberExpression;
112+
return memberExpression?.ToString() ?? expression.ToString();
103113
}
104114

105115
public static implicit operator Field(string name)
106116
{
107117
return name.IsNullOrEmpty() ? null : new Field
108118
{
109-
Name = name
119+
Name = name,
110120
};
111121
}
112122

@@ -126,7 +136,12 @@ public static implicit operator Field(PropertyInfo property)
126136
};
127137
}
128138

129-
public override int GetHashCode() => ComparisonValue?.GetHashCode() ?? 0;
139+
public override int GetHashCode()
140+
{
141+
var hashCode = ComparisonValue?.GetHashCode() ?? 0;
142+
hashCode = (hashCode * 397) ^ (Type?.GetHashCode() ?? 0);
143+
return hashCode;
144+
}
130145

131146
bool IEquatable<Field>.Equals(Field other) => Equals(other);
132147

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

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,57 @@
1212

1313
namespace Nest
1414
{
15-
internal class FieldExpressionVisitor : ExpressionVisitor
15+
internal class HasConstantExpressionVisitor : ExpressionVisitor
16+
{
17+
public bool Found { get; private set; }
18+
19+
public HasConstantExpressionVisitor(Expression e)
20+
{
21+
this.Visit(e);
22+
}
23+
24+
public override Expression Visit(Expression node)
25+
{
26+
if (!Found)
27+
return base.Visit(node);
28+
return node;
29+
}
30+
31+
protected override Expression VisitMethodCall(MethodCallExpression node)
32+
{
33+
if (node.Method.Name == "Suffix" && node.Arguments.Any())
34+
{
35+
var lastArg = node.Arguments.Last();
36+
var constantExpression = lastArg as ConstantExpression;
37+
this.Found = constantExpression == null;
38+
return node;
39+
}
40+
else if (node.Method.Name == "get_Item" && node.Arguments.Any())
41+
{
42+
var t = node.Object.Type;
43+
var isDict =
44+
typeof(IDictionary).IsAssignableFrom(t)
45+
|| typeof(IDictionary<,>).IsAssignableFrom(t)
46+
|| (t.IsGeneric() && t.GetGenericTypeDefinition() == typeof(IDictionary<,>));
47+
48+
if (!isDict)
49+
return base.VisitMethodCall(node);
50+
var lastArg = node.Arguments.Last();
51+
var constantExpression = lastArg as ConstantExpression;
52+
this.Found = constantExpression == null;
53+
return node;
54+
}
55+
return base.VisitMethodCall(node);
56+
}
57+
58+
protected override Expression VisitConstant(ConstantExpression node)
59+
{
60+
this.Found = true;
61+
return node;
62+
}
63+
}
64+
65+
internal class FieldExpressionVisitor : ExpressionVisitor
1666
{
1767
private Stack<string> _stack = new Stack<string>();
1868

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@ public class FieldResolver
1717
{
1818
private readonly IConnectionSettingsValues _settings;
1919

20-
private readonly ConcurrentDictionary<Field, string> Fields = new ConcurrentDictionary<Field, string>();
21-
private readonly ConcurrentDictionary<PropertyName, string> Properties = new ConcurrentDictionary<PropertyName, string>();
20+
protected readonly ConcurrentDictionary<Field, string> Fields = new ConcurrentDictionary<Field, string>();
21+
protected readonly ConcurrentDictionary<PropertyName, string> Properties = new ConcurrentDictionary<PropertyName, string>();
2222

2323
public FieldResolver(IConnectionSettingsValues settings)
2424
{
@@ -37,6 +37,10 @@ private string ResolveFieldName(Field field)
3737
{
3838
if (field.IsConditionless()) return null;
3939
if (!field.Name.IsNullOrEmpty()) return field.Name;
40+
if (field.Expression != null && !field.CacheableExpression)
41+
{
42+
return this.Resolve(field.Expression, field.Property);
43+
}
4044

4145
string f;
4246
if (this.Fields.TryGetValue(field, out f))

‎src/Nest/CommonAbstractions/Static/Static.cs‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,15 @@ public static Fields Fields<T>(params string[] fields) where T : class =>
3939
/// <typeparam name="T">The type of the object</typeparam>
4040
/// <param name="path">The path we want to specify</param>
4141
/// <param name="boost">An optional ^boost postfix, only make sense with certain queries</param>
42-
public static Field Field<T>(Expression<Func<T, object>> path, double? boost = null)
42+
public static Field Field<T>(Expression<Func<T, object>> path, double? boost = null)
4343
where T : class =>
4444
Nest.Field.Create(path, boost);
4545

4646
public static Field Field(string field, double? boost = null) => Nest.Field.Create(field, boost);
47+
48+
public static PropertyName Property(string property) => property;
49+
50+
public static PropertyName Property<T>(Expression<Func<T, object>> path)
51+
where T : class => path;
4752
}
4853
}

‎src/Tests/ClientConcepts/HighLevel/Caching/FieldResolverCacheBusterTests.cs‎

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
using FluentAssertions;
2+
using Nest;
3+
using System;
4+
using System.Collections.Generic;
5+
using System.Linq;
6+
using System.Text;
7+
using System.Threading.Tasks;
8+
using Tests.Framework;
9+
using Tests.Framework.MockData;
10+
using static Nest.Infer;
11+
12+
namespace Tests.ClientConcepts.HighLevel.Caching
13+
{
14+
public class FieldResolverCacheBusterTests
15+
{
16+
public class TestableFieldResolver : FieldResolver
17+
{
18+
public TestableFieldResolver(IConnectionSettingsValues settings) : base(settings) { }
19+
20+
public long CachedFields => Fields.Count;
21+
public long CachedProperties => Properties.Count;
22+
}
23+
24+
public class Fields
25+
{
26+
27+
[U]
28+
public void ExpressionWithVariableSuffix()
29+
{
30+
var suffix = "raw";
31+
var resolver = new TestableFieldResolver(new ConnectionSettings());
32+
var resolved = resolver.Resolve(Field<Project>(p => p.Name.Suffix(suffix)));
33+
resolver.CachedFields.Should().Be(0);
34+
resolved.Should().EndWith("raw");
35+
suffix = "foo";
36+
resolver.Resolve(Field<Project>(p => p.Name.Suffix(suffix)));
37+
resolved = resolver.Resolve(Field<Project>(p => p.Name.Suffix(suffix)));
38+
resolver.CachedFields.Should().Be(0);
39+
resolved.Should().EndWith("foo");
40+
}
41+
42+
[U]
43+
public void ExpressionWithConstantSuffix()
44+
{
45+
var resolver = new TestableFieldResolver(new ConnectionSettings());
46+
var resolved = resolver.Resolve(Field<Project>(p => p.Name.Suffix("raw")));
47+
resolver.CachedFields.Should().Be(1);
48+
resolved.Should().EndWith("raw");
49+
resolved = resolver.Resolve(Field<Project>(p => p.Name.Suffix("foo")));
50+
resolver.CachedFields.Should().Be(2);
51+
resolved.Should().EndWith("foo");
52+
}
53+
54+
[U]
55+
public void ExpressionWithDictionaryItemVariableExpression()
56+
{
57+
var resolver = new TestableFieldResolver(new ConnectionSettings());
58+
var key = "key1";
59+
var resolved = resolver.Resolve(Field<Project>(p => p.Metadata[key]));
60+
resolver.CachedFields.Should().Be(0);
61+
resolved.Should().Contain(key);
62+
key = "key2";
63+
resolved = resolver.Resolve(Field<Project>(p => p.Metadata[key]));
64+
resolver.CachedFields.Should().Be(0);
65+
resolved.Should().Contain(key);
66+
}
67+
68+
[U]
69+
public void ExpressionWithDictionaryItemConstantExpression()
70+
{
71+
var resolver = new TestableFieldResolver(new ConnectionSettings());
72+
var resolved = resolver.Resolve(Field<Project>(p => p.Metadata["key1"]));
73+
resolver.CachedFields.Should().Be(1);
74+
resolved.Should().Contain("key1");
75+
resolved = resolver.Resolve(Field<Project>(p => p.Metadata["key2"]));
76+
resolver.CachedFields.Should().Be(2);
77+
resolved.Should().Contain("key2");
78+
}
79+
80+
[U]
81+
public void ExpressionWithDictionarySuffix()
82+
{
83+
var resolver = new TestableFieldResolver(new ConnectionSettings());
84+
var key = "key1";
85+
var d = new Dictionary<string, string> { { "key1", "raw" }, { "key2", "foo" } };
86+
resolver.Resolve(Field<Project>(p => p.Name.Suffix(d[key])));
87+
resolver.CachedFields.Should().Be(0);
88+
resolver.Resolve(Field<Project>(p => p.Name.Suffix(d["key1"])));
89+
resolver.CachedFields.Should().Be(0);
90+
key = "key2";
91+
resolver.Resolve(Field<Project>(p => p.Name.Suffix(d[key])));
92+
resolver.CachedFields.Should().Be(0);
93+
resolver.Resolve(Field<Project>(p => p.Name.Suffix(d["key2"])));
94+
resolver.CachedFields.Should().Be(0);
95+
}
96+
}
97+
98+
}
99+
}

0 commit comments

Comments
 (0)