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

Refactor, deangularize courier/search source #45235

Merged
merged 38 commits into from
Oct 18, 2019
Merged
Show file tree
Hide file tree
Changes from 35 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
9aa6e48
Initial refactor of search source
lukasolson Sep 10, 2019
039c507
Add abort signal to search source fetch and remove cancel queued
lukasolson Sep 10, 2019
fb1f959
Remove usages of Angular Promises
lukasolson Sep 11, 2019
1a40af4
Remove usages of angular "sessionId" service
lukasolson Sep 11, 2019
6331629
Merge branch 'master' into refactorSearchSource
lukasolson Sep 13, 2019
8aa6496
Remove config as dependency
lukasolson Sep 13, 2019
027f6b6
Update deps on config and esShardTimeout
lukasolson Sep 23, 2019
ae21ec8
Remove remaining angular dependencies from SearchSource
lukasolson Sep 24, 2019
ac12f45
Merge branch 'master' into refactorSearchSource
lukasolson Sep 24, 2019
493f882
Fix Karma tests
lukasolson Sep 24, 2019
68e7529
Separate callClient and handleResponse and add tests for each
lukasolson Sep 25, 2019
dcbcded
Merge branch 'master' into refactorSearchSource
lukasolson Sep 25, 2019
ca27f89
Add tests for fetchSoon
lukasolson Sep 25, 2019
9ff10c4
Add back search source test and convert to jest
lukasolson Sep 25, 2019
fa1ad58
Create search strategy registry test
lukasolson Sep 25, 2019
aa87511
Revert empty test
lukasolson Sep 25, 2019
43305ec
Remove filter predicates from search source
lukasolson Sep 26, 2019
6240738
Merge branch 'master' into refactorSearchSource
lukasolson Sep 30, 2019
df0a353
Update typings and throw response errors
lukasolson Oct 2, 2019
5496403
Fix proxy to properly return response from ES
lukasolson Oct 2, 2019
2c23fe5
Merge branch 'master' into refactorSearchSource
lukasolson Oct 3, 2019
41567df
Update jest snapshots
lukasolson Oct 3, 2019
c1e1ddb
Remove unused translations
lukasolson Oct 3, 2019
b3e8d11
Don't pass search request to onRequestStart, create it afterwards
lukasolson Oct 3, 2019
649e6fa
Fix search source & get search params tests
lukasolson Oct 3, 2019
6fb788c
Merge branch 'master' into refactorSearchSource
lukasolson Oct 3, 2019
7c6b7b8
Merge branch 'master' into refactorSearchSource
lukasolson Oct 4, 2019
1a44042
Merge branch 'master' into refactorSearchSource
lukasolson Oct 7, 2019
0d7c97d
Fix issue with angular scope not firing after setting state on vis
lukasolson Oct 8, 2019
54f0a98
Merge branch 'master' into refactorSearchSource
lukasolson Oct 14, 2019
60122b4
Fix tag cloud vis
lukasolson Oct 14, 2019
eca50a9
Fix setting of visConfig to not happen async
lukasolson Oct 14, 2019
04a423c
Remove unused snapshots
lukasolson Oct 14, 2019
e1399bb
Merge branch 'master' into refactorSearchSource
lukasolson Oct 15, 2019
e29142a
Merge branch 'master' into refactorSearchSource
lukasolson Oct 15, 2019
2a6d8e3
Merge branch 'master' into refactorSearchSource
lukasolson Oct 16, 2019
441aa2b
Remove unused reference to IPrivate
lukasolson Oct 16, 2019
3f59890
Merge branch 'master' into refactorSearchSource
lukasolson Oct 17, 2019
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
26 changes: 26 additions & 0 deletions packages/kbn-es-query/src/es_query/__tests__/from_filters.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,32 @@ describe('build query', function () {
expect(result.filter).to.eql(expectedESQueries);
});

it('should remove disabled filters', function () {
const filters = [
{
match_all: {},
meta: { type: 'match_all', negate: true, disabled: true },
},
];

const expectedESQueries = [];

const result = buildQueryFromFilters(filters);

expect(result.must_not).to.eql(expectedESQueries);
});

it('should remove falsy filters', function () {
const filters = [null, undefined];

const expectedESQueries = [];

const result = buildQueryFromFilters(filters);

expect(result.must_not).to.eql(expectedESQueries);
expect(result.must).to.eql(expectedESQueries);
});

