Barry Warsaw pushed to branch master at mailman / Mailman

Commits:
d75a7ebb by Barry Warsaw at 2016-01-13T00:17:49-05:00
Refactor API contexts.

Rather than sprinkle API version string tests all over the place, create
an IAPI interface which encapsulates the differences between API 3.0 and
3.1, and arrange for this to be used to convert to and from UUIDs.

- - - - -
98c074f1 by Barry Warsaw at 2016-01-13T11:16:38-05:00
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.

- - - - -


21 changed files:

- .gitignore
- coverage.ini
- src/mailman/commands/cli_info.py
- + src/mailman/core/api.py
- + src/mailman/interfaces/api.py
- src/mailman/rest/addresses.py
- src/mailman/rest/docs/helpers.rst
- src/mailman/rest/domains.py
- src/mailman/rest/helpers.py
- src/mailman/rest/lists.py
- src/mailman/rest/members.py
- src/mailman/rest/post_moderation.py
- src/mailman/rest/preferences.py
- src/mailman/rest/queues.py
- src/mailman/rest/root.py
- src/mailman/rest/tests/test_addresses.py
- src/mailman/rest/tests/test_validator.py
- src/mailman/rest/users.py
- src/mailman/rest/validator.py
- src/mailman/rest/wsgiapp.py
- tox.ini


Changes:

=====================================
.gitignore
=====================================
--- a/.gitignore
+++ b/.gitignore
@@ -4,3 +4,5 @@ var
 htmlcov
 __pycache__
 .tox
+coverage.xml
+diffcov.html


=====================================
coverage.ini
=====================================
--- a/coverage.ini
+++ b/coverage.ini
@@ -5,6 +5,7 @@ omit =
      setup*
      */showme.py
     .tox/coverage/lib/python3.5/site-packages/*
+    .tox/diffcov/lib/python3.5/site-packages/*
     */test_*.py
 
 [report]


=====================================
src/mailman/commands/cli_info.py
=====================================
--- a/src/mailman/commands/cli_info.py
+++ b/src/mailman/commands/cli_info.py
@@ -26,9 +26,9 @@ import sys
 
 from lazr.config import as_boolean
 from mailman.config import config
+from mailman.core.api import API30, API31
 from mailman.core.i18n import _
 from mailman.interfaces.command import ICLISubCommand
-from mailman.rest.helpers import path_to
 from mailman.version import MAILMAN_VERSION_FULL
 from zope.interface import implementer
 
@@ -68,9 +68,8 @@ class Info:
         print('devmode:',
               'ENABLED' if as_boolean(config.devmode.enabled) else 'DISABLED',
               file=output)
-        print('REST root url:',
-              path_to('/', config.webservice.api_version),
-              file=output)
+        api = (API30 if config.webservice.api_version == '3.0' else API31)
+        print('REST root url:', api.path_to('/'), file=output)
         print('REST credentials: {0}:{1}'.format(
             config.webservice.admin_user, config.webservice.admin_pass),
             file=output)


=====================================
src/mailman/core/api.py
=====================================
--- /dev/null
+++ b/src/mailman/core/api.py
@@ -0,0 +1,82 @@
+# 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 lazr.config import as_boolean
+from mailman.config import config
+from mailman.interfaces.api import IAPI
+from uuid import UUID
+from zope.interface import implementer
+
+
+@implementer(IAPI)
+class API30:
+    version = '3.0'
+
+    @classmethod
+    def path_to(cls, resource):
+        """See `IAPI`."""
+        return '{}://{}:{}/{}/{}'.format(
+            ('https' if as_boolean(config.webservice.use_https) else 'http'),
+            config.webservice.hostname,
+            config.webservice.port,
+            cls.version,
+            (resource[1:] if resource.startswith('/') else resource),
+            )
+
+    @staticmethod
+    def from_uuid(uuid):
+        """See `IAPI`."""
+        return uuid.int
+
+    @staticmethod
+    def to_uuid(uuid_repr):
+        """See `IAPI`."""
+        return UUID(int=int(uuid_repr))
+
+
+@implementer(IAPI)
+class API31:
+    version = '3.1'
+
+    @classmethod
+    def path_to(cls, resource):
+        """See `IAPI`."""
+        return '{}://{}:{}/{}/{}'.format(
+            ('https' if as_boolean(config.webservice.use_https) else 'http'),
+            config.webservice.hostname,
+            config.webservice.port,
+            cls.version,
+            (resource[1:] if resource.startswith('/') else resource),
+            )
+
+    @staticmethod
+    def from_uuid(uuid):
+        """See `IAPI`."""
+        return uuid.hex
+
+    @staticmethod
+    def to_uuid(uuid_repr):
+        """See `IAPI`."""
+        return UUID(hex=uuid_repr)


