Files remain locked after content editing

danmunn · May 6, 2021 · d19586a · d19586a
2 parents 589531d + 1e719b2
commit d19586a
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 20 deletions.
diff --git a/lib/redmine_dmsf/lockable.rb b/lib/redmine_dmsf/lockable.rb
@@ -97,12 +97,12 @@ def locked_for_user?(args = nil)
         locks.each do |lock|
           next if lock.expired? # In case we're in between updates
           owner = args[:owner] if args
-          owner ||= User.current&.login
+          owner ||= User.current&.login if lock.owner
           if lock.lock_scope == :scope_exclusive
-            return true if (lock.user&.id != User.current.id) || (lock.owner && (lock.owner != owner))
+            return true if (lock.user&.id != User.current.id) || (lock.owner != owner)
           else
             shared = true if shared.nil?
-            if shared && (lock.user&.id == User.current.id) && (!lock.owner || (lock.owner == owner)) ||
+            if shared && (lock.user&.id == User.current.id) && (lock.owner == owner) ||
               (args && (args[:scope] == 'shared'))
               shared = false
             end

diff --git a/lib/redmine_dmsf/webdav/dmsf_resource.rb b/lib/redmine_dmsf/webdav/dmsf_resource.rb
@@ -479,28 +479,28 @@ def unlock(token)
           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})/
             token = $1
           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

diff --git a/test/integration/webdav/dmsf_webdav_unlock_test.rb b/test/integration/webdav/dmsf_webdav_unlock_test.rb
@@ -58,7 +58,7 @@ def test_unlock_file_not_locked
     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 @@ def test_unlock_folder_not_locked
     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