Skip to content

Commit

Permalink
ENGCOM-4712: Fix for Issue #7227: "x_forwarded_for" value is always e…
Browse files Browse the repository at this point in the history
…mpty in Order object #21787
  • Loading branch information
sidolov authored Apr 25, 2019
2 parents 2da1c1f + 11cedb4 commit 0ed0838
Show file tree
Hide file tree
Showing 2 changed files with 91 additions and 18 deletions.
31 changes: 30 additions & 1 deletion app/code/Magento/Quote/Model/QuoteManagement.php
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,16 @@ class QuoteManagement implements \Magento\Quote\Api\CartManagementInterface
*/
private $addressesToSync = [];

/**
* @var \Magento\Framework\App\RequestInterface
*/
private $request;

/**
* @var \Magento\Framework\HTTP\PhpEnvironment\RemoteAddress
*/
private $remoteAddress;

/**
* @param EventManager $eventManager
* @param QuoteValidator $quoteValidator
Expand All @@ -169,6 +179,8 @@ class QuoteManagement implements \Magento\Quote\Api\CartManagementInterface
* @param QuoteFactory $quoteFactory
* @param \Magento\Quote\Model\QuoteIdMaskFactory|null $quoteIdMaskFactory
* @param \Magento\Customer\Api\AddressRepositoryInterface|null $addressRepository
* @param \Magento\Framework\App\RequestInterface|null $request
* @param \Magento\Framework\HTTP\PhpEnvironment\RemoteAddress $remoteAddress
* @SuppressWarnings(PHPMD.ExcessiveParameterList)
*/
public function __construct(
Expand All @@ -193,7 +205,9 @@ public function __construct(
\Magento\Customer\Api\AccountManagementInterface $accountManagement,
\Magento\Quote\Model\QuoteFactory $quoteFactory,
\Magento\Quote\Model\QuoteIdMaskFactory $quoteIdMaskFactory = null,
\Magento\Customer\Api\AddressRepositoryInterface $addressRepository = null
\Magento\Customer\Api\AddressRepositoryInterface $addressRepository = null,
\Magento\Framework\App\RequestInterface $request = null,
\Magento\Framework\HTTP\PhpEnvironment\RemoteAddress $remoteAddress = null
) {
$this->eventManager = $eventManager;
$this->quoteValidator = $quoteValidator;
Expand All @@ -219,6 +233,10 @@ public function __construct(
->get(\Magento\Quote\Model\QuoteIdMaskFactory::class);
$this->addressRepository = $addressRepository ?: ObjectManager::getInstance()
->get(\Magento\Customer\Api\AddressRepositoryInterface::class);
$this->request = $request ?: ObjectManager::getInstance()
->get(\Magento\Framework\App\RequestInterface::class);
$this->remoteAddress = $remoteAddress ?: ObjectManager::getInstance()
->get(\Magento\Framework\HTTP\PhpEnvironment\RemoteAddress::class);
}

/**
Expand Down Expand Up @@ -281,6 +299,7 @@ public function assignCustomer($cartId, $customerId, $storeId)
throw new StateException(
__("The customer can't be assigned to the cart because the customer already has an active cart.")
);
// phpcs:ignore Magento2.CodeAnalysis.EmptyBlock
} catch (\Magento\Framework\Exception\NoSuchEntityException $e) {
}

Expand Down Expand Up @@ -368,6 +387,14 @@ public function placeOrder($cartId, PaymentInterface $paymentMethod = null)
$quote->setCustomerGroupId(\Magento\Customer\Api\Data\GroupInterface::NOT_LOGGED_IN_ID);
}

$remoteAddress = $this->remoteAddress->getRemoteAddress();
if ($remoteAddress !== false) {
$quote->setRemoteIp($remoteAddress);
$quote->setXForwardedFor(
$this->request->getServer('HTTP_X_FORWARDED_FOR')
);
}

$this->eventManager->dispatch('checkout_submit_before', ['quote' => $quote]);

$order = $this->submit($quote);
Expand Down Expand Up @@ -627,12 +654,14 @@ private function rollbackAddresses(
'exception' => $e,
]
);
// phpcs:ignore Magento2.Exceptions.ThrowCatch
} catch (\Exception $consecutiveException) {
$message = sprintf(
"An exception occurred on 'sales_model_service_quote_submit_failure' event: %s",
$consecutiveException->getMessage()
);

// phpcs:ignore Magento2.Exceptions.DirectThrow
throw new \Exception($message, 0, $e);
}
}
Expand Down
78 changes: 61 additions & 17 deletions app/code/Magento/Quote/Test/Unit/Model/QuoteManagementTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,12 @@

namespace Magento\Quote\Test\Unit\Model;

use Magento\Framework\App\RequestInterface;
use Magento\Framework\Exception\NoSuchEntityException;

use Magento\Framework\HTTP\PhpEnvironment\RemoteAddress;
use Magento\Quote\Model\CustomerManagement;
use Magento\Quote\Model\QuoteIdMaskFactory;
use Magento\Sales\Api\Data\OrderAddressInterface;

/**
Expand Down Expand Up @@ -137,6 +140,21 @@ class QuoteManagementTest extends \PHPUnit\Framework\TestCase
*/
private $quoteFactoryMock;

/**
* @var \Magento\Framework\App\RequestInterface|\PHPUnit_Framework_MockObject_MockObject
*/
private $requestMock;

/**
* @var \Magento\Framework\HTTP\PhpEnvironment\RemoteAddress|\PHPUnit_Framework_MockObject_MockObject
*/
private $remoteAddressMock;

/**
* @var \Magento\Quote\Model\QuoteIdMaskFactory|\PHPUnit_Framework_MockObject_MockObject
*/
private $quoteIdMaskFactoryMock;

/**
* @SuppressWarnings(PHPMD.ExcessiveMethodLength)
*/
Expand Down Expand Up @@ -178,18 +196,20 @@ protected function setUp()
);

$this->quoteMock = $this->createPartialMock(\Magento\Quote\Model\Quote::class, [
'getId',
'getCheckoutMethod',
'setCheckoutMethod',
'setCustomerId',
'setCustomerEmail',
'getBillingAddress',
'setCustomerIsGuest',
'setCustomerGroupId',
'assignCustomer',
'getPayment',
'collectTotals'
]);
'assignCustomer',
'collectTotals',
'getBillingAddress',
'getCheckoutMethod',
'getPayment',
'setCheckoutMethod',
'setCustomerEmail',
'setCustomerGroupId',
'setCustomerId',
'setCustomerIsGuest',
'setRemoteIp',
'setXForwardedFor',
'getId',
]);

