Add a possibility to re-add deleted elements with preserved ids

packages/transformer/src/IModelImporter.ts

@@ -63,13 +63,6 @@ export interface IModelImportOptions {
    * @see [IModelImporter Options]($docs/learning/transformer/index.md#IModelImporter)
    */
   autoExtendProjectExtents?: boolean | { excludeOutliers: boolean };
-  /**
-   * Indicates whether the importer is used in a transformation process.
-   * If `true`, the importer was passed into the transformer, or the transformer constructed this instance of the importer
-   */
-  fromTransformation?: boolean;
-  /** See [IModelTransformOptions]($transformer) */
-  preserveElementIdsForFiltering?: boolean;
   /** If `true`, simplify the element geometry for visualization purposes. For example, convert b-reps into meshes.
    * @default false
    */
@@ -110,17 +103,17 @@ export class IModelImporter {
 
   /**
    * 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.
+   * Defaults to undefined.
    * @note
    *
-   * This is used as an optimization when `preserveElementIdsForFiltering` is set to `true`
+   * This is used as an optimization when the transformer `preserveElementIdsForFiltering` option 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 `markElementAsUpdate`.
+   * 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 _elementsToUpdate = new Set<Id64String>([]);
+  public elementIdsToUpdateForPreserveId: Set<Id64String> | undefined;
 
   /** The set of elements that should not be updated by this IModelImporter.
    * Defaults to an empty set.
@@ -152,9 +145,6 @@ export class IModelImporter {
     this.targetDb = targetDb;
     this.options = {
       autoExtendProjectExtents: options?.autoExtendProjectExtents ?? true,
-      fromTransformation: options?.fromTransformation ?? false,
-      preserveElementIdsForFiltering:
-        options?.preserveElementIdsForFiltering ?? false,
       simplifyElementGeometry: options?.simplifyElementGeometry ?? false,
       skipPropagateChangesToRootElements:
         options?.skipPropagateChangesToRootElements ?? true,
@@ -176,13 +166,19 @@ export class IModelImporter {
   }
 
   /**
-   * Marks an element so that it can be updated during import when [[IModelImportOptions.preserveElementIdsForFiltering]] is set to true
+   * Marks an element so that it can be updated during import when the [[IModelTransformOptions.preserveElementIdsForFiltering]] is set to true.
    */
-  public markElementAsUpdate(elementId: Id64String) {
+  public markElementToUpdateForPreserveId(elementId: Id64String) {
+    if (!this.elementIdsToUpdateForPreserveId) {
+      throw new Error(
+        "The elementIdsToUpdateForPreserveId set is not initialized. Ensure this set is initialized when `preserveElementIdsForFiltering` is set true"
+      );
+    }
+
     if (this._rootElementIds.has(elementId)) {
       return;
     }
-    this._elementsToUpdate.add(elementId);
+    this.elementIdsToUpdateForPreserveId.add(elementId);
   }
 
   /** Import the specified ModelProps (either as an insert or an update) into the target iModel. */
@@ -257,27 +253,45 @@ export class IModelImporter {
     return `${modelProps.classFullName} [${modelProps.id!}]`;
   }
 
-  /** Import the specified ElementProps (either as an insert or an update) into the target iModel. */
-  public importElement(elementProps: ElementProps): Id64String {
-    const tryUpdateElement = (elemProps: ElementProps): void => {
-      try {
+  /**
+   * Determines whether element IDs should be preserved for filtering.
+   *
+   * This function checks if the `elementIdsToUpdateForPreserveId` set is defined.
+   * If it is undefined, the function returns `false`, indicating that element IDs should not be preserved.
+   * Otherwise, it returns `true`.   *
+   */
+  private shouldPreserveElementIdsForFiltering(): boolean {
+    if (this.elementIdsToUpdateForPreserveId === undefined) {
+      return false;
+    }
+    return true;
+  }
+
+  /**
+   * 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);
-      } 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;
-        }
+      } else {
+        throw err;
       }
-    };
+    }
+  }
 
+  /** Import the specified ElementProps (either as an insert or an update) into the target iModel. */
+  public importElement(elementProps: ElementProps): Id64String {
     if (
       undefined !== elementProps.id &&
       this.doNotUpdateElement(elementProps.id)
@@ -288,7 +302,8 @@ export class IModelImporter {
       );
       return elementProps.id;
     }
-    if (this.options.preserveElementIdsForFiltering) {
+
+    if (this.shouldPreserveElementIdsForFiltering()) {
       if (elementProps.id === undefined) {
         throw new IModelError(
           IModelStatus.BadElement,
@@ -300,30 +315,22 @@ export class IModelImporter {
       // 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 {
-        // if [[IModelImportOptions.fromTransformation]] is false, try and get the element (update if it exists)
-        const shouldUpdateElement =
-          !this.options.fromTransformation &&
-          this.targetDb.elements.tryGetElement(elementProps.id);
-        if (
-          this._elementsToUpdate.has(elementProps.id) ||
-          shouldUpdateElement
-        ) {
-          tryUpdateElement(elementProps);
-          this._elementsToUpdate.delete(elementProps.id);
+        if (this.elementIdsToUpdateForPreserveId?.has(elementProps.id)) {
+          this.tryUpdateElement(elementProps);
+          this.elementIdsToUpdateForPreserveId?.delete(elementProps.id);
         } else {
           this.onInsertElement(elementProps);
         }
       }
     } else {
       if (undefined !== elementProps.id) {
-        tryUpdateElement(elementProps);
+        this.tryUpdateElement(elementProps);
       } else {
         this.onInsertElement(elementProps); // targetElementProps.id assigned by insertElement
       }
@@ -339,7 +346,7 @@ export class IModelImporter {
     /* eslint-disable deprecation/deprecation */
     try {
       const elementId = this.targetDb.nativeDb.insertElement(elementProps, {
-        forceUseId: this.options.preserveElementIdsForFiltering,
+        forceUseId: this.shouldPreserveElementIdsForFiltering(),
       });
       // set the id like [IModelDb.insertElement]($backend), does, the raw nativeDb method does not
       elementProps.id = elementId;

packages/transformer/src/IModelTransformer.ts

@@ -658,8 +658,6 @@ export class IModelTransformer extends IModelExportHandler {
     // initialize importer and targetDb
     if (target instanceof IModelDb) {
       this.importer = new IModelImporter(target, {
-        preserveElementIdsForFiltering:
-          this._options.preserveElementIdsForFiltering,
         skipPropagateChangesToRootElements:
           this._options.skipPropagateChangesToRootElements,
       });
@@ -668,7 +666,6 @@ export class IModelTransformer extends IModelExportHandler {
       this.validateSharedOptionsMatch();
     }
     this.targetDb = this.importer.targetDb;
-    this.importer.options.fromTransformation = true;
     // create the IModelCloneContext, it must be initialized later
     this.context = new IModelCloneContext(this.sourceDb, this.targetDb);
 
@@ -697,14 +694,6 @@ export class IModelTransformer extends IModelExportHandler {
    *  @note This expects that the importer is already set on the transformer.
    */
   private validateSharedOptionsMatch() {
-    if (
-      Boolean(this._options.preserveElementIdsForFiltering) !==
-      this.importer.options.preserveElementIdsForFiltering
-    ) {
-      const errMessage =
-        "A custom importer was passed as a target but its 'preserveElementIdsForFiltering' option is out of sync with the transformer's option.";
-      throw new Error(errMessage);
-    }
     if (
       Boolean(this._options.skipPropagateChangesToRootElements) !==
       this.importer.options.skipPropagateChangesToRootElements
@@ -1943,17 +1932,21 @@ export class IModelTransformer extends IModelExportHandler {
       : undefined;
 
     if (this._options.preserveElementIdsForFiltering) {
-      const isInvalid = !Id64.isValid(targetElementId);
+      // initialize set if undefined to indicate in importer that preserveElementIdsForFiltering option is true
+      if (!this.importer.elementIdsToUpdateForPreserveId) {
+        this.importer.elementIdsToUpdateForPreserveId = new Set<Id64String>([]);
+      }
 
-      if (!isInvalid && targetElementId !== sourceElement.id) {
+      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 (!isInvalid && targetElementId === sourceElement.id) {
+      } else if (isValid && targetElementId === sourceElement.id) {
         // targetElementId is valid (indicating update)
-        this.importer.markElementAsUpdate(sourceElement.id);
-      } else if (isInvalid) {
+        this.importer.markElementToUpdateForPreserveId(sourceElement.id);
+      } else if (!isValid) {
         const sourceInTargetElemProps =
           this.targetDb.elements.tryGetElementProps(sourceElement.id);
 

packages/transformer/src/test/standalone/IModelTransformer.test.ts

@@ -2664,7 +2664,6 @@ describe("IModelTransformer", () => {
       rootSubject: { name: "iModelA" },
     });
     Subject.insert(sourceDb, IModel.rootSubjectId, "Subject1");
-    Subject.insert(sourceDb, IModel.rootSubjectId, "Subject2");
     sourceDb.saveChanges();
 
     const targetDbPath = IModelTransformerTestUtils.prepareOutputFile(
@@ -2697,18 +2696,6 @@ describe("IModelTransformer", () => {
     targetDb.elements.deleteElement(targetSubjectId1!);
     targetDb.saveChanges();
 
-    // update subject 2 in source
-    const code2 = Subject.createCode(
-      sourceDb,
-      IModel.rootSubjectId,
-      "Subject2"
-    );
-    const targetSubjectId2 = sourceDb.elements.queryElementIdByCode(code2);
-    const subject2 = sourceDb.elements.getElement<Subject>(targetSubjectId2!);
-    subject2.description = "Subject2 Updated Description";
-    sourceDb.elements.updateElement(subject2.toJSON());
-    sourceDb.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,
@@ -2718,10 +2705,10 @@ describe("IModelTransformer", () => {
 
     // verify that deleted element in target is added back - redundant check for explicitness
     const sourceElementJSON = sourceDb.elements
-      .tryGetElement<Subject>(targetSubjectId1!)
+      .getElement<Subject>(targetSubjectId1!)
       ?.toJSON();
     const deletedElementInTargetJSON = targetDb.elements
-      .tryGetElement<Subject>(targetSubjectId1!)
+      .getElement<Subject>(targetSubjectId1!)
       ?.toJSON();
     expect(sourceElementJSON).to.be.deep.equal(deletedElementInTargetJSON);
 
@@ -2733,6 +2720,70 @@ describe("IModelTransformer", () => {
     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" },
+    });
+    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);
+
+    const code = Subject.createCode(targetDb, IModel.rootSubjectId, "Subject1");
+    const targetSubjectId = targetDb.elements.queryElementIdByCode(code);
+    expect(targetSubjectId).to.not.be.undefined;
+
+    // update subject in source
+    const sourceSubject = sourceDb.elements.getElement<Subject>(
+      targetSubjectId!
+    );
+    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 = sourceDb.elements.getElement<Subject>(
+      targetSubjectId!
+    ).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")
@@ -2830,7 +2881,7 @@ describe("IModelTransformer", () => {
     expect(newSubjectId).to.not.be.undefined;
     targetDb.saveChanges();
 
-    // Calling process() for second time with option to preserve elements in hopes of restoring deleted element
+    // Calling process() for second time with option to preserve elements in hopes of throwing expected error
     const secondTransformer = new IModelTransformer(sourceDb, targetDb, {
       preserveElementIdsForFiltering: true,
     });
@@ -2907,7 +2958,7 @@ describe("IModelTransformer", () => {
     expect(targetSubjectId3).to.not.be.undefined;
     targetDb.saveChanges();
 
-    // Calling process() for second time with option to preserve elements in hopes of restoring deleted element
+    // 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,
     });