Improve highlighting by using highlight_query with all_fields enabled (…

…#9671)

* Add all_fields to highlight query to improve highlighting

* Refactor highlighting and move out of _flatten

* Make changes as per @Bargs' requests

* Add documentation about highlightAll setting

* Fix docs typo

* Remove unused function

* Remove unused code

docs/management/advanced-options.asciidoc

@@ -35,6 +35,8 @@ document.
 `discover:sampleSize`:: The number of rows to show in the Discover table.
 `doc_table:highlight`:: Highlight results in Discover and Saved Searches Dashboard. Highlighting makes request slow when
 working on big documents. Set this property to `false` to disable highlighting.
+`doc_table:highlight:all_fields`:: Improves highlighting by using a separate `highlight_query` that uses `all_fields` mode on
+`query_string` queries. Set to `false` if you are using a `default_field` in your index.
 `courier:maxSegmentCount`:: Kibana splits requests in the Discover app into segments to limit the size of requests sent to
 the Elasticsearch cluster. This setting constrains the length of the segment list. Long segment lists can significantly
 increase request processing time.
@@ -86,4 +88,4 @@ Markdown.
 `timelion:default_rows`:: The default number of rows to use on a timelion sheet.
 `timelion:graphite.url`:: [experimental] Used with graphite queries, this it the URL of your host
 `timelion:quandl.key`:: [experimental] Used with quandl queries, this is your API key from www.quandl.com
-`state:storeInSessionStorage`:: [experimental] Kibana tracks UI state in the URL, which can lead to problems when there is a lot of information there and the URL gets very long. Enabling this will store parts of the state in your browser session instead, to keep the URL shorter.
+`state:storeInSessionStorage`:: [experimental] Kibana tracks UI state in the URL, which can lead to problems when there is a lot of information there and the URL gets very long. Enabling this will store parts of the state in your browser session instead, to keep the URL shorter.

src/core_plugins/kibana/public/discover/controllers/discover.js

@@ -13,7 +13,6 @@ import 'ui/courier';
 import 'ui/index_patterns';
 import 'ui/state_management/app_state';
 import 'ui/timefilter';
-import 'ui/highlight/highlight_tags';
 import 'ui/share';
 import VisProvider from 'ui/vis';
 import DocTitleProvider from 'ui/doc_title';
@@ -89,7 +88,7 @@ app.directive('discoverApp', function () {
 });
 
 function discoverController($scope, config, courier, $route, $window, Notifier,
-  AppState, timefilter, Promise, Private, kbnUrl, highlightTags) {
+  AppState, timefilter, Promise, Private, kbnUrl) {
 
   const Vis = Private(VisProvider);
   const docTitle = Private(DocTitleProvider);
@@ -143,7 +142,9 @@ function discoverController($scope, config, courier, $route, $window, Notifier,
   // the actual courier.SearchSource
   $scope.searchSource = savedSearch.searchSource;
   $scope.indexPattern = resolveIndexPatternLoading();
-  $scope.searchSource.set('index', $scope.indexPattern);
+  $scope.searchSource
+  .set('index', $scope.indexPattern)
+  .highlightAll(true);
 
   if (savedSearch.id) {
     docTitle.change(savedSearch.title);
@@ -475,22 +476,12 @@ function discoverController($scope, config, courier, $route, $window, Notifier,
     kbnUrl.change('/discover');
   };
 
-  $scope.updateDataSource = Promise.method(function () {
+  $scope.updateDataSource = Promise.method(function updateDataSource() {
     $scope.searchSource
     .size($scope.opts.sampleSize)
     .sort(getSort($state.sort, $scope.indexPattern))
     .query(!$state.query ? null : $state.query)
     .set('filter', queryFilter.getFilters());
-
-    if (config.get('doc_table:highlight')) {
-      $scope.searchSource.highlight({
-        pre_tags: [highlightTags.pre],
-        post_tags: [highlightTags.post],
-        fields: { '*': {} },
-        require_field_match: false,
-        fragment_size: 2147483647 // Limit of an integer.
-      });
-    }
   });
 
   // TODO: On array fields, negating does not negate the combination, rather all terms

src/ui/public/courier/data_source/_abstract.js

@@ -8,12 +8,14 @@ import ErrorHandlersProvider from '../_error_handlers';
 import FetchProvider from '../fetch';
 import DecorateQueryProvider from './_decorate_query';
 import FieldWildcardProvider from '../../field_wildcard';
+import { getHighlightRequestProvider } from '../../highlight';
 
 export default function SourceAbstractFactory(Private, Promise, PromiseEmitter) {
   const requestQueue = Private(RequestQueueProvider);
   const errorHandlers = Private(ErrorHandlersProvider);
   const courierFetch = Private(FetchProvider);
   const { fieldWildcardFilter } = Private(FieldWildcardProvider);
+  const getHighlightRequest = Private(getHighlightRequestProvider);
 
   function SourceAbstract(initialState, strategy) {
     const self = this;
@@ -371,6 +373,13 @@ export default function SourceAbstractFactory(Private, Promise, PromiseEmitter)
           delete flatState.filters;
         }
 
+        if (flatState.highlightAll != null) {
+          if (flatState.highlightAll && flatState.body.query) {
+            flatState.body.highlight = getHighlightRequest(flatState.body.query);
+          }
+          delete flatState.highlightAll;
+        }
+
         // re-write filters within filter aggregations
         (function recurse(aggBranch) {
           if (!aggBranch) return;

src/ui/public/courier/data_source/search_source.js

@@ -93,6 +93,7 @@ export default function SearchSourceFactory(Promise, Private, config) {
     'filter',
     'sort',
     'highlight',
+    'highlightAll',
     'aggs',
     'from',
     'size',
@@ -178,7 +179,6 @@ export default function SearchSourceFactory(Promise, Private, config) {
     });
   };
 
-
   /******
    * PRIVATE APIS
    ******/
@@ -249,6 +249,7 @@ export default function SearchSourceFactory(Promise, Private, config) {
       case 'index':
       case 'type':
       case 'id':
+      case 'highlightAll':
         if (key && state[key] == null) {
           state[key] = val;
         }

src/ui/public/highlight/__tests__/highlight_html.js

@@ -0,0 +1,69 @@
+import expect from 'expect.js';
+import highlightTags from '../highlight_tags';
+import htmlTags from '../html_tags';
+import getHighlightHtml from '../highlight_html';
+
+describe('getHighlightHtml', function () {
+  const text = '' +
+    'Bacon ipsum dolor amet pork loin pork cow pig beef chuck ground round shankle sirloin landjaeger kevin ' +
+    'venison sausage ribeye tongue. Chicken bacon ball tip pork. Brisket pork capicola spare ribs pastrami rump ' +
+    'sirloin, t-bone ham shoulder jerky turducken bresaola. Chicken cow beef picanha. Picanha hamburger alcatra ' +
+    'cupim. Salami capicola boudin pork belly shank picanha.';
+
+  it('should not modify text if highlight is empty', function () {
+    expect(getHighlightHtml(text, undefined)).to.be(text);
+    expect(getHighlightHtml(text, null)).to.be(text);
+    expect(getHighlightHtml(text, [])).to.be(text);
+  });
+
+  it('should preserve escaped text', function () {
+    const highlights = ['<foo>'];
+    const result = getHighlightHtml('&lt;foo&gt;', highlights);
+    expect(result.indexOf('<foo>')).to.be(-1);
+    expect(result.indexOf('&lt;foo&gt;')).to.be.greaterThan(-1);
+  });
+
+  it('should highlight a single result', function () {
+    const highlights = [
+      highlightTags.pre + 'hamburger' + highlightTags.post + ' alcatra cupim. Salami capicola boudin pork belly shank picanha.'
+    ];
+    const result = getHighlightHtml(text, highlights);
+    expect(result.indexOf(htmlTags.pre + 'hamburger' + htmlTags.post)).to.be.greaterThan(-1);
+    expect(result.split(htmlTags.pre + 'hamburger' + htmlTags.post).length).to.be(text.split('hamburger').length);
+  });
+
+  it('should highlight multiple results', function () {
+    const highlights = [
+      'kevin venison sausage ribeye tongue. ' + highlightTags.pre + 'Chicken' + highlightTags.post + ' bacon ball tip pork. Brisket ' +
+      'pork capicola spare ribs pastrami rump sirloin, t-bone ham shoulder jerky turducken bresaola. ' + highlightTags.pre +
+      'Chicken' + highlightTags.post + ' cow beef picanha. Picanha'
+    ];
+    const result = getHighlightHtml(text, highlights);
+    expect(result.indexOf(htmlTags.pre + 'Chicken' + htmlTags.post)).to.be.greaterThan(-1);
+    expect(result.split(htmlTags.pre + 'Chicken' + htmlTags.post).length).to.be(text.split('Chicken').length);
+  });
+
+  it('should highlight multiple hits in a result', function () {
+    const highlights = [
+      'Bacon ipsum dolor amet ' + highlightTags.pre + 'pork' + highlightTags.post + ' loin ' +
+        '' + highlightTags.pre + 'pork' + highlightTags.post + ' cow pig beef chuck ground round shankle ' +
+        'sirloin landjaeger',
+      'kevin venison sausage ribeye tongue. Chicken bacon ball tip ' +
+        '' + highlightTags.pre + 'pork' + highlightTags.post + '. Brisket ' +
+        '' + highlightTags.pre + 'pork' + highlightTags.post + ' capicola spare ribs',
+      'hamburger alcatra cupim. Salami capicola boudin ' + highlightTags.pre + 'pork' + highlightTags.post + ' ' +
+        'belly shank picanha.'
+    ];
+    const result = getHighlightHtml(text, highlights);
+    expect(result.indexOf(htmlTags.pre + 'pork' + htmlTags.post)).to.be.greaterThan(-1);
+    expect(result.split(htmlTags.pre + 'pork' + htmlTags.post).length).to.be(text.split('pork').length);
+  });
+
+  it('should accept an object and return a string containing its properties', function () {
+    const obj = { foo: 1, bar: 2 };
+    const result = getHighlightHtml(obj, null);
+    expect(result.indexOf('' + obj)).to.be(-1);
+    expect(result.indexOf('foo')).to.be.greaterThan(-1);
+    expect(result.indexOf('bar')).to.be.greaterThan(-1);
+  });
+});

src/ui/public/highlight/__tests__/highlight_request.js

@@ -0,0 +1,82 @@
+import expect from 'expect.js';
+import ngMock from 'ng_mock';
+import getHighlightRequestProvider from '../highlight_request';
+
+describe('getHighlightRequest', () => {
+  const queryStringQuery = { query_string: { query: 'foo' } };
+  const rangeQuery = { range: { '@timestamp': { gte: 0, lte: 0 } } };
+  const boolQueryWithSingleCondition = {
+    bool: {
+      must: queryStringQuery,
+    }
+  };
+  const boolQueryWithMultipleConditions = {
+    bool: {
+      must: [
+        queryStringQuery,
+        rangeQuery
+      ]
+    }
+  };
+
+  let config;
+  let previousHighlightConfig;
+  let previousAllFieldsConfig;
+  let getHighlightRequest;
+
+  beforeEach(ngMock.module('kibana'));
+  beforeEach(ngMock.inject(function (_config_) {
+    config = _config_;
+    previousHighlightConfig = config.get('doc_table:highlight');
+    previousAllFieldsConfig = config.get('doc_table:highlight:all_fields');
+  }));
+
+  afterEach(() => {
+    config.set('doc_table:highlight', previousHighlightConfig);
+    config.set('doc_table:highlight:all_fields', previousAllFieldsConfig);
+  });
+
+  it('should be a function', () => {
+    getHighlightRequest = getHighlightRequestProvider(config);
+    expect(getHighlightRequest).to.be.a(Function);
+  });
+
+  it('should add the all_fields param with query_string query without modifying original query', () => {
+    getHighlightRequest = getHighlightRequestProvider(config);
+    const request = getHighlightRequest(queryStringQuery);
+    expect(request.fields['*']).to.have.property('highlight_query');
+    expect(request.fields['*'].highlight_query.query_string).to.have.property('all_fields');
+    expect(queryStringQuery.query_string).to.not.have.property('all_fields');
+  });
+
+  it('should add the all_fields param with bool query with single condition without modifying original query', () => {
+    getHighlightRequest = getHighlightRequestProvider(config);
+    const request = getHighlightRequest(boolQueryWithSingleCondition);
+    expect(request.fields['*']).to.have.property('highlight_query');
+    expect(request.fields['*'].highlight_query.bool.must.query_string).to.have.property('all_fields');
+    expect(queryStringQuery.query_string).to.not.have.property('all_fields');
+  });
+
+  it('should add the all_fields param with bool query with multiple conditions without modifying original query', () => {
+    getHighlightRequest = getHighlightRequestProvider(config);
+    const request = getHighlightRequest(boolQueryWithMultipleConditions);
+    expect(request.fields['*']).to.have.property('highlight_query');
+    expect(request.fields['*'].highlight_query.bool.must).to.have.length(boolQueryWithMultipleConditions.bool.must.length);
+    expect(request.fields['*'].highlight_query.bool.must[0].query_string).to.have.property('all_fields');
+    expect(queryStringQuery.query_string).to.not.have.property('all_fields');
+  });
+
+  it('should return undefined if highlighting is turned off', () => {
+    config.set('doc_table:highlight', false);
+    getHighlightRequest = getHighlightRequestProvider(config);
+    const request = getHighlightRequest(queryStringQuery);
+    expect(request).to.be(undefined);
+  });
+
+  it('should not add the highlight_query param if all_fields is turned off', () => {
+    config.set('doc_table:highlight:all_fields', false);
+    getHighlightRequest = getHighlightRequestProvider(config);
+    const request = getHighlightRequest(queryStringQuery);
+    expect(request.fields['*']).to.not.have.property('highlight_query');
+  });
+});

src/ui/public/highlight/highlight.js

@@ -1,31 +1,7 @@
-import 'ui/highlight/highlight_tags';
-import _ from 'lodash';
-import angular from 'angular';
-import uiModules from 'ui/modules';
+import getHighlightHtml from './highlight_html';
+import getHighlightRequestProvider from './highlight_request';
 
-const module = uiModules.get('kibana');
-
-module.filter('highlight', function (highlightTags) {
-  return function (formatted, highlight) {
-    if (typeof formatted === 'object') formatted = angular.toJson(formatted);
-
-    _.each(highlight, function (section) {
-      section = _.escape(section);
-
-      // Strip out the highlight tags to compare against the formatted string
-      const untagged = section
-        .split(highlightTags.pre).join('')
-        .split(highlightTags.post).join('');
-
-      // Replace all highlight tags with proper html tags
-      const tagged = section
-        .split(highlightTags.pre).join('<mark>')
-        .split(highlightTags.post).join('</mark>');
-
-      // Replace all instances of the untagged string with the properly tagged string
-      formatted = formatted.split(untagged).join(tagged);
-    });
-
-    return formatted;
-  };
-});
+export default {
+  getHighlightHtml,
+  getHighlightRequestProvider
+};