From 5076deccf395f36c1a0ff90be30578b81cc4ac28 Mon Sep 17 00:00:00 2001 From: Tony Murray Date: Mon, 16 May 2022 02:57:58 -0500 Subject: [PATCH] Improve the efficiency of some queries (#13974) * Improve the efficiency of some queries Mostly by switching from whereIn to whereIntegerInRaw. This inserts integers directly into the query instead of using placeholders (also escapes them) also remove extra json_encode/json_decode in PingCheck * Fix return types Probably will result in some missing baseline exceptions. * Update PingCheck.php * whitespace --- LibreNMS/Cache/PermissionsCache.php | 2 +- LibreNMS/Component.php | 2 +- LibreNMS/Modules/Nac.php | 2 +- .../Controllers/Table/FdbTablesController.php | 24 +++++++++---------- .../Controllers/Widgets/GraphController.php | 2 +- app/Jobs/PingCheck.php | 6 +++-- app/Models/BaseModel.php | 6 ++--- app/Models/Device.php | 2 +- app/Models/DeviceGroup.php | 2 +- app/Models/Location.php | 2 +- app/Models/User.php | 2 +- app/Observers/DeviceObserver.php | 4 ++-- includes/discovery/fdb-table.inc.php | 2 +- includes/polling/ports.inc.php | 2 +- 14 files changed, 31 insertions(+), 29 deletions(-) diff --git a/LibreNMS/Cache/PermissionsCache.php b/LibreNMS/Cache/PermissionsCache.php index 936dbddb8f..eb6640438c 100644 --- a/LibreNMS/Cache/PermissionsCache.php +++ b/LibreNMS/Cache/PermissionsCache.php @@ -177,7 +177,7 @@ class PermissionsCache // if we don't have a map for this user yet, populate it. if (! isset($this->deviceGroupMap[$user_id])) { $this->deviceGroupMap[$user_id] = DB::table('device_group_device') - ->whereIn('device_id', $this->devicesForUser($user)) + ->whereIntegerInRaw('device_id', $this->devicesForUser($user)) ->distinct('device_group_id') ->pluck('device_group_id'); } diff --git a/LibreNMS/Component.php b/LibreNMS/Component.php index 5893e8cc48..3e4703219f 100644 --- a/LibreNMS/Component.php +++ b/LibreNMS/Component.php @@ -218,7 +218,7 @@ class Component public function setComponentPrefs($device_id, $updated) { $updated = Arr::wrap($updated); - \App\Models\Component::whereIn('id', array_keys($updated)) + \App\Models\Component::whereIntegerInRaw('id', array_keys($updated)) ->with('prefs') ->get() ->each(function (\App\Models\Component $component) use ($device_id, $updated) { diff --git a/LibreNMS/Modules/Nac.php b/LibreNMS/Modules/Nac.php index 41501ad37a..44f17381af 100644 --- a/LibreNMS/Modules/Nac.php +++ b/LibreNMS/Modules/Nac.php @@ -72,7 +72,7 @@ class Nac implements Module $delete = $existing_entries->diffKeys($nac_entries)->pluck('ports_nac_id'); if ($delete->isNotEmpty()) { - $count = PortsNac::query()->whereIn('ports_nac_id', $delete)->delete(); + $count = PortsNac::query()->whereIntegerInRaw('ports_nac_id', $delete)->delete(); d_echo('Deleted ' . $count, str_repeat('-', $count)); } } diff --git a/app/Http/Controllers/Table/FdbTablesController.php b/app/Http/Controllers/Table/FdbTablesController.php index 0535eb8e13..175ed6fa68 100644 --- a/app/Http/Controllers/Table/FdbTablesController.php +++ b/app/Http/Controllers/Table/FdbTablesController.php @@ -83,19 +83,19 @@ class FdbTablesController extends TableController case 'mac': return $query->where('ports_fdb.mac_address', 'like', $mac_search); case 'vlan': - return $query->whereIn('ports_fdb.vlan_id', $this->findVlans($search)); + return $query->whereIntegerInRaw('ports_fdb.vlan_id', $this->findVlans($search)); case 'dnsname': $search = gethostbyname($search); // no break case 'ip': return $query->whereIn('ports_fdb.mac_address', $this->findMacs($search)); case 'description': - return $query->whereIn('ports_fdb.port_id', $this->findPorts($search)); + return $query->whereIntegerInRaw('ports_fdb.port_id', $this->findPorts($search)); default: return $query->where(function ($query) use ($search, $mac_search) { $query->where('ports_fdb.mac_address', 'like', $mac_search) - ->orWhereIn('ports_fdb.port_id', $this->findPorts($search)) - ->orWhereIn('ports_fdb.vlan_id', $this->findVlans($search)) + ->orWhereIntegerInRaw('ports_fdb.port_id', $this->findPorts($search)) + ->orWhereIntegerInRaw('ports_fdb.vlan_id', $this->findVlans($search)) ->orWhereIn('ports_fdb.mac_address', $this->findMacs($search)); }); } @@ -193,9 +193,9 @@ class FdbTablesController extends TableController /** * @param string $ip - * @return Builder + * @return \Illuminate\Support\Collection */ - protected function findMacs($ip) + protected function findMacs($ip): \Illuminate\Support\Collection { $port_id = \Request::get('port_id'); $device_id = \Request::get('device_id'); @@ -212,9 +212,9 @@ class FdbTablesController extends TableController /** * @param string $vlan - * @return Builder + * @return \Illuminate\Support\Collection */ - protected function findVlans($vlan) + protected function findVlans($vlan): \Illuminate\Support\Collection { $port_id = \Request::get('port_id'); $device_id = \Request::get('device_id'); @@ -233,9 +233,9 @@ class FdbTablesController extends TableController /** * @param string $ifAlias - * @return Builder + * @return \Illuminate\Support\Collection */ - protected function findPorts($ifAlias) + protected function findPorts($ifAlias): \Illuminate\Support\Collection { $port_id = \Request::get('port_id'); $device_id = \Request::get('device_id'); @@ -252,9 +252,9 @@ class FdbTablesController extends TableController /** * @param string $mac_address - * @return \Illuminate\Support\Collection + * @return array */ - protected function findIps($mac_address) + protected function findIps($mac_address): array { if (! isset($this->ipCache[$mac_address])) { $ips = Ipv4Mac::where('mac_address', $mac_address) diff --git a/app/Http/Controllers/Widgets/GraphController.php b/app/Http/Controllers/Widgets/GraphController.php index 59138b6a02..be767a24ff 100644 --- a/app/Http/Controllers/Widgets/GraphController.php +++ b/app/Http/Controllers/Widgets/GraphController.php @@ -145,7 +145,7 @@ class GraphController extends WidgetController } $data['service_text'] = isset($service) ? $service->device->displayName() . ' - ' . $service->service_type . ' (' . $service->service_desc . ')' : __('Service does not exist'); - $data['graph_ports'] = Port::whereIn('port_id', $data['graph_ports']) + $data['graph_ports'] = Port::whereIntegerInRaw('port_id', $data['graph_ports']) ->select('ports.device_id', 'port_id', 'ifAlias', 'ifName', 'ifDescr') ->with(['device' => function ($query) { $query->select('device_id', 'hostname', 'sysName'); diff --git a/app/Jobs/PingCheck.php b/app/Jobs/PingCheck.php index 139b2be571..056a082819 100644 --- a/app/Jobs/PingCheck.php +++ b/app/Jobs/PingCheck.php @@ -161,11 +161,13 @@ class PingCheck implements ShouldQueue ->orderBy('max_depth'); if ($this->groups) { - $query->whereIn('poller_group', $this->groups); + $query->whereIntegerInRaw('poller_group', $this->groups); } $this->devices = $query->get()->keyBy(function ($device) { - return Device::pollerTarget(json_decode(json_encode($device), true)); + /** @var Device $device */ + + return $device->overwrite_ip ?: $device->hostname; }); // working collections diff --git a/app/Models/BaseModel.php b/app/Models/BaseModel.php index 42de8cbbe8..98f52aae70 100644 --- a/app/Models/BaseModel.php +++ b/app/Models/BaseModel.php @@ -70,7 +70,7 @@ abstract class BaseModel extends Model $table = $this->getTable(); } - return $query->whereIn("$table.device_id", \Permissions::devicesForUser($user)); + return $query->whereIntegerInRaw("$table.device_id", \Permissions::devicesForUser($user)); } /** @@ -92,8 +92,8 @@ abstract class BaseModel extends Model } return $query->where(function ($query) use ($table, $user) { - return $query->whereIn("$table.port_id", \Permissions::portsForUser($user)) - ->orWhereIn("$table.device_id", \Permissions::devicesForUser($user)); + return $query->whereIntegerInRaw("$table.port_id", \Permissions::portsForUser($user)) + ->orWhereIntegerInRaw("$table.device_id", \Permissions::devicesForUser($user)); }); } } diff --git a/app/Models/Device.php b/app/Models/Device.php index 620bc02c41..bf970557f8 100644 --- a/app/Models/Device.php +++ b/app/Models/Device.php @@ -210,7 +210,7 @@ class Device extends BaseModel if ($this->groups->isNotEmpty()) { $query->orWhereHas('deviceGroups', function (Builder $query) { - $query->whereIn('alert_schedulables.alert_schedulable_id', $this->groups->pluck('id')); + $query->whereIntegerInRaw('alert_schedulables.alert_schedulable_id', $this->groups->pluck('id')); }); } diff --git a/app/Models/DeviceGroup.php b/app/Models/DeviceGroup.php index 74738aa33e..8a8b65bc70 100644 --- a/app/Models/DeviceGroup.php +++ b/app/Models/DeviceGroup.php @@ -89,7 +89,7 @@ class DeviceGroup extends BaseModel return $query; } - return $query->whereIn('id', Permissions::deviceGroupsForUser($user)); + return $query->whereIntegerInRaw('id', Permissions::deviceGroupsForUser($user)); } public function scopeInServiceTemplate($query, $serviceTemplate) diff --git a/app/Models/Location.php b/app/Models/Location.php index 7dc4a20e0a..b9fb21eb17 100644 --- a/app/Models/Location.php +++ b/app/Models/Location.php @@ -155,7 +155,7 @@ class Location extends Model ->whereNotNull('location_id') ->pluck('location_id'); - return $query->whereIn('id', $ids); + return $query->whereIntegerInRaw('id', $ids); } public function scopeInDeviceGroup($query, $deviceGroup) diff --git a/app/Models/User.php b/app/Models/User.php index 47e22a9aad..e1a88806c4 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -213,7 +213,7 @@ class User extends Authenticatable { // pseudo relation return Device::query()->when(! $this->hasGlobalRead(), function ($query) { - return $query->whereIn('device_id', Permissions::devicesForUser($this)); + return $query->whereIntegerInRaw('device_id', Permissions::devicesForUser($this)); }); } diff --git a/app/Observers/DeviceObserver.php b/app/Observers/DeviceObserver.php index c510e3ccc4..cbdac00789 100644 --- a/app/Observers/DeviceObserver.php +++ b/app/Observers/DeviceObserver.php @@ -190,7 +190,7 @@ class DeviceObserver // a parent attached to this device // update the parent's max depth incase it used to be standalone - Device::whereIn('device_id', $pivotIds)->get()->each->validateStandalone(); + Device::whereIntegerInRaw('device_id', $pivotIds)->get()->each->validateStandalone(); // make sure this device's max depth is updated $device->updateMaxDepth(); @@ -201,7 +201,7 @@ class DeviceObserver $device->validateStandalone(); // make sure the child's max depth is updated - Device::whereIn('device_id', $pivotIds)->get()->each->updateMaxDepth(); + Device::whereIntegerInRaw('device_id', $pivotIds)->get()->each->updateMaxDepth(); } } diff --git a/includes/discovery/fdb-table.inc.php b/includes/discovery/fdb-table.inc.php index a7ce0025fa..2765438402 100644 --- a/includes/discovery/fdb-table.inc.php +++ b/includes/discovery/fdb-table.inc.php @@ -80,7 +80,7 @@ if (! empty($insert)) { echo PHP_EOL; } - DB::table('ports_fdb')->whereIn('ports_fdb_id', $update_time_only)->update(['updated_at' => $now]); + DB::table('ports_fdb')->whereIntegerInRaw('ports_fdb_id', $update_time_only)->update(['updated_at' => $now]); //We do not delete anything here, as daily.sh will take care of the cleaning. diff --git a/includes/polling/ports.inc.php b/includes/polling/ports.inc.php index 15c07b717f..5b3f23f431 100644 --- a/includes/polling/ports.inc.php +++ b/includes/polling/ports.inc.php @@ -961,7 +961,7 @@ foreach ($ports as $port) { } //end port update // Update the poll_time, poll_prev and poll_period of all ports in an unique request -$updated = DB::table('ports')->whereIn('port_id', $globally_updated_port_ids)->update($device_global_ports); +$updated = DB::table('ports')->whereIntegerInRaw('port_id', $globally_updated_port_ids)->update($device_global_ports); d_echo("$updated updated\n");