🐛 fix: bypass DAO cache in isSegmentDisabled to prevent stale disabled …#4552
🐛 fix: bypass DAO cache in isSegmentDisabled to prevent stale disabled …#4552mauretto78 wants to merge 1 commit intodevelopfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes isSegmentDisabled returning stale false values by bypassing the DAO-level Redis query cache (default 1-week TTL) when checking translation_disabled, relying instead on the trait’s own cache layer/invalidation.
Changes:
- Bypass
SegmentMetadataDao::get()caching inisSegmentDisabled()by passingttl = 0. - Ensure disabled state is determined from fresh DB reads on cache miss, then stored in the trait cache.
| if (empty($cachedValue)) { | ||
| $metadataList = SegmentMetadataDao::get($id_segment, 'translation_disabled'); | ||
| $metadataList = SegmentMetadataDao::get($id_segment, 'translation_disabled', 0); | ||
| $metadata = $metadataList[0] ?? null; | ||
| $isDisabled = (($metadata->meta_value ?? '0') === '1'); |
There was a problem hiding this comment.
isSegmentDisabled()'s cache-miss path behavior changed (DAO cache bypass via ttl=0), but the existing unit tests for this trait only cover the Redis-hit path and skip DB/DAO interactions. Please add coverage for the cache-miss branch (e.g., by extracting the SegmentMetadataDao::get(...) call into an overridable method or injecting a DAO so the test can assert the ttl=0 behavior without a real DB).
🧪 Test-Guard Report❌ FAIL — Some changed source files lack adequate test coverage. Coverage Analysis: ❌ FAILNo changed source files found in coverage report (threshold: 80%)
Test File Matching:
|
| File | Verdict | Reason |
|---|---|---|
lib/Controller/Traits/SegmentDisabledTrait.php |
Test file exists (tests/unit/Traits/SegmentDisabledTraitTest.php) but was not modified in this PR |
Per-File Evaluation: ❌ FAIL
All files resolved by deterministic shortcuts.
| File | Verdict | Reason |
|---|---|---|
lib/Controller/Traits/SegmentDisabledTrait.php |
❌ fail | shortcut → no relevant tests in PR and no/low coverage |
Result: ❌ FAIL
Why this FAIL?
- Coverage Analysis: lib/Controller/Traits/SegmentDisabledTrait.php is missing from the coverage report, indicating a coverage configuration issue → update coverage instrumentation or filters to include this file.
- Test File Matching: The existing test file tests/unit/Traits/SegmentDisabledTraitTest.php was not modified in this PR → no action needed; tests exist but were not changed.
- Per-File Evaluation: No relevant tests were added or modified in this PR for lib/Controller/Traits/SegmentDisabledTrait.php, and coverage is missing → add or modify tests to cover changes or fix coverage config.
To resolve: update coverage configuration to include lib/Controller/Traits/SegmentDisabledTrait.php and add or modify tests to cover the changes.
Summary
Fix
isSegmentDisabledreturning stalefalsevalues due to DAO-level cache (1-week TTL) not being invalidated when a segment is disabled.Type
fix— bug fixChanges
lib/Controller/Traits/SegmentDisabledTrait.phpttl: 0toSegmentMetadataDao::get()to bypass the DAO cache, which was serving stale results for up to 1 weekTesting
vendor/bin/phpunit --exclude-group=ExternalServices --no-coveragepasses./vendor/bin/phpstanpasses (0 errors, with baseline)Verified that after disabling a segment via
setTranslationDisabled, thesegment-analysisAPI correctly returnsdisabled: truewithout waiting for cache expiry.AI Disclosure
GitHub Copilot
Notes
Root cause:
SegmentMetadataDao::get()defaults to a 604800s (1 week) TTL. WhenisSegmentDisabledqueried a segment before it was disabled, the DAO cache stored an empty result for 1 week. Subsequent calls tosetTranslationDisabledwrote the DB row but never invalidated this specific DAO cache entry, soisSegmentDisabledkept returningfalse.Fix: Since
isSegmentDisabledalready manages its own Redis cache layer (1-hour TTL) with proper invalidation viadestroySegmentDisabledCacheandsaveSegmentDisabledInCache, the DAO-level cache is redundant. Settingttl: 0bypasses it.