Merge branch 'MDL-62418-master' of git://github.com/sarjona/moodle
authorEloy Lafuente (stronk7) <stronk7@moodle.org>
Mon, 14 May 2018 01:01:29 +0000 (03:01 +0200)
committerEloy Lafuente (stronk7) <stronk7@moodle.org>
Mon, 14 May 2018 01:01:29 +0000 (03:01 +0200)
15 files changed:
admin/tool/dataprivacy/classes/api.php
admin/tool/dataprivacy/classes/local/helper.php
admin/tool/dataprivacy/classes/task/initiate_data_request_task.php
admin/tool/dataprivacy/classes/task/process_data_request_task.php
admin/tool/dataprivacy/createdatarequest_form.php
admin/tool/dataprivacy/lang/en/tool_dataprivacy.php
admin/tool/dataprivacy/tests/api_test.php
backup/converter/convertlib.php
lang/en/privacy.php
lib/classes/task/file_temp_cleanup_task.php
lib/tests/cronlib_test.php
privacy/classes/local/request/moodle_content_writer.php
privacy/classes/manager.php
privacy/classes/tests/request/content_writer.php
privacy/tests/moodle_content_writer_test.php

index 9054612..7bffc9f 100644 (file)
@@ -37,6 +37,7 @@ use moodle_url;
 use required_capability_exception;
 use stdClass;
 use tool_dataprivacy\external\data_request_exporter;
+use tool_dataprivacy\local\helper;
 use tool_dataprivacy\task\initiate_data_request_task;
 use tool_dataprivacy\task\process_data_request_task;
 
