14 files changed, 155 insertions, 55 deletions

diff --git a/src/mailman/rest/addresses.py b/src/mailman/rest/addresses.py
index ca00938ba..9dbbf3fc8 100644
--- a/src/mailman/rest/addresses.py
+++ b/src/mailman/rest/addresses.py
@@ -29,7 +29,7 @@ from mailman.interfaces.address import (
 from mailman.interfaces.usermanager import IUserManager
 from mailman.rest.helpers import (
     BadRequest, CollectionMixin, NotFound, bad_request, child, created, etag,
-    no_content, not_found, okay, path_to)
+    no_content, not_found, okay)
 from mailman.rest.members import MemberCollection
 from mailman.rest.preferences import Preferences
 from mailman.rest.validator import Validator
@@ -51,7 +51,7 @@ class _AddressBase(CollectionMixin):
             email=address.email,
             original_email=address.original_email,
             registered_on=address.registered_on,
-            self_link=path_to('addresses/{0}'.format(address.email)),
+            self_link=self.path_to('addresses/{0}'.format(address.email)),
             )
         # Add optional attributes.  These can be None or the empty string.
         if address.display_name:
@@ -59,7 +59,7 @@ class _AddressBase(CollectionMixin):
         if address.verified_on:
             representation['verified_on'] = address.verified_on
         if address.user:
-            representation['user'] = path_to(
+            representation['user'] = self.path_to(
                 'users/{0}'.format(address.user.user_id.int))
         return representation
 
@@ -178,7 +178,7 @@ class UserAddresses(_AddressBase):
 
     def __init__(self, user):
         self._user = user
-        super(UserAddresses, self).__init__()
+        super().__init__()
 
     def _get_collection(self, request):
         """See `CollectionMixin`."""
@@ -215,7 +215,8 @@ class UserAddresses(_AddressBase):
         else:
             # Link the address to the current user and return it.
             address.user = self._user
-            created(response, path_to('addresses/{0}'.format(address.email)))
+            location = self.path_to('addresses/{0}'.format(address.email))
+            created(response, location)
 
 
 
@@ -228,7 +229,7 @@ class AddressMemberships(MemberCollection):
     """All the memberships of a particular email address."""
 
     def __init__(self, address):
-        super(AddressMemberships, self).__init__()
+        super().__init__()
         self._address = address
 
     def _get_collection(self, request):
diff --git a/src/mailman/rest/docs/basic.rst b/src/mailman/rest/docs/basic.rst
index 5efa2526d..45ace87ec 100644
--- a/src/mailman/rest/docs/basic.rst
+++ b/src/mailman/rest/docs/basic.rst
@@ -42,6 +42,7 @@ System version information can be retrieved from the server, in the form of a
 JSON encoded response.
 
     >>> dump_json('http://localhost:9001/3.0/system/versions')
+    api_version: 3.0
     http_etag: "..."
     mailman_version: GNU Mailman 3...
     python_version: ...
diff --git a/src/mailman/rest/docs/helpers.rst b/src/mailman/rest/docs/helpers.rst
index c40619c01..ccd09a061 100644
--- a/src/mailman/rest/docs/helpers.rst
+++ b/src/mailman/rest/docs/helpers.rst
@@ -10,14 +10,15 @@ Resource paths
 
 For example, most resources don't have to worry about where they are rooted.
 They only need to know where they are relative to the root URI, and this
-function can return them the full path to the resource.
+function can return them the full path to the resource.  We have to pass in
+the REST API version because there is no request in flight.
 
     >>> from mailman.rest.helpers import path_to
-    >>> print(path_to('system'))
+    >>> print(path_to('system', '3.0'))
     http://localhost:9001/3.0/system
 
-Parameters like the ``scheme``, ``host``, ``port``, and API version number can
-be set in the configuration file.
+Parameters like the ``scheme``, ``host``, and ``port`` can be set in the
+configuration file.
 ::
 
     >>> config.push('helpers', """
@@ -25,11 +26,10 @@ be set in the configuration file.
     ... hostname: geddy
     ... port: 2112
     ... use_https: yes
-    ... api_version: 4.2
     ... """)
     >>> cleanups.append((config.pop, 'helpers'))
 
-    >>> print(path_to('system'))
+    >>> print(path_to('system', '4.2'))
     https://geddy:2112/4.2/system
 
 
diff --git a/src/mailman/rest/domains.py b/src/mailman/rest/domains.py
index bf6fc5ca5..751dfb407 100644
--- a/src/mailman/rest/domains.py
+++ b/src/mailman/rest/domains.py
@@ -27,7 +27,7 @@ from mailman.interfaces.domain import (
     BadDomainSpecificationError, IDomainManager)
 from mailman.rest.helpers import (
     BadRequest, CollectionMixin, NotFound, bad_request, child, created, etag,
-    no_content, not_found, okay, path_to)
+    no_content, not_found, okay)
 from mailman.rest.lists import ListsForDomain
 from mailman.rest.users import OwnersForDomain
 from mailman.rest.validator import Validator, list_of_strings_validator
