Starting lock system overhaul in line with #14 - designed new approach will allow for folder locking, and shared locking - todo: rewrite lock! and

unlock! methods in lib/redmine_dmsf/lockable.rb

Gemfile

@@ -2,6 +2,7 @@ source :rubygems
 
 gem "zip"
 gem "dav4rack", :github => "chrisroberts/dav4rack"
+gem "simple_enum"
 
 #Allows --without=xapian
 group :xapian do

app/controllers/dmsf_files_controller.rb

@@ -25,9 +25,6 @@ class DmsfFilesController < ApplicationController
   before_filter :find_revision, :only => [:delete_revision]
   before_filter :authorize
 
-#  verify :method => :post, :only => [:create_revision, :delete_revision, :delete, :lock, :unlock, :notify_activate, :notify_deactivate], 
-#    :render => { :nothing => true, :status => :method_not_allowed }
-
   helper :all
 
   def show
@@ -161,7 +158,7 @@ class DmsfFilesController < ApplicationController
     if @file.locked?
       flash[:warning] = l(:warning_file_already_locked)
     else
-      @file.lock
+      @file.lock!
       flash[:notice] = l(:notice_file_locked)
     end
       redirect_to params[:current] ? params[:current] : 
@@ -173,7 +170,7 @@ class DmsfFilesController < ApplicationController
       flash[:warning] = l(:warning_file_not_locked)
     else
       if @file.locks[0].user == User.current || User.current.allowed_to?(:force_file_unlock, @file.project)
-        @file.unlock
+        @file.unlock!
         flash[:notice] = l(:notice_file_unlocked)
       else
         flash[:error] = l(:error_only_user_that_locked_file_can_unlock_it)

app/controllers/dmsf_files_copy_controller.rb

@@ -24,8 +24,6 @@ class DmsfFilesCopyController < ApplicationController
   before_filter :find_file
   before_filter :authorize
 
-#  verify :method => :post, :only => [:create, :move], :render => { :nothing => true, :status => :method_not_allowed }
-
   helper :all
 
   def new

app/controllers/dmsf_upload_controller.rb

@@ -25,9 +25,6 @@ class DmsfUploadController < ApplicationController
   before_filter :authorize
   before_filter :find_folder, :except => [:upload_file]
   
-#  verify :method => :post, :only => [:upload_files, :upload_file, :commit_files], 
-#    :render => { :nothing => true, :status => :method_not_allowed }
-
   helper :all
 
   def upload_files

app/models/dmsf_file.rb

@@ -26,13 +26,17 @@ end
 
 class DmsfFile < ActiveRecord::Base
   unloadable
+
+  include RedmineDmsf::Lockable
+ 
   belongs_to :project
   belongs_to :folder, :class_name => "DmsfFolder", :foreign_key => "dmsf_folder_id"
   has_many :revisions, :class_name => "DmsfFileRevision", :foreign_key => "dmsf_file_id", 
     :order => "major_version DESC, minor_version DESC, updated_at DESC", 
     :dependent => :destroy
-  has_many :locks, :class_name => "DmsfFileLock", :foreign_key => "dmsf_file_id", 
+  has_many :locks, :class_name => "DmsfLock", :foreign_key => "entity_id",
     :order => "updated_at DESC",
+    :conditions => {:entity_type => 0},
     :dependent => :destroy
   belongs_to :deleted_by_user, :class_name => "User", :foreign_key => "deleted_by_user_id"
   scope :visible, lambda {|*args| {:conditions => DmsfFile.visible_condition(args.shift || User.current, *args) }}
@@ -115,26 +119,6 @@ class DmsfFile < ActiveRecord::Base
     end
   end
   
-  def locked?
-    self.locks.empty? ? false : self.locks[0].locked
-  end
-  
-  def locked_for_user?
-    self.locked? && self.locks[0].user != User.current
-  end
-  
-  def lock
-    l = DmsfFileLock.file_lock_state(self, true)
-    self.reload
-    return l
-  end
-  
-  def unlock
-    l = DmsfFileLock.file_lock_state(self, false)
-    self.reload
-    return l
-  end
-  
   def title
     self.last_revision.title
   end

app/models/dmsf_folder.rb

@@ -18,6 +18,8 @@
 
 class DmsfFolder < ActiveRecord::Base
   unloadable
