Refactor candidate classes

classes/local/object_manipulator/candidates/bitmask_candidates.php

@@ -0,0 +1,119 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+/**
+ * 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);
+    }
+}

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 [];
+        }
+    }
 }

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);
     }
 }

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
      */

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
-        );
-    }
 }

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);
     }
 }

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];
     }
 }

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);
     }
 }

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;