From f0852f474b3902a33cff3aedd7fc755230dde8f3 Mon Sep 17 00:00:00 2001 From: "Eloy Lafuente (stronk7)" Date: Sun, 10 Jun 2012 04:57:40 +0200 Subject: [PATCH] MDL-33630 backup: invalidate ids cache on temptable drop. --- .../dbops/restore_controller_dbops.class.php | 2 + backup/util/dbops/restore_dbops.class.php | 25 +++++ backup/util/dbops/tests/dbops_test.php | 98 ++++++++++++++++++- 3 files changed, 123 insertions(+), 2 deletions(-) diff --git a/backup/util/dbops/restore_controller_dbops.class.php b/backup/util/dbops/restore_controller_dbops.class.php index 0b47c5f017b..94674e71e46 100644 --- a/backup/util/dbops/restore_controller_dbops.class.php +++ b/backup/util/dbops/restore_controller_dbops.class.php @@ -125,5 +125,7 @@ abstract class restore_controller_dbops extends restore_dbops { $table = new xmldb_table($targettablename); $dbman->drop_table($table); // And drop it } + // Invalidate the backup_ids caches. + restore_dbops::reset_backup_ids_cached(); } } diff --git a/backup/util/dbops/restore_dbops.class.php b/backup/util/dbops/restore_dbops.class.php index 81dbaf6ea75..4254e698e90 100644 --- a/backup/util/dbops/restore_dbops.class.php +++ b/backup/util/dbops/restore_dbops.class.php @@ -312,6 +312,31 @@ abstract class restore_dbops { } } + /** + * Reset the ids caches completely + * + * Any destructive operation (partial delete, truncate, drop or recreate) performed + * with the backup_ids table must cause the backup_ids caches to be + * invalidated by calling this method. See MDL-33630. + * + * Note that right now, the only operation of that type is the recreation + * (drop & restore) of the table that may happen once the prechecks have ended. All + * the rest of operations are always routed via {@link set_backup_ids_record()}, 1 by 1, + * keeping the caches on sync. + * + * @todo MDL-25290 static should be replaced with MUC code. + */ + public static function reset_backup_ids_cached() { + // Reset the ids cache. + $cachetoadd = count(self::$backupidscache); + self::$backupidscache = array(); + self::$backupidscachesize = self::$backupidscachesize + $cachetoadd; + // Reset the exists cache. + $existstoadd = count(self::$backupidsexist); + self::$backupidsexist = array(); + self::$backupidsexistsize = self::$backupidsexistsize + $existstoadd; + } + /** * Given one role, as loaded from XML, perform the best possible matching against the assignable * roles, using different fallback alternatives (shortname, archetype, editingteacher => teacher, defaultcourseroleid) diff --git a/backup/util/dbops/tests/dbops_test.php b/backup/util/dbops/tests/dbops_test.php index dc02ad9ca38..e088084b2f6 100644 --- a/backup/util/dbops/tests/dbops_test.php +++ b/backup/util/dbops/tests/dbops_test.php @@ -26,10 +26,104 @@ defined('MOODLE_INTERNAL') || die(); // Include all the needed stuff global $CFG; require_once($CFG->dirroot . '/backup/util/includes/backup_includes.php'); +require_once($CFG->dirroot . '/backup/util/includes/restore_includes.php'); +/** + * Restore dbops tests (all). + */ +class restore_dbops_testcase extends advanced_testcase { -/* - * dbops tests (all) + /** + * Verify the xxx_ids_cached (in-memory backup_ids cache) stuff works as expected. + * + * Note that those private implementations are tested here by using the public + * backup_ids API and later performing low-level tests. + */ + public function test_backup_ids_cached() { + global $DB; + $dbman = $DB->get_manager(); // We are going to use database_manager services. + + $this->resetAfterTest(true); // Playing with temp tables, better reset once finished. + + // Some variables and objects for testing. + $restoreid = 'testrestoreid'; + + $mapping = new stdClass(); + $mapping->itemname = 'user'; + $mapping->itemid = 1; + $mapping->newitemid = 2; + $mapping->parentitemid = 3; + $mapping->info = 'info'; + + // Create the backup_ids temp tables used by restore. + restore_controller_dbops::create_restore_temp_tables($restoreid); + + // Send one mapping using the public api with defaults. + restore_dbops::set_backup_ids_record($restoreid, $mapping->itemname, $mapping->itemid); + // Get that mapping and verify everything is returned as expected. + $result = restore_dbops::get_backup_ids_record($restoreid, $mapping->itemname, $mapping->itemid); + $this->assertSame($mapping->itemname, $result->itemname); + $this->assertSame($mapping->itemid, $result->itemid); + $this->assertSame(0, $result->newitemid); + $this->assertSame(null, $result->parentitemid); + $this->assertSame(null, $result->info); + + // Drop the backup_xxx_temp temptables manually, so memory cache won't be invalidated. + $dbman->drop_table(new xmldb_table('backup_ids_temp')); + $dbman->drop_table(new xmldb_table('backup_files_temp')); + + // Verify the mapping continues returning the same info, + // now from cache (the table does not exist). + $result = restore_dbops::get_backup_ids_record($restoreid, $mapping->itemname, $mapping->itemid); + $this->assertSame($mapping->itemname, $result->itemname); + $this->assertSame($mapping->itemid, $result->itemid); + $this->assertSame(0, $result->newitemid); + $this->assertSame(null, $result->parentitemid); + $this->assertSame(null, $result->info); + + // Recreate the temp table, just to drop it using the restore API in + // order to check that, then, the cache becomes invalid for the same request. + restore_controller_dbops::create_restore_temp_tables($restoreid); + restore_controller_dbops::drop_restore_temp_tables($restoreid); + + // No cached info anymore, so the mapping request will arrive to + // DB leading to error (temp table does not exist). + try { + $result = restore_dbops::get_backup_ids_record($restoreid, $mapping->itemname, $mapping->itemid); + $this->fail('Expecting an exception, none occurred'); + } catch (Exception $e) { + $this->assertTrue($e instanceof dml_exception); + $this->assertSame('Table "backup_ids_temp" does not exist', $e->getMessage()); + } + + // Create the backup_ids temp tables once more. + restore_controller_dbops::create_restore_temp_tables($restoreid); + + // Send one mapping using the public api with complete values. + restore_dbops::set_backup_ids_record($restoreid, $mapping->itemname, $mapping->itemid, + $mapping->newitemid, $mapping->parentitemid, $mapping->info); + // Get that mapping and verify everything is returned as expected. + $result = restore_dbops::get_backup_ids_record($restoreid, $mapping->itemname, $mapping->itemid); + $this->assertSame($mapping->itemname, $result->itemname); + $this->assertSame($mapping->itemid, $result->itemid); + $this->assertSame($mapping->newitemid, $result->newitemid); + $this->assertSame($mapping->parentitemid, $result->parentitemid); + $this->assertSame($mapping->info, $result->info); + + // Finally, drop the temp tables properly and get the DB error again (memory caches empty). + restore_controller_dbops::drop_restore_temp_tables($restoreid); + try { + $result = restore_dbops::get_backup_ids_record($restoreid, $mapping->itemname, $mapping->itemid); + $this->fail('Expecting an exception, none occurred'); + } catch (Exception $e) { + $this->assertTrue($e instanceof dml_exception); + $this->assertSame('Table "backup_ids_temp" does not exist', $e->getMessage()); + } + } +} + +/** + * Backup dbops tests (all). */ class backup_dbops_testcase extends advanced_testcase {