Reduce the verbosity of the predicate/projection/sort DSLs and index schema DSL

Description

The DSLs in Search 6 are relatively verbose, even when using the lambda syntax:

FullTextEntityManager fullTextEntityManager = Search.getFullTextEntityManager( entityManager );

FullTextQuery<MyProjectionBean> query = fullTextEntityManager.search( Book.class ).query()
                .asProjection( f ->
                                 f.composite(
                                                MyProjectionBean::new,
                                                f.field( "id_stored", Long.class )
                                                f.field( "title", String.class )
                                ).toProjection()
                )
                .predicate( f -> f.match()
                        .onFields( "title", "authors.name" )
                        .matching( "Refactoring: Improving the Design of Existing Code" )
                        .toPredicate()
                )
                .sort( f -> f.byField( "title" ).asc()
                        .then().byField( "subtitle" ).asc()
                        .toSort()
                )
                .build();

List<MyProjectionBean> result = query.getResultList();

We could try to improve on that.
One idea would be to remove the need to call the toPredicate, toProjection, toSort}}‌and {{toIndexFieldType methods even at the top level:

FullTextEntityManager fullTextEntityManager = Search.getFullTextEntityManager( entityManager );

FullTextQuery<MyProjectionBean> query = fullTextEntityManager.search( Book.class ).query()
                .asProjection( f -> f.composite(
                        MyProjectionBean::new,
                        f.field( "id_stored", Long.class )
                        f.field( "title", String.class )
                 ) )
                .predicate( f -> f.match()
                        .onFields( "title", "authors.name" )
                        .matching( "Refactoring: Improving the Design of Existing Code" )
                )
                .sort( f -> f.byField( "title" ).asc()
                        .then().byField( "subtitle" ).asc()
                )
                .build();

List<MyProjectionBean> result = query.getResultList();

That would, however, remove the ability to return a cached SearchPredicate/SearchProjection/SearchSort object from the lambda: the lambda would be expected to return a Search(Predicate/Projection/Sort)TerminalContext.

Maybe we should add a method to the DSL to convert a SearchPredicate/SearchProjection/SearchSort to a Search(Predicate/Projection/Sort)TerminalContext? Something like f.from(searchPredicate)? If we do that, it could make sense to change the naming around SearchPredicate/SearchProjection/SearchSort: we could make it more obvious that these are mainly for re-use, and are not really necessary if you don't cache them. Maybe rename them to ReusablePredicate, or something similar?

Note that the index schema DSL also suffers from unnecessary verbosity:

this.accessor = root.field( "geoPoint_1", f -> f.asGeoPoint().toIndexFieldType() )
      .createAccessor();

With the same solution as above, we would get to something like this:

this.accessor = root.field( "geoPoint_1", f -> f.asGeoPoint() )
      .createAccessor();

We could improve this even further by removing the createAccessor call:

this.accessor = root.field( "geoPoint_1", f -> f.asGeoPoint() );

… but then there would be an inconsistency with object fields, where the‌{{createAccessor}} call is actually necessary, because the user needs a builder to add sub-fields to the object field:

IndexSchemaObjectField objectField = context.getIndexSchemaElement().objectField( "object" );
this.objectFieldAccessor = objectField.createAccessor();
this.subField1Accessor = objectField.field( "subField1", f -> f.asString() );
this.subField2Accessor = objectField.field( "subField2", f -> f.asInteger() );

Maybe we should just acknowledge this inconsistency by considering the objectField object above as a builder? Then it would make more sense: when you use a builder, you expect a final call. Something like this maybe:

IndexSchemaObjectField objectFieldBuilder = context.getIndexSchemaElement().objectFieldBuilder( "object" );
this.subField1Accessor = objectFieldBuilder.field( "subField1", f -> f.asString() );
this.subField2Accessor = objectFieldBuilder.field( "subField2", f -> f.asInteger() );
this.objectFieldAccessor = objectFieldBuilder.build(); // or ".createAccessor()"? It seems odd with a builder...

Also to be taken into account: we may want one day to add "sub-fields" to non-object fields; see HSEARCH-3465.

Linked issues

is followed up by

HSEARCH-3476

Make re-use of predicate/sort/fieldtype easier and safer

Activity

Show:

Yoann Rodière January 30, 2019 at 4:58 PM

Another idea: we could make SearchPredicateTerminalContext extend SearchPredicate, or completely replace it with SearchPredicate.

Essentially, that would mean making the SearchPredicate retrieved from the DSL mutable. But if you think about it, they currently are anyway: nothing stops you from calling methods on the DSL contexts after you retrieved the SearchPredicate, and they will mutate builders that are used by the SearchPredicates.

We could solve that internally by “freezing” the DSL contexts upon first use (and we could take this opportunity to get rid of unnecessary references such as the SearchPredicateFactoryContext).

One advantage is that even the non-lambda syntax would no longer require the use of .toPredicate(); the obvious downside being that it may not always be clear when the user can use a DSL context as a SearchPredicate. Maybe we should leave the .toPredicate() method, just to guide users through auto-completion?

Anyway, I don’t have time to implement this right now, but it’s something to think about… @Guillaume Smet @Fabio Massimo Ercoli maybe we can talk about it during our next Google Hangout?

Yoann Rodière December 21, 2018 at 1:43 PM

Pushing this to the next sprint. I suspect some changes will be required in HSEARCH‌-3421 that would make the Elasticsearch predicate builders reusable, which means using the DSL context objects themselves as re-usable predicates would now be an option.

I’m not sure though, so let’s address that ticket first.

Fixed

Details

Assignee

Yoann Rodière

Reporter

Yoann Rodière

Components

engine

Sprint

None

Fix versions

6.0.0.Alpha3

Priority

Major

Created December 10, 2018 at 7:52 AM

Updated March 21, 2019 at 5:27 PM

Resolved February 5, 2019 at 1:25 PM

Configure