Merge pull request #647 from rannmann/prunebans-fix#647
Conversation
| $dsn = 'mysql:host=' . $host . ';port=' . $port . ';dbname=' . $dbname . ';charset=' . $charset; | ||
| $options = array( | ||
| \PDO::ATTR_ERRMODE => \PDO::ERRMODE_EXCEPTION | ||
| PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION |
There was a problem hiding this comment.
We're already at a global scope. The root notation is superfluous.
| public function execute(?array $inputParams = null) | ||
| { | ||
| return $this->stmt->execute(); | ||
| return $this->stmt->execute($inputParams); |
There was a problem hiding this comment.
We can now bind nameless params at execution time.
| * @param int $fetchType | ||
| * @return mixed | ||
| */ | ||
| public function resultset(?array $inputParams = null, $fetchType = PDO::FETCH_ASSOC) |
There was a problem hiding this comment.
We can now bind nameless params at execution time, and choose the fetch mode ourselves.
| * @param int $fetchType | ||
| * @return mixed | ||
| */ | ||
| public function single(?array $inputParams = null, $fetchType = PDO::FETCH_ASSOC) |
There was a problem hiding this comment.
We can now bind nameless params at execution time, and choose the fetch mode ourselves.
| * @param string $tooltip The tooltip message | ||
| * @param string $target The new links target | ||
| * @param bool $wide | ||
| * @param string $onclick |
There was a problem hiding this comment.
Added missing params from phpdoc
| } | ||
|
|
||
| return $out; | ||
| return isset($out) ? $out : false; |
There was a problem hiding this comment.
Fixes potential php warning about $out not being defined.
| } | ||
|
|
||
| return $out; | ||
| return isset($out) ? $out : false; |
There was a problem hiding this comment.
Fixes potential php warning about $out not being defined.
| $GLOBALS['PDO']->bind(':id', $userbank->GetAid() < 0 ? 0 : $userbank->GetAid()); | ||
| $GLOBALS['PDO']->bind(':id', $adminId); | ||
| $GLOBALS['PDO']->execute(); | ||
|
|
| */ | ||
| function getDirSize($dir) | ||
| { | ||
| $size = 0; |
There was a problem hiding this comment.
Fixes PHP warning about size not being defined if the foreach loop doesn't run
| if (!$ret) | ||
| return false; | ||
| if (!$ret) { | ||
| return []; |
There was a problem hiding this comment.
This method is only used in 1 place, and the result is always treated like an array. It was never checked for a false value.
|
|
||
| $output = $rcon->Rcon($cmd); | ||
| } catch (AuthenticationException $e) { | ||
| } catch (\xPaw\SourceQuery\Exception\AuthenticationException $e) { |
There was a problem hiding this comment.
Class was unknown. Not sure what happened in the past if this code was ever reached.
| $GLOBALS['PDO']->execute(); | ||
|
|
||
| Log::add('e', "Rcon Password Error [ServerID: $sid]", $e->getMessage); | ||
| Log::add('e', "Rcon Password Error [ServerID: $sid]", $e->getMessage()); |
There was a problem hiding this comment.
Exceptions don't have this property, but they do have this method.
|
|
||
| preg_match_all($regex, $status, $status, PREG_SET_ORDER); | ||
| $result = []; | ||
| preg_match_all($regex, $status, $result, PREG_SET_ORDER); |
There was a problem hiding this comment.
Don't put the results back into the original variable that held the string. This fixes PHPStorm's warnings.
This reverts commit 84406f7.
Description
Primarily fixes #644. Cleans up PHPDoc in a few files, fixes some namespace issues, fixes some bad return types, and adds some minor functionality to the Database class so we can use more PDO features (used in PruneBans)
Motivation and Context
Database query for prune bans related to the submissions table was very slow. Split this query into multiple queries to fix it. See #644.
How Has This Been Tested?
Plugged a copy of a production database into these changes and loaded all the major pages, focusing mostly on the bans page. I'm running PHP 7.3.13, but my IDE is configured to provide notices for PHP 7.1 as per the minimum requirements of this project.
PruneBans() is only called in 3 places, but takes no parameters so testing it in one context should be sufficient.
Screenshots (if appropriate):
N/A
Types of changes
Checklist: