Fix %TOTALPAGES% caching
#175
Conversation
|
@coderabbitai review |
Actions PerformedReview triggered.
|
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe recent changes streamline the handling of row counts in the Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- includes/Parse.php (3 hunks)
- includes/Query.php (3 hunks)
Additional Context Used
Path-based Instructions (2)
includes/Parse.php (1)
Pattern
**/*.php: Follow MediaWiki PHP Coding Standards:Indentation:
- Use tabs for indentation, not spaces.
Spaces:
- Use spaces on either side of binary operators.
Example: $a = $b + $c; (Correct) $a=$b+$c; (Incorrect)- Add spaces inside parentheses, except where they are empty.
Example: $a = getFoo( $b ); (Correct) $a = getFoo($b); (Incorrect)- Do not add a space after a function name.
Example: $c = getBar(); (Correct) $c = getBar (); (Incorrect)Function Return Type Hint:
- Add a space after the colon in the function return type hint.
Example: function square( int $x ): int { return $x * $x; }Arrays:
- Use spaces in brackets when declaring an array, except where the array is empty.
Example: $a = [ 'foo', 'bar' ]; (Correct) $a = ['foo', 'bar']; (Incorrect)- Do not put spaces in brackets when accessing array elements.
Example: $c = $a[0]; (Correct) $c = $a[ 0 ]; (Incorrect)Control Structures:
- Add a space after control structure keywords.
Example: if ( isFoo() ) { ... } (Correct) if(isFoo()){ ... } (Incorrect)Type Casting:
- Do not use a space within or after the cast operator.
Example: (int)$foo; (Correct) (int) $bar; (Incorrect)Comments:
- In comments, there should be one space between the # or // and the comment text.
Example: // This is a comment. (Correct) //This is a comment. (Incorrect)Ternary Operator:
- Keep ternary operator expressions short and obvious.
- For multi-line ternary expressions, place the '?' and ':' at the beginning of the new lines.
File Formatting:
- Avoid trailing whitespace on lines.includes/Query.php (1)
Pattern
**/*.php: Follow MediaWiki PHP Coding Standards:Indentation:
- Use tabs for indentation, not spaces.
Spaces:
- Use spaces on either side of binary operators.
Example: $a = $b + $c; (Correct) $a=$b+$c; (Incorrect)- Add spaces inside parentheses, except where they are empty.
Example: $a = getFoo( $b ); (Correct) $a = getFoo($b); (Incorrect)- Do not add a space after a function name.
Example: $c = getBar(); (Correct) $c = getBar (); (Incorrect)Function Return Type Hint:
- Add a space after the colon in the function return type hint.
Example: function square( int $x ): int { return $x * $x; }Arrays:
- Use spaces in brackets when declaring an array, except where the array is empty.
Example: $a = [ 'foo', 'bar' ]; (Correct) $a = ['foo', 'bar']; (Incorrect)- Do not put spaces in brackets when accessing array elements.
Example: $c = $a[0]; (Correct) $c = $a[ 0 ]; (Incorrect)Control Structures:
- Add a space after control structure keywords.
Example: if ( isFoo() ) { ... } (Correct) if(isFoo()){ ... } (Incorrect)Type Casting:
- Do not use a space within or after the cast operator.
Example: (int)$foo; (Correct) (int) $bar; (Incorrect)Comments:
- In comments, there should be one space between the # or // and the comment text.
Example: // This is a comment. (Correct) //This is a comment. (Incorrect)Ternary Operator:
- Keep ternary operator expressions short and obvious.
- For multi-line ternary expressions, place the '?' and ':' at the beginning of the new lines.
File Formatting:
- Avoid trailing whitespace on lines.
Additional comments not posted (6)
includes/Parse.php (2)
204-204: Ensure proper error handling for thebuildAndSelectmethod call.Verification successful
The
buildAndSelectmethod inincludes/Query.phpincludes proper error handling by checking the success of query modification functions and throwing anMWExceptionif an error occurs.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the `buildAndSelect` method properly handles errors and exceptions. # Test: Search for the method implementation. Expect: Proper error handling mechanisms. ast-grep --lang php --pattern $'class Query { $$$ buildAndSelect($_, $_) { $$$ } $$$ }'Length of output: 209
Script:
#!/bin/bash # Description: Locate the `buildAndSelect` method implementation and verify its error handling. # Search for the `buildAndSelect` method definition in the codebase. rg 'function buildAndSelect' -t phpLength of output: 116
Script:
#!/bin/bash # Description: Extract the implementation of the `buildAndSelect` method to verify its error handling. # Extract the `buildAndSelect` method implementation from the `includes/Query.php` file. rg -A 20 'function buildAndSelect' includes/Query.phpLength of output: 768
267-267: Consider verifying the existence ofgetRowCountmethod inListerclass.Verification successful
The
getRowCountmethod does exist in theListerclass.
- Location:
includes/Lister/Lister.phpScripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the `getRowCount` method exists in the `Lister` class. # Test: Search for the method implementation. Expect: Method definition in the `Lister` class. ast-grep --lang php --pattern $'class Lister { $$$ getRowCount() { $$$ } $$$ }'Length of output: 201
Script:
#!/bin/bash # Description: Verify that the `getRowCount` method exists in the `Lister` class. # Search for the `Lister` class definition and then look for the `getRowCount` method within it. rg -A 20 'class Lister' | rg 'function getRowCount'Length of output: 53
Script:
#!/bin/bash # Description: Verify the methods in the `Lister` class. # Search for the `Lister` class definition and list all its methods. rg -A 50 'class Lister' | rg 'function 'Length of output: 42
Script:
#!/bin/bash # Description: Verify the existence of the `Lister` class. # Search for the `Lister` class definition in the codebase. rg 'class Lister'Length of output: 61
Script:
#!/bin/bash # Description: Verify the existence of the `getRowCount` method in the `Lister` class. # Search for the `getRowCount` method in the `includes/Lister/Lister.php` file. rg 'function getRowCount' includes/Lister/Lister.phpLength of output: 87
includes/Query.php (4)
171-171: Method signature change inbuildAndSelect.The removal of the
?int &$foundRowsparameter simplifies the method signature and internalizes the row counting logic, aligning with the PR's objectives to manage caching more effectively.
378-378: Use of$dbr->selectFieldforFOUND_ROWS().Switching to
$dbr->selectFieldfor retrieving the count of found rows is a more efficient approach than executing a separate query, which should help in reducing overhead and improving performance.
Line range hint
171-381: Complex SQL query construction logic.The method effectively handles a variety of parameters to construct SQL queries dynamically. Exception handling for potential SQL build errors is appropriately managed, which is crucial for maintaining stability.
398-398: Caching mechanism implementation.The implementation of caching using
MediaWikiServices::getInstance()->getMainWANObjectCache()and handling with a pool counter is well-designed. This approach should improve performance by reducing database load and managing concurrent query executions effectively.
There was a problem hiding this comment.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- includes/Parse.php (3 hunks)
- includes/Query.php (3 hunks)
Files skipped from review as they are similar to previous changes (2)
- includes/Parse.php
- includes/Query.php
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| * @param string $profilingContext Used to see the origin of a query in the profiling | ||
| * @return array|bool | ||
| */ | ||
| public function buildAndSelect( bool $calcRows = false, ?int &$foundRows = null, $profilingContext = '' ) { | ||
| public function buildAndSelect( bool $calcRows = false, $profilingContext = '' ) { |
Check notice
Code scanning / Phpmd (reported by Codacy)
This pattern reports excessively long methods. Note
| * @param string $profilingContext Used to see the origin of a query in the profiling | ||
| * @return array|bool | ||
| */ | ||
| public function buildAndSelect( bool $calcRows = false, ?int &$foundRows = null, $profilingContext = '' ) { | ||
| public function buildAndSelect( bool $calcRows = false, $profilingContext = '' ) { |
Check notice
Code scanning / Phpmd (reported by Codacy)
This pattern reports methods with a large number of possible paths Note
| * @param string $profilingContext Used to see the origin of a query in the profiling | ||
| * @return array|bool | ||
| */ | ||
| public function buildAndSelect( bool $calcRows = false, ?int &$foundRows = null, $profilingContext = '' ) { | ||
| public function buildAndSelect( bool $calcRows = false, $profilingContext = '' ) { |
Check notice
Code scanning / Phpmd (reported by Codacy)
This pattern reports methods with high cyclomatic complexity Note
| * @param string $profilingContext Used to see the origin of a query in the profiling | ||
| * @return array|bool | ||
| */ | ||
| public function buildAndSelect( bool $calcRows = false, ?int &$foundRows = null, $profilingContext = '' ) { | ||
| public function buildAndSelect( bool $calcRows = false, $profilingContext = '' ) { |
Check warning
Code scanning / Phpmd (reported by Codacy)
You can fix this problem by extracting the logic in the boolean flag into its own class or method Warning
| static function ( $oldVal, &$ttl, &$setOpts ) use ( $worker, $db ){ | ||
| $setOpts += Database::getCacheSetOptions( $db ); | ||
| static function ( $oldVal, &$ttl, &$setOpts ) use ( $worker, $dbr ){ | ||
| $setOpts += Database::getCacheSetOptions( $dbr ); |
Check warning
Code scanning / Phpmd (reported by Codacy)
Static access leads to hard to test code Warning
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Check commit and GitHub actions for more details
Fixes #144 (comment)
Summary by CodeRabbit