AssetAdminTest::testSaveOrPublish() - Code Metrics - Inspection of "BUG Fix saving of folders (#344)" - silverstripe/silverstripe-asset-admin - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( 84ae23...c3e3e3 )

by Ingo

created 2016-12-23 00:35 UTC

AssetAdminTest::testSaveOrPublish() A

↳ Parent: AssetAdminTest

Complexity

Conditions	1
Paths	1

Size

Total Lines	17
Code Lines	11

Duplication

Lines	0
Ratio	0 %

Importance

Changes	1
Bugs	0	Features	0

Metric	Value
c	1
b	0
f	0
dl	0
loc	17
rs	9.4285
cc	1
eloc	11
nc	1
nop	0

<?php

namespace SilverStripe\AssetAdmin\Tests\Controller;

use SilverStripe\AssetAdmin\Tests\Controller\AssetAdminTest\FileExtension;
use SilverStripe\AssetAdmin\Tests\Controller\AssetAdminTest\FolderExtension;
use SilverStripe\Assets\File;
use SilverStripe\Assets\Folder;
use SilverStripe\Assets\Tests\Storage\AssetStoreTest\TestAssetStore;
use SilverStripe\Control\Director;
use SilverStripe\Control\Session;
use SilverStripe\Dev\FunctionalTest;
use SilverStripe\ORM\Versioning\Versioned;
use SilverStripe\Security\SecurityToken;

/**
 * Tests {@see AssetAdmin}
 */
class AssetAdminTest extends FunctionalTest
{

    protected static $fixture_file = '../fixtures.yml';

    /**
     * @var Session
     */
    protected $session = null;

    public function setUp()
    {
        parent::setUp();

        TestAssetStore::activate('AssetAdminTest');
        $memberID = $this->logInWithPermission('ADMIN');
        $this->session = Session::create(array('loggedInAs' => $memberID));

        File::add_extension(FileExtension::class);
        Folder::add_extension(FolderExtension::class);

        // Create a test folders for each of the fixture references
        foreach (File::get()->filter('ClassName', Folder::class) as $folder) {
            /** @var Folder $folder */
            $folder->copyVersionToStage(Versioned::DRAFT, Versioned::LIVE);
        }

        // Create a test files for each of the fixture references
        $content = str_repeat('x', 1000000);
        foreach (File::get()->exclude('ClassName', Folder::class) as $file) {
            /** @var File $file */
            $file->setFromString($content, $file->generateFilename());
            $file->copyVersionToStage(Versioned::DRAFT, Versioned::LIVE);
        }

        // Override FunctionalTest defaults
        SecurityToken::enable();
        $this->session->inst_set('SecurityID', SecurityToken::inst()->getValue());
    }

    public function tearDown()
    {
        File::remove_extension(FileExtension::class);
        Folder::remove_extension(FolderExtension::class);

        TestAssetStore::reset();
        parent::tearDown();
    }


    public function testApiHistory()
    {
        $file = $this->objFromFixture(File::class, 'file1');
        $response = Director::test(
            'admin/assets/api/history?fileId='. $file->ID,
            null,
            $this->session,
            'GET'
        );

        $this->assertFalse($response->isError());

        $body = json_decode($response->getBody(), true);

        $this->assertArrayHasKey('summary', $body[0]);
        $this->assertArrayHasKey('versionid', $body[0]);
        $this->assertArrayHasKey('summary', $body[0]);

        // test permission filtering and
    }

    public function testItCreatesFile()
// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
    {
        $folder1 = $this->objFromFixture(Folder::class, 'folder1');

        /** @skipUpgrade */
        $fileData = array('Upload' => $this->getUploadFile('Upload', 'testItCreatesFile.txt'));
        $_FILES = $fileData;
        $postedData = array_merge(
            $fileData,
            [
                    'ParentID' => $folder1->ID,
                    'SecurityID' => SecurityToken::inst()->getValue(),
                ]
        );
        $response = Director::test(
            'admin/assets/api/createFile',
            $postedData,
            $this->session,
            'POST'
        );
        $this->assertFalse($response->isError());
        $responseData = json_decode($response->getBody(), true);
        $newFile = File::get()->byID($responseData[0]['id']);
        $this->assertNotNull($newFile);
        $this->assertEquals($folder1->ID, $newFile->ParentID);
        $this->assertEquals('testItCreatesFile.txt', $newFile->Name);

        // Test that duplicate uploads are renamed
        $response = Director::test(
            'admin/assets/api/createFile',
            $postedData,
            $this->session,
            'POST'
        );
        $this->assertFalse($response->isError());
        $responseData = json_decode($response->getBody(), true);
        $newFile2 = File::get()->byID($responseData[0]['id']);
        $this->assertNotNull($newFile2);
        $this->assertEquals($folder1->ID, $newFile2->ParentID);
        $this->assertNotEquals($newFile->ID, $newFile2->ID);
        $this->assertEquals('testItCreatesFile-v2.txt', $newFile2->Name);
    }

    public function testItRestrictsCreateFileOnCanCreate()
// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
    {
        $folder = $this->objFromFixture(Folder::class, 'folder1');

        $fileData = array('Upload' => $this->getUploadFile('Upload', 'disallowCanCreate.txt'));
        $_FILES = $fileData;
        $response = Director::test(
            'admin/assets/api/createFile',
            array_merge(
                $fileData,
                [
                    'ParentID' => $folder->ID,
                    'SecurityID' => SecurityToken::inst()->getValue(),
                ]
            ),
            $this->session,
            'POST'
        );
        $this->assertTrue($response->isError());
        $this->assertEquals(403, $response->getStatusCode());
    }

    public function testItRestrictsCreateFileOnCanAddChildren()
// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
    {
        $folder = $this->objFromFixture(Folder::class, 'disallowCanAddChildren');

        /** @skipUpgrade */
        $fileData = array('Upload' => $this->getUploadFile('Upload', 'test.txt'));
        $_FILES = $fileData;
        $response = Director::test(
            'admin/assets/api/createFile',
            array_merge(
                $fileData,
                [
                    'ParentID' => $folder->ID,
                    'SecurityID' => SecurityToken::inst()->getValue(),
                ]
            ),
            $this->session,
            'POST'
        );
        $this->assertTrue($response->isError());
        $this->assertEquals(403, $response->getStatusCode());
    }

    public function testItRestrictsCreateFileOnExtension()
// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
    {
        $folder1 = $this->objFromFixture(
            Folder::class,
            'folder1'
        );

        /** @skipUpgrade */
        $fileData = array('Upload' => $this->getUploadFile('Upload', 'disallowed.php'));
        $_FILES = $fileData;
        $response = Director::test(
            'admin/assets/api/createFile',
            array_merge(
                $fileData,
                [
                    'ParentID' => $folder1->ID,
                    'SecurityID' => SecurityToken::inst()->getValue(),
                ]
            ),
            $this->session,
            'POST'
        );
        $this->assertTrue($response->isError());
        $this->assertEquals(400, $response->getStatusCode());
        $responseData = json_decode($response->getBody(), true);
        $this->assertContains(
            'Extension is not allowed',
            $responseData['message']['value']
        );
    }

    public function testItRestrictsUpdateFile()
    {
        $allowedFile = $this->objFromFixture(File::class, 'file1');
        $disallowedFile = $this->objFromFixture(File::class, 'disallowCanEdit');

        $response = Director::test(
            'admin/assets/FileEditForm',
            [
                'action_save' => 1,
                'ID' => $allowedFile->ID,
                'Name' => 'disallowCanEdit.txt',
                'Title' => 'new',
                'SecurityID' => SecurityToken::inst()->getValue(),
            ],
            $this->session
        );
        $this->assertFalse($response->isError());

        $response = Director::test(
            'admin/assets/FileEditForm',
            [
                'action_save' => 1,
                'ID' => $disallowedFile->ID,
                'Title' => 'new',
                'SecurityID' => SecurityToken::inst()->getValue(),
            ],
            $this->session
        );
        $this->assertTrue($response->isError());
    }

    /**
     * @param string $paramName
     * @param string $tmpFileName
     * @return array Emulating an entry in the $_FILES superglobal
     */
    protected function getUploadFile($paramName, $tmpFileName = 'AssetAdminTest.txt')

    {
        $tmpFilePath = TEMP_FOLDER . '/' . $tmpFileName;
        $tmpFileContent = '';
        for ($i = 0; $i < 10000; $i++) {
            $tmpFileContent .= '0';
        }
        file_put_contents($tmpFilePath, $tmpFileContent);

        // emulates the $_FILES array
        return array(
            'name' => $tmpFileName,
            'type' => 'text/plaintext',
            'size' => filesize($tmpFilePath),
            'tmp_name' => $tmpFilePath,
            'error' => UPLOAD_ERR_OK,
        );
    }

