Skip to content

Commit

Permalink
Merge pull request #619 from TheRestartProject/RES-1842_sentry
Browse files Browse the repository at this point in the history
RES 1842 reduce N+1 queries
  • Loading branch information
edwh authored Feb 20, 2023
2 parents 07866a3 + b2d71b7 commit d259b18
Show file tree
Hide file tree
Showing 9 changed files with 102 additions and 9 deletions.
3 changes: 3 additions & 0 deletions app/Group.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ class Group extends Model implements Auditable
protected $table = 'groups';
protected $primaryKey = 'idgroups';

// Eager-loading reduces N+1 queries.
protected $with = ['networks'];

/**
* The attributes that are mass assignable.
*
Expand Down
3 changes: 2 additions & 1 deletion app/Http/Controllers/GroupController.php
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ private function indexVariations($tab, $network)

// Look for groups we have joined, not just been invited to. We have to explicitly test on deleted_at because
// the normal filtering out of soft deletes won't happen for joins.
$your_groups =array_column(Group::join('users_groups', 'users_groups.group', '=', 'groups.idgroups')
$your_groups =array_column(Group::with(['networks'])
->join('users_groups', 'users_groups.group', '=', 'groups.idgroups')
->leftJoin('events', 'events.group', '=', 'groups.idgroups')
->where('users_groups.user', $user->id)
->where('users_groups.status', 1)
Expand Down
7 changes: 4 additions & 3 deletions app/Http/Controllers/PartyController.php
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public static function expandEvent($event, $group = null)
if (is_null($group)) {
// We are showing events for multiple groups and so we need to pass the relevant group, in order that
// we can show the group name and link to it.
$thisone['group'] = \App\Group::find($event->group);
$thisone['group'] = \App\Group::with('networks')->find($event->group);

$group_image = $thisone['group']->groupImage;
if (is_object($group_image) && is_object($group_image->image)) {
Expand Down Expand Up @@ -137,7 +137,7 @@ public function index($group_id = null)
}

// ...and any other upcoming approved events
$other_upcoming_events = Party::future()->
$other_upcoming_events = Party::with('theGroup.networks')->future()->
whereNotIn('idevents', \Illuminate\Support\Arr::pluck($events, 'idevents'))->
get();

Expand Down Expand Up @@ -378,7 +378,8 @@ public function edit($id, Request $request)
$images = null;
}

$allGroups = Group::orderBy('name')->get();
// Fetch the nextwork here - avoids fetching for each group as we encode.
$allGroups = Group::with('networks')->orderBy('name')->get();

if ($request->isMethod('post') && ! empty($request->post())) {
$id = $request->post('id');
Expand Down
11 changes: 8 additions & 3 deletions app/Party.php
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@ class Party extends Model implements Auditable
];
protected $hidden = ['created_at', 'deleted_at', 'frequency', 'group', 'group', 'user_id', 'wordpress_post_id', 'cancelled', 'devices_updated_at'];

// Eager-loading the group reduces N+1 queries.
protected $with = 'theGroup';

// Append data to Model
protected $appends = ['participants', 'ShareableLink', 'event_date_local', 'start_local', 'end_local'];

Expand Down Expand Up @@ -334,9 +337,9 @@ public function scopeForUser($query, $userids = null) {
// The queries here are not desperately efficient, but we're battling Eloquent a bit. The data size is
// low enough it's not really an issue.
$this->defaultUserIds($userids);
$hostFor = Party::hostFor($userids);
$attending = Party::attendingOrAttended($userids);
$memberOf = Party::memberOfGroup($userids);
$hostFor = Party::with('theGroup.networks')->hostFor($userids);
$attending = Party::with('theGroup.networks')->attendingOrAttended($userids);
$memberOf = Party::with('theGroup.networks')->memberOfGroup($userids);

// In theory $query could contain something other than all().
return $query->whereIn('idevents', $hostFor->
Expand Down Expand Up @@ -385,6 +388,8 @@ public function scopeUpcomingEventsInUserArea($query, $user)
return $this
->select(DB::raw('`events`.*, ( 6371 * acos( cos( radians('.$user->latitude.') ) * cos( radians( events.latitude ) ) * cos( radians( events.longitude ) - radians('.$user->longitude.') ) + sin( radians('.$user->latitude.') ) * sin( radians( events.latitude ) ) ) ) AS distance'))
->join('groups', 'groups.idgroups', '=', 'events.group')
->join('group_network', 'groups.idgroups', '=', 'group_network.group_id')
->join('networks', 'networks.id', '=', 'group_network.network_id')
->join('users_groups', 'users_groups.group', '=', 'groups.idgroups')
->where(function ($query) use ($exclude_group_ids) {
$query->whereNotIn('events.group', $exclude_group_ids)
Expand Down
5 changes: 5 additions & 0 deletions app/Role.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use DB;
use Illuminate\Database\Eloquent\Model;
use Rennokki\QueryCache\Traits\QueryCacheable;

class Role extends Model
{
Expand All @@ -22,6 +23,10 @@ class Role extends Model
protected $table = 'roles';
protected $primaryKey = 'idroles';

use QueryCacheable;

protected $cacheFor = 180; // 3 minutes

/**
* The attributes that are mass assignable.
*
Expand Down
1 change: 1 addition & 0 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
"mcamara/laravel-localization": "^1.7",
"msurguy/honeypot": "^1.1",
"owen-it/laravel-auditing": "^12.1",
"rennokki/laravel-eloquent-query-cache": "^3.3",
"sentry/sentry-laravel": "^2.11",
"soundasleep/html2text": "^1.1",
"spatie/calendar-links": "^1.6",
Expand Down
70 changes: 69 additions & 1 deletion composer.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 9 additions & 0 deletions routes/web.php
Original file line number Diff line number Diff line change
Expand Up @@ -449,3 +449,12 @@
Route::get('/find', [StyleController::class, 'find']);
});
});

// Useful code to log all queries. This is particularly useful when trying to reduce the number of queries; if
// Laravel debug is turned on then the Queries tab on the client shows them briefly and then gets reset. That's
// long enough to spot pages with too many queries, but not long enough to see what they are.
//\DB::listen(function($sql) {
// \Log::info($sql->sql);
// \Log::info($sql->bindings);
// \Log::info($sql->time);
//});
2 changes: 1 addition & 1 deletion tests/Feature/Admin/Users/ViewUsersTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public function an_admin_can_view_list_of_users()
$response = $this->get('/user/all');

// Then the users should be in the list
$response->assertSeeText(e($users[0]->name));
$response->assertSeeText($users[0]->name);
}

/** @test */
Expand Down

0 comments on commit d259b18

Please sign in to comment.