Uses IShare::TYPE_... in Server.php rather than the obsolete Constants#32647
Uses IShare::TYPE_... in Server.php rather than the obsolete Constants#32647
Conversation
Signed-off-by: Cyrille Bollu <cyrpub@bollu.be>
Any app can register a provider: |
Yeah so would be good to double check if external apps like talk and circle still work |
Indeed. IIUC, plugins registered via an app are processed via server/lib/private/legacy/OC_App.php and this won't work anymore with my change. Should I just add a test in public function registerPlugin(array $pluginInfo) {
--- $shareType = constant(Share::class . '::' . $pluginInfo['shareType']);
+++ if (str_starts_with($pluginInfo['shareType'], 'SHARE_TYPE') {
+++ $shareType = constant(Share::class . '::' . $pluginInfo['shareType']);
+++ else {
+++ $shareType = $pluginInfo['shareType'];
+++ }
if ($shareType === null) {
throw new \InvalidArgumentException('Provided ShareType is invalid');
}and create issues in identified apps? |
nickvergessen
left a comment
There was a problem hiding this comment.
Blocking merge until apps have been fixed
|
|
||
| public function registerPlugin(array $pluginInfo) { | ||
| $shareType = constant(Share::class . '::' . $pluginInfo['shareType']); | ||
| $shareType = $pluginInfo['shareType']; |
There was a problem hiding this comment.
So your proposal is:
| $shareType = $pluginInfo['shareType']; | |
| if (str_starts_with($pluginInfo['shareType'], 'SHARE_TYPE') { | |
| $shareType = constant(Share::class . '::' . $pluginInfo['shareType']); | |
| } else { | |
| $shareType = $pluginInfo['shareType']; | |
| } |
There was a problem hiding this comment.
How about:
| $shareType = $pluginInfo['shareType']; | |
| if (str_starts_with($pluginInfo['shareType'], 'SHARE_TYPE') { | |
| $shareType = constant(IShare::class . '::' . substr($pluginInfo['shareType'], 6)); | |
| } else { | |
| $shareType = $pluginInfo['shareType']; | |
| } |
This way we still get rid of the old constats?
There was a problem hiding this comment.
Please also note that $shareType is a int after constant() but not with your code.
So should use the following on the else branch
$shareType = (int) $pluginInfo['shareType'];
juliusknorr
left a comment
There was a problem hiding this comment.
@StCyr Any update on this? Any chance you could open issues for the apps that need fixing? For backward compatiblity we could also go the str_starts_with approach that was commented already.
|
As there is no feedback since a while I will close this ticket. Thanks for the interest in Nextcloud and the effort put into this! 🙇 |
It doesn't look like those search plugins are used somewhere else....
hmmm.... Here might be another attention point though:
server/lib/private/legacy/OC_App.php
Line 257 in 5a12e8f
Signed-off-by: Cyrille Bollu cyrpub@bollu.be