Merge pull request #1823 from elastic/fix/bool-null-querycontainer

Handle collection of QueryContainer where the first item is null

Author: gmarz

src/Nest/QueryDsl/Compound/Bool/BoolQuery.cs

@@ -9,21 +9,43 @@ namespace Nest
 	[JsonObject(MemberSerialization = MemberSerialization.OptIn)]
 	public interface IBoolQuery : IQuery
 	{
-		[JsonProperty("must")]
+		/// <summary>
+		/// The clause(s) that must appear in matching documents
+		/// </summary>
+		[JsonProperty("must", DefaultValueHandling = DefaultValueHandling.Ignore)]
 		IEnumerable<QueryContainer> Must { get; set; }
 
-		[JsonProperty("must_not")]
+		/// <summary>
+		/// The clause (query) must not appear in the matching documents. Note that it is not possible to search on documents that only consists of a must_not clauses.
+		/// </summary>
+		[JsonProperty("must_not", DefaultValueHandling = DefaultValueHandling.Ignore)]
 		IEnumerable<QueryContainer> MustNot { get; set; }
 
-		[JsonProperty("should")]
+		/// <summary>
+		/// The clause (query) should appear in the matching document. A boolean query with no must clauses, one or more should clauses must match a document. 
+		/// The minimum number of should clauses to match can be set using minimum_should_match parameter.
+		/// </summary>
+		[JsonProperty("should", DefaultValueHandling = DefaultValueHandling.Ignore)]
 		IEnumerable<QueryContainer> Should { get; set; }
 
-		[JsonProperty("filter")]
+		/// <summary>
+		/// The clause (query) which is to be used as a filter (in filter context).
+		/// </summary>
+		[JsonProperty("filter", DefaultValueHandling = DefaultValueHandling.Ignore)]
 		IEnumerable<QueryContainer> Filter { get; set; }
 
+		/// <summary>
+		/// Specifies a minimum number of the optional BooleanClauses which must be satisfied.
+		/// </summary>
 		[JsonProperty("minimum_should_match")]
 		MinimumShouldMatch MinimumShouldMatch { get; set; }
 
+		/// <summary>
+		/// Specifies if the coordination factor for the query should be disabled.
+		/// The coordination factor is used to reward documents that contain a higher 
+		/// percentage of the query terms. The more query terms that appear in the document, 
+		/// the greater the chances that the document is a good match for the query.
+		/// </summary>
 		[JsonProperty("disable_coord")]
 		bool? DisableCoord { get; set; }
 
@@ -35,11 +57,38 @@ public class BoolQuery : QueryBase, IBoolQuery
 		internal static bool Locked(IBoolQuery q) => !q.Name.IsNullOrEmpty() || q.Boost.HasValue || q.DisableCoord.HasValue || q.MinimumShouldMatch != null;
 		bool IBoolQuery.Locked => BoolQuery.Locked(this);
 
+		/// <summary>
+		/// The clause(s) that must appear in matching documents
+		/// </summary>
 		public IEnumerable<QueryContainer> Must { get; set; }
+
+		/// <summary>
+		/// The clause (query) must not appear in the matching documents. Note that it is not possible to search on documents that only consists of a must_not clauses.
+		/// </summary>
 		public IEnumerable<QueryContainer> MustNot { get; set; }
+
+		/// <summary>
+		/// The clause (query) should appear in the matching document. A boolean query with no must clauses, one or more should clauses must match a document. 
+		/// The minimum number of should clauses to match can be set using minimum_should_match parameter.
+		/// </summary>
 		public IEnumerable<QueryContainer> Should { get; set; }
+
+		/// <summary>
+		/// The clause (query) which is to be used as a filter (in filter context).
+		/// </summary>
 		public IEnumerable<QueryContainer> Filter { get; set; }
+
+		/// <summary>
+		/// Specifies a minimum number of the optional BooleanClauses which must be satisfied.
+		/// </summary>
 		public MinimumShouldMatch MinimumShouldMatch { get; set; }
+
+		/// <summary>
+		/// Specifies if the coordination factor for the query should be disabled.
+		/// The coordination factor is used to reward documents that contain a higher 
+		/// percentage of the query terms. The more query terms that appear in the document, 
+		/// the greater the chances that the document is a good match for the query.
+		/// </summary>
 		public bool? DisableCoord { get; set; }
 
 		internal override void WrapInContainer(IQueryContainer c) => c.Bool = this;
@@ -50,10 +99,10 @@ internal static bool IsConditionless(IBoolQuery q)
 			if (!q.Must.HasAny() && !q.Should.HasAny() && !q.MustNot.HasAny() && !q.Filter.HasAny())
 				return true;
 
-			var mustNots = q.MustNot.HasAny() && q.MustNot.All(qq => qq.IsConditionless);
-			var shoulds = q.Should.HasAny() && q.Should.All(qq => qq.IsConditionless);
-			var musts = q.Must.HasAny() && q.Must.All(qq => qq.IsConditionless);
-			var filters =  q.Filter.HasAny() && q.Filter.All(qq => qq.IsConditionless);
+			var mustNots = q.MustNot.HasAny() && q.MustNot.All(qq => qq.IsConditionless());
+			var shoulds = q.Should.HasAny() && q.Should.All(qq => qq.IsConditionless());
+			var musts = q.Must.HasAny() && q.Must.All(qq => qq.IsConditionless());
+			var filters =  q.Filter.HasAny() && q.Filter.All(qq => qq.IsConditionless());
 
 			return mustNots && shoulds && musts && filters;
 		}
