Skip to content

Commit de9e387

Browse files
author
Jannik Zschiesche
authored
Merge pull request #33 from Becklyn/asset-load-dev
Add specialized existence check in `AssetUrl` in debug mode when creating URLs
2 parents b96960f + bc82f1b commit de9e387

File tree

5 files changed

+82
-26
lines changed

5 files changed

+82
-26
lines changed

‎.github/pull_request_template.md‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
| Q | A
2-
| ------------- | ---
3-
| Bug fix? | yes/no
4-
| Improvement? | yes/no <!-- improves an existing feature, not adding a new one -->
5-
| New feature? | yes/no <!-- don't forget to update CHANGELOG.md -->
6-
| BC breaks? | no
7-
| Deprecations? | yes/no <!-- don't forget to update UPGRADE.md and CHANGELOG.md -->
8-
| Docs PR | **missing** <!-- insert URL here -->
2+
| ------------- | --------------------------------------------------------------------- |
3+
| BC breaks? | yes/no |
4+
| New feature? | yes/no <!-- don't forget to update CHANGELOG.md --> |
5+
| Improvement? | yes/no <!-- improves an existing feature, not adding a new one --> |
6+
| Bug fix? | yes/no |
7+
| Deprecations? | yes/no <!-- don't forget to update UPGRADE.md and CHANGELOG.md --> |
8+
| Docs PR | **missing** <!-- insert URL here --> |
99

1010
<!-- describe your changes below -->

‎CHANGELOG.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,9 @@
1+
2.6.4
2+
=====
3+
4+
* (improvement) Add specialized existence check in `AssetUrl` in debug mode when creating URLs. It circumvents the file loading, this heavily improves performance in debug requests with a cold cache.
5+
6+
17
2.6.3
28
=====
39

‎src/File/FileLoader.php‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,12 +83,19 @@ public function loadFile (Asset $asset, ?bool $mode) : string
8383
/**
8484
* Returns the file path for the given asset.
8585
*
86-
* @throws AssetsException
87-
*
8886
* @return string
8987
*/
9088
public function getFilePath (Asset $asset)
9189
{
9290
return $this->namespaceRegistry->getFilePath($asset);
9391
}
92+
93+
94+
/**
95+
* Checks whether the file for the asset actually exists on disk.
96+
*/
97+
public function fileForAssetExists (Asset $asset) : bool
98+
{
99+
return \is_file($this->getFilePath($asset));
100+
}
94101
}

‎src/Url/AssetUrl.php‎

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
use Becklyn\AssetsBundle\Asset\Asset;
66
use Becklyn\AssetsBundle\Asset\AssetsRegistry;
77
use Becklyn\AssetsBundle\Exception\AssetsException;
8+
use Becklyn\AssetsBundle\Exception\FileNotFoundException;
9+
use Becklyn\AssetsBundle\File\FileLoader;
810
use Becklyn\AssetsBundle\RouteLoader\AssetsRouteLoader;
911
use Psr\Log\LoggerInterface;
1012
use Symfony\Component\Routing\RouterInterface;
@@ -36,13 +38,20 @@ class AssetUrl
3638

3739

3840
/**
41+
* @var FileLoader
3942
*/
40-
public function __construct (AssetsRegistry $assetsRegistry, RouterInterface $router, bool $isDebug, ?LoggerInterface $logger)
43+
private $fileLoader;
44+
45+
46+
/**
47+
*/
48+
public function __construct (AssetsRegistry $assetsRegistry, RouterInterface $router, FileLoader $fileLoader, bool $isDebug, ?LoggerInterface $logger)
4149
{
4250
$this->assetsRegistry = $assetsRegistry;
4351
$this->router = $router;
4452
$this->isDebug = $isDebug;
4553
$this->logger = $logger;
54+
$this->fileLoader = $fileLoader;
4655
}
4756

4857

@@ -55,12 +64,25 @@ public function generateUrl (Asset $asset) : string
5564

