phpmyadmin /
error-reporting-server
Complex classes like IncidentsTable often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.
Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.
While breaking up the class, it is a good idea to analyze how other classes use IncidentsTable, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 30 | class IncidentsTable extends Table |
||
| 31 | { |
||
| 32 | /** |
||
| 33 | * @var array |
||
| 34 | * |
||
| 35 | * @see http://book.cakephp.org/2.0/en/models/behaviors.html#using-behaviors |
||
| 36 | * @see Model::$actsAs |
||
| 37 | */ |
||
| 38 | public $actsAs = array('Summarizable'); |
||
| 39 | |||
| 40 | /** |
||
| 41 | * @var array |
||
| 42 | * |
||
| 43 | * @see http://book.cakephp.org/2.0/en/models/model-attributes.html#validate |
||
| 44 | * @see http://book.cakephp.org/2.0/en/models/data-validation.html |
||
| 45 | * @see Model::$validate |
||
| 46 | */ |
||
| 47 | public $validate = array( |
||
| 48 | 'pma_version' => array( |
||
| 49 | 'rule' => 'notEmpty', |
||
| 50 | 'required' => true, |
||
| 51 | ), |
||
| 52 | 'php_version' => array( |
||
| 53 | 'rule' => 'notEmpty', |
||
| 54 | 'required' => true, |
||
| 55 | ), |
||
| 56 | 'full_report' => array( |
||
| 57 | 'rule' => 'notEmpty', |
||
| 58 | 'required' => true, |
||
| 59 | ), |
||
| 60 | 'stacktrace' => array( |
||
| 61 | 'rule' => 'notEmpty', |
||
| 62 | 'required' => true, |
||
| 63 | ), |
||
| 64 | 'browser' => array( |
||
| 65 | 'rule' => 'notEmpty', |
||
| 66 | 'required' => true, |
||
| 67 | ), |
||
| 68 | 'stackhash' => array( |
||
| 69 | 'rule' => 'notEmpty', |
||
| 70 | 'required' => true, |
||
| 71 | ), |
||
| 72 | 'user_os' => array( |
||
| 73 | 'rule' => 'notEmpty', |
||
| 74 | 'required' => true, |
||
| 75 | ), |
||
| 76 | 'locale' => array( |
||
| 77 | 'rule' => 'notEmpty', |
||
| 78 | 'required' => true, |
||
| 79 | ), |
||
| 80 | 'script_name' => array( |
||
| 81 | 'rule' => 'notEmpty', |
||
| 82 | 'required' => true, |
||
| 83 | ), |
||
| 84 | 'server_software' => array( |
||
| 85 | 'rule' => 'notEmpty', |
||
| 86 | 'required' => true, |
||
| 87 | ), |
||
| 88 | 'configuration_storage' => array( |
||
| 89 | 'rule' => 'notEmpty', |
||
| 90 | 'required' => true, |
||
| 91 | ), |
||
| 92 | ); |
||
| 93 | |||
| 94 | /** |
||
| 95 | * @var array |
||
| 96 | * |
||
| 97 | * @see http://book.cakephp.org/2.0/en/models/associations-linking-models-together.html#belongsto |
||
| 98 | * @see Model::$belongsTo |
||
| 99 | */ |
||
| 100 | |||
| 101 | /** |
||
| 102 | * The fields which are summarized in the report page with charts and are also |
||
| 103 | * used in the overall stats and charts for the website. |
||
| 104 | * |
||
| 105 | * @var array |
||
| 106 | */ |
||
| 107 | public $summarizableFields = array( |
||
| 108 | 'browser', 'pma_version', 'php_version', |
||
| 109 | 'locale', 'server_software', 'user_os', 'script_name', |
||
| 110 | 'configuration_storage', |
||
| 111 | ); |
||
| 112 | |||
| 113 | 24 | public function __construct($id = false, $table = null, $ds = null) |
|
| 145 | |||
| 146 | /** |
||
| 147 | * creates an incident/report record given a raw bug report object. |
||
| 148 | * |
||
| 149 | * This gets a decoded bug report from the submitted json body. This has not |
||
| 150 | * yet been santized. It either adds it as an incident to another report or |
||
| 151 | * creates a new report if nothing matches. |
||
| 152 | * |
||
| 153 | * @param array $bugReport the bug report being submitted |
||
| 154 | * |
||
| 155 | * @return array of: |
||
| 156 | * 1. array of inserted incident ids. If the report/incident was not |
||
| 157 | * correctly saved, false is put in it place. |
||
| 158 | * 2. array of newly created report ids. If no new report was created, |
||
| 159 | * an empty array is returned |
||
| 160 | */ |
||
| 161 | 2 | public function createIncidentFromBugReport($bugReport) |
|
| 233 | |||
| 234 | /** |
||
| 235 | * retrieves the closest report to a given bug report. |
||
| 236 | * |
||
| 237 | * it checks for another report with the same line number, filename and |
||
| 238 | * pma_version |
||
| 239 | * |
||
| 240 | * @param array $bugReport the bug report being checked |
||
| 241 | * Integer $index: for php exception type |
||
| 242 | * @param mixed $index |
||
| 243 | * |
||
| 244 | * @return array the first similar report or null |
||
| 245 | */ |
||
| 246 | 3 | protected function _getClosestReport($bugReport, $index = 0) |
|
| 247 | { |
||
| 248 | 3 | if (isset($bugReport['exception_type']) |
|
| 249 | 3 | && $bugReport['exception_type'] == 'php' |
|
| 250 | ) { |
||
| 251 | 2 | $location = $bugReport['errors'][$index]['file']; |
|
| 252 | 2 | $linenumber = $bugReport['errors'][$index]['lineNum']; |
|
| 253 | } else { |
||
| 254 | list($location, $linenumber) = |
||
| 255 | 3 | $this->_getIdentifyingLocation($bugReport['exception']['stack']); |
|
| 256 | } |
||
| 257 | 3 | $report = TableRegistry::get('Reports')->findByLocationAndLinenumberAndPmaVersion( |
|
| 258 | 3 | $location, $linenumber, |
|
| 259 | 3 | $this->getStrippedPmaVersion($bugReport['pma_version']) |
|
| 260 | 3 | )->all()->first(); |
|
| 261 | |||
| 262 | 3 | return $report; |
|
| 263 | } |
||
| 264 | |||
| 265 | /** |
||
| 266 | * creates the report data from an incident that has no related report. |
||
| 267 | * |
||
| 268 | * @param array $bugReport the bug report the report record is being created for |
||
| 269 | * Integer $index: for php exception type |
||
| 270 | * @param mixed $index |
||
| 271 | * |
||
| 272 | * @return array an array with the report fields can be used with Report->save |
||
| 273 | */ |
||
| 274 | 3 | protected function _getReportDetails($bugReport, $index = 0) |
|
| 275 | { |
||
| 276 | 3 | if (isset($bugReport['exception_type']) |
|
| 277 | 3 | && $bugReport['exception_type'] == 'php' |
|
| 278 | ) { |
||
| 279 | 3 | $location = $bugReport['errors'][$index]['file']; |
|
| 280 | 3 | $linenumber = $bugReport['errors'][$index]['lineNum']; |
|
| 281 | $reportDetails = array( |
||
| 282 | 3 | 'error_message' => $bugReport['errors'][$index]['msg'], |
|
| 283 | 3 | 'error_name' => $bugReport['errors'][$index]['type'], |
|
| 284 | ); |
||
| 285 | 3 | $exception_type = 1; |
|
| 286 | } else { |
||
| 287 | list($location, $linenumber) = |
||
| 288 | 3 | $this->_getIdentifyingLocation($bugReport['exception']['stack']); |
|
| 289 | |||
| 290 | $reportDetails = array( |
||
| 291 | 3 | 'error_message' => $bugReport['exception']['message'], |
|
| 292 | 3 | 'error_name' => $bugReport['exception']['name'], |
|
| 293 | ); |
||
| 294 | 3 | $exception_type = 0; |
|
| 295 | } |
||
| 296 | |||
| 297 | 3 | $reportDetails = array_merge( |
|
| 298 | 3 | $reportDetails, |
|
| 299 | array( |
||
| 300 | 3 | 'status' => 'new', |
|
| 301 | 3 | 'location' => $location, |
|
| 302 | 3 | 'linenumber' => $linenumber, |
|
| 303 | 3 | 'pma_version' => $bugReport['pma_version'], |
|
| 304 | 3 | 'exception_type' => $exception_type, |
|
| 305 | ) |
||
| 306 | ); |
||
| 307 | |||
| 308 | 3 | return $reportDetails; |
|
| 309 | } |
||
| 310 | |||
| 311 | /** |
||
| 312 | * creates the incident data from the submitted bug report. |
||
| 313 | * |
||
| 314 | * @param array $bugReport the bug report the report record is being created for |
||
| 315 | * |
||
| 316 | * @return array an array of schematized incident. |
||
| 317 | * Can be used with Incident->save |
||
| 318 | */ |
||
| 319 | 3 | protected function _getSchematizedIncidents($bugReport) |
|
| 320 | { |
||
| 321 | //$bugReport = Sanitize::clean($bugReport, array('escape' => false)); |
||
| 322 | 3 | $schematizedReports = array(); |
|
| 323 | $schematizedCommonReport = array( |
||
| 324 | 3 | 'pma_version' => $this->getStrippedPmaVersion($bugReport['pma_version']), |
|
| 325 | 3 | 'php_version' => $this->_getSimpleVersion($bugReport['php_version'], 2), |
|
| 326 | 3 | 'browser' => $bugReport['browser_name'] . ' ' |
|
| 327 | 3 | . $this->_getSimpleVersion($bugReport['browser_version'], 1), |
|
| 328 | 3 | 'user_os' => $bugReport['user_os'], |
|
| 329 | 3 | 'locale' => $bugReport['locale'], |
|
| 330 | 3 | 'configuration_storage' => $bugReport['configuration_storage'], |
|
| 331 | 3 | 'server_software' => $this->_getServer($bugReport['server_software']), |
|
| 332 | 3 | 'full_report' => json_encode($bugReport), |
|
| 333 | ); |
||
| 334 | |||
| 335 | 3 | if (isset($bugReport['exception_type']) |
|
| 336 | 3 | && $bugReport['exception_type'] == 'php' |
|
| 337 | ) { |
||
| 338 | // for each "errors" |
||
| 339 | 3 | foreach ($bugReport['errors'] as $error) { |
|
| 340 | 3 | $tmpReport = array_merge( |
|
| 341 | 3 | $schematizedCommonReport, |
|
| 342 | array( |
||
| 343 | 3 | 'error_name' => $error['type'], |
|
| 344 | 3 | 'error_message' => $error['msg'], |
|
| 345 | 3 | 'script_name' => $error['file'], |
|
| 346 | 3 | 'stacktrace' => json_encode($error['stackTrace']), |
|
| 347 | 3 | 'stackhash' => $error['stackhash'], |
|
| 348 | 3 | 'exception_type' => 1, // 'php' |
|
| 349 | ) |
||
| 350 | ); |
||
| 351 | 3 | array_push($schematizedReports, $tmpReport); |
|
| 352 | } |
||
| 353 | } else { |
||
| 354 | 3 | $tmpReport = array_merge( |
|
| 355 | 3 | $schematizedCommonReport, |
|
| 356 | array( |
||
| 357 | 3 | 'error_name' => $bugReport['exception']['name'], |
|
| 358 | 3 | 'error_message' => $bugReport['exception']['message'], |
|
| 359 | 3 | 'script_name' => $bugReport['script_name'], |
|
| 360 | 3 | 'stacktrace' => json_encode($bugReport['exception']['stack']), |
|
| 361 | 3 | 'stackhash' => $this->getStackHash($bugReport['exception']['stack']), |
|
| 362 | 3 | 'exception_type' => 0, //'js' |
|
| 363 | ) |
||
| 364 | ); |
||
| 365 | |||
| 366 | 3 | if (isset($bugReport['steps'])) { |
|
| 367 | 3 | $tmpReport['steps'] = $bugReport['steps']; |
|
| 368 | } |
||
| 369 | 3 | array_push($schematizedReports, $tmpReport); |
|
| 370 | } |
||
| 371 | |||
| 372 | 3 | return $schematizedReports; |
|
| 373 | } |
||
| 374 | |||
| 375 | /** |
||
| 376 | * Gets the identifiying location info from a stacktrace. |
||
| 377 | * |
||
| 378 | * This is used to skip stacktrace levels that are within the error reporting js |
||
| 379 | * files that sometimes appear in the stacktrace but are not related to the bug |
||
| 380 | * report |
||
| 381 | * |
||
| 382 | * returns two things in an array: |
||
| 383 | * - the first element is the filename/scriptname of the error |
||
| 384 | * - the second element is the linenumber of the error |
||
| 385 | * |
||
| 386 | * @param array $stacktrace the stacktrace being examined |
||
| 387 | * |
||
| 388 | * @return array an array with the filename/scriptname and linenumber of the |
||
| 389 | * error |
||
| 390 | */ |
||
| 391 | 5 | protected function _getIdentifyingLocation($stacktrace) |
|
| 417 | |||
| 418 | /** |
||
| 419 | * Gets a part of a version string according to the specified version Length. |
||
| 420 | * |
||
| 421 | * @param string $versionString the version string |
||
| 422 | * @param string $versionLength the number of version components to return. eg |
||
| 423 | * 1 for major version only and 2 for major and |
||
| 424 | * minor version |
||
| 425 | * |
||
| 426 | * @return string the major and minor version part |
||
| 427 | */ |
||
| 428 | 4 | protected function _getSimpleVersion($versionString, $versionLength) |
|
| 429 | { |
||
| 430 | 4 | $versionLength = (int) $versionLength; |
|
| 431 | 4 | if ($versionLength < 1) { |
|
| 432 | 1 | $versionLength = 1; |
|
| 433 | } |
||
| 434 | /* modify the re to accept a variable number of version components. I |
||
| 435 | * atleast take one component and optionally get more components if need be. |
||
| 436 | * previous code makes sure that the $versionLength variable is a positive |
||
| 437 | * int |
||
| 438 | */ |
||
| 439 | 4 | $result = preg_match( |
|
| 440 | 4 | "/^(\d+\.){" . ($versionLength - 1) . "}\d+/", |
|
| 441 | 4 | $versionString, |
|
| 442 | 4 | $matches |
|
| 443 | ); |
||
| 444 | 4 | if ($result) { |
|
| 445 | 4 | $simpleVersion = $matches[0]; |
|
| 446 | |||
| 447 | 4 | return $simpleVersion; |
|
| 448 | } |
||
| 449 | |||
| 450 | return $versionString; |
||
| 451 | } |
||
| 452 | |||
| 453 | /** |
||
| 454 | * Returns the version string stripped of |
||
| 455 | * 'deb', 'ubuntu' and other suffixes |
||
| 456 | * |
||
| 457 | * @param string $versionString phpMyAdmin version |
||
| 458 | * |
||
| 459 | * @return string stripped phpMyAdmin version |
||
| 460 | */ |
||
| 461 | 10 | public function getStrippedPmaVersion($versionString) |
|
| 476 | |||
| 477 | /** |
||
| 478 | * Gets the server name and version from the server signature. |
||
| 479 | * |
||
| 480 | * @param string $signature the server signature |
||
| 481 | * |
||
| 482 | * @return string the server name and version or UNKNOWN |
||
| 483 | */ |
||
| 484 | 4 | protected function _getServer($signature) |
|
| 494 | |||
| 495 | /** |
||
| 496 | * returns the hash pertaining to a stacktrace. |
||
| 497 | * |
||
| 498 | * @param array $stacktrace the stacktrace in question |
||
| 499 | * |
||
| 500 | * @return string the hash string of the stacktrace |
||
| 501 | */ |
||
| 502 | 4 | public function getStackHash($stacktrace) |
|
| 517 | |||
| 518 | /** |
||
| 519 | * Checks the length of stacktrace and full_report |
||
| 520 | * and logs if it is greater than what it can hold |
||
| 521 | * |
||
| 522 | * @param array $si submitted incident |
||
| 523 | * @param array $incident_ids incident IDs |
||
| 524 | * |
||
| 525 | * @return array $incident_ids |
||
| 526 | */ |
||
| 527 | 2 | private function _logLongIncidentSubmissions($si, &$incident_ids) { |
|
| 552 | } |
||
| 553 |
Loading history...
This check compares calls to functions or methods with their respective definitions. If the call has more arguments than are defined, it raises an issue.
If a function is defined several times with a different number of parameters, the check may pick up the wrong definition and report false positives. One codebase where this has been known to happen is Wordpress.
In this case you can add the
@ignorePhpDoc annotation to the duplicate definition and it will be ignored.