@@ -64,7 +113,7 @@ public class BoolQueryDescriptor<T>
 		, IBoolQuery where T : class
 	{
 		bool IBoolQuery.Locked => BoolQuery.Locked(this);
-
+		
 		protected override bool Conditionless => BoolQuery.IsConditionless(this);
 		IEnumerable<QueryContainer> IBoolQuery.Must { get; set; }
 		IEnumerable<QueryContainer> IBoolQuery.MustNot { get; set; }
@@ -73,6 +122,13 @@ public class BoolQueryDescriptor<T>
 		MinimumShouldMatch IBoolQuery.MinimumShouldMatch { get; set; }
 		bool? IBoolQuery.DisableCoord { get; set; }
 
+		/// <summary>
+		/// Specifies if the coordination factor for the query should be disabled.
+		/// The coordination factor is used to reward documents that contain a higher 
+		/// percentage of the query terms. The more query terms that appear in the document, 
+		/// the greater the chances that the document is a good match for the query.
+		/// </summary>
+		/// <returns></returns>
 		public BoolQueryDescriptor<T> DisableCoord() => Assign(a => a.DisableCoord = true);
 
 		/// <summary>
@@ -97,7 +153,8 @@ public BoolQueryDescriptor<T> Must(IEnumerable<Func<QueryContainerDescriptor<T>,
 		/// <summary>
 		/// The clause(s) that must appear in matching documents
 		/// </summary>
-		public BoolQueryDescriptor<T> Must(params QueryContainer[] queries) => Assign(a => a.Must = queries.Where(q => !q.IsNullOrConditionless()).ToListOrNullIfEmpty());
+		public BoolQueryDescriptor<T> Must(params QueryContainer[] queries) =>
+			Assign(a => a.Must = queries.Where(q => !q.IsNullOrConditionless()).ToListOrNullIfEmpty());
 
 		/// <summary>
 		/// The clause (query) must not appear in the matching documents. Note that it is not possible to search on documents that only consists of a must_not clauses.
@@ -120,7 +177,8 @@ public BoolQueryDescriptor<T> MustNot(IEnumerable<Func<QueryContainerDescriptor<
 		/// </summary>
 		/// <param name="queries"></param>
 		/// <returns></returns>
-		public BoolQueryDescriptor<T> MustNot(params QueryContainer[] queries) => Assign(a => a.MustNot = queries.Where(q => !q.IsNullOrConditionless()).ToListOrNullIfEmpty());
+		public BoolQueryDescriptor<T> MustNot(params QueryContainer[] queries) =>
+			Assign(a => a.MustNot = queries.Where(q => !q.IsNullOrConditionless()).ToListOrNullIfEmpty());
 
 		/// <summary>
 		/// The clause (query) should appear in the matching document. A boolean query with no must clauses, one or more should clauses must match a document. 
@@ -146,7 +204,8 @@ public BoolQueryDescriptor<T> Should(IEnumerable<Func<QueryContainerDescriptor<T
 		/// </summary>
 		/// <param name="queries"></param>
 		/// <returns></returns>
-		public BoolQueryDescriptor<T> Should(params QueryContainer[] queries) => Assign(a => a.Should = queries.Where(q => !q.IsNullOrConditionless()).ToListOrNullIfEmpty());
+		public BoolQueryDescriptor<T> Should(params QueryContainer[] queries) =>
+			Assign(a => a.Should = queries.Where(q => !q.IsNullOrConditionless()).ToListOrNullIfEmpty());
 
 		/// <summary>
 		/// The clause (query) which is to be used as a filter (in filter context).
@@ -169,6 +228,7 @@ public BoolQueryDescriptor<T> Filter(IEnumerable<Func<QueryContainerDescriptor<T
 		/// </summary>
 		/// <param name="queries"></param>
 		/// <returns></returns>
-		public BoolQueryDescriptor<T> Filter(params QueryContainer[] queries) => Assign(a => a.Filter = queries.Where(q => !q.IsNullOrConditionless()).ToListOrNullIfEmpty());
+		public BoolQueryDescriptor<T> Filter(params QueryContainer[] queries) => 
+			Assign(a => a.Filter = queries.Where(q => !q.IsNullOrConditionless()).ToListOrNullIfEmpty());
 	}
 }

src/Tests/Aggregations/Bucket/Filter/FilterAggregationUsageTests.cs

@@ -95,7 +95,7 @@ public EmptyFilterAggregationUsageTests(ReadOnlyCluster i, EndpointUsage usage)
 			{
 				empty_filter = new
 				{
-					filter = new object()
+					filter = new {}
 				}
 			}
 		};

src/Tests/Search/Search/SearchApiTests.cs

@@ -188,9 +188,9 @@ protected override void ExpectResponse(ISearchResponse<Project> response)
 	}
 
 	[Collection(IntegrationContext.ReadOnly)]
-	public class SearchApiConditionlessQueryContainerTests : SearchApiTests
+	public class SearchApiContainingConditionlessQueryContainerTests : SearchApiTests
 	{
-		public SearchApiConditionlessQueryContainerTests(ReadOnlyCluster cluster, EndpointUsage usage) : base(cluster, usage) { }
+		public SearchApiContainingConditionlessQueryContainerTests(ReadOnlyCluster cluster, EndpointUsage usage) : base(cluster, usage) { }
 
 		protected override object ExpectJson => new
 		{
@@ -211,19 +211,22 @@ public SearchApiConditionlessQueryContainerTests(ReadOnlyCluster cluster, Endpoi
 						m => m.QueryString(qs => qs.Query("query")),
 						m => m.QueryString(qs => qs.Query(string.Empty)),
 						m => m.QueryString(qs => qs.Query(null)),
-						m => new QueryContainer()
+						m => new QueryContainer(),
+						null
 					)
 					.Should(
 						m => m.QueryString(qs => qs.Query("query")),
 						m => m.QueryString(qs => qs.Query(string.Empty)),
 						m => m.QueryString(qs => qs.Query(null)),
-						m => new QueryContainer()
+						m => new QueryContainer(),
+						null
 					)
 					.MustNot(
 						m => m.QueryString(qs => qs.Query("query")),
 						m => m.QueryString(qs => qs.Query(string.Empty)),
 						m => m.QueryString(qs => qs.Query(null)),
-						m => new QueryContainer()
+						m => new QueryContainer(),
+						null
 					)
 				)
 			);
@@ -237,21 +240,24 @@ public SearchApiConditionlessQueryContainerTests(ReadOnlyCluster cluster, Endpoi
 					new QueryStringQuery{ Query = "query" },
 					new QueryStringQuery{ Query = string.Empty },
 					new QueryStringQuery{ Query =  null },
-					new QueryContainer()
+					new QueryContainer(),
+					null
 				},
 				Should = new List<QueryContainer>
 				{
 					new QueryStringQuery{ Query = "query" },
 					new QueryStringQuery{ Query = string.Empty },
 					new QueryStringQuery{ Query =  null },
-					new QueryContainer()
+					new QueryContainer(),
+					null
 				},
 				MustNot = new List<QueryContainer>
 				{
 					new QueryStringQuery{ Query = "query" },
 					new QueryStringQuery{ Query = string.Empty },
 					new QueryStringQuery{ Query =  null },
-					new QueryContainer()
+					new QueryContainer(),
+					null
 				}
 			}
 		};
