Handle max_age: nil

[brian-kephart]

Right now, Solid Cache will accept a config with max_age: nil but will throw a NoMethodError when attempting to expire entries with that config. Such a config should be handled or disallowed.

Since max_entries: nil is allowed, this PR allows a nil value for max_age as well.

[djmb]

Thanks for this @brian-kephart 👍

I had a couple of minor notes on the implementation, but this is looking good otherwise!

[djmb]

Could you do .limit(count * 3) and .sample(count) here like in the other query?

The idea is when you have concurrent attempts to delete records, by choosing a random selection of the latest candidates, you reduce the overlap between them.

Whether that's a useful thing to do in practice is maybe debatable and this might be removed, but we should be consistent until then.

[brian-kephart]

Ah, I was assuming the extra records were to compensate for reducing the count with filter_map. I restored that behavior and added a comment and local variable to show the intent.

[djmb]

If we move the cache_full check before the max_age one, we can remove the cache_full check from the filter_map.

Something like:

  if cache_full
    uncached do
      order(:id).limit(count * 3).pluck(:id).sample(count)
    end
  elsif max_age
    uncached do
      order(:id)
        .limit(count * 3)
        .pluck(:id, :created_at)
        .filter_map { |id, created_at| id if created_at < min_created_at }
        .sample(count)
    end
  else
    []
  end

[brian-kephart]

I went ahead and did this, but went further to try and DRY it up and show the intent of the two branches. Let me know if it doesn't pass muster.

[djmb]

I don't think value omission is available in Ruby 3.0, so we'll need to keep specifying them (here and below)

[brian-kephart]

Apologies, I didn't intend to change those lines. I'll have a stern discussion with my linter settings.