Skip to content

Commit

Permalink
Fix #857: Imported resources saved to wrong location
Browse files Browse the repository at this point in the history
Fix #834: Limit CSV file size
  • Loading branch information
bohare committed Oct 28, 2016
1 parent 22b4e7d commit b8bdb96
Show file tree
Hide file tree
Showing 6 changed files with 157 additions and 2 deletions.
4 changes: 4 additions & 0 deletions cadasta/organization/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -483,6 +483,7 @@ def get_file(self):

class SelectImportForm(forms.Form):
VALID_IMPORT_MIME_TYPES = ['text/plain', 'text/csv']
MAX_FILE_SIZE = 512000

class Media:
js = ('js/import.js',)
Expand Down Expand Up @@ -511,6 +512,9 @@ def __init__(self, project, user, *args, **kwargs):

def clean_file(self):
file = self.cleaned_data.get("file", False)
if file.size > self.MAX_FILE_SIZE:
raise ValidationError(
_('File too large, max size 512kb'))
mime = magic.Magic(mime=True)
mime_type = str(mime.from_buffer(file.read(1024)), 'utf-8')
if mime_type not in self.VALID_IMPORT_MIME_TYPES:
Expand Down
13 changes: 13 additions & 0 deletions cadasta/organization/tests/test_forms.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import os
import random
from string import ascii_lowercase
from zipfile import ZipFile

from django.conf import settings
Expand Down Expand Up @@ -1041,6 +1042,18 @@ def test_invalid_file_type(self):
assert form.is_valid() is False
assert form.errors['file'][0] == 'Invalid file type'

def test_file_size_validation(self):
contents = "".join(
random.choice(ascii_lowercase) for i in range(520000))
file = SimpleUploadedFile(
'test_too_big.csv', bytes(contents, 'ascii'), 'text/csv')
file_dict = {'file': file}
form = forms.SelectImportForm(
files=file_dict, data=self.data,
project=self.project, user=self.user)
assert form.is_valid() is False
assert form.errors['file'][0] == 'File too large, max size 512kb'

def test_set_mime_type(self):
valid_file = open(self.path + self.valid_file_type, 'rb').read()
file = SimpleUploadedFile(
Expand Down
3 changes: 3 additions & 0 deletions cadasta/organization/tests/test_views_default_projects.py
Original file line number Diff line number Diff line change
Expand Up @@ -1517,6 +1517,9 @@ def test_full_flow_valid(self):
if su.geometry is not None:
assert type(su.geometry) is Point

resource = Resource.objects.filter(project_id=proj.pk).first()
assert resource.file.url == '/media/s3/uploads/resources/test.csv'

def test_full_flow_invalid_value(self):
self.client.force_login(self.user)
csvfile = open(self.path + self.invalid_csv, 'rb').read()
Expand Down
7 changes: 5 additions & 2 deletions cadasta/organization/views/default.py
Original file line number Diff line number Diff line change
Expand Up @@ -843,11 +843,14 @@ def done(self, form_list, **kwargs):
if is_resource:
default_storage = DefaultStorage()
file.seek(0)
url = default_storage.save(file.name, file.read())
resource = Resource(
name=name, description=description, file=url,
name=name, description=description,
original_file=original_file, mime_type=mime_type,
contributor=self.request.user, project=self.get_project())
upload_to = getattr(resource.file.field, 'upload_to')
url = default_storage.save(
upload_to + '/' + file.name, file.read())
resource.file.url = url
resource.save()
ContentObject.objects.create(resource=resource,
content_object=resource.project)
Expand Down
31 changes: 31 additions & 0 deletions cadasta/resources/migrations/0005_fix_import_resource_urls.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
from __future__ import unicode_literals

import os

from django.db import migrations


def fix_imported_resource_urls(apps, schema_editor):
Resource = apps.get_model("resources", "Resource")
resources = Resource.objects.filter(mime_type='text/csv')
for resource in resources:
url = resource.file.url
if url.endswith('.csv'):
parts = list(os.path.split(url))
if 'resources' not in parts:
parts.insert(-1, 'resources')
url = '/'.join(parts)
resource.file = url
resource.save()


class Migration(migrations.Migration):

dependencies = [
('resources', '0004_add_ordering_for_resources'),
]

operations = [
migrations.RunPython(
fix_imported_resource_urls, migrations.RunPython.noop),
]
101 changes: 101 additions & 0 deletions cadasta/resources/tests/test_migrations.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
from core.tests.utils.cases import UserTestCase
from core.util import random_id
from django.apps import apps
from django.core.management import call_command
from django.db import connection
from django.db.migrations.loader import MigrationLoader
from django.test import TestCase


class MigrationTestCase(UserTestCase, TestCase):

@property
def app(self):
return apps.get_containing_app_config(type(self).__module__).name

migrate_from = None
migrate_to = None

def setUp(self):
super(UserTestCase, self).setUp()

assert self.migrate_from and self.migrate_to
"TestCase '{}' must define migrate_from and migrate_to "
"properties".format(type(self).__name__)

# get the application state pre-migration
apps_before = self._get_apps_for_migration(
[(self.app, self.migrate_from)])

# Reverse to migrate_from
call_command('migrate', self.app, self.migrate_from)

# setup pre-migration test data
self.setUpBeforeMigration(apps_before)

# Run the migration to test
call_command('migrate', self.app, self.migrate_to)

# get application state post-migration
self.apps_after = self._get_apps_for_migration(
[(self.app, self.migrate_to)])

def setUpBeforeMigration(self, apps):
pass

def _get_apps_for_migration(self, migration_states):
loader = MigrationLoader(connection)
full_names = []
for app_name, migration_name in migration_states:
if migration_name != 'zero':
migration_name = loader.get_migration_by_prefix(
app_name, migration_name).name
full_names.append((app_name, migration_name))
state = loader.project_state(full_names)
return state.apps


class TestFixImportedResourceUrls(MigrationTestCase):

migrate_from = '0004_add_ordering_for_resources'
migrate_to = '0005_fix_import_resource_urls'

def setUpBeforeMigration(self, apps_before):
Resource = apps_before.get_model('resources', 'Resource')
User = apps_before.get_model('accounts', 'User')
Organization = apps_before.get_model('organization', 'Organization')
Project = apps_before.get_model('organization', 'Project')

user = User.objects.create(username='testuser')
org = Organization.objects.create(name='Test Org')
project = Project.objects.create(name='Test Proj', organization=org)

base_path = (
'https://s3-us-west-2.amazonaws.com/cadasta-platformprod-bucket/'
)

# cannot call custom save methods on models in migrations
# as models are serialized from migration scripts
# so custom methods are not available
# docs.djangoproject.com/en/1.9/topics/migrations/#historical-models
for f in range(10):
file_name = base_path + 'test_' + str(f) + '.csv'
resource_name = 'test-resource-' + str(f)
Resource.objects.create(
id=random_id(), name=resource_name, file=file_name,
mime_type='text/csv', contributor=user, project=project
)
Resource.objects.create(
id=random_id(), file=base_path + 'test.jpg', mime_type='image/jpg',
contributor=user, project=project
)

def test_migration(self):
Resource = self.apps_after.get_model('resources', 'Resource')
resources = Resource.objects.filter(mime_type='text/csv')
assert len(resources) == 10
base_path = (
'https://s3-us-west-2.amazonaws.com/cadasta-platformprod-bucket/'
)
resource = Resource.objects.get(name='test-resource-0')
assert resource.file.url == base_path + 'resources/test_0.csv'

0 comments on commit b8bdb96

Please sign in to comment.