diff --git a/src/Flysystem/FlysystemAssetStore.php b/src/Flysystem/FlysystemAssetStore.php index 5dfd4af8..89ca0dab 100644 --- a/src/Flysystem/FlysystemAssetStore.php +++ b/src/Flysystem/FlysystemAssetStore.php @@ -217,9 +217,6 @@ public function getAsString($filename, $hash, $variant = null) public function getAsURL($filename, $hash, $variant = null, $grant = true) { - if ($grant) { - $this->grant($filename, $hash); - } $fileID = $this->getFileID($filename, $hash, $variant); // Check with filesystem this asset exists in @@ -229,11 +226,15 @@ public function getAsURL($filename, $hash, $variant = null, $grant = true) /** @var PublicAdapter $publicAdapter */ $publicAdapter = $public->getAdapter(); return $publicAdapter->getPublicUrl($fileID); - } else { - /** @var ProtectedAdapter $protectedAdapter */ - $protectedAdapter = $protected->getAdapter(); - return $protectedAdapter->getProtectedUrl($fileID); } + + if ($grant) { + $this->grant($filename, $hash); + } + + /** @var ProtectedAdapter $protectedAdapter */ + $protectedAdapter = $protected->getAdapter(); + return $protectedAdapter->getProtectedUrl($fileID); } public function setFromLocalFile($path, $filename = null, $hash = null, $variant = null, $config = array()) diff --git a/src/Shortcodes/ImageShortcodeProvider.php b/src/Shortcodes/ImageShortcodeProvider.php index 715dd842..5e69477d 100644 --- a/src/Shortcodes/ImageShortcodeProvider.php +++ b/src/Shortcodes/ImageShortcodeProvider.php @@ -52,7 +52,8 @@ public static function handle_shortcode($args, $content, $parser, $shortcode, $e /** @var AssetStore $store */ $store = Injector::inst()->get(AssetStore::class); if (!empty($item['filename'])) { - $store->grant($item['filename'], $item['hash']); + // Initiate a protected asset grant if necessary + $store->getAsURL($item['filename'], $item['hash']); } return $item['markup']; } diff --git a/tests/php/Flysystem/FlysystemAssetStoreTest.php b/tests/php/Flysystem/FlysystemAssetStoreTest.php new file mode 100644 index 00000000..d1f6679e --- /dev/null +++ b/tests/php/Flysystem/FlysystemAssetStoreTest.php @@ -0,0 +1,109 @@ +<?php + +namespace SilverStripe\Assets\Tests\Flysystem; + +use League\Flysystem\Filesystem; +use PHPUnit_Framework_MockObject_MockObject; +use SilverStripe\Assets\Flysystem\FlysystemAssetStore; +use SilverStripe\Assets\Flysystem\ProtectedAssetAdapter; +use SilverStripe\Assets\Flysystem\PublicAssetAdapter; +use SilverStripe\Dev\SapphireTest; + +class FlysystemAssetStoreTest extends SapphireTest +{ + /** + * @var PublicAssetAdapter + */ + protected $publicAdapter; + + /** + * @var Filesystem + */ + protected $publicFilesystem; + + /** + * @var ProtectedAssetAdapter + */ + protected $protectedAdapter; + + /** + * @var Filesystem + */ + protected $protectedFilesystem; + + protected function setUp() + { + parent::setUp(); + + $this->publicAdapter = $this->getMockBuilder(PublicAssetAdapter::class) + ->setMethods(['getPublicUrl']) + ->getMock(); + + $this->publicFilesystem = $this->getMockBuilder(Filesystem::class) + ->setMethods(['has']) + ->setConstructorArgs([$this->publicAdapter]) + ->getMock(); + + $this->protectedAdapter = $this->getMockBuilder(ProtectedAssetAdapter::class) + ->setMethods(['getProtectedUrl']) + ->getMock(); + + $this->protectedFilesystem = $this->getMockBuilder(Filesystem::class) + ->setMethods(['has']) + ->setConstructorArgs([$this->protectedAdapter]) + ->getMock(); + } + + public function testGetAsUrlDoesntGrantForPublicAssets() + { + $this->publicFilesystem->expects($this->once())->method('has')->willReturn(true); + $this->publicAdapter->expects($this->once())->method('getPublicUrl')->willReturn('public.jpg'); + $this->protectedFilesystem->expects($this->never())->method('has'); + + /** @var FlysystemAssetStore|PHPUnit_Framework_MockObject_MockObject $assetStore */ + $assetStore = $this->getMockBuilder(FlysystemAssetStore::class) + ->setMethods(['getFileID']) + ->getMock(); + $assetStore->expects($this->once())->method('getFileID')->willReturn('public.jpg'); + + $assetStore->setPublicFilesystem($this->publicFilesystem); + $assetStore->setProtectedFilesystem($this->protectedFilesystem); + + $this->assertSame('public.jpg', $assetStore->getAsURL('foo', 'bar')); + } + + /** + * @param boolean $grant + * @dataProvider protectedUrlGrantProvider + */ + public function testGetAsUrlWithGrant($grant) + { + $this->publicFilesystem->expects($this->once())->method('has')->willReturn(false); + $this->publicAdapter->expects($this->never())->method('getPublicUrl'); + $this->protectedFilesystem->expects($this->once())->method('has')->willReturn(true); + $this->protectedAdapter->expects($this->once())->method('getProtectedUrl')->willReturn('protected.jpg'); + + /** @var FlysystemAssetStore|PHPUnit_Framework_MockObject_MockObject $assetStore */ + $assetStore = $this->getMockBuilder(FlysystemAssetStore::class) + ->setMethods(['getFileID', 'grant']) + ->getMock(); + $assetStore->expects($this->once())->method('getFileID')->willReturn('protected.jpg'); + $assetStore->expects($grant ? $this->once() : $this->never())->method('grant'); + + $assetStore->setPublicFilesystem($this->publicFilesystem); + $assetStore->setProtectedFilesystem($this->protectedFilesystem); + + $this->assertSame('protected.jpg', $assetStore->getAsURL('foo', 'bar', 'baz', $grant)); + } + + /** + * @return array[] + */ + public function protectedUrlGrantProvider() + { + return [ + [true], + [false], + ]; + } +}