Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Throw SchemaMergingError if mergeing of two schemas failed #7273

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@itwin/ecschema-editing",
"comment": "",
"type": "none"
}
],
"packageName": "@itwin/ecschema-editing"
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export class SchemaDifferenceWalker {
/**
* Calls the appropriate method on the visitor based on the schema difference type.
*/
private async visit(difference: AnySchemaDifference, index: number, array: AnySchemaDifference[]) {
protected async visit(difference: AnySchemaDifference, index: number, array: AnySchemaDifference[]) {
switch (difference.schemaType) {
case SchemaOtherTypes.Schema:
return this._visitor.visitSchemaDifference(difference, index, array);
Expand Down
69 changes: 51 additions & 18 deletions core/ecschema-editing/src/Merging/SchemaMerger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import { Schema, type SchemaContext, SchemaKey } from "@itwin/ecschema-metadata";
import { SchemaContextEditor } from "../Editing/Editor";
import { SchemaConflictsError } from "../Differencing/Errors";
import { getSchemaDifferences, type SchemaDifferenceResult } from "../Differencing/SchemaDifference";
import { AnySchemaDifference, getSchemaDifferences, type SchemaDifferenceResult } from "../Differencing/SchemaDifference";
import { SchemaMergingVisitor } from "./SchemaMergingVisitor";
import { SchemaMergingWalker } from "./SchemaMergingWalker";
import type { SchemaEdits } from "./Edits/SchemaEdits";
Expand Down Expand Up @@ -80,28 +80,61 @@ export class SchemaMerger {
);
}

const schema = await this._editor.getSchema(targetSchemaKey).catch((error: Error) => {
if (error instanceof SchemaEditingError && error.errorNumber === ECEditingStatus.SchemaNotFound) {
throw new Error(`The target schema '${targetSchemaKey.name}' could not be found in the editing context.`);
const mergedDifferences: AnySchemaDifference[] = [];

try {
const schema = await this._editor.getSchema(targetSchemaKey).catch((error: Error) => {
if (error instanceof SchemaEditingError && error.errorNumber === ECEditingStatus.SchemaNotFound) {
throw new Error(`The target schema '${targetSchemaKey.name}' could not be found in the editing context.`);
}
throw error;
});

if (!schema.customAttributes || !schema.customAttributes.has("CoreCustomAttributes.DynamicSchema")) {
throw new Error(`The target schema '${targetSchemaKey.name}' is not dynamic. Only dynamic schemas are supported for merging.`);
}
throw error;
});

if (!schema.customAttributes || !schema.customAttributes.has("CoreCustomAttributes.DynamicSchema")) {
throw new Error(`The target schema '${targetSchemaKey.name}' is not dynamic. Only dynamic schemas are supported for merging.`);
const visitor = new SchemaMergingVisitor({
editor: this._editor,
targetSchema: schema,
targetSchemaKey,
sourceSchemaKey,
});

const walker = new SchemaMergingWalker(visitor);
walker.on("mergedDifference", (difference) => mergedDifferences.push(difference));

await walker.traverse(differenceResult.differences, "add");
await walker.traverse(differenceResult.differences, "modify");

return schema;
} catch(error: any) {
throw new SchemaMergingError("An error occurred while merging the schemas.", error, mergedDifferences);
}
}
}

const visitor = new SchemaMergingVisitor({
editor: this._editor,
targetSchema: schema,
targetSchemaKey,
sourceSchemaKey,
});
/**
* Error class that provides additional information why a schema merge failed.
* @beta
*/
export class SchemaMergingError extends Error {

const walker = new SchemaMergingWalker(visitor);
await walker.traverse(differenceResult.differences, "add");
await walker.traverse(differenceResult.differences, "modify");
/** The differences that have been already merged into the target schema. */
public readonly mergedDifferences: ReadonlyArray<AnySchemaDifference>;

return schema;
/** The error that occurred while merging the schemas. */
public readonly mergeError: Error;

/**
* Initializes a new instance of the SchemaMergingError class.
* @param message The error message.
* @param mergeError The error that occurred while merging the schemas.
* @param mergedDifferences The differences that have been already merged into the target schema.
*/
constructor(message: string, mergeError: Error, mergedDifferences: ReadonlyArray<AnySchemaDifference>) {
super(message);
this.mergedDifferences = mergedDifferences;
this.mergeError = mergeError;
}
}
14 changes: 14 additions & 0 deletions core/ecschema-editing/src/Merging/SchemaMergingWalker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@ import { AnySchemaDifference, DifferenceType, SchemaOtherTypes, SchemaType } fro
* @internal
*/
export class SchemaMergingWalker extends SchemaDifferenceWalker {
private _mergeEventHandler?: (difference: AnySchemaDifference) => void;

public on(_event: "mergedDifference", listener: (difference: AnySchemaDifference) => void) {
this._mergeEventHandler = listener;
}

/**
* Traverses the schema differences and calls the appropriate method on the visitor.
Expand Down Expand Up @@ -62,4 +67,13 @@ export class SchemaMergingWalker extends SchemaDifferenceWalker {
...differences.filter(filterByType(SchemaOtherTypes.CustomAttributeInstance)),
]);
}

/**
* Calls the appropriate method on the visitor based on the schema difference type.
* Overrides the base class method to track the merged differences.
*/
protected override async visit(difference: AnySchemaDifference, index: number, array: AnySchemaDifference[]): Promise<void> {
return super.visit(difference, index, array)
.then(() => this._mergeEventHandler && this._mergeEventHandler(difference));
}
}
2 changes: 1 addition & 1 deletion core/ecschema-editing/src/ecschema-editing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export * from "./Differencing/SchemaDifference";
export * from "./Differencing/SchemaConflicts";
export * from "./Differencing/Errors";
export * from "./Differencing/Utils";
export { SchemaMerger } from "./Merging/SchemaMerger";
export { SchemaMerger, SchemaMergingError } from "./Merging/SchemaMerger";
export * from "./Merging/Edits/SchemaEdits";

/** @docs-package-description
Expand Down
16 changes: 12 additions & 4 deletions core/ecschema-editing/src/test/Merging/ConstantMerger.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* See LICENSE.md in the project root for license terms and full copyright notice.
*--------------------------------------------------------------------------------------------*/
import { Constant, Schema, SchemaItemType } from "@itwin/ecschema-metadata";
import { SchemaMerger } from "../../Merging/SchemaMerger";
import { SchemaMerger, SchemaMergingError } from "../../Merging/SchemaMerger";
import { SchemaOtherTypes } from "../../Differencing/SchemaDifference";
import { BisTestHelper } from "../TestUtils/BisTestHelper";
import { expect } from "chai";
Expand Down Expand Up @@ -170,8 +170,10 @@ describe("Constant merger tests", () => {
},
],
});
await expect(merge).to.be.rejectedWith("The Constant testConstant has an invalid 'definition' attribute.");

await expect(merge).to.be.eventually.rejectedWith(SchemaMergingError).then((error) => {
expect(error).has.a.nested.property("mergeError.message", "The Constant testConstant has an invalid 'definition' attribute.");
});
});

