Exporter::__construct()   A
last analyzed

Complexity

Conditions 1
Paths 1

Size

Total Lines 10
Code Lines 0

Duplication

Lines 0
Ratio 0 %

Code Coverage

Tests 2
CRAP Score 1

Importance

Changes 0
Metric Value
eloc 0
c 0
b 0
f 0
dl 0
loc 10
rs 10
ccs 2
cts 2
cp 1
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\Enum\ExportFormat;
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\Service\Mailer;
14
use InvalidArgumentException;
15
use Throwable;
16
17
class Exporter
18
{
19 1
    public function __construct(
20
        private readonly ExportRepository $exportRepository,
21
        private readonly CardRepository $cardRepository,
22
        private readonly MessageQueuer $messageQueuer,
23
        private readonly Mailer $mailer,
24
        private readonly Writer $zip,
25
        private readonly Writer $pptx,
26
        private readonly Writer $csv,
27
        private readonly string $phpPath,
28 1
    ) {}
29
30
    /**
31
     * Export asynchronously in a separate process.
32
     *
33
     * This should be the preferred way to do big export
34
     */
35
    public function exportAsync(Export $export): void
36
    {
37
        $args = [
38
            realpath('bin/export.php'),
39
            $export->getId(),
40
        ];
41
42
        $escapedArgs = array_map('escapeshellarg', $args);
43
44
        // Forward SERVER_NAME to CLI command
45
        $env = 'SERVER_NAME=' . escapeshellarg(getenv('SERVER_NAME'));
46
47
        $cmd = $env . ' ' . escapeshellcmd($this->phpPath) . ' ' . implode(' ', $escapedArgs) . ' > /dev/null 2>&1 &';
48
        exec($cmd);
49
    }
50
51
    /**
52
     * Export immediately and return the v $export object.
53
     *
54
     * Because this method will indirectly clear the EntityManager any existing object
55
     * before calling this method will become invalid and must be re-fetched from DB.
56
     */
57 2
    public function export(Export $export): Export
58
    {
59 2
        $writer = $this->getWriter($export);
60 2
        $title = $export->getSite()->value . '-' . $export->getId();
61
62
        // Poor man's security by using hard-to-guess suffix
63 2
        $suffix = bin2hex(random_bytes(5));
64
65 2
        $filename = $title . '-' . $suffix . '.' . $writer->getExtension();
66
67 2
        $export->markAsInProgress($filename);
68 2
        _em()->flush();
69
70
        try {
71 2
            $writer->initialize($export, $title);
72 2
            $this->writeCards($writer, $export);
73 2
            $writer->finalize();
74
75 2
            $reloadExport = $this->reloadExport($export);
76 2
            $reloadExport->markAsDone();
77
        } catch (Throwable $throwable) {
78
            $reloadExport = $this->reloadExport($export);
79
            $reloadExport->markAsErrored($throwable);
80
81
            throw $throwable;
82
        } finally {
83 2
            _em()->flush();
84
        }
85
86 2
        return $reloadExport;
87
    }
88
89 2
    private function reloadExport(Export $export): Export
90
    {
91 2
        User::reloadCurrentUser();
92
93 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

93
        return $this->exportRepository->/** @scrutinizer ignore-call */ findOneById($export->getId());
Loading history...
94
    }
95
96
    /**
97
     * Export immediately and send an email to the export creator.
98
     *
99
     * This must only be used via CLI, because export might be very long (several minutes)
100
     * and the email is sent synchronously
101
     */
102
    public function exportAndSendMessage(int $id): void
103
    {
104
        /** @var null|Export $export */
105
        $export = $this->exportRepository->findOneById($id);
106
        if (!$export) {
107
            throw new InvalidArgumentException('Could not find export with ID: ' . $id);
108
        }
109
110
        $export = $this->export($export);
111
112
        $user = $export->getCreator();
113
        $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

113
        $message = $this->messageQueuer->queueExportDone(/** @scrutinizer ignore-type */ $user, $export);
Loading history...
114
115
        $this->mailer->sendMessage($message);
116
    }
117
118 2
    private function getWriter(Export $export): Writer
119
    {
120 2
        return match ($export->getFormat()) {
121 2
            ExportFormat::Zip => $this->zip,
122 2
            ExportFormat::Pptx => $this->pptx,
123 2
            ExportFormat::Csv => $this->csv,
124 2
        };
125
    }
126
127
    /**
128
     * Write all cards in small batches to avoid exploding PHP memory.
129
     */
130 2
    private function writeCards(Writer $writer, Export $export): void
131
    {
132 2
        $lastCardId = 0;
133 2
        while (true) {
134 2
            $cards = $this->cardRepository->getExportCards($export, $lastCardId);
135
            // If nothing to process anymore, stop the whole thing
136 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...
137 2
                break;
138
            }
139
140 2
            foreach ($cards as $card) {
141 2
                $writer->write($card);
142 2
                $lastCardId = $card->getId();
143
            }
144
145 2
            _em()->flush();
146 2
            _em()->clear();
147
        }
148
    }
149
}
150