Merge branch '216_cwebber_style_unique_slugs'

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)
+        #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
 
-        duplicate = check_media_slug_used(self.uploader, self.slug, self.id)
+        # Do we have anything at this point?
-
+        # If not, we're not going to get a slug
-        if duplicate:
+        # so just return... we're not going to force one.
-            if self.id is not None:
+        if not self.slug:
-                self.slug = u"%s-%s" % (self.id, self.slug)
+            return  # giving up!
-            else:
+  
-                self.slug = None
+        # 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):

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

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'-'):

setup.py

@@ -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