search

owncloud / core

:cloud: ownCloud web server core (Files, DAV, etc.)
https://owncloud.com
GNU Affero General Public License v3.0
8.39k stars 2.05k forks source link

Provision API pagination for subadmins is broken #19964

Closed rullzer closed 3 years ago

rullzer commented 9 years ago

Now that we allow subadmins to get the list of users they have access to via the provisioning API we run into issues.

https://github.com/owncloud/core/blob/e1e1f4fd99eafbfe17d7d3729d17739c221d75de/apps/provisioning_api/lib/users.php#L84-L97

As you can see what we do there is for each group the subadmin is a subadmin of we get the list of users. We merge those lists and then slice the array accordingly.

Now this introduces a few issues:

  1. Assume 10 groups with 1000 members each, your system won't be happy
  2. Since we ask the backends to sort it will be hard to write proper pagination test

To solve problem 1 I think we might need to introduce new calls to the groupManager i.e. displayNamesInGroups and then the additional calls all the way down. This will allow us to let the backend handle deduplication (which is most likely can do much more efficiently).

Problem 2 is something different but can be solved with integration tests. Basically we do not quarantee any order on the results. So if we fill the database with random users to query all we need to check is if (when doing pagination) all users get enumerated exactly once.

Tagging for 9.0 since especially problem 1 could be an issue for large installs.

CC: @DeepDiver1975 @tomneedham

DeepDiver1975 commented 9 years ago

Tagging for 9.0 since especially problem 1 could be an issue for large installs.

@rperezb please add tests to the QA plan - THX

rullzer commented 9 years ago

I already made an issue a while back for the displayNamesInGroups: https://github.com/owncloud/core/issues/18418

PVince81 commented 8 years ago

@rullzer is that still an issue ?

rullzer commented 8 years ago

yes.

karlitschek commented 8 years ago

Is this really a sev2?

PVince81 commented 8 years ago

Moving to 9.2 as per PR

PVince81 commented 7 years ago

@tomneedham assigning to you as you're familiar with that API. I'm aware that you might not have time, but let's see if you have some insights.

PVince81 commented 7 years ago

moving to backlog as no one complained about this, maybe no one is actually using this pagination currently

ownclouders commented 6 years ago

Hey, this issue has been closed because the label status/STALE is set and there were no updates for 7 days. Feel free to reopen this issue if you deem it appropriate.

(This is an automated comment from GitMate.io.)

ownclouders commented 6 years ago

Hey, this issue has been closed because the label status/STALE is set and there were no updates for 7 days. Feel free to reopen this issue if you deem it appropriate.

(This is an automated comment from GitMate.io.)

stale[bot] commented 3 years ago

This issue has been automatically closed.