From 790c89b34d4fad043e174c88302a52431b52adcf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Massart?= Date: Thu, 26 Apr 2018 12:54:01 +0800 Subject: [PATCH] MDL-62208 core_fileconverter: Implement privacy API --- files/classes/conversion.php | 26 +++++ .../classes/task/conversion_cleanup_task.php | 1 + files/converter/classes/privacy/provider.php | 96 +++++++++++++++++++ files/tests/conversion_test.php | 43 +++++++++ lang/en/fileconverter.php | 28 ++++++ 5 files changed, 194 insertions(+) create mode 100644 files/converter/classes/privacy/provider.php create mode 100644 lang/en/fileconverter.php diff --git a/files/classes/conversion.php b/files/classes/conversion.php index d207d6cab0fe6..9c61530c02c37 100644 --- a/files/classes/conversion.php +++ b/files/classes/conversion.php @@ -199,6 +199,32 @@ public static function remove_old_conversion_records() { ]); } + /** + * Remove orphan records. + * + * Records are considered orphans when their source file not longer exists. + * In this scenario we do not want to keep the converted file any longer, + * in particular to be compliant with privacy laws. + */ + public static function remove_orphan_records() { + global $DB; + + $sql = " + SELECT c.id + FROM {" . self::TABLE . "} c + LEFT JOIN {files} f + ON f.id = c.sourcefileid + WHERE f.id IS NULL"; + $ids = $DB->get_fieldset_sql($sql, []); + + if (empty($ids)) { + return; + } + + list($insql, $inparams) = $DB->get_in_or_equal($ids, SQL_PARAMS_NAMED); + $DB->delete_records_select(self::TABLE, "id $insql", $inparams); + } + /** * Set the source file id for the conversion. * diff --git a/files/classes/task/conversion_cleanup_task.php b/files/classes/task/conversion_cleanup_task.php index 4f774f1eac140..182260fcd044b 100644 --- a/files/classes/task/conversion_cleanup_task.php +++ b/files/classes/task/conversion_cleanup_task.php @@ -48,6 +48,7 @@ public function get_name() { */ public function execute() { \core_files\conversion::remove_old_conversion_records(); + \core_files\conversion::remove_orphan_records(); } } diff --git a/files/converter/classes/privacy/provider.php b/files/converter/classes/privacy/provider.php new file mode 100644 index 0000000000000..cd1874b6309d0 --- /dev/null +++ b/files/converter/classes/privacy/provider.php @@ -0,0 +1,96 @@ +. + +/** + * Data provider. + * + * @package core_fileconverter + * @copyright 2018 Frédéric Massart + * @author Frédéric Massart + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +namespace core_fileconverter\privacy; +defined('MOODLE_INTERNAL') || die(); + +use context; +use core_privacy\local\metadata\collection; +use core_privacy\local\request\approved_contextlist; + +/** + * Data provider class. + * + * @package core_fileconverter + * @copyright 2018 Frédéric Massart + * @author Frédéric Massart + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class provider implements + \core_privacy\local\metadata\provider, + \core_privacy\local\request\subsystem\provider { + + /** + * Returns metadata. + * + * @param collection $collection The initialised collection to add items to. + * @return collection A listing of user data stored through this system. + */ + public static function get_metadata(collection $collection) : collection { + $collection->add_plugintype_link('fileconverter', [], 'privacy:metadata:plugintypefileconverter'); + return $collection; + } + + /** + * Get the list of contexts that contain user information for the specified user. + * + * @param int $userid The user to search. + * @return \contextlist $contextlist The contextlist containing the list of contexts used in this plugin. + */ + public static function get_contexts_for_userid(int $userid) : \core_privacy\local\request\contextlist { + // We cannot associate files with a particular user as it would require to know all the details about + // the source file, which only the component owning it knows about. And, the 'usermodified' attribute + // of the conversion class is not an identifier of data ownership, merely that this user was logged in + // when the conversion was requested. As such, as the owning component should declare the source file, + // and we can't make sense of 'usermodified', we will not be reporting anything here. Also note that + // the clean up task will ensure that whenever a source file is delete, its conversions also are. + return new \core_privacy\local\request\contextlist(); + } + + /** + * Export all user data for the specified user, in the specified contexts. + * + * @param approved_contextlist $contextlist The approved contexts to export information for. + */ + public static function export_user_data(approved_contextlist $contextlist) { + } + + /** + * Delete all data for all users in the specified context. + * + * @param context $context The specific context to delete data for. + */ + public static function delete_data_for_all_users_in_context(context $context) { + } + + /** + * Delete all user data for the specified user, in the specified contexts. + * + * @param approved_contextlist $contextlist The approved contexts and user information to delete information for. + */ + public static function delete_data_for_user(approved_contextlist $contextlist) { + } + +} diff --git a/files/tests/conversion_test.php b/files/tests/conversion_test.php index 662ccc3003f76..3293bcc85020c 100644 --- a/files/tests/conversion_test.php +++ b/files/tests/conversion_test.php @@ -412,4 +412,47 @@ public function test_remove_old_conversion_records_young() { $this->assertEquals(1, $DB->count_records(conversion::TABLE)); } + + /** + * Test orphan records are removed. + */ + public function test_remove_orphan_records() { + global $DB; + $this->resetAfterTest(); + + $sf1 = $this->create_stored_file('1', '1'); + $sf2 = $this->create_stored_file('2', '2'); + $sf3 = $this->create_stored_file('3', '3'); + $c1 = new conversion(0, (object) ['sourcefileid' => $sf1->get_id(), 'targetformat' => 'pdf']); + $c1->create(); + $c2 = new conversion(0, (object) ['sourcefileid' => $sf2->get_id(), 'targetformat' => 'pdf']); + $c2->create(); + $c3 = new conversion(0, (object) ['sourcefileid' => $sf3->get_id(), 'targetformat' => 'pdf']); + $c3->create(); + + $this->assertTrue(conversion::record_exists($c1->get('id'))); + $this->assertTrue(conversion::record_exists($c2->get('id'))); + $this->assertTrue(conversion::record_exists($c3->get('id'))); + + // Nothing should happen here. + conversion::remove_orphan_records(); + $this->assertTrue(conversion::record_exists($c1->get('id'))); + $this->assertTrue(conversion::record_exists($c2->get('id'))); + $this->assertTrue(conversion::record_exists($c3->get('id'))); + + // Delete file #2. + $sf2->delete(); + conversion::remove_orphan_records(); + $this->assertTrue(conversion::record_exists($c1->get('id'))); + $this->assertFalse(conversion::record_exists($c2->get('id'))); + $this->assertTrue(conversion::record_exists($c3->get('id'))); + + // Delete file #1, #3. + $sf1->delete(); + $sf3->delete(); + conversion::remove_orphan_records(); + $this->assertFalse(conversion::record_exists($c1->get('id'))); + $this->assertFalse(conversion::record_exists($c2->get('id'))); + $this->assertFalse(conversion::record_exists($c3->get('id'))); + } } diff --git a/lang/en/fileconverter.php b/lang/en/fileconverter.php new file mode 100644 index 0000000000000..add9823151a50 --- /dev/null +++ b/lang/en/fileconverter.php @@ -0,0 +1,28 @@ +. + +/** + * Language file. + * + * @package core_fileconverter + * @copyright 2018 Frédéric Massart + * @author Frédéric Massart + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +$string['privacy:metadata:plugintypefileconverter'] = 'The fileconverter subsystem acts as a channel, passing requests from plugins to the various fileconverter plugins.';