Allow configured Cache in moka-store

[arcstur]

What do you think about allowing MokaStore to be created using a built moka Cache? I'm thinking of a new method implemented like this:

pub fn from_built_cache(cache: Cache<Id, Record>) -> Self {
    Self { cache }
}

I'm guessing maybe there are some configuration options for Cache that would not make MokaStore work correctly?

I'm talking about this because the new method receiving an Option felt awkward to me, because the new method of Cache takes a u64 directly. So I had to guess what None would do.

[maxcountryman]

I believe the original implementer mentioned it could probably be refactored to be more flexible.

I would probably recommend rethinking new altogether such that it fits moka better.

Also worth noting that moka provides cache eviction on a time basis, but that isn't implemented here yet (ideally it would be).

[arcstur]

I tried implementing the Cache eviction on a time basis, but couldn't. It seems Moka expects, in its Expiry trait, that the implementor calculates the duration per-entry from an std::time::Instant. To calculate a Duration, we would need to convert this Instant to an OffsetDateTime or vice-versa, which it seems is not possible. In theory we would need to do value.expiry_date - created_at.

One way out could be running OffsetDateTime::now() on the Expiry trait, but probably OffsetDateTime::now() != created_at:

impl Expiry<Id, Record> for RecordExpiry {
    fn expire_after_create(
        &self,
        _key: &Id,
        value: &Record,
        _created_at: std::time::Instant,
    ) -> Option<std::time::Duration> {
        let expiry_date = value.expiry_date;
        let now = OffsetDateTime::now_utc();

        Some(if expiry_date <= now {
            std::time::Duration::default()
        } else {
            (expiry_date - now).unsigned_abs()
        })
    }
}

This (and also implementing the after_update method, if the Record's expiry date changed) could let us remove the check on load for the MokaStore, so we would have to calculate OffsetDateTime::now on every create/save, instead of every load, but accepting the imprecision that current_time can differ from OffsetDateTime::now. How do you feel about this and what do you think?

[maxcountryman]

I think your proposed solution seems reasonable (it's certainly nicer to have the cache manage eviction for us).

[arcstur]

Nice, I'll work on it then.

[arcstur]

I finished my PR working on the cache eviction: #30.

Now, I think I would like to work on allowing more of the builder configuration. I think we could provide, for example, a default weigher that calculates the rough binary size of the Record (because probably that is what users would want). What do you think?

The other options don't seem that useful to expose to the user at first glance.

[arcstur]

Another thing I thought about is using a hash better-suited for i128. It seems ahash is pretty nice.

[maxcountryman]

I'm not too familiar with moka (someone had contributed this store when they were bundled with tower-sessions) so I'll trust your judgement.

At a high level making it so folks can provide arbitrary configuration of moka itself seems nice.