Skip to content

Commit

Permalink
[Discover] Add long numerals support
Browse files Browse the repository at this point in the history
* Add long numerals support to Discover tabular view, document view, inspector, hash-url, and share
* Use a fork of `numeral-js` that supports long numerals
* Patch `rison` to correctly serialize BigInts
* Replace the uses of `withLongNumerals` with `withLongNumeralsSupport` to match the rest of the codebase
* Add `withLongNumeralsSupport` as an option to `code/public/http/fetch`
* Add long numerals support to `UiSettingsClient`
* Add long numerals support to `core/server/http/router` response handler
* Add long numerals support to `RangeFilter` and `FilterBar`
* Add long numerals support to `kuery/ast`
* Introduce `OPENSEARCH_SEARCH_WITH_LONG_NUMERALS_STRATEGY` in `core/plugins/data/search`

Signed-off-by: Miki <[email protected]>
  • Loading branch information
AMoo-Miki committed Dec 11, 2023
1 parent 9fd2078 commit cb392a4
Show file tree
Hide file tree
Showing 37 changed files with 187 additions and 62 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@
"@elastic/datemath": "5.0.3",
"@elastic/eui": "npm:@opensearch-project/[email protected]",
"@elastic/good": "^9.0.1-kibana3",
"@elastic/numeral": "^2.5.0",
"@elastic/numeral": "npm:@amoo-miki/[email protected].0",
"@elastic/request-crypto": "2.0.0",
"@elastic/safer-lodash-set": "0.0.0",
"@hapi/accept": "^5.0.2",
Expand Down
3 changes: 2 additions & 1 deletion packages/osd-ui-shared-deps/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
"dependencies": {
"@elastic/charts": "31.1.0",
"@elastic/eui": "npm:@opensearch-project/[email protected]",
"@elastic/numeral": "^2.5.0",
"@elastic/numeral": "npm:@amoo-miki/[email protected].0",
"@opensearch/datemath": "5.0.3",
"@osd/i18n": "1.0.0",
"@osd/monaco": "1.0.0",
Expand Down Expand Up @@ -52,3 +52,4 @@
"webpack": "npm:@amoo-miki/[email protected]"
}
}

13 changes: 13 additions & 0 deletions scripts/postinstall.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,19 @@ const run = async () => {
},
])
);
promises.push(
patchFile('node_modules/rison-node/js/rison.js', [
{
from: 'return Number(s)',
to:
'return s > Number.MAX_SAFE_INTEGER || s < Number.MIN_SAFE_INTEGER ? BigInt(s) : Number(s)',
},
{
from: 's = {',
to: 's = {\n bigint: x => x.toString(),',
},
])
);

await Promise.all(promises);
};
Expand Down
8 changes: 6 additions & 2 deletions src/core/public/http/fetch.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -839,7 +839,9 @@ describe('Fetch', () => {
},
});

