[Validator] Remove a race condition in the ClassMetaDataFactory (fix #…

…3217)
symfony · Mar 13, 2012 · 93cc9ef · 93cc9ef
1 parent b062cc7
commit 93cc9ef
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 31 deletions.
diff --git a/src/Symfony/Component/Validator/Mapping/Cache/CacheInterface.php b/src/Symfony/Component/Validator/Mapping/Cache/CacheInterface.php
@@ -32,7 +32,7 @@ function has($class);
      *
      * @param string $class Class Name
      *
-     * @return ClassMetadata
+     * @return ClassMetadata|false A ClassMetadata instance or false on miss
      */
     function read($class);
 

diff --git a/src/Symfony/Component/Validator/Mapping/ClassMetadataFactory.php b/src/Symfony/Component/Validator/Mapping/ClassMetadataFactory.php
@@ -45,32 +45,32 @@ public function getClassMetadata($class)
     {
         $class = ltrim($class, '\\');
 
-        if (!isset($this->loadedClasses[$class])) {
-            if ($this->cache !== null && $this->cache->has($class)) {
-                $this->loadedClasses[$class] = $this->cache->read($class);
-            } else {
-                $metadata = new ClassMetadata($class);
-
-                // Include constraints from the parent class
-                if ($parent = $metadata->getReflectionClass()->getParentClass()) {
-                    $metadata->mergeConstraints($this->getClassMetadata($parent->getName()));
-                }
-
-                // Include constraints from all implemented interfaces
-                foreach ($metadata->getReflectionClass()->getInterfaces() as $interface) {
-                    $metadata->mergeConstraints($this->getClassMetadata($interface->getName()));
-                }
-
-                $this->loader->loadClassMetadata($metadata);
-
-                $this->loadedClasses[$class] = $metadata;
-
-                if ($this->cache !== null) {
-                    $this->cache->write($metadata);
-                }
-            }
+        if (isset($this->loadedClasses[$class])) {
+            return $this->loadedClasses[$class];
         }
 
-        return $this->loadedClasses[$class];
+        if (null !== $this->cache && false !== ($this->loadedClasses[$class] = $this->cache->read($class))) {
+            return $this->loadedClasses[$class];
+        }
+
+        $metadata = new ClassMetadata($class);
+
+        // Include constraints from the parent class
+        if ($parent = $metadata->getReflectionClass()->getParentClass()) {
+            $metadata->mergeConstraints($this->getClassMetadata($parent->getName()));
+        }
+
+        // Include constraints from all implemented interfaces
+        foreach ($metadata->getReflectionClass()->getInterfaces() as $interface) {
+            $metadata->mergeConstraints($this->getClassMetadata($interface->getName()));
+        }
+
+        $this->loader->loadClassMetadata($metadata);
+
+        if ($this->cache !== null) {
+            $this->cache->write($metadata);
+        }
+
+        return $this->loadedClasses[$class] = $metadata;
     }
 }
diff --git a/tests/Symfony/Tests/Component/Validator/Mapping/ClassMetadataFactoryTest.php b/tests/Symfony/Tests/Component/Validator/Mapping/ClassMetadataFactoryTest.php
@@ -74,8 +74,10 @@ public function testWriteMetadataToCache()
             new ConstraintA(array('groups' => array('Default', 'EntityParent'))),
         );
 
+        $cache->expects($this->never())
+              ->method('has');
         $cache->expects($this->once())
-              ->method('has')
+              ->method('read')
               ->with($this->equalTo(self::PARENTCLASS))
               ->will($this->returnValue(false));
         $cache->expects($this->once())
@@ -103,10 +105,8 @@ public function testReadMetadataFromCache()
         $loader->expects($this->never())
                ->method('loadClassMetadata');
 
-        $cache->expects($this->once())
-              ->method('has')
-              ->with($this->equalTo(self::PARENTCLASS))
-              ->will($this->returnValue(true));
+        $cache->expects($this->never())
+              ->method('has');
         $cache->expects($this->once())
               ->method('read')
               ->will($this->returnValue($metadata));