-
Notifications
You must be signed in to change notification settings - Fork 13
Fix: Cache artikel tidak dihapus setelah operasi hapus #991
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
3f5cda6
9471c25
86e88ed
c0281e3
aee9263
b63a6a9
fc1882e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,31 +2,65 @@ | |
|
|
||
| namespace App\Services; | ||
|
|
||
| use Illuminate\Support\Collection; | ||
| use Illuminate\Support\Facades\Cache; | ||
| use stdClass; | ||
|
|
||
| class ArtikelService extends BaseApiService | ||
| { | ||
| protected int $cacheTtl = 3600; // TTL dalam detik (1 jam) | ||
|
|
||
| public function artikel(array $filters = []) | ||
| private string $cacheSingleArtikel = 'artikel_'; | ||
|
|
||
| private string $cacheRegistryKey = 'artikel_cache_registry'; | ||
|
|
||
| /** | ||
| * Daftarkan cache key ke registry setiap kali generate | ||
| */ | ||
| private function registerCacheKey(string $key): void | ||
| { | ||
| $keys = Cache::get($this->cacheRegistryKey, []); | ||
|
|
||
| // Pastikan selalu array meskipun cache corrupt | ||
| if (! is_array($keys)) { | ||
| $keys = []; | ||
| } | ||
|
|
||
| $keys[$key] = time(); | ||
|
|
||
| // Gunakan TTL 7 hari, tidak forever untuk mencegah memory bloat | ||
| Cache::put($this->cacheRegistryKey, $keys, now()->addDays(7)); | ||
| } | ||
|
|
||
| /** | ||
| * Mendapatkan daftar artikel dengan filter opsional | ||
| * | ||
| * @param array<int|string, mixed> $filters | ||
| */ | ||
| public function artikel(array $filters = []): Collection | ||
| { | ||
| $cacheKey = $this->buildCacheKey('artikel', $filters); | ||
|
|
||
| // ✅ Daftarkan key setiap kali generate | ||
| $this->registerCacheKey($cacheKey); | ||
|
|
||
| // Ambil dari cache dulu | ||
| return Cache::remember($cacheKey, $this->cacheTtl, function () use ($filters) { | ||
| return Cache::remember($cacheKey, $this->cacheTtl, function () use ($filters): Collection { | ||
| $data = $this->apiRequest('/api/v1/artikel/list', $filters); | ||
| if (!$data) { | ||
|
|
||
| if (empty($data)) { | ||
| return collect([]); | ||
| } | ||
| return collect($data)->map(function ($item) { | ||
| // Return 'attributes' but with 'id' populated | ||
|
|
||
| return collect($data)->map(function (array $item): stdClass { | ||
| // Return 'attributes' but with 'id' populated | ||
| $attributes = $item['attributes'] ?? []; | ||
| $attributes['id'] = $item['id'] ?? null; | ||
|
|
||
| // Fetch detail to enrich with gambar and isi if missing | ||
| if (isset($attributes['id']) && (!isset($attributes['gambar']) || !isset($attributes['isi']))) { | ||
| $detail = $this->artikelById($attributes['id']); | ||
| if ($detail) { | ||
| if (isset($attributes['id']) && (! isset($attributes['gambar']) || ! isset($attributes['isi']))) { | ||
| $detail = $this->artikelById((int) $attributes['id']); | ||
| if ($detail !== null) { | ||
| $attributes['gambar'] = $detail->gambar ?? null; | ||
| $attributes['isi'] = $detail->isi ?? null; | ||
| } | ||
|
|
@@ -37,11 +71,17 @@ public function artikel(array $filters = []) | |
| }); | ||
| } | ||
|
|
||
| public function artikelById(int $id) | ||
| /** | ||
| * Mendapatkan detail artikel berdasarkan ID | ||
| */ | ||
| public function artikelById(int $id): ?stdClass | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [HIGH] 📝 Code Quality: Missing Tests Kategori: PHP Quality 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');
} |
||
| { | ||
| $cacheKey = "artikel_$id"; | ||
| $cacheKey = $this->cacheSingleArtikel.$id; | ||
|
|
||
| // ✅ Daftarkan key setiap kali generate | ||
| $this->registerCacheKey($cacheKey); | ||
|
|
||
| return Cache::remember($cacheKey, $this->cacheTtl, function () use ($id) { | ||
| 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [HIGH] ⚡ Performance: Unbatched Cache Deletion Loop Masalah: Loop Kode: foreach (array_keys($keys) as $key) {
Cache::forget($key);
}Dampak: Dengan 1000 cache keys, ini akan melakukan 1000 operasi 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [HIGH] 📝 Code Quality: Missing Tests Kategori: PHP Quality 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();
} |
||
|
|
@@ -54,9 +94,50 @@ public function artikelById(int $id) | |
| }); | ||
| } | ||
|
|
||
| public function clearCache(string $prefix = 'artikel', array $filters = []) | ||
| /** | ||
| * Menghapus cache artikel tunggal berdasarkan ID | ||
| */ | ||
| public function clearCacheSingle(int $id): void | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [MEDIUM] 🔒 Security: Type Confusion - Missing Array Validation Masalah: Method Kode: public function clearAllCache(): void
{
$keys = Cache::get($this->cacheRegistryKey, []);
foreach (array_keys($keys) as $key) {
Cache::forget($key);
}
Cache::forget($this->cacheRegistryKey);
}Risiko:
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);
} |
||
| { | ||
| $cacheKey = $this->buildCacheKey($prefix, $filters); | ||
| $cacheKey = $this->cacheSingleArtikel.$id; | ||
| Cache::forget($cacheKey); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [HIGH] 📝 Code Quality: Missing Tests Kategori: PHP Quality 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);
} |
||
| } | ||
|
|
||
| /** | ||
| * ✅ HAPUS SEMUA CACHE ARTIKEL 100% BERFUNGSI DI SEMUA DRIVER! | ||
| * Termasuk semua cache list dengan hash MD5 apapun | ||
| */ | ||
| public function clearAllCache(): void | ||
| { | ||
| // Ambil semua key yang pernah terdaftar | ||
| $keys = Cache::get($this->cacheRegistryKey, []); | ||
|
|
||
| // Validasi tipe data, hindari fatal error jika cache corrupt | ||
| if (! is_array($keys)) { | ||
| Cache::forget($this->cacheRegistryKey); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| $cacheKeys = array_keys($keys); | ||
|
|
||
| if (empty($cacheKeys)) { | ||
| Cache::forget($this->cacheRegistryKey); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| // Laravel 10+ mendukung deleteMultiple untuk batch operation | ||
| try { | ||
| Cache::deleteMultiple($cacheKeys); | ||
| } catch (\BadMethodCallException $e) { | ||
| // Fallback untuk driver yang tidak mendukung deleteMultiple | ||
| foreach ($cacheKeys as $key) { | ||
| Cache::forget($key); | ||
| } | ||
| } | ||
|
|
||
| // Reset registry | ||
| Cache::forget($this->cacheRegistryKey); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[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:
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:
Alternatif: Implementasi cleanup mechanism untuk registry yang sudah tidak relevan: