r102331 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r102330‎ | r102331 | r102332 >
Date:21:54, 7 November 2011
Author:aaron
Status:ok
Tags:lamecommitsummary 
Comment:
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
Modified paths:
  • /trunk/phase3/includes/filerepo/File.php (modified) (history)
  • /trunk/phase3/includes/filerepo/ForeignAPIFile.php (modified) (history)
  • /trunk/phase3/includes/filerepo/LocalFile.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/filerepo/LocalFile.php
——@@ -58,6 +58,8 @@
5959
6060 /**#@-*/
6161
 62+ protected $repoClass = 'LocalRepo';
 63+
6264 /**
6365 * Create a LocalFile from a title
6466 * Do not call this except from inside a repo class.
——@@ -144,16 +146,15 @@
145147 * Do not call this except from inside a repo class.
146148 */
147149 function __construct( $title, $repo ) {
148 - if ( !is_object( $title ) ) { // LocalFile requires a title object
149 - throw new MWException( __CLASS__ . ' constructor given bogus title.' );
150 - }
151 -
152150 parent::__construct( $title, $repo );
153151
154152 $this->metadata = '';
155153 $this->historyLine = 0;
156154 $this->historyRes = null;
157155 $this->dataLoaded = false;
 156+
 157+ $this->assertRepoDefined();
 158+ $this->assertTitleDefined();
158159 }
159160
160161 /**
Index: trunk/phase3/includes/filerepo/File.php
——@@ -61,7 +61,7 @@
6262 */
6363
6464 /**
65 - * @var FileRepo
 65+ * @var FileRepo|false
6666 */
6767 var $repo;
6868
——@@ -85,13 +85,22 @@
8686 protected $canRender, $isSafeFile;
8787
8888 /**
89 - * Call this constructor from child classes
 89+ * @var string Required Repository class type
 90+ */
 91+ protected $repoClass = 'FileRepo';
 92+
 93+ /**
 94+ * Call this constructor from child classes.
 95+ *
 96+ * Both $title and $repo are optional, though some functions
 97+ * may return false or throw exceptions if they are not set.
 98+ * Most subclasses will want to call assertRepoDefined() here.
9099 *
91 - * @param $title Title|false
 100+ * @param $title Title|string|false
92101 * @param $repo FileRepo|false
93102 */
94103 function __construct( $title, $repo ) {
95 - if ( $title !== false ) { // account for UnregisteredLocalFile et all
 104+ if ( $title !== false ) { // subclasses may not use MW titles
96105 $title = self::normalizeTitle( $title, 'exception' );
97106 }
98107 $this->title = $title;
——@@ -205,6 +214,7 @@
206215 */
207216 public function getName() {
208217 if ( !isset( $this->name ) ) {
 218+ $this->assertRepoDefined();
209219 $this->name = $this->repo->getNameFromTitle( $this->title );
210220 }
211221 return $this->name;
——@@ -226,7 +236,7 @@
227237
228238 /**
229239 * Return the associated title object
230 - * @return Title
 240+ * @return Title|false
231241 */
232242 public function getTitle() { return $this->title; }
233243
——@@ -249,6 +259,7 @@
250260 */
251261 public function getUrl() {
252262 if ( !isset( $this->url ) ) {
 263+ $this->assertRepoDefined();
253264 $this->url = $this->repo->getZoneUrl( 'public' ) . '/' . $this->getUrlRel();
254265 }
255266 return $this->url;
——@@ -303,7 +314,8 @@
304315 */
305316 public function getPath() {
306317 if ( !isset( $this->path ) ) {
307 - $this->path = $this->repo->getZonePath('public') . '/' . $this->getRel();
 318+ $this->assertRepoDefined();
 319+ $this->path = $this->repo->getZonePath( 'public' ) . '/' . $this->getRel();
308320 }
309321 return $this->path;
310322 }
——@@ -928,6 +940,7 @@
929941 */
930942 function getHashPath() {
931943 if ( !isset( $this->hashPath ) ) {
 944+ $this->assertRepoDefined();
932945 $this->hashPath = $this->repo->getHashPath( $this->getName() );
933946 }
934947 return $this->hashPath;
——@@ -995,6 +1008,7 @@
9961009 * @return string
9971010 */
9981011 function getArchivePath( $suffix = false ) {
 1012+ $this->assertRepoDefined();
9991013 return $this->repo->getZonePath( 'public' ) . '/' . $this->getArchiveRel( $suffix );
10001014 }
10011015
——@@ -1007,7 +1021,9 @@
10081022 * @return string
10091023 */
10101024 function getArchiveThumbPath( $archiveName, $suffix = false ) {
1011 - return $this->repo->getZonePath( 'thumb' ) . '/' . $this->getArchiveThumbRel( $archiveName, $suffix );
 1025+ $this->assertRepoDefined();
 1026+ return $this->repo->getZonePath( 'thumb' ) . '/' .
 1027+ $this->getArchiveThumbRel( $archiveName, $suffix );
10121028 }
10131029
10141030 /**
——@@ -1018,6 +1034,7 @@
10191035 * @return string
10201036 */
10211037 function getThumbPath( $suffix = false ) {
 1038+ $this->assertRepoDefined();
10221039 $path = $this->repo->getZonePath( 'thumb' ) . '/' . $this->getRel();
10231040 if ( $suffix !== false ) {
10241041 $path .= '/' . $suffix;
——@@ -1033,7 +1050,8 @@
10341051 * @return string
10351052 */
10361053 function getArchiveUrl( $suffix = false ) {
1037 - $path = $this->repo->getZoneUrl('public') . '/archive/' . $this->getHashPath();
 1054+ $this->assertRepoDefined();
 1055+ $path = $this->repo->getZoneUrl( 'public' ) . '/archive/' . $this->getHashPath();
10381056 if ( $suffix === false ) {
10391057 $path = substr( $path, 0, -1 );
10401058 } else {
——@@ -1051,7 +1069,9 @@
10521070 * @return string
10531071 */
10541072 function getArchiveThumbUrl( $archiveName, $suffix = false ) {
1055 - $path = $this->repo->getZoneUrl('thumb') . '/archive/' . $this->getHashPath() . rawurlencode( $archiveName ) . "/";
 1073+ $this->assertRepoDefined();
 1074+ $path = $this->repo->getZoneUrl( 'thumb' ) . '/archive/' .
 1075+ $this->getHashPath() . rawurlencode( $archiveName ) . "/";
10561076 if ( $suffix === false ) {
10571077 $path = substr( $path, 0, -1 );
10581078 } else {
——@@ -1068,6 +1088,7 @@
10691089 * @return path
10701090 */
10711091 function getThumbUrl( $suffix = false ) {
 1092+ $this->assertRepoDefined();
10721093 $path = $this->repo->getZoneUrl('thumb') . '/' . $this->getUrlRel();
10731094 if ( $suffix !== false ) {
10741095 $path .= '/' . rawurlencode( $suffix );
——@@ -1083,6 +1104,7 @@
10841105 * @return string
10851106 */
10861107 function getArchiveVirtualUrl( $suffix = false ) {
 1108+ $this->assertRepoDefined();
10871109 $path = $this->repo->getVirtualUrl() . '/public/archive/' . $this->getHashPath();
10881110 if ( $suffix === false ) {
10891111 $path = substr( $path, 0, -1 );
——@@ -1100,6 +1122,7 @@
11011123 * @return string
11021124 */
11031125 function getThumbVirtualUrl( $suffix = false ) {
 1126+ $this->assertRepoDefined();
11041127 $path = $this->repo->getVirtualUrl() . '/thumb/' . $this->getUrlRel();
11051128 if ( $suffix !== false ) {
11061129 $path .= '/' . rawurlencode( $suffix );
——@@ -1115,6 +1138,7 @@
11161139 * @return string
11171140 */
11181141 function getVirtualUrl( $suffix = false ) {
 1142+ $this->assertRepoDefined();
11191143 $path = $this->repo->getVirtualUrl() . '/public/' . $this->getUrlRel();
11201144 if ( $suffix !== false ) {
11211145 $path .= '/' . rawurlencode( $suffix );
——@@ -1126,6 +1150,7 @@
11271151 * @return bool
11281152 */
11291153 function isHashed() {
 1154+ $this->assertRepoDefined();
11301155 return $this->repo->isHashed();
11311156 }
11321157
——@@ -1205,7 +1230,7 @@
12061231 /**
12071232 * Returns the repository
12081233 *
1209 - * @return FileRepo
 1234+ * @return FileRepo|false
12101235 */
12111236 function getRepo() {
12121237 return $this->repo;
——@@ -1384,7 +1409,7 @@
13851410 */
13861411 function getDescriptionText() {
13871412 global $wgMemc, $wgLang;
1388 - if ( !$this->repo->fetchDescription ) {
 1413+ if ( !$this->repo || !$this->repo->fetchDescription ) {
13891414 return false;
13901415 }
13911416 $renderUrl = $this->repo->getDescriptionRenderUrl( $this->getName(), $wgLang->getCode() );
——@@ -1635,6 +1660,26 @@
16361661 function isMissing() {
16371662 return false;
16381663 }
 1664+
 1665+ /**
 1666+ * Assert that $this->repo is set to a valid FileRepo instance
 1667+ * @throws MWException
 1668+ */
 1669+ protected function assertRepoDefined() {
 1670+ if ( !( $this->repo instanceof $this->repoClass ) ) {
 1671+ throw new MWException( "A {$this->repoClass} object is not set for this File.\n" );
 1672+ }
 1673+ }
 1674+
 1675+ /**
 1676+ * Assert that $this->title is set to a Title
 1677+ * @throws MWException
 1678+ */
 1679+ protected function assertTitleDefined() {
 1680+ if ( !( $this->title instanceof Title ) ) {
 1681+ throw new MWException( "A Title object is not set for this File.\n" );
 1682+ }
 1683+ }
16391684 }
16401685 /**
16411686 * Aliases for backwards compatibility with 1.6
Index: trunk/phase3/includes/filerepo/ForeignAPIFile.php
——@@ -16,6 +16,8 @@
1717
1818 private $mExists;
1919
 20+ protected $repoClass = 'ForeignApiRepo';
 21+
2022 /**
2123 * @param $title
2224 * @param $repo ForeignApiRepo
——@@ -24,8 +26,11 @@
2527 */
2628 function __construct( $title, $repo, $info, $exists = false ) {
2729 parent::__construct( $title, $repo );
 30+
2831 $this->mInfo = $info;
2932 $this->mExists = $exists;
 33+
 34+ $this->assertRepoDefined();
3035 }
3136
3237 /**

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r102073* Added File::normalizeTitle() function and used it to consolidate file title...aaron23:49, 4 November 2011

Status & tagging log