Switch default cache config to make use of laminas-cache, when doctrine/cache ^2.0 is installed

[driehle]

The library doctrine/cache is deprecated and no longer actively developed, see #786 and doctrine/cache. Hence, we should replace the caches provided by this module with implementation of laminas/laminas-cache, see docs.

This PR enables the installation of doctrine/cache: ^2.0, which is just a set of interfaces and no actual implementations anymore. If users have doctrine/cache: ^1.0 installed, there will be no changes in behaviour. Hence, I do not consider this PR as a BC break!

If users opt it to install doctrine/cache: ^2.0, the following changes will occur on the caches provided:

Cache Key	Old Instance	New Instance
doctrine.cache.apc	Doctrine\Common\Cache\ApcCache	No replacement
doctrine.cache.apcu	Doctrine\Common\Cache\ApcuCache	Laminas\Cache\Storage\Adapter\Apcu (wrapped)
doctrine.cache.array	Doctrine\Common\Cache\ArrayCache	Laminas\Cache\Storage\Adapter\Memory (wrapped)
doctrine.cache.filesystem	Doctrine\Common\Cache\FilesystemCache	Laminas\Cache\Storage\Adapter\Filesystem (wrapped)
doctrine.cache.memcache	Doctrine\Common\Cache\MemcacheCache	No replacement
doctrine.cache.memcached	Doctrine\Common\Cache\MemcachedCache	Laminas\Cache\Storage\Adapter\Memcached (wrapped)
doctrine.cache.predis	Doctrine\Common\Cache\PredisCache	No replacement
doctrine.cache.redis	Doctrine\Common\Cache\RedisCache	Laminas\Cache\Storage\Adapter\Redis (wrapped)
doctrine.cache.wincache	Doctrine\Common\Cache\WinCacheCache	No replacement
doctrine.cache.xcache	Doctrine\Common\Cache\XcacheCache	No replacement
doctrine.cache.zenddata	Doctrine\Common\Cache\ZendDataCache	No replacement

Caches not supported by laminas-cache have been dropped without replacement. Caches, where a Laminas implementation is available, are instantiated and wrapped in an instance of DoctrineModule\Cache\LaminasStorageCache. This effectively decorates the laminas cache instances as doctrine cache and ensures backward compatibility.

Users who want to work with the laminas instances directly, i.e. not the wrapped object, can access these objects as follows:

• doctrinemodule.cache.apcu
• doctrinemodule.cache.array
• doctrinemodule.cache.filesystem
• doctrinemodule.cache.memcached
• doctrinemodule.cache.redis

In the release notes of 5.3.0 we will have to state that users who want to continue using doctrine/cache must ensure they have doctrine/cache: ^1.0 in their composer.json. This should, however, be best practice anyways, if they are using it in their application. Again, this is another argument why I do not consider this PR as a BC break.

Moreover, the documentation needs to be updated. I will provide this as a separate PR, once this one is merged.

[driehle]

@fezfez @snapshotpl @Ocramius

I'd be happy if you could provide some feedback to this proposed PR! :-)

[Ocramius]

I'm OK with the direction, but this feels wrong for a minor release: a new major is more appropriate, given that this can drag in many unexpected BC breaks, which seems to be highlighted by many test changes.

[Ocramius]

This moving out made it a bit hard to figure out what's going on.

[driehle]

Yes, but we need this extraction for usage in Module.php, where the laminas cache config is exposed to the service manager. In the next major release, we will drop doctrine/cache completely and so will the method getDoctrineCacheConfig be removed.

[Ocramius]

Type should probably be array<non-empty-string, array{adapter: non-empty-string, options?: mixed[]}>

[driehle]

Added more specific types for all configurations with 2c915c6

[driehle]

@Ocramius

[...] but this feels wrong for a minor release: a new major is more appropriate, given that this can drag in many unexpected BC breaks, which seems to be highlighted by many test changes

