Merge pull request #1384 from diox/convert-rating-source-to-drf

Convert RatingResource and RatingFlagResource to DRF (bug 910570, bug 910571)

mkt/api/authorization.py

@@ -1,3 +1,5 @@
+from collections import defaultdict
+
 import commonware.log
 
 from rest_framework.permissions import BasePermission, SAFE_METHODS
@@ -129,9 +131,9 @@ class AllowSelf(BasePermission):
     def has_permission(self, request, view):
         return request.user.is_authenticated()
 
-    def has_object_permission(self, request, view, object):
+    def has_object_permission(self, request, view, obj):
         try:
-            return object.pk == request.amo_user.pk
+            return obj.pk == request.amo_user.pk
 
         # Appropriately handles AnonymousUsers when `amo_user` is None.
         except AttributeError:
@@ -168,17 +170,25 @@ class AllowAppOwner(BasePermission):
     def has_permission(self, request, view):
         return not request.user.is_anonymous()
 
-    def has_object_permission(self, request, view, object):
+    def has_object_permission(self, request, view, obj):
         try:
-            return object.authors.filter(user__id=request.amo_user.pk).exists()
+            return obj.authors.filter(user__id=request.amo_user.pk).exists()
 
         # Appropriately handles AnonymousUsers when `amo_user` is None.
         except AttributeError:
             return False
 
 
-class AllowReviewerReadOnly(BasePermission):
+class AllowRelatedAppOwner(BasePermission):
+
+    def has_permission(self, request, view):
+        return not request.user.is_anonymous()
+
+    def has_object_permission(self, request, view, obj):
+        return AllowAppOwner().has_object_permission(request, view, obj.addon)
+
 
+class AllowReviewerReadOnly(BasePermission):
     def has_permission(self, request, view):
         return request.method in SAFE_METHODS  and acl.action_allowed(
             request, 'Apps', 'Review')
@@ -217,6 +227,9 @@ def has_permission(self, request, view):
             return switch_is_active(self.name)
         raise NotImplementedError
 
+    def has_object_permission(self, request, view, obj):
+        return self.has_permission(request, view)
+
 
 class GroupPermission(BasePermission):
 
@@ -227,8 +240,42 @@ def __init__(self, app, action):
     def has_permission(self, request, view):
         return acl.action_allowed(request, self.app, self.action)
 
+    def has_object_permission(self, request, view, obj):
+        return self.has_permission(request, view)
+
     def __call__(self, *a):
         """
         ignore DRF's nonsensical need to call this object.
         """
         return self
+
+
+class ByHttpMethod(BasePermission):
+    """
+    Permission class allowing you to define different Permissions depending on
+    the HTTP method used.
+
+    method_permission is a dict with the lowercase http method names as keys,
+    permission classes (not instantiated, like DRF expects them) as values.
+
+    Careful, you probably want to define AllowAny for 'options' if you are
+    using a CORS-enabled endpoint.
+    """
+    def __init__(self, method_permissions, default=None):
+        if default is None:
+            default = AllowNone()
+        self.method_permissions = defaultdict(lambda: default)
+        for method, perm in method_permissions.items():
+            # Initialize the permissions by calling them like DRF does.
+            self.method_permissions[method] = perm()
+
+    def has_permission(self, request, view):
+        perm = self.method_permissions[request.method.lower()]
+        return perm.has_permission(request, view)
+
+    def has_object_permission(self, request, view, obj):
+        perm = self.method_permissions[request.method.lower()]
+        return perm.has_object_permission(request, view, obj)
+
+    def __call__(self):
+        return self

mkt/api/base.py

@@ -16,6 +16,7 @@
 from rest_framework.routers import Route, SimpleRouter
 from rest_framework.relations import HyperlinkedRelatedField
 from rest_framework.response import Response
+from rest_framework.serializers import ValidationError
 from rest_framework.viewsets import GenericViewSet
 from tastypie import fields, http
 from tastypie.bundle import Bundle
@@ -420,23 +421,44 @@ def check_potatocaptcha(data):
 
 class CompatRelatedField(HyperlinkedRelatedField):
     """
-    Upsell field for connecting Tastypie resources to
-    django-rest-framework instances, this got complicated.
+    Field for connecting Tastypie resources to django-rest-framework instances.
+
+    Serializes to an URL, deserializes URLs, PKs and slugs.
     """
 
     def __init__(self, *args, **kwargs):
         self.tastypie = kwargs.pop('tastypie')
-        return super(CompatRelatedField, self).__init__(*args, **kwargs)
-
-    def to_native(self, obj):
-        if getattr(obj, 'pk', None) is None:
-            return
-
-        self.tastypie['pk'] = obj.pk
-        return reverse('api_dispatch_detail', kwargs=self.tastypie)
+        kwargs['view_name'] = 'api_dispatch_detail'
+        super(CompatRelatedField, self).__init__(*args, **kwargs)
+
+    def get_url(self, obj, view_name, request, format):
+        lookup_field = getattr(obj, self.lookup_field)
+        kwargs = {self.lookup_field: lookup_field}
+        kwargs.update(self.tastypie)
+        return reverse(view_name, kwargs=kwargs)
+
+    def from_native(self, value):
+        kwargs = {}
+
+        if isinstance(value, basestring):
+            # value could be a slug, a pk in string form, or an URL.
+            if value.isdigit():
+                kwargs[self.lookup_field] = int(value)
+            elif not value.startswith(('http:', 'https:', '/')):
+                kwargs[self.slug_field] = value
+            else:
+                # Default behaviour is to handle URLs.
+                return super(CompatRelatedField, self).from_native(value)
+        elif isinstance(value, int):
+            kwargs[self.lookup_field] = value
+        else:
+            msg = self.error_messages['incorrect_type']
+            raise ValidationError(msg % type(value).__name__)
 
-    def get_object(self, queryset, view_name, view_args, view_kwargs):
-        return queryset.get(pk=view_kwargs['pk'])
+        try:
+            return self.get_object(self.queryset, self.view_name, None, kwargs)
+        except (ObjectDoesNotExist, TypeError, ValueError):
+            raise ValidationError(self.error_messages['does_not_exist'])
 
 
 class CompatToOneField(ToOneField):

mkt/api/exceptions.py

@@ -12,6 +12,14 @@ class AlreadyPurchased(Exception):
     pass
 
 
+class Conflict(APIException):
+    status_code = status.HTTP_409_CONFLICT
+    default_detail = 'Conflict detected.'
+
+    def __init__(self, detail=None):
+        self.detail = detail or self.default_detail
+
+
 class NotImplemented(APIException):
     status_code = status.HTTP_501_NOT_IMPLEMENTED
     default_detail = 'API not implemented.'

mkt/api/paginator.py

@@ -1,39 +1,61 @@
+import urlparse
+
+from django.http import QueryDict
+
 from rest_framework import serializers
 from rest_framework import pagination
-from rest_framework.templatetags.rest_framework import replace_query_param
-
 
-class NextPageField(serializers.Field):
-    """Wrapper to remove absolute_uri."""
-    page_field = 'page'
 
-    def to_native(self, value):
-        if not value.has_next():
-            return None
-        page = value.next_page_number()
+class MetaSerializer(serializers.Serializer):
+    """
+    Serializer for the 'meta' dict holding pagination info that allows to stay
+    backwards-compatible with the way tastypie does pagination (using offsets
+    instead of page numbers), while still using a "standard" Paginator class.
+    """
+    next = serializers.SerializerMethodField('get_next')
+    previous = serializers.SerializerMethodField('get_previous')
+    total_count = serializers.SerializerMethodField('get_total_count')
+    offset = serializers.SerializerMethodField('get_offset')
+    limit = serializers.SerializerMethodField('get_limit')
+
+    def replace_query_params(self, url, params):
+        (scheme, netloc, path, query, fragment) = urlparse.urlsplit(url)
+        query_dict = QueryDict(query).copy()
+        query_dict.update(params)
+        query = query_dict.urlencode()
+        return urlparse.urlunsplit((scheme, netloc, path, query, fragment))
+
+    def get_offset_link_for_page(self, page, number):
         request = self.context.get('request')
         url = request and request.get_full_path() or ''
-        return replace_query_param(url, self.page_field, page)
-
+        number = number - 1  # Pages are 1-based, but offsets are 0-based.
+        per_page = page.paginator.per_page
+        return self.replace_query_params(url, {'offset': number * per_page,
+                                               'limit': per_page})
 
-class PreviousPageField(serializers.Field):
-    """Wrapper to remove absolute_uri."""
-    page_field = 'page'
+    def get_next(self, page):
+        if not page.has_next():
+            return None
+        return self.get_offset_link_for_page(page, page.next_page_number())
 
-    def to_native(self, value):
-        if not value.has_previous():
+    def get_previous(self, page):
+        if not page.has_previous():
             return None
-        page = value.previous_page_number()
-        request = self.context.get('request')
-        url = request and request.get_full_path() or ''
-        return replace_query_param(url, self.page_field, page)
+        return self.get_offset_link_for_page(page, page.previous_page_number())
 
+    def get_total_count(self, page):
+        return page.paginator.count
 
-class MetaSerializer(serializers.Serializer):
-    next = NextPageField(source='*')
-    prev = PreviousPageField(source='*')
-    page = serializers.Field(source='number')
-    total_count = serializers.Field(source='paginator.count')
+    def get_offset(self, page):
+        index = page.start_index()
+        if index > 0:
+            # start_index() is 1-based, and we want a 0-based offset, so we
+            # need to remove 1, unless it's already 0.
+            return index - 1
+        return index
+
+    def get_limit(self, page):
+        return page.paginator.per_page
 
 
 class CustomPaginationSerializer(pagination.BasePaginationSerializer):

mkt/api/tests/nose.cfg

@@ -11,6 +11,7 @@ tests=mkt.abuse.tests.test_resources,
       mkt.api.tests.test_handlers,
       mkt.api.tests.test_middleware,
       mkt.api.tests.test_oauth,
+      mkt.api.tests.test_paginator,
       mkt.api.tests.test_resources,
       mkt.api.tests.test_serializer,
       mkt.api.tests.test_throttle,