From 41a3bfbba118c3f39bd5e6f264230e83986ad87e Mon Sep 17 00:00:00 2001 From: Tim Jennison Date: Thu, 30 Jan 2025 19:45:50 +0000 Subject: [PATCH] Fix errors caused by numeric IDs that are too big for JS numbers * Numbers in JS are doubles by default. This generally works fine for numeric data but can cause truncation for IDs that use the entire space of an int64. * The JSON functions can't correctly handle the large numbers without some change to the format. The easiest is to send int64s as strings. * Those numbers are converted to BigInts to make them numeric but retain their precision. This requires using them throughout the code since our keys/ids are generally int64s. * BigInts don't follow the same coercion rules as numbers, so many places that previously just worked (e.g. data maps, JSX keys) no longer do. Convert data maps to actual Map objects and explicitly convert keys to strings. * The change to Maps exposes some clunkiness in the way TreeGrid data is handled. Sticking to a simple conversion for now but will consider other changes in a followup. --- .../controller/objmapping/FromApiUtils.java | 7 +- .../app/controller/objmapping/ToApiUtils.java | 5 +- .../main/resources/api/service_openapi.yaml | 4 +- ui/codegen.sh | 2 +- ui/src/addByCode.tsx | 2 +- ui/src/addCohort.tsx | 2 +- ui/src/addCriteria.tsx | 27 ++++---- ui/src/cohortReview/cohortReview.tsx | 19 +++--- ui/src/cohortReview/cohortReviewList.tsx | 4 +- ui/src/cohortReview/participantsList.tsx | 15 +++-- .../cohortReview/plugins/occurrenceTable.tsx | 21 +++--- ui/src/cohortReview/plugins/textSearch.tsx | 2 +- ui/src/components/treegrid.test.tsx | 67 ++++++++++++------- ui/src/components/treegrid.tsx | 37 ++++++---- ui/src/criteria/attribute.tsx | 2 +- ui/src/criteria/classification.tsx | 54 ++++++++------- ui/src/criteria/filterableGroup.tsx | 5 +- ui/src/criteria/survey.tsx | 51 ++++++++------ ui/src/data/source.tsx | 31 +++++---- ui/src/data/types.ts | 5 +- ui/src/export.tsx | 13 ++-- ui/src/fakeApis.ts | 6 +- ui/src/featureSet/addFeatureSet.tsx | 2 +- ui/src/featureSet/featureSet.tsx | 12 ++-- ui/src/sampleApp/studiesList.tsx | 8 +-- ui/src/sampleApp/studyOverview.tsx | 8 +-- ui/src/viz/viz.ts | 13 +++- 27 files changed, 245 insertions(+), 179 deletions(-) diff --git a/service/src/main/java/bio/terra/tanagra/app/controller/objmapping/FromApiUtils.java b/service/src/main/java/bio/terra/tanagra/app/controller/objmapping/FromApiUtils.java index 4b5ba60a2..c1c8569ae 100644 --- a/service/src/main/java/bio/terra/tanagra/app/controller/objmapping/FromApiUtils.java +++ b/service/src/main/java/bio/terra/tanagra/app/controller/objmapping/FromApiUtils.java @@ -503,7 +503,12 @@ private static ValueDisplayField buildRelationshipField( public static Literal fromApiObject(ApiLiteral apiLiteral) { return switch (apiLiteral.getDataType()) { - case INT64 -> Literal.forInt64(apiLiteral.getValueUnion().getInt64Val()); + // JavaScript can't handle the full int64 range when parsing JSON, so parse them as strings. + case INT64 -> + Literal.forInt64( + apiLiteral.getValueUnion().getInt64Val() != null + ? Long.parseLong(apiLiteral.getValueUnion().getInt64Val()) + : null); case STRING -> Literal.forString(apiLiteral.getValueUnion().getStringVal()); case BOOLEAN -> Literal.forBoolean(apiLiteral.getValueUnion().isBoolVal()); case DATE -> Literal.forDate(apiLiteral.getValueUnion().getDateVal()); diff --git a/service/src/main/java/bio/terra/tanagra/app/controller/objmapping/ToApiUtils.java b/service/src/main/java/bio/terra/tanagra/app/controller/objmapping/ToApiUtils.java index 3f254f1c8..b2a20a4cb 100644 --- a/service/src/main/java/bio/terra/tanagra/app/controller/objmapping/ToApiUtils.java +++ b/service/src/main/java/bio/terra/tanagra/app/controller/objmapping/ToApiUtils.java @@ -90,7 +90,10 @@ public static ApiLiteral toApiObject(Literal literal) { new ApiLiteral().dataType(ApiDataType.fromValue(literal.getDataType().name())); return switch (literal.getDataType()) { case INT64 -> - apiLiteral.valueUnion(new ApiLiteralValueUnion().int64Val(literal.getInt64Val())); + // JavaScript can't handle the full int64 range when parsing JSON, so send them as + // strings. + apiLiteral.valueUnion( + new ApiLiteralValueUnion().int64Val(literal.getInt64Val().toString())); case STRING -> apiLiteral.valueUnion(new ApiLiteralValueUnion().stringVal(literal.getStringVal())); case BOOLEAN -> diff --git a/service/src/main/resources/api/service_openapi.yaml b/service/src/main/resources/api/service_openapi.yaml index 0a791ecbc..909e9711f 100644 --- a/service/src/main/resources/api/service_openapi.yaml +++ b/service/src/main/resources/api/service_openapi.yaml @@ -1522,8 +1522,8 @@ components: boolVal: type: boolean int64Val: - type: integer - format: int64 + # Transfer int64s as strings to avoid overflowing JavaScript's number precision. + type: string stringVal: type: string dateVal: diff --git a/ui/codegen.sh b/ui/codegen.sh index 87c4bb256..f4b162c38 100755 --- a/ui/codegen.sh +++ b/ui/codegen.sh @@ -7,4 +7,4 @@ openapi-generator-cli generate -i ../service/src/main/resources/api/service_open # Generate plugin proto code. rm -rf src/proto mkdir src/proto -npx protoc --proto_path=../underlay/src/main/proto/ --plugin=./node_modules/.bin/protoc-gen-ts_proto --ts_proto_out=./src/proto/ --ts_proto_opt=esModuleInterop=true --ts_proto_opt=outputClientImpl=false ../underlay/src/main/proto/criteriaselector/dataschema/*.proto ../underlay/src/main/proto/criteriaselector/configschema/*.proto ../underlay/src/main/proto/viz/*.proto +npx protoc --proto_path=../underlay/src/main/proto/ --plugin=./node_modules/.bin/protoc-gen-ts_proto --ts_proto_out=./src/proto/ --ts_proto_opt=esModuleInterop=true --ts_proto_opt=outputClientImpl=false --ts_proto_opt=forceLong=bigint ../underlay/src/main/proto/criteriaselector/dataschema/*.proto ../underlay/src/main/proto/criteriaselector/configschema/*.proto ../underlay/src/main/proto/viz/*.proto diff --git a/ui/src/addByCode.tsx b/ui/src/addByCode.tsx index 1ba25c85c..54ff37858 100644 --- a/ui/src/addByCode.tsx +++ b/ui/src/addByCode.tsx @@ -204,7 +204,7 @@ export function AddByCode() { return undefined; } - const item = data[id]?.data as LookupEntryItem; + const item = data.get(id)?.data as LookupEntryItem; if (!item) { return undefined; } diff --git a/ui/src/addCohort.tsx b/ui/src/addCohort.tsx index d6350fea8..40dc87749 100644 --- a/ui/src/addCohort.tsx +++ b/ui/src/addCohort.tsx @@ -117,7 +117,7 @@ export function AddCohort() { return undefined; } - const cohortData = data[id].data as CohortData; + const cohortData = data.get(id)?.data as CohortData; if (!cohortData) { return undefined; } diff --git a/ui/src/addCriteria.tsx b/ui/src/addCriteria.tsx index a2fe0c468..0bb903992 100644 --- a/ui/src/addCriteria.tsx +++ b/ui/src/addCriteria.tsx @@ -21,7 +21,6 @@ import Loading from "components/loading"; import { Search } from "components/search"; import { TreeGrid, - TreeGridData, TreeGridId, TreeGridItem, TreeGridRowData, @@ -353,9 +352,9 @@ function AddCriteria(props: AddCriteriaProps) { const search = useCallback(async () => { const children: DataKey[] = []; - const data: TreeGridData = { - root: { data: {}, children }, - }; + const data = new Map([ + ["root", { data: {}, children }], + ]); if (query) { const res = await searchCriteria( @@ -377,13 +376,13 @@ function AddCriteria(props: AddCriteriaProps) { }, entry: entry, }; - data[key] = item; + data.set(key, item); }); } return data; }, [underlaySource, query, selectedOptions, optionsMap]); - const searchState = useSWRImmutable( + const searchState = useSWRImmutable>( { component: "AddCriteria", underlayName: underlay.name, @@ -523,7 +522,7 @@ function AddCriteria(props: AddCriteriaProps) { {!!query ? ( - {!searchState.data?.root?.children?.length ? ( + {!searchState.data?.get("root")?.children?.length ? (