Fix "When using QueryBuilder with Actions memory exhausts almost instantly"

[FDT2k]

Description

This fix is related to #12554 and #15720.

This fixes the issue #12554 using the tests from #15720 (slighlty modified, but not my strong skill, so worth a double check )

Visual changes

No visual changes

Functional changes

• Code style has been fixed by running the composer cs command.
• Changes have been tested to not break existing functionality.
• Documentation is up-to-date.

[danharrin]

Is this an infinite loop issue? If so, where does the infinite loop originate from? This does feel a little like a bandaid instead of addressing the root cause

[FDT2k]

The start of the infinite loop is in filament/tables/src/Table/Concerns/HasActions.php line 126

        $mountedRecord = $this->getLivewire()->getMountedTableActionRecord();

From what I understand:

• QueryBuilder tries to validate the form, which cache the form.
• Caching the form triggers getTraitForms, including the filters form
• Query builder tries to validate the form, which cache the form.
• Etc.

The loop look like this

#4899 /var/www/html/vendor/filament/tables/src/Filters/QueryBuilder.php(266): Filament\\Forms\\ComponentContainer->validate()
#4900 /var/www/html/vendor/filament/tables/src/Filters/QueryBuilder.php(153): Filament\\Tables\\Filters\\QueryBuilder->tapOperatorFromRule()
#4901 /var/www/html/vendor/filament/tables/src/Filters/QueryBuilder.php(46): Filament\\Tables\\Filters\\QueryBuilder->applyRulesToBaseQuery()
#4902 /var/www/html/vendor/filament/support/src/Concerns/EvaluatesClosures.php(35): Filament\\Tables\\Filters\\QueryBuilder->Filament\\Tables\\Filters\\{closure}()
#4903 /var/www/html/vendor/filament/tables/src/Filters/Concerns/InteractsWithTableQuery.php(57): Filament\\Support\\Components\\Component->evaluate()
#4904 /var/www/html/vendor/filament/tables/src/Concerns/HasFilters.php(145): Filament\\Tables\\Filters\\BaseFilter->applyToBaseQuery()
#4905 /var/www/html/vendor/filament/tables/src/Concerns/HasRecords.php(31): Filament\\Resources\\Pages\\ListRecords->applyFiltersToTableQuery()
#4906 /var/www/html/vendor/filament/tables/src/Concerns/HasRecords.php(26): Filament\\Resources\\Pages\\ListRecords->filterTableQuery()
#4907 /var/www/html/vendor/filament/tables/src/Concerns/HasRecords.php(121): Filament\\Resources\\Pages\\ListRecords->getFilteredTableQuery()
#4908 /var/www/html/vendor/filament/tables/src/Concerns/HasRecords.php(145): Filament\\Resources\\Pages\\ListRecords->resolveTableRecord()
#4909 /var/www/html/vendor/filament/tables/src/Concerns/HasActions.php(301): Filament\\Resources\\Pages\\ListRecords->getTableRecord()
#4910 /var/www/html/vendor/filament/tables/src/Table/Concerns/HasActions.php(126): Filament\\Resources\\Pages\\ListRecords->getMountedTableActionRecord()
#4911 /var/www/html/vendor/filament/tables/src/Concerns/HasActions.php(263): Filament\\Tables\\Table->getAction()
#4912 /var/www/html/vendor/filament/tables/src/Concerns/HasActions.php(268): Filament\\Resources\\Pages\\ListRecords->getMountedTableAction()
#4913 /var/www/html/vendor/filament/tables/src/Concerns/InteractsWithTable.php(264): Filament\\Resources\\Pages\\ListRecords->getMountedTableActionForm()
#4914 /var/www/html/vendor/filament/forms/src/Concerns/InteractsWithForms.php(377): Filament\\Resources\\Pages\\ListRecords->getInteractsWithTableForms()
#4915 /var/www/html/vendor/filament/forms/src/Concerns/InteractsWithForms.php(331): Filament\\Pages\\BasePage->getTraitForms()
#4916 /var/www/html/vendor/filament/forms/src/Concerns/InteractsWithForms.php(401): Filament\\Pages\\BasePage->cacheForms()
#4917 /var/www/html/vendor/filament/forms/src/Concerns/InteractsWithForms.php(256): Filament\\Pages\\BasePage->getCachedForms()
#4918 /var/www/html/vendor/livewire/livewire/src/Features/SupportValidation/HandlesValidation.php(237): Filament\\Pages\\BasePage->prepareForValidation()
#4919 /var/www/html/vendor/filament/forms/src/Concerns/InteractsWithForms.php(217): Livewire\\Component->validate()
#4920 /var/www/html/vendor/filament/forms/src/Concerns/CanBeValidated.php(121): Filament\\Pages\\BasePage->validate()
#4921 /var/www/html/vendor/filament/tables/src/Filters/QueryBuilder.php(266): Filament\\Forms\\ComponentContainer->validate()
#4922 /var/www/html/vendor/filament/tables/src/Filters/QueryBuilder.php(153): Filament\\Tables\\Filters\\QueryBuilder->tapOperatorFromRule()
#4923 /var/www/html/vendor/filament/tables/src/Filters/QueryBuilder.php(46): Filament\\Tables\\Filters\\QueryBuilder->applyRulesToBaseQuery()

There is probably a better way to fix this, but my understanding of the whole picture is limited.

It happens only when there is a mounted action in a table using QueryBuilder.

[FDT2k]

Just submitted a reproduction repository in the original issue #12554

[danharrin]

I do really appreciate the investigation here. I haven't looked as far into it as you have, but this fix does feel a little bit like a workaround to a flaw in the query builder. Maybe we should be working out a way to prevent the validation from running in certain places?

Also, I appreciate the tests, but there are changes I would make to them and I don't want them to prevent me from merging a bug fix. Would you mind removing the tests and testing helpers please and PRing them to v4 instead, so we have more time to do a proper review?

[FDT2k]

Yeah, I don't have your expertise of what happens in filament, but my first intuition was to fix it at the root, It was odd for me to not have a general condition to prevent recursive caching. I sure went the "easy way" on this fix 👿

On my second dive, I attempted to fix it in the query builder directly, but I wasn't sure how to keep it clean.
I'll have a deeper look into it when I have some time to see if I can fix this in the query builder directly. May be, like you said, just skipping the validation when an action is mounted on the table can do the trick.

I'll remove the tests from this PR asap, and let the original author @bzy107 push them on v4 .

[danharrin]

I'm having a look into this. I am currently thinking that we can refactor prepareForValidation() to avoid trying to recache schemas, not sure if it will work yet but I will keep this thread updated.

[danharrin]

#12554 (comment)