it("it should throw error if numerator conflict exist", async () => {
Expand Down Expand Up @@ -213,7 +215,10 @@ describe("Constant merger tests", () => {
},
],
});
await expect(merge).to.be.rejectedWith(Error, "Failed to merged, constant numerator conflict: 5.5 -> 4.5");

await expect(merge).to.be.eventually.rejectedWith(SchemaMergingError).then((error) => {
expect(error).has.a.nested.property("mergeError.message", "Failed to merged, constant numerator conflict: 5.5 -> 4.5");
});
});

it("it should throw error if denominator conflict exist", async () => {
Expand Down Expand Up @@ -255,6 +260,9 @@ describe("Constant merger tests", () => {
},
],
});
await expect(merge).to.be.rejectedWith(Error, "Failed to merged, constant denominator conflict: 5.1 -> 4.2");

await expect(merge).to.be.eventually.rejectedWith(SchemaMergingError).then((error) => {
expect(error).has.a.nested.property("mergeError.message", "Failed to merged, constant denominator conflict: 5.1 -> 4.2");
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* See LICENSE.md in the project root for license terms and full copyright notice.
*--------------------------------------------------------------------------------------------*/
import { CustomAttributeClass, CustomAttributeContainerType, Schema, SchemaContext, SchemaItemType } from "@itwin/ecschema-metadata";
import { SchemaMerger } from "../../Merging/SchemaMerger";
import { SchemaMerger, SchemaMergingError } from "../../Merging/SchemaMerger";
import { expect } from "chai";
import { ECEditingStatus } from "../../Editing/Exception";
import { BisTestHelper } from "../TestUtils/BisTestHelper";
Expand Down Expand Up @@ -172,7 +172,9 @@ describe("CustomAttributeClass merger tests", () => {
],
});

await expect(merge).to.be.rejectedWith("Changing the class 'testCAClass' baseClass is not supported.");
await expect(merge).to.be.eventually.rejectedWith(SchemaMergingError).then((error) => {
expect(error).has.a.nested.property("mergeError.message", "Changing the class 'testCAClass' baseClass is not supported.");
});
});

