Skip to content

Commit 40e25c6

Browse files
russcamgmarz
russcam
authored and
gmarz
committed
Introduce ResolveException for bubbling exceptions out of Field/IndexNameResolver
This is thrown if a field/property or index name cannot be resolved. Naming conventions are deferred to Elasticsearch with the exception of uppercase characters in index names (since this is a common pitfall). Fixes #1889
1 parent 81986a4 commit 40e25c6

File tree

8 files changed

+200
-33
lines changed

8 files changed

+200
-33
lines changed

‎src/Elasticsearch.Net/Elasticsearch.Net.csproj‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@
8383
<Compile Include="ElasticLowLevelClient.cs" />
8484
<Compile Include="ElasticLowLevelClient.Generated.cs" />
8585
<Compile Include="Exceptions\ElasticsearchClientException.cs" />
86+
<Compile Include="Exceptions\ResolveException.cs" />
8687
<Compile Include="Exceptions\UnexpectedElasticsearchClientException.cs" />
8788
<Compile Include="Extensions\ExceptionExtensions.cs" />
8889
<Compile Include="Extensions\Extensions.cs" />
@@ -139,4 +140,4 @@
139140
</Target>
140141
-->
141142
<Import Project="..\..\.paket\paket.targets" />
142-
</Project>
143+
</Project>

‎src/Elasticsearch.Net/Exceptions/ResolveException.cs‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
using System;
2+
using System.Collections.Generic;
3+
using System.Linq;
4+
using System.Text;
5+
using System.Threading.Tasks;
6+
7+
namespace Elasticsearch.Net
8+
{
9+
public class ResolveException : Exception
10+
{
11+
public ResolveException(string message) : base(message) { }
12+
}
13+
}

‎src/Elasticsearch.Net/Transport/Transport.cs‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,15 +15,15 @@ public class Transport<TConnectionSettings> : ITransport<TConnectionSettings>
1515
public IRequestPipelineFactory PipelineProvider { get; }
1616

1717
/// <summary>
18-
/// Transport coordinates the client requests over the connection pool nodes and is in charge of falling over on different nodes
18+
/// Transport coordinates the client requests over the connection pool nodes and is in charge of falling over on different nodes
1919
/// </summary>
2020
/// <param name="configurationValues">The connectionsettings to use for this transport</param>
2121
public Transport(TConnectionSettings configurationValues)
2222
: this(configurationValues, null, null, null)
2323
{ }
2424

2525
/// <summary>
26-
/// Transport coordinates the client requests over the connection pool nodes and is in charge of falling over on different nodes
26+
/// Transport coordinates the client requests over the connection pool nodes and is in charge of falling over on different nodes
2727
/// </summary>
2828
/// <param name="configurationValues">The connectionsettings to use for this transport</param>
2929
/// <param name="pipelineProvider">In charge of create a new pipeline, safe to pass null to use the default</param>
@@ -83,6 +83,11 @@ public ElasticsearchResponse<TReturn> Request<TReturn>(HttpMethod method, string
8383
pipeline.MarkDead(node);
8484
seenExceptions.Add(pipelineException);
8585
}
86+
catch (ResolveException resolveException)
87+
{
88+
resolveException.RethrowKeepingStackTrace();
89+
return null; //dead code due to call to RethrowKeepingStackTrace()
90+
}
8691
catch (Exception killerException)
8792
{
8893
throw new UnexpectedElasticsearchClientException(killerException, seenExceptions)
@@ -143,6 +148,11 @@ public async Task<ElasticsearchResponse<TReturn>> RequestAsync<TReturn>(HttpMeth
143148
pipeline.MarkDead(node);
144149
seenExceptions.Add(pipelineException);
145150
}
151+
catch (ResolveException resolveException)
152+
{
153+
resolveException.RethrowKeepingStackTrace();
154+
return null; //dead code due to call to RethrowKeepingStackTrace()
155+
}
146156
catch (Exception killerException)
147157
{
148158
throw new UnexpectedElasticsearchClientException(killerException, seenExceptions)

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

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -51,17 +51,14 @@ public string Resolve(PropertyName property)
5151
{
5252
if (property.IsConditionless()) return null;
5353
if (!property.Name.IsNullOrEmpty())
54-
{
55-
if (property.Name.Contains("."))
56-
throw new ArgumentException("Property names cannot contain dots.");
5754
return property.Name;
58-
}
59-
string f;
60-
if (this.Properties.TryGetValue(property, out f))
61-
return f;
62-
f = this.Resolve(property.Expression, property.Property, true);
63-
this.Properties.TryAdd(property, f);
64-
return f;
55+
56+
string propertyName;
57+
if (this.Properties.TryGetValue(property, out propertyName))
58+
return propertyName;
59+
propertyName = this.Resolve(property.Expression, property.Property, true);
60+
this.Properties.TryAdd(property, propertyName);
61+
return propertyName;
6562
}
6663

6764
private string Resolve(Expression expression, MemberInfo member, bool toLastToken = false)
@@ -74,7 +71,7 @@ private string Resolve(Expression expression, MemberInfo member, bool toLastToke
7471
: null;
7572

7673
if (name == null)
77-
throw new ArgumentException("Could not resolve a name from the given Expression or MemberInfo.");
74+
throw new ResolveException("Name resolved to null for the given Expression or MemberInfo.");
7875

7976
return name;
8077
}

‎src/Nest/CommonAbstractions/Infer/IndexName/IndexNameResolver.cs‎

Lines changed: 27 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
using System;
1+
using Elasticsearch.Net;
2+
using System;
23

34
namespace Nest
45
{
@@ -15,26 +16,37 @@ public IndexNameResolver(IConnectionSettingsValues connectionSettings)
1516

1617
public string Resolve(IndexName i)
1718
{
18-
if (i == null) return this.Resolve((Type)null);
19-
return i.Name ?? this.Resolve(i.Type);
19+
if (i == null || string.IsNullOrEmpty(i.Name))
20+
return this.Resolve((Type)null);
21+
ValidateIndexName(i.Name);
22+
return i.Name;
2023
}
2124

2225
public string Resolve(Type type)
2326
{
27+
var indexName = this._connectionSettings.DefaultIndex;
2428
var defaultIndices = this._connectionSettings.DefaultIndices;
25-
26-
if (defaultIndices == null)
27-
return this._connectionSettings.DefaultIndex;
28-
29-
if (type == null)
30-
return this._connectionSettings.DefaultIndex;
31-
32-
string value;
33-
if (defaultIndices.TryGetValue(type, out value) && !string.IsNullOrWhiteSpace(value))
34-
return value;
35-
return this._connectionSettings.DefaultIndex;
29+
if (defaultIndices != null && type != null)
30+
{
31+
string value;
32+
if (defaultIndices.TryGetValue(type, out value) && !string.IsNullOrEmpty(value))
33+
indexName = value;
34+
}
35+
ValidateIndexName(indexName, type);
36+
return indexName;
3637
}
3738

38-
39+
private void ValidateIndexName(string indexName, Type type = null)
40+
{
41+
if (string.IsNullOrWhiteSpace(indexName))
42+
throw new ResolveException(
43+
"Index name is null for the given type and no default index is set. "
44+
+ "Map an index name using ConnectionSettings.MapDefaultTypeIndices() "
45+
+ "or set a default index using ConnectionSettings.DefaultIndex()."
46+
);
47+
48+
if (indexName.HasAny(c => char.IsUpper(c)))
49+
throw new ResolveException($"Index names cannot contain uppercase characters: {indexName}.");
50+
}
3951
}
4052
}

‎src/Tests/ClientConcepts/HighLevel/Inferrence/IndexNameInference.doc.cs‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
using Elasticsearch.Net;
2+
using FluentAssertions;
3+
using Nest;
4+
using System;
5+
using System.Collections.Generic;
6+
using System.Linq;
7+
using System.Text;
8+
using System.Threading.Tasks;
9+
using Tests.Framework;
10+
using Tests.Framework.MockData;
11+
using Xunit;
12+
13+
namespace Tests.ClientConcepts.HighLevel.Inferrence
14+
{
15+
public class IndexNameInference
16+
{
17+
[U]
18+
public void DefaultIndexIsInferred()
19+
{
20+
var settings = new ConnectionSettings()
21+
.DefaultIndex("defaultindex");
22+
var resolver = new IndexNameResolver(settings);
23+
var index = resolver.Resolve<Project>();
24+
index.Should().Be("defaultindex");
25+
}
26+
27+
[U]
28+
public void ExplicitMappingIsInferred()
29+
{
30+
var settings = new ConnectionSettings()
31+
.MapDefaultTypeIndices(m => m
32+
.Add(typeof(Project), "projects")
33+
);
34+
var resolver = new IndexNameResolver(settings);
35+
var index = resolver.Resolve<Project>();
36+
index.Should().Be("projects");
37+
}
38+
39+
[U]
40+
public void ExplicitMappingTakesPrecedence()
41+
{
42+
var settings = new ConnectionSettings()
43+
.DefaultIndex("defaultindex")
44+
.MapDefaultTypeIndices(m => m
45+
.Add(typeof(Project), "projects")
46+
);
47+
var resolver = new IndexNameResolver(settings);
48+
var index = resolver.Resolve<Project>();
49+
index.Should().Be("projects");
50+
}
51+
52+
[U]
53+
public void UppercaseCharacterThrowsResolveException()
54+
{
55+
var settings = new ConnectionSettings()
56+
.DefaultIndex("Default")
57+
.MapDefaultTypeIndices(m => m
58+
.Add(typeof(Project), "myProjects")
59+
);
60+
61+
var resolver = new IndexNameResolver(settings);
62+
63+
var e = Assert.Throws<ResolveException>(() => resolver.Resolve<Project>());
64+
e.Message.Should().Be($"Index names cannot contain uppercase characters: myProjects.");
65+
e = Assert.Throws<ResolveException>(() => resolver.Resolve<Tag>());
66+
e.Message.Should().Be($"Index names cannot contain uppercase characters: Default.");
67+
e = Assert.Throws<ResolveException>(() => resolver.Resolve("Foo"));
68+
e.Message.Should().Be($"Index names cannot contain uppercase characters: Foo.");
69+
}
70+
71+
[U]
72+
public void NoIndexThrowsResolveException()
73+
{
74+
var settings = new ConnectionSettings();
75+
var resolver = new IndexNameResolver(settings);
76+
var e = Assert.Throws<ResolveException>(() => resolver.Resolve<Project>());
77+
e.Message.Should().Contain("Index name is null");
78+
}
79+
80+
[U]
81+
public void ResolveExceptionBubblesOut()
82+
{
83+
var client = TestClient.GetInMemoryClient(s => new ConnectionSettings());
84+
var e = Assert.Throws<ResolveException>(() => client.Search<Project>());
85+
86+
}
87+
}
88+
}

‎src/Tests/ClientConcepts/HighLevel/Inferrence/PropertyInference.doc.cs‎

Lines changed: 49 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,24 +2,69 @@
22
using System.Linq.Expressions;
33
using Nest;
44
using Tests.Framework;
5+
using Tests.Framework.Integration;
56
using Tests.Framework.MockData;
67
using static Tests.Framework.RoundTripper;
78
using Xunit;
9+
using FluentAssertions;
810

911
namespace Tests.ClientConcepts.HighLevel.Inferrence.PropertyNames
1012
{
11-
12-
public class PropertyNames
13+
/** == Property Names */
14+
[Collection(IntegrationContext.Indexing)]
15+
public class PropertyNames : SimpleIntegration
1316
{
17+
private IElasticClient _client;
18+
19+
public PropertyNames(IndexingCluster cluster) : base(cluster)
20+
{
21+
_client = cluster.Node.Client();
22+
}
23+
24+
/** Property names resolve to the last token */
1425
[U] public void PropertyNamesAreResolvedToLastToken()
1526
{
1627
Expression<Func<Project, object>> expression = p => p.Name.Suffix("raw");
1728
Expect("raw").WhenSerializing<PropertyName>(expression);
1829
}
1930

20-
[U] public void StringsContainingDotsIsAnException()
31+
/** :multi-field: {ref_current}/_multi_fields.html
32+
*Property names cannot contain a `.` (dot), because of the potential for ambiguity with
33+
*a field that is mapped as a {multi-field}[`multi_field`].
34+
*
35+
*NEST allows the call to go to Elasticsearch, deferring the naming conventions to the server side and,
36+
* in the case of dots in field names, returns a `400 Bad Response` with a server error indicating the reason.
37+
*/
38+
[I] public void PropertyNamesContainingDotsCausesElasticsearchServerError()
2139
{
22-
Assert.Throws<ArgumentException>(() => Expect("exception!").WhenSerializing<PropertyName>("name.raw"));
40+
var createIndexResponse = _client.CreateIndex("random-" + Guid.NewGuid().ToString().ToLowerInvariant(), c => c
41+
.Mappings(m => m
42+
.Map("type-with-dot", mm => mm
43+
.Properties(p => p
44+
.String(s => s
45+
.Name("name-with.dot")
46+
)
47+
)
48+
)
49+
)
50+
);
51+
52+
/** The response is not valid */
53+
createIndexResponse.IsValid.Should().BeFalse();
54+
55+
/** `DebugInformation` provides an audit trail of information to help diagnose the issue */
56+
createIndexResponse.DebugInformation.Should().NotBeNullOrEmpty();
57+
58+
/** `ServerError` contains information from the response from Elasticsearch */
59+
createIndexResponse.ServerError.Should().NotBeNull();
60+
createIndexResponse.ServerError.Status.Should().Be(400);
61+
createIndexResponse.ServerError.Error.Should().NotBeNull();
62+
createIndexResponse.ServerError.Error.RootCause.Should().NotBeNullOrEmpty();
63+
64+
var rootCause = createIndexResponse.ServerError.Error.RootCause[0];
65+
66+
rootCause.Reason.Should().Be("Field name [name-with.dot] cannot contain '.'");
67+
rootCause.Type.Should().Be("mapper_parsing_exception");
2368
}
2469
}
2570
}

‎src/Tests/Tests.csproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -542,6 +542,7 @@
542542
<Compile Include="Cat\CatRepositories\CatRepositoriesUrlTests.cs" />
543543
<Compile Include="Cat\CatSnapshots\CatSnapshotsApiTests.cs" />
544544
<Compile Include="Cat\CatSnapshots\CatSnapshotsUrlTests.cs" />
545+
<Compile Include="ClientConcepts\HighLevel\Inferrence\IndexNameInference.doc.cs" />
545546
<Compile Include="Search\Request\ProfileUsageTests.cs" />
546547
<Compile Include="Indices\StatusManagement\ForceMerge\ForceMergeApiTests.cs" />
547548
<Compile Include="Indices\StatusManagement\ForceMerge\ForceMergeUrlTests.cs" />

0 commit comments

Comments
 (0)