Add Value to Magento Framework's public API

[navarr]

Description

These classes are necessary/useful for creating a backend source for a
Magento Configuration field. As such they should be reliable and part
of the public API.

Contribution checklist

• Pull request has a meaningful description of its purpose
• All commits are accompanied by meaningful commit messages
• All new or changed code is covered with unit/integration tests (if applicable)
• All automated tests passed successfully (all builds on Travis CI are green)

[magento-engcom-team]

Hi @navarr. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

• @magento-engcom-team give me test instance - deploy test instance based on PR changes
• @magento-engcom-team give me {$VERSION} instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Assistant documentation

[orlangur]

I don't think it's a good idea to make fat ugly model a part of API. Why isn't interface sufficient?

[navarr]

When writing a backend model for a config, Magento expects that the model implements methods that do not exist in the Interface (such as "addData"). Though, this can be solved by any implementation of ValueInterface extending from AbstractModel (as Value does).

However, this model contains all the default functionality expected from a configuration backend model - including setting the correct events to fire and invalidating the cache. I have hard trouble finding a backend model that does not extend from Value.

Tutorials and examples online also show that the typical config backend model utilizes this model. I would think for those reasons it should be part of the public @api so that custom module's backend models may depend on it not removing functionality without a major version break.

[navarr]

If you do not use the Value model, you must also specify the resource and resourcecollection for configuration yourself. Magento does it for Value as such:

    <type name="Magento\Framework\App\Config\Value">
        <arguments>
            <argument name="resource" xsi:type="object">Magento\Config\Model\ResourceModel\Config\Data</argument>
            <argument name="resourceCollection" xsi:type="object">Magento\Config\Model\ResourceModel\Config\Data\Collection\Proxy</argument>
        </arguments>
    </type>

It appears that in all cases the expectation is that you extend from this model

[orlangur]

@navarr do we have any other model extending AbstractModel marked as @api?

[navarr]

@orlangur Yes. For example \Magento\Customer\Model\Customer and Magento\Eav\Model\Entity\Type. Nothing in Magento\Framework, however.

[slavvka]

Currently we have the interface for this model marked with @api. There's no sense to mark the model itself as @api as well since we won't be needed the interface. That's why it is important to extend the interface while we can deliver that to 2.3.0

[slavvka]

@navarr If you you're still interested in delivering this and your other PR #17300 to 2.3.0 please answer me

[navarr]

@slavvka I've had little personal time to work on open source - so forgive me for the delay - though I had last spoken about this over two months ago.

I laid out a variety of good reasons that this model should be part of the API on August 1st, and that simply updating the interface is not good enough. My fallback position on this is that "developers currently rely on this model to perform the basic tasks associated with a backend value"

And similarly Magento expects that it is receiving a Value model for a vast majority of the information it conveys to such objects:

// Magento\Config\Model\PreparedValueFactory
            /** @var ValueInterface $backendModel */
            $backendModel = $this->valueFactory->create(
                $backendModelName,
                ['config' => $this->config]
            );

            if ($backendModel instanceof Value) {
                $scopeId = 0;

                $scope = $this->scopeTypeNormalizer->normalize($scope);

                if ($scope !== ScopeInterface::SCOPE_DEFAULT) {
                    $scopeId = $this->scopeResolverPool->get($scope)
                        ->getScope($scopeCode)
                        ->getId();
                }

                if ($field instanceof Structure\Element\Field) {
                    $groupPath = $field->getGroupPath();
                    $group = $structure->getElement($groupPath);
                    $backendModel->setField($field->getId());
                    $backendModel->setGroupId($group->getId());
                    $backendModel->setFieldConfig($field->getData());
                }

                $backendModel->setPath($configPath);
                $backendModel->setScope($scope);
                $backendModel->setScopeId($scopeId);
                $backendModel->setScopeCode($scopeCode);
                $backendModel->setValue($value);
            }

[paliarush]

Since current interface is not enough and we are in code-freeze stage now, I propose the following:
In 2.3.0

1. Mark the model as @api
2. Do NOT mark the interface as @api

In 2.4.0

1. Create a task to investigate how the interface should look like to cover the needs
2. Extend existing interface or create a new one based on the investigation. Mark it as @api
3. Deprecate the model

in 2.5.0

1. Remove @api from the model and leave only one interface marked as @api

[slavvka]

So, the decision is made. We are following to what @paliarush described. Please remove @api mark from the interface and add there a description what is going to be done with it. Also please add a description to the model that what it will be marked as @api temporarily until we introduce a new interface and make everybody use use it.

[slavvka]

@navarr thank you for your valuable contribution! Your changes was cherry-picked and delivered to 2.3.0 release branch. This PR is also accepted and will be delivered to 2.3.1 release

[magento-engcom-team]

Hi @slavvka, thank you for the review.
ENGCOM-3221 has been created to process this Pull Request

[magento-engcom-team]

Hi @navarr. Thank you for your contribution.
We will aim to release these changes as part of 2.3.1.
Please check the release notes for final confirmation.