Mailer::buildMessage() - Code Metrics - Inspection of "Add batch messaging support (closes #3)" - kinglozzer/silverstripe-mailgunner - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Branch — master (7a4c7d)

by Loz

created 2016-03-01 14:24 UTC

Mailer::buildMessage() F

↳ Parent: Mailer

Complexity

Conditions	13
Paths	1024

Size

Total Lines	65
Code Lines	33

Duplication

Lines	0
Ratio	0 %

Code Coverage

Tests	44
CRAP Score	13.0018

Importance

Changes	5
Bugs	0	Features	3

Metric	Value
c	5
b	0
f	3
dl	0
loc	65
ccs	44
cts	45
cp	0.9778
rs	2.7063
cc	13
eloc	33
nc	1024
nop	8
crap	13.0018

How to fix Long Method Complexity Many Parameters

<?php

namespace Kinglozzer\SilverStripeMailgunner;

use Mailer as SilverstripeMailer;
use Mailgun\Mailgun;
use Mailgun\Messages\MessageBuilder;

class Mailer extends SilverstripeMailer
{
    /**
     * @var string
     * @config
     */
    private static $api_domain = '';

    /**
     * @var string
     * @config
     */
    private static $api_endpoint = 'api.mailgun.net';

    /**
     * @var string
     * @config
     */
    private static $api_key = '';

    /**
     * @var boolean
     * @config
     */
    private static $api_ssl = true;

    /**
     * @var string
     * @config
     */
    private static $api_version = 'v3';

    /**
     * An array of temporary file handles opened to store attachments
     * @var array
     */
    protected $tempFileHandles = [];

    /**
     * @var Mailgun
     */
    protected $mailgunClient;

    /**
     * {@inheritdoc}
     */
    public function __construct()
    {
        $config = $this->config();
        $this->setMailgunClient(new Mailgun(
            $config->api_key,
            $config->api_endpoint,
            $config->api_version,
            $config->api_ssl
        ));
    }

    /**
     * @param Mailgun $client
     * @return self
     */
    public function setMailgunClient(Mailgun $client)
    {
        $this->mailgunClient = $client;
        return $this;
    }

    /**
     * @return Mailgun
     */
    public function getMailgunClient()
    {
        return $this->mailgunClient;
    }

    /**
     * {@inheritdoc}
     */
    public function sendPlain($to, $from, $subject, $plainContent, $attachments = [], $headers = [])
    {
        $this->sendMessage($to, $from, $subject, $htmlContent = '', $plainContent, $attachments, $headers);
    }

    /**
     * {@inheritdoc}
     */
    public function sendHTML($to, $from, $subject, $htmlContent, $attachments = [], $headers = [], $plainContent = '')
    {
        $this->sendMessage($to, $from, $subject, $htmlContent, $plainContent, $attachments, $headers);
    }

    /**
     * @param string $to
     * @param string $from
     * @param string $subject
     * @param string $content
     * @param string $plainContent
     * @param array $attachments
     * @param array $headers
     */
    protected function sendMessage($to, $from, $subject, $content, $plainContent, $attachments, $headers)
    {
        $domain = $this->config()->api_domain;
        $client = $this->getMailgunClient();

        if(isset($headers['X-Mailgunner-Batch-Message'])) {
            $messageBuilder = $client->BatchMessage($domain);
            unset($headers['X-Mailgunner-Batch-Message']);
        } else {
            $messageBuilder = $client->MessageBuilder();
        }
        
        if (!empty($attachments)) {
            $attachments = $this->prepareAttachments($attachments);
        }

        try {
            $this->buildMessage($messageBuilder, $to, $from, $subject, $content, $plainContent, $attachments, $headers);
function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);

            if ($messageBuilder instanceof \Mailgun\Messages\BatchMessage) {
                $messageBuilder->finalize();
            } else {
                $client->sendMessage($domain, $messageBuilder->getMessage(), $messageBuilder->getFiles());
            }
        } catch (\Exception $e) {
            // Close and remove any temp files created for attachments, then let the exception bubble up
            $this->closeTempFileHandles();
            throw $e;
        }

        $this->closeTempFileHandles();
    }

    /**
     * @param MessageBuilder $builder
     * @param string $to
     * @param string $from
     * @param string $subject
     * @param string $content
     * @param string $plainContent
     * @param string $attachments
     * @param array $headers
     */
    protected function buildMessage(MessageBuilder $builder, $to, $from, $subject, $content, $plainContent, $attachments, $headers)
    {
        // Add base info
        $parsedFrom = $this->parseAddresses($from);
        if (!empty($parsedFrom)) {
            foreach ($parsedFrom as $email => $name) {
                $builder->setFromAddress($email, ['full_name' => $name]);
            }
        } else {
            $builder->setFromAddress($from);
        }
        
        $builder->setSubject($subject);

        // HTML content (if not empty)
        if ($content) {
            $builder->setHtmlBody($content);
        }

        // Plain text content (if not empty)
        if ($plainContent) {
            $builder->setTextBody($plainContent);
        }

        // Add attachments
        if (!empty($attachments)) {
            foreach ($attachments as $attachment) {

                $builder->addAttachment($attachment['filePath'], $attachment['remoteName']);
            }
        }

        // Parse Cc & Bcc headers out if they're set
        $ccAddresses = $bccAddresses = '';
        if (isset($headers['Cc'])) {
            $ccAddresses = $headers['Cc'];
            unset($headers['Cc']);
        }

        if (isset($headers['Bcc'])) {
            $bccAddresses = $headers['Bcc'];
            unset($headers['Bcc']);
        }

        // Add remaining custom headers
        foreach ($headers as $name => $data) {
            $builder->addCustomHeader($name, $data);
        }

        // Add recipients. This is done last as the 'BatchMessage' message builder
        // will trigger sends for every 1000 addresses
        $to = $this->parseAddresses($to);
        foreach ($to as $email => $name) {
            $builder->addToRecipient($email, ['full_name' => $name]);
        }

        $ccAddresses = $this->parseAddresses($ccAddresses);
        foreach ($ccAddresses as $email => $name) {
            $builder->addCcRecipient($email, ['full_name' => $name]);
        }

        $bccAddresses = $this->parseAddresses($bccAddresses);
        foreach ($bccAddresses as $email => $name) {
            $builder->addBccRecipient($email, ['full_name' => $name]);
        }
    }

    /**
     * @todo This can't deal with mismatched quotes, or commas in names.
     *       E.g. "Smith, John" <[email protected]> or "John O'smith" <[email protected]>
     * @param string
     * @return array
     */
    protected function parseAddresses($addresses)
    {
        $parsed = [];

        $expr = '/\s*["\']?([^><,;"\']+)["\']?\s*((?:<[^><,]+>)?)\s*/';
        if (preg_match_all($expr, $addresses, $matches, PREG_SET_ORDER) > 0) {
            foreach ($matches as $result) {
                if (empty($result[2])) {
                    // If we couldn't parse out a name
                    $parsed[$result[1]] = '';
                } else {
                    $email = trim($result[2], '<>');
                    $parsed[$email] = trim($result[1]);
                }
            }
        }

        return $parsed;
    }

    /**
     * Prepare attachments for sending. SilverStripe extracts the content and
     * passes that to the mailer, so to save encoding it we just write them all
     * to individual files and let Mailgun deal with the rest.
     * 
     * @todo Can we handle this better?
     * @param array $attachments
     * @return array
     */
    protected function prepareAttachments(array $attachments)
    {
        $prepared = [];
            
        foreach ($attachments as $attachment) {
            $tempFile = $this->writeToTempFile($attachment['contents']);
            
            $prepared[] = [
                'filePath' => $tempFile,
                'remoteName' => $attachment['filename']
            ];
        }

        return $prepared;
    }

    /**
     * @param string $contents
     * @return string
     */
    protected function writeToTempFile($contents)
    {
        $tempFile = tempnam(sys_get_temp_dir(), 'SS_MG_TMP');
        $fileHandle = fopen($tempFile, 'r+');
        fwrite($fileHandle, $contents);

        $this->tempFileHandles[] = [
            'handle' => $fileHandle,
            'path' => $tempFile
        ];

        return $tempFile;
    }

    /**
     * @return void
     */
    protected function closeTempFileHandles()
    {
        foreach ($this->tempFileHandles as $key => $data) {
            fclose($data['handle']);
            unlink($data['path']);
            unset($this->tempFileHandles[$key]);
        }
    }
}


1		<?php
2
3		namespace Kinglozzer\SilverStripeMailgunner;
4
5		use Mailer as SilverstripeMailer;
6		use Mailgun\Mailgun;
7		use Mailgun\Messages\MessageBuilder;
8
9		class Mailer extends SilverstripeMailer
10		{
11		/**
12		* @var string
13		* @config
14		*/
15		private static $api_domain = '';
16
17		/**
18		* @var string
19		* @config
20		*/
21		private static $api_endpoint = 'api.mailgun.net';
22
23		/**
24		* @var string
25		* @config
26		*/
27		private static $api_key = '';
28
29		/**
30		* @var boolean
31		* @config
32		*/
33		private static $api_ssl = true;
34
35		/**
36		* @var string
37		* @config
38		*/
39		private static $api_version = 'v3';
40
41		/**
42		* An array of temporary file handles opened to store attachments
43		* @var array
44		*/
45		protected $tempFileHandles = [];
46
47		/**
48		* @var Mailgun
49		*/
50		protected $mailgunClient;
51
52		/**
53		* {@inheritdoc}
54		*/
55	12	public function __construct()
56		{
57	12	$config = $this->config();
58	12	$this->setMailgunClient(new Mailgun(
59	12	$config->api_key,
60	12	$config->api_endpoint,
61	12	$config->api_version,
62	12	$config->api_ssl
63	12	));
64	12	}
65
66		/**
67		* @param Mailgun $client
68		* @return self
69		*/
70	12	public function setMailgunClient(Mailgun $client)
71		{
72	12	$this->mailgunClient = $client;
73	12	return $this;
74		}
75
76		/**
77		* @return Mailgun
78		*/
79	1	public function getMailgunClient()
80		{
81	1	return $this->mailgunClient;
82		}
83
84		/**
85		* {@inheritdoc}
86		*/
87	1	public function sendPlain($to, $from, $subject, $plainContent, $attachments = [], $headers = [])
88		{
89	1	$this->sendMessage($to, $from, $subject, $htmlContent = '', $plainContent, $attachments, $headers);
90	1	}
91
92		/**
93		* {@inheritdoc}
94		*/
95	1	public function sendHTML($to, $from, $subject, $htmlContent, $attachments = [], $headers = [], $plainContent = '')
96		{
97	1	$this->sendMessage($to, $from, $subject, $htmlContent, $plainContent, $attachments, $headers);
98	1	}
99
100		/**
101		* @param string $to
102		* @param string $from
103		* @param string $subject
104		* @param string $content
105		* @param string $plainContent
106		* @param array $attachments
107		* @param array $headers
108		*/
109	3	protected function sendMessage($to, $from, $subject, $content, $plainContent, $attachments, $headers)
110		{
111	3	$domain = $this->config()->api_domain;
112	3	$client = $this->getMailgunClient();
113
114	3	if(isset($headers['X-Mailgunner-Batch-Message'])) {
115	1	$messageBuilder = $client->BatchMessage($domain);
116	1	unset($headers['X-Mailgunner-Batch-Message']);
117	1	} else {
118	2	$messageBuilder = $client->MessageBuilder();
119		}
120
121	3	if (!empty($attachments)) {
122	3	$attachments = $this->prepareAttachments($attachments);
123	3	}
124
125		try {
126	3	$this->buildMessage($messageBuilder, $to, $from, $subject, $content, $plainContent, $attachments, $headers);
		0 ignored issues – show Documentation introduced 2016-03-01 14:27 UTC by Report Bug Copy Issue Report `$attachments` is of type `array`, but the function expects a `string`. It seems like the type of the argument is not accepted by the function/method which you are calling. In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug. We suggest to add an explicit type cast like in the following example: function acceptsInteger($int) { } $x = '123'; // string "123" // Instead of acceptsInteger($x); // we recommend to use acceptsInteger((integer) $x); Loading history...
127
128	3	if ($messageBuilder instanceof \Mailgun\Messages\BatchMessage) {
129	1	$messageBuilder->finalize();
130	1	} else {
131	2	$client->sendMessage($domain, $messageBuilder->getMessage(), $messageBuilder->getFiles());
132		}
133	3	} catch (\Exception $e) {
134		// Close and remove any temp files created for attachments, then let the exception bubble up
135	1	$this->closeTempFileHandles();
136	1	throw $e;
137		}
138
139	2	$this->closeTempFileHandles();
140	2	}
141
142		/**
143		* @param MessageBuilder $builder
144		* @param string $to
145		* @param string $from
146		* @param string $subject
147		* @param string $content
148		* @param string $plainContent
149		* @param string $attachments
150		* @param array $headers
151		*/
152	2	protected function buildMessage(MessageBuilder $builder, $to, $from, $subject, $content, $plainContent, $attachments, $headers)
153		{
154		// Add base info
155	2	$parsedFrom = $this->parseAddresses($from);
156	2	if (!empty($parsedFrom)) {
157	2	foreach ($parsedFrom as $email => $name) {
158	2	$builder->setFromAddress($email, ['full_name' => $name]);
159	2	}
160	2	} else {
161		$builder->setFromAddress($from);
162		}
163
164	2	$builder->setSubject($subject);
165
166		// HTML content (if not empty)
167	2	if ($content) {
168	2	$builder->setHtmlBody($content);
169	2	}
170
171		// Plain text content (if not empty)
172	2	if ($plainContent) {
173	2	$builder->setTextBody($plainContent);
174	2	}
175
176		// Add attachments
177	2	if (!empty($attachments)) {
178	2	foreach ($attachments as $attachment) {
		0 ignored issues – show Bug introduced 2016-03-01 14:27 UTC by Report Bug Copy Issue Report The expression `$attachments` of type `string` is not traversable. Loading history...
179	2	$builder->addAttachment($attachment['filePath'], $attachment['remoteName']);
180	2	}
181	2	}
182
183		// Parse Cc & Bcc headers out if they're set
184	2	$ccAddresses = $bccAddresses = '';
185	2	if (isset($headers['Cc'])) {
186	2	$ccAddresses = $headers['Cc'];
187	2	unset($headers['Cc']);
188	2	}
189
190	2	if (isset($headers['Bcc'])) {
191	2	$bccAddresses = $headers['Bcc'];
192	2	unset($headers['Bcc']);
193	2	}
194
195		// Add remaining custom headers
196	2	foreach ($headers as $name => $data) {
197	2	$builder->addCustomHeader($name, $data);
198	2	}
199
200		// Add recipients. This is done last as the 'BatchMessage' message builder
201		// will trigger sends for every 1000 addresses
202	2	$to = $this->parseAddresses($to);
203	2	foreach ($to as $email => $name) {
204	2	$builder->addToRecipient($email, ['full_name' => $name]);
205	2	}
206
207	2	$ccAddresses = $this->parseAddresses($ccAddresses);
208	2	foreach ($ccAddresses as $email => $name) {
209	2	$builder->addCcRecipient($email, ['full_name' => $name]);
210	2	}
211
212	2	$bccAddresses = $this->parseAddresses($bccAddresses);
213	2	foreach ($bccAddresses as $email => $name) {
214	2	$builder->addBccRecipient($email, ['full_name' => $name]);
215	2	}
216	2	}
217
218		/**
219		* @todo This can't deal with mismatched quotes, or commas in names.
220		* E.g. "Smith, John" <[email protected]> or "John O'smith" <[email protected]>
221		* @param string
222		* @return array
223		*/
224	3	protected function parseAddresses($addresses)
225		{
226	3	$parsed = [];
227
228	3	$expr = '/\s["\']?([^><,;"\']+)["\']?\s((?:<[^><,]+>)?)\s*/';
229	3	if (preg_match_all($expr, $addresses, $matches, PREG_SET_ORDER) > 0) {
230	3	foreach ($matches as $result) {
231	3	if (empty($result[2])) {
232		// If we couldn't parse out a name
233	3	$parsed[$result[1]] = '';
234	3	} else {
235	3	$email = trim($result[2], '<>');
236	3	$parsed[$email] = trim($result[1]);
237		}
238	3	}
239	3	}
240
241	3	return $parsed;
242		}
243
244		/**
245		* Prepare attachments for sending. SilverStripe extracts the content and
246		* passes that to the mailer, so to save encoding it we just write them all
247		* to individual files and let Mailgun deal with the rest.
248		*
249		* @todo Can we handle this better?
250		* @param array $attachments
251		* @return array
252		*/
253	2	protected function prepareAttachments(array $attachments)
254		{
255	2	$prepared = [];
256
257	2	foreach ($attachments as $attachment) {
258	2	$tempFile = $this->writeToTempFile($attachment['contents']);
259
260	2	$prepared[] = [
261	2	'filePath' => $tempFile,
262	2	'remoteName' => $attachment['filename']
263	2	];
264	2	}
265
266	2	return $prepared;
267		}
268
269		/**
270		* @param string $contents
271		* @return string
272		*/
273	2	protected function writeToTempFile($contents)
274		{
275	2	$tempFile = tempnam(sys_get_temp_dir(), 'SS_MG_TMP');
276	2	$fileHandle = fopen($tempFile, 'r+');
277	2	fwrite($fileHandle, $contents);
278
279	2	$this->tempFileHandles[] = [
280	2	'handle' => $fileHandle,
281		'path' => $tempFile
282	2	];
283
284	2	return $tempFile;
285		}
286
287		/**
288		* @return void
289		*/
290	1	protected function closeTempFileHandles()
291		{
292	1	foreach ($this->tempFileHandles as $key => $data) {
293	1	fclose($data['handle']);
294	1	unlink($data['path']);
295	1	unset($this->tempFileHandles[$key]);
296	1	}
297	1	}
298		}
299

kinglozzer / silverstripe-mailgunner

Branch — master (7a4c7d)

Mailer::buildMessage() F

Complexity

Size

Duplication

Code Coverage

Importance

How to fix Long Method Complexity Many Parameters

Long Method

Many Parameters

Duplication Side-by-Side

Filter issues like