Merge pull request #201 from backtrace-labs/backtrace-db-context-batch-fix

Fix Backtrace Database Context retry mechanism

Author: BartoszLitwiniuk

backtrace-library/src/androidTest/java/backtraceio/library/database/BacktraceDatabaseContextTest.java

@@ -33,11 +33,14 @@ public class BacktraceDatabaseContextTest {
     private BacktraceDatabaseSettings databaseSettings;
     private final String testMessage = "Example test string";
 
+    private final int RETRY_LIMIT = 5;
+
     @Before
     public void setUp() {
         this.context = InstrumentationRegistry.getInstrumentation().getContext();
         this.dbPath = this.context.getFilesDir().getAbsolutePath();
         this.databaseSettings = new BacktraceDatabaseSettings(this.dbPath, RetryOrder.Queue);
+        this.databaseSettings.setRetryLimit(RETRY_LIMIT);
         this.databaseContext = new BacktraceDatabaseContext(this.databaseSettings);
     }
     @After
@@ -254,17 +257,40 @@ public void tryAddNullBacktraceDbRecord() {
         databaseContext.add((BacktraceDatabaseRecord) null);
     }
 
+    @Test
+    public void testIncrementBatchRetry() {
+        // GIVEN
+        int numberOfRetries = 0;
+        fillDatabase(10);
+
+        // WHEN
+
+        for (int i = 0; i < 10; i++) {
+            BacktraceDatabaseRecord first = databaseContext.first(); // QUEUE
+            if (first == null) {
+                break;
+            }
+            first.close();
+            this.databaseContext.incrementBatchRetry();
+            numberOfRetries++;
+        }
+
+        // THEN
+        assertEquals(0, databaseContext.count());
+        assertEquals(RETRY_LIMIT, numberOfRetries);
+    }
+
     private List<BacktraceDatabaseRecord> fillDatabase() {
+        return this.fillDatabase(3);
+    }
+    private List<BacktraceDatabaseRecord> fillDatabase(int numberOfReports) {
         List<BacktraceDatabaseRecord> result = new ArrayList<>();
-        BacktraceReport report = new BacktraceReport(this.testMessage);
-        BacktraceReport report2 = new BacktraceReport(this.testMessage);
-        BacktraceReport report3 = new BacktraceReport(this.testMessage);
-        BacktraceData data = new BacktraceData.Builder(report).setAttributes(this.context, null).build();
-        BacktraceData data2 = new BacktraceData.Builder(report2).setAttributes(this.context, null).build();
-        BacktraceData data3 = new BacktraceData.Builder(report3).setAttributes(this.context, null).build();
-        result.add(databaseContext.add(data));
-        result.add(databaseContext.add(data2));
-        result.add(databaseContext.add(data3));
+
+        for (int i = 0; i < numberOfReports; i++) {
+            BacktraceReport report = new BacktraceReport(testMessage);
+            BacktraceData data = new BacktraceData.Builder(report).setAttributes(this.context, null).build();
+            result.add(databaseContext.add(data));
+        }
 
         // Dispose all records
         for (BacktraceDatabaseRecord record : result) {
@@ -273,4 +299,4 @@ private List<BacktraceDatabaseRecord> fillDatabase() {
 
         return result;
     }
-}
+}

backtrace-library/src/main/java/backtraceio/library/BacktraceDatabase.java

@@ -316,7 +316,7 @@ public void onEvent(BacktraceResult backtraceResult) {
                                     } else {
                                         BacktraceLogger.d(LOG_TAG, "Backtrace DB Timer - closing record");
                                         currentRecord.close();
-                                        // backtraceDatabaseContext.incrementBatchRetry(); TODO: consider another way to remove some records after few retries
+                                        backtraceDatabaseContext.incrementBatchRetry(); // If we are not able to send single record we are moving all reports to next batch
                                     }
                                     threadWaiter.countDown();
                                 }

backtrace-library/src/main/java/backtraceio/library/services/BacktraceDatabaseContext.java