@@ -261,4 +267,79 @@ protected override void ExpectResponse(ISearchResponse<Project> response)
 			response.IsValid.Should().BeTrue();
 		}
 	}
+
+	[Collection(IntegrationContext.ReadOnly)]
+	public class SearchApiNullQueryContainerTests : SearchApiTests
+	{
+		public SearchApiNullQueryContainerTests(ReadOnlyCluster cluster, EndpointUsage usage) : base(cluster, usage) { }
+
+		protected override object ExpectJson => new {};
+
+		protected override Func<SearchDescriptor<Project>, ISearchRequest> Fluent => s => s
+			.Query(q => q
+				.Bool(b => b
+					.Must((Func<QueryContainerDescriptor<Project>,QueryContainer>)null)
+					.Should((Func<QueryContainerDescriptor<Project>, QueryContainer>)null)
+					.MustNot((Func<QueryContainerDescriptor<Project>, QueryContainer>)null)
+				)
+			);
+
+		protected override SearchRequest<Project> Initializer => new SearchRequest<Project>()
+		{
+			Query = new BoolQuery
+			{
+				Must = null,
+				Should = null,
+				MustNot = null
+			}
+		};
+
+		protected override void ExpectResponse(ISearchResponse<Project> response)
+		{
+			response.IsValid.Should().BeTrue();
+		}
+	}
+
+	[Collection(IntegrationContext.ReadOnly)]
+	public class SearchApiNullQueriesInQueryContainerTests : SearchApiTests
+	{
+		public SearchApiNullQueriesInQueryContainerTests(ReadOnlyCluster cluster, EndpointUsage usage) : base(cluster, usage) { }
+
+		protected override object ExpectJson => new
+		{
+			query = new
+			{
+				@bool = new { }
+			}
+		};
+
+		// There is no *direct equivalent* to a query container collection only with a null querycontainer
+		// since the fluent methods filter them out
+		protected override Func<SearchDescriptor<Project>, ISearchRequest> Fluent => s => s
+			.Query(q => q
+				.Bool(b =>
+				{
+					IBoolQuery bq = b;
+					bq.Must = new QueryContainer[] {null};
+					bq.Should = new QueryContainer[] {null};
+					bq.MustNot = new QueryContainer[] {null};
+					return bq;
+				})
+			);
+
+		protected override SearchRequest<Project> Initializer => new SearchRequest<Project>()
+		{
+			Query = new BoolQuery
+			{
+				Must = new QueryContainer[] { null },
+				Should = new QueryContainer[] { null },
+				MustNot = new QueryContainer[] { null }
+			}
+		};
+
+		protected override void ExpectResponse(ISearchResponse<Project> response)
+		{
+			response.IsValid.Should().BeTrue();
+		}
+	}
 }