Skip to content

Commit

Permalink
Support BigInt values
Browse files Browse the repository at this point in the history
JSON doesn't constrain numeric values to be specific sizes. In most
languages this isn't an issue thanks to proper support for a wide
variety of number precision.

Unfortunately JavaScript struggles with integer numbers bigger than
`Number.MAX_SAFE_INTEGER`. This is due to JavaScript's implementation of
integer values as `double`. For anything bigger than this max safe
integer value JavaScript provides a `BigInt` type.

The first issue with this behavior is that when parsing JSON, there is
no way of telling which "number flavor" to expect, APIs just don't allow
us to define so. Another issue is that the default JSON
serializer/deserializer doesn't even know how to handle `BigInt`.

This commit fixes those issues as followed:

1. Use a custom JSON parser `json-bigint` that can serialize and
   deserialze `BigInt` values. A quirk is that it will only deserialize
   a number into a `BigInt` if the serialized value ends up being bigger
   than JavaScript's max safe integer. This means there is ambiguity
   when deserialing data.
2. To address the ambiguity issue, I added a TypeScript mapped type
   `Deserialized<T>` that given a type `T` will replace all fields
   that are either `bigint` or `number`. Note that this is only static
   typing to teach TypeScript about this behavior. Then when receiving
   messages, one has to define `Normalizer` functions which will take
   this ambiguous type as input and return a "normalized" version that
   matches the original type `T`. See this as post-processing to make
   sure the received data is using the proper data types in the right
   places.
3. To help with the normalization I added a `createNormalizer` helper
   function that should statically figure out what fields need to be
   normalized for a given type and ensure that those are properly
   handled.
4. Rewrite all the tests to validate this logic using test data as
   coming out of the current Trace Server.

Signed-off-by: Paul Marechal <paul.marechal@ericsson.com>
Co-Authored-by: Patrick Tasse <patrick.tasse@ericsson.com>
  • Loading branch information
paul-marechal and PatrickTasse committed Nov 22, 2021
1 parent fa7ee18 commit f6f2d26
Show file tree
Hide file tree
Showing 46 changed files with 1,351 additions and 435 deletions.
19 changes: 19 additions & 0 deletions .editorconfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
root = true

[*]
charset = utf-8
end_of_line = lf
insert_final_newline = true
trim_trailing_whitespace = true

[*.ts]
indent_size = 4
indent_style = space

[*.test.ts]
indent_size = 2
indent_style = space

[*.json]
indent_size = 2
indent_style = space
18 changes: 12 additions & 6 deletions .github/workflows/ci-cd.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ on:
- master
workflow_dispatch:


jobs:
build:
runs-on: ${{ matrix.os }}
Expand All @@ -36,13 +35,18 @@ jobs:

- name: Build
shell: bash
# Note: `yarn build` is done as part of `yarn install` already.
run: |
yarn install --no-lockfile
yarn build
env:
NODE_OPTIONS: "--max_old_space_size=4096"
NODE_OPTIONS: --max_old_space_size=4096
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # https://github.com/microsoft/vscode-ripgrep/issues/9

- name: Test
shell: bash
run: |
yarn test
publish:
needs: build
if: github.ref == 'refs/heads/master' && github.event_name == 'push' && github.repository == 'theia-ide/tsp-typescript-client'
Expand All @@ -65,11 +69,11 @@ jobs:
node-version: ${{ matrix.node }}
registry-url: 'https://registry.npmjs.org'

- name: Pre-Publish
- name: Pre-Publish
shell: bash
# Note: `yarn build` is done as part of `yarn install` already.
run: |
yarn install --no-lockfile
yarn build
env:
NODE_OPTIONS: --max_old_space_size=4096
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # https://github.com/microsoft/vscode-ripgrep/issues/9
Expand All @@ -83,4 +87,6 @@ jobs:
retry_on: error
command: yarn run publish:next
env:
NODE_AUTH_TOKEN: ${{ secrets.NPM_AUTH_TOKEN }} # The variable name comes from here: https://github.com/actions/setup-node/blob/70b9252472eee7495c93bb1588261539c3c2b98d/src/authutil.ts#L48
# The variable name comes from here:
# https://github.com/actions/setup-node/blob/70b9252472eee7495c93bb1588261539c3c2b98d/src/authutil.ts#L48
NODE_AUTH_TOKEN: ${{ secrets.NPM_AUTH_TOKEN }}
3 changes: 3 additions & 0 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"typescript.tsdk": "node_modules/typescript/lib"
}
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
"typescript": "latest"
},
"dependencies": {
"@types/json-bigint": "^1.0.1",
"json-bigint": "sidorares/json-bigint#2c0a5f896d7888e68e5f4ae3c7ea5cd42fd54473",
"node-fetch": "^2.5.0"
},
"workspaces": [
Expand Down
2 changes: 1 addition & 1 deletion tsp-typescript-client/jest.config.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,4 @@
"json",
"node"
]
}
}
18 changes: 15 additions & 3 deletions tsp-typescript-client/src/models/annotation.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { array, assertNumber, createNormalizer, record } from '../protocol/serialization';
import { OutputElementStyle } from './styles';

export enum Type {
Expand All @@ -9,9 +10,12 @@ export interface AnnotationCategoriesModel {
annotationCategories: string[];
}

export interface AnnotationModel {
annotations: { [category: string]: Annotation[] };
}
export const Annotation = createNormalizer<Annotation>({
duration: BigInt,
entryId: assertNumber,
time: BigInt,
style: OutputElementStyle,
});

/**
* Model for annotation
Expand Down Expand Up @@ -48,3 +52,11 @@ export interface Annotation {
*/
style?: OutputElementStyle;
}

export const AnnotationModel = createNormalizer<AnnotationModel>({
annotations: record(array(Annotation)),
});

export interface AnnotationModel {
annotations: { [category: string]: Annotation[] };
}
7 changes: 7 additions & 0 deletions tsp-typescript-client/src/models/bookmark.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
import { createNormalizer } from '../protocol/serialization';

export const Bookmark = createNormalizer<Bookmark>({
endTime: BigInt,
startTime: BigInt,
});

/**
* Model for bookmark
*/
Expand Down
15 changes: 15 additions & 0 deletions tsp-typescript-client/src/models/entry.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
import { array, assertNumber, createNormalizer, Normalizer } from '../protocol/serialization';
import { OutputElementStyle } from './styles';

export const Entry = createNormalizer<Entry>({
id: assertNumber,
parentId: assertNumber,
style: {
values: undefined,
},
});

/**
* Basic entry interface
*/
Expand Down Expand Up @@ -45,6 +54,12 @@ export interface EntryHeader {
tooltip: string
}

export function EntryModel<T extends Entry>(normalizer: Normalizer<T>): Normalizer<EntryModel<T>> {
return createNormalizer<EntryModel<any>>({
entries: array(normalizer),
});
}

/**
* Entry model that will be returned by the server
*/
Expand Down
8 changes: 8 additions & 0 deletions tsp-typescript-client/src/models/experiment.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
import { array, assertNumber, createNormalizer } from '../protocol/serialization';
import { Trace } from './trace';

export const Experiment = createNormalizer<Experiment>({
end: BigInt,
nbEvents: assertNumber,
start: BigInt,
traces: array(Trace),
});

/**
* Model of an experiment that contain one or more traces
*/
Expand Down
18 changes: 13 additions & 5 deletions tsp-typescript-client/src/models/output-descriptor.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
import { createNormalizer } from '../protocol/serialization';

export const OutputDescriptor = createNormalizer<OutputDescriptor>({
end: BigInt,
queryParameters: undefined,
start: BigInt,
});

/**
* Descriptor of a specific output provider
*/
Expand Down Expand Up @@ -26,26 +34,26 @@ export interface OutputDescriptor {
/**
* Map of query parameters that the provider accept
*/
queryParameters: Map<string, any>;
queryParameters?: Record<string, any>;

/**
* Start time
*/
start: bigint;
start?: bigint;

/**
* End time
*/
end: bigint;
end?: bigint;

/**
* Indicate if the start, end times and current model are final,
* or if they will need to be refreshed later to represent a more up to date version
*/
final: boolean;
final?: boolean;

/**
* List of compatible outputs that can be used in the same view (ex. as overlay)
*/
compatibleProviders: string[];
compatibleProviders?: string[];
}
2 changes: 2 additions & 0 deletions tsp-typescript-client/src/models/query/query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@
* The output response will contain only elements that pass these filters.
*/
export class Query {

/**
* Map of parameters used for the query
*/
// @ts-expect-error TS doesn't like unused private fields.
private parameters: object;

/**
Expand Down
15 changes: 9 additions & 6 deletions tsp-typescript-client/src/models/response/responses.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { OutputDescriptor } from '../output-descriptor';
import { Deserialized, createNormalizer, Normalizer } from '../../protocol/serialization';

/**
* Response status
Expand All @@ -24,6 +24,14 @@ export enum ResponseStatus {
CANCELLED = 'CANCELLED'
}

export function GenericResponse<T>(): Normalizer<GenericResponse<Deserialized<T>>>;
export function GenericResponse<T>(normalizer: Normalizer<T>): Normalizer<GenericResponse<T>>;
export function GenericResponse<T>(normalizer?: Normalizer<T>): Normalizer<GenericResponse<T>> | Normalizer<GenericResponse<Deserialized<T>>> {
return createNormalizer<GenericResponse<any>>({
model: normalizer,
});
}

/**
* Generic response that contains a model
*/
Expand All @@ -33,11 +41,6 @@ export interface GenericResponse<T> {
*/
model: T;

/**
* Output descriptor
*/
output: OutputDescriptor;

/**
* Response status as described by ResponseStatus
*/
Expand Down
6 changes: 6 additions & 0 deletions tsp-typescript-client/src/models/styles.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
import { createNormalizer } from '../protocol/serialization';

export const OutputElementStyle = createNormalizer<OutputElementStyle>({
values: undefined,
});

/**
* Output element style object for one style key. It supports style
* inheritance. To avoid creating new styles the element style can have a parent
Expand Down
53 changes: 38 additions & 15 deletions tsp-typescript-client/src/models/table.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
import { array, assertNumber, createNormalizer } from '../protocol/serialization';

export const ColumnHeaderEntry = createNormalizer<ColumnHeaderEntry>({
id: assertNumber,
});

/**
* Column header
*/
Expand All @@ -23,6 +29,31 @@ export interface ColumnHeaderEntry {
type: string;
}

export const Cell = createNormalizer<Cell>({
tags: assertNumber,
});

/**
* Cell inside a table line
*/
export interface Cell {
/**
* Content of the cell, can use markdown for formatting
*/
content: string;

/**
* Tag associated to the cell, used when the cell pass a filter
*/
tags?: number;
}

export const Line = createNormalizer<Line>({
cells: array(Cell),
index: assertNumber,
tags: assertNumber,
});

/**
* Line of a table
*/
Expand All @@ -40,23 +71,15 @@ export interface Line {
/**
* Tag associated to the line, used when the line pass a filter
*/
tags: number;
tags?: number;
}

/**
* Cell inside a table line
*/
export interface Cell {
/**
* Content of the cell, can use markdown for formatting
*/
content: string;

/**
* Tag associated to the cell, used when the cell pass a filter
*/
tags: number;
}
export const TableModel = createNormalizer<TableModel>({
columnIds: array(assertNumber),
lines: array(Line),
lowIndex: assertNumber,
size: assertNumber,
});

/**
* Model of a table
Expand Down
Loading

0 comments on commit f6f2d26

Please sign in to comment.