sunlightlabs/congress — pull request #473 by konklone (2014-09-08 00:25:08+00:00)
Title: Fix the build by fixing and restructuring custom scoring

This fixes the broken tests that have been bedeviling the API since Travis updated its Elasticsearch version from 1.1 to 1.3.

I was incorrect in my diagnosis in #472 that the issue was enabling dynamic scripting. The custom scoring no longer uses dynamic scripting. Instead, the issue was that Elasticsearch [made its query parsing stricter](https://github.com/elasticsearch/elasticsearch/issues/6433#issuecomment-45358884) in 1.2.x, and the query construction for the `multi_match` query was not correct.

I've fixed it, but I'm not sure I've totally maintained the functionality that was there before. The functional test for custom scoring only checked that the query returned the indexed item, and didn't error out.

* Replaced `use_dis_max: true` with `type: "phrase"`.

Elasticsearch [deprecated](http://www.elasticsearch.org/guide/en/elasticsearch/reference/current/query-dsl-multi-match-query.html#_literal_use_dis_max_literal) `use_dis_max: true` for `multi_match` queries in 1.1.0. You now need to pick a `type` parameter instead. The suggestion in the docs, `type: "best_fields"`, does not enforce a phrase check. This PR selects `type: "phrase"`, under the assumption that the old custom scoring wanted phrase queries.

**If this is not the case**, change this to be `type: "best_fields"`.

* Removed the `fields` array from the `query` object during custom scoring.

Elasticsearch's [tightened query parsing](https://github.com/elasticsearch/elasticsearch/issues/5685) suggests to me that no other keys should be directly underneath the `query` key than the name of the query. And since the `multi_match` query takes its own `fields` array, it seemed superfluous and possibly buggy.

**It's possible it's still not as intended**, and that you intended all of the model's usual fields (such as the `text` of a bill) to be searched during custom scoring, but to only apply the `function_score` query to a subset of them. If so, you'll need to figure out the right place to put those `fields` on the query, because I'm reasonably certain it was incorrect as it was, and wasn't using those `fields` anyway.

* Restructured how an additional `filter` gets treated during custom scoring.

Previously, any additional `filter`, as might be generated by `chamber=house` in the query string, was placed as a sibling to `function_score` beneath the `query` parameter. As described in the prior change, I believe this was incorrect, and just previously ignored.

I've now restructured how additional filters are handled by making the behavior match how a query+filter are handled during queries that do not use custom scoring — a `filtered` query is used that contains a `query` and `filter` key together. This seems to work in empirical testing, though I have not added any further functional tests.

In general, I suggest adding additional functional tests for custom scoring, that verify various additional aspects of what is expected from custom scoring.

Fixes #472.