Skip to content

Commit

Permalink
Fix error when converting multiple units parallel with the same UnitC…
Browse files Browse the repository at this point in the history
…onverter instance (#7623)
  • Loading branch information
StefanApfel-Bentley authored Jan 30, 2025
1 parent e085ce8 commit 2718c66
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 9 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@itwin/ecschema-metadata",
"comment": "Fix for error occuring when converting multiple units with the same UnitConverter instance",
"type": "none"
}
],
"packageName": "@itwin/ecschema-metadata"
}
12 changes: 12 additions & 0 deletions core/ecschema-metadata/src/UnitConversion/UnitTree.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ export class GraphUtils {
/** @internal */
export class UnitGraph {
private _graph = new Graph<Unit | Constant>();
private _unitsInProgress = new Map<string, Promise<void>>();

constructor(private _context: SchemaContext) {
this._graph.setGraph("Unit tree processor");
Expand Down Expand Up @@ -120,13 +121,24 @@ export class UnitGraph {
* @param unit Current unit to be added to graph
*/
public async addUnit(unit: Unit | Constant): Promise<void> {
if(this._unitsInProgress.has(unit.key.fullName))
return this._unitsInProgress.get(unit.key.fullName);

if (this._graph.hasNode(unit.key.fullName))
return;

this._graph.setNode(unit.key.fullName, unit);
if (this.isIdentity(unit))
return;

const promise = this.addUnitToGraph(unit);
this._unitsInProgress.set(unit.key.fullName, promise);

await promise
.finally(() => this._unitsInProgress.delete(unit.key.fullName));
}

private async addUnitToGraph(unit: Unit | Constant) {
const umap = parseDefinition(unit.definition);

const promiseArray: Promise<[Unit | Constant, DefinitionFragment]>[] = [];
Expand Down
31 changes: 22 additions & 9 deletions core/ecschema-metadata/src/test/UnitConversion/Convert.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,18 +30,31 @@ describe("Unit Conversion tests", () => {
deserializeXmlSync(schemaXml, context);
});

async function convertAndVerifyTestData(test: TestData, converter: UnitConverter) {
const fromFullName = `Units:${test.from}`;
const toFullName = `Units:${test.to}`;
const map = await converter.calculateConversion(fromFullName, toFullName);
const actual = map.evaluate(test.input);
expect(
almostEqual(test.expect, actual, tolerance),
`${test.input} ${test.from} in ${test.to} should be ${test.expect}
and not ${actual} error = ${Math.abs(test.expect - actual)} > ${tolerance}`,
).to.be.true;
}

testData.forEach((test: TestData) => {
it(`should convert ${test.from} to ${test.to}`, async () => {
const converter = new UnitConverter(context);
const fromFullName = `Units:${test.from}`;
const toFullName = `Units:${test.to}`;
const map = await converter.calculateConversion(fromFullName, toFullName);
const actual = map.evaluate(test.input);
expect(
almostEqual(test.expect, actual, tolerance),
`${test.input} ${test.from} in ${test.to} should be ${test.expect}
and not ${actual} error = ${Math.abs(test.expect - actual)} > ${tolerance}`,
).to.be.true;
await convertAndVerifyTestData(test, converter);
});
});

it(`should convert units parallel`, async () => {
const converter = new UnitConverter(context);
const conversionPromises = testData.map(async (test: TestData) => {
await convertAndVerifyTestData(test, converter);
});

await Promise.all(conversionPromises);
});
});

0 comments on commit 2718c66

Please sign in to comment.