From d3e040d984166791fcb8becbbca0c44350ddfff7 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Tue, 18 Nov 2008 15:11:58 -0800 Subject: [PATCH] Blog post URLs are now user-friendly and SEO-friendly. --- controller/Forums.py | 28 +++++++++++++++++++++++----- controller/Validate.py | 11 +++++++++++ controller/test/Test_forums.py | 3 +-- controller/test/Test_root.py | 27 --------------------------- model/Notebook.py | 12 ++++++++++++ model/delta/1.5.7.sql | 4 ++++ model/drop.sql | 1 + model/schema.sql | 6 ++++++ model/test/Test_notebook.py | 5 +++++ view/Forum_page.py | 2 +- view/Main_page.py | 5 ++++- 11 files changed, 68 insertions(+), 36 deletions(-) create mode 100644 model/delta/1.5.7.sql diff --git a/controller/Forums.py b/controller/Forums.py index b1a78ab..44652df 100644 --- a/controller/Forums.py +++ b/controller/Forums.py @@ -6,7 +6,7 @@ from model.Note import Note from model.Tag import Tag from Expose import expose from Expire import strongly_expire -from Validate import validate, Valid_string, Valid_int +from Validate import validate, Valid_string, Valid_int, Valid_friendly_id from Database import Valid_id, end_transaction from Users import grab_user_id from Notebooks import Notebooks @@ -165,7 +165,7 @@ class Forum( object ): @end_transaction @grab_user_id @validate( - thread_id = Valid_id(), + thread_id = unicode, start = Valid_int( min = 0 ), count = Valid_int( min = 1, max = 50 ), note_id = Valid_id( none_okay = True ), @@ -176,7 +176,7 @@ class Forum( object ): Provide the information necessary to display a forum thread. @type thread_id: unicode - @param thread_id: id of thread notebook to display + @param thread_id: id or "friendly id" of thread notebook to display @type start: unicode or NoneType @param start: index of recent note to start with (defaults to 0, the most recent note) @type count: int or NoneType @@ -187,6 +187,26 @@ class Forum( object ): @return: rendered HTML page @raise Validation_error: one of the arguments is invalid """ + # first try loading the thread by id, and then if not found, try loading by "friendly id" + try: + Valid_id()( thread_id ) + if not self.__database.load( Notebook, thread_id ): + raise ValueError() + except ValueError: + try: + Valid_friendly_id()( thread_id ) + except ValueError: + raise cherrypy.NotFound + + try: + thread = self.__database.select_one( Notebook, Notebook.sql_load_by_friendly_id( thread_id ) ) + except: + raise cherrypy.NotFound + if not thread: + raise cherrypy.NotFound + + thread_id = thread.object_id + result = self.__users.current( user_id ) result.update( self.__notebooks.old_notes( thread_id, start, count, user_id ) ) @@ -196,8 +216,6 @@ class Forum( object ): return result - default.exposed = True - @expose() @end_transaction @grab_user_id diff --git a/controller/Validate.py b/controller/Validate.py index 301c7e1..43e1c17 100644 --- a/controller/Validate.py +++ b/controller/Validate.py @@ -1,4 +1,5 @@ import cherrypy +import re from cgi import escape from Html_cleaner import Html_cleaner @@ -167,6 +168,16 @@ class Valid_int( object ): return value +class Valid_friendly_id( object ): + FRIENDLY_ID_PATTERN = re.compile( "^[a-zA-Z0-9\-]+$" ) + + def __call__( self, value ): + if self.FRIENDLY_ID_PATTERN.search( value ): + return value + + raise ValueError() + + def validate( **expected ): """ validate() can be used to require that the arguments of the decorated method successfully pass diff --git a/controller/test/Test_forums.py b/controller/test/Test_forums.py index 757f51c..2ab4e61 100644 --- a/controller/test/Test_forums.py +++ b/controller/test/Test_forums.py @@ -300,8 +300,7 @@ class Test_forums( Test_controller ): result = self.http_get( path ) headers = result.get( "headers" ) - assert headers - assert headers.get( "Location" ) == u"http:///login?after_login=%s" % urllib.quote( path ) + assert headers.get( "Status" ) == u"404 Not Found" def __make_notes( self ): note_id = self.database.next_id( Note, commit = False ) diff --git a/controller/test/Test_root.py b/controller/test/Test_root.py index aa3dc50..4241c6d 100644 --- a/controller/test/Test_root.py +++ b/controller/test/Test_root.py @@ -367,33 +367,6 @@ class Test_root( Test_controller ): assert result.get( "redirect" ) assert result.get( "redirect" ).startswith( "https://" ) - def test_blog( self ): - result = self.http_get( - "/blog", - ) - - assert result - assert u"error" not in result - assert result[ u"notebook" ].object_id == self.blog_notebook.object_id - - def test_blog_with_note_id( self ): - result = self.http_get( - "/blog?note_id=%s" % self.blog_note.object_id, - ) - - assert result - assert u"error" not in result - assert result[ u"notebook" ].object_id == self.blog_notebook.object_id - - def test_blog_rss( self ): - result = self.http_get( - "/blog?rss", - ) - - assert result - assert u"error" not in result - assert result[ u"notebook" ].object_id == self.blog_notebook.object_id - def test_guide( self ): result = self.http_get( "/guide", diff --git a/model/Notebook.py b/model/Notebook.py index 420bf40..58b5a18 100644 --- a/model/Notebook.py +++ b/model/Notebook.py @@ -126,6 +126,10 @@ class Notebook( Persistent ): def sql_update( self ): return self.sql_create() + @staticmethod + def sql_load_by_friendly_id( friendly_id ): + return "select * from notebook_current where friendly_id( name ) = %s;" % quote( friendly_id ) + def sql_load_notes( self, start = 0, count = None ): """ Return a SQL string to load a list of all the notes within this notebook. @@ -340,6 +344,7 @@ class Notebook( Persistent ): d.update( dict( name = self.__name, + friendly_id = self.friendly_id, trash_id = self.__trash_id, read_write = self.__read_write, owner = self.__owner, @@ -355,6 +360,12 @@ class Notebook( Persistent ): self.__name = name self.update_revision() + FRIENDLY_ID_STRIP_PATTERN = re.compile( "[^a-zA-Z0-9\-]+" ) + + def __friendly_id( self ): + friendly_id = self.WHITESPACE_PATTERN.sub( u"-", self.__name.lower() ) + return self.FRIENDLY_ID_STRIP_PATTERN.sub( u"", friendly_id ) + def __set_read_write( self, read_write ): # The read_write member isn't actually saved to the database, so setting it doesn't need to # call update_revision(). @@ -390,6 +401,7 @@ class Notebook( Persistent ): self.__tags = tags name = property( lambda self: self.__name, __set_name ) + friendly_id = property( __friendly_id ) trash_id = property( lambda self: self.__trash_id ) read_write = property( lambda self: self.__read_write, __set_read_write ) owner = property( lambda self: self.__owner, __set_owner ) diff --git a/model/delta/1.5.7.sql b/model/delta/1.5.7.sql new file mode 100644 index 0000000..ce4146d --- /dev/null +++ b/model/delta/1.5.7.sql @@ -0,0 +1,4 @@ +CREATE FUNCTION friendly_id(text) RETURNS text + AS $_$select regexp_replace( regexp_replace( lower( $1 ), '\\s+', '-', 'g' ), '[^a-zA-Z0-9\\-]', '', 'g' );$_$ + LANGUAGE sql IMMUTABLE; +CREATE INDEX notebook_friendly_id_index ON notebook USING btree (friendly_id(name)); diff --git a/model/drop.sql b/model/drop.sql index 0652bb8..8ace40d 100644 --- a/model/drop.sql +++ b/model/drop.sql @@ -19,3 +19,4 @@ DROP TABLE schema_version; DROP TABLE session; DROP FUNCTION drop_html_tags( text ); DROP FUNCTION log_note_revision(); +DROP FUNCTION friendly_id(text); diff --git a/model/schema.sql b/model/schema.sql index d40dad8..2e7543a 100644 --- a/model/schema.sql +++ b/model/schema.sql @@ -25,6 +25,10 @@ create function log_note_revision() returns trigger as $_$ end; $_$ language plpgsql; ALTER FUNCTION public.log_note_revision() OWNER TO luminotes; +CREATE FUNCTION friendly_id(text) RETURNS text + AS $_$select regexp_replace( regexp_replace( lower( $1 ), '\\s+', '-', 'g' ), '[^a-zA-Z0-9\\-]', '', 'g' );$_$ + LANGUAGE sql IMMUTABLE; +ALTER FUNCTION public.friendly_id(text) OWNER TO luminotes; CREATE TABLE file ( id text NOT NULL, revision timestamp with time zone, @@ -235,6 +239,8 @@ CREATE INDEX note_current_user_id_index ON note_current USING btree (user_id); CREATE INDEX note_current_search_index ON note_current USING gist (search); +CREATE INDEX notebook_friendly_id_index ON notebook USING btree (friendly_id(name)); + CREATE INDEX password_reset_email_address_index ON password_reset USING btree (email_address); CREATE INDEX download_access_transaction_id_index ON download_access USING btree (transaction_id); diff --git a/model/test/Test_notebook.py b/model/test/Test_notebook.py index 57a240f..994160a 100644 --- a/model/test/Test_notebook.py +++ b/model/test/Test_notebook.py @@ -173,6 +173,10 @@ class Test_notebook( object ): assert self.notebook.name == new_name assert self.notebook.revision > previous_revision + def test_friendly_id( self ): + self.notebook.name = u"This is Bob's notebook!" + assert self.notebook.friendly_id == u"this-is-bobs-notebook" + def test_set_read_write( self ): original_revision = self.notebook.revision self.notebook.read_write = Notebook.READ_WRITE_FOR_OWN_NOTES @@ -233,6 +237,7 @@ class Test_notebook( object ): d = self.notebook.to_dict() assert d.get( "name" ) == self.name + assert d.get( "friendly_id" ) == u"my-notebook" assert d.get( "trash_id" ) == self.trash.object_id assert d.get( "read_write" ) == self.read_write assert d.get( "deleted" ) == self.notebook.deleted diff --git a/view/Forum_page.py b/view/Forum_page.py index ed80123..d1320b1 100644 --- a/view/Forum_page.py +++ b/view/Forum_page.py @@ -49,7 +49,7 @@ class Forum_page( Product_page ): [ Div( A( thread.name, - href = os.path.join( base_path, thread.object_id ), + href = os.path.join( base_path, ( forum_name == u"blog" ) and thread.friendly_id or thread.object_id ), ), Span( self.post_count( thread, forum_name ), diff --git a/view/Main_page.py b/view/Main_page.py index 1eff93e..eb4a85d 100644 --- a/view/Main_page.py +++ b/view/Main_page.py @@ -103,7 +103,10 @@ class Main_page( Page ): notebook_path = u"/guide" elif forum_tags: forum_tag = forum_tags[ 0 ] - notebook_path = u"/forums/%s/%s" % ( forum_tag.value, notebook.object_id ) + if forum_tag.value == u"blog": + notebook_path = u"/blog/%s" % notebook.friendly_id + else: + notebook_path = u"/forums/%s/%s" % ( forum_tag.value, notebook.object_id ) else: notebook_path = u"/notebooks/%s" % notebook.object_id