Skip to content

Commit

Permalink
Allow for wp_cache_add() and wp_cache_delete()
Browse files Browse the repository at this point in the history
This sniff checks that any direct database calls are accompanied by
caching. Up to now, only `wp_cache_get()` and `wp_cache_set()` were
checked for. The sniff requires that these two functions both be called
in order for the database call to pass.

But it is possible to use `wp_cache_add()` to add to the cache instead
of `wp_cache_set()`, so I have updated the sniff to provide for this,
checking for either set or add, but either must still be used along
with get.

There are also cases, such as when a complex `UPDATE` query must be run
with `$wpdb->query()`, where this form of get-set/add caching does not
apply. In such a case, it is necessary to call `wp_cache_delete()`
instead. So I have updated the sniff to allow for the `query()` method
to pass when accompanied by a cache delete **or** when accompanied by
cache get and set/add.

In the process I’ve realized that whenever the `update()`, `delete()`,
or `replace()` methods are used, there is also a need to clear the
cache. So I have moved these to the list of cacheable functions,
leaving only `insert()` as non-cacheable. Like `query()`, these
functions will pass the caching part of the sniff if they are
accompanied by a cache delete, or if they are accompanied by a cache
get and add/set. The reason that the latter is also allowed, is for the
case when the cache make be an array of things, only one of which is
being updated/deleted.

In the process, I found out that there was a method in the list,
`execute()` which does not exist as part of `$wpdb`. I don’t know where
it came from, but I’ve removed it from the list and added `delete()`,
which was previously absent.

For those who care, I think that I might even have squeaked a small
performance boost out of the sniff in the process.

Fixes #397
  • Loading branch information
JDGrimes committed Jun 5, 2015
1 parent 3bf09ea commit b7d5416
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 17 deletions.
42 changes: 26 additions & 16 deletions WordPress/Sniffs/VIP/DirectDatabaseQuerySniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
class WordPress_Sniffs_VIP_DirectDatabaseQuerySniff implements PHP_CodeSniffer_Sniff
{
protected $methods = array(
'cachable' => array( 'get_var', 'get_col', 'get_row', 'get_results', 'query' ),
'noncachable' => array( 'execute', 'insert', 'update', 'replace' ),
'cachable' => array( 'delete', 'get_var', 'get_col', 'get_row', 'get_results', 'query', 'replace', 'update' ),
'noncachable' => array( 'insert' ),
);

/**
Expand Down Expand Up @@ -95,35 +95,45 @@ public function process( PHP_CodeSniffer_File $phpcsFile, $stackPtr )
}

$whitelisted_cache = false;
$cached = false;
$cached = $wp_cache_get = false;
if ( preg_match( '/cache\s+(ok|pass|clear|whitelist)/i', $endOfLineComment, $matches ) ) {
$whitelisted_cache = true;
}
if ( ! $whitelisted_cache && ! empty( $tokens[$stackPtr]['conditions'] ) ) {
$conditions = $tokens[$stackPtr]['conditions'];
$scope_function = null;
foreach ( $conditions as $condPtr => $condType ) {
if ( $condType == T_FUNCTION ) {
$scope_function = $condPtr;
}
}
$scope_function = $phpcsFile->getCondition( $stackPtr, T_FUNCTION );

if ( $scope_function ) {
$scopeStart = $tokens[$scope_function]['scope_opener'];
$scopeEnd = $tokens[$scope_function]['scope_closer'];

$wpcacheget = $phpcsFile->findNext( array( T_STRING ), $scopeStart + 1, $stackPtr - 1, null, 'wp_cache_get' );
$wpcacheset = $phpcsFile->findNext( array( T_STRING ), $stackPtr + 1, $scopeEnd - 1, null, 'wp_cache_set' );
for ( $i = $scopeStart + 1; $i < $scopeEnd; $i++ ) {
if ( T_STRING === $tokens[ $i ]['code'] ) {

if ( 'wp_cache_delete' === $tokens[ $i ]['content'] ) {

if ( in_array( $method, array( 'query', 'update', 'replace', 'delete' ) ) ) {
$cached = true;
break;
}

if ( $wpcacheget && $wpcacheset ) {
$cached = true;
} elseif ( 'wp_cache_get' === $tokens[ $i ]['content'] ) {

$wp_cache_get = true;

} elseif ( in_array( $tokens[ $i ]['content'], array( 'wp_cache_set', 'wp_cache_add' ) ) ) {

if ( $wp_cache_get ) {
$cached = true;
break;
}
}
}
}
}

}

if ( ! $cached && ! $whitelisted_cache ) {
$message = 'Usage of a direct database call without caching is prohibited. Use wp_cache_get / wp_cache_set.';
$message = 'Usage of a direct database call without caching is prohibited. Use wp_cache_get / wp_cache_set or wp_cache_delete.';
$phpcsFile->addError( $message, $stackPtr, 'NoCaching' );
}

Expand Down
43 changes: 42 additions & 1 deletion WordPress/Tests/VIP/DirectDatabaseQueryUnitTest.inc
Original file line number Diff line number Diff line change
Expand Up @@ -61,4 +61,45 @@ function taz() {
/* @var wpdb $wpdb */
global $wpdb;
echo $wpdb->insert_id; // Good, no actual call, and doesn't need any caching
}
}

// Some $wpdb methods can pass with only deleting the cache.
function cache_delete_only() {
global $wpdb;

$data = $where = array();

// These methods are allowed to be used with just wp_cache_delete().
$wpdb->update( $wpdb->users, $data, $where ); // db call ok; OK
$wpdb->replace( $wpdb->users, $data, $where ); // db call ok; OK
$wpdb->delete( $wpdb->users, $data, $where ); // db call ok; OK
$wpdb->query( 'SELECT X FROM Y' ); // db call ok; OK

$wpdb->get_results( 'SELECT X FROM Y' ); // db call ok; Bad
$wpdb->get_row( 'SELECT X FROM Y' ); // db call ok; Bad
$wpdb->get_col( 'SELECT X FROM Y' ); // db call ok; Bad

wp_cache_delete( 'key', 'group' );
}

// It is OK to use the wp_cache_add() function in place of wp_cache_set().
function cache_add_instead_of_set() {
global $wpdb;

$baz = wp_cache_get( 'baz' );

if ( false !== $baz ) {

$data = $where = array();

$wpdb->update( $wpdb->users, $data, $where ); // db call ok; OK
$wpdb->replace( $wpdb->users, $data, $where ); // db call ok; OK
$wpdb->delete( $wpdb->users, $data, $where ); // db call ok; OK
$wpdb->query( 'SELECT X FROM Y' ); // db call ok; OK
$wpdb->get_row( 'SELECT X FROM Y' ); // db call ok; OK
$wpdb->get_col( 'SELECT X FROM Y' ); // db call ok; OK
$baz = $wpdb->get_results( $wpdb->prepare( 'SELECT X FROM Y ' ) ); // db call ok; OK

wp_cache_add( 'baz', $baz );
}
}
3 changes: 3 additions & 0 deletions WordPress/Tests/VIP/DirectDatabaseQueryUnitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ public function getErrorList()
32 => 1,
34 => 1,
50 => 1,
78 => 1,
79 => 1,
80 => 1,
);

}//end getErrorList()
Expand Down

0 comments on commit b7d5416

Please sign in to comment.