Reviewers: Stanton, dev_shindig.apache.org,

Description:
Allow fatal errors to be reported to waiting callbacks.
Allow callbacks to indicate they aren't pushy, and don't want to trigger
an immediate refresh.
Allow containers to ask the refresh mechanism to call back again in x
seconds when there was an error.

Please review this at http://codereview.appspot.com/6304075/

Affected files:
  features/src/main/javascript/features/container.util/constant.js
  features/src/main/javascript/features/container/container.js
  features/src/main/javascript/features/container/service.js


Index: features/src/main/javascript/features/container.util/constant.js
===================================================================
--- features/src/main/javascript/features/container.util/constant.js (revision 1349992) +++ features/src/main/javascript/features/container.util/constant.js (working copy)
@@ -332,7 +332,14 @@
    *       var token, ttl, error = false;
    *       // Do work to set token and ttl values
    *       if (error) {
-   *         result();
+   *         var undef;
+   *         if (error.isFatal()) {
+ * // Run all callbacks and let them know there was a horrible error. + * // The container token is not valid, and probably won't be any time soon. + * result(undef, 30, 'There was an error!'); // Try again for a miracle in 30 seconds.
+   *         } else {
+   *           result(undef, 15); // Call me again in 15 seconds, please
+   *         }
    *       } else {
    *         result(token, ttl);
    *       }
Index: features/src/main/javascript/features/container/service.js
===================================================================
--- features/src/main/javascript/features/container/service.js (revision 1349992) +++ features/src/main/javascript/features/container/service.js (working copy)
@@ -410,9 +410,9 @@
       callbacks = [];


-  function runCallbacks(callbacks) {
+  function runCallbacks(callbacks, error) {
     while (callbacks.length) {
-      callbacks.shift().call(null); // Window context
+      callbacks.shift().call(null, error); // Window context
     }
   }

@@ -426,11 +426,11 @@
var fetch = fetch_once || this.config_[osapi.container.ContainerConfig.GET_CONTAINER_TOKEN];
     if (fetch) {
       var self = this;
- fetch(function(token, ttl) { // token and ttl may be undefined in the case of an error + fetch(function(token, ttl, error) { // token and ttl may be undefined in the case of an error
         fetching = false;

         // Use last known ttl if there was an error
-        containerTokenTTL = token ? (ttl * 1000 * 0.8) : containerTokenTTL;
+ containerTokenTTL = typeof(ttl) == 'number' ? (ttl * 1000 * 0.8) : containerTokenTTL;
         if (containerTokenTTL) {
           // Refresh again in 80% of the reported ttl
// Pass null in to closure because FF behaves un-expectedly when that param is not explicitly provided.
@@ -443,6 +443,8 @@
           lastRefresh =  osapi.container.util.getCurrentTimeMs();
           // And then run all the callbacks waiting for this.
           runCallbacks(callbacks);
+        } else if (error) {
+          runCallbacks(callbacks, error);
         }
       });
     } else {
@@ -455,8 +457,11 @@
   /**
    * @see osapi.container.Container.prototype.updateContainerSecurityToken
    */
- osapi.container.Service.prototype.updateContainerSecurityToken = function(callback, token, ttl) {
-    var now = osapi.container.util.getCurrentTimeMs(),
+ osapi.container.Service.prototype.updateContainerSecurityToken = function(callback, tokenORwait, ttl) {
+    var undef,
+        now = osapi.container.util.getCurrentTimeMs(),
+        token = typeof(tokenORwait) != 'boolean' && tokenORwait || undef,
+        wait = typeof(tokenORwait) == 'boolean' && tokenORwait,
         needsRefresh = containerTokenTTL &&
(fetching || token || !lastRefresh || now > lastRefresh + containerTokenTTL);
     if (needsRefresh) {
@@ -478,8 +483,14 @@
         refresh.call(this, function(result) {
           result(token, ttl);
         });
-      } else if (!fetching) {
- // There's no fetch going on right now. We need to start one because the token needs a refresh
+      } else if (!fetching && !wait) {
+ // There's no fetch going on right now. Unless wait is true, we need to start one right away
+        // because the token needs a refresh.
+
+ // If wait is true, the callback really just wants a valid token. It may be called with an + // error for informational purposes, but it's likely the callback will simply queue up + // immediately if there was an error. To avoid spamming the refresh method, we allow them to + // specify `wait` so that it can wait for success without forcing a fetch.
         refresh.call(this);
       }
     } else if (callback) {
Index: features/src/main/javascript/features/container/container.js
===================================================================
--- features/src/main/javascript/features/container/container.js (revision 1349992) +++ features/src/main/javascript/features/container/container.js (working copy)
@@ -519,8 +519,11 @@
* unless the token is specified in the optional parameter, in which case the token will be
  * updated with the provided value immediately.
  *
- * @param {function=} callback Function to run when container token is valid.
- * @param {String=} token The containers new security token.
+ * @param {function(error)=} callback Function to run when refresh completes or is cancelled.
+ *          error will be undefined if there is no error.
+ * @param {String=|boolean} tokenORwait
+ *          token The containers new security token.
+ *          wait If the callback should not trigger a token fetch.
* @param {number=} ttl The token's ttl in seconds. If token is specified and ttl is 0,
  *   token refresh will be disabled.
  * @see osapi.container.ContainerConfig.GET_CONTAINER_TOKEN (constants.js)


Reply via email to