=====================================
src/mailman/interfaces/api.py
=====================================
--- /dev/null
+++ b/src/mailman/interfaces/api.py
@@ -0,0 +1,65 @@
+# 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 context."""
+
+__all__ = [
+    'IAPI',
+    ]
+
+
+from zope.interface import Attribute, Interface
+
+
+class IAPI(Interface):
+    """The REST web service context."""
+
+    version = Attribute("""The REST API version.""")
+
+    def path_to(resource):
+        """Return the full REST URL to the given resource.
+
+        :param resource: Resource path string without the leading scheme,
+            host, port, or API version information.
+        :type resource: str
+        :return: Full URL path to the resource, with the scheme, host, port
+            and API version prepended.
+        :rtype: str
+        """
+
+    def from_uuid(uuid):
+        """Return the string representation of a UUID.
+
+        :param uuid: The UUID to convert.
+        :type uuid: UUID
+        :return: The string representation of the UUID, as appropriate for the
+            API version.  In 3.0 this is the representation of an integer,
+            while in 3.1 it is the hex representation.
+        :rtype: str
+        """
+
+    def to_uuid(uuid):
+        """Return the UUID from the string representation.
+
+        :param uuid: The string representation of the UUID.
+        :type uuid: str
+        :return: The UUID converted from the string representation, as
+            appropriate for the API version.  In 3.0, uuid is interpreted as
+            the integer representation of a UUID, while in 3.1 it is the hex
+            representation of the UUID.
+        :rtype: UUID
+        """


=====================================
src/mailman/rest/addresses.py
=====================================
--- 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:
@@ -59,9 +59,8 @@ class _AddressBase(CollectionMixin):
         if address.verified_on:
             representation['verified_on'] = address.verified_on
         if address.user:
-            uid = getattr(address.user.user_id,
-                          'int' if self.api_version == '3.0' else 'hex')
-            representation['user'] = self.path_to('users/{}'.format(uid))
+            uid = self.api.from_uuid(address.user.user_id)
+            representation['user'] = self.api.path_to('users/{}'.format(uid))
         return representation
 
     def _get_collection(self, request):
@@ -214,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)
 
 


=====================================
src/mailman/rest/docs/helpers.rst
=====================================
--- 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
 =====
 


=====================================
src/mailman/rest/domains.py
=====================================
--- 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):


=====================================
src/mailman/rest/helpers.py
=====================================
--- 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:


=====================================
src/mailman/rest/lists.py
=====================================
--- 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):
@@ -155,7 +155,7 @@ class AList(_ListBase):
             email, self._mlist.list_id, role)
         if member is None:
             return NotFound(), []
-        return AMember(request.context['api_version'], member.member_id)
+        return AMember(request.context['api'], member.member_id)
 
     @child(roster_matcher)
     def roster(self, request, segments, role):
@@ -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):


