-
-
Notifications
You must be signed in to change notification settings - Fork 1
Make nextupdatetime more reliable #14
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
base: main
Are you sure you want to change the base?
Changes from all commits
0ba48be
4abf7cb
dbfd4d8
e7ebe12
cf04e30
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -31,14 +31,22 @@ class UpdateStats | |||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| protected array $intervals = []; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| protected int $newestItemDate = 0; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||
| * UpdateStats constructor. | ||||||||||||||||||||||||||
| * @param FeedInterface $feed | ||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||
| public function __construct( | ||||||||||||||||||||||||||
| protected FeedInterface $feed | ||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||
| $this->intervals = $this->computeIntervals($this->extractDates($feed)); | ||||||||||||||||||||||||||
| $dates = $this->extractDates($feed); | ||||||||||||||||||||||||||
| if (count($dates) > 0) { | ||||||||||||||||||||||||||
| $this->newestItemDate = min(max($dates), time()); | ||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||
| $this->newestItemDate = $this->getFeedTimestamp(); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| $this->intervals = $this->computeIntervals($dates); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||
|
|
@@ -57,7 +65,6 @@ public function computeNextUpdate( | |||||||||||||||||||||||||
| if ($this->isSleepy($sleepyDuration, $marginRatio)) { | ||||||||||||||||||||||||||
| return (new \DateTime())->setTimestamp(time() + $sleepyDelay); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| $feedTimeStamp = $this->getFeedTimestamp(); | ||||||||||||||||||||||||||
| $now = time(); | ||||||||||||||||||||||||||
| $intervals = [ | ||||||||||||||||||||||||||
| $this->getAverageInterval(), | ||||||||||||||||||||||||||
|
|
@@ -66,7 +73,7 @@ public function computeNextUpdate( | |||||||||||||||||||||||||
| sort($intervals); | ||||||||||||||||||||||||||
| $newTimestamp = $now + $minDelay; | ||||||||||||||||||||||||||
| foreach ($intervals as $interval) { | ||||||||||||||||||||||||||
| $computedTimestamp = $this->addInterval($feedTimeStamp, $interval, $marginRatio); | ||||||||||||||||||||||||||
| $computedTimestamp = $this->addInterval($this->newestItemDate, $interval, $marginRatio); | ||||||||||||||||||||||||||
| if ($computedTimestamp > $now) { | ||||||||||||||||||||||||||
| $newTimestamp = $computedTimestamp; | ||||||||||||||||||||||||||
| break; | ||||||||||||||||||||||||||
|
|
@@ -82,7 +89,7 @@ public function computeNextUpdate( | |||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||
| public function isSleepy(int $sleepyDuration, float $marginRatio): bool | ||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||
| return time() > $this->addInterval($this->getFeedTimestamp(), $sleepyDuration, $marginRatio); | ||||||||||||||||||||||||||
| return time() > $this->addInterval($this->newestItemDate, $sleepyDuration, $marginRatio); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||
|
|
@@ -125,7 +132,27 @@ public function getMaxInterval(): int | |||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||
| public function getAverageInterval(): int | ||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||
| $total = array_sum($this->intervals); | ||||||||||||||||||||||||||
| sort($this->intervals); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| $count = count($this->intervals); | ||||||||||||||||||||||||||
| if ($count === 0) { | ||||||||||||||||||||||||||
| return 0; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // some feeds could have very old historic | ||||||||||||||||||||||||||
| // articles so eliminate them with statistic | ||||||||||||||||||||||||||
| $q1 = $this->intervals[floor($count * 0.25)]; | ||||||||||||||||||||||||||
| $q3 = $this->intervals[floor($count * 0.75)]; | ||||||||||||||||||||||||||
|
Comment on lines
+144
to
+145
|
||||||||||||||||||||||||||
| $q1 = $this->intervals[floor($count * 0.25)]; | |
| $q3 = $this->intervals[floor($count * 0.75)]; | |
| $q1_index = max(0, intdiv($count - 1, 4)); // Calculate Q1 index | |
| $q3_index = min($count - 1, intdiv(3 * ($count - 1), 4)); // Calculate Q3 index | |
| $q1 = $this->intervals[$q1_index]; | |
| $q3 = $this->intervals[$q3_index]; |
Copilot uses AI. Check for mistakes.
Copilot
AI
Jul 23, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same quartile calculation issue as Q1 - using floor() without bounds checking can lead to incorrect quartile values or potential array access issues in edge cases.
| $q1 = $this->intervals[floor($count * 0.25)]; | |
| $q3 = $this->intervals[floor($count * 0.75)]; | |
| $q1_index = max(0, min($count - 1, intval(floor($count * 0.25)))); | |
| $q3_index = max(0, min($count - 1, intval(floor($count * 0.75)))); | |
| $q1 = $this->intervals[$q1_index]; | |
| $q3 = $this->intervals[$q3_index]; |
Copilot uses AI. Check for mistakes.
Copilot
AI
Jul 23, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The average calculation uses count($this->intervals) as divisor but $total is calculated from the filtered $result array. This will produce incorrect averages since the divisor should be count($result) to match the filtered data.
| return count($this->intervals) ? intval(floor($total / count($this->intervals))) : 0; | |
| return count($result) ? intval(floor($total / count($result))) : 0; |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The expression min(max($dates), time()) is unclear. Consider adding a comment explaining why the newest date is capped at the current time, or extract this logic to a descriptively named method.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
possible comment here could be "get the most recent item date that is not in the future"