MAGETWO-38857: Decrease crap

yuri kovsher · yuri kovsher · commit d23cb1b1a59d · 2015-07-04T12:24:40.000+03:00
diff --git a/app/code/Magento/Sitemap/Controller/Adminhtml/Sitemap/Save.php b/app/code/Magento/Sitemap/Controller/Adminhtml/Sitemap/Save.php
@@ -12,13 +12,13 @@
 class Save extends \Magento\Sitemap\Controller\Adminhtml\Sitemap
 {
     /**
-     * Validate path to generate
+     * Validate path for generation
      *
      * @param array $data
      * @return bool
      * @throws \Exception
      */
-    protected function validatePathToGenerate(array $data)
+    protected function validatePath(array $data)
     {
         if (!empty($data['sitemap_filename']) && !empty($data['sitemap_path'])) {
             $data['sitemap_path'] = '/' . ltrim($data['sitemap_path'], '/');
@@ -65,12 +65,12 @@ protected function clearSiteMap(\Magento\Sitemap\Model\Sitemap $model)
     }
 
     /**
-     * Save model
+     * Save data
      *
      * @param array $data
-     * @return \Magento\Backend\Model\View\Result\Redirect|\Magento\Backend\Model\View\Result\Forward
+     * @return string|bool
      */
-    protected function saveModel($data)
+    protected function saveData($data)
     {
         // init model and set data
         /** @var \Magento\Sitemap\Model\Sitemap $model */
@@ -86,27 +86,46 @@ protected function saveModel($data)
             $this->messageManager->addSuccess(__('You saved the sitemap.'));
             // clear previously saved data from session
             $this->_objectManager->get('Magento\Backend\Model\Session')->setFormData(false);
+            return $model->getId();
+        } catch (\Exception $e) {
+            // display error message
+            $this->messageManager->addError($e->getMessage());
+            // save data in session
+            $this->_objectManager->get('Magento\Backend\Model\Session')->setFormData($data);
+        }
+        return false;
+    }
 
+    /**
+     * Get result after saving data
+     *
+     * @param string|bool $id
+     * @return \Magento\Framework\Controller\ResultInterface
+     */
+    protected function getResult($id)
+    {
+        /** @var \Magento\Backend\Model\View\Result\Redirect $resultRedirect */
+        $resultRedirect = $this->resultFactory->create(Controller\ResultFactory::TYPE_REDIRECT);
+        if ($id) {
             // check if 'Save and Continue'
             if ($this->getRequest()->getParam('back')) {
-                return $this->resultFactory->create(Controller\ResultFactory::TYPE_REDIRECT)
-                    ->setPath('adminhtml/*/edit', ['sitemap_id' => $model->getId()]);
+                $resultRedirect->setPath('adminhtml/*/edit', ['sitemap_id' => $id]);
+                return $resultRedirect;
             }
             // go to grid or forward to generate action
             if ($this->getRequest()->getParam('generate')) {
-                $this->getRequest()->setParam('sitemap_id', $model->getId());
-                return $this->resultFactory->create(Controller\ResultFactory::TYPE_FORWARD)->forward('generate');
+                $this->getRequest()->setParam('sitemap_id', $id);
+                return $this->resultFactory->create(Controller\ResultFactory::TYPE_FORWARD)
+                    ->forward('generate');
             }
-            return $this->resultFactory->create(Controller\ResultFactory::TYPE_REDIRECT)->setPath('adminhtml/*/');
-        } catch (\Exception $e) {
-            // display error message
-            $this->messageManager->addError($e->getMessage());
-            // save data in session
-            $this->_objectManager->get('Magento\Backend\Model\Session')->setFormData($data);
-            // redirect to edit form
-            return $this->resultFactory->create(Controller\ResultFactory::TYPE_REDIRECT)
-                ->setPath('adminhtml/*/edit', ['sitemap_id' => $this->getRequest()->getParam('sitemap_id')]);
+            $resultRedirect->setPath('adminhtml/*/');
+            return $resultRedirect;
         }
+        $resultRedirect->setPath(
+            'adminhtml/*/edit',
+            ['sitemap_id' => $this->getRequest()->getParam('sitemap_id')]
+        );
+        return $resultRedirect;
     }
 
     /**
@@ -118,17 +137,19 @@ public function execute()
     {
         // check if data sent
         $data = $this->getRequest()->getPostValue();
+        /** @var \Magento\Backend\Model\View\Result\Redirect $resultRedirect */
+        $resultRedirect = $this->resultFactory->create(Controller\ResultFactory::TYPE_REDIRECT);
         if ($data) {
-            if (!$this->validatePathToGenerate($data)) {
-                /** @var \Magento\Backend\Model\View\Result\Redirect $resultRedirect */
-                $resultRedirect = $this->resultFactory->create(Controller\ResultFactory::TYPE_REDIRECT);
-                return $resultRedirect->setPath(
+            if (!$this->validatePath($data)) {
+                $resultRedirect->setPath(
                     'adminhtml/*/edit',
                     ['sitemap_id' => $this->getRequest()->getParam('sitemap_id')]
                 );
+                return $resultRedirect;
             }
-            $this->saveModel($data);
+            return $this->getResult($this->saveData($data));
         }
-        return $this->resultFactory->create(Controller\ResultFactory::TYPE_REDIRECT)->setPath('adminhtml/*/');
+        $resultRedirect->setPath('adminhtml/*/');
+        return $resultRedirect;
     }
 }
diff --git a/app/code/Magento/Sitemap/Test/Unit/Controller/Adminhtml/Sitemap/SaveTest.php b/app/code/Magento/Sitemap/Test/Unit/Controller/Adminhtml/Sitemap/SaveTest.php
@@ -5,58 +5,102 @@
  */
 namespace Magento\Sitemap\Test\Unit\Controller\Adminhtml\Sitemap;
 
+use Magento\Framework\TestFramework\Unit\Helper\ObjectManager as ObjectManagerHelper;
+use Magento\Framework\Controller\ResultFactory;
+
 class SaveTest extends \PHPUnit_Framework_TestCase
 {
+    /**
+     * @var \Magento\Sitemap\Controller\Adminhtml\Sitemap\Save
+     */
+    protected $saveController;
+
+    /**
+     * @var \Magento\Backend\App\Action\Context
+     */
+    protected $context;
+
+    /**
+     * @var \Magento\Framework\TestFramework\Unit\Helper\ObjectManager;
+     */
+    protected $objectManagerHelper;
+
     /**
      * @var \Magento\Framework\HTTP\PhpEnvironment\Request|\PHPUnit_Framework_MockObject_MockObject
      */
-    protected $request;
+    protected $requestMock;
 
     /**
      * @var \Magento\Framework\Controller\ResultFactory|\PHPUnit_Framework_MockObject_MockObject
      */
-    protected $resultFactory;
+    protected $resultFactoryMock;
 
     /**
      * @var \Magento\Backend\Model\View\Result\Redirect|\PHPUnit_Framework_MockObject_MockObject
      */
-    protected $resultRedirect;
+    protected $resultRedirectMock;
 
     /**
-     * @var \Magento\Sitemap\Controller\Adminhtml\Sitemap\Save|\PHPUnit_Framework_MockObject_MockObject
+     * @var \Magento\Framework\ObjectManagerInterface|\PHPUnit_Framework_MockObject_MockObject
      */
-    protected $controller;
+    protected $objectManagerMock;
 
     /**
-     * @var \Magento\Backend\App\Action\Context|\PHPUnit_Framework_MockObject_MockObject
+     * @var \Magento\Framework\Message\ManagerInterface|\PHPUnit_Framework_MockObject_MockObject
      */
-    protected $context;
+    protected $messageManagerMock;
 
     protected function setUp()
     {
-        $this->request = $this->getMock('Magento\Framework\HTTP\PhpEnvironment\Request', [], [], '', false);
-        $this->resultRedirect = $this->getMock('Magento\Backend\Model\View\Result\Redirect', [], [], '', false);
-
-        $this->resultFactory = $this->getMock('Magento\Framework\Controller\ResultFactory', [], [], '', false);
-        $this->resultFactory->expects($this->once())
+        $this->requestMock = $this->getMockBuilder('Magento\Framework\App\RequestInterface')
+            ->disableOriginalConstructor()
+            ->setMethods(['getPostValue'])
+            ->getMockForAbstractClass();
+        $this->resultRedirectMock = $this->getMockBuilder('Magento\Backend\Model\View\Result\Redirect')
+            ->disableOriginalConstructor()
+            ->getMock();
+        $this->resultFactoryMock = $this->getMockBuilder('Magento\Framework\Controller\ResultFactory')
+            ->disableOriginalConstructor()
+            ->getMock();
+        $this->objectManagerMock = $this->getMockBuilder('Magento\Framework\ObjectManagerInterface')
+            ->getMock();
+        $this->messageManagerMock = $this->getMockBuilder('Magento\Framework\Message\ManagerInterface')
+            ->getMock();
+
+        $this->resultFactoryMock->expects($this->once())
             ->method('create')
-            ->with(\Magento\Framework\Controller\ResultFactory::TYPE_REDIRECT)
-            ->willReturn($this->resultRedirect);
-
-        $this->context = $this->getMock('Magento\Backend\App\Action\Context', [], [], '', false);
-        $this->context->expects($this->once())->method('getResultFactory')->willReturn($this->resultFactory);
-        $this->context->expects($this->once())->method('getRequest')->willReturn($this->request);
+            ->with(ResultFactory::TYPE_REDIRECT)
+            ->willReturn($this->resultRedirectMock);
+
+        $this->objectManagerHelper = new ObjectManagerHelper($this);
+        $this->context = $this->objectManagerHelper->getObject(
+            'Magento\Backend\App\Action\Context',
+            [
+                'resultFactory' => $this->resultFactoryMock,
+                'request' => $this->requestMock,
+                'messageManager' => $this->messageManagerMock,
+                'objectManager' => $this->objectManagerMock
+            ]
+        );
+        $this->saveController = $this->objectManagerHelper->getObject(
+            'Magento\Sitemap\Controller\Adminhtml\Sitemap\Save',
+            [
+                'context' => $this->context
+            ]
+        );
     }
 
     public function testSaveEmptyDataShouldRedirectToDefault()
     {
-        $this->request->expects($this->once())->method('getPostValue')->willReturn([]);
-        $this->resultRedirect->expects($this->once())->method('setPath')->with('adminhtml/*/')->willReturnSelf();
-
-        $this->controller = (new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this))
-            ->getObject('Magento\Sitemap\Controller\Adminhtml\Sitemap\Save', ['context' => $this->context]);
+        $this->requestMock->expects($this->once())
+            ->method('getPostValue')
+            ->willReturn([]);
+        $this->resultRedirectMock->expects($this->once())
+            ->method('setPath')
+            ->with('adminhtml/*/')
+            ->willReturnSelf();
 
-        $this->assertSame($this->resultRedirect, $this->controller->execute());
+        $this->assertSame($this->resultRedirectMock, $this->saveController->execute());
     }
 
     public function testTryToSaveInvalidDataShouldFailWithErrors()
@@ -69,47 +113,59 @@ public function testTryToSaveInvalidDataShouldFailWithErrors()
         $data = ['sitemap_filename' => 'sitemap_filename', 'sitemap_path' => '/sitemap_path'];
         $siteMapId = 1;
 
-        $this->request->expects($this->once())->method('getPostValue')->willReturn($data);
-        $this->request->expects($this->once())->method('getParam')->with('sitemap_id')->willReturn($siteMapId);
+        $this->requestMock->expects($this->once())
+            ->method('getPostValue')
+            ->willReturn($data);
+        $this->requestMock->expects($this->once())
+            ->method('getParam')
+            ->with('sitemap_id')
+            ->willReturn($siteMapId);
 
         $validator = $this->getMock($validatorClass, [], [], '', false);
-        $validator->expects($this->once())->method('setPaths')->with($validPaths);
+        $validator->expects($this->once())
+            ->method('setPaths')
+            ->with($validPaths)
+            ->willReturnSelf();
         $validator->expects($this->once())
             ->method('isValid')
             ->with('/sitemap_path/sitemap_filename')
             ->willReturn(false);
-        $validator->expects($this->once())->method('getMessages')->willReturn($messages);
+        $validator->expects($this->once())
+            ->method('getMessages')
+            ->willReturn($messages);
 
         $helper = $this->getMock($helperClass, [], [], '', false);
-        $helper->expects($this->once())->method('getValidPaths')->willReturn($validPaths);
+        $helper->expects($this->once())
+            ->method('getValidPaths')
+            ->willReturn($validPaths);
 
         $session = $this->getMock($sessionClass, ['setFormData'], [], '', false);
-        $session->expects($this->once())->method('setFormData')->with($data);
+        $session->expects($this->once())
+            ->method('setFormData')
+            ->with($data)
+            ->willReturnSelf();
 
-        $objectManager = $this->getMock('Magento\Framework\ObjectManagerInterface', [], [], '', false);
-        $objectManager->expects($this->once())
+        $this->objectManagerMock->expects($this->once())
             ->method('create')
             ->with($validatorClass)
             ->willReturn($validator);
-        $objectManager->expects($this->any())
+        $this->objectManagerMock->expects($this->any())
             ->method('get')
             ->willReturnMap([[$helperClass, $helper], [$sessionClass, $session]]);
 
-        $messageManager = $this->getMock('Magento\Framework\Message\ManagerInterface', [], [], '', false);
-        $messageManager->expects($this->at(0))->method('addError')->with($messages[0]);
-        $messageManager->expects($this->at(1))->method('addError')->with($messages[1]);
+        $this->messageManagerMock->expects($this->at(0))
+            ->method('addError')
+            ->withConsecutive(
+                [$messages[0]],
+                [$messages[1]]
+            )
+            ->willReturnSelf();
 
-        $this->resultRedirect->expects($this->once())
+        $this->resultRedirectMock->expects($this->once())
             ->method('setPath')
             ->with('adminhtml/*/edit', ['sitemap_id' => $siteMapId])
             ->willReturnSelf();
 
-        $this->context->expects($this->once())->method('getObjectManager')->willReturn($objectManager);
-        $this->context->expects($this->once())->method('getMessageManager')->willReturn($messageManager);
-
-        $this->controller = (new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this))
-            ->getObject('Magento\Sitemap\Controller\Adminhtml\Sitemap\Save', ['context' => $this->context]);
-
-        $this->assertSame($this->resultRedirect, $this->controller->execute());
+        $this->assertSame($this->resultRedirectMock, $this->saveController->execute());
     }
 }
diff --git a/app/code/Magento/Widget/Model/NamespaceResolver.php b/app/code/Magento/Widget/Model/NamespaceResolver.php
@@ -86,10 +86,7 @@ protected function prepareModuleNamespaces()
      */
     protected function prepareName($name)
     {
-        $explodeString = strpos(
-            $name,
-            '\\'
-        ) === false ? '_' : '\\';
+        $explodeString = strpos($name, '\\') === false ? '_' : '\\';
         return explode($explodeString, strtolower($name));
     }
 }
diff --git a/app/code/Magento/Widget/Model/Widget.php b/app/code/Magento/Widget/Model/Widget.php
@@ -359,7 +359,7 @@ public function getPlaceholderImageUrls()
     }
 
     /**
-     * Remove attributes from widget array and emulates how \Magento\Framework\Simplexml\Element::asCanonicalArray works
+     * Remove attributes from widget array and emulate work of \Magento\Framework\Simplexml\Element::asCanonicalArray
      *
      * @param array $inputArray
      * @return array
diff --git a/app/code/Magento/Widget/Test/Unit/Model/WidgetTest.php b/app/code/Magento/Widget/Test/Unit/Model/WidgetTest.php
@@ -23,7 +23,6 @@ public function setUp()
             ->disableOriginalConstructor()
             ->getMock();
         $objectManagerHelper = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this);
-        $objectManagerHelper->getObject('Magento\Widget\Model\Widget', ['dataStorage' => $this->dataStorageMock]);
         $this->widget = $objectManagerHelper->getObject(
             'Magento\Widget\Model\Widget',
             ['dataStorage' => $this->dataStorageMock]

Original file line number	Diff line number	Diff line change
`@@ -86,10 +86,7 @@ protected function prepareModuleNamespaces()`
`86`	`86`	`*/`
`87`	`87`	`protected function prepareName($name)`
`88`	`88`	`{`
`89`		`- $explodeString = strpos(`
`90`		`- $name,`
`91`		`- '\\'`
`92`		`- ) === false ? '_' : '\\';`
	`89`	`+ $explodeString = strpos($name, '\\') === false ? '_' : '\\';`
`93`	`90`	`return explode($explodeString, strtolower($name));`
`94`	`91`	`}`
`95`	`92`	`}`
Original file line number	Diff line number	Diff line change
`@@ -359,7 +359,7 @@ public function getPlaceholderImageUrls()`
`359`	`359`	`}`
`360`	`360`
`361`	`361`	`/**`
`362`		`- * Remove attributes from widget array and emulates how \Magento\Framework\Simplexml\Element::asCanonicalArray works`
	`362`	`+ * Remove attributes from widget array and emulate work of \Magento\Framework\Simplexml\Element::asCanonicalArray`
`363`	`363`	`*`
`364`	`364`	`* @param array $inputArray`
`365`	`365`	`* @return array`