diff --git a/classes/local/object_manipulator/candidates/bitmask_candidates.php b/classes/local/object_manipulator/candidates/bitmask_candidates.php new file mode 100644 index 00000000..e46a985e --- /dev/null +++ b/classes/local/object_manipulator/candidates/bitmask_candidates.php @@ -0,0 +1,119 @@ +. + +/** + * Unified bitmask-based candidate class for object manipulation. + * + * Replaces separate pusher_candidates, puller_candidates, deleter_candidates, + * and recoverer_candidates with a single parameterized class. + * + * @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\object_manipulator\candidates; + +use stdClass; + +/** + * Universal candidate finder using bitmask filters on the location column. + * + * Accepts two bitmasks: + * - "has" mask: bits the file MUST have (location & has_mask = has_mask) + * - "not" mask: bits the file must NOT have (location & not_mask = 0) + * + * Optional filters for filesize and timeduplicated are applied when provided. + */ +class bitmask_candidates extends manipulator_candidates_base { + /** @var string Query name for logging. */ + protected $queryname; + + /** @var int Bits that must be set in location. */ + private $hasmask; + + /** @var int Bits that must NOT be set in location. */ + private $notmask; + + /** @var array Optional filter options. */ + private $options; + + /** + * Constructor. + * + * @param stdClass $config Plugin config (must include batchsize). + * @param int $hasmask Bits the location MUST have. + * @param int $notmask Bits the location must NOT have. + * @param string $queryname Name for logging. + * @param array $options Optional filters: + * 'threshold' => int — filesize > threshold (minimum file size) + * 'max_filesize' => int — filesize < max_filesize (maximum file size) + * 'size_ceiling' => int — filesize <= size_ceiling (upper file size bound) + * 'maxage' => int — timeduplicated <= maxage (timestamp threshold) + */ + public function __construct(stdClass $config, int $hasmask, int $notmask, string $queryname, array $options = []) { + parent::__construct($config); + $this->hasmask = $hasmask; + $this->notmask = $notmask; + $this->queryname = $queryname; + $this->options = $options; + } + + /** + * Get candidate objects matching the bitmask filters. + * + * @return array + */ + public function get() { + global $DB; + + $conditions = []; + $conditions[] = 'location & :has_mask = :has_mask2'; + $conditions[] = 'location & :not_mask = 0'; + + $params = [ + 'has_mask' => $this->hasmask, + 'has_mask2' => $this->hasmask, + 'not_mask' => $this->notmask, + ]; + + if (isset($this->options['threshold'])) { + $conditions[] = 'filesize > :threshold'; + $params['threshold'] = $this->options['threshold']; + } + if (isset($this->options['max_filesize'])) { + $conditions[] = 'filesize < :max_filesize'; + $params['max_filesize'] = $this->options['max_filesize']; + } + if (isset($this->options['size_ceiling'])) { + $conditions[] = 'filesize <= :size_ceiling'; + $params['size_ceiling'] = $this->options['size_ceiling']; + } + if (isset($this->options['maxage'])) { + $conditions[] = 'timeduplicated <= :maxage'; + $params['maxage'] = $this->options['maxage']; + } + + $where = implode("\n AND ", $conditions); + $sql = "SELECT contenthash, + filesize + FROM {tool_objectfs_objects} + WHERE {$where}"; + + return $DB->get_records_sql($sql, $params, 0, $this->config->batchsize); + } +} diff --git a/classes/local/object_manipulator/candidates/candidates_factory.php b/classes/local/object_manipulator/candidates/candidates_factory.php index f1479aaf..3500e561 100644 --- a/classes/local/object_manipulator/candidates/candidates_factory.php +++ b/classes/local/object_manipulator/candidates/candidates_factory.php @@ -35,31 +35,115 @@ /** * Candidates Factory + * + * Maps manipulator classes to candidate finders. Uses bitmask_candidates for + * manipulators whose candidates are determined by location bitmask filters. */ class candidates_factory { - /** @var array $manipulatormap */ - private static $manipulatormap = [ + /** + * Manipulators that use the legacy class-based mapping (non-bitmask queries). + * @var array + */ + private static $legacymap = [ 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 + * Get the bitmask map for manipulators. + * Must be a method rather than a static property because the OBJECT_LOCATION_* + * constants are defined at runtime via define(). + * + * @return array + */ + private static function get_bitmask_map(): array { + return [ + pusher::class => [ + 'has_mask' => OBJECT_LOCATION_IN_FILEDIR | OBJECT_LOCATION_IN_MDL_FILES, // Must be local and referenced. + 'not_mask' => OBJECT_LOCATION_IN_REMOTE, // Must not be in remote. + 'queryname' => 'get_push_candidates', + ], + puller::class => [ + 'has_mask' => OBJECT_LOCATION_IN_MDL_FILES | OBJECT_LOCATION_IN_REMOTE, // Must be referenced and in remote. + 'not_mask' => OBJECT_LOCATION_IN_FILEDIR, // Must not be local. + 'queryname' => 'get_pull_candidates', + ], + deleter::class => [ + 'has_mask' => OBJECT_LOCATION_IN_FILEDIR | OBJECT_LOCATION_IN_MDL_FILES | OBJECT_LOCATION_IN_REMOTE, + 'not_mask' => 0, // All bits set, nothing excluded. + 'queryname' => 'get_delete_candidates', + ], + recoverer::class => [ + 'has_mask' => OBJECT_LOCATION_IN_MDL_FILES, // Must be referenced. + 'not_mask' => OBJECT_LOCATION_IN_FILEDIR | OBJECT_LOCATION_IN_REMOTE, // Must not be local or remote. + 'queryname' => 'get_recover_candidates', + ], + ]; + } + + /** + * Create a candidate finder for the given manipulator. * - * @return mixed + * @param string $manipulator Manipulator class name. + * @param stdClass $config Plugin config. + * @return manipulator_candidates * @throws moodle_exception */ public static function finder($manipulator, stdClass $config) { - if (isset(self::$manipulatormap[$manipulator])) { - $classname = self::$manipulatormap[$manipulator]; + // Legacy candidates (checker, orphaner) use non-bitmask SQL. + if (isset(self::$legacymap[$manipulator])) { + $classname = self::$legacymap[$manipulator]; return new $classname($config); } + + // Bitmask-based candidates. + $bitmaskmap = self::get_bitmask_map(); + if (isset($bitmaskmap[$manipulator])) { + $entry = $bitmaskmap[$manipulator]; + $options = self::get_options_for_manipulator($manipulator, $config); + return new bitmask_candidates( + $config, + $entry['has_mask'], + $entry['not_mask'], + $entry['queryname'], + $options + ); + } + throw new moodle_exception('invalidclass', 'error', '', 'Invalid manipulator class'); } + + /** + * Build filter options for a bitmask manipulator based on config. + * + * @param string $manipulator Manipulator class name. + * @param stdClass $config Plugin config. + * @return array + */ + private static function get_options_for_manipulator(string $manipulator, stdClass $config): array { + switch ($manipulator) { + case pusher::class: + $filesystem = new $config->filesystem(); + return [ + 'threshold' => $config->sizethreshold, + 'max_filesize' => $filesystem->get_maximum_upload_filesize(), + 'maxage' => time() - $config->minimumage, + ]; + + case puller::class: + return [ + 'size_ceiling' => $config->sizethreshold, + ]; + + case deleter::class: + return [ + 'threshold' => $config->sizethreshold, + 'maxage' => time() - $config->consistencydelay, + ]; + + case recoverer::class: + default: + return []; + } + } } diff --git a/classes/local/object_manipulator/candidates/checker_candidates.php b/classes/local/object_manipulator/candidates/checker_candidates.php index e52af679..28e0a122 100644 --- a/classes/local/object_manipulator/candidates/checker_candidates.php +++ b/classes/local/object_manipulator/candidates/checker_candidates.php @@ -35,23 +35,18 @@ class checker_candidates extends manipulator_candidates_base { protected $queryname = 'get_check_candidates'; /** - * get_candiates_sql - * @return string + * Get files that exist in {files} but have no tracking row in {tool_objectfs_objects}. + * + * @return array */ - public function get_candidates_sql() { - return 'SELECT f.contenthash + public function get() { + global $DB; + $sql = '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 []; + return $DB->get_records_sql($sql, [], 0, $this->config->batchsize); } } diff --git a/classes/local/object_manipulator/candidates/manipulator_candidates.php b/classes/local/object_manipulator/candidates/manipulator_candidates.php index 234ee2cb..679e7e30 100644 --- a/classes/local/object_manipulator/candidates/manipulator_candidates.php +++ b/classes/local/object_manipulator/candidates/manipulator_candidates.php @@ -34,21 +34,8 @@ interface manipulator_candidates { public function get_query_name(); /** - * Returns SQL to retrieve objects for manipulation. + * Get candidate 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 */ diff --git a/classes/local/object_manipulator/candidates/manipulator_candidates_base.php b/classes/local/object_manipulator/candidates/manipulator_candidates_base.php index a542217c..87a31819 100644 --- a/classes/local/object_manipulator/candidates/manipulator_candidates_base.php +++ b/classes/local/object_manipulator/candidates/manipulator_candidates_base.php @@ -49,19 +49,4 @@ public function __construct(stdClass $config) { 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 index e2cd1db4..5622be30 100644 --- a/classes/local/object_manipulator/candidates/orphaner_candidates.php +++ b/classes/local/object_manipulator/candidates/orphaner_candidates.php @@ -35,24 +35,18 @@ class orphaner_candidates extends manipulator_candidates_base { protected $queryname = 'get_orphan_candidates'; /** - * get_candidates_sql - * @return string + * Get tracked objects that no longer have a reference in {files}. + * + * @return array */ - public function get_candidates_sql() { - return 'SELECT o.id, o.contenthash, o.location + public function get() { + global $DB; + $sql = '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, - ]; + $params = ['location' => OBJECT_LOCATION_ORPHANED]; + return $DB->get_records_sql($sql, $params, 0, $this->config->batchsize); } } diff --git a/classes/local/object_manipulator/candidates/recoverer_candidates.php b/classes/local/object_manipulator/candidates/recoverer_candidates.php index 9e29c868..525e71f5 100644 --- a/classes/local/object_manipulator/candidates/recoverer_candidates.php +++ b/classes/local/object_manipulator/candidates/recoverer_candidates.php @@ -50,6 +50,6 @@ public function get_candidates_sql() { * @return array */ public function get_candidates_sql_params() { - return ['location' => OBJECT_LOCATION_ERROR]; + return ['location' => OBJECT_LOCATION_MISSING]; } } diff --git a/classes/local/object_manipulator/recoverer.php b/classes/local/object_manipulator/recoverer.php index cceb6a59..93c2d2fe 100644 --- a/classes/local/object_manipulator/recoverer.php +++ b/classes/local/object_manipulator/recoverer.php @@ -37,6 +37,8 @@ class recoverer extends manipulator { * @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..e33dd892 100644 --- a/classes/local/report/location_report_builder.php +++ b/classes/local/report/location_report_builder.php @@ -46,7 +46,7 @@ public function build_report($reportid) { OBJECT_LOCATION_DUPLICATED, OBJECT_LOCATION_EXTERNAL, OBJECT_LOCATION_ORPHANED, - OBJECT_LOCATION_ERROR, + OBJECT_LOCATION_MISSING, ]; $totalcount = 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..feb959e8 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; 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/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/tests/testcase.php b/classes/tests/testcase.php index 9384e120..df6f92da 100644 --- a/classes/tests/testcase.php +++ b/classes/tests/testcase.php @@ -180,7 +180,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 +311,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); } /** diff --git a/db/install.xml b/db/install.xml index 0065b1d7..7afa27ce 100644 --- a/db/install.xml +++ b/db/install.xml @@ -9,7 +9,7 @@ - + diff --git a/db/upgrade.php b/db/upgrade.php index bf06b09c..9870d5ce 100644 --- a/db/upgrade.php +++ b/db/upgrade.php @@ -220,5 +220,34 @@ function xmldb_tool_objectfs_upgrade($oldversion) { upgrade_plugin_savepoint(true, 2024120600, 'tool', 'objectfs'); } + if ($oldversion < 2026041007) { + // Migrate OBJECT_LOCATION_* constants from sequential integers to bit flags. + // Old values: ORPHANED=-2, ERROR(missing)=-1, LOCAL=0, DUPLICATED=1, EXTERNAL=2 + // New values: ERROR=0, ORPHANED=1, MISSING=2, LOCAL=3, EXTERNAL=6, DUPLICATED=7 + // + // Processing in an order that avoids value collisions (highest old value first). + + // Widen the location column to match install.xml (LENGTH 1 -> 3) before migrating values. + $table = new xmldb_table('tool_objectfs_objects'); + $field = new xmldb_field('location', XMLDB_TYPE_INTEGER, '3', null, XMLDB_NOTNULL, null, null, 'timeduplicated'); + $dbman->change_field_precision($table, $field); + + // Tool_objectfs_objects.location column. + $DB->execute('UPDATE {tool_objectfs_objects} SET location = 6 WHERE location = 2'); + $DB->execute('UPDATE {tool_objectfs_objects} SET location = 7 WHERE location = 1'); + $DB->execute('UPDATE {tool_objectfs_objects} SET location = 3 WHERE location = 0'); + $DB->execute('UPDATE {tool_objectfs_objects} SET location = 2 WHERE location = -1'); + $DB->execute('UPDATE {tool_objectfs_objects} SET location = 1 WHERE location = -2'); + + // The tool_objectfs_report_data.datakey stores location values as strings for the 'location' report type. + $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..816667d2 100644 --- a/lang/en/tool_objectfs.php +++ b/lang/en/tool_objectfs.php @@ -72,6 +72,7 @@ $string['object_status:location:duplicatedsize'] = 'Duplicated (size)'; $string['object_status:location:duplicatedsizechart'] = 'Duplicated (MB)'; $string['object_status:location:error'] = 'Missing from filedir and external storage (view files)'; +$string['object_status:location:missing'] = 'Missing from filedir and external storage (view files)'; $string['object_status:location:external'] = 'Only in external storage'; $string['object_status:location:externalcount'] = 'External (count)'; $string['object_status:location:externalsize'] = 'External (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..4bbee860 100644 --- a/tests/local/object_manipulator/checker_test.php +++ b/tests/local/object_manipulator/checker_test.php @@ -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/recoverer_test.php b/tests/local/object_manipulator/recoverer_test.php index 03abfce0..554769a2 100644 --- a/tests/local/object_manipulator/recoverer_test.php +++ b/tests/local/object_manipulator/recoverer_test.php @@ -61,7 +61,7 @@ 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]); + $DB->set_field('tool_objectfs_objects', 'location', OBJECT_LOCATION_MISSING, ['contenthash' => $object->contenthash]); $this->recoverer->execute([$object]); @@ -72,7 +72,7 @@ public function test_recoverer_will_recover_local_objects(): void { 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]); + $DB->set_field('tool_objectfs_objects', 'location', OBJECT_LOCATION_MISSING, ['contenthash' => $object->contenthash]); $this->recoverer->execute([$object]); @@ -83,7 +83,7 @@ public function test_recoverer_will_recover_duplicated_objects(): void { 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]); + $DB->set_field('tool_objectfs_objects', 'location', OBJECT_LOCATION_MISSING, ['contenthash' => $object->contenthash]); $this->recoverer->execute([$object]); @@ -94,11 +94,11 @@ public function test_recoverer_will_recover_remote_objects(): void { 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]); + $DB->set_field('tool_objectfs_objects', 'location', OBJECT_LOCATION_MISSING, ['contenthash' => $object->contenthash]); $this->recoverer->execute([$object]); $location = $DB->get_field('tool_objectfs_objects', 'location', ['contenthash' => $object->contenthash]); - $this->assertEquals(OBJECT_LOCATION_ERROR, $location); + $this->assertEquals(OBJECT_LOCATION_MISSING, $location); } } diff --git a/tests/object_file_system_test.php b/tests/object_file_system_test.php index 48d49db8..30a893bc 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 { @@ -322,7 +322,7 @@ public function test_readfile_updates_object_with_error_location_on_fail(): void restore_error_handler(); $location = $DB->get_field('tool_objectfs_objects', 'location', ['contenthash' => $fakefile->get_contenthash()]); - $this->assertEquals(OBJECT_LOCATION_ERROR, $location); + $this->assertEquals(OBJECT_LOCATION_MISSING, $location); } public function test_get_content_if_object_is_local(): void { @@ -354,7 +354,7 @@ public function test_get_content_updates_object_with_error_location_on_fail(): v restore_error_handler(); $location = $DB->get_field('tool_objectfs_objects', 'location', ['contenthash' => $fakefile->get_contenthash()]); - $this->assertEquals(OBJECT_LOCATION_ERROR, $location); + $this->assertEquals(OBJECT_LOCATION_MISSING, $location); } /** @@ -427,7 +427,7 @@ public function test_get_content_file_handle_does_not_set_error_location_on_tran } $location = $DB->get_field('tool_objectfs_objects', 'location', ['contenthash' => $file->get_contenthash()]); - $this->assertNotEquals(OBJECT_LOCATION_ERROR, $location); + $this->assertNotEquals(OBJECT_LOCATION_MISSING, $location); } public function test_remove_file_will_remove_local_file(): void { 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;