I can understand that this feels a bit wrong, but there are no signature changes related to DoctrineModule. The change is the major upgrade from doctrine/cache 1 to 2. If we'd go for a major release for each of a dependency's new major, we'd be releaseing new majors each week which - from my perspective - wouldn't be helpful either.

Since simply sticking with "doctrine/cache": "^1.0" won't cause any change for the user, I'd still like to release this as a minor. This will allow people to choose when they are willing to switch to laminas-cache. When we release the next major, i.e. DoctrineModule 6, people will be forced to move to laminas-cache, because doctrine/cache will then be fully removed.

[Ocramius]

there are no signature changes related to DoctrineModule.

The breakages are mostly in the types of services produced: such is the nature of integration work when building integration/glue packages :-\

If we'd go for a major release for each of a dependency's new major, we'd be releaseing new majors each week which - from my perspective - wouldn't be helpful either.

Inherited BC breaks are still BC breaks.

I don't have a full overview of breakages here, but what I want to avoid here is that a composer update with a minor upgrade of only this package in composer.json leads to the consumer receiving a completely incompatible cache instance:

• if you say that won't happen, then this is fine for a minor release.
• if you are not 100% sure that this won't happen, then this can go to a minor release
• if the returned types are still compatible after removal of ^1 support, it is even feasible to drop ^1 support in a subsequent minor release too

[driehle]

The breakages are mostly in the types of services produced: such is the nature of integration work when building integration/glue packages :-\

Yes, but there are two cases to be considered here:

Case 1: The user has doctrine/cache ^1 required. If that's the case, then there are no changes in the services produced, hence, no breaking changes.

Case 2: The user does not have doctrine/cache required at all (which IMHO is their fault). Then, this change will upgrade from doctrine/cache ^1 to ^2. This bears two risks:

1. Our CacheFactory promises the return of an instance of Doctrine\Common\Cache\Cache. That promise won't be broken, but instead of Doctrine\Common\Cache\ArrayCache it will return an DoctrineModule\Cache\LaminasStorageCache (which wraps Laminas\Cache\Storage\Adapter\Memory). This may result in breaks if the user for some reason explicitly checks for instanceof ArrayCache::class.
2. The caches doctrine.cache.apc, doctrine.cache.memcache (note: memcache, not memcacheD!), doctrine.cache.predis, doctrine.cache.windata, doctrine.cache.xcache and doctrine.cache.zenddata aren't available anymore.
   • apc: The PHP extension apc has been replaced by apcu sind PHP 7.0
   • memcache: The PHP extension memcache has been replaced by memcached sind PHP 7.0
   • predis: Currently not supported, could be added by a wrapper/decorator class
   • windata: The PHP extension wincache is not available for PHP 7
   • xcache: The PHP extension xcache is not available for PHP 7
   • zenddata: Zend Server 2021 seems to support only PHP <= 7.4

To me, these risks seem to be considerably low, given that they can be mitigated by simply added doctrine/cache: ^1.0 to the project's composer.json. The caches that are not available anymore with doctrine/cache ^2 cannot be used with recent PHP versions anyways, except for Predis. We can add a wrapper/decorator for Predis, to circumvent that.

if the returned types are still compatible after removal of ^1 support, it is even feasible to drop ^1 support in a subsequent minor release too

Yes, that'd be the ultimate goal. :-)

[fezfez]

I agree with @Ocramius I think it will break a lot of applications (including mine 😵‍💫) and it should be reserved for a new major release

[driehle]

@fezfez

I agree with Ocramius I think it will break a lot of applications (including mine 😵‍💫) and it should be reserved for a new major release

Do you see a break with doctrine/cache ^1.0, or do you not want to add that constraint to your application? 🤔

[rogervila]

As @driehle explains in the PR description, as long as a user defines doctrine/cache ^1.0, there is no break at all.

I suggest adding a Warning on the readme so, if a user does not define it and finds a break, the solution can be easily found.

Additionally, doctrine/cache ^2.0 could be added as a suggestion on the composer.json file.

[rogervila]

Since this PR needs further discussion, I propose to release a 5.3.0 version with PHP 8.2 Support and later decide if this PR should be released as 5.4.0 or 6.0.

What do you think @driehle @Ocramius @fezfez?

[Ocramius]

BTW, I don't have anything against this patch: just saying that it carries substantial risk baggage, and in the interest of downstream consumers, it would be best to mark it as new major, to prevent confusion and make the risk visible to downstream.

Can then bump again afterwards 👍

[rogervila]

Perfect.

Then, @driehle, could you please release 5.3.0 with PHP 8.2 Support?

Let me know if there is something else I can do from my side.

[driehle]

5.3.0 is now released, still I think this PR should go on a minor. Targeting at 5.4.0. If people really experience breaks with this change, I'd be happy to hear about them in more detail.

[Ocramius]

@driehle your call on that: besides the risk of BC, this patch is OK :)

[demiankatz]

Hello, everyone! I'm trying to get this project compatible with annotations v2 (see #801) and I've run into a problem with caching. The Doctrine\Common\Annotations\CachedReader is removed in v2, replaced by a PsrCachedReader. The PsrCachedReader expects an instance of Psr\Cache\CacheItemPoolInterface, but DoctrineModule\Cache\LaminasStorageCache does not implement that interface (and indeed, cannot easily implement it, due to a conflicting definition of the save method). I'm wondering if this is a problem worth trying to sort out as part of the work here, so we can get ahead of further compatibility problems down the road.

Any thoughts? And I apologize if I've gotten any details wrong -- I'm a newbie to Doctrine, but looking to migrate from Laminas\Db to Doctrine in an application I work on, and I've fallen down this update-related rabbit hole. :-)

[TomHAnderson]

@demiankatz Using the PSR standard is a step forward for this module. May I strongly recommend you don't use annotations at all, ever, and instead use XML metadata? That may sound daunting but if you'll investigate https://skipper18.com you'll find a tool that can convert from annotations to ERD to XML.

[demiankatz]

@TomHAnderson, thanks for the advice; I will look into the XML metadata option! But I'm not trying to update annotations here because I'm determined to use them, but simply because I'm trying to resolve a dependency conflict between DoctrineModule and php-cs-fixer that's preventing my composer install from working. :-)

[TomHAnderson]

This module uses phpcs by squizlabs. It is odd your php-cs-fixer is conflicting though. Not to discount @fabpot work but a switch may be in order?

[demiankatz]

@TomHAnderson, my project uses both php-cs-fixer and phpcs; I find the tools to be complementary. In any case, php-cs-fixer has a dependency on doctrine/annotations for one of its advanced features, and it raised that to require version 2, which is incompatible with the requirements of this module, which thus creates a dependency headache for any project that tries to use both things. :-)

[TomHAnderson]

@demiankatz Thanks for the explanation. That clears it up for me. I see two paths for you:

1. Use an older version of php-cs-fixer. That's only 1 version back of course: https://github.com/PHP-CS-Fixer/PHP-CS-Fixer/releases/tag/v3.14.3
2. Create a PR for this module and we'll release a major version that increments the annotations dependency.

[demiankatz]

Thanks, @TomHAnderson. As a stopgap measure, I'm just deleting the php-cs-fixer dependency from the branch of my repository that incorporates Doctrine... and I can add it back in when we get everything fixed. I think the most important point is that Doctrine is moving forward in various ways and we need to make sure this module can catch up with it.

I already have a PR in progress to update the annotations dependency (#801), but I believe it is contingent on getting this PR sorted out, and also adding PSR support. So that's the knot we need to untangle at this point! I'm not entirely sure where this PR stands, and I'm also not sure how much work it would take to add PSR cache support -- e.g. whether that's an additional step that should be done here, or a subsequent PR after this one is merged. As a total newcomer to this project (and with limited time, since I already have full-time responsibilities to other projects), I'm probably not the best person to figure it all out, but I'm certainly happy to contribute as much as time will permit. :-)