diff --git a/app/code/Magento/Braintree/Observer/DataAssignObserver.php b/app/code/Magento/Braintree/Observer/DataAssignObserver.php index 512ea25c103b6..fb528e3554d23 100644 --- a/app/code/Magento/Braintree/Observer/DataAssignObserver.php +++ b/app/code/Magento/Braintree/Observer/DataAssignObserver.php @@ -7,6 +7,7 @@ use Magento\Framework\Event\Observer; use Magento\Payment\Observer\AbstractDataAssignObserver; +use Magento\Quote\Api\Data\PaymentInterface; /** * Class DataAssignObserver @@ -31,13 +32,19 @@ class DataAssignObserver extends AbstractDataAssignObserver public function execute(Observer $observer) { $data = $this->readDataArgument($observer); + + $additionalData = $data->getData(PaymentInterface::KEY_ADDITIONAL_DATA); + if (!is_array($additionalData)) { + return; + } + $paymentInfo = $this->readPaymentModelArgument($observer); foreach ($this->additionalInformationList as $additionalInformationKey) { - if ($data->getDataByKey($additionalInformationKey) !== null) { + if (isset($additionalData[$additionalInformationKey])) { $paymentInfo->setAdditionalInformation( $additionalInformationKey, - $data->getDataByKey($additionalInformationKey) + $additionalData[$additionalInformationKey] ); } } diff --git a/app/code/Magento/Braintree/Test/Unit/Observer/DataAssignObserverTest.php b/app/code/Magento/Braintree/Test/Unit/Observer/DataAssignObserverTest.php index 92e6820f44778..c14227d73d7bb 100644 --- a/app/code/Magento/Braintree/Test/Unit/Observer/DataAssignObserverTest.php +++ b/app/code/Magento/Braintree/Test/Unit/Observer/DataAssignObserverTest.php @@ -10,6 +10,7 @@ use Magento\Payment\Model\InfoInterface; use Magento\Payment\Observer\AbstractDataAssignObserver; use Magento\Braintree\Observer\DataAssignObserver; +use Magento\Quote\Api\Data\PaymentInterface; /** * Class DataAssignObserverTest @@ -30,8 +31,10 @@ public function testExecute() $paymentInfoModel = $this->getMock(InfoInterface::class); $dataObject = new DataObject( [ - 'payment_method_nonce' => self::PAYMENT_METHOD_NONCE, - 'device_data' => self::DEVICE_DATA, + PaymentInterface::KEY_ADDITIONAL_DATA => [ + 'payment_method_nonce' => self::PAYMENT_METHOD_NONCE, + 'device_data' => self::DEVICE_DATA + ] ] ); $observerContainer->expects(static::atLeastOnce()) diff --git a/app/code/Magento/Catalog/Ui/DataProvider/Product/Form/Modifier/AdvancedPricing.php b/app/code/Magento/Catalog/Ui/DataProvider/Product/Form/Modifier/AdvancedPricing.php index 3539c33ec34a9..06c55a765ad3e 100644 --- a/app/code/Magento/Catalog/Ui/DataProvider/Product/Form/Modifier/AdvancedPricing.php +++ b/app/code/Magento/Catalog/Ui/DataProvider/Product/Form/Modifier/AdvancedPricing.php @@ -419,6 +419,7 @@ protected function getTierPriceStructure($tierPricePath) 'dndConfig' => [ 'enabled' => false, ], + 'disabled' => false, 'sortOrder' => $this->arrayManager->get($tierPricePath . '/arguments/data/config/sortOrder', $this->meta), ], diff --git a/app/code/Magento/Checkout/Model/Cart.php b/app/code/Magento/Checkout/Model/Cart.php index 80cdf61499197..4ce23c9c7f709 100644 --- a/app/code/Magento/Checkout/Model/Cart.php +++ b/app/code/Magento/Checkout/Model/Cart.php @@ -257,7 +257,11 @@ public function addOrderItem($orderItem, $qtyFlag = null) if ($orderItem->getParentItem() === null) { $storeId = $this->_storeManager->getStore()->getId(); try { - $product = $this->productRepository->getById($orderItem->getProductId(), false, $storeId); + /** + * We need to reload product in this place, because products + * with the same id may have different sets of order attributes. + */ + $product = $this->productRepository->getById($orderItem->getProductId(), false, $storeId, true); } catch (NoSuchEntityException $e) { return $this; } diff --git a/app/code/Magento/Customer/view/frontend/web/js/password-strength-indicator.js b/app/code/Magento/Customer/view/frontend/web/js/password-strength-indicator.js index 315827211f1ed..e08ccee32e402 100644 --- a/app/code/Magento/Customer/view/frontend/web/js/password-strength-indicator.js +++ b/app/code/Magento/Customer/view/frontend/web/js/password-strength-indicator.js @@ -6,11 +6,13 @@ /** * jshint browser:true */ +/*eslint no-unused-vars: 0*/ define([ 'jquery', + 'Magento_Customer/js/zxcvbn', 'mage/translate', - 'Magento_Customer/js/zxcvbn' -], function ($, $t, zxcvbn) { + 'mage/validation' +], function ($, zxcvbn, $t, validation) { 'use strict'; $.widget('mage.passwordStrengthIndicator', { @@ -50,6 +52,8 @@ define([ className = this._getClassName(score); this._displayStrength(className, score); + //update error messages + $.validator.validateSingleElement(this.element.find('input[type="password"]')); }, /** diff --git a/app/code/Magento/ImportExport/Model/Export.php b/app/code/Magento/ImportExport/Model/Export.php index 4a9ba34f4dc77..2adfe4cb9a532 100644 --- a/app/code/Magento/ImportExport/Model/Export.php +++ b/app/code/Magento/ImportExport/Model/Export.php @@ -238,6 +238,9 @@ public static function getStaticAttributeFilterType(\Magento\Eav\Model\Entity\At return self::FILTER_TYPE_INPUT; } $columns = $attribute->getFlatColumns(); + if (empty($columns)) { + return self::FILTER_TYPE_INPUT; + } switch ($columns[$attribute->getAttributeCode()]['type']) { case \Magento\Framework\DB\Ddl\Table::TYPE_INTEGER: case \Magento\Framework\DB\Ddl\Table::TYPE_BIGINT: diff --git a/app/code/Magento/OfflinePayments/Model/Purchaseorder.php b/app/code/Magento/OfflinePayments/Model/Purchaseorder.php index b209af925a9db..52eee8df4ba78 100644 --- a/app/code/Magento/OfflinePayments/Model/Purchaseorder.php +++ b/app/code/Magento/OfflinePayments/Model/Purchaseorder.php @@ -47,10 +47,6 @@ class Purchaseorder extends \Magento\Payment\Model\Method\AbstractMethod */ public function assignData(\Magento\Framework\DataObject $data) { - if (!$data instanceof \Magento\Framework\DataObject) { - $data = new \Magento\Framework\DataObject($data); - } - $this->getInfoInstance()->setPoNumber($data->getPoNumber()); return $this; } diff --git a/app/code/Magento/Payment/Model/Info.php b/app/code/Magento/Payment/Model/Info.php index c98f2c98f3983..6f61ccdd02cad 100644 --- a/app/code/Magento/Payment/Model/Info.php +++ b/app/code/Magento/Payment/Model/Info.php @@ -220,9 +220,6 @@ protected function _initAdditionalInformation() { $additionalInfo = $this->_getData('additional_information'); if (empty($this->_additionalInformation) && $additionalInfo) { - if (!is_array($additionalInfo)) { - $additionalInfo = unserialize($additionalInfo); - } $this->_additionalInformation = $additionalInfo; } } diff --git a/app/code/Magento/Payment/Model/Method/AbstractMethod.php b/app/code/Magento/Payment/Model/Method/AbstractMethod.php index 57128e37dde9b..24b543ca02d62 100644 --- a/app/code/Magento/Payment/Model/Method/AbstractMethod.php +++ b/app/code/Magento/Payment/Model/Method/AbstractMethod.php @@ -21,6 +21,7 @@ * @SuppressWarnings(PHPMD.ExcessivePublicCount) * @SuppressWarnings(PHPMD.TooManyFields) * @SuppressWarnings(PHPMD.CouplingBetweenObjects) + * @deprecated */ abstract class AbstractMethod extends \Magento\Framework\Model\AbstractExtensibleModel implements MethodInterface, @@ -769,12 +770,6 @@ public function getConfigData($field, $storeId = null) */ public function assignData(\Magento\Framework\DataObject $data) { - if (is_array($data)) { - $this->getInfoInstance()->addData($data); - } elseif ($data instanceof \Magento\Framework\DataObject) { - $this->getInfoInstance()->addData($data->getData()); - } - $this->_eventManager->dispatch( 'payment_method_assign_data_' . $this->getCode(), [ diff --git a/app/code/Magento/Payment/Test/Unit/Model/InfoTest.php b/app/code/Magento/Payment/Test/Unit/Model/InfoTest.php index 22ebf692706b3..72a6e3eda5507 100644 --- a/app/code/Magento/Payment/Test/Unit/Model/InfoTest.php +++ b/app/code/Magento/Payment/Test/Unit/Model/InfoTest.php @@ -90,7 +90,6 @@ public function ccKeysDataProvider() ]; } - public function testGetMethodInstanceWithRealMethod() { $method = 'real_method'; @@ -108,7 +107,6 @@ public function testGetMethodInstanceWithRealMethod() $this->info->getMethodInstance(); } - public function testGetMethodInstanceWithUnrealMethod() { $method = 'unreal_method'; @@ -131,7 +129,6 @@ public function testGetMethodInstanceWithUnrealMethod() $this->info->getMethodInstance(); } - /** * @expectedException \Magento\Framework\Exception\LocalizedException * @expectedExceptionMessage The payment method you requested is not available. @@ -141,8 +138,7 @@ public function testGetMethodInstanceWithNoMethod() $this->info->setData('method', false); $this->info->getMethodInstance(); } - - + public function testGetMethodInstanceRequestedMethod() { $code = 'real_method'; @@ -251,9 +247,9 @@ public function testHasAdditionalInformation() public function testInitAdditionalInformationWithUnserialize() { - $data = serialize(['key1' => 'data1', 'key2' => 'data2']); + $data = ['key1' => 'data1', 'key2' => 'data2']; $this->info->setData('additional_information', $data); - $this->assertEquals(unserialize($data), $this->info->getAdditionalInformation()); + $this->assertEquals($data, $this->info->getAdditionalInformation()); } } diff --git a/app/code/Magento/Payment/Test/Unit/Model/Method/AbstractMethodTest.php b/app/code/Magento/Payment/Test/Unit/Model/Method/AbstractMethodTest.php index 1504aff5308f8..22c35df17e897 100644 --- a/app/code/Magento/Payment/Test/Unit/Model/Method/AbstractMethodTest.php +++ b/app/code/Magento/Payment/Test/Unit/Model/Method/AbstractMethodTest.php @@ -5,6 +5,9 @@ */ namespace Magento\Payment\Test\Unit\Model\Method; +use Magento\Framework\DataObject; +use Magento\Payment\Model\InfoInterface; +use Magento\Payment\Observer\AbstractDataAssignObserver; use Magento\Store\Model\ScopeInterface; use Magento\Payment\Test\Unit\Model\Method\AbstractMethod\Stub; @@ -65,7 +68,7 @@ protected function setUp() $helper = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this); $this->payment = $helper->getObject( - 'Magento\Payment\Test\Unit\Model\Method\AbstractMethod\Stub', + Stub::class, [ 'scopeConfig' => $this->scopeConfigMock, 'context' => $contextMock, @@ -114,6 +117,37 @@ public function testIsAvailable($result) $this->assertEquals($result, $this->payment->isAvailable($this->quoteMock)); } + public function testAssignData() + { + $data = new DataObject(); + $paymentInfo = $this->getMock(InfoInterface::class); + + $this->payment->setInfoInstance($paymentInfo); + + $eventData = [ + AbstractDataAssignObserver::METHOD_CODE => $this, + AbstractDataAssignObserver::MODEL_CODE => $paymentInfo, + AbstractDataAssignObserver::DATA_CODE => $data + ]; + + $this->eventManagerMock->expects(static::exactly(2)) + ->method('dispatch') + ->willReturnMap( + [ + [ + 'payment_method_assign_data_' . Stub::STUB_CODE, + $eventData + ], + [ + 'payment_method_assign_data', + $eventData + ] + ] + ); + + $this->payment->assignData($data); + } + /** * @return array */ diff --git a/app/code/Magento/Paypal/Model/Express.php b/app/code/Magento/Paypal/Model/Express.php index a10a2654bf529..80c9ef0625d96 100644 --- a/app/code/Magento/Paypal/Model/Express.php +++ b/app/code/Magento/Paypal/Model/Express.php @@ -8,6 +8,7 @@ use Magento\Paypal\Model\Api\Nvp; use Magento\Paypal\Model\Api\ProcessableException as ApiProcessableException; use Magento\Paypal\Model\Express\Checkout as ExpressCheckout; +use Magento\Quote\Api\Data\PaymentInterface; use Magento\Sales\Api\Data\OrderPaymentInterface; use Magento\Sales\Model\Order\Payment; use Magento\Sales\Model\Order\Payment\Transaction; @@ -667,14 +668,24 @@ public function getApi() */ public function assignData(\Magento\Framework\DataObject $data) { - $result = parent::assignData($data); - $key = ExpressCheckout::PAYMENT_INFO_TRANSPORT_BILLING_AGREEMENT; - if (is_array($data)) { - $this->getInfoInstance()->setAdditionalInformation($key, isset($data[$key]) ? $data[$key] : null); - } elseif ($data instanceof \Magento\Framework\DataObject) { - $this->getInfoInstance()->setAdditionalInformation($key, $data->getData($key)); + parent::assignData($data); + + $additionalData = $data->getData(PaymentInterface::KEY_ADDITIONAL_DATA); + + if ( + !is_array($additionalData) + || !isset($additionalData[ExpressCheckout::PAYMENT_INFO_TRANSPORT_BILLING_AGREEMENT]) + ) { + return $this; } - return $result; + + $this->getInfoInstance() + ->setAdditionalInformation( + ExpressCheckout::PAYMENT_INFO_TRANSPORT_BILLING_AGREEMENT, + $additionalData[ExpressCheckout::PAYMENT_INFO_TRANSPORT_BILLING_AGREEMENT] + ); + + return $this; } /** diff --git a/app/code/Magento/Paypal/Model/Payment/Method/Billing/AbstractAgreement.php b/app/code/Magento/Paypal/Model/Payment/Method/Billing/AbstractAgreement.php index f6f6154012a77..42894ddeec49d 100644 --- a/app/code/Magento/Paypal/Model/Payment/Method/Billing/AbstractAgreement.php +++ b/app/code/Magento/Paypal/Model/Payment/Method/Billing/AbstractAgreement.php @@ -5,6 +5,9 @@ */ namespace Magento\Paypal\Model\Payment\Method\Billing; +use Magento\Paypal\Model\Billing\Agreement; +use Magento\Quote\Api\Data\PaymentInterface; + /** * Billing Agreement Payment Method Abstract model * @@ -110,34 +113,35 @@ public function isAvailable(\Magento\Quote\Api\Data\CartInterface $quote = null) */ public function assignData(\Magento\Framework\DataObject $data) { - $result = parent::assignData($data); - - $key = self::TRANSPORT_BILLING_AGREEMENT_ID; - $id = false; - if (is_array($data) && isset($data[$key])) { - $id = $data[$key]; - } elseif ($data instanceof \Magento\Framework\DataObject && $data->getData($key)) { - $id = $data->getData($key); + parent::assignData($data); + + $additionalData = $data->getData(PaymentInterface::KEY_ADDITIONAL_DATA); + + if (!is_array($additionalData) || !isset($additionalData[self::TRANSPORT_BILLING_AGREEMENT_ID])) { + return $this; } - if ($id) { - $info = $this->getInfoInstance(); - $ba = $this->_agreementFactory->create()->load($id); - if ($ba->getId() && $ba->getCustomerId() == $info->getQuote()->getCustomerId()) { - $info->setAdditionalInformation( - $key, - $id - )->setAdditionalInformation( - self::PAYMENT_INFO_REFERENCE_ID, - $ba->getReferenceId() - ); - } + + $id = $additionalData[self::TRANSPORT_BILLING_AGREEMENT_ID]; + if (!$id || !is_numeric($id)) { + return $this; } - return $result; + + $info = $this->getInfoInstance(); + /** @var Agreement $ba */ + $ba = $this->_agreementFactory->create(); + $ba->load($id); + + if ($ba->getId() && $ba->getCustomerId() == $info->getQuote()->getCustomerId()) { + $info->setAdditionalInformation(self::TRANSPORT_BILLING_AGREEMENT_ID, $id); + $info->setAdditionalInformation(self::PAYMENT_INFO_REFERENCE_ID, $ba->getReferenceId()); + } + + return $this; } /** * @param object $quote - * @return void + * @return bool */ abstract protected function _isAvailable($quote); } diff --git a/app/code/Magento/Paypal/Observer/PayflowProAddCcData.php b/app/code/Magento/Paypal/Observer/PayflowProAddCcData.php index 857322608e0d3..92980e46ed8d7 100644 --- a/app/code/Magento/Paypal/Observer/PayflowProAddCcData.php +++ b/app/code/Magento/Paypal/Observer/PayflowProAddCcData.php @@ -8,6 +8,7 @@ use Magento\Framework\Event\Observer; use Magento\Payment\Observer\AbstractDataAssignObserver; use Magento\Paypal\Model\Payflow\Transparent; +use Magento\Quote\Api\Data\PaymentInterface; class PayflowProAddCcData extends AbstractDataAssignObserver { @@ -29,7 +30,13 @@ public function execute(\Magento\Framework\Event\Observer $observer) { $dataObject = $this->readDataArgument($observer); - $ccData = array_intersect_key($dataObject->getData(), array_flip($this->ccKeys)); + $additionalData = $dataObject->getData(PaymentInterface::KEY_ADDITIONAL_DATA); + + if (!is_array($additionalData)) { + return; + } + + $ccData = array_intersect_key($additionalData, array_flip($this->ccKeys)); if (count($ccData) !== count($this->ccKeys)) { return; } @@ -39,6 +46,11 @@ public function execute(\Magento\Framework\Event\Observer $observer) Transparent::CC_DETAILS, $this->sortCcData($ccData) ); + + // CC data should be stored explicitly + foreach ($ccData as $ccKey => $ccValue) { + $paymentModel->setData($ccKey, $ccValue); + } } /** diff --git a/app/code/Magento/Paypal/Test/Unit/Model/ExpressTest.php b/app/code/Magento/Paypal/Test/Unit/Model/ExpressTest.php index 75f4c14d8517e..6abf963cf7b26 100644 --- a/app/code/Magento/Paypal/Test/Unit/Model/ExpressTest.php +++ b/app/code/Magento/Paypal/Test/Unit/Model/ExpressTest.php @@ -5,7 +5,13 @@ */ namespace Magento\Paypal\Test\Unit\Model; +use Magento\Framework\DataObject; +use Magento\Framework\Event\ManagerInterface; +use Magento\Payment\Model\InfoInterface; +use Magento\Payment\Observer\AbstractDataAssignObserver; use Magento\Paypal\Model\Api\ProcessableException as ApiProcessableException; +use Magento\Paypal\Model\Express; +use Magento\Quote\Api\Data\PaymentInterface; class ExpressTest extends \PHPUnit_Framework_TestCase { @@ -55,6 +61,11 @@ class ExpressTest extends \PHPUnit_Framework_TestCase */ protected $transactionBuilder; + /** + * @var ManagerInterface|\PHPUnit_Framework_MockObject_MockObject + */ + private $eventManagerMock; + protected function setUp() { $this->_checkoutSession = $this->getMock( @@ -79,24 +90,28 @@ protected function setUp() false ); $this->_pro = $this->getMock( - 'Magento\Paypal\Model\ProFactory', - ['create', 'setMethod', 'getApi', 'importPaymentInfo', 'resetApi'], + 'Magento\Paypal\Model\Pro', + ['setMethod', 'getApi', 'importPaymentInfo', 'resetApi'], [], '', false ); - $this->_pro->expects($this->any())->method('create')->will($this->returnSelf()); + $this->eventManagerMock = $this->getMockBuilder(ManagerInterface::class) + ->setMethods(['dispatch']) + ->getMockForAbstractClass(); + + $this->_pro->expects($this->any())->method('getApi')->will($this->returnValue($this->_nvp)); $this->_helper = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this); } public function testSetApiProcessableErrors() { $this->_nvp->expects($this->once())->method('setProcessableErrors')->with($this->errorCodes); - $this->_pro->expects($this->any())->method('getApi')->will($this->returnValue($this->_nvp)); + $this->_model = $this->_helper->getObject( 'Magento\Paypal\Model\Express', [ - 'proFactory' => $this->_pro, + 'data' => [$this->_pro], 'checkoutSession' => $this->_checkoutSession, 'transactionBuilder' => $this->transactionBuilder ] @@ -110,7 +125,7 @@ public function testOrder() $this->_nvp->expects($this->any())->method('setCurrencyCode')->will($this->returnSelf()); $this->_nvp->expects($this->any())->method('setTransactionId')->will($this->returnSelf()); $this->_nvp->expects($this->any())->method('callDoAuthorization')->will($this->returnSelf()); - $this->_pro->expects($this->any())->method('getApi')->will($this->returnValue($this->_nvp)); + $this->_checkoutSession->expects($this->once())->method('getPaypalTransactionData')->will( $this->returnValue([]) ); @@ -148,11 +163,75 @@ public function testOrder() $this->_model = $this->_helper->getObject( 'Magento\Paypal\Model\Express', [ - 'proFactory' => $this->_pro, + 'data' => [$this->_pro], 'checkoutSession' => $this->_checkoutSession, 'transactionBuilder' => $this->transactionBuilder ] ); $this->assertEquals($this->_model, $this->_model->order($paymentModel, 12.3)); } + + public function testAssignData() + { + $transportValue = 'something'; + + $data = new DataObject( + [ + PaymentInterface::KEY_ADDITIONAL_DATA => [ + Express\Checkout::PAYMENT_INFO_TRANSPORT_BILLING_AGREEMENT => $transportValue + ] + ] + ); + + $this->_model = $this->_helper->getObject( + 'Magento\Paypal\Model\Express', + [ + 'data' => [$this->_pro], + 'checkoutSession' => $this->_checkoutSession, + 'transactionBuilder' => $this->transactionBuilder, + 'eventDispatcher' => $this->eventManagerMock, + ] + ); + + $paymentInfo = $this->getMock(InfoInterface::class); + $this->_model->setInfoInstance($paymentInfo); + + $this->parentAssignDataExpectation($data); + + $paymentInfo->expects(static::once()) + ->method('setAdditionalInformation') + ->with( + Express\Checkout::PAYMENT_INFO_TRANSPORT_BILLING_AGREEMENT, + $transportValue + ); + + $this->_model->assignData($data); + } + + /** + * @param DataObject $data + */ + private function parentAssignDataExpectation(DataObject $data) + { + $eventData = [ + AbstractDataAssignObserver::METHOD_CODE => $this, + AbstractDataAssignObserver::MODEL_CODE => $this->_model->getInfoInstance(), + AbstractDataAssignObserver::DATA_CODE => $data + ]; + + $this->eventManagerMock->expects(static::exactly(2)) + ->method('dispatch') + ->willReturnMap( + [ + [ + 'payment_method_assign_data_' . $this->_model->getCode(), + $eventData + ], + [ + 'payment_method_assign_data', + $eventData + ] + ] + ); + } } diff --git a/app/code/Magento/Paypal/Test/Unit/Model/Payment/Method/Billing/AbstractAgreementStub.php b/app/code/Magento/Paypal/Test/Unit/Model/Payment/Method/Billing/AbstractAgreementStub.php new file mode 100644 index 0000000000000..c0eba7bb1807f --- /dev/null +++ b/app/code/Magento/Paypal/Test/Unit/Model/Payment/Method/Billing/AbstractAgreementStub.php @@ -0,0 +1,30 @@ +eventManagerMock = $this->getMockBuilder(ManagerInterface::class) + ->setMethods(['dispatch']) + ->getMockForAbstractClass(); + + $this->agreementFactory = $this->getMockBuilder('Magento\Paypal\Model\Billing\AgreementFactory') + ->disableOriginalConstructor() + ->setMethods(['create']) + ->getMock(); + + $this->payment = $helper->getObject( + AbstractAgreementStub::class, + [ + 'eventDispatcher' => $this->eventManagerMock, + 'agreementFactory' => $this->agreementFactory + ] + ); + } + + public function testAssignData() + { + $baId = '1678235'; + $customerId = 67; + $referenceId = '1234124'; + + $data = new DataObject( + [ + PaymentInterface::KEY_ADDITIONAL_DATA => [ + AbstractAgreement::TRANSPORT_BILLING_AGREEMENT_ID => $baId + ] + ] + ); + $paymentInfo = $this->getMockBuilder(Payment::class) + ->disableOriginalConstructor() + ->getMock(); + $quote = $this->getMockBuilder(Quote::class) + ->disableOriginalConstructor() + ->setMethods(['__wakeup', 'getCustomerId']) + ->getMock(); + + $this->payment->setInfoInstance($paymentInfo); + $this->parentAssignDataExpectation($data); + + $agreementModel = $this->getMockBuilder(Agreement::class) + ->disableOriginalConstructor() + ->setMethods(['__wakeup', 'load', 'getCustomerId', 'getId', 'getReferenceId']) + ->getMock(); + + $this->agreementFactory->expects(static::once()) + ->method('create') + ->willReturn($agreementModel); + + $paymentInfo->expects(static::once()) + ->method('getQuote') + ->willReturn($quote); + + $agreementModel->expects(static::once()) + ->method('load') + ->with($baId); + $agreementModel->expects(static::once()) + ->method('getId') + ->willReturn($baId); + $agreementModel->expects(static::atLeastOnce()) + ->method('getCustomerId') + ->willReturn($customerId); + $agreementModel->expects(static::atLeastOnce()) + ->method('getReferenceId') + ->willReturn($referenceId); + + $quote->expects(static::once()) + ->method('getCustomerId') + ->willReturn($customerId); + + $paymentInfo->expects(static::exactly(2)) + ->method('setAdditionalInformation') + ->willReturnMap( + [ + AbstractAgreement::TRANSPORT_BILLING_AGREEMENT_ID, $baId, + AbstractAgreement::PAYMENT_INFO_REFERENCE_ID, $referenceId + ] + ); + + $this->payment->assignData($data); + } + + /** + * @param DataObject $data + * @throws \Magento\Framework\Exception\LocalizedException + */ + private function parentAssignDataExpectation(DataObject $data) + { + $eventData = [ + AbstractDataAssignObserver::METHOD_CODE => $this, + AbstractDataAssignObserver::MODEL_CODE => $this->payment->getInfoInstance(), + AbstractDataAssignObserver::DATA_CODE => $data + ]; + + $this->eventManagerMock->expects(static::exactly(2)) + ->method('dispatch') + ->willReturnMap( + [ + [ + 'payment_method_assign_data_' . AbstractAgreementStub::STUB_CODE, + $eventData + ], + [ + 'payment_method_assign_data', + $eventData + ] + ] + ); + } +} diff --git a/app/code/Magento/Quote/Model/PaymentMethodManagement.php b/app/code/Magento/Quote/Model/PaymentMethodManagement.php index 63d3fc166467e..6bff056cd034a 100644 --- a/app/code/Magento/Quote/Model/PaymentMethodManagement.php +++ b/app/code/Magento/Quote/Model/PaymentMethodManagement.php @@ -61,10 +61,6 @@ public function set($cartId, \Magento\Quote\Api\Data\PaymentInterface $method) $payment = $quote->getPayment(); $data = $method->getData(); - if (isset($data['additional_data'])) { - $data = array_merge($data, (array)$data['additional_data']); - unset($data['additional_data']); - } $payment->importData($data); if ($quote->isVirtual()) { diff --git a/app/code/Magento/Quote/Model/Quote/Payment.php b/app/code/Magento/Quote/Model/Quote/Payment.php index 0efdf0620e66a..cdd295114c3d5 100644 --- a/app/code/Magento/Quote/Model/Quote/Payment.php +++ b/app/code/Magento/Quote/Model/Quote/Payment.php @@ -5,6 +5,8 @@ */ namespace Magento\Quote\Model\Quote; +use Magento\Quote\Api\Data\PaymentInterface; + /** * Quote payment information * @@ -34,7 +36,7 @@ * @author Magento Core Team * @SuppressWarnings(PHPMD.CouplingBetweenObjects) */ -class Payment extends \Magento\Payment\Model\Info implements \Magento\Quote\Api\Data\PaymentInterface +class Payment extends \Magento\Payment\Model\Info implements PaymentInterface { /** * @var string @@ -167,6 +169,7 @@ public function importData(array $data) } $method->assignData($data); + /* * validating the payment data */ diff --git a/app/code/Magento/Quote/Model/QuoteManagement.php b/app/code/Magento/Quote/Model/QuoteManagement.php index 23c3db6f558a0..2325ccaad612a 100644 --- a/app/code/Magento/Quote/Model/QuoteManagement.php +++ b/app/code/Magento/Quote/Model/QuoteManagement.php @@ -334,10 +334,6 @@ public function placeOrder($cartId, PaymentInterface $paymentMethod = null) $quote->getPayment()->setQuote($quote); $data = $paymentMethod->getData(); - if (isset($data['additional_data'])) { - $data = array_merge($data, (array)$data['additional_data']); - unset($data['additional_data']); - } $quote->getPayment()->importData($data); } diff --git a/app/code/Magento/Quote/Test/Unit/Model/PaymentMethodManagementTest.php b/app/code/Magento/Quote/Test/Unit/Model/PaymentMethodManagementTest.php index d4bea1326812f..60d7618413676 100644 --- a/app/code/Magento/Quote/Test/Unit/Model/PaymentMethodManagementTest.php +++ b/app/code/Magento/Quote/Test/Unit/Model/PaymentMethodManagementTest.php @@ -16,6 +16,7 @@ class PaymentMethodManagementTest extends \PHPUnit_Framework_TestCase * @var \Magento\Framework\TestFramework\Unit\Helper\ObjectManager */ protected $objectManager; + /** * @var \PHPUnit_Framework_MockObject_MockObject */ @@ -111,7 +112,7 @@ public function testSetVirtualProduct() $cartId = 100; $paymentId = 200; $methodDataWithAdditionalData = ['method' => 'data', 'additional_data' => ['additional' => 'value']]; - $methodData = ['method' => 'data', 'additional' => 'value']; + $methodData = $methodDataWithAdditionalData; $paymentMethod = 'checkmo'; $quoteMock = $this->getMock( diff --git a/app/code/Magento/Sales/Model/Order/Payment/Info.php b/app/code/Magento/Sales/Model/Order/Payment/Info.php index 45289510a9695..4e5dfee783aa2 100644 --- a/app/code/Magento/Sales/Model/Order/Payment/Info.php +++ b/app/code/Magento/Sales/Model/Order/Payment/Info.php @@ -226,9 +226,6 @@ protected function initAdditionalInformation() { $additionalInfo = $this->getData('additional_information'); if (empty($this->additionalInformation) && $additionalInfo) { - if (!is_array($additionalInfo)) { - $additionalInfo = unserialize($additionalInfo); - } $this->additionalInformation = $additionalInfo; } } diff --git a/app/code/Magento/Sales/Test/Unit/Model/Order/Payment/InfoTest.php b/app/code/Magento/Sales/Test/Unit/Model/Order/Payment/InfoTest.php index f47806866aef6..3516aef705982 100644 --- a/app/code/Magento/Sales/Test/Unit/Model/Order/Payment/InfoTest.php +++ b/app/code/Magento/Sales/Test/Unit/Model/Order/Payment/InfoTest.php @@ -89,8 +89,7 @@ public function ccKeysDataProvider() ['cc_cid', 'cc_cid_enc'] ]; } - - + public function testGetMethodInstanceWithRealMethod() { $method = 'real_method'; @@ -108,7 +107,6 @@ public function testGetMethodInstanceWithRealMethod() $this->info->getMethodInstance(); } - public function testGetMethodInstanceWithUnrealMethod() { $method = 'unreal_method'; @@ -131,7 +129,6 @@ public function testGetMethodInstanceWithUnrealMethod() $this->info->getMethodInstance(); } - /** * @expectedException \Magento\Framework\Exception\LocalizedException * @expectedExceptionMessage The payment method you requested is not available. @@ -253,9 +250,9 @@ public function testHasAdditionalInformation() public function testInitAdditionalInformationWithUnserialize() { - $data = serialize(['key1' => 'data1', 'key2' => 'data2']); + $data = ['key1' => 'data1', 'key2' => 'data2']; $this->info->setData('additional_information', $data); - $this->assertEquals(unserialize($data), $this->info->getAdditionalInformation()); + $this->assertEquals($data, $this->info->getAdditionalInformation()); } } diff --git a/app/code/Magento/Swatches/Setup/UpgradeData.php b/app/code/Magento/Swatches/Setup/UpgradeData.php index d9f72127c0447..1d9b2856e1720 100644 --- a/app/code/Magento/Swatches/Setup/UpgradeData.php +++ b/app/code/Magento/Swatches/Setup/UpgradeData.php @@ -43,7 +43,7 @@ public function upgrade(ModuleDataSetupInterface $setup, ModuleContextInterface if (version_compare($context->getVersion(), '2.0.1', '<')) { /** @var \Magento\Eav\Setup\EavSetup $eavSetup */ - $eavSetup = $this->eavSetupFactory->create(); + $eavSetup = $this->eavSetupFactory->create(['setup' => $setup]); $groupId = (int)$eavSetup->getAttributeGroupByCode( Product::ENTITY, 'Default', diff --git a/app/code/Magento/Vault/Observer/PaymentTokenAssigner.php b/app/code/Magento/Vault/Observer/PaymentTokenAssigner.php index 760c655d3cb66..64f7f5b27226f 100644 --- a/app/code/Magento/Vault/Observer/PaymentTokenAssigner.php +++ b/app/code/Magento/Vault/Observer/PaymentTokenAssigner.php @@ -7,6 +7,7 @@ use Magento\Framework\Event\Observer; use Magento\Payment\Observer\AbstractDataAssignObserver; +use Magento\Quote\Api\Data\PaymentInterface; use Magento\Quote\Model\Quote\Payment; use Magento\Vault\Api\Data\PaymentTokenInterface; use Magento\Vault\Api\PaymentTokenManagementInterface; @@ -37,7 +38,13 @@ public function execute(Observer $observer) { $dataObject = $this->readDataArgument($observer); - $tokenPublicHash = $dataObject->getData(PaymentTokenInterface::PUBLIC_HASH); + $additionalData = $dataObject->getData(PaymentInterface::KEY_ADDITIONAL_DATA); + + if (!is_array($additionalData) || !isset($additionalData[PaymentTokenInterface::PUBLIC_HASH])) { + return; + } + + $tokenPublicHash = $additionalData[PaymentTokenInterface::PUBLIC_HASH]; if ($tokenPublicHash === null) { return; diff --git a/app/code/Magento/Vault/Observer/VaultEnableAssigner.php b/app/code/Magento/Vault/Observer/VaultEnableAssigner.php index 2c98b5a2af83b..db112cf453912 100644 --- a/app/code/Magento/Vault/Observer/VaultEnableAssigner.php +++ b/app/code/Magento/Vault/Observer/VaultEnableAssigner.php @@ -7,6 +7,7 @@ use Magento\Framework\Event\Observer; use Magento\Payment\Observer\AbstractDataAssignObserver; +use Magento\Quote\Api\Data\PaymentInterface; use Magento\Vault\Model\Ui\VaultConfigProvider; class VaultEnableAssigner extends AbstractDataAssignObserver @@ -19,11 +20,17 @@ public function execute(\Magento\Framework\Event\Observer $observer) { $data = $this->readDataArgument($observer); - if (isset($data[VaultConfigProvider::IS_ACTIVE_CODE])) { + $additionalData = $data->getData(PaymentInterface::KEY_ADDITIONAL_DATA); + + if (!is_array($additionalData)) { + return; + } + + if (isset($additionalData[VaultConfigProvider::IS_ACTIVE_CODE])) { $payment = $this->readPaymentModelArgument($observer); $payment->setAdditionalInformation( VaultConfigProvider::IS_ACTIVE_CODE, - filter_var($data[VaultConfigProvider::IS_ACTIVE_CODE], FILTER_VALIDATE_BOOLEAN) + filter_var($additionalData[VaultConfigProvider::IS_ACTIVE_CODE], FILTER_VALIDATE_BOOLEAN) ); } } diff --git a/app/code/Magento/Vault/Test/Unit/Observer/PaymentTokenAssignerTest.php b/app/code/Magento/Vault/Test/Unit/Observer/PaymentTokenAssignerTest.php index 655f0abac2501..40fa32fc7988d 100644 --- a/app/code/Magento/Vault/Test/Unit/Observer/PaymentTokenAssignerTest.php +++ b/app/code/Magento/Vault/Test/Unit/Observer/PaymentTokenAssignerTest.php @@ -12,6 +12,7 @@ use Magento\Payment\Model\InfoInterface; use Magento\Payment\Observer\AbstractDataAssignObserver; use Magento\Quote\Api\Data\CartInterface; +use Magento\Quote\Api\Data\PaymentInterface; use Magento\Quote\Model\Quote\Payment; use Magento\Vault\Api\Data\PaymentTokenInterface; use Magento\Vault\Api\PaymentTokenManagementInterface; @@ -58,7 +59,9 @@ public function testExecuteNotOrderPaymentModel() { $dataObject = new DataObject( [ - PaymentTokenInterface::PUBLIC_HASH => 'public_hash_value' + PaymentInterface::KEY_ADDITIONAL_DATA => [ + PaymentTokenInterface::PUBLIC_HASH => 'public_hash_value' + ] ] ); $paymentModel = $this->getMock(InfoInterface::class); @@ -79,7 +82,9 @@ public function testExecuteNoCustomerId() { $dataObject = new DataObject( [ - PaymentTokenInterface::PUBLIC_HASH => 'public_hash_value' + PaymentInterface::KEY_ADDITIONAL_DATA => [ + PaymentTokenInterface::PUBLIC_HASH => 'public_hash_value' + ] ] ); @@ -117,7 +122,9 @@ public function testExecuteNoPaymentToken() $publicHash = 'public_hash_value'; $dataObject = new DataObject( [ - PaymentTokenInterface::PUBLIC_HASH => $publicHash + PaymentInterface::KEY_ADDITIONAL_DATA => [ + PaymentTokenInterface::PUBLIC_HASH => $publicHash + ] ] ); @@ -161,7 +168,9 @@ public function testExecuteSaveMetadata() $publicHash = 'public_hash_value'; $dataObject = new DataObject( [ - PaymentTokenInterface::PUBLIC_HASH => $publicHash + PaymentInterface::KEY_ADDITIONAL_DATA => [ + PaymentTokenInterface::PUBLIC_HASH => $publicHash + ] ] ); diff --git a/app/code/Magento/Vault/Test/Unit/Observer/VaultEnableAssignerTest.php b/app/code/Magento/Vault/Test/Unit/Observer/VaultEnableAssignerTest.php index 0c96592f7301f..2d59188c232ee 100644 --- a/app/code/Magento/Vault/Test/Unit/Observer/VaultEnableAssignerTest.php +++ b/app/code/Magento/Vault/Test/Unit/Observer/VaultEnableAssignerTest.php @@ -10,6 +10,7 @@ use Magento\Framework\Event\Observer; use Magento\Payment\Model\InfoInterface; use Magento\Payment\Observer\AbstractDataAssignObserver; +use Magento\Quote\Api\Data\PaymentInterface; use Magento\Vault\Model\Ui\VaultConfigProvider; use Magento\Vault\Observer\VaultEnableAssigner; @@ -39,7 +40,9 @@ public function testExecute($activeCode, $expectedBool) { $dataObject = new DataObject( [ - VaultConfigProvider::IS_ACTIVE_CODE => $activeCode + PaymentInterface::KEY_ADDITIONAL_DATA => [ + VaultConfigProvider::IS_ACTIVE_CODE => $activeCode + ] ] ); $paymentModel = $this->getMock(InfoInterface::class); diff --git a/dev/tests/integration/testsuite/Magento/Paypal/Model/Payment/Method/Billing/AbstractAgreementTest.php b/dev/tests/integration/testsuite/Magento/Paypal/Model/Payment/Method/Billing/AbstractAgreementTest.php index 25405d261abdb..e05b3f6cdefe5 100644 --- a/dev/tests/integration/testsuite/Magento/Paypal/Model/Payment/Method/Billing/AbstractAgreementTest.php +++ b/dev/tests/integration/testsuite/Magento/Paypal/Model/Payment/Method/Billing/AbstractAgreementTest.php @@ -6,6 +6,8 @@ namespace Magento\Paypal\Model\Payment\Method\Billing; +use Magento\Quote\Api\Data\PaymentInterface; + class AbstractAgreementTest extends \PHPUnit_Framework_TestCase { /** @var \Magento\Paypal\Model\Method\Agreement */ @@ -60,7 +62,9 @@ public function testAssignData() )->getFirstItem(); $data = new \Magento\Framework\DataObject( [ - AbstractAgreement::TRANSPORT_BILLING_AGREEMENT_ID => $billingAgreement->getId() + PaymentInterface::KEY_ADDITIONAL_DATA => [ + AbstractAgreement::TRANSPORT_BILLING_AGREEMENT_ID => $billingAgreement->getId() + ] ] ); $this->_model->assignData($data); diff --git a/lib/internal/Magento/Framework/Model/ResourceModel/AbstractResource.php b/lib/internal/Magento/Framework/Model/ResourceModel/AbstractResource.php index 451ec868bef31..4e38d13aa72f6 100644 --- a/lib/internal/Magento/Framework/Model/ResourceModel/AbstractResource.php +++ b/lib/internal/Magento/Framework/Model/ResourceModel/AbstractResource.php @@ -3,11 +3,9 @@ * Copyright © 2016 Magento. All rights reserved. * See COPYING.txt for license details. */ - -// @codingStandardsIgnoreFile - namespace Magento\Framework\Model\ResourceModel; +use Magento\Framework\DataObject; use Magento\Framework\Model\CallbackPool; /** @@ -33,7 +31,6 @@ public function __construct() */ abstract protected function _construct(); - /** * Get connection * @@ -108,26 +105,19 @@ public function rollBack() /** * Serialize specified field in an object * - * @param \Magento\Framework\DataObject $object + * @param DataObject $object * @param string $field * @param mixed $defaultValue * @param bool $unsetEmpty * @return $this */ - protected function _serializeField(\Magento\Framework\DataObject $object, $field, $defaultValue = null, $unsetEmpty = false) + protected function _serializeField(DataObject $object, $field, $defaultValue = null, $unsetEmpty = false) { $value = $object->getData($field); - if (empty($value)) { - if ($unsetEmpty) { - $object->unsetData($field); - } else { - if (is_object($defaultValue) || is_array($defaultValue)) { - $defaultValue = serialize($defaultValue); - } - $object->setData($field, $defaultValue); - } - } elseif (is_array($value) || is_object($value)) { - $object->setData($field, serialize($value)); + if (empty($value) && $unsetEmpty) { + $object->unsetData($field); + } else { + $object->setData($field, serialize($value ?: $defaultValue)); } return $this; @@ -141,24 +131,30 @@ protected function _serializeField(\Magento\Framework\DataObject $object, $field * @param mixed $defaultValue * @return void */ - protected function _unserializeField(\Magento\Framework\DataObject $object, $field, $defaultValue = null) + protected function _unserializeField(DataObject $object, $field, $defaultValue = null) { $value = $object->getData($field); + + if ($value) { + $unserializedValue = @unserialize($value); + $value = $unserializedValue !== false || $value === 'b:0;' ? $unserializedValue : $value; + } + if (empty($value)) { $object->setData($field, $defaultValue); - } elseif (!is_array($value) && !is_object($value)) { - $object->setData($field, unserialize($value)); + } else { + $object->setData($field, $value); } } /** * Prepare data for passed table * - * @param \Magento\Framework\DataObject $object + * @param DataObject $object * @param string $table * @return array */ - protected function _prepareDataForTable(\Magento\Framework\DataObject $object, $table) + protected function _prepareDataForTable(DataObject $object, $table) { $data = []; $fields = $this->getConnection()->describeTable($table); diff --git a/lib/internal/Magento/Framework/Model/Test/Unit/ResourceModel/AbstractResourceStub.php b/lib/internal/Magento/Framework/Model/Test/Unit/ResourceModel/AbstractResourceStub.php new file mode 100644 index 0000000000000..13370114aa7ae --- /dev/null +++ b/lib/internal/Magento/Framework/Model/Test/Unit/ResourceModel/AbstractResourceStub.php @@ -0,0 +1,54 @@ +_serializeField($dataObject, $field, $defaultValue, $unsetEmpty); + + static::assertEquals($expectation, $dataObject->getDataByKey($field)); + } + + /** + * @return array + */ + public function serializableFieldsDataProvider() + { + $dataObject = new DataObject( + [ + 'object' => new \stdClass(), + 'array' => ['a', 'b', 'c'], + 'string' => 'i am string', + 'int' => 969, + 'serialized_object' => 'O:8:"stdClass":0:{}', + 'empty_value' => '', + 'empty_value_with_default' => '' + ] + ); + + return [ + [[$dataObject, 'object', null, false], serialize($dataObject->getDataByKey('object'))], + [[$dataObject, 'array', null, false], serialize($dataObject->getDataByKey('array'))], + [[$dataObject, 'string', null, false], serialize($dataObject->getDataByKey('string'))], + [[$dataObject, 'int', null, false], serialize($dataObject->getDataByKey('int'))], + [ + [$dataObject, 'serialized_object', null, false], + serialize($dataObject->getDataByKey('serialized_object')) + ], + [[$dataObject, 'empty_value', null, true], null], + [[$dataObject, 'empty_value_with_default', new \stdClass(), false], 'O:8:"stdClass":0:{}'], + ]; + } + + /** + * @param array $arguments + * @param mixed $expectation + * @dataProvider unserializableFieldsDataProvider + */ + public function testUnserializeFields(array $arguments, $expectation) + { + /** @var DataObject $dataObject */ + list($dataObject, $field, $defaultValue) = $arguments; + + $abstractResource = new AbstractResourceStub(); + + $abstractResource->_unserializeField($dataObject, $field, $defaultValue); + + static::assertEquals($expectation, $dataObject->getDataByKey($field)); + } + + /** + * @return array + */ + public function unserializableFieldsDataProvider() + { + $dataObject = new DataObject( + [ + 'object' => serialize(new \stdClass()), + 'array' => serialize(['a', 'b', 'c']), + 'string' => serialize('i am string'), + 'int' => serialize(969), + 'serialized_object' => serialize('O:8:"stdClass":0:{}'), + 'empty_value_with_default' => serialize(''), + 'not_serialized_string' => 'i am string', + 'serialized_boolean_false' => serialize(false) + ] + ); + + $defaultValue = new \stdClass(); + + return [ + [[$dataObject, 'object', null], unserialize($dataObject->getDataByKey('object'))], + [[$dataObject, 'array', null], unserialize($dataObject->getDataByKey('array'))], + [[$dataObject, 'string', null], unserialize($dataObject->getDataByKey('string'))], + [[$dataObject, 'int', null], unserialize($dataObject->getDataByKey('int'))], + [[$dataObject, 'serialized_object', null], unserialize($dataObject->getDataByKey('serialized_object'))], + [[$dataObject, 'empty_value_with_default', $defaultValue], $defaultValue], + [[$dataObject, 'not_serialized_string', null], 'i am string'], + [[$dataObject, 'serialized_boolean_false', null], false] + ]; + } +} diff --git a/setup/src/Magento/Setup/Console/Command/AbstractModuleCommand.php b/setup/src/Magento/Setup/Console/Command/AbstractModuleCommand.php index 41034aa34909d..9a06568e1385f 100644 --- a/setup/src/Magento/Setup/Console/Command/AbstractModuleCommand.php +++ b/setup/src/Magento/Setup/Console/Command/AbstractModuleCommand.php @@ -84,14 +84,18 @@ protected function cleanup(InputInterface $input, OutputInterface $output) /** @var \Magento\Framework\App\State\CleanupFiles $cleanupFiles */ $cleanupFiles = $this->objectManager->get('Magento\Framework\App\State\CleanupFiles'); $cleanupFiles->clearCodeGeneratedClasses(); - $output->writeln('Generated classes cleared successfully. Please re-run Magento compile command'); + $output->writeln( + "Generated classes cleared successfully. Please run the 'setup:di:compile' command to " + . 'generate classes.' + ); if ($input->getOption(self::INPUT_KEY_CLEAR_STATIC_CONTENT)) { $cleanupFiles->clearMaterializedViewFiles(); $output->writeln('Generated static view files cleared successfully.'); } else { $output->writeln( - 'Info: Some modules might require static view files to be cleared. Use the optional --' . - self::INPUT_KEY_CLEAR_STATIC_CONTENT . ' option to clear them.' + "Info: Some modules might require static view files to be cleared. To do this, run '" + . $this->getName() . "' with the --" . self::INPUT_KEY_CLEAR_STATIC_CONTENT + . ' option to clear them.' ); } } diff --git a/setup/src/Magento/Setup/Console/Command/AbstractModuleManageCommand.php b/setup/src/Magento/Setup/Console/Command/AbstractModuleManageCommand.php index 143632420a226..e207f114a69eb 100644 --- a/setup/src/Magento/Setup/Console/Command/AbstractModuleManageCommand.php +++ b/setup/src/Magento/Setup/Console/Command/AbstractModuleManageCommand.php @@ -6,8 +6,10 @@ namespace Magento\Setup\Console\Command; use Symfony\Component\Console\Input\InputInterface; -use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; +use Symfony\Component\Console\Input\InputOption; +use Magento\Framework\App\DeploymentConfig; +use Magento\Framework\Module\Status; abstract class AbstractModuleManageCommand extends AbstractModuleCommand { @@ -17,6 +19,11 @@ abstract class AbstractModuleManageCommand extends AbstractModuleCommand const INPUT_KEY_ALL = 'all'; const INPUT_KEY_FORCE = 'force'; + /** + * @var DeploymentConfig + */ + protected $deploymentConfig; + /** * {@inheritdoc} */ @@ -64,12 +71,8 @@ protected function execute(InputInterface $input, OutputInterface $output) $output->writeln(implode(PHP_EOL, $messages)); return; } - /** - * @var \Magento\Framework\Module\Status $status - */ - $status = $this->objectManager->get('Magento\Framework\Module\Status'); try { - $modulesToChange = $status->getModulesToChange($isEnable, $modules); + $modulesToChange = $this->getStatus()->getModulesToChange($isEnable, $modules); } catch (\LogicException $e) { $output->writeln('' . $e->getMessage() . ''); return; @@ -77,7 +80,7 @@ protected function execute(InputInterface $input, OutputInterface $output) if (!empty($modulesToChange)) { $force = $input->getOption(self::INPUT_KEY_FORCE); if (!$force) { - $constraints = $status->checkConstraints($isEnable, $modulesToChange); + $constraints = $this->getStatus()->checkConstraints($isEnable, $modulesToChange); if ($constraints) { $output->writeln( "Unable to change status of modules because of the following constraints:" @@ -86,20 +89,7 @@ protected function execute(InputInterface $input, OutputInterface $output) return; } } - $status->setIsEnabled($isEnable, $modulesToChange); - if ($isEnable) { - $output->writeln('The following modules have been enabled:'); - $output->writeln('- ' . implode("\n- ", $modulesToChange) . ''); - $output->writeln(''); - $output->writeln( - 'To make sure that the enabled modules are properly registered,' - . " run 'setup:upgrade'." - ); - } else { - $output->writeln('The following modules have been disabled:'); - $output->writeln('- ' . implode("\n- ", $modulesToChange) . ''); - $output->writeln(''); - } + $this->setIsEnabled($isEnable, $modulesToChange, $output); $this->cleanup($input, $output); if ($force) { $output->writeln( @@ -112,6 +102,44 @@ protected function execute(InputInterface $input, OutputInterface $output) } } + /** + * Enable/disable modules + * + * @param bool $isEnable + * @param string[] $modulesToChange + * @param OutputInterface $output + * @return void + */ + private function setIsEnabled($isEnable, $modulesToChange, $output) + { + $this->getStatus()->setIsEnabled($isEnable, $modulesToChange); + if ($isEnable) { + $output->writeln('The following modules have been enabled:'); + $output->writeln('- ' . implode("\n- ", $modulesToChange) . ''); + $output->writeln(''); + if ($this->getDeploymentConfig()->isAvailable()) { + $output->writeln( + 'To make sure that the enabled modules are properly registered,' + . " run 'setup:upgrade'." + ); + } + } else { + $output->writeln('The following modules have been disabled:'); + $output->writeln('- ' . implode("\n- ", $modulesToChange) . ''); + $output->writeln(''); + } + } + + /** + * Get module status + * + * @return Status + */ + private function getStatus() + { + return $this->objectManager->get(Status::class); + } + /** * Validate list of modules and return error messages * @@ -134,4 +162,18 @@ protected function validate(array $modules) * @return bool */ abstract protected function isEnable(); + + /** + * Get deployment config + * + * @return DeploymentConfig + * @deprecated + */ + private function getDeploymentConfig() + { + if (!($this->deploymentConfig instanceof DeploymentConfig)) { + return $this->objectManager->get(DeploymentConfig::class); + } + return $this->deploymentConfig; + } } diff --git a/setup/src/Magento/Setup/Console/Command/DiCompileCommand.php b/setup/src/Magento/Setup/Console/Command/DiCompileCommand.php index d86b29babc33d..c4002f4043e1d 100644 --- a/setup/src/Magento/Setup/Console/Command/DiCompileCommand.php +++ b/setup/src/Magento/Setup/Console/Command/DiCompileCommand.php @@ -3,15 +3,17 @@ * Copyright © 2016 Magento. All rights reserved. * See COPYING.txt for license details. */ - namespace Magento\Setup\Console\Command; +use Magento\Framework\ObjectManagerInterface; +use Magento\Framework\Filesystem\DriverInterface; +use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Output\OutputInterface; use Magento\Framework\Filesystem; use Magento\Framework\App\Filesystem\DirectoryList; -use Magento\Framework\Filesystem\DriverInterface; -use Magento\Framework\ObjectManagerInterface; use Magento\Framework\App\DeploymentConfig; use Magento\Framework\Component\ComponentRegistrar; +use Magento\Framework\Config\ConfigOptionsListConstants; use Magento\Setup\Model\ObjectManagerProvider; use Magento\Setup\Module\Di\App\Task\Manager; use Magento\Setup\Module\Di\App\Task\OperationFactory; @@ -19,8 +21,6 @@ use Magento\Setup\Module\Di\App\Task\OperationInterface; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Helper\ProgressBar; -use Symfony\Component\Console\Input\InputInterface; -use Symfony\Component\Console\Output\OutputInterface; /** * Command to run compile in single-tenant mode @@ -107,8 +107,10 @@ protected function configure() private function checkEnvironment() { $messages = []; - if (!$this->deploymentConfig->isAvailable()) { - $messages[] = 'You cannot run this command because the Magento application is not installed.'; + $config = $this->deploymentConfig->get(ConfigOptionsListConstants::KEY_MODULES); + if (!$config) { + $messages[] = 'You cannot run this command because modules are not enabled. You can enable modules by' + . ' running the \'module:enable --all\' command.'; } return $messages; diff --git a/setup/src/Magento/Setup/Test/Unit/Console/Command/DiCompileCommandTest.php b/setup/src/Magento/Setup/Test/Unit/Console/Command/DiCompileCommandTest.php index 51a65035ee921..31b86e7cec70c 100644 --- a/setup/src/Magento/Setup/Test/Unit/Console/Command/DiCompileCommandTest.php +++ b/setup/src/Magento/Setup/Test/Unit/Console/Command/DiCompileCommandTest.php @@ -12,13 +12,13 @@ class DiCompileCommandTest extends \PHPUnit_Framework_TestCase { /** @var \Magento\Framework\App\DeploymentConfig|\PHPUnit_Framework_MockObject_MockObject */ - private $deploymentConfig; + private $deploymentConfigMock; /** @var \Magento\Setup\Module\Di\App\Task\Manager|\PHPUnit_Framework_MockObject_MockObject */ - private $manager; + private $managerMock; /** @var \Magento\Framework\ObjectManagerInterface|\PHPUnit_Framework_MockObject_MockObject */ - private $objectManager; + private $objectManagerMock; /** @var DiCompileCommand|\PHPUnit_Framework_MockObject_MockObject */ private $command; @@ -27,28 +27,28 @@ class DiCompileCommandTest extends \PHPUnit_Framework_TestCase private $cacheMock; /** @var \Magento\Framework\Filesystem|\PHPUnit_Framework_MockObject_MockObject */ - private $filesystem; + private $filesystemMock; - /** @var \Magento\Framework\Filesystem\Driver\File | \PHPUnit_Framework_MockObject_MockObject*/ - private $fileDriver; + /** @var \Magento\Framework\Filesystem\Driver\File|\PHPUnit_Framework_MockObject_MockObject */ + private $fileDriverMock; - /** @var \Magento\Framework\App\Filesystem\DirectoryList | \PHPUnit_Framework_MockObject_MockObject*/ - private $directoryList; + /** @var \Magento\Framework\App\Filesystem\DirectoryList|\PHPUnit_Framework_MockObject_MockObject */ + private $directoryListMock; /** @var \Magento\Framework\Component\ComponentRegistrar|\PHPUnit_Framework_MockObject_MockObject */ - private $componentRegistrar; + private $componentRegistrarMock; public function setUp() { - $this->deploymentConfig = $this->getMock('Magento\Framework\App\DeploymentConfig', [], [], '', false); - $objectManagerProvider = $this->getMock( + $this->deploymentConfigMock = $this->getMock('Magento\Framework\App\DeploymentConfig', [], [], '', false); + $objectManagerProviderMock = $this->getMock( 'Magento\Setup\Model\ObjectManagerProvider', [], [], '', false ); - $this->objectManager = $this->getMockForAbstractClass( + $this->objectManagerMock = $this->getMockForAbstractClass( 'Magento\Framework\ObjectManagerInterface', [], '', @@ -58,79 +58,86 @@ public function setUp() ->disableOriginalConstructor() ->getMock(); - $objectManagerProvider->expects($this->once()) + $objectManagerProviderMock->expects($this->once()) ->method('get') - ->willReturn($this->objectManager); - $this->manager = $this->getMock('Magento\Setup\Module\Di\App\Task\Manager', [], [], '', false); - $this->directoryList = $this->getMock('Magento\Framework\App\Filesystem\DirectoryList', [], [], '', false); - $this->filesystem = $this->getMockBuilder('Magento\Framework\Filesystem') + ->willReturn($this->objectManagerMock); + $this->managerMock = $this->getMock('Magento\Setup\Module\Di\App\Task\Manager', [], [], '', false); + $this->directoryListMock = $this->getMock('Magento\Framework\App\Filesystem\DirectoryList', [], [], '', false); + $this->filesystemMock = $this->getMockBuilder('Magento\Framework\Filesystem') ->disableOriginalConstructor() ->getMock(); - $this->fileDriver = $this->getMockBuilder('Magento\Framework\Filesystem\Driver\File') + $this->fileDriverMock = $this->getMockBuilder('Magento\Framework\Filesystem\Driver\File') ->disableOriginalConstructor() ->getMock(); - $this->componentRegistrar = $this->getMock( + $this->componentRegistrarMock = $this->getMock( '\Magento\Framework\Component\ComponentRegistrar', [], [], '', false ); - $this->componentRegistrar->expects($this->any())->method('getPaths')->willReturnMap([ + $this->componentRegistrarMock->expects($this->any())->method('getPaths')->willReturnMap([ [ComponentRegistrar::MODULE, ['/path/to/module/one', '/path/to/module/two']], [ComponentRegistrar::LIBRARY, ['/path/to/library/one', '/path/to/library/two']], ]); $this->command = new DiCompileCommand( - $this->deploymentConfig, - $this->directoryList, - $this->manager, - $objectManagerProvider, - $this->filesystem, - $this->fileDriver, - $this->componentRegistrar + $this->deploymentConfigMock, + $this->directoryListMock, + $this->managerMock, + $objectManagerProviderMock, + $this->filesystemMock, + $this->fileDriverMock, + $this->componentRegistrarMock ); } - public function testExecuteNotInstalled() + public function testExecuteModulesNotEnabled() { - $this->deploymentConfig->expects($this->once())->method('isAvailable')->willReturn(false); + $this->deploymentConfigMock->expects($this->once()) + ->method('get') + ->with(\Magento\Framework\Config\ConfigOptionsListConstants::KEY_MODULES) + ->willReturn(null); $tester = new CommandTester($this->command); $tester->execute([]); $this->assertEquals( - 'You cannot run this command because the Magento application is not installed.' . PHP_EOL, + 'You cannot run this command because modules are not enabled. You can enable modules by running the ' + . "'module:enable --all' command." . PHP_EOL, $tester->getDisplay() ); } public function testExecute() { - $this->directoryList->expects($this->atLeastOnce())->method('getPath')->willReturn(null); - $this->objectManager->expects($this->once()) + $this->directoryListMock->expects($this->atLeastOnce())->method('getPath')->willReturn(null); + $this->objectManagerMock->expects($this->once()) ->method('get') ->with('Magento\Framework\App\Cache') ->willReturn($this->cacheMock); $this->cacheMock->expects($this->once())->method('clean'); $writeDirectory = $this->getMock('Magento\Framework\Filesystem\Directory\WriteInterface'); $writeDirectory->expects($this->atLeastOnce())->method('delete'); - $this->filesystem->expects($this->atLeastOnce())->method('getDirectoryWrite')->willReturn($writeDirectory); + $this->filesystemMock->expects($this->atLeastOnce())->method('getDirectoryWrite')->willReturn($writeDirectory); - $this->deploymentConfig->expects($this->once())->method('isAvailable')->willReturn(true); + $this->deploymentConfigMock->expects($this->once()) + ->method('get') + ->with(\Magento\Framework\Config\ConfigOptionsListConstants::KEY_MODULES) + ->willReturn(['Magento_Catalog' => 1]); $progressBar = $this->getMockBuilder( 'Symfony\Component\Console\Helper\ProgressBar' ) ->disableOriginalConstructor() ->getMock(); - $this->objectManager->expects($this->once())->method('configure'); - $this->objectManager + $this->objectManagerMock->expects($this->once())->method('configure'); + $this->objectManagerMock ->expects($this->once()) ->method('create') ->with('Symfony\Component\Console\Helper\ProgressBar') ->willReturn($progressBar); - $this->manager->expects($this->exactly(7))->method('addOperation'); - $this->manager->expects($this->once())->method('process'); + $this->managerMock->expects($this->exactly(7))->method('addOperation'); + $this->managerMock->expects($this->once())->method('process'); $tester = new CommandTester($this->command); $tester->execute([]); $this->assertContains( diff --git a/setup/src/Magento/Setup/Test/Unit/Console/Command/ModuleEnableDisableCommandTest.php b/setup/src/Magento/Setup/Test/Unit/Console/Command/ModuleEnableDisableCommandTest.php index 7001224977d1f..41b05cc302b19 100644 --- a/setup/src/Magento/Setup/Test/Unit/Console/Command/ModuleEnableDisableCommandTest.php +++ b/setup/src/Magento/Setup/Test/Unit/Console/Command/ModuleEnableDisableCommandTest.php @@ -14,46 +14,58 @@ class ModuleEnableDisableCommandTest extends \PHPUnit_Framework_TestCase /** * @var \Magento\Setup\Model\ObjectManagerProvider|\PHPUnit_Framework_MockObject_MockObject */ - private $objectManagerProvider; + private $objectManagerProviderMock; /** * @var \Magento\Framework\Module\Status|\PHPUnit_Framework_MockObject_MockObject */ - private $status; + private $statusMock; /** * @var \Magento\Framework\App\Cache|\PHPUnit_Framework_MockObject_MockObject */ - private $cache; + private $cacheMock; /** * @var \Magento\Framework\App\State\CleanupFiles|\PHPUnit_Framework_MockObject_MockObject */ - private $cleanupFiles; + private $cleanupFilesMock; /** * @var \Magento\Framework\Module\FullModuleList|\PHPUnit_Framework_MockObject_MockObject */ - private $fullModuleList; + private $fullModuleListMock; + + /** + * @var \Magento\Framework\App\DeploymentConfig|\PHPUnit_Framework_MockObject_MockObject + */ + private $deploymentConfigMock; protected function setUp() { - $this->objectManagerProvider = $this->getMock('Magento\Setup\Model\ObjectManagerProvider', [], [], '', false); + $this->objectManagerProviderMock = $this->getMock( + 'Magento\Setup\Model\ObjectManagerProvider', + [], + [], + '', + false + ); $objectManager = $this->getMockForAbstractClass('Magento\Framework\ObjectManagerInterface'); - $this->objectManagerProvider->expects($this->any()) + $this->objectManagerProviderMock->expects($this->any()) ->method('get') ->will($this->returnValue($objectManager)); - $this->status = $this->getMock('Magento\Framework\Module\Status', [], [], '', false); - $this->cache = $this->getMock('Magento\Framework\App\Cache', [], [], '', false); - $this->cleanupFiles = $this->getMock('Magento\Framework\App\State\CleanupFiles', [], [], '', false); - $this->fullModuleList = $this->getMock('Magento\Framework\Module\FullModuleList', [], [], '', false); + $this->statusMock = $this->getMock('Magento\Framework\Module\Status', [], [], '', false); + $this->cacheMock = $this->getMock('Magento\Framework\App\Cache', [], [], '', false); + $this->cleanupFilesMock = $this->getMock('Magento\Framework\App\State\CleanupFiles', [], [], '', false); + $this->fullModuleListMock = $this->getMock('Magento\Framework\Module\FullModuleList', [], [], '', false); + $this->deploymentConfigMock = $this->getMock(\Magento\Framework\App\DeploymentConfig::class, [], [], '', false); $objectManager->expects($this->any()) ->method('get') ->will($this->returnValueMap([ - ['Magento\Framework\Module\Status', $this->status], - ['Magento\Framework\App\Cache', $this->cache], - ['Magento\Framework\App\State\CleanupFiles', $this->cleanupFiles], - ['Magento\Framework\Module\FullModuleList', $this->fullModuleList], + ['Magento\Framework\Module\Status', $this->statusMock], + ['Magento\Framework\App\Cache', $this->cacheMock], + ['Magento\Framework\App\State\CleanupFiles', $this->cleanupFilesMock], + ['Magento\Framework\Module\FullModuleList', $this->fullModuleListMock], ])); } @@ -66,35 +78,30 @@ protected function setUp() */ public function testExecute($isEnable, $clearStaticContent, $expectedMessage) { - $this->status->expects($this->once()) + $this->statusMock->expects($this->once()) ->method('getModulesToChange') ->with($isEnable, ['Magento_Module1', 'Magento_Module2']) ->will($this->returnValue(['Magento_Module1'])); - - $this->status->expects($this->any()) + $this->statusMock->expects($this->any()) ->method('checkConstraints') ->will($this->returnValue([])); - - $this->status->expects($this->once()) + $this->statusMock->expects($this->once()) ->method('setIsEnabled') ->with($isEnable, ['Magento_Module1']); - - $this->cache->expects($this->once()) + $this->cacheMock->expects($this->once()) ->method('clean'); - $this->cleanupFiles->expects($this->once()) + $this->cleanupFilesMock->expects($this->once()) ->method('clearCodeGeneratedClasses'); - $this->cleanupFiles->expects($clearStaticContent ? $this->once() : $this->never()) + $this->cleanupFilesMock->expects($clearStaticContent ? $this->once() : $this->never()) ->method('clearMaterializedViewFiles'); - - $commandTester = $isEnable - ? new CommandTester(new ModuleEnableCommand($this->objectManagerProvider)) - : new CommandTester(new ModuleDisableCommand($this->objectManagerProvider)); + $commandTester = $this->getCommandTester($isEnable); $input = ['module' => ['Magento_Module1', 'Magento_Module2']]; if ($clearStaticContent) { $input['--clear-static-content'] = true; } $commandTester->execute($input); - $this->assertStringMatchesFormat($expectedMessage, $commandTester->getDisplay()); + $display = $commandTester->getDisplay(); + $this->assertStringMatchesFormat($expectedMessage, $display); } /** @@ -106,14 +113,16 @@ public function executeDataProvider() 'enable, do not clear static content' => [ true, false, - '%amodules have been enabled%aMagento_Module1%a' . - 'Info: Some modules might require static view files to be cleared.%a' + '%amodules have been enabled%aMagento_Module1%a' + . "Info: Some modules might require static view files to be cleared. To do this, run " + . "'module:enable' with the --clear-static-content%a" ], 'disable, do not clear static content' => [ false, false, - '%amodules have been disabled%aMagento_Module1%a' . - 'Info: Some modules might require static view files to be cleared.%a' + '%amodules have been disabled%aMagento_Module1%a' + . "Info: Some modules might require static view files to be cleared. To do this, run " + . "'module:disable' with the --clear-static-content%a" ], 'enable, clear static content' => [ true, @@ -124,17 +133,17 @@ public function executeDataProvider() false, true, '%amodules have been disabled%aMagento_Module1%aGenerated static view files cleared%a' - ], + ] ]; } public function testExecuteEnableInvalidModule() { - $this->status->expects($this->once()) + $this->statusMock->expects($this->once()) ->method('getModulesToChange') ->with(true, ['invalid']) ->willThrowException(new \LogicException('Unknown module(s): invalid')); - $commandTester = new CommandTester(new ModuleEnableCommand($this->objectManagerProvider)); + $commandTester = $this->getCommandTester(true); $input = ['module' => ['invalid']]; $commandTester->execute($input); $this->assertEquals('Unknown module(s): invalid' . PHP_EOL, $commandTester->getDisplay()); @@ -142,11 +151,11 @@ public function testExecuteEnableInvalidModule() public function testExecuteDisableInvalidModule() { - $this->status->expects($this->once()) + $this->statusMock->expects($this->once()) ->method('getModulesToChange') ->with(false, ['invalid']) ->willThrowException(new \LogicException('Unknown module(s): invalid')); - $commandTester = new CommandTester(new ModuleDisableCommand($this->objectManagerProvider)); + $commandTester = $this->getCommandTester(false); $input = ['module' => ['invalid']]; $commandTester->execute($input); $this->assertEquals('Unknown module(s): invalid' . PHP_EOL, $commandTester->getDisplay()); @@ -155,34 +164,44 @@ public function testExecuteDisableInvalidModule() /** * @param bool $isEnable * @param string $expectedMessage + * @param bool $isInstalled * * @dataProvider executeAllDataProvider */ public function testExecuteAll($isEnable, $expectedMessage) { - $this->fullModuleList->expects($this->once()) + $setupUpgradeMessage = 'To make sure that the enabled modules are properly registered, run \'setup:upgrade\'.'; + $this->fullModuleListMock->expects($this->once()) ->method('getNames') ->will($this->returnValue(['Magento_Module1', 'Magento_Module2'])); - - $this->status->expects($this->once()) + $this->statusMock->expects($this->once()) ->method('getModulesToChange') ->with($isEnable, ['Magento_Module1', 'Magento_Module2']) ->will($this->returnValue(['Magento_Module1'])); - - $this->status->expects($this->any()) + $this->statusMock->expects($this->any()) ->method('checkConstraints') ->will($this->returnValue([])); - - $this->status->expects($this->once()) + $this->statusMock->expects($this->once()) ->method('setIsEnabled') ->with($isEnable, ['Magento_Module1']); - - $commandTester = $isEnable - ? new CommandTester(new ModuleEnableCommand($this->objectManagerProvider)) - : new CommandTester(new ModuleDisableCommand($this->objectManagerProvider)); + if ($isEnable) { + $this->deploymentConfigMock->expects($this->once()) + ->method('isAvailable') + ->willReturn(['Magento_Module1']); + } else { + $this->deploymentConfigMock->expects($this->never()) + ->method('isAvailable'); + } + $commandTester = $this->getCommandTester($isEnable); $input = ['--all' => true]; $commandTester->execute($input); - $this->assertStringMatchesFormat($expectedMessage, $commandTester->getDisplay()); + $output = $commandTester->getDisplay(); + $this->assertStringMatchesFormat($expectedMessage, $output); + if ($isEnable) { + $this->assertContains($setupUpgradeMessage, $output); + } else { + $this->assertNotContains($setupUpgradeMessage, $output); + } } /** @@ -203,21 +222,16 @@ public function executeAllDataProvider() */ public function testExecuteWithConstraints($isEnable) { - $this->status->expects($this->once()) + $this->statusMock->expects($this->once()) ->method('getModulesToChange') ->with($isEnable, ['Magento_Module1', 'Magento_Module2']) ->will($this->returnValue(['Magento_Module1'])); - - $this->status->expects($this->any()) + $this->statusMock->expects($this->any()) ->method('checkConstraints') ->will($this->returnValue(['constraint1', 'constraint2'])); - - $this->status->expects($this->never()) + $this->statusMock->expects($this->never()) ->method('setIsEnabled'); - - $commandTester = $isEnable - ? new CommandTester(new ModuleEnableCommand($this->objectManagerProvider)) - : new CommandTester(new ModuleDisableCommand($this->objectManagerProvider)); + $commandTester = $this->getCommandTester($isEnable); $commandTester->execute(['module' => ['Magento_Module1', 'Magento_Module2']]); $this->assertStringMatchesFormat( 'Unable to change status of modules%aconstraint1%aconstraint2%a', @@ -244,21 +258,16 @@ public function executeWithConstraintsDataProvider() */ public function testExecuteForce($isEnable, $expectedMessage) { - $this->status->expects($this->once()) + $this->statusMock->expects($this->once()) ->method('getModulesToChange') ->with($isEnable, ['Magento_Module1', 'Magento_Module2']) ->will($this->returnValue(['Magento_Module1'])); - - $this->status->expects($this->never()) + $this->statusMock->expects($this->never()) ->method('checkConstraints'); - - $this->status->expects($this->once()) + $this->statusMock->expects($this->once()) ->method('setIsEnabled') ->with($isEnable, ['Magento_Module1']); - - $commandTester = $isEnable - ? new CommandTester(new ModuleEnableCommand($this->objectManagerProvider)) - : new CommandTester(new ModuleDisableCommand($this->objectManagerProvider)); + $commandTester = $this->getCommandTester($isEnable); $commandTester->execute(['module' => ['Magento_Module1', 'Magento_Module2'], '--force' => true]); $this->assertStringMatchesFormat( $expectedMessage . '%amodules might not function properly%a', @@ -284,21 +293,31 @@ public function executeExecuteForceDataProvider() */ public function testExecuteNoChanges($isEnable) { - $this->status->expects($this->once()) + $this->statusMock->expects($this->once()) ->method('getModulesToChange') ->with($isEnable, ['Magento_Module1', 'Magento_Module2']) ->will($this->returnValue([])); - - $this->status->expects($this->never()) + $this->statusMock->expects($this->never()) ->method('setIsEnabled'); - - $commandTester = $isEnable - ? new CommandTester(new ModuleEnableCommand($this->objectManagerProvider)) - : new CommandTester(new ModuleDisableCommand($this->objectManagerProvider)); + $commandTester = $this->getCommandTester($isEnable); $commandTester->execute(['module' => ['Magento_Module1', 'Magento_Module2']]); $this->assertStringMatchesFormat( 'No modules were changed%a', $commandTester->getDisplay() ); } + + /** + * @param bool $isEnable + * @return CommandTester + */ + private function getCommandTester($isEnable) + { + $class = $isEnable ? ModuleEnableCommand::class : ModuleDisableCommand::class; + $command = new $class($this->objectManagerProviderMock); + $deploymentConfigProperty = new \ReflectionProperty($class, 'deploymentConfig'); + $deploymentConfigProperty->setAccessible(true); + $deploymentConfigProperty->setValue($command, $this->deploymentConfigMock); + return new CommandTester($command); + } }