Change interface for determining if the user doc is loaded to a new reactive

function Meteor.userLoaded(), which is true if you are logged in and the user doc is loaded, and a currentUserLoaded Handlebars helper to match. If logged in and the user doc is not yet loaded, Meteor.user() now returns an object which only contains _id. The current user subscription is now named meteor.currentUser rather than being an unnamed sub. (loginServiceConfiguration is renamed meteor.loginServiceConfiguration to match.) This subscription is sub'd from when you log in and unsub'd from when you log out (or if you log in with different credentials). I was very careful to make sure that in the case of "sub #1, unsub #1, sub #2, sub #1 is ready" we do not declare the user to be ready. I could have instead modified livedata_connection to not call ready callbacks for unsub'd subscriptions (add a "delete self.sub_ready_callbacks[obj._id]" to the self.subs removed function) but this seemed less invasive. The password and email tests use this to take a more rigorous approach to waiting for the data to load, and they change the localStorage keys so that multiple tabs running tests don't interact via localStorage.
2026-05-02 03:01:46 -04:00 · 2012-10-07 00:10:36 -07:00
parent e733b17d6a
commit d4e4a6300a
7 changed files with 192 additions and 122 deletions
--- a/packages/accounts-base/accounts_client.js
+++ b/packages/accounts-base/accounts_client.js
@@ -4,23 +4,64 @@
    return Meteor.default_connection.userId();
  };

