fix: expand bitmap when turning off a value larger than size

sepgh · Oct 22, 2024 · 7486a57 · 7486a57
1 parent 858d371
commit 7486a57
Show file tree

Hide file tree

Showing 3 changed files with 55 additions and 37 deletions.
diff --git a/src/main/java/com/github/sepgh/testudo/index/Bitmap.java b/src/main/java/com/github/sepgh/testudo/index/Bitmap.java
@@ -19,28 +19,40 @@ public Bitmap(Class<K> kClass, byte[] data) {
         this.width = data.length * Byte.SIZE;
     }
 
-    public void on(K k) {
+    public boolean on(K k) {
         BigInteger bitIndex = convertKeyToBigInteger(k);  // Convert to BigInteger for safety
         ensureCapacity(bitIndex);  // Resize if necessary
 
         // Calculate the byte and bit position
         int byteIndex = bitIndex.divide(BigInteger.valueOf(Byte.SIZE)).intValue();
         int bitPosition = bitIndex.mod(BigInteger.valueOf(Byte.SIZE)).intValue();
 
+        if ((data[byteIndex] & (1 << bitPosition)) != 0)
+            return false;
+
         // Set the bit
         data[byteIndex] |= (byte) (1 << bitPosition);
+        return true;
     }
 
-    public void off(K k) {
+    public boolean off(K k) {
         BigInteger bitIndex = convertKeyToBigInteger(k);  // Convert to BigInteger for safety
-        ensureCapacity(bitIndex);  // Resize if necessary
+
+        if (needsExpansion(bitIndex)){
+            return false;
+        }
 
         // Calculate the byte and bit position
         int byteIndex = bitIndex.divide(BigInteger.valueOf(Byte.SIZE)).intValue();
         int bitPosition = bitIndex.mod(BigInteger.valueOf(Byte.SIZE)).intValue();
 
+        if ((data[byteIndex] & (1 << bitPosition)) == 0){
+            return false;
+        }
+
         // Clear the bit
         data[byteIndex] &= (byte) ~(1 << bitPosition);
+        return true;
     }
 
     // Convert K to a BigInteger, handling UnsignedLong values beyond Long.MAX_VALUE
@@ -51,9 +63,13 @@ private BigInteger convertKeyToBigInteger(K k) {
         return BigInteger.valueOf(k.longValue());  // Handles Long, Integer, etc.
     }
 
+    private boolean needsExpansion(BigInteger bitIndex) {
+        return bitIndex.compareTo(BigInteger.valueOf(width)) >= 0;
+    }
+
     // Ensure the byte array has enough space to handle the bit at bitIndex
     private void ensureCapacity(BigInteger bitIndex) {
-        if (bitIndex.compareTo(BigInteger.valueOf(width)) >= 0) {
+        if (needsExpansion(bitIndex)) {
             // We need to grow the array to accommodate the new bit index
             int newWidth = bitIndex.intValue() + 1;  // At least one more than the current bit index
             int newLength = (newWidth + Byte.SIZE - 1) / Byte.SIZE;  // Convert bits to bytes
@@ -114,7 +130,6 @@ public K next() {
 
                 if ((isBitSet && checkForOnBit) || (!isBitSet && !checkForOnBit)) {
                     lastReturnedBitIndex = BigInteger.valueOf(currentByteIndex).multiply(BigInteger.valueOf(Byte.SIZE)).add(BigInteger.valueOf(currentBitPosition));
-                    System.out.println(lastReturnedBitIndex.intValue());
                     moveToNextBit();
                     return convertIndexToK(lastReturnedBitIndex);
                 }

diff --git a/src/main/java/com/github/sepgh/testudo/index/DuplicateBitmapIndexManager.java b/src/main/java/com/github/sepgh/testudo/index/DuplicateBitmapIndexManager.java
@@ -30,7 +30,6 @@ public DuplicateBitmapIndexManager(int collectionId, UniqueTreeIndexManager<K, P
     }
 
 
-    // Todo: current implementation never returns false
     @Override
     public boolean addIndex(K identifier, V value) throws InternalOperationException, IndexBinaryObject.InvalidIndexBinaryObject, IOException, ExecutionException, InterruptedException {
         Optional<Pointer> pointerOptional = this.indexManager.getIndex(identifier);
@@ -45,38 +44,42 @@ public boolean addIndex(K identifier, V value) throws InternalOperationException
             int initialSize = dbObject.getDataSize();
 
             Bitmap<V> vBitmap = new Bitmap<>(valueIndexBinaryObjectFactory.getType(), dbObject.getData());
-            vBitmap.on(value);
-
-            if (initialSize >= vBitmap.getData().length) {
-                databaseStorageManager.update(pointer, dbObject1 -> {
+            boolean result = vBitmap.on(value);
+
+            if (result) {
+                if (initialSize >= vBitmap.getData().length) {
+                    databaseStorageManager.update(pointer, dbObject1 -> {
+                        try {
+                            dbObject1.modifyData(vBitmap.getData());
+                        } catch (VerificationException.InvalidDBObjectWrapper e) {
+                            throw new RuntimeException(e);
+                        }
+                    });
+                } else {
+                    Pointer pointer1 = databaseStorageManager.store(collectionId, 1, vBitmap.getData());
                     try {
-                        dbObject1.modifyData(vBitmap.getData());
-                    } catch (VerificationException.InvalidDBObjectWrapper e) {
+                        this.indexManager.updateIndex(identifier, pointer1);
+                        databaseStorageManager.remove(pointer);
+                    } catch (IndexMissingException e) {
                         throw new RuntimeException(e);
                     }
-                });
-            } else {
-                Pointer pointer1 = databaseStorageManager.store(collectionId, 1, vBitmap.getData());
-                try {
-                    this.indexManager.updateIndex(identifier, pointer1);
-                    databaseStorageManager.remove(pointer);
-                } catch (IndexMissingException e) {
-                    throw new RuntimeException(e);
                 }
             }
+
+            return result;
         } else {
             Bitmap<V> vBitmap = new Bitmap<>(valueIndexBinaryObjectFactory.getType(), new byte[1]);
             vBitmap.on(value);
             Pointer pointer = databaseStorageManager.store(collectionId, 1, vBitmap.getData());
             try {
                 this.indexManager.addIndex(identifier, pointer);
+                return true;
             } catch (IndexExistsException e) {
                 // Another thread stored index? shouldn't happen, but retry
-                this.addIndex(identifier, value);
+                return this.addIndex(identifier, value);
             }
         }
 
-        return true;
     }
 
 
@@ -104,15 +107,19 @@ public boolean removeIndex(K identifier, V value) throws InternalOperationExcept
             if (dbObjectOptional.isPresent()) {
                 DBObject dbObject = dbObjectOptional.get();
                 Bitmap<V> vBitmap = new Bitmap<>(valueIndexBinaryObjectFactory.getType(), dbObject.getData());
-                vBitmap.off(value);
-                databaseStorageManager.update(pointer, dbObject1 -> {
-                    try {
-                        dbObject1.modifyData(vBitmap.getData());
-                    } catch (VerificationException.InvalidDBObjectWrapper e) {
-                        throw new RuntimeException(e);
-                    }
-                });
-                return true;
+                boolean result = vBitmap.off(value);
+
+                if (result) {
+                    databaseStorageManager.update(pointer, dbObject1 -> {
+                        try {
+                            dbObject1.modifyData(vBitmap.getData());
+                        } catch (VerificationException.InvalidDBObjectWrapper e) {
+                            throw new RuntimeException(e);
+                        }
+                    });
+                }
+
+                return result;
             }
         }
 

diff --git a/src/test/java/com/github/sepgh/test/index/DuplicateBitmapIndexManagerTestCase.java b/src/test/java/com/github/sepgh/test/index/DuplicateBitmapIndexManagerTestCase.java
@@ -79,7 +79,7 @@ public void test_addRemoveAndIterate() throws Exception {
         int[] testItems = new int[]{1,2,3,4,9,10,11,12};
 
         for (int item : testItems) {
-            duplicateIndexManager.addIndex(1, item);
+            Assertions.assertTrue(duplicateIndexManager.addIndex(1, item));
         }
 
 
@@ -96,11 +96,7 @@ public void test_addRemoveAndIterate() throws Exception {
 
 
         Assertions.assertTrue(duplicateIndexManager.removeIndex(1, testItems[0]));
-
-        // Todo: items that dont exist should return false when getting removed
-        //       Currently an exception is thrown because doing `off()` on bitmap will extend it's size,
-        //       and updating with extended size is not valid
-//        Assertions.assertFalse(duplicateIndexManager.removeIndex(1, 10000));
+        Assertions.assertFalse(duplicateIndexManager.removeIndex(1, 10000));
 
 
         listIteratorOptional = duplicateIndexManager.getIndex(1);