Force refetching source if our cached copy will expire#44
Force refetching source if our cached copy will expire#44ghalse wants to merge 2 commits intosimplesamlphp:masterfrom
Conversation
If we set the `expiresAfter` option in metarefresh's config, we effectively alter the inbound metadata. Thus when we're conditionally GETting, we need to be aware that we've introduced a validUntil attribute that the remote server may not know about. That means we might need to force a refresh of the source even if the remote server does not think it has been modified.
ghalse
left a comment
ghalse
left a comment
There was a problem hiding this comment.
Because metarefresh is typically called periodically by cron, we can't force the refresh at exactly the expiry time. To avoid metadata expiring between calls, we need a bit of a buffer. The choice of one hour for the buffer is essentially arbitrary, but based on the example config setting 'cron' => ['hourly'].
| * so we may need to refresh the metadata even if the source indicates it has not changed. | ||
| */ | ||
| if (isset($source['expireAfter']) && isset($sourceState['requested_at'])) { | ||
| $requestedAt = (new DateTimeImmutable($sourceState['requested_at']))->getTimestamp(); |
There was a problem hiding this comment.
It's good to use DateTimeImmutable here, but converting it to a unix timestamp kind of destroys the benefits of having such an object. Would it be a better idea to use DateTimes and DateIntervals consistently in this piece of code and then compare the DateTimes (you can use <= on them)
I know I'm nitpicking here, but since it's a relatively small piece of code, I think it's worth the (your) trouble ;)
There was a problem hiding this comment.
I think the new push is what you had in mind?
2 participants
If we set the
expiresAfteroption in metarefresh's config, we effectively alter the inbound metadata. Thus when we're conditionally GETting, we need to be aware that we've introduced a validUntil attribute that the remote server may not know about. That means we might need to force a refresh of the source even if the remote server does not think it has been modified.