launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

Steve Kowalik has proposed merging lp:~stevenk/launchpad/bulk-eid-to-obj into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/bulk-eid-to-obj/+merge/171705

Destroy enterpriseid_to_object for being terrible and issuing one query per eid passed to it. Its replacement, enterpriseids_to_objects will make one query per type of object.
-- 
https://code.launchpad.net/~stevenk/launchpad/bulk-eid-to-obj/+merge/171705
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/bulk-eid-to-obj into lp:launchpad.

=== modified file 'lib/lp/services/auditor/client.py'
--- lib/lp/services/auditor/client.py	2013-06-18 03:57:26 +0000
+++ lib/lp/services/auditor/client.py	2013-06-27 02:35:33 +0000
@@ -12,7 +12,7 @@
 
 from lp.services.config import config
 from lp.services.enterpriseid import (
-    enterpriseid_to_object,
+    enterpriseids_to_objects,
     object_to_enterpriseid,
     )
 
@@ -42,7 +42,11 @@
         logs = super(AuditorClient, self).receive(
             obj, operation, actorobj, limit)
         # Process the actors and objects back from enterprise ids.
-        for entry in logs['log-entries']:
-            entry['actor'] = enterpriseid_to_object(entry['actor'])
-            entry['object'] = enterpriseid_to_object(entry['object'])
+        eids = []
+        for entry in logs['log-entries']:
+            eids.extend([entry['actor'], entry['object']])
+        map_eids_to_obj = enterpriseids_to_objects(eids)
+        for entry in logs['log-entries']:
+            entry['actor'] = map_eids_to_obj[entry['actor']]
+            entry['object'] = map_eids_to_obj[entry['object']]
         return logs['log-entries']

=== modified file 'lib/lp/services/enterpriseid.py'
--- lib/lp/services/enterpriseid.py	2012-07-26 22:12:53 +0000
+++ lib/lp/services/enterpriseid.py	2013-06-27 02:35:33 +0000
@@ -1,4 +1,4 @@
-# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2012-2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Enterprise ID utilities."""
@@ -6,12 +6,14 @@
 __metaclass__ = type
 __all__ = [
     'object_to_enterpriseid',
-    'enterpriseid_to_object',
+    'enterpriseids_to_objects',
     ]
 
+from collections import defaultdict
 import os
 
 from lp.services.config import config
+from lp.services.database.bulk import load
 
 
 def object_to_enterpriseid(obj):
@@ -25,17 +27,34 @@
     return '%s:%s:%d' % (instance, otype, obj.id)
 
 
-def enterpriseid_to_object(eid):
-    """Given an SOA Enterprise ID, return the object that it references."""
+def _known_types():
     # Circular imports.
     from lp.registry.model.person import Person
     from lp.soyuz.model.queue import PackageUpload
-    scheme = eid.split(':')
-    if not scheme[0].startswith('lp'):
-        raise TypeError
-    known_types = {
+    return {
         'PackageUpload': PackageUpload,
         'Person': Person,
     }
-    klass = known_types[scheme[1]]
-    return klass.get(scheme[2])
+
+
+def enterpriseids_to_objects(eids):
+    """Given a list of SOA Enterprise IDs, return a dict that maps the ID to
+    its concrete object."""
+    map_id_to_obj = {}
+    obj_id_to_eid = defaultdict(dict)
+    type_ids = _known_types()
+    for kind in type_ids:
+        type_ids[kind] = []
+    for eid in eids:
+        if not eid.startswith('lp'):
+            raise TypeError
+        map_id_to_obj[eid] = None
+        scheme = eid.split(':')
+        type_ids[scheme[1]].append(int(scheme[2]))
+        obj_id_to_eid[scheme[1]][int(scheme[2])] = eid
+    types = _known_types()
+    for kind in types:
+        objs = load(types[kind], type_ids[kind])
+        for obj in objs:
+            map_id_to_obj[obj_id_to_eid[kind][obj.id]] = obj
+    return map_id_to_obj

=== modified file 'lib/lp/services/tests/test_enterpriseid.py'
--- lib/lp/services/tests/test_enterpriseid.py	2012-03-05 02:24:17 +0000
+++ lib/lp/services/tests/test_enterpriseid.py	2013-06-27 02:35:33 +0000
@@ -1,16 +1,23 @@
-# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2012-2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Test Enterprise ID utilities."""
 
 __metaclass__ = type
 
+from testtools.matchers import Equals
+
+from lp.services.database.interfaces import IStore
 from lp.services.enterpriseid import (
-    enterpriseid_to_object,
+    enterpriseids_to_objects,
     object_to_enterpriseid,
     )
-from lp.testing import TestCaseWithFactory
+from lp.testing import (
+    StormStatementRecorder,
+    TestCaseWithFactory,
+    )
 from lp.testing.layers import DatabaseFunctionalLayer
+from lp.testing.matchers import HasQueryCount
 
 
 class TestEnterpriseId(TestCaseWithFactory):
@@ -22,8 +29,14 @@
         expected = 'lp-development:Person:%d' % person.id
         self.assertEqual(expected, eid)
 
-    def test_enterpriseid_to_object(self):
-        person = self.factory.makePerson()
-        eid = 'lp-development:Person:%d' % person.id
-        obj = enterpriseid_to_object(eid)
-        self.assertEqual(person, obj)
+    def test_enterpriseids_to_objects(self):
+        expected = {}
+        for x in range(10):
+            person = self.factory.makePerson()
+            expected['lp-development:Person:%d' % person.id] = person
+        IStore(expected.values()[0].__class__).invalidate()
+        with StormStatementRecorder() as recorder:
+            objects = enterpriseids_to_objects(expected.keys())
+        self.assertThat(recorder, HasQueryCount(Equals(1)))
+        self.assertContentEqual(expected.keys(), objects.keys())
+        self.assertContentEqual(expected.values(), objects.values())