=====================================
src/mailman/rest/members.py
=====================================
--- a/src/mailman/rest/members.py
+++ b/src/mailman/rest/members.py
@@ -27,7 +27,7 @@ __all__ = [
 
 from mailman.app.membership import add_member, delete_member
 from mailman.interfaces.action import Action
-from mailman.interfaces.address import IAddress, InvalidEmailAddressError
+from mailman.interfaces.address import IAddress
 from mailman.interfaces.listmanager import IListManager
 from mailman.interfaces.member import (
     AlreadySubscribedError, DeliveryMode, MemberRole, MembershipError,
@@ -43,7 +43,6 @@ from mailman.rest.helpers import (
 from mailman.rest.preferences import Preferences, ReadOnlyPreferences
 from mailman.rest.validator import (
     Validator, enum_validator, subscriber_validator)
-from operator import attrgetter
 from uuid import UUID
 from zope.component import getUtility
 
@@ -52,10 +51,6 @@ from zope.component import getUtility
 class _MemberBase(CollectionMixin):
     """Shared base class for member representations."""
 
-    def _get_uuid(self, member):
-        return getattr(member.member_id,
-                       'int' if self.api_version == '3.0' else 'hex')
-
     def _resource_as_dict(self, member):
         """See `CollectionMixin`."""
         enum, dot, role = str(member.role).partition('.')
@@ -66,23 +61,23 @@ class _MemberBase(CollectionMixin):
         # member_id are UUIDs.  In API 3.0 we use the integer equivalent of
         # the UID in the URL, but in API 3.1 we use the hex equivalent.  See
         # issue #121 for details.
-        member_id = self._get_uuid(member)
+        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 = getattr(user.user_id,
-                              'int' if self.api_version == '3.0' else 'hex')
-            response['user'] = self.path_to('users/{}'.format(user_id))
+            user_id = self.api.from_uuid(user.user_id)
+            response['user'] = self.api.path_to('users/{}'.format(user_id))
         return response
 
     def _get_collection(self, request):
@@ -112,16 +107,13 @@ class MemberCollection(_MemberBase):
 class AMember(_MemberBase):
     """A member."""
 
-    def __init__(self, api_version, member_id_string):
+    def __init__(self, api, member_id_string):
         # The member_id_string is the string representation of the member's
         # UUID.  In API 3.0, the argument is the string representation of the
         # int representation of the UUID.  In API 3.1 it's the hex.
-        self.api_version = api_version
+        self.api = api
         try:
-            if api_version == '3.0':
-                member_id = UUID(int=int(member_id_string))
-            else:
-                member_id = UUID(hex=member_id_string)
+            member_id = api.to_uuid(member_id_string)
         except ValueError:
             # The string argument could not be converted to a UUID.
             self._member = None
@@ -143,7 +135,7 @@ class AMember(_MemberBase):
             return NotFound(), []
         if self._member is None:
             return NotFound(), []
-        member_id = self._get_uuid(self._member)
+        member_id = self.api.from_uuid(self._member.member_id)
         child = Preferences(
             self._member.preferences, 'members/{}'.format(member_id))
         return child, []
@@ -157,7 +149,8 @@ class AMember(_MemberBase):
             return NotFound(), []
         child = ReadOnlyPreferences(
             self._member,
-            'members/{}/all'.format(self._get_uuid(self._member)))
+            'members/{}/all'.format(
+                self.api.from_uuid(self._member.member_id)))
         return child, []
 
     def on_delete(self, request, response):
@@ -220,7 +213,7 @@ class AllMembers(_MemberBase):
         try:
             validator = Validator(
                 list_id=str,
-                subscriber=subscriber_validator(self.api_version),
+                subscriber=subscriber_validator(self.api),
                 display_name=str,
                 delivery_mode=enum_validator(DeliveryMode),
                 role=enum_validator(MemberRole),
@@ -292,8 +285,8 @@ class AllMembers(_MemberBase):
                 # and return the location to the new member.  Member ids are
                 # UUIDs and need to be converted to URLs because JSON doesn't
                 # directly support UUIDs.
-                member_id = self._get_uuid(member)
-                location = self.path_to('members/{}'.format(member_id))
+                member_id = self.api.from_uuid(member.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
@@ -339,8 +332,8 @@ class AllMembers(_MemberBase):
         # and return the location to the new member.  Member ids are
         # UUIDs and need to be converted to URLs because JSON doesn't
         # directly support UUIDs.
-        member_id = self._get_uuid(member)
-        location = self.path_to('members/{}'.format(member_id))
+        member_id = self.api.from_uuid(member.member_id)
+        location = self.api.path_to('members/{}'.format(member_id))
         created(response, location)
 
     def on_get(self, request, response):
@@ -353,10 +346,10 @@ class AllMembers(_MemberBase):
 class _FoundMembers(MemberCollection):
     """The found members collection."""
 
-    def __init__(self, members, api_version):
+    def __init__(self, members, api):
         super().__init__()
         self._members = members
-        self.api_version = api_version
+        self.api = api
 
     def _get_collection(self, request):
         """See `CollectionMixin`."""
@@ -380,5 +373,5 @@ class FindMembers(_MemberBase):
             bad_request(response, str(error))
         else:
             members = service.find_members(**data)
-            resource = _FoundMembers(members, self.api_version)
+            resource = _FoundMembers(members, self.api)
             okay(response, etag(resource._make_collection(request)))


=====================================
src/mailman/rest/post_moderation.py
=====================================
--- 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
 
 


=====================================
src/mailman/rest/preferences.py
=====================================
--- 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))
 
 


=====================================
src/mailman/rest/queues.py
=====================================
--- 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))


=====================================
src/mailman/rest/root.py
=====================================
--- a/src/mailman/rest/root.py
+++ b/src/mailman/rest/root.py
@@ -26,6 +26,7 @@ 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
@@ -33,7 +34,7 @@ from mailman.model.uid import UID
 from mailman.rest.addresses import AllAddresses, AnAddress
 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
@@ -59,7 +60,7 @@ class Root:
     @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'
+        request.context['api'] = API30
         return self._check_authorization(request, segments)
 
     @child('3.1')
@@ -68,7 +69,7 @@ class Root:
         # 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'
+        request.context['api'] = API31
         return self._check_authorization(request, segments)
 
     def _check_authorization(self, request, segments):
@@ -102,8 +103,8 @@ class Versions:
         resource = dict(
             mailman_version=system.mailman_version,
             python_version=system.python_version,
-            api_version=self.api_version,
-            self_link=path_to('system/versions', self.api_version),
+            api_version=self.api.version,
+            self_link=self.api.path_to('system/versions'),
             )
         okay(response, etag(resource))
 
@@ -179,12 +180,12 @@ class TopLevel:
         """
         if len(segments) == 0:
             resource = AllAddresses()
-            resource.api_version = request.context['api_version']
+            resource.api = request.context['api']
             return resource
         else:
             email = segments.pop(0)
             resource = AnAddress(email)
-            resource.api_version = request.context['api_version']
+            resource.api = request.context['api']
             return resource, segments
 
     @child()
@@ -218,32 +219,32 @@ class TopLevel:
     @child()
     def members(self, request, segments):
         """/<api>/members"""
-        api_version = request.context['api_version']
+        api = request.context['api']
         if len(segments) == 0:
             resource = AllMembers()
-            resource.api_version = api_version
+            resource.api = api
             return resource
         # Either the next segment is the string "find" or a member id.  They
         # cannot collide.
         segment = segments.pop(0)
         if segment == 'find':
             resource = FindMembers()
-            resource.api_version = api_version
+            resource.api = api
         else:
-            resource = AMember(api_version, segment)
+            resource = AMember(api, segment)
         return resource, segments
 
     @child()
     def users(self, request, segments):
         """/<api>/users"""
-        api_version = request.context['api_version']
+        api = request.context['api']
         if len(segments) == 0:
             resource = AllUsers()
-            resource.api_version = api_version
+            resource.api = api
             return resource
         else:
             user_id = segments.pop(0)
-            return AUser(api_version, user_id), segments
+            return AUser(api, user_id), segments
 
     @child()
     def owners(self, request, segments):
@@ -252,7 +253,7 @@ class TopLevel:
             return BadRequest(), []
         else:
             resource = ServerOwners()
-            resource.api_version = request.context['api_version']
+            resource.api = request.context['api']
             return resource, segments
 
     @child()


=====================================
src/mailman/rest/tests/test_addresses.py
=====================================
--- 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('a...@example.com')
+        with self.assertRaises(HTTPError) as cm:
+            call_api(
+                'http://localhost:9001/3.0/addresses/a...@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.
@@ -542,7 +554,7 @@ class TestAPI31Addresses(unittest.TestCase):
                     })
         self.assertEqual(cm.exception.code, 400)
         self.assertEqual(cm.exception.reason,
-                         b'badly formed hexadecimal UUID string')
+                         b'Cannot convert parameters: user_id')
 
     def test_user_subresource_put(self):
         # By PUTing to the 'user' resource, you can change the user that an
@@ -577,4 +589,4 @@ class TestAPI31Addresses(unittest.TestCase):
                     }, method='PUT')
         self.assertEqual(cm.exception.code, 400)
         self.assertEqual(cm.exception.reason,
-                         b'badly formed hexadecimal UUID string')
+                         b'Cannot convert parameters: user_id')


=====================================
src/mailman/rest/tests/test_validator.py
=====================================
--- a/src/mailman/rest/tests/test_validator.py
+++ b/src/mailman/rest/tests/test_validator.py
@@ -25,6 +25,7 @@ __all__ = [
 import unittest
 
 from mailman.interfaces.usermanager import IUserManager
+from mailman.core.api import API30, API31
 from mailman.rest.validator import (
     list_of_strings_validator, subscriber_validator)
 from mailman.testing.layers import RESTLayer
@@ -53,35 +54,35 @@ class TestValidators(unittest.TestCase):
     def test_subscriber_validator_int_uuid(self):
         # Convert from an existing user id to a UUID.
         anne = getUtility(IUserManager).make_user('a...@example.com')
-        uuid = subscriber_validator('3.0')(str(anne.user_id.int))
+        uuid = subscriber_validator(API30)(str(anne.user_id.int))
         self.assertEqual(anne.user_id, uuid)
 
     def test_subscriber_validator_hex_uuid(self):
         # Convert from an existing user id to a UUID.
         anne = getUtility(IUserManager).make_user('a...@example.com')
-        uuid = subscriber_validator('3.1')(anne.user_id.hex)
+        uuid = subscriber_validator(API31)(anne.user_id.hex)
         self.assertEqual(anne.user_id, uuid)
 
     def test_subscriber_validator_no_int_uuid(self):
         # API 3.1 does not accept ints as subscriber id's.
         anne = getUtility(IUserManager).make_user('a...@example.com')
         self.assertRaises(ValueError,
-                          subscriber_validator('3.1'), str(anne.user_id.int))
+                          subscriber_validator(API31), str(anne.user_id.int))
 
     def test_subscriber_validator_bad_int_uuid(self):
         # In API 3.0, UUIDs are ints.
         self.assertRaises(ValueError,
-                          subscriber_validator('3.0'), 'not-a-thing')
+                          subscriber_validator(API30), 'not-a-thing')
 
     def test_subscriber_validator_bad_int_hex(self):
         # In API 3.1, UUIDs are hexes.
         self.assertRaises(ValueError,
-                          subscriber_validator('3.1'), 'not-a-thing')
+                          subscriber_validator(API31), 'not-a-thing')
 
     def test_subscriber_validator_email_address_API30(self):
-        self.assertEqual(subscriber_validator('3.0')('a...@example.com'),
+        self.assertEqual(subscriber_validator(API30)('a...@example.com'),
                          'a...@example.com')
 
     def test_subscriber_validator_email_address_API31(self):
-        self.assertEqual(subscriber_validator('3.1')('a...@example.com'),
+        self.assertEqual(subscriber_validator(API31)('a...@example.com'),
                          'a...@example.com')


=====================================
src/mailman/rest/users.py
=====================================
--- a/src/mailman/rest/users.py
+++ b/src/mailman/rest/users.py
@@ -35,12 +35,11 @@ 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)
 from passlib.utils import generate_password as generate
-from uuid import UUID
 from zope.component import getUtility
 
 
@@ -117,9 +116,9 @@ def create_user(arguments, request, response):
         password = generate(int(config.passwords.password_length))
     user.password = config.password_context.encrypt(password)
     user.is_server_owner = is_server_owner
-    api_version = request.context['api_version']
-    user_id = getattr(user.user_id, 'int' if api_version == '3.0' else 'hex')
-    location = path_to('users/{}'.format(user_id), api_version)
+    api = request.context['api']
+    user_id = api.from_uuid(user.user_id)
+    location = request.context.get('api').path_to('users/{}'.format(user_id))
     created(response, location)
     return user
 
@@ -128,21 +127,17 @@ def create_user(arguments, request, response):
 class _UserBase(CollectionMixin):
     """Shared base class for user representations."""
 
-    def _get_uuid(self, user):
-        return getattr(user.user_id,
-                       'int' if self.api_version == '3.0' else 'hex')
-
     def _resource_as_dict(self, user):
         """See `CollectionMixin`."""
         # The canonical URL for a user is their unique user id, although we
         # can always look up a user based on any registered and validated
         # email address associated with their account.  The user id is a UUID,
         # but we serialize its integer equivalent.
-        user_id = self._get_uuid(user)
+        user_id = self.api.from_uuid(user.user_id)
         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
@@ -182,9 +177,11 @@ class AllUsers(_UserBase):
 class AUser(_UserBase):
     """A user."""
 
-    def __init__(self, api_version, user_identifier):
+    def __init__(self, api, user_identifier):
         """Get a user by various type of identifiers.
 
