Enforce string for folder id when obtaining the trash folder

[juliusknorr]

Fix for #907 (comment)

[solracsf]

LGTM

[kesselb]

groupfolders/lib/Trash/TrashBackend.php

Lines 270 to 272 in 6633e64

	$folderId = $folder['folder_id'];
	$mountPoint = $folder['mount_point'];
	$trashFolder = $this->getTrashFolder($folderId);

To use strict types later we may cast the folder id to a string here. However i'm a bit supprised to see $folderId as integer here as the value comes from the database and should be a string.

[fschrempf]

However i'm a bit supprised to see $folderId as integer here as the value comes from the database and should be a string.

Looks like we already cast the id to int right after we get it from the db:

groupfolders/lib/Folder/FolderManager.php

Line 418 in 6e6d353

'folder_id' => (int)$folder['folder_id'],

[fschrempf]

Casting the types back and forth (the folder id is originally a string when it is retrieved from the db) in several places seems kind of messy, but cleaning this is out of scope of this PR.

[kesselb]

Casting the types back and forth (the folder id is originally a string when it is retrieved from the db) in several places seems kind of messy, but cleaning this is out of scope of this PR.

In groupfolders it's okay to treat the folderId as integer. As soon as we use the folderId to access the filesystem the folderId must be a string (because the filesystem api expects a string).

A string typehint for getTrashFolder tell's php to convert the method parameter to a string. But only if strict types are not enabled for the current file. The goal is to have more files with strict code. Adding a cast to string before calling the typed method ensures that the code still works with strict mode enabled.

groupfolders/lib/Trash/TrashBackend.php

Line 270 in 6633e64

$folderId = $folder['folder_id'];

$folderId = (string)$folder['folder_id']; and it will also work with strict mode.

[fschrempf]

A string typehint for getTrashFolder tell's php to convert the method parameter to a string. But only if strict types are not enabled for the current file. The goal is to have more files with strict code. Adding a cast to string before calling the typed method ensures that the code still works with strict mode enabled.

Ok, sounds reasonable. But then it looks like there is at least one more case where getTrashFolder() is called with an int parameter. I guess this needs to be fixed, too:

groupfolders/lib/Trash/TrashBackend.php

Lines 323 to 326 in 7d7abd6

	public function cleanTrashFolder(int $folderid) {
	$trashFolder = $this->getTrashFolder($folderid);
	if (!($trashFolder instanceof Folder)) {

[kesselb]

Yep. As getTrashFolder is a groupfolder method it would also work to typehint the method to int and cast the string inside getTrashFolder.

[fschrempf]

@juliushaertl Could you improve this according to the suggestion from @kesselb? Thanks!

[AndyXheli]

Anyway we can get this on the next release ?

[juliusknorr]

Adjusted accordingly to always use an int internally and only cast to string when referring to the nodes API.

I also added a cast to avoid possible further string occurences when building the folder array from the database results.

[juliusknorr]

/backport to stable21

[juliusknorr]

/backport to stable20

[juliusknorr]

/backport to stable19