From 6b83425dac56549e7a13bada56aeed416d5529ac Mon Sep 17 00:00:00 2001 From: "karel.picman@lbcfree.net" Date: Mon, 12 Oct 2020 14:30:29 +0200 Subject: [PATCH] #1179 sub-projects reworked --- lib/redmine_dmsf/webdav/base_resource.rb | 144 +++++++++------- lib/redmine_dmsf/webdav/dmsf_resource.rb | 162 +++--------------- lib/redmine_dmsf/webdav/index_resource.rb | 9 - lib/redmine_dmsf/webdav/project_resource.rb | 35 ++-- lib/redmine_dmsf/webdav/resource_proxy.rb | 32 +++- .../webdav/dmsf_webdav_move_test.rb | 9 + .../webdav/dmsf_webdav_propfind_test.rb | 6 +- 7 files changed, 166 insertions(+), 231 deletions(-) diff --git a/lib/redmine_dmsf/webdav/base_resource.rb b/lib/redmine_dmsf/webdav/base_resource.rb index 7c59417f..85e93f08 100644 --- a/lib/redmine_dmsf/webdav/base_resource.rb +++ b/lib/redmine_dmsf/webdav/base_resource.rb @@ -32,6 +32,15 @@ module RedmineDmsf attr_reader :public_path + DIR_FILE = %{ + + %s + %s + %s + %s + + } + def initialize(path, request, response, options) raise NotFound if Setting.plugin_redmine_dmsf['dmsf_webdav'].blank? @project = nil @@ -40,8 +49,6 @@ module RedmineDmsf super path, request, response, options end - DIR_FILE = "%s%s%s%s" - def accessor=(klass) @__proxy = klass end @@ -136,71 +143,36 @@ module RedmineDmsf OK end + def project + get_resource_info + @project + end + + def subproject + get_resource_info + @subproject + end + + def folder + get_resource_info + @folder + end + + def file + get_resource_info + @file + end + protected def uri_encode(uri) - uri.gsub(/[\(\)&]/, '(' => '%28', ')' => '%29', '&' => '&') + uri.gsub /[\(\)&]/, '(' => '%28', ')' => '%29', '&' => '&' end def basename File.basename @path end - # Return instance of Project based on the path - def project - unless @project - i = 1 - while true - pinfo = @path.split('/').drop(i) - #break if (pinfo.length == 1) && @project - prj = nil - if pinfo.length > 0 - if Setting.plugin_redmine_dmsf['dmsf_webdav_use_project_names'] - if pinfo.first =~ / (\d+)$/ - prj = Project.visible.find_by(id: $1, parent_id: @project&.id) - if prj - # Check again whether it's really the project and not a folder with a number as a suffix - prj = nil unless pinfo.first.start_with?(DmsfFolder::get_valid_title(prj.name)) - end - end - else - prj = Project.visible.find_by(identifier: pinfo.first, parent_id: @project&.id) - end - end - break unless prj - i = i + 1 - @project = prj - end - end - @project - end - - # Make it easy to find the path without project in it. - def projectless_path - i = 1 - project = nil - while true - prj = nil - pinfo = @path.split('/').drop(i) - if pinfo.length > 0 - if Setting.plugin_redmine_dmsf['dmsf_webdav_use_project_names'] - if pinfo.first =~ / (\d+)$/ - prj = Project.visible.find_by(id: $1, parent_id: project&.id) - if prj - # Check again whether it's really the project and not a folder with a number as a suffix - prj = nil unless pinfo.first.start_with?(DmsfFolder::get_valid_title(prj.name)) - end - end - else - prj = Project.visible.find_by(identifier: pinfo.first, parent_id: project&.id) - project = prj - end - end - return '/' + @path.split('/').drop(i).join('/') unless prj - i = i + 1 - end - end - def path_prefix @public_path.gsub /#{Regexp.escape(path)}$/, '' end @@ -213,8 +185,64 @@ module RedmineDmsf end end + def self.get_project(name, parent_project) + prj = nil + if Setting.plugin_redmine_dmsf['dmsf_webdav_use_project_names'] + if name =~ /^\[?.+ (\d+)\]?$/ + prj = Project.visible.find_by(id: $1, parent_id: parent_project&.id) + if prj + # Check again whether it's really the project and not a folder with a number as a suffix + prj = nil unless name.include?(DmsfFolder::get_valid_title(prj.name)) + end + end + else + if name =~ /^\[?([^\]]+)\]?$/ + prj = Project.visible.find_by(identifier: $1, parent_id: parent_project&.id) + end + end + prj + end + private + def get_resource_info + return if @project # We have already got it + pinfo = @path.split('/').drop(1) + i = 1 + while pinfo.length > 0 + prj = BaseResource::get_project(pinfo.first, @project) + if prj + @project = prj + if pinfo.length == 1 + @subproject = @project + break # We're at the end + end + else + @subproject = nil + fld = get_folder(pinfo.first) + if fld + @folder = fld + else + @file = DmsfFile.find_file_by_name(@project, @folder, pinfo.first) + @folder = nil + break # We're at the end + end + end + i = i + 1 + pinfo = path.split('/').drop(i) + end + end + + def get_folder(name) + return nil unless @project + f = DmsfFolder.visible.find_by(project_id: @project.id, dmsf_folder_id: @folder&.id, title: name) + if f && (!DmsfFolder.permissions?(f, false)) + nil + else + f + end + end + # Go recursively through the project tree until a dmsf enabled project is found def dmsf_available?(p) return true if(p.visible? && p.module_enabled?(:dmsf)) diff --git a/lib/redmine_dmsf/webdav/dmsf_resource.rb b/lib/redmine_dmsf/webdav/dmsf_resource.rb index 0eba9a88..3209cee6 100644 --- a/lib/redmine_dmsf/webdav/dmsf_resource.rb +++ b/lib/redmine_dmsf/webdav/dmsf_resource.rb @@ -28,18 +28,6 @@ module RedmineDmsf class DmsfResource < BaseResource include Redmine::I18n - def initialize(path, request, response, options) - @folder = nil - @file = nil - @subproject = nil - super path, request, response, options - end - - # Here we make sure our folder and file methods are not aliased - it should shave a few cycles off of processing - def setup - @skip_alias |= [ :folder, :file, :subproject ] - end - # Gather collection of objects that denote current entities child entities # Used for listing directories etc, implemented basic caching because otherwise # Our already quite heavy usage of DB would just get silly every time we called @@ -58,25 +46,6 @@ module RedmineDmsf folder.dmsf_files.visible.pluck(:name).each do |name| @children.push child(name) end - elsif subproject - # Projects - load_projects subproject.children - if subproject.module_enabled?(:dmsf) - # Folders - if User.current.allowed_to?(:view_dmsf_folders, project) - subproject.dmsf_folders.visible.each do |f| - if DmsfFolder.permissions?(f, false) - @children.push child(f.title) - end - end - end - # Files - if User.current.allowed_to?(:view_dmsf_files, project) - subproject.dmsf_files.visible.pluck(:name).each do |name| - @children.push child(name) - end - end - end end end @children @@ -85,14 +54,8 @@ module RedmineDmsf # Does the object exist? # If it is either a subproject or a folder or a file, then it exists def exist? - case @request.request_method.downcase - when 'mkcol' - (project && project.module_enabled?('dmsf') && (folder || file) && - (User.current.admin? || User.current.allowed_to?(:view_dmsf_folders, project))) - else - subproject || (project && project.module_enabled?('dmsf') && (folder || file) && - (User.current.admin? || User.current.allowed_to?(:view_dmsf_folders, project))) - end + project && project.module_enabled?('dmsf') && (subproject || folder || file) && + (User.current.admin? || User.current.allowed_to?(:view_dmsf_folders, project)) end # Is this entity a folder? @@ -100,58 +63,13 @@ module RedmineDmsf folder || subproject end - # Check if current entity is a folder and return DmsfFolder object if found (nil if not) - def folder - unless @folder - @folder = DmsfFolder.visible.find_by(project_id: project&.id, title: basename, - dmsf_folder_id: parent&.folder&.id) - if @folder && (!DmsfFolder.permissions?(@folder, false)) - @folder = nil - end - end - @folder - end - - # Check if the current entity exists as a file (DmsfFile), and returns corresponding object if found (nil otherwise) - def file - unless @file - @file = DmsfFile.find_file_by_name(project, parent&.folder, basename) - end - @file - end - - def subproject - unless @subproject - if Setting.plugin_redmine_dmsf['dmsf_webdav_use_project_names'] - if basename =~ / (\d+)$/ - @subproject = Project.visible.find_by(id: $1, parent_id: parent_project&.id) - if @subproject - # Check again whether it's really the project and not a folder with a number as a suffix - @subproject = nil unless basename.start_with?(DmsfFolder::get_valid_title(@subproject.name)) - end - end - else - @subproject = Project.visible.find_by(parent_id: parent_project&.id, identifier: basename) - end - end - @subproject - end - - def parent_project - project&.parent - end - # Return the content type of file # will return inode/directory for any collections, and appropriate for File entities def content_type - if folder - 'inode/directory' - elsif file && file.last_revision + if file&.last_revision file.last_revision.detect_content_type - elsif subproject - 'inode/directory' else - NotFound + 'inode/directory' end end @@ -160,29 +78,29 @@ module RedmineDmsf folder.created_at elsif file file.created_at - elsif subproject - subproject.created_on else - NotFound + raise NotFound end end def last_modified if folder folder.updated_at - elsif file && file.last_revision + elsif file&.last_revision file.last_revision.updated_at - elsif subproject - subproject.updated_on else - NotFound + raise NotFound end end def etag - filesize = file ? file.size : 4096 - fileino = (file && file.last_revision && File.exist?(file.last_revision.disk_file)) ? File.stat(file.last_revision.disk_file).ino : 2 - sprintf('%x-%x-%x', fileino, filesize, last_modified.to_i) + ino = 2 + if file + if file.last_revision && File.exist?(file.last_revision.disk_file) + ino = File.stat(file.last_revision.disk_file).ino + end + end + sprintf '%x-%x-%x', ino, content_length, (last_modified ? last_modified.to_i : 0) end def content_length @@ -190,11 +108,7 @@ module RedmineDmsf end def special_type - if folder - l(:field_folder) - elsif subproject - l(:field_project) - end + l(:field_folder) if folder end # Process incoming GET request @@ -224,14 +138,9 @@ module RedmineDmsf raise Forbidden unless User.current.admin? || User.current.allowed_to?(:folder_manipulation, project) raise Forbidden unless (!parent.exist? || !parent.folder || DmsfFolder.permissions?(parent.folder, false)) return MethodNotAllowed if exist? # If we already exist, why waste the time trying to save? - parent_folder_id = nil - if parent.projectless_path != '/' - return Conflict unless parent.folder - parent_folder_id = parent.folder.id - end f = DmsfFolder.new f.title = basename - f.dmsf_folder_id = parent_folder_id + f.dmsf_folder_id = parent.folder&.id f.project = project f.user = User.current f.save ? Created : Conflict @@ -342,24 +251,18 @@ module RedmineDmsf NotImplemented end else - if parent.projectless_path == '/' # Project root - f = nil - else - return PreconditionFailed unless parent.exist? && parent.folder - f = parent.folder - end return PreconditionFailed unless exist? && file if (project == resource.project) && resource.basename.match(/.\.tmp$/i) Rails.logger.info "WebDAV MOVE: #{file.name} -> #{resource.basename}, possible MSOffice rename to .tmp when saving." # Renaming the file to X.tmp, might be Office that is saving a file. Keep the original file. - file.copy_to_filename resource.project, f, resource.basename + file.copy_to_filename resource.project, parent&.folder, resource.basename Created else if (project == resource.project) && (file.last_revision.size == 0) # Moving a zero sized file within the same project, just update the dmsf_folder - file.dmsf_folder = f + file.dmsf_folder = parent&.folder else - return InternalServerError unless file.move_to(resource.project, f) + return InternalServerError unless file.move_to(resource.project, parent&.folder) end # Update Revision and names of file [We can link to old physical resource, as it's not changed] if file.last_revision @@ -399,6 +302,8 @@ module RedmineDmsf return Conflict unless dest.parent.exist? + return PreconditionFailed unless parent.exist? && parent.folder + if collection? # Permission check if they can manipulate folders and view folders # Can they: @@ -413,7 +318,6 @@ module RedmineDmsf User.current.allowed_to?(:view_dmsf_folders, project)) raise Forbidden unless DmsfFolder.permissions?(folder, false) - return PreconditionFailed if (parent.projectless_path != '/' && !parent.folder) folder.title = resource.basename new_folder = folder.copy_to(resource.project, parent.folder) return PreconditionFailed if new_folder.nil? || new_folder.id.nil? @@ -429,14 +333,8 @@ module RedmineDmsf User.current.allowed_to?(:view_dmsf_files, resource.project) && User.current.allowed_to?(:view_dmsf_files, project)) - if parent.projectless_path == '/' # Project root - f = nil - else - return PreconditionFailed unless parent.exist? && parent.folder - f = parent.folder - end return PreconditionFailed unless exist? && file - new_file = file.copy_to(resource.project, f) + new_file = file.copy_to(resource.project, parent&.folder) return InternalServerError unless (new_file && new_file.last_revision) # Update Revision and names of file [We can link to old physical resource, as it's not changed] @@ -462,7 +360,7 @@ module RedmineDmsf # Lock def lock(args) - if parent.nil? || ((parent.projectless_path != '/') && (!parent.exist?)) + unless parent&.exist? e = DAV4Rack::LockFailure.new e.add_failure @path, Conflict raise e @@ -473,7 +371,7 @@ module RedmineDmsf raise e end lock_check args[:scope] - entity = file ? file : folder + entity = file || folder unless entity e = DAV4Rack::LockFailure.new e.add_failure @path, MethodNotAllowed @@ -538,12 +436,12 @@ module RedmineDmsf return BadRequest end begin - entity = file ? file : folder + entity = file || folder l = DmsfLock.find(token) return NoContent unless l # Additional case: if a user tries 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 - return NoContent unless entity.locked? + return NoContent unless entity&.locked? l_entity = l.file || l.folder if entity.locked_for_user? || (l_entity != entity) Forbidden @@ -682,16 +580,12 @@ module RedmineDmsf Created end - def project_id - project.id if project - end - # array of lock info hashes # required keys are :time, :token, :depth # other valid keys are :scope, :type, :root and :owner def lockdiscovery entity = file || folder - return [] unless entity.locked? + return [] unless entity&.locked? if entity.dmsf_folder && entity.dmsf_folder.locked? entity.lock.reverse[0].folder.locks(false) # longwinded way of getting base items locks else @@ -743,7 +637,7 @@ module RedmineDmsf # implementation of service for request, which allows for us to pipe a single file through # also best-utilising DAV4Rack's implementation. def download - raise NotFound unless file.last_revision + raise NotFound unless file&.last_revision disk_file = file.last_revision.disk_file raise NotFound unless disk_file && File.exist?(disk_file) raise Forbidden unless (!parent.exist? || !parent.folder || DmsfFolder.permissions?(parent.folder)) diff --git a/lib/redmine_dmsf/webdav/index_resource.rb b/lib/redmine_dmsf/webdav/index_resource.rb index 9b47daa4..92963372 100644 --- a/lib/redmine_dmsf/webdav/index_resource.rb +++ b/lib/redmine_dmsf/webdav/index_resource.rb @@ -23,10 +23,6 @@ module RedmineDmsf module Webdav class IndexResource < BaseResource - - def initialize(path, request, response, options) - super(path, request, response, options) - end def children unless @children @@ -72,11 +68,6 @@ module RedmineDmsf OK end - # Bugfix: Ensure that this level never indicates a parent - def parent - nil - end - end end end diff --git a/lib/redmine_dmsf/webdav/project_resource.rb b/lib/redmine_dmsf/webdav/project_resource.rb index a22b8338..50ebf0b9 100644 --- a/lib/redmine_dmsf/webdav/project_resource.rb +++ b/lib/redmine_dmsf/webdav/project_resource.rb @@ -25,10 +25,6 @@ module RedmineDmsf class ProjectResource < BaseResource include Redmine::I18n - def initialize(path, request, response, options) - super path, request, response, options - end - def children unless @children @children = [] @@ -75,11 +71,11 @@ module RedmineDmsf end def name - ProjectResource.create_project_name project + ProjectResource.create_project_name(project) end def long_name - project&.name + '[' + project&.name + ']' end def content_type @@ -102,28 +98,29 @@ module RedmineDmsf end def make_collection - # It's not allowed to create folders on project level + MethodNotAllowed + end + + def move(dest, overwrite) MethodNotAllowed end - def folder - nil + def delete + MethodNotAllowed end - def file - nil + def lock(args) + e = DAV4Rack::LockFailure.new + e.add_failure @path, MethodNotAllowed + raise e end - def project_id - project&.id - end - - def self.create_project_name(project) - if project + def self.create_project_name(prj) + if prj if Setting.plugin_redmine_dmsf['dmsf_webdav_use_project_names'] - "#{DmsfFolder::get_valid_title(project.name)} #{project.id}" + "#{DmsfFolder::get_valid_title(prj.name)} #{prj.id}" else - project.identifier + "[#{prj.identifier}]" end end end diff --git a/lib/redmine_dmsf/webdav/resource_proxy.rb b/lib/redmine_dmsf/webdav/resource_proxy.rb index e7da2522..e40c1f33 100644 --- a/lib/redmine_dmsf/webdav/resource_proxy.rb +++ b/lib/redmine_dmsf/webdav/resource_proxy.rb @@ -40,14 +40,8 @@ module RedmineDmsf raise NotFound end super path, request, response, options - pinfo = path.split('/').drop(1) - if pinfo.length == 0 # If this is the base_path, we're at root - @resource_c = IndexResource.new(path, request, response, options) - elsif (pinfo.length == 1) || options[:project] # The first level or we know that it's a project - @resource_c = ProjectResource.new(path, request, response, options) - else # We made it all the way to DMSF Data - @resource_c = DmsfResource.new(path, request, response, options) - end + rc = get_resource_class(path) + @resource_c = rc.new(path, request, response, options) @resource_c.accessor = self if @resource_c @read_only = Setting.plugin_redmine_dmsf['dmsf_webdav_strategy'] == 'WEBDAV_READ_ONLY' end @@ -176,6 +170,28 @@ module RedmineDmsf @resource_c.propstats response, stats end + private + + def get_resource_class(path) + pinfo = path.split('/').drop(1) + return IndexResource if pinfo.length == 0 + return ProjectResource if pinfo.length == 1 + i = 1 + project = nil + prj = nil + while pinfo.length > 0 + prj = BaseResource::get_project(pinfo.first, project) + if prj + project = prj + else + break + end + i = i + 1 + pinfo = path.split('/').drop(i) + end + prj ? ProjectResource : DmsfResource + end + end end diff --git a/test/integration/webdav/dmsf_webdav_move_test.rb b/test/integration/webdav/dmsf_webdav_move_test.rb index 8d7d0b83..b4972e91 100644 --- a/test/integration/webdav/dmsf_webdav_move_test.rb +++ b/test/integration/webdav/dmsf_webdav_move_test.rb @@ -313,6 +313,15 @@ class DmsfWebdavMoveTest < RedmineDmsf::Test::IntegrationTest assert_equal 'new_folder_name', @folder10.title end + def test_move_folder_in_subproject_to_the_same_name_as_subproject + process :move, "/dmsf/webdav/#{@project1.identifier}/#{@project3.identifier}/#{@folder10.title}", params: nil, + headers: @admin.merge!({ + destination: "http://www.example.com/dmsf/webdav/#{@project1.identifier}/#{@project3.identifier}/#{@project3.identifier}" }) + assert_response :created + @folder10.reload + assert_equal @project3.identifier, @folder10.title + end + def test_move_subproject process :move, "/dmsf/webdav/#{@project1.identifier}/#{@project3.identifier}", params: nil, headers: @admin.merge!({ diff --git a/test/integration/webdav/dmsf_webdav_propfind_test.rb b/test/integration/webdav/dmsf_webdav_propfind_test.rb index 59978239..40d9fde0 100644 --- a/test/integration/webdav/dmsf_webdav_propfind_test.rb +++ b/test/integration/webdav/dmsf_webdav_propfind_test.rb @@ -88,7 +88,7 @@ class DmsfWebdavPropfindTest < RedmineDmsf::Test::IntegrationTest process :propfind, "/dmsf/webdav/#{@project1.identifier}", params: nil, headers: @admin.merge!({ HTTP_DEPTH: '0' }) assert_response :multi_status assert response.body.include?("http://www.example.com:80/dmsf/webdav/#{@project1.identifier}/") - assert response.body.include?("#{@project1.identifier}") + assert response.body.include?("#{RedmineDmsf::Webdav::ProjectResource::create_project_name(@project1)}") end def test_propfind_depth0_on_project1_for_admin_with_project_names @@ -106,7 +106,7 @@ class DmsfWebdavPropfindTest < RedmineDmsf::Test::IntegrationTest assert_response :multi_status # Project assert response.body.include?("http://www.example.com:80/dmsf/webdav/#{@project1.identifier}/") - assert response.body.include?("#{@project1.identifier}") + assert response.body.include?("#{RedmineDmsf::Webdav::ProjectResource::create_project_name(@project1)}") # Folders assert response.body.include?("http://www.example.com:80/dmsf/webdav/#{@project1.identifier}/#{@folder1.title}/") assert response.body.include?("#{@folder1.title}") @@ -158,7 +158,7 @@ class DmsfWebdavPropfindTest < RedmineDmsf::Test::IntegrationTest headers: @admin.merge!({ HTTP_DEPTH: '1'}) assert_response :multi_status assert response.body.include?("http://www.example.com:80/dmsf/webdav/#{@project1.identifier}/#{@project3.identifier}/") - assert response.body.include?("#{@project3.identifier}") + assert response.body.include?("#{RedmineDmsf::Webdav::ProjectResource::create_project_name(@project3)}") end end \ No newline at end of file