Lockable Records

[catmando]

Hyperstack has no built in way to lock records so that only one client can be editing a record at a time. Here is a pattern for doing so, that could be turned into a standalone gem or a gist.

Summary

It is fairly easy to do using stuff already available in Hyperstack. There are some tricky application level user experience (UX) decisions about how to insure that things eventually become unlocked, but these are application design problems, and a lot of the details could be hidden in a nice reusable module.

Locking / Unlocking

Locking implementation is straight forward, you just need to add a belongs_to attribute in the record for the user who has locked the record. You can't lock the record with a normal active record update because you need to prevent race conditions between users. However we can add a generic Lock ServerOp that takes a class name, record id, attribute name, time_out and attempts to lock the record.

module Lockable 
  class Lock < ServerOp 
    param :acting_user  # all remote server ops must include this parameter which is 
                                      # securely filled in by hyperstack before running the operation
    param :record_class
    param :record_id 
    param :lock_attribute
    param :time_out  # this will be used to prevent permanent locking of records
    step do 
      ActiveRecord::Base.transaction do
        record = const_get(params.record_class).find(params.record_id, :lock => true)
        record.lock_attribute = nil if record.updated_at < params.time_out
        fail if record[lock_attribute] && record[lock_attribute] != params.acting_user
        record # use permission system to check if acting user is allowed to update this record
          .check_permission_with_acting_user(params.acting_user, :update)
          .update(lock_attribute => params.acting_user)
      end
    end
  end
end

Note that the Lock server op if called by the same user when the lock is already acquired by that user will simply update the lock time.

Since there is no need to insure mutual exclusion when unlocking the record, we can simply set the lock attribute value to nil using a normal ActiveRecord update. This could be done when the form is saved to avoid extra calls to the server. Alternatively you could include an after_save hook in the Model, so it would be done automatically on save.

class MyDataRecord < ActiveRecord::Base 
  ...
  before_save { self.lock_user = nil }
  ...
end

The normal broadcast mechanism will insure that the lock attribute is updated across all browsers as the lock changes, so now we can use the value the lock attribute to control access to the form.

Basic Button Behavior

The application will have to be responsible for insuring that the "Edit" button is disabled if the lock field value is non-nil. If two users click the edit button at the same time one will acquire the lock, while the other user will see the button disabled, and the click will simply be ignored. In the disabled state the button could have an optional tool tip explaining what is going on. This can all be built easily using the base HTML components, tool tip libraries, styles, and click handlers.

Meanwhile inside the form the save button will just set the lock attribute to nil and save the record with the changed data, while the cancel button will revert the record, and then do an update of the lock attribute.

Note that if the system is one in which users "know about each other" you can add the locking user's name to the tool tip as well since it referenced in the lock attribute.

For any given form you are just adding a very few lines of code and classes to the edit button, and the cancel button. Save works as normal! But it's hard to generalize this as the specific UX will depend on the requirements of the application. For example edit might be a link not a button. The tool tip could be implemented in many ways, etc. So its probably best just to make this a programming pattern rather than try to make it into some kind of reusable component (at least until we have more experience.)

Insuring records don't get permanently locked

The real problem is how do you deal with making sure the record gets unlocked?

The first case to consider is what if the user just closes the browser? In this case the record will stay locked forever. The solution is to have the Edit button check for both the lock attribute, and the last_updated_at value of the record. If the record has not been updated for some period of time, the button can assume the record is lockable.

But kicking a user out from editing a record at some timeout is not the full solution. If the timeout is short, then users might not finish editing. If the timeout is long then you might wait a long time just because somebody else abandoned the edit, without cancelling.

The solution is to pick a first timeout (60 seconds for example) and then to set an every interval timer in the form component's after_mount hook. When ever this interval expires the code will check to see if any edits have been made, and if so the Lock operation is called again which updates the last_updated_at field. If no edits have been made, a modal dialog pops up asking the user to confirm they are still there. If yes the lock is updated, and life goes on, if no then the form edit is cancelled. Finally the dialog is watched by a second timer, and if this timer expires, the form is also cancelled.

class MyForm < HyperComponent

  param :record

  EDIT_TIMEOUT = 60
  CONTINUE_TIMEOUT = 30

  self << class
    def editable?(record)
      !record.lock_user ||
      record.updated_at > Time.now - (EDIT_TIMEOUT + CONTINUE_TIME_OUT)
    end

    def editing?
      record.lock_user.id == Hyperstack::Application.acting_user_id
    end

    def edit!(record)
        Lockable::Lock.run(
          record_class:    MyDataRecord, 
          record_id:         record.id, 
          lock_attribute: :lock_user, 
          user_id:             Hyperstack::Application.acting_user_id,
          time_out:           EDIT_TIMEOUT + CONTINUE_TIME_OUT
        )
    end
  end

  after_mount do 
    every(EDIT_TIMEOUT) do 
      if @form_edited
        self.class.edit!
      else
        show_continue_dialog
      end
      @form_edited = false
    end
  end

  after_update do 
    @form_edited = true
  end

  def show_continue_dialog
    @continue = false
    timer = after(CONTINUE_TIME_OUT) do 
      record.update(lock_user: nil)
    end
    if continue_dialog
      timer.abort!
    else
      record.update(lock_user: nil)
    end
  end

  # continue_dialog is application dependent, by default we raise a confirm box
  # the method should return truthy if the user wishes to continue editing
  def continue_dialog
    confirm("continue editing?")
  end

  # be  sure to update lock_user to nil before saving the record as well...
end

class FormContainer < HyperComponent 
  render do 
    if MyForm.editing?
      MyForm(...)
    else
      BUTTON(disabled: !MyForm.editable?) { 'Edit' }
      .on(:click) { MyForm.edit!(record: record) }
    end
  end
end

You could easily make the contents of "MyForm" above into a nice module builder that could be included like this:

class MyForm < HyperComponent
  include Lockable::ComponentHelper[  # these would be the defaults
    record_accessor: :record,  
    lock_attribute: :lock_user, 
    edit_timeout: 60, 
    confirm_timeout: 30
  ]
  # override the continue_dialog method if desired
end

see https://dejimata.com/2017/5/20/the-ruby-module-builder-pattern for hints how to do this.

[Tim-Blokdijk]

Yes, makes a lot of sense. I definitely think we need a "hyper-lock" gem, people should not have to invent their own locking system. But it's a complex topic, needs to be simple but also flexible.. and atomic without race conditions.
And the timeout with confirmation dialog is probably the way to go on the "permanently locked" problem.

The belongs_to change to the models that need locking feels a bit itchy. My gut feeling is telling me that using cache is a better fit, allows for more flexible locking data, no need for migrations..
I think cache can be atomic and distributed, but I'm not an expert.

I would also want to lock a single attribute, and still allow others to edit other attributes of the same record. For example, I could set the location of an appointment on a map while someone else is writing the memo field of the same appointment. Only locking the location and memo inputs for each other.
I also have this (bad) habit of serializing data structures in my models.. saving many values as one attribute. I'm trying to think of ways to do partial locking for such an attribute if each value is its own form input field that can be disabled.

[catmando]

both ideas here are interesting (using the cache for locking and locking single attributes) are really interesting.

You are down to locking bits of data inside an attribute, and in this case what you want is just a unified mutex that works between server and client. HyperMutex if you will. As you suggest by using the Rails cache this can be separate from ActiveRecord.

Instead of locking on an ActiveRecord model, you lock on an instance of any class, and you provide the 'identifying' method of the class (which defaults to id).

The rest works pretty much the same as previously described, except you need an Unlock Operation.

Internally instead of manipulating active record objects, it just stores in the cache the class, the value returned by the id method, and a TTL value.

The component part is about the same as well.

Good thinking!

[Tim-Blokdijk]

Thanks, I must say that although I do know what a mutex is, I have always skillfully avoided situations where I would actually have to use them beyond a file or db lock. Wrangling threads is not my thing.
So, designing a mutex system.. isn't something I'd be good at.
But willing to help.

[catmando]

the locking mechanism is builtin to rails cache fetch method.

I believe something like this would work:

# note that this class should be in the `app/hyperstack/shared` directory
class Lockable < SimpleDelegator

  # Lockable.new(obj) adds the locking methods to any obj

  include Hyperstack::State::Observable

  # obj: the object to be locked.  
  # lock_id: if present will be used to uniquely identify the object
  #          otherwise the lock_id will be computed (see the lock_id_for method)
  # attribute: allows parts of an object to be locked independently
  # lock_time_out: user should be warned once lock_time expires
  # continue_time_out: lock should be released after lock_time_out + continue_time_out
  
  # normally the UI will manually release the lock within lock_time_out + continue_time_out
  # however the lock will always be released after lock_time_out + continue_time_out + 60

  # before allocating a new lock object we check to make sure one has not already been
  # created for this object, lock_id and attribute.  If it has return the already allocated lock object

  # This allows us  the application to repeatedly call Lock.new(...) with the same params 
  # without the overhead of constantly setting up and tearing down 
  # Unlock receivers.

  def self.new(obj, lock_id = nil, attribute: nil, lock_time_out: 60, continue_time_out: 30)
    lock_id = lock_id_for(obj, lock_id, attribute)
    locks = obj.instance_variable_get(:'@__hyperlock_locks_hash')
    locks ||= obj.instance_variable_set(:'@__hyperlock_locks_hash', {})
    lock = locks[lock_id])
    lock || (locks[lock_id] = super(obj, lock_id, lock_time_out, continue_time_out))
  end

  # convenience methods:

  attr_reader :lock_time_out
  attr_reader :continue_time_out

  # observer methods - these will notify the caller when the state changes
  
  observer(:lockable?){ @lockable }
  observer(:locked?) { @locked }

  # mutator methods - these may change the state of the lock
  
  def lock! 
    # Note: @locked is set only if the promise successfully resolves
    Lock.run(lock_id: @lock_id, time_out: @edit_time_out + @continue_time_out).then do
      mutate @locked = true
    end
  end

  def unlock! 
    mutate @locked = false
    Unlock.run(lock_id: @lock_id)
  end

  def self.lock_id_for(obj, lock_id, attribute)
    lock_id ||=
      if obj.respond_to? :lock_id 
        obj.lock_id
      elsif obj.respond_to? :id
        "#{obj.class}-#{obj.id}"
      else
        raise "Lock.new must be provided a lock_id or the object must respond to either lock_id or id"
      end
    "HYPERLOCK-LOCK-KEY-#{lock_id}-#{attribute}"
  end

  def initialize(obj, lock_id, lock_time_out: 60, continue_time_out: 30)
    @lockable = true
    @lock_id = lock_id
    @lock_time_out = lock_time_out
    @continue_time_out = continue_time_out 
     receives Unlock { |params| mutate @lockable = true if params.lock_id = lock_id }
    super obj
  end

  class Lock < ServerOp 
    param :acting_user  # all remote server ops must include this parameter which is 
                                      # securely filled in by hyperstack before running the operation
    param :lock_id 
    param :time_out  # this prevents permanent locking of objects by using the cache expiration time
    step { fail if params.acting_user != Rails.cache.fetch(params.lock_id) { params.acting_user } }
    step { params.time_out += 60 } # give a margin of error.
    step { Rails.cache.write(params.lock_id, params.acting_user, expires_in: params.time_out)  }   
  end

  class Unlock < ServerOp
    param :acting_user 
    param :lock_id 
    # make sure the acting_user owns the lock
    step { fail unless params.acting_user == Rails.cache.read(params.lock_id, expires_in: nil) }
    # clear the cache lock
    step { Rails.cache.delete(params.lock_id) }
    # and broadcast the good news to all
    dispatch_to { Hyperstack::Application }
  end
end

To create a lockable active record class you would say:

  lockable_record = Lockable.new(record) 
  # lock_id will be "#{record.class}-#{record.id}-")
  ...
  # lockable_record responds to all active record methods as normal, plus...
  ...
  lockable_record.lockable? # true if the lock can be acquired
  lockable_record.lock # attempts to lock the record
  lockable_record.locked? # returns true once record has been locked
  lockable_record.unlock # unlocks the record
  # these methods are all observer/mutators so they will trigger rerenders
  # when their internal state changes

The lock_id param can be any string uniquely identifying what you are trying to lock.

The defaults give sensible values for any object that responds to the id method.

Or if the class responds to lock_id then that will be used.

If you wanted to lock a specific attribute only (a_big_string for example) you can provide the attribute parameter. Note it does not have to be an actual attribute, this just represents a unique name for the lock within the overall object.

Any object can be lockable, as long as you can compute a lock_id that will be unique across the entire application. For example you can't use object_id as part of the lock_id as it will have different values in each browser in the system.

class MyForm < HyperComponent  
  param :record # must be a Lockable object  
  after_mount do 
    every(record.lock_timeout) do 
      if @form_edited
        record.lock!
      else
        show_continue_dialog
      end
      @form_edited = false
    end
  end

  after_update do 
    # assumes you will use controlled components so that you rerender on each change
    @form_edited = true
  end

  def show_continue_dialog
    @continue = false
    timer = after(record.continue_timeout) do 
      record.unlock
    end
    if continue_dialog
      timer.abort!
    else
      record.update(lock_user: nil)
    end
  end

  # continue_dialog is application dependent, by default we raise a confirm box
  # the method should return truthy if the user wishes to continue editing
  def continue_dialog
    confirm("continue editing?")
  end
  
  render do 
     # .... show the form
     # be  sure to call record.unlock! after saving the record as well...
  end
