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

Fixed onDeleteModel "Missing Id" and "ForeignKey constraint" errors #104

Merged
merged 5 commits into from
Aug 7, 2023
Merged
Show file tree
Hide file tree
Changes from 3 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,7 @@
{
"type": "patch",
"comment": "Added a fix for \"Missing id\" and \"ForeignKey constraint\" errors while using onDeleteModel",
"packageName": "@itwin/imodel-transformer",
"email": "deividas.davidavicius@bentley.com",
"dependentChangeType": "patch"
}
33 changes: 31 additions & 2 deletions packages/transformer/src/IModelTransformer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -983,8 +983,37 @@ export class IModelTransformer extends IModelExportHandler {
// - If only the model is deleted, [[initFromExternalSourceAspects]] will have already remapped the underlying element since it still exists.
// - If both were deleted, [[remapDeletedSourceElements]] will find and remap the deleted element making this operation valid
const targetModelId: Id64String = this.context.findTargetElementId(sourceModelId);
if (Id64.isValidId64(targetModelId)) {
this.importer.deleteModel(targetModelId);
try {
MichaelBelousov marked this conversation as resolved.
Show resolved Hide resolved
if (Id64.isValidId64(targetModelId)) {
if (this.exporter.sourceDbChanges?.element.deleteIds.has(sourceModelId)) {
const targetPartitionElement = this.targetDb.elements.tryGetElement(targetModelId);
MichaelBelousov marked this conversation as resolved.
Show resolved Hide resolved
const isDefinitionPartition = targetPartitionElement && targetPartitionElement instanceof DefinitionPartition;
if (isDefinitionPartition) {
// Skipping model deletion because model's partition will also be deleted.
// It expects that model will be present and will fail if it's missing.
// Model will be deleted when its partition will be deleted.
return;
}
}
this.importer.deleteModel(targetModelId);
}
} catch (error) {
const isDeletionProhibitedErr = error instanceof IModelError && (error.errorNumber === IModelStatus.DeletionProhibited || error.errorNumber === IModelStatus.ForeignKeyConstraint);
if (!isDeletionProhibitedErr)
throw error;

// Transformer tries to delete models before it deletes elements. Definition models cannot be deleted unless all of their modeled elements are deleted first.
// In case a definition model needs to be deleted we need to skip it for now and register its modeled partition for deletion.
// The `OnDeleteElement` calls `DeleteElementTree` Which deletes the model together with its partition after deleting all of the modeled elements.
this.scheduleModeledPartitionDeletion(sourceModelId);
}
}

/** Schedule modeled partition deletion */
private scheduleModeledPartitionDeletion(sourceModelId: Id64String): void {
const deletedElements = this.exporter.sourceDbChanges?.element.deleteIds as Set<Id64String>;
MichaelBelousov marked this conversation as resolved.
Show resolved Hide resolved
if (!deletedElements.has(sourceModelId)) {
deletedElements.add(sourceModelId);
}
}

Expand Down
38 changes: 37 additions & 1 deletion packages/transformer/src/test/TestUtils/IModelTestUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ import { AccessToken, BeEvent, DbResult, Guid, GuidString, Id64, Id64String, IMo
import {
AuxCoordSystem2dProps, Base64EncodedString, ChangesetIdWithIndex, Code, CodeProps, CodeScopeSpec, CodeSpec, ColorDef, ElementAspectProps,
ElementProps, Environment, ExternalSourceProps, GeometricElement2dProps, GeometryParams, GeometryPartProps, GeometryStreamBuilder,
GeometryStreamProps, ImageSourceFormat, IModel, IModelError, IModelReadRpcInterface, IModelVersion, IModelVersionProps, LocalFileName,
GeometryStreamProps, ImageSourceFormat, IModel, IModelError, IModelReadRpcInterface, IModelVersion, IModelVersionProps, InformationPartitionElementProps, LocalFileName,
ModelProps,
PhysicalElementProps, PlanProjectionSettings, RelatedElement, RepositoryLinkProps, RequestNewBriefcaseProps, RpcConfiguration, RpcManager,
RpcPendingResponse, SkyBoxImageType, SubCategoryAppearance, SubCategoryOverride, SyncMode,
} from "@itwin/core-common";
Expand Down Expand Up @@ -1057,6 +1058,29 @@ export class ExtensiveTestScenario {

const physicalObjectId6 = sourceDb.elements.insertElement(physicalObjectProps6);
assert.isTrue(Id64.isValidId64(physicalObjectId6));

// Insert DefinitionPartition1
const definitionPartitionProps1: InformationPartitionElementProps = {
classFullName: DefinitionPartition.classFullName,
model: IModel.repositoryModelId,
parent: new SubjectOwnsPartitionElements(subjectId),
code: Code.createEmpty(),
userLabel: "DefinitionPartition1",
};

const definitionPartitionId1 = sourceDb.elements.insertElement(definitionPartitionProps1);
assert.isTrue(Id64.isValidId64(definitionPartitionId1));

// Insert DefinitionModel1
const definitionModelProps1: ModelProps = {
classFullName: DefinitionModel.classFullName,
modeledElement: { id: definitionPartitionId1 },
parentModel: IModel.repositoryModelId,
};

const definitionModelId1 = sourceDb.models.insertModel(definitionModelProps1);
assert.isTrue(Id64.isValidId64(definitionModelId1));
assert.equal(definitionPartitionId1, definitionModelId1);
}

public static updateDb(sourceDb: IModelDb): void {
Expand Down Expand Up @@ -1125,6 +1149,17 @@ export class ExtensiveTestScenario {
sourceDb.elements.deleteElement(physicalObjectId5);
assert.equal(Id64.invalid, IModelTestUtils.queryByUserLabel(sourceDb, "PhysicalObject5"));

// delete DefinitionModel1
const definitionModelId1 = IModelTestUtils.queryByUserLabel(sourceDb, "DefinitionPartition1");
assert.isTrue(Id64.isValidId64(definitionModelId1));
sourceDb.models.deleteModel(definitionModelId1);

// delete DefinitionPartition1
const definitionPartitionId1 = IModelTestUtils.queryByUserLabel(sourceDb, "DefinitionPartition1");
assert.isTrue(Id64.isValidId64(definitionPartitionId1));
sourceDb.elements.deleteElement(definitionPartitionId1);
assert.equal(Id64.invalid, IModelTestUtils.queryByUserLabel(sourceDb, "DefinitionPartition1"));

// Insert PhysicalObject7
const physicalObjectProps5: PhysicalElementProps = {
classFullName: PhysicalObject.classFullName,
Expand Down Expand Up @@ -1263,6 +1298,7 @@ export class ExtensiveTestScenario {
assert.equal(Id64.invalid, IModelTestUtils.queryByUserLabel(iModelDb, "PhysicalObject3"));
assert.equal(Id64.invalid, IModelTestUtils.queryByUserLabel(iModelDb, "PhysicalObject5"));
assert.equal(Id64.invalid, IModelTestUtils.queryByUserLabel(iModelDb, "PhysicalObject6"));
assert.equal(Id64.invalid, IModelTestUtils.queryByUserLabel(iModelDb, "DefinitionPartition1"));
assert.throws(() => iModelDb.relationships.getInstanceProps(DrawingGraphicRepresentsElement.classFullName, { sourceId: drawingGraphicId2, targetId: physicalObjectId1 }));
assert.isUndefined(iModelDb.elements.queryElementIdByCode(new Code({ spec: informationRecordCodeSpec.id, scope: informationModelId, value: "InformationRecord3" })));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ describe("IModelTransformer", () => {
assert.equal(targetImporter.numModelsUpdated, 0);
assert.equal(targetImporter.numElementsInserted, 1);
assert.equal(targetImporter.numElementsUpdated, 5);
assert.equal(targetImporter.numElementsDeleted, 3);
assert.equal(targetImporter.numElementsDeleted, 4);
assert.equal(targetImporter.numElementAspectsInserted, 0);
assert.equal(targetImporter.numElementAspectsUpdated, 2);
assert.equal(targetImporter.numRelationshipsInserted, 2);
Expand Down Expand Up @@ -264,7 +264,7 @@ describe("IModelTransformer", () => {
branchToMasterTransformer.dispose();
masterDb.saveChanges();
TransformerExtensiveTestScenario.assertUpdatesInDb(masterDb, false);
assert.equal(numBranchElements, count(masterDb, Element.classFullName) - 4); // processAll cannot detect deletes when isReverseSynchronization=true
assert.equal(numBranchElements, count(masterDb, Element.classFullName) - 5); // processAll cannot detect deletes when isReverseSynchronization=true
assert.equal(numBranchRelationships, count(masterDb, ElementRefersToElements.classFullName) - 1); // processAll cannot detect deletes when isReverseSynchronization=true
assert.equal(0, count(masterDb, ExternalSourceAspect.classFullName));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,14 @@ import { assert, expect } from "chai";
import * as path from "path";
import * as semver from "semver";
import {
BisCoreSchema, BriefcaseDb, BriefcaseManager, CategorySelector, deleteElementTree, DisplayStyle3d, Element, ElementOwnsChildElements, ElementRefersToElements,
BisCoreSchema, BriefcaseDb, BriefcaseManager, CategorySelector, DefinitionContainer, DefinitionModel, DefinitionPartition, deleteElementTree, DisplayStyle3d, Element, ElementOwnsChildElements, ElementRefersToElements,
ExternalSourceAspect, GenericSchema, HubMock, IModelDb, IModelHost, IModelJsFs, IModelJsNative, ModelSelector, NativeLoggerCategory, PhysicalModel,
PhysicalObject, SnapshotDb, SpatialCategory, SpatialViewDefinition, Subject,
PhysicalObject, SnapshotDb, SpatialCategory, SpatialViewDefinition, Subject, SubjectOwnsPartitionElements,
} from "@itwin/core-backend";

import * as TestUtils from "../TestUtils";
import { AccessToken, Guid, GuidString, Id64, Id64String, Logger, LogLevel } from "@itwin/core-bentley";
import { Code, ColorDef, ElementProps, IModel, IModelVersion, SubCategoryAppearance } from "@itwin/core-common";
import { Code, ColorDef, DefinitionElementProps, ElementProps, IModel, IModelVersion, InformationPartitionElementProps, ModelProps, SubCategoryAppearance } from "@itwin/core-common";
import { Point3d, YawPitchRollAngles } from "@itwin/core-geometry";
import { IModelExporter, IModelImporter, IModelTransformer, TransformerLoggerCategory } from "../../transformer";
import {
Expand Down Expand Up @@ -214,11 +214,11 @@ describe("IModelTransformerHub", () => {
// expect some deletes from updateDb
assert.isAtLeast(sourceDbChanges.element.deleteIds.size, 1);
assert.equal(sourceDbChanges.relationship.deleteIds.size, 1);
assert.equal(sourceDbChanges.model.deleteIds.size, 1);
// don't expect other changes from updateDb
assert.equal(sourceDbChanges.codeSpec.updateIds.size, 0);
assert.equal(sourceDbChanges.codeSpec.deleteIds.size, 0);
assert.equal(sourceDbChanges.aspect.deleteIds.size, 0);
assert.equal(sourceDbChanges.model.deleteIds.size, 0);

const transformer = new TestIModelTransformer(sourceDb, targetDb);
await transformer.processChanges({ accessToken });
Expand Down Expand Up @@ -250,10 +250,10 @@ describe("IModelTransformerHub", () => {
assert.isAtLeast(targetDbChanges.element.deleteIds.size, 1);
assert.isAtLeast(targetDbChanges.aspect.deleteIds.size, 1);
assert.equal(targetDbChanges.relationship.deleteIds.size, 1);
assert.equal(targetDbChanges.model.deleteIds.size, 1);
// don't expect other changes from transforming the result of updateDb
assert.equal(targetDbChanges.codeSpec.updateIds.size, 0);
assert.equal(targetDbChanges.codeSpec.deleteIds.size, 0);
assert.equal(targetDbChanges.model.deleteIds.size, 0);
}

const sourceIModelChangeSets = await IModelHost.hubAccess.queryChangesets({ accessToken, iModelId: sourceIModelId });
Expand Down Expand Up @@ -857,5 +857,97 @@ describe("IModelTransformerHub", () => {
await tearDown();
sinon.restore();
});

it("should delete definition elements and models when processing changes", async () => {
let definitionPartitionId1: string;
let definitionPartitionModelId1: string;
let definitionPartitionId2: string;
let definitionPartitionModelId2: string;
let definitionContainerId1: string;
let definitionContainerModelId1: string;

const timeline: Timeline = {
DeividasDavidav marked this conversation as resolved.
Show resolved Hide resolved
0: {
master: {
manualUpdate(db) {
const definitionPartitionProps: InformationPartitionElementProps = {
classFullName: DefinitionPartition.classFullName,
model: IModel.repositoryModelId,
parent: new SubjectOwnsPartitionElements(IModel.rootSubjectId),
code: Code.createEmpty(),
};
definitionPartitionId1 = db.elements.insertElement(definitionPartitionProps);
definitionPartitionId2 = db.elements.insertElement(definitionPartitionProps);

const definitionModelProps1: ModelProps = {
classFullName: DefinitionModel.classFullName,
modeledElement: { id: definitionPartitionId1 },
parentModel: IModel.repositoryModelId,
};
definitionPartitionModelId1 = db.models.insertModel(definitionModelProps1);

const definitionModelProps2: ModelProps = {
classFullName: DefinitionModel.classFullName,
modeledElement: { id: definitionPartitionId2 },
parentModel: IModel.repositoryModelId,
};
definitionPartitionModelId2 = db.models.insertModel(definitionModelProps2);

const definitionContainerProps1: DefinitionElementProps = {
classFullName: DefinitionContainer.classFullName,
model: definitionPartitionModelId1,
code: Code.createEmpty(),
};
definitionContainerId1 = db.elements.insertElement(definitionContainerProps1);

const definitionModelProps3: ModelProps = {
classFullName: DefinitionModel.classFullName,
modeledElement: { id: definitionContainerId1 },
parentModel: definitionPartitionModelId1,
};
definitionContainerModelId1 = db.models.insertModel(definitionModelProps3);
},
},
},
1: { branch: { branch: "master" } },
2: {
master: {
manualUpdate(db) {
db.models.deleteModel(definitionContainerModelId1);
db.models.deleteModel(definitionPartitionModelId1);
},
},
},
3: { branch: { sync: ["master", 2] } },
4: {
master: {
manualUpdate(db) {
db.models.deleteModel(definitionPartitionModelId2);
},
},
},
5: { branch: { sync: ["master", 4] } },
};

const { trackedIModels, tearDown } = await runTimeline(timeline, { iTwinId, accessToken });

const master = trackedIModels.get("master")!;
const branch = trackedIModels.get("branch")!;

expect(master.db.models.tryGetModel(definitionContainerModelId1!)).to.be.undefined;
expect(master.db.elements.tryGetElement(definitionContainerId1!)).to.be.undefined;
expect(master.db.models.tryGetModel(definitionPartitionModelId1!)).to.be.undefined;
expect(master.db.elements.tryGetElement(definitionPartitionId2!)).to.not.be.undefined;
expect(master.db.models.tryGetModel(definitionPartitionModelId2!)).to.be.undefined;

expect(branch.db.models.tryGetModel(definitionContainerModelId1!)).to.be.undefined;
expect(branch.db.elements.tryGetElement(definitionContainerId1!)).to.be.undefined;
expect(branch.db.models.tryGetModel(definitionPartitionModelId1!)).to.be.undefined;
expect(branch.db.elements.tryGetElement(definitionPartitionId2!)).to.not.be.undefined;
expect(branch.db.models.tryGetModel(definitionPartitionModelId2!)).to.be.undefined;

await tearDown();
sinon.restore();
});
});