diff --git a/docs/manual/webapi/2.0/errors/229-file-already-exists.rst b/docs/manual/webapi/2.0/errors/229-file-already-exists.rst new file mode 100644 index 0000000000000000000000000000000000000000..f6ac146789580b0664b57bd0546bf6b17296d2ac --- /dev/null +++ b/docs/manual/webapi/2.0/errors/229-file-already-exists.rst @@ -0,0 +1,6 @@ +.. webapi-error:: + :title: File Already Exists + :instance: reviewboard.webapi.errors.FILE_ALREADY_EXISTS + + The attempt to update the file attachment has failed because the file + attachment already has a file associated with it. \ No newline at end of file diff --git a/docs/manual/webapi/2.0/errors/index.rst b/docs/manual/webapi/2.0/errors/index.rst index 854940f929db3a7bfacc208015b1d4a4dc3e3487..bd77d30bf62f7cbe2c9b0758d8c86928d29cce87 100644 --- a/docs/manual/webapi/2.0/errors/index.rst +++ b/docs/manual/webapi/2.0/errors/index.rst @@ -41,3 +41,4 @@ Errors 226-user-query-error 227-commit-id-already-exists 228-token-generation-failed + 229-file-already-exists diff --git a/reviewboard/attachments/evolutions/__init__.py b/reviewboard/attachments/evolutions/__init__.py index d5f274dc03f0368796b3fe197d91b2d9ab22cc3a..a3ae7d9e4311e570d56eb5fc00ec59fd9eaf7bef 100644 --- a/reviewboard/attachments/evolutions/__init__.py +++ b/reviewboard/attachments/evolutions/__init__.py @@ -7,4 +7,5 @@ SEQUENCE = [ 'file_attachment_repo_path_no_index', 'file_attachment_repo_revision_max_length_64', 'file_attachment_revision', + 'file_attachment_ownership', ] diff --git a/reviewboard/attachments/evolutions/file_attachment_ownership.py b/reviewboard/attachments/evolutions/file_attachment_ownership.py new file mode 100644 index 0000000000000000000000000000000000000000..0a55de541b4d0d6d73c87a336265afce4b8ec2ea --- /dev/null +++ b/reviewboard/attachments/evolutions/file_attachment_ownership.py @@ -0,0 +1,15 @@ +from __future__ import unicode_literals + +from django_evolution.mutations import AddField, ChangeField +from django.db import models + + +MUTATIONS = [ + AddField('FileAttachment', 'user', models.ForeignKey, null=True, + related_model='auth.User'), + AddField('FileAttachment', 'local_site', models.ForeignKey, null=True, + related_model='site.LocalSite'), + AddField('FileAttachment', 'uuid', models.CharField, max_length=255, + initial=None, null=True), + ChangeField('FileAttachment', 'file', initial=None, null=True), +] diff --git a/reviewboard/attachments/forms.py b/reviewboard/attachments/forms.py index 66a28fa75bf1a79c8a1aaffff4febefd02a77efe..b99965b0e2306b44b9d7ef059d694a84aa33f863 100644 --- a/reviewboard/attachments/forms.py +++ b/reviewboard/attachments/forms.py @@ -2,12 +2,11 @@ from __future__ import unicode_literals from uuid import uuid4 import os -import subprocess from django import forms from django.utils import timezone -from djblets.util.filesystem import is_exe_in_path +from reviewboard.attachments.mimetypes import get_uploaded_file_mimetype from reviewboard.attachments.models import (FileAttachment, FileAttachmentHistory) from reviewboard.reviews.models import (ReviewRequestDraft, @@ -19,9 +18,6 @@ class UploadFileForm(forms.Form): A file takes a path argument and optionally a caption. """ - DEFAULT_MIMETYPE = 'application/octet-stream' - READ_BUF_SIZE = 1024 - caption = forms.CharField(required=False) path = forms.FileField(required=True) attachment_history = forms.ModelChoiceField( @@ -49,21 +45,8 @@ class UploadFileForm(forms.Form): file = self.files['path'] caption = self.cleaned_data['caption'] or file.name - # There are several things that can go wrong with browser-provided - # mimetypes. In one case (bug 3427), Firefox on Linux Mint was - # providing a mimetype that looked like 'text/text/application/pdf', - # which is unparseable. IE also has a habit of setting any unknown file - # type to 'application/octet-stream', rather than just choosing not to - # provide a mimetype. In the case where what we get from the browser - # is obviously wrong, try to guess. - if (file.content_type and - len(file.content_type.split('/')) == 2 and - file.content_type != 'application/octet-stream'): - mimetype = file.content_type - else: - mimetype = self._guess_mimetype(file) - - filename = '%s__%s' % (uuid4(), file.name) + mimetype = get_uploaded_file_mimetype(file) + filename = get_unique_filename(file.name) if self.cleaned_data['attachment_history'] is None: # This is a new file: create a new FileAttachmentHistory for it @@ -121,45 +104,65 @@ class UploadFileForm(forms.Form): return file_attachment - def _guess_mimetype(self, file): - """Guess the mimetype of an uploaded file. - - Uploaded files don't necessarily have valid mimetypes provided, - so attempt to guess them when they're blank. - - This only works if `file` is in the path. If it's not, or guessing - fails, we fall back to a mimetype of application/octet-stream. - """ - if not is_exe_in_path('file'): - return self.DEFAULT_MIMETYPE - - # The browser didn't know what this was, so we'll need to do - # some guess work. If we have 'file' available, use that to - # figure it out. - p = subprocess.Popen(['file', '--mime-type', '-b', '-'], - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - stdin=subprocess.PIPE) - - # Write the content from the file until file has enough data to - # make a determination. - for chunk in file.chunks(): - try: - p.stdin.write(chunk) - except IOError: - # file closed, so we hopefully have an answer. - break - p.stdin.close() - ret = p.wait() +class UploadUserFileForm(forms.Form): + """A form that handles uploading of user files. + + A file takes a path argument and optionally a caption. + """ + caption = forms.CharField(required=False) + path = forms.FileField(required=False) + + def create(self, user, local_site=None): + file = self.files.get('path') - if ret == 0: - mimetype = p.stdout.read().strip() + if file: + mimetype = get_uploaded_file_mimetype(file) + filename = get_unique_filename(file.name) - # Reset the read position so we can properly save this. - file.seek(0) + attachment_kwargs = { + 'caption': self.cleaned_data['caption'] or file.name, + 'uuid': uuid4(), + 'orig_filename': os.path.basename(file.name), + 'mimetype': mimetype, + 'user': user, + 'local_site': local_site, + } - return mimetype or self.DEFAULT_MIMETYPE + file_attachment = FileAttachment(**attachment_kwargs) + file_attachment.file.save(filename, file, save=True) + else: + attachment_kwargs = { + 'caption': self.cleaned_data['caption'] or '', + 'uuid': uuid4(), + 'user': user, + 'local_site': local_site, + } + + file_attachment = FileAttachment(**attachment_kwargs) + + file_attachment.save() + + return file_attachment + + def update(self, file_attachment): + caption = self.cleaned_data['caption'] + file = self.files.get('path') + + if caption: + file_attachment.caption = caption + + if file: + mimetype = get_uploaded_file_mimetype(file) + filename = get_unique_filename(file.name) + + file_attachment.mimetype = mimetype + file_attachment.orig_filename = os.path.basename(file.name) + file_attachment.file.save(filename, file, save=True) + + file_attachment.save() + + return file_attachment class CommentFileForm(forms.Form): @@ -181,3 +184,11 @@ class CommentFileForm(forms.Form): draft.save() return comment + + +def get_unique_filename(filename): + """Creates a unique filename. + + Creates a unique filename by concatenating a UUID with the given filename. + """ + return '%s__%s' % (uuid4(), filename) diff --git a/reviewboard/attachments/mimetypes.py b/reviewboard/attachments/mimetypes.py index 5b7278824f982ee550922690a77314574c1216f1..042b88f3e8058c83b7d6dac2798b846adea6e4ca 100644 --- a/reviewboard/attachments/mimetypes.py +++ b/reviewboard/attachments/mimetypes.py @@ -2,12 +2,14 @@ from __future__ import unicode_literals import logging import os +import subprocess from django.contrib.staticfiles.templatetags.staticfiles import static from django.utils.html import escape from django.utils.encoding import smart_str, force_unicode from django.utils.safestring import mark_safe from djblets.cache.backend import cache_memoize +from djblets.util.filesystem import is_exe_in_path from djblets.util.templatetags.djblets_images import thumbnail from pipeline.storage import default_storage from pygments import highlight @@ -20,6 +22,70 @@ import mimeparse _registered_mimetype_handlers = [] +DEFAULT_MIMETYPE = 'application/octet-stream' + + +def guess_mimetype(uploaded_file): + """Guess the mimetype of an uploaded file. + + Uploaded files don't necessarily have valid mimetypes provided, + so attempt to guess them when they're blank. + + This only works if `file` is in the path. If it's not, or guessing + fails, we fall back to a mimetype of application/octet-stream. + """ + if not is_exe_in_path('file'): + return DEFAULT_MIMETYPE + + # The browser didn't know what this was, so we'll need to do + # some guess work. If we have 'file' available, use that to + # figure it out. + p = subprocess.Popen(['file', '--mime-type', '-b', '-'], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + stdin=subprocess.PIPE) + + # Write the content from the file until file has enough data to + # make a determination. + for chunk in uploaded_file.chunks(): + try: + p.stdin.write(chunk) + except IOError: + # file closed, so we hopefully have an answer. + break + + p.stdin.close() + ret = p.wait() + + if ret == 0: + mimetype = p.stdout.read().strip() + + # Reset the read position so we can properly save this. + uploaded_file.seek(0) + + return mimetype or DEFAULT_MIMETYPE + + +def get_uploaded_file_mimetype(uploaded_file): + """Returns the mimetype of a file that was uploaded. + + There are several things that can go wrong with browser-provided + mimetypes. In one case (bug 3427), Firefox on Linux Mint was + providing a mimetype that looked like 'text/text/application/pdf', + which is unparseable. IE also has a habit of setting any unknown file + type to 'application/octet-stream', rather than just choosing not to + provide a mimetype. In the case where what we get from the browser + is obviously wrong, try to guess. + """ + if (uploaded_file.content_type and + len(uploaded_file.content_type.split('/')) == 2 and + uploaded_file.content_type != 'application/octet-stream'): + mimetype = uploaded_file.content_type + else: + mimetype = guess_mimetype(uploaded_file) + + return mimetype + def register_mimetype_handler(handler): """Registers a MimetypeHandler class. diff --git a/reviewboard/attachments/models.py b/reviewboard/attachments/models.py index 6057942695e01b2ee93a5bdb21385b1c028eb82b..971dc46ae74b920920b44ccdb7bdf15cfeba0769 100644 --- a/reviewboard/attachments/models.py +++ b/reviewboard/attachments/models.py @@ -3,6 +3,7 @@ from __future__ import unicode_literals import logging import os +from django.contrib.auth.models import User from django.core.exceptions import ObjectDoesNotExist from django.db import models from django.db.models import Max @@ -15,6 +16,7 @@ from reviewboard.attachments.managers import FileAttachmentManager from reviewboard.attachments.mimetypes import MimetypeHandler from reviewboard.diffviewer.models import FileDiff from reviewboard.scmtools.models import Repository +from reviewboard.site.models import LocalSite class FileAttachmentHistory(models.Model): @@ -62,8 +64,22 @@ class FileAttachment(models.Model): max_length=256, blank=True) orig_filename = models.CharField(_('original filename'), max_length=256, blank=True, null=True) + user = models.ForeignKey(User, + blank=True, + null=True, + related_name="file_attachments") + + local_site = models.ForeignKey(LocalSite, + blank=True, + null=True, + related_name="file_attachments") + + uuid = models.CharField(_("uuid"), max_length=255, blank=True) + file = models.FileField(_("file"), max_length=512, + blank=True, + null=True, upload_to=os.path.join('uploaded', 'files', '%Y', '%m', '%d')) mimetype = models.CharField(_('mimetype'), max_length=256, blank=True) @@ -142,7 +158,12 @@ class FileAttachment(models.Model): # Older versions of Review Board didn't store the original filename, # instead just using the FileField's name. Newer versions have # a dedicated filename field. - return self.orig_filename or os.path.basename(self.file.name) + if self.file: + alt = os.path.basename(self.file.name) + else: + alt = None + + return self.orig_filename or alt @property def display_name(self): @@ -204,6 +225,9 @@ class FileAttachment(models.Model): return self._comments def get_absolute_url(self): + if not self.file: + return None + url = self.file.url if url.startswith('http:') or url.startswith('https:'): @@ -211,5 +235,26 @@ class FileAttachment(models.Model): return build_server_url(url) + def is_accessible_by(self, user): + """Returns whether or not the user has access to this FileAttachment. + + This checks that the user has access to the LocalSite if the attachment + is associated with a local site. This is only applicable for user owned + file attachments. + """ + return (self.user and user.is_authenticated() and + (user.is_superuser or self.user == user) and + (not self.local_site or + self.local_site.is_accessible_by(user))) + + def is_mutable_by(self, user): + """Returns whether or not a user can modify this FileAttachment. + + This checks that the user is either a superuser or the owner of the + file attachment. This is only applicable for user owned file + attachments. + """ + return self.user and (user.is_superuser or self.user == user) + class Meta: get_latest_by = 'attachment_revision' diff --git a/reviewboard/attachments/tests.py b/reviewboard/attachments/tests.py index 9863df68f309781c0af217e5c3aa3e237357c0a0..be5425f2f5aa36529436b7738315c39820819806 100644 --- a/reviewboard/attachments/tests.py +++ b/reviewboard/attachments/tests.py @@ -11,7 +11,7 @@ from djblets.testing.decorators import add_fixtures from kgb import SpyAgency from reviewboard import initialize -from reviewboard.attachments.forms import UploadFileForm +from reviewboard.attachments.forms import UploadFileForm, UploadUserFileForm from reviewboard.attachments.mimetypes import (MimetypeHandler, register_mimetype_handler, unregister_mimetype_handler) @@ -20,6 +20,7 @@ from reviewboard.attachments.models import (FileAttachment, from reviewboard.diffviewer.models import DiffSet, DiffSetHistory, FileDiff from reviewboard.reviews.models import ReviewRequest from reviewboard.scmtools.core import PRE_CREATION +from reviewboard.site.models import LocalSite from reviewboard.testing import TestCase @@ -236,6 +237,106 @@ class FileAttachmentTests(BaseFileAttachmentTestCase): '') +class UserFileAttachmentTests(BaseFileAttachmentTestCase): + fixtures = ['test_users'] + + def test_user_file_add_file_after_create(self): + """Testing user FileAttachment create without initial file and + adding file through update + """ + user = User.objects.get(username='doc') + + form = UploadUserFileForm(files={}) + self.assertTrue(form.is_valid()) + + file_attachment = form.create(user) + self.assertFalse(file_attachment.file) + self.assertEqual(file_attachment.user, user) + + uploaded_file = self.make_uploaded_file() + form = UploadUserFileForm(files={ + 'path': uploaded_file, + }) + self.assertTrue(form.is_valid()) + + file_attachment = form.update(file_attachment) + + self.assertTrue(os.path.basename(file_attachment.file.name).endswith( + '__trophy.png')) + self.assertEqual(file_attachment.mimetype, 'image/png') + + def test_user_file_with_upload_file(self): + """Testing user FileAttachment create with initial file""" + user = User.objects.get(username='doc') + uploaded_file = self.make_uploaded_file() + + form = UploadUserFileForm(files={ + 'path': uploaded_file, + }) + self.assertTrue(form.is_valid()) + + file_attachment = form.create(user) + + self.assertEqual(file_attachment.user, user) + self.assertTrue(os.path.basename(file_attachment.file.name).endswith( + '__trophy.png')) + self.assertEqual(file_attachment.mimetype, 'image/png') + + @add_fixtures(['test_site']) + def test_user_file_local_sites(self): + """Testing user FileAttachment create with local site""" + user = User.objects.get(username='doc') + local_site = LocalSite.objects.get(name='local-site-1') + + form = UploadUserFileForm(files={}) + self.assertTrue(form.is_valid()) + + file_attachment = form.create(user, local_site) + + self.assertEqual(file_attachment.user, user) + self.assertEqual(file_attachment.local_site, local_site) + + @add_fixtures(['test_site']) + def test_user_file_is_accessible_by(self): + """Testing user FileAttachment.is_accessible_by""" + creating_user = User.objects.get(username='doc') + admin_user = User.objects.get(username='admin') + same_site_user = User.objects.get(username='dopey') + different_site_user = User.objects.get(username='grumpy') + + local_site = LocalSite.objects.get(name='local-site-1') + local_site.users.add(same_site_user) + + form = UploadUserFileForm(files={}) + self.assertTrue(form.is_valid()) + file_attachment = form.create(creating_user, local_site) + + self.assertTrue(file_attachment.is_accessible_by(admin_user)) + self.assertTrue(file_attachment.is_accessible_by(creating_user)) + self.assertFalse(file_attachment.is_accessible_by(same_site_user)) + self.assertFalse(file_attachment.is_accessible_by(different_site_user)) + + @add_fixtures(['test_site']) + def test_user_file_is_mutably_by(self): + """Testing user FileAttachment.is_mutable_by""" + creating_user = User.objects.get(username='doc') + admin_user = User.objects.get(username='admin') + same_site_user = User.objects.get(username='dopey') + different_site_user = User.objects.get(username='grumpy') + + local_site = LocalSite.objects.get(name='local-site-1') + local_site.users.add(same_site_user) + + form = UploadUserFileForm(files={}) + self.assertTrue(form.is_valid()) + file_attachment = form.create(creating_user, local_site) + + self.assertTrue(file_attachment.is_mutable_by(admin_user)) + self.assertTrue(file_attachment.is_mutable_by(creating_user)) + self.assertFalse(file_attachment.is_mutable_by(same_site_user)) + self.assertFalse(file_attachment.is_mutable_by(different_site_user)) + + class MimetypeTest(MimetypeHandler): supported_mimetypes = ['test/*'] diff --git a/reviewboard/attachments/views.py b/reviewboard/attachments/views.py new file mode 100644 index 0000000000000000000000000000000000000000..435505be47479239d58d35b2623271cea83931cf --- /dev/null +++ b/reviewboard/attachments/views.py @@ -0,0 +1,25 @@ +from __future__ import unicode_literals + +from django.contrib.auth.models import User +from django.shortcuts import get_object_or_404, redirect + +from reviewboard.attachments.models import FileAttachment +from reviewboard.site.decorators import check_local_site_access + + +@check_local_site_access +def view_user_file_attachment(request, + file_attachment_uuid, + username, + local_site=None): + """Redirects to the file attachment's url given a file attachment's + uuid.""" + user = get_object_or_404(User, username=username) + + file_attachment = get_object_or_404(FileAttachment, + uuid=file_attachment_uuid, + user=user, + local_site=local_site, + file__isnull=False) + + return redirect(file_attachment) diff --git a/reviewboard/testing/testcase.py b/reviewboard/testing/testcase.py index 4f8f56bc91952896b73e9ae57c22858ca321af8c..392f12a3ae91a2de90d8ce27863471e662c2140f 100644 --- a/reviewboard/testing/testcase.py +++ b/reviewboard/testing/testcase.py @@ -208,18 +208,11 @@ class TestCase(DjbletsTestCase): The FileAttachment is tied to the given ReviewRequest. It's populated with default data that can be overridden by the caller. """ - file_attachment = FileAttachment( + file_attachment = self._create_base_file_attachment( caption=caption, orig_filename=orig_filename, - mimetype='image/png', **kwargs) - filename = os.path.join(settings.STATIC_ROOT, 'rb', 'images', - 'trophy.png') - - with open(filename, 'r') as f: - file_attachment.file.save(filename, File(f), save=True) - if draft: review_request_draft = ReviewRequestDraft.create(review_request) review_request_draft.file_attachments.add(file_attachment) @@ -228,6 +221,33 @@ class TestCase(DjbletsTestCase): return file_attachment + def create_user_file_attachment(self, user, + caption='My Caption', + with_local_site=False, + local_site_name=None, + local_site=None, + has_file=False, + orig_filename='filename.png', + **kwargs): + """Creates a user FileAttachment for testing. + + The FileAttachment is tied to the given User. It's populated + with default data that can be overridden by the caller. + Notably, by default the FileAttachment will be created without a file + or a local_site + """ + file_attachment = self._create_base_file_attachment( + caption=caption, + user=user, + has_file=has_file, + orig_filename=orig_filename, + with_local_site=with_local_site, + local_site_name=local_site_name, + local_site=local_site, + **kwargs) + + return file_attachment + def create_file_attachment_comment(self, review, file_attachment, text='My comment', issue_opened=False, extra_fields=None, reply_to=None): @@ -498,6 +518,44 @@ class TestCase(DjbletsTestCase): return comment + def _create_base_file_attachment(self, + caption='My Caption', + orig_filename='filename.png', + has_file=True, + user=None, + with_local_site=False, + local_site_name=None, + local_site=None, + **kwargs): + """Creates a FileAttachment object with the given parameters. + + When creating a FileAttachment that will be associated to a review + request, a user and local_site should not be specified. + """ + if with_local_site: + local_site = self.get_local_site(name=local_site_name) + + file_attachment = FileAttachment( + caption=caption, + user=user, + uuid='test-uuid', + local_site=local_site, + **kwargs) + + if has_file: + filename = os.path.join(settings.STATIC_ROOT, 'rb', 'images', + 'trophy.png') + + file_attachment.orig_filename = orig_filename + file_attachment.mimetype = 'image/png' + + with open(filename, 'r') as f: + file_attachment.file.save(filename, File(f), save=True) + + file_attachment.save() + + return file_attachment + def _fixture_setup(self): """Set up fixtures for unit tests. diff --git a/reviewboard/urls.py b/reviewboard/urls.py index ec0d1ce099ac4efc3d63a4eeace515b5007c800b..4d3340427ec12b0a2bee9479860823a96a3a743a 100644 --- a/reviewboard/urls.py +++ b/reviewboard/urls.py @@ -74,6 +74,19 @@ if settings.DEBUG and not settings.PRODUCTION: TemplateView.as_view(template_name='js/tests.html')), ) +user_urlpatterns = patterns( + '', + + # User info box + url(r"^infobox/$", + 'reviewboard.reviews.views.user_infobox', name="user-infobox"), + + # User file attachments + url(r"file-attachments/(?P[a-zA-Z0-9\-]+)/$$", + 'reviewboard.attachments.views.view_user_file_attachment', + name='view-user-file-attachment'), +) + localsite_urlpatterns = patterns( '', @@ -86,9 +99,9 @@ localsite_urlpatterns = patterns( url(r'^support/$', 'reviewboard.admin.views.support_redirect', name="support"), - # User info box - url(r"^users/(?P[A-Za-z0-9@_\-\.'\+]+)/infobox/$", - 'reviewboard.reviews.views.user_infobox', name="user-infobox"), + # Users + (r"^users/(?P[A-Za-z0-9@_\-\.'\+]+)/", + include(user_urlpatterns)), ) localsite_urlpatterns += datagrid_urlpatterns diff --git a/reviewboard/webapi/errors.py b/reviewboard/webapi/errors.py index 75e640c0f663e2216487d5bc6ec985348a044d38..7ef33d0ef670e989f7c71d90fb31bce0d61b3a84 100644 --- a/reviewboard/webapi/errors.py +++ b/reviewboard/webapi/errors.py @@ -156,3 +156,8 @@ TOKEN_GENERATION_FAILED = WebAPIError( 228, 'There was an error generating the API token. Please try again.', http_status=500) # 500 Internal Server Error. + +FILE_ALREADY_EXISTS = WebAPIError( + 229, + "There is already a file that is associated with this file attachment.", + http_status=400) # 400 Bad Request diff --git a/reviewboard/webapi/resources/base_file_attachment.py b/reviewboard/webapi/resources/base_file_attachment.py index 6b00e73818c87b4e87f338f94f87f4e4401bd61c..a69e15c380434addd97d303a6d04066825366050 100644 --- a/reviewboard/webapi/resources/base_file_attachment.py +++ b/reviewboard/webapi/resources/base_file_attachment.py @@ -1,22 +1,9 @@ from __future__ import unicode_literals -import logging - -from django.core.exceptions import ObjectDoesNotExist, PermissionDenied -from django.db.models import Q from django.utils import six -from djblets.webapi.decorators import (webapi_login_required, - webapi_response_errors, - webapi_request_fields) -from djblets.webapi.errors import (DOES_NOT_EXIST, INVALID_FORM_DATA, - NOT_LOGGED_IN, PERMISSION_DENIED) -from reviewboard.attachments.forms import UploadFileForm from reviewboard.attachments.models import FileAttachment -from reviewboard.site.urlresolvers import local_site_reverse from reviewboard.webapi.base import WebAPIResource -from reviewboard.webapi.decorators import webapi_check_local_site -from reviewboard.webapi.resources import resources class BaseFileAttachmentResource(WebAPIResource): @@ -36,15 +23,6 @@ class BaseFileAttachmentResource(WebAPIResource): 'type': six.text_type, 'description': "The name of the file.", }, - 'url': { - 'type': six.text_type, - 'description': "The URL of the file, for downloading purposes. " - "If this is not an absolute URL, then it's " - "relative to the Review Board server's URL. " - "This is deprecated and will be removed in a " - "future version.", - 'deprecated_in': '2.0', - }, 'absolute_url': { 'type': six.text_type, 'description': "The absolute URL of the file, for downloading " @@ -63,10 +41,6 @@ class BaseFileAttachmentResource(WebAPIResource): 'type': six.text_type, 'description': 'A thumbnail representing this file.', }, - 'review_url': { - 'type': six.text_type, - 'description': 'The URL to a review UI for this file.', - }, 'attachment_history_id': { 'type': int, 'description': 'ID of the corresponding FileAttachmentHistory.', @@ -77,34 +51,6 @@ class BaseFileAttachmentResource(WebAPIResource): uri_object_key = 'file_attachment_id' autogenerate_etags = True - def get_queryset(self, request, is_list=False, *args, **kwargs): - review_request = resources.review_request.get_object( - request, *args, **kwargs) - - q = (Q(review_request=review_request) & - Q(added_in_filediff__isnull=True) & - Q(repository__isnull=True)) - - if not is_list: - q = q | Q(inactive_review_request=review_request) - - if request.user == review_request.submitter: - try: - draft = resources.review_request_draft.get_object( - request, *args, **kwargs) - - q = q | Q(drafts=draft) - - if not is_list: - q = q | Q(inactive_drafts=draft) - except ObjectDoesNotExist: - pass - - return self.model.objects.filter(q) - - def serialize_url_field(self, obj, **kwargs): - return obj.get_absolute_url() - def serialize_absolute_url_field(self, obj, request, **kwargs): return request.build_absolute_uri(obj.get_absolute_url()) @@ -116,203 +62,3 @@ class BaseFileAttachmentResource(WebAPIResource): # are changing an existing one. return obj.caption or obj.draft_caption - - def serialize_review_url_field(self, obj, **kwargs): - if obj.review_ui: - review_request = obj.get_review_request() - if review_request.local_site_id: - local_site_name = review_request.local_site.name - else: - local_site_name = None - - return local_site_reverse( - 'file-attachment', local_site_name=local_site_name, - kwargs={ - 'review_request_id': review_request.display_id, - 'file_attachment_id': obj.pk, - }) - - return '' - - def has_access_permissions(self, request, obj, *args, **kwargs): - return obj.get_review_request().is_accessible_by(request.user) - - def has_modify_permissions(self, request, obj, *args, **kwargs): - return obj.get_review_request().is_mutable_by(request.user) - - def has_delete_permissions(self, request, obj, *args, **kwargs): - return obj.get_review_request().is_mutable_by(request.user) - - @webapi_check_local_site - @webapi_login_required - @webapi_response_errors(DOES_NOT_EXIST, PERMISSION_DENIED, - INVALID_FORM_DATA, NOT_LOGGED_IN) - @webapi_request_fields( - required={ - 'path': { - 'type': file, - 'description': 'The file to upload.', - }, - }, - optional={ - 'caption': { - 'type': six.text_type, - 'description': 'The optional caption describing the ' - 'file.', - }, - 'attachment_history': { - 'type': int, - 'description': 'ID of the corresponding ' - 'FileAttachmentHistory.', - 'added_in': '2.1', - }, - }, - ) - def create(self, request, *args, **kwargs): - """Creates a new file from a file attachment. - - This accepts any file type and associates it with a draft of a - review request. - - It is expected that the client will send the data as part of a - :mimetype:`multipart/form-data` mimetype. The file's name - and content should be stored in the ``path`` field. A typical request - may look like:: - - -- SoMe BoUnDaRy - Content-Disposition: form-data; name=path; filename="foo.zip" - - - -- SoMe BoUnDaRy -- - """ - try: - review_request = \ - resources.review_request.get_object(request, *args, **kwargs) - except ObjectDoesNotExist: - return DOES_NOT_EXIST - - if not review_request.is_mutable_by(request.user): - return self._no_access_error(request.user) - - form_data = request.POST.copy() - form = UploadFileForm(review_request, form_data, request.FILES) - - if not form.is_valid(): - return INVALID_FORM_DATA, { - 'fields': self._get_form_errors(form), - } - - try: - file = form.create() - except ValueError as e: - return INVALID_FORM_DATA, { - 'fields': { - 'path': [six.text_type(e)], - }, - } - - return 201, { - self.item_result_key: self.serialize_object( - file, request=request, *args, **kwargs), - } - - @webapi_check_local_site - @webapi_login_required - @webapi_response_errors(DOES_NOT_EXIST, NOT_LOGGED_IN, PERMISSION_DENIED) - @webapi_request_fields( - optional={ - 'caption': { - 'type': six.text_type, - 'description': 'The new caption for the file.', - }, - 'thumbnail': { - 'type': six.text_type, - 'description': 'The thumbnail data for the file.', - }, - } - ) - def update(self, request, caption=None, thumbnail=None, *args, **kwargs): - """Updates the file's data. - - This allows updating the file in a draft. The caption, currently, - is the only thing that can be updated. - """ - try: - review_request = \ - resources.review_request.get_object(request, *args, **kwargs) - except ObjectDoesNotExist: - return DOES_NOT_EXIST - - if not review_request.is_mutable_by(request.user): - return PERMISSION_DENIED - - try: - file = resources.file_attachment.get_object(request, *args, - **kwargs) - except ObjectDoesNotExist: - return DOES_NOT_EXIST - - if caption is not None: - try: - resources.review_request_draft.prepare_draft(request, - review_request) - except PermissionDenied: - return self._no_access_error(request.user) - - file.draft_caption = caption - file.save() - - if thumbnail is not None: - try: - file.thumbnail = thumbnail - except Exception as e: - logging.error( - 'Failed to store thumbnail for attachment %d: %s', - file.pk, e, request=request) - return INVALID_FORM_DATA, { - 'fields': { - 'thumbnail': [six.text_type(e)], - } - } - - return 200, { - self.item_result_key: self.serialize_object( - file, request=request, *args, **kwargs), - } - - @webapi_check_local_site - @webapi_login_required - @webapi_response_errors(DOES_NOT_EXIST, NOT_LOGGED_IN, PERMISSION_DENIED) - def delete(self, request, *args, **kwargs): - try: - review_request = \ - resources.review_request.get_object(request, *args, **kwargs) - file_attachment = self.get_object(request, *args, **kwargs) - except ObjectDoesNotExist: - return DOES_NOT_EXIST - - if not self.has_delete_permissions(request, file_attachment, *args, - **kwargs): - return self._no_access_error(request.user) - - try: - draft = resources.review_request_draft.prepare_draft( - request, review_request) - except PermissionDenied: - return self._no_access_error(request.user) - - if file_attachment.attachment_history_id is None: - draft.inactive_file_attachments.add(file_attachment) - draft.file_attachments.remove(file_attachment) - else: - # "Delete" all revisions of the given file - all_revs = FileAttachment.objects.filter( - attachment_history=file_attachment.attachment_history_id) - - for revision in all_revs: - draft.inactive_file_attachments.add(revision) - draft.file_attachments.remove(revision) - - draft.save() - - return 204, {} diff --git a/reviewboard/webapi/resources/base_review_request_file_attachment.py b/reviewboard/webapi/resources/base_review_request_file_attachment.py new file mode 100644 index 0000000000000000000000000000000000000000..d17b0f7245e39450ff185f59846645a275f01ad4 --- /dev/null +++ b/reviewboard/webapi/resources/base_review_request_file_attachment.py @@ -0,0 +1,267 @@ +from __future__ import unicode_literals + +import logging + +from django.core.exceptions import ObjectDoesNotExist, PermissionDenied +from django.db.models import Q +from django.utils import six +from djblets.webapi.decorators import (webapi_login_required, + webapi_response_errors, + webapi_request_fields) +from djblets.webapi.errors import (DOES_NOT_EXIST, INVALID_FORM_DATA, + NOT_LOGGED_IN, PERMISSION_DENIED) + +from reviewboard.attachments.forms import UploadFileForm +from reviewboard.attachments.models import FileAttachment +from reviewboard.site.urlresolvers import local_site_reverse +from reviewboard.webapi.decorators import webapi_check_local_site +from reviewboard.webapi.resources import resources +from reviewboard.webapi.resources.base_file_attachment import \ + BaseFileAttachmentResource + + +class BaseReviewRequestFileAttachmentResource(BaseFileAttachmentResource): + """A base resource representing file attachments.""" + fields = dict({ + 'review_url': { + 'type': six.text_type, + 'description': 'The URL to a review UI for this file.', + }, + 'url': { + 'type': six.text_type, + 'description': "The URL of the file, for downloading purposes. " + "If this is not an absolute URL, then it's " + "relative to the Review Board server's URL. " + "This is deprecated and will be removed in a " + "future version.", + 'deprecated_in': '2.0', + }, + }, **BaseFileAttachmentResource.fields) + + def get_queryset(self, request, is_list=False, *args, **kwargs): + review_request = resources.review_request.get_object( + request, *args, **kwargs) + + q = (Q(review_request=review_request) & + Q(added_in_filediff__isnull=True) & + Q(repository__isnull=True) & + Q(user__isnull=True)) + + if not is_list: + q = q | Q(inactive_review_request=review_request) + + if request.user == review_request.submitter: + try: + draft = resources.review_request_draft.get_object( + request, *args, **kwargs) + + q = q | Q(drafts=draft) + + if not is_list: + q = q | Q(inactive_drafts=draft) + except ObjectDoesNotExist: + pass + + return self.model.objects.filter(q) + + def serialize_url_field(self, obj, **kwargs): + return obj.get_absolute_url() + + def serialize_review_url_field(self, obj, **kwargs): + if obj.review_ui: + review_request = obj.get_review_request() + if review_request.local_site_id: + local_site_name = review_request.local_site.name + else: + local_site_name = None + + return local_site_reverse( + 'file-attachment', local_site_name=local_site_name, + kwargs={ + 'review_request_id': review_request.display_id, + 'file_attachment_id': obj.pk, + }) + + return '' + + def has_access_permissions(self, request, obj, *args, **kwargs): + return obj.get_review_request().is_accessible_by(request.user) + + def has_modify_permissions(self, request, obj, *args, **kwargs): + return obj.get_review_request().is_mutable_by(request.user) + + def has_delete_permissions(self, request, obj, *args, **kwargs): + return obj.get_review_request().is_mutable_by(request.user) + + @webapi_check_local_site + @webapi_login_required + @webapi_response_errors(DOES_NOT_EXIST, PERMISSION_DENIED, + INVALID_FORM_DATA, NOT_LOGGED_IN) + @webapi_request_fields( + required={ + 'path': { + 'type': file, + 'description': 'The file to upload.', + }, + }, + optional={ + 'caption': { + 'type': six.text_type, + 'description': 'The optional caption describing the ' + 'file.', + }, + 'attachment_history': { + 'type': int, + 'description': 'ID of the corresponding ' + 'FileAttachmentHistory.', + 'added_in': '2.1', + }, + }, + ) + def create(self, request, *args, **kwargs): + """Creates a new file from a file attachment. + + This accepts any file type and associates it with a draft of a + review request. + + It is expected that the client will send the data as part of a + :mimetype:`multipart/form-data` mimetype. The file's name + and content should be stored in the ``path`` field. A typical request + may look like:: + + -- SoMe BoUnDaRy + Content-Disposition: form-data; name=path; filename="foo.zip" + + + -- SoMe BoUnDaRy -- + """ + try: + review_request = \ + resources.review_request.get_object(request, *args, **kwargs) + except ObjectDoesNotExist: + return DOES_NOT_EXIST + + if not review_request.is_mutable_by(request.user): + return self._no_access_error(request.user) + + form_data = request.POST.copy() + form = UploadFileForm(review_request, form_data, request.FILES) + + if not form.is_valid(): + return INVALID_FORM_DATA, { + 'fields': self._get_form_errors(form), + } + + try: + file = form.create() + except ValueError as e: + return INVALID_FORM_DATA, { + 'fields': { + 'path': [six.text_type(e)], + }, + } + + return 201, { + self.item_result_key: self.serialize_object( + file, request=request, *args, **kwargs), + } + + @webapi_check_local_site + @webapi_login_required + @webapi_response_errors(DOES_NOT_EXIST, NOT_LOGGED_IN, PERMISSION_DENIED) + @webapi_request_fields( + optional={ + 'caption': { + 'type': six.text_type, + 'description': 'The new caption for the file.', + }, + 'thumbnail': { + 'type': six.text_type, + 'description': 'The thumbnail data for the file.', + }, + } + ) + def update(self, request, caption=None, thumbnail=None, *args, **kwargs): + """Updates the file's data. + + This allows updating the file in a draft. The caption, currently, + is the only thing that can be updated. + """ + try: + review_request = \ + resources.review_request.get_object(request, *args, **kwargs) + except ObjectDoesNotExist: + return DOES_NOT_EXIST + + if not review_request.is_mutable_by(request.user): + return PERMISSION_DENIED + + try: + file = resources.file_attachment.get_object(request, *args, + **kwargs) + except ObjectDoesNotExist: + return DOES_NOT_EXIST + + if caption is not None: + try: + resources.review_request_draft.prepare_draft(request, + review_request) + except PermissionDenied: + return self._no_access_error(request.user) + + file.draft_caption = caption + file.save() + + if thumbnail is not None: + try: + file.thumbnail = thumbnail + except Exception as e: + logging.error( + 'Failed to store thumbnail for attachment %d: %s', + file.pk, e, request=request) + return INVALID_FORM_DATA, { + 'fields': { + 'thumbnail': [six.text_type(e)], + } + } + + return 200, { + self.item_result_key: self.serialize_object( + file, request=request, *args, **kwargs), + } + + @webapi_check_local_site + @webapi_login_required + @webapi_response_errors(DOES_NOT_EXIST, NOT_LOGGED_IN, PERMISSION_DENIED) + def delete(self, request, *args, **kwargs): + try: + review_request = \ + resources.review_request.get_object(request, *args, **kwargs) + file_attachment = self.get_object(request, *args, **kwargs) + except ObjectDoesNotExist: + return DOES_NOT_EXIST + + if not self.has_delete_permissions(request, file_attachment, *args, + **kwargs): + return self._no_access_error(request.user) + + try: + draft = resources.review_request_draft.prepare_draft( + request, review_request) + except PermissionDenied: + return self._no_access_error(request.user) + + if file_attachment.attachment_history_id is None: + draft.inactive_file_attachments.add(file_attachment) + draft.file_attachments.remove(file_attachment) + else: + # "Delete" all revisions of the given file + all_revs = FileAttachment.objects.filter( + attachment_history=file_attachment.attachment_history_id) + for revision in all_revs: + draft.inactive_file_attachments.add(revision) + draft.file_attachments.remove(revision) + + draft.save() + + return 204, {} diff --git a/reviewboard/webapi/resources/diff_file_attachment.py b/reviewboard/webapi/resources/diff_file_attachment.py index 87f0600a45700e101f8dccc2d405c4c041476bd9..152c7e63b1dfde8d12a6dd7699d68a74f020c77b 100644 --- a/reviewboard/webapi/resources/diff_file_attachment.py +++ b/reviewboard/webapi/resources/diff_file_attachment.py @@ -7,11 +7,11 @@ from djblets.webapi.decorators import webapi_request_fields from reviewboard.webapi.decorators import webapi_check_local_site from reviewboard.webapi.resources import resources -from reviewboard.webapi.resources.base_file_attachment import \ - BaseFileAttachmentResource +from reviewboard.webapi.resources.base_review_request_file_attachment import \ + BaseReviewRequestFileAttachmentResource -class DiffFileAttachmentResource(BaseFileAttachmentResource): +class DiffFileAttachmentResource(BaseReviewRequestFileAttachmentResource): """Provides information on file attachments associated with files in diffs. The list of file attachments are tied to files either committed to the @@ -45,7 +45,7 @@ class DiffFileAttachmentResource(BaseFileAttachmentResource): 'this file is just part of a proposed change, and ' 'not necessarily committed in the repository.', }, - }, **BaseFileAttachmentResource.fields) + }, **BaseReviewRequestFileAttachmentResource.fields) def serialize_repository_file_path_field(self, attachment, **kwargs): if attachment.added_in_filediff_id: @@ -114,10 +114,11 @@ class DiffFileAttachmentResource(BaseFileAttachmentResource): 'Filter file attachments with the given mimetype.' ), }, - }, **BaseFileAttachmentResource.get_list.optional_fields), - required=BaseFileAttachmentResource.get_list.required_fields + }, **BaseReviewRequestFileAttachmentResource.get_list.optional_fields), + required=BaseReviewRequestFileAttachmentResource.get_list + .required_fields ) - @augment_method_from(BaseFileAttachmentResource) + @augment_method_from(BaseReviewRequestFileAttachmentResource) def get_list(self, request, *args, **kwargs): """Returns the list of file attachments associated with diffs. diff --git a/reviewboard/webapi/resources/draft_file_attachment.py b/reviewboard/webapi/resources/draft_file_attachment.py index c05bd10c9d32374b5580008cc2d653bb636831c2..72dc1d6ac5914e0262c61263e3958e0c1d90b136 100644 --- a/reviewboard/webapi/resources/draft_file_attachment.py +++ b/reviewboard/webapi/resources/draft_file_attachment.py @@ -9,11 +9,11 @@ from djblets.webapi.responses import WebAPIResponsePaginated from reviewboard.webapi.base import WebAPIResource from reviewboard.webapi.decorators import webapi_check_local_site from reviewboard.webapi.resources import resources -from reviewboard.webapi.resources.base_file_attachment import \ - BaseFileAttachmentResource +from reviewboard.webapi.resources.base_review_request_file_attachment import \ + BaseReviewRequestFileAttachmentResource -class DraftFileAttachmentResource(BaseFileAttachmentResource): +class DraftFileAttachmentResource(BaseReviewRequestFileAttachmentResource): """Provides information on new file attachments being added to a draft of a review request. @@ -45,13 +45,13 @@ class DraftFileAttachmentResource(BaseFileAttachmentResource): @webapi_check_local_site @webapi_login_required - @augment_method_from(BaseFileAttachmentResource) + @augment_method_from(BaseReviewRequestFileAttachmentResource) def get(self, *args, **kwargs): pass @webapi_check_local_site @webapi_login_required - @augment_method_from(BaseFileAttachmentResource) + @augment_method_from(BaseReviewRequestFileAttachmentResource) def delete(self, *args, **kwargs): """Deletes the file attachment from the draft. diff --git a/reviewboard/webapi/resources/file_attachment.py b/reviewboard/webapi/resources/file_attachment.py index 996fffe1b0ed77559f45e51ec4c7c049a7debc76..09d9bcef28b661f0e5deb1362daca60983a732d2 100644 --- a/reviewboard/webapi/resources/file_attachment.py +++ b/reviewboard/webapi/resources/file_attachment.py @@ -5,11 +5,11 @@ from djblets.webapi.decorators import webapi_login_required from reviewboard.webapi.decorators import webapi_check_local_site from reviewboard.webapi.resources import resources -from reviewboard.webapi.resources.base_file_attachment import \ - BaseFileAttachmentResource +from reviewboard.webapi.resources.base_review_request_file_attachment import \ + BaseReviewRequestFileAttachmentResource -class FileAttachmentResource(BaseFileAttachmentResource): +class FileAttachmentResource(BaseReviewRequestFileAttachmentResource): """A resource representing a file attachment on a review request.""" model_parent_key = 'review_request' @@ -25,7 +25,7 @@ class FileAttachmentResource(BaseFileAttachmentResource): def get_parent_object(self, obj): return obj.get_review_request() - @augment_method_from(BaseFileAttachmentResource) + @augment_method_from(BaseReviewRequestFileAttachmentResource) def get_list(self, *args, **kwargs): """Returns a list of file attachments on the review request. @@ -34,7 +34,7 @@ class FileAttachmentResource(BaseFileAttachmentResource): """ pass - @augment_method_from(BaseFileAttachmentResource) + @augment_method_from(BaseReviewRequestFileAttachmentResource) def create(self, request, *args, **kwargs): """Creates a new file attachment from a file attachment. @@ -59,7 +59,7 @@ class FileAttachmentResource(BaseFileAttachmentResource): """ pass - @augment_method_from(BaseFileAttachmentResource) + @augment_method_from(BaseReviewRequestFileAttachmentResource) def update(self, request, caption=None, *args, **kwargs): """Updates the file attachment's data. @@ -73,7 +73,7 @@ class FileAttachmentResource(BaseFileAttachmentResource): @webapi_check_local_site @webapi_login_required - @augment_method_from(BaseFileAttachmentResource) + @augment_method_from(BaseReviewRequestFileAttachmentResource) def delete(self, *args, **kwargs): """Deletes the file attachment. diff --git a/reviewboard/webapi/resources/user.py b/reviewboard/webapi/resources/user.py index 12cf9b77f53673c0f77075aac08f65122b0c5a8b..7a83400d61c00f1c555fb881d595044ddace86ea 100644 --- a/reviewboard/webapi/resources/user.py +++ b/reviewboard/webapi/resources/user.py @@ -30,6 +30,7 @@ class UserResource(WebAPIResource, DjbletsUserResource): """ item_child_resources = [ resources.api_token, + resources.user_file_attachment, resources.watched, ] diff --git a/reviewboard/webapi/resources/user_file_attachment.py b/reviewboard/webapi/resources/user_file_attachment.py new file mode 100644 index 0000000000000000000000000000000000000000..6b26d2dabf10a2bb48fee4c2d7ddbc8fcca67d70 --- /dev/null +++ b/reviewboard/webapi/resources/user_file_attachment.py @@ -0,0 +1,259 @@ +from __future__ import unicode_literals + +from django.core.exceptions import ObjectDoesNotExist +from django.db.models import Q +from django.utils import six +from djblets.util.decorators import augment_method_from +from djblets.webapi.decorators import (webapi_login_required, + webapi_request_fields, + webapi_response_errors) +from djblets.webapi.errors import (DOES_NOT_EXIST, INVALID_FORM_DATA, + NOT_LOGGED_IN, PERMISSION_DENIED) + +from reviewboard.admin.server import build_server_url +from reviewboard.attachments.forms import UploadUserFileForm +from reviewboard.site.urlresolvers import local_site_reverse +from reviewboard.webapi import resources as res +from reviewboard.webapi.decorators import webapi_check_local_site +from reviewboard.webapi.errors import FILE_ALREADY_EXISTS +from reviewboard.webapi.resources import resources +from reviewboard.webapi.resources.base_file_attachment import \ + BaseFileAttachmentResource + + +class UserFileAttachmentResource(BaseFileAttachmentResource): + """A resource representing a file attachment owned by a user. + + A file attachment that is owned by a user and is not attached to any + particular review request. + """ + name = 'file_attachment' + model_parent_key = 'user' + + mimetype_list_resource_name = 'user-file-attachments' + mimetype_item_resource_name = 'user-file-attachment' + + allowed_methods = ('GET', 'POST', 'PUT', 'DELETE') + + def serialize_absolute_url_field(self, obj, request, **kwargs): + if obj.local_site: + local_site_name = request._local_site_name + else: + local_site_name = None + + url = local_site_reverse( + 'view-user-file-attachment', + local_site_name=local_site_name, + kwargs={ + 'file_attachment_uuid': obj.uuid, + 'username': obj.user.username, + }) + + return build_server_url(url) + + def get_serializer_for_object(self, obj): + return res.user_file_attachment.user_file_attachment_resource + + def get_queryset(self, request, is_list=False, local_site_name=None, *args, + **kwargs): + user = resources.user.get_object( + request, local_site_name=local_site_name, *args, **kwargs) + + q = Q(user=user) + + local_site = self._get_local_site(local_site_name) + q = q & Q(local_site=local_site) + + return self.model.objects.filter(q) + + def has_list_access_permissions(self, request, local_site_name=None, *args, + **kwargs): + user = resources.user.get_object( + request, local_site_name=local_site_name, *args, **kwargs) + + return (request.user.is_authenticated() and + (request.user.is_superuser or request.user == user)) + + def has_access_permissions(self, request, obj, *args, **kwargs): + return obj.is_accessible_by(request.user) + + def has_modify_permissions(self, request, obj, *args, **kwargs): + return obj.is_mutable_by(request.user) + + def has_delete_permissions(self, request, obj, *args, **kwargs): + return obj.is_mutable_by(request.user) + + @augment_method_from(BaseFileAttachmentResource) + def get(self, *args, **kwargs): + """Returns information on a particular file attachment that is owned by + the user.""" + pass + + @augment_method_from(BaseFileAttachmentResource) + def get_list(self, *args, **kwargs): + """Returns a list of file attachments that are owned by the user. + + Each item in this list is a file attachment that is owned by the user. + """ + pass + + @webapi_check_local_site + @webapi_login_required + @webapi_response_errors(DOES_NOT_EXIST, PERMISSION_DENIED, + NOT_LOGGED_IN, INVALID_FORM_DATA) + @webapi_request_fields( + optional={ + 'caption': { + 'type': six.text_type, + 'description': 'The optional caption describing the ' + 'file.', + }, + 'path': { + 'type': file, + 'description': 'The file to upload.', + }, + }, + ) + def create(self, request, local_site_name=None, *args, **kwargs): + """Creates a new file attachment that is owned by the user. + + This accepts any file type and associates it with the user. Optionally, + the file may be omitted here and uploaded later by updating the file + attachment. + + It is expected that the client will send the data as part of a + :mimetype:`multipart/form-data` mimetype. The file's name + and content should be stored in the ``path`` field. A typical request + may look like:: + + -- SoMe BoUnDaRy + Content-Disposition: form-data; name=path; filename="foo.zip" + + + -- SoMe BoUnDaRy -- + """ + try: + user = resources.user.get_object(request, *args, **kwargs) + except ObjectDoesNotExist: + return DOES_NOT_EXIST + + local_site = self._get_local_site(local_site_name) + + if ((local_site and not local_site.is_accessible_by(request.user) or + not request.user.is_authenticated() or + (request.user != user and not request.user.is_superuser))): + return self._no_access_error(request.user) + + form = UploadUserFileForm(request.POST.copy(), request.FILES) + + if not form.is_valid(): + return INVALID_FORM_DATA, { + 'fields': self._get_form_errors(form), + } + + file_attachment = form.create(request.user, local_site) + + return 201, { + self.item_result_key: file_attachment + } + + @webapi_check_local_site + @webapi_login_required + @webapi_response_errors(DOES_NOT_EXIST, PERMISSION_DENIED, + NOT_LOGGED_IN, INVALID_FORM_DATA, + FILE_ALREADY_EXISTS) + @webapi_request_fields( + optional={ + 'caption': { + 'type': six.text_type, + 'description': 'The optional caption describing the ' + 'file.', + }, + 'path': { + 'type': file, + 'description': 'The file to upload.', + }, + }, + ) + def update(self, request, local_site_name=None, *args, **kwargs): + """Updates the file attachment's data. + + This allows updating information on the file attachment. It also allows + the file to be uploaded if this was not done when the file attachment + was created. + + The file attachment's file cannot be updated once it has been uploaded. + Attempting to update the file attachment's file if it has already been + uploaded will result in a :ref:`webapi2.0-error-229`. + + The file attachment can only be updated by its owner or by an + administrator. + + It is expected that the client will send the data as part of a + :mimetype:`multipart/form-data` mimetype. The file's name + and content should be stored in the ``path`` field. A typical request + may look like:: + + -- SoMe BoUnDaRy + Content-Disposition: form-data; name=path; filename="foo.zip" + + + -- SoMe BoUnDaRy -- + """ + try: + file_attachment = self.get_object( + request, local_site_name=local_site_name, *args, **kwargs) + except ObjectDoesNotExist: + return DOES_NOT_EXIST + + if not self.has_modify_permissions(request, file_attachment, + *args, **kwargs): + return self._no_access_error(request.user) + + if request.FILES.get('path') and file_attachment.file: + return FILE_ALREADY_EXISTS + + form_data = request.POST.copy() + form = UploadUserFileForm(form_data, request.FILES) + + if not form.is_valid(): + return INVALID_FORM_DATA, { + 'fields': self._get_form_errors(form), + } + + file_attachment = form.update(file_attachment) + + return 200, { + self.item_result_key: file_attachment + } + + @webapi_check_local_site + @webapi_login_required + @webapi_response_errors(DOES_NOT_EXIST, PERMISSION_DENIED, NOT_LOGGED_IN) + def delete(self, request, local_site_name=None, *args, **kwargs): + """Deletes a file attachment. + + This will permanently remove the file attachment owned by the user. + This cannot be undone. + + The file attachment can only be deleted by its owner or an + administrator. + """ + try: + file_attachment = self.get_object( + request, local_site_name=local_site_name, *args, **kwargs) + except ObjectDoesNotExist: + return DOES_NOT_EXIST + + if not self.has_delete_permissions(request, file_attachment, + *args, **kwargs): + return self._no_access_error(request.user) + + if file_attachment.file: + file_attachment.file.delete() + + file_attachment.delete() + + return 204, {} + +user_file_attachment_resource = UserFileAttachmentResource() diff --git a/reviewboard/webapi/tests/mimetypes.py b/reviewboard/webapi/tests/mimetypes.py index a73aac2b443a0496d789359196a5330cc7d2dcda..ee8a432f4e7e7389de4b7ce98a3a987578f6a74b 100644 --- a/reviewboard/webapi/tests/mimetypes.py +++ b/reviewboard/webapi/tests/mimetypes.py @@ -147,6 +147,10 @@ user_list_mimetype = _build_mimetype('users') user_item_mimetype = _build_mimetype('user') +user_file_attachment_list_mimetype = _build_mimetype('user-file-attachments') +user_file_attachment_item_mimetype = _build_mimetype('user-file-attachment') + + validate_diff_mimetype = _build_mimetype('diff-validation') diff --git a/reviewboard/webapi/tests/test_user_file_attachment.py b/reviewboard/webapi/tests/test_user_file_attachment.py new file mode 100644 index 0000000000000000000000000000000000000000..8f88e6aca04c0aebebf905f8601c5a8262f252b7 --- /dev/null +++ b/reviewboard/webapi/tests/test_user_file_attachment.py @@ -0,0 +1,200 @@ +from __future__ import unicode_literals + +from django.utils import six + +from reviewboard.attachments.models import FileAttachment +from reviewboard.site.models import LocalSite +from reviewboard.webapi.errors import FILE_ALREADY_EXISTS +from reviewboard.webapi.resources import resources +from reviewboard.webapi.tests.base import BaseWebAPITestCase +from reviewboard.webapi.tests.mimetypes import ( + user_file_attachment_item_mimetype, + user_file_attachment_list_mimetype) +from reviewboard.webapi.tests.mixins import BasicTestsMetaclass +from reviewboard.webapi.tests.urls import (get_user_file_attachment_item_url, + get_user_file_attachment_list_url) + + +@six.add_metaclass(BasicTestsMetaclass) +class ResourceListTests(BaseWebAPITestCase): + """Testing the FileAttachmentUserResource list APIs.""" + fixtures = ['test_users', 'test_site'] + resource = resources.user_file_attachment + sample_api_url = 'users//file-attachments/' + + def compare_item(self, item_rsp, attachment): + self.assertEqual(item_rsp['id'], attachment.pk) + self.assertEqual(item_rsp['filename'], attachment.filename) + + # + # HTTP GET tests + # + + def setup_basic_get_test(self, user, with_local_site, local_site_name, + populate_items): + if populate_items: + local_site = LocalSite.objects.get(name='local-site-1') + + if with_local_site: + self.create_user_file_attachment(user, + has_file=True, + orig_filename='Trophy1.png', + mimetype='image/png') + + self.create_user_file_attachment(user) + + items = [ + self.create_user_file_attachment(user, + local_site=local_site), + ] + else: + self.create_user_file_attachment(user, + local_site=local_site) + + items = [ + self.create_user_file_attachment(user, + has_file=True, + orig_filename='Trph.png', + mimetype='image/png'), + self.create_user_file_attachment(user), + ] + else: + items = [] + + return (get_user_file_attachment_list_url(user, local_site_name), + user_file_attachment_list_mimetype, + items) + + # + # HTTP POST tests + # + + def setup_basic_post_test(self, user, with_local_site, local_site_name, + post_valid_data): + caption = 'My initial caption.' + + return ( + get_user_file_attachment_list_url(user, local_site_name), + user_file_attachment_item_mimetype, + { + 'path': open(self._getTrophyFilename(), 'r'), + 'caption': caption, + }, + [caption] + ) + + def check_post_result(self, user, rsp, caption): + self.assertIn('file_attachment', rsp) + item_rsp = rsp['file_attachment'] + + attachment = FileAttachment.objects.get(pk=item_rsp['id']) + self.compare_item(item_rsp, attachment) + self.assertEqual(attachment.caption, caption) + + def test_post_no_file_attachment(self): + """Testing the POST users//file-attachments/ API without a + file attached""" + caption = 'My initial caption.' + + rsp = self.api_post( + get_user_file_attachment_list_url(self.user), + {'caption': caption}, + expected_status=201, + expected_mimetype=user_file_attachment_item_mimetype) + + self.check_post_result(None, rsp, caption) + + +@six.add_metaclass(BasicTestsMetaclass) +class ResourceItemTests(BaseWebAPITestCase): + """Testing the FileAttachmentUserResource item APIs.""" + fixtures = ['test_users'] + sample_api_url = 'users//file-attachments//' + resource = resources.user_file_attachment + + def compare_item(self, item_rsp, attachment): + self.assertEqual(item_rsp['id'], attachment.pk) + self.assertEqual(item_rsp['filename'], attachment.filename) + + # + # HTTP DELETE tests + # + + def setup_basic_delete_test(self, user, with_local_site, local_site_name): + file_attachment = self.create_user_file_attachment( + user, + with_local_site=with_local_site, + local_site_name=local_site_name) + + return (get_user_file_attachment_item_url(user, + file_attachment, + local_site_name), + [file_attachment]) + + def check_delete_result(self, user, file_attachment): + file_attachments = FileAttachment.objects.all() + self.assertNotIn(file_attachment, file_attachments) + + # + # HTTP GET tests + # + + def setup_basic_get_test(self, user, with_local_site, local_site_name): + file_attachment = self.create_user_file_attachment( + user, + with_local_site=with_local_site, + local_site_name=local_site_name) + + return (get_user_file_attachment_item_url(user, + file_attachment, + local_site_name), + user_file_attachment_item_mimetype, + file_attachment) + + # + # HTTP PUT tests + # + + def setup_basic_put_test(self, user, with_local_site, local_site_name, + put_valid_data): + file_attachment = self.create_user_file_attachment( + user, + with_local_site=with_local_site, + local_site_name=local_site_name) + + return (get_user_file_attachment_item_url(user, + file_attachment, + local_site_name), + user_file_attachment_item_mimetype, + {'caption': 'My new caption'}, + file_attachment, + []) + + def check_put_result(self, user, item_rsp, file_attachment): + file_attachment = FileAttachment.objects.get(pk=file_attachment.pk) + self.assertEqual(item_rsp['id'], file_attachment.pk) + self.assertEqual(file_attachment.caption, 'My new caption') + self.assertEqual(file_attachment.user, user) + self.compare_item(item_rsp, file_attachment) + + def test_put_file_already_exists(self): + """Testing the PUT users//file-attachments// API + attaching file to object that already has a file attached to it.""" + file_attachment = self.create_user_file_attachment( + self.user, + has_file=True, + orig_filename='Trophy1.png', + mimetype='image/png') + + with open(self._getTrophyFilename(), "r") as f: + self.assertTrue(f) + rsp = self.api_put( + get_user_file_attachment_item_url(self.user, file_attachment), + { + 'caption': 'My new caption.', + 'path': f, + }, + expected_status=400) + + self.assertEqual(rsp['stat'], 'fail') + self.assertEqual(rsp['err']['code'], FILE_ALREADY_EXISTS.code) diff --git a/reviewboard/webapi/tests/urls.py b/reviewboard/webapi/tests/urls.py index 7da702a66a47e061aabf11687b9cb9978b0c21a1..c2f9ff7651eddccab3ee5ad1eefe67b208c76773 100644 --- a/reviewboard/webapi/tests/urls.py +++ b/reviewboard/webapi/tests/urls.py @@ -671,6 +671,23 @@ def get_user_item_url(username, local_site_name=None): # +# UserFileAttachmentResource +# +def get_user_file_attachment_list_url(user, local_site_name=None): + return resources.user_file_attachment.get_list_url( + username=user.username, + local_site_name=local_site_name) + + +def get_user_file_attachment_item_url(user, file_attachment, + local_site_name=None): + return resources.user_file_attachment.get_item_url( + username=user.username, + local_site_name=local_site_name, + file_attachment_id=file_attachment.id) + + +# # ValidateDiffResource # def get_validate_diff_url(local_site_name=None):