Skip to content

Commit 3c3518d

Browse files
MpdreamzMpdreamz
committed
fix #1930 unwrap .Terms(new List<T>) to just T
When calling ```csharp .Terms(new List<T>()); ``` The user expects T[] to be written to elasticsearch but due to C# resolution they end up calling ```csharp .Terms<List<T>>(params List<T>) ``` and we serialize to T[][] which is not a valid terms query. The fluent Terms() method now tries to preempt these cases and unwraps to T[] when a single IEnumerable is passed as T, special casing `string` to be exluded from the unwrapping behavior so we do not unwrap string to char[]
1 parent 81986a4 commit 3c3518d

File tree

6 files changed

+227
-13
lines changed

6 files changed

+227
-13
lines changed

‎src/Nest/QueryDsl/TermLevel/Terms/TermsQuery.cs

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
using System;
2+
using System.Collections;
23
using System.Collections.Generic;
34
using System.Linq;
45
using Newtonsoft.Json;
@@ -26,14 +27,14 @@ public class TermsQuery : FieldNameQueryBase, ITermsQuery
2627
internal override void WrapInContainer(IQueryContainer c) => c.Terms = this;
2728
internal static bool IsConditionless(ITermsQuery q)
2829
{
29-
return q.Field.IsConditionless()
30+
return q.Field.IsConditionless()
3031
|| (
31-
(q.Terms == null
32-
|| !q.Terms.HasAny()
33-
|| q.Terms.All(t=>t == null
32+
(q.Terms == null
33+
|| !q.Terms.HasAny()
34+
|| q.Terms.All(t=>t == null
3435
|| ((t as string)?.IsNullOrEmpty()).GetValueOrDefault(false))
3536
)
36-
&&
37+
&&
3738
(q.TermsLookup == null
3839
|| q.TermsLookup.Id == null
3940
|| q.TermsLookup.Path.IsConditionless()
@@ -44,11 +45,11 @@ internal static bool IsConditionless(ITermsQuery q)
4445
}
4546

4647
/// <summary>
47-
/// A query that match on any (configurable) of the provided terms.
48+
/// A query that match on any (configurable) of the provided terms.
4849
/// This is a simpler syntax query for using a bool query with several term queries in the should clauses.
4950
/// </summary>
5051
/// <typeparam name="T">The type that represents the expected hit type</typeparam>
51-
public class TermsQueryDescriptor<T>
52+
public class TermsQueryDescriptor<T>
5253
: FieldNameQueryDescriptorBase<TermsQueryDescriptor<T>, ITermsQuery, T>
5354
, ITermsQuery where T : class
5455
{
@@ -67,7 +68,13 @@ public TermsQueryDescriptor<T> TermsLookup<TOther>(Func<FieldLookupDescriptor<TO
6768

6869
public TermsQueryDescriptor<T> Terms<TValue>(IEnumerable<TValue> terms) => Assign(a => a.Terms = terms?.Cast<object>());
6970

70-
public TermsQueryDescriptor<T> Terms<TValue>(params TValue[] terms) => Assign(a => a.Terms = terms?.Cast<object>());
71+
public TermsQueryDescriptor<T> Terms<TValue>(params TValue[] terms) => Assign(a => {
72+
if(terms?.Length == 1 && typeof(IEnumerable).IsAssignableFrom(typeof(TValue)) && typeof(TValue) != typeof(string))
73+
{
74+
a.Terms = (terms.First() as IEnumerable)?.Cast<object>();
75+
}
76+
else a.Terms = terms?.Cast<object>();
77+
});
7178

7279
}
7380
}

‎src/Nest/QueryDsl/TermLevel/Terms/TermsQueryJsonConverter.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ public override object ReadJson(JsonReader reader, Type objectType, object exist
7575
case "minimum_should_match":
7676
reader.Read();
7777
var min = serializer.Deserialize<MinimumShouldMatch>(reader);
78-
f.MinimumShouldMatch = min;
78+
f.MinimumShouldMatch = min;
7979
break;
8080
case "boost":
8181
reader.Read();
@@ -131,7 +131,7 @@ private void ReadTerms(ITermsQuery termsQuery, JsonReader reader, JsonSerializer
131131
}
132132
else if (reader.TokenType == JsonToken.StartArray)
133133
{
134-
var values = JArray.Load(reader).Values<string>();
134+
var values = JArray.Load(reader).Values<object>();
135135
termsQuery.Terms = values;
136136
}
137137
}

‎src/Tests/QueryDsl/QueryDslIntegrationTestsBase.cs

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
using System;
2+
using System.Collections.Generic;
3+
using Elasticsearch.Net;
4+
using FluentAssertions;
5+
using Nest;
6+
using Tests.Framework;
7+
using Tests.Framework.Integration;
8+
using Tests.Framework.MockData;
9+
using Xunit;
10+
11+
namespace Tests.QueryDsl
12+
{
13+
[Collection(IntegrationContext.ReadOnly)]
14+
public abstract class QueryDslIntegrationTestsBase : ApiIntegrationTestBase<ISearchResponse<Project>, ISearchRequest, SearchDescriptor<Project>, SearchRequest<Project>>
15+
{
16+
protected QueryDslIntegrationTestsBase(IIntegrationCluster cluster, EndpointUsage usage) : base(cluster, usage) { }
17+
protected override LazyResponses ClientUsage() => Calls(
18+
fluent: (client, f) => client.Search<Project>(f),
19+
fluentAsync: (client, f) => client.SearchAsync<Project>(f),
20+
request: (client, r) => client.Search<Project>(r),
21+
requestAsync: (client, r) => client.SearchAsync<Project>(r)
22+
);
23+
24+
protected override HttpMethod HttpMethod => HttpMethod.POST;
25+
protected override string UrlPath => "/project/project/_search";
26+
protected override int ExpectStatusCode => 200;
27+
protected override bool ExpectIsValid => true;
28+
29+
protected abstract object QueryJson { get; }
30+
31+
protected abstract QueryContainer QueryInitializer { get; }
32+
protected abstract QueryContainer QueryFluent(QueryContainerDescriptor<Project> q);
33+
34+
protected override object ExpectJson => new { query = this.QueryJson };
35+
36+
protected override Func<SearchDescriptor<Project>, ISearchRequest> Fluent => s => s
37+
.Query(this.QueryFluent);
38+
39+
protected override SearchRequest<Project> Initializer =>
40+
new SearchRequest<Project>
41+
{
42+
Query = this.QueryInitializer
43+
};
44+
}
45+
}

‎src/Tests/QueryDsl/TermLevel/Terms/TermsListQueryUsageTests.cs

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
using System.Collections.Generic;
2+
using System.Linq;
3+
using System.Threading.Tasks;
4+
using FluentAssertions;
5+
using Nest;
6+
using Tests.Framework;
7+
using Tests.Framework.Integration;
8+
using Tests.Framework.MockData;
9+
10+
namespace Tests.QueryDsl.TermLevel.Terms
11+
{
12+
public class TermsListQueryUsageTests : QueryDslUsageTestsBase
13+
{
14+
public TermsListQueryUsageTests(ReadOnlyCluster cluster, EndpointUsage usage) : base(cluster, usage) {}
15+
16+
protected override object QueryJson => new
17+
{
18+
terms = new
19+
{
20+
_name = "named_query",
21+
boost = 1.1,
22+
description = new[] { "term1", "term2" },
23+
disable_coord = true,
24+
minimum_should_match = 2
25+
}
26+
};
27+
28+
protected override QueryContainer QueryInitializer => new TermsQuery
29+
{
30+
Name = "named_query",
31+
Boost = 1.1,
32+
Field = "description",
33+
Terms = new List<string> { "term1", "term2" },
34+
DisableCoord = true,
35+
MinimumShouldMatch = 2
36+
};
37+
38+
protected override QueryContainer QueryFluent(QueryContainerDescriptor<Project> q) => q
39+
.Terms(c => c
40+
.Name("named_query")
41+
.Boost(1.1)
42+
.Field(p => p.Description)
43+
.DisableCoord()
44+
.MinimumShouldMatch(MinimumShouldMatch.Fixed(2))
45+
.Terms(new List<string> { "term1", "term2" })
46+
);
47+
}
48+
49+
public class TermsListOfListIntegrationTests : QueryDslIntegrationTestsBase
50+
{
51+
public TermsListOfListIntegrationTests(ReadOnlyCluster cluster, EndpointUsage usage) : base(cluster, usage) { }
52+
53+
private List<List<string>> _terms = new List<List<string>> { new List<string> { "term1", "term2" } };
54+
55+
protected override object QueryJson => new
56+
{
57+
terms = new
58+
{
59+
_name = "named_query",
60+
boost = 1.1,
61+
description = new[] { new [] { "term1", "term2" } },
62+
disable_coord = true,
63+
}
64+
};
65+
66+
protected override QueryContainer QueryInitializer => new TermsQuery
67+
{
68+
Name = "named_query",
69+
Boost = 1.1,
70+
Field = "description",
71+
Terms = _terms,
72+
DisableCoord = true,
73+
};
74+
75+
protected override QueryContainer QueryFluent(QueryContainerDescriptor<Project> q) => q
76+
.Terms(c => c
77+
.Name("named_query")
78+
.Boost(1.1)
79+
.Field(p => p.Description)
80+
.DisableCoord()
81+
.Terms(_terms)
82+
);
83+
84+
}
85+
86+
public class TermsListOfListStringAgainstNumericFieldIntegrationTests : QueryDslIntegrationTestsBase
87+
{
88+
public TermsListOfListStringAgainstNumericFieldIntegrationTests(ReadOnlyCluster cluster, EndpointUsage usage) : base(cluster, usage) { }
89+
90+
private List<List<string>> _terms = new List<List<string>> { new List<string> { "term1", "term2" } };
91+
92+
93+
protected override int ExpectStatusCode => 400;
94+
protected override bool ExpectIsValid => false;
95+
96+
protected override object QueryJson => new
97+
{
98+
terms = new
99+
{
100+
_name = "named_query",
101+
boost = 1.1,
102+
numberOfCommits = new[] { new [] { "term1", "term2" } },
103+
disable_coord = true,
104+
}
105+
};
106+
107+
protected override QueryContainer QueryInitializer => new TermsQuery
108+
{
109+
Name = "named_query",
110+
Boost = 1.1,
111+
Field = "numberOfCommits",
112+
Terms = _terms,
113+
DisableCoord = true,
114+
};
115+
116+
protected override QueryContainer QueryFluent(QueryContainerDescriptor<Project> q) => q
117+
.Terms(c => c
118+
.Name("named_query")
119+
.Boost(1.1)
120+
.Field(p => p.NumberOfCommits)
121+
.DisableCoord()
122+
.Terms(_terms)
123+
);
124+
125+
[I] public Task AsserResponse() => AssertOnAllResponses(r =>
126+
{
127+
r.ServerError.Should().NotBeNull();
128+
r.ServerError.Status.Should().Be(400);
129+
r.ServerError.Error.Should().NotBeNull();
130+
var rootCauses = r.ServerError.Error.RootCause;
131+
rootCauses.Should().NotBeNullOrEmpty();
132+
var rootCause = rootCauses.First();
133+
rootCause.Type.Should().Be("number_format_exception");
134+
});
135+
136+
}
137+
138+
}

‎src/Tests/QueryDsl/TermLevel/Terms/TermsQueryUsageTests.cs

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
using System.Collections.Generic;
12
using System.Linq;
23
using Nest;
34
using Tests.Framework.Integration;
@@ -7,6 +8,8 @@ namespace Tests.QueryDsl.TermLevel.Terms
78
{
89
public class TermsQueryUsageTests : QueryDslUsageTestsBase
910
{
11+
protected virtual string[] ExpectedTerms => new [] { "term1", "term2" };
12+
1013
public TermsQueryUsageTests(ReadOnlyCluster cluster, EndpointUsage usage) : base(cluster, usage) {}
1114

1215
protected override object QueryJson => new
@@ -15,7 +18,7 @@ public TermsQueryUsageTests(ReadOnlyCluster cluster, EndpointUsage usage) : base
1518
{
1619
_name = "named_query",
1720
boost = 1.1,
18-
description = new[] { "term1", "term2" },
21+
description = ExpectedTerms,
1922
disable_coord = true,
2023
minimum_should_match = 2
2124
}
@@ -26,7 +29,7 @@ public TermsQueryUsageTests(ReadOnlyCluster cluster, EndpointUsage usage) : base
2629
Name = "named_query",
2730
Boost = 1.1,
2831
Field = "description",
29-
Terms = new [] { "term1", "term2" },
32+
Terms = ExpectedTerms,
3033
DisableCoord = true,
3134
MinimumShouldMatch = 2
3235
};
@@ -49,4 +52,23 @@ protected override QueryContainer QueryFluent(QueryContainerDescriptor<Project>
4952
q => q.Terms = new [] { "" }
5053
};
5154
}
52-
}
55+
56+
public class SingleTermTermsQueryUsageTests : TermsQueryUsageTests
57+
{
58+
protected override string[] ExpectedTerms => new [] { "term1" };
59+
60+
public SingleTermTermsQueryUsageTests(ReadOnlyCluster cluster, EndpointUsage usage) : base(cluster, usage) { }
61+
62+
protected override QueryContainer QueryFluent(QueryContainerDescriptor<Project> q) => q
63+
.Terms(c => c
64+
.Name("named_query")
65+
.Boost(1.1)
66+
.Field(p => p.Description)
67+
.DisableCoord()
68+
.MinimumShouldMatch(MinimumShouldMatch.Fixed(2))
69+
.Terms("term1")
70+
);
71+
}
72+
73+
74+
}

‎src/Tests/Tests.csproj

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -542,6 +542,8 @@
542542
<Compile Include="Cat\CatRepositories\CatRepositoriesUrlTests.cs" />
543543
<Compile Include="Cat\CatSnapshots\CatSnapshotsApiTests.cs" />
544544
<Compile Include="Cat\CatSnapshots\CatSnapshotsUrlTests.cs" />
545+
<Compile Include="QueryDsl\QueryDslIntegrationTestsBase.cs" />
546+
<Compile Include="QueryDsl\TermLevel\Terms\TermsListQueryUsageTests.cs" />
545547
<Compile Include="Search\Request\ProfileUsageTests.cs" />
546548
<Compile Include="Indices\StatusManagement\ForceMerge\ForceMergeApiTests.cs" />
547549
<Compile Include="Indices\StatusManagement\ForceMerge\ForceMergeUrlTests.cs" />

0 commit comments

Comments
 (0)