Merge pull request #96 from doctrine/fix/#62-check-trait-and-parent-c…

…lass-ttl-in-annotations

Fix/#62 check trait and parent class ttl in annotations

.travis.yml

@@ -7,11 +7,9 @@ cache:
     - $HOME/.composer/cache
 
 php:
-  - 5.3
-  - 5.4
-  - 5.5
   - 5.6
   - 7
+  - 7.1
   - hhvm
 
 before_script:

composer.json

@@ -13,19 +13,19 @@
         {"name": "Johannes Schmitt", "email": "[email protected]"}
     ],
     "require": {
-        "php": ">=5.3.2",
+        "php": "^5.6 || ^7.0",
         "doctrine/lexer": "1.*"
     },
     "require-dev": {
         "doctrine/cache": "1.*",
-        "phpunit/phpunit": "4.*"
+        "phpunit/phpunit": "^5.6.1"
     },
     "autoload": {
-        "psr-0": { "Doctrine\\Common\\Annotations\\": "lib/" }
+        "psr-4": { "Doctrine\\Common\\Annotations\\": "lib/Doctrine/Common/Annotations" }
     },
     "extra": {
         "branch-alias": {
-            "dev-master": "1.3.x-dev"
+            "dev-master": "1.4.x-dev"
         }
     }
 }

lib/Doctrine/Common/Annotations/CachedReader.php

@@ -20,6 +20,7 @@
 namespace Doctrine\Common\Annotations;
 
 use Doctrine\Common\Cache\Cache;
+use ReflectionClass;
 
 /**
  * A cache aware annotation reader.
@@ -71,7 +72,7 @@ public function __construct(Reader $reader, Cache $cache, $debug = false)
     /**
      * {@inheritDoc}
      */
