From 5ad664224932c43e3ff2145cdc2d0de63d73d301 Mon Sep 17 00:00:00 2001 From: Andy McKay Date: Mon, 21 May 2012 18:46:41 +0100 Subject: [PATCH] add in put and category support (bug 753570) --- mkt/api/authentication.py | 2 +- mkt/api/base.py | 41 ++++++------------ mkt/api/resources.py | 44 +++++++++++++++++-- mkt/api/tests/test_handlers.py | 78 +++++++++++++++++++++++++++++----- mkt/api/tests/test_oauth.py | 16 +++---- 5 files changed, 128 insertions(+), 53 deletions(-) diff --git a/mkt/api/authentication.py b/mkt/api/authentication.py index 57371f1ff8..cb2909ef89 100644 --- a/mkt/api/authentication.py +++ b/mkt/api/authentication.py @@ -32,8 +32,8 @@ class OwnerAuthorization(Authorization): # If the user on the object and the amo_user match, we are golden. return object.user.pk == request.amo_user.pk -class AppOwnerAuthorization(OwnerAuthorization): +class AppOwnerAuthorization(OwnerAuthorization): def check_owner(self, request, object): # If the user on the object and the amo_user match, we are golden. diff --git a/mkt/api/base.py b/mkt/api/base.py index 801d5955e0..de9a8fb16c 100644 --- a/mkt/api/base.py +++ b/mkt/api/base.py @@ -1,9 +1,7 @@ import json -from tastypie import http from tastypie.bundle import Bundle from tastypie.resources import ModelResource -from tastypie.utils import dict_strip_unicode_keys from translations.fields import PurifiedField, TranslatedField @@ -27,31 +25,6 @@ class MarketplaceResource(ModelResource): return self._build_reverse_url("api_dispatch_detail", kwargs=kwargs) - def post_list(self, request, **kwargs): - # TODO: This has to be request.META['body'] because otherwise this - # will be empty and all the tests will fail. Boo! - deserialized = self.deserialize(request, - request.META.get('body', request.raw_post_data), - format=request.META.get('CONTENT_TYPE', 'application/json')) - # The rest is the same. - deserialized = self.alter_deserialized_detail_data(request, - deserialized) - bundle = self.build_bundle(data=dict_strip_unicode_keys(deserialized), - request=request) - updated_bundle = self.obj_create(bundle, request=request, - **self.remove_api_resource_names(kwargs)) - location = self.get_resource_uri(updated_bundle) - - if not self._meta.always_return_data: - return http.HttpCreated(location=location) - else: - updated_bundle = self.full_dehydrate(updated_bundle) - updated_bundle = self.alter_detail_data_to_serialize(request, - updated_bundle) - return self.create_response(request, updated_bundle, - response_class=http.HttpCreated, - location=location) - @classmethod def should_skip_field(cls, field): # We don't want to skip translated fields. @@ -60,5 +33,15 @@ class MarketplaceResource(ModelResource): return True if getattr(field, 'rel') else False - def form_errors(self, form): - return json.dumps({'error_message': dict(form.errors.items())}) + def form_errors(self, forms): + errors = {} + if not isinstance(forms, list): + forms = [forms] + for f in forms: + if isinstance(f.errors, list): # Cope with formsets. + for e in f.errors: + errors.update(e) + continue + errors.update(dict(f.errors.items())) + + return json.dumps({'error_message': errors}) diff --git a/mkt/api/resources.py b/mkt/api/resources.py index 808b7473bc..6aae2b6cb8 100644 --- a/mkt/api/resources.py +++ b/mkt/api/resources.py @@ -18,6 +18,8 @@ from mkt.api.forms import UploadForm from mkt.developers import tasks from mkt.developers.forms import NewManifestForm from mkt.webapps.models import Webapp +from mkt.submit.forms import AppDetailsBasicForm +from addons.forms import CategoryFormSet log = commonware.log.getLogger('z.api') @@ -80,9 +82,9 @@ class AppResource(MarketplaceResource): class Meta: queryset = Webapp.objects.all().no_transforms() fields = ['id', 'name', 'description', 'homepage', 'status', 'summary', - 'support_email', 'support_url'] + 'support_email', 'support_url', 'categories'] list_allowed_methods = ['post'] - allowed_methods = ['get'] + allowed_methods = ['get', 'put'] always_return_data = True authentication = MarketplaceAuthentication() authorization = AppOwnerAuthorization() @@ -104,7 +106,8 @@ class AppResource(MarketplaceResource): # Create app, user and fetch the icon. bundle.obj = Webapp.from_upload(form.obj, plats) AddonUser(addon=bundle.obj, user=request.amo_user).save() - tasks.fetch_icon.delay(bundle.obj,) + tasks.fetch_icon.delay(bundle.obj) + log.info('App created: %s' % bundle.obj.pk) return bundle def obj_get(self, request=None, **kwargs): @@ -115,10 +118,45 @@ class AppResource(MarketplaceResource): log.info('App retreived: %s' % obj.pk) return obj + def formset(self, data): + cats = data.pop('categories', []) + return {'form-TOTAL_FORMS': 1, + 'form-INITIAL_FORMS': 1, + 'form-MAX_NUM_FORMS': '', + 'form-0-categories': cats} + + @write + @transaction.commit_on_success + def obj_update(self, bundle, request, **kwargs): + try: + bundle.obj = self.get_object_list(bundle.request).get(**kwargs) + except Webapp.DoesNotExist: + raise ImmediateHttpResponse(response=http.HttpNotFound()) + + if not (AppOwnerAuthorization() + .is_authorized(request, object=bundle.obj)): + raise ImmediateHttpResponse(response=http.HttpUnauthorized()) + + bundle.data['slug'] = bundle.data.get('slug', bundle.obj.app_slug) + bundle.data.update(self.formset(bundle.data)) + + forms = [AppDetailsBasicForm(bundle.data, instance=bundle.obj, + request=request), + CategoryFormSet(bundle.data, addon=bundle.obj, + request=request)] + valid = all([f.is_valid() for f in forms]) + if not valid: + raise ValidationError(self.form_errors(forms)) + + forms[0].save(bundle.obj) + forms[1].save() + return bundle + def dehydrate(self, bundle): obj = bundle.obj bundle.data['slug'] = obj.app_slug bundle.data['premium_type'] = amo.ADDON_PREMIUM_API[obj.premium_type] + bundle.data['categories'] = [c.pk for c in obj.categories.all()] return bundle diff --git a/mkt/api/tests/test_handlers.py b/mkt/api/tests/test_handlers.py index 0eceb92d01..7a36fe8d59 100644 --- a/mkt/api/tests/test_handlers.py +++ b/mkt/api/tests/test_handlers.py @@ -27,7 +27,7 @@ class ValidationHandler(BaseOAuth): def create(self): res = self.client.post(self.list_url, - body=json.dumps({'manifest': + data=json.dumps({'manifest': 'http://foo.com'})) self.get_url = ('api_dispatch_detail', {'resource_name': 'validation', @@ -59,13 +59,13 @@ class TestAddValidationHandler(ValidationHandler): assert fetch.called def test_missing(self): - res = self.client.post(self.list_url, body=json.dumps({})) + res = self.client.post(self.list_url, data=json.dumps({})) eq_(res.status_code, 400) eq_(self.get_error(res)['manifest'], ['This field is required.']) def test_bad(self): res = self.client.post(self.list_url, - body=json.dumps({'manifest': 'blurgh'})) + data=json.dumps({'manifest': 'blurgh'})) eq_(res.status_code, 400) eq_(self.get_error(res)['manifest'], ['Enter a valid URL.']) @@ -136,6 +136,11 @@ class CreateHandler(BaseOAuth): self.user = UserProfile.objects.get(pk=2519) self.file = tempfile.NamedTemporaryFile('w', suffix='.webapp').name self.manifest_copy_over(self.file, 'mozball-nice-slug.webapp') + self.categories = [] + for x in range(0, 2): + self.categories.append(Category.objects.create( + name='cat-%s' % x, + type=amo.ADDON_WEBAPP)) def create(self): return FileUpload.objects.create(user=self.user, path=self.file, @@ -152,23 +157,23 @@ class TestAppCreateHandler(CreateHandler, AMOPaths): return Addon.objects.count() def test_verbs(self): - obj = self.create() + self.create() self._allowed_verbs(self.list_url, ['post']) - obj = self.create_app() - self._allowed_verbs(self.get_url, ['get']) + self.create_app() + self._allowed_verbs(self.get_url, ['get', 'put']) def test_not_valid(self): obj = self.create() obj.update(valid=False) res = self.client.post(self.list_url, - body=json.dumps({'manifest': obj.uuid})) + data=json.dumps({'manifest': obj.uuid})) eq_(res.status_code, 400) eq_(self.get_error(res)['manifest'], ['Upload not valid.']) eq_(self.count(), 0) def test_not_there(self): res = self.client.post(self.list_url, - body=json.dumps({'manifest': + data=json.dumps({'manifest': 'some-random-32-character-stringy'})) eq_(res.status_code, 400) eq_(self.get_error(res)['manifest'], ['No upload found.']) @@ -178,14 +183,14 @@ class TestAppCreateHandler(CreateHandler, AMOPaths): obj = self.create() obj.update(user=UserProfile.objects.get(email='admin@mozilla.com')) res = self.client.post(self.list_url, - body=json.dumps({'manifest': obj.uuid})) + data=json.dumps({'manifest': obj.uuid})) eq_(res.status_code, 401) eq_(self.count(), 0) def test_create(self): obj = self.create() res = self.client.post(self.list_url, - body=json.dumps({'manifest': obj.uuid})) + data=json.dumps({'manifest': obj.uuid})) eq_(res.status_code, 201) content = json.loads(res.content) eq_(content['status'], 0) @@ -199,7 +204,7 @@ class TestAppCreateHandler(CreateHandler, AMOPaths): def create_app(self): obj = self.create() res = self.client.post(self.list_url, - body=json.dumps({'manifest': obj.uuid})) + data=json.dumps({'manifest': obj.uuid})) pk = json.loads(res.content)['id'] self.get_url = ('api_dispatch_detail', {'resource_name': 'app', 'pk': pk}) @@ -218,6 +223,57 @@ class TestAppCreateHandler(CreateHandler, AMOPaths): res = self.client.get(self.get_url) eq_(res.status_code, 401) + def base_data(self): + return {'support_email': 'a@a.com', + 'privacy_policy': 'wat', + 'name': 'mozball', + 'categories': [c.pk for c in self.categories], + 'summary': 'wat...'} + + def test_put(self): + app = self.create_app() + res = self.client.put(self.get_url, data=json.dumps(self.base_data())) + eq_(res.status_code, 202) + app = Webapp.objects.get(pk=app.pk) + eq_(app.privacy_policy, 'wat') + + def test_put_categories_worked(self): + app = self.create_app() + res = self.client.put(self.get_url, data=json.dumps(self.base_data())) + eq_(res.status_code, 202) + app = Webapp.objects.get(pk=app.pk) + eq_(set([c.pk for c in app.categories.all()]), + set([c.pk for c in self.categories])) + + def test_dehydrate(self): + self.create_app() + res = self.client.put(self.get_url, data=json.dumps(self.base_data())) + eq_(res.status_code, 202) + res = self.client.get(self.get_url) + eq_(res.status_code, 200) + data = json.loads(res.content) + eq_(set(data['categories']), set([c.pk for c in self.categories])) + eq_(data['premium_type'], 'free') + + def test_put_no_categories(self): + self.create_app() + data = self.base_data() + del data['categories'] + res = self.client.put(self.get_url, data=json.dumps(data)) + eq_(res.status_code, 400) + eq_(self.get_error(res)['categories'], ['This field is required.']) + + def test_put_not_mine(self): + obj = self.create_app() + obj.authors.clear() + res = self.client.put(self.get_url, data='{}') + eq_(res.status_code, 401) + + def test_put_not_there(self): + url = ('api_dispatch_detail', {'resource_name': 'app', 'pk': 123}) + res = self.client.put(url, data='{}') + eq_(res.status_code, 404) + @patch.object(settings, 'SITE_URL', 'http://api/') class TestCategoryHandler(BaseOAuth): diff --git a/mkt/api/tests/test_oauth.py b/mkt/api/tests/test_oauth.py index 04dc6381da..9364dda418 100644 --- a/mkt/api/tests/test_oauth.py +++ b/mkt/api/tests/test_oauth.py @@ -86,31 +86,29 @@ class OAuthClient(Client): HTTP_HOST='api', HTTP_AUTHORIZATION=self.header('DELETE', url)) - def post(self, url, body=''): + def post(self, url, data=''): url = get_absolute_url(url) - return super(OAuthClient, self).post(url, body=body, + return super(OAuthClient, self).post(url, data=data, content_type='application/json', HTTP_HOST='api', HTTP_AUTHORIZATION=self.header('POST', url)) - def put(self, url, body=''): + def put(self, url, data=''): url = get_absolute_url(url) - # Note the processing of data for a PUT is different from a POST - # And this works around some odd support in PUT. - return super(OAuthClient, self).put(url, data=str({}), body=body, + return super(OAuthClient, self).put(url, data=data, content_type='application/json', HTTP_HOST='api', HTTP_AUTHORIZATION=self.header('PUT', url)) - def patch(self, url, body=''): + def patch(self, url, data=''): url = get_absolute_url(url) parsed = urlparse.urlparse(url) r = { - 'CONTENT_LENGTH': len(body), + 'CONTENT_LENGTH': len(data), 'CONTENT_TYPE': 'application/json', 'PATH_INFO': urllib.unquote(parsed[2]), 'REQUEST_METHOD': 'PATCH', - 'wsgi.input': body, + 'wsgi.input': data, 'HTTP_HOST': 'api', 'HTTP_AUTHORIZATION': self.header('PATCH', url) }