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`
* Remove `ng-non-bindable` leftovers

Signed-off-by: Miki <[email protected]>
  • Loading branch information
AMoo-Miki committed Dec 11, 2023
1 parent 9fd2078 commit d67697a
Show file tree
Hide file tree
Showing 42 changed files with 242 additions and 122 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
30 changes: 15 additions & 15 deletions src/plugins/data/common/field_formats/converters/color.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,14 @@ describe('Color Format', () => {
jest.fn()
);

expect(colorer.convert(99, HTML_CONTEXT_TYPE)).toBe('<span ng-non-bindable>99</span>');
expect(colorer.convert(99, HTML_CONTEXT_TYPE)).toBe('<span>99</span>');
expect(colorer.convert(100, HTML_CONTEXT_TYPE)).toBe(
'<span ng-non-bindable><span style="color: blue;background-color: yellow;">100</span></span>'
'<span><span style="color: blue;background-color: yellow;">100</span></span>'
);
expect(colorer.convert(150, HTML_CONTEXT_TYPE)).toBe(
'<span ng-non-bindable><span style="color: blue;background-color: yellow;">150</span></span>'
'<span><span style="color: blue;background-color: yellow;">150</span></span>'
);
expect(colorer.convert(151, HTML_CONTEXT_TYPE)).toBe('<span ng-non-bindable>151</span>');
expect(colorer.convert(151, HTML_CONTEXT_TYPE)).toBe('<span>151</span>');
});

test('should not convert invalid ranges', () => {
Expand All @@ -73,7 +73,7 @@ describe('Color Format', () => {
jest.fn()
);

expect(colorer.convert(99, HTML_CONTEXT_TYPE)).toBe('<span ng-non-bindable>99</span>');
expect(colorer.convert(99, HTML_CONTEXT_TYPE)).toBe('<span>99</span>');
});
});

Expand All @@ -94,26 +94,26 @@ describe('Color Format', () => {
);
const converter = colorer.getConverterFor(HTML_CONTEXT_TYPE) as Function;

expect(converter('B', HTML_CONTEXT_TYPE)).toBe('<span ng-non-bindable>B</span>');
expect(converter('B', HTML_CONTEXT_TYPE)).toBe('<span>B</span>');
expect(converter('AAA', HTML_CONTEXT_TYPE)).toBe(
'<span ng-non-bindable><span style="color: blue;background-color: yellow;">AAA</span></span>'
'<span><span style="color: blue;background-color: yellow;">AAA</span></span>'
);
expect(converter('AB', HTML_CONTEXT_TYPE)).toBe(
'<span ng-non-bindable><span style="color: blue;background-color: yellow;">AB</span></span>'
'<span><span style="color: blue;background-color: yellow;">AB</span></span>'
);
expect(converter('a', HTML_CONTEXT_TYPE)).toBe('<span ng-non-bindable>a</span>');
expect(converter('a', HTML_CONTEXT_TYPE)).toBe('<span>a</span>');

expect(converter('B', HTML_CONTEXT_TYPE)).toBe('<span ng-non-bindable>B</span>');
expect(converter('B', HTML_CONTEXT_TYPE)).toBe('<span>B</span>');
expect(converter('AAA', HTML_CONTEXT_TYPE)).toBe(
'<span ng-non-bindable><span style="color: blue;background-color: yellow;">AAA</span></span>'
'<span><span style="color: blue;background-color: yellow;">AAA</span></span>'
);
expect(converter('AB', HTML_CONTEXT_TYPE)).toBe(
'<span ng-non-bindable><span style="color: blue;background-color: yellow;">AB</span></span>'
'<span><span style="color: blue;background-color: yellow;">AB</span></span>'
);
expect(converter('AB <', HTML_CONTEXT_TYPE)).toBe(
'<span ng-non-bindable><span style="color: blue;background-color: yellow;">AB &lt;</span></span>'
'<span><span style="color: blue;background-color: yellow;">AB &lt;</span></span>'
);
expect(converter('a', HTML_CONTEXT_TYPE)).toBe('<span ng-non-bindable>a</span>');
expect(converter('a', HTML_CONTEXT_TYPE)).toBe('<span>a</span>');
});

test('returns original value (escaped) when regex is invalid', () => {
Expand All @@ -132,7 +132,7 @@ describe('Color Format', () => {
);
const converter = colorer.getConverterFor(HTML_CONTEXT_TYPE) as Function;

expect(converter('<', HTML_CONTEXT_TYPE)).toBe('<span ng-non-bindable>&lt;</span>');
expect(converter('<', HTML_CONTEXT_TYPE)).toBe('<span>&lt;</span>');
});
});
});
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
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ describe('Source Format', () => {
};

expect(convertHtml(hit)).toBe(
'<span ng-non-bindable>{&quot;foo&quot;:&quot;bar&quot;,&quot;number&quot;:42,&quot;hello&quot;:&quot;&lt;h1&gt;World&lt;/h1&gt;&quot;,&quot;also&quot;:&quot;with \\&quot;quotes\\&quot; or &#39;single quotes&#39;&quot;}</span>'
'<span>{&quot;foo&quot;:&quot;bar&quot;,&quot;number&quot;:42,&quot;hello&quot;:&quot;&lt;h1&gt;World&lt;/h1&gt;&quot;,&quot;also&quot;:&quot;with \\&quot;quotes\\&quot; or &#39;single quotes&#39;&quot;}</span>'
);
});
});
Loading

0 comments on commit d67697a

Please sign in to comment.