apcu adapter

[juliangut]

memcached adapter will have to wait till this gets merged

[juliangut]

I think this PR is ready

[nikolaposa]

You should fix composer.json conflict.

I'll have more time to review PRs during the weekend.

[juliangut]

I didn't previously see the conflict. As mentioned above if this gets merged I can PR the memcached adapter and I'll be done with the original contribution

[Rainrider]

Please correct me if I'm wrong, but this basically doubles memory usage just to satisfy the constraints of the SilentRateLimiter interface, even if the user does not want to use it. Wouldn't it be better if Status::$resetAt has a possible null value when we can't get ttl values from the cache provider like it is the case with APCu?

[juliangut]

@Rainrider you are right, the amount of memory needed for this implementation, or any other in which remaining ttl is not returned by the provider, is more than doubled

There is another possible implementation in which you save both counter and timestamp in a single key: "counter::timestamp", but you loose atomic operations such as apcu_inc because you have to retrieve the value, explode, increment the counter and store it again, race conditions are more likely to occur

About not supporting remaining time to reset on an HTTP header I wouldn't consider it. If you have a peak to API rate limiting headers in the wild you'll see it is always there in a way or another (https://stackoverflow.com/questions/16022624/examples-of-http-api-rate-limiting-http-response-headers) there is even a IETF proposal for this headers (https://tools.ietf.org/id/draft-polli-ratelimit-headers-00.html), strange enough they are not standard

[nikolaposa]

I think @Rainrider brought up a good point. Why does default rate limiting implementation needs to be coupled with the logic around $timeKey? The very reason why interfaces are segregated is that the default implementation is intended for scenarios of having rate limit guard clauses in contexts that do not care about status information (i.e. remaining attempts), so it should have at less as possible communication with the back-end, APC in this case.

Try to focus on optimizing limit() and limitSilently() methods for their individual purposes, as if you are writing them in separate classes, without worrying about code reuse. It is completely ok that internally they have completely different flow of operation.

[nikolaposa]

Build is stuck on this one for 2 days... 🤔

[juliangut]

It's not stuck, if you take a look you'll see it passed, but github does not reflect it