Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding support for delayed deletion of orphaned external files. #549

Open
wants to merge 2 commits into
base: MOODLE_310_STABLE
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions classes/local/manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ public static function get_objectfs_config() {
$config->batchsize = 10000;
$config->useproxy = 0;
$config->deleteexternal = 0;
$config->delaydeleteexternalobject = 0;

$config->filesystem = '';
$config->enablepresignedurls = 0;
Expand Down Expand Up @@ -185,6 +186,12 @@ public static function update_object(stdClass $object, $newlocation) {
$object->timeduplicated = time();
}

// If location change is 'orphaned' we update timeorphaned.
// Set time orphaned clock is ticking now for delay deletion comparison...
if ($newlocation === OBJECT_LOCATION_ORPHANED) {
$object->timeorphaned = time();
}

$object->location = $newlocation;
$DB->update_record('tool_objectfs_objects', $object);

Expand Down
12 changes: 10 additions & 2 deletions classes/task/delete_orphaned_object_metadata.php
Original file line number Diff line number Diff line change
Expand Up @@ -52,15 +52,23 @@ public function execute() {
'ageforremoval' => time() - $ageforremoval
];

// Check for delay deletion if enabled.
$delayquery = '';
if (!empty($this->config->delaydeleteexternalobject)) {
$params['deletetime'] = time() - $this->config->delaydeleteexternalobject;
$delayquery = 'AND o.timeorphaned < :deletetime';
}

if (!empty($this->config->deleteexternal) && $this->config->deleteexternal == TOOL_OBJECTFS_DELETE_EXTERNAL_TRASH) {
// We need to delete the external files as well as the orphaned data.
$filesystem = new $this->config->filesystem();

// Join with files table to make extra sure we aren't deleting something that already exists.
$sql = 'SELECT o.*
$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 o.location = :location AND o.timeduplicated < :ageforremoval
$delayquery";

$objects = $DB->get_recordset_sql($sql, $params);
$count = 0;
Expand Down
3 changes: 2 additions & 1 deletion db/install.xml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<?xml version="1.0" encoding="UTF-8" ?>
<XMLDB PATH="admin/tool/objectfs/db" VERSION="20161207" COMMENT="XMLDB file for Moodle tool/objectfs plugin"
<XMLDB PATH="admin/tool/objectfs/db" VERSION="20230121" COMMENT="XMLDB file for Moodle tool/objectfs plugin"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:noNamespaceSchemaLocation="../../../../lib/xmldb/xmldb.xsd"
>
Expand All @@ -11,6 +11,7 @@
<FIELD NAME="timeduplicated" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="false"/>
<FIELD NAME="location" TYPE="int" LENGTH="1" NOTNULL="true" SEQUENCE="false"/>
<FIELD NAME="filesize" TYPE="int" LENGTH="20" NOTNULL="false" SEQUENCE="false"/>
<FIELD NAME="timeorphaned" TYPE="int" LENGTH="20" NOTNULL="false" SEQUENCE="false"/>
</FIELDS>
<KEYS>
<KEY NAME="contenthash" TYPE="unique" FIELDS="contenthash"/>
Expand Down
1 change: 0 additions & 1 deletion db/tasks.php
Original file line number Diff line number Diff line change
Expand Up @@ -108,4 +108,3 @@
'month' => '*'
),
);

15 changes: 15 additions & 0 deletions db/upgrade.php
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,21 @@ function xmldb_tool_objectfs_upgrade($oldversion) {
upgrade_plugin_savepoint(true, 2022070401, 'tool', 'objectfs');
}

if ($oldversion < 2022120806) {

// Define field timeorphaned to be added to tool_objectfs_objects.
$table = new xmldb_table('tool_objectfs_objects');
$field = new xmldb_field('timeorphaned', XMLDB_TYPE_INTEGER, '20', null, null, null, null, 'filesize');

// Conditionally launch add field timeorphaned.
if (!$dbman->field_exists($table, $field)) {
$dbman->add_field($table, $field);
}

// Objectfs savepoint reached.
upgrade_plugin_savepoint(true, 2022120806, 'tool', 'objectfs');
}

