`increment` returns a boolean

Hello! I'm using Litestack in my app, mainly just Litecache right now, using it as my cache implementation in Rails.

config.cache_store = :litecache

This is being used by Rack::Attack, and it's hitting an error when a request comes in.

Error: NoMethodError - undefined method `>' for true
/usr/local/bundle/ruby/3.3.0/gems/rack-attack-6.7.0/lib/rack/attack/throttle.rb:41:in `matched_by?'
/usr/local/bundle/ruby/3.3.0/gems/rack-attack-6.7.0/lib/rack/attack/configuration.rb:91:in `block in throttled?'
/usr/local/bundle/ruby/3.3.0/gems/rack-attack-6.7.0/lib/rack/attack/configuration.rb:90:in `any?'
/usr/local/bundle/ruby/3.3.0/gems/rack-attack-6.7.0/lib/rack/attack/configuration.rb:90:in `throttled?'
/usr/local/bundle/ruby/3.3.0/gems/rack-attack-6.7.0/lib/rack/attack.rb:118:in `call'

Digging into this a bit, it looks like Rack::Attack is calling cache.count(...) which actually calls into Rack::Attack::Cache and runs this code

def do_count(key, expires_in)
  enforce_store_presence!
  enforce_store_method_presence!(:increment)

  result = store.increment(key, 1, expires_in: expires_in)   # <<<<< problem starts here

  # NB: Some stores return nil when incrementing uninitialized values
  if result.nil?
    enforce_store_method_presence!(:write)

    store.write(key, 1, expires_in: expires_in)
  end
  result || 1
end

The store.increment call returns true but it looks like this code (and the caller) expect it to return a number.

Looking at the SQL for this, I'm wondering if it needs a RETURNING value at the end?

https://github.com/oldmoe/litestack/blob/f8283e97fd4cdcce768572a2b8ef77886f953f89/lib/litestack/sql/litecache.sql.yml#L73-L80

I tried patching this locally and it didn't have the effect I was hoping for! So I did some more digging.

It turns out the code is actually getting tripped up by a different increment method, the one in active_support/cache/litecache.rb here:

https://github.com/oldmoe/litestack/blob/f8283e97fd4cdcce768572a2b8ef77886f953f89/lib/active_support/cache/litecache.rb#L24-L36

The transaction returns a boolean where I think this should be returning the result of the increment.

I made this change which seems to solve the error, and the tests pass.

I'm not sure if setting result = amount is the right way to go here, but there was a test failing when I initialized it with result = nil.

https://github.com/dceddia/litestack/blob/ece67131a7b734f53996399a8cae17cc7f3ad342/lib/active_support/cache/litecache.rb#L24-L40

def increment(key, amount = 1, options = nil)
  key = key.to_s
  options = merged_options(options)

  result = amount
  @cache.transaction do
    if (value = read(key, options))
      value = value.to_i + amount
      write(key, value, options)
      result = value
    else
      write(key, amount, options)
      result = amount
    end
  end
  result
end

I figure there probably should be a new test to accompany this but I wasn't sure where that would go.

oldmoe / litestack

`increment` returns a boolean #122