diff --git a/change/@itwin-imodel-transformer-635c5a66-27ec-4eae-9501-ca020a3b90aa.json b/change/@itwin-imodel-transformer-635c5a66-27ec-4eae-9501-ca020a3b90aa.json new file mode 100644 index 00000000..d5fde13c --- /dev/null +++ b/change/@itwin-imodel-transformer-635c5a66-27ec-4eae-9501-ca020a3b90aa.json @@ -0,0 +1,7 @@ +{ + "type": "prerelease", + "comment": "add more cases to support preserveElementIdsForFiltering options", + "packageName": "@itwin/imodel-transformer", + "email": "Daniel.Erbynn@bentley.com", + "dependentChangeType": "patch" +} diff --git a/packages/transformer/src/IModelImporter.ts b/packages/transformer/src/IModelImporter.ts index ccbd7463..a858b8c7 100644 --- a/packages/transformer/src/IModelImporter.ts +++ b/packages/transformer/src/IModelImporter.ts @@ -103,6 +103,20 @@ export class IModelImporter { */ private _duplicateCodeValueMap: Map; + /** + * A set of elementIds that the transformer adds to while exporting elements to indicate that the element already exists in the target. + * Defaults to an empty set. + * @note + * + * This is used as an optimization when `[[IModelTransformOptions.preserveElementIdsForFiltering]]` is set to `true` + * In normal cases where this option set to `false`, + * the importer determines whether to insert or update based off of whether the ID is defined on the `elementProps` passed to `importElement`. + * However, with `preserveElementIdsForFiltering` set to `true`, IDs are always set, so we can't determine insert/update like the normal case. + * The transformer already knows if an element exists or not by the time `importElement` is called and pushes to this set with `markElementToUpdateForPreserveId`. + * @note This set should stay small, as right after the transformer pushes to it, the importer will remove from the set. + */ + private _elementsToUpdateDuringPreserveIds = new Set([]); + /** The set of elements that should not be updated by this IModelImporter. * Defaults to an empty set. * @note Adding an element to this set is typically necessary when remapping a source element to one that already exists in the target and already has the desired properties. @@ -155,6 +169,16 @@ export class IModelImporter { return false; } + /** + * Marks an element so that it can be updated during import when [[IModelTransformOptions.preserveElementIdsForFiltering]] is set to true. + */ + public markElementToUpdateDuringPreserveIds(elementId: Id64String) { + if (this._rootElementIds.has(elementId)) { + return; + } + this._elementsToUpdateDuringPreserveIds.add(elementId); + } + /** Import the specified ModelProps (either as an insert or an update) into the target iModel. */ public importModel(modelProps: ModelProps): void { if (undefined === modelProps.id || !Id64.isValidId64(modelProps.id)) @@ -227,6 +251,29 @@ export class IModelImporter { return `${modelProps.classFullName} [${modelProps.id!}]`; } + /** + * Tries to update an element with the specified element properties. + * If a duplicate code error occurs, it assigns a new unique code value and retries the update + */ + private tryUpdateElement(elemProps: ElementProps) { + try { + this.onUpdateElement(elemProps); + } catch (err) { + if ((err as IModelError).errorNumber === IModelStatus.DuplicateCode) { + assert( + elemProps.code.value !== undefined, + "NULL code values are always considered unique and cannot clash" + ); + this._duplicateCodeValueMap.set(elemProps.id!, elemProps.code.value); + // Using NULL code values as an alternative is not valid because definition elements cannot have NULL code values. + elemProps.code.value = Guid.createValue(); + this.onUpdateElement(elemProps); + } else { + throw err; + } + } + } + /** Import the specified ElementProps (either as an insert or an update) into the target iModel. */ public importElement(elementProps: ElementProps): Id64String { if ( @@ -239,6 +286,7 @@ export class IModelImporter { ); return elementProps.id; } + if (this.options.preserveElementIdsForFiltering) { if (elementProps.id === undefined) { throw new IModelError( @@ -246,40 +294,27 @@ export class IModelImporter { "elementProps.id must be defined during a preserveIds operation" ); } + // Categories are the only element that onInserted will immediately insert a new element (their default subcategory) // since default subcategories always exist and always will be inserted after their categories, we treat them as an update // to prevent duplicate inserts. // Always present elements (0xe, 0x1, 0x10) also will be updated to prevent duplicate inserts. - // Otherwise we always insert during a preserveElementIdsForFiltering operation if ( (isSubCategory(elementProps) && isDefaultSubCategory(elementProps)) || this._rootElementIds.has(elementProps.id) ) { this.onUpdateElement(elementProps); } else { - this.onInsertElement(elementProps); + if (this._elementsToUpdateDuringPreserveIds.has(elementProps.id)) { + this.tryUpdateElement(elementProps); + this._elementsToUpdateDuringPreserveIds.delete(elementProps.id); + } else { + this.onInsertElement(elementProps); + } } } else { if (undefined !== elementProps.id) { - try { - this.onUpdateElement(elementProps); - } catch (err) { - if ((err as IModelError).errorNumber === IModelStatus.DuplicateCode) { - assert( - elementProps.code.value !== undefined, - "NULL code values are always considered unique and cannot clash" - ); - this._duplicateCodeValueMap.set( - elementProps.id, - elementProps.code.value - ); - // Using NULL code values as an alternative is not valid because definition elements cannot have NULL code values. - elementProps.code.value = Guid.createValue(); - this.onUpdateElement(elementProps); - } else { - throw err; - } - } + this.tryUpdateElement(elementProps); } else { this.onInsertElement(elementProps); // targetElementProps.id assigned by insertElement } diff --git a/packages/transformer/src/IModelTransformer.ts b/packages/transformer/src/IModelTransformer.ts index ebe66cd9..bcfb2579 100644 --- a/packages/transformer/src/IModelTransformer.ts +++ b/packages/transformer/src/IModelTransformer.ts @@ -1790,10 +1790,7 @@ export class IModelTransformer extends IModelExportHandler { public override onExportElement(sourceElement: Element): void { let targetElementId: Id64String; let targetElementProps: ElementProps; - if (this._options.preserveElementIdsForFiltering) { - targetElementId = sourceElement.id; - targetElementProps = this.onTransformElement(sourceElement); - } else if (this._options.wasSourceIModelCopiedToTarget) { + if (this._options.wasSourceIModelCopiedToTarget) { targetElementId = sourceElement.id; targetElementProps = this.targetDb.elements.getElementProps(targetElementId); @@ -1854,6 +1851,33 @@ export class IModelTransformer extends IModelExportHandler { ? targetElementId : undefined; + if (this._options.preserveElementIdsForFiltering) { + const isValid = Id64.isValid(targetElementId); + if (isValid && targetElementId !== sourceElement.id) { + // Element found with different id + throw new Error( + `Element id(${sourceElement.id}) cannot be preserved. Found a different mapping(${targetElementId}) from source element` + ); + } else if (isValid && targetElementId === sourceElement.id) { + // targetElementId is valid (indicating update) + this.importer.markElementToUpdateDuringPreserveIds(sourceElement.id); + } else if (!isValid) { + const sourceInTargetElemProps = + this.targetDb.elements.tryGetElementProps(sourceElement.id); + + // if we don't find mapping for source element in target(invalid) but another element with source id exists in target + if (sourceInTargetElemProps) { + // Element id is already taken by another element + throw new Error( + `Element id(${sourceElement.id}) cannot be preserved. An unrelated element in the target already uses id: ${sourceElement.id}` + ); + } else { + // Element id in target is available to be remapped + targetElementProps.id = sourceElement.id; + } + } + } + if (!this._options.wasSourceIModelCopiedToTarget) { this.importer.importElement(targetElementProps); // don't need to import if iModel was copied } diff --git a/packages/transformer/src/test/standalone/IModelTransformer.test.ts b/packages/transformer/src/test/standalone/IModelTransformer.test.ts index 9389d8e1..bf361923 100644 --- a/packages/transformer/src/test/standalone/IModelTransformer.test.ts +++ b/packages/transformer/src/test/standalone/IModelTransformer.test.ts @@ -2655,6 +2655,320 @@ describe("IModelTransformer", () => { } } + it("process() with preserveElementIdsForFiltering set to true should re-add deleted element with same id", async () => { + const sourceDbPath = IModelTransformerTestUtils.prepareOutputFile( + "IModelTransformer", + "PreserveIdOnTestModel-Source.bim" + ); + const sourceDb = SnapshotDb.createEmpty(sourceDbPath, { + rootSubject: { name: "iModelA" }, + }); + Subject.insert(sourceDb, IModel.rootSubjectId, "Subject1"); + sourceDb.saveChanges(); + + const targetDbPath = IModelTransformerTestUtils.prepareOutputFile( + "IModelTransformer", + "PreserveIdOnTestModel-Target.bim" + ); + const targetDb = SnapshotDb.createEmpty(targetDbPath, { + rootSubject: sourceDb.rootSubject, + }); + + // Execute process() so that elements from source are copied to target + const transformer = new IModelTransformer(sourceDb, targetDb, { + preserveElementIdsForFiltering: true, + }); + await transformer.process(); + targetDb.saveChanges(); + + let sourceContent = await getAllElementsInvariants(sourceDb); + let targetContent = await getAllElementsInvariants(targetDb); + expect(targetContent).to.deep.equal(sourceContent); + + // Delete subject 1 from target + const code1 = Subject.createCode( + targetDb, + IModel.rootSubjectId, + "Subject1" + ); + const targetSubjectId1 = targetDb.elements.queryElementIdByCode(code1); + expect(targetSubjectId1).to.not.be.undefined; + targetDb.elements.deleteElement(targetSubjectId1!); + targetDb.saveChanges(); + + // Calling process() for second time with option to preserve elements in hopes of restoring deleted element + const secondTransformer = new IModelTransformer(sourceDb, targetDb, { + preserveElementIdsForFiltering: true, + }); + await secondTransformer.process(); // should not throw error: duplicate code (65547) and should re-add deleted element + targetDb.saveChanges(); + + // verify that deleted element in target is added back - redundant check for explicitness + const sourceElementJSON = sourceDb.elements + .getElement(targetSubjectId1!) + .toJSON(); + const deletedElementInTargetJSON = targetDb.elements + .getElement(targetSubjectId1!) + .toJSON(); + expect(sourceElementJSON).to.be.deep.equal(deletedElementInTargetJSON); + + sourceContent = await getAllElementsInvariants(sourceDb); + targetContent = await getAllElementsInvariants(targetDb); + expect(targetContent).to.deep.equal(sourceContent); + + sourceDb.close(); + targetDb.close(); + }); + + it("process() with preserveElementIdsForFiltering set to true should update the element properties if element exists with desired id in target", async () => { + const sourceDbPath = IModelTransformerTestUtils.prepareOutputFile( + "IModelTransformer", + "PreserveIdOnTestModel-Source.bim" + ); + const sourceDb = SnapshotDb.createEmpty(sourceDbPath, { + rootSubject: { name: "iModelA" }, + }); + const sourceSubjectId = Subject.insert( + sourceDb, + IModel.rootSubjectId, + "Subject1" + ); + sourceDb.saveChanges(); + + const targetDbPath = IModelTransformerTestUtils.prepareOutputFile( + "IModelTransformer", + "PreserveIdOnTestModel-Target.bim" + ); + const targetDb = SnapshotDb.createEmpty(targetDbPath, { + rootSubject: sourceDb.rootSubject, + }); + + // Execute process() so that elements from source are copied to target + const transformer = new IModelTransformer(sourceDb, targetDb, { + preserveElementIdsForFiltering: true, + }); + await transformer.process(); + targetDb.saveChanges(); + + let sourceContent = await getAllElementsInvariants(sourceDb); + let targetContent = await getAllElementsInvariants(targetDb); + expect(targetContent).to.deep.equal(sourceContent); + + // update subject in source + const sourceSubject = + sourceDb.elements.getElement(sourceSubjectId); + const updatedDescription = "Subject1 Updated Description"; + sourceSubject.description = updatedDescription; + sourceDb.elements.updateElement(sourceSubject.toJSON()); + sourceDb.saveChanges(); + + // Calling process() for second time with option to preserve elements in hopes of updating element with desired id + const secondTransformer = new IModelTransformer(sourceDb, targetDb, { + preserveElementIdsForFiltering: true, + }); + await secondTransformer.process(); // should update description for subject element + targetDb.saveChanges(); + + // target subject should have updated description + const targetSubjectDescription = + targetDb.elements.getElement(sourceSubjectId).description; + expect(targetSubjectDescription).to.equal(updatedDescription); + + sourceContent = await getAllElementsInvariants(sourceDb); + targetContent = await getAllElementsInvariants(targetDb); + expect(targetContent).to.deep.equal(sourceContent); + + sourceDb.close(); + targetDb.close(); + }); + + it("process() with preserveElementIdsForFiltering set to true should not throw when called on 2 identical iModels", async () => { + const seedDb = SnapshotDb.openFile( + TestUtils.IModelTestUtils.resolveAssetFile("CompatibilityTestSeed.bim") + ); + const sourceDbPath = IModelTransformerTestUtils.prepareOutputFile( + "IModelTransformer", + "PreserveIdOnTestModel-Source.bim" + ); + // transforming the seed to an empty will update it to the latest bis from the new target + // which minimizes differences we'd otherwise need to filter later + const sourceDb = SnapshotDb.createEmpty(sourceDbPath, { + rootSubject: seedDb.rootSubject, + }); + const seedTransformer = new IModelTransformer(seedDb, sourceDb); + await seedTransformer.process(); + sourceDb.saveChanges(); + + const targetDbPath = IModelTransformerTestUtils.prepareOutputFile( + "IModelTransformer", + "PreserveIdOnTestModel-Target.bim" + ); + const targetDb = SnapshotDb.createEmpty(targetDbPath, { + rootSubject: sourceDb.rootSubject, + }); + + // Calling process() for first time will add all elements from source to target + const transformer = new IModelTransformer(sourceDb, targetDb, { + preserveElementIdsForFiltering: true, + }); + await transformer.process(); + targetDb.saveChanges(); + + // should not throw error: duplicate code (65547) + const thirdTransformer = new IModelTransformer(sourceDb, targetDb, { + preserveElementIdsForFiltering: true, + }); + await thirdTransformer.process(); + targetDb.saveChanges(); + + const sourceContent = await getAllElementsInvariants(sourceDb); + const targetContent = await getAllElementsInvariants(targetDb); + expect(targetContent).to.deep.equal(sourceContent); + + sourceDb.close(); + targetDb.close(); + }); + + it("process() with preserveElementIdsForFiltering set to true should throw error if element exists but has different id in target", async () => { + const sourceDbPath = IModelTransformerTestUtils.prepareOutputFile( + "IModelTransformer", + "PreserveIdOnTestModel-Source.bim" + ); + const sourceDb = SnapshotDb.createEmpty(sourceDbPath, { + rootSubject: { name: "iModelA" }, + }); + Subject.insert(sourceDb, IModel.rootSubjectId, "Subject1"); + sourceDb.saveChanges(); + + const targetDbPath = IModelTransformerTestUtils.prepareOutputFile( + "IModelTransformer", + "PreserveIdOnTestModel-Target.bim" + ); + const targetDb = SnapshotDb.createEmpty(targetDbPath, { + rootSubject: sourceDb.rootSubject, + }); + + // Execute process() so that elements from source are copied to target + const transformer = new IModelTransformer(sourceDb, targetDb, { + preserveElementIdsForFiltering: true, + }); + await transformer.process(); + targetDb.saveChanges(); + + const sourceContent = await getAllElementsInvariants(sourceDb); + const targetContent = await getAllElementsInvariants(targetDb); + expect(targetContent).to.deep.equal(sourceContent); + + // Delete subject 1 from target + const code = Subject.createCode(targetDb, IModel.rootSubjectId, "Subject1"); + const targetSubjectId = targetDb.elements.queryElementIdByCode(code); + expect(targetSubjectId).to.not.be.undefined; + + targetDb.elements.deleteElement(targetSubjectId!); + targetDb.saveChanges(); + + // save subject 1 element properties for new subject(it should have same fed guid and code) + const targetSubjectProps = sourceDb.elements.getElementProps( + targetSubjectId! + ); + targetSubjectProps.id = undefined; + assert.isDefined(targetSubjectProps); + + // create new subject that is the same as subject 1 but has a different Id + const newSubjectId = targetDb.elements.insertElement(targetSubjectProps); + expect(newSubjectId).to.not.be.undefined; + targetDb.saveChanges(); + + // Calling process() for second time with option to preserve elements in hopes of throwing expected error + const secondTransformer = new IModelTransformer(sourceDb, targetDb, { + preserveElementIdsForFiltering: true, + }); + + await expect(secondTransformer.process()).to.be.rejectedWith( + `Element id(${targetSubjectId}) cannot be preserved. Found a different mapping(${newSubjectId}) from source element` + ); + + sourceDb.close(); + targetDb.close(); + }); + + it("process() with preserveElementIdsForFiltering set to true should throw error if an unrelated element in the target already uses id", async () => { + const sourceDbPath = IModelTransformerTestUtils.prepareOutputFile( + "IModelTransformer", + "PreserveIdOnTestModel-Source.bim" + ); + const sourceDb = SnapshotDb.createEmpty(sourceDbPath, { + rootSubject: { name: "iModelA" }, + }); + Subject.insert(sourceDb, IModel.rootSubjectId, "Subject1"); + sourceDb.saveChanges(); + + const targetDbPath = IModelTransformerTestUtils.prepareOutputFile( + "IModelTransformer", + "PreserveIdOnTestModel-Target.bim" + ); + const targetDb = SnapshotDb.createEmpty(targetDbPath, { + rootSubject: sourceDb.rootSubject, + }); + + // Execute process() so that elements from source are copied to target + const transformer = new IModelTransformer(sourceDb, targetDb, { + preserveElementIdsForFiltering: true, + }); + await transformer.process(); + targetDb.saveChanges(); + + const sourceContent = await getAllElementsInvariants(sourceDb); + const targetContent = await getAllElementsInvariants(targetDb); + expect(targetContent).to.deep.equal(sourceContent); + + // Delete subject 1 from target + const code1 = Subject.createCode( + targetDb, + IModel.rootSubjectId, + "Subject1" + ); + const targetSubjectId1 = targetDb.elements.queryElementIdByCode(code1); + expect(targetSubjectId1).to.not.be.undefined; + + targetDb.elements.deleteElement(targetSubjectId1!); + targetDb.saveChanges(); + + // save subject 1 element properties but only use the same id + const newPropsForSubject3 = sourceDb.elements.getElementProps( + targetSubjectId1! + ); + newPropsForSubject3.federationGuid = undefined; + const code3 = Subject.createCode( + targetDb, + IModel.rootSubjectId, + "Subject3" + ); + newPropsForSubject3.code = code3; + + // insert an unrelated element that uses same id as subject1 + // insertElement public api does not support forceUseId option + // eslint-disable-next-line @itwin/no-internal, deprecation/deprecation + const targetSubjectId3 = targetDb.nativeDb.insertElement( + newPropsForSubject3, + { forceUseId: true } + ); + expect(targetSubjectId3).to.not.be.undefined; + targetDb.saveChanges(); + + // Calling process() for second time with option to preserve elements in hopes of of throwing expected error + const secondTransformer = new IModelTransformer(sourceDb, targetDb, { + preserveElementIdsForFiltering: true, + }); + + await expect(secondTransformer.process()).to.be.rejectedWith( + `Element id(${targetSubjectId1}) cannot be preserved. An unrelated element in the target already uses id: ${targetSubjectId1}` + ); + + sourceDb.close(); + targetDb.close(); + }); + it("reference deletion is considered invalid when danglingReferencesBehavior='reject' and that is the default", async () => { const sourceDbPath = IModelTransformerTestUtils.prepareOutputFile( "IModelTransformer",