diff --git a/controller/Database.py b/controller/Database.py index 22a2000..1ee9391 100644 --- a/controller/Database.py +++ b/controller/Database.py @@ -259,7 +259,23 @@ class Valid_id( object ): self.__none_okay = none_okay def __call__( self, value ): - if self.__none_okay and value in ( None, "" ): return None + if self.__none_okay and value in ( None, "None", "" ): return None if self.ID_PATTERN.search( value ): return str( value ) raise ValueError() + + +class Valid_revision( object ): + """ + Validator for an object id. + """ + REVISION_PATTERN = re.compile( "^\d\d\d\d-\d\d-\d\d \d\d:\d\d:\d\d\.\d+$" ) + + def __init__( self, none_okay = False ): + self.__none_okay = none_okay + + def __call__( self, value ): + if self.__none_okay and value in ( None, "None", "" ): return None + if self.REVISION_PATTERN.search( value ): return str( value ) + + raise ValueError() diff --git a/controller/Notebooks.py b/controller/Notebooks.py index 1a00015..7506ab0 100644 --- a/controller/Notebooks.py +++ b/controller/Notebooks.py @@ -2,7 +2,7 @@ import cherrypy from Scheduler import Scheduler from Expose import expose from Validate import validate, Valid_string, Validation_error, Valid_bool -from Database import Valid_id +from Database import Valid_id, Valid_revision from Users import grab_user_id from Updater import wait_for_update, update_client from Expire import strongly_expire @@ -52,7 +52,7 @@ class Notebooks( object ): @validate( notebook_id = Valid_id(), note_id = Valid_id(), - revision = Valid_string( min = 19, max = 30 ), + revision = Valid_revision(), ) def default( self, notebook_id, note_id = None, revision = None ): """ @@ -135,7 +135,7 @@ class Notebooks( object ): @validate( notebook_id = Valid_id(), note_id = Valid_id(), - revision = Valid_string( min = 19, max = 30 ), + revision = Valid_revision(), user_id = Valid_id( none_okay = True ), ) def load_note( self, notebook_id, note_id, revision = None, user_id = None ): @@ -269,14 +269,15 @@ class Notebooks( object ): note_id = Valid_id(), contents = Valid_string( min = 1, max = 25000, escape_html = False ), startup = Valid_bool(), + previous_revision = Valid_revision( none_okay = True ), user_id = Valid_id( none_okay = True ), ) - def save_note( self, notebook_id, note_id, contents, startup, user_id ): + def save_note( self, notebook_id, note_id, contents, startup, previous_revision, user_id ): """ Save a new revision of the given note. This function will work both for creating a new note and for updating an existing note. If the note exists and the given contents are identical to the - existing contents, then no saving takes place and a new_revision of None is returned. Otherwise - this method returns the timestamp of the new revision. + existing contents for the given previous_revision, then no saving takes place and a new_revision + of None is returned. Otherwise this method returns the timestamp of the new revision. @type notebook_id: unicode @param notebook_id: id of notebook the note is in @@ -286,10 +287,16 @@ class Notebooks( object ): @param contents: new textual contents of the note, including its title @type startup: bool @param startup: whether the note should be displayed on startup + @type previous_revision: unicode or NoneType + @param previous_revision: previous known revision timestamp of the provided note, or None if + the note is new @type user_id: unicode or NoneType @param user_id: id of current logged-in user (if any), determined by @grab_user_id @rtype: json dict - @return: { 'new_revision': new revision of saved note, or None } + @return: { + 'new_revision': new revision of saved note, or None if nothing was saved, + 'previous_revision': revision immediately before new_revision, or None if the note is new + } @raise Access_error: the current user doesn't have access to the given notebook @raise Validation_error: one of the arguments is invalid """ @@ -306,26 +313,40 @@ class Notebooks( object ): self.__database.load( note_id, self.__scheduler.thread ) note = ( yield Scheduler.SLEEP ) - # if the note is already in the database, load it and update it. otherwise, create it + # if the note is already in the database, load it and update it if note and note in notebook.notes: - orig_revision = note.revision - notebook.update_note( note, contents ) + # check whether the provided note contents have been changed since the previous revision + self.__database.load( note_id, self.__scheduler.thread, previous_revision ) + old_note = ( yield Scheduler.SLEEP ) + + # the note hasn't been changed, so bail without updating it + if contents == old_note.contents: + previous_revision = note.revision + new_revision = None + # the note has changed, so update it + else: + previous_revision = note.revision + notebook.update_note( note, contents ) + new_revision = note.revision + # the note is not already in the database, so create it else: - orig_revision = None + previous_revision = None note = Note( note_id, contents ) notebook.add_note( note ) + new_revision = note.revision if startup: - notebook.add_startup_note( note ) + startup_changed = notebook.add_startup_note( note ) else: - notebook.remove_startup_note( note ) + startup_changed = notebook.remove_startup_note( note ) - self.__database.save( notebook ) + if new_revision or startup_changed: + self.__database.save( notebook ) - if note.revision == orig_revision: - yield dict( new_revision = None ) - else: - yield dict( new_revision = note.revision ) + yield dict( + new_revision = new_revision, + previous_revision = previous_revision, + ) @expose( view = Json ) @wait_for_update @@ -508,6 +529,12 @@ class Notebooks( object ): note = ( yield Scheduler.SLEEP ) if note and notebook.trash: + # if the note isn't deleted, and it's already in this notebook, just return + if note.deleted_from is None and notebook.lookup_note( note.object_id ): + yield dict() + return + + # if the note was deleted from a different notebook than the notebook given, raise if note.deleted_from != notebook_id: raise Access_error() diff --git a/controller/test/Test_notebooks.py b/controller/test/Test_notebooks.py index f7b707c..7e2b084 100644 --- a/controller/test/Test_notebooks.py +++ b/controller/test/Test_notebooks.py @@ -310,9 +310,11 @@ class Test_notebooks( Test_controller ): note_id = self.note.object_id, contents = new_note_contents, startup = startup, + previous_revision = previous_revision, ), session_id = self.session_id ) assert result[ "new_revision" ] and result[ "new_revision" ] != previous_revision + assert result[ "previous_revision" ] == previous_revision # make sure the old title can no longer be loaded result = self.http_post( "/notebooks/load_note_by_title/", dict( @@ -359,6 +361,154 @@ class Test_notebooks( Test_controller ): def test_save_startup_note_without_login( self ): self.test_save_note_without_login( startup = True ) + def test_save_unchanged_note( self, startup = False ): + self.login() + + # save over an existing note supplying new contents and a new title + previous_revision = self.note.revision + new_note_contents = u"

new title

new blah" + self.http_post( "/notebooks/save_note/", dict( + notebook_id = self.notebook.object_id, + note_id = self.note.object_id, + contents = new_note_contents, + startup = startup, + previous_revision = previous_revision, + ), session_id = self.session_id ) + + # now attempt to save over that note again without changing the contents + previous_revision = self.note.revision + result = self.http_post( "/notebooks/save_note/", dict( + notebook_id = self.notebook.object_id, + note_id = self.note.object_id, + contents = new_note_contents, + startup = startup, + previous_revision = previous_revision, + ), session_id = self.session_id ) + + # assert that the note wasn't actually updated the second time + assert result[ "new_revision" ] == None + assert result[ "previous_revision" ] == previous_revision + + result = self.http_post( "/notebooks/load_note_by_title/", dict( + notebook_id = self.notebook.object_id, + note_title = "new title", + ), session_id = self.session_id ) + + note = result[ "note" ] + + assert note.object_id == self.note.object_id + assert note.title == self.note.title + assert note.contents == self.note.contents + assert note.revision == previous_revision + + def test_save_unchanged_note_with_startup_change( self, startup = False ): + self.login() + + # save over an existing note supplying new contents and a new title + previous_revision = self.note.revision + new_note_contents = u"

new title

new blah" + self.http_post( "/notebooks/save_note/", dict( + notebook_id = self.notebook.object_id, + note_id = self.note.object_id, + contents = new_note_contents, + startup = startup, + previous_revision = previous_revision, + ), session_id = self.session_id ) + + # now attempt to save over that note again without changing the contents, but with a change + # to its startup flag + previous_revision = self.note.revision + result = self.http_post( "/notebooks/save_note/", dict( + notebook_id = self.notebook.object_id, + note_id = self.note.object_id, + contents = new_note_contents, + startup = not startup, + previous_revision = previous_revision, + ), session_id = self.session_id ) + + # assert that the note wasn't actually updated the second time + assert result[ "new_revision" ] == None + assert result[ "previous_revision" ] == previous_revision + + result = self.http_post( "/notebooks/load_note_by_title/", dict( + notebook_id = self.notebook.object_id, + note_title = "new title", + ), session_id = self.session_id ) + + note = result[ "note" ] + + assert note.object_id == self.note.object_id + assert note.title == self.note.title + assert note.contents == self.note.contents + assert note.revision == previous_revision + + # assert that the notebook now has the proper startup status for the note + if startup: + assert note not in self.notebook.startup_notes + else: + assert note in self.notebook.startup_notes + + def test_save_note_from_an_older_revision( self ): + self.login() + + # save over an existing note supplying new contents and a new title + first_revision = self.note.revision + new_note_contents = u"

new title

new blah" + result = self.http_post( "/notebooks/save_note/", dict( + notebook_id = self.notebook.object_id, + note_id = self.note.object_id, + contents = new_note_contents, + startup = False, + previous_revision = first_revision, + ), session_id = self.session_id ) + + # save over that note again with new contents, providing the original + # revision as the previous known revision + second_revision = self.note.revision + new_note_contents = u"

new new title

new new blah" + result = self.http_post( "/notebooks/save_note/", dict( + notebook_id = self.notebook.object_id, + note_id = self.note.object_id, + contents = new_note_contents, + startup = False, + previous_revision = first_revision, + ), session_id = self.session_id ) + + # make sure the second save actually caused an update + assert result[ "new_revision" ] + assert result[ "new_revision" ] not in ( first_revision, second_revision ) + assert result[ "previous_revision" ] == second_revision + + # make sure the first title can no longer be loaded + result = self.http_post( "/notebooks/load_note_by_title/", dict( + notebook_id = self.notebook.object_id, + note_title = "my title", + ), session_id = self.session_id ) + + note = result[ "note" ] + assert note == None + + # make sure the second title can no longer be loaded + result = self.http_post( "/notebooks/load_note_by_title/", dict( + notebook_id = self.notebook.object_id, + note_title = "new title", + ), session_id = self.session_id ) + + note = result[ "note" ] + assert note == None + + # make sure the new title is now loadable + result = self.http_post( "/notebooks/load_note_by_title/", dict( + notebook_id = self.notebook.object_id, + note_title = "new new title", + ), session_id = self.session_id ) + + note = result[ "note" ] + + assert note.object_id == self.note.object_id + assert note.title == self.note.title + assert note.contents == self.note.contents + def test_save_note_with_unknown_notebook( self ): self.login() @@ -384,9 +534,11 @@ class Test_notebooks( Test_controller ): note_id = new_note.object_id, contents = new_note.contents, startup = startup, + previous_revision = None, ), session_id = self.session_id ) assert result[ "new_revision" ] and result[ "new_revision" ] != previous_revision + assert result[ "previous_revision" ] == None # make sure the new title is now loadable result = self.http_post( "/notebooks/load_note_by_title/", dict( @@ -424,9 +576,11 @@ class Test_notebooks( Test_controller ): note_id = new_note.object_id, contents = new_note.contents, startup = False, + previous_revision = None, ), session_id = self.session_id ) assert result[ "new_revision" ] and result[ "new_revision" ] != previous_revision + assert result[ "previous_revision" ] == None # make sure the new title is now loadable result = self.http_post( "/notebooks/load_note_by_title/", dict( @@ -455,9 +609,11 @@ class Test_notebooks( Test_controller ): note_id = new_note.object_id, contents = new_note.contents, startup = False, + previous_revision = None, ), session_id = self.session_id ) assert result[ "new_revision" ] and result[ "new_revision" ] != previous_revision + assert result[ "previous_revision" ] == None # make sure the new title is now loadable result = self.http_post( "/notebooks/load_note_by_title/", dict( @@ -713,6 +869,36 @@ class Test_notebooks( Test_controller ): assert result.get( "note" ) == None + def test_undelete_note_that_is_not_deleted( self ): + self.login() + + # "undelete" the note + result = self.http_post( "/notebooks/undelete_note/", dict( + notebook_id = self.notebook.object_id, + note_id = self.note.object_id, + ), session_id = self.session_id ) + + assert result.get( "error" ) == None + + # test that the "undeleted" is where it should be + result = self.http_post( "/notebooks/load_note/", dict( + notebook_id = self.notebook.object_id, + note_id = self.note.object_id, + ), session_id = self.session_id ) + + note = result.get( "note" ) + assert note + assert note.object_id == self.note.object_id + assert note.deleted_from == None + + # test that the note is not somehow in the trash + result = self.http_post( "/notebooks/load_note/", dict( + notebook_id = self.notebook.trash.object_id, + note_id = self.note.object_id, + ), session_id = self.session_id ) + + assert result.get( "note" ) == None + def test_undelete_note_without_login( self ): result = self.http_post( "/notebooks/undelete_note/", dict( notebook_id = self.notebook.object_id, @@ -789,6 +975,26 @@ class Test_notebooks( Test_controller ): assert note.object_id == self.note.object_id assert note.deleted_from == self.notebook.object_id + def test_undelete_note_that_is_not_deleted_from_incorrect_notebook( self ): + self.login() + + result = self.http_post( "/notebooks/undelete_note/", dict( + notebook_id = self.anon_notebook, + note_id = self.note.object_id, + ), session_id = self.session_id ) + + assert result.get( "error" ) + + # test that the note is still in its notebook + result = self.http_post( "/notebooks/load_note/", dict( + notebook_id = self.notebook.object_id, + note_id = self.note.object_id, + ), session_id = self.session_id ) + + note = result.get( "note" ) + assert note.object_id == self.note.object_id + assert note.deleted_from == None + def test_blank_note( self ): result = self.http_get( "/notebooks/blank_note/5" ) assert result[ u"id" ] == u"5" diff --git a/model/Notebook.py b/model/Notebook.py index aa31cf6..f4f434e 100644 --- a/model/Notebook.py +++ b/model/Notebook.py @@ -106,9 +106,6 @@ class Notebook( Persistent ): if old_note is None: raise Notebook.UnknownNoteError( note.object_id ) - if contents == old_note.contents: - return - self.update_revision() self.__titles.pop( note.title, None ) diff --git a/model/test/Test_notebook.py b/model/test/Test_notebook.py index d71c689..0b7839f 100644 --- a/model/test/Test_notebook.py +++ b/model/test/Test_notebook.py @@ -99,17 +99,6 @@ class Test_notebook( object ): note = self.notebook.lookup_note_by_title( new_title ) assert note == self.note - def test_update_unrevised_note( self ): - self.notebook.add_note( self.note ) - old_title = self.note.title - - revision = self.notebook.revision - self.notebook.update_note( self.note, self.note.contents ) - assert self.notebook.revision == revision - - note = self.notebook.lookup_note( self.note.object_id ) - assert note == self.note - @raises( Notebook.UnknownNoteError ) def test_update_unknown_note( self ): new_contents = u"

new title

new blah" diff --git a/static/css/style.css b/static/css/style.css index 8e425fd..236e706 100644 --- a/static/css/style.css +++ b/static/css/style.css @@ -248,6 +248,7 @@ ol li { .message_inner { padding: 0.5em; + line-height: 140%; background-color: #ffaa44; -moz-border-radius: 0.5em; -webkit-border-radius: 0.5em; @@ -266,6 +267,7 @@ ol li { .error_inner { padding: 0.5em; + line-height: 140%; color: #ffffff; background-color: #dd1111; -moz-border-radius: 0.5em; diff --git a/static/js/Editor.js b/static/js/Editor.js index 9002d96..9a023c4 100644 --- a/static/js/Editor.js +++ b/static/js/Editor.js @@ -3,7 +3,7 @@ function Editor( id, notebook_id, note_text, deleted_from, revisions_list, inser this.notebook_id = notebook_id; this.initial_text = note_text; this.deleted_from = deleted_from || null; - this.revisions_list = revisions_list; + this.revisions_list = revisions_list || new Array(); this.read_write = read_write; this.startup = startup || false; // whether this Editor is for a startup note this.init_highlight = highlight || false; diff --git a/static/js/Wiki.js b/static/js/Wiki.js index bb5fe21..3e85df8 100644 --- a/static/js/Wiki.js +++ b/static/js/Wiki.js @@ -568,19 +568,19 @@ Wiki.prototype.delete_editor = function ( event, editor ) { if ( editor == this.focused_editor ) this.focused_editor = null; - editor.shutdown(); - } + if ( this.notebook.trash && !editor.empty() ) { + var undo_button = createDOM( "input", { + "type": "button", + "class": "message_button", + "value": "undo", + "title": "undo deletion" + } ); + this.display_message( "The note has been moved to the trash.", [ undo_button ] ) + var self = this; + connect( undo_button, "onclick", function ( event ) { self.undelete_editor_via_undo( event, editor ); } ); + } - if ( this.notebook.trash ) { - var undo_button = createDOM( "input", { - "type": "button", - "class": "message_button", - "value": "undo", - "title": "undo deletion" - } ); - this.display_message( "The note has been moved to the trash.", [ undo_button ] ) - var self = this; - connect( undo_button, "onclick", function ( event ) { self.undelete_editor_via_undo( event, editor ); } ); + editor.shutdown(); } event.stop(); @@ -636,28 +636,60 @@ Wiki.prototype.undelete_editor_via_undo = function( event, editor ) { event.stop(); } +Wiki.prototype.compare_versions = function( event, editor, previous_revision ) { + this.clear_pulldowns(); + + // display the two revisions for comparison by the user + this.load_editor( editor.title, null, editor.id, previous_revision ); + this.load_editor( editor.title, null, editor.id ); +} + Wiki.prototype.save_editor = function ( editor, fire_and_forget ) { if ( !editor ) editor = this.focused_editor; var self = this; if ( editor && editor.read_write && !editor.empty() ) { + var revisions = editor.revisions_list; this.invoker.invoke( "/notebooks/save_note", "POST", { "notebook_id": this.notebook_id, "note_id": editor.id, "contents": editor.contents(), - "startup": editor.startup + "startup": editor.startup, + "previous_revision": revisions.length ? revisions[ revisions.length - 1 ] : "None" }, function ( result ) { self.update_editor_revisions( result, editor ); }, null, fire_and_forget ); } } Wiki.prototype.update_editor_revisions = function ( result, editor ) { - if ( result.new_revision ) { - if ( !editor.revisions_list ) - editor.revisions_list = new Array(); + // if there's not a newly saved revision, then the contents are unchanged, so bail + if ( !result.new_revision ) + return; - editor.revisions_list.push( result.new_revision ); + var revisions = editor.revisions_list; + var client_previous_revision = revisions.length ? revisions[ revisions.length - 1 ] : null; + + // if the server's idea of the previous revision doesn't match the client's, then someone has + // gone behind our back and saved the editor's note from another window + if ( result.previous_revision != client_previous_revision ) { + var compare_button = createDOM( "input", { + "type": "button", + "class": "message_button", + "value": "compare versions", + "title": "compare your version with the modified version" + } ); + this.display_error( 'Your changes to the note titled "' + editor.title + '" have overwritten changes made in another window.', [ compare_button ] ); + + var self = this; + connect( compare_button, "onclick", function ( event ) { + self.compare_versions( event, editor, result.previous_revision ); + } ); + + revisions.push( result.previous_revision ); } + + // add the new revision to the editor's revisions list + revisions.push( result.new_revision ); } Wiki.prototype.search = function ( event ) { @@ -722,9 +754,8 @@ Wiki.prototype.display_message = function ( text, buttons ) { this.clear_pulldowns(); var inner_div = DIV( { "class": "message_inner" }, text + " " ); - for ( var i in buttons ) { + for ( var i in buttons ) appendChildNodes( inner_div, buttons[ i ] ); - } var div = DIV( { "class": "message" }, inner_div ); div.buttons = buttons; @@ -733,7 +764,7 @@ Wiki.prototype.display_message = function ( text, buttons ) { ScrollTo( div ); } -Wiki.prototype.display_error = function ( text ) { +Wiki.prototype.display_error = function ( text, buttons ) { this.clear_messages(); this.clear_pulldowns(); @@ -745,8 +776,13 @@ Wiki.prototype.display_error = function ( text ) { editor.shutdown(); } - var inner_div = DIV( { "class": "error_inner" }, text ); + var inner_div = DIV( { "class": "error_inner" }, text + " " ); + for ( var i in buttons ) + appendChildNodes( inner_div, buttons[ i ] ); + var div = DIV( { "class": "error" }, inner_div ); + div.buttons = buttons; + appendChildNodes( "notes", div ); ScrollTo( div ); }