2.3 only - creating attribute option value using API returns unexpected response #18392

app/code/Magento/Eav/Model/Entity/Attribute/OptionManagement.php

@@ -6,36 +6,54 @@
 
 namespace Magento\Eav\Model\Entity\Attribute;
 
+use Magento\Eav\Api\AttributeOptionManagementInterface;
+use Magento\Eav\Api\Data\AttributeOptionInterface;
+use Magento\Eav\Model\AttributeRepository;
+use Magento\Eav\Model\ResourceModel\Entity\Attribute;
+use Magento\Eav\Model\ResourceModel\GetAttributeOptionId;
 use Magento\Framework\Exception\InputException;
 use Magento\Framework\Exception\NoSuchEntityException;
 use Magento\Framework\Exception\StateException;
+use Magento\Framework\App\ObjectManager;
 
 /**
  * Eav Option Management
  */
-class OptionManagement implements \Magento\Eav\Api\AttributeOptionManagementInterface
+class OptionManagement implements AttributeOptionManagementInterface
 {
     /**
-     * @var \Magento\Eav\Model\AttributeRepository
+     * @var AttributeRepository
      */
     protected $attributeRepository;
 
     /**
-     * @var \Magento\Eav\Model\ResourceModel\Entity\Attribute
+     * @var Attribute
      */
     protected $resourceModel;
 
     /**
-     * @param \Magento\Eav\Model\AttributeRepository $attributeRepository
-     * @param \Magento\Eav\Model\ResourceModel\Entity\Attribute $resourceModel
+     * @var GetAttributeOptionId
+     */
+    private $getAttributeOptionId;
+
+    /**
+     * @param AttributeRepository $attributeRepository
+     * @param Attribute $resourceModel
+     * @param GetAttributeOptionId $getAttributeOptionId
      * @codeCoverageIgnore
      */
     public function __construct(
-        \Magento\Eav\Model\AttributeRepository $attributeRepository,
-        \Magento\Eav\Model\ResourceModel\Entity\Attribute $resourceModel
+        AttributeRepository $attributeRepository = null,
+        Attribute $resourceModel,
+        GetAttributeOptionId $getAttributeOptionId = null
     ) {
-        $this->attributeRepository = $attributeRepository;
+        $this->attributeRepository = $attributeRepository ?: ObjectManager::getInstance()->get(
+            AttributeRepository::class
+        );
         $this->resourceModel = $resourceModel;
+        $this->getAttributeOptionId = $getAttributeOptionId ?: ObjectManager::getInstance()->get(
+            GetAttributeOptionId::class
+        );
     }
 
     /**
@@ -53,19 +71,16 @@ public function add($entityType, $attributeCode, $option)
         }
 
         $optionLabel = $option->getLabel();
-        $optionId = $this->getOptionId($option);
+        $optionValue = $this->getOptionValue($option);
         $options = [];
-        $options['value'][$optionId][0] = $optionLabel;
-        $options['order'][$optionId] = $option->getSortOrder();
+        $options['value'][$optionValue][0] = $optionLabel;
+        $options['order'][$optionValue] = $option->getSortOrder();
+        $attributeId = $attribute->getAttributeId();
 
-        if (is_array($option->getStoreLabels())) {
-            foreach ($option->getStoreLabels() as $label) {
-                $options['value'][$optionId][$label->getStoreId()] = $label->getLabel();
-            }
-        }
+        $options = $this->processStoreLabels($option, $options, $optionValue);
 
         if ($option->getIsDefault()) {
-            $attribute->setDefault([$optionId]);
+            $attribute->setDefault([$optionValue]);
         }
 
         $attribute->setOption($options);
@@ -78,7 +93,9 @@ public function add($entityType, $attributeCode, $option)
             throw new StateException(__('The "%1" attribute can\'t be saved.', $attributeCode));
         }
 
-        return $this->getOptionId($option);
+        $optionValue = is_array($option->getStoreLabels()) ? $optionValue : $optionLabel;
+
+        return $this->getAttributeOptionId->execute($attributeId, $optionValue);
     }
 
     /**
@@ -153,26 +170,26 @@ protected function validateOption($attribute, $optionId)
     }
 
     /**
-     * Returns option id
+     * Returns option value
      *
-     * @param \Magento\Eav\Api\Data\AttributeOptionInterface $option
+     * @param AttributeOptionInterface $option
      * @return string
      */
-    private function getOptionId(\Magento\Eav\Api\Data\AttributeOptionInterface $option) : string
+    private function getOptionValue(AttributeOptionInterface $option) : string
     {
-        return 'id_' . ($option->getValue() ?: 'new_option');
+        return 'value_' . ($option->getValue() ?: 'new_option');
     }
 
     /**
      * Set option value
      *
-     * @param \Magento\Eav\Api\Data\AttributeOptionInterface $option
+     * @param AttributeOptionInterface $option
      * @param \Magento\Eav\Api\Data\AttributeInterface $attribute
      * @param string $optionLabel
      * @return void
      */
     private function setOptionValue(
-        \Magento\Eav\Api\Data\AttributeOptionInterface $option,
+        AttributeOptionInterface $option,
         \Magento\Eav\Api\Data\AttributeInterface $attribute,
         string $optionLabel
     ) {
@@ -188,4 +205,23 @@ private function setOptionValue(
             }
         }
     }
+
+    /**
+     * Process option store labels.
+     *
+     * @param AttributeOptionInterface $option
+     * @param array $options
+     * @param string $optionValue
+     * @return array
+     */
+    private function processStoreLabels(AttributeOptionInterface $option, array $options, string $optionValue): array
+    {
+        if (is_array($option->getStoreLabels())) {
+            foreach ($option->getStoreLabels() as $label) {
+                $options['value'][$optionValue][$label->getStoreId()] = $label->getLabel();
+            }
+        }
+
+        return $options;
+    }
 }

app/code/Magento/Eav/Model/ResourceModel/GetAttributeOptionId.php

@@ -0,0 +1,62 @@
+<?php
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+declare(strict_types=1);
+
+namespace Magento\Eav\Model\ResourceModel;
+
+use Magento\Framework\App\ResourceConnection;
+
+/**
+ * Get Attribute Option Id by Attribute Id and Option Label.
+ */
+class GetAttributeOptionId
+{
+    /**
+     * @var ResourceConnection
+     */
+    private $resource;
+
+    /**
+     * @param ResourceConnection $connection
+     */
+    public function __construct(
+        ResourceConnection $connection
+    ) {
+        $this->resource = $connection;
+    }
+
+    /**
+     * Returns Attribute Option Id by Attribute Id and Option Value.
+     *
+     * @param int $attributeId
+     * @param string $optionValue
+     *
+     * @return string
+     */
+    public function execute(int $attributeId, string $optionValue): string
+    {
+        $connection = $this->resource->getConnection();
+        $optionTable = $this->resource->getTableName('eav_attribute_option');
+        $optionValueTable = $this->resource->getTableName('eav_attribute_option_value');
+        $select = $connection->select()
+            ->from(
+                ['ovt' => $optionValueTable],
+                'option_id'
+            )->joinInner(
+                ['ot' => $optionTable],
+                'ovt.option_id = ot.option_id',
+                []
+            )->where(
+                'ovt.value = ?',
+                $optionValue
+            )->where(
+                'ot.attribute_id = ?',
+                $attributeId
+            );
+
+        return (string)$connection->fetchOne($select);
+    }
+}

app/code/Magento/Eav/Test/Unit/Model/Entity/Attribute/OptionManagementTest.php

@@ -23,14 +23,22 @@ class OptionManagementTest extends \PHPUnit\Framework\TestCase
      */
     protected $resourceModelMock;
 
+    /**
+     * @var \PHPUnit_Framework_MockObject_MockObject
+     */
+    private $getAttributeOptionIdMock;
+
     protected function setUp()
     {
         $this->attributeRepositoryMock = $this->createMock(\Magento\Eav\Model\AttributeRepository::class);
         $this->resourceModelMock =
             $this->createMock(\Magento\Eav\Model\ResourceModel\Entity\Attribute::class);
+        $this->getAttributeOptionIdMock =
+            $this->createMock(\Magento\Eav\Model\ResourceModel\GetAttributeOptionId::class);
         $this->model = new \Magento\Eav\Model\Entity\Attribute\OptionManagement(
             $this->attributeRepositoryMock,
-            $this->resourceModelMock
+            $this->resourceModelMock,
+            $this->getAttributeOptionIdMock
         );
     }
 
@@ -54,18 +62,18 @@ public function testAdd()
             false,
             false,
             true,
