Skip to content

[Backport 8.2] Remove inheritance for code-generated types #6614

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
2 changes: 1 addition & 1 deletion src/Elastic.Clients.Elasticsearch/Api/BulkRequest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ namespace Elastic.Clients.Elasticsearch
{
public partial class BulkRequest : IStreamSerializable
{
protected IRequest Self => this;
internal IRequest Self => this;

public BulkOperationsCollection Operations { get; set; }

Expand Down
2 changes: 1 addition & 1 deletion src/Elastic.Clients.Elasticsearch/Api/CountRequest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

namespace Elastic.Clients.Elasticsearch
{
public partial class CountRequest<TDocument> : CountRequest
public sealed partial class CountRequest<TDocument> : CountRequest
{
//protected CountRequest<TDocument> TypedSelf => this;

Expand Down
16 changes: 16 additions & 0 deletions src/Elastic.Clients.Elasticsearch/Common/Aggregations/Aggregate.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Licensed to Elasticsearch B.V under one or more agreements.
// Elasticsearch B.V licenses this file to you under the Apache 2.0 License.
// See the LICENSE file in the project root for more information.

namespace Elastic.Clients.Elasticsearch.Aggregations;

public interface IAggregate { }

/// <summary>
/// Base class for all aggregates.
/// </summary>
public abstract class Aggregate : IAggregate
{
}


Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// Licensed to Elasticsearch B.V under one or more agreements.
// Elasticsearch B.V licenses this file to you under the Apache 2.0 License.
// See the LICENSE file in the project root for more information.

namespace Elastic.Clients.Elasticsearch.Aggregations;

public interface IAggregation
{
string? Name { get; }
}

/// <summary>
/// Base class for all aggregations.
/// </summary>
public abstract class Aggregation : IAggregation
{
public abstract string? Name { get; internal set; }

//always evaluate to false so that each side of && equation is evaluated
public static bool operator false(Aggregation _) => false;

//always evaluate to false so that each side of && equation is evaluated
public static bool operator true(Aggregation _) => false;

public static Aggregation operator &(Aggregation left, Aggregation right) =>
new AggregationCombinator(null, left, right);
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,20 @@ namespace Elastic.Clients.Elasticsearch.Aggregations;
/// <summary>
/// Combines aggregations into a single list of aggregations.
/// </summary>
internal class AggregationCombinator : AggregationBase
internal class AggregationCombinator : Aggregation
{
public AggregationCombinator(string name, AggregationBase left, AggregationBase right) : base(name)
public AggregationCombinator(string name, Aggregation left, Aggregation right)
{
AddAggregation(left);
AddAggregation(right);
Name = name;
}

internal List<AggregationBase> Aggregations { get; } = new List<AggregationBase>();
public override string? Name { get; internal set; }

//internal override void WrapInContainer(AggregationContainer container) { }
internal List<Aggregation> Aggregations { get; } = new List<Aggregation>();

private void AddAggregation(AggregationBase agg)
private void AddAggregation(Aggregation agg)
{
switch (agg)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
namespace Elastic.Clients.Elasticsearch.Aggregations;

/// <summary>
/// Describes aggregations that we would like to execute on Elasticsearch.
/// Describes aggregations to execute as part of a search.
/// </summary>
public sealed class AggregationDictionary : IsADictionaryBase<string, AggregationContainer>
{
Expand All @@ -24,9 +24,9 @@ public AggregationDictionary(Dictionary<string, AggregationContainer> dictionary
public static implicit operator AggregationDictionary(Dictionary<string, AggregationContainer> dictionary) =>
new(dictionary);

public static implicit operator AggregationDictionary(AggregationBase aggregator)
public static implicit operator AggregationDictionary(Aggregation aggregator)
{
AggregationBase b;
Aggregation b;
if (aggregator is AggregationCombinator combinator)
{
var dict = new AggregationDictionary();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,6 @@

namespace Elastic.Clients.Elasticsearch.QueryDsl
{
// TODO: FieldNameQueryConvertor (see FieldNameQueryFormatter)
public interface IFieldNameQuery
{
Field Field { get; set; }
}

public abstract class FieldNameQueryBase : QueryBase, IFieldNameQuery
{
[JsonIgnore]
public Field Field { get; set; }
}

public interface IQuery
{
/// <summary>
Expand Down Expand Up @@ -53,19 +41,19 @@ public interface IQuery
/// <summary>
/// Whether the query should be treated as writable. Used when determining how to combine queries.
/// </summary>
[JsonIgnore]
bool IsWritable { get; }
//[JsonIgnore]
//bool IsWritable { get; }

/// <summary>
/// The name of the query. Allows you to retrieve for each document what part of the query it matched on.
/// </summary>
//string Name { get; set; }
}

public abstract partial class QueryBase : IQuery
public abstract partial class Query : IQuery
{
[JsonIgnore]
public bool IsWritable => throw new NotImplementedException();
//[JsonIgnore]
//public bool IsWritable => throw new NotImplementedException();

////protected abstract bool Conditionless { get; }
//[JsonIgnore]
Expand All @@ -80,10 +68,10 @@ public abstract partial class QueryBase : IQuery
//bool IQuery.Conditionless => Conditionless;

//always evaluate to false so that each side of && equation is evaluated
public static bool operator false(QueryBase a) => false;
public static bool operator false(Query a) => false;

//always evaluate to false so that each side of && equation is evaluated
public static bool operator true(QueryBase a) => false;
public static bool operator true(Query a) => false;

//public static QueryBase operator &(QueryBase leftQuery, QueryBase rightQuery) => Combine(leftQuery, rightQuery, (l, r) => l && r);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace Elastic.Clients.Elasticsearch

internal static class AggregationContainerSerializationHelper
{
public static AggregationContainer ReadContainer<T>(ref Utf8JsonReader reader, JsonSerializerOptions options) where T : AggregationBase
public static AggregationContainer ReadContainer<T>(ref Utf8JsonReader reader, JsonSerializerOptions options) where T : Aggregation
{
var variant = JsonSerializer.Deserialize<T?>(ref reader, options);

Expand All @@ -19,7 +19,7 @@ public static AggregationContainer ReadContainer<T>(ref Utf8JsonReader reader, J
return container;
}

public static AggregationContainer ReadContainer<T>(string variantName, ref Utf8JsonReader reader, JsonSerializerOptions options) where T : AggregationBase
public static AggregationContainer ReadContainer<T>(string variantName, ref Utf8JsonReader reader, JsonSerializerOptions options) where T : Aggregation
{
var variant = JsonSerializer.Deserialize<T?>(ref reader, options);

Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -54,3 +54,7 @@ public override void Write(Utf8JsonWriter writer, TType value, JsonSerializerOpt
}


internal interface IUnionVerifiable
{
bool IsSuccessful { get; }
}
Original file line number Diff line number Diff line change
Expand Up @@ -42,16 +42,16 @@ public static void SerializeParams<T>(T toSerialize, Utf8JsonWriter writer, IEla
JsonSerializer.Serialize(writer, toSerialize, options);
}

public static void DeserializeParams<T>(ref Utf8JsonReader reader, IElasticsearchClientSettings settings)
public static T DeserializeParams<T>(ref Utf8JsonReader reader, IElasticsearchClientSettings settings)
{
if (settings.Experimental.UseSourceSerializerForScriptParameters)
{
Deserialize<T>(ref reader, settings);
return;
var result = Deserialize<T>(ref reader, settings);
return result;
}

_ = settings.RequestResponseSerializer.TryGetJsonSerializerOptions(out var options);
JsonSerializer.Deserialize<T>(ref reader, options);
return JsonSerializer.Deserialize<T>(ref reader, options);
}

public static void Serialize<T>(T toSerialize, Utf8JsonWriter writer, IElasticsearchClientSettings settings) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ internal static class TermsAggregateSerializationHelper
private static readonly byte[] s_key = Encoding.UTF8.GetBytes("key");
private static readonly byte s_period = (byte)'.';

public static bool TryDeserialiseTermsAggregate(ref Utf8JsonReader reader, JsonSerializerOptions options, out AggregateBase? aggregate)
public static bool TryDeserialiseTermsAggregate(ref Utf8JsonReader reader, JsonSerializerOptions options, out IAggregate? aggregate)
{
aggregate = null;

Expand Down
31 changes: 14 additions & 17 deletions src/Elastic.Clients.Elasticsearch/Serialization/UnionConverter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,13 @@ namespace Elastic.Clients.Elasticsearch;

internal sealed class UnionConverter : JsonConverterFactory
{
// Because converters registered on JsonSerializerOptions take priority over the JsonConverter attribute on the type, we need a way to
// mark those types we don't want to use the default union converter. This set is used for that purpose, until a better option can be
// found.
private static readonly HashSet<Type> TypesToSkip = new()
{
typeof(SourceConfig)
typeof(SourceConfig),
typeof(Script)
};

public override bool CanConvert(Type typeToConvert) => !TypesToSkip.Contains(typeToConvert) &&
Expand Down Expand Up @@ -67,27 +71,23 @@ private class DerivedUnionConverterInner<TType, TItem1, TItem2> : JsonConverter<
{
// TODO - Aggregate Exception if both fail

//var requiresEndObject = false;

var readerCopy = reader;

//if (readerCopy.TokenType == JsonTokenType.StartObject)
//{
// requiresEndObject = true;
// readerCopy.Read();
//}

try
{
var itemOne = JsonSerializer.Deserialize<TItem1>(ref readerCopy, options);

if (itemOne is not null)
if (itemOne is IUnionVerifiable verifiable)
{
if (verifiable.IsSuccessful)
{
reader = readerCopy;
return (TType)Activator.CreateInstance(typeof(TType), itemOne);
}
}
else if (itemOne is not null)
{
//if (requiresEndObject)
// readerCopy.Read();

reader = readerCopy;

return (TType)Activator.CreateInstance(typeof(TType), itemOne);
}
}
Expand All @@ -108,9 +108,6 @@ private class DerivedUnionConverterInner<TType, TItem1, TItem2> : JsonConverter<

if (itemTwo is not null)
{
//if (requiresEndObject)
// reader.Read();

return (TType)Activator.CreateInstance(typeof(TType), itemTwo);
}
}
Expand Down
Loading