Merge pull request #83 from anmcgrath/row-delete-bug

Fix overflow error in RegionDataStore when rows/columns were removed.
anmcgrath · Jun 23, 2024 · 3c74e1d · 3c74e1d
2 parents 69d97cd + a090205
commit 3c74e1d
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 11 deletions.
diff --git a/src/BlazorDatasheet.DataStructures/Geometry/Region.cs b/src/BlazorDatasheet.DataStructures/Geometry/Region.cs
@@ -28,7 +28,16 @@ public class Region : IRegion
 
     public int Height => Bottom >= int.MaxValue ? int.MaxValue : Bottom - Top + 1;
     public int Width => Right >= int.MaxValue ? int.MaxValue : Right - Left + 1;
-    public int Area => Height * Width;
+    public int Area
+    {
+        get
+        {
+            if (Height == int.MaxValue || Width == int.MaxValue)
+                return int.MaxValue;
+
+            return Height * Width;
+        }
+    }
 
 
     /// <summary>

diff --git a/src/BlazorDatasheet.DataStructures/Store/RegionDataStore.cs b/src/BlazorDatasheet.DataStructures/Store/RegionDataStore.cs
@@ -220,14 +220,6 @@ private RegionRestoreData<T> RemoveRowsOrColumsAndShift(int start, int end, Axis
 
             // if the region is partially overlapping (it must be because we know it is overlapping and it doesn't fully
             // overlap) then shift the overlap left and contract the left edge
-            // First check if it will result in the area being smaller than the minArea, in which case it should be 
-            // removed instead
-            if (overlap.Region.Break(region).Sum(x => x.Area) <= MinArea)
-            {
-                Tree.Delete(overlap);
-                removed.Add(overlap);
-                continue;
-            }
 
             var intersection = overlap.Region.GetIntersection(region)!;
             // contraction amount in row direction
@@ -246,7 +238,12 @@ private RegionRestoreData<T> RemoveRowsOrColumsAndShift(int start, int end, Axis
 
             var newRegion = new DataRegion<T>(overlap.Data, overlap.Region.Clone());
             newRegion.Region.Contract(cEdge, Math.Max(cRow, cCol));
-            newRegion.Region.Shift(sRow,sCol);
+            newRegion.Region.Shift(sRow, sCol);
+
+            // if the region is less than the minimum area, don't add it back
+            if(newRegion.Region.Area <= MinArea)
+                continue;
+
             newRegion.UpdateEnvelope();
             dataAdded.Add(newRegion);
             newDataToAdd.Add(newRegion);

diff --git a/test/BlazorDatasheet.Test/Store/RegionStoreTests.cs b/test/BlazorDatasheet.Test/Store/RegionStoreTests.cs
@@ -185,7 +185,7 @@ public void Delete_First_Col_Of_region_Restores()
             .Select(x => x.Region)
             .Should().BeEquivalentTo([r]);
     }
-    
+
     [Test]
     public void Delete_First_Cols_From_Behind_region_Restores()
     {
@@ -253,4 +253,13 @@ public void Get_Sub_Store_Gets_Sub_Storage_Works_When_Resetting_Indices()
         subStore.GetData(0, 1).Should().BeEquivalentTo(new int[] { 1 });
         subStore.GetData(1, 2).Should().BeEquivalentTo(new int[] { 2 });
     }
+
+    [Test]
+    public void Removing_Across_Column_Region_Doesnt_Overflow()
+    {
+        var store = new ConsolidatedDataStore<int>();
+        store.Add(new ColumnRegion(10, 20), 1);
+
+        store.RemoveRows(10, 10);
+    }
 }