Merge remote-tracking branch 'mainline/develop' into FearlessKiwis-MA…

…GETWO-35278-Random-Integration-Test-Failure
magento · Mar 19, 2015 · 04d76cb · 04d76cb
2 parents ed1f1b5 + 9050a27
commit 04d76cb
Show file tree

Hide file tree

Showing 6 changed files with 159 additions and 298 deletions.
diff --git a/lib/internal/Magento/Framework/Api/Test/Unit/Code/Generator/EntityChildTestAbstract.php b/lib/internal/Magento/Framework/Api/Test/Unit/Code/Generator/EntityChildTestAbstract.php
@@ -84,41 +84,34 @@ public function testGenerate()
         //Mocking _validateData call
         $this->mockDefinedClassesCall();
 
-        $this->ioObjectMock->expects($this->once())
-            ->method('makeGenerationDirectory')
-            ->will($this->returnValue(true));
         $this->ioObjectMock->expects($this->once())
             ->method('makeResultFileDirectory')
             ->with($this->getResultClassName())
-            ->will($this->returnValue(true));
-        $this->ioObjectMock->expects($this->once())
-            ->method('fileExists')
-            ->with($resultFileName)
-            ->will($this->returnValue(false));
+            ->willReturn(true);
 
         //Mocking _generateCode call
         $this->classGenerator->expects($this->once())
             ->method('setName')
             ->with($this->getResultClassName())
-            ->will($this->returnSelf());
+            ->willReturnSelf();
         $this->classGenerator->expects($this->once())
             ->method('addProperties')
-            ->will($this->returnSelf());
+            ->willReturnSelf();
         $this->classGenerator->expects($this->once())
             ->method('addMethods')
-            ->will($this->returnSelf());
+            ->willReturnSelf();
         $this->classGenerator->expects($this->once())
             ->method('setClassDocBlock')
-            ->will($this->returnSelf());
+            ->willReturnSelf();
         $this->classGenerator->expects($this->once())
             ->method('generate')
-            ->will($this->returnValue($generatedCode));
+            ->willReturn($generatedCode);
 
         //Mocking generation
         $this->ioObjectMock->expects($this->any())
             ->method('getResultFileName')
             ->with($this->getResultClassName())
-            ->will($this->returnValue($resultFileName));
+            ->willReturn($resultFileName);
         $this->ioObjectMock->expects($this->once())
             ->method('writeResultFile')
             ->with($resultFileName, $generatedCode);

diff --git a/lib/internal/Magento/Framework/Code/Generator/EntityAbstract.php b/lib/internal/Magento/Framework/Code/Generator/EntityAbstract.php
@@ -247,52 +247,23 @@ protected function _validateData()
     {
         $sourceClassName = $this->getSourceClassName();
         $resultClassName = $this->_getResultClassName();
-        $resultFileName = $this->_ioObject->getResultFileName($resultClassName);
-
-        // @todo the controller handling logic below must be removed when controllers become PSR-0 compliant
-        $controllerSuffix = 'Controller';
-        $pathParts = explode('_', $sourceClassName);
-        if (strrpos(
-            $sourceClassName,
-            $controllerSuffix
-        ) === strlen(
-            $sourceClassName
-        ) - strlen(
-            $controllerSuffix
-        ) && isset(
-            $pathParts[2]
-        ) && !in_array(
-            $pathParts[2],
-            ['Block', 'Helper', 'Model']
-        )
-        ) {
-            $controllerPath = preg_replace(
-                '/^([0-9A-Za-z]*)_([0-9A-Za-z]*)/',
-                '\\1_\\2_controllers',
-                $sourceClassName
-            );
-            $filePath = stream_resolve_include_path(str_replace('_', '/', $controllerPath) . '.php');
-            $isSourceClassValid = !empty($filePath);
-        } else {
-            $isSourceClassValid = $this->definedClasses->classLoadable($sourceClassName);
-        }
+        $resultDir = $this->_ioObject->getResultFileDirectory($resultClassName);
 
-        if (!$isSourceClassValid) {
+        if (!$this->definedClasses->classLoadable($sourceClassName)) {
             $this->_addError('Source class ' . $sourceClassName . ' doesn\'t exist.');
             return false;
         } elseif ($this->definedClasses->classLoadable($resultClassName)) {
             $this->_addError('Result class ' . $resultClassName . ' already exists.');
             return false;
-        } elseif (!$this->_ioObject->makeGenerationDirectory()) {
-            $this->_addError('Can\'t create directory ' . $this->_ioObject->getGenerationDirectory() . '.');
-            return false;
-        } elseif (!$this->_ioObject->makeResultFileDirectory($resultClassName)) {
-            $this->_addError(
-                'Can\'t create directory ' . $this->_ioObject->getResultFileDirectory($resultClassName) . '.'
-            );
-            return false;
-        } elseif ($this->_ioObject->fileExists($resultFileName)) {
-            $this->_addError('Result file ' . $resultFileName . ' already exists.');
+        } elseif (
+            /**
+             * If makeResultFileDirectory only fails because the file is already created,
+             * a competing process has generated the file, no exception should be thrown.
+             */
+            !$this->_ioObject->makeResultFileDirectory($resultClassName)
+            && !$this->_ioObject->fileExists($resultDir)
+        ) {
+            $this->_addError('Can\'t create directory ' . $resultDir . '.');
             return false;
         }
         return true;

diff --git a/lib/internal/Magento/Framework/Code/Generator/Io.php b/lib/internal/Magento/Framework/Code/Generator/Io.php
@@ -5,6 +5,8 @@
  */
 namespace Magento\Framework\Code\Generator;
 
+use Magento\Framework\Filesystem\FilesystemException;
+
 class Io
 {
     /**
@@ -82,12 +84,36 @@ public function getResultFileName($className)
     /**
      * @param string $fileName
      * @param string $content
+     * @throws FilesystemException
      * @return bool
      */
     public function writeResultFile($fileName, $content)
     {
+        /**
+         * Rename is atomic on *nix systems, while file_put_contents is not. Writing to a
+         * temporary file whose name is process-unique and renaming to the real location helps
+         * avoid race conditions. Race condition can occur if the compiler has not been run, when
+         * multiple processes are attempting to access the generated file simultaneously.
+         */
         $content = "<?php\n" . $content;
-        return $this->filesystemDriver->filePutContents($fileName, $content);
+        $tmpFile = $fileName . "." . getmypid();
+        $this->filesystemDriver->filePutContents($tmpFile, $content);
+
+        try {
+            $success = $this->filesystemDriver->rename($tmpFile, $fileName);
+        } catch (FilesystemException $e) {
+            if (!file_exists($fileName)) {
+                throw $e;
+            } else {
+                /**
+                 * Due to race conditions, file may have already been written, causing rename to fail. As long as
+                 * the file exists, everything is okay.
+                 */
+                $success = true;
+            }
+        }
+
+        return $success;
     }
 
     /**
@@ -138,7 +164,7 @@ private function _makeDirectory($directory)
                 $this->filesystemDriver->createDirectory($directory, self::DIRECTORY_PERMISSION);
             }
             return true;
-        } catch (\Magento\Framework\Filesystem\FilesystemException $e) {
+        } catch (FilesystemException $e) {
             return false;
         }
     }