diff --git a/lib/private/Files/Type/Loader.php b/lib/private/Files/Type/Loader.php index 49cd44d1772db..ef5a9acf81363 100644 --- a/lib/private/Files/Type/Loader.php +++ b/lib/private/Files/Type/Loader.php @@ -12,6 +12,8 @@ use OCP\AppFramework\Db\TTransactional; use OCP\DB\Exception as DBException; use OCP\Files\IMimeTypeLoader; +use OCP\ICache; +use OCP\ICacheFactory; use OCP\IDBConnection; /** @@ -22,20 +24,32 @@ class Loader implements IMimeTypeLoader { use TTransactional; + private const string CACHE_KEY = 'mimetypes'; + private const int CACHE_TTL = 3600; + /** @psalm-var array */ - protected array $mimetypes; + protected array $mimetypes = []; /** @psalm-var array */ - protected array $mimetypeIds; + protected array $mimetypeIds = []; + + private ICache $cache; + + /** + * The cached copy of the table may lack rows added by another process, + * so a miss reloads from the database once per instance. + */ + private bool $reloaded = false; /** * @param IDBConnection $dbConnection + * @param ICacheFactory $cacheFactory */ public function __construct( private IDBConnection $dbConnection, + ICacheFactory $cacheFactory, ) { - $this->mimetypes = []; - $this->mimetypeIds = []; + $this->cache = $cacheFactory->createLocal('mimetypes'); } /** @@ -46,10 +60,10 @@ public function getMimetypeById(int $id): ?string { if (!$this->mimetypes) { $this->loadMimetypes(); } - if (isset($this->mimetypes[$id])) { - return $this->mimetypes[$id]; + if (!isset($this->mimetypes[$id])) { + $this->reloadOnMiss(); } - return null; + return $this->mimetypes[$id] ?? null; } /** @@ -60,6 +74,9 @@ public function getId(string $mimetype): int { if (!$this->mimetypeIds) { $this->loadMimetypes(); } + if (!isset($this->mimetypeIds[$mimetype])) { + $this->reloadOnMiss(); + } if (isset($this->mimetypeIds[$mimetype])) { return $this->mimetypeIds[$mimetype]; } @@ -74,6 +91,9 @@ public function exists(string $mimetype): bool { if (!$this->mimetypeIds) { $this->loadMimetypes(); } + if (!isset($this->mimetypeIds[$mimetype])) { + $this->reloadOnMiss(); + } return isset($this->mimetypeIds[$mimetype]); } @@ -84,6 +104,8 @@ public function exists(string $mimetype): bool { public function reset(): void { $this->mimetypes = []; $this->mimetypeIds = []; + $this->reloaded = false; + $this->cache->remove(self::CACHE_KEY); } /** @@ -118,30 +140,63 @@ protected function store(string $mimetype): int { if ($id === false) { throw new \Exception("Database threw an unique constraint on inserting a new mimetype, but couldn't return the ID for this very mimetype"); } - $mimetypeId = (int)$id; } $this->mimetypes[$mimetypeId] = $mimetype; $this->mimetypeIds[$mimetype] = $mimetypeId; + $this->cache->remove(self::CACHE_KEY); + return $mimetypeId; } /** - * Load all mimetypes from DB + * Load all mimetypes from the cache, falling back to the DB */ private function loadMimetypes(): void { + $cached = $this->cache->get(self::CACHE_KEY); + if (is_array($cached) && $cached !== []) { + $this->setMimetypes($cached); + return; + } + $this->loadMimetypesFromDatabase(); + } + + private function reloadOnMiss(): void { + if ($this->reloaded) { + return; + } + $this->reloaded = true; + $this->loadMimetypesFromDatabase(); + } + + private function loadMimetypesFromDatabase(): void { $qb = $this->dbConnection->getQueryBuilder(); $qb->select('id', 'mimetype') ->from('mimetypes'); - $result = $qb->executeQuery(); $results = $result->fetchAllAssociative(); $result->closeCursor(); + $mimetypes = []; foreach ($results as $row) { - $this->mimetypes[(int)$row['id']] = $row['mimetype']; - $this->mimetypeIds[$row['mimetype']] = (int)$row['id']; + $mimetypes[(int)$row['id']] = (string)$row['mimetype']; + } + $this->setMimetypes($mimetypes); + if ($mimetypes !== []) { + $this->cache->set(self::CACHE_KEY, $mimetypes, self::CACHE_TTL); + } + } + + /** + * @param array $mimetypes + */ + private function setMimetypes(array $mimetypes): void { + $this->mimetypes = []; + $this->mimetypeIds = []; + foreach ($mimetypes as $id => $mimetype) { + $this->mimetypes[(int)$id] = $mimetype; + $this->mimetypeIds[$mimetype] = (int)$id; } } diff --git a/tests/lib/Files/Type/LoaderTest.php b/tests/lib/Files/Type/LoaderTest.php index 35c549f321e94..12982ccd83ce2 100644 --- a/tests/lib/Files/Type/LoaderTest.php +++ b/tests/lib/Files/Type/LoaderTest.php @@ -9,6 +9,9 @@ namespace Test\Files\Type; use OC\Files\Type\Loader; +use OC\Memcache\ArrayCache; +use OCP\ICache; +use OCP\ICacheFactory; use OCP\IDBConnection; use OCP\Server; use PHPUnit\Framework\Attributes\Group; @@ -17,6 +20,7 @@ #[Group('DB')] class LoaderTest extends TestCase { protected IDBConnection $db; + protected ICache $cache; protected Loader $loader; #[\Override] @@ -24,7 +28,8 @@ protected function setUp(): void { parent::setUp(); $this->db = Server::get(IDBConnection::class); - $this->loader = new Loader($this->db); + $this->cache = new ArrayCache(); + $this->loader = $this->createLoader($this->db); } #[\Override] @@ -38,13 +43,24 @@ protected function tearDown(): void { parent::tearDown(); } - public function testGetMimetype(): void { + private function createLoader(IDBConnection $db): Loader { + $cacheFactory = $this->createMock(ICacheFactory::class); + $cacheFactory->method('createLocal')->willReturn($this->cache); + return new Loader($db, $cacheFactory); + } + + private function insertMimetype(string $mimetype): int { $qb = $this->db->getQueryBuilder(); $qb->insert('mimetypes') ->values([ - 'mimetype' => $qb->createPositionalParameter('testing/mymimetype') + 'mimetype' => $qb->createPositionalParameter($mimetype) ]); $qb->executeStatement(); + return $qb->getLastInsertId(); + } + + public function testGetMimetype(): void { + $this->insertMimetype('testing/mymimetype'); $this->assertTrue($this->loader->exists('testing/mymimetype')); $mimetypeId = $this->loader->getId('testing/mymimetype'); @@ -84,4 +100,43 @@ public function testStoreExists(): void { $this->assertEquals($mimetypeId, $mimetypeId2); } + + public function testMimetypesAreServedFromTheLocalCache(): void { + $mimetypeId = $this->insertMimetype('testing/cached'); + $this->assertEquals('testing/cached', $this->loader->getMimetypeById($mimetypeId)); + + $db = $this->createMock(IDBConnection::class); + $db->expects($this->never())->method('getQueryBuilder'); + $loader = $this->createLoader($db); + + $this->assertEquals('testing/cached', $loader->getMimetypeById($mimetypeId)); + $this->assertEquals($mimetypeId, $loader->getId('testing/cached')); + $this->assertTrue($loader->exists('testing/cached')); + } + + public function testUnknownIdIsReloadedFromTheDatabase(): void { + $this->assertTrue($this->loader->exists('httpd/unix-directory')); + + // added by another process after this instance loaded the table + $mimetypeId = $this->insertMimetype('testing/late'); + + $this->assertEquals('testing/late', $this->loader->getMimetypeById($mimetypeId)); + $this->assertEquals($mimetypeId, $this->createLoader($this->db)->getId('testing/late')); + // only one reload per instance + $this->assertNull($this->loader->getMimetypeById(12345)); + } + + public function testStoreAndResetInvalidateTheCache(): void { + $this->assertTrue($this->loader->exists('httpd/unix-directory')); + $this->assertTrue($this->cache->hasKey('mimetypes')); + + $this->loader->getId('testing/new'); + $this->assertFalse($this->cache->hasKey('mimetypes')); + + $this->assertTrue($this->createLoader($this->db)->exists('testing/new')); + $this->assertTrue($this->cache->hasKey('mimetypes')); + + $this->loader->reset(); + $this->assertFalse($this->cache->hasKey('mimetypes')); + } }