diff --git a/src/Shortcodes/FileShortcodeProvider.php b/src/Shortcodes/FileShortcodeProvider.php index 4f4a4459..69a29e12 100644 --- a/src/Shortcodes/FileShortcodeProvider.php +++ b/src/Shortcodes/FileShortcodeProvider.php @@ -136,17 +136,30 @@ public static function handle_shortcode($arguments, $content, $parser, $shortcod */ protected static function getCachedMarkup($cache, $cacheKey, $arguments): string { + // Retrieve the cache markup $item = $cache->get($cacheKey); + if (empty($item) || empty($item['markup'])) { + // This is a cache miss + return ''; + } + + // Check the file exists + $filename = $item['filename']; + $hash = $item['hash']; $assetStore = Injector::inst()->get(AssetStore::class); - if ($item && $item['markup'] && !empty($item['filename'])) { - // Initiate a protected asset grant if necessary - $allowSessionGrant = static::getGrant(null, $arguments); - if ($allowSessionGrant && $assetStore->exists($item['filename'], $item['hash'])) { - $assetStore->grant($item['filename'], $item['hash']); - return $item['markup']; - } + if (empty($filename) || empty($hash) || !$assetStore->exists($filename, $hash)) { + // Cache entry is wrong, delete it + $cache->delete($cacheKey); + return ''; } - return ''; + + // Shortcode can be configured to session grant access to files even if the file is protected + $allowSessionGrant = static::getGrant(null, $arguments); + if ($allowSessionGrant) { + $assetStore->grant($filename, $hash); + } + + return $item['markup']; } /** diff --git a/src/Shortcodes/ImageShortcodeProvider.php b/src/Shortcodes/ImageShortcodeProvider.php index 9b8cd419..2e5c7b9c 100644 --- a/src/Shortcodes/ImageShortcodeProvider.php +++ b/src/Shortcodes/ImageShortcodeProvider.php @@ -7,7 +7,6 @@ use SilverStripe\Assets\Image; use SilverStripe\Core\Flushable; use SilverStripe\Core\Injector\Injector; -use SilverStripe\View\HTML; use SilverStripe\View\Parsers\ShortcodeHandler; use SilverStripe\View\Parsers\ShortcodeParser; diff --git a/tests/php/Shortcodes/FileShortcodeProviderTest.php b/tests/php/Shortcodes/FileShortcodeProviderTest.php index 16c7c4ec..4e59fb12 100644 --- a/tests/php/Shortcodes/FileShortcodeProviderTest.php +++ b/tests/php/Shortcodes/FileShortcodeProviderTest.php @@ -44,6 +44,8 @@ protected function setUp(): void Config::inst()->set(SiteTree::class, 'create_default_pages', true); ErrorPage::singleton()->requireDefaultRecords(); } + + FileShortcodeProvider::getCache()->clear(); } protected function tearDown(): void @@ -179,4 +181,30 @@ public function testMarkupHasStringValue() 'Test that shortcode with invalid file ID is not parsed.' ); } + + public function testCacheHit() + { + $testFile = $this->objFromFixture(File::class, 'asdf'); + $testFileID = $testFile->ID; + $tuple = $testFile->File->getValue(); + + // Prewarm cache with non-sense value + $markup = ''; + $key = FileShortcodeProvider::getCacheKey(['id' => (string)$testFileID], ''); + FileShortcodeProvider::getCache()->set($key, [ + 'markup' => $markup, + 'filename' => $tuple['Filename'], + 'hash' => $tuple['Hash'] + ]); + + // Try to retrieve short code ... which should hit the cache + $parser = new ShortcodeParser(); + $parser->register('file_link', [FileShortcodeProvider::class, 'handle_shortcode']); + $fileShortcode = sprintf('[file_link,id=%d]', $testFileID); + $this->assertEquals( + $markup, + $parser->parse(sprintf('[file_link,id=%d]', $testFileID)), + 'Value Stored in the FileShortcodeProvider cache is returned by the shortcode.' + ); + } }