Turn find_user into UserFinder class

This moves the cache inside an instance. Also add a few more tests.

Signed-off-by: Dan McGee <dan@archlinux.org>

3 files changed, 103 insertions, 58 deletions

diff --git a/devel/management/commands/reporead.py b/devel/management/commands/reporead.py
index 138931ff..470b785d 100644
--- a/devel/management/commands/reporead.py
+++ b/devel/management/commands/reporead.py
@@ -27,7 +27,7 @@ import logging
 from datetime import datetime
 from optparse import make_option
 
-from devel.utils import find_user
+from devel.utils import UserFinder
 from main.models import Arch, Package, PackageDepend, PackageFile, Repo
 from packages.models import Conflict, Provision, Replacement
 
@@ -182,6 +182,8 @@ def create_multivalued(dbpkg, repopkg, db_attr, repo_attr):
     for name in getattr(repopkg, repo_attr):
         collection.create(name=name)
 
+finder = UserFinder()
+
 def populate_pkg(dbpkg, repopkg, force=False, timestamp=None):
     db_score = 1
 
@@ -200,7 +202,7 @@ def populate_pkg(dbpkg, repopkg, force=False, timestamp=None):
     dbpkg.build_date = repopkg.builddate
     dbpkg.packager_str = repopkg.packager
     # attempt to find the corresponding django user for this string
-    dbpkg.packager = find_user(repopkg.packager)
+    dbpkg.packager = finder.find(repopkg.packager)
 
     if timestamp:
         dbpkg.flag_date = None
diff --git a/devel/tests.py b/devel/tests.py
index 33b02582..c982e502 100644
--- a/devel/tests.py
+++ b/devel/tests.py
@@ -1,7 +1,7 @@
 from django.test import TestCase
 
 from django.contrib.auth.models import User
-from devel.utils import find_user
+from devel.utils import UserFinder 
 from main.models import UserProfile
 
 class DevelTest(TestCase):
@@ -33,6 +33,8 @@ class DevelTest(TestCase):
 class FindUserTest(TestCase):
 
     def setUp(self):
