Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[2.x] Major performance and data handling improvements to media assets #1918

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
92 commits
Select commit Hold shift + click to select a range
1bdf4ba
Annotate the collection generics
caendesilva Jul 28, 2024
96781f9
Normalize implementation syntax
caendesilva Jul 28, 2024
2777892
Document identifier method
caendesilva Jul 28, 2024
58cfaf8
Add more tests for the identifier method
caendesilva Jul 28, 2024
4e4ef03
Override parent constructor
caendesilva Jul 28, 2024
c8ca302
Normalize stored internal path
caendesilva Jul 28, 2024
c4b7af3
Normalize formatter syntax
caendesilva Jul 28, 2024
b20eab6
Revert "Normalize formatter syntax"
caendesilva Jul 28, 2024
0907bc5
Test normalization
caendesilva Jul 28, 2024
c3b7ca8
Add initial normalization
caendesilva Jul 28, 2024
e82649c
Update MediaFileTest.php
caendesilva Jul 28, 2024
4e9ea80
Change normalization to enforce parent documented state
caendesilva Jul 28, 2024
82bbc1c
Update tests to expect media file paths are normalized on construct
caendesilva Jul 28, 2024
67f056f
Add more unit tests
caendesilva Jul 28, 2024
9f920f7
Fix missing test reset
caendesilva Jul 28, 2024
4aa11b4
Add hash accessor
caendesilva Jul 28, 2024
dca20a0
Use CRC32 for media asset hashing
caendesilva Jul 28, 2024
4cb592f
Use CRC32 for media asset cache busting
caendesilva Jul 28, 2024
9af2d81
Get the hash through creating an instance
caendesilva Jul 28, 2024
43e99ae
Use static to reference the same class
caendesilva Jul 28, 2024
09689fb
Add the hash to the array representation
caendesilva Jul 28, 2024
b936744
Throw exception if media file does not exist
caendesilva Jul 29, 2024
b1d6f30
Revert "Throw exception if media file does not exist"
caendesilva Jul 29, 2024
0b77a4d
Reapply "Throw exception if media file does not exist"
caendesilva Jul 29, 2024
21340c9
Files should receive missing and return false
caendesilva Jul 29, 2024
6e5c261
Validate the normalized path
caendesilva Jul 29, 2024
070a7ff
Refactor to use mocks
caendesilva Jul 29, 2024
f22f8e1
Revert "Refactor to use mocks"
caendesilva Jul 29, 2024
2eb56a9
Revert "Files should receive missing and return false"
caendesilva Jul 29, 2024
000f19a
Create public internal static property to control validation
caendesilva Jul 29, 2024
21df2e3
Disable existence validation during unit tests
caendesilva Jul 29, 2024
bfda133
Normalize paths using output directory to have source directory prefix
caendesilva Jul 29, 2024
1326659
Test existence validation
caendesilva Jul 29, 2024
0ccec37
Remove content length validation now performed at construct time
caendesilva Jul 29, 2024
9a7f58b
Extract method to normalize path
caendesilva Jul 29, 2024
5f03721
Set metadata properties at construct time
caendesilva Jul 29, 2024
937917f
Return value when set
caendesilva Jul 29, 2024
cb37c4e
Return value when set
caendesilva Jul 29, 2024
523051f
Return value when set
caendesilva Jul 29, 2024
e3104fb
Assume value is set
caendesilva Jul 29, 2024
134d24e
Revert "Assume value is set"
caendesilva Jul 29, 2024
8b9de19
Find all media file metadata at construct time
caendesilva Jul 29, 2024
d4a9390
Remove tests for weird edge cases with no benefit
caendesilva Jul 29, 2024
39d5169
Split out unit test and feature test
caendesilva Jul 29, 2024
df1bc7c
Generate feature test with Claude
caendesilva Jul 29, 2024
84591e4
Fix all 'Unused import' problems in file
caendesilva Jul 29, 2024
ec8099b
Expect default file
caendesilva Jul 29, 2024
d819bff
Remove setter for already default value
caendesilva Jul 29, 2024
c51915e
Expect CRC32 hash
caendesilva Jul 29, 2024
36f3545
Cleanup test code comments
caendesilva Jul 29, 2024
3a1c66b
Use assert same instead of assert equals
caendesilva Jul 29, 2024
521df1e
Assert on the array
caendesilva Jul 29, 2024
7772225
Bypass discovery to isolate unit test
caendesilva Jul 29, 2024
1f28f21
Refactor media file class to use the Filesystem facade
caendesilva Jul 29, 2024
bc4c511
No need to qualify path handled by Filesystem facade
caendesilva Jul 29, 2024
5e3e279
Prepare unit test for refactor
caendesilva Jul 29, 2024
749e3ea
Refactor unit test to use the filesystem mocks
caendesilva Jul 29, 2024
cbdf540
Fix failing unit test
caendesilva Jul 29, 2024
7757422
Reset kernel
caendesilva Jul 30, 2024
160e401
Run test without app stylesheet
caendesilva Jul 30, 2024
c585ee3
Revert "Reset kernel"
caendesilva Jul 30, 2024
afc9b01
Revert "Run test without app stylesheet"
caendesilva Jul 30, 2024
025b35e
Update MediaFileUnitTest.php
caendesilva Jul 30, 2024
04e2ead
Merge branch '2.x-dev' into bring-media-assets-into-the-hyde-kernel
caendesilva Jul 30, 2024
9d28878
Swap for a simpler implementation
caendesilva Jul 30, 2024
6d26c9c
Merge branch 'normalize-the-asset-api' into bring-media-assets-into-t…
caendesilva Aug 1, 2024
3089fbc
Use fully qualified class name in PHPDoc
caendesilva Aug 1, 2024
ee1b3b7
Merge branch 'normalize-the-asset-api' into bring-media-assets-into-t…
caendesilva Aug 1, 2024
e1a3909
Clarify existence of internal property
caendesilva Aug 1, 2024
cc87ff9
Mock base filesystem
caendesilva Aug 1, 2024
bec2d60
Remove file existence check before getting data
caendesilva Aug 1, 2024
818f939
Match method declaration order to the base page class
caendesilva Aug 1, 2024
2471dfa
Add an example return value
caendesilva Aug 1, 2024
1b85baa
Merge example to single line
caendesilva Aug 1, 2024
8c20420
Revert "Merge example to single line"
caendesilva Aug 1, 2024
814b3eb
Add return prefix
caendesilva Aug 1, 2024
2802ae2
Revert "Add return prefix"
caendesilva Aug 1, 2024
3441c53
Improve the method documentation
caendesilva Aug 1, 2024
067a805
Improve the method documentation
caendesilva Aug 1, 2024
31d8d2a
Format documented class name
caendesilva Aug 1, 2024
c4dc0f7
Generate more unit tests to cover all code paths
caendesilva Aug 1, 2024
e314584
Generate more constructor unit tests
caendesilva Aug 1, 2024
a6b83a8
Annotate context
caendesilva Aug 1, 2024
53023b5
Change protected properties to public readonly
caendesilva Aug 1, 2024
a39ba5b
Normalize extension access between class structure
caendesilva Aug 1, 2024
e077ac6
Remove internal validate existence flag
caendesilva Aug 1, 2024
398a5b8
Merge branch 'normalize-the-asset-api' into bring-media-assets-into-t…
caendesilva Aug 2, 2024
dbbd957
Introduce local variable
caendesilva Aug 3, 2024
d361846
Rename helper method
caendesilva Aug 3, 2024
e6ae28d
Move file existence check to normalizer method
caendesilva Aug 3, 2024
89feb9c
Document code reasoning
caendesilva Aug 3, 2024
c3bcc78
Inline local variable
caendesilva Aug 3, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<section id="hyde-kernel-filesystem-methods">

