Working on type-hints, please watch out


Dear !Friendica Developers ,

to avoid some confusion, I wanted to let you know that I work on my development instance at the following points:

1) Investigate and fix missing type-hints in parameters and returned values
2) Add missing documentation or remove out-dated (e.g. removed parameter)
3) Adding @TODO to point out badly/missing written documentation of methods

These changes will later be put into a separate branch and made a PR of. Yes, both non-trivial changes as they may break a lot (like it happened before) and they don't fix or add features.

PS: For non-developers out there: Please stay at stable versions and please avoid branches called develop or master (mostly also a development branch).

in reply to Roland Häder🇩🇪

Concerning the third point: I'm not a big fan of adding a @todo for the purpose of adding a documentation in the future. Better is to add the documentation, at least with some basic information.

Concerning the type hints: Before adding them, please have a look at each call for these functions. We have got functions that can be called with multiple types or at least with null and some other type. When a function is supposed to accept both a regular type and null then the type hint has got to look slightly different: ..., int $uid = null, ....

in reply to Roland Häder🇩🇪

[16-Jun-2022 16:16:05 UTC] PHP Fatal error:  Declaration of Friendica\Profile\ProfileField\Collection\ProfileFields::map(callable $callback): Friendica\Profile\ProfileField\Collection\ProfileFields must be compatible with Friendica\BaseCollection::map(callable $callback): Friendica\BaseCollection in /var/www/.../src/Profile/ProfileField/Collection/ProfileFields.php on line 0
[16-Jun-2022 16:16:05 UTC] PHP Stack trace:
[16-Jun-2022 16:16:05 UTC] PHP   1. {main}() /var/www/.../index.php:0
[16-Jun-2022 16:16:05 UTC] PHP   2. Friendica\App->runFrontend($router = *uninitialized*, $pconfig = *uninitialized*, $auth = *uninitialized*, $page = *uninitialized*, $httpInput = *uninitialized*, $start_time = *uninitialized*) /var/www/.../index.php:44
[16-Jun-2022 16:16:05 UTC] PHP   3. Friendica\Module\Profile\Index->run($request = *uninitialized*) /var/www/.../src/App.php:722
[16-Jun-2022 16:16:05 UTC] PHP   4. Friendica\Module\Profile\Index->rawContent($request = *uninitialized*) /var/www/.../src/BaseModule.php:233
[16-Jun-2022 16:16:05 UTC] PHP   5. Friendica\Module\Profile\Profile->rawContent($request = *uninitialized*) /var/www/.../src/Module/Profile/Index.php:40
[16-Jun-2022 16:16:05 UTC] PHP   6. Friendica\Protocol\ActivityPub\Transmitter::getProfile($uid = *uninitialized*) /var/www/.../src/Module/Profile/Profile.php:55
[16-Jun-2022 16:16:05 UTC] PHP   7. Friendica\Profile\ProfileField\Repository\ProfileField->selectByContactId($cid = *uninitialized*, $uid = *uninitialized*) /var/www/.../src/Protocol/ActivityPub/Transmitter.php:517
[16-Jun-2022 16:16:05 UTC] PHP   8. Friendica\Profile\ProfileField\Repository\ProfileField->select($condition = *uninitialized*, $params = *uninitialized*) /var/www/.../src/Profile/ProfileField/Repository/ProfileField.php:174
[16-Jun-2022 16:16:05 UTC] PHP   9. spl_autoload_call(*uninitialized*) /var/www/.../src/Profile/ProfileField/Repository/ProfileField.php:87
[16-Jun-2022 16:16:05 UTC] PHP  10. Composer\Autoload\ClassLoader->loadClass($class = *uninitialized*) /var/www/.../src/Profile/ProfileField/Repository/ProfileField.php:87
[16-Jun-2022 16:16:05 UTC] PHP  11. Composer\Autoload\includeFile($file = *uninitialized*) /var/www/.../vendor/composer/ClassLoader.php:322

Ops! My bad.
in reply to Roland Häder🇩🇪

I'm currently seeing this in my fpm-log:

[16-Jun-2022 18:03:40 UTC] PHP Notice:  Undefined index: contact-type in /var/www/.../htdocs/src/Module/Directory.php on line 169
[16-Jun-2022 18:03:40 UTC] PHP Stack trace:
[16-Jun-2022 18:03:40 UTC] PHP   1. {main}() /var/www/.../htdocs/index.php:0
[16-Jun-2022 18:03:40 UTC] PHP   2. Friendica\App->runFrontend($router = *uninitialized*, $pconfig = *uninitialized*, $auth = *uninitialized*, $page = *uninitialized*, $httpInput = *uninitialized*, $start_time = *uninitialized*) /var/www/.../htdocs/index.php:44
[16-Jun-2022 18:03:40 UTC] PHP   3. Friendica\LegacyModule->run($request = *uninitialized*) /var/www/.../htdocs/src/App.php:722
[16-Jun-2022 18:03:40 UTC] PHP   4. Friendica\LegacyModule->content($request = *uninitialized*) /var/www/.../htdocs/src/BaseModule.php:239
[16-Jun-2022 18:03:40 UTC] PHP   5. Friendica\LegacyModule->runModuleFunction($function_suffix = *uninitialized*) /var/www/.../htdocs/src/LegacyModule.php:73
[16-Jun-2022 18:03:40 UTC] PHP   6. forumdirectory_content($a = *uninitialized*) /var/www/.../htdocs/src/LegacyModule.php:96
[16-Jun-2022 18:03:40 UTC] PHP   7. Friendica\Module\Directory::formatEntry($contact = *uninitialized*, $photo_size = *uninitialized*) /var/www/.../htdocs/addon/forumdirectory/forumdirectory.php:127

The line reads:
'account_type' => Model\Contact::getAccountType($contact['contact-type']),

Is there a possibility that $contact['contact-type'] is not included? I guess adding a ?? '' or so maybe only hides an other bug.

in reply to Roland Häder🇩🇪

Next up:
Argument 5 passed to Friendica\Protocol\ActivityPub\Receiver::getReceiverForActor() must be of the type array, bool given, called in /var/www/.../htdocs/src/Protocol/ActivityPub/Receiver.php on line 943
The 5th parameter is $profile and when I look inside the method, it is used as an array and never as a boolean. So the invocation was wrong here?
in reply to Roland Häder🇩🇪

I went to line 943 and searched for declaration of $profile. There is only one inside an if() block. In other languages such #Java this won't be allowed because the other if() block (where the variable is used) will be out of scope of the if() block where it has been defined. So in other words, $profile didn't exist or was returned as such from $profile = APContact::getByURL($actor); invocation.
#Java
This entry was edited (3 years ago)
in reply to Roland Häder🇩🇪

I'm not completely sure, but we could have got situations where functions can return an array as a valid value and a boolean as an indicator for an error. At least we do so in the database area, but possibly somewhere else as well. So we really have to have a look at each call for a specific function, since we cannot be sure that an error there will pop up immediately.
in reply to Michael 🇺🇦

@Michael Vogel So I rather change these lines back to return false; and remove the returned type-hint array from it?

1be0f4c25c (Roland Häder     2022-06-17 10:44:13 +0200  235)                                    return [];
1be0f4c25c (Roland Häder     2022-06-17 10:44:13 +0200  254)                            return [];
1be0f4c25c (Roland Häder     2022-06-17 10:44:13 +0200  280)                            return [];
1be0f4c25c (Roland Häder     2022-06-17 10:44:13 +0200  290)                            return [];
1be0f4c25c (Roland Häder     2022-06-17 10:44:13 +0200  331)                    return [];
1be0f4c25c (Roland Häder     2022-06-17 10:44:13 +0200  345)                            return [];
in reply to Michael 🇺🇦

@Michael Vogel Receiver::decodePost() seem to rely on a non-array type if an error has happened. Hmm. Cannot we change this whole error handling away from returning false or anything returned value to throwing exceptions?

I propose this as I have seen it with e.g. #Java and found it a very clean approach:
1) If the method finished, return proper value (and type, e.g. array)
2) If the method was unsuccessful, e.g. an unexpected error happened, throw an exception and other methods need to handle it (e.g. catching it, doing some stuff, like closing resources and maybe continue throwing it).

in reply to Roland Häder🇩🇪

Of course I understand @Hypolite Petovan here wanting to have a clean file (here: src/Protocol/Diaspora.php). I totally understand it. :-) The changes would make the already humongous PR much larger than it already is. So let's move on with next rewrites, code lines with log messages are definitely not the pretties in #Friendica code.

So how do I do with DI::logger->level('Message with parameters', 'foo' => $foo, 'bar' => $bar); here?

in reply to Roland Häder🇩🇪

I'm still having this one:
2022-06-20T16:49:58Z worker [ERROR]: Uncaught Exception TypeError: "Argument 2 passed to Friendica\Model\Item::guidFromUri() must be of the type string, null given, called in /var/www/.../src/Protocol/Feed.php on line 315" at /var/www/.../src/Model/Item.php line 1837 {"exception":"TypeError: Argument 2 passed to Friendica\\Model\\Item::guidFromUri() must be of the type string, null given, called in /var/www/.../src/Protocol/Feed.php on line 315 and defined in /var/www/.../src/Model/Item.php:1837\nStack trace:\n#0 /var/www/.../src/Protocol/Feed.php(315): Friendica\\Model\\Item::guidFromUri('/blog/2022/03/1...', NULL)\n#1 /var/www/.../src/Network/Probe.php(1795): Friendica\\Protocol\\Feed::import('<rss version=\"2...')\n#2 /var/www/.../src/Network/Probe.php(1808): Friendica\\Network\\Probe::feed('https://keyconc...', false)\n#3 /var/www/.../src/Network/Probe.php(676): Friendica\\Network\\Probe::feed('https://keyconc...')\n#4 /var/www/.../src/Network/Probe.php(321): Friendica\\Network\\Probe::detect('https://keyconc...', '', 0, Array)\n#5 /var/www/.../src/Model/Contact.php(2328): Friendica\\Network\\Probe::uri('https://keyconc...', '', 0)\n#6 /var/www/.../src/Worker/UpdateContact.php(35): Friendica\\Model\\Contact::updateFromProbe(100664)\n#7 [internal function]: Friendica\\Worker\\UpdateContact::execute(100664)\n#8 /var/www/.../src/Core/Worker.php(486): call_user_func_array('Friendica\\\\Worke...', Array)\n#9 /var/www/.../src/Core/Worker.php(381): Friendica\\Core\\Worker::execFunction(Array, 'UpdateContact', Array, true)\n#10 /var/www/.../src/Core/Worker.php(106): Friendica\\Core\\Worker::execute(Array)\n#11 /var/www/.../bin/worker.php(86): Friendica\\Core\\Worker::processQueue(false, Object(Friendica\\Core\\Worker\\Entity\\Process))\n#12 {main}"} - {"file":null,"line":null,"function":null,"uid":"e22a9d","process_id":1737}
This is why you should not use develop branches. cc @Hypolite Petovan How to fix this?
This entry was edited (3 years ago)
in reply to Roland Häder🇩🇪

I have noticed that view/smarty3/compiled's descriptor is getting very large:

$ ls -l view/smarty3/
total 300
drwxr-xr-x 2 vuXXXX www-data 303104 Jun 23 03:58 compiled
$

Minimum is 4k here (at least with #EXT4). This means that each time the directory is accessed (I guess the system call stat() counts in, too), it has to be read all over again which can take more and more time.

So I went out and added $this->smarty->setUseSubDirs(true); to #Friendica code.

Friendica Developers reshared this.

in reply to Roland Häder🇩🇪

As it looks like you cannot use to big varbinary() and also no text column type for columns url and preview as the #Antelope engine of #InnoDB might be still configured with user's installations and these both may break their installation or slows it down or not enough data for the unique (SQL) key.

This is very unfortunate! I guess there needs to be implemented something what @Hypolite Petovan has suggested, with only a hash of the URLs being stored (and referenced to the actual URLs?).

Friendica Developers reshared this.