+        :param api: The REST API object.
+        :type api: IAPI
         :param user_identifier: The identifier used to retrieve the user.  The
             identifier may either be an email address controlled by the user
             or the UUID of the user.  The type of identifier is auto-detected
@@ -193,7 +190,7 @@ class AUser(_UserBase):
             API 3.0 are integers, while in 3.1 are hex.
         :type user_identifier: string
         """
-        self.api_version = api_version
+        self.api = api
         user_manager = getUtility(IUserManager)
         if '@' in user_identifier:
             self._user = user_manager.get_user(user_identifier)
@@ -201,10 +198,7 @@ class AUser(_UserBase):
             # The identifier is the string representation of a UUID, either an
             # int in API 3.0 or a hex in API 3.1.
             try:
-                if api_version == '3.0':
-                    user_id = UUID(int=int(user_identifier))
-                else:
-                    user_id = UUID(hex=user_identifier)
+                user_id = api.to_uuid(user_identifier)
             except ValueError:
                 self._user = None
             else:
@@ -244,7 +238,7 @@ class AUser(_UserBase):
             return NotFound(), []
         child = Preferences(
             self._user.preferences,
-            'users/{}'.format(self._get_uuid(self._user)))
+            'users/{}'.format(self.api.from_uuid(self._user.user_id)))
         return child, []
 
     def on_patch(self, request, response):
@@ -319,14 +313,14 @@ class AddressUser(_UserBase):
         if self._user:
             conflict(response)
             return
-        api_version = request.context['api_version']
+        api = request.context['api']
         # When creating a linked user by POSTing, the user either must already
         # exist, or it can be automatically created, if the auto_create flag
         # is given and true (if missing, it defaults to true).  However, in
         # this case we do not accept 'email' as a POST field.
         fields = CREATION_FIELDS.copy()
         del fields['email']
-        fields['user_id'] = (int if api_version == '3.0' else str)
+        fields['user_id'] = api.to_uuid
         fields['auto_create'] = as_boolean
         fields['_optional'] = fields['_optional'] + (
             'user_id', 'auto_create', 'is_server_owner')
@@ -338,16 +332,11 @@ class AddressUser(_UserBase):
             return
         user_manager = getUtility(IUserManager)
         if 'user_id' in arguments:
-            raw_uid = arguments['user_id']
-            kws = {('int' if api_version == '3.0' else 'hex'): raw_uid}
-            try:
-                user_id = UUID(**kws)
-            except ValueError as error:
-                bad_request(response, str(error))
-                return
+            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(raw_uid))
+                bad_request(response, 'No user with ID {}'.format(
+                    self.api.from_uuid(user_id)))
                 return
             okay(response)
         else:
@@ -364,12 +353,12 @@ class AddressUser(_UserBase):
 
     def on_put(self, request, response):
         """Set or replace the addresses's user."""
-        api_version = request.context['api_version']
+        api = request.context['api']
         if self._user:
             self._user.unlink(self._address)
         # Process post data and check for an existing user.
         fields = CREATION_FIELDS.copy()
-        fields['user_id'] = (int if api_version == '3.0' else str)
+        fields['user_id'] = api.to_uuid
         fields['_optional'] = fields['_optional'] + (
             'user_id', 'email', 'is_server_owner')
         try:
@@ -380,16 +369,10 @@ class AddressUser(_UserBase):
             return
         user_manager = getUtility(IUserManager)
         if 'user_id' in arguments:
-            raw_uid = arguments['user_id']
-            kws = {('int' if api_version == '3.0' else 'hex'): raw_uid}
-            try:
-                user_id = UUID(**kws)
-            except ValueError as error:
-                bad_request(response, str(error))
-                return
+            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(raw_uid))
+                not_found(response, b'No user with ID {}'.format(user_id))
                 return
             okay(response)
         else:


=====================================
src/mailman/rest/validator.py
=====================================
--- a/src/mailman/rest/validator.py
+++ b/src/mailman/rest/validator.py
@@ -31,7 +31,6 @@ from mailman.core.errors import (
     ReadOnlyPATCHRequestError, UnknownPATCHRequestError)
 from mailman.interfaces.address import IEmailValidator
 from mailman.interfaces.languages import ILanguageManager
-from uuid import UUID
 from zope.component import getUtility
 
 
@@ -55,18 +54,11 @@ class enum_validator:
             raise ValueError(exception.args[0])
 
 
-def subscriber_validator(api_version):
+def subscriber_validator(api):
     """Convert an email-or-(int|hex) to an email-or-UUID."""
     def _inner(subscriber):
-        # In API 3.0, the uuid is represented by an int, so if we can int
-        # convert the value, we know it's a UUID-as-int.  In API 3.1 though,
-        # uuids are represented by the hex version, which of course cannot
-        # include an @ sign.
         try:
-            if api_version == '3.0':
-                return UUID(int=int(subscriber))
-            else:
-                return UUID(hex=subscriber)
+            return api.to_uuid(subscriber)
         except ValueError:
             # It must be an email address.
             if getUtility(IEmailValidator).is_valid(subscriber):


=====================================
src/mailman/rest/wsgiapp.py
=====================================
--- a/src/mailman/rest/wsgiapp.py
+++ b/src/mailman/rest/wsgiapp.py
@@ -76,7 +76,7 @@ class AdminWebServiceWSGIRequestHandler(WSGIRequestHandler):
 
 
 class SetAPIVersion:
-    """Falcon middleware object that sets the api_version on resources."""
+    """Falcon middleware object that sets the API on resources."""
 
     def process_resource(self, request, response, resource):
         # Set this attribute on the resource right before it is dispatched
@@ -88,7 +88,7 @@ class SetAPIVersion:
         # 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')
+            resource.api = request.context.get('api')
 
 
 class RootedAPI(API):


=====================================
tox.ini
=====================================
--- a/tox.ini
+++ b/tox.ini
@@ -30,9 +30,26 @@ commands =
     python -m coverage report -m {[coverage]rc}
 #sitepackages = True
 usedevelop = True
-whitelist_externals = python-coverage
 deps = coverage
 setenv =
     COVERAGE_PROCESS_START={[coverage]rcfile}
     COVERAGE_OPTIONS="-p"
     COVERAGE_FILE={toxinidir}/.coverage
+
+[testenv:diffcov]
+basepython = python3.5
+commands =
+    python -m coverage run {[coverage]rc} -m nose2 -v
+    python -m coverage combine {[coverage]rc}
+    python -m coverage xml {[coverage]rc}
+    diff-cover coverage.xml --html-report diffcov.html
+    diff-cover coverage.xml
+#sitepackages = True
+usedevelop = True
+deps =
+    coverage
+    diff_cover
+setenv =
+    COVERAGE_PROCESS_START={[coverage]rcfile}
+    COVERAGE_OPTIONS="-p"
+    COVERAGE_FILE={toxinidir}/.coverage



View it on GitLab: 
https://gitlab.com/mailman/mailman/compare/03bb57c8c2a47a08e19b20975622ebb2ef2b81c6...98c074f19492d81ebf5b5c3f4d4f2210aa56230d
_______________________________________________
Mailman-checkins mailing list
Mailman-checkins@python.org
Unsubscribe: 
https://mail.python.org/mailman/options/mailman-checkins/archive%40jab.org

Reply via email to