From b8716a5cc2d96692267cd9065a911d8dc8daf810 Mon Sep 17 00:00:00 2001 From: Daniel Munn Date: Fri, 29 Jun 2012 08:26:43 +0100 Subject: [PATCH] Multiple fixes: upload not rendered for DMSF root; dmsf_lock is now storing uuid for lock; db upgrade / downgrade script updated for support --- CHANGELOG.md | 1 + app/models/dmsf_lock.rb | 24 ++++++++++++++-- app/views/dmsf/show.html.erb | 3 +- db/migrate/07_dmsf_1_4_4.rb | 13 +++++++++ lib/redmine_dmsf/lockable.rb | 19 +++++-------- lib/redmine_dmsf/webdav/dmsf_resource.rb | 35 ++++++++++++------------ 6 files changed, 63 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2b2af455..09978ee2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ Changelog for Redmine DMSF * Update: Webdav locks files for 1 hour at a time (requested time is ignored) * New: Files are now stored in project relevent folder * New: Implementation of lockdiscovery and supportedlock property requests +* New: Locks store a timestamp based UUID string enabling better interaction with webservices 1.4.3: *2012-06-26* ----------------- diff --git a/app/models/dmsf_lock.rb b/app/models/dmsf_lock.rb index 79f623e1..470727b4 100644 --- a/app/models/dmsf_lock.rb +++ b/app/models/dmsf_lock.rb @@ -17,8 +17,8 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. class DmsfLock < ActiveRecord::Base - unloadable - +# unloadable + before_create :generate_uuid belongs_to :file, :class_name => "DmsfFile", :foreign_key => "entity_id" belongs_to :folder, :class_name => "DmsfFolder", :foreign_key => "entity_id" belongs_to :user @@ -46,8 +46,28 @@ class DmsfLock < ActiveRecord::Base return expires_at <= Time.now end + def generate_uuid + self.uuid = UUIDTools::UUID.timestamp_create().to_s + end + def self.delete_expired self.delete_all ['expires_at IS NOT NULL && expires_at < ?', Time.now] end + + #Lets allow our UUID to be searchable + def self.find(*args) + if args.first && args.first.is_a?(String) && !args.first.match(/^\d*$/) + lock = find_by_uuid(*args) + raise ActiveRecord::RecordNotFound, "Couldn't find lock with uuid=#{args.first}" if lock.nil? + lock + else + super + end + end + + def self.find_by_param(*args) + self.find(*args) + end + end diff --git a/app/views/dmsf/show.html.erb b/app/views/dmsf/show.html.erb index 68651a4d..d24ceb00 100644 --- a/app/views/dmsf/show.html.erb +++ b/app/views/dmsf/show.html.erb @@ -315,5 +315,6 @@ sUrl = "jquery.dataTables/#{I18n.locale.to_s.downcase}.json" if I18n.locale && ! <% end %> -<%= render(:partial => "multi_upload") if (User.current.allowed_to?(:file_manipulation, @project) && (!@folder.nil? && !@folder.locked_for_user?)) %> +<%= render(:partial => "multi_upload") if (User.current.allowed_to?(:file_manipulation, @project) && + ( @folder.nil? || (!@folder.nil? &&!@folder.locked_for_user?) ) ) %>
diff --git a/db/migrate/07_dmsf_1_4_4.rb b/db/migrate/07_dmsf_1_4_4.rb index ea6dc393..eea1d139 100644 --- a/db/migrate/07_dmsf_1_4_4.rb +++ b/db/migrate/07_dmsf_1_4_4.rb @@ -17,6 +17,7 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. require 'fileutils' +require 'uuidtools' class Dmsf144 < ActiveRecord::Migration @@ -34,6 +35,7 @@ class Dmsf144 < ActiveRecord::Migration #Add our lock relevent columns (ENUM) - null (till we upgrade data) add_column :dmsf_file_locks, :lock_type_cd, :integer, :null => true add_column :dmsf_file_locks, :lock_scope_cd, :integer, :null => true + add_column :dmsf_file_locks, :uuid, :string, :null => true, :limit => 36 #Add our expires_at column add_column :dmsf_file_locks, :expires_at, :datetime, :null => true @@ -45,10 +47,20 @@ class Dmsf144 < ActiveRecord::Migration end end + #Generate new lock Id's for whats being persisted + do_not_delete.each {|l| + #Find the lock + next unless lock = DmsfFileLock.find(l) + lock.uuid = UUIDTools::UUID.timestamp_create().to_s + lock.save! + } + say "Preserving #{do_not_delete.count} file lock(s) found in old schema" DmsfFileLock.delete_all(['id NOT IN (?)', do_not_delete]) + #We need to force our newly found + say "Applying default lock scope / type - Exclusive / Write" DmsfFileLock.update_all ['entity_type = ?, lock_type_cd = ?, lock_scope_cd = ?', 0, 0, 0] @@ -101,6 +113,7 @@ class Dmsf144 < ActiveRecord::Migration remove_column :dmsf_file_locks, :lock_type_cd remove_column :dmsf_file_locks, :lock_scope_cd remove_column :dmsf_file_locks, :expires_at + remove_column :dmsf_file_locks, :uuid #Not sure if this is the right place to do this, as its file manipulation, not database (stricly) begin diff --git a/lib/redmine_dmsf/lockable.rb b/lib/redmine_dmsf/lockable.rb index 8f272f47..5881d862 100644 --- a/lib/redmine_dmsf/lockable.rb +++ b/lib/redmine_dmsf/lockable.rb @@ -26,23 +26,18 @@ module RedmineDmsf 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].lock_scope == :scope_exclusive - # If it's an exclusive lock and you're re-requesting the actual desired behaviour is to not return a new lock, - # but the same lock (extended) + if existing[0].lock_scope == :scope_shared && scope == :scope_shared + # RFC states if an item is exclusively locked and another lock is attempted we reject + # if the item is shared locked however, we can always add another lock to it if self.folder.locked? raise DmsfLockError.new("Unable to complete lock - resource parent is locked") else - if existing[0].user.id == User.current.id then - l = existing[0] - l.expires_at = expire - l.save! - return l - else - raise DmsfLockError.new("Unable to complete lock - resource is locked") - end + existing.each {|l| + raise DmsfLockError.new("Unable to complete lock - resource is locked") if l.user.id == User.current.id + } end else - raise DmsfLockError.new("unable to exclusively lock a shared-locked resource") if scope == :scope_exclusive + raise DmsfLockError.new("unable to lock exclusively an already-locked resource") if scope == :scope_exclusive end end l = DmsfLock.new diff --git a/lib/redmine_dmsf/webdav/dmsf_resource.rb b/lib/redmine_dmsf/webdav/dmsf_resource.rb index 09d6b31b..50854009 100644 --- a/lib/redmine_dmsf/webdav/dmsf_resource.rb +++ b/lib/redmine_dmsf/webdav/dmsf_resource.rb @@ -398,7 +398,6 @@ module RedmineDmsf # Lock def lock(args) return Conflict unless (parent.projectless_path == "/" || parent_exists?) - token = UUIDTools::UUID.md5_create(UUIDTools::UUID_URL_NAMESPACE, projectless_path).to_s lock_check(args[:scope]) entity = file? ? file : folder begin @@ -416,17 +415,12 @@ module RedmineDmsf return Conflict if http_if.nil? http_if = http_if.slice(1, http_if.length - 2) - - return Conflict unless http_if == token - - entity.lock(false).each {|l| - if l.user.id == User.current.id - l.expires_at = Time.now + 1.hour - l.save! - @response['Lock-Token'] = token - return [1.hours.to_i, token] - end - } + l = DmsfLock.find(http_if) + return Conflict if l.nil? + l.expires_at = Time.now + 1.hour + l.save! + @response['Lock-Token'] = l.uuid + return [1.hours.to_i, l.uuid] #Unfortunately if we're here, then it's updating a lock we can't find @@ -436,9 +430,10 @@ module RedmineDmsf scope = "scope_#{(args[:scope] || "exclusive")}".to_sym type = "type_#{(args[:type] || "write")}".to_sym - entity.lock! scope, type, Time.now + 1.hours - @response['Lock-Token'] = token - [1.hours.to_i, token] + #l should be the instance of the lock we've just created + l = entity.lock!(scope, type, Time.now + 1.hours) + @response['Lock-Token'] = l.uuid + [1.hours.to_i, l.uuid] end rescue DmsfLockError raise DAV4Rack::LockFailure.new("Failed to lock: #{@path}") @@ -455,9 +450,12 @@ module RedmineDmsf BadRequest else begin - _token = UUIDTools::UUID.md5_create(UUIDTools::UUID_URL_NAMESPACE, projectless_path).to_s entity = file? ? file : folder - if (!entity.locked? || entity.locked_for_user? || token != _token) + l = DmsfLock.find(token) + l_entity = l.file || l.folder + # Additional case: if a user trys to unlock the file instead of the folder that's locked + # This should throw forbidden as only the lock at level initiated should be unlocked + if (!entity.locked? || entity.locked_for_user? || l_entity != entity) Forbidden else entity.unlock! @@ -607,6 +605,9 @@ module RedmineDmsf lock_path << lock_entity.dmsf_path.map {|x| URI.escape(x.respond_to?('name') ? x.name : x.title) }.join('/') lock_path << "/" if lock_entity.is_a?(DmsfFolder) && lock_path[-1,1] != '/' doc.lockroot { doc.href lock_path } + if (lock.user.id == User.current.id || User.current.allowed_to?(:force_file_unlock, self.project)) + doc.locktoken { doc.href lock.uuid } + end } } }