+  var userLoadedListeners = new Meteor.deps._ContextSet;
+  var currentUserSubscriptionData;
+
+  Meteor.userLoaded = function () {
+    userLoadedListeners.addCurrentContext();
+    return currentUserSubscriptionData && currentUserSubscriptionData.loaded;
+  };
+
  Meteor.user = function () {
    var userId = Meteor.userId();
-    if (userId) {
-      var result = Meteor.users.findOne(userId);
-      if (result) {
-        return result;
-      } else {
-        // If the login method completes but new subcriptions haven't
-        // yet been sent down to the client, this is the best we can
-        // do
-        return {_id: userId, loading: true};
-      }
-    } else {
+    if (!userId)
      return null;
+    if (currentUserSubscriptionData && currentUserSubscriptionData.loaded)
+      return Meteor.users.findOne(userId);
+    // Not yet loaded: return a minimal object.
+    return {_id: userId};
+  };
+
+  Accounts._makeClientLoggedOut = function() {
+    Accounts._unstoreLoginToken();
+    Meteor.default_connection.setUserId(null);
+    Meteor.default_connection.onReconnect = null;
+    userLoadedListeners.invalidateAll();
+    if (currentUserSubscriptionData) {
+      currentUserSubscriptionData.handle.stop();
+      currentUserSubscriptionData = null;
    }
  };

+  Accounts._makeClientLoggedIn = function(userId, token) {
+    Accounts._storeLoginToken(userId, token);
+    Meteor.default_connection.setUserId(userId);
+    Meteor.default_connection.onReconnect = function() {
+      Meteor.apply('login', [{resume: token}], {wait: true}, function(error, result) {
+        if (error) {
+          Accounts._makeClientLoggedOut();
+          throw error;
+        } else {
+          // nothing to do
+        }
+      });
+    };
+    userLoadedListeners.invalidateAll();
+    if (currentUserSubscriptionData) {
+      currentUserSubscriptionData.handle.stop();
+    }
+    var data = currentUserSubscriptionData = {loaded: false};
+    data.handle = Meteor.subscribe(
+      "meteor.currentUser", function () {
+        // Important! We use "data" here, not "currentUserSubscriptionData", so
+        // that if we log out and in again before this subscription is ready, we
+        // don't make currentUserSubscriptionData look ready just because this
+        // older iteration of subscribing is ready.
+        data.loaded = true;
+        userLoadedListeners.invalidateAll();
+      });
+  };
+
  Meteor.logout = function (callback) {
    Meteor.apply('logout', [], {wait: true}, function(error, result) {
      if (error) {
@@ -32,19 +73,22 @@
    });
  };

-  // If we're using Handlebars, register the {{currentUser}} global
-  // helper
-  if (window.Handlebars) {
+  // If we're using Handlebars, register the {{currentUser}} and
+  // {{currentUserLoaded}} global helpers.
+  if (typeof Handlebars !== 'undefined') {
    Handlebars.registerHelper('currentUser', function () {
      return Meteor.user();
    });
+    Handlebars.registerHelper('currentUserLoaded', function () {
+      return Meteor.userLoaded();
+    });
  }

  // XXX this can be simplified if we merge in
  // https://github.com/meteor/meteor/pull/273
  var loginServicesConfigured = false;
  var loginServicesConfiguredListeners = new Meteor.deps._ContextSet;
-  Meteor.subscribe("loginServiceConfiguration", function () {
+  Meteor.subscribe("meteor.loginServiceConfiguration", function () {
    loginServicesConfigured = true;
    loginServicesConfiguredListeners.invalidateAll();
  });
--- a/packages/accounts-base/accounts_server.js
+++ b/packages/accounts-base/accounts_server.js
@@ -233,13 +233,22 @@
  /// PUBLISHING DATA
  ///

-  // Always publish the current user's record to the client.
-  Meteor.publish(null, function() {
+  // Publish the current user's record to the client.
+  // XXX This should just be a universal subscription, but we want to know when
+  //     we've gotten the data after a 'login' method, which currently requires
+  //     us to unsub, sub, and wait for onComplete. This is wasteful because
+  //     we're actually guaranteed to have the data by the time that 'login'
+  //     returns. But we don't expose a callback to Meteor.apply which lets us
+  //     know when the data has been processed (ie, quiescence, or at least
+  //     partial quiescence).
+  Meteor.publish("meteor.currentUser", function() {
    if (this.userId)
      return Meteor.users.find({_id: this.userId},
                               {fields: {profile: 1, username: 1, emails: 1}});
-    else
+    else {
+      this.complete();
      return null;
+    }
  }, {is_auto: true});

  // If autopublish is on, also publish everyone else's user record.
@@ -252,7 +261,7 @@
  });

  // Publish all login service configuration fields other than secret.
-  Meteor.publish("loginServiceConfiguration", function () {
+  Meteor.publish("meteor.loginServiceConfiguration", function () {
    return Accounts.configuration.find({}, {fields: {secret: 0}});
  }, {is_auto: true}); // not techincally autopublish, but stops the warning.

--- a/packages/accounts-base/localstorage_token.js
+++ b/packages/accounts-base/localstorage_token.js
@@ -3,6 +3,14 @@
  var loginTokenKey = "Meteor.loginToken";
  var userIdKey = "Meteor.userId";

+  // Call this from the top level of the test file for any test that does
+  // logging in and out, to protect multiple tabs running the same tests
+  // simultaneously from interfering with each others' localStorage.
+  Accounts._isolateLoginTokenForTest = function () {
+    loginTokenKey = loginTokenKey + Meteor.uuid();
+    userIdKey = userIdKey + Meteor.uuid();
+  };
+
  Accounts._storeLoginToken = function(userId, token) {
    localStorage.setItem(userIdKey, userId);
    localStorage.setItem(loginTokenKey, token);
@@ -28,27 +36,6 @@
  Accounts._storedUserId = function() {
    return localStorage.getItem(userIdKey);
  };
-
-  Accounts._makeClientLoggedOut = function() {
-    Accounts._unstoreLoginToken();
-    Meteor.default_connection.setUserId(null);
-    Meteor.default_connection.onReconnect = null;
-  };
-
-  Accounts._makeClientLoggedIn = function(userId, token) {
-    Accounts._storeLoginToken(userId, token);
-    Meteor.default_connection.setUserId(userId);
-    Meteor.default_connection.onReconnect = function() {
-      Meteor.apply('login', [{resume: token}], {wait: true}, function(error, result) {
-        if (error) {
-          Accounts._makeClientLoggedOut();
-          throw error;
-        } else {
-          // nothing to do
-        }
-      });
-    };
-  };
 })();

 // Login with a Meteor access token
--- a/packages/accounts-password/email_tests.js
+++ b/packages/accounts-password/email_tests.js
@@ -11,6 +11,8 @@
  var validateEmailToken;
  var enrollAccountToken;

+  Accounts._isolateLoginTokenForTest();
+
  testAsyncMulti("accounts emails - reset password flow", [
    function (test, expect) {
      email1 = Meteor.uuid() + "-intercept@example.com";
@@ -65,6 +67,7 @@

  var getValidateEmailToken = function (email, test, expect) {
    Meteor.call("getInterceptedEmails", email, expect(function (error, result) {
+      test.isFalse(error);
      test.notEqual(result, undefined);
      test.equal(result.length, 1);
      var content = result[0];
@@ -77,15 +80,28 @@
    }));
  };

+  var waitUntilLoggedIn = function (test, expect) {
+    var unblockNextFunction = expect();
+    var quiesceCallback = function () {
+      Meteor._autorun(function (handle) {
+        if (!Meteor.userLoaded()) return;
+        handle.stop();
+        unblockNextFunction();
+      });
+    };
+    return expect(function (error) {
+      test.equal(error, undefined);
+      Meteor.default_connection.onQuiesce(quiesceCallback);
+    });
+  };
+
  testAsyncMulti("accounts emails - validate email flow", [
    function (test, expect) {
      email2 = Meteor.uuid() + "-intercept@example.com";
      email3 = Meteor.uuid() + "-intercept@example.com";
      Accounts.createUser(
        {email: email2, password: 'foobar'},
-        expect(function (error) {
-          test.equal(error, undefined);
-        }));
+        waitUntilLoggedIn(test, expect));
    },
    function (test, expect) {
      test.equal(Meteor.user().emails.length, 1);
@@ -96,16 +112,22 @@
      getValidateEmailToken(email2, test, expect);
    },
    function (test, expect) {
-      Accounts.validateEmail(validateEmailToken, expect(function(error) {
-        test.isFalse(error);
-      }));
-      // ARGH! ON QUIESCE!!
-      Meteor.default_connection.onQuiesce(expect(function () {
-        test.equal(Meteor.user().emails.length, 1);
-        test.equal(Meteor.user().emails[0].address, email2);
-        test.isTrue(Meteor.user().emails[0].validated);
+      // Log out, to test that validateEmail logs us back in. (And if we don't
+      // do that, waitUntilLoggedIn won't be able to prevent race conditions.)
+      Meteor.logout(expect(function (error) {
+        test.equal(error, undefined);
+        test.equal(Meteor.user(), null);
      }));
    },
+    function (test, expect) {
+      Accounts.validateEmail(validateEmailToken,
+                             waitUntilLoggedIn(test, expect));
+    },
+    function (test, expect) {
+      test.equal(Meteor.user().emails.length, 1);
+      test.equal(Meteor.user().emails[0].address, email2);
+      test.isTrue(Meteor.user().emails[0].validated);
+    },
    function (test, expect) {
      Meteor.call(
        "addEmailForTestAndValidate", email3,
@@ -124,15 +146,20 @@
      getValidateEmailToken(email3, test, expect);
    },
    function (test, expect) {
-      Accounts.validateEmail(validateEmailToken, expect(function(error) {
-        test.isFalse(error);
+      // Log out, to test that validateEmail logs us back in. (And if we don't
+      // do that, waitUntilLoggedIn won't be able to prevent race conditions.)
+      Meteor.logout(expect(function (error) {
+        test.equal(error, undefined);
+        test.equal(Meteor.user(), null);
      }));
    },
    function (test, expect) {
-      Meteor.default_connection.onQuiesce(expect(function () {
-        test.equal(Meteor.user().emails[1].address, email3);
-        test.isTrue(Meteor.user().emails[1].validated);
-      }));
+      Accounts.validateEmail(validateEmailToken,
+                             waitUntilLoggedIn(test, expect));
+    },
+    function (test, expect) {
+      test.equal(Meteor.user().emails[1].address, email3);
+      test.isTrue(Meteor.user().emails[1].validated);
    },
    function (test, expect) {
      Meteor.logout(expect(function (error) {
@@ -172,16 +199,13 @@
      getEnrollAccountToken(email4, test, expect);
    },
    function (test, expect) {
-      Accounts.resetPassword(enrollAccountToken, 'password', expect(function(error) {
-        test.isFalse(error);
-      }));
+      Accounts.resetPassword(enrollAccountToken, 'password',
+                             waitUntilLoggedIn(test, expect));
    },
    function (test, expect) {
-      Meteor.default_connection.onQuiesce(expect(function () {
-        test.equal(Meteor.user().emails.length, 1);
-        test.equal(Meteor.user().emails[0].address, email4);
-        test.isTrue(Meteor.user().emails[0].validated);
-      }));
+      test.equal(Meteor.user().emails.length, 1);
+      test.equal(Meteor.user().emails[0].address, email4);
+      test.isTrue(Meteor.user().emails[0].validated);
    },
    function (test, expect) {
      Meteor.logout(expect(function (error) {
@@ -190,9 +214,13 @@
      }));
    },
    function (test, expect) {
-      Meteor.loginWithPassword({email: email4}, 'password', expect(function(error) {
-        test.isFalse(error);
-      }));
+      Meteor.loginWithPassword({email: email4}, 'password',
+                               waitUntilLoggedIn(test ,expect));
+    },
+    function (test, expect) {
+      test.equal(Meteor.user().emails.length, 1);
+      test.equal(Meteor.user().emails[0].address, email4);
+      test.isTrue(Meteor.user().emails[0].validated);
    },
    function (test, expect) {
      Meteor.logout(expect(function (error) {
--- a/packages/accounts-password/passwords_tests.js
+++ b/packages/accounts-password/passwords_tests.js
@@ -3,6 +3,7 @@ if (Meteor.isClient) (function () {
  // XXX note, only one test can do login/logout things at once! for
  // now, that is this test.

+  Accounts._isolateLoginTokenForTest();

  var logoutStep = function (test, expect) {
    Meteor.logout(expect(function (error) {
@@ -11,6 +12,25 @@ if (Meteor.isClient) (function () {
    }));
  };

+  var verifyUsername = function (someUsername, test, expect) {
+    var callWhenLoaded = expect(function() {
+      test.equal(Meteor.user().username, someUsername);
+    });
+    return function () {
+      Meteor._autorun(function(handle) {
+        if (!Meteor.userLoaded()) return;
+        handle.stop();
+        callWhenLoaded();
+      });
+    };
+  };
+  var loggedInAs = function (someUsername, test, expect) {
+    var quiesceCallback = verifyUsername(someUsername, test, expect);
+    return expect(function (error) {
+      test.equal(error, undefined);
+      Meteor.default_connection.onQuiesce(quiesceCallback);
+    });
+  };

  // declare variable outside the testAsyncMulti, so we can refer to
  // them from multiple tests, but initialize them to new values inside
@@ -33,46 +53,29 @@ if (Meteor.isClient) (function () {
    },

    function (test, expect) {
-      // XXX argh quiescence + tests === wtf. and i have no idea why
-      // this was necessary here and not in other places. probably
-      // because it's dependant on how long method call chains are in
-      // other tests
-      var quiesceCallback = expect(function () {
-        test.equal(Meteor.user().username, username);
-      });
-      Accounts.createUser({username: username, email: email, password: password},
-                        expect(function (error) {
-                          test.equal(error, undefined);
-                          Meteor.default_connection.onQuiesce(quiesceCallback);
-                        }));
+      Accounts.createUser(
+        {username: username, email: email, password: password},
+        loggedInAs(username, test, expect));
    },
    logoutStep,
    function (test, expect) {
-      Meteor.loginWithPassword(username, password, expect(function (error) {
-        test.equal(error, undefined);
-        test.equal(Meteor.user().username, username);
-      }));
+      Meteor.loginWithPassword(username, password,
+                               loggedInAs(username, test, expect));
    },
    logoutStep,
    function (test, expect) {
-      Meteor.loginWithPassword({username: username}, password, expect(function (error) {
-        test.equal(error, undefined);
-        test.equal(Meteor.user().username, username);
-      }));
+      Meteor.loginWithPassword({username: username}, password,
+                               loggedInAs(username, test, expect));
    },
    logoutStep,
    function (test, expect) {
-      Meteor.loginWithPassword(email, password, expect(function (error) {
-        test.equal(error, undefined);
-        test.equal(Meteor.user().username, username);
-      }));
+      Meteor.loginWithPassword(email, password,
+                               loggedInAs(username, test, expect));
    },
    logoutStep,
    function (test, expect) {
-      Meteor.loginWithPassword({email: email}, password, expect(function (error) {
-        test.equal(error, undefined);
-        test.equal(Meteor.user().username, username);
-      }));
+      Meteor.loginWithPassword({email: email}, password,
+                               loggedInAs(username, test, expect));
    },
    logoutStep,
    // plain text password. no API for this, have to send a raw message.
@@ -87,6 +90,7 @@ if (Meteor.isClient) (function () {
      }));
    },
    function (test, expect) {
+      var quiesceCallback = verifyUsername(username, test, expect);
      Meteor.call(
        // right password
        'login', {user: {email: email}, password: password},
@@ -96,22 +100,21 @@ if (Meteor.isClient) (function () {
          test.isTrue(result.token);
          // emulate the real login behavior, so as not to confuse test.
          Accounts._makeClientLoggedIn(result.id, result.token);
-          test.equal(Meteor.user().username, username);
+          Meteor.default_connection.onQuiesce(quiesceCallback);
      }));
    },
-    // change password with bad old password.
+    // change password with bad old password. we stay logged in.
    function (test, expect) {
+      var quiesceCallback = verifyUsername(username, test, expect);
      Accounts.changePassword(password2, password2, expect(function (error) {
        test.isTrue(error);
-        test.equal(Meteor.user().username, username);
+        Meteor.default_connection.onQuiesce(quiesceCallback);
      }));
    },
    // change password with good old password.
    function (test, expect) {
-      Accounts.changePassword(password, password2, expect(function (error) {
-        test.equal(error, undefined);
-        test.equal(Meteor.user().username, username);
-      }));
+      Accounts.changePassword(password, password2,
+                              loggedInAs(username, test, expect));
    },
    logoutStep,
    // old password, failed login
@@ -123,14 +126,13 @@ if (Meteor.isClient) (function () {
    },
    // new password, success
    function (test, expect) {
-      Meteor.loginWithPassword(email, password2, expect(function (error) {
-        test.equal(error, undefined);
-        test.equal(Meteor.user().username, username);
-      }));
+      Meteor.loginWithPassword(email, password2,
+                               loggedInAs(username, test, expect));
    },
    logoutStep,
    // create user with raw password
    function (test, expect) {
+      var quiesceCallback = verifyUsername(username2, test, expect);
      Meteor.call('createUser', {username: username2, password: password2},
                  expect(function (error, result) {
                    test.equal(error, undefined);
@@ -138,16 +140,13 @@ if (Meteor.isClient) (function () {
                    test.isTrue(result.token);
                    // emulate the real login behavior, so as not to confuse test.
                    Accounts._makeClientLoggedIn(result.id, result.token);
-                    test.equal(Meteor.user().username, username2);
+                    Meteor.default_connection.onQuiesce(quiesceCallback);
                  }));
    },
    logoutStep,
    function(test, expect) {
      Meteor.loginWithPassword({username: username2}, password2,
-                               expect(function (error) {
-                                 test.equal(error, undefined);
-                                 test.equal(Meteor.user().username, username2);
-                               }));
+                               loggedInAs(username2, test, expect));
    },
    logoutStep,
    // test Accounts.validateNewUser
@@ -173,10 +172,13 @@ if (Meteor.isClient) (function () {
    },
    // test Accounts.onCreateUser
    function(test, expect) {
-      Accounts.createUser({username: username3, password: password3},
-                        {testOnCreateUserHook: true}, expect(function () {
-        test.equal(Meteor.user().profile.touchedByOnCreateUser, true);
-      }));
+      Accounts.createUser(
+        {username: username3, password: password3},
+        {testOnCreateUserHook: true},
+        loggedInAs(username3, test, expect));
+    },
+    function(test, expect) {
+      test.equal(Meteor.user().profile.touchedByOnCreateUser, true);
    },

    // test Meteor.user(). This test properly belongs in
--- a/packages/accounts-ui-unstyled/login_buttons.html
+++ b/packages/accounts-ui-unstyled/login_buttons.html
@@ -13,10 +13,10 @@
    {{> loginButtonsLoggedInDropdown}}
  {{else}}
    <div class="login-header">
-      {{#if currentUser.loading}} {{! XXX this will change }}
-        <div class="loading"></div>
-      {{else}}
+      {{#if currentUserLoaded}}
        {{displayName}}
+      {{else}}
+        <div class="loading"></div>
      {{/if}}
    </div>
    <div class="login-button" id="login-buttons-logout">Logout</div>
--- a/packages/accounts-ui-unstyled/login_buttons_dropdown.html
+++ b/packages/accounts-ui-unstyled/login_buttons_dropdown.html
@@ -3,9 +3,7 @@
 <!--           -->
 <template name="loginButtonsLoggedInDropdown">
  <div class="login-link-and-dropdown-list">
-    {{#if currentUser.loading}}
-      <div class="loading"></div>
-    {{else}}
+    {{#if currentUserLoaded}}
      <a class="login-link-text" id="login-name-link">
        {{displayName}} ▾
      </a>
@@ -22,6 +20,8 @@
          {{/if}}
        </div>
      {{/if}}
+    {{else}}
+      <div class="loading"></div>
    {{/if}}
  </div>
 </template>