Fixes bug with auto_increment_path and non-title updates

- We now safely ignore updates that do not change the route name.

Author: dantleech

AutoRoute/PathExists/AutoIncrementPath.php

@@ -32,12 +32,19 @@ public function execute(RouteStack $routeStack)
 
         $path = $routeStack->getFullPath();
 
+        $route = $this->dm->find(null, $path);
+        $context = $routeStack->getContext();
+
+        if ($route->getRouteContent() === $context->getContent()) {
+            $routeStack->addRoute($route);
+            return;
+        }
+
         do {
             $newPath = sprintf('%s-%d', $path, $inc++);
         } while (null !== $this->dm->find(null, $newPath));
 
         $routeStack->replaceLastPathElement(basename($newPath));
-
         $this->routeMaker->make($routeStack);
     }
 }

Tests/AutoRoute/PathExists/AutoIncrementPathTest.php

@@ -20,32 +20,81 @@ public function setUp()
             'Symfony\Cmf\Bundle\RoutingAutoBundle\AutoRoute\RouteStack'
         )->disableOriginalConstructor()->getMock();
 
+        $this->builderContext = $this->getMock('Symfony\Cmf\Bundle\RoutingAutoBundle\AutoRoute\BuilderContext');
+
         $this->aiPath = new AutoIncrementPath($this->dm, $this->routeMaker);
+        $this->route1 = $this->getMockBuilder('Symfony\Cmf\Bundle\RoutingExtraBundle\Document\Route')
+            ->disableOriginalConstructor()
+            ->getMock();
+        $this->route2 = $this->getMockBuilder('Symfony\Cmf\Bundle\RoutingExtraBundle\Document\Route')
+            ->disableOriginalConstructor()
+            ->getMock();
+
+        $this->content1 = new \stdClass;
+        $this->content2 = new \stdClass;
     }
 
-    public function testAutoIncrement()
+    public function provideAutoIncrement()
+    {
+        return array(
+            array(false),
+            array(true),
+        );
+    }
+
+    /**
+     * @dataProvider provideAutoIncrement
+     */
+    public function testAutoIncrement($testUpdate)
     {
         $this->routeStack->expects($this->once())
             ->method('getFullPath')
             ->will($this->returnValue('/foo/bar'));
 
+        $this->routeStack->expects($this->once())
+            ->method('getContext')
+            ->will($this->returnValue($this->builderContext));
+
+        // if we test update we have the same content found
+        // by the DM and set in the builder context
         $this->dm->expects($this->at(0))
             ->method('find')
-            ->with(null, '/foo/bar-1')
-            ->will($this->returnValue(new \stdClass));
+            ->with(null, '/foo/bar')
+            ->will($this->returnValue($this->route1));
 
-        $this->dm->expects($this->at(1))
-            ->method('find')
-            ->with(null, '/foo/bar-2')
-            ->will($this->returnValue(null));
+        $this->route1->expects($this->once())
+            ->method('getRouteContent')
+            ->will($this->returnValue($this->content1));
 
-        $this->routeStack->expects($this->once())
-            ->method('replaceLastPathElement')
-            ->with('bar-2');
+        if (true === $testUpdate) {
+            $this->builderContext->expects($this->once())
+                ->method('getContent')
+                ->will($this->returnValue($this->content1));
+        } else {
+            $this->builderContext->expects($this->once())
+                ->method('getContent')
+                ->will($this->returnValue($this->content2));
+        }
+
+        if (false === $testUpdate) {
+            $this->dm->expects($this->at(1))
+                ->method('find')
+                ->with(null, '/foo/bar-1')
+                ->will($this->returnValue(new \stdClass));
+
+            $this->dm->expects($this->at(2))
+                ->method('find')
+                ->with(null, '/foo/bar-2')
+                ->will($this->returnValue(null));
+
+            $this->routeStack->expects($this->once())
+                ->method('replaceLastPathElement')
+                ->with('bar-2');
 
-        $this->routeMaker->expects($this->once())
-            ->method('make')
-            ->with($this->routeStack);
+            $this->routeMaker->expects($this->once())
+                ->method('make')
+                ->with($this->routeStack);
+        }
 
         $this->aiPath->execute($this->routeStack);
     }

Tests/Functional/EventListener/AutoRouteListenerTest.php

@@ -55,7 +55,7 @@ public function provideTestUpdateBlog()
     /**
      * @dataProvider provideTestUpdateBlog
      */
-    public function testUpdateBlog($withPosts = false)
+    public function testUpdateRenameBlog($withPosts = false)
     {
         $this->createBlog($withPosts);
 
@@ -74,8 +74,6 @@ public function testUpdateBlog($withPosts = false)
         $this->assertCount(1, $routes);
         $this->assertInstanceOf('Symfony\Cmf\Bundle\RoutingAutoBundle\Document\AutoRoute', $routes[0]);
 
-        $this->getDm()->refresh($routes[0]);
-
         $this->assertEquals('foobar', $routes[0]->getName());
         $this->assertEquals('/test/auto-route/blog/foobar', $routes[0]->getId());
 
@@ -90,6 +88,28 @@ public function testUpdateBlog($withPosts = false)
         }
     }
 
+    public function testUpdatePostNotChangingTitle()
+    {
+        $this->createBlog(true);
+
+        $post = $this->getDm()->find(null, '/test/test-blog/This is a post title');
+        $this->assertNotNull($post);
+
+        $post->body = 'Test';
+
+        $this->getDm()->persist($post);
+        $this->getDm()->flush();
+        $this->getDm()->clear();
+
+        $post = $this->getDm()->find(null, '/test/test-blog/This is a post title');
+        $routes = $post->routes;
+
+        $this->assertCount(1, $routes);
+        $this->assertInstanceOf('Symfony\Cmf\Bundle\RoutingAutoBundle\Document\AutoRoute', $routes[0]);
+
+        $this->assertEquals('this-is-a-post-title', $routes[0]->getName());
+    }
+
     public function testRemoveBlog()
     {
         $this->createBlog();

Tests/Functional/app/Document/Post.php

@@ -35,6 +35,11 @@ class Post
      */
     public $title;
 
+    /**
+     * @PHPCR\String
+     */
+    public $body;
+
     public function getTitle()
     {
         return $this->title;