From 0caccb3d28dafa19866e36c9ec0267616036f3e5 Mon Sep 17 00:00:00 2001 From: Trevor Parscal Date: Wed, 20 Oct 2010 00:22:25 +0000 Subject: [PATCH] Whitespace, comments and general cleanup. --- includes/resourceloader/ResourceLoader.php | 152 +++++++++--------- .../ResourceLoaderFileModule.php | 40 ++--- 2 files changed, 96 insertions(+), 96 deletions(-) diff --git a/includes/resourceloader/ResourceLoader.php b/includes/resourceloader/ResourceLoader.php index 37ef160b56..d75d573237 100644 --- a/includes/resourceloader/ResourceLoader.php +++ b/includes/resourceloader/ResourceLoader.php @@ -25,33 +25,35 @@ defined( 'MEDIAWIKI' ) || die( 1 ); /** * Dynamic JavaScript and CSS resource loading system. * - * Most of the documention is on the MediaWiki documentation wiki starting at + * Most of the documention is on the MediaWiki documentation wiki starting at: * http://www.mediawiki.org/wiki/ResourceLoader */ class ResourceLoader { /* Protected Static Members */ - // @var array list of module name/ResourceLoaderModule object pairs + /** @var {array} List of module name/ResourceLoaderModule object pairs */ protected $modules = array(); /* Protected Methods */ - + /** - * Loads information stored in the database about modules + * Loads information stored in the database about modules. + * + * This method grabs modules dependencies from the database and updates modules objects. * * This is not inside the module code because it's so much more performant to request all of the information at once - * than it is to have each module requests it's own information. - * - * This method grab modules dependencies from the database and initialize modules object. - * A first pass compute dependencies, a second one the blob mtime. - * - * @param $modules Array List of module names to preload information for - * @param $context ResourceLoaderContext Context to load the information within + * than it is to have each module requests it's own information. This sacrifice of modularity yields a profound + * performance improvement. + * + * A first pass calculates dependent file modified times, a second one calculates message blob modified times. + * + * @param {array} $modules List of module names to preload information for + * @param {ResourceLoaderContext} $context Context to load the information within */ protected function preloadModuleInfo( array $modules, ResourceLoaderContext $context ) { if ( !count( $modules ) ) { - return; # or Database*::select() will explode + return; // or else Database*::select() will explode, plus it's cheaper! } $dbr = wfGetDb( DB_SLAVE ); $skin = $context->getSkin(); @@ -72,6 +74,7 @@ class ResourceLoader { ); $modulesWithDeps[] = $row->md_module; } + // Register the absence of a dependency row too foreach ( array_diff( $modules, $modulesWithDeps ) as $name ) { $this->modules[$name]->setFileDependencies( $skin, array() ); @@ -103,44 +106,40 @@ class ResourceLoader { } /** - * Runs text (js,CSS) through a filter, caching the filtered result for future calls. + * Runs JavaScript or CSS data through a filter, caching the filtered result for future calls. * * Availables filters are: * - minify-js \see JSMin::minify * - minify-css \see CSSMin::minify * - flip-css \see CSSJanus::transform - * When the filter names does not exist, text is returned as is. - * - * @param $filter String: name of filter to run - * @param $data String: text to filter, such as JavaScript or CSS text - * @param $file String: path to file being filtered, (optional: only required for CSS to resolve paths) - * @return String: filtered data + * + * If data is empty, only whitespace or the filter was unknown, data is returned unmodified. + * + * @param {string} $filter Name of filter to run + * @param {string} $data Text to filter, such as JavaScript or CSS text + * @return {string} Filtered data */ protected function filter( $filter, $data ) { global $wgMemc; + wfProfileIn( __METHOD__ ); - // For empty or whitespace-only things, don't do any processing - # FIXME: we should return the data unfiltered if $filter is not supported. - # that would save up a md5 computation and one memcached get. - if ( trim( $data ) === '' ) { + // For empty/whitespace-only data or for unknown filters, don't perform any caching or processing + if ( trim( $data ) === '' || !in_array( $filter, array( 'minify-js', 'minify-css', 'flip-css' ) ) ) { wfProfileOut( __METHOD__ ); return $data; } - // Try memcached + // Try for Memcached hit $key = wfMemcKey( 'resourceloader', 'filter', $filter, md5( $data ) ); - $cached = $wgMemc->get( $key ); - - if ( $cached !== false && $cached !== null ) { + if ( is_string( $cache = $wgMemc->get( $key ) ) ) { wfProfileOut( __METHOD__ ); - return $cached; + return $cache; } - // Run the filter + // Run the filter - we've already verified one of these will work try { switch ( $filter ) { - # note: if adding a new filter. Please update method documentation above. case 'minify-js': $result = JSMin::minify( $data ); break; @@ -150,26 +149,23 @@ class ResourceLoader { case 'flip-css': $result = CSSJanus::transform( $data, true, false ); break; - default: - // Don't cache anything, just pass right through - wfProfileOut( __METHOD__ ); - return $data; } } catch ( Exception $exception ) { - throw new MWException( 'Filter threw an exception: ' . $exception->getMessage() ); + throw new MWException( 'ResourceLoader filter error. Exception was thrown: ' . $exception->getMessage() ); } - // Save filtered text to memcached + // Save filtered text to Memcached $wgMemc->set( $key, $result ); wfProfileOut( __METHOD__ ); + return $result; } /* Methods */ /** - * Registers core modules and runs registration hooks + * Registers core modules and runs registration hooks. */ public function __construct() { global $IP; @@ -186,23 +182,17 @@ class ResourceLoader { /** * Registers a module with the ResourceLoader system. - * - * Note that registering the same object under multiple names is not supported - * and may silently fail in all kinds of interesting ways. - * - * @param $name Mixed: string of name of module or array of name/object pairs - * @param $object ResourceLoaderModule: module object (optional when using - * multiple-registration calling style) - * @return Boolean: false if there were any errors, in which case one or more - * modules were not registered - * - * @todo We need much more clever error reporting, not just in detailing what - * happened, but in bringing errors to the client in a way that they can - * easily see them if they want to, such as by using FireBug + * + * @param {mixed} $name string of name of module or array of name/object pairs + * @param {ResourceLoaderModule} $object module object (optional when using multiple-registration calling style) + * @throws {MWException} If a duplicate module registration is attempted + * @throws {MWException} If something other than a ResourceLoaderModule is being registered + * @return {bool} false if there were any errors, in which case one or more modules were not registered */ public function register( $name, ResourceLoaderModule $object = null ) { + wfProfileIn( __METHOD__ ); - + // Allow multiple modules to be registered in one call if ( is_array( $name ) && !isset( $object ) ) { foreach ( $name as $key => $value ) { @@ -210,20 +200,23 @@ class ResourceLoader { } wfProfileOut( __METHOD__ ); + return; } // Disallow duplicate registrations if ( isset( $this->modules[$name] ) ) { // A module has already been registered by this name - throw new MWException( 'Another module has already been registered as ' . $name ); + throw new MWException( + 'ResourceLoader duplicate registration error. Another module has already been registered as ' . $name + ); } - + // Validate the input (type hinting lets null through) if ( !( $object instanceof ResourceLoaderModule ) ) { - throw new MWException( 'Invalid ResourceLoader module error. Instances of ResourceLoaderModule expected.' ); + throw new MWException( 'ResourceLoader invalid module error. Instances of ResourceLoaderModule expected.' ); } - + // Attach module $this->modules[$name] = $object; $object->setName( $name ); @@ -234,36 +227,35 @@ class ResourceLoader { /** * Gets a map of all modules and their options * - * @return Array: array( modulename => ResourceLoaderModule ) + * @return {array} array( modulename => ResourceLoaderModule ) */ public function getModules() { return $this->modules; } /** - * Get the ResourceLoaderModule object for a given module name + * Get the ResourceLoaderModule object for a given module name. * - * @param $name String: module name - * @return mixed ResourceLoaderModule or null if not registered + * @param {string} $name module name + * @return {mixed} ResourceLoaderModule if module has been registered, null otherwise */ public function getModule( $name ) { return isset( $this->modules[$name] ) ? $this->modules[$name] : null; } /** - * Outputs a response to a resource load-request, including a content-type header + * Outputs a response to a resource load-request, including a content-type header. * - * @param $context ResourceLoaderContext object + * @param {ResourceLoaderContext} $context Context in which a response should be formed */ public function respond( ResourceLoaderContext $context ) { global $wgResourceLoaderMaxage, $wgCacheEpoch; wfProfileIn( __METHOD__ ); - + // Split requested modules into two groups, modules and missing $modules = array(); $missing = array(); - foreach ( $context->getModules() as $name ) { if ( isset( $this->modules[$name] ) ) { $modules[$name] = $this->modules[$name]; @@ -272,14 +264,12 @@ class ResourceLoader { } } - // If a version wasn't specified we need a shorter expiry time for updates to - // propagate to clients quickly + // If a version wasn't specified we need a shorter expiry time for updates to propagate to clients quickly if ( is_null( $context->getVersion() ) ) { $maxage = $wgResourceLoaderMaxage['unversioned']['client']; $smaxage = $wgResourceLoaderMaxage['unversioned']['server']; } - // If a version was specified we can use a longer expiry time since changing - // version numbers causes cache misses + // If a version was specified we can use a longer expiry time since changing version numbers causes cache misses else { $maxage = $wgResourceLoaderMaxage['versioned']['client']; $smaxage = $wgResourceLoaderMaxage['versioned']['server']; @@ -288,9 +278,9 @@ class ResourceLoader { // Preload information needed to the mtime calculation below $this->preloadModuleInfo( array_keys( $modules ), $context ); - // To send Last-Modified and support If-Modified-Since, we need to detect - // the last modified time wfProfileIn( __METHOD__.'-getModifiedTime' ); + + // To send Last-Modified and support If-Modified-Since, we need to detect the last modified time $mtime = wfTimestamp( TS_UNIX, $wgCacheEpoch ); foreach ( $modules as $module ) { // Bypass squid cache if the request includes any private modules @@ -300,6 +290,7 @@ class ResourceLoader { // Calculate maximum modified time $mtime = max( $mtime, $module->getModifiedTime( $context ) ); } + wfProfileOut( __METHOD__.'-getModifiedTime' ); header( 'Content-Type: ' . ( $context->getOnly() === 'styles' ? 'text/css' : 'text/javascript' ) ); @@ -315,11 +306,15 @@ class ResourceLoader { wfProfileOut( __METHOD__ ); return; } - + + // Generate a response $response = $this->makeModuleResponse( $context, $modules, $missing ); + + // Tack on PHP warnings as a comment in debug mode if ( $context->getDebug() && strlen( $warnings = ob_get_contents() ) ) { $response .= "/*\n$warnings\n*/"; } + // Clear any warnings from the buffer ob_clean(); echo $response; @@ -328,10 +323,12 @@ class ResourceLoader { } /** - * - * @param $context ResourceLoaderContext - * @param $modules Array array( modulename => ResourceLoaderModule ) - * @param $missing Unavailables modules (Default null) + * Generates code for a response + * + * @param {ResourceLoaderContext} $context Context in which to generate a response + * @param {array} $modules List of module objects keyed by module name + * @param {array} $missing List of unavailables modules (optional) + * @return {string} Response data */ public function makeModuleResponse( ResourceLoaderContext $context, array $modules, $missing = null ) { // Pre-fetch blobs @@ -341,6 +338,7 @@ class ResourceLoader { // Generate output $out = ''; foreach ( $modules as $name => $module ) { + wfProfileIn( __METHOD__ . '-' . $name ); // Scripts @@ -384,7 +382,7 @@ class ResourceLoader { $out .= self::makeLoaderImplementScript( $name, $scripts, $styles, $messages ); break; } - + wfProfileOut( __METHOD__ . '-' . $name ); } @@ -410,9 +408,9 @@ class ResourceLoader { } } } - + /* Static Methods */ - + public static function makeLoaderImplementScript( $name, $scripts, $styles, $messages ) { if ( is_array( $scripts ) ) { $scripts = implode( $scripts, "\n" ); diff --git a/includes/resourceloader/ResourceLoaderFileModule.php b/includes/resourceloader/ResourceLoaderFileModule.php index 02d38b09ff..18e820ad39 100644 --- a/includes/resourceloader/ResourceLoaderFileModule.php +++ b/includes/resourceloader/ResourceLoaderFileModule.php @@ -23,7 +23,7 @@ defined( 'MEDIAWIKI' ) || die( 1 ); /** - * ResourceLoader module based on local JS/CSS files + * ResourceLoader module based on local JavaScript/CSS files. */ class ResourceLoaderFileModule extends ResourceLoaderModule { @@ -55,7 +55,7 @@ class ResourceLoaderFileModule extends ResourceLoaderModule { /* Methods */ /** - * Construct a new module from an options array. + * Constructs a new module from an options array. * * @param {array} $options Options array. If not given or empty, an empty module will be constructed * @param {string} $basePath base path to prepend to all paths in $options @@ -121,7 +121,7 @@ class ResourceLoaderFileModule extends ResourceLoaderModule { } /** - * Gets all scripts for a given context concatenated together + * Gets all scripts for a given context concatenated together. * * @param {ResourceLoaderContext} $context Context in which to generate script * @return {string} JavaScript code for $context @@ -137,7 +137,7 @@ class ResourceLoaderFileModule extends ResourceLoaderModule { } /** - * Gets loader script + * Gets loader script. * * @return {string} JavaScript code to be added to startup module */ @@ -149,7 +149,7 @@ class ResourceLoaderFileModule extends ResourceLoaderModule { } /** - * Gets all styles for a given context concatenated together + * Gets all styles for a given context concatenated together. * * @param {ResourceLoaderContext} $context Context in which to generate styles * @return {string} CSS code for $context @@ -185,7 +185,7 @@ class ResourceLoaderFileModule extends ResourceLoaderModule { } /** - * Gets list of message keys used by this module + * Gets list of message keys used by this module. * * @return {array} List of message keys */ @@ -194,7 +194,7 @@ class ResourceLoaderFileModule extends ResourceLoaderModule { } /** - * Gets the name of the group this module should be loaded in + * Gets the name of the group this module should be loaded in. * * @return {string} Group name */ @@ -203,7 +203,7 @@ class ResourceLoaderFileModule extends ResourceLoaderModule { } /** - * Gets list of names of modules this module depends on + * Gets list of names of modules this module depends on. * * @return {array} List of module names */ @@ -212,9 +212,11 @@ class ResourceLoaderFileModule extends ResourceLoaderModule { } /** - * Get the last modified timestamp of this module, which is calculated as the highest last modified timestamp of its - * constituent files and the files it depends on. This function is context-sensitive, only performing calculations - * on files relevant to the given language, skin and debug mode. + * Get the last modified timestamp of this module. + * + * Last modified timestamps are calculated from the highest last modified timestamp of this module's constituent + * files as well as the files it depends on. This function is context-sensitive, only performing calculations on + * files relevant to the given language, skin and debug mode. * * @param {ResourceLoaderContext} $context Context in which to calculate the modified time * @return {integer} UNIX timestamp @@ -262,7 +264,7 @@ class ResourceLoaderFileModule extends ResourceLoaderModule { /* Protected Members */ /** - * Prefixes each file path in a list + * Prefixes each file path in a list. * * @param {array} $list List of file paths in any combination of index/path or path/options pairs * @param {string} $prefix String to prepend to each file path in $list @@ -283,7 +285,7 @@ class ResourceLoaderFileModule extends ResourceLoaderModule { } /** - * Collates file paths by option (where provided) + * Collates file paths by option (where provided). * * @param {array} $list List of file paths in any combination of index/path or path/options pairs * @return {array} List of file paths, collated by $option @@ -310,7 +312,7 @@ class ResourceLoaderFileModule extends ResourceLoaderModule { } /** - * Gets a list of element that match a key, optionally using a fallback key + * Gets a list of element that match a key, optionally using a fallback key. * * @param {array} $map Map of lists to select from * @param {string} $key Key to look for in $map @@ -327,7 +329,7 @@ class ResourceLoaderFileModule extends ResourceLoaderModule { } /** - * Get the contents of a list of JavaScript files + * Gets the contents of a list of JavaScript files. * * @param {array} $scripts List of file paths to scripts to read, remap and concetenate * @return {string} Concatenated and remapped JavaScript data from $scripts @@ -340,7 +342,7 @@ class ResourceLoaderFileModule extends ResourceLoaderModule { } /** - * Get the contents of a list of CSS files + * Gets the contents of a list of CSS files. * * @param {array} $styles List of file paths to styles to read, remap and concetenate * @return {array} List of concatenated and remapped CSS data from $styles, keyed by media type @@ -359,7 +361,7 @@ class ResourceLoaderFileModule extends ResourceLoaderModule { } /** - * Reads a script file + * Reads a script file. * * This method can be used as a callback for array_map() * @@ -373,7 +375,7 @@ class ResourceLoaderFileModule extends ResourceLoaderModule { } /** - * Reads a style file + * Reads a style file. * * This method can be used as a callback for array_map() * @@ -389,7 +391,7 @@ class ResourceLoaderFileModule extends ResourceLoaderModule { } /** - * Resolve a file name + * Resolves a file name. * * This method can be used as a callback for array_map() * -- 2.20.1