jenkins-bot has submitted this change and it was merged. Change subject: Replace old API code with providers + fix a few provider bugs ......................................................................
Replace old API code with providers + fix a few provider bugs Mingle: https://wikimedia.mingle.thoughtworks.com/projects/multimedia/cards/123 Change-Id: Id7952f0a0eafa34d7c8691c63c2daccff2eea64a --- M resources/mmv/mmv.js M resources/mmv/provider/mmv.provider.FileRepoInfo.js M resources/mmv/provider/mmv.provider.ImageInfo.js M resources/mmv/provider/mmv.provider.ThumbnailInfo.js M tests/qunit/provider/mmv.provider.ThumbnailInfo.test.js 5 files changed, 63 insertions(+), 140 deletions(-) Approvals: Aarcos: Looks good to me, approved jenkins-bot: Verified diff --git a/resources/mmv/mmv.js b/resources/mmv/mmv.js index af52dd1..98d279b 100755 --- a/resources/mmv/mmv.js +++ b/resources/mmv/mmv.js @@ -32,20 +32,6 @@ 'tif': true }, - iiprops = [ - 'timestamp', - 'user', - 'userid', - 'comment', - 'url', - 'size', - 'sha1', - 'mime', - 'mediatype', - 'metadata', - 'extmetadata' - ], - mmvLogActions = { 'thumbnail-link-click': 'User clicked on thumbnail to open lightbox.', 'enlarge-link-click': 'User clicked on enlarge link to open lightbox.', @@ -115,33 +101,44 @@ this.api = new mw.Api(); /** + * @property {mw.mmv.provider.ImageInfo} + * @private + */ + this.imageInfoProvider = new mw.mmv.provider.ImageInfo( this.api, { + // Short-circuit, don't fallback, to save some tiny amount of time + language: mw.config.get( 'wgUserLanguage', false ) || mw.config.get( 'wgContentLanguage', 'en' ) + } ); + + /** + * @property {mw.mmv.provider.FileRepoInfo} + * @private + */ + this.fileRepoInfoProvider = new mw.mmv.provider.FileRepoInfo( this.api ); + + /** + * @property {mw.mmv.provider.ThumbnailInfo} + * @private + */ + this.thumbnailInfoProvider = new mw.mmv.provider.ThumbnailInfo( this.api ); + + /** * @property {mw.mmv.provider.ImageUsage} * @private */ - this.imageUsageDataProvider = new mw.mmv.provider.ImageUsage( this.api ); + this.imageUsageProvider = new mw.mmv.provider.ImageUsage( this.api ); /** * @property {mw.mmv.provider.GlobalUsage} * @private */ - this.globalUsageDataProvider = new mw.mmv.provider.GlobalUsage( this.api, { + this.globalUsageProvider = new mw.mmv.provider.GlobalUsage( this.api, { doNotUseApi: !mw.config.get( 'wgMultimediaViewer' ).globalUsageAvailable } ); // replace with this one to test global usage on a local wiki without going through all the // hassle required for installing the extension: - //this.globalUsageDataProvider = new mw.mmv.provider.GlobalUsage( + //this.globalUsageProvider = new mw.mmv.provider.GlobalUsage( // new mw.Api( {ajax: { url: 'http://commons.wikimedia.org/w/api.php', dataType: 'jsonp' } } ) //); - - /** - * imageInfo object, used for caching - promises will resolve with - * an mw.mmv.model.Image object, a repoInfo object, the best width for - * the current screen configuration, and the width requested from - * the server (if any). - * @property {jQuery.Promise[]} - * @private - */ - this.imageInfo = {}; // Traverse DOM, looking for potential thumbnails $thumbs.each( function ( i, thumb ) { @@ -339,7 +336,7 @@ fileTitle = this.currentImageFileTitle; if ( fileTitle ) { - this.fetchImageInfo( fileTitle, [ 'url' ] ).done( function ( imageData, repoInfo, targetWidth, requestedWidth ) { + this.fetchImageInfo( fileTitle ).done( function ( imageData, repoInfo, targetWidth, requestedWidth ) { viewer.loadResizedImage( ui, imageData, targetWidth, requestedWidth ); } ); } @@ -396,21 +393,6 @@ viewer.log( 'defullscreen-link-click' ); } } ); - }; - - MMVP.cacheRepoInfo = function ( repos ) { - var i, repo; - - repos = repos || []; - - if ( this.repoInfo === undefined ) { - this.repoInfo = {}; - } - - for ( i = 0; i < repos.length; i++ ) { - repo = repos[i]; - this.repoInfo[repo.name] = repo; - } }; /** @@ -749,93 +731,23 @@ * * The target * @param {mw.Title} fileTitle Title of the file page for the image. - * @param {string[]} [props] List of properties to get from imageinfo * @returns {jQuery.Promise} */ - MMVP.fetchImageInfo = function ( fileTitle, props ) { - var filename = fileTitle.getPrefixedText(), - apiArgs = { - action: 'query', - format: 'json', - titles: filename, - prop: 'imageinfo', - // Short-circuit, don't fallback, to save some tiny amount of time - iiextmetadatalanguage: mw.config.get( 'wgUserLanguage', false ) || mw.config.get( 'wgContentLanguage', 'en' ) - }, - viewer = this, - - widths = this.getImageSizeApiArgs( this.ui ), + MMVP.fetchImageInfo = function ( fileTitle ) { + var widths = this.getImageSizeApiArgs( this.ui ), targetWidth = widths.target, requestedWidth = widths.requested; - function handleApiData( data ) { - var imageInfo, imageData; - - if ( !data || !data.query ) { - // No information, oh well - return $.Deferred().reject(); - } - - viewer.cacheRepoInfo( data.query.repos ); - imageInfo = viewer.getFirst( data.query.pages ); - - if ( imageInfo ) { - imageData = mw.mmv.model.Image.newFromImageInfo( fileTitle, imageInfo ); - - // Give back the information we have - return $.Deferred().resolve( imageData, viewer.repoInfo, targetWidth, requestedWidth ); - } else { - return $.Deferred().reject(); - } - } - - function makeImageInfoRequest( args ) { - if ( viewer.repoInfo === undefined ) { - args.meta = 'filerepoinfo'; - } - - return viewer.api.get( args ).then( handleApiData ); - } - - props = props || iiprops; - apiArgs.iiprop = props.join( '|' ); - apiArgs.iiurlwidth = requestedWidth; - - if ( this.imageInfo[filename] === undefined ) { - // Fetch all image info in the same API query, save a request later - apiArgs.iiprop = iiprops.join( '|' ); - this.imageInfo[filename] = makeImageInfoRequest( apiArgs ); - return this.imageInfo[filename]; - } else if ( props.indexOf( 'url' ) === -1 ) { - // Just return the cached promise, because we don't need to - // fetch this information again. - return this.imageInfo[filename]; - } else { - // Fetch the new thumb url but nothing else, because it's - // the only non-cacheable thing - apiArgs.iiprop = 'url'; - return this.imageInfo[filename].then( function ( imageData, repoInfo ) { - var maybeThumb = imageData.getThumbUrl( requestedWidth ); - - // Thumbnail caching! Woo! - if ( maybeThumb ) { - return $.Deferred().resolve( imageData, repoInfo, targetWidth, requestedWidth ); - } - - return viewer.api.get( apiArgs ).then( function ( data ) { - var imageInfo, innerInfo; - - imageInfo = viewer.getFirst( data.query.pages ); - innerInfo = viewer.getFirst( imageInfo.imageinfo ); - - if ( innerInfo.thumburl ) { - imageData.addThumbUrl( innerInfo.thumbwidth, innerInfo.thumburl ); - } - - return $.Deferred().resolve( imageData, repoInfo, targetWidth, requestedWidth ); - } ); - } ); - } + return $.when( + this.fileRepoInfoProvider.get(), + this.imageInfoProvider.get( fileTitle ), + this.thumbnailInfoProvider.get( fileTitle, requestedWidth ) + ).then( function( fileRepoInfoHash, imageInfo, thumbnailData ) { + var thumbnailUrl = thumbnailData[0], + thumbnailWidth = thumbnailData[1]; + imageInfo.addThumbUrl( thumbnailWidth, thumbnailUrl ); + return $.Deferred().resolve( imageInfo, fileRepoInfoHash, targetWidth, requestedWidth ); + } ); }; /** @@ -848,8 +760,8 @@ */ MMVP.fetchFileUsageInfo = function ( fileTitle ) { return $.when( - this.imageUsageDataProvider.get( fileTitle ), - this.globalUsageDataProvider.get( fileTitle ) + this.imageUsageProvider.get( fileTitle ), + this.globalUsageProvider.get( fileTitle ) ); }; diff --git a/resources/mmv/provider/mmv.provider.FileRepoInfo.js b/resources/mmv/provider/mmv.provider.FileRepoInfo.js index 799750f..9a17ff6 100644 --- a/resources/mmv/provider/mmv.provider.FileRepoInfo.js +++ b/resources/mmv/provider/mmv.provider.FileRepoInfo.js @@ -32,7 +32,8 @@ /** * @method * Runs an API GET request to get the repo info. - * @return {jQuery.Promise} a promise which resolves to an array of mw.mmv.model.Repo objects. + * @return {jQuery.Promise<Object<string, mw.mmv.model.Repo>>} a promise which resolves to + * a hash of mw.mmv.model.Repo objects, indexed by repo names. */ FileRepoInfo.prototype.get = function() { var provider = this; diff --git a/resources/mmv/provider/mmv.provider.ImageInfo.js b/resources/mmv/provider/mmv.provider.ImageInfo.js index b2c8c49..b9fbb55 100644 --- a/resources/mmv/provider/mmv.provider.ImageInfo.js +++ b/resources/mmv/provider/mmv.provider.ImageInfo.js @@ -24,15 +24,21 @@ * @extends mw.mmv.provider.Api * @inheritdoc * @param {mw.Api} api + * @param {Object} [options] + * @param {string} [options.language] image metadata language */ - function ImageInfo( api ) { - mw.mmv.provider.Api.call( this, api ); + function ImageInfo( api, options ) { + options = $.extend( { + language: null + }, options ); + + mw.mmv.provider.Api.call( this, api, options ); } oo.inheritClass( ImageInfo, mw.mmv.provider.Api ); /** * List of imageinfo API properties which are needed to construct an Image model. - * @type {string[]} + * @type {string} */ ImageInfo.prototype.iiprop = [ 'timestamp', @@ -42,11 +48,11 @@ 'mime', 'mediatype', 'extmetadata' - ]; + ].join('|'); /** * List of imageinfo extmetadata fields which are needed to construct an Image model. - * @type {string[]} + * @type {string} */ ImageInfo.prototype.iiextmetadatafilter = [ 'DateTime', @@ -57,7 +63,7 @@ 'Artist', 'GPSLatitude', 'GPSLongitude' - ]; + ].join('|'); /** * @method @@ -76,6 +82,7 @@ titles: file.getPrefixedDb(), iiprop: this.iiprop, iiextmetadatafilter: this.iiextmetadatafilter, + iiextmetadatalanguage: this.options.language, format: 'json' } ).then( function( data ) { return provider.getQueryPage( file, data ); diff --git a/resources/mmv/provider/mmv.provider.ThumbnailInfo.js b/resources/mmv/provider/mmv.provider.ThumbnailInfo.js index f5c044d..2affa77 100644 --- a/resources/mmv/provider/mmv.provider.ThumbnailInfo.js +++ b/resources/mmv/provider/mmv.provider.ThumbnailInfo.js @@ -35,7 +35,9 @@ * Runs an API GET request to get the thumbnail info. * @param {mw.Title} file * @param {number} width thumbnail width - * @return {jQuery.Promise} a promise which resolves to the thumbnail URL + * @return {jQuery.Promise<string, number>} a promise which resolves to the thumbnail URL and + * the actual width of the thumbnail (which might be smaller than the requested width, + * in case the size we requested was larger than the full image size). */ ThumbnailInfo.prototype.get = function( file, width ) { var provider = this, @@ -46,14 +48,14 @@ action: 'query', prop: 'imageinfo', titles: file.getPrefixedDb(), - iiprop: ['url'], + iiprop: 'url', iiurlwidth: width, format: 'json' } ).then( function( data ) { return provider.getQueryPage( file, data ); } ).then( function( page ) { if ( page.imageinfo && page.imageinfo[0] ) { - return page.imageinfo[0].thumburl; + return $.Deferred().resolve( page.imageinfo[0].thumburl, page.imageinfo[0].thumbwidth ); } else if ( page.missing === '' && page.imagerepository === '' ) { return $.Deferred().reject( 'file does not exist: ' + file.getPrefixedDb() ); } else { diff --git a/tests/qunit/provider/mmv.provider.ThumbnailInfo.test.js b/tests/qunit/provider/mmv.provider.ThumbnailInfo.test.js index f3d9168..27b4145 100644 --- a/tests/qunit/provider/mmv.provider.ThumbnailInfo.test.js +++ b/tests/qunit/provider/mmv.provider.ThumbnailInfo.test.js @@ -25,7 +25,7 @@ assert.ok( thumbnailInfoProvider ); } ); - QUnit.asyncTest( 'ThumbnailInfo get test', 4, function ( assert ) { + QUnit.asyncTest( 'ThumbnailInfo get test', 5, function ( assert ) { var apiCallCount = 0, api = { get: function() { apiCallCount++; @@ -40,7 +40,7 @@ imageinfo: [ { thumburl: 'https://upload.wikimedia.org/wikipedia/commons/thumb/1/19/Stuff.jpg/51px-Stuff.jpg', - thumbwidth: 100, + thumbwidth: 95, thumbheight: 200, url: 'https://upload.wikimedia.org/wikipedia/commons/1/19/Stuff.jpg', descriptionurl: 'https://commons.wikimedia.org/wiki/File:Stuff.jpg' @@ -54,10 +54,11 @@ file = new mw.Title( 'File:Stuff.jpg' ), thumbnailInfoProvider = new mw.mmv.provider.ThumbnailInfo( api ); - thumbnailInfoProvider.get( file, 100 ).then( function( thumnailUrl ) { + thumbnailInfoProvider.get( file, 100 ).then( function( thumnailUrl, thumbnailWidth ) { assert.strictEqual( thumnailUrl, 'https://upload.wikimedia.org/wikipedia/commons/thumb/1/19/Stuff.jpg/51px-Stuff.jpg', 'URL is set correctly' ); + assert.strictEqual( thumbnailWidth, 95, 'actual width is set correctly' ); } ).then( function() { assert.strictEqual( apiCallCount, 1 ); // call the data provider a second time to check caching -- To view, visit https://gerrit.wikimedia.org/r/111151 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: Id7952f0a0eafa34d7c8691c63c2daccff2eea64a Gerrit-PatchSet: 4 Gerrit-Project: mediawiki/extensions/MultimediaViewer Gerrit-Branch: master Gerrit-Owner: Gergő Tisza <[email protected]> Gerrit-Reviewer: Aarcos <[email protected]> Gerrit-Reviewer: Gergő Tisza <[email protected]> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
