Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix file uploader, test typed attributes #20167

Merged
merged 5 commits into from
May 30, 2024
Merged

Fix file uploader, test typed attributes #20167

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8deadac
Fix file uploader, test typed attributes
bizley May 26, 2024
1f618b6
Fix file uploader
bizley May 26, 2024
19f65fb
Merge branch 'master' into 19855-file-validator-updates
samdark May 28, 2024
06fa521
CHANGELOG
bizley May 30, 2024
7c5b213
Merge branch 'refs/heads/master' into 19855-file-validator-updates
bizley May 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 14 additions & 31 deletions framework/validators/FileValidator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,40 +208,23 @@
*/
public function validateAttribute($model, $attribute)
{
if ($this->maxFiles != 1 || $this->minFiles > 1) {
$rawFiles = $model->$attribute;
if (!is_array($rawFiles)) {
$this->addError($model, $attribute, $this->uploadRequired);
$files = $this->filterFiles(is_array($model->$attribute) ? $model->$attribute : [$model->$attribute]);
$filesCount = count($files);
if ($filesCount === 0 && $this->minFiles > 0) {
$this->addError($model, $attribute, $this->uploadRequired);

Check warning on line 214 in framework/validators/FileValidator.php

View check run for this annotation

Codecov / codecov/patch

framework/validators/FileValidator.php#L211-L214 

Added lines #L211 - L214 were not covered by tests

return;
}

$files = $this->filterFiles($rawFiles);
$model->$attribute = $files;

if (empty($files)) {
$this->addError($model, $attribute, $this->uploadRequired);

return;
}

$filesCount = count($files);
if ($this->maxFiles && $filesCount > $this->maxFiles) {
$this->addError($model, $attribute, $this->tooMany, ['limit' => $this->maxFiles]);
}
return;

Check warning on line 216 in framework/validators/FileValidator.php

View check run for this annotation

Codecov / codecov/patch

framework/validators/FileValidator.php#L216 

Added line #L216 was not covered by tests
}

if ($this->minFiles && $this->minFiles > $filesCount) {
$this->addError($model, $attribute, $this->tooFew, ['limit' => $this->minFiles]);
}
if ($this->maxFiles > 0 && $filesCount > $this->maxFiles) {
$this->addError($model, $attribute, $this->tooMany, ['limit' => $this->maxFiles]);

Check warning on line 220 in framework/validators/FileValidator.php

View check run for this annotation

Codecov / codecov/patch

framework/validators/FileValidator.php#L219-L220 

Added lines #L219 - L220 were not covered by tests
}
if ($this->minFiles > 0 && $this->minFiles > $filesCount) {
$this->addError($model, $attribute, $this->tooFew, ['limit' => $this->minFiles]);

Check warning on line 223 in framework/validators/FileValidator.php

View check run for this annotation

Codecov / codecov/patch

framework/validators/FileValidator.php#L222-L223 

Added lines #L222 - L223 were not covered by tests
}

foreach ($files as $file) {
$result = $this->validateValue($file);
if (!empty($result)) {
$this->addError($model, $attribute, $result[0], $result[1]);
}
}
} else {
$result = $this->validateValue($model->$attribute);
foreach ($files as $file) {
$result = $this->validateValue($file);

Check warning on line 227 in framework/validators/FileValidator.php

View check run for this annotation

Codecov / codecov/patch

framework/validators/FileValidator.php#L226-L227 

Added lines #L226 - L227 were not covered by tests
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bizley validateValue() is now not called anymore when submitting an empty form, I created an issue with further details: #20231

if (!empty($result)) {
$this->addError($model, $attribute, $result[0], $result[1]);
}
Expand Down
18 changes: 18 additions & 0 deletions tests/data/validators/models/FakedValidationTypedModel.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<?php
/**
* @link https://www.yiiframework.com/
* @copyright Copyright (c) 2008 Yii Software LLC
* @license https://www.yiiframework.com/license/
*/

namespace yiiunit\data\validators\models;

use yii\base\Model;
use yii\web\UploadedFile;

class FakedValidationTypedModel extends Model
{
public ?UploadedFile $single = null;

public array $multiple = [];
}
167 changes: 160 additions & 7 deletions tests/framework/validators/FileValidatorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use yii\validators\FileValidator;
use yii\web\UploadedFile;
use yiiunit\data\validators\models\FakedValidationModel;
use yiiunit\data\validators\models\FakedValidationTypedModel;
use yiiunit\TestCase;

/**
Expand Down Expand Up @@ -114,11 +115,11 @@ public function testValidateAttributeMultiple()
]);
$m = FakedValidationModel::createWithAttributes(['attr_files' => 'path']);
$val->validateAttribute($m, 'attr_files');
$this->assertTrue($m->hasErrors('attr_files'));
$this->assertFalse($m->hasErrors('attr_files'));
$m = FakedValidationModel::createWithAttributes(['attr_files' => []]);
$val->validateAttribute($m, 'attr_files');
$this->assertTrue($m->hasErrors('attr_files'));
$this->assertSame($val->uploadRequired, current($m->getErrors('attr_files')));
$this->assertFalse($m->hasErrors('attr_files'));

$m = FakedValidationModel::createWithAttributes(
[
'attr_files' => $this->createTestFiles(
Expand Down Expand Up @@ -314,6 +315,32 @@ public function testValidateAttribute_minFilesTwoMaxFilesUnlimited_hasError()
$this->assertTrue($model->hasErrors('attr_images'));
}

/**
* https://github.com/yiisoft/yii2/issues/19855
*/
public function testValidateArrayAttributeWithMinMaxOneAndOneFile()
{
$validator = new FileValidator(['maxFiles' => 1, 'minFiles' => 0]);
$files = $this->createTestFiles(
[
[
'name' => 'image.png',
'size' => 1024,
'type' => 'image/png',
],
[
'name' => 'image.png',
'size' => 1024,
'type' => 'image/png',
],
]
)[0];
$model = FakedValidationModel::createWithAttributes(['attr_images' => [$files]]);

$validator->validateAttribute($model, 'attr_images');
$this->assertFalse($model->hasErrors('attr_images'));
samdark marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* @param array $params
* @return UploadedFile[]
Expand Down Expand Up @@ -395,17 +422,15 @@ public function testValidateAttribute()
$val->validateAttribute($m, 'attr_files');
$this->assertFalse($m->hasErrors());
$val->validateAttribute($m, 'attr_files_empty');
$this->assertTrue($m->hasErrors('attr_files_empty'));
$this->assertSame($val->uploadRequired, current($m->getErrors('attr_files_empty')));
$this->assertFalse($m->hasErrors('attr_files_empty'));

// single File with skipOnEmpty = false
$val = new FileValidator(['skipOnEmpty' => false]);
$m = $this->createModelForAttributeTest();
$val->validateAttribute($m, 'attr_files');
$this->assertFalse($m->hasErrors());
$val->validateAttribute($m, 'attr_files_empty');
$this->assertTrue($m->hasErrors('attr_files_empty'));
$this->assertSame($val->uploadRequired, current($m->getErrors('attr_files_empty')));
$this->assertFalse($m->hasErrors('attr_files_empty'));
$m = $this->createModelForAttributeTest();

// too big
Expand Down Expand Up @@ -672,4 +697,132 @@ public function mimeTypeCaseInsensitive() {
['image/jxra', 'image/jxrA', true],
];
}

public function testValidateTypedAttributeNoErrors()
{
if (version_compare(PHP_VERSION, '7.4', '<')) {
$this->markTestSkipped('Requires typed properties');
}

$validator = new FileValidator(['minFiles' => 0, 'maxFiles' => 2]);
$file = $this->createTestFiles(
[
[
'name' => 'image.png',
'size' => 1024,
'type' => 'image/png',
]
]
);
$model = new FakedValidationTypedModel();
$model->single = $file;
$model->multiple = [$file];
$validator->validateAttribute($model, 'single');
$this->assertFalse($model->hasErrors('single'));
$validator->validateAttribute($model, 'multiple');
$this->assertFalse($model->hasErrors('multiple'));
}

public function testValidateTypedAttributeExactMinNoErrors()
{
if (version_compare(PHP_VERSION, '7.4', '<')) {
$this->markTestSkipped('Requires typed properties');
}

$validator = new FileValidator(['minFiles' => 1]);
$file = $this->createTestFiles(
[
[
'name' => 'image.png',
'size' => 1024,
'type' => 'image/png',
]
]
);
$model = new FakedValidationTypedModel();
$model->single = $file;
$model->multiple = [$file];
$validator->validateAttribute($model, 'single');
$this->assertFalse($model->hasErrors('single'));
$validator->validateAttribute($model, 'multiple');
$this->assertFalse($model->hasErrors('multiple'));
}

public function testValidateTypedAttributeExactMaxNoErrors()
{
if (version_compare(PHP_VERSION, '7.4', '<')) {
$this->markTestSkipped('Requires typed properties');
}

$validator = new FileValidator(['maxFiles' => 1]);
$file = $this->createTestFiles(
[
[
'name' => 'image.png',
'size' => 1024,
'type' => 'image/png',
]
]
);
$model = new FakedValidationTypedModel();
$model->single = $file;
$model->multiple = [$file];
$validator->validateAttribute($model, 'single');
$this->assertFalse($model->hasErrors('single'));
$validator->validateAttribute($model, 'multiple');
$this->assertFalse($model->hasErrors('multiple'));
}

public function testValidateTypedAttributeMinError()
{
if (version_compare(PHP_VERSION, '7.4', '<')) {
$this->markTestSkipped('Requires typed properties');
}

$validator = new FileValidator(['minFiles' => 2]);
$file = $this->createTestFiles(
[
[
'name' => 'image.png',
'size' => 1024,
'type' => 'image/png',
]
]
);
$model = new FakedValidationTypedModel();
$model->single = $file;
$model->multiple = [$file];
$validator->validateAttribute($model, 'single');
$this->assertTrue($model->hasErrors('single'));
$validator->validateAttribute($model, 'multiple');
$this->assertTrue($model->hasErrors('multiple'));
}

public function testValidateTypedAttributeMaxError()
{
if (version_compare(PHP_VERSION, '7.4', '<')) {
$this->markTestSkipped('Requires typed properties');
}

$validator = new FileValidator(['maxFiles' => 1]);
$files = $this->createTestFiles(
[
[
'name' => 'image.png',
'size' => 1024,
'type' => 'image/png',
],
[
'name' => 'image.png',
'size' => 1024,
'type' => 'image/png',
]
]
);
$model = new FakedValidationTypedModel();
// single attribute cannot be checked because maxFiles = 0 === no limits
$model->multiple = $files;
$validator->validateAttribute($model, 'multiple');
$this->assertTrue($model->hasErrors('multiple'));
}
}
Loading