if ($oldversion < 2023013100) {
// Check to make sure adhoc task not already running.
if (!$DB->record_exists('task_adhoc', ['classname' => '\tool_objectfs\task\populate_objects_filesize'])) {
Expand Down
2 changes: 2 additions & 0 deletions lang/en/tool_objectfs.php
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,8 @@
$string['settings:batchsize_help'] = 'Number of files to be transferred in one cron run';
$string['settings:maxorphanedage'] = 'Max orphaned object age';
$string['settings:maxorphanedage_help'] = 'If set to zero, this will not delete old orphaned metadata for objects. Otherwise, it will remove these records as they are no longer relevant. An orphaned object is one where the metadata exists on the {tool_objectfs_objects} table but referenced file no longer exists.';
$string['settings:delaydeleteexternalobject'] = 'Delay delete external object';
$string['settings:delaydeleteexternalobject_help'] = 'Delay delete of external <strong>orphaned</strong> objects. They will be deleted once this time has passed after they became orphaned.';
$string['settings:minimumage'] = 'Minimum age';
$string['settings:minimumage_help'] = 'Minimum age that a object must exist on the local filedir before it will be considered for transfer.';
$string['settings:deleteexternal'] = 'Delete external objects';
Expand Down
4 changes: 4 additions & 0 deletions settings.php
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,10 @@
new lang_string('settings:maxorphanedage', 'tool_objectfs'),
new lang_string('settings:maxorphanedage_help', 'tool_objectfs'), 0, DAYSECS));

$settings->add(new admin_setting_configduration('tool_objectfs/delaydeleteexternalobject',
new lang_string('settings:delaydeleteexternalobject', 'tool_objectfs'),
new lang_string('settings:delaydeleteexternalobject_help', 'tool_objectfs'), 0, DAYSECS));

$settings->add(new admin_setting_configcheckbox('tool_objectfs/enablelogging',
new lang_string('settings:enablelogging', 'tool_objectfs'), '', ''));

Expand Down
54 changes: 54 additions & 0 deletions tests/local/tasks_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

namespace tool_objectfs\local;

use tool_objectfs\tests\test_file_system;

/**
* End to end tests for tasks. Make sure all the plumbing is ok.
*
Expand All @@ -32,6 +34,14 @@ protected function tearDown(): void {
ob_end_clean();
}

public static function get_orphaned_delayed_count() {
global $DB;

$orphanedcount = $DB->count_records_sql("SELECT COUNT(id) FROM {tool_objectfs_objects} WHERE timeorphaned > 0");

return $orphanedcount;
}

public function test_run_legacy_cron() {
$config = manager::get_objectfs_config();
$config->enabletasks = true;
Expand Down Expand Up @@ -69,4 +79,48 @@ public function test_run_scheduled_tasks() {
$this->expectNotToPerformAssertions(); // Just check we get this far without any exceptions.
}

public function test_delay_delete_orphaned_object() {
global $CFG, $DB;

$this->resetAfterTest(true);

$this->filesystem = new test_file_system();
$file = $this->create_remote_file();
$filehash = $file->get_contenthash();
$objectrecord = $DB->get_record('tool_objectfs_objects', ['contenthash' => $filehash]);
$file->delete(); // This makes it orphaned.

// Set config.
set_config('delaydeleteexternalobject', (3 * DAYSECS), 'tool_objectfs');
set_config('maxorphanedage', (1 * DAYSECS), 'tool_objectfs');
set_config('deleteexternal', TOOL_OBJECTFS_DELETE_EXTERNAL_TRASH, 'tool_objectfs');
set_config('filesystem', "tool_objectfs\\tests\\test_file_system", 'tool_objectfs');

unset($CFG->forced_plugin_settings['tool_objectfs']['deleteexternal']); // This will be reset in parent::setUp().

// Update time so it is considered for orphaned deletion. It should be skipped due to delay setting.
$objectrecord->timeduplicated = time() - (2 * DAYSECS); // Time for it to be considered orphaned.
$DB->update_record('tool_objectfs_objects', $objectrecord);

$objectrecord = manager::update_object($objectrecord, OBJECT_LOCATION_ORPHANED);

$pretaskcount = $this->get_orphaned_delayed_count();

$task = \core\task\manager::get_scheduled_task('\\tool_objectfs\\task\\delete_orphaned_object_metadata');
$task->execute();

$posttaskcount = $this->get_orphaned_delayed_count();

$this->assertSame($pretaskcount, $posttaskcount, 'No file should have been deleted due to delay setting');

$objectrecord->timeorphaned = time() - (4 * DAYSECS); // Time for it to be able to be deleted.
$DB->update_record('tool_objectfs_objects', $objectrecord);

$task = \core\task\manager::get_scheduled_task('\\tool_objectfs\\task\\delete_orphaned_object_metadata');
$task->execute();

$posttaskcount = $this->get_orphaned_delayed_count();

$this->assertGreaterThan($posttaskcount, $pretaskcount, 'There should be less files after removing delayed orphaned files');
}
}