`increment` returns a boolean

[dceddia]

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?

litestack/lib/litestack/sql/litecache.sql.yml

Lines 73 to 80 in f8283e9

	incrementer: >
	INSERT INTO data (id, value, expires_in, last_used)
	VALUES ($1, $2, unixepoch('now') + $3, unixepoch('now'))
	ON CONFLICT(id) DO UPDATE
	SET
	value = CAST(value AS int) + CAST(EXCLUDED.value AS int),
	last_used = EXCLUDED.last_used,
	expires_in = EXCLUDED.expires_in;

[dceddia]

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:

litestack/lib/active_support/cache/litecache.rb

Lines 24 to 36 in f8283e9

	def increment(key, amount = 1, options = nil)
	key = key.to_s
	options = merged_options(options)
	@cache.transaction do
	if (value = read(key, options))
	value = value.to_i + amount
	write(key, value, options)
	else
	write(key, amount, options)
	end
	end
	end

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.