+        self.finder = UserFinder()
+
         self.user1 = User.objects.create(username="joeuser", first_name="Joe",
                 last_name="User", email="user1@example.com")
         self.user2 = User.objects.create(username="john", first_name="John",
@@ -44,29 +46,60 @@ class FindUserTest(TestCase):
             email_addr = "%s@awesome.com" % user.username
             UserProfile.objects.create(user=user, public_email=email_addr)
 
+        self.user4 = User.objects.create(username="tim1", first_name="Tim",
+                last_name="One", email="tim@example.com")
+        self.user5 = User.objects.create(username="tim2", first_name="Tim",
+                last_name="Two", email="timtwo@example.com")
+
     def test_not_matching(self):
-        self.assertIsNone(find_user(None))
-        self.assertIsNone(find_user(""))
-        self.assertIsNone(find_user("Bogus"))
-        self.assertIsNone(find_user("Bogus <invalid"))
-        self.assertIsNone(find_user("Bogus User <bogus@example.com>"))
-        self.assertIsNone(find_user("<bogus@example.com>"))
-        self.assertIsNone(find_user("bogus@example.com"))
-        self.assertIsNone(find_user("Unknown Packager"))
+        self.assertIsNone(self.finder.find(None))
+        self.assertIsNone(self.finder.find(""))
+        self.assertIsNone(self.finder.find("Bogus"))
+        self.assertIsNone(self.finder.find("Bogus <invalid"))
+        self.assertIsNone(self.finder.find("Bogus User <bogus@example.com>"))
+        self.assertIsNone(self.finder.find("<bogus@example.com>"))
+        self.assertIsNone(self.finder.find("bogus@example.com"))
+        self.assertIsNone(self.finder.find("Unknown Packager"))
 
     def test_by_email(self):
-        self.assertEqual(self.user1, find_user("XXX YYY <user1@example.com>"))
-        self.assertEqual(self.user2, find_user("YYY ZZZ <user2@example.com>"))
+        self.assertEqual(self.user1,
+                self.finder.find("XXX YYY <user1@example.com>"))
+        self.assertEqual(self.user2,
+                self.finder.find("YYY ZZZ <user2@example.com>"))
 
     def test_by_profile_email(self):
-        self.assertEqual(self.user1, find_user("XXX <joeuser@awesome.com>"))
-        self.assertEqual(self.user2, find_user("YYY <john@awesome.com>"))
-        self.assertEqual(self.user3, find_user("ZZZ <bjones@awesome.com>"))
+        self.assertEqual(self.user1,
+                self.finder.find("XXX <joeuser@awesome.com>"))
+        self.assertEqual(self.user2,
+                self.finder.find("YYY <john@awesome.com>"))
+        self.assertEqual(self.user3,
+                self.finder.find("ZZZ <bjones@awesome.com>"))
 
     def test_by_name(self):
-        self.assertEqual(self.user1, find_user("Joe User <joe@differentdomain.com>"))
-        self.assertEqual(self.user1, find_user("Joe User"))
-        self.assertEqual(self.user2, find_user("John <john@differentdomain.com>"))
-        self.assertEqual(self.user3, find_user("Bob Jones <bjones AT Arch Linux DOT org>"))
+        self.assertEqual(self.user1,
+                self.finder.find("Joe User <joe@differentdomain.com>"))
+        self.assertEqual(self.user1,
+                self.finder.find("Joe User"))
+        self.assertEqual(self.user2,
+                self.finder.find("John <john@differentdomain.com>"))
+        self.assertEqual(self.user2,
+                self.finder.find("John"))
+        self.assertEqual(self.user3,
+                self.finder.find("Bob Jones <bjones AT Arch Linux DOT org>"))
+
+    def test_cache(self):
+        # simply look two of them up, but then do it repeatedly
+        for i in range(50):
+            self.assertEqual(self.user1,
+                    self.finder.find("XXX YYY <user1@example.com>"))
+            self.assertEqual(self.user3,
+                    self.finder.find("Bob Jones <bjones AT Arch Linux DOT org>"))
+
+    def test_ambiguous(self):
+        self.assertEqual(self.user4,
+                self.finder.find("Tim One <tim@anotherdomain.com>"))
+        self.assertEqual(self.user5,
+                self.finder.find("Tim Two <tim@anotherdomain.com>"))
+        self.assertIsNone(self.finder.find("Tim <tim@anotherdomain.com>"))
 
 # vim: set ts=4 sw=4 et:
diff --git a/devel/utils.py b/devel/utils.py
index acdda959..3a6ad699 100644
--- a/devel/utils.py
+++ b/devel/utils.py
@@ -43,35 +43,25 @@ SELECT pr.user_id, COUNT(*), COUNT(p.flag_date)
 
     return maintainers
 
-def find_user(userstring):
-    '''
-    Attempt to find the corresponding User object for a standard
-    packager string, e.g. something like
-        'A. U. Thor <author@example.com>'.
-    We start by searching for a matching email address; we then move onto
-    matching by first/last name. If we cannot find a user, then return None.
-    '''
-    if not userstring:
-        return None
-    if userstring in find_user.cache:
-        return find_user.cache[userstring]
-    matches = re.match(r'^([^<]+)? ?<([^>]*)>', userstring)
-    if not matches:
-        name = userstring
-        email = None
-    else:
-        name = matches.group(1)
-        email = matches.group(2)
-
-    def user_email():
+
+class UserFinder(object):
+    def __init__(self):
+        self.cache = {}
+
+    @staticmethod
+    def user_email(name, email):
         if email:
             return User.objects.get(email=email)
         return None
-    def profile_email():
+
+    @staticmethod
+    def profile_email(name, email):
         if email:
             return User.objects.get(userprofile__public_email=email)
         return None
-    def user_name():
+
+    @staticmethod
+    def user_name(name, email):
         # yes, a bit odd but this is the easiest way since we can't always be
         # sure how to split the name. Ensure every 'token' appears in at least
         # one of the two name fields.
@@ -86,20 +76,40 @@ def find_user(userstring):
                     Q(last_name__icontains=token))
         return User.objects.get(name_q)
 
-    user = None
-    for matcher in (user_email, profile_email, user_name):
-        try:
-            user = matcher()
-            if user != None:
-                break
-        except (User.DoesNotExist, User.MultipleObjectsReturned):
-            pass
-
-    find_user.cache[userstring] = user
-    return user
-
-# cached mappings of user strings -> User objects so we don't have to do the
-# lookup more than strictly necessary.
-find_user.cache = {}
+    def find(self, userstring):
+        '''
+        Attempt to find the corresponding User object for a standard
+        packager string, e.g. something like
+            'A. U. Thor <author@example.com>'.
+        We start by searching for a matching email address; we then move onto
+        matching by first/last name. If we cannot find a user, then return None.
+        '''
+        if not userstring:
+            return None
+        if userstring in self.cache:
+            return self.cache[userstring]
+        matches = re.match(r'^([^<]+)? ?<([^>]*)>', userstring)
+        if not matches:
+            name = userstring
+            email = None
+        else:
+            name = matches.group(1)
+            email = matches.group(2)
+
+        user = None
+        find_methods = (self.user_email, self.profile_email, self.user_name)
+        for matcher in find_methods:
+            try:
+                user = matcher(name, email)
+                if user != None:
+                    break
+            except (User.DoesNotExist, User.MultipleObjectsReturned):
+                pass
+
+        self.cache[userstring] = user
+        return user
+
+    def clear_cache(self):
+        self.cache = {}
 
 # vim: set ts=4 sw=4 et: