Algo.ts enhancements, test deleting relationship and delete element r…

…ight after then syncing (#127)

findRangeContaining uses binary search, added tests for algo.ts, fix bug
where range is just a single point and we don't skip over it.
resolve todo: move to clonerelationship in imodelclonecontext
fixme: delete relationship and delete element right after. includes fix
to not throw if aspect is not found

---------

Co-authored-by: Michael Belousov <mike.belousov@bentley.com>

packages/transformer/src/Algo.ts

@@ -19,17 +19,14 @@ export function rangesFromRangeAndSkipped(start: number, end: number, skipped: n
     throw RangeError(`invalid range: [${start}, ${end}]`);
 
   const ranges = [firstRange];
-
-  function findRangeContaining(pt: number, inRanges: [number, number][]): number {
-    // TODO: binary search
-    return inRanges.findIndex((r) => (pt >= r[0] && pt <= r[1]));
-  }
-
   for (const skip of skipped) {
     const rangeIndex = findRangeContaining(skip, ranges);
     if (rangeIndex === -1)
       continue;
     const range = ranges[rangeIndex];
+    // If the range we find ourselves in is just a single point (range[0] === range[1]) then we need to remove it if (range[0] === skip)
+    if (range[0] === range[1] && skip === range[0])
+      ranges.splice(rangeIndex, 1);
     const leftRange = [range[0], skip - 1] as [number, number];
     const rightRange = [skip + 1, range[1]] as [number, number];
     if (validRange(leftRange) && validRange(rightRange))
@@ -43,6 +40,25 @@ export function rangesFromRangeAndSkipped(start: number, end: number, skipped: n
   return ranges;
 }
 
+function findRangeContaining(pt: number, inRanges: [number, number][]): number {
+  let begin = 0;
+  let end = inRanges.length - 1;
+  while (end >= begin) {
+    const mid = begin + Math.floor((end - begin) / 2);
+    const range = inRanges[mid];
+
+    if (pt >= range[0] && pt <= range[1])
+      return mid;
+
+    if (pt < range[0]) {
+      end = mid - 1;
+    } else {
+      begin = mid + 1;
+    }
+  }
+  return -1;
+}
+
 export function renderRanges(ranges: [number, number][]): number[] {
   const result = [];
   for (const range of ranges)

packages/transformer/src/IModelTransformer.ts

@@ -556,8 +556,8 @@ export class IModelTransformer extends IModelExportHandler {
 
   /** Create an ExternalSourceAspectProps in a standard way for a Relationship in an iModel --> iModel transformations.
    * The ExternalSourceAspect is meant to be owned by the Element in the target iModel that is the `sourceId` of transformed relationship.
-   * The `identifier` property of the ExternalSourceAspect will be the ECInstanceId of the relationship in the source iModel.
-   * The ECInstanceId of the relationship in the target iModel will be stored in the JsonProperties of the ExternalSourceAspect.
+   * The `identifier` property of the ExternalSourceAspect will be the ECInstanceId of the relationship in the master iModel.
+   * The ECInstanceId of the relationship in the branch iModel will be stored in the JsonProperties of the ExternalSourceAspect.
    */
   private initRelationshipProvenance(sourceRelationship: Relationship, targetRelInstanceId: Id64String): ExternalSourceAspectProps {
     return IModelTransformer.initRelationshipProvenanceOptions(
@@ -677,7 +677,7 @@ export class IModelTransformer extends IModelExportHandler {
       jsonProperties: undefined as TargetScopeProvenanceJsonProps | undefined,
     };
 
-    // FIXME: handle older transformed iModels which do NOT have the version
+    // FIXME: handle older transformed iModels which do NOT have the version. Add test where we don't set those and then start setting them.
     // or reverseSyncVersion set correctly
     const externalSource = this.queryScopeExternalSource(aspectProps, { getJsonProperties: true }); // this query includes "identifier"
     aspectProps.id = externalSource.aspectId;
@@ -2011,7 +2011,14 @@ export class IModelTransformer extends IModelExportHandler {
     }
 
     if (deletedRelData.provenanceAspectId) {
-      this.provenanceDb.elements.deleteAspect(deletedRelData.provenanceAspectId);
+      try {
+        this.provenanceDb.elements.deleteAspect(deletedRelData.provenanceAspectId);
+      } catch (error: any) {
+        // This aspect may no longer exist if it was deleted at some other point during the transformation. This is fine.
+        if (error.errorNumber === IModelStatus.NotFound)
+          return;
+        throw error;
+      }
     }
   }
 

packages/transformer/src/test/TestUtils/TimelineTestUtil.ts

@@ -153,6 +153,10 @@ export interface TimelineIModelElemStateDelta {
   [name: string]: TimelineElemDelta;
 }
 
+/** [name: string]: Becomes the userlabel / codevalue of a physical object in the iModel.
+* Note that since JS converts all keys to strings, passing keys as numbers is also allowed. They will be converted to strings.
+* TimelineElemState if it is a number sets a json property on the physicalobject to the number provided.
+*/
 export interface TimelineIModelElemState {
   [name: string]: TimelineElemState;
 }

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

@@ -0,0 +1,64 @@
+import { rangesFromRangeAndSkipped } from "../../Algo";
+import { expect } from "chai";
+        /** given a discrete inclusive range [start, end] e.g. [-10, 12] and several "skipped" values", e.g.
+     * (-10, 1, -3, 5, 15), return the ordered set of subranges of the original range that exclude
+     * those values
+     */
+      // function rangesFromRangeAndSkipped(start: number, end: number, skipped: number[]): [number, number][]
+
+describe("Test rangesFromRangeAndSkipped", async () => {
+    it("should return proper ranges with skipped at beginning of range", async () => {
+        const ranges = rangesFromRangeAndSkipped(-10, 12, [-10, 1, -3, 5, 15]);
+        expect(ranges).to.eql([[-9, -4], [-2, 0], [2, 4], [6, 12]]);
+    });
+
+    it("should return proper ranges with two consecutive values skipped at beginning of range ascending order", async () => {
+        const ranges = rangesFromRangeAndSkipped(-10, 12, [-10, -9, 1, -3, 5, 15]);
+        expect(ranges).to.eql([[-8, -4], [-2, 0], [2, 4], [6, 12]]);
+    });
+
+    it("should return proper ranges with two consecutive values skipped at beginning of range descending order", async () => {
+        const ranges = rangesFromRangeAndSkipped(-10, -8, [-9, -10]);
+        expect(ranges).to.eql([[-8, -8]]);
+    });
+
+    it("should return proper ranges with two consecutive values skipped at beginning of range descending order and more skips", async () => {
+        const ranges = rangesFromRangeAndSkipped(-10, 12, [-9, -10, 1, -3, 5, 15]);
+        expect(ranges).to.eql([[-8, -4],[-2, 0], [2, 4], [6, 12]]);
+    });
+
+    it("should return proper ranges with two consecutive values skipped at beginning of range but at the end of the skipped array", async () => {
+        const ranges = rangesFromRangeAndSkipped(-10, 12, [1, -3, 5, -9, -10, 15]);
+        expect(ranges).to.eql([[-8, -4],[-2, 0], [2, 4], [6, 12]]);
+    });
+
+    it("should return proper ranges with two consecutive values skipped at beginning of range but the two values are duplicated in middle and end of skipped array", async () => {
+        const ranges = rangesFromRangeAndSkipped(-10, 12, [1, -3, 5, -9, -10, 15, -9, -10]);
+        expect(ranges).to.eql([[-8, -4],[-2, 0], [2, 4], [6, 12]]);
+    });
+
+    it("should return proper ranges with two non-consecutive values skipped at beginning of range", async () => {
+        const ranges = rangesFromRangeAndSkipped(-10, 12, [-8, -10, 1, -3, 5, 15]);
+        expect(ranges).to.eql([[-9, -9], [-7, -4],[-2, 0], [2, 4], [6, 12]]);
+    });
+
+    it("should return proper ranges with two consecutive values skipped at middle of range", async () => {
+        const ranges = rangesFromRangeAndSkipped(-10, 12, [-10, 1, -2, -3, 5, 15]);
+        expect(ranges).to.eql([[-9, -4],[-1, 0], [2, 4], [6, 12]]);
+    });
+
+    it("should return proper ranges with two consecutive values skipped at end of range ascending order", async () => {
+        const ranges = rangesFromRangeAndSkipped(8, 10, [9, 10]);
+        expect(ranges).to.eql([[8,8]]);
+    });
+
+    it("should return proper ranges with two consecutive values skipped at end of range descending order", async () => {
+        const ranges = rangesFromRangeAndSkipped(8, 10, [10, 9]);
+        expect(ranges).to.eql([[8,8]]);
+    });
+
+    it("should return proper ranges with skipped array being sorted in descending order", async () => {
+        const ranges = rangesFromRangeAndSkipped(-10, 12, [15, 5, 1, -3, -9, -10]);
+        expect(ranges).to.eql([[-8, -4],[-2, 0], [2, 4], [6, 12]]);
+    });
+});

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

@@ -207,7 +207,9 @@ describe("IModelTransformer", () => {
       // assert.equal(targetImporter.numRelationshipsDeleted, 0);
       targetDb.saveChanges();
       // FIXME: upgrade this test to use a briefcase so that we can detect element deletes
-      TransformerExtensiveTestScenario.assertUpdatesInDb(targetDb, /* FIXME: */ false);
+      TransformerExtensiveTestScenario.assertUpdatesInDb(targetDb, /* FIXME: */ false); // Switch back to true once we have the old detect deltes behavior flag here. also enable force old provenance method.
+      // which is only used in in-imodel transformations.
+
       assert.equal(numTargetRelationships + targetImporter.numRelationshipsInserted - targetImporter.numRelationshipsDeleted, count(targetDb, ElementRefersToElements.classFullName));
       // FIXME: why?
       expect(count(targetDb, "ExtensiveTestScenarioTarget:TargetInformationRecord")).to.equal(3);