-
Notifications
You must be signed in to change notification settings - Fork 25
Symfony 7 and phpBB 4 compatibility #190
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.
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
Changes from 5 commits
2a229a1
9ac2d73
c3d4e01
e44d61d
787b886
5029457
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,78 +3,44 @@ | |
| * | ||
| * Pages extension for the phpBB Forum Software package. | ||
| * | ||
| * @copyright (c) 2015 phpBB Limited <https://www.phpbb.com> | ||
| * @copyright (c) 2015, 2025 phpBB Limited <https://www.phpbb.com> | ||
| * @license GNU General Public License, version 2 (GPL-2.0) | ||
| * | ||
| */ | ||
| // phpcs:disable PSR1.Files.SideEffects | ||
|
|
||
| namespace phpbb\pages\routing; | ||
|
|
||
| use phpbb\db\driver\driver_interface; | ||
| use Symfony\Component\Config\Loader\Loader; | ||
| use Symfony\Component\Routing\Route; | ||
| use Symfony\Component\Routing\RouteCollection; | ||
| use ReflectionMethod; | ||
| use Symfony\Component\Config\Loader\LoaderInterface; | ||
|
|
||
| if (!defined('IN_PHPBB')) | ||
| { | ||
| exit; | ||
| } | ||
|
|
||
| /** | ||
| * Loads routes defined in Page's database. | ||
| * This code determines which page_loader class to use based on the phpBB version. | ||
| * It checks if the Symfony LoaderInterface::load() method has a return type, | ||
| * which indicates Symfony 7+ (phpBB4), otherwise falls back to phpBB3 compatibility. | ||
| * The conditional is mandatory to ensure we only define the class if it does not | ||
| * already exist in this request. | ||
| * | ||
| * @noinspection PhpMultipleClassDeclarationsInspection | ||
| */ | ||
| class page_loader extends Loader | ||
| if (!class_exists(page_loader::class, false)) | ||
| { | ||
| /** @var driver_interface */ | ||
| protected $db; | ||
|
|
||
| /** @var string */ | ||
| protected $pages_table; | ||
| $method = new ReflectionMethod(LoaderInterface::class, 'load'); | ||
|
|
||
| /** | ||
| * Constructor | ||
| * | ||
| * @param driver_interface $db Database connection | ||
| * @param string $pages_table Table name | ||
| * @access public | ||
| */ | ||
| public function __construct(driver_interface $db, $pages_table) | ||
| // phpcs:disable PSR1.Classes.ClassDeclaration.MultipleClasses | ||
| if ($method->hasReturnType()) | ||
| { | ||
| $this->db = $db; | ||
| $this->pages_table = $pages_table; | ||
| class page_loader extends page_loader_phpbb4 {} | ||
| } | ||
|
|
||
| /** | ||
| * Loads routes defined in Page's database. | ||
| * | ||
| * @param string $resource Resource (not used, but required by parent interface) | ||
| * @param string|null $type The resource type | ||
| * | ||
| * @return RouteCollection A RouteCollection instance | ||
| * | ||
| * @api | ||
| */ | ||
| public function load($resource, $type = null) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could just return type be added instead? That should work for both
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The problem is the type hinting is in the arguments, which must match, but is not compatible then with phpBB3, and if its not hinted and matching, then its not compatible with phpBB4, so wee need the separate versions. It's: versus: |
||
| { | ||
| $collection = new RouteCollection(); | ||
|
|
||
| $sql = 'SELECT page_id, page_route | ||
| FROM ' . $this->pages_table; | ||
| $result = $this->db->sql_query($sql); | ||
| while ($row = $this->db->sql_fetchrow($result)) | ||
| { | ||
| $route = new Route('/' . $row['page_route']); | ||
| $route->setDefault('_controller', 'phpbb.pages.controller:display'); | ||
| $route->setDefault('route', $row['page_route']); | ||
| $collection->add('phpbb_pages_dynamic_route_' . $row['page_id'], $route); | ||
| } | ||
| $this->db->sql_freeresult(); | ||
|
|
||
| return $collection; | ||
| } | ||
|
|
||
| /** | ||
| * {@inheritdoc} | ||
| * | ||
| * @api | ||
| */ | ||
| public function supports($resource, $type = null) | ||
| else | ||
| { | ||
| return $type === 'phpbb_pages_route'; | ||
| class page_loader extends page_loader_phpbb3 {} | ||
| } | ||
|
iMattPro marked this conversation as resolved.
|
||
| // phpcs:enable PSR1.Classes.ClassDeclaration.MultipleClasses | ||
| } | ||
| // phpcs:enable PSR1.Files.SideEffects | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| <?php | ||
| /** | ||
| * | ||
| * Pages extension for the phpBB Forum Software package. | ||
| * | ||
| * @copyright (c) 2015, 2025 phpBB Limited <https://www.phpbb.com> | ||
| * @license GNU General Public License, version 2 (GPL-2.0) | ||
| * | ||
| */ | ||
|
|
||
| namespace phpbb\pages\routing; | ||
|
|
||
| use phpbb\db\driver\driver_interface; | ||
| use Symfony\Component\Routing\Route; | ||
| use Symfony\Component\Routing\RouteCollection; | ||
|
|
||
| /** | ||
| * Core loader logic for loading routes from the database. | ||
| */ | ||
| class page_loader_core | ||
| { | ||
| /** @var driver_interface */ | ||
| protected $db; | ||
|
|
||
| /** @var string */ | ||
| protected $pages_table; | ||
|
|
||
| /** | ||
| * Constructor for the page_loader_core class. | ||
| * | ||
| * @param driver_interface $db The database driver instance. | ||
| * @param string $pages_table The name of the pages table in the database. | ||
| */ | ||
| public function __construct(driver_interface $db, string $pages_table) | ||
| { | ||
| $this->db = $db; | ||
| $this->pages_table = $pages_table; | ||
| } | ||
|
|
||
| /** | ||
| * Loads routes defined in Page's database. | ||
| * | ||
| * @return RouteCollection A RouteCollection instance | ||
| */ | ||
| public function load_routes(): RouteCollection | ||
| { | ||
| $collection = new RouteCollection(); | ||
|
|
||
| $sql = 'SELECT page_id, page_route | ||
| FROM ' . $this->pages_table; | ||
| $result = $this->db->sql_query($sql); | ||
|
iMattPro marked this conversation as resolved.
|
||
| while ($row = $this->db->sql_fetchrow($result)) | ||
| { | ||
| $route = new Route('/' . $row['page_route']); | ||
| $route->setDefault('_controller', 'phpbb.pages.controller:display'); | ||
| $route->setDefault('route', $row['page_route']); | ||
| $collection->add('phpbb_pages_dynamic_route_' . $row['page_id'], $route); | ||
| } | ||
| $this->db->sql_freeresult($result); | ||
|
|
||
| return $collection; | ||
| } | ||
|
|
||
| /** | ||
| * Checks if the loader supports the specified type. | ||
| * | ||
| * @param mixed $type The type to check support for. | ||
| * @return bool True if the type is supported, false otherwise. | ||
| */ | ||
| public function supports_type($type): bool | ||
| { | ||
| return $type === 'phpbb_pages_route'; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| <?php | ||
| /** | ||
| * | ||
| * Pages extension for the phpBB Forum Software package. | ||
| * | ||
| * @copyright (c) 2015, 2025 phpBB Limited <https://www.phpbb.com> | ||
| * @license GNU General Public License, version 2 (GPL-2.0) | ||
| * | ||
| */ | ||
|
|
||
| namespace phpbb\pages\routing; | ||
|
|
||
| use InvalidArgumentException; | ||
| use phpbb\db\driver\driver_interface; | ||
| use Symfony\Component\Config\Loader\Loader; | ||
| use Symfony\Component\Routing\RouteCollection; | ||
|
|
||
| /** | ||
| * phpBB 3 and Symfony 3 through 6 adapter for page loader. | ||
| */ | ||
| class page_loader_phpbb3 extends Loader | ||
| { | ||
| /** @var page_loader_core */ | ||
| protected $core; | ||
|
|
||
| /** | ||
| * Constructor for the page_loader_phpbb3 class. | ||
| * | ||
| * @param driver_interface $db The database driver instance. | ||
| * @param string $pages_table The name of the pages table in the database. | ||
| */ | ||
| public function __construct(driver_interface $db, string $pages_table) | ||
| { | ||
|
iMattPro marked this conversation as resolved.
|
||
| $this->core = new page_loader_core($db, $pages_table); | ||
| } | ||
|
|
||
| /** | ||
| * Loads routes from the database. | ||
| * | ||
| * @param mixed $resource The resource to load routes from. | ||
| * @param string|null $type The type of the resource, or null if not specified. | ||
| * @return RouteCollection The collection of loaded routes. | ||
| */ | ||
| public function load($resource, $type = null) | ||
| { | ||
| if (!is_string($type) && $type !== null) | ||
| { | ||
| throw new InvalidArgumentException('Type must be string or null'); | ||
| } | ||
|
iMattPro marked this conversation as resolved.
|
||
| return $this->core->load_routes(); | ||
| } | ||
|
|
||
| /** | ||
| * Determines if the given resource is supported based on its type. | ||
| * | ||
| * @param mixed $resource The resource to be checked. | ||
| * @param string|null $type The type of the resource, or null for default processing. | ||
| * @return bool True if the resource type is supported, false otherwise. | ||
| */ | ||
| public function supports($resource, $type = null): bool | ||
| { | ||
| return $this->core->supports_type($type); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| <?php | ||
| /** | ||
| * | ||
| * Pages extension for the phpBB Forum Software package. | ||
| * | ||
| * @copyright (c) 2015, 2025 phpBB Limited <https://www.phpbb.com> | ||
| * @license GNU General Public License, version 2 (GPL-2.0) | ||
| * | ||
| */ | ||
|
|
||
| namespace phpbb\pages\routing; | ||
|
|
||
| use phpbb\db\driver\driver_interface; | ||
| use Symfony\Component\Config\Loader\Loader; | ||
| use Symfony\Component\Routing\RouteCollection; | ||
|
|
||
| /** | ||
| * phpBB 4 and Symfony 7 adapter for page loader. | ||
| */ | ||
| class page_loader_phpbb4 extends Loader | ||
| { | ||
| /** @var page_loader_core */ | ||
| protected page_loader_core $core; | ||
|
|
||
| /** | ||
| * Constructor for the page_loader_phpbb4 class. | ||
| * | ||
| * @param driver_interface $db The database driver instance. | ||
| * @param string $pages_table The name of the pages table in the database. | ||
| */ | ||
| public function __construct(driver_interface $db, string $pages_table) | ||
| { | ||
| $this->core = new page_loader_core($db, $pages_table); | ||
| parent::__construct(); | ||
| } | ||
|
|
||
| /** | ||
| * Loads a set of routes from a specified resource. | ||
| * | ||
| * @param mixed $resource The resource to load routes from. | ||
| * @param string|null $type The type of the resource, or null if not specified. | ||
| * @return RouteCollection The collection of loaded routes. | ||
| */ | ||
| public function load(mixed $resource, ?string $type = null): RouteCollection | ||
| { | ||
| return $this->core->load_routes(); | ||
| } | ||
|
|
||
| /** | ||
| * Checks if the loader supports the specified resource and type. | ||
| * | ||
| * @param mixed $resource The resource to check support for. | ||
| * @param string|null $type The type of the resource, or null if not specified. | ||
| * @return bool True if the loader supports the resource and type, false otherwise. | ||
| */ | ||
| public function supports(mixed $resource, ?string $type = null): bool | ||
| { | ||
| return $this->core->supports_type($type); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| <?php | ||
| /** | ||
| * | ||
| * Pages extension for the phpBB Forum Software package. | ||
| * | ||
| * @copyright (c) 2025 phpBB Limited <https://www.phpbb.com> | ||
| * @license GNU General Public License, version 2 (GPL-2.0) | ||
| * | ||
| */ | ||
|
|
||
| namespace phpbb\pages\tests\routing; | ||
|
|
||
| class page_loader_core_test extends \phpbb_database_test_case | ||
| { | ||
| protected static function setup_extensions() | ||
| { | ||
| return array('phpbb/pages'); | ||
| } | ||
|
|
||
| /** @var \phpbb\db\driver\driver_interface */ | ||
| protected $db; | ||
|
|
||
| /** @var \phpbb\pages\routing\page_loader_core */ | ||
| protected $loader; | ||
|
|
||
| public function getDataSet() | ||
| { | ||
| return $this->createXMLDataSet(__DIR__ . '/../operators/fixtures/page.xml'); | ||
| } | ||
|
|
||
| protected function setUp(): void | ||
| { | ||
| parent::setUp(); | ||
| $this->db = $this->new_dbal(); | ||
| $this->loader = new \phpbb\pages\routing\page_loader_core($this->db, 'phpbb_pages'); | ||
| } | ||
|
|
||
| public function test_load_routes_returns_collection() | ||
| { | ||
| $collection = $this->loader->load_routes(); | ||
| self::assertInstanceOf('Symfony\Component\Routing\RouteCollection', $collection); | ||
| } | ||
|
|
||
| public static function route_data() | ||
| { | ||
| return array( | ||
| array(1, '/page_1'), | ||
| array(2, '/page_2'), | ||
| array(3, '/page_3'), | ||
| array(4, '/page_4'), | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * @dataProvider route_data | ||
| */ | ||
| public function test_routes_have_correct_paths($id, $expected) | ||
| { | ||
| $collection = $this->loader->load_routes(); | ||
| $route = $collection->get('phpbb_pages_dynamic_route_' . $id); | ||
| self::assertInstanceOf('Symfony\Component\Routing\Route', $route); | ||
| self::assertSame($expected, $route->getPath()); | ||
| } | ||
|
|
||
| public function test_supports_type() | ||
| { | ||
| self::assertTrue($this->loader->supports_type('phpbb_pages_route')); | ||
| self::assertFalse($this->loader->supports_type('other_type')); | ||
| self::assertFalse($this->loader->supports_type(null)); | ||
| } | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.