Complex classes like MailQueueHandler often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.
Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.
While breaking up the class, it is a good idea to analyze how other classes use MailQueueHandler, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
48 | class MailQueueHandler { |
||
49 | |||
50 | const CLI_EMAIL_BATCH_SIZE = 500; |
||
51 | |||
52 | const WEB_EMAIL_BATCH_SIZE = 25; |
||
53 | |||
54 | /** Number of entries we want to list in the email */ |
||
55 | const ENTRY_LIMIT = 200; |
||
56 | |||
57 | /** @var array */ |
||
58 | protected $languages; |
||
59 | |||
60 | /** @var string */ |
||
61 | protected $senderAddress; |
||
62 | |||
63 | /** @var string */ |
||
64 | protected $senderName; |
||
65 | |||
66 | /** @var IDateTimeFormatter */ |
||
67 | protected $dateFormatter; |
||
68 | |||
69 | /** @var DataHelper */ |
||
70 | protected $dataHelper; |
||
71 | |||
72 | /** @var IDBConnection */ |
||
73 | protected $connection; |
||
74 | |||
75 | /** @var IMailer */ |
||
76 | protected $mailer; |
||
77 | |||
78 | /** @var IURLGenerator */ |
||
79 | protected $urlGenerator; |
||
80 | |||
81 | /** @var IUserManager */ |
||
82 | protected $userManager; |
||
83 | |||
84 | /** @var IFactory */ |
||
85 | protected $lFactory; |
||
86 | |||
87 | /** @var IManager */ |
||
88 | protected $activityManager; |
||
89 | |||
90 | /** @var LegacyParser */ |
||
91 | protected $legacyParser; |
||
92 | |||
93 | /** @var IConfig */ |
||
94 | protected $config; |
||
95 | |||
96 | /** @var ILogger */ |
||
97 | protected $logger; |
||
98 | |||
99 | /** |
||
100 | * Constructor |
||
101 | * |
||
102 | * @param IDateTimeFormatter $dateFormatter |
||
103 | * @param IDBConnection $connection |
||
104 | * @param DataHelper $dataHelper |
||
105 | * @param IMailer $mailer |
||
106 | * @param IURLGenerator $urlGenerator |
||
107 | * @param IUserManager $userManager |
||
108 | * @param IFactory $lFactory |
||
109 | * @param IManager $activityManager |
||
110 | * @param LegacyParser $legacyParser |
||
111 | * @param IConfig $config |
||
112 | * @param ILogger $logger |
||
113 | */ |
||
114 | 10 | public function __construct(IDateTimeFormatter $dateFormatter, |
|
137 | |||
138 | /** |
||
139 | * Send an email to {$limit} users |
||
140 | * |
||
141 | * @param int $limit Number of users we want to send an email to |
||
142 | * @param int $sendTime The latest send time |
||
143 | * @param bool $forceSending Ignores latest send and just sends all emails |
||
144 | * @param null|int $restrictEmails null or one of UserSettings::EMAIL_SEND_* |
||
145 | * @return int Number of users we sent an email to |
||
146 | */ |
||
147 | public function sendEmails($limit, $sendTime, $forceSending = false, $restrictEmails = null) { |
||
204 | |||
205 | /** |
||
206 | * Get the users we want to send an email to |
||
207 | * |
||
208 | * @param int|null $limit |
||
209 | * @param int $latestSend |
||
210 | * @param bool $forceSending |
||
211 | * @param int|null $restrictEmails |
||
212 | * @return array |
||
213 | */ |
||
214 | 6 | protected function getAffectedUsers($limit, $latestSend, $forceSending, $restrictEmails) { |
|
254 | |||
255 | /** |
||
256 | * Get all items for the user we want to send an email to |
||
257 | * |
||
258 | * @param string $affectedUser |
||
259 | * @param int $maxTime |
||
260 | * @param int $maxNumItems |
||
261 | * @return array [data of the first max. 200 entries, total number of entries] |
||
262 | */ |
||
263 | 7 | protected function getItemsForUser($affectedUser, $maxTime, $maxNumItems = self::ENTRY_LIMIT) { |
|
264 | 7 | $query = $this->connection->getQueryBuilder(); |
|
265 | 7 | $query->select('*') |
|
266 | 7 | ->from('activity_mq') |
|
267 | 7 | ->where($query->expr()->lte('amq_timestamp', $query->createNamedParameter($maxTime))) |
|
268 | 7 | ->andWhere($query->expr()->eq('amq_affecteduser', $query->createNamedParameter($affectedUser))) |
|
269 | 7 | ->orderBy('amq_timestamp', 'ASC') |
|
270 | 7 | ->setMaxResults($maxNumItems); |
|
271 | 7 | $result = $query->execute(); |
|
272 | |||
273 | 7 | $activities = []; |
|
274 | 7 | while ($row = $result->fetch()) { |
|
275 | 7 | $activities[] = $row; |
|
276 | } |
||
277 | 7 | $result->closeCursor(); |
|
278 | |||
279 | 7 | if (isset($activities[$maxNumItems - 1])) { |
|
280 | // Reached the limit, run a query to get the actual count. |
||
281 | 1 | $query = $this->connection->getQueryBuilder(); |
|
282 | 1 | $query->selectAlias($query->func()->count('*'), 'actual_count') |
|
283 | 1 | ->from('activity_mq') |
|
284 | 1 | ->where($query->expr()->lte('amq_timestamp', $query->createNamedParameter($maxTime))) |
|
285 | 1 | ->andWhere($query->expr()->eq('amq_affecteduser', $query->createNamedParameter($affectedUser))); |
|
286 | 1 | $result = $query->execute(); |
|
287 | 1 | $row = $result->fetch(); |
|
288 | 1 | $result->closeCursor(); |
|
289 | |||
290 | 1 | return [$activities, $row['actual_count'] - $maxNumItems]; |
|
291 | } |
||
292 | |||
293 | 7 | return [$activities, 0]; |
|
294 | } |
||
295 | |||
296 | /** |
||
297 | * Get a language object for a specific language |
||
298 | * |
||
299 | * @param string $lang Language identifier |
||
300 | * @return \OCP\IL10N Language object of $lang |
||
301 | */ |
||
302 | 1 | protected function getLanguage($lang) { |
|
309 | |||
310 | /** |
||
311 | * Get the sender data |
||
312 | * @param string $setting Either `email` or `name` |
||
313 | * @return string |
||
314 | */ |
||
315 | 1 | protected function getSenderData($setting) { |
|
329 | |||
330 | /** |
||
331 | * Send a notification to one user |
||
332 | * |
||
333 | * @param string $userName Username of the recipient |
||
334 | * @param string $email Email address of the recipient |
||
335 | * @param string $lang Selected language of the recipient |
||
336 | * @param string $timezone Selected timezone of the recipient |
||
337 | * @param int $maxTime |
||
338 | * @return bool True if the entries should be removed, false otherwise |
||
339 | * @throws \UnexpectedValueException |
||
340 | */ |
||
341 | 1 | protected function sendEmailToUser($userName, $email, $lang, $timezone, $maxTime) { |
|
429 | |||
430 | /** |
||
431 | * @param IEvent $event |
||
432 | * @return string |
||
433 | */ |
||
434 | 1 | protected function getHTMLSubject(IEvent $event): string { |
|
454 | |||
455 | /** |
||
456 | * @param string $lang |
||
457 | * @param IEvent $event |
||
458 | * @return IEvent |
||
459 | * @throws \InvalidArgumentException when the event could not be parsed |
||
460 | */ |
||
461 | 1 | protected function parseEvent($lang, IEvent $event) { |
|
479 | |||
480 | /** |
||
481 | * Delete all entries we dealt with |
||
482 | * |
||
483 | * @param array $affectedUsers |
||
484 | * @param int $maxTime |
||
485 | */ |
||
486 | 5 | protected function deleteSentItems(array $affectedUsers, $maxTime) { |
|
497 | } |
||
498 |
This error could be the result of:
1. Missing dependencies
PHP Analyzer uses your
composer.json
file (if available) to determine the dependencies of your project and to determine all the available classes and functions. It expects thecomposer.json
to be in the root folder of your repository.Are you sure this class is defined by one of your dependencies, or did you maybe not list a dependency in either the
require
orrequire-dev
section?2. Missing use statement
PHP does not complain about undefined classes in
ìnstanceof
checks. For example, the following PHP code will work perfectly fine:If you have not tested against this specific condition, such errors might go unnoticed.