Feature request: allow lock! to be extended

[nikolai-b]

I would like to extend a lock!

Would you consider making lock! return the lock_info and also allow it to be called without a block, e.g.

def lock!(*args)
  lock(*args) do |lock_info|
    raise LockError, 'failed to acquire lock' unless lock_info
    if block_given?
      yield lock_info
    else
      lock_info
    end
  end
end

[maltoe]

@nikolai-b Not sure I understand, what's the difference to lock() then? If it's only about the raise on error, why not do

lock_info = client.lock(...)
raise Redlock::LockError unless lock_info

?

[nikolai-b]

No real difference, just often in rails bang methods do the same thing but raise instead of returning false / nil.

> User.find_by(id: 1)
=> nil
> User.find_by!(id: 1)
=> ActiveRecord::RecordNotFound: Couldn't find User

I understand that in ruby core the bang means it modifies the receiver.

Yes I can do what you suggest if you prefer to keep the lock! method having a different pattern but it surprised me and stops lock! being able to be extended.

[maltoe]

Hmm, the original implementation of this dates back before I stumbled into this gem :)

I see what you mean now, and agree that the behaviour of lock/lock! is a bit confusing. TBH, I think the lock function probably just shouldn't accept a block at all (as when you call it with a block and don't validate the result, you're not sure the block was called at all), but that's probably a different matter.

So yes, if you think this is needed, please come up with a PR for it. Please be aware though lock() with a block given will unlock the resource when the block exits, so the implementation would need to look more like:

def lock!(*args)
  if block_given?
    lock(*args).tap { |lock_info| raise ... unless lock_info }
  else
    lock(*args) do |lock_info|
      raise ... unless lock_info
      yield
    end
  end
end

Not sure if it's really worth it..