WebDAV optimization

.gitignore

@@ -1 +1,3 @@
 /redmine_dmsf.iml
+/test/fixtures/files/**
+!/test/fixtures/files/2017/**

app/models/dmsf_file.rb

@@ -58,7 +58,7 @@ class DmsfFile < ActiveRecord::Base
   validate :validates_name_uniqueness
 
   def validates_name_uniqueness
-    existing_file = DmsfFile.visible.findn_file_by_name(self.project_id, self.dmsf_folder, self.name)
+    existing_file = DmsfFile.select(:id).findn_file_by_name(self.project_id, self.dmsf_folder, self.name)
     errors.add(:name, l('activerecord.errors.messages.taken')) unless (existing_file.nil? || existing_file.id == self.id)
   end
 
@@ -259,9 +259,8 @@ class DmsfFile < ActiveRecord::Base
   end
   
   def copy_to_filename(project, folder, filename)
-    source = "#{project.identifier}: #{self.dmsf_path_str}"
     file = DmsfFile.new
-    file.dmsf_folder = folder
+    file.dmsf_folder_id = folder.id if folder
     file.project_id = project.id
     file.name = filename
     file.notification = Setting.plugin_redmine_dmsf['dmsf_default_notifications'].present?
@@ -286,7 +285,7 @@ class DmsfFile < ActiveRecord::Base
       if File.exist? self.last_revision.disk_file
         FileUtils.cp self.last_revision.disk_file, new_revision.disk_file(false)
       end
-      new_revision.comment = l(:comment_copied_from, :source => source)
+      new_revision.comment = l(:comment_copied_from, :source => "#{project.identifier}: #{self.dmsf_path_str}")
       new_revision.custom_values = []
       self.last_revision.custom_values.each do |cv|
         v = CustomValue.new
@@ -294,9 +293,19 @@ class DmsfFile < ActiveRecord::Base
         v.value = cv.value
         new_revision.custom_values << v
       end
-      file.delete(true) unless new_revision.save
+      unless new_revision.save
+        Rails.logger.error new_revision.errors.full_messages.to_sentence
+        file.delete(true)
+        file = nil
+      else
+        file.set_last_revision new_revision
+      end
+    else
+      Rails.logger.error file.errors.full_messages.to_sentence
+      file.delete(true)
+      file = nil
     end
-    return file
+    file
   end
 
   # To fulfill searchable module expectations

lib/redmine_dmsf/webdav/dmsf_resource.rb

@@ -77,7 +77,6 @@ module RedmineDmsf
       def folder
         unless @folder
           return nil unless project
-          #f = parent.folder
           @folder = DmsfFolder.visible.where(:project_id => project.id, :title => basename,
             :dmsf_folder_id => parent.folder ? parent.folder.id : nil).first
         end
@@ -236,6 +235,11 @@ module RedmineDmsf
         raise Forbidden unless (!parent.exist? || !parent.folder || DmsfFolder.permissions?(parent.folder, false))
 
         if collection?
+
+          if dest.exist?
+            return overwrite ? NotImplemented : PreconditionFailed
+          end
+
           # At the moment we don't support cross project destinations
           return MethodNotImplemented unless (project.id == resource.project.id)
           raise Forbidden unless User.current.admin? || User.current.allowed_to?(:folder_manipulation, project)
@@ -243,7 +247,7 @@ module RedmineDmsf
 
           # Current object is a folder, so now we need to figure out information about Destination
           if dest.exist?
-            MethodNotAllowed
+            return overwrite ? NotImplemented : PreconditionFailed
           else
             if(parent.projectless_path == '/') #Project root
               folder.dmsf_folder_id = nil
@@ -260,6 +264,7 @@ module RedmineDmsf
               User.current.allowed_to?(:folder_manipulation, resource.project)
 
           if dest.exist?
+            return PreconditionFailed unless overwrite
             if (project == resource.project) && file.name.match(/.\.tmp$/i)
               # 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"
@@ -290,7 +295,7 @@ module RedmineDmsf
             else
               # Files cannot be merged at this point, until a decision is made on how to merge them
               # ideally, we would merge revision history for both, ensuring the origin file wins with latest revision.
-              MethodNotAllowed
+              NotImplemented
             end
           else
             if(parent.projectless_path == '/') #Project root
@@ -322,7 +327,7 @@ module RedmineDmsf
               file.name = resource.basename
 
               # Save Changes
-              (file.last_revision.save! && file.save!) ? Created : PreconditionFailed
+              (file.last_revision.save && file.save) ? Created : PreconditionFailed
             end
           end
         end
@@ -347,10 +352,13 @@ module RedmineDmsf
         parent = resource.parent
         raise Forbidden unless (!parent.exist? || !parent.folder || DmsfFolder.permissions?(parent.folder, false))
 
-        if collection?
+        if dest.exist?
-          # Current object is a folder, so now we need to figure out information about Destination
+          return overwrite ? NotImplemented : PreconditionFailed
-          return MethodNotAllowed if(dest.exist?)
+        end
 
+        return Conflict unless dest.parent.exist?
+
+        if collection?
           # Permission check if they can manipulate folders and view folders
           # Can they:
           #  Manipulate folders on destination project :folder_manipulation
@@ -370,37 +378,32 @@ module RedmineDmsf
           return PreconditionFailed if new_folder.nil? || new_folder.id.nil?
           Created
         else
-          if dest.exist?
+          # Permission check if they can manipulate folders and view folders
-            MethodNotAllowed
+          # Can they:
-            # Files cannot be merged at this point, until a decision is made on how to merge them
+          #  Manipulate files on destination project   :file_manipulation
-            # ideally, we would merge revision history for both, ensuring the origin file wins with latest revision.
+          #  View files on destination project         :view_dmsf_files
+          #  View files on the source project          :view_dmsf_files
+          raise Forbidden unless User.current.admin? ||
+             (User.current.allowed_to?(:file_manipulation, resource.project) &&
+              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
-            # Permission check if they can manipulate folders and view folders
+            return PreconditionFailed unless parent.exist? && parent.folder
-            # Can they:
+            f = parent.folder
-            #  Manipulate files on destination project   :file_manipulation
-            #  View files on destination project         :view_dmsf_files
-            #  View files on the source project          :view_dmsf_files
-            raise Forbidden unless User.current.admin? ||
-               (User.current.allowed_to?(:file_manipulation, resource.project) &&
-                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
-            return InternalServerError unless file.copy_to(resource.project, f)
-
-            # Update Revision and names of file [We can link to old physical resource, as it's not changed]
-            file.last_revision.name = resource.basename if file.last_revision
-            file.name = resource.basename
-
-            # Save Changes
-            (file.last_revision.save! && file.save!) ? Created : PreconditionFailed
           end
+          return PreconditionFailed unless exist? && file
+          new_file = file.copy_to(resource.project, f)
+          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]
+          new_file.last_revision.name = resource.basename
+          new_file.name = resource.basename
+
+          # Save Changes
+          (new_file.last_revision.save && new_file.save) ? Created : PreconditionFailed
         end
       end
 

test/integration/webdav/dmsf_webdav_move_test.rb

@@ -200,7 +200,7 @@ class DmsfWebdavMoveTest < RedmineDmsf::Test::IntegrationTest
         assert_no_difference '@file1.dmsf_file_revisions.count' do
           xml_http_request :move, "/dmsf/webdav/#{@project1.identifier}/#{@file1.name}", nil,
             @jsmith.merge!({:destination => "http://www.example.com/dmsf/webdav/#{@project1.identifier}/#{new_name}"})
-          assert_response 405 # MethodNotAllowed
+          assert_response 501 # NotImplemented
         end
       end
     end