5665
try
5766
{
58-
// always load asset to catch missing assets in dev
59-
$cachedAsset = $this->assetsRegistry->get($asset);
60-
61-
// use dumped file path in prod
62-
if (!$this->isDebug)
67+
if ($this->isDebug)
68+
{
69+
// in debug only check that the file exists, so we can eagerly throw an exception
70+
if (!$this->fileLoader->fileForAssetExists($asset))
71+
{
72+
throw new FileNotFoundException(\sprintf(
73+
"Asset '%s' not found at '%s'.",
74+
$asset->getAssetPath(),
75+
$this->fileLoader->getFilePath($asset)
76+
));
77+
}
78+
}
79+
else
6380
{
81+
// Only actually load the asset in production due to the importing of files being very expensive.
82+
// Be aware that you will only catch missing assets in your browser dev tools 404 errors.
83+
$cachedAsset = $this->assetsRegistry->get($asset);
84+
85+
// use dumped file path in prod
6486
$asset = $cachedAsset;
6587
$filePath = $cachedAsset->getDumpFilePath();
6688
}

‎tests/Url/AssetUrlTest.php‎

Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use Becklyn\AssetsBundle\Asset\Asset;
66
use Becklyn\AssetsBundle\Asset\AssetsRegistry;
77
use Becklyn\AssetsBundle\Exception\AssetsException;
8+
use Becklyn\AssetsBundle\File\FileLoader;
89
use Becklyn\AssetsBundle\Url\AssetUrl;
910
use PHPUnit\Framework\TestCase;
1011
use Psr\Log\LoggerInterface;
@@ -22,10 +23,15 @@ private function buildObject (bool $isDebug)
2223
->disableOriginalConstructor()
2324
->getMock();
2425

26+
$fileLoader = $this->getMockBuilder(FileLoader::class)
27+
->disableOriginalConstructor()
28+
->getMock();
29+
2530
return [
26-
new AssetUrl($registry, $router, $isDebug, null),
31+
new AssetUrl($registry, $router, $fileLoader, $isDebug, null),
2732
$router,
2833
$registry,
34+
$fileLoader,
2935
];
3036
}
3137

@@ -35,11 +41,17 @@ public function testDev () : void
3541
/**
3642
* @var AssetUrl
3743
* @var \PHPUnit_Framework_MockObject_MockObject $router
44+
* @var \PHPUnit_Framework_MockObject_MockObject $registry
45+
* @var \PHPUnit_Framework_MockObject_MockObject $fileLoader
3846
*/
39-
[$assetUrl, $router] = $this->buildObject(true);
47+
[$assetUrl, $router, $registry, $fileLoader] = $this->buildObject(true);
4048

4149
$asset = new Asset("namespace", "test.jpg");
4250

51+
$fileLoader
52+
->method("fileForAssetExists")
53+
->willReturn(true);
54+
4355
$router
4456
->expects(self::once())
4557
->method("generate")
@@ -141,7 +153,11 @@ public function testMissingFileInProdWithLogger () : void
141153
->disableOriginalConstructor()
142154
->getMock();
143155

144-
$assetUrl = new AssetUrl($registry, $router, false, $logger);
156+
$fileLoader = $this->getMockBuilder(FileLoader::class)
157+
->disableOriginalConstructor()
158+
->getMock();
159+
160+
$assetUrl = new AssetUrl($registry, $router, $fileLoader, false, $logger);
145161

146162
$asset = new Asset("test", "abc.jpg");
147163

@@ -173,20 +189,25 @@ public function testMissingFileInProdWithLogger () : void
173189
public function testMissingFileInDev () : void
174190
{
175191
/**
176-
* @var AssetUrl
192+
* @var AssetUrl $assetUrl
177193
* @var \PHPUnit_Framework_MockObject_MockObject $router
178194
* @var \PHPUnit_Framework_MockObject_MockObject $registry
195+
* @var \PHPUnit_Framework_MockObject_MockObject $fileLoader
179196
*/
180-
[$assetUrl, $router, $registry] = $this->buildObject(true);
197+
[$assetUrl, $router, $registry, $fileLoader] = $this->buildObject(true);
181198

182-
$asset = new Asset("test", "abc.jpg");
199+
$fileLoader
200+
->method("fileForAssetExists")
201+
->willReturn(false);
183202

184-
$registry
185-
->expects(self::once())
186-
->method("get")
187-
->with($asset)
188-
->willThrowException(new AssetsException());
203+
$fileLoader
204+
->method("getFilePath")
205+
->willReturn("/some/path");
189206

207+
$this->expectException(AssetsException::class);
208+
$this->expectExceptionMessage("Asset '@test/abc.jpg' not found at '/some/path'.");
209+
210+
$asset = new Asset("test", "abc.jpg");
190211
$assetUrl->generateUrl($asset);
191212
}
192213
}

0 commit comments

Comments
 (0)