@@ -44,7 +44,7 @@ class _DomainBase(CollectionMixin):
             base_url=domain.base_url,
             description=domain.description,
             mail_host=domain.mail_host,
-            self_link=path_to('domains/{0}'.format(domain.mail_host)),
+            self_link=self.path_to('domains/{0}'.format(domain.mail_host)),
             url_host=domain.url_host,
             )
 
@@ -125,7 +125,8 @@ class AllDomains(_DomainBase):
         except ValueError as error:
             bad_request(response, str(error))
         else:
-            created(response, path_to('domains/{0}'.format(domain.mail_host)))
+            location = self.path_to('domains/{0}'.format(domain.mail_host))
+            created(response, location)
 
     def on_get(self, request, response):
         """/domains"""
diff --git a/src/mailman/rest/helpers.py b/src/mailman/rest/helpers.py
index c737fcbc7..84aa3663b 100644
--- a/src/mailman/rest/helpers.py
+++ b/src/mailman/rest/helpers.py
@@ -47,12 +47,13 @@ from pprint import pformat
 
 
 
-def path_to(resource):
+def path_to(resource, api_version):
     """Return the url path to a resource.
 
     :param resource: The canonical path to the resource, relative to the
         system base URI.
     :type resource: string
+    :param api_version: API version to report.
     :return: The full path to the resource.
     :rtype: bytes
     """
@@ -60,7 +61,7 @@ def path_to(resource):
         ('https' if as_boolean(config.webservice.use_https) else 'http'),
         config.webservice.hostname,
         config.webservice.port,
-        config.webservice.api_version,
+        api_version,
         (resource[1:] if resource.startswith('/') else resource),
         )
 
@@ -185,6 +186,9 @@ class CollectionMixin:
                 entries=entries,
                 )
 
+    def path_to(self, resource):
+        return path_to(resource, self.api_version)
+
 
 
 class GetterSetter:
diff --git a/src/mailman/rest/lists.py b/src/mailman/rest/lists.py
index 0607102cb..3409a3a00 100644
--- a/src/mailman/rest/lists.py
+++ b/src/mailman/rest/lists.py
@@ -40,7 +40,7 @@ from mailman.interfaces.subscriptions import ISubscriptionService
 from mailman.rest.listconf import ListConfiguration
 from mailman.rest.helpers import (
     CollectionMixin, GetterSetter, NotFound, bad_request, child, created,
-    etag, no_content, not_found, okay, paginate, path_to)
+    etag, no_content, not_found, okay, paginate)
 from mailman.rest.members import AMember, MemberCollection
 from mailman.rest.post_moderation import HeldMessages
 from mailman.rest.sub_moderation import SubscriptionRequests
@@ -109,7 +109,7 @@ class _ListBase(CollectionMixin):
             mail_host=mlist.mail_host,
             member_count=mlist.members.member_count,
             volume=mlist.volume,
-            self_link=path_to('lists/{0}'.format(mlist.list_id)),
+            self_link=self.path_to('lists/{0}'.format(mlist.list_id)),
             )
 
     @paginate
@@ -213,7 +213,8 @@ class AllLists(_ListBase):
         except ValueError as error:
             bad_request(response, str(error))
         else:
-            created(response, path_to('lists/{0}'.format(mlist.list_id)))
+            location = self.path_to('lists/{0}'.format(mlist.list_id))
+            created(response, location)
 
     def on_get(self, request, response):
         """/lists"""
@@ -234,7 +235,8 @@ class MembersOfList(MemberCollection):
     def _get_collection(self, request):
         """See `CollectionMixin`."""
         # Overrides _MemberBase._get_collection() because we only want to
-        # return the members from the requested roster.
+        # return the members from the requested roster.  Don't call super()
+        # but be sure to set the request object.
         roster = self._mlist.get_roster(self._role)
         address_of_member = attrgetter('address.email')
         return list(sorted(roster.members, key=address_of_member))
diff --git a/src/mailman/rest/members.py b/src/mailman/rest/members.py
index d6a57a673..41cf56e3c 100644
--- a/src/mailman/rest/members.py
+++ b/src/mailman/rest/members.py
@@ -38,7 +38,7 @@ from mailman.interfaces.user import IUser, UnverifiedAddressError
 from mailman.interfaces.usermanager import IUserManager
 from mailman.rest.helpers import (
     CollectionMixin, NotFound, accepted, bad_request, child, conflict,
-    created, etag, no_content, not_found, okay, paginate, path_to)
+    created, etag, no_content, not_found, okay, paginate)
 from mailman.rest.preferences import Preferences, ReadOnlyPreferences
 from mailman.rest.validator import (
     Validator, enum_validator, subscriber_validator)
@@ -65,15 +65,16 @@ class _MemberBase(CollectionMixin):
             list_id=member.list_id,
             email=member.address.email,
             role=role,
-            address=path_to('addresses/{}'.format(member.address.email)),
-            self_link=path_to('members/{}'.format(member_id)),
+            address=self.path_to('addresses/{}'.format(member.address.email)),
+            self_link=self.path_to('members/{}'.format(member_id)),
             delivery_mode=member.delivery_mode,
             member_id=member_id,
             )
         # Add the user link if there is one.
         user = member.user
         if user is not None:
-            response['user'] = path_to('users/{}'.format(user.user_id.int))
+            response['user'] = self.path_to(
+                'users/{}'.format(user.user_id.int))
         return response
 
     @paginate
@@ -277,7 +278,7 @@ class AllMembers(_MemberBase):
                 # UUIDs and need to be converted to URLs because JSON doesn't
                 # directly support UUIDs.
                 member_id = member.member_id.int
-                location = path_to('members/{0}'.format(member_id))
+                location = self.path_to('members/{0}'.format(member_id))
                 created(response, location)
                 return
             # The member could not be directly subscribed because there are
@@ -327,9 +328,8 @@ class AllMembers(_MemberBase):
         # UUIDs and need to be converted to URLs because JSON doesn't
         # directly support UUIDs.
         member_id = member.member_id.int
-        location = path_to('members/{0}'.format(member_id))
+        location = self.path_to('members/{0}'.format(member_id))
         created(response, location)
-        return
 
     def on_get(self, request, response):
         """/members"""
@@ -341,9 +341,10 @@ class AllMembers(_MemberBase):
 class _FoundMembers(MemberCollection):
     """The found members collection."""
 
-    def __init__(self, members):
-        super(_FoundMembers, self).__init__()
+    def __init__(self, members, api_version):
+        super().__init__()
         self._members = members
+        self.api_version = api_version
 
     def _get_collection(self, request):
         """See `CollectionMixin`."""
@@ -367,5 +368,5 @@ class FindMembers(_MemberBase):
         except ValueError as error:
             bad_request(response, str(error))
         else:
-            resource = _FoundMembers(members)._make_collection(request)
-            okay(response, etag(resource))
+            resource = _FoundMembers(members, self.api_version)
+            okay(response, etag(resource._make_collection(request)))
diff --git a/src/mailman/rest/post_moderation.py b/src/mailman/rest/post_moderation.py
index 6156fa39f..75520904c 100644
--- a/src/mailman/rest/post_moderation.py
+++ b/src/mailman/rest/post_moderation.py
@@ -134,7 +134,6 @@ class HeldMessages(_HeldMessageBase, CollectionMixin):
 
     def __init__(self, mlist):
         self._mlist = mlist
-        self._requests = None
 
     def _resource_as_dict(self, request):
         """See `CollectionMixin`."""
@@ -142,7 +141,6 @@ class HeldMessages(_HeldMessageBase, CollectionMixin):
 
     def _get_collection(self, request):
         requests = IListRequests(self._mlist)
-        self._requests = requests
         return list(requests.of_type(RequestType.held_message))
 
     def on_get(self, request, response):
diff --git a/src/mailman/rest/preferences.py b/src/mailman/rest/preferences.py
index f54e62a45..8e13fd443 100644
--- a/src/mailman/rest/preferences.py
+++ b/src/mailman/rest/preferences.py
@@ -65,7 +65,8 @@ class ReadOnlyPreferences:
             resource['preferred_language'] = preferred_language.code
         # Add the self link.
         resource['self_link'] = path_to(
-            '{0}/preferences'.format(self._base_url))
+            '{0}/preferences'.format(self._base_url),
+            self.api_version)
         okay(response, etag(resource))
 
 
diff --git a/src/mailman/rest/queues.py b/src/mailman/rest/queues.py
index 190f9091e..de0747b84 100644
--- a/src/mailman/rest/queues.py
+++ b/src/mailman/rest/queues.py
@@ -29,7 +29,7 @@ from mailman.app.inject import inject_text
 from mailman.interfaces.listmanager import IListManager
 from mailman.rest.helpers import (
     CollectionMixin, bad_request, created, etag, no_content, not_found, okay,
-    paginate, path_to)
+    paginate)
 from mailman.rest.validator import Validator
 from zope.component import getUtility
 
@@ -47,7 +47,7 @@ class _QueuesBase(CollectionMixin):
             directory=switchboard.queue_directory,
             count=len(files),
             files=files,
-            self_link=path_to('queues/{}'.format(name)),
+            self_link=self.path_to('queues/{}'.format(name)),
             )
 
     @paginate
@@ -91,7 +91,8 @@ class AQueue(_QueuesBase):
             bad_request(response, str(error))
             return
         else:
-            location = path_to('queues/{}/{}'.format(self._name, filebase))
+            location = self.path_to(
+                'queues/{}/{}'.format(self._name, filebase))
             created(response, location)
 
 
@@ -123,5 +124,5 @@ class AllQueues(_QueuesBase):
     def on_get(self, request, response):
         """<api>/queues"""
         resource = self._make_collection(request)
-        resource['self_link'] = path_to('queues')
+        resource['self_link'] = self.path_to('queues')
         okay(response, etag(resource))
