Skip to content

Commit fd1ac35

Browse files
gmarzgmarz
committed
Merge pull request #1336 from elastic/fix/id-inferrer-alt-prop-cache
fix id properties configured on connection settings
2 parents 2f1ce22 + 6d6c444 commit fd1ac35

File tree

5 files changed

+50
-25
lines changed

5 files changed

+50
-25
lines changed

‎src/Nest/ExposedInternals/ElasticInferrer.cs‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ public string DefaultIndex
4747
public ElasticInferrer(IConnectionSettingsValues connectionSettings)
4848
{
4949
this._connectionSettings = connectionSettings;
50-
this.IdResolver = new IdResolver();
50+
this.IdResolver = new IdResolver(this._connectionSettings);
5151
this.IndexNameResolver = new IndexNameResolver(this._connectionSettings);
5252
this.TypeNameResolver = new TypeNameResolver(this._connectionSettings);
5353
this.PropertyNameResolver = new PropertyNameResolver(this._connectionSettings);
@@ -128,11 +128,6 @@ public string Id<T>(T obj) where T : class
128128
{
129129
if (obj == null) return null;
130130

131-
string idProperty;
132-
this._connectionSettings.IdProperties.TryGetValue(typeof(T), out idProperty);
133-
if (!idProperty.IsNullOrEmpty())
134-
return this.IdResolver.GetIdFor(obj, idProperty);
135-
136131
return this.IdResolver.GetIdFor(obj);
137132
}
138133

‎src/Nest/Resolvers/IdResolver.cs‎

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,17 @@ namespace Nest.Resolvers
88
{
99
public class IdResolver
1010
{
11+
private readonly IConnectionSettingsValues _connectionSettings;
12+
private ConcurrentDictionary<Type, Func<object, string>> LocalIdDelegates = new ConcurrentDictionary<Type, Func<object, string>>();
1113
private static ConcurrentDictionary<Type, Func<object, string>> IdDelegates = new ConcurrentDictionary<Type, Func<object, string>>();
1214
private static MethodInfo MakeDelegateMethodInfo = typeof(IdResolver).GetMethod("MakeDelegate", BindingFlags.Static | BindingFlags.NonPublic);
1315

16+
public IdResolver(IConnectionSettingsValues connectionSettings)
17+
{
18+
_connectionSettings = connectionSettings;
19+
}
20+
21+
1422
internal Func<T, string> CreateIdSelector<T>() where T : class
1523
{
1624
Func<T, string> idSelector = (@object) => this.GetIdFor(@object);
@@ -24,6 +32,7 @@ internal static Func<T, object> MakeDelegate<T, U>(MethodInfo @get)
2432
return t => f(t);
2533
}
2634

35+
[Obsolete("Scheduled to be removed in 2.0, is not cached and not used internally")]
2736
public string GetIdFor<T>(T @object, string idProperty)
2837
{
2938
try
@@ -48,7 +57,14 @@ public string GetIdFor<T>(T @object)
4857

4958
var type = typeof(T);
5059
Func<object, string> cachedLookup;
51-
if (IdDelegates.TryGetValue(type, out cachedLookup))
60+
string propertyName;
61+
62+
var preferLocal = this._connectionSettings.IdProperties.TryGetValue(type, out propertyName);
63+
64+
if (LocalIdDelegates.TryGetValue(type, out cachedLookup))
65+
return cachedLookup(@object);
66+
67+
if (!preferLocal && IdDelegates.TryGetValue(type, out cachedLookup))
5268
return cachedLookup(@object);
5369

5470
var idProperty = GetInferredId(type);
@@ -67,7 +83,10 @@ public string GetIdFor<T>(T @object)
6783
var v = func(obj);
6884
return v != null ? v.ToString() : null;
6985
};
70-
IdDelegates.TryAdd(type, cachedLookup);
86+
if (preferLocal)
87+
LocalIdDelegates.TryAdd(type, cachedLookup);
88+
else
89+
IdDelegates.TryAdd(type, cachedLookup);
7190
return cachedLookup(@object);
7291
}
7392
catch
@@ -81,26 +100,31 @@ private PropertyInfo GetInferredId(Type type)
81100
{
82101
// if the type specifies through ElasticAttribute what the id prop is
83102
// use that no matter what
103+
104+
string propertyName;
105+
this._connectionSettings.IdProperties.TryGetValue(type, out propertyName);
106+
if (!propertyName.IsNullOrEmpty())
107+
return GetPropertyCaseInsensitive(type, propertyName);
108+
84109
var esTypeAtt = ElasticAttributes.Type(type);
85110
if (esTypeAtt != null && !string.IsNullOrWhiteSpace(esTypeAtt.IdProperty))
86111
return GetPropertyCaseInsensitive(type, esTypeAtt.IdProperty);
87112

88-
var propertyName = "Id";
89-
113+
propertyName = "Id";
90114
//Try Id on its own case insensitive
91115
var idProperty = GetPropertyCaseInsensitive(type, propertyName);
92116
if (idProperty != null)
93117
return idProperty;
94118

119+
//TODO remove in 2.0 ?
95120
//Try TypeNameId case insensitive
96121
idProperty = GetPropertyCaseInsensitive(type, type.Name + propertyName);
97122
if (idProperty != null)
98123
return idProperty;
99124

125+
//TODO remove in 2.0 ?
100126
//Try TypeName_Id case insensitive
101127
idProperty = GetPropertyCaseInsensitive(type, type.Name + "_" + propertyName);
102-
if (idProperty != null)
103-
return idProperty;
104128

105129
return idProperty;
106130
}

‎src/Nest/Resolvers/TypeNameResolver.cs‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,11 @@ namespace Nest.Resolvers
77
public class TypeNameResolver
88
{
99
private readonly IConnectionSettingsValues _connectionSettings;
10-
private PropertyNameResolver _propertyNameResolver;
1110

1211
public TypeNameResolver(IConnectionSettingsValues connectionSettings)
1312
{
1413
connectionSettings.ThrowIfNull("connectionSettings");
1514
this._connectionSettings = connectionSettings;
16-
this._propertyNameResolver = new PropertyNameResolver(this._connectionSettings);
1715
}
1816

1917
public string GetTypeNameFor<T>()

‎src/Tests/Nest.Tests.Unit/Internals/Inferno/IdLookupTests.cs‎

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,13 @@ namespace Nest.Tests.Unit.Internals.Inferno
88
[TestFixture]
99
public class IdLookupTests
1010
{
11+
private IdResolver _idResolver;
12+
13+
public IdLookupTests()
14+
{
15+
this._idResolver = new IdResolver(new ConnectionSettings());
16+
}
17+
1118
[ElasticType(IdProperty = "Guid")]
1219
internal class AlternateIdClass
1320
{
@@ -35,60 +42,60 @@ internal class InheritedIdClass : BaseIdClass { public string Name { get; set; }
3542
public void TestAlternateIdLookup()
3643
{
3744
var expectedGuid = Guid.NewGuid();
38-
var id = new IdResolver().GetIdFor(new AlternateIdClass { Guid = expectedGuid });
45+
var id = this._idResolver.GetIdFor(new AlternateIdClass { Guid = expectedGuid });
3946
StringAssert.AreEqualIgnoringCase(expectedGuid.ToString(), id);
4047
}
4148

4249
[Test]
4350
public void TestIntLookup()
4451
{
4552
var expected = 12;
46-
var id = new IdResolver().GetIdFor(new IntIdClass { Id = expected });
53+
var id = this._idResolver.GetIdFor(new IntIdClass { Id = expected });
4754
StringAssert.AreEqualIgnoringCase(expected.ToString(CultureInfo.InvariantCulture), id);
4855
}
4956
[Test]
5057
public void TestDecimalLookup()
5158
{
5259
var expected = 12m;
53-
var id = new IdResolver().GetIdFor(new DecimalIdClass { Id = expected });
60+
var id = this._idResolver.GetIdFor(new DecimalIdClass { Id = expected });
5461
StringAssert.AreEqualIgnoringCase(expected.ToString(CultureInfo.InvariantCulture), id);
5562
}
5663
[Test]
5764
public void TestFloatLookup()
5865
{
5966
var expected = 12f;
60-
var id = new IdResolver().GetIdFor(new FloatIdClass { Id = expected });
67+
var id = this._idResolver.GetIdFor(new FloatIdClass { Id = expected });
6168
StringAssert.AreEqualIgnoringCase(expected.ToString(CultureInfo.InvariantCulture), id);
6269
}
6370
[Test]
6471
public void TestLongLookup()
6572
{
6673
var expected = long.MaxValue;
67-
var id = new IdResolver().GetIdFor(new LongIdClass { Id = expected });
74+
var id = this._idResolver.GetIdFor(new LongIdClass { Id = expected });
6875
StringAssert.AreEqualIgnoringCase(expected.ToString(CultureInfo.InvariantCulture), id);
6976
}
7077

7178
[Test]
7279
public void TestDoubleLookup()
7380
{
7481
var expected = 12d;
75-
var id = new IdResolver().GetIdFor(new DoubleIdClass { Id = expected });
82+
var id = this._idResolver.GetIdFor(new DoubleIdClass { Id = expected });
7683
StringAssert.AreEqualIgnoringCase(expected.ToString(CultureInfo.InvariantCulture), id);
7784
}
7885
[Test]
7986
public void TestCustomLookup()
8087
{
8188
var expected = new MyCustomClass();
82-
var id = new IdResolver().GetIdFor(new CustomObjectIdClass { Id = expected });
89+
var id = this._idResolver.GetIdFor(new CustomObjectIdClass { Id = expected });
8390
StringAssert.AreEqualIgnoringCase(expected.ToString(), id);
8491
}
8592

8693
[Test]
8794
public void TestInheritedLookup()
8895
{
8996
var expected = new InheritedIdClass() { Id = 123 };
90-
var id = new IdResolver().GetIdFor(expected);
91-
id = new IdResolver().GetIdFor(expected);
97+
var id = this._idResolver.GetIdFor(expected);
98+
id = this._idResolver.GetIdFor(expected);
9299
Assert.AreEqual(expected.Id.ToString(), id);
93100
}
94101

@@ -97,8 +104,8 @@ public void TestInheritedLookup()
97104
public void TestHitsCache()
98105
{
99106
var expected = 12m;
100-
var id = new IdResolver().GetIdFor(new DecimalIdClass { Id = expected });
101-
id = new IdResolver().GetIdFor(new DecimalIdClass { Id = expected });
107+
var id = this._idResolver.GetIdFor(new DecimalIdClass { Id = expected });
108+
id = this._idResolver.GetIdFor(new DecimalIdClass { Id = expected });
102109
StringAssert.AreEqualIgnoringCase(expected.ToString(), id);
103110
}
104111
}

‎src/Tests/Nest.Tests.Unit/Internals/Inferno/MapIdPropertyForTests.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@ public void IdPropertyMapped_And_TypeHasIdPropertyAttribute_MappingTakesPreceden
118118
};
119119

120120
Assert.AreEqual(doc.Id, client.Infer.Id<IdPropertyTestWithAttribute>(doc));
121+
Assert.AreEqual(doc.Id, client.Infer.Id<IdPropertyTestWithAttribute>(doc));
121122
}
122123
}
123124
}

0 commit comments

Comments
 (0)