Skip to content

Commit

Permalink
Fix a typo while inspecting values for large numerals in OSD and the …
Browse files Browse the repository at this point in the history
…JS client (#8839)

* [@osd/std] Fix typo while inspecting values for large numerals

Signed-off-by: Miki <[email protected]>

* Patch @opensearch-project/opensearch to fix a typo

Ref: opensearch-project/opensearch-js#889

Signed-off-by: Miki <[email protected]>

* Changeset file for PR #8839 created/updated

---------

Signed-off-by: Miki <[email protected]>
Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
(cherry picked from commit e993d24)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
  • Loading branch information
1 parent 6256188 commit 82a70a3
Show file tree
Hide file tree
Showing 4 changed files with 93 additions and 4 deletions.
2 changes: 2 additions & 0 deletions changelogs/fragments/8839.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
fix:
- Fix a typo while inspecting values for large numerals in OSD and the JS client ([#8839](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/8839))
84 changes: 81 additions & 3 deletions packages/osd-std/src/json.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

import JSON11 from 'json11';
import { stringify, parse } from './json';

describe('json', () => {
Expand Down Expand Up @@ -90,9 +91,55 @@ describe('json', () => {
expect(stringify(input, replacer, 2)).toEqual(JSON.stringify(input, replacer, 2));
});

it('can handle long numerals while parsing', () => {
const longPositive = BigInt(Number.MAX_SAFE_INTEGER) * 2n;
const longNegative = BigInt(Number.MIN_SAFE_INTEGER) * 2n;
it('can handle positive long numerals while parsing', () => {
const longPositiveA = BigInt(Number.MAX_SAFE_INTEGER) * 2n;
const longPositiveB = BigInt(Number.MAX_SAFE_INTEGER) * 2n + 1n;
const text =
`{` +
// The space before and after the values, and the lack of spaces before comma are intentional
`"\\":${longPositiveA}": "[ ${longPositiveB.toString()}, ${longPositiveA.toString()} ]", ` +
`"positive": ${longPositiveA.toString()}, ` +
`"array": [ ${longPositiveB.toString()}, ${longPositiveA.toString()} ], ` +
`"negative": ${longPositiveB.toString()},` +
`"number": 102931203123987` +
`}`;

const result = parse(text);
expect(result.positive).toBe(longPositiveA);
expect(result.negative).toBe(longPositiveB);
expect(result.array).toEqual([longPositiveB, longPositiveA]);
expect(result['":' + longPositiveA]).toBe(
`[ ${longPositiveB.toString()}, ${longPositiveA.toString()} ]`
);
expect(result.number).toBe(102931203123987);
});

it('can handle negative long numerals while parsing', () => {
const longNegativeA = BigInt(Number.MIN_SAFE_INTEGER) * 2n;
const longNegativeB = BigInt(Number.MIN_SAFE_INTEGER) * 2n - 1n;
const text =
`{` +
// The space before and after the values, and the lack of spaces before comma are intentional
`"\\":${longNegativeA}": "[ ${longNegativeB.toString()}, ${longNegativeA.toString()} ]", ` +
`"positive": ${longNegativeA.toString()}, ` +
`"array": [ ${longNegativeB.toString()}, ${longNegativeA.toString()} ], ` +
`"negative": ${longNegativeB.toString()},` +
`"number": 102931203123987` +
`}`;

const result = parse(text);
expect(result.positive).toBe(longNegativeA);
expect(result.negative).toBe(longNegativeB);
expect(result.array).toEqual([longNegativeB, longNegativeA]);
expect(result['":' + longNegativeA]).toBe(
`[ ${longNegativeB.toString()}, ${longNegativeA.toString()} ]`
);
expect(result.number).toBe(102931203123987);
});

it('can handle mixed long numerals while parsing', () => {
const longPositive = BigInt(Number.MAX_SAFE_INTEGER) * 2n + 1n;
const longNegative = BigInt(Number.MIN_SAFE_INTEGER) * 2n - 1n;
const text =
`{` +
// The space before and after the values, and the lack of spaces before comma are intentional
Expand All @@ -113,6 +160,37 @@ describe('json', () => {
expect(result.number).toBe(102931203123987);
});

it('does not use JSON11 when not needed', () => {
const spyParse = jest.spyOn(JSON11, 'parse');

const longPositive = BigInt(Number.MAX_SAFE_INTEGER) * 2n + 1n;
const longNegative = BigInt(Number.MIN_SAFE_INTEGER) * 2n - 1n;
const text =
`{` +
`"\\":${longPositive}": "[ ${longNegative.toString()}, ${longPositive.toString()} ]", ` +
`"number": 102931203123987` +
`}`;
parse(text);

expect(spyParse).not.toHaveBeenCalled();
});

it('uses JSON11 when dealing with long numerals', () => {
const spyParse = jest.spyOn(JSON11, 'parse');

const longPositive = BigInt(Number.MAX_SAFE_INTEGER) * 2n + 1n;
const longNegative = BigInt(Number.MIN_SAFE_INTEGER) * 2n - 1n;
const text =
`{` +
`"\\":${longPositive}": "[ ${longNegative.toString()}, ${longPositive.toString()} ]", ` +
`"positive": ${longPositive.toString()}, ` +
`"number": 102931203123987` +
`}`;
parse(text);

expect(spyParse).toHaveBeenCalled();
});

it('can handle BigInt values while stringifying', () => {
const longPositive = BigInt(Number.MAX_SAFE_INTEGER) * 2n;
const longNegative = BigInt(Number.MIN_SAFE_INTEGER) * 2n;
Expand Down
2 changes: 1 addition & 1 deletion packages/osd-std/src/json.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ export const parse = (
numeralsAreNumbers &&
typeof val === 'number' &&
isFinite(val) &&
(val < Number.MAX_SAFE_INTEGER || val > Number.MAX_SAFE_INTEGER)
(val < Number.MIN_SAFE_INTEGER || val > Number.MAX_SAFE_INTEGER)
) {
numeralsAreNumbers = false;
}
Expand Down
9 changes: 9 additions & 0 deletions scripts/postinstall.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,15 @@ const run = async () => {
},
])
);
//ToDo: Remove when opensearch-js is released to include https://github.com/opensearch-project/opensearch-js/pull/889
promises.push(
patchFile('node_modules/@opensearch-project/opensearch/lib/Serializer.js', [
{
from: 'val < Number.MAX_SAFE_INTEGER',
to: 'val < Number.MIN_SAFE_INTEGER',
},
])
);

//Axios's type definition is far too advanced for OSD
promises.push(
Expand Down

0 comments on commit 82a70a3

Please sign in to comment.