Failed Conditions
Push — master ( b1d4df...5a9cf3 )
by David
04:05
created

Exporter::__construct()   A

Complexity

Conditions 1
Paths 1

Size

Total Lines 10
Code Lines 0

Duplication

Lines 0
Ratio 0 %

Code Coverage

Tests 1
CRAP Score 1

Importance

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

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

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