From 6237e095812f4c8a23fad35af90f1d33401fae32 Mon Sep 17 00:00:00 2001 From: Daniel Munn Date: Thu, 28 Jun 2012 14:29:13 +0100 Subject: [PATCH] New: Support for lockdiscover and supported lock PROPFIND requests Update: file_revision visible scope now is joined with parent item and checked for both revision and file visibility New: Projects now store into their own folder within configured path Fix: db upgrade script does a one-time re-sort of existing physical data to ensure it can be read in new world Fix: webdav controller (from Gem) did not flag output as utf-8 with correct header, overloaded in dmsf controller instance to fix Update: Testsuites updated to support a few more conditions for lock processing --- CHANGELOG.md | 9 ++- app/models/dmsf_file.rb | 2 +- app/models/dmsf_file_revision.rb | 11 ++- db/migrate/07_dmsf_1_4_4.rb | 42 ++++++++++- lib/redmine_dmsf/webdav/controller.rb | 19 +++++ lib/redmine_dmsf/webdav/dmsf_resource.rb | 90 +++++++++++++++++++++++ lib/redmine_dmsf/webdav/resource_proxy.rb | 9 +++ lib/tasks/dmsf_convert_documents.rake | 2 +- test/unit/dmsf_lock_test.rb | 66 ++++++++++++++++- 9 files changed, 240 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dd61c9c9..2b2af455 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,11 +3,14 @@ Changelog for Redmine DMSF 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: Locking model updated to support shared and exclusive write locks. [At present UI and Webdav only support exclusive locking however] +* New: Folders are now write lockable (shared and exclusively) [UI upgraded to support folder locking, however only exclusively] +* New: Locks can now have a time-limit [Not yet supported from UI] * New: Inereted lock support (locked folders child entries are now flagged as locked) * Fix: Some testcases erroniously passed when files are locked, when they should be unlocked +* 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 1.4.3: *2012-06-26* ----------------- diff --git a/app/models/dmsf_file.rb b/app/models/dmsf_file.rb index b37e6d82..55d8e037 100644 --- a/app/models/dmsf_file.rb +++ b/app/models/dmsf_file.rb @@ -48,7 +48,7 @@ class DmsfFile < ActiveRecord::Base validate :validates_name_uniqueness def self.visible_condition(user, options = {}) - "deleted=0" + "#{self.table_name}.deleted = 0" end def validates_name_uniqueness diff --git a/app/models/dmsf_file_revision.rb b/app/models/dmsf_file_revision.rb index 9e8c56c0..627fa30c 100644 --- a/app/models/dmsf_file_revision.rb +++ b/app/models/dmsf_file_revision.rb @@ -26,7 +26,8 @@ class DmsfFileRevision < ActiveRecord::Base belongs_to :project has_many :access, :class_name => "DmsfFileRevisionAccess", :foreign_key => "dmsf_file_revision_id", :dependent => :destroy - scope :visible, lambda {|*args| {:conditions => DmsfFile.visible_condition(args.shift || User.current, *args) }} + #Returns a list of revisions that are not deleted here, or deleted at parent level either + scope :visible, lambda {|*args| joins(:file).where(DmsfFile.visible_condition(args.shift || User.current, *args)).where("#{self.table_name}.deleted = 0") } acts_as_customizable @@ -113,7 +114,13 @@ class DmsfFileRevision < ActiveRecord::Base end def disk_file - "#{DmsfFile.storage_path}/#{self.disk_filename}" + storage_base = "#{DmsfFile.storage_path}" #perhaps .dup? + unless project.nil? + project_base = project.identifier.gsub(/[^\w\.\-]/,'_') + storage_base << "/p_#{project_base}" + end + Dir.mkdir(storage_base) unless File.exists?(storage_base) + "#{storage_base}/#{self.disk_filename}" end def detect_content_type diff --git a/db/migrate/07_dmsf_1_4_4.rb b/db/migrate/07_dmsf_1_4_4.rb index 362ebf6c..ea6dc393 100644 --- a/db/migrate/07_dmsf_1_4_4.rb +++ b/db/migrate/07_dmsf_1_4_4.rb @@ -16,6 +16,8 @@ # along with this program; if not, write to the Free Software # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +require 'fileutils' + class Dmsf144 < ActiveRecord::Migration @@ -58,8 +60,26 @@ class Dmsf144 < ActiveRecord::Migration #Data cleanup rename_column :dmsf_file_locks, :dmsf_file_id, :entity_id remove_column :dmsf_file_locks, :locked - rename_table :dmsf_file_locks, :dmsf_locks + + #Not sure if this is the right place to do this, as its file manipulation, not database (stricly) + begin + DmsfFileRevision.visible.each {|rev| + next if rev.project.nil? + existing = "#{DmsfFile.storage_path}/#{rev.disk_filename}" + new_path = rev.disk_file + if File.exist?(existing) + if File.exist?(new_path) + rev.disk_filename = rev.new_storage_filename + new_path = rev.disk_file + rev.save! + end + FileUtils.mv(existing, new_path) + end + } + rescue + #Nothing here, we just dont want a migration to break + end end def self.down @@ -81,6 +101,26 @@ 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 + + #Not sure if this is the right place to do this, as its file manipulation, not database (stricly) + begin + DmsfFileRevision.visible.each {|rev| + next if rev.project.nil? + project = rev.project.identifier.gsub(/[^\w\.\-]/,'_') + existing = "#{DmsfFile.storage_path}/p_#{project}/#{rev.disk_filename}" + new_path = "#{DmsfFile.storage_path}/#{rev.disk_filename}" + if File.exist?(existing) + if File.exist?(new_path) + rev.disk_filename = rev.new_storage_filename + rev.save! + new_path = "#{DmsfFile.storage_path}/#{rev.disk_filename}" + end + FileUtils.mv(existing, new_path) + end + } + rescue + #Nothing here, we just dont want a migration to break + end end end diff --git a/lib/redmine_dmsf/webdav/controller.rb b/lib/redmine_dmsf/webdav/controller.rb index ce411fb6..cced9480 100644 --- a/lib/redmine_dmsf/webdav/controller.rb +++ b/lib/redmine_dmsf/webdav/controller.rb @@ -80,6 +80,25 @@ module RedmineDmsf end end + # root_type:: Root tag name + # Render XML and set Rack::Response#body= to final XML + # Another override (they don't seem to flag UTF-8 [at this point I'm considering forking the gem to fix, + # and making DMSF compliant on that .. *sigh* + def render_xml(root_type) + raise ArgumentError.new 'Expecting block' unless block_given? + doc = Nokogiri::XML::Builder.new(:encoding => 'utf-8') do |xml_base| + xml_base.send(root_type.to_s, {'xmlns:D' => 'DAV:'}.merge(resource.root_xml_attributes)) do + xml_base.parent.namespace = xml_base.parent.namespace_definitions.first + xml = xml_base['D'] + yield xml + end + end + response.body = doc.to_xml + response["Content-Type"] = 'application/xml; charset="utf-8"' + response["Content-Length"] = response.body.size.to_s + end + + private def ns(opt_head = '') _ns = opt_head diff --git a/lib/redmine_dmsf/webdav/dmsf_resource.rb b/lib/redmine_dmsf/webdav/dmsf_resource.rb index 606631a0..09d6b31b 100644 --- a/lib/redmine_dmsf/webdav/dmsf_resource.rb +++ b/lib/redmine_dmsf/webdav/dmsf_resource.rb @@ -527,6 +527,23 @@ module RedmineDmsf Created end + # get_property + # Overriding the base definition (extending it really) with functionality + # for lock information to be presented + def get_property(name) + case name + when 'supportedlock' then supported_lock + when 'lockdiscovery' then discover_lock + else super + end + end + + def property_names + %w(creationdate displayname getlastmodified getetag resourcetype getcontenttype getcontentlength supportedlock lockdiscovery) + end + + + private # Prepare file for download using Rack functionality: # Download (see RedmineDmsf::Webdav::Download) extends Rack::File to allow single-file @@ -544,6 +561,79 @@ module RedmineDmsf end Download.new(file.last_revision.disk_file) end + + # discover_lock + # As the name suggests, we're returning lock recovery information for requested resource + def discover_lock + x = Nokogiri::XML::DocumentFragment.parse "" + entity = file || folder + return nil unless entity.locked? + + if !entity.folder.nil? && entity.folder.locked? + locks = entity.lock.reverse[0].folder.locks(false)# longwinded way of getting base items locks + else + locks = entity.lock(false) + end + + Nokogiri::XML::Builder.with(x) do |doc| + doc.lockdiscovery { + locks.each {|lock| + next if lock.expired? + doc.activelock { + doc.locktype { + doc.write + } + doc.lockscope { + if lock.lock_scope == :scope_exclusive + doc.exclusive + else + doc.shared + end + } + if lock.folder.nil? + doc.depth "0" + else + doc.depth "infinity" + end + doc.owner lock.user.to_s + if lock.expires_at.nil? + doc.timeout = "Infinite" + else + doc.timeout "Second-#{(lock.expires_at.to_i - Time.now.to_i)}" + end + + lock_entity = lock.folder || lock.file + lock_path = "#{request.scheme}://#{request.host}:#{request.port}/dmsf/webdav/#{URI.escape(lock_entity.project.identifier)}/" + 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 } + } + } + } + end + + x + end + + # supported_lock + # As the name suggests, we're returning locks supported by our implementation + def supported_lock + x = Nokogiri::XML::DocumentFragment.parse "" + Nokogiri::XML::Builder.with(x) do |doc| + doc.supportedlock { + doc.lockentry { + doc.lockscope { doc.exclusive } + doc.locktype { doc.write } + } + doc.lockentry { + doc.lockscope { doc.shared } + doc.locktype { doc.write } + } + } + end + x + end + end end end diff --git a/lib/redmine_dmsf/webdav/resource_proxy.rb b/lib/redmine_dmsf/webdav/resource_proxy.rb index 0ce25c31..50ca06c5 100644 --- a/lib/redmine_dmsf/webdav/resource_proxy.rb +++ b/lib/redmine_dmsf/webdav/resource_proxy.rb @@ -145,6 +145,15 @@ module RedmineDmsf def resource @resource_c end + + def get_property(*args) + @resource_c.get_property(*args) + end + + def property_names + @resource_c.property_names + end + end end end diff --git a/lib/tasks/dmsf_convert_documents.rake b/lib/tasks/dmsf_convert_documents.rake index 4a430eb3..e9191e3a 100644 --- a/lib/tasks/dmsf_convert_documents.rake +++ b/lib/tasks/dmsf_convert_documents.rake @@ -120,7 +120,7 @@ class DmsfConvertDocuments if dry puts "Dry check file: " + file.name if file.invalid? - file.errors.each {|e.msg| puts "#{e}: #{msg}"} + file.errors.each {|e,msg| puts "#{e}: #{msg}"} end else file.save! diff --git a/test/unit/dmsf_lock_test.rb b/test/unit/dmsf_lock_test.rb index 90250e76..9563c668 100644 --- a/test/unit/dmsf_lock_test.rb +++ b/test/unit/dmsf_lock_test.rb @@ -1,4 +1,4 @@ -require File.dirname(__FILE__) + '/../test_helper' +require File.expand_path('../../test_helper.rb', __FILE__) class DmsfFileTest < RedmineDmsf::Test::UnitTest attr_reader :lock @@ -61,6 +61,68 @@ class DmsfFileTest < RedmineDmsf::Test::UnitTest assert lock.file.is_a?(DmsfFolder) end end - + + test "locked folder reports un-locked child file as locked" do + #folder id 2 is locked by fixture + #files 4 and 5 are file resources within locked folder (declared by fixture) + folder = dmsf_folders(:dmsf_folders_002) + file = dmsf_files(:dmsf_files_004) + + assert folder.locked?, "Folder (2) should be locked by fixture" + assert_equal 1, folder.lock.count #Check the folder lists 1 lock + + assert file.locked?, "File (4) sits within Folder(2) and should be locked" + assert_equal 1, file.lock.count #Check the file lists 1 lock + + assert_equal 0, file.lock(false).count #Check the file does not list any entries for itself + end + + test "locked folder cannot be unlocked by someone without rights (or anon)" do + folder = dmsf_folders(:dmsf_folders_002) + assert_no_difference ('folder.lock.count') do + assert_raise DmsfLockError do + folder.unlock! + end + end + + User.current = users(:users_002) + assert_no_difference ('folder.lock.count') do + assert_raise DmsfLockError do + folder.unlock! + end + end + end + + test "locked folder can be unlocked by permission :force_file_unlock" do + User.current = users(:users_001) + folder = dmsf_folders(:dmsf_folders_002) + + assert_difference('folder.lock.count', -1) do + assert_nothing_raised do + folder.unlock! + end + end + + User.current = users(:users_002) + + assert_difference('folder.lock.count') do + assert_nothing_raised do + folder.lock! + end + end + + User.current = users(:users_001) + assert_difference('folder.lock.count', -1) do + assert_nothing_raised do + folder.unlock! + end + end + + + #We need to re-establish locks for other test + folder.lock! + User.current = nil + + end end