Merge pull request #2274 from elastic/fix/aggs-nre

Fix NRE when deserializing a multi bucket aggregate with an empty buc…

Author: gmarz

src/Nest/Aggregations/AggregateJsonConverter.cs

@@ -374,15 +374,14 @@ private IAggregate GetMultiBucketAggregate(JsonReader reader, JsonSerializer ser
 			{
 				reader.Read();
 				var aggs = new Dictionary<string, IAggregate>();
-				do
+				while (reader.TokenType != JsonToken.EndObject)
 				{
 					var name = reader.Value.ToString();
 					reader.Read();
 					var innerAgg = this.ReadAggregate(reader, serializer);
 					aggs.Add(name, innerAgg);
 					reader.Read();
-				} while (reader.TokenType != JsonToken.EndObject);
-
+				}
 				reader.Read();
 				return new FiltersAggregate(aggs);
 			}

src/Tests/Reproduce/AggsDeserialization.cs

@@ -0,0 +1,68 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+using Tests.Framework;
+using Tests.Framework.Integration;
+using Tests.Framework.MockData;
+using Xunit;
+
+using Elasticsearch.Net;
+using Nest;
+using FluentAssertions;
+
+namespace Tests.Reproduce
+{
+	public class AggsDeserialization : IClusterFixture<ReadOnlyCluster>
+	{
+		private readonly ReadOnlyCluster _cluster;
+
+		public AggsDeserialization(ReadOnlyCluster cluster)
+		{
+			_cluster = cluster;
+		}
+
+		[I]
+		public void EmptyBucketsThrowsNullReferenceException()
+		{
+			var client = _cluster.Client;
+
+			var response = client.Search<Project>(s => s
+				.SearchType(SearchType.Count)
+				.Aggregations(a => a
+					.Terms("names", ts => ts
+						.Field(p => p.Name)
+						.Size(0)
+						.Order(TermsOrder.TermAscending)
+						.Aggregations(aa => aa
+							.Filters("filters", fs => fs
+								.NamedFilters(nfs => nfs
+									.Filter("foo", f => f
+										.Term(t => t
+											.Field(p => p.State)
+											.Value(StateOfBeing.Stable)
+										)
+									)
+								)
+								.Aggregations(aaa => aaa
+									.ValueCount("counts", vc => vc
+										.Field(p => p.NumberOfCommits)
+									)
+									.BucketSelector("counts_bucket_filter", bs => bs
+										.BucketsPath(bp => bp
+											.Add("totalCounts", "counts")
+										)
+										.Script("totalCounts < 0") // intentionally filter all documents to cause empty buckets object
+									)
+								)
+							)
+						)
+					)
+				)
+			);
+
+			response.IsValid.Should().BeTrue();
+		}
+	}
+}

src/Tests/Tests.csproj

@@ -589,6 +589,7 @@
     <Compile Include="QueryDsl\BoolDsl\Operators\OrAssignManyManualBoolsTests.cs" />
     <Compile Include="QueryDsl\Span\FieldMasking\SpanFieldMaskingUsageTests.cs" />
     <Compile Include="QueryDsl\Verbatim\VerbatimAndStrictQueryUsageTests.cs" />
+    <Compile Include="Reproduce\AggsDeserialization.cs" />
     <Compile Include="Reproduce\GithubIssue2101.cs" />
     <Compile Include="Reproduce\GithubIssue2152.cs" />
     <Compile Include="Reproduce\GithubIssue2052.cs" />

src/Tests/tests.yaml

@@ -1,5 +1,5 @@
 # mode either u (unit test), i (integration test) or m (mixed mode)
-mode: u
+mode: i
 # the elasticsearch version that should be started
 elasticsearch_version: 2.4.0
 # whether we want to forcefully reseed on the node, if you are starting the tests with a node already running