$this->quoteAddressFactory = $this->createPartialMock(
\Magento\Quote\Model\Quote\AddressFactory::class,
Expand Down Expand Up @@ -237,8 +257,11 @@ protected function setUp()

// Set the new dependency
$this->quoteIdMock = $this->createMock(\Magento\Quote\Model\QuoteIdMask::class);
$quoteIdFactoryMock = $this->createPartialMock(\Magento\Quote\Model\QuoteIdMaskFactory::class, ['create']);
$this->setPropertyValue($this->model, 'quoteIdMaskFactory', $quoteIdFactoryMock);
$this->quoteIdMaskFactoryMock = $this->createPartialMock(QuoteIdMaskFactory::class, ['create']);
$this->setPropertyValue($this->model, 'quoteIdMaskFactory', $this->quoteIdMaskFactoryMock);

$this->requestMock = $this->createPartialMockForAbstractClass(RequestInterface::class, ['getServer']);
$this->remoteAddressMock = $this->createMock(RemoteAddress::class);
}

public function testCreateEmptyCartAnonymous()
Expand Down Expand Up @@ -676,7 +699,11 @@ public function testPlaceOrderIfCustomerIsGuest()
'checkoutSession' => $this->checkoutSessionMock,
'customerSession' => $this->customerSessionMock,
'accountManagement' => $this->accountManagementMock,
'quoteFactory' => $this->quoteFactoryMock
'quoteFactory' => $this->quoteFactoryMock,
'quoteIdMaskFactory' => $this->quoteIdMaskFactoryMock,
'addressRepository' => $this->addressRepositoryMock,
'request' => $this->requestMock,
'remoteAddress' => $this->remoteAddressMock,
]
)
->getMock();
Expand Down Expand Up @@ -709,13 +736,15 @@ public function testPlaceOrder()
$orderId = 332;
$orderIncrementId = 100003332;
$orderStatus = 'status1';
$remoteAddress = '192.168.1.10';
$forwardedForIp = '192.168.1.11';

/** @var \PHPUnit_Framework_MockObject_MockObject|\Magento\Quote\Model\QuoteManagement $service */
$service = $this->getMockBuilder(\Magento\Quote\Model\QuoteManagement::class)
->setMethods(['submit'])
->setConstructorArgs(
[
'eventManager' => $this->eventManager,
'eventManager' => $this->eventManager,
'quoteValidator' => $this->quoteValidator,
'orderFactory' => $this->orderFactory,
'orderManagement' => $this->orderManagement,
Expand All @@ -734,7 +763,11 @@ public function testPlaceOrder()
'checkoutSession' => $this->checkoutSessionMock,
'customerSession' => $this->customerSessionMock,
'accountManagement' => $this->accountManagementMock,
'quoteFactory' => $this->quoteFactoryMock
'quoteFactory' => $this->quoteFactoryMock,
'quoteIdMaskFactory' => $this->quoteIdMaskFactoryMock,
'addressRepository' => $this->addressRepositoryMock,
'request' => $this->requestMock,
'remoteAddress' => $this->remoteAddressMock,
]
)
->getMock();
Expand Down Expand Up @@ -762,6 +795,17 @@ public function testPlaceOrder()
->method('setCustomerIsGuest')
->with(true);

$this->remoteAddressMock
->method('getRemoteAddress')
->willReturn($remoteAddress);

$this->requestMock
->method('getServer')
->willReturn($forwardedForIp);

$this->quoteMock->expects($this->once())->method('setRemoteIp')->with($remoteAddress);
$this->quoteMock->expects($this->once())->method('setXForwardedFor')->with($forwardedForIp);

$service->expects($this->once())->method('submit')->willReturn($orderMock);

$this->quoteMock->expects($this->atLeastOnce())->method('getId')->willReturn($cartId);
Expand Down

0 comments on commit 0ed0838

Please sign in to comment.