Move AlreadySubscribedError to interfaces/member.py in the (new) preference

for putting exceptions close to the interfaces where they are used.

MakeRandomPassword() returns unicodes now.

Flesh out bin/add_members.py and bin/list_members.py

2 files changed, 137 insertions, 152 deletions

diff --git a/mailman/bin/add_members.py b/mailman/bin/add_members.py
index 44ddebc7f..f7f65b953 100644
--- a/mailman/bin/add_members.py
+++ b/mailman/bin/add_members.py
@@ -19,82 +19,69 @@ from __future__ import with_statement
 
 import os
 import sys
-import optparse
+import codecs
 
 from cStringIO import StringIO
 from email.utils import parseaddr
 
 from mailman import Errors
-from mailman import MailList
 from mailman import Message
 from mailman import Utils
 from mailman import Version
 from mailman import i18n
 from mailman.app.membership import add_member
 from mailman.configuration import config
-from mailman.initialize import initialize
-from mailman.interfaces import DeliveryMode
+from mailman.interfaces import AlreadySubscribedError, DeliveryMode
+from mailman.options import SingleMailingListOptions
 
 _ = i18n._
 
 
 
-def parseargs():
-    parser = optparse.OptionParser(version=Version.MAILMAN_VERSION,
-                                   usage=_("""\
-%prog [options] listname
+class ScriptOptions(SingleMailingListOptions):
+    usage=_("""\
+%prog [options]
 
 Add members to a list.  'listname' is the name of the Mailman list you are
 adding members to; the list must already exist.
 
 You must supply at least one of -r and -d options.  At most one of the
 files can be '-'.
-"""))
-    parser.add_option('-r', '--regular-members-file',
-                      type='string', dest='regular', help=_("""\
+""")
+
+    def add_options(self):
+        super(ScriptOptions, self).add_options()
+        self.parser.add_option(
+            '-r', '--regular-members-file',
+            type='string', dest='regular', help=_("""\
 A file containing addresses of the members to be added, one address per line.
 This list of people become non-digest members.  If file is '-', read addresses
 from stdin."""))
-    parser.add_option('-d', '--digest-members-file',
-                      type='string', dest='digest', help=_("""\
+        self.parser.add_option(
+            '-d', '--digest-members-file',
+            type='string', dest='digest', help=_("""\
 Similar to -r, but these people become digest members."""))
-    parser.add_option('-w', '--welcome-msg',
-                      type='string', metavar='<y|n>', help=_("""\
+        self.parser.add_option(
+            '-w', '--welcome-msg',
+            type='yesno', metavar='<y|n>', help=_("""\
 Set whether or not to send the list members a welcome message, overriding
 whatever the list's 'send_welcome_msg' setting is."""))
-    parser.add_option('-a', '--admin-notify',
-                      type='string', metavar='<y|n>', help=_("""\
+        self.parser.add_option(
+            '-a', '--admin-notify',
+            type='yesno', metavar='<y|n>', help=_("""\
 Set whether or not to send the list administrators a notification on the
 success/failure of these subscriptions, overriding whatever the list's
 'admin_notify_mchanges' setting is."""))
-    parser.add_option('-C', '--config',
-                      help=_('Alternative configuration file to use'))
-    opts, args = parser.parse_args()
-    if not args:
-        parser.error(_('Missing listname'))
-    if len(args) > 1:
-        parser.error(_('Unexpected arguments'))
-    if opts.welcome_msg is not None:
-        ch = opts.welcome_msg[0].lower()
-        if ch == 'y':
-            opts.welcome_msg = True
-        elif ch == 'n':
-            opts.welcome_msg = False
-        else:
-            parser.error(_('Illegal value for -w: $opts.welcome_msg'))
-    if opts.admin_notify is not None:
-        ch = opts.admin_notify[0].lower()
-        if ch == 'y':
-            opts.admin_notify = True
-        elif ch == 'n':
-            opts.admin_notify = False
-        else:
-            parser.error(_('Illegal value for -a: $opts.admin_notify'))
-    if opts.regular is None and opts.digest is None:
-        parser.error(_('At least one of -r or -d is required'))
-    if opts.regular == '-' and opts.digest == '-':
-        parser.error(_("-r and -d cannot both be '-'"))
-    return parser, opts, args
+
+    def sanity_check(self):
+        if not self.options.listname:
+            self.parser.error(_('Missing listname'))
+        if len(self.arguments) > 0:
+            self.parser.print_error(_('Unexpected arguments'))
+        if self.options.regular is None and self.options.digest is None:
+            parser.error(_('At least one of -r or -d is required'))
+        if self.options.regular == '-' and self.options.digest == '-':
+            parser.error(_("-r and -d cannot both be '-'"))
 
 
 
@@ -102,10 +89,11 @@ def readfile(filename):
     if filename == '-':
         fp = sys.stdin
     else:
-        fp = open(filename)
+        # XXX Need to specify other encodings.
+        fp = codecs.open(filename, encoding='utf-8')
     # Strip all the lines of whitespace and discard blank lines
     try:
-        return [line.strip() for line in fp if line]
+        return set(line.strip() for line in fp if line)
     finally:
         if fp is not sys.stdin:
             fp.close()
@@ -127,70 +115,71 @@ def addall(mlist, subscribers, delivery_mode, ack, admin_notify, outfp):
     for subscriber in subscribers:
         try:
             fullname, address = parseaddr(subscriber)
+            # Watch out for the empty 8-bit string.
+            if not fullname:
+                fullname = u''
             password = Utils.MakeRandomPassword()
             add_member(mlist, address, fullname, password, delivery_mode,
                        config.DEFAULT_SERVER_LANGUAGE, ack, admin_notify)
         except AlreadySubscribedError:
             print >> tee, _('Already a member: $subscriber')
         except Errors.InvalidEmailAddress:
-            if userdesc.address == '':
+            if not address:
                 print >> tee, _('Bad/Invalid email address: blank line')
             else:
-                print >> tee, _('Bad/Invalid email address: $member')
+                print >> tee, _('Bad/Invalid email address: $subscriber')
         else:
             print >> tee, _('Subscribing: $subscriber')
 
 
 
 def main():
-    parser, opts, args = parseargs()
-    initialize(opts.config)
+    options = ScriptOptions()
+    options.initialize()
 
-    listname = args[0].lower().strip()
-    mlist = config.db.list_manager.get(listname)
+    fqdn_listname = options.options.listname
+    mlist = config.db.list_manager.get(fqdn_listname)
     if mlist is None:
-        parser.error(_('No such list: $listname'))
+        parser.error(_('No such list: $fqdn_listname'))
 
     # Set up defaults.
-    if opts.welcome_msg is None:
-        send_welcome_msg = mlist.send_welcome_msg
-    else:
-        send_welcome_msg = opts.welcome_msg
-    if opts.admin_notify is None:
-        admin_notify = mlist.admin_notify_mchanges
-    else:
-        admin_notify = opts.admin_notify
+    send_welcome_msg = (options.options.welcome_msg
+                        if options.options.welcome_msg is not None
+                        else mlist.send_welcome_msg)
+    admin_notify = (options.options.admin_notify
+                    if options.options.admin_notify is not None
+                    else mlist.admin_notify)
 
     with i18n.using_language(mlist.preferred_language):
-        if opts.digest:
-            dmembers = readfile(opts.digest)
+        if options.options.digest:
+            dmembers = readfile(options.options.digest)
         else:
-            dmembers = []
-        if opts.regular:
-            nmembers = readfile(opts.regular)
+            dmembers = set()
+        if options.options.regular:
+            nmembers = readfile(options.options.regular)
         else:
-            nmembers = []
+            nmembers = set()
 
         if not dmembers and not nmembers:
             print _('Nothing to do.')
             sys.exit(0)
 
-        s = StringIO()
+        outfp = StringIO()
         if nmembers:
             addall(mlist, nmembers, DeliveryMode.regular,
-                   send_welcome_msg, admin_notify, s)
+                   send_welcome_msg, admin_notify, outfp)
 
         if dmembers:
             addall(mlist, dmembers, DeliveryMode.mime_digests,
-                   send_welcome_msg, admin_notify, s)
+                   send_welcome_msg, admin_notify, outfp)
 
-        config.db.flush()
+        config.db.commit()
 
         if admin_notify:
             subject = _('$mlist.real_name subscription notification')
             msg = Message.UserNotification(
-                mlist.owner, mlist.no_reply_address, subject, s.getvalue(),
-                mlist.preferred_language)
+                mlist.owner, mlist.no_reply_address, subject,
+                outfp.getvalue(), mlist.preferred_language)
             msg.send(mlist)
 
 
diff --git a/mailman/bin/list_members.py b/mailman/bin/list_members.py
index 727a2df72..f13246ba9 100644
--- a/mailman/bin/list_members.py
+++ b/mailman/bin/list_members.py
@@ -16,7 +16,6 @@
 # USA.
 
 import sys
-import optparse
 
 from email.Utils import formataddr
 
@@ -25,11 +24,10 @@ from mailman import Utils
 from mailman import Version
 from mailman.configuration import config
 from mailman.i18n import _
-from mailman.initialize import initialize
 from mailman.interfaces import DeliveryStatus
+from mailman.options import SingleMailingListOptions
 
 
-ENC = sys.getdefaultencoding()
 COMMASPACE = ', '
 
 WHYCHOICES = {
@@ -43,78 +41,80 @@ KINDCHOICES = set(('mime', 'plain', 'any'))
 
 
 
-def parseargs():
-    parser = optparse.OptionParser(version=Version.MAILMAN_VERSION,
-                                   usage=_("""\
-%prog [options] listname
+class ScriptOptions(SingleMailingListOptions):
+    usage=_("""\
+%prog [options]
 
 List all the members of a mailing list.  Note that with the options below, if
 neither -r or -d is supplied, regular members are printed first, followed by
 digest members, but no indication is given as to address status.
 
-listname is the name of the mailing list to use."""))
-    parser.add_option('-o', '--output',
-                      type='string', help=_("""\
+listname is the name of the mailing list to use.""")
+
+    def add_options(self):
+        super(ScriptOptions, self).add_options()
+        self.parser.add_option(
+            '-o', '--output',
+            type='string', help=_("""\
 Write output to specified file instead of standard out."""))
-    parser.add_option('-r', '--regular',
-                      default=None, action='store_true',
-                      help=_('Print just the regular (non-digest) members.'))
-    parser.add_option('-d', '--digest',
-                      default=None, type='string', metavar='KIND',
-                      help=_("""\
+        self.parser.add_option(
+            '-r', '--regular',
+            default=None, action='store_true',
+            help=_('Print just the regular (non-digest) members.'))
+        self.parser.add_option(
+            '-d', '--digest',
+            default=None, type='string', metavar='KIND',
+            help=_("""\
 Print just the digest members.  KIND can be 'mime', 'plain', or
 'any'.  'mime' prints just the members receiving MIME digests, while 'plain'
 prints just the members receiving plain text digests.  'any' prints all
 members receiving any kind of digest."""))
-    parser.add_option('-n', '--nomail',
-                      type='string', metavar='WHY', help=_("""\
+        self.parser.add_option(
+            '-n', '--nomail',
+            type='string', metavar='WHY', help=_("""\
 Print the members that have delivery disabled.  WHY selects just the subset of
 members with delivery disabled for a particular reason, where 'any' prints all
 disabled members.  'byadmin', 'byuser', 'bybounce', and 'unknown' prints just
 the users who are disabled for that particular reason.  WHY can also be
 'enabled' which prints just those members for whom delivery is enabled."""))
-    parser.add_option('-f', '--fullnames',
-                      default=False, action='store_true',
-                      help=_('Include the full names in the output'))
-    parser.add_option('-i', '--invalid',
-                      default=False, action='store_true', help=_("""\
+        self.parser.add_option(
+            '-f', '--fullnames',
+            default=False, action='store_true',
+            help=_('Include the full names in the output'))
+        self.parser.add_option(
+            '-i', '--invalid',
+            default=False, action='store_true', help=_("""\
 Print only the addresses in the membership list that are invalid.  Ignores -r,
 -d, -n."""))
-    parser.add_option('-u', '--unicode',
-                      default=False, action='store_true', help=_("""\
-Print addresses which are stored as Unicode objects instead of normal string
-objects.  Ignores -r, -d, -n."""))
-    parser.add_option('-C', '--config',
-                      help=_('Alternative configuration file to use'))
-    opts, args = parser.parse_args()
-    if not args:
-        parser.error(_('Missing listname'))
-    if len(args) > 1:
-        parser.print_error(_('Unexpected arguments'))
-    if opts.digest is not None:
-        opts.kind = opts.digest.lower()
-        if opts.kind not in KINDCHOICES:
-            parser.error(_('Invalid value for -d: $opts.digest'))
-    if opts.nomail is not None:
-        why = opts.nomail.lower()
-        if why == 'any':
-            opts.why = 'any'
-        elif why not in WHYCHOICES:
-            parser.error(_('Invalid value for -n: $opts.nomail'))
-        opts.why = why
-    if opts.regular is None and opts.digest is None:
-        opts.regular = opts.digest = True
-        opts.kind = 'any'
-    return parser, opts, args
+
+    def sanity_check(self):
+        if not self.options.listname:
+            self.parser.error(_('Missing listname'))
+        if len(self.arguments) > 0:
+            self.parser.print_error(_('Unexpected arguments'))
+        if self.options.digest is not None:
+            self.options.kind = self.options.digest.lower()
+            if self.options.kind not in KINDCHOICES:
+                self.parser.error(
+                    _('Invalid value for -d: $self.options.digest'))
+        if self.options.nomail is not None:
+            why = self.options.nomail.lower()
+            if why == 'any':
+                self.options.why = 'any'
+            elif why not in WHYCHOICES:
+                self.parser.error(
+                    _('Invalid value for -n: $self.options.nomail'))
+            self.options.why = why
+        if self.options.regular is None and self.options.digest is None:
+            self.options.regular = self.options.digest = True
+            self.options.kind = 'any'
 
 
 
-def safe(s):
-    if not s:
+def safe(string):
+    if not string:
         return ''
-    if isinstance(s, unicode):
-        return s.encode(ENC, 'replace')
-    return unicode(s, ENC, 'replace').encode(ENC, 'replace')
+    return string.encode(sys.getdefaultencoding(), 'replace')
 
 
 def isinvalid(addr):
@@ -138,56 +138,52 @@ def whymatches(mlist, addr, why):
 
 
 def main():
-    parser, opts, args = parseargs()
-    initialize(opts.config)
+    options = ScriptOptions()
+    options.initialize()
 
-    listname = args[0].lower().strip()
-    if opts.output:
+    fqdn_listname = options.options.listname
+    if options.options.output:
         try:
             fp = open(opts.output, 'w')
         except IOError:
-            print >> sys.stderr, _(
-                'Could not open file for writing: $opts.output')
-            sys.exit(1)
+            options.parser.error(
+                _('Could not open file for writing: $options.options.output'))
     else:
         fp = sys.stdout
 
-    mlist = config.db.list_manager.get(listname)
+    mlist = config.db.list_manager.get(fqdn_listname)
     if mlist is None:
-        print >> sys.stderr, _('No such list: $listname')
-        sys.exit(1)
+        options.parser.error(_('No such list: $fqdn_listname'))
 
     # The regular delivery and digest members.
     rmembers = set(mlist.regular_members.members)
     dmembers = set(mlist.digest_members.members)
 
-    if opts.invalid or opts.unicode:
+    fullnames = options.options.fullnames
+    if options.options.invalid:
         all = sorted(member.address.address for member in rmembers + dmembers)
         for address in all:
             user = config.db.user_manager.get_user(address)
-            name = (user.real_name if opts.fullnames and user else '')
-            showit = False
-            if opts.invalid and isinvalid(address):
-                showit = True
-            if opts.unicode and isinstance(address, unicode):
-                showit = True
-            if showit:
+            name = (user.real_name if fullnames and user else u'')
+            if options.options.invalid and isinvalid(address):
                 print >> fp, formataddr((safe(name), address))
         return
-    if opts.regular:
+    if options.options.regular:
         for address in sorted(member.address.address for member in rmembers):
             user = config.db.user_manager.get_user(address)
-            name = (user.real_name if opts.fullnames and user else '')
+            name = (user.real_name if fullnames and user else u'')
             # Filter out nomails
-            if opts.nomail and not whymatches(mlist, address, opts.why):
+            if (options.options.nomail and
+                not whymatches(mlist, address, options.options.why)):
                 continue
             print >> fp, formataddr((safe(name), address))
-    if opts.digest:
+    if options.options.digest:
         for address in sorted(member.address.address for member in dmembers):
             user = config.db.user_manager.get_user(address)
-            name = (user.real_name if opts.fullnames and user else '')
+            name = (user.real_name if fullnames and user else u'')
             # Filter out nomails
-            if opts.nomail and not whymatches(mlist, address, opts.why):
+            if (options.options.nomail and
+                not whymatches(mlist, address, options.options.why)):
                 continue
             # Filter out digest kinds
 ##             if mlist.getMemberOption(addr, config.DisableMime):