http://www.mediawiki.org/wiki/Special:Code/MediaWiki/96704
Revision: 96704
Author: nelson
Date: 2011-09-09 23:42:28 +0000 (Fri, 09 Sep 2011)
Log Message:
-----------
need to have extensions on the files being transformed, and put in some checks
for transform failure.
Modified Paths:
--------------
trunk/extensions/SwiftMedia/SwiftMedia.body.php
trunk/extensions/SwiftMedia/TODO
Modified: trunk/extensions/SwiftMedia/SwiftMedia.body.php
===================================================================
--- trunk/extensions/SwiftMedia/SwiftMedia.body.php 2011-09-09 22:57:39 UTC
(rev 96703)
+++ trunk/extensions/SwiftMedia/SwiftMedia.body.php 2011-09-09 23:42:28 UTC
(rev 96704)
@@ -126,20 +126,23 @@
}
}
- /** getTransformScript inherited */
- /** getUnscaledThumb inherited */
- /** thumbName inherited */
- /** createThumb inherited */
- /** getThumbnail inherited */
-
/**
- * class-specific transform (from an original into a thumb).
+ * Do the work of a transform (from an original into a thumb).
+ * Contains filesystem-specific functions.
+ *
+ * @param $thumbName string: the name of the thumbnail file.
+ * @param $thumbUrl string: the URL of the thumbnail file.
+ * @param $params Array: an associative array of handler-specific
parameters.
+ * Typical keys are width, height and page.
+ * @param $flags Integer: a bitfield, may contain self::RENDER_NOW to
force rendering
+ *
+ * @return MediaTransformOutput | false
*/
- function maybeDoTransform( $thumbName, $thumbUrl, $params, $flags = 0 )
{
+ function maybeDoTransform( $thumbName, $thumbUrl, $params, $flags ) {
global $wgIgnoreImageErrors, $wgThumbnailEpoch, $wgTmpDirectory;
// get a temporary place to put the original.
- $thumbPath = tempnam( $wgTmpDirectory, 'transform_out_' );
+ $thumbPath = tempnam( $wgTmpDirectory, 'transform_out_') . "."
. pathinfo( $thumbName, PATHINFO_EXTENSION );
if ( $this->repo && $this->repo->canTransformVia404() &&
!($flags & self::RENDER_NOW ) ) {
return $this->handler->getTransform( $this, $thumbPath,
$thumbUrl, $params );
@@ -176,15 +179,17 @@
}
// what if they didn't actually write out a thumbnail? Check
the file size.
- if ( $thumb && filesize($thumbPath)) {
+ if ( $thumb && file_exists( $thumbPath ) && filesize(
$thumbPath ) ) {
// Store the thumbnail into Swift, but in the thumb
version of the container.
wfDebug( __METHOD__ . "Creating thumb " .
$this->getRel() . "/" . $thumbName . "\n" );
$this->repo->write_swift_object( $thumbPath,
$container, $this->getRel() . "/" . $thumbName );
// php-cloudfiles throws exceptions, so failure never
gets here.
}
- // Clean up temporary data.
- unlink( $thumbPath );
+ // Clean up temporary data, if it exists.
+ if ( file_exists( $thumbPath ) ) {
+ @unlink( $thumbPath );
+ }
return $thumb;
}
@@ -878,7 +883,7 @@
function getLocalCopy($container, $rel) {
// get a temporary place to put the original.
- $tempPath = tempnam( wfTempDir(), 'swift_in_' );
+ $tempPath = tempnam( wfTempDir(), 'swift_in_' ) . "." .
pathinfo( $rel, PATHINFO_EXTENSION );
/* Fetch the image out of Swift */
$conn = $this->connect();
Modified: trunk/extensions/SwiftMedia/TODO
===================================================================
--- trunk/extensions/SwiftMedia/TODO 2011-09-09 22:57:39 UTC (rev 96703)
+++ trunk/extensions/SwiftMedia/TODO 2011-09-09 23:42:28 UTC (rev 96704)
@@ -16,7 +16,19 @@
12) Why is anybody calling resolveVirtualUrl()? It's defined in the Repo, but
getPath() is defined against a file.
Why is UploadStashFile() being called with a virtual URL? Once the file has
been stashed() it has an object name. The container name is implicit.
Should UploadStashFile *always* (in our case) be called with a virtual URL?
+19) TS: "I suggest you test transform errors throughout your whole system.
With our current NFS system, transform errors work by having thumb.php return
an HTTP 500 response with an informative HTML error message. The error message
is passed through to Squid by the 404 handler, and Squid won't cache it. The
user will see a broken image icon in their browser, and choosing "view image"
from the context menu of the image will show them the error message from
thumb.php."
+20) TS: "It's not appropriate to allow CloudFiles exceptions to be propagated
back to the callers of File/FileRepo methods such as transform(). Doing this
will cause the page to not be displayed at all, with an exceptionally ugly
error message in its place. FileRepo has a system for returning user-friendly
error messages from pretty much anywhere. For example, SwiftRepo::storeBatch()
has a Status object to hold error messages, but your code just sets it to a
"good" result, it never sets any errors in it."
+"You can throw an MWException in response to configuration errors, or
assertion-like unexpected errors, but it's not appropriate to be throwing
exceptions in response to network errors or errors generated by the Swift
server.
+21) TS: "// Check overwriting
+ if (0) { #FIXME
+Fix that. It's important to avoid data loss in file rename operations."
+22) TS: "I think this [MD65] validation feature in FSRepo is just a hack for
NFS. Okay to remove it."
+23) TS: "When you get around to implementing SwiftRepo::append(), it will need
some sort of concurrency control to avoid having chunks overwrite each other. "
+24) TS: "SwiftRepo::swiftcopy() should return a Status object instead of
throwing exceptions."
+25) TS: In the short term, just copy ForeignDBViaLBRepo to ForeignDBViaSMRepo
and change the class parent.
+
+
Resolved:
5) The Upload seems to take more time than I expect, but that could be a
function of generating the six thumbnails.
@@ -27,8 +39,16 @@
13) Do we need $wgLocalRepo->ThumbUrl to be configurable given that the Python
middleware presumes it?
We currently have no need for it to be configurable in Swift. I'll just
hard-code it to .../thumb with a note saying
that if it gets changed here, it needs to be changed in the Swift
middleware as well.
+14) TS: "File::transform() needs to be split into two functions, one with the
code that you're duplicating, and the other with the code that you're
overriding."
+15) TS: "Scripted transform needs to keep working, ... We use it for private
wikis. The "transform via 404" feature needs to be left in as well."
+16) TS: "You need to re-add an equivalent for the file_exists($thumbPath)
check that's in File::transform()."
+17) TS: "LocalFile uses the 404 handler. Swift will probably use the 404
handler. That's why you need to support canTransformVia404()."
+18) TS: "You need to handle errors from MediaHandler::doTransform() correctly.
"
+20) TS: "SwiftMedia::migrateThumbFile() should just do nothing,"
+21) TS: "[$wgExcludeFromThumbnailPurge] looks pretty trivial to implement to
me." (RN: It was!)
+
neilk_: okay, the moment when an upload passes from being a temp file into
something else is at $upload->processUpload()
neilk_: in the old design, in essence, all this does is move a file into an
NFS directory, and creates the matching database entry which creates a wiki
page.
neilk_: so far I don't think this should be news?
@@ -81,3 +101,4 @@
nelson____: yeah, I think part of that problem is you're always stuck between
the database and the filestore.
nelson____: they both have opinions about how things work, and you have to
keep them consistent.
nelson____: Okay, I'm gonna take some notes then go home. taking the weekend
off for a bicycle road trip.
+
_______________________________________________
MediaWiki-CVS mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-cvs
