feat(files_sharing): Add types filter to share API

[nfebe]

Description

Add share filtering to Share API

[susnux]

Not sure we need two requests to the backend, we also can just filter on the frontend

[provokateurin]

Agreed, the filtering should be done on the frontend.
Also this breaks other API users because you introduce a new field without a default value that is the same as the current behavior.

[nfebe]

Not sure we need two requests to the backend, we also can just filter on the frontend

I think having this in the API is necessary. An API that returns different types of shares should naturally support filtering by type. Beyond this specific use case, I’m actually surprised that this wasn’t already possible in the GET. It seems like a basic and useful capability.

cc: @susnux

Agreed, the filtering should be done on the frontend.
Also this breaks other API users because you introduce a new field without a default value that is the same as the current behavior.

That is not true. This change is not a breaking change. Omitting the parameter results in the current behavior, returning all shares as expected. Internal and external apps will continue to function as before.

cc: @provokateurin

---

Generally, this increases performance, especially for instances/users with a sizable number of shares, Splitting and parallelizing the request has clear benefits and some drawbacks and so does frontend filtering.

The new new API option/field, is a valuable addition because asides giving the client more flexibility it does not change current behavior, as long as it gets into the OpenAPI spec/admin docs I believe this has more advantages than drawbacks.

[susnux]

Generally, this increases performance, especially for instances/users with a sizable number of shares, Splitting and parallelizing the request has clear benefits and some drawbacks and so does frontend filtering.

I am not sure here.

1. the sidebar loading will increase as the second request itself will add a delay (network)
2. You need to setup the whole enviroment (filesystem / shares) two times as we never ask for only one of both, so this basically duplicates the request time. Meaning if in worst case there is no other php process available this now will take twice the time as before and even if handled sequentially it will double the server load.

So I do not see any benefit here as the shares are requested the same way, there is no shortcut taken while fetching the shares, only formatting them is skipped. But this means now we have two fetch all shares 2 times iterate overthem and filter them.

[nfebe]

the sidebar loading will increase as the second request itself will add a delay (network)

If making two requests results to smaller response sizes then, it is generally faster. The different components can render as the data is available.

You need to setup the whole enviroment (filesystem / shares) two times as we never ask for only one of both, so this basically duplicates the request time. Meaning if in worst case there is no other php process available this now will take twice the time as before and even if handled sequentially it will double the server load.

This is a legitimate concern, I have added a commit to avoid fetching ALL shares, as the method that does it sequentially iterates over all the providers, if we simply pass the types (providers) to this method, then it stops most of this problem.

server/apps/files_sharing/lib/Controller/ShareAPIController.php

Lines 1872 to 1880 in bb6b462

	foreach ($providers as $provider) {
	if (!$this->shareManager->shareProviderExists($provider)) {
	continue;
	}
	$providerShares =
	$this->shareManager->getSharesBy($viewer, $provider, $node, $reShares, -1, 0);
	$shares = array_merge($shares, $providerShares);
	}

The issue of more requests -> more server load remains nonetheless this kind of filtering is usually handled at the backend and even we do not want to use it here, I still think it is essential that shares can be filtered by type.

cc: @susnux

[nfebe]

I am aware of the missing openapi update, had some issues with composer exec generate-spec.

While the main reviews are being cleared that would be handled.

[provokateurin]

I'm mostly siding with @susnux here and my previous comments were under the assumption that the providers were told which share types to query (because I didn't look at the code further).
If we really want to make some performance improvements, then the providers need to be optimized to only query the expected types.
TBH the entire system is a bit weird with fetching all shares from all provider types all the time, but at least it is consistent. We can't just randomly change this, with minimal performance reward, and possible break API consumers.

[nfebe]

/compile

[nfebe]

I've moved the fix to: #52423.
I'll treat this as a non-backportable addition, which also gives us more time to address any related performance issues without blocking the original fix.