diff --git a/change/@itwin-imodel-transformer-8c55efdc-a7a7-4194-84a9-982e3cea1e7f.json b/change/@itwin-imodel-transformer-8c55efdc-a7a7-4194-84a9-982e3cea1e7f.json new file mode 100644 index 000000000..f9076cc4b --- /dev/null +++ b/change/@itwin-imodel-transformer-8c55efdc-a7a7-4194-84a9-982e3cea1e7f.json @@ -0,0 +1,7 @@ +{ + "type": "minor", + "comment": "support changing exclude criteria alongside entity recreations.", + "packageName": "@itwin/imodel-transformer", + "email": "22119573+nick4598@users.noreply.github.com", + "dependentChangeType": "patch" +} diff --git a/common/api/imodel-transformer.api.md b/common/api/imodel-transformer.api.md index 5bf124715..610084f8f 100644 --- a/common/api/imodel-transformer.api.md +++ b/common/api/imodel-transformer.api.md @@ -292,6 +292,7 @@ export class IModelTransformer extends IModelExportHandler { onExportModel(sourceModel: Model): void; onExportRelationship(sourceRelationship: Relationship): void; onExportSchema(schema: ECSchemaMetaData.Schema): Promise; + onSkipElement(elementId: Id64String): void; onTransformElement(sourceElement: Element_2): ElementProps; protected onTransformElementAspect(sourceElementAspect: ElementAspect): ElementAspectProps; onTransformModel(sourceModel: Model, targetModeledElementId: Id64String): ModelProps; diff --git a/packages/transformer/src/IModelTransformer.ts b/packages/transformer/src/IModelTransformer.ts index 60a8144ea..273156ae4 100644 --- a/packages/transformer/src/IModelTransformer.ts +++ b/packages/transformer/src/IModelTransformer.ts @@ -380,6 +380,14 @@ export class IModelTransformer extends IModelExportHandler { public readonly context: IModelCloneContext; private _syncType?: SyncType; + // The following two maps are used to aid in entity recreation when changing exclude criteria. A recreated entity is one which has been deleted in one changeset and then re-inserted in another changeset. They both would point to the same targetId by virtue of having the same federationGuid. + // If the transformer is processing changesets which include an entity recreation and IModelExporter.excludeElement is called on the re-inserted entity id then the initial delete should propagate to the target. + // These maps aid in propagating the initial delete. + /** a map of the id in the target to the id of the re-inserted entity id */ + private _targetIdToSourceInsertId = new Map(); + /** a map of the inserted entity Id to the deleted entity id. Note these ids are different but they are considered the same entity due to having the same federationguid. */ + private _sourceInsertIdToSourceDeleteId = new Map(); + /** The Id of the Element in the **target** iModel that represents the **source** repository as a whole and scopes its [ExternalSourceAspect]($backend) instances. */ public get targetScopeElementId(): Id64String { return this._options.targetScopeElementId; @@ -2375,6 +2383,19 @@ export class IModelTransformer extends IModelExportHandler { } } + /** Override of [IModelExportHandler.onSkipElement]($transformer) that is called when [IModelExporter]($transformer) excludes an element from being exported. */ + public override onSkipElement(elementId: Id64String): void { + if (this._sourceInsertIdToSourceDeleteId.has(elementId)) { + const deleteId = this._sourceInsertIdToSourceDeleteId.get(elementId)!; + Logger.logInfo( + TransformerLoggerCategory.IModelTransformer, + "Adding element back to sourceDbChanges.deleteIds because it was excluded from being exported and detected as an entity recreation.", + { elementId, deleteId } + ); + this.exporter.sourceDbChanges?.element.deleteIds.add(deleteId); + } + } + /** Override of [IModelExportHandler.onDeleteRelationship]($transformer) that is called when [IModelExporter]($transformer) detects that a [Relationship]($backend) has been deleted from the source iModel. * This override propagates the delete to the target iModel via [IModelImporter.deleteRelationship]($transformer). */ @@ -2820,8 +2841,13 @@ export class IModelTransformer extends IModelExportHandler { const targetElementId = this.context.findTargetElementId( insertedSourceElementId ); - if (Id64.isValid(targetElementId)) + if (Id64.isValid(targetElementId)) { alreadyImportedElementInserts.add(targetElementId); + this._targetIdToSourceInsertId.set( + targetElementId, + insertedSourceElementId + ); + } } ); this.exporter.sourceDbChanges?.model.insertIds.forEach( @@ -3021,6 +3047,19 @@ export class IModelTransformer extends IModelExportHandler { this.exporter.sourceDbChanges?.element.deleteIds.delete( changedInstanceId ); + const insertId = this._targetIdToSourceInsertId.get(targetId!); + // All IDs in alreadyImportedElementInserts are also added as a key to targetIdtoSourceInsertId map. + nodeAssert(insertId); + this._sourceInsertIdToSourceDeleteId.set(insertId, changedInstanceId); + Logger.logTrace( + TransformerLoggerCategory.IModelTransformer, + "Element recreation detected.", + { + targetId, + idWhenDeleted: changedInstanceId, + idWhenInserted: insertId, + } + ); } if (alreadyImportedModelInserts.has(targetId!)) { this.exporter.sourceDbChanges?.model.deleteIds.delete( diff --git a/packages/transformer/src/test/TestUtils/TimelineTestUtil.ts b/packages/transformer/src/test/TestUtils/TimelineTestUtil.ts index fd9cff0b1..23fb2c45a 100644 --- a/packages/transformer/src/test/TestUtils/TimelineTestUtil.ts +++ b/packages/transformer/src/test/TestUtils/TimelineTestUtil.ts @@ -34,6 +34,7 @@ import { } from "../IModelTransformerUtils"; import { IModelTestUtils } from "./IModelTestUtils"; import { omit } from "@itwin/core-bentley"; +import { IModelExporter } from "../../IModelExporter"; const saveAndPushChanges = async ( accessToken: string, @@ -253,7 +254,15 @@ export type TimelineStateChange = source: string, opts?: { since?: number; - initTransformer?: (transformer: IModelTransformer) => void; + init?: { + initTransformer?: (transformer: IModelTransformer) => void; + initExporterBeforeTransformerInitialize?: ( + exporter: IModelExporter + ) => Promise; + initExporterAfterTransformerInitialize?: ( + exporter: IModelExporter + ) => Promise; + }; expectThrow?: boolean; assert?: { afterProcessChanges?: (transformer: IModelTransformer) => void; @@ -357,7 +366,15 @@ export async function runTimeline( src: string, opts: { since?: number; - initTransformer?: (transformer: IModelTransformer) => void; + init?: { + initTransformer?: (transformer: IModelTransformer) => void; + initExporterBeforeTransformerInitialize?: ( + exporter: IModelExporter + ) => Promise; + initExporterAfterTransformerInitialize?: ( + exporter: IModelExporter + ) => Promise; + }; expectThrow?: boolean; assert?: { afterProcessChanges?: (transformer: IModelTransformer) => void; @@ -498,7 +515,7 @@ export async function runTimeline( syncSource, { since: startIndex, - initTransformer, + init: initFxns, expectThrow, assert: assertFxns, }, @@ -520,7 +537,18 @@ export async function runTimeline( : { index: undefined }, }, }); - initTransformer?.(syncer); + await initFxns?.initExporterBeforeTransformerInitialize?.( + syncer.exporter + ); + + initFxns?.initTransformer?.(syncer); + if (initFxns?.initExporterAfterTransformerInitialize) { + // only call initialize if initExporterAfterTransformerInitialize is set, otherwise process() will take care of it. + // The expectThrow logic currently relies on process calling initialize + await initFxns?.initExporterAfterTransformerInitialize?.( + syncer.exporter + ); + } try { await syncer.process(); expect( diff --git a/packages/transformer/src/test/standalone/IModelTransformerHub.test.ts b/packages/transformer/src/test/standalone/IModelTransformerHub.test.ts index 814caa23a..5cf517d17 100644 --- a/packages/transformer/src/test/standalone/IModelTransformerHub.test.ts +++ b/packages/transformer/src/test/standalone/IModelTransformerHub.test.ts @@ -1302,7 +1302,11 @@ describe("IModelTransformerHub", () => { master: { sync: [ "branch1", - { initTransformer: setForceOldRelationshipProvenanceMethod }, + { + init: { + initTransformer: setForceOldRelationshipProvenanceMethod, + }, + }, ], }, }, // first master<-branch1 reverse sync picking up new relationship from branch imodel @@ -1346,7 +1350,11 @@ describe("IModelTransformerHub", () => { branch1: { sync: [ "master", - { initTransformer: setForceOldRelationshipProvenanceMethod }, + { + init: { + initTransformer: setForceOldRelationshipProvenanceMethod, + }, + }, ], }, }, // forward sync master->branch1 to pick up delete of relationship @@ -2975,6 +2983,101 @@ describe("IModelTransformerHub", () => { } }); + for (const excludeBeforeTransformerInitialize of [true, false]) { + it.only(`should be able to handle entity recreation and a switching of exclude criteria ${excludeBeforeTransformerInitialize ? "before transformer initialize" : "after transformer initialize"}`, async () => { + // Create imodel, insert element + // Fork iModel + // push two changesets to source iModel, first one deletes element. second inserts element with same fed guid as the deleted element. + // Run transformation calling excludeElement on the inserted element id. + // expect that the delete from the first changeset makes its way to the target iModel. + + const syncMasterToBranch = excludeBeforeTransformerInitialize + ? { + branch: { + sync: [ + "master", + { + init: { + initExporterBeforeTransformerInitialize: async ( + exporter: IModelExporter + ) => { + exporter.excludeElement(idOfElementAfterRecreation); + }, + }, + }, + ], + }, + } + : { + branch: { + sync: [ + "master", + { + init: { + initExporterAfterTransformerInitialize: async ( + exporter: IModelExporter + ) => { + exporter.excludeElement(idOfElementAfterRecreation); + }, + }, + }, + ], + }, + }; + + let propsOfElementToRecreate: ElementProps; + let idOfElementAfterRecreation: Id64String; + + const timeline: Timeline = { + 0: { master: { 1: 1 } }, + 1: { branch: { branch: "master" } }, + 2: { + assert({ master, branch }) { + const sourceId = IModelTestUtils.queryByUserLabel(master.db, "1"); + expect(sourceId).to.not.be.undefined; + const elem = master.db.elements.getElement(sourceId); + expect(elem).to.not.be.undefined; + expect(elem.federationGuid).to.not.be.undefined; + propsOfElementToRecreate = elem.toJSON(); + expect(IModelTestUtils.queryByUserLabel(branch.db, "1")).to.not.be + .undefined; + }, + }, + 3: { master: { 1: deleted } }, + 4: { + master: { + manualUpdate(db) { + delete propsOfElementToRecreate.id; + propsOfElementToRecreate.userLabel = "recreated"; + idOfElementAfterRecreation = db.elements.insertElement( + propsOfElementToRecreate + ); + expect(idOfElementAfterRecreation).to.not.be.undefined; + }, + }, + }, + 5: syncMasterToBranch as any, + 6: { + assert({ branch }) { + const elem = IModelTestUtils.queryByUserLabel(branch.db, "1"); + const newElemLabel = IModelTestUtils.queryByUserLabel( + branch.db, + "recreated" + ); // we changed the userlabel in master so make sure that didnt make it to target + expect(elem).to.equal(Id64.invalid); + expect(newElemLabel).to.equal(Id64.invalid); + }, + }, + }; + + const { tearDown } = await runTimeline(timeline, { + iTwinId, + accessToken, + }); + await tearDown(); + }); + } + it("should preserve FederationGuid when element is recreated within the same changeset and across changesets", async () => { const sourceIModelName: string = IModelTransformerTestUtils.generateUniqueName("Source"); @@ -3797,7 +3900,10 @@ describe("IModelTransformerHub", () => { "branch", { expectThrow: false, - initTransformer: setBranchRelationshipDataBehaviorToUnsafeMigrate, + init: { + initTransformer: + setBranchRelationshipDataBehaviorToUnsafeMigrate, + }, }, ], }, @@ -3894,7 +4000,10 @@ describe("IModelTransformerHub", () => { sync: [ "branch", { - initTransformer: setBranchRelationshipDataBehaviorToUnsafeMigrate, + init: { + initTransformer: + setBranchRelationshipDataBehaviorToUnsafeMigrate, + }, }, ], }, @@ -3947,7 +4056,10 @@ describe("IModelTransformerHub", () => { sync: [ "master", { - initTransformer: setBranchRelationshipDataBehaviorToUnsafeMigrate, + init: { + initTransformer: + setBranchRelationshipDataBehaviorToUnsafeMigrate, + }, }, ], }, @@ -4033,9 +4145,11 @@ describe("IModelTransformerHub", () => { sync: [ "branch", { - initTransformer: (transformer) => - (transformer["_options"]["branchRelationshipDataBehavior"] = - "unsafe-migrate"), + init: { + initTransformer: (transformer) => + (transformer["_options"]["branchRelationshipDataBehavior"] = + "unsafe-migrate"), + }, }, ], // Sync again with no changes except for ones which may get made by unsafe-migrate. }, @@ -4174,7 +4288,10 @@ describe("IModelTransformerHub", () => { "master", { expectThrow: false, - initTransformer: setBranchRelationshipDataBehaviorToUnsafeMigrate, + init: { + initTransformer: + setBranchRelationshipDataBehaviorToUnsafeMigrate, + }, }, ], }, @@ -4185,7 +4302,10 @@ describe("IModelTransformerHub", () => { "branch", { expectThrow: false, - initTransformer: setBranchRelationshipDataBehaviorToUnsafeMigrate, + init: { + initTransformer: + setBranchRelationshipDataBehaviorToUnsafeMigrate, + }, }, ], },