From 0bc0a3b1a51863b170ee8785ca9c78701510312a Mon Sep 17 00:00:00 2001 From: z38 Date: Sat, 8 Apr 2017 01:24:37 +0200 Subject: [PATCH] Fix disordered POST parameters (#279) --- composer.json | 2 +- src/Mailgun/Api/Message.php | 12 +--- src/Mailgun/Connection/RestClient.php | 13 +---- tests/Functional/FileFromMemoryTest.php | 6 +- tests/Functional/InlineFileTest.php | 4 +- tests/Functional/MessageBuilderHeaderTest.php | 4 +- tests/Functional/NoSamePostNameTest.php | 55 ------------------- tests/Messages/ComplexMessageTest.php | 11 ++-- 8 files changed, 19 insertions(+), 88 deletions(-) delete mode 100644 tests/Functional/NoSamePostNameTest.php diff --git a/composer.json b/composer.json index cbcc9b1..bd5f3db 100644 --- a/composer.json +++ b/composer.json @@ -4,7 +4,7 @@ "require": { "php": "^5.5|^7.0", "php-http/httplug": "^1.0", - "php-http/multipart-stream-builder": "^0.1 || ^0.2", + "php-http/multipart-stream-builder": "^0.2", "php-http/message": "^1.0", "php-http/client-common": "^1.1", "php-http/discovery": "^1.0", diff --git a/src/Mailgun/Api/Message.php b/src/Mailgun/Api/Message.php index 3811276..bf12caa 100644 --- a/src/Mailgun/Api/Message.php +++ b/src/Mailgun/Api/Message.php @@ -39,10 +39,8 @@ class Message extends HttpApi if (!is_array($params[$fieldName])) { $postDataMultipart[] = $this->prepareFile($fieldName, $params[$fieldName]); } else { - $fileIndex = 0; foreach ($params[$fieldName] as $file) { - $postDataMultipart[] = $this->prepareFile($fieldName, $file, $fileIndex); - ++$fileIndex; + $postDataMultipart[] = $this->prepareFile($fieldName, $file); } } @@ -51,10 +49,9 @@ class Message extends HttpApi foreach ($params as $key => $value) { if (is_array($value)) { - $index = 0; foreach ($value as $subValue) { $postDataMultipart[] = [ - 'name' => sprintf('%s[%d]', $key, $index++), + 'name' => $key, 'content' => $subValue, ]; } @@ -98,16 +95,13 @@ class Message extends HttpApi * * @param string $fieldName * @param array $filePath array('fileContent' => 'content') or array('filePath' => '/foo/bar') - * @param int $fileIndex * * @return array * * @throws InvalidArgumentException */ - private function prepareFile($fieldName, array $filePath, $fileIndex = 0) + private function prepareFile($fieldName, array $filePath) { - // Add index for multiple file support - $fieldName .= '['.$fileIndex.']'; $filename = isset($filePath['filename']) ? $filePath['filename'] : null; if (isset($filePath['fileContent'])) { diff --git a/src/Mailgun/Connection/RestClient.php b/src/Mailgun/Connection/RestClient.php index 8000000..31c7b8b 100644 --- a/src/Mailgun/Connection/RestClient.php +++ b/src/Mailgun/Connection/RestClient.php @@ -150,10 +150,8 @@ class RestClient foreach ($fields as $fieldName) { if (isset($files[$fieldName])) { if (is_array($files[$fieldName])) { - $fileIndex = 0; foreach ($files[$fieldName] as $file) { - $postFiles[] = $this->prepareFile($fieldName, $file, $fileIndex); - ++$fileIndex; + $postFiles[] = $this->prepareFile($fieldName, $file); } } else { $postFiles[] = $this->prepareFile($fieldName, $files[$fieldName]); @@ -164,10 +162,9 @@ class RestClient $postDataMultipart = []; foreach ($postData as $key => $value) { if (is_array($value)) { - $index = 0; foreach ($value as $subValue) { $postDataMultipart[] = [ - 'name' => sprintf('%s[%d]', $key, $index++), + 'name' => $key, 'contents' => $subValue, ]; } @@ -285,11 +282,10 @@ class RestClient * * @param string $fieldName * @param string|array $filePath - * @param int $fileIndex * * @return array */ - protected function prepareFile($fieldName, $filePath, $fileIndex = 0) + protected function prepareFile($fieldName, $filePath) { $filename = null; @@ -314,9 +310,6 @@ class RestClient $resource = fopen($filePath, 'r'); } - // Add index for multiple file support - $fieldName .= '['.$fileIndex.']'; - return [ 'name' => $fieldName, 'contents' => $resource, diff --git a/tests/Functional/FileFromMemoryTest.php b/tests/Functional/FileFromMemoryTest.php index d055783..ca0d5ee 100644 --- a/tests/Functional/FileFromMemoryTest.php +++ b/tests/Functional/FileFromMemoryTest.php @@ -33,9 +33,9 @@ class FileFromMemoryTest extends \PHPUnit_Framework_TestCase ]; }, $attachments); - $this->assertContains(['name' => 'attachment[0]', 'contents' => 'File content', 'filename' => 'file1.txt'], $attachments); - $this->assertContains(['name' => 'attachment[1]', 'contents' => 'File content 2', 'filename' => 'file2.txt'], $attachments); - $this->assertContains(['name' => 'attachment[2]', 'contents' => 'Contents of a text file', 'filename' => 'text_file.txt'], $attachments); + $this->assertContains(['name' => 'attachment', 'contents' => 'File content', 'filename' => 'file1.txt'], $attachments); + $this->assertContains(['name' => 'attachment', 'contents' => 'File content 2', 'filename' => 'file2.txt'], $attachments); + $this->assertContains(['name' => 'attachment', 'contents' => 'Contents of a text file', 'filename' => 'text_file.txt'], $attachments); }; $attachments = [ diff --git a/tests/Functional/InlineFileTest.php b/tests/Functional/InlineFileTest.php index ffd5636..87e5820 100644 --- a/tests/Functional/InlineFileTest.php +++ b/tests/Functional/InlineFileTest.php @@ -18,8 +18,8 @@ class InlineFileTest extends \PHPUnit_Framework_TestCase { $fileValidator = function ($files) { $fileNames = [ - ['name' => 'inline[0]', 'filename' => 'foo.png'], - ['name' => 'inline[1]', 'filename' => 'bar.png'], + ['name' => 'inline', 'filename' => 'foo.png'], + ['name' => 'inline', 'filename' => 'bar.png'], ]; // Make sure that both files exists diff --git a/tests/Functional/MessageBuilderHeaderTest.php b/tests/Functional/MessageBuilderHeaderTest.php index 9d3fa33..9441b1f 100644 --- a/tests/Functional/MessageBuilderHeaderTest.php +++ b/tests/Functional/MessageBuilderHeaderTest.php @@ -18,8 +18,8 @@ class MessageBuilderHeaderTest extends \PHPUnit_Framework_TestCase { $messageValidator = function ($headers) { $this->assertContains(['name' => 'h:My-Singular-Header', 'contents' => '123'], $headers); - $this->assertContains(['name' => 'h:My-Plural-Header[0]', 'contents' => '123'], $headers); - $this->assertContains(['name' => 'h:My-Plural-Header[1]', 'contents' => '456'], $headers); + $this->assertContains(['name' => 'h:My-Plural-Header', 'contents' => '123'], $headers); + $this->assertContains(['name' => 'h:My-Plural-Header', 'contents' => '456'], $headers); }; // Create the mocked mailgun client. diff --git a/tests/Functional/NoSamePostNameTest.php b/tests/Functional/NoSamePostNameTest.php deleted file mode 100644 index dd116d1..0000000 --- a/tests/Functional/NoSamePostNameTest.php +++ /dev/null @@ -1,55 +0,0 @@ - - */ -class NoSamePostNameTest extends \PHPUnit_Framework_TestCase -{ - /** - * No post names should ever be the same. - */ - public function testNames() - { - $fileValidator = function ($files) { - $usedNames = []; - foreach ($files as $file) { - $this->assertFalse(in_array($file['name'], $usedNames), 'No files should have the same POST name.'); - $usedNames[] = $file['name']; - } - }; - - // Create the mocked mailgun client. We use $this->assertEquals on $method, $uri and $body parameters. - $mailgun = MockedMailgun::createMock($this, 'POST', 'domain/messages', [], $fileValidator); - - $builder = $mailgun->MessageBuilder(); - $builder->setFromAddress('bob@example.com'); - $builder->addToRecipient('to1@example.com'); - $builder->addToRecipient('to2@example.com'); - $builder->addCcRecipient('cc1@example.com'); - $builder->addCcRecipient('cc2@example.com'); - $builder->addBccRecipient('bcc1@example.com'); - $builder->addBccRecipient('bcc2@example.com'); - $builder->addCustomParameter('foo', 'bar'); - $builder->addCustomParameter('foo', 'baz'); - $builder->addCampaignId('campaign0'); - $builder->addCampaignId('campaign1'); - $builder->setSubject('Foo'); - $builder->setTextBody('Bar'); - - $builder->addAttachment('@./tests/TestAssets/mailgun_icon1.png', 'foo.png'); - $builder->addAttachment('@./tests/TestAssets/mailgun_icon1.png', 'foo.png'); - $builder->addInlineImage('@./tests/TestAssets/mailgun_icon2.png', 'bar.png'); - $builder->addInlineImage('@./tests/TestAssets/mailgun_icon2.png', 'bar.png'); - - $mailgun->post('domain/messages', $builder->getMessage(), $builder->getFiles()); - } -} diff --git a/tests/Messages/ComplexMessageTest.php b/tests/Messages/ComplexMessageTest.php index 764cafe..3fdedac 100644 --- a/tests/Messages/ComplexMessageTest.php +++ b/tests/Messages/ComplexMessageTest.php @@ -81,6 +81,7 @@ class ComplexMessageTest extends \Mailgun\Tests\MailgunTestCase // Start a counter, make sure all files are asserted $testCount = 0; + $expectedFilenames = ['mailgun_icon1.png', 'mailgun_icon2.png']; foreach ($result->files as $file) { if ($file['name'] == 'to') { $this->assertEquals($file['contents'], 'test@test.mailgun.org'); @@ -98,12 +99,10 @@ class ComplexMessageTest extends \Mailgun\Tests\MailgunTestCase $this->assertEquals($file['contents'], 'Testing!'); ++$testCount; } - if ($file['name'] == 'inline[0]') { - $this->assertEquals($file['filename'], 'mailgun_icon1.png'); - ++$testCount; - } - if ($file['name'] == 'inline[1]') { - $this->assertEquals($file['filename'], 'mailgun_icon2.png'); + if ($file['name'] == 'inline') { + $expectedFilename = array_shift($expectedFilenames); + $this->assertNotNull($expectedFilename); + $this->assertSame($expectedFilename, $file['filename']); ++$testCount; } }