commit a51f553e281091bc365d4161da2fb8d1b33923d9 Author: Isis Lovecruft isis@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.