Description
Summary (*)
The EAV source model base class \Magento\Eav\Model\Entity\Attribute\Source\AbstractSource
contains a protected attribute $_options
that is meant to cache the options. Unfortunately this is not enforced, instead each implementation of getAllOptions()
is responsible to use that cache properly.
EAV source models often load possible values for attributes from external resources. Every access to those values, e.g. with $product->getAttributeText()
calls getAllOptions()
on the source model. So it makes sense that the result of this method should only be calculated once. As far as I see, all core source models already do this, using $_options
, but it is easy to miss in custom implementations, which can have significant performance impact.
Examples (*)
A typical "correct" implementation of getAllOptions()
:
public function getAllOptions()
{
if (empty($this->_options)) {
$materials = $this->materialRepository->getList($this->searchCriteriaBuilder->create())->getItems();
foreach ($materials as $material) {
$this->_options[] = ['value' => $material->getId(), 'label' => $material->getTitle()];
}
}
return $this->_options;
}
A typical problematic implementation:
public function getAllOptions()
{
$options = [];
$materials = $this->materialRepository->getList($this->searchCriteriaBuilder->create())->getItems();
foreach ($materials as $material) {
$options[] = ['value' => $material->getId(), 'label' => $material->getTitle()];
}
return $options;
}
Proposed solution
Move caching as default behavior into AbstractSource
:
public function getAllOptions()
{
if (empty($this->_options)) {
$this->_options = $this->loadOptions();
}
return $this->_options;
}
abstract protected function loadOptions() : array;
Implementations then override loadOptions()
instead of getAllOptions()
. The core source models should lead by example.
BC considerations
To maintain backwards compatibility, loadOptions()
can be optional instead:
protected function loadOptions() : array
{
return [];
}
This way existing implementations, that override getAllOptions()
still work.