OrchardCMS / OrchardCore

Orchard Core is an open-source modular and multi-tenant application framework built with ASP.NET Core, and a content management system (CMS) built on top of that framework.
https://orchardcore.net
BSD 3-Clause "New" or "Revised" License
7.45k stars 2.41k forks source link

Indexing constants simplify #15090

Open giannik opened 10 months ago

giannik commented 10 months ago

Currently the indexing constants when using elastic search use the dot notatation in the name :

image

When creating an index in elasticsearch the mappings are structured in a hierarchy based on the dot notation and not as a flat property as expected.

image

this makes it difficult to read and follow .

I propose to remove the dot notation and use an underscore and make the constant names more shorter as so :

image

@Skrypt @MikeAlhayek any reason not to do this ? Before i submit a PR.

Skrypt commented 10 months ago

Some of these IndexingConstants are based on what ElasticSearch does by default. Changing this means changing the default naming convention of ElasticSearch.

As for the ContentItem it is a question of taste. I prefer how they actually are because they reflect how the shape is built. In the end what you are doing is removing the "Content" part to shorten the name here. If you are changing these, you are also breaking everyone's integrations.

I would strongly advise to not change these.

giannik commented 10 months ago

Some of these IndexingConstants are based on what ElasticSearch does by default. Changing this means changing the default naming convention of ElasticSearch.

By using the dot notation we are inferring that these are nested properties (sub fields) of a composite object when in fact its just a flat field. Also when running queries we view them as fields.

Just think it would be cleaner to treat the mappings as flat objects

https://www.elastic.co/guide/en/elasticsearch/reference/8.11/properties.html

sebastienros commented 9 months ago

Option 1: "this makes it difficult to read and follow"

Option 2: We break everyone's index, queries and settings even for those who don't use elasticsearch

Easy to pick one. Can you think of a solution that would work for everyone? I know @MikeAlhayek was replacing these dots for Azure AI Search.