From 0a89073c475632a296b7315d96894503a78bbf43 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Tue, 1 Dec 2015 17:06:48 +0100 Subject: Run .htaccess updates in any case This is the same what we also do in updater.php and thus this aligns the code. Makes the code paths more consistent. --- tests/lib/setup.php | 13 ------------- 1 file changed, 13 deletions(-) (limited to 'tests') diff --git a/tests/lib/setup.php b/tests/lib/setup.php index 72c84520056..bc78c14008f 100644 --- a/tests/lib/setup.php +++ b/tests/lib/setup.php @@ -130,17 +130,4 @@ class Test_OC_Setup extends \Test\TestCase { ->will($this->returnValue('NotAnArray')); $this->setupClass->getSupportedDatabases(); } - - /** - * This is actual more an integration test whether the version parameter in the .htaccess - * was updated as well when the version has been incremented. - * If it hasn't this test will fail. - */ - public function testHtaccessIsCurrent() { - $result = self::invokePrivate( - $this->setupClass, - 'isCurrentHtaccess' - ); - $this->assertTrue($result); - } } -- cgit v1.2.3 From 28e9bc115615aab6c951fb846b8bd28aa517331e Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Tue, 8 Dec 2015 16:33:39 +0100 Subject: Fix more unit tests to pass a mock storage instead of null to FileInfo --- apps/dav/tests/unit/connector/sabre/file.php | 44 ++++++++++++++++------------ tests/lib/files/node/file.php | 10 ++++++- tests/lib/files/node/folder.php | 10 ++++++- tests/lib/files/node/node.php | 10 ++++++- 4 files changed, 53 insertions(+), 21 deletions(-) (limited to 'tests') diff --git a/apps/dav/tests/unit/connector/sabre/file.php b/apps/dav/tests/unit/connector/sabre/file.php index 2a6cf46ef16..ad4c1d29ed4 100644 --- a/apps/dav/tests/unit/connector/sabre/file.php +++ b/apps/dav/tests/unit/connector/sabre/file.php @@ -48,6 +48,14 @@ class File extends \Test\TestCase { parent::tearDown(); } + private function getMockStorage() { + $storage = $this->getMock('\OCP\Files\Storage'); + $storage->expects($this->any()) + ->method('getId') + ->will($this->returnValue('home::someuser')); + return $storage; + } + /** * @param string $string */ @@ -149,7 +157,7 @@ class File extends \Test\TestCase { ->method('getRelativePath') ->will($this->returnArgument(0)); - $info = new \OC\Files\FileInfo('/test.txt', null, null, array( + $info = new \OC\Files\FileInfo('/test.txt', $this->getMockStorage(), null, array( 'permissions' => \OCP\Constants::PERMISSION_ALL ), null); @@ -209,7 +217,7 @@ class File extends \Test\TestCase { $_SERVER['HTTP_OC_CHUNKED'] = true; - $info = new \OC\Files\FileInfo('/test.txt-chunking-12345-2-0', null, null, [ + $info = new \OC\Files\FileInfo('/test.txt-chunking-12345-2-0', $this->getMockStorage(), null, [ 'permissions' => \OCP\Constants::PERMISSION_ALL ], null); $file = new \OCA\DAV\Connector\Sabre\File($view, $info); @@ -219,7 +227,7 @@ class File extends \Test\TestCase { $this->assertNull($file->put('test data one')); $file->releaseLock(ILockingProvider::LOCK_SHARED); - $info = new \OC\Files\FileInfo('/test.txt-chunking-12345-2-1', null, null, [ + $info = new \OC\Files\FileInfo('/test.txt-chunking-12345-2-1', $this->getMockStorage(), null, [ 'permissions' => \OCP\Constants::PERMISSION_ALL ], null); $file = new \OCA\DAV\Connector\Sabre\File($view, $info); @@ -261,7 +269,7 @@ class File extends \Test\TestCase { $info = new \OC\Files\FileInfo( $viewRoot . '/' . ltrim($path, '/'), - null, + $this->getMockStorage(), null, ['permissions' => \OCP\Constants::PERMISSION_ALL], null @@ -450,7 +458,7 @@ class File extends \Test\TestCase { $_SERVER['CONTENT_LENGTH'] = 123456; $_SERVER['REQUEST_METHOD'] = 'PUT'; - $info = new \OC\Files\FileInfo('/test.txt', null, null, array( + $info = new \OC\Files\FileInfo('/test.txt', $this->getMockStorage(), null, array( 'permissions' => \OCP\Constants::PERMISSION_ALL ), null); @@ -483,7 +491,7 @@ class File extends \Test\TestCase { // simulate situation where the target file is locked $view->lockFile('/test.txt', ILockingProvider::LOCK_EXCLUSIVE); - $info = new \OC\Files\FileInfo('/' . $this->user . '/files/test.txt', null, null, array( + $info = new \OC\Files\FileInfo('/' . $this->user . '/files/test.txt', $this->getMockStorage(), null, array( 'permissions' => \OCP\Constants::PERMISSION_ALL ), null); @@ -518,7 +526,7 @@ class File extends \Test\TestCase { $_SERVER['HTTP_OC_CHUNKED'] = true; - $info = new \OC\Files\FileInfo('/' . $this->user . '/files/test.txt-chunking-12345-2-0', null, null, [ + $info = new \OC\Files\FileInfo('/' . $this->user . '/files/test.txt-chunking-12345-2-0', $this->getMockStorage(), null, [ 'permissions' => \OCP\Constants::PERMISSION_ALL ], null); $file = new \OCA\DAV\Connector\Sabre\File($view, $info); @@ -526,7 +534,7 @@ class File extends \Test\TestCase { $this->assertNull($file->put('test data one')); $file->releaseLock(ILockingProvider::LOCK_SHARED); - $info = new \OC\Files\FileInfo('/' . $this->user . '/files/test.txt-chunking-12345-2-1', null, null, [ + $info = new \OC\Files\FileInfo('/' . $this->user . '/files/test.txt-chunking-12345-2-1', $this->getMockStorage(), null, [ 'permissions' => \OCP\Constants::PERMISSION_ALL ], null); $file = new \OCA\DAV\Connector\Sabre\File($view, $info); @@ -555,7 +563,7 @@ class File extends \Test\TestCase { ->method('getRelativePath') ->will($this->returnArgument(0)); - $info = new \OC\Files\FileInfo('/*', null, null, array( + $info = new \OC\Files\FileInfo('/*', $this->getMockStorage(), null, array( 'permissions' => \OCP\Constants::PERMISSION_ALL ), null); $file = new \OCA\DAV\Connector\Sabre\File($view, $info); @@ -591,7 +599,7 @@ class File extends \Test\TestCase { ->method('getRelativePath') ->will($this->returnArgument(0)); - $info = new \OC\Files\FileInfo('/*', null, null, array( + $info = new \OC\Files\FileInfo('/*', $this->getMockStorage(), null, array( 'permissions' => \OCP\Constants::PERMISSION_ALL ), null); $file = new \OCA\DAV\Connector\Sabre\File($view, $info); @@ -618,7 +626,7 @@ class File extends \Test\TestCase { $_SERVER['CONTENT_LENGTH'] = 12345; $_SERVER['REQUEST_METHOD'] = 'PUT'; - $info = new \OC\Files\FileInfo('/test.txt', null, null, array( + $info = new \OC\Files\FileInfo('/test.txt', $this->getMockStorage(), null, array( 'permissions' => \OCP\Constants::PERMISSION_ALL ), null); @@ -654,7 +662,7 @@ class File extends \Test\TestCase { ->method('unlink') ->will($this->returnValue(true)); - $info = new \OC\Files\FileInfo('/test.txt', null, null, array( + $info = new \OC\Files\FileInfo('/test.txt', $this->getMockStorage(), null, array( 'permissions' => \OCP\Constants::PERMISSION_ALL ), null); @@ -672,7 +680,7 @@ class File extends \Test\TestCase { $view = $this->getMock('\OC\Files\View', array()); - $info = new \OC\Files\FileInfo('/test.txt', null, null, array( + $info = new \OC\Files\FileInfo('/test.txt', $this->getMockStorage(), null, array( 'permissions' => 0 ), null); @@ -695,7 +703,7 @@ class File extends \Test\TestCase { ->method('unlink') ->will($this->returnValue(false)); - $info = new \OC\Files\FileInfo('/test.txt', null, null, array( + $info = new \OC\Files\FileInfo('/test.txt', $this->getMockStorage(), null, array( 'permissions' => \OCP\Constants::PERMISSION_ALL ), null); @@ -718,7 +726,7 @@ class File extends \Test\TestCase { ->method('unlink') ->willThrowException(new ForbiddenException('', true)); - $info = new \OC\Files\FileInfo('/test.txt', null, null, array( + $info = new \OC\Files\FileInfo('/test.txt', $this->getMockStorage(), null, array( 'permissions' => \OCP\Constants::PERMISSION_ALL ), null); @@ -753,7 +761,7 @@ class File extends \Test\TestCase { $path = 'test-locking.txt'; $info = new \OC\Files\FileInfo( '/' . $this->user . '/files/' . $path, - null, + $this->getMockStorage(), null, ['permissions' => \OCP\Constants::PERMISSION_ALL], null @@ -865,7 +873,7 @@ class File extends \Test\TestCase { ->method('fopen') ->will($this->returnValue(false)); - $info = new \OC\Files\FileInfo('/test.txt', null, null, array( + $info = new \OC\Files\FileInfo('/test.txt', $this->getMockStorage(), null, array( 'permissions' => \OCP\Constants::PERMISSION_ALL ), null); @@ -883,7 +891,7 @@ class File extends \Test\TestCase { ->method('fopen') ->willThrowException(new ForbiddenException('', true)); - $info = new \OC\Files\FileInfo('/test.txt', null, null, array( + $info = new \OC\Files\FileInfo('/test.txt', $this->getMockStorage(), null, array( 'permissions' => \OCP\Constants::PERMISSION_ALL ), null); diff --git a/tests/lib/files/node/file.php b/tests/lib/files/node/file.php index d0072949c7f..ccc777c499f 100644 --- a/tests/lib/files/node/file.php +++ b/tests/lib/files/node/file.php @@ -21,8 +21,16 @@ class File extends \Test\TestCase { $this->user = new \OC\User\User('', new \Test\Util\User\Dummy); } + protected function getMockStorage() { + $storage = $this->getMock('\OCP\Files\Storage'); + $storage->expects($this->any()) + ->method('getId') + ->will($this->returnValue('home::someuser')); + return $storage; + } + protected function getFileInfo($data) { - return new FileInfo('', null, '', $data, null); + return new FileInfo('', $this->getMockStorage(), '', $data, null); } public function testDelete() { diff --git a/tests/lib/files/node/folder.php b/tests/lib/files/node/folder.php index d95e1b5d2b2..09bf32561e6 100644 --- a/tests/lib/files/node/folder.php +++ b/tests/lib/files/node/folder.php @@ -31,8 +31,16 @@ class Folder extends \Test\TestCase { $this->user = new \OC\User\User('', new \Test\Util\User\Dummy); } + protected function getMockStorage() { + $storage = $this->getMock('\OCP\Files\Storage'); + $storage->expects($this->any()) + ->method('getId') + ->will($this->returnValue('home::someuser')); + return $storage; + } + protected function getFileInfo($data) { - return new FileInfo('', null, '', $data, null); + return new FileInfo('', $this->getMockStorage(), '', $data, null); } public function testDelete() { diff --git a/tests/lib/files/node/node.php b/tests/lib/files/node/node.php index afcf4cbabaa..a1693b034fa 100644 --- a/tests/lib/files/node/node.php +++ b/tests/lib/files/node/node.php @@ -18,8 +18,16 @@ class Node extends \Test\TestCase { $this->user = new \OC\User\User('', new \Test\Util\User\Dummy); } + protected function getMockStorage() { + $storage = $this->getMock('\OCP\Files\Storage'); + $storage->expects($this->any()) + ->method('getId') + ->will($this->returnValue('home::someuser')); + return $storage; + } + protected function getFileInfo($data) { - return new FileInfo('', null, '', $data, null); + return new FileInfo('', $this->getMockStorage(), '', $data, null); } public function testStat() { -- cgit v1.2.3 From 5c0be3b565035f173b611f82e0b7ba6bd386b2e0 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 9 Dec 2015 09:43:23 +0100 Subject: Fix the last insert id test by changing to an autoincrement table --- tests/lib/db/querybuilder/querybuildertest.php | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) (limited to 'tests') diff --git a/tests/lib/db/querybuilder/querybuildertest.php b/tests/lib/db/querybuilder/querybuildertest.php index c8e029d9e40..de8f84ac345 100644 --- a/tests/lib/db/querybuilder/querybuildertest.php +++ b/tests/lib/db/querybuilder/querybuildertest.php @@ -1124,11 +1124,12 @@ class QueryBuilderTest extends \Test\TestCase { $this->assertTrue(true); } - $qB->insert('appconfig') + $qB->insert('properties') ->values([ - 'appid' => $qB->expr()->literal('testFirstResult'), - 'configkey' => $qB->expr()->literal('testing' . 50), - 'configvalue' => $qB->expr()->literal(100 - 50), + 'userid' => $qB->expr()->literal('testFirstResult'), + 'propertypath' => $qB->expr()->literal('testing'), + 'propertyname' => $qB->expr()->literal('testing'), + 'propertyvalue' => $qB->expr()->literal('testing'), ]) ->execute(); @@ -1136,7 +1137,18 @@ class QueryBuilderTest extends \Test\TestCase { $this->assertNotNull($actual); $this->assertInternalType('int', $actual); - $this->assertEquals($this->connection->lastInsertId('*PREFIX*appconfig'), $actual); + $this->assertEquals($this->connection->lastInsertId('*PREFIX*properties'), $actual); + + $qB->delete('properties') + ->where($qB->expr()->eq('userid', $qB->expr()->literal('testFirstResult'))) + ->execute(); + + try { + $qB->getLastInsertId(); + $this->fail('getLastInsertId() should throw an exception, when being called after delete()'); + } catch (\BadMethodCallException $e) { + $this->assertTrue(true); + } } public function dataGetTableName() { -- cgit v1.2.3 From bb02488cc2b25d9dbf0dde536e24ffa2d6a04ff7 Mon Sep 17 00:00:00 2001 From: Thomas Müller Date: Wed, 9 Dec 2015 12:10:12 +0100 Subject: remove deprecated strict setting --- tests/phpunit-autotest-external.xml | 1 - 1 file changed, 1 deletion(-) (limited to 'tests') diff --git a/tests/phpunit-autotest-external.xml b/tests/phpunit-autotest-external.xml index b9402bfa572..31d2e395a01 100644 --- a/tests/phpunit-autotest-external.xml +++ b/tests/phpunit-autotest-external.xml @@ -1,6 +1,5 @@ Date: Mon, 23 Nov 2015 23:53:55 +0100 Subject: add icommentsmanger and icomment implementation register CommentsManager service, allow override, document in config.sample.php don't insert autoincrement ids in tests, because of dislikes from oracle and pgsql specify timezone in null date only accepts strings for ID parameter that can be converted to int replace forgotten hardcoded IDs in tests react on deleted users react on file deletion Postgresql compatibility lastInsertId needs *PREFIX* with the table name do not listen for file deletion, because it is not reliable (trashbin, external storages) add runtime cache for comments --- config/config.sample.php | 13 + lib/private/comments/comment.php | 350 +++++++++++++++ lib/private/comments/manager.php | 566 ++++++++++++++++++++++++ lib/private/comments/managerfactory.php | 24 + lib/private/server.php | 11 + lib/public/comments/icomment.php | 19 +- lib/public/comments/icommentsmanager.php | 22 +- lib/public/comments/icommentsmanagerfactory.php | 22 + tests/lib/comments/comment.php | 102 +++++ tests/lib/comments/fakefactory.php | 22 + tests/lib/comments/manager.php | 560 +++++++++++++++++++++++ tests/lib/server.php | 1 + 12 files changed, 1706 insertions(+), 6 deletions(-) create mode 100644 lib/private/comments/comment.php create mode 100644 lib/private/comments/manager.php create mode 100644 lib/private/comments/managerfactory.php create mode 100644 lib/public/comments/icommentsmanagerfactory.php create mode 100644 tests/lib/comments/comment.php create mode 100644 tests/lib/comments/fakefactory.php create mode 100644 tests/lib/comments/manager.php (limited to 'tests') diff --git a/config/config.sample.php b/config/config.sample.php index c3abe3a2b87..9b10792944f 100644 --- a/config/config.sample.php +++ b/config/config.sample.php @@ -814,6 +814,19 @@ $CONFIG = array( */ 'enforce_home_folder_naming_rule' => true, +/** + * Comments + * + * Global settings for the Comments infrastructure + */ + +/** + * Replaces the default Comments Manager Factory. This can be utilized if an + * own or 3rdParty CommentsManager should be used that – for instance – uses the + * filesystem instead of the database to keep the comments. + */ +'comments.managerFactory' => '\OC\Comments\ManagerFactory', + /** * Maintenance * diff --git a/lib/private/comments/comment.php b/lib/private/comments/comment.php new file mode 100644 index 00000000000..2e9e4bd3d84 --- /dev/null +++ b/lib/private/comments/comment.php @@ -0,0 +1,350 @@ + '', + 'parentId' => '0', + 'topmostParentId' => '0', + 'childrenCount' => '0', + 'message' => '', + 'verb' => '', + 'actorType' => '', + 'actorId' => '', + 'objectType' => '', + 'objectId' => '', + 'creationDT' => null, + 'latestChildDT' => null, + ]; + + /** + * Comment constructor. + * + * @param [] $data optional, array with keys according to column names from + * the comments database scheme + */ + public function __construct(array $data = null) { + if(is_array($data)) { + $this->fromArray($data); + } + } + + /** + * returns the ID of the comment + * + * It may return an empty string, if the comment was not stored. + * It is expected that the concrete Comment implementation gives an ID + * by itself (e.g. after saving). + * + * @return string + * @since 9.0.0 + */ + public function getId() { + return $this->data['id']; + } + + /** + * sets the ID of the comment and returns itself + * + * It is only allowed to set the ID only, if the current id is an empty + * string (which means it is not stored in a database, storage or whatever + * the concrete implementation does), or vice versa. Changing a given ID is + * not permitted and must result in an IllegalIDChangeException. + * + * @param string $id + * @return IComment + * @throws IllegalIDChangeException + * @since 9.0.0 + */ + public function setId($id) { + if(!is_string($id)) { + throw new \InvalidArgumentException('String expected.'); + } + + if($this->data['id'] === '' || ($this->data['id'] !== '' && $id === '')) { + $this->data['id'] = $id; + return $this; + } + + throw new IllegalIDChangeException('Not allowed to assign a new ID to an already saved comment.'); + } + + /** + * returns the parent ID of the comment + * + * @return string + * @since 9.0.0 + */ + public function getParentId() { + return $this->data['parentId']; + } + + /** + * sets the parent ID and returns itself + * + * @param string $parentId + * @return IComment + * @since 9.0.0 + */ + public function setParentId($parentId) { + if(!is_string($parentId)) { + throw new \InvalidArgumentException('String expected.'); + } + $this->data['parentId'] = $parentId; + return $this; + } + + /** + * returns the topmost parent ID of the comment + * + * @return string + * @since 9.0.0 + */ + public function getTopmostParentId() { + return $this->data['topmostParentId']; + } + + + /** + * sets the topmost parent ID and returns itself + * + * @param string $id + * @return IComment + * @since 9.0.0 + */ + public function setTopmostParentId($id) { + if(!is_string($id)) { + throw new \InvalidArgumentException('String expected.'); + } + $this->data['topmostParentId'] = $id; + return $this; + } + + /** + * returns the number of children + * + * @return int + * @since 9.0.0 + */ + public function getChildrenCount() { + return $this->data['childrenCount']; + } + + /** + * sets the number of children + * + * @param int $count + * @return IComment + * @since 9.0.0 + */ + public function setChildrenCount($count) { + if(!is_int($count)) { + throw new \InvalidArgumentException('Integer expected.'); + } + $this->data['childrenCount'] = $count; + return $this; + } + + /** + * returns the message of the comment + * + * @return string + * @since 9.0.0 + */ + public function getMessage() { + return $this->data['message']; + } + + /** + * sets the message of the comment and returns itself + * + * @param string $message + * @return IComment + * @since 9.0.0 + */ + public function setMessage($message) { + if(!is_string($message)) { + throw new \InvalidArgumentException('String expected.'); + } + $this->data['message'] = $message; + return $this; + } + + /** + * returns the verb of the comment + * + * @return string + * @since 9.0.0 + */ + public function getVerb() { + return $this->data['verb']; + } + + /** + * sets the verb of the comment, e.g. 'comment' or 'like' + * + * @param string $verb + * @return IComment + * @since 9.0.0 + */ + public function setVerb($verb) { + if(!is_string($verb)) { + throw new \InvalidArgumentException('String expected.'); + } + $this->data['verb'] = $verb; + return $this; + } + + /** + * returns the actor type + * + * @return string + * @since 9.0.0 + */ + public function getActorType() { + return $this->data['actorType']; + } + + /** + * returns the actor ID + * + * @return string + * @since 9.0.0 + */ + public function getActorId() { + return $this->data['actorId']; + } + + /** + * sets (overwrites) the actor type and id + * + * @param string $actorType e.g. 'user' + * @param string $actorId e.g. 'zombie234' + * @return IComment + * @since 9.0.0 + */ + public function setActor($actorType, $actorId) { + if(!is_string($actorType) || !is_string($actorId)) { + throw new \InvalidArgumentException('String expected.'); + } + $this->data['actorType'] = $actorType; + $this->data['actorId'] = $actorId; + return $this; + } + + /** + * returns the creation date of the comment. + * + * If not explicitly set, it shall default to the time of initialization. + * + * @return \DateTime + * @since 9.0.0 + */ + public function getCreationDateTime() { + return $this->data['creationDT']; + } + + /** + * sets the creation date of the comment and returns itself + * + * @param \DateTime $timestamp + * @return IComment + * @since 9.0.0 + */ + public function setCreationDateTime(\DateTime $timestamp) { + $this->data['creationDT'] = $timestamp; + return $this; + } + + /** + * returns the timestamp of the most recent child + * + * @return int + * @since 9.0.0 + */ + public function getLatestChildDateTime() { + return $this->data['latestChildDT']; + } + + /** + * sets the date of the most recent child + * + * @param \DateTime $dateTime + * @return IComment + * @since 9.0.0 + */ + public function setLatestChildDateTime(\DateTime $dateTime = null) { + $this->data['latestChildDT'] = $dateTime; + return $this; + } + + /** + * returns the object type the comment is attached to + * + * @return string + * @since 9.0.0 + */ + public function getObjectType() { + return $this->data['objectType']; + } + + /** + * returns the object id the comment is attached to + * + * @return string + * @since 9.0.0 + */ + public function getObjectId() { + return $this->data['objectId']; + } + + /** + * sets (overwrites) the object of the comment + * + * @param string $objectType e.g. 'file' + * @param string $objectId e.g. '16435' + * @return IComment + * @since 9.0.0 + */ + public function setObject($objectType, $objectId) { + if(!is_string($objectType) || !is_string($objectId)) { + throw new \InvalidArgumentException('String expected.'); + } + $this->data['objectType'] = $objectType; + $this->data['objectId'] = $objectId; + return $this; + } + + /** + * sets the comment data based on an array with keys as taken from the + * database. + * + * @param [] $data + * @return IComment + */ + protected function fromArray($data) { + foreach(array_keys($data) as $key) { + // translate DB keys to internal setter names + $setter = 'set' . str_replace('_', '', ucwords($key,'_')); + $setter = str_replace('Timestamp', 'DateTime', $setter); + + if(method_exists($this, $setter)) { + $this->$setter($data[$key]); + } + } + + foreach(['actor', 'object'] as $role) { + if(isset($data[$role . '_type']) && isset($data[$role . '_id'])) { + $setter = 'set' . ucfirst($role); + $this->$setter($data[$role . '_type'], $data[$role . '_id']); + } + } + + return $this; + } +} diff --git a/lib/private/comments/manager.php b/lib/private/comments/manager.php new file mode 100644 index 00000000000..78b2b71c8dd --- /dev/null +++ b/lib/private/comments/manager.php @@ -0,0 +1,566 @@ +dbConn = $dbConn; + $this->logger = $logger; + $userManager->listen('\OC\User', 'postDelete', function($user) { + /** @var \OCP\IUser $user */ + $this->deleteReferencesOfActor('user', $user->getUid()); + }); + } + + /** + * converts data base data into PHP native, proper types as defined by + * IComment interface. + * + * @param array $data + * @return array + */ + protected function normalizeDatabaseData(array $data) { + $data['id'] = strval($data['id']); + $data['parent_id'] = strval($data['parent_id']); + $data['topmost_parent_id'] = strval($data['topmost_parent_id']); + $data['creation_timestamp'] = new \DateTime($data['creation_timestamp']); + $data['latest_child_timestamp'] = new \DateTime($data['latest_child_timestamp']); + $data['children_count'] = intval($data['children_count']); + return $data; + } + + /** + * prepares a comment for an insert or update operation after making sure + * all necessary fields have a value assigned. + * + * @param IComment $comment + * @return IComment + * @throws \UnexpectedValueException + */ + protected function prepareCommentForDatabaseWrite(IComment $comment) { + if( empty($comment->getActorType()) + || empty($comment->getActorId()) + || empty($comment->getObjectType()) + || empty($comment->getObjectId()) + || empty($comment->getVerb()) + ) { + throw new \UnexpectedValueException('Actor, Object and Verb information must be provided for saving'); + } + + if($comment->getId() === '') { + $comment->setChildrenCount(0); + $comment->setLatestChildDateTime(new \DateTime('0000-00-00 00:00:00', new \DateTimeZone('UTC'))); + $comment->setLatestChildDateTime(null); + } + + if(is_null($comment->getCreationDateTime())) { + $comment->setCreationDateTime(new \DateTime()); + } + + if($comment->getParentId() !== '0') { + $comment->setTopmostParentId($this->determineTopmostParentId($comment->getParentId())); + } else { + $comment->setTopmostParentId('0'); + } + + $this->cache($comment); + + return $comment; + } + + /** + * returns the topmost parent id of a given comment identified by ID + * + * @param string $id + * @return string + * @throws NotFoundException + */ + protected function determineTopmostParentId($id) { + $comment = $this->get($id); + if($comment->getParentId() === '0') { + return $comment->getId(); + } else { + return $this->determineTopmostParentId($comment->getId()); + } + } + + /** + * updates child information of a comment + * + * @param string $id + * @param \DateTime $cDateTime the date time of the most recent child + * @throws NotFoundException + */ + protected function updateChildrenInformation($id, \DateTime $cDateTime) { + $qb = $this->dbConn->getQueryBuilder(); + $query = $qb->select($qb->createFunction('COUNT(`id`)')) + ->from('comments') + ->where($qb->expr()->eq('parent_id', $qb->createParameter('id'))) + ->setParameter('id', $id); + + $resultStatement = $query->execute(); + $data = $resultStatement->fetch(\PDO::FETCH_NUM); + $resultStatement->closeCursor(); + $children = intval($data[0]); + + $comment = $this->get($id); + $comment->setChildrenCount($children); + $comment->setLatestChildDateTime($cDateTime); + $this->save($comment); + } + + /** + * Tests whether actor or object type and id parameters are acceptable. + * Throws exception if not. + * + * @param string $role + * @param string $type + * @param string $id + * @throws \InvalidArgumentException + */ + protected function checkRoleParameters($role, $type, $id) { + if( + !is_string($type) || empty($type) + || !is_string($id) || empty($id) + ) { + throw new \InvalidArgumentException($role . ' parameters must be string and not empty'); + } + } + + /** + * run-time caches a comment + * + * @param IComment $comment + */ + protected function cache(IComment $comment) { + $id = $comment->getId(); + if(empty($id)) { + return; + } + $this->commentsCache[strval($id)] = $comment; + } + + /** + * removes an entry from the comments run time cache + * + * @param mixed $id the comment's id + */ + protected function uncache($id) { + $id = strval($id); + if (isset($this->commentsCache[$id])) { + unset($this->commentsCache[$id]); + } + } + + /** + * returns a comment instance + * + * @param string $id the ID of the comment + * @return IComment + * @throws NotFoundException + * @throws \InvalidArgumentException + * @since 9.0.0 + */ + public function get($id) { + if(intval($id) === 0) { + throw new \InvalidArgumentException('IDs must be translatable to a number in this implementation.'); + } + + if(isset($this->commentsCache[$id])) { + return $this->commentsCache[$id]; + } + + $qb = $this->dbConn->getQueryBuilder(); + $resultStatement = $qb->select('*') + ->from('comments') + ->where($qb->expr()->eq('id', $qb->createParameter('id'))) + ->setParameter('id', $id, \PDO::PARAM_INT) + ->execute(); + + $data = $resultStatement->fetch(); + $resultStatement->closeCursor(); + if(!$data) { + throw new NotFoundException(); + } + + $comment = new Comment($this->normalizeDatabaseData($data)); + $this->cache($comment); + return $comment; + } + + /** + * returns the comment specified by the id and all it's child comments. + * At this point of time, we do only support one level depth. + * + * @param string $id + * @param int $limit max number of entries to return, 0 returns all + * @param int $offset the start entry + * @return array + * @since 9.0.0 + * + * The return array looks like this + * [ + * 'comment' => IComment, // root comment + * 'replies' => + * [ + * 0 => + * [ + * 'comment' => IComment, + * 'replies' => [] + * ] + * 1 => + * [ + * 'comment' => IComment, + * 'replies'=> [] + * ], + * … + * ] + * ] + */ + public function getTree($id, $limit = 0, $offset = 0) { + $tree = []; + $tree['comment'] = $this->get($id); + $tree['replies'] = []; + + $qb = $this->dbConn->getQueryBuilder(); + $query = $qb->select('*') + ->from('comments') + ->where($qb->expr()->eq('topmost_parent_id', $qb->createParameter('id'))) + ->orderBy('creation_timestamp', 'DESC') + ->setParameter('id', $id); + + if($limit > 0) { + $query->setMaxResults($limit); + } + if($offset > 0) { + $query->setFirstResult($offset); + } + + $resultStatement = $query->execute(); + while($data = $resultStatement->fetch()) { + $comment = new Comment($this->normalizeDatabaseData($data)); + $this->cache($comment); + $tree['replies'][] = [ + 'comment' => $comment, + 'replies' => [] + ]; + } + $resultStatement->closeCursor(); + + return $tree; + } + + /** + * returns comments for a specific object (e.g. a file). + * + * The sort order is always newest to oldest. + * + * @param string $objectType the object type, e.g. 'files' + * @param string $objectId the id of the object + * @param int $limit optional, number of maximum comments to be returned. if + * not specified, all comments are returned. + * @param int $offset optional, starting point + * @param \DateTime $notOlderThan optional, timestamp of the oldest comments + * that may be returned + * @return IComment[] + * @since 9.0.0 + */ + public function getForObject( + $objectType, + $objectId, + $limit = 0, + $offset = 0, + \DateTime $notOlderThan = null + ) { + $comments = []; + + $qb = $this->dbConn->getQueryBuilder(); + $query = $qb->select('*') + ->from('comments') + ->where($qb->expr()->eq('object_type', $qb->createParameter('type'))) + ->andWhere($qb->expr()->eq('object_id', $qb->createParameter('id'))) + ->orderBy('creation_timestamp', 'DESC') + ->setParameter('type', $objectType) + ->setParameter('id', $objectId); + + if($limit > 0) { + $query->setMaxResults($limit); + } + if($offset > 0) { + $query->setFirstResult($offset); + } + if(!is_null($notOlderThan)) { + $query + ->andWhere($qb->expr()->gt('creation_timestamp', $qb->createParameter('notOlderThan'))) + ->setParameter('notOlderThan', $notOlderThan, 'datetime'); + } + + $resultStatement = $query->execute(); + while($data = $resultStatement->fetch()) { + $comment = new Comment($this->normalizeDatabaseData($data)); + $this->cache($comment); + $comments[] = $comment; + } + $resultStatement->closeCursor(); + + return $comments; + } + + /** + * @param $objectType string the object type, e.g. 'files' + * @param $objectId string the id of the object + * @return Int + * @since 9.0.0 + */ + public function getNumberOfCommentsForObject($objectType, $objectId) { + $qb = $this->dbConn->getQueryBuilder(); + $query = $qb->select($qb->createFunction('COUNT(`id`)')) + ->from('comments') + ->where($qb->expr()->eq('object_type', $qb->createParameter('type'))) + ->andWhere($qb->expr()->eq('object_id', $qb->createParameter('id'))) + ->setParameter('type', $objectType) + ->setParameter('id', $objectId); + + $resultStatement = $query->execute(); + $data = $resultStatement->fetch(\PDO::FETCH_NUM); + $resultStatement->closeCursor(); + return intval($data[0]); + } + + /** + * creates a new comment and returns it. At this point of time, it is not + * saved in the used data storage. Use save() after setting other fields + * of the comment (e.g. message or verb). + * + * @param string $actorType the actor type (e.g. 'user') + * @param string $actorId a user id + * @param string $objectType the object type the comment is attached to + * @param string $objectId the object id the comment is attached to + * @return IComment + * @throws \InvalidArgumentException + * @since 9.0.0 + */ + public function create($actorType, $actorId, $objectType, $objectId) { + if( + !is_string($actorType) || empty($actorType) + || !is_string($actorId) || empty($actorId) + || !is_string($objectType) || empty($objectType) + || !is_string($objectId) || empty($objectId) + ) { + // unsure whether it's a good place to enforce it here, since the + // comment instance can be manipulated anyway. + throw new \InvalidArgumentException('All arguments must be non-empty strings'); + } + $comment = new Comment(); + $comment + ->setActor($actorType, $actorId) + ->setObject($objectType, $objectId); + return $comment; + } + + /** + * permanently deletes the comment specified by the ID + * + * When the comment has child comments, their parent ID will be changed to + * the parent ID of the item that is to be deleted. + * + * @param string $id + * @return bool + * @throws \InvalidArgumentException + * @since 9.0.0 + */ + public function delete($id) { + if(!is_string($id)) { + throw new \InvalidArgumentException('Parameter must be string'); + } + + $qb = $this->dbConn->getQueryBuilder(); + $query = $qb->delete('comments') + ->where($qb->expr()->eq('id', $qb->createParameter('id'))) + ->setParameter('id', $id); + + try { + $affectedRows = $query->execute(); + $this->uncache($id); + } catch (DriverException $e) { + $this->logger->logException($e, ['app' => 'core_comments']); + return false; + } + return ($affectedRows > 0); + } + + /** + * saves the comment permanently and returns it + * + * if the supplied comment has an empty ID, a new entry comment will be + * saved and the instance updated with the new ID. + * + * Otherwise, an existing comment will be updated. + * + * Throws NotFoundException when a comment that is to be updated does not + * exist anymore at this point of time. + * + * @param IComment &$comment + * @return bool + * @throws NotFoundException + * @since 9.0.0 + */ + public function save(IComment &$comment) { + $comment = $this->prepareCommentForDatabaseWrite($comment); + if($comment->getId() === '') { + $result = $this->insert($comment); + } else { + $result = $this->update($comment); + } + + if($result && !empty($comment->getParentId())) { + $this->updateChildrenInformation( + $comment->getParentId(), + $comment->getCreationDateTime() + ); + $this->cache($comment); + } + + return $result; + } + + /** + * inserts the provided comment in the database + * + * @param IComment $comment + * @return bool + */ + protected function insert(IComment &$comment) { + $qb = $this->dbConn->getQueryBuilder(); + $affectedRows = $qb + ->insert('comments') + ->values([ + 'parent_id' => $qb->createNamedParameter($comment->getParentId()), + 'topmost_parent_id' => $qb->createNamedParameter($comment->getTopmostParentId()), + 'children_count' => $qb->createNamedParameter($comment->getChildrenCount()), + 'actor_type' => $qb->createNamedParameter($comment->getActorType()), + 'actor_id' => $qb->createNamedParameter($comment->getActorId()), + 'message' => $qb->createNamedParameter($comment->getMessage()), + 'verb' => $qb->createNamedParameter($comment->getVerb()), + 'creation_timestamp' => $qb->createNamedParameter($comment->getCreationDateTime(), 'datetime'), + 'latest_child_timestamp' => $qb->createNamedParameter($comment->getLatestChildDateTime(), 'datetime'), + 'object_type' => $qb->createNamedParameter($comment->getObjectType()), + 'object_id' => $qb->createNamedParameter($comment->getObjectId()), + ]) + ->execute(); + + if ($affectedRows > 0) { + $comment->setId(strval($this->dbConn->lastInsertId('*PREFIX*comments'))); + } + + return $affectedRows > 0; + } + + /** + * updates a Comment data row + * + * @param IComment $comment + * @return bool + * @throws NotFoundException + */ + protected function update(IComment $comment) { + $qb = $this->dbConn->getQueryBuilder(); + $affectedRows = $qb + ->update('comments') + ->set('parent_id', $qb->createNamedParameter($comment->getParentId())) + ->set('topmost_parent_id', $qb->createNamedParameter($comment->getTopmostParentId())) + ->set('children_count', $qb->createNamedParameter($comment->getChildrenCount())) + ->set('actor_type', $qb->createNamedParameter($comment->getActorType())) + ->set('actor_id', $qb->createNamedParameter($comment->getActorId())) + ->set('message', $qb->createNamedParameter($comment->getMessage())) + ->set('verb', $qb->createNamedParameter($comment->getVerb())) + ->set('creation_timestamp', $qb->createNamedParameter($comment->getCreationDateTime(), 'datetime')) + ->set('latest_child_timestamp', $qb->createNamedParameter($comment->getLatestChildDateTime(), 'datetime')) + ->set('object_type', $qb->createNamedParameter($comment->getObjectType())) + ->set('object_id', $qb->createNamedParameter($comment->getObjectId())) + ->where($qb->expr()->eq('id', $qb->createParameter('id'))) + ->setParameter('id', $comment->getId()) + ->execute(); + + if($affectedRows === 0) { + throw new NotFoundException('Comment to update does ceased to exist'); + } + + return $affectedRows > 0; + } + + /** + * removes references to specific actor (e.g. on user delete) of a comment. + * The comment itself must not get lost/deleted. + * + * @param string $actorType the actor type (e.g. 'user') + * @param string $actorId a user id + * @return boolean + * @since 9.0.0 + */ + public function deleteReferencesOfActor($actorType, $actorId) { + $this->checkRoleParameters('Actor', $actorType, $actorId); + + $qb = $this->dbConn->getQueryBuilder(); + $affectedRows = $qb + ->update('comments') + ->set('actor_type', $qb->createNamedParameter(ICommentsManager::DELETED_USER)) + ->set('actor_id', $qb->createNamedParameter(ICommentsManager::DELETED_USER)) + ->where($qb->expr()->eq('actor_type', $qb->createParameter('type'))) + ->andWhere($qb->expr()->eq('actor_id', $qb->createParameter('id'))) + ->setParameter('type', $actorType) + ->setParameter('id', $actorId) + ->execute(); + + $this->commentsCache = []; + + return is_int($affectedRows); + } + + /** + * deletes all comments made of a specific object (e.g. on file delete) + * + * @param string $objectType the object type (e.g. 'file') + * @param string $objectId e.g. the file id + * @return boolean + * @since 9.0.0 + */ + public function deleteCommentsAtObject($objectType, $objectId) { + $this->checkRoleParameters('Object', $objectType, $objectId); + + $qb = $this->dbConn->getQueryBuilder(); + $affectedRows = $qb + ->delete('comments') + ->where($qb->expr()->eq('object_type', $qb->createParameter('type'))) + ->andWhere($qb->expr()->eq('object_id', $qb->createParameter('id'))) + ->setParameter('type', $objectType) + ->setParameter('id', $objectId) + ->execute(); + + $this->commentsCache = []; + + return is_int($affectedRows); + } +} diff --git a/lib/private/comments/managerfactory.php b/lib/private/comments/managerfactory.php new file mode 100644 index 00000000000..71d73571b10 --- /dev/null +++ b/lib/private/comments/managerfactory.php @@ -0,0 +1,24 @@ +getDatabaseConnection(), + \oc::$server->getUserManager(), + \oc::$server->getLogger() + ); + } +} diff --git a/lib/private/server.php b/lib/private/server.php index 6692e6f6bbf..ecac18d6a9b 100644 --- a/lib/private/server.php +++ b/lib/private/server.php @@ -528,6 +528,13 @@ class Server extends SimpleContainer implements IServerContainer { }); return $manager; }); + $this->registerService('CommentsManager', function(Server $c) { + $config = $c->getConfig(); + $factoryClass = $config->getSystemValue('comments.managerFactory', '\OC\Comments\ManagerFactory'); + /** @var \OCP\Comments\ICommentsManagerFactory $factory */ + $factory = new $factoryClass(); + return $factory->getManager(); + }); $this->registerService('EventDispatcher', function() { return new EventDispatcher(); }); @@ -1121,6 +1128,10 @@ class Server extends SimpleContainer implements IServerContainer { return $this->query('NotificationManager'); } + public function getCommentsManager() { + return $this->query('CommentsManager'); + } + /** * @return \OC\IntegrityCheck\Checker */ diff --git a/lib/public/comments/icomment.php b/lib/public/comments/icomment.php index c8f407624a0..8cfc504fc3f 100644 --- a/lib/public/comments/icomment.php +++ b/lib/public/comments/icomment.php @@ -49,13 +49,30 @@ interface IComment { /** * sets the parent ID and returns itself - * * @param string $parentId * @return IComment * @since 9.0.0 */ public function setParentId($parentId); + /** + * returns the topmost parent ID of the comment + * + * @return string + * @since 9.0.0 + */ + public function getTopmostParentId(); + + + /** + * sets the topmost parent ID and returns itself + * + * @param string $id + * @return IComment + * @since 9.0.0 + */ + public function setTopmostParentId($id); + /** * returns the number of children * diff --git a/lib/public/comments/icommentsmanager.php b/lib/public/comments/icommentsmanager.php index ebf7a34bf27..f6883224d60 100644 --- a/lib/public/comments/icommentsmanager.php +++ b/lib/public/comments/icommentsmanager.php @@ -12,6 +12,17 @@ namespace OCP\Comments; */ interface ICommentsManager { + /** + * @const DELETED_USER type and id for a user that has been deleted + * @see deleteReferencesOfActor + * @since 9.0.0 + * + * To be used as replacement for user type actors in deleteReferencesOfActor(). + * + * User interfaces shall show "Deleted user" as display name, if needed. + */ + const DELETED_USER = 'deleted_user'; + /** * returns a comment instance * @@ -28,7 +39,7 @@ interface ICommentsManager { * @param string $id * @param int $limit max number of entries to return, 0 returns all * @param int $offset the start entry - * @return [] + * @return array * @since 9.0.0 * * The return array looks like this @@ -73,7 +84,6 @@ interface ICommentsManager { * @param \DateTime $notOlderThan optional, timestamp of the oldest comments * that may be returned * @return IComment[] - * @throws NotFoundException in case the requested type or id is not present * @since 9.0.0 */ public function getForObject( @@ -88,7 +98,6 @@ interface ICommentsManager { * @param $objectType string the object type, e.g. 'files' * @param $objectId string the id of the object * @return Int - * @throws NotFoundException in case the requested type or id is not present * @since 9.0.0 */ public function getNumberOfCommentsForObject($objectType, $objectId); @@ -130,17 +139,20 @@ interface ICommentsManager { * Throws NotFoundException when a comment that is to be updated does not * exist anymore at this point of time. * - * @param IComment + * @param IComment &$comment * @return bool * @throws NotFoundException * @since 9.0.0 */ - public function save(&$comment); + public function save(IComment &$comment); /** * removes references to specific actor (e.g. on user delete) of a comment. * The comment itself must not get lost/deleted. * + * A 'user' type actor (type and id) should get replaced by the + * value of the DELETED_USER constant of this interface. + * * @param string $actorType the actor type (e.g. 'user') * @param string $actorId a user id * @return boolean diff --git a/lib/public/comments/icommentsmanagerfactory.php b/lib/public/comments/icommentsmanagerfactory.php new file mode 100644 index 00000000000..7bfb79aae00 --- /dev/null +++ b/lib/public/comments/icommentsmanagerfactory.php @@ -0,0 +1,22 @@ + 'user', 'id' => 'alice']; + $creationDT = new \DateTime(); + $latestChildDT = new \DateTime('yesterday'); + $object = ['type' => 'file', 'id' => 'file64']; + + $comment + ->setId($id) + ->setParentId($parentId) + ->setChildrenCount($childrenCount) + ->setMessage($message) + ->setVerb($verb) + ->setActor($actor['type'], $actor['id']) + ->setCreationDateTime($creationDT) + ->setLatestChildDateTime($latestChildDT) + ->setObject($object['type'], $object['id']); + + $this->assertSame($id, $comment->getId()); + $this->assertSame($parentId, $comment->getParentId()); + $this->assertSame($childrenCount, $comment->getChildrenCount()); + $this->assertSame($message, $comment->getMessage()); + $this->assertSame($verb, $comment->getVerb()); + $this->assertSame($actor['type'], $comment->getActorType()); + $this->assertSame($actor['id'], $comment->getActorId()); + $this->assertSame($creationDT, $comment->getCreationDateTime()); + $this->assertSame($latestChildDT, $comment->getLatestChildDateTime()); + $this->assertSame($object['type'], $comment->getObjectType()); + $this->assertSame($object['id'], $comment->getObjectId()); + } + + public function testSetIdIllegalInput() { + $comment = new \OC\Comments\Comment(); + + $this->setExpectedException('\OCP\Comments\IllegalIDChangeException'); + $comment->setId('c23'); + $comment->setId('c17'); + } + + public function testResetId() { + $comment = new \OC\Comments\Comment(); + $comment->setId('c23'); + $comment->setId(''); + } + + public function simpleSetterProvider() { + return [ + ['Id'], + ['ParentId'], + ['Message'], + ['Verb'], + ['ChildrenCount'], + ]; + } + + /** + * @dataProvider simpleSetterProvider + */ + public function testSimpleSetterInvalidInput($field) { + $comment = new \OC\Comments\Comment(); + $setter = 'set' . $field; + + $this->setExpectedException('InvalidArgumentException'); + // we have no field that is supposed to accept a Bool + $comment->$setter(true); + } + + public function roleSetterProvider() { + return [ + ['Actor', true, true], + ['Actor', 'user', true], + ['Actor', true, 'alice'], + ['Object', true, true], + ['Object', 'file', true], + ['Object', true, 'file64'], + ]; + } + + /** + * @dataProvider roleSetterProvider + */ + public function testSetRoleInvalidInput($role, $type, $id){ + $comment = new \OC\Comments\Comment(); + $setter = 'set' . $role; + $this->setExpectedException('InvalidArgumentException'); + $comment->$setter($type, $id); + } + + + +} diff --git a/tests/lib/comments/fakefactory.php b/tests/lib/comments/fakefactory.php new file mode 100644 index 00000000000..202b02f6411 --- /dev/null +++ b/tests/lib/comments/fakefactory.php @@ -0,0 +1,22 @@ +markTestSkipped(); + } + + public function getManager() { + return $this->getMock('\OCP\Comments\ICommentsManager'); + } +} diff --git a/tests/lib/comments/manager.php b/tests/lib/comments/manager.php new file mode 100644 index 00000000000..610dfe51a47 --- /dev/null +++ b/tests/lib/comments/manager.php @@ -0,0 +1,560 @@ +getDatabaseConnection()->getDatabasePlatform()->getTruncateTableSQL('`*PREFIX*comments`'); + \oc::$server->getDatabaseConnection()->prepare($sql)->execute(); + } + + protected function addDatabaseEntry($parentId, $topmostParentId, $creationDT = null, $latestChildDT = null) { + if(is_null($creationDT)) { + $creationDT = new \DateTime(); + } + if(is_null($latestChildDT)) { + $latestChildDT = new \DateTime('yesterday'); + } + + $qb = \oc::$server->getDatabaseConnection()->getQueryBuilder(); + $qb + ->insert('comments') + ->values([ + 'parent_id' => $qb->createNamedParameter($parentId), + 'topmost_parent_id' => $qb->createNamedParameter($topmostParentId), + 'children_count' => $qb->createNamedParameter(2), + 'actor_type' => $qb->createNamedParameter('user'), + 'actor_id' => $qb->createNamedParameter('alice'), + 'message' => $qb->createNamedParameter('nice one'), + 'verb' => $qb->createNamedParameter('comment'), + 'creation_timestamp' => $qb->createNamedParameter($creationDT, 'datetime'), + 'latest_child_timestamp' => $qb->createNamedParameter($latestChildDT, 'datetime'), + 'object_type' => $qb->createNamedParameter('file'), + 'object_id' => $qb->createNamedParameter('file64'), + ]) + ->execute(); + + return \oc::$server->getDatabaseConnection()->lastInsertId('*PREFIX*comments'); + } + + protected function getManager() { + $factory = new \OC\Comments\ManagerFactory(); + return $factory->getManager(); + } + + public function testGetCommentNotFound() { + $manager = $this->getManager(); + $this->setExpectedException('\OCP\Comments\NotFoundException'); + $manager->get('22'); + } + + public function testGetCommentNotFoundInvalidInput() { + $manager = $this->getManager(); + $this->setExpectedException('\InvalidArgumentException'); + $manager->get('unexisting22'); + } + + public function testGetComment() { + $manager = $this->getManager(); + + $creationDT = new \DateTime(); + $latestChildDT = new \DateTime('yesterday'); + + $qb = \oc::$server->getDatabaseConnection()->getQueryBuilder(); + $qb + ->insert('comments') + ->values([ + 'parent_id' => $qb->createNamedParameter('2'), + 'topmost_parent_id' => $qb->createNamedParameter('1'), + 'children_count' => $qb->createNamedParameter(2), + 'actor_type' => $qb->createNamedParameter('user'), + 'actor_id' => $qb->createNamedParameter('alice'), + 'message' => $qb->createNamedParameter('nice one'), + 'verb' => $qb->createNamedParameter('comment'), + 'creation_timestamp' => $qb->createNamedParameter($creationDT, 'datetime'), + 'latest_child_timestamp' => $qb->createNamedParameter($latestChildDT, 'datetime'), + 'object_type' => $qb->createNamedParameter('file'), + 'object_id' => $qb->createNamedParameter('file64'), + ]) + ->execute(); + + $id = strval(\oc::$server->getDatabaseConnection()->lastInsertId('*PREFIX*comments')); + + $comment = $manager->get($id); + $this->assertTrue($comment instanceof \OCP\Comments\IComment); + $this->assertSame($comment->getId(), $id); + $this->assertSame($comment->getParentId(), '2'); + $this->assertSame($comment->getTopmostParentId(), '1'); + $this->assertSame($comment->getChildrenCount(), 2); + $this->assertSame($comment->getActorType(), 'user'); + $this->assertSame($comment->getActorId(), 'alice'); + $this->assertSame($comment->getMessage(), 'nice one'); + $this->assertSame($comment->getVerb(), 'comment'); + $this->assertSame($comment->getObjectType(), 'file'); + $this->assertSame($comment->getObjectId(), 'file64'); + $this->assertEquals($comment->getCreationDateTime(), $creationDT); + $this->assertEquals($comment->getLatestChildDateTime(), $latestChildDT); + } + + public function testGetTreeNotFound() { + $manager = $this->getManager(); + $this->setExpectedException('\OCP\Comments\NotFoundException'); + $manager->getTree('22'); + } + + public function testGetTreeNotFoundInvalidIpnut() { + $manager = $this->getManager(); + $this->setExpectedException('\InvalidArgumentException'); + $manager->getTree('unexisting22'); + } + + public function testGetTree() { + $headId = $this->addDatabaseEntry(0, 0); + + $this->addDatabaseEntry($headId, $headId, new \DateTime('-3 hours')); + $this->addDatabaseEntry($headId, $headId, new \DateTime('-2 hours')); + $id = $this->addDatabaseEntry($headId, $headId, new \DateTime('-1 hour')); + + $manager = $this->getManager(); + $tree = $manager->getTree($headId); + + // Verifying the root comment + $this->assertTrue(isset($tree['comment'])); + $this->assertTrue($tree['comment'] instanceof \OCP\Comments\IComment); + $this->assertSame($tree['comment']->getId(), strval($headId)); + $this->assertTrue(isset($tree['replies'])); + $this->assertSame(count($tree['replies']), 3); + + // one level deep + foreach($tree['replies'] as $reply) { + $this->assertTrue($reply['comment'] instanceof \OCP\Comments\IComment); + $this->assertSame($reply['comment']->getId(), strval($id)); + $this->assertSame(count($reply['replies']), 0); + $id--; + } + } + + public function testGetTreeNoReplies() { + $id = $this->addDatabaseEntry(0, 0); + + $manager = $this->getManager(); + $tree = $manager->getTree($id); + + // Verifying the root comment + $this->assertTrue(isset($tree['comment'])); + $this->assertTrue($tree['comment'] instanceof \OCP\Comments\IComment); + $this->assertSame($tree['comment']->getId(), strval($id)); + $this->assertTrue(isset($tree['replies'])); + $this->assertSame(count($tree['replies']), 0); + + // one level deep + foreach($tree['replies'] as $reply) { + throw new \Exception('This ain`t happen'); + } + } + + public function testGetTreeWithLimitAndOffset() { + $headId = $this->addDatabaseEntry(0, 0); + + $this->addDatabaseEntry($headId, $headId, new \DateTime('-3 hours')); + $this->addDatabaseEntry($headId, $headId, new \DateTime('-2 hours')); + $this->addDatabaseEntry($headId, $headId, new \DateTime('-1 hour')); + $idToVerify = $this->addDatabaseEntry($headId, $headId, new \DateTime()); + + $manager = $this->getManager(); + + for ($offset = 0; $offset < 3; $offset += 2) { + $tree = $manager->getTree(strval($headId), 2, $offset); + + // Verifying the root comment + $this->assertTrue(isset($tree['comment'])); + $this->assertTrue($tree['comment'] instanceof \OCP\Comments\IComment); + $this->assertSame($tree['comment']->getId(), strval($headId)); + $this->assertTrue(isset($tree['replies'])); + $this->assertSame(count($tree['replies']), 2); + + // one level deep + foreach ($tree['replies'] as $reply) { + $this->assertTrue($reply['comment'] instanceof \OCP\Comments\IComment); + $this->assertSame($reply['comment']->getId(), strval($idToVerify)); + $this->assertSame(count($reply['replies']), 0); + $idToVerify--; + } + } + } + + public function testGetForObject() { + $this->addDatabaseEntry(0, 0); + + $manager = $this->getManager(); + $comments = $manager->getForObject('file', 'file64'); + + $this->assertTrue(is_array($comments)); + $this->assertSame(count($comments), 1); + $this->assertTrue($comments[0] instanceof \OCP\Comments\IComment); + $this->assertSame($comments[0]->getMessage(), 'nice one'); + } + + public function testGetForObjectWithLimitAndOffset() { + $this->addDatabaseEntry(0, 0, new \DateTime('-6 hours')); + $this->addDatabaseEntry(0, 0, new \DateTime('-5 hours')); + $this->addDatabaseEntry(1, 1, new \DateTime('-4 hours')); + $this->addDatabaseEntry(0, 0, new \DateTime('-3 hours')); + $this->addDatabaseEntry(2, 2, new \DateTime('-2 hours')); + $this->addDatabaseEntry(2, 2, new \DateTime('-1 hours')); + $idToVerify = $this->addDatabaseEntry(3, 1, new \DateTime()); + + $manager = $this->getManager(); + $offset = 0; + do { + $comments = $manager->getForObject('file', 'file64', 3, $offset); + + $this->assertTrue(is_array($comments)); + foreach($comments as $comment) { + $this->assertTrue($comment instanceof \OCP\Comments\IComment); + $this->assertSame($comment->getMessage(), 'nice one'); + $this->assertSame($comment->getId(), strval($idToVerify)); + $idToVerify--; + } + $offset += 3; + } while(count($comments) > 0); + } + + public function testGetForObjectWithDateTimeConstraint() { + $this->addDatabaseEntry(0, 0, new \DateTime('-6 hours')); + $this->addDatabaseEntry(0, 0, new \DateTime('-5 hours')); + $id1 = $this->addDatabaseEntry(0, 0, new \DateTime('-3 hours')); + $id2 = $this->addDatabaseEntry(2, 2, new \DateTime('-2 hours')); + + $manager = $this->getManager(); + $comments = $manager->getForObject('file', 'file64', 0, 0, new \DateTime('-4 hours')); + + $this->assertSame(count($comments), 2); + $this->assertSame($comments[0]->getId(), strval($id2)); + $this->assertSame($comments[1]->getId(), strval($id1)); + } + + public function testGetForObjectWithLimitAndOffsetAndDateTimeConstraint() { + $this->addDatabaseEntry(0, 0, new \DateTime('-7 hours')); + $this->addDatabaseEntry(0, 0, new \DateTime('-6 hours')); + $this->addDatabaseEntry(1, 1, new \DateTime('-5 hours')); + $this->addDatabaseEntry(0, 0, new \DateTime('-3 hours')); + $this->addDatabaseEntry(2, 2, new \DateTime('-2 hours')); + $this->addDatabaseEntry(2, 2, new \DateTime('-1 hours')); + $idToVerify = $this->addDatabaseEntry(3, 1, new \DateTime()); + + $manager = $this->getManager(); + $offset = 0; + do { + $comments = $manager->getForObject('file', 'file64', 3, $offset, new \DateTime('-4 hours')); + + $this->assertTrue(is_array($comments)); + foreach($comments as $comment) { + $this->assertTrue($comment instanceof \OCP\Comments\IComment); + $this->assertSame($comment->getMessage(), 'nice one'); + $this->assertSame($comment->getId(), strval($idToVerify)); + $this->assertTrue(intval($comment->getId()) >= 4); + $idToVerify--; + } + $offset += 3; + } while(count($comments) > 0); + } + + public function testGetNumberOfCommentsForObject() { + for($i = 1; $i < 5; $i++) { + $this->addDatabaseEntry(0, 0); + } + + $manager = $this->getManager(); + + $amount = $manager->getNumberOfCommentsForObject('untype', '00'); + $this->assertSame($amount, 0); + + $amount = $manager->getNumberOfCommentsForObject('file', 'file64'); + $this->assertSame($amount, 4); + } + + public function invalidCreateArgsProvider() { + return [ + ['', 'aId-1', 'oType-1', 'oId-1'], + ['aType-1', '', 'oType-1', 'oId-1'], + ['aType-1', 'aId-1', '', 'oId-1'], + ['aType-1', 'aId-1', 'oType-1', ''], + [1, 'aId-1', 'oType-1', 'oId-1'], + ['aType-1', 1, 'oType-1', 'oId-1'], + ['aType-1', 'aId-1', 1, 'oId-1'], + ['aType-1', 'aId-1', 'oType-1', 1], + ]; + } + + /** + * @dataProvider invalidCreateArgsProvider + */ + public function testCreateCommentInvalidArguments($aType, $aId, $oType, $oId) { + $manager = $this->getManager(); + $this->setExpectedException('\InvalidArgumentException'); + $manager->create($aType, $aId, $oType, $oId); + } + + public function testCreateComment() { + $actorType = 'bot'; + $actorId = 'bob'; + $objectType = 'weather'; + $objectId = 'bielefeld'; + + $comment = $this->getManager()->create($actorType, $actorId, $objectType, $objectId); + $this->assertTrue($comment instanceof \OCP\Comments\IComment); + $this->assertSame($comment->getActorType(), $actorType); + $this->assertSame($comment->getActorId(), $actorId); + $this->assertSame($comment->getObjectType(), $objectType); + $this->assertSame($comment->getObjectId(), $objectId); + } + + public function testDelete() { + $manager = $this->getManager(); + + $done = $manager->delete('404'); + $this->assertFalse($done); + + $done = $manager->delete('%'); + $this->assertFalse($done); + + $done = $manager->delete(''); + $this->assertFalse($done); + + $id = strval($this->addDatabaseEntry(0, 0)); + $comment = $manager->get($id); + $this->assertTrue($comment instanceof \OCP\Comments\IComment); + $done = $manager->delete($id); + $this->assertTrue($done); + $this->setExpectedException('\OCP\Comments\NotFoundException'); + $manager->get($id); + } + + public function testSaveNew() { + $manager = $this->getManager(); + $comment = new \OC\Comments\Comment(); + $comment + ->setActor('user', 'alice') + ->setObject('file', 'file64') + ->setMessage('very beautiful, I am impressed!') + ->setVerb('comment'); + + $saveSuccessful = $manager->save($comment); + $this->assertTrue($saveSuccessful); + $this->assertTrue($comment->getId() !== ''); + $this->assertTrue($comment->getId() !== '0'); + $this->assertTrue(!is_null($comment->getCreationDateTime())); + + $loadedComment = $manager->get($comment->getId()); + $this->assertSame($comment->getMessage(), $loadedComment->getMessage()); + $this->assertEquals($comment->getCreationDateTime(), $loadedComment->getCreationDateTime()); + } + + public function testSaveUpdate() { + $manager = $this->getManager(); + $comment = new \OC\Comments\Comment(); + $comment + ->setActor('user', 'alice') + ->setObject('file', 'file64') + ->setMessage('very beautiful, I am impressed!') + ->setVerb('comment'); + + $manager->save($comment); + + $comment->setMessage('very beautiful, I am really so much impressed!'); + $manager->save($comment); + + $loadedComment = $manager->get($comment->getId()); + $this->assertSame($comment->getMessage(), $loadedComment->getMessage()); + } + + public function testSaveUpdateException() { + $manager = $this->getManager(); + $comment = new \OC\Comments\Comment(); + $comment + ->setActor('user', 'alice') + ->setObject('file', 'file64') + ->setMessage('very beautiful, I am impressed!') + ->setVerb('comment'); + + $manager->save($comment); + + $manager->delete($comment->getId()); + $comment->setMessage('very beautiful, I am really so much impressed!'); + $this->setExpectedException('\OCP\Comments\NotFoundException'); + $manager->save($comment); + } + + public function testSaveIncomplete() { + $manager = $this->getManager(); + $comment = new \OC\Comments\Comment(); + $comment->setMessage('from no one to nothing'); + $this->setExpectedException('\UnexpectedValueException'); + $manager->save($comment); + } + + public function testSaveAsChild() { + $id = $this->addDatabaseEntry(0, 0); + + $manager = $this->getManager(); + + for($i = 0; $i < 3; $i++) { + $comment = new \OC\Comments\Comment(); + $comment + ->setActor('user', 'alice') + ->setObject('file', 'file64') + ->setParentId(strval($id)) + ->setMessage('full ack') + ->setVerb('comment') + // setting the creation time avoids using sleep() while making sure to test with different timestamps + ->setCreationDateTime(new \DateTime('+' . $i . ' minutes')); + + $manager->save($comment); + + $this->assertSame($comment->getTopmostParentId(), strval($id)); + $parentComment = $manager->get(strval($id)); + $this->assertSame($parentComment->getChildrenCount(), $i + 1); + $this->assertEquals($parentComment->getLatestChildDateTime(), $comment->getCreationDateTime()); + } + } + + public function invalidActorArgsProvider() { + return + [ + ['', ''], + [1, 'alice'], + ['user', 1], + ]; + } + + /** + * @dataProvider invalidActorArgsProvider + */ + public function testDeleteReferencesOfActorInvalidInput($type, $id) { + $manager = $this->getManager(); + $this->setExpectedException('\InvalidArgumentException'); + $manager->deleteReferencesOfActor($type, $id); + } + + public function testDeleteReferencesOfActor() { + $ids = []; + $ids[] = $this->addDatabaseEntry(0, 0); + $ids[] = $this->addDatabaseEntry(0, 0); + $ids[] = $this->addDatabaseEntry(0, 0); + + $manager = $this->getManager(); + + // just to make sure they are really set, with correct actor data + $comment = $manager->get(strval($ids[1])); + $this->assertSame($comment->getActorType(), 'user'); + $this->assertSame($comment->getActorId(), 'alice'); + + $wasSuccessful = $manager->deleteReferencesOfActor('user', 'alice'); + $this->assertTrue($wasSuccessful); + + foreach($ids as $id) { + $comment = $manager->get(strval($id)); + $this->assertSame($comment->getActorType(), ICommentsManager::DELETED_USER); + $this->assertSame($comment->getActorId(), ICommentsManager::DELETED_USER); + } + + // actor info is gone from DB, but when database interaction is alright, + // we still expect to get true back + $wasSuccessful = $manager->deleteReferencesOfActor('user', 'alice'); + $this->assertTrue($wasSuccessful); + } + + public function testDeleteReferencesOfActorWithUserManagement() { + $user = \oc::$server->getUserManager()->createUser('xenia', '123456'); + $this->assertTrue($user instanceof \OCP\IUser); + + $manager = $this->getManager(); + $comment = $manager->create('user', $user->getUID(), 'file', 'file64'); + $comment + ->setMessage('Most important comment I ever left on the Internet.') + ->setVerb('comment'); + $status = $manager->save($comment); + $this->assertTrue($status); + + $commentID = $comment->getId(); + $user->delete(); + + $comment =$manager->get($commentID); + $this->assertSame($comment->getActorType(), \OCP\Comments\ICommentsManager::DELETED_USER); + $this->assertSame($comment->getActorId(), \OCP\Comments\ICommentsManager::DELETED_USER); + } + + public function invalidObjectArgsProvider() { + return + [ + ['', ''], + [1, 'file64'], + ['file', 1], + ]; + } + + /** + * @dataProvider invalidObjectArgsProvider + */ + public function testDeleteCommentsAtObjectInvalidInput($type, $id) { + $manager = $this->getManager(); + $this->setExpectedException('\InvalidArgumentException'); + $manager->deleteCommentsAtObject($type, $id); + } + + public function testDeleteCommentsAtObject() { + $ids = []; + $ids[] = $this->addDatabaseEntry(0, 0); + $ids[] = $this->addDatabaseEntry(0, 0); + $ids[] = $this->addDatabaseEntry(0, 0); + + $manager = $this->getManager(); + + // just to make sure they are really set, with correct actor data + $comment = $manager->get(strval($ids[1])); + $this->assertSame($comment->getObjectType(), 'file'); + $this->assertSame($comment->getObjectId(), 'file64'); + + $wasSuccessful = $manager->deleteCommentsAtObject('file', 'file64'); + $this->assertTrue($wasSuccessful); + + $verified = 0; + foreach($ids as $id) { + try { + $manager->get(strval($id)); + } catch (\OCP\Comments\NotFoundException $e) { + $verified++; + } + } + $this->assertSame($verified, 3); + + // actor info is gone from DB, but when database interaction is alright, + // we still expect to get true back + $wasSuccessful = $manager->deleteCommentsAtObject('file', 'file64'); + $this->assertTrue($wasSuccessful); + } + + public function testOverwriteDefaultManager() { + $config = \oc::$server->getConfig(); + $defaultManagerFactory = $config->getSystemValue('comments.managerFactory', '\OC\Comments\ManagerFactory'); + + $managerMock = $this->getMock('\OCP\Comments\ICommentsManager'); + + $config->setSystemValue('comments.managerFactory', 'Test_Comments_FakeFactory'); + $manager = \oc::$server->getCommentsManager(); + $this->assertEquals($managerMock, $manager); + + $config->setSystemValue('comments.managerFactory', $defaultManagerFactory); + } + +} diff --git a/tests/lib/server.php b/tests/lib/server.php index b72bef82036..0bee19822d2 100644 --- a/tests/lib/server.php +++ b/tests/lib/server.php @@ -61,6 +61,7 @@ class Server extends \Test\TestCase { ['CapabilitiesManager', '\OC\CapabilitiesManager'], ['ContactsManager', '\OC\ContactsManager'], ['ContactsManager', '\OCP\Contacts\IManager'], + ['CommentsManager', '\OCP\Comments\ICommentsManager'], ['Crypto', '\OC\Security\Crypto'], ['Crypto', '\OCP\Security\ICrypto'], ['CryptoWrapper', '\OC\Session\CryptoWrapper'], -- cgit v1.2.3 From e3dbc3d40ccd9874dadb32b59633cb159afddc48 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Thu, 3 Dec 2015 16:35:57 +0100 Subject: different strategy in cleaning up after user was deleted we do not listen to deletion hooks anymore, because there is no guarantee that they will be heard - requires that something fetches the CommentsManager first. Instead, in the user deletion routine the clean up method will be called directly. Same way as it happens for files, group memberships, config values. --- lib/private/comments/manager.php | 5 ----- lib/private/comments/managerfactory.php | 1 - lib/private/server.php | 3 +++ lib/private/user/user.php | 2 ++ tests/lib/comments/fakefactory.php | 19 +++++++++++++------ tests/lib/comments/manager.php | 19 +++---------------- 6 files changed, 21 insertions(+), 28 deletions(-) (limited to 'tests') diff --git a/lib/private/comments/manager.php b/lib/private/comments/manager.php index 78b2b71c8dd..bb0782c77fd 100644 --- a/lib/private/comments/manager.php +++ b/lib/private/comments/manager.php @@ -23,15 +23,10 @@ class Manager implements ICommentsManager { public function __construct( IDBConnection $dbConn, - Emitter $userManager, ILogger $logger ) { $this->dbConn = $dbConn; $this->logger = $logger; - $userManager->listen('\OC\User', 'postDelete', function($user) { - /** @var \OCP\IUser $user */ - $this->deleteReferencesOfActor('user', $user->getUid()); - }); } /** diff --git a/lib/private/comments/managerfactory.php b/lib/private/comments/managerfactory.php index 71d73571b10..0c9fce3e640 100644 --- a/lib/private/comments/managerfactory.php +++ b/lib/private/comments/managerfactory.php @@ -17,7 +17,6 @@ class ManagerFactory implements ICommentsManagerFactory { public function getManager() { return new Manager( \oc::$server->getDatabaseConnection(), - \oc::$server->getUserManager(), \oc::$server->getLogger() ); } diff --git a/lib/private/server.php b/lib/private/server.php index ecac18d6a9b..8439500706d 100644 --- a/lib/private/server.php +++ b/lib/private/server.php @@ -1128,6 +1128,9 @@ class Server extends SimpleContainer implements IServerContainer { return $this->query('NotificationManager'); } + /** + * @return \OCP\Comments\ICommentsManager + */ public function getCommentsManager() { return $this->query('CommentsManager'); } diff --git a/lib/private/user/user.php b/lib/private/user/user.php index d827097ee39..6c89dd06f77 100644 --- a/lib/private/user/user.php +++ b/lib/private/user/user.php @@ -189,6 +189,8 @@ class User implements IUser { // Delete the users entry in the storage table \OC\Files\Cache\Storage::remove('home::' . $this->uid); + + \OC::$server->getCommentsManager()->deleteReferencesOfActor('user', $this->uid); } if ($this->emitter) { diff --git a/tests/lib/comments/fakefactory.php b/tests/lib/comments/fakefactory.php index 202b02f6411..cd85a4f34ca 100644 --- a/tests/lib/comments/fakefactory.php +++ b/tests/lib/comments/fakefactory.php @@ -10,13 +10,20 @@ */ class Test_Comments_FakeFactory extends Test\TestCase implements \OCP\Comments\ICommentsManagerFactory { - public function testNothing() { - // If there would not be at least one test, phpunit would scream failure - // So we have one and skip it. - $this->markTestSkipped(); - } - public function getManager() { return $this->getMock('\OCP\Comments\ICommentsManager'); } + + public function testOverwriteDefaultManager() { + $config = \oc::$server->getConfig(); + $defaultManagerFactory = $config->getSystemValue('comments.managerFactory', '\OC\Comments\ManagerFactory'); + + $managerMock = $this->getMock('\OCP\Comments\ICommentsManager'); + + $config->setSystemValue('comments.managerFactory', 'Test_Comments_FakeFactory'); + $manager = \oc::$server->getCommentsManager(); + $this->assertEquals($managerMock, $manager); + + $config->setSystemValue('comments.managerFactory', $defaultManagerFactory); + } } diff --git a/tests/lib/comments/manager.php b/tests/lib/comments/manager.php index 610dfe51a47..35a1c8a2af5 100644 --- a/tests/lib/comments/manager.php +++ b/tests/lib/comments/manager.php @@ -475,10 +475,10 @@ class Test_Comments_Manager extends Test\TestCase } public function testDeleteReferencesOfActorWithUserManagement() { - $user = \oc::$server->getUserManager()->createUser('xenia', '123456'); + $user = \OC::$server->getUserManager()->createUser('xenia', '123456'); $this->assertTrue($user instanceof \OCP\IUser); - $manager = $this->getManager(); + $manager = \OC::$server->getCommentsManager(); $comment = $manager->create('user', $user->getUID(), 'file', 'file64'); $comment ->setMessage('Most important comment I ever left on the Internet.') @@ -489,7 +489,7 @@ class Test_Comments_Manager extends Test\TestCase $commentID = $comment->getId(); $user->delete(); - $comment =$manager->get($commentID); + $comment = $manager->get($commentID); $this->assertSame($comment->getActorType(), \OCP\Comments\ICommentsManager::DELETED_USER); $this->assertSame($comment->getActorId(), \OCP\Comments\ICommentsManager::DELETED_USER); } @@ -544,17 +544,4 @@ class Test_Comments_Manager extends Test\TestCase $this->assertTrue($wasSuccessful); } - public function testOverwriteDefaultManager() { - $config = \oc::$server->getConfig(); - $defaultManagerFactory = $config->getSystemValue('comments.managerFactory', '\OC\Comments\ManagerFactory'); - - $managerMock = $this->getMock('\OCP\Comments\ICommentsManager'); - - $config->setSystemValue('comments.managerFactory', 'Test_Comments_FakeFactory'); - $manager = \oc::$server->getCommentsManager(); - $this->assertEquals($managerMock, $manager); - - $config->setSystemValue('comments.managerFactory', $defaultManagerFactory); - } - } -- cgit v1.2.3 From 9a440c06b0da6d46417bda3fd6c84d42b69bb328 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Thu, 3 Dec 2015 17:19:40 +0100 Subject: OC not oc --- lib/private/comments/managerfactory.php | 4 ++-- tests/lib/comments/fakefactory.php | 4 ++-- tests/lib/comments/manager.php | 12 ++++++------ 3 files changed, 10 insertions(+), 10 deletions(-) (limited to 'tests') diff --git a/lib/private/comments/managerfactory.php b/lib/private/comments/managerfactory.php index 0c9fce3e640..41978d0cf4b 100644 --- a/lib/private/comments/managerfactory.php +++ b/lib/private/comments/managerfactory.php @@ -16,8 +16,8 @@ class ManagerFactory implements ICommentsManagerFactory { */ public function getManager() { return new Manager( - \oc::$server->getDatabaseConnection(), - \oc::$server->getLogger() + \OC::$server->getDatabaseConnection(), + \OC::$server->getLogger() ); } } diff --git a/tests/lib/comments/fakefactory.php b/tests/lib/comments/fakefactory.php index cd85a4f34ca..15b267b721e 100644 --- a/tests/lib/comments/fakefactory.php +++ b/tests/lib/comments/fakefactory.php @@ -15,13 +15,13 @@ class Test_Comments_FakeFactory extends Test\TestCase implements \OCP\Comments\I } public function testOverwriteDefaultManager() { - $config = \oc::$server->getConfig(); + $config = \OC::$server->getConfig(); $defaultManagerFactory = $config->getSystemValue('comments.managerFactory', '\OC\Comments\ManagerFactory'); $managerMock = $this->getMock('\OCP\Comments\ICommentsManager'); $config->setSystemValue('comments.managerFactory', 'Test_Comments_FakeFactory'); - $manager = \oc::$server->getCommentsManager(); + $manager = \OC::$server->getCommentsManager(); $this->assertEquals($managerMock, $manager); $config->setSystemValue('comments.managerFactory', $defaultManagerFactory); diff --git a/tests/lib/comments/manager.php b/tests/lib/comments/manager.php index 35a1c8a2af5..3543393308b 100644 --- a/tests/lib/comments/manager.php +++ b/tests/lib/comments/manager.php @@ -13,8 +13,8 @@ class Test_Comments_Manager extends Test\TestCase public function setUp() { parent::setUp(); - $sql = \oc::$server->getDatabaseConnection()->getDatabasePlatform()->getTruncateTableSQL('`*PREFIX*comments`'); - \oc::$server->getDatabaseConnection()->prepare($sql)->execute(); + $sql = \OC::$server->getDatabaseConnection()->getDatabasePlatform()->getTruncateTableSQL('`*PREFIX*comments`'); + \OC::$server->getDatabaseConnection()->prepare($sql)->execute(); } protected function addDatabaseEntry($parentId, $topmostParentId, $creationDT = null, $latestChildDT = null) { @@ -25,7 +25,7 @@ class Test_Comments_Manager extends Test\TestCase $latestChildDT = new \DateTime('yesterday'); } - $qb = \oc::$server->getDatabaseConnection()->getQueryBuilder(); + $qb = \OC::$server->getDatabaseConnection()->getQueryBuilder(); $qb ->insert('comments') ->values([ @@ -43,7 +43,7 @@ class Test_Comments_Manager extends Test\TestCase ]) ->execute(); - return \oc::$server->getDatabaseConnection()->lastInsertId('*PREFIX*comments'); + return \OC::$server->getDatabaseConnection()->lastInsertId('*PREFIX*comments'); } protected function getManager() { @@ -69,7 +69,7 @@ class Test_Comments_Manager extends Test\TestCase $creationDT = new \DateTime(); $latestChildDT = new \DateTime('yesterday'); - $qb = \oc::$server->getDatabaseConnection()->getQueryBuilder(); + $qb = \OC::$server->getDatabaseConnection()->getQueryBuilder(); $qb ->insert('comments') ->values([ @@ -87,7 +87,7 @@ class Test_Comments_Manager extends Test\TestCase ]) ->execute(); - $id = strval(\oc::$server->getDatabaseConnection()->lastInsertId('*PREFIX*comments')); + $id = strval(\OC::$server->getDatabaseConnection()->lastInsertId('*PREFIX*comments')); $comment = $manager->get($id); $this->assertTrue($comment instanceof \OCP\Comments\IComment); -- cgit v1.2.3 From d3692c32837a54198f8f8a00713d93dd64db8e61 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Thu, 3 Dec 2015 17:23:22 +0100 Subject: namespaces for tests --- tests/lib/comments/comment.php | 6 +++++- tests/lib/comments/fakefactory.php | 13 ++++++------- tests/lib/comments/manager.php | 5 ++++- 3 files changed, 15 insertions(+), 9 deletions(-) (limited to 'tests') diff --git a/tests/lib/comments/comment.php b/tests/lib/comments/comment.php index 790aca42967..f00dfd527f1 100644 --- a/tests/lib/comments/comment.php +++ b/tests/lib/comments/comment.php @@ -1,6 +1,10 @@ getMock('\OCP\Comments\ICommentsManager'); @@ -20,7 +19,7 @@ class Test_Comments_FakeFactory extends Test\TestCase implements \OCP\Comments\I $managerMock = $this->getMock('\OCP\Comments\ICommentsManager'); - $config->setSystemValue('comments.managerFactory', 'Test_Comments_FakeFactory'); + $config->setSystemValue('comments.managerFactory', '\Test\Comments\Test_Comments_FakeFactory'); $manager = \OC::$server->getCommentsManager(); $this->assertEquals($managerMock, $manager); diff --git a/tests/lib/comments/manager.php b/tests/lib/comments/manager.php index 3543393308b..d5c415c250a 100644 --- a/tests/lib/comments/manager.php +++ b/tests/lib/comments/manager.php @@ -1,13 +1,16 @@ Date: Thu, 3 Dec 2015 21:53:58 +0100 Subject: rework test about overwriting default comments manager --- tests/lib/comments/fakefactory.php | 21 +++------------------ tests/lib/comments/fakemanager.php | 33 +++++++++++++++++++++++++++++++++ tests/lib/server.php | 12 ++++++++++++ 3 files changed, 48 insertions(+), 18 deletions(-) create mode 100644 tests/lib/comments/fakemanager.php (limited to 'tests') diff --git a/tests/lib/comments/fakefactory.php b/tests/lib/comments/fakefactory.php index 0fa68e4cb0c..837bcb10585 100644 --- a/tests/lib/comments/fakefactory.php +++ b/tests/lib/comments/fakefactory.php @@ -2,27 +2,12 @@ namespace Test\Comments; -use Test\TestCase; - /** - * Class Test_Comments_FakeFactory + * Class FakeFactory */ -class Test_Comments_FakeFactory extends TestCase implements \OCP\Comments\ICommentsManagerFactory { +class FakeFactory implements \OCP\Comments\ICommentsManagerFactory { public function getManager() { - return $this->getMock('\OCP\Comments\ICommentsManager'); - } - - public function testOverwriteDefaultManager() { - $config = \OC::$server->getConfig(); - $defaultManagerFactory = $config->getSystemValue('comments.managerFactory', '\OC\Comments\ManagerFactory'); - - $managerMock = $this->getMock('\OCP\Comments\ICommentsManager'); - - $config->setSystemValue('comments.managerFactory', '\Test\Comments\Test_Comments_FakeFactory'); - $manager = \OC::$server->getCommentsManager(); - $this->assertEquals($managerMock, $manager); - - $config->setSystemValue('comments.managerFactory', $defaultManagerFactory); + return new FakeManager(); } } diff --git a/tests/lib/comments/fakemanager.php b/tests/lib/comments/fakemanager.php new file mode 100644 index 00000000000..a3cd9c0c064 --- /dev/null +++ b/tests/lib/comments/fakemanager.php @@ -0,0 +1,33 @@ +assertInstanceOf('\OC_EventSource', $this->server->createEventSource(), 'service returned by "createEventSource" did not return the right class'); $this->assertInstanceOf('\OCP\IEventSource', $this->server->createEventSource(), 'service returned by "createEventSource" did not return the right class'); } + + public function testOverwriteDefaultCommentsManager() { + $config = $this->server->getConfig(); + $defaultManagerFactory = $config->getSystemValue('comments.managerFactory', '\OC\Comments\ManagerFactory'); + + $config->setSystemValue('comments.managerFactory', '\Test\Comments\FakeFactory'); + + $manager = $this->server->getCommentsManager(); + $this->assertInstanceOf('\OCP\Comments\ICommentsManager', $manager); + + $config->setSystemValue('comments.managerFactory', $defaultManagerFactory); + } } -- cgit v1.2.3 From 0c1c0295717f0e75aa725d1c6699a68151f2c758 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Fri, 4 Dec 2015 11:13:39 +0100 Subject: hardening, add some checks for whitespace-only strings --- lib/private/comments/comment.php | 29 +++++++++++++++-------------- tests/lib/comments/comment.php | 18 ++++++++++-------- 2 files changed, 25 insertions(+), 22 deletions(-) (limited to 'tests') diff --git a/lib/private/comments/comment.php b/lib/private/comments/comment.php index 8efd7d5613a..15d721d099a 100644 --- a/lib/private/comments/comment.php +++ b/lib/private/comments/comment.php @@ -66,6 +66,7 @@ class Comment implements IComment { throw new \InvalidArgumentException('String expected.'); } + $id = trim($id); if($this->data['id'] === '' || ($this->data['id'] !== '' && $id === '')) { $this->data['id'] = $id; return $this; @@ -95,7 +96,7 @@ class Comment implements IComment { if(!is_string($parentId)) { throw new \InvalidArgumentException('String expected.'); } - $this->data['parentId'] = $parentId; + $this->data['parentId'] = trim($parentId); return $this; } @@ -121,7 +122,7 @@ class Comment implements IComment { if(!is_string($id)) { throw new \InvalidArgumentException('String expected.'); } - $this->data['topmostParentId'] = $id; + $this->data['topmostParentId'] = trim($id); return $this; } @@ -171,7 +172,7 @@ class Comment implements IComment { if(!is_string($message)) { throw new \InvalidArgumentException('String expected.'); } - $this->data['message'] = $message; + $this->data['message'] = trim($message); return $this; } @@ -193,10 +194,10 @@ class Comment implements IComment { * @since 9.0.0 */ public function setVerb($verb) { - if(!is_string($verb)) { - throw new \InvalidArgumentException('String expected.'); + if(!is_string($verb) || empty(trim($verb))) { + throw new \InvalidArgumentException('Non-empty String expected.'); } - $this->data['verb'] = $verb; + $this->data['verb'] = trim($verb); return $this; } @@ -230,13 +231,13 @@ class Comment implements IComment { */ public function setActor($actorType, $actorId) { if( - !is_string($actorType) || empty($actorType) - || !is_string($actorId) || empty($actorId) + !is_string($actorType) || empty(trim($actorType)) + || !is_string($actorId) || empty(trim($actorId)) ) { throw new \InvalidArgumentException('String expected.'); } - $this->data['actorType'] = $actorType; - $this->data['actorId'] = $actorId; + $this->data['actorType'] = trim($actorType); + $this->data['actorId'] = trim($actorId); return $this; } @@ -316,13 +317,13 @@ class Comment implements IComment { */ public function setObject($objectType, $objectId) { if( - !is_string($objectType) || empty($objectType) - || !is_string($objectId) || empty($objectId) + !is_string($objectType) || empty(trim($objectType)) + || !is_string($objectId) || empty(trim($objectId)) ) { throw new \InvalidArgumentException('String expected.'); } - $this->data['objectType'] = $objectType; - $this->data['objectId'] = $objectId; + $this->data['objectType'] = trim($objectType); + $this->data['objectId'] = trim($objectId); return $this; } diff --git a/tests/lib/comments/comment.php b/tests/lib/comments/comment.php index f00dfd527f1..c6a8f118dd1 100644 --- a/tests/lib/comments/comment.php +++ b/tests/lib/comments/comment.php @@ -60,24 +60,24 @@ class Test_Comments_Comment extends TestCase public function simpleSetterProvider() { return [ - ['Id'], - ['ParentId'], - ['Message'], - ['Verb'], - ['ChildrenCount'], + ['Id', true], + ['ParentId', true], + ['Message', true], + ['Verb', true], + ['Verb', ''], + ['ChildrenCount', true], ]; } /** * @dataProvider simpleSetterProvider */ - public function testSimpleSetterInvalidInput($field) { + public function testSimpleSetterInvalidInput($field, $input) { $comment = new \OC\Comments\Comment(); $setter = 'set' . $field; $this->setExpectedException('InvalidArgumentException'); - // we have no field that is supposed to accept a Bool - $comment->$setter(true); + $comment->$setter($input); } public function roleSetterProvider() { @@ -85,9 +85,11 @@ class Test_Comments_Comment extends TestCase ['Actor', true, true], ['Actor', 'user', true], ['Actor', true, 'alice'], + ['Actor', ' ', ' '], ['Object', true, true], ['Object', 'file', true], ['Object', true, 'file64'], + ['Object', ' ', ' '], ]; } -- cgit v1.2.3 From 8e298f51f84f0cca7f3ae51fa095bcabd06e8cdd Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Wed, 9 Dec 2015 14:38:01 +0100 Subject: adjust test's fakemanager to interface change --- tests/lib/comments/fakemanager.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tests') diff --git a/tests/lib/comments/fakemanager.php b/tests/lib/comments/fakemanager.php index a3cd9c0c064..e5cf58dda4f 100644 --- a/tests/lib/comments/fakemanager.php +++ b/tests/lib/comments/fakemanager.php @@ -25,7 +25,7 @@ class FakeManager implements \OCP\Comments\ICommentsManager { public function delete($id) {} - public function save(\OCP\Comments\IComment &$comment) {} + public function save(\OCP\Comments\IComment $comment) {} public function deleteReferencesOfActor($actorType, $actorId) {} -- cgit v1.2.3 From e191953942fe6b32917c5e01f27879db87e369ce Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Wed, 9 Dec 2015 14:13:05 +0100 Subject: Remove all locks after ttl from the db --- lib/private/lock/dblockingprovider.php | 6 +++--- tests/lib/lock/dblockingprovider.php | 8 +------- 2 files changed, 4 insertions(+), 10 deletions(-) (limited to 'tests') diff --git a/lib/private/lock/dblockingprovider.php b/lib/private/lock/dblockingprovider.php index 90657e6725f..8d6ab52737c 100644 --- a/lib/private/lock/dblockingprovider.php +++ b/lib/private/lock/dblockingprovider.php @@ -235,10 +235,10 @@ class DBLockingProvider extends AbstractLockingProvider { /** * cleanup empty locks */ - public function cleanEmptyLocks() { + public function cleanExpiredLocks() { $expire = $this->timeFactory->getTime(); $this->connection->executeUpdate( - 'DELETE FROM `*PREFIX*file_locks` WHERE `lock` = 0 AND `ttl` < ?', + 'DELETE FROM `*PREFIX*file_locks` WHERE `ttl` < ?', [$expire] ); } @@ -262,7 +262,7 @@ class DBLockingProvider extends AbstractLockingProvider { public function __destruct() { try { - $this->cleanEmptyLocks(); + $this->cleanExpiredLocks(); } catch (\Exception $e) { // If the table is missing, the clean up was successful if ($this->connection->tableExists('file_locks')) { diff --git a/tests/lib/lock/dblockingprovider.php b/tests/lib/lock/dblockingprovider.php index d679b1ea677..2032110f4f0 100644 --- a/tests/lib/lock/dblockingprovider.php +++ b/tests/lib/lock/dblockingprovider.php @@ -85,13 +85,7 @@ class DBLockingProvider extends LockingProvider { $this->assertEquals(3, $this->getLockEntryCount()); - $this->instance->cleanEmptyLocks(); - - $this->assertEquals(3, $this->getLockEntryCount()); - - $this->instance->releaseAll(); - - $this->instance->cleanEmptyLocks(); + $this->instance->cleanExpiredLocks(); $this->assertEquals(2, $this->getLockEntryCount()); } -- cgit v1.2.3 From fdd06ba1f81d07e4597beb0b5b17f44c296b5921 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Wed, 9 Dec 2015 16:33:34 +0100 Subject: use getLastInsertId from query builder for convenience --- lib/private/comments/manager.php | 2 +- tests/lib/comments/manager.php | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'tests') diff --git a/lib/private/comments/manager.php b/lib/private/comments/manager.php index 05f981f244d..09e59f28370 100644 --- a/lib/private/comments/manager.php +++ b/lib/private/comments/manager.php @@ -455,7 +455,7 @@ class Manager implements ICommentsManager { ->execute(); if ($affectedRows > 0) { - $comment->setId(strval($this->dbConn->lastInsertId('*PREFIX*comments'))); + $comment->setId(strval($qb->getLastInsertId())); } return $affectedRows > 0; diff --git a/tests/lib/comments/manager.php b/tests/lib/comments/manager.php index d5c415c250a..c8d65c951e3 100644 --- a/tests/lib/comments/manager.php +++ b/tests/lib/comments/manager.php @@ -46,7 +46,7 @@ class Test_Comments_Manager extends TestCase ]) ->execute(); - return \OC::$server->getDatabaseConnection()->lastInsertId('*PREFIX*comments'); + return $qb->getLastInsertId(); } protected function getManager() { @@ -90,7 +90,7 @@ class Test_Comments_Manager extends TestCase ]) ->execute(); - $id = strval(\OC::$server->getDatabaseConnection()->lastInsertId('*PREFIX*comments')); + $id = strval($qb->getLastInsertId()); $comment = $manager->get($id); $this->assertTrue($comment instanceof \OCP\Comments\IComment); -- cgit v1.2.3 From 81d763be4224adeb5676c620d76b3e4ec407187f Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Wed, 9 Dec 2015 16:55:07 +0100 Subject: use expectedException annotation, not via method call, for consistency --- tests/lib/comments/comment.php | 8 +++++--- tests/lib/comments/manager.php | 34 ++++++++++++++++++++++++---------- 2 files changed, 29 insertions(+), 13 deletions(-) (limited to 'tests') diff --git a/tests/lib/comments/comment.php b/tests/lib/comments/comment.php index c6a8f118dd1..ae7e913eaab 100644 --- a/tests/lib/comments/comment.php +++ b/tests/lib/comments/comment.php @@ -44,10 +44,12 @@ class Test_Comments_Comment extends TestCase $this->assertSame($object['id'], $comment->getObjectId()); } + /** + * @expectedException \OCP\Comments\IllegalIDChangeException + */ public function testSetIdIllegalInput() { $comment = new \OC\Comments\Comment(); - $this->setExpectedException('\OCP\Comments\IllegalIDChangeException'); $comment->setId('c23'); $comment->setId('c17'); } @@ -71,12 +73,12 @@ class Test_Comments_Comment extends TestCase /** * @dataProvider simpleSetterProvider + * @expectedException \InvalidArgumentException */ public function testSimpleSetterInvalidInput($field, $input) { $comment = new \OC\Comments\Comment(); $setter = 'set' . $field; - $this->setExpectedException('InvalidArgumentException'); $comment->$setter($input); } @@ -95,11 +97,11 @@ class Test_Comments_Comment extends TestCase /** * @dataProvider roleSetterProvider + * @expectedException \InvalidArgumentException */ public function testSetRoleInvalidInput($role, $type, $id){ $comment = new \OC\Comments\Comment(); $setter = 'set' . $role; - $this->setExpectedException('InvalidArgumentException'); $comment->$setter($type, $id); } diff --git a/tests/lib/comments/manager.php b/tests/lib/comments/manager.php index c8d65c951e3..248de683253 100644 --- a/tests/lib/comments/manager.php +++ b/tests/lib/comments/manager.php @@ -54,15 +54,19 @@ class Test_Comments_Manager extends TestCase return $factory->getManager(); } + /** + * @expectedException \OCP\Comments\NotFoundException + */ public function testGetCommentNotFound() { $manager = $this->getManager(); - $this->setExpectedException('\OCP\Comments\NotFoundException'); $manager->get('22'); } + /** + * @expectedException \InvalidArgumentException + */ public function testGetCommentNotFoundInvalidInput() { $manager = $this->getManager(); - $this->setExpectedException('\InvalidArgumentException'); $manager->get('unexisting22'); } @@ -108,15 +112,19 @@ class Test_Comments_Manager extends TestCase $this->assertEquals($comment->getLatestChildDateTime(), $latestChildDT); } + /** + * @expectedException \OCP\Comments\NotFoundException + */ public function testGetTreeNotFound() { $manager = $this->getManager(); - $this->setExpectedException('\OCP\Comments\NotFoundException'); $manager->getTree('22'); } + /** + * @expectedException \InvalidArgumentException + */ public function testGetTreeNotFoundInvalidIpnut() { $manager = $this->getManager(); - $this->setExpectedException('\InvalidArgumentException'); $manager->getTree('unexisting22'); } @@ -301,10 +309,10 @@ class Test_Comments_Manager extends TestCase /** * @dataProvider invalidCreateArgsProvider + * @expectedException \InvalidArgumentException */ public function testCreateCommentInvalidArguments($aType, $aId, $oType, $oId) { $manager = $this->getManager(); - $this->setExpectedException('\InvalidArgumentException'); $manager->create($aType, $aId, $oType, $oId); } @@ -322,6 +330,9 @@ class Test_Comments_Manager extends TestCase $this->assertSame($comment->getObjectId(), $objectId); } + /** + * @expectedException \OCP\Comments\NotFoundException + */ public function testDelete() { $manager = $this->getManager(); @@ -339,7 +350,6 @@ class Test_Comments_Manager extends TestCase $this->assertTrue($comment instanceof \OCP\Comments\IComment); $done = $manager->delete($id); $this->assertTrue($done); - $this->setExpectedException('\OCP\Comments\NotFoundException'); $manager->get($id); } @@ -381,6 +391,9 @@ class Test_Comments_Manager extends TestCase $this->assertSame($comment->getMessage(), $loadedComment->getMessage()); } + /** + * @expectedException \OCP\Comments\NotFoundException + */ public function testSaveUpdateException() { $manager = $this->getManager(); $comment = new \OC\Comments\Comment(); @@ -394,15 +407,16 @@ class Test_Comments_Manager extends TestCase $manager->delete($comment->getId()); $comment->setMessage('very beautiful, I am really so much impressed!'); - $this->setExpectedException('\OCP\Comments\NotFoundException'); $manager->save($comment); } + /** + * @expectedException \UnexpectedValueException + */ public function testSaveIncomplete() { $manager = $this->getManager(); $comment = new \OC\Comments\Comment(); $comment->setMessage('from no one to nothing'); - $this->setExpectedException('\UnexpectedValueException'); $manager->save($comment); } @@ -442,10 +456,10 @@ class Test_Comments_Manager extends TestCase /** * @dataProvider invalidActorArgsProvider + * @expectedException \InvalidArgumentException */ public function testDeleteReferencesOfActorInvalidInput($type, $id) { $manager = $this->getManager(); - $this->setExpectedException('\InvalidArgumentException'); $manager->deleteReferencesOfActor($type, $id); } @@ -508,10 +522,10 @@ class Test_Comments_Manager extends TestCase /** * @dataProvider invalidObjectArgsProvider + * @expectedException \InvalidArgumentException */ public function testDeleteCommentsAtObjectInvalidInput($type, $id) { $manager = $this->getManager(); - $this->setExpectedException('\InvalidArgumentException'); $manager->deleteCommentsAtObject($type, $id); } -- cgit v1.2.3 From 715b0941eeff2578c8d2bff89dbc901481cb4c7e Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Thu, 10 Dec 2015 09:22:41 +0100 Subject: Make compatible with PHPUnit 5.1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `addWarning` needs to be defined as well for it: ``` ➜ master git:(master) ✗ bash autotest.sh sqlite Using PHP executable /usr/local/opt/php56/bin/php Parsing all files in lib/public for the presence of @since or @deprecated on each method... Using database oc_autotest Setup environment for sqlite testing on local storage ... Installing .... ownCloud is not installed - only a limited number of commands are available Mac OS X is not supported and ownCloud will not work properly on this platform. Use it at your own risk! -> For the best results, please consider using a GNU/Linux server instead. creating sqlite db ownCloud was successfully installed Testing with sqlite ... No coverage /usr/local/bin/phpunit --configuration phpunit-autotest.xml --log-junit autotest-results-sqlite.xml PHP Notice: Constant PHPUNIT_RUN already defined in /Users/lukasreschke/Documents/Programming/master/apps/firstrunwizard/tests/bootstrap.php on line 6 PHP Stack trace: PHP 1. {main}() /usr/local/Cellar/phpunit/5.1.0/libexec/phpunit-5.1.0.phar:0 PHP 2. PHPUnit_TextUI_Command::main() /usr/local/Cellar/phpunit/5.1.0/libexec/phpunit-5.1.0.phar:514 PHP 3. PHPUnit_TextUI_Command->run() phar:///usr/local/Cellar/phpunit/5.1.0/libexec/phpunit-5.1.0.phar/phpunit/TextUI/Command.php:106 PHP 4. PHPUnit_TextUI_Command->handleArguments() phar:///usr/local/Cellar/phpunit/5.1.0/libexec/phpunit-5.1.0.phar/phpunit/TextUI/Command.php:117 PHP 5. PHPUnit_Util_Configuration->getTestSuiteConfiguration() phar:///usr/local/Cellar/phpunit/5.1.0/libexec/phpunit-5.1.0.phar/phpunit/TextUI/Command.php:663 PHP 6. PHPUnit_Util_Configuration->getTestSuite() phar:///usr/local/Cellar/phpunit/5.1.0/libexec/phpunit-5.1.0.phar/phpunit/Util/Configuration.php:796 PHP 7. PHPUnit_Framework_TestSuite->addTestFile() phar:///usr/local/Cellar/phpunit/5.1.0/libexec/phpunit-5.1.0.phar/phpunit/Util/Configuration.php:926 PHP 8. PHPUnit_Util_Fileloader::checkAndLoad() phar:///usr/local/Cellar/phpunit/5.1.0/libexec/phpunit-5.1.0.phar/phpunit/Framework/TestSuite.php:335 PHP 9. PHPUnit_Util_Fileloader::load() phar:///usr/local/Cellar/phpunit/5.1.0/libexec/phpunit-5.1.0.phar/phpunit/Util/Fileloader.php:38 PHP 10. include_once() phar:///usr/local/Cellar/phpunit/5.1.0/libexec/phpunit-5.1.0.phar/phpunit/Util/Fileloader.php:56 PHP 11. loadDirectory() /Users/lukasreschke/Documents/Programming/master/tests/apps.php:42 PHP 12. require_once() /Users/lukasreschke/Documents/Programming/master/tests/apps.php:20 PHP 13. define() /Users/lukasreschke/Documents/Programming/master/apps/firstrunwizard/tests/bootstrap.php:6 PHP Fatal error: Class StartSessionListener contains 1 abstract method and must therefore be declared abstract or implement the remaining methods (PHPUnit_Framework_TestListener::addWarning) in /Users/lukasreschke/Documents/Programming/master/tests/startsessionlistener.php on line 47 PHP Stack trace: PHP 1. {main}() /usr/local/Cellar/phpunit/5.1.0/libexec/phpunit-5.1.0.phar:0 PHP 2. PHPUnit_TextUI_Command::main() /usr/local/Cellar/phpunit/5.1.0/libexec/phpunit-5.1.0.phar:514 PHP 3. PHPUnit_TextUI_Command->run() phar:///usr/local/Cellar/phpunit/5.1.0/libexec/phpunit-5.1.0.phar/phpunit/TextUI/Command.php:106 PHP 4. PHPUnit_TextUI_TestRunner->doRun() phar:///usr/local/Cellar/phpunit/5.1.0/libexec/phpunit-5.1.0.phar/phpunit/TextUI/Command.php:155 PHP 5. PHPUnit_TextUI_TestRunner->handleConfiguration() phar:///usr/local/Cellar/phpunit/5.1.0/libexec/phpunit-5.1.0.phar/phpunit/TextUI/TestRunner.php:153 PHP 6. require_once() phar:///usr/local/Cellar/phpunit/5.1.0/libexec/phpunit-5.1.0.phar/phpunit/TextUI/TestRunner.php:805 ``` --- tests/startsessionlistener.php | 3 +++ 1 file changed, 3 insertions(+) (limited to 'tests') diff --git a/tests/startsessionlistener.php b/tests/startsessionlistener.php index 1f3573555ca..88544cc6ce9 100644 --- a/tests/startsessionlistener.php +++ b/tests/startsessionlistener.php @@ -44,4 +44,7 @@ class StartSessionListener implements PHPUnit_Framework_TestListener { public function endTestSuite(PHPUnit_Framework_TestSuite $suite) { } + public function addWarning(\PHPUnit_Framework_Test $test, \PHPUnit_Framework_Warning $e, $time) { + } + } -- cgit v1.2.3 From 29b6a03fc1b81a2ae8272e2b18d57335558955d0 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Thu, 10 Dec 2015 09:29:24 +0100 Subject: Add assertion to test This seems to be missing on that test. --- tests/lib/comments/comment.php | 2 ++ 1 file changed, 2 insertions(+) (limited to 'tests') diff --git a/tests/lib/comments/comment.php b/tests/lib/comments/comment.php index ae7e913eaab..02adea8729e 100644 --- a/tests/lib/comments/comment.php +++ b/tests/lib/comments/comment.php @@ -58,6 +58,8 @@ class Test_Comments_Comment extends TestCase $comment = new \OC\Comments\Comment(); $comment->setId('c23'); $comment->setId(''); + + $this->assertSame('', $comment->getId()); } public function simpleSetterProvider() { -- cgit v1.2.3 From ffc49a24f02639afce362384fc3d6e4a0799d1d7 Mon Sep 17 00:00:00 2001 From: Scrutinizer Auto-Fixer Date: Tue, 8 Dec 2015 15:01:20 +0000 Subject: Scrutinizer Auto-Fixes This commit consists of patches automatically generated for this project on https://scrutinizer-ci.com --- apps/dav/lib/systemtag/systemtagsbyidcollection.php | 12 ++++++++++++ apps/dav/lib/systemtag/systemtagsobjecttypecollection.php | 15 +++++++++++++++ apps/dav/tests/unit/systemtag/systemtagmappingnode.php | 4 ---- .../dav/tests/unit/systemtag/systemtagsbyidcollection.php | 1 - .../unit/systemtag/systemtagsobjectmappingcollection.php | 1 - apps/files/tests/backgroundjob/ScanFilesTest.php | 1 - apps/files_external/lib/personalmount.php | 2 +- apps/files_external/service/storagesservice.php | 3 +++ apps/files_external/service/userglobalstoragesservice.php | 3 +++ .../tests/service/globalstoragesservicetest.php | 1 - .../features/bootstrap/CapabilitiesContext.php | 2 -- .../middleware/security/securitymiddleware.php | 1 - lib/private/share/mailnotifications.php | 6 +++--- tests/lib/share/MailNotificationsTest.php | 4 +++- 14 files changed, 40 insertions(+), 16 deletions(-) (limited to 'tests') diff --git a/apps/dav/lib/systemtag/systemtagsbyidcollection.php b/apps/dav/lib/systemtag/systemtagsbyidcollection.php index e7b7b6d0acc..79b0aa8fbbd 100644 --- a/apps/dav/lib/systemtag/systemtagsbyidcollection.php +++ b/apps/dav/lib/systemtag/systemtagsbyidcollection.php @@ -46,14 +46,23 @@ class SystemTagsByIdCollection implements ICollection { $this->tagManager = $tagManager; } + /** + * @param string $name + */ function createFile($name, $data = null) { throw new Forbidden('Cannot create tags by id'); } + /** + * @param string $name + */ function createDirectory($name) { throw new Forbidden('Permission denied to create collections'); } + /** + * @param string $name + */ function getChild($name) { try { $tags = $this->tagManager->getTagsByIds([$name]); @@ -72,6 +81,9 @@ class SystemTagsByIdCollection implements ICollection { }, $tags); } + /** + * @param string $name + */ function childExists($name) { try { $this->tagManager->getTagsByIds([$name]); diff --git a/apps/dav/lib/systemtag/systemtagsobjecttypecollection.php b/apps/dav/lib/systemtag/systemtagsobjecttypecollection.php index e544073613f..543a684c5bb 100644 --- a/apps/dav/lib/systemtag/systemtagsobjecttypecollection.php +++ b/apps/dav/lib/systemtag/systemtagsobjecttypecollection.php @@ -61,14 +61,23 @@ class SystemTagsObjectTypeCollection implements ICollection { $this->objectType = $objectType; } + /** + * @param string $name + */ function createFile($name, $data = null) { throw new Forbidden('Permission denied to create nodes'); } + /** + * @param string $name + */ function createDirectory($name) { throw new Forbidden('Permission denied to create collections'); } + /** + * @param string $objectId + */ function getChild($objectId) { return new SystemTagsObjectMappingCollection( $objectId, @@ -83,6 +92,9 @@ class SystemTagsObjectTypeCollection implements ICollection { throw new MethodNotAllowed(); } + /** + * @param string $name + */ function childExists($name) { return true; } @@ -95,6 +107,9 @@ class SystemTagsObjectTypeCollection implements ICollection { return $this->objectType; } + /** + * @param string $name + */ function setName($name) { throw new Forbidden('Permission denied to rename this collection'); } diff --git a/apps/dav/tests/unit/systemtag/systemtagmappingnode.php b/apps/dav/tests/unit/systemtag/systemtagmappingnode.php index 849f7c2fa54..aef62a1acb2 100644 --- a/apps/dav/tests/unit/systemtag/systemtagmappingnode.php +++ b/apps/dav/tests/unit/systemtag/systemtagmappingnode.php @@ -9,12 +9,8 @@ namespace OCA\DAV\Tests\Unit\SystemTag; use Sabre\DAV\Exception\NotFound; -use Sabre\DAV\Exception\MethodNotAllowed; -use Sabre\DAV\Exception\Conflict; - use OC\SystemTag\SystemTag; use OCP\SystemTag\TagNotFoundException; -use OCP\SystemTag\TagAlreadyExistsException; class SystemTagMappingNode extends SystemTagNode { diff --git a/apps/dav/tests/unit/systemtag/systemtagsbyidcollection.php b/apps/dav/tests/unit/systemtag/systemtagsbyidcollection.php index 104ce366034..fa9e42d04f9 100644 --- a/apps/dav/tests/unit/systemtag/systemtagsbyidcollection.php +++ b/apps/dav/tests/unit/systemtag/systemtagsbyidcollection.php @@ -11,7 +11,6 @@ namespace OCA\DAV\Tests\Unit\SystemTag; use OC\SystemTag\SystemTag; use OCP\SystemTag\TagNotFoundException; -use OCP\SystemTag\TagAlreadyExistsException; class SystemTagsByIdCollection extends \Test\TestCase { diff --git a/apps/dav/tests/unit/systemtag/systemtagsobjectmappingcollection.php b/apps/dav/tests/unit/systemtag/systemtagsobjectmappingcollection.php index 6e15bb78e7c..a9b34f5ae19 100644 --- a/apps/dav/tests/unit/systemtag/systemtagsobjectmappingcollection.php +++ b/apps/dav/tests/unit/systemtag/systemtagsobjectmappingcollection.php @@ -11,7 +11,6 @@ namespace OCA\DAV\Tests\Unit\SystemTag; use OC\SystemTag\SystemTag; use OCP\SystemTag\TagNotFoundException; -use OCP\SystemTag\TagAlreadyExistsException; class SystemTagsObjectMappingCollection extends \Test\TestCase { diff --git a/apps/files/tests/backgroundjob/ScanFilesTest.php b/apps/files/tests/backgroundjob/ScanFilesTest.php index 907cad64f9e..087696f0cfc 100644 --- a/apps/files/tests/backgroundjob/ScanFilesTest.php +++ b/apps/files/tests/backgroundjob/ScanFilesTest.php @@ -24,7 +24,6 @@ use Test\TestCase; use OCP\IConfig; use OCP\IUserManager; use OCA\Files\BackgroundJob\ScanFiles; -use OCP\ILogger; /** * Class ScanFilesTest diff --git a/apps/files_external/lib/personalmount.php b/apps/files_external/lib/personalmount.php index f81dee5be1e..34ae516ea5e 100644 --- a/apps/files_external/lib/personalmount.php +++ b/apps/files_external/lib/personalmount.php @@ -40,7 +40,7 @@ class PersonalMount extends MountPoint implements MoveableMount { /** * @param UserStoragesService $storagesService * @param int $storageId - * @param string|\OC\Files\Storage\Storage $storage + * @param \OCP\Files\Storage $storage * @param string $mountpoint * @param array $arguments (optional) configuration for the storage backend * @param \OCP\Files\Storage\IStorageFactory $loader diff --git a/apps/files_external/service/storagesservice.php b/apps/files_external/service/storagesservice.php index 9be6498435c..97f79c13324 100644 --- a/apps/files_external/service/storagesservice.php +++ b/apps/files_external/service/storagesservice.php @@ -185,6 +185,9 @@ abstract class StoragesService { */ abstract public function getVisibilityType(); + /** + * @return integer + */ protected function getType() { return DBConfigService::MOUNT_TYPE_ADMIN; } diff --git a/apps/files_external/service/userglobalstoragesservice.php b/apps/files_external/service/userglobalstoragesservice.php index cb49f0f6726..e58815f8a79 100644 --- a/apps/files_external/service/userglobalstoragesservice.php +++ b/apps/files_external/service/userglobalstoragesservice.php @@ -89,6 +89,9 @@ class UserGlobalStoragesService extends GlobalStoragesService { throw new \DomainException('UserGlobalStoragesService writing disallowed'); } + /** + * @param integer $id + */ public function removeStorage($id) { throw new \DomainException('UserGlobalStoragesService writing disallowed'); } diff --git a/apps/files_external/tests/service/globalstoragesservicetest.php b/apps/files_external/tests/service/globalstoragesservicetest.php index 7c77616563c..e620b05a51e 100644 --- a/apps/files_external/tests/service/globalstoragesservicetest.php +++ b/apps/files_external/tests/service/globalstoragesservicetest.php @@ -23,7 +23,6 @@ namespace OCA\Files_external\Tests\Service; use \OC\Files\Filesystem; -use OCA\Files_External\Service\DBConfigService; use \OCA\Files_external\Service\GlobalStoragesService; use \OCA\Files_external\NotFoundException; use \OCA\Files_external\Lib\StorageConfig; diff --git a/build/integration/features/bootstrap/CapabilitiesContext.php b/build/integration/features/bootstrap/CapabilitiesContext.php index 4e200bdf421..1b0015dce73 100644 --- a/build/integration/features/bootstrap/CapabilitiesContext.php +++ b/build/integration/features/bootstrap/CapabilitiesContext.php @@ -2,8 +2,6 @@ use Behat\Behat\Context\Context; use Behat\Behat\Context\SnippetAcceptingContext; -use GuzzleHttp\Client; -use GuzzleHttp\Message\ResponseInterface; require __DIR__ . '/../../vendor/autoload.php'; diff --git a/lib/private/appframework/middleware/security/securitymiddleware.php b/lib/private/appframework/middleware/security/securitymiddleware.php index d0b7202a360..725ce689b48 100644 --- a/lib/private/appframework/middleware/security/securitymiddleware.php +++ b/lib/private/appframework/middleware/security/securitymiddleware.php @@ -27,7 +27,6 @@ namespace OC\AppFramework\Middleware\Security; -use OC\AppFramework\Http; use OC\Appframework\Middleware\Security\Exceptions\AppNotEnabledException; use OC\Appframework\Middleware\Security\Exceptions\CrossSiteRequestForgeryException; use OC\Appframework\Middleware\Security\Exceptions\NotAdminException; diff --git a/lib/private/share/mailnotifications.php b/lib/private/share/mailnotifications.php index 4d282158ba4..f071c7f3a3c 100644 --- a/lib/private/share/mailnotifications.php +++ b/lib/private/share/mailnotifications.php @@ -170,7 +170,7 @@ class MailNotifications { * @param string $filename the shared file * @param string $link the public link * @param int $expiration expiration date (timestamp) - * @return array $result of failed recipients + * @return string[] $result of failed recipients */ public function sendLinkShareMail($recipient, $filename, $link, $expiration) { $subject = (string)$this->l->t('%s shared »%s« with you', [$this->senderDisplayName, $filename]); @@ -232,8 +232,8 @@ class MailNotifications { } /** - * @param $itemSource - * @param $itemType + * @param string $itemSource + * @param string $itemType * @param IUser $recipient * @return array */ diff --git a/tests/lib/share/MailNotificationsTest.php b/tests/lib/share/MailNotificationsTest.php index 8684886e798..66bec8653fb 100644 --- a/tests/lib/share/MailNotificationsTest.php +++ b/tests/lib/share/MailNotificationsTest.php @@ -20,7 +20,6 @@ */ use OC\Share\MailNotifications; -use OCP\IConfig; use OCP\IL10N; use OCP\IUser; use OCP\Mail\IMailer; @@ -234,6 +233,9 @@ class MailNotificationsTest extends \Test\TestCase { } + /** + * @param string $subject + */ protected function setupMailerMock($subject, $to, $exceptionOnSend = true) { $message = $this->getMockBuilder('\OC\Mail\Message') ->disableOriginalConstructor()->getMock(); -- cgit v1.2.3 From f3360d51c6d069fc873a0b5563c01d37d58727c7 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Fri, 11 Dec 2015 06:17:47 +0100 Subject: Use PHP polyfills --- 3rdparty | 2 +- apps/encryption/lib/crypto/crypt.php | 31 +++-------- apps/encryption/vendor/pbkdf2fallback.php | 87 ------------------------------- apps/federation/api/ocsauthapi.php | 2 +- lib/private/appframework/http/request.php | 2 +- lib/private/log.php | 2 +- lib/private/security/crypto.php | 2 +- lib/private/security/hasher.php | 2 +- lib/private/security/securerandom.php | 45 +++++----------- lib/private/security/stringutils.php | 60 --------------------- lib/public/security/isecurerandom.php | 11 ++-- lib/public/security/stringutils.php | 3 +- tests/lib/security/securerandom.php | 7 ++- tests/lib/security/stringutils.php | 38 -------------- 14 files changed, 38 insertions(+), 256 deletions(-) delete mode 100644 apps/encryption/vendor/pbkdf2fallback.php delete mode 100644 lib/private/security/stringutils.php delete mode 100644 tests/lib/security/stringutils.php (limited to 'tests') diff --git a/3rdparty b/3rdparty index a7b34d6f831..56934b1fc0f 160000 --- a/3rdparty +++ b/3rdparty @@ -1 +1 @@ -Subproject commit a7b34d6f831c8fa363f389d27acd0150128fc0b9 +Subproject commit 56934b1fc0f15760690c7a28c6c6429ce6ce6bef diff --git a/apps/encryption/lib/crypto/crypt.php b/apps/encryption/lib/crypto/crypt.php index dbc0364a157..12e9008545a 100644 --- a/apps/encryption/lib/crypto/crypt.php +++ b/apps/encryption/lib/crypto/crypt.php @@ -30,7 +30,6 @@ use OC\Encryption\Exceptions\DecryptionFailedException; use OC\Encryption\Exceptions\EncryptionFailedException; use OCA\Encryption\Exceptions\MultiKeyDecryptException; use OCA\Encryption\Exceptions\MultiKeyEncryptException; -use OCA\Encryption\Vendor\PBKDF2Fallback; use OCP\Encryption\Exceptions\GenericEncryptionException; use OCP\IConfig; use OCP\ILogger; @@ -293,28 +292,14 @@ class Crypt { $salt = hash('sha256', $uid . $instanceId . $instanceSecret, true); $keySize = $this->getKeySize($cipher); - if (function_exists('hash_pbkdf2')) { - $hash = hash_pbkdf2( - 'sha256', - $password, - $salt, - 100000, - $keySize, - true - ); - } else { - // fallback to 3rdparty lib for PHP <= 5.4. - // FIXME: Can be removed as soon as support for PHP 5.4 was dropped - $fallback = new PBKDF2Fallback(); - $hash = $fallback->pbkdf2( - 'sha256', - $password, - $salt, - 100000, - $keySize, - true - ); - } + $hash = hash_pbkdf2( + 'sha256', + $password, + $salt, + 100000, + $keySize, + true + ); return $hash; } diff --git a/apps/encryption/vendor/pbkdf2fallback.php b/apps/encryption/vendor/pbkdf2fallback.php deleted file mode 100644 index ca579f8e7dc..00000000000 --- a/apps/encryption/vendor/pbkdf2fallback.php +++ /dev/null @@ -1,87 +0,0 @@ -dbHandler->getToken($url); - return StringUtils::equals($storedToken, $token); + return hash_equals($storedToken, $token); } } diff --git a/lib/private/appframework/http/request.php b/lib/private/appframework/http/request.php index 2bbb70db0f8..6ba1d8f644d 100644 --- a/lib/private/appframework/http/request.php +++ b/lib/private/appframework/http/request.php @@ -447,7 +447,7 @@ class Request implements \ArrayAccess, \Countable, IRequest { $deobfuscatedToken = base64_decode($obfuscatedToken) ^ $secret; // Check if the token is valid - if(\OCP\Security\StringUtils::equals($deobfuscatedToken, $this->items['requesttoken'])) { + if(hash_equals($deobfuscatedToken, $this->items['requesttoken'])) { return true; } else { return false; diff --git a/lib/private/log.php b/lib/private/log.php index ee5d61e98df..a722243dc69 100644 --- a/lib/private/log.php +++ b/lib/private/log.php @@ -227,7 +227,7 @@ class Log implements ILogger { $request = \OC::$server->getRequest(); // if token is found in the request change set the log condition to satisfied - if($request && StringUtils::equals($request->getParam('log_secret'), $logCondition['shared_secret'])) { + if($request && hash_equals($logCondition['shared_secret'], $request->getParam('log_secret'))) { $this->logConditionSatisfied = true; } } diff --git a/lib/private/security/crypto.php b/lib/private/security/crypto.php index 0bd34df3f36..46d0c750b2f 100644 --- a/lib/private/security/crypto.php +++ b/lib/private/security/crypto.php @@ -123,7 +123,7 @@ class Crypto implements ICrypto { $this->cipher->setIV($iv); - if(!\OCP\Security\StringUtils::equals($this->calculateHMAC($parts[0].$parts[1], $password), $hmac)) { + if(!hash_equals($this->calculateHMAC($parts[0].$parts[1], $password), $hmac)) { throw new \Exception('HMAC does not match.'); } diff --git a/lib/private/security/hasher.php b/lib/private/security/hasher.php index a5dd22e5dc8..318141b6852 100644 --- a/lib/private/security/hasher.php +++ b/lib/private/security/hasher.php @@ -109,7 +109,7 @@ class Hasher implements IHasher { // Verify whether it matches a legacy PHPass or SHA1 string $hashLength = strlen($hash); if($hashLength === 60 && password_verify($message.$this->legacySalt, $hash) || - $hashLength === 40 && StringUtils::equals($hash, sha1($message))) { + $hashLength === 40 && hash_equals($hash, sha1($message))) { $newHash = $this->hash($message); return true; } diff --git a/lib/private/security/securerandom.php b/lib/private/security/securerandom.php index 87dca68985e..24affbe8988 100644 --- a/lib/private/security/securerandom.php +++ b/lib/private/security/securerandom.php @@ -27,25 +27,15 @@ use Sabre\DAV\Exception; use OCP\Security\ISecureRandom; /** - * Class SecureRandom provides a layer around RandomLib to generate - * secure random strings. For PHP 7 the native CSPRNG is used. + * Class SecureRandom provides a wrapper around the random_int function to generate + * secure random strings. For PHP 7 the native CSPRNG is used, older versions do + * use a fallback. * * Usage: - * \OC::$server->getSecureRandom()->getMediumStrengthGenerator()->generate(10); - * + * \OC::$server->getSecureRandom()->generate(10); * @package OC\Security */ class SecureRandom implements ISecureRandom { - - /** @var \RandomLib\Factory */ - var $factory; - /** @var \RandomLib\Generator */ - var $generator; - - function __construct() { - $this->factory = new RandomLib\Factory; - } - /** * Convenience method to get a low strength random number generator. * @@ -53,10 +43,10 @@ class SecureRandom implements ISecureRandom { * in a non-cryptographical setting. They are not strong enough to be * used as keys or salts. They are however useful for one-time use tokens. * + * @deprecated 9.0.0 Use \OC\Security\SecureRandom::generate directly or random_bytes() / random_int() * @return $this */ public function getLowStrengthGenerator() { - $this->generator = $this->factory->getLowStrengthGenerator(); return $this; } @@ -67,10 +57,10 @@ class SecureRandom implements ISecureRandom { * They are strong enough to be used as keys and salts. However, they do * take some time and resources to generate, so they should not be over-used * + * @deprecated 9.0.0 Use \OC\Security\SecureRandom::generate directly or random_bytes() / random_int() * @return $this */ public function getMediumStrengthGenerator() { - $this->generator = $this->factory->getMediumStrengthGenerator(); return $this; } @@ -80,26 +70,17 @@ class SecureRandom implements ISecureRandom { * @param string $characters An optional list of characters to use if no character list is * specified all valid base64 characters are used. * @return string - * @throws \Exception If the generator is not initialized. */ public function generate($length, $characters = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/') { - if(is_null($this->generator)) { - throw new \Exception('Generator is not initialized.'); - } + $maxCharIndex = strlen($characters) - 1; + $randomString = ''; - if(function_exists('random_int')) { - $maxCharIndex = strlen($characters) - 1; - $randomString = ''; - - while($length > 0) { - $randomNumber = random_int(0, $maxCharIndex); - $randomString .= $characters[$randomNumber]; - $length--; - } - return $randomString; + while($length > 0) { + $randomNumber = random_int(0, $maxCharIndex); + $randomString .= $characters[$randomNumber]; + $length--; } - - return $this->generator->generateString($length, $characters); + return $randomString; } } diff --git a/lib/private/security/stringutils.php b/lib/private/security/stringutils.php deleted file mode 100644 index fa4342a2b45..00000000000 --- a/lib/private/security/stringutils.php +++ /dev/null @@ -1,60 +0,0 @@ - - * @author Morris Jobke - * - * @copyright Copyright (c) 2015, ownCloud, Inc. - * @license AGPL-3.0 - * - * This code is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License, version 3, - * as published by the Free Software Foundation. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License, version 3, - * along with this program. If not, see - * - */ - -namespace OC\Security; - -class StringUtils { - - /** - * Compares whether two strings are equal. To prevent guessing of the string - * length this is done by comparing two hashes against each other and afterwards - * a comparison of the real string to prevent against the unlikely chance of - * collisions. - * - * Be aware that this function may leak whether the string to compare have a different - * length. - * - * @param string $expected The expected value - * @param string $input The input to compare against - * @return bool True if the two strings are equal, otherwise false. - */ - public static function equals($expected, $input) { - - if(!is_string($expected) || !is_string($input)) { - return false; - } - - if(function_exists('hash_equals')) { - return hash_equals($expected, $input); - } - - $randomString = \OC::$server->getSecureRandom()->getLowStrengthGenerator()->generate(10); - - if(hash('sha512', $expected.$randomString) === hash('sha512', $input.$randomString)) { - if($expected === $input) { - return true; - } - } - - return false; - } -} \ No newline at end of file diff --git a/lib/public/security/isecurerandom.php b/lib/public/security/isecurerandom.php index 1b72e4f4377..8315d0f971a 100644 --- a/lib/public/security/isecurerandom.php +++ b/lib/public/security/isecurerandom.php @@ -23,12 +23,12 @@ namespace OCP\Security; /** - * Class SecureRandom provides a layer around RandomLib to generate - * secure random strings. For PHP 7 the native CSPRNG is used. + * Class SecureRandom provides a wrapper around the random_int function to generate + * secure random strings. For PHP 7 the native CSPRNG is used, older versions do + * use a fallback. * * Usage: - * $rng = new \OC\Security\SecureRandom(); - * $randomString = $rng->getMediumStrengthGenerator()->generateString(30); + * \OC::$server->getSecureRandom()->generate(10); * * @package OCP\Security * @since 8.0.0 @@ -52,6 +52,7 @@ interface ISecureRandom { * * @return $this * @since 8.0.0 + * @deprecated 9.0.0 Use \OC\Security\SecureRandom::generate directly or random_bytes() / random_int() */ public function getLowStrengthGenerator(); @@ -64,6 +65,7 @@ interface ISecureRandom { * * @return $this * @since 8.0.0 + * @deprecated 9.0.0 Use \OC\Security\SecureRandom::generate directly or random_bytes() / random_int() */ public function getMediumStrengthGenerator(); @@ -73,7 +75,6 @@ interface ISecureRandom { * @param string $characters An optional list of characters to use if no character list is * specified all valid base64 characters are used. * @return string - * @throws \Exception If the generator is not initialized. * @since 8.0.0 */ public function generate($length, diff --git a/lib/public/security/stringutils.php b/lib/public/security/stringutils.php index 4f41fcf8262..7cf12ea2702 100644 --- a/lib/public/security/stringutils.php +++ b/lib/public/security/stringutils.php @@ -39,8 +39,9 @@ class StringUtils { * @param string $input The input to compare against * @return bool True if the two strings are equal, otherwise false. * @since 8.0.0 + * @deprecated 9.0.0 Use hash_equals */ public static function equals($expected, $input) { - return \OC\Security\StringUtils::equals($expected, $input); + return hash_equals($expected, $input); } } diff --git a/tests/lib/security/securerandom.php b/tests/lib/security/securerandom.php index d9bbd0e71e5..af437640805 100644 --- a/tests/lib/security/securerandom.php +++ b/tests/lib/security/securerandom.php @@ -57,11 +57,10 @@ class SecureRandomTest extends \Test\TestCase { } /** - * @expectedException \Exception - * @expectedExceptionMessage Generator is not initialized + * @dataProvider stringGenerationProvider */ - function testUninitializedGenerate() { - $this->rng->generate(30); + function testUninitializedGenerate($length, $expectedLength) { + $this->assertEquals($expectedLength, strlen($this->rng->generate($length))); } /** diff --git a/tests/lib/security/stringutils.php b/tests/lib/security/stringutils.php deleted file mode 100644 index 060315debb4..00000000000 --- a/tests/lib/security/stringutils.php +++ /dev/null @@ -1,38 +0,0 @@ - - * This file is licensed under the Affero General Public License version 3 or - * later. - * See the COPYING-README file. - */ - -use \OC\Security\StringUtils; - -class StringUtilsTest extends \Test\TestCase { - - public function dataProvider() - { - return array( - array('Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt.', 'Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt.'), - array('', ''), - array('我看这本书。 我看這本書', '我看这本书。 我看這本書'), - array('GpKY9fSnWNJbES99zVGvA', 'GpKY9fSnWNJbES99zVGvA') - ); - } - - /** - * @dataProvider dataProvider - */ - function testWrongEquals($string) { - $this->assertFalse(StringUtils::equals($string, 'A Completely Wrong String')); - $this->assertFalse(StringUtils::equals($string, null)); - } - - /** - * @dataProvider dataProvider - */ - function testTrueEquals($string, $expected) { - $this->assertTrue(StringUtils::equals($string, $expected)); - } - -} -- cgit v1.2.3 From efc030aa25b047a7c9f720cf781f26cbe1d274e0 Mon Sep 17 00:00:00 2001 From: Björn Schießle Date: Wed, 9 Dec 2015 12:00:00 +0100 Subject: don't allow to create a federated share if source and target server are the same --- apps/files_sharing/ajax/external.php | 8 ++++++++ lib/private/share/helper.php | 34 +++++++++++++++++++++++++++++++++ lib/private/share/share.php | 13 +++++++++++-- tests/lib/share/helper.php | 37 ++++++++++++++++++++++++++++++++++++ 4 files changed, 90 insertions(+), 2 deletions(-) (limited to 'tests') diff --git a/apps/files_sharing/ajax/external.php b/apps/files_sharing/ajax/external.php index 0f8a3d56cf0..65861ac6a04 100644 --- a/apps/files_sharing/ajax/external.php +++ b/apps/files_sharing/ajax/external.php @@ -49,6 +49,14 @@ if(!\OCP\Util::isValidFileName($name)) { exit(); } +$currentUser = \OC::$server->getUserSession()->getUser()->getUID(); +$currentServer = \OC::$server->getURLGenerator()->getAbsoluteURL('/'); +if (\OC\Share\Helper::isSameUserOnSameServer($owner, $remote, $currentUser, $currentServer )) { + \OCP\JSON::error(array('data' => array('message' => $l->t('Not allowed to create a federated share with the same user server')))); + exit(); +} + + $externalManager = new \OCA\Files_Sharing\External\Manager( \OC::$server->getDatabaseConnection(), \OC\Files\Filesystem::getMountManager(), diff --git a/lib/private/share/helper.php b/lib/private/share/helper.php index 26bbca81317..0441647df83 100644 --- a/lib/private/share/helper.php +++ b/lib/private/share/helper.php @@ -289,4 +289,38 @@ class Helper extends \OC\Share\Constants { $hint = $l->t('Invalid Federated Cloud ID'); throw new HintException('Invalid Fededrated Cloud ID', $hint); } + + /** + * check if two federated cloud IDs refer to the same user + * + * @param string $user1 + * @param string $server1 + * @param string $user2 + * @param string $server2 + * @return bool true if both users and servers are the same + */ + public static function isSameUserOnSameServer($user1, $server1, $user2, $server2) { + $normalizedServer1 = strtolower(\OC\Share\Share::removeProtocolFromUrl($server1)); + $normalizedServer2 = strtolower(\OC\Share\Share::removeProtocolFromUrl($server2)); + + if (rtrim($normalizedServer1, '/') === rtrim($normalizedServer2, '/')) { + // FIXME this should be a method in the user management instead + \OCP\Util::emitHook( + '\OCA\Files_Sharing\API\Server2Server', + 'preLoginNameUsedAsUserName', + array('uid' => &$user1) + ); + \OCP\Util::emitHook( + '\OCA\Files_Sharing\API\Server2Server', + 'preLoginNameUsedAsUserName', + array('uid' => &$user2) + ); + + if ($user1 === $user2) { + return true; + } + } + + return false; + } } diff --git a/lib/private/share/share.php b/lib/private/share/share.php index 3edffba8a3f..fff437b3ff7 100644 --- a/lib/private/share/share.php +++ b/lib/private/share/share.php @@ -849,11 +849,20 @@ class Share extends Constants { throw new \Exception($message_t); } + // don't allow federated shares if source and target server are the same + list($user, $remote) = Helper::splitUserRemote($shareWith); + $currentServer = self::removeProtocolFromUrl(\OC::$server->getURLGenerator()->getAbsoluteURL('/')); + $currentUser = \OC::$server->getUserSession()->getUser()->getUID(); + if (Helper::isSameUserOnSameServer($user, $remote, $currentUser, $currentServer)) { + $message = 'Not allowed to create a federated share with the same user.'; + $message_t = $l->t('Not allowed to create a federated share with the same user'); + \OCP\Util::writeLog('OCP\Share', $message, \OCP\Util::DEBUG); + throw new \Exception($message_t); + } $token = \OC::$server->getSecureRandom()->getMediumStrengthGenerator()->generate(self::TOKEN_LENGTH, \OCP\Security\ISecureRandom::CHAR_LOWER . \OCP\Security\ISecureRandom::CHAR_UPPER . \OCP\Security\ISecureRandom::CHAR_DIGITS); - list($user, $remote) = Helper::splitUserRemote($shareWith); $shareWith = $user . '@' . $remote; $shareId = self::put($itemType, $itemSource, $shareType, $shareWith, $uidOwner, $permissions, null, $token, $itemSourceName); @@ -2510,7 +2519,7 @@ class Share extends Constants { * @param string $url * @return string */ - private static function removeProtocolFromUrl($url) { + public static function removeProtocolFromUrl($url) { if (strpos($url, 'https://') === 0) { return substr($url, strlen('https://')); } else if (strpos($url, 'http://') === 0) { diff --git a/tests/lib/share/helper.php b/tests/lib/share/helper.php index e37a3db8bf0..eaa29c8cb90 100644 --- a/tests/lib/share/helper.php +++ b/tests/lib/share/helper.php @@ -19,6 +19,10 @@ * License along with this library. If not, see . */ +/** + * @group DB + * Class Test_Share_Helper + */ class Test_Share_Helper extends \Test\TestCase { public function expireDateProvider() { @@ -121,4 +125,37 @@ class Test_Share_Helper extends \Test\TestCase { public function testSplitUserRemoteError($id) { \OC\Share\Helper::splitUserRemote($id); } + + /** + * @dataProvider dataTestCompareServerAddresses + * + * @param string $server1 + * @param string $server2 + * @param bool $expected + */ + public function testIsSameUserOnSameServer($user1, $server1, $user2, $server2, $expected) { + $this->assertSame($expected, + \OC\Share\Helper::isSameUserOnSameServer($user1, $server1, $user2, $server2) + ); + } + + public function dataTestCompareServerAddresses() { + return [ + ['user1', 'http://server1', 'user1', 'http://server1', true], + ['user1', 'https://server1', 'user1', 'http://server1', true], + ['user1', 'http://serVer1', 'user1', 'http://server1', true], + ['user1', 'http://server1/', 'user1', 'http://server1', true], + ['user1', 'server1', 'user1', 'http://server1', true], + ['user1', 'http://server1', 'user1', 'http://server2', false], + ['user1', 'https://server1', 'user1', 'http://server2', false], + ['user1', 'http://serVer1', 'user1', 'http://serer2', false], + ['user1', 'http://server1/', 'user1', 'http://server2', false], + ['user1', 'server1', 'user1', 'http://server2', false], + ['user1', 'http://server1', 'user2', 'http://server1', false], + ['user1', 'https://server1', 'user2', 'http://server1', false], + ['user1', 'http://serVer1', 'user2', 'http://server1', false], + ['user1', 'http://server1/', 'user2', 'http://server1', false], + ['user1', 'server1', 'user2', 'http://server1', false], + ]; + } } -- cgit v1.2.3 From 38a2467a4f6344affc17a4c21862b047bfdf51e1 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 15 Dec 2015 13:48:17 +0100 Subject: test for statcache after fopen --- tests/lib/files/storage/storage.php | 13 +++++++++++++ 1 file changed, 13 insertions(+) (limited to 'tests') diff --git a/tests/lib/files/storage/storage.php b/tests/lib/files/storage/storage.php index d381b4cdf40..95dd70bfdac 100644 --- a/tests/lib/files/storage/storage.php +++ b/tests/lib/files/storage/storage.php @@ -598,4 +598,17 @@ abstract class Storage extends \Test\TestCase { $this->instance->mkdir('source'); $this->assertTrue($this->instance->isSharable('source')); } + + public function testStatAfterWrite() { + $this->instance->file_put_contents('foo.txt', 'bar'); + $stat = $this->instance->stat('foo.txt'); + $this->assertEquals(3, $stat['size']); + + $fh = $this->instance->fopen('foo.txt', 'w'); + fwrite($fh, 'qwerty'); + fclose($fh); + + $stat = $this->instance->stat('foo.txt'); + $this->assertEquals(6, $stat['size']); + } } -- cgit v1.2.3 From d796c43841f5f22e39af76e2c11d60335e985738 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Wed, 16 Dec 2015 20:26:00 +0100 Subject: [Avatars] Add function to get the Node of the avatar Since we usually just get the avatar and stream the content to the users there is no need to first create an image in memory. --- lib/private/avatar.php | 69 +++++++++++++++++++++++++++++++++++------------- lib/public/iavatar.php | 11 ++++++++ tests/lib/avatartest.php | 15 ++++++++++- 3 files changed, 76 insertions(+), 19 deletions(-) (limited to 'tests') diff --git a/lib/private/avatar.php b/lib/private/avatar.php index 37a813f3ff8..c87facd25da 100644 --- a/lib/private/avatar.php +++ b/lib/private/avatar.php @@ -31,6 +31,7 @@ namespace OC; use OCP\Files\Folder; use OCP\Files\File; +use OCP\Files\NotFoundException; use OCP\IL10N; use OC_Image; @@ -62,28 +63,14 @@ class Avatar implements \OCP\IAvatar { * @return boolean|\OCP\IImage containing the avatar or false if there's no image */ public function get ($size = 64) { - if ($this->folder->nodeExists('avatar.jpg')) { - $ext = 'jpg'; - } elseif ($this->folder->nodeExists('avatar.png')) { - $ext = 'png'; - } else { + try { + $file = $this->getFile($size); + } catch (NotFoundException $e) { return false; } $avatar = new OC_Image(); - if ($this->folder->nodeExists('avatar.' . $size . '.' . $ext)) { - /** @var File $node */ - $node = $this->folder->get('avatar.' . $size . '.' . $ext); - $avatar->loadFromData($node->getContent()); - } else { - /** @var File $node */ - $node = $this->folder->get('avatar.' . $ext); - $avatar->loadFromData($node->getContent()); - if ($size > 0) { - $avatar->resize($size); - } - $this->folder->newFile('avatar.' . $size . '.' . $ext)->putContent($avatar->data()); - } + $avatar->loadFromData($file->getContent()); return $avatar; } @@ -144,4 +131,50 @@ class Avatar implements \OCP\IAvatar { $this->folder->get('avatar.png')->delete(); } catch (\OCP\Files\NotFoundException $e) {} } + + /** + * Get the File of an avatar of size $size. + * + * @param int $size + * @return File + * @throws NotFoundException + */ + public function getFile($size) { + $ext = $this->getExtention(); + + $path = 'avatar.' . $size . '.' . $ext; + + try { + $file = $this->folder->get($path); + } catch (NotFoundException $e) { + if ($size <= 0) { + throw new NotFoundException; + } + + $avatar = new OC_Image(); + /** @var File $file */ + $file = $this->folder->get('avatar.' . $ext); + $avatar->loadFromData($file->getContent()); + $avatar->resize($size); + $file = $this->folder->newFile($path); + $file->putContent($avatar->data()); + } + + return $file; + } + + /** + * Get the extention of the avatar. If there is no avatar throw Exception + * + * @return string + * @throws NotFoundException + */ + private function getExtention() { + if ($this->folder->nodeExists('avatar.jpg')) { + return 'jpg'; + } elseif ($this->folder->nodeExists('avatar.png')) { + return 'png'; + } + throw new NotFoundException; + } } diff --git a/lib/public/iavatar.php b/lib/public/iavatar.php index fc29212a599..3d92d00b83d 100644 --- a/lib/public/iavatar.php +++ b/lib/public/iavatar.php @@ -24,6 +24,8 @@ */ namespace OCP; +use OCP\Files\File; +use OCP\Files\NotFoundException; /** * This class provides avatar functionality @@ -64,4 +66,13 @@ interface IAvatar { * @since 6.0.0 */ public function remove(); + + /** + * Get the file of the avatar + * @param int $size + * @return File + * @throws NotFoundException + * @since 9.0.0 + */ + public function getFile($size); } diff --git a/tests/lib/avatartest.php b/tests/lib/avatartest.php index 49e8be98c83..3d77a282a7d 100644 --- a/tests/lib/avatartest.php +++ b/tests/lib/avatartest.php @@ -60,12 +60,25 @@ class AvatarTest extends \Test\TestCase { $file = $this->getMock('\OCP\Files\File'); $file->method('getContent')->willReturn($expected->data()); - $this->folder->method('get')->with('avatar.png')->willReturn($file); + + $this->folder->method('get') + ->will($this->returnCallback( + function($path) use ($file) { + if ($path === 'avatar.png') { + return $file; + } else { + throw new \OCP\Files\NotFoundException; + } + } + )); $newFile = $this->getMock('\OCP\Files\File'); $newFile->expects($this->once()) ->method('putContent') ->with($expected2->data()); + $newFile->expects($this->once()) + ->method('getContent') + ->willReturn($expected2->data()); $this->folder->expects($this->once()) ->method('newFile') ->with('avatar.32.png') -- cgit v1.2.3 From 3e80f142693fd0d78ff6f4fcd8fd36dd1651e209 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Wed, 16 Dec 2015 20:29:17 +0100 Subject: [Avatar] Make the avatar controller use the avatar node --- core/avatar/avatarcontroller.php | 24 ++++++++++------- tests/core/avatar/avatarcontrollertest.php | 42 ++++++++++++++++++------------ 2 files changed, 40 insertions(+), 26 deletions(-) (limited to 'tests') diff --git a/core/avatar/avatarcontroller.php b/core/avatar/avatarcontroller.php index 6c0321e6b5e..e8139aa50ae 100644 --- a/core/avatar/avatarcontroller.php +++ b/core/avatar/avatarcontroller.php @@ -30,6 +30,7 @@ use OCP\AppFramework\Controller; use OCP\AppFramework\Http; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\Http\DataDisplayResponse; +use OCP\Files\NotFoundException; use OCP\IAvatarManager; use OCP\ILogger; use OCP\IL10N; @@ -112,20 +113,23 @@ class AvatarController extends Controller { $size = 64; } - $avatar = $this->avatarManager->getAvatar($userId); - $image = $avatar->get($size); - - if ($image instanceof \OCP\IImage) { - $resp = new DataDisplayResponse($image->data(), + try { + $avatar = $this->avatarManager->getAvatar($userId)->getFile($size); + $resp = new DataDisplayResponse($avatar->getContent(), Http::STATUS_OK, - ['Content-Type' => $image->mimeType()]); - $resp->setETag(crc32($image->data())); - } else { + ['Content-Type' => $avatar->getMimeType()]); + $resp->setETag($avatar->getEtag()); + } catch (NotFoundException $e) { $user = $this->userManager->get($userId); - $userName = $user ? $user->getDisplayName() : ''; $resp = new DataResponse([ 'data' => [ - 'displayname' => $userName, + 'displayname' => $user->getDisplayName(), + ], + ]); + } catch (\Exception $e) { + $resp = new DataResponse([ + 'data' => [ + 'displayname' => '', ], ]); } diff --git a/tests/core/avatar/avatarcontrollertest.php b/tests/core/avatar/avatarcontrollertest.php index a113add72b9..7f69ba0aadb 100644 --- a/tests/core/avatar/avatarcontrollertest.php +++ b/tests/core/avatar/avatarcontrollertest.php @@ -26,8 +26,10 @@ use OCP\AppFramework\IAppContainer; use OCP\AppFramework\Http; use OCP\Files\Folder; use OCP\Files\File; +use OCP\Files\NotFoundException; use OCP\IUser; use OCP\IAvatar; +use Punic\Exception; use Test\Traits\UserTrait; /** @@ -56,6 +58,8 @@ class AvatarControllerTest extends \Test\TestCase { private $avatarMock; /** @var IUser */ private $userMock; + /** @var File */ + private $avatarFile; protected function setUp() { parent::setUp(); @@ -88,6 +92,10 @@ class AvatarControllerTest extends \Test\TestCase { ->willReturnMap([['userId', $this->userMock]]); $this->container['UserSession']->method('getUser')->willReturn($this->userMock); + $this->avatarFile = $this->getMock('OCP\Files\File'); + $this->avatarFile->method('getContnet')->willReturn('image data'); + $this->avatarFile->method('getMimeType')->willReturn('image type'); + $this->avatarFile->method('getEtag')->willReturn('my etag'); } public function tearDown() { @@ -100,6 +108,7 @@ class AvatarControllerTest extends \Test\TestCase { */ public function testGetAvatarNoAvatar() { $this->container['AvatarManager']->method('getAvatar')->willReturn($this->avatarMock); + $this->avatarMock->method('getFile')->will($this->throwException(new NotFoundException())); $response = $this->avatarController->getAvatar('userId', 32); //Comment out until JS is fixed @@ -112,12 +121,8 @@ class AvatarControllerTest extends \Test\TestCase { * Fetch the user's avatar */ public function testGetAvatar() { - $image = $this->getMock('OCP\IImage'); - $image->method('data')->willReturn('image data'); - $image->method('mimeType')->willReturn('image type'); - - $this->avatarMock->method('get')->willReturn($image); - $this->container['AvatarManager']->method('getAvatar')->willReturn($this->avatarMock); + $this->avatarMock->method('getFile')->willReturn($this->avatarFile); + $this->container['AvatarManager']->method('getAvatar')->with('userId')->willReturn($this->avatarMock); $response = $this->avatarController->getAvatar('userId', 32); @@ -125,17 +130,19 @@ class AvatarControllerTest extends \Test\TestCase { $this->assertArrayHasKey('Content-Type', $response->getHeaders()); $this->assertEquals('image type', $response->getHeaders()['Content-Type']); - $this->assertEquals(crc32('image data'), $response->getEtag()); + $this->assertEquals('my etag', $response->getEtag()); } /** * Fetch the avatar of a non-existing user */ public function testGetAvatarNoUser() { - $this->avatarMock->method('get')->willReturn(null); - $this->container['AvatarManager']->method('getAvatar')->willReturn($this->avatarMock); + $this->container['AvatarManager'] + ->method('getAvatar') + ->with('userDoesNotExist') + ->will($this->throwException(new \Exception('user does not exist'))); - $response = $this->avatarController->getAvatar('userDoesnotexist', 32); + $response = $this->avatarController->getAvatar('userDoesNotExist', 32); //Comment out until JS is fixed //$this->assertEquals(Http::STATUS_NOT_FOUND, $response->getStatus()); @@ -148,8 +155,9 @@ class AvatarControllerTest extends \Test\TestCase { */ public function testGetAvatarSize() { $this->avatarMock->expects($this->once()) - ->method('get') - ->with($this->equalTo(32)); + ->method('getFile') + ->with($this->equalTo(32)) + ->willReturn($this->avatarFile); $this->container['AvatarManager']->method('getAvatar')->willReturn($this->avatarMock); @@ -161,8 +169,9 @@ class AvatarControllerTest extends \Test\TestCase { */ public function testGetAvatarSizeMin() { $this->avatarMock->expects($this->once()) - ->method('get') - ->with($this->equalTo(64)); + ->method('getFile') + ->with($this->equalTo(64)) + ->willReturn($this->avatarFile); $this->container['AvatarManager']->method('getAvatar')->willReturn($this->avatarMock); @@ -174,8 +183,9 @@ class AvatarControllerTest extends \Test\TestCase { */ public function testGetAvatarSizeMax() { $this->avatarMock->expects($this->once()) - ->method('get') - ->with($this->equalTo(2048)); + ->method('getFile') + ->with($this->equalTo(2048)) + ->willReturn($this->avatarFile); $this->container['AvatarManager']->method('getAvatar')->willReturn($this->avatarMock); -- cgit v1.2.3 From 205c239eab1912d1a8650ae9ed926d28839ac1ec Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Thu, 17 Dec 2015 11:51:40 +0100 Subject: Remove deprecated OC_Helper::linkTo function --- lib/private/helper.php | 15 --------------- tests/lib/helper.php | 46 ---------------------------------------------- 2 files changed, 61 deletions(-) (limited to 'tests') diff --git a/lib/private/helper.php b/lib/private/helper.php index ed127a62f38..779a67a2340 100644 --- a/lib/private/helper.php +++ b/lib/private/helper.php @@ -66,21 +66,6 @@ class OC_Helper { return OC::$server->getURLGenerator()->linkToRoute($route, $parameters); } - /** - * Creates an url - * @param string $app app - * @param string $file file - * @param array $args array with param=>value, will be appended to the returned url - * The value of $args will be urlencoded - * @return string the url - * @deprecated Use \OC::$server->getURLGenerator()->linkTo($app, $file, $args) - * - * Returns a url to the given app and file. - */ - public static function linkTo( $app, $file, $args = array() ) { - return OC::$server->getURLGenerator()->linkTo($app, $file, $args); - } - /** * Creates an absolute url * @param string $app app diff --git a/tests/lib/helper.php b/tests/lib/helper.php index b7deb3fc13d..469ffecc773 100644 --- a/tests/lib/helper.php +++ b/tests/lib/helper.php @@ -294,52 +294,6 @@ class Test_Helper extends \Test\TestCase { ); } - /** - * @small - * test linkTo URL construction - * @dataProvider provideDocRootAppUrlParts - */ - public function testLinkToDocRoot($app, $file, $args, $expectedResult) { - \OC::$WEBROOT = ''; - $result = \OC_Helper::linkTo($app, $file, $args); - - $this->assertEquals($expectedResult, $result); - } - - /** - * @small - * test linkTo URL construction in sub directory - * @dataProvider provideSubDirAppUrlParts - */ - public function testLinkToSubDir($app, $file, $args, $expectedResult) { - \OC::$WEBROOT = '/owncloud'; - $result = \OC_Helper::linkTo($app, $file, $args); - - $this->assertEquals($expectedResult, $result); - } - - /** - * @return array - */ - public function provideDocRootAppUrlParts() { - return array( - array('files', 'ajax/list.php', array(), '/index.php/apps/files/ajax/list.php'), - array('files', 'ajax/list.php', array('trut' => 'trat', 'dut' => 'dat'), '/index.php/apps/files/ajax/list.php?trut=trat&dut=dat'), - array('', 'index.php', array('trut' => 'trat', 'dut' => 'dat'), '/index.php?trut=trat&dut=dat'), - ); - } - - /** - * @return array - */ - public function provideSubDirAppUrlParts() { - return array( - array('files', 'ajax/list.php', array(), '/owncloud/index.php/apps/files/ajax/list.php'), - array('files', 'ajax/list.php', array('trut' => 'trat', 'dut' => 'dat'), '/owncloud/index.php/apps/files/ajax/list.php?trut=trat&dut=dat'), - array('', 'index.php', array('trut' => 'trat', 'dut' => 'dat'), '/owncloud/index.php?trut=trat&dut=dat'), - ); - } - /** * @small * test linkToAbsolute URL construction -- cgit v1.2.3 From 7e44ea5da068bae204715e545b95c4852ad6283c Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Thu, 17 Dec 2015 13:03:37 +0100 Subject: Remove deprecated function OC_User::getManager Private deprecated function => removed Replaced all instances with suggested replacement --- apps/files/appinfo/register_command.php | 2 +- core/templates/internalmail.php | 2 +- core/templates/mail.php | 2 +- core/templates/untrustedDomain.php | 2 +- lib/base.php | 2 +- lib/private/user.php | 50 ++++++++++++++------------------- lib/private/util.php | 2 +- settings/templates/email.new_user.php | 2 +- settings/users.php | 2 +- tests/lib/group.php | 6 ++-- tests/lib/user.php | 2 +- 11 files changed, 33 insertions(+), 41 deletions(-) (limited to 'tests') diff --git a/apps/files/appinfo/register_command.php b/apps/files/appinfo/register_command.php index 3042c259872..7e541ca26f2 100644 --- a/apps/files/appinfo/register_command.php +++ b/apps/files/appinfo/register_command.php @@ -20,5 +20,5 @@ * */ -$application->add(new OCA\Files\Command\Scan(OC_User::getManager())); +$application->add(new OCA\Files\Command\Scan(\OC::$server->getUserManager())); $application->add(new OCA\Files\Command\DeleteOrphanedFiles(\OC::$server->getDatabaseConnection())); diff --git a/core/templates/internalmail.php b/core/templates/internalmail.php index 0e73a601857..c2d84184d38 100644 --- a/core/templates/internalmail.php +++ b/core/templates/internalmail.php @@ -4,7 +4,7 @@   -<?php p($theme->getName()); ?> +<?php p($theme->getName()); ?>   diff --git a/core/templates/mail.php b/core/templates/mail.php index 0e73a601857..c2d84184d38 100644 --- a/core/templates/mail.php +++ b/core/templates/mail.php @@ -4,7 +4,7 @@   -<?php p($theme->getName()); ?> +<?php p($theme->getName()); ?>   diff --git a/core/templates/untrustedDomain.php b/core/templates/untrustedDomain.php index 361495636cd..46bad216822 100644 --- a/core/templates/untrustedDomain.php +++ b/core/templates/untrustedDomain.php @@ -10,7 +10,7 @@ t('Depending on your configuration, as an administrator you might also be able to use the button below to trust this domain.')); ?>

- + t('Add "%s" as trusted domain', array($_['domain']))); ?>

diff --git a/lib/base.php b/lib/base.php index 038c259ecf6..8f1432d6fb9 100644 --- a/lib/base.php +++ b/lib/base.php @@ -875,7 +875,7 @@ class OC { // Handle redirect URL for logged in users if (isset($_REQUEST['redirect_url']) && OC_User::isLoggedIn()) { - $location = OC_Helper::makeURLAbsolute(urldecode($_REQUEST['redirect_url'])); + $location = \OC::$server->getURLGenerator()->getAbsoluteURL(urldecode($_REQUEST['redirect_url'])); // Deny the redirect if the URL contains a @ // This prevents unvalidated redirects like ?redirect_url=:user@domain.com diff --git a/lib/private/user.php b/lib/private/user.php index 74441d9175a..9094f388f9e 100644 --- a/lib/private/user.php +++ b/lib/private/user.php @@ -66,14 +66,6 @@ class OC_User { return OC::$server->getUserSession(); } - /** - * @return \OC\User\Manager - * @deprecated Use \OC::$server->getUserManager() - */ - public static function getManager() { - return OC::$server->getUserManager(); - } - private static $_backends = array(); private static $_usedBackends = array(); @@ -132,7 +124,7 @@ class OC_User { public static function useBackend($backend = 'database') { if ($backend instanceof OC_User_Interface) { self::$_usedBackends[get_class($backend)] = $backend; - self::getManager()->registerBackend($backend); + \OC::$server->getUserManager()->registerBackend($backend); } else { // You'll never know what happens if (null === $backend OR !is_string($backend)) { @@ -146,17 +138,17 @@ class OC_User { case 'sqlite': \OCP\Util::writeLog('core', 'Adding user backend ' . $backend . '.', \OCP\Util::DEBUG); self::$_usedBackends[$backend] = new OC_User_Database(); - self::getManager()->registerBackend(self::$_usedBackends[$backend]); + \OC::$server->getUserManager()->registerBackend(self::$_usedBackends[$backend]); break; case 'dummy': self::$_usedBackends[$backend] = new \Test\Util\User\Dummy(); - self::getManager()->registerBackend(self::$_usedBackends[$backend]); + \OC::$server->getUserManager()->registerBackend(self::$_usedBackends[$backend]); break; default: \OCP\Util::writeLog('core', 'Adding default user backend ' . $backend . '.', \OCP\Util::DEBUG); $className = 'OC_USER_' . strToUpper($backend); self::$_usedBackends[$backend] = new $className(); - self::getManager()->registerBackend(self::$_usedBackends[$backend]); + \OC::$server->getUserManager()->registerBackend(self::$_usedBackends[$backend]); break; } } @@ -168,7 +160,7 @@ class OC_User { */ public static function clearBackends() { self::$_usedBackends = array(); - self::getManager()->clearBackends(); + \OC::$server->getUserManager()->clearBackends(); } /** @@ -213,7 +205,7 @@ class OC_User { * @deprecated Use \OC::$server->getUserManager()->createUser($uid, $password) */ public static function createUser($uid, $password) { - return self::getManager()->createUser($uid, $password); + return \OC::$server->getUserManager()->createUser($uid, $password); } /** @@ -226,7 +218,7 @@ class OC_User { * @deprecated Use \OC::$server->getUserManager()->get() and then run delete() on the return */ public static function deleteUser($uid) { - $user = self::getManager()->get($uid); + $user = \OC::$server->getUserManager()->get($uid); if ($user) { return $user->delete(); } else { @@ -343,7 +335,7 @@ class OC_User { if (is_null($displayName)) { $displayName = $uid; } - $user = self::getManager()->get($uid); + $user = \OC::$server->getUserManager()->get($uid); if ($user) { return $user->setDisplayName($displayName); } else { @@ -452,7 +444,7 @@ class OC_User { */ public static function getDisplayName($uid = null) { if ($uid) { - $user = self::getManager()->get($uid); + $user = \OC::$server->getUserManager()->get($uid); if ($user) { return $user->getDisplayName(); } else { @@ -490,7 +482,7 @@ class OC_User { * Change the password of a user */ public static function setPassword($uid, $password, $recoveryPassword = null) { - $user = self::getManager()->get($uid); + $user = \OC::$server->getUserManager()->get($uid); if ($user) { return $user->setPassword($password, $recoveryPassword); } else { @@ -507,7 +499,7 @@ class OC_User { * Check whether a specified user can change his avatar */ public static function canUserChangeAvatar($uid) { - $user = self::getManager()->get($uid); + $user = \OC::$server->getUserManager()->get($uid); if ($user) { return $user->canChangeAvatar(); } else { @@ -524,7 +516,7 @@ class OC_User { * Check whether a specified user can change his password */ public static function canUserChangePassword($uid) { - $user = self::getManager()->get($uid); + $user = \OC::$server->getUserManager()->get($uid); if ($user) { return $user->canChangePassword(); } else { @@ -541,7 +533,7 @@ class OC_User { * Check whether a specified user can change his display name */ public static function canUserChangeDisplayName($uid) { - $user = self::getManager()->get($uid); + $user = \OC::$server->getUserManager()->get($uid); if ($user) { return $user->canChangeDisplayName(); } else { @@ -560,7 +552,7 @@ class OC_User { * returns the user id or false */ public static function checkPassword($uid, $password) { - $manager = self::getManager(); + $manager = \OC::$server->getUserManager(); $username = $manager->checkPassword($uid, $password); if ($username !== false) { return $username->getUID(); @@ -576,7 +568,7 @@ class OC_User { * @deprecated Use \OC::$server->getUserManager->getHome() */ public static function getHome($uid) { - $user = self::getManager()->get($uid); + $user = \OC::$server->getUserManager()->get($uid); if ($user) { return $user->getHome(); } else { @@ -595,7 +587,7 @@ class OC_User { * @param integer $offset */ public static function getUsers($search = '', $limit = null, $offset = null) { - $users = self::getManager()->search($search, $limit, $offset); + $users = \OC::$server->getUserManager()->search($search, $limit, $offset); $uids = array(); foreach ($users as $user) { $uids[] = $user->getUID(); @@ -616,7 +608,7 @@ class OC_User { */ public static function getDisplayNames($search = '', $limit = null, $offset = null) { $displayNames = array(); - $users = self::getManager()->searchDisplayName($search, $limit, $offset); + $users = \OC::$server->getUserManager()->searchDisplayName($search, $limit, $offset); foreach ($users as $user) { $displayNames[$user->getUID()] = $user->getDisplayName(); } @@ -630,7 +622,7 @@ class OC_User { * @return boolean */ public static function userExists($uid) { - return self::getManager()->userExists($uid); + return \OC::$server->getUserManager()->userExists($uid); } /** @@ -639,7 +631,7 @@ class OC_User { * @param string $uid the user to disable */ public static function disableUser($uid) { - $user = self::getManager()->get($uid); + $user = \OC::$server->getUserManager()->get($uid); if ($user) { $user->setEnabled(false); } @@ -651,7 +643,7 @@ class OC_User { * @param string $uid */ public static function enableUser($uid) { - $user = self::getManager()->get($uid); + $user = \OC::$server->getUserManager()->get($uid); if ($user) { $user->setEnabled(true); } @@ -664,7 +656,7 @@ class OC_User { * @return bool */ public static function isEnabled($uid) { - $user = self::getManager()->get($uid); + $user = \OC::$server->getUserManager()->get($uid); if ($user) { return $user->isEnabled(); } else { diff --git a/lib/private/util.php b/lib/private/util.php index 578001988d4..c63befb3f32 100644 --- a/lib/private/util.php +++ b/lib/private/util.php @@ -1250,7 +1250,7 @@ class OC_Util { fclose($fp); // accessing the file via http - $url = OC_Helper::makeURLAbsolute(OC::$WEBROOT . '/data' . $fileName); + $url = \OC::$server->getURLGenerator()->getAbsoluteURL(OC::$WEBROOT . '/data' . $fileName); try { $content = \OC::$server->getHTTPClientService()->newClient()->get($url)->getBody(); } catch (\Exception $e) { diff --git a/settings/templates/email.new_user.php b/settings/templates/email.new_user.php index 74149632cb8..8b13ac753eb 100644 --- a/settings/templates/email.new_user.php +++ b/settings/templates/email.new_user.php @@ -4,7 +4,7 @@   - <?php p($theme->getName()); ?> + <?php p($theme->getName()); ?>   diff --git a/settings/users.php b/settings/users.php index ccf1bcd9e1a..0e9823e73e3 100644 --- a/settings/users.php +++ b/settings/users.php @@ -37,7 +37,7 @@ OC_Util::checkSubAdminUser(); \OC::$server->getNavigationManager()->setActiveEntry('core_users'); -$userManager = \OC_User::getManager(); +$userManager = \OC::$server->getUserManager(); $groupManager = \OC_Group::getManager(); // Set the sort option: SORT_USERCOUNT or SORT_GROUPNAME diff --git a/tests/lib/group.php b/tests/lib/group.php index 066dddc738e..4bb888ed725 100644 --- a/tests/lib/group.php +++ b/tests/lib/group.php @@ -31,7 +31,7 @@ class Test_Group extends \Test\TestCase { public function testSingleBackend() { $userBackend = new \Test\Util\User\Dummy(); - \OC_User::getManager()->registerBackend($userBackend); + \OC::$server->getUserManager()->registerBackend($userBackend); OC_Group::useBackend(new OC_Group_Dummy()); $group1 = $this->getUniqueID(); @@ -113,7 +113,7 @@ class Test_Group extends \Test\TestCase { public function testUsersInGroup() { OC_Group::useBackend(new OC_Group_Dummy()); $userBackend = new \Test\Util\User\Dummy(); - \OC_User::getManager()->registerBackend($userBackend); + \OC::$server->getUserManager()->registerBackend($userBackend); $group1 = $this->getUniqueID(); $group2 = $this->getUniqueID(); @@ -142,7 +142,7 @@ class Test_Group extends \Test\TestCase { public function testMultiBackend() { $userBackend = new \Test\Util\User\Dummy(); - \OC_User::getManager()->registerBackend($userBackend); + \OC::$server->getUserManager()->registerBackend($userBackend); $backend1 = new OC_Group_Dummy(); $backend2 = new OC_Group_Dummy(); OC_Group::useBackend($backend1); diff --git a/tests/lib/user.php b/tests/lib/user.php index bc1ba063c8f..5e9a38a43c0 100644 --- a/tests/lib/user.php +++ b/tests/lib/user.php @@ -26,7 +26,7 @@ class User extends TestCase { parent::setUp(); $this->backend = $this->getMock('\Test\Util\User\Dummy'); - $manager = \OC_User::getManager(); + $manager = \OC::$server->getUserManager(); $manager->registerBackend($this->backend); } -- cgit v1.2.3 From 835911bce5cf8da9ce6d4021f0836b11d6b97d31 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Thu, 17 Dec 2015 15:10:11 +0100 Subject: Removed deprecated private OC_User::createUser All function calls are replaced with the recommended (which was already the body of the function). --- apps/encryption/tests/lib/MigrationTest.php | 6 +++--- apps/files/tests/service/tagservice.php | 2 +- .../files_sharing/tests/controller/sharecontroller.php | 2 +- apps/files_sharing/tests/testcase.php | 2 +- apps/files_trashbin/tests/trashbin.php | 2 +- lib/private/user.php | 18 ------------------ tests/lib/cache/file.php | 2 +- tests/lib/files/cache/cache.php | 2 +- tests/lib/files/cache/updaterlegacy.php | 2 +- tests/lib/files/filesystem.php | 8 ++++---- tests/lib/files/objectstore/swift.php | 2 +- tests/lib/files/storage/homestoragequota.php | 4 ++-- tests/lib/helperstorage.php | 2 +- tests/lib/security/certificatemanager.php | 2 +- tests/lib/share/share.php | 14 +++++++------- tests/lib/tags.php | 4 ++-- tests/lib/user.php | 17 +---------------- 17 files changed, 29 insertions(+), 62 deletions(-) (limited to 'tests') diff --git a/apps/encryption/tests/lib/MigrationTest.php b/apps/encryption/tests/lib/MigrationTest.php index fc3d014345b..ec1dccb87b8 100644 --- a/apps/encryption/tests/lib/MigrationTest.php +++ b/apps/encryption/tests/lib/MigrationTest.php @@ -43,9 +43,9 @@ class MigrationTest extends \Test\TestCase { public static function setUpBeforeClass() { parent::setUpBeforeClass(); - \OC_User::createUser(self::TEST_ENCRYPTION_MIGRATION_USER1, 'foo'); - \OC_User::createUser(self::TEST_ENCRYPTION_MIGRATION_USER2, 'foo'); - \OC_User::createUser(self::TEST_ENCRYPTION_MIGRATION_USER3, 'foo'); + \OC::$server->getUserManager()->createUser(self::TEST_ENCRYPTION_MIGRATION_USER1, 'foo'); + \OC::$server->getUserManager()->createUser(self::TEST_ENCRYPTION_MIGRATION_USER2, 'foo'); + \OC::$server->getUserManager()->createUser(self::TEST_ENCRYPTION_MIGRATION_USER3, 'foo'); } public static function tearDownAfterClass() { diff --git a/apps/files/tests/service/tagservice.php b/apps/files/tests/service/tagservice.php index 36da3edc61e..a34bd05afcc 100644 --- a/apps/files/tests/service/tagservice.php +++ b/apps/files/tests/service/tagservice.php @@ -54,7 +54,7 @@ class TagServiceTest extends \Test\TestCase { protected function setUp() { parent::setUp(); $this->user = $this->getUniqueId('user'); - \OC_User::createUser($this->user, 'test'); + \OC::$server->getUserManager()->createUser($this->user, 'test'); \OC_User::setUserId($this->user); \OC_Util::setupFS($this->user); /** diff --git a/apps/files_sharing/tests/controller/sharecontroller.php b/apps/files_sharing/tests/controller/sharecontroller.php index 168488f5613..d165151b87f 100644 --- a/apps/files_sharing/tests/controller/sharecontroller.php +++ b/apps/files_sharing/tests/controller/sharecontroller.php @@ -78,7 +78,7 @@ class ShareControllerTest extends \Test\TestCase { // Create a dummy user $this->user = \OC::$server->getSecureRandom()->getLowStrengthGenerator()->generate(12, ISecureRandom::CHAR_LOWER); - \OC_User::createUser($this->user, $this->user); + \OC::$server->getUserManager()->createUser($this->user, $this->user); \OC_Util::tearDownFS(); $this->loginAsUser($this->user); diff --git a/apps/files_sharing/tests/testcase.php b/apps/files_sharing/tests/testcase.php index a74ee83c25b..cc82fc3d949 100644 --- a/apps/files_sharing/tests/testcase.php +++ b/apps/files_sharing/tests/testcase.php @@ -149,7 +149,7 @@ abstract class TestCase extends \Test\TestCase { } if ($create) { - \OC_User::createUser($user, $password); + \OC::$server->getUserManager()->createUser($user, $password); \OC_Group::createGroup('group'); \OC_Group::addToGroup($user, 'group'); } diff --git a/apps/files_trashbin/tests/trashbin.php b/apps/files_trashbin/tests/trashbin.php index db7e7e6e840..e8d586816c3 100644 --- a/apps/files_trashbin/tests/trashbin.php +++ b/apps/files_trashbin/tests/trashbin.php @@ -636,7 +636,7 @@ class Test_Trashbin extends \Test\TestCase { public static function loginHelper($user, $create = false) { if ($create) { try { - \OC_User::createUser($user, $user); + \OC::$server->getUserManager()->createUser($user, $user); } catch(\Exception $e) { // catch username is already being used from previous aborted runs } diff --git a/lib/private/user.php b/lib/private/user.php index 9094f388f9e..3c011d5cc81 100644 --- a/lib/private/user.php +++ b/lib/private/user.php @@ -190,24 +190,6 @@ class OC_User { } } - /** - * Create a new user - * - * @param string $uid The username of the user to create - * @param string $password The password of the new user - * @throws Exception - * @return bool true/false - * - * Creates a new user. Basic checking of username is done in OC_User - * itself, not in its subclasses. - * - * Allowed characters in the username are: "a-z", "A-Z", "0-9" and "_.@-" - * @deprecated Use \OC::$server->getUserManager()->createUser($uid, $password) - */ - public static function createUser($uid, $password) { - return \OC::$server->getUserManager()->createUser($uid, $password); - } - /** * delete a user * diff --git a/tests/lib/cache/file.php b/tests/lib/cache/file.php index 7aa969df02c..0880e7e1282 100644 --- a/tests/lib/cache/file.php +++ b/tests/lib/cache/file.php @@ -71,7 +71,7 @@ class FileCache extends \Test_Cache { \OC_User::useBackend(new \Test\Util\User\Dummy()); //login - \OC_User::createUser('test', 'test'); + \OC::$server->getUserManager()->createUser('test', 'test'); $this->user = \OC_User::getUser(); \OC_User::setUserId('test'); diff --git a/tests/lib/files/cache/cache.php b/tests/lib/files/cache/cache.php index 503d25597cd..6ae095fa5c2 100644 --- a/tests/lib/files/cache/cache.php +++ b/tests/lib/files/cache/cache.php @@ -317,7 +317,7 @@ class Cache extends \Test\TestCase { function testSearchByTag() { $userId = $this->getUniqueId('user'); - \OC_User::createUser($userId, $userId); + \OC::$server->getUserManager()->createUser($userId, $userId); $this->loginAsUser($userId); $user = new \OC\User\User($userId, null); diff --git a/tests/lib/files/cache/updaterlegacy.php b/tests/lib/files/cache/updaterlegacy.php index 1946913bba4..ca59850eb0b 100644 --- a/tests/lib/files/cache/updaterlegacy.php +++ b/tests/lib/files/cache/updaterlegacy.php @@ -57,7 +57,7 @@ class UpdaterLegacy extends \Test\TestCase { self::$user = $this->getUniqueID(); } - \OC_User::createUser(self::$user, 'password'); + \OC::$server->getUserManager()->createUser(self::$user, 'password'); $this->loginAsUser(self::$user); Filesystem::init(self::$user, '/' . self::$user . '/files'); diff --git a/tests/lib/files/filesystem.php b/tests/lib/files/filesystem.php index c5ebbdd1a2d..1de8cba5446 100644 --- a/tests/lib/files/filesystem.php +++ b/tests/lib/files/filesystem.php @@ -325,7 +325,7 @@ class Filesystem extends \Test\TestCase { public function testHomeMount() { $userId = $this->getUniqueID('user_'); - \OC_User::createUser($userId, $userId); + \OC::$server->getUserManager()->createUser($userId, $userId); \OC\Files\Filesystem::initMountPoints($userId); @@ -360,7 +360,7 @@ class Filesystem extends \Test\TestCase { // this will trigger the insert $cache = $localStorage->getCache(); - \OC_User::createUser($userId, $userId); + \OC::$server->getUserManager()->createUser($userId, $userId); \OC\Files\Filesystem::initMountPoints($userId); $homeMount = \OC\Files\Filesystem::getStorage('/' . $userId . '/'); @@ -388,7 +388,7 @@ class Filesystem extends \Test\TestCase { // no cache path configured $config->setSystemValue('cache_path', ''); - \OC_User::createUser($userId, $userId); + \OC::$server->getUserManager()->createUser($userId, $userId); \OC\Files\Filesystem::initMountPoints($userId); $this->assertEquals( @@ -416,7 +416,7 @@ class Filesystem extends \Test\TestCase { $cachePath = \OC_Helper::tmpFolder() . '/extcache'; $config->setSystemValue('cache_path', $cachePath); - \OC_User::createUser($userId, $userId); + \OC::$server->getUserManager()->createUser($userId, $userId); \OC\Files\Filesystem::initMountPoints($userId); $this->assertEquals( diff --git a/tests/lib/files/objectstore/swift.php b/tests/lib/files/objectstore/swift.php index 63332af68da..906efb6390f 100644 --- a/tests/lib/files/objectstore/swift.php +++ b/tests/lib/files/objectstore/swift.php @@ -52,7 +52,7 @@ class Swift extends \Test\Files\Storage\Storage { $users = array('test'); foreach($users as $userName) { \OC_User::deleteUser($userName); - \OC_User::createUser($userName, $userName); + \OC::$server->getUserManager()->createUser($userName, $userName); } // main test user diff --git a/tests/lib/files/storage/homestoragequota.php b/tests/lib/files/storage/homestoragequota.php index 49e8f499efd..bee05438c80 100644 --- a/tests/lib/files/storage/homestoragequota.php +++ b/tests/lib/files/storage/homestoragequota.php @@ -32,7 +32,7 @@ class HomeStorageQuota extends \Test\TestCase { */ function testHomeStorageWrapperWithoutQuota() { $user1 = $this->getUniqueID(); - \OC_User::createUser($user1, 'test'); + \OC::$server->getUserManager()->createUser($user1, 'test'); \OC::$server->getConfig()->setUserValue($user1, 'files', 'quota', 'none'); \OC_User::setUserId($user1); @@ -54,7 +54,7 @@ class HomeStorageQuota extends \Test\TestCase { */ function testHomeStorageWrapperWithQuota() { $user1 = $this->getUniqueID(); - \OC_User::createUser($user1, 'test'); + \OC::$server->getUserManager()->createUser($user1, 'test'); \OC::$server->getConfig()->setUserValue($user1, 'files', 'quota', '1024'); \OC_User::setUserId($user1); diff --git a/tests/lib/helperstorage.php b/tests/lib/helperstorage.php index cf022109c27..1b2d1ec4fea 100644 --- a/tests/lib/helperstorage.php +++ b/tests/lib/helperstorage.php @@ -23,7 +23,7 @@ class Test_Helper_Storage extends \Test\TestCase { parent::setUp(); $this->user = $this->getUniqueID('user_'); - \OC_User::createUser($this->user, $this->user); + \OC::$server->getUserManager()->createUser($this->user, $this->user); $this->storage = \OC\Files\Filesystem::getStorage('/'); \OC\Files\Filesystem::tearDown(); diff --git a/tests/lib/security/certificatemanager.php b/tests/lib/security/certificatemanager.php index 43b2f1cf980..a4e6d8d68ac 100644 --- a/tests/lib/security/certificatemanager.php +++ b/tests/lib/security/certificatemanager.php @@ -24,7 +24,7 @@ class CertificateManagerTest extends \Test\TestCase { parent::setUp(); $this->username = $this->getUniqueID('', 20); - OC_User::createUser($this->username, $this->getUniqueID('', 20)); + \OC::$server->getUserManager()->createUser($this->username, $this->getUniqueID('', 20)); \OC_Util::tearDownFS(); \OC_User::setUserId(''); diff --git a/tests/lib/share/share.php b/tests/lib/share/share.php index fa19577cb72..273073239c9 100644 --- a/tests/lib/share/share.php +++ b/tests/lib/share/share.php @@ -54,13 +54,13 @@ class Test_Share extends \Test\TestCase { $this->user5 = $this->getUniqueID('user5_'); $this->user6 = $this->getUniqueID('user6_'); $this->groupAndUser = $this->getUniqueID('groupAndUser_'); - OC_User::createUser($this->user1, 'pass'); - OC_User::createUser($this->user2, 'pass'); - OC_User::createUser($this->user3, 'pass'); - OC_User::createUser($this->user4, 'pass'); - OC_User::createUser($this->user5, 'pass'); - OC_User::createUser($this->user6, 'pass'); // no group - OC_User::createUser($this->groupAndUser, 'pass'); + \OC::$server->getUserManager()->createUser($this->user1, 'pass'); + \OC::$server->getUserManager()->createUser($this->user2, 'pass'); + \OC::$server->getUserManager()->createUser($this->user3, 'pass'); + \OC::$server->getUserManager()->createUser($this->user4, 'pass'); + \OC::$server->getUserManager()->createUser($this->user5, 'pass'); + \OC::$server->getUserManager()->createUser($this->user6, 'pass'); // no group + \OC::$server->getUserManager()->createUser($this->groupAndUser, 'pass'); OC_User::setUserId($this->user1); OC_Group::clearBackends(); OC_Group::useBackend(new OC_Group_Dummy); diff --git a/tests/lib/tags.php b/tests/lib/tags.php index a8f59ff16e4..537c898da13 100644 --- a/tests/lib/tags.php +++ b/tests/lib/tags.php @@ -44,7 +44,7 @@ class Test_Tags extends \Test\TestCase { OC_User::clearBackends(); OC_User::useBackend('dummy'); $userId = $this->getUniqueID('user_'); - OC_User::createUser($userId, 'pass'); + \OC::$server->getUserManager()->createUser($userId, 'pass'); OC_User::setUserId($userId); $this->user = new OC\User\User($userId, null); $this->userSession = $this->getMock('\OCP\IUserSession'); @@ -290,7 +290,7 @@ class Test_Tags extends \Test\TestCase { $tagger->tagAs(1, $testTag); $otherUserId = $this->getUniqueID('user2_'); - OC_User::createUser($otherUserId, 'pass'); + \OC::$server->getUserManager()->createUser($otherUserId, 'pass'); OC_User::setUserId($otherUserId); $otherUserSession = $this->getMock('\OCP\IUserSession'); $otherUserSession diff --git a/tests/lib/user.php b/tests/lib/user.php index 5e9a38a43c0..26596e2fb54 100644 --- a/tests/lib/user.php +++ b/tests/lib/user.php @@ -56,25 +56,10 @@ class User extends TestCase { $fail = \OC_User::deleteUser('victim'); $this->assertFalse($fail); - $success = \OC_User::createUser('victim', 'password'); + $success = \OC::$server->getUserManager()->createUser('victim', 'password'); $success = \OC_User::deleteUser('victim'); $this->assertTrue($success); } - - public function testCreateUser(){ - $this->backend->expects($this->any()) - ->method('implementsActions') - ->will($this->returnCallback(function ($actions) { - if ($actions === \OC_USER_BACKEND_CREATE_USER) { - return true; - } else { - return false; - } - })); - - $user = \OC_User::createUser('newuser', 'newpassword'); - $this->assertEquals('newuser', $user->getUid()); - } } -- cgit v1.2.3 From 1f715289bf1cafb7e51dce7a4ab478dd3db5088e Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Thu, 17 Dec 2015 15:59:23 +0100 Subject: Removed deprecated function OC_User::deleteUser Replaced with proper OCP calls --- apps/encryption/tests/lib/MigrationTest.php | 9 +++++--- apps/files/tests/service/tagservice.php | 3 ++- .../tests/controller/sharecontroller.php | 3 ++- apps/files_sharing/tests/testcase.php | 9 +++++--- apps/files_trashbin/tests/storage.php | 3 ++- apps/files_trashbin/tests/trashbin.php | 3 ++- apps/files_versions/tests/versions.php | 6 ++++-- lib/private/user.php | 18 ---------------- tests/lib/files/cache/cache.php | 3 ++- tests/lib/files/cache/updaterlegacy.php | 5 ++++- tests/lib/files/filesystem.php | 12 +++++++---- tests/lib/files/objectstore/swift.php | 6 ++++-- tests/lib/files/storage/homestoragequota.php | 6 ++++-- tests/lib/helperstorage.php | 3 ++- tests/lib/security/certificatemanager.php | 3 ++- tests/lib/share/share.php | 24 ++++++++++++++-------- tests/lib/user.php | 10 --------- 17 files changed, 66 insertions(+), 60 deletions(-) (limited to 'tests') diff --git a/apps/encryption/tests/lib/MigrationTest.php b/apps/encryption/tests/lib/MigrationTest.php index ec1dccb87b8..6df72ac1bf6 100644 --- a/apps/encryption/tests/lib/MigrationTest.php +++ b/apps/encryption/tests/lib/MigrationTest.php @@ -49,9 +49,12 @@ class MigrationTest extends \Test\TestCase { } public static function tearDownAfterClass() { - \OC_User::deleteUser(self::TEST_ENCRYPTION_MIGRATION_USER1); - \OC_User::deleteUser(self::TEST_ENCRYPTION_MIGRATION_USER2); - \OC_User::deleteUser(self::TEST_ENCRYPTION_MIGRATION_USER3); + $user = \OC::$server->getUserManager()->get(self::TEST_ENCRYPTION_MIGRATION_USER1); + if ($user !== null) { $user->delete(); } + $user = \OC::$server->getUserManager()->get(self::TEST_ENCRYPTION_MIGRATION_USER2); + if ($user !== null) { $user->delete(); } + $user = \OC::$server->getUserManager()->get(self::TEST_ENCRYPTION_MIGRATION_USER3); + if ($user !== null) { $user->delete(); } parent::tearDownAfterClass(); } diff --git a/apps/files/tests/service/tagservice.php b/apps/files/tests/service/tagservice.php index a34bd05afcc..b93dedd0efd 100644 --- a/apps/files/tests/service/tagservice.php +++ b/apps/files/tests/service/tagservice.php @@ -82,7 +82,8 @@ class TagServiceTest extends \Test\TestCase { protected function tearDown() { \OC_User::setUserId(''); - \OC_User::deleteUser($this->user); + $user = \OC::$server->getUserManager()->get($this->user); + if ($user !== null) { $user->delete(); } } public function testUpdateFileTags() { diff --git a/apps/files_sharing/tests/controller/sharecontroller.php b/apps/files_sharing/tests/controller/sharecontroller.php index d165151b87f..398538f0943 100644 --- a/apps/files_sharing/tests/controller/sharecontroller.php +++ b/apps/files_sharing/tests/controller/sharecontroller.php @@ -98,7 +98,8 @@ class ShareControllerTest extends \Test\TestCase { \OC_Util::tearDownFS(); \OC_User::setUserId(''); Filesystem::tearDown(); - \OC_User::deleteUser($this->user); + $user = \OC::$server->getUserManager()->get($this->user); + if ($user !== null) { $user->delete(); } \OC_User::setIncognitoMode(false); \OC::$server->getSession()->set('public_link_authenticated', ''); diff --git a/apps/files_sharing/tests/testcase.php b/apps/files_sharing/tests/testcase.php index cc82fc3d949..c4037c7c42e 100644 --- a/apps/files_sharing/tests/testcase.php +++ b/apps/files_sharing/tests/testcase.php @@ -117,9 +117,12 @@ abstract class TestCase extends \Test\TestCase { public static function tearDownAfterClass() { // cleanup users - \OC_User::deleteUser(self::TEST_FILES_SHARING_API_USER1); - \OC_User::deleteUser(self::TEST_FILES_SHARING_API_USER2); - \OC_User::deleteUser(self::TEST_FILES_SHARING_API_USER3); + $user = \OC::$server->getUserManager()->get(self::TEST_FILES_SHARING_API_USER1); + if ($user !== null) { $user->delete(); } + $user = \OC::$server->getUserManager()->get(self::TEST_FILES_SHARING_API_USER2); + if ($user !== null) { $user->delete(); } + $user = \OC::$server->getUserManager()->get(self::TEST_FILES_SHARING_API_USER3); + if ($user !== null) { $user->delete(); } // delete group \OC_Group::deleteGroup(self::TEST_FILES_SHARING_API_GROUP1); diff --git a/apps/files_trashbin/tests/storage.php b/apps/files_trashbin/tests/storage.php index 387bb20c6d4..30735fe7bc3 100644 --- a/apps/files_trashbin/tests/storage.php +++ b/apps/files_trashbin/tests/storage.php @@ -75,7 +75,8 @@ class Storage extends \Test\TestCase { protected function tearDown() { \OC\Files\Filesystem::getLoader()->removeStorageWrapper('oc_trashbin'); $this->logout(); - \OC_User::deleteUser($this->user); + $user = \OC::$server->getUserManager()->get($this->user); + if ($user !== null) { $user->delete(); } \OC_Hook::clear(); parent::tearDown(); } diff --git a/apps/files_trashbin/tests/trashbin.php b/apps/files_trashbin/tests/trashbin.php index e8d586816c3..9c19b67a904 100644 --- a/apps/files_trashbin/tests/trashbin.php +++ b/apps/files_trashbin/tests/trashbin.php @@ -88,7 +88,8 @@ class Test_Trashbin extends \Test\TestCase { public static function tearDownAfterClass() { // cleanup test user - \OC_User::deleteUser(self::TEST_TRASHBIN_USER1); + $user = \OC::$server->getUserManager()->get(self::TEST_TRASHBIN_USER1); + if ($user !== null) { $user->delete(); } \OC::$server->getConfig()->setSystemValue('trashbin_retention_obligation', self::$rememberRetentionObligation); diff --git a/apps/files_versions/tests/versions.php b/apps/files_versions/tests/versions.php index ffc98c2e98c..ee4978ff784 100644 --- a/apps/files_versions/tests/versions.php +++ b/apps/files_versions/tests/versions.php @@ -61,8 +61,10 @@ class Test_Files_Versioning extends \Test\TestCase { public static function tearDownAfterClass() { // cleanup test user - \OC_User::deleteUser(self::TEST_VERSIONS_USER); - \OC_User::deleteUser(self::TEST_VERSIONS_USER2); + $user = \OC::$server->getUserManager()->get(self::TEST_VERSIONS_USER); + if ($user !== null) { $user->delete(); } + $user = \OC::$server->getUserManager()->get(self::TEST_VERSIONS_USER2); + if ($user !== null) { $user->delete(); } parent::tearDownAfterClass(); } diff --git a/lib/private/user.php b/lib/private/user.php index e84da5cf843..f806aa07251 100644 --- a/lib/private/user.php +++ b/lib/private/user.php @@ -152,24 +152,6 @@ class OC_User { } } - /** - * delete a user - * - * @param string $uid The username of the user to delete - * @return bool - * - * Deletes a user - * @deprecated Use \OC::$server->getUserManager()->get() and then run delete() on the return - */ - public static function deleteUser($uid) { - $user = \OC::$server->getUserManager()->get($uid); - if ($user) { - return $user->delete(); - } else { - return false; - } - } - /** * Try to login a user * diff --git a/tests/lib/files/cache/cache.php b/tests/lib/files/cache/cache.php index 6ae095fa5c2..d674ac27fa1 100644 --- a/tests/lib/files/cache/cache.php +++ b/tests/lib/files/cache/cache.php @@ -373,7 +373,8 @@ class Cache extends \Test\TestCase { $tagManager->delete('tag2'); $this->logout(); - \OC_User::deleteUser($userId); + $user = \OC::$server->getUserManager()->get($userId); + if ($user !== null) { $user->delete(); } } function testMove() { diff --git a/tests/lib/files/cache/updaterlegacy.php b/tests/lib/files/cache/updaterlegacy.php index ca59850eb0b..09688afd465 100644 --- a/tests/lib/files/cache/updaterlegacy.php +++ b/tests/lib/files/cache/updaterlegacy.php @@ -72,7 +72,10 @@ class UpdaterLegacy extends \Test\TestCase { if ($this->cache) { $this->cache->clear(); } - $result = \OC_User::deleteUser(self::$user); + + $result = false; + $user = \OC::$server->getUserManager()->get(self::$user); + if ($user !== null) { $result = $user->delete(); } $this->assertTrue($result); $this->logout(); diff --git a/tests/lib/files/filesystem.php b/tests/lib/files/filesystem.php index 1de8cba5446..de8979affd1 100644 --- a/tests/lib/files/filesystem.php +++ b/tests/lib/files/filesystem.php @@ -340,7 +340,8 @@ class Filesystem extends \Test\TestCase { $this->assertEquals('home::' . $userId, $homeMount->getId()); } - \OC_User::deleteUser($userId); + $user = \OC::$server->getUserManager()->get($userId); + if ($user !== null) { $user->delete(); } } /** @@ -368,7 +369,8 @@ class Filesystem extends \Test\TestCase { $this->assertTrue($homeMount->instanceOfStorage('\OC\Files\Storage\Home')); $this->assertEquals('local::' . $datadir . '/' . $userId . '/', $homeMount->getId()); - \OC_User::deleteUser($userId); + $user = \OC::$server->getUserManager()->get($userId); + if ($user !== null) { $user->delete(); } // delete storage entry $cache->clear(); } @@ -398,7 +400,8 @@ class Filesystem extends \Test\TestCase { list($storage, $internalPath) = \OC\Files\Filesystem::resolvePath('/' . $userId . '/cache'); $this->assertTrue($storage->instanceOfStorage('\OCP\Files\IHomeStorage')); $this->assertEquals('cache', $internalPath); - \OC_User::deleteUser($userId); + $user = \OC::$server->getUserManager()->get($userId); + if ($user !== null) { $user->delete(); } $config->setSystemValue('cache_path', $oldCachePath); } @@ -426,7 +429,8 @@ class Filesystem extends \Test\TestCase { list($storage, $internalPath) = \OC\Files\Filesystem::resolvePath('/' . $userId . '/cache'); $this->assertTrue($storage->instanceOfStorage('\OC\Files\Storage\Local')); $this->assertEquals('', $internalPath); - \OC_User::deleteUser($userId); + $user = \OC::$server->getUserManager()->get($userId); + if ($user !== null) { $user->delete(); } $config->setSystemValue('cache_path', $oldCachePath); } diff --git a/tests/lib/files/objectstore/swift.php b/tests/lib/files/objectstore/swift.php index 906efb6390f..a63f5844145 100644 --- a/tests/lib/files/objectstore/swift.php +++ b/tests/lib/files/objectstore/swift.php @@ -51,7 +51,8 @@ class Swift extends \Test\Files\Storage\Storage { // create users $users = array('test'); foreach($users as $userName) { - \OC_User::deleteUser($userName); + $user = \OC::$server->getUserManager()->get($userName); + if ($user !== null) { $user->delete(); } \OC::$server->getUserManager()->createUser($userName, $userName); } @@ -76,7 +77,8 @@ class Swift extends \Test\Files\Storage\Storage { $users = array('test'); foreach($users as $userName) { - \OC_User::deleteUser($userName); + $user = \OC::$server->getUserManager()->get($userName); + if ($user !== null) { $user->delete(); } } parent::tearDown(); } diff --git a/tests/lib/files/storage/homestoragequota.php b/tests/lib/files/storage/homestoragequota.php index bee05438c80..9a3c6b151fc 100644 --- a/tests/lib/files/storage/homestoragequota.php +++ b/tests/lib/files/storage/homestoragequota.php @@ -44,7 +44,8 @@ class HomeStorageQuota extends \Test\TestCase { // clean up \OC_User::setUserId(''); - \OC_User::deleteUser($user1); + $user = \OC::$server->getUserManager()->get($user1); + if ($user !== null) { $user->delete(); } \OC::$server->getConfig()->deleteAllUserValues($user1); \OC_Util::tearDownFS(); } @@ -71,7 +72,8 @@ class HomeStorageQuota extends \Test\TestCase { // clean up \OC_User::setUserId(''); - \OC_User::deleteUser($user1); + $user = \OC::$server->getUserManager()->get($user1); + if ($user !== null) { $user->delete(); } \OC::$server->getConfig()->deleteAllUserValues($user1); \OC_Util::tearDownFS(); } diff --git a/tests/lib/helperstorage.php b/tests/lib/helperstorage.php index 1b2d1ec4fea..3109b509549 100644 --- a/tests/lib/helperstorage.php +++ b/tests/lib/helperstorage.php @@ -45,7 +45,8 @@ class Test_Helper_Storage extends \Test\TestCase { \OC\Files\Filesystem::mount($this->storage, array(), '/'); \OC_User::setUserId(''); - \OC_User::deleteUser($this->user); + $user = \OC::$server->getUserManager()->get($this->user); + if ($user !== null) { $user->delete(); } \OC::$server->getConfig()->deleteAllUserValues($this->user); parent::tearDown(); diff --git a/tests/lib/security/certificatemanager.php b/tests/lib/security/certificatemanager.php index a4e6d8d68ac..f2e29cab18e 100644 --- a/tests/lib/security/certificatemanager.php +++ b/tests/lib/security/certificatemanager.php @@ -39,7 +39,8 @@ class CertificateManagerTest extends \Test\TestCase { } protected function tearDown() { - \OC_User::deleteUser($this->username); + $user = \OC::$server->getUserManager()->get($this->username); + if ($user !== null) { $user->delete(); } parent::tearDown(); } diff --git a/tests/lib/share/share.php b/tests/lib/share/share.php index 273073239c9..5162a03f367 100644 --- a/tests/lib/share/share.php +++ b/tests/lib/share/share.php @@ -94,13 +94,20 @@ class Test_Share extends \Test\TestCase { $query->execute(array('test')); \OC::$server->getAppConfig()->setValue('core', 'shareapi_allow_resharing', $this->resharing); - OC_User::deleteUser($this->user1); - OC_User::deleteUser($this->user2); - OC_User::deleteUser($this->user3); - OC_User::deleteUser($this->user4); - OC_User::deleteUser($this->user5); - OC_User::deleteUser($this->user6); - OC_User::deleteUser($this->groupAndUser); + $user = \OC::$server->getUserManager()->get($this->user1); + if ($user !== null) { $user->delete(); } + $user = \OC::$server->getUserManager()->get($this->user2); + if ($user !== null) { $user->delete(); } + $user = \OC::$server->getUserManager()->get($this->user3); + if ($user !== null) { $user->delete(); } + $user = \OC::$server->getUserManager()->get($this->user4); + if ($user !== null) { $user->delete(); } + $user = \OC::$server->getUserManager()->get($this->user5); + if ($user !== null) { $user->delete(); } + $user = \OC::$server->getUserManager()->get($this->user6); + if ($user !== null) { $user->delete(); } + $user = \OC::$server->getUserManager()->get($this->groupAndUser); + if ($user !== null) { $user->delete(); } OC_Group::deleteGroup($this->group1); OC_Group::deleteGroup($this->group2); @@ -375,7 +382,8 @@ class Test_Share extends \Test\TestCase { // Remove user OC_User::setUserId($this->user1); - OC_User::deleteUser($this->user1); + $user = \OC::$server->getUserManager()->get($this->user1); + if ($user !== null) { $user->delete(); } OC_User::setUserId($this->user2); $this->assertEquals(array('test1.txt'), OCP\Share::getItemsSharedWith('test', Test_Share_Backend::FORMAT_TARGET)); } diff --git a/tests/lib/user.php b/tests/lib/user.php index 26596e2fb54..c4c74cbc254 100644 --- a/tests/lib/user.php +++ b/tests/lib/user.php @@ -51,15 +51,5 @@ class User extends TestCase { $uid = \OC_User::checkPassword('foo', 'bar'); $this->assertEquals($uid, 'foo'); } - - public function testDeleteUser() { - $fail = \OC_User::deleteUser('victim'); - $this->assertFalse($fail); - - $success = \OC::$server->getUserManager()->createUser('victim', 'password'); - - $success = \OC_User::deleteUser('victim'); - $this->assertTrue($success); - } } -- cgit v1.2.3 From ecdf88e41b898889fe8e05bb9c05415dd6b03a95 Mon Sep 17 00:00:00 2001 From: Morris Jobke Date: Thu, 17 Dec 2015 17:07:48 +0100 Subject: Use proper public API for OC_Helper::getFileNameMimeType --- apps/files_sharing/api/local.php | 6 ++++-- apps/files_trashbin/ajax/preview.php | 2 +- apps/files_trashbin/lib/helper.php | 2 +- apps/files_versions/ajax/preview.php | 2 +- lib/private/files/objectstore/objectstorestorage.php | 2 +- lib/private/files/storage/common.php | 2 +- lib/private/helper.php | 8 -------- tests/lib/helper.php | 9 --------- 8 files changed, 9 insertions(+), 24 deletions(-) (limited to 'tests') diff --git a/apps/files_sharing/api/local.php b/apps/files_sharing/api/local.php index aaafafb269f..5b2f2e06e75 100644 --- a/apps/files_sharing/api/local.php +++ b/apps/files_sharing/api/local.php @@ -64,9 +64,10 @@ class Local { if ($shares === false) { return new \OC_OCS_Result(null, 404, 'could not get shares'); } else { + $mimetypeDetector = \OC::$server->getMimeTypeDetector(); foreach ($shares as &$share) { if ($share['item_type'] === 'file' && isset($share['path'])) { - $share['mimetype'] = \OC_Helper::getFileNameMimeType($share['path']); + $share['mimetype'] = $mimetypeDetector->detectPath($share['path']); if (\OC::$server->getPreviewManager()->isMimeSupported($share['mimetype'])) { $share['isPreviewAvailable'] = true; } @@ -227,9 +228,10 @@ class Local { private static function getFilesSharedWithMe() { try { $shares = \OCP\Share::getItemsSharedWith('file'); + $mimetypeDetector = \OC::$server->getMimeTypeDetector(); foreach ($shares as &$share) { if ($share['item_type'] === 'file') { - $share['mimetype'] = \OC_Helper::getFileNameMimeType($share['file_target']); + $share['mimetype'] = $mimetypeDetector->detectPath($share['file_target']); if (\OC::$server->getPreviewManager()->isMimeSupported($share['mimetype'])) { $share['isPreviewAvailable'] = true; } diff --git a/apps/files_trashbin/ajax/preview.php b/apps/files_trashbin/ajax/preview.php index 49d6d93f574..ecb4971253f 100644 --- a/apps/files_trashbin/ajax/preview.php +++ b/apps/files_trashbin/ajax/preview.php @@ -62,7 +62,7 @@ try{ $fileName = substr($fileName, 0, $i); } } - $mimetype = \OC_Helper::getFileNameMimeType($fileName); + $mimetype = \OC::$server->getMimeTypeDetector()->detectPath($fileName); } $preview->setMimetype($mimetype); $preview->setMaxX($maxX); diff --git a/apps/files_trashbin/lib/helper.php b/apps/files_trashbin/lib/helper.php index d14e97285c5..0ccf15cd2bc 100644 --- a/apps/files_trashbin/lib/helper.php +++ b/apps/files_trashbin/lib/helper.php @@ -87,7 +87,7 @@ class Helper $i = array( 'name' => $id, 'mtime' => $timestamp, - 'mimetype' => $view->is_dir($dir . '/' . $entryName) ? 'httpd/unix-directory' : \OC_Helper::getFileNameMimeType($id), + 'mimetype' => $view->is_dir($dir . '/' . $entryName) ? 'httpd/unix-directory' : \OC::$server->getMimeTypeDetector()->detectPath($id), 'type' => $view->is_dir($dir . '/' . $entryName) ? 'dir' : 'file', 'directory' => ($dir === '/') ? '' : $dir, 'size' => $size, diff --git a/apps/files_versions/ajax/preview.php b/apps/files_versions/ajax/preview.php index 0da518f3eaa..f21911abc9b 100644 --- a/apps/files_versions/ajax/preview.php +++ b/apps/files_versions/ajax/preview.php @@ -48,7 +48,7 @@ if($maxX === 0 || $maxY === 0) { try { list($user, $file) = \OCA\Files_Versions\Storage::getUidAndFilename($file); $preview = new \OC\Preview($user, 'files_versions', $file.'.v'.$version); - $mimetype = \OC_Helper::getFileNameMimeType($file); + $mimetype = \OC::$server->getMimeTypeDetector()->detectPath($file); $preview->setMimetype($mimetype); $preview->setMaxX($maxX); $preview->setMaxY($maxY); diff --git a/lib/private/files/objectstore/objectstorestorage.php b/lib/private/files/objectstore/objectstorestorage.php index 5ec05a3529e..a053ea6d6d2 100644 --- a/lib/private/files/objectstore/objectstorestorage.php +++ b/lib/private/files/objectstore/objectstorestorage.php @@ -329,7 +329,7 @@ class ObjectStoreStorage extends \OC\Files\Storage\Common { $stat['mtime'] = $mtime; $this->getCache()->update($stat['fileid'], $stat); } else { - $mimeType = \OC_Helper::getFileNameMimeType($path); + $mimeType = \OC::$server->getMimeTypeDetector()->detectPath($path); // create new file $stat = array( 'etag' => $this->getETag($path), diff --git a/lib/private/files/storage/common.php b/lib/private/files/storage/common.php index b06543d0a6a..091f2edb629 100644 --- a/lib/private/files/storage/common.php +++ b/lib/private/files/storage/common.php @@ -225,7 +225,7 @@ abstract class Common implements Storage { if ($this->is_dir($path)) { return 'httpd/unix-directory'; } elseif ($this->file_exists($path)) { - return \OC_Helper::getFileNameMimeType($path); + return \OC::$server->getMimeTypeDetector()->detectPath($path); } else { return false; } diff --git a/lib/private/helper.php b/lib/private/helper.php index 779a67a2340..c13ac1990dd 100644 --- a/lib/private/helper.php +++ b/lib/private/helper.php @@ -348,14 +348,6 @@ class OC_Helper { return !file_exists($dir); } - /** - * @return \OC\Files\Type\Detection - * @deprecated 8.2.0 use \OC::$server->getMimeTypeDetector() - */ - static public function getMimetypeDetector() { - return \OC::$server->getMimeTypeDetector(); - } - /** * @return \OC\Files\Type\TemplateManager */ diff --git a/tests/lib/helper.php b/tests/lib/helper.php index 469ffecc773..51f7342f456 100644 --- a/tests/lib/helper.php +++ b/tests/lib/helper.php @@ -83,15 +83,6 @@ class Test_Helper extends \Test\TestCase { $this->assertEquals($result, $expected); } - function testGetFileNameMimeType() { - $this->assertEquals('text/plain', OC_Helper::getFileNameMimeType('foo.txt')); - $this->assertEquals('image/png', OC_Helper::getFileNameMimeType('foo.png')); - $this->assertEquals('image/png', OC_Helper::getFileNameMimeType('foo.bar.png')); - $this->assertEquals('application/octet-stream', OC_Helper::getFileNameMimeType('.png')); - $this->assertEquals('application/octet-stream', OC_Helper::getFileNameMimeType('foo')); - $this->assertEquals('application/octet-stream', OC_Helper::getFileNameMimeType('')); - } - function testGetStringMimeType() { if (\OC_Util::runningOnWindows()) { $this->markTestSkipped('[Windows] Strings have mimetype application/octet-stream on Windows'); -- cgit v1.2.3 From 26f416b9f53e646434da0308b6ff4254122d4db2 Mon Sep 17 00:00:00 2001 From: Thomas Müller Date: Thu, 17 Dec 2015 17:15:04 +0100 Subject: Assert that there are no xml error left --- tests/lib/testcase.php | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'tests') diff --git a/tests/lib/testcase.php b/tests/lib/testcase.php index 5d88fa08a6d..c2a72beffcd 100644 --- a/tests/lib/testcase.php +++ b/tests/lib/testcase.php @@ -92,6 +92,12 @@ abstract class TestCase extends \PHPUnit_Framework_TestCase { throw $hookExceptions[0]; } + // fail hard if xml errors have not been cleaned up + $errors = libxml_get_errors(); + libxml_clear_errors(); + $this->assertEquals([], $errors); + + // tearDown the traits $traits = $this->getTestTraits(); foreach ($traits as $trait) { $methodName = 'tearDown' . basename(str_replace('\\', '/', $trait)); -- cgit v1.2.3 From 8f6a257f1bffd5e27a5aae89eb61fb5553d4670e Mon Sep 17 00:00:00 2001 From: Thomas Müller Date: Thu, 17 Dec 2015 17:29:17 +0100 Subject: Use proper base class --- tests/lib/app/dependencyanalyzer.php | 3 ++- tests/lib/app/infoparser.php | 3 ++- tests/lib/app/manager.php | 3 ++- 3 files changed, 6 insertions(+), 3 deletions(-) (limited to 'tests') diff --git a/tests/lib/app/dependencyanalyzer.php b/tests/lib/app/dependencyanalyzer.php index 58f1c0a7a99..fecba518856 100644 --- a/tests/lib/app/dependencyanalyzer.php +++ b/tests/lib/app/dependencyanalyzer.php @@ -12,8 +12,9 @@ namespace Test\App; use OC; use OC\App\Platform; use OCP\IL10N; +use Test\TestCase; -class DependencyAnalyzer extends \PHPUnit_Framework_TestCase { +class DependencyAnalyzer extends TestCase { /** @var Platform */ private $platformMock; diff --git a/tests/lib/app/infoparser.php b/tests/lib/app/infoparser.php index fb4ffe8af94..1e5257abec3 100644 --- a/tests/lib/app/infoparser.php +++ b/tests/lib/app/infoparser.php @@ -10,8 +10,9 @@ namespace Test\App; use OC; +use Test\TestCase; -class InfoParser extends \PHPUnit_Framework_TestCase { +class InfoParser extends TestCase { /** * @var \OC\App\InfoParser diff --git a/tests/lib/app/manager.php b/tests/lib/app/manager.php index 38358fd61cf..a3e55c6b890 100644 --- a/tests/lib/app/manager.php +++ b/tests/lib/app/manager.php @@ -11,8 +11,9 @@ namespace Test\App; use OC\Group\Group; use OC\User\User; +use Test\TestCase; -class Manager extends \PHPUnit_Framework_TestCase { +class Manager extends TestCase { /** * @return \OCP\IAppConfig | \PHPUnit_Framework_MockObject_MockObject */ -- cgit v1.2.3 From 2709244acd6eb385d5c353ff8486470c14670984 Mon Sep 17 00:00:00 2001 From: Thomas Müller Date: Thu, 17 Dec 2015 17:29:50 +0100 Subject: Only perform tearDown operations on the database if we have a connection to it ..... --- tests/lib/testcase.php | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'tests') diff --git a/tests/lib/testcase.php b/tests/lib/testcase.php index c2a72beffcd..1ee0c85b98a 100644 --- a/tests/lib/testcase.php +++ b/tests/lib/testcase.php @@ -156,11 +156,13 @@ abstract class TestCase extends \PHPUnit_Framework_TestCase { public static function tearDownAfterClass() { $dataDir = \OC::$server->getConfig()->getSystemValue('datadirectory', \OC::$SERVERROOT . '/data-autotest'); - $queryBuilder = \OC::$server->getDatabaseConnection()->getQueryBuilder(); + if (\OC::$server->getDatabaseConnection()) { + $queryBuilder = \OC::$server->getDatabaseConnection()->getQueryBuilder(); - self::tearDownAfterClassCleanShares($queryBuilder); - self::tearDownAfterClassCleanStorages($queryBuilder); - self::tearDownAfterClassCleanFileCache($queryBuilder); + self::tearDownAfterClassCleanShares($queryBuilder); + self::tearDownAfterClassCleanStorages($queryBuilder); + self::tearDownAfterClassCleanFileCache($queryBuilder); + } self::tearDownAfterClassCleanStrayDataFiles($dataDir); self::tearDownAfterClassCleanStrayHooks(); self::tearDownAfterClassCleanStrayLocks(); -- cgit v1.2.3 From 345e68cafac4db880ef5cf41f714cab9599b1f8a Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 18 Dec 2015 09:47:54 +0100 Subject: Use the query builder for the joblist queries --- lib/private/backgroundjob/joblist.php | 123 ++++++++++++++++++++++++---------- tests/lib/backgroundjob/joblist.php | 13 ++-- 2 files changed, 92 insertions(+), 44 deletions(-) (limited to 'tests') diff --git a/lib/private/backgroundjob/joblist.php b/lib/private/backgroundjob/joblist.php index 03c9180ddb0..226564e2175 100644 --- a/lib/private/backgroundjob/joblist.php +++ b/lib/private/backgroundjob/joblist.php @@ -28,22 +28,20 @@ use OCP\BackgroundJob\IJobList; use OCP\AutoloadNotAllowedException; class JobList implements IJobList { - /** - * @var \OCP\IDBConnection - */ - private $conn; + /** @var \OCP\IDBConnection */ + protected $connection; /** * @var \OCP\IConfig $config */ - private $config; + protected $config; /** - * @param \OCP\IDBConnection $conn + * @param \OCP\IDBConnection $connection * @param \OCP\IConfig $config */ - public function __construct($conn, $config) { - $this->conn = $conn; + public function __construct($connection, $config) { + $this->connection = $connection; $this->config = $config; } @@ -58,12 +56,20 @@ class JobList implements IJobList { } else { $class = $job; } + $argument = json_encode($argument); if (strlen($argument) > 4000) { throw new \InvalidArgumentException('Background job arguments can\'t exceed 4000 characters (json encoded)'); } - $query = $this->conn->prepare('INSERT INTO `*PREFIX*jobs`(`class`, `argument`, `last_run`) VALUES(?, ?, 0)'); - $query->execute(array($class, $argument)); + + $query = $this->connection->getQueryBuilder(); + $query->insert('jobs') + ->values([ + 'class' => $query->createNamedParameter($class), + 'argument' => $query->createNamedParameter($argument), + 'last_run' => $query->createNamedParameter(0, \PDO::PARAM_INT), + ]); + $query->execute(); } } @@ -77,19 +83,25 @@ class JobList implements IJobList { } else { $class = $job; } + + $query = $this->connection->getQueryBuilder(); + $query->delete('jobs') + ->where($query->expr()->eq('class', $query->createNamedParameter($class))); if (!is_null($argument)) { $argument = json_encode($argument); - $query = $this->conn->prepare('DELETE FROM `*PREFIX*jobs` WHERE `class` = ? AND `argument` = ?'); - $query->execute(array($class, $argument)); - } else { - $query = $this->conn->prepare('DELETE FROM `*PREFIX*jobs` WHERE `class` = ?'); - $query->execute(array($class)); + $query->andWhere($query->expr()->eq('argument', $query->createNamedParameter($argument))); } + $query->execute(); } + /** + * @param int $id + */ protected function removeById($id) { - $query = $this->conn->prepare('DELETE FROM `*PREFIX*jobs` WHERE `id` = ?'); - $query->execute([$id]); + $query = $this->connection->getQueryBuilder(); + $query->delete('jobs') + ->where($query->expr()->eq('id', $query->createNamedParameter($id, \PDO::PARAM_INT))); + $query->execute(); } /** @@ -106,9 +118,19 @@ class JobList implements IJobList { $class = $job; } $argument = json_encode($argument); - $query = $this->conn->prepare('SELECT `id` FROM `*PREFIX*jobs` WHERE `class` = ? AND `argument` = ?'); - $query->execute(array($class, $argument)); - return (bool)$query->fetch(); + + $query = $this->connection->getQueryBuilder(); + $query->select('id') + ->from('jobs') + ->where($query->expr()->eq('class', $query->createNamedParameter($class))) + ->andWhere($query->expr()->eq('argument', $query->createNamedParameter($argument))) + ->setMaxResults(1); + + $result = $query->execute(); + $row = $result->fetch(); + $result->closeCursor(); + + return (bool) $row; } /** @@ -117,15 +139,20 @@ class JobList implements IJobList { * @return Job[] */ public function getAll() { - $query = $this->conn->prepare('SELECT `id`, `class`, `last_run`, `argument` FROM `*PREFIX*jobs`'); - $query->execute(); - $jobs = array(); - while ($row = $query->fetch()) { + $query = $this->connection->getQueryBuilder(); + $query->select('*') + ->from('jobs'); + $result = $query->execute(); + + $jobs = []; + while ($row = $result->fetch()) { $job = $this->buildJob($row); if ($job) { $jobs[] = $job; } } + $result->closeCursor(); + return $jobs; } @@ -136,22 +163,39 @@ class JobList implements IJobList { */ public function getNext() { $lastId = $this->getLastJob(); - $query = $this->conn->prepare('SELECT `id`, `class`, `last_run`, `argument` FROM `*PREFIX*jobs` WHERE `id` > ? ORDER BY `id` ASC', 1); - $query->execute(array($lastId)); - if ($row = $query->fetch()) { + + $query = $this->connection->getQueryBuilder(); + $query->select('*') + ->from('jobs') + ->where($query->expr()->gt('id', $query->createNamedParameter($lastId, \PDO::PARAM_INT))) + ->orderBy('id', 'ASC') + ->setMaxResults(1); + $result = $query->execute(); + $row = $result->fetch(); + $result->closeCursor(); + + if ($row) { $jobId = $row['id']; $job = $this->buildJob($row); } else { //begin at the start of the queue - $query = $this->conn->prepare('SELECT `id`, `class`, `last_run`, `argument` FROM `*PREFIX*jobs` ORDER BY `id` ASC', 1); - $query->execute(); - if ($row = $query->fetch()) { + $query = $this->connection->getQueryBuilder(); + $query->select('*') + ->from('jobs') + ->orderBy('id', 'ASC') + ->setMaxResults(1); + $result = $query->execute(); + $row = $result->fetch(); + $result->closeCursor(); + + if ($row) { $jobId = $row['id']; $job = $this->buildJob($row); } else { return null; //empty job list } } + if (is_null($job)) { $this->removeById($jobId); return $this->getNext(); @@ -165,9 +209,15 @@ class JobList implements IJobList { * @return Job|null */ public function getById($id) { - $query = $this->conn->prepare('SELECT `id`, `class`, `last_run`, `argument` FROM `*PREFIX*jobs` WHERE `id` = ?'); - $query->execute(array($id)); - if ($row = $query->fetch()) { + $query = $this->connection->getQueryBuilder(); + $query->select('*') + ->from('jobs') + ->where($query->expr()->eq('id', $query->createNamedParameter($id, \PDO::PARAM_INT))); + $result = $query->execute(); + $row = $result->fetch(); + $result->closeCursor(); + + if ($row) { return $this->buildJob($row); } else { return null; @@ -225,7 +275,10 @@ class JobList implements IJobList { * @param Job $job */ public function setLastRun($job) { - $query = $this->conn->prepare('UPDATE `*PREFIX*jobs` SET `last_run` = ? WHERE `id` = ?'); - $query->execute(array(time(), $job->getId())); + $query = $this->connection->getQueryBuilder(); + $query->update('jobs') + ->set('last_run', $query->createNamedParameter(time(), \PDO::PARAM_INT)) + ->where($query->expr()->eq('id', $query->createNamedParameter($job->getId(), \PDO::PARAM_INT))); + $query->execute(); } } diff --git a/tests/lib/backgroundjob/joblist.php b/tests/lib/backgroundjob/joblist.php index 73b3255c079..c0796d8253a 100644 --- a/tests/lib/backgroundjob/joblist.php +++ b/tests/lib/backgroundjob/joblist.php @@ -15,26 +15,21 @@ use Test\TestCase; * Class JobList * * @group DB - * * @package Test\BackgroundJob */ class JobList extends TestCase { - /** - * @var \OC\BackgroundJob\JobList - */ + /** @var \OC\BackgroundJob\JobList */ protected $instance; - /** - * @var \OCP\IConfig | \PHPUnit_Framework_MockObject_MockObject $config - */ + /** @var \OCP\IConfig|\PHPUnit_Framework_MockObject_MockObject */ protected $config; protected function setUp() { parent::setUp(); - $conn = \OC::$server->getDatabaseConnection(); + $connection = \OC::$server->getDatabaseConnection(); $this->config = $this->getMock('\OCP\IConfig'); - $this->instance = new \OC\BackgroundJob\JobList($conn, $this->config); + $this->instance = new \OC\BackgroundJob\JobList($connection, $this->config); } protected function getAllSorted() { -- cgit v1.2.3 From 90b0831cec88763038e241cc4816db204c045e69 Mon Sep 17 00:00:00 2001 From: Morris Jobke Date: Fri, 18 Dec 2015 11:19:53 +0100 Subject: Use TempManager instead of tmpFolder --- lib/private/archive/tar.php | 2 +- lib/private/files/storage/common.php | 2 +- lib/private/files/storage/temporary.php | 2 +- lib/private/helper.php | 12 ------------ lib/private/installer.php | 2 +- tests/lib/configtests.php | 2 +- tests/lib/files/cache/homecache.php | 2 +- tests/lib/files/etagtest.php | 2 +- tests/lib/files/filesystem.php | 4 ++-- tests/lib/files/storage/commontest.php | 2 +- tests/lib/files/storage/home.php | 2 +- tests/lib/files/storage/local.php | 2 +- tests/lib/files/storage/wrapper/quota.php | 2 +- tests/lib/files/storage/wrapper/wrapper.php | 2 +- tests/lib/files/view.php | 2 +- tests/lib/helper.php | 2 +- tests/lib/utilcheckserver.php | 2 +- 17 files changed, 17 insertions(+), 29 deletions(-) (limited to 'tests') diff --git a/lib/private/archive/tar.php b/lib/private/archive/tar.php index 4448e56850d..4066e1d86c1 100644 --- a/lib/private/archive/tar.php +++ b/lib/private/archive/tar.php @@ -86,7 +86,7 @@ class OC_Archive_TAR extends OC_Archive { * @return bool */ function addFolder($path) { - $tmpBase = OC_Helper::tmpFolder(); + $tmpBase = \OC::$server->getTempManager()->getTemporaryFolder(); if (substr($path, -1, 1) != '/') { $path .= '/'; } diff --git a/lib/private/files/storage/common.php b/lib/private/files/storage/common.php index 091f2edb629..1e30d48f613 100644 --- a/lib/private/files/storage/common.php +++ b/lib/private/files/storage/common.php @@ -248,7 +248,7 @@ abstract class Common implements Storage { } public function getLocalFolder($path) { - $baseDir = \OC_Helper::tmpFolder(); + $baseDir = \OC::$server->getTempManager()->getTemporaryFolder(); $this->addLocalFolder($path, $baseDir); return $baseDir; } diff --git a/lib/private/files/storage/temporary.php b/lib/private/files/storage/temporary.php index c8b99a55637..8abc19929b0 100644 --- a/lib/private/files/storage/temporary.php +++ b/lib/private/files/storage/temporary.php @@ -29,7 +29,7 @@ namespace OC\Files\Storage; */ class Temporary extends Local{ public function __construct($arguments = null) { - parent::__construct(array('datadir' => \OC_Helper::tmpFolder())); + parent::__construct(array('datadir' => \OC::$server->getTempManager()->getTemporaryFolder())); } public function cleanUp() { diff --git a/lib/private/helper.php b/lib/private/helper.php index 72282362fe6..556286f3519 100644 --- a/lib/private/helper.php +++ b/lib/private/helper.php @@ -469,18 +469,6 @@ class OC_Helper { return \OC::$server->getTempManager()->getTemporaryFile($postfix); } - /** - * create a temporary folder with an unique filename - * - * @return string - * @deprecated Use the TempManager instead - * - * temporary files are automatically cleaned up after the script is finished - */ - public static function tmpFolder() { - return \OC::$server->getTempManager()->getTemporaryFolder(); - } - /** * Adds a suffix to the name in case the file exists * diff --git a/lib/private/installer.php b/lib/private/installer.php index fa9fc6704df..c188f15718a 100644 --- a/lib/private/installer.php +++ b/lib/private/installer.php @@ -284,7 +284,7 @@ class OC_Installer{ } //extract the archive in a temporary folder - $extractDir=OC_Helper::tmpFolder(); + $extractDir = \OC::$server->getTempManager()->getTemporaryFolder(); OC_Helper::rmdirr($extractDir); mkdir($extractDir); if($archive=OC_Archive::open($path)) { diff --git a/tests/lib/configtests.php b/tests/lib/configtests.php index 0269ae542f4..c0251e693c6 100644 --- a/tests/lib/configtests.php +++ b/tests/lib/configtests.php @@ -23,7 +23,7 @@ class ConfigTests extends TestCase { protected function setUp() { parent::setUp(); - $this->randomTmpDir = \OC_Helper::tmpFolder(); + $this->randomTmpDir = \OC::$server->getTempManager()->getTemporaryFolder(); $this->configFile = $this->randomTmpDir.'testconfig.php'; file_put_contents($this->configFile, self::TESTCONTENT); $this->config = new \OC\Config($this->randomTmpDir, 'testconfig.php'); diff --git a/tests/lib/files/cache/homecache.php b/tests/lib/files/cache/homecache.php index 3adb25fa9d4..e133d0afc55 100644 --- a/tests/lib/files/cache/homecache.php +++ b/tests/lib/files/cache/homecache.php @@ -69,7 +69,7 @@ class HomeCache extends \Test\TestCase { protected function setUp() { parent::setUp(); - $this->user = new DummyUser('foo', \OC_Helper::tmpFolder()); + $this->user = new DummyUser('foo', \OC::$server->getTempManager()->getTemporaryFolder()); $this->storage = new \OC\Files\Storage\Home(array('user' => $this->user)); $this->cache = $this->storage->getCache(); } diff --git a/tests/lib/files/etagtest.php b/tests/lib/files/etagtest.php index c214a3d4da6..d8e44000f9c 100644 --- a/tests/lib/files/etagtest.php +++ b/tests/lib/files/etagtest.php @@ -39,7 +39,7 @@ class EtagTest extends \Test\TestCase { $config = \OC::$server->getConfig(); $this->datadir = $config->getSystemValue('datadirectory'); - $this->tmpDir = \OC_Helper::tmpFolder(); + $this->tmpDir = \OC::$server->getTempManager()->getTemporaryFolder(); $config->setSystemValue('datadirectory', $this->tmpDir); $this->userBackend = new \Test\Util\User\Dummy(); diff --git a/tests/lib/files/filesystem.php b/tests/lib/files/filesystem.php index de8979affd1..328ba5f4488 100644 --- a/tests/lib/files/filesystem.php +++ b/tests/lib/files/filesystem.php @@ -72,7 +72,7 @@ class Filesystem extends \Test\TestCase { * @return array */ private function getStorageData() { - $dir = \OC_Helper::tmpFolder(); + $dir = \OC::$server->getTempManager()->getTemporaryFolder(); $this->tmpDirs[] = $dir; return array('datadir' => $dir); } @@ -416,7 +416,7 @@ class Filesystem extends \Test\TestCase { $config = \OC::$server->getConfig(); $oldCachePath = $config->getSystemValue('cache_path', ''); // set cache path to temp dir - $cachePath = \OC_Helper::tmpFolder() . '/extcache'; + $cachePath = \OC::$server->getTempManager()->getTemporaryFolder() . '/extcache'; $config->setSystemValue('cache_path', $cachePath); \OC::$server->getUserManager()->createUser($userId, $userId); diff --git a/tests/lib/files/storage/commontest.php b/tests/lib/files/storage/commontest.php index bbe6f2a73e2..38faa9b0b21 100644 --- a/tests/lib/files/storage/commontest.php +++ b/tests/lib/files/storage/commontest.php @@ -37,7 +37,7 @@ class CommonTest extends Storage { protected function setUp() { parent::setUp(); - $this->tmpDir=\OC_Helper::tmpFolder(); + $this->tmpDir = \OC::$server->getTempManager()->getTemporaryFolder(); $this->instance=new \OC\Files\Storage\CommonTest(array('datadir'=>$this->tmpDir)); } diff --git a/tests/lib/files/storage/home.php b/tests/lib/files/storage/home.php index a51912ca1b2..7e10f09d554 100644 --- a/tests/lib/files/storage/home.php +++ b/tests/lib/files/storage/home.php @@ -70,7 +70,7 @@ class Home extends Storage { protected function setUp() { parent::setUp(); - $this->tmpDir = \OC_Helper::tmpFolder(); + $this->tmpDir = \OC::$server->getTempManager()->getTemporaryFolder(); $this->userId = $this->getUniqueID('user_'); $this->user = new DummyUser($this->userId, $this->tmpDir); $this->instance = new \OC\Files\Storage\Home(array('user' => $this->user)); diff --git a/tests/lib/files/storage/local.php b/tests/lib/files/storage/local.php index 36267dc6605..2583863b554 100644 --- a/tests/lib/files/storage/local.php +++ b/tests/lib/files/storage/local.php @@ -38,7 +38,7 @@ class Local extends Storage { protected function setUp() { parent::setUp(); - $this->tmpDir = \OC_Helper::tmpFolder(); + $this->tmpDir = \OC::$server->getTempManager()->getTemporaryFolder(); $this->instance = new \OC\Files\Storage\Local(array('datadir' => $this->tmpDir)); } diff --git a/tests/lib/files/storage/wrapper/quota.php b/tests/lib/files/storage/wrapper/quota.php index b0a06b0d898..95bc2ff7a1a 100644 --- a/tests/lib/files/storage/wrapper/quota.php +++ b/tests/lib/files/storage/wrapper/quota.php @@ -27,7 +27,7 @@ class Quota extends \Test\Files\Storage\Storage { protected function setUp() { parent::setUp(); - $this->tmpDir = \OC_Helper::tmpFolder(); + $this->tmpDir = \OC::$server->getTempManager()->getTemporaryFolder(); $storage = new \OC\Files\Storage\Local(array('datadir' => $this->tmpDir)); $this->instance = new \OC\Files\Storage\Wrapper\Quota(array('storage' => $storage, 'quota' => 10000000)); } diff --git a/tests/lib/files/storage/wrapper/wrapper.php b/tests/lib/files/storage/wrapper/wrapper.php index 486cd0495c1..a5a678cb9f7 100644 --- a/tests/lib/files/storage/wrapper/wrapper.php +++ b/tests/lib/files/storage/wrapper/wrapper.php @@ -17,7 +17,7 @@ class Wrapper extends \Test\Files\Storage\Storage { protected function setUp() { parent::setUp(); - $this->tmpDir = \OC_Helper::tmpFolder(); + $this->tmpDir = \OC::$server->getTempManager()->getTemporaryFolder(); $storage = new \OC\Files\Storage\Local(array('datadir' => $this->tmpDir)); $this->instance = new \OC\Files\Storage\Wrapper\Wrapper(array('storage' => $storage)); } diff --git a/tests/lib/files/view.php b/tests/lib/files/view.php index 1fc4b9ab684..3e88a5306f8 100644 --- a/tests/lib/files/view.php +++ b/tests/lib/files/view.php @@ -757,7 +757,7 @@ class View extends \Test\TestCase { * 228 is the max path length in windows */ $folderName = 'abcdefghijklmnopqrstuvwxyz012345678901234567890123456789'; - $tmpdirLength = strlen(\OC_Helper::tmpFolder()); + $tmpdirLength = strlen(\OC::$server->getTempManager()->getTemporaryFolder()); if (\OC_Util::runningOnWindows()) { $this->markTestSkipped('[Windows] '); $depth = ((260 - $tmpdirLength) / 57); diff --git a/tests/lib/helper.php b/tests/lib/helper.php index 51f7342f456..9ad3af3d60c 100644 --- a/tests/lib/helper.php +++ b/tests/lib/helper.php @@ -403,7 +403,7 @@ class Test_Helper extends \Test\TestCase { * Tests recursive folder deletion with rmdirr() */ public function testRecursiveFolderDeletion() { - $baseDir = \OC_Helper::tmpFolder() . '/'; + $baseDir = \OC::$server->getTempManager()->getTemporaryFolder() . '/'; mkdir($baseDir . 'a/b/c/d/e', 0777, true); mkdir($baseDir . 'a/b/c1/d/e', 0777, true); mkdir($baseDir . 'a/b/c2/d/e', 0777, true); diff --git a/tests/lib/utilcheckserver.php b/tests/lib/utilcheckserver.php index a5ec529ff85..64757d0acd9 100644 --- a/tests/lib/utilcheckserver.php +++ b/tests/lib/utilcheckserver.php @@ -37,7 +37,7 @@ class Test_Util_CheckServer extends \Test\TestCase { protected function setUp() { parent::setUp(); - $this->datadir = \OC_Helper::tmpFolder(); + $this->datadir = \OC::$server->getTempManager()->getTemporaryFolder(); file_put_contents($this->datadir . '/.ocdata', ''); \OC::$server->getSession()->set('checkServer_succeeded', false); -- cgit v1.2.3 From a18c0983d5db32e98461404888200118f83011d7 Mon Sep 17 00:00:00 2001 From: Morris Jobke Date: Fri, 18 Dec 2015 11:25:33 +0100 Subject: Use TempManager instead of tmpFile --- lib/private/files/objectstore/objectstorestorage.php | 2 +- lib/private/files/storage/localtempfiletrait.php | 2 +- lib/private/files/type/detection.php | 2 +- lib/private/files/view.php | 2 +- lib/private/helper.php | 13 ------------- lib/private/installer.php | 2 +- lib/private/preview/movie.php | 4 ++-- tests/lib/files/filesystem.php | 2 +- tests/lib/installer.php | 6 +++--- tests/lib/streamwrappers.php | 4 ++-- 10 files changed, 13 insertions(+), 26 deletions(-) (limited to 'tests') diff --git a/lib/private/files/objectstore/objectstorestorage.php b/lib/private/files/objectstore/objectstorestorage.php index a053ea6d6d2..b34a6bdfb84 100644 --- a/lib/private/files/objectstore/objectstorestorage.php +++ b/lib/private/files/objectstore/objectstorestorage.php @@ -274,7 +274,7 @@ class ObjectStoreStorage extends \OC\Files\Storage\Common { } else { $ext = ''; } - $tmpFile = \OC_Helper::tmpFile($ext); + $tmpFile = \OC::$server->getTempManager()->getTemporaryFile($ext); \OC\Files\Stream\Close::registerCallback($tmpFile, array($this, 'writeBack')); if ($this->file_exists($path)) { $source = $this->fopen($path, 'r'); diff --git a/lib/private/files/storage/localtempfiletrait.php b/lib/private/files/storage/localtempfiletrait.php index 84331f49b19..8875c2c4493 100644 --- a/lib/private/files/storage/localtempfiletrait.php +++ b/lib/private/files/storage/localtempfiletrait.php @@ -70,7 +70,7 @@ trait LocalTempFileTrait { } else { $extension = ''; } - $tmpFile = \OC_Helper::tmpFile($extension); + $tmpFile = \OC::$server->getTempManager()->getTemporaryFile($extension); $target = fopen($tmpFile, 'w'); \OC_Helper::streamCopy($source, $target); fclose($target); diff --git a/lib/private/files/type/detection.php b/lib/private/files/type/detection.php index c102e739e04..0e2bab39e5b 100644 --- a/lib/private/files/type/detection.php +++ b/lib/private/files/type/detection.php @@ -238,7 +238,7 @@ class Detection implements IMimeTypeDetector { $finfo = finfo_open(FILEINFO_MIME); return finfo_buffer($finfo, $data); } else { - $tmpFile = \OC_Helper::tmpFile(); + $tmpFile = \OC::$server->getTempManager()->getTemporaryFile(); $fh = fopen($tmpFile, 'wb'); fwrite($fh, $data, 8024); fclose($fh); diff --git a/lib/private/files/view.php b/lib/private/files/view.php index b8b1b8a50d6..fcea4828c49 100644 --- a/lib/private/files/view.php +++ b/lib/private/files/view.php @@ -912,7 +912,7 @@ class View { $source = $this->fopen($path, 'r'); if ($source) { $extension = pathinfo($path, PATHINFO_EXTENSION); - $tmpFile = \OC_Helper::tmpFile($extension); + $tmpFile = \OC::$server->getTempManager()->getTemporaryFile($extension); file_put_contents($tmpFile, $source); return $tmpFile; } else { diff --git a/lib/private/helper.php b/lib/private/helper.php index 556286f3519..08ddf2b0522 100644 --- a/lib/private/helper.php +++ b/lib/private/helper.php @@ -456,19 +456,6 @@ class OC_Helper { return array($count, $result); } - /** - * create a temporary file with an unique filename - * - * @param string $postfix - * @return string - * @deprecated Use the TempManager instead - * - * temporary files are automatically cleaned up after the script is finished - */ - public static function tmpFile($postfix = '') { - return \OC::$server->getTempManager()->getTemporaryFile($postfix); - } - /** * Adds a suffix to the name in case the file exists * diff --git a/lib/private/installer.php b/lib/private/installer.php index c188f15718a..e1447db0db5 100644 --- a/lib/private/installer.php +++ b/lib/private/installer.php @@ -264,7 +264,7 @@ class OC_Installer{ //download the file if necessary if($data['source']=='http') { $pathInfo = pathinfo($data['href']); - $path=OC_Helper::tmpFile('.' . $pathInfo['extension']); + $path = \OC::$server->getTempManager()->getTemporaryFile('.' . $pathInfo['extension']); if(!isset($data['href'])) { throw new \Exception($l->t("No href specified when installing app from http")); } diff --git a/lib/private/preview/movie.php b/lib/private/preview/movie.php index f71eaaf3eb2..2c2e6d09399 100644 --- a/lib/private/preview/movie.php +++ b/lib/private/preview/movie.php @@ -46,7 +46,7 @@ class Movie extends Provider { if ($useFileDirectly) { $absPath = $fileview->getLocalFile($path); } else { - $absPath = \OC_Helper::tmpFile(); + $absPath = \OC::$server->getTempManager()->getTemporaryFile(); $handle = $fileview->fopen($path, 'rb'); @@ -79,7 +79,7 @@ class Movie extends Provider { * @return bool|\OCP\IImage */ private function generateThumbNail($maxX, $maxY, $absPath, $second) { - $tmpPath = \OC_Helper::tmpFile(); + $tmpPath = \OC::$server->getTempManager()->getTemporaryFile(); if (self::$avconvBinary) { $cmd = self::$avconvBinary . ' -an -y -ss ' . escapeshellarg($second) . diff --git a/tests/lib/files/filesystem.php b/tests/lib/files/filesystem.php index 328ba5f4488..db1f22f894a 100644 --- a/tests/lib/files/filesystem.php +++ b/tests/lib/files/filesystem.php @@ -302,7 +302,7 @@ class Filesystem extends \Test\TestCase { \OC\Files\Filesystem::mkdir('/bar'); // \OC\Files\Filesystem::file_put_contents('/bar//foo', 'foo'); - $tmpFile = \OC_Helper::tmpFile(); + $tmpFile = \OC::$server->getTempManager()->getTemporaryFile(); file_put_contents($tmpFile, 'foo'); $fh = fopen($tmpFile, 'r'); // \OC\Files\Filesystem::file_put_contents('/bar//foo', $fh); diff --git a/tests/lib/installer.php b/tests/lib/installer.php index c3c2f8a275e..cfad4d7f0de 100644 --- a/tests/lib/installer.php +++ b/tests/lib/installer.php @@ -32,7 +32,7 @@ class Test_Installer extends \Test\TestCase { $pathOfTestApp .= '/../data/'; $pathOfTestApp .= 'testapp.zip'; - $tmp = OC_Helper::tmpFile('.zip'); + $tmp = \OC::$server->getTempManager()->getTemporaryFile('.zip'); OC_Helper::copyr($pathOfTestApp, $tmp); $data = array( @@ -51,7 +51,7 @@ class Test_Installer extends \Test\TestCase { $pathOfOldTestApp .= '/../data/'; $pathOfOldTestApp .= 'testapp.zip'; - $oldTmp = OC_Helper::tmpFile('.zip'); + $oldTmp = \OC::$server->getTempManager()->getTemporaryFile('.zip'); OC_Helper::copyr($pathOfOldTestApp, $oldTmp); $oldData = array( @@ -63,7 +63,7 @@ class Test_Installer extends \Test\TestCase { $pathOfNewTestApp .= '/../data/'; $pathOfNewTestApp .= 'testapp2.zip'; - $newTmp = OC_Helper::tmpFile('.zip'); + $newTmp = \OC::$server->getTempManager()->getTemporaryFile('.zip'); OC_Helper::copyr($pathOfNewTestApp, $newTmp); $newData = array( diff --git a/tests/lib/streamwrappers.php b/tests/lib/streamwrappers.php index 9b097535280..7175683a60b 100644 --- a/tests/lib/streamwrappers.php +++ b/tests/lib/streamwrappers.php @@ -55,7 +55,7 @@ class Test_StreamWrappers extends \Test\TestCase { public function testCloseStream() { //ensure all basic stream stuff works $sourceFile = OC::$SERVERROOT . '/tests/data/lorem.txt'; - $tmpFile = OC_Helper::TmpFile('.txt'); + $tmpFile = \OC::$server->getTempManager()->getTemporaryFile('.txt'); $file = 'close://' . $tmpFile; $this->assertTrue(file_exists($file)); file_put_contents($file, file_get_contents($sourceFile)); @@ -65,7 +65,7 @@ class Test_StreamWrappers extends \Test\TestCase { $this->assertFalse(file_exists($file)); //test callback - $tmpFile = OC_Helper::TmpFile('.txt'); + $tmpFile = \OC::$server->getTempManager()->getTemporaryFile('.txt'); $file = 'close://' . $tmpFile; $actual = false; $callback = function($path) use (&$actual) { $actual = $path; }; -- cgit v1.2.3 From 3d55569a277a68c9dac54b33684c3e22839386d8 Mon Sep 17 00:00:00 2001 From: Morris Jobke Date: Fri, 18 Dec 2015 11:37:18 +0100 Subject: OC_Helper::makeURLAbsolute is not used anymore --- lib/private/helper.php | 12 ------------ tests/lib/helper.php | 42 ------------------------------------------ 2 files changed, 54 deletions(-) (limited to 'tests') diff --git a/lib/private/helper.php b/lib/private/helper.php index 72282362fe6..92ae25de4cc 100644 --- a/lib/private/helper.php +++ b/lib/private/helper.php @@ -69,18 +69,6 @@ class OC_Helper { ); } - /** - * Makes an $url absolute - * @param string $url the url - * @return string the absolute url - * @deprecated Use \OC::$server->getURLGenerator()->getAbsoluteURL($url) - * - * Returns a absolute url to the given app and file. - */ - public static function makeURLAbsolute($url) { - return OC::$server->getURLGenerator()->getAbsoluteURL($url); - } - /** * Creates an url for remote use * @param string $service id diff --git a/tests/lib/helper.php b/tests/lib/helper.php index 51f7342f456..794502e5e21 100644 --- a/tests/lib/helper.php +++ b/tests/lib/helper.php @@ -243,48 +243,6 @@ class Test_Helper extends \Test\TestCase { // Url generator methods - /** - * @small - * test absolute URL construction - * @dataProvider provideDocRootURLs - */ - function testMakeAbsoluteURLDocRoot($url, $expectedResult) { - \OC::$WEBROOT = ''; - $result = \OC_Helper::makeURLAbsolute($url); - - $this->assertEquals($expectedResult, $result); - } - - /** - * @small - * test absolute URL construction - * @dataProvider provideSubDirURLs - */ - function testMakeAbsoluteURLSubDir($url, $expectedResult) { - \OC::$WEBROOT = '/owncloud'; - $result = \OC_Helper::makeURLAbsolute($url); - - $this->assertEquals($expectedResult, $result); - } - - public function provideDocRootURLs() { - return array( - array('index.php', 'http://localhost/index.php'), - array('/index.php', 'http://localhost/index.php'), - array('/apps/index.php', 'http://localhost/apps/index.php'), - array('apps/index.php', 'http://localhost/apps/index.php'), - ); - } - - public function provideSubDirURLs() { - return array( - array('index.php', 'http://localhost/owncloud/index.php'), - array('/index.php', 'http://localhost/owncloud/index.php'), - array('/apps/index.php', 'http://localhost/owncloud/apps/index.php'), - array('apps/index.php', 'http://localhost/owncloud/apps/index.php'), - ); - } - /** * @small * test linkToAbsolute URL construction -- cgit v1.2.3 From e42f262d85ddd891ce823dd5d9b5a4a87c8a7786 Mon Sep 17 00:00:00 2001 From: Morris Jobke Date: Fri, 18 Dec 2015 11:46:21 +0100 Subject: properly use OCP\Util instead of OC_Helper --- lib/private/helper.php | 16 ---------------- lib/private/util.php | 8 ++++---- lib/public/util.php | 5 ++++- settings/help.php | 4 ++-- tests/lib/helper.php | 46 ---------------------------------------------- 5 files changed, 10 insertions(+), 69 deletions(-) (limited to 'tests') diff --git a/lib/private/helper.php b/lib/private/helper.php index 92ae25de4cc..efbc6bda1db 100644 --- a/lib/private/helper.php +++ b/lib/private/helper.php @@ -53,22 +53,6 @@ use Symfony\Component\Process\ExecutableFinder; class OC_Helper { private static $templateManager; - /** - * Creates an absolute url - * @param string $app app - * @param string $file file - * @param array $args array with param=>value, will be appended to the returned url - * The value of $args will be urlencoded - * @return string the url - * - * Returns a absolute url to the given app and file. - */ - public static function linkToAbsolute($app, $file, $args = array()) { - return OC::$server->getURLGenerator()->getAbsoluteURL( - OC::$server->getURLGenerator()->linkTo($app, $file, $args) - ); - } - /** * Creates an url for remote use * @param string $service id diff --git a/lib/private/util.php b/lib/private/util.php index c63befb3f32..2ecacc53bd4 100644 --- a/lib/private/util.php +++ b/lib/private/util.php @@ -974,7 +974,7 @@ class OC_Util { */ public static function checkAppEnabled($app) { if (!OC_App::isEnabled($app)) { - header('Location: ' . OC_Helper::linkToAbsolute('', 'index.php')); + header('Location: ' . \OCP\Util::linkToAbsolute('', 'index.php')); exit(); } } @@ -988,7 +988,7 @@ class OC_Util { public static function checkLoggedIn() { // Check if we are a user if (!OC_User::isLoggedIn()) { - header('Location: ' . OC_Helper::linkToAbsolute('', 'index.php', + header('Location: ' . \OCP\Util::linkToAbsolute('', 'index.php', [ 'redirect_url' => \OC::$server->getRequest()->getRequestUri() ] @@ -1006,7 +1006,7 @@ class OC_Util { public static function checkAdminUser() { OC_Util::checkLoggedIn(); if (!OC_User::isAdminUser(OC_User::getUser())) { - header('Location: ' . OC_Helper::linkToAbsolute('', 'index.php')); + header('Location: ' . \OCP\Util::linkToAbsolute('', 'index.php')); exit(); } } @@ -1046,7 +1046,7 @@ class OC_Util { } if (!$isSubAdmin) { - header('Location: ' . OC_Helper::linkToAbsolute('', 'index.php')); + header('Location: ' . \OCP\Util::linkToAbsolute('', 'index.php')); exit(); } return true; diff --git a/lib/public/util.php b/lib/public/util.php index da4aa6e9deb..6f5016fa4a1 100644 --- a/lib/public/util.php +++ b/lib/public/util.php @@ -269,7 +269,10 @@ class Util { * @since 4.0.0 - parameter $args was added in 4.5.0 */ public static function linkToAbsolute( $app, $file, $args = array() ) { - return(\OC_Helper::linkToAbsolute( $app, $file, $args )); + $urlGenerator = \OC::$server->getURLGenerator(); + return $urlGenerator->getAbsoluteURL( + $urlGenerator->linkTo($app, $file, $args) + ); } /** diff --git a/settings/help.php b/settings/help.php index 21b48242706..848ce06cf49 100644 --- a/settings/help.php +++ b/settings/help.php @@ -34,11 +34,11 @@ OC_Util::addStyle( "settings", "settings" ); if(isset($_GET['mode']) and $_GET['mode'] === 'admin') { - $url=OC_Helper::linkToAbsolute( 'core', 'doc/admin/index.html' ); + $url=\OCP\Util::linkToAbsolute( 'core', 'doc/admin/index.html' ); $style1=''; $style2=' active'; }else{ - $url=OC_Helper::linkToAbsolute( 'core', 'doc/user/index.html' ); + $url=\OCP\Util::linkToAbsolute( 'core', 'doc/user/index.html' ); $style1=' active'; $style2=''; } diff --git a/tests/lib/helper.php b/tests/lib/helper.php index 794502e5e21..22737f170d8 100644 --- a/tests/lib/helper.php +++ b/tests/lib/helper.php @@ -243,52 +243,6 @@ class Test_Helper extends \Test\TestCase { // Url generator methods - /** - * @small - * test linkToAbsolute URL construction - * @dataProvider provideDocRootAppAbsoluteUrlParts - */ - public function testLinkToAbsoluteDocRoot($app, $file, $args, $expectedResult) { - \OC::$WEBROOT = ''; - $result = \OC_Helper::linkToAbsolute($app, $file, $args); - - $this->assertEquals($expectedResult, $result); - } - - /** - * @small - * test linkToAbsolute URL construction in sub directory - * @dataProvider provideSubDirAppAbsoluteUrlParts - */ - public function testLinkToAbsoluteSubDir($app, $file, $args, $expectedResult) { - \OC::$WEBROOT = '/owncloud'; - $result = \OC_Helper::linkToAbsolute($app, $file, $args); - - $this->assertEquals($expectedResult, $result); - } - - /** - * @return array - */ - public function provideDocRootAppAbsoluteUrlParts() { - return array( - array('files', 'ajax/list.php', array(), 'http://localhost/index.php/apps/files/ajax/list.php'), - array('files', 'ajax/list.php', array('trut' => 'trat', 'dut' => 'dat'), 'http://localhost/index.php/apps/files/ajax/list.php?trut=trat&dut=dat'), - array('', 'index.php', array('trut' => 'trat', 'dut' => 'dat'), 'http://localhost/index.php?trut=trat&dut=dat'), - ); - } - - /** - * @return array - */ - public function provideSubDirAppAbsoluteUrlParts() { - return array( - array('files', 'ajax/list.php', array(), 'http://localhost/owncloud/index.php/apps/files/ajax/list.php'), - array('files', 'ajax/list.php', array('trut' => 'trat', 'dut' => 'dat'), 'http://localhost/owncloud/index.php/apps/files/ajax/list.php?trut=trat&dut=dat'), - array('', 'index.php', array('trut' => 'trat', 'dut' => 'dat'), 'http://localhost/owncloud/index.php?trut=trat&dut=dat'), - ); - } - /** * @small * test linkToRemoteBase URL construction -- cgit v1.2.3 From 0a09004d39b5d27124d59ed45debf109170b24d2 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Fri, 18 Dec 2015 11:24:15 +0100 Subject: Inject Config into SystemConfig --- lib/base.php | 4 ++-- lib/private/server.php | 7 ++++--- lib/private/systemconfig.php | 17 ++++++++++++----- tests/lib/allconfig.php | 16 ++++++++++++---- tests/lib/server.php | 3 ++- 5 files changed, 32 insertions(+), 15 deletions(-) (limited to 'tests') diff --git a/lib/base.php b/lib/base.php index 887499b58a5..aee1698e222 100644 --- a/lib/base.php +++ b/lib/base.php @@ -142,7 +142,7 @@ class OC { 'SCRIPT_FILENAME' => $_SERVER['SCRIPT_FILENAME'], ], ]; - $fakeRequest = new \OC\AppFramework\Http\Request($params, null, new \OC\AllConfig(new \OC\SystemConfig())); + $fakeRequest = new \OC\AppFramework\Http\Request($params, null, new \OC\AllConfig(new \OC\SystemConfig(self::$config))); $scriptName = $fakeRequest->getScriptName(); if (substr($scriptName, -1) == '/') { $scriptName .= 'index.php'; @@ -522,7 +522,7 @@ class OC { } // setup the basic server - self::$server = new \OC\Server(\OC::$WEBROOT); + self::$server = new \OC\Server(\OC::$WEBROOT, self::$config); \OC::$server->getEventLogger()->log('autoloader', 'Autoloader', $loaderStart, $loaderEnd); \OC::$server->getEventLogger()->start('boot', 'Initialize'); diff --git a/lib/private/server.php b/lib/private/server.php index 8439500706d..3e1af0310d1 100644 --- a/lib/private/server.php +++ b/lib/private/server.php @@ -84,8 +84,9 @@ class Server extends SimpleContainer implements IServerContainer { /** * @param string $webRoot + * @param \OC\Config $config */ - public function __construct($webRoot) { + public function __construct($webRoot, \OC\Config $config) { parent::__construct(); $this->webRoot = $webRoot; @@ -238,8 +239,8 @@ class Server extends SimpleContainer implements IServerContainer { $c->getSystemConfig() ); }); - $this->registerService('SystemConfig', function ($c) { - return new \OC\SystemConfig(); + $this->registerService('SystemConfig', function ($c) use ($config) { + return new \OC\SystemConfig($config); }); $this->registerService('AppConfig', function ($c) { return new \OC\AppConfig(\OC_DB::getConnection()); diff --git a/lib/private/systemconfig.php b/lib/private/systemconfig.php index d2ceeb8272d..fb8c18123d7 100644 --- a/lib/private/systemconfig.php +++ b/lib/private/systemconfig.php @@ -44,12 +44,19 @@ class SystemConfig { 'objectstore' => ['arguments' => ['password' => true]], ]; + /** @var Config */ + private $config; + + public function __construct(Config $config) { + $this->config = $config; + } + /** * Lists all available config keys * @return array an array of key names */ public function getKeys() { - return \OC::$config->getKeys(); + return $this->config->getKeys(); } /** @@ -59,7 +66,7 @@ class SystemConfig { * @param mixed $value the value that should be stored */ public function setValue($key, $value) { - \OC::$config->setValue($key, $value); + $this->config->setValue($key, $value); } /** @@ -69,7 +76,7 @@ class SystemConfig { * If value is null, the config key will be deleted */ public function setValues(array $configs) { - \OC::$config->setValues($configs); + $this->config->setValues($configs); } /** @@ -80,7 +87,7 @@ class SystemConfig { * @return mixed the value or $default */ public function getValue($key, $default = '') { - return \OC::$config->getValue($key, $default); + return $this->config->getValue($key, $default); } /** @@ -106,7 +113,7 @@ class SystemConfig { * @param string $key the key of the value, under which it was saved */ public function deleteValue($key) { - \OC::$config->deleteKey($key); + $this->config->deleteKey($key); } /** diff --git a/tests/lib/allconfig.php b/tests/lib/allconfig.php index ca3dce12eaf..0caf8163cfc 100644 --- a/tests/lib/allconfig.php +++ b/tests/lib/allconfig.php @@ -28,7 +28,9 @@ class TestAllConfig extends \Test\TestCase { $connection = $this->connection; } if($systemConfig === null) { - $systemConfig = $this->getMock('\OC\SystemConfig'); + $systemConfig = $this->getMockBuilder('\OC\SystemConfig') + ->disableOriginalConstructor() + ->getMock(); } return new \OC\AllConfig($systemConfig, $connection); } @@ -89,7 +91,9 @@ class TestAllConfig extends \Test\TestCase { public function testSetUserValueWithPreCondition() { // mock the check for the database to run the correct SQL statements for each database type - $systemConfig = $this->getMock('\OC\SystemConfig'); + $systemConfig = $this->getMockBuilder('\OC\SystemConfig') + ->disableOriginalConstructor() + ->getMock(); $systemConfig->expects($this->once()) ->method('getValue') ->with($this->equalTo('dbtype'), @@ -133,7 +137,9 @@ class TestAllConfig extends \Test\TestCase { */ public function testSetUserValueWithPreConditionFailure() { // mock the check for the database to run the correct SQL statements for each database type - $systemConfig = $this->getMock('\OC\SystemConfig'); + $systemConfig = $this->getMockBuilder('\OC\SystemConfig') + ->disableOriginalConstructor() + ->getMock(); $systemConfig->expects($this->once()) ->method('getValue') ->with($this->equalTo('dbtype'), @@ -394,7 +400,9 @@ class TestAllConfig extends \Test\TestCase { public function testGetUsersForUserValue() { // mock the check for the database to run the correct SQL statements for each database type - $systemConfig = $this->getMock('\OC\SystemConfig'); + $systemConfig = $this->getMockBuilder('\OC\SystemConfig') + ->disableOriginalConstructor() + ->getMock(); $systemConfig->expects($this->once()) ->method('getValue') ->with($this->equalTo('dbtype'), diff --git a/tests/lib/server.php b/tests/lib/server.php index 6b569e77dd9..e2670061e8d 100644 --- a/tests/lib/server.php +++ b/tests/lib/server.php @@ -38,7 +38,8 @@ class Server extends \Test\TestCase { public function setUp() { parent::setUp(); - $this->server = new \OC\Server(''); + $config = new \OC\Config(\OC::$configDir); + $this->server = new \OC\Server('', $config); } public function dataTestQuery() { -- cgit v1.2.3 From 450e2f3bd308388e5df01b2b2ea38e6bf16b1c67 Mon Sep 17 00:00:00 2001 From: Morris Jobke Date: Fri, 18 Dec 2015 12:00:18 +0100 Subject: Move OC_Helper code to OCP\Util for linkToRemote --- lib/private/helper.php | 26 -------------------------- lib/public/util.php | 6 +++++- tests/lib/helper.php | 32 -------------------------------- 3 files changed, 5 insertions(+), 59 deletions(-) (limited to 'tests') diff --git a/lib/private/helper.php b/lib/private/helper.php index efbc6bda1db..bd69c13de50 100644 --- a/lib/private/helper.php +++ b/lib/private/helper.php @@ -53,32 +53,6 @@ use Symfony\Component\Process\ExecutableFinder; class OC_Helper { private static $templateManager; - /** - * Creates an url for remote use - * @param string $service id - * @return string the url - * - * Returns a url to the given service. - */ - public static function linkToRemoteBase($service) { - return OC::$server->getURLGenerator()->linkTo('', 'remote.php') . '/' . $service; - } - - /** - * Creates an absolute url for remote use - * @param string $service id - * @param bool $add_slash - * @return string the url - * - * Returns a absolute url to the given service. - */ - public static function linkToRemote($service, $add_slash = true) { - return OC::$server->getURLGenerator()->getAbsoluteURL( - self::linkToRemoteBase($service) - . (($add_slash && $service[strlen($service) - 1] != '/') ? '/' : '') - ); - } - /** * Creates an absolute url for public use * @param string $service id diff --git a/lib/public/util.php b/lib/public/util.php index 6f5016fa4a1..a9fe0e47de6 100644 --- a/lib/public/util.php +++ b/lib/public/util.php @@ -282,7 +282,11 @@ class Util { * @since 4.0.0 */ public static function linkToRemote( $service ) { - return(\OC_Helper::linkToRemote( $service )); + $urlGenerator = \OC::$server->getURLGenerator(); + $remoteBase = $urlGenerator->linkTo('', 'remote.php') . '/' . $service; + return $urlGenerator->getAbsoluteURL( + $remoteBase . (($service[strlen($service) - 1] != '/') ? '/' : '') + ); } /** diff --git a/tests/lib/helper.php b/tests/lib/helper.php index 22737f170d8..468d32bc37a 100644 --- a/tests/lib/helper.php +++ b/tests/lib/helper.php @@ -243,38 +243,6 @@ class Test_Helper extends \Test\TestCase { // Url generator methods - /** - * @small - * test linkToRemoteBase URL construction - */ - public function testLinkToRemoteBase() { - \OC::$WEBROOT = ''; - $result = \OC_Helper::linkToRemoteBase('webdav'); - $this->assertEquals('/remote.php/webdav', $result); - - \OC::$WEBROOT = '/owncloud'; - $result = \OC_Helper::linkToRemoteBase('webdav'); - $this->assertEquals('/owncloud/remote.php/webdav', $result); - } - - /** - * @small - * test linkToRemote URL construction - */ - public function testLinkToRemote() { - \OC::$WEBROOT = ''; - $result = \OC_Helper::linkToRemote('webdav'); - $this->assertEquals('http://localhost/remote.php/webdav/', $result); - $result = \OC_Helper::linkToRemote('webdav', false); - $this->assertEquals('http://localhost/remote.php/webdav', $result); - - \OC::$WEBROOT = '/owncloud'; - $result = \OC_Helper::linkToRemote('webdav'); - $this->assertEquals('http://localhost/owncloud/remote.php/webdav/', $result); - $result = \OC_Helper::linkToRemote('webdav', false); - $this->assertEquals('http://localhost/owncloud/remote.php/webdav', $result); - } - /** * @small * test linkToPublic URL construction -- cgit v1.2.3 From 4f20e3bac0dec72ede10f947ee3d1125071d4564 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Fri, 18 Dec 2015 13:42:59 +0100 Subject: Removed deprecated OC_Helper::getSecureMimeType --- apps/dav/lib/connector/sabre/file.php | 2 +- apps/files/download.php | 2 +- apps/files_versions/download.php | 2 +- lib/private/helper.php | 11 ----------- tests/lib/helper.php | 12 ------------ 5 files changed, 3 insertions(+), 26 deletions(-) (limited to 'tests') diff --git a/apps/dav/lib/connector/sabre/file.php b/apps/dav/lib/connector/sabre/file.php index c66f627c0a3..6a0a39d04e7 100644 --- a/apps/dav/lib/connector/sabre/file.php +++ b/apps/dav/lib/connector/sabre/file.php @@ -329,7 +329,7 @@ class File extends Node implements IFile { if (isset($_SERVER['REQUEST_METHOD']) && $_SERVER['REQUEST_METHOD'] === 'PROPFIND') { return $mimeType; } - return \OC_Helper::getSecureMimeType($mimeType); + return \OC::$server->getMimeTypeDetector()->getSecureMimeType($mimeType); } /** diff --git a/apps/files/download.php b/apps/files/download.php index b0628e394be..b058f0ebf5b 100644 --- a/apps/files/download.php +++ b/apps/files/download.php @@ -39,7 +39,7 @@ if(!\OC\Files\Filesystem::file_exists($filename)) { exit; } -$ftype=\OC_Helper::getSecureMimeType(\OC\Files\Filesystem::getMimeType( $filename )); +$ftype=\OC::$server->getMimeTypeDetector()->getSecureMimeType(\OC\Files\Filesystem::getMimeType( $filename )); header('Content-Type:'.$ftype); OCP\Response::setContentDispositionHeader(basename($filename), 'attachment'); diff --git a/apps/files_versions/download.php b/apps/files_versions/download.php index 22a218f472a..d3c38f3d4e1 100644 --- a/apps/files_versions/download.php +++ b/apps/files_versions/download.php @@ -35,7 +35,7 @@ $versionName = '/'.$uid.'/files_versions/'.$filename.'.v'.$revision; $view = new OC\Files\View('/'); -$ftype = \OC_Helper::getSecureMimeType($view->getMimeType('/'.$uid.'/files/'.$filename)); +$ftype = \OC::$server->getMimeTypeDetector()->getSecureMimeType($view->getMimeType('/'.$uid.'/files/'.$filename)); header('Content-Type:'.$ftype); OCP\Response::setContentDispositionHeader(basename($filename), 'attachment'); diff --git a/lib/private/helper.php b/lib/private/helper.php index c6223d2147a..29ecd85388f 100644 --- a/lib/private/helper.php +++ b/lib/private/helper.php @@ -346,17 +346,6 @@ class OC_Helper { return \OC::$server->getMimeTypeDetector()->detectPath($path); } - /** - * Get a secure mimetype that won't expose potential XSS. - * - * @param string $mimeType - * @return string - * @deprecated 8.2.0 Use \OC::$server->getMimeTypeDetector()->getSecureMimeType($mimeType) - */ - static function getSecureMimeType($mimeType) { - return \OC::$server->getMimeTypeDetector()->getSecureMimeType($mimeType); - } - /** * get the mimetype form a data string * diff --git a/tests/lib/helper.php b/tests/lib/helper.php index 9ad3af3d60c..576209df06b 100644 --- a/tests/lib/helper.php +++ b/tests/lib/helper.php @@ -71,18 +71,6 @@ class Test_Helper extends \Test\TestCase { ]; } - function testGetSecureMimeType() { - $dir=OC::$SERVERROOT.'/tests/data'; - - $result = OC_Helper::getSecureMimeType('image/svg+xml'); - $expected = 'text/plain'; - $this->assertEquals($result, $expected); - - $result = OC_Helper::getSecureMimeType('image/png'); - $expected = 'image/png'; - $this->assertEquals($result, $expected); - } - function testGetStringMimeType() { if (\OC_Util::runningOnWindows()) { $this->markTestSkipped('[Windows] Strings have mimetype application/octet-stream on Windows'); -- cgit v1.2.3 From a54be132fca86ceb6d6bb87368f55937f41e94a2 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Fri, 18 Dec 2015 13:43:44 +0100 Subject: Removed deprecated unsused function OC_Helper::getStringMimeType --- lib/private/helper.php | 11 ----------- tests/lib/helper.php | 10 ---------- 2 files changed, 21 deletions(-) (limited to 'tests') diff --git a/lib/private/helper.php b/lib/private/helper.php index 29ecd85388f..f329d53fa76 100644 --- a/lib/private/helper.php +++ b/lib/private/helper.php @@ -346,17 +346,6 @@ class OC_Helper { return \OC::$server->getMimeTypeDetector()->detectPath($path); } - /** - * get the mimetype form a data string - * - * @param string $data - * @return string - * @deprecated 8.2.0 Use \OC::$server->getMimeTypeDetector()->detectString($data) - */ - static function getStringMimeType($data) { - return \OC::$server->getMimeTypeDetector()->detectString($data); - } - /** * detect if a given program is found in the search PATH * diff --git a/tests/lib/helper.php b/tests/lib/helper.php index 576209df06b..114354c7936 100644 --- a/tests/lib/helper.php +++ b/tests/lib/helper.php @@ -71,16 +71,6 @@ class Test_Helper extends \Test\TestCase { ]; } - function testGetStringMimeType() { - if (\OC_Util::runningOnWindows()) { - $this->markTestSkipped('[Windows] Strings have mimetype application/octet-stream on Windows'); - } - - $result = OC_Helper::getStringMimeType("/data/data.tar.gz"); - $expected = 'text/plain; charset=us-ascii'; - $this->assertEquals($result, $expected); - } - function testIsSubDirectory() { $result = OC_Helper::isSubDirectory("./data/", "/anotherDirectory/"); $this->assertFalse($result); -- cgit v1.2.3 From 54558bb5b296740233036601d2b3eb7f87ee48c4 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 18 Dec 2015 15:05:32 +0100 Subject: Fix the test to expect the new behaviour --- tests/lib/backgroundjob/job.php | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) (limited to 'tests') diff --git a/tests/lib/backgroundjob/job.php b/tests/lib/backgroundjob/job.php index fec9b0a792d..912e0e13b57 100644 --- a/tests/lib/backgroundjob/job.php +++ b/tests/lib/backgroundjob/job.php @@ -23,10 +23,17 @@ class Job extends \Test\TestCase { }); $jobList->add($job); + $logger = $this->getMockBuilder('OCP\ILogger') + ->disableOriginalConstructor() + ->getMock(); + $logger->expects($this->once()) + ->method('error') + ->with('Error while running background job: '); + $this->assertCount(1, $jobList->getAll()); - $job->execute($jobList); + $job->execute($jobList, $logger); $this->assertTrue($this->run); - $this->assertCount(0, $jobList->getAll()); + $this->assertCount(1, $jobList->getAll()); } public function markRun() { -- cgit v1.2.3 From ed98cdf532ae475f4d5f5ec88afa55972cba3ba2 Mon Sep 17 00:00:00 2001 From: Morris Jobke Date: Fri, 18 Dec 2015 15:26:54 +0100 Subject: Use OCP\Util::getVersion instead of the internal private implementation --- core/command/status.php | 2 +- core/js/config.php | 2 +- lib/base.php | 4 ++-- lib/private/app.php | 12 ++++++------ lib/private/app/platform.php | 2 +- lib/private/defaults.php | 2 +- lib/private/installer.php | 8 ++++---- lib/private/ocs/cloud.php | 2 +- lib/private/setup.php | 2 +- lib/private/templatelayout.php | 2 +- lib/private/updater.php | 10 +++++----- lib/private/util.php | 2 +- settings/controller/appsettingscontroller.php | 8 +++++--- status.php | 2 +- tests/lib/updater.php | 2 +- tests/lib/util.php | 2 +- tests/lib/utilcheckserver.php | 8 ++++---- 17 files changed, 37 insertions(+), 35 deletions(-) (limited to 'tests') diff --git a/core/command/status.php b/core/command/status.php index 2eb58525d3e..c2a28ff822f 100644 --- a/core/command/status.php +++ b/core/command/status.php @@ -39,7 +39,7 @@ class Status extends Base { protected function execute(InputInterface $input, OutputInterface $output) { $values = array( 'installed' => (bool) \OC::$server->getConfig()->getSystemValue('installed', false), - 'version' => implode('.', \OC_Util::getVersion()), + 'version' => implode('.', \OCP\Util::getVersion()), 'versionstring' => \OC_Util::getVersionString(), 'edition' => \OC_Util::getEditionString(), ); diff --git a/core/js/config.php b/core/js/config.php index e51ae903729..c975c6db2dc 100644 --- a/core/js/config.php +++ b/core/js/config.php @@ -138,7 +138,7 @@ $array = array( array( 'session_lifetime' => min(\OCP\Config::getSystemValue('session_lifetime', OC::$server->getIniWrapper()->getNumeric('session.gc_maxlifetime')), OC::$server->getIniWrapper()->getNumeric('session.gc_maxlifetime')), 'session_keepalive' => \OCP\Config::getSystemValue('session_keepalive', true), - 'version' => implode('.', OC_Util::getVersion()), + 'version' => implode('.', \OCP\Util::getVersion()), 'versionstring' => OC_Util::getVersionString(), 'enable_avatars' => \OC::$server->getConfig()->getSystemValue('enable_avatars', true), 'lost_password_link'=> \OC::$server->getConfig()->getSystemValue('lost_password_link', null), diff --git a/lib/base.php b/lib/base.php index c0db0454f6a..ce4546e8fa3 100644 --- a/lib/base.php +++ b/lib/base.php @@ -377,7 +377,7 @@ class OC { // check whether this is a core update or apps update $installedVersion = $systemConfig->getValue('version', '0.0.0'); - $currentVersion = implode('.', OC_Util::getVersion()); + $currentVersion = implode('.', \OCP\Util::getVersion()); $appManager = \OC::$server->getAppManager(); @@ -392,7 +392,7 @@ class OC { } // get third party apps - $ocVersion = OC_Util::getVersion(); + $ocVersion = \OCP\Util::getVersion(); $tmpl->assign('appsToUpgrade', $appManager->getAppsNeedingUpgrade($ocVersion)); $tmpl->assign('incompatibleAppsList', $appManager->getIncompatibleApps($ocVersion)); $tmpl->assign('productName', 'ownCloud'); // for now diff --git a/lib/private/app.php b/lib/private/app.php index ff711e82424..5f6ca9596c8 100644 --- a/lib/private/app.php +++ b/lib/private/app.php @@ -318,8 +318,8 @@ class OC_App { \OC::$server->getConfig(), \OC::$server->getLogger() ); - $appData = $ocsClient->getApplication($app, \OC_Util::getVersion()); - $download= $ocsClient->getApplicationDownload($app, \OC_Util::getVersion()); + $appData = $ocsClient->getApplication($app, \OCP\Util::getVersion()); + $download= $ocsClient->getApplicationDownload($app, \OCP\Util::getVersion()); if(isset($download['downloadlink']) and $download['downloadlink']!='') { // Replace spaces in download link without encoding entire URL $download['downloadlink'] = str_replace(' ', '%20', $download['downloadlink']); @@ -880,7 +880,7 @@ class OC_App { if (is_null($category)) { - $categoryNames = $ocsClient->getCategories(\OC_Util::getVersion()); + $categoryNames = $ocsClient->getCategories(\OCP\Util::getVersion()); if (is_array($categoryNames)) { // Check that categories of apps were retrieved correctly if (!$categories = array_keys($categoryNames)) { @@ -892,7 +892,7 @@ class OC_App { } $page = 0; - $remoteApps = $ocsClient->getApplications($categories, $page, $filter, \OC_Util::getVersion()); + $remoteApps = $ocsClient->getApplications($categories, $page, $filter, \OCP\Util::getVersion()); $apps = []; $i = 0; $l = \OC::$server->getL10N('core'); @@ -1050,7 +1050,7 @@ class OC_App { $config, \OC::$server->getLogger() ); - $appData = $ocsClient->getApplication($app, \OC_Util::getVersion()); + $appData = $ocsClient->getApplication($app, \OCP\Util::getVersion()); // check if app is a shipped app or not. OCS apps have an integer as id, shipped apps use a string if (!is_numeric($app)) { @@ -1080,7 +1080,7 @@ class OC_App { if ($app !== false) { // check if the app is compatible with this version of ownCloud $info = self::getAppInfo($app); - $version = OC_Util::getVersion(); + $version = \OCP\Util::getVersion(); if (!self::isAppCompatible($version, $info)) { throw new \Exception( $l->t('App "%s" cannot be installed because it is not compatible with this version of ownCloud.', diff --git a/lib/private/app/platform.php b/lib/private/app/platform.php index f433ecd9f9e..c16f050e13c 100644 --- a/lib/private/app/platform.php +++ b/lib/private/app/platform.php @@ -52,7 +52,7 @@ class Platform { * @return string */ public function getOcVersion() { - $v = OC_Util::getVersion(); + $v = \OCP\Util::getVersion(); return join('.', $v); } diff --git a/lib/private/defaults.php b/lib/private/defaults.php index 16f45943f54..23f0baad96e 100644 --- a/lib/private/defaults.php +++ b/lib/private/defaults.php @@ -49,7 +49,7 @@ class OC_Defaults { function __construct() { $this->l = \OC::$server->getL10N('lib'); - $version = OC_Util::getVersion(); + $version = \OCP\Util::getVersion(); $this->defaultEntity = 'ownCloud'; /* e.g. company name, used for footers and copyright notices */ $this->defaultName = 'ownCloud'; /* short name, used when referring to the software */ diff --git a/lib/private/installer.php b/lib/private/installer.php index e1447db0db5..bbd976cda91 100644 --- a/lib/private/installer.php +++ b/lib/private/installer.php @@ -232,8 +232,8 @@ class OC_Installer{ \OC::$server->getConfig(), \OC::$server->getLogger() ); - $appData = $ocsClient->getApplication($ocsId, \OC_Util::getVersion()); - $download = $ocsClient->getApplicationDownload($ocsId, \OC_Util::getVersion()); + $appData = $ocsClient->getApplication($ocsId, \OCP\Util::getVersion()); + $download = $ocsClient->getApplicationDownload($ocsId, \OCP\Util::getVersion()); if (isset($download['downloadlink']) && trim($download['downloadlink']) !== '') { $download['downloadlink'] = str_replace(' ', '%20', $download['downloadlink']); @@ -342,7 +342,7 @@ class OC_Installer{ } // check if the app is compatible with this version of ownCloud - if(!OC_App::isAppCompatible(OC_Util::getVersion(), $info)) { + if(!OC_App::isAppCompatible(\OCP\Util::getVersion(), $info)) { OC_Helper::rmdirr($extractDir); throw new \Exception($l->t("App can't be installed because it is not compatible with this version of ownCloud")); } @@ -400,7 +400,7 @@ class OC_Installer{ \OC::$server->getConfig(), \OC::$server->getLogger() ); - $ocsdata = $ocsClient->getApplication($ocsid, \OC_Util::getVersion()); + $ocsdata = $ocsClient->getApplication($ocsid, \OCP\Util::getVersion()); $ocsversion= (string) $ocsdata['version']; $currentversion=OC_App::getAppVersion($app); if (version_compare($ocsversion, $currentversion, '>')) { diff --git a/lib/private/ocs/cloud.php b/lib/private/ocs/cloud.php index 2cf40c449ff..1b04f43d477 100644 --- a/lib/private/ocs/cloud.php +++ b/lib/private/ocs/cloud.php @@ -26,7 +26,7 @@ class OC_OCS_Cloud { public static function getCapabilities() { $result = array(); - list($major, $minor, $micro) = OC_Util::getVersion(); + list($major, $minor, $micro) = \OCP\Util::getVersion(); $result['version'] = array( 'major' => $major, 'minor' => $minor, diff --git a/lib/private/setup.php b/lib/private/setup.php index 4d11cb44a83..770f5cdab52 100644 --- a/lib/private/setup.php +++ b/lib/private/setup.php @@ -322,7 +322,7 @@ class Setup { 'datadirectory' => $dataDir, 'overwrite.cli.url' => $request->getServerProtocol() . '://' . $request->getInsecureServerHost() . \OC::$WEBROOT, 'dbtype' => $dbType, - 'version' => implode('.', \OC_Util::getVersion()), + 'version' => implode('.', \OCP\Util::getVersion()), ]); try { diff --git a/lib/private/templatelayout.php b/lib/private/templatelayout.php index 95688268c47..bf25b2d31a9 100644 --- a/lib/private/templatelayout.php +++ b/lib/private/templatelayout.php @@ -159,7 +159,7 @@ class OC_TemplateLayout extends OC_Template { if(empty(self::$versionHash)) { $v = OC_App::getAppVersions(); - $v['core'] = implode('.', \OC_Util::getVersion()); + $v['core'] = implode('.', \OCP\Util::getVersion()); self::$versionHash = md5(implode(',', $v)); } diff --git a/lib/private/updater.php b/lib/private/updater.php index d5598d339d3..04f8dcf7226 100644 --- a/lib/private/updater.php +++ b/lib/private/updater.php @@ -155,7 +155,7 @@ class Updater extends BasicEmitter { $this->config->setAppValue('core', 'installedat', microtime(true)); } - $version = \OC_Util::getVersion(); + $version = \OCP\Util::getVersion(); $version['installed'] = $this->config->getAppValue('core', 'installedat'); $version['updated'] = $this->config->getAppValue('core', 'lastupdatedat'); $version['updatechannel'] = \OC_Util::getChannel(); @@ -208,7 +208,7 @@ class Updater extends BasicEmitter { } $installedVersion = $this->config->getSystemValue('version', '0.0.0'); - $currentVersion = implode('.', \OC_Util::getVersion()); + $currentVersion = implode('.', \OCP\Util::getVersion()); $this->log->debug('starting upgrade from ' . $installedVersion . ' to ' . $currentVersion, array('app' => 'core')); $success = true; @@ -353,7 +353,7 @@ class Updater extends BasicEmitter { } // only set the final version if everything went well - $this->config->setSystemValue('version', implode('.', \OC_Util::getVersion())); + $this->config->setSystemValue('version', implode('.', \OCP\Util::getVersion())); } } @@ -472,7 +472,7 @@ class Updater extends BasicEmitter { private function checkAppsRequirements() { $isCoreUpgrade = $this->isCodeUpgrade(); $apps = OC_App::getEnabledApps(); - $version = OC_Util::getVersion(); + $version = \OCP\Util::getVersion(); $disabledApps = []; foreach ($apps as $app) { // check if the app is compatible with this version of ownCloud @@ -509,7 +509,7 @@ class Updater extends BasicEmitter { */ private function isCodeUpgrade() { $installedVersion = $this->config->getSystemValue('version', '0.0.0'); - $currentVersion = implode('.', OC_Util::getVersion()); + $currentVersion = implode('.', \OCP\Util::getVersion()); if (version_compare($currentVersion, $installedVersion, '>')) { return true; } diff --git a/lib/private/util.php b/lib/private/util.php index 0e029d74b7d..12146f6980b 100644 --- a/lib/private/util.php +++ b/lib/private/util.php @@ -1509,7 +1509,7 @@ class OC_Util { public static function needUpgrade(\OCP\IConfig $config) { if ($config->getSystemValue('installed', false)) { $installedVersion = $config->getSystemValue('version', '0.0.0'); - $currentVersion = implode('.', OC_Util::getVersion()); + $currentVersion = implode('.', \OCP\Util::getVersion()); $versionDiff = version_compare($currentVersion, $installedVersion); if ($versionDiff > 0) { return true; diff --git a/settings/controller/appsettingscontroller.php b/settings/controller/appsettingscontroller.php index d0e465bfa9c..d2430f9b559 100644 --- a/settings/controller/appsettingscontroller.php +++ b/settings/controller/appsettingscontroller.php @@ -159,7 +159,7 @@ class AppSettingsController extends Controller { if($this->ocsClient->isAppStoreEnabled()) { // apps from external repo via OCS - $ocs = $this->ocsClient->getCategories(\OC_Util::getVersion()); + $ocs = $this->ocsClient->getCategories(\OCP\Util::getVersion()); if ($ocs) { foreach($ocs as $k => $v) { $name = str_replace('ownCloud ', '', $v); @@ -205,9 +205,10 @@ class AppSettingsController extends Controller { } return ($a < $b) ? -1 : 1; }); + $version = \OCP\Util::getVersion(); foreach($apps as $key => $app) { if(!array_key_exists('level', $app) && array_key_exists('ocsid', $app)) { - $remoteAppEntry = $this->ocsClient->getApplication($app['ocsid'], \OC_Util::getVersion()); + $remoteAppEntry = $this->ocsClient->getApplication($app['ocsid'], $version); if(is_array($remoteAppEntry) && array_key_exists('level', $remoteAppEntry)) { $apps[$key]['level'] = $remoteAppEntry['level']; @@ -221,9 +222,10 @@ class AppSettingsController extends Controller { $apps = array_filter($apps, function ($app) { return !$app['active']; }); + $version = \OCP\Util::getVersion(); foreach($apps as $key => $app) { if(!array_key_exists('level', $app) && array_key_exists('ocsid', $app)) { - $remoteAppEntry = $this->ocsClient->getApplication($app['ocsid'], \OC_Util::getVersion()); + $remoteAppEntry = $this->ocsClient->getApplication($app['ocsid'], $version); if(is_array($remoteAppEntry) && array_key_exists('level', $remoteAppEntry)) { $apps[$key]['level'] = $remoteAppEntry['level']; diff --git a/status.php b/status.php index f59dc16b6a2..65418336e26 100644 --- a/status.php +++ b/status.php @@ -36,7 +36,7 @@ try { $values=array( 'installed'=>$installed, 'maintenance' => $maintenance, - 'version'=>implode('.', OC_Util::getVersion()), + 'version'=>implode('.', \OCP\Util::getVersion()), 'versionstring'=>OC_Util::getVersionString(), 'edition'=>OC_Util::getEditionString()); if (OC::$CLI) { diff --git a/tests/lib/updater.php b/tests/lib/updater.php index 14ae3db3276..313ffb65738 100644 --- a/tests/lib/updater.php +++ b/tests/lib/updater.php @@ -66,7 +66,7 @@ class UpdaterTest extends \Test\TestCase { * @return string */ private function buildUpdateUrl($baseUrl) { - return $baseUrl . '?version='.implode('x', \OC_Util::getVersion()).'xinstalledatxlastupdatedatx'.\OC_Util::getChannel().'x'.\OC_Util::getEditionString().'x'; + return $baseUrl . '?version='.implode('x', \OCP\Util::getVersion()).'xinstalledatxlastupdatedatx'.\OC_Util::getChannel().'x'.\OC_Util::getEditionString().'x'; } /** diff --git a/tests/lib/util.php b/tests/lib/util.php index fa559c17c80..cb5d28b48a7 100644 --- a/tests/lib/util.php +++ b/tests/lib/util.php @@ -8,7 +8,7 @@ */ class Test_Util extends \Test\TestCase { public function testGetVersion() { - $version = \OC_Util::getVersion(); + $version = \OCP\Util::getVersion(); $this->assertTrue(is_array($version)); foreach ($version as $num) { $this->assertTrue(is_int($num)); diff --git a/tests/lib/utilcheckserver.php b/tests/lib/utilcheckserver.php index 64757d0acd9..94e7fd2f779 100644 --- a/tests/lib/utilcheckserver.php +++ b/tests/lib/utilcheckserver.php @@ -123,7 +123,7 @@ class Test_Util_CheckServer extends \Test\TestCase { $result = \OC_Util::checkServer($this->getConfig(array( 'installed' => true, - 'version' => implode('.', OC_Util::getVersion()) + 'version' => implode('.', \OCP\Util::getVersion()) ))); $this->assertCount(1, $result); } @@ -134,7 +134,7 @@ class Test_Util_CheckServer extends \Test\TestCase { public function testDataDirWritable() { $result = \OC_Util::checkServer($this->getConfig(array( 'installed' => true, - 'version' => implode('.', OC_Util::getVersion()) + 'version' => implode('.', \OCP\Util::getVersion()) ))); $this->assertEmpty($result); } @@ -150,7 +150,7 @@ class Test_Util_CheckServer extends \Test\TestCase { chmod($this->datadir, 0300); $result = \OC_Util::checkServer($this->getConfig(array( 'installed' => true, - 'version' => implode('.', OC_Util::getVersion()) + 'version' => implode('.', \OCP\Util::getVersion()) ))); $this->assertCount(1, $result); } @@ -162,7 +162,7 @@ class Test_Util_CheckServer extends \Test\TestCase { chmod($this->datadir, 0300); $result = \OC_Util::checkServer($this->getConfig(array( 'installed' => false, - 'version' => implode('.', OC_Util::getVersion()) + 'version' => implode('.', \OCP\Util::getVersion()) ))); chmod($this->datadir, 0700); //needed for cleanup $this->assertEmpty($result); -- cgit v1.2.3 From d7169ffdec2a744683b9fb613dd08d560ed9842f Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Mon, 21 Dec 2015 18:08:08 +0100 Subject: Restore DB connection after failure In case of failure, PHPUnit seems to skip `tearDown`, so any useful assertion messages cannot be shown because `tearDownAfterClass` is throwing an error because of database usage. This commit makes sure we also restore the database in `tearDownAfterClass` to prevent the data root restoration to fail --- tests/lib/testcase.php | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) (limited to 'tests') diff --git a/tests/lib/testcase.php b/tests/lib/testcase.php index 1ee0c85b98a..93b354863a9 100644 --- a/tests/lib/testcase.php +++ b/tests/lib/testcase.php @@ -33,7 +33,8 @@ abstract class TestCase extends \PHPUnit_Framework_TestCase { private $commandBus; /** @var IDBConnection */ - static private $realDatabase; + static protected $realDatabase = null; + static private $wasDatabaseAllowed = false; protected function getTestTraits() { $traits = []; @@ -52,7 +53,9 @@ abstract class TestCase extends \PHPUnit_Framework_TestCase { protected function setUp() { // detect database access + self::$wasDatabaseAllowed = true; if (!$this->IsDatabaseAccessAllowed()) { + self::$wasDatabaseAllowed = false; if (is_null(self::$realDatabase)) { self::$realDatabase = \OC::$server->getDatabaseConnection(); } @@ -155,8 +158,15 @@ abstract class TestCase extends \PHPUnit_Framework_TestCase { } public static function tearDownAfterClass() { + if (!self::$wasDatabaseAllowed && self::$realDatabase !== null) { + // in case an error is thrown in a test, PHPUnit jumps straight to tearDownAfterClass, + // so we need the database again + \OC::$server->registerService('DatabaseConnection', function () { + return self::$realDatabase; + }); + } $dataDir = \OC::$server->getConfig()->getSystemValue('datadirectory', \OC::$SERVERROOT . '/data-autotest'); - if (\OC::$server->getDatabaseConnection()) { + if (self::$wasDatabaseAllowed && \OC::$server->getDatabaseConnection()) { $queryBuilder = \OC::$server->getDatabaseConnection()->getQueryBuilder(); self::tearDownAfterClassCleanShares($queryBuilder); -- cgit v1.2.3 From 23c754aed3bfa5c795a03e63183158d028a6a408 Mon Sep 17 00:00:00 2001 From: Bernhard Posselt Date: Thu, 2 Jul 2015 11:54:17 +0200 Subject: prefer scalar type hints over phpdoc annotation use method exists lookup to be safe and not break on old hhvm versions add test that checks if type hint is preferred over annotation --- .../utility/controllermethodreflector.php | 22 ++++++++++++---------- .../utility/ControllerMethodReflectorTest.php | 19 +++++++++++++++++++ 2 files changed, 31 insertions(+), 10 deletions(-) (limited to 'tests') diff --git a/lib/private/appframework/utility/controllermethodreflector.php b/lib/private/appframework/utility/controllermethodreflector.php index 63cf5ac24f0..1118332f930 100644 --- a/lib/private/appframework/utility/controllermethodreflector.php +++ b/lib/private/appframework/utility/controllermethodreflector.php @@ -60,16 +60,18 @@ class ControllerMethodReflector implements IControllerMethodReflector{ // extract type parameter information preg_match_all('/@param\h+(?P\w+)\h+\$(?P\w+)/', $docs, $matches); - // this is just a fix for PHP 5.3 (array_combine raises warning if called with - // two empty arrays - if($matches['var'] === array() && $matches['type'] === array()) { - $this->types = array(); - } else { - $this->types = array_combine($matches['var'], $matches['type']); - } + $this->types = array_combine($matches['var'], $matches['type']); - // get method parameters foreach ($reflection->getParameters() as $param) { + // extract type information from PHP 7 scalar types and prefer them + // over phpdoc annotations + if (method_exists($param, 'getType')) { + $type = $param->getType(); + if ($type !== null) { + $this->types[$param->getName()] = (string) $type; + } + } + if($param->isOptional()) { $default = $param->getDefaultValue(); } else { @@ -82,9 +84,9 @@ class ControllerMethodReflector implements IControllerMethodReflector{ /** * Inspects the PHPDoc parameters for types - * @param string $parameter the parameter whose type comments should be + * @param string $parameter the parameter whose type comments should be * parsed - * @return string|null type in the type parameters (@param int $something) + * @return string|null type in the type parameters (@param int $something) * would return int or null if not existing */ public function getType($parameter) { diff --git a/tests/lib/appframework/utility/ControllerMethodReflectorTest.php b/tests/lib/appframework/utility/ControllerMethodReflectorTest.php index a584b5481ba..38bd537e073 100644 --- a/tests/lib/appframework/utility/ControllerMethodReflectorTest.php +++ b/tests/lib/appframework/utility/ControllerMethodReflectorTest.php @@ -104,6 +104,25 @@ class ControllerMethodReflectorTest extends \Test\TestCase { $this->assertEquals('int', $reader->getType('test')); } + /** + * @Annotation + * @param int $a + * @param int $b + * @requires PHP 7 + */ + public function testReadTypeIntAnnotationsScalarTypes($a, float $b, int $c, $d){ + $reader = new ControllerMethodReflector(); + $reader->reflect( + '\OC\AppFramework\Utility\ControllerMethodReflectorTest', + 'testReadTypeIntAnnotationsScalarTypes' + ); + + $this->assertEquals('int', $reader->getType('a')); + $this->assertEquals('float', $reader->getType('b')); + $this->assertEquals('int', $reader->getType('c')); + $this->assertNull($reader->getType('d')); + } + /** * @Annotation -- cgit v1.2.3 From b2df7b6b8ae420766f1ce0a7b22e153f9ee1b23e Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Wed, 23 Dec 2015 14:12:37 +0100 Subject: Fix unit test --- tests/lib/appframework/utility/ControllerMethodReflectorTest.php | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'tests') diff --git a/tests/lib/appframework/utility/ControllerMethodReflectorTest.php b/tests/lib/appframework/utility/ControllerMethodReflectorTest.php index 38bd537e073..c643c362a9c 100644 --- a/tests/lib/appframework/utility/ControllerMethodReflectorTest.php +++ b/tests/lib/appframework/utility/ControllerMethodReflectorTest.php @@ -108,13 +108,17 @@ class ControllerMethodReflectorTest extends \Test\TestCase { * @Annotation * @param int $a * @param int $b + */ + public function arguments3($a, float $b, int $c, $d){} + + /** * @requires PHP 7 */ - public function testReadTypeIntAnnotationsScalarTypes($a, float $b, int $c, $d){ + public function testReadTypeIntAnnotationsScalarTypes(){ $reader = new ControllerMethodReflector(); $reader->reflect( '\OC\AppFramework\Utility\ControllerMethodReflectorTest', - 'testReadTypeIntAnnotationsScalarTypes' + 'arguments3' ); $this->assertEquals('int', $reader->getType('a')); -- cgit v1.2.3