Merge pull request #69 from taigaio/storage-refactor

Attachments and user photo storage refactor.

settings/common.py

@@ -58,8 +58,6 @@ SEND_BROKEN_LINK_EMAILS = True
 IGNORABLE_404_ENDS = (".php", ".cgi")
 IGNORABLE_404_STARTS = ("/phpmyadmin/",)
 
-
-# Default django tz/i18n config
 ATOMIC_REQUESTS = True
 TIME_ZONE = "UTC"
 LANGUAGE_CODE = "en"
@@ -94,13 +92,21 @@ EVENTS_PUSH_BACKEND = "taiga.events.backends.postgresql.EventsPushBackend"
 # Message System
 MESSAGE_STORAGE = "django.contrib.messages.storage.session.SessionStorage"
 
-# Static configuration.
+# The absolute url is mandatory because attachments
-MEDIA_ROOT = os.path.join(BASE_DIR, "media")
+# urls depends on it. On production should be set
-MEDIA_URL = "/media/"
+# something like https://media.taiga.io/
-STATIC_ROOT = os.path.join(BASE_DIR, "static")
+MEDIA_URL = "http://localhost:8000/media/"
+
+# Static url is not widelly used by taiga (only
+# if admin is activated).
 STATIC_URL = "/static/"
 ADMIN_MEDIA_PREFIX = "/static/admin/"
 
+# Static configuration.
+MEDIA_ROOT = os.path.join(BASE_DIR, "media")
+STATIC_ROOT = os.path.join(BASE_DIR, "static")
+
+
 STATICFILES_FINDERS = [
     "django.contrib.staticfiles.finders.FileSystemFinder",
     "django.contrib.staticfiles.finders.AppDirectoriesFinder",

taiga/base/utils/iterators.py

@@ -16,6 +16,8 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 from functools import wraps, partial
+from django.core.paginator import Paginator
+
 
 def as_tuple(function=None, *, remove_nulls=False):
     if function is None:
@@ -33,3 +35,24 @@ def as_dict(function):
     def _decorator(*args, **kwargs):
         return dict(function(*args, **kwargs))
     return _decorator
+
+
+def split_by_n(seq:str, n:int):
+    """
+    A generator to divide a sequence into chunks of n units.
+    """
+    while seq:
+        yield seq[:n]
+        seq = seq[n:]
+
+
+def iter_queryset(queryset, itersize:int=20):
+    """
+    Util function for iterate in more efficient way
+    all queryset.
+    """
+    paginator = Paginator(queryset, itersize)
+    for page_num in paginator.page_range:
+        page = paginator.page(page_num)
+        for element in page.object_list:
+            yield element

taiga/projects/attachments/api.py

@@ -15,6 +15,7 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import os
+import os.path as path
 import hashlib
 import mimetypes
 mimetypes.init()
@@ -59,6 +60,8 @@ class BaseAttachmentViewSet(HistoryResourceMixin, WatchedResourceMixin, ModelCru
         if not obj.id:
             obj.content_type = self.get_content_type()
             obj.owner = self.request.user
+            obj.size = obj.attached_file.size
+            obj.name = path.basename(obj.attached_file.name).lower()
 
         if obj.project_id != obj.content_object.project_id:
             raise exc.WrongArguments("Project ID not matches between object and project")
@@ -97,39 +100,3 @@ class WikiAttachmentViewSet(BaseAttachmentViewSet):
     permission_classes = (permissions.WikiAttachmentPermission,)
     filter_backends = (filters.CanViewWikiAttachmentFilterBackend,)
     content_type = "wiki.wikipage"
-
-
-class RawAttachmentView(generics.RetrieveAPIView):
-    queryset = models.Attachment.objects.all()
-    permission_classes = (permissions.RawAttachmentPermission,)
-
-    def _serve_attachment(self, attachment):
-        if settings.IN_DEVELOPMENT_SERVER:
-            return http.HttpResponseRedirect(attachment.url)
-
-        name = attachment.name
-        response = http.HttpResponse()
-        response['X-Accel-Redirect'] = "/{filepath}".format(filepath=name)
-        response['Content-Disposition'] = 'inline;filename={filename}'.format(
-            filename=os.path.basename(name))
-        response['Content-Type'] = mimetypes.guess_type(name)[0]
-
-        return response
-
-    def check_permissions(self, request, action='retrieve', obj=None):
-        self.object = self.get_object()
-        user_id = self.request.QUERY_PARAMS.get('user', None)
-        token = self.request.QUERY_PARAMS.get('token', None)
-
-        if token and user_id:
-            token_src = "{}-{}-{}".format(settings.ATTACHMENTS_TOKEN_SALT, user_id, self.object.id)
-            if token == hashlib.sha1(token_src.encode("utf-8")).hexdigest():
-                request.user = get_object_or_404(User, pk=user_id)
-
-        return super().check_permissions(request, action, self.object)
-
-    def retrieve(self, request, *args, **kwargs):
-        self.object = self.get_object()
-
-        self.check_permissions(request, 'retrieve', self.object)
-        return self._serve_attachment(self.object.attached_file)

taiga/projects/attachments/management/commands/migrate_attachments.py

@@ -0,0 +1,130 @@
+import re
+
+from django.apps import apps
+from django.core.management.base import BaseCommand, CommandError
+from django.core.files import File
+from django.conf import settings
+from django.db import transaction
+
+from taiga.base.utils.iterators import iter_queryset
+
+url = """
+https://api-taiga.kaleidos.net/attachments/446/?user=8&token=9ac0fc593e9c07740975c6282e1e501189578faa
+"""
+
+class Command(BaseCommand):
+    help = "Parses all objects and try replace old attachments url with one new"
+
+
+    trx = r"((?:https?)://api-taiga\.kaleidos\.net/attachments/(\d+)/[^\s]+)"
+
+    @transaction.atomic
+    def handle(self, *args, **options):
+        settings.MEDIA_URL="https://media.taiga.io/"
+
+        self.move_user_photo()
+        self.move_attachments()
+        self.process_userstories()
+        self.process_issues()
+        self.process_wiki()
+        self.process_tasks()
+        self.process_history()
+
+    def move_attachments(self):
+        print("Moving all attachments to new location")
+
+        Attachment = apps.get_model("attachments", "Attachment")
+        qs = Attachment.objects.all()
+
+        for item in iter_queryset(qs):
+            try:
+                with transaction.atomic():
+                    old_file = item.attached_file
+                    item.attached_file = File(old_file)
+                    item.save()
+            except FileNotFoundError:
+                item.delete()
+
+    def move_user_photo(self):
+        print("Moving all user photos to new location")
+
+        User = apps.get_model("users", "User")
+        qs = User.objects.all()
+
+        for item in iter_queryset(qs):
+            try:
+                with transaction.atomic():
+                    old_file = item.photo
+                    item.photo = File(old_file)
+                    item.save()
+            except FileNotFoundError:
+                pass
+
+    def get_attachment_real_url(self, pk):
+        if isinstance(pk, str):
+            pk = int(pk)
+
+        Attachment = apps.get_model("attachments", "Attachment")
+        return Attachment.objects.get(pk=pk).attached_file.url
+
+    def replace_matches(self, data):
+        matches = re.findall(self.trx, data)
+
+        original_data = data
+
+        if len(matches) == 0:
+            return data
+
+        for url, attachment_id in matches:
+            new_url = self.get_attachment_real_url(attachment_id)
+            print("Match {} replaced by {}".format(url, new_url))
+
+            try:
+                data = data.replace(url, self.get_attachment_real_url(attachment_id))
+            except Exception as e:
+                print("Exception found but ignoring:", e)
+
+            assert data != original_data
+
+        return data
+
+    def process_userstories(self):
+        UserStory = apps.get_model("userstories", "UserStory")
+        qs = UserStory.objects.all()
+
+        for item in iter_queryset(qs):
+            description = self.replace_matches(item.description)
+            UserStory.objects.filter(pk=item.pk).update(description=description)
+
+    def process_tasks(self):
+        Task = apps.get_model("tasks", "Task")
+        qs = Task.objects.all()
+
+        for item in iter_queryset(qs):
+            description = self.replace_matches(item.description)
+            Task.objects.filter(pk=item.pk).update(description=description)
+
+    def process_issues(self):
+        Issue = apps.get_model("issues", "Issue")
+        qs = Issue.objects.all()
+
+        for item in iter_queryset(qs):
+            description = self.replace_matches(item.description)
+            Issue.objects.filter(pk=item.pk).update(description=description)
+
+    def process_wiki(self):
+        WikiPage = apps.get_model("wiki", "WikiPage")
+        qs = WikiPage.objects.all()
+
+        for item in iter_queryset(qs):
+            content = self.replace_matches(item.content)
+            WikiPage.objects.filter(pk=item.pk).update(content=content)
+
+    def process_history(self):
+        HistoryEntry = apps.get_model("history", "HistoryEntry")
+        qs = HistoryEntry.objects.all()
+
+        for item in iter_queryset(qs):
+            comment = self.replace_matches(item.comment)
+            comment_html = self.replace_matches(item.comment_html)
+            HistoryEntry.objects.filter(pk=item.pk).update(comment=comment, comment_html=comment_html)

taiga/projects/attachments/migrations/0002_add_size_and_name_fields.py

@@ -0,0 +1,41 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+import os.path as path
+from django.db import models, migrations
+
+
+def parse_filenames_and_sizes(apps, schema_editor):
+    Attachment = apps.get_model("attachments", "Attachment")
+
+    for item in Attachment.objects.all():
+        try:
+            item.size = item.attached_file.size
+        except Exception as e:
+            item.size = 0
+
+        item.name = path.basename(item.attached_file.name)
+        item.save()
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('attachments', '0001_initial'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='attachment',
+            name='name',
+            field=models.CharField(default='', blank=True, max_length=500),
+            preserve_default=True,
+        ),
+        migrations.AddField(
+            model_name='attachment',
+            name='size',
+            field=models.IntegerField(editable=False, null=True, blank=True, default=None),
+            preserve_default=True,
+        ),
+        migrations.RunPython(parse_filenames_and_sizes)
+    ]

taiga/projects/attachments/models.py

@@ -14,25 +14,32 @@
 # You should have received a copy of the GNU Affero General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-import time
+import hashlib
+import os
+import os.path as path
 
 from django.db import models
 from django.conf import settings
 from django.contrib.contenttypes.models import ContentType
 from django.contrib.contenttypes import generic
-from django.utils.translation import ugettext_lazy as _
 from django.utils import timezone
+from django.utils.encoding import force_bytes
+from django.utils.translation import ugettext_lazy as _
+
+from taiga.base.utils.iterators import split_by_n
 
 
 def get_attachment_file_path(instance, filename):
-    template = "attachment-files/{project}/{model}/{stamp}/{filename}"
+    basename = path.basename(filename).lower()
-    current_timestamp = int(time.mktime(timezone.now().timetuple()))
 
-    upload_to_path = template.format(stamp=current_timestamp,
+    hs = hashlib.sha256()
-                                     project=instance.project.slug,
+    hs.update(force_bytes(timezone.now().isoformat()))
-                                     model=instance.content_type.model,
+    hs.update(os.urandom(1024))
-                                     filename=filename)
+
-    return upload_to_path
+    p1, p2, p3, p4, *p5 = split_by_n(hs.hexdigest(), 1)
+    hash_part = path.join(p1, p2, p3, p4, "".join(p5))
+
+    return path.join("attachments", hash_part, basename)
 
 
 class Attachment(models.Model):
@@ -51,13 +58,17 @@ class Attachment(models.Model):
                                         default=timezone.now)
     modified_date = models.DateTimeField(null=False, blank=False,
                                          verbose_name=_("modified date"))
