Merge remote-tracking branch 'origin/MC-28948' into 2.3.4-develop-pr87

zakdma · zakdma · commit 13196f2ef490 · 2019-11-27T21:25:44.000+02:00
diff --git a/dev/tests/integration/testsuite/Magento/Framework/Session/SaveHandlerTest.php b/dev/tests/integration/testsuite/Magento/Framework/Session/SaveHandlerTest.php
@@ -9,12 +9,18 @@
 use Magento\Framework\Exception\SessionException;
 use Magento\Framework\Phrase;
 use Magento\Framework\Session\Config\ConfigInterface;
-use Magento\Framework\App\ObjectManager;
+use Magento\TestFramework\Helper\Bootstrap;
+use Magento\TestFramework\ObjectManager;
 
+/**
+ * Tests \Magento\Framework\Session\SaveHandler functionality.
+ *
+ * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
+ */
 class SaveHandlerTest extends \PHPUnit\Framework\TestCase
 {
     /**
-     * @var \Magento\TestFramework\ObjectManager
+     * @var ObjectManager
      */
     private $objectManager;
 
@@ -28,115 +34,96 @@ class SaveHandlerTest extends \PHPUnit\Framework\TestCase
      */
     private $saveHandlerFactoryMock;
 
+    /**
+     * @inheritdoc
+     */
     protected function setUp()
     {
-        $this->objectManager = \Magento\TestFramework\Helper\Bootstrap::getObjectManager();
+        $this->objectManager = Bootstrap::getObjectManager();
         $this->deploymentConfigMock = $this->createMock(DeploymentConfig::class);
         $this->objectManager->addSharedInstance($this->deploymentConfigMock, DeploymentConfig::class);
         $this->saveHandlerFactoryMock = $this->createMock(SaveHandlerFactory::class);
         $this->objectManager->addSharedInstance($this->saveHandlerFactoryMock, SaveHandlerFactory::class);
     }
 
+    /**
+     * @inheritdoc
+     */
     protected function tearDown()
     {
         $this->objectManager->removeSharedInstance(DeploymentConfig::class);
         $this->objectManager->removeSharedInstance(SaveHandlerFactory::class);
     }
 
     /**
-     * Tests that the session handler is correctly set when object is created.
-     *
-     * @dataProvider saveHandlerProvider
-     * @param string $deploymentConfigHandler
+     * @return void
      */
-    public function testConstructor($deploymentConfigHandler)
+    public function testRedisSaveHandler(): void
     {
-        $expected = $this->getExpectedSaveHandler($deploymentConfigHandler, ini_get('session.save_handler'));
-
         $this->deploymentConfigMock->method('get')
-            ->willReturnCallback(function ($configPath) use ($deploymentConfigHandler) {
-                switch ($configPath) {
-                    case Config::PARAM_SESSION_SAVE_METHOD:
-                        return $deploymentConfigHandler;
-                    case Config::PARAM_SESSION_CACHE_LIMITER:
-                        return 'private_no_expire';
-                    case Config::PARAM_SESSION_SAVE_PATH:
-                        return 'explicit_save_path';
-                    default:
-                        return null;
-                }
-            });
+            ->willReturnMap(
+                [
+                    [Config::PARAM_SESSION_SAVE_METHOD, null, 'redis'],
+                    [Config::PARAM_SESSION_SAVE_PATH, null, 'explicit_save_path'],
+                ]
+            );
 
-        $this->saveHandlerFactoryMock->expects($this->once())
-            ->method('create')
-            ->with($expected);
-        $sessionConfig = $this->objectManager->create(ConfigInterface::class);
-        $this->objectManager->create(SaveHandler::class, ['sessionConfig' => $sessionConfig]);
+        $redisHandlerMock = $this->getMockBuilder(SaveHandler\Redis::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+        $redisHandlerMock->method('open')
+            ->with('explicit_save_path', 'test_session_id')
+            ->willReturn(true);
 
-        // Test expectation
-        $this->assertEquals(
-            $expected,
-            $sessionConfig->getOption('session.save_handler')
-        );
-    }
+        $this->saveHandlerFactoryMock->expects($this->exactly(1))
+            ->method('create')
+            ->with('redis')
+            ->willReturn($redisHandlerMock);
 
-    public function saveHandlerProvider()
-    {
-        return [
-            ['db'],
-            ['redis'],
-            ['files'],
-            [false],
-        ];
+        $sessionConfig = $this->objectManager->create(ConfigInterface::class);
+        /** @var SaveHandler $saveHandler */
+        $saveHandler = $this->objectManager->create(SaveHandler::class, ['sessionConfig' => $sessionConfig]);
+        $result = $saveHandler->open('explicit_save_path', 'test_session_id');
+        $this->assertTrue($result);
     }
 
     /**
-     * Retrieve expected session.save_handler
-     *
-     * @param string $deploymentConfigHandler
-     * @param string $iniHandler
-     * @return string
+     * @return void
      */
-    private function getExpectedSaveHandler($deploymentConfigHandler, $iniHandler)
-    {
-        if ($deploymentConfigHandler) {
-            return $deploymentConfigHandler;
-        } elseif ($iniHandler) {
-            return $iniHandler;
-        } else {
-            return SaveHandlerInterface::DEFAULT_HANDLER;
-        }
-    }
-
-    public function testConstructorWithException()
+    public function testRedisSaveHandlerFallbackToDefaultOnSessionException(): void
     {
         $this->deploymentConfigMock->method('get')
-            ->willReturnCallback(function ($configPath) {
-                switch ($configPath) {
-                    case Config::PARAM_SESSION_SAVE_METHOD:
-                        return 'db';
-                    case Config::PARAM_SESSION_CACHE_LIMITER:
-                        return 'private_no_expire';
-                    case Config::PARAM_SESSION_SAVE_PATH:
-                        return 'explicit_save_path';
-                    default:
-                        return null;
-                }
-            });
+            ->willReturnMap(
+                [
+                    [Config::PARAM_SESSION_SAVE_METHOD, null, 'redis'],
+                    [Config::PARAM_SESSION_SAVE_PATH, null, 'explicit_save_path'],
+                ]
+            );
+
+        $redisHandlerMock = $this->getMockBuilder(SaveHandler\Redis::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+        $redisHandlerMock->method('open')
+            ->with('explicit_save_path', 'test_session_id')
+            ->willThrowException(new SessionException(new Phrase('Session Exception')));
+
+        $defaultHandlerMock = $this->getMockBuilder(SaveHandler\Native::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+        $defaultHandlerMock->expects($this->once())->method('open')->with('explicit_save_path', 'test_session_id');
 
         $this->saveHandlerFactoryMock->expects($this->at(0))
             ->method('create')
-            ->willThrowException(new SessionException(new Phrase('Session Exception')));
+            ->with('redis')
+            ->willReturn($redisHandlerMock);
         $this->saveHandlerFactoryMock->expects($this->at(1))
             ->method('create')
-            ->with(SaveHandlerInterface::DEFAULT_HANDLER);
-        $sessionConfig = $this->objectManager->create(ConfigInterface::class);
-        $this->objectManager->create(SaveHandler::class, ['sessionConfig' => $sessionConfig]);
+            ->with(SaveHandlerInterface::DEFAULT_HANDLER)
+            ->willReturn($defaultHandlerMock);
 
-        // Test expectation
-        $this->assertEquals(
-            'db',
-            $sessionConfig->getOption('session.save_handler')
-        );
+        $sessionConfig = $this->objectManager->create(ConfigInterface::class);
+        /** @var SaveHandler $saveHandler */
+        $saveHandler = $this->objectManager->create(SaveHandler::class, ['sessionConfig' => $sessionConfig]);
+        $saveHandler->open('explicit_save_path', 'test_session_id');
     }
 }
diff --git a/lib/internal/Magento/Framework/Session/SaveHandler.php b/lib/internal/Magento/Framework/Session/SaveHandler.php
@@ -6,7 +6,7 @@
 namespace Magento\Framework\Session;
 
 use Magento\Framework\Session\Config\ConfigInterface;
-use \Magento\Framework\Exception\SessionException;
+use Magento\Framework\Exception\SessionException;
 
 /**
  * Magento session save handler
@@ -21,8 +21,26 @@ class SaveHandler implements SaveHandlerInterface
     protected $saveHandlerAdapter;
 
     /**
-     * Constructor
-     *
+     * @var SaveHandlerFactory
+     */
+    private $saveHandlerFactory;
+
+    /**
+     * @var ConfigInterface
+     */
+    private $sessionConfig;
+
+    /**
+     * @var string
+     */
+    private $defaultHandler;
+
+    /**
+     * @var string
+     */
+    private $saveHandler;
+
+    /**
      * @param SaveHandlerFactory $saveHandlerFactory
      * @param ConfigInterface $sessionConfig
      * @param string $default
@@ -32,19 +50,10 @@ public function __construct(
         ConfigInterface $sessionConfig,
         $default = self::DEFAULT_HANDLER
     ) {
-        /**
-         * Session handler
-         *
-         * Save handler may be set to custom value in deployment config, which will override everything else.
-         * Otherwise, try to read PHP settings for session.save_handler value. Otherwise, use 'files' as default.
-         */
-        $saveMethod = $sessionConfig->getOption('session.save_handler') ?: $default;
-
-        try {
-            $this->saveHandlerAdapter = $saveHandlerFactory->create($saveMethod);
-        } catch (SessionException $e) {
-            $this->saveHandlerAdapter = $saveHandlerFactory->create($default);
-        }
+        $this->saveHandlerFactory = $saveHandlerFactory;
+        $this->sessionConfig = $sessionConfig;
+        $this->defaultHandler = $default;
+        $this->saveHandler = $this->sessionConfig->getOption('session.save_handler') ?: $this->defaultHandler;
     }
 
     /**
@@ -56,7 +65,7 @@ public function __construct(
      */
     public function open($savePath, $name)
     {
-        return $this->saveHandlerAdapter->open($savePath, $name);
+        return $this->callSafely('open', $savePath, $name);
     }
 
     /**
@@ -66,7 +75,7 @@ public function open($savePath, $name)
      */
     public function close()
     {
-        return $this->saveHandlerAdapter->close();
+        return $this->callSafely('close');
     }
 
     /**
@@ -77,7 +86,7 @@ public function close()
      */
     public function read($sessionId)
     {
-        return $this->saveHandlerAdapter->read($sessionId);
+        return $this->callSafely('read', $sessionId);
     }
 
     /**
@@ -89,7 +98,7 @@ public function read($sessionId)
      */
     public function write($sessionId, $data)
     {
-        return $this->saveHandlerAdapter->write($sessionId, $data);
+        return $this->callSafely('write', $sessionId, $data);
     }
 
     /**
@@ -100,19 +109,41 @@ public function write($sessionId, $data)
      */
     public function destroy($sessionId)
     {
-        return $this->saveHandlerAdapter->destroy($sessionId);
+        return $this->callSafely('destroy', $sessionId);
     }
 
     /**
-     * Garbage Collection - remove old session data older
-     * than $maxLifetime (in seconds)
+     * Garbage Collection - remove old session data older than $maxLifetime (in seconds)
      *
      * @param int $maxLifetime
      * @return bool
      * @SuppressWarnings(PHPMD.ShortMethodName)
      */
     public function gc($maxLifetime)
     {
-        return $this->saveHandlerAdapter->gc($maxLifetime);
+        return $this->callSafely('gc', $maxLifetime);
+    }
+
+    /**
+     * Call save handler adapter method.
+     *
+     * In case custom handler failed, default files handler is used.
+     *
+     * @param string $method
+     * @param mixed $arguments
+     *
+     * @return mixed
+     */
+    private function callSafely(string $method, ...$arguments)
+    {
+        try {
+            if ($this->saveHandlerAdapter === null) {
+                $this->saveHandlerAdapter = $this->saveHandlerFactory->create($this->saveHandler);
+            }
+            return $this->saveHandlerAdapter->{$method}(...$arguments);
+        } catch (SessionException $exception) {
+            $this->saveHandlerAdapter = $this->saveHandlerFactory->create($this->defaultHandler);
+            return $this->saveHandlerAdapter->{$method}(...$arguments);
+        }
     }
 }
diff --git a/lib/internal/Magento/Framework/Session/SaveHandler/Redis.php b/lib/internal/Magento/Framework/Session/SaveHandler/Redis.php
@@ -14,6 +14,9 @@
 use Magento\Framework\Filesystem;
 use Magento\Framework\App\Filesystem\DirectoryList;
 
+/**
+ * Redis session save handler
+ */
 class Redis implements \SessionHandlerInterface
 {
     /**
@@ -40,7 +43,6 @@ class Redis implements \SessionHandlerInterface
      * @param ConfigInterface $config
      * @param LoggerInterface $logger
      * @param Filesystem $filesystem
-     * @throws SessionException
      */
     public function __construct(ConfigInterface $config, LoggerInterface $logger, Filesystem $filesystem)
     {
