diff --git a/common/changes/@itwin/core-common/issue-9530-bindidset-null_2026-07-22-19-25.json b/common/changes/@itwin/core-common/issue-9530-bindidset-null_2026-07-22-19-25.json new file mode 100644 index 000000000000..18d3cd788b86 --- /dev/null +++ b/common/changes/@itwin/core-common/issue-9530-bindidset-null_2026-07-22-19-25.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@itwin/core-common", + "comment": "QueryBinder.bindIdSet now throws a descriptive ITwinError (scope \"itwin-QueryBinder\", key \"invalid-arguments\") when an entry is not a valid Id64String, instead of an uncaught TypeError.", + "type": "none" + } + ], + "packageName": "@itwin/core-common" +} diff --git a/core/common/src/ConcurrentQuery.ts b/core/common/src/ConcurrentQuery.ts index 5876f01bb2e9..720895595a77 100644 --- a/core/common/src/ConcurrentQuery.ts +++ b/core/common/src/ConcurrentQuery.ts @@ -5,7 +5,7 @@ /** @packageDocumentation * @module iModels */ -import { BentleyError, CompressedId64Set, DbResult, Id64, Id64String, OrderedId64Iterable } from "@itwin/core-bentley"; +import { BentleyError, CompressedId64Set, DbResult, Id64, Id64String, ITwinError, OrderedId64Iterable } from "@itwin/core-bentley"; import { LowAndHighXYZ, Point2d, Point3d, Range3d } from "@itwin/core-geometry"; import { Base64 } from "js-base64"; @@ -478,15 +478,29 @@ export class QueryBinder { * @param indexOrName Specify parameter index or its name used in ECSQL statement. * @param val @type OrderedId64Iterable value to bind to ECSQL statement. * @returns @type QueryBinder to allow fluent interface. + * @throws an [[ITwinError]] with scope `"itwin-QueryBinder"` and key `"invalid-arguments"` if any entry is not a valid [Id64String]($bentley). */ public bindIdSet(indexOrName: string | number, val: OrderedId64Iterable) { this.verify(indexOrName); const name = String(indexOrName); - OrderedId64Iterable.uniqueIterator(val); + const ids: Id64String[] = []; + // `string` is an Iterable. In that case assume caller passed a single Id64String, matching CompressedId64Set.sortAndCompress. + const iterable = typeof val === "string" ? [val] : val; + for (const id of iterable) { + if (typeof id !== "string" || !Id64.isValidId64(id)) { + ITwinError.throwError({ + message: `QueryBinder.bindIdSet: entry ${JSON.stringify(id)} for parameter "${name}" is not a valid Id64String`, + iTwinErrorId: { scope: "itwin-QueryBinder", key: "invalid-arguments" }, + }); + } + ids.push(id); + } + + OrderedId64Iterable.sortArray(ids); Object.defineProperty(this._args, name, { enumerable: true, value: { type: QueryParamType.IdSet, - value: CompressedId64Set.sortAndCompress(OrderedId64Iterable.uniqueIterator(val)), + value: CompressedId64Set.compressArray(ids), }, }); return this; diff --git a/core/common/src/test/ConcurrentQuery.test.ts b/core/common/src/test/ConcurrentQuery.test.ts index ad0af51f4971..79974cba9cf2 100644 --- a/core/common/src/test/ConcurrentQuery.test.ts +++ b/core/common/src/test/ConcurrentQuery.test.ts @@ -7,7 +7,7 @@ import { Point2d, Point3d, Range3d } from "@itwin/core-geometry"; import { assert, describe, it } from "vitest"; import { Base64 } from "js-base64"; import { QueryBinder, QueryParamType } from "../ConcurrentQuery"; -import { Id64String } from "@itwin/core-bentley"; +import { Id64String, ITwinError } from "@itwin/core-bentley"; describe("QueryBinder", () => { it("binds values", async () => { @@ -111,6 +111,110 @@ describe("QueryBinder", () => { ); }); + describe("bindIdSet invalid entries", () => { + const invalidIds = [undefined, null, "0", "50", "", "not an id", 123, {}, ["0x1"]]; + const cases = invalidIds.flatMap((invalidId) => [ + { label: `${JSON.stringify(invalidId)} as the first entry`, ids: [invalidId, "0x22bd8"] }, + { label: `${JSON.stringify(invalidId)} as the last entry`, ids: ["0x22bd8", invalidId] }, + { label: `${JSON.stringify(invalidId)} as the only entry`, ids: [invalidId] }, + ]); + + for (const { label, ids } of cases) { + it(`throws an ITwinError with ${label}`, () => { + const queryBinder = new QueryBinder(); + let thrown: unknown; + try { + queryBinder.bindIdSet("idSetValue", ids as unknown as Id64String[]); + } catch (error) { + thrown = error; + } + assert.isDefined(thrown, "expected bindIdSet to throw"); + assert.isTrue(ITwinError.isError(thrown, "itwin-QueryBinder", "invalid-arguments"), 'expected an ITwinError with scope "itwin-QueryBinder" and key "invalid-arguments"'); + }); + } + + it("throws when a single invalid Id64String (not wrapped in an array) is passed directly", () => { + const queryBinder = new QueryBinder(); + assert.throws(() => queryBinder.bindIdSet("idSetValue", "not an id")); + }); + + it("does not bind a value when it throws", () => { + const queryBinder = new QueryBinder(); + assert.throws(() => queryBinder.bindIdSet("idSetValue", ["0x22bd8", undefined] as unknown as Id64String[])); + assert.deepEqual(queryBinder.serialize(), {}); + }); + + it("includes the offending value and the parameter name in the error message", () => { + const queryBinder = new QueryBinder(); + try { + queryBinder.bindIdSet("idSetValue", ["0x22bd8", "not an id"]); + assert.fail("expected bindIdSet to throw"); + } catch (error) { + if (!ITwinError.isError(error, "itwin-QueryBinder", "invalid-arguments")) + throw error; + assert.include(error.message, "\"not an id\""); + assert.include(error.message, "idSetValue"); + } + }); + + it("distinguishes a string entry from an array entry in the error message", () => { + const queryBinder = new QueryBinder(); + try { + queryBinder.bindIdSet("idSetValue", [["0x1"]] as unknown as Id64String[]); + assert.fail("expected bindIdSet to throw"); + } catch (error) { + if (!ITwinError.isError(error, "itwin-QueryBinder", "invalid-arguments")) + throw error; + // JSON.stringify(["0x1"]) preserves the brackets; naive string interpolation would collapse it to "0x1", indistinguishable from a valid id. + assert.include(error.message, "[\"0x1\"]"); + } + }); + }); + + it("bindIdSet accepts an empty iterable", () => { + const queryBinder = new QueryBinder(); + queryBinder.bindIdSet("idSetValue", []); + assert.deepEqual(queryBinder.serialize(), { + idSetValue: { + type: QueryParamType.IdSet, + value: "", + }, + }); + }); + + it("bindIdSet accepts any Iterable, not just arrays", () => { + const queryBinder = new QueryBinder(); + queryBinder.bindIdSet("idSetValue", new Set(["0x22bd9", "0x22bd8"])); + assert.deepEqual(queryBinder.serialize(), { + idSetValue: { + type: QueryParamType.IdSet, + value: "+22BD8+1", + }, + }); + }); + + it("bindIdSet sorts and deduplicates ids", () => { + const queryBinder = new QueryBinder(); + queryBinder.bindIdSet("idSetValue", ["0x22bd9", "0x22bd8", "0x22bd8"]); + assert.deepEqual(queryBinder.serialize(), { + idSetValue: { + type: QueryParamType.IdSet, + value: "+22BD8+1", + }, + }); + }); + + it("bindIdSet treats a single Id64String as a single id, not a string of characters", () => { + const queryBinder = new QueryBinder(); + queryBinder.bindIdSet("idSetValue", "0x22bd8"); + assert.deepEqual(queryBinder.serialize(), { + idSetValue: { + type: QueryParamType.IdSet, + value: "+22BD8", + }, + }); + }); + it("allows bulk binding", () => { assert.deepEqual(QueryBinder.from(undefined), new QueryBinder()); diff --git a/docs/changehistory/NextVersion.md b/docs/changehistory/NextVersion.md index 7745b914edd4..0506009c05ee 100644 --- a/docs/changehistory/NextVersion.md +++ b/docs/changehistory/NextVersion.md @@ -3,3 +3,12 @@ publish: false --- # NextVersion +- [NextVersion](#nextversion) + - [@itwin/core-common](#itwincore-common) + - [QueryBinder.bindIdSet now throws on invalid ids](#querybinderbindidset-now-throws-on-invalid-ids) + +## @itwin/core-common + +### QueryBinder.bindIdSet now throws on invalid ids + +[QueryBinder.bindIdSet]($common) previously threw an uncaught `TypeError` when given an entry that was not a valid [Id64String]($bentley) (for example `undefined` or `null`, which can occur when collecting ids from a nullable column via [ECSqlReader]($common)). It now throws a descriptive [ITwinError]($bentley) instead, identifiable via `ITwinError.isError(error, "itwin-QueryBinder", "invalid-arguments")`, so callers can catch and diagnose the invalid entry rather than encountering an opaque internal error.