From fd5569c5dcef75ef639f8e667a393f76d367d004 Mon Sep 17 00:00:00 2001 From: Ian Gulliver Date: Sat, 31 May 2014 23:25:15 -0700 Subject: [PATCH] Generate the client_id on the client, so we don't need a round trip to get it before we can send arbitrary RPCs. This changes the Profile/Client relationship; we never needed to be transactional with them, so split them up entirely so we can reparent Clients. --- api.py | 21 ++++++++++++++------- lib/models.py | 15 ++++++++------- lib/session.py | 43 +++++++++++++++++++++---------------------- static/cosmopolite.js | 17 ++++++----------- 4 files changed, 49 insertions(+), 47 deletions(-) diff --git a/api.py b/api.py index 0875602..4477f7e 100644 --- a/api.py +++ b/api.py @@ -36,14 +36,14 @@ def CreateChannel(google_user, client, instance_id, args): events = [] if google_user: events.append({ - 'event_type': 'login', - 'profile': str(client.parent_key()), - 'google_user': google_user.email(), + 'event_type': 'login', + 'profile': str(models.Client.profile.get_value_for_datastore(client)), + 'google_user': google_user.email(), }) else: events.append({ 'event_type': 'logout', - 'profile': str(client.parent_key()), + 'profile': str(models.Client.profile.get_value_for_datastore(client)), }) return { @@ -66,7 +66,10 @@ def Pin(google_user, client, instance_id, args): try: models.Subject.FindOrCreate(subject).Pin( - message, client.parent_key(), sender_message_id, instance) + message, + models.Client.profile.get_value_for_datastore(client), + sender_message_id, + instance) except models.DuplicateMessage: logging.warning('Duplicate pin: %s', sender_message_id) return { @@ -90,7 +93,9 @@ def SendMessage(google_user, client, instance_id, args): try: models.Subject.FindOrCreate(subject).SendMessage( - message, client.parent_key(), sender_message_id) + message, + models.Client.profile.get_value_for_datastore(client), + sender_message_id) except models.DuplicateMessage: logging.warning('Duplicate message: %s', sender_message_id) return { @@ -139,7 +144,9 @@ def Unpin(google_user, client, instance_id, args): try: models.Subject.FindOrCreate(subject).Unpin( - client.parent_key(), sender_message_id, instance.key()) + models.Client.profile.get_value_for_datastore(client), + sender_message_id, + instance.key()) except models.AccessDenied: logging.warning('Pin access denied') return { diff --git a/lib/models.py b/lib/models.py index 647e218..eabc249 100644 --- a/lib/models.py +++ b/lib/models.py @@ -25,7 +25,8 @@ import utils # Profile -# ↳ Client +# +# Client (⤴︎ Profile) # # Instance # @@ -73,20 +74,20 @@ class Profile(db.Model): class Client(db.Model): - # parent=Profile + profile = db.ReferenceProperty(reference_class=Profile) first_seen = db.DateTimeProperty(required=True, auto_now_add=True) @classmethod - def FromProfile(cls, profile): - client = cls(parent=profile) + def FromProfile(cls, client_id, profile): + client = cls(key_name=client_id, profile=profile) client.put() return client @classmethod - def FromGoogleUser(cls, google_user): + def FromGoogleUser(cls, client_id, google_user): profile = Profile.FromGoogleUser(google_user) - return cls.FromProfile(profile) + return cls.FromProfile(client_id, profile) class Instance(db.Model): @@ -308,7 +309,7 @@ class Subscription(db.Model): readable_only_by = ( Subject.readable_only_by.get_value_for_datastore(subject)) if (readable_only_by and - readable_only_by != client.parent_key()): + readable_only_by != Client.profile.get_value_for_datastore(client)): raise AccessDenied subscriptions = ( diff --git a/lib/session.py b/lib/session.py index 0d5449b..8f62560 100644 --- a/lib/session.py +++ b/lib/session.py @@ -23,15 +23,16 @@ from cosmopolite.lib import models def _CheckClientAndGoogleUser(client, google_user): if not google_user: # Nothing to check. If there's a user on the profile, it can stay there. - return client + return - client_profile_google_user = client.parent().google_user + client_profile_google_user = client.profile.google_user if client_profile_google_user: if client_profile_google_user == google_user: - return client + return else: - # Shared computer? Google account wins. - return models.Client.FromGoogleUser(google_user) + client.profile = models.Profile.FromGoogleUser(google_user) + client.put() + return # User just signed in. Their anonymous profile gets permanently # associated with this Google account. @@ -44,14 +45,17 @@ def _CheckClientAndGoogleUser(client, google_user): # profile. # TODO(flamingcow): Fetch-then-store uniqueness is a race. google_profile = profiles[0] - google_profile.MergeFrom(client.parent_key()) - return models.Client.FromProfile(google_profile) + google_profile.MergeFrom( + models.Client.profile.get_value_for_datastore(client)) + client.profile = google_profile + client.put() + return # First time signin. - client_profile = client.parent() + client_profile = client.profile client_profile.google_user = google_user client_profile.put() - return client + return def session_required(handler): @@ -65,21 +69,16 @@ def session_required(handler): @functools.wraps(handler) def FindOrCreateSession(self): - client_key = auth.ParseKey(self.request_json.get('client_id', None)) + client_id = self.request_json['client_id'] - # The hunt for a Profile begins. - if client_key: - self.client = _CheckClientAndGoogleUser( - models.Client.get(client_key), - self.verified_google_user) + self.client = models.Client.get_by_key_name(client_id) + + if self.client: + _CheckClientAndGoogleUser(self.client, self.verified_google_user) else: - self.client = models.Client.FromGoogleUser(self.verified_google_user) + self.client = models.Client.FromGoogleUser( + client_id, self.verified_google_user) - ret = handler(self) - if client_key != self.client.key(): - # Tell the client that this changed - ret['client_id'] = auth.Sign(self.client.key()) - - return ret + return handler(self) return FindOrCreateSession diff --git a/static/cosmopolite.js b/static/cosmopolite.js index ab7fa30..06eaf79 100644 --- a/static/cosmopolite.js +++ b/static/cosmopolite.js @@ -96,6 +96,10 @@ var Cosmopolite = function( */ this.profilePromises_ = []; + if (!localStorage[this.namespace_ + ':client_id']) { + localStorage[this.namespace_ + ':client_id'] = this.uuid_(); + } + /** * @type {string} * @private @@ -639,7 +643,7 @@ Cosmopolite.prototype.onReceiveMessage_ = function(data) { } break; case 'logout_complete': - localStorage.removeItem(this.namespace_ + ':client_id'); + localStorage[this.namespace_ + ':client_id'] = this.uuid_(); localStorage.removeItem(this.namespace_ + ':google_user_id'); if (this.socket_) { this.socket_.close(); @@ -751,6 +755,7 @@ Cosmopolite.prototype.sendRPCs_ = function(commands, opt_delay) { } var request = { 'instance_id': this.instanceID_, + 'client_id': localStorage[this.namespace_ + ':client_id'], 'commands': [] }; commands.forEach(function(command) { @@ -762,9 +767,6 @@ Cosmopolite.prototype.sendRPCs_ = function(commands, opt_delay) { } request.commands.push(request_command); }); - if (this.namespace_ + ':client_id' in localStorage) { - request['client_id'] = localStorage[this.namespace_ + ':client_id']; - } if (this.namespace_ + ':google_user_id' in localStorage) { request['google_user_id'] = localStorage[this.namespace_ + ':google_user_id']; @@ -797,9 +799,6 @@ Cosmopolite.prototype.sendRPCs_ = function(commands, opt_delay) { localStorage[this.namespace_ + ':google_user_id'] = data['google_user_id']; } - if ('client_id' in data) { - localStorage[this.namespace_ + ':client_id'] = data['client_id']; - } if (data['status'] == 'retry') { // Discard delay @@ -852,10 +851,6 @@ Cosmopolite.prototype.sendRPCs_ = function(commands, opt_delay) { * @private */ Cosmopolite.prototype.maySendRPC_ = function() { - if (!(this.namespace_ + ':client_id' in localStorage)) { - return false; - } - if (this.channelState_ != Cosmopolite.ChannelState_.OPEN) { return false; }