it('should place negated filters in the must_not clause', function () {
const filters = [
{
Expand Down
1 change: 1 addition & 0 deletions packages/kbn-es-query/src/es_query/from_filters.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ const cleanFilter = function (filter) {
};

export function buildQueryFromFilters(filters = [], indexPattern, ignoreFilterIfFieldNotInIndex) {
filters = filters.filter(filter => filter && !_.get(filter, ['meta', 'disabled']));
return {
must: [],
filter: filters
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,12 +123,12 @@ class ListControl extends Control {
this.useTimeFilter,
ancestorFilters
);
this.abortController.signal.addEventListener('abort', () => searchSource.cancelQueued());
const abortSignal = this.abortController.signal;

this.lastQuery = query;
let resp;
try {
resp = await searchSource.fetch();
resp = await searchSource.fetch({ abortSignal });
} catch(error) {
// If the fetch was aborted then no need to surface this error in the UI
if (error.name === 'AbortError') return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,11 @@ class RangeControl extends Control {

const aggs = minMaxAgg(indexPattern.fields.getByName(fieldName));
const searchSource = createSearchSource(this.kbnApi, null, indexPattern, aggs, this.useTimeFilter);
this.abortController.signal.addEventListener('abort', () => searchSource.cancelQueued());
const abortSignal = this.abortController.signal;

let resp;
try {
resp = await searchSource.fetch();
resp = await searchSource.fetch({ abortSignal });
} catch(error) {
// If the fetch was aborted then no need to surface this error in the UI
if (error.name === 'AbortError') return;
Expand Down
38 changes: 16 additions & 22 deletions src/legacy/core_plugins/interpreter/public/functions/esaggs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import chrome from 'ui/chrome';
import { TimeRange } from 'src/plugins/data/public';
import { SearchSource } from '../../../../ui/public/courier/search_source';
// @ts-ignore
import { SearchSourceProvider } from '../../../../ui/public/courier/search_source';
import { FilterBarQueryFilterProvider } from '../../../../ui/public/filter_manager/query_filter';

import { buildTabularInspectorData } from '../../../../ui/public/inspector/build_tabular_inspector_data';
Expand Down Expand Up @@ -100,8 +99,8 @@ const handleCourierRequest = async ({
return aggs.toDsl(metricsAtAllLevels);
});

requestSearchSource.onRequestStart((paramSearchSource: SearchSource, searchRequest: unknown) => {
return aggs.onSearchRequestStart(paramSearchSource, searchRequest);
requestSearchSource.onRequestStart((paramSearchSource: SearchSource, options: any) => {
return aggs.onSearchRequestStart(paramSearchSource, options);
});

if (timeRange) {
Expand All @@ -118,7 +117,7 @@ const handleCourierRequest = async ({
const queryHash = calculateObjectHash(reqBody);
// We only need to reexecute the query, if forceFetch was true or the hash of the request body has changed
// since the last request
const shouldQuery = forceFetch || searchSource.lastQuery !== queryHash;
const shouldQuery = forceFetch || (searchSource as any).lastQuery !== queryHash;

if (shouldQuery) {
inspectorAdapters.requests.reset();
Expand All @@ -139,18 +138,13 @@ const handleCourierRequest = async ({
request.stats(getRequestInspectorStats(requestSearchSource));

try {
// Abort any in-progress requests before fetching again
if (abortSignal) {
abortSignal.addEventListener('abort', () => requestSearchSource.cancelQueued());
}

const response = await requestSearchSource.fetch();
const response = await requestSearchSource.fetch({ abortSignal });

searchSource.lastQuery = queryHash;
(searchSource as any).lastQuery = queryHash;

request.stats(getResponseInspectorStats(searchSource, response)).ok({ json: response });

searchSource.rawResponse = response;
(searchSource as any).rawResponse = response;
} catch (e) {
// Log any error during request to the inspector
request.error({ json: e });
Expand All @@ -166,7 +160,7 @@ const handleCourierRequest = async ({
// Note that rawResponse is not deeply cloned here, so downstream applications using courier
// must take care not to mutate it, or it could have unintended side effects, e.g. displaying
// response data incorrectly in the inspector.
let resp = searchSource.rawResponse;
let resp = (searchSource as any).rawResponse;
for (const agg of aggs.aggs) {
if (has(agg, 'type.postFlightRequest')) {
resp = await agg.type.postFlightRequest(
Expand All @@ -180,7 +174,7 @@ const handleCourierRequest = async ({
}
}

searchSource.finalResponse = resp;
(searchSource as any).finalResponse = resp;

const parsedTimeRange = timeRange ? getTime(aggs.indexPattern, timeRange) : null;
const tabifyParams = {
Expand All @@ -191,23 +185,24 @@ const handleCourierRequest = async ({

const tabifyCacheHash = calculateObjectHash({ tabifyAggs: aggs, ...tabifyParams });
// We only need to reexecute tabify, if either we did a new request or some input params to tabify changed
const shouldCalculateNewTabify = shouldQuery || searchSource.lastTabifyHash !== tabifyCacheHash;
const shouldCalculateNewTabify =
shouldQuery || (searchSource as any).lastTabifyHash !== tabifyCacheHash;

if (shouldCalculateNewTabify) {
searchSource.lastTabifyHash = tabifyCacheHash;
searchSource.tabifiedResponse = tabifyAggResponse(
(searchSource as any).lastTabifyHash = tabifyCacheHash;
(searchSource as any).tabifiedResponse = tabifyAggResponse(
aggs,
searchSource.finalResponse,
(searchSource as any).finalResponse,
tabifyParams
);
}

inspectorAdapters.data.setTabularLoader(
() => buildTabularInspectorData(searchSource.tabifiedResponse, queryFilter),
() => buildTabularInspectorData((searchSource as any).tabifiedResponse, queryFilter),
{ returnsFormattedValues: true }
);

return searchSource.tabifiedResponse;
return (searchSource as any).tabifiedResponse;
};

export const esaggs = (): ExpressionFunction<typeof name, Context, Arguments, Return> => ({
Expand Down Expand Up @@ -249,15 +244,14 @@ export const esaggs = (): ExpressionFunction<typeof name, Context, Arguments, Re
const $injector = await chrome.dangerouslyGetActiveInjector();
const Private: Function = $injector.get('Private');
const { indexPatterns } = data.indexPatterns;
const SearchSourceClass = Private(SearchSourceProvider);
const queryFilter = Private(FilterBarQueryFilterProvider);

const aggConfigsState = JSON.parse(args.aggConfigs);
const indexPattern = await indexPatterns.get(args.index);
const aggs = new AggConfigs(indexPattern, aggConfigsState);

// we should move searchSource creation inside courier request handler
const searchSource = new SearchSourceClass();
const searchSource = new SearchSource();
searchSource.setField('index', indexPattern);
searchSource.setField('size', 0);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,16 @@ export const visualization = () => ({
const { visData, visConfig, params } = config;
const visType = config.visType || visConfig.type;
const $injector = await chrome.dangerouslyGetActiveInjector();
const $rootScope = $injector.get('$rootScope') as any;
const Private = $injector.get('Private') as any;
const Vis = Private(VisProvider);

if (handlers.vis) {
// special case in visualize, we need to render first (without executing the expression), for maps to work
if (visConfig) {
handlers.vis.setCurrentState({ type: visType, params: visConfig });
$rootScope.$apply(() => {
handlers.vis.setCurrentState({ type: visType, params: visConfig });
});
}
} else {
handlers.vis = new Vis({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import sinon from 'sinon';
import moment from 'moment';
import { SearchSource } from 'ui/courier';

export function createIndexPatternsStub() {
return {
Expand All @@ -31,7 +32,10 @@ export function createIndexPatternsStub() {
};
}

export function createSearchSourceStubProvider(hits, timeField) {
/**
* A stubbed search source with a `fetch` method that returns all of `_stubHits`.
*/
export function createSearchSourceStub(hits, timeField) {
const searchSourceStub = {
_stubHits: hits,
_stubTimeField: timeField,
Expand All @@ -41,13 +45,37 @@ export function createSearchSourceStubProvider(hits, timeField) {
}),
};

searchSourceStub.setParent = sinon.stub().returns(searchSourceStub);
searchSourceStub.setField = sinon.stub().returns(searchSourceStub);
searchSourceStub.getField = sinon.spy(key => {
searchSourceStub.setParent = sinon.stub(SearchSource.prototype, 'setParent').returns(searchSourceStub);
searchSourceStub.setField = sinon.stub(SearchSource.prototype, 'setField').returns(searchSourceStub);
searchSourceStub.getField = sinon.stub(SearchSource.prototype, 'getField').callsFake(key => {
const previousSetCall = searchSourceStub.setField.withArgs(key).lastCall;
return previousSetCall ? previousSetCall.args[1] : null;
});
searchSourceStub.fetch = sinon.spy(() => {
searchSourceStub.fetch = sinon.stub(SearchSource.prototype, 'fetch').callsFake(() => Promise.resolve({
hits: {
hits: searchSourceStub._stubHits,
total: searchSourceStub._stubHits.length,
},
}));

searchSourceStub._restore = () => {
searchSourceStub.setParent.restore();
searchSourceStub.setField.restore();
searchSourceStub.getField.restore();
searchSourceStub.fetch.restore();
};

return searchSourceStub;
}

/**
* A stubbed search source with a `fetch` method that returns a filtered set of `_stubHits`.
*/
export function createContextSearchSourceStub(hits, timeField = '@timestamp') {
const searchSourceStub = createSearchSourceStub(hits, timeField);

searchSourceStub.fetch.restore();
searchSourceStub.fetch = sinon.stub(SearchSource.prototype, 'fetch').callsFake(() => {
const timeField = searchSourceStub._stubTimeField;
const lastQuery = searchSourceStub.setField.withArgs('query').lastCall.args[1];
const timeRange = lastQuery.query.constant_score.filter.range[timeField];
Expand All @@ -71,7 +99,5 @@ export function createSearchSourceStubProvider(hits, timeField) {
});
});

return function SearchSourceStubProvider() {
return searchSourceStub;
};
return searchSourceStub;
}
Loading