await expect(fetchInstance.fetch('/my/path', { withLongNumerals: true })).resolves.toEqual({
await expect(
fetchInstance.fetch('/my/path', { withLongNumeralsSupport: true })
).resolves.toEqual({
'long-max': longPositive,
'long-min': longNegative,
});
Expand All @@ -854,7 +856,9 @@ describe('Fetch', () => {
},
});

await expect(fetchInstance.fetch('/my/path', { withLongNumerals: true })).resolves.toEqual({
await expect(
fetchInstance.fetch('/my/path', { withLongNumeralsSupport: true })
).resolves.toEqual({
'long-max': longPositive,
'long-min': longNegative,
});
Expand Down
6 changes: 4 additions & 2 deletions src/core/public/http/fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -190,12 +190,14 @@ export class Fetch {
if (NDJSON_CONTENT.test(contentType)) {
body = await response.blob();
} else if (JSON_CONTENT.test(contentType)) {
body = fetchOptions.withLongNumerals ? parse(await response.text()) : await response.json();
body = fetchOptions.withLongNumeralsSupport
? parse(await response.text())
: await response.json();
} else {
const text = await response.text();

try {
body = fetchOptions.withLongNumerals ? parse(text) : JSON.parse(text);
body = fetchOptions.withLongNumeralsSupport ? parse(text) : JSON.parse(text);
} catch (err) {
body = text;
}
Expand Down
2 changes: 1 addition & 1 deletion src/core/public/http/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ export interface HttpFetchOptions extends HttpRequestInit {
* When `true`, if the response has a JSON mime type, the {@link HttpResponse} will use an alternate JSON parser
* that converts long numerals to BigInts. Defaults to `false`.
*/
withLongNumerals?: boolean;
withLongNumeralsSupport?: boolean;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/core/public/ui_settings/ui_settings_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ You can use \`IUiSettingsClient.get("${key}", defaultValue)\`, which will just r
return JSON.parse(value);
}

if (type === 'number') {
if (type === 'number' && typeof value !== 'bigint') {
return parseFloat(value);
}

Expand Down
2 changes: 2 additions & 0 deletions src/core/server/http/router/response.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,8 @@ export interface HttpResponseOptions {
body?: HttpResponsePayload;
/** HTTP Headers with additional information about response */
headers?: ResponseHeaders;
/** Indicates if alternate serialization should be employed */
withLongNumeralsSupport?: boolean;
}

/**
Expand Down
12 changes: 11 additions & 1 deletion src/core/server/http/router/response_adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import {
import typeDetect from 'type-detect';
import Boom from '@hapi/boom';
import * as stream from 'stream';
import { stringify } from '@osd/std';

import {
HttpResponsePayload,
Expand Down Expand Up @@ -108,9 +109,18 @@ export class HapiResponseAdapter {
opensearchDashboardsResponse: OpenSearchDashboardsResponse<HttpResponsePayload>
) {
const response = this.responseToolkit
.response(opensearchDashboardsResponse.payload)
.response(
opensearchDashboardsResponse.options.withLongNumeralsSupport
? stringify(opensearchDashboardsResponse.payload)
: opensearchDashboardsResponse.payload
)
.code(opensearchDashboardsResponse.status);
setHeaders(response, opensearchDashboardsResponse.options.headers);
if (opensearchDashboardsResponse.options.withLongNumeralsSupport) {
setHeaders(response, {
'content-type': 'application/json',
});
}
return response;
}

Expand Down
2 changes: 1 addition & 1 deletion src/plugins/console/public/lib/opensearch/opensearch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export async function send(
body: data,
prependBasePath: true,
asResponse: true,
withLongNumerals: true,
withLongNumeralsSupport: true,
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ export const setup = (
};

const wrap: HtmlContextTypeConvert = (value, options) => {
return `<span ng-non-bindable>${recurse(value, options)}</span>`;
return `<span>${recurse(value, options)}</span>`;
};

return wrap;
Expand Down
7 changes: 5 additions & 2 deletions src/plugins/data/common/field_formats/converters/numeral.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,14 @@ export abstract class NumeralFormat extends FieldFormat {
protected getConvertedValue(val: any): string {
if (val === -Infinity) return '-∞';
if (val === +Infinity) return '+∞';
if (typeof val !== 'number') {

const isBigInt = typeof val === 'bigint';

if (typeof val !== 'number' && !isBigInt) {
val = parseFloat(val);
}

if (isNaN(val)) return '';
if (!isBigInt && isNaN(val)) return '';

const previousLocale = numeral.language();
const defaultLocale =
Expand Down
15 changes: 9 additions & 6 deletions src/plugins/data/common/opensearch_query/filters/range_filter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,19 +124,22 @@ export const buildRangeFilter = (
filter.meta.formattedValue = formattedValue;
}

params = mapValues(params, (value: any) => (field.type === 'number' ? parseFloat(value) : value));
params = mapValues(params, (value: any) =>
field.type === 'number'
? value > Number.MAX_SAFE_INTEGER || value < Number.MIN_SAFE_INTEGER
? BigInt(value)
: parseFloat(value)
: value
);

if ('gte' in params && 'gt' in params) throw new Error('gte and gt are mutually exclusive');
if ('lte' in params && 'lt' in params) throw new Error('lte and lt are mutually exclusive');

const totalInfinite = ['gt', 'lt'].reduce((acc: number, op: any) => {
const key = op in params ? op : `${op}e`;
const isInfinite = Math.abs(get(params, key)) === Infinity;
const key: keyof RangeFilterParams = op in params ? op : `${op}e`;

if (isInfinite) {
if (typeof params[key] !== 'bigint' && !isFinite(params[key] as any)) {
acc++;

// @ts-ignore
delete params[key];
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -233,8 +233,14 @@ module.exports = (function() {
if (sequence === 'true') return buildLiteralNode(true);
if (sequence === 'false') return buildLiteralNode(false);
if (chars.includes(wildcardSymbol)) return buildWildcardNode(sequence);
const isNumberPattern = /^(-?[1-9]+\d*([.]\d+)?)$|^(-?0[.]\d*[1-9]+)$|^0$|^0.0$|^[.]\d{1,}$/
return buildLiteralNode(isNumberPattern.test(sequence) ? Number(sequence) : sequence);
const isNumberPattern = /^(-?[1-9]+\d*([.]\d+)?)$|^(-?0[.]\d*[1-9]+)$|^0$|^0.0$|^[.]\d{1,}$/;
return buildLiteralNode(
isNumberPattern.test(sequence)
? sequence > Number.MAX_SAFE_INTEGER || sequence < Number.MIN_SAFE_INTEGER
? BigInt(sequence)
: Number(sequence)
: sequence
);
},
peg$c50 = { type: "any", description: "any character" },
peg$c51 = "*",
Expand Down
10 changes: 8 additions & 2 deletions src/plugins/data/common/opensearch_query/kuery/ast/kuery.peg
Original file line number Diff line number Diff line change
Expand Up @@ -247,8 +247,14 @@ UnquotedLiteral
if (sequence === 'true') return buildLiteralNode(true);
if (sequence === 'false') return buildLiteralNode(false);
if (chars.includes(wildcardSymbol)) return buildWildcardNode(sequence);
const isNumberPattern = /^(-?[1-9]+\d*([.]\d+)?)$|^(-?0[.]\d*[1-9]+)$|^0$|^0.0$|^[.]\d{1,}$/
return buildLiteralNode(isNumberPattern.test(sequence) ? Number(sequence) : sequence);
const isNumberPattern = /^(-?[1-9]+\d*([.]\d+)?)$|^(-?0[.]\d*[1-9]+)$|^0$|^0.0$|^[.]\d{1,}$/;
return buildLiteralNode(
isNumberPattern.test(sequence)
? sequence > Number.MAX_SAFE_INTEGER || sequence < Number.MIN_SAFE_INTEGER
? BigInt(sequence)
: Number(sequence)
: sequence
);
}

UnquotedCharacter
Expand Down
5 changes: 5 additions & 0 deletions src/plugins/data/common/search/opensearch_search/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import { Search } from '@opensearch-project/opensearch/api/requestParams';
import { IOpenSearchDashboardsSearchRequest, IOpenSearchDashboardsSearchResponse } from '../types';

export const OPENSEARCH_SEARCH_STRATEGY = 'opensearch';
export const OPENSEARCH_SEARCH_WITH_LONG_NUMERALS_STRATEGY = 'opensearch-with-long-numerals';

export interface ISearchOptions {
/**
Expand All @@ -43,6 +44,10 @@ export interface ISearchOptions {
* Use this option to force using a specific server side search strategy. Leave empty to use the default strategy.
*/
strategy?: string;
/**
* Use this option to enable support for long numerals.
*/
withLongNumeralsSupport?: boolean;
}

export type ISearchRequestParams<T = Record<string, any>> = {
Expand Down
3 changes: 2 additions & 1 deletion src/plugins/data/common/search/search_source/parse_json.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,15 @@
* under the License.
*/

import { parse } from '@osd/std';
import { SearchSourceFields } from './types';
import { InvalidJSONProperty } from '../../../../opensearch_dashboards_utils/common';

export const parseSearchSourceJSON = (searchSourceJSON: string) => {
// if we have a searchSource, set its values based on the searchSourceJson field
let searchSourceValues: SearchSourceFields;
try {
searchSourceValues = JSON.parse(searchSourceJSON);
searchSourceValues = parse(searchSourceJSON);
} catch (e) {
throw new InvalidJSONProperty(
`Invalid JSON in search source. ${e.message} JSON: ${searchSourceJSON}`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@
*/

import { setWith } from '@elastic/safer-lodash-set';
import { stringify } from '@osd/std';
import { uniqueId, uniq, extend, pick, difference, omit, isObject, keys, isFunction } from 'lodash';
import { normalizeSortRequest } from './normalize_sort_request';
import { filterDocvalueFields } from './filter_docvalue_fields';
Expand Down Expand Up @@ -561,7 +562,7 @@ export class SearchSource {
* @public */
public serialize() {
const [searchSourceFields, references] = extractReferences(this.getSerializedFields());
return { searchSourceJSON: JSON.stringify(searchSourceFields), references };
return { searchSourceJSON: stringify(searchSourceFields), references };
}

private getFilters(filterField: SearchSourceFields['filter']): Filter[] {
Expand Down
5 changes: 5 additions & 0 deletions src/plugins/data/common/search/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,11 @@ export interface IOpenSearchDashboardsSearchResponse<RawResponse = any> {
*/
isPartial?: boolean;

/**
* Indicates whether the results returned need long numerals treatment
*/
withLongNumeralsSupport?: boolean;

rawResponse: RawResponse;
}

Expand Down
35 changes: 31 additions & 4 deletions src/plugins/data/public/search/search_interceptor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,15 @@ import { get, trimEnd, debounce } from 'lodash';
import { BehaviorSubject, throwError, timer, defer, from, Observable, NEVER } from 'rxjs';
import { catchError, finalize } from 'rxjs/operators';
import { CoreStart, CoreSetup, ToastsSetup } from 'opensearch-dashboards/public';
import { stringify } from '@osd/std';
import {
getCombinedSignal,
AbortError,
IOpenSearchDashboardsSearchRequest,
IOpenSearchDashboardsSearchResponse,
ISearchOptions,
OPENSEARCH_SEARCH_STRATEGY,
OPENSEARCH_SEARCH_WITH_LONG_NUMERALS_STRATEGY,
} from '../../common';
import { SearchUsageCollector } from './collectors';
import { SearchTimeoutError, PainlessError, isPainlessError } from './errors';
Expand Down Expand Up @@ -113,20 +115,40 @@ export class SearchInterceptor {
protected runSearch(
request: IOpenSearchDashboardsSearchRequest,
signal: AbortSignal,
strategy?: string
strategy?: string,
withLongNumeralsSupport?: boolean
): Observable<IOpenSearchDashboardsSearchResponse> {
const { id, ...searchRequest } = request;
const path = trimEnd(
`/internal/search/${strategy || OPENSEARCH_SEARCH_STRATEGY}/${id || ''}`,
`/internal/search/${
strategy ||
(withLongNumeralsSupport
? OPENSEARCH_SEARCH_WITH_LONG_NUMERALS_STRATEGY
: OPENSEARCH_SEARCH_STRATEGY)
}/${id || ''}`,
'/'
);
const body = JSON.stringify(searchRequest);
/* When long-numerals support is not explicitly requested, it indicates that the response handler
* is not capable of handling BigInts. Ideally, because this is the outward request, that wouldn't
* matter and `body = stringify(searchRequest)` would be the best option. However, to maintain the
* existing behavior, we use @osd/std/json.stringify only when long-numerals support is explicitly
* requested. Otherwise, JSON.stringify is used with imprecise numbers for BigInts.
*
* ToDo: With OSD v3, change to `body = stringify(searchRequest)`.
*/
const body = withLongNumeralsSupport
? stringify(searchRequest)
: JSON.stringify(searchRequest, (key: string, value: any) =>
typeof value === 'bigint' ? Number(value) : value
);

return from(
this.deps.http.fetch({
method: 'POST',
path,
body,
signal,
withLongNumeralsSupport,
})
);
}
Expand Down Expand Up @@ -214,7 +236,12 @@ export class SearchInterceptor {
});
this.pendingCount$.next(this.pendingCount$.getValue() + 1);

return this.runSearch(request, combinedSignal, options?.strategy).pipe(
return this.runSearch(
request,
combinedSignal,
options?.strategy,
options?.withLongNumeralsSupport
).pipe(
catchError((e: any) => {
return throwError(
this.handleSearchError(e, request, timeoutSignal, options?.abortSignal)
Expand Down
3 changes: 2 additions & 1 deletion src/plugins/data/public/ui/filter_bar/filter_bar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import {
import { FormattedMessage, InjectedIntl, injectI18n } from '@osd/i18n/react';
import classNames from 'classnames';
import React, { useState } from 'react';
import { stringify } from '@osd/std';

import { FilterEditor } from './filter_editor';
import { FilterItem } from './filter_item';
Expand Down Expand Up @@ -143,7 +144,7 @@ function FilterBarUI(props: Props) {
indexPatterns={props.indexPatterns}
onSubmit={onAdd}
onCancel={() => setIsAddFilterPopoverOpen(false)}
key={JSON.stringify(newFilter)}
key={stringify(newFilter)}
/>
</EuiFlexItem>
</div>
Expand Down
Loading

0 comments on commit cb392a4

Please sign in to comment.