Skip to content

Commit

Permalink
Add SearchStrategyRegistry and defaultSearchStrategy to support exis…
Browse files Browse the repository at this point in the history
…ting search behavior, and integrate it with CallClient. (#20497)

* Add SearchStrategyRegistry and defaultSearchStrategy to support existing search behavior, and integrate it with CallClient.
* Move fetch param logic from CallClient into defaultSearchStrategy.
* Move defaultSearchStrategy configuration into kibana plugin via search uiExport to avoid race conditions with other plugins.
* Add call-out react directive.
* Show error in Discover if user tries to access a rollup index pattern without the right search strategy. Sentence-case copy in field chooser.
* Add tests with multiple searchStrategies and fix errors in logic.
  • Loading branch information
cjcenizal committed Jul 18, 2018
1 parent e90f652 commit a36b87a
Show file tree
Hide file tree
Showing 20 changed files with 618 additions and 115 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,10 @@
</div>
</div>

<div class="discover-sidebar-list-header sidebar-list-header">
<h3 class="sidebar-list-header-label" id="selected_fields" tabindex="0">Selected Fields</h3>
<div class="discover-sidebar-list-header sidebar-list-header" ng-if="fields.length">
<h3 class="sidebar-list-header-label" id="selected_fields" tabindex="0">
Selected fields
</h3>
</div>
<ul class="list-unstyled discover-selected-fields" >
<discover-field
Expand All @@ -46,9 +48,9 @@ <h3 class="sidebar-list-header-label" id="selected_fields" tabindex="0">Selected
</discover-field>
</ul>

<div class="sidebar-list-header sidebar-item euiFlexGroup euiFlexGroup--gutterMedium">
<div class="sidebar-list-header sidebar-item euiFlexGroup euiFlexGroup--gutterMedium" ng-if="fields.length">
<h3 class="euiFlexItem sidebar-list-header-label" id="available_fields" tabindex="0">
Available Fields
Available fields
</h3>

<div class="euiFlexItem euiFlexItem--flexGrowZero">
Expand Down Expand Up @@ -128,15 +130,15 @@ <h3 class="euiFlexItem sidebar-list-header-label" id="available_fields" tabindex
<div class="form-group">
<label for="discoverFieldChooserHideMissingFields">
<input id="discoverFieldChooserHideMissingFields" type="checkbox" ng-model="filter.vals.missing">
Hide Missing Fields
Hide missing fields
</label>
</div>
<button
ng-click="filter.reset()"
ng-disabled="!filter.active"
class="kuiButton kuiButton--danger kuiButton--fullWidth"
>
Reset Filters
Reset filters
</button>
</form>
</div>
Expand Down
16 changes: 14 additions & 2 deletions src/core_plugins/kibana/public/discover/controllers/discover.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,12 @@ import 'ui/visualize';
import 'ui/fixed_scroll';
import 'ui/directives/validate_json';
import 'ui/filters/moment';
import 'ui/courier';
import 'ui/index_patterns';
import 'ui/state_management/app_state';
import { timefilter } from 'ui/timefilter';
import 'ui/share';
import 'ui/query_bar';
import { hasSearchStategyForIndexPattern, isDefaultTypeIndexPattern } from 'ui/courier';
import { toastNotifications, getPainlessError } from 'ui/notify';
import { VisProvider } from 'ui/vis';
import { BasicResponseHandlerProvider } from 'ui/vis/response_handlers/basic';
Expand Down Expand Up @@ -195,6 +195,7 @@ function discoverController(
// the actual courier.SearchSource
$scope.searchSource = savedSearch.searchSource;
$scope.indexPattern = resolveIndexPatternLoading();

$scope.searchSource
.setField('index', $scope.indexPattern)
.setField('highlightAll', true)
Expand Down Expand Up @@ -237,7 +238,6 @@ function discoverController(
});
};


