Skip to content

Commit 5e8cbb7

Browse files
alzotaalzota
committed
ACP2E-4223: Improved validation for rest api
1 parent e72e81e commit 5e8cbb7

9 files changed

+142
-37
lines changed

‎app/code/Magento/Checkout/Model/GuestPaymentInformationManagement.php‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use Magento\Quote\Api\CartRepositoryInterface;
1616
use Magento\Framework\Exception\CouldNotSaveException;
1717
use Magento\Quote\Model\Quote;
18+
use Magento\Quote\Model\QuoteAddressValidationService;
1819
use Psr\Log\LoggerInterface as Logger;
1920

2021
/**
@@ -81,6 +82,11 @@ class GuestPaymentInformationManagement implements \Magento\Checkout\Api\GuestPa
8182
*/
8283
private $addressComparator;
8384

85+
/**
86+
* @var QuoteAddressValidationService
87+
*/
88+
private $quoteAddressValidationService;
89+
8490
/**
8591
* @param \Magento\Quote\Api\GuestBillingAddressManagementInterface $billingAddressManagement
8692
* @param \Magento\Quote\Api\GuestPaymentMethodManagementInterface $paymentMethodManagement
@@ -92,7 +98,9 @@ class GuestPaymentInformationManagement implements \Magento\Checkout\Api\GuestPa
9298
* @param PaymentProcessingRateLimiterInterface|null $paymentsRateLimiter
9399
* @param PaymentSavingRateLimiterInterface|null $savingRateLimiter
94100
* @param AddressComparatorInterface|null $addressComparator
101+
* @param QuoteAddressValidationService|null $quoteAddressValidationService
95102
* @codeCoverageIgnore
103+
* @SuppressWarnings(PHPMD.ExcessiveParameterList)
96104
*/
97105
public function __construct(
98106
\Magento\Quote\Api\GuestBillingAddressManagementInterface $billingAddressManagement,
@@ -104,7 +112,8 @@ public function __construct(
104112
Logger $logger,
105113
?PaymentProcessingRateLimiterInterface $paymentsRateLimiter = null,
106114
?PaymentSavingRateLimiterInterface $savingRateLimiter = null,
107-
?AddressComparatorInterface $addressComparator = null
115+
?AddressComparatorInterface $addressComparator = null,
116+
?QuoteAddressValidationService $quoteAddressValidationService = null
108117
) {
109118
$this->billingAddressManagement = $billingAddressManagement;
110119
$this->paymentMethodManagement = $paymentMethodManagement;
@@ -118,6 +127,8 @@ public function __construct(
118127
?? ObjectManager::getInstance()->get(PaymentSavingRateLimiterInterface::class);
119128
$this->addressComparator = $addressComparator
120129
?? ObjectManager::getInstance()->get(AddressComparatorInterface::class);
130+
$this->quoteAddressValidationService = $quoteAddressValidationService
131+
?? ObjectManager::getInstance()->get(QuoteAddressValidationService::class);
121132
$this->logger = $logger;
122133
}
123134

@@ -185,6 +196,9 @@ public function savePaymentInformation(
185196
$quoteIdMask = $this->quoteIdMaskFactory->create()->load($cartId, 'masked_id');
186197
/** @var Quote $quote */
187198
$quote = $this->cartRepository->getActive($quoteIdMask->getQuoteId());
199+
200+
$this->quoteAddressValidationService->validateAddressesWithRules($quote, null, $billingAddress);
201+
188202
$shippingAddress = $quote->getShippingAddress();
189203
if ($this->addressComparator->isEqual($shippingAddress, $billingAddress)) {
190204
$shippingAddress->setSameAsBilling(1);

‎app/code/Magento/Checkout/Model/PaymentInformationManagement.php‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
use Magento\Quote\Api\Data\PaymentInterface;
2020
use Magento\Quote\Model\Quote;
2121
use Magento\Quote\Model\Quote\Address;
22+
use Magento\Quote\Model\QuoteAddressValidationService;
2223
use Psr\Log\LoggerInterface;
2324

2425
/**
@@ -90,6 +91,11 @@ class PaymentInformationManagement implements \Magento\Checkout\Api\PaymentInfor
9091
*/
9192
private $logger;
9293

94+
/**
95+
* @var QuoteAddressValidationService
96+
*/
97+
private $quoteAddressValidationService;
98+
9399
/**
94100
* @param \Magento\Quote\Api\BillingAddressManagementInterface $billingAddressManagement
95101
* @param \Magento\Quote\Api\PaymentMethodManagementInterface $paymentMethodManagement
@@ -102,6 +108,7 @@ class PaymentInformationManagement implements \Magento\Checkout\Api\PaymentInfor
102108
* @param AddressRepositoryInterface|null $addressRepository
103109
* @param AddressComparatorInterface|null $addressComparator
104110
* @param LoggerInterface|null $logger
111+
* @param QuoteAddressValidationService|null $quoteAddressValidationService
105112
* @codeCoverageIgnore
106113
* @SuppressWarnings(PHPMD.ExcessiveParameterList)
107114
*/
@@ -116,7 +123,8 @@ public function __construct(
116123
?CartRepositoryInterface $cartRepository = null,
117124
?AddressRepositoryInterface $addressRepository = null,
118125
?AddressComparatorInterface $addressComparator = null,
119-
?LoggerInterface $logger = null
126+
?LoggerInterface $logger = null,
127+
?QuoteAddressValidationService $quoteAddressValidationService = null
120128
) {
121129
$this->billingAddressManagement = $billingAddressManagement;
122130
$this->paymentMethodManagement = $paymentMethodManagement;
@@ -134,6 +142,8 @@ public function __construct(
134142
$this->addressComparator = $addressComparator
135143
?? ObjectManager::getInstance()->get(AddressComparatorInterface::class);
136144
$this->logger = $logger ?? ObjectManager::getInstance()->get(LoggerInterface::class);
145+
$this->quoteAddressValidationService = $quoteAddressValidationService
146+
?? ObjectManager::getInstance()->get(QuoteAddressValidationService::class);
137147
}
138148

139149
/**
@@ -205,6 +215,13 @@ public function savePaymentInformation(
205215
//It's necessary to verify the price rules with the customer data
206216
$billingAddress->setCustomerId($customerId);
207217
}
218+
219+
$this->quoteAddressValidationService->validateAddressesWithRules(
220+
$quote,
221+
null,
222+
$billingAddress
223+
);
224+
208225
$this->updateCustomerBillingAddressId($quote, $billingAddress);
209226
$quote->removeAddress($quote->getBillingAddress()->getId());
210227
$quote->setBillingAddress($billingAddress);

‎app/code/Magento/Checkout/Model/ShippingInformationManagement.php‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,11 @@ public function saveAddressInformation(
196196

197197
try {
198198
$billingAddress = $addressInformation->getBillingAddress();
199+
$this->quoteAddressValidationService->validateAddressesWithRules(
200+
$quote,
201+
$address,
202+
$billingAddress
203+
);
199204
if ($billingAddress) {
200205
$this->updateCustomerBillingAddressId($quote, $billingAddress);
201206
if (!$billingAddress->getCustomerAddressId()) {
@@ -213,8 +218,6 @@ public function saveAddressInformation(
213218

214219
$quote->setIsMultiShipping(false);
215220

216-
$this->quoteAddressValidationService->validateAddressesWithRules($address, $billingAddress);
217-
218221
$this->quoteRepository->save($quote);
219222
} catch (LocalizedException $e) {
220223
$this->logger->critical($e);

‎app/code/Magento/Checkout/Test/Unit/Model/ShippingInformationManagementTest.php‎

Lines changed: 57 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
use Magento\Checkout\Api\Data\PaymentDetailsInterface;
1111
use Magento\Checkout\Api\Data\ShippingInformationInterface;
12+
use Magento\Checkout\Model\AddressComparatorInterface;
1213
use Magento\Checkout\Model\PaymentDetailsFactory;
1314
use Magento\Checkout\Model\ShippingInformationManagement;
1415
use Magento\Framework\Exception\InputException;
@@ -27,6 +28,7 @@
2728
use Magento\Quote\Model\Quote;
2829
use Magento\Quote\Model\Quote\Address;
2930
use Magento\Quote\Model\QuoteAddressValidator;
31+
use Magento\Quote\Model\QuoteAddressValidationService;
3032
use Magento\Quote\Model\Shipping;
3133
use Magento\Quote\Model\ShippingAssignment;
3234
use Magento\Quote\Model\ShippingAssignmentFactory;
@@ -123,6 +125,16 @@ class ShippingInformationManagementTest extends TestCase
123125
*/
124126
private $addressValidatorMock;
125127

128+
/**
129+
* @var QuoteAddressValidationService|MockObject
130+
*/
131+
private $quoteAddressValidationServiceMock;
132+
133+
/**
134+
* @var AddressComparatorInterface|MockObject
135+
*/
136+
private $addressComparatorMock;
137+
126138
/**
127139
* @inheritdoc
128140
*/
@@ -185,6 +197,9 @@ protected function setUp(): void
185197
);
186198
$this->shippingFactoryMock = $this->createPartialMock(ShippingFactory::class, ['create']);
187199
$this->addressValidatorMock = $this->createMock(QuoteAddressValidator::class);
200+
$this->quoteAddressValidationServiceMock = $this->createMock(QuoteAddressValidationService::class);
201+
$this->addressComparatorMock = $this->getMockForAbstractClass(AddressComparatorInterface::class);
202+
$this->addressComparatorMock->method('isEqual')->willReturn(false);
188203

189204
$this->model = $this->objectManager->getObject(
190205
ShippingInformationManagement::class,
@@ -196,7 +211,9 @@ protected function setUp(): void
196211
'shippingAssignmentFactory' => $this->shippingAssignmentFactoryMock,
197212
'cartExtensionFactory' => $this->cartExtensionFactoryMock,
198213
'shippingFactory' => $this->shippingFactoryMock,
199-
'addressValidator' => $this->addressValidatorMock
214+
'addressValidator' => $this->addressValidatorMock,
215+
'addressComparator' => $this->addressComparatorMock,
216+
'quoteAddressValidationService' => $this->quoteAddressValidationServiceMock
200217
]
201218
);
202219
}
@@ -336,6 +353,9 @@ public function testSaveAddressInformationWithLocalizedException(): void
336353
$this->addressValidatorMock->expects($this->exactly(2))
337354
->method('validateForCart');
338355

356+
$this->quoteAddressValidationServiceMock->expects($this->once())
357+
->method('validateAddressesWithRules');
358+
339359
$this->quoteRepositoryMock->expects($this->once())
340360
->method('getActive')
341361
->with($cartId)
@@ -355,6 +375,7 @@ public function testSaveAddressInformationWithLocalizedException(): void
355375
$billingAddress->expects($this->once())
356376
->method('getCustomerAddressId')
357377
->willReturn(1);
378+
$billingAddress->method('setCustomerAddressId')->willReturnSelf();
358379

359380
$addressInformationMock->expects($this->once())
360381
->method('getBillingAddress')
@@ -377,19 +398,23 @@ public function testSaveAddressInformationWithLocalizedException(): void
377398
->method('setBillingAddress')
378399
->with($billingAddress)
379400
->willReturnSelf();
380-
$this->quoteMock->expects($this->once())
401+
402+
$quoteBillingAddressMock = $this->getMockForAbstractClass(AddressInterface::class);
403+
$quoteBillingAddressMock->method('getCustomerAddressId')->willReturn(null);
404+
$this->quoteMock->expects($this->atLeastOnce())
381405
->method('getBillingAddress')
382-
->willReturnSelf();
383-
$this->quoteMock->expects($this->once())
406+
->willReturn($quoteBillingAddressMock);
407+
408+
$this->quoteMock->expects($this->atLeastOnce())
384409
->method('getShippingAddress')
385-
->willReturnSelf();
410+
->willReturn($this->shippingAddressMock);
386411

387412
$this->quoteRepositoryMock->expects($this->once())
388413
->method('save')
389414
->with($this->quoteMock)
390415
->willThrowException($exception);
391416

392-
$this->expectException(LocalizedException::class);
417+
$this->expectException(InputException::class);
393418
$this->expectExceptionMessage(
394419
'The shipping information was unable to be saved. Error: "' . $errorMessage . '"'
395420
);
@@ -428,6 +453,8 @@ public function testSaveAddressInformationIfCanNotSaveQuote(): void
428453
->willReturn($shippingMethod);
429454

430455
$billingAddress = $this->getMockForAbstractClass(AddressInterface::class);
456+
$billingAddress->method('getCustomerAddressId')->willReturn(null);
457+
$billingAddress->method('setCustomerAddressId')->willReturnSelf();
431458
$addressInformationMock->expects($this->once())
432459
->method('getBillingAddress')
433460
->willReturn($billingAddress);
@@ -450,12 +477,13 @@ public function testSaveAddressInformationIfCanNotSaveQuote(): void
450477
->willReturnSelf();
451478

452479
$quoteBillingAddress = $this->createMock(Address::class);
453-
$this->quoteMock->expects($this->once())
480+
$quoteBillingAddress->method('getCustomerAddressId')->willReturn(null);
481+
$this->quoteMock->expects($this->atLeastOnce())
454482
->method('getBillingAddress')
455483
->willReturn($quoteBillingAddress);
456484

457485
$quoteShippingAddress = $this->createMock(Address::class);
458-
$this->quoteMock->expects($this->once())
486+
$this->quoteMock->expects($this->atLeastOnce())
459487
->method('getShippingAddress')
460488
->willReturn($quoteShippingAddress);
461489

@@ -505,6 +533,7 @@ public function testSaveAddressInformationIfCarrierCodeIsInvalid(): void
505533
$billingAddress->expects($this->once())
506534
->method('getCustomerAddressId')
507535
->willReturn(1);
536+
$billingAddress->method('setCustomerAddressId')->willReturnSelf();
508537

509538
$addressInformationMock->expects($this->once())
510539
->method('getBillingAddress')
@@ -518,9 +547,12 @@ public function testSaveAddressInformationIfCarrierCodeIsInvalid(): void
518547
$this->quoteMock->expects($this->once())
519548
->method('getItemsCount')
520549
->willReturn(self::STUB_ITEMS_COUNT);
521-
$this->quoteMock->expects($this->once())
550+
551+
$quoteBillingAddressMock = $this->getMockForAbstractClass(AddressInterface::class);
552+
$quoteBillingAddressMock->method('getCustomerAddressId')->willReturn(null);
553+
$this->quoteMock->expects($this->atLeastOnce())
522554
->method('getBillingAddress')
523-
->willReturnSelf();
555+
->willReturn($quoteBillingAddressMock);
524556

525557
$this->quoteMock->expects($this->once())
526558
->method('setIsMultiShipping')
@@ -530,7 +562,8 @@ public function testSaveAddressInformationIfCarrierCodeIsInvalid(): void
530562
->method('setBillingAddress')
531563
->with($billingAddress)
532564
->willReturnSelf();
533-
$this->quoteMock->expects($this->exactly(2))
565+
566+
$this->quoteMock->expects($this->atLeastOnce())
534567
->method('getShippingAddress')
535568
->willReturn($this->shippingAddressMock);
536569

@@ -558,6 +591,7 @@ public function testSaveAddressInformationIfCarrierCodeIsInvalid(): void
558591
* Save address info test.
559592
*
560593
* @return void
594+
* @SuppressWarnings(PHPMD.ExcessiveMethodLength)
561595
*/
562596
public function testSaveAddressInformation(): void
563597
{
@@ -585,6 +619,8 @@ public function testSaveAddressInformation(): void
585619
->willReturn($shippingMethod);
586620

587621
$billingAddress = $this->getMockForAbstractClass(AddressInterface::class);
622+
$billingAddress->method('getCustomerAddressId')->willReturn(null);
623+
$billingAddress->method('setCustomerAddressId')->willReturnSelf();
588624
$addressInformationMock->expects($this->once())
589625
->method('getBillingAddress')
590626
->willReturn($billingAddress);
@@ -597,9 +633,12 @@ public function testSaveAddressInformation(): void
597633
$this->quoteMock->expects($this->once())
598634
->method('getItemsCount')
599635
->willReturn(self::STUB_ITEMS_COUNT);
600-
$this->quoteMock->expects($this->once())
636+
637+
$quoteBillingAddressMock = $this->getMockForAbstractClass(AddressInterface::class);
638+
$quoteBillingAddressMock->method('getCustomerAddressId')->willReturn(null);
639+
$this->quoteMock->expects($this->atLeastOnce())
601640
->method('getBillingAddress')
602-
->willReturnSelf();
641+
->willReturn($quoteBillingAddressMock);
603642

604643
$this->quoteMock->expects($this->once())
605644
->method('setIsMultiShipping')
@@ -609,7 +648,7 @@ public function testSaveAddressInformation(): void
609648
->method('setBillingAddress')
610649
->with($billingAddress)
611650
->willReturnSelf();
612-
$this->quoteMock->expects($this->exactly(2))
651+
$this->quoteMock->expects($this->atLeastOnce())
613652
->method('getShippingAddress')
614653
->willReturn($this->shippingAddressMock);
615654

@@ -664,10 +703,11 @@ public function testSaveAddressInformation(): void
664703
private function getCartExtensionMock(): MockObject
665704
{
666705
$mockBuilder = $this->getMockBuilder(CartExtension::class);
667-
try {
706+
707+
if (method_exists(CartExtension::class, 'getShippingAssignments')) {
708+
$mockBuilder->onlyMethods(['getShippingAssignments', 'setShippingAssignments']);
709+
} else {
668710
$mockBuilder->addMethods(['getShippingAssignments', 'setShippingAssignments']);
669-
} catch (RuntimeException $e) {
670-
// CartExtension already generated.
671711
}
672712

673713
return $mockBuilder->getMock();

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,11 +127,12 @@ public function assign($cartId, AddressInterface $address, $useForShipping = fal
127127
private function assignBillingAddress(AddressInterface $address, Quote $quote, bool $useForShipping = false)
128128
{
129129
$address->setCustomerId($quote->getCustomerId());
130+
131+
$this->quoteAddressValidationService->validateAddressesWithRules($quote, null, $address);
132+
130133
$quote->removeAddress($quote->getBillingAddress()->getId());
131134
$quote->setBillingAddress($address);
132135

133-
$this->quoteAddressValidationService->validateAddressesWithRules(null, $address);
134-
135136
try {
136137
$this->getShippingAddressAssignment()->setAddress($quote, $address, $useForShipping);
137138
$quote->setDataChanges(true);

0 commit comments

Comments
 (0)