Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 10 additions & 14 deletions app/Http/Controllers/Master/ArtikelKabupatenController.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
namespace App\Http\Controllers\Master;

use App\Http\Controllers\Controller;
use App\Services\ArtikelService;
use Illuminate\Http\Request;
use Illuminate\View\View;

class ArtikelKabupatenController extends Controller
Expand All @@ -11,24 +13,22 @@ class ArtikelKabupatenController extends Controller

/**
* Display a listing of the resource.
*
* @return \Illuminate\Http\Response
*/
public function index()
public function index(Request $request): View
{
$listPermission = $this->generateListPermission();
$clearCache = request('clear_cache', false);
if ($clearCache) {
(new \App\Services\ArtikelService)->clearCache('artikel', ['filter[id]' => $clearCache]);
$listPermission = $this->generateListPermission();
$clearAllCache = $request->query('clear_all_cache', 0);

if ($clearAllCache > 0) {
// setiap ada perubahan di clear cache semua, termasuk ketika edit karena bisa jadi edit judul saja
(new ArtikelService)->clearAllCache();
}

return view('master.artikel.index')->with($listPermission);
}

/**
* Show the form for creating a new resource.
*
* @return \Illuminate\Http\Response
*/
public function create(): View
{
Expand All @@ -37,12 +37,8 @@ public function create(): View

/**
* Show the form for editing the specified resource.
*
* @param int $id
*
* @return \Illuminate\Http\Response
*/
public function edit($id): View
public function edit(int $id): View
{
return view('master.artikel.edit', compact('id'));
}
Expand Down
107 changes: 94 additions & 13 deletions app/Services/ArtikelService.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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
Copy link
Copy Markdown

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:

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));
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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');
}

{
$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,
]);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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();
}

Expand All @@ -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
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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_registry key
  • Fatal Error: array_keys() akan throw TypeError jika $keys bukan 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);
}

{
$cacheKey = $this->buildCacheKey($prefix, $filters);
$cacheKey = $this->cacheSingleArtikel.$id;
Cache::forget($cacheKey);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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);
}

}

/**
* ✅ 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);
}
}
1 change: 1 addition & 0 deletions catatan_rilis.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ Di rilis ini, versi 2604.0.0 berisi penambahan dan perbaikan yang diminta penggu
#### Perbaikan BUG

1. [#954](https://github.com/OpenSID/OpenKab/issues/954) Perbaikan list menu tidak tampil.
2. [#369](https://github.com/OpenSID/API-Database-Gabungan/issues/369) Perbaikan cache artikel tidak dihapus setelah operasi hapus.

#### Perubahan Teknis

Expand Down
2 changes: 1 addition & 1 deletion resources/views/master/artikel/create.blade.php
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ function artikel() {
});
setTimeout(() => {
window.location.href =
'{{ route('master-data-artikel.index') }}';
'{{ route('master-data-artikel.index') }}?clear_all_cache=1';
}, 1500);
} else {
Swal.fire({
Expand Down
3 changes: 1 addition & 2 deletions resources/views/master/artikel/edit.blade.php
Original file line number Diff line number Diff line change
Expand Up @@ -362,8 +362,7 @@ function artikel() {
});
setTimeout(() => {
window.location.href =
'{{ route('master-data-artikel.index') }}?clear_cache=' +
artikelId;
'{{ route('master-data-artikel.index') }}?clear_all_cache=1';
}, 1500);
} else {
Swal.fire({
Expand Down
5 changes: 4 additions & 1 deletion resources/views/master/artikel/index.blade.php
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,10 @@ className: 'text-center',
showConfirmButton: true,
timer: 1500
})
table.ajax.reload(null, false);
setTimeout(() => {
window.location.href =
'{{ route('master-data-artikel.index') }}?clear_all_cache=1';
}, 1500);
} else {
Swal.fire({
title: 'Error!',
Expand Down
58 changes: 52 additions & 6 deletions tests/Unit/ArtikelServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

use App\Services\ArtikelService;
use Illuminate\Support\Facades\Cache;
use Mockery;
use ReflectionClass;
use Tests\TestCase;

class ArtikelServiceTest extends TestCase
Expand All @@ -30,7 +32,7 @@ public function it_builds_cache_key_correctly()
$method->setAccessible(true);

$cacheKey = $method->invokeArgs($this->service, ['artikel', ['id' => 1]]);

$this->assertIsString($cacheKey);
$this->assertStringContainsString('artikel', $cacheKey);
}
Expand All @@ -40,11 +42,11 @@ public function clear_cache_removes_cached_data()
{
$cacheKey = 'test_artikel_cache';
Cache::put($cacheKey, 'test_data', 3600);

$this->assertTrue(Cache::has($cacheKey));

Cache::forget($cacheKey);

$this->assertFalse(Cache::has($cacheKey));
}

Expand All @@ -54,9 +56,9 @@ public function it_has_cache_ttl_property()
$reflection = new \ReflectionClass($this->service);
$property = $reflection->getProperty('cacheTtl');
$property->setAccessible(true);

$ttl = $property->getValue($this->service);

$this->assertEquals(3600, $ttl);
$this->assertIsInt($ttl);
}
Expand Down Expand Up @@ -87,4 +89,48 @@ public function service_extends_base_api_service()
{
$this->assertInstanceOf(\App\Services\BaseApiService::class, $this->service);
}

// 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();
}
}
Loading