diff --git a/src/mailman/rest/root.py b/src/mailman/rest/root.py
index 9ec84da68..ca8bbc4a9 100644
--- a/src/mailman/rest/root.py
+++ b/src/mailman/rest/root.py
@@ -56,8 +56,22 @@ class Root:
     always be the case though.
     """
 
-    @child(config.webservice.api_version)
-    def api_version(self, request, segments):
+    @child('3.0')
+    def api_version_30(self, request, segments):
+        # API version 3.0 was introduced in Mailman 3.0.
+        request.context['api_version'] = '3.0'
+        return self._check_authorization(request, segments)
+
+    @child('3.1')
+    def api_version_31(self, request, segments):
+        # API version 3.1 was introduced in Mailman 3.1.  Primary backward
+        # incompatible difference is that uuids are represented as hex strings
+        # instead of 128 bit integers.  The latter is not compatible with all
+        # versions of JavaScript.
+        request.context['api_version'] = '3.1'
+        return self._check_authorization(request, segments)
+
+    def _check_authorization(self, request, segments):
         # We have to do this here instead of in a @falcon.before() handler
         # because those handlers are not compatible with our custom traversal
         # logic.  Specifically, falcon's before/after handlers will call the
@@ -88,7 +102,8 @@ class Versions:
         resource = dict(
             mailman_version=system.mailman_version,
             python_version=system.python_version,
-            self_link=path_to('system/versions'),
+            api_version=self.api_version,
+            self_link=path_to('system/versions', self.api_version),
             )
         okay(response, etag(resource))
 
diff --git a/src/mailman/rest/tests/test_api.py b/src/mailman/rest/tests/test_api.py
new file mode 100644
index 000000000..d5d48bdc2
--- /dev/null
+++ b/src/mailman/rest/tests/test_api.py
@@ -0,0 +1,61 @@
+# Copyright (C) 2015 by the Free Software Foundation, Inc.
+#
+# This file is part of GNU Mailman.
+#
+# GNU Mailman is free software: you can redistribute it and/or modify it under
+# the terms of the GNU General Public License as published by the Free
+# Software Foundation, either version 3 of the License, or (at your option)
+# any later version.
+#
+# GNU Mailman is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+# FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+# more details.
+#
+# You should have received a copy of the GNU General Public License along with
+# GNU Mailman.  If not, see <http://www.gnu.org/licenses/>.
+
+"""API version tests."""
+
+__all__ = [
+    'TestAPIVersion',
+    ]
+
+
+import unittest
+
+from mailman.core.system import system
+from mailman.testing.helpers import call_api
+from mailman.testing.layers import RESTLayer
+from urllib.error import HTTPError
+
+
+
+class TestAPIVersion(unittest.TestCase):
+    layer = RESTLayer
+
+    def test_api_31(self):
+        # API version 3.1 was introduced in Mailman 3.1.
+        url = 'http://localhost:9001/3.1/system'
+        new = '{}/versions'.format(url)
+        json, response = call_api(url)
+        self.assertEqual(json['mailman_version'], system.mailman_version)
+        self.assertEqual(json['python_version'], system.python_version)
+        self.assertEqual(json['api_version'], '3.1')
+        self.assertEqual(json['self_link'], new)
+
+    def test_api_30(self):
+        # API version 3.0 is still supported.
+        url = 'http://localhost:9001/3.0/system'
+        new = '{}/versions'.format(url)
+        json, response = call_api(url)
+        self.assertEqual(json['mailman_version'], system.mailman_version)
+        self.assertEqual(json['python_version'], system.python_version)
+        self.assertEqual(json['api_version'], '3.0')
+        self.assertEqual(json['self_link'], new)
+
+    def test_bad_api(self):
+        # There is no API version earlier than 3.0.
+        with self.assertRaises(HTTPError) as cm:
+            call_api('http://localhost:9001/2.9/system')
+        self.assertEqual(cm.exception.code, 404)
diff --git a/src/mailman/rest/users.py b/src/mailman/rest/users.py
index da18d2ab3..252e7757b 100644
--- a/src/mailman/rest/users.py
+++ b/src/mailman/rest/users.py
@@ -89,7 +89,7 @@ CREATION_FIELDS = dict(
 
 
 
-def create_user(arguments, response):
+def create_user(arguments, request, response):
     """Create a new user."""
     # We can't pass the 'password' argument to the user creation method, so
     # strip that out (if it exists), then create the user, adding the password
@@ -118,7 +118,8 @@ def create_user(arguments, response):
         password = generate(int(config.passwords.password_length))
     user.password = config.password_context.encrypt(password)
     user.is_server_owner = is_server_owner
-    location = path_to('users/{}'.format(user.user_id.int))
+    location = path_to('users/{}'.format(user.user_id.int),
+                       request.context['api_version'])
     created(response, location)
     return user
 
@@ -137,7 +138,7 @@ class _UserBase(CollectionMixin):
         resource = dict(
             created_on=user.created_on,
             is_server_owner=user.is_server_owner,
-            self_link=path_to('users/{}'.format(user_id)),
+            self_link=self.path_to('users/{}'.format(user_id)),
             user_id=user_id,
         )
         # Add the password attribute, only if the user has a password.  Same
@@ -171,7 +172,7 @@ class AllUsers(_UserBase):
         except ValueError as error:
             bad_request(response, str(error))
             return
-        create_user(arguments, response)
+        create_user(arguments, request, response)
 
 
 
@@ -345,7 +346,7 @@ class AddressUser(_UserBase):
             auto_create = arguments.pop('auto_create', True)
             if auto_create:
                 # This sets the 201 or 400 status.
-                user = create_user(arguments, response)
+                user = create_user(arguments, request, response)
                 if user is None:
                     return
             else:
@@ -378,7 +379,7 @@ class AddressUser(_UserBase):
                 return
             okay(response)
         else:
-            user = create_user(arguments, response)
+            user = create_user(arguments, request, response)
             if user is None:
                 return
         user.link(self._address)
diff --git a/src/mailman/rest/wsgiapp.py b/src/mailman/rest/wsgiapp.py
index 15a4cf60a..035125864 100644
--- a/src/mailman/rest/wsgiapp.py
+++ b/src/mailman/rest/wsgiapp.py
@@ -50,19 +50,32 @@ class AdminWebServiceWSGIRequestHandler(WSGIRequestHandler):
         log.info('%s - - %s', self.address_string(), format % args)
 
 
+class SetAPIVersion:
+    """Falcon middleware object that sets the api_version on resources."""
+    def process_resource(self, request, response, resource):
+        # Set this attribute on the resource right before it is dispatched
+        # too.  This can be used by the resource to provide different
+        # responses based on the API version, and for path_to() to provide an
+        # API version-specific path.
+        #
+        # Note that it's possible that resource is None, e.g. such as when a
+        # resource path does not exist.  This middleware method will still get
+        # called, but there's nothing to set the api_version on.
+        if resource is not None:
+            resource.api_version = request.context.get('api_version')
+
+
 class RootedAPI(API):
     def __init__(self, root, *args, **kws):
         self._root = root
-        super(RootedAPI, self).__init__(*args, **kws)
+        super().__init__(*args, middleware=SetAPIVersion(), **kws)
 
     @transactional
     def __call__(self, environ, start_response):
-        # The only difference between this and the super class's wsgi API is
-        # that this wraps a transactional handler around the call.  If an
-        # error occurs, the current transaction is aborted, otherwise it is
-        # committed.
-        return super(RootedAPI, self).__call__(
-            environ, start_response)
+        # Override the base class implementation to wrap a transactional
+        # handler around the call, such that the current transaction is
+        # committed if no errors occur, and aborted otherwise.
+        return super().__call__(environ, start_response)
 
     def _get_responder(self, req):
         path = req.path