it("should throw an error when merging custom attribute base class to one that doesn't derive from", async () => {
Expand Down Expand Up @@ -224,10 +226,10 @@ describe("CustomAttributeClass merger tests", () => {
],
});

await expect(merge).to.be.eventually.rejected.then(function (error) {
expect(error).to.have.property("errorNumber", ECEditingStatus.SetBaseClass);
expect(error).to.have.nested.property("innerError.message", `Base class TargetSchema.TestBase must derive from TargetSchema.TargetBase.`);
expect(error).to.have.nested.property("innerError.errorNumber", ECEditingStatus.InvalidBaseClass);
await expect(merge).to.be.eventually.rejectedWith(SchemaMergingError).then((error) => {
expect(error).to.have.nested.property("mergeError.errorNumber", ECEditingStatus.SetBaseClass);
expect(error).to.have.nested.property("mergeError.innerError.message", `Base class TargetSchema.TestBase must derive from TargetSchema.TargetBase.`);
expect(error).to.have.nested.property("mergeError.innerError.errorNumber", ECEditingStatus.InvalidBaseClass);
});
});
});
37 changes: 23 additions & 14 deletions core/ecschema-editing/src/test/Merging/EntityClassMerger.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* See LICENSE.md in the project root for license terms and full copyright notice.
*--------------------------------------------------------------------------------------------*/
import { ECClassModifier, EntityClass, Schema, SchemaContext, SchemaItemKey, SchemaItemType } from "@itwin/ecschema-metadata";
import { SchemaMerger } from "../../Merging/SchemaMerger";
import { SchemaMerger, SchemaMergingError } from "../../Merging/SchemaMerger";
import { SchemaOtherTypes } from "../../Differencing/SchemaDifference";
import { ECEditingStatus } from "../../Editing/Exception";
import { BisTestHelper } from "../TestUtils/BisTestHelper";
Expand Down Expand Up @@ -282,7 +282,10 @@ describe("EntityClass merger tests", () => {
},
],
});
await expect(merge).to.be.rejectedWith("Changing the type of item 'TestClass' not supported.");

await expect(merge).to.be.eventually.rejectedWith(SchemaMergingError).then((error) => {
expect(error).has.a.nested.property("mergeError.message", "Changing the type of item 'TestClass' not supported.");
});
});

it("should throw an error when merging class modifier changed from Abstract to Sealed", async () => {
Expand Down Expand Up @@ -312,7 +315,9 @@ describe("EntityClass merger tests", () => {
],
});

await expect(merge).to.be.rejectedWith("Changing the class 'TestEntity' modifier is not supported.");
await expect(merge).to.be.eventually.rejectedWith(SchemaMergingError).then((error) => {
expect(error).has.a.nested.property("mergeError.message", "Changing the class 'TestEntity' modifier is not supported.");
});
});

