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.

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

Jump to bottom

Put into cache using root entity name #8009

Closed
wants to merge 8 commits into from
Closed

Put into cache using root entity name #8009

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
a999918
Put into cache using root entity name
goodtravelsoftware-zz Jan 31, 2020
87292e8
Coding standards fixes
goodtravelsoftware-zz Jan 31, 2020
42f20f7
CS fix
peterkeatingie Jan 31, 2020
8120eb3
Fix scrutinizer issues
peterkeatingie Feb 1, 2020
c813064
Move method call, fix alignment
goodtravelsoftware-zz Feb 4, 2020
f8c1f0a
Add return type
goodtravelsoftware-zz Feb 4, 2020
ba0785c
Add space before colon
goodtravelsoftware-zz Feb 4, 2020
e520a98
Use property directly. Update test.
peterkeatingie Feb 13, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion lib/Doctrine/ORM/Cache/DefaultQueryCache.php
View file
Open in desktop
Expand Up @@ -279,9 +279,11 @@ public function put(QueryCacheKey $key, ResultSetMapping $rsm, $result, array $h

$region = $persister->getCacheRegion();

$cm = $this->em->getClassMetadata($entityName);

foreach ($result as $index => $entity) {
$identifier = $this->uow->getEntityIdentifier($entity);
$entityKey = new EntityCacheKey($entityName, $identifier);
$entityKey = new EntityCacheKey($cm->getMetadataValue('rootEntityName'), $identifier);
Copy link
Member

Choose a reason for hiding this comment

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

Why do you call $cm->getMetadataValue('rootEntityName') inside the loop?

Can you please also put the = in line? Probably removing spaces of $identifier

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At first I called the property of the metadata directly and then changed to the method call in situ, hence remaining inside the loop. I see your point though, no need to call each time. Updated now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed the alignment too.


if (($key->cacheMode & Cache::MODE_REFRESH) || ! $region->contains($entityKey)) {
// Cancel put result if entity put fail
Expand Down
48 changes: 48 additions & 0 deletions tests/Doctrine/Tests/ORM/Functional/Ticket/DDC7969Test.php
View file
Open in desktop
@@ -0,0 +1,48 @@
<?php

declare(strict_types=1);

namespace tests\Doctrine\Tests\ORM\Functional\Ticket;

use Doctrine\ORM\Cache\Region\DefaultMultiGetRegion;
use Doctrine\Tests\Models\Cache\Attraction;
use Doctrine\Tests\Models\Cache\Bar;
use Doctrine\Tests\ORM\Functional\SecondLevelCacheAbstractTest;

class DDC7969Test extends SecondLevelCacheAbstractTest
{
public function testChildEntityRetrievedFromCache()
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
public function testChildEntityRetrievedFromCache()
public function testChildEntityRetrievedFromCache() : void

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated now.

{
$this->loadFixturesCountries();
$this->loadFixturesStates();
$this->loadFixturesCities();
$this->loadFixturesAttractions();

// Entities are already cached due to fixtures - hence flush before testing
$region = $this->cache->getEntityCacheRegion(Attraction::class);

if ($region instanceof DefaultMultiGetRegion) {
$region->getCache()->flushAll();
}

/** @var Bar $bar */
$bar = $this->attractions[0];

$repository = $this->_em->getRepository(Bar::class);

$this->assertFalse($this->cache->containsEntity(Bar::class, $bar->getId()));
beberlei marked this conversation as resolved.
Show resolved Hide resolved

$repository->findOneBy([
'name' => $bar->getName(),
]);

$this->assertTrue($this->cache->containsEntity(Bar::class, $bar->getId()));

$repository->findOneBy([
'name' => $bar->getName(),
]);

// One hit for entity cache, one hit for query cache
$this->assertEquals(2, $this->secondLevelCacheLogger->getHitCount());
}
}