diff options
| -rw-r--r-- | mediagoblin/db/mixin.py | 70 | ||||
| -rw-r--r-- | mediagoblin/tests/test_modelmethods.py | 130 | ||||
| -rw-r--r-- | mediagoblin/tools/url.py | 2 | ||||
| -rw-r--r-- | setup.py | 1 |
4 files changed, 193 insertions, 10 deletions
diff --git a/mediagoblin/db/mixin.py b/mediagoblin/db/mixin.py index 001b7826..b69e7d51 100644 --- a/mediagoblin/db/mixin.py +++ b/mediagoblin/db/mixin.py @@ -27,6 +27,8 @@ These functions now live here and get "mixed in" into the real objects. """ +import uuid + from werkzeug.utils import cached_property from mediagoblin import mg_globals @@ -52,19 +54,69 @@ class UserMixin(object): class MediaEntryMixin(object): def generate_slug(self): + """ + Generate a unique slug for this MediaEntry. + + This one does not *force* slugs, but usually it will probably result + in a niceish one. + + The end *result* of the algorithm will result in these resolutions for + these situations: + - If we have a slug, make sure it's clean and sanitized, and if it's + unique, we'll use that. + - If we have a title, slugify it, and if it's unique, we'll use that. + - If we can't get any sort of thing that looks like it'll be a useful + slug out of a title or an existing slug, bail, and don't set the + slug at all. Don't try to create something just because. Make + sure we have a reasonable basis for a slug first. + - If we have a reasonable basis for a slug (either based on existing + slug or slugified title) but it's not unique, first try appending + the entry's id, if that exists + - If that doesn't result in something unique, tack on some randomly + generated bits until it's unique. That'll be a little bit of junk, + but at least it has the basis of a nice slug. + """ # import this here due to a cyclic import issue # (db.models -> db.mixin -> db.util -> db.models) from mediagoblin.db.util import check_media_slug_used - self.slug = slugify(self.title) - - duplicate = check_media_slug_used(self.uploader, self.slug, self.id) - - if duplicate: - if self.id is not None: - self.slug = u"%s-%s" % (self.id, self.slug) - else: - self.slug = None + #Is already a slug assigned? Check if it is valid + if self.slug: + self.slug = slugify(self.slug) + + # otherwise, try to use the title. + elif self.title: + # assign slug based on title + self.slug = slugify(self.title) + + # We don't want any empty string slugs + if self.slug == u"": + self.slug = None + + # Do we have anything at this point? + # If not, we're not going to get a slug + # so just return... we're not going to force one. + if not self.slug: + return # giving up! + + # Otherwise, let's see if this is unique. + if check_media_slug_used(self.uploader, self.slug, self.id): + # It looks like it's being used... lame. + + # Can we just append the object's id to the end? + if self.id: + slug_with_id = u"%s-%s" % (self.slug, self.id) + if not check_media_slug_used(self.uploader, + slug_with_id, self.id): + self.slug = slug_with_id + return # success! + + # okay, still no success; + # let's whack junk on there till it's unique. + self.slug += '-' + uuid.uuid4().hex[:4] + # keep going if necessary! + while check_media_slug_used(self.uploader, self.slug, self.id): + self.slug += uuid.uuid4().hex[:4] @property def description_html(self): diff --git a/mediagoblin/tests/test_modelmethods.py b/mediagoblin/tests/test_modelmethods.py new file mode 100644 index 00000000..4f2df19b --- /dev/null +++ b/mediagoblin/tests/test_modelmethods.py @@ -0,0 +1,130 @@ +# GNU MediaGoblin -- federated, autonomous media hosting +# Copyright (C) 2011, 2012 MediaGoblin contributors. See AUTHORS. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# 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/>. + +# Maybe not every model needs a test, but some models have special +# methods, and so it makes sense to test them here. + + +from mediagoblin.db.models import MediaEntry + +from mediagoblin.tests.tools import get_test_app, \ + fixture_add_user + +import mock + + +class FakeUUID(object): + hex = 'testtest-test-test-test-testtesttest' + +UUID_MOCK = mock.Mock(return_value=FakeUUID()) + + +class TestMediaEntrySlugs(object): + def setUp(self): + self.test_app = get_test_app(dump_old_app=True) + self.chris_user = fixture_add_user(u'chris') + self.emily_user = fixture_add_user(u'emily') + self.existing_entry = self._insert_media_entry_fixture( + title=u"Beware, I exist!", + slug=u"beware-i-exist") + + def _insert_media_entry_fixture(self, title=None, slug=None, this_id=None, + uploader=None, save=True): + entry = MediaEntry() + entry.title = title or u"Some title" + entry.slug = slug + entry.id = this_id + entry.uploader = uploader or self.chris_user.id + entry.media_type = u'image' + + if save: + entry.save() + + return entry + + def test_unique_slug_from_title(self): + entry = self._insert_media_entry_fixture(u"Totally unique slug!", save=False) + entry.generate_slug() + assert entry.slug == u'totally-unique-slug' + + def test_old_good_unique_slug(self): + entry = self._insert_media_entry_fixture( + u"A title here", u"a-different-slug-there", save=False) + entry.generate_slug() + assert entry.slug == u"a-different-slug-there" + + def test_old_weird_slug(self): + entry = self._insert_media_entry_fixture( + slug=u"wowee!!!!!", save=False) + entry.generate_slug() + assert entry.slug == u"wowee" + + def test_existing_slug_use_id(self): + entry = self._insert_media_entry_fixture( + u"Beware, I exist!!", this_id=9000, save=False) + entry.generate_slug() + assert entry.slug == u"beware-i-exist-9000" + + @mock.patch('uuid.uuid4', UUID_MOCK) + def test_existing_slug_cant_use_id(self): + # This one grabs the nine thousand slug + self._insert_media_entry_fixture( + slug=u"beware-i-exist-9000") + + entry = self._insert_media_entry_fixture( + u"Beware, I exist!!", this_id=9000, save=False) + entry.generate_slug() + assert entry.slug == u"beware-i-exist-test" + + @mock.patch('uuid.uuid4', UUID_MOCK) + def test_existing_slug_cant_use_id_extra_junk(self): + # This one grabs the nine thousand slug + self._insert_media_entry_fixture( + slug=u"beware-i-exist-9000") + + # This one grabs makes sure the annoyance doesn't stop + self._insert_media_entry_fixture( + slug=u"beware-i-exist-test") + + entry = self._insert_media_entry_fixture( + u"Beware, I exist!!", this_id=9000, save=False) + entry.generate_slug() + assert entry.slug == u"beware-i-exist-testtest" + + def test_garbage_slug(self): + """ + Titles that sound totally like Q*Bert shouldn't have slugs at + all. We'll just reference them by id. + + , + / \ (@!#?@!) + |\,/| ,-, / + | |#| ( ")~ + / \|/ \ L L + |\,/|\,/| + | |#, |#| + / \|/ \|/ \ + |\,/|\,/|\,/| + | |#| |#| |#| + / \|/ \|/ \|/ \ + |\,/|\,/|\,/|\,/| + | |#| |#| |#| |#| + \|/ \|/ \|/ \|/ + """ + qbert_entry = self._insert_media_entry_fixture( + u"@!#?@!", save=False) + qbert_entry.generate_slug() + assert qbert_entry.slug is None diff --git a/mediagoblin/tools/url.py b/mediagoblin/tools/url.py index 8604ad5f..d9179f9e 100644 --- a/mediagoblin/tools/url.py +++ b/mediagoblin/tools/url.py @@ -25,7 +25,7 @@ except ImportError: USING_TRANSLITCODEC = False -_punct_re = re.compile(r'[\t !"#$%&\'()*\-/<=>?@\[\\\]^_`{|},.]+') +_punct_re = re.compile(r'[\t !"#:$%&\'()*\-/<=>?@\[\\\]^_`{|},.]+') def slugify(text, delim=u'-'): @@ -59,6 +59,7 @@ setup( 'Markdown', 'sqlalchemy>=0.7.0', 'sqlalchemy-migrate', + 'mock', ## This is optional! # 'translitcodec', ## For now we're expecting that users will install this from |