[MediaWiki-commits] [Gerrit] Follow-up 8250c8ad54: unbreak ApiResponseCache - change (mediawiki...VisualEditor)
Catrope has uploaded a new change for review. https://gerrit.wikimedia.org/r/227589 Change subject: Follow-up 8250c8ad54: unbreak ApiResponseCache .. Follow-up 8250c8ad54: unbreak ApiResponseCache .set() should not overwrite existing deferreds; instead, it should resolve the existing deferred if it's pending. This is necessary because .set() is used by processResult(). Without this, passing .get() a title that no information is known for results in a promise that is never resolved, because the associated deferred is overwritten as soon as the API response arrives. Still make .set() a no-op if data has already been set, by checking if the deferred is pending. For .resolve() this doesn't matter, but for modifying this.cacheValues it does. Bug: T107212 Change-Id: I70e8c5450f23062db214ccc5c585624d41de6509 --- M modules/ve-mw/init/ve.init.mw.ApiResponseCache.js 1 file changed, 8 insertions(+), 4 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/VisualEditor refs/changes/89/227589/1 diff --git a/modules/ve-mw/init/ve.init.mw.ApiResponseCache.js b/modules/ve-mw/init/ve.init.mw.ApiResponseCache.js index 9b9e8f7..5143aec 100644 --- a/modules/ve-mw/init/ve.init.mw.ApiResponseCache.js +++ b/modules/ve-mw/init/ve.init.mw.ApiResponseCache.js @@ -109,7 +109,7 @@ */ /** - * Add entries to the cache. + * Add entries to the cache. Does not overwrite already-set entries. * * @param {Object} entries Object keyed by page title, with the values being data objects * @fires add @@ -117,9 +117,13 @@ ve.init.mw.ApiResponseCache.prototype.set = function ( entries ) { var name; for ( name in entries ) { - this.deferreds[name] = $.Deferred(); - this.deferreds[name].resolve( entries[name] ); - this.cacheValues[name] = entries[name]; + if ( !Object.prototype.hasOwnProperty.call( this.deferreds, name ) ) { + this.deferreds[name] = $.Deferred(); + } + if ( this.deferreds[name].state() === 'pending' ) { + this.deferreds[name].resolve( entries[name] ); + this.cacheValues[name] = entries[name]; + } } this.emit( 'add', Object.keys( entries ) ); }; -- To view, visit https://gerrit.wikimedia.org/r/227589 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I70e8c5450f23062db214ccc5c585624d41de6509 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/VisualEditor Gerrit-Branch: master Gerrit-Owner: Catrope ___ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
[MediaWiki-commits] [Gerrit] Follow-up 8250c8ad54: unbreak ApiResponseCache - change (mediawiki...VisualEditor)
Alex Monk has uploaded a new change for review. https://gerrit.wikimedia.org/r/227590 Change subject: Follow-up 8250c8ad54: unbreak ApiResponseCache .. Follow-up 8250c8ad54: unbreak ApiResponseCache .set() should not overwrite existing deferreds; instead, it should resolve the existing deferred if it's pending. This is necessary because .set() is used by processResult(). Without this, passing .get() a title that no information is known for results in a promise that is never resolved, because the associated deferred is overwritten as soon as the API response arrives. Still make .set() a no-op if data has already been set, by checking if the deferred is pending. For .resolve() this doesn't matter, but for modifying this.cacheValues it does. Bug: T107212 Change-Id: I70e8c5450f23062db214ccc5c585624d41de6509 --- M modules/ve-mw/init/ve.init.mw.ApiResponseCache.js 1 file changed, 8 insertions(+), 4 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/VisualEditor refs/changes/90/227590/1 diff --git a/modules/ve-mw/init/ve.init.mw.ApiResponseCache.js b/modules/ve-mw/init/ve.init.mw.ApiResponseCache.js index 9b9e8f7..5143aec 100644 --- a/modules/ve-mw/init/ve.init.mw.ApiResponseCache.js +++ b/modules/ve-mw/init/ve.init.mw.ApiResponseCache.js @@ -109,7 +109,7 @@ */ /** - * Add entries to the cache. + * Add entries to the cache. Does not overwrite already-set entries. * * @param {Object} entries Object keyed by page title, with the values being data objects * @fires add @@ -117,9 +117,13 @@ ve.init.mw.ApiResponseCache.prototype.set = function ( entries ) { var name; for ( name in entries ) { - this.deferreds[name] = $.Deferred(); - this.deferreds[name].resolve( entries[name] ); - this.cacheValues[name] = entries[name]; + if ( !Object.prototype.hasOwnProperty.call( this.deferreds, name ) ) { + this.deferreds[name] = $.Deferred(); + } + if ( this.deferreds[name].state() === 'pending' ) { + this.deferreds[name].resolve( entries[name] ); + this.cacheValues[name] = entries[name]; + } } this.emit( 'add', Object.keys( entries ) ); }; -- To view, visit https://gerrit.wikimedia.org/r/227590 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I70e8c5450f23062db214ccc5c585624d41de6509 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/VisualEditor Gerrit-Branch: wmf/1.26wmf16 Gerrit-Owner: Alex Monk Gerrit-Reviewer: Catrope ___ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
[MediaWiki-commits] [Gerrit] Follow-up 8250c8ad54: unbreak ApiResponseCache - change (mediawiki...VisualEditor)
jenkins-bot has submitted this change and it was merged. Change subject: Follow-up 8250c8ad54: unbreak ApiResponseCache .. Follow-up 8250c8ad54: unbreak ApiResponseCache .set() should not overwrite existing deferreds; instead, it should resolve the existing deferred if it's pending. This is necessary because .set() is used by processResult(). Without this, passing .get() a title that no information is known for results in a promise that is never resolved, because the associated deferred is overwritten as soon as the API response arrives. Still make .set() a no-op if data has already been set, by checking if the deferred is pending. For .resolve() this doesn't matter, but for modifying this.cacheValues it does. Bug: T107212 Change-Id: I70e8c5450f23062db214ccc5c585624d41de6509 --- M modules/ve-mw/init/ve.init.mw.ApiResponseCache.js 1 file changed, 8 insertions(+), 4 deletions(-) Approvals: Alex Monk: Looks good to me, approved jenkins-bot: Verified diff --git a/modules/ve-mw/init/ve.init.mw.ApiResponseCache.js b/modules/ve-mw/init/ve.init.mw.ApiResponseCache.js index 9b9e8f7..5143aec 100644 --- a/modules/ve-mw/init/ve.init.mw.ApiResponseCache.js +++ b/modules/ve-mw/init/ve.init.mw.ApiResponseCache.js @@ -109,7 +109,7 @@ */ /** - * Add entries to the cache. + * Add entries to the cache. Does not overwrite already-set entries. * * @param {Object} entries Object keyed by page title, with the values being data objects * @fires add @@ -117,9 +117,13 @@ ve.init.mw.ApiResponseCache.prototype.set = function ( entries ) { var name; for ( name in entries ) { - this.deferreds[name] = $.Deferred(); - this.deferreds[name].resolve( entries[name] ); - this.cacheValues[name] = entries[name]; + if ( !Object.prototype.hasOwnProperty.call( this.deferreds, name ) ) { + this.deferreds[name] = $.Deferred(); + } + if ( this.deferreds[name].state() === 'pending' ) { + this.deferreds[name].resolve( entries[name] ); + this.cacheValues[name] = entries[name]; + } } this.emit( 'add', Object.keys( entries ) ); }; -- To view, visit https://gerrit.wikimedia.org/r/227589 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I70e8c5450f23062db214ccc5c585624d41de6509 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/VisualEditor Gerrit-Branch: master Gerrit-Owner: Catrope Gerrit-Reviewer: Alex Monk Gerrit-Reviewer: jenkins-bot <> ___ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
[MediaWiki-commits] [Gerrit] Follow-up 8250c8ad54: unbreak ApiResponseCache - change (mediawiki...VisualEditor)
jenkins-bot has submitted this change and it was merged. Change subject: Follow-up 8250c8ad54: unbreak ApiResponseCache .. Follow-up 8250c8ad54: unbreak ApiResponseCache .set() should not overwrite existing deferreds; instead, it should resolve the existing deferred if it's pending. This is necessary because .set() is used by processResult(). Without this, passing .get() a title that no information is known for results in a promise that is never resolved, because the associated deferred is overwritten as soon as the API response arrives. Still make .set() a no-op if data has already been set, by checking if the deferred is pending. For .resolve() this doesn't matter, but for modifying this.cacheValues it does. Bug: T107212 Change-Id: I70e8c5450f23062db214ccc5c585624d41de6509 --- M modules/ve-mw/init/ve.init.mw.ApiResponseCache.js 1 file changed, 8 insertions(+), 4 deletions(-) Approvals: Alex Monk: Looks good to me, approved jenkins-bot: Verified diff --git a/modules/ve-mw/init/ve.init.mw.ApiResponseCache.js b/modules/ve-mw/init/ve.init.mw.ApiResponseCache.js index 9b9e8f7..5143aec 100644 --- a/modules/ve-mw/init/ve.init.mw.ApiResponseCache.js +++ b/modules/ve-mw/init/ve.init.mw.ApiResponseCache.js @@ -109,7 +109,7 @@ */ /** - * Add entries to the cache. + * Add entries to the cache. Does not overwrite already-set entries. * * @param {Object} entries Object keyed by page title, with the values being data objects * @fires add @@ -117,9 +117,13 @@ ve.init.mw.ApiResponseCache.prototype.set = function ( entries ) { var name; for ( name in entries ) { - this.deferreds[name] = $.Deferred(); - this.deferreds[name].resolve( entries[name] ); - this.cacheValues[name] = entries[name]; + if ( !Object.prototype.hasOwnProperty.call( this.deferreds, name ) ) { + this.deferreds[name] = $.Deferred(); + } + if ( this.deferreds[name].state() === 'pending' ) { + this.deferreds[name].resolve( entries[name] ); + this.cacheValues[name] = entries[name]; + } } this.emit( 'add', Object.keys( entries ) ); }; -- To view, visit https://gerrit.wikimedia.org/r/227590 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I70e8c5450f23062db214ccc5c585624d41de6509 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/VisualEditor Gerrit-Branch: wmf/1.26wmf16 Gerrit-Owner: Alex Monk Gerrit-Reviewer: Alex Monk Gerrit-Reviewer: Catrope Gerrit-Reviewer: jenkins-bot <> ___ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits