mirror_php/moodle
1
0
Fork 0
mirror of https://github.com/moodle/moodle.git synced 2025-04-13 04:22:07 +02:00
Code Issues Projects Releases Wiki Activity

MDL-62003 privacy: Consistently export files into _files folder

Browse Source 
There were two major issues with the previous implementation:

* The exported folder name was localised so it was "Files" or "Soubory"
  etc depending on the current language. Yet URLs referring to the files
  in that folder were always rewritten with hard-coded English "files".
* Files from all fileareas and itemids were all exported to a single
  target directory. So if there were two files with the same name being
  exported from multiple areas (such as submission_content and
  submission_attachment in the workshop module), one would overwrite
  another.

The patch addresses these issues as follows:

* To unify the folder name and also to minimise the risk of conflict
  with a subcontext folder, we now always export stored files under
  "_files" folder.
* Under that folder, there is a subdirectory with the area name and then
  eventually another subdirectory with non-zero itemid. And there
  finally the stored_file is exported to under its own file path.
This commit is contained in:
David Mudrák 2018-04-18 13:12:47 +02:00
parent 20bf0c45ff
commit 3ecbf154db
3 changed files with 102 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
privacy/classes/local/request/moodle_content_writer.php
View File

@ -159,7 +159,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@@/', 'files/', $text);
return str_replace('@@PLUGINFILE@@/', $this->get_files_target_path($component, $filearea, $itemid).'/', $text);
}
/**
@ -188,11 +188,12 @@ class moodle_content_writer implements content_writer {
*/
public function export_file(array $subcontext, \stored_file $file) : content_writer {
if (!$file->is_directory()) {
$subcontextextra = [
get_string('files'),
$file->get_filepath(),
];
$path = $this->get_path(array_merge($subcontext, $subcontextextra), $file->get_filename());
$pathitems = array_merge(
$subcontext,
[$this->get_files_target_path($file->get_component(), $file->get_filearea(), $file->get_itemid())],
[$file->get_filepath()]
);
$path = $this->get_path($pathitems, $file->get_filename());
check_dir_exists(dirname($path), true, true);
$this->files[$path] = $file;
}
@ -285,6 +286,26 @@ class moodle_content_writer implements content_writer {
return preg_replace('@' . DIRECTORY_SEPARATOR . '+@', DIRECTORY_SEPARATOR, $filepath);
}
/**
* Get a path within a subcontext where exported files should be written to.
*
* @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 path
*/
protected function get_files_target_path($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(DIRECTORY_SEPARATOR, $parts);
}
/**
* Write the data to the specified path.
*

6
privacy/classes/tests/request/content_writer.php
View File

@ -385,6 +385,10 @@ class content_writer implements \core_privacy\local\request\content_writer {
/**
* Prepare a text area by processing pluginfile URLs within it.
*
* Note that this method does not implement the pluginfile URL rewriting. Such a job tightly depends on how the
* actual writer exports files so it can be reliably tested only in real writers such as
* {@link core_privacy\local\request\moodle_content_writer}.
*
* @param array $subcontext The location within the current context that this data belongs.
* @param string $component The name of the component that the files belong to.
* @param string $filearea The filearea within that component.
@ -393,7 +397,7 @@ class content_writer implements \core_privacy\local\request\content_writer {
* @return string The processed string
*/
public function rewrite_pluginfile_urls(array $subcontext, $component, $filearea, $itemid, $text) : string {
return str_replace('@@PLUGINFILE@@/', 'files/', $text);
return $text;
}
/**

79
privacy/tests/moodle_content_writer_test.php
View File

@ -340,7 +340,7 @@ class moodle_content_writer_test extends advanced_testcase {
'component' => 'core_privacy',
'filearea' => 'tests',
'itemid' => 0,
'path' => '/',
'path' => '/sub/',
'name' => 'b.txt',
'content' => 'Test file 1',
];
@ -389,7 +389,7 @@ class moodle_content_writer_test extends advanced_testcase {
'filename' => $file->name,
];
$file->namepath = $file->path . $file->name;
$file->namepath = '/' . $file->filearea . '/' . ($file->itemid ?: '') . $file->path . $file->name;
$file->storedfile = $fs->create_file_from_string($record, $file->content);
}
@ -401,14 +401,14 @@ class moodle_content_writer_test extends advanced_testcase {
$firstfiles = array_slice($files, 0, 2);
foreach ($firstfiles as $file) {
$contextpath = $this->get_context_path($context, [get_string('files')], $file->namepath);
$contextpath = $this->get_context_path($context, ['_files'], $file->namepath);
$this->assertTrue($fileroot->hasChild($contextpath));
$this->assertEquals($file->content, $fileroot->getChild($contextpath)->getContent());
}
$otherfiles = array_slice($files, 2);
foreach ($otherfiles as $file) {
$contextpath = $this->get_context_path($context, [get_string('files')], $file->namepath);
$contextpath = $this->get_context_path($context, ['_files'], $file->namepath);
$this->assertFalse($fileroot->hasChild($contextpath));
}
}
@ -421,16 +421,16 @@ class moodle_content_writer_test extends advanced_testcase {
* @param string $filename File name
* @param string $content Content
*/
public function test_export_file($filepath, $filename, $content) {
public function test_export_file($filearea, $itemid, $filepath, $filename, $content) {
$this->resetAfterTest();
$context = \context_system::instance();
$filenamepath = $filepath . $filename;
$filenamepath = '/' . $filearea . '/' . ($itemid ?: '') . $filepath . $filename;
$filerecord = array(
'contextid' => $context->id,
'component' => 'core_privacy',
'filearea' => 'tests',
'itemid' => 0,
'filearea' => $filearea,
'itemid' => $itemid,
'filepath' => $filepath,
'filename' => $filename,
);
@ -444,7 +444,7 @@ class moodle_content_writer_test extends advanced_testcase {
$fileroot = $this->fetch_exported_content($writer);
$contextpath = $this->get_context_path($context, [get_string('files')], $filenamepath);
$contextpath = $this->get_context_path($context, ['_files'], $filenamepath);
$this->assertTrue($fileroot->hasChild($contextpath));
$this->assertEquals($content, $fileroot->getChild($contextpath)->getContent());
}
@ -457,36 +457,50 @@ class moodle_content_writer_test extends advanced_testcase {
public function export_file_provider() {
return [
'basic' => [
'intro',
0,
'/',
'testfile.txt',
'An example file content',
],
'longpath' => [
'attachments',
'12',
'/path/within/a/path/within/a/path/',
'testfile.txt',
'An example file content',
],
'pathwithspaces' => [
'intro',
0,
'/path with/some spaces/',
'testfile.txt',
'An example file content',
],
'filewithspaces' => [
'submission_attachments',
1,
'/path with/some spaces/',
'test file.txt',
'An example file content',
],
'image' => [
'intro',
0,
'/',
'logo.png',
file_get_contents(__DIR__ . '/fixtures/logo.png'),
],
'UTF8' => [
'submission_content',
2,
'/Žluťoučký/',
'koníček.txt',
'koníček',
],
'EUC-JP' => [
'intro',
0,
'/言語設定/',
'言語設定.txt',
'言語設定',
@ -835,4 +849,51 @@ class moodle_content_writer_test extends advanced_testcase {
return $rcm->invoke($writer, $subcontext, $name);
}
}
/**
* Test correct rewriting of @@PLUGINFILE@@ in the exported contents.
*
* @dataProvider rewrite_pluginfile_urls_provider
* @param string $filearea The filearea within that component.
* @param int $itemid Which item those files belong to.
* @param string $input Raw text as stored in the database.
* @param string $expectedoutput Expected output of URL rewriting.
*/
public function test_rewrite_pluginfile_urls($filearea, $itemid, $input, $expectedoutput) {
$writer = $this->get_writer_instance();
$writer->set_context(\context_system::instance());
$realoutput = $writer->rewrite_pluginfile_urls([], 'core_test', $filearea, $itemid, $input);
$this->assertEquals($expectedoutput, $realoutput);
}
/**
* Provides testable sample data for {@link self::test_rewrite_pluginfile_urls()}.
*
* @return array
*/
public function rewrite_pluginfile_urls_provider() {
return [
'zeroitemid' => [
'intro',
0,
'<p><img src="@@PLUGINFILE@@/hello.gif" /></p>',
'<p><img src="_files/intro/hello.gif" /></p>',
],
'nonzeroitemid' => [
'submission_content',
34,
'<p><img src="@@PLUGINFILE@@/first.png" alt="First" /></p>',
'<p><img src="_files/submission_content/34/first.png" alt="First" /></p>',
],
'withfilepath' => [
'post_content',
9889,
'<a href="@@PLUGINFILE@@/embedded/docs/muhehe.exe">Click here!</a>',
'<a href="_files/post_content/9889/embedded/docs/muhehe.exe">Click here!</a>',
],
];
}
}