Skip to content
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

Closes #7125: PHPStan: Ensure no hooks are present inside the ORM logic #7152

Merged
merged 4 commits into from
Nov 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 15 additions & 0 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,11 @@ parameters:
count: 3
path: inc/Engine/Media/AboveTheFold/AJAX/Controller.php

-
message: "#^Hooks should not be used in ORM classes\\: WP_Rocket\\\\Engine\\\\Media\\\\AboveTheFold\\\\Database\\\\Queries\\\\AboveTheFold\\:\\:apply_filters$#"
count: 1
path: inc/Engine/Media/AboveTheFold/Database/Queries/AboveTheFold.php

-
message: "#^Usage of apply_filters\\(\\) is discouraged\\. Use wpm_apply_filters_typed\\(\\) instead\\.$#"
count: 1
Expand Down Expand Up @@ -270,6 +275,11 @@ parameters:
count: 1
path: inc/Engine/Optimization/LazyRenderContent/AJAX/Controller.php

-
message: "#^Hooks should not be used in ORM classes\\: WP_Rocket\\\\Engine\\\\Optimization\\\\LazyRenderContent\\\\Database\\\\Queries\\\\LazyRenderContent\\:\\:apply_filters$#"
count: 1
path: inc/Engine/Optimization/LazyRenderContent/Database/Queries/LazyRenderContent.php

-
message: "#^Usage of apply_filters\\(\\) is discouraged\\. Use wpm_apply_filters_typed\\(\\) instead\\.$#"
count: 1
Expand Down Expand Up @@ -355,6 +365,11 @@ parameters:
count: 5
path: inc/Engine/Preload/Cron/Subscriber.php

-
message: "#^Hooks should not be used in ORM classes\\: WP_Rocket\\\\Engine\\\\Preload\\\\Database\\\\Queries\\\\Cache\\:\\:apply_filters$#"
count: 4
path: inc/Engine/Preload/Database/Queries/Cache.php

-
message: "#^Usage of apply_filters\\(\\) is discouraged\\. Use wpm_apply_filters_typed\\(\\) instead\\.$#"
count: 4
Expand Down
2 changes: 2 additions & 0 deletions phpstan.neon.dist
Original file line number Diff line number Diff line change
Expand Up @@ -57,3 +57,5 @@ parameters:
rules:
- WP_Rocket\Tests\phpstan\Rules\DiscourageApplyFilters
- WP_Rocket\Tests\phpstan\Rules\EnsureCallbackMethodsExistsInSubscribedEvents
- WP_Rocket\Tests\phpstan\Rules\NoHooksInORM

55 changes: 55 additions & 0 deletions tests/phpstan/Rules/NoHooksInORM.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
<?php

namespace WP_Rocket\Tests\phpstan\Rules;

use PhpParser\Node;
use PhpParser\Node\Expr\FuncCall;
use PHPStan\Analyser\Scope;
use PHPStan\Reflection\ReflectionProvider;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleErrorBuilder;

class NoHooksInORM implements Rule
{
private $reflectionProvider;

public function __construct(ReflectionProvider $reflectionProvider)
{
$this->reflectionProvider = $reflectionProvider;
}

public function getNodeType(): string
{
return FuncCall::class;
}

public function processNode(Node $node, Scope $scope): array
{
if (!$node instanceof FuncCall) {
return [];
}

$functionName = $node->name;
if (!$functionName instanceof Node\Name) {
return [];
}

$functionName = $functionName->toString();
$hookFunctions = ['add_action', 'add_filter', 'do_action', 'apply_filters', 'wpm_apply_filters_typed', 'apply_filters_ref_array', 'wpm_apply_filters_typesafe'];

if (in_array($functionName, $hookFunctions, true)) {
$classReflection = $scope->getClassReflection();
if ($classReflection !== null) {
$className = $classReflection->getName();
$queryClassReflection = $this->reflectionProvider->getClass('WP_Rocket\Dependencies\BerlinDB\Database\Query');
if ($classReflection->isSubclassOf($queryClassReflection->getName())) {
return [
RuleErrorBuilder::message(sprintf('Hooks should not be used in ORM classes: %s::%s', $className, $functionName))->identifier('noHooksInORM')->build(),
];
}
}
}

return [];
}
}
32 changes: 32 additions & 0 deletions tests/phpstan/tests/Rules/NoHooksInORMTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<?php

namespace WP_Rocket\Tests\phpstan\tests\Rules;

use PHPStan\Testing\RuleTestCase;
use WP_Rocket\Tests\phpstan\Rules\NoHooksInORM;
use PHPStan\Reflection\ReflectionProvider;

class NoHooksInORMTest extends RuleTestCase
{
protected function getRule(): \PHPStan\Rules\Rule
{
$reflectionProvider = $this->createReflectionProvider();
return new NoHooksInORM($reflectionProvider);
}

public function testShouldReturnErrorBecauseOfHooks()
{
$this->analyse([__DIR__ . '/../data/NoHooksInORMTest/orm-class-with-hooks.php'], [
[
'Hooks should not be used in ORM classes: WP_Rocket\Tests\phpstan\tests\Rules\ORMWithHooks::apply_filters',
90,
],
]);
}


public function testShouldNotReturnError()
{
$this->analyse([__DIR__ . '/../data/NoHooksInORMTest/orm-class-without-hooks.php'], []);
}
}
101 changes: 101 additions & 0 deletions tests/phpstan/tests/data/NoHooksInORMTest/orm-class-with-hooks.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
<?php
declare(strict_types=1);

namespace WP_Rocket\Tests\phpstan\tests\Rules;

use WP_Rocket\Engine\Common\PerformanceHints\Database\Queries\AbstractQueries;
use WP_Rocket\Engine\Common\PerformanceHints\Database\Queries\QueriesInterface;
use WP_Rocket\Engine\Optimization\LazyRenderContent\Database\Schema\LazyRenderContent as LRCSchema;
use WP_Rocket\Engine\Optimization\LazyRenderContent\Database\Rows\LazyRenderContent as LRCRow;

class ORMWithHooks extends AbstractQueries implements QueriesInterface {
/**
* Name of the database table to query.
*
* @var string
*/
protected $table_name = 'wpr_lazy_render_content';

/**
* String used to alias the database table in MySQL statement.
*
* Keep this short, but descriptive. I.E. "tr" for term relationships.
*
* This is used to avoid collisions with JOINs.
*
* @var string
*/
protected $table_alias = 'wpr_lrc';

/**
* Name of class used to setup the database schema.
*
* @var string
*/
protected $table_schema = LRCSchema::class;

/** Item ******************************************************************/

/**
* Name for a single item.
*
* Use underscores between words. I.E. "term_relationship"
*
* This is used to automatically generate action hooks.
*
* @var string
*/
protected $item_name = 'lazy_render_content';

/**
* Plural version for a group of items.
*
* Use underscores between words. I.E. "term_relationships"
*
* This is used to automatically generate action hooks.
*
* @var string
*/
protected $item_name_plural = 'lazy_render_content';

/**
* Name of class used to turn IDs into first-class objects.
*
* This is used when looping through return values to guarantee their shape.
*
* @var mixed
*/
protected $item_shape = LRCRow::class;

/**
* Delete all rows which were not accessed in the last month.
*
* @return bool|int
*/
public function delete_old_rows() {
// Get the database interface.
$db = $this->get_db();

// Bail if no database interface is available.
if ( ! $db ) {
return false;
}

/**
* Filters the interval (in months) to determine when Below The Fold entry is considered 'old'.
* Old LRC entries are eligible for deletion. By default, LRC entry is considered old if it hasn't been accessed in the last month.
*
* @param int $delete_interval The interval in months after which LRC entry is considered old. Default is 1 month.
*/
$delete_interval = (int) apply_filters( 'rocket_lrc_cleanup_interval', 1 );

if ( $delete_interval <= 0 ) {
return false;
}

$prefixed_table_name = $db->prefix . $this->table_name;
$query = "DELETE FROM `$prefixed_table_name` WHERE status = 'failed' OR `last_accessed` <= date_sub(now(), interval $delete_interval month)";

return $db->query( $query );
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
<?php
declare(strict_types=1);

namespace WP_Rocket\Tests\phpstan\tests\Rules;

use WP_Rocket\Engine\Common\PerformanceHints\Database\Queries\AbstractQueries;
use WP_Rocket\Engine\Common\PerformanceHints\Database\Queries\QueriesInterface;
use WP_Rocket\Engine\Optimization\LazyRenderContent\Database\Schema\LazyRenderContent as LRCSchema;
use WP_Rocket\Engine\Optimization\LazyRenderContent\Database\Rows\LazyRenderContent as LRCRow;

class ORMWithoutHooks extends AbstractQueries implements QueriesInterface {
/**
* Name of the database table to query.
*
* @var string
*/
protected $table_name = 'wpr_lazy_render_content';

/**
* String used to alias the database table in MySQL statement.
*
* Keep this short, but descriptive. I.E. "tr" for term relationships.
*
* This is used to avoid collisions with JOINs.
*
* @var string
*/
protected $table_alias = 'wpr_lrc';

/**
* Name of class used to setup the database schema.
*
* @var string
*/
protected $table_schema = LRCSchema::class;

/** Item ******************************************************************/

/**
* Name for a single item.
*
* Use underscores between words. I.E. "term_relationship"
*
* This is used to automatically generate action hooks.
*
* @var string
*/
protected $item_name = 'lazy_render_content';

/**
* Plural version for a group of items.
*
* Use underscores between words. I.E. "term_relationships"
*
* This is used to automatically generate action hooks.
*
* @var string
*/
protected $item_name_plural = 'lazy_render_content';

/**
* Name of class used to turn IDs into first-class objects.
*
* This is used when looping through return values to guarantee their shape.
*
* @var mixed
*/
protected $item_shape = LRCRow::class;

/**
* Delete all rows which were not accessed in the last month.
*
* @return bool|int
*/
public function delete_old_rows() {
// Get the database interface.
$db = $this->get_db();

// Bail if no database interface is available.
if ( ! $db ) {
return false;
}

$delete_interval = 30;

if ( $delete_interval <= 0 ) {
return false;
}

$prefixed_table_name = $db->prefix . $this->table_name;
$query = "DELETE FROM `$prefixed_table_name` WHERE status = 'failed' OR `last_accessed` <= date_sub(now(), interval $delete_interval month)";

return $db->query( $query );
}
}
Loading