Skip to content

Refactor entites and operators #203

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
iMattPro wants to merge 1 commit into from
Closed

Refactor entites and operators #203

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions acp/pages_module.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 5 additions & 6 deletions config/services.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ services:
- '@request'
- '@template'
- '@user'
- '@service_container'
- '@pagination'
- '@dispatcher'
- '%core.root_path%'
- '%core.php_ext%'
Expand All @@ -50,7 +50,7 @@ services:
class: phpbb\pages\controller\main_controller
arguments:
- '@auth'
- '@service_container'
- '@phpbb.pages.operator'
- '@controller.helper'
- '@language'
- '@template'
Expand Down Expand Up @@ -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'
Comment on lines +113 to 117
Expand All @@ -125,7 +124,7 @@ services:
class: phpbb\pages\operators\page
arguments:
- '@cache.driver'
- '@service_container'
- '@phpbb.pages.entity_factory'
- '@dbal.conn'
- '@ext.manager'
- '@user'
Expand Down
91 changes: 60 additions & 31 deletions controller/admin_controller.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@

namespace phpbb\pages\controller;

use Symfony\Component\DependencyInjection\ContainerInterface;

/**
* Admin controller
*/
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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 */
Expand All @@ -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(
Expand All @@ -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(
Expand All @@ -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(
Expand All @@ -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)
{
Expand Down Expand Up @@ -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']);
Expand Down Expand Up @@ -406,19 +424,19 @@ 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);
}
catch (\phpbb\pages\exception\base $e)
{
// 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
Expand Down Expand Up @@ -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));

Expand All @@ -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,
));
}
}
Expand Down Expand Up @@ -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);
}
}
29 changes: 12 additions & 17 deletions controller/main_controller.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

namespace phpbb\pages\controller;

use Symfony\Component\DependencyInjection\ContainerInterface;
use phpbb\exception\http_exception;

/**
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -104,22 +103,18 @@ 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)
{
throw new http_exception(404, 'PAGE_NOT_AVAILABLE', array($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));
}
Expand Down
Loading