hasExpired() should use self::getTimeNow() #993
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #993 +/- ##
===========================================
Coverage 100.00% 100.00%
- Complexity 191 193 +2
===========================================
Files 20 20
Lines 516 521 +5
===========================================
+ Hits 516 521 +5
|
| @@ -196,7 +196,7 @@ public function hasExpired() | |||
| throw new RuntimeException('"expires" is not set on the token'); | |||
| } | |||
|
|
|||
| return $expires < time(); | |||
| return $expires < self::getTimeNow(); | |||
There was a problem hiding this comment.
Should this be $this->getTimeNow(), since getTimeNow() is an instance method and not a static method?
There was a problem hiding this comment.
Oh, I missed that—probably because the accompanying setTimeNow() and resetTimeNow() are static methods.
Maybe getTimeNow() should be a static method too instead? It does not reference any instance-specific properties, only self::timeNow, or time() if self::timeNow is not set, so I can't see a good reason to have it as an instance method.
I have done that in my local development repo, and all tests are still passing. Will commit if I get a thumb up.
|
Merged to the Thank you for contributing! 🎉 |
While trying to test some code that caches an AccessToken and refreshes it if it has expired, I discovered that AccessToken->hasExpired() uses time() instead of self::getTimeNow(), and thus ignores self::$timeNow when testing.
This pull request fixes that.