diff --git a/classes/local/location_helper.php b/classes/local/location_helper.php new file mode 100644 index 00000000..9810f3ba --- /dev/null +++ b/classes/local/location_helper.php @@ -0,0 +1,156 @@ +. + +/** + * Helper for converting between bitmask location values and individual DB columns. + * + * The PHP layer works with bitmask integers (OBJECT_LOCATION_IN_FILEDIR | OBJECT_LOCATION_IN_MDL_FILES etc.) + * while the DB stores individual boolean columns (in_filedir, in_mdl_files, in_remote). + * + * @package tool_objectfs + * @author Catalyst IT + * @copyright Catalyst IT + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +namespace tool_objectfs\local; + +defined('MOODLE_INTERNAL') || die(); + +require_once(__DIR__ . '/../../lib.php'); + +/** + * Translates between the PHP bitmask representation and the DB boolean columns. + */ +class location_helper { + /** + * Get the registry mapping each bit flag constant to its corresponding DB column name. + * Must be a method rather than a class constant because OBJECT_LOCATION_* are defined at runtime. + * + * @return array [bit_value => column_name, ...] + */ + public static function get_bit_columns(): array { + return [ + OBJECT_LOCATION_IN_FILEDIR => 'in_filedir', + OBJECT_LOCATION_IN_MDL_FILES => 'in_mdl_files', + OBJECT_LOCATION_IN_REMOTE => 'in_remote', + ]; + } + + /** + * Convert a bitmask to an associative array of column => 0/1 values for DB storage. + * + * @param int $location Bitmask (e.g. OBJECT_LOCATION_DUPLICATED = 7). + * @return array ['in_filedir' => 1, 'in_mdl_files' => 1, 'in_remote' => 1] + */ + public static function bits_to_columns(int $location): array { + $columns = []; + foreach (self::get_bit_columns() as $bit => $column) { + $columns[$column] = ($location & $bit) ? 1 : 0; + } + return $columns; + } + + /** + * Reconstruct a bitmask from a DB row's boolean columns. + * + * @param object $row A DB record containing in_filedir, in_mdl_files, in_remote fields. + * @return int The composed bitmask. + */ + public static function columns_to_bits(object $row): int { + $location = 0; + foreach (self::get_bit_columns() as $bit => $column) { + if (!empty($row->$column)) { + $location |= $bit; + } + } + return $location; + } + + /** + * Generate SQL WHERE conditions for bitmask-based candidate queries. + * + * Converts has_mask/not_mask into column equality checks. + * Example: has_mask=3 (IN_FILEDIR|IN_MDL_FILES), not_mask=4 (IN_REMOTE) + * → "in_filedir = 1 AND in_mdl_files = 1 AND in_remote = 0" + * + * @param int $hasmask Bits that MUST be set. + * @param int $notmask Bits that must NOT be set. + * @return string SQL conditions (without leading WHERE/AND). + */ + public static function bits_to_sql_conditions(int $hasmask, int $notmask): string { + $conditions = []; + foreach (self::get_bit_columns() as $bit => $column) { + if ($hasmask & $bit) { + $conditions[] = $column . ' = 1'; + } + if ($notmask & $bit) { + $conditions[] = $column . ' = 0'; + } + } + return implode(' AND ', $conditions); + } + + /** + * Generate SQL WHERE conditions with a table alias prefix. + * + * @param int $hasmask Bits that MUST be set. + * @param int $notmask Bits that must NOT be set. + * @param string $alias Table alias (e.g. 'o'). + * @return string SQL conditions with alias prefix. + */ + public static function bits_to_sql_conditions_aliased(int $hasmask, int $notmask, string $alias): string { + $conditions = []; + foreach (self::get_bit_columns() as $bit => $column) { + if ($hasmask & $bit) { + $conditions[] = $alias . '.' . $column . ' = 1'; + } + if ($notmask & $bit) { + $conditions[] = $alias . '.' . $column . ' = 0'; + } + } + return implode(' AND ', $conditions); + } + + /** + * Get the column name for a given bit flag. + * + * @param int $bit A single bit flag constant. + * @return string|null Column name or null if not registered. + */ + public static function get_column_for_bit(int $bit): ?string { + return self::get_bit_columns()[$bit] ?? null; + } + + /** + * Generate SQL WHERE conditions for an exact location match. + * + * Unlike bits_to_sql_conditions which only checks specified has/not bits, + * this checks ALL registered bits - those in the bitmask must be 1, those not in it must be 0. + * + * @param int $location The exact bitmask to match. + * @param string $alias Optional table alias prefix. + * @return string SQL conditions. + */ + public static function bits_to_exact_sql(int $location, string $alias = ''): string { + $prefix = $alias !== '' ? $alias . '.' : ''; + $conditions = []; + foreach (self::get_bit_columns() as $bit => $column) { + $conditions[] = $prefix . $column . ' = ' . (($location & $bit) ? '1' : '0'); + } + return implode(' AND ', $conditions); + } +} diff --git a/classes/local/manager.php b/classes/local/manager.php index b5268667..069030c0 100644 --- a/classes/local/manager.php +++ b/classes/local/manager.php @@ -148,11 +148,15 @@ public static function update_object_by_hash($contenthash, $newlocation, $filesi $oldobject = $DB->get_record('tool_objectfs_objects', ['contenthash' => $contenthash]); if ($oldobject) { + // Reconstruct bitmask from stored columns for comparison. + $oldlocation = location_helper::columns_to_bits($oldobject); + $newobject->timeduplicated = $oldobject->timeduplicated; $newobject->id = $oldobject->id; // If location hasn't changed we do not need to update unless filesize is not populated. - if ((int)$oldobject->location === $newlocation && isset($oldobject->filesize)) { + if ($oldlocation === $newlocation && isset($oldobject->filesize)) { + $oldobject->location = $oldlocation; return $oldobject; } @@ -162,7 +166,6 @@ public static function update_object_by_hash($contenthash, $newlocation, $filesi return self::upsert_object($newobject, $newlocation); } - $newobject->location = $newlocation; // Use existing file data related to the object if it exists. $filerecord = $DB->get_record('files', ['contenthash' => $contenthash], 'filesize,timecreated', IGNORE_MULTIPLE); @@ -191,14 +194,28 @@ public static function upsert_object(stdClass $object, $newlocation) { $object->timeduplicated = time(); } - $locationchanged = !isset($object->location) || $object->location != $newlocation; + // Determine if location actually changed. + $oldlocation = isset($object->location) ? $object->location : null; + $locationchanged = $oldlocation === null || $oldlocation != $newlocation; + + // Store the bitmask on the PHP object for callers. $object->location = $newlocation; + // Split bitmask into individual DB columns. + $columns = location_helper::bits_to_columns($newlocation); + foreach ($columns as $col => $val) { + $object->$col = $val; + } + + // Remove the virtual 'location' property before DB write - it's not a real column. + $dbobject = clone $object; + unset($dbobject->location); + // If id is set, update, else insert new. if (empty($object->id)) { - $object->id = $DB->insert_record('tool_objectfs_objects', $object); + $object->id = $DB->insert_record('tool_objectfs_objects', $dbobject); } else { - $DB->update_record('tool_objectfs_objects', $object); + $DB->update_record('tool_objectfs_objects', $dbobject); } // Post update, notify tag manager since the location tag likely needs changing. @@ -210,6 +227,25 @@ public static function upsert_object(stdClass $object, $newlocation) { return $object; } + /** + * Get the location bitmask for an object by its contenthash. + * + * @param string $contenthash + * @return int The location bitmask, or OBJECT_LOCATION_ERROR (0) if not found. + */ + public static function get_location_by_hash(string $contenthash): int { + global $DB; + $record = $DB->get_record( + 'tool_objectfs_objects', + ['contenthash' => $contenthash], + 'in_filedir, in_mdl_files, in_remote' + ); + if (!$record) { + return OBJECT_LOCATION_ERROR; + } + return location_helper::columns_to_bits($record); + } + /** * cloudfront_pem_exists * @return string diff --git a/classes/local/object_manipulator/candidates/candidates_factory.php b/classes/local/object_manipulator/candidates/candidates_factory.php deleted file mode 100644 index f1479aaf..00000000 --- a/classes/local/object_manipulator/candidates/candidates_factory.php +++ /dev/null @@ -1,65 +0,0 @@ -. - -/** - * Class candidates_factory - * @package tool_objectfs - * @author Gleimer Mora - * @copyright Catalyst IT - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ - -namespace tool_objectfs\local\object_manipulator\candidates; - -use moodle_exception; -use stdClass; -use tool_objectfs\local\object_manipulator\checker; -use tool_objectfs\local\object_manipulator\deleter; -use tool_objectfs\local\object_manipulator\puller; -use tool_objectfs\local\object_manipulator\pusher; -use tool_objectfs\local\object_manipulator\recoverer; -use tool_objectfs\local\object_manipulator\orphaner; - -/** - * Candidates Factory - */ -class candidates_factory { - /** @var array $manipulatormap */ - private static $manipulatormap = [ - checker::class => checker_candidates::class, - deleter::class => deleter_candidates::class, - puller::class => puller_candidates::class, - pusher::class => pusher_candidates::class, - recoverer::class => recoverer_candidates::class, - orphaner::class => orphaner_candidates::class, - ]; - - /** - * Finder - * @param mixed $manipulator - * @param stdClass $config - * - * @return mixed - * @throws moodle_exception - */ - public static function finder($manipulator, stdClass $config) { - if (isset(self::$manipulatormap[$manipulator])) { - $classname = self::$manipulatormap[$manipulator]; - return new $classname($config); - } - throw new moodle_exception('invalidclass', 'error', '', 'Invalid manipulator class'); - } -} diff --git a/classes/local/object_manipulator/candidates/candidates_finder.php b/classes/local/object_manipulator/candidates/candidates_finder.php deleted file mode 100644 index 17c266f9..00000000 --- a/classes/local/object_manipulator/candidates/candidates_finder.php +++ /dev/null @@ -1,62 +0,0 @@ -. - -/** - * Class candidates_finder - * @package tool_objectfs - * @author Gleimer Mora - * @copyright Catalyst IT - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ - -namespace tool_objectfs\local\object_manipulator\candidates; - -use moodle_exception; -use stdClass; - -/** - * Candidates Finder - */ -class candidates_finder { - /** @var string $finder */ - private $finder = ''; - - /** - * candidates_finder constructor. - * @param string $manipulator - * @param stdClass $config - * @throws moodle_exception - */ - public function __construct($manipulator, stdClass $config) { - $this->finder = candidates_factory::finder($manipulator, $config); - } - - /** - * get - * @return array - */ - public function get() { - return $this->finder->get(); - } - - /** - * get_query_name - * @return string - */ - public function get_query_name() { - return $this->finder->get_query_name(); - } -} diff --git a/classes/local/object_manipulator/candidates/checker_candidates.php b/classes/local/object_manipulator/candidates/checker_candidates.php deleted file mode 100644 index e52af679..00000000 --- a/classes/local/object_manipulator/candidates/checker_candidates.php +++ /dev/null @@ -1,57 +0,0 @@ -. - -/** - * Class checker_candidates - * @package tool_objectfs - * @author Gleimer Mora - * @copyright Catalyst IT - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ - -namespace tool_objectfs\local\object_manipulator\candidates; - -/** - * chcker_candiates - */ -class checker_candidates extends manipulator_candidates_base { - /** - * queryname - * @var string - */ - protected $queryname = 'get_check_candidates'; - - /** - * get_candiates_sql - * @return string - */ - public function get_candidates_sql() { - return 'SELECT f.contenthash - FROM {files} f - LEFT JOIN {tool_objectfs_objects} o ON f.contenthash = o.contenthash - WHERE f.filesize > 0 - AND o.location is NULL - GROUP BY f.contenthash'; - } - - /** - * get_candidates_sql_params - * @return array - */ - public function get_candidates_sql_params() { - return []; - } -} diff --git a/classes/local/object_manipulator/candidates/deleter_candidates.php b/classes/local/object_manipulator/candidates/deleter_candidates.php deleted file mode 100644 index 5985bd3b..00000000 --- a/classes/local/object_manipulator/candidates/deleter_candidates.php +++ /dev/null @@ -1,62 +0,0 @@ -. - -/** - * Class deleter_candidates - * @package tool_objectfs - * @author Gleimer Mora - * @copyright Catalyst IT - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ - -namespace tool_objectfs\local\object_manipulator\candidates; - -/** - * deleter_candidates - */ -class deleter_candidates extends manipulator_candidates_base { - /** - * queryname - * @var string - */ - protected $queryname = 'get_delete_candidates'; - - /** - * get_candiates_sql - * @return string - */ - public function get_candidates_sql() { - return 'SELECT contenthash, - filesize - FROM {tool_objectfs_objects} - WHERE timeduplicated <= :consistancythreshold - AND location = :location - AND filesize > :sizethreshold'; - } - - /** - * get_candiates_sql_params - * @return array - */ - public function get_candidates_sql_params() { - $consistancythreshold = time() - $this->config->consistencydelay; - return [ - 'consistancythreshold' => $consistancythreshold, - 'location' => OBJECT_LOCATION_DUPLICATED, - 'sizethreshold' => $this->config->sizethreshold, - ]; - } -} diff --git a/classes/local/object_manipulator/candidates/manipulator_candidates.php b/classes/local/object_manipulator/candidates/manipulator_candidates.php deleted file mode 100644 index 234ee2cb..00000000 --- a/classes/local/object_manipulator/candidates/manipulator_candidates.php +++ /dev/null @@ -1,56 +0,0 @@ -. - -namespace tool_objectfs\local\object_manipulator\candidates; - -use dml_exception; - -/** - * Interface manipulator_candidates - * @package tool_objectfs - * @author Gleimer Mora - * @copyright Catalyst IT - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ -interface manipulator_candidates { - /** - * Returns a manipulator query name for logging. - * - * @return string - */ - public function get_query_name(); - - /** - * Returns SQL to retrieve objects for manipulation. - * - * @return string - */ - public function get_candidates_sql(); - - /** - * Returns a list of parameters for SQL from get_candidates_sql. - * - * @return array - */ - public function get_candidates_sql_params(); - - /** - * get - * @return array - * @throws dml_exception - */ - public function get(); -} diff --git a/classes/local/object_manipulator/candidates/manipulator_candidates_base.php b/classes/local/object_manipulator/candidates/manipulator_candidates_base.php deleted file mode 100644 index a542217c..00000000 --- a/classes/local/object_manipulator/candidates/manipulator_candidates_base.php +++ /dev/null @@ -1,67 +0,0 @@ -. - -/** - * Class candidates_factory - * @package tool_objectfs - * @author Gleimer Mora - * @copyright Catalyst IT - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ - -namespace tool_objectfs\local\object_manipulator\candidates; - -use dml_exception; -use stdClass; - -/** - * manipulator_candidates_base - */ -abstract class manipulator_candidates_base implements manipulator_candidates { - /** @var stdClass $config */ - protected $config; - - /** - * manipulator_candidates_base constructor. - * @param stdClass $config - */ - public function __construct(stdClass $config) { - $this->config = $config; - } - - /** - * get_query_name - * @return string - */ - public function get_query_name() { - return $this->queryname; - } - - /** - * get - * @return array - * @throws dml_exception - */ - public function get() { - global $DB; - return $DB->get_records_sql( - $this->get_candidates_sql(), - $this->get_candidates_sql_params(), - 0, - $this->config->batchsize - ); - } -} diff --git a/classes/local/object_manipulator/candidates/orphaner_candidates.php b/classes/local/object_manipulator/candidates/orphaner_candidates.php deleted file mode 100644 index e2cd1db4..00000000 --- a/classes/local/object_manipulator/candidates/orphaner_candidates.php +++ /dev/null @@ -1,58 +0,0 @@ -. - -/** - * Class orphaner_candidates - * @package tool_objectfs - * @author Nathan Mares - * @copyright Catalyst IT - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ - -namespace tool_objectfs\local\object_manipulator\candidates; - -/** - * orphaner_candidates - */ -class orphaner_candidates extends manipulator_candidates_base { - /** - * queryname - * @var string - */ - protected $queryname = 'get_orphan_candidates'; - - /** - * get_candidates_sql - * @return string - */ - public function get_candidates_sql() { - return 'SELECT o.id, o.contenthash, o.location - FROM {tool_objectfs_objects} o - LEFT JOIN {files} f ON o.contenthash = f.contenthash - WHERE f.id is null - AND o.location != :location'; - } - - /** - * get_candidates_sql_params - * @return array - */ - public function get_candidates_sql_params() { - return [ - 'location' => OBJECT_LOCATION_ORPHANED, - ]; - } -} diff --git a/classes/local/object_manipulator/candidates/puller_candidates.php b/classes/local/object_manipulator/candidates/puller_candidates.php deleted file mode 100644 index 4a497df4..00000000 --- a/classes/local/object_manipulator/candidates/puller_candidates.php +++ /dev/null @@ -1,56 +0,0 @@ -. - -/** - * Class puller_candidates - * @package tool_objectfs - * @author Gleimer Mora - * @copyright Catalyst IT - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ - -namespace tool_objectfs\local\object_manipulator\candidates; - -/** - * puller_candidates - */ -class puller_candidates extends manipulator_candidates_base { - /** - * queryname - * @var string - */ - protected $queryname = 'get_pull_candidates'; - - /** - * get_candidates_sql - * @return string - */ - public function get_candidates_sql() { - return 'SELECT contenthash, - filesize - FROM {tool_objectfs_objects} - WHERE filesize <= :sizethreshold - AND location = :location'; - } - - /** - * get_candidates_sql_params - * @return array - */ - public function get_candidates_sql_params() { - return ['sizethreshold' => $this->config->sizethreshold, 'location' => OBJECT_LOCATION_EXTERNAL]; - } -} diff --git a/classes/local/object_manipulator/candidates/pusher_candidates.php b/classes/local/object_manipulator/candidates/pusher_candidates.php deleted file mode 100644 index ae45b05b..00000000 --- a/classes/local/object_manipulator/candidates/pusher_candidates.php +++ /dev/null @@ -1,64 +0,0 @@ -. - -/** - * Class pusher_candidates - * @package tool_objectfs - * @author Gleimer Mora - * @copyright Catalyst IT - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ - -namespace tool_objectfs\local\object_manipulator\candidates; - -/** - * pusher_candidates - */ -class pusher_candidates extends manipulator_candidates_base { - /** - * queryname - * @var string - */ - protected $queryname = 'get_push_candidates'; - - /** - * get_candidates_sql - * @return string - */ - public function get_candidates_sql() { - return 'SELECT contenthash, - filesize - FROM {tool_objectfs_objects} - WHERE filesize > :threshold - AND filesize < :maximum_file_size - AND timeduplicated <= :maxcreatedtimestamp - AND location = :object_location'; - } - - /** - * get_candidates_sql_params - * @return array - */ - public function get_candidates_sql_params() { - $filesystem = new $this->config->filesystem(); - return [ - 'maxcreatedtimestamp' => time() - $this->config->minimumage, - 'threshold' => $this->config->sizethreshold, - 'maximum_file_size' => $filesystem->get_maximum_upload_filesize(), - 'object_location' => OBJECT_LOCATION_LOCAL, - ]; - } -} diff --git a/classes/local/object_manipulator/candidates/recoverer_candidates.php b/classes/local/object_manipulator/candidates/recoverer_candidates.php deleted file mode 100644 index 9e29c868..00000000 --- a/classes/local/object_manipulator/candidates/recoverer_candidates.php +++ /dev/null @@ -1,55 +0,0 @@ -. - -/** - * Class recoverer_candidates - * @package tool_objectfs - * @author Gleimer Mora - * @copyright Catalyst IT - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ - -namespace tool_objectfs\local\object_manipulator\candidates; - -/** - * recoverer_candidates - */ -class recoverer_candidates extends manipulator_candidates_base { - /** - * queryname - * @var string - */ - protected $queryname = 'get_recover_candidates'; - - /** - * get_candidates_sql - * @return string - */ - public function get_candidates_sql() { - return 'SELECT contenthash, - filesize - FROM {tool_objectfs_objects} - WHERE location = :location'; - } - - /** - * get_candidates_sql_params - * @return array - */ - public function get_candidates_sql_params() { - return ['location' => OBJECT_LOCATION_ERROR]; - } -} diff --git a/classes/local/object_manipulator/checker.php b/classes/local/object_manipulator/checker.php index 6a39e160..d737abbf 100644 --- a/classes/local/object_manipulator/checker.php +++ b/classes/local/object_manipulator/checker.php @@ -26,6 +26,7 @@ namespace tool_objectfs\local\object_manipulator; use stdClass; +use tool_objectfs\local\location_helper; use tool_objectfs\local\store\object_file_system; use tool_objectfs\log\aggregate_logger; @@ -33,6 +34,31 @@ * checker */ class checker extends manipulator { + /** + * Get query name for logging. + * @return string + */ + public static function get_query_name(): string { + return 'get_check_candidates'; + } + + /** + * Get files that exist in {files} but have no tracking row or are in error state. + * @param stdClass $config Plugin config. + * @return array + */ + public static function get_candidates(stdClass $config): array { + global $DB; + $errorconditions = location_helper::bits_to_exact_sql(OBJECT_LOCATION_ERROR, 'o'); + $sql = "SELECT f.contenthash + FROM {files} f + LEFT JOIN {tool_objectfs_objects} o ON f.contenthash = o.contenthash + WHERE f.filesize > 0 + AND (o.id IS NULL OR ({$errorconditions})) + GROUP BY f.contenthash"; + return $DB->get_records_sql($sql, [], 0, $config->batchsize * 10); + } + /** * Checker constructor. * This manipulator adds location for files that do not have records in {tool_objectfs_objects} table. diff --git a/classes/local/object_manipulator/deleter.php b/classes/local/object_manipulator/deleter.php index 7afa7b74..98432e2d 100644 --- a/classes/local/object_manipulator/deleter.php +++ b/classes/local/object_manipulator/deleter.php @@ -26,6 +26,7 @@ namespace tool_objectfs\local\object_manipulator; use stdClass; +use tool_objectfs\local\location_helper; use tool_objectfs\local\store\object_file_system; use tool_objectfs\log\aggregate_logger; @@ -49,6 +50,37 @@ class deleter extends manipulator { */ private $deletelocal; + /** + * Get query name for logging. + * @return string + */ + public static function get_query_name(): string { + return 'get_delete_candidates'; + } + + /** + * Get candidate objects to delete from local storage. + * @param stdClass $config Plugin config. + * @return array + */ + public static function get_candidates(stdClass $config): array { + global $DB; + $locationconds = location_helper::bits_to_sql_conditions( + OBJECT_LOCATION_IN_FILEDIR | OBJECT_LOCATION_IN_MDL_FILES | OBJECT_LOCATION_IN_REMOTE, + 0 + ) ?: '1=1'; + $sql = "SELECT contenthash, filesize + FROM {tool_objectfs_objects} + WHERE {$locationconds} + AND filesize > :threshold + AND timeduplicated <= :maxage"; + $params = [ + 'threshold' => $config->sizethreshold, + 'maxage' => time() - $config->consistencydelay, + ]; + return $DB->get_records_sql($sql, $params, 0, $config->batchsize); + } + /** * deleter constructor. * @param object_file_system $filesystem diff --git a/classes/local/object_manipulator/manipulator_builder.php b/classes/local/object_manipulator/manipulator_builder.php index 52745279..13e01d8b 100644 --- a/classes/local/object_manipulator/manipulator_builder.php +++ b/classes/local/object_manipulator/manipulator_builder.php @@ -29,7 +29,6 @@ use moodle_exception; use stdClass; use tool_objectfs\local\manager; -use tool_objectfs\local\object_manipulator\candidates\candidates_finder; use tool_objectfs\log\aggregate_logger; defined('MOODLE_INTERNAL') || die(); @@ -53,9 +52,6 @@ class manipulator_builder { /** @var string $manipulatorclass */ private $manipulatorclass; - /** @var candidates_finder $finder */ - private $finder; - /** @var stdClass $config */ private $config; @@ -107,10 +103,9 @@ private function build($manipulator) { $this->config = manager::get_objectfs_config(); $this->manipulatorclass = $manipulator; $this->logger = new aggregate_logger(); - $this->finder = new candidates_finder($manipulator, $this->config); - $this->candidates = $this->finder->get(); + $this->candidates = $manipulator::get_candidates($this->config); $countcandidates = count($this->candidates); - $this->logger->log_object_query($this->finder->get_query_name(), $countcandidates); + $this->logger->log_object_query($manipulator::get_query_name(), $countcandidates); if ($countcandidates === 0) { mtrace('No candidate objects found.'); } diff --git a/classes/local/object_manipulator/orphaner.php b/classes/local/object_manipulator/orphaner.php index 3576ddc3..cbaccdde 100644 --- a/classes/local/object_manipulator/orphaner.php +++ b/classes/local/object_manipulator/orphaner.php @@ -30,11 +30,36 @@ namespace tool_objectfs\local\object_manipulator; use stdClass; +use tool_objectfs\local\location_helper; /** * orphaner */ class orphaner extends manipulator { + /** + * Get query name for logging. + * @return string + */ + public static function get_query_name(): string { + return 'get_orphan_candidates'; + } + + /** + * Get tracked objects that no longer have a reference in {files}. + * @param stdClass $config Plugin config. + * @return array + */ + public static function get_candidates(stdClass $config): array { + global $DB; + $notorphaned = 'NOT (' . location_helper::bits_to_exact_sql(OBJECT_LOCATION_ORPHANED, 'o') . ')'; + $sql = "SELECT o.id, o.contenthash + FROM {tool_objectfs_objects} o + LEFT JOIN {files} f ON o.contenthash = f.contenthash + WHERE f.id is null + AND {$notorphaned}"; + return $DB->get_records_sql($sql, [], 0, $config->batchsize); + } + /** * Updates the location of {tool_objectfs_objects} records for files that * have been deleted from the core {files} table. diff --git a/classes/local/object_manipulator/puller.php b/classes/local/object_manipulator/puller.php index 6a5c18ea..1e9243ea 100644 --- a/classes/local/object_manipulator/puller.php +++ b/classes/local/object_manipulator/puller.php @@ -26,11 +26,38 @@ namespace tool_objectfs\local\object_manipulator; use stdClass; +use tool_objectfs\local\location_helper; /** * puller */ class puller extends manipulator { + /** + * Get query name for logging. + * @return string + */ + public static function get_query_name(): string { + return 'get_pull_candidates'; + } + + /** + * Get candidate objects to pull from remote storage. + * @param stdClass $config Plugin config. + * @return array + */ + public static function get_candidates(stdClass $config): array { + global $DB; + $locationconds = location_helper::bits_to_sql_conditions( + OBJECT_LOCATION_IN_MDL_FILES | OBJECT_LOCATION_IN_REMOTE, + OBJECT_LOCATION_IN_FILEDIR + ); + $sql = "SELECT contenthash, filesize + FROM {tool_objectfs_objects} + WHERE {$locationconds} + AND filesize <= :size_ceiling"; + return $DB->get_records_sql($sql, ['size_ceiling' => $config->sizethreshold], 0, $config->batchsize); + } + /** * manipulate_object * @param stdClass $objectrecord diff --git a/classes/local/object_manipulator/pusher.php b/classes/local/object_manipulator/pusher.php index fa301971..eb2668d9 100644 --- a/classes/local/object_manipulator/pusher.php +++ b/classes/local/object_manipulator/pusher.php @@ -26,6 +26,7 @@ namespace tool_objectfs\local\object_manipulator; use stdClass; +use tool_objectfs\local\location_helper; use tool_objectfs\local\store\object_file_system; use tool_objectfs\log\aggregate_logger; @@ -47,6 +48,40 @@ class pusher extends manipulator { */ private $maximumfilesize; + /** + * Get query name for logging. + * @return string + */ + public static function get_query_name(): string { + return 'get_push_candidates'; + } + + /** + * Get candidate objects to push to remote storage. + * @param stdClass $config Plugin config. + * @return array + */ + public static function get_candidates(stdClass $config): array { + global $DB; + $filesystem = new $config->filesystem(); + $locationconds = location_helper::bits_to_sql_conditions( + OBJECT_LOCATION_IN_FILEDIR | OBJECT_LOCATION_IN_MDL_FILES, + OBJECT_LOCATION_IN_REMOTE + ); + $sql = "SELECT contenthash, filesize + FROM {tool_objectfs_objects} + WHERE {$locationconds} + AND filesize > :threshold + AND filesize < :max_filesize + AND timeduplicated <= :maxage"; + $params = [ + 'threshold' => $config->sizethreshold, + 'max_filesize' => $filesystem->get_maximum_upload_filesize(), + 'maxage' => time() - $config->minimumage, + ]; + return $DB->get_records_sql($sql, $params, 0, $config->batchsize); + } + /** * pusher constructor. * @param object_file_system $filesystem diff --git a/classes/local/object_manipulator/recoverer.php b/classes/local/object_manipulator/recoverer.php index cceb6a59..147bd923 100644 --- a/classes/local/object_manipulator/recoverer.php +++ b/classes/local/object_manipulator/recoverer.php @@ -26,17 +26,45 @@ namespace tool_objectfs\local\object_manipulator; use stdClass; +use tool_objectfs\local\location_helper; /** * recoverer */ class recoverer extends manipulator { + /** + * Get query name for logging. + * @return string + */ + public static function get_query_name(): string { + return 'get_recover_candidates'; + } + + /** + * Get candidate objects to recover from error state. + * @param stdClass $config Plugin config. + * @return array + */ + public static function get_candidates(stdClass $config): array { + global $DB; + $locationconds = location_helper::bits_to_sql_conditions( + OBJECT_LOCATION_IN_MDL_FILES, + OBJECT_LOCATION_IN_FILEDIR | OBJECT_LOCATION_IN_REMOTE + ); + $sql = "SELECT contenthash, filesize + FROM {tool_objectfs_objects} + WHERE {$locationconds}"; + return $DB->get_records_sql($sql, [], 0, $config->batchsize); + } + /** * manipulate_object * @param stdClass $objectrecord * @return int */ public function manipulate_object(stdClass $objectrecord) { - return $this->filesystem->get_object_location_from_hash($objectrecord->contenthash); + // The recoverer only knows the object is in the objectfs table, not necessarily + // in mdl_files, so pass 0 to check all location bits fresh. + return $this->filesystem->get_object_location_from_hash($objectrecord->contenthash, 0); } } diff --git a/classes/local/report/location_report_builder.php b/classes/local/report/location_report_builder.php index 99cb4da4..8bc50063 100644 --- a/classes/local/report/location_report_builder.php +++ b/classes/local/report/location_report_builder.php @@ -25,6 +25,7 @@ namespace tool_objectfs\local\report; +use tool_objectfs\local\location_helper; use tool_objectfs\local\manager; use tool_objectfs\local\store\object_file_system; @@ -46,7 +47,7 @@ public function build_report($reportid) { OBJECT_LOCATION_DUPLICATED, OBJECT_LOCATION_EXTERNAL, OBJECT_LOCATION_ORPHANED, - OBJECT_LOCATION_ERROR, + OBJECT_LOCATION_MISSING, ]; $totalcount = 0; @@ -54,12 +55,14 @@ public function build_report($reportid) { $filedircount = 0; $filedirsum = 0; foreach ($locations as $location) { + $locationwhere = location_helper::bits_to_exact_sql($location, 'o'); + $sql = - 'WITH + "WITH cte_objects AS ( - SELECT o.contenthash, o.location + SELECT o.contenthash FROM {tool_objectfs_objects} o - WHERE o.location = ? ), + WHERE {$locationwhere} ), cte_obj_files AS ( SELECT f.contenthash, MAX(f.filesize) AS filesize FROM {files} f @@ -68,39 +71,40 @@ public function build_report($reportid) { GROUP BY f.contenthash, f.filesize) SELECT COALESCE(COUNT(cof.contenthash),0) AS objectcount, COALESCE(SUM(cof.filesize),0) AS objectsum - FROM cte_obj_files cof'; + FROM cte_obj_files cof"; if ($location == OBJECT_LOCATION_LOCAL) { + $localwhere = location_helper::bits_to_exact_sql($location, 'co'); $sql = - 'WITH + "WITH cte_objects AS ( - SELECT o.contenthash, o.location + SELECT o.contenthash, o.in_filedir, o.in_mdl_files, o.in_remote FROM {tool_objectfs_objects} o ), cte_obj_files AS ( SELECT f.contenthash, MAX(f.filesize) AS filesize FROM {files} f LEFT JOIN cte_objects co ON f.contenthash = co.contenthash - WHERE filesize > 0 AND ( co.location = ? OR co.location IS NULL ) + WHERE filesize > 0 AND ( {$localwhere} OR co.in_filedir IS NULL ) GROUP BY f.contenthash, f.filesize) SELECT COALESCE(COUNT(cof.contenthash),0) AS objectcount, COALESCE(SUM(cof.filesize),0) AS objectsum - FROM cte_obj_files cof'; + FROM cte_obj_files cof"; } if ($location !== OBJECT_LOCATION_ORPHANED) { // Process the query normally. - $result = $DB->get_record_sql($sql, [$location]); + $result = $DB->get_record_sql($sql); } else if ($location === OBJECT_LOCATION_ORPHANED) { // Start the query from objectfs, for ORPHANED objects, they are not located in the files table. $sql = - 'WITH + "WITH cte_objects AS ( SELECT o.contenthash FROM {tool_objectfs_objects} o - WHERE o.location = ?) + WHERE {$locationwhere}) SELECT COALESCE(COUNT(co.contenthash),0) AS objectcount - FROM cte_objects co'; - $result = $DB->get_record_sql($sql, [$location]); + FROM cte_objects co"; + $result = $DB->get_record_sql($sql); $result->objectsum = 0; } diff --git a/classes/local/report/object_location_history_table.php b/classes/local/report/object_location_history_table.php index faee6163..cbcfac70 100644 --- a/classes/local/report/object_location_history_table.php +++ b/classes/local/report/object_location_history_table.php @@ -136,7 +136,7 @@ public function query_db($pagesize, $useinitialsbar = true) { $duplicatedrecord = $rawrecords[$id . OBJECT_LOCATION_DUPLICATED] ?? $emptyrecord; $orphanedrecord = $rawrecords[$id . OBJECT_LOCATION_ORPHANED] ?? $emptyrecord; $externalrecord = $rawrecords[$id . OBJECT_LOCATION_EXTERNAL] ?? $emptyrecord; - $errorrecord = $rawrecords[$id . OBJECT_LOCATION_ERROR] ?? $emptyrecord; + $missingrecord = $rawrecords[$id . OBJECT_LOCATION_MISSING] ?? $emptyrecord; $filedir = $rawrecords[$id . 'filedir'] ?? $emptyrecord; $total = $rawrecords[$id . 'total'] ?? $emptyrecord; @@ -154,8 +154,8 @@ public function query_db($pagesize, $useinitialsbar = true) { $row['orphaned_size'] = get_string('object_status:location:orphanedsizeunknown', 'tool_objectfs'); $row['external_count'] = $externalrecord->count; $row['external_size'] = $externalrecord->size; - $row['missing_count'] = $errorrecord->count; - $row['missing_size'] = $errorrecord->size; + $row['missing_count'] = $missingrecord->count; + $row['missing_size'] = $missingrecord->size; $row['total_count'] = $total->count; $row['total_size'] = $total->size; $row['filedir_count'] = $filedir->count; @@ -167,7 +167,7 @@ public function query_db($pagesize, $useinitialsbar = true) { $row['duplicated_count'] = number_format($duplicatedrecord->count); $row['orphaned_count'] = number_format($orphanedrecord->count); $row['external_count'] = number_format($externalrecord->count); - $row['missing_count'] = number_format($errorrecord->count); + $row['missing_count'] = number_format($missingrecord->count); $row['total_count'] = number_format($total->count); $row['filedir_count'] = number_format($filedir->count); $row['delta_count'] = number_format($deltacount); @@ -176,7 +176,7 @@ public function query_db($pagesize, $useinitialsbar = true) { $this->duplicatedsizes[] = $this->size_to_mb($duplicatedrecord->size); $this->orphanedsizes[] = $this->size_to_mb($orphanedrecord->size); $this->externalsizes[] = $this->size_to_mb($externalrecord->size); - $this->missingsizes[] = $this->size_to_mb($errorrecord->size); + $this->missingsizes[] = $this->size_to_mb($missingrecord->size); $this->totalsizes[] = $this->size_to_mb($total->size); $this->filedirsizes[] = $this->size_to_mb($filedir->size); $this->deltasizes[] = $this->size_to_mb($deltasize); diff --git a/classes/local/report/object_status_history_table.php b/classes/local/report/object_status_history_table.php index f037e294..c31d6606 100644 --- a/classes/local/report/object_status_history_table.php +++ b/classes/local/report/object_status_history_table.php @@ -240,11 +240,11 @@ public function get_file_location_string($filelocation) { 'filedir' => 'object_status:filedir', 'deltaa' => 'object_status:delta:a', 'deltab' => 'object_status:delta:b', - OBJECT_LOCATION_ERROR => 'object_status:location:error', + OBJECT_LOCATION_MISSING => 'object_status:location:missing', + OBJECT_LOCATION_ORPHANED => 'object_status:location:orphaned', OBJECT_LOCATION_LOCAL => 'object_status:location:local', OBJECT_LOCATION_DUPLICATED => 'object_status:location:duplicated', OBJECT_LOCATION_EXTERNAL => 'object_status:location:external', - OBJECT_LOCATION_ORPHANED => 'object_status:location:orphaned', ]; if (isset($locationstringmap[$filelocation])) { return get_string($locationstringmap[$filelocation], 'tool_objectfs'); @@ -262,7 +262,7 @@ public function get_file_location_class($filelocation): string { switch ($filelocation) { case 'deltaa': case 'deltab': - case OBJECT_LOCATION_ERROR: + case OBJECT_LOCATION_MISSING: case OBJECT_LOCATION_ORPHANED: $class = 'table-danger'; break; diff --git a/classes/local/store/object_file_system.php b/classes/local/store/object_file_system.php index d7df5b93..0a18f94a 100644 --- a/classes/local/store/object_file_system.php +++ b/classes/local/store/object_file_system.php @@ -306,24 +306,35 @@ public function is_file_readable_externally_by_hash($contenthash) { /** * get_object_location_from_hash * @param mixed $contenthash - * + * @param int $knownlocations Bitmask of locations already known to be present without re-checking. + * Defaults to OBJECT_LOCATION_IN_MDL_FILES since most callers reach + * this function via a stored_file or mdl_files reference. + * Pass 0 when the caller only knows the object is in the objectfs + * table (e.g. the recoverer), so all bits are checked fresh. * @return int */ - public function get_object_location_from_hash($contenthash) { - $localreadable = $this->is_file_readable_locally_by_hash($contenthash); - $externalreadable = $this->is_file_readable_externally_by_hash($contenthash); + public function get_object_location_from_hash($contenthash, $knownlocations = OBJECT_LOCATION_IN_MDL_FILES) { + $location = $knownlocations; - if ($localreadable && $externalreadable) { - return OBJECT_LOCATION_DUPLICATED; - } else if ($localreadable && !$externalreadable) { - return OBJECT_LOCATION_LOCAL; - } else if (!$localreadable && $externalreadable) { - return OBJECT_LOCATION_EXTERNAL; - } else { - // Object is not anywhere - we toggle an error state in the DB. - manager::update_object_by_hash($contenthash, OBJECT_LOCATION_ERROR); - return OBJECT_LOCATION_ERROR; + // Only check each location if not already known — avoids redundant DB/filesystem calls. + if (!($location & OBJECT_LOCATION_IN_MDL_FILES)) { + global $DB; + if ($DB->record_exists('files', ['contenthash' => $contenthash])) { + $location |= OBJECT_LOCATION_IN_MDL_FILES; + } + } + if (!($location & OBJECT_LOCATION_IN_FILEDIR) && $this->is_file_readable_locally_by_hash($contenthash)) { + $location |= OBJECT_LOCATION_IN_FILEDIR; } + if (!($location & OBJECT_LOCATION_IN_REMOTE) && $this->is_file_readable_externally_by_hash($contenthash)) { + $location |= OBJECT_LOCATION_IN_REMOTE; + } + + if ($location === OBJECT_LOCATION_MISSING) { + // Object exists in mdl_files but is not anywhere physically - record missing state. + manager::update_object_by_hash($contenthash, OBJECT_LOCATION_MISSING); + } + return $location; } /** @@ -487,7 +498,7 @@ public function readfile(\stored_file $file) { $this->logger->log_object_read('readfile', $path, $file->get_filesize()); if ($success === false) { - manager::update_object_by_hash($file->get_contenthash(), OBJECT_LOCATION_ERROR); + manager::update_object_by_hash($file->get_contenthash(), OBJECT_LOCATION_MISSING); } } } @@ -517,7 +528,7 @@ public function get_content(\stored_file $file) { $this->logger->log_object_read('file_get_contents', $path, $file->get_filesize()); if (!$contents) { - manager::update_object_by_hash($file->get_contenthash(), OBJECT_LOCATION_ERROR); + manager::update_object_by_hash($file->get_contenthash(), OBJECT_LOCATION_MISSING); } return $contents; @@ -627,7 +638,7 @@ public function get_content_file_handle(\stored_file $file, $type = \stored_file $this->logger->log_object_read('get_file_handle_for_path', $path, $file->get_filesize()); if (!$filehandle) { - manager::update_object_by_hash($file->get_contenthash(), OBJECT_LOCATION_ERROR); + manager::update_object_by_hash($file->get_contenthash(), OBJECT_LOCATION_MISSING); } return $filehandle; @@ -764,6 +775,7 @@ public function delete_object_from_hash($contenthash) { $this->delete_external_file_from_hash($contenthash); break; + case OBJECT_LOCATION_MISSING: case OBJECT_LOCATION_ERROR: default: return; @@ -1317,8 +1329,7 @@ private function can_set_object_tags(string $contenthash): bool { */ private function is_file_stored_externally_by_hash(string $contenthash): bool { global $DB; - $location = (int) $DB->get_field('tool_objectfs_objects', 'location', ['contenthash' => $contenthash]); - - return $location === OBJECT_LOCATION_DUPLICATED || $location === OBJECT_LOCATION_EXTERNAL; + $record = $DB->get_record('tool_objectfs_objects', ['contenthash' => $contenthash], 'in_remote'); + return $record && !empty($record->in_remote); } } diff --git a/classes/local/store/s3/file_system.php b/classes/local/store/s3/file_system.php index 029f66fd..8787d518 100644 --- a/classes/local/store/s3/file_system.php +++ b/classes/local/store/s3/file_system.php @@ -71,7 +71,7 @@ public function readfile(\stored_file $file) { $this->get_logger()->log_object_read('readfile', $path, $file->get_filesize()); if ($success === false) { - manager::update_object_by_hash($file->get_contenthash(), OBJECT_LOCATION_ERROR); + manager::update_object_by_hash($file->get_contenthash(), OBJECT_LOCATION_MISSING); throw new \file_exception('storedfilecannotreadfile', $file->get_filename()); } } diff --git a/classes/local/table/files_table.php b/classes/local/table/files_table.php index a4c6f5e7..84674d07 100644 --- a/classes/local/table/files_table.php +++ b/classes/local/table/files_table.php @@ -46,8 +46,8 @@ public function __construct($uniqueid, $objectlocation) { $from = '{files} f'; $from .= ' LEFT JOIN {tool_objectfs_objects} o on f.contenthash = o.contenthash'; $from .= ' LEFT JOIN {context} ctx ON f.contextid = ctx.id'; - $where = 'o.location = ?'; - $params = [$objectlocation]; + $where = \tool_objectfs\local\location_helper::bits_to_exact_sql((int)$objectlocation, 'o'); + $params = []; $this->columns = $this->headers = ['id', 'contextid', 'contenthash', 'localpath', 'link', 'component', 'filearea', 'filename', 'filepath', 'mimetype', 'filesize', 'timecreated']; diff --git a/classes/local/tag/location_source.php b/classes/local/tag/location_source.php index 1353a03a..9ff3ec2d 100644 --- a/classes/local/tag/location_source.php +++ b/classes/local/tag/location_source.php @@ -49,8 +49,9 @@ public static function get_description(): string { public function get_value_for_contenthash(string $contenthash): ?string { global $DB; - $isorphaned = $DB->record_exists('tool_objectfs_objects', ['contenthash' => $contenthash, - 'location' => OBJECT_LOCATION_ORPHANED]); + $columns = \tool_objectfs\local\location_helper::bits_to_columns(OBJECT_LOCATION_ORPHANED); + $conditions = array_merge(['contenthash' => $contenthash], $columns); + $isorphaned = $DB->record_exists('tool_objectfs_objects', $conditions); return $isorphaned ? 'orphan' : 'active'; } diff --git a/classes/local/tag/tag_manager.php b/classes/local/tag/tag_manager.php index 2866329a..cb750bcf 100644 --- a/classes/local/tag/tag_manager.php +++ b/classes/local/tag/tag_manager.php @@ -138,13 +138,15 @@ public static function get_objects_needing_sync(int $limit) { global $DB; // Find object records where the status is NEEDS_SYNC and is replicated. - [$insql, $inparams] = $DB->get_in_or_equal([ - OBJECT_LOCATION_DUPLICATED, OBJECT_LOCATION_EXTERNAL, OBJECT_LOCATION_ORPHANED], SQL_PARAMS_NAMED); - $inparams['syncstatus'] = self::SYNC_STATUS_NEEDS_SYNC; + $locationconditions = '(' . + '(' . \tool_objectfs\local\location_helper::bits_to_exact_sql(OBJECT_LOCATION_DUPLICATED) . ')' . + ' OR (' . \tool_objectfs\local\location_helper::bits_to_exact_sql(OBJECT_LOCATION_EXTERNAL) . ')' . + ' OR (' . \tool_objectfs\local\location_helper::bits_to_exact_sql(OBJECT_LOCATION_ORPHANED) . ')' . + ')'; $records = $DB->get_records_select( 'tool_objectfs_objects', - 'tagsyncstatus = :syncstatus AND location ' . $insql, - $inparams, + "tagsyncstatus = :syncstatus AND {$locationconditions}", + ['syncstatus' => self::SYNC_STATUS_NEEDS_SYNC], '', 'contenthash', 0, diff --git a/classes/log/aggregate_logger.php b/classes/log/aggregate_logger.php index 406ea584..63694e66 100644 --- a/classes/log/aggregate_logger.php +++ b/classes/log/aggregate_logger.php @@ -64,7 +64,8 @@ class aggregate_logger extends objectfs_logger { public function __construct() { parent::__construct(); $this->movestatistics = [ - OBJECT_LOCATION_ERROR => [], + OBJECT_LOCATION_MISSING => [], + OBJECT_LOCATION_ORPHANED => [], OBJECT_LOCATION_LOCAL => [], OBJECT_LOCATION_DUPLICATED => [], OBJECT_LOCATION_EXTERNAL => [], @@ -157,16 +158,16 @@ protected function output_move_statistic($movestatistic, $initiallocation, $fina */ public function location_to_string($location) { switch ($location) { - case OBJECT_LOCATION_ERROR: - return 'error'; + case OBJECT_LOCATION_MISSING: + return 'missing'; + case OBJECT_LOCATION_ORPHANED: + return 'orphaned'; case OBJECT_LOCATION_LOCAL: return 'local'; case OBJECT_LOCATION_DUPLICATED: return 'duplicated'; case OBJECT_LOCATION_EXTERNAL: return 'remote'; - case OBJECT_LOCATION_ORPHANED: - return 'orphaned'; default: return $location; } diff --git a/classes/task/delete_orphaned_object_metadata.php b/classes/task/delete_orphaned_object_metadata.php index 10b4ac67..40c23453 100644 --- a/classes/task/delete_orphaned_object_metadata.php +++ b/classes/task/delete_orphaned_object_metadata.php @@ -51,8 +51,8 @@ public function execute() { return; } + $orphanconditions = \tool_objectfs\local\location_helper::bits_to_exact_sql(OBJECT_LOCATION_ORPHANED, 'o'); $params = [ - 'location' => OBJECT_LOCATION_ORPHANED, 'ageforremoval' => time() - $ageforremoval, ]; @@ -64,7 +64,7 @@ public function execute() { $sql = 'SELECT o.* FROM {tool_objectfs_objects} o LEFT JOIN {files} f ON o.contenthash = f.contenthash - WHERE f.id is null AND o.location = :location AND timeduplicated < :ageforremoval'; + WHERE f.id is null AND {$orphanconditions} AND timeduplicated < :ageforremoval'; $objects = $DB->get_recordset_sql($sql, $params); $count = 0; @@ -79,7 +79,8 @@ public function execute() { mtrace("Deleted $count orphaned files and their metadata (orphaned tool_objectfs_objects)"); } else { // Delete external files is turned off, we only delete the metadata. - $wheresql = 'location = :location and timeduplicated < :ageforremoval'; + $orphanconditions2 = \tool_objectfs\local\location_helper::bits_to_exact_sql(OBJECT_LOCATION_ORPHANED); + $wheresql = "{$orphanconditions2} and timeduplicated < :ageforremoval"; $count = $DB->count_records_select('tool_objectfs_objects', $wheresql, $params); if (!empty($count)) { mtrace("Deleting $count records with orphaned metadata (orphaned tool_objectfs_objects)"); diff --git a/classes/task/populate_objects_filesize.php b/classes/task/populate_objects_filesize.php index 98936f0a..a76a1eed 100644 --- a/classes/task/populate_objects_filesize.php +++ b/classes/task/populate_objects_filesize.php @@ -42,14 +42,19 @@ public function execute() { $maxupdates = !empty($data->maxupdates) ? $data->maxupdates : self::MAX_UPDATES; // Get all objects without a filesize and join them to a filesize from the files table. - // Values less than 0 for object's location indicate an error for the object. - $sql = "SELECT o.id, o.contenthash, o.timeduplicated, o.location, f.filesize + // Exclude error objects (those with all location bits = 0). + $sql = "SELECT o.id, o.contenthash, o.timeduplicated, o.in_filedir, o.in_mdl_files, o.in_remote, f.filesize FROM {tool_objectfs_objects} o JOIN {files} f ON o.contenthash = f.contenthash WHERE o.filesize IS NULL - AND o.location >= 0 + AND o.in_mdl_files = 1 + AND (o.in_filedir = 1 OR o.in_remote = 1) GROUP BY o.id, o.contenthash, + o.timeduplicated, + o.in_filedir, + o.in_mdl_files, + o.in_remote, f.filesize"; $records = $DB->get_recordset_sql($sql, null, 0, $maxupdates + 1); diff --git a/classes/task/reconcile_filedir.php b/classes/task/reconcile_filedir.php index 98cdb895..bb3c01e8 100644 --- a/classes/task/reconcile_filedir.php +++ b/classes/task/reconcile_filedir.php @@ -64,7 +64,7 @@ public function execute(): void { // The config deletelocal must be enabled. if (!get_config('tool_objectfs', 'deletelocal')) { - mtrace('ObjectFS: deletelocal disabled — skipping filedir reconciliation.'); + mtrace('ObjectFS: deletelocal disabled - skipping filedir reconciliation.'); return; } @@ -75,7 +75,7 @@ public function execute(): void { try { $filesystem = new $this->config->filesystem(); } catch (\Throwable $e) { - mtrace('ObjectFS: Could not instantiate filesystem — will register new files as local only. ' . $e->getMessage()); + mtrace('ObjectFS: Could not instantiate filesystem - will register new files as local only. ' . $e->getMessage()); } } @@ -295,7 +295,8 @@ public function execute(): void { } else { // If ObjectFS thinks the file is remote-only, // update it so it knows the file is duplicated. - if ($object->location == OBJECT_LOCATION_EXTERNAL) { + $objectlocation = \tool_objectfs\local\location_helper::columns_to_bits($object); + if ($objectlocation == OBJECT_LOCATION_EXTERNAL) { manager::upsert_object($object, OBJECT_LOCATION_DUPLICATED); $updated++; } diff --git a/classes/tests/testcase.php b/classes/tests/testcase.php index 9384e120..382ef848 100644 --- a/classes/tests/testcase.php +++ b/classes/tests/testcase.php @@ -21,7 +21,6 @@ use stdClass; use stored_file; use tool_objectfs\local\manager; -use tool_objectfs\local\object_manipulator\candidates\candidates_finder; use tool_objectfs\local\store\object_file_system; use tool_objectfs\local\store\signed_url; @@ -180,7 +179,7 @@ protected function create_error_file() { $file = $this->create_local_file(); $path = $this->get_local_path_from_storedfile($file); unlink($path); - manager::update_object_by_hash($file->get_contenthash(), OBJECT_LOCATION_ERROR); + manager::update_object_by_hash($file->get_contenthash(), OBJECT_LOCATION_MISSING); return $file; } @@ -311,7 +310,7 @@ protected function create_remote_object($content = 'remote object content') { */ protected function create_error_object($content = 'error object content') { $file = $this->create_error_file($content); - return $this->create_object_record($file, OBJECT_LOCATION_ERROR); + return $this->create_object_record($file, OBJECT_LOCATION_MISSING); } /** @@ -408,8 +407,7 @@ private function create_object_record(stored_file $file, $location) { protected function objects_contain_hash($contenthash) { $config = manager::get_objectfs_config(); $config->filesystem = get_class($this->filesystem); - $candidatesfinder = new candidates_finder($this->manipulator, $config); - $candidateobjects = $candidatesfinder->get(); + $candidateobjects = $this->manipulator::get_candidates($config); foreach ($candidateobjects as $candidateobject) { if ($contenthash === $candidateobject->contenthash) { return true; diff --git a/db/install.xml b/db/install.xml index 0065b1d7..267ab4ac 100644 --- a/db/install.xml +++ b/db/install.xml @@ -9,7 +9,9 @@ - + + + @@ -19,7 +21,7 @@ - + diff --git a/db/upgrade.php b/db/upgrade.php index bf06b09c..95415994 100644 --- a/db/upgrade.php +++ b/db/upgrade.php @@ -220,5 +220,67 @@ function xmldb_tool_objectfs_upgrade($oldversion) { upgrade_plugin_savepoint(true, 2024120600, 'tool', 'objectfs'); } + if ($oldversion < 2026041007) { + // Migrate from single 'location' integer column to individual boolean columns. + // Old values: ORPHANED=-2, ERROR(missing)=-1, LOCAL=0, DUPLICATED=1, EXTERNAL=2 + // New columns: in_filedir, in_mdl_files, in_remote (each 0 or 1). + + $table = new xmldb_table('tool_objectfs_objects'); + + // Add the new boolean columns. + $field = new xmldb_field('in_filedir', XMLDB_TYPE_INTEGER, '1', null, XMLDB_NOTNULL, null, '0', 'timeduplicated'); + if (!$dbman->field_exists($table, $field)) { + $dbman->add_field($table, $field); + } + $field = new xmldb_field('in_mdl_files', XMLDB_TYPE_INTEGER, '1', null, XMLDB_NOTNULL, null, '0', 'in_filedir'); + if (!$dbman->field_exists($table, $field)) { + $dbman->add_field($table, $field); + } + $field = new xmldb_field('in_remote', XMLDB_TYPE_INTEGER, '1', null, XMLDB_NOTNULL, null, '0', 'in_mdl_files'); + if (!$dbman->field_exists($table, $field)) { + $dbman->add_field($table, $field); + } + + // Populate new columns from old location values. + // ORPHANED (-2): in_filedir=1, in_mdl_files=0, in_remote=0. + $DB->execute('UPDATE {tool_objectfs_objects} SET in_filedir = 1 WHERE location = -2'); + // ERROR/MISSING (-1): in_filedir=0, in_mdl_files=1, in_remote=0. + $DB->execute('UPDATE {tool_objectfs_objects} SET in_mdl_files = 1 WHERE location = -1'); + // LOCAL (0): in_filedir=1, in_mdl_files=1, in_remote=0. + $DB->execute('UPDATE {tool_objectfs_objects} SET in_filedir = 1, in_mdl_files = 1 WHERE location = 0'); + // DUPLICATED (1): in_filedir=1, in_mdl_files=1, in_remote=1. + $DB->execute('UPDATE {tool_objectfs_objects} SET in_filedir = 1, in_mdl_files = 1, in_remote = 1 WHERE location = 1'); + // EXTERNAL (2): in_filedir=0, in_mdl_files=1, in_remote=1. + $DB->execute('UPDATE {tool_objectfs_objects} SET in_mdl_files = 1, in_remote = 1 WHERE location = 2'); + + // Drop the old index that references location. + $index = new xmldb_index('toolobjeobje_con_idu_ix', XMLDB_INDEX_UNIQUE, ['contenthash', 'location']); + if ($dbman->index_exists($table, $index)) { + $dbman->drop_index($table, $index); + } + + // Drop the old location column. + $field = new xmldb_field('location'); + if ($dbman->field_exists($table, $field)) { + $dbman->drop_field($table, $field); + } + + // Add composite index on the new boolean columns. + $index = new xmldb_index('ix_location_bits', XMLDB_INDEX_NOTUNIQUE, ['in_filedir', 'in_mdl_files', 'in_remote']); + if (!$dbman->index_exists($table, $index)) { + $dbman->add_index($table, $index); + } + + // Migrate report_data datakey values to the new bitmask values. + // Reports store the composite location value as a string key. + $DB->execute("UPDATE {tool_objectfs_report_data} SET datakey = '6' WHERE reporttype = 'location' AND datakey = '2'"); + $DB->execute("UPDATE {tool_objectfs_report_data} SET datakey = '7' WHERE reporttype = 'location' AND datakey = '1'"); + $DB->execute("UPDATE {tool_objectfs_report_data} SET datakey = '3' WHERE reporttype = 'location' AND datakey = '0'"); + $DB->execute("UPDATE {tool_objectfs_report_data} SET datakey = '2' WHERE reporttype = 'location' AND datakey = '-1'"); + $DB->execute("UPDATE {tool_objectfs_report_data} SET datakey = '1' WHERE reporttype = 'location' AND datakey = '-2'"); + + upgrade_plugin_savepoint(true, 2026041007, 'tool', 'objectfs'); + } + return true; } diff --git a/lang/en/tool_objectfs.php b/lang/en/tool_objectfs.php index b1884d74..f3849645 100644 --- a/lang/en/tool_objectfs.php +++ b/lang/en/tool_objectfs.php @@ -83,9 +83,10 @@ $string['object_status:location:localcount'] = 'Local (count)'; $string['object_status:location:localsize'] = 'Local (size)'; $string['object_status:location:localsizechart'] = 'Local (MB)'; -$string['object_status:location:missingcount'] = 'Error (count)'; -$string['object_status:location:missingsize'] = 'Error (size)'; -$string['object_status:location:missingsizechart'] = 'Error (MB)'; +$string['object_status:location:missing'] = 'Missing from filedir and external storage (view files)'; +$string['object_status:location:missingcount'] = 'Missing (count)'; +$string['object_status:location:missingsize'] = 'Missing (size)'; +$string['object_status:location:missingsizechart'] = 'Missing (MB)'; $string['object_status:location:orphaned'] = 'Marked as orphaned (not in the {files} table)'; $string['object_status:location:orphanedcount'] = 'Orphaned (count)'; $string['object_status:location:orphanedsize'] = 'Orphaned (size)'; diff --git a/lib.php b/lib.php index 870e6f32..24dc7a69 100644 --- a/lib.php +++ b/lib.php @@ -29,36 +29,62 @@ define('OBJECTFS_PLUGIN_NAME', 'tool_objectfs'); /** - * Location enum of the object - * ORPHANED is when the {objectfs_objects} table contains a record linking to a - * moodle {files} record which is no longer present. + * Location bit flag: the object exists in the local filedir. + * Bit 1. */ -define('OBJECT_LOCATION_ORPHANED', -2); +define('OBJECT_LOCATION_IN_FILEDIR', 1); /** - * Location enum of the object - * ERROR is when the file is missing when it is expected to be there. + * Location bit flag: the object is referenced in the Moodle files table (mdl_files). + * Bit 2. + */ +define('OBJECT_LOCATION_IN_MDL_FILES', 2); + +/** + * Location bit flag: the object exists in the primary remote object store. + * Bit 4. + */ +define('OBJECT_LOCATION_IN_REMOTE', 4); + +/** + * Location of the object: completely absent — no bits set. + * This is an invalid state that should not normally be stored in the database. + * Value: 0. + */ +define('OBJECT_LOCATION_ERROR', 0); + +/** + * Location of the object: in filedir only, no mdl_files reference. + * This is a trashdir candidate marked for cleanup. + * Value: OBJECT_LOCATION_IN_FILEDIR = 1. + */ +define('OBJECT_LOCATION_ORPHANED', OBJECT_LOCATION_IN_FILEDIR); + +/** + * Location of the object: referenced in mdl_files but not present in filedir or any remote store. + * This is a missing-file error state. + * Value: OBJECT_LOCATION_IN_MDL_FILES = 2. * @see tests/object_file_system_test.php for examples. */ -define('OBJECT_LOCATION_ERROR', -1); +define('OBJECT_LOCATION_MISSING', OBJECT_LOCATION_IN_MDL_FILES); /** - * Location enum of the object - * LOCAL is when the object exists locally only. + * Location of the object: in filedir and mdl_files, not yet pushed to any remote store. + * Value: OBJECT_LOCATION_IN_FILEDIR | OBJECT_LOCATION_IN_MDL_FILES = 3. */ -define('OBJECT_LOCATION_LOCAL', 0); +define('OBJECT_LOCATION_LOCAL', OBJECT_LOCATION_IN_FILEDIR | OBJECT_LOCATION_IN_MDL_FILES); /** - * Location enum of the object - * DUPLICATED is when the object exists both locally, and remotely. + * Location of the object: in mdl_files and the primary remote store, no local filedir copy. + * Value: OBJECT_LOCATION_IN_MDL_FILES | OBJECT_LOCATION_IN_REMOTE = 6. */ -define('OBJECT_LOCATION_DUPLICATED', 1); +define('OBJECT_LOCATION_EXTERNAL', OBJECT_LOCATION_IN_MDL_FILES | OBJECT_LOCATION_IN_REMOTE); /** - * Location enum of the object - * EXTERNAL is when when the object lives remotely only. + * Location of the object: in filedir, mdl_files, and the primary remote store. + * Value: OBJECT_LOCATION_IN_FILEDIR | OBJECT_LOCATION_IN_MDL_FILES | OBJECT_LOCATION_IN_REMOTE = 7. */ -define('OBJECT_LOCATION_EXTERNAL', 2); +define('OBJECT_LOCATION_DUPLICATED', OBJECT_LOCATION_IN_FILEDIR | OBJECT_LOCATION_IN_MDL_FILES | OBJECT_LOCATION_IN_REMOTE); define('OBJECTFS_REPORT_OBJECT_LOCATION', 0); define('OBJECTFS_REPORT_LOG_SIZE', 1); diff --git a/missing_files.php b/missing_files.php index 1c22ff90..0436add3 100644 --- a/missing_files.php +++ b/missing_files.php @@ -37,7 +37,7 @@ $PAGE->set_context(context_system::instance()); $PAGE->set_cacheable(false); $output = $PAGE->get_renderer('tool_objectfs'); -$table = new files_table('missing-files', OBJECT_LOCATION_ERROR); +$table = new files_table('missing-files', OBJECT_LOCATION_MISSING); $table->define_baseurl('/admin/tool/objectfs/missing_files.php'); if ($table->is_downloading($download, get_string('filename:missingfiles', 'tool_objectfs'))) { diff --git a/tests/local/object_manipulator/checker_test.php b/tests/local/object_manipulator/checker_test.php index 3ec59e18..d8fc3f69 100644 --- a/tests/local/object_manipulator/checker_test.php +++ b/tests/local/object_manipulator/checker_test.php @@ -50,24 +50,24 @@ protected function tearDown(): void { public function test_checker_get_location_local_if_object_is_local(): void { global $DB; $file = $this->create_local_object(); - $location = $DB->get_field('tool_objectfs_objects', 'location', ['contenthash' => $file->contenthash]); - $this->assertEquals('string', gettype($location)); + $location = manager::get_location_by_hash($file->contenthash); + $this->assertIsInt($location); $this->assertEquals(OBJECT_LOCATION_LOCAL, $location); } public function test_checker_get_location_duplicated_if_object_is_duplicated(): void { global $DB; $file = $this->create_duplicated_object(); - $location = $DB->get_field('tool_objectfs_objects', 'location', ['contenthash' => $file->contenthash]); - $this->assertEquals('string', gettype($location)); + $location = manager::get_location_by_hash($file->contenthash); + $this->assertIsInt($location); $this->assertEquals(OBJECT_LOCATION_DUPLICATED, $location); } public function test_checker_get_location_external_if_object_is_external(): void { global $DB; $file = $this->create_remote_object(); - $location = $DB->get_field('tool_objectfs_objects', 'location', ['contenthash' => $file->contenthash]); - $this->assertEquals('string', gettype($location)); + $location = manager::get_location_by_hash($file->contenthash); + $this->assertIsInt($location); $this->assertEquals(OBJECT_LOCATION_EXTERNAL, $location); } @@ -95,9 +95,9 @@ public function test_checker_can_update_object(): void { $localobject->id = null; $DB->delete_records('tool_objectfs_objects', ['contenthash' => $localobject->contenthash]); $this->checker->execute([$localobject]); - $dblocation = $DB->get_field('tool_objectfs_objects', 'location', ['contenthash' => $localobject->contenthash]); + $dblocation = manager::get_location_by_hash($localobject->contenthash); - $this->assertEquals('string', gettype($dblocation)); + $this->assertIsInt($dblocation); $this->assertEquals(OBJECT_LOCATION_LOCAL, $dblocation); self::assertFalse($this->objects_contain_hash($localobject->contenthash)); } @@ -127,6 +127,6 @@ public function test_checker_manipulate_object_method_will_get_error_location_on $file = $this->create_error_object(); $reflection = new \ReflectionMethod(checker::class, "manipulate_object"); $reflection->setAccessible(true); - $this->assertEquals(OBJECT_LOCATION_ERROR, $reflection->invokeArgs($this->checker, [$file])); + $this->assertEquals(OBJECT_LOCATION_MISSING, $reflection->invokeArgs($this->checker, [$file])); } } diff --git a/tests/local/object_manipulator/deleter_test.php b/tests/local/object_manipulator/deleter_test.php index 09fd974f..b95d7088 100644 --- a/tests/local/object_manipulator/deleter_test.php +++ b/tests/local/object_manipulator/deleter_test.php @@ -91,7 +91,7 @@ public function test_deleter_can_delete_object(): void { $this->deleter->execute([$object]); - $location = $DB->get_field('tool_objectfs_objects', 'location', ['contenthash' => $object->contenthash]); + $location = manager::get_location_by_hash($object->contenthash); $this->assertEquals(OBJECT_LOCATION_EXTERNAL, $location); $this->assertFalse($this->is_locally_readable_by_hash($object->contenthash)); $this->assertTrue($this->is_externally_readable_by_hash($object->contenthash)); @@ -103,7 +103,7 @@ public function test_deleter_can_handle_local_object(): void { $this->deleter->execute([$object]); - $location = $DB->get_field('tool_objectfs_objects', 'location', ['contenthash' => $object->contenthash]); + $location = manager::get_location_by_hash($object->contenthash); $this->assertEquals(OBJECT_LOCATION_LOCAL, $location); $this->assertTrue($this->is_locally_readable_by_hash($object->contenthash)); $this->assertFalse($this->is_externally_readable_by_hash($object->contenthash)); @@ -115,7 +115,7 @@ public function test_deleter_can_handle_remote_object(): void { $this->deleter->execute([$object]); - $location = $DB->get_field('tool_objectfs_objects', 'location', ['contenthash' => $object->contenthash]); + $location = manager::get_location_by_hash($object->contenthash); $this->assertEquals(OBJECT_LOCATION_EXTERNAL, $location); $this->assertFalse($this->is_locally_readable_by_hash($object->contenthash)); $this->assertTrue($this->is_externally_readable_by_hash($object->contenthash)); @@ -128,7 +128,7 @@ public function test_deleter_will_delete_no_objects_if_deletelocal_disabled(): v $this->deleter->execute([$object]); - $location = $DB->get_field('tool_objectfs_objects', 'location', ['contenthash' => $object->contenthash]); + $location = manager::get_location_by_hash($object->contenthash); $this->assertEquals(OBJECT_LOCATION_DUPLICATED, $location); $this->assertTrue($this->is_locally_readable_by_hash($object->contenthash)); $this->assertTrue($this->is_externally_readable_by_hash($object->contenthash)); @@ -144,7 +144,7 @@ public function test_deleter_can_delete_multiple_objects(): void { $this->deleter->execute($objects); foreach ($objects as $object) { - $location = $DB->get_field('tool_objectfs_objects', 'location', ['contenthash' => $object->contenthash]); + $location = manager::get_location_by_hash($object->contenthash); $this->assertEquals(OBJECT_LOCATION_EXTERNAL, $location); $this->assertFalse($this->is_locally_readable_by_hash($object->contenthash)); $this->assertTrue($this->is_externally_readable_by_hash($object->contenthash)); diff --git a/tests/local/object_manipulator/orphaner_test.php b/tests/local/object_manipulator/orphaner_test.php index 43e1a17e..de43625c 100644 --- a/tests/local/object_manipulator/orphaner_test.php +++ b/tests/local/object_manipulator/orphaner_test.php @@ -17,7 +17,6 @@ namespace tool_objectfs\local\object_manipulator; use tool_objectfs\local\manager; -use tool_objectfs\local\object_manipulator\candidates\candidates_finder; /** * Tests for object orphaner. @@ -71,9 +70,9 @@ public function test_orphaner_can_orphan_files(): void { $object2 = $this->create_duplicated_object(), $object3 = $this->create_remote_object(), ]); - $location1 = $DB->get_field('tool_objectfs_objects', 'location', ['contenthash' => $object1->contenthash]); - $location2 = $DB->get_field('tool_objectfs_objects', 'location', ['contenthash' => $object2->contenthash]); - $location3 = $DB->get_field('tool_objectfs_objects', 'location', ['contenthash' => $object3->contenthash]); + $location1 = manager::get_location_by_hash($object1->contenthash); + $location2 = manager::get_location_by_hash($object2->contenthash); + $location3 = manager::get_location_by_hash($object3->contenthash); $this->assertEquals(OBJECT_LOCATION_ORPHANED, $location1); $this->assertEquals(OBJECT_LOCATION_ORPHANED, $location2); $this->assertEquals(OBJECT_LOCATION_ORPHANED, $location3); @@ -85,15 +84,14 @@ public function test_orphaner_finds_correct_candidates(): void { // Initialise the candidate finder. $config = manager::get_objectfs_config(); $config->filesystem = get_class($this->filesystem); - $finder = new candidates_finder($this->manipulator, $config); - $objects = $finder->get(); + $objects = $this->manipulator::get_candidates($config); $this->assertCount(0, $objects); // No candidates. // Create an object. $object = $this->create_local_object(); // Still no candidates - object created but nothing is missing from {files} table. - $objects = $finder->get(); + $objects = $this->manipulator::get_candidates($config); $this->assertCount(0, $objects); // Update that object to have a different hash, to mock a non-existent @@ -101,12 +99,12 @@ public function test_orphaner_finds_correct_candidates(): void { $DB->set_field('files', 'contenthash', 'different', ['contenthash' => $object->contenthash]); // Expect one candidate - no matching contenthash in {files}. - $objects = $finder->get(); + $objects = $this->manipulator::get_candidates($config); $this->assertCount(1, $objects); // Ensure it ignores orphaned records during the find. - $DB->set_field('tool_objectfs_objects', 'location', OBJECT_LOCATION_ORPHANED, ['contenthash' => $object->contenthash]); - $objects = $finder->get(); + manager::update_object_by_hash($object->contenthash, OBJECT_LOCATION_ORPHANED); + $objects = $this->manipulator::get_candidates($config); $this->assertCount(0, $objects); // No candidates - only candidate has been orphaned. } @@ -117,9 +115,9 @@ public function test_orphaner_correctly_orphans_provided_files(): void { $object2 = $this->create_duplicated_object(), $object3 = $this->create_remote_object(), ]); - $location1 = $DB->get_field('tool_objectfs_objects', 'location', ['contenthash' => $object1->contenthash]); - $location2 = $DB->get_field('tool_objectfs_objects', 'location', ['contenthash' => $object2->contenthash]); - $location3 = $DB->get_field('tool_objectfs_objects', 'location', ['contenthash' => $object3->contenthash]); + $location1 = manager::get_location_by_hash($object1->contenthash); + $location2 = manager::get_location_by_hash($object2->contenthash); + $location3 = manager::get_location_by_hash($object3->contenthash); $this->assertEquals(OBJECT_LOCATION_ORPHANED, $location1); $this->assertEquals(OBJECT_LOCATION_ORPHANED, $location2); $this->assertEquals(OBJECT_LOCATION_ORPHANED, $location3); diff --git a/tests/local/object_manipulator/puller_test.php b/tests/local/object_manipulator/puller_test.php index 1a061563..f055262c 100644 --- a/tests/local/object_manipulator/puller_test.php +++ b/tests/local/object_manipulator/puller_test.php @@ -91,7 +91,7 @@ public function test_puller_can_pull_remote_file(): void { $this->puller->execute([$object]); - $location = $DB->get_field('tool_objectfs_objects', 'location', ['contenthash' => $object->contenthash]); + $location = manager::get_location_by_hash($object->contenthash); $this->assertEquals(OBJECT_LOCATION_DUPLICATED, $location); $this->assertTrue($this->is_locally_readable_by_hash($object->contenthash)); $this->assertTrue($this->is_externally_readable_by_hash($object->contenthash)); @@ -103,7 +103,7 @@ public function test_puller_can_handle_duplicated_file(): void { $this->puller->execute([$object]); - $location = $DB->get_field('tool_objectfs_objects', 'location', ['contenthash' => $object->contenthash]); + $location = manager::get_location_by_hash($object->contenthash); $this->assertEquals(OBJECT_LOCATION_DUPLICATED, $location); $this->assertTrue($this->is_locally_readable_by_hash($object->contenthash)); $this->assertTrue($this->is_externally_readable_by_hash($object->contenthash)); @@ -115,7 +115,7 @@ public function test_puller_can_handle_local_file(): void { $this->puller->execute([$object]); - $location = $DB->get_field('tool_objectfs_objects', 'location', ['contenthash' => $object->contenthash]); + $location = manager::get_location_by_hash($object->contenthash); $this->assertEquals(OBJECT_LOCATION_LOCAL, $location); $this->assertTrue($this->is_locally_readable_by_hash($object->contenthash)); $this->assertFalse($this->is_externally_readable_by_hash($object->contenthash)); diff --git a/tests/local/object_manipulator/pusher_test.php b/tests/local/object_manipulator/pusher_test.php index 1cf28e55..d23d164b 100644 --- a/tests/local/object_manipulator/pusher_test.php +++ b/tests/local/object_manipulator/pusher_test.php @@ -17,7 +17,6 @@ namespace tool_objectfs\local\object_manipulator; use tool_objectfs\local\manager; -use tool_objectfs\local\object_manipulator\candidates\candidates_finder; /** * Tests for object pusher. @@ -109,7 +108,7 @@ public function test_pusher_can_push_local_file(): void { $this->pusher->execute([$object]); - $location = $DB->get_field('tool_objectfs_objects', 'location', ['contenthash' => $object->contenthash]); + $location = manager::get_location_by_hash($object->contenthash); $this->assertEquals(OBJECT_LOCATION_DUPLICATED, $location); $this->assertTrue($this->is_locally_readable_by_hash($object->contenthash)); $this->assertTrue($this->is_externally_readable_by_hash($object->contenthash)); @@ -121,7 +120,7 @@ public function test_pusher_can_handle_duplicated_file(): void { $this->pusher->execute([$object]); - $location = $DB->get_field('tool_objectfs_objects', 'location', ['contenthash' => $object->contenthash]); + $location = manager::get_location_by_hash($object->contenthash); $this->assertEquals(OBJECT_LOCATION_DUPLICATED, $location); $this->assertTrue($this->is_locally_readable_by_hash($object->contenthash)); $this->assertTrue($this->is_externally_readable_by_hash($object->contenthash)); @@ -133,7 +132,7 @@ public function test_pusher_can_handle_remote_file(): void { $this->pusher->execute([$object]); - $location = $DB->get_field('tool_objectfs_objects', 'location', ['contenthash' => $object->contenthash]); + $location = manager::get_location_by_hash($object->contenthash); $this->assertEquals(OBJECT_LOCATION_EXTERNAL, $location); $this->assertFalse($this->is_locally_readable_by_hash($object->contenthash)); $this->assertTrue($this->is_externally_readable_by_hash($object->contenthash)); @@ -149,7 +148,7 @@ public function test_pusher_can_push_multiple_objects(): void { $this->pusher->execute($objects); foreach ($objects as $object) { - $location = $DB->get_field('tool_objectfs_objects', 'location', ['contenthash' => $object->contenthash]); + $location = manager::get_location_by_hash($object->contenthash); $this->assertEquals(OBJECT_LOCATION_DUPLICATED, $location); $this->assertTrue($this->is_locally_readable_by_hash($object->contenthash)); $this->assertTrue($this->is_externally_readable_by_hash($object->contenthash)); @@ -161,8 +160,7 @@ public function test_get_candidate_objects_get_one_object_if_files_have_same_has // Push initial objects so they arnt candidates. $config = manager::get_objectfs_config(); $config->filesystem = get_class($this->filesystem); - $finder = new candidates_finder($this->manipulator, $config); - $objects = $finder->get(); + $objects = $this->manipulator::get_candidates($config); $this->pusher->execute($objects); $object = $this->create_local_object(); @@ -173,7 +171,7 @@ public function test_get_candidate_objects_get_one_object_if_files_have_same_has $file->pathnamehash = '1234'; $DB->insert_record('files', $file); - $objects = $finder->get(); + $objects = $this->manipulator::get_candidates($config); $this->assertEquals(1, count($objects)); } diff --git a/tests/local/object_manipulator/recoverer_test.php b/tests/local/object_manipulator/recoverer_test.php index 03abfce0..fb578ddf 100644 --- a/tests/local/object_manipulator/recoverer_test.php +++ b/tests/local/object_manipulator/recoverer_test.php @@ -17,7 +17,6 @@ namespace tool_objectfs\local\object_manipulator; use tool_objectfs\local\manager; -use tool_objectfs\local\object_manipulator\candidates\candidates_finder; /** * Tests for object recoverer. @@ -28,16 +27,12 @@ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ final class recoverer_test extends \tool_objectfs\tests\testcase { - /** @var candidates_finder Candidates finder object */ - protected $candidatesfinder; - /** @var recoverer Recoverer object */ protected $recoverer; protected function setUp(): void { parent::setUp(); $config = manager::get_objectfs_config(); - $this->candidatesfinder = new candidates_finder(recoverer::class, $config); manager::set_objectfs_config($config); $this->logger = new \tool_objectfs\log\aggregate_logger(); $this->recoverer = new recoverer($this->filesystem, $config, $this->logger); @@ -51,7 +46,7 @@ protected function tearDown(): void { public function test_recoverer_get_candidate_objects_will_get_error_objects(): void { $recovererobject = $this->create_error_object(); - $candidateobjects = $this->candidatesfinder->get(); + $candidateobjects = recoverer::get_candidates(manager::get_objectfs_config()); foreach ($candidateobjects as $candidate) { $this->assertEquals($recovererobject->contenthash, $candidate->contenthash); @@ -61,44 +56,44 @@ public function test_recoverer_get_candidate_objects_will_get_error_objects(): v public function test_recoverer_will_recover_local_objects(): void { global $DB; $object = $this->create_local_object(); - $DB->set_field('tool_objectfs_objects', 'location', OBJECT_LOCATION_ERROR, ['contenthash' => $object->contenthash]); + manager::update_object_by_hash($object->contenthash, OBJECT_LOCATION_MISSING); $this->recoverer->execute([$object]); - $location = $DB->get_field('tool_objectfs_objects', 'location', ['contenthash' => $object->contenthash]); + $location = manager::get_location_by_hash($object->contenthash); $this->assertEquals(OBJECT_LOCATION_LOCAL, $location); } public function test_recoverer_will_recover_duplicated_objects(): void { global $DB; $object = $this->create_duplicated_object(); - $DB->set_field('tool_objectfs_objects', 'location', OBJECT_LOCATION_ERROR, ['contenthash' => $object->contenthash]); + manager::update_object_by_hash($object->contenthash, OBJECT_LOCATION_MISSING); $this->recoverer->execute([$object]); - $location = $DB->get_field('tool_objectfs_objects', 'location', ['contenthash' => $object->contenthash]); + $location = manager::get_location_by_hash($object->contenthash); $this->assertEquals(OBJECT_LOCATION_DUPLICATED, $location); } public function test_recoverer_will_recover_remote_objects(): void { global $DB; $object = $this->create_remote_object(); - $DB->set_field('tool_objectfs_objects', 'location', OBJECT_LOCATION_ERROR, ['contenthash' => $object->contenthash]); + manager::update_object_by_hash($object->contenthash, OBJECT_LOCATION_MISSING); $this->recoverer->execute([$object]); - $location = $DB->get_field('tool_objectfs_objects', 'location', ['contenthash' => $object->contenthash]); + $location = manager::get_location_by_hash($object->contenthash); $this->assertEquals(OBJECT_LOCATION_EXTERNAL, $location); } public function test_recoverer_will_not_recover_error_objects(): void { global $DB; $object = $this->create_error_object(); - $DB->set_field('tool_objectfs_objects', 'location', OBJECT_LOCATION_ERROR, ['contenthash' => $object->contenthash]); + manager::update_object_by_hash($object->contenthash, OBJECT_LOCATION_MISSING); $this->recoverer->execute([$object]); - $location = $DB->get_field('tool_objectfs_objects', 'location', ['contenthash' => $object->contenthash]); - $this->assertEquals(OBJECT_LOCATION_ERROR, $location); + $location = manager::get_location_by_hash($object->contenthash); + $this->assertEquals(OBJECT_LOCATION_MISSING, $location); } } diff --git a/tests/local/tagging_test.php b/tests/local/tagging_test.php index a3629028..fdc62a46 100644 --- a/tests/local/tagging_test.php +++ b/tests/local/tagging_test.php @@ -171,7 +171,9 @@ public function test_gather_object_tags_for_upload_orphaned(): void { $object = $this->create_duplicated_object('gather tags for upload test'); // Change the object record to be orphaned. - $DB->update_record('tool_objectfs_objects', ['id' => $object->id, 'location' => OBJECT_LOCATION_ORPHANED]); + $columns = \tool_objectfs\local\location_helper::bits_to_columns(OBJECT_LOCATION_ORPHANED); + $columns['id'] = $object->id; + $DB->update_record('tool_objectfs_objects', $columns); $tags = tag_manager::gather_object_tags_for_upload($object->contenthash); diff --git a/tests/object_file_system_test.php b/tests/object_file_system_test.php index 48d49db8..3cde3f43 100644 --- a/tests/object_file_system_test.php +++ b/tests/object_file_system_test.php @@ -162,7 +162,7 @@ public function test_copy_object_from_external_to_local_by_hash_if_not_local_and $location = $this->filesystem->copy_object_from_external_to_local_by_hash($fakehash); - $this->assertEquals(OBJECT_LOCATION_ERROR, $location); + $this->assertEquals(OBJECT_LOCATION_MISSING, $location); } public function test_copy_object_from_local_to_external_by_hash(): void { @@ -199,7 +199,7 @@ public function test_copy_object_from_local_to_external_by_hash_if_not_local_and $location = $this->filesystem->copy_object_from_local_to_external_by_hash($fakehash); - $this->assertEquals(OBJECT_LOCATION_ERROR, $location); + $this->assertEquals(OBJECT_LOCATION_MISSING, $location); } public function test_delete_object_from_local_by_hash(): void { @@ -229,7 +229,7 @@ public function test_delete_object_from_local_by_hash_if_not_local(): void { $location = $this->filesystem->delete_object_from_local_by_hash($fakehash); - $this->assertEquals(OBJECT_LOCATION_ERROR, $location); + $this->assertEquals(OBJECT_LOCATION_MISSING, $location); } public function test_delete_object_from_local_by_hash_if_can_verify_external_object(): void { @@ -321,8 +321,8 @@ public function test_readfile_updates_object_with_error_location_on_fail(): void $this->filesystem->readfile($fakefile); restore_error_handler(); - $location = $DB->get_field('tool_objectfs_objects', 'location', ['contenthash' => $fakefile->get_contenthash()]); - $this->assertEquals(OBJECT_LOCATION_ERROR, $location); + $location = manager::get_location_by_hash($fakefile->get_contenthash()); + $this->assertEquals(OBJECT_LOCATION_MISSING, $location); } public function test_get_content_if_object_is_local(): void { @@ -353,8 +353,8 @@ public function test_get_content_updates_object_with_error_location_on_fail(): v $this->filesystem->get_content($fakefile); restore_error_handler(); - $location = $DB->get_field('tool_objectfs_objects', 'location', ['contenthash' => $fakefile->get_contenthash()]); - $this->assertEquals(OBJECT_LOCATION_ERROR, $location); + $location = manager::get_location_by_hash($fakefile->get_contenthash()); + $this->assertEquals(OBJECT_LOCATION_MISSING, $location); } /** @@ -426,8 +426,8 @@ public function test_get_content_file_handle_does_not_set_error_location_on_tran $this->assertStringContainsString('Failed to open', $e->getMessage()); } - $location = $DB->get_field('tool_objectfs_objects', 'location', ['contenthash' => $file->get_contenthash()]); - $this->assertNotEquals(OBJECT_LOCATION_ERROR, $location); + $location = manager::get_location_by_hash($file->get_contenthash()); + $this->assertNotEquals(OBJECT_LOCATION_MISSING, $location); } public function test_remove_file_will_remove_local_file(): void { diff --git a/tests/task/populate_objects_filesize_test.php b/tests/task/populate_objects_filesize_test.php index b7083dfd..1f86d21e 100644 --- a/tests/task/populate_objects_filesize_test.php +++ b/tests/task/populate_objects_filesize_test.php @@ -16,6 +16,8 @@ namespace tool_objectfs\task; +use tool_objectfs\local\manager; + /** * Test adhoc-task populate_objects_filesize. * @@ -197,8 +199,10 @@ public function test_orphaned_objects_are_not_updated(): void { // Set all objects to have a filesize of null. $DB->set_field('tool_objectfs_objects', 'filesize', null); - // Set first object to be orphaned. - $DB->set_field('tool_objectfs_objects', 'location', -2, ['contenthash' => $filehashes[0]]); + // Set first object to be orphaned (direct column write to preserve NULL filesize). + $columns = \tool_objectfs\local\location_helper::bits_to_columns(OBJECT_LOCATION_ORPHANED); + $columns['id'] = $DB->get_field('tool_objectfs_objects', 'id', ['contenthash' => $filehashes[0]]); + $DB->update_record('tool_objectfs_objects', $columns); // Call ad-hoc task to populate filesizes. $task = new \tool_objectfs\task\populate_objects_filesize(); @@ -232,8 +236,10 @@ public function test_objects_with_error_are_not_updated(): void { // Set all objects to have a filesize of null. $DB->set_field('tool_objectfs_objects', 'filesize', null); - // Set first object to be orphaned. - $DB->set_field('tool_objectfs_objects', 'location', -1, ['contenthash' => $file1->get_contenthash()]); + // Set first object to have error/missing state (direct column write to preserve NULL filesize). + $columns = \tool_objectfs\local\location_helper::bits_to_columns(OBJECT_LOCATION_MISSING); + $columns['id'] = $DB->get_field('tool_objectfs_objects', 'id', ['contenthash' => $file1->get_contenthash()]); + $DB->update_record('tool_objectfs_objects', $columns); // Call ad-hoc task to populate filesizes. $task = new \tool_objectfs\task\populate_objects_filesize(); diff --git a/tests/task/task_reconcile_filedir_test.php b/tests/task/task_reconcile_filedir_test.php index e06822b3..cb89f9dc 100644 --- a/tests/task/task_reconcile_filedir_test.php +++ b/tests/task/task_reconcile_filedir_test.php @@ -16,6 +16,7 @@ namespace tool_objectfs\task; +use tool_objectfs\local\manager; use tool_objectfs\tests\testcase; use tool_objectfs\task\reconcile_filedir; @@ -57,12 +58,7 @@ public function test_reconcile_filedir_behaviours(): void { // 4. File marked EXTERNAL but exists locally, should update to DUPLICATED. $remotefile = $this->create_local_file('remote content'); - $DB->set_field( - 'tool_objectfs_objects', - 'location', - OBJECT_LOCATION_EXTERNAL, - ['contenthash' => $remotefile->get_contenthash()] - ); + manager::update_object_by_hash($remotefile->get_contenthash(), OBJECT_LOCATION_EXTERNAL); // Execute scheduled task. ob_start(); @@ -99,15 +95,9 @@ public function test_reconcile_filedir_behaviours(): void { ); // 4. External file should now be marked as DUPLICATED. - $updatedobject = $DB->get_record( - 'tool_objectfs_objects', - ['contenthash' => $remotefile->get_contenthash()], - '*', - MUST_EXIST - ); $this->assertEquals( OBJECT_LOCATION_DUPLICATED, - $updatedobject->location, + manager::get_location_by_hash($remotefile->get_contenthash()), 'External file should now be marked as DUPLICATED.' ); diff --git a/version.php b/version.php index 3f1c91e1..4ad26fc3 100644 --- a/version.php +++ b/version.php @@ -25,8 +25,8 @@ defined('MOODLE_INTERNAL') || die(); -$plugin->version = 2026041006; // The current plugin version (Date: YYYYMMDDXX). -$plugin->release = 2026041006; // Same as version. +$plugin->version = 2026041007; // The current plugin version (Date: YYYYMMDDXX). +$plugin->release = 2026041007; // Same as version. $plugin->requires = 2024042200; // Requires 4.4. $plugin->component = "tool_objectfs"; $plugin->maturity = MATURITY_STABLE;