Removed wave issue in Reset-At by going to millis

[nikolaposa]

This change conflicts with https://github.com/nikolaposa/rate-limit/pull/11, so please sort that first, and then I'll review it. At first glance I don't like the fact that ttl(string $key) method signature is changed - return type is now float. Even though it's a internal method, it's still a BC break because it's not private but protected, so someone might override it in their custom implementation.

[poursal]

Hello Nikola,

I have fixed the conflict. Without going to millis (either as int or as float) the waving issue will continue to exist, unless you stop using the expire capabilities of redis.

[nikolaposa]

@poursal Thanks for the clarification. So what should be the ultimate solution in your opinion? Can we somehow stick with protected function ttl(string $key) : int method signature (integer) while still tackling waving issue?

[poursal]

@nikolaposa We can document that ttl will return millis (instead of seconds) and have getResetAt perform the ceil to seconds. But this also counts as BC break right? If not this is an easy fix.

On the other hand if we want ttl to return seconds we can change the functionality of getResetAt and just return the expiration time:

from: public function getResetAt(string $key) : int { return time() + $this->ttl($key); }

to: public function getResetAt(string $key) : int { return $this->ttl($key); }

and have ttl directly compute the expiration time in the future.

[nikolaposa]

@poursal I like the first approach more. However the current one is self-documenting and explicit, ttl() method returns float, so the intent is pretty obvious. Second alternative is not only a huge BC break, but also a change in semantics of the getResetAt() method that is supposed to tell in which point in time key will be reset (timestamp) and not how much time has left.

So I would definitely go with either current solution or first alternative. Will think about it...

[nikolaposa]

Closing PR due to staleness. I did a complete rewrite for v2.0, much of the classes affected by these changes do not exist anymore.