0

I have a quick question and this could help me determine what is the good practice using the repositories.

To provide an example to the question I ask in the title of this subject, I can take as example the catalog rules and the sales rules. As the structure of these two models are similar, I don't get why these two entities do have a different way to work in their repositories.

  1. Catalog rules

The catalog rule repository implements the save function as :

save(\Magento\CatalogRule\Api\Data\RuleInterface)

This interface is mapped to the rule model :

<preference for="Magento\CatalogRule\Api\Data\RuleInterface" type="Magento\CatalogRule\Model\Rule" />

That means that if we want to save an object, we can quickly do :

    public function __construct(
        \Magento\CatalogRule\Model\RuleFactory $ruleFactory,
        \Magento\CatalogRule\Model\CatalogRuleRepository $catalogRuleRepository
    )
    {
        ...
        $this->ruleFactory = $ruleFactory;
        ...
        $this->catalogRuleRepository = $catalogRuleRepository;
        ...
    }

    ...

    public function example() 
    {
        $catalogRule = $this->ruleFactory->create(); 
        ....
        $this->catalogRuleRepository->save($catalogRule); 
        ...
    }

The rule is saved, everybody's happy.

  1. Sales rules

The sales rule repository implements the following :

save(\Magento\SalesRule\Api\Data\RuleInterface $rule)

The interface is mapped as :

<preference for="Magento\SalesRule\Api\Data\RuleInterface"
            type="Magento\SalesRule\Model\Data\Rule" />

So if we want to programatically create a rule, we can't do like the catalog rule previously but we have to do something like :

    public function __construct(
        \Magento\SalesRule\Model\RuleFactory $ruleFactory,
        \Magento\SalesRule\Model\Converter\ToDataModel $ruleToDataModelConverter,
        \Magento\SalesRule\Model\Converter\ToModel $ruleToModelConverter,
        \Magento\SalesRule\Model\RuleRepository $salesRuleRepository,
    ) {
        $this->ruleFactory = $ruleFactory;
        $this->ruleToDataModelConverter = $ruleToDataModelConverter;
        $this->ruleToModelConverter = $ruleToModelConverter;
        $this->salesRuleRepository = $salesRuleRepository;
    }

    ...

    public function example() 
    {
        ...
        $rule = $this->ruleFactory->create(); 
        ....
        $ruleDataModel = $this->ruleToDataModelConverter($rule); 
        $this->salesRuleRepository->save($ruleDataModel); 
        ...
    }

As the two core extensions are similar (and more than that, the models structure are almost identicals), I don't get why one of them uses a model while the other implements a data model during a repo save.

The same problem happens with other repositories at the save function level : customer repository uses data models while product repository use models.

I guess that one of the extension (probably catalog rules) has older code and it would need a refactoring work but imo I don't think it is a good practice to maintain two ways of working with repositories.

As a magento developer, I find it annoying to have to convert models to data models as it makes the code longer (not so much but still) particularly with some entities like the customer that are regularly used and for which we always have to inject the converters in the constructors.

What is the best practice for the repository usage ? Also, why, in the core modules, implementations of entity / model save is so different ?

1 Answer 1

0

In the Customer module, \Magento\Customer\Model\Data\Customer implements \Magento\Customer\Api\Data\CustomerInterface. But \Magento\Customer\Model\Customer doesn't implement the same interface.

In the Catalog Module, the \Magento\Catalog\Model\Product model itself implements the \Magento\Catalog\Api\Data\ProductInterface.

Both approaches are correct. But if we are to follow SOLID principles, then it is preferred to use object that implements EntityInterface and pass it to the repository instead of object that extends AbstractModel.

As per Liskov Substitution Principle (LSP), objects of a parent class should be able to be replaced with objects of a child class without causing issues in the application. Therefore, a child class should never change the characteristics of its parent class (such as parameters and return types).

In object oriented programming, when Class B extends Class A, Class B CAN BE a substitute for Class A. LSP extends the same fundamental principle, with a significant change – Class B SHOULD BE a substitute of Class A.

In the case of Magento, we must follow the same principle. If you intend to remove any method or modify its parameters, then it will result in the violation of this principle. The violation may cause the application to break or produce unexpected results.

Therefore, we must keep our repository tied only to the EntityInterface. We don't know if Magento team has a better data persistence approach in mind for the future. Until then, we can think of our EntityInterface implementation as a model for our data and the AbstractModel implementation as Magento's way of persisting data to the database.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.