mirror/snipe-it
3
0
Fork 0
mirror of https://github.com/snipe/snipe-it.git synced 2026-08-18 11:15:42 +00:00
Code Issues Projects Releases Wiki Activity

Categories: performance improvements

Browse Source
This commit is contained in:
snipe
2026-06-30 14:52:55 +01:00
parent a61d720342
commit 969eeae4e2
10 changed files with 185 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
app/Models/Asset.php
View File

@ -1481,7 +1481,7 @@ class Asset extends Depreciable
public function scopePending($query)
{
// Pluck IDs then whereIn — do NOT replace with whereHas. whereHas generates a correlated EXISTS per row and causes severe slowdowns in withCount contexts.
$ids = Statuslabel::where('deployable', 0)->where('pending', 1)->where('archived', 0)->whereNull('deleted_at')->pluck('id');
$ids = Statuslabel::idsFor('pending');
return $query->whereIn('assets.status_id', $ids->isEmpty() ? [0] : $ids);
}
@ -1534,7 +1534,7 @@ class Asset extends Depreciable
public function scopeRTD($query)
{
// Pluck IDs then whereIn — do NOT replace with whereHas. whereHas generates a correlated EXISTS per row and causes severe slowdowns in withCount contexts.
$ids = Statuslabel::where('deployable', 1)->where('pending', 0)->where('archived', 0)->whereNull('deleted_at')->pluck('id');
$ids = Statuslabel::idsFor('deployable');
return $query->whereNull('assets.assigned_to')
->whereIn('assets.status_id', $ids->isEmpty() ? [0] : $ids);
@ -1549,7 +1549,7 @@ class Asset extends Depreciable
public function scopeUndeployable($query)
{
// Pluck IDs then whereIn — do NOT replace with whereHas. whereHas generates a correlated EXISTS per row and causes severe slowdowns in withCount contexts.
$ids = Statuslabel::where('deployable', 0)->where('pending', 0)->where('archived', 0)->whereNull('deleted_at')->pluck('id');
$ids = Statuslabel::idsFor('undeployable');
return $query->whereIn('assets.status_id', $ids->isEmpty() ? [0] : $ids);
}
@ -1563,7 +1563,7 @@ class Asset extends Depreciable
public function scopeNotArchived($query)
{
// Pluck IDs then whereIn — do NOT replace with whereHas. whereHas generates a correlated EXISTS per row and causes severe slowdowns in withCount contexts.
$ids = Statuslabel::where('archived', 0)->whereNull('deleted_at')->pluck('id');
$ids = Statuslabel::idsFor('not_archived');
return $query->whereIn('assets.status_id', $ids->isEmpty() ? [0] : $ids);
}
@ -1730,9 +1730,7 @@ class Asset extends Depreciable
{
// Pluck IDs then whereIn — do NOT replace with whereHas. whereHas generates a correlated EXISTS per row and causes severe slowdowns in withCount contexts.
if (Setting::getSettings()->show_archived_in_list != 1) {
$validStatusIds = Statuslabel::where('archived', 0)
->whereNull('deleted_at')
->pluck('id');
$validStatusIds = Statuslabel::idsFor('not_archived');
return $query->whereIn('assets.status_id', $validStatusIds->isEmpty() ? [0] : $validStatusIds);
}
@ -1749,7 +1747,7 @@ class Asset extends Depreciable
public function scopeArchived($query)
{
// Pluck IDs then whereIn — do NOT replace with whereHas. whereHas generates a correlated EXISTS per row and causes severe slowdowns in withCount contexts.
$ids = Statuslabel::where('deployable', 0)->where('pending', 0)->where('archived', 1)->whereNull('deleted_at')->pluck('id');
$ids = Statuslabel::idsFor('archived');
return $query->whereIn('assets.status_id', $ids->isEmpty() ? [0] : $ids);
}

10
app/Models/AssetModel.php
View File

@ -160,11 +160,17 @@ class AssetModel extends SnipeModel
public function percentRemaining()
{
if ($this->availableAssets()->count() == 0) {
// Cache the available count locally — the old code called
// availableAssets()->count() twice, which under the API transformer
// loop showed up as a duplicated `select count(*) … assets where
// model_id = ?` for every model in the response (visible in any
// /api/v1/models?category_id=… query log).
$available = $this->availableAssets()->count();
if ($available === 0) {
return 0;
}
return $this->availableAssets()->count() / $this->assets()->count() * 100;
return $available / $this->assets()->count() * 100;
}
/**

46
app/Models/Statuslabel.php
View File

@ -10,6 +10,7 @@ use Illuminate\Database\Eloquent\Factories\HasFactory;
use Illuminate\Database\Eloquent\Relations\Relation;
use Illuminate\Database\Eloquent\SoftDeletes;
use Illuminate\Database\Query\Builder;
use Illuminate\Support\Collection;
use Illuminate\Support\Facades\Gate;
use Watson\Validating\ValidatingTrait;
@ -47,6 +48,51 @@ class Statuslabel extends SnipeModel
use Searchable;
/**
* Per-request memo of the ID lists used by Asset's status-driven scopes
* (RTD / Pending / Undeployable / Archived / NotArchived). Each scope
* fired a separate `SELECT id FROM status_labels WHERE …` query on every
* call before this was introduced, which under withCount + transformer
* loops (see AssetModelsTransformer) added dozens of redundant queries
* per page or API hit.
*
* Cleared on save/delete via model events below, and reset between tests
* in InitializesSettings (alongside Setting::$_cache).
*/
protected static array $statusIdCache = [];
/**
* Return the cached set of status_label IDs matching the requested
* "kind" — one of: deployable, pending, undeployable, archived,
* not_archived. Each call after the first in a single request reads
* from memory.
*/
public static function idsFor(string $type): Collection
{
return self::$statusIdCache[$type] ??= match ($type) {
'deployable' => self::where('deployable', 1)->where('pending', 0)->where('archived', 0)->whereNull('deleted_at')->pluck('id'),
'pending' => self::where('deployable', 0)->where('pending', 1)->where('archived', 0)->whereNull('deleted_at')->pluck('id'),
'undeployable' => self::where('deployable', 0)->where('pending', 0)->where('archived', 0)->whereNull('deleted_at')->pluck('id'),
'archived' => self::where('deployable', 0)->where('pending', 0)->where('archived', 1)->whereNull('deleted_at')->pluck('id'),
'not_archived' => self::where('archived', 0)->whereNull('deleted_at')->pluck('id'),
default => throw new \InvalidArgumentException('Unknown status type: '.$type),
};
}
public static function clearIdCache(): void
{
self::$statusIdCache = [];
}
protected static function booted(): void
{
// Any mutation to a status label invalidates the cached ID lists.
$invalidate = fn () => self::clearIdCache();
static::saved($invalidate);
static::deleted($invalidate);
static::restored($invalidate);
}
/**
* The attributes that should be included when searching the model.
*

7
resources/views/blade/box/index.blade.php
View File

@ -31,9 +31,10 @@
</div>
@endif
@if (($slot) && (!$slot->isEmpty()))
{{ $slot }}
@endif
{{-- Render slot unconditionally — ComponentSlot::isEmpty()
materializes the slot to inspect it, doubling every DB call
inside. An empty slot renders nothing visible. --}}
{{ $slot }}
</div>

13
resources/views/blade/tabs/index.blade.php
View File

@ -6,16 +6,17 @@
<!-- start tab container -->
<div class="nav-tabs-custom">
{{-- Do NOT guard the slot renders with $slot->isEmpty() — ComponentSlot
::isEmpty() materializes the slot once just to inspect it, and the
actual {{ $slot }} below materializes it a second time. Every count()
/ DB call inside a slot then fires twice. Render unconditionally;
an empty slot renders empty content and the wrapper markup is fine. --}}
<ul class="nav nav-tabs hidden-print nav-tabs-dropdown" role="tablist">
@if (!$tabnav->isEmpty())
{{ $tabnav }}
@endif
{{ $tabnav }}
</ul>
<div class="tab-content">
@if (!$tabpanes->isEmpty())
{{ $tabpanes }}
@endif
{{ $tabpanes }}
</div>

13
resources/views/blade/tabs/pane.blade.php
View File

@ -20,13 +20,16 @@
</div>
@endif
@if ((isset($content)) && (!$content->isEmpty()))
{{-- Render slots unconditionally — ComponentSlot::isEmpty()
materializes the slot to inspect it, doubling every DB call
inside (asset/model counts, presenter dataTableLayout, etc.).
`isset($content)` is fine because it's a named slot check that
doesn't render anything. --}}
@isset($content)
{{ $content }}
@endif
@endisset
@if (($slot) && (!$slot->isEmpty()))
{{ $slot }}
@endif
{{ $slot }}
</div>
</div>

17
resources/views/categories/view.blade.php
View File

@ -20,15 +20,22 @@
<x-slot:tabnav>
@if ($category->category_type=='asset')
<x-tabs.asset-tab count="{{ $category->showableAssets()->count() }}"/>
<x-tabs.model-tab count="{{ $category->models->count() }}"/>
{{-- Use the relation method (->models()) — property access (->models) hydrates
the whole AssetModel collection just to take ->count(), and on a category
with hundreds of models it's a notable allocator on the shell render. --}}
<x-tabs.model-tab count="{{ $category->models()->count() }}"/>
@elseif ($category->category_type=='accessory')
<x-tabs.accessory-tab count="{{ $category->accessories->count() }}"/>
{{-- Method-style ->accessories()->count() so we issue a
SELECT count(*) instead of hydrating the full
collection just to read its size. Same rationale
as the ->models() call above. --}}
<x-tabs.accessory-tab count="{{ $category->accessories()->count() }}"/>
@elseif ($category->category_type=='license')
<x-tabs.license-tab count="{{ $category->licenses->count() }}"/>
<x-tabs.license-tab count="{{ $category->licenses()->count() }}"/>
@elseif ($category->category_type=='consumable')
<x-tabs.consumable-tab count="{{ $category->consumables->count() }}"/>
<x-tabs.consumable-tab count="{{ $category->consumables()->count() }}"/>
@elseif ($category->category_type=='component')
<x-tabs.component-tab count="{{ $category->components->count() }}"/>
<x-tabs.component-tab count="{{ $category->components()->count() }}"/>
@endif
</x-slot:tabnav>

6
tests/Support/InitializesSettings.php
View File

@ -3,6 +3,7 @@
namespace Tests\Support;
use App\Models\Setting;
use App\Models\Statuslabel;
trait InitializesSettings
{
@ -13,5 +14,10 @@ trait InitializesSettings
$this->settings = Settings::initialize();
$this->beforeApplicationDestroyed(fn () => Setting::$_cache = null);
// Same idea as Setting::$_cache — a per-request memo lives across
// tests if we don't explicitly reset it. Without this, the second
// test in a file would see status_label IDs from the *previous*
// test's transactional rows (which have since rolled back).
$this->beforeApplicationDestroyed(fn () => Statuslabel::clearIdCache());
}
}

30
tests/Unit/AssetModelTest.php
View File

@ -5,6 +5,7 @@ namespace Tests\Unit;
use App\Models\Asset;
use App\Models\AssetModel;
use App\Models\Category;
use Illuminate\Support\Facades\DB;
use Tests\TestCase;
class AssetModelTest extends TestCase
@ -84,6 +85,35 @@ class AssetModelTest extends TestCase
$this->assertEquals(40.0, $model->percentRemaining());
}
public function test_percent_remaining_only_calls_available_assets_count_once(): void
{
// Regression: the old implementation called $this->availableAssets()
// ->count() twice — once for the zero guard, once for the ratio.
// Under the API transformer loop in /api/v1/models that doubled the
// status-label pluck + count(*) queries for every row. Pin the
// single-call contract so a future cleanup doesn't reintroduce it.
$category = Category::factory()->create(['category_type' => 'asset']);
$model = AssetModel::factory()->create(['category_id' => $category->id]);
Asset::factory()->count(2)->create(['model_id' => $model->id]);
$availableCountQueries = 0;
DB::listen(function ($query) use (&$availableCountQueries, $model) {
// The availableAssets() relation generates a count(*) on assets
// filtered by model_id AND assigned_to IS NULL.
if (str_contains($query->sql, 'count(*)')
&& str_contains($query->sql, '"assets"')
&& str_contains($query->sql, '"assigned_to" is null')
&& in_array($model->id, $query->bindings, true)
) {
$availableCountQueries++;
}
});
$model->percentRemaining();
$this->assertSame(1, $availableCountQueries, 'availableAssets()->count() must be called exactly once per percentRemaining()');
}
public function test_percent_remaining_returns_one_hundred_when_all_assets_are_available()
{
$model = new class extends AssetModel

58
tests/Unit/StatuslabelTest.php
View File

@ -3,6 +3,7 @@
namespace Tests\Unit;
use App\Models\Statuslabel;
use Illuminate\Support\Facades\DB;
use Tests\TestCase;
class StatuslabelTest extends TestCase
@ -42,4 +43,61 @@ class StatuslabelTest extends TestCase
$statuslabel = Statuslabel::factory()->lost()->create();
$this->assertModelExists($statuslabel);
}
public function test_ids_for_returns_matching_status_labels_per_kind(): void
{
$rtd = Statuslabel::factory()->rtd()->create();
$pending = Statuslabel::factory()->pending()->create();
$archived = Statuslabel::factory()->archived()->create();
Statuslabel::clearIdCache();
$this->assertContains($rtd->id, Statuslabel::idsFor('deployable')->all());
$this->assertNotContains($pending->id, Statuslabel::idsFor('deployable')->all());
$this->assertContains($pending->id, Statuslabel::idsFor('pending')->all());
$this->assertNotContains($rtd->id, Statuslabel::idsFor('pending')->all());
$this->assertContains($archived->id, Statuslabel::idsFor('archived')->all());
$this->assertContains($rtd->id, Statuslabel::idsFor('not_archived')->all(), 'RTD label is "not archived"');
$this->assertNotContains($archived->id, Statuslabel::idsFor('not_archived')->all());
}
public function test_ids_for_memoizes_within_a_request(): void
{
// Each scope (RTD / Pending / Undeployable / Archived / NotArchived)
// used to fire a fresh `SELECT id FROM status_labels` every time it
// was called. Under the API transformer loops that became dozens of
// identical queries per request. The cache should collapse repeated
// calls to a single query.
Statuslabel::factory()->rtd()->create();
Statuslabel::clearIdCache();
$queries = 0;
DB::listen(function ($q) use (&$queries) {
if (str_contains($q->sql, 'status_labels')) {
$queries++;
}
});
Statuslabel::idsFor('deployable');
Statuslabel::idsFor('deployable');
Statuslabel::idsFor('deployable');
$this->assertSame(1, $queries, 'idsFor should hit the DB once per kind per request');
}
public function test_ids_for_cache_invalidates_on_save(): void
{
$original = Statuslabel::factory()->rtd()->create();
Statuslabel::clearIdCache();
$this->assertContains($original->id, Statuslabel::idsFor('deployable')->all());
// New deployable label after the cache was warmed — must show up
// because the saved event clears the cache.
$added = Statuslabel::factory()->rtd()->create();
$this->assertContains($added->id, Statuslabel::idsFor('deployable')->all());
}
}