foreach内でクエリは発行しちゃダメですね

PDFファイルを一括でダウンロードできる機能を作成しましたが、高負荷なクエリがあった部分をリファクタリングしたので、その記録を残しておきます。

よくやってしまいがち（私だけ？）ですが、foreachの中でクエリを発行してしまっておりました。

    public function downloadMultiplePDFs(Request $request)
    {
        $invoiceIds = $request->input('invoice_ids');
        $zip = new ZipArchive;
        $zipFileName = 'invoice.zip';

        if (empty($invoiceIds)) {
            return redirect()
                ->route('invoice.index')
                ->with('message', 'ダウンロードできる請求がありません');
        }

        if ($zip->open(storage_path($zipFileName), ZipArchive::CREATE) === TRUE) {
            foreach ($invoiceIds as $invoice_id) {
                // ここでクエリを発行しているのが良くない
                $invoice = $this->viewListInvoiceService->findByOne($invoice_id);
                $data['endOfMonth'] = $this->invoiceDownloadPDFService->getEndOfMonth($invoice);
                $data += $this->invoiceDownloadPDFService->getTotalPriceWithTax($invoice);
                $fileName = $this->invoiceDownloadPDFService->generateFilename($invoice);
                $pdf = PDF::loadView('pdf.invoice', compact('invoice', 'data'));

                // 一時ファイルにPDFを保存
                $tempPath = storage_path('app/temp/' . $fileName);
                $pdf->save($tempPath);

                // ZIPファイルに追加
                $zip->addFile($tempPath, $fileName);
            }
            $zip->close();
        }

        return response()->download(storage_path($zipFileName))->deleteFileAfterSend(true);

上記だとforeachの中で毎回データを取りにいってるので、量が増えると高負荷になってしまいますね…

なのでforeachの前でwhereInでデータを一回で全て取得してから、foreachで取得したデータ分をループさせるという方法にすればよさそうです。

    public function downloadMultiplePDFs(Request $request)
    {
        $invoiceIds = $request->input('invoice_ids');
        $zip = new ZipArchive;
        $zipFileName = 'invoice.zip';

        if (empty($invoiceIds)) {
            return redirect()
                ->route('invoice.index')
                ->with('message', 'ダウンロードできる請求がありません');
        }
        // ここで一回で取得する
        $invoices = $this->viewListInvoiceService->findByIds($invoiceIds)->get();
        if ($zip->open(storage_path($zipFileName), ZipArchive::CREATE) === TRUE) {
            foreach ($invoices as $invoice) {
                $data['endOfMonth'] = $this->invoiceDownloadPDFService->getEndOfMonth($invoice);
                $data += $this->invoiceDownloadPDFService->getTotalPriceWithTax($invoice);
                $fileName = $this->invoiceDownloadPDFService->generateFilename($invoice);
                $pdf = PDF::loadView('pdf.invoice', compact('invoice', 'data'));

                // 一時ファイルにPDFを保存
                $tempPath = storage_path('app/temp/' . $fileName);
                $pdf->save($tempPath);

                // ZIPファイルに追加
                $zip->addFile($tempPath, $fileName);
            }
            $zip->close();
        }

        return response()->download(storage_path($zipFileName))->deleteFileAfterSend(true);
    }

$this->viewListInvoiceService->findByIds($invoiceIds)の部分でデータ取得してます。
service層は省略するとして、repositoryでデータベースから取得は下記のように書きました。

    public function findByIds(array $invoiceIds, array $relations)
    {
        return $this->invoice
            ->with($relations)
            ->whereIn('id', $invoiceIds);
    }

これで何回もデータベースにアクセスしなくてもよさそうです。
以上です。