Add OCS API for sharees list #18234

nickvergessen · 2015-08-12T13:08:28Z

@DeepDiver1975 @rullzer

Fix #18117

Todo

Add a filter that prevents sharing with the same user/group/remote again
Add tests for all methods
Make shareType an array
Turn existingShares into an array of share IDs instead of strings

Related/Depending

Implement the autocomplete blocker from #18341 into the OCS endpoint #18352 Autocomplete blocker
Use the new OCS sharee API endpoint in the share dropdown #18236 Change share dropdown request

nickvergessen · 2015-08-12T13:09:48Z

DeepDiver1975 · 2015-08-12T13:29:19Z

As per specification multiple shareTypes can be passed in

$ curl -g -v -u admin "http://localhost:8080/ocs/v1.php/apps/files_sharing/api/v1/sharees?itemType=file&shareType[]=0&shareType[]=1"

rullzer · 2015-08-13T05:33:32Z

apps/files_sharing/api/sharees.php

+ return in_array($sharee['value']['shareWith'], $existingSharees) ? null : $sharee;
+ }, $potentialSharees);
+
+ return array_filter($sharees);


why do the array_map first and then the array_filter. Just 1 array_filter with callback should do the trick

right, I just forgot

nickvergessen · 2015-08-17T11:38:36Z

apps/files_sharing/api/sharees.php

