Skip to content

Commit e402598

Browse files
gmarzgmarz
committed
Port of #2321 caching of constant Field Expressions
1 parent 9750fa6 commit e402598

File tree

4 files changed

+95
-24
lines changed

4 files changed

+95
-24
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ public Field(Expression expression, double? boost = null)
4545
Type type;
4646
_comparisonValue = expression.ComparisonValueFromExpression(out type);
4747
_type = type;
48-
CachableExpression = !new HasConstantExpressionVisitor(expression).Found;
48+
CachableExpression = !new HasVariableExpressionVisitor(expression).Found;
4949
}
5050

5151
public Field(PropertyInfo property, double? boost = null)

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

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -12,18 +12,24 @@
1212

1313
namespace Nest
1414
{
15-
internal class HasConstantExpressionVisitor : ExpressionVisitor
15+
internal class HasVariableExpressionVisitor : ExpressionVisitor
1616
{
17-
public bool Found { get; private set; }
17+
private bool _found;
18+
public bool Found
19+
{
20+
get { return _found; }
21+
// This is only set to true once to prevent clobbering from subsequent node visits
22+
private set { if (!_found) _found = value; }
23+
}
1824

19-
public HasConstantExpressionVisitor(Expression e)
25+
public HasVariableExpressionVisitor(Expression e)
2026
{
2127
this.Visit(e);
2228
}
2329

2430
public override Expression Visit(Expression node)
2531
{
26-
if (!Found)
32+
if (!this.Found)
2733
return base.Visit(node);
2834
return node;
2935
}
@@ -35,7 +41,6 @@ protected override Expression VisitMethodCall(MethodCallExpression node)
3541
var lastArg = node.Arguments.Last();
3642
var constantExpression = lastArg as ConstantExpression;
3743
this.Found = constantExpression == null;
38-
return node;
3944
}
4045
else if (node.Method.Name == "get_Item" && node.Arguments.Any())
4146
{
@@ -50,21 +55,13 @@ protected override Expression VisitMethodCall(MethodCallExpression node)
5055
var lastArg = node.Arguments.Last();
5156
var constantExpression = lastArg as ConstantExpression;
5257
this.Found = constantExpression == null;
53-
return node;
5458
}
5559
return base.VisitMethodCall(node);
5660
}
57-
58-
protected override Expression VisitConstant(ConstantExpression node)
59-
{
60-
this.Found = true;
61-
return node;
62-
}
6361
}
6462

65-
66-
internal class FieldExpressionVisitor : ExpressionVisitor
67-
{
63+
internal class FieldExpressionVisitor : ExpressionVisitor
64+
{
6865
private readonly Stack<string> _stack = new Stack<string>();
6966

7067
private readonly IConnectionSettingsValues _settings;
@@ -132,7 +129,9 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCall)
132129
|| (t.IsGeneric() && t.GetGenericTypeDefinition() == typeof(IDictionary<,>));
133130

134131
if (!isDict)
132+
{
135133
return base.VisitMethodCall(methodCall);
134+
}
136135
VisitConstantOrVariable(methodCall, _stack);
137136
Visit(methodCall.Object);
138137
return methodCall;
@@ -166,5 +165,5 @@ private static bool IsLinqOperator(MethodInfo methodInfo)
166165

167166
return methodInfo.GetCustomAttribute<ExtensionAttribute>() != null;
168167
}
169-
}
168+
}
170169
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ public PropertyName(Expression expression)
2626
{
2727
Type type;
2828
Expression = expression;
29-
CacheableExpression = !new HasConstantExpressionVisitor(expression).Found;
29+
CacheableExpression = !new HasVariableExpressionVisitor(expression).Found;
3030
_comparisonValue = expression.ComparisonValueFromExpression(out type);
3131
_type = type;
3232
}

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

Lines changed: 78 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -147,11 +147,11 @@ public void ExpressionWithVariableSuffix()
147147
var suffix = "raw";
148148
var resolver = new TestableFieldResolver(new ConnectionSettings());
149149
var resolved = resolver.Resolve(Field<Project>(p => p.Name.Suffix(suffix)));
150-
resolved.Should().EndWith("raw");
150+
resolved.Should().Be("name.raw");
151151
resolver.CachedFields.Should().Be(0);
152152
suffix = "foo";
153153
resolved = resolver.Resolve(Field<Project>(p => p.Name.Suffix(suffix)));
154-
resolved.Should().EndWith("foo");
154+
resolved.Should().Be("name.foo");
155155
resolver.CachedFields.Should().Be(0);
156156
}
157157

@@ -169,16 +169,76 @@ public void ExpressionWithDictionaryItemVariableExpression()
169169
resolved.Should().Contain(key);
170170
}
171171

