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

fix: add tests for observing purged tags + fix edge cases #5249

Merged
merged 6 commits into from
Jun 22, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
75 changes: 58 additions & 17 deletions api/src/HttpCache/PurgeHttpCacheListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
use Doctrine\ORM\Event\OnFlushEventArgs;
use Doctrine\ORM\Event\PreUpdateEventArgs;
use Doctrine\ORM\Mapping\AssociationMapping;
use Doctrine\ORM\Mapping\ClassMetadataInfo;
use Doctrine\ORM\PersistentCollection;
use FOS\HttpCacheBundle\CacheManager;
use Symfony\Component\PropertyAccess\PropertyAccessorInterface;
Expand Down Expand Up @@ -72,26 +73,37 @@ public function preUpdate(PreUpdateEventArgs $eventArgs): void {
* Collects tags from inserted, updated and deleted entities, including relations.
*/
public function onFlush(OnFlushEventArgs $eventArgs): void {
/** @var EntityManagerInterface */
$em = method_exists($eventArgs, 'getObjectManager') ? $eventArgs->getObjectManager() : $eventArgs->getEntityManager();
$uow = $em->getUnitOfWork();

foreach ($uow->getScheduledEntityInsertions() as $entity) {
$this->gatherResourceTags($entity);
$this->gatherResourceTags($em, $entity);
$this->gatherRelationTags($em, $entity);
}

foreach ($uow->getScheduledEntityUpdates() as $entity) {
$originalEntity = $this->getOriginalEntity($entity, $em);
$this->addTagForItem($entity);
$this->gatherResourceTags($entity, $originalEntity);
$this->gatherResourceTags($em, $entity, $originalEntity);
}

foreach ($uow->getScheduledEntityDeletions() as $entity) {
$originalEntity = $this->getOriginalEntity($entity, $em);
$this->addTagForItem($originalEntity);
$this->gatherResourceTags($originalEntity);
$this->gatherResourceTags($em, $originalEntity);
$this->gatherRelationTags($em, $originalEntity);
}

// trigger cache purges for changes on many-to-many relations
// for some reason, changes to Many-To-Many relations are not included in the preUpdate changeSet
foreach ($uow->getScheduledCollectionUpdates() as $collection) {
$this->addTagsForManyToManyRelations($collection, $collection->getInsertDiff());
$this->addTagsForManyToManyRelations($collection, $collection->getDeleteDiff());
}
foreach ($uow->getScheduledCollectionDeletions() as $collection) {
$this->addTagsForManyToManyRelations($collection, $collection->getDeleteDiff());
}
}

/**
Expand All @@ -101,6 +113,23 @@ public function postFlush(): void {
$this->cacheManager->flush();
}

private function addTagsForManyToManyRelations($collection, $entities) {
$associationMapping = $collection->getMapping();

if (ClassMetadataInfo::MANY_TO_MANY !== $associationMapping['type']) {
return;
}

foreach ($entities as $entity) {
$relatedProperty = $associationMapping['isOwningSide'] ? $associationMapping['inversedBy'] : $associationMapping['mappedBy'];
if (!$relatedProperty) {
continue;
}

$this->addTagForItem($entity, $relatedProperty);
}
}

/**
* Computes the original state of the entity based on the current entity and on the changeset.
*/
Expand All @@ -124,23 +153,35 @@ private function getOriginalEntity($entity, $em) {
* If oldEntity is provided, purge is only done if the IRI of the collection has changed
* (e.g. for updating period on a ScheduleEntry and the IRI changes from /periods/1/schedule_entries to /periods/2/schedule_entries)
*/
private function gatherResourceTags(object $entity, ?object $oldEntity = null): void {
private function gatherResourceTags(EntityManagerInterface $em, object $entity, ?object $oldEntity = null): void {
$entityClass = $this->getObjectClass($entity);
if ($this->resourceClassResolver->isResourceClass($entityClass)) {
$resourceClass = $this->resourceClassResolver->getResourceClass($entity);
$resourceMetadataCollection = $this->resourceMetadataCollectionFactory->create($resourceClass);
$resourceIterator = $resourceMetadataCollection->getIterator();
while ($resourceIterator->valid()) {
/** @var ApiResource $metadata */
$metadata = $resourceIterator->current();

foreach ($metadata->getOperations() ?? [] as $operation) {
if ($operation instanceof GetCollection) {
$this->invalidateCollection($operation, $entity, $oldEntity);
}
if (!$this->resourceClassResolver->isResourceClass($entityClass)) {
return;
}

$resourceClass = $this->resourceClassResolver->getResourceClass($entity);
$this->gatherResourceTagsForClass($resourceClass, $entity, $oldEntity);

// also purge parent classes (e.g. /content_nodes)
$classMetadata = $em->getClassMetadata(ClassUtils::getClass($entity));
foreach ($classMetadata->parentClasses as $parentClass) {
$this->gatherResourceTagsForClass($parentClass, $entity, $oldEntity);
}
}

private function gatherResourceTagsForClass(string $resourceClass, object $entity, ?object $oldEntity = null): void {
$resourceMetadataCollection = $this->resourceMetadataCollectionFactory->create($resourceClass);
$resourceIterator = $resourceMetadataCollection->getIterator();
while ($resourceIterator->valid()) {
/** @var ApiResource $metadata */
$metadata = $resourceIterator->current();

foreach ($metadata->getOperations() ?? [] as $operation) {
if ($operation instanceof GetCollection) {
$this->invalidateCollection($operation, $entity, $oldEntity);
}
$resourceIterator->next();
}
$resourceIterator->next();
}
}

Expand Down
20 changes: 20 additions & 0 deletions api/tests/Api/Categories/CreateCategoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -552,6 +552,26 @@ public function testCreateCategoryFromCopySourceActivityAcrossCamp() {
$this->assertResponseStatusCodeSame(201);
}

public function testCreateCategoryPurgesCacheTags() {
$client = static::createClientWithCredentials();
$purgedCacheTags = &$this->getPurgedCacheTags();

$client->request('POST', '/categories', ['json' => $this->getExampleWritePayload()]);

$this->assertResponseStatusCodeSame(201);

$camp1 = static::getFixture('camp1');
$contentType = static::getFixture('contentTypeSafetyConcept');
self::assertEqualsCanonicalizing([
'/categories',
'/camps/'.$camp1->getId().'/categories',
'/content_nodes',
'/content_node/column_layouts',
$camp1->getId().'#categories',
$contentType->getId().'#categories',
], $purgedCacheTags);
}

/**
* @throws RedirectionExceptionInterface
* @throws DecodingExceptionInterface
Expand Down
23 changes: 23 additions & 0 deletions api/tests/Api/Categories/DeleteCategoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -97,4 +97,27 @@ public function testDeleteCategoryValidatesThatCategoryHasNoActivities() {
'detail' => 'activities: It\'s not possible to delete a category as long as it has an activity linked to it.',
]);
}

public function testDeleteCategoryPurgesCacheTags() {
$client = static::createClientWithCredentials();
$purgedCacheTags = &$this->getPurgedCacheTags();

$category = static::getFixture('categoryWithNoActivities');
$client->request('DELETE', '/categories/'.$category->getId());

$this->assertResponseStatusCodeSame(204);

$camp = $category->getCamp();
$rootContentNode = $category->getRootContentNode();
self::assertEqualsCanonicalizing([
$category->getId(),
'/categories',
'/camps/'.$camp->getId().'/categories',
$camp->getId().'#categories',
'/content_nodes',
'/content_node/column_layouts',
$rootContentNode->getId(),
$rootContentNode->getId().'#rootDescendants',
], $purgedCacheTags);
}
}
26 changes: 26 additions & 0 deletions api/tests/Api/Categories/UpdateCategoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -511,4 +511,30 @@ public function testPatchCategoryValidatesInvalidNumberingStyle() {
],
]);
}

public function testPatchCategoryPurgesCacheTags() {
$client = static::createClientWithCredentials();
$purgedCacheTags = &$this->getPurgedCacheTags();

$category = static::getFixture('category1');
$client->request('PATCH', '/categories/'.$category->getId(), ['json' => [
'short' => 'LP',
'preferredContentTypes' => [
$this->getIriFor('contentTypeColumnLayout'),
$this->getIriFor('contentTypeNotes'),
],
], 'headers' => ['Content-Type' => 'application/merge-patch+json']]);

$this->assertResponseStatusCodeSame(200);

$contentTypeColumnLayout = static::getFixture('contentTypeColumnLayout');
$contentTypeNotes = static::getFixture('contentTypeNotes');
$contentTypeSafetyConcept = static::getFixture('contentTypeSafetyConcept');
self::assertEqualsCanonicalizing([
$category->getId(),
$contentTypeColumnLayout->getId().'#categories',
$contentTypeNotes->getId().'#categories',
$contentTypeSafetyConcept->getId().'#categories', // SafetyConcept was previously in the list, so this is purged because it was removed
], $purgedCacheTags);
}
}
15 changes: 15 additions & 0 deletions api/tests/Api/ContentNodes/CreateContentNodeTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,21 @@ public function testCreateResponseStructureMatchesReadResponseStructure() {
assertThat($createArray, CompatibleHalResponse::isHalCompatibleWith($getItemResponse->toArray()));
}

public function testCreatePurgesCacheTags() {
$client = static::createClientWithCredentials();
$purgedCacheTags = &$this->getPurgedCacheTags();
Copy link
Contributor

Choose a reason for hiding this comment

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

If you don't know the & (dereference) operator, it is difficult to know why this array would be mutated with the $client->request method.

And that it does not work anymore when you call the method a second time

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed with 0d7296c


$client->request('POST', $this->endpoint, ['json' => $this->getExampleWritePayload()]);

$this->assertResponseStatusCodeSame(201);
self::assertEqualsCanonicalizing([
'/content_nodes',
$this->endpoint,
$this->defaultParent->getRoot()->getId().'#rootDescendants',
$this->defaultParent->getId().'#children',
], $purgedCacheTags);
}

public static function getContentNodesWhichCannotHaveChildren(): array {
return [
ContentNode\MaterialNode::class => [
Expand Down
22 changes: 22 additions & 0 deletions api/tests/Api/ECampApiTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
use App\Util\ArrayDeepSort;
use Doctrine\Bundle\DoctrineBundle\DataCollector\DoctrineDataCollector;
use Doctrine\ORM\EntityManagerInterface;
use FOS\HttpCacheBundle\CacheManager;
use Hautelook\AliceBundle\PhpUnit\FixtureStore;
use Hautelook\AliceBundle\PhpUnit\RefreshDatabaseTrait;
use Spatie\Snapshots\MatchesSnapshots;
Expand Down Expand Up @@ -360,6 +361,27 @@ protected function assertMatchesResponseSnapshot(ResponseInterface $response): v
$this->assertMatchesJsonSnapshot($sortedResponseArray);
}

/**
* mocks CacheManager and keeps track of purged cache tags.
*/
protected function &getPurgedCacheTags(): array {
Copy link
Contributor

Choose a reason for hiding this comment

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

to improve this further we could use a class which implements the cacheManager, but allows to get the purged cache tags, instead of sharing a reference to an array

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, good idea. I can give it a try.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed with 0d7296c

$container = static::getContainer();
$cacheManager = $this->createMock(CacheManager::class);

$purgedCacheTags = [];
$cacheManager
->method('invalidateTags')
->willReturnCallback(function ($tags) use (&$purgedCacheTags, $cacheManager) {
$purgedCacheTags = array_unique(array_merge($purgedCacheTags, $tags));

return $cacheManager;
})
;
$container->set(CacheManager::class, $cacheManager);

return $purgedCacheTags;
}

private static function escapeValues(mixed &$object): void {
if (!is_array($object)) {
$object = 'escaped_value';
Expand Down
10 changes: 10 additions & 0 deletions api/tests/HttpCache/PurgeHttpCacheListenerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,8 @@ public function testOnFlush(): void {
$uowMock->method('getScheduledEntityInsertions')->willReturn([$toInsert1, $toInsert2]);
$uowMock->method('getScheduledEntityUpdates')->willReturn([]);
$uowMock->method('getScheduledEntityDeletions')->willReturn([$toDelete1, $toDelete2, $toDeleteNoPurge]);
$uowMock->method('getScheduledCollectionUpdates')->willReturn([]);
$uowMock->method('getScheduledCollectionDeletions')->willReturn([]);
$uowMock->method('getEntityChangeSet')->willReturn([]);

$emProphecy = $this->prophesize(EntityManagerInterface::class);
Expand Down Expand Up @@ -327,6 +329,8 @@ public function testPropertyIsNotAResourceClass(): void {
$uowProphecy->getScheduledEntityInsertions()->willReturn([$containNonResource])->shouldBeCalled();
$uowProphecy->getScheduledEntityDeletions()->willReturn([])->shouldBeCalled();
$uowProphecy->getScheduledEntityUpdates()->willReturn([])->shouldBeCalled();
$uowProphecy->getScheduledCollectionUpdates()->willReturn([])->shouldBeCalled();
$uowProphecy->getScheduledCollectionDeletions()->willReturn([])->shouldBeCalled();

$emProphecy = $this->prophesize(EntityManagerInterface::class);
$emProphecy->getUnitOfWork()->willReturn($uowProphecy->reveal())->shouldBeCalled();
Expand Down Expand Up @@ -361,6 +365,8 @@ public function testInsertingShouldPurgeSubresourceCollections(): void {
$this->uowProphecy->getScheduledEntityInsertions()->willReturn([$toInsert1]);
$this->uowProphecy->getScheduledEntityDeletions()->willReturn([]);
$this->uowProphecy->getScheduledEntityUpdates()->willReturn([])->shouldBeCalled();
$this->uowProphecy->getScheduledCollectionUpdates()->willReturn([]);
$this->uowProphecy->getScheduledCollectionDeletions()->willReturn([]);

// then
$this->cacheManagerProphecy->invalidateTags(['/dummies'])->willReturn($this->cacheManagerProphecy)->shouldBeCalled();
Expand All @@ -384,6 +390,8 @@ public function testDeleteShouldPurgeSubresourceCollections(): void {
$uowMock->method('getScheduledEntityInsertions')->willReturn([]);
$uowMock->method('getScheduledEntityUpdates')->willReturn([]);
$uowMock->method('getScheduledEntityDeletions')->willReturn([$toDelete1]);
$uowMock->method('getScheduledCollectionUpdates')->willReturn([]);
$uowMock->method('getScheduledCollectionDeletions')->willReturn([]);
$uowMock->method('getEntityChangeSet')->willReturn([]);

$this->emProphecy->getUnitOfWork()->willReturn($uowMock)->shouldBeCalled();
Expand Down Expand Up @@ -414,6 +422,8 @@ public function testUpdateShouldPurgeSubresourceCollections(): void {
$uowMock->method('getScheduledEntityInsertions')->willReturn([]);
$uowMock->method('getScheduledEntityUpdates')->willReturn([$toUpdate1]);
$uowMock->method('getScheduledEntityDeletions')->willReturn([]);
$uowMock->method('getScheduledCollectionUpdates')->willReturn([]);
$uowMock->method('getScheduledCollectionDeletions')->willReturn([]);
$uowMock->method('getEntityChangeSet')->willReturn(['relatedDummy' => [$relatedDummyOld, $relatedDummy]]);

$this->emProphecy->getUnitOfWork()->willReturn($uowMock)->shouldBeCalled();
Expand Down
Loading