Repository navigation
fix: cache DeepL translations per text and translate missing texts - #8
Conversation
The cache stored a complete response per object and language and returned it regardless of the requested texts. Texts outside the post content (menu, footer, news list, greetings) that changed after caching were never translated, and texts cached by logged-in users (admin bar) leaked into visitor responses. Cached translations are now looked up per text. Missing texts are sent to DeepL, merged into the cache when the user may cache, and the response contains exactly the requested texts. Requests with missing texts count towards the rate limit; when rate limited, the cached texts are returned instead of a 429.
Composer package changes
|
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Exempt requests still consume the shared rate limit, and concurrent cache writes can lose translations.
Review effort: Balanced
Findings: 2
Open (2)
What changed in this PR
Updates translation caching to operate per text, requesting only cache misses from DeepL and returning cached results when rate-limited.
Changes:
- Adds per-text cache lookup and merging.
- Revises rate-limit behavior for cached requests.
- Adds Pest unit tests and documentation.
| File | Description |
|---|---|
src/Services/TranslationService.php |
Implements per-text caching and merging. |
src/Controllers/RestAPIController.php |
Handles cache-aware rate limiting. |
tests/Unit/TranslationServiceTest.php |
Tests translation cache behavior. |
tests/Unit/RestAPIControllerTest.php |
Tests rate-limit behavior. |
tests/Pest.php |
Provides WordPress test doubles. |
phpunit.xml |
Configures the unit-test suite. |
composer.json |
Adds Pest and a test command. |
composer.lock |
Locks testing dependencies. |
README.md |
Documents the updated behavior. |
readme.txt |
Updates WordPress plugin documentation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| // Apply rate limit check if object ID is absent or translation is not cached when an object ID is present. | ||
| if ( ! $cached_translation ) { | ||
| if ( $this->is_rate_limit_exceeded() && ! $user_has_cache_capability ) { | ||
| if ( array() !== $missing_texts && $this->is_rate_limit_exceeded() && ! $user_has_cache_capability ) { |
There was a problem hiding this comment.
Aangepast, de capability-check staat nu vooraan, dus editors hogen de teller niet meer op. Er is een test bij die dat afdekt.
| if ( $cache ) { | ||
| $this->repository->store_translation( $object_id, $target_lang, $merged ); |
There was a problem hiding this comment.
Bewust zo gelaten. Alleen gebruikers met de cache-capability schrijven naar de cache, dus gelijktijdige writes zijn zeldzaam. Gaat het toch mis, dan wordt een tekst bij het volgende editorbezoek opnieuw vertaald en opgeslagen. Een lock of losse meta per tekst is die complexiteit niet waard.
Visitors without the cache capability never write to the cache, so any text missing from an editor's snapshot made every language switch count towards the rate limit and ended in a 429 after three switches. When the cache contains any of the requested texts, visitors now get those cached texts only, without a DeepL call or rate limit hit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
mvdhoek1
left a comment
mvdhoek1
left a comment
There was a problem hiding this comment.
Verhoging van de rate limit lijkt mij ook handiger!
|
|
||
| return $translation; | ||
| /** | ||
| * @since 2.2.0 |
There was a problem hiding this comment.
Waarschijnlijk gaat niemand conflicteren met andere versies maar in feature branches zou ik @since NEXT gebruiken. (Alleen niet vergeten om deze aan te passen na de release)
There was a problem hiding this comment.
Aangepast naar @since NEXT.
| */ | ||
| public function missing_texts( array $text, array $cached_translation ): array | ||
| { | ||
| return array_values( array_unique( array_diff( $text, array_keys( $this->translations_by_text( $cached_translation ) ) ) ) ); |
There was a problem hiding this comment.
Ik zou hem iets duidelijker schrijven:
public function missing_texts( array $text, array $cached_translation ): array
{
$translations_by_text = $this->translations_by_text( $cached_translation );
$cached_texts = array_keys( $translations_by_text );
$missing_texts = array_diff( $text, $cached_texts );
$unique_missing_texts = array_unique( $missing_texts );
return array_values( $unique_missing_texts );
}| } | ||
| } | ||
|
|
||
| if ( array() !== $missing_texts && $this->is_rate_limit_exceeded() && ! $user_has_cache_capability ) { |
There was a problem hiding this comment.
Foei! Ik zie dat ik had moeten beginnen met ! $user_has_cache_capability. Nu hogen we de rate limit ook op voor editors.
There was a problem hiding this comment.
Aangepast, de capability-check staat nu vooraan, dus editors hogen de teller niet meer op. Er is een test bij die dat afdekt.
| $cached_texts = $this->service->translations_for( $text, $cached_translation ?? array() ); | ||
|
|
||
| if ( array() !== $cached_texts ) { | ||
| return new WP_REST_Response( $cached_texts ); |
There was a problem hiding this comment.
Hierdoor zien editors in de admin columns niet meer hoe volledig een cache is. Een deels gevulde cache staat gewoon als "gecachet" en bezoekers die alleen de gecachete subset krijgen verhogen de teller niet.
Omdat ontbrekende teksten pas vertaald worden bij een editorbezoek, moeten editors kunnen zien welke pagina's (en talen) dat nodig hebben.
Wellicht een aparte teller hiervoor toevoegen?
There was a problem hiding this comment.
Goed punt. Er is nu een aparte teller per taal voor bezoekers die maar een deel van hun teksten uit de cache kregen. In de kolom staat die taal dan geel met het aantal, in plaats van groen. Een editorbezoek dat de ontbrekende teksten opslaat of het wissen van de cache zet de teller terug.
| - **Serving Cached Translations:** If a cached translation is newer than the `post_modified` date of the object, the cached version is served. | ||
| - **Serving Cached Translations:** Translations are cached per text. If a cached translation is newer than the `post_modified` date of the object, every requested text found in the cache is served from it. Texts that are not cached yet (for example a changed menu or footer) are fetched from DeepL and merged into the cache. The response contains exactly the requested texts. | ||
| - **Fetching New Translations:** When the `post_modified` date of the object is more recent than the cached translation, a new translation is fetched from DeepL. Once retrieved, this translation is immediately cached for future use. | ||
| - **Cache Authorization:** Only logged-in users with the `edit_posts` capability (or a custom capability configured via the `yard::deepl/cache_capability` filter) are permitted to create new cache entries. Requests from users without this capability will still return a translation from DeepL, but the result will not be stored in the cache. |
There was a problem hiding this comment.
Deze regel klopt niet meer met deze PR. Graag dezelfde aanpassing ook in readme.txt.
| - **Cache Authorization:** Only logged-in users with the `edit_posts` capability (or a custom capability configured via the `yard::deepl/cache_capability` filter) are permitted to create new cache entries. Requests from users without this capability will still return a translation from DeepL, but the result will not be stored in the cache. | |
| - **Cache Authorization:** Only logged-in users with the `edit_posts` capability (or a custom capability configured via the `yard::deepl/cache_capability` filter) are permitted to create new cache entries. Requests from users without this | |
| capability never write to the cache: when any of the requested texts are cached, only those cached texts are returned; when none are cached, the texts are translated by DeepL without storing the result. |
There was a problem hiding this comment.
Overgenomen in README.md en readme.txt, met daarbij een uitleg van de gele badge.
…e limit Requests from users with the cache capability incremented the per-IP rate limit counter before the capability was checked, so editors used up the budget of visitors behind the same IP. Visitors served only part of their texts from the cache left no trace, so a partially filled cache showed as fully cached in the admin column. Those requests are now counted per language and shown as a yellow badge until an editor visit stores the missing texts or the cache is cleared. Also writes out missing_texts() step by step and marks new methods with @SInCE NEXT, as requested in review.
Clearing the cache via the metabox left _ydpl_uncached_request_count_<lang> behind, so the grey badge kept showing an old count for a post without cache.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… post types Visitors without the cache capability now receive texts missing from the cache from DeepL, merged with the cached texts but not stored. When the rate limit is reached they fall back to the cached texts only. The cache metabox and translation cache column now default to every public post type except attachments, so editors also see the cache status on custom post types such as pdc-item. Columns are registered on admin_init so post types registered later on init are included. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
De cache bewaarde de complete respons per pagina en gaf die terug ongeacht welke teksten de client stuurde. Menu, footer, nieuwslijst of groet die na het cachen wijzigen werden daardoor nooit vertaald, en admin-bar-teksten van de editor die de cache vulde kwamen mee in de respons voor bezoekers. Op geertruidenberg.nl ontbraken zo 9 teksten op de EN-GB homepage. Nu wordt per tekst opgezocht en gaan alleen de ontbrekende teksten naar DeepL. Zie ZOHO#11191 https://support.yard.nl/support/yarddigital/ShowHomePage.do#Cases/dv/300585000070220196
Bezoekers zonder
edit_postsschrijven niets naar de cache. Staat een deel van hun teksten in de cache, dan krijgen ze alleen dat deel terug, zonder DeepL-call en zonder dat het meetelt voor de rate limit. Ontbrekende teksten worden dus pas vertaald zodra een editor de pagina bezoekt. Zodat editors zien welke pagina's dat nodig hebben, krijgt een gecachete taal in de admin-kolom een gele badge met het aantal bezoekers dat een onvolledig antwoord kreeg.De rate limit voor bezoekers gaat van 3 naar 9 per minuut, omdat een request nu meetelt zodra geen van de gevraagde teksten gecachet is. Editors tellen niet meer mee.
Live op geertruidenberg.nl: TR is geel omdat een bezoeker daar een onvolledig antwoord kreeg.