From e7b822b0d3e24131473fa342a919017ca0e8226f Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Mon, 7 Nov 2011 21:54:19 +0000 Subject: [PATCH] FU r102073: * Added assertRepoDefined()/assertRepoTitle() functions to throw exceptions if a valid FileRepo or Title member is not set * Fixed comment about UnregisteredLocalFile (it always sets a Title object) * Note that $title in File constructor can be a string and that getTitle() can return false in docs --- includes/filerepo/File.php | 67 +++++++++++++++++++++++----- includes/filerepo/ForeignAPIFile.php | 5 +++ includes/filerepo/LocalFile.php | 9 ++-- 3 files changed, 66 insertions(+), 15 deletions(-) diff --git a/includes/filerepo/File.php b/includes/filerepo/File.php index ca340c818d..4dffbfe179 100644 --- a/includes/filerepo/File.php +++ b/includes/filerepo/File.php @@ -61,7 +61,7 @@ abstract class File { */ /** - * @var FileRepo + * @var FileRepo|false */ var $repo; @@ -85,13 +85,22 @@ abstract class File { protected $canRender, $isSafeFile; /** - * Call this constructor from child classes + * @var string Required Repository class type + */ + protected $repoClass = 'FileRepo'; + + /** + * Call this constructor from child classes. + * + * Both $title and $repo are optional, though some functions + * may return false or throw exceptions if they are not set. + * Most subclasses will want to call assertRepoDefined() here. * - * @param $title Title|false + * @param $title Title|string|false * @param $repo FileRepo|false */ function __construct( $title, $repo ) { - if ( $title !== false ) { // account for UnregisteredLocalFile et all + if ( $title !== false ) { // subclasses may not use MW titles $title = self::normalizeTitle( $title, 'exception' ); } $this->title = $title; @@ -205,6 +214,7 @@ abstract class File { */ public function getName() { if ( !isset( $this->name ) ) { + $this->assertRepoDefined(); $this->name = $this->repo->getNameFromTitle( $this->title ); } return $this->name; @@ -226,7 +236,7 @@ abstract class File { /** * Return the associated title object - * @return Title + * @return Title|false */ public function getTitle() { return $this->title; } @@ -249,6 +259,7 @@ abstract class File { */ public function getUrl() { if ( !isset( $this->url ) ) { + $this->assertRepoDefined(); $this->url = $this->repo->getZoneUrl( 'public' ) . '/' . $this->getUrlRel(); } return $this->url; @@ -303,7 +314,8 @@ abstract class File { */ public function getPath() { if ( !isset( $this->path ) ) { - $this->path = $this->repo->getZonePath('public') . '/' . $this->getRel(); + $this->assertRepoDefined(); + $this->path = $this->repo->getZonePath( 'public' ) . '/' . $this->getRel(); } return $this->path; } @@ -928,6 +940,7 @@ abstract class File { */ function getHashPath() { if ( !isset( $this->hashPath ) ) { + $this->assertRepoDefined(); $this->hashPath = $this->repo->getHashPath( $this->getName() ); } return $this->hashPath; @@ -995,6 +1008,7 @@ abstract class File { * @return string */ function getArchivePath( $suffix = false ) { + $this->assertRepoDefined(); return $this->repo->getZonePath( 'public' ) . '/' . $this->getArchiveRel( $suffix ); } @@ -1007,7 +1021,9 @@ abstract class File { * @return string */ function getArchiveThumbPath( $archiveName, $suffix = false ) { - return $this->repo->getZonePath( 'thumb' ) . '/' . $this->getArchiveThumbRel( $archiveName, $suffix ); + $this->assertRepoDefined(); + return $this->repo->getZonePath( 'thumb' ) . '/' . + $this->getArchiveThumbRel( $archiveName, $suffix ); } /** @@ -1018,6 +1034,7 @@ abstract class File { * @return string */ function getThumbPath( $suffix = false ) { + $this->assertRepoDefined(); $path = $this->repo->getZonePath( 'thumb' ) . '/' . $this->getRel(); if ( $suffix !== false ) { $path .= '/' . $suffix; @@ -1033,7 +1050,8 @@ abstract class File { * @return string */ function getArchiveUrl( $suffix = false ) { - $path = $this->repo->getZoneUrl('public') . '/archive/' . $this->getHashPath(); + $this->assertRepoDefined(); + $path = $this->repo->getZoneUrl( 'public' ) . '/archive/' . $this->getHashPath(); if ( $suffix === false ) { $path = substr( $path, 0, -1 ); } else { @@ -1051,7 +1069,9 @@ abstract class File { * @return string */ function getArchiveThumbUrl( $archiveName, $suffix = false ) { - $path = $this->repo->getZoneUrl('thumb') . '/archive/' . $this->getHashPath() . rawurlencode( $archiveName ) . "/"; + $this->assertRepoDefined(); + $path = $this->repo->getZoneUrl( 'thumb' ) . '/archive/' . + $this->getHashPath() . rawurlencode( $archiveName ) . "/"; if ( $suffix === false ) { $path = substr( $path, 0, -1 ); } else { @@ -1068,6 +1088,7 @@ abstract class File { * @return path */ function getThumbUrl( $suffix = false ) { + $this->assertRepoDefined(); $path = $this->repo->getZoneUrl('thumb') . '/' . $this->getUrlRel(); if ( $suffix !== false ) { $path .= '/' . rawurlencode( $suffix ); @@ -1083,6 +1104,7 @@ abstract class File { * @return string */ function getArchiveVirtualUrl( $suffix = false ) { + $this->assertRepoDefined(); $path = $this->repo->getVirtualUrl() . '/public/archive/' . $this->getHashPath(); if ( $suffix === false ) { $path = substr( $path, 0, -1 ); @@ -1100,6 +1122,7 @@ abstract class File { * @return string */ function getThumbVirtualUrl( $suffix = false ) { + $this->assertRepoDefined(); $path = $this->repo->getVirtualUrl() . '/thumb/' . $this->getUrlRel(); if ( $suffix !== false ) { $path .= '/' . rawurlencode( $suffix ); @@ -1115,6 +1138,7 @@ abstract class File { * @return string */ function getVirtualUrl( $suffix = false ) { + $this->assertRepoDefined(); $path = $this->repo->getVirtualUrl() . '/public/' . $this->getUrlRel(); if ( $suffix !== false ) { $path .= '/' . rawurlencode( $suffix ); @@ -1126,6 +1150,7 @@ abstract class File { * @return bool */ function isHashed() { + $this->assertRepoDefined(); return $this->repo->isHashed(); } @@ -1205,7 +1230,7 @@ abstract class File { /** * Returns the repository * - * @return FileRepo + * @return FileRepo|false */ function getRepo() { return $this->repo; @@ -1384,7 +1409,7 @@ abstract class File { */ function getDescriptionText() { global $wgMemc, $wgLang; - if ( !$this->repo->fetchDescription ) { + if ( !$this->repo || !$this->repo->fetchDescription ) { return false; } $renderUrl = $this->repo->getDescriptionRenderUrl( $this->getName(), $wgLang->getCode() ); @@ -1635,6 +1660,26 @@ abstract class File { function isMissing() { return false; } + + /** + * Assert that $this->repo is set to a valid FileRepo instance + * @throws MWException + */ + protected function assertRepoDefined() { + if ( !( $this->repo instanceof $this->repoClass ) ) { + throw new MWException( "A {$this->repoClass} object is not set for this File.\n" ); + } + } + + /** + * Assert that $this->title is set to a Title + * @throws MWException + */ + protected function assertTitleDefined() { + if ( !( $this->title instanceof Title ) ) { + throw new MWException( "A Title object is not set for this File.\n" ); + } + } } /** * Aliases for backwards compatibility with 1.6 diff --git a/includes/filerepo/ForeignAPIFile.php b/includes/filerepo/ForeignAPIFile.php index 2e48517f85..9cd798da97 100644 --- a/includes/filerepo/ForeignAPIFile.php +++ b/includes/filerepo/ForeignAPIFile.php @@ -16,6 +16,8 @@ class ForeignAPIFile extends File { private $mExists; + protected $repoClass = 'ForeignApiRepo'; + /** * @param $title * @param $repo ForeignApiRepo @@ -24,8 +26,11 @@ class ForeignAPIFile extends File { */ function __construct( $title, $repo, $info, $exists = false ) { parent::__construct( $title, $repo ); + $this->mInfo = $info; $this->mExists = $exists; + + $this->assertRepoDefined(); } /** diff --git a/includes/filerepo/LocalFile.php b/includes/filerepo/LocalFile.php index b14966a10f..6cd30e13ab 100644 --- a/includes/filerepo/LocalFile.php +++ b/includes/filerepo/LocalFile.php @@ -58,6 +58,8 @@ class LocalFile extends File { /**#@-*/ + protected $repoClass = 'LocalRepo'; + /** * Create a LocalFile from a title * Do not call this except from inside a repo class. @@ -144,16 +146,15 @@ class LocalFile extends File { * Do not call this except from inside a repo class. */ function __construct( $title, $repo ) { - if ( !is_object( $title ) ) { // LocalFile requires a title object - throw new MWException( __CLASS__ . ' constructor given bogus title.' ); - } - parent::__construct( $title, $repo ); $this->metadata = ''; $this->historyLine = 0; $this->historyRes = null; $this->dataLoaded = false; + + $this->assertRepoDefined(); + $this->assertTitleDefined(); } /** -- 2.20.1