Skip to content

Commit b6d844d

Browse files
turegjorupturegjorup
authored
Merge pull request os2display#385 from os2display/feature/feed-caching-refactor
Feed caching refactor and HTTP client logging improvements
2 parents f25214e + 9d7b17d commit b6d844d

5 files changed

Lines changed: 108 additions & 153 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,15 @@ All notable changes to this project will be documented in this file.
44

55
## [Unreleased]
66

7+
- [#385](https://github.com/os2display/display-api-service/pull/385)
8+
- Replaced PSR-6 caching with Symfony `CacheInterface::get()` in
9+
`FeedService` and `EventDatabaseApiV2FeedType` for stampede prevention.
10+
- Renamed HTTP client logging channel from `http_client` to `app_http` to separate from Symfony's built-in logging.
11+
- Injected container-managed `HttpClientInterface` into `RssFeedType` for logging coverage.
12+
- Improved `CalendarApiFeedType` error logging with feed ID, tenant key, and exception context.
13+
- Added default TTL (24h) to `feed.without.expire.cache` pool as safety net for orphaned keys.
14+
- Added Redis `maxmemory` and `allkeys-lru` eviction policy to dev config.
15+
- Added unit tests for `LoggingHttpClient`.
716
- [#383](https://github.com/os2display/display-api-service/pull/383)
817
- Fixed `testUnlinkSlide` using same slide for both lookups, causing "Relation not found" failure.
918
- [#382](https://github.com/os2display/display-api-service/pull/382)

‎config/packages/cache.yaml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@ framework:
1919

2020
feed.without.expire.cache:
2121
adapter: cache.adapter.redis
22+
# Safety net (24 hours) — code sets explicit TTL on items,
23+
# but this ensures orphaned keys don't persist indefinitely.
24+
default_lifetime: 86400
2225

2326
# Creates a "calendar.api.cache" service
2427
calendar.api.cache:

‎docker-compose.override.yml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ services:
1111

1212
redis:
1313
image: 'redis:6'
14+
command: redis-server --maxmemory 128mb --maxmemory-policy allkeys-lru
1415
networks:
1516
- app
1617
ports:

‎src/Feed/EventDatabaseApiV2FeedType.php‎

Lines changed: 80 additions & 125 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,13 @@
99
use App\Feed\OutputModel\Poster\PosterOutput;
1010
use App\Service\FeedService;
1111
use Doctrine\ORM\EntityManagerInterface;
12-
use Psr\Cache\CacheItemPoolInterface;
1312
use Psr\Log\LoggerInterface;
1413
use Symfony\Component\HttpClient\Exception\ClientException;
1514
use Symfony\Component\HttpFoundation\Request;
1615
use Symfony\Component\HttpFoundation\Response;
1716
use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
17+
use Symfony\Contracts\Cache\CacheInterface;
18+
use Symfony\Contracts\Cache\ItemInterface;
1819

1920
/**
2021
* @see https://github.com/itk-dev/event-database-api
@@ -24,16 +25,14 @@ class EventDatabaseApiV2FeedType implements FeedTypeInterface
2425
{
2526
final public const string SUPPORTED_FEED_TYPE = FeedOutputModels::POSTER_OUTPUT;
2627

27-
private const string CACHE_OPTIONS_PREFIX = 'options_';
28-
private const string CACHE_EXPIRE_SUFFIX = '_expire';
29-
private const int CACHE_TTL = 60 * 60; // An hour.
28+
private const int CACHE_OPTIONS_TTL = 60 * 60; // An hour.
3029

3130
public function __construct(
3231
private readonly FeedService $feedService,
3332
private readonly LoggerInterface $logger,
3433
private readonly EntityManagerInterface $entityManager,
3534
private readonly EventDatabaseApiV2Helper $helper,
36-
private readonly CacheItemPoolInterface $feedWithoutExpireCache,
35+
private readonly CacheInterface $feedWithoutExpireCache,
3736
private readonly int $cacheExpire,
3837
) {}
3938

@@ -42,104 +41,38 @@ public function __construct(
4241
*/
4342
public function getData(Feed $feed): array
4443
{
45-
$cacheKey = 'eventdb2-'.$feed->getId();
46-
$cacheKeyLatestFetch = 'eventdb2-latest-fetch-'.$feed->getId();
47-
48-
$cacheItem = $this->feedWithoutExpireCache->getItem($cacheKey);
49-
$latestFetchCacheItem = $this->feedWithoutExpireCache->getItem($cacheKeyLatestFetch);
50-
51-
// Serve cached item if latestFetchCacheItem has not expired and feed has not changed.
52-
// The expiration is set on latestFetchCacheItem and not cacheItem, so cacheItem can be used as fallback.
53-
if ($latestFetchCacheItem->isHit()) {
54-
// If feed has not been modified since the item was cached.
55-
if ($feed->getModifiedAt()?->format('c') == $latestFetchCacheItem->get()) {
56-
if ($cacheItem->isHit()) {
57-
return $cacheItem->get();
58-
}
59-
}
60-
}
44+
// Include modifiedAt in cache key so the cache naturally invalidates when feed config changes.
45+
$modifiedAt = $feed->getModifiedAt()?->getTimestamp() ?? 0;
46+
$cacheKey = 'eventdb2-'.$feed->getId().'-'.$modifiedAt;
6147

6248
try {
63-
$feedSource = $feed->getFeedSource();
64-
$configuration = $feed->getConfiguration();
65-
66-
if (null === $feedSource) {
67-
throw new \Exception('Feed source is null');
68-
}
69-
70-
if (isset($configuration['posterType'])) {
71-
switch ($configuration['posterType']) {
72-
case 'subscription':
73-
$locations = $configuration['subscriptionPlaceValue'] ?? null;
74-
$organizers = $configuration['subscriptionOrganizerValue'] ?? null;
75-
$tags = $configuration['subscriptionTagValue'] ?? null;
76-
$numberOfItems = isset($configuration['subscriptionNumberValue']) ? (int) $configuration['subscriptionNumberValue'] : 5;
77-
78-
$queryParams = [];
79-
80-
if (is_array($locations) && count($locations) > 0) {
81-
$queryParams['event.location.entityId'] = implode(',', array_map(static fn ($location) => (int) $location['value'], $locations));
82-
}
83-
if (is_array($organizers) && count($organizers) > 0) {
84-
$queryParams['event.organizer.entityId'] = implode(',', array_map(static fn ($organizer) => (int) $organizer['value'], $organizers));
85-
}
86-
if (is_array($tags) && count($tags) > 0) {
87-
$queryParams['event.tags'] = implode(',', array_map(static fn ($tag) => (string) $tag['value'], $tags));
88-
}
89-
90-
$result = $this->getSubscriptionData($feedSource, $queryParams, $numberOfItems);
91-
92-
$posterOutput = (new PosterOutput($result))->toArray();
93-
94-
$cacheItem->set($posterOutput);
95-
$latestFetchCacheItem->expiresAfter($this->cacheExpire)->set($feed->getModifiedAt()?->format('c') ?? '');
96-
$this->feedWithoutExpireCache->save($cacheItem);
97-
$this->feedWithoutExpireCache->save($latestFetchCacheItem);
98-
99-
return $posterOutput;
100-
case 'single':
101-
if (isset($configuration['singleSelectedOccurrence'])) {
102-
$occurrenceId = $configuration['singleSelectedOccurrence'];
49+
return $this->feedWithoutExpireCache->get($cacheKey, function (ItemInterface $item) use ($feed) {
50+
$item->expiresAfter($this->cacheExpire);
10351

104-
$responseData = $this->helper->request($feedSource, 'occurrences', null, $occurrenceId);
105-
$members = $responseData->{'hydra:member'};
52+
$feedSource = $feed->getFeedSource();
53+
$configuration = $feed->getConfiguration();
10654

107-
if (empty($members)) {
108-
return [];
109-
}
110-
111-
$occurrenceData = $members[0];
112-
113-
$result = [];
114-
115-
$occurrence = $this->helper->mapOccurrenceToOutput($occurrenceData);
116-
117-
if (null !== $occurrence) {
118-
$result[] = $occurrence;
119-
}
120-
121-
$posterOutput = (new PosterOutput($result))->toArray();
122-
123-
$cacheItem->set($posterOutput);
124-
$latestFetchCacheItem->expiresAfter($this->cacheExpire)->set($feed->getModifiedAt()?->format('c') ?? '');
125-
$this->feedWithoutExpireCache->save($cacheItem);
126-
$this->feedWithoutExpireCache->save($latestFetchCacheItem);
55+
if (null === $feedSource) {
56+
throw new \Exception('Feed source is null');
57+
}
12758

128-
return $posterOutput;
129-
}
130-
// no break
131-
default:
132-
throw new \Exception('Supported posterType: '.$configuration['posterType'], 400);
59+
if (!isset($configuration['posterType'])) {
60+
return [];
13361
}
134-
}
62+
63+
return match ($configuration['posterType']) {
64+
'subscription' => $this->getSubscriptionPosterOutput($feedSource, $configuration),
65+
'single' => $this->getSinglePosterOutput($feedSource, $configuration),
66+
default => throw new \Exception('Unsupported posterType: '.$configuration['posterType'], 400),
67+
};
68+
});
13569
} catch (\Throwable $throwable) {
136-
// If the content does not exist anymore, unpublished the slide.
70+
// If the content does not exist anymore, unpublish the slide.
13771
if ($throwable instanceof ClientException && Response::HTTP_NOT_FOUND == $throwable->getCode()) {
13872
try {
13973
$slide = $feed->getSlide();
14074

14175
if (null !== $slide) {
142-
// Slide publishedTo is set to now. This will make the slide unpublished from this point on.
14376
$slide->setPublishedTo(new \DateTime('now', new \DateTimeZone('UTC')));
14477
$this->entityManager->flush();
14578

@@ -149,25 +82,70 @@ public function getData(Feed $feed): array
14982
]);
15083
}
15184
} catch (\Exception $exception) {
152-
$this->logger->error('{code}: {message}', [
153-
'code' => $exception->getCode(),
85+
$this->logger->error('EventDatabaseApiV2FeedType: Failed to unpublish slide for feed {feedId}: {message}', [
86+
'feedId' => $feed->getId(),
15487
'message' => $exception->getMessage(),
88+
'exception' => $exception,
15589
]);
15690
}
15791
} else {
158-
$this->logger->error('{code}: {message}', [
159-
'code' => $throwable->getCode(),
92+
$this->logger->error('EventDatabaseApiV2FeedType: Failed to get data for feed {feedId}: {message}', [
93+
'feedId' => $feed->getId(),
16094
'message' => $throwable->getMessage(),
95+
'exception' => $throwable,
16196
]);
16297
}
16398
}
16499

165-
// Fallback option is to return the cached data.
166-
if ($cacheItem->isHit()) {
167-
return $cacheItem->get();
168-
} else {
100+
return [];
101+
}
102+
103+
private function getSubscriptionPosterOutput(FeedSource $feedSource, array $configuration): array
104+
{
105+
$locations = $configuration['subscriptionPlaceValue'] ?? null;
106+
$organizers = $configuration['subscriptionOrganizerValue'] ?? null;
107+
$tags = $configuration['subscriptionTagValue'] ?? null;
108+
$numberOfItems = isset($configuration['subscriptionNumberValue']) ? (int) $configuration['subscriptionNumberValue'] : 5;
109+
110+
$queryParams = [];
111+
112+
if (is_array($locations) && count($locations) > 0) {
113+
$queryParams['event.location.entityId'] = implode(',', array_map(static fn ($location) => (int) $location['value'], $locations));
114+
}
115+
if (is_array($organizers) && count($organizers) > 0) {
116+
$queryParams['event.organizer.entityId'] = implode(',', array_map(static fn ($organizer) => (int) $organizer['value'], $organizers));
117+
}
118+
if (is_array($tags) && count($tags) > 0) {
119+
$queryParams['event.tags'] = implode(',', array_map(static fn ($tag) => (string) $tag['value'], $tags));
120+
}
121+
122+
$result = $this->getSubscriptionData($feedSource, $queryParams, $numberOfItems);
123+
124+
return (new PosterOutput($result))->toArray();
125+
}
126+
127+
private function getSinglePosterOutput(FeedSource $feedSource, array $configuration): array
128+
{
129+
if (!isset($configuration['singleSelectedOccurrence'])) {
169130
return [];
170131
}
132+
133+
$occurrenceId = $configuration['singleSelectedOccurrence'];
134+
$responseData = $this->helper->request($feedSource, 'occurrences', null, $occurrenceId);
135+
$members = $responseData->{'hydra:member'};
136+
137+
if (empty($members)) {
138+
return [];
139+
}
140+
141+
$result = [];
142+
$occurrence = $this->helper->mapOccurrenceToOutput($members[0]);
143+
144+
if (null !== $occurrence) {
145+
$result[] = $occurrence;
146+
}
147+
148+
return (new PosterOutput($result))->toArray();
171149
}
172150

173151
/**
@@ -232,20 +210,9 @@ public function getConfigOptions(Request $request, FeedSource $feedSource, strin
232210
throw new BadRequestHttpException('Unsupported entityType: '.$entityType);
233211
}
234212

235-
$expireCacheItem = $this->feedWithoutExpireCache->getItem($this::CACHE_OPTIONS_PREFIX.$entityType.$this::CACHE_EXPIRE_SUFFIX);
236-
$cacheItem = $this->feedWithoutExpireCache->getItem($this::CACHE_OPTIONS_PREFIX.$entityType);
237-
238-
if ($expireCacheItem->isHit()) {
239-
$result = $expireCacheItem->get();
240-
241-
if ($result > time()) {
242-
if ($cacheItem->isHit()) {
243-
return $cacheItem->get();
244-
}
245-
}
246-
}
213+
return $this->feedWithoutExpireCache->get('options_'.$entityType, function (ItemInterface $item) use ($feedSource, $entityType) {
214+
$item->expiresAfter(self::CACHE_OPTIONS_TTL);
247215

248-
try {
249216
$page = 1;
250217
$results = [];
251218
$itemsPerPage = 50;
@@ -271,20 +238,8 @@ public function getConfigOptions(Request $request, FeedSource $feedSource, strin
271238
}
272239
} while ($fetchMore);
273240

274-
$cacheItem->set($results);
275-
$this->feedWithoutExpireCache->save($cacheItem);
276-
277-
$expireCacheItem->set(time() + $this::CACHE_TTL);
278-
$this->feedWithoutExpireCache->save($expireCacheItem);
279-
280241
return $results;
281-
} catch (\Exception) {
282-
if ($cacheItem->isHit()) {
283-
return $cacheItem->get();
284-
} else {
285-
return [];
286-
}
287-
}
242+
});
288243
} elseif ('subscription' === $name) {
289244
$query = $request->query->all();
290245

‎src/Service/FeedService.php‎

Lines changed: 15 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,16 @@
88
use App\Entity\Tenant\FeedSource;
99
use App\Exceptions\UnknownFeedTypeException;
1010
use App\Feed\FeedTypeInterface;
11-
use Psr\Cache\CacheItemInterface;
12-
use Psr\Cache\CacheItemPoolInterface;
1311
use Symfony\Component\HttpFoundation\Request;
1412
use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
13+
use Symfony\Contracts\Cache\CacheInterface;
14+
use Symfony\Contracts\Cache\ItemInterface;
1515

1616
class FeedService
1717
{
1818
public function __construct(
1919
private readonly iterable $feedTypes,
20-
private readonly CacheItemPoolInterface $feedsCache,
20+
private readonly CacheInterface $feedsCache,
2121
private readonly UrlGeneratorInterface $urlGenerator,
2222
) {}
2323

@@ -90,44 +90,31 @@ public function getFeedSourceConfigUrl(FeedSource $feedSource, string $name): st
9090
*/
9191
public function getData(Feed $feed): ?array
9292
{
93-
// Get feed id.
9493
$feedId = $feed->getId()?->jsonSerialize();
9594

9695
if (is_null($feedId)) {
9796
return null;
9897
}
9998

100-
/** @var CacheItemInterface $cacheItem */
101-
$cacheItem = $this->feedsCache->getItem($feedId);
99+
$feedSource = $feed->getFeedSource();
100+
$feedTypeClassName = $feedSource?->getFeedType();
101+
$feedConfiguration = $feed->getConfiguration();
102102

103-
if ($cacheItem->isHit()) {
104-
/** @var array $data */
105-
$data = $cacheItem->get();
106-
} else {
107-
$feedSource = $feed->getFeedSource();
108-
$feedTypeClassName = $feedSource?->getFeedType();
109-
$feedConfiguration = $feed->getConfiguration();
110-
111-
/** @var FeedTypeInterface $feedType */
112-
foreach ($this->feedTypes as $feedType) {
113-
if ($feedType::class === $feedTypeClassName) {
114-
$data = $feedType->getData($feed);
115-
116-
$cacheItem->set($data);
103+
/** @var FeedTypeInterface $feedType */
104+
foreach ($this->feedTypes as $feedType) {
105+
if ($feedType::class === $feedTypeClassName) {
106+
return $this->feedsCache->get($feedId, function (ItemInterface $item) use ($feed, $feedType, $feedConfiguration) {
117107
if (isset($feedConfiguration['cache_expire'])) {
118-
$cacheItem->expiresAfter($feedConfiguration['cache_expire']);
108+
$item->expiresAfter($feedConfiguration['cache_expire']);
119109
}
120-
$this->feedsCache->save($cacheItem);
121110

122-
return $data;
123-
}
111+
return $feedType->getData($feed);
112+
});
124113
}
125-
126-
// If feed type was not known in the system return null. API platform will convert this to 404 not found.
127-
return null;
128114
}
129115

130-
return $data;
116+
// If feed type was not known in the system return null. API platform will convert this to 404 not found.
117+
return null;
131118
}
132119

133120
/**

0 commit comments

Comments
 (0)