jenkins-bot has submitted this change and it was merged.
Change subject: Refactor code to be more modular
......................................................................
Refactor code to be more modular
Bug: T507
Change-Id: I414c0b51401597f8acfec472c030c6f2e7d8639f
---
M .jshintrc
M ImageMetrics.php
D resources/ext.imageMetrics.loader.js
R resources/head.js
A resources/loader.js
R resources/logger/LoadingTimeLogger.js
A resources/logger/Logger.js
R tests/qunit/logger/LoadingTimeLogger.test.js
8 files changed, 212 insertions(+), 136 deletions(-)
Approvals:
Gilles: Looks good to me, approved
jenkins-bot: Verified
diff --git a/.jshintrc b/.jshintrc
index c6a531b..bda7504 100644
--- a/.jshintrc
+++ b/.jshintrc
@@ -22,10 +22,12 @@
"predef": [
"window",
+ "document",
"Geo",
"QUnit",
"jQuery",
"mediaWiki",
- "mediaWikiLoadStart"
+ "mediaWikiLoadStart",
+ "OO"
]
}
diff --git a/ImageMetrics.php b/ImageMetrics.php
index 2898b3e..a494eab 100644
--- a/ImageMetrics.php
+++ b/ImageMetrics.php
@@ -38,21 +38,27 @@
$wgResourceModules += array(
'ext.imageMetrics' => array(
- 'scripts' => 'ext.imageMetrics.js',
+ 'scripts' => array(
+ 'logger/Logger.js',
+ 'logger/LoadingTimeLogger.js',
+ ),
'localBasePath' => __DIR__ . '/resources',
'remoteExtPath' => 'ImageMetrics/resources',
- 'dependencies' => 'schema.ImageMetricsLoadingTime',
+ 'dependencies' => array(
+ 'oojs',
+ 'schema.ImageMetricsLoadingTime',
+ ),
'targets' => array( 'desktop', 'mobile' ),
),
'ext.imageMetrics.head' => array(
- 'scripts' => 'ext.imageMetrics.head.js',
+ 'scripts' => 'head.js',
'localBasePath' => __DIR__ . '/resources',
'remoteExtPath' => 'ImageMetrics/resources',
'targets' => array( 'desktop', 'mobile' ),
'position' => 'top',
),
'ext.imageMetrics.loader' => array(
- 'scripts' => 'ext.imageMetrics.loader.js',
+ 'scripts' => 'loader.js',
'localBasePath' => __DIR__ . '/resources',
'remoteExtPath' => 'ImageMetrics/resources',
'targets' => array( 'desktop', 'mobile' ),
@@ -77,8 +83,12 @@
*/
$wgHooks[ 'ResourceLoaderGetConfigVars' ][] = function ( &$vars ) {
global $wgImageMetricsSamplingFactor,
$wgImageMetricsLoggedinSamplingFactor;
- $vars[ 'wgImageMetricsSamplingFactor' ] = $wgImageMetricsSamplingFactor;
- $vars[ 'wgImageMetricsLoggedinSamplingFactor' ] =
$wgImageMetricsLoggedinSamplingFactor;
+ $vars[ 'wgImageMetrics' ] = array(
+ 'samplingFactor' => array(
+ 'image' => $wgImageMetricsSamplingFactor,
+ 'imageLoggedin' =>
$wgImageMetricsLoggedinSamplingFactor,
+ ),
+ );
return true;
};
@@ -90,7 +100,7 @@
$wgHooks['ResourceLoaderTestModules'][] = function ( array &$testModules,
ResourceLoader &$resourceLoader ) {
$testModules['qunit']['ext.imageMetrics.tests'] = array(
'scripts' => array(
- 'tests/qunit/ext.imageMetrics.test.js',
+ 'tests/qunit/logger/LoadingTimeLogger.test.js',
),
'dependencies' => array(
'ext.imageMetrics',
diff --git a/resources/ext.imageMetrics.loader.js
b/resources/ext.imageMetrics.loader.js
deleted file mode 100644
index e6ae00c..0000000
--- a/resources/ext.imageMetrics.loader.js
+++ /dev/null
@@ -1,35 +0,0 @@
-/**
- * JavaScript module for image-related metrics.
- * @see https://mediawiki.org/wiki/Extension:ImageMetrics
- *
- * @licence GNU GPL v2 or later
- * @author Tisza Gergő <[email protected]>
- */
-( function ( mw, $ ) {
- 'use strict';
-
- var factor = mw.config.get( 'wgImageMetricsSamplingFactor', false ),
- loggedinFactor = mw.config.get(
'wgImageMetricsLoggedinSamplingFactor', false );
-
- if ( !mw.user.isAnon() && loggedinFactor ) {
- factor = loggedinFactor;
- }
-
- /**
- * Makes a random decision (based on the sampling factor configuration
setting) whether the current
- * request should be logged.
- * @return {boolean}
- */
- function isInSample() {
- if ( !$.isNumeric( factor ) || factor < 1 ) {
- return false;
- }
- return Math.floor( Math.random() * factor ) === 0;
- }
-
- if ( isInSample() ) {
- mw.loader.using( 'ext.imageMetrics', function () {
- mw.ImageMetrics.install( factor );
- } );
- }
-} ( mediaWiki, jQuery ) );
diff --git a/resources/ext.imageMetrics.head.js b/resources/head.js
similarity index 91%
rename from resources/ext.imageMetrics.head.js
rename to resources/head.js
index 8b30709..a0c9278 100644
--- a/resources/ext.imageMetrics.head.js
+++ b/resources/head.js
@@ -1,5 +1,6 @@
/**
* JavaScript module for image-related metrics.
+ * Top-loaded on every request to measure timing of events which might happen
before normal script load.
* @see https://mediawiki.org/wiki/Extension:ImageMetrics
*
* @licence GNU GPL v2 or later
diff --git a/resources/loader.js b/resources/loader.js
new file mode 100644
index 0000000..4004604
--- /dev/null
+++ b/resources/loader.js
@@ -0,0 +1,46 @@
+/**
+ * JavaScript module for image-related metrics.
+ * This module will be loaded on every request to perform sampling and load
the real module if needed.
+ * @see https://mediawiki.org/wiki/Extension:ImageMetrics
+ *
+ * @licence GNU GPL v2 or later
+ * @author Tisza Gergő <[email protected]>
+ */
+( function ( mw, $ ) {
+ 'use strict';
+
+ var logImage,
+ config = mw.config.get( 'wgImageMetrics', { samplingFactor: {}
} ),
+ imageFactor = config.samplingFactor.image,
+ loggedinImageFactor = config.samplingFactor.imageLoggedin;
+
+ /**
+ * Makes a random decision (based on samplingRatio) whether an event
should be logged.
+ * Returns true with 1/samplingRatio probability, or false if
samplingRatio is not a number or smaller than 1.
+ * @param {number|boolean} samplingRatio
+ * @return {boolean}
+ */
+ function isInSample( samplingRatio ) {
+ if ( !$.isNumeric( samplingRatio ) || samplingRatio < 1 ) {
+ return false;
+ }
+ return Math.floor( Math.random() * samplingRatio ) === 0;
+ }
+
+
+ if ( !mw.user.isAnon() && loggedinImageFactor ) {
+ imageFactor = loggedinImageFactor;
+ }
+
+ logImage = isInSample( imageFactor );
+
+ if ( !logImage ) {
+ return;
+ }
+
+ mw.loader.using( 'ext.imageMetrics', function () {
+ if ( logImage ) {
+ mw.imageMetrics.LoadingTimeLogger.install( imageFactor
);
+ }
+ } );
+} ( mediaWiki, jQuery ) );
diff --git a/resources/ext.imageMetrics.js
b/resources/logger/LoadingTimeLogger.js
similarity index 67%
rename from resources/ext.imageMetrics.js
rename to resources/logger/LoadingTimeLogger.js
index a2d31be..442a9c6 100644
--- a/resources/ext.imageMetrics.js
+++ b/resources/logger/LoadingTimeLogger.js
@@ -5,7 +5,7 @@
* @licence GNU GPL v2 or later
* @author Tisza Gergő <[email protected]>
*/
-( function ( mw, $ ) {
+( function ( mw, $, oo ) {
'use strict';
/**
@@ -18,42 +18,40 @@
*/
/**
- * @class ImageMetrics
+ * @class mw.imageMetrics.LoadingTimeLogger
+ * @extends mw.imageMetrics.Logger
* @constructor
* @param {number} samplingFactor sampling factor
- * @param {Object} performance window.performance
* @param {Object} location window.location
* @param {Object} mwConfig mw.config
* @param {Object} geo window.Geo
* @param {Object} eventLog mw.eventLog
+ * @param {Object} performance window.performance
*/
- function ImageMetrics( samplingFactor, performance, location, mwConfig,
geo, eventLog ) {
- this.samplingFactor = samplingFactor;
+ function LoadingTimeLogger(
+ /* inherited arguments */
+ samplingFactor, location, mwConfig, geo, eventLog,
+ /* custom arguments */
+ performance
+ ) {
+ LoadingTimeLogger.parent.call( this, samplingFactor, location,
mwConfig, geo, eventLog );
/** @property {Object} performance window.performance */
this.performance = performance;
-
- /** @property {Object} location window.location */
- this.location = location;
-
- /** @property {Object} mwConfig mw.config */
- this.mwConfig = mwConfig;
-
- /** @property {Object} geo window.Geo */
- this.geo = geo;
-
- /** @property {Object} eventLog mw.eventLog */
- this.eventLog = eventLog;
}
+ oo.inheritClass( LoadingTimeLogger, mw.imageMetrics.Logger );
+
+ LoadingTimeLogger.prototype.schema = 'ImageMetricsLoadingTime';
/**
* Factory function to take care of dependency injection.
* @static
* @param {number} samplingFactor sampling factor
- * @return {ImageMetrics}
+ * @return {mw.imageMetrics.LoadingTimeLogger}
*/
- ImageMetrics.create = function( samplingFactor ) {
- return new ImageMetrics( samplingFactor, window.performance,
window.location, mw.config, window.Geo, mw.eventLog );
+ LoadingTimeLogger.create = function( samplingFactor ) {
+ return new LoadingTimeLogger( samplingFactor, window.location,
mw.config, window.Geo,
+ mw.eventLog, window.performance );
};
/**
@@ -61,52 +59,50 @@
* @static
* @param {number} samplingFactor sampling factor
*/
- ImageMetrics.install = function( samplingFactor ) {
- var imageMetrics = ImageMetrics.create( samplingFactor );
+ LoadingTimeLogger.install = function( samplingFactor ) {
+ var logger = LoadingTimeLogger.create( samplingFactor );
$( window ).load( function () {
- imageMetrics.log();
+ logger.collect();
} );
+ };
+
+ /**
+ * Collects image metrics data and logs it via EventLogging.
+ */
+ LoadingTimeLogger.prototype.collect = function() {
+ var $file,
+ data = {};
+
+ data.imageType = 'filepage-main'; // the only supported
measurement type ATM
+ $file = $( '#file' ).find( 'img' ); // more efficient than
'#file img'
+
+ if ( !$file.length ) {
+ return;
+ }
+
+ this.addNavigationTimingData( data );
+ this.addResourceTimingData( data, $file );
+ this.addOtherData( data, $file );
+
+ this.log( data );
};
/**
* @property {Object<integer, string>} navigationTypes map of
NavigationTiming.type constants to
* human-readable strings
*/
- ImageMetrics.prototype.navigationTypes = {
+ LoadingTimeLogger.prototype.navigationTypes = {
0: 'navigate',
1: 'reload',
2: 'back_forward'
};
/**
- * Adds information provided by MediaWiki.
- * @param {Object} data
- * @param {jQuery} $file jQuery object containing the img element
- */
- ImageMetrics.prototype.addMediaWikiData = function ( data, $file ) {
- if ( this.geo && typeof this.geo.country === 'string' ) {
- data.country = this.geo.country;
- }
- if ( $file.attr( 'alt' ) ) {
- data.fileType = $file.attr( 'alt' ).split( '.' ).pop();
- }
- data.isAnon = this.mwConfig.get( 'wgUserId' ) === null;
- };
-
- /**
- * Adds timing data from the image onload event.
- * @param {Object} data
- */
- ImageMetrics.prototype.addOnloadData = function ( data ) {
- data.fallbackFullLoadingTime = mw.imageMetricsLoadTime;
- };
-
- /**
* Adds navigation type (reload, back etc) to the log data from the
NavigationTiming API.
* @param {Object} data
*/
- ImageMetrics.prototype.addNavigationTimingData = function ( data ) {
+ LoadingTimeLogger.prototype.addNavigationTimingData = function ( data )
{
if ( this.performance.navigation &&
this.performance.navigation.type in this.navigationTypes ) {
data.navigationType =
this.navigationTypes[this.performance.navigation.type];
}
@@ -118,7 +114,7 @@
* @param {jQuery} $file jQuery object containing the img element
* @return {PerformanceResourceTiming|boolean} timing object or false
if not supported
*/
- ImageMetrics.prototype.getResourceTiming = function ( $file ) {
+ LoadingTimeLogger.prototype.getResourceTiming = function ( $file ) {
var url, timing;
if ( !this.performance || !this.performance.getEntriesByName ) {
@@ -143,7 +139,7 @@
* @param {Object} data
* @param {jQuery} $file jQuery object containing the img element
*/
- ImageMetrics.prototype.addResourceTimingData = function ( data, $file )
{
+ LoadingTimeLogger.prototype.addResourceTimingData = function ( data,
$file ) {
var timing = this.getResourceTiming( $file );
if ( timing ) {
@@ -154,30 +150,16 @@
};
/**
- * Collects image metrics data and logs it via EventLogging.
+ * Adds non-ResourceTiming/NavigationTimning-based information.
+ * @param {Object} data
+ * @param {jQuery} $file jQuery object containing the img element
*/
- ImageMetrics.prototype.log = function() {
- var $file,
- data = {};
-
- data.samplingFactor = this.samplingFactor;
-
- data.isHttps = this.location.protocol === 'https:';
-
- data.imageType = 'filepage-main'; // the only supported
measurement type ATM
- $file = $( '#file' ).find( 'img' ); // more efficient than
'#file img'
-
- if ( !$file.length ) {
- return;
+ LoadingTimeLogger.prototype.addOtherData = function ( data, $file ) {
+ if ( $file.attr( 'alt' ) ) {
+ data.fileType = $file.attr( 'alt' ).split( '.' ).pop();
}
-
- this.addMediaWikiData( data, $file );
- this.addOnloadData( data );
- this.addNavigationTimingData( data );
- this.addResourceTimingData( data, $file );
-
- this.eventLog.logEvent( 'ImageMetricsLoadingTime', data );
+ data.fallbackFullLoadingTime = mw.imageMetricsLoadTime;
};
- mw.ImageMetrics = ImageMetrics;
-} ( mediaWiki, jQuery ) );
+ mw.imageMetrics.LoadingTimeLogger = LoadingTimeLogger;
+} ( mediaWiki, jQuery, OO ) );
diff --git a/resources/logger/Logger.js b/resources/logger/Logger.js
new file mode 100644
index 0000000..891a855
--- /dev/null
+++ b/resources/logger/Logger.js
@@ -0,0 +1,70 @@
+/**
+ * JavaScript module for image-related metrics.
+ * @see https://mediawiki.org/wiki/Extension:Logger
+ *
+ * @licence GNU GPL v2 or later
+ * @author Tisza Gergő <[email protected]>
+ */
+( function ( mw, $ ) {
+ 'use strict';
+
+ /**
+ * Abstract parent class for metrics tasks.
+ * @class mw.imageMetrics.Logger
+ * @constructor
+ * @param {number} samplingFactor sampling factor
+ * @param {Object} location window.location
+ * @param {Object} mwConfig mw.config
+ * @param {Object} geo window.Geo
+ * @param {Object} eventLog mw.eventLog
+ */
+ function Logger( samplingFactor, location, mwConfig, geo, eventLog ) {
+ this.samplingFactor = samplingFactor;
+
+ /** @property {Object} location window.location */
+ this.location = location;
+
+ /** @property {Object} mwConfig mw.config */
+ this.mwConfig = mwConfig;
+
+ /** @property {Object} geo window.Geo */
+ this.geo = geo;
+
+ /** @property {Object} eventLog mw.eventLog */
+ this.eventLog = eventLog;
+ }
+
+ /**
+ * EventLogging schema name.
+ * @type {string}
+ */
+ Logger.prototype.schema = null;
+
+ /**
+ * Factory function to take care of dependency injection.
+ * @static
+ * @param {number} samplingFactor sampling factor
+ * @return {mw.imageMetrics.Logger}
+ */
+ Logger.create = function( samplingFactor ) {
+ return new Logger( samplingFactor, window.location, mw.config,
window.Geo, mw.eventLog );
+ };
+
+ /**
+ * Collects image metrics data and logs it via EventLogging.
+ */
+ Logger.prototype.log = function( data ) {
+ data = $.extend( {}, data );
+ data.samplingFactor = this.samplingFactor;
+ data.isHttps = this.location.protocol === 'https:';
+ data.isAnon = this.mwConfig.get( 'wgUserId' ) === null;
+ if ( this.geo && typeof this.geo.country === 'string' ) {
+ data.country = this.geo.country;
+ }
+
+ this.eventLog.logEvent( this.schema, data );
+ };
+
+ mw.imageMetrics = {};
+ mw.imageMetrics.Logger = Logger;
+} ( mediaWiki, jQuery ) );
diff --git a/tests/qunit/ext.imageMetrics.test.js
b/tests/qunit/logger/LoadingTimeLogger.test.js
similarity index 79%
rename from tests/qunit/ext.imageMetrics.test.js
rename to tests/qunit/logger/LoadingTimeLogger.test.js
index 82e799a..60b3036 100644
--- a/tests/qunit/ext.imageMetrics.test.js
+++ b/tests/qunit/logger/LoadingTimeLogger.test.js
@@ -1,23 +1,23 @@
( function ( mw, $ ) {
'use strict';
- QUnit.module( 'ImageMetrics', QUnit.newMwEnvironment() );
+ QUnit.module( 'mw.imageMetrics.LoadingTimeLogger',
QUnit.newMwEnvironment() );
- function createImageMetrics( sandbox, options ) {
- var imageMetrics,
+ function createLoadingTimeLogger( sandbox, options ) {
+ var logger,
logEvent = sandbox.stub(),
config = new mw.Map( options.config || {} );
- imageMetrics = new mw.ImageMetrics( options.samplingFactor,
options.performance || {}, options.location || {}, config,
- options.geo || {}, { logEvent: logEvent } );
+ logger = new mw.imageMetrics.LoadingTimeLogger(
options.samplingFactor, options.location || {}, config,
+ options.geo || {}, { logEvent: logEvent },
options.performance || {} );
options.logEvent = logEvent;
- return imageMetrics;
+ return logger;
}
QUnit.test( 'Constructor sanity test', 1, function ( assert ) {
- var imageMetrics = createImageMetrics( this.sandbox, {} );
- assert.ok( imageMetrics, 'Object created' );
+ var logger = createLoadingTimeLogger( this.sandbox, {} );
+ assert.ok( logger, 'Object created' );
} );
QUnit.test( 'Minimal logging scenario', 12, function ( assert ) {
@@ -26,10 +26,10 @@
samplingFactor: 1,
config: {}
},
- imageMetrics = createImageMetrics( this.sandbox,
options );
+ logger = createLoadingTimeLogger( this.sandbox, options
);
$( '#qunit-fixture' ).append( '<div id="file"><img
alt="Foo.jpg" /></div>' );
- imageMetrics.log();
+ logger.collect();
data = options.logEvent.firstCall.args[1];
assert.strictEqual( data.samplingFactor, 1, 'samplingFactor is
logged correctly' );
@@ -45,8 +45,8 @@
assert.strictEqual( data.fetchDelay, undefined, 'fetchDelay is
not logged when Resource Timing API is not available' );
options.config.wgUserId = 1;
- imageMetrics = createImageMetrics( this.sandbox, options );
- imageMetrics.log();
+ logger = createLoadingTimeLogger( this.sandbox, options );
+ logger.collect();
data = options.logEvent.firstCall.args[1];
assert.strictEqual( data.isAnon, false, 'isAnon is logged
correctly' );
} );
@@ -57,10 +57,10 @@
samplingFactor: 1,
geo: { country: 'London' }
},
- imageMetrics = createImageMetrics( this.sandbox,
options );
+ logger = createLoadingTimeLogger( this.sandbox, options
);
$( '#qunit-fixture' ).append( '<div id="file"><img
alt="Foo.jpg" /></div>' );
- imageMetrics.log();
+ logger.collect();
data = options.logEvent.firstCall.args[1];
assert.strictEqual( data.country, 'London', 'country is logged
correctly' );
@@ -74,10 +74,10 @@
performance: { navigation: { type: 0 } },
samplingFactor: 1
},
- imageMetrics = createImageMetrics( this.sandbox,
options );
+ logger = createLoadingTimeLogger( this.sandbox, options
);
$( '#qunit-fixture' ).append( '<div id="file"><img
alt="Foo.jpg" /></div>' );
- imageMetrics.log();
+ logger.collect();
data = options.logEvent.firstCall.args[1];
assert.strictEqual( data.navigationType, 'navigate',
'navigationType is logged correctly' );
@@ -94,10 +94,10 @@
}] ) },
config: { wgImageMetricsSamplingFactor: 1 }
},
- imageMetrics = createImageMetrics( this.sandbox,
options );
+ logger = createLoadingTimeLogger( this.sandbox, options
);
$( '#qunit-fixture' ).append( '<div id="file"><img
alt="Foo.jpg" /></div>' );
- imageMetrics.log();
+ logger.collect();
data = options.logEvent.firstCall.args[1];
assert.strictEqual( data.ownLoadingTime, 111, 'ownLoadingTime
is logged correctly' );
@@ -110,10 +110,10 @@
performance: { navigation: { type: 0 } },
samplingFactor: 1
},
- imageMetrics = createImageMetrics( this.sandbox,
options );
+ logger = createLoadingTimeLogger( this.sandbox, options
);
$( '#qunit-fixture' ).append( '<div id="file"></div>' );
- imageMetrics.log();
+ logger.collect();
assert.strictEqual( options.logEvent.called, false, 'logEvent
was not called' );
} );
--
To view, visit https://gerrit.wikimedia.org/r/182762
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I414c0b51401597f8acfec472c030c6f2e7d8639f
Gerrit-PatchSet: 3
Gerrit-Project: mediawiki/extensions/ImageMetrics
Gerrit-Branch: master
Gerrit-Owner: Gergő Tisza <[email protected]>
Gerrit-Reviewer: Gergő Tisza <[email protected]>
Gerrit-Reviewer: Gilles <[email protected]>
Gerrit-Reviewer: MarkTraceur <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
