codeigniter4 / CodeIgniter4

Open Source PHP Framework (originally from EllisLab)
https://codeigniter.com/
MIT License
5.35k stars 1.9k forks source link

Bug: Calling validateData Method more than once #9225

Closed zcaotica closed 1 week ago

zcaotica commented 1 week ago

PHP Version

8.2

CodeIgniter4 Version

4.5.5

CodeIgniter4 Installation Method

Composer (using codeigniter4/appstarter)

Which operating systems have you tested for this bug?

Windows

Which server did you use?

cli

Database

Invariant

What happened?

Hello,

I found an issue with the validateData method. When calling it multiple times, the second run seems to inherit the errors from the first run, causing the validation to fail unexpectedly.

Here is an example to demonstrate the issue:


$rule = ['name' => 'required|max_length[255]'];

// First validation - this should fail (as expected)
$item = ['name1' => 'invalid']; // Incorrect field name
$body = json_encode($item);  // Simulating JSON request body
$json = json_decode($body, true);

if (!$this->validateData($json, $rule)) {
    // Validation fails correctly
}

// Second validation - this should pass but fails
$item = ['name' => "Uno_T1"]; // Valid input
$body = json_encode($item);  // Simulating JSON request body
$json = json_decode($body, true);

if (!$this->validateData($json, $rule)) {
    // Unexpected failure: validation fails even though the input is correct
}

Issue Details: After checking the source code, I noticed that the errors variable is initialized only when the class is created and not reset with each validation attempt.

While this could be intentional—possibly useful when checking multiple rules in a Controller, but it could causes issues in a test class that have more than one test method. Specifically:

I have three test methods:

"testRightBody" (expects success) "testWrongBody1" (expects failure) "testWrongBody2" (expects failure) The third test always fails, even when the input is correct accidentally.

If I change the order of the tests, the one with valid input also fails.

This behavior suggests that validation errors are being carried over between runs, which may not be the expected behavior in all cases.

Steps to Reproduce

Create a test class with two test method (first with wrong input).

Expected Output

That the to second method in the test class, with right input, do not fail.

Anything else?

No response

neznaika0 commented 1 week ago

No, see manual. Add reset() after first validation data https://codeigniter4.github.io/userguide/libraries/validation.html#running-multiple-validations

zcaotica commented 1 week ago

Ok, sorry, I didn’t read that paragraph. But I can’t use it in a test class, right? So, do I always have to put it in my Controller?

neznaika0 commented 1 week ago

What is the problem? Clear the validator and set the rules again.

zcaotica commented 1 week ago

I create a new empty project with: composer create-project codeigniter4/appstarter val-test

I create a controller: \val-test\app\Controllers\ValidationTestController.php

namespace App\Controllers;

use CodeIgniter\Controller;
use CodeIgniter\API\ResponseTrait;

class ValidationTestController extends BaseController
{
    use ResponseTrait;

    public function validationTest() {
        $rule = ['name' => 'required|max_length[255]'];
        $request = $this->request;
        $body = $request->getBody();
        $json = json_decode($body, true);
        if (!$this->validateData($json, $rule)) {
            $errors = $this->validator->getErrors();
            return $this->failValidationErrors($errors);
        } 
        return $this->respond("Success"); 
    }
}

I create a test file: \val-test\tests\app\Controllers\ValidationTest.php

namespace App\Controllers;

use CodeIgniter\Test\CIUnitTestCase;
use CodeIgniter\Test\ControllerTestTrait;

class ValidationTest extends CIUnitTestCase
{
    use ControllerTestTrait;

    public function testWrongBody() { 
        $item1 = ['name1' => 'wrong1'];
        $body1 = json_encode($item1);
        $result = $this
            ->withBody($body1)
            ->controller(ValidationTestController::class)
            ->execute('validationTest');
        $result->assertStatus(400);
    }

    public function testRightBody() { 
        $item1 = ['name' => 'right'];
        $body1 = json_encode($item1);
        $result = $this
            ->withBody($body1)
            ->controller(ValidationTestController::class)
            ->execute('validationTest');
        $result->assertStatus(200);
    }
    public function testWrongBody2() { 
        #I write an incorrect test case, so i must fail
        $item1 = ['name' => 'right']; 
        $body1 = json_encode($item1);
        $result = $this
            ->withBody($body1)
            ->controller(ValidationTestController::class)
            ->execute('validationTest');
        $result->assertStatus(400);
    }

}

I run the test with this command: vendor\bin\phpunit --filter test tests\app\Controllers\ValidationTest.php

An this is the output Runtime: PHP 8.3.3 Configuration: \val-test\phpunit.xml.dist

.F. 3 / 3 (100%)

Time: 00:00.218, Memory: 12.00 MB

There was 1 PHPUnit test runner warning:

1) No code coverage driver available

--

There was 1 failure:

1) App\Controllers\ValidationTest::testRightBody Failed asserting that 400 is identical to 200.

C:\Dev\cassa\val-test\vendor\codeigniter4\framework\system\Test\TestResponse.php:146 C:\Dev\cassa\val-test\tests\app\Controllers\ValidationTest.php:29

FAILURES! Tests: 3, Assertions: 3, Failures: 1, Warnings: 1.

The testWrongBody work fine The testRightBody fails, but it shouldn't (if I run "--filter testRightBody" work fine) The testWrongBody2 work fine , but it have to fail.

Is this the expected behavior? I use only one validation in the Controller method, so I don’t expect to need reset(). Do I have to use it?

Thank you Zeta

zcaotica commented 1 week ago

Same result with

    public static $expectedList = array(
            ['key1' => 'name1', 'value1' => 'wrong', 'status1' => 400],
            ['key1' => 'name', 'value1' => 'right', 'status1' => 200],
            ['key1' => 'name', 'value1' => 'wrong', 'status1' => 400],
        );

    public static function dynamicDataProvider(): array
    {
        return self::$expectedList;
    }       
    #[DataProvider('dynamicDataProvider')]
    public function testDynamicBody(String $key1, String $value1, int $status1) { 
        #I write an incorrect test case, so i must fail
        $item1 = [$key1 => $value1]; 
        $body1 = json_encode($item1);
        $result = $this
            ->withBody($body1)
            ->controller(ValidationTestController::class)
            ->execute('validationTest');
        $result->assertStatus($status1);
    }
neznaika0 commented 1 week ago

Add in test class

public function tearDown(): void {
    service('validation')->reset();
}

and in testWrongBody2() assertion $result->assertStatus(200);

zcaotica commented 1 week ago

This was nice, thank you