[5.1] Fix filters for complex painless scripted fields

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(' ');