6 files changed, 125 insertions, 31 deletions

diff --git a/src/mailman/interfaces/membership.py b/src/mailman/interfaces/membership.py
index ec5f9ea69..233d248fd 100644
--- a/src/mailman/interfaces/membership.py
+++ b/src/mailman/interfaces/membership.py
@@ -27,6 +27,20 @@ __all__ = [
 
 from zope.interface import Interface
 
+from mailman.interfaces.errors import MailmanError
+
+
+
+class MissingUserError(MailmanError):
+    """A an invalid user id was given."""
+
+    def __init__(self, user_id):
+        super(MissingUserError, self).__init__()
+        self.user_id = user_id
+
+    def __str__(self):
+        return self.user_id
+
 
 
 class ISubscriptionService(Interface):
@@ -57,7 +71,7 @@ class ISubscriptionService(Interface):
     def __iter__():
         """See `get_members()`."""
 
-    def join(fqdn_listname, address, real_name=None, delivery_mode=None):
+    def join(fqdn_listname, subscriber, real_name=None, delivery_mode=None):
         """Subscribe to a mailing list.
 
         A user for the address is created if it is not yet known to Mailman,
@@ -85,6 +99,7 @@ class ISubscriptionService(Interface):
             the mailing list.
         :raises InvalidEmailAddressError: if the email address is not valid.
         :raises MembershipIsBannedError: if the membership is not allowed.
+        :raises MissingUserError: when a bogus user id is given.
         :raises NoSuchListError: if the named mailing list does not exist.
         :raises ValueError: when `delivery_mode` is invalid.
         """
@@ -102,4 +117,3 @@ class ISubscriptionService(Interface):
         :raises NotAMemberError: if the given address is not a member of the
             mailing list.
         """
-        
diff --git a/src/mailman/interfaces/usermanager.py b/src/mailman/interfaces/usermanager.py
index e742505d4..59895af7b 100644
--- a/src/mailman/interfaces/usermanager.py
+++ b/src/mailman/interfaces/usermanager.py
@@ -62,6 +62,15 @@ class IUserManager(Interface):
         :rtype: `IUser`.
         """
 
+    def get_user_by_id(user_id):
+        """Get the user associated with the given id.
+
+        :param user_id: The user id.
+        :type user_id: unicode
+        :return: The user found or None.
+        :rtype: `IUser`.
+        """
+
     users = Attribute(
         """An iterator over all the `IUsers` managed by this user manager.""")
 
diff --git a/src/mailman/rest/adapters.py b/src/mailman/rest/adapters.py
index 792609161..5be128d3d 100644
--- a/src/mailman/rest/adapters.py
+++ b/src/mailman/rest/adapters.py
@@ -36,7 +36,9 @@ from mailman.core.constants import system_preferences
 from mailman.interfaces.address import InvalidEmailAddressError
 from mailman.interfaces.listmanager import IListManager, NoSuchListError
 from mailman.interfaces.member import DeliveryMode
-from mailman.interfaces.membership import ISubscriptionService
+from mailman.interfaces.membership import (
+    ISubscriptionService, MissingUserError)
+from mailman.interfaces.usermanager import IUserManager
 from mailman.model.member import Member
 from mailman.utilities.passwords import make_user_friendly_password
 
@@ -81,7 +83,7 @@ class SubscriptionService:
         for member in self.get_members():
             yield member
 
-    def join(self, fqdn_listname, address,
+    def join(self, fqdn_listname, subscriber,
              real_name= None, delivery_mode=None):
         """See `ISubscriptionService`."""
         mlist = getUtility(IListManager).get(fqdn_listname)
@@ -91,20 +93,29 @@ class SubscriptionService:
         mode = (DeliveryMode.regular
                 if delivery_mode is None
                 else delivery_mode)
-        if real_name is None:
-            real_name, at, domain = address.partition('@')
-            if len(at) == 0:
-                # It can't possibly be a valid email address.
-                raise InvalidEmailAddressError(address)
-        # Because we want to keep the REST API simple, there is no password or
-        # language given to us.  We'll use the system's default language for
-        # the user's default language.  We'll set the password to a system
-        # default.  This will have to get reset since it can't be retrieved.
-        # Note that none of these are used unless the address is completely
-        # new to us.
-        password = make_user_friendly_password()
-        return add_member(mlist, address, real_name, password, mode,
-                          system_preferences.preferred_language)
+        # Is the subscriber a user or email address?
+        if '@' in subscriber:
+            # It's an email address, so we'll want a real name.
+            if real_name is None:
+                real_name, at, domain = subscriber.partition('@')
+                if len(at) == 0:
+                    # It can't possibly be a valid email address.
+                    raise InvalidEmailAddressError(subscriber)
+            # Because we want to keep the REST API simple, there is no
+            # password or language given to us.  We'll use the system's
+            # default language for the user's default language.  We'll set the
+            # password to a system default.  This will have to get reset since
+            # it can't be retrieved.  Note that none of these are used unless
+            # the address is completely new to us.
+            password = make_user_friendly_password()
+            return add_member(mlist, subscriber, real_name, password, mode,
+                              system_preferences.preferred_language)
+        else:
+            # We have to assume it's a user id.
+            user = getUtility(IUserManager).get_user_by_id(subscriber)
+            if user is None:
+                raise MissingUserError(subscriber)
+            return mlist.subscribe(user)
 
     def leave(self, fqdn_listname, address):
         """See `ISubscriptionService`."""
diff --git a/src/mailman/rest/docs/membership.txt b/src/mailman/rest/docs/membership.txt
index 70f7fc357..493772492 100644
--- a/src/mailman/rest/docs/membership.txt
+++ b/src/mailman/rest/docs/membership.txt
@@ -262,13 +262,13 @@ A user can be subscribed to a mailing list via the REST API, either by a
 specific address, or more generally by their preferred address.  A subscribed
 user is called a member.
 
-Elly subscribes to the alpha mailing list.  By default, get gets a regular
-delivery.  Since Elly's email address is not yet known to Mailman, a user is
-created for her.
+Elly wants to subscribes to the alpha mailing list.  Since Elly's email
+address is not yet known to Mailman, a user is created for her.  By default,
+get gets a regular delivery.
 
     >>> dump_json('http://localhost:9001/3.0/members', {
     ...           'fqdn_listname': 'alpha@example.com',
-    ...           'address': 'eperson@example.com',
+    ...           'subscriber': 'eperson@example.com',
     ...           'real_name': 'Elly Person',
     ...           })
     content-length: 0
@@ -277,7 +277,7 @@ created for her.
     server: ...
     status: 201
 
-Elly is now a member of the mailing list.
+Elly is now a known user, and a member of the mailing list.
 ::
 
     >>> elly = user_manager.get_user('eperson@example.com')
@@ -299,6 +299,66 @@ Elly is now a member of the mailing list.
         user: http://localhost:9001/3.0/users/5
     ...
 
+Gwen is a user with a preferred address.  She subscribes to the alpha mailing
+list with her preferred address.
+
+    >>> from mailman.utilities.datetime import now
+    >>> gwen = user_manager.create_user('gwen@example.com', 'Gwen Person')
+    >>> preferred = list(gwen.addresses)[0]
+    >>> preferred.verified_on = now()
+    >>> gwen.preferred_address = preferred
+
+    # Note that we must extract the user id before we commit the transaction.
+    # This is because accessing the .user_id attribute will lock the database
+    # in the testing process, breaking the REST queue process.
+    >>> user_id = gwen.user_id
+    >>> transaction.commit()
+
+    >>> dump_json('http://localhost:9001/3.0/members', {
+    ...     'fqdn_listname': 'alpha@example.com',
+    ...     'subscriber': user_id,
+    ...     })
+    content-length: 0
+    date: ...
+    location: http://localhost:9001/3.0/members/9
+    server: ...
+    status: 201
+
+    >>> dump_json('http://localhost:9001/3.0/members')
+    entry 0:
+    ...
+    entry 4:
+        address: gwen@example.com
+        fqdn_listname: alpha@example.com
+        http_etag: "..."
+        role: member
+        self_link: http://localhost:9001/3.0/members/9
+        user: http://localhost:9001/3.0/users/6
+    ...
+    total_size: 9
+
+When Gwen changes her preferred address, her subscription automatically tracks
+the new address.
+::
+
+    >>> new_preferred = gwen.register('gwen.person@example.com')
+    >>> new_preferred.verified_on = now()
+    >>> gwen.preferred_address = new_preferred
+    >>> transaction.commit()
+
+    >>> dump_json('http://localhost:9001/3.0/members')
+    entry 0:
+    ...
+    entry 4:
+        address: gwen.person@example.com
+        fqdn_listname: alpha@example.com
+        http_etag: "..."
+        role: member
+        self_link: http://localhost:9001/3.0/members/9
+        user: http://localhost:9001/3.0/users/6
+    ...
+    total_size: 9
+
 
 Leaving a mailing list
 ======================
@@ -330,13 +390,13 @@ Fred joins the alpha mailing list but wants MIME digest delivery.
     >>> transaction.abort()
     >>> dump_json('http://localhost:9001/3.0/members', {
     ...           'fqdn_listname': 'alpha@example.com',
-    ...           'address': 'fperson@example.com',
+    ...           'subscriber': 'fperson@example.com',
     ...           'real_name': 'Fred Person',
     ...           'delivery_mode': 'mime_digests',
     ...           })
     content-length: 0
     date: ...
-    location: http://localhost:9001/3.0/members/9
+    location: http://localhost:9001/3.0/members/10
     server: ...
     status: 201
 
diff --git a/src/mailman/rest/members.py b/src/mailman/rest/members.py
index 1f6f1a913..06d917d11 100644
--- a/src/mailman/rest/members.py
+++ b/src/mailman/rest/members.py
@@ -102,10 +102,10 @@ class AllMembers(_MemberBase):
         service = getUtility(ISubscriptionService)
         try:
             validator = Validator(fqdn_listname=unicode,
-                                  address=unicode,
+                                  subscriber=unicode,
                                   real_name=unicode,
                                   delivery_mode=enum_validator(DeliveryMode),
-                                  _optional=('real_name', 'delivery_mode'))
+                                  _optional=('delivery_mode', 'real_name'))
             member = service.join(**validator(request))
         except AlreadySubscribedError:
             return http.conflict([], b'Member already subscribed')
diff --git a/src/mailman/rest/tests/test_membership.py b/src/mailman/rest/tests/test_membership.py
index 7f6d5c237..7f5c8fd38 100644
--- a/src/mailman/rest/tests/test_membership.py
+++ b/src/mailman/rest/tests/test_membership.py
@@ -53,7 +53,7 @@ class TestMembership(unittest.TestCase):
             # For Python 2.6.
             call_api('http://localhost:9001/3.0/members', {
                 'fqdn_listname': 'missing@example.com',
-                'address': 'nobody@example.com',
+                'subscriber': 'nobody@example.com',
                 })
         except HTTPError as exc:
             self.assertEqual(exc.code, 400)
@@ -112,7 +112,7 @@ class TestMembership(unittest.TestCase):
             # For Python 2.6.
             call_api('http://localhost:9001/3.0/members', {
                 'fqdn_listname': 'test@example.com',
-                'address': 'anne@example.com',
+                'subscriber': 'anne@example.com',
                 })
         except HTTPError as exc:
             self.assertEqual(exc.code, 409)
@@ -124,7 +124,7 @@ class TestMembership(unittest.TestCase):
         try:
             call_api('http://localhost:9001/3.0/members', {
                 'fqdn_listname': 'test@example.com',
-                'address': 'anne@example.com',
+                'subscriber': 'anne@example.com',
                 'real_name': 'Anne Person',
                 'delivery_mode': 'invalid-mode',
                 })
@@ -138,7 +138,7 @@ class TestMembership(unittest.TestCase):
     def test_join_email_contains_slash(self):
         content, response = call_api('http://localhost:9001/3.0/members', {
             'fqdn_listname': 'test@example.com',
-            'address': 'hugh/person@example.com',
+            'subscriber': 'hugh/person@example.com',
             'real_name': 'Hugh Person',
             })
         self.assertEqual(content, None)