Skip to content

fix(media): restore Media state on unserialize from Doctrine cache #88

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.

Sign up for GitHub

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

Open
BriceFab wants to merge 3 commits into
base: main
Choose a base branch
from
Open

fix(media): restore Media state on unserialize from Doctrine cache #88

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0cc9424
fix(media): restore Media state on unserialize from Doctrine cache
BriceFab Apr 6, 2026
57a32a6
castor qa:all
BriceFab Apr 6, 2026
efecf41
Fix PHPStan errors
loic425 Apr 9, 2026
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
2 changes: 2 additions & 0 deletions src/JoliMediaBundle.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
use JoliCode\MediaBundle\Doctrine\Type\MediaType;
use JoliCode\MediaBundle\Doctrine\Types;
use JoliCode\MediaBundle\Model\Format;
use JoliCode\MediaBundle\Model\Media;
use JoliCode\MediaBundle\PreProcessor\HeifPreProcessor;
use JoliCode\MediaBundle\Processor\Imagine;
use JoliCode\MediaBundle\Transformer\Resize\Mode;
Expand All @@ -39,6 +40,7 @@ public function boot(): void

// doctrine media type
$resolverInitializer = fn (): ?object => $this->container->get('joli_media.resolver');
Media::$resolverInitializer = $resolverInitializer;
MediaType::$resolverInitializer = $resolverInitializer;
MediaLongType::$resolverInitializer = $resolverInitializer;
}
Expand Down
52 changes: 51 additions & 1 deletion src/Model/Media.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,22 @@
namespace JoliCode\MediaBundle\Model;

use JoliCode\MediaBundle\Binary\Binary;
use JoliCode\MediaBundle\Exception\MediaNotFoundException;
use JoliCode\MediaBundle\Library\Library;
Comment thread
BriceFab marked this conversation as resolved.
use JoliCode\MediaBundle\Resolver\Resolver;
use JoliCode\MediaBundle\Storage\OriginalStorage;
use JoliCode\MediaBundle\Variation\Variation;
use Symfony\Component\Routing\Generator\UrlGeneratorInterface;

class Media implements StorableInterface
{
/**
* @var callable(): Resolver|null
*/
public static $resolverInitializer;

private static Resolver $resolver;

/**
* @var array<string, MediaVariation>
*/
Expand All @@ -26,7 +35,35 @@ public function __construct(

public function __serialize(): array
{
return [$this->path, $this->storage->getLibrary()->getName()];
return [
'path' => $this->path,
'library' => $this->storage->getLibrary()->getName(),
Copy link
Copy Markdown
Contributor

@loic425 loic425 Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that'd be cool to serialize the storage to be able to deserialize the media without the resolver.

Suggested change
'library' => $this->storage->getLibrary()->getName(),
'storage' => $this->storage->__serialize(),

Copy link
Copy Markdown
Contributor

@loic425 loic425 Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hum the serialize method on the storage seems not ok...

];
}

/**
* @param array<int|string, mixed> $data
*/
public function __unserialize(array $data): void
{
$path = $data['path'] ?? $data[0] ?? null;
$libraryName = $data['library'] ?? $data[1] ?? null;

if (!\is_string($path) || !\is_string($libraryName)) {
throw new \UnexpectedValueException('Invalid serialized media payload.');
}

try {
$resolvedMedia = $this->getResolver()->resolveMedia($path, $libraryName);
} catch (MediaNotFoundException) {
$resolvedMedia = $this->getResolver()->createUnresolvedMedia($path, $libraryName);
}
Copy link
Copy Markdown
Member

@damienalexandre damienalexandre Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xavierlacot Any opinion about this resolver embedded in the DTO? 🤔 I'm not sure we need to force those resolution when unserializing. Having the resolver attached to the DTO feels weird.

Copy link
Copy Markdown
Member

@damienalexandre damienalexandre Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When we store something with a state, there is no reason to retrieve the same object from cache with a different state. It should be the same state, the same object.

Introducing a resolver here leads to unexpected output from the cache.

For example, when you get your doctrine entity from the cache, there is no refresh with the original entity from the table, you get what you stored.

Copy link
Copy Markdown
Contributor

@loic425 loic425 Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we could serialize the storage into the __serialize method and then we will be able to deserialize the media without the resolver.

if (!isset($data['storage']) {
    throw Invalid...
}

$this->path = $data['path'];
$this->storage = $data['storage'];


$this->path = $resolvedMedia->path;
$this->storage = $resolvedMedia->storage;
$this->binary = null;
$this->stored = null;
$this->variations = [];
}

public function addVariation(MediaVariation $variation): void
Expand Down Expand Up @@ -214,4 +251,17 @@ public function store(?Binary $binary = null): void
$this->storage->createMediaFromBinary($this->path, $this->binary);
$this->stored = true;
}

private function getResolver(): Resolver
{
if (!isset(self::$resolver)) {
if (!isset(self::$resolverInitializer)) {
throw new \LogicException('Resolver Initializer is not set.');
}

self::$resolver = (self::$resolverInitializer)();
}

return self::$resolver;
}
}
4 changes: 2 additions & 2 deletions tests/src/Bridge/EasyAdmin/Controller/MediaAdminControllerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ public function testUploadAtRoot(): void
$responseContent = $this->client->getResponse()->getContent();
$this->assertIsString($responseContent);

$response = json_decode($responseContent, true);
$response = json_decode((string) $responseContent, true);
$this->assertArrayHasKey('files', $response);
$this->assertCount(1, $response['files']);

Expand Down Expand Up @@ -192,7 +192,7 @@ protected static function getKernelClass(): string
private function findInGalleryFromName(Crawler $crawler, string $name): Crawler
{
return $crawler->filter('ul.gallery-grid--files .gallery-grid-item__link')
->reduce(static fn (Crawler $node): bool => str_contains($node->filter('.gallery-grid-item__name')->text(), $name))
->reduce(static fn (Crawler $node): bool => str_contains((string) $node->filter('.gallery-grid-item__name')->text(), $name))
;
}
}
26 changes: 26 additions & 0 deletions tests/src/Model/MediaTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use JoliCode\MediaBundle\Binary\Binary;
use JoliCode\MediaBundle\Model\Format;
use JoliCode\MediaBundle\Model\Media;
use JoliCode\MediaBundle\Resolver\Resolver;
use JoliCode\MediaBundle\Tests\BaseTestCase;

class MediaTest extends BaseTestCase
Expand Down Expand Up @@ -160,4 +161,29 @@ public function testStoreWithNoBinary(): void

$media->store();
}

public function testSerializeUnserializeRestoresMedia(): void
{
Media::$resolverInitializer = fn (): Resolver => $this->resolver;

$restored = unserialize(serialize($this->media));

self::assertInstanceOf(Media::class, $restored);
self::assertSame('test.jpg', $restored->getPath());
self::assertSame('default', $restored->getLibrary()->getName());
}

public function testUnserializeSupportsLegacyIndexedPayload(): void
{
Media::$resolverInitializer = fn (): Resolver => $this->resolver;

$restored = (new \ReflectionClass(Media::class))->newInstanceWithoutConstructor();
$restored->__unserialize([
0 => 'test.jpg',
1 => 'default',
]);

self::assertSame('test.jpg', $restored->getPath());
self::assertSame('default', $restored->getLibrary()->getName());
}
}
2 changes: 0 additions & 2 deletions tests/src/Storage/CacheKeyWithSubfolderTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,6 @@ class CacheKeyWithSubfolderTest extends TestCase

protected function setUp(): void
{
parent::setUp();

$this->cache = new ArrayAdapter();
$this->filesystem = new Filesystem(new InMemoryFilesystemAdapter());
$this->mimeTypeGuesser = new MimeTypeGuesser(new MimeTypes(), new FileBinaryMimeTypeGuesser());
Expand Down
6 changes: 1 addition & 5 deletions tests/src/Twig/Component/ImgTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@ class ImgTest extends BaseTestCase

public static function setUpBeforeClass(): void
{
parent::setUpBeforeClass();

$container = static::getContainer();

/** @var Converter */
Expand Down Expand Up @@ -78,8 +76,6 @@ public static function setUpBeforeClass(): void

public static function tearDownAfterClass(): void
{
parent::tearDownAfterClass();

$container = static::getContainer();

/** @var LibraryContainer */
Expand All @@ -106,7 +102,7 @@ public function testComponents(string $component, array $configuration, string $
);
$crawler = new Crawler(\sprintf('<!DOCTYPE html><html><body>%s</body></html>', $rendered));
$img = $crawler->filterXPath('//body/*')->first();
$html = preg_replace(['/(\n\s*)+/', '/\s+/'], ['', ' '], $img->outerHtml());
$html = preg_replace(['/(\n\s*)+/', '/\s+/'], ['', ' '], (string) $img->outerHtml());

if ($expected !== $html) {
echo "\n\n" . $html . "\n\n";
Expand Down
2 changes: 1 addition & 1 deletion tests/src/Twig/Component/PictureTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ public function testComponents(string $component, array $configuration, string $
);
$crawler = new Crawler(\sprintf('<!DOCTYPE html><html><body>%s</body></html>', $rendered));
$img = $crawler->filterXPath('//body/*')->first();
$html = preg_replace(['/(\n\s*)+/', '/\s+/'], ['', ' '], $img->outerHtml());
$html = preg_replace(['/(\n\s*)+/', '/\s+/'], ['', ' '], (string) $img->outerHtml());

// @phpstan-ignore-next-line
if (Kernel::MAJOR_VERSION < 8 && !(Kernel::MAJOR_VERSION === 7 && Kernel::MINOR_VERSION === 4 && \PHP_VERSION_ID >= 80400)) {
Expand Down
Loading