From 0403834daf0992da3857fb0cf316df36462260ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= <1005065+DeepDiver1975@users.noreply.github.com> Date: Tue, 28 Jul 2026 15:57:39 +0200 Subject: [PATCH] feat(cache): expose createLocal() on ICacheFactory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit OC\Memcache\Factory has offered a host local cache tier since the class was written, but OCP\ICacheFactory only ever declared create() - which is an alias for the distributed tier. Core therefore had to reach for the local tier through method_exists() and a phan suppression, and apps could not use it at all. Declaring createLocal() on the interface removes the guard from LocalCacheFactory and the last of those TODOs from AppManager. The NullCache fallback stays: an installation with no memcache.local configured still gets a request scoped cache rather than no caching. This is a breaking change for a class outside core implementing OCP\ICacheFactory, which is why it lands before 11.0.0 is out rather than after. Co-Authored-By: Claude Opus 5 Signed-off-by: Thomas Müller <1005065+DeepDiver1975@users.noreply.github.com> --- changelog/unreleased/41753 | 13 ++++++++++++ lib/private/App/AppManager.php | 14 ++++--------- lib/private/Memcache/LocalCacheFactory.php | 11 +++------- lib/public/ICacheFactory.php | 17 ++++++++++++++++ tests/lib/Memcache/LocalCacheFactoryTest.php | 21 +++++++++++--------- 5 files changed, 49 insertions(+), 27 deletions(-) create mode 100644 changelog/unreleased/41753 diff --git a/changelog/unreleased/41753 b/changelog/unreleased/41753 new file mode 100644 index 000000000000..bd29bbc719ac --- /dev/null +++ b/changelog/unreleased/41753 @@ -0,0 +1,13 @@ +Change: Expose createLocal() on ICacheFactory + +The cache factory has always been able to hand out a cache from the host local +tier, but the method was missing from the public ICacheFactory interface, so +core had to ask for it defensively and apps had no way to use it at all. It is +now part of the interface, which lets values that are only meaningful on the +machine that produced them be kept out of the cache shared between the nodes of +an installation. + +Note for app developers: a class implementing OCP\ICacheFactory has to declare +createLocal() from this release on. + +https://github.com/owncloud/core/pull/41753 diff --git a/lib/private/App/AppManager.php b/lib/private/App/AppManager.php index b1480cd73590..4e73066b8a4a 100644 --- a/lib/private/App/AppManager.php +++ b/lib/private/App/AppManager.php @@ -31,8 +31,7 @@ namespace OC\App; use OC_App; -use OC\Memcache\ArrayCache; -use OC\Memcache\NullCache; +use OC\Memcache\LocalCacheFactory; use OC\Installer; use OCP\App\AppNotFoundException; use OCP\App\IAppManager; @@ -120,14 +119,9 @@ public function __construct( $this->dispatcher = $dispatcher; $this->config = $config; - // TODO we have no public API for this - if (\method_exists($this->memCacheFactory, 'createLocal')) { - /* @phan-suppress-next-line PhanUndeclaredMethod */ - $this->appInfo = $this->memCacheFactory->createLocal('app-info'); - } - if ($this->appInfo === null || $this->appInfo instanceof NullCache) { - $this->appInfo = new ArrayCache('app-info'); - } + // app info describes the apps installed on this machine, so it belongs in + // the host local tier + $this->appInfo = LocalCacheFactory::create($this->memCacheFactory, 'app-info'); } /** diff --git a/lib/private/Memcache/LocalCacheFactory.php b/lib/private/Memcache/LocalCacheFactory.php index 003cbeec2155..0e3ac480beaa 100644 --- a/lib/private/Memcache/LocalCacheFactory.php +++ b/lib/private/Memcache/LocalCacheFactory.php @@ -33,9 +33,8 @@ * consume what any single node (or anything else able to talk to the backend) * wrote there. * - * ICacheFactory does not declare createLocal(), so the tier is requested - * defensively and a request scoped ArrayCache is used whenever a usable local - * cache cannot be obtained. + * A request scoped ArrayCache is used whenever a usable local cache cannot be + * obtained. * * @package OC\Memcache */ @@ -51,11 +50,7 @@ class LocalCacheFactory { * @return ICache */ public static function create(ICacheFactory $factory, $prefix = '') { - $cache = null; - if (\method_exists($factory, 'createLocal')) { - /* @phan-suppress-next-line PhanUndeclaredMethod */ - $cache = $factory->createLocal($prefix); - } + $cache = $factory->createLocal($prefix); if (!$cache instanceof ICache || $cache instanceof NullCache) { $cache = new ArrayCache($prefix); } diff --git a/lib/public/ICacheFactory.php b/lib/public/ICacheFactory.php index 977bb31c42ee..988e741d1f55 100644 --- a/lib/public/ICacheFactory.php +++ b/lib/public/ICacheFactory.php @@ -40,6 +40,23 @@ interface ICacheFactory { */ public function create($prefix = ''); + /** + * Get a memory cache instance in the host local tier + * + * Unlike the cache returned by create(), this cache is not shared between the + * nodes of an installation. Values which are only meaningful on the machine + * which produced them - resolved paths on disk, results of inspecting the + * local installation - belong here and not in the shared cache, where every + * node would consume what any single node wrote. + * + * All entries added trough the cache instance will be namespaced by $prefix to prevent collisions between apps + * + * @param string $prefix + * @return \OCP\ICache + * @since 11.0.0 + */ + public function createLocal($prefix = ''); + /** * Check if any memory cache backend is available * diff --git a/tests/lib/Memcache/LocalCacheFactoryTest.php b/tests/lib/Memcache/LocalCacheFactoryTest.php index 374e54b79b63..a6d866681771 100644 --- a/tests/lib/Memcache/LocalCacheFactoryTest.php +++ b/tests/lib/Memcache/LocalCacheFactoryTest.php @@ -77,20 +77,12 @@ public function testNeverFallsBackToTheDistributedTier(): void { $this->assertNull($factory->createDistributed('test')->get('key')); } - public function testFallsBackWhenCreateLocalIsNotImplemented(): void { - // ICacheFactory does not declare createLocal(), so a bare interface - // mock has no such method - $factory = $this->createMock(ICacheFactory::class); - - $this->assertInstanceOf(ArrayCache::class, LocalCacheFactory::create($factory, 'test')); - } - /** * @dataProvider unusableLocalCacheProvider * @param mixed $returnValue */ public function testFallsBackWhenCreateLocalReturnsSomethingUnusable($returnValue): void { - $factory = $this->createMock(FixedCacheFactory::class); + $factory = $this->createMock(ICacheFactory::class); $factory->method('createLocal')->willReturn($returnValue); $this->assertInstanceOf(ArrayCache::class, LocalCacheFactory::create($factory, 'test')); @@ -103,6 +95,17 @@ public function unusableLocalCacheProvider() { ]; } + /** + * The helper calls createLocal() unconditionally, which is only safe as long + * as every ICacheFactory has to provide it. + */ + public function testTheLocalTierIsPartOfThePublicApi(): void { + $this->assertTrue( + (new \ReflectionClass(ICacheFactory::class))->hasMethod('createLocal'), + 'ICacheFactory declares createLocal()' + ); + } + public function testPassesThroughThePrefix(): void { $factory = new FixedCacheFactory($this->createMock(ICache::class));