<!-- Start generated docs for Hyde\Foundation\Concerns\ForwardsFilesystem -->
<!-- Generated by HydePHP DocGen script at 2024-07-28 11:20:12 in 0.11ms -->
<!-- Generated by HydePHP DocGen script at 2024-08-01 10:01:06 in 0.13ms -->

#### `filesystem()`

Expand Down Expand Up @@ -56,7 +56,7 @@ Hyde::pathToRelative(string $path): string
No description provided.

```php
Hyde::assets(): Illuminate\Support\Collection
Hyde::assets(): \Illuminate\Support\Collection<string, \Hyde\Support\Filesystem\MediaFile>
```

<!-- End generated docs for Hyde\Foundation\Concerns\ForwardsFilesystem -->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ public function pathToRelative(string $path): string
return $this->filesystem->pathToRelative($path);
}

/** @return \Illuminate\Support\Collection<string, \Hyde\Support\Filesystem\MediaFile> */
public function assets(): Collection
{
return $this->filesystem->assets();
Expand Down
102 changes: 79 additions & 23 deletions packages/framework/src/Support/Filesystem/MediaFile.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,16 @@

use Hyde\Hyde;
use Hyde\Facades\Config;
use Hyde\Facades\Filesystem;
use Illuminate\Support\Collection;
use Hyde\Framework\Exceptions\FileNotFoundException;
use Illuminate\Support\Str;

use function Hyde\unslash;
use function Hyde\path_join;
use function Hyde\trim_slashes;
use function extension_loaded;
use function file_exists;
use function array_merge;
use function filesize;
use function pathinfo;
use function is_file;

/**
* File abstraction for a project media file.
Expand All @@ -27,18 +25,39 @@ class MediaFile extends ProjectFile
/** @var array<string> The default extensions for media types */
final public const EXTENSIONS = ['png', 'svg', 'jpg', 'jpeg', 'gif', 'ico', 'css', 'js'];

/** @return \Illuminate\Support\Collection<string, \Hyde\Support\Filesystem\MediaFile> The array keys are the filenames relative to the _media/ directory */
public static function all(): Collection
public readonly int $length;
public readonly string $mimeType;
public readonly string $hash;

public function __construct(string $path)
{
return Hyde::assets();
parent::__construct($this->getNormalizedPath($path));

$this->length = $this->findContentLength();
$this->mimeType = $this->findMimeType();
$this->hash = $this->findHash();
}

/** @return array<string> Array of filenames relative to the _media/ directory */
/**
* Get an array of media asset filenames relative to the `_media/` directory.
*
* @return array<int, string> {@example `['app.css', 'images/logo.svg']`}
*/
public static function files(): array
{
return static::all()->keys()->all();
}

/**
* Get a collection of all media files, parsed into `MediaFile` instances, keyed by the filenames relative to the `_media/` directory.
*
* @return \Illuminate\Support\Collection<string, \Hyde\Support\Filesystem\MediaFile>
*/
public static function all(): Collection
{
return Hyde::assets();
}

/**
* Get the absolute path to the media source directory, or a file within it.
*/
Expand All @@ -60,11 +79,12 @@ public static function outputPath(string $path = ''): string
return Hyde::sitePath(Hyde::getMediaOutputDirectory());
}

$path = unslash($path);

return Hyde::sitePath(Hyde::getMediaOutputDirectory()."/$path");
return Hyde::sitePath(path_join(Hyde::getMediaOutputDirectory(), unslash($path)));
}

/**
* Get the path to the media file relative to the media directory.
*/
public function getIdentifier(): string
{
return Str::after($this->getPath(), Hyde::getMediaDirectory().'/');
Expand All @@ -75,21 +95,60 @@ public function toArray(): array
return array_merge(parent::toArray(), [
'length' => $this->getContentLength(),
'mimeType' => $this->getMimeType(),
'hash' => $this->getHash(),
]);
}

public function getContentLength(): int
{
if (! is_file($this->getAbsolutePath())) {
throw new FileNotFoundException($this->path);
return $this->length;
}

public function getMimeType(): string
{
return $this->mimeType;
}

public function getHash(): string
{
return $this->hash;
}

/** @internal */
public static function getCacheBustKey(string $file): string
{
return Config::getBool('hyde.enable_cache_busting', true) && Filesystem::exists(static::sourcePath("$file"))
? '?v='.static::make($file)->getHash()
: '';
}

protected function getNormalizedPath(string $path): string
{
$path = Hyde::pathToRelative($path);

// Normalize paths using output directory to have source directory prefix
if (str_starts_with($path, Hyde::getMediaOutputDirectory()) && str_starts_with(Hyde::getMediaDirectory(), '_')) {
$path = '_'.$path;
}

return filesize($this->getAbsolutePath());
// Normalize the path to include the media directory
$path = static::sourcePath(trim_slashes(Str::after($path, Hyde::getMediaDirectory())));

if (Filesystem::missing($path)) {
throw new FileNotFoundException($path);
}

return $path;
}

public function getMimeType(): string
protected function findContentLength(): int
{
return Filesystem::size($this->getPath());
}

protected function findMimeType(): string
{
$extension = pathinfo($this->getAbsolutePath(), PATHINFO_EXTENSION);
$extension = $this->getExtension();

// See if we can find a mime type for the extension instead of
// having to rely on a PHP extension and filesystem lookups.
Expand All @@ -112,18 +171,15 @@ public function getMimeType(): string
return $lookup[$extension];
}

if (extension_loaded('fileinfo') && file_exists($this->getAbsolutePath())) {
return mime_content_type($this->getAbsolutePath());
if (extension_loaded('fileinfo') && Filesystem::exists($this->getPath())) {
return Filesystem::mimeType($this->getPath());
}

return 'text/plain';
}

/** @internal */
public static function getCacheBustKey(string $file): string
protected function findHash(): string
{
return Config::getBool('hyde.enable_cache_busting', true) && file_exists(MediaFile::sourcePath("$file"))
? '?v='.md5_file(MediaFile::sourcePath("$file"))
: '';
return Filesystem::hash($this->getPath(), 'crc32');
}
}
4 changes: 1 addition & 3 deletions packages/framework/src/Support/Filesystem/ProjectFile.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@
use Hyde\Support\Concerns\Serializable;
use Hyde\Support\Contracts\SerializableContract;

use function pathinfo;

/**
* Filesystem abstraction for a file stored in the project.
*/
Expand Down Expand Up @@ -71,6 +69,6 @@ public function getContents(): string

public function getExtension(): string
{
return pathinfo($this->getAbsolutePath(), PATHINFO_EXTENSION);
return Filesystem::extension($this->getPath());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,7 @@ public function testAssetsMethodGetsAllSiteAssetsAsArray()
'path' => '_media/app.css',
'length' => 123,
'mimeType' => 'text/css',
'hash' => hash_file('crc32', Hyde::path('_media/app.css')),
],
], $assets);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,8 @@ public function testMediaLinkHelperUsesConfiguredMediaDirectory()

public function testMediaLinkHelperWithValidationAndExistingFile()
{
$this->file('_media/foo');
$this->assertSame('media/foo?v=d41d8cd98f00b204e9800998ecf8427e', $this->class->mediaLink('foo', true));
$this->file('_media/foo', 'test');
$this->assertSame('media/foo?v=accf8b33', $this->class->mediaLink('foo', true));
}

public function testMediaLinkHelperWithValidationAndNonExistingFile()
Expand Down
121 changes: 121 additions & 0 deletions packages/framework/tests/Feature/Support/MediaFileTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
<?php

declare(strict_types=1);

namespace Hyde\Framework\Testing\Feature\Support;

use Hyde\Hyde;
use Hyde\Testing\TestCase;
use Hyde\Support\Filesystem\MediaFile;
use Hyde\Framework\Exceptions\FileNotFoundException;

/**
* @covers \Hyde\Support\Filesystem\MediaFile
*
* @see \Hyde\Framework\Testing\Unit\Support\MediaFileUnitTest
*/
class MediaFileTest extends TestCase
{
public function testMediaFileCreationAndBasicProperties()
{
$this->file('_media/test.txt', 'Hello, World!');

$mediaFile = MediaFile::make('test.txt');

$this->assertInstanceOf(MediaFile::class, $mediaFile);
$this->assertSame('test.txt', $mediaFile->getName());
$this->assertSame('_media/test.txt', $mediaFile->getPath());
$this->assertSame(Hyde::path('_media/test.txt'), $mediaFile->getAbsolutePath());
$this->assertSame('Hello, World!', $mediaFile->getContents());
$this->assertSame('txt', $mediaFile->getExtension());

$this->assertSame([
'name' => 'test.txt',
'path' => '_media/test.txt',
'length' => 13,
'mimeType' => 'text/plain',
'hash' => 'dffed8e6',
], $mediaFile->toArray());
}

public function testMediaFileDiscovery()
{
// App.css is a default file
$this->file('_media/image.png', 'PNG content');
$this->file('_media/style.css', 'CSS content');
$this->file('_media/script.js', 'JS content');

$allFiles = MediaFile::all();

$this->assertCount(4, $allFiles);
$this->assertArrayHasKey('image.png', $allFiles);
$this->assertArrayHasKey('style.css', $allFiles);
$this->assertArrayHasKey('script.js', $allFiles);

$fileNames = MediaFile::files();
$this->assertSame(['image.png', 'app.css', 'style.css', 'script.js'], $fileNames);
}

public function testMediaFileProperties()
{
$content = str_repeat('a', 1024); // 1KB content
$this->file('_media/large_file.txt', $content);

$mediaFile = MediaFile::make('large_file.txt');

$this->assertSame(1024, $mediaFile->getContentLength());
$this->assertSame('text/plain', $mediaFile->getMimeType());
$this->assertSame(hash('crc32', $content), $mediaFile->getHash());
}

public function testMediaFilePathHandling()
{
$this->file('_media/subfolder/nested_file.txt', 'Nested content');

$mediaFile = MediaFile::make('subfolder/nested_file.txt');

$this->assertSame('subfolder/nested_file.txt', $mediaFile->getIdentifier());
$this->assertSame('_media/subfolder/nested_file.txt', $mediaFile->getPath());
}

public function testMediaFileExceptionHandling()
{
$this->expectException(FileNotFoundException::class);
MediaFile::make('non_existent_file.txt');
}

public function testMediaDirectoryCustomization()
{
Hyde::setMediaDirectory('custom_media');

$this->file('custom_media/custom_file.txt', 'Custom content');

$mediaFile = MediaFile::make('custom_file.txt');

$this->assertSame('custom_media/custom_file.txt', $mediaFile->getPath());
$this->assertSame(Hyde::path('custom_media/custom_file.txt'), $mediaFile->getAbsolutePath());

Hyde::setMediaDirectory('_media');
}

public function testMediaFileOutputPaths()
{
$this->assertSame(Hyde::path('_site/media'), MediaFile::outputPath());
$this->assertSame(Hyde::path('_site/media/test.css'), MediaFile::outputPath('test.css'));

Hyde::setOutputDirectory('custom_output');
$this->assertSame(Hyde::path('custom_output/media'), MediaFile::outputPath());

Hyde::setOutputDirectory('_site');
}

public function testMediaFileCacheBusting()
{
$this->file('_media/cachebust_test.js', 'console.log("Hello");');

$cacheBustKey = MediaFile::getCacheBustKey('cachebust_test.js');

$this->assertStringStartsWith('?v=', $cacheBustKey);
$this->assertSame('?v=cd5de5e7', $cacheBustKey); // Expect CRC32 hash
}
}
4 changes: 2 additions & 2 deletions packages/framework/tests/Unit/Facades/AssetFacadeUnitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public function testHasMediaFileHelperReturnsTrueForExistingFile()
public function testMediaLinkReturnsMediaPathWithCacheKey()
{
$this->assertIsString($path = Asset::mediaLink('app.css'));
$this->assertSame('media/app.css?v='.md5_file(Hyde::path('_media/app.css')), $path);
$this->assertSame('media/app.css?v='.hash_file('crc32', Hyde::path('_media/app.css')), $path);
}

public function testMediaLinkReturnsMediaPathWithoutCacheKeyIfCacheBustingIsDisabled()
Expand All @@ -72,6 +72,6 @@ public function testMediaLinkSupportsCustomMediaDirectories()
$path = Asset::mediaLink('app.css');

$this->assertIsString($path);
$this->assertSame('assets/app.css?v='.md5_file(Hyde::path('_assets/app.css')), $path);
$this->assertSame('assets/app.css?v='.hash_file('crc32', Hyde::path('_assets/app.css')), $path);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@

namespace Hyde\Framework\Testing\Unit\Foundation;

use Mockery;
use Hyde\Foundation\Kernel\Filesystem;
use Hyde\Hyde;
use Hyde\Support\Filesystem\MediaFile;
use Hyde\Testing\UnitTestCase;
use Illuminate\Support\Collection;
use Illuminate\Filesystem\Filesystem as BaseFilesystem;

/**
* @covers \Hyde\Foundation\Kernel\Filesystem
Expand All @@ -24,6 +26,12 @@ protected function setUp(): void
{
parent::setUp();
$this->filesystem = new TestableFilesystem(Hyde::getInstance());

$mock = Mockery::mock(BaseFilesystem::class)->makePartial();
$mock->shouldReceive('missing')->andReturn(false)->byDefault();
$mock->shouldReceive('size')->andReturn(100)->byDefault();
$mock->shouldReceive('hash')->andReturn('hash')->byDefault();
app()->instance(BaseFilesystem::class, $mock);
}

public function testAssetsMethodReturnsSameInstanceOnSubsequentCalls()
Expand Down
Loading