Refactor API differences into a separate class.

We now have an IAPI interface which defines methods to convert to/from
UUIDs to their REST representations, and to calculate the API-homed full
URL path to a resource.  Add implementations API30 and API31 to handle
the two different implementations so far.  This also simplifies the
various path_to() calls.

Also: Add support for diff_cover to tox.ini to check that all
differences against the master branch have full test coverage.

14 files changed, 45 insertions, 140 deletions

diff --git a/src/mailman/rest/addresses.py b/src/mailman/rest/addresses.py
index 83ff87454..421e17bf1 100644
--- a/src/mailman/rest/addresses.py
+++ b/src/mailman/rest/addresses.py
@@ -51,7 +51,7 @@ class _AddressBase(CollectionMixin):
             email=address.email,
             original_email=address.original_email,
             registered_on=address.registered_on,
-            self_link=self.path_to('addresses/{}'.format(address.email)),
+            self_link=self.api.path_to('addresses/{}'.format(address.email)),
             )
         # Add optional attributes.  These can be None or the empty string.
         if address.display_name:
@@ -60,7 +60,7 @@ class _AddressBase(CollectionMixin):
             representation['verified_on'] = address.verified_on
         if address.user:
             uid = self.api.from_uuid(address.user.user_id)
-            representation['user'] = self.path_to('users/{}'.format(uid))
+            representation['user'] = self.api.path_to('users/{}'.format(uid))
         return representation
 
     def _get_collection(self, request):
@@ -213,14 +213,15 @@ class UserAddresses(_AddressBase):
             address = user_manager.get_address(validator(request)['email'])
             if address.user is None:
                 address.user = self._user
-                location = self.path_to('addresses/{}'.format(address.email))
+                location = self.api.path_to(
+                    'addresses/{}'.format(address.email))
                 created(response, location)
             else:
                 bad_request(response, 'Address belongs to other user.')
         else:
             # Link the address to the current user and return it.
             address.user = self._user
-            location = self.path_to('addresses/{}'.format(address.email))
+            location = self.api.path_to('addresses/{}'.format(address.email))
             created(response, location)
 
 
diff --git a/src/mailman/rest/api.py b/src/mailman/rest/api.py
deleted file mode 100644
index 998fac268..000000000
--- a/src/mailman/rest/api.py
+++ /dev/null
@@ -1,54 +0,0 @@
-# Copyright (C) 2016 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/>.
-
-"""REST web service API contexts."""
-
-__all__ = [
-    'API30',
-    'API31',
-    ]
-
-
-from mailman.interfaces.api import IAPI
-from uuid import UUID
-from zope.interface import implementer
-
-
-@implementer(IAPI)
-class API30:
-    version = '3.0'
-
-    @staticmethod
-    def from_uuid(uuid):
-        return uuid.int
-
-    @staticmethod
-    def to_uuid(uuid_repr):
-        return UUID(int=int(uuid_repr))
-
-
-@implementer(IAPI)
-class API31:
-    version = '3.1'
-
-    @staticmethod
-    def from_uuid(uuid):
-        return uuid.hex
-
-    @staticmethod
-    def to_uuid(uuid_repr):
-        return UUID(hex=uuid_repr)
diff --git a/src/mailman/rest/docs/helpers.rst b/src/mailman/rest/docs/helpers.rst
index ccd09a061..36870c079 100644
--- a/src/mailman/rest/docs/helpers.rst
+++ b/src/mailman/rest/docs/helpers.rst
@@ -5,34 +5,6 @@ REST API helpers
 There are a number of helpers that make building out the REST API easier.
 
 
-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.  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', '3.0'))
-    http://localhost:9001/3.0/system
-
-Parameters like the ``scheme``, ``host``, and ``port`` can be set in the
-configuration file.
-::
-
-    >>> config.push('helpers', """
-    ... [webservice]
-    ... hostname: geddy
-    ... port: 2112
-    ... use_https: yes
-    ... """)
-    >>> cleanups.append((config.pop, 'helpers'))
-
-    >>> print(path_to('system', '4.2'))
-    https://geddy:2112/4.2/system
-
-
 Etags
 =====
 
diff --git a/src/mailman/rest/domains.py b/src/mailman/rest/domains.py
index b115f63bd..0128c9da0 100644
--- a/src/mailman/rest/domains.py
+++ b/src/mailman/rest/domains.py
@@ -44,7 +44,7 @@ class _DomainBase(CollectionMixin):
             base_url=domain.base_url,
             description=domain.description,
             mail_host=domain.mail_host,
