Currency import Undefined index error #21487:

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

@@ -7,30 +7,30 @@
 
 namespace Magento\Directory\Model\Currency\Import;
 
+use Magento\Store\Model\ScopeInterface;
+
 class CurrencyConverterApi extends AbstractImport
 {
     /**
      * @var string
      */
-    const CURRENCY_CONVERTER_URL = 'http://free.currencyconverterapi.com/api/v3/convert?q={{CURRENCY_FROM}}_{{CURRENCY_TO}}&compact=ultra'; //@codingStandardsIgnoreLine
+    const CURRENCY_CONVERTER_URL = 'https://free.currencyconverterapi.com/api/v6/convert?q={{CURRENCY_FROM}}_{{CURRENCY_TO}}'
+    . '&compact=ultra&apiKey={{ACCESS_KEY}}'; //@codingStandardsIgnoreLine
 
     /**
      * Http Client Factory
-     *
      * @var \Magento\Framework\HTTP\ZendClientFactory
      */
     private $httpClientFactory;
 
     /**
      * Core scope config
-     *
      * @var \Magento\Framework\App\Config\ScopeConfigInterface
      */
     private $scopeConfig;
 
     /**
      * Initialize dependencies
-     *
      * @param \Magento\Directory\Model\CurrencyFactory $currencyFactory
      * @param \Magento\Framework\App\Config\ScopeConfigInterface $scopeConfig
      * @param \Magento\Framework\HTTP\ZendClientFactory $httpClientFactory
@@ -51,14 +51,22 @@ public function __construct(
     public function fetchRates()
     {
         $data = [];
+        $accessKey = $this->scopeConfig->getValue(
+            'currency/currencyconverterapi/api_key',
+            ScopeInterface::SCOPE_STORE
+        );
+        if (empty($accessKey)) {
+            $this->_messages[] = __('No API Key was specified or an invalid API Key was specified.');
+            return $data;
+        }
         $currencies = $this->_getCurrencyCodes();
         $defaultCurrencies = $this->_getDefaultCurrencyCodes();
 
         foreach ($defaultCurrencies as $currencyFrom) {
             if (!isset($data[$currencyFrom])) {
                 $data[$currencyFrom] = [];
             }
-            $data = $this->convertBatch($data, $currencyFrom, $currencies);
+            $data = $this->convertBatch($data, $currencyFrom, $currencies, $accessKey);
             ksort($data[$currencyFrom]);
         }
         return $data;
@@ -67,19 +75,31 @@ public function fetchRates()
     /**
      * Return currencies convert rates in batch mode
      *
+     * @param string $accessKey
      * @param array $data
      * @param string $currencyFrom
      * @param array $currenciesTo
      * @return array
      */
-    private function convertBatch($data, $currencyFrom, $currenciesTo)
+    private function convertBatch($data, $currencyFrom, $currenciesTo, $accessKey)
     {
         foreach ($currenciesTo as $to) {
             set_time_limit(0);
             try {
-                $url = str_replace('{{CURRENCY_FROM}}', $currencyFrom, self::CURRENCY_CONVERTER_URL);
-                $url = str_replace('{{CURRENCY_TO}}', $to, $url);
+                $url = str_replace(
+                    ['{{ACCESS_KEY}}', '{{CURRENCY_FROM}}', '{{CURRENCY_TO}}'],
+                    [$accessKey, $currencyFrom, $to],
+                    self::CURRENCY_CONVERTER_URL
+                );
+
                 $response = $this->getServiceResponse($url);
+
+                if (!$this->validateResponse($response, $currencyFrom, $to)) {
+                    $data[$currencyFrom][$to] = null;
+
+                    return $data;
+                }
+
                 if ($currencyFrom == $to) {
                     $data[$currencyFrom][$to] = $this->_numberFormat(1);
                 } else {
@@ -143,4 +163,27 @@ protected function _convert($currencyFrom, $currencyTo)
     {
         return 1;
     }
+
+    /**
+     * Validates rates response.
+     * @param array $response
+     * @param string $currencyFrom
+     * @param string $currenciesTo
+     * @return bool
+     */
+    private function validateResponse(array $response, string $currencyFrom, string $currenciesTo): bool
+    {
+        if (isset($response['error'])) {
+            $this->_messages[] = $response['error'];
+
+            return false;
+        }
+        if (empty($response[$currencyFrom . '_' . $currenciesTo])) {
+            $this->_messages[] = __('We can\'t retrieve a rate for %1.', $currenciesTo);
+
+            return false;
+        }
+
+        return true;
+    }
 }

app/code/Magento/Directory/Test/Unit/Model/Currency/Import/CurrencyConverterApiTest.php

@@ -0,0 +1,124 @@
+<?php
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+declare(strict_types=1);
+
+namespace Magento\Directory\Test\Unit\Model\Currency\Import;
+
+use Magento\Directory\Model\Currency;
+use Magento\Directory\Model\Currency\Import\CurrencyConverterApi;
+use Magento\Directory\Model\CurrencyFactory;
+use Magento\Framework\App\Config\ScopeConfigInterface;
+use Magento\Framework\DataObject;
+use Magento\Framework\HTTP\ZendClient;
+use Magento\Framework\HTTP\ZendClientFactory;
+use PHPUnit_Framework_MockObject_MockObject as MockObject;
+
+/**
+ * CurrencyConverterApiTest Test
+ */
+class CurrencyConverterApiTest extends \PHPUnit\Framework\TestCase
+{
+    /**
+     * @var CurrencyConverterApi
+     */
+    private $model;
+
+    /**
+     * @var CurrencyFactory|MockObject
+     */
+    private $currencyFactory;
+
+    /**
+     * @var ZendClientFactory|MockObject
+     */
+    private $httpClientFactory;
+
+    /**
+     * @var ScopeConfigInterface|MockObject
+     */
+    private $scopeConfig;
+
+    /**
+     * @inheritdoc
+     */
+    protected function setUp()
+    {
+        $this->currencyFactory = $this->getMockBuilder(CurrencyFactory::class)
+            ->disableOriginalConstructor()
+            ->setMethods(['create'])
+            ->getMock();
+        $this->httpClientFactory = $this->getMockBuilder(ZendClientFactory::class)
+            ->disableOriginalConstructor()
+            ->setMethods(['create'])
+            ->getMock();
+        $this->scopeConfig = $this->getMockBuilder(ScopeConfigInterface::class)
+            ->disableOriginalConstructor()
+            ->setMethods([])
+            ->getMock();
+
+        $this->model = new CurrencyConverterApi($this->currencyFactory, $this->scopeConfig, $this->httpClientFactory);
+    }
+
+    /**
+     * Test Fetch Rates
+     *
+     * @return void
+     */
+    public function testFetchRates(): void
+    {
+        $currencyFromList = ['USD'];
+        $currencyToList = ['EUR', 'UAH'];
+        $responseBody = '{"USD_EUR":0.879215}';
+        $expectedRateList = ['USD' => ['EUR' => 0.879215, 'UAH' => null]];
+        $message = "We can't retrieve a rate for UAH.";
+
+        $this->scopeConfig->method('getValue')
+            ->withConsecutive(
+                ['currency/currencyconverterapi/api_key', 'store'],
+                ['currency/currencyconverterapi/timeout', 'store']
+            )
+            ->willReturnOnConsecutiveCalls('api_key', 100);
+
+        /** @var Currency|MockObject $currency */
+        $currency = $this->getMockBuilder(Currency::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+        /** @var ZendClient|MockObject $httpClient */
+        $httpClient = $this->getMockBuilder(ZendClient::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+        /** @var DataObject|MockObject $currencyMock */
+        $httpResponse = $this->getMockBuilder(DataObject::class)
+            ->disableOriginalConstructor()
+            ->setMethods(['getBody'])
+            ->getMock();
+
+        $this->currencyFactory->method('create')
+            ->willReturn($currency);
+        $currency->method('getConfigBaseCurrencies')
+            ->willReturn($currencyFromList);
+        $currency->method('getConfigAllowCurrencies')
+            ->willReturn($currencyToList);
+
+        $this->httpClientFactory->method('create')
+            ->willReturn($httpClient);
+        $httpClient->method('setUri')
+            ->willReturnSelf();
+        $httpClient->method('setConfig')
+            ->willReturnSelf();
+        $httpClient->method('request')
+            ->willReturn($httpResponse);
+        $httpResponse->method('getBody')
+            ->willReturn($responseBody);
+
+        self::assertEquals($expectedRateList, $this->model->fetchRates());
+
+        $messages = $this->model->getMessages();
+        self::assertNotEmpty($messages);
+        self::assertTrue(is_array($messages));
+        self::assertEquals($message, (string)$messages[0]);
+    }
+}

app/code/Magento/Directory/etc/adminhtml/system.xml

@@ -47,7 +47,12 @@
             </group>
             <group id="currencyconverterapi" translate="label" sortOrder="45" showInDefault="1" showInWebsite="0" showInStore="0">
                 <label>Currency Converter API</label>
-                <field id="timeout" translate="label" type="text" sortOrder="0" showInDefault="1" showInWebsite="0" showInStore="0">
+                <field id="api_key" translate="label" type="obscure" sortOrder="5" showInDefault="1" showInWebsite="0" showInStore="0">
+                    <label>API Key</label>
+                    <config_path>currency/currencyconverterapi/api_key</config_path>
+                    <backend_model>Magento\Config\Model\Config\Backend\Encrypted</backend_model>
+                </field>
+                <field id="timeout" translate="label" type="text" sortOrder="10" showInDefault="1" showInWebsite="0" showInStore="0">
                     <label>Connection Timeout in Seconds</label>
                 </field>
             </group>

app/code/Magento/Directory/etc/config.xml

@@ -24,6 +24,7 @@
             </fixerio>
             <currencyconverterapi>
                 <timeout>100</timeout>
+                <api_key backend_model="Magento\Config\Model\Config\Backend\Encrypted" />
             </currencyconverterapi>
             <import>
                 <enabled>0</enabled>