[MediaWiki-commits] [Gerrit] Follow-up 8250c8ad54: unbreak ApiResponseCache - change (mediawiki...VisualEditor)

2015-07-28 Thread Catrope (Code Review)
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)

2015-07-28 Thread Alex Monk (Code Review)
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)

2015-07-28 Thread jenkins-bot (Code Review)
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)

2015-07-28 Thread jenkins-bot (Code Review)
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