Skip to content

Commit ca162c1

Browse files
soyukasoyuka
authored
Merge pull request #1892 from bendavies/missing-local-caches
add some local caches where they are missing
2 parents 3ac4c89 + f6bd5fc commit ca162c1

File tree

4 files changed

+100
-53
lines changed

4 files changed

+100
-53
lines changed

‎src/Bridge/Symfony/Routing/CachedRouteNameResolver.php

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ final class CachedRouteNameResolver implements RouteNameResolverInterface
2727

2828
private $cacheItemPool;
2929
private $decorated;
30+
private $localCache = [];
3031

3132
public function __construct(CacheItemPoolInterface $cacheItemPool, RouteNameResolverInterface $decorated)
3233
{
@@ -42,21 +43,33 @@ public function getRouteName(string $resourceClass, $operationType /**, array $c
4243
$context = \func_num_args() > 2 ? func_get_arg(2) : [];
4344
$cacheKey = self::CACHE_KEY_PREFIX.md5(serialize([$resourceClass, $operationType, $context['subresource_resources'] ?? null]));
4445

46+
if (isset($this->localCache[$cacheKey])) {
47+
return $this->localCache[$cacheKey];
48+
}
49+
4550
try {
4651
$cacheItem = $this->cacheItemPool->getItem($cacheKey);
47-
} catch (CacheException $e) {
48-
return $this->decorated->getRouteName($resourceClass, $operationType, $context);
49-
}
5052

51-
if ($cacheItem->isHit()) {
52-
return $cacheItem->get();
53+
if ($cacheItem->isHit()) {
54+
return $this->localCache[$cacheKey] = $cacheItem->get();
55+
}
56+
} catch (CacheException $e) {
57+
//do nothing
5358
}
5459

5560
$routeName = $this->decorated->getRouteName($resourceClass, $operationType, $context);
5661

57-
$cacheItem->set($routeName);
58-
$this->cacheItemPool->save($cacheItem);
62+
if (!isset($cacheItem)) {
63+
return $this->localCache[$cacheKey] = $routeName;
64+
}
65+
66+
try {
67+
$cacheItem->set($routeName);
68+
$this->cacheItemPool->save($cacheItem);
69+
} catch (CacheException $e) {
70+
// do nothing
71+
}
5972

60-
return $routeName;
73+
return $this->localCache[$cacheKey] = $routeName;
6174
}
6275
}

‎src/Operation/Factory/CachedSubresourceOperationFactory.php

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ final class CachedSubresourceOperationFactory implements SubresourceOperationFac
2525

2626
private $cacheItemPool;
2727
private $decorated;
28+
private $localCache = [];
2829

2930
public function __construct(CacheItemPoolInterface $cacheItemPool, SubresourceOperationFactoryInterface $decorated)
3031
{
@@ -39,21 +40,33 @@ public function create(string $resourceClass): array
3940
{
4041
$cacheKey = self::CACHE_KEY_PREFIX.md5($resourceClass);
4142

43+
if (isset($this->localCache[$cacheKey])) {
44+
return $this->localCache[$cacheKey];
45+
}
46+
4247
try {
4348
$cacheItem = $this->cacheItemPool->getItem($cacheKey);
4449

4550
if ($cacheItem->isHit()) {
46-
return $cacheItem->get();
51+
return $this->localCache[$cacheKey] = $cacheItem->get();
4752
}
4853
} catch (CacheException $e) {
49-
return $this->decorated->create($resourceClass);
54+
// do nothing
5055
}
5156

5257
$subresourceOperations = $this->decorated->create($resourceClass);
5358

54-
$cacheItem->set($subresourceOperations);
55-
$this->cacheItemPool->save($cacheItem);
59+
if (!isset($cacheItem)) {
60+
return $this->localCache[$cacheKey] = $subresourceOperations;
61+
}
62+
63+
try {
64+
$cacheItem->set($subresourceOperations);
65+
$this->cacheItemPool->save($cacheItem);
66+
} catch (CacheException $e) {
67+
// do nothing
68+
}
5669

57-
return $subresourceOperations;
70+
return $this->localCache[$cacheKey] = $subresourceOperations;
5871
}
5972
}

‎tests/Bridge/Symfony/Routing/CachedRouteNameResolverTest.php

Lines changed: 42 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
use ApiPlatform\Core\Exception\InvalidArgumentException;
2020
use PHPUnit\Framework\TestCase;
2121
use Prophecy\Argument;
22+
use Psr\Cache\CacheException;
2223
use Psr\Cache\CacheItemInterface;
2324
use Psr\Cache\CacheItemPoolInterface;
2425

@@ -63,39 +64,39 @@ public function testGetRouteNameForItemRouteWithNoMatchingRoute()
6364
public function testGetRouteNameForItemRouteOnCacheMiss()
6465
{
6566
$cacheItemProphecy = $this->prophesize(CacheItemInterface::class);
66-
$cacheItemProphecy->isHit()->willReturn(false)->shouldBeCalled();
67-
$cacheItemProphecy->set('some_item_route')->shouldBeCalled();
67+
$cacheItemProphecy->isHit()->willReturn(false)->shouldBeCalledTimes(1);
68+
$cacheItemProphecy->set('some_item_route')->shouldBeCalledTimes(1);
6869

6970
$cacheItemPoolProphecy = $this->prophesize(CacheItemPoolInterface::class);
70-
$cacheItemPoolProphecy->getItem(Argument::type('string'))->willReturn($cacheItemProphecy);
71-
$cacheItemPoolProphecy->save($cacheItemProphecy)->willReturn(true)->shouldBeCalled();
71+
$cacheItemPoolProphecy->getItem(Argument::type('string'))->shouldBeCalledTimes(1)->willReturn($cacheItemProphecy);
72+
$cacheItemPoolProphecy->save($cacheItemProphecy)->shouldBeCalledTimes(1)->willReturn(true);
7273

7374
$decoratedProphecy = $this->prophesize(RouteNameResolverInterface::class);
74-
$decoratedProphecy->getRouteName('AppBundle\Entity\User', false, [])->willReturn('some_item_route')->shouldBeCalled();
75+
$decoratedProphecy->getRouteName('AppBundle\Entity\User', false, [])->willReturn('some_item_route')->shouldBeCalledTimes(1);
7576

7677
$cachedRouteNameResolver = new CachedRouteNameResolver($cacheItemPoolProphecy->reveal(), $decoratedProphecy->reveal());
77-
$actual = $cachedRouteNameResolver->getRouteName('AppBundle\Entity\User', false);
7878

79-
$this->assertSame('some_item_route', $actual);
79+
$this->assertSame('some_item_route', $cachedRouteNameResolver->getRouteName('AppBundle\Entity\User', false));
80+
$this->assertSame('some_item_route', $cachedRouteNameResolver->getRouteName('AppBundle\Entity\User', false), 'Trigger the local cache');
8081
}
8182

8283
public function testGetRouteNameForItemRouteOnCacheHit()
8384
{
8485
$cacheItemProphecy = $this->prophesize(CacheItemInterface::class);
85-
$cacheItemProphecy->isHit()->willReturn(true)->shouldBeCalled();
86-
$cacheItemProphecy->get()->willReturn('some_item_route')->shouldBeCalled();
86+
$cacheItemProphecy->isHit()->shouldBeCalledTimes(1)->willReturn(true);
87+
$cacheItemProphecy->get()->shouldBeCalledTimes(1)->willReturn('some_item_route');
8788

8889
$cacheItemPoolProphecy = $this->prophesize(CacheItemPoolInterface::class);
89-
$cacheItemPoolProphecy->getItem(Argument::type('string'))->willReturn($cacheItemProphecy);
90+
$cacheItemPoolProphecy->getItem(Argument::type('string'))->shouldBeCalledTimes(1)->willReturn($cacheItemProphecy);
9091
$cacheItemPoolProphecy->save($cacheItemProphecy)->shouldNotBeCalled();
9192

9293
$decoratedProphecy = $this->prophesize(RouteNameResolverInterface::class);
9394
$decoratedProphecy->getRouteName(Argument::cetera())->shouldNotBeCalled();
9495

9596
$cachedRouteNameResolver = new CachedRouteNameResolver($cacheItemPoolProphecy->reveal(), $decoratedProphecy->reveal());
96-
$actual = $cachedRouteNameResolver->getRouteName('AppBundle\Entity\User', OperationType::ITEM);
9797

98-
$this->assertSame('some_item_route', $actual);
98+
$this->assertSame('some_item_route', $cachedRouteNameResolver->getRouteName('AppBundle\Entity\User', OperationType::ITEM));
99+
$this->assertSame('some_item_route', $cachedRouteNameResolver->getRouteName('AppBundle\Entity\User', OperationType::ITEM), 'Trigger the local cache');
99100
}
100101

101102
/**
@@ -123,38 +124,55 @@ public function testGetRouteNameForCollectionRouteWithNoMatchingRoute()
123124
public function testGetRouteNameForCollectionRouteOnCacheMiss()
124125
{
125126
$cacheItemProphecy = $this->prophesize(CacheItemInterface::class);
126-
$cacheItemProphecy->isHit()->willReturn(false)->shouldBeCalled();
127-
$cacheItemProphecy->set('some_collection_route')->shouldBeCalled();
127+
$cacheItemProphecy->isHit()->shouldBeCalledTimes(1)->willReturn(false);
128+
$cacheItemProphecy->set('some_collection_route')->shouldBeCalledTimes(1);
128129

129130
$cacheItemPoolProphecy = $this->prophesize(CacheItemPoolInterface::class);
130-
$cacheItemPoolProphecy->getItem(Argument::type('string'))->willReturn($cacheItemProphecy);
131-
$cacheItemPoolProphecy->save($cacheItemProphecy)->willReturn(true)->shouldBeCalled();
131+
$cacheItemPoolProphecy->getItem(Argument::type('string'))->shouldBeCalledTimes(1)->willReturn($cacheItemProphecy);
132+
$cacheItemPoolProphecy->save($cacheItemProphecy)->shouldBeCalledTimes(1)->willReturn(true);
132133

133134
$decoratedProphecy = $this->prophesize(RouteNameResolverInterface::class);
134-
$decoratedProphecy->getRouteName('AppBundle\Entity\User', true, [])->willReturn('some_collection_route')->shouldBeCalled();
135+
$decoratedProphecy->getRouteName('AppBundle\Entity\User', true, [])->willReturn('some_collection_route')->shouldBeCalledTimes(1);
135136

136137
$cachedRouteNameResolver = new CachedRouteNameResolver($cacheItemPoolProphecy->reveal(), $decoratedProphecy->reveal());
137-
$actual = $cachedRouteNameResolver->getRouteName('AppBundle\Entity\User', true);
138138

139-
$this->assertSame('some_collection_route', $actual);
139+
$this->assertSame('some_collection_route', $cachedRouteNameResolver->getRouteName('AppBundle\Entity\User', true));
140+
$this->assertSame('some_collection_route', $cachedRouteNameResolver->getRouteName('AppBundle\Entity\User', true), 'Trigger the local cache');
140141
}
141142

142143
public function testGetRouteNameForCollectionRouteOnCacheHit()
143144
{
144145
$cacheItemProphecy = $this->prophesize(CacheItemInterface::class);
145-
$cacheItemProphecy->isHit()->willReturn(true)->shouldBeCalled();
146-
$cacheItemProphecy->get()->willReturn('some_collection_route')->shouldBeCalled();
146+
$cacheItemProphecy->isHit()->willReturn(true)->shouldBeCalledTimes(1);
147+
$cacheItemProphecy->get()->willReturn('some_collection_route')->shouldBeCalledTimes(1);
147148

148149
$cacheItemPoolProphecy = $this->prophesize(CacheItemPoolInterface::class);
149-
$cacheItemPoolProphecy->getItem(Argument::type('string'))->willReturn($cacheItemProphecy);
150+
$cacheItemPoolProphecy->getItem(Argument::type('string'))->shouldBeCalledTimes(1)->willReturn($cacheItemProphecy);
150151
$cacheItemPoolProphecy->save($cacheItemProphecy)->shouldNotBeCalled();
151152

152153
$decoratedProphecy = $this->prophesize(RouteNameResolverInterface::class);
153154
$decoratedProphecy->getRouteName(Argument::cetera())->shouldNotBeCalled();
154155

155156
$cachedRouteNameResolver = new CachedRouteNameResolver($cacheItemPoolProphecy->reveal(), $decoratedProphecy->reveal());
156-
$actual = $cachedRouteNameResolver->getRouteName('AppBundle\Entity\User', OperationType::COLLECTION);
157157

158-
$this->assertSame('some_collection_route', $actual);
158+
$this->assertSame('some_collection_route', $cachedRouteNameResolver->getRouteName('AppBundle\Entity\User', OperationType::COLLECTION));
159+
$this->assertSame('some_collection_route', $cachedRouteNameResolver->getRouteName('AppBundle\Entity\User', OperationType::COLLECTION), 'Trigger the local cache');
160+
}
161+
162+
public function testGetRouteNameWithCacheItemThrowsCacheException()
163+
{
164+
$cacheException = $this->prophesize(CacheException::class);
165+
$cacheException->willExtend(\Exception::class);
166+
167+
$cacheItemPool = $this->prophesize(CacheItemPoolInterface::class);
168+
$cacheItemPool->getItem(Argument::type('string'))->shouldBeCalledTimes(1)->willThrow($cacheException->reveal());
169+
170+
$decoratedProphecy = $this->prophesize(RouteNameResolverInterface::class);
171+
$decoratedProphecy->getRouteName('AppBundle\Entity\User', OperationType::ITEM, [])->willReturn('some_item_route')->shouldBeCalledTimes(1);
172+
173+
$cachedRouteNameResolver = new CachedRouteNameResolver($cacheItemPool->reveal(), $decoratedProphecy->reveal());
174+
175+
$this->assertSame('some_item_route', $cachedRouteNameResolver->getRouteName('AppBundle\Entity\User', OperationType::ITEM));
176+
$this->assertSame('some_item_route', $cachedRouteNameResolver->getRouteName('AppBundle\Entity\User', OperationType::ITEM), 'Trigger the local cache');
159177
}
160178
}

‎tests/Operation/Factory/CachedSubresourceOperationFactoryTest.php

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -29,38 +29,40 @@ class CachedSubresourceOperationFactoryTest extends TestCase
2929
public function testCreateWithItemHit()
3030
{
3131
$cacheItem = $this->prophesize(CacheItemInterface::class);
32-
$cacheItem->isHit()->willReturn(true)->shouldBeCalled();
33-
$cacheItem->get()->willReturn(['foo' => 'bar'])->shouldBeCalled();
32+
$cacheItem->isHit()->willReturn(true)->shouldBeCalledTimes(1);
33+
$cacheItem->get()->willReturn(['foo' => 'bar'])->shouldBeCalledTimes(1);
3434

3535
$cacheItemPool = $this->prophesize(CacheItemPoolInterface::class);
36-
$cacheItemPool->getItem($this->generateCacheKey())->willReturn($cacheItem->reveal())->shouldBeCalled();
36+
$cacheItemPool->getItem($this->generateCacheKey())->willReturn($cacheItem->reveal())->shouldBeCalledTimes(1);
3737

3838
$decoratedSubresourceOperationFactory = $this->prophesize(SubresourceOperationFactoryInterface::class);
3939
$decoratedSubresourceOperationFactory->create()->shouldNotBeCalled();
4040

4141
$cachedSubresourceOperationFactory = new CachedSubresourceOperationFactory($cacheItemPool->reveal(), $decoratedSubresourceOperationFactory->reveal());
42-
$resultedSubresourceOperation = $cachedSubresourceOperationFactory->create(Dummy::class);
4342

44-
$this->assertEquals(['foo' => 'bar'], $resultedSubresourceOperation);
43+
$expectedResult = ['foo' => 'bar'];
44+
$this->assertEquals($expectedResult, $cachedSubresourceOperationFactory->create(Dummy::class));
45+
$this->assertEquals($expectedResult, $cachedSubresourceOperationFactory->create(Dummy::class), 'Trigger the local cache');
4546
}
4647

4748
public function testCreateWithItemNotHit()
4849
{
4950
$cacheItem = $this->prophesize(CacheItemInterface::class);
50-
$cacheItem->isHit()->willReturn(false)->shouldBeCalled();
51-
$cacheItem->set(['foo' => 'bar'])->willReturn($cacheItem->reveal())->shouldBeCalled();
51+
$cacheItem->isHit()->willReturn(false)->shouldBeCalledTimes(1);
52+
$cacheItem->set(['foo' => 'bar'])->willReturn($cacheItem->reveal())->shouldBeCalledTimes(1);
5253

5354
$cacheItemPool = $this->prophesize(CacheItemPoolInterface::class);
54-
$cacheItemPool->getItem($this->generateCacheKey())->willReturn($cacheItem->reveal())->shouldBeCalled();
55-
$cacheItemPool->save($cacheItem->reveal())->willReturn(true)->shouldBeCalled();
55+
$cacheItemPool->getItem($this->generateCacheKey())->willReturn($cacheItem->reveal())->shouldBeCalledTimes(1);
56+
$cacheItemPool->save($cacheItem->reveal())->willReturn(true)->shouldBeCalledTimes(1);
5657

5758
$decoratedSubresourceOperationFactory = $this->prophesize(SubresourceOperationFactoryInterface::class);
58-
$decoratedSubresourceOperationFactory->create(Dummy::class)->shouldBeCalled()->willReturn(['foo' => 'bar']);
59+
$decoratedSubresourceOperationFactory->create(Dummy::class)->shouldBeCalledTimes(1)->willReturn(['foo' => 'bar']);
5960

6061
$cachedSubresourceOperationFactory = new CachedSubresourceOperationFactory($cacheItemPool->reveal(), $decoratedSubresourceOperationFactory->reveal());
61-
$resultedSubresourceOperation = $cachedSubresourceOperationFactory->create(Dummy::class);
6262

63-
$this->assertEquals(['foo' => 'bar'], $resultedSubresourceOperation);
63+
$expectedResult = ['foo' => 'bar'];
64+
$this->assertEquals($expectedResult, $cachedSubresourceOperationFactory->create(Dummy::class));
65+
$this->assertEquals($expectedResult, $cachedSubresourceOperationFactory->create(Dummy::class), 'Trigger the local cache');
6466
}
6567

6668
public function testCreateWithGetCacheItemThrowsCacheException()
@@ -69,15 +71,16 @@ public function testCreateWithGetCacheItemThrowsCacheException()
6971
$cacheException->willExtend(\Exception::class);
7072

7173
$cacheItemPool = $this->prophesize(CacheItemPoolInterface::class);
72-
$cacheItemPool->getItem($this->generateCacheKey())->willThrow($cacheException->reveal())->shouldBeCalled();
74+
$cacheItemPool->getItem($this->generateCacheKey())->willThrow($cacheException->reveal())->shouldBeCalledTimes(1);
7375

7476
$decoratedSubresourceOperationFactory = $this->prophesize(SubresourceOperationFactoryInterface::class);
75-
$decoratedSubresourceOperationFactory->create(Dummy::class)->shouldBeCalled()->willReturn(['foo' => 'bar']);
77+
$decoratedSubresourceOperationFactory->create(Dummy::class)->shouldBeCalledTimes(1)->willReturn(['foo' => 'bar']);
7678

7779
$cachedSubresourceOperationFactory = new CachedSubresourceOperationFactory($cacheItemPool->reveal(), $decoratedSubresourceOperationFactory->reveal());
78-
$resultedSubresourceOperation = $cachedSubresourceOperationFactory->create(Dummy::class);
7980

80-
$this->assertEquals(['foo' => 'bar'], $resultedSubresourceOperation);
81+
$expectedResult = ['foo' => 'bar'];
82+
$this->assertEquals($expectedResult, $cachedSubresourceOperationFactory->create(Dummy::class));
83+
$this->assertEquals($expectedResult, $cachedSubresourceOperationFactory->create(Dummy::class), 'Trigger the local cache');
8184
}
8285

8386
private function generateCacheKey(string $resourceClass = Dummy::class)

0 commit comments

Comments
 (0)