Files remain locked after content editing

lib/redmine_dmsf/lockable.rb

@@ -90,23 +90,25 @@ module RedmineDmsf
     # By using the path upwards, surely this would be quicker?
     def locked_for_user?(args = nil)
       return false unless locked?
-      b_shared = nil
+      shared = nil
       self.dmsf_path.each do |entity|
         locks = entity.locks || entity.lock(false)
         next if locks.empty?
         locks.each do |lock|
           next if lock.expired? # In case we're in between updates
+          owner = args[:owner] if args
+          owner ||= User.current&.login if lock.owner
           if lock.lock_scope == :scope_exclusive
-            return true if (lock.user&.id != User.current.id) || ((lock.owner != (args ? args[:owner] : nil)))
+            return true if (lock.user&.id != User.current.id) || (lock.owner != owner)
           else
-            b_shared = true if b_shared.nil?
-            if b_shared && (lock.user&.id == User.current.id) && (lock.owner == (args ? args[:owner] : nil)) ||
+            shared = true if shared.nil?
+            if shared && (lock.user&.id == User.current.id) && (lock.owner == owner) ||
               (args && (args[:scope] == 'shared'))
-              b_shared = false
+              shared = false
             end
           end
         end
-        return true if b_shared
+        return true if shared
       end
       false
     end
@@ -114,6 +116,7 @@ module RedmineDmsf
     def unlock!(force_file_unlock_allowed = false, owner = nil)
       raise DmsfLockError.new(l(:warning_file_not_locked)) unless self.locked?
       existing = self.lock(true)
+      destroyed = false
       # If its empty its a folder that's locked (not root)
       if existing.empty? || (!self.dmsf_folder.nil? && self.dmsf_folder.locked?)
         raise DmsfLockError.new(l(:error_unlock_parent_locked))
@@ -126,18 +129,18 @@ module RedmineDmsf
         if (existing.count == 1) && (existing[0].lock_scope == :exclusive)
           existing[0].destroy
         else
-          b_destroyed = false
           existing.each do |lock|
+            owner = User.current&.login if lock.owner && owner.nil?
             if ((lock.user&.id == User.current.id) && (lock.owner == owner)) || User.current.admin?
               lock.destroy
-              b_destroyed = true
+              destroyed = true
               break
             end
           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 shared 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) || force_file_unlock_allowed)
+          if !destroyed && (User.current.allowed_to?(:force_file_unlock, self.project) || force_file_unlock_allowed)
             locks.delete_all
           end
         end
@@ -145,6 +148,6 @@ module RedmineDmsf
       reload
       locks.reload
     end
-    true
+
   end
 end

lib/redmine_dmsf/webdav/dmsf_resource.rb

@@ -479,6 +479,7 @@ module RedmineDmsf
           return super(token)
         end
         if token.nil? || token.empty? || (token == '<(null)>') || User.current.anonymous?
+          Rails.logger.info ">>> bad token 2: #{token}"
           BadRequest
         else
           if token =~ /([a-f0-9]{8}-[a-f0-9]{4}-4[a-f0-9]{3}-[89aAbB][a-f0-9]{3}-[a-f0-9]{12})/
@@ -486,21 +487,20 @@ module RedmineDmsf
           else
             return BadRequest
           end
-          begin
-            l = DmsfLock.find(token)
-            # 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
-            entity = file || folder
-            return NoContent unless entity&.locked?
-            l_entity = l.file || l.folder
-            if l_entity != entity
-              Forbidden
-            else
-              entity.unlock!
-              NoContent
-            end
-          rescue
-            BadRequest
+          l = DmsfLock.find_by_uuid(token)
+          unless l
+            return NoContent
+          end
+          # 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
+          entity = file || folder
+          return NoContent unless entity&.locked?
+          l_entity = l.file || l.folder
+          if l_entity != entity
+            Forbidden
+          else
+            entity.unlock!
+            NoContent
           end
         end
       end

test/integration/webdav/dmsf_webdav_unlock_test.rb

@@ -58,7 +58,7 @@ class DmsfWebdavUnlockTest < RedmineDmsf::Test::IntegrationTest
     assert_response :success
     process :unlock, "/dmsf/webdav/#{@file2.project.identifier}/#{@file2.name}", params: nil,
             headers: @admin.merge!({ HTTP_DEPTH: 'infinity', HTTP_TIMEOUT: 'Infinite', HTTP_LOCK_TOKEN: l.uuid })
-    assert_response :bad_request
+    assert_response :no_content
   end
 
   def test_unlock_folder_wrong_path
@@ -89,7 +89,7 @@ class DmsfWebdavUnlockTest < RedmineDmsf::Test::IntegrationTest
     process :unlock, "/dmsf/webdav/#{@folder2.project.identifier}/#{@folder2.dmsf_folder.title}/#{@folder2.title}",
             params: nil,
             headers: @jsmith.merge!({ HTTP_DEPTH: 'infinity', HTTP_TIMEOUT: 'Infinite', HTTP_LOCK_TOKEN: l.uuid })
-    assert_response :bad_request
+    assert_response :no_content
   end
 
   def test_unlock_file_in_subproject