Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #44: Use PSR-20 ClockInterface instead of TimerInterface #47

Closed
wants to merge 7 commits into from

Conversation

samdark
Copy link
Member

@samdark samdark commented Jul 25, 2023

Q A
Is bugfix?
New feature?
Breaks BC? ✔️
Fixed issues #44

@samdark samdark requested review from vjik and xepozz July 25, 2023 14:13
@what-the-diff
Copy link

what-the-diff bot commented Jul 25, 2023

PR Summary:

  • Clock Interface Integration and Timer Interface Replacement
    Instead of having the system measure time intervals based on a self-created TimerInterface, this PR integrates a standardized ClockInterface (PSR-20) from the PHP Fig group, making it more reliable for time-related functionalities.

  • Addition of APCu as Counters Storage
    Additional support for APCu, a common caching system, was integrated into the software package with this PR, providing another storage method to keep track of counters.

  • Concurrent Requests Handling
    The software package now has improved capabilities to handle concurrent requests. Previously, it used a single save() method for data storage, but it's now split into saveIfNotExists() and saveCompareAndSwap() methods for better performance and reliability during concurrent requests.

  • General file modifications, additions and deletions

    • A new file, SystemClock.php, which uses ClockInterface, is added and the MicrotimeTimer.php and TimerInterface.php which were previous timing system components are removed.
    • A few test files (ApcuCounterTest.php, BaseCounterTest.php, CounterTest.php, MiddlewareTest.php, ApcuStorageTest.php) were updated to replace TimeTimer components with ClockInterface components, thus reflecting aforementioned system changes.
    • FrozenTimeTimer.php and FrozenClock.php were removed as they became obsolete with the integration of a more reliable time measurement tool, ClockInterface.
  • Minor Corrections and Updates
    The PR comes with minor amendments like a typo fix in README.md and an update (PSR-20 ClockInterface) in the dependencies listed in composer.json.

@codecov
Copy link

codecov bot commented Jul 25, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (b852afa) 100.00% compared to head (3d9c254) 100.00%.

Additional details and impacted files
@@             Coverage Diff             @@
##              master       #47   +/-   ##
===========================================
  Coverage     100.00%   100.00%           
  Complexity        51        51           
===========================================
  Files              9         9           
  Lines            135       135           
===========================================
  Hits             135       135           
Files Changed Coverage Δ
src/Counter.php 100.00% <100.00%> (ø)
src/Time/SystemClock.php 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -75,7 +75,7 @@ public function hit(string $id): CounterState
do {
// Last increment time.
// In GCRA it's known as arrival time.
$lastIncrementTimeInMilliseconds = $this->timer->nowInMilliseconds();
$lastIncrementTimeInMilliseconds = round((float)$this->timer->now()->format('U.u') * 1000);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$lastIncrementTimeInMilliseconds = round((float)$this->timer->now()->format('U.u') * 1000);
$lastIncrementTimeInMilliseconds = round($this->timer->now()->format('U.u') * 1000);

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would make static analysis complain.

@@ -227,7 +227,7 @@ public function testWithLimitingFunction(): void
*/
public function testWithExceedingMaxAttempts(): void
{
$timer = new FrozenTimeTimer();
$timer = new FrozenClock();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we can don't use specific timer and replace it to null.

@@ -259,7 +259,7 @@ public function testWithExceedingMaxAttempts(): void

public function testFailStoreUpdatedDataMiddleware(): void
{
$timer = new FrozenTimeTimer();
$timer = new FrozenClock();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we can don't use specific timer and replace it to null.

@@ -34,20 +34,20 @@ public function testGetKeyWithExistsKey(): void
{
$storage = $this->getStorage();

$want = (new FrozenTimeTimer())->nowInMilliseconds();
$want = round((float)(new DateTimeImmutable())->format('U.u') * 1000);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$want = round((float)(new DateTimeImmutable())->format('U.u') * 1000);
$want = round((new DateTimeImmutable())->format('U.u') * 1000);

}

public function testSaveIfNotExistsWithNewKey(): void
{
$storage = $this->getStorage();

$value = (new FrozenTimeTimer())->nowInMilliseconds();
$value = round((float)(new DateTimeImmutable())->format('U.u') * 1000);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$value = round((float)(new DateTimeImmutable())->format('U.u') * 1000);
$value = round((new DateTimeImmutable())->format('U.u') * 1000);

@@ -58,7 +58,7 @@ public function testSaveCompareAndSwapWithExistsKeyAndOldValueSame(): void
{
$storage = $this->getStorage();

$oldValue = (new FrozenTimeTimer())->nowInMilliseconds();
$oldValue = round((float)(new DateTimeImmutable())->format('U.u') * 1000);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$oldValue = round((float)(new DateTimeImmutable())->format('U.u') * 1000);
$oldValue = round((new DateTimeImmutable())->format('U.u') * 1000);

{
$storage = $this->getStorage();

$oldValue = (new FrozenTimeTimer())->nowInMilliseconds();
$oldValue = round((float)(new DateTimeImmutable())->format('U.u') * 1000);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$oldValue = round((float)(new DateTimeImmutable())->format('U.u') * 1000);
$oldValue = round((new DateTimeImmutable())->format('U.u') * 1000);

@@ -47,7 +47,7 @@ public function testSaveCompareAndSwapWithNewKey(): void
{
$storage = $this->getStorage();

$newValue = (new FrozenTimeTimer())->nowInMilliseconds();
$newValue = round((float)(new DateTimeImmutable())->format('U.u') * 1000);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$newValue = round((float)(new DateTimeImmutable())->format('U.u') * 1000);
$newValue = round((new DateTimeImmutable())->format('U.u') * 1000);

@@ -35,7 +35,7 @@ public function testSaveIfNotExistsWithExistsKey(): void
{
$storage = $this->getStorage();

$value = (new FrozenTimeTimer())->nowInMilliseconds();
$value = round((float)(new DateTimeImmutable())->format('U.u') * 1000);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$value = round((float)(new DateTimeImmutable())->format('U.u') * 1000);
$value = round((new DateTimeImmutable())->format('U.u') * 1000);

*/
public function testConcurrentHitsWithDirtyReading(): void
{
$timer = new FrozenTimeTimer();
$timer = new FrozenClock();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we can don't use specific timer and replace it to null.

@samdark samdark closed this Jul 26, 2023
@samdark samdark deleted the 44-use-psr-20 branch July 26, 2023 08:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants