r97154 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r97153‎ | r97154 | r97155 >
Date:14:17, 15 September 2011
Author:catrope
Status:ok (Comments)
Tags:
Comment:
RL2: Rewrite of ext.gadgets.api that Timo and I worked on together at http://etherpad.wikimedia.org/RL2-GadgetAbstract2 . ext.gadgets.api now supports foreign repositories as well.

I just copied the code from Etherpad verbatim, with some whitespace and comment tweaks. I'll fix it up properly in the next commit.
Modified paths:
  • /branches/RL2/extensions/Gadgets/modules/ext.gadgets.api.js (modified) (history)

Diff [purge]

Index: branches/RL2/extensions/Gadgets/modules/ext.gadgets.api.js
@@ -1,24 +1,26 @@
22 /**
3 - * Implement the editing API for the gadget manager.
 3+ * Interface to the API for the gadgets extension.
44 *
55 * @author Timo Tijhof
6 - * @copyright © 2011 Timo Tijhof
 6+ * @author Roan Kattouw
77 * @license GNU General Public Licence 2.0 or later
88 */
 9+
910 ( function( $ ) {
10 -
1111 var
1212 /**
13 - * @var {Object} Keyed by gadget id, contains the metadata as an object.
 13+ * @var {Object} Keyed by repo, object of gadget objects by id
 14+ * @example { repoName: { gadgetID: { title: .., metadata: ..}, otherId: { .. } } }
1415 */
1516 gadgetCache = {},
1617 /**
17 - * @var {Object} If cached, object keyed by category id with categormember-count as value.
18 - * Set to null if there is no cache, yet, or when the cache is cleared. */
 18+ * @var {Object} Keyed by repo, array of category objects
 19+ * @example { repoName: [ {name: .., title: .., members: .. }, { .. }, { .. } ] }
 20+ */
1921 gadgetCategoryCache = null;
20 -
 22+
2123 /* Local functions */
22 -
 24+
2325 /**
2426 * For most returns from api.* functions, a clone is made when data from
2527 * cache is used. This is to avoid situations where later modifications
@@ -26,7 +28,7 @@
2729 * the object would otherwise be passed by reference).
2830 */
2931 function objClone( obj ) {
30 - /**
 32+ /*
3133 * A normal `$.extend( {}, obj );` is not suffecient,
3234 * it has to be recursive, because the values of this
3335 * object are also refererenes to objects.
@@ -40,55 +42,163 @@
4143 */
4244 return $.extend( true /* recursive */, {}, obj );
4345 }
 46+
4447 function arrClone( arr ) {
4548 return arr.slice();
4649 }
47 -
 50+
 51+ /**
 52+ * Reformat an array of gadget objects, into an object keyed by the id.
 53+ * Note: Maintains object reference
 54+ * @param arr {Array}
 55+ * @return {Object}
 56+ */
 57+ function gadgetArrToObj( arr ) {
 58+ for( var obj = {}, i = 0, g = arr[i], len = arr.length; i < len; g = arr[++i] ) {
 59+ obj[g.id] = g;
 60+ }
 61+ return obj;
 62+ }
 63+
 64+ /**
 65+ * Write data to gadgetCache, taking into account that id may be null
 66+ * and working around JS's annoying refusal to just let us do
 67+ * var foo = {}; foo[bar][baz] = quux;
 68+ *
 69+ * This sets gadgetCache[repoName][id] = data; if id is not null,
 70+ * or gadgetCache[repoName] = data; if id is null.
 71+ *
 72+ * @param repoName {String} Repository name
 73+ * @param id {String|null} Gadget ID or null
 74+ * @param data {Object} Data to put in the cache
 75+ */
 76+ function cacheGadgetData( repoName, id, data ) {
 77+ if ( id === null ) {
 78+ gadgetCache[repoName] = data;
 79+ } else {
 80+ if ( !( repoName in cache ) ) {
 81+ gadgetCache[repoName] = {};
 82+ }
 83+ gadgetCache[repoName][id] = data;
 84+ }
 85+ }
 86+
4887 /* Public functions */
49 -
50 - mw.gadgetManager = {
51 -
52 - conf: mw.config.get( 'gadgetManagerConf' ),
53 -
 88+
 89+ mw.gadgets = {
 90+ /**
 91+ * @todo: Add something derived from $wgGadgetRepositories to gadgetsConf
 92+ * ... + repos: { local: { apiScript: .. }, awesomeRepo: { .. }, .. }
 93+ */
 94+ conf: mw.config.get( 'gadgetsConf' ),
5495 api: {
55 -
5696 /**
 97+ * Get the gadget blobs for all gadgets from all repositories.
 98+ *
 99+ * @param success {Function} To be called with an object of arrays of gadget objects, keyed by repository name, as first argument.
 100+ * @param error {Function} To be called with a string (error code) as first argument.
 101+ */
 102+ getForeignGadgetsData: function( success, error ) {
 103+ var combined = {}, successes = 0, numRepos = 0, repo;
 104+ // Find out how many repos there are
 105+ // Needs to be in a separate loop because we have to have the final number ready
 106+ // before we fire the first potentially (since it could be cached) async request
 107+ for ( repo in this.conf.repos ) {
 108+ numRepos++;
 109+ }
 110+
 111+ for ( repo in this.conf.repos ) {
 112+ mw.gadgets.api.getGadgetData( null, function( data ) {
 113+ combined[repo] = data;
 114+ if ( ++successes === numRepos ) {
 115+ success( combined );
 116+ }
 117+ }, function( errorCode ) {
 118+ error( errorCode );
 119+ }, repo
 120+ );
 121+ }
 122+ },
 123+
 124+ /**
 125+ * Get the gadget categories from all repositories.
 126+ *
 127+ * @param success {Function} To be called with an array
 128+ * @param success {Function} To be called with an object of arrays of category objects, keyed by repository name, as first argument.
 129+ * @param error {Function} To be called with a string (error code) as the first argument.
 130+ */
 131+ getForeignGadgetCategories: function( success, error ){
 132+ // TODO: Almost entirely duplicated from the function above. Factoring this out is easy
 133+ var combined = {}, successes = 0, numRepos = 0, repo;
 134+ // Find out how many repos there are
 135+ // Needs to be in a separate loop because we have to have the final number ready
 136+ // before we fire the first async request
 137+ for ( repo in this.conf.repos ) {
 138+ numRepos++;
 139+ }
 140+
 141+ for ( repo in this.conf.repos ) {
 142+ mw.gadgets.api.getGadgetCategories( function( data ) {
 143+ combined[repo] = data;
 144+ if ( ++successes === numRepos ) {
 145+ success( combined );
 146+ }
 147+ }, function( errorCode ) {
 148+ error( errorCode );
 149+ }, repo
 150+ );
 151+ }
 152+ },
 153+ /**
57154 * Get gadget blob from the API (or from cache if available).
58155 *
59 - * @param id {String} Gadget id.
60 - * @param success {Function} To be called with the gadget object as first argument.
61 - * @param error {Fucntion} If something went wrong (inexisting gadget, api
 156+ * @param id {String|null} Gadget id, or null to get all from the repo.
 157+ * @param success {Function} To be called with the gadget object or array of gadget objects as first argument.
 158+ * @param error {Function} If something went wrong (inexisting gadget, api
62159 * error, request error), this is called with error code as first argument.
63 - * @return {jqXHR|Null}: Null if served from cache, otherwise the jqXHR.
 160+ * @param repoName {String} Name of the repository, key in this.conf.repos
64161 */
65 - getGadgetData: function( id, success, error ) {
 162+ getGadgetData: function( id, success, error, repoName ) {
66163 // Check cache
67 - if ( id in gadgetCache && gadgetCache[id] !== null ) {
68 - success( objClone( gadgetCache[id] ) );
69 - return null;
 164+ if ( repoName in gadgetCache && gadgetCache[repoName] !== null ) {
 165+ if ( id === null ) {
 166+ success( objClone( gadgetCache[repoName] ) );
 167+ return;
 168+ } else if ( id in gadgetCache[repoName] && gadgetCache[repoName][id] !== null ) {
 169+ success( objClone( gadgetCache[repoName][id] ) );
 170+ return;
 171+ }
70172 }
71173 // Get from API if not cached
72 - return $.ajax({
73 - url: mw.util.wikiScript( 'api' ),
74 - data: {
75 - format: 'json',
76 - action: 'query',
77 - list: 'gadgets',
78 - gaprop: 'id|title|metadata|definitiontimestamp',
79 - gaids: id,
80 - galanguage: mw.config.get( 'wgUserLanguage' )
81 - },
 174+ var queryData = {
 175+ format: 'json',
 176+ action: 'query',
 177+ list: 'gadgets',
 178+ gaprop: 'id|title|metadata|definitiontimestamp',
 179+ galanguage: mw.config.get( 'wgUserLanguage' )
 180+ };
 181+ if ( id !== null ) {
 182+ data.gaids = id;
 183+ }
 184+ $.ajax({
 185+ url: this.conf.repos[repoName].apiScript || mw.util.wikiScript( 'api' ),
 186+ data: queryData,
82187 type: 'GET',
83188 dataType: 'json',
84189 success: function( data ) {
85 - if ( data && data.query && data.query.gadgets && data.query.gadgets[0] ) {
86 - data = data.query.gadgets[0];
 190+ if ( data && data.query && data.query.gadgets ) {
 191+ data = data.query.gadgets;
 192+ if ( id !== null ) {
 193+ data = data[0] || null;
 194+ } else {
 195+ data = gadgetArrToObj( data );
 196+ }
87197 // Update cache
88 - gadgetCache[id] = data;
 198+ cacheGadgetData( repoName, id, data );
89199 success( objClone( data ) );
90200 } else {
91201 // Invalidate cache
92 - gadgetCache[id] = null;
 202+ cacheGadgetData( repoName, id, null );
93203 if ( data && data.error ) {
94204 error( data.error.code );
95205 } else {
@@ -98,25 +208,28 @@
99209 },
100210 error: function() {
101211 // Invalidate cache
102 - gadgetCache[id] = null;
 212+ cacheGadgetData( repoName, id, null );
103213 error( 'unknown' );
104214 }
105215 });
106216 },
107 -
108217 /**
109 - * @param callback {Function} To be called with an array as first argument.
 218+ * Get the gadget categories for a certain repository from the API.
 219+ *
 220+ * @param success {Function} To be called with an array as first argument.
 221+ * @param error {Function} To be called with a string (error code) as first argument.
 222+ * @param repoName {String} Name of the repository, key in this.conf.repos
110223 * @return {jqXHR|Null}: Null if served from cache, otherwise the jqXHR.
111224 */
112 - getGadgetCategories: function( callback ) {
 225+ getGadgetCategories: function( success, error, repoName ) {
113226 // Check cache
114 - if ( gadgetCategoryCache !== null ) {
115 - callback( arrClone( gadgetCategoryCache ) );
 227+ if ( repoName in gadgetCategoryCache && gadgetCategoryCache[repoName] !== null ) {
 228+ success( arrClone( gadgetCategoryCache[repoName] ) );
116229 return null;
117230 }
118231 // Get from API if not cached
119232 return $.ajax({
120 - url: mw.util.wikiScript( 'api' ),
 233+ url: this.conf.repos[repoName].apiScript || mw.util.wikiScript( 'api' ),
121234 data: {
122235 format: 'json',
123236 action: 'query',
@@ -132,22 +245,25 @@
133246 {
134247 data = data.query.gadgetcategories;
135248 // Update cache
136 - gadgetCategoryCache = data;
137 - callback( arrClone( data ), 'success' );
 249+ gadgetCategoryCache[repoName] = data;
 250+ success( arrClone( data ) );
138251 } else {
139252 // Invalidate cache
140 - gadgetCategoryCache = null;
141 - callback( [] );
 253+ gadgetCategoryCache[repoName] = null;
 254+ if ( data && data.error ) {
 255+ error( data.error.code );
 256+ } else {
 257+ error( 'unknown' );
 258+ }
142259 }
143260 },
144261 error: function() {
145262 // Invalidate cache
146 - gadgetCategoryCache = null;
147 - callback( [] );
 263+ gadgetCategoryCache[repoName] = null;
 264+ error( 'unknown' );
148265 }
149266 });
150267 },
151 -
152268 /**
153269 * Creates or edits an existing gadget definition.
154270 *
@@ -183,8 +299,7 @@
184300 dataType: 'json',
185301 success: function( data ) {
186302 // Invalidate cache
187 - gadgetCache[gadget.id] = null;
188 -
 303+ cacheGadgetData( 'local', id, null );
189304 if ( data && data.edit && data.edit ) {
190305 if ( data.edit.result === 'Success' ) {
191306 o.success( data.edit );
@@ -199,12 +314,11 @@
200315 },
201316 error: function(){
202317 // Invalidate cache
203 - gadgetCache[gadget.id] = null;
 318+ cacheGadgetData( 'local', id, null );
204319 o.error( 'unknown' );
205320 }
206321 });
207322 },
208 -
209323 /**
210324 * Deletes a gadget definition.
211325 *
@@ -215,11 +329,10 @@
216330 doDeleteGadget: function( id, success, error ) {
217331 // @todo ApiDelete
218332 // Invalidate cache
219 - gadgetCache[id] = null;
 333+ cacheGadgetData( 'local', id, null );
220334 error( '@todo' );
221335 return null;
222336 }
223337 }
224338 };
225 -
226339 })( jQuery );

Follow-up revisions

RevisionCommit summaryAuthorDate
r97162RL2: Followup r97154, use mw.gadgets instead of mw.gadgetManagercatrope15:48, 15 September 2011
r97163RL2: Followup r97154, add dependencies for ext.gadgets.apicatrope15:50, 15 September 2011
r97164RL2: Followup r97154, rename gadgetManagerConf to gadgetConf and add the repo...catrope15:51, 15 September 2011
r97166RL2: Followup r97154, various fixes for ext.gadgets.api . This unbreaks Speci...catrope15:57, 15 September 2011
r97176RL2: Followup r97154, factor out the common code in getForeignGadgetsData() a...catrope16:29, 15 September 2011
r97364[RL2] ReferenceError: id not defined. Fix copy-paste failure from r97154 (see...krinkle02:37, 17 September 2011

Comments

#Comment by Krinkle (talk | contribs)   02:38, 17 September 2011

 					success: function( data ) {
 						// Invalidate cache
-						gadgetCache[gadget.id] = null;
-
+						cacheGadgetData( 'local', id, null );
 						if ( data && data.edit && data.edit ) {
 							if ( data.edit.result === 'Success' ) {
 								o.success( data.edit );
@@ -199,12 +314,11 @@
 					},
 					error: function(){
 						// Invalidate cache
-						gadgetCache[gadget.id] = null;
+						cacheGadgetData( 'local', id, null );
 						o.error( 'unknown' );

This broke ajax saving of gadgets as id is undefined. Just like previous code it's in gadget.id

Probably one of us making a copy-paste from another section on the etherpad.

Fixed in r97364

Status & tagging log