Skip to content

Commit eaff3fc

Browse files
committed
Address review: inline cache, detect scheduler context, trim README
- Inline the cache read/write in getSenderAddresses instead of the readCache/writeCache helpers. Drop the try/catch around them: a cache failure now fails sender detection entirely rather than silently degrading. - Detect the scheduler context explicitly so a task run manually from the scheduler backend module also uses the longer timeout (not just cron). CLI is detected via PHP_SAPI, the browser case via the request's "module" attribute (identifier "scheduler"). This also removes the Environment::isCli() guard that only existed for the test harness. - Remove the cache note from the README (implementation detail).
1 parent 3e3184d commit eaff3fc

2 files changed

Lines changed: 26 additions & 43 deletions

File tree

Classes/Import/Provider/BrevoSenderProvider.php

Lines changed: 26 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,11 @@
66

77
use Hn\MailSender\Import\SenderAddressSourceProviderInterface;
88
use Hn\MailSender\Import\ValueObject\SenderAddress;
9+
use Psr\Http\Message\ServerRequestInterface;
910
use Psr\Log\LoggerInterface;
11+
use TYPO3\CMS\Backend\Module\ModuleInterface;
1012
use TYPO3\CMS\Core\Cache\CacheManager;
1113
use TYPO3\CMS\Core\Configuration\ExtensionConfiguration;
12-
use TYPO3\CMS\Core\Core\Environment;
1314
use TYPO3\CMS\Core\Http\RequestFactory;
1415

1516
/**
@@ -73,8 +74,9 @@ public function getSenderAddresses(): array
7374
// Include the key in the cache identifier so switching keys (e.g. a
7475
// different Brevo account) never serves senders from the previous one.
7576
$cacheIdentifier = 'brevo_senders_' . sha1($apiKey);
77+
$cache = $this->cacheManager->getCache(self::CACHE_IDENTIFIER);
7678

77-
$cached = $this->readCache($cacheIdentifier);
79+
$cached = $cache->get($cacheIdentifier);
7880
if (is_array($cached)) {
7981
return $this->mapToSenderAddresses($cached);
8082
}
@@ -83,7 +85,7 @@ public function getSenderAddresses(): array
8385

8486
// Cache even an empty result so a misconfigured key or transient outage
8587
// does not trigger a fresh API call on every backend page view.
86-
$this->writeCache($cacheIdentifier, $senders);
88+
$cache->set($cacheIdentifier, $senders, [], self::CACHE_LIFETIME);
8789

8890
return $this->mapToSenderAddresses($senders);
8991
}
@@ -103,32 +105,6 @@ private function getApiKey(): string
103105
}
104106
}
105107

106-
/**
107-
* @return array<int, array{email: string, name: string}>|null Cached senders, or null on miss/error
108-
*/
109-
private function readCache(string $identifier): ?array
110-
{
111-
try {
112-
$value = $this->cacheManager->getCache(self::CACHE_IDENTIFIER)->get($identifier);
113-
return is_array($value) ? $value : null;
114-
} catch (\Throwable) {
115-
// Cache unavailable (not registered, backend error, ...): treat as a miss.
116-
return null;
117-
}
118-
}
119-
120-
/**
121-
* @param array<int, array{email: string, name: string}> $senders
122-
*/
123-
private function writeCache(string $identifier, array $senders): void
124-
{
125-
try {
126-
$this->cacheManager->getCache(self::CACHE_IDENTIFIER)->set($identifier, $senders, [], self::CACHE_LIFETIME);
127-
} catch (\Throwable) {
128-
// Caching is best-effort: never let a cache failure break sender import.
129-
}
130-
}
131-
132108
/**
133109
* Fetch active senders from the Brevo API.
134110
*
@@ -186,22 +162,32 @@ private function fetchActiveSenders(string $apiKey): array
186162
}
187163

188164
/**
189-
* Use a shorter timeout in interactive contexts (e.g. the backend module) and a
190-
* longer one in non-interactive ones (scheduler/CLI cron), where completeness
191-
* matters more than latency.
192-
*
193-
* Environment::isCli() is the idiomatic check and is always initialized at TYPO3
194-
* runtime; it is guarded so a non-initialized environment can never break the import.
165+
* Use a shorter timeout in the interactive backend module, where a user waits
166+
* for the page to render, and a longer one when sender detection runs from the
167+
* scheduler.
195168
*/
196169
private function getRequestTimeout(): int
197170
{
198-
try {
199-
$isCli = Environment::isCli();
200-
} catch (\Throwable) {
201-
$isCli = PHP_SAPI === 'cli';
171+
return $this->isSchedulerContext() ? self::TIMEOUT_BACKGROUND : self::TIMEOUT_INTERACTIVE;
172+
}
173+
174+
/**
175+
* Whether sender detection is currently triggered by the scheduler, either via
176+
* cron (CLI) or run manually from the scheduler backend module. Running a task
177+
* from the browser must not shorten the timeout.
178+
*/
179+
private function isSchedulerContext(): bool
180+
{
181+
// Cron: `typo3 scheduler:run`.
182+
if (PHP_SAPI === 'cli') {
183+
return true;
202184
}
203185

204-
return $isCli ? self::TIMEOUT_BACKGROUND : self::TIMEOUT_INTERACTIVE;
186+
// Manual execution via the scheduler backend module.
187+
$request = $GLOBALS['TYPO3_REQUEST'] ?? null;
188+
$module = $request instanceof ServerRequestInterface ? $request->getAttribute('module') : null;
189+
190+
return $module instanceof ModuleInterface && $module->getIdentifier() === 'scheduler';
205191
}
206192

207193
/**

README.md

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -126,9 +126,6 @@ Notes:
126126
- Only **active** Brevo senders are imported. The import is additive: senders
127127
that are later deactivated or removed in Brevo are **not** automatically
128128
disabled or deleted locally.
129-
- The API response is cached for 60 seconds (in the *system* cache group), which
130-
also rate-limits the API. Flushing the system caches forces an immediate
131-
refresh.
132129

133130
> **More providers welcome.** The importer is built around a simple provider
134131
> interface (`SenderAddressSourceProviderInterface`). If you use another email

0 commit comments

Comments
 (0)