Skip to content

Commit

Permalink
Address PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
russcam committed Aug 20, 2018
1 parent d6eee91 commit eb62e8a
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 16 deletions.
5 changes: 4 additions & 1 deletion src/Nest/Aggregations/Aggregate.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,15 @@
namespace Nest
{
/// <summary>
/// Represents the result of an aggregation on the response
/// Aggregation response for an aggregation request
/// </summary>
[ExactContractJsonConverter(typeof(AggregateJsonConverter))]
public interface IAggregate
{
//TODO this public set is problematic
/// <summary>
/// Metadata for the aggregation
/// </summary>
IReadOnlyDictionary<string, object> Meta { get; set; }
}
}
4 changes: 2 additions & 2 deletions src/Nest/Aggregations/AggregateDictionary.cs
Original file line number Diff line number Diff line change
Expand Up @@ -179,11 +179,11 @@ public CompositeBucketAggregate Composite(string key)
{
var bucket = this.TryGet<BucketAggregate>(key);
if (bucket == null) return null;
return new CompositeBucketAggregate()
return new CompositeBucketAggregate
{
Buckets = bucket.Items.OfType<CompositeBucket>().ToList(),
Meta = bucket.Meta,
AfterKey = bucket.AfterKey
AfterKey = new CompositeKey(bucket.AfterKey)
};
}

Expand Down
29 changes: 20 additions & 9 deletions src/Nest/Aggregations/Bucket/BucketAggregate.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,32 +23,43 @@ public SingleBucketAggregate(IReadOnlyDictionary<string, IAggregate> aggregation
[Obsolete("Use methods on this instance to access sub aggregations. Will be removed in NEST 7.x")]
public AggregateDictionary Aggregations { get; protected internal set; }

/// <summary>
/// Count of documents in the bucket
/// </summary>
public long DocCount { get; internal set; }
}

/// <summary>
/// Aggregation response for a bucket aggregation
/// </summary>
/// <typeparam name="TBucket"></typeparam>
public class MultiBucketAggregate<TBucket> : IAggregate
where TBucket : IBucket
{
/// <inheritdoc />
public IReadOnlyDictionary<string, object> Meta { get; set; }

/// <summary>
/// The buckets into which results are grouped
/// </summary>
public IReadOnlyCollection<TBucket> Buckets { get; set; } = EmptyReadOnly<TBucket>.Collection;
}

public class CompositeBucketAggregate : IAggregate
/// <summary>
/// Aggregation response of <see cref="CompositeAggregation"/>
/// </summary>
public class CompositeBucketAggregate : MultiBucketAggregate<CompositeBucket>
{
public IReadOnlyDictionary<string, object> Meta { get; set; }

public IReadOnlyCollection<CompositeBucket> Buckets { get; set; } = EmptyReadOnly<CompositeBucket>.Collection;

/// <summary>
/// The after_key is equals to the last bucket returned in the response before any filtering that could be done by Pipeline aggregations.
/// If all buckets are filtered/removed by a pipeline aggregation, the after_key will contain the last bucket before filtering.
/// The composite key of the last bucket returned
/// in the response before any filtering by pipeline aggregations.
/// If all buckets are filtered/removed by pipeline aggregations,
/// <see cref="AfterKey"/> will contain the composite key of the last bucket before filtering.
/// </summary>
/// <remarks> Valid for Elasticsearch 6.3.0+ </remarks>
public IReadOnlyDictionary<string, object> AfterKey { get; set; } = EmptyReadOnly<string, object>.Dictionary;
public CompositeKey AfterKey { get; set; }
}


// Intermediate object used for deserialization
public class BucketAggregate : IAggregate
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,13 @@ protected ApiIntegrationTestBase(TCluster cluster, EndpointUsage usage) : base(c
public override IElasticClient Client => this.Cluster.Client;
protected override TInitializer Initializer => Activator.CreateInstance<TInitializer>();

[I] public virtual async Task ReturnsExpectedStatusCode() =>
[I] public async Task ReturnsExpectedStatusCode() =>
await this.AssertOnAllResponses(r => r.ApiCall.HttpStatusCode.Should().Be(this.ExpectStatusCode));

[I] public virtual async Task ReturnsExpectedIsValid() =>
[I] public async Task ReturnsExpectedIsValid() =>
await this.AssertOnAllResponses(r => r.ShouldHaveExpectedIsValid(this.ExpectIsValid));

[I] public virtual async Task ReturnsExpectedResponse() => await this.AssertOnAllResponses(ExpectResponse);
[I] public async Task ReturnsExpectedResponse() => await this.AssertOnAllResponses(ExpectResponse);

protected override Task AssertOnAllResponses(Action<TResponse> assert)
{
Expand Down
2 changes: 1 addition & 1 deletion src/Tests/Tests/Tests.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
<NoWarn>$(NoWarn);xUnit1013</NoWarn>
</PropertyGroup>
<ItemGroup>
<DotNetCliToolReference Include="dotnet-xunit" Version="2.3.0-beta1-build3642" />
<DotNetCliToolReference Include="dotnet-xunit" Version="2.3.1" />
</ItemGroup>
<ItemGroup>
<ProjectReference Include="..\Tests.Core\Tests.Core.csproj" />
Expand Down

2 comments on commit eb62e8a

@Mpdreamz
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@russcam
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't you be on vacation @Mpdreamz 😃

Please sign in to comment.