Mod. SFW. Atomic approach to updating SFW, phpunit tests

lib/Cleantalk/ApbctWP/DB.php

@@ -180,34 +180,4 @@ public function getLastError()
 
         return $wpdb->last_error;
     }
-
-    /**
-     * Check if all tables exists in DB.
-     * @param array $tables Tables names
-     *
-     * @return bool true if all tables exist, false otherwise
-     */
-    public function tablesExist($tables = array())
-    {
-        if ( ! is_array($tables) || empty($tables) ) {
-            return false;
-        }
-
-        if (count($tables) === 1) {
-            $table = reset($tables);
-            return parent::isTableExists($table);
-        } else {
-            $query = '
-            SELECT 
-                COUNT(TABLE_NAME) as count
-            FROM 
-                INFORMATION_SCHEMA.TABLES 
-            WHERE 
-                TABLE_SCHEMA = DATABASE()
-                AND TABLE_NAME IN (\'' . implode('\', \'', $tables) . '\');
-            ';
-        }
-        $result = $this->fetch($query);
-        return is_array($result) && isset($result['count']) && (int)$result['count'] === count($tables);
-    }
 }

lib/Cleantalk/ApbctWP/Firewall/SFW.php

@@ -132,8 +132,7 @@ public function check()
 
         if (
             empty($this->db__table__data) ||
-            empty($this->db__table__data_personal) ||
-            !$this->db->tablesExist(array($this->db__table__data_personal, $this->db__table__data)) //skip if any of SFW tables missed
+            empty($this->db__table__data_personal)
         ) {
             return $results;
         }
@@ -948,6 +947,73 @@ public static function renameDataTablesFromTempToMain($db, $table_names)
         return true;
     }
 
+    /**
+     * Atomically replace main tables with temp tables
+     * Uses RENAME TABLE which is atomic in MySQL - tables are never missing
+     *
+     * @param DB $db database handler
+     * @param array|string $table_names Array with table names to replace
+     *
+     * @return bool|array
+     */
+    public static function replaceDataTablesAtomically($db, $table_names)
+    {
+        // Cast it to array for simple input
+        $table_names = (array)$table_names;
+
+        $rename_pairs = array();
+        $tables_to_drop = array();
+
+        foreach ($table_names as $table_name) {
+            $table_name_temp = $table_name . '_temp';
+            $table_name_old = $table_name . '_old';
+
+            // Check temp table exists
+            if (!$db->isTableExists($table_name_temp)) {
+                return array('error' => 'ATOMIC RENAME: TEMP TABLE NOT EXISTS: ' . $table_name_temp);
+            }
+
+            // Drop _old table if exists from previous failed update
+            if ($db->isTableExists($table_name_old)) {
+                if ($db->execute('DROP TABLE IF EXISTS `' . $table_name_old . '`;') === false) {
+                    return array(
+                        'error' => 'ATOMIC RENAME: FAILED TO DROP OLD TABLE: ' . $table_name_old
+                                . ' DB Error: ' . $db->getLastError()
+                    );
+                }
+            }
+
+            // Build rename pairs
+            if ($db->isTableExists($table_name)) {
+                // Main exists: main -> old, temp -> main
+                $rename_pairs[] = "`$table_name` TO `$table_name_old`";
+                $rename_pairs[] = "`$table_name_temp` TO `$table_name`";
+                $tables_to_drop[] = $table_name_old;
+            } else {
+                // Main doesn't exist: just temp -> main
+                $rename_pairs[] = "`$table_name_temp` TO `$table_name`";
+            }
+        }
+
+        // Execute atomic rename
+        if (!empty($rename_pairs)) {
+            $query = 'RENAME TABLE ' . implode(', ', $rename_pairs) . ';';
+
+            if ($db->execute($query) === false) {
+                return array(
+                    'error' => 'ATOMIC RENAME: FAILED: ' . $query . ' DB Error: ' . $db->getLastError()
+                );
+            }
+        }
+
+        // Clean up old tables (non-critical)
+        foreach ($tables_to_drop as $table_to_drop) {
+            $db->execute('DROP TABLE IF EXISTS `' . $table_to_drop . '`;');
+        }
+
+        return true;
+    }
+
     /**
      * Add a new records to the SFW table. Duplicates will be updated on "status" field.
      * @param DB $db

lib/Cleantalk/ApbctWP/Firewall/SFWUpdateHelper.php

@@ -189,15 +189,10 @@ public static function removeAndRenameSfwTables($sfw_load_type)
         global $apbct;
 
         if ( $sfw_load_type === 'all' ) {
-            //common table delete
-            $result_deletion = SFW::dataTablesDelete(DB::getInstance(), $apbct->data['sfw_common_table_name']);
-            if ( !empty($result_deletion['error']) ) {
-                throw new \Exception('SFW_COMMON_TABLE_DELETION_ERROR');
-            }
-            //common table rename
-            $result_renaming = SFW::renameDataTablesFromTempToMain(DB::getInstance(), $apbct->data['sfw_common_table_name']);
-            if ( !empty($result_renaming['error']) ) {
-                throw new \Exception('SFW_COMMON_TABLE_RENAME_ERROR');
+            //common table delete and rename in one transaction
+            $result = SFW::replaceDataTablesAtomically(DB::getInstance(), $apbct->data['sfw_common_table_name']);
+            if ( !empty($result['error']) ) {
+                throw new \Exception('SFW_COMMON_TABLE_ATOMIC_RENAME_ERROR: ' . $result['error']);
             }
 
             //personal table delete
@@ -222,15 +217,10 @@ public static function removeAndRenameSfwTables($sfw_load_type)
                 throw new \Exception('SFW_PERSONAL_TABLE_RENAME_ERROR');
             }
         } elseif ( $sfw_load_type === 'common' ) {
-            //common table delete
-            $result_deletion = SFW::dataTablesDelete(DB::getInstance(), $apbct->data['sfw_common_table_name']);
-            if ( !empty($result_deletion['error']) ) {
-                throw new \Exception('SFW_COMMON_TABLE_DELETION_ERROR');
-            }
-            //common table rename
-            $result_renaming = SFW::renameDataTablesFromTempToMain(DB::getInstance(), $apbct->data['sfw_common_table_name']);
-            if ( !empty($result_renaming['error']) ) {
-                throw new \Exception('SFW_COMMON_TABLE_RENAME_ERROR');
+             //common table delete and rename in one transaction
+            $result = SFW::replaceDataTablesAtomically(DB::getInstance(), $apbct->data['sfw_common_table_name']);
+            if ( !empty($result['error']) ) {
+                throw new \Exception('SFW_COMMON_TABLE_ATOMIC_RENAME_ERROR: ' . $result['error']);
             }
         }
     }

tests/ApbctWP/Firewall/TestReplaceDataTablesAtomically.php

@@ -0,0 +1,214 @@
+<?php
+
+use Cleantalk\ApbctWP\DB;
+use Cleantalk\ApbctWP\Firewall\SFW;
+use PHPUnit\Framework\TestCase;
+
+class TestReplaceDataTablesAtomically extends TestCase
+{
+    private $db;
+    private $test_table_name;
+    private $test_table_temp;
+    private $test_table_old;
+
+    public function setUp(): void
+    {
+        $this->db = DB::getInstance();
+        $this->test_table_name = $this->db->prefix . 'test_atomic_rename';
+        $this->test_table_temp = $this->test_table_name . '_temp';
+        $this->test_table_old = $this->test_table_name . '_old';
+
+        // Clean up before each test
+        $this->dropAllTestTables();
+    }
+
+    public function tearDown(): void
+    {
+        // Clean up after each test
+        $this->dropAllTestTables();
+    }
+
+    private function dropAllTestTables(): void
+    {
+        $this->db->execute('DROP TABLE IF EXISTS `' . $this->test_table_name . '`;');
+        $this->db->execute('DROP TABLE IF EXISTS `' . $this->test_table_temp . '`;');
+        $this->db->execute('DROP TABLE IF EXISTS `' . $this->test_table_old . '`;');
+    }
+
+    private function createTable(string $table_name, int $test_value = 1): void
+    {
+        $this->db->execute(
+            'CREATE TABLE `' . $table_name . '` (
+                id INT AUTO_INCREMENT PRIMARY KEY,
+                network INT UNSIGNED NOT NULL,
+                mask INT UNSIGNED NOT NULL,
+                status TINYINT NOT NULL DEFAULT 0
+            );'
+        );
+        // Insert test data to identify which table we're reading from
+        $this->db->execute(
+            'INSERT INTO `' . $table_name . '` (network, mask, status) VALUES (' . $test_value . ', 4294967295, 0);'
+        );
+    }
+
+    private function getTableFirstNetwork(string $table_name): ?int
+    {
+        $result = $this->db->fetch('SELECT network FROM `' . $table_name . '` LIMIT 1;');
+        return $result ? (int)$result['network'] : null;
+    }
+
+    /**
+     * Test: Temp table doesn't exist - should return error
+     */
+    public function testErrorWhenTempTableNotExists(): void
+    {
+        // Create only main table, no temp
+        $this->createTable($this->test_table_name, 100);
+
+        $result = SFW::replaceDataTablesAtomically($this->db, $this->test_table_name);
+
+        $this->assertIsArray($result);
+        $this->assertArrayHasKey('error', $result);
+        $this->assertStringContainsString('TEMP TABLE NOT EXISTS', $result['error']);
+    }
+
+    /**
+     * Test: Main table exists - should be replaced with temp
+     */
+    public function testReplaceExistingMainTable(): void
+    {
+        // Create main table with value 100
+        $this->createTable($this->test_table_name, 100);
+        // Create temp table with value 200
+        $this->createTable($this->test_table_temp, 200);
+
+        $result = SFW::replaceDataTablesAtomically($this->db, $this->test_table_name);
+
+        // Should succeed
+        $this->assertTrue($result);
+
+        // Main table should now have data from temp (200)
+        $this->assertEquals(200, $this->getTableFirstNetwork($this->test_table_name));
+
+        // Temp table should not exist
+        $this->assertFalse($this->db->isTableExists($this->test_table_temp));
+
+        // Old table should be cleaned up
+        $this->assertFalse($this->db->isTableExists($this->test_table_old));
+    }
+
+    /**
+     * Test: Main table doesn't exist - temp should become main
+     */
+    public function testCreateMainFromTemp(): void
+    {
+        // Create only temp table with value 300
+        $this->createTable($this->test_table_temp, 300);
+
+        $result = SFW::replaceDataTablesAtomically($this->db, $this->test_table_name);
+
+        // Should succeed
+        $this->assertTrue($result);
+
+        // Main table should now exist with data from temp
+        $this->assertTrue($this->db->isTableExists($this->test_table_name));
+        $this->assertEquals(300, $this->getTableFirstNetwork($this->test_table_name));
+
+        // Temp table should not exist
+        $this->assertFalse($this->db->isTableExists($this->test_table_temp));
+    }
+
+    /**
+     * Test: Old table exists from previous failed update - should be cleaned up
+     */
+    public function testCleanupExistingOldTable(): void
+    {
+        // Simulate previous failed update: _old table exists
+        $this->createTable($this->test_table_old, 50);
+        $this->createTable($this->test_table_name, 100);
+        $this->createTable($this->test_table_temp, 200);
+
+        $result = SFW::replaceDataTablesAtomically($this->db, $this->test_table_name);
+
+        // Should succeed
+        $this->assertTrue($result);
+
+        // Old table should be cleaned up
+        $this->assertFalse($this->db->isTableExists($this->test_table_old));
+
+        // Main should have new data
+        $this->assertEquals(200, $this->getTableFirstNetwork($this->test_table_name));
+    }
+
+    /**
+     * Test: Multiple tables at once
+     */
+    public function testMultipleTables(): void
+    {
+        $table1 = $this->test_table_name . '_1';
+        $table2 = $this->test_table_name . '_2';
+
+        // Create tables
+        $this->createTable($table1, 100);
+        $this->createTable($table1 . '_temp', 101);
+        $this->createTable($table2, 200);
+        $this->createTable($table2 . '_temp', 201);
+
+        $result = SFW::replaceDataTablesAtomically($this->db, [$table1, $table2]);
+
+        // Should succeed
+        $this->assertTrue($result);
+
+        // Both tables should have new data
+        $this->assertEquals(101, $this->getTableFirstNetwork($table1));
+        $this->assertEquals(201, $this->getTableFirstNetwork($table2));
+
+        // Cleanup
+        $this->db->execute('DROP TABLE IF EXISTS `' . $table1 . '`;');
+        $this->db->execute('DROP TABLE IF EXISTS `' . $table2 . '`;');
+    }
+
+    /**
+     * Test: Atomicity - main table is never missing during operation
+     * This is more of a conceptual test - RENAME TABLE is atomic in MySQL
+     */
+    public function testAtomicityMainTableAlwaysAccessible(): void
+    {
+        $this->createTable($this->test_table_name, 100);
+        $this->createTable($this->test_table_temp, 200);
+
+        // Before rename - main exists
+        $this->assertTrue($this->db->isTableExists($this->test_table_name));
+
+        $result = SFW::replaceDataTablesAtomically($this->db, $this->test_table_name);
+
+        // After rename - main still exists (with new data)
+        $this->assertTrue($result);
+        $this->assertTrue($this->db->isTableExists($this->test_table_name));
+    }
+
+    /**
+     * Test: Empty array input
+     */
+    public function testEmptyArrayInput(): void
+    {
+        $result = SFW::replaceDataTablesAtomically($this->db, []);
+
+        // Should succeed (nothing to do)
+        $this->assertTrue($result);
+    }
+
+    /**
+     * Test: String input is cast to array
+     */
+    public function testStringInputCastToArray(): void
+    {
+        $this->createTable($this->test_table_temp, 500);
+
+        // Pass string instead of array
+        $result = SFW::replaceDataTablesAtomically($this->db, $this->test_table_name);
+
+        $this->assertTrue($result);
+        $this->assertEquals(500, $this->getTableFirstNetwork($this->test_table_name));
+    }
+}