Failed Conditions
Push — master ( 27554e...857869 )
by Luca
09:08
created

Exporter::__construct()   A

Complexity

Conditions 1
Paths 1

Size

Total Lines 2
Code Lines 0

Duplication

Lines 0
Ratio 0 %

Code Coverage

Tests 1
CRAP Score 1

Importance

Changes 1
Bugs 0 Features 0
Metric Value
eloc 0
dl 0
loc 2
ccs 1
cts 1
cp 1
rs 10
c 1
b 0
f 0
cc 1
nc 1
nop 8
crap 1

How to fix   Many Parameters   

Many Parameters

Methods with many parameters are not only hard to understand, but their parameters also often become inconsistent when you need more, or different data.

There are several approaches to avoid long parameter lists:

1
<?php
2
3
declare(strict_types=1);
4
5
namespace Application\Service\Exporter;
6
7
use Application\DBAL\Types\ExportFormatType;
8
use Application\Model\Export;
9
use Application\Model\User;
10
use Application\Repository\CardRepository;
11
use Application\Repository\ExportRepository;
12
use Application\Service\MessageQueuer;
13
use Ecodev\Felix\Api\Exception;
14
use Ecodev\Felix\Service\Mailer;
15
use InvalidArgumentException;
16
use Throwable;
17
18
class Exporter
19
{
20 1
    public function __construct(private readonly ExportRepository $exportRepository, private readonly CardRepository $cardRepository, private readonly MessageQueuer $messageQueuer, private readonly Mailer $mailer, private readonly Writer $zip, private readonly Writer $pptx, private readonly Writer $csv, private readonly string $phpPath)
21
    {
22 1
    }
23
24
    /**
25
     * Export asynchronously in a separate process.
26
     *
27
     * This should be the preferred way to do big export
28
     */
29
    public function exportAsync(Export $export): void
30
    {
31
        $args = [
32
            realpath('bin/export.php'),
33
            $export->getId(),
34
        ];
35
36
        $escapedArgs = array_map('escapeshellarg', $args);
37
38
        // Forward SERVER_NAME to CLI command
39
        $env = 'SERVER_NAME=' . escapeshellarg(getenv('SERVER_NAME'));
40
41
        $cmd = $env . ' ' . escapeshellcmd($this->phpPath) . ' ' . implode(' ', $escapedArgs) . ' > /dev/null 2>&1 &';
42
        exec($cmd);
43
    }
44
45
    /**
46
     * Export immediately and return the v $export object.
47
     *
48
     * Because this method will indirectly clear the EntityManager any existing object
49
     * before calling this method will become invalid and must be re-fetched from DB.
50
     */
51 2
    public function export(Export $export): Export
52
    {
53 2
        $writer = $this->getWriter($export);
54 2
        $title = $export->getSite() . '-' . $export->getId();
55
56
        // Poor man's security by using hard-to-guess suffix
57 2
        $suffix = bin2hex(random_bytes(5));
58
59 2
        $filename = $title . '-' . $suffix . '.' . $writer->getExtension();
60
61 2
        $export->markAsInProgress($filename);
62 2
        _em()->flush();
63
64
        try {
65 2
            $writer->initialize($export, $title);
66 2
            $this->writeCards($writer, $export);
67 2
            $writer->finalize();
68
69 2
            $reloadExport = $this->reloadExport($export);
70 2
            $reloadExport->markAsDone();
71
        } catch (Throwable $throwable) {
72
            $reloadExport = $this->reloadExport($export);
73
            $reloadExport->markAsErrored($throwable);
74
75
            throw $throwable;
76
        } finally {
77 2
            _em()->flush();
78
        }
79
80 2
        return $reloadExport;
81
    }
82
83 2
    private function reloadExport(Export $export): Export
84
    {
85 2
        User::reloadCurrentUser();
86
87 2
        return $this->exportRepository->findOneById($export->getId());
0 ignored issues
show
Bug introduced by
The method findOneById() does not exist on Application\Repository\ExportRepository. Since you implemented __call, consider adding a @method annotation. ( Ignorable by Annotation )

If this is a false-positive, you can also ignore this issue in your code via the ignore-call  annotation

87
        return $this->exportRepository->/** @scrutinizer ignore-call */ findOneById($export->getId());
Loading history...
88
    }
89
90
    /**
91
     * Export immediately and send an email to the export creator.
92
     *
93
     * This must only be used via CLI, because export might be very long (several minutes)
94
     * and the email is sent synchronously
95
     */
96
    public function exportAndSendMessage(int $id): void
97
    {
98
        /** @var null|Export $export */
99
        $export = $this->exportRepository->findOneById($id);
100
        if (!$export) {
101
            throw new InvalidArgumentException('Could not find export with ID: ' . $id);
102
        }
103
104
        $export = $this->export($export);
105
106
        $user = $export->getCreator();
107
        $message = $this->messageQueuer->queueExportDone($user, $export);
0 ignored issues
show
Bug introduced by
It seems like $user can also be of type null; however, parameter $user of Application\Service\Mess...euer::queueExportDone() does only seem to accept Application\Model\User, maybe add an additional type check? ( Ignorable by Annotation )

If this is a false-positive, you can also ignore this issue in your code via the ignore-type  annotation

107
        $message = $this->messageQueuer->queueExportDone(/** @scrutinizer ignore-type */ $user, $export);
Loading history...
108
109
        $this->mailer->sendMessage($message);
110
    }
111
112 2
    private function getWriter(Export $export): Writer
113
    {
114 2
        return match ($export->getFormat()) {
115 2
            ExportFormatType::ZIP => $this->zip,
116 2
            ExportFormatType::PPTX => $this->pptx,
117 2
            ExportFormatType::CSV => $this->csv,
118 2
            default => throw new Exception('Invalid export format:' . $export->getFormat()),
119 2
        };
120
    }
121
122
    /**
123
     * Write all cards in small batches to avoid exploding PHP memory.
124
     */
125 2
    private function writeCards(Writer $writer, Export $export): void
126
    {
127 2
        $lastCardId = 0;
128 2
        while (true) {
129 2
            $cards = $this->cardRepository->getExportCards($export, $lastCardId);
130
            // If nothing to process anymore, stop the whole thing
131 2
            if (!$cards) {
0 ignored issues
show
Bug Best Practice introduced by
The expression $cards of type array is implicitly converted to a boolean; are you sure this is intended? If so, consider using empty($expr) instead to make it clear that you intend to check for an array without elements.

This check marks implicit conversions of arrays to boolean values in a comparison. While in PHP an empty array is considered to be equal (but not identical) to false, this is not always apparent.

Consider making the comparison explicit by using empty(..) or ! empty(...) instead.

Loading history...
132 2
                break;
133
            }
134
135 2
            foreach ($cards as $card) {
136 2
                $writer->write($card);
137 2
                $lastCardId = $card->getId();
138
            }
139
140 2
            _em()->flush();
141 2
            _em()->clear();
142
        }
143
    }
144
}
145