-            self_link=self.path_to('domains/{}'.format(domain.mail_host)),
+            self_link=self.api.path_to('domains/{}'.format(domain.mail_host)),
             url_host=domain.url_host,
             )
 
@@ -123,7 +123,7 @@ class AllDomains(_DomainBase):
         except BadDomainSpecificationError as error:
             bad_request(response, str(error))
         else:
-            location = self.path_to('domains/{}'.format(domain.mail_host))
+            location = self.api.path_to('domains/{}'.format(domain.mail_host))
             created(response, location)
 
     def on_get(self, request, response):
diff --git a/src/mailman/rest/helpers.py b/src/mailman/rest/helpers.py
index 425164ef7..135d950ac 100644
--- a/src/mailman/rest/helpers.py
+++ b/src/mailman/rest/helpers.py
@@ -32,7 +32,6 @@ __all__ = [
     'no_content',
     'not_found',
     'okay',
-    'path_to',
     ]
 
 
@@ -48,27 +47,6 @@ from pprint import pformat
 
 
 
-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.
-    :type api_version: string
-    :return: The full path to the resource.
-    :rtype: bytes
-    """
-    return '{0}://{1}:{2}/{3}/{4}'.format(
-        ('https' if as_boolean(config.webservice.use_https) else 'http'),
-        config.webservice.hostname,
-        config.webservice.port,
-        api_version,
-        (resource[1:] if resource.startswith('/') else resource),
-        )
-
-
-
 class ExtendedEncoder(json.JSONEncoder):
     """An extended JSON encoder which knows about other data types."""
 
@@ -185,9 +163,6 @@ class CollectionMixin:
             result['entries'] = entries
         return result
 
-    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 426c9c633..3e0c0bbca 100644
--- a/src/mailman/rest/lists.py
+++ b/src/mailman/rest/lists.py
@@ -110,7 +110,7 @@ class _ListBase(CollectionMixin):
             mail_host=mlist.mail_host,
             member_count=mlist.members.member_count,
             volume=mlist.volume,
-            self_link=self.path_to('lists/{}'.format(mlist.list_id)),
+            self_link=self.api.path_to('lists/{}'.format(mlist.list_id)),
             )
 
     def _get_collection(self, request):
@@ -216,7 +216,7 @@ class AllLists(_ListBase):
             reason = 'Domain does not exist: {}'.format(error.domain)
             bad_request(response, reason.encode('utf-8'))
         else:
-            location = self.path_to('lists/{0}'.format(mlist.list_id))
+            location = self.api.path_to('lists/{0}'.format(mlist.list_id))
             created(response, location)
 
     def on_get(self, request, response):
diff --git a/src/mailman/rest/members.py b/src/mailman/rest/members.py
index b666bc2f4..91db982c7 100644
--- a/src/mailman/rest/members.py
+++ b/src/mailman/rest/members.py
@@ -63,20 +63,21 @@ class _MemberBase(CollectionMixin):
         # issue #121 for details.
         member_id = self.api.from_uuid(member.member_id)
         response = dict(
-            address=self.path_to('addresses/{}'.format(member.address.email)),
+            address=self.api.path_to(
+                'addresses/{}'.format(member.address.email)),
             delivery_mode=member.delivery_mode,
             email=member.address.email,
             list_id=member.list_id,
             member_id=member_id,
             moderation_action=member.moderation_action,
             role=role,
-            self_link=self.path_to('members/{}'.format(member_id)),
+            self_link=self.api.path_to('members/{}'.format(member_id)),
             )
         # Add the user link if there is one.
         user = member.user
         if user is not None:
             user_id = self.api.from_uuid(user.user_id)
-            response['user'] = self.path_to('users/{}'.format(user_id))
+            response['user'] = self.api.path_to('users/{}'.format(user_id))
         return response
 
     def _get_collection(self, request):
@@ -285,7 +286,7 @@ class AllMembers(_MemberBase):
                 # UUIDs and need to be converted to URLs because JSON doesn't
                 # directly support UUIDs.
                 member_id = self.api.from_uuid(member.member_id)
-                location = self.path_to('members/{}'.format(member_id))
+                location = self.api.path_to('members/{}'.format(member_id))
                 created(response, location)
                 return
             # The member could not be directly subscribed because there are
@@ -332,7 +333,7 @@ class AllMembers(_MemberBase):
         # UUIDs and need to be converted to URLs because JSON doesn't
         # directly support UUIDs.
         member_id = self.api.from_uuid(member.member_id)
-        location = self.path_to('members/{}'.format(member_id))
+        location = self.api.path_to('members/{}'.format(member_id))
         created(response, location)
 
     def on_get(self, request, response):
diff --git a/src/mailman/rest/post_moderation.py b/src/mailman/rest/post_moderation.py
index a9b3ba1b7..6283add75 100644
--- a/src/mailman/rest/post_moderation.py
+++ b/src/mailman/rest/post_moderation.py
@@ -28,8 +28,7 @@ from mailman.interfaces.action import Action
 from mailman.interfaces.messages import IMessageStore
 from mailman.interfaces.requests import IListRequests, RequestType
 from mailman.rest.helpers import (
-    CollectionMixin, bad_request, child, etag, no_content, not_found, okay,
-    path_to)
+    CollectionMixin, bad_request, child, etag, no_content, not_found, okay)
 from mailman.rest.validator import Validator, enum_validator
 from zope.component import getUtility
 
@@ -62,9 +61,8 @@ class _ModerationBase:
         # that's fine too.
         resource.pop('id', None)
         # Add a self_link.
-        resource['self_link'] = path_to(
-            'lists/{}/held/{}'.format(self._mlist.list_id, request_id),
-            self.api.version)
+        resource['self_link'] = self.api.path_to(
+            'lists/{}/held/{}'.format(self._mlist.list_id, request_id))
         return resource
 
 
diff --git a/src/mailman/rest/preferences.py b/src/mailman/rest/preferences.py
index 4299dc0cd..694aa47e9 100644
--- a/src/mailman/rest/preferences.py
+++ b/src/mailman/rest/preferences.py
@@ -26,7 +26,7 @@ __all__ = [
 from lazr.config import as_boolean
 from mailman.interfaces.member import DeliveryMode, DeliveryStatus
 from mailman.rest.helpers import (
-    GetterSetter, bad_request, etag, no_content, not_found, okay, path_to)
+    GetterSetter, bad_request, etag, no_content, not_found, okay)
 from mailman.rest.validator import (
     Validator, enum_validator, language_validator)
 
@@ -64,9 +64,8 @@ class ReadOnlyPreferences:
         if preferred_language is not None:
             resource['preferred_language'] = preferred_language.code
         # Add the self link.
-        resource['self_link'] = path_to(
-            '{}/preferences'.format(self._base_url),
-            self.api.version)
+        resource['self_link'] = self.api.path_to(
+            '{}/preferences'.format(self._base_url))
         okay(response, etag(resource))
 
 
diff --git a/src/mailman/rest/queues.py b/src/mailman/rest/queues.py
index 983b7481e..4d3c9f58b 100644
--- a/src/mailman/rest/queues.py
+++ b/src/mailman/rest/queues.py
@@ -46,7 +46,7 @@ class _QueuesBase(CollectionMixin):
             directory=switchboard.queue_directory,
             count=len(files),
             files=files,
-            self_link=self.path_to('queues/{}'.format(name)),
+            self_link=self.api.path_to('queues/{}'.format(name)),
             )
 
     def _get_collection(self, request):
@@ -89,7 +89,7 @@ class AQueue(_QueuesBase):
             bad_request(response, str(error))
             return
         else:
-            location = self.path_to(
+            location = self.api.path_to(
                 'queues/{}/{}'.format(self._name, filebase))
             created(response, location)
 
@@ -122,5 +122,5 @@ class AllQueues(_QueuesBase):
     def on_get(self, request, response):
         """<api>/queues"""
         resource = self._make_collection(request)
-        resource['self_link'] = self.path_to('queues')
+        resource['self_link'] = self.api.path_to('queues')
         okay(response, etag(resource))
diff --git a/src/mailman/rest/root.py b/src/mailman/rest/root.py
index 8e6e9f716..ca6e031e8 100644
--- a/src/mailman/rest/root.py
+++ b/src/mailman/rest/root.py
@@ -26,15 +26,15 @@ import falcon
 
 from base64 import b64decode
 from mailman.config import config
+from mailman.core.api import API30, API31
 from mailman.core.constants import system_preferences
 from mailman.core.system import system
 from mailman.interfaces.listmanager import IListManager
 from mailman.model.uid import UID
 from mailman.rest.addresses import AllAddresses, AnAddress
-from mailman.rest.api import API30, API31
 from mailman.rest.domains import ADomain, AllDomains
 from mailman.rest.helpers import (
-    BadRequest, NotFound, child, etag, no_content, not_found, okay, path_to)
+    BadRequest, NotFound, child, etag, no_content, not_found, okay)
 from mailman.rest.lists import AList, AllLists, Styles
 from mailman.rest.members import AMember, AllMembers, FindMembers
 from mailman.rest.preferences import ReadOnlyPreferences
@@ -104,7 +104,7 @@ class Versions:
             mailman_version=system.mailman_version,
             python_version=system.python_version,
             api_version=self.api.version,
-            self_link=path_to('system/versions', self.api.version),
+            self_link=self.api.path_to('system/versions'),
             )
         okay(response, etag(resource))
 
diff --git a/src/mailman/rest/tests/test_addresses.py b/src/mailman/rest/tests/test_addresses.py
index bbb6ee2d1..0af52f607 100644
--- a/src/mailman/rest/tests/test_addresses.py
+++ b/src/mailman/rest/tests/test_addresses.py
@@ -369,6 +369,18 @@ class TestAddresses(unittest.TestCase):
                 })
         self.assertEqual(cm.exception.code, 403)
 
+    def test_user_subresource_post_no_such_user(self):
+        # Try to link an address to a nonexistent user.
+        with transaction():
+            getUtility(IUserManager).create_address('anne@example.com')
+        with self.assertRaises(HTTPError) as cm:
+            call_api(
+                'http://localhost:9001/3.0/addresses/anne@example.com/user', {
+                    'user_id': 2,
+                    })
+        self.assertEqual(cm.exception.code, 400)
+        self.assertEqual(cm.exception.reason, b'No user with ID 2')
+
     def test_user_subresource_unlink(self):
         # By DELETEing the usr subresource, you can unlink a user from an
         # address.
diff --git a/src/mailman/rest/tests/test_validator.py b/src/mailman/rest/tests/test_validator.py
index 7d7900aaf..287928d3c 100644
--- a/src/mailman/rest/tests/test_validator.py
+++ b/src/mailman/rest/tests/test_validator.py
@@ -25,7 +25,7 @@ __all__ = [
 import unittest
 
 from mailman.interfaces.usermanager import IUserManager
-from mailman.rest.api import API30, API31
+from mailman.core.api import API30, API31
 from mailman.rest.validator import (
     list_of_strings_validator, subscriber_validator)
 from mailman.testing.layers import RESTLayer
diff --git a/src/mailman/rest/users.py b/src/mailman/rest/users.py
index 017a09525..238ec4562 100644
--- a/src/mailman/rest/users.py
+++ b/src/mailman/rest/users.py
@@ -35,7 +35,7 @@ from mailman.interfaces.usermanager import IUserManager
 from mailman.rest.addresses import UserAddresses
 from mailman.rest.helpers import (
     BadRequest, CollectionMixin, GetterSetter, NotFound, bad_request, child,
-    conflict, created, etag, forbidden, no_content, not_found, okay, path_to)
+    conflict, created, etag, forbidden, no_content, not_found, okay)
 from mailman.rest.preferences import Preferences
 from mailman.rest.validator import (
     PatchValidator, Validator, list_of_strings_validator)
@@ -118,7 +118,7 @@ def create_user(arguments, request, response):
     user.is_server_owner = is_server_owner
     api = request.context['api']
     user_id = api.from_uuid(user.user_id)
-    location = path_to('users/{}'.format(user_id), api.version)
+    location = request.context.get('api').path_to('users/{}'.format(user_id))
     created(response, location)
     return user
 
@@ -137,7 +137,7 @@ class _UserBase(CollectionMixin):
         resource = dict(
             created_on=user.created_on,
             is_server_owner=user.is_server_owner,
-            self_link=self.path_to('users/{}'.format(user_id)),
+            self_link=self.api.path_to('users/{}'.format(user_id)),
             user_id=user_id,
         )
         # Add the password attribute, only if the user has a password.  Same
@@ -335,7 +335,8 @@ class AddressUser(_UserBase):
             user_id = arguments['user_id']
             user = user_manager.get_user_by_id(user_id)
             if user is None:
-                not_found(response, b'No user with ID {}'.format(user_id))
+                bad_request(response, 'No user with ID {}'.format(
+                    self.api.from_uuid(user_id)))
                 return
             okay(response)
         else: