r104625 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r104624‎ | r104625 | r104626 >
Date:00:09, 30 November 2011
Author:tstarling
Status:resolved
Tags:
Comment:
Merged changes from trunk up to r104623
Modified paths:
  • /branches/wmf/1.18wmf1/extensions/VipsScaler (modified) (history)
  • /branches/wmf/1.18wmf1/extensions/VipsScaler/SpecialVipsTest.php (modified) (history)
  • /branches/wmf/1.18wmf1/extensions/VipsScaler/VipsScaler_body.php (modified) (history)
  • /branches/wmf/1.18wmf1/extensions/VipsScaler/VipsTest.php (modified) (history)

Diff [purge]

Index: branches/wmf/1.18wmf1/extensions/VipsScaler/VipsTest.php
@@ -1,7 +1,7 @@
2 -<?php
 2+ <?php
33
44 /**
5 - * Extension registration file for Special:VipsTest. The VipsScaler extension
 5+ * Extension registration file for Special:VipsTest. The VipsScaler extension
66 * must be enabled.
77 */
88
@@ -18,6 +18,8 @@
1919 */
2020 $wgVipsTestExpiry = 3600;
2121
 22+ $dir = dirname( __FILE__ );
 23+
2224 /** Registration */
2325 $wgAutoloadClasses['SpecialVipsTest'] = "$dir/SpecialVipsTest.php";
2426 $wgExtensionAliasesFiles['VipsTest'] = "$dir/VipsScaler.alias.php";
@@ -25,7 +27,7 @@
2628 $wgGroupPermissions['*']['vipsscaler-test'] = true;
2729 $wgSpecialPages['VipsTest'] = 'SpecialVipsTest';
2830
29 -/**
 31+/**
3032 * Disable VipsScaler for ordinary image scaling so that the test has something
3133 * to compare against.
3234 */
Index: branches/wmf/1.18wmf1/extensions/VipsScaler/SpecialVipsTest.php
@@ -79,8 +79,7 @@
8080 if ( $request->getCheck( 'bilinear' ) ) {
8181 $vipsUrlOptions['bilinear'] = 1;
8282 }
83 -
84 -
 83+
8584 # Generate normal thumbnail
8685 $params = array( 'width' => $width );
8786 $thumb = $file->transform( $params );
@@ -97,7 +96,7 @@
9897
9998 # Make url to the vips thumbnail
10099 $vipsThumbUrl = $this->getTitle()->getLocalUrl( $vipsUrlOptions );
101 -
 100+
