allow consumer to pass the entitytype for the change they're adding

iTwin · Dec 6, 2024 · 62f4f21 · 62f4f21
1 parent 00da1b2
commit 62f4f21
Show file tree

Hide file tree

Showing 3 changed files with 60 additions and 83 deletions.
diff --git a/packages/transformer/src/IModelExporter.ts b/packages/transformer/src/IModelExporter.ts
@@ -1218,72 +1218,46 @@ export class ChangedInstanceIds {
    * @param federationGuid federationGuid defined on the element
    */
   public async addCustomChange(
-    ecClassId: string,
+    entityType: ChangedInstanceType,
     changeType: SqliteChangeOp,
-    id: Id64String,
-    federationGuid?: Id64String
+    id: Id64String // TODO: SUPPORT BULK ADDS (per entity type okay?)
   ): Promise<void> {
     if (!this._ecClassIdsInitialized) await this.setupECClassIds();
-    if (this._relationshipSubclassIdsToSkip?.has(ecClassId)) return;
-    if (this._relationshipSubclassIds?.has(ecClassId))
+    if (entityType === "relationship") {
       throw new Error(
-        `Misuse. id: ${id}, ecClassId: ${ecClassId} is a relationship class. Use 'addCustomRelationshipChange' instead.`
-      );
-    this._hasCustomChanges = true;
-
-    const customData: ChangedInstanceCustomData = { ecClassId };
-    if (federationGuid) customData.federationGuid = federationGuid;
-    if (this.isCodeSpec(ecClassId)) {
-      this.handleChange(this.codeSpec, changeType, id);
-      this._entityReferenceToCustomDataMap.set(`c${id}`, customData);
-    } else if (this.isAspect(ecClassId)) {
-      this._entityReferenceToCustomDataMap.set(
-        EntityReferences.fromEntityType(id, ConcreteEntityTypes.ElementAspect),
-        customData
+        `Misuse. id: ${id} is a relationship. Use 'addCustomRelationshipChange' instead.`
       );
-      this.handleChange(this.aspect, changeType, id);
-    } else if (this.isModel(ecClassId)) {
-      this._entityReferenceToCustomDataMap.set(
-        EntityReferences.fromEntityType(id, ConcreteEntityTypes.Model),
-        customData
-      );
-      this.handleChange(this.model, changeType, id);
-    } else if (this.isElement(ecClassId)) {
-      this._entityReferenceToCustomDataMap.set(
-        EntityReferences.fromEntityType(id, ConcreteEntityTypes.Element),
-        customData
-      );
-      this.handleChange(this.element, changeType, id);
     }
-  }
+    this._hasCustomChanges = true;
 
-  public getCustomDataFromId(
-    id: Id64String,
-    type: ChangedInstanceType
-  ): ChangedInstanceCustomData | undefined {
-    let entityType: ConcreteEntityTypes | undefined;
-    switch (type) {
-      case "aspect":
-        entityType = ConcreteEntityTypes.ElementAspect;
+    switch (entityType) {
+      case "codeSpec":
+        this.handleChange(this.codeSpec, changeType, id);
         break;
-      case "element":
-        entityType = ConcreteEntityTypes.Element;
+      case "model": // TODO: support models better. We need to mark parent models as updated too otherwise the sub-model will get skipped.
+        this.handleChange(this.model, changeType, id);
         break;
-      case "model":
-        entityType = ConcreteEntityTypes.Model;
+      case "element":
+        this.handleChange(this.element, changeType, id);
         break;
-      case "relationship":
-        entityType = ConcreteEntityTypes.Relationship;
+      case "aspect":
+        this.handleChange(this.aspect, changeType, id);
         break;
     }
+  }
 
-    if (entityType !== undefined)
+  /** TODO: Maybe relationships only? maybe not. */
+  public getCustomRelationshipDataFromId(
+    id: Id64String,
+    type: ChangedInstanceType
+  ): ChangedInstanceCustomData | undefined {
+    if (type === "relationship") {
       return this._entityReferenceToCustomDataMap.get(
-        EntityReferences.fromEntityType(id, entityType)
+        EntityReferences.fromEntityType(id, ConcreteEntityTypes.Relationship)
       );
+    }
 
-    // codeSpecs do not have a 'ConcreteEntityType' but are still stored in entityReferenceToDataMap as if they did with a prefix of 'c' to their id.
-    return this._entityReferenceToCustomDataMap.get(`c${id}`);
+    return undefined;
   }
 
   /**

diff --git a/packages/transformer/src/IModelTransformer.ts b/packages/transformer/src/IModelTransformer.ts
@@ -2930,41 +2930,44 @@ export class IModelTransformer extends IModelExportHandler {
       csReader.close();
     }
 
-    // Because there is a possibility that someone could manually add ids to exporter.sourceDbChanges.deleteIds, we must separately process exporter.sourceDbChanges.deleteIds as deletedOps.
+    // This loop is to process all custom deleteIds. Unclear if the special logic is still necessary for relationships or not (TODO!!). For all other entities, we assume that the element is still present in the sourceDb because it is not
+    // a real delete and instead a simulated delete to update filtering criteria between source and target. Since the element is still present, we do not need to call processDeletedOp to find the corresponding targetId.
+    // We can instead rely on `forEachTrackedElement` at the top of processChangesets to find the corresponding targetId.
+    // Note this also assumes we don't need to handle entity recreation for these custom deletes. I.e. a caller of API would not be able to add a custom delete for an entity that was recreated.
+    // a delete followed by an insert.
+    // ASSUME: If a changeset has a deleteId then custom change will never reference it. Is this still true if it was re-inserted? (TODO!!)
     if (this.exporter.sourceDbChanges?.hasCustomChanges) {
-      for (const changedInstances of changedInstanceOps) {
-        for (const id of this.exporter.sourceDbChanges?.[
-          changedInstances
-        ].deleteIds.keys() ?? []) {
-          // If id was already processed, that means it was a normal delete found in a changeset. We can skip it.
-          if (deleteIdsProcessed.has(id)) continue;
-          const customData = this.exporter.sourceDbChanges?.getCustomDataFromId(
+      for (const id of this.exporter.sourceDbChanges?.relationship.deleteIds.keys() ??
+        []) {
+        if (deleteIdsProcessed.has(id)) continue;
+
+        const customData =
+          this.exporter.sourceDbChanges?.getCustomRelationshipDataFromId(
             id,
-            changedInstances
+            "relationship"
           );
-          if (customData) {
-            const ecClassId = customData?.ecClassId;
-            const classFullName =
-              this.exporter.sourceDbChanges?.getClassFullNameFromECClassId(
-                ecClassId
-              );
-            const fedGuid = customData?.federationGuid;
-            const sourceIdOfRelationshipInSource =
-              customData?.sourceIdOfRelationship;
-            const targetIdOfRelationshipInSource =
-              customData?.targetIdOfRelationship;
-            await this.processDeletedOp(
-              id,
-              classFullName ?? "",
-              fedGuid,
-              elemIdToScopeEsa,
-              relationshipECClassIds.has(ecClassId ?? ""),
-              sourceIdOfRelationshipInSource,
-              targetIdOfRelationshipInSource,
-              alreadyImportedElementInserts,
-              alreadyImportedModelInserts
+        if (customData) {
+          const ecClassId = customData?.ecClassId;
+          const classFullName =
+            this.exporter.sourceDbChanges?.getClassFullNameFromECClassId(
+              ecClassId
             );
-          }
+          const fedGuid = customData?.federationGuid;
+          const sourceIdOfRelationshipInSource =
+            customData?.sourceIdOfRelationship;
+          const targetIdOfRelationshipInSource =
+            customData?.targetIdOfRelationship;
+          await this.processDeletedOp(
+            id,
+            classFullName ?? "",
+            fedGuid,
+            elemIdToScopeEsa,
+            relationshipECClassIds.has(ecClassId ?? ""),
+            sourceIdOfRelationshipInSource,
+            targetIdOfRelationshipInSource,
+            alreadyImportedElementInserts,
+            alreadyImportedModelInserts
+          );
         }
       }
     }

diff --git a/packages/transformer/src/test/standalone/IModelTransformerHub.test.ts b/packages/transformer/src/test/standalone/IModelTransformerHub.test.ts
@@ -1928,7 +1928,7 @@ describe("IModelTransformerHub", () => {
                     targetIdOfRel!
                   );
                   await exporter.sourceDbChanges?.addCustomChange(
-                    ecClassIdOfElement!,
+                    "element",
                     "Inserted",
                     elementIdInSource!
                   );
@@ -1979,7 +1979,7 @@ describe("IModelTransformerHub", () => {
                     targetIdOfRel!
                   );
                   await exporter.sourceDbChanges?.addCustomChange(
-                    ecClassIdOfElement!,
+                    "element",
                     "Deleted",
                     elementIdInSource!
                   );