Duplicate code is one of the most pungent code smells. A rule that is often used is to re-structure code once it is duplicated in three or more places.
Common duplication problems, and corresponding solutions are:
Complex classes like ForeignAPIRepo 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 ForeignAPIRepo, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 41 | class ForeignAPIRepo extends FileRepo { |
||
| 42 | /* This version string is used in the user agent for requests and will help |
||
| 43 | * server maintainers in identify ForeignAPI usage. |
||
| 44 | * Update the version every time you make breaking or significant changes. */ |
||
| 45 | const VERSION = "2.1"; |
||
| 46 | |||
| 47 | /** |
||
| 48 | * List of iiprop values for the thumbnail fetch queries. |
||
| 49 | * @since 1.23 |
||
| 50 | */ |
||
| 51 | protected static $imageInfoProps = [ |
||
| 52 | 'url', |
||
| 53 | 'timestamp', |
||
| 54 | ]; |
||
| 55 | |||
| 56 | protected $fileFactory = [ 'ForeignAPIFile', 'newFromTitle' ]; |
||
| 57 | /** @var int Check back with Commons after this expiry */ |
||
| 58 | protected $apiThumbCacheExpiry = 86400; // 1 day (24*3600) |
||
| 59 | |||
| 60 | /** @var int Redownload thumbnail files after this expiry */ |
||
| 61 | protected $fileCacheExpiry = 2592000; // 1 month (30*24*3600) |
||
| 62 | |||
| 63 | /** @var array */ |
||
| 64 | protected $mFileExists = []; |
||
| 65 | |||
| 66 | /** @var array */ |
||
| 67 | private $mQueryCache = []; |
||
| 68 | |||
| 69 | /** |
||
| 70 | * @param array|null $info |
||
| 71 | */ |
||
| 72 | function __construct( $info ) { |
||
| 97 | |||
| 98 | /** |
||
| 99 | * @return string |
||
| 100 | * @since 1.22 |
||
| 101 | */ |
||
| 102 | function getApiUrl() { |
||
| 105 | |||
| 106 | /** |
||
| 107 | * Per docs in FileRepo, this needs to return false if we don't support versioned |
||
| 108 | * files. Well, we don't. |
||
| 109 | * |
||
| 110 | * @param Title $title |
||
| 111 | * @param string|bool $time |
||
| 112 | * @return File |
||
| 113 | */ |
||
| 114 | function newFile( $title, $time = false ) { |
||
| 121 | |||
| 122 | /** |
||
| 123 | * @param array $files |
||
| 124 | * @return array |
||
| 125 | */ |
||
| 126 | function fileExistsBatch( array $files ) { |
||
| 127 | $results = []; |
||
| 128 | foreach ( $files as $k => $f ) { |
||
| 129 | if ( isset( $this->mFileExists[$f] ) ) { |
||
| 130 | $results[$k] = $this->mFileExists[$f]; |
||
| 131 | unset( $files[$k] ); |
||
| 132 | } elseif ( self::isVirtualUrl( $f ) ) { |
||
| 133 | # @todo FIXME: We need to be able to handle virtual |
||
| 134 | # URLs better, at least when we know they refer to the |
||
| 135 | # same repo. |
||
| 136 | $results[$k] = false; |
||
| 137 | unset( $files[$k] ); |
||
| 138 | } elseif ( FileBackend::isStoragePath( $f ) ) { |
||
| 139 | $results[$k] = false; |
||
| 140 | unset( $files[$k] ); |
||
| 141 | wfWarn( "Got mwstore:// path '$f'." ); |
||
| 142 | } |
||
| 143 | } |
||
| 144 | |||
| 145 | $data = $this->fetchImageQuery( [ |
||
| 146 | 'titles' => implode( $files, '|' ), |
||
| 147 | 'prop' => 'imageinfo' ] |
||
| 148 | ); |
||
| 149 | |||
| 150 | if ( isset( $data['query']['pages'] ) ) { |
||
| 151 | # First, get results from the query. Note we only care whether the image exists, |
||
| 152 | # not whether it has a description page. |
||
| 153 | foreach ( $data['query']['pages'] as $p ) { |
||
| 154 | $this->mFileExists[$p['title']] = ( $p['imagerepository'] !== '' ); |
||
| 155 | } |
||
| 156 | # Second, copy the results to any redirects that were queried |
||
| 157 | View Code Duplication | if ( isset( $data['query']['redirects'] ) ) { |
|
| 158 | foreach ( $data['query']['redirects'] as $r ) { |
||
| 159 | $this->mFileExists[$r['from']] = $this->mFileExists[$r['to']]; |
||
| 160 | } |
||
| 161 | } |
||
| 162 | # Third, copy the results to any non-normalized titles that were queried |
||
| 163 | View Code Duplication | if ( isset( $data['query']['normalized'] ) ) { |
|
| 164 | foreach ( $data['query']['normalized'] as $n ) { |
||
| 165 | $this->mFileExists[$n['from']] = $this->mFileExists[$n['to']]; |
||
| 166 | } |
||
| 167 | } |
||
| 168 | # Finally, copy the results to the output |
||
| 169 | foreach ( $files as $key => $file ) { |
||
| 170 | $results[$key] = $this->mFileExists[$file]; |
||
| 171 | } |
||
| 172 | } |
||
| 173 | |||
| 174 | return $results; |
||
| 175 | } |
||
| 176 | |||
| 177 | /** |
||
| 178 | * @param string $virtualUrl |
||
| 179 | * @return bool |
||
| 180 | */ |
||
| 181 | function getFileProps( $virtualUrl ) { |
||
| 184 | |||
| 185 | /** |
||
| 186 | * @param array $query |
||
| 187 | * @return string |
||
| 188 | */ |
||
| 189 | function fetchImageQuery( $query ) { |
||
| 190 | global $wgLanguageCode; |
||
| 191 | |||
| 192 | $query = array_merge( $query, |
||
| 193 | [ |
||
| 194 | 'format' => 'json', |
||
| 195 | 'action' => 'query', |
||
| 196 | 'redirects' => 'true' |
||
| 197 | ] ); |
||
| 198 | |||
| 199 | if ( !isset( $query['uselang'] ) ) { // uselang is unset or null |
||
| 200 | $query['uselang'] = $wgLanguageCode; |
||
| 201 | } |
||
| 202 | |||
| 203 | $data = $this->httpGetCached( 'Metadata', $query ); |
||
| 204 | |||
| 205 | if ( $data ) { |
||
| 206 | return FormatJson::decode( $data, true ); |
||
| 207 | } else { |
||
| 208 | return null; |
||
| 209 | } |
||
| 210 | } |
||
| 211 | |||
| 212 | /** |
||
| 213 | * @param array $data |
||
| 214 | * @return bool|array |
||
| 215 | */ |
||
| 216 | function getImageInfo( $data ) { |
||
| 231 | |||
| 232 | /** |
||
| 233 | * @param string $hash |
||
| 234 | * @return array |
||
| 235 | */ |
||
| 236 | function findBySha1( $hash ) { |
||
| 237 | $results = $this->fetchImageQuery( [ |
||
| 238 | 'aisha1base36' => $hash, |
||
| 239 | 'aiprop' => ForeignAPIFile::getProps(), |
||
| 240 | 'list' => 'allimages', |
||
| 241 | ] ); |
||
| 242 | $ret = []; |
||
| 243 | if ( isset( $results['query']['allimages'] ) ) { |
||
| 244 | foreach ( $results['query']['allimages'] as $img ) { |
||
| 245 | // 1.14 was broken, doesn't return name attribute |
||
| 246 | if ( !isset( $img['name'] ) ) { |
||
| 247 | continue; |
||
| 248 | } |
||
| 249 | $ret[] = new ForeignAPIFile( Title::makeTitle( NS_FILE, $img['name'] ), $this, $img ); |
||
| 250 | } |
||
| 251 | } |
||
| 252 | |||
| 253 | return $ret; |
||
| 254 | } |
||
| 255 | |||
| 256 | /** |
||
| 257 | * @param string $name |
||
| 258 | * @param int $width |
||
| 259 | * @param int $height |
||
| 260 | * @param array $result Out parameter that will be changed by the function. |
||
| 261 | * @param string $otherParams |
||
| 262 | * |
||
| 263 | * @return bool |
||
| 264 | */ |
||
| 265 | function getThumbUrl( $name, $width = -1, $height = -1, &$result = null, $otherParams = '' ) { |
||
| 266 | $data = $this->fetchImageQuery( [ |
||
| 267 | 'titles' => 'File:' . $name, |
||
| 268 | 'iiprop' => self::getIIProps(), |
||
| 269 | 'iiurlwidth' => $width, |
||
| 270 | 'iiurlheight' => $height, |
||
| 271 | 'iiurlparam' => $otherParams, |
||
| 272 | 'prop' => 'imageinfo' ] ); |
||
| 273 | $info = $this->getImageInfo( $data ); |
||
| 274 | |||
| 275 | View Code Duplication | if ( $data && $info && isset( $info['thumburl'] ) ) { |
|
| 276 | wfDebug( __METHOD__ . " got remote thumb " . $info['thumburl'] . "\n" ); |
||
| 277 | $result = $info; |
||
| 278 | |||
| 279 | return $info['thumburl']; |
||
| 280 | } else { |
||
| 281 | return false; |
||
| 282 | } |
||
| 283 | } |
||
| 284 | |||
| 285 | /** |
||
| 286 | * @param string $name |
||
| 287 | * @param int $width |
||
| 288 | * @param int $height |
||
| 289 | * @param string $otherParams |
||
| 290 | * @param string $lang Language code for language of error |
||
| 291 | * @return bool|MediaTransformError |
||
| 292 | * @since 1.22 |
||
| 293 | */ |
||
| 294 | function getThumbError( $name, $width = -1, $height = -1, $otherParams = '', $lang = null ) { |
||
| 295 | $data = $this->fetchImageQuery( [ |
||
| 296 | 'titles' => 'File:' . $name, |
||
| 297 | 'iiprop' => self::getIIProps(), |
||
| 298 | 'iiurlwidth' => $width, |
||
| 299 | 'iiurlheight' => $height, |
||
| 300 | 'iiurlparam' => $otherParams, |
||
| 301 | 'prop' => 'imageinfo', |
||
| 302 | 'uselang' => $lang, |
||
| 303 | ] ); |
||
| 304 | $info = $this->getImageInfo( $data ); |
||
| 305 | |||
| 306 | View Code Duplication | if ( $data && $info && isset( $info['thumberror'] ) ) { |
|
| 307 | wfDebug( __METHOD__ . " got remote thumb error " . $info['thumberror'] . "\n" ); |
||
| 308 | |||
| 309 | return new MediaTransformError( |
||
| 310 | 'thumbnail_error_remote', |
||
| 311 | $width, |
||
| 312 | $height, |
||
| 313 | $this->getDisplayName(), |
||
| 314 | $info['thumberror'] // already parsed message from foreign repo |
||
| 315 | ); |
||
| 316 | } else { |
||
| 317 | return false; |
||
| 318 | } |
||
| 319 | } |
||
| 320 | |||
| 321 | /** |
||
| 322 | * Return the imageurl from cache if possible |
||
| 323 | * |
||
| 324 | * If the url has been requested today, get it from cache |
||
| 325 | * Otherwise retrieve remote thumb url, check for local file. |
||
| 326 | * |
||
| 327 | * @param string $name Is a dbkey form of a title |
||
| 328 | * @param int $width |
||
| 329 | * @param int $height |
||
| 330 | * @param string $params Other rendering parameters (page number, etc) |
||
| 331 | * from handler's makeParamString. |
||
| 332 | * @return bool|string |
||
| 333 | */ |
||
| 334 | function getThumbUrlFromCache( $name, $width, $height, $params = "" ) { |
||
| 335 | $cache = ObjectCache::getMainWANInstance(); |
||
| 336 | // We can't check the local cache using FileRepo functions because |
||
| 337 | // we override fileExistsBatch(). We have to use the FileBackend directly. |
||
| 338 | $backend = $this->getBackend(); // convenience |
||
| 339 | |||
| 340 | if ( !$this->canCacheThumbs() ) { |
||
| 341 | $result = null; // can't pass "null" by reference, but it's ok as default value |
||
| 342 | return $this->getThumbUrl( $name, $width, $height, $result, $params ); |
||
| 343 | } |
||
| 344 | $key = $this->getLocalCacheKey( 'ForeignAPIRepo', 'ThumbUrl', $name ); |
||
| 345 | $sizekey = "$width:$height:$params"; |
||
| 346 | |||
| 347 | /* Get the array of urls that we already know */ |
||
| 348 | $knownThumbUrls = $cache->get( $key ); |
||
| 349 | View Code Duplication | if ( !$knownThumbUrls ) { |
|
| 350 | /* No knownThumbUrls for this file */ |
||
| 351 | $knownThumbUrls = []; |
||
| 352 | } else { |
||
| 353 | if ( isset( $knownThumbUrls[$sizekey] ) ) { |
||
| 354 | wfDebug( __METHOD__ . ': Got thumburl from local cache: ' . |
||
| 355 | "{$knownThumbUrls[$sizekey]} \n" ); |
||
| 356 | |||
| 357 | return $knownThumbUrls[$sizekey]; |
||
| 358 | } |
||
| 359 | /* This size is not yet known */ |
||
| 360 | } |
||
| 361 | |||
| 362 | $metadata = null; |
||
| 363 | $foreignUrl = $this->getThumbUrl( $name, $width, $height, $metadata, $params ); |
||
| 364 | |||
| 365 | if ( !$foreignUrl ) { |
||
| 366 | wfDebug( __METHOD__ . " Could not find thumburl\n" ); |
||
| 367 | |||
| 368 | return false; |
||
| 369 | } |
||
| 370 | |||
| 371 | // We need the same filename as the remote one :) |
||
| 372 | $fileName = rawurldecode( pathinfo( $foreignUrl, PATHINFO_BASENAME ) ); |
||
| 373 | if ( !$this->validateFilename( $fileName ) ) { |
||
| 374 | wfDebug( __METHOD__ . " The deduced filename $fileName is not safe\n" ); |
||
| 375 | |||
| 376 | return false; |
||
| 377 | } |
||
| 378 | $localPath = $this->getZonePath( 'thumb' ) . "/" . $this->getHashPath( $name ) . $name; |
||
| 379 | $localFilename = $localPath . "/" . $fileName; |
||
| 380 | $localUrl = $this->getZoneUrl( 'thumb' ) . "/" . $this->getHashPath( $name ) . |
||
| 381 | rawurlencode( $name ) . "/" . rawurlencode( $fileName ); |
||
| 382 | |||
| 383 | if ( $backend->fileExists( [ 'src' => $localFilename ] ) |
||
| 384 | && isset( $metadata['timestamp'] ) |
||
| 385 | ) { |
||
| 386 | wfDebug( __METHOD__ . " Thumbnail was already downloaded before\n" ); |
||
| 387 | $modified = $backend->getFileTimestamp( [ 'src' => $localFilename ] ); |
||
| 388 | $remoteModified = strtotime( $metadata['timestamp'] ); |
||
| 389 | $current = time(); |
||
| 390 | $diff = abs( $modified - $current ); |
||
| 391 | if ( $remoteModified < $modified && $diff < $this->fileCacheExpiry ) { |
||
| 392 | /* Use our current and already downloaded thumbnail */ |
||
| 393 | $knownThumbUrls[$sizekey] = $localUrl; |
||
| 394 | $cache->set( $key, $knownThumbUrls, $this->apiThumbCacheExpiry ); |
||
| 395 | |||
| 396 | return $localUrl; |
||
| 397 | } |
||
| 398 | /* There is a new Commons file, or existing thumbnail older than a month */ |
||
| 399 | } |
||
| 400 | $thumb = self::httpGet( $foreignUrl ); |
||
| 401 | if ( !$thumb ) { |
||
| 402 | wfDebug( __METHOD__ . " Could not download thumb\n" ); |
||
| 403 | |||
| 404 | return false; |
||
| 405 | } |
||
| 406 | |||
| 407 | # @todo FIXME: Delete old thumbs that aren't being used. Maintenance script? |
||
| 408 | $backend->prepare( [ 'dir' => dirname( $localFilename ) ] ); |
||
| 409 | $params = [ 'dst' => $localFilename, 'content' => $thumb ]; |
||
| 410 | if ( !$backend->quickCreate( $params )->isOK() ) { |
||
| 411 | wfDebug( __METHOD__ . " could not write to thumb path '$localFilename'\n" ); |
||
| 412 | |||
| 413 | return $foreignUrl; |
||
| 414 | } |
||
| 415 | $knownThumbUrls[$sizekey] = $localUrl; |
||
| 416 | $cache->set( $key, $knownThumbUrls, $this->apiThumbCacheExpiry ); |
||
| 417 | wfDebug( __METHOD__ . " got local thumb $localUrl, saving to cache \n" ); |
||
| 418 | |||
| 419 | return $localUrl; |
||
| 420 | } |
||
| 421 | |||
| 422 | /** |
||
| 423 | * @see FileRepo::getZoneUrl() |
||
| 424 | * @param string $zone |
||
| 425 | * @param string|null $ext Optional file extension |
||
| 426 | * @return string |
||
| 427 | */ |
||
| 428 | function getZoneUrl( $zone, $ext = null ) { |
||
| 438 | |||
| 439 | /** |
||
| 440 | * Get the local directory corresponding to one of the basic zones |
||
| 441 | * @param string $zone |
||
| 442 | * @return bool|null|string |
||
| 443 | */ |
||
| 444 | function getZonePath( $zone ) { |
||
| 452 | |||
| 453 | /** |
||
| 454 | * Are we locally caching the thumbnails? |
||
| 455 | * @return bool |
||
| 456 | */ |
||
| 457 | public function canCacheThumbs() { |
||
| 460 | |||
| 461 | /** |
||
| 462 | * The user agent the ForeignAPIRepo will use. |
||
| 463 | * @return string |
||
| 464 | */ |
||
| 465 | public static function getUserAgent() { |
||
| 468 | |||
| 469 | /** |
||
| 470 | * Get information about the repo - overrides/extends the parent |
||
| 471 | * class's information. |
||
| 472 | * @return array |
||
| 473 | * @since 1.22 |
||
| 474 | */ |
||
| 475 | function getInfo() { |
||
| 502 | |||
| 503 | /** |
||
| 504 | * Like a Http:get request, but with custom User-Agent. |
||
| 505 | * @see Http::get |
||
| 506 | * @param string $url |
||
| 507 | * @param string $timeout |
||
| 508 | * @param array $options |
||
| 509 | * @return bool|string |
||
| 510 | */ |
||
| 511 | public static function httpGet( $url, $timeout = 'default', $options = [] ) { |
||
| 537 | |||
| 538 | /** |
||
| 539 | * @return string |
||
| 540 | * @since 1.23 |
||
| 541 | */ |
||
| 542 | protected static function getIIProps() { |
||
| 545 | |||
| 546 | /** |
||
| 547 | * HTTP GET request to a mediawiki API (with caching) |
||
| 548 | * @param string $target Used in cache key creation, mostly |
||
| 549 | * @param array $query The query parameters for the API request |
||
| 550 | * @param int $cacheTTL Time to live for the memcached caching |
||
| 551 | * @return null |
||
| 552 | */ |
||
| 553 | public function httpGetCached( $target, $query, $cacheTTL = 3600 ) { |
||
| 583 | |||
| 584 | /** |
||
| 585 | * @param callable $callback |
||
| 586 | * @throws MWException |
||
| 587 | */ |
||
| 588 | function enumFiles( $callback ) { |
||
| 591 | |||
| 592 | /** |
||
| 593 | * @throws MWException |
||
| 594 | */ |
||
| 595 | protected function assertWritableRepo() { |
||
| 598 | } |
||
| 599 |
In PHP it is possible to write to properties without declaring them. For example, the following is perfectly valid PHP code:
Generally, it is a good practice to explictly declare properties to avoid accidental typos and provide IDE auto-completion: