Merge pull request #2021 from elastic/fix/verbatim-queries

Verbatim and strict query fixes

Author: gmarz

Diff for: src/Nest/Aggregations/Bucket/Filter/FilterAggregation.cs

@@ -22,15 +22,15 @@ public FilterAggregation(string name) : base(name) { }
 		internal override void WrapInContainer(AggregationContainer c) => c.Filter = this;
 	}
 
-	public class FilterAggregationDescriptor<T> 
-		: BucketAggregationDescriptorBase<FilterAggregationDescriptor<T>,IFilterAggregation, T> 
-			, IFilterAggregation 
+	public class FilterAggregationDescriptor<T>
+		: BucketAggregationDescriptorBase<FilterAggregationDescriptor<T>,IFilterAggregation, T>
+			, IFilterAggregation
 		where T : class
 	{
 		QueryContainer IFilterAggregation.Filter { get; set; }
 
 		public FilterAggregationDescriptor<T> Filter(Func<QueryContainerDescriptor<T>, QueryContainer> selector) =>
-			Assign(a=> a.Filter = selector?.InvokeQuery(new QueryContainerDescriptor<T>()));
+			Assign(a=> a.Filter = selector?.Invoke(new QueryContainerDescriptor<T>()));
 
 	}
-}
+}

Diff for: src/Nest/Aggregations/Bucket/Filters/FiltersAggregation.cs

@@ -16,16 +16,16 @@ public interface IFiltersAggregation : IBucketAggregation
 		bool? OtherBucket { get; set; }
 
 		[JsonProperty("other_bucket_key")]
-		string OtherBucketKey { get; set; } 
+		string OtherBucketKey { get; set; }
 	}
 
 	public class FiltersAggregation : BucketAggregationBase, IFiltersAggregation
 	{
 		public Union<INamedFiltersContainer, List<QueryContainer>> Filters { get; set; }
 
 		/// <summary>
-		/// Gets or sets whether to add a bucket to the response which will contain all documents 
-		/// that do not match any of the given filters. 
+		/// Gets or sets whether to add a bucket to the response which will contain all documents
+		/// that do not match any of the given filters.
 		/// When set to <c>true</c>, the other bucket will be returned either in a bucket
 		/// (named "_other_" by default) if named filters are being used,
 		///  or as the last bucket if anonymous filters are being used
@@ -35,7 +35,7 @@ public class FiltersAggregation : BucketAggregationBase, IFiltersAggregation
 		public bool? OtherBucket { get; set; }
 
 		/// <summary>
-		/// Gets or sets the key for the other bucket to a value other than the default "_other_". 
+		/// Gets or sets the key for the other bucket to a value other than the default "_other_".
 		/// Setting this parameter will implicitly set the <see cref="OtherBucket"/> parameter to true
 		/// </summary>
 		public string OtherBucketKey { get; set; }
@@ -47,7 +47,7 @@ public FiltersAggregation(string name) : base(name) { }
 		internal override void WrapInContainer(AggregationContainer c) => c.Filters = this;
 	}
 
-	public class FiltersAggregationDescriptor<T> 
+	public class FiltersAggregationDescriptor<T>
 		: BucketAggregationDescriptorBase<FiltersAggregationDescriptor<T>, IFiltersAggregation, T>
 		, IFiltersAggregation
 		where T : class
@@ -59,21 +59,21 @@ public class FiltersAggregationDescriptor<T>
 		string IFiltersAggregation.OtherBucketKey{ get; set; }
 
 		/// <summary>
-		/// Adds a bucket to the response which will contain all documents 
-		/// that do not match any of the given filters. 
+		/// Adds a bucket to the response which will contain all documents
+		/// that do not match any of the given filters.
 		/// When set to <c>true</c>, the other bucket will be returned either in a bucket
 		/// (named "_other_" by default) if named filters are being used,
 		///  or as the last bucket if anonymous filters are being used
 		/// When set to <c>false</c>, does not compute
-		/// the other bucket. 
+		/// the other bucket.
 		/// </summary>
 		/// <param name="otherBucket">whether to set the other bucket</param>
 		/// <returns>the <see cref="FiltersAggregationDescriptor{T}"/></returns>
 		public FiltersAggregationDescriptor<T> OtherBucket(bool otherBucket = true) =>
 			Assign(a => a.OtherBucket = otherBucket);
 
 		/// <summary>
-		/// Sets the key for the other bucket to a value other than the default "_other_". 
+		/// Sets the key for the other bucket to a value other than the default "_other_".
 		/// Setting this parameter will implicitly set the <see cref="OtherBucket"/> parameter to true
 		/// </summary>
 		/// <param name="otherBucketKey">the name for the other bucket</param>
@@ -85,10 +85,10 @@ public FiltersAggregationDescriptor<T> NamedFilters(Func<NamedFiltersContainerDe
 			Assign(a => a.Filters = new Union<INamedFiltersContainer, List<QueryContainer>>(selector?.Invoke(new NamedFiltersContainerDescriptor<T>())?.Value));
 
 		public FiltersAggregationDescriptor<T> AnonymousFilters(params Func<QueryContainerDescriptor<T>, QueryContainer>[] selectors) =>
-			Assign(a => a.Filters = selectors.Select(s=>s?.InvokeQuery(new QueryContainerDescriptor<T>())).ToList());
+			Assign(a => a.Filters = selectors.Select(s=>s?.Invoke(new QueryContainerDescriptor<T>())).ToList());
 
 		public FiltersAggregationDescriptor<T> AnonymousFilters(IEnumerable<Func<QueryContainerDescriptor<T>, QueryContainer>> selectors) =>
-			Assign(a => a.Filters = selectors.Select(s=>s?.InvokeQuery(new QueryContainerDescriptor<T>())).ToList());
+			Assign(a => a.Filters = selectors.Select(s=>s?.Invoke(new QueryContainerDescriptor<T>())).ToList());
 
 	}
-}
+}

Diff for: src/Nest/Aggregations/Bucket/Filters/NamedFiltersContainer.cs

@@ -31,11 +31,11 @@ public NamedFiltersContainerDescriptor() : base(new NamedFiltersContainer()) { }
 		public NamedFiltersContainerDescriptor<T> Filter(string name, IQueryContainer filter) => Assign(name, filter);
 
 		public NamedFiltersContainerDescriptor<T> Filter(string name, Func<QueryContainerDescriptor<T>, QueryContainer> selector) =>
-			Assign(name, selector?.InvokeQuery(new QueryContainerDescriptor<T>()));
+			Assign(name, selector?.Invoke(new QueryContainerDescriptor<T>()));
 
 		public NamedFiltersContainerDescriptor<T> Filter<TOther>(string name, Func<QueryContainerDescriptor<TOther>, QueryContainer> selector)
 			where TOther : class =>
-			Assign(name, selector?.InvokeQuery(new QueryContainerDescriptor<TOther>()));
+			Assign(name, selector?.Invoke(new QueryContainerDescriptor<TOther>()));
 
 	}
 

Diff for: src/Nest/Aggregations/Bucket/SignificantTerms/SignificantTermsAggregation.cs

@@ -139,6 +139,6 @@ public SignificantTermsAggregationDescriptor<T> Script(Func<ScriptedHeuristicDes
 			Assign(a => a.Script = scriptSelector?.Invoke(new ScriptedHeuristicDescriptor()));
 
 		public SignificantTermsAggregationDescriptor<T> BackgroundFilter(Func<QueryContainerDescriptor<T>, QueryContainer> selector) =>
-			Assign(a => a.BackgroundFilter = selector?.InvokeQuery(new QueryContainerDescriptor<T>()));
+			Assign(a => a.BackgroundFilter = selector?.Invoke(new QueryContainerDescriptor<T>()));
 	}
 }

Diff for: src/Nest/CommonAbstractions/Extensions/Extensions.cs

@@ -23,26 +23,6 @@ internal static TReturn InvokeOrDefault<T1, T2, TReturn>(this Func<T1, T2, TRetu
 			where T1 : class, TReturn where TReturn : class =>
 			func?.Invoke(@default, param2) ?? @default;
 
-		internal static QueryContainer InvokeQuery<T>(
-			this Func<QueryContainerDescriptor<T>, QueryContainer> f,
-			QueryContainerDescriptor<T> container)
-			where T : class
-		{
-			var c = f.Invoke(container);
-			//if query is not conditionless or is verbatim: return a container that holds the query
-			if (c != null && (!c.IsConditionless || c.IsVerbatim))
-				return c;
-
-			//query is conditionless but the container is marked as strict, throw exception
-			if (c != null && c.IsStrict)
-				throw new ArgumentException("Query is conditionless but strict is turned on");
-
-			//query is conditionless return an empty container that can later be rewritten
-			return null;
-		}
-
-		internal static bool IsNullOrConditionless(this QueryContainer c) => c == null || c.IsConditionless;
-
 		internal static string GetStringValue(this Enum enumValue)
 		{
 			var knownEnum = KnownEnums.Resolve(enumValue);

Diff for: src/Nest/Document/Multiple/DeleteByQuery/DeleteByQueryRequest.cs

@@ -31,6 +31,6 @@ public partial class DeleteByQueryDescriptor<T> : IDeleteByQueryRequest<T>
 		public DeleteByQueryDescriptor<T> MatchAll() => Assign(a => a.Query = new QueryContainerDescriptor<T>().MatchAll());
 
 		public DeleteByQueryDescriptor<T> Query(Func<QueryContainerDescriptor<T>, QueryContainer> querySelector) =>
-			Assign(a => a.Query = querySelector?.InvokeQuery(new QueryContainerDescriptor<T>()));
+			Assign(a => a.Query = querySelector?.Invoke(new QueryContainerDescriptor<T>()));
 	}
 }

