Roadmap 3.0

[erichard]

I've started a full rewrite with better terminology and wider elastic support.

It will also include test and documentation.

Check the UPGRADE.md for more information

Features

• [ ] Query/Filter
  • [x] Boolean
  • [x] GeoDistance
  • [x] GeoBoundingBox (thanks to @pionl)
  • [x] GeoShape (thanks to @jaetooledev)
  • [x] MatchPhrasePrefix
  • [x] MatchPhrase
  • [x] Match
  • [x] MultiMatch
  • [x] Nested
  • [x] Prefix
  • [x] Range
  • [x] Term
  • [x] Terms
  • [x] Wildcard
  • [x] Exists (thanks to @wucdbm )
  • [x] QueryString
• [ ] Aggregation
  • [x] Filter
  • [x] Max
  • [x] Min
  • [x] Nested
  • [x] Reverse
  • [x] Terms
  • [x] TopHits
  • [x] Cardinality
  • [x] DateHistogram
  • [x] Histogram (thanks to @pionl)
  • [x] Range (thanks to @pionl)
  • [x] Stat (thanks to @pionl)
  • [x] Sum
  • [x] WidthHistogram (thanks to @pionl)

[wucdbm]

Hey,

Nice work, thank you very much for the library.

I just started using ~v2 and hit a brick wall assuming I could add multiple Filter instances to the query builder, but then noticed this line in QueryBuilder $query['body']['query'] = $filter->build(); Just not the best DX for a novice in Elastic, but hey.

How much work do you think v3 still needs before rolling out? Is there anything I could help with?

PS Not very experienced with Elastic

[erichard]

Hi,

You cannot have multiples queries in Elastic Search, that why the filter build is only done one time. To use multiple filters you need a Boolean Query : https://www.elastic.co/guide/en/elasticsearch/reference/current/query-dsl-bool-query.html. In v2 you can you use Filter::bool() to create one.

I use v3 in production already but I want a better feature coverage before taggind a stable version.

[wucdbm]

Fair enough, I'm also already using it, will be in production in a month or two.

@Query, noticed that after reverting back to the documentation of Elastic again.

New API seems decent, are all query classes supposed to have a constructor with nullable arguments, as well as creatable through the Query factory class?

[erichard]

I'm not sure about the design of the constructor yet. What I'm trying to acheive is to avoid the extras checks in build(), for that we need to make sure all required properties are setted at constructor level.

[wucdbm]

Hm, alright. Then maybe we could make the constructors require all possible arguments, make them private, and create named constructors that describe what you'd like to create. Then keep the setters ofc, as you would want to keep adding filters etc to a BoolQuery for example.

On the other hand, that may not be possible for BoolQuery where you could have arbitrary filters applied, possibly zero, and you still have to do if (!$query->isEmpty()) before setting it @ QueryBuilder

Another approach I can think of is doing an internal check in QueryBuilder, whether a Query is empty. If it is, do pretend it wasn't set at all. This will allow getting rid of those checks altogether, enforcing most queries to require all of their args @ constructor, and for BoolQuery, that would be somewhat special, allowing you to construct it with empty lists.

if (null !== $this->query && !$this->query->isEmpty()) {
            $query['body']['query'] = $this->query->build();
        }

WDYT?

[byt3sage]

@erichard in V3, Query is has functions such as match() which returns a new MatchQuery without constructor arguments. I presume that either these need updating to pass in the arguments (like the term() and terms() function) or, the constructors need removing.

I'd like to go ahead and update all of the query classes so that they can be used both match('testField', 'testvalue') and match()->setField('testField')->setValue('testvalue').

If this is okay I'll crack on and get that change PR'd

[erichard]

So we need to make a decision here. I have postponed this enough 😄

The main goal is to avoid building invalid query. We can do that either by asserting things in the build () methods or by ensuring we cannot build an invalid object. V2 use the first method and the idea of V3 is to get rid of assertion in build().

Besides I think it's a better DX to use a library that prevent you to do bad things.

So from now:

• Every QueryInterface MUST have a constructor with all arguments needed to build a valid query
• All others arguments MUST have a setter.

@jaetooledev If you are up to the task, feel free to rock the place 👍

[byt3sage]

@erichard is there any other easy wins outstanding?

[erichard]

@jaetooledev The next step is to add support for more query

[pionl]

@jaetooledev This is what I was missing. I've some additional filters / aggregaations implemented but did not have time to finalize it. Also was planning to do these changes and send PR if it was "desired". Maybe we can join "efforts" in near future.

[pionl]

What PHP support you are planning?

[erichard]

@pionl 3.x is not released yet so go for php >= 8.0

[pionl]

@erichard Ok, thanks. Anyway, I wanted to experiment with this: https://github.com/rectorphp/rector, this could enable releasing multiple versions of a package downgraded to different PHP versions (main code base in PHP 8.1, then sub-releases for different PHP versions). Would you be interested to try it here?

[erichard]

@pionl I think this bundle is not widely used so no need to support old PHP version. Maybe in the future if someone ask for it. Or maybe you need 7.x for yourself ?

[pionl]

I'm personally using PHP 8.1, I've old project on 7.4 but I'm not the maintainer anymore. It was just an option to give people the ability to use old versions. But I've not tested it if it is usable (like if Enums are converted to class, etc), but I think that would be g8 feature to allow old projects use "modern" libraries that are maintained on type strict code base :) If I have the time and can do a proof of concept if it is usable. At this moment I will convert the code base to PHP 8.0.

Imho:

Just for the code conventions:

• do you want to drop static functions for Filter / Aggregation ? (like Aggregation::filter), personally I do not use it. I would prefer "factory" to be able to unit test it but still make some overhead (constructor and "arguments" of function must be same).
• I'm used to do AbstractAggregation naming instead of Aggregation. Do you want to leave it without Abstract

I've already have this in my codebase while extending base classes.

[pionl]

So I've made the changes but I'm unable to push changes (github is experiences some issues).

Did not upgraded my code to test changes properly, but should be ok - ive checked the test. Would be gr8 to implement more tests (don't have to much time left).

Will send PR when it works.

For now this is my changelog:

- upgrade code to PHP 8.0 with strict types
- reactor with automatic downgrade script
- phpstan 8 + phpcs fixer
- Upgrade queries / aggregations to use strict constructors
- Add GeoBoundingBoxQuery
- Add HistogramAggregation / WidthHistogramAggregation
- Add collapse support
- Improve sub aggregation on aggregations

I've added queries / aggregations that I've implemented and used in my codebase in 2 different projects.

For the sake of conventions I would propose:

• Move Query to Queries namespace
• Move Aggregation to Aggregations namespace

For Aggregation / Query class as a "factory" I would propose 3 solutions:

1. personally I do not use this and I would remove it, it give us more overhead for adding new features
2. change it to non static solution - as a factory. This would enable depedency injection and mockable tests3.

   __construct(QueryBuilder $builder, QueryFactory $queryFactory) {
   $builder->setQuery($queryFactory->bool()->addShould());
   }

3. Use static solution as now.

This could be then tested in the code with simple mocks and no need to test "arrays".

I've tried to upgrade the code to PHP 8.1 and there is little difference (only readonly typehints) so we do not need it now unless we want enums (which I would welcome, I personally hate strings :D ).

It would be great to add enums for sort and other "string" properties to prevent duplicated code and mistakes. rector supports enum -> class / class -> enum converting.

[pionl]

Hi @erichard , any plans on releasing the new version? :) Need help with something?

[erichard]

Hi @pionl , I just released a 3.0.0-beta version

[pionl]

Thank you, I'll update be composer :)

[janaculenova]

Hi guys,

we started use this library for a few months ago. Thanks a lot!

I just wanted to ask... what about things like function_score and weight https://www.elastic.co/guide/en/elasticsearch/reference/8.4/query-dsl-function-score-query.html

I did not find it before. Do you have any solution for this functionality? We implemented QueryInterface and wrote it ourselves. But for some clean up I look around for solution in original library.

Or are you opened to contributions?

[erichard]

@janaculenova We are certainly opened to contribution. Feel free to submit PR and we will glady review them.