launchpad-reviewers team mailing list archive

[Ioana Lasc <ioana.lasc@xxxxxxxxxxxxx>]

Ioana Lasc has proposed merging ~ilasc/launchpad:stormify-person-location into launchpad:master.

Commit message:
Stormify PersonLocation

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ilasc/launchpad/+git/launchpad/+merge/390093

Move PersonLocation from SQLObject to Storm.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~ilasc/launchpad:stormify-person-location into launchpad:master.

diff --git a/lib/lp/registry/model/person.py b/lib/lp/registry/model/person.py
index 5917c6e..e5b66aa 100644
--- a/lib/lp/registry/model/person.py
+++ b/lib/lp/registry/model/person.py
@@ -766,7 +766,10 @@ class Person(
     @cachedproperty
     def location(self):
         """See `IObjectWithLocation`."""
-        return PersonLocation.selectOneBy(person=self)
+        location = IStore(PersonLocation).find(
+            PersonLocation,
+            PersonLocation.person == self).one()
+        return location
 
     @property
     def time_zone(self):
@@ -785,7 +788,7 @@ class Person(
             "Cannot set a latitude without longitude (and vice-versa).")
 
         if self.location is not None:
-            self.location.time_zone = time_zone
+            self.location.time_zone = six.ensure_text(time_zone)
             self.location.latitude = latitude
             self.location.longitude = longitude
             self.location.last_modified_by = user
@@ -4097,7 +4100,7 @@ class SSHKey(SQLBase):
         try:
             ssh_keytype = getNS(base64.b64decode(self.keytext))[0].decode(
                 'ascii')
-        except Exception as e:
+        except Exception:
             # We didn't always validate keys, so there might be some that
             # can't be loaded this way.
             if self.keytype == SSHKeyType.RSA:
diff --git a/lib/lp/registry/model/personlocation.py b/lib/lp/registry/model/personlocation.py
index 391d430..50b14c4 100644
--- a/lib/lp/registry/model/personlocation.py
+++ b/lib/lp/registry/model/personlocation.py
@@ -14,36 +14,59 @@ __all__ = [
     'PersonLocation',
     ]
 
-from sqlobject import (
-    BoolCol,
-    FloatCol,
-    ForeignKey,
-    StringCol,
+import pytz
+import six
+from storm.locals import (
+    Bool,
+    DateTime,
+    Float,
+    Int,
+    Reference,
+    Unicode,
     )
 from zope.interface import implementer
 
 from lp.registry.interfaces.location import IPersonLocation
 from lp.registry.interfaces.person import validate_public_person
 from lp.services.database.constants import UTC_NOW
-from lp.services.database.datetimecol import UtcDateTimeCol
-from lp.services.database.sqlbase import SQLBase
+from lp.services.database.stormbase import StormBase
 
 
 @implementer(IPersonLocation)
-class PersonLocation(SQLBase):
+class PersonLocation(StormBase):
     """A person's location."""
+    __storm_table__ = 'PersonLocation'
 
-    _defaultOrder = ['id']
-
-    date_created = UtcDateTimeCol(notNull=True, default=UTC_NOW)
-    person = ForeignKey(
-        dbName='person', foreignKey='Person',
-        storm_validator=validate_public_person, notNull=True, unique=True)
-    latitude = FloatCol(notNull=False)
-    longitude = FloatCol(notNull=False)
-    time_zone = StringCol(notNull=True)
-    last_modified_by = ForeignKey(
-        dbName='last_modified_by', foreignKey='Person',
-        storm_validator=validate_public_person, notNull=True)
-    date_last_modified = UtcDateTimeCol(notNull=True, default=UTC_NOW)
-    visible = BoolCol(notNull=True, default=True)
+    __storm_order__ = 'id'
+    id = Int(primary=True)
+
+    date_created = DateTime(
+        tzinfo=pytz.UTC, name='date_created', allow_none=False,
+        default=UTC_NOW)
+
+    person_id = Int(name='person', allow_none=False)
+    person = Reference(person_id, 'Person.id')
+
+    latitude = Float(allow_none=True)
+    longitude = Float(allow_none=True)
+    time_zone = Unicode(allow_none=False)
+
+    last_modified_by_id = Int(
+        name='last_modified_by',
+        validator=validate_public_person,
+        allow_none=False)
+    last_modified_by = Reference(last_modified_by_id, 'Person.id')
+
+    date_last_modified = DateTime(
+        tzinfo=pytz.UTC, name='date_last_modified', allow_none=False,
+        default=UTC_NOW)
+
+    visible = Bool(name='visible', allow_none=False, default=True)
+
+    def __init__(self, person, time_zone, latitude,
+                 longitude, last_modified_by):
+        self.person = person
+        self.time_zone = six.ensure_text(time_zone)
+        self.latitude = latitude
+        self.longitude = longitude
+        self.last_modified_by = last_modified_by
diff --git a/lib/lp/registry/stories/webservice/xx-personlocation.txt b/lib/lp/registry/stories/webservice/xx-personlocation.txt
index e475673..1a6f739 100644
--- a/lib/lp/registry/stories/webservice/xx-personlocation.txt
+++ b/lib/lp/registry/stories/webservice/xx-personlocation.txt
@@ -1,5 +1,3 @@
-= Person location =
-
 The location of a person is readable through the Web Service API, and can
 be set that way too, but it has been deprecated as we no longer have that
 information in our database, so the latitude/longitude will always be None.
@@ -23,7 +21,8 @@ latitude/longitude read via the Web API will still be None.
     UTC
     >>> print webservice.named_post(
     ...     '/~jdub', 'setLocation', {},
-    ...     latitude='-34.6', longitude='157.0', time_zone='Australia/Sydney')
+    ...     latitude='-34.6', longitude='157.0',
+    ...     time_zone=u'Australia/Sydney')
     HTTP/1.1 200 Ok
     ...
     >>> webservice.get("/~jdub").jsonBody()['time_zone']