Repository navigation
Bug: in-model pager ignores the model event (with where clause, run via callback), reporting incorrect paging information #8412
Description
PHP Version
8.1
CodeIgniter4 Version
4.4.4
CodeIgniter4 Installation Method
Composer (using codeigniter4/appstarter)
Which operating systems have you tested for this bug?
Linux
Which server did you use?
cli-server (PHP built-in webserver)
Database
SQLite3
What happened?
I have a custom event in a model that applies where clause to every search implemented thus:
protected $beforeFind = ['restrictToUser'];
protected function restrictToUser(array $data)
{
// in my case the user_id comes from session, hardcode here:
$this->where($this->table . '.user_id', 3);
return $data;
}When I use it with an in-model pager, the find results apply this event, but the pager does not, resulting in wrong page count and wrong total record number.
Steps to Reproduce
Start with empty project.
Rename env to .env, change these lines in it to the values:
app.baseURL = 'http://localhost:8080/'
database.default.database = ci4
database.default.DBDriver = SQLite3
Add a cli route in app/Config/Routes.php:
$routes->cli('home', 'Home::index');
Add migration for a table:
php spark make:migration CreateTestTable
Replace up() and down() methods with:
public function up()
{
$this->forge->addField([
'id' => [
'type' => 'INT',
'constraint' => 10,
'auto_increment' => true,
'null' => false,
],
'user_id' => [
'type' => 'INT',
'constraint' => 10,
'null' => false,
],
'data' => [
'type' => 'VARCHAR',
'constraint' => 255,
'null' => false,
],
'created_at' => [
'type' => 'DATETIME',
'null' => true,
],
'updated_at' => [
'type' => 'DATETIME',
'null' => true,
],
'deleted_at' => [
'type' => 'DATETIME',
'null' => true,
],
]);
$this->forge->addKey('id', true);
$this->forge->createTable('test');
}
public function down()
{
$this->forge->dropTable('test');
}make a seeder:
php spark make:seeder test --suffix
Replace the seeder method run() with this:
public function run()
{
$builder = $this->db->table('test');
$data = [];
$i = 1;
while ($i < 100) {
$user_id = rand(2, 4);
$data = [
'user_id' => $user_id,
'data' => 'Some text by user ' . $user_id . ', entry ' . $i,
'created_at' => '2024-01-09 12:51:33',
'updated_at' => '2024-01-09 12:51:33',
];
$builder->insert($data);
$i++;
}
}Create model:
php spark make:model TestModel
in model, update the $beforeFind callback thus:
protected $beforeFind = ['restrictToUser'];
and add event method to model:
protected function restrictToUser(array $data)
{
// in my case the user_id comes from session, hardcode here:
$this->where($this->table . '.user_id', 3);
return $data;
}Add test code to the Home controller, replace the index() method with:
public function index()
{
$model = model(\App\Models\TestModel::class);
echo PHP_EOL;
echo 'Total number of entries in table (should be 99): ' . $model->countAllResults(false);
echo PHP_EOL . PHP_EOL;
$list = $model->findAll();
//$list = $model->where('user_id', 3)->findAll();
echo 'Number with user_id = 3, using findAll(), event restrictToUser applied (should be ~1/3 of 99): ' . count($list);
echo PHP_EOL . PHP_EOL;
echo 'Rerun find with paginate() (squeze all results in one page):' ;
$list = $model->paginate(101);
$pager = $model->pager;
echo '... done.' . PHP_EOL . PHP_EOL;
echo 'Real number of results, that honours event restrictToUser (should be ~1/3 of 99): ' . count($list);
echo PHP_EOL . PHP_EOL;
echo 'pager-reported number, that ignores event restrictToUser (should be ~1/3 of 99, but is 99): ' . $pager->getTotal();
echo PHP_EOL . PHP_EOL;
echo 'Rerun find with paginate() (10 per page):' ;
$list = $model->paginate(10);
$pager = $model->pager;
echo '... done.' . PHP_EOL . PHP_EOL;
echo 'Pager-reported number, that ignores event restrictToUser (should be ~1/3 of 99, but is 99): ' . $pager->getTotal();
echo PHP_EOL . PHP_EOL;
echo 'Pager-reported page number, that ignores event restrictToUser (should be ~3, but is 10) : ' . $pager->getPageCount();
echo PHP_EOL . PHP_EOL;
}Run the test :
dg@tvenkinys:~/Programavimas/MOK/ci4bugDemo$ php spark migrate
CodeIgniter v4.4.4 Command Line Tool - Server Time: 2024-01-12 08:44:50 UTC+00:00
Running all new migrations...
Running: (App) 2024-01-10-214032_App\Database\Migrations\CreateTestTable
Migrations complete.
dg@tvenkinys:~/Programavimas/MOK/ci4bugDemo$ php spark db:seed TestSeeder
CodeIgniter v4.4.4 Command Line Tool - Server Time: 2024-01-12 08:45:12 UTC+00:00
Seeded: App\Database\Seeds\TestSeeder
dg@tvenkinys:~/Programavimas/MOK/ci4bugDemo$ php public/index.php home
Total number of entries in table (should be 99): 99
Number with user_id = 3, using findAll(), event restrictToUser applied (should be ~1/3 of 99): 28
Rerun find with paginate() (squeze all results in one page):... done.
Real number of results, that honours event restrictToUser (should be ~1/3 of 99): 28
pager-reported number, that ignores event restrictToUser (should be ~1/3 of 99, but is 99): 99
Rerun find with paginate() (10 per page):... done.
Pager-reported number, that ignores event restrictToUser (should be ~1/3 of 99, but is 99): 99
Pager-reported page number, that ignores event restrictToUser (should be ~3, but is 10) : 10Expected Output
I expect the in-model pager report the same number of entries as returned by the model paginate() method, applying the in-model specified event correctly.
Anything else?
No response
Activity
kenjis commentedon Jan 12, 2024
You reset the query with
countAllResults().Try
countAllResults(false).dgvirtual commentedon Jan 12, 2024
Tried; it changes nothing. Updated the test code above. And thanks for code colouring :)
kenjis commentedon Jan 12, 2024
Ah,
countAllResults()does not take care of events, so the result is always wrong.dgvirtual commentedon Jan 12, 2024
I assumed that, no problem here. It is the results at this line in output:
pager-reported number, that ignores event restrictToUser ...and later that are the problem. Pager counts the number of results that the model returns wrong.
dgvirtual commentedon Jan 12, 2024
My theory of this error after looking in the code of
BaseModel.php:The
$beforeFindcallbacks are fired in thefindAll()method, so, at the very last element of the chain of building sql command.The method
paginate()uses the thefindAll()method internally, but only calls it AFTER in initiates the Pager library and adds all the data to it. So, the Pager is initiated before the callback runs within thefindAll(), on line 1239:How does that sound?
kenjis commentedon Jan 12, 2024
It seems that we cannot solve this issue easily.
When we generate pagination, we need to:
countAllResults(false))findAll($limit, $offset)) (Note that we limit the result)Also:
3.
$beforeFindcallbacks are NOT fired incountAllResults().4.
findAll()resets the query.If we call
findAll($limit, $offset)beforecountAllResults(false),countAllResults(false)does not work.Even if we stop
findAll()to reset the query (there is no API to do so now), thecountAllResults(false)result will be limited and wrong.So the query order is not wrong in the current code:
CodeIgniter4/system/BaseModel.php
Lines 1228 to 1244 in 4308c0b
kenjis commentedon Jan 13, 2024
I don't know how to fix this issue or if we should not fix it.
Workaround is to override
paginate()something like this?dgvirtual commentedon Jan 13, 2024
I see; I was looking at the code yesterday and also could not envisage a way to improve it (but thought maybe the more experienced people might). The suggested workaround would work I guess while I only have that single callback; either that or just add the needed where clause manually before the call to pagination (as I do now; it does generate a double WHERE but it works).
I am leaving the issue open, but I am ok with it being closed as WONTFIX if an efficient solution cannot be found. Thanks for investigating, @kenjis!
michalsn commentedon Jan 13, 2024
I wouldn't say it's a bug. Documentation is clear, what methods are affected by the database events: https://codeigniter.com/user_guide/models/model.html#event-parameters
Although this may be inconvenient, just like in the presented case. To make it work, we would have to modify the
countAllResults()method, to also respect thebeforeFindevents. TBH, I'm not convinced that we should since these events are designed to work with different types of queries.kenjis commentedon Jan 14, 2024
According to the current documentation, it is not a bug. The behavior is expected.
My question is: should
countAllResults()respect Model*FindEvents?Because it is also a method to find. But I'm not sure we should or not.
lonnieezell commentedon Jan 14, 2024
The events only work on model-specific methods and countAllResults is from the query builder. I understand that this is a weird case but no I don't think so. The events were never intended to change the number of results, only modify the results that were found. Filtering should happen after results are returned.
kenjis commentedon Jan 14, 2024
@lonnieezell Model has
countAllResults():CodeIgniter4/system/Model.php
Line 611 in 4308c0b
Do you say the following Model callback is kind of a misuse?
lonnieezell commentedon Jan 14, 2024
Ah, I forgot that was overridden for soft-deletes. Seems I misspoke.
Would I say it's a misuse? No, just not what events were originally thought of. Everything always gets used in creative ways you didn't expect. Part of the fun - seeing how others use your stuff. :)
Thinking of it another way, though, no I wouldn't expect a
count()query to use the same event as afind()query, though.juanma386 commentedon Aug 25, 2024
This is Controller function:
Testing
juanma386 commentedon Aug 25, 2024
To solve this error of deprecate documentation oficial of with bug on:
Displayed at 00:34:00am — PHP: 8.3.4 Displayed at 00:34:00am — PHP: 8.3.4 — CodeIgniter: 4.5.4 -- Environment: development
Simple solve error with CI please use this:
use to query please use warning with Validator to restrict access or sql insection to $userId;
To testing in the dev enviroment is:
{ "status": 200, "error": null, "data": [ { "client_id": "50512010-624f-11ef-b9bf-7071bcbe887a" }, { "client_id": "5c1a2230-6254-11ef-b9bf-7071bcbe887a" }, { "client_id": "620bd04f-6258-11ef-b9bf-7071bcbe887a" }, { "client_id": "57f55e69-625c-11ef-b9bf-7071bcbe887a" } ], "pagination": { "total": "4", "per_page": 10, "current_page": 1, "last_page": 1, "next": null, "previous": null } }mavelo-llc commentedon Jun 27, 2025
Same issue?Figured using
beforeFindto apply conditions should work, but pager ignores them.Example Table:
Usershas 5 rows; only one withstatus = '1'Result: 1 row; 1 page link (correct)
Result: 5 rows; 3 page links (correct)
After moving
whereto the model'sonlyEnabledmethod...Result: 1 row; 3 page links (wrong)
Was hoping to prevent code duplication without manually calling a 'filter' method before every find|findAll|first call, but seems there is no way to make pager work with before/after events the way we hoped. Our models are shared between multiple modules, so we don't really want to rewrite pager.
michalsn commentedon Jun 28, 2025
@mavelo-llc Please check this PR: #9617
michalsn commentedon Jul 15, 2025
We've decided to document the current behavior, as "fixing" it could introduce confusion in other situations -
beforeFindevent is specifically intended to run only beforefind*methods.If this behavior is desired in your app, feel free to follow the implementation shared in #9617 or the dedicated article, which includes a handy trait.