HSEARCH-4577 @ProjectionConstructor: aggregating multi-valued field/object projections in Set, SortedSet, etc. instead of List

[marko-bekhta]

https://hibernate.atlassian.net/browse/HSEARCH-4577

for now, just to get a CI run 🙈
still need more tests, docs and migration guide udpates

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license
and can be relicensed under the terms of the LGPL v2.1 license in the future at the maintainers' discretion.
For more information on licensing, please check here.

---

[marko-bekhta]

I wonder whether renaming this from multi to container and then adding built-in type references for optional is a good idea ...

[yrodiere]

+1 for container as the default is already "multi", so it feels a bit weird to introduce a "multi".

However, don't we expose "multi" methods in other DSLs, especially in the search DSL? We'd need to be consistent...

[marko-bekhta]

yeah we also have that HighlightProjectionOptionsStep#single() ...
It would be nice if we can just deprecate all that and do something like

.select(
	f -> f.composite()
		.from(
			f.highlight( "anotherString" ),  // default list as it is now
			f.highlight( "anotherString" ).accumulator(ProjectionAccumulator.single() ), // instead of calling  f.highlight( "anotherString" ).single(),
			f.highlight( "anotherString" ).accumulator(ProjectionAccumulator.optional() ), // still single but wraps the string into an optional
			f.highlight( "anotherString" ).accumulator(ProjectionAccumulator.sortedSet() ), // multi but non-default collection type
			f.highlight( "anotherString" ).accumulator(ProjectionAccumulator.sortedSet(customComparator) ), // multi but non-default collection type with custom comparator
			f.highlight( "anotherString" ).accumulator(ProjectionAccumulator.array() ), // multi but non-default collection type
			f.highlight( "anotherString" ).accumulator(CustomUserDefinedThing.smth() ), // whatever the user came up with
		).asList()
)

(well and the same thing in other places: deprecate the multi and give this accumulator as an alternative)

[marko-bekhta]

I couldn't come up with a better name for this one .... so any suggestions are more than welcome 😃

[yrodiere]

Couldn't you just promote ProjectionAccumulator to API, and use ProjectionAccumulator.Provider? It's admittedly a complex interface, but it's what we use under the hood, and if you provide implementations for the main container types, users will only need to implement it very rarely.

Then you could rename ProjectionAccumulator#single() to nullable() and introduce optional().

Well, you'll probably need to create a new interface, and juggle with inheritance and deprecations, but you see what I mean.

[marko-bekhta]

It's admittedly a complex interface

yeah ... that's why I was thinking that since we are using this index-based approach for transformation operations in the accumulator that most implementations would still be backed by a list as a temporary storage option and the only difference would be the R finish(A accumulated) that performs the final transformation. Hence, I thought extracting this method (as convert()) would work for the users with some custom collections and we'd just delegate to in the list-based accumulator.

but let me see how promoting the accumulator to the API would work out 😃

[yrodiere]

Fair enough, either solution has its merits. I'll let you have a look and decide :)

Hence, I thought extracting this method (as convert()) would work for the users with some custom collections and we'd just delegate to in the list-based accumulator.

Note you could also provide a static <V, C> Provider<V, C> simple(Function<List<V>, C> converter) "factory method" to allow simple list-based implementations.

[yrodiere]

Note you could also provide a static <V, C> Provider<V, C> simple(Function<List<V>, C> converter) "factory method" to allow simple list-based implementations.

#4334 😀

EDIT: It could probably be generalized to any type of "delegate" accumulator, in particular to allow single-valued simple accumulators, but... this is probably enough?

[sonarcloud]

Quality Gate passed

Issues
6 New issues
0 Accepted issues

Measures
0 Security Hotspots
80.2% Coverage on New Code
0.8% Duplication on New Code

See analysis details on SonarCloud

[marko-bekhta]

superseded by #4344