+ * @return bool True if the entry is an autocomplete hint, false otherwise
+ */
+ protected function filterAutocompletion($search, $sharee, $label) {
+ if ($this->config->getSystemValue('webui-sharing-autocompletion.enabled', true)) {


#18341 (comment)

nickvergessen · 2015-08-17T13:52:18Z

apps/files_sharing/api/sharees.php

+ $users = array_unique($users);
+ } else {
+ // Search in all users
+ $users_tmp = $this->userManager->searchDisplayName($search);


Just saying, this is a performance hell, we grab all users and trim the sorted result later.
The problem is our custom sorter does not work on the user/group backend as they can only sort by uid/gid

If user enumeration is disabled, we can change this to a simple check if the group/user exists, but if type hinting is enabled it's 😞

The custom sorter moves results to the beginning of the list, which start with the entered search term. If searchDisplayName() would support wildcards, we could easily circumvent this, by running two requests:

search* and 2. *search* - both with a limit

But for now this seems to be implossible @DeepDiver1975

I share your concern. This approach can really hurt on huge installations.
But if we want to present merged results like this I do not really see an other option.

Well the old version (ajax.php) did not grab all users. The only disadvantage was, that the sorting was different (uid, gid, instead of starting with searchterm, otherwise sort by label).
I guess there are people which prefer the "nicer" way of sorting.

But I'd say it's not worth the performance troubles. This will make the drop down unusable on installations with a few hundred users/groups.

Perf hell: no thanks!

But lets move this in to allow Clients to implement this feature.
We can work in perf afterwards.

ghost · 2015-08-17T14:56:18Z

💣 Test FAILed. 💣

DeepDiver1975 · 2015-08-19T07:58:11Z

PostgreSQL

OCA\Files_Sharing\Tests\API\ShareesTest::testGetShareesForShareIds with data set #1 (array(1, 2, 3), array(array('user1'), array('group1')))
Doctrine\DBAL\Exception\DriverException: An exception occurred while executing 'INSERT INTO "oc_share" ("share_type", "share_with", "uid_owner", "parent", "item_type", "item_source", "item_target", "file_source", "file_target", "permissions", "stime", "accepted", "expiration", "token", "mail_send") VALUES(?, ?, ?, ?, 'fake', '', '', '0', '', '0', '0', '0', '', '', '0')' with params [0, "user1", "test7v2g8jX2wj0x5", null]:

SQLSTATE[22007]: Invalid datetime format: 7 ERROR:  invalid input syntax for type timestamp: ""
LINE 1: ..., $3, $4, 'fake', '', '', '0', '', '0', '0', '0', '', '', '0...
                                                             ^

/var/jenkins/workspace/pull-request-analyser-ng-simple@2/label/SLAVE/3rdparty/doctrine/dbal/lib/Doctrine/DBAL/Driver/AbstractPostgreSQLDriver.php:91
/var/jenkins/workspace/pull-request-analyser-ng-simple@2/label/SLAVE/3rdparty/doctrine/dbal/lib/Doctrine/DBAL/DBALException.php:116
/var/jenkins/workspace/pull-request-analyser-ng-simple@2/label/SLAVE/3rdparty/doctrine/dbal/lib/Doctrine/DBAL/Connection.php:996
/var/jenkins/workspace/pull-request-analyser-ng-simple@2/label/SLAVE/lib/private/db/connection.php:203
/var/jenkins/workspace/pull-request-analyser-ng-simple@2/label/SLAVE/3rdparty/doctrine/dbal/lib/Doctrine/DBAL/Query/QueryBuilder.php:208
/var/jenkins/workspace/pull-request-analyser-ng-simple@2/label/SLAVE/lib/private/db/querybuilder/querybuilder.php:121
/var/jenkins/workspace/pull-request-analyser-ng-simple@2/label/SLAVE/apps/files_sharing/tests/api/shareestest.php:860
/var/jenkins/workspace/pull-request-analyser-ng-simple@2/label/SLAVE/apps/files_sharing/tests/api/shareestest.php:804

nickvergessen · 2015-08-19T08:15:09Z

hmpf, forgot to push...

Anyway, rebased and pushed now

ghost · 2015-08-19T09:44:55Z

💣 Test FAILed. 💣

ghost · 2015-08-19T11:12:26Z

🚀 Test PASSed.🚀

To ensure that pagination is working properly we need to make sure the shares are always in the same order. Sorting is first done by label (catches most instances) If there is a user and a group with the same label we sort by shareType If there are multiple users with the same label we sort those by shareWith

scrutinizer-notifier · 2015-08-26T09:54:51Z

A new inspection was created.

nickvergessen · 2015-08-26T09:56:41Z

Fixed all the points that would result in performance issues or incorrect results

ghost · 2015-08-26T19:03:04Z

🚀 Test PASSed.🚀

rullzer · 2015-08-29T14:09:52Z

Why the exact part in the response? Clients know what they requested right. So they can pretty easily find the exact match on their own...

nickvergessen · 2015-08-29T14:22:26Z

exact parts are full matches === term instead of wildcard matches LIKE %term%
Those should be listed first

rullzer · 2015-08-29T15:37:11Z

looking good 👍

schiessle · 2015-09-01T15:08:04Z

code looks good 👍

Add OCS API for sharees list

nickvergessen added 3 - To Review feature:sharing dev:ocs labels Aug 12, 2015

nickvergessen added this to the 8.2-current milestone Aug 12, 2015

nickvergessen mentioned this pull request Aug 12, 2015

Start of OCS Share API sharees endpoint #18147

Closed

8 tasks

nickvergessen mentioned this pull request Aug 12, 2015

Use the new OCS sharee API endpoint in the share dropdown #18236

Closed

3 tasks

nickvergessen added 2 - Developing and removed 3 - To Review labels Aug 12, 2015

rullzer reviewed Aug 13, 2015
View reviewed changes

nickvergessen force-pushed the ocs_api_for_sharees_list branch from aef3993 to 9201407 Compare August 13, 2015 08:57

nickvergessen added 3 - To Review and removed 2 - Developing labels Aug 17, 2015

nickvergessen reviewed Aug 17, 2015
View reviewed changes

nickvergessen force-pushed the ocs_api_for_sharees_list branch from 36aedee to 9f85d1e Compare August 17, 2015 13:08

nickvergessen mentioned this pull request Aug 17, 2015

Implement the autocomplete blocker from #18341 into the OCS endpoint #18352

Merged

2 tasks

nickvergessen reviewed Aug 17, 2015
View reviewed changes

nickvergessen force-pushed the ocs_api_for_sharees_list branch from 9f85d1e to 91071f1 Compare August 17, 2015 14:14

nickvergessen force-pushed the ocs_api_for_sharees_list branch from 91071f1 to 3951415 Compare August 19, 2015 08:14

rullzer added 2 commits August 26, 2015 11:54

Start of OCS Share API sharees endpoint

9830121

nickvergessen added 15 commits August 26, 2015 11:54

Split logic and global usage and add tests for "searchSharees()"

3f64e94

Do not use deprecated method in new code

327c47a

Add tests for "search()"

068a818

Make shareType an array

c6ed40c

Add all possible links next, prev, first and last

a0ab7a2

Inject the logger as well

5c4fbf5

Take a list of share IDs instead of the user and group names

0227cff

Do not return the current user himself

83b88c9

Add tests for "getShareesForShareIds()"

6b69e7b

Rename file to match the class name

937586a

Fix wrong value for datetime field

f4186d3

Manually query for the last id

ac8941f

Fix performance issues of the sharees api

aa2a894

Adjust tests

2a6e676

Bring the coverage back to 100%

199d1dc

nickvergessen force-pushed the ocs_api_for_sharees_list branch from 0cd07d4 to 199d1dc Compare August 26, 2015 09:54

schiessle added a commit that referenced this pull request Sep 1, 2015

Merge pull request #18234 from owncloud/ocs_api_for_sharees_list

39bd4ea

Add OCS API for sharees list

schiessle merged commit 39bd4ea into master Sep 1, 2015

schiessle deleted the ocs_api_for_sharees_list branch September 1, 2015 15:09

This was referenced Sep 1, 2015

Document new Sharee API owncloud-archive/documentation#1626

Closed

Feature request: Allow sharing with users on sync client owncloud/client#3537

Closed

MorrisJobke mentioned this pull request Nov 9, 2015

[Share] It´s not possible to distinguish with whom you have shared something #20291

Closed

lock bot locked as resolved and limited conversation to collaborators Aug 10, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add OCS API for sharees list #18234

Add OCS API for sharees list #18234

nickvergessen commented Aug 12, 2015

nickvergessen commented Aug 12, 2015

DeepDiver1975 commented Aug 12, 2015

rullzer Aug 13, 2015

nickvergessen Aug 13, 2015

nickvergessen Aug 17, 2015

nickvergessen Aug 17, 2015

rullzer Aug 19, 2015

nickvergessen Aug 19, 2015

DeepDiver1975 Aug 19, 2015

DeepDiver1975 Aug 19, 2015

ghost commented Aug 17, 2015

DeepDiver1975 commented Aug 19, 2015

nickvergessen commented Aug 19, 2015

ghost commented Aug 19, 2015

ghost commented Aug 19, 2015

scrutinizer-notifier commented Aug 26, 2015

nickvergessen commented Aug 26, 2015

ghost commented Aug 26, 2015

rullzer commented Aug 29, 2015

nickvergessen commented Aug 29, 2015

rullzer commented Aug 29, 2015

schiessle commented Sep 1, 2015

Add OCS API for sharees list #18234

Add OCS API for sharees list #18234

Conversation

nickvergessen commented Aug 12, 2015

Todo

Related/Depending

nickvergessen commented Aug 12, 2015

DeepDiver1975 commented Aug 12, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost commented Aug 17, 2015

DeepDiver1975 commented Aug 19, 2015

nickvergessen commented Aug 19, 2015

ghost commented Aug 19, 2015

ghost commented Aug 19, 2015

scrutinizer-notifier commented Aug 26, 2015

nickvergessen commented Aug 26, 2015

ghost commented Aug 26, 2015

rullzer commented Aug 29, 2015

nickvergessen commented Aug 29, 2015

rullzer commented Aug 29, 2015

schiessle commented Sep 1, 2015