Merge pull request #298 from ezsystems/fix-EZP-20677-REST-delete-user…

…-cleanup

Fix EZP-20677: Removing users do not clean up ezuser table and urlaliases

eZ/Publish/Core/Persistence/Cache/Tests/UserHandlerTest.php

@@ -137,12 +137,18 @@ public function testDelete()
         $this->cacheMock
             ->expects( $this->at( 0 ) )
             ->method( 'clear' )
-            ->with( 'user', 'role', 'assignments', 'byGroup', 14 )
+            ->with( 'content', 14 )
             ->will( $this->returnValue( true ) );
 
         $this->cacheMock
             ->expects( $this->at( 1 ) )
             ->method( 'clear' )
+            ->with( 'user', 'role', 'assignments', 'byGroup', 14 )
+            ->will( $this->returnValue( true ) );
+
+        $this->cacheMock
+            ->expects( $this->at( 2 ) )
+            ->method( 'clear' )
             ->with( 'user', 'role', 'assignments', 'byGroup', 'inherited', 14 )
             ->will( $this->returnValue( true ) );
 

eZ/Publish/Core/Persistence/Cache/UserHandler.php

@@ -73,6 +73,7 @@ public function delete( $userId )
         $return = $this->persistenceFactory->getUserHandler()->delete( $userId );
 
         // user id == content id == group id
+        $this->cache->clear( 'content', $userId );
         $this->cache->clear( 'user', 'role', 'assignments', 'byGroup', $userId );
         $this->cache->clear( 'user', 'role', 'assignments', 'byGroup', 'inherited', $userId );
 

eZ/Publish/Core/Persistence/InMemory/UserHandler.php

@@ -129,13 +129,18 @@ public function update( User $user )
      * Delete user with the given ID.
      *
      * @param mixed $userId
-     *
-     * @todo Throw on missing user?
      */
     public function delete( $userId )
     {
-        $this->backend->delete( 'User', $userId );
-        $this->backend->deleteByMatch( 'User\\RoleAssignment', array( 'contentId' => $userId ) );
+        try
+        {
+            $this->backend->delete( 'User', $userId );
+            $this->backend->deleteByMatch( 'User\\RoleAssignment', array( 'contentId' => $userId ) );
+        }
+        catch ( NotFound $e )
+        {
+            // Do nothing, we do not throw if User or RoleAssignments do not exist
+        }
     }
 
     /**

eZ/Publish/Core/Repository/ContentService.php

@@ -907,13 +907,21 @@ protected function publishUrlAliasesForContent( APIContent $content, $updatePath
      */
     public function deleteContent( ContentInfo $contentInfo )
     {
+        $contentInfo = $this->internalLoadContentInfo( $contentInfo->id );
+
         if ( !$this->repository->canUser( 'content', 'remove', $contentInfo ) )
             throw new UnauthorizedException( 'content', 'remove' );
 
         $this->repository->beginTransaction();
         try
         {
+            // Load Locations first as deleting Content also deletes belonging Locations
+            $spiLocations = $this->persistenceHandler->locationHandler()->loadLocationsByContent( $contentInfo->id );
             $this->persistenceHandler->contentHandler()->deleteContent( $contentInfo->id );
+            foreach ( $spiLocations as $spiLocation )
+            {
+                $this->persistenceHandler->urlAliasHandler()->locationDeleted( $spiLocation->id );
+            }
             $this->repository->commit();
         }
         catch ( Exception $e )

eZ/Publish/Core/Repository/Tests/Service/Integration/ContentBase.php

@@ -2184,52 +2184,6 @@ public function testDeleteVersionThrowsUnauthorizedException()
         $contentService->deleteVersion( $draftContent->versionInfo );
     }
 
-    /**
-     * Test for the deleteContent() method.
-     *
-     * @depends testLoadContent
-     * @covers \eZ\Publish\Core\Repository\ContentService::deleteContent
-     */
-    public function testDeleteContent()
-    {
-        /* BEGIN: Use Case */
-        $contentService = $this->repository->getContentService();
-
-        $contentInfo = $contentService->loadContentInfo( 4 );
-
-        $contentService->deleteContent( $contentInfo );
-        /* END: Use Case */
-
-        try
-        {
-            $contentService->loadContent( $contentInfo->id );
-
-            $this->fail( "Content was not successfully deleted!" );
-        }
-        catch ( NotFoundException $e )
-        {
-            // Do nothing
-        }
-    }
-
-    /**
-     * Test for the deleteContent() method.
-     *
-     * @covers \eZ\Publish\Core\Repository\ContentService::deleteContent
-     * @expectedException \eZ\Publish\API\Repository\Exceptions\UnauthorizedException
-     */
-    public function testDeleteContentThrowsUnauthorizedException()
-    {
-        $contentService = $this->repository->getContentService();
-
-        $contentInfo = $contentService->loadContentInfo( 4 );
-
-        // Set anonymous as current user
-        $this->repository->setCurrentUser( $this->getStubbedUser( 10 ) );
-
-        $contentService->deleteContent( $contentInfo );
-    }
-
     /**
      * Test for the copyContent() method.
      *

eZ/Publish/Core/Repository/Tests/Service/Integration/UserBase.php

@@ -606,28 +606,6 @@ public function testLoadUserByCredentialsThrowsNotFoundExceptionBadPassword()
         $userService->loadUserByCredentials( 'admin', 'some_password' );
     }
 
-    /**
-     * Test deleting a user
-     * @covers \eZ\Publish\API\Repository\UserService::deleteUser
-     */
-    public function testDeleteUser()
-    {
-        $userService = $this->repository->getUserService();
-
-        $user = $userService->loadUser( 14 );
-        $userService->deleteUser( $user );
-
-        try
-        {
-            $userService->loadUser( 14 );
-            self::fail( "failed deleting a user" );
-        }
-        catch ( NotFoundException $e )
-        {
-            // Do nothing
-        }
-    }
-
     /**
      * Test updating a user
      * @covers \eZ\Publish\API\Repository\UserService::updateUser

eZ/Publish/Core/Repository/Tests/Service/Mock/ContentTest.php

@@ -12,6 +12,7 @@
 use eZ\Publish\Core\Repository\Tests\Service\Mock\Base as BaseServiceMockTest;
 use eZ\Publish\Core\Repository\Values\Content\VersionInfo;
 use eZ\Publish\API\Repository\Values\Content\ContentInfo;
+use eZ\Publish\SPI\Persistence\Content\Location as SPILocation;
 
 /**
  * Mock test case for Content service
@@ -101,6 +102,143 @@ public function testLoadContentByVersionInfo()
         );
     }
 
+    /**
+     * Test for the deleteContent() method.
+     *
+     * @covers \eZ\Publish\Core\Repository\ContentService::deleteContent
+     * @expectedException \eZ\Publish\Core\Base\Exceptions\UnauthorizedException
+     */
+    public function testDeleteContentThrowsUnauthorizedException()
+    {
+        $repository = $this->getRepositoryMock();
+        $contentService = $this->getPartlyMockedContentService( array( "internalLoadContentInfo" ) );
+        $contentInfo = $this->getMock( "eZ\\Publish\\API\\Repository\\Values\\Content\\ContentInfo" );
+
+        $contentInfo->expects( $this->any() )
+            ->method( "__get" )
+            ->with( "id" )
+            ->will( $this->returnValue( 42 ) );
+
+        $contentService->expects( $this->once() )
+            ->method( "internalLoadContentInfo" )
+            ->with( 42 )
+            ->will( $this->returnValue( $contentInfo ) );
+
+        $repository->expects( $this->once() )
+            ->method( "canUser" )
+            ->with( "content", "remove" )
+            ->will( $this->returnValue( false ) );
+
+        /** @var \eZ\Publish\API\Repository\Values\Content\ContentInfo $contentInfo */
+        $contentService->deleteContent( $contentInfo );
+    }
+
+    /**
+     * Test for the deleteContent() method.
+     *
+     * @covers \eZ\Publish\Core\Repository\ContentService::deleteContent
+     */
+    public function testDeleteContent()
+    {
+        $repository = $this->getRepositoryMock();
+
+        $repository->expects( $this->once() )
+            ->method( "canUser" )
+            ->with( "content", "remove" )
+            ->will( $this->returnValue( true ) );
+
+        $contentService = $this->getPartlyMockedContentService( array( "internalLoadContentInfo" ) );
+        /** @var \PHPUnit_Framework_MockObject_MockObject $urlAliasHandler */
+        $urlAliasHandler = $this->getPersistenceMock()->urlAliasHandler();
+        /** @var \PHPUnit_Framework_MockObject_MockObject $locationHandler */
+        $locationHandler = $this->getPersistenceMock()->locationHandler();
+        /** @var \PHPUnit_Framework_MockObject_MockObject $contentHandler */
+        $contentHandler = $this->getPersistenceMock()->contentHandler();
+
+        $contentInfo = $this->getMock( "eZ\\Publish\\API\\Repository\\Values\\Content\\ContentInfo" );
+
+        $contentService->expects( $this->once() )
+            ->method( "internalLoadContentInfo" )
+            ->with( 42 )
+            ->will( $this->returnValue( $contentInfo ) );
+
+        $contentInfo->expects( $this->any() )
+            ->method( "__get" )
+            ->with( "id" )
+            ->will( $this->returnValue( 42 ) );
+
+        $repository->expects( $this->once() )->method( "beginTransaction" );
+
+        $spiLocations = array(
+            new SPILocation( array( "id" => 1 ) ),
+            new SPILocation( array( "id" => 2 ) ),
+        );
+        $locationHandler->expects( $this->once() )
+            ->method( "loadLocationsByContent" )
+            ->with( 42 )
+            ->will( $this->returnValue( $spiLocations ) );
+
+        $contentHandler->expects( $this->once() )
+            ->method( "deleteContent" )
+            ->with( 42 );
+
+        foreach ( $spiLocations as $index => $spiLocation )
+        {
+            $urlAliasHandler->expects( $this->at( $index ) )
+                ->method( "locationDeleted" )
+                ->with( $spiLocation->id );
+        }
+
+        $repository->expects( $this->once() )->method( "commit" );
+
+        /** @var \eZ\Publish\API\Repository\Values\Content\ContentInfo $contentInfo */
+        $contentService->deleteContent( $contentInfo );
+    }
+
+    /**
+     * Test for the deleteContent() method.
+     *
+     * @covers \eZ\Publish\Core\Repository\ContentService::deleteContent
+     * @expectedException \Exception
+     */
+    public function testDeleteContentWithRollback()
+    {
+        $repository = $this->getRepositoryMock();
+
+        $repository->expects( $this->once() )
+            ->method( "canUser" )
+            ->with( "content", "remove" )
+            ->will( $this->returnValue( true ) );
+
+        $contentService = $this->getPartlyMockedContentService( array( "internalLoadContentInfo" ) );
+        /** @var \PHPUnit_Framework_MockObject_MockObject $locationHandler */
+        $locationHandler = $this->getPersistenceMock()->locationHandler();
+
+        $contentInfo = $this->getMock( "eZ\\Publish\\API\\Repository\\Values\\Content\\ContentInfo" );
+
+        $contentService->expects( $this->once() )
+            ->method( "internalLoadContentInfo" )
+            ->with( 42 )
+            ->will( $this->returnValue( $contentInfo ) );
+
+        $contentInfo->expects( $this->any() )
+            ->method( "__get" )
+            ->with( "id" )
+            ->will( $this->returnValue( 42 ) );
+
+        $repository->expects( $this->once() )->method( "beginTransaction" );
+
+        $locationHandler->expects( $this->once() )
+            ->method( "loadLocationsByContent" )
+            ->with( 42 )
+            ->will( $this->throwException( new \Exception ) );
+
+        $repository->expects( $this->once() )->method( "rollback" );
+
+        /** @var \eZ\Publish\API\Repository\Values\Content\ContentInfo $contentInfo */
+        $contentService->deleteContent( $contentInfo );
+    }
+
     /**
      * Returns the content service to test with $methods mocked
      *

eZ/Publish/Core/Repository/Tests/Service/Mock/UrlAliasTest.php

@@ -1,6 +1,6 @@
 <?php
 /**
- * File contains: eZ\Publish\Core\Repository\Tests\Service\Mock\UrlAliasBase class
+ * File contains: eZ\Publish\Core\Repository\Tests\Service\Mock\UrlAliasTest class
  *
  * @copyright Copyright (C) 1999-2013 eZ Systems AS. All rights reserved.
  * @license http://www.gnu.org/licenses/gpl-2.0.txt GNU General Public License v2
@@ -18,7 +18,6 @@
  */
 class UrlAliasTest extends BaseServiceMockTest
 {
-
     public function providerForTestListAutogeneratedLocationAliasesPath()
     {
         $pathElement1 = array(