Diff for: src/Nest/Document/Multiple/Reindex/ReindexRequest.cs

@@ -75,7 +75,7 @@ public ReindexDescriptor(IndexName from, IndexName to)
 		/// A query to optionally limit the documents to use for the reindex operation.  
 		/// </summary>
 		public ReindexDescriptor<T> Query(Func<QueryContainerDescriptor<T>, QueryContainer> querySelector) =>
-			Assign(a => a.Query = querySelector?.InvokeQuery(new QueryContainerDescriptor<T>()));
+			Assign(a => a.Query = querySelector?.Invoke(new QueryContainerDescriptor<T>()));
 
 		/// <summary>
 		/// A query to optionally limit the documents to use for the reindex operation.  

Diff for: src/Nest/Document/Multiple/ReindexOnServer/ReindexSource.cs

@@ -38,7 +38,7 @@ public class ReindexSourceDescriptor : DescriptorBase<ReindexSourceDescriptor, I
 		Types IReindexSource.Type { get; set; }
 
 		public ReindexSourceDescriptor Query<T>(Func<QueryContainerDescriptor<T>, QueryContainer> querySelector) where T : class =>
-			Assign(a => a.Query = querySelector?.InvokeQuery(new QueryContainerDescriptor<T>()));
+			Assign(a => a.Query = querySelector?.Invoke(new QueryContainerDescriptor<T>()));
 
 		public ReindexSourceDescriptor Sort<T>(Func<SortDescriptor<T>, IPromise<IList<ISort>>> selector) where T : class =>
 			Assign(a => a.Sort = selector?.Invoke(new SortDescriptor<T>())?.Value);

Diff for: src/Nest/Document/Multiple/UpdateByQuery/UpdateByQueryRequest.cs

@@ -37,7 +37,7 @@ public partial class UpdateByQueryDescriptor<T> : IUpdateByQueryRequest<T>
 		public UpdateByQueryDescriptor<T> MatchAll() => Assign(a => a.Query = new QueryContainerDescriptor<T>().MatchAll());
 
 		public UpdateByQueryDescriptor<T> Query(Func<QueryContainerDescriptor<T>, QueryContainer> querySelector) =>
-			Assign(a => a.Query = querySelector?.InvokeQuery(new QueryContainerDescriptor<T>()));
+			Assign(a => a.Query = querySelector?.Invoke(new QueryContainerDescriptor<T>()));
 
 		public UpdateByQueryDescriptor<T> Script(string script) => Assign(a => a.Script = (InlineScript)script);
 

Diff for: src/Nest/Indices/AliasManagement/Alias.cs

@@ -38,6 +38,6 @@ public class AliasDescriptor : DescriptorBase<AliasDescriptor, IAlias>, IAlias
 		public AliasDescriptor IndexRouting(string indexRouting) => Assign(a => a.IndexRouting = indexRouting);
 		public AliasDescriptor SearchRouting(string searchRouting) => Assign(a => a.SearchRouting = searchRouting);
 		public AliasDescriptor Filter<T>(Func<QueryContainerDescriptor<T>, QueryContainer> filterSelector) where T : class => 
-			Assign(a => a.Filter = filterSelector?.InvokeQuery(new QueryContainerDescriptor<T>()));
+			Assign(a => a.Filter = filterSelector?.Invoke(new QueryContainerDescriptor<T>()));
 	}
 }

Diff for: src/Nest/Indices/AliasManagement/Alias/Actions/AliasAdd.cs

@@ -63,7 +63,7 @@ public AliasAddDescriptor SearchRouting(string searchRouting)
 		public AliasAddDescriptor Filter<T>(Func<QueryContainerDescriptor<T>, QueryContainer> filterSelector)
 			where T : class
 		{
-			Self.Add.Filter = filterSelector?.InvokeQuery(new QueryContainerDescriptor<T>());
+			Self.Add.Filter = filterSelector?.Invoke(new QueryContainerDescriptor<T>());
 			return this;
 		}
 	}

Diff for: src/Nest/Indices/AliasManagement/PutAlias/PutAliasRequest.cs