@@ -56,12 +56,15 @@ public class BacktraceDatabaseContext implements DatabaseContext {
 
     /**
      * @deprecated This constructor will be removed in future versions.
-     * The {@code context} parameter is no longer used.
-     * Please use the constructor without the {@code context} parameter.
+     *             The {@code context} parameter is no longer used.
+     *             Please use the constructor without the {@code context} parameter.
      *
-     * <p>Use {@link #BacktraceDatabaseContext(BacktraceDatabaseSettings)} instead.</p>
+     *             <p>
+     *             Use {@link #BacktraceDatabaseContext(BacktraceDatabaseSettings)}
+     *             instead.
+     *             </p>
      *
-     * @param context The unused Android context parameter.
+     * @param context  The unused Android context parameter.
      * @param settings The database settings.
      */
     @Deprecated
@@ -145,7 +148,8 @@ public BacktraceDatabaseRecord add(BacktraceDatabaseRecord backtraceDatabaseReco
     }
 
     /**
-     * Get first existing database record. Method returns record based on order in database
+     * Get first existing database record. Method returns record based on order in
+     * database
      *
      * @return first Backtrace database record
      */
@@ -156,15 +160,15 @@ public BacktraceDatabaseRecord first() {
     }
 
     /**
-     * Get last existing database record. Method returns record based on order in database
+     * Get last existing database record. Method returns record based on order in
+     * database
      *
      * @return last Backtrace database record
      */
     public BacktraceDatabaseRecord last() {
         return this.retryOrder == RetryOrder.Queue ? getLastRecord() : getFirstRecord();
     }
 
-
     /**
      * Get all database records
      *
@@ -187,7 +191,6 @@ public long getDatabaseSize() {
         return this.totalSize.get();
     }
 
-
     /**
      * Delete existing record from database
      *
@@ -241,7 +244,7 @@ public boolean contains(BacktraceDatabaseRecord record) {
             Queue<BacktraceDatabaseRecord> records = entry.getValue();
 
             for (BacktraceDatabaseRecord databaseRecord : records) {
-                if (databaseRecord != null && databaseRecord.id == record.id) {
+                if (databaseRecord != null && databaseRecord.id.equals(record.id)) {
                     return true;
                 }
             }
@@ -288,7 +291,7 @@ public void clear() {
         }
     }
 
-        /**
+    /**
      * Delete the oldest file
      *
      * @return is deletion was successful
@@ -308,26 +311,44 @@ public boolean removeOldestRecord() {
      * Increment each batch
      */
     private void incrementBatches() {
+        // Move batches forward: batch i -> batch i+1
+        // Iterate in reverse to avoid overwriting data before moving it
         for (int i = this._retryNumber - 2; i >= 0; i--) {
             Queue<BacktraceDatabaseRecord> currentBatch = this.batchRetry.get(i);
-            batchRetry.put(i, new ConcurrentLinkedQueue<>());
-            batchRetry.put(i + 1, currentBatch);
+
+            // Ensure we have a valid queue (shouldn't be null, but handle it safely)
+            if (currentBatch == null) {
+                currentBatch = new ConcurrentLinkedQueue<>();
+            }
+
+            // Store reference to the batch we're moving
+            Queue<BacktraceDatabaseRecord> batchToMove = currentBatch;
+
+            // Clear batch at position i and replace with new empty queue
+            this.batchRetry.put(i, new ConcurrentLinkedQueue<>());
+
+            // Move the batch to the next position (overwriting whatever was there)
+            this.batchRetry.put(i + 1, batchToMove);
         }
     }
 
     /**
      * Remove last batch
      */
     private void removeMaxRetries() {
-        Queue<BacktraceDatabaseRecord> currentBatch = this.batchRetry.get(_retryNumber - 1);
+        Queue<BacktraceDatabaseRecord> currentBatch = this.batchRetry.put(_retryNumber - 1, new ConcurrentLinkedQueue<>());
+
+        if (currentBatch == null) {
+            this.batchRetry.put(_retryNumber - 1, new ConcurrentLinkedQueue<>());
+            return;
+        }
 
         for (BacktraceDatabaseRecord record : currentBatch) {
-            if (!record.valid()) {
-                continue;
+            if (record.valid()) {
+                record.delete();
+                this.totalRecords.decrementAndGet();
+                totalSize.addAndGet(-record.getSize());
             }
-            record.delete();
-            this.totalRecords.decrementAndGet();
-            totalSize.addAndGet(-record.getSize());
         }
     }