Trevor Parscal has uploaded a new change for review.
https://gerrit.wikimedia.org/r/71161
Change subject: Improve async template data loading
......................................................................
Improve async template data loading
Objective:
* Fix issue where async behavior of addTemplate caused templates to be added in
the wrong order
Bonus:
* Get rid of special adders for transclusion parts, just construct objects
outside and use addPart
Change-Id: Ibe579f033873446376d72d3bd1b9f92d9f361de5
---
M modules/ve/dm/models/ve.dm.MWTemplateModel.js
M modules/ve/dm/models/ve.dm.MWTemplateParameterModel.js
M modules/ve/dm/models/ve.dm.MWTransclusionModel.js
M modules/ve/ui/dialogs/ve.ui.MWTransclusionDialog.js
4 files changed, 115 insertions(+), 172 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/VisualEditor
refs/changes/61/71161/1
diff --git a/modules/ve/dm/models/ve.dm.MWTemplateModel.js
b/modules/ve/dm/models/ve.dm.MWTemplateModel.js
index bea88e6..52acc5a 100644
--- a/modules/ve/dm/models/ve.dm.MWTemplateModel.js
+++ b/modules/ve/dm/models/ve.dm.MWTemplateModel.js
@@ -135,32 +135,30 @@
* Add a parameter to template.
*
* @method
+ * @param {ve.dm.MWTemplateParameterModel} param Parameter to add
* @param {string} name Parameter name
* @param {string} value Parameter value
- * @returns {ve.dm.MWTemplateParameterModel} Added param
* @emits add
*/
-ve.dm.MWTemplateModel.prototype.addParameter = function ( name, value ) {
- var param = new ve.dm.MWTemplateParameterModel( this, name, value );
+ve.dm.MWTemplateModel.prototype.addParameter = function ( param ) {
+ var name = param.getName();
this.sequence = null;
this.params[name] = param;
this.spec.fill();
this.emit( 'add', param );
- return param;
};
/**
* Remove parameter from template.
*
* @method
- * @param {string} name Parameter name
+ * @param {ve.dm.MWTemplateParameterModel} param Parameter to remove
* @emits remove
*/
-ve.dm.MWTemplateModel.prototype.removeParameter = function ( name ) {
- var param = this.params[name];
+ve.dm.MWTemplateModel.prototype.removeParameter = function ( param ) {
if ( param ) {
this.sequence = null;
- delete this.params[name];
+ delete this.params[param.getName()];
this.emit( 'remove', param );
}
};
diff --git a/modules/ve/dm/models/ve.dm.MWTemplateParameterModel.js
b/modules/ve/dm/models/ve.dm.MWTemplateParameterModel.js
index 6b1c389..ed5836e 100644
--- a/modules/ve/dm/models/ve.dm.MWTemplateParameterModel.js
+++ b/modules/ve/dm/models/ve.dm.MWTemplateParameterModel.js
@@ -80,5 +80,5 @@
* @method
*/
ve.dm.MWTemplateParameterModel.prototype.remove = function () {
- this.template.removeParameter( this.name );
+ this.template.removeParameter( this );
};
diff --git a/modules/ve/dm/models/ve.dm.MWTransclusionModel.js
b/modules/ve/dm/models/ve.dm.MWTransclusionModel.js
index de0d845..1599b1f 100644
--- a/modules/ve/dm/models/ve.dm.MWTransclusionModel.js
+++ b/modules/ve/dm/models/ve.dm.MWTransclusionModel.js
@@ -27,6 +27,7 @@
this.parts = [];
this.uid = 0;
this.requests = [];
+ this.queue = [];
};
/* Inheritance */
@@ -55,8 +56,7 @@
* @returns {jQuery.Promise} Promise, resolved when spec is loaded
*/
ve.dm.MWTransclusionModel.prototype.load = function ( data ) {
- var i, len, key, part, template,
- templates = [];
+ var i, len, key, part, template;
// Convert single part format to multi-part format
if ( data.params && data.target ) {
@@ -67,50 +67,85 @@
for ( i = 0, len = data.parts.length; i < len; i++ ) {
part = data.parts[i];
if ( part.template ) {
- template = this.addTemplate(
part.template.target );
+ template = new ve.dm.MWTemplateModel( this,
part.template.target );
for ( key in part.template.params ) {
- template.addParameter( key,
part.template.params[key].wt );
+ template.addParameter(
+ new
ve.dm.MWTemplateParameterModel(
+ template, key,
part.template.params[key].wt
+ )
+ );
}
- // Don't load specs for templates that don't
have a resolvable target
- if ( part.template.target.href ) {
- templates.push( template );
- }
+ this.queue.push( { 'part': template } );
} else if ( typeof part === 'string' ) {
- this.addContent( part );
+ this.queue.push( { 'part': new
ve.dm.MWTransclusionContentModel( this, part ) } );
}
}
+ setTimeout( ve.bind( this.fetch, this ) );
}
- // Add fetched specs to #specs store when the promise is resolved
- return this.fetchSpecs( templates ).done( function ( specs ) {
- ve.extendObject( specCache, specs );
- } );
};
/**
- * Fetch template specifications from server.
+ * Process one or more queue items.
*
- * @param {ve.dm.MWTransclusionModel[]} templates List of templates to load
data for
- * @returns {jQuery.Promise} Promise, resolved when spec is loaded
+ * @method
+ * @param {Object[]} queue List of objects containing parts to add and
optionally indexes to add
+ * them at, if no index is given parts will be added at the end
+ * @emits add For each item added
*/
-ve.dm.MWTransclusionModel.prototype.fetchSpecs = function ( templates ) {
- var i, len, title, request,
- requests = this.requests,
- deferred = $.Deferred(),
- specs = {},
- titles = [];
+ve.dm.MWTransclusionModel.prototype.process = function ( queue ) {
+ var i, len, item, title, index;
- // Get unique list of titles that aren't already loaded
- for ( i = 0, len = templates.length; i < len; i++ ) {
- title = templates[i].getTitle();
- if ( !specCache[title] && ve.indexOf( title, titles ) === -1 ) {
- titles.push( title );
+ for ( i = 0, len = queue.length; i < len; i++ ) {
+ item = queue[i];
+ if ( item.part instanceof ve.dm.MWTemplateModel ) {
+ title = item.part.getTitle();
+ if ( hasOwn.call( specCache, title ) &&
specCache[title] ) {
+ item.part.getSpec().extend( specCache[title] );
+ }
+ }
+ index = item.index === undefined ? this.parts.length :
item.index;
+ this.parts.splice( index, 0, item.part );
+ this.emit( 'add', item.part );
+ }
+};
+
+ve.dm.MWTransclusionModel.prototype.fetch = function () {
+ if ( !this.queue.length ) {
+ return;
+ }
+
+ var i, len, item, title, request,
+ titles = [],
+ specs = {},
+ queue = this.queue.slice();
+
+ // Clear shared queue for future calls
+ this.queue.length = 0;
+
+ // Get unique list of template titles that aren't already loaded
+ for ( i = 0, len = queue.length; i < len; i++ ) {
+ item = queue[i];
+ if ( item.part instanceof ve.dm.MWTemplateModel ) {
+ title = item.part.getTitle();
+ if (
+ // Skip titles that don't have a resolvable href
+ title &&
+ // Skip titles outside the template namespace
+ title.charAt( 0 ) !== ':' &&
+ // Skip already cached data
+ !hasOwn.call( specCache, title ) &&
+ // Skip duplicate titles in the same batch
+ ve.indexOf( title, titles ) === -1
+ ) {
+ titles.push( title );
+ }
}
}
// Bypass server for empty lists
if ( !titles.length ) {
- setTimeout( deferred.reject );
- return deferred.promise();
+ setTimeout( ve.bind( this.process, this, queue ) );
+ return;
}
// Request template specs from server
@@ -124,7 +159,7 @@
}
} )
.done( function ( data ) {
- var i, len, id, title;
+ var i, len, id;
if ( data && data.pages ) {
// Keep spec data on hand for future use
@@ -141,31 +176,28 @@
}
}
}
- // Load into existing templates
- for ( i = 0, len = templates.length; i < len;
i++ ) {
- title = templates[i].getTitle();
- if ( hasOwn.call( specs, title ) ) {
- templates[i].getSpec().extend(
specs[title] );
+ // Prevent asking again for templates that have
no specs
+ for ( i = 0, len = titles.length; i < len; i++
) {
+ title = titles[i];
+ if ( !specs[title] ) {
+ specs[title] = null;
}
}
- deferred.resolve( specs );
- } else {
- deferred.reject( 'unavailable', arguments );
- }
- } )
- .fail( function () {
- deferred.reject( 'http', arguments );
- } )
- .always( function () {
- // Prune requests when complete
- var index = requests.indexOf( request );
- if ( index !== -1 ) {
- requests.splice( index, 1 );
- }
- } );
- requests.push( request );
- return deferred.promise();
+ ve.extendObject( specCache, specs );
+ }
+ } )
+ .always( ve.bind( function () {
+ // Prune completed request
+ var index = ve.indexOf( request, this.requests );
+ if ( index !== -1 ) {
+ this.requests.splice( index, 1 );
+ }
+ // Actually add queued items
+ this.process( queue );
+ }, this ) );
+
+ this.requests.push( request );
};
/**
@@ -231,78 +263,23 @@
};
/**
- * Add content part.
- *
- * @method
- * @param {string} value Content value
- * @param {number} [index] Specific index to add content at
- * @returns {ve.dm.MWTransclusionContentModel} Added content part
- * @emits add
- */
-ve.dm.MWTransclusionModel.prototype.addContent = function ( value, index ) {
- var part = new ve.dm.MWTransclusionContentModel( this, value );
- this.addPart( part, index );
- return part;
-};
-
-/**
- * Add template part.
- *
- * Templates are added asynchronously.
- *
- * @method
- * @param {Object} target Template target
- * @param {string} target.wt Original wikitext of target
- * @param {string} [target.href] Hypertext reference to target
- * @param {number} [index] Specific index to add template at
- * @returns {ve.dm.MWTemplateModel} Added template part
- * @emits add
- */
-ve.dm.MWTransclusionModel.prototype.addTemplate = function ( target, index ) {
- var part = new ve.dm.MWTemplateModel( this, target ),
- title = part.getTitle(),
- finish = ve.bind( this.addPart, this, part, index );
-
- if ( hasOwn.call( specCache, title ) ) {
- part.getSpec().extend( specCache[title] );
- setTimeout( finish );
- } else {
- // Add fetched specs to #specs store when the promise is
resolved
- this.fetchSpecs( [ part ] )
- .done( function ( specs ) {
- ve.extendObject( specCache, specs );
- } )
- .always( finish );
- }
- return part;
-};
-
-/**
- * Add template placeholder part.
- *
- * @method
- * @param {number} [index] Specific index to add placeholder at
- * @returns {ve.dm.MWTransclusionModel} Added template part
- * @emits add
- */
-ve.dm.MWTransclusionModel.prototype.addPlaceholder = function ( index ) {
- var part = new ve.dm.MWTemplatePlaceholderModel( this );
-
- this.addPart( part, index );
- return part;
-};
-
-/**
* Add part.
+ *
+ * Added asynchronously, but order is preserved.
*
* @method
* @param {ve.dm.MWTransclusionPartModel} part Part to add
- * @param {number} [index] Specific index to add content at
- * @emits add
+ * @param {number} [index] Specific index to add content at, defaults to the
end
+ * @throws {Error} If part is not valid
*/
ve.dm.MWTransclusionModel.prototype.addPart = function ( part, index ) {
- this.parts.splice( index === undefined ? this.parts.length : index, 0,
part );
- this.emit( 'add', part );
+ if ( !( part instanceof ve.dm.MWTransclusionPartModel ) ) {
+ throw new Error( 'Invalid transclusion part' );
+ }
+ this.queue.push( { 'part': part, 'index': index } );
+ // Fetch on next yeild to process items in the queue together,
subsequent calls to fetch will
+ // have no effect because the queue will be clear
+ setTimeout( ve.bind( this.fetch, this ) );
};
/**
diff --git a/modules/ve/ui/dialogs/ve.ui.MWTransclusionDialog.js
b/modules/ve/ui/dialogs/ve.ui.MWTransclusionDialog.js
index 1583a78..2b3b813 100644
--- a/modules/ve/ui/dialogs/ve.ui.MWTransclusionDialog.js
+++ b/modules/ve/ui/dialogs/ve.ui.MWTransclusionDialog.js
@@ -80,13 +80,14 @@
// Properties
this.transclusion = new ve.dm.MWTransclusionModel();
+ // Events
+ this.transclusion.connect( this, { 'add': 'onAddPart', 'remove':
'onRemovePart' } );
+
// Initialization
if ( this.node instanceof ve.ce.MWTransclusionNode ) {
- this.transclusion.load( ve.copyObject(
this.node.getModel().getAttribute( 'mw' ) ) )
- .always( ve.bind( this.setupPages, this ) );
+ this.transclusion.load( ve.copyObject(
this.node.getModel().getAttribute( 'mw' ) ) );
} else {
- this.transclusion.addPlaceholder();
- this.setupPages();
+ this.transclusion.addPart( new
ve.dm.MWTemplatePlaceholderModel( this.transclusion ) );
}
};
@@ -249,11 +250,13 @@
switch ( type ) {
case 'content':
- part = this.transclusion.addContent( '',
this.getPartInsertionIndex() );
+ part = new ve.dm.MWTransclusionContentModel(
this.transclusion, '' );
+ this.transclusion.addPart( part,
this.getPartInsertionIndex() );
this.setPageByName( part.getId() );
break;
case 'template':
- part = this.transclusion.addPlaceholder(
this.getPartInsertionIndex() );
+ part = new ve.dm.MWTemplatePlaceholderModel(
this.transclusion );
+ this.transclusion.addPart( part,
this.getPartInsertionIndex() );
this.setPageByName( part.getId() );
break;
}
@@ -321,43 +324,6 @@
};
/**
- * Synchronize pages with transclusion.
- *
- * @method
- */
-ve.ui.MWTransclusionDialog.prototype.setupPages = function () {
- // Build pages from parts
- var i, iLen, j, jLen, part, param, names,
- parts = this.transclusion.getParts();
-
- // Populate pages
- for ( i = 0, iLen = parts.length; i < iLen; i++ ) {
- part = parts[i];
- if ( part instanceof ve.dm.MWTemplateModel ) {
- // Add template page
- this.addPage( part.getId(), this.getTemplatePage( part
) );
- // Listen for changes to parameters
- part.connect( this, { 'add': 'onAddParameter',
'remove': 'onRemoveParameter' } );
- // Add parameter pages
- names = part.getParameterNames();
- for ( j = 0, jLen = names.length; j < jLen; j++ ) {
- param = part.getParameter( names[j] );
- this.addPage( param.getId(),
this.getParameterPage( param ) );
- }
- } else if ( part instanceof ve.dm.MWTransclusionContentModel ) {
- // Add wikitext page
- this.addPage( part.getId(), this.getContentPage( part )
);
- } else if ( part instanceof ve.dm.MWTemplatePlaceholderModel ) {
- // Add template placeholder page
- this.addPage( part.getId(), this.getPlaceholderPage(
part ) );
- }
- }
-
- // Listen for changes to parts
- this.transclusion.connect( this, { 'add': 'onAddPart', 'remove':
'onRemovePart' } );
-};
-
-/**
* Get page for transclusion content.
*
* @method
@@ -418,7 +384,8 @@
description = spec.getDescription();
function addParameter() {
- var param = template.addParameter( addParameterInput.getValue()
);
+ var param = new ve.dm.MWTemplateParameterModel( template,
addParameterInput.getValue() );
+ template.addParameter( param );
addParameterInput.setValue();
this.setPageByName( param.getId() );
}
@@ -568,7 +535,8 @@
}
target = { 'href': new mw.Title( href ).getPrefixedText(),
'wt': value };
- part = this.transclusion.addTemplate( target, ve.indexOf(
placeholder, parts ) );
+ part = new ve.dm.MWTemplateModel( this.transclusion, target );
+ this.transclusion.addPart( part, ve.indexOf( placeholder, parts
) );
this.pending.push( { 'part': part, 'placeholder': placeholder }
);
addTemplateInput.pushPending();
addTemplateButton.setDisabled( true );
--
To view, visit https://gerrit.wikimedia.org/r/71161
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ibe579f033873446376d72d3bd1b9f92d9f361de5
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Trevor Parscal <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
