Code that prevents the Map logic to be executed is duplicated in a lot of IndexProviders

[barthamark]

Describe the bug

It's not a bug in the functionality, more like a code styling issue. I found that many IndexProviders have both .When() methods to specify when the mapping logic should be executed while the mapping logic contains the exact same logic using conditions.

Please take a look at this example here: https://github.com/OrchardCMS/OrchardCore/blob/main/src/OrchardCore.Modules/OrchardCore.Alias/Indexes/AliasPartIndex.cs

I see two issues here:

1. Why isn't the !contentItem.Published && !contentItem.Latest logic added to the When() method? See: https://github.com/OrchardCMS/OrchardCore/blob/main/src/OrchardCore.Modules/OrchardCore.Alias/Indexes/AliasPartIndex.cs#L68
2. Why do we have the existence check for the AliasPart in the Map() and When() method? See: https://github.com/OrchardCMS/OrchardCore/blob/main/src/OrchardCore.Modules/OrchardCore.Alias/Indexes/AliasPartIndex.cs#L68

Note that it was only an example; there are a lot of similar occurrences in the codebase.

To Reproduce

N/A

Expected behavior

The When() method should contain all the checks without duplicating them in the Map() method. The Map() logic shouldn't contain such checks.

Screenshots

N/A

[ns8482e]

I guess the condition inside Map is for to delete the already indexed record when state changes?

[jtkech]

Yes, returning null allows to concretely remove rows from the index table, for example for soft deleted items or when a part was removed from the definition, but only if desired as we may need to keep the full history.

[barthamark]

Are you referring to no.1, right? Because it's not applicable to no.2 as the Map() won't be executed at all if the condition in When is false.

[jtkech]

Yes, the when is false if it doesn't have the part, but it is still true if the part was just removed, this by checking before if the part is no more in the definition but still present in the content item elements, in that case the Id is added to the _partRemoved collection that is checked in the when, here it is an optimized way.

https://github.com/OrchardCMS/OrchardCore/blob/dfbd971f564bac4348afe49c6c7761a49df29477/src/OrchardCore.Modules/OrchardCore.Alias/Indexes/AliasPartIndex.cs#L38-L52