From 6e8143299f0c3fbf89fb91f4b5b55fe90e52a5fc Mon Sep 17 00:00:00 2001 From: Matt Friedman Date: Thu, 17 Sep 2026 11:17:02 -0700 Subject: [PATCH] refactor(entities)!: move persistence to operators Keep hydration side-effect free so stored values are not transformed as new input. Use explicit factories and update only dirty fields. BREAKING CHANGE: Page and rule entities no longer expose load(), insert(), or save(). Their non-shared service IDs were also removed. Use operator create, get, add, and save methods instead. Refinements to refactoring --- acp/pages_module.php | 1 + config/services.yml | 11 +- controller/admin_controller.php | 91 +++++--- controller/main_controller.php | 29 +-- entity/factory.php | 78 +++++++ entity/page.php | 220 +++++++----------- entity/page_interface.php | 89 +++---- exception/base.php | 12 - operators/page.php | 100 ++++++-- operators/page_interface.php | 35 ++- tests/controller/admin_controller_test.php | 87 +++++-- .../controller/page_main_controller_test.php | 36 +-- tests/entity/page_entity_description_test.php | 7 + tests/entity/page_entity_import_test.php | 72 ++++-- tests/entity/page_entity_insert_test.php | 116 --------- tests/entity/page_entity_load_test.php | 162 ------------- tests/entity/page_entity_route_test.php | 4 +- tests/entity/page_entity_save_test.php | 119 ---------- tests/entity/page_entity_title_test.php | 7 + tests/event/show_page_links_test.php | 4 +- .../operators/page_operator_add_page_test.php | 7 +- tests/operators/page_operator_base.php | 27 +-- .../page_operator_delete_page_test.php | 5 +- .../page_operator_save_page_test.php | 77 ++++++ 24 files changed, 629 insertions(+), 767 deletions(-) create mode 100644 entity/factory.php delete mode 100644 tests/entity/page_entity_insert_test.php delete mode 100644 tests/entity/page_entity_load_test.php delete mode 100644 tests/entity/page_entity_save_test.php create mode 100644 tests/operators/page_operator_save_page_test.php diff --git a/acp/pages_module.php b/acp/pages_module.php index 7849473..35d4275 100644 --- a/acp/pages_module.php +++ b/acp/pages_module.php @@ -37,6 +37,7 @@ public function main($id, $mode) $lang->add_lang('pages_acp', 'phpbb/pages'); // Get an instance of the admin controller + /** @var \phpbb\pages\controller\admin_controller $admin_controller */ $admin_controller = $phpbb_container->get('phpbb.pages.admin.controller'); // Requests diff --git a/config/services.yml b/config/services.yml index b80f1e1..381fac7 100644 --- a/config/services.yml +++ b/config/services.yml @@ -34,7 +34,7 @@ services: - '@request' - '@template' - '@user' - - '@service_container' + - '@pagination' - '@dispatcher' - '%core.root_path%' - '%core.php_ext%' @@ -50,7 +50,7 @@ services: class: phpbb\pages\controller\main_controller arguments: - '@auth' - - '@service_container' + - '@phpbb.pages.operator' - '@controller.helper' - '@language' - '@template' @@ -110,9 +110,8 @@ services: - [configure_smilies_path, ['@config', '@path_helper']] - [configure_user, ['@user', '@config', '@auth']] - phpbb.pages.entity: - class: phpbb\pages\entity\page - shared: false # service MUST not be shared for this to work! + phpbb.pages.entity_factory: + class: phpbb\pages\entity\factory arguments: - '@dbal.conn' - '@config' @@ -125,7 +124,7 @@ services: class: phpbb\pages\operators\page arguments: - '@cache.driver' - - '@service_container' + - '@phpbb.pages.entity_factory' - '@dbal.conn' - '@ext.manager' - '@user' diff --git a/controller/admin_controller.php b/controller/admin_controller.php index 06f57e4..e8da41e 100644 --- a/controller/admin_controller.php +++ b/controller/admin_controller.php @@ -10,8 +10,6 @@ namespace phpbb\pages\controller; -use Symfony\Component\DependencyInjection\ContainerInterface; - /** * Admin controller */ @@ -44,8 +42,8 @@ class admin_controller implements admin_interface /** @var \phpbb\user */ protected $user; - /** @var ContainerInterface */ - protected $container; + /** @var \phpbb\pagination */ + protected $pagination; /** @var \phpbb\event\dispatcher_interface */ protected $dispatcher; @@ -71,13 +69,13 @@ class admin_controller implements admin_interface * @param \phpbb\request\request $request Request object * @param \phpbb\template\template $template Template object * @param \phpbb\user $user User object - * @param ContainerInterface $phpbb_container Service container interface + * @param \phpbb\pagination $pagination Pagination service * @param \phpbb\event\dispatcher_interface $phpbb_dispatcher Event dispatcher * @param string $root_path phpBB root path * @param string $php_ext phpEx * @access public */ - public function __construct(\phpbb\cache\driver\driver_interface $cache, \phpbb\pages\routing\route_cache $route_cache, \phpbb\controller\helper $helper, \phpbb\language\language $lang, \phpbb\log\log $log, \phpbb\pages\operators\page $page_operator, \phpbb\request\request $request, \phpbb\template\template $template, \phpbb\user $user, ContainerInterface $phpbb_container, \phpbb\event\dispatcher_interface $phpbb_dispatcher, $root_path, $php_ext) + public function __construct(\phpbb\cache\driver\driver_interface $cache, \phpbb\pages\routing\route_cache $route_cache, \phpbb\controller\helper $helper, \phpbb\language\language $lang, \phpbb\log\log $log, \phpbb\pages\operators\page $page_operator, \phpbb\request\request $request, \phpbb\template\template $template, \phpbb\user $user, \phpbb\pagination $pagination, \phpbb\event\dispatcher_interface $phpbb_dispatcher, $root_path, $php_ext) { $this->cache = $cache; $this->route_cache = $route_cache; @@ -88,7 +86,7 @@ public function __construct(\phpbb\cache\driver\driver_interface $cache, \phpbb\ $this->request = $request; $this->template = $template; $this->user = $user; - $this->container = $phpbb_container; + $this->pagination = $pagination; $this->dispatcher = $phpbb_dispatcher; $this->root_path = $root_path; $this->php_ext = $php_ext; @@ -104,14 +102,20 @@ public function display_pages() { add_form_key('phpbb_pages_purge_icons'); - /* @var $pagination \phpbb\pagination */ - $pagination = $this->container->get('pagination'); $start = $this->request->variable('start', 0); $total = $this->page_operator->get_total_pages(); $limit = 25; // Grab all the pages from the db - $entities = $this->page_operator->get_pages($limit, $start); + try + { + $entities = $this->page_operator->get_pages($limit, $start); + } + catch (\phpbb\pages\exception\base $e) + { + $this->display_page_error($e); + return; + } // Process each page entity for display /* @var $entity \phpbb\pages\entity\page */ @@ -134,7 +138,7 @@ public function display_pages() )); } - $pagination->generate_template_pagination($this->u_action, 'pagination', 'start', $total, $limit, $start); + $this->pagination->generate_template_pagination($this->u_action, 'pagination', 'start', $total, $limit, $start); // Set output vars for display in the template $this->template->assign_vars(array( @@ -148,16 +152,23 @@ public function display_pages() * * @return void * @access public - * @throws \phpbb\pages\exception\out_of_bounds */ public function add_page() { - // Initiate a page entity - /* @var $entity \phpbb\pages\entity\page */ - $entity = $this->container->get('phpbb.pages.entity'); + try + { + // Initiate a page entity + /* @var $entity \phpbb\pages\entity\page */ + $entity = $this->page_operator->create_page(); - // Process the new page - $this->add_edit_page_data($entity); + // Process the new page + $this->add_edit_page_data($entity); + } + catch (\phpbb\pages\exception\base $e) + { + $this->display_page_error($e); + return; + } // Set output vars for display in the template $this->template->assign_vars(array( @@ -172,16 +183,23 @@ public function add_page() * @param int $page_id The page identifier to edit * @return void * @access public - * @throws \phpbb\pages\exception\out_of_bounds */ public function edit_page($page_id) { - // Initiate and load the page entity - /* @var $entity \phpbb\pages\entity\page */ - $entity = $this->container->get('phpbb.pages.entity')->load($page_id); + try + { + // Initiate and load the page entity + /* @var $entity \phpbb\pages\entity\page */ + $entity = $this->page_operator->get_page($page_id); - // Process the edited page - $this->add_edit_page_data($entity); + // Process the edited page + $this->add_edit_page_data($entity); + } + catch (\phpbb\pages\exception\base $e) + { + $this->display_page_error($e); + return; + } // Set output vars for display in the template $this->template->assign_vars(array( @@ -197,7 +215,7 @@ public function edit_page($page_id) * @param \phpbb\pages\entity\page_interface $entity The page entity object * @return void * @access protected - * @throws \phpbb\pages\exception\out_of_bounds + * @throws \phpbb\pages\exception\base If persistence or hydration fails */ protected function add_edit_page_data($entity) { @@ -305,7 +323,7 @@ protected function add_edit_page_data($entity) if ($entity->get_id()) { // Save the edited page entity to the database - $entity->save(); + $entity = $this->page_operator->save_page($entity); // Save the page link location data $this->page_operator->insert_page_links($entity->get_id(), $data['page_links']); @@ -406,12 +424,11 @@ protected function add_edit_page_data($entity) */ public function delete_page($page_id) { - // Initiate and load the page entity - /* @var $entity \phpbb\pages\entity\page */ - $entity = $this->container->get('phpbb.pages.entity')->load($page_id); - try { + // Load the page before deleting it so its title remains available for logging. + $entity = $this->page_operator->get_page($page_id); + // Delete the page $this->page_operator->delete_page($page_id); } @@ -419,6 +436,7 @@ public function delete_page($page_id) { // Display an error message if delete failed trigger_error($this->lang->lang('ACP_PAGES_DELETE_ERRORED') . adm_back_link($this->u_action), E_USER_WARNING); + return; } // Log the action @@ -479,7 +497,7 @@ protected function create_page_template_options($current) $page_templates = $this->page_operator->get_page_templates(); // Clean up template names and simplify the array - $page_templates = array_map(function ($value) { + $page_templates = array_map(static function ($value) { return basename($value); }, array_keys($page_templates)); @@ -491,7 +509,7 @@ protected function create_page_template_options($current) { $this->template->assign_block_vars('page_template_options', array( 'VALUE' => $page_template, - 'S_SELECTED' => $page_template == $current, + 'S_SELECTED' => $page_template === $current, )); } } @@ -530,4 +548,15 @@ protected function create_page_link_options($page_id = 0, $current = array()) )); } } + + /** + * Display a translated entity or operator failure in the ACP. + * + * @param \phpbb\pages\exception\base $exception + * @return void + */ + protected function display_page_error(\phpbb\pages\exception\base $exception) + { + trigger_error($exception->get_message($this->lang) . adm_back_link($this->u_action), E_USER_WARNING); + } } diff --git a/controller/main_controller.php b/controller/main_controller.php index d0638b5..c4930a6 100644 --- a/controller/main_controller.php +++ b/controller/main_controller.php @@ -10,7 +10,6 @@ namespace phpbb\pages\controller; -use Symfony\Component\DependencyInjection\ContainerInterface; use phpbb\exception\http_exception; /** @@ -21,8 +20,8 @@ class main_controller implements main_interface /** @var \phpbb\auth\auth */ protected $auth; - /** @var ContainerInterface */ - protected $container; + /** @var \phpbb\pages\operators\page */ + protected $page_operator; /** @var \phpbb\controller\helper */ protected $helper; @@ -39,18 +38,18 @@ class main_controller implements main_interface /** * Constructor * - * @param \phpbb\auth\auth $auth Authentication object - * @param ContainerInterface $container Service container interface - * @param \phpbb\controller\helper $helper Controller helper object - * @param \phpbb\language\language $lang Language object - * @param \phpbb\template\template $template Template object - * @param \phpbb\user $user User object + * @param \phpbb\auth\auth $auth Authentication object + * @param \phpbb\pages\operators\page $page_operator Pages operator + * @param \phpbb\controller\helper $helper Controller helper object + * @param \phpbb\language\language $lang Language object + * @param \phpbb\template\template $template Template object + * @param \phpbb\user $user User object * @access public */ - public function __construct(\phpbb\auth\auth $auth, ContainerInterface $container, \phpbb\controller\helper $helper, \phpbb\language\language $lang, \phpbb\template\template $template, \phpbb\user $user) + public function __construct(\phpbb\auth\auth $auth, \phpbb\pages\operators\page $page_operator, \phpbb\controller\helper $helper, \phpbb\language\language $lang, \phpbb\template\template $template, \phpbb\user $user) { $this->auth = $auth; - $this->container = $container; + $this->page_operator = $page_operator; $this->helper = $helper; $this->lang = $lang; $this->template = $template; @@ -104,14 +103,10 @@ public function display($route) */ protected function load_page_data($route) { - // Initiate the page entity - /* @var $entity \phpbb\pages\entity\page */ - $entity = $this->container->get('phpbb.pages.entity'); - // Load the requested page by route try { - $entity->load(0, $route); + $entity = $this->page_operator->get_page(0, $route); } catch (\phpbb\pages\exception\base $e) { @@ -119,7 +114,7 @@ protected function load_page_data($route) } // Throw 404 error if page display to guests is disabled - if ($this->user->data['user_id'] == ANONYMOUS && !$entity->get_page_display_to_guests()) + if ((int) $this->user->data['user_id'] === ANONYMOUS && !$entity->get_page_display_to_guests()) { throw new http_exception(404, 'PAGE_NOT_AVAILABLE', array($route)); } diff --git a/entity/factory.php b/entity/factory.php new file mode 100644 index 0000000..a9f0101 --- /dev/null +++ b/entity/factory.php @@ -0,0 +1,78 @@ + +* @license GNU General Public License, version 2 (GPL-2.0) +* +*/ + +namespace phpbb\pages\entity; + +use phpbb\config\config; +use phpbb\db\driver\driver_interface; +use phpbb\event\dispatcher_interface; +use phpbb\pages\textformatter\litedown; +use phpbb\textformatter\s9e\utils; + +/** + * Factory for page entities. + */ +class factory +{ + /** @var driver_interface */ + protected $db; + + /** @var config */ + protected $config; + + /** @var dispatcher_interface */ + protected $dispatcher; + + /** @var string */ + protected $pages_table; + + /** @var utils */ + protected $text_formatter_utils; + + /** @var litedown */ + protected $litedown; + + /** + * Constructor. + * + * @param driver_interface $db + * @param config $config + * @param dispatcher_interface $dispatcher + * @param string $pages_table + * @param utils $text_formatter_utils + * @param litedown $litedown + */ + public function __construct(driver_interface $db, config $config, dispatcher_interface $dispatcher, string $pages_table, utils $text_formatter_utils, litedown $litedown) + { + $this->db = $db; + $this->config = $config; + $this->dispatcher = $dispatcher; + $this->pages_table = $pages_table; + $this->text_formatter_utils = $text_formatter_utils; + $this->litedown = $litedown; + } + + /** + * Create a fresh page entity. + * + * @return page_interface + */ + public function create() + { + return new page( + $this->db, + $this->config, + $this->dispatcher, + $this->pages_table, + $this->text_formatter_utils, + $this->litedown + ); + } +} diff --git a/entity/page.php b/entity/page.php index 0c6703f..cd3d377 100644 --- a/entity/page.php +++ b/entity/page.php @@ -38,7 +38,14 @@ class page implements page_interface * page_icon_font * @access protected */ - protected $data; + protected $data = array(); + + /** + * Storage-form data captured when this entity was hydrated. + * + * @var array + */ + protected $original_data = array(); /** @var \phpbb\db\driver\driver_interface */ protected $db; @@ -83,68 +90,35 @@ public function __construct(\phpbb\db\driver\driver_interface $db, \phpbb\config $this->litedown = $litedown; } - /** - * Load the data from the database for a page - * - * @param int $id Page identifier - * @param string $route Page route - * @return page_interface $this object for chaining calls; load()->set()->save() - * @access public - * @throws \phpbb\pages\exception\out_of_bounds - */ - public function load($id = 0, $route = '') - { - // Load by id if provided, otherwise default to load by page route - $sql_where = ($id !== 0) ? 'page_id = ' . (int) $id : "page_route = '" . $this->db->sql_escape($route) . "'"; - - // Get page from the database - $sql = 'SELECT * - FROM ' . $this->pages_table . ' - WHERE ' . $sql_where; - $result = $this->db->sql_query($sql); - $this->data = $this->db->sql_fetchrow($result); - $this->db->sql_freeresult($result); - - if ($this->data === false) - { - // The page does not exist - throw new \phpbb\pages\exception\out_of_bounds('page_id'); - } - - return $this; - } - /** * Import data for a page * * Used when the data is already loaded externally. * Any existing data on this page is over-written. - * All data is validated and an exception is thrown if any data is invalid. + * Required fields are checked and storage types are normalized. Values already loaded + * from storage are not passed through write-time transformations again. * * @param array $data Data array, typically from the database - * @return page_interface $this object for chaining calls; load()->set()->save() + * @return page_interface $this object for chaining calls * @access public * @throws \phpbb\pages\exception\base */ public function import($data) { - // Clear out any saved data - $this->data = array(); - // All of our fields $fields = array( // column => data type (see settype()) 'page_id' => 'integer', - 'page_order' => 'set_order', // call set_order() - 'page_title' => 'set_title', // call set_title() - 'page_description' => 'set_description', // call set_description() - 'page_description_display' => 'set_description_display', // call set_description_display() - 'page_route' => 'set_route', // call set_route() - 'page_display' => 'set_page_display', // call set_page_display() - 'page_display_to_guests' => 'set_page_display_to_guests', // call set_page_display_to_guests() - 'page_title_switch' => 'set_page_title_switch', // call set_page_title_switch() - 'page_template' => 'set_template', // call set_template() - 'page_icon_font' => 'set_icon_font', // call set_icon_font() + 'page_order' => 'integer', + 'page_title' => 'string', + 'page_description' => 'string', + 'page_description_display' => 'bool', + 'page_route' => 'string', + 'page_display' => 'bool', + 'page_display_to_guests' => 'bool', + 'page_title_switch' => 'bool', + 'page_template' => 'string', + 'page_icon_font' => 'string', // We do not pass to set_content() as generate_text_for_storage would run twice 'page_content' => 'string', @@ -155,7 +129,9 @@ public function import($data) 'page_content_markdown' => 'bool', ); - // Go through the basic fields and set them to our data array + $hydrated = array(); + + // Cast storage values without invoking write-time setters. foreach ($fields as $field => $type) { // If the data wasn't sent to us, throw an exception @@ -164,97 +140,58 @@ public function import($data) throw new \phpbb\pages\exception\invalid_argument(array($field, 'FIELD_MISSING')); } - // If the type is a method on this class, call it - if (method_exists($this, $type)) - { - $this->$type($data[$field]); - } - else - { - // settype passes values by reference - $value = $data[$field]; - - // We're using settype to enforce data types - settype($value, $type); - - $this->data[$field] = $value; - } + // settype passes values by reference + $value = $data[$field]; + settype($value, $type); + $hydrated[$field] = $value; } // Some fields must be unsigned (>= 0) $validate_unsigned = array( 'page_id', + 'page_order', 'page_content_bbcode_options', ); foreach ($validate_unsigned as $field) { - // If the data is less than 0, it's not unsigned and we'll throw an exception - if ($this->data[$field] < 0) + // If the data is less than 0, it's not unsigned, and we'll throw an exception + if ($hydrated[$field] < 0) { throw new \phpbb\pages\exception\out_of_bounds($field); } } - return $this; - } - - /** - * Insert the page data for the first time - * - * Will throw an exception if the page was already inserted (call save() instead) - * - * @return page_interface $this object for chaining calls; load()->set()->save() - * @access public - * @throws \phpbb\pages\exception\out_of_bounds - */ - public function insert() - { - if (!empty($this->data['page_id'])) + if ($hydrated['page_order'] > 16777215) { - // The page already exists - throw new \phpbb\pages\exception\out_of_bounds('page_id'); + throw new \phpbb\pages\exception\out_of_bounds('page_order'); } - // Insert the page data to the database - $sql = 'INSERT INTO ' . $this->pages_table . ' ' . $this->db->sql_build_array('INSERT', $this->data); - $this->db->sql_query($sql); - - // Set the page_id using the id created by the SQL insert - $this->data['page_id'] = (int) $this->db->sql_nextid(); + // Replace state only after the entire row has passed hydration checks. + $this->data = $hydrated; + $this->original_data = $hydrated; return $this; } /** - * Save the current settings to the database - * - * This must be called before closing or any changes will not be saved! - * If adding a page (saving for the first time), you must call insert() or an exeception will be thrown - * - * @return page_interface $this object for chaining calls; load()->set()->save() - * @access public - * @throws \phpbb\pages\exception\out_of_bounds - */ - public function save() + * Export current storage-form data. + * + * @return array + */ + public function get_data() { - if (empty($this->data['page_id'])) - { - // The page does not exist - throw new \phpbb\pages\exception\out_of_bounds('page_id'); - } - - // Copy the data array, filtering out the page_id identifier - // so we do not attempt to update the row's identity column. - $sql_array = array_diff_key($this->data, array('page_id' => null)); - - // Update the page data in the database - $sql = 'UPDATE ' . $this->pages_table . ' - SET ' . $this->db->sql_build_array('UPDATE', $sql_array) . ' - WHERE page_id = ' . $this->get_id(); - $this->db->sql_query($sql); + return $this->data; + } - return $this; + /** + * Export storage-form fields changed since hydration. + * + * @return array + */ + public function get_changes() + { + return array_diff_assoc($this->data, $this->original_data); } /** @@ -283,7 +220,7 @@ public function get_title() * Set title * * @param string $title - * @return page_interface $this object for chaining calls; load()->set()->save() + * @return page_interface $this object for chaining calls * @access public * @throws \phpbb\pages\exception\unexpected_value */ @@ -300,8 +237,8 @@ public function set_title($title) throw new \phpbb\pages\exception\unexpected_value(array('title', 'FIELD_MISSING')); } - // Limit both the displayed and stored title lengths to the column size. - if (truncate_string($title, 200, 200) !== $title) + // Enforce the database column length after storage encoding. + if (utf8_strlen($title) > 200) { throw new \phpbb\pages\exception\unexpected_value(array('title', 'TOO_LONG')); } @@ -327,7 +264,7 @@ public function get_description() * Set description * * @param string $description Description text - * @return page_interface $this object for chaining calls; load()->set()->save() + * @return page_interface $this object for chaining calls * @access public * @throws \phpbb\pages\exception\unexpected_value */ @@ -338,8 +275,8 @@ public function set_description($description) $description = $this->encode_unicode_for_storage($description); - // Limit both the displayed and stored description lengths to the column size. - if (truncate_string($description, 255, 255) !== $description) + // Enforce the database column length after storage encoding. + if (utf8_strlen($description) > 255) { throw new \phpbb\pages\exception\unexpected_value(array('description', 'TOO_LONG')); } @@ -365,7 +302,7 @@ public function get_description_display() * Set description display setting * * @param bool $option Description display setting - * @return page_interface $this object for chaining calls; load()->set()->save() + * @return page_interface $this object for chaining calls * @access public */ public function set_description_display($option) @@ -394,7 +331,7 @@ public function get_route() * Set route * * @param string $route Route text - * @return page_interface $this object for chaining calls; load()->set()->save() + * @return page_interface $this object for chaining calls * @access public * @throws \phpbb\pages\exception\unexpected_value */ @@ -459,7 +396,7 @@ public function get_order() * Set order * * @param int $order Page sort order - * @return page_interface $this object for chaining calls; load()->set()->save() + * @return page_interface $this object for chaining calls * @access public * @throws \phpbb\pages\exception\out_of_bounds */ @@ -499,7 +436,7 @@ public function get_template() * Set page template * * @param string $template Page template name - * @return page_interface $this object for chaining calls; load()->set()->save() + * @return page_interface $this object for chaining calls * @access public * @throws \phpbb\pages\exception\unexpected_value */ @@ -542,7 +479,7 @@ public function get_icon_font() * Set page icon font name * * @param string $name icon font name - * @return page_interface $this object for chaining calls; load()->set()->save() + * @return page_interface $this object for chaining calls * @access public * @throws \phpbb\pages\exception\unexpected_value */ @@ -594,6 +531,9 @@ public function get_content_for_edit() * @param bool $censor_text True to censor the text (Default: true) * @return string * @access public + * @noinspection PhpVarTagWithoutVariableNameInspection + * @noinspection PassingByReferenceCorrectnessInspection + * @noinspection PhpUnusedLocalVariableInspection */ public function get_content_for_display($censor_text = true) { @@ -610,7 +550,7 @@ public function get_content_for_display($censor_text = true) if ($content_html_enabled) { // This is required by s9e text formatter to - // remove extra xml formatting from the content. + // remove extra XML formatting from the content. $content = $this->text_formatter_utils->unparse($content); $content = htmlspecialchars_decode($content, ENT_COMPAT); @@ -644,7 +584,7 @@ public function get_content_for_display($censor_text = true) * Set content * * @param string $content - * @return page_interface $this object for chaining calls; load()->set()->save() + * @return page_interface $this object for chaining calls * @access public */ public function set_content($content) @@ -687,7 +627,7 @@ public function content_bbcode_enabled() * Enable bbcode on the content * This should be called before set_content(); content_enable_bbcode()->set_content() * - * @return page_interface $this object for chaining calls; load()->set()->save() + * @return page_interface $this object for chaining calls * @access public */ public function content_enable_bbcode() @@ -701,7 +641,7 @@ public function content_enable_bbcode() * Disable bbcode on the content * This should be called before set_content(); content_disable_bbcode()->set_content() * - * @return page_interface $this object for chaining calls; load()->set()->save() + * @return page_interface $this object for chaining calls * @access public */ public function content_disable_bbcode() @@ -726,7 +666,7 @@ public function content_magic_url_enabled() * Enable magic url on the content * This should be called before set_content(); content_enable_magic_url()->set_content() * - * @return page_interface $this object for chaining calls; load()->set()->save() + * @return page_interface $this object for chaining calls * @access public */ public function content_enable_magic_url() @@ -740,7 +680,7 @@ public function content_enable_magic_url() * Disable magic url on the content * This should be called before set_content(); content_disable_magic_url()->set_content() * - * @return page_interface $this object for chaining calls; load()->set()->save() + * @return page_interface $this object for chaining calls * @access public */ public function content_disable_magic_url() @@ -765,7 +705,7 @@ public function content_smilies_enabled() * Enable smilies on the content * This should be called before set_content(); content_enable_smilies()->set_content() * - * @return page_interface $this object for chaining calls; load()->set()->save() + * @return page_interface $this object for chaining calls * @access public */ public function content_enable_smilies() @@ -779,7 +719,7 @@ public function content_enable_smilies() * Disable smilies on the content * This should be called before set_content(); content_disable_smilies()->set_content() * - * @return page_interface $this object for chaining calls; load()->set()->save() + * @return page_interface $this object for chaining calls * @access public */ public function content_disable_smilies() @@ -803,7 +743,7 @@ public function content_markdown_enabled() /** * Enable Markdown on the content * - * @return page_interface $this object for chaining calls; load()->set()->save() + * @return page_interface $this object for chaining calls * @access public */ public function content_enable_markdown() @@ -817,7 +757,7 @@ public function content_enable_markdown() /** * Disable Markdown on the content * - * @return page_interface $this object for chaining calls; load()->set()->save() + * @return page_interface $this object for chaining calls * @access public */ public function content_disable_markdown() @@ -843,7 +783,7 @@ public function content_html_enabled() * This should be called before set_content(); content_enable_html()->set_content() * This should also be called after the bbcode, smilies and magic url setters * - * @return page_interface $this object for chaining calls; load()->set()->save() + * @return page_interface $this object for chaining calls * @access public */ public function content_enable_html() @@ -863,7 +803,7 @@ public function content_enable_html() * Disable HTML on the content * This should be called before set_content(); content_disable_html()->set_content() * - * @return page_interface $this object for chaining calls; load()->set()->save() + * @return page_interface $this object for chaining calls * @access public */ public function content_disable_html() @@ -888,7 +828,7 @@ public function get_page_display() * Set page display setting * * @param bool $option Page display setting - * @return page_interface $this object for chaining calls; load()->set()->save() + * @return page_interface $this object for chaining calls * @access public */ public function set_page_display($option) @@ -917,7 +857,7 @@ public function get_page_display_to_guests() * Set page display to guests setting * * @param bool $option Page display to guests setting - * @return page_interface $this object for chaining calls; load()->set()->save() + * @return page_interface $this object for chaining calls * @access public */ public function set_page_display_to_guests($option) @@ -946,7 +886,7 @@ public function get_page_title_switch() * Set page title switch setting * * @param bool $option Page title switch setting - * @return page_interface $this object for chaining calls; load()->set()->save() + * @return page_interface $this object for chaining calls * @access public */ public function set_page_title_switch($option) diff --git a/entity/page_interface.php b/entity/page_interface.php index e3befa2..403290d 100644 --- a/entity/page_interface.php +++ b/entity/page_interface.php @@ -13,57 +13,38 @@ /** * Interface for a page * -* This describes all of the methods we'll have for a single page +* This describes all the methods we'll have for a single page */ interface page_interface { - /** - * Load the data from the database for a page - * - * @param int $id Page identifier - * @param string $route Page route - * @return page_interface $this object for chaining calls; load()->set()->save() - * @access public - * @throws \phpbb\pages\exception\out_of_bounds - */ - public function load($id = 0, $route = ''); - /** * Import data for a page * * Used when the data is already loaded externally. * Any existing data on this page is over-written. - * All data is validated and an exception is thrown if any data is invalid. + * Required fields are checked and storage types are normalized. Values already loaded + * from storage are not passed through write-time transformations again. * * @param array $data Data array, typically from the database - * @return page_interface $this object for chaining calls; load()->set()->save() + * @return page_interface $this object for chaining calls * @access public * @throws \phpbb\pages\exception\base */ public function import($data); /** - * Insert the page data for the first time - * - * Will throw an exception if the page was already inserted (call save() instead) - * - * @return page_interface $this object for chaining calls; load()->set()->save() - * @access public - * @throws \phpbb\pages\exception\out_of_bounds - */ - public function insert(); + * Export current storage-form data. + * + * @return array + */ + public function get_data(); /** - * Save the current settings to the database - * - * This must be called before closing or any changes will not be saved! - * If adding a page (saving for the first time), you must call insert() or an exeception will be thrown - * - * @return page_interface $this object for chaining calls; load()->set()->save() - * @access public - * @throws \phpbb\pages\exception\out_of_bounds - */ - public function save(); + * Export storage-form fields changed since hydration. + * + * @return array + */ + public function get_changes(); /** * Get id @@ -85,7 +66,7 @@ public function get_title(); * Set title * * @param string $title - * @return page_interface $this object for chaining calls; load()->set()->save() + * @return page_interface $this object for chaining calls * @access public * @throws \phpbb\pages\exception\unexpected_value */ @@ -103,7 +84,7 @@ public function get_description(); * Set description * * @param string $description Description text - * @return page_interface $this object for chaining calls; load()->set()->save() + * @return page_interface $this object for chaining calls * @access public * @throws \phpbb\pages\exception\unexpected_value */ @@ -121,7 +102,7 @@ public function get_description_display(); * Set description display setting * * @param bool $option Description display setting - * @return page_interface $this object for chaining calls; load()->set()->save() + * @return page_interface $this object for chaining calls * @access public */ public function set_description_display($option); @@ -138,7 +119,7 @@ public function get_route(); * Set route * * @param string $route Route text - * @return page_interface $this object for chaining calls; load()->set()->save() + * @return page_interface $this object for chaining calls * @access public * @throws \phpbb\pages\exception\unexpected_value */ @@ -156,7 +137,7 @@ public function get_order(); * Set order * * @param int $order Page sort order - * @return page_interface $this object for chaining calls; load()->set()->save() + * @return page_interface $this object for chaining calls * @access public * @throws \phpbb\pages\exception\out_of_bounds */ @@ -174,7 +155,7 @@ public function get_template(); * Set page template * * @param string $template Page template name - * @return page_interface $this object for chaining calls; load()->set()->save() + * @return page_interface $this object for chaining calls * @access public * @throws \phpbb\pages\exception\unexpected_value */ @@ -192,7 +173,7 @@ public function get_icon_font(); * Set page icon font name * * @param string $name icon font name - * @return page_interface $this object for chaining calls; load()->set()->save() + * @return page_interface $this object for chaining calls * @access public * @throws \phpbb\pages\exception\unexpected_value */ @@ -219,7 +200,7 @@ public function get_content_for_display($censor_text = true); * Set content * * @param string $content - * @return page_interface $this object for chaining calls; load()->set()->save() + * @return page_interface $this object for chaining calls * @access public */ public function set_content($content); @@ -236,7 +217,7 @@ public function content_bbcode_enabled(); * Enable bbcode on the content * This should be called before set_content(); content_enable_bbcode()->set_content() * - * @return page_interface $this object for chaining calls; load()->set()->save() + * @return page_interface $this object for chaining calls * @access public */ public function content_enable_bbcode(); @@ -245,7 +226,7 @@ public function content_enable_bbcode(); * Disable bbcode on the content * This should be called before set_content(); content_disable_bbcode()->set_content() * - * @return page_interface $this object for chaining calls; load()->set()->save() + * @return page_interface $this object for chaining calls * @access public */ public function content_disable_bbcode(); @@ -262,7 +243,7 @@ public function content_magic_url_enabled(); * Enable magic url on the content * This should be called before set_content(); content_enable_magic_url()->set_content() * - * @return page_interface $this object for chaining calls; load()->set()->save() + * @return page_interface $this object for chaining calls * @access public */ public function content_enable_magic_url(); @@ -271,7 +252,7 @@ public function content_enable_magic_url(); * Disable magic url on the content * This should be called before set_content(); content_disable_magic_url()->set_content() * - * @return page_interface $this object for chaining calls; load()->set()->save() + * @return page_interface $this object for chaining calls * @access public */ public function content_disable_magic_url(); @@ -288,7 +269,7 @@ public function content_smilies_enabled(); * Enable smilies on the content * This should be called before set_content(); content_enable_smilies()->set_content() * - * @return page_interface $this object for chaining calls; load()->set()->save() + * @return page_interface $this object for chaining calls * @access public */ public function content_enable_smilies(); @@ -297,7 +278,7 @@ public function content_enable_smilies(); * Disable smilies on the content * This should be called before set_content(); content_disable_smilies()->set_content() * - * @return page_interface $this object for chaining calls; load()->set()->save() + * @return page_interface $this object for chaining calls * @access public */ public function content_disable_smilies(); @@ -313,7 +294,7 @@ public function content_markdown_enabled(); /** * Enable Markdown on the content * - * @return page_interface $this object for chaining calls; load()->set()->save() + * @return page_interface $this object for chaining calls * @access public */ public function content_enable_markdown(); @@ -321,7 +302,7 @@ public function content_enable_markdown(); /** * Disable Markdown on the content * - * @return page_interface $this object for chaining calls; load()->set()->save() + * @return page_interface $this object for chaining calls * @access public */ public function content_disable_markdown(); @@ -339,7 +320,7 @@ public function content_html_enabled(); * This should be called before set_content(); content_enable_html()->set_content() * This should also be called after the Markdown, BBCode, smilies and magic URL setters * - * @return page_interface $this object for chaining calls; load()->set()->save() + * @return page_interface $this object for chaining calls * @access public */ public function content_enable_html(); @@ -348,7 +329,7 @@ public function content_enable_html(); * Disable HTML on the content * This should be called before set_content(); content_disable_html()->set_content() * - * @return page_interface $this object for chaining calls; load()->set()->save() + * @return page_interface $this object for chaining calls * @access public */ public function content_disable_html(); @@ -365,7 +346,7 @@ public function get_page_display(); * Set page display setting * * @param bool $option Page display setting - * @return page_interface $this object for chaining calls; load()->set()->save() + * @return page_interface $this object for chaining calls * @access public */ public function set_page_display($option); @@ -382,7 +363,7 @@ public function get_page_display_to_guests(); * Set page display to guests setting * * @param bool $option Page display to guests setting - * @return page_interface $this object for chaining calls; load()->set()->save() + * @return page_interface $this object for chaining calls * @access public */ public function set_page_display_to_guests($option); @@ -399,7 +380,7 @@ public function get_page_title_switch(); * Set page title switch setting * * @param bool $option Page title switch setting - * @return page_interface $this object for chaining calls; load()->set()->save() + * @return page_interface $this object for chaining calls * @access public */ public function set_page_title_switch($option); diff --git a/exception/base.php b/exception/base.php index b3053c1..12482f0 100644 --- a/exception/base.php +++ b/exception/base.php @@ -135,19 +135,7 @@ protected function translate_portions(\phpbb\language\language $lang, $message_p */ public function add_lang(\phpbb\language\language $lang) { - static $is_loaded = false; - - // We only need to load the language file once - if ($is_loaded) - { - return; - } - - // Add our language file $lang->add_lang('exceptions', 'phpbb/pages'); - - // So the language file is only loaded once - $is_loaded = true; } /** diff --git a/operators/page.php b/operators/page.php index 2a69825..afff5eb 100644 --- a/operators/page.php +++ b/operators/page.php @@ -10,8 +10,6 @@ namespace phpbb\pages\operators; -use Symfony\Component\DependencyInjection\ContainerInterface; - /** * Operator for a set of pages */ @@ -20,8 +18,8 @@ class page implements page_interface /** @var \phpbb\cache\driver\driver_interface */ protected $cache; - /** @var ContainerInterface */ - protected $container; + /** @var \phpbb\pages\entity\factory */ + protected $entity_factory; /** @var \phpbb\db\driver\driver_interface */ protected $db; @@ -45,7 +43,7 @@ class page implements page_interface * Constructor * * @param \phpbb\cache\driver\driver_interface $cache Cache driver interface - * @param ContainerInterface $container Service container interface + * @param \phpbb\pages\entity\factory $entity_factory Page entity factory * @param \phpbb\db\driver\driver_interface $db Database connection * @param \phpbb\extension\manager $extension_manager Extension manager object * @param \phpbb\user $user User object @@ -54,10 +52,10 @@ class page implements page_interface * @param string $pages_pages_links_table Table name * @access public */ - public function __construct(\phpbb\cache\driver\driver_interface $cache, ContainerInterface $container, \phpbb\db\driver\driver_interface $db, \phpbb\extension\manager $extension_manager, \phpbb\user $user, $pages_table, $pages_links_table, $pages_pages_links_table) + public function __construct(\phpbb\cache\driver\driver_interface $cache, \phpbb\pages\entity\factory $entity_factory, \phpbb\db\driver\driver_interface $db, \phpbb\extension\manager $extension_manager, \phpbb\user $user, $pages_table, $pages_links_table, $pages_pages_links_table) { $this->cache = $cache; - $this->container = $container; + $this->entity_factory = $entity_factory; $this->db = $db; $this->extension_manager = $extension_manager; $this->user = $user; @@ -66,12 +64,52 @@ public function __construct(\phpbb\cache\driver\driver_interface $cache, Contain $this->pages_pages_links_table = $pages_pages_links_table; } + /** + * Create an empty page entity. + * + * @return \phpbb\pages\entity\page_interface + */ + public function create_page() + { + return $this->entity_factory->create(); + } + + /** + * Get one page by identifier or route. + * + * @param int $id Page identifier + * @param string $route Page route + * @return \phpbb\pages\entity\page_interface + * @throws \phpbb\pages\exception\base If the page is missing or stored data is invalid + */ + public function get_page($id = 0, $route = '') + { + $sql_where = ($id !== 0) + ? 'page_id = ' . (int) $id + : "page_route = '" . $this->db->sql_escape($route) . "'"; + + $sql = 'SELECT * + FROM ' . $this->pages_table . ' + WHERE ' . $sql_where; + $result = $this->db->sql_query($sql); + $row = $this->db->sql_fetchrow($result); + $this->db->sql_freeresult($result); + + if ($row === false) + { + throw new \phpbb\pages\exception\out_of_bounds('page_id'); + } + + return $this->create_page()->import($row); + } + /** * Get all pages * * @param int $limit * @param int $start * @return array Array of page data entities + * @throws \phpbb\pages\exception\base If stored page data is invalid * @access public */ public function get_pages($limit = 0, $start = 0) @@ -87,7 +125,7 @@ public function get_pages($limit = 0, $start = 0) while ($row = $this->db->sql_fetchrow($result)) { // Import each page row into an entity - $entities[] = $this->container->get('phpbb.pages.entity')->import($row); + $entities[] = $this->create_page()->import($row); } $this->db->sql_freeresult($result); @@ -100,19 +138,49 @@ public function get_pages($limit = 0, $start = 0) * * @param \phpbb\pages\entity\page_interface $entity Page entity with new data to insert * @return \phpbb\pages\entity\page_interface Added page entity - * @throws \phpbb\pages\exception\out_of_bounds + * @throws \phpbb\pages\exception\base If the entity already exists or stored data is invalid * @access public */ public function add_page($entity) { - // Insert the page data to the database - $entity->insert(); + if ($entity->get_id()) + { + throw new \phpbb\pages\exception\out_of_bounds('page_id'); + } + + $data = array_diff_key($entity->get_data(), array('page_id' => null)); + $sql = 'INSERT INTO ' . $this->pages_table . ' ' . $this->db->sql_build_array('INSERT', $data); + $this->db->sql_query($sql); + $page_id = (int) $this->db->sql_nextid(); + + return $this->get_page($page_id); + } - // Get the newly inserted page's identifier + /** + * Persist changes to an existing page. + * + * @param \phpbb\pages\entity\page_interface $entity Page entity + * @return \phpbb\pages\entity\page_interface Persisted page entity + * @throws \phpbb\pages\exception\base If the entity is new, missing, or stored data is invalid + */ + public function save_page($entity) + { $page_id = $entity->get_id(); + if (!$page_id) + { + throw new \phpbb\pages\exception\out_of_bounds('page_id'); + } + + $changes = array_diff_key($entity->get_changes(), array('page_id' => null)); + if (!empty($changes)) + { + $sql = 'UPDATE ' . $this->pages_table . ' + SET ' . $this->db->sql_build_array('UPDATE', $changes) . ' + WHERE page_id = ' . $page_id; + $this->db->sql_query($sql); + } - // Reload the data to return a fresh page entity - return $entity->load($page_id); + return $this->get_page($page_id); } /** @@ -139,7 +207,7 @@ public function delete_page($page_id) } /** - * Get page routes (for use in viewonline) + * Get page routes (for use in view online) * * @return array Array of routes and page titles for all pages * @access public @@ -219,7 +287,7 @@ public function get_page_icon($name) /** * Get custom page templates (pages_*.html) - * Added by the user to the core style/template directores + * Added by the user to the core style/template directories * * @return array Array of template file paths * @access public diff --git a/operators/page_interface.php b/operators/page_interface.php index 16c611d..162180d 100644 --- a/operators/page_interface.php +++ b/operators/page_interface.php @@ -17,12 +17,30 @@ */ interface page_interface { + /** + * Create an empty page entity. + * + * @return \phpbb\pages\entity\page_interface + */ + public function create_page(); + + /** + * Get one page by identifier or route. + * + * @param int $id Page identifier + * @param string $route Page route + * @return \phpbb\pages\entity\page_interface + * @throws \phpbb\pages\exception\base If the page is missing or stored data is invalid + */ + public function get_page($id = 0, $route = ''); + /** * Get all pages * * @param int $limit * @param int $start * @return array Array of page data entities + * @throws \phpbb\pages\exception\base If stored page data is invalid * @access public */ public function get_pages($limit = 0, $start = 0); @@ -31,12 +49,21 @@ public function get_pages($limit = 0, $start = 0); * Add a page * * @param \phpbb\pages\entity\page_interface $entity Page entity with new data to insert - * @return page_interface Added page entity - * @throws \phpbb\pages\exception\out_of_bounds + * @return \phpbb\pages\entity\page_interface Added page entity + * @throws \phpbb\pages\exception\base If the entity already exists or stored data is invalid * @access public */ public function add_page($entity); + /** + * Persist changes to an existing page. + * + * @param \phpbb\pages\entity\page_interface $entity Page entity + * @return \phpbb\pages\entity\page_interface Persisted page entity + * @throws \phpbb\pages\exception\base If the entity is new, missing, or stored data is invalid + */ + public function save_page($entity); + /** * Delete a page * @@ -48,7 +75,7 @@ public function add_page($entity); public function delete_page($page_id); /** - * Get page routes (for use in viewonline) + * Get page routes (for use in view online) * * @return array Array of routes and page titles for all pages * @access public @@ -76,7 +103,7 @@ public function get_page_icon($name); /** * Get custom page templates (pages_*.html) - * Added by the user to the core style/template directores + * Added by the user to the core style/template directories * * @return array Array of template file paths * @access public diff --git a/tests/controller/admin_controller_test.php b/tests/controller/admin_controller_test.php index 39e118f..2edbcfa 100644 --- a/tests/controller/admin_controller_test.php +++ b/tests/controller/admin_controller_test.php @@ -103,24 +103,17 @@ protected function setUp(): void $user->data['user_id'] = 2; $user->ip = '127.0.0.1'; - $entity_factory = function () use ($config, $phpbb_dispatcher, $text_formatter_utils, $litedown) { - return new \phpbb\pages\entity\page( - $this->db, - $config, - $phpbb_dispatcher, - 'phpbb_pages', - $text_formatter_utils, - $litedown - ); - }; - - $operator_container = $this->createMock(\Symfony\Component\DependencyInjection\ContainerInterface::class); - $operator_container->method('get') - ->with('phpbb.pages.entity') - ->willReturnCallback($entity_factory); + $entity_factory = new \phpbb\pages\entity\factory( + $this->db, + $config, + $phpbb_dispatcher, + 'phpbb_pages', + $text_formatter_utils, + $litedown + ); $this->page_operator = new \phpbb\pages\operators\page( $cache, - $operator_container, + $entity_factory, $this->db, $extension_manager, $user, @@ -132,10 +125,6 @@ protected function setUp(): void $pagination = $this->getMockBuilder(\phpbb\pagination::class) ->disableOriginalConstructor() ->getMock(); - $container = $this->createMock(\Symfony\Component\DependencyInjection\ContainerInterface::class); - $container->method('get')->willReturnCallback(function ($service) use ($entity_factory, $pagination) { - return $service === 'pagination' ? $pagination : $entity_factory(); - }); $this->request = $this->createMock(\phpbb\request\request::class); $this->request->method('variable')->willReturnCallback(function ($name, $default) { @@ -184,7 +173,7 @@ protected function setUp(): void $this->request, $this->template, $user, - $container, + $pagination, $phpbb_dispatcher, $phpbb_root_path, $phpEx @@ -203,6 +192,19 @@ public function test_display_pages_uses_real_entities() self::assertSame('adm.php?i=pages&action=add', $this->assigned_vars['U_ADD_PAGE']); } + public function test_display_pages_reports_hydration_failure() + { + $operator = $this->getMockBuilder(\phpbb\pages\operators\page::class) + ->disableOriginalConstructor() + ->getMock(); + $operator->method('get_total_pages')->willReturn(1); + $operator->method('get_pages')->willThrowException(new \phpbb\pages\exception\invalid_argument(array('page_title', 'FIELD_MISSING'))); + $this->replace_controller_service('page_operator', $operator); + $this->setExpectedTriggerError(E_USER_WARNING, 'Invalid argument specified for `page_title`. Reason: Required field missing|back:adm.php?i=pages'); + + $this->controller->display_pages(); + } + public function test_add_page_initial_form_uses_real_entity() { $this->controller->add_page(); @@ -215,6 +217,22 @@ public function test_add_page_initial_form_uses_real_entity() self::assertSame(1, admin_test_state::$custom_bbcodes_displayed); } + public function test_add_page_reports_persistence_failure() + { + $entity = $this->page_operator->create_page(); + $operator = $this->getMockBuilder(\phpbb\pages\operators\page::class) + ->disableOriginalConstructor() + ->getMock(); + $operator->method('create_page')->willReturn($entity); + $operator->method('add_page')->willThrowException(new \phpbb\pages\exception\invalid_argument(array('page_title', 'FIELD_MISSING'))); + $this->replace_controller_service('page_operator', $operator); + $this->post['submit'] = true; + $this->variables = $this->valid_page_data('new-page', 'New page'); + $this->setExpectedTriggerError(E_USER_WARNING, 'Invalid argument specified for `page_title`. Reason: Required field missing|back:adm.php?i=pages'); + + $this->controller->add_page(); + } + public function test_edit_page_initial_form_loads_links_and_parse_options() { $this->controller->edit_page(1); @@ -227,6 +245,18 @@ public function test_edit_page_initial_form_loads_links_and_parse_options() self::assertSame(0, $this->assigned_vars['S_PARSE_BBCODE_CHECKED']); } + public function test_edit_page_reports_hydration_failure() + { + $operator = $this->getMockBuilder(\phpbb\pages\operators\page::class) + ->disableOriginalConstructor() + ->getMock(); + $operator->method('get_page')->willThrowException(new \phpbb\pages\exception\invalid_argument(array('page_title', 'FIELD_MISSING'))); + $this->replace_controller_service('page_operator', $operator); + $this->setExpectedTriggerError(E_USER_WARNING, 'Invalid argument specified for `page_title`. Reason: Required field missing|back:adm.php?i=pages'); + + $this->controller->edit_page(1); + } + public function test_page_link_options_load_stored_links_when_current_is_empty() { $method = new \ReflectionMethod(admin_controller::class, 'create_page_link_options'); @@ -320,6 +350,21 @@ public function test_delete_page_reports_operator_failure() $this->controller->delete_page(1); } + public function test_delete_page_reports_lookup_failure() + { + $operator = $this->getMockBuilder(\phpbb\pages\operators\page::class) + ->disableOriginalConstructor() + ->getMock(); + $operator->method('get_page') + ->willThrowException(new \phpbb\pages\exception\out_of_bounds('page_id')); + $operator->expects(self::never())->method('delete_page'); + $this->log->expects(self::never())->method('add'); + $this->replace_controller_service('page_operator', $operator); + $this->setExpectedTriggerError(E_USER_WARNING, 'Page could not be deleted.|back:adm.php?i=pages'); + + $this->controller->delete_page(99); + } + public function test_purge_icons_destroys_only_pages_icon_cache() { $cache = $this->createMock(\phpbb\cache\driver\driver_interface::class); diff --git a/tests/controller/page_main_controller_test.php b/tests/controller/page_main_controller_test.php index bcbfe34..c7aee41 100644 --- a/tests/controller/page_main_controller_test.php +++ b/tests/controller/page_main_controller_test.php @@ -15,15 +15,15 @@ class page_main_controller_test extends \phpbb_database_test_case /** @var \PHPUnit\Framework\MockObject\MockObject|\phpbb\auth\auth */ protected $auth; - /** @var \PHPUnit\Framework\MockObject\MockObject|\Symfony\Component\DependencyInjection\ContainerInterface */ - protected $container; - /** @var \PHPUnit\Framework\MockObject\MockObject|\phpbb\controller\helper */ protected $controller_helper; /** @var \phpbb\language\language */ protected $lang; + /** @var \phpbb\pages\operators\page */ + protected $page_operator; + /** @var \PHPUnit\Framework\MockObject\MockObject|\phpbb\template\template */ protected $template; @@ -69,16 +69,14 @@ protected function setUp(): void return $text; }); - $this->container = $this->getMockBuilder('\Symfony\Component\DependencyInjection\ContainerInterface') - ->disableOriginalConstructor() - ->getMock(); - $this->container - ->method('get') - ->with('phpbb.pages.entity') - ->willReturnCallback(function () use ($db, $config, $phpbb_dispatcher, $text_formatter_utils, $litedown) { - return new \phpbb\pages\entity\page($db, $config, $phpbb_dispatcher, 'phpbb_pages', $text_formatter_utils, $litedown); - }) - ; + $entity_factory = new \phpbb\pages\entity\factory( + $db, + $config, + $phpbb_dispatcher, + 'phpbb_pages', + $text_formatter_utils, + $litedown + ); $this->template = $this->getMockBuilder('\phpbb\template\template') ->getMock() @@ -107,13 +105,23 @@ protected function setUp(): void )) ->getMock(); $phpbb_extension_manager = new \phpbb_mock_extension_manager($phpbb_root_path); + $this->page_operator = new \phpbb\pages\operators\page( + $cache, + $entity_factory, + $db, + $phpbb_extension_manager, + $user, + 'phpbb_pages', + 'phpbb_pages_links', + 'phpbb_pages_pages_links' + ); } public function get_controller() { return new \phpbb\pages\controller\main_controller( $this->auth, - $this->container, + $this->page_operator, $this->controller_helper, $this->lang, $this->template, diff --git a/tests/entity/page_entity_description_test.php b/tests/entity/page_entity_description_test.php index 274244c..7d879df 100644 --- a/tests/entity/page_entity_description_test.php +++ b/tests/entity/page_entity_description_test.php @@ -34,6 +34,10 @@ public function description_test_data() str_repeat('a', 255), str_repeat('a', 255), ), + array( + str_repeat('😀', 28), + str_repeat('😀', 28), + ), ); } @@ -71,6 +75,9 @@ public function description_fails_test_data() array( str_repeat('a', 256), ), + array( + str_repeat('😀', 29), + ), ); } diff --git a/tests/entity/page_entity_import_test.php b/tests/entity/page_entity_import_test.php index eb3d058..d367b39 100644 --- a/tests/entity/page_entity_import_test.php +++ b/tests/entity/page_entity_import_test.php @@ -23,6 +23,8 @@ class page_entity_import_test extends page_entity_base public function import_test_data() { $import_data = $this->get_import_data(); + $import_data[1]['page_title'] = str_repeat('К', 200); + $import_data[1]['page_description'] = str_repeat('Ж', 255); return array( array($import_data[1]), @@ -71,6 +73,51 @@ public function test_import($data) } } + /** + * Stored values bypass write-time validation and remain unchanged. + */ + public function test_import_preserves_storage_values() + { + $data = $this->get_import_data()[1]; + $data['page_title'] = 'Emoji 😀 title'; + $data['page_route'] = str_repeat('a', 101); + $data['page_template'] = 'legacy-template'; + $data['page_icon_font'] = 'legacy_icon'; + + $entity = $this->get_page_entity()->import($data); + + self::assertSame('Emoji 😀 title', $entity->get_data()['page_title']); + self::assertSame('Emoji 😀 title', $entity->get_title()); + self::assertSame($data['page_route'], $entity->get_route()); + self::assertSame($data['page_template'], $entity->get_template()); + self::assertSame($data['page_icon_font'], $entity->get_icon_font()); + self::assertSame(array(), $entity->get_changes()); + + $entity->set_title('Changed title'); + self::assertSame(array('page_title' => 'Changed title'), $entity->get_changes()); + } + + /** + * Failed hydration does not leave partially replaced entity state. + */ + public function test_import_is_atomic() + { + $data = $this->get_import_data()[1]; + $entity = $this->get_page_entity()->import($data); + unset($data['page_route']); + + try + { + $entity->import($data); + self::fail('Expected invalid_argument exception was not thrown.'); + } + catch (\phpbb\pages\exception\invalid_argument $e) + { + self::assertSame(1, $entity->get_id()); + self::assertSame('route1', $entity->get_route()); + } + } + /** * Test data for the test_import_fail() function * @@ -97,31 +144,6 @@ public function import_test_fail_data() 'page_content_bbcode_options' => -1, )); - // Too long - $data[] = array_merge($import_data[1], array( - 'page_route' => str_repeat('a', 101), - )); - - // Too long - $data[] = array_merge($import_data[1], array( - 'page_title' => str_repeat('a', 201), - )); - - // Too long - $data[] = array_merge($import_data[1], array( - 'page_description' => str_repeat('a', 256), - )); - - // Too long - $data[] = array_merge($import_data[1], array( - 'page_template' => str_repeat('a', 256), - )); - - // Too long - $data[] = array_merge($import_data[1], array( - 'page_icon_font' => str_repeat('a', 256), - )); - // Go through every field and unset it while submitting everything else foreach ($import_data[1] as $field => $value) { diff --git a/tests/entity/page_entity_insert_test.php b/tests/entity/page_entity_insert_test.php deleted file mode 100644 index 005b751..0000000 --- a/tests/entity/page_entity_insert_test.php +++ /dev/null @@ -1,116 +0,0 @@ - -* @license GNU General Public License, version 2 (GPL-2.0) -* -*/ - -namespace phpbb\pages\tests\entity; - -/** -* Tests related to insert on page entity -*/ -class page_entity_insert_test extends page_entity_base -{ - /** - * Test inserting new page data - */ - public function test_insert() - { - // This is needed to set up the s9e text formatter services - // This can lead to a test failure if PCRE is old. - $this->get_test_case_helpers()->set_s9e_services(); - - $data = array( - 'page_id' => 5, - 'page_order' => 0, - 'page_route' => 'inserted-route', - 'page_title' => 'inserted-title', - 'page_description' => 'inserted-description', - 'page_description_display' => 1, - 'page_content' => 'inserted-content', - 'page_content_allow_html' => 0, - 'page_content_markdown' => 0, - 'page_display' => 1, - 'page_display_to_guests' => 0, - 'page_title_switch' => 0, - 'page_icon_font' => 'inserted-icon' - ); - - // Setup the entity class - $entity = $this->get_page_entity(); - - // Insert a table row - $result = $entity - ->set_route($data['page_route']) - ->set_title($data['page_title']) - ->set_description($data['page_description']) - ->set_description_display($data['page_description_display']) - ->set_content($data['page_content']) - ->set_order($data['page_order']) - ->set_page_display($data['page_display']) - ->set_page_display_to_guests($data['page_display_to_guests']) - ->set_page_title_switch($data['page_title_switch']) - ->set_icon_font($data['page_icon_font']) - ->insert() - ; - - // Assert the returned value is what we expect - self::assertInstanceOf('\phpbb\pages\entity\page', $result); - - // Assert that the new page_id was created - self::assertEquals($data['page_id'], $result->get_id()); - - // Reload the inserted data from the db - $result = $entity->load($result->get_id()); - - // Assert the returned value is what we expect - self::assertInstanceOf('\phpbb\pages\entity\page', $result); - - // Map the fields to the getters - $map = array( - 'page_id' => 'get_id', - 'page_order' => 'get_order', - 'page_route' => 'get_route', - 'page_title' => 'get_title', - 'page_description' => 'get_description', - 'page_description_display' => 'get_description_display', - 'page_content' => 'get_content_for_edit', - 'page_display' => 'get_page_display', - 'page_display_to_guests' => 'get_page_display_to_guests', - 'page_title_switch' => 'get_page_title_switch', - 'page_icon_font' => 'get_icon_font', - ); - - // Go through each field in the data and make sure the function returns - // what we saved - foreach ($map as $field => $function) - { - self::assertEquals($data[$field], $entity->$function()); - } - } - - /** - * Try inserting a page that already exists into the database - * Entities with an existing page_id will fail to insert - */ - public function test_insert_fails() - { - $this->expectException(\phpbb\pages\exception\out_of_bounds::class); - - // Load some import test data - $import_data = $this->get_import_data(); - - // Setup the entity class - $entity = $this->get_page_entity(); - - // Import an existing page entity - $entity->import($import_data[1]); - - // Try to insert the existing page entity - $entity->insert(); - } -} diff --git a/tests/entity/page_entity_load_test.php b/tests/entity/page_entity_load_test.php deleted file mode 100644 index 6fb4ead..0000000 --- a/tests/entity/page_entity_load_test.php +++ /dev/null @@ -1,162 +0,0 @@ - -* @license GNU General Public License, version 2 (GPL-2.0) -* -*/ - -namespace phpbb\pages\tests\entity; - -/** -* Tests related to load on page entity -*/ -class page_entity_load_test extends page_entity_base -{ - /** - * Test data for the test_load() function - * - * @return array Array of test data - */ - public function load_test_data() - { - return array( - // id to search, data which should match - array( - 1, '', - array( - 'page_id' => 1, - 'page_order' => 1, - 'page_route' => 'page_1', - 'page_title' => 'title_1', - 'page_description' => 'description_1', - 'page_description_display' => 0, - 'page_content' => 'message_1', - 'page_display' => 1, - 'page_display_to_guests' => 1, - 'page_title_switch' => 0, - 'page_icon_font' => 'foo', - ), - ), - array( - 2, '', - array( - 'page_id' => 2, - 'page_order' => 2, - 'page_route' => 'page_2', - 'page_title' => 'title_2', - 'page_description' => 'description_2', - 'page_description_display' => 0, - 'page_content' => 'message_2', - 'page_display' => 1, - 'page_display_to_guests' => 1, - 'page_title_switch' => 0, - 'page_icon_font' => '', - ), - ), - array( - 0, 'page_3', - array( - 'page_id' => 3, - 'page_order' => 1, - 'page_route' => 'page_3', - 'page_title' => 'title_3', - 'page_description' => 'description_3', - 'page_description_display' => 1, - 'page_content' => 'message_3', - 'page_display' => 1, - 'page_display_to_guests' => 0, - 'page_title_switch' => 0, - 'page_icon_font' => '', - ), - ), - array( - 0, 'page_4', - array( - 'page_id' => 4, - 'page_order' => 2, - 'page_route' => 'page_4', - 'page_title' => 'title_4', - 'page_description' => 'description_4', - 'page_description_display' => 0, - 'page_content' => 'message_4', - 'page_display' => 0, - 'page_display_to_guests' => 0, - 'page_title_switch' => 0, - 'page_icon_font' => '', - ), - ), - ); - } - - /** - * Test loading page data from the database - * - * @dataProvider load_test_data - */ - public function test_load($id, $route, $data) - { - // Setup the entity class - $entity = $this->get_page_entity(); - - // Set the data - $result = $entity->load($id, $route); - - // Assert the returned value is what we expect - self::assertInstanceOf('\phpbb\pages\entity\page', $result); - - // Map the fields to the getters - $map = array( - 'page_id' => 'get_id', - 'page_order' => 'get_order', - 'page_description' => 'get_description', - 'page_description_display' => 'get_description_display', - 'page_route' => 'get_route', - 'page_title' => 'get_title', - 'page_content' => 'get_content_for_edit', - 'page_display' => 'get_page_display', - 'page_display_to_guests' => 'get_page_display_to_guests', - 'page_title_switch' => 'get_page_title_switch', - 'page_icon_font' => 'get_icon_font', - ); - - // Go through each field in the data and make sure the function returns - // what we saved - foreach ($map as $field => $function) - { - self::assertEquals($data[$field], $entity->$function()); - } - } - - /** - * Test data for the test_load_fails() function - * - * @return array Array of test data - */ - public function load_fails_test_data() - { - return array( - // id to search - array(0), - array(100), - ); - } - - /** - * Test loading (non-existant) pages from the database - * - * @dataProvider load_fails_test_data - */ - public function test_load_fails($id) - { - $this->expectException(\phpbb\pages\exception\out_of_bounds::class); - - // Setup the entity class - $entity = $this->get_page_entity(); - - // Load the entity - $entity->load($id); - } -} diff --git a/tests/entity/page_entity_route_test.php b/tests/entity/page_entity_route_test.php index a95bec5..2c30b6a 100644 --- a/tests/entity/page_entity_route_test.php +++ b/tests/entity/page_entity_route_test.php @@ -158,7 +158,7 @@ public function test_unique_route($id, $route, $expected) // Load the page from the db if it exists if (null !== $id) { - $entity->load($id); + $entity->import($this->get_import_data()[$id]); } // Set the route @@ -202,7 +202,7 @@ public function test_unique_route_fails($id, $route) // Load the page from the db if it exists if (null !== $id) { - $entity->load($id); + $entity->import($this->get_import_data()[$id]); } // Set the route diff --git a/tests/entity/page_entity_save_test.php b/tests/entity/page_entity_save_test.php deleted file mode 100644 index 4f49091..0000000 --- a/tests/entity/page_entity_save_test.php +++ /dev/null @@ -1,119 +0,0 @@ - -* @license GNU General Public License, version 2 (GPL-2.0) -* -*/ - -namespace phpbb\pages\tests\entity; - -/** -* Tests related to save on page entity -*/ -class page_entity_save_test extends page_entity_base -{ - /** - * Test data for the test_save() function - * - * @return array Array of test data - */ - public function save_test_data() - { - return array( - array( - 1, - array( - 'page_id' => 1, - 'page_route' => 'new_route_1', - 'page_title' => 'new_title_1', - ), - ), - array( - 2, - array( - 'page_id' => 2, - 'page_route' => 'new_route_2', - 'page_title' => 'new_title_2', - ), - ), - ); - } - - /** - * Test saving data - * - * @dataProvider save_test_data - */ - public function test_save($id, $expected) - { - // Setup the entity class - $entity = $this->get_page_entity(); - - // Load the data - $result = $entity->load($id); - - // Assert the returned value is what we expect - self::assertInstanceOf('\phpbb\pages\entity\page', $result); - - // Set some new data - $entity - ->set_route($expected['page_route']) - ->set_title($expected['page_title']) - ->save() - ; - - // Re-load the data from the database - $result = $entity->load($id); - - // Assert expected matches actual - self::assertEquals($expected['page_id'], $result->get_id()); - self::assertEquals($expected['page_route'], $result->get_route()); - self::assertEquals($expected['page_title'], $result->get_title()); - } - - /** - * Test saving to (non-existent) pages from the database - */ - public function test_save_fails() - { - $this->expectException(\phpbb\pages\exception\out_of_bounds::class); - - // Setup the entity class - $entity = $this->get_page_entity(); - - // Save the entity with no rule ID set - $entity->save(); - } - - /** - * Test Unicode data is safely encoded for storage and decoded by the entity - */ - public function test_save_unicode_page_details() - { - $entity = $this->get_page_entity(); - $entity - ->load(1) - ->set_title('Emoji 😀 title') - ->set_description('中文 and Кириллица 😀 description') - ->save(); - - $result = $this->db->sql_query('SELECT page_title, page_description - FROM phpbb_pages - WHERE page_id = 1'); - $row = $this->db->sql_fetchrow($result); - $this->db->sql_freeresult($result); - - self::assertSame('Emoji 😀 title', $row['page_title']); - $expected_description = strpos($this->db->get_sql_layer(), 'mssql') === 0 - ? '中文 and Кириллица 😀 description' - : '中文 and Кириллица 😀 description'; - self::assertSame($expected_description, $row['page_description']); - - $entity->load(1); - self::assertSame('Emoji 😀 title', $entity->get_title()); - self::assertSame('中文 and Кириллица 😀 description', $entity->get_description()); - } -} diff --git a/tests/entity/page_entity_title_test.php b/tests/entity/page_entity_title_test.php index 621fe69..6efef6a 100644 --- a/tests/entity/page_entity_title_test.php +++ b/tests/entity/page_entity_title_test.php @@ -33,6 +33,10 @@ public function title_test_data() str_repeat('a', 200), str_repeat('a', 200), ), + array( + str_repeat('😀', 22), + str_repeat('😀', 22), + ), ); } @@ -72,6 +76,9 @@ public function title_fails_test_data() array( str_repeat('a', 201), ), + array( + str_repeat('😀', 23), + ), ); } diff --git a/tests/event/show_page_links_test.php b/tests/event/show_page_links_test.php index 8c09b6f..6df9294 100644 --- a/tests/event/show_page_links_test.php +++ b/tests/event/show_page_links_test.php @@ -56,7 +56,7 @@ public function test_show_page_links() ->willReturnCallback(function ($route, array $params = array()) { return $route . '#' . serialize($params); }); - $phpbb_container = $this->getMockBuilder('Symfony\Component\DependencyInjection\ContainerInterface') + $entity_factory = $this->getMockBuilder('\phpbb\pages\entity\factory') ->disableOriginalConstructor() ->getMock(); $router = $this->getMockBuilder('\phpbb\routing\router') @@ -70,7 +70,7 @@ public function test_show_page_links() $lang, new \phpbb\pages\operators\page( $cache, - $phpbb_container, + $entity_factory, $db, $ext_manager, $user, diff --git a/tests/operators/page_operator_add_page_test.php b/tests/operators/page_operator_add_page_test.php index 6b59d9a..b11c5d2 100644 --- a/tests/operators/page_operator_add_page_test.php +++ b/tests/operators/page_operator_add_page_test.php @@ -60,14 +60,9 @@ public function test_add_page_fails() { $this->expectException(\phpbb\pages\exception\base::class); - // Setup the entity class - $entity = $this->get_page_entity(); - - // Load an existing page data - $entity->load(1); - // Setup the operator class $operator = $this->get_page_operator(); + $entity = $operator->get_page(1); // Attempt to add the existing the page data $operator->add_page($entity); diff --git a/tests/operators/page_operator_base.php b/tests/operators/page_operator_base.php index 068af6e..0d05874 100644 --- a/tests/operators/page_operator_base.php +++ b/tests/operators/page_operator_base.php @@ -31,8 +31,8 @@ protected static function setup_extensions() /** @var \phpbb\config\config */ protected $config; - /** @var \PHPUnit\Framework\MockObject\MockObject|\Symfony\Component\DependencyInjection\ContainerInterface */ - protected $container; + /** @var \phpbb\pages\entity\factory */ + protected $entity_factory; /** @var \phpbb\db\driver\driver_interface */ protected $db; @@ -64,15 +64,9 @@ protected function setUp(): void global $config, $phpbb_dispatcher, $phpbb_root_path; $this->db = $this->new_dbal(); - $db = $this->db; - // Global vars called upon during execution $config = $this->config = new \phpbb\config\config(array()); - // mock container for the entity service - $this->container = $this->getMockBuilder('\Symfony\Component\DependencyInjection\ContainerInterface') - ->disableOriginalConstructor() - ->getMock(); $phpbb_dispatcher = $this->dispatcher = new \phpbb_mock_event_dispatcher(); $text_formatter_utils = $this->text_formatter_utils = new \phpbb\textformatter\s9e\utils(); $litedown = $this->litedown = $this->getMockBuilder('\phpbb\pages\textformatter\litedown') @@ -84,13 +78,14 @@ protected function setUp(): void $litedown->method('render')->willReturnCallback(function ($text) { return $text; }); - $this->container - ->method('get') - ->with('phpbb.pages.entity') - ->willReturnCallback(function () use ($db, $config, $phpbb_dispatcher, $text_formatter_utils, $litedown) { - return new \phpbb\pages\entity\page($db, $config, $phpbb_dispatcher, 'phpbb_pages', $text_formatter_utils, $litedown); - }) - ; + $this->entity_factory = new \phpbb\pages\entity\factory( + $this->db, + $config, + $phpbb_dispatcher, + 'phpbb_pages', + $text_formatter_utils, + $litedown + ); $this->cache = new \phpbb_mock_cache(); $this->user = $this->getMockBuilder('\phpbb\user') ->disableOriginalConstructor() @@ -116,7 +111,7 @@ protected function get_page_operator() { return new \phpbb\pages\operators\page( $this->cache, - $this->container, + $this->entity_factory, $this->db, $this->extension_manager, $this->user, diff --git a/tests/operators/page_operator_delete_page_test.php b/tests/operators/page_operator_delete_page_test.php index 6b5aec1..e9e0782 100644 --- a/tests/operators/page_operator_delete_page_test.php +++ b/tests/operators/page_operator_delete_page_test.php @@ -45,10 +45,7 @@ public function test_delete_page($page_id) // Try to load the deleted page try { - // Setup the entity class - $entity = new \phpbb\pages\entity\page($this->db, $this->config, $this->dispatcher, 'phpbb_pages', $this->text_formatter_utils, $this->litedown); - - $deleted = $entity->load($page_id); + $deleted = $operator->get_page($page_id); } catch (\phpbb\pages\exception\base $e) { diff --git a/tests/operators/page_operator_save_page_test.php b/tests/operators/page_operator_save_page_test.php new file mode 100644 index 0000000..0ffcc8a --- /dev/null +++ b/tests/operators/page_operator_save_page_test.php @@ -0,0 +1,77 @@ + +* @license GNU General Public License, version 2 (GPL-2.0) +* +*/ + +namespace phpbb\pages\tests\operators; + +class page_operator_save_page_test extends page_operator_base +{ + public function test_get_and_save_page() + { + $operator = $this->get_page_operator(); + $entity = $operator->get_page(1); + + self::assertSame('page_1', $entity->get_route()); + $entity->set_title('Changed title'); + + $saved = $operator->save_page($entity); + + self::assertSame('Changed title', $saved->get_title()); + self::assertSame(array(), $saved->get_changes()); + self::assertSame('Changed title', $operator->get_page(0, 'page_1')->get_title()); + } + + public function test_save_page_rejects_new_entity() + { + $this->expectException(\phpbb\pages\exception\out_of_bounds::class); + $this->expectExceptionMessage('page_id'); + + $this->get_page_operator()->save_page($this->entity_factory->create()); + } + + public function test_get_page_rejects_unknown_id() + { + $this->expectException(\phpbb\pages\exception\out_of_bounds::class); + $this->expectExceptionMessage('page_id'); + + $this->get_page_operator()->get_page(100); + } + + public function test_save_unicode_page_details() + { + $operator = $this->get_page_operator(); + $entity = $operator->get_page(1); + $entity + ->set_title('Emoji 😀 title') + ->set_description('中文 and Кириллица 😀 description'); + + $saved = $operator->save_page($entity); + + $result = $this->db->sql_query('SELECT page_title, page_description + FROM phpbb_pages + WHERE page_id = 1'); + $row = $this->db->sql_fetchrow($result); + $this->db->sql_freeresult($result); + + self::assertSame('Emoji 😀 title', $row['page_title']); + $expected_description = strpos($this->db->get_sql_layer(), 'mssql') === 0 + ? '中文 and Кириллица 😀 description' + : '中文 and Кириллица 😀 description'; + self::assertSame($expected_description, $row['page_description']); + self::assertSame('Emoji 😀 title', $saved->get_title()); + self::assertSame('中文 and Кириллица 😀 description', $saved->get_description()); + } + + public function test_create_page_returns_fresh_entities() + { + $operator = $this->get_page_operator(); + + self::assertNotSame($operator->create_page(), $operator->create_page()); + } +}