Rewrite the session code, again. Now use the username as the profile key name.

lib/models.py

@@ -47,27 +47,10 @@ class AccessDenied(Exception):
 class Profile(db.Model):
   google_user = db.UserProperty()
 
-  @classmethod
-  def FromGoogleUser(cls, google_user):
-    if not google_user:
-      profile = Profile()
-      profile.put()
-      return profile
-
-    profiles = Profile.all().filter('google_user =', google_user).fetch(1)
-    if profiles:
-      return profiles[0]
-    else:
-      # TODO(flamingcow): Fetch-then-store uniqueness is a race.
-      profile = Profile(google_user=google_user)
-      profile.put()
-      return profile
-
   def MergeFrom(self, source_profile):
     # This is non-transactional and racy (new messages can be introduced by the
-    # old client after we start). This is hard to solve because a) we're not in
+    # old client after we start). This is hard to solve because we're not in
-    # a single hierarchy and b) we don't revoke the old client ID, so it can
+    # a single hierarchy.
-    # still be used.
     for message in Message.all().filter('sender =', source_profile):
       message.sender = self;
       message.put()
@@ -84,11 +67,6 @@ class Client(db.Model):
     client.put()
     return client
 
-  @classmethod
-  def FromGoogleUser(cls, client_id, google_user):
-    profile = Profile.FromGoogleUser(google_user)
-    return cls.FromProfile(client_id, profile)
-
 
 class Instance(db.Model):
   # key_name=instance_id

lib/session.py

@@ -14,48 +14,50 @@
 
 import functools
 
-from google.appengine.api import users
+from google.appengine.ext import db
 
-from cosmopolite.lib import auth
 from cosmopolite.lib import models
 
 
-def _CheckClientAndGoogleUser(client, google_user):
+@db.transactional(xg=True)
-  if not google_user:
+def CreateClientAndProfile(client_id, google_user):
-    # Nothing to check. If there's a user on the profile, it can stay there.
+  if google_user:
-    return
+    # We're going to need a profile for this user regardless
-
+    profile = models.Profile.get_by_key_name(str(google_user))
-  client_profile_google_user = client.profile.google_user
+    if not profile:
-  if client_profile_google_user:
+      profile = models.Profile(
-    if client_profile_google_user == google_user:
+          key_name=str(google_user), google_user=google_user)
-      return
+      profile.put()
   else:
-      client.profile = models.Profile.FromGoogleUser(google_user)
+    profile = None
-      client.put()
-      return
 
-  # User just signed in. Their anonymous profile gets permanently
+  client = models.Client.get_by_key_name(client_id)
-  # associated with this Google account.
-  profiles = (models.Profile.all()
-              .filter('google_user =', google_user)
-              .fetch(1))
-  if profiles:
-    # We can't convert the anonymous profile because there's already
-    # a profile for this Google user. Create a new client_id pointing to that
-    # profile.
-    # TODO(flamingcow): Fetch-then-store uniqueness is a race.
-    google_profile = profiles[0]
-    google_profile.MergeFrom(
-        models.Client.profile.get_value_for_datastore(client))
-    client.profile = google_profile
-    client.put()
-    return
 
-  # First time signin.
+  if not client:
-  client_profile = client.profile
+    # First time seeing this client; create it
-  client_profile.google_user = google_user
+    if not profile:
-  client_profile.put()
+      # Create an anonymous profile
-  return
+      profile = models.Profile()
+      profile.put()
+    client = models.Client(key_name=client_id, profile=profile)
+    client.put()
+    return (client, None)
+
+  if not profile:
+    # No Google user, whatever profile we had is fine
+    return (client, None)
+
+  if (profile.key() !=
+      models.Client.profile.get_value_for_datastore(client)):
+    # Google user doesn't match. Create a new profile, ask our caller to
+    # merge profiles outside the transaction
+    old_profile = client.profile
+    client.profile = profile
+    client.put()
+    return (client, old_profile)
+
+  # Everything already exists and matches
+  return (client, None)
 
 
 def session_required(handler):
@@ -66,18 +68,12 @@ def session_required(handler):
 
   Make sure to wrap this in google_user_xsrf_protection.
   """
-
   @functools.wraps(handler)
   def FindOrCreateSession(self):
-    client_id = self.request_json['client_id']
+    self.client, old_profile = CreateClientAndProfile(
-
+        self.request_json['client_id'], self.verified_google_user)
-    self.client = models.Client.get_by_key_name(client_id)
+    if old_profile:
-
+      self.client.profile.MergeFrom(old_profile)
-    if self.client:
-      _CheckClientAndGoogleUser(self.client, self.verified_google_user)
-    else:
-      self.client = models.Client.FromGoogleUser(
-          client_id, self.verified_google_user)
 
     return handler(self)