end

class FormContainer < HyperComponent 
  param :record
  render do 
    lockable_record = Lockable.new(record)
    if lockable_record.locked?
      MyForm(record: lockable_record)
    else
      BUTTON(disabled: !lockable_record.lockable?) { 'Edit' }
      .on(:click) { lockable_record.lock! }
    end
  end
end

[Tim-Blokdijk]

Yes. I can see this working.

The ’acting_user’ might need some more thought. If the same user has two browser windows open.. it probably needs to lock.
Also, ’acting_user’ might not be defined. Maybe it needs a ’connection’ (ActionCable) object?

[catmando]

Good point. You don't want to lock on acting user, but on the specific browser.

[Tim-Blokdijk]

Yea, but I do want to know that it's me having a lock in another browser tab, so some sort of optional link to acting_user would be good to have.

How hard would it be to serialize the locked object (edited, but not saved) and transfer its state to another browser? Presuming the serialized state is not initialized on the server..

[Tim-Blokdijk]

If we lock on the ActionCable connection then a disconnect could also trigger a clean up of stale locks.
Would still need the time_out as users can keep the connection alive while afk.

[catmando]

The way to so the lock per browser tab (which you are correct is the right way to do it NOT by acting user) is to create a GUID then pass this to the Lock and Unlock server operations, and use that instead of param.acting_user.

You do not require a logged in user to lock (i.e. any browser can lock) then you change the acting_user param to be param :acting_user, allow_nil: true

The channel connections are made based on sessions, and logged in users, there is no concept of channel per browser tab, so there is no way to detect that a specific tab has disappeared. Even if you could the disconnecting of channels is also based on timeouts, and is dependent on the transport mechanism (i.e. you don't have to use action-cable) so you would not want to depend on that anyway.

[Tim-Blokdijk]

I think a GUID can work, it would need to be associated with the browser tab.. wait,.. how would we prevent the GUID from being regenerated with a page reload? I think it would need to be saved in sessionStorage? It's data storage per tab unlike localStorage that works across tabs.
Or am I missing something?

And I think you're right about not clearing the locks on loss of a connection. It would allow you to lock a record with a time_out of many hours, work on a airplane, reconnect after landing and save your record.

[Tim-Blokdijk]

Still, not clearing locks can be a problem.
You need to prevent a situation where you lock a record (for lets say 5 min), close the browser tab, open a browser tab and then having to wait for the stale lock to time out. Refreshing would use the same GUID if stored in localStorage. But closing and reopening a tab won't let you recover the lock.

Might be an idea to hook in the navigator.sendBeacon(url, data) function on page unload to release any locks or update the lock time to something like 10 seconds in case the browser tab does not recover the lock after a page reload.

Another approach could be a way to break the existing lock. A function where you can overwrite the lock unless the tab with the lock GUID reaffirms the continue_dialog lock within x amount of seconds.

[catmando]

This is why nobody uses file locking for version control anymore :-)

That said I don't think you would have a lock for anywhere near 5 minutes. If you watch the mouse movement as well as detect fields changing, I would think most apps would need a timeout of 20 seconds until a warning is displayed, followed by another 20 (or even less) warning timeout.

So probably less than 1 minute wait max. Annoying but its a tails case, and its the price you pay for some kind conflict managment.

If you try a tool like Trello (very popular) they have no conflict management control. If two people are editing the same trello card, they just overwrite each other's saves. I would much rather wait occassionally for maybe 30 seconds, to prevent that.

But perhaps there is a better way?

[Tim-Blokdijk]

It's a complex topic. Overwriting might be the best default.

The ideal would be something like Google Docs where changes are merged automatically. That's not something we should try to build ourselfs. And I'm not familiar with any open source library that can merge changes to data structures on the fly like Google Docs does.
If it does exist I would be interested to know about it. It's not impossible to to keep a shared state among different nodes. Game engines have been doing it for decades.

But Hyperstack does need some best practices.
Even with overwriting as the default, just a consistent API to show what data other users are touching would be great. Actually locking the resource in the view can be optional.

---

Conceptually the lock_id in your example code is also something to expand on. It's the ID that would allow a more general object sharing between clients. A client (GUID) could subscribe to a lock_id at which point attributes could be synced like with the ActiveRecord implementation.