Skip to content

Commit dbb603f

Browse files
Corey Robertsonelasticmachine
andauthored
[Canvas][tech-debt] Ensure cursor is called until full results are received (#73347)
* Ensure cursor is called until full results are receeived * Fix Typecheck * Convert dependencies to typescript * Fix typings Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
1 parent 10f8bea commit dbb603f

File tree

14 files changed

+320
-110
lines changed

14 files changed

+320
-110
lines changed

x-pack/plugins/canvas/canvas_plugin_src/functions/server/esdocs.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
import squel from 'squel';
88
import { ExpressionFunctionDefinition } from 'src/plugins/expressions';
99
/* eslint-disable */
10-
// @ts-expect-error untyped local
1110
import { queryEsSQL } from '../../../server/lib/query_es_sql';
1211
/* eslint-enable */
1312
import { ExpressionValueFilter } from '../../../types';

x-pack/plugins/canvas/canvas_plugin_src/functions/server/essql.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66

77
import { ExpressionFunctionDefinition } from 'src/plugins/expressions/common';
88
/* eslint-disable */
9-
// @ts-expect-error untyped local
109
import { queryEsSQL } from '../../../server/lib/query_es_sql';
1110
/* eslint-enable */
1211
import { ExpressionValueFilter } from '../../../types';

x-pack/plugins/canvas/server/lib/build_bool_array.js renamed to x-pack/plugins/canvas/server/lib/build_bool_array.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,11 @@
55
*/
66

77
import { getESFilter } from './get_es_filter';
8+
import { ExpressionValueFilter } from '../../types';
89

9-
const compact = (arr) => (Array.isArray(arr) ? arr.filter((val) => Boolean(val)) : []);
10+
const compact = <T>(arr: T[]) => (Array.isArray(arr) ? arr.filter((val) => Boolean(val)) : []);
1011

11-
export function buildBoolArray(canvasQueryFilterArray) {
12+
export function buildBoolArray(canvasQueryFilterArray: ExpressionValueFilter[]) {
1213
return compact(
1314
canvasQueryFilterArray.map((clause) => {
1415
try {

x-pack/plugins/canvas/server/lib/filters.js

Lines changed: 0 additions & 38 deletions
This file was deleted.
Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
7+
import {
8+
FilterType,
9+
ExpressionValueFilter,
10+
CanvasTimeFilter,
11+
CanvasLuceneFilter,
12+
CanvasExactlyFilter,
13+
} from '../../types';
14+
15+
/*
16+
TODO: This could be pluggable
17+
*/
18+
19+
const isTimeFilter = (
20+
maybeTimeFilter: ExpressionValueFilter
21+
): maybeTimeFilter is CanvasTimeFilter => {
22+
return maybeTimeFilter.filterType === FilterType.time;
23+
};
24+
const isLuceneFilter = (
25+
maybeLuceneFilter: ExpressionValueFilter
26+
): maybeLuceneFilter is CanvasLuceneFilter => {
27+
return maybeLuceneFilter.filterType === FilterType.luceneQueryString;
28+
};
29+
const isExactlyFilter = (
30+
maybeExactlyFilter: ExpressionValueFilter
31+
): maybeExactlyFilter is CanvasExactlyFilter => {
32+
return maybeExactlyFilter.filterType === FilterType.exactly;
33+
};
34+
35+
export function time(filter: ExpressionValueFilter) {
36+
if (!isTimeFilter(filter) || !filter.column) {
37+
throw new Error('column is required for Elasticsearch range filters');
38+
}
39+
return {
40+
range: {
41+
[filter.column]: { gte: filter.from, lte: filter.to },
42+
},
43+
};
44+
}
45+
46+
export function luceneQueryString(filter: ExpressionValueFilter) {
47+
if (!isLuceneFilter(filter)) {
48+
throw new Error('Filter is not a lucene filter');
49+
}
50+
return {
51+
query_string: {
52+
query: filter.query || '*',
53+
},
54+
};
55+
}
56+
57+
export function exactly(filter: ExpressionValueFilter) {
58+
if (!isExactlyFilter(filter)) {
59+
throw new Error('Filter is not an exactly filter');
60+
}
61+
return {
62+
term: {
63+
[filter.column]: {
64+
value: filter.value,
65+
},
66+
},
67+
};
68+
}
69+
70+
export const filters: Record<string, any> = {
71+
exactly,
72+
time,
73+
luceneQueryString,
74+
};

x-pack/plugins/canvas/server/lib/get_es_filter.js renamed to x-pack/plugins/canvas/server/lib/get_es_filter.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,11 @@
1010
filter is the abstracted canvas filter.
1111
*/
1212

13-
/*eslint import/namespace: ['error', { allowComputed: true }]*/
14-
import * as filters from './filters';
13+
import { filters } from './filters';
14+
import { ExpressionValueFilter } from '../../types';
1515

16-
export function getESFilter(filter) {
17-
if (!filters[filter.filterType]) {
16+
export function getESFilter(filter: ExpressionValueFilter) {
17+
if (!filter.filterType || !filters[filter.filterType]) {
1818
throw new Error(`Unknown filter type: ${filter.filterType}`);
1919
}
2020

x-pack/plugins/canvas/server/lib/normalize_type.js renamed to x-pack/plugins/canvas/server/lib/normalize_type.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@
44
* you may not use this file except in compliance with the Elastic License.
55
*/
66

7-
export function normalizeType(type) {
8-
const normalTypes = {
7+
export function normalizeType(type: string) {
8+
const normalTypes: Record<string, string[]> = {
99
string: ['string', 'text', 'keyword', '_type', '_id', '_index', 'geo_point'],
1010
number: [
1111
'float',

x-pack/plugins/canvas/server/lib/query_es_sql.js

Lines changed: 0 additions & 59 deletions
This file was deleted.
Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
7+
import { zipObject } from 'lodash';
8+
import { queryEsSQL } from './query_es_sql';
9+
// @ts-expect-error
10+
import { buildBoolArray } from './build_bool_array';
11+
12+
const response = {
13+
columns: [
14+
{ name: 'One', type: 'keyword' },
15+
{ name: 'Two', type: 'keyword' },
16+
],
17+
rows: [
18+
['foo', 'bar'],
19+
['buz', 'baz'],
20+
],
21+
cursor: 'cursor-value',
22+
};
23+
24+
const baseArgs = {
25+
count: 1,
26+
query: 'query',
27+
filter: [],
28+
timezone: 'timezone',
29+
};
30+
31+
const getApi = (resp = response) => {
32+
const api = jest.fn();
33+
api.mockResolvedValue(resp);
34+
return api;
35+
};
36+
37+
describe('query_es_sql', () => {
38+
it('should call the api with the given args', async () => {
39+
const api = getApi();
40+
41+
queryEsSQL(api, baseArgs);
42+
43+
expect(api).toHaveBeenCalled();
44+
const givenArgs = api.mock.calls[0][1];
45+
46+
expect(givenArgs.body.fetch_size).toBe(baseArgs.count);
47+
expect(givenArgs.body.query).toBe(baseArgs.query);
48+
expect(givenArgs.body.time_zone).toBe(baseArgs.timezone);
49+
});
50+
51+
it('formats the response', async () => {
52+
const api = getApi();
53+
54+
const result = await queryEsSQL(api, baseArgs);
55+
56+
const expectedColumns = response.columns.map((c) => ({ name: c.name, type: 'string' }));
57+
const columnNames = expectedColumns.map((c) => c.name);
58+
const expectedRows = response.rows.map((r) => zipObject(columnNames, r));
59+
60+
expect(result.type).toBe('datatable');
61+
expect(result.columns).toEqual(expectedColumns);
62+
expect(result.rows).toEqual(expectedRows);
63+
});
64+
65+
it('fetches pages until it has the requested count', async () => {
66+
const pageOne = {
67+
columns: [
68+
{ name: 'One', type: 'keyword' },
69+
{ name: 'Two', type: 'keyword' },
70+
],
71+
rows: [['foo', 'bar']],
72+
cursor: 'cursor-value',
73+
};
74+
75+
const pageTwo = {
76+
rows: [['buz', 'baz']],
77+
};
78+
79+
const api = getApi(pageOne);
80+
api.mockReturnValueOnce(pageOne).mockReturnValueOnce(pageTwo);
81+
82+
const result = await queryEsSQL(api, { ...baseArgs, count: 2 });
83+
expect(result.rows).toHaveLength(2);
84+
});
85+
86+
it('closes any cursors that remain open', async () => {
87+
const api = getApi();
88+
89+
await queryEsSQL(api, baseArgs);
90+
expect(api.mock.calls[1][1].body.cursor).toBe(response.cursor);
91+
});
92+
93+
it('throws on errors', async () => {
94+
const api = getApi();
95+
api.mockRejectedValueOnce(new Error('parsing_exception'));
96+
api.mockRejectedValueOnce(new Error('generic es error'));
97+
98+
expect(queryEsSQL(api, baseArgs)).rejects.toThrowErrorMatchingInlineSnapshot(
99+
`"Couldn't parse Elasticsearch SQL query. You may need to add double quotes to names containing special characters. Check your query and try again. Error: parsing_exception"`
100+
);
101+
102+
expect(queryEsSQL(api, baseArgs)).rejects.toThrowErrorMatchingInlineSnapshot(
103+
`"Unexpected error from Elasticsearch: generic es error"`
104+
);
105+
});
106+
});

0 commit comments

Comments
 (0)