Add a single public api for resolving a cloud id to a user and remote and back #3297

icewind1991 · 2017-01-27T15:16:17Z

Instead of having at least 3 different implementations that might not all behave in the same way.

Additionally, having a single api point makes it easier to add more "fancy" ways to resolve the cloud id such as the various ways described in #782

mention-bot · 2017-01-27T15:16:19Z

@icewind1991, thanks for your PR! By analyzing the history of the files in this pull request, we identified @nickvergessen, @LukasReschke and @schiessle to be potential reviewers.

icewind1991 · 2017-02-06T14:28:01Z

All green, @schiessle @MorrisJobke @rullzer please review

MorrisJobke

Please add tests for the new classes

… and back Signed-off-by: Robin Appelman <[email protected]>

Signed-off-by: Robin Appelman <[email protected]>

icewind1991 · 2017-02-08T15:11:27Z

Rebased and added tests

nickvergessen

Only minor things mostly, but please add it to lib/private/AppFramework/DependencyInjection/DIContainer so it can be auto injected in apps

nickvergessen · 2017-02-09T09:10:12Z

apps/federatedfilesharing/lib/FederatedShareProvider.php


-		$share->setSharedWith($user . '@' . $remote);
+
+		$share->setSharedWith(rtrim($cloudId->getId(), '/'));


that should be done by getId()

nickvergessen · 2017-02-09T09:13:01Z

lib/private/Federation/CloudId.php

+	public function getRemote() {
+		return $this->remote;
+	}
+}


missing new line

nickvergessen · 2017-02-09T09:14:35Z

lib/private/Federation/CloudId.php

+<?php
+/**
+ * @copyright Copyright (c) 2017, Robin Appelman <[email protected]>
+ *


Missing @license GNU AGPL version 3 or any later version

nickvergessen · 2017-02-09T09:14:43Z

lib/private/Federation/CloudIdManager.php

+<?php
+/**
+ * @copyright Copyright (c) 2017, Robin Appelman <[email protected]>
+ *


Missing @license GNU AGPL version 3 or any later version

nickvergessen · 2017-02-09T09:16:05Z

lib/public/Federation/ICloudId.php

+	 * @since 12.0.0
+	 */
+	public function getRemote();
+}


nickvergessen · 2017-02-09T09:16:10Z

lib/public/Federation/ICloudId.php

+<?php
+/**
+ * @copyright Copyright (c) 2017, Robin Appelman <[email protected]>
+ *


Missing @license GNU AGPL version 3 or any later version

nickvergessen · 2017-02-09T09:16:16Z

lib/public/Federation/ICloudIdManager.php

+<?php
+/**
+ * @copyright Copyright (c) 2017, Robin Appelman <[email protected]>
+ *


Missing @license GNU AGPL version 3 or any later version

nickvergessen · 2017-02-09T09:16:22Z

lib/public/Federation/ICloudIdManager.php

+	 * @since 12.0.0
+	 */
+	public function isValidCloudId($cloudId);
+}


Signed-off-by: Robin Appelman <[email protected]>

icewind1991 · 2017-02-09T12:33:42Z

@nickvergessen all fixed

codecov-io · 2017-02-09T13:34:42Z

Codecov Report

Merging #3297 into master will decrease coverage by -0.04%.

@@             Coverage Diff              @@
##             master    #3297      +/-   ##
============================================
- Coverage     54.21%   54.17%   -0.04%     
- Complexity    20987    21042      +55     
============================================
  Files          1304     1306       +2     
  Lines         80105    80336     +231     
  Branches       1253     1253              
============================================
+ Hits          43426    43526     +100     
- Misses        36679    36810     +131

Impacted Files	Coverage Δ	Complexity Δ
apps/federatedfilesharing/lib/Notifier.php	`0% <ø> (ø)`	`24 <7> (ø)`	❌
...es_sharing/lib/Activity/Providers/RemoteShares.php	`0% <ø> (ø)`	`15 <1> (+1)`	✅
...ederatedfilesharing/lib/BackgroundJob/RetryJob.php	`0% <ø> (ø)`	`10 <ø> (ø)`	❌
ocs/v1.php	`0% <ø> (ø)`	`0 <ø> (ø)`	❌
apps/files_sharing/lib/External/MountProvider.php	`100% <100%> (ø)`	`4 <1> (ø)`	❌
lib/private/Share20/ProviderFactory.php	`88.75% <100%> (+0.28%)`	`23 <ø> (ø)`	❌
lib/private/Server.php	`92.62% <100%> (-0.13%)`	`120 <1> (+1)`
...s/federatedfilesharing/lib/AppInfo/Application.php	`93.93% <100%> (+0.39%)`	`6 <ø> (ø)`	❌
apps/federatedfilesharing/lib/AddressHandler.php	`92.3% <100%> (-3.01%)`	`13 <ø> (-11)`
lib/private/Federation/CloudIdManager.php	`100% <100%> (ø)`	`15 <15> (?)`
... and 21 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1a591ce...0d8d658. Read the comment docs.

icewind1991 added the 3. to review Waiting for reviews label Jan 27, 2017

icewind1991 added this to the Nextcloud 12.0 milestone Jan 27, 2017

icewind1991 requested review from nickvergessen and schiessle January 27, 2017 15:16

icewind1991 force-pushed the cloud-id-resolve branch from 1547393 to 3117228 Compare January 27, 2017 15:54

icewind1991 force-pushed the cloud-id-resolve branch from 3117228 to acba0cb Compare February 6, 2017 12:03

MorrisJobke requested changes Feb 7, 2017

View reviewed changes

icewind1991 force-pushed the cloud-id-resolve branch from a7b3f80 to dfaf999 Compare February 8, 2017 14:09

icewind1991 added 5 commits February 8, 2017 15:17

Add a single public api for resolving a cloud id to a user and remote…

fa49c4a

… and back Signed-off-by: Robin Appelman <[email protected]>

remove test for removed code

a359d0d

Signed-off-by: Robin Appelman <[email protected]>

log ocs exceptions

e7e614d

Signed-off-by: Robin Appelman <[email protected]>

register cloudidmanager for files_sharing

976cff7

Signed-off-by: Robin Appelman <[email protected]>

add tests

ae66cf8

Signed-off-by: Robin Appelman <[email protected]>

icewind1991 force-pushed the cloud-id-resolve branch from dfaf999 to ae66cf8 Compare February 8, 2017 14:18

MorrisJobke approved these changes Feb 9, 2017

View reviewed changes

nickvergessen requested changes Feb 9, 2017

View reviewed changes

icewind1991 added 2 commits February 9, 2017 13:31

clean cloud ids

f6cd520

Signed-off-by: Robin Appelman <[email protected]>

fix copyright headers

0d8d658

Signed-off-by: Robin Appelman <[email protected]>

nickvergessen approved these changes Feb 9, 2017

View reviewed changes

icewind1991 merged commit bf88060 into master Feb 9, 2017

icewind1991 deleted the cloud-id-resolve branch February 9, 2017 13:54

nickvergessen mentioned this pull request Feb 14, 2017

Fix DI of the cloud id manager into apps #3477

Merged

patrick1990 mentioned this pull request Jun 14, 2017

Use LDAP user name instead of UUID for federated cloud id #5406

Open


		$share->setSharedWith($user . '@' . $remote);

		$share->setSharedWith(rtrim($cloudId->getId(), '/'));

Uh oh!

Add a single public api for resolving a cloud id to a user and remote and back #3297

Add a single public api for resolving a cloud id to a user and remote and back #3297

Uh oh!

Conversation

icewind1991 commented Jan 27, 2017

Uh oh!

mention-bot commented Jan 27, 2017

Uh oh!

icewind1991 commented Feb 6, 2017

Uh oh!

MorrisJobke left a comment

Choose a reason for hiding this comment

Uh oh!

icewind1991 commented Feb 8, 2017

Uh oh!

nickvergessen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

icewind1991 commented Feb 9, 2017

Uh oh!

codecov-io commented Feb 9, 2017

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants