From 742e9289cf6312e254117a499676c4eb20cfd871 Mon Sep 17 00:00:00 2001 From: Craig Blaszczyk Date: Fri, 22 Jul 2011 11:17:42 +0100 Subject: [PATCH 01/19] inital work on autodiscover --- djangorestframework/__init__.py | 71 +++++++++++++++++++++++++ djangorestframework/builtins.py | 89 ++++++++++++++++++++++++++++++++ djangorestframework/resources.py | 14 +++-- 3 files changed, 171 insertions(+), 3 deletions(-) create mode 100644 djangorestframework/builtins.py diff --git a/djangorestframework/__init__.py b/djangorestframework/__init__.py index b1ef6ddaa..683375441 100644 --- a/djangorestframework/__init__.py +++ b/djangorestframework/__init__.py @@ -1,3 +1,74 @@ + __version__ = '0.2.3' VERSION = __version__ # synonym + +from djangorestframework.builtins import DjangoRestFrameworkSite + +try: + VERSION = __import__('pkg_resources') \ + .get_distribution('gargoyle').version +except Exception, e: + VERSION = 'unknown' + + +from django.utils.importlib import import_module + +import imp + +__all__ = ('autodiscover','site') + +api = DjangoRestFrameworkSite() + +# A flag to tell us if autodiscover is running. autodiscover will set this to +# True while running, and False when it finishes. +LOADING = False + +def autodiscover(): + """ + Auto-discover INSTALLED_APPS api.py modules and fail silently when + not present. This forces an import on them to register any api bits they + may want. + """ + + # Bail out if autodiscover didn't finish loading from a previous call so + # that we avoid running autodiscover again when the URLconf is loaded by + # the exception handler to resolve the handler500 view. This prevents an + # admin.py module with errors from re-registering models and raising a + # spurious AlreadyRegistered exception. + global LOADING + if LOADING: + return + LOADING = True + + from django.conf import settings + + for app in settings.INSTALLED_APPS: + # For each app, we need to look for a api.py inside that + # app's package. We can't use os.path here -- recall that modules may be + # imported different ways (think zip files) -- so we need to get + # the app's __path__ and look for api.py on that path. + + # Step 1: find out the app's __path__ Import errors here will (and + # should) bubble up, but a missing __path__ (which is legal, but weird) + # fails silently -- apps that do weird things with __path__ might + # need to roll their own api registration. + try: + app_path = import_module(app).__path__ + except (AttributeError, ImportError): + continue + + # Step 2: use imp.find_module to find the app's gargoyle_conditions.py. + # For some # reason imp.find_module raises ImportError if the app can't + # be found # but doesn't actually try to import the module. So skip this + # app if its gargoyle.py doesn't exist + try: + imp.find_module('api', app_path) + except ImportError: + continue + + import_module("%s.api" % app) + print 'aaaaaaaaaaaaa',app + + # autodiscover was successful, reset loading flag. + LOADING = False diff --git a/djangorestframework/builtins.py b/djangorestframework/builtins.py new file mode 100644 index 000000000..7bbb1f4cf --- /dev/null +++ b/djangorestframework/builtins.py @@ -0,0 +1,89 @@ +from djangorestframework.mixins import ListModelMixin, InstanceMixin +from django.conf.urls.defaults import patterns, url + +class DjangoRestFrameworkSite(object): + app_name = 'api' + name = 'api' + + def __init__(self, *args, **kwargs): + self._registry = {} + super(DjangoRestFrameworkSite, self).__init__(*args, **kwargs) + + def register(self, view, resource, prefix=None, resource_name=None): + if resource_name is None: + if hasattr(resource, 'model'): + resource_name = resource.model.__name__.lower() + else: + resource_name = resource.__name__.lower() + + if prefix not in self._registry: + self._registry[prefix] = {} + + if resource_name not in self._registry[prefix]: + self._registry[prefix][resource_name] = [] + + self._registry[prefix][resource_name].append((resource, view)) + + +# def unregister(self, prefix=None, resource_name=None, resource=None): +# """ +# Unregisters a resource. +# """ +# if resource_name is None and resource is not None and \ +# hasattr(resource, 'model'): +# resource_name = resource.model.__name__.lower() +# +# if resource_name is None: +# # do nothing +# return +# +# prefix_registry = self._registry.get(prefix, {}) +# if resource_name in prefix_registry: +# del prefix_registry[resource_name] + + @property + def urls(self): + return self.get_urls(), self.app_name, self.name + + def get_urls(self): + + # Site-wide views. + urlpatterns = patterns('', + + + ) + + # Add in each resource's views. + for prefix in self._registry.keys(): + for resource_name in self._registry[prefix].keys(): + for resource, view in self._registry[prefix][resource_name]: + urlpatterns += self.__get_urlpatterns( + prefix, resource_name, resource, view + ) + + return urlpatterns + + def __get_urlpatterns(self, prefix, resource_name, resource, view): + """ + Calculates the URL pattern for a given resource and view + """ + if prefix is None: + prefix = '' + else: + prefix += '/' + if issubclass(view, ListModelMixin): + urlpatterns = patterns('', + url(r'^%s%s/' % (prefix,resource_name), + view.as_view(resource=resource), + name=resource_name + ) + ) + elif issubclass(view, InstanceMixin): + urlpatterns = patterns('', + url(r'^%s%s/(?P[0-9]+)/$' % (prefix,resource_name), + view.as_view(resource=resource), + name=resource_name + '_change' + ) + ) + + return urlpatterns diff --git a/djangorestframework/resources.py b/djangorestframework/resources.py index f4a2ab147..27f9b0509 100644 --- a/djangorestframework/resources.py +++ b/djangorestframework/resources.py @@ -279,13 +279,13 @@ class ModelResource(FormResource): include = ('url',) - def __init__(self, view=None, depth=None, stack=[], **kwargs): + def __init__(self, view): """ Allow :attr:`form` and :attr:`model` attributes set on the :class:`View` to override the :attr:`form` and :attr:`model` attributes set on the :class:`Resource`. """ - super(ModelResource, self).__init__(view, depth, stack, **kwargs) + super(ModelResource, self).__init__(view) self.model = getattr(view, 'model', None) or self.model @@ -355,7 +355,7 @@ class ModelResource(FormResource): # dis does teh magicks... urlconf = get_urlconf() resolver = get_resolver(urlconf) - + possibilities = resolver.reverse_dict.getlist(self.view_callable[0]) for tuple_item in possibilities: possibility = tuple_item[0] @@ -379,6 +379,14 @@ class ModelResource(FormResource): return reverse(self.view_callable[0], kwargs=instance_attrs) except NoReverseMatch: pass + try: + model_name = instance.__class__.__name__.split('.')[0].lower() + return reverse('%s:%s_change' % ('api', model_name), args=(instance.pk,)) + except NoReverseMatch: + pass + import pdb + pdb.set_trace() + raise _SkipField From fbc483e85491ad02003ded26f3871b6a39c57f27 Mon Sep 17 00:00:00 2001 From: Craig Blaszczyk Date: Fri, 22 Jul 2011 11:59:25 +0100 Subject: [PATCH 02/19] remove bad version guessing mechanism --- djangorestframework/__init__.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/djangorestframework/__init__.py b/djangorestframework/__init__.py index 683375441..2b777e59c 100644 --- a/djangorestframework/__init__.py +++ b/djangorestframework/__init__.py @@ -4,14 +4,6 @@ __version__ = '0.2.3' VERSION = __version__ # synonym from djangorestframework.builtins import DjangoRestFrameworkSite - -try: - VERSION = __import__('pkg_resources') \ - .get_distribution('gargoyle').version -except Exception, e: - VERSION = 'unknown' - - from django.utils.importlib import import_module import imp From 0490ca39ac874af1b2a33fe7eacaef4b61886bae Mon Sep 17 00:00:00 2001 From: Craig Blaszczyk Date: Fri, 22 Jul 2011 12:00:58 +0100 Subject: [PATCH 03/19] add version codes into __all__ --- djangorestframework/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/djangorestframework/__init__.py b/djangorestframework/__init__.py index 2b777e59c..83ca204b7 100644 --- a/djangorestframework/__init__.py +++ b/djangorestframework/__init__.py @@ -8,7 +8,7 @@ from django.utils.importlib import import_module import imp -__all__ = ('autodiscover','site') +__all__ = ('autodiscover','site', '__version__', 'VERSION') api = DjangoRestFrameworkSite() From a3af60651f15e4cd89fcd7c380e70a733d566dda Mon Sep 17 00:00:00 2001 From: Craig Blaszczyk Date: Fri, 22 Jul 2011 12:08:09 +0100 Subject: [PATCH 04/19] remove extra newline, so version finding regexp works --- djangorestframework/__init__.py | 1 - 1 file changed, 1 deletion(-) diff --git a/djangorestframework/__init__.py b/djangorestframework/__init__.py index 83ca204b7..3c316629f 100644 --- a/djangorestframework/__init__.py +++ b/djangorestframework/__init__.py @@ -1,4 +1,3 @@ - __version__ = '0.2.3' VERSION = __version__ # synonym From 8837903f83d1d2a9f185890f170572c2190fd8e9 Mon Sep 17 00:00:00 2001 From: Craig Blaszczyk Date: Fri, 22 Jul 2011 14:23:39 +0100 Subject: [PATCH 05/19] remove print --- djangorestframework/__init__.py | 3 +-- djangorestframework/mixins.py | 1 - djangorestframework/renderers.py | 1 - 3 files changed, 1 insertion(+), 4 deletions(-) diff --git a/djangorestframework/__init__.py b/djangorestframework/__init__.py index 3c316629f..ceb76b3b3 100644 --- a/djangorestframework/__init__.py +++ b/djangorestframework/__init__.py @@ -59,7 +59,6 @@ def autodiscover(): continue import_module("%s.api" % app) - print 'aaaaaaaaaaaaa',app - + # autodiscover was successful, reset loading flag. LOADING = False diff --git a/djangorestframework/mixins.py b/djangorestframework/mixins.py index bb26ad963..6e66cfe45 100644 --- a/djangorestframework/mixins.py +++ b/djangorestframework/mixins.py @@ -607,7 +607,6 @@ class ListModelMixin(object): """ Behavior to list a set of `model` instances on GET requests """ - # NB. Not obvious to me if it would be better to set this on the resource? # # Presumably it's more useful to have on the view, because that way you can diff --git a/djangorestframework/renderers.py b/djangorestframework/renderers.py index aae2cab25..9600ef571 100644 --- a/djangorestframework/renderers.py +++ b/djangorestframework/renderers.py @@ -109,7 +109,6 @@ class JSONRenderer(BaseRenderer): sort_keys = True except (ValueError, TypeError): indent = None - return json.dumps(obj, cls=DateTimeAwareJSONEncoder, indent=indent, sort_keys=sort_keys) From 68487c846b44c47668c8f2a7e411ae80c16df58f Mon Sep 17 00:00:00 2001 From: Craig Blaszczyk Date: Fri, 22 Jul 2011 14:35:05 +0100 Subject: [PATCH 06/19] update list URL to not match instead of the change url --- djangorestframework/builtins.py | 2 +- djangorestframework/serializer.py | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/djangorestframework/builtins.py b/djangorestframework/builtins.py index 7bbb1f4cf..3056b2880 100644 --- a/djangorestframework/builtins.py +++ b/djangorestframework/builtins.py @@ -73,7 +73,7 @@ class DjangoRestFrameworkSite(object): prefix += '/' if issubclass(view, ListModelMixin): urlpatterns = patterns('', - url(r'^%s%s/' % (prefix,resource_name), + url(r'^%s%s/$' % (prefix,resource_name), view.as_view(resource=resource), name=resource_name ) diff --git a/djangorestframework/serializer.py b/djangorestframework/serializer.py index 22efa5ed3..ce5e343de 100644 --- a/djangorestframework/serializer.py +++ b/djangorestframework/serializer.py @@ -143,7 +143,10 @@ class Serializer(object): def get_related_serializer(self, key): - info = _fields_to_dict(self.fields).get(key, None) + fields = _fields_to_dict(self.fields) + fields.update(_fields_to_dict(self.include)) + info = fields.get(key, None) + # If an element in `fields` is a 2-tuple of (str, tuple) # then the second element of the tuple is the fields to From f71f3aa422b528ebddfa817639dee74ba62a003b Mon Sep 17 00:00:00 2001 From: Craig Blaszczyk Date: Tue, 26 Jul 2011 11:57:33 +0100 Subject: [PATCH 07/19] reword error message when user not logged in; add request to permission (so we can check permissions based on the type of request) --- djangorestframework/mixins.py | 1 + djangorestframework/permissions.py | 6 +++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/djangorestframework/mixins.py b/djangorestframework/mixins.py index 6e66cfe45..f3964ad7f 100644 --- a/djangorestframework/mixins.py +++ b/djangorestframework/mixins.py @@ -388,6 +388,7 @@ class AuthMixin(object): user = self.user for permission_cls in self.permissions: permission = permission_cls(self) + permission.request = self.request permission.check_permission(user) diff --git a/djangorestframework/permissions.py b/djangorestframework/permissions.py index 59c5f481f..2540218fd 100644 --- a/djangorestframework/permissions.py +++ b/djangorestframework/permissions.py @@ -26,6 +26,10 @@ _403_FORBIDDEN_RESPONSE = ErrorResponse( {'detail': 'You do not have permission to access this resource. ' + 'You may need to login or otherwise authenticate the request.'}) +_403_NOT_LOGGED_IN_RESPONSE = ErrorResponse( + status.HTTP_403_FORBIDDEN, + {'detail': 'You need to login to access this resource.'}) + _503_SERVICE_UNAVAILABLE = ErrorResponse( status.HTTP_503_SERVICE_UNAVAILABLE, {'detail': 'request was throttled'}) @@ -64,7 +68,7 @@ class IsAuthenticated(BasePermission): def check_permission(self, user): if not user.is_authenticated(): - raise _403_FORBIDDEN_RESPONSE + raise _403_NOT_LOGGED_IN_RESPONSE class IsAdminUser(BasePermission): From 3eba22eea4ce0859600791b3c48caa8b504179a0 Mon Sep 17 00:00:00 2001 From: Craig Blaszczyk Date: Tue, 26 Jul 2011 12:03:27 +0100 Subject: [PATCH 08/19] add permission denied error message --- djangorestframework/permissions.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/djangorestframework/permissions.py b/djangorestframework/permissions.py index 2540218fd..cf131e0b6 100644 --- a/djangorestframework/permissions.py +++ b/djangorestframework/permissions.py @@ -30,6 +30,10 @@ _403_NOT_LOGGED_IN_RESPONSE = ErrorResponse( status.HTTP_403_FORBIDDEN, {'detail': 'You need to login to access this resource.'}) +_403_PERMISSION_DENIED_RESPONSE = ErrorResponse( + status.HTTP_403_FORBIDDEN, + {'detail': 'You do not have permission to access this resource.'}) + _503_SERVICE_UNAVAILABLE = ErrorResponse( status.HTTP_503_SERVICE_UNAVAILABLE, {'detail': 'request was throttled'}) From 7070fa298c86b11dba0a48058c183b844c78619c Mon Sep 17 00:00:00 2001 From: Craig Blaszczyk Date: Tue, 26 Jul 2011 12:10:54 +0100 Subject: [PATCH 09/19] change text of PERMISSION_DENIED response --- djangorestframework/permissions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/djangorestframework/permissions.py b/djangorestframework/permissions.py index cf131e0b6..731f4a203 100644 --- a/djangorestframework/permissions.py +++ b/djangorestframework/permissions.py @@ -28,7 +28,7 @@ _403_FORBIDDEN_RESPONSE = ErrorResponse( _403_NOT_LOGGED_IN_RESPONSE = ErrorResponse( status.HTTP_403_FORBIDDEN, - {'detail': 'You need to login to access this resource.'}) + {'detail': 'You need to login before you can access this resource.'}) _403_PERMISSION_DENIED_RESPONSE = ErrorResponse( status.HTTP_403_FORBIDDEN, From 46ec20be79e37e04add415f8dad5ee3f4211ea49 Mon Sep 17 00:00:00 2001 From: Craig Blaszczyk Date: Tue, 26 Jul 2011 12:58:03 +0100 Subject: [PATCH 10/19] make CSRF check optional on POST requests --- djangorestframework/authentication.py | 3 ++- djangorestframework/builtins.py | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/djangorestframework/authentication.py b/djangorestframework/authentication.py index be22103e6..a40c5e653 100644 --- a/djangorestframework/authentication.py +++ b/djangorestframework/authentication.py @@ -82,6 +82,7 @@ class UserLoggedInAuthentication(BaseAuthentication): """ Use Django's session framework for authentication. """ + check_csrf = True def authenticate(self, request): """ @@ -91,7 +92,7 @@ class UserLoggedInAuthentication(BaseAuthentication): # TODO: Switch this back to request.POST, and let FormParser/MultiPartParser deal with the consequences. if getattr(request, 'user', None) and request.user.is_active: # If this is a POST request we enforce CSRF validation. - if request.method.upper() == 'POST': + if request.method.upper() == 'POST' and self.check_csrf: # Temporarily replace request.POST with .DATA, # so that we use our more generic request parsing request._post = self.view.DATA diff --git a/djangorestframework/builtins.py b/djangorestframework/builtins.py index 3056b2880..122bc01c3 100644 --- a/djangorestframework/builtins.py +++ b/djangorestframework/builtins.py @@ -1,5 +1,6 @@ from djangorestframework.mixins import ListModelMixin, InstanceMixin from django.conf.urls.defaults import patterns, url +from django.views.decorators.csrf import csrf_exempt class DjangoRestFrameworkSite(object): app_name = 'api' From 7814bc9761e340da88f99d9c927893af2fdae926 Mon Sep 17 00:00:00 2001 From: Craig Blaszczyk Date: Fri, 29 Jul 2011 13:03:52 +0100 Subject: [PATCH 11/19] add flag to skip tests --- djangorestframework/tests/__init__.py | 29 ++++++++++++++++++--------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/djangorestframework/tests/__init__.py b/djangorestframework/tests/__init__.py index f664c5c12..c58b88e7f 100644 --- a/djangorestframework/tests/__init__.py +++ b/djangorestframework/tests/__init__.py @@ -1,13 +1,22 @@ """Force import of all modules in this package in order to get the standard test runner to pick up the tests. Yowzers.""" -import os +from django.conf import settings -modules = [filename.rsplit('.', 1)[0] - for filename in os.listdir(os.path.dirname(__file__)) - if filename.endswith('.py') and not filename.startswith('_')] -__test__ = dict() - -for module in modules: - exec("from djangorestframework.tests.%s import __doc__ as module_doc" % module) - exec("from djangorestframework.tests.%s import *" % module) - __test__[module] = module_doc or "" +# Try importing all tests if asked for (then we can run 'em) +try: + skiptest = settings.SKIP_DJANGORESTFRAMEWORK_TESTS +except: + skiptest = True + +if not skiptest: + import os + + modules = [filename.rsplit('.', 1)[0] + for filename in os.listdir(os.path.dirname(__file__)) + if filename.endswith('.py') and not filename.startswith('_')] + __test__ = dict() + + for module in modules: + exec("from djangorestframework.tests.%s import __doc__ as module_doc" % module) + exec("from djangorestframework.tests.%s import *" % module) + __test__[module] = module_doc or "" From 912399c12afd1ffc311be67120b77d31ceb8546b Mon Sep 17 00:00:00 2001 From: Craig Blaszczyk Date: Fri, 29 Jul 2011 13:04:22 +0100 Subject: [PATCH 12/19] change flag default for backwards compatibility --- djangorestframework/tests/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/djangorestframework/tests/__init__.py b/djangorestframework/tests/__init__.py index c58b88e7f..597bd4cd6 100644 --- a/djangorestframework/tests/__init__.py +++ b/djangorestframework/tests/__init__.py @@ -5,7 +5,7 @@ from django.conf import settings try: skiptest = settings.SKIP_DJANGORESTFRAMEWORK_TESTS except: - skiptest = True + skiptest = False if not skiptest: import os From c18d8f02d0352bcc81b124352dbcc82a4cb9f8f3 Mon Sep 17 00:00:00 2001 From: Craig Blaszczyk Date: Mon, 1 Aug 2011 09:56:54 +0100 Subject: [PATCH 13/19] remove pdb --- djangorestframework/resources.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/djangorestframework/resources.py b/djangorestframework/resources.py index 27f9b0509..5dee88356 100644 --- a/djangorestframework/resources.py +++ b/djangorestframework/resources.py @@ -379,13 +379,17 @@ class ModelResource(FormResource): return reverse(self.view_callable[0], kwargs=instance_attrs) except NoReverseMatch: pass + try: - model_name = instance.__class__.__name__.split('.')[0].lower() - return reverse('%s:%s_change' % ('api', model_name), args=(instance.pk,)) + if hasattr(self, 'resource_name'): + resource_name = self.resource_name + else: + resource_name = instance.__class__.__name__.split('.')[0].lower() + return reverse( + '%s:%s_change' % ('api', resource_name), args=(instance.pk,) + ) except NoReverseMatch: pass - import pdb - pdb.set_trace() raise _SkipField From 98ec89a5759f3100a2ae5444d541726063499fe4 Mon Sep 17 00:00:00 2001 From: Craig Blaszczyk Date: Mon, 1 Aug 2011 12:49:54 +0100 Subject: [PATCH 14/19] refactor the way URLs works to be more like the admin site, where there is a subobject for each resource. --- djangorestframework/builtins.py | 76 ++++++++++++++++++++------------- 1 file changed, 46 insertions(+), 30 deletions(-) diff --git a/djangorestframework/builtins.py b/djangorestframework/builtins.py index 122bc01c3..6ce771fcd 100644 --- a/djangorestframework/builtins.py +++ b/djangorestframework/builtins.py @@ -1,7 +1,43 @@ from djangorestframework.mixins import ListModelMixin, InstanceMixin -from django.conf.urls.defaults import patterns, url +from django.conf.urls.defaults import patterns, url, include from django.views.decorators.csrf import csrf_exempt +class ApiEntry(object): + def __init__(self, resource, view, prefix, resource_name): + self.resource, self.view = resource, view + self.prefix, self.resource_name = prefix, resource_name + if self.prefix is None: + self.prefix = '' + + def get_urls(self): + from django.conf.urls.defaults import patterns, url + + if self.prefix == '': + url_prefix = '' + else: + url_prefix = self.prefix + '/' + + if issubclass(self.view, ListModelMixin): + urlpatterns = patterns('', + url(r'^%s%s/$' % (url_prefix, self.resource_name), + self.view.as_view(resource=self.resource), + name=self.resource_name, + ) + ) + elif issubclass(self.view, InstanceMixin): + urlpatterns = patterns('', + url(r'^%s%s/(?P[0-9]+)/$' % (url_prefix, self.resource_name), + self.view.as_view(resource=self.resource), + name=self.resource_name + '_change', + ) + ) + return urlpatterns + + + def urls(self): + return self.get_urls(), 'api', self.prefix + urls = property(urls) + class DjangoRestFrameworkSite(object): app_name = 'api' name = 'api' @@ -17,13 +53,16 @@ class DjangoRestFrameworkSite(object): else: resource_name = resource.__name__.lower() + resource.resource_name = resource_name + if prefix not in self._registry: self._registry[prefix] = {} if resource_name not in self._registry[prefix]: self._registry[prefix][resource_name] = [] - self._registry[prefix][resource_name].append((resource, view)) + api_entry = ApiEntry(resource, view, prefix, resource_name) + self._registry[prefix][resource_name].append(api_entry) # def unregister(self, prefix=None, resource_name=None, resource=None): @@ -57,34 +96,11 @@ class DjangoRestFrameworkSite(object): # Add in each resource's views. for prefix in self._registry.keys(): for resource_name in self._registry[prefix].keys(): - for resource, view in self._registry[prefix][resource_name]: - urlpatterns += self.__get_urlpatterns( - prefix, resource_name, resource, view + for api_entry in self._registry[prefix][resource_name]: + urlpatterns += patterns('', + url(r'^', include(api_entry.urls)) ) - - return urlpatterns - - def __get_urlpatterns(self, prefix, resource_name, resource, view): - """ - Calculates the URL pattern for a given resource and view - """ - if prefix is None: - prefix = '' - else: - prefix += '/' - if issubclass(view, ListModelMixin): - urlpatterns = patterns('', - url(r'^%s%s/$' % (prefix,resource_name), - view.as_view(resource=resource), - name=resource_name - ) - ) - elif issubclass(view, InstanceMixin): - urlpatterns = patterns('', - url(r'^%s%s/(?P[0-9]+)/$' % (prefix,resource_name), - view.as_view(resource=resource), - name=resource_name + '_change' - ) - ) + return urlpatterns + From 0d213f1439d6c88fb555e21af7a746d4669904f5 Mon Sep 17 00:00:00 2001 From: Craig Blaszczyk Date: Tue, 9 Aug 2011 16:52:19 +0100 Subject: [PATCH 15/19] update form to have access to the request object --- djangorestframework/mixins.py | 1 - djangorestframework/resources.py | 23 +++++++++++++++++------ 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/djangorestframework/mixins.py b/djangorestframework/mixins.py index f3964ad7f..5be528cb0 100644 --- a/djangorestframework/mixins.py +++ b/djangorestframework/mixins.py @@ -572,7 +572,6 @@ class UpdateModelMixin(object): else: # Otherwise assume the kwargs uniquely identify the model self.model_instance = model.objects.get(**kwargs) - for (key, val) in self.CONTENT.items(): setattr(self.model_instance, key, val) except model.DoesNotExist: diff --git a/djangorestframework/resources.py b/djangorestframework/resources.py index 5dee88356..badec8104 100644 --- a/djangorestframework/resources.py +++ b/djangorestframework/resources.py @@ -212,9 +212,14 @@ class FormResource(Resource): return None if data is not None or files is not None: - return form(data, files) - - return form() + form_ = form(data, files) + else: + form_ = form() + + if hasattr(self.view, 'request'): + form_.request = self.view.request + + return form_ @@ -333,11 +338,17 @@ class ModelResource(FormResource): if data is not None or files is not None: if issubclass(form, forms.ModelForm) and hasattr(self.view, 'model_instance'): # Bound to an existing model instance - return form(data, files, instance=self.view.model_instance) + form_ = form(data, files, instance=self.view.model_instance) else: - return form(data, files) + form_ = form(data, files) - return form() + else: + form_ = form() + + if hasattr(self.view, 'request'): + form_.request = self.view.request + + return form_ def url(self, instance): From eef995a21c5a55cb997cdba8ba05d170ae8abb47 Mon Sep 17 00:00:00 2001 From: Craig Blaszczyk Date: Mon, 15 Aug 2011 14:59:24 +0100 Subject: [PATCH 16/19] allow primary key to be non-numeric --- djangorestframework/builtins.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/djangorestframework/builtins.py b/djangorestframework/builtins.py index 6ce771fcd..9c029ed81 100644 --- a/djangorestframework/builtins.py +++ b/djangorestframework/builtins.py @@ -26,7 +26,7 @@ class ApiEntry(object): ) elif issubclass(self.view, InstanceMixin): urlpatterns = patterns('', - url(r'^%s%s/(?P[0-9]+)/$' % (url_prefix, self.resource_name), + url(r'^%s%s/(?P[0-9a-zA-Z]+)/$' % (url_prefix, self.resource_name), self.view.as_view(resource=self.resource), name=self.resource_name + '_change', ) From 06ac5209acff6b5e7768534d1da983e8cf8d6e94 Mon Sep 17 00:00:00 2001 From: Craig Blaszczyk Date: Sun, 28 Aug 2011 22:54:02 +0100 Subject: [PATCH 17/19] initial XML parsing checkin --- djangorestframework/mixins.py | 1 - djangorestframework/parsers.py | 62 +++++++++++++++++++++- djangorestframework/tests/__init__.py | 1 + djangorestframework/tests/parsers.py | 29 +++++++++++ djangorestframework/tests/renderers.py | 71 ++++++++++++++++++++++++-- djangorestframework/utils/__init__.py | 4 +- 6 files changed, 162 insertions(+), 6 deletions(-) diff --git a/djangorestframework/mixins.py b/djangorestframework/mixins.py index 9fed61221..9b238d306 100644 --- a/djangorestframework/mixins.py +++ b/djangorestframework/mixins.py @@ -185,7 +185,6 @@ class RequestMixin(object): return (None, None) parsers = as_tuple(self.parsers) - for parser_cls in parsers: parser = parser_cls(self) if parser.can_handle_request(content_type): diff --git a/djangorestframework/parsers.py b/djangorestframework/parsers.py index 2ff64bd3e..235017fdb 100644 --- a/djangorestframework/parsers.py +++ b/djangorestframework/parsers.py @@ -19,6 +19,9 @@ from djangorestframework import status from djangorestframework.compat import yaml from djangorestframework.response import ErrorResponse from djangorestframework.utils.mediatypes import media_type_matches +from xml.etree import ElementTree as ET +import datetime +import decimal __all__ = ( @@ -28,6 +31,7 @@ __all__ = ( 'FormParser', 'MultiPartParser', 'YAMLParser', + 'XMLParser', ) @@ -167,10 +171,66 @@ class MultiPartParser(BaseParser): raise ErrorResponse(status.HTTP_400_BAD_REQUEST, {'detail': 'multipart parse error - %s' % unicode(exc)}) return django_parser.parse() + + + +class XMLParser(BaseParser): + """ + XML parser. + """ + + media_type = 'application/xml' + + def parse(self, stream): + """ + Returns a 2-tuple of `(data, files)`. + + `data` will simply be a string representing the body of the request. + `files` will always be `None`. + """ + data = {} + tree = ET.parse(stream) + for child in tree.getroot().getchildren(): + data[child.tag] = self._type_convert(child.text) + + return (data, None) + + def _type_convert(self, value): + """ + Converts the value returned by the XMl parse into the equivalent + Python type + """ + if value is None: + return value + + try: + return datetime.datetime.strptime(value,'%Y-%m-%d %H:%M:%S') + except ValueError: + pass + + try: + return int(value) + except ValueError: + pass + + + try: + return decimal.Decimal(value) + except decimal.InvalidOperation: + pass + + return value + + DEFAULT_PARSERS = ( JSONParser, FormParser, - MultiPartParser ) + MultiPartParser, + XMLParser + ) if YAMLParser: DEFAULT_PARSERS += ( YAMLParser, ) + + + diff --git a/djangorestframework/tests/__init__.py b/djangorestframework/tests/__init__.py index f664c5c12..65d8ec43c 100644 --- a/djangorestframework/tests/__init__.py +++ b/djangorestframework/tests/__init__.py @@ -11,3 +11,4 @@ for module in modules: exec("from djangorestframework.tests.%s import *" % module) __test__[module] = module_doc or "" +print 'TestXMLParser' in locals().keys() diff --git a/djangorestframework/tests/parsers.py b/djangorestframework/tests/parsers.py index deba688e5..08b963304 100644 --- a/djangorestframework/tests/parsers.py +++ b/djangorestframework/tests/parsers.py @@ -136,6 +136,8 @@ from cgi import parse_qs from django import forms from django.test import TestCase from djangorestframework.parsers import FormParser +from djangorestframework.parsers import XMLParser +import datetime class Form(forms.Form): field1 = forms.CharField(max_length=3) @@ -153,3 +155,30 @@ class TestFormParser(TestCase): (data, files) = parser.parse(stream) self.assertEqual(Form(data).is_valid(), True) + + +class TestXMLParser(TestCase): + def setUp(self): + self.input = StringIO( + '' + '' + '121.0' + 'dasd' + '' + '2011-12-25 12:45:00' + '' + ) + self.data = { + 'field_a': 121, + 'field_b': 'dasd', + 'field_c': None, + 'field_d': datetime.datetime(2011, 12, 25, 12, 45, 00) + + } + + def test_parse(self): + parser = XMLParser(None) + + (data, files) = parser.parse(self.input) + self.assertEqual(data, self.data) + diff --git a/djangorestframework/tests/renderers.py b/djangorestframework/tests/renderers.py index d2046212f..9ae43cdda 100644 --- a/djangorestframework/tests/renderers.py +++ b/djangorestframework/tests/renderers.py @@ -4,13 +4,16 @@ from django.test import TestCase from djangorestframework import status from djangorestframework.compat import View as DjangoView -from djangorestframework.renderers import BaseRenderer, JSONRenderer, YAMLRenderer -from djangorestframework.parsers import JSONParser, YAMLParser +from djangorestframework.renderers import BaseRenderer, JSONRenderer, YAMLRenderer,\ + XMLRenderer +from djangorestframework.parsers import JSONParser, YAMLParser, XMLParser from djangorestframework.mixins import ResponseMixin from djangorestframework.response import Response from djangorestframework.utils.mediatypes import add_media_type_param from StringIO import StringIO +import datetime +from decimal import Decimal DUMMYSTATUS = status.HTTP_200_OK DUMMYCONTENT = 'dummycontent' @@ -223,4 +226,66 @@ if YAMLRenderer: content = renderer.render(obj, 'application/yaml') (data, files) = parser.parse(StringIO(content)) - self.assertEquals(obj, data) \ No newline at end of file + self.assertEquals(obj, data) + + +class XMLRendererTestCase(TestCase): + """ + Tests specific to the JSON Renderer + """ + + def test_render_string(self): + """ + Test XML rendering. + """ + renderer = XMLRenderer(None) + content = renderer.render({'field': 'astring'}, 'application/xml') + self.assertXMLContains(content, 'astring') + + def test_render_integer(self): + """ + Test XML rendering. + """ + renderer = XMLRenderer(None) + content = renderer.render({'field': 111}, 'application/xml') + self.assertXMLContains(content, '111') + + def test_render_datetime(self): + """ + Test XML rendering. + """ + renderer = XMLRenderer(None) + content = renderer.render({ + 'field': datetime.datetime(2011, 12, 25, 12, 45, 00) + }, 'application/xml') + self.assertXMLContains(content, '2011-12-25 12:45:00') + + def test_render_float(self): + """ + Test XML rendering. + """ + renderer = XMLRenderer(None) + content = renderer.render({'field': 123.4}, 'application/xml') + self.assertXMLContains(content, '123.4') + + def test_render_decimal(self): + """ + Test XML rendering. + """ + renderer = XMLRenderer(None) + content = renderer.render({'field': Decimal('111.2')}, 'application/xml') + self.assertXMLContains(content, '111.2') + + def test_render_none(self): + """ + Test XML rendering. + """ + renderer = XMLRenderer(None) + content = renderer.render({'field': None}, 'application/xml') + self.assertXMLContains(content, '') + + + def assertXMLContains(self, xml, string): + self.assertTrue(xml.startswith('\n')) + self.assertTrue(xml.endswith('')) + self.assertTrue(string in xml, '%r not in %r' % (string, xml)) \ No newline at end of file diff --git a/djangorestframework/utils/__init__.py b/djangorestframework/utils/__init__.py index 99f9724ce..1f1a08661 100644 --- a/djangorestframework/utils/__init__.py +++ b/djangorestframework/utils/__init__.py @@ -150,7 +150,9 @@ class XMLRenderer(): xml.startElement(key, {}) self._to_xml(xml, value) xml.endElement(key) - + elif data is None: + # Don't output any value + pass else: xml.characters(smart_unicode(data)) From e97cafabb6864b03e469a9cd1237630ff696178e Mon Sep 17 00:00:00 2001 From: Craig Blaszczyk Date: Fri, 2 Sep 2011 11:57:48 +0100 Subject: [PATCH 18/19] update to pass data and files as kwargs --- djangorestframework/resources.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/djangorestframework/resources.py b/djangorestframework/resources.py index badec8104..8ac31545a 100644 --- a/djangorestframework/resources.py +++ b/djangorestframework/resources.py @@ -212,7 +212,7 @@ class FormResource(Resource): return None if data is not None or files is not None: - form_ = form(data, files) + form_ = form(data=data, files=files) else: form_ = form() @@ -338,9 +338,9 @@ class ModelResource(FormResource): if data is not None or files is not None: if issubclass(form, forms.ModelForm) and hasattr(self.view, 'model_instance'): # Bound to an existing model instance - form_ = form(data, files, instance=self.view.model_instance) + form_ = form(data=data, files=files, instance=self.view.model_instance) else: - form_ = form(data, files) + form_ = form(data=data, files=files) else: form_ = form() From 335583bb111c1902491e6d519728139d061aa9bd Mon Sep 17 00:00:00 2001 From: Craig Blaszczyk Date: Mon, 12 Sep 2011 16:05:15 +0100 Subject: [PATCH 19/19] make unit tests run again --- djangorestframework/builtins.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/djangorestframework/builtins.py b/djangorestframework/builtins.py index 9c029ed81..55d257ca5 100644 --- a/djangorestframework/builtins.py +++ b/djangorestframework/builtins.py @@ -1,4 +1,4 @@ -from djangorestframework.mixins import ListModelMixin, InstanceMixin + from django.conf.urls.defaults import patterns, url, include from django.views.decorators.csrf import csrf_exempt @@ -10,6 +10,7 @@ class ApiEntry(object): self.prefix = '' def get_urls(self): + from djangorestframework.mixins import ListModelMixin, InstanceMixin from django.conf.urls.defaults import patterns, url if self.prefix == '':