Skip to content

Invalidate dependent elements #37

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

Draft
jan888adams wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

Invalidate dependent elements #37

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e7c60f8
Invalidate dependent elements
jan888adams Aug 7, 2025
507cb99
Refactor test factories
jan888adams Aug 12, 2025
3edf2b2
Add testcases for asset & document invalidation
jan888adams Aug 13, 2025
c38800b
Only invalidate dependencies from objects
jan888adams Aug 20, 2025
af41501
Test invalidation for dependent documents
jan888adams Aug 20, 2025
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
3 changes: 2 additions & 1 deletion config/services.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,8 @@

$services->set('neusta_pimcore_http_cache.element.invalidate_listener', InvalidateElementListener::class)
->arg('$cacheInvalidator', service('neusta_pimcore_http_cache.cache_invalidator'))
->arg('$dispatcher', service('event_dispatcher'));
->arg('$dispatcher', service('event_dispatcher'))
->arg('$elementRepository', service('.neusta_pimcore_http_cache.element.repository'));

$services->set('neusta_pimcore_http_cache.data_collector', DataCollector::class)
->arg('$cacheTagCollector', service('.neusta_pimcore_http_cache.collect_tags_response_tagger'))
Expand Down
38 changes: 36 additions & 2 deletions src/Element/InvalidateElementListener.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use Neusta\Pimcore\HttpCacheBundle\Cache\CacheInvalidator;
use Pimcore\Event\Model\ElementEventInterface;
use Pimcore\Model\Dependency;
use Pimcore\Model\Element\ElementInterface;
use Symfony\Component\EventDispatcher\EventDispatcherInterface;

Expand All @@ -12,6 +13,7 @@ final class InvalidateElementListener
public function __construct(
private readonly CacheInvalidator $cacheInvalidator,
private readonly EventDispatcherInterface $dispatcher,
private readonly ElementRepository $elementRepository,
) {
}

Expand All @@ -21,12 +23,24 @@ public function onUpdate(ElementEventInterface $event): void
return;
}

$this->invalidateElement($event->getElement());
$element = $event->getElement();

$this->invalidateElement($element);

if (ElementType::Object === ElementType::tryFrom($element->getType())) {
Copy link
Member

@jdreesen jdreesen Aug 20, 2025

Choose a reason for hiding this comment

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

Why only for objects?

Copy link
Contributor Author

@jan888adams jan888adams Aug 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

It wouldn’t make sense in every case.
If an object is required by a document, why should all sites that use this object be invalidated?
If a snippet is required by a page, why should all pages containing that snippet be invalidated?
I think we need to carefully consider in which cases this behavior actually makes sense.

Copy link
Member

@jdreesen jdreesen Aug 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

It wouldn’t make sense in every case.

You don't know that. Only the app will know, right?

If an object is required by a document, why should all sites that use this object be invalidated?

Probably because it is somehow used in the document, so when it's not invalidated it displays outdated data?

If a snippet is required by a page, why should all pages containing that snippet be invalidated?

Same reason: why should the page require the snippet if it doesn't use it? And if it does, and it isn't invalidated, it shows outdated data.

I think we need to carefully consider in which cases this behavior actually makes sense.

Here I'm with you. Maybe we need an (easy) mechanism that controls whether the dependencies should be invalidated or not (also: which ones?) 🤔

Copy link
Contributor Author

@jan888adams jan888adams Aug 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, you are right. The examples were not selected carefully.

Indeed, it should be: a Document is required by an Object. That may or may not make sense, but it’s a case the user of the bundle could have control over.

Also, a Page being required by a Snippet doesn’t sound like a standard case. If we want to do that, it should be considered more carefully.

So the idea was: cases where an Object is required by another Object or by a Document are clear. The other cases are not standard and should not be included in our default behavior.

The best way to handle them is, as you said, through configuration.

$this->invalidateDependencies($element->getDependencies());
}
}

public function onDelete(ElementEventInterface $event): void
{
$this->invalidateElement($event->getElement());
$element = $event->getElement();

$this->invalidateElement($element);

if (ElementType::Object === ElementType::tryFrom($element->getType())) {
$this->invalidateDependencies($element->getDependencies());
}
}

private function invalidateElement(ElementInterface $element): void
Expand All @@ -40,4 +54,24 @@ private function invalidateElement(ElementInterface $element): void

$this->cacheInvalidator->invalidate($invalidationEvent->cacheTags());
}

private function invalidateDependencies(Dependency $dependency): void
{
foreach ($dependency->getRequiredBy() as $required) {
if (!isset($required['id'], $required['type'])) {
continue;
}

$element = match (ElementType::tryFrom($required['type'])) {
ElementType::Object => $this->elementRepository->findObject($required['id']),
ElementType::Document => $this->elementRepository->findDocument($required['id']),
ElementType::Asset => $this->elementRepository->findAsset($required['id']),
Copy link
Contributor Author

@jan888adams jan888adams Aug 21, 2025

Choose a reason for hiding this comment

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

We need to check if invalidating an asset makes sense.

default => null,
};

if ($element) {
$this->invalidateElement($element);
}
}
}
}
4 changes: 2 additions & 2 deletions tests/Integration/Configuration/CollectConfigurationDataTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ protected function setUp(): void
])]
public function collects_configuration_data(): void
{
self::arrange(fn () => TestDocumentFactory::simplePage())->save();
self::arrange(fn () => TestDocumentFactory::simplePage(5))->save();

$this->client->request('GET', '/test_document_page');
$this->client->enableProfiler();
Expand All @@ -65,7 +65,7 @@ public function collects_configuration_data(): void
])]
public function does_not_collect_configuration_data_when_profiler_is_disabled(): void
{
self::arrange(fn () => TestDocumentFactory::simplePage())->save();
self::arrange(fn () => TestDocumentFactory::simplePage(5))->save();

$this->client->request('GET', '/test_document_page');
$this->client->enableProfiler();
Expand Down
18 changes: 9 additions & 9 deletions tests/Integration/Helpers/TestAssetFactory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,34 +6,34 @@

final class TestAssetFactory
{
public static function simpleAsset(): Asset
public static function simpleAsset(int $id, string $fileName = 'test-asset.txt'): Asset
{
$asset = new Asset();
$asset->setId(42);
$asset->setFilename('test-asset.txt');
$asset->setId($id);
$asset->setFilename($fileName);
$asset->setParentId(1);
$asset->setData('This is the content of the test asset.');
$asset->setMimetype('text/plain');

return $asset;
}

public static function simpleImage(): Asset\Image
public static function simpleImage(int $id, string $fileName = 'test-asset.jpg'): Asset\Image
{
$image = new Asset\Image();
$image->setId(17);
$image->setFilename('test-asset.jpg');
$image->setId($id);
$image->setFilename($fileName);
$image->setParentId(1);
$image->setMimetype('image/jpeg');

return $image;
}

public static function simpleFolder(): Asset\Folder
public static function simpleFolder(int $id, string $key = 'test-asset-folder'): Asset\Folder
{
$folder = new Asset\Folder();
$folder->setKey('test-asset-folder');
$folder->setId(23);
$folder->setId($id);
$folder->setKey($key);
$folder->setParentId(1);

return $folder;
Expand Down
44 changes: 29 additions & 15 deletions tests/Integration/Helpers/TestDocumentFactory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

namespace Neusta\Pimcore\HttpCacheBundle\Tests\Integration\Helpers;

use Pimcore\Model\DataObject\TestObject;
use Pimcore\Model\Document\Editable\Relation;
use Pimcore\Model\Document\Email;
use Pimcore\Model\Document\Folder;
use Pimcore\Model\Document\Hardlink;
Expand All @@ -10,55 +12,67 @@

final class TestDocumentFactory
{
public static function simplePage(): Page
public static function simplePage(int $id, string $key = 'test_document_page', ?TestObject $relatedObject = null): Page
{
$page = new Page();
$page->setId(42);
$page->setKey('test_document_page');
$page->setId($id);
$page->setKey($key);
$page->setPublished(true);
$page->setParentId(1);

if (null !== $relatedObject) {
$objectRelation = new Relation();
$objectRelation->setName('relatedObject');
$objectRelation->setDataFromResource([
'id' => $relatedObject->getId(),
'type' => 'object',
'subtype' => 'object',
]);

$page->setEditable($objectRelation);
}

return $page;
}

public static function simpleSnippet(): Snippet
public static function simpleSnippet(int $id, string $key = 'test_document_snippet'): Snippet
{
$snippet = new Snippet();
$snippet->setId(23);
$snippet->setKey('test_document_snippet');
$snippet->setId($id);
$snippet->setKey($key);
$snippet->setPublished(true);
$snippet->setParentId(1);

return $snippet;
}

public static function simpleEmail(): Email
public static function simpleEmail(int $id, string $key = 'test_document_email'): Email
{
$email = new Email();
$email->setId(17);
$email->setKey('test_document_link');
$email->setId($id);
$email->setKey($key);
$email->setPublished(true);
$email->setParentId(1);

return $email;
}

public static function simpleHardLink(): Hardlink
public static function simpleHardLink(int $id, string $key = 'test_document_hard_link'): Hardlink
{
$hardlink = new Hardlink();
$hardlink->setId(33);
$hardlink->setKey('test_document_hard_link');
$hardlink->setId($id);
$hardlink->setKey($key);
$hardlink->setPublished(true);
$hardlink->setParentId(1);

return $hardlink;
}

public static function simpleFolder(): Folder
public static function simpleFolder(int $id, string $key = 'test_document_folder'): Folder
{
$folder = new Folder();
$folder->setId(97);
$folder->setKey('test_document_folder');
$folder->setId($id);
$folder->setKey($key);
$folder->setPublished(true);
$folder->setParentId(1);

Expand Down
30 changes: 18 additions & 12 deletions tests/Integration/Helpers/TestObjectFactory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,29 +2,35 @@

namespace Neusta\Pimcore\HttpCacheBundle\Tests\Integration\Helpers;

use Pimcore\Image;
use Pimcore\Model\DataObject;
use Pimcore\Model\DataObject\AbstractObject;
use Pimcore\Model\DataObject\TestDataObject;
use Pimcore\Model\DataObject\TestObject;
use Pimcore\Model\Document\Page;

final class TestObjectFactory
{
public static function simpleObject(): TestDataObject
/**
* @param list<TestObject|Page|Image> $related
*/
public static function simpleObject(int $id, string $key = 'test_object', array $related = []): TestObject
{
$object = new TestDataObject();
$object->setId(42);
$object->setKey('test_object');
$object = new TestObject();
$object->setId($id);
$object->setKey($key);
$object->setContent('Test content');
$object->setRelated($related);
$object->setPublished(true);
$object->setParentId(1);

return $object;
}

public static function simpleVariant(): TestDataObject
public static function simpleVariant(int $id, string $key = 'simple_variant'): TestObject
{
$object = new TestDataObject();
$object->setId(17);
$object->setKey('test_variant');
$object = new TestObject();
$object->setId($id);
$object->setKey($key);
$object->setContent('Test content');
$object->setPublished(true);
$object->setParentId(1);
Expand All @@ -33,11 +39,11 @@ public static function simpleVariant(): TestDataObject
return $object;
}

public static function simpleFolder(): DataObject\Folder
public static function simpleFolder(int $id, string $key = 'simple_folder'): DataObject\Folder
{
$folder = new DataObject\Folder();
$folder->setId(23);
$folder->setKey('test_folder');
$folder->setId($id);
$folder->setKey($key);
$folder->setParentId(1);

return $folder;
Expand Down
12 changes: 6 additions & 6 deletions tests/Integration/Invalidation/CancelInvalidationTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ protected function setUp(): void
])]
public function cancel_invalidation_on_object_update(): void
{
$object = self::arrange(fn () => TestObjectFactory::simpleObject()->save());
$object = self::arrange(fn () => TestObjectFactory::simpleObject(42)->save());

$object->setContent('Updated test content')->save();

Expand All @@ -62,7 +62,7 @@ public function cancel_invalidation_on_object_update(): void
])]
public function cancel_invalidation_on_document_update(): void
{
$document = self::arrange(fn () => TestDocumentFactory::simplePage()->save());
$document = self::arrange(fn () => TestDocumentFactory::simplePage(5)->save());

$document->setKey('updated_test_document_page')->save();

Expand All @@ -79,7 +79,7 @@ public function cancel_invalidation_on_document_update(): void
])]
public function cancel_invalidation_on_asset_update(): void
{
$asset = self::arrange(fn () => TestAssetFactory::simpleAsset()->save());
$asset = self::arrange(fn () => TestAssetFactory::simpleAsset(5)->save());

$asset->setData('Updated test content')->save();

Expand All @@ -96,7 +96,7 @@ public function cancel_invalidation_on_asset_update(): void
])]
public function cancel_invalidation_on_object_delete(): void
{
$object = self::arrange(fn () => TestObjectFactory::simpleObject()->save());
$object = self::arrange(fn () => TestObjectFactory::simpleObject(42)->save());

$object->delete();

Expand All @@ -113,7 +113,7 @@ public function cancel_invalidation_on_object_delete(): void
])]
public function cancel_invalidation_on_document_delete(): void
{
$document = self::arrange(fn () => TestDocumentFactory::simplePage()->save());
$document = self::arrange(fn () => TestDocumentFactory::simplePage(5)->save());

$document->delete();

Expand All @@ -130,7 +130,7 @@ public function cancel_invalidation_on_document_delete(): void
])]
public function cancel_invalidation_on_asset_delete(): void
{
$asset = self::arrange(fn () => TestAssetFactory::simpleAsset()->save());
$asset = self::arrange(fn () => TestAssetFactory::simpleAsset(5)->save());

$asset->delete();

Expand Down
Loading