Merge pull request #32748 from nextcloud/bugfix/noid/keep-non-default-protocol-in-cloud-id

Keep non default protocol in cloud id
3 years ago · fe45eaca3b
4 changed files with 61 additions and 62 deletions
--- a/build/integration/features/bootstrap/WebDav.php
+++ b/build/integration/features/bootstrap/WebDav.php
@ -32,8 +32,8 @@
 *
 */
 use GuzzleHttp\Client as GClient;
-use GuzzleHttp\Message\ResponseInterface;
 use PHPUnit\Framework\Assert;
+use Psr\Http\Message\ResponseInterface;
 use Sabre\DAV\Client as SClient;
 use Sabre\DAV\Xml\Property\ResourceType;

--- a/build/integration/federation_features/federated.feature
+++ b/build/integration/federation_features/federated.feature
@ -26,7 +26,7 @@ Feature: federated
 			| file_parent | A_NUMBER |
 			| displayname_owner | user0 |
 			| share_with | user1@REMOTE |
-			| share_with_displayname | user1@REMOTE |
+			| share_with_displayname | user1 |

 	Scenario: Federated group share a file with another server
 		Given Using server "REMOTE"
--- a/lib/private/User/User.php
+++ b/lib/private/User/User.php
@ -555,15 +555,9 @@ class User implements IUser {
 		return $uid . '@' . $server;
 	}

-	/**
-	 * @param string $url
-	 * @return string
-	 */
-	private function removeProtocolFromUrl($url) {
+	private function removeProtocolFromUrl(string $url): string {
 		if (strpos($url, 'https://') === 0) {
 			return substr($url, strlen('https://'));
-		} elseif (strpos($url, 'http://') === 0) {
-			return substr($url, strlen('http://'));
 		}

 		return $url;
--- a/tests/lib/User/UserTest.php
+++ b/tests/lib/User/UserTest.php
@ -16,6 +16,7 @@ use OC\User\User;
 use OCP\Comments\ICommentsManager;
 use OCP\Files\Storage\IStorageFactory;
 use OCP\IConfig;
+use OCP\IURLGenerator;
 use OCP\IUser;
 use OCP\Notification\IManager as INotificationManager;
 use OCP\Notification\INotification;
@ -43,7 +44,7 @@ class UserTest extends TestCase {

 	public function testDisplayName() {
 		/**
-		 * @var \OC\User\Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var \OC\User\Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(\OC\User\Backend::class);
 		$backend->expects($this->once())
@ -65,7 +66,7 @@ class UserTest extends TestCase {
 	 */
 	public function testDisplayNameEmpty() {
 		/**
-		 * @var \OC\User\Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var \OC\User\Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(\OC\User\Backend::class);
 		$backend->expects($this->once())
@ -84,7 +85,7 @@ class UserTest extends TestCase {

 	public function testDisplayNameNotSupported() {
 		/**
-		 * @var \OC\User\Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var \OC\User\Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(\OC\User\Backend::class);
 		$backend->expects($this->never())
@ -101,7 +102,7 @@ class UserTest extends TestCase {

 	public function testSetPassword() {
 		/**
-		 * @var Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);
 		$backend->expects($this->once())
@ -124,7 +125,7 @@ class UserTest extends TestCase {

 	public function testSetPasswordNotSupported() {
 		/**
-		 * @var Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);
 		$backend->expects($this->never())
@ -140,7 +141,7 @@ class UserTest extends TestCase {

 	public function testChangeAvatarSupportedYes() {
 		/**
-		 * @var Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(AvatarUserDummy::class);
 		$backend->expects($this->once())
@ -164,7 +165,7 @@ class UserTest extends TestCase {

 	public function testChangeAvatarSupportedNo() {
 		/**
-		 * @var Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(AvatarUserDummy::class);
 		$backend->expects($this->once())
@ -188,7 +189,7 @@ class UserTest extends TestCase {

 	public function testChangeAvatarNotSupported() {
 		/**
-		 * @var Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(AvatarUserDummy::class);
 		$backend->expects($this->never())
@ -204,7 +205,7 @@ class UserTest extends TestCase {

 	public function testDelete() {
 		/**
-		 * @var Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);
 		$backend->expects($this->once())
@ -227,11 +228,11 @@ class UserTest extends TestCase {
 		}

 		/**
-		 * @var Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);

-		$backend->expects($this->at(0))
+		$backend->expects($this->once())
 			->method('implementsActions')
 			->willReturnCallback(function ($actions) {
 				if ($actions === \OC\User\Backend::GET_HOME) {
@ -244,12 +245,12 @@ class UserTest extends TestCase {
 		// important: getHome MUST be called before deleteUser because
 		// once the user is deleted, getHome implementations might not
 		// return anything
-		$backend->expects($this->at(1))
+		$backend->expects($this->once())
 			->method('getHome')
 			->with($this->equalTo('foo'))
 			->willReturn('/home/foo');

-		$backend->expects($this->at(2))
+		$backend->expects($this->once())
 			->method('deleteUser')
 			->with($this->equalTo('foo'));

@ -259,7 +260,7 @@ class UserTest extends TestCase {

 	public function testGetHome() {
 		/**
-		 * @var Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);
 		$backend->expects($this->once())
@ -290,7 +291,7 @@ class UserTest extends TestCase {

 	public function testGetHomeNotSupported() {
 		/**
-		 * @var Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);
 		$backend->expects($this->never())
@ -317,7 +318,7 @@ class UserTest extends TestCase {

 	public function testCanChangePassword() {
 		/**
-		 * @var Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);

@ -337,7 +338,7 @@ class UserTest extends TestCase {

 	public function testCanChangePasswordNotSupported() {
 		/**
-		 * @var Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);

@ -351,7 +352,7 @@ class UserTest extends TestCase {

 	public function testCanChangeDisplayName() {
 		/**
-		 * @var Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);

@ -371,7 +372,7 @@ class UserTest extends TestCase {

 	public function testCanChangeDisplayNameNotSupported() {
 		/**
-		 * @var Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);

@ -385,7 +386,7 @@ class UserTest extends TestCase {

 	public function testSetDisplayNameSupported() {
 		/**
-		 * @var Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(\OC\User\Database::class);

@ -414,7 +415,7 @@ class UserTest extends TestCase {
 	 */
 	public function testSetDisplayNameEmpty() {
 		/**
-		 * @var Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(\OC\User\Database::class);

@ -435,7 +436,7 @@ class UserTest extends TestCase {

 	public function testSetDisplayNameNotSupported() {
 		/**
-		 * @var Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(\OC\User\Database::class);

@ -456,7 +457,7 @@ class UserTest extends TestCase {
 		$test = $this;

 		/**
-		 * @var Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);
 		$backend->expects($this->once())
@ -509,7 +510,7 @@ class UserTest extends TestCase {
 		$test = $this;

 		/**
-		 * @var Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);
 		$backend->expects($this->once())
@ -587,26 +588,30 @@ class UserTest extends TestCase {
 		$this->assertEquals($expectedHooks, $hooksCalled);
 	}

-	public function testGetCloudId() {
-		/**
-		 * @var Backend | \PHPUnit\Framework\MockObject\MockObject $backend
-		 */
+	public function dataGetCloudId(): array {
+		return [
+			['https://localhost:8888/nextcloud', 'foo@localhost:8888/nextcloud'],
+			['http://localhost:8888/nextcloud', 'foo@http://localhost:8888/nextcloud'],
+		];
+	}
+
+	/**
+	 * @dataProvider dataGetCloudId
+	 */
+	public function testGetCloudId(string $absoluteUrl, string $cloudId): void {
+		/** @var Backend|MockObject $backend */
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);
-		$urlGenerator = $this->getMockBuilder('\OC\URLGenerator')
-				->setMethods(['getAbsoluteURL'])
-				->disableOriginalConstructor()->getMock();
-		$urlGenerator
-				->expects($this->any())
-				->method('getAbsoluteURL')
-				->withAnyParameters()
-				->willReturn('http://localhost:8888/owncloud');
+		$urlGenerator = $this->createMock(IURLGenerator::class);
+		$urlGenerator->method('getAbsoluteURL')
+			->withAnyParameters()
+			->willReturn($absoluteUrl);
 		$user = new User('foo', $backend, $this->dispatcher, null, null, $urlGenerator);
-		$this->assertEquals('foo@localhost:8888/owncloud', $user->getCloudId());
+		$this->assertEquals($cloudId, $user->getCloudId());
 	}

 	public function testSetEMailAddressEmpty() {
 		/**
-		 * @var Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);

@ -642,7 +647,7 @@ class UserTest extends TestCase {

 	public function testSetEMailAddress() {
 		/**
-		 * @var UserInterface | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var UserInterface | MockObject $backend
 		 */
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);

@ -679,11 +684,11 @@ class UserTest extends TestCase {

 	public function testSetEMailAddressNoChange() {
 		/**
-		 * @var UserInterface | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var UserInterface | MockObject $backend
 		 */
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);

-		/** @var PublicEmitter|\PHPUnit\Framework\MockObject\MockObject $emitter */
+		/** @var PublicEmitter|MockObject $emitter */
 		$emitter = $this->createMock(PublicEmitter::class);
 		$emitter->expects($this->never())
 			->method('emit');
@ -704,7 +709,7 @@ class UserTest extends TestCase {

 	public function testSetQuota() {
 		/**
-		 * @var UserInterface | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var UserInterface | MockObject $backend
 		 */
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);

@ -741,11 +746,11 @@ class UserTest extends TestCase {

 	public function testGetDefaultUnlimitedQuota() {
 		/**
-		 * @var UserInterface | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var UserInterface | MockObject $backend
 		 */
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);

-		/** @var PublicEmitter|\PHPUnit\Framework\MockObject\MockObject $emitter */
+		/** @var PublicEmitter|MockObject $emitter */
 		$emitter = $this->createMock(PublicEmitter::class);
 		$emitter->expects($this->never())
 			->method('emit');
@ -772,11 +777,11 @@ class UserTest extends TestCase {

 	public function testGetDefaultUnlimitedQuotaForbidden() {
 		/**
-		 * @var UserInterface | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var UserInterface | MockObject $backend
 		 */
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);

-		/** @var PublicEmitter|\PHPUnit\Framework\MockObject\MockObject $emitter */
+		/** @var PublicEmitter|MockObject $emitter */
 		$emitter = $this->createMock(PublicEmitter::class);
 		$emitter->expects($this->never())
 			->method('emit');
@ -806,11 +811,11 @@ class UserTest extends TestCase {

 	public function testSetQuotaAddressNoChange() {
 		/**
-		 * @var UserInterface | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var UserInterface | MockObject $backend
 		 */
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);

-		/** @var PublicEmitter|\PHPUnit\Framework\MockObject\MockObject $emitter */
+		/** @var PublicEmitter|MockObject $emitter */
 		$emitter = $this->createMock(PublicEmitter::class);
 		$emitter->expects($this->never())
 			->method('emit');
@ -828,7 +833,7 @@ class UserTest extends TestCase {

 	public function testGetLastLogin() {
 		/**
-		 * @var Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);

@ -848,7 +853,7 @@ class UserTest extends TestCase {

 	public function testSetEnabled() {
 		/**
-		 * @var Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);

@ -868,7 +873,7 @@ class UserTest extends TestCase {

 	public function testSetDisabled() {
 		/**
-		 * @var Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);

@ -908,7 +913,7 @@ class UserTest extends TestCase {

 	public function testSetDisabledAlreadyDisabled() {
 		/**
-		 * @var Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);

@ -938,7 +943,7 @@ class UserTest extends TestCase {

 	public function testGetEMailAddress() {
 		/**
-		 * @var Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);