const getSharingDataFields = async () => {
const selectedFields = $state.columns;
if (selectedFields.length === 1 && selectedFields[0] === '_source') {
Expand Down Expand Up @@ -777,5 +777,17 @@ function discoverController(
return loadedIndexPattern;
}

// Block the UI from loading if the user has loaded a rollup index pattern but it isn't
// supported.
$scope.isUnsupportedIndexPattern = (
!isDefaultTypeIndexPattern($route.current.locals.ip.loaded)
&& !hasSearchStategyForIndexPattern($route.current.locals.ip.loaded)
);

if ($scope.isUnsupportedIndexPattern) {
$scope.unsupportedIndexPatternType = $route.current.locals.ip.loaded.type;
return;
}

init();
}
9 changes: 9 additions & 0 deletions src/core_plugins/kibana/public/discover/directives/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,17 @@ import {
DiscoverNoResults,
} from './no_results';

import {
DiscoverUnsupportedIndexPattern,
} from './unsupported_index_pattern';

import './timechart';

const app = uiModules.get('apps/discover', ['react']);

app.directive('discoverNoResults', reactDirective => reactDirective(DiscoverNoResults));

app.directive(
'discoverUnsupportedIndexPattern',
reactDirective => reactDirective(DiscoverUnsupportedIndexPattern, ['unsupportedType'])
);
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import React, { Fragment } from 'react';

import {
EuiCallOut,
EuiFlexGroup,
EuiFlexItem,
EuiSpacer,
} from '@elastic/eui';

export const DiscoverUnsupportedIndexPattern = ({ unsupportedType }) => {
// This message makes the assumption that X-Pack will support this type, as is the case with
// rollup index patterns.
const message = `Index patterns based on ${unsupportedType} indices require the` +
` ${unsupportedType} plugin from X-Pack, which is not installed or disabled`;

return (
<Fragment>
<EuiSpacer size="xl" />

<EuiFlexGroup justifyContent="center">
<EuiFlexItem grow={false} className="discoverNoResults">
<EuiCallOut
title={message}
color="danger"
iconType="alert"
/>
</EuiFlexItem>
</EuiFlexGroup>
</Fragment>
);
};
5 changes: 5 additions & 0 deletions src/core_plugins/kibana/public/discover/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,11 @@ <h1 tabindex="0" id="kui_local_breadcrumb" class="kuiLocalBreadcrumb">

<div class="discover-wrapper col-md-10">
<div class="discover-content">
<discover-unsupported-index-pattern
ng-if="isUnsupportedIndexPattern"
unsupported-type="unsupportedIndexPatternType"
></discover-unsupported-index-pattern>

<discover-no-results
ng-show="resultState === 'none'"
top-nav-toggle="kbnTopNav.toggle"
Expand Down
1 change: 1 addition & 0 deletions src/core_plugins/kibana/public/kibana.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import 'uiExports/devTools';
import 'uiExports/docViews';
import 'uiExports/embeddableFactories';
import 'uiExports/inspectorViews';
import 'uiExports/search';

import 'ui/autoload/all';
import './home';
Expand Down
4 changes: 2 additions & 2 deletions src/ui/public/courier/courier.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,12 @@ uiModules.get('kibana/courier').service('courier', ($rootScope, Private) => {
/**
* Fetch the pending requests.
*/
fetch = () => {
fetch() {
fetchSoon.fetchQueued().then(() => {
// Reset the timer using the time that we get this response as the starting point.
searchPoll.resetTimer();
});
};
}
}

return new Courier();
Expand Down
99 changes: 95 additions & 4 deletions src/ui/public/courier/fetch/__tests__/call_client.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import { CallClientProvider } from '../call_client';
import { RequestStatus } from '../req_status';
import { SearchRequestProvider } from '../request';
import { MergeDuplicatesRequestProvider } from '../merge_duplicate_requests';
import { addSearchStrategy } from '../../search_strategy';

describe('callClient', () => {
NoDigestPromises.activateForSuite();
Expand All @@ -42,14 +43,18 @@ describe('callClient', () => {
let esPromiseAbortSpy;

const createSearchRequest = (id, overrides = {}) => {
const { source: overrideSource, ...rest } = overrides;

const source = {
_flatten: () => ({}),
requestIsStopped: () => {},
getField: () => 'indexPattern',
...overrideSource
};

const errorHandler = () => {};

const searchRequest = new SearchRequest({ source, errorHandler, ...overrides });
const searchRequest = new SearchRequest({ source, errorHandler, ...rest });
searchRequest.__testId__ = id;
return searchRequest;
};
Expand Down Expand Up @@ -145,8 +150,11 @@ describe('callClient', () => {
searchRequest.handleFailure = handleFailureSpy;

searchRequests = [ searchRequest ];
await callClient(searchRequests);
sinon.assert.calledWith(handleFailureSpy, 'fake es error');
try {
await callClient(searchRequests);
} catch(e) {
sinon.assert.calledWith(handleFailureSpy, 'fake es error');
}
});
});

Expand Down Expand Up @@ -227,7 +235,7 @@ describe('callClient', () => {
});
});

it(`aborting all searchRequests calls abort() on the promise returned by es.msearch()`, () => {
it(`aborting all searchRequests calls abort() on the promise returned by searchStrategy.search()`, () => {
esRequestDelay = 100;

const searchRequest1 = createSearchRequest();
Expand Down Expand Up @@ -261,4 +269,87 @@ describe('callClient', () => {
});
});
});

describe('searchRequests with multiple searchStrategies map correctly to their responses', () => {
const search = ({ searchRequests }) => {
return {
searching: Promise.resolve(searchRequests.map(searchRequest => searchRequest.__testId__)),
failedSearchRequests: [],
abort: () => {},
};
};

const searchStrategyA = {
id: 'a',
isViable: indexPattern => {
return indexPattern.type === 'a';
},
search,
};

const searchStrategyB = {
id: 'b',
isViable: indexPattern => {
return indexPattern.type === 'b';
},
search,
};

let searchRequestA;
let searchRequestB;
let searchRequestA2;

beforeEach(() => {
addSearchStrategy(searchStrategyA);
addSearchStrategy(searchStrategyB);

searchRequestA = createSearchRequest('a', {
source: {
getField: () => ({ type: 'a' }),
},
});

searchRequestB = createSearchRequest('b', {
source: {
getField: () => ({ type: 'b' }),
},
});

searchRequestA2 = createSearchRequest('a2', {
source: {
getField: () => ({ type: 'a' }),
},
});
});

it('if the searchRequests are reordered by the searchStrategies', () => {
// Add requests in an order which will be reordered by the strategies.
searchRequests = [ searchRequestA, searchRequestB, searchRequestA2 ];
const callingClient = callClient(searchRequests);

return callingClient.then(results => {
expect(results).to.eql(['a', 'b', 'a2']);
});
});

it('if one is aborted after being provided', () => {
// Add requests in an order which will be reordered by the strategies.
searchRequests = [ searchRequestA, searchRequestB, searchRequestA2 ];
const callingClient = callClient(searchRequests);
searchRequestA2.abort();

return callingClient.then(results => {
expect(results).to.eql(['a', 'b', ABORTED]);
});
});

it(`if one is already aborted when it's provided`, () => {
searchRequests = [ searchRequestA, searchRequestB, ABORTED, searchRequestA2 ];
const callingClient = callClient(searchRequests);

return callingClient.then(results => {
expect(results).to.eql(['a', 'b', ABORTED, 'a2']);
});
});
});
});
Loading

0 comments on commit a36b87a

Please sign in to comment.