From c080ef3dd76a6c9fc7a75f1b035cec3061ac2f5d Mon Sep 17 00:00:00 2001 From: "COLA@Redmine.local" Date: Fri, 10 Feb 2017 21:29:01 +0100 Subject: [PATCH 1/2] A locked file only increase the revision once when updated/saved via the WebDav. #615 --- ...0170204214753_add_revision_to_dmsf_lock.rb | 5 +++ lib/redmine_dmsf/lockable.rb | 1 + lib/redmine_dmsf/webdav/dmsf_resource.rb | 41 +++++++++++++------ 3 files changed, 35 insertions(+), 12 deletions(-) create mode 100644 db/migrate/20170204214753_add_revision_to_dmsf_lock.rb diff --git a/db/migrate/20170204214753_add_revision_to_dmsf_lock.rb b/db/migrate/20170204214753_add_revision_to_dmsf_lock.rb new file mode 100644 index 00000000..45a1db9d --- /dev/null +++ b/db/migrate/20170204214753_add_revision_to_dmsf_lock.rb @@ -0,0 +1,5 @@ +class AddRevisionToDmsfLock < ActiveRecord::Migration + def change + add_column :dmsf_locks, :revision, :integer + end +end diff --git a/lib/redmine_dmsf/lockable.rb b/lib/redmine_dmsf/lockable.rb index 017a4af1..8b1d5de0 100644 --- a/lib/redmine_dmsf/lockable.rb +++ b/lib/redmine_dmsf/lockable.rb @@ -69,6 +69,7 @@ module RedmineDmsf l.lock_scope = scope l.user = User.current l.expires_at = expire + l.revision = self.last_revision.id unless self.is_a?(DmsfFolder) l.save! reload locks.reload diff --git a/lib/redmine_dmsf/webdav/dmsf_resource.rb b/lib/redmine_dmsf/webdav/dmsf_resource.rb index 9b8e74eb..cea063f1 100644 --- a/lib/redmine_dmsf/webdav/dmsf_resource.rb +++ b/lib/redmine_dmsf/webdav/dmsf_resource.rb @@ -293,7 +293,7 @@ module RedmineDmsf # Renaming a *.tmp file to an existing file in the same project, probably Office that is saving a file. Rails.logger.info "WebDAV MOVE: #{file.name} -> #{resource.basename} (exists), possible MSOffice rename from .tmp when saving" - if resource.file.last_revision.size == 0 + if resource.file.last_revision.size == 0 || reuse_version_for_locked_file(resource.file) # Last revision in the destination has zero size so reuse that revision new_revision = resource.file.last_revision else @@ -549,21 +549,25 @@ module RedmineDmsf return NoContent end - # Disable versioning for file name patterns given in the plugin settings. - pattern = Setting.plugin_redmine_dmsf['dmsf_webdav_disable_versioning'] - if !pattern.blank? && basename.match(pattern) - Rails.logger.info "Versioning disabled for #{basename}" - reuse_revision = true - else - reuse_revision = false - end + reuse_revision = false if exist? # We're over-writing something, so ultimately a new revision f = file + + # Disable versioning for file name patterns given in the plugin settings. + pattern = Setting.plugin_redmine_dmsf['dmsf_webdav_disable_versioning'] + if !pattern.blank? && basename.match(pattern) + Rails.logger.info "Versioning disabled for #{basename}" + reuse_revision = true + end + + if reuse_version_for_locked_file(file) + reuse_revision = true + end + last_revision = file.last_revision if last_revision.size == 0 || reuse_revision new_revision = last_revision - new_revision.minor_version -= 1 reuse_revision = true else new_revision = DmsfFileRevision.new @@ -585,7 +589,6 @@ module RedmineDmsf new_revision = DmsfFileRevision.new new_revision.minor_version = 0 new_revision.major_version = 0 - reuse_revision = false end new_revision.dmsf_file = f @@ -594,7 +597,7 @@ module RedmineDmsf new_revision.title = DmsfFileRevision.filename_to_title(basename) new_revision.description = nil new_revision.comment = nil - new_revision.increase_version(1) + new_revision.increase_version(1) unless reuse_revision new_revision.mime_type = Redmine::MimeType.of(new_revision.name) # Phusion passenger does not have a method "length" in its model @@ -735,6 +738,20 @@ module RedmineDmsf x end +private + def reuse_version_for_locked_file(file) + locks = file.lock + locks.each do |lock| + next if lock.expired? + # lock should be exclusive but just in case make sure we find this users lock + next if lock.user != User.current + if lock.revision != file.last_revision.id + return true + end + end + return false + end + end end end From cd14a01490fe49d718156b92e79fba3406e20471 Mon Sep 17 00:00:00 2001 From: "COLA@Redmine.local" Date: Fri, 10 Feb 2017 21:29:37 +0100 Subject: [PATCH 2/2] Updated tests --- .../webdav/dmsf_webdav_move_test.rb | 43 ++++++++++++++++--- .../webdav/dmsf_webdav_put_test.rb | 5 +++ 2 files changed, 43 insertions(+), 5 deletions(-) diff --git a/test/integration/webdav/dmsf_webdav_move_test.rb b/test/integration/webdav/dmsf_webdav_move_test.rb index cb0c78d1..2aa781ae 100644 --- a/test/integration/webdav/dmsf_webdav_move_test.rb +++ b/test/integration/webdav/dmsf_webdav_move_test.rb @@ -233,7 +233,7 @@ class DmsfWebdavMoveTest < RedmineDmsf::Test::IntegrationTest assert_response 201 # Created end - # Move twice + # Move twice, make sure that the MsOffice store sequence is not disrupting normal move new_name2 = "#{new_name}.m2" assert_difference 'file.dmsf_file_revisions.count', +1 do xml_http_request :move, "/dmsf/webdav/#{@project1.identifier}/#{new_name}", nil, @@ -242,7 +242,7 @@ class DmsfWebdavMoveTest < RedmineDmsf::Test::IntegrationTest end end - def test_move_msoffice_save_file + def test_move_msoffice_save_locked_file # When some versions of MsOffice saves a file it use the following sequence: # 1. Save changes to a new temporary document, XXX.tmp # 2. Rename (MOVE) document to YYY.tmp. History is lost here if original document is moved. @@ -251,13 +251,19 @@ class DmsfWebdavMoveTest < RedmineDmsf::Test::IntegrationTest # Verify that steps 2 and 3 works. original_file = DmsfFile.find_by_id 1 - temp_file_name = "ABCDEF.tmp" + + log_user 'jsmith', 'jsmith' # login as jsmith + assert !User.current.anonymous?, 'Current user is anonymous' + assert original_file.lock!, "File failed to be locked by #{User.current.name}" + # First save while file is locked, should create new revision + temp_file_name = "AAAAAAAA.tmp" + # Make sure that the temp-file does not exist. temp_file = DmsfFile.find_file_by_name @project1, nil, "#{temp_file_name}" assert !temp_file, "File '#{temp_file_name}' should not exist yet." - # Move the original file to ABCDEF.tmp. The original file should not change but a new file should be created. + # Move the original file to AAAAAAAA.tmp. The original file should not change but a new file should be created. assert_no_difference 'original_file.dmsf_file_revisions.count' do xml_http_request :move, "/dmsf/webdav/#{@project1.identifier}/#{original_file.name}", nil, @jsmith.merge!({:destination => "http://www.example.com/dmsf/webdav/#{@project1.identifier}/#{temp_file_name}"}) @@ -270,12 +276,39 @@ class DmsfWebdavMoveTest < RedmineDmsf::Test::IntegrationTest assert_equal temp_file.dmsf_file_revisions.count,1 assert_not_equal temp_file.id, original_file.id - # Move a temporary file (use ABCDEF.tmp) to the original file. + # Move a temporary file (use AAAAAAAA.tmp) to the original file. assert_difference 'original_file.dmsf_file_revisions.count', +1 do xml_http_request :move, "/dmsf/webdav/#{@project1.identifier}/#{temp_file_name}", nil, @jsmith.merge!({:destination => "http://www.example.com/dmsf/webdav/#{@project1.identifier}/#{original_file.name}"}) assert_response 201 # Created end + + # Second save while file is locked, should NOT create new revision + temp_file_name = "BBBBBBBB.tmp" + + # Make sure that the temp-file does not exist. + temp_file = DmsfFile.find_file_by_name @project1, nil, "#{temp_file_name}" + assert !temp_file, "File '#{temp_file_name}' should not exist yet." + + # Move the original file to BBBBBBBB.tmp. The original file should not change but a new file should be created. + assert_no_difference 'original_file.dmsf_file_revisions.count' do + xml_http_request :move, "/dmsf/webdav/#{@project1.identifier}/#{original_file.name}", nil, + @jsmith.merge!({:destination => "http://www.example.com/dmsf/webdav/#{@project1.identifier}/#{temp_file_name}"}) + assert_response 201 # Created + end + + # Verify that a new file acutally has been created + temp_file = DmsfFile.find_file_by_name @project1, nil, "#{temp_file_name}" + assert temp_file, "File '#{temp_file_name}' not found, move failed." + assert_equal temp_file.dmsf_file_revisions.count,1 + assert_not_equal temp_file.id, original_file.id + + # Move a temporary file (use BBBBBBBB.tmp) to the original file. + assert_no_difference 'original_file.dmsf_file_revisions.count' do + xml_http_request :move, "/dmsf/webdav/#{@project1.identifier}/#{temp_file_name}", nil, + @jsmith.merge!({:destination => "http://www.example.com/dmsf/webdav/#{@project1.identifier}/#{original_file.name}"}) + assert_response 201 # Created + end end end \ No newline at end of file diff --git a/test/integration/webdav/dmsf_webdav_put_test.rb b/test/integration/webdav/dmsf_webdav_put_test.rb index 20073b86..f3eb8206 100644 --- a/test/integration/webdav/dmsf_webdav_put_test.rb +++ b/test/integration/webdav/dmsf_webdav_put_test.rb @@ -204,6 +204,11 @@ class DmsfWebdavPutTest < RedmineDmsf::Test::IntegrationTest put "/dmsf/webdav/#{@project1.identifier}/test.txt", '1234', @jsmith.merge!({:content_type => :text}) assert_response :success # 201 - Created end + # Second PUT on a locked file should only update the revision that were created on the first PUT + assert_no_difference 'file.dmsf_file_revisions.count' do + put "/dmsf/webdav/#{@project1.identifier}/test.txt", '1234', @jsmith.merge!({:content_type => :text}) + assert_response :success # 201 - Created + end end end