Merge pull request #1349 from gollum/detect_simultaneous_edits

Implement infrastructure for detecting and handling simultaneous edits

lib/gollum/app.rb

@@ -176,6 +176,7 @@ module Precious
               @page         = page
               @page.version = wiki.repo.log(wiki.ref, @page.path).first
               @content      = page.text_data
+              @etag         = page.sha
               mustache :edit
           else
             redirect_to("/create/#{encodeURIComponent(@name)}")
@@ -278,11 +279,18 @@ module Precious
       end
 
       post '/edit/*' do
+        etag      = params[:etag]        
         path      = "/#{clean_url(sanitize_empty_params(params[:path]))}"
         page_name = CGI.unescape(params[:page])
         wiki      = wiki_new
         page      = wiki.paged(page_name, path, exact = true)
+        
         return if page.nil?
+        if etag != page.sha
+          # Signal edit collision and return the page's most recent version
+          halt 412, {etag: page.sha, text_data: page.text_data}.to_json
+        end
+        
         committer = Gollum::Committer.new(wiki, commit_message)
         commit    = { :committer => committer }
 
@@ -292,7 +300,6 @@ module Precious
         update_wiki_page(wiki, page.sidebar, params[:sidebar], commit) if params[:sidebar]
         committer.commit
 
-        redirect to("/#{page.escaped_url_path}") unless page.nil?
       end
 
 

lib/gollum/public/gollum/javascript/gollum.js.erb

@@ -367,6 +367,34 @@ $(document).ready(function() {
       window.onbeforeunload = function(){ return "Leaving will discard all edits!" };
     });
     $.GollumEditor();
+    $("#gollum-editor-submit").click( function(e) {
+      e.preventDefault();
+      // Prevent button from being clicked again
+      $(this).attr('disabled', true);
+
+      var formData = new FormData($('#gollum-editor-form').get(0));
+      var endpoint = $('#gollum-editor-form').attr("action");
+
+      $.ajax({
+        url: endpoint,
+        type: 'POST',
+        data: formData,
+        processData: false,
+        contentType: false,
+        success: function(data) {
+          window.location = window.location.href.replace(/gollum\/edit\//, '')
+        },
+        error: function(data, textStatus, errorThrown) {
+          if (data.status == 412) {
+            $('#gollum-editor-submit').attr('disabled', false)
+            alert('Someone else has modified this page while you were editing it. Please store your version on disk outside of the browser, reload this page and reapply your modifications.');
+          } else {
+            alert('Error updating page: ' + textStatus + ' ' + errorThrown);
+          }  
+        }
+      });
+      
+    });
   }
 
   if( $("#last-edit").length ) {

lib/gollum/templates/editor.mustache

@@ -1,9 +1,9 @@
 <div id="gollum-editor" data-escaped-name="{{escaped_name}}" class="{{#is_create_page}}create{{/is_create_page}}{{#is_edit_page}}edit{{/is_edit_page}} {{#allow_uploads}}uploads-allowed{{/allow_uploads}}">
 {{#is_create_page}}
-<form name="gollum-editor" action="{{create_path}}" method="post">
+<form id="gollum-editor-form" name="gollum-editor" action="{{create_path}}" method="post">
 {{/is_create_page}}
 {{#is_edit_page}}
-<form name="gollum-editor" action="{{edit_path}}/{{escaped_name}}" method="post">
+<form id="gollum-editor-form" name="gollum-editor" action="{{edit_path}}/{{escaped_name}}" method="post">
 {{/is_edit_page}}
   <fieldset id="gollum-editor-fields">
   {{#is_create_page}}
@@ -17,6 +17,7 @@
   {{/is_create_page}}
   {{#is_edit_page}}
   <input type="hidden" name="page" id="gollum-editor-page-title" value="{{page_name}}">
+  <input type="hidden" name="etag" id="gollum-editor-etag" value="{{etag}}">
   {{/is_edit_page}}
   <input type="hidden" name="path" id="gollum-editor-page-path" value="{{path}}">
   <div id="gollum-editor-function-bar">

lib/gollum/views/edit.rb

@@ -60,6 +60,9 @@ module Precious
         true
       end
 
+      def etag
+        @etag
+      end
       
       def allow_uploads
         @allow_uploads

test/test_app.rb

@@ -80,8 +80,7 @@ context "Frontend" do
   test "edits page" do
     page_1 = @wiki.page('A')
     post "/gollum/edit/A", :content => 'abc', :page => 'A',
-         :format             => page_1.format, :message => 'def'
+         :format => page_1.format, :message => 'def', :etag => page_1.sha
-    follow_redirect!
     assert last_response.ok?
 
     @wiki.clear_cache
@@ -90,12 +89,28 @@ context "Frontend" do
     assert_equal 'def', page_2.version.message
     assert_not_equal page_1.version.sha, page_2.version.sha
   end
+  
+  test "edit page fails when page is outdated (edit collision)" do
+    page = @wiki.page('A')
+    old_sha = page.sha
+    post "/gollum/edit/A", :content => 'abc', :page => 'A',
+         :format => page.format, :message => 'def', :etag => old_sha
+    assert last_response.ok?
+    
+    @wiki.clear_cache
+    page = @wiki.page('A')
+    new_sha = page.sha
+    assert_not_equal old_sha, new_sha
+    
+    post "/gollum/edit/A", :content => 'def', :page => 'A',
+         :format => page.format, :message => 'def', :etag => old_sha
+    assert_equal last_response.status, 412    
+  end
 
   test "edit page with empty message" do
     page_1 = @wiki.page('A')
     post "/gollum/edit/A", :content => 'abc', :page => 'A',
-         :format             => page_1.format
+         :format => page_1.format, :etag => page_1.sha
-    follow_redirect!
     assert last_response.ok?
 
     @wiki.clear_cache
@@ -108,8 +123,7 @@ context "Frontend" do
   test "edit page with slash" do
     page_1 = @wiki.page('A')
     post "/gollum/edit/A", :content => 'abc', :page => 'A', :path => '/////',
-         :format             => page_1.format, :message => 'def'
+         :format => page_1.format, :message => 'def', :etag => page_1.sha
-    follow_redirect!
     assert last_response.ok?
   end
 
@@ -121,9 +135,7 @@ context "Frontend" do
     side_1   = page_1.sidebar
 
     post "/gollum/edit/A", :header => 'header',
-         :footer            => 'footer', :page => "A", :sidebar => 'sidebar', :message => 'def'
+         :footer => 'footer', :page => "A", :sidebar => 'sidebar', :message => 'def', :etag => page_1.sha
-    follow_redirect!
-    assert_equal "/A.md", last_request.fullpath
     assert last_response.ok?
 
     @wiki.clear_cache
@@ -325,7 +337,7 @@ context "Frontend" do
     page = 'not-real-page'
     path = '/'
     post '/gollum/edit/', :content => 'edit_msg',
-         :page              => page, :path => path, :message => ''
+         :page => page, :path => path, :message => ''
     page_e = @wiki.paged(page, path)
     assert_equal nil, page_e
   end
@@ -335,7 +347,7 @@ context "Frontend" do
     path = 'a/b/' # path must end with /
 
     post '/gollum/create', :content => 'create_msg', :page => page,
-         :path               => path, :format => 'markdown', :message => ''
+         :path => path, :format => 'markdown', :message => ''
     page_c = @wiki.paged(page, path)
     assert_equal 'create_msg', page_c.raw_data
 
@@ -344,7 +356,7 @@ context "Frontend" do
 
     # post '/edit' fails. post '/edit/' works.
     post '/gollum/edit/', :content => 'edit_msg',
-         :page              => page, :path => path, :message => ''
+         :page => page, :path => path, :message => '', :etag => page_c.sha
     page_e = @wiki.paged(page, path)
     assert_equal 'edit_msg', page_e.raw_data
 
@@ -469,8 +481,7 @@ context "Frontend" do
     gollum_author = { :name => 'ghi', :email => 'jkl' }
     session       = { 'gollum.author' => gollum_author }
 
-    post "/gollum/edit/A", { :content => 'abc', :page => 'A', :format => page1.format, :message => 'def' }, { 'rack.session' => session }
+    post "/gollum/edit/A", { :content => 'abc', :page => 'A', :format => page1.format, :message => 'def', :etag => page1.sha }, { 'rack.session' => session }
-    follow_redirect!
     assert last_response.ok?
 
     @wiki.clear_cache
@@ -548,9 +559,11 @@ context "Frontend" do
          :content => 'りんご',
          :page    => 'Multibyte', :format => :markdown, :message => 'mesg'
 
+    page = @wiki.paged('Multibyte')
+    
     post "/gollum/edit/Multibyte",
          :content => 'りんご', :header => 'みかん', :footer => 'バナナ', :sidebar => 'スイカ',
-         :page    => 'Multibyte', :format => :markdown, :message => 'mesg'
+         :page    => 'Multibyte', :format => :markdown, :message => 'mesg', :etag => page.sha
 
     get "/gollum/edit/Multibyte"
 
@@ -695,13 +708,14 @@ context "Frontend with lotr" do
 
   test "edit pages within sub-directories" do
     post "/gollum/create", :content => 'big smelly creatures', :page => 'Orc',
-         :path               => 'Mordor', :format => 'markdown', :message => 'oooh, scary'
+         :path => 'Mordor', :format => 'markdown', :message => 'oooh, scary'
 
     assert_equal 'http://example.org/Mordor/Orc.md', last_response.headers['Location']
 
+    page = @wiki.paged('Orc', 'Mordor')
     post "/gollum/edit/Mordor/Orc", :content => 'not so big smelly creatures',
-         :page                        => 'Orc', :path => 'Mordor', :message => 'minor edit'
+         :page => 'Orc', :path => 'Mordor', :message => 'minor edit', :etag => page.sha
-    assert_equal 'http://example.org/Mordor/Orc.md', last_response.headers['Location']
+    assert last_response.ok?
 
     get "/Mordor/Orc"
     assert_match /not so big smelly creatures/, last_response.body

test/test_unicode.rb

@@ -57,8 +57,7 @@ context "Frontend Unicode support" do
     page = @wiki.page('PG')
     assert_equal '다른 text', utf8(page.raw_data)
 
-    post '/gollum/edit/PG', :page => 'PG', :content => '바뀐 text', :message => 'ghi'
+    post '/gollum/edit/PG', :page => 'PG', :content => '바뀐 text', :message => 'ghi', :etag => page.sha
-    follow_redirect!
     assert last_response.ok?
 
     @wiki = Gollum::Wiki.new(@path)
@@ -79,8 +78,7 @@ context "Frontend Unicode support" do
     assert_equal '다른 text', utf8(page.raw_data)
 
     post '/gollum/edit/' + CGI.escape('한글'), :page => 'k', :content => '바뀐 text',
-         :format                            => 'markdown', :message => 'ghi'
+         :format => 'markdown', :message => 'ghi', :etag => page.sha
-    follow_redirect!
     assert last_response.ok?
 
     @wiki = Gollum::Wiki.new(@path)