mbk-lab
/
nextcloud
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Fix comments from Thomas

This commit is contained in:
Roeland Jago Douma 2016-01-26 14:29:30 +01:00
parent 44c073b1e6
commit d11682dcb4
4 changed files with 38 additions and 37 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
lib/private/share20/defaultshareprovider.php
View File

@ -241,11 +241,11 @@ class DefaultShareProvider implements IShareProvider {
->set('item_source', $qb->createNamedParameter($share->getPath()->getId())) ->set('item_source', $qb->createNamedParameter($share->getPath()->getId()))
->set('file_source', $qb->createNamedParameter($share->getPath()->getId())) ->set('file_source', $qb->createNamedParameter($share->getPath()->getId()))
->set('token', $qb->createNamedParameter($share->getToken())) ->set('token', $qb->createNamedParameter($share->getToken()))
->set('expiration', $qb->createNamedParameter($share->getExpirationDate(), 'datetime')) ->set('expiration', $qb->createNamedParameter($share->getExpirationDate(), IQueryBuilder::PARAM_DATE))
->execute(); ->execute();
} }
return $this->getShareById($share->getId()); return $share;
} }
/** /**

28
lib/private/share20/manager.php
View File

@ -217,19 +217,19 @@ class Manager {
/** /**
* Validate if the expiration date fits the system settings * Validate if the expiration date fits the system settings
* *
* @param \DateTime $expireDate The current expiration date (can be null) * @param \DateTime $expirationDate The current expiration date (can be null)
* @return \DateTime|null The expiration date or null if $expireDate was null and it is not required * @return \DateTime|null The expiration date or null if $expireDate was null and it is not required
* @throws \OC\HintException * @throws \OC\HintException
*/ */
protected function validateExpiredate($expireDate) { protected function validateExpirationDate($expirationDate) {
if ($expireDate !== null) { if ($expirationDate !== null) {
//Make sure the expiration date is a date //Make sure the expiration date is a date
$expireDate->setTime(0, 0, 0); $expirationDate->setTime(0, 0, 0);
$date = new \DateTime(); $date = new \DateTime();
$date->setTime(0, 0, 0); $date->setTime(0, 0, 0);
if ($date >= $expireDate) { if ($date >= $expirationDate) {
$message = $this->l->t('Expiration date is in the past'); $message = $this->l->t('Expiration date is in the past');
throw new \OC\HintException($message, $message, 404); throw new \OC\HintException($message, $message, 404);
} }
@ -237,30 +237,30 @@ class Manager {
// If we enforce the expiration date check that is does not exceed // If we enforce the expiration date check that is does not exceed
if ($this->shareApiLinkDefaultExpireDateEnforced()) { if ($this->shareApiLinkDefaultExpireDateEnforced()) {
if ($expireDate === null) { if ($expirationDate === null) {
throw new \InvalidArgumentException('Expiration date is enforced'); throw new \InvalidArgumentException('Expiration date is enforced');
} }
$date = new \DateTime(); $date = new \DateTime();
$date->setTime(0, 0, 0); $date->setTime(0, 0, 0);
$date->add(new \DateInterval('P' . $this->shareApiLinkDefaultExpireDays() . 'D')); $date->add(new \DateInterval('P' . $this->shareApiLinkDefaultExpireDays() . 'D'));
if ($date < $expireDate) { if ($date < $expirationDate) {
$message = $this->l->t('Cannot set expiration date more than %s days in the future', [$this->shareApiLinkDefaultExpireDays()]); $message = $this->l->t('Cannot set expiration date more than %s days in the future', [$this->shareApiLinkDefaultExpireDays()]);
throw new \OC\HintException($message, $message, 404); throw new \OC\HintException($message, $message, 404);
} }
return $expireDate; return $expirationDate;
} }
// If expiredate is empty set a default one if there is a default // If expiredate is empty set a default one if there is a default
if ($expireDate === null && $this->shareApiLinkDefaultExpireDate()) { if ($expirationDate === null && $this->shareApiLinkDefaultExpireDate()) {
$date = new \DateTime(); $date = new \DateTime();
$date->setTime(0,0,0); $date->setTime(0,0,0);
$date->add(new \DateInterval('P'.$this->shareApiLinkDefaultExpireDays().'D')); $date->add(new \DateInterval('P'.$this->shareApiLinkDefaultExpireDays().'D'));
return $date; return $date;
} }
return $expireDate; return $expirationDate;
} }
/** /**
@ -440,7 +440,7 @@ class Manager {
); );
//Verify the expiration date //Verify the expiration date
$share->setExpirationDate($this->validateExpiredate($share->getExpirationDate())); $share->setExpirationDate($this->validateExpirationDate($share->getExpirationDate()));
//Verify the password //Verify the password
$this->verifyPassword($share->getPassword()); $this->verifyPassword($share->getPassword());
@ -542,13 +542,13 @@ class Manager {
// We can't change the share type! // We can't change the share type!
if ($share->getShareType() !== $originalShare->getShareType()) { if ($share->getShareType() !== $originalShare->getShareType()) {
throw new \Exception('Can\'t change share type'); throw new \InvalidArgumentException('Can\'t change share type');
} }
// We can only change the recipient on user shares // We can only change the recipient on user shares
if ($share->getSharedWith() !== $originalShare->getSharedWith() && if ($share->getSharedWith() !== $originalShare->getSharedWith() &&
$share->getShareType() !== \OCP\Share::SHARE_TYPE_USER) { $share->getShareType() !== \OCP\Share::SHARE_TYPE_USER) {
throw new \Exception('Can only update recipient on user shares'); throw new \InvalidArgumentException('Can only update recipient on user shares');
} }
// Cannot share with the owner // Cannot share with the owner
@ -578,7 +578,7 @@ class Manager {
if ($share->getExpirationDate() !== $originalShare->getExpirationDate()) { if ($share->getExpirationDate() !== $originalShare->getExpirationDate()) {
//Verify the expiration date //Verify the expiration date
$share->setExpirationDate($this->validateExpiredate($share->getExpirationDate())); $share->setExpirationDate($this->validateExpirationDate($share->getExpirationDate()));
$expirationDateUpdated = true; $expirationDateUpdated = true;
} }
} }

7
tests/lib/share20/defaultshareprovidertest.php
View File

@ -21,6 +21,7 @@
namespace Test\Share20; namespace Test\Share20;
use OC\Share20\Exception\ProviderException; use OC\Share20\Exception\ProviderException;
use OCP\DB\QueryBuilder\IQueryBuilder;
use OCP\IDBConnection; use OCP\IDBConnection;
use OCP\IUserManager; use OCP\IUserManager;
use OCP\IGroupManager; use OCP\IGroupManager;
@ -99,7 +100,7 @@ class DefaultShareProviderTest extends \Test\TestCase {
if ($fileTarget) $qb->setValue('file_target', $qb->expr()->literal($fileTarget)); if ($fileTarget) $qb->setValue('file_target', $qb->expr()->literal($fileTarget));
if ($permissions) $qb->setValue('permissions', $qb->expr()->literal($permissions)); if ($permissions) $qb->setValue('permissions', $qb->expr()->literal($permissions));
if ($token) $qb->setValue('token', $qb->expr()->literal($token)); if ($token) $qb->setValue('token', $qb->expr()->literal($token));
if ($expiration) $qb->setValue('expiration', $qb->createNamedParameter($expiration, 'datetime')); if ($expiration) $qb->setValue('expiration', $qb->createNamedParameter($expiration, IQueryBuilder::PARAM_DATE));
if ($parent) $qb->setValue('parent', $qb->expr()->literal($parent)); if ($parent) $qb->setValue('parent', $qb->expr()->literal($parent));
$this->assertEquals(1, $qb->execute()); $this->assertEquals(1, $qb->execute());
@ -1653,7 +1654,7 @@ class DefaultShareProviderTest extends \Test\TestCase {
$share = $this->provider->getShareById($id); $share = $this->provider->getShareById($id);
$share->setSharedWith($groups['group1']); $share->setSharedWith($groups['group0']);
$share->setSharedBy($users['user4']); $share->setSharedBy($users['user4']);
$share->setShareOwner($users['user5']); $share->setShareOwner($users['user5']);
$share->setPath($file2); $share->setPath($file2);
@ -1722,7 +1723,7 @@ class DefaultShareProviderTest extends \Test\TestCase {
$share = $this->provider->getShareById($id); $share = $this->provider->getShareById($id);
$share->setSharedWith($groups['group1']); $share->setSharedWith($groups['group0']);
$share->setSharedBy($users['user4']); $share->setSharedBy($users['user4']);
$share->setShareOwner($users['user5']); $share->setShareOwner($users['user5']);
$share->setPath($file2); $share->setPath($file2);

36
tests/lib/share20/managertest.php
View File

@ -599,29 +599,29 @@ class ManagerTest extends \Test\TestCase {
* @expectedException \OC\HintException * @expectedException \OC\HintException
* @expectedExceptionMessage Expiration date is in the past * @expectedExceptionMessage Expiration date is in the past
*/ */
public function testValidateExpiredateInPast() { public function testvalidateExpirationDateInPast() {
// Expire date in the past // Expire date in the past
$past = new \DateTime(); $past = new \DateTime();
$past->sub(new \DateInterval('P1D')); $past->sub(new \DateInterval('P1D'));
$this->invokePrivate($this->manager, 'validateExpiredate', [$past]); $this->invokePrivate($this->manager, 'validateExpirationDate', [$past]);
} }
/** /**
* @expectedException InvalidArgumentException * @expectedException InvalidArgumentException
* @expectedExceptionMessage Expiration date is enforced * @expectedExceptionMessage Expiration date is enforced
*/ */
public function testValidateExpiredateEnforceButNotSet() { public function testvalidateExpirationDateEnforceButNotSet() {
$this->config->method('getAppValue') $this->config->method('getAppValue')
->will($this->returnValueMap([ ->will($this->returnValueMap([
['core', 'shareapi_enforce_expire_date', 'no', 'yes'], ['core', 'shareapi_enforce_expire_date', 'no', 'yes'],
])); ]));
$this->invokePrivate($this->manager, 'validateExpiredate', [null]); $this->invokePrivate($this->manager, 'validateExpirationDate', [null]);
} }
public function testValidateExpiredateEnforceToFarIntoFuture() { public function testvalidateExpirationDateEnforceToFarIntoFuture() {
// Expire date in the past // Expire date in the past
$future = new \DateTime(); $future = new \DateTime();
$future->add(new \DateInterval('P7D')); $future->add(new \DateInterval('P7D'));
@ -633,7 +633,7 @@ class ManagerTest extends \Test\TestCase {
])); ]));
try { try {
$this->invokePrivate($this->manager, 'validateExpiredate', [$future]); $this->invokePrivate($this->manager, 'validateExpirationDate', [$future]);
} catch (\OC\HintException $e) { } catch (\OC\HintException $e) {
$this->assertEquals('Cannot set expiration date more than 3 days in the future', $e->getMessage()); $this->assertEquals('Cannot set expiration date more than 3 days in the future', $e->getMessage());
$this->assertEquals('Cannot set expiration date more than 3 days in the future', $e->getHint()); $this->assertEquals('Cannot set expiration date more than 3 days in the future', $e->getHint());
@ -641,7 +641,7 @@ class ManagerTest extends \Test\TestCase {
} }
} }
public function testValidateExpiredateEnforceValid() { public function testvalidateExpirationDateEnforceValid() {
// Expire date in the past // Expire date in the past
$future = new \DateTime(); $future = new \DateTime();
$future->add(new \DateInterval('P2D')); $future->add(new \DateInterval('P2D'));
@ -655,27 +655,27 @@ class ManagerTest extends \Test\TestCase {
['core', 'shareapi_expire_after_n_days', '7', '3'], ['core', 'shareapi_expire_after_n_days', '7', '3'],
])); ]));
$future = $this->invokePrivate($this->manager, 'validateExpiredate', [$future]); $future = $this->invokePrivate($this->manager, 'validateExpirationDate', [$future]);
$this->assertEquals($expected, $future->format(\DateTime::ISO8601)); $this->assertEquals($expected, $future->format(\DateTime::ISO8601));
} }
public function testValidateExpiredateNoDateNoDefaultNull() { public function testvalidateExpirationDateNoDateNoDefaultNull() {
$date = new \DateTime(); $date = new \DateTime();
$date->add(new \DateInterval('P5D')); $date->add(new \DateInterval('P5D'));
$res = $this->invokePrivate($this->manager, 'validateExpiredate', [$date]); $res = $this->invokePrivate($this->manager, 'validateExpirationDate', [$date]);
$this->assertEquals($date, $res); $this->assertEquals($date, $res);
} }
public function testValidateExpiredateNoDateNoDefault() { public function testvalidateExpirationDateNoDateNoDefault() {
$date = $this->invokePrivate($this->manager, 'validateExpiredate', [null]); $date = $this->invokePrivate($this->manager, 'validateExpirationDate', [null]);
$this->assertNull($date); $this->assertNull($date);
} }
public function testValidateExpiredateNoDateDefault() { public function testvalidateExpirationDateNoDateDefault() {
$future = new \DateTime(); $future = new \DateTime();
$future->add(new \DateInterval('P3D')); $future->add(new \DateInterval('P3D'));
$future->setTime(0,0,0); $future->setTime(0,0,0);
@ -686,7 +686,7 @@ class ManagerTest extends \Test\TestCase {
['core', 'shareapi_expire_after_n_days', '7', '3'], ['core', 'shareapi_expire_after_n_days', '7', '3'],
])); ]));
$date = $this->invokePrivate($this->manager, 'validateExpiredate', [null]); $date = $this->invokePrivate($this->manager, 'validateExpirationDate', [null]);
$this->assertEquals($future->format(\DateTime::ISO8601), $date->format(\DateTime::ISO8601)); $this->assertEquals($future->format(\DateTime::ISO8601), $date->format(\DateTime::ISO8601));
} }
@ -1314,7 +1314,7 @@ class ManagerTest extends \Test\TestCase {
'generalCreateChecks', 'generalCreateChecks',
'linkCreateChecks', 'linkCreateChecks',
'pathCreateChecks', 'pathCreateChecks',
'validateExpiredate', 'validateExpirationDate',
'verifyPassword', 'verifyPassword',
]) ])
->getMock(); ->getMock();
@ -1352,7 +1352,7 @@ class ManagerTest extends \Test\TestCase {
->method('pathCreateChecks') ->method('pathCreateChecks')
->with($path); ->with($path);
$manager->expects($this->once()) $manager->expects($this->once())
->method('validateExpiredate') ->method('validateExpirationDate')
->with($date) ->with($date)
->will($this->returnArgument(0)); ->will($this->returnArgument(0));
$manager->expects($this->once()) $manager->expects($this->once())
@ -1745,7 +1745,7 @@ class ManagerTest extends \Test\TestCase {
'linkCreateChecks', 'linkCreateChecks',
'pathCreateChecks', 'pathCreateChecks',
'verifyPassword', 'verifyPassword',
'validateExpiredate', 'validateExpirationDate',
]) ])
->getMock(); ->getMock();
@ -1761,7 +1761,7 @@ class ManagerTest extends \Test\TestCase {
$manager->expects($this->once())->method('canShare')->willReturn(true); $manager->expects($this->once())->method('canShare')->willReturn(true);
$manager->expects($this->once())->method('getShareById')->with('foo:42')->willReturn($originalShare); $manager->expects($this->once())->method('getShareById')->with('foo:42')->willReturn($originalShare);
$manager->expects($this->once())->method('validateExpireDate')->with($tomorrow)->willReturn($tomorrow); $manager->expects($this->once())->method('validateExpirationDate')->with($tomorrow)->willReturn($tomorrow);
$file = $this->getMock('OCP\Files\File', [], [], 'File'); $file = $this->getMock('OCP\Files\File', [], [], 'File');
$file->method('getId')->willReturn(100); $file->method('getId')->willReturn(100);