it("should throw an error when merging entity base class changed from existing one to undefined", async () => {
Expand Down Expand Up @@ -345,7 +350,9 @@ describe("EntityClass merger tests", () => {
],
});

await expect(merge).to.be.rejectedWith("Changing the class 'TestEntity' baseClass is not supported.");
await expect(merge).to.be.eventually.rejectedWith(SchemaMergingError).then((error) => {
expect(error).has.a.nested.property("mergeError.message", "Changing the class 'TestEntity' baseClass is not supported.");
});
});

it("should throw an error when merging entity base class changed from undefined to existing one", async () => {
Expand Down Expand Up @@ -381,7 +388,9 @@ describe("EntityClass merger tests", () => {
],
});

await expect(merge).to.be.rejectedWith("Changing the class 'TestEntity' baseClass is not supported.");
await expect(merge).to.be.eventually.rejectedWith(SchemaMergingError).then((error) => {
expect(error).has.a.nested.property("mergeError.message", "Changing the class 'TestEntity' baseClass is not supported.");
});
});

it("should throw an error when merging entity base class to one that doesn't derive from", async () => {
Expand Down Expand Up @@ -430,9 +439,9 @@ describe("EntityClass merger tests", () => {
});

await expect(merge).to.be.eventually.rejected.then(function (error) {
expect(error).to.have.property("errorNumber", ECEditingStatus.SetBaseClass);
expect(error).to.have.nested.property("innerError.message", `Base class TargetSchema.TestBase must derive from TargetSchema.TargetBase.`);
expect(error).to.have.nested.property("innerError.errorNumber", ECEditingStatus.InvalidBaseClass);
expect(error).to.have.nested.property("mergeError.errorNumber", ECEditingStatus.SetBaseClass);
expect(error).to.have.nested.property("mergeError.innerError.message", `Base class TargetSchema.TestBase must derive from TargetSchema.TargetBase.`);
expect(error).to.have.nested.property("mergeError.innerError.errorNumber", ECEditingStatus.InvalidBaseClass);
});
});

Expand Down Expand Up @@ -463,9 +472,9 @@ describe("EntityClass merger tests", () => {
});

await expect(merge).to.be.eventually.rejected.then(function (error) {
expect(error).to.have.property("errorNumber", ECEditingStatus.AddMixin);
expect(error).to.have.nested.property("innerError.message", `Mixin TargetSchema.NotExistingMixin could not be found in the schema context.`);
expect(error).to.have.nested.property("innerError.errorNumber", ECEditingStatus.SchemaItemNotFoundInContext);
expect(error).to.have.nested.property("mergeError.errorNumber", ECEditingStatus.AddMixin);
expect(error).to.have.nested.property("mergeError.innerError.message", `Mixin TargetSchema.NotExistingMixin could not be found in the schema context.`);
expect(error).to.have.nested.property("mergeError.innerError.errorNumber", ECEditingStatus.SchemaItemNotFoundInContext);
});
});

Expand Down Expand Up @@ -513,9 +522,9 @@ describe("EntityClass merger tests", () => {
});

await expect(merge).to.be.eventually.rejected.then(function (error) {
expect(error).to.have.property("errorNumber", ECEditingStatus.SetBaseClass);
expect(error).to.have.nested.property("innerError.message", `Base class TargetSchema.TestBase must derive from TargetSchema.BaseMixin.`);
expect(error).to.have.nested.property("innerError.errorNumber", ECEditingStatus.InvalidBaseClass);
expect(error).to.have.nested.property("mergeError.errorNumber", ECEditingStatus.SetBaseClass);
expect(error).to.have.nested.property("mergeError.innerError.message", `Base class TargetSchema.TestBase must derive from TargetSchema.BaseMixin.`);
expect(error).to.have.nested.property("mergeError.innerError.errorNumber", ECEditingStatus.InvalidBaseClass);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* See LICENSE.md in the project root for license terms and full copyright notice.
*--------------------------------------------------------------------------------------------*/
import { Enumeration, Schema, SchemaItemType } from "@itwin/ecschema-metadata";
import { SchemaMerger } from "../../Merging/SchemaMerger";
import { SchemaMerger, SchemaMergingError } from "../../Merging/SchemaMerger";
import { SchemaOtherTypes } from "../../Differencing/SchemaDifference";
import { BisTestHelper } from "../TestUtils/BisTestHelper";
import { expect } from "chai";
Expand Down Expand Up @@ -290,7 +290,9 @@ describe("Enumeration merge tests", () => {
],
});

await expect(merge).to.be.rejectedWith(Error, "The Enumeration TestEnumeration has an incompatible type. It must be \"string\", not \"int\".");
await expect(merge).to.be.eventually.rejectedWith(SchemaMergingError).then((error) => {
expect(error).has.a.nested.property("mergeError.message", "The Enumeration TestEnumeration has an incompatible type. It must be \"string\", not \"int\".");
});
});

it("should throw an error if enumerator value attribute conflict exist", async () => {
Expand Down Expand Up @@ -329,6 +331,8 @@ describe("Enumeration merge tests", () => {
],
});

await expect(merge).to.be.rejectedWith("Failed to merge enumerator attribute, Enumerator \"EnumeratorOne\" has different values.");
await expect(merge).to.be.eventually.rejectedWith(SchemaMergingError).then((error) => {
expect(error).has.a.nested.property("mergeError.message", "Failed to merge enumerator attribute, Enumerator \"EnumeratorOne\" has different values.");
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* See LICENSE.md in the project root for license terms and full copyright notice.
*--------------------------------------------------------------------------------------------*/
import { KindOfQuantity, Schema, SchemaContext, SchemaItemType } from "@itwin/ecschema-metadata";
import { SchemaMerger } from "../../Merging/SchemaMerger";
import { SchemaMerger, SchemaMergingError } from "../../Merging/SchemaMerger";
import { expect } from "chai";
import { SchemaOtherTypes } from "../../Differencing/SchemaDifference";
import { BisTestHelper } from "../TestUtils/BisTestHelper";
Expand Down Expand Up @@ -504,6 +504,8 @@ describe("KindOfQuantity merge tests", () => {
conflicts: undefined,
});

await expect(merge).to.be.rejectedWith("Changing the kind of quantity 'TestKoq' persistenceUnit is not supported.");
await expect(merge).to.be.eventually.rejectedWith(SchemaMergingError).then((error) => {
expect(error).has.a.nested.property("mergeError.message", "Changing the kind of quantity 'TestKoq' persistenceUnit is not supported.");
});
});
});
6 changes: 4 additions & 2 deletions core/ecschema-editing/src/test/Merging/MixinMerger.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* See LICENSE.md in the project root for license terms and full copyright notice.
*--------------------------------------------------------------------------------------------*/
import { Mixin, Schema, SchemaContext, SchemaItemType } from "@itwin/ecschema-metadata";
import { SchemaMerger } from "../../Merging/SchemaMerger";
import { SchemaMerger, SchemaMergingError } from "../../Merging/SchemaMerger";
import { expect } from "chai";
import { BisTestHelper } from "../TestUtils/BisTestHelper";

Expand Down Expand Up @@ -158,6 +158,8 @@ describe("Mixin merger tests", () => {
],
});

await expect(merge).to.be.rejectedWith("Changing the mixin 'TestMixin' appliesTo is not supported.");
await expect(merge).to.be.eventually.rejectedWith(SchemaMergingError).then((error) => {
expect(error).has.a.nested.property("mergeError.message", "Changing the mixin 'TestMixin' appliesTo is not supported.");
});
});
});
Loading
Loading