commit e68eee92426acc8597547e57116b85f539b84ca8 Author: Christian Fromme <kaner@strace.org> Date: Sat Feb 26 21:44:24 2011 +0100 Simplify the 'send email is Tor version is out of date' feature. --- weather/weatherapp/ctlutil.py | 60 +++++++++++++++++++++----------------- weather/weatherapp/emails.py | 3 +- weather/weatherapp/models.py | 61 +++++++-------------------------------- weather/weatherapp/tests.py | 12 ++++---- weather/weatherapp/updaters.py | 9 ++--- 5 files changed, 55 insertions(+), 90 deletions(-) diff --git a/weather/weatherapp/ctlutil.py b/weather/weatherapp/ctlutil.py index 0487157..c59047c 100644 --- a/weather/weatherapp/ctlutil.py +++ b/weather/weatherapp/ctlutil.py @@ -232,6 +232,14 @@ class CtlUtil: return search.group().split()[2].replace(' ', '') else: return '' + + def get_highest_version(self, versionlist): + """Return the highest Tor version from a list of versions. + """ + if len(versionlist) is 0: + return "" + versionlist.sort() + return versionlist[-1] def get_version_type(self, fingerprint): """Get the type of version the relay with fingerprint C{fingerprint} @@ -242,13 +250,18 @@ class CtlUtil: @rtype: str @return: The type of version of Tor the client is running, where the - types are RECOMMENDED, OBSOLETE, and UNRECOMMENDED. Returns RECOMMENDED - if the relay is running the most recent stable release or a more - recent unstable release , UNRECOMMENDED - if it is running an older version than the most recent stable release - that is contained in the list returned by C{get_rec_version_list()}, - and OBSOLETE if the version isn't on the list. If the relay's version - cannot be determined, return ERROR. + types are RECOMMENDED or OBSOLETE. + + Returns RECOMMENDED if the relay is running a version that is found + in the `recommended' versions list or if the version is a more recent + dev version than the most recent recommended dev version. (Basically, + we don't want to bother people for being nice and testing new versions + for us) + There is one more special case where we return RECOMMENDED and that is + when there is *no* recommended version currently known. + + We return OBSOLETE if neither of the above criteria matches. + If the version cannot be determined, return ERROR. """ version_list = self.get_rec_version_list() client_version = self.get_version(fingerprint) @@ -256,31 +269,24 @@ class CtlUtil: if client_version == '': return 'ERROR' - current_stable_index = -1 - for version in version_list: - if 'alpha' in version or 'beta' in version: - current_stable_index = version_list.index(version) - 1 - break - - #if the client has one of these versions, return RECOMMENDED - rec_list = version_list[current_stable_index:] - - #if the client has one of these, return UNRECOMMENDED - unrec_list = version_list[:current_stable_index] + # Special case when the dirauth can't agree on recommended versions, + # the list is empty. In that case we play along as if everything was + # fine + if len(version_list) is 0: + return 'RECOMMENDED' + + if client_version in version_list: + return 'RECOMMENDED' - for version in rec_list: - if client_version == version: + # Check if the user is running a more recent dev version than is found + # in the `recommended' list + if client_version.endswith("-dev"): + version_list.append(client_version) + if get_highest_version(version_list) is client_version: return 'RECOMMENDED' - - for version in unrec_list: - if client_version == version: - return 'UNRECOMMENDED' - #the client doesn't have a RECOMMENDED or UNRECOMMENDED version, - #so it must be OBSOLETE return 'OBSOLETE' - def has_rec_version(self, fingerprint): """Check if a Tor relay is running a recommended version of the Tor software. diff --git a/weather/weatherapp/emails.py b/weather/weatherapp/emails.py index 4739682..5b8b8db 100644 --- a/weather/weatherapp/emails.py +++ b/weather/weatherapp/emails.py @@ -354,8 +354,7 @@ def version_tuple(recipient, fingerprint, name, version_type, unsubs_auth, @param fingerprint: The fingerprint for the router this user is subscribed to. @type version_type: str - @param version_type: Either 'UNRECOMMENDED' or 'OBSOLETE', depending on the - user's preferences for this notification type. + @param version_type: Currently can only be 'OBSOLETE' @rtype: tuple @return: A tuple containing information about the email to be sent in diff --git a/weather/weatherapp/models.py b/weather/weatherapp/models.py index 0833b47..a5a9e00 100644 --- a/weather/weatherapp/models.py +++ b/weather/weatherapp/models.py @@ -370,7 +370,7 @@ class Subscriber(models.Model): v = VersionSub.objects.get(subscriber = self) data['version_type'] = v.notify_type else: - data['version_type'] = u'UNRECOMMENDED' + data['version_type'] = u'OBSOLETE' data['get_band_low'] = self.has_bandwidth_sub() if data['get_band_low']: @@ -474,9 +474,7 @@ class VersionSub(Subscription): notifications to their C{subscriber} if the C{subscriber}'s C{router} is running a version of Tor that is out-of-date. OBSOLETE notifications are triggered if the C{router}'s version of Tor is not in the list of - recommended versions (obtained via TorCtl), and UNRECOMMENDED notifications - are triggered if the C{router}'s version fo Tor is not the most recent - stable (non-alpha/beta) version of Tor in the list of recommended versions. + recommended versions (obtained via TorCtl), with a few exceptions. Django uses class variables to specify model fields, but these fields are practically used and thought of as instance variables, so this documentation will refer to them as such. Field types are specified as @@ -487,7 +485,7 @@ class VersionSub(Subscription): @cvar _NOTIFY_TYPE_MAX_LEN: Maximum length for L{notify_type} field. @type notify_type: CharField (str) - @ivar notify_type: The type of notification, either 'UNRECOMMENDED' or + @ivar notify_type: The type of notification, currently can only be 'OBSOLETE'. Required constructor argument. """ @@ -742,25 +740,10 @@ class GenericForm(forms.Form): @type _GET_VERSION_INIT: bool @cvar _GET_VERSION_INIT: Initial display value and default submission value of the L{get_version} checkbox. - @type _GET_VERSION_LABEL: str - @cvar _GET_VERSION_LABEL: Text displayed next to L{get_version} checkbox. - @type _VERSION_TYPE_CHOICE_1: str - @cvar _VERSION_TYPE_CHOICE_1: Backend name for the first choice of the - L{version_type} field. - @type _VERSION_TYPE_CHOICE_1_H: str - @cvar _VERSION_TYPE_CHOICE_1_H: Frontend (human readable) name for the - first choice of the L{version_type} field. - @type _VERSION_TYPE_CHOICE_2: str - @cvar _VERSION_TYPE_CHOICE_2: Backend name for the second choice of the - L{version_type} field. - @type _VERSION_TYPE_CHOICE_2_H: str - @cvar _VERSION_TYPE_CHOICE_2_H: Frontend (human readable) name for the - second choice of the L{version_type} field. - @type _VERSION_TYPE_CHOICES: list [tuple (str)] - @cvar _VERSION_TYPE_CHOICES: List of tuples of backend and frontend names - for each choice of the L{version_type} field. @type _VERSION_TYPE_INIT: str @cvar _VERSION_TYPE_INIT: Initial tuple for the L{version_type} field. + @type _GET_VERSION_LABEL: str + @cvar _GET_VERSION_LABEL: Text displayed next to L{get_version} checkbox. @type _VERSION_SECTION_INFO: str @cvar _VERSION_SECTION_INFO: Text explaining the version subscription, displayed in the expandable version section of the form, with HTML @@ -823,9 +806,6 @@ class GenericForm(forms.Form): @type get_version: BooleanField @ivar get_version: Checkbox letting users choose to subscribe to a L{VersionSub}. - @type version_type: ChoiceField - @ivar version_type: Radio button list letting users choose the type of - L{VersionSub} to subscribe to. @type get_band_low: BooleanField @ivar get_band_low: Checkbox letting users choose to subscribe to a @@ -854,18 +834,10 @@ class GenericForm(forms.Form): _NODE_DOWN_GRACE_PD_UNIT_INIT = ('H', 'hours') _GET_VERSION_INIT = False + _VERSION_TYPE_INIT = "OBSOLETE" _GET_VERSION_LABEL = 'Email me when the router\'s Tor version is out of date' - _VERSION_TYPE_CHOICE_1 = 'UNRECOMMENDED' - _VERSION_TYPE_CHOICE_1_H = 'Recommended Updates' - _VERSION_TYPE_CHOICE_2 = 'OBSOLETE' - _VERSION_TYPE_CHOICE_2_H = 'Required Updates' - _VERSION_TYPE_CHOICES = [ ('UNRECOMMENDED', 'Recommended Updates'), - ('OBSOLETE', 'Required Updates') ] - _VERSION_TYPE_INIT = 'UNRECOMMENDED' - _VERSION_SECTION_INFO = '<p><em>Recommended Updates:</em> Emails when\ - the router is not running the most up-to-date stable version of Tor.</p> \ - <p><em>Required Updates:</em> Emails when the router is running \ - an obsolete version of Tor.</p>' + _VERSION_SECTION_INFO = 'Emails when\ + the router is not running a recommended version of Tor.' _GET_BAND_LOW_INIT = False _GET_BAND_LOW_LABEL = 'Email me when the router has low bandwidth capacity' @@ -914,9 +886,6 @@ class GenericForm(forms.Form): get_version = forms.BooleanField(required=False, label=_GET_VERSION_LABEL, widget=forms.CheckboxInput(attrs={'class':_CLASS_CHECK})) - version_type = forms.ChoiceField(required=True, - choices=(_VERSION_TYPE_CHOICES), - widget=forms.RadioSelect(attrs={'class':_CLASS_RADIO})) get_band_low = forms.BooleanField(required=False, label=_GET_BAND_LOW_LABEL, @@ -976,9 +945,6 @@ class GenericForm(forms.Form): if 'node_down_grace_pd' in errors and not data['get_node_down']: del errors['node_down_grace_pd'] data['node_down_grace_pd'] = GenericForm._NODE_DOWN_GRACE_PD_INIT - if 'version_type' in errors and not data['get_version']: - del errors['version_type'] - data['version_type'] = GenericForm._VERSION_TYPE_INIT if 'band_low_threshold' in errors and not data['get_band_low']: del errors['band_low_threshold'] data['band_low_threshold'] = GenericForm._BAND_LOW_THRESHOLD_INIT @@ -1230,8 +1196,7 @@ class SubscribeForm(GenericForm): grace_pd=self.cleaned_data['node_down_grace_pd']) node_down_sub.save() if self.cleaned_data['get_version']: - version_sub = VersionSub(subscriber=subscriber, - notify_type = self.cleaned_data['version_type']) + version_sub = VersionSub(subscriber=subscriber) version_sub.save() if self.cleaned_data['get_band_low']: band_low_sub = BandwidthSub(subscriber=subscriber, @@ -1325,16 +1290,12 @@ class PreferencesForm(GenericForm): # it depending on the current value. if old_data['get_version']: v = VersionSub.objects.get(subscriber = self.user) - if new_data['get_version']: - v.notify_type = new_data['version_type'] - v.save() - else: + if not new_data['get_version']: v.delete() # If there wasn't a subscription before and it is checked now, then # make one. elif new_data['get_version']: - v = VersionSub(subscriber=self.user, - notify_type=new_data['version_type']) + v = VersionSub(subscriber=self.user) v.save() # If there already was a subscription, get it and update it or delete diff --git a/weather/weatherapp/tests.py b/weather/weatherapp/tests.py index fd112cd..931bdc9 100644 --- a/weather/weatherapp/tests.py +++ b/weather/weatherapp/tests.py @@ -96,7 +96,7 @@ class TestWeb(TestCase): 'get_node_down' : False, 'node_down_grace_pd' : '', 'get_version' : True, - 'version_type' : 'UNRECOMMENDED', + 'version_type' : 'OBSOLETE', 'get_band_low': False, 'band_low_threshold' : '', 'get_t_shirt' : False}, @@ -118,7 +118,7 @@ class TestWeb(TestCase): #Verify that the subscription info was stored correctly version_sub = VersionSub.objects.get(subscriber = subscriber) self.assertEqual(version_sub.emailed, False) - self.assertEqual(version_sub.notify_type, 'UNRECOMMENDED') + self.assertEqual(version_sub.notify_type, 'OBSOLETE') #Test that one message has been sent for i in range(0, 100, 1): @@ -218,7 +218,7 @@ class TestWeb(TestCase): 'get_node_down' : False, 'node_down_grace_pd' : 1, 'get_version' : False, - 'version_type' : 'UNRECOMMENDED', + 'version_type' : 'OBSOLETE', 'get_band_low' : False, 'band_low_threshold' : '', 'get_t_shirt' : True}, @@ -285,7 +285,7 @@ class TestWeb(TestCase): 'get_node_down' : True, 'node_down_grace_pd' : '', 'get_version' : True, - 'version_type' : 'UNRECOMMENDED', + 'version_type' : 'OBSOLETE', 'get_band_low': True, 'band_low_threshold' : '', 'get_t_shirt' : True}, @@ -312,7 +312,7 @@ class TestWeb(TestCase): version = VersionSub.objects.get(subscriber = subscriber) self.assertEqual(version.emailed, False) - self.assertEqual(version.notify_type, 'UNRECOMMENDED') + self.assertEqual(version.notify_type, 'OBSOLETE') bandwidth = BandwidthSub.objects.get(subscriber = subscriber) self.assertEqual(bandwidth.emailed, False) @@ -364,7 +364,7 @@ class TestWeb(TestCase): 'get_node_down' : True, 'node_down_grace_pd' : '', 'get_version' : True, - 'version_type' : 'UNRECOMMENDED', + 'version_type' : 'OBSOLETE', 'get_band_low': True, 'band_low_threshold' : '', 'get_t_shirt' : True}, diff --git a/weather/weatherapp/updaters.py b/weather/weatherapp/updaters.py index 586f2d8..9042ec5 100644 --- a/weather/weatherapp/updaters.py +++ b/weather/weatherapp/updaters.py @@ -195,8 +195,7 @@ def check_version(ctl_util, email_list): str(sub.subscriber.router.fingerprint)) if version_type != 'ERROR': - if (version_type == 'OBSOLETE' or sub.notify_type == \ - version_type): + if (version_type == 'OBSOLETE'): if sub.emailed == False: fingerprint = sub.subscriber.router.fingerprint @@ -312,12 +311,12 @@ def update_all_routers(ctl_util, email_list): #send a welcome email if indicated if router_data.welcomed == False and ctl_util.is_stable(finger): - address = ctl_util.get_email(finger) + recipient = ctl_util.get_email(finger) # Don't spam people for now XXX - #address = "kaner@strace.org" + #recipient = "kaner@strace.org" is_exit = ctl_util.is_exit(finger) if not address == "": - email = emails.welcome_tuple(address, finger, name, is_exit) + email = emails.welcome_tuple(recipient, finger, name, is_exit) email_list.append(email) router_data.welcomed = True