From 8453f0b184b182aebfa94fc9f761655a3219774e Mon Sep 17 00:00:00 2001 From: Stanislav Lopukhov Date: Mon, 3 Aug 2015 13:05:43 +0300 Subject: [PATCH 01/20] MAGETWO-40881: Redirect "_setup" connection to requested connection --- lib/internal/Magento/Framework/App/Resource/Config.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/internal/Magento/Framework/App/Resource/Config.php b/lib/internal/Magento/Framework/App/Resource/Config.php index 27a611265bc06..561718c5a6643 100644 --- a/lib/internal/Magento/Framework/App/Resource/Config.php +++ b/lib/internal/Magento/Framework/App/Resource/Config.php @@ -54,6 +54,8 @@ public function getConnectionName($resourceName) { $connectionName = \Magento\Framework\App\Resource::DEFAULT_CONNECTION; + $resourceName = preg_replace("/_setup$/", '', $resourceName); + if (!isset($this->_connectionNames[$resourceName])) { $resourcesConfig = $this->get(); $pointerResourceName = $resourceName; From 7c2de5cade888dcf7476bbe7208ebab8e9d09f30 Mon Sep 17 00:00:00 2001 From: Volodymyr Kholoshenko Date: Tue, 4 Aug 2015 13:37:12 +0300 Subject: [PATCH 02/20] MAGETWO-41084: Framework\App\Resource refactoring --- .../Magento/Framework/App/Resource.php | 49 ++++++------- .../Framework/App/Test/Unit/ResourceTest.php | 72 ++++++++++--------- .../Resource/Type/Db/ConnectionFactory.php | 1 + 3 files changed, 63 insertions(+), 59 deletions(-) diff --git a/lib/internal/Magento/Framework/App/Resource.php b/lib/internal/Magento/Framework/App/Resource.php index 782b57436578a..ffa6d2af4781e 100644 --- a/lib/internal/Magento/Framework/App/Resource.php +++ b/lib/internal/Magento/Framework/App/Resource.php @@ -27,28 +27,28 @@ class Resource * * @var \Magento\Framework\DB\Adapter\AdapterInterface[] */ - protected $_connections = []; + protected $connections = []; /** * Mapped tables cache array * * @var array */ - protected $_mappedTableNames; + protected $mappedTableNames; /** * Resource config * * @var ResourceConfigInterface */ - protected $_config; + protected $config; /** * Resource connection adapter factory * * @var ConnectionFactoryInterface */ - protected $_connectionFactory; + protected $connectionFactory; /** * @var DeploymentConfig $deploymentConfig @@ -58,7 +58,7 @@ class Resource /** * @var string */ - protected $_tablePrefix; + protected $tablePrefix; /** * @param ResourceConfigInterface $resourceConfig @@ -72,22 +72,23 @@ public function __construct( DeploymentConfig $deploymentConfig, $tablePrefix = '' ) { - $this->_config = $resourceConfig; - $this->_connectionFactory = $connectionFactory; + $this->config = $resourceConfig; + $this->connectionFactory = $connectionFactory; $this->deploymentConfig = $deploymentConfig; - $this->_tablePrefix = $tablePrefix ?: null; + $this->tablePrefix = $tablePrefix ?: null; } /** * Retrieve connection to resource specified by $resourceName * * @param string $resourceName - * @return \Magento\Framework\DB\Adapter\AdapterInterface|false + * @return \Magento\Framework\DB\Adapter\AdapterInterface + * @throws \DomainException * @codeCoverageIgnore */ public function getConnection($resourceName = self::DEFAULT_CONNECTION) { - $connectionName = $this->_config->getConnectionName($resourceName); + $connectionName = $this->config->getConnectionName($resourceName); return $this->getConnectionByName($connectionName); } @@ -95,12 +96,13 @@ public function getConnection($resourceName = self::DEFAULT_CONNECTION) * Retrieve connection by $connectionName * * @param string $connectionName - * @return bool|\Magento\Framework\DB\Adapter\AdapterInterface + * @return \Magento\Framework\DB\Adapter\AdapterInterface + * @throws \DomainException */ public function getConnectionByName($connectionName) { - if (isset($this->_connections[$connectionName])) { - return $this->_connections[$connectionName]; + if (isset($this->connections[$connectionName])) { + return $this->connections[$connectionName]; } $connectionConfig = $this->deploymentConfig->get( @@ -108,13 +110,12 @@ public function getConnectionByName($connectionName) ); if ($connectionConfig) { - $connection = $this->_connectionFactory->create($connectionConfig); - } - if (empty($connection)) { - return false; + $connection = $this->connectionFactory->create($connectionConfig); + } else { + throw new \DomainException('Connection "' . $connectionName . '" is not defined'); } - $this->_connections[$connectionName] = $connection; + $this->connections[$connectionName] = $connection; return $connection; } @@ -174,7 +175,7 @@ public function getTriggerName($tableName, $time, $event) */ public function setMappedTableName($tableName, $mappedName) { - $this->_mappedTableNames[$tableName] = $mappedName; + $this->mappedTableNames[$tableName] = $mappedName; return $this; } @@ -186,8 +187,8 @@ public function setMappedTableName($tableName, $mappedName) */ public function getMappedTableName($tableName) { - if (isset($this->_mappedTableNames[$tableName])) { - return $this->_mappedTableNames[$tableName]; + if (isset($this->mappedTableNames[$tableName])) { + return $this->mappedTableNames[$tableName]; } else { return false; } @@ -240,11 +241,11 @@ public function getFkName($priTableName, $priColumnName, $refTableName, $refColu */ private function getTablePrefix() { - if (null === $this->_tablePrefix) { - $this->_tablePrefix = (string)$this->deploymentConfig->get( + if (null === $this->tablePrefix) { + $this->tablePrefix = (string)$this->deploymentConfig->get( ConfigOptionsListConstants::CONFIG_PATH_DB_PREFIX ); } - return $this->_tablePrefix; + return $this->tablePrefix; } } diff --git a/lib/internal/Magento/Framework/App/Test/Unit/ResourceTest.php b/lib/internal/Magento/Framework/App/Test/Unit/ResourceTest.php index e579bc576bad7..f48b84c57b5b4 100644 --- a/lib/internal/Magento/Framework/App/Test/Unit/ResourceTest.php +++ b/lib/internal/Magento/Framework/App/Test/Unit/ResourceTest.php @@ -4,12 +4,11 @@ * See COPYING.txt for license details. */ -// @codingStandardsIgnoreFile - namespace Magento\Framework\App\Test\Unit; -use \Magento\Framework\App\Resource; +use Magento\Framework\App\Resource; use Magento\Framework\Config\ConfigOptionsListConstants; +use Magento\Framework\Model\Resource\Type\Db\ConnectionFactoryInterface; class ResourceTest extends \PHPUnit_Framework_TestCase { @@ -20,12 +19,12 @@ class ResourceTest extends \PHPUnit_Framework_TestCase /** * @var \Magento\Framework\App\Resource\ConfigInterface|\PHPUnit_Framework_MockObject_MockObject */ - protected $_config; + protected $config; /** - * @var \Magento\Framework\Model\Resource\Type\Db\ConnectionFactory|\PHPUnit_Framework_MockObject_MockObject + * @var ConnectionFactoryInterface|\PHPUnit_Framework_MockObject_MockObject */ - protected $_connectionFactory; + protected $connectionFactory; /** * @var \Magento\Framework\App\DeploymentConfig|\PHPUnit_Framework_MockObject_MockObject @@ -44,36 +43,39 @@ class ResourceTest extends \PHPUnit_Framework_TestCase public function setUp() { - $this->_connectionFactory = $this->getMockBuilder('Magento\Framework\Model\Resource\Type\Db\ConnectionFactory') - ->disableOriginalConstructor() + $this->connectionFactory = $this->getMockBuilder(ConnectionFactoryInterface::class) ->setMethods(['create']) - ->getMock(); - $this->_config = $this->getMockBuilder('Magento\Framework\App\Resource\ConfigInterface') + ->getMockForAbstractClass(); + $this->config = $this->getMockBuilder('Magento\Framework\App\Resource\ConfigInterface') ->disableOriginalConstructor() ->setMethods(['getConnectionName']) ->getMock(); - $this->_config->expects($this->any()) + $this->config->expects($this->any()) ->method('getConnectionName') ->with(self::RESOURCE_NAME) ->will($this->returnValue(self::CONNECTION_NAME)); $this->deploymentConfig = $this->getMock('Magento\Framework\App\DeploymentConfig', [], [], '', false); - $this->deploymentConfig->expects($this->any()) + $this->deploymentConfig + ->expects($this->any()) ->method('get') - ->will($this->returnValue( + ->willReturnMap( + [ [ - 'default' => [ - 'host' => 'localhost', - 'dbname' => 'magento', - 'username' => 'username', - ], - self::CONNECTION_NAME => [ + ConfigOptionsListConstants::CONFIG_PATH_DB_CONNECTIONS . '/connection-name', + null, + [ 'host' => 'localhost', 'dbname' => 'magento', 'username' => 'username', - ], + ] + ], + [ + ConfigOptionsListConstants::CONFIG_PATH_DB_PREFIX, + null, + self::TABLE_PREFIX ] - ) + ] ); $this->connection = $this->getMockForAbstractClass('Magento\Framework\DB\Adapter\AdapterInterface'); @@ -82,24 +84,24 @@ public function setUp() ->will($this->returnArgument(0)); $this->resource = new Resource( - $this->_config, - $this->_connectionFactory, - $this->deploymentConfig, - self::TABLE_PREFIX + $this->config, + $this->connectionFactory, + $this->deploymentConfig ); } + /** + * @expectedException \DomainException + * @expectedExceptionMessage Connection "invalid" is not defined + */ public function testGetConnectionFail() { - $this->_connectionFactory->expects($this->once()) - ->method('create') - ->will($this->returnValue(null)); - $this->assertFalse($this->resource->getConnection(self::RESOURCE_NAME)); + $this->resource->getConnectionByName('invalid'); } public function testGetConnectionInitConnection() { - $this->_connectionFactory->expects($this->once()) + $this->connectionFactory->expects($this->once()) ->method('create') ->will($this->returnValue($this->connection)); $this->assertSame($this->connection, $this->resource->getConnection(self::RESOURCE_NAME)); @@ -114,7 +116,7 @@ public function testGetConnectionInitConnection() */ public function testGetTableName($modelEntity, $expected) { - $this->_connectionFactory->expects($this->once()) + $this->connectionFactory->expects($this->once()) ->method('create') ->will($this->returnValue($this->connection)); $this->assertSame($expected, $this->resource->getTableName($modelEntity)); @@ -141,7 +143,7 @@ public function getTableNameDataProvider() */ public function testGetTableNameMapped($modelEntity, $tableName, $mappedName, $expected) { - $this->_connectionFactory->expects($this->once()) + $this->connectionFactory->expects($this->once()) ->method('create') ->will($this->returnValue($this->connection)); $this->resource->setMappedTableName($tableName, $mappedName); @@ -171,7 +173,7 @@ public function testGetIdxName() ->method('getIndexName') ->with($calculatedTableName, $fields, $indexType) ->will($this->returnValue($expectedIdxName)); - $this->_connectionFactory->expects($this->once()) + $this->connectionFactory->expects($this->once()) ->method('create') ->will($this->returnValue($this->connection)); @@ -191,7 +193,7 @@ public function testGetFkName() ->method('getForeignKeyName') ->with($calculatedTableName, $columnName, $calculatedRefTableName, $refColumnName) ->will($this->returnValue('fkName')); - $this->_connectionFactory->expects($this->once()) + $this->connectionFactory->expects($this->once()) ->method('create') ->will($this->returnValue($this->connection)); @@ -205,7 +207,7 @@ public function testGetTriggerName() $event = 'insert'; $triggerName = 'trg_subject_table_before_insert'; - $this->_connectionFactory->expects($this->once()) + $this->connectionFactory->expects($this->once()) ->method('create') ->will($this->returnValue($this->connection)); $this->connection->expects($this->once()) diff --git a/lib/internal/Magento/Framework/Model/Resource/Type/Db/ConnectionFactory.php b/lib/internal/Magento/Framework/Model/Resource/Type/Db/ConnectionFactory.php index 0ddbfb420bd27..3a404d045eacb 100644 --- a/lib/internal/Magento/Framework/Model/Resource/Type/Db/ConnectionFactory.php +++ b/lib/internal/Magento/Framework/Model/Resource/Type/Db/ConnectionFactory.php @@ -31,6 +31,7 @@ public function __construct(ObjectManagerInterface $objectManager) */ public function create(array $connectionConfig) { + /** @var \Magento\Framework\App\Resource\ConnectionAdapterInterface $adapterInstance */ $adapterInstance = $this->objectManager->create( \Magento\Framework\App\Resource\ConnectionAdapterInterface::class, ['config' => $connectionConfig] From 9ddcaf11d63bf17f27ec08bb56dc18af91d0ae1a Mon Sep 17 00:00:00 2001 From: Stanislav Lopukhov Date: Wed, 5 Aug 2015 15:32:38 +0300 Subject: [PATCH 03/20] MAGETWO-40884: DB/Adapter/Pdo/Mysql.php update --- lib/internal/Magento/Framework/DB/Adapter/Pdo/Mysql.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/internal/Magento/Framework/DB/Adapter/Pdo/Mysql.php b/lib/internal/Magento/Framework/DB/Adapter/Pdo/Mysql.php index 2ab2fdf21be1b..d730e40ad36a7 100644 --- a/lib/internal/Magento/Framework/DB/Adapter/Pdo/Mysql.php +++ b/lib/internal/Magento/Framework/DB/Adapter/Pdo/Mysql.php @@ -177,10 +177,11 @@ class Mysql extends \Zend_Db_Adapter_Pdo_Mysql implements AdapterInterface * @var DateTime */ protected $dateTime; + /** * @var LoggerInterface */ - private $logger; + protected $logger; /** * @param \Magento\Framework\Stdlib\StringUtils|String $string From 3121e017c7b288d90adb75c8bfe716442ca5cd7a Mon Sep 17 00:00:00 2001 From: Kateryna Muntianu Date: Thu, 6 Aug 2015 15:33:01 +0300 Subject: [PATCH 04/20] MAGETWO-40697: [Optional] Create test to deprecate direct Zend* classes usage except specific lib DB wrappers --- .../TestFramework/Utility/CodeCheck.php | 150 ++++++++++++++++++ .../Magento/Test/Legacy/ObsoleteCodeTest.php | 43 +++-- .../Test/Legacy/RestrictedCodeTest.php | 124 +++++++++++++++ .../Test/Legacy/_files/restricted_classes.php | 29 ++++ 4 files changed, 322 insertions(+), 24 deletions(-) create mode 100644 dev/tests/static/framework/Magento/TestFramework/Utility/CodeCheck.php create mode 100644 dev/tests/static/testsuite/Magento/Test/Legacy/RestrictedCodeTest.php create mode 100644 dev/tests/static/testsuite/Magento/Test/Legacy/_files/restricted_classes.php diff --git a/dev/tests/static/framework/Magento/TestFramework/Utility/CodeCheck.php b/dev/tests/static/framework/Magento/TestFramework/Utility/CodeCheck.php new file mode 100644 index 0000000000000..907a972becd04 --- /dev/null +++ b/dev/tests/static/framework/Magento/TestFramework/Utility/CodeCheck.php @@ -0,0 +1,150 @@ +|function\s)' . $quotedMethod . '\s*\(/iS', + $content + ); + } + // without opening parentheses to match static callbacks notation + if (self::_isRegexpMatched( + '/' . preg_quote($class, '/') . '::\s*' . $quotedMethod . '[^a-z\d_]/iS', + $content + ) + ) { + return true; + } + if (self::isClassOrInterface($content, $class) || self::isDirectDescendant($content, $class)) { + return self::_isRegexpMatched('/this->' . $quotedMethod . '\s*\(/iS', $content) + || self::_isRegexpMatched( + '/(self|static|parent)::\s*' . $quotedMethod . '\s*\(/iS', + $content + ); + } + } + + /** + * Check if methods or functions are used in the content + * + * If class context is specified, only the class and its descendants will be checked. + * + * @param string $property + * @param string $content + * @param string $class + * @return bool + */ + public static function isPropertyUsed($property, $content, $class = null) + { + if ($class) { + if (!self::isClassOrInterface($content, $class) && !self::isDirectDescendant($content, $class)) { + return false; + } + } + return self::_isRegexpMatched( + '/[^a-z\d_]' . preg_quote($property, '/') . '[^a-z\d_]/iS', + $content + ); + } + + /** + * Analyze content to determine whether it is a declaration of the specified class/interface + * + * @param string $content + * @param string $name + * @return bool + */ + public static function isClassOrInterface($content, $name) + { + return self::_isRegexpMatched('/\b(?:class|interface)\s+' . preg_quote($name, '/') . '\b[^{]*\{/iS', $content); + } + + /** + * Analyze content to determine whether this is a direct descendant of the specified class/interface + * + * @param string $content + * @param string $name + * @return bool + */ + public static function isDirectDescendant($content, $name) + { + $name = preg_quote($name, '/'); + return self::_isRegexpMatched( + '/\s+extends\s+' . $name . '\b|\s+implements\s+[^{]*\b' . $name . '\b[^{^\\\\]*\{/iS', + $content + ); + } + + /** + * Check if content matches the regexp + * + * @param string $regexp + * @param string $content + * @throws \Exception + * @return bool True if the content matches the regexp + */ + protected static function _isRegexpMatched($regexp, $content) + { + $result = preg_match($regexp, $content); + if ($result === false) { + throw new \Exception('An error occurred when matching regexp "' . $regexp . '""'); + } + return 1 === $result; + } +} diff --git a/dev/tests/static/testsuite/Magento/Test/Legacy/ObsoleteCodeTest.php b/dev/tests/static/testsuite/Magento/Test/Legacy/ObsoleteCodeTest.php index a6a2253c57e5d..091472e472f40 100644 --- a/dev/tests/static/testsuite/Magento/Test/Legacy/ObsoleteCodeTest.php +++ b/dev/tests/static/testsuite/Magento/Test/Legacy/ObsoleteCodeTest.php @@ -12,11 +12,6 @@ class ObsoleteCodeTest extends \PHPUnit_Framework_TestCase { - /** - * Message text that is used to render suggestions - */ - const SUGGESTION_MESSAGE = 'Use "%s" instead.'; - /**@#+ * Lists of obsolete entities from fixtures * @@ -427,11 +422,11 @@ protected function _testObsoleteConstants($content) list($constant, $class, $replacement) = $row; if ($class) { $class = ltrim($class, '\\'); - $this->checkConstantWithFullClasspath($constant, $class, $replacement, $content); - $this->checkConstantWithClasspath($constant, $class, $replacement, $content); + $this->_checkConstantWithFullClasspath($constant, $class, $replacement, $content); + $this->_checkConstantWithClasspath($constant, $class, $replacement, $content); } else { $regex = '\b' . preg_quote($constant, '/') . '\b'; - $this->checkExistenceOfObsoleteConstants($regex, '', $content, $constant, $replacement, $class); + $this->_checkExistenceOfObsoleteConstants($regex, '', $content, $constant, $replacement, $class); } } } @@ -469,11 +464,11 @@ private function buildRegExFromObsoleteConstant($classPartialPath, $content, $co * @param string $replacement * @param string $content */ - private function checkConstantWithFullClasspath($constant, $class, $replacement, $content) + private function _checkConstantWithFullClasspath($constant, $class, $replacement, $content) { $constantRegex = preg_quote($constant, '/'); $classRegex = preg_quote($class); - $this->checkExistenceOfObsoleteConstants( + $this->_checkExistenceOfObsoleteConstants( $constantRegex, $classRegex, $content, @@ -491,7 +486,7 @@ private function checkConstantWithFullClasspath($constant, $class, $replacement, * @param string $replacement * @param string $content */ - private function checkConstantWithClasspath($constant, $class, $replacement, $content) + private function _checkConstantWithClasspath($constant, $class, $replacement, $content) { $classPathParts = explode('\\', $class); $classPartialPath = ''; @@ -521,7 +516,7 @@ private function checkConstantWithClasspath($constant, $class, $replacement, $co $this->_suggestReplacement(sprintf("Constant '%s' is obsolete.", $constant), $replacement) ); } else { - $this->checkExistenceOfObsoleteConstants( + $this->_checkExistenceOfObsoleteConstants( $constantRegex, $classRegex, $content, @@ -543,7 +538,7 @@ private function checkConstantWithClasspath($constant, $class, $replacement, $co * @param string $replacement * @param string $class */ - private function checkExistenceOfObsoleteConstants( + private function _checkExistenceOfObsoleteConstants( $constantRegex, $classRegex, $content, @@ -562,14 +557,14 @@ private function checkExistenceOfObsoleteConstants( $matchClass = preg_match($classRegexFull, $content, $matchClassString); if ($matchClass === 1) { if ($matchClassString['classAlias']) { - $result = $this->checkAliasUseNamespace( + $result = $this->_checkAliasUseNamespace( $constantRegex, $matchConstantString, $matchClassString, $class ); } else { - $result = $this->checkNoAliasUseNamespace($matchConstantString, $matchClassString, $class); + $result = $this->_checkNoAliasUseNamespace($matchConstantString, $matchClassString, $class); } } else { foreach ($matchConstantString['classWithConst'] as $constantMatch) { @@ -600,7 +595,7 @@ private function checkExistenceOfObsoleteConstants( * @param string $class * @return int */ - private function checkAliasUseNamespace( + private function _checkAliasUseNamespace( $constantRegex, $matchConstantString, $matchClassString, @@ -618,7 +613,7 @@ private function checkAliasUseNamespace( $foundAsComponent = true; } if (strpos($constantMatch, '::') !== false) { - $foundProperUse = $this->checkCompletePathOfClass( + $foundProperUse = $this->_checkCompletePathOfClass( $constantMatch, $matchClassString, $class, @@ -645,14 +640,14 @@ private function checkAliasUseNamespace( * @param string $class * @return int */ - private function checkNoAliasUseNamespace( + private function _checkNoAliasUseNamespace( $matchConstantString, $matchClassString, $class ) { $foundProperUse = false; foreach ($matchConstantString['constPart'] as $constantMatch) { - $foundProperUse = $this->checkCompletePathOfClass( + $foundProperUse = $this->_checkCompletePathOfClass( $constantMatch, $matchClassString, $class @@ -678,7 +673,7 @@ private function checkNoAliasUseNamespace( * @param string $asComponent * @return bool */ - private function checkCompletePathOfClass( + private function _checkCompletePathOfClass( $constantMatch, $matchClassString, $class, @@ -706,7 +701,7 @@ private function checkCompletePathOfClass( ), '\\' )); - if ($this->checkClasspathProperDivisionNoConstantPath( + if ($this->_checkClasspathProperDivisionNoConstantPath( $pathInUseNamespaceTruncated, $pathInUseNamespace, $matchClassString, @@ -715,7 +710,7 @@ private function checkCompletePathOfClass( )) { return true; } else { - return $this->checkClasspathProperDivisionWithConstantPath( + return $this->_checkClasspathProperDivisionWithConstantPath( $pathInUseNamespaceTruncated, $pathInUseNamespace, $pathWithConst, @@ -735,7 +730,7 @@ private function checkCompletePathOfClass( * @param bool $foundAsComponent * @return bool */ - private function checkClasspathProperDivisionNoConstantPath( + private function _checkClasspathProperDivisionNoConstantPath( $pathInUseNamespaceTruncated, $pathInUseNamespace, $matchClassString, @@ -760,7 +755,7 @@ private function checkClasspathProperDivisionNoConstantPath( * @param bool $foundAsComponent * @return bool */ - private function checkClasspathProperDivisionWithConstantPath( + private function _checkClasspathProperDivisionWithConstantPath( $pathInUseNamespaceTruncated, $pathInUseNamespace, $pathWithConst, diff --git a/dev/tests/static/testsuite/Magento/Test/Legacy/RestrictedCodeTest.php b/dev/tests/static/testsuite/Magento/Test/Legacy/RestrictedCodeTest.php new file mode 100644 index 0000000000000..8280ee586a4bf --- /dev/null +++ b/dev/tests/static/testsuite/Magento/Test/Legacy/RestrictedCodeTest.php @@ -0,0 +1,124 @@ +getPathToSource(), '', $file) + ); + array_push(self::$_fixtureFiles, $relativePath); + $data = array_merge($data, self::_readList($file)); + } + } + + /** + * Isolate including a file into a method to reduce scope + * + * @param string $file + * @return array + */ + protected static function _readList($file) + { + return include $file; + } + + /** + * Test that restricted entities are not used in PHP files + * @return void + */ + public function testPhpFiles() + { + $invoker = new \Magento\Framework\App\Utility\AggregateInvoker($this); + $testFiles = \Magento\TestFramework\Utility\ChangedFiles::getPhpFiles(__DIR__ . '/_files/changed_files*'); + foreach (self::$_fixtureFiles as $fixtureFile) { + if (array_key_exists($fixtureFile, $testFiles)) { + unset($testFiles[$fixtureFile]); + } + } + $invoker( + function ($file) { + $this->_testRestrictedClasses($file); + }, + $testFiles + ); + } + + /** + * Assert that restricted classes are not used in the file + * + * @param string $file + * @return void + */ + protected function _testRestrictedClasses($file) + { + $content = file_get_contents($file); + foreach (self::$_classes as $row) { + list($class, $replacement, $whiteList) = $row; + foreach ($whiteList as $skippedPath) { + if ($this->_isFileInPath($skippedPath, $file)) { + continue 2; + } + } + $this->assertFalse( + \Magento\TestFramework\Utility\CodeCheck::isClassUsed($class, $content), + sprintf("Class '%s' is restricted. Suggested replacement: %s", $class, $replacement) + ); + } + } + + /** + * Checks if the file path (relative to Magento folder) starts with the given path + * + * @param string $subPath + * @param string $file + * @return bool + */ + protected function _isFileInPath($subPath, $file) + { + $relativePath = str_replace(\Magento\Framework\App\Utility\Files::init()->getPathToSource(), '', $file); + return 0 === strpos($relativePath, $subPath); + } +} diff --git a/dev/tests/static/testsuite/Magento/Test/Legacy/_files/restricted_classes.php b/dev/tests/static/testsuite/Magento/Test/Legacy/_files/restricted_classes.php new file mode 100644 index 0000000000000..ff9eb9c2b1527 --- /dev/null +++ b/dev/tests/static/testsuite/Magento/Test/Legacy/_files/restricted_classes.php @@ -0,0 +1,29 @@ + will be suggested to be used instead. + * Use to specify files and directories that are allowed to use restricted classes. + * + * Format: array(, [, array()]]) + * + * Copyright © 2015 Magento. All rights reserved. + * See COPYING.txt for license details. + */ +return [ + [ + 'Zend_Db_Select', + '\Magento\Framework\DB\Select', + [ + '/lib/internal/Magento/Framework/DB/Select.php', + '/lib/internal/Magento/Framework/DB/Adapter/Pdo/Mysql.php', + '/lib/internal/Magento/Framework/Model/Resource/Iterator.php', + ] + ], + [ + 'Zend_Db_Adapter_Pdo_Mysql', + '\Magento\Framework\DB\Adapter\Pdo\Mysql', + [ + '/lib/internal/Magento/Framework/DB/Adapter/Pdo/Mysql.php', + ] + ], +]; From e5c17ac057c623253f06cdd6fe5268f23b2975a7 Mon Sep 17 00:00:00 2001 From: Stanislav Lopukhov Date: Fri, 7 Aug 2015 19:39:15 +0300 Subject: [PATCH 05/20] MAGETWO-40884: Update TestFramework/ObjectManagerFactory.php --- .../integration/etc/di/preferences/ce.php | 21 +++++++++++++ .../TestFramework/ObjectManagerFactory.php | 30 +++++++++---------- 2 files changed, 35 insertions(+), 16 deletions(-) create mode 100644 dev/tests/integration/etc/di/preferences/ce.php diff --git a/dev/tests/integration/etc/di/preferences/ce.php b/dev/tests/integration/etc/di/preferences/ce.php new file mode 100644 index 0000000000000..52573b8d3d289 --- /dev/null +++ b/dev/tests/integration/etc/di/preferences/ce.php @@ -0,0 +1,21 @@ + 'Magento\TestFramework\CookieManager', + 'Magento\Framework\ObjectManager\DynamicConfigInterface' => + '\Magento\TestFramework\ObjectManager\Configurator', + 'Magento\Framework\App\RequestInterface' => 'Magento\TestFramework\Request', + 'Magento\Framework\App\Request\Http' => 'Magento\TestFramework\Request', + 'Magento\Framework\App\ResponseInterface' => 'Magento\TestFramework\Response', + 'Magento\Framework\App\Response\Http' => 'Magento\TestFramework\Response', + 'Magento\Framework\Interception\PluginListInterface' => + 'Magento\TestFramework\Interception\PluginList', + 'Magento\Framework\Interception\ObjectManager\Config' => + 'Magento\TestFramework\ObjectManager\Config', + 'Magento\Framework\View\LayoutInterface' => 'Magento\TestFramework\View\Layout', + 'Magento\Framework\App\Resource\ConnectionAdapterInterface' => 'Magento\TestFramework\Db\ConnectionAdapter', +]; diff --git a/dev/tests/integration/framework/Magento/TestFramework/ObjectManagerFactory.php b/dev/tests/integration/framework/Magento/TestFramework/ObjectManagerFactory.php index 763011a315b36..e7bdb275e88ce 100644 --- a/dev/tests/integration/framework/Magento/TestFramework/ObjectManagerFactory.php +++ b/dev/tests/integration/framework/Magento/TestFramework/ObjectManagerFactory.php @@ -89,24 +89,22 @@ protected function _loadPrimaryConfig(DirectoryList $directoryList, $driverPool, 'default_setup' => ['type' => 'Magento\TestFramework\Db\ConnectionAdapter'] ] ); + $diPreferences = []; + $diPreferencesPath = __DIR__ . '/../../../etc/di/preferences/'; + $directory = new \RecursiveDirectoryIterator($diPreferencesPath, \FilesystemIterator::SKIP_DOTS); + + foreach (new \RecursiveIteratorIterator($directory) as $file) { + if (!is_readable($file->getPathname())) { + throw new \Magento\Framework\Exception\LocalizedException( + __("'%1' is not readable file.", $file->getPathname()) + ); + } + $diPreferences = array_replace($diPreferences, include $file->getPathname()); + } + $this->_primaryConfigData['preferences'] = array_replace( $this->_primaryConfigData['preferences'], - [ - 'Magento\Framework\Stdlib\CookieManagerInterface' => 'Magento\TestFramework\CookieManager', - 'Magento\Framework\ObjectManager\DynamicConfigInterface' => - '\Magento\TestFramework\ObjectManager\Configurator', - 'Magento\Framework\App\RequestInterface' => 'Magento\TestFramework\Request', - 'Magento\Framework\App\Request\Http' => 'Magento\TestFramework\Request', - 'Magento\Framework\App\ResponseInterface' => 'Magento\TestFramework\Response', - 'Magento\Framework\App\Response\Http' => 'Magento\TestFramework\Response', - 'Magento\Framework\Interception\PluginListInterface' => - 'Magento\TestFramework\Interception\PluginList', - 'Magento\Framework\Interception\ObjectManager\Config' => - 'Magento\TestFramework\ObjectManager\Config', - 'Magento\Framework\View\LayoutInterface' => 'Magento\TestFramework\View\Layout', - 'Magento\Framework\App\Resource\ConnectionAdapterInterface' => - 'Magento\TestFramework\Db\ConnectionAdapter', - ] + $diPreferences ); } return $this->_primaryConfigData; From 1e440de81e1713c1742ca309f33fb95ec7096095 Mon Sep 17 00:00:00 2001 From: Stanislav Lopukhov Date: Mon, 10 Aug 2015 16:13:00 +0300 Subject: [PATCH 06/20] MAGETWO-40884: Update App/Request/Http.php --- .../Magento/Framework/App/Request/Http.php | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/lib/internal/Magento/Framework/App/Request/Http.php b/lib/internal/Magento/Framework/App/Request/Http.php index 18502821429cf..0e0ea69a2148c 100644 --- a/lib/internal/Magento/Framework/App/Request/Http.php +++ b/lib/internal/Magento/Framework/App/Request/Http.php @@ -77,6 +77,16 @@ class Http extends Request implements RequestInterface */ protected $objectManager; + /** + * @var bool|null + */ + protected $isSafeMethod = null; + + /** + * @var array + */ + protected $safeRequestTypes = ['GET', 'HEAD', 'TRACE', 'CONNECT']; + /** * @param CookieReaderInterface $cookieReader * @param ConfigInterface $routeConfig @@ -411,6 +421,23 @@ public function isSecure() return $this->initialRequestSecure($offLoaderHeader); } + /** + * Check that this is safe request + * + * @return bool + */ + public function isSafeMethod() + { + if ($this->isSafeMethod === null) { + if (isset($_SERVER['REQUEST_METHOD']) && (in_array($_SERVER['REQUEST_METHOD'], $this->safeRequestTypes))) { + $this->isSafeMethod = true; + } else { + $this->isSafeMethod = false; + } + } + return $this->isSafeMethod; + } + /** * Checks if the immediate request is delivered over HTTPS * From fc08fb9e6ddc1a4ccb354c15b3ddcc10afbb22b5 Mon Sep 17 00:00:00 2001 From: Stanislav Lopukhov Date: Tue, 11 Aug 2015 11:53:29 +0300 Subject: [PATCH 07/20] MAGETWO-40884: Move 'initStatements' apply to Pdo/Mysql.php --- lib/internal/Magento/Framework/DB/Adapter/Pdo/Mysql.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/internal/Magento/Framework/DB/Adapter/Pdo/Mysql.php b/lib/internal/Magento/Framework/DB/Adapter/Pdo/Mysql.php index d730e40ad36a7..285e41e550bde 100644 --- a/lib/internal/Magento/Framework/DB/Adapter/Pdo/Mysql.php +++ b/lib/internal/Magento/Framework/DB/Adapter/Pdo/Mysql.php @@ -335,6 +335,10 @@ protected function _connect() /** @link http://bugs.mysql.com/bug.php?id=18551 */ $this->_connection->query("SET SQL_MODE=''"); + if (isset($this->_config['initStatements'])) { + $this->query($this->_config['initStatements']); + } + if (!$this->_connectionFlagsSet) { $this->_connection->setAttribute(\PDO::ATTR_EMULATE_PREPARES, true); $this->_connection->setAttribute(\PDO::MYSQL_ATTR_USE_BUFFERED_QUERY, true); From 208c013acf1f656afadff09a8b6ec128786d007a Mon Sep 17 00:00:00 2001 From: Egor Shitikov Date: Tue, 11 Aug 2015 13:36:38 +0300 Subject: [PATCH 08/20] MAGETWO-41221: Recheck unit tests coverage for all changes and create unit tests for uncovered classes --- .../Test/Unit/Module/ConfigOptionsListTest.php | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/setup/src/Magento/Setup/Test/Unit/Module/ConfigOptionsListTest.php b/setup/src/Magento/Setup/Test/Unit/Module/ConfigOptionsListTest.php index d1281f7c30c55..7fd4e605d8e33 100644 --- a/setup/src/Magento/Setup/Test/Unit/Module/ConfigOptionsListTest.php +++ b/setup/src/Magento/Setup/Test/Unit/Module/ConfigOptionsListTest.php @@ -9,6 +9,7 @@ use Magento\Setup\Model\ConfigGenerator; use Magento\Setup\Model\ConfigOptionsList; use Magento\Setup\Validator\DbValidator; +use Magento\Framework\Config\ConfigOptionsListConstants; class ConfigOptionsListTest extends \PHPUnit_Framework_TestCase { @@ -100,4 +101,21 @@ public function testCreateOptionsWithOptionalNull() $configData = $this->object->createConfig([], $this->deploymentConfig); $this->assertEquals(6, count($configData)); } + + public function testValidate() + { + $options = [ + ConfigOptionsListConstants::INPUT_KEY_DB_PREFIX => 'prefix', + ConfigOptionsListConstants::INPUT_KEY_SKIP_DB_VALIDATION => false, + ConfigOptionsListConstants::INPUT_KEY_DB_NAME => 'name', + ConfigOptionsListConstants::INPUT_KEY_DB_HOST => 'host', + ConfigOptionsListConstants::INPUT_KEY_DB_USER => 'user', + ConfigOptionsListConstants::INPUT_KEY_DB_PASSWORD => 'pass' + ]; + $configDataMock = $this->getMock('Magento\Framework\Config\Data\ConfigData', [], [], '', false); + $this->dbValidator->expects($this->once())->method('checkDatabaseTablePrefix')->willReturn($configDataMock); + $this->dbValidator->expects($this->once())->method('checkDatabaseConnection')->willReturn($configDataMock); + $result = $this->object->validate($options, $this->deploymentConfig); + $this->assertEquals([], $result); + } } From 4545cb15a7f0fc6dded4fb913989cd57a390d260 Mon Sep 17 00:00:00 2001 From: Roman Ganin Date: Wed, 12 Aug 2015 16:23:34 +0300 Subject: [PATCH 09/20] MAGETWO-41421: Extend restricted code test --- .../Magento/Test/Legacy/RestrictedCodeTest.php | 15 +++++++++------ .../Test/Legacy/_files/restricted_classes.php | 14 ++++++-------- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/dev/tests/static/testsuite/Magento/Test/Legacy/RestrictedCodeTest.php b/dev/tests/static/testsuite/Magento/Test/Legacy/RestrictedCodeTest.php index 8280ee586a4bf..12b052cab56f1 100644 --- a/dev/tests/static/testsuite/Magento/Test/Legacy/RestrictedCodeTest.php +++ b/dev/tests/static/testsuite/Magento/Test/Legacy/RestrictedCodeTest.php @@ -50,7 +50,7 @@ protected static function _loadData(array &$data, $filePattern) str_replace(\Magento\Framework\App\Utility\Files::init()->getPathToSource(), '', $file) ); array_push(self::$_fixtureFiles, $relativePath); - $data = array_merge($data, self::_readList($file)); + $data = array_merge_recursive($data, self::_readList($file)); } } @@ -95,16 +95,19 @@ function ($file) { protected function _testRestrictedClasses($file) { $content = file_get_contents($file); - foreach (self::$_classes as $row) { - list($class, $replacement, $whiteList) = $row; - foreach ($whiteList as $skippedPath) { + foreach (self::$_classes as $restrictedClass => $classRules) { + foreach ($classRules['exclude'] as $skippedPath) { if ($this->_isFileInPath($skippedPath, $file)) { continue 2; } } $this->assertFalse( - \Magento\TestFramework\Utility\CodeCheck::isClassUsed($class, $content), - sprintf("Class '%s' is restricted. Suggested replacement: %s", $class, $replacement) + \Magento\TestFramework\Utility\CodeCheck::isClassUsed($restrictedClass, $content), + sprintf( + "Class '%s' is restricted. Suggested replacement: %s", + $restrictedClass, + $classRules['replacement'] + ) ); } } diff --git a/dev/tests/static/testsuite/Magento/Test/Legacy/_files/restricted_classes.php b/dev/tests/static/testsuite/Magento/Test/Legacy/_files/restricted_classes.php index ff9eb9c2b1527..2ccf17ec4386e 100644 --- a/dev/tests/static/testsuite/Magento/Test/Legacy/_files/restricted_classes.php +++ b/dev/tests/static/testsuite/Magento/Test/Legacy/_files/restricted_classes.php @@ -10,19 +10,17 @@ * See COPYING.txt for license details. */ return [ - [ - 'Zend_Db_Select', - '\Magento\Framework\DB\Select', - [ + 'Zend_Db_Select' => [ + 'replacement' => '\Magento\Framework\DB\Select', + 'exclude' => [ '/lib/internal/Magento/Framework/DB/Select.php', '/lib/internal/Magento/Framework/DB/Adapter/Pdo/Mysql.php', '/lib/internal/Magento/Framework/Model/Resource/Iterator.php', ] ], - [ - 'Zend_Db_Adapter_Pdo_Mysql', - '\Magento\Framework\DB\Adapter\Pdo\Mysql', - [ + 'Zend_Db_Adapter_Pdo_Mysql' => [ + 'replacement' => '\Magento\Framework\DB\Adapter\Pdo\Mysql', + 'exclude' => [ '/lib/internal/Magento/Framework/DB/Adapter/Pdo/Mysql.php', ] ], From c2aedf2231982d53ef3628dabbb4801c2c71358f Mon Sep 17 00:00:00 2001 From: Stanislav Lopukhov Date: Wed, 12 Aug 2015 16:39:47 +0300 Subject: [PATCH 10/20] MAGETWO-40884: Update TemporaryStorage.php --- .../Magento/Framework/Search/Adapter/Mysql/TemporaryStorage.php | 2 +- .../Search/Test/Unit/Adapter/Mysql/TemporaryStorageTest.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/internal/Magento/Framework/Search/Adapter/Mysql/TemporaryStorage.php b/lib/internal/Magento/Framework/Search/Adapter/Mysql/TemporaryStorage.php index e86d7746274cd..e26cc87a1d1a7 100644 --- a/lib/internal/Magento/Framework/Search/Adapter/Mysql/TemporaryStorage.php +++ b/lib/internal/Magento/Framework/Search/Adapter/Mysql/TemporaryStorage.php @@ -86,7 +86,7 @@ private function createTemporaryTable() { $connection = $this->getConnection(); $table = $connection->newTable($this->resource->getTableName(self::TEMPORARY_TABLE_PREFIX . time())); - $connection->dropTable($table->getName()); + $connection->dropTemporaryTable($table->getName()); $table->addColumn( self::FIELD_ENTITY_ID, Table::TYPE_INTEGER, diff --git a/lib/internal/Magento/Framework/Search/Test/Unit/Adapter/Mysql/TemporaryStorageTest.php b/lib/internal/Magento/Framework/Search/Test/Unit/Adapter/Mysql/TemporaryStorageTest.php index 4033afb052e99..f7ea0d748ab9f 100644 --- a/lib/internal/Magento/Framework/Search/Test/Unit/Adapter/Mysql/TemporaryStorageTest.php +++ b/lib/internal/Magento/Framework/Search/Test/Unit/Adapter/Mysql/TemporaryStorageTest.php @@ -143,7 +143,7 @@ private function createTemporaryTable() ->with($this->tableName) ->willReturn($table); $this->adapter->expects($this->once()) - ->method('dropTable'); + ->method('dropTemporaryTable'); $this->adapter->expects($this->once()) ->method('createTemporaryTable') ->with($table); From c9a7c0df10b22bd0fddc4b53ff16c2f81e9b1be3 Mon Sep 17 00:00:00 2001 From: Egor Shitikov Date: Thu, 13 Aug 2015 21:54:19 +0300 Subject: [PATCH 11/20] MAGETWO-41356: Cover all changes with unit tests --- .../App/Test/Unit/Request/HttpTest.php | 31 ++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/lib/internal/Magento/Framework/App/Test/Unit/Request/HttpTest.php b/lib/internal/Magento/Framework/App/Test/Unit/Request/HttpTest.php index 8bfb9be2f3bb2..6c1693ffde219 100644 --- a/lib/internal/Magento/Framework/App/Test/Unit/Request/HttpTest.php +++ b/lib/internal/Magento/Framework/App/Test/Unit/Request/HttpTest.php @@ -83,7 +83,7 @@ private function getModel($uri = null) public function testGetOriginalPathInfoWithTestUri() { - $uri = 'http://test.com/value'; + $uri = 'http://test.com/value?key=value'; $this->_model = $this->getModel($uri); $this->assertEquals('/value', $this->_model->getOriginalPathInfo()); } @@ -339,6 +339,35 @@ public function testIsSecure($isSecure, $serverHttps, $headerOffloadKey, $header $this->assertSame($isSecure, $this->_model->isSecure()); } + /** + * @dataProvider httpMethodProvider + * + * @param string $method value of $_SERVER['REQUEST_METHOD'] + */ + public function testIsSafeMethodTrue($httpMethod) + { + $this->_model = $this->getModel(); + $_SERVER['REQUEST_METHOD'] = $httpMethod; + $this->assertEquals(true, $this->_model->IsSafeMethod()); + } + + public function testIsSafeMethodFalse() + { + $this->_model = $this->getModel(); + $_SERVER['REQUEST_METHOD'] = 'OTHER'; + $this->assertEquals(false, $this->_model->IsSafeMethod()); + } + + public function httpMethodProvider() + { + return [ + 'Test 1' => ['GET'], + 'Test 2' => ['HEAD'], + 'Test 3' => ['TRACE'], + 'Test 4' => ['CONNECT'] + ]; + } + public function isSecureDataProvider() { /** From c578328c8589556c4633d526d9c7c418329b4de2 Mon Sep 17 00:00:00 2001 From: Volodymyr Kholoshenko Date: Fri, 14 Aug 2015 10:24:40 +0300 Subject: [PATCH 12/20] MAGETWO-41523: Update Framework\App\Resource\ConnectionFactoryTest --- .../Magento/Framework/App/Resource/ConnectionFactoryTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/dev/tests/integration/testsuite/Magento/Framework/App/Resource/ConnectionFactoryTest.php b/dev/tests/integration/testsuite/Magento/Framework/App/Resource/ConnectionFactoryTest.php index 90da5fc6deb9d..5b77db64a1c62 100644 --- a/dev/tests/integration/testsuite/Magento/Framework/App/Resource/ConnectionFactoryTest.php +++ b/dev/tests/integration/testsuite/Magento/Framework/App/Resource/ConnectionFactoryTest.php @@ -34,7 +34,6 @@ public function testCreate() ]; $connection = $this->model->create($dbConfig); $this->assertInstanceOf('\Magento\Framework\DB\Adapter\AdapterInterface', $connection); - $this->assertAttributeInstanceOf('\Magento\Framework\Cache\FrontendInterface', '_cacheAdapter', $connection); $this->assertAttributeInstanceOf('\Magento\Framework\Db\LoggerInterface', 'logger', $connection); } } From 3ec43ad3988b145e789f38c31224c0629e0b968b Mon Sep 17 00:00:00 2001 From: Volodymyr Kholoshenko Date: Fri, 14 Aug 2015 13:49:16 +0300 Subject: [PATCH 13/20] MAGETWO-40892: Bugfixing buffer - update integration tests --- .../Magento/TestFramework/ObjectManagerFactory.php | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/dev/tests/integration/framework/Magento/TestFramework/ObjectManagerFactory.php b/dev/tests/integration/framework/Magento/TestFramework/ObjectManagerFactory.php index e7bdb275e88ce..54d587be455fe 100644 --- a/dev/tests/integration/framework/Magento/TestFramework/ObjectManagerFactory.php +++ b/dev/tests/integration/framework/Magento/TestFramework/ObjectManagerFactory.php @@ -6,6 +6,7 @@ namespace Magento\TestFramework; use Magento\Framework\App\Filesystem\DirectoryList; +use Magento\Framework\Exception\LocalizedException; use Magento\Framework\Filesystem\DriverPool; /** @@ -91,15 +92,14 @@ protected function _loadPrimaryConfig(DirectoryList $directoryList, $driverPool, ); $diPreferences = []; $diPreferencesPath = __DIR__ . '/../../../etc/di/preferences/'; - $directory = new \RecursiveDirectoryIterator($diPreferencesPath, \FilesystemIterator::SKIP_DOTS); - foreach (new \RecursiveIteratorIterator($directory) as $file) { - if (!is_readable($file->getPathname())) { - throw new \Magento\Framework\Exception\LocalizedException( - __("'%1' is not readable file.", $file->getPathname()) - ); + $preferenceFiles = glob($diPreferencesPath . '*.php'); + + foreach ($preferenceFiles as $file) { + if (!is_readable($file)) { + throw new LocalizedException(__("'%1' is not readable file.", $file)); } - $diPreferences = array_replace($diPreferences, include $file->getPathname()); + $diPreferences = array_replace($diPreferences, include $file); } $this->_primaryConfigData['preferences'] = array_replace( From 3069b43c2ca4db94225b4462046a644a64bba035 Mon Sep 17 00:00:00 2001 From: Egor Shitikov Date: Mon, 17 Aug 2015 17:20:41 +0300 Subject: [PATCH 14/20] MAGETWO-41356: Cover all changes with unit tests --- .../App/Test/Unit/Request/HttpTest.php | 21 ++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/lib/internal/Magento/Framework/App/Test/Unit/Request/HttpTest.php b/lib/internal/Magento/Framework/App/Test/Unit/Request/HttpTest.php index 6c1693ffde219..ab020ec6b0f4e 100644 --- a/lib/internal/Magento/Framework/App/Test/Unit/Request/HttpTest.php +++ b/lib/internal/Magento/Framework/App/Test/Unit/Request/HttpTest.php @@ -340,8 +340,8 @@ public function testIsSecure($isSecure, $serverHttps, $headerOffloadKey, $header } /** - * @dataProvider httpMethodProvider - * + * @dataProvider httpSafeMethodProvider + * @backupGlobals * @param string $method value of $_SERVER['REQUEST_METHOD'] */ public function testIsSafeMethodTrue($httpMethod) @@ -351,6 +351,11 @@ public function testIsSafeMethodTrue($httpMethod) $this->assertEquals(true, $this->_model->IsSafeMethod()); } + /** + * @dataProvider httpNotSafeMethodProvider + * @backupGlobals + * @param string $method value of $_SERVER['REQUEST_METHOD'] + */ public function testIsSafeMethodFalse() { $this->_model = $this->getModel(); @@ -358,7 +363,7 @@ public function testIsSafeMethodFalse() $this->assertEquals(false, $this->_model->IsSafeMethod()); } - public function httpMethodProvider() + public function httpSafeMethodProvider() { return [ 'Test 1' => ['GET'], @@ -368,6 +373,16 @@ public function httpMethodProvider() ]; } + public function httpNotSafeMethodProvider() + { + return [ + 'Test 1' => ['POST'], + 'Test 2' => ['PUT'], + 'Test 3' => ['DELETE'], + 'Test 4' => ['PATCH'] + ]; + } + public function isSecureDataProvider() { /** From c930e9fded244db4be04df0d54dfdb38e0737f2b Mon Sep 17 00:00:00 2001 From: Egor Shitikov Date: Mon, 17 Aug 2015 17:48:50 +0300 Subject: [PATCH 15/20] MAGETWO-41356: Cover all changes with unit tests --- .../Magento/Framework/App/Test/Unit/Request/HttpTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/internal/Magento/Framework/App/Test/Unit/Request/HttpTest.php b/lib/internal/Magento/Framework/App/Test/Unit/Request/HttpTest.php index ab020ec6b0f4e..81831d2a416c4 100644 --- a/lib/internal/Magento/Framework/App/Test/Unit/Request/HttpTest.php +++ b/lib/internal/Magento/Framework/App/Test/Unit/Request/HttpTest.php @@ -356,10 +356,10 @@ public function testIsSafeMethodTrue($httpMethod) * @backupGlobals * @param string $method value of $_SERVER['REQUEST_METHOD'] */ - public function testIsSafeMethodFalse() + public function testIsSafeMethodFalse($httpMethod) { $this->_model = $this->getModel(); - $_SERVER['REQUEST_METHOD'] = 'OTHER'; + $_SERVER['REQUEST_METHOD'] = $httpMethod; $this->assertEquals(false, $this->_model->IsSafeMethod()); } From 8fe87a7879d3ee3e8a3f44e22463482beb8af931 Mon Sep 17 00:00:00 2001 From: Stanislav Lopukhov Date: Tue, 18 Aug 2015 15:14:36 +0300 Subject: [PATCH 16/20] MAGETWO-40884: Update App/Request/Http.php --- lib/internal/Magento/Framework/App/Request/Http.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/Magento/Framework/App/Request/Http.php b/lib/internal/Magento/Framework/App/Request/Http.php index 0e0ea69a2148c..53a790732dea7 100644 --- a/lib/internal/Magento/Framework/App/Request/Http.php +++ b/lib/internal/Magento/Framework/App/Request/Http.php @@ -85,7 +85,7 @@ class Http extends Request implements RequestInterface /** * @var array */ - protected $safeRequestTypes = ['GET', 'HEAD', 'TRACE', 'CONNECT']; + protected $safeRequestTypes = ['GET', 'HEAD', 'TRACE', 'OPTIONS']; /** * @param CookieReaderInterface $cookieReader From 938bf97bdf66a2fa0521dba30c880147b1501b3c Mon Sep 17 00:00:00 2001 From: Egor Shitikov Date: Wed, 19 Aug 2015 20:33:34 +0300 Subject: [PATCH 17/20] MAGETWO-41356: Cover all changes with unit tests --- .../Framework/App/Test/Unit/Request/HttpTest.php | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/internal/Magento/Framework/App/Test/Unit/Request/HttpTest.php b/lib/internal/Magento/Framework/App/Test/Unit/Request/HttpTest.php index 10fcaaa1cb6ba..79a04def74c1a 100644 --- a/lib/internal/Magento/Framework/App/Test/Unit/Request/HttpTest.php +++ b/lib/internal/Magento/Framework/App/Test/Unit/Request/HttpTest.php @@ -352,7 +352,7 @@ public function testIsSecure($isSecure, $serverHttps, $headerOffloadKey, $header /** * @dataProvider httpSafeMethodProvider - * @backupGlobals + * @backupGlobals enabled * @param string $method value of $_SERVER['REQUEST_METHOD'] */ public function testIsSafeMethodTrue($httpMethod) @@ -364,7 +364,7 @@ public function testIsSafeMethodTrue($httpMethod) /** * @dataProvider httpNotSafeMethodProvider - * @backupGlobals + * @backupGlobals enabled * @param string $method value of $_SERVER['REQUEST_METHOD'] */ public function testIsSafeMethodFalse($httpMethod) @@ -380,7 +380,8 @@ public function httpSafeMethodProvider() 'Test 1' => ['GET'], 'Test 2' => ['HEAD'], 'Test 3' => ['TRACE'], - 'Test 4' => ['CONNECT'] + 'Test 4' => ['OPTIONS'] + ]; } @@ -390,7 +391,9 @@ public function httpNotSafeMethodProvider() 'Test 1' => ['POST'], 'Test 2' => ['PUT'], 'Test 3' => ['DELETE'], - 'Test 4' => ['PATCH'] + 'Test 4' => ['PATCH'], + 'Test 5' => ['CONNECT'], + 'Test 6' => [''] ]; } From 940e311fb71fdcfcdf1248f141b44bd48d893040 Mon Sep 17 00:00:00 2001 From: Egor Shitikov Date: Wed, 19 Aug 2015 20:33:51 +0300 Subject: [PATCH 18/20] MAGETWO-41356: Cover all changes with unit tests --- .../Magento/Framework/App/Test/Unit/Request/HttpTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/internal/Magento/Framework/App/Test/Unit/Request/HttpTest.php b/lib/internal/Magento/Framework/App/Test/Unit/Request/HttpTest.php index 79a04def74c1a..ec124acc75055 100644 --- a/lib/internal/Magento/Framework/App/Test/Unit/Request/HttpTest.php +++ b/lib/internal/Magento/Framework/App/Test/Unit/Request/HttpTest.php @@ -381,7 +381,6 @@ public function httpSafeMethodProvider() 'Test 2' => ['HEAD'], 'Test 3' => ['TRACE'], 'Test 4' => ['OPTIONS'] - ]; } From 451740739d43e4c9a374e76580bcb6c89630925e Mon Sep 17 00:00:00 2001 From: Egor Shitikov Date: Thu, 20 Aug 2015 12:42:33 +0300 Subject: [PATCH 19/20] MAGETWO-41356: Cover all changes with unit tests --- .../Magento/Framework/App/Test/Unit/Request/HttpTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/Magento/Framework/App/Test/Unit/Request/HttpTest.php b/lib/internal/Magento/Framework/App/Test/Unit/Request/HttpTest.php index ec124acc75055..c6c309f0fd55a 100644 --- a/lib/internal/Magento/Framework/App/Test/Unit/Request/HttpTest.php +++ b/lib/internal/Magento/Framework/App/Test/Unit/Request/HttpTest.php @@ -392,7 +392,7 @@ public function httpNotSafeMethodProvider() 'Test 3' => ['DELETE'], 'Test 4' => ['PATCH'], 'Test 5' => ['CONNECT'], - 'Test 6' => [''] + 'Test 6' => [null] ]; } From b0e955c79ed23ebca55cb1df8efe757aace2011e Mon Sep 17 00:00:00 2001 From: Roman Ganin Date: Fri, 21 Aug 2015 12:12:13 +0300 Subject: [PATCH 20/20] MAGETWO-41084: Framework\App\Resource refactoring --- .../Magento/Framework/App/Request/Http.php | 7 +++--- .../Framework/App/RequestSafetyInterface.php | 24 +++++++++++++++++++ 2 files changed, 27 insertions(+), 4 deletions(-) create mode 100644 lib/internal/Magento/Framework/App/RequestSafetyInterface.php diff --git a/lib/internal/Magento/Framework/App/Request/Http.php b/lib/internal/Magento/Framework/App/Request/Http.php index 3953968be522b..fed64841c76f9 100644 --- a/lib/internal/Magento/Framework/App/Request/Http.php +++ b/lib/internal/Magento/Framework/App/Request/Http.php @@ -7,6 +7,7 @@ use Magento\Framework\App\Config\ScopeConfigInterface; use Magento\Framework\App\RequestInterface; +use Magento\Framework\App\RequestSafetyInterface; use Magento\Framework\App\Route\ConfigInterface\Proxy as ConfigInterface; use Magento\Framework\HTTP\PhpEnvironment\Request; use Magento\Framework\ObjectManagerInterface; @@ -16,7 +17,7 @@ /** * Http request */ -class Http extends Request implements RequestInterface +class Http extends Request implements RequestInterface, RequestSafetyInterface { /**#@+ * HTTP Ports @@ -427,9 +428,7 @@ public function isSecure() } /** - * Check that this is safe request - * - * @return bool + * {@inheritdoc} */ public function isSafeMethod() { diff --git a/lib/internal/Magento/Framework/App/RequestSafetyInterface.php b/lib/internal/Magento/Framework/App/RequestSafetyInterface.php new file mode 100644 index 0000000000000..d4e8f113f3d93 --- /dev/null +++ b/lib/internal/Magento/Framework/App/RequestSafetyInterface.php @@ -0,0 +1,24 @@ +