[bridgedb/develop] Move Dist.getNumBridgesPerAnswer() → Distributor.bridgesPerResponse().

[isis at torproject.org]

commit a51f553e281091bc365d4161da2fb8d1b33923d9
Author: Isis Lovecruft <isis at torproject.org>
Date:   Sat Apr 18 22:41:27 2015 +0000

    Move Dist.getNumBridgesPerAnswer() → Distributor.bridgesPerResponse().
    
    It was meant to be a method; it operates on the length of the primary
    hashring of a Distributor.  Having it outside of the class that it
    interacts with is just wacky.
    
     * CHANGE getNumBridgesPerAnswer into a method for the Distributor
       class.
     * CHANGE the unittests for the function to take this into account.
---
 lib/bridgedb/Dist.py           |   43 +++++++++++++++++-------------
 lib/bridgedb/test/test_Dist.py |   56 ++++++++++++++++++----------------------
 2 files changed, 50 insertions(+), 49 deletions(-)

diff --git a/lib/bridgedb/Dist.py b/lib/bridgedb/Dist.py
index e66c1e6..93c08c1 100644
--- a/lib/bridgedb/Dist.py
+++ b/lib/bridgedb/Dist.py
@@ -48,26 +48,13 @@ class EmailRequestedKey(Exception):
     """Raised when an incoming email requested a copy of our GnuPG keys."""
 
 
-def getNumBridgesPerAnswer(ring, max_bridges_per_answer=3):
-    if len(ring) < 20:
-        n_bridges_per_answer = 1
-    if 20 <= len(ring) < 100:
-        n_bridges_per_answer = min(2, max_bridges_per_answer)
-    if len(ring) >= 100:
-        n_bridges_per_answer = max_bridges_per_answer
-
-    logging.debug("Returning %d bridges from ring of len: %d" %
-                  (n_bridges_per_answer, len(ring)))
-
-    return n_bridges_per_answer
-
-
 class Distributor(object):
     """Distributes bridges to clients."""
 
     def __init__(self):
         super(Distributor, self).__init__()
         self.name = None
+        self.hashring = None
 
     def setDistributorName(self, name):
         """Set a **name** for identifying this distributor.
@@ -89,6 +76,21 @@ class Distributor(object):
         self.name = name
         self.hashring.distributorName = name
 
+    def bridgesPerResponse(self, hashring=None, maximum=3):
+        if hashring is None:
+            hashring = self.hashring
+
+        if len(hashring) < 20:
+            n = 1
+        if 20 <= len(hashring) < 100:
+            n = min(2, maximum)
+        if len(hashring) >= 100:
+            n = maximum
+
+        logging.debug("Returning %d bridges from ring of len: %d" %
+                      (n, len(hashring)))
+        return n
+
 
 class HTTPSDistributor(Distributor):
     """A Distributor that hands out bridges based on the IP address of an
@@ -157,6 +159,9 @@ class HTTPSDistributor(Distributor):
 
         self.setDistributorName('HTTPS')
 
+    def bridgesPerResponse(self, hashring=None, maximum=3):
+        return super(HTTPSDistributor, self).bridgesPerResponse(hashring, maximum)
+
     @classmethod
     def getSubnet(cls, ip, usingProxy=False, proxySubnets=4):
         """Map all clients whose **ip**s are within the same subnet to the same
@@ -386,7 +391,7 @@ class HTTPSDistributor(Distributor):
                                   populate_from=self.hashring.bridges)
 
         # Determine the appropriate number of bridges to give to the client:
-        returnNum = getNumBridgesPerAnswer(ring, max_bridges_per_answer=N)
+        returnNum = self.bridgesPerResponse(ring, maximum=N)
         answer = ring.getBridges(position, returnNum)
 
         return answer
@@ -440,6 +445,9 @@ class EmailBasedDistributor(Distributor):
 
         self.setDistributorName('Email')
 
+    def bridgesPerResponse(self, hashring=None, maximum=3):
+        return super(EmailBasedDistributor, self).bridgesPerResponse(hashring, maximum)
+
     def insert(self, bridge):
         """Assign a bridge to this distributor."""
         self.hashring.insert(bridge)
@@ -515,9 +523,8 @@ class EmailBasedDistributor(Distributor):
                                       filterBridgesByRules(ruleset),
                                       populate_from=self.hashring.bridges)
 
-            numBridgesToReturn = getNumBridgesPerAnswer(ring,
-                                                        max_bridges_per_answer=N)
-            result = ring.getBridges(pos, numBridgesToReturn)
+            returnNum = self.bridgesPerResponse(ring, maximum=N)
+            result = ring.getBridges(pos, returnNum)
 
             db.setEmailTime(bridgeRequest.client, now)
             db.commit()
diff --git a/lib/bridgedb/test/test_Dist.py b/lib/bridgedb/test/test_Dist.py
index a3c8308..f47449c 100644
--- a/lib/bridgedb/test/test_Dist.py
+++ b/lib/bridgedb/test/test_Dist.py
@@ -63,37 +63,6 @@ def _generateFakeBridges(n=500):
 BRIDGES = _generateFakeBridges()
 
 
-class GetNumBridgesPerAnswerTests(unittest.TestCase):
-    """Unittests for :func:`bridgedb.Dist.getNumBridgesPerAnswer`."""
-
-    def setUp(self):
-        self.key = 'aQpeOFIj8q20s98awfoiq23rpOIjFaqpEWFoij1X'
-        self.ring = BridgeRing(self.key)
-        self.bridges = _generateFakeBridges()
-
-    def test_Dist_getNumBridgesPerAnswer_120(self):
-        [self.ring.insert(bridge) for bridge in self.bridges[:120]]
-        self.assertEqual(Dist.getNumBridgesPerAnswer(self.ring), 3)
-
-    def test_Dist_getNumBridgesPerAnswer_100(self):
-        [self.ring.insert(bridge) for bridge in self.bridges[:100]]
-        self.assertEqual(Dist.getNumBridgesPerAnswer(self.ring), 3)
-
-    def test_Dist_getNumBridgesPerAnswer_50(self):
-        [self.ring.insert(bridge) for bridge in self.bridges[:60]]
-        self.assertEqual(Dist.getNumBridgesPerAnswer(self.ring), 2)
-
-    def test_Dist_getNumBridgesPerAnswer_15(self):
-        [self.ring.insert(bridge) for bridge in self.bridges[:15]]
-        self.assertEqual(Dist.getNumBridgesPerAnswer(self.ring), 1)
-
-    def test_Dist_getNumBridgesPerAnswer_100_max_5(self):
-        [self.ring.insert(bridge) for bridge in self.bridges[:100]]
-        self.assertEqual(
-            Dist.getNumBridgesPerAnswer(self.ring, max_bridges_per_answer=5),
-            5)
-
-
 class HTTPSDistributorTests(unittest.TestCase):
     """Tests for :class:`HTTPSDistributor`."""
 
@@ -134,6 +103,31 @@ class HTTPSDistributorTests(unittest.TestCase):
         self.assertEqual(dist.proxySubring, 4)
         self.assertEqual(dist.totalSubrings, 4)
 
+    def test_HTTPSDistributor_bridgesPerResponse_120(self):
+        dist = Dist.HTTPSDistributor(3, self.key)
+        [dist.insert(bridge) for bridge in self.bridges[:120]]
+        self.assertEqual(dist.bridgesPerResponse(), 3)
+
+    def test_HTTPSDistributor_bridgesPerResponse_100(self):
+        dist = Dist.HTTPSDistributor(3, self.key)
+        [dist.hashring.insert(bridge) for bridge in self.bridges[:100]]
+        self.assertEqual(dist.bridgesPerResponse(), 3)
+
+    def test_HTTPSDistributor_bridgesPerResponse_50(self):
+        dist = Dist.HTTPSDistributor(3, self.key)
+        [dist.insert(bridge) for bridge in self.bridges[:60]]
+        self.assertEqual(dist.bridgesPerResponse(), 2)
+
+    def test_HTTPSDistributor_bridgesPerResponse_15(self):
+        dist = Dist.HTTPSDistributor(3, self.key)
+        [dist.insert(bridge) for bridge in self.bridges[:15]]
+        self.assertEqual(dist.bridgesPerResponse(), 1)
+
+    def test_HTTPSDistributor_bridgesPerResponse_100_max_5(self):
+        dist = Dist.HTTPSDistributor(3, self.key)
+        [dist.insert(bridge) for bridge in self.bridges[:100]]
+        self.assertEqual(dist.bridgesPerResponse(maximum=5), 5)
+
     def test_HTTPSDistributor_getSubnet_usingProxy(self):
         """HTTPSDistributor.getSubnet(usingProxy=True) should return a proxy
         group number.