Use Psr16 cache as Default, deprecate \SimplePie\Cache\Base implementations #752

Art4 · 2022-10-14T10:48:58Z

Hi all 👋

This is a follow-up PR of #742. I propose this PR for SimplePie 1.8, see #731.

In this PR I've changed the internal cache usage to a more PSR-16 style, see the new interface SimplePie\Cache\DataCache. This interface could be removed in SimplePie 2.0 and the provided PSR-16 implementation can be used directly. This will bring us more advantages like \Psr\SimpleCache\CacheInterface::setMultiple().

For BC reasons I've created a simple adapter class SimplePie\Cache\BaseDataCache for the current SimplePie\Cache\Base implementations. This allows us to use the PSR-16 implementation from #742, but also support the old cache implementations. Furthermore I deprecated all cache implementations in favor of the PSR-16 cache.

This will

fix The file_put_contents() is not atomic and can cause race in file caching #740,
fix [BUG] Redis options not set - default to expire 0 #616
and make obsolete Fix Redis expiry (#616) #695.

src/Cache/DataCache.php

because we allow to inject the PSR-16 cache and also a cache_name_function, the user should be notified about bad cache config

jtojnar · 2022-10-26T13:48:36Z

Might be nice to rebase this onto master so it can be reviewed commit by commit more easily.

Art4 · 2022-10-27T06:52:08Z

Sorry, I tried to rebase but I failed. I don't have much experience with rebase (maybe because I prefer merge) and the conflicts were to complicated for me.

You can start reviewing by commit 07ff3cb. Older commits are already merged with #742.

jtojnar · 2022-10-26T13:57:19Z

CHANGELOG.md

+
+- The method `SimplePie\SimplePie::set_cache_location()` is deprecated, use `SimplePie\SimplePie::set_cache()` instead
+- The method `SimplePie\SimplePie::force_cache_fallback()` is deprecated, expired cache will not be used anymore
+- The class `SimplePie\Cache` is deprecated, use implementation of `SimplePie\SimplePie::set_cache()` instead


Should this say Psr\SimpleCache\CacheInterface?

I thought while using SimplePie\Cache will not only allow you to get a cache implementation but also to register a cache implementation the reference to SimplePie\SimplePie::set_cache() makes that clearer.

jtojnar · 2022-10-26T14:04:35Z

src/Cache/BaseDataCache.php

@@ -0,0 +1,157 @@
+<?php
+/**
+ * SimplePie


A bit of topic but I wonder if it might make sense to replace blurbs with SPDX identifiers, see

https://reuse.software/tutorial/

https://www.linuxfoundation.org/blog/blog/solving-license-compliance-at-the-source-adding-spdx-license-ids

Never heard of reuse.software before, but it sounds good.

jtojnar · 2022-10-27T12:02:16Z

src/Cache/BaseDataCache.php

+            return $default;
+        }
+
+        // ingore data if internal cache expiration time is not set


fixed in #761

jtojnar · 2022-10-27T12:08:58Z

src/Cache/DataCache.php

+use InvalidArgumentException;
+
+/**
+ * Simplified PSR-16 Cache client for caching data arrays


Simplifies how?

It uses only a subset of PSR-16 (only get(), set() and delete(), but not has(), getMultiple(), setMultiple() or deleteMultiple()),

Might be nice to mention it explicitly in the comment.

fixed in #761

@jtojnar

see comment from @jtojnar in simplepie#752

@jtojnar

see comment from @jtojnar in simplepie#752

@jtojnar

* bump version to 1.8.0 * Update CHANGELOG.md * Fix version tags in deprecated messages * fix version in old deprecation messages * Fix typo see comment from @jtojnar in #752 * Add comment for DataCache interface see comment from @jtojnar in #752 * Update CHANGELOG.md for #760, #764 and #765 * Update CHANGELOG.md for #762, #767 and #763 * Update CHANGELOG.md for #768 and #770 * Update release date * Update CHANGELOG.md for #769 and #771 * Update CHANGELOG.md for #766

Art4 and others added 30 commits August 29, 2022 10:47

Create wrapper for PSR-16 cache

078078f

Add method set_psr16_cache()

3aca282

Add cache ttl, cache raw data in PSR-16 cache

3320510

Add tests for save() and load()

f73bc33

improve mtime() for PSR-16 cache

783cddb

Fix tests for PHP < 8.0

b63012e

Fix tests for PHP 5.6

fd9f6bc

Fix tests for PHP 5.6

486178c

Improve tests

ff26e20

Merge branch 'master' into allow-psr-16-cache

e139e91

Rename method for setting cache

c43618b

Update CHANGELOG.md

28fc661

Fix typo

6d333f1

make cache miss more comprehensible

7dfd76c

rename $set to $unset

be3702e

ensure $data for save() has the correct data type

ec855a9

mark Psr16 cache as final and internal

bedd273

Replace spc and spi with Cache Base constants

ba1b7ac

Improve descriptive names of test cases

e9c230f

Change variable name

411932e

Mark Psr16 class as internal

fea5476

Create new DataCache interface

07ff3cb

Implement DataCache in Psr16 cache class

1d49233

create adapter for Base cache implementations

58de82d

Rename SimplePie\SimplePie::$cache to $enable_cache

195d9c2

Create integration tests to safeguard the bahavior of the cache

2627d31

refactore integration tests

9e3150c

Provide also default cache data in integration test

bddb870

Test cache with outdated build and cache collision

463c2f5

test stored non feed url with valid autodiscovery cache

fc4766c

pull-request-size bot added the size/XXL label Oct 14, 2022

Art4 marked this pull request as draft October 14, 2022 10:49

Art4 mentioned this pull request Oct 14, 2022

Possible roadmap #731

Open

48 tasks

Fix code style

114e1c9

Art4 mentioned this pull request Oct 14, 2022

Allow usage of PSR-16 cache implementation #742

Merged

Art4 commented Oct 18, 2022

View reviewed changes

src/Cache/DataCache.php Outdated Show resolved Hide resolved

jtojnar reviewed Oct 18, 2022

View reviewed changes

src/Cache/DataCache.php Show resolved Hide resolved

Art4 added 8 commits October 20, 2022 21:07

rename DataCache methods to snake_case

5e1d008

define DataCache interface as internal

2c85bfd

rename intergration tests for caching

2763ca3

Merge branch 'master' into use-psr16-cache-internally

b436d89

Merge branch 'master' into use-psr16-cache-internally

b423b7f

Fix Memcached cache class

af3dfb4

do not catch CacheExceptions

39d7ed3

because we allow to inject the PSR-16 cache and also a cache_name_function, the user should be notified about bad cache config

Add deprecation notes

19e167c

Art4 marked this pull request as ready for review October 24, 2022 13:31

Art4 added 2 commits October 24, 2022 15:38

Deprecate SimplePie::force_cache_fallback()

5e887d3

Update CHANGELOG.md

9e8aa24

mblaney merged commit bb38d74 into simplepie:master Oct 30, 2022

Art4 deleted the use-psr16-cache-internally branch October 30, 2022 05:24

Art4 mentioned this pull request Oct 30, 2022

Type declarations in cache #759

Merged

jtojnar reviewed Oct 31, 2022

View reviewed changes

Art4 added a commit to Art4/simplepie that referenced this pull request Nov 4, 2022

Fix typo

94609ea

see comment from @jtojnar in simplepie#752

Art4 added a commit to Art4/simplepie that referenced this pull request Nov 4, 2022

Add comment for DataCache interface

d65fce5

see comment from @jtojnar in simplepie#752

jtojnar mentioned this pull request Jan 28, 2023

Use SPDX comments #778

Closed

bart-v mentioned this pull request Aug 18, 2023

[BUG] Cache constantly updating #830

Open

Art4 mentioned this pull request Aug 28, 2023

Stop legacy file cache constantly updating #846

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use Psr16 cache as Default, deprecate \SimplePie\Cache\Base implementations #752

Use Psr16 cache as Default, deprecate \SimplePie\Cache\Base implementations #752

Art4 commented Oct 14, 2022 •

edited

jtojnar commented Oct 26, 2022

Art4 commented Oct 27, 2022 •

edited

jtojnar Oct 26, 2022

Art4 Nov 3, 2022

jtojnar Oct 26, 2022

Art4 Nov 3, 2022

jtojnar Oct 27, 2022

Art4 Nov 4, 2022

jtojnar Oct 27, 2022

Art4 Nov 3, 2022

jtojnar Nov 3, 2022

Art4 Nov 4, 2022

Use Psr16 cache as Default, deprecate \SimplePie\Cache\Base implementations #752

Use Psr16 cache as Default, deprecate \SimplePie\Cache\Base implementations #752

Conversation

Art4 commented Oct 14, 2022 • edited

jtojnar commented Oct 26, 2022

Art4 commented Oct 27, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Art4 commented Oct 14, 2022 •

edited

Art4 commented Oct 27, 2022 •

edited