commit 346d321fa4f78626c49cd4ff6d82a346267f6c0c Author: Isis Lovecruft isis@torproject.org Date: Sun Apr 19 03:25:11 2015 +0000
Remove Bridges.BridgeHolder class.
It wasn't a real metaclass, base class, or interface; its methods were never used; it didn't provide any decent documentation; not everything which inherited from it used its methods; nothing called super() for it, or otherwise called methods on it. Totally useless.
* REMOVE bridgedb.Bridges.BridgeHolder class. * CHANGE everything which previously inherited from BridgeHolder to simply inherit from object instead. --- lib/bridgedb/Bridges.py | 63 ++++++++++++++++------------------------ lib/bridgedb/Main.py | 4 +-- lib/bridgedb/persistent.py | 3 +- lib/bridgedb/test/test_Main.py | 6 ++-- 4 files changed, 32 insertions(+), 44 deletions(-)
diff --git a/lib/bridgedb/Bridges.py b/lib/bridgedb/Bridges.py index 5742794..1a06dd8 100644 --- a/lib/bridgedb/Bridges.py +++ b/lib/bridgedb/Bridges.py @@ -39,18 +39,6 @@ DIGEST_LEN = 20 PORTSPEC_LEN = 16
-class BridgeHolder(object): - """Abstract base class for all classes that hold bridges.""" - def insert(self, bridge): - raise NotImplementedError - - def clear(self): - pass - - def dumpAssignments(self, f, description=""): - pass - - class BridgeRingParameters(object): """Store validated settings on minimum number of Bridges with certain attributes which should be included in any generated subring of a @@ -99,7 +87,8 @@ class BridgeRingParameters(object): self.needPorts = needPorts[:] self.needFlags = [(flag.lower(), count) for flag, count in needFlags[:]]
-class BridgeRing(BridgeHolder): + +class BridgeRing(object): """Arranges bridges into a hashring based on an hmac function."""
def __init__(self, key, answerParameters=None): @@ -333,15 +322,13 @@ class BridgeRing(BridgeHolder): f.write("%s %s\n" % (b.fingerprint, " ".join(desc).strip()))
-class FixedBridgeSplitter(BridgeHolder): - """A bridgeholder that splits bridges up based on an hmac and assigns - them to several sub-bridgeholders with equal probability. +class FixedBridgeSplitter(object): + """Splits bridges up based on an HMAC and assigns them to one of several + subhashrings with equal probability. """ def __init__(self, key, rings): self.hmac = getHMACFunc(key, hex=True) self.rings = rings[:] - for r in self.rings: - assert(isinstance(r, BridgeHolder))
def insert(self, bridge): # Grab the first 4 bytes @@ -366,7 +353,7 @@ class FixedBridgeSplitter(BridgeHolder): """Write all bridges assigned to this hashring to ``filename``.
:param string description: If given, include a description next to the - index number of the ring from :attr:`FilteredBridgeHolder.rings` + index number of the ring from :attr:`FilteredBridgeSplitter.rings` the following bridges were assigned to. For example, if the description is ``"IPv6 obfs2 bridges"`` the line would read: ``"IPv6 obfs2 bridges ring=3"``. @@ -374,7 +361,8 @@ class FixedBridgeSplitter(BridgeHolder): for index, ring in zip(xrange(len(self.rings)), self.rings): ring.dumpAssignments(filename, "%s ring=%s" % (description, index))
-class UnallocatedHolder(BridgeHolder): + +class UnallocatedHolder(object): """A pseudo-bridgeholder that ignores its bridges and leaves them unassigned. """ @@ -407,10 +395,11 @@ class UnallocatedHolder(BridgeHolder): continue f.write("%s %s\n" % (bridge.hex_key, " ".join(desc).strip()))
-class BridgeSplitter(BridgeHolder): - """A BridgeHolder that splits incoming bridges up based on an hmac, - and assigns them to sub-bridgeholders with different probabilities. - Bridge-to-bridgeholder associations are recorded in a store. + +class BridgeSplitter(object): + """Splits incoming bridges up based on an HMAC, and assigns them to + sub-bridgeholders with different probabilities. Bridge ←→ BridgeSplitter + associations are recorded in a store. """ def __init__(self, key): self.hmac = getHMACFunc(key, hex=True) @@ -428,12 +417,13 @@ class BridgeSplitter(BridgeHolder): return n
def addRing(self, ring, ringname, p=1): - """Add a new bridgeholder. - ring -- the bridgeholder to add. - ringname -- a string representing the bridgeholder. This is used - to record which bridges have been assigned where in the store. - p -- the relative proportion of bridges to assign to this - bridgeholder. + """Add a new subring. + + :param ring: The subring to add. + :param str ringname: This is used to record which bridges have been + assigned where in the store. + :param int p: The relative proportion of bridges to assign to this + bridgeholder. """ self.ringsByName[ringname] = ring self.pValues.append(self.totalP) @@ -493,8 +483,8 @@ class BridgeSplitter(BridgeHolder): ring.dumpAssignments(f, "%s %s" % (description, name))
-class FilteredBridgeSplitter(BridgeHolder): - """A configurable BridgeHolder that filters bridges into subrings. +class FilteredBridgeSplitter(object): + """Places bridges into subrings based upon sets of filters.
The set of subrings and conditions used to assign :class:`Bridge`s should be passed to :meth:`~FilteredBridgeSplitter.addRing`. @@ -513,7 +503,9 @@ class FilteredBridgeSplitter(BridgeHolder): - ``ringname`` is a unique string identifying the subring. - ``filterFn`` is a callable which filters Bridges in some manner, i.e. by whether they are IPv4 or IPv6, etc. - - ``subring`` is a :class:`BridgeHolder`. + - ``subring`` is any of the horribly-implemented, + I-guess-it-passes-for-some-sort-of-hashring classes in this + module. :ivar hmac: DOCDOC :ivar bridges: DOCDOC :type distributorName: str @@ -591,7 +583,6 @@ class FilteredBridgeSplitter(BridgeHolder): def addRing(self, subring, ringname, filterFn, populate_from=None): """Add a subring to this hashring.
- :type subring: :class:`BridgeHolder` :param subring: The subring to add. :param str ringname: A unique name for identifying the new subring. :param filterFn: A function whose input is a :class:`Bridge`, and @@ -617,10 +608,6 @@ class FilteredBridgeSplitter(BridgeHolder): # I suppose since it contains memory addresses, it *is* technically # likely to be a unique string, but it is messy.
- if not isinstance(subring, BridgeHolder): - logging.fatal("%s hashring can't add invalid subring: %r" - % (self.distributorName, subring)) - return False if ringname in self.filterRings.keys(): logging.fatal("%s hashring already has a subring named '%s'!" % (self.distributorName, ringname)) diff --git a/lib/bridgedb/Main.py b/lib/bridgedb/Main.py index 0bb319d..f382cc5 100644 --- a/lib/bridgedb/Main.py +++ b/lib/bridgedb/Main.py @@ -47,7 +47,7 @@ def load(state, hashring, clear=False): store them into our ``state.hashring`` instance. The ``state`` will be saved again at the end of this function.
- :type hashring: :class:`BridgeSplitter <bridgedb.Bridges.BridgeHolder>` + :type hashring: :class:`~bridgedb.Bridges.BridgeSplitter` :param hashring: A class which provides a mechanism for HMACing Bridges in order to assign them to hashrings. :param boolean clear: If True, clear all previous bridges from the @@ -323,7 +323,7 @@ def run(options, reactor=reactor): :ivar cfg: The current configuration, including any in-memory settings (i.e. settings whose values were not obtained from the config file, but were set via a function somewhere) - :type hashring: A :class:`bridgedb.Bridges.BridgeHolder` + :type hashring: A :class:`~bridgedb.Bridges.BridgeSplitter` :ivar hashring: A class which takes an HMAC key and splits bridges into their hashring assignments. :type proxyList: :class:`~bridgedb.proxy.ProxySet` diff --git a/lib/bridgedb/persistent.py b/lib/bridgedb/persistent.py index 1b2507c..c996daa 100644 --- a/lib/bridgedb/persistent.py +++ b/lib/bridgedb/persistent.py @@ -88,7 +88,8 @@ class State(jelly.Jellyable): modules which are known to be unjelliable/unpicklable so far are:
- bridgedb.Dist - - bridgedb.Bridges.BridgeHolder and all other ``splitter`` classes + - bridgedb.Bridges, and all "splitter" and "ring" classes contained + within
:property statefile: The filename to retrieve a pickled, jellied :class:`~bridgedb.persistent.State` instance from. (default: diff --git a/lib/bridgedb/test/test_Main.py b/lib/bridgedb/test/test_Main.py index 26012f7..e53256f 100644 --- a/lib/bridgedb/test/test_Main.py +++ b/lib/bridgedb/test/test_Main.py @@ -58,7 +58,7 @@ def mockUpdateBridgeHistory(bridges, timestamps): (fingerprint, timestamp))
-class MockBridgeHolder(object): +class MockHashring(object): def __init__(self): self._bridges = {} def __len__(self): @@ -170,8 +170,8 @@ class MainTests(unittest.TestCase): self.state = Main.persistent.State(**self.config.__dict__) self.key = base64.b64decode('TvPS1y36BFguBmSOvhChgtXB2Lt+BOw0mGfz9SZe12Y=')
- # Create a BridgeSplitter - self.hashring = MockBridgeHolder() + # Create a pseudo hashring + self.hashring = MockHashring()
# Functions which some tests mock, which we'll need to re-replace # later in tearDown():