Introduce resource model pool to replace UniversalFactory #13302
Conversation
| * @param string $resourceClassName | ||
| * @return AbstractResource | ||
| */ | ||
| public function get($resourceClassName): AbstractResource; |
There was a problem hiding this comment.
Please add string type hinting, as 2.3 supports PHP 7.0+
|
@buskamuza right, I updated the PR! |
|
@schmengler sorry for slow response, can you please update your branch and resolve the conflicts |
…llection This prevents multiple instantiation of resource models
|
Will do |
|
@schmengler see my messages in slack as well, there is a rebased branch with this pull request that you can use https://github.com/sivaschenko/magento2/tree/resource-model-pool-rebased (but I didn't want to force push to your branch) |
|
@sivaschenko So I did it now :) |
|
Hi @schmengler running |
…el-pool # Conflicts: # app/code/Magento/Catalog/Test/Unit/Model/ResourceModel/Product/CollectionTest.php
|
Hi @schmengler, thank you for your contribution! |
| * See COPYING.txt for license details. | ||
| */ | ||
|
|
||
| namespace Magento\Framework\Model\ResourceModel; |
There was a problem hiding this comment.
Shouldn't we have strict_types=1 there?
Description
I was digging a bit for unnecessary object instantiations and one thing I found was the good old
_initmethod in collections where the class name of the resource model is passed. resource models are usually shared instances, but collections misuse a "Magento\Framework\Validator\UniversalFactory" (which has a TODO comment that it should be eliminated) to create a new instance every time.As discussed with @antonkril I introduced a
ResourceModelPoolas replacement.For BC, the universalFactory argument stays in place, and the resourceModelPool argument has been added at the end with
nulldefault. The protected _universalFactory property is not used by the core anymore and is now marked as@deprecatedContribution checklist