Skip to content
Open
1 change: 1 addition & 0 deletions common/api/core-common.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -1740,6 +1740,7 @@ export enum CommonLoggerCategory {
Annotations = "core-common.Annotations",
ElementProps = "core-common.ElementProps",
Geometry = "core-common.Geometry",
QueryBinder = "core-common.QueryBinder",
RpcInterfaceBackend = "core-backend.RpcInterface",
RpcInterfaceFrontend = "core-frontend.RpcInterface"
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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"
}
2 changes: 2 additions & 0 deletions core/common/src/CommonLoggerCategory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ export enum CommonLoggerCategory {
ElementProps = "core-common.ElementProps",
/** The logger category used by common classes relating to Geometry. */
Geometry = "core-common.Geometry",
/** The logger category used by [QueryBinder]($common). */
QueryBinder = "core-common.QueryBinder",
Comment thread
anmolshres98 marked this conversation as resolved.
Outdated
/** The logger category used by the portions of the RpcInterface framework that run on the backend. */
RpcInterfaceBackend = "core-backend.RpcInterface",
/** The logger category used by the portions of the RpcInterface framework that run on the frontend. */
Expand Down
20 changes: 17 additions & 3 deletions core/common/src/ConcurrentQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -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<string>. 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<ITwinError>({
message: `QueryBinder.bindIdSet: "${id}" is not a valid Id64String`,
Comment thread
anmolshres98 marked this conversation as resolved.
Outdated
iTwinErrorId: { scope: "itwin-QueryBinder", key: "invalid-arguments" },
});
}
ids.push(id);
}
Comment thread
anmolshres98 marked this conversation as resolved.
Comment thread
anmolshres98 marked this conversation as resolved.

OrderedId64Iterable.sortArray(ids);
Object.defineProperty(this._args, name, {
enumerable: true, value: {
type: QueryParamType.IdSet,
value: CompressedId64Set.sortAndCompress(OrderedId64Iterable.uniqueIterator(val)),
value: CompressedId64Set.compressArray(ids),
},
Comment thread
anmolshres98 marked this conversation as resolved.
});
return this;
Expand Down
92 changes: 91 additions & 1 deletion core/common/src/test/ConcurrentQuery.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down Expand Up @@ -111,6 +111,96 @@ 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 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");
}
});
});

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<Id64String>, 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());

Expand Down
8 changes: 8 additions & 0 deletions docs/changehistory/NextVersion.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ publish: false
- [Backend-to-frontend IPC invoke](#backend-to-frontend-ipc-invoke)
- [@itwin/core-backend](#itwincore-backend)
- [ChangesetReader.setBatchSize](#changesetreadersetbatchsize)
- [@itwin/core-common](#itwincore-common)
- [QueryBinder.bindIdSet now throws on invalid ids](#querybinderbindidset-now-throws-on-invalid-ids)
- [@itwin/core-geometry](#itwincore-geometry)
- [Region Boolean enhancements](#region-boolean-enhancements)

Expand Down Expand Up @@ -104,6 +106,12 @@ while (reader.step()) { /* ... */ }
| SqliteBackedCache | 1,000 | 0.399 | 0.207 | 48.1% |
| SqliteBackedCache | 10,000 | 3.342 | 1.981 | 40.7% |

## @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.

## @itwin/core-geometry

### Region Boolean enhancements
Expand Down
Loading