@@ -186,8 +187,25 @@ class api {
         $datarequest = new data_request();
         // The user the request is being made for.
         $datarequest->set('userid', $foruser);
+
+        $requestinguser = $USER->id;
+        // Check when the user is making a request on behalf of another.
+        if ($requestinguser != $foruser) {
+            if (self::is_site_dpo($requestinguser)) {
+                // The user making the request is a DPO. Should be fine.
+                $datarequest->set('dpo', $requestinguser);
+            } else {
+                // If not a DPO, only users with the capability to make data requests for the user should be allowed.
+                // (e.g. users with the Parent role, etc).
+                if (!api::can_create_data_request_for_user($foruser)) {
+                    $forusercontext = \context_user::instance($foruser);
+                    throw new required_capability_exception($forusercontext,
+                            'tool/dataprivacy:makedatarequestsforchildren', 'nopermissions', '');
+                }
+            }
+        }
         // The user making the request.
-        $datarequest->set('requestedby', $USER->id);
+        $datarequest->set('requestedby', $requestinguser);
         // Set status.
         $datarequest->set('status', self::DATAREQUEST_STATUS_PENDING);
         // Set request type.
@@ -218,16 +236,29 @@ class api {
      * @throws dml_exception
      */
     public static function get_data_requests($userid = 0) {
-        global $USER;
+        global $DB, $USER;
         $results = [];
         $sort = 'status ASC, timemodified ASC';
         if ($userid) {
             // Get the data requests for the user or data requests made by the user.
-            $select = "userid = :userid OR requestedby = :requestedby";
+            $select = "(userid = :userid OR requestedby = :requestedby)";
             $params = [
                 'userid' => $userid,
                 'requestedby' => $userid
             ];
+
+            // Build a list of user IDs that the user is allowed to make data requests for.
+            // Of course, the user should be included in this list.
+            $alloweduserids = [$userid];
+            // Get any users that the user can make data requests for.
+            if ($children = helper::get_children_of_user($userid)) {
+                // Get the list of user IDs of the children and merge to the allowed user IDs.
+                $alloweduserids = array_merge($alloweduserids, array_keys($children));
+            }
+            list($insql, $inparams) = $DB->get_in_or_equal($alloweduserids, SQL_PARAMS_NAMED);
+            $select .= " AND userid $insql";
+            $params = array_merge($params, $inparams);
+
             $results = data_request::get_records_select($select, $params, $sort);
         } else {
             // If the current user is one of the site's Data Protection Officers, then fetch all data requests.
@@ -290,17 +321,19 @@ class api {
      * @param int $requestid The request identifier.
      * @param int $status The request status.
      * @param int $dpoid The user ID of the Data Protection Officer
+     * @param string $comment The comment about the status update.
      * @return bool
      * @throws invalid_persistent_exception
      * @throws coding_exception
      */
-    public static function update_request_status($requestid, $status, $dpoid = 0) {
+    public static function update_request_status($requestid, $status, $dpoid = 0, $comment = '') {
         // Update the request.
         $datarequest = new data_request($requestid);
         $datarequest->set('status', $status);
         if ($dpoid) {
             $datarequest->set('dpo', $dpoid);
         }
+        $datarequest->set('dpocomment', $comment);
         return $datarequest->update();
     }
 
@@ -454,6 +487,19 @@ class api {
         return message_send($message);
     }
 
+    /**
+     * Checks whether a non-DPO user can make a data request for another user.
+     *
+     * @param int $user The user ID of the target user.
+     * @param int $requester The user ID of the user making the request.
+     * @return bool
+     * @throws coding_exception
+     */
+    public static function can_create_data_request_for_user($user, $requester = null) {
+        $usercontext = \context_user::instance($user);
+        return has_capability('tool/dataprivacy:makedatarequestsforchildren', $usercontext, $requester);
+    }
+
     /**
      * Creates a new data purpose.
      *
index 77664b8..d7c3436 100644 (file)
@@ -108,4 +108,42 @@ class helper {
                 throw new moodle_exception('errorinvalidrequeststatus', 'tool_dataprivacy');
         }
     }
+
+    /**
+     * Get the users that a user can make data request for.
+     *
+     * E.g. User having a parent role and has the 'tool/dataprivacy:makedatarequestsforchildren' capability.
+     * @param int $userid The user's ID.
+     * @return array
+     */
+    public static function get_children_of_user($userid) {
+        global $DB;
+
+        // Get users that the user has role assignments to.
+        $allusernames = get_all_user_name_fields(true, 'u');
+        $sql = "SELECT u.id, $allusernames
+                  FROM {role_assignments} ra, {context} c, {user} u
+                 WHERE ra.userid = :userid
+                       AND ra.contextid = c.id
+                       AND c.instanceid = u.id
+                       AND c.contextlevel = :contextlevel";
+        $params = [
+            'userid' => $userid,
+            'contextlevel' => CONTEXT_USER
+        ];
+
+        // The final list of users that we will return;
+        $finalresults = [];
+
+        // Our prospective list of users.
+        if ($candidates = $DB->get_records_sql($sql, $params)) {
+            foreach ($candidates as $key => $child) {
+                $childcontext = \context_user::instance($child->id);
+                if (has_capability('tool/dataprivacy:makedatarequestsforchildren', $childcontext, $userid)) {
+                    $finalresults[$key] = $child;
+                }
+            }
+        }
+        return $finalresults;
+    }
 }
index 4e62b74..0cebeb0 100644 (file)
@@ -70,6 +70,27 @@ class initiate_data_request_task extends adhoc_task {
             return;
         }
 
+        $requestedby = $datarequest->get('requestedby');
+        $valid = true;
+        $comment = '';
+        $foruser = $datarequest->get('userid');
+        if ($foruser != $requestedby) {
+            if (!$valid = api::can_create_data_request_for_user($foruser, $requestedby)) {
+                $params = (object)[
+                    'requestedby' => $requestedby,
+                    'userid' => $foruser
+                ];
+                $comment = get_string('errornocapabilitytorequestforothers', 'tool_dataprivacy', $params);
+                mtrace($comment);
+            }
+        }
+        // Reject the request outright if it's invalid.
+        if (!$valid) {
+            $dpo = $datarequest->get('dpo');
+            api::update_request_status($requestid, api::DATAREQUEST_STATUS_REJECTED, $dpo, $comment);
+            return;
+        }
+
         // Update the status of this request as pre-processing.
         mtrace('Generating the contexts containing personal data for the user...');
         api::update_request_status($requestid, api::DATAREQUEST_STATUS_PREPROCESSING);
index bacf29a..589ef01 100644 (file)
@@ -182,23 +182,27 @@ class process_data_request_task extends adhoc_task {
         }
         mtrace('Message sent to user: ' . $messagetextdata['username']);
 
-        // Send to the requester as well. requestedby is 0 if the request was made on behalf of the user by a DPO.
-        if (!empty($request->requestedby) && $foruser->id != $request->requestedby) {
-            $requestedby = core_user::get_user($request->requestedby);
-            $message->userto = $requestedby;
-            $messagetextdata['username'] = fullname($requestedby);
-            // Render message email body.
-            $messagehtml = $output->render_from_template('tool_dataprivacy/data_request_results_email', $messagetextdata);
-            $message->fullmessage = html_to_text($messagehtml);
-            $message->fullmessagehtml = $messagehtml;
-
-            // Send message.
-            if ($emailonly) {
-                email_to_user($requestedby, $dpo, $subject, $message->fullmessage, $messagehtml);
-            } else {
-                message_send($message);
+        // Send to requester as well if this request was made on behalf of another user who's not a DPO,
+        // and has the capability to make data requests for the user (e.g. Parent).
+        if (!api::is_site_dpo($request->requestedby) && $foruser->id != $request->requestedby) {
+            // Ensure the requester has the capability to make data requests for this user.
+            if (api::can_create_data_request_for_user($request->userid, $request->requestedby)) {
+                $requestedby = core_user::get_user($request->requestedby);
+                $message->userto = $requestedby;
+                $messagetextdata['username'] = fullname($requestedby);
+                // Render message email body.
+                $messagehtml = $output->render_from_template('tool_dataprivacy/data_request_results_email', $messagetextdata);
+                $message->fullmessage = html_to_text($messagehtml);
+                $message->fullmessagehtml = $messagehtml;
+
+                // Send message.
+                if ($emailonly) {
+                    email_to_user($requestedby, $dpo, $subject, $message->fullmessage, $messagehtml);
+                } else {
+                    message_send($message);
+                }
+                mtrace('Message sent to requester: ' . $messagetextdata['username']);
             }
-            mtrace('Message sent to requester: ' . $messagetextdata['username']);
         }
 
         if ($request->type == api::DATAREQUEST_TYPE_DELETE) {
index b42aaf3..c02d9d4 100644 (file)
@@ -23,6 +23,7 @@
  */
 
 use tool_dataprivacy\api;
+use tool_dataprivacy\local\helper;
 
 defined('MOODLE_INTERNAL') || die();
 
@@ -58,27 +59,12 @@ class tool_dataprivacy_data_request_form extends moodleform {
 
         } else {
             // Get users whom you are being a guardian to if your role has the capability to make data requests for children.
-            $allusernames = get_all_user_name_fields(true, 'u');
-            $sql = "SELECT u.id, $allusernames
-                      FROM {role_assignments} ra, {context} c, {user} u
-                     WHERE ra.userid = :userid
-                           AND ra.contextid = c.id
-                           AND c.instanceid = u.id
-                           AND c.contextlevel = :contextlevel";
-            $params = [
-                'userid' => $USER->id,
-                'contextlevel' => CONTEXT_USER
-            ];
-            $children = $DB->get_records_sql($sql, $params);
-
-            if ($children) {
-                $useroptions = [];
-                $useroptions[$USER->id] = fullname($USER);
-                foreach ($children as $child) {
-                    $childcontext = context_user::instance($child->id);
-                    if (has_capability('tool/dataprivacy:makedatarequestsforchildren', $childcontext)) {
-                        $useroptions[$child->id] = fullname($child);
-                    }
+            if ($children = helper::get_children_of_user($USER->id)) {
+                $useroptions = [
+                    $USER->id => fullname($USER)
+                ];
+                foreach ($children as $key => $child) {
+                    $useroptions[$key] = fullname($child);
                 }
                 $mform->addElement('autocomplete', 'userid', get_string('requestfor', 'tool_dataprivacy'), $useroptions);
                 $mform->addRule('userid', null, 'required', null, 'client');
index c4461f5..3d4dcf4 100644 (file)
@@ -89,6 +89,7 @@ $string['effectiveretentionperioduser'] = '{$a} (since the last time the user ac
 $string['emailsalutation'] = 'Dear {$a},';
 $string['errorinvalidrequeststatus'] = 'Invalid request status!';
 $string['errorinvalidrequesttype'] = 'Invalid request type!';
+$string['errornocapabilitytorequestforothers'] = 'User {$a->requestedby} doesn\'t have the capability to make a data request on behalf of user {$a->userid}';
 $string['errornoexpiredcontexts'] = 'There are no expired contexts to process';
 $string['errorcontexthasunexpiredchildren'] = 'The context "{$a}" still has sub-contexts that have not yet expired. No contexts have been flagged for deletion.';
 $string['errorrequestalreadyexists'] = 'You already have an ongoing request.';
index 2589607..6e06478 100644 (file)
@@ -57,6 +57,7 @@ class tool_dataprivacy_api_testcase extends advanced_testcase {
     public function test_update_request_status() {
         $generator = new testing_data_generator();
         $s1 = $generator->create_user();
+        $this->setUser($s1);
 
         // Create the sample data request.
         $datarequest = api::create_data_request($s1->id, api::DATAREQUEST_TYPE_EXPORT);
@@ -145,6 +146,7 @@ class tool_dataprivacy_api_testcase extends advanced_testcase {
         set_config('dporoles', $managerroleid, 'tool_dataprivacy');
 
         // Create the sample data request.
+        $this->setUser($s1);
         $datarequest = api::create_data_request($s1->id, api::DATAREQUEST_TYPE_EXPORT);
         $requestid = $datarequest->get('id');
 
@@ -186,6 +188,7 @@ class tool_dataprivacy_api_testcase extends advanced_testcase {
         set_config('dporoles', $managerroleid, 'tool_dataprivacy');
 
         // Create the sample data request.
+        $this->setUser($s1);
         $datarequest = api::create_data_request($s1->id, api::DATAREQUEST_TYPE_EXPORT);
         $requestid = $datarequest->get('id');
 
@@ -203,6 +206,7 @@ class tool_dataprivacy_api_testcase extends advanced_testcase {
         $teacher = $generator->create_user();
 
         // Create the sample data request.
+        $this->setUser($student);
         $datarequest = api::create_data_request($student->id, api::DATAREQUEST_TYPE_EXPORT);
 
         $requestid = $datarequest->get('id');
@@ -286,6 +290,71 @@ class tool_dataprivacy_api_testcase extends advanced_testcase {
         $datarequest = api::create_data_request($user->id, api::DATAREQUEST_TYPE_EXPORT, $comment);
         $this->assertEquals($user->id, $datarequest->get('userid'));
         $this->assertEquals($user->id, $datarequest->get('requestedby'));
+        $this->assertEquals(0, $datarequest->get('dpo'));
+        $this->assertEquals(api::DATAREQUEST_TYPE_EXPORT, $datarequest->get('type'));
+        $this->assertEquals(api::DATAREQUEST_STATUS_PENDING, $datarequest->get('status'));
+        $this->assertEquals($comment, $datarequest->get('comments'));
+
+        // Test adhoc task creation.
+        $adhoctasks = manager::get_adhoc_tasks(initiate_data_request_task::class);
+        $this->assertCount(1, $adhoctasks);
+    }
+
+    /**
+     * Test for api::create_data_request() made by DPO.
+     */
+    public function test_create_data_request_by_dpo() {
+        global $USER;
+
+        $generator = new testing_data_generator();
+        $user = $generator->create_user();
+        $comment = 'sample comment';
+
+        // Login as DPO (Admin is DPO by default).
+        $this->setAdminUser();
+
+        // Test data request creation.
+        $datarequest = api::create_data_request($user->id, api::DATAREQUEST_TYPE_EXPORT, $comment);
+        $this->assertEquals($user->id, $datarequest->get('userid'));
+        $this->assertEquals($USER->id, $datarequest->get('requestedby'));
+        $this->assertEquals($USER->id, $datarequest->get('dpo'));
+        $this->assertEquals(api::DATAREQUEST_TYPE_EXPORT, $datarequest->get('type'));
+        $this->assertEquals(api::DATAREQUEST_STATUS_PENDING, $datarequest->get('status'));
+        $this->assertEquals($comment, $datarequest->get('comments'));
+
+        // Test adhoc task creation.
+        $adhoctasks = manager::get_adhoc_tasks(initiate_data_request_task::class);
+        $this->assertCount(1, $adhoctasks);
+    }
+
+    /**
+     * Test for api::create_data_request() made by a parent.
+     */
+    public function test_create_data_request_by_parent() {
+        global $DB;
+
+        $generator = new testing_data_generator();
+        $user = $generator->create_user();
+        $parent = $generator->create_user();
+        $comment = 'sample comment';
+
+        // Get the teacher role pretend it's the parent roles ;).
+        $systemcontext = context_system::instance();
+        $usercontext = context_user::instance($user->id);
+        $parentroleid = $DB->get_field('role', 'id', array('shortname' => 'teacher'));
+        // Give the manager role with the capability to manage data requests.
+        assign_capability('tool/dataprivacy:makedatarequestsforchildren', CAP_ALLOW, $parentroleid, $systemcontext->id, true);
+        // Assign the parent to user.
+        role_assign($parentroleid, $parent->id, $usercontext->id);
+
+        // Login as the user's parent.
+        $this->setUser($parent);
+
+        // Test data request creation.
+        $datarequest = api::create_data_request($user->id, api::DATAREQUEST_TYPE_EXPORT, $comment);
+        $this->assertEquals($user->id, $datarequest->get('userid'));
+        $this->assertEquals($parent->id, $datarequest->get('requestedby'));
+        $this->assertEquals(0, $datarequest->get('dpo'));
         $this->assertEquals(api::DATAREQUEST_TYPE_EXPORT, $datarequest->get('type'));
         $this->assertEquals(api::DATAREQUEST_STATUS_PENDING, $datarequest->get('status'));
         $this->assertEquals($comment, $datarequest->get('comments'));
@@ -351,8 +420,10 @@ class tool_dataprivacy_api_testcase extends advanced_testcase {
         $comment = 'sample comment';
 
         // Make a data request as user 1.
+        $this->setUser($user1);
         $d1 = api::create_data_request($user1->id, api::DATAREQUEST_TYPE_EXPORT, $comment);
         // Make a data request as user 2.
+        $this->setUser($user2);
         $d2 = api::create_data_request($user2->id, api::DATAREQUEST_TYPE_EXPORT, $comment);
 
         // Fetching data requests of specific users.
@@ -406,6 +477,7 @@ class tool_dataprivacy_api_testcase extends advanced_testcase {
         $user1 = $generator->create_user();
 
         // Make a data request as user 1.
+        $this->setUser($user1);
         $request = api::create_data_request($user1->id, api::DATAREQUEST_TYPE_EXPORT);
         // Set the status.
         api::update_request_status($request->get('id'), $status);
index 1fdcf1d..9a3f87a 100644 (file)
@@ -239,15 +239,17 @@ abstract class base_converter implements loggable {
     protected function replace_tempdir() {
         global $CFG;
 
+        $tempdir = $this->get_tempdir_path();
+
         if (empty($CFG->keeptempdirectoriesonbackup)) {
-            fulldelete($this->get_tempdir_path());
+            fulldelete($tempdir);
         } else {
-            if (!rename($this->get_tempdir_path(), $this->get_tempdir_path()  . '_' . $this->get_name() . '_' . $this->id . '_source')) {
+            if (!rename($tempdir, $tempdir . '_' . $this->get_name() . '_' . $this->id . '_source')) {
                 throw new convert_exception('failed_rename_source_tempdir');
             }
         }
 
-        if (!rename($this->get_workdir_path(), $this->get_tempdir_path())) {
+        if (!rename($this->get_workdir_path(), $tempdir)) {
             throw new convert_exception('failed_move_converted_into_place');
         }
     }
index 073daac..f20c57c 100644 (file)
  */
 
 $string['privacy:metadata'] = 'The privacy subsystem does not store any data of its own and is designed to act as a channel between components and the interface used to describe, export, and remove their data.';
+$string['trace:done'] = 'Complete';
+$string['trace:exportcomplete'] = 'Export complete';
+$string['trace:exportingapproved'] = 'Performing primary export for {$a->total} components ({$a->datetime})';
+$string['trace:exportingrelated'] = 'Performing related export for {$a->total} components ({$a->datetime})';
+$string['trace:finalisingexport'] = 'Finalising export';
+$string['trace:processingcomponent'] = 'Processing {$a->component} ({$a->progress}/{$a->total}) ({$a->datetime})';
+$string['trace:fetchcomponents'] = 'Fetching {$a->total} components ({$a->datetime})';
+$string['trace:deletingapproved'] = 'Performing removal of approved {$a->total} contexts ({$a->datetime})';
+$string['trace:deletingcontext'] = 'Performing removal of context from {$a->total} components ({$a->datetime})';
index 7166872..ac3359c 100644 (file)
@@ -110,7 +110,7 @@ class file_temp_cleanup_task extends scheduled_task {
         $this->execute_on($CFG->tempdir);
 
         // Run on $CFG->backuptempdir too, if different from the default one, '$CFG->tempdir/backup'.
-        if (realpath(dirname($CFG->backuptempdir)) !== $CFG->tempdir) {
+        if (realpath(dirname($CFG->backuptempdir)) !== realpath($CFG->tempdir)) {
             // The $CFG->backuptempdir setting is different from the default '$CFG->tempdir/backup'.
             $this->execute_on($CFG->backuptempdir);
         }
index 8db487a..4c4deef 100644 (file)
@@ -171,8 +171,8 @@ class cronlib_testcase extends basic_testcase {
             $isvalid = true;
             $isvalid = $isvalid && !$iter->isDot();
             // Remove the default $CFG->tempdir/backup directory and $CFG->tempdir/.htaccess file from this comparison.
-            $isvalid = $isvalid && !($iter->isDir() && ($iter->getRealpath() === "{$tmpdir}/backup"));
-            $isvalid = $isvalid && !($iter->isFile() && ($iter->getRealpath() === "{$tmpdir}/.htaccess"));
+            $isvalid = $isvalid && !($iter->isDir() && ($iter->getRealpath() === $tmpdir . DIRECTORY_SEPARATOR . 'backup'));
+            $isvalid = $isvalid && !($iter->isFile() && ($iter->getRealpath() === $tmpdir . DIRECTORY_SEPARATOR . '.htaccess'));
             if ($isvalid) {
                 $actual[] = $iter->getRealPath();
             }
index 2e82e6a..bf25a06 100644 (file)
@@ -162,7 +162,7 @@ class moodle_content_writer implements content_writer {
      * @return  string                      The processed string
      */
     public function rewrite_pluginfile_urls(array $subcontext, $component, $filearea, $itemid, $text) : string {
-        return str_replace('@@PLUGINFILE@@/', $this->get_files_target_path($component, $filearea, $itemid).'/', $text);
+        return str_replace('@@PLUGINFILE@@/', $this->get_files_target_url($component, $filearea, $itemid).'/', $text);
     }
 
     /**
@@ -272,7 +272,9 @@ class moodle_content_writer implements content_writer {
         // Join the directory together with the name.
         $filepath = implode(DIRECTORY_SEPARATOR, $path) . DIRECTORY_SEPARATOR . $name;
 
-        return preg_replace('@' . DIRECTORY_SEPARATOR . '+@', DIRECTORY_SEPARATOR, $filepath);
+        // To use backslash, it must be doubled ("\\\\" PHP string).
+        $separator = str_replace('\\', '\\\\', DIRECTORY_SEPARATOR);
+        return preg_replace('@(' . $separator . '|/)+@', $separator, $filepath);
     }
 
     /**
@@ -291,7 +293,9 @@ class moodle_content_writer implements content_writer {
         // Join the directory together with the name.
         $filepath = implode(DIRECTORY_SEPARATOR, $path);
 
-        return preg_replace('@' . DIRECTORY_SEPARATOR . '+@', DIRECTORY_SEPARATOR, $filepath);
+        // To use backslash, it must be doubled ("\\\\" PHP string).
+        $separator = str_replace('\\', '\\\\', DIRECTORY_SEPARATOR);
+        return preg_replace('@(' . $separator . '|/)+@', $separator, $filepath);
     }
 
     /**
@@ -314,6 +318,25 @@ class moodle_content_writer implements content_writer {
         return implode(DIRECTORY_SEPARATOR, $parts);
     }
 
+    /**
+     * Get a relative url to the directory of the exported files within a subcontext.
+     *
+     * @param string $component The name of the component that the files belong to.
+     * @param string $filearea The filearea within that component.
+     * @param string $itemid Which item those files belong to.
+     * @return string The url
+     */
+    protected function get_files_target_url($component, $filearea, $itemid) : string {
+        // We do not need to include the component because we organise things by context.
+        $parts = ['_files', $filearea];
+
+        if (!empty($itemid)) {
+            $parts[] = $itemid;
+        }
+
+        return implode('/', $parts);
+    }
+
     /**
      * Write the data to the specified path.
      *
index 0cd66f2..69f9667 100644 (file)
@@ -168,8 +168,23 @@ class manager {
      * @return contextlist_collection the collection of contextlist items for the respective components.
      */
     public function get_contexts_for_userid(int $userid) : contextlist_collection {
+        $progress = static::get_log_tracer();
+
+        $components = $this->get_component_list();
+        $a = (object) [
+            'total' => count($components),
+            'progress' => 0,
+            'component' => '',
+            'datetime' => userdate(time()),
+        ];
         $clcollection = new contextlist_collection($userid);
-        foreach ($this->get_component_list() as $component) {
+
+        $progress->output(get_string('trace:fetchcomponents', 'core_privacy', $a), 1);
+        foreach ($components as $component) {
+            $a->component = $component;
+            $a->progress++;
+            $a->datetime = userdate(time());
+            $progress->output(get_string('trace:processingcomponent', 'core_privacy', $a), 2);
             if ($this->component_implements($component, \core_privacy\local\request\core_user_data_provider::class)) {
                 $contextlist = $this->get_provider_classname($component)::get_contexts_for_userid($userid);
             } else {
@@ -187,6 +202,7 @@ class manager {
                 $clcollection->add_contextlist($contextlist);
             }
         }
+        $progress->output(get_string('trace:done', 'core_privacy'), 1);
 
         return $clcollection;
     }
@@ -202,13 +218,29 @@ class manager {
      * approved_contextlists' components is not a core_data_provider.
      */
     public function export_user_data(contextlist_collection $contextlistcollection) {
+        $progress = static::get_log_tracer();
+
+        $a = (object) [
+            'total' => count($contextlistcollection),
+            'progress' => 0,
+            'component' => '',
+            'datetime' => userdate(time()),
+        ];
+
         // Export for the various components/contexts.
+        $progress->output(get_string('trace:exportingapproved', 'core_privacy', $a), 1);
         foreach ($contextlistcollection as $approvedcontextlist) {
+
             if (!$approvedcontextlist instanceof \core_privacy\local\request\approved_contextlist) {
                 throw new \moodle_exception('Contextlist must be an approved_contextlist');
             }
 
             $component = $approvedcontextlist->get_component();
+            $a->component = $component;
+            $a->progress++;
+            $a->datetime = userdate(time());
+            $progress->output(get_string('trace:processingcomponent', 'core_privacy', $a), 2);
+
             // Core user data providers.
             if ($this->component_implements($component, \core_privacy\local\request\core_user_data_provider::class)) {
                 if (count($approvedcontextlist)) {
@@ -221,9 +253,19 @@ class manager {
                 local\request\helper::export_data_for_null_provider($approvedcontextlist);
             }
         }
+        $progress->output(get_string('trace:done', 'core_privacy'), 1);
 
         // Check each component for non contextlist items too.
-        foreach ($this->get_component_list() as $component) {
+        $components = $this->get_component_list();
+        $a->total = count($components);
+        $a->progress = 0;
+        $a->datetime = userdate(time());
+        $progress->output(get_string('trace:exportingrelated', 'core_privacy', $a), 1);
+        foreach ($components as $component) {
+            $a->component = $component;
+            $a->progress++;
+            $a->datetime = userdate(time());
+            $progress->output(get_string('trace:processingcomponent', 'core_privacy', $a), 2);
             // Core user preference providers.
             if ($this->component_implements($component, \core_privacy\local\request\user_preference_provider::class)) {
                 $this->get_provider_classname($component)::export_user_preferences($contextlistcollection->get_userid());
@@ -235,8 +277,13 @@ class manager {
                 $this->get_provider_classname($component)::export_context_data($contextlistcollection);
             }
         }
+        $progress->output(get_string('trace:done', 'core_privacy'), 1);
+
+        $progress->output(get_string('trace:finalisingexport', 'core_privacy'), 1);
+        $location = local\request\writer::with_context(\context_system::instance())->finalise_content();
 
-        return local\request\writer::with_context(\context_system::instance())->finalise_content();
+        $progress->output(get_string('trace:exportcomplete', 'core_privacy'), 1);
+        return $location;
     }
 
     /**
@@ -251,13 +298,29 @@ class manager {
      * for an approved_contextlist isn't a core provider.
      */
     public function delete_data_for_user(contextlist_collection $contextlistcollection) {
+        $progress = static::get_log_tracer();
+
+        $a = (object) [
+            'total' => count($contextlistcollection),
+            'progress' => 0,
+            'component' => '',
+            'datetime' => userdate(time()),
+        ];
+
         // Delete the data.
+        $progress->output(get_string('trace:deletingapproved', 'core_privacy', $a), 1);
         foreach ($contextlistcollection as $approvedcontextlist) {
             if (!$approvedcontextlist instanceof \core_privacy\local\request\approved_contextlist) {
                 throw new \moodle_exception('Contextlist must be an approved_contextlist');
             }
 
-            if ($this->component_is_core_provider($approvedcontextlist->get_component())) {
+            $component = $approvedcontextlist->get_component();
+            $a->component = $component;
+            $a->progress++;
+            $a->datetime = userdate(time());
+            $progress->output(get_string('trace:processingcomponent', 'core_privacy', $a), 2);
+
+            if ($this->component_is_core_provider($component)) {
                 if (count($approvedcontextlist)) {
                     // The component knows about data that it has.
                     // Have it delete its own data.
@@ -268,6 +331,7 @@ class manager {
             // Delete any shared user data it doesn't know about.
             local\request\helper::delete_data_for_user($approvedcontextlist);
         }
+        $progress->output(get_string('trace:done', 'core_privacy'), 1);
     }
 
     /**
@@ -276,7 +340,23 @@ class manager {
      * @param   context         $context   The specific context to delete data for.
      */
     public function delete_data_for_all_users_in_context(\context $context) {
+        $progress = static::get_log_tracer();
+
+        $components = $this->get_component_list();
+        $a = (object) [
+            'total' => count($components),
+            'progress' => 0,
+            'component' => '',
+            'datetime' => userdate(time()),
+        ];
+
+        $progress->output(get_string('trace:deletingcontext', 'core_privacy', $a), 1);
         foreach ($this->get_component_list() as $component) {
+            $a->component = $component;
+            $a->progress++;
+            $a->datetime = userdate(time());
+            $progress->output(get_string('trace:processingcomponent', 'core_privacy', $a), 2);
+
             if ($this->component_implements($component, \core_privacy\local\request\core_user_data_provider::class)) {
                 // This component knows about specific data that it owns.
                 // Have it delete all of that user data for the context.
@@ -286,6 +366,7 @@ class manager {
             // Delete any shared user data it doesn't know about.
             local\request\helper::delete_data_for_all_users_in_context($component, $context);
         }
+        $progress->output(get_string('trace:done', 'core_privacy'), 1);
     }
 
     /**
@@ -378,4 +459,19 @@ class manager {
 
         return null;
     }
+
+    /**
+     * Get the tracer used for logging.
+     *
+     * The text tracer is used except for unit tests.
+     *
+     * @return  \progress_trace
+     */
+    protected static function get_log_tracer() {
+        if (PHPUNIT_TEST) {
+            return new \null_progress_trace();
+        }
+
+        return new \text_progress_trace();
+    }
 }
index d46ae39..93c7026 100644 (file)
@@ -411,7 +411,10 @@ class content_writer implements \core_privacy\local\request\content_writer {
      */
     public function export_file(array $subcontext, \stored_file $file) : \core_privacy\local\request\content_writer  {
         if (!$file->is_directory()) {
-            $filepath = explode(DIRECTORY_SEPARATOR, $file->get_filepath());
+            $filepath = $file->get_filepath();
+            // Directory separator in the stored_file class should always be '/'. The following line is just a fail safe.
+            $filepath = str_replace(DIRECTORY_SEPARATOR, '/', $filepath);
+            $filepath = explode('/', $filepath);
             $filepath[] = $file->get_filename();
             $filepath = array_filter($filepath);
             $filepath = implode('/', $filepath);
index 7e81a3d..f4ccc3b 100644 (file)
@@ -1162,12 +1162,18 @@ class moodle_content_writer_test extends advanced_testcase {
         if (null === $subcontext) {
             $rcm = $rc->getMethod('get_context_path');
             $rcm->setAccessible(true);
-            return $rcm->invoke($writer);
+            $path = $rcm->invoke($writer);
         } else {
             $rcm = $rc->getMethod('get_path');
             $rcm->setAccessible(true);
-            return $rcm->invoke($writer, $subcontext, $name);
+            $path = $rcm->invoke($writer, $subcontext, $name);
         }
+
+        // PHPUnit uses mikey179/vfsStream which is a stream wrapper for a virtual file system that uses '/'
+        // as the directory separator.
+        $path = str_replace(DIRECTORY_SEPARATOR, '/', $path);
+
+        return $path;
     }
 
     /**