172+
[U]
173+
public void ExpressionWithDictionaryItemVariableExpressionAndVariableSuffix()
174+
{
175+
var resolver = new TestableFieldResolver(new ConnectionSettings());
176+
var key = "key1";
177+
var suffix = "x";
178+
var resolved = resolver.Resolve(Field<Project>(p => p.Metadata[key].Suffix(suffix)));
179+
resolver.CachedFields.Should().Be(0);
180+
resolved.Should().Be($"metadata.{key}.x");
181+
key = "key2";
182+
suffix = "y";
183+
resolved = resolver.Resolve(Field<Project>(p => p.Metadata[key].Suffix(suffix)));
184+
resolver.CachedFields.Should().Be(0);
185+
resolved.Should().Be($"metadata.{key}.y");
186+
}
187+
188+
[U]
189+
public void ExpressionWithDictionaryItemVariableExpressionAndEquivalentVariableSuffix()
190+
{
191+
var resolver = new TestableFieldResolver(new ConnectionSettings());
192+
var key = "key1";
193+
var suffix = "x";
194+
var resolved = resolver.Resolve(Field<Project>(p => p.Metadata[key].Suffix(suffix)));
195+
resolver.CachedFields.Should().Be(0);
196+
resolved.Should().Be($"metadata.{key}.x");
197+
key = "key2";
198+
resolved = resolver.Resolve(Field<Project>(p => p.Metadata[key].Suffix(suffix)));
199+
resolver.CachedFields.Should().Be(0);
200+
resolved.Should().Be($"metadata.{key}.x");
201+
}
202+
203+
[U]
204+
public void ExpressionWithDictionaryItemVariableExpressionAndConstantSuffix()
205+
{
206+
var resolver = new TestableFieldResolver(new ConnectionSettings());
207+
var key = "key1";
208+
var resolved = resolver.Resolve(Field<Project>(p => p.Metadata[key].Suffix("x")));
209+
resolver.CachedFields.Should().Be(0);
210+
resolved.Should().Be($"metadata.{key}.x");
211+
key = "key2";
212+
resolved = resolver.Resolve(Field<Project>(p => p.Metadata[key].Suffix("y")));
213+
resolver.CachedFields.Should().Be(0);
214+
resolved.Should().Be($"metadata.{key}.y");
215+
}
216+
217+
[U]
218+
public void ExpressionWithDictionaryItemVariableExpressionAndEquivalentConstantSuffix()
219+
{
220+
var resolver = new TestableFieldResolver(new ConnectionSettings());
221+
var key = "key1";
222+
var resolved = resolver.Resolve(Field<Project>(p => p.Metadata[key].Suffix("x")));
223+
resolver.CachedFields.Should().Be(0);
224+
resolved.Should().Be($"metadata.{key}.x");
225+
key = "key2";
226+
resolved = resolver.Resolve(Field<Project>(p => p.Metadata[key].Suffix("x")));
227+
resolver.CachedFields.Should().Be(0);
228+
resolved.Should().Be($"metadata.{key}.x");
229+
}
230+
231+
172232
[U]
173233
public void ExpressionWithDictionaryItemConstantExpression()
174234
{
175235
var resolver = new TestableFieldResolver(new ConnectionSettings());
176236
var resolved = resolver.Resolve(Field<Project>(p => p.Metadata["key1"]));
177237
resolver.CachedFields.Should().Be(1);
178-
resolved.Should().Contain("key1");
238+
resolved.Should().Be("metadata.key1");
179239
resolved = resolver.Resolve(Field<Project>(p => p.Metadata["key2"]));
180240
resolver.CachedFields.Should().Be(2);
181-
resolved.Should().Contain("key2");
241+
resolved.Should().Be("metadata.key2");
182242
}
183243

184244
[U]
@@ -188,11 +248,23 @@ public void ExpressionWithDictionaryItemConstantExpressionAndVariableSuffix()
188248
var suffix = "x";
189249
var resolved = resolver.Resolve(Field<Project>(p => p.Metadata["key1"].Suffix(suffix)));
190250
resolver.CachedFields.Should().Be(0);
191-
resolved.Should().Contain("key1").And.EndWith(".x");
251+
resolved.Should().Be("metadata.key1.x");
192252
suffix = "y";
193253
resolved = resolver.Resolve(Field<Project>(p => p.Metadata["key2"].Suffix(suffix)));
194254
resolver.CachedFields.Should().Be(0);
195-
resolved.Should().Contain("key2").And.EndWith(".y");
255+
resolved.Should().Be("metadata.key2.y");
256+
}
257+
258+
[U]
259+
public void ExpressionWithDictionaryItemConstantExpressionAndConstantSuffix()
260+
{
261+
var resolver = new TestableFieldResolver(new ConnectionSettings());
262+
var resolved = resolver.Resolve(Field<Project>(p => p.Metadata["key1"].Suffix("x")));
263+
resolver.CachedFields.Should().Be(1);
264+
resolved.Should().Be("metadata.key1.x");
265+
resolved = resolver.Resolve(Field<Project>(p => p.Metadata["key2"].Suffix("y")));
266+
resolver.CachedFields.Should().Be(2);
267+
resolved.Should().Be("metadata.key2.y");
196268
}
197269

198270
[U]

0 commit comments

Comments
 (0)