-    public function getClassAnnotations(\ReflectionClass $class)
+    public function getClassAnnotations(ReflectionClass $class)
     {
         $cacheKey = $class->getName();
 
@@ -90,7 +91,7 @@ public function getClassAnnotations(\ReflectionClass $class)
     /**
      * {@inheritDoc}
      */
-    public function getClassAnnotation(\ReflectionClass $class, $annotationName)
+    public function getClassAnnotation(ReflectionClass $class, $annotationName)
     {
         foreach ($this->getClassAnnotations($class) as $annot) {
             if ($annot instanceof $annotationName) {
@@ -183,11 +184,11 @@ public function clearLoadedAnnotations()
      * Fetches a value from the cache.
      *
      * @param string           $rawCacheKey The cache key.
-     * @param \ReflectionClass $class       The related class.
+     * @param ReflectionClass $class       The related class.
      *
      * @return mixed The cached value or false when the value is not in cache.
      */
-    private function fetchFromCache($rawCacheKey, \ReflectionClass $class)
+    private function fetchFromCache($rawCacheKey, ReflectionClass $class)
     {
         $cacheKey = $rawCacheKey . self::$CACHE_SALT;
         if (($data = $this->cache->fetch($cacheKey)) !== false) {
@@ -220,16 +221,45 @@ private function saveToCache($rawCacheKey, $value)
      * Checks if the cache is fresh.
      *
      * @param string           $cacheKey
-     * @param \ReflectionClass $class
+     * @param ReflectionClass $class
      *
      * @return boolean
      */
-    private function isCacheFresh($cacheKey, \ReflectionClass $class)
+    private function isCacheFresh($cacheKey, ReflectionClass $class)
     {
-        if (false === $filename = $class->getFilename()) {
+        if (null === $lastModification = $this->getLastModification($class)) {
             return true;
         }
 
-        return $this->cache->fetch('[C]'.$cacheKey) >= filemtime($filename);
+        return $this->cache->fetch('[C]'.$cacheKey) >= $lastModification;
+    }
+
+    /**
+     * Returns the time the class was last modified, testing traits and parents
+     *
+     * @param ReflectionClass $class
+     * @return int
+     */
+    private function getLastModification(ReflectionClass $class)
+    {
+        $filename = $class->getFileName();
+        $parent   = $class->getParentClass();
+
+        return max(array_merge(
+            [$filename ? filemtime($filename) : 0],
+            array_map([$this, 'getTraitLastModificationTimes'], $class->getTraits()),
+            array_map([$this, 'getLastModification'], $class->getInterfaces()),
+            $parent ? [$this->getLastModification($parent)] : []
+        ));
+    }
+
+    private function getTraitLastModificationTimes(ReflectionClass $reflectionTrait)
+    {
+        $fileName = $reflectionTrait->getFileName();
+
+        return array_merge(
+            [$fileName ? filemtime($fileName) : 0],
+            array_map([$this, 'getTraitLastModificationTimes'], $reflectionTrait->getTraits())
+        );
     }
 }

tests/Doctrine/Tests/Common/Annotations/CachedReaderTest.php

@@ -13,23 +13,84 @@ class CachedReaderTest extends AbstractReaderTest
 
     public function testIgnoresStaleCache()
     {
-        $file = __DIR__.'/Fixtures/Controller.php';
-        touch($file);
-        $name = 'Doctrine\Tests\Common\Annotations\Fixtures\Controller';
-        $cacheKey = $name.'@[Annot]';
+        $cache = time() - 10;
+        touch(__DIR__.'/Fixtures/Controller.php', $cache + 10);
 
-        $cache = $this->getMock('Doctrine\Common\Cache\Cache');
+        $this->doTestCacheStale('Doctrine\Tests\Common\Annotations\Fixtures\Controller', $cache);
+    }
+
+    /**
+     * @group 62
+     */
+    public function testIgnoresStaleCacheWithParentClass()
+    {
+        $cache = time() - 10;
+        touch(__DIR__.'/Fixtures/ControllerWithParentClass.php', $cache - 10);
+        touch(__DIR__.'/Fixtures/AbstractController.php', $cache + 10);
+
+        $this->doTestCacheStale('Doctrine\Tests\Common\Annotations\Fixtures\ControllerWithParentClass', $cache);
+    }
+
+    /**
+     * @group 62
+     */
+    public function testIgnoresStaleCacheWithTraits()
+    {
+        $cache = time() - 10;
+        touch(__DIR__.'/Fixtures/ControllerWithTrait.php', $cache - 10);
+        touch(__DIR__.'/Fixtures/Traits/SecretRouteTrait.php', $cache + 10);
+
+        $this->doTestCacheStale('Doctrine\Tests\Common\Annotations\Fixtures\ControllerWithTrait', $cache);
+    }
+
+    /**
+     * @group 62
+     */
+    public function testIgnoresStaleCacheWithTraitsThatUseOtherTraits()
+    {
+        $cache = time() - 10;
+
+        touch(__DIR__ . '/Fixtures/ClassThatUsesTraitThatUsesAnotherTrait.php', $cache - 10);
+        touch(__DIR__ . '/Fixtures/Traits/EmptyTrait.php', $cache + 10);
+
+        $this->doTestCacheStale(
+            'Doctrine\Tests\Common\Annotations\Fixtures\ClassThatUsesTraitThatUsesAnotherTrait',
+            $cache
+        );
+    }
+
+    /**
+     * @group 62
+     */
+    public function testIgnoresStaleCacheWithInterfacesThatExtendOtherInterfaces()
+    {
+        $cache = time() - 10;
+
+        touch(__DIR__ . '/Fixtures/InterfaceThatExtendsAnInterface.php', $cache - 10);
+        touch(__DIR__ . '/Fixtures/EmptyInterface.php', $cache + 10);
+
+        $this->doTestCacheStale(
+            'Doctrine\Tests\Common\Annotations\Fixtures\InterfaceThatExtendsAnInterface',
+            $cache
+        );
+    }
+
+    protected function doTestCacheStale($className, $lastCacheModification)
+    {
+        $cacheKey = $className.'@[Annot]';
+
+        $cache = $this->createMock('Doctrine\Common\Cache\Cache');
         $cache
             ->expects($this->at(0))
             ->method('fetch')
             ->with($this->equalTo($cacheKey))
-            ->will($this->returnValue(array()))
+            ->will($this->returnValue(array())) // Result was cached, but there was no annotation
         ;
         $cache
             ->expects($this->at(1))
             ->method('fetch')
             ->with($this->equalTo('[C]'.$cacheKey))
-            ->will($this->returnValue(time() - 10))
+            ->will($this->returnValue($lastCacheModification))
         ;
         $cache
             ->expects($this->at(2))
@@ -45,7 +106,8 @@ public function testIgnoresStaleCache()
         $reader = new CachedReader(new AnnotationReader(), $cache, true);
         $route = new Route();
         $route->pattern = '/someprefix';
-        $this->assertEquals(array($route), $reader->getClassAnnotations(new \ReflectionClass($name)));
+
+        $this->assertEquals(array($route), $reader->getClassAnnotations(new \ReflectionClass($className)));
     }
 
     protected function getReader()

tests/Doctrine/Tests/Common/Annotations/Fixtures/AbstractController.php

@@ -0,0 +1,7 @@
+<?php
+
+namespace Doctrine\Tests\Common\Annotations\Fixtures;
+
+abstract class AbstractController
+{
+}

tests/Doctrine/Tests/Common/Annotations/Fixtures/ClassThatUsesTraitThatUsesAnotherTrait.php

@@ -0,0 +1,14 @@
+<?php
+
+namespace Doctrine\Tests\Common\Annotations\Fixtures;
+
+use Doctrine\Tests\Common\Annotations\Fixtures\Annotation\Route;
+use Doctrine\Tests\Common\Annotations\Fixtures\Traits\TraitThatUsesAnotherTrait;
+
+/**
+ * @Route("/someprefix")
+ */
+class ClassThatUsesTraitThatUsesAnotherTrait
+{
+    use TraitThatUsesAnotherTrait;
+}

tests/Doctrine/Tests/Common/Annotations/Fixtures/ControllerWithParentClass.php

@@ -0,0 +1,22 @@
+<?php
+
+namespace Doctrine\Tests\Common\Annotations\Fixtures;
+
+use Doctrine\Tests\Common\Annotations\Fixtures\Annotation\Template;
+use Doctrine\Tests\Common\Annotations\Fixtures\Annotation\Route;
+
+/**
+ * @Route("/someprefix")
+ * @author Johannes M. Schmitt <[email protected]>
+ */
+class ControllerWithParentClass extends AbstractController
+{
+    /**
+     * @Route("/", name="_demo")
+     * @Template()
+     */
+    public function indexAction()
+    {
+        return array();
+    }
+}

tests/Doctrine/Tests/Common/Annotations/Fixtures/ControllerWithTrait.php

@@ -0,0 +1,25 @@
+<?php
+
+namespace Doctrine\Tests\Common\Annotations\Fixtures;
+
+use Doctrine\Tests\Common\Annotations\Fixtures\Annotation\Template;
+use Doctrine\Tests\Common\Annotations\Fixtures\Annotation\Route;
+use Doctrine\Tests\Common\Annotations\Fixtures\Traits\SecretRouteTrait;
+
+/**
+ * @Route("/someprefix")
+ * @author Johannes M. Schmitt <[email protected]>
+ */
+class ControllerWithTrait
+{
+    use SecretRouteTrait;
+
+    /**
+     * @Route("/", name="_demo")
+     * @Template()
+     */
+    public function indexAction()
+    {
+        return array();
+    }
+}

tests/Doctrine/Tests/Common/Annotations/Fixtures/EmptyInterface.php

@@ -0,0 +1,7 @@
+<?php
+
+namespace Doctrine\Tests\Common\Annotations\Fixtures;
+
+interface EmptyInterface
+{
+}

tests/Doctrine/Tests/Common/Annotations/Fixtures/InterfaceThatExtendsAnInterface.php

@@ -0,0 +1,12 @@
+<?php
+
+namespace Doctrine\Tests\Common\Annotations\Fixtures;
+
+use Doctrine\Tests\Common\Annotations\Fixtures\Annotation\Route;
+
+/**
+ * @Route("/someprefix")
+ */
+interface InterfaceThatExtendsAnInterface extends EmptyInterface
+{
+}

tests/Doctrine/Tests/Common/Annotations/Fixtures/Traits/EmptyTrait.php

@@ -0,0 +1,7 @@
+<?php
+
+namespace Doctrine\Tests\Common\Annotations\Fixtures\Traits;
+
+trait EmptyTrait
+{
+}

tests/Doctrine/Tests/Common/Annotations/Fixtures/Traits/SecretRouteTrait.php

@@ -0,0 +1,18 @@
+<?php
+
+namespace Doctrine\Tests\Common\Annotations\Fixtures\Traits;
+
+use Doctrine\Tests\Common\Annotations\Fixtures\Annotation\Template;
+use Doctrine\Tests\Common\Annotations\Fixtures\Annotation\Route;
+
+trait SecretRouteTrait
+{
+    /**
+     * @Route("/secret", name="_secret")
+     * @Template()
+     */
+    public function secretAction()
+    {
+        return array();
+    }
+}

tests/Doctrine/Tests/Common/Annotations/Fixtures/Traits/TraitThatUsesAnotherTrait.php

@@ -0,0 +1,8 @@
+<?php
+
+namespace Doctrine\Tests\Common\Annotations\Fixtures\Traits;
+
+trait TraitThatUsesAnotherTrait
+{
+    use EmptyTrait;
+}