MAGETWO-33109 : Refactor code that uses customer builders in Quote module

 -- changed Quote module to use customerFactory
 -- fixed unit tests

Author: Robert He

app/code/Magento/Quote/Model/CustomerManagement.php

@@ -8,7 +8,6 @@
 
 use Magento\Customer\Api\CustomerRepositoryInterface as CustomerRepository;
 use Magento\Customer\Api\AccountManagementInterface as AccountManagement;
-use Magento\Customer\Api\Data\CustomerDataBuilder as CustomerBuilder;
 use Magento\Customer\Api\AddressRepositoryInterface as CustomerAddressRepository;
 use Magento\Quote\Model\Quote as QuoteEntity;
 
@@ -32,27 +31,19 @@ class CustomerManagement
      */
     protected $accountManagement;
 
-    /**
-     * @var CustomerBuilder
-     */
-    protected $customerBuilder;
-
     /**
      * @param CustomerRepository $customerRepository
      * @param CustomerAddressRepository $customerAddressRepository
      * @param AccountManagement $accountManagement
-     * @param CustomerBuilder $customerBuilder
      */
     public function __construct(
         CustomerRepository $customerRepository,
         CustomerAddressRepository $customerAddressRepository,
-        AccountManagement $accountManagement,
-        CustomerBuilder $customerBuilder
+        AccountManagement $accountManagement
     ) {
         $this->customerRepository = $customerRepository;
         $this->customerAddressRepository = $customerAddressRepository;
         $this->accountManagement = $accountManagement;
-        $this->customerBuilder = $customerBuilder;
     }
 
     /**
@@ -67,7 +58,7 @@ public function populateCustomerInfo(QuoteEntity $quote)
 
         if (!$customer->getId()) {
             $customer = $this->accountManagement->createAccountWithPasswordHash(
-                $this->customerBuilder->populate($customer)->create(),
+                $customer,
                 $quote->getPasswordHash()
             );
             $quote->setCustomer($customer);

app/code/Magento/Quote/Model/Observer/Frontend/Quote/Address/CollectTotals.php

@@ -23,9 +23,9 @@ class CollectTotals
     protected $vatValidator;
 
     /**
-     * @var \Magento\Customer\Api\Data\CustomerDataBuilder
+     * @var \Magento\Customer\Api\Data\CustomerInterfaceFactory
      */
-    protected $customerBuilder;
+    protected $customerDataFactory;
 
     /**
      * Group Management
@@ -34,27 +34,35 @@ class CollectTotals
      */
     protected $groupManagement;
 
+    /**
+     * @var \Magento\Framework\Api\DataObjectHelper
+     */
+    protected $dataObjectHelper;
+
     /**
      * Initialize dependencies.
      *
      * @param \Magento\Customer\Helper\Address $customerAddressHelper
      * @param \Magento\Customer\Model\Vat $customerVat
      * @param VatValidator $vatValidator
-     * @param \Magento\Customer\Api\Data\CustomerDataBuilder $customerBuilder
+     * @param \Magento\Customer\Api\Data\CustomerInterfaceFactory $customerDataFactory
      * @param \Magento\Customer\Api\GroupManagementInterface $groupManagement
+     * @param \Magento\Framework\Api\DataObjectHelper $dataObjectHelper
      */
     public function __construct(
         \Magento\Customer\Helper\Address $customerAddressHelper,
         \Magento\Customer\Model\Vat $customerVat,
         VatValidator $vatValidator,
-        \Magento\Customer\Api\Data\CustomerDataBuilder $customerBuilder,
-        \Magento\Customer\Api\GroupManagementInterface $groupManagement
+        \Magento\Customer\Api\Data\CustomerInterfaceFactory $customerDataFactory,
+        \Magento\Customer\Api\GroupManagementInterface $groupManagement,
+        \Magento\Framework\Api\DataObjectHelper $dataObjectHelper
     ) {
         $this->customerVat = $customerVat;
         $this->customerAddressHelper = $customerAddressHelper;
         $this->vatValidator = $vatValidator;
-        $this->customerBuilder = $customerBuilder;
+        $this->customerDataFactory = $customerDataFactory;
         $this->groupManagement = $groupManagement;
+        $this->dataObjectHelper = $dataObjectHelper;
     }
 
     /**
@@ -99,7 +107,8 @@ public function dispatch(\Magento\Framework\Event\Observer $observer)
         if ($groupId) {
             $quoteAddress->setPrevQuoteCustomerGroupId($quote->getCustomerGroupId());
             $quote->setCustomerGroupId($groupId);
-            $customer = $this->customerBuilder->mergeDataObjectWithArray($customer, ['group_id' => $groupId])->create();
+            $customer = $this->customerDataFactory->create();
+            $this->dataObjectHelper->populateWithArray($customer, ['group_id' => $groupId]);
             $quote->setCustomer($customer);
         }
     }

app/code/Magento/Quote/Model/Quote.php

@@ -311,14 +311,14 @@ class Quote extends AbstractExtensibleModel implements \Magento\Quote\Api\Data\C
     protected $extensibleDataObjectConverter;
 
     /**
-     * @var \Magento\Customer\Api\Data\AttributeValueFactory
+     * @var \Magento\Customer\Api\Data\AddressInterfaceFactory
      */
-    protected $addressFactory;
+    protected $addressDataFactory;
 
     /**
-     * @var \Magento\Customer\Api\Data\CustomerDataBuilder
+     * @var \Magento\Customer\Api\Data\CustomerInterfaceFactory
      */
-    protected $customerBuilder;
+    protected $customerDataFactory;
 
     /**
      * @var \Magento\Customer\Api\CustomerRepositoryInterface
@@ -330,6 +330,11 @@ class Quote extends AbstractExtensibleModel implements \Magento\Quote\Api\Data\C
      */
     protected $currencyFactory;
 
+    /**
+     * @var \Magento\Framework\Api\DataObjectHelper
+     */
+    protected $dataObjectHelper;
+
     /**
      * @param \Magento\Framework\Model\Context $context
      * @param \Magento\Framework\Registry $registry
@@ -357,9 +362,10 @@ class Quote extends AbstractExtensibleModel implements \Magento\Quote\Api\Data\C
      * @param \Magento\Customer\Api\AddressRepositoryInterface $addressRepository
      * @param \Magento\Framework\Api\SearchCriteriaBuilder $criteriaBuilder
      * @param \Magento\Framework\Api\FilterBuilder $filterBuilder
-     * @param \Magento\Customer\Api\Data\AddressDataBuilder $addressBuilder
-     * @param \Magento\Customer\Api\Data\CustomerDataBuilder $customerBuilder
+     * @param \Magento\Customer\Api\Data\AddressInterfaceFactory $addressDataFactory
+     * @param \Magento\Customer\Api\Data\CustomerInterfaceFactory $customerDataFactory
      * @param \Magento\Customer\Api\CustomerRepositoryInterface $customerRepository
+     * @param \Magento\Framework\Api\DataObjectHelper $dataObjectHelper
      * @param \Magento\Framework\Api\ExtensibleDataObjectConverter $extensibleDataObjectConverter
      * @param Cart\CurrencyFactory $currencyFactory
      * @param \Magento\Framework\Model\Resource\AbstractResource $resource
@@ -394,9 +400,10 @@ public function __construct(
         \Magento\Customer\Api\AddressRepositoryInterface $addressRepository,
         \Magento\Framework\Api\SearchCriteriaBuilder $criteriaBuilder,
         \Magento\Framework\Api\FilterBuilder $filterBuilder,
-        \Magento\Customer\Api\Data\AddressDataBuilder $addressBuilder,
-        \Magento\Customer\Api\Data\CustomerDataBuilder $customerBuilder,
+        \Magento\Customer\Api\Data\AddressInterfaceFactory $addressDataFactory,
+        \Magento\Customer\Api\Data\CustomerInterfaceFactory $customerDataFactory,
         \Magento\Customer\Api\CustomerRepositoryInterface $customerRepository,
+        \Magento\Framework\Api\DataObjectHelper $dataObjectHelper,
         \Magento\Framework\Api\ExtensibleDataObjectConverter $extensibleDataObjectConverter,
         \Magento\Quote\Model\Cart\CurrencyFactory $currencyFactory,
         \Magento\Framework\Model\Resource\AbstractResource $resource = null,
@@ -425,9 +432,10 @@ public function __construct(
         $this->stockRegistry = $stockRegistry;
         $this->itemProcessor = $itemProcessor;
         $this->objectFactory = $objectFactory;
-        $this->addressFactory = $addressBuilder;
-        $this->customerBuilder = $customerBuilder;
+        $this->addressDataFactory = $addressDataFactory;
+        $this->customerDataFactory = $customerDataFactory;
         $this->customerRepository = $customerRepository;
+        $this->dataObjectHelper = $dataObjectHelper;
         $this->extensibleDataObjectConverter = $extensibleDataObjectConverter;
         $this->currencyFactory = $currencyFactory;
         parent::__construct(
@@ -823,14 +831,17 @@ public function setCustomer(\Magento\Customer\Api\Data\CustomerInterface $custom
         /* @TODO: Remove the method after all external usages are refactored in MAGETWO-19930 */
         $this->_customer = $customer;
         $this->setCustomerId($customer->getId());
-        $customerData = $this->objectFactory->create(
+        $customerData = $this->customerDataFactory->create();
+        $customerData->populate($customer);
+        $customerData->setAddresses([]);
+        $customerDataFlatArray = $this->objectFactory->create(
             $this->extensibleDataObjectConverter->toFlatArray(
-                $this->customerBuilder->populate($customer)->setAddresses([])->create(),
+                $customerData,
                 [],
                 '\Magento\Customer\Api\Data\CustomerInterface'
             )
         );
-        $this->_objectCopyService->copyFieldsetToTarget('customer_account', 'to_quote', $customerData, $this);
+        $this->_objectCopyService->copyFieldsetToTarget('customer_account', 'to_quote', $customerDataFlatArray, $this);
 
         return $this;
     }
@@ -850,7 +861,8 @@ public function getCustomer()
             try {
                 $this->_customer = $this->customerRepository->getById($this->getCustomerId());
             } catch (\Magento\Framework\Exception\NoSuchEntityException $e) {
-                $this->_customer = $this->customerBuilder->setId(null)->create();
+                $this->_customer = $this->customerDataFactory->create();
+                $this->_customer->setId(null);
             }
         }
 
@@ -886,10 +898,7 @@ public function addCustomerAddress(\Magento\Customer\Api\Data\AddressInterface $
     {
         $addresses = (array)$this->getCustomer()->getAddresses();
         $addresses[] = $address;
-        $customer = $this->customerBuilder->populate($this->getCustomer())
-            ->setAddresses($addresses)
-            ->create();
-        $this->setCustomer($customer);
+        $this->getCustomer()->setAddresses($addresses);
         return $this;
     }
 
@@ -901,8 +910,8 @@ public function addCustomerAddress(\Magento\Customer\Api\Data\AddressInterface $
      */
     public function updateCustomerData(\Magento\Customer\Api\Data\CustomerInterface $customer)
     {
-        $customer = $this->customerBuilder->mergeDataObjects($this->getCustomer(), $customer)
-            ->create();
+        $customer = $this->customerDataFactory->mergeDataObjects(get_class($this->getCustomer()),
+            $this->getCustomer(), $customer);
         $this->setCustomer($customer);
         return $this;
     }

dev/tests/unit/testsuite/Magento/Quote/Model/CustomerManagementTest.php

@@ -21,16 +21,11 @@ class CustomerManagementTest extends \PHPUnit_Framework_TestCase
     /**
      * @var \Magento\Customer\Api\AccountManagementInterface|\PHPUnit_Framework_MockObject_MockObject
      */
-    protected $customerAddressRepositoryMock;
-    /**
-     * @var \Magento\Customer\Api\Data\CustomerDataBuilder|\PHPUnit_Framework_MockObject_MockObject
-     *
-     */
     protected $accountManagementMock;
     /**
      * @var \Magento\Customer\Api\AddressRepositoryInterface|\PHPUnit_Framework_MockObject_MockObject
      */
-    protected $customerBuilderMock;
+    protected $customerAddressRepositoryMock;
     /**
      * @var \Magento\Quote\Model\Quote|\PHPUnit_Framework_MockObject_MockObject
      */
@@ -39,7 +34,6 @@ class CustomerManagementTest extends \PHPUnit_Framework_TestCase
      * @var \Magento\Quote\Model\Quote\Address|\PHPUnit_Framework_MockObject_MockObject
      */
     protected $quoteAddressMock;
-
     /**
      * @var \Magento\Customer\Api\Data\CustomerInterface|\PHPUnit_Framework_MockObject_MockObject
      */
@@ -79,13 +73,6 @@ public function setUp()
             true,
             []
         );
-        $this->customerBuilderMock = $this->getMock(
-            'Magento\Customer\Api\Data\CustomerDataBuilder',
-            ['populate', 'create'],
-            [],
-            '',
-            false
-        );
         $this->quoteMock = $this->getMock(
             'Magento\Quote\Model\Quote',
             ['getId', 'getCustomer', 'getBillingAddress', 'getShippingAddress', 'setCustomer', 'getPasswordHash'],
@@ -121,8 +108,7 @@ public function setUp()
         $this->customerManagement = new \Magento\Quote\Model\CustomerManagement(
             $this->customerRepositoryMock,
             $this->customerAddressRepositoryMock,
-            $this->accountManagementMock,
-            $this->customerBuilderMock
+            $this->accountManagementMock
         );
     }
 
@@ -134,16 +120,9 @@ public function testPopulateCustomerInfo()
         $this->customerMock->expects($this->atLeastOnce())
             ->method('getId')
             ->willReturn(null);
-        $this->customerBuilderMock->expects($this->once())
-            ->method('populate')
-            ->with($this->customerMock)
-            ->willReturnSelf();
         $this->customerMock->expects($this->atLeastOnce())
             ->method('getDefaultBilling')
             ->willReturn(100500);
-        $this->customerBuilderMock->expects($this->once())
-            ->method('create')
-            ->willReturn($this->customerMock);
         $this->quoteMock->expects($this->atLeastOnce())
             ->method('getBillingAddress')
             ->willReturn($this->quoteAddressMock);

dev/tests/unit/testsuite/Magento/Quote/Model/Observer/Frontend/Quote/Address/CollectTotalsTest.php

@@ -61,7 +61,7 @@ class CollectTotalsTest extends \PHPUnit_Framework_TestCase
     /**
      * @var \PHPUnit_Framework_MockObject_MockObject
      */
-    protected $customerBuilderMock;
+    protected $customerDataFactoryMock;
 
     /**
      * @var \Magento\TestFramework\Helper\ObjectManager
@@ -78,6 +78,11 @@ class CollectTotalsTest extends \PHPUnit_Framework_TestCase
      */
     protected $groupInterfaceMock;
 
+    /**
+     * @var \PHPUnit_Framework_MockObject_MockObject
+     */
+    protected $dataObjectHelperMock;
+
     /**
      * @SuppressWarnings(PHPMD.ExcessiveMethodLength)
      */
@@ -96,8 +101,8 @@ protected function setUp()
         );
         $this->customerAddressMock = $this->getMock('Magento\Customer\Helper\Address', [], [], '', false);
         $this->customerVatMock = $this->getMock('Magento\Customer\Model\Vat', [], [], '', false);
-        $this->customerBuilderMock = $this->getMock(
-            'Magento\Customer\Api\Data\CustomerDataBuilder',
+        $this->customerDataFactoryMock = $this->getMock(
+            'Magento\Customer\Api\Data\CustomerInterfaceFactory',
             ['mergeDataObjectWithArray', 'create'],
             [],
             '',
@@ -158,6 +163,10 @@ protected function setUp()
             ['getId']
         );
 
+        $this->dataObjectHelperMock = $this->getMockBuilder('Magento\Framework\Api\DataObjectHelper')
+            ->disableOriginalConstructor()
+            ->getMock();
+
         $this->observerMock->expects($this->any())
             ->method('getQuoteAddress')
             ->will($this->returnValue($this->quoteAddressMock));
@@ -176,8 +185,9 @@ protected function setUp()
                 'customerAddressHelper' => $this->customerAddressMock,
                 'customerVat' => $this->customerVatMock,
                 'vatValidator' => $this->vatValidatorMock,
-                'customerBuilder' => $this->customerBuilderMock,
-                'groupManagement' => $this->groupManagementMock
+                'customerDataFactory' => $this->customerDataFactoryMock,
+                'groupManagement' => $this->groupManagementMock,
+                'dataObjectHelper' => $this->dataObjectHelperMock,
             ]
         );
     }
@@ -235,7 +245,7 @@ public function testDispatchWithCustomerCountryNotInEUAndNotLoggedCustomerInGrou
 
         /** Assertions */
         $this->quoteAddressMock->expects($this->never())->method('setPrevQuoteCustomerGroupId');
-        $this->customerBuilderMock->expects($this->never())->method('mergeDataObjectWithArray');
+        $this->customerDataFactoryMock->expects($this->never())->method('mergeDataObjectWithArray');
         $this->quoteMock->expects($this->never())->method('setCustomerGroupId');
 
         /** SUT execution */
@@ -270,11 +280,11 @@ public function testDispatchWithDefaultCustomerGroupId()
             ->method('setPrevQuoteCustomerGroupId')
             ->with('customerGroupId');
         $this->quoteMock->expects($this->once())->method('setCustomerGroupId')->with('defaultCustomerGroupId');
-        $this->customerBuilderMock->expects($this->once())
-            ->method('mergeDataObjectWithArray')
+        $this->dataObjectHelperMock->expects($this->once())
+            ->method('populateWithArray')
             ->with($this->customerMock, ['group_id' => 'defaultCustomerGroupId'])
             ->will($this->returnSelf());
-        $this->customerBuilderMock->expects($this->any())
+        $this->customerDataFactoryMock->expects($this->any())
             ->method('create')
             ->willReturn($this->customerMock);
 
@@ -326,11 +336,11 @@ public function testDispatchWithCustomerCountryInEU()
 
         $this->quoteMock->expects($this->once())->method('setCustomerGroupId')->with('customerGroupId');
         $this->quoteMock->expects($this->once())->method('setCustomer')->with($this->customerMock);
-        $this->customerBuilderMock->expects($this->once())
-            ->method('mergeDataObjectWithArray')
+        $this->dataObjectHelperMock->expects($this->once())
+            ->method('populateWithArray')
             ->with($this->customerMock, ['group_id' => 'customerGroupId'])
             ->will($this->returnSelf());
-        $this->customerBuilderMock->expects($this->any())
+        $this->customerDataFactoryMock->expects($this->any())
             ->method('create')
             ->willReturn($this->customerMock);
         $this->model->dispatch($this->observerMock);