+
+  include RedmineDmsf::Lockable
   
   cattr_reader :invalid_characters
   @@invalid_characters = /\A[^\/\\\?":<>]*\z/
@@ -30,6 +32,11 @@ class DmsfFolder < ActiveRecord::Base
            :dependent => :destroy
   belongs_to :user
 
+  has_many :locks, :class_name => "DmsfLock", :foreign_key => "entity_id",
+    :order => "updated_at DESC",
+    :conditions => {:entity_type => 1},
+    :dependent => :destroy
+
   scope :visible, lambda {|*args| {:conditions => "" }} #For future use, however best to be referenced now
 
   acts_as_customizable

app/models/dmsf_lock.rb

@@ -16,11 +16,31 @@
 # along with this program; if not, write to the Free Software
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
 
-class DmsfFileLock < ActiveRecord::Base
+class DmsfLock < ActiveRecord::Base
   unloadable
-  belongs_to :file, :class_name => "DmsfFile", :foreign_key => "dmsf_file_id"
+
-  belongs_to :user  
+  belongs_to :file, :class_name => "DmsfFile", :foreign_key => "entity_id"
+  belongs_to :folder, :class_name => "DmsfFolder", :foreign_key => "entity_id"
+  belongs_to :user
+
+  #At the moment apparently we're only supporting a write lock?
+
+  as_enum :lock_type, [:type_write]
+  as_enum :lock_scope, [:scope_exclusive, :scope_shared]
   
+  # We really loosly bind the value in the belongs_to above
+  # here we just ensure the data internal to the model is correct
+  # to ensure everything lists fine - it's the same as a join
+  # just without runing the join in the first place
+  def file
+    entity_type == 0 ? super : nil;
+  end
+
+  # see file, exact same scenario
+  def folder
+    entity_type == 1 ? super : nil;
+  end
+
   def self.file_lock_state(file, locked)
     lock = DmsfFileLock.new
     lock.file = file
@@ -28,5 +48,10 @@ class DmsfFileLock < ActiveRecord::Base
     lock.locked = locked
     lock.save!
   end
+
+  def expired?
+    return false if expires_at.nil?
+    return expires_at <= Time.now
+  end
   
-end
+end

db/migrate/07_dmsf_1_4_4.rb

@@ -0,0 +1,86 @@
+# Redmine plugin for Document Management System "Features"
+#
+# Copyright (C) 2012 Daniel Munn <dan.munn@munnster.co.uk>
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+
+class Dmsf144 < ActiveRecord::Migration
+
+
+  class DmsfFileLock < ActiveRecord::Base
+    belongs_to :file, :class_name => "DmsfFile", :foreign_key => "dmsf_file_id"
+    belongs_to :user
+  end  
+
+  def self.up
+
+    #Add our entity_type column (used with our entity type)
+    add_column :dmsf_file_locks, :entity_type, :integer, :null => true
+
+    #Add our lock relevent columns (ENUM) - null (till we upgrade data)
+    add_column :dmsf_file_locks, :lock_type_cd, :integer, :null => true
+    add_column :dmsf_file_locks, :lock_scope_cd, :integer, :null => true
+
+    #Add our expires_at column
+    add_column :dmsf_file_locks, :expires_at, :datetime, :null => true
+
+    do_not_delete = []
+    DmsfFileLock.order('updated_at DESC').group('dmsf_file_id').find_each do |lock|
+      if (lock.locked)
+        do_not_delete << lock.id
+      end
+    end
+
+    say "Preserving #{do_not_delete.count} file lock(s) found in old schema"
+
+    DmsfFileLock.delete_all(['id NOT IN (?)', do_not_delete])
+
+    say "Applying default lock scope / type - Exclusive / Write"
+    DmsfFileLock.update_all ['entity_type = ?, lock_type_cd = ?, lock_scope_cd = ?', 0, 0, 0]
+
+    #These are not null-allowed columns
+    change_column :dmsf_file_locks, :entity_type, :integer, :null => false
+    change_column :dmsf_file_locks, :lock_type_cd, :integer, :null => false
+    change_column :dmsf_file_locks, :lock_scope_cd, :integer, :null => false
+
+    #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
+  end
+
+  def self.down
+
+    rename_table :dmsf_locks, :dmsf_file_locks
+
+    add_column :dmsf_file_locks, :locked, :boolean, :default => false, :null => false
+
+    #Data cleanup - delete all expired locks, or any folder locks
+    say "Removing all expired and/or folder locks"
+    DmsfFileLock.delete_all ['expires_at < ? OR entity_type = 1', Time.now]
+
+    say "Changing all records to be locked"
+    DmsfFileLock.update_all ['locked = ?', true]
+
+    rename_column :dmsf_file_locks, :entity_id, :dmsf_file_id
+
+    remove_column :dmsf_file_locks, :entity_type
+    remove_column :dmsf_file_locks, :lock_type_cd
+    remove_column :dmsf_file_locks, :lock_scope_cd
+    remove_column :dmsf_file_locks, :expires_at
+  end
+
+end

init.rb

@@ -24,7 +24,7 @@ Redmine::Plugin.register :redmine_dmsf do
   name "DMSF"
   author "Vit Jonas / Daniel Munn"
   description "Document Management System Features"
-  version "1.4.3 stable"
+  version "1.4.4 devel"
   url "https://github.com/danmunn/redmine_dmsf"
   author_url "https://code.google.com/p/redmine-dmsf/"
   

lib/redmine_dmsf/lockable.rb

@@ -0,0 +1,81 @@
+module RedmineDmsf
+  module Lockable
+
+    def locked?
+      !lock.empty?
+    end
+
+    # lock:
+    #   Returns an array with current lock objects that affect the current object
+    #   optional: tree = true (show entire tree?)
+    def lock(tree = true)
+      ret = []
+      unless locks.empty?
+        locks.each {|lock|
+          ret << lock unless lock.expired?
+        }
+      end
+      if tree
+        ret = ret | folder.lock unless folder.nil?
+      end
+      return ret
+    end
+
+    def lock! scope = :exclusive, type = :write
+      l = DmsfLock.lock_state(self, scope, type)
+      self.reload
+      return l
+    end
+
+    #
+    # By using the path upwards, surely this would be quicker?
+    def locked_for_user?(tree = true)
+      return false unless locked?
+      b_shared = nil
+      heirarchy = self.dmsf_path
+      heirarchy.each {|folder|
+        locks = folder.lock(false)
+        next if locks.empty?
+        locks.each {|lock|
+          next if lock.expired? #Incase we're inbetween updates
+          if (lock.lock_scope == :scope_exclusive && b_shared.nil?)
+            return true if lock.user.id != User.current.id
+          else
+            b_shared = true if b_shared.nil?
+            b_shared = false if lock.user.id == User.current.id
+          end
+        }
+        return true if b_shared
+      }
+      false
+    end
+
+#    #Any better suggestions on this? - This is quite cumbersome
+#    def locked_for_user_old?
+#      return false unless locked?
+#      b_shared = nil
+#
+#      unless locks.empty?
+#        locks.each {|lock|
+#          continue if lock.expired? #Incase we're inbetween updates
+#          if (lock.lock_scope == :scope_exclusive && b_shared.nil?)
+#            return true if lock.user.id != User.current.id
+#          else
+#            b_shared = true if b_shared.nil?
+#            b_shared = false if lock.user.id == User.current.id
+#          end
+#        }
+#        return true if b_shared
+#      end
+#      return folder.locked_for_user? unless folder.nil?
+#      false
+#    end
+
+    def unlock!
+      l = DmsfLock.lock_state(self, false)
+      self.reload
+      return l
+    end
+
+  end
+end

lib/redmine_dmsf/webdav/dmsf_resource.rb

@@ -403,7 +403,7 @@ module RedmineDmsf
         if (file.locked? && file.locked_for_user?)
           raise DAV4Rack::LockFailure.new("Failed to lock: #{@path}")
         else
-          file.lock unless file.locked?
+          file.lock! unless file.locked?
           @response['Lock-Token'] = token
           Locked
           [8600, token]
@@ -423,7 +423,7 @@ module RedmineDmsf
           if (!file.locked? || file.locked_for_user? || token != _token)
             Forbidden
           else
-            file.unlock
+            file.unlock!
             NoContent
           end
         end

test/fixtures/dmsf_files.yml

@@ -28,3 +28,21 @@ dmsf_files_003:
   deleted:            1
   deleted_by_user_id: 1
 
+dmsf_files_004:
+  id:                 4
+  project_id:         1
+  dmsf_folder_id:     2
+  name:               "test.txt"
+  notification:       0
+  deleted:            0
+  deleted_by_user_id: NULL
+
+dmsf_files_005:
+  id:                 5
+  project_id:         1
+  dmsf_folder_id:     5
+  name:               "test.txt"
+  notification:       0
+  deleted:            0
+  deleted_by_user_id: NULL
+

test/fixtures/dmsf_folders.yml

@@ -26,4 +26,10 @@
     project_id:     2
     dmsf_folder_id: 3
     user_id:        1
+- dmsf_folders_005:
+    id:             5
+    title:          folder3
+    project_id:     1
+    dmsf_folder_id: 2
+    user_id:        1
 

test/fixtures/dmsf_locks.yml

@@ -0,0 +1,15 @@
+---
+dmsf_locks_001:
+  id:                 1
+  entity_id:          2
+  user_id:            1
+  entity_type:        0
+  lock_type_cd:       0
+  lock_scope_cd:      0
+dmsf_locks_002:
+  id:                 2
+  entity_id:          2
+  user_id:            1
+  entity_type:        1
+  lock_type_cd:       0
+  lock_scope_cd:      0

test/unit/dmsf_file_test.rb

@@ -2,7 +2,8 @@ require File.dirname(__FILE__) + '/../test_helper'
 
 class DmsfFileTest < RedmineDmsf::Test::UnitTest
   fixtures :projects, :users, :dmsf_folders, :dmsf_files, :dmsf_file_revisions,
-           :roles, :members, :member_roles, :enabled_modules, :enumerations
+           :roles, :members, :member_roles, :enabled_modules, :enumerations,
+           :dmsf_locks
 
   test "file data is created" do
     assert_not_nil(dmsf_files(:dmsf_files_001))
@@ -29,4 +30,32 @@ class DmsfFileTest < RedmineDmsf::Test::UnitTest
     }
   end
 
