Whitespace, comments and general cleanup.
authorTrevor Parscal <tparscal@users.mediawiki.org>
Wed, 20 Oct 2010 00:22:25 +0000 (00:22 +0000)
committerTrevor Parscal <tparscal@users.mediawiki.org>
Wed, 20 Oct 2010 00:22:25 +0000 (00:22 +0000)
includes/resourceloader/ResourceLoader.php
includes/resourceloader/ResourceLoaderFileModule.php

index 37ef160..d75d573 100644 (file)
@@ -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" );
index 02d38b0..18e820a 100644 (file)
@@ -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()
         *