Fix filters for complex painless scripted fields (#9171)

* Fix filters for complex painless scripted fields

Painless scripted fields that consisted of more than a single expression
would break when the user attempted to filter on them in Discover or
Visualize because of the naive way we were building them. We now wrap
the user's script in a lambda so that it can be as complex at they need
it to be. The only special exception is that the user cannot define
named functions since those cannot go inside a Painless lambda.

Fixes #9024
Related elastic/elasticsearch#21635

* DRY it up

* Phrase tests

* Only include necessary comparators and add tests

src/ui/public/field_editor/field_editor.html

@@ -147,16 +147,7 @@ <h4>
         </p>
 
         <p>
-          Kibana currently imposes one special limitation on the scripts you write. Your scripts must consist of a single expression. Multi-statement scripts with an explicit return statement will not work.
-          In other words, the above example is a valid script, but something like the following is not.
-        </p>
-
-        <p>
-          <strong>Invalid</strong><br/>
-          <code>
-            def myField = doc['some_field'].value;<br/>
-            return myField;
-          </code>
+          Kibana currently imposes one special limitation on the painless scripts you write. They cannot contain named functions.
         </p>
 
         <p>

src/ui/public/filter_manager/__tests__/filter_manager.js

@@ -5,6 +5,7 @@ import expect from 'expect.js';
 import ngMock from 'ng_mock';
 import FilterManagerProvider from 'ui/filter_manager';
 import FilterBarQueryFilterProvider from 'ui/filter_bar/query_filter';
+import { buildInlineScriptForPhraseFilter } from '../lib/phrase';
 let $rootScope;
 let queryFilter;
 let filterManager;
@@ -120,7 +121,7 @@ describe('Filter Manager', function () {
       meta: {index: 'myIndex', negate: false, field: 'scriptedField'},
       script: {
         script: {
-          inline: '(' + scriptedField.script + ') == params.value',
+          inline: buildInlineScriptForPhraseFilter(scriptedField),
           lang: scriptedField.lang,
           params: {value: 1}
         }

src/ui/public/filter_manager/filter_manager.js

@@ -1,5 +1,7 @@
 import _ from 'lodash';
 import FilterBarQueryFilterProvider from 'ui/filter_bar/query_filter';
+import { buildInlineScriptForPhraseFilter } from './lib/phrase';
+
 // Adds a filter to a passed state
 export default function (Private) {
   let queryFilter = Private(FilterBarQueryFilterProvider);
@@ -54,13 +56,11 @@ export default function (Private) {
           break;
         default:
           if (field.scripted) {
-            // painless expects params.value while groovy and expression languages expect value.
-            const valueClause = field.lang === 'painless' ? 'params.value' : 'value';
             filter = {
               meta: { negate: negate, index: index, field: fieldName },
               script: {
                 script: {
-                  inline: '(' + field.script + ') == ' + valueClause,
+                  inline: buildInlineScriptForPhraseFilter(field),
                   lang: field.lang,
                   params: {
                     value: value

src/ui/public/filter_manager/lib/__tests__/phrase.js

@@ -1,11 +1,13 @@
 
-import fn from 'ui/filter_manager/lib/phrase';
+import { buildInlineScriptForPhraseFilter, default as fn } from 'ui/filter_manager/lib/phrase';
 import expect from 'expect.js';
 import _ from 'lodash';
 import ngMock from 'ng_mock';
 import FixturesStubbedLogstashIndexPatternProvider from 'fixtures/stubbed_logstash_index_pattern';
+
 let indexPattern;
 let expected;
+
 describe('Filter Manager', function () {
   describe('Phrase filter builder', function () {
     beforeEach(ngMock.module('kibana'));
@@ -42,4 +44,30 @@ describe('Filter Manager', function () {
       expect(fn(indexPattern.fields.byName['script number'], 5, indexPattern)).to.eql(expected);
     });
   });
+
+  describe('buildInlineScriptForPhraseFilter', function () {
+
+    it('should wrap painless scripts in a lambda', function () {
+      const field = {
+        lang: 'painless',
+        script: 'return foo;',
+      };
+
+      const expected = `boolean compare(Supplier s, def v) {return s.get() == v;}` +
+                       `compare(() -> { return foo; }, params.value);`;
+
+      expect(buildInlineScriptForPhraseFilter(field)).to.be(expected);
+    });
+
+    it('should create a simple comparison for other langs', function () {
+      const field = {
+        lang: 'expression',
+        script: 'doc[bytes].value',
+      };
+
+      const expected = `(doc[bytes].value) == value`;
+
+      expect(buildInlineScriptForPhraseFilter(field)).to.be(expected);
+    });
+  });
 });

src/ui/public/filter_manager/lib/__tests__/range.js

@@ -43,6 +43,16 @@ describe('Filter Manager', function () {
       expect(fn(indexPattern.fields.byName['script number'], {gte: 1, lte: 3}, indexPattern)).to.eql(expected);
     });
 
+    it('should wrap painless scripts in comparator lambdas', function () {
+      const expected = `boolean gte(Supplier s, def v) {return s.get() >= v} ` +
+              `boolean lte(Supplier s, def v) {return s.get() <= v}` +
+              `gte(() -> { ${indexPattern.fields.byName['script date'].script} }, params.gte) && ` +
+              `lte(() -> { ${indexPattern.fields.byName['script date'].script} }, params.lte)`;
+
+      const inlineScript = fn(indexPattern.fields.byName['script date'], {gte: 1, lte: 3}, indexPattern).script.script.inline;
+      expect(inlineScript).to.be(expected);
+    });
+
     it('should throw an error when gte and gt, or lte and lt are both passed', function () {
       expect(function () {
         fn(indexPattern.fields.byName['script number'], {gte: 1, gt: 3}, indexPattern);

src/ui/public/filter_manager/lib/phrase.js

@@ -3,9 +3,6 @@ export default function buildPhraseFilter(field, value, indexPattern) {
   let filter = { meta: { index: indexPattern.id} };
 
   if (field.scripted) {
-    // painless expects params.value while groovy and expression languages expect value.
-    const valueClause = field.lang === 'painless' ? 'params.value' : 'value';
-
     // See https://github.com/elastic/elasticsearch/issues/20941 and https://github.com/elastic/kibana/issues/8677
     // for the reason behind this change. ES doesn't handle boolean types very well, so they come
     // back as strings.
@@ -17,8 +14,10 @@ export default function buildPhraseFilter(field, value, indexPattern) {
       convertedValue = value === 'true' ? true : false;
     }
 
+    const script = buildInlineScriptForPhraseFilter(field);
+
     _.set(filter, 'script.script', {
-      inline: '(' + field.script + ') == ' + valueClause,
+      inline: script,
       lang: field.lang,
       params: {
         value: convertedValue
@@ -34,3 +33,23 @@ export default function buildPhraseFilter(field, value, indexPattern) {
   }
   return filter;
 };
+
+
+/**
+ * Takes a scripted field and returns an inline script appropriate for use in a script query.
+ * Handles lucene expression and Painless scripts. Other langs aren't guaranteed to generate valid
+ * scripts.
+ *
+ * @param {object} scriptedField A Field object representing a scripted field
+ * @returns {string} The inline script string
+ */
+export function buildInlineScriptForPhraseFilter(scriptedField) {
+  // We must wrap painless scripts in a lambda in case they're more than a simple expression
+  if (scriptedField.lang === 'painless') {
+    return `boolean compare(Supplier s, def v) {return s.get() == v;}` +
+           `compare(() -> { ${scriptedField.script} }, params.value);`;
+  }
+  else {
+    return `(${scriptedField.script}) == value`;
+  }
+}

src/ui/public/filter_manager/lib/range.js

@@ -32,14 +32,29 @@ export default function buildRangeFilter(field, params, indexPattern, formattedV
       lte: '<=',
       lt: '<',
     };
+    const comparators = {
+      gt: 'boolean gt(Supplier s, def v) {return s.get() > v}',
+      gte: 'boolean gte(Supplier s, def v) {return s.get() >= v}',
+      lte: 'boolean lte(Supplier s, def v) {return s.get() <= v}',
+      lt: 'boolean lt(Supplier s, def v) {return s.get() < v}',
+    };
 
     const knownParams = _.pick(params, (val, key) => { return key in operators; });
-    const script = _.map(knownParams, function (val, key) {
-      // painless expects params.[key] while groovy and expression languages expect [key] only.
-      const valuePrefix = field.lang === 'painless' ? 'params.' : '';
-      return '(' + field.script + ')' + operators[key] + valuePrefix + key;
+    let script = _.map(knownParams, function (val, key) {
+      return '(' + field.script + ')' + operators[key] + key;
     }).join(' && ');
 
+    // We must wrap painless scripts in a lambda in case they're more than a simple expression
+    if (field.lang === 'painless') {
+      const currentComparators = _.reduce(knownParams, (acc, val, key) => acc.concat(comparators[key]), []).join(' ');
+
+      const comparisons = _.map(knownParams, function (val, key) {
+        return `${key}(() -> { ${field.script} }, params.${key})`;
+      }).join(' && ');
+
+      script = `${currentComparators}${comparisons}`;
+    }
+
     const value = _.map(knownParams, function (val, key) {
       return operators[key] + field.format.convert(val);
     }).join(' ');