#1521: Configure which fields are indexed in the Ditto search index per namespace pattern

[an1310]

Long time coming.

Resolves: #1521

[thjaeckle]

Thanks a lot @an1310 for that awesome PR.
Looking already really good - I did a quick round of comments to what I found.

What is still missing before getting merged:

• an "empty" default configuration in the search.conf file
  • containing e.g. commented out example of how the config for a concrete namespace would look like
• documentation on how to enable the SearchIndexingSignalEnrichmentFacadeProvider, maybe both:
  • in search.conf as commented out alternative to the default DittoCachingSignalEnrichmentFacadeProvider for ditto.extensions.caching-signal-enrichment-facade-provider
  • and as a subsection in the Operating Ditto docs
    • Next to Restricting entity creation
    • Source is here: installation-operating.md
    • Similar description as for the "Restricting entity creation"

And one optional wish from me 😁
I just figured out that the entity-creation configuration already supports adding wildcards.
So maybe we could integrate that in the same way, see:

ditto/policies/enforcement/src/main/java/org/eclipse/ditto/policies/enforcement/config/DefaultCreationRestrictionConfig.java

Lines 37 to 55 in 2a35c6f

	private DefaultCreationRestrictionConfig(final ConfigWithFallback configWithFallback) {
	this.resourceTypes = Set.copyOf(configWithFallback.getStringList(
	CreationRestrictionConfigValues.RESOURCE_TYPES.getConfigPath()
	));
	this.namespacePatterns = compile(List.copyOf(configWithFallback.getStringList(
	CreationRestrictionConfigValues.NAMESPACES.getConfigPath())
	));
	this.authSubjectPatterns = compile(List.copyOf(configWithFallback.getStringList(
	CreationRestrictionConfigValues.AUTH_SUBJECTS.getConfigPath())
	));
	}
	private static List<Pattern> compile(final List<String> patterns) {
	return patterns.stream()
	.map(LikeHelper::convertToRegexSyntax)
	.filter(Objects::nonNull)
	.map(Pattern::compile)
	.toList();
	}

So basically the getNamespace would return a Pattern instead which is created during the config parsing.

An additional benefit of that would be that is configured commonly as the "Restriction entity creation".

What do you think about that?

[thjaeckle]

Feature wise it works really great 👍
I just manually tested and only the defined fields were inserted into the search index, awesome 🏅

[an1310]

Thanks a lot @an1310 for that awesome PR. Looking already really good - I did a quick round of comments to what I found.

What is still missing before getting merged:

• an "empty" default configuration in the search.conf file

  • containing e.g. commented out example of how the config for a concrete namespace would look like

• documentation on how to enable the SearchIndexingSignalEnrichmentFacadeProvider, maybe both:

  • in search.conf as commented out alternative to the default DittoCachingSignalEnrichmentFacadeProvider for ditto.extensions.caching-signal-enrichment-facade-provider

  • and as a subsection in the Operating Ditto docs

    • Next to Restricting entity creation
    • Source is here: installation-operating.md
    • Similar description as for the "Restricting entity creation"

And one optional wish from me 😁 I just figured out that the entity-creation configuration already supports adding wildcards. So maybe we could integrate that in the same way, see:

ditto/policies/enforcement/src/main/java/org/eclipse/ditto/policies/enforcement/config/DefaultCreationRestrictionConfig.java

Lines 37 to 55 in 2a35c6f

	private DefaultCreationRestrictionConfig(final ConfigWithFallback configWithFallback) {
	this.resourceTypes = Set.copyOf(configWithFallback.getStringList(
	CreationRestrictionConfigValues.RESOURCE_TYPES.getConfigPath()
	));
	this.namespacePatterns = compile(List.copyOf(configWithFallback.getStringList(
	CreationRestrictionConfigValues.NAMESPACES.getConfigPath())
	));
	this.authSubjectPatterns = compile(List.copyOf(configWithFallback.getStringList(
	CreationRestrictionConfigValues.AUTH_SUBJECTS.getConfigPath())
	));
	}
	private static List<Pattern> compile(final List<String> patterns) {
	return patterns.stream()
	.map(LikeHelper::convertToRegexSyntax)
	.filter(Objects::nonNull)
	.map(Pattern::compile)
	.toList();
	}

So basically the getNamespace would return a Pattern instead which is created during the config parsing.

An additional benefit of that would be that is configured commonly as the "Restriction entity creation".

What do you think about that?

I think this suggestion is great. If it's okay, can we do this in a separate issue and I can contribute it?

[thjaeckle]

I think this suggestion is great. If it's okay, can we do this in a separate issue and I can contribute it?

I would like to add it to 3.5.0 still - Because otherwise it would again take ~3 months for the next minor release of Ditto.
Do you think you can make it within the next 2 days? :D

Then it would not really matter if new issue and PR or use this one ..

[thjaeckle]

One TODO to check (I can also check - but maybe you are quicker):

• what happens when you search for a field which is not indexed?
  • do you find nothing (I assume)
  • is there an error (there probably could be - and maybe even should be) indicating that this field is not indexed?

[thjaeckle]

@an1310 I did a review commit, hope you don't mind - providing:

• added a cache to SearchIndexingSignalEnrichmentFacade in order to only evaluate "patterns" once for a given namespace
• removed copy&pasted unit tests in SearchIndexingSignalEnrichmentFacadeTest by adding another abstract test class AbstractCachingSignalEnrichmentFacadeTest
• added a unit test testing selection of JsonFieldSelectors based on different namespaces
• minor cleanup and formatting
• enhanced documentation about how to configure the indexed namespaces via system properties

For me, this would now good to be merged 👍

[an1310]

I don't mind in the slightest. Glad to have helped.

[thjaeckle]

@an1310 I also just provided configuration via the Helm chart - so that it is easily enabled and configured via values.yaml

Once the build is green (nowadays GitLab runners seem quite busy), I will merge :)

Thanks a lot for this contribution - I guess not only you will benefit from that configuration.