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

[BUG] Cache constantly updating #830

Open
Sir-Will opened this issue Apr 18, 2023 · 11 comments · May be fixed by #846
Open

[BUG] Cache constantly updating #830

Sir-Will opened this issue Apr 18, 2023 · 11 comments · May be fixed by #846
Assignees
Labels

Comments

@Sir-Will
Copy link

Description

It doesn't seem like the cache is working correctly for me. Every time I reload the page, I see the timestamps of the cache files update.
This shouldn't be the case, or?

Steps to Reproduce

        $infoFeed = new SimplePie();
        $infoFeed->set_feed_url($infoFeeds);
        $infoFeed->set_cache_duration(600);
        $infoFeed->init();
        $infoFeed->handle_content_type();

Expected Behavior

Cache not updating every run.

Actual Behavior

Cache is updating on every run.

Possible Solutions

If you have already ideas how to solve the issue, add them here. (remove this section if not needed)

Environment

PHP 8.1
SimplePie 1.8.0

@bart-v
Copy link

bart-v commented Aug 17, 2023

Same problem here.

This is caused by these 4 occurrences of cache_expiration_time
https://github.com/simplepie/simplepie/blob/master/src/SimplePie.php#L1676
https://github.com/simplepie/simplepie/blob/master/src/SimplePie.php#L1769
https://github.com/simplepie/simplepie/blob/master/src/SimplePie.php#L1920

cache_expiration_time => should be => __cache_expiration_time

Or at least the array index should be consistent everywhere...

@Art4
Copy link
Contributor

Art4 commented Aug 18, 2023

The default file cache is deprecated an will be removed in the next major release. Instead you should use a PSR-16 cache implementation like Symfony Cache or every other implementation.

$simplepie = new \SimplePie\SimplePie();
$simplepie->set_cache(
    new \Symfony\Component\Cache\Psr16Cache(
        new \Symfony\Component\Cache\Adapter\FilesystemAdapter()
    ),
);

@bart-v
Copy link

bart-v commented Aug 18, 2023

Then, a simple example on how to then set the cache type (i.e. file), location and duration would be nice.
The current documentation only mentions a deprecated example and does not say anything about PSR-16 ...

@bart-v
Copy link

bart-v commented Aug 18, 2023

At least it's clear that the PSR-16 changes in #752 broke the legacy cache.
I think reverting 3ba7c91 will fix this bug
Would you consider this, please?

@Art4
Copy link
Contributor

Art4 commented Aug 25, 2023

Then, a simple example on how to then set the cache type (i.e. file), location and duration would be nice. The current documentation only mentions a deprecated example and does not say anything about PSR-16 ...

The code example is from #777. Setting cache type and location is part of the PSR-16 implementation. The cache duration is still configured using $simplepie->set_cache_duration($seconds);.

@bart-v
Copy link

bart-v commented Aug 25, 2023

I still don't see an example that one can easily copy/past and just fill in 2 or 3 parameters.
Not everyone can read the whole PSR-16 spec to write a few lines of code...

@Art4
Copy link
Contributor

Art4 commented Aug 25, 2023

I still don't see an example that one can easily copy/past and just fill in 2 or 3 parameters.

Please take a look again at my comment above. It already shows you a working example using Symfony Cache using the FilesystemAdapter. But you are free to choose another adapter or even every other implementation.

Not everyone can read the whole PSR-16 spec to write a few lines of code...

You don't have to read the PSR-16 spec. You have to look at the more than 100 implementations (and the PSR-16 spec allows you to try and use them all). Choose the one that works best for you. No one knows your project and use case better than you do.

@bart-v
Copy link

bart-v commented Aug 26, 2023

The example above does not show how to define the (custom) cache file location.
Which PSR-16 is really not so important, so any will do, that is why people needs a simple example

@Art4
Copy link
Contributor

Art4 commented Aug 26, 2023

The example above does not show how to define the (custom) cache file location. Which PSR-16 is really not so important, so any will do, that is why people needs a simple example

The file location in the FilesytemAdapter is not required, it uses sys_get_temp_dir() by default. If needed please read the docs for more customization.

I would like to avoid more complex examples for a specific library. Mentioning the Symfony cache implementation alone is already very opinionated. Using the filesystem adapter as cache type is also opinionated. If you would like to have a more customized example for this library in the docs you are welcome to open a PR.

@Art4 Art4 linked a pull request Aug 28, 2023 that will close this issue
@Art4
Copy link
Contributor

Art4 commented Aug 28, 2023

I've found the reason for this issue and opened #846.

@bart-v Could you please confirm that this PR will fix your issue?

@bart-v
Copy link

bart-v commented Aug 28, 2023

Looks good, please go ahead

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants