diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ae53eb9..dc4335df 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,8 +1,15 @@ Changelog for Redmine DMSF ========================== -1.4.3: *Not yet released* +1.4.4: *Not yet released* ----------------------- +* New: Locking model updated to support shared and exclusive write locks. +* New: Folders are now write lockable (shared and exclusively) +* New: Locks can now have a time-limit +* New: Inereted lock support (locked folders child entries are now flagged as locked) + +1.4.3: *2012-06-26* +----------------- * New: Hook into project copy functionality to permit (although not attractively) functionality for DMSF to be duplicated accross projects * Update: Project patch defines linkage between DMSF files and DMSF folders. diff --git a/README.md b/README.md index c9c0fc1b..c7c4add7 100644 --- a/README.md +++ b/README.md @@ -35,13 +35,14 @@ Features Dependencies ------------ -As of version 1.4.3 of this plugin: +As of version 1.4.4 of this plugin: * Bundler 1.1 or greater (Gem) * Redmine 2.0.x * Rails 3.2.x (Inline with Redmine installation requirement) * zip (Gem) * DAV4Rack (Github Gem) + * simple_enum (Gem) ### Fulltext search (optional) diff --git a/app/models/dmsf_lock.rb b/app/models/dmsf_lock.rb index 5106a6b0..79f623e1 100644 --- a/app/models/dmsf_lock.rb +++ b/app/models/dmsf_lock.rb @@ -25,7 +25,7 @@ class DmsfLock < ActiveRecord::Base #At the moment apparently we're only supporting a write lock? - as_enum :lock_type, [:type_write] + as_enum :lock_type, [:type_write, :type_other] as_enum :lock_scope, [:scope_exclusive, :scope_shared] # We really loosly bind the value in the belongs_to above @@ -41,17 +41,13 @@ class DmsfLock < ActiveRecord::Base entity_type == 1 ? super : nil; end - def self.file_lock_state(file, locked) - lock = DmsfFileLock.new - lock.file = file - lock.user = User.current - lock.locked = locked - lock.save! - end - def expired? return false if expires_at.nil? return expires_at <= Time.now end + + def self.delete_expired + self.delete_all ['expires_at IS NOT NULL && expires_at < ?', Time.now] + end end diff --git a/lib/dmsf_lock_error.rb b/lib/dmsf_lock_error.rb new file mode 100644 index 00000000..b74493c5 --- /dev/null +++ b/lib/dmsf_lock_error.rb @@ -0,0 +1,24 @@ +# Redmine plugin for Document Management System "Features" +# +# Copyright (C) 2012 Daniel Munn +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# 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 General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + +class DmsfLockError < StandardError + +end + + + diff --git a/lib/redmine_dmsf/lockable.rb b/lib/redmine_dmsf/lockable.rb index 903959b5..9e94db5f 100644 --- a/lib/redmine_dmsf/lockable.rb +++ b/lib/redmine_dmsf/lockable.rb @@ -16,14 +16,32 @@ module RedmineDmsf } end if tree - ret = ret | folder.lock unless folder.nil? + ret = ret | (folder.locks || folder.lock) unless folder.nil? end return ret end - def lock! scope = :exclusive, type = :write - l = DmsfLock.lock_state(self, scope, type) - self.reload + def lock! scope = :scope_exclusive, type = :type_write, expire = nil + # Raise a lock error if entity is locked, but its not at resource level + existing = locks(false) + raise DmsfLockError.new("Unable to complete lock - resource (or parent) is locked") if self.locked? && existing.empty? + unless existing.empty? + if existing[0].scope == :scope_exclusive + raise DmsfLockError.new("Unable to complete lock - resource (or parent) is locked") + else + raise DmsfLockError.new("unable to exclusively lock a shared-locked resource") if scope == :scope_exclusive + end + end + l = DmsfLock.new + l.entity_id = self.id + l.entity_type = self.is_a?(DmsfFile) ? 0 : 1 + l.lock_type = type + l.lock_scope = scope + l.user = User.current + l.expires_at = expire + l.save! + reload + locks.reload return l end @@ -34,7 +52,7 @@ module RedmineDmsf b_shared = nil heirarchy = self.dmsf_path heirarchy.each {|folder| - locks = folder.lock(false) + locks = folder.locks || folder.lock(false) next if locks.empty? locks.each {|lock| next if lock.expired? #Incase we're inbetween updates @@ -50,32 +68,42 @@ module RedmineDmsf false end -# #Any better suggestions on this? - This is quite cumbersome -# def locked_for_user_old? -# return false unless locked? -# b_shared = nil -# -# unless locks.empty? -# locks.each {|lock| -# continue if lock.expired? #Incase we're inbetween updates -# if (lock.lock_scope == :scope_exclusive && b_shared.nil?) -# return true if lock.user.id != User.current.id -# else -# b_shared = true if b_shared.nil? -# b_shared = false if lock.user.id == User.current.id -# end -# } -# return true if b_shared -# end -# return folder.locked_for_user? unless folder.nil? -# false -# end - def unlock! - l = DmsfLock.lock_state(self, false) - self.reload - return l - end + raise DmsfLockError.new("Unable to complete unlock - requested resource is not reported locked") unless self.locked? + existing = self.lock(true) + if existing.empty? || (!self.folder.nil? && self.folder.locked?) #If its empty its a folder thats locked (not root) + raise DmsfLockError.new("Unlock failed - resource parent is locked") + else + # If entity is locked to you, you arent the lock originator (or named in a shared lock) so deny action + # Unless of course you have the rights to force an unlock + raise DmsfLockError.new("Unlock failed - resource is locked by another user") if ( + self.locked_for_user?(false) && + !User.current.allowed_to?(:force_file_unlock, self.project)) + #Now we need to determine lock type and do the needful + if (existing.count == 1 && existing[0].lock_scope == :exclusive) + existing[0].destroy + else + b_destroyed = false + existing.each {|lock| + if (lock.user.id == User.current.id) + lock.destroy + b_destroyed = true + break + end + } + # At first it was going to be allowed for someone with force_file_unlock to delete all shared by default + # Instead, they by default remove themselves from sahred lock, and everyone from shared lock if they're not + # on said lock + if (!b_destroyed && User.current.allowed_to?(:force_file_unlock, self.project)) + locks.delete_all + end + end + end + + reload + locks.reload + end + true end end diff --git a/test/integration/dmsf_webdav_delete_test.rb b/test/integration/dmsf_webdav_delete_test.rb index 5744c908..6c732f61 100644 --- a/test/integration/dmsf_webdav_delete_test.rb +++ b/test/integration/dmsf_webdav_delete_test.rb @@ -6,6 +6,7 @@ class DmsfWebdavIntegrationTest < RedmineDmsf::Test::IntegrationTest def setup DmsfFile.storage_path = File.expand_path("../fixtures/files", __FILE__) + DmsfLock.delete_all @admin = credentials('admin') @jsmith = credentials('jsmith') super @@ -211,7 +212,7 @@ class DmsfWebdavIntegrationTest < RedmineDmsf::Test::IntegrationTest assert !User.current.anonymous?, "Current user is not anonymous" file = DmsfFile.find_file_by_name(project, nil, "test.txt") - assert file.lock, "File failed to be locked by #{User.current.name}" + assert file.lock!, "File failed to be locked by #{User.current.name}" delete "dmsf/webdav/#{project.identifier}/test.txt", nil, @jsmith assert_response 423 #Locked @@ -219,9 +220,10 @@ class DmsfWebdavIntegrationTest < RedmineDmsf::Test::IntegrationTest file = DmsfFile.find_file_by_name(project, nil, "test.txt") assert !file.nil?, 'File test.txt is expected to exist' - file.unlock - assert file.locked?, "File failed to unlock by #{User.current.name}" + User.current = User.find(1) #For some reason the above delete request changes User.current + file.unlock! + assert !file.locked?, "File failed to unlock by #{User.current.name}" project.disable_module! :dmsf role.add_permission! :view_dmsf_folders role.add_permission! :file_manipulation diff --git a/test/integration/dmsf_webdav_put_test.rb b/test/integration/dmsf_webdav_put_test.rb index 6389d918..50fed5da 100644 --- a/test/integration/dmsf_webdav_put_test.rb +++ b/test/integration/dmsf_webdav_put_test.rb @@ -5,6 +5,7 @@ class DmsfWebdavIntegrationTest < RedmineDmsf::Test::IntegrationTest fixtures :projects, :users, :members, :member_roles, :roles, :enabled_modules, :dmsf_folders, :dmsf_files, :dmsf_file_revisions def setup + DmsfLock.delete_all #Delete all locks that are in our test DB - probably not safe but ho hum timestamp = DateTime.now.strftime("%y%m%d%H%M%S") DmsfFile.storage_path = File.expand_path("./dmsf_test-#{timestamp}", DmsfHelper.temp_dir) Dir.mkdir(DmsfFile.storage_path) unless File.directory?(DmsfFile.storage_path) @@ -167,15 +168,17 @@ class DmsfWebdavIntegrationTest < RedmineDmsf::Test::IntegrationTest assert !User.current.anonymous?, "Current user is not anonymous" file = DmsfFile.find_file_by_name(project, nil, "test.txt") - assert file.lock, "File failed to be locked by #{User.current.name}" + assert file.lock!, "File failed to be locked by #{User.current.name}" assert_no_difference('file.revisions.count') do put "dmsf/webdav/#{project.identifier}/test.txt", "1234", @jsmith.merge!({:content_type => :text}) assert_response 423 #Locked end - file.unlock - assert file.locked?, "File failed to unlock by #{User.current.name}" + User.current = User.find(1) + file.unlock! + + assert !file.locked?, "File failed to unlock by #{User.current.name}" role.add_permission! :view_dmsf_folders role.add_permission! :file_manipulation @@ -195,15 +198,19 @@ class DmsfWebdavIntegrationTest < RedmineDmsf::Test::IntegrationTest assert !User.current.anonymous?, "Current user is not anonymous" file = DmsfFile.find_file_by_name(project, nil, "test.txt") - assert file.lock, "File failed to be locked by #{User.current.name}" + assert file.lock!, "File failed to be locked by #{User.current.name}" assert_no_difference('file.revisions.count') do put "dmsf/webdav/#{project.identifier}/test.txt", "1234", @admin.merge!({:content_type => :text}) assert_response 423 #Created end - - file.unlock - assert file.locked?, "File failed to unlock by #{User.current.name}" + User.current = User.find(2) + begin + file.unlock! + rescue + #nothing + end + assert !file.locked?, "File failed to unlock by #{User.current.name}" role.add_permission! :view_dmsf_folders role.add_permission! :file_manipulation @@ -222,15 +229,16 @@ class DmsfWebdavIntegrationTest < RedmineDmsf::Test::IntegrationTest assert !User.current.anonymous?, "Current user is not anonymous" file = DmsfFile.find_file_by_name(project, nil, "test.txt") - assert file.lock, "File failed to be locked by #{User.current.name}" + assert file.lock!, "File failed to be locked by #{User.current.name}" assert_difference('file.revisions.count') do put "dmsf/webdav/#{project.identifier}/test.txt", "1234", @jsmith.merge!({:content_type => :text}) assert_response 201 #Created end - file.unlock - assert file.locked?, "File failed to unlock by #{User.current.name}" + file.unlock! + + assert !file.locked?, "File failed to unlock by #{User.current.name}" role.add_permission! :view_dmsf_folders role.add_permission! :file_manipulation