Skip to content

Commit

Permalink
Merge pull request #340 from saloonphp/fix/v3-allow-multiple-multipar…
Browse files Browse the repository at this point in the history 
…t-values-with-the-same-name

Fix | V3 - Allow multiple multipart values with the same name
  • Loading branch information
@Sammyjo20
Sammyjo20 authored Nov 30, 2023
2 parents 9a44948 + aab874e commit c14e0b0
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 28 deletions.
28 changes: 22 additions & 6 deletions src/Repositories/Body/MultipartBodyRepository.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

use InvalidArgumentException;
use Saloon\Data\MultipartValue;
use Saloon\Helpers\ArrayHelpers;
use Saloon\Traits\Conditionable;
use Saloon\Helpers\StringHelpers;
use Saloon\Exceptions\BodyException;
Expand Down Expand Up @@ -105,7 +104,7 @@ public function add(string $name, mixed $contents, string $filename = null, arra
*/
public function attach(MultipartValue $file): static
{
$this->data->add($file->name, $file);
$this->data->add(null, $file);

return $this;
}
Expand All @@ -124,10 +123,23 @@ public function all(): array
* Get a specific key of the array
*
* @param array-key $key
* @return MultipartValue|array<MultipartValue>
*/
public function get(string|int $key, mixed $default = null): MultipartValue
public function get(string|int $key, mixed $default = null): MultipartValue|array
{
return $this->data->get($key, $default);
$values = array_filter($this->all(), static function (MultipartValue $value) use ($key) {
return $value->name === $key;
});

if (count($values) === 0) {
return $default;
}

if (count($values) === 1) {
return $values[0];
}

return $values;
}

/**
Expand All @@ -137,7 +149,11 @@ public function get(string|int $key, mixed $default = null): MultipartValue
*/
public function remove(string $key): static
{
$this->data->remove($key);
$values = array_filter($this->all(), static function (MultipartValue $value) use ($key) {
return $value->name !== $key;
});

$this->set($values);

return $this;
}
Expand Down Expand Up @@ -172,7 +188,7 @@ protected function parseMultipartArray(array $value): array
throw new InvalidArgumentException(sprintf('The value array must only contain %s objects.', MultipartValue::class));
}

return ArrayHelpers::mapWithKeys($multipartValues, static fn (MultipartValue $value) => [$value->name => $value]);
return array_values($value);
}

/**
Expand Down
46 changes: 40 additions & 6 deletions tests/Feature/Body/HasMultipartBodyTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
$request = new HasMultipartBodyRequest();

expect($request->body()->all())->toEqual([
'nickname' => new MultipartValue('nickname', 'Sam', 'user.txt', ['X-Saloon' => 'Yee-haw!']),
new MultipartValue('nickname', 'Sam', 'user.txt', ['X-Saloon' => 'Yee-haw!']),
]);

$connector = new TestConnector;
Expand All @@ -35,12 +35,12 @@
$request = new HasMultipartBodyRequest;

expect($connector->body()->all())->toEqual([
'nickname' => new MultipartValue('nickname', 'Gareth', 'user.txt', ['X-Saloon' => 'Yee-haw!']),
'drink' => new MultipartValue('drink', 'Moonshine', 'moonshine.txt', ['X-My-Head' => 'Spinning!']),
new MultipartValue('nickname', 'Gareth', 'user.txt', ['X-Saloon' => 'Yee-haw!']),
new MultipartValue('drink', 'Moonshine', 'moonshine.txt', ['X-My-Head' => 'Spinning!']),
]);

expect($request->body()->all())->toEqual([
'nickname' => new MultipartValue('nickname', 'Sam', 'user.txt', ['X-Saloon' => 'Yee-haw!']),
new MultipartValue('nickname', 'Sam', 'user.txt', ['X-Saloon' => 'Yee-haw!']),
]);

// Nickname should be overwritten to "Sam" and "drink" should be merged in
Expand All @@ -51,8 +51,9 @@
expect($pendingRequestBody)->toBeInstanceOf(MultipartBodyRepository::class);

expect($pendingRequestBody->all())->toEqual([
'nickname' => new MultipartValue('nickname', 'Sam', 'user.txt', ['X-Saloon' => 'Yee-haw!']),
'drink' => new MultipartValue('drink', 'Moonshine', 'moonshine.txt', ['X-My-Head' => 'Spinning!']),
new MultipartValue('nickname', 'Gareth', 'user.txt', ['X-Saloon' => 'Yee-haw!']),
new MultipartValue('drink', 'Moonshine', 'moonshine.txt', ['X-My-Head' => 'Spinning!']),
new MultipartValue('nickname', 'Sam', 'user.txt', ['X-Saloon' => 'Yee-haw!']),
]);
});

Expand Down Expand Up @@ -119,3 +120,36 @@
expect($data)->toHaveKey('name', 'Howdy');
expect($data)->toHaveKey('file_contents', '');
});

test('can send multiple multipart files with the same key name', function () {
$connector = new TestConnector;
$request = new HasMultipartBodyRequest;

$request->body()->add('nickname', 'Alfie', 'user.txt');
$request->body()->add('nickname', 'Tom', 'user.txt');

$asserted = false;

$connector->sender()->addMiddleware(function (callable $handler) use ($request, &$asserted) {
return function (RequestInterface $guzzleRequest, array $options) use ($request, &$asserted) {
expect($guzzleRequest->getBody()->getContents())->toContain(
'X-Saloon: Yee-haw!',
'Content-Disposition: form-data; name="nickname"; filename="user.txt"',
'Content-Length: 3',
'Sam',
'Alfie',
'Tom',
);

$asserted = true;

$factory = new HttpFactory;

return new FulfilledPromise(MockResponse::make()->createPsrResponse($factory, $factory));
};
});

$connector->send($request);

expect($asserted)->toBeTrue();
});
46 changes: 30 additions & 16 deletions tests/Unit/Body/MultipartBodyRepositoryTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@
]);

expect($body->all())->toEqual([
'name' => new MultipartValue('name', 'Sam'),
'sidekick' => new MultipartValue('sidekick', 'Mantas'),
new MultipartValue('name', 'Sam'),
new MultipartValue('sidekick', 'Mantas'),
]);
});

Expand Down Expand Up @@ -50,39 +50,40 @@
]);

expect($body->all())->toEqual([
'username' => new MultipartValue('username', 'Sammyjo20'),
new MultipartValue('username', 'Sammyjo20'),
]);
});

test('you can add an item', function () {
$body = new MultipartBodyRepository();
test('you can add multiple items', function () {
$body = new MultipartBodyRepository;

$body->add('name', 'Sam', 'welcome.txt', ['a' => 'b']);

expect($body->all())->toEqual([
'name' => new MultipartValue('name', 'Sam', 'welcome.txt', ['a' => 'b']),
new MultipartValue('name', 'Sam', 'welcome.txt', ['a' => 'b']),
]);

// Test it being overwritten
// Test it gets added to the array

$body->add('name', 'Charlotte', 'welcome.txt', ['a' => 'b']);

expect($body->all())->toEqual([
'name' => new MultipartValue('name', 'Charlotte', 'welcome.txt', ['a' => 'b']),
new MultipartValue('name', 'Sam', 'welcome.txt', ['a' => 'b']),
new MultipartValue('name', 'Charlotte', 'welcome.txt', ['a' => 'b']),
]);
});

test('you can conditionally add items to the array store', function () {
$body = new MultipartBodyRepository();
$body = new MultipartBodyRepository;

$body->when(true, fn (MultipartBodyRepository $body) => $body->add('name', 'Gareth'));
$body->when(false, fn (MultipartBodyRepository $body) => $body->add('name', 'Sam'));
$body->when(true, fn (MultipartBodyRepository $body) => $body->add('sidekick', 'Mantas'));
$body->when(false, fn (MultipartBodyRepository $body) => $body->add('sidekick', 'Teo'));

expect($body->all())->toEqual([
'name' => new MultipartValue('name', 'Gareth'),
'sidekick' => new MultipartValue('sidekick', 'Mantas'),
new MultipartValue('name', 'Gareth'),
new MultipartValue('sidekick', 'Mantas'),
]);
});

Expand All @@ -103,15 +104,27 @@
expect($body->get('name'))->toEqual(new MultipartValue('name', 'Sam'));
});

test('you can get multiple items with the same name', function () {
$body = new MultipartBodyRepository();

$body->add('name', 'Sam');
$body->add('name', 'Alex');

expect($body->get('name'))->toEqual([
new MultipartValue('name', 'Sam'),
new MultipartValue('name', 'Alex'),
]);
});

test('you can get all items', function () {
$body = new MultipartBodyRepository();

$body->add('name', 'Sam');
$body->add('superhero', 'Iron Man');

$allResults = [
'name' => new MultipartValue('name', 'Sam'),
'superhero' => new MultipartValue('superhero', 'Iron Man'),
new MultipartValue('name', 'Sam'),
new MultipartValue('superhero', 'Iron Man'),
];

expect($body->all())->toEqual($allResults);
Expand All @@ -129,9 +142,10 @@
$body->merge([new MultipartValue('sidekick', 'Gareth')], [new MultipartValue('superhero', 'Black Widow')]);

expect($body->all())->toEqual([
'name' => new MultipartValue('name', 'Sam'),
'sidekick' => new MultipartValue('sidekick', 'Gareth'),
'superhero' => new MultipartValue('superhero', 'Black Widow'),
new MultipartValue('name', 'Sam'),
new MultipartValue('sidekick', 'Mantas'),
new MultipartValue('sidekick', 'Gareth'),
new MultipartValue('superhero', 'Black Widow'),
]);
});

Expand Down

0 comments on commit c14e0b0

Please sign in to comment.