-            ['usesSource', 'setDefault', 'setOption']
+            ['usesSource', 'setDefault', 'setOption', 'getAttributeId']
         );
         $labelMock = $this->createMock(\Magento\Eav\Api\Data\AttributeOptionLabelInterface::class);
         $option =
             ['value' => [
-                'id_new_option' => [
+                'value_new_option' => [
                     0 => 'optionLabel',
                     42 => 'labelLabel',
                 ],
             ],
             'order' => [
-                'id_new_option' => 'optionSortOrder',
+                'value_new_option' => 'optionSortOrder',
             ],
             ];
 
@@ -74,14 +82,20 @@ public function testAdd()
         $attributeMock->expects($this->once())->method('usesSource')->willReturn(true);
         $optionMock->expects($this->once())->method('getLabel')->willReturn('optionLabel');
         $optionMock->expects($this->once())->method('getSortOrder')->willReturn('optionSortOrder');
-        $optionMock->expects($this->exactly(2))->method('getStoreLabels')->willReturn([$labelMock]);
+        $optionMock->expects($this->exactly(3))->method('getStoreLabels')->willReturn([$labelMock]);
         $labelMock->expects($this->once())->method('getStoreId')->willReturn(42);
         $labelMock->expects($this->once())->method('getLabel')->willReturn('labelLabel');
         $optionMock->expects($this->once())->method('getIsDefault')->willReturn(true);
-        $attributeMock->expects($this->once())->method('setDefault')->with(['id_new_option']);
+        $attributeMock->expects($this->once())->method('setDefault')->with(['value_new_option']);
         $attributeMock->expects($this->once())->method('setOption')->with($option);
+        $attributeMock->expects($this->once())->method('getAttributeId')->willReturn(93);
         $this->resourceModelMock->expects($this->once())->method('save')->with($attributeMock);
-        $this->assertEquals('id_new_option', $this->model->add($entityType, $attributeCode, $optionMock));
+        $this->getAttributeOptionIdMock
+            ->expects($this->once())
+            ->method('execute')
+            ->with(93, 'value_new_option')
+            ->willReturn(42);
+        $this->assertEquals(42, $this->model->add($entityType, $attributeCode, $optionMock));
     }
 
     /**
@@ -162,18 +176,18 @@ public function testAddWithCannotSaveException()
             false,
             false,
             true,
-            ['usesSource', 'setDefault', 'setOption']
+            ['usesSource', 'setDefault', 'setOption', 'getAttributeCode']
         );
         $labelMock = $this->createMock(\Magento\Eav\Api\Data\AttributeOptionLabelInterface::class);
         $option =
             ['value' => [
-                'id_new_option' => [
+                'value_new_option' => [
                     0 => 'optionLabel',
                     42 => 'labelLabel',
                 ],
             ],
                 'order' => [
-                    'id_new_option' => 'optionSortOrder',
+                    'value_new_option' => 'optionSortOrder',
                 ],
             ];
 
@@ -186,7 +200,7 @@ public function testAddWithCannotSaveException()
         $labelMock->expects($this->once())->method('getStoreId')->willReturn(42);
         $labelMock->expects($this->once())->method('getLabel')->willReturn('labelLabel');
         $optionMock->expects($this->once())->method('getIsDefault')->willReturn(true);
-        $attributeMock->expects($this->once())->method('setDefault')->with(['id_new_option']);
+        $attributeMock->expects($this->once())->method('setDefault')->with(['value_new_option']);
         $attributeMock->expects($this->once())->method('setOption')->with($option);
         $this->resourceModelMock->expects($this->once())->method('save')->with($attributeMock)
             ->willThrowException(new \Exception());

dev/tests/api-functional/testsuite/Magento/Eav/Api/OptionManagementTest.php

@@ -0,0 +1,58 @@
+<?php
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+
+namespace Magento\Eav\Api;
+
+use Magento\TestFramework\TestCase\WebapiAbstract;
+
+class OptionManagementTest extends WebapiAbstract
+{
+    /**
+     * Test creating Attribute Option.
+     *
+     * @magentoApiDataFixture Magento/Catalog/_files/multiselect_attribute.php
+     */
+    public function testAttributeOptionAdding()
+    {
+        $option = [
+            'label' => 'test_option_' . md5(random_int(0, PHP_INT_MAX)),
+            'value' => 'test_option_' . md5(random_int(0, PHP_INT_MAX)),
+        ];
+        $attributeCode = 'multiselect_attribute';
+
+        // Integer value should be returned - this indicates Option was created.
+        $this->assertInternalType(
+            'numeric',
+            $this->addAttributeOption($attributeCode, $option)
+        );
+        // Empty result should be returned - this indicates Option already exists.
+        $this->assertEmpty(
+            $this->addAttributeOption($attributeCode, $option)
+        );
+    }
+
+    /**
+     * @param string $attributeCode
+     * @param array $option
+     * @return array|bool|float|int|string
+     */
+    private function addAttributeOption(string $attributeCode, array $option)
+    {
+        $serviceInfo = [
+            'rest' => [
+                'resourcePath' => '/V1/products/attributes/' . $attributeCode . '/options',
+                'httpMethod' => \Magento\Framework\Webapi\Rest\Request::HTTP_METHOD_POST,
+            ],
+            'soap' => [
+                'service' => 'catalogProductAttributeOptionManagementV1',
+                'serviceVersion' => 'V1',
+                'operation' => 'catalogProductAttributeOptionManagementV1add',
+            ],
+        ];
+
+        return $this->_webApiCall($serviceInfo, ['option' => $option, 'attributeCode' => $attributeCode]);
+    }
+}