Fix: Cache artikel tidak dihapus setelah operasi hapus#991
Fix: Cache artikel tidak dihapus setelah operasi hapus#991vickyrolanda merged 7 commits intorilis-devfrom
Conversation
…penKab into fix/cache_artikel_tidak_update
|
🔄 AI PR Review sedang antri di server...
|
pandigresik
changed the title
🔒 Security ReviewTotal Temuan: 1 isu (0 Critical, 0 High, 1 Medium)
Catatan Positif:
|
| /** | ||
| * Menghapus cache artikel tunggal berdasarkan ID | ||
| */ | ||
| public function clearCacheSingle(int $id): void |
There was a problem hiding this comment.
[MEDIUM] 🔒 Security: Type Confusion - Missing Array Validation
Masalah: Method clearAllCache() tidak memvalidasi bahwa Cache::get() mengembalikan array sebelum memanggil array_keys(). Jika cache registry di-corrupt atau di-poison dengan non-array value, aplikasi akan crash dengan fatal error.
Kode:
public function clearAllCache(): void
{
$keys = Cache::get($this->cacheRegistryKey, []);
foreach (array_keys($keys) as $key) {
Cache::forget($key);
}
Cache::forget($this->cacheRegistryKey);
}Risiko:
- Denial of Service (DoS): Jika attacker bisa manipulasi cache backend (misalnya via cache poisoning, compromised Redis/Memcached, atau file cache manipulation), mereka bisa inject non-array value ke
artikel_cache_registrykey - Fatal Error:
array_keys()akan throw TypeError jika$keysbukan array, menyebabkan aplikasi crash - Impact: Semua request yang trigger
clearAllCache()(create/edit/delete artikel) akan gagal
PoC (Chrome Console):
// CATATAN: PoC ini memerlukan akses ke cache backend (Redis/Memcached/File)
// Simulasi: Jika attacker punya akses ke cache backend, mereka bisa inject:
// Contoh untuk Redis (via redis-cli):
// SET laravel_cache:artikel_cache_registry "malicious_string"
// Atau untuk File cache (via file system access):
// echo 's:16:"malicious_string";' > storage/framework/cache/data/XX/XX/artikel_cache_registry
// Setelah cache di-poison, trigger clear cache via normal flow:
const resp = await fetch('/master-data-artikel', {
method: 'POST',
headers: {
'Content-Type': 'application/json',
'X-CSRF-TOKEN': document.querySelector('meta[name="csrf-token"]').content
},
body: JSON.stringify({
judul: 'Test Artikel',
konten: 'Test content'
})
});
// Redirect akan trigger clearAllCache() dan aplikasi crash dengan:
// TypeError: array_keys(): Argument #1 ($array) must be of type array, string given
console.log('Status:', resp.status);Fix:
public function clearAllCache(): void
{
$keys = Cache::get($this->cacheRegistryKey, []);
// Validasi bahwa $keys adalah array sebelum diproses
if (!is_array($keys)) {
\Log::warning('Cache registry corrupted, resetting', [
'key' => $this->cacheRegistryKey,
'type' => gettype($keys)
]);
Cache::forget($this->cacheRegistryKey);
return;
}
foreach (array_keys($keys) as $key) {
Cache::forget($key);
}
Cache::forget($this->cacheRegistryKey);
}Alternatif Fix (Defensive):
public function clearAllCache(): void
{
$keys = Cache::get($this->cacheRegistryKey, []);
// Type coercion untuk memastikan selalu array
$keys = is_array($keys) ? $keys : [];
foreach (array_keys($keys) as $key) {
Cache::forget($key);
}
Cache::forget($this->cacheRegistryKey);
}
⚡ Performance ReviewTotal Temuan: 2 isu (0 Critical, 2 High)
|
| /** | ||
| * Mendapatkan detail artikel berdasarkan ID | ||
| */ | ||
| public function artikelById(int $id): ?stdClass |
There was a problem hiding this comment.
[HIGH] ⚡ Performance: Cache Registry Tanpa TTL
Masalah: Menggunakan Cache::forever() untuk menyimpan registry tanpa expiration time. Jika sistem berjalan lama dengan banyak kombinasi filter, registry akan terus tumbuh dan tidak pernah dibersihkan otomatis, berpotensi menyebabkan memory bloat.
Kode:
Cache::forever($this->cacheRegistryKey, $keys);Dampak: Di production dengan traffic tinggi dan banyak variasi filter, registry bisa mencapai ribuan entries. Meskipun di-reset saat clearAllCache(), jika cache jarang di-clear, registry akan terus membesar dan memakan memory cache driver.
Fix:
// Gunakan TTL yang reasonable, misal 7 hari
Cache::put($this->cacheRegistryKey, $keys, now()->addDays(7));
// Atau gunakan sliding expiration
Cache::put($this->cacheRegistryKey, $keys, now()->addHours(24));Alternatif: Implementasi cleanup mechanism untuk registry yang sudah tidak relevan:
private function cleanupRegistry(): void
{
$keys = Cache::get($this->cacheRegistryKey, []);
$validKeys = [];
foreach ($keys as $key => $timestamp) {
// Keep only keys from last 7 days
if ($timestamp > (time() - 604800)) {
$validKeys[$key] = $timestamp;
}
}
Cache::put($this->cacheRegistryKey, $validKeys, now()->addDays(7));
}| return Cache::remember($cacheKey, $this->cacheTtl, function () use ($id): ?stdClass { | ||
| $data = $this->apiRequest('/api/v1/artikel/tampil', [ | ||
| 'id' => $id, | ||
| ]); |
There was a problem hiding this comment.
[HIGH] ⚡ Performance: Unbatched Cache Deletion Loop
Masalah: Loop foreach yang memanggil Cache::forget() untuk setiap key secara individual. Jika registry berisi ratusan atau ribuan keys, ini akan menyebabkan ratusan/ribuan operasi cache delete yang sequential.
Kode:
foreach (array_keys($keys) as $key) {
Cache::forget($key);
}Dampak: Dengan 1000 cache keys, ini akan melakukan 1000 operasi Cache::forget() secara sequential. Di Redis/Memcached, setiap operasi adalah network round-trip. Estimasi: 1000 keys × 1-2ms per operation = 1-2 detik blocking time.
Fix:
// Untuk Redis driver, gunakan pipeline
if (Cache::getStore() instanceof \Illuminate\Cache\RedisStore) {
$redis = Cache::getStore()->connection();
$redis->pipeline(function ($pipe) use ($keys) {
foreach (array_keys($keys) as $key) {
$pipe->del(Cache::getStore()->getPrefix() . $key);
}
});
} else {
// Fallback untuk driver lain
foreach (array_keys($keys) as $key) {
Cache::forget($key);
}
}
// Atau gunakan Cache::many() jika tersedia di driver
Cache::deleteMultiple(array_keys($keys));Catatan: Laravel 10+ mendukung Cache::deleteMultiple() yang otomatis menggunakan batching jika driver support. Pertimbangkan upgrade atau implementasi manual batching untuk Redis/Memcached.
📝 Code Quality ReviewTotal Temuan: 3 isu (0 Critical, 3 High)
|
| /** | ||
| * Mendapatkan detail artikel berdasarkan ID | ||
| */ | ||
| public function artikelById(int $id): ?stdClass |
There was a problem hiding this comment.
[HIGH] 📝 Code Quality: Missing Tests
Kategori: PHP Quality
Masalah: Method registerCacheKey() adalah logika baru yang kritis untuk cache registry pattern, namun tidak memiliki unit test. Method ini bertanggung jawab mencatat setiap cache key yang dibuat - jika gagal, seluruh sistem cache clearing akan broken.
Kode:
private function registerCacheKey(string $key): void
{
$keys = Cache::get($this->cacheRegistryKey, []);
$keys[$key] = time();
Cache::forever($this->cacheRegistryKey, $keys);
}Fix: Tambahkan unit test untuk memverifikasi:
// tests/Unit/Services/ArtikelServiceTest.php
public function test_register_cache_key_stores_key_in_registry(): void
{
Cache::shouldReceive('get')
->once()
->with('artikel_cache_registry', [])
->andReturn([]);
Cache::shouldReceive('forever')
->once()
->with('artikel_cache_registry', Mockery::on(function ($keys) {
return isset($keys['test_key']) && is_int($keys['test_key']);
}));
$service = new ArtikelService();
$reflection = new ReflectionClass($service);
$method = $reflection->getMethod('registerCacheKey');
$method->setAccessible(true);
$method->invoke($service, 'test_key');
}| return Cache::remember($cacheKey, $this->cacheTtl, function () use ($id): ?stdClass { | ||
| $data = $this->apiRequest('/api/v1/artikel/tampil', [ | ||
| 'id' => $id, | ||
| ]); |
There was a problem hiding this comment.
[HIGH] 📝 Code Quality: Missing Tests
Kategori: PHP Quality
Masalah: Method clearAllCache() adalah core functionality untuk bug fix ini - menghapus semua cache artikel sekaligus. Tanpa test, tidak ada jaminan method ini bekerja dengan benar di semua cache driver atau ketika registry kosong/corrupt.
Kode:
public function clearAllCache(): void
{
$keys = Cache::get($this->cacheRegistryKey, []);
foreach (array_keys($keys) as $key) {
Cache::forget($key);
}
Cache::forget($this->cacheRegistryKey);
}Fix: Tambahkan unit test untuk memverifikasi:
// tests/Unit/Services/ArtikelServiceTest.php
public function test_clear_all_cache_removes_all_registered_keys(): void
{
$registeredKeys = [
'artikel_cache_key_1' => time(),
'artikel_cache_key_2' => time(),
];
Cache::shouldReceive('get')
->once()
->with('artikel_cache_registry', [])
->andReturn($registeredKeys);
Cache::shouldReceive('forget')
->once()
->with('artikel_cache_key_1');
Cache::shouldReceive('forget')
->once()
->with('artikel_cache_key_2');
Cache::shouldReceive('forget')
->once()
->with('artikel_cache_registry');
$service = new ArtikelService();
$service->clearAllCache();
}
public function test_clear_all_cache_handles_empty_registry(): void
{
Cache::shouldReceive('get')
->once()
->with('artikel_cache_registry', [])
->andReturn([]);
Cache::shouldReceive('forget')
->once()
->with('artikel_cache_registry');
$service = new ArtikelService();
$service->clearAllCache();
}| { | ||
| $cacheKey = $this->buildCacheKey($prefix, $filters); | ||
| $cacheKey = $this->cacheSingleArtikel.$id; | ||
| Cache::forget($cacheKey); |
There was a problem hiding this comment.
[HIGH] 📝 Code Quality: Missing Tests
Kategori: PHP Quality
Masalah: Method clearCacheSingle() menghapus cache spesifik berdasarkan filter. Logic ini kompleks karena harus rebuild cache key dengan MD5 hash yang sama seperti saat cache dibuat. Tanpa test, risiko cache key mismatch tinggi.
Kode:
public function clearCacheSingle(string $cacheKey, array $filters = []): void
{
$key = $this->buildCacheKey($cacheKey, $filters);
Cache::forget($key);
$keys = Cache::get($this->cacheRegistryKey, []);
unset($keys[$key]);
if (empty($keys)) {
Cache::forget($this->cacheRegistryKey);
} else {
Cache::forever($this->cacheRegistryKey, $keys);
}
}Fix: Tambahkan unit test untuk memverifikasi:
// tests/Unit/Services/ArtikelServiceTest.php
public function test_clear_cache_single_removes_specific_key(): void
{
$filters = ['filter[id]' => 123];
$expectedKey = 'artikel_' . md5(json_encode($filters));
$registeredKeys = [
$expectedKey => time(),
'other_key' => time(),
];
Cache::shouldReceive('forget')
->once()
->with($expectedKey);
Cache::shouldReceive('get')
->once()
->with('artikel_cache_registry', [])
->andReturn($registeredKeys);
Cache::shouldReceive('forever')
->once()
->with('artikel_cache_registry', ['other_key' => Mockery::any()]);
$service = new ArtikelService();
$service->clearCacheSingle('artikel', $filters);
}
public function test_clear_cache_single_removes_registry_when_empty(): void
{
$filters = ['filter[id]' => 123];
$expectedKey = 'artikel_' . md5(json_encode($filters));
Cache::shouldReceive('forget')->with($expectedKey);
Cache::shouldReceive('get')->andReturn([$expectedKey => time()]);
Cache::shouldReceive('forget')->with('artikel_cache_registry');
$service = new ArtikelService();
$service->clearCacheSingle('artikel', $filters);
}
🐛 Bug Detection ReviewTotal Temuan: 0 isu (0 Critical, 0 High) ✅ Tidak ada bug HIGH atau CRITICAL yang ditemukan pada baris yang ditambah/diubah (prefix +) Analisis yang dilakukan:
Catatan:
|
🤖 AI Code Review — Selesai📋 Ringkasan Semua Review
Total inline comments: 6 |
3 participants
Pull Request: Fix: Cache artikel tidak terupdate setelah create/edit/delete
Deskripsi
Perbaikan bug pada sistem cache artikel yang tidak terupdate secara otomatis setelah melakukan operasi create, edit, atau delete artikel. Masalah ini menyebabkan user masih melihat data artikel lama meskipun sudah melakukan perubahan.
Solusi yang diterapkan menggunakan sistem cache registry yang mencatat semua cache key yang pernah dibuat, sehingga memungkinkan untuk menghapus SEMUA cache artikel dengan 100% akurat dan kompatibel dengan semua cache driver Laravel.
Perubahan yang dilakukan:
Refactor Service:
app/Services/ArtikelService.phpcache registryuntuk mencatat semua cache key yang pernah digenerateregisterCacheKey()untuk otomatis mendaftarkan setiap cache key yang dibuatclearAllCache()untuk menghapus SEMUA cache artikel sekaligusclearCacheSingle()untuk menghapus cache artikel tunggal berdasarkan IDartikel()danartikelById()sekarang otomatis mendaftarkan cache key ke registryController Update:
app/Http/Controllers/Master/ArtikelKabupatenController.phpclear_cacheyang lama denganclear_all_cacheclearAllCache()bukan hanya menghapus 1 cache keyView Create:
resources/views/master/artikel/create.blade.php?clear_all_cache=1View Edit:
resources/views/master/artikel/edit.blade.php?clear_all_cache=1View Index:
resources/views/master/artikel/index.blade.php?clear_all_cache=1table.ajax.reload()dengan full page refresh untuk memastikan cache benar-benar terhapusAlasan perubahan:
Cache::tags()tidak didukung oleh semua driver cache (khususnyafiledandatabasedriver yang umum digunakan di shared hosting).Dampak perubahan:
✅ Cache Selalu Fresh: Setiap create/edit/delete artikel, SEMUA cache artikel di seluruh halaman akan dihapus sekaligus
✅ Kompatibel Semua Driver: Bekerja 100% di semua cache driver Laravel (file, database, redis, memcached, dll)
✅ Tidak Ada Breaking Change: Tidak merubah API atau output dari
ArtikelService, hanya menambahkan fitur cache clearing✅ Backward Compatible: Code lama yang menggunakan
ArtikelServicetetap berfungsi normal tanpa perubahanMasalah Terkait (Related Issue)
Langkah untuk mereproduksi (Steps to Reproduce)
Sebelum perbaikan (masalah):
Setelah perbaikan (fix):
Daftar Periksa (Checklist)
Testing Checklist:
Code Checklist:
Teknis Detail
Penjelasan Teknis Sistem Cache Registry:
Dependencies yang ditambahkan:
Breaking Changes
Tidak ada breaking changes. Semua method dan interface
ArtikelServicetetap sama seperti sebelumnya.Migration Guide
Tidak diperlukan migration. Perubahan ini bekerja otomatis tanpa perlu konfigurasi tambahan.
simplescreenrecorder-2026-04-17_16.13.43.mp4