102101 # HTML for the thumbnails
103102 $thumbs = Html::rawElement( 'div', array( 'id' => 'mw-vipstest-thumbnails' ),
104103 Html::element( 'img', array(
@@ -107,14 +106,14 @@
108107 Html::element( 'img', array(
109108 'src' => $vipsThumbUrl,
110109 'alt' => wfMessage( 'vipsscaler-vips-thumb' ),
111 - ) )
 110+ ) )
112111 );
113112
114113 # Helper messages shown above the thumbnails rendering
115114 $help = wfMessage( 'vipsscaler-thumbs-help' )->parseAsBlock();
116115
117116 # A checkbox to easily alternate between both views:
118 - $checkbox = Xml::checkLabel(
 117+ $checkbox = Xml::checkLabel(
119118 wfMessage( 'vipsscaler-thumbs-switch-label' ),
120119 'mw-vipstest-thumbs-switch',
121120 'mw-vipstest-thumbs-switch'
@@ -148,7 +147,7 @@
149148 // was correct. So we have to show it again.
150149 // See HTMLForm::show()
151150 $result = $form->show();
152 - if( $result === true or $result instanceof Status && $result->isGood() ) {
 151+ if( $result === true || $result instanceof Status && $result->isGood() ) {
153152 $form->displayForm( $result );
154153 $this->showThumbnails();
155154 }
@@ -188,7 +187,7 @@
189188 'Bilinear' => array(
190189 'name' => 'bilinear',
191190 'class' => 'HTMLCheckField',
192 - 'label-message' => 'vipsscaler-form-bilinear',
 191+ 'label-message' => 'vipsscaler-form-bilinear',
193192 ),
194193 );
195194
@@ -202,13 +201,18 @@
203202 return $fields;
204203 }
205204
 205+ /**
 206+ * @param $input
 207+ * @param $alldata
 208+ * @return bool|String
 209+ */
206210 public static function validateFileInput( $input, $alldata ) {
207211 if ( !trim( $input ) ) {
208 - # Don't show an error if the file is not yet specified,
 212+ # Don't show an error if the file is not yet specified,
209213 # because it is annoying
210214 return true;
211215 }
212 -
 216+
213217 $title = Title::makeTitleSafe( NS_FILE, $input );
214218 if( is_null( $title ) ) {
215219 return wfMsg( 'vipsscaler-invalid-file' );
@@ -221,8 +225,14 @@
222226 // Looks sane enough.
223227 return true;
224228 }
 229+
 230+ /**
 231+ * @param $input
 232+ * @param $allData
 233+ * @return bool|String
 234+ */
225235 public static function validateWidth( $input, $allData ) {
226 - if ( self::validateFileInput( $allData['File'], $allData ) !== true
 236+ if ( self::validateFileInput( $allData['File'], $allData ) !== true
227237 || !trim( $allData['File'] ) ) {
228238 # Invalid file, error will already be shown at file field
229239 return true;
@@ -234,16 +244,23 @@
235245 }
236246 return true;
237247 }
 248+
 249+ /**
 250+ * @param $input
 251+ * @param $allData
 252+ * @return bool|String
 253+ */
238254 public static function validateSharpen( $input, $allData ) {
239255 if ( $input >= 5.0 || $input < 0.0 ) {
240256 return wfMsg( 'vipsscaler-invalid-sharpen' );
241257 }
242258 return true;
243 -
244259 }
245260
246261 /**
247262 * Process data submitted by the form.
 263+ * @param $data array
 264+ * @return Status
248265 */
249266 public static function processForm( array $data ) {
250267 return Status::newGood();
@@ -278,7 +295,6 @@
279296 if ( !$handler->normaliseParams( $file, $params ) ) {
280297 return $this->streamError( 500, "VipsScaler: invalid parameters\n" );
281298 }
282 -
283299
284300 # Get the thumbnail
285301 if ( is_null( $wgVipsThumbnailerHost ) || $request->getBool( 'noproxy' ) ) {
@@ -308,7 +324,7 @@
309325 'dstPath' => $dstPath,
310326 'dstUrl' => $dstUrl,
311327 );
312 -
 328+
313329 $options = array();
314330 if ( $request->getBool( 'bilinear' ) ) {
315331 $options['bilinear'] = true;
@@ -347,9 +363,22 @@
348364 $url = wfExpandUrl( $request->getRequestURL(), PROTO_INTERNAL );
349365 $url = wfAppendQuery( $url, array( 'noproxy' => '1' ) );
350366 wfDebug( __METHOD__ . ": Getting vips thumb from remote url $url\n" );
351 -
352 - $options = array( 'method' => 'GET' );
353367
 368+ $bits = IP::splitHostAndPort( $wgVipsThumbnailerHost );
 369+ if ( !$bits ) {
 370+ throw new MWException( __METHOD__.': $wgVipsThumbnailerHost is not set to a valid host' );
 371+ }
 372+ list( $host, $port ) = $bits;
 373+ if ( $port === false ) {
 374+ $port = 80;
 375+ }
 376+ $proxy = IP::combineHostAndPort( $host, $port );
 377+
 378+ $options = array(
 379+ 'method' => 'GET',
 380+ 'proxy' => $proxy,
 381+ );
 382+
354383 $req = MWHttpRequest::factory( $url, $options );
355384 $status = $req->execute();
356385 if ( $status->isOk() ) {
@@ -361,18 +390,20 @@
362391 header( "Cache-Control: public, max-age=$wgVipsTestExpiry, s-maxage=$wgVipsTestExpiry" );
363392 header( 'Expires: ' . gmdate( 'r ', time() + $wgVipsTestExpiry ) );
364393 print $req->getContent();
 394+ } elseif ( $status->hasMessage( 'http-bad-status' ) ) {
 395+ return $this->streamError( 500, $req->getContent() );
365396 } else {
366 - return $this->streamError( 500, $req->getContent() );
 397+ global $wgOut;
 398+ return $this->streamError( 500, $wgOut->parse( $status->getWikiText() ) );
367399 }
368 -
369400 }
370401 }
371402
372 -
373403 /**
374404 * Generates a blank page with given HTTP error code
375405 *
376 - * @param $code Integer: HTTP error either 404 or 500
 406+ * @param $code Integer HTTP error either 404 or 500
 407+ * @param $error string
377408 */
378409 protected function streamError( $code, $error = '' ) {
379410 $this->getOutput()->setStatusCode( $code );
Index: branches/wmf/1.18wmf1/extensions/VipsScaler/VipsScaler_body.php
@@ -38,6 +38,7 @@
3939 * @param File $file
4040 * @param array $params
4141 * @param MediaTransformOutput $mto
 42+ * @return bool
4243 */
4344 public static function onTransform( $handler, $file, &$params, &$mto ) {
4445 # Check $wgVipsConditions
@@ -46,15 +47,14 @@
4748 wfDebug( "...\n" );
4849 return true;
4950 }
50 -
5151 return self::doTransform( $handler, $file, $params, $options, $mto );
5252 }
53 -
 53+
5454 /**
5555 * Performs a transform with VIPS
56 - *
 56+ *
5757 * @see VipsScaler::onTransform
58 - *
 58+ *
5959 * @param BitmapHandler $handler
6060 * @param File $file
6161 * @param array $params
@@ -108,10 +108,11 @@
109109 }
110110
111111 /**
112 - * @param $handler
113 - * @param $file
114 - * @param $params
115 - * @param $options
 112+ * @param $handler BitmapHandler
 113+ * @param $file File
 114+ * @param $params array
 115+ * @param $options array
 116+ * @return array
116117 */
117118 public static function makeCommands( $handler, $file, $params, $options ) {
118119 global $wgVipsCommand;
@@ -136,10 +137,10 @@
137138 # Calculate shrink factors. Offsetting by a small amount is required
138139 # because of rounding down of the target size by VIPS. See 25990#c7
139140 #
140 - # No need to invert source and physical dimensions. They already got
 141+ # No need to invert source and physical dimensions. They already got
141142 # switched if needed.
142143 #
143 - # Use sprintf() instead of plain string conversion so that we can
 144+ # Use sprintf() instead of plain string conversion so that we can
144145 # control the precision
145146 $rx = sprintf( "%.18e", $params['srcWidth'] / ($params['physicalWidth'] + 0.125) );
146147 $ry = sprintf( "%.18e", $params['srcHeight'] / ($params['physicalHeight'] + 0.125) );
@@ -178,7 +179,7 @@
179180 }
180181
181182 if ( !empty( $options['convolution'] ) ) {
182 - $commands[] = new VipsConvolution( $wgVipsCommand,
 183+ $commands[] = new VipsConvolution( $wgVipsCommand,
183184 array( 'im_convf', $options['convolution'] ) );
184185 }
185186
@@ -352,6 +353,7 @@
353354 $this->vips = $vips;
354355 $this->args = $args;
355356 }
 357+
356358 /**
357359 * Set the input and output file of this command
358360 *
@@ -374,6 +376,7 @@
375377 $this->output = $output;
376378 }
377379 }
 380+
378381 /**
379382 * Returns the output filename
380383 * @return string
@@ -381,6 +384,7 @@
382385 public function getOutput() {
383386 return $this->output;
384387 }
 388+
385389 /**
386390 * Return the output of the command
387391 * @return string
@@ -396,9 +400,13 @@
397401 */
398402 public function execute() {
399403 # Build and escape the command string
400 - $cmd = wfEscapeShellArg( $this->vips,
401 - array_shift( $this->args ),
402 - $this->input, $this->output );
 404+ $cmd =
 405+ 'IM_CONCURRENCY=1 ' .
 406+ wfEscapeShellArg(
 407+ $this->vips,
 408+ array_shift( $this->args ),
 409+ $this->input, $this->output
 410+ );
403411
404412 foreach ( $this->args as $arg ) {
405413 $cmd .= ' ' . wfEscapeShellArg( $arg );
@@ -445,26 +453,30 @@
446454 * matrix file as its last argument
447455 */
448456 class VipsConvolution extends VipsCommand {
 457+
 458+ /**
 459+ * @return int
 460+ */
449461 public function execute() {
450462 # Convert a 2D array into a space/newline separated matrix
451463 $convolutionMatrix = array_pop( $this->args );
452464 $convolutionString = '';
453 - foreach ( $convolutionMatrix as $i=>$row ) {
 465+ foreach ( $convolutionMatrix as $row ) {
454466 $convolutionString .= implode( ' ', $row ) . "\n";
455467 }
456468 # Save the matrix in a tempfile
457469 $convolutionFile = self::makeTemp( 'conv' );
458470 file_put_contents( $convolutionFile, $convolutionString );
459471 array_push( $this->args, $convolutionFile );
460 -
 472+
461473 wfDebug( __METHOD__ . ": Convolving image [\n" . $convolutionString . "] \n" );
462 -
 474+
463475 # Call the parent to actually execute the command
464476 $retval = parent::execute();
465 -
 477+
466478 # Remove the temporary matrix file
467479 unlink( $convolutionFile );
468 -
 480+
469481 return $retval;
470482 }
471483 }
Property changes on: branches/wmf/1.18wmf1/extensions/VipsScaler
___________________________________________________________________
Modified: svn:mergeinfo
472484 Merged /trunk/extensions/VipsScaler:r104604-104623

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r104623* Fix for r103575: actually use $wgVipsThumbnailerHost...tstarling00:04, 30 November 2011

Status & tagging log