From eb3ced8fbeb3cb9ca98d24942a0fc69b26b25f2c Mon Sep 17 00:00:00 2001
From: Nicolas Chan <nicolas@nicolaschan.com>
Date: Thu, 18 Mar 2021 13:39:08 -0700
Subject: [PATCH] Fix bug with sequential deletion block order

---
 backup.sh    | 14 +++++++-------
 test/test.sh | 15 ++++++++++++++-
 2 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/backup.sh b/backup.sh
index 4479f0f..750e217 100755
--- a/backup.sh
+++ b/backup.sh
@@ -276,7 +276,7 @@ delete-backup () {
 
 # Sequential delete method
 delete-sequentially () {
-  local BACKUPS=("$BACKUP_DIRECTORY"/*)
+  local BACKUPS=("$BACKUP_DIRECTORY"/*) # List oldest first
   while [[ $MAX_BACKUPS -ge 0 && ${#BACKUPS[@]} -gt $MAX_BACKUPS ]]; do
     delete-backup "$(basename "${BACKUPS[0]}")"
     BACKUPS=("$BACKUP_DIRECTORY"/*)
@@ -342,19 +342,19 @@ delete-thinning () {
   fi
 
   local CURRENT_INDEX=0
-  local BACKUPS=("$BACKUP_DIRECTORY"/*) # List newest first
+  local BACKUPS=("$BACKUP_DIRECTORY"/*) # Oldest first
+  local NUM_BACKUPS="${#BACKUPS[@]}"
 
   for BLOCK_INDEX in "${!BLOCK_SIZES[@]}"; do
     local BLOCK_SIZE=${BLOCK_SIZES[BLOCK_INDEX]}
     local BLOCK_FUNCTION=${BLOCK_FUNCTIONS[BLOCK_INDEX]}
-    local OLDEST_BACKUP_IN_BLOCK_INDEX=$((BLOCK_SIZE + CURRENT_INDEX)) # Not an off-by-one error because a new backup was already saved 
+    local OLDEST_BACKUP_IN_BLOCK_INDEX=$((NUM_BACKUPS - 1 - (BLOCK_SIZE + CURRENT_INDEX))) # Not an off-by-one error because a new backup was already saved 
+    if [ "$OLDEST_BACKUP_IN_BLOCK_INDEX" -lt 0 ]; then
+      break;
+    fi
     local OLDEST_BACKUP_IN_BLOCK
     OLDEST_BACKUP_IN_BLOCK="$(basename "${BACKUPS[OLDEST_BACKUP_IN_BLOCK_INDEX]}")"
 
-    if [[ "$OLDEST_BACKUP_IN_BLOCK" == "" ]]; then
-      break
-    fi
-
     local OLDEST_BACKUP_TIMESTAMP
     OLDEST_BACKUP_TIMESTAMP=$(parse-file-timestamp "${OLDEST_BACKUP_IN_BLOCK:0:19}")
     local BLOCK_COMMAND="$BLOCK_FUNCTION $OLDEST_BACKUP_TIMESTAMP"
diff --git a/test/test.sh b/test/test.sh
index 1becca5..15a2a9e 100755
--- a/test/test.sh
+++ b/test/test.sh
@@ -227,7 +227,11 @@ test-rcon-interface-not-running () {
 }
 
 test-sequential-delete () {
-  for i in $(seq 0 99); do
+  for i in $(seq 0 20); do
+    TIMESTAMP="$(date +%F_%H-%M-%S --date="2021-01-01 +$i hour")"
+    ./backup.sh -d "sequential" -m 30 -i "$TEST_TMP/server/world" -o "$TEST_TMP/backups" -s "$SCREEN_TMP" -f "$TIMESTAMP"
+  done
+  for i in $(seq 20 99); do
     TIMESTAMP="$(date +%F_%H-%M-%S --date="2021-01-01 +$i hour")"
     ./backup.sh -d "sequential" -m 10 -i "$TEST_TMP/server/world" -o "$TEST_TMP/backups" -s "$SCREEN_TMP" -f "$TIMESTAMP"
   done
@@ -293,6 +297,15 @@ test-thinning-delete-long () {
     TIMESTAMP="$(date +%F_%H-%M-%S --date="2021-01-01 +$i day")"
     OUTPUT="$(./backup.sh -v -i "$TEST_TMP/server/world" -o "$TEST_TMP/backups" -s "$SCREEN_TMP" -f "$TIMESTAMP")"
   done
+  UNEXPECTED_TIMESTAMPS=(
+    "2021-01-05_00-00-00"
+    "2021-01-12_00-00-00"
+    "2021-01-24_00-00-00"
+  )
+  for TIMESTAMP in "${UNEXPECTED_TIMESTAMPS[@]}"; do
+    assertFalse '[ -f '"$TEST_TMP/backups/$TIMESTAMP.tar.gz"' ]'
+  done
+  assertEquals 74 "$(find "$TEST_TMP/backups" -type f | wc -l)" 
   EXPECTED_TIMESTAMPS=(
     # Weekly
     "2021-01-04_00-00-00"