Incompatible with ElasticSearch 5.2

[fangel]

The part of the ElasticSearch Search-DSL that we are using in the E.S.-backend was deprecated in 2.0.0-beta, and doesn't work in 5.2 (I haven't checked 5.1, and I think we run 5.0 in production where it works).

So we should update the ElasticSearch DSL-transformation to use a syntax that works from 2.0 and upwards.

We currently use the filtered-syntax, and we should probably move over to the bool-syntax - I've made a few tests that indicate that the bool-syntax seems to support the types of query that we support.

But what are peoples opinion on this? Should we simply change the transformation, which means that pre-2.0 will stop working (and thus should bump the semver-major version of this package), or should we have two different E.S.-backends depending on which version you run?

[christeredvartsen]

I haven't touched this code at all so I'm not sure I follow. Is the DSL-transformation part of this package, or is that some related package with it's own releases?

[fangel]

The transformation is part of this package.

A rough summary is that this package contains a DSL for writing queries in (which is a superset of Mongo's query-syntax), as well as interfaces for how to write a backend. Each backend is responsible for receiving a query in the DSL-language and the performing it in the backend.

This package then also contains a ElasticSearch backend-implementation, and the part that takes the query-DSL and converts it into something E.S. can understand is the DSL-transformation.
(This transformation is the one that gets tested here: https://github.com/imbo/imbo-metadata-search/blob/b2c77728f25088403024570808cb65280c45ec4e/tests/phpunit/ImboMetadataSearchUnitTest/Dsl/Transformations/ElasticSearchDslTest.php)

[christeredvartsen]

Right. I think that when changing the public DSL, if that is needed to fix this issue), the semver-major should be bumped. If there is a way to fix this by only fixing the ES backend, and keeping bc by making it work with 5.0, 5.1 and 5.2, I'd go for it. Can't really see how this would be possible though, but as I said earlier, I'm not to familiar with the inner workings of this component.

[fangel]

The public DSL wouldn't have to change. The only thing that needs to change is how the ES backend translate that into a ES-query.
Right now we translate into a ES query that got deprecated by ES 2.0 - but I think I've found a way to translate it into a query that works identically using the new syntax introduced in 2.0.

Basically, right now we translate something like { "$and": [ { "foo": "bar" }, { "baz": "blargh" } ] } into something like

{ "query": {
   "filtered": { "filter": { "and": [
    { "query": { "match": { "foo": "bar" } } },
    { "query": { "match": { "baz": "blargh" } } }
  ] } }
} }

This is called the filteredsyntax in E.S. - this is what got deprecated in 2.0 (It is also the reason why the search-filtering is currently suboptimal, but that's another story).

Instead we should use something like the bool-syntax which would instead look like

{ "query": {
  "bool": { "must": [
    { "match": { "foo": "bar" } },
    { "match": { "baz": "blargh" } }
  ] }
} }

(The two examples are off-the-top-of-my-head, so there might be some slight syntax differences to this - but it should convey the right point)

So the bc break would be that we increase the minimum version of ES servers we support. Right now we support 1.x -> 5.0, but I would like to change it so that it's 2.0+

[christeredvartsen]

Should this issue be closed as a consequence of #39 being merged?

[fangel]

So far #39 has only been merged to develop. I ran into some troubles testing it on our stage-server, because it unfortunately was running a very old 1.x-version of Elastic too.

My thinking was to wait with merging it into master and releasing a new version until after I could test it on our stage-servers for a slightly bigger real-world test than my laptop. And then after merging to master, close this issue.