diff --git a/app/code/Magento/Catalog/Model/ProductRepository.php b/app/code/Magento/Catalog/Model/ProductRepository.php index e8b5dea903b9e4c9c0cf77673ae69db5ada199e2..14ee790b382344285422dfa8583312b835a9d5fe 100644 --- a/app/code/Magento/Catalog/Model/ProductRepository.php +++ b/app/code/Magento/Catalog/Model/ProductRepository.php @@ -15,6 +15,7 @@ use Magento\Framework\Api\ImageContentValidatorInterface; use Magento\Framework\Api\ImageProcessorInterface; use Magento\Framework\Api\SortOrder; use Magento\Framework\Exception\InputException; +use Magento\Framework\Exception\LocalizedException; use Magento\Framework\Exception\NoSuchEntityException; use Magento\Framework\Exception\StateException; use Magento\Framework\Exception\ValidatorException; @@ -546,6 +547,8 @@ class ProductRepository implements \Magento\Catalog\Api\ProductRepositoryInterfa ); } catch (ValidatorException $e) { throw new CouldNotSaveException(__($e->getMessage())); + } catch (LocalizedException $e) { + throw $e; } catch (\Exception $e) { throw new \Magento\Framework\Exception\CouldNotSaveException(__('Unable to save product')); } diff --git a/app/code/Magento/CatalogSearch/Model/Indexer/Fulltext/Plugin/Product.php b/app/code/Magento/CatalogSearch/Model/Indexer/Fulltext/Plugin/Product.php index 72b5b776750ec09afa2da08bf296bcd9a84f524b..26e4336f53d03f005003f1ae87d50d2728fae91a 100644 --- a/app/code/Magento/CatalogSearch/Model/Indexer/Fulltext/Plugin/Product.php +++ b/app/code/Magento/CatalogSearch/Model/Indexer/Fulltext/Plugin/Product.php @@ -6,45 +6,58 @@ namespace Magento\CatalogSearch\Model\Indexer\Fulltext\Plugin; +use Magento\Catalog\Model\ResourceModel\Product as ResourceProduct; +use Magento\Framework\Model\AbstractModel; + class Product extends AbstractPlugin { /** * Reindex on product save * - * @param \Magento\Catalog\Model\ResourceModel\Product $productResource + * @param ResourceProduct $productResource * @param \Closure $proceed - * @param \Magento\Framework\Model\AbstractModel $product - * @return \Magento\Catalog\Model\ResourceModel\Product - * @SuppressWarnings(PHPMD.UnusedFormalParameter) + * @param AbstractModel $product + * @return ResourceProduct */ - public function aroundSave( - \Magento\Catalog\Model\ResourceModel\Product $productResource, - \Closure $proceed, - \Magento\Framework\Model\AbstractModel $product - ) { - $productResource->addCommitCallback(function () use ($product) { - $this->reindexRow($product->getEntityId()); - }); - return $proceed($product); + public function aroundSave(ResourceProduct $productResource, \Closure $proceed, AbstractModel $product) + { + return $this->addCommitCallback($productResource, $proceed, $product); } /** * Reindex on product delete * - * @param \Magento\Catalog\Model\ResourceModel\Product $productResource + * @param ResourceProduct $productResource * @param \Closure $proceed - * @param \Magento\Framework\Model\AbstractModel $product - * @return \Magento\Catalog\Model\ResourceModel\Product - * @SuppressWarnings(PHPMD.UnusedFormalParameter) + * @param AbstractModel $product + * @return ResourceProduct */ - public function aroundDelete( - \Magento\Catalog\Model\ResourceModel\Product $productResource, - \Closure $proceed, - \Magento\Framework\Model\AbstractModel $product - ) { - $productResource->addCommitCallback(function () use ($product) { - $this->reindexRow($product->getEntityId()); - }); - return $proceed($product); + public function aroundDelete(ResourceProduct $productResource, \Closure $proceed, AbstractModel $product) + { + return $this->addCommitCallback($productResource, $proceed, $product); + } + + /** + * @param ResourceProduct $productResource + * @param \Closure $proceed + * @param AbstractModel $product + * @return ResourceProduct + * @throws \Exception + */ + private function addCommitCallback(ResourceProduct $productResource, \Closure $proceed, AbstractModel $product) + { + try { + $productResource->beginTransaction(); + $result = $proceed($product); + $productResource->addCommitCallback(function () use ($product) { + $this->reindexRow($product->getEntityId()); + }); + $productResource->commit(); + } catch (\Exception $e) { + $productResource->rollBack(); + throw $e; + } + + return $result; } } diff --git a/app/code/Magento/CatalogSearch/Test/Unit/Model/Indexer/Fulltext/Plugin/ProductTest.php b/app/code/Magento/CatalogSearch/Test/Unit/Model/Indexer/Fulltext/Plugin/ProductTest.php index c6666a5a2e22e0a039bf33168550643fc81dc933..649b332fbf80309631444b4be5ba741082053289 100644 --- a/app/code/Magento/CatalogSearch/Test/Unit/Model/Indexer/Fulltext/Plugin/ProductTest.php +++ b/app/code/Magento/CatalogSearch/Test/Unit/Model/Indexer/Fulltext/Plugin/ProductTest.php @@ -6,22 +6,28 @@ namespace Magento\CatalogSearch\Test\Unit\Model\Indexer\Fulltext\Plugin; +use Magento\Catalog\Model\Product as ProductModel; +use Magento\Catalog\Model\ResourceModel\Product as ProductResourceModel; use \Magento\CatalogSearch\Model\Indexer\Fulltext\Plugin\Product; +use Magento\Framework\DB\Adapter\AdapterInterface; +use Magento\Framework\Indexer\IndexerInterface; +use Magento\Framework\Indexer\IndexerRegistry; +use Magento\Framework\TestFramework\Unit\Helper\ObjectManager; class ProductTest extends \PHPUnit_Framework_TestCase { /** - * @var \PHPUnit_Framework_MockObject_MockObject|\Magento\Framework\Indexer\IndexerInterface + * @var \PHPUnit_Framework_MockObject_MockObject|IndexerInterface */ protected $indexerMock; /** - * @var \PHPUnit_Framework_MockObject_MockObject|\Magento\Catalog\Model\ResourceModel\Product + * @var \PHPUnit_Framework_MockObject_MockObject|ProductResourceModel */ protected $subjectMock; /** - * @var \PHPUnit_Framework_MockObject_MockObject|\Magento\Catalog\Model\Product + * @var \PHPUnit_Framework_MockObject_MockObject|ProductModel */ protected $productMock; @@ -31,7 +37,7 @@ class ProductTest extends \PHPUnit_Framework_TestCase protected $proceed; /** - * @var \Magento\Framework\Indexer\IndexerRegistry|\PHPUnit_Framework_MockObject_MockObject + * @var IndexerRegistry|\PHPUnit_Framework_MockObject_MockObject */ protected $indexerRegistryMock; @@ -42,30 +48,34 @@ class ProductTest extends \PHPUnit_Framework_TestCase protected function setUp() { - $this->productMock = $this->getMock('Magento\Catalog\Model\Product', [], [], '', false); - $this->subjectMock = $this->getMock('Magento\Catalog\Model\ResourceModel\Product', [], [], '', false); - $this->indexerMock = $this->getMockForAbstractClass( - 'Magento\Framework\Indexer\IndexerInterface', - [], - '', - false, - false, - true, - ['getId', 'getState', '__wakeup'] - ); - $this->indexerRegistryMock = $this->getMock( - 'Magento\Framework\Indexer\IndexerRegistry', - ['get'], - [], - '', - false - ); + $this->productMock = $this->getMockBuilder(ProductModel::class) + ->disableOriginalConstructor() + ->getMock(); + $this->subjectMock = $this->getMockBuilder(ProductResourceModel::class) + ->disableOriginalConstructor() + ->getMock(); + $connection = $this->getMockBuilder(AdapterInterface::class) + ->disableOriginalConstructor() + ->getMockForAbstractClass(); + $this->subjectMock->method('getConnection')->willReturn($connection); + + $this->indexerMock = $this->getMockBuilder(IndexerInterface::class) + ->disableOriginalConstructor() + ->setMethods(['getId', 'getState', '__wakeup']) + ->getMockForAbstractClass(); + $this->indexerRegistryMock = $this->getMockBuilder(IndexerRegistry::class) + ->disableOriginalConstructor() + ->setMethods(['get']) + ->getMock(); $this->proceed = function () { return $this->subjectMock; }; - $this->model = new Product($this->indexerRegistryMock); + $this->model = (new ObjectManager($this))->getObject( + Product::class, + ['indexerRegistry' => $this->indexerRegistryMock] + ); } public function testAfterSaveNonScheduled() diff --git a/app/code/Magento/GoogleOptimizer/Observer/AbstractSave.php b/app/code/Magento/GoogleOptimizer/Observer/AbstractSave.php index dc1a5ef3166f0711f6918b660bcbc0513681d952..3c3afd7a784b65ea1d2d23ee694104b43fc861ea 100644 --- a/app/code/Magento/GoogleOptimizer/Observer/AbstractSave.php +++ b/app/code/Magento/GoogleOptimizer/Observer/AbstractSave.php @@ -58,7 +58,7 @@ abstract class AbstractSave implements ObserverInterface { $this->_initEntity($observer); - if ($this->_isGoogleExperimentActive()) { + if ($this->_isGoogleExperimentActive() && $this->isDataAvailable()) { $this->_processCode(); } @@ -112,11 +112,10 @@ abstract class AbstractSave implements ObserverInterface */ protected function _initRequestParams() { - $params = $this->_request->getParam('google_experiment'); - if (!is_array($params) || !isset($params['experiment_script']) || !isset($params['code_id'])) { + if (!$this->isDataAvailable()) { throw new \InvalidArgumentException('Wrong request parameters'); } - $this->_params = $params; + $this->_params = $this->getRequestData(); } /** @@ -181,4 +180,21 @@ abstract class AbstractSave implements ObserverInterface { $this->_modelCode->delete(); } + + /** + * @return array + */ + private function isDataAvailable() + { + $params = $this->getRequestData(); + return is_array($params) && isset($params['experiment_script']) && isset($params['code_id']); + } + + /** + * @return mixed + */ + private function getRequestData() + { + return $this->_request->getParam('google_experiment'); + } } diff --git a/app/code/Magento/GoogleOptimizer/Test/Unit/Observer/Category/SaveGoogleExperimentScriptObserverTest.php b/app/code/Magento/GoogleOptimizer/Test/Unit/Observer/Category/SaveGoogleExperimentScriptObserverTest.php index 14f0cd8f43727b985f8283ff5e703a9c71fc38e7..f6f6b4670bffcf9f1e5e6b7a43531cbcd4ce21b9 100644 --- a/app/code/Magento/GoogleOptimizer/Test/Unit/Observer/Category/SaveGoogleExperimentScriptObserverTest.php +++ b/app/code/Magento/GoogleOptimizer/Test/Unit/Observer/Category/SaveGoogleExperimentScriptObserverTest.php @@ -85,7 +85,7 @@ class SaveGoogleExperimentScriptObserverTest extends \PHPUnit_Framework_TestCase ); $this->_requestMock->expects( - $this->once() + $this->exactly(3) )->method( 'getParam' )->with( @@ -113,8 +113,6 @@ class SaveGoogleExperimentScriptObserverTest extends \PHPUnit_Framework_TestCase /** * @param array $params - * @expectedException \InvalidArgumentException - * @expectedExceptionMessage Wrong request parameters * @dataProvider dataProviderWrongRequestForCreating */ public function testCreatingCodeIfRequestIsNotValid($params) @@ -174,7 +172,7 @@ class SaveGoogleExperimentScriptObserverTest extends \PHPUnit_Framework_TestCase ); $this->_requestMock->expects( - $this->once() + $this->exactly(3) )->method( 'getParam' )->with( @@ -223,7 +221,7 @@ class SaveGoogleExperimentScriptObserverTest extends \PHPUnit_Framework_TestCase ); $this->_requestMock->expects( - $this->once() + $this->exactly(3) )->method( 'getParam' )->with( @@ -254,7 +252,7 @@ class SaveGoogleExperimentScriptObserverTest extends \PHPUnit_Framework_TestCase ); $this->_requestMock->expects( - $this->once() + $this->exactly(3) )->method( 'getParam' )->with( diff --git a/app/code/Magento/GoogleOptimizer/Test/Unit/Observer/CmsPage/SaveGoogleExperimentScriptObserverTest.php b/app/code/Magento/GoogleOptimizer/Test/Unit/Observer/CmsPage/SaveGoogleExperimentScriptObserverTest.php index 485fc36343df1d8a6e440af7ed3bc0daafac9cb3..4e1f0245176b951ddfc1642de6206e7baaf0bd0e 100644 --- a/app/code/Magento/GoogleOptimizer/Test/Unit/Observer/CmsPage/SaveGoogleExperimentScriptObserverTest.php +++ b/app/code/Magento/GoogleOptimizer/Test/Unit/Observer/CmsPage/SaveGoogleExperimentScriptObserverTest.php @@ -70,7 +70,7 @@ class SaveGoogleExperimentScriptObserverTest extends \PHPUnit_Framework_TestCase $this->_helperMock->expects($this->once())->method('isGoogleExperimentActive')->will($this->returnValue(true)); $this->_requestMock->expects( - $this->once() + $this->exactly(3) )->method( 'getParam' )->with( @@ -98,8 +98,6 @@ class SaveGoogleExperimentScriptObserverTest extends \PHPUnit_Framework_TestCase /** * @param array $params - * @expectedException \InvalidArgumentException - * @expectedExceptionMessage Wrong request parameters * @dataProvider dataProviderWrongRequestForCreating */ public function testCreatingCodeIfRequestIsNotValid($params) @@ -143,7 +141,7 @@ class SaveGoogleExperimentScriptObserverTest extends \PHPUnit_Framework_TestCase $this->_helperMock->expects($this->once())->method('isGoogleExperimentActive')->will($this->returnValue(true)); $this->_requestMock->expects( - $this->once() + $this->exactly(3) )->method( 'getParam' )->with( @@ -184,7 +182,7 @@ class SaveGoogleExperimentScriptObserverTest extends \PHPUnit_Framework_TestCase $this->_helperMock->expects($this->once())->method('isGoogleExperimentActive')->will($this->returnValue(true)); $this->_requestMock->expects( - $this->once() + $this->exactly(3) )->method( 'getParam' )->with( @@ -215,7 +213,7 @@ class SaveGoogleExperimentScriptObserverTest extends \PHPUnit_Framework_TestCase ); $this->_requestMock->expects( - $this->once() + $this->exactly(3) )->method( 'getParam' )->with( diff --git a/app/code/Magento/GoogleOptimizer/Test/Unit/Observer/Product/SaveGoogleExperimentScriptObserverTest.php b/app/code/Magento/GoogleOptimizer/Test/Unit/Observer/Product/SaveGoogleExperimentScriptObserverTest.php index fe25ce77646eb82e55ee201dd3021ff2342691e2..34821ca212159b4dde351da10099040da9f17395 100644 --- a/app/code/Magento/GoogleOptimizer/Test/Unit/Observer/Product/SaveGoogleExperimentScriptObserverTest.php +++ b/app/code/Magento/GoogleOptimizer/Test/Unit/Observer/Product/SaveGoogleExperimentScriptObserverTest.php @@ -85,7 +85,7 @@ class SaveGoogleExperimentScriptObserverTest extends \PHPUnit_Framework_TestCase ); $this->_requestMock->expects( - $this->once() + $this->exactly(3) )->method( 'getParam' )->with( @@ -113,8 +113,6 @@ class SaveGoogleExperimentScriptObserverTest extends \PHPUnit_Framework_TestCase /** * @param array $params - * @expectedException \InvalidArgumentException - * @expectedExceptionMessage Wrong request parameters * @dataProvider dataProviderWrongRequestForCreating */ public function testCreatingCodeIfRequestIsNotValid($params) @@ -174,7 +172,7 @@ class SaveGoogleExperimentScriptObserverTest extends \PHPUnit_Framework_TestCase ); $this->_requestMock->expects( - $this->once() + $this->exactly(3) )->method( 'getParam' )->with( @@ -223,7 +221,7 @@ class SaveGoogleExperimentScriptObserverTest extends \PHPUnit_Framework_TestCase ); $this->_requestMock->expects( - $this->once() + $this->exactly(3) )->method( 'getParam' )->with( @@ -254,7 +252,7 @@ class SaveGoogleExperimentScriptObserverTest extends \PHPUnit_Framework_TestCase ); $this->_requestMock->expects( - $this->once() + $this->exactly(3) )->method( 'getParam' )->with( diff --git a/dev/tests/functional/tests/app/Magento/Catalog/Test/Block/Product/Compare/Sidebar.php b/dev/tests/functional/tests/app/Magento/Catalog/Test/Block/Product/Compare/Sidebar.php index 773a18bac7e2ccce07d8c0e43e0277d072d25ddd..596dbd38664b3b097302e6f6f0e62d13fbe96017 100644 --- a/dev/tests/functional/tests/app/Magento/Catalog/Test/Block/Product/Compare/Sidebar.php +++ b/dev/tests/functional/tests/app/Magento/Catalog/Test/Block/Product/Compare/Sidebar.php @@ -79,6 +79,5 @@ class Sidebar extends ListCompare } ); $this->_rootElement->find($this->clearAll)->click(); - $this->browser->acceptAlert(); } } diff --git a/lib/internal/Magento/Framework/Model/ResourceModel/AbstractResource.php b/lib/internal/Magento/Framework/Model/ResourceModel/AbstractResource.php index d2b901d950555508b7f7edbb1b58cb3338af1a0d..451ec868bef316b3393b31912c4811530b0a8b25 100644 --- a/lib/internal/Magento/Framework/Model/ResourceModel/AbstractResource.php +++ b/lib/internal/Magento/Framework/Model/ResourceModel/AbstractResource.php @@ -56,7 +56,7 @@ abstract class AbstractResource /** * Subscribe some callback to transaction commit * - * @param array $callback + * @param callable|array $callback * @return $this * @api */ diff --git a/lib/internal/Magento/Framework/Search/Search.php b/lib/internal/Magento/Framework/Search/Search.php index 3bd25d6dec9c3b905cab4b03dcb684f6b64c1941..d7d00b08f484df6cfe2a4bdbbf4f31f83c0a405e 100644 --- a/lib/internal/Magento/Framework/Search/Search.php +++ b/lib/internal/Magento/Framework/Search/Search.php @@ -84,7 +84,7 @@ class Search implements SearchInterface */ private function addFieldToFilter($field, $condition = null) { - if (!is_array($condition) || !in_array(key($condition), ['from', 'to'])) { + if (!is_array($condition) || !in_array(key($condition), ['from', 'to'], true)) { $this->requestBuilder->bind($field, $condition); } else { if (!empty($condition['from'])) { diff --git a/lib/internal/Magento/Framework/Search/Test/Unit/SearchTest.php b/lib/internal/Magento/Framework/Search/Test/Unit/SearchTest.php index 0fbf9215f63e7c424714b8073cc3c9b6a793fc11..89e83e7cc8bf1609419c76f79f387064a4b51ac5 100644 --- a/lib/internal/Magento/Framework/Search/Test/Unit/SearchTest.php +++ b/lib/internal/Magento/Framework/Search/Test/Unit/SearchTest.php @@ -66,29 +66,24 @@ class SearchTest extends \PHPUnit_Framework_TestCase { $requestName = 'requestName'; $scopeId = 333; - $filterField = 'filterField'; - $filterValue = 'filterValue'; + $filters = [ + $this->createFilterMock('array_filter', ['arrayValue1', 'arrayValue2']), + $this->createFilterMock('simple_filter', 'filterValue'), + $this->createFilterMock('from_filter', ['from' => 30]), + $this->createFilterMock('to_filter', ['to' => 100]), + $this->createFilterMock('range_filter', ['from' => 60, 'to' => 82]), + ]; $scope = $this->getMockBuilder('Magento\Framework\App\ScopeInterface') ->disableOriginalConstructor() ->getMockForAbstractClass(); - - $filter = $this->getMockBuilder('Magento\Framework\Api\Filter') - ->disableOriginalConstructor() - ->getMock(); - $filter->expects($this->once()) - ->method('getField') - ->willReturn($filterField); - $filter->expects($this->once()) - ->method('getValue') - ->willReturn($filterValue); - + $filterGroup = $this->getMockBuilder('Magento\Framework\Api\Search\FilterGroup') ->disableOriginalConstructor() ->getMock(); $filterGroup->expects($this->once()) ->method('getFilters') - ->willReturn([$filter]); + ->willReturn($filters); $searchCriteria = $this->getMockBuilder('Magento\Framework\Api\Search\SearchCriteriaInterface') ->disableOriginalConstructor() @@ -118,7 +113,7 @@ class SearchTest extends \PHPUnit_Framework_TestCase $this->requestBuilder->expects($this->once()) ->method('bindDimension') ->with('scope', $scopeId); - $this->requestBuilder->expects($this->any()) + $this->requestBuilder->expects($this->exactly(6)) ->method('bind'); $this->requestBuilder->expects($this->once()) ->method('create') @@ -146,4 +141,23 @@ class SearchTest extends \PHPUnit_Framework_TestCase $this->assertInstanceOf('Magento\Framework\Api\Search\SearchResultInterface', $searchResult); } + + /** + * @param $field + * @param $value + * @return \Magento\Framework\Api\Filter|\PHPUnit_Framework_MockObject_MockObject + */ + private function createFilterMock($field, $value) + { + $filter = $this->getMockBuilder('Magento\Framework\Api\Filter') + ->disableOriginalConstructor() + ->getMock(); + $filter->expects($this->once()) + ->method('getField') + ->willReturn($field); + $filter->expects($this->once()) + ->method('getValue') + ->willReturn($value); + return $filter; + } }