MAGETWO-52856: [WebAPI] Improve WebAPI performance of Checkout Payment Info/Place order call

irenelagno · irenelagno · commit 3495e603b578 · 2016-12-30T12:47:16.000+02:00
diff --git a/app/code/Magento/Checkout/Model/PaymentInformationManagement.php b/app/code/Magento/Checkout/Model/PaymentInformationManagement.php
@@ -14,6 +14,7 @@ class PaymentInformationManagement implements \Magento\Checkout\Api\PaymentInfor
 {
     /**
      * @var \Magento\Quote\Api\BillingAddressManagementInterface
+     * @deprecated This call was substituted to eliminate extra quote::save call
      */
     protected $billingAddressManagement;
 
@@ -42,6 +43,11 @@ class PaymentInformationManagement implements \Magento\Checkout\Api\PaymentInfor
      */
     private $logger;
 
+    /**
+     * @var \Magento\Quote\Api\CartRepositoryInterface
+     */
+    private $cartRepository;
+
     /**
      * @param \Magento\Quote\Api\BillingAddressManagementInterface $billingAddressManagement
      * @param \Magento\Quote\Api\PaymentMethodManagementInterface $paymentMethodManagement
@@ -99,7 +105,19 @@ public function savePaymentInformation(
         \Magento\Quote\Api\Data\AddressInterface $billingAddress = null
     ) {
         if ($billingAddress) {
-            $this->billingAddressManagement->assign($cartId, $billingAddress);
+            /** @var \Magento\Quote\Api\CartRepositoryInterface $quoteRepository */
+            $quoteRepository = $this->getCartRepository();
+            /** @var \Magento\Quote\Model\Quote $quote */
+            $quote = $quoteRepository->getActive($cartId);
+            $quote->removeAddress($quote->getBillingAddress()->getId());
+            $quote->setBillingAddress($billingAddress);
+            $quote->setDataChanges(true);
+            $shippingAddress = $quote->getShippingAddress();
+            if ($shippingAddress && $shippingAddress->getShippingMethod()) {
+                $shippingDataArray = explode('_', $shippingAddress->getShippingMethod());
+                $shippingCarrier = array_shift($shippingDataArray);
+                $shippingAddress->setLimitCarrier($shippingCarrier);
+            }
         }
         $this->paymentMethodManagement->set($cartId, $paymentMethod);
         return true;
@@ -130,4 +148,19 @@ private function getLogger()
         }
         return $this->logger;
     }
+
+    /**
+     * Get Cart repository
+     *
+     * @return \Magento\Quote\Api\CartRepositoryInterface
+     * @deprecated
+     */
+    private function getCartRepository()
+    {
+        if (!$this->cartRepository) {
+            $this->cartRepository = \Magento\Framework\App\ObjectManager::getInstance()
+                ->get(\Magento\Quote\Api\CartRepositoryInterface::class);
+        }
+        return $this->cartRepository;
+    }
 }
diff --git a/app/code/Magento/Checkout/Model/ShippingInformationManagement.php b/app/code/Magento/Checkout/Model/ShippingInformationManagement.php
@@ -144,6 +144,7 @@ public function saveAddressInformation(
 
         /** @var \Magento\Quote\Model\Quote $quote */
         $quote = $this->quoteRepository->getActive($cartId);
+        $address->setLimitCarrier($carrierCode);
         $quote = $this->prepareShippingAssignment($quote, $address, $carrierCode . '_' . $methodCode);
         $this->validateQuote($quote);
         $quote->setIsMultiShipping(false);
diff --git a/app/code/Magento/Checkout/Test/Unit/Model/PaymentInformationManagementTest.php b/app/code/Magento/Checkout/Test/Unit/Model/PaymentInformationManagementTest.php
@@ -5,8 +5,9 @@
  */
 namespace Magento\Checkout\Test\Unit\Model;
 
-use Magento\Framework\Exception\CouldNotSaveException;
-
+/**
+ * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
+ */
 class PaymentInformationManagementTest extends \PHPUnit_Framework_TestCase
 {
     /**
@@ -34,6 +35,11 @@ class PaymentInformationManagementTest extends \PHPUnit_Framework_TestCase
      */
     private $loggerMock;
 
+    /**
+     * @var \PHPUnit_Framework_MockObject_MockObject
+     */
+    private $cartRepositoryMock;
+
     protected function setUp()
     {
         $objectManager = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this);
@@ -46,7 +52,7 @@ protected function setUp()
         $this->cartManagementMock = $this->getMock(\Magento\Quote\Api\CartManagementInterface::class);
 
         $this->loggerMock = $this->getMock(\Psr\Log\LoggerInterface::class);
-
+        $this->cartRepositoryMock = $this->getMockBuilder(\Magento\Quote\Api\CartRepositoryInterface::class)->getMock();
         $this->model = $objectManager->getObject(
             \Magento\Checkout\Model\PaymentInformationManagement::class,
             [
@@ -56,6 +62,7 @@ protected function setUp()
             ]
         );
         $objectManager->setBackwardCompatibleProperty($this->model, 'logger', $this->loggerMock);
+        $objectManager->setBackwardCompatibleProperty($this->model, 'cartRepository', $this->cartRepositoryMock);
     }
 
     public function testSavePaymentInformationAndPlaceOrder()
@@ -65,9 +72,7 @@ public function testSavePaymentInformationAndPlaceOrder()
         $paymentMock = $this->getMock(\Magento\Quote\Api\Data\PaymentInterface::class);
         $billingAddressMock = $this->getMock(\Magento\Quote\Api\Data\AddressInterface::class);
 
-        $this->billingAddressManagementMock->expects($this->once())
-            ->method('assign')
-            ->with($cartId, $billingAddressMock);
+        $this->getMockForAssignBillingAddress($cartId, $billingAddressMock);
         $this->paymentMethodManagementMock->expects($this->once())->method('set')->with($cartId, $paymentMock);
         $this->cartManagementMock->expects($this->once())->method('placeOrder')->with($cartId)->willReturn($orderId);
 
@@ -87,9 +92,7 @@ public function testSavePaymentInformationAndPlaceOrderException()
         $paymentMock = $this->getMock(\Magento\Quote\Api\Data\PaymentInterface::class);
         $billingAddressMock = $this->getMock(\Magento\Quote\Api\Data\AddressInterface::class);
 
-        $this->billingAddressManagementMock->expects($this->once())
-            ->method('assign')
-            ->with($cartId, $billingAddressMock);
+        $this->getMockForAssignBillingAddress($cartId, $billingAddressMock);
         $this->paymentMethodManagementMock->expects($this->once())->method('set')->with($cartId, $paymentMock);
         $exception = new \Exception(__('DB exception'));
         $this->loggerMock->expects($this->once())->method('critical');
@@ -104,7 +107,6 @@ public function testSavePaymentInformationAndPlaceOrderIfBillingAddressNotExist(
         $orderId = 200;
         $paymentMock = $this->getMock(\Magento\Quote\Api\Data\PaymentInterface::class);
 
-        $this->billingAddressManagementMock->expects($this->never())->method('assign');
         $this->paymentMethodManagementMock->expects($this->once())->method('set')->with($cartId, $paymentMock);
         $this->cartManagementMock->expects($this->once())->method('placeOrder')->with($cartId)->willReturn($orderId);
 
@@ -120,9 +122,7 @@ public function testSavePaymentInformation()
         $paymentMock = $this->getMock(\Magento\Quote\Api\Data\PaymentInterface::class);
         $billingAddressMock = $this->getMock(\Magento\Quote\Api\Data\AddressInterface::class);
 
-        $this->billingAddressManagementMock->expects($this->once())
-            ->method('assign')
-            ->with($cartId, $billingAddressMock);
+        $this->getMockForAssignBillingAddress($cartId, $billingAddressMock);
         $this->paymentMethodManagementMock->expects($this->once())->method('set')->with($cartId, $paymentMock);
 
         $this->assertTrue($this->model->savePaymentInformation($cartId, $paymentMock, $billingAddressMock));
@@ -133,7 +133,6 @@ public function testSavePaymentInformationWithoutBillingAddress()
         $cartId = 100;
         $paymentMock = $this->getMock(\Magento\Quote\Api\Data\PaymentInterface::class);
 
-        $this->billingAddressManagementMock->expects($this->never())->method('assign');
         $this->paymentMethodManagementMock->expects($this->once())->method('set')->with($cartId, $paymentMock);
 
         $this->assertTrue($this->model->savePaymentInformation($cartId, $paymentMock));
@@ -149,9 +148,8 @@ public function testSavePaymentInformationAndPlaceOrderWithLocolizedException()
         $paymentMock = $this->getMock(\Magento\Quote\Api\Data\PaymentInterface::class);
         $billingAddressMock = $this->getMock(\Magento\Quote\Api\Data\AddressInterface::class);
 
-        $this->billingAddressManagementMock->expects($this->once())
-            ->method('assign')
-            ->with($cartId, $billingAddressMock);
+        $this->getMockForAssignBillingAddress($cartId, $billingAddressMock);
+
         $this->paymentMethodManagementMock->expects($this->once())->method('set')->with($cartId, $paymentMock);
         $phrase = new \Magento\Framework\Phrase(__('DB exception'));
         $exception = new \Magento\Framework\Exception\LocalizedException($phrase);
@@ -160,4 +158,31 @@ public function testSavePaymentInformationAndPlaceOrderWithLocolizedException()
 
         $this->model->savePaymentInformationAndPlaceOrder($cartId, $paymentMock, $billingAddressMock);
     }
+
+    /**
+     * @param int $cartId
+     * @param \PHPUnit_Framework_MockObject_MockObject $billingAddressMock
+     */
+    private function getMockForAssignBillingAddress($cartId, $billingAddressMock)
+    {
+        $billingAddressId = 1;
+        $quoteMock = $this->getMock(\Magento\Quote\Model\Quote::class, [], [], '', false);
+        $quoteBillingAddress = $this->getMock(\Magento\Quote\Model\Quote\Address::class, [], [], '', false);
+        $quoteShippingAddress = $this->getMock(
+            \Magento\Quote\Model\Quote\Address::class,
+            ['setLimitCarrier', 'getShippingMethod'],
+            [],
+            '',
+            false
+        );
+        $this->cartRepositoryMock->expects($this->any())->method('getActive')->with($cartId)->willReturn($quoteMock);
+        $quoteMock->expects($this->once())->method('getBillingAddress')->willReturn($quoteBillingAddress);
+        $quoteMock->expects($this->once())->method('getShippingAddress')->willReturn($quoteShippingAddress);
+        $quoteBillingAddress->expects($this->once())->method('getId')->willReturn($billingAddressId);
+        $quoteMock->expects($this->once())->method('removeAddress')->with($billingAddressId);
+        $quoteMock->expects($this->once())->method('setBillingAddress')->with($billingAddressMock);
+        $quoteMock->expects($this->once())->method('setDataChanges')->willReturnSelf();
+        $quoteShippingAddress->expects($this->any())->method('getShippingMethod')->willReturn('flatrate_flatrate');
+        $quoteShippingAddress->expects($this->once())->method('setLimitCarrier')->with('flatrate')->willReturnSelf();
+    }
 }
diff --git a/app/code/Magento/Checkout/Test/Unit/Model/ShippingInformationManagementTest.php b/app/code/Magento/Checkout/Test/Unit/Model/ShippingInformationManagementTest.php
@@ -109,7 +109,8 @@ protected function setUp()
                 'importCustomerAddressData',
                 'save',
                 'getShippingRateByCode',
-                'getShippingMethod'
+                'getShippingMethod',
+                'setLimitCarrier'
             ],
             [],
             '',
@@ -208,7 +209,7 @@ public function testSaveAddressInformationIfCartIsEmpty()
     private function setShippingAssignmentsMocks($shippingMethod)
     {
         $this->quoteMock->expects($this->once())->method('getExtensionAttributes')->willReturn(null);
-
+        $this->shippingAddressMock->expects($this->once())->method('setLimitCarrier');
         $this->cartExtensionMock = $this->getMock(
             \Magento\Quote\Api\Data\CartExtension::class,
             ['getShippingAssignments', 'setShippingAssignments'],
diff --git a/app/code/Magento/Quote/Model/CustomerManagement.php b/app/code/Magento/Quote/Model/CustomerManagement.php
@@ -62,8 +62,6 @@ public function populateCustomerInfo(QuoteEntity $quote)
                 $quote->getPasswordHash()
             );
             $quote->setCustomer($customer);
-        } else {
-            $this->customerRepository->save($customer);
         }
         if (!$quote->getBillingAddress()->getId() && $customer->getDefaultBilling()) {
             $quote->getBillingAddress()->importCustomerAddressData(
diff --git a/app/code/Magento/Quote/Test/Unit/Model/CustomerManagementTest.php b/app/code/Magento/Quote/Test/Unit/Model/CustomerManagementTest.php
@@ -158,4 +158,34 @@ public function testPopulateCustomerInfo()
             ->willReturn($this->customerMock);
         $this->customerManagement->populateCustomerInfo($this->quoteMock);
     }
+
+    public function testPopulateCustomerInfoForExistingCustomer()
+    {
+        $this->quoteMock->expects($this->once())
+            ->method('getCustomer')
+            ->willReturn($this->customerMock);
+        $this->customerMock->expects($this->atLeastOnce())
+            ->method('getId')
+            ->willReturn(1);
+        $this->customerMock->expects($this->atLeastOnce())
+            ->method('getDefaultBilling')
+            ->willReturn(100500);
+        $this->quoteMock->expects($this->atLeastOnce())
+            ->method('getBillingAddress')
+            ->willReturn($this->quoteAddressMock);
+        $this->quoteMock->expects($this->atLeastOnce())
+            ->method('getShippingAddress')
+            ->willReturn($this->quoteAddressMock);
+        $this->quoteAddressMock->expects($this->atLeastOnce())
+            ->method('getId')
+            ->willReturn(null);
+        $this->customerAddressRepositoryMock->expects($this->atLeastOnce())
+            ->method('getById')
+            ->with(100500)
+            ->willReturn($this->customerAddressMock);
+        $this->quoteAddressMock->expects($this->atLeastOnce())
+            ->method('importCustomerAddressData')
+            ->willReturnSelf();
+        $this->customerManagement->populateCustomerInfo($this->quoteMock);
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -62,8 +62,6 @@ public function populateCustomerInfo(QuoteEntity $quote)`
`62`	`62`	`$quote->getPasswordHash()`
`63`	`63`	`);`
`64`	`64`	`$quote->setCustomer($customer);`
`65`		`- } else {`
`66`		`- $this->customerRepository->save($customer);`
`67`	`65`	`}`
`68`	`66`	`if (!$quote->getBillingAddress()->getId() && $customer->getDefaultBilling()) {`
`69`	`67`	`$quote->getBillingAddress()->importCustomerAddressData(`