-
+    name = models.CharField(blank=True, default="", max_length=500)
+    size = models.IntegerField(null=True, blank=True, editable=False, default=None)
     attached_file = models.FileField(max_length=500, null=True, blank=True,
                                      upload_to=get_attachment_file_path,
                                      verbose_name=_("attached file"))
+
+
     is_deprecated = models.BooleanField(default=False, verbose_name=_("is deprecated"))
     description = models.TextField(null=False, blank=True, verbose_name=_("description"))
     order = models.IntegerField(default=0, null=False, blank=False, verbose_name=_("order"))
+
     _importing = None
 
     class Meta:

taiga/projects/attachments/serializers.py

@@ -28,10 +28,7 @@ from . import models
 
 
 class AttachmentSerializer(serializers.ModelSerializer):
-    name = serializers.SerializerMethodField("get_name")
     url = serializers.SerializerMethodField("get_url")
-    size = serializers.SerializerMethodField("get_size")
-
     attached_file = serializers.FileField(required=True)
 
     class Meta:
@@ -41,28 +38,5 @@ class AttachmentSerializer(serializers.ModelSerializer):
                   "object_id", "order")
         read_only_fields = ("owner", "created_date", "modified_date")
 
-    def get_name(self, obj):
-        if obj.attached_file:
-            return path.basename(obj.attached_file.path)
-        return ""
-
     def get_url(self, obj):
-        token = None
+        return obj.attached_file.url
-
-        url = reverse("attachment-url", kwargs={"pk": obj.pk})
-        if "request" in self.context and self.context["request"].user.is_authenticated():
-            user_id = self.context["request"].user.id
-            token_src = "{}-{}-{}".format(settings.ATTACHMENTS_TOKEN_SALT, user_id, obj.id)
-            token = hashlib.sha1(token_src.encode("utf-8"))
-
-            return "{}?user={}&token={}".format(url, user_id, token.hexdigest())
-
-        return url
-
-    def get_size(self, obj):
-        if obj.attached_file:
-            try:
-                return obj.attached_file.size
-            except FileNotFoundError:
-                pass
-        return 0

taiga/urls.py

@@ -20,14 +20,8 @@ from django.contrib.staticfiles.urls import staticfiles_urlpatterns
 from django.contrib import admin
 
 from .routers import router
-from .projects.attachments.api import RawAttachmentView
-
-
-
-admin.autodiscover()
 
 urlpatterns = patterns('',
-    url(r'^attachments/(?P<pk>\d+)/$', RawAttachmentView.as_view(), name="attachment-url"),
     url(r'^api/v1/', include(router.urls)),
     url(r'^api/v1/api-auth/', include('rest_framework.urls', namespace='rest_framework')),
     url(r'^admin/', include(admin.site.urls)),

taiga/users/migrations/0005_alter_user_photo.py

@@ -0,0 +1,20 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import models, migrations
+import taiga.users.models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('users', '0004_auto_20140913_1914'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='user',
+            name='photo',
+            field=models.FileField(upload_to=taiga.users.models.get_user_file_path, blank=True, max_length=500, verbose_name='photo', null=True),
+        ),
+    ]

taiga/users/models.py

@@ -14,26 +14,44 @@
 # You should have received a copy of the GNU Affero General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+import hashlib
+import os
+import os.path as path
+import random
+import re
+
 from django.db import models
 from django.dispatch import receiver
 from django.utils.translation import ugettext_lazy as _
 from django.contrib.auth.models import UserManager, AbstractBaseUser
 from django.core import validators
 from django.utils import timezone
+from django.utils.encoding import force_bytes
 
 from djorm_pgarray.fields import TextArrayField
 
 from taiga.base.utils.slug import slugify_uniquely
+from taiga.base.utils.iterators import split_by_n
 from taiga.permissions.permissions import MEMBERS_PERMISSIONS
 
-import random
-import re
-
 
 def generate_random_hex_color():
     return "#{:06x}".format(random.randint(0,0xFFFFFF))
 
 
+def get_user_file_path(instance, filename):
+    basename = path.basename(filename).lower()
+
+    hs = hashlib.sha256()
+    hs.update(force_bytes(timezone.now().isoformat()))
+    hs.update(os.urandom(1024))
+
+    p1, p2, p3, p4, *p5 = split_by_n(hs.hexdigest(), 1)
+    hash_part = path.join(p1, p2, p3, p4, "".join(p5))
+
+    return path.join("user", hash_part, basename)
+
+
 class PermissionsMixin(models.Model):
     """
     A mixin class that adds the fields and methods necessary to support
@@ -85,7 +103,8 @@ class User(AbstractBaseUser, PermissionsMixin):
     color = models.CharField(max_length=9, null=False, blank=True, default=generate_random_hex_color,
                              verbose_name=_("color"))
     bio = models.TextField(null=False, blank=True, default="", verbose_name=_("biography"))
-    photo = models.FileField(upload_to="users/photo", max_length=500, null=True, blank=True,
+    photo = models.FileField(upload_to=get_user_file_path,
+                             max_length=500, null=True, blank=True,
                              verbose_name=_("photo"))
     date_joined = models.DateTimeField(_('date joined'), default=timezone.now)
     default_language = models.CharField(max_length=20, null=False, blank=True, default="",

tests/factories.py

@@ -99,6 +99,7 @@ class UserStoryAttachmentFactory(Factory):
     project = factory.SubFactory("tests.factories.ProjectFactory")
     owner = factory.SubFactory("tests.factories.UserFactory")
     content_object = factory.SubFactory("tests.factories.UserStoryFactory")
+    attached_file = factory.django.FileField(data=b"File contents")
 
     class Meta:
         model = "attachments.Attachment"
@@ -109,6 +110,7 @@ class TaskAttachmentFactory(Factory):
     project = factory.SubFactory("tests.factories.ProjectFactory")
     owner = factory.SubFactory("tests.factories.UserFactory")
     content_object = factory.SubFactory("tests.factories.TaskFactory")
+    attached_file = factory.django.FileField(data=b"File contents")
 
     class Meta:
         model = "attachments.Attachment"
@@ -119,15 +121,18 @@ class IssueAttachmentFactory(Factory):
     project = factory.SubFactory("tests.factories.ProjectFactory")
     owner = factory.SubFactory("tests.factories.UserFactory")
     content_object = factory.SubFactory("tests.factories.IssueFactory")
+    attached_file = factory.django.FileField(data=b"File contents")
 
     class Meta:
         model = "attachments.Attachment"
         strategy = factory.CREATE_STRATEGY
 
+
 class WikiAttachmentFactory(Factory):
     project = factory.SubFactory("tests.factories.ProjectFactory")
     owner = factory.SubFactory("tests.factories.UserFactory")
     content_object = factory.SubFactory("tests.factories.WikiFactory")
+    attached_file = factory.django.FileField(data=b"File contents")
 
     class Meta:
         model = "attachments.Attachment"

tests/integration/test_attachments.py

@@ -4,73 +4,22 @@ from django.core.urlresolvers import reverse
 from django.core.files.base import File
 from django.core.files.uploadedfile import SimpleUploadedFile
 
-from .. import factories as f
-from ..utils import set_settings
-
 from taiga.projects.attachments.serializers import AttachmentSerializer
+from .. import factories as f
 
 pytestmark = pytest.mark.django_db
 
 
-def test_authentication(client):
-    "User can't access an attachment if not authenticated"
-    attachment = f.UserStoryAttachmentFactory.create()
-    url = reverse("attachment-url", kwargs={"pk": attachment.pk})
-
-    response = client.get(url)
-
-    assert response.status_code == 401
-
-
-def test_authorization(client):
-    "User can't access an attachment if not authorized"
-    attachment = f.UserStoryAttachmentFactory.create()
-    user = f.UserFactory.create()
-
-    url = reverse("attachment-url", kwargs={"pk": attachment.pk})
-
-    client.login(user)
-    response = client.get(url)
-
-    assert response.status_code == 403
-
-
-@set_settings(IN_DEVELOPMENT_SERVER=True)
-def test_attachment_redirect_in_devserver(client):
-    "When accessing the attachment in devserver redirect to the real attachment url"
-    attachment = f.UserStoryAttachmentFactory.create(attached_file="test")
-
-    url = reverse("attachment-url", kwargs={"pk": attachment.pk})
-
-    client.login(attachment.owner)
-    response = client.get(url)
-
-    assert response.status_code == 302
-
-
-@set_settings(IN_DEVELOPMENT_SERVER=False)
-def test_attachment_redirect(client):
-    "When accessing the attachment redirect using X-Accel-Redirect header"
-    attachment = f.UserStoryAttachmentFactory.create()
-
-    url = reverse("attachment-url", kwargs={"pk": attachment.pk})
-
-    client.login(attachment.owner)
-    response = client.get(url)
-
-    assert response.status_code == 200
-    assert response.has_header('x-accel-redirect')
-
-
-# Bug test "Don't create attachments without attached_file"
 def test_create_user_story_attachment_without_file(client):
+    """
+    Bug test "Don't create attachments without attached_file"
+    """
     us = f.UserStoryFactory.create()
-    attachment = f.UserStoryAttachmentFactory(project=us.project, content_object=us)
+    attachment_data = {
-
+        "description": "test",
-    attachment_data = AttachmentSerializer(attachment).data
+        "attached_file": None,
-    attachment_data["id"] = None
+        "project": us.project_id,
-    attachment_data["description"] = "test"
+    }
-    attachment_data["attached_file"] = None
 
     url = reverse('userstory-attachments-list')