Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add SearchStrategyRegistry and defaultSearchStrategy to support existing search behavior, and integrate it with CallClient. #20497

Merged
merged 32 commits into from
Jul 18, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
2bd4962
Add SearchStrategyRegistry and defaultSearchStrategy to support exist…
cjcenizal Jul 5, 2018
1c1b99a
Fix typo.
cjcenizal Jul 5, 2018
a0386ea
Move fetch param logic from CallClient into defaultSearchStrategy.
cjcenizal Jul 6, 2018
ebd3544
Move defaultSearchStrategy configuration into kibana plugin via searc…
cjcenizal Jul 6, 2018
27ba85c
Fix typo in comment.
cjcenizal Jul 6, 2018
8a7e05f
Fix bug where Dashboard Only Mode was broken.
cjcenizal Jul 6, 2018
0a8442b
Don't throw ABORTED.
cjcenizal Jul 9, 2018
9777a60
Add comments and refactor for clarity.
cjcenizal Jul 9, 2018
31925d5
Refactor strategy interface to be more flexible with the arguments it…
cjcenizal Jul 10, 2018
b74224c
Add call-out react directive.
cjcenizal Jul 10, 2018
9d02ff2
Show error in Discover if user tries to access a rollup index pattern…
cjcenizal Jul 10, 2018
d48f65b
Improve logic for resolving an early response if all searchRequests h…
cjcenizal Jul 11, 2018
bcce546
Merge branch 'master' of github.com:elastic/kibana into search-strate…
cjcenizal Jul 12, 2018
5c7666c
Fix bugs in callClient using the tests as a guide.
cjcenizal Jul 12, 2018
6b97760
Fix bug caused by typo in defaultSearchStrategy.
cjcenizal Jul 13, 2018
3964b3a
Improve appearance of Discover when an unsupported rollup index patte…
cjcenizal Jul 13, 2018
596a8c9
Remove unnecessary if.
cjcenizal Jul 13, 2018
dcddc1c
Return separate searching and abort properties from strategy.search().
cjcenizal Jul 13, 2018
00a8fe5
Simplify isViable to only expect an indexPattern.
cjcenizal Jul 13, 2018
ae033f2
Remove duplicate call to respondToSearchRequests().
cjcenizal Jul 13, 2018
5bd4b01
Send responses in original order to respondToSearchRequests.
cjcenizal Jul 13, 2018
d867a3f
Fix Jest tests.
cjcenizal Jul 13, 2018
714990c
Add tests with multiple searchStrategies and fix errors in logic.
cjcenizal Jul 13, 2018
683a2f3
Merge branch 'master' of github.com:elastic/kibana into search-strate…
cjcenizal Jul 13, 2018
5ad6a55
Rename describe block.
cjcenizal Jul 13, 2018
464b568
Rename describe block.
cjcenizal Jul 13, 2018
6d4fc57
Merge branch 'master' of github.com:elastic/kibana into search-strate…
cjcenizal Jul 16, 2018
4aba1fa
Check for default type index pattern instead of rollup type.
cjcenizal Jul 16, 2018
4c51a51
CallClient now expects search to resolve with responses array.
cjcenizal Jul 16, 2018
1c4e668
Fix tests.
cjcenizal Jul 17, 2018
ff73efb
Tweak copy.
cjcenizal Jul 17, 2018
e6379d8
Merge branch 'master' of github.com:elastic/kibana into search-strate…
cjcenizal Jul 18, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

references to rollup message can be removed with implementation of renderUnsupportedIndexPatternMessage or similar

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