Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Latest url strip spaces #823

Merged
merged 5 commits into from
Mar 20, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
- Upgrade to Flask-Login 0.4.0 and switch from Flask-Security to the latest
[Flask-Security-Fork](https://pypi.python.org/pypi/Flask-Security-Fork)
[#813](https://github.com/opendatateam/udata/pull/813)
- Ensure URLs are stripped [#823](https://github.com/opendatateam/udata/pull/823)

## 1.0.4 (2017-03-01)

Expand Down
2 changes: 1 addition & 1 deletion udata/core/dataset/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ class ResourceMixin(object):
description = db.StringField()
filetype = db.StringField(
choices=RESOURCE_TYPES.keys(), default='file', required=True)
url = db.StringField()
url = db.URLField(required=True)
urlhash = db.StringField()
checksum = db.EmbeddedDocumentField(Checksum)
format = db.StringField()
Expand Down
2 changes: 1 addition & 1 deletion udata/core/dataset/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ def resource_redirect(id):
resource = get_by(dataset.resources, 'id', id)
else:
resource = CommunityResource.objects(id=id).first()
return redirect(resource.url) if resource else abort(404)
return redirect(resource.url.strip()) if resource else abort(404)


@sitemap.register_generator
Expand Down
5 changes: 5 additions & 0 deletions udata/forms/fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,11 @@ def pre_validate(self, form):
raise validators.ValidationError(_('Invalid URL'))
return True

def process_formdata(self, valuelist):
super(URLField, self).process_formdata(valuelist)
if self.data:
self.data = self.data.strip()


class UploadableURLField(URLField):
def __init__(self, *args, **kwargs):
Expand Down
2 changes: 2 additions & 0 deletions udata/models/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
from .datetime_fields import DateField, DateRange, Datetimed
from .extras_fields import ExtrasField, Extra
from .slug_fields import SlugField
from .url_field import URLField
from .uuid_fields import AutoUUIDField
from .owned import Owned, OwnedQuerySet
from .queryset import UDataQuerySet
Expand Down Expand Up @@ -43,6 +44,7 @@ def __init__(self, app=None):
self.BaseDocumentMetaclass = TopLevelDocumentMetaclass
self.FileField = FileField
self.ImageField = ImageField
self.URLField = URLField
self.ValidationError = ValidationError
self.ObjectId = ObjectId
self.DBRef = DBRef
Expand Down
14 changes: 14 additions & 0 deletions udata/models/url_field.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# -*- coding: utf-8 -*-
from __future__ import unicode_literals

from mongoengine.fields import URLField as MEURLField


class URLField(MEURLField):
'''
An URL field that automatically strips extra spaces
'''
def to_python(self, value):
value = super(URLField, self).to_python(value)
if value:
return value.strip()
16 changes: 15 additions & 1 deletion udata/tests/dataset/test_dataset_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
from udata.core.discussions.factories import (
MessageDiscussionFactory, DiscussionFactory
)
from udata.core.organization.factories import OrganizationFactory
from udata.core.user.factories import UserFactory

from .. import TestCase, DBTestMixin
Expand Down Expand Up @@ -221,3 +220,18 @@ def test_send_on_delete(self):
with self.assert_emit(Dataset.on_delete):
dataset.deleted = datetime.now()
dataset.save()


class ResourceModelTest(TestCase, DBTestMixin):
def test_url_is_required(self):
with self.assertRaises(db.ValidationError):
DatasetFactory(resources=[ResourceFactory(url=None)])

def test_bad_url(self):
with self.assertRaises(db.ValidationError):
DatasetFactory(resources=[ResourceFactory(url='not-an-url')])

def test_url_is_stripped(self):
url = 'http://www.somewhere.com/with/spaces/ '
dataset = DatasetFactory(resources=[ResourceFactory(url=url)])
self.assertEqual(dataset.resources[0].url, url.strip())
74 changes: 74 additions & 0 deletions udata/tests/forms/test_basic_fields.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
# -*- coding: utf-8 -*-
from __future__ import unicode_literals

from werkzeug.datastructures import MultiDict

from udata.forms import Form, fields
from udata.models import db
from udata.tests import TestCase
from udata.utils import faker


class UrlFieldTest(TestCase):
def factory(self):
class Fake(db.Document):
url = db.URLField()

class FakeForm(Form):
url = fields.URLField()

return Fake, FakeForm

def test_empty_data(self):
Fake, FakeForm = self.factory()

form = FakeForm()
self.assertEqual(form.url.data, None)

fake = Fake()
form.populate_obj(fake)
self.assertIsNone(fake.url)

def test_initial_values(self):
Fake, FakeForm = self.factory()

fake = Fake(url=faker.url())
form = FakeForm(None, obj=fake)
self.assertEqual(form.url.data, fake.url)

def test_with_valid_url(self):
Fake, FakeForm = self.factory()

fake = Fake()
url = faker.url()
form = FakeForm(MultiDict({'url': url}))

form.validate()
self.assertEqual(form.errors, {})

form.populate_obj(fake)

self.assertEqual(fake.url, url)

def test_with_valid_url_is_stripped(self):
Fake, FakeForm = self.factory()

fake = Fake()
url = faker.url()
form = FakeForm(MultiDict({'url': url + ' '}))

form.validate()
self.assertEqual(form.errors, {})

form.populate_obj(fake)

self.assertEqual(fake.url, url)

def test_with_invalid_url(self):
Fake, FakeForm = self.factory()

form = FakeForm(MultiDict({'url': 'this-is-not-an-url'}))

form.validate()
self.assertIn('url', form.errors)
self.assertEqual(len(form.errors['url']), 1)
10 changes: 10 additions & 0 deletions udata/tests/frontend/test_dataset_frontend.py
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,16 @@ def test_resource_latest_url_404(self):
id=resource.id))
self.assert404(response)

def test_resource_latest_url_stripped(self):
'''It should return strip extras spaces from the resource URL'''
url = 'http://www.somewhere.com/path/with/spaces/ '
resource = ResourceFactory(url=url)
DatasetFactory(resources=[resource])
response = self.get(url_for('datasets.resource',
id=resource.id))
self.assertStatus(response, 302)
self.assertEqual(response.location, url.strip())

def test_recent_feed(self):
datasets = [DatasetFactory(
resources=[ResourceFactory()]) for i in range(3)]
Expand Down
24 changes: 24 additions & 0 deletions udata/tests/test_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ class DatetimedTester(db.Datetimed, db.Document):
name = db.StringField()


class URLTester(db.Document):
url = db.URLField()


class AutoUUIDFieldTest(DBTestMixin, TestCase):
def test_auto_populate(self):
'''AutoUUIDField should populate itself if not set'''
Expand Down Expand Up @@ -204,6 +208,26 @@ def test_not_valid(self):
obj.save()


class URLFieldTest(DBTestMixin, TestCase):
def test_none_if_empty_and_not_required(self):
obj = URLTester()
self.assertIsNone(obj.url)
obj.save()
obj.reload()
self.assertIsNone(obj.url)

def test_not_valid(self):
obj = URLTester(url='invalid')
with self.assertRaises(ValidationError):
obj.save()

def test_strip_spaces(self):
url = ' https://www.somewhere.com/with/spaces/ '
obj = URLTester(url=url)
obj.save().reload()
self.assertEqual(obj.url, url.strip())


class DatetimedTest(DBTestMixin, TestCase):
def test_class(self):
self.assertIsInstance(DatetimedTester.created_at, db.DateTimeField)
Expand Down