From 517baef4794e76488a629ab2037b2d06cd3f22d7 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Fri, 13 Apr 2018 21:35:07 +0800 Subject: [PATCH] MDL-61974 privacy: Rewrite the content writer storage --- .../classes/tests/request/content_writer.php | 298 +++++++----------- privacy/tests/tests_content_writer_test.php | 84 +++++ 2 files changed, 202 insertions(+), 180 deletions(-) diff --git a/privacy/classes/tests/request/content_writer.php b/privacy/classes/tests/request/content_writer.php index ef5fa93aef9a6..90550af9d22ff 100644 --- a/privacy/classes/tests/request/content_writer.php +++ b/privacy/classes/tests/request/content_writer.php @@ -39,32 +39,32 @@ class content_writer implements \core_privacy\local\request\content_writer { /** * @var \context The context currently being exported. */ - protected $context = null; + protected $context; /** - * @var array The collection of metadata which has been exported. + * @var \stdClass The collection of metadata which has been exported. */ - protected $metadata = []; + protected $metadata; /** - * @var array The data which has been exported. + * @var \stdClass The data which has been exported. */ - protected $data = []; + protected $data; /** - * @var array The related data which has been exported. + * @var \stdClass The related data which has been exported. */ - protected $relateddata = []; + protected $relateddata; /** - * @var array The list of stored files which have been exported. + * @var \stdClass The list of stored files which have been exported. */ - protected $files = []; + protected $files; /** - * @var array The custom files which have been exported. + * @var \stdClass The custom files which have been exported. */ - protected $customfiles = []; + protected $customfiles; /** * @var array The site-wide user preferences which have been exported. @@ -75,11 +75,11 @@ class content_writer implements \core_privacy\local\request\content_writer { * Whether any data has been exported at all within the current context. */ public function has_any_data() { - $hasdata = !empty($this->data[$this->context->id]); - $hasrelateddata = !empty($this->relateddata[$this->context->id]); - $hasmetadata = !empty($this->metadata[$this->context->id]); - $hasfiles = !empty($this->files[$this->context->id]); - $hascustomfiles = !empty($this->customfiles[$this->context->id]); + $hasdata = !empty($this->data->{$this->context->id}); + $hasrelateddata = !empty($this->relateddata->{$this->context->id}); + $hasmetadata = !empty($this->metadata->{$this->context->id}); + $hasfiles = !empty($this->files->{$this->context->id}); + $hascustomfiles = !empty($this->customfiles->{$this->context->id}); $hasuserprefs = !empty($this->userprefs); return $hasdata || $hasrelateddata || $hasmetadata || $hasfiles || $hascustomfiles || $hasuserprefs; @@ -92,6 +92,11 @@ public function has_any_data() { * @param \core_privacy\local\request\writer $writer The writer factory. */ public function __construct(\core_privacy\local\request\writer $writer) { + $this->data = (object) []; + $this->relateddata = (object) []; + $this->metadata = (object) []; + $this->files = (object) []; + $this->customfiles = (object) []; } /** @@ -102,24 +107,39 @@ public function __construct(\core_privacy\local\request\writer $writer) { public function set_context(\context $context) { $this->context = $context; - if (empty($this->data[$this->context->id])) { - $this->data[$this->context->id] = []; + if (isset($this->data->{$this->context->id}) && empty((array) $this->data->{$this->context->id})) { + $this->data->{$this->context->id} = (object) [ + 'children' => (object) [], + 'data' => [], + ]; } - if (empty($this->relateddata[$this->context->id])) { - $this->relateddata[$this->context->id] = []; + if (isset($this->relateddata->{$this->context->id}) && empty((array) $this->relateddata->{$this->context->id})) { + $this->relateddata->{$this->context->id} = (object) [ + 'children' => (object) [], + 'data' => [], + ]; } - if (empty($this->metadata[$this->context->id])) { - $this->metadata[$this->context->id] = []; + if (isset($this->metadata->{$this->context->id}) && empty((array) $this->metadata->{$this->context->id})) { + $this->metadata->{$this->context->id} = (object) [ + 'children' => (object) [], + 'data' => [], + ]; } - if (empty($this->files[$this->context->id])) { - $this->files[$this->context->id] = []; + if (isset($this->files->{$this->context->id}) && empty((array) $this->files->{$this->context->id})) { + $this->files->{$this->context->id} = (object) [ + 'children' => (object) [], + 'data' => [], + ]; } - if (empty($this->customfiles[$this->context->id])) { - $this->customfiles[$this->context->id] = []; + if (isset($this->customfiles->{$this->context->id}) && empty((array) $this->customfiles->{$this->context->id})) { + $this->customfiles->{$this->context->id} = (object) [ + 'children' => (object) [], + 'data' => [], + ]; } return $this; @@ -141,21 +161,8 @@ public function get_current_context() { * @param \stdClass $data The data to be exported */ public function export_data(array $subcontext, \stdClass $data) { - $finalcontent = [ - 'children' => [], - 'data' => $data, - ]; - - while ($pathtail = array_pop($subcontext)) { - $finalcontent = [ - 'children' => [ - $pathtail => $finalcontent, - ], - 'data' => [], - ]; - } - - $this->data[$this->context->id] = array_replace_recursive($this->data[$this->context->id], $finalcontent); + $current = $this->fetch_root($this->data, $subcontext); + $current->data = $data; return $this; } @@ -167,18 +174,7 @@ public function export_data(array $subcontext, \stdClass $data) { * @return array The metadata as a series of keys to value + descrition objects. */ public function get_data(array $subcontext = []) { - $basepath = $this->data[$this->context->id]; - while ($subpath = array_shift($subcontext)) { - if (isset($basepath['children']) && isset($basepath['children'][$subpath])) { - $basepath = $basepath['children'][$subpath]; - } - } - - if (isset($basepath['data'])) { - return $basepath['data']; - } else { - return []; - } + return $this->fetch_data_root($this->data, $subcontext); } /** @@ -192,31 +188,12 @@ public function get_data(array $subcontext = []) { * @param string $description The description of the value. * @return $this */ - public function export_metadata(array $subcontext, - $key, - $value, - $description - ) { - $finalcontent = [ - 'children' => [], - 'metadata' => [ - $key => (object) [ - 'value' => $value, - 'description' => $description, - ], - ], - ]; - - while ($pathtail = array_pop($subcontext)) { - $finalcontent = [ - 'children' => [ - $pathtail => $finalcontent, - ], - 'metadata' => [], + public function export_metadata(array $subcontext, $key, $value, $description) { + $current = $this->fetch_root($this->metadata, $subcontext); + $current->data[$key] = (object) [ + 'value' => $value, + 'description' => $description, ]; - } - - $this->metadata[$this->context->id] = array_replace_recursive($this->metadata[$this->context->id], $finalcontent); return $this; } @@ -228,18 +205,7 @@ public function export_metadata(array $subcontext, * @return array The metadata as a series of keys to value + descrition objects. */ public function get_all_metadata(array $subcontext = []) { - $basepath = $this->metadata[$this->context->id]; - while ($subpath = array_shift($subcontext)) { - if (isset($basepath['children']) && isset($basepath['children'][$subpath])) { - $basepath = $basepath['children'][$subpath]; - } - } - - if (isset($basepath['metadata'])) { - return $basepath['metadata']; - } else { - return []; - } + return $this->fetch_data_root($this->metadata, $subcontext); } /** @@ -274,23 +240,8 @@ public function get_metadata(array $subcontext = [], $key, $valueonly = true) { * @param \stdClass $data The related data to export. */ public function export_related_data(array $subcontext, $name, $data) { - $finalcontent = [ - 'children' => [], - 'data' => [ - $name => $data, - ], - ]; - - while ($pathtail = array_pop($subcontext)) { - $finalcontent = [ - 'children' => [ - $pathtail => $finalcontent, - ], - 'data' => [], - ]; - } - - $this->relateddata[$this->context->id] = array_replace_recursive($this->relateddata[$this->context->id], $finalcontent); + $current = $this->fetch_root($this->relateddata, $subcontext); + $current->data[$name] = $data; return $this; } @@ -303,26 +254,17 @@ public function export_related_data(array $subcontext, $name, $data) { * @return array The metadata as a series of keys to value + descrition objects. */ public function get_related_data(array $subcontext = [], $filename = null) { - $basepath = $this->relateddata[$this->context->id]; - while ($subpath = array_shift($subcontext)) { - if (isset($basepath['children']) && isset($basepath['children'][$subpath])) { - $basepath = $basepath['children'][$subpath]; - } - } + $current = $this->fetch_data_root($this->relateddata, $subcontext); - if (isset($basepath['data'])) { - $data = $basepath['data']; - if (null !== $filename) { - if (isset($data[$filename])) { - return $data[$filename]; - } - return null; - } + if (null === $filename) { + return $current; + } - return $data; + if (isset($current[$filename])) { + return $current[$filename]; } - return null; + return []; } /** @@ -335,22 +277,8 @@ public function get_related_data(array $subcontext = [], $filename = null) { public function export_custom_file(array $subcontext, $filename, $filecontent) { $filename = clean_param($filename, PARAM_FILE); - $finalcontent = [ - 'children' => [], - 'files' => [ - $filename => $filecontent, - ], - ]; - while ($pathtail = array_pop($subcontext)) { - $finalcontent = [ - 'children' => [ - $pathtail => $finalcontent, - ], - 'files' => [], - ]; - } - - $this->customfiles[$this->context->id] = array_replace_recursive($this->customfiles[$this->context->id], $finalcontent); + $current = $this->fetch_root($this->customfiles, $subcontext); + $current->data[$filename] = $filecontent; return $this; } @@ -363,22 +291,16 @@ public function export_custom_file(array $subcontext, $filename, $filecontent) { * @return string The content of the file. */ public function get_custom_file(array $subcontext = [], $filename = null) { - $basepath = $this->customfiles[$this->context->id]; - while ($subpath = array_shift($subcontext)) { - if (isset($basepath['children']) && isset($basepath['children'][$subpath])) { - $basepath = $basepath['children'][$subpath]; - } + $current = $this->fetch_data_root($this->customfiles, $subcontext); + + if (null === $filename) { + return $current; } - if (isset($basepath['files'])) { - if (null !== $filename) { - if (isset($basepath['files'][$filename])) { - return $basepath['files'][$filename]; - } - return null; - } - return $basepath['files']; + if (isset($current[$filename])) { + return $current[$filename]; } + return null; } @@ -427,23 +349,8 @@ public function export_file(array $subcontext, \stored_file $file) { $filepath = array_filter($filepath); $filepath = implode('/', $filepath); - $finalcontent = [ - 'children' => [], - 'files' => [ - $filepath => $file, - ], - ]; - - while ($pathtail = array_pop($subcontext)) { - $finalcontent = [ - 'children' => [ - $pathtail => $finalcontent, - ], - 'files' => [], - ]; - } - - $this->files[$this->context->id] = array_replace_recursive($this->files[$this->context->id], $finalcontent); + $current = $this->fetch_root($this->files, $subcontext); + $current->data[$filepath] = $file; } return $this; @@ -456,18 +363,7 @@ public function export_file(array $subcontext, \stored_file $file) { * @return \stored_file[] The list of stored_files in this context + subcontext. */ public function get_files(array $subcontext = []) { - $basepath = $this->files[$this->context->id]; - while ($subpath = array_shift($subcontext)) { - if (isset($basepath['children']) && isset($basepath['children'][$subpath])) { - $basepath = $basepath['children'][$subpath]; - } - } - - if (isset($basepath['files'])) { - return $basepath['files']; - } - - return []; + return $this->fetch_data_root($this->files, $subcontext); } /** @@ -519,4 +415,46 @@ public function get_user_preferences($component) { public function finalise_content() { return 'mock_path'; } + + /** + * Fetch the entire root record at the specified location type, creating it if required. + * + * @param \stdClass $base The base to use - e.g. $this->data + * @param array $subcontext The subcontext to fetch + * @return array + */ + protected function fetch_root($base, $subcontext) { + if (!isset($base->{$this->context->id})) { + $base->{$this->context->id} = (object) [ + 'children' => (object) [], + 'data' => [], + ]; + } + + $current = $base->{$this->context->id}; + foreach ($subcontext as $node) { + if (!isset($current->children->{$node})) { + $current->children->{$node} = (object) [ + 'children' => (object) [], + 'data' => [], + ]; + } + $current = $current->children->{$node}; + } + + return $current; + } + + /** + * Fetch the data region of the specified root. + * + * @param \stdClass $base The base to use - e.g. $this->data + * @param array $subcontext The subcontext to fetch + * @return array + */ + protected function fetch_data_root($base, $subcontext) { + $root = $this->fetch_root($base, $subcontext); + + return $root->data; + } } diff --git a/privacy/tests/tests_content_writer_test.php b/privacy/tests/tests_content_writer_test.php index 1f815c11434b8..7a70e59284691 100644 --- a/privacy/tests/tests_content_writer_test.php +++ b/privacy/tests/tests_content_writer_test.php @@ -93,6 +93,22 @@ public function test_export_data_no_context_clash() { $this->assertSame($datab, $data); } + /** + * Test export and recover with children. + */ + public function test_get_data_with_children() { + $writer = $this->get_writer_instance(); + $context = \context_system::instance(); + + $writer->set_context($context) + ->export_data(['a'], (object) ['parent' => true]) + ->export_data(['a', 'b'], (object) ['parent' => false]); + + $this->assertTrue($writer->get_data(['a'])->parent); + $this->assertFalse($writer->get_data(['a', 'b'])->parent); + $this->assertEquals([], $writer->get_data(['a', 'b', 'c'])); + } + /** * It should be possible to store and retrieve metadata. */ @@ -167,6 +183,21 @@ public function test_export_metadata_no_context_clash() { $this->assertEquals('value2', $writer->get_metadata(['metadata'], 'somekey', true)); } + /** + * Test export and recover with children. + */ + public function test_get_metadata_with_children() { + $writer = $this->get_writer_instance(); + $context = \context_system::instance(); + + $writer->set_context($context) + ->export_metadata(['a'], 'abc', 'ABC', 'A, B, C') + ->export_metadata(['a', 'b'], 'def', 'DEF', 'D, E, F'); + + $this->assertEquals('ABC', $writer->get_metadata(['a'], 'abc')); + $this->assertEquals('DEF', $writer->get_metadata(['a', 'b'], 'def')); + } + /** * It should be possible to export files in the files and children contexts. */ @@ -217,6 +248,29 @@ public function test_export_file_no_context_clash() { $this->assertEquals($fileb, $files['foo/foo.txt']); } + /** + * Test export and recover with children. + */ + public function test_get_file_with_children() { + $writer = $this->get_writer_instance(); + $context = \context_system::instance(); + + $filea = $this->get_stored_file('/foo/', 'foo.txt'); + $fileb = $this->get_stored_file('/foo/', 'foo.txt'); + + $writer->set_context($context) + ->export_file(['a'], $filea) + ->export_file(['a', 'b'], $fileb); + + $files = $writer->get_files(['a']); + $this->assertCount(1, $files); + $this->assertEquals($filea, $files['foo/foo.txt']); + + $files = $writer->get_files(['a', 'b']); + $this->assertCount(1, $files); + $this->assertEquals($fileb, $files['foo/foo.txt']); + } + /** * It should be possible to export related data in the files and children contexts. */ @@ -269,6 +323,21 @@ public function test_export_related_data_no_context_clash() { $this->assertEquals('data2', $data['file']); } + /** + * Test export and recover with children. + */ + public function test_get_related_data_with_children() { + $writer = $this->get_writer_instance(); + $context = \context_system::instance(); + + $writer->set_context($context) + ->export_related_data(['a'], 'abc', 'ABC') + ->export_related_data(['a', 'b'], 'def', 'DEF'); + + $this->assertEquals('ABC', $writer->get_related_data(['a'], 'abc')); + $this->assertEquals('DEF', $writer->get_related_data(['a', 'b'], 'def')); + } + /** * It should be possible to export related files in the files and children contexts. */ @@ -320,6 +389,21 @@ public function test_export_custom_file_no_context_clash() { $this->assertEquals('Content 2', $files['file.txt']); } + /** + * Test export and recover with children. + */ + public function test_get_custom_file_with_children() { + $writer = $this->get_writer_instance(); + $context = \context_system::instance(); + + $writer->set_context($context) + ->export_custom_file(['a'], 'file.txt', 'ABC') + ->export_custom_file(['a', 'b'], 'file.txt', 'DEF'); + + $this->assertEquals('ABC', $writer->get_custom_file(['a'], 'file.txt')); + $this->assertEquals('DEF', $writer->get_custom_file(['a', 'b'], 'file.txt')); + } + /** * Get a fresh content writer. *