[9.x] Prevent queries during response preparation

[timacdonald]

Related PR of Octane: laravel/octane#631

The problem

In many contexts, queries within a response object can be problematic and hard to detect. The response object may be a view(), a JsonResource, or something else entirely.

Two database queries are run in the following view. For some applications this may be totally fine, while others may find this problematic from a performance / resources / conventions perspective.

@foreach(['users' => 'Users', 'teams' => 'Teams'] as $table => $title)
  <dl>
      <dt>Total {{ $title }}</dt>
      <dd>{{ DB::table($table)->count() }}</dd>
  </dl>
@endforeach

The solution

To help applications detect these issues, I propose we introduce a new opt-in mechanism that will throw exceptions when a query is run while the response is being prepared.

// in a service provider or middleware

App::preventQueriesWhilePreparingResponse();

Response preparation involves the following code:

framework/src/Illuminate/Routing/Router.php

Lines 885 to 914 in c0affdd

	public static function toResponse($request, $response)
	{
	if ($response instanceof Responsable) {
	$response = $response->toResponse($request);
	}
	if ($response instanceof PsrResponseInterface) {
	$response = (new HttpFoundationFactory)->createResponse($response);
	} elseif ($response instanceof Model && $response->wasRecentlyCreated) {
	$response = new JsonResponse($response, 201);
	} elseif ($response instanceof Stringable) {
	$response = new Response($response->__toString(), 200, ['Content-Type' => 'text/html']);
	} elseif (! $response instanceof SymfonyResponse &&
	($response instanceof Arrayable ||
	$response instanceof Jsonable ||
	$response instanceof ArrayObject ||
	$response instanceof JsonSerializable ||
	$response instanceof stdClass ||
	is_array($response))) {
	$response = new JsonResponse($response);
	} elseif (! $response instanceof SymfonyResponse) {
	$response = new Response($response, 200, ['Content-Type' => 'text/html']);
	}
	if ($response->getStatusCode() === Response::HTTP_NOT_MODIFIED) {
	$response->setNotModified();
	}
	return $response->prepare($request);
	}

If an exception is thrown while the response is passing through this method, an exception will be thrown.

This means apps can throw if a query occurs in a blade view, as seen above, or while a JsonResource is being resolved. The following resource would throw an exception.

class TeamResource extends JsonResource
{
    public function toArray($request)
    {
        return [
            'name' => $this->name,
            'member_count' => $this->members()->count(), // query
        ];
    }
}

As soon as this resource is a collection, like in a UserResource that includes all the users teams, this will trigger a query for every team included in the resource.

There are other places where "response preparation" occurs, such as the exception handler, but this feature does not concern itself with that. It is only related to values returned from a Controller.

public function show(Team $team)
{
    return TeamResource::make($team);

    return view('teams.show', ['team' => $team]);
}

Doing it manually

As part of this main feature is a lower level feature for preventing queries for certain parts of an application.

DB::preventQueries(function () {
   // queries here will throw an exception.
});

// queries can run again.

DB::preventQueries();

// queries here will throw an exception.

DB::allowQueries();

// queries can run again.

This manual mechanism is handy when an application wants to prevent queries while a view is being prepared manually. Take the following example:

public function update(UpdateTeamRequest $request, Team $team)
{
    $team->update($request->validated());

    return response()->json($team);
}

This will allow queries to run when the toArray method is called even with preventQueriesWhilePreparingResponse in use. In this type of scenario where you are preparing the response yourself you may opt-in to prevent queries:

public function update(UpdateTeamRequest $request, Team $team)
{
    $team->update($request->validated());

    return DB::preventQueries(fn () => response()->json($team));
}

Note This could impact things like dispatching jobs to the queue. This is a sharp knife. Use wisely.

[bastien-phi]

Maybe a App::allowQueriesWhilePreparingResponse() is missing to rollback the behaviour.

[imanghafoori1]

App::preventBadPractices([
    'queriesInBlades',
    'envCallsOutsideConfig',
    'fullModelPathInPolymorphicRelation',
     ...
]);

[Rizky92]

Question, will this prevent unwanted extra queries in blade too? Something like calling aggregate function from relationship for example:

<ul>
    @foreach($someModelCollection as $model)
        <li>{{ $model->relationship()->count() }}</li>
    @endforeach
</ul>

[timacdonald]

@Rizky92 yes it will.

Given your example, if you had App::preventQueriesWhilePreparingResponse() in a service provider an exception would be thrown when $model->relationship()->count() is invoked.

@imanghafoori1 I wouldn't say queries in the view are inherently a bad practice. In certain teams, application sizes, and contexts they are perfectly fine. In others, not so much.

[garygreen]

Does this prevent relationships from lazily loading? E.g.

@foreach ($user->invoices as $invoice)
   ...
@endforeach

Would that throw if that relation wasn't eagerly loaded?

It is an interesting approach because it'll bring more awareness of db performance considerations when rendering views. I hope that this won't be frustrating to debug if it's throwing errors and you can't easily decipher where it came from exactly. This PR kinda reminds me of https://github.com/beyondcode/laravel-query-detector