MAGETWO-83329: 8003: Using System Value for Base Currency Results in Config Error. #11809

 - Merge Pull Request magento/magento2#11809 from nmalevanec/magento2:Github-8003
 - Merged commits:
   1. 38735ac4
   2. b63ceb99
   3. 3e0c7858

app/code/Magento/Directory/Model/Currency.php

@@ -6,6 +6,7 @@
 
 namespace Magento\Directory\Model;
 
+use Magento\Framework\App\ObjectManager;
 use Magento\Framework\Exception\InputException;
 use Magento\Directory\Model\Currency\Filter;
 
@@ -65,6 +66,11 @@ class Currency extends \Magento\Framework\Model\AbstractModel
      */
     protected $_localeCurrency;
 
+    /**
+     * @var CurrencyConfig
+     */
+    private $currencyConfig;
+
     /**
      * @param \Magento\Framework\Model\Context $context
      * @param \Magento\Framework\Registry $registry
@@ -76,6 +82,7 @@ class Currency extends \Magento\Framework\Model\AbstractModel
      * @param \Magento\Framework\Model\ResourceModel\AbstractResource $resource
      * @param \Magento\Framework\Data\Collection\AbstractDb $resourceCollection
      * @param array $data
+     * @param CurrencyConfig $currencyConfig
      * @SuppressWarnings(PHPMD.ExcessiveParameterList)
      */
     public function __construct(
@@ -88,7 +95,8 @@ class Currency extends \Magento\Framework\Model\AbstractModel
         \Magento\Framework\Locale\CurrencyInterface $localeCurrency,
         \Magento\Framework\Model\ResourceModel\AbstractResource $resource = null,
         \Magento\Framework\Data\Collection\AbstractDb $resourceCollection = null,
-        array $data = []
+        array $data = [],
+        CurrencyConfig $currencyConfig = null
     ) {
         parent::__construct(
             $context,
@@ -102,6 +110,7 @@ class Currency extends \Magento\Framework\Model\AbstractModel
         $this->_directoryHelper = $directoryHelper;
         $this->_currencyFilterFactory = $currencyFilterFactory;
         $this->_localeCurrency = $localeCurrency;
+        $this->currencyConfig = $currencyConfig ?: ObjectManager::getInstance()->get(CurrencyConfig::class);
     }
 
     /**
@@ -347,7 +356,7 @@ class Currency extends \Magento\Framework\Model\AbstractModel
      */
     public function getConfigAllowCurrencies()
     {
-        $allowedCurrencies = $this->_getResource()->getConfigCurrencies($this, self::XML_PATH_CURRENCY_ALLOW);
+        $allowedCurrencies = $this->currencyConfig->getConfigCurrencies(self::XML_PATH_CURRENCY_ALLOW);
         $appBaseCurrencyCode = $this->_directoryHelper->getBaseCurrencyCode();
         if (!in_array($appBaseCurrencyCode, $allowedCurrencies)) {
             $allowedCurrencies[] = $appBaseCurrencyCode;
@@ -369,8 +378,7 @@ class Currency extends \Magento\Framework\Model\AbstractModel
      */
     public function getConfigDefaultCurrencies()
     {
-        $defaultCurrencies = $this->_getResource()->getConfigCurrencies($this, self::XML_PATH_CURRENCY_DEFAULT);
-        return $defaultCurrencies;
+        return $this->currencyConfig->getConfigCurrencies(self::XML_PATH_CURRENCY_DEFAULT);
     }
 
     /**
@@ -378,8 +386,7 @@ class Currency extends \Magento\Framework\Model\AbstractModel
      */
     public function getConfigBaseCurrencies()
     {
-        $defaultCurrencies = $this->_getResource()->getConfigCurrencies($this, self::XML_PATH_CURRENCY_BASE);
-        return $defaultCurrencies;
+        return $this->currencyConfig->getConfigCurrencies(self::XML_PATH_CURRENCY_BASE);
     }
 
     /**

app/code/Magento/Directory/Model/CurrencyConfig.php

+<?php
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+
+namespace Magento\Directory\Model;
+
+use Magento\Framework\App\Area;
+use Magento\Framework\App\Config\ScopeConfigInterface;
+use Magento\Framework\App\State;
+use Magento\Store\Model\ScopeInterface;
+use Magento\Store\Model\StoreManagerInterface;
+
+/**
+ * Provide config values for allowed, base and default currencies.
+ */
+class CurrencyConfig
+{
+    /**
+     * @var State
+     */
+    private $appState;
+
+    /**
+     * @var ScopeConfigInterface
+     */
+    private $config;
+
+    /**
+     * @var StoreManagerInterface
+     */
+    private $storeManager;
+
+    /**
+     * CurrencyConfig constructor.
+     *
+     * @param State $appState
+     * @param ScopeConfigInterface $config
+     * @param StoreManagerInterface $storeManager
+     */
+    public function __construct(
+        State $appState,
+        ScopeConfigInterface $config,
+        StoreManagerInterface $storeManager
+    ) {
+        $this->appState = $appState;
+        $this->config = $config;
+        $this->storeManager = $storeManager;
+    }
+
+    /**
+     * Retrieve config currency data by config path.
+     *
+     * @param string $path
+     * @return array
+     */
+    public function getConfigCurrencies(string $path)
+    {
+        $result = $this->appState->getAreaCode() === Area::AREA_ADMINHTML
+            ? $this->getConfigForAllStores($path)
+            : $this->getConfigForCurrentStore($path);
+        sort($result);
+
+        return array_unique($result);
+    }
+
+    /**
+     * Get allowed, base and default currency codes for all stores.
+     *
+     * @param string $path
+     * @return array
+     */
+    private function getConfigForAllStores(string $path)
+    {
+        $storesResult = [[]];
+        foreach ($this->storeManager->getStores() as $store) {
+            $storesResult[] = explode(
+                ',',
+                $this->config->getValue($path, ScopeInterface::SCOPE_STORE, $store->getCode())
+            );
+        }
+
+        return array_merge(...$storesResult);
+    }
+
+    /**
+     * Get allowed, base and default currency codes for current store.
+     *
+     * @param string $path
+     * @return mixed
+     */
+    private function getConfigForCurrentStore(string $path)
+    {
+        $store = $this->storeManager->getStore();
+
+        return explode(',', $this->config->getValue($path, ScopeInterface::SCOPE_STORE, $store->getCode()));
+    }
+}

app/code/Magento/Directory/Model/ResourceModel/Currency.php

@@ -165,6 +165,8 @@ class Currency extends \Magento\Framework\Model\ResourceModel\Db\AbstractDb
      * @param string $path
      * @return array
      * @SuppressWarnings(PHPMD.UnusedFormalParameter)
+     * @deprecated because doesn't take into consideration scopes and system config values.
+     * @see \Magento\Directory\Model\CurrencyConfig::getConfigCurrencies()
      */
     public function getConfigCurrencies($model, $path)
     {

app/code/Magento/Directory/Test/Unit/Model/CurrencyConfigTest.php

+<?php
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+
+namespace Magento\Directory\Test\Unit\Model;
+
+use Magento\Config\App\Config\Type\System;
+use Magento\Directory\Model\CurrencyConfig;
+use Magento\Framework\App\Area;
+use Magento\Framework\App\Config\ScopeConfigInterface;
+use Magento\Framework\App\State;
+use Magento\Framework\TestFramework\Unit\Helper\ObjectManager;
+use Magento\Store\Api\Data\StoreInterface;
+use Magento\Store\Model\StoreManagerInterface;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * Provide tests for CurrencyConfig model.
+ */
+class CurrencyConfigTest extends TestCase
+{
+    /**
+     * @var CurrencyConfig
+     */
+    private $testSubject;
+
+    /**
+     * @var System|\PHPUnit_Framework_MockObject_MockObject
+     */
+    private $config;
+
+    /**
+     * @var StoreManagerInterface|\PHPUnit_Framework_MockObject_MockObject
+     */
+    private $storeManager;
+
+    /**
+     * @var State|\PHPUnit_Framework_MockObject_MockObject
+     */
+    private $appState;
+
+    /**
+     * @inheritdoc
+     */
+    protected function setUp()
+    {
+        $this->config = $this->getMockBuilder(ScopeConfigInterface::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+        $this->storeManager = $this->getMockBuilder(StoreManagerInterface::class)
+            ->setMethods(['getStores', 'getWebsites'])
+            ->disableOriginalConstructor()
+            ->getMockForAbstractClass();
+        $this->appState = $this->getMockBuilder(State::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+        $objectManager = new ObjectManager($this);
+        $this->testSubject = $objectManager->getObject(
+            CurrencyConfig::class,
+            [
+                'storeManager' => $this->storeManager,
+                'appState' => $this->appState,
+                'config' => $this->config,
+            ]
+        );
+    }
+
+    /**
+     * Test get currency config for admin and storefront areas.
+     *
+     * @dataProvider getConfigCurrenciesDataProvider
+     * @return void
+     */
+    public function testGetConfigCurrencies(string $areCode)
+    {
+        $path = 'test/path';
+        $expected = ['ARS', 'AUD', 'BZD'];
+
+        $this->appState->expects(self::once())
+            ->method('getAreaCode')
+            ->willReturn($areCode);
+
+        /** @var StoreInterface|\PHPUnit_Framework_MockObject_MockObject $store */
+        $store = $this->getMockBuilder(StoreInterface::class)
+            ->setMethods(['getCode'])
+            ->disableOriginalConstructor()
+            ->getMockForAbstractClass();
+        $store->expects(self::once())
+            ->method('getCode')
+            ->willReturn('testCode');
+
+        if ($areCode === Area::AREA_ADMINHTML) {
+            $this->storeManager->expects(self::once())
+                ->method('getStores')
+                ->willReturn([$store]);
+        } else {
+            $this->storeManager->expects(self::once())
+                ->method('getStore')
+                ->willReturn($store);
+        }
+
+        $this->config->expects(self::once())
+            ->method('getValue')
+            ->with(
+                self::identicalTo($path)
+            )->willReturn('ARS,AUD,BZD');
+
+        $result = $this->testSubject->getConfigCurrencies($path);
+
+        self::assertEquals($expected, $result);
+    }
+
+    /**
+     * Provide test data for getConfigCurrencies test.
+     *
+     * @return array
+     */
+    public function getConfigCurrenciesDataProvider()
+    {
+        return [
+            ['areaCode' => Area::AREA_ADMINHTML],
+            ['areaCode' => Area::AREA_FRONTEND],
+        ];
+    }
+}

dev/tests/integration/testsuite/Magento/Directory/Model/CurrencyConfigTest.php

+<?php
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+
+namespace Magento\Directory\Model;
+
+use Magento\Directory\Model\Currency as CurrencyModel;
+use Magento\Framework\App\Area;
+use Magento\Framework\App\Config\ConfigResource\ConfigInterface;
+use Magento\Framework\App\Config\ReinitableConfigInterface;
+use Magento\Store\Model\Store;
+use Magento\Store\Model\StoreManagerInterface;
+use Magento\TestFramework\App\State;
+use Magento\TestFramework\Helper\Bootstrap;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * Provide tests for CurrencyConfig model.
+ */
+class CurrencyConfigTest extends TestCase
+{
+    /**
+     * @var string
+     */
+    private $baseCurrencyPath = 'currency/options/base';
+
+    /**
+     * @var string
+     */
+    private $defaultCurrencyPath = 'currency/options/default';
+
+    /**
+     * @var string
+     */
+    private $allowedCurrenciesPath = 'currency/options/allow';
+
+    /**
+     * @var ConfigInterface
+     */
+    private $config;
+
+    /**
+     * @var CurrencyModel
+     */
+    private $currency;
+
+    /**
+     * @inheritdoc
+     */
+    protected function setUp()
+    {
+        $this->currency = Bootstrap::getObjectManager()->get(CurrencyModel::class);
+        $this->config = Bootstrap::getObjectManager()->get(ConfigInterface::class);
+    }
+
+    /**
+     * Test get currency config for admin and storefront areas.
+     *
+     * @dataProvider getConfigCurrenciesDataProvider
+     * @magentoDataFixture Magento/Store/_files/store.php
+     * @magentoDbIsolation disabled
+     * @param string $areaCode
+     * @param array $expected
+     * @return void
+     */
+    public function testGetConfigCurrencies(string $areaCode, array $expected)
+    {
+        /** @var State $appState */
+        $appState = Bootstrap::getObjectManager()->get(State::class);
+        $appState->setAreaCode($areaCode);
+        $store = Bootstrap::getObjectManager()->get(Store::class);
+        $store->load('test', 'code');
+        $this->clearCurrencyConfig();
+        $this->setStoreConfig($store->getId());
+        $storeManager = Bootstrap::getObjectManager()->get(StoreManagerInterface::class);
+        $storeManager->setCurrentStore($store->getId());
+
+        if ($areaCode === Area::AREA_ADMINHTML) {
+            self::assertEquals($expected['allowed'], $this->currency->getConfigAllowCurrencies());
+            self::assertEquals($expected['base'], $this->currency->getConfigBaseCurrencies());
+            self::assertEquals($expected['default'], $this->currency->getConfigDefaultCurrencies());
+        } else {
+            /** @var StoreManagerInterface $storeManager */
+            $storeManager = Bootstrap::getObjectManager()->get(StoreManagerInterface::class);
+            foreach ($storeManager->getStores() as $store) {
+                $storeManager->setCurrentStore($store->getId());
+                self::assertEquals(
+                    $expected[$store->getCode()]['allowed'],
+                    $this->currency->getConfigAllowCurrencies()
+                );
+                self::assertEquals(
+                    $expected[$store->getCode()]['base'],
+                    $this->currency->getConfigBaseCurrencies()
+                );
+                self::assertEquals(
+                    $expected[$store->getCode()]['default'],
+                    $this->currency->getConfigDefaultCurrencies()
+                );
+            }
+        }
+    }
+
+    /**
+     * Provide test data for getConfigCurrencies test.
+     *
+     * @return array
+     */
+    public function getConfigCurrenciesDataProvider()
+    {
+        return [
+            [
+                'areaCode' => Area::AREA_ADMINHTML,
+                'expected' => [
+                    'allowed' => ['BDT', 'BNS', 'BTD', 'EUR', 'USD'],
+                    'base' => ['BDT', 'USD'],
+                    'default' => ['BDT', 'USD'],
+                ],
+            ],
+            [
+                'areaCode' => Area::AREA_FRONTEND,
+                'expected' => [
+                    'default' => [
+                        'allowed' => ['EUR', 'USD'],
+                        'base' => ['USD'],
+                        'default' => ['USD'],
+                    ],
+                    'test' => [
+                        'allowed' => ['BDT', 'BNS', 'BTD', 'USD'],
+                        'base' => ['BDT'],
+                        'default' => ['BDT'],
+                    ],
+                ],
+            ],
+        ];
+    }
+
+    /**
+     * Remove currency config form Db.
+     *
+     * @return void
+     */
+    private function clearCurrencyConfig()
+    {
+        $storeManager = Bootstrap::getObjectManager()->get(StoreManagerInterface::class);
+        foreach ($storeManager->getStores() as $store) {
+            $this->config->deleteConfig(
+                $this->allowedCurrenciesPath,
+                'stores',
+                $store->getId()
+            );
+            $this->config->deleteConfig(
+                $this->baseCurrencyPath,
+                'stores',
+                $store->getId()
+            );
+            $this->config->deleteConfig(
+                $this->defaultCurrencyPath,
+                'stores',
+                $store->getId()
+            );
+        }
+    }
+
+    /**
+     * Set allowed, base and default currency config values for given store.
+     *
+     * @param string $storeId
+     * @return void
+     */
+    private function setStoreConfig(string $storeId)
+    {
+        $allowedCurrencies = 'BDT,BNS,BTD';
+        $baseCurrency = 'BDT';
+        $this->config->saveConfig(
+            $this->baseCurrencyPath,
+            $baseCurrency,
+            'stores',
+            $storeId
+        );
+        $this->config->saveConfig(
+            $this->defaultCurrencyPath,
+            $baseCurrency,
+            'stores',
+            $storeId
+        );
+        $this->config->saveConfig(
+            $this->allowedCurrenciesPath,
+            $allowedCurrencies,
+            'stores',
+            $storeId
+        );
+        Bootstrap::getObjectManager()->get(ReinitableConfigInterface::class)->reinit();
+        Bootstrap::getObjectManager()->create(StoreManagerInterface::class)->reinitStores();
+    }
+
+    protected function tearDown()
+    {
+        $this->clearCurrencyConfig();
+    }
+}