ChilliCream / graphql-platform

Welcome to the home of the Hot Chocolate GraphQL server for .NET, the Strawberry Shake GraphQL client for .NET and Banana Cake Pop the awesome Monaco based GraphQL IDE.
https://chillicream.com
MIT License
5.26k stars 746 forks source link

`ElasticSearch` attribute #6158

Open PascalSenn opened 1 year ago

PascalSenn commented 1 year ago

Product

Hot Chocolate

Is your feature request related to a problem?

The current implementation of the ElasticSearch attribute and ConfigureElastic method in our codebase is not sufficiently flexible to handle various use cases. It does not allow for the configuration of multiple parameters like "Field", "Boost", and "Keyword" in a unified and structured way.

Currently, we would have separate attributes and methods for different configurations. This approach is a bit cluttered. Given [ElasticSearchFieldName] [ElasticSearchBoost] etc.

For instance, we have the ElasticSearch attribute for defining the "Field" and potentially "Boost" and we have the ConfigureElastic method for code-first configuration. The lack of alignment between these two configuration methods might lead to confusion and misuse.

The solution you'd like

A unified attribute structure for ElasticSearch would make more sense, allowing us to define all the necessary configurations in one place.

For example: [ElasticSearch(Field = "foo", Boost = 12, Keyword = ...)]

This approach would not only make our code cleaner but also enhance its usability by providing a consistent way of defining configurations.

The new ElasticSearch attribute should accept parameters for "Field", "Boost", and "Keyword". The attribute should be designed in a way that it uses the ConfigureElastic method and handles the underlying configurations appropriately.

See https://github.com/ChilliCream/graphql-platform/pull/5639#discussion_r1186913714

Related: https://github.com/ChilliCream/graphql-platform/pull/4998, https://github.com/ChilliCream/graphql-platform/discussions/4883

CC: @A360JMaxxgamer

A360JMaxxgamer commented 1 year ago

I will start to work on this issue today.

A360JMaxxgamer commented 1 year ago

I managed to get the boost field interpreted and forwarded to the search request. I guess we don't need a keyword property. Using the keyword field can also be achieved by setting the attrbiute field to it like;

[ElasticSearch(Field: "somefield.keyword")]
public string SomeProperty { get; set; }

But now I am stuck.

I would like to use the the custom ElasticField attribute to setup the serializer mappings on the elastic client. At the moment it only works by using the PropertyName attribute. That seems a bit wrong because both attributes basically describe the same desired behaviour.

[ElasticSearch("special")]
[PropertyName("special")]
public string Value { get; set; } = string.Empty;

The friendly ai of the neigbourhood told me, that setting up the mapping can also be done via the ConnectionStrings class of the nest client. But that would mean, we might need some kind of client factory which we can tell the mapping somehow.

Basically we would change the IExecutable approach to something else.

Or we could create a source generator which annotates the model class with the PropertyNameAttribute based on the ElasticSearch attribute.

@PascalSenn what are your thoughts? Is there some kind of solution in other integrations? Changing IExecutable would also impact #6159 I guess.