    public function testSaveOrPublish()
    {
        // Test rename folder
        $folder1ID = $this->idFromFixture(Folder::class, 'folder1');
        $response = $this->post(
            'admin/assets/fileEditForm',
            [
                'ID' => $folder1ID,
                'action_save' => 1,
                'Name' => 'folder1-renamed',
                'SecurityID' => SecurityToken::inst()->getValue(),
            ]
        );
        $this->assertFalse($response->isError());
        $folder1 = Folder::get()->byID($folder1ID);
        $this->assertEquals('folder1-renamed', $folder1->Name);
    }
}


1		<?php
2
3		namespace SilverStripe\AssetAdmin\Tests\Controller;
4
5		use SilverStripe\AssetAdmin\Tests\Controller\AssetAdminTest\FileExtension;
6		use SilverStripe\AssetAdmin\Tests\Controller\AssetAdminTest\FolderExtension;
7		use SilverStripe\Assets\File;
8		use SilverStripe\Assets\Folder;
9		use SilverStripe\Assets\Tests\Storage\AssetStoreTest\TestAssetStore;
10		use SilverStripe\Control\Director;
11		use SilverStripe\Control\Session;
12		use SilverStripe\Dev\FunctionalTest;
13		use SilverStripe\ORM\Versioning\Versioned;
14		use SilverStripe\Security\SecurityToken;
15
16		/**
17		* Tests {@see AssetAdmin}
18		*/
19		class AssetAdminTest extends FunctionalTest
20		{
21
22		protected static $fixture_file = '../fixtures.yml';
23
24		/**
25		* @var Session
26		*/
27		protected $session = null;
28
29		public function setUp()
30		{
31		parent::setUp();
32
33		TestAssetStore::activate('AssetAdminTest');
34		$memberID = $this->logInWithPermission('ADMIN');
35		$this->session = Session::create(array('loggedInAs' => $memberID));
36
37		File::add_extension(FileExtension::class);
38		Folder::add_extension(FolderExtension::class);
39
40		// Create a test folders for each of the fixture references
41		foreach (File::get()->filter('ClassName', Folder::class) as $folder) {
42		/** @var Folder $folder */
43		$folder->copyVersionToStage(Versioned::DRAFT, Versioned::LIVE);
44		}
45
46		// Create a test files for each of the fixture references
47		$content = str_repeat('x', 1000000);
48		foreach (File::get()->exclude('ClassName', Folder::class) as $file) {
49		/** @var File $file */
50		$file->setFromString($content, $file->generateFilename());
51		$file->copyVersionToStage(Versioned::DRAFT, Versioned::LIVE);
52		}
53
54		// Override FunctionalTest defaults
55		SecurityToken::enable();
56		$this->session->inst_set('SecurityID', SecurityToken::inst()->getValue());
57		}
58
59		public function tearDown()
60		{
61		File::remove_extension(FileExtension::class);
62		Folder::remove_extension(FolderExtension::class);
63
64		TestAssetStore::reset();
65		parent::tearDown();
66		}
67
68
69		public function testApiHistory()
70		{
71		$file = $this->objFromFixture(File::class, 'file1');
72		$response = Director::test(
73		'admin/assets/api/history?fileId='. $file->ID,
74		null,
75		$this->session,
76		'GET'
77		);
78
79		$this->assertFalse($response->isError());
80
81		$body = json_decode($response->getBody(), true);
82
83		$this->assertArrayHasKey('summary', $body[0]);
84		$this->assertArrayHasKey('versionid', $body[0]);
85		$this->assertArrayHasKey('summary', $body[0]);
86
87		// test permission filtering and
88		}
89
90		public function testItCreatesFile()
		0 ignored issues – show Coding Style introduced 2016-06-29 22:37 UTC by Report Bug Copy Issue Report `testItCreatesFile` uses the super-global variable `$_FILES` which is generally not recommended. Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: // Bad class Router { public function generate($path) { return $_SERVER['HOST'].$path; } } // Better class Router { private $host; public function __construct($host) { $this->host = $host; } public function generate($path) { return $this->host.$path; } } class Controller { public function myAction(Request $request) { // Instead of $page = isset($_GET['page']) ? intval($_GET['page']) : 1; // Better (assuming you use the Symfony2 request) $page = $request->query->get('page', 1); } } Loading history...
91		{
92		$folder1 = $this->objFromFixture(Folder::class, 'folder1');
93
94		/** @skipUpgrade */
95		$fileData = array('Upload' => $this->getUploadFile('Upload', 'testItCreatesFile.txt'));
96		$_FILES = $fileData;
97		$postedData = array_merge(
98		$fileData,
99		[
100		'ParentID' => $folder1->ID,
101		'SecurityID' => SecurityToken::inst()->getValue(),
102		]
103		);
104		$response = Director::test(
105		'admin/assets/api/createFile',
106		$postedData,
107		$this->session,
108		'POST'
109		);
110		$this->assertFalse($response->isError());
111		$responseData = json_decode($response->getBody(), true);
112		$newFile = File::get()->byID($responseData[0]['id']);
113		$this->assertNotNull($newFile);
114		$this->assertEquals($folder1->ID, $newFile->ParentID);
115		$this->assertEquals('testItCreatesFile.txt', $newFile->Name);
116
117		// Test that duplicate uploads are renamed
118		$response = Director::test(
119		'admin/assets/api/createFile',
120		$postedData,
121		$this->session,
122		'POST'
123		);
124		$this->assertFalse($response->isError());
125		$responseData = json_decode($response->getBody(), true);
126		$newFile2 = File::get()->byID($responseData[0]['id']);
127		$this->assertNotNull($newFile2);
128		$this->assertEquals($folder1->ID, $newFile2->ParentID);
129		$this->assertNotEquals($newFile->ID, $newFile2->ID);
130		$this->assertEquals('testItCreatesFile-v2.txt', $newFile2->Name);
131		}
132
133	View Code Duplication	public function testItRestrictsCreateFileOnCanCreate()
		0 ignored issues – show Duplication introduced 2016-06-29 22:37 UTC by Report Bug Copy Issue Report This method seems to be duplicated in your project. Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation. You can also find more detailed suggestions in the “Code” section of your repository. Loading history... Coding Style introduced 2016-06-29 22:37 UTC by Report Bug Copy Issue Report `testItRestrictsCreateFileOnCanCreate` uses the super-global variable `$_FILES` which is generally not recommended. Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: // Bad class Router { public function generate($path) { return $_SERVER['HOST'].$path; } } // Better class Router { private $host; public function __construct($host) { $this->host = $host; } public function generate($path) { return $this->host.$path; } } class Controller { public function myAction(Request $request) { // Instead of $page = isset($_GET['page']) ? intval($_GET['page']) : 1; // Better (assuming you use the Symfony2 request) $page = $request->query->get('page', 1); } } Loading history...
134		{
135		$folder = $this->objFromFixture(Folder::class, 'folder1');
136
137		$fileData = array('Upload' => $this->getUploadFile('Upload', 'disallowCanCreate.txt'));
138		$_FILES = $fileData;
139		$response = Director::test(
140		'admin/assets/api/createFile',
141		array_merge(
142		$fileData,
143		[
144		'ParentID' => $folder->ID,
145		'SecurityID' => SecurityToken::inst()->getValue(),
146		]
147		),
148		$this->session,
149		'POST'
150		);
151		$this->assertTrue($response->isError());
152		$this->assertEquals(403, $response->getStatusCode());
153		}
154
155	View Code Duplication	public function testItRestrictsCreateFileOnCanAddChildren()
		0 ignored issues – show Duplication introduced 2016-06-29 22:37 UTC by Report Bug Copy Issue Report This method seems to be duplicated in your project. Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation. You can also find more detailed suggestions in the “Code” section of your repository. Loading history... Coding Style introduced 2016-06-29 22:37 UTC by Report Bug Copy Issue Report `testItRestrictsCreateFileOnCanAddChildren` uses the super-global variable `$_FILES` which is generally not recommended. Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: // Bad class Router { public function generate($path) { return $_SERVER['HOST'].$path; } } // Better class Router { private $host; public function __construct($host) { $this->host = $host; } public function generate($path) { return $this->host.$path; } } class Controller { public function myAction(Request $request) { // Instead of $page = isset($_GET['page']) ? intval($_GET['page']) : 1; // Better (assuming you use the Symfony2 request) $page = $request->query->get('page', 1); } } Loading history...
156		{
157		$folder = $this->objFromFixture(Folder::class, 'disallowCanAddChildren');
158
159		/** @skipUpgrade */
160		$fileData = array('Upload' => $this->getUploadFile('Upload', 'test.txt'));
161		$_FILES = $fileData;
162		$response = Director::test(
163		'admin/assets/api/createFile',
164		array_merge(
165		$fileData,
166		[
167		'ParentID' => $folder->ID,
168		'SecurityID' => SecurityToken::inst()->getValue(),
169		]
170		),
171		$this->session,
172		'POST'
173		);
174		$this->assertTrue($response->isError());
175		$this->assertEquals(403, $response->getStatusCode());
176		}
177
178		public function testItRestrictsCreateFileOnExtension()
		0 ignored issues – show Coding Style introduced 2016-06-29 22:37 UTC by Report Bug Copy Issue Report `testItRestrictsCreateFileOnExtension` uses the super-global variable `$_FILES` which is generally not recommended. Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: // Bad class Router { public function generate($path) { return $_SERVER['HOST'].$path; } } // Better class Router { private $host; public function __construct($host) { $this->host = $host; } public function generate($path) { return $this->host.$path; } } class Controller { public function myAction(Request $request) { // Instead of $page = isset($_GET['page']) ? intval($_GET['page']) : 1; // Better (assuming you use the Symfony2 request) $page = $request->query->get('page', 1); } } Loading history...
179		{
180		$folder1 = $this->objFromFixture(
181		Folder::class,
182		'folder1'
183		);
184
185		/** @skipUpgrade */
186		$fileData = array('Upload' => $this->getUploadFile('Upload', 'disallowed.php'));
187		$_FILES = $fileData;
188		$response = Director::test(
189		'admin/assets/api/createFile',
190		array_merge(
191		$fileData,
192		[
193		'ParentID' => $folder1->ID,
194		'SecurityID' => SecurityToken::inst()->getValue(),
195		]
196		),
197		$this->session,
198		'POST'
199		);
200		$this->assertTrue($response->isError());
201		$this->assertEquals(400, $response->getStatusCode());
202		$responseData = json_decode($response->getBody(), true);
203		$this->assertContains(
204		'Extension is not allowed',
205		$responseData['message']['value']
206		);
207		}
208
209		public function testItRestrictsUpdateFile()
210		{
211		$allowedFile = $this->objFromFixture(File::class, 'file1');
212		$disallowedFile = $this->objFromFixture(File::class, 'disallowCanEdit');
213
214		$response = Director::test(
215		'admin/assets/FileEditForm',
216		[
217		'action_save' => 1,
218		'ID' => $allowedFile->ID,
219		'Name' => 'disallowCanEdit.txt',
220		'Title' => 'new',
221		'SecurityID' => SecurityToken::inst()->getValue(),
222		],
223		$this->session
224		);
225		$this->assertFalse($response->isError());
226
227		$response = Director::test(
228		'admin/assets/FileEditForm',
229		[
230		'action_save' => 1,
231		'ID' => $disallowedFile->ID,
232		'Title' => 'new',
233		'SecurityID' => SecurityToken::inst()->getValue(),
234		],
235		$this->session
236		);
237		$this->assertTrue($response->isError());
238		}
239
240		/**
241		* @param string $paramName
242		* @param string $tmpFileName
243		* @return array Emulating an entry in the $_FILES superglobal
244		*/
245		protected function getUploadFile($paramName, $tmpFileName = 'AssetAdminTest.txt')
		0 ignored issues – show Unused Code introduced 2016-06-29 22:37 UTC by Report Bug Copy Issue Report The parameter `$paramName` is not used and could be removed. This check looks from parameters that have been defined for a function or method, but which are not used in the method body. Loading history...
246		{
247		$tmpFilePath = TEMP_FOLDER . '/' . $tmpFileName;
248		$tmpFileContent = '';
249		for ($i = 0; $i < 10000; $i++) {
250		$tmpFileContent .= '0';
251		}
252		file_put_contents($tmpFilePath, $tmpFileContent);
253
254		// emulates the $_FILES array
255		return array(
256		'name' => $tmpFileName,
257		'type' => 'text/plaintext',
258		'size' => filesize($tmpFilePath),
259		'tmp_name' => $tmpFilePath,
260		'error' => UPLOAD_ERR_OK,
261		);
262		}
263
264		public function testSaveOrPublish()
265		{
266		// Test rename folder
267		$folder1ID = $this->idFromFixture(Folder::class, 'folder1');
268		$response = $this->post(
269		'admin/assets/fileEditForm',
270		[
271		'ID' => $folder1ID,
272		'action_save' => 1,
273		'Name' => 'folder1-renamed',
274		'SecurityID' => SecurityToken::inst()->getValue(),
275		]
276		);
277		$this->assertFalse($response->isError());
278		$folder1 = Folder::get()->byID($folder1ID);
279		$this->assertEquals('folder1-renamed', $folder1->Name);
280		}
281		}
282

silverstripe / silverstripe-asset-admin

Push — master ( 84ae23...c3e3e3 )

AssetAdminTest::testSaveOrPublish() A

Complexity

Size

Duplication

Importance

Duplication Side-by-Side

Filter issues like