@@ -30,6 +30,6 @@ public partial class PutAliasDescriptor
 
 		public PutAliasDescriptor Filter<T>(Func<QueryContainerDescriptor<T>, QueryContainer> filterSelector)
 			where T : class =>
-			Assign(a => a.Filter = filterSelector?.InvokeQuery(new QueryContainerDescriptor<T>()));
+			Assign(a => a.Filter = filterSelector?.Invoke(new QueryContainerDescriptor<T>()));
 	}
 }

Diff for: src/Nest/QueryDsl/Abstractions/Container/IQueryContainer.cs

@@ -15,6 +15,9 @@ public interface IQueryContainer
 		[JsonIgnore]
 		bool IsVerbatim { get; set; }
 
+		[JsonIgnore]
+		bool IsWritable { get; }
+
 		[JsonIgnore]
 		IRawQuery RawQuery { get; set; }
 

Diff for: src/Nest/QueryDsl/Abstractions/Container/QueryContainer-Dsl.cs

@@ -1,10 +1,11 @@
-using System.Linq;
+using System;
+using System.Linq;
 
 namespace Nest
 {
 	internal static class QueryContainerExtensions
 	{
-		public static bool IsConditionless(this QueryContainer q) => q == null || q.IsConditionless;
+		public static bool IsConditionless(this QueryContainer q) => q == null || (q.IsConditionless);
 	}
 
 	public partial class QueryContainer : IQueryContainer, IDescriptor
@@ -15,47 +16,48 @@ public partial class QueryContainer : IQueryContainer, IDescriptor
 		bool IQueryContainer.IsStrict { get; set; }
 		internal bool IsStrict => Self.IsStrict;
 
+		bool IQueryContainer.IsWritable => Self.IsVerbatim || !Self.IsConditionless;
+		internal bool IsWritable => Self.IsWritable;
+
 		bool IQueryContainer.IsVerbatim { get; set; }
 		internal bool IsVerbatim => Self.IsVerbatim;
 
-		public QueryContainer()
-		{
-		}
-	
+		public QueryContainer() { }
+
 		public QueryContainer(QueryBase query) : this()
 		{
 			query?.WrapInContainer(this);
 		}
-	
+
 		public static QueryContainer operator &(QueryContainer leftContainer, QueryContainer rightContainer)
 		{
 			QueryContainer queryContainer;
-			return IfEitherIsEmptyReturnTheOtherOrEmpty(leftContainer, rightContainer, out queryContainer) 
-				? queryContainer 
+			return IfEitherIsEmptyReturnTheOtherOrEmpty(leftContainer, rightContainer, out queryContainer)
+				? queryContainer
 				: leftContainer.CombineAsMust(rightContainer);
 		}
-		
+
 		public static QueryContainer operator |(QueryContainer leftContainer, QueryContainer rightContainer)
 		{
 			QueryContainer queryContainer;
-			return IfEitherIsEmptyReturnTheOtherOrEmpty(leftContainer, rightContainer, out queryContainer) 
-				? queryContainer 
+			return IfEitherIsEmptyReturnTheOtherOrEmpty(leftContainer, rightContainer, out queryContainer)
+				? queryContainer
 				: leftContainer.CombineAsShould(rightContainer);
 		}
 
 		private static bool IfEitherIsEmptyReturnTheOtherOrEmpty(QueryContainer leftContainer, QueryContainer rightContainer, out QueryContainer queryContainer)
 		{
 			var combined = new[] {leftContainer, rightContainer};
-			var any = combined.Any(bf => bf == null || bf.IsConditionless);
-			queryContainer = any ? combined.FirstOrDefault(bf => bf != null && !bf.IsConditionless) : null;
-			return any;
+			var anyEmpty = combined.Any(q => q == null || !q.IsWritable);
+			queryContainer = anyEmpty ? combined.FirstOrDefault(q => q != null && q.IsWritable) : null;
+			return anyEmpty;
 		}
 
-		public static QueryContainer operator !(QueryContainer queryContainer) => queryContainer == null || queryContainer.IsConditionless
+		public static QueryContainer operator !(QueryContainer queryContainer) => queryContainer == null || (!queryContainer.IsWritable)
 			? null
 			: new QueryContainer(new BoolQuery {MustNot = new[] {queryContainer}});
 
-		public static QueryContainer operator +(QueryContainer queryContainer) => queryContainer == null || queryContainer.IsConditionless
+		public static QueryContainer operator +(QueryContainer queryContainer) => queryContainer == null || (!queryContainer.IsWritable)
 			? null
 			: new QueryContainer(new BoolQuery {Filter = new[] {queryContainer}});
 
@@ -69,5 +71,4 @@ public void Accept(IQueryVisitor visitor)
 			new QueryWalker().Walk(this, visitor);
 		}
 	}
-
 }