Skip to content

Commit 8ccd5b9

Browse files
iMattProiMattPro
authored
Merge pull request #188 from iMattPro/future-prrof
Future proof issues going into phpBB4
2 parents 9f3e1b7 + c604e81 commit 8ccd5b9

File tree

5 files changed

+298
-9
lines changed

5 files changed

+298
-9
lines changed

‎config/services.yml‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -81,16 +81,13 @@ services:
8181
tags:
8282
- { name: text_reparser.plugin }
8383

84+
phpbb.pages.cron.task.text_reparser.factory:
85+
class: phpbb\pages\textreparser\cron_text_reparser_factory
86+
8487
phpbb.pages.cron.task.text_reparser:
8588
class: phpbb\cron\task\text_reparser\reparser
89+
factory: ['@phpbb.pages.cron.task.text_reparser.factory', 'create']
8690
arguments:
87-
- '@config'
88-
- '@config_text'
89-
- '@text_reparser.lock'
90-
- '@text_reparser.manager'
91-
- '@text_reparser_collection'
92-
calls:
93-
- [set_name, [cron.task.text_reparser.phpbb_pages]]
94-
- [set_reparser, [phpbb.pages.text_reparser.page_text]]
91+
- '@service_container'
9592
tags:
9693
- { name: cron.task }

‎controller/admin_controller.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -336,7 +336,7 @@ protected function add_edit_page_data($entity)
336336
* @event phpbb.pages.acp_add_edit_page
337337
* @since 1.0.0-RC1
338338
*/
339-
$this->dispatcher->dispatch('phpbb.pages.acp_add_edit_page');
339+
$this->dispatcher->trigger_event('phpbb.pages.acp_add_edit_page');
340340

341341
// Set template vars for Page Template select menu
342342
$this->create_page_template_options($entity->get_template());

‎tests/functional/cron_reparser_test.php‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
<?php
2+
/**
3+
*
4+
* Pages extension for the phpBB Forum Software package.
5+
*
6+
* @copyright (c) 2025 phpBB Limited <https://www.phpbb.com>
7+
* @license GNU General Public License, version 2 (GPL-2.0)
8+
*
9+
*/
10+
11+
namespace functional;
12+
13+
use phpbb\pages\tests\functional\pages_functional_base;
14+
15+
/**
16+
* @group functional
17+
*/
18+
class cron_reparser_test extends pages_functional_base
19+
{
20+
/**
21+
* Test the cron reparser functionality
22+
*/
23+
public function test_cron_reparser()
24+
{
25+
$this->login();
26+
$this->admin_login();
27+
28+
// Store some of our data in variables
29+
$page_title = 'Cron Reparser Test Page';
30+
$page_content = '[b]This is a functional test page for the cron task reparser.[/b]';
31+
32+
// Create a test page
33+
$route = $this->create_page($page_title, $page_content);
34+
35+
// Go to the test page
36+
$crawler = self::request('GET', "app.php/$route?sid=$this->sid");
37+
$this->assertStringContainsString($page_title, $crawler->filter('h2')->text());
38+
39+
// Assert no reparsers have run yet
40+
$this->assertEmpty($this->get_reparser_resume());
41+
42+
// Run the cron task to reparse pages
43+
self::request('GET', "app.php/cron/cron.task.text_reparser.phpbb_pages", [], false);
44+
45+
// Try to ensure that the cron can actually run before we start to wait for it
46+
sleep(1);
47+
$cron_lock = new \phpbb\lock\db(
48+
'cron_lock',
49+
new \phpbb\config\db(
50+
$this->db,
51+
new \phpbb\cache\driver\dummy(),
52+
'phpbb_config'
53+
),
54+
$this->db
55+
);
56+
57+
// Add timeout to prevent infinite loop
58+
$timeout = time() + 30; // 30-second timeout
59+
while (!$cron_lock->acquire())
60+
{
61+
if (time() > $timeout)
62+
{
63+
$this->fail('Cron lock could not be acquired within 30 seconds');
64+
}
65+
usleep(100000); // Sleep for 100 ms between attempts
66+
}
67+
$cron_lock->release();
68+
69+
// Assert there's now a record of reparsing pages in the database
70+
$this->assertEquals(
71+
['phpbb.pages.text_reparser.page_text'],
72+
array_keys(unserialize($this->get_reparser_resume(), ['allowed_classes' => false]))
73+
);
74+
}
75+
76+
/**
77+
* Get the reparser resume data from the database
78+
*
79+
* @return string|null The config value or null if not found
80+
*/
81+
private function get_reparser_resume()
82+
{
83+
$sql = "SELECT config_value
84+
FROM " . CONFIG_TEXT_TABLE . "
85+
WHERE config_name = 'reparser_resume'";
86+
$result = $this->db->sql_query($sql);
87+
$row = $this->db->sql_fetchrow($result);
88+
$this->db->sql_freeresult($result);
89+
90+
return $row['config_value'] ?? null;
91+
}
92+
}

‎tests/text_reparser/cron_text_reparser_factory_test.php‎

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
<?php
2+
/**
3+
*
4+
* Pages extension for the phpBB Forum Software package.
5+
*
6+
* @copyright (c) 2025 phpBB Limited <https://www.phpbb.com>
7+
* @license GNU General Public License, version 2 (GPL-2.0)
8+
*
9+
*/
10+
11+
namespace phpbb\pages\tests\text_reparser;
12+
13+
use phpbb\pages\textreparser\cron_text_reparser_factory;
14+
use Symfony\Component\DependencyInjection\ContainerInterface;
15+
16+
class cron_text_reparser_factory_test extends \phpbb_test_case
17+
{
18+
/** @var \PHPUnit\Framework\MockObject\MockObject|ContainerInterface */
19+
protected $container;
20+
21+
/** @var cron_text_reparser_factory */
22+
protected $factory;
23+
24+
protected function setUp(): void
25+
{
26+
$this->container = $this->createMock(ContainerInterface::class);
27+
$this->factory = new cron_text_reparser_factory();
28+
}
29+
30+
public function test_create_reparser()
31+
{
32+
// Mock the necessary services
33+
$config = $this->createMock(\phpbb\config\config::class);
34+
$config_text = $this->createMock(\phpbb\config\db_text::class);
35+
$reparse_lock = $this->createMock(\phpbb\lock\db::class);
36+
$reparser_manager = $this->createMock(\phpbb\textreparser\manager::class);
37+
$reparsers = $this->createMock(\phpbb\di\service_collection::class);
38+
39+
// Configure a container to return our mocked services
40+
$this->container->method('has')
41+
->willReturnCallback(function($service) {
42+
$valid_services = [
43+
'config' => true,
44+
'config_text' => true,
45+
'text_reparser.lock' => true,
46+
'text_reparser.manager' => true,
47+
'text_reparser_collection' => true,
48+
];
49+
return $valid_services[$service] ?? false;
50+
});
51+
52+
$this->container->method('get')
53+
->willReturnCallback(function($service) use ($config, $config_text, $reparse_lock, $reparser_manager, $reparsers) {
54+
$services = [
55+
'config' => $config,
56+
'config_text' => $config_text,
57+
'text_reparser.lock' => $reparse_lock,
58+
'text_reparser.manager' => $reparser_manager,
59+
'text_reparser_collection' => $reparsers,
60+
];
61+
return $services[$service] ?? null;
62+
});
63+
64+
$reparser = $this->factory->create($this->container);
65+
66+
// Assert the reparser was created successfully
67+
$this->assertInstanceOf(\phpbb\cron\task\text_reparser\reparser::class, $reparser);
68+
$this->assertEquals('cron.task.text_reparser.phpbb_pages', $reparser->get_name());
69+
}
70+
71+
public function test_missing_required_service()
72+
{
73+
// Mock container to simulate missing service
74+
$this->container->method('has')
75+
->willReturn(false);
76+
77+
$this->container->method('get')
78+
->willReturn(null);
79+
80+
$this->expectException(\ArgumentCountError::class);
81+
$this->expectExceptionMessage('Too few arguments to function phpbb\cron\task\text_reparser\reparser::__construct(), 0 passed and exactly 5 expected');
82+
83+
$this->factory->create($this->container);
84+
}
85+
86+
public function test_service_name_mapping()
87+
{
88+
$reflection = new \ReflectionClass(cron_text_reparser_factory::class);
89+
$method = $reflection->getMethod('get_service_name');
90+
$method->setAccessible(true);
91+
92+
$expected_mappings = [
93+
'config' => 'config',
94+
'config_text' => 'config_text',
95+
'reparse_lock' => 'text_reparser.lock',
96+
'reparser_manager' => 'text_reparser.manager',
97+
'reparsers' => 'text_reparser_collection',
98+
'unknown_param' => 'unknown_param',
99+
];
100+
101+
foreach ($expected_mappings as $param => $expected)
102+
{
103+
$this->assertEquals(
104+
$expected,
105+
$method->invoke($this->factory, $param),
106+
"Service mapping failed for parameter '$param'"
107+
);
108+
}
109+
}
110+
}

‎textreparser/cron_text_reparser_factory.php‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
<?php
2+
/**
3+
*
4+
* Pages extension for the phpBB Forum Software package.
5+
*
6+
* @copyright (c) 2025 phpBB Limited <https://www.phpbb.com>
7+
* @license GNU General Public License, version 2 (GPL-2.0)
8+
*
9+
*/
10+
11+
namespace phpbb\pages\textreparser;
12+
13+
/**
14+
* Factory for creating cron text reparser task instances
15+
*
16+
* This factory handles the dynamic creation of text reparser tasks,
17+
* ensuring compatibility with different phpBB versions by using reflection
18+
* to determine and provide the correct constructor arguments.
19+
*/
20+
class cron_text_reparser_factory
21+
{
22+
/**
23+
* Creates a new instance of the text reparser cron task
24+
*
25+
* @param \Symfony\Component\DependencyInjection\ContainerInterface $container Service container
26+
* @return \phpbb\cron\task\text_reparser\reparser Configured a text reparser cron task
27+
* @throws \ReflectionException If the reparser class cannot be reflected
28+
*/
29+
public function create($container)
30+
{
31+
$args = $this->get_constructor_arguments($container);
32+
33+
// Using ReflectionClass to instantiate with a variable number of arguments
34+
$reflection = new \ReflectionClass(\phpbb\cron\task\text_reparser\reparser::class);
35+
$reparser = $reflection->newInstanceArgs($args);
36+
37+
$reparser->set_name('cron.task.text_reparser.phpbb_pages');
38+
$reparser->set_reparser('phpbb.pages.text_reparser.page_text');
39+
return $reparser;
40+
}
41+
42+
/**
43+
* Gets the constructor arguments for the reparser based on reflection
44+
*
45+
* @param \Symfony\Component\DependencyInjection\ContainerInterface $container Service container
46+
* @return array Array of constructor arguments
47+
* @throws \ReflectionException If the reparser class cannot be reflected
48+
*/
49+
private function get_constructor_arguments($container)
50+
{
51+
$reflection = new \ReflectionClass(\phpbb\cron\task\text_reparser\reparser::class);
52+
$constructor = $reflection->getConstructor();
53+
$params = $constructor->getParameters();
54+
55+
$arguments = array();
56+
foreach ($params as $param)
57+
{
58+
$service_name = $this->get_service_name($param->getName());
59+
if ($container->has($service_name))
60+
{
61+
$arguments[] = $container->get($service_name);
62+
}
63+
else if ($param->isOptional())
64+
{
65+
$arguments[] = $param->getDefaultValue();
66+
}
67+
}
68+
69+
return $arguments;
70+
}
71+
72+
/**
73+
* Maps parameter names to their corresponding service IDs
74+
*
75+
* @param string $param_name Name of the parameter from the constructor
76+
* @return string Service ID corresponding to the parameter
77+
*/
78+
private function get_service_name($param_name)
79+
{
80+
$serviceMap = [
81+
'config' => 'config',
82+
'config_text' => 'config_text',
83+
'reparse_lock' => 'text_reparser.lock',
84+
'reparser_manager' => 'text_reparser.manager',
85+
'reparsers' => 'text_reparser_collection'
86+
];
87+
88+
return $serviceMap[$param_name] ?? $param_name;
89+
}
90+
}

0 commit comments

Comments
 (0)