-
-
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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using floor() for quartile calculation can cause index out of bounds when the array is small. For example, with count=1, floor(10.25)=0 is valid, but with count=2, floor(20.75)=1 accesses a valid index, but the logic doesn't handle edge cases consistently.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback
Comment on lines
+144
to
+145
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||||||||||||||||||||||
$iqr = $q3 - $q1; | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
$lower_bound = $q1 - 1.5 * $iqr; | ||||||||||||||||||||||||||
$upper_bound = $q3 + 1.5 * $iqr; | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
$result = array_filter($this->intervals, function($value) use ($lower_bound, $upper_bound) { | ||||||||||||||||||||||||||
return $value >= $lower_bound && $value <= $upper_bound; | ||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
$total = array_sum($result); | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
return count($this->intervals) ? intval(floor($total / count($this->intervals))) : 0; | ||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
|
@@ -136,9 +163,27 @@ public function getAverageInterval(): int | |||||||||||||||||||||||||
public function getMedianInterval(): int | ||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||
sort($this->intervals); | ||||||||||||||||||||||||||
$num = floor(count($this->intervals) / 2); | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
return isset($this->intervals[$num]) ? $this->intervals[$num] : 0; | ||||||||||||||||||||||||||
$count = count($this->intervals); | ||||||||||||||||||||||||||
if ($count === 0) { | ||||||||||||||||||||||||||
return 0; | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
$num = floor($count / 2); | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
if ($count % 2 === 0) { | ||||||||||||||||||||||||||
return intval(floor(($this->intervals[$num - 1] + $this->intervals[$num]) / 2)); | ||||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||||
return $this->intervals[$num]; | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||
* @return int | ||||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||||
public function getNewestItemDate(): int | ||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||
return $this->newestItemDate; | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
private function computeIntervals(array $dates): array | ||||||||||||||||||||||||||
|
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"