+  test "Known locked file responds as being locked" do
+    file = dmsf_files(:dmsf_files_002)
+    assert file.locked?
+  end
+
+  test "File with locked folder is reported as locked" do
+    file = dmsf_files(:dmsf_files_004)
+    assert file.locked?
+  end
+
+  test "File with folder up heirarchy (locked) is reported as locked" do 
+    file = dmsf_files(:dmsf_files_005)
+    assert file.locked?
+  end
+
+  test "File with folder up heirarchy (locked) is not locked for user id 1" do
+    User.current = User.find(1)
+    file = dmsf_files(:dmsf_files_005)
+
+    assert file.locked?
+    assert !file.locked_for_user?
+  end
+
+  test "File with no locks reported unlocked" do
+    file = dmsf_files(:dmsf_files_001)
+    assert !file.locked?
+  end
+
 end

test/unit/dmsf_lock_test.rb

@@ -0,0 +1,66 @@
+require File.dirname(__FILE__) + '/../test_helper'
+
+class DmsfFileTest < RedmineDmsf::Test::UnitTest
+  attr_reader :lock
+  fixtures :projects, :users, :dmsf_folders, :dmsf_files, :dmsf_file_revisions,
+           :roles, :members, :member_roles, :enabled_modules, :enumerations,
+           :dmsf_locks
+
+  def setup
+    @lock = dmsf_locks(:dmsf_locks_001)
+  end
+
+  test "lock data is created" do
+    assert_not_nil(lock)
+  end
+
+  test "lock_type is enumerable" do
+    assert DmsfLock.respond_to?(:lock_types) #lock_types is a method created by as_enum
+    assert DmsfLock.lock_types.is_a?(Hash)
+  end
+
+  test "lock_scope is enumerable" do
+    assert DmsfLock.respond_to?(:lock_scopes) #lock_types is a method created by as_enum
+    assert DmsfLock.lock_scopes.is_a?(Hash)
+  end
+
+  test "lock_type does not accept invalid values" do
+    assert lock.lock_type = :type_write
+    assert_raise ArgumentError do
+      assert lock.lock_type = :write
+    end
+  end
+
+  test "lock_type accepts a valid answer" do
+    assert_nothing_raised ArgumentError do
+      lock.lock_type = :type_write
+      assert lock.lock_type == :type_write
+    end
+  end
+
+  test "lock_scope does not accept invalid values" do
+    assert lock.lock_scope = :scope_exclusive
+    assert_raise ArgumentError do
+      assert lock.lock_scope = :write
+    end
+  end
+
+  test "lock_scope accepts a valid answer" do
+    assert_nothing_raised ArgumentError do
+      lock.lock_scope = :scope_shared
+      assert lock.lock_scope == :scope_shared
+    end
+  end
+
+  test "linked to either file or folder" do
+    assert !(lock.file.nil? && lock.folder.nil?)
+    assert !lock.file.nil? || !lock.folder.nil?
+    if !lock.file.nil?
+      assert lock.file.is_a?(DmsfFile)
+    else
+      assert lock.file.is_a?(DmsfFolder)
+    end
+  end
+  
+
+end