Skip to content

Commit

Permalink
[TASK] Mock ResourceFactory Singletons in unit tests
Browse files Browse the repository at this point in the history
This prevents indirect SignalSlotDispatcher and ObjectManager
invocation by stubbing the ResourceFactory (or removing unneeded
mocks by preventing constructor invocation as in ImageServiceTest).

Releases: master, 9.5
Resolves: #87739
Change-Id: I70c5413fbdf66f3deb8ae0113e62f17ad8168f19
Reviewed-on: https://review.typo3.org/c/59754
Tested-by: TYPO3com <noreply@typo3.com>
Tested-by: Susanne Moog <susanne.moog@typo3.org>
Tested-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Reviewed-by: Benni Mack <benni@typo3.org>
Reviewed-by: Susanne Moog <susanne.moog@typo3.org>
Reviewed-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
  • Loading branch information
bnf authored and maddy2101 committed Feb 23, 2019
1 parent 020b444 commit 191d0bd
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 19 deletions.
38 changes: 25 additions & 13 deletions typo3/sysext/core/Tests/Unit/Resource/ResourceStorageTest.php
Expand Up @@ -75,13 +75,15 @@ protected function setUp(): void
* @param array $configuration
* @param bool $mockPermissionChecks
* @param AbstractDriver|\PHPUnit_Framework_MockObject_MockObject $driverObject
* @param ResourceFactory $resourceFactory
* @param array $storageRecord
* @param array $mockedMethods
*/
protected function prepareSubject(
array $configuration,
bool $mockPermissionChecks = false,
AbstractDriver $driverObject = null,
ResourceFactory $resourceFactory = null,
array $storageRecord = [],
array $mockedMethods = []
): void {
Expand All @@ -100,6 +102,10 @@ protected function prepareSubject(
if ($driverObject === null) {
$driverObject = $this->getMockForAbstractClass(AbstractDriver::class, [], '', false);
}
if ($resourceFactory === null) {
$resourceFactory = $this->createMock(ResourceFactory::class);
}
$mockedMethods[] = 'getResourceFactoryInstance';
if ($mockPermissionChecks) {
$mockedMethods = array_merge($mockedMethods, $permissionMethods);
}
Expand All @@ -109,6 +115,7 @@ protected function prepareSubject(
->setMethods(array_unique($mockedMethods))
->setConstructorArgs([$driverObject, $storageRecord])
->getMock();
$this->subject->expects($this->any())->method('getResourceFactoryInstance')->will($this->returnValue($resourceFactory));
$this->subject->expects($this->any())->method('getIndexer')->will($this->returnValue($this->createMock(Indexer::class)));
if ($mockPermissionChecks) {
foreach ($permissionMethods as $method) {
Expand Down Expand Up @@ -241,7 +248,7 @@ public function capabilitiesOfStorageObjectAreCorrectlySet(array $capabilities):
$this->subject,
null
);
$this->prepareSubject([], false, $mockedDriver, $storageRecord);
$this->prepareSubject([], false, $mockedDriver, null, $storageRecord);
$this->assertEquals(
$capabilities['public'],
$this->subject->isPublic(),
Expand Down Expand Up @@ -295,12 +302,14 @@ public function getPublicUrlReturnsNullIfStorageIsNotOnline(): void
$driver = $this->getMockBuilder(LocalDriver::class)
->setConstructorArgs([['basePath' => $this->getMountRootUrl()]])
->getMock();
$mockedResourceFactory = $this->createMock(ResourceFactory::class);
/** @var $subject ResourceStorage|\PHPUnit_Framework_MockObject_MockObject */
$subject = $this->getMockBuilder(ResourceStorage::class)
->setMethods(['isOnline'])
->setMethods(['isOnline', 'getResourceFactoryInstance'])
->setConstructorArgs([$driver, ['configuration' => []]])
->getMock();
$subject->expects($this->once())->method('isOnline')->will($this->returnValue(false));
$subject->expects($this->any())->method('getResourceFactoryInstance')->will($this->returnValue($mockedResourceFactory));

$sourceFileIdentifier = '/sourceFile.ext';
$sourceFile = $this->getSimpleFileMock($sourceFileIdentifier);
Expand Down Expand Up @@ -349,17 +358,19 @@ public function checkFolderPermissionsRespectsFilesystemPermissions(
/** @var $mockedDriver LocalDriver|\PHPUnit_Framework_MockObject_MockObject */
$mockedDriver = $this->createMock(LocalDriver::class);
$mockedDriver->expects($this->any())->method('getPermissions')->will($this->returnValue($permissionsFromDriver));
$mockedResourceFactory = $this->createMock(ResourceFactory::class);
/** @var $mockedFolder Folder|\PHPUnit_Framework_MockObject_MockObject */
$mockedFolder = $this->createMock(Folder::class);
// Let all other checks pass
/** @var $subject ResourceStorage|\PHPUnit_Framework_MockObject_MockObject */
$subject = $this->getMockBuilder(ResourceStorage::class)
->setMethods(['isWritable', 'isBrowsable', 'checkUserActionPermission'])
->setMethods(['isWritable', 'isBrowsable', 'checkUserActionPermission', 'getResourceFactoryInstance'])
->setConstructorArgs([$mockedDriver, []])
->getMock();
$subject->expects($this->any())->method('isWritable')->will($this->returnValue(true));
$subject->expects($this->any())->method('isBrowsable')->will($this->returnValue(true));
$subject->expects($this->any())->method('checkUserActionPermission')->will($this->returnValue(true));
$subject->expects($this->any())->method('getResourceFactoryInstance')->will($this->returnValue($mockedResourceFactory));
$subject->setDriver($mockedDriver);

$this->assertSame($expectedResult, $subject->checkFolderActionPermission($action, $mockedFolder));
Expand Down Expand Up @@ -449,7 +460,7 @@ public function userActionIsDisallowedIfPermissionIsNotSet(): void
*/
public function metaDataEditIsNotAllowedWhenWhenNoFileMountsAreSet(): void
{
$this->prepareSubject([], false, null, [], ['isWithinProcessingFolder']);
$this->prepareSubject([], false, null, null, [], ['isWithinProcessingFolder']);
$this->subject->setEvaluatePermissions(true);
$this->assertFalse($this->subject->checkFileActionPermission('editMeta', new File(['identifier' => '/foo/bar.jpg'], $this->subject)));
}
Expand All @@ -460,7 +471,7 @@ public function metaDataEditIsNotAllowedWhenWhenNoFileMountsAreSet(): void
public function metaDataEditIsAllowedWhenWhenInFileMount(): void
{
$driverMock = $this->getMockForAbstractClass(AbstractDriver::class, [], '', false);
$this->prepareSubject([], false, $driverMock, [], ['isWithinProcessingFolder']);
$this->prepareSubject([], false, $driverMock, null, [], ['isWithinProcessingFolder']);

$fileStub = new File(['identifier' => '/foo/bar.jpg'], $this->subject);
$folderStub = new Folder($this->subject, '/foo/', 'foo');
Expand All @@ -487,7 +498,7 @@ public function metaDataEditIsAllowedWhenWhenInFileMount(): void
public function metaDataEditIsNotAllowedWhenWhenInReadOnlyFileMount(): void
{
$driverMock = $this->getMockForAbstractClass(AbstractDriver::class, [], '', false);
$this->prepareSubject([], false, $driverMock, [], ['isWithinProcessingFolder']);
$this->prepareSubject([], false, $driverMock, null, [], ['isWithinProcessingFolder']);

$fileStub = new File(['identifier' => '/foo/bar.jpg'], $this->subject);
$folderStub = new Folder($this->subject, '/foo/', 'foo');
Expand Down Expand Up @@ -784,7 +795,7 @@ public function renameFileRenamesFileAsRequested(): void
{
$mockedDriver = $this->createDriverMock([], $this->subject);
$mockedDriver->expects($this->once())->method('renameFile')->will($this->returnValue('bar'));
$this->prepareSubject([], true, $mockedDriver, [], ['emitPreFileRenameSignal', 'emitPostFileRenameSignal']);
$this->prepareSubject([], true, $mockedDriver, null, [], ['emitPreFileRenameSignal', 'emitPostFileRenameSignal']);
/** @var File $file */
$file = new File(['identifier' => 'foo', 'name' => 'foo'], $this->subject);
$result = $this->subject->renameFile($file, 'bar');
Expand All @@ -808,16 +819,19 @@ public function renameFileRenamesWithUniqueNameIfConflictAndConflictModeIsRename
'bar',
'bar_01'
));
$resourceFactory = $this->createMock(ResourceFactory::class);
$this->prepareSubject(
[],
true,
$mockedDriver,
$resourceFactory,
[],
['emitPreFileRenameSignal', 'emitPostFileRenameSignal', 'getUniqueName']
);
$resourceFactory->expects($this->once())->method('createFolderObject')->will($this->returnValue(new Folder($this->subject, '', '')));
/** @var File $file */
$file = new File(['identifier' => 'foo', 'name' => 'foo'], $this->subject);
$this->subject->expects($this->once())->method('getUniqueName')->will($this->returnValue('bar_01'));
$this->subject->expects($this->any())->method('getUniqueName')->will($this->returnValue('bar_01'));
$result = $this->subject->renameFile($file, 'bar');
// fake what the indexer does in updateIndexEntry
$result->updateProperties(['name' => $result->getIdentifier()]);
Expand All @@ -834,7 +848,7 @@ public function renameFileThrowsExceptionIfConflictAndConflictModeIsCancel(): vo
'foo',
1489593099
)));
$this->prepareSubject([], true, $mockedDriver, [], ['emitPreFileRenameSignal', 'emitPostFileRenameSignal']);
$this->prepareSubject([], true, $mockedDriver, null, [], ['emitPreFileRenameSignal', 'emitPostFileRenameSignal']);
/** @var File $file */
$file = new File(['identifier' => 'foo', 'name' => 'foo'], $this->subject);
$this->expectException(ExistingTargetFileNameException::class);
Expand All @@ -852,19 +866,17 @@ public function renameFileReplacesIfConflictAndConflictModeIsReplace(): void
1489593098
)));
$mockedDriver->expects($this->any())->method('sanitizeFileName')->will($this->returnValue('bar'));
$this->prepareSubject([], true, $mockedDriver, [], [
$resourceFactory = $this->prophesize(ResourceFactory::class);
$this->prepareSubject([], true, $mockedDriver, $resourceFactory->reveal(), [], [
'emitPreFileRenameSignal',
'emitPostFileRenameSignal',
'replaceFile',
'getPublicUrl',
'getResourceFactoryInstance'
]);
$this->subject->expects($this->once())->method('getPublicUrl')->will($this->returnValue('somePath'));
$resourceFactory = $this->prophesize(ResourceFactory::class);
$file = $this->prophesize(FileInterface::class);
$resourceFactory->getFileObjectFromCombinedIdentifier(Argument::any())->willReturn($file->reveal());
$this->subject->expects($this->once())->method('replaceFile')->will($this->returnValue($file->reveal()));
$this->subject->expects($this->any())->method('getResourceFactoryInstance')->will(self::returnValue($resourceFactory->reveal()));
/** @var File $file */
$file = new File(['identifier' => 'foo', 'name' => 'foo', 'missing' => false], $this->subject);
$this->subject->renameFile($file, 'bar', DuplicationBehavior::REPLACE);
Expand Down
Expand Up @@ -18,6 +18,7 @@
use TYPO3\CMS\Core\Resource\File;
use TYPO3\CMS\Core\Resource\FileReference;
use TYPO3\CMS\Core\Resource\ProcessedFile;
use TYPO3\CMS\Core\Resource\ResourceFactory;
use TYPO3\CMS\Extbase\Service\EnvironmentService;
use TYPO3\CMS\Extbase\Service\ImageService;
use TYPO3\TestingFramework\Core\Unit\UnitTestCase;
Expand Down Expand Up @@ -48,7 +49,8 @@ class ImageScriptServiceTest extends UnitTestCase
protected function setUp(): void
{
$this->environmentService = $this->createMock(EnvironmentService::class);
$this->subject = new ImageService($this->environmentService);
$resourceFactory = $this->createMock(ResourceFactory::class);
$this->subject = new ImageService($this->environmentService, $resourceFactory);
$_SERVER['HTTP_HOST'] = 'foo.bar';
}

Expand Down
Expand Up @@ -156,9 +156,7 @@ public function renderMethodCreatesExpectedTag(array $arguments, array $expected

$this->inject($image, 'originalFile', $originalFile);
$this->inject($image, 'propertiesOfFileReference', []);
$imageService = $this->getMockBuilder(ImageService::class)
->setMethods(['getImage', 'applyProcessingInstructions', 'getImageUri'])
->getMock();
$imageService = $this->createMock(ImageService::class);
$imageService->expects($this->once())->method('getImage')->willReturn($image);
$imageService->expects($this->once())->method('applyProcessingInstructions')->with($image, $this->anything())->willReturn($processedFile);
$imageService->expects($this->once())->method('getImageUri')->with($processedFile)->willReturn('test.png');
Expand Down
Expand Up @@ -3064,6 +3064,9 @@ public function typolinkReturnsCorrectLinksFiles($linkText, $configuration, $exp
$typoScriptFrontendControllerMockObject->tmpl = $templateServiceObjectMock;
$GLOBALS['TSFE'] = $typoScriptFrontendControllerMockObject;

$resourceFactory = $this->prophesize(ResourceFactory::class);
GeneralUtility::setSingletonInstance(ResourceFactory::class, $resourceFactory->reveal());

$this->subject->_set('typoScriptFrontendController', $typoScriptFrontendControllerMockObject);

$this->assertEquals($expectedResult, $this->subject->typoLink($linkText, $configuration));
Expand Down Expand Up @@ -3215,6 +3218,9 @@ public function typolinkReturnsCorrectLinksForFilesWithAbsRefPrefix(
'parseFunc.' => $this->getLibParseFunc(),
],
];
$resourceFactory = $this->prophesize(ResourceFactory::class);
GeneralUtility::setSingletonInstance(ResourceFactory::class, $resourceFactory->reveal());

$typoScriptFrontendControllerMockObject = $this->createMock(TypoScriptFrontendController::class);
$typoScriptFrontendControllerMockObject->config = [
'config' => [],
Expand Down
Expand Up @@ -424,7 +424,7 @@ public function renderReturnsFilesForFiles(array $configuration, string $expecte
$fileMap[] = [$i, [], $file];
}

$resourceFactory = $this->getMockBuilder(ResourceFactory::class)->getMock();
$resourceFactory = $this->createMock(ResourceFactory::class);
$resourceFactory->expects($this->any())
->method('getFileObject')
->will($this->returnValueMap($fileMap));
Expand Down Expand Up @@ -946,7 +946,7 @@ public function renderReturnsFilesForFolders(array $configuration, string $expec
}
}

$resourceFactory = $this->getMockBuilder(ResourceFactory::class)->getMock();
$resourceFactory = $this->createMock(ResourceFactory::class);
$resourceFactory->expects($this->any())
->method('getFolderObjectFromCombinedIdentifier')
->will($this->returnValueMap($folderMap));
Expand Down

0 comments on commit 191d0bd

Please sign in to comment.