Skip to content
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

8.0 Feature request Add Fluent helper for DynamicTemplates #8465

Open
niemyjski opened this issue Feb 28, 2025 · 3 comments
Open

8.0 Feature request Add Fluent helper for DynamicTemplates #8465

niemyjski opened this issue Feb 28, 2025 · 3 comments
Labels
8.x Relates to 8.x client version Category: Enhancement

Comments

@niemyjski
Copy link
Contributor

Is your feature request related to a problem? Please describe.
The new list syntax leaves a lot ot be desired and a bit of a pain to upgrade to.

Describe the solution you'd like
.DynamicTemplates(t => t.DynamicTemplate("idx_text", t1 => t1.Match("text*").Mapping(m1 => m1.Text(mp => mp.AddKeywordAndSortFields()))))

Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.

Additional context
Add any other context or screenshots about the feature request here.

@flobernd flobernd added 8.x Relates to 8.x client version Category: Enhancement and removed Category: Feature labels Mar 7, 2025
@flobernd
Copy link
Member

flobernd commented Mar 7, 2025

Hi @niemyjski ,

I guess this is about this construct:

public ICollection<IDictionary<string, DynamicTemplate>>? DynamicTemplates { get; set; }

Specification

I recently made some changes to the related specification (elastic/elasticsearch-specification#3824). This alone does not help here, but I have a strong feeling that the actual type should be SingleKeyDictionary<string, DynamicTemplate>[] which would map to:

public ICollection<KeyValuePair<string, DynamicTemplate>>? DynamicTemplates { get; set; }

... which in theory is the same as a regular Dictionary, with the exception that this explicitly intends to preserve the order of items.

Will do

My plan is to expose this property as just IDictionary and take care about the ordering at serialization layer (e.g. by using OrderedDictionary<K,V>).

For descriptors, this would also be fully transparent to the user as you won't have to care about manually creating a dictionary.

The problem is with the regular object initializer syntax where the IDictionary could lead users to initialize the property using a regular dictionary (which might or might not preserve order of items; depending on the implementation). I have to think about this when I get to that point.

If my above assumtion is correct and we can get away with just IDictionary<string, DynamicTemplate>, the descriptor usage would look similar to:

.DynamicTemplates(templates => templates
    .Add("my_template", template => template.XXX()...)
    .Add("my_second_template", template => template.XXX().YYY()...)

Won't do

Other than that, the dictionary type will stay.

It's basically the same change that has been made for Aggregation where the name was previously pulled in to the object and now is external, in form of the dictionary key.

For the object representation, this means that the DynamicTemplate type itself won't have a property holding its name.

For the usages this means that we have to deal with IDictionary<string, DynamicTemplate> instead of ICollection<DynamicTemplate>.

@niemyjski
Copy link
Contributor Author

That sounds good, just curious why ordering matters?

@flobernd
Copy link
Member

flobernd commented Mar 7, 2025

@niemyjski I was surprised as well, but turns out Elasticsearch will always use the first matching template. That allows to add templates for very specific conditions first while still having more generic fallbacks later in the list.

And we already confirmed that my assumption was indeed correct and I can move forward with implementing this usability improvement 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.x Relates to 8.x client version Category: Enhancement
Projects
None yet
Development

No branches or pull requests

2 participants