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).
Philipp Holzer likes this.
Michael 🇺🇦
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 andnull
then the type hint has got to look slightly different:..., int $uid = null, ...
.Roland Häder🇩🇪
in reply to Michael 🇺🇦 • • •$foo['something']
I assume that$foo
is type ofarray
and not something else. This type of code with multiple types has made it so hard to enforce strong type-hints.Roland Häder🇩🇪
in reply to Roland Häder🇩🇪 • • •@Michael Vogel (aka. annando on github) is bringing up a valid point:
github.com/friendica/friendica…
As I don't want to make such choice all alone, what is your opinion on this?
Roland Häder🇩🇪
in reply to Roland Häder🇩🇪 • • •Ops! My bad.
Roland Häder🇩🇪
in reply to Roland Häder🇩🇪 • • •Roland Häder🇩🇪
in reply to Roland Häder🇩🇪 • • •I'm currently seeing this in my fpm-log:
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.Roland Häder🇩🇪
in reply to Roland Häder🇩🇪 • • •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?Roland Häder🇩🇪
in reply to Roland Häder🇩🇪 • • •$profile
. There is only one inside anif()
block. In other languages such #Java this won't be allowed because the otherif()
block (where the variable is used) will be out of scope of theif()
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.Roland Häder🇩🇪
in reply to Roland Häder🇩🇪 • • •$profile
into the declaration list in line 883. Let's see if that error comes back, if so the said invocation of$profile = APContact::getByURL($actor);
then has failed.Roland Häder🇩🇪
in reply to Roland Häder🇩🇪 • • •Protocol/Diaspora.php
return an empty array orfalse
on error. Can we make them all the same on error behavior and avoid the 2 returned types?Roland Häder🇩🇪
in reply to Roland Häder🇩🇪 • • •Diaspora::buildEvent()
can return an empty array on failure but many others cannot?Roland Häder🇩🇪
in reply to Roland Häder🇩🇪 • • •object
which I cannot use as a type-hint. So I have to add some debugging lines that logsget_class($object)
to the log file. I will later remove these lines when I have the proper type.Michael 🇺🇦
in reply to Roland Häder🇩🇪 • • •Roland Häder🇩🇪
in reply to Michael 🇺🇦 • • •@Michael Vogel So I rather change these lines back to
return false;
and remove the returned type-hintarray
from it?Michael 🇺🇦
in reply to Roland Häder🇩🇪 • • •Roland Häder🇩🇪
in reply to Michael 🇺🇦 • • •return false;
as other methods heavily rely on it (e.g. checks with!is_array($msg)
).Roland Häder🇩🇪
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 returningfalse
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).
Roland Häder🇩🇪
in reply to Michael 🇺🇦 • • •false
, sometimesnull
. It feels more like programming in old days with procedures and not the modern way with exceptions and lesser returned error codes/types.Roland Häder🇩🇪
in reply to Roland Häder🇩🇪 • • •TypeError
messages for several hours. I think I can safely assume that all wrong type-hints has been fixed and bad invocations located. The only error messages I can see are about "Maximum function nesting level of 256 reached" and some DB errors (overlong URLs and to big values forsize
field).Roland Häder🇩🇪
in reply to Roland Häder🇩🇪 • • •Roland Häder🇩🇪
in reply to Roland Häder🇩🇪 • • •The reason why I don't like to contribute much to #Friendica or any other project when your ticket is being abused for off-topic changes, like changing double-quotes to single or next rewriting log messages.
github.com/friendica/friendica…
Roland Häder🇩🇪
in reply to Roland Häder🇩🇪 • • •Roland Häder🇩🇪
in reply to Roland Häder🇩🇪 • • •Of course I understand @Hypolite Petovan here wanting to have a clean file (here:
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.
src/Protocol/Diaspora.php
). I totally understand it.So how do I do with
DI::logger->level('Message with parameters', 'foo' => $foo, 'bar' => $bar);
here?Roland Häder🇩🇪
in reply to Roland Häder🇩🇪 • • •src/Model/Profile.php
an out-datedApp $a
parameter. How can I rewrite this?Roland Häder🇩🇪
in reply to Roland Häder🇩🇪 • • •DI::logger->level('foo', ['foo' => $foo]);
is correct? @Hypolite PetovanPhilipp Holzer likes this.
Philipp Holzer
in reply to Roland Häder🇩🇪 • •DI::logger()->level('foo' ['foo' => $foo]);
Roland Häder🇩🇪 likes this.
Friendica Developers reshared this.
Roland Häder🇩🇪
in reply to Philipp Holzer • • •,
.Philipp Holzer likes this.
Friendica Developers reshared this.
Roland Häder🇩🇪
in reply to Roland Häder🇩🇪 • • •After 10 commits and 16 changed files, I have only the
from the worker process in log file. But I let it run longer to see if everything went okay.
Roland Häder🇩🇪
in reply to Roland Häder🇩🇪 • • •Roland Häder🇩🇪
in reply to Roland Häder🇩🇪 • • •DB Error
because ofsite_name
is to short (or their instance's name is to long) and the "usual" error from worker.Roland Häder🇩🇪
in reply to Roland Häder🇩🇪 • • •fixes/more-type-hints-003
is the new branch.Roland Häder🇩🇪
in reply to Roland Häder🇩🇪 • • •-
DBA::exists()
should only be used for checking if records exists.- if you want to check if a table exists, please ALWAYS use
DBStructure::existsTable()
instead-
Strings::isHex()
should not be misused for checking onNULL
Roland Häder🇩🇪
in reply to Roland Häder🇩🇪 • • •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?Roland Häder🇩🇪
in reply to Roland Häder🇩🇪 • • •Friendica Developers reshared this.
Roland Häder🇩🇪
in reply to Roland Häder🇩🇪 • • •I have noticed that
view/smarty3/compiled
's descriptor is getting very large: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.
Roland Häder🇩🇪
in reply to Roland Häder🇩🇪 • • •view/smarty3/compiled/
is now down at 12k instead of 296k.Friendica Developers reshared this.
Roland Häder🇩🇪
in reply to Roland Häder🇩🇪 • • •Friendica Developers reshared this.
Roland Häder🇩🇪
in reply to Roland Häder🇩🇪 • • •Now the
FIXED: Just needed to pressfavicon.ico
isn't shown anymore ...CTRL+F5
.Friendica Developers reshared this.
Roland Häder🇩🇪
in reply to Roland Häder🇩🇪 • • •As it looks like you cannot use to big
varbinary()
and also notext
column type for columnsurl
andpreview
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.
Roland Häder🇩🇪
in reply to Roland Häder🇩🇪 • • •Friendica Developers reshared this.
Michael 🇺🇦
in reply to Roland Häder🇩🇪 • • •Roland Häder🇩🇪 likes this.
Friendica Developers reshared this.
Roland Häder🇩🇪
in reply to Michael 🇺🇦 • • •Friendica Developers reshared this.
Roland Häder🇩🇪
in reply to Roland Häder🇩🇪 • • •Friendica Developers reshared this.
Roland Häder🇩🇪
in reply to Roland Häder🇩🇪 • • •Image
andUrl
nodes contain no data. Also I tried to enforce UTF-8 as shown here. But it also doesn't show up in the generated XML. Any ideas?Friendica Developers reshared this.
Roland Häder🇩🇪
in reply to Roland Häder🇩🇪 • • •$ rm -rf view/smarty3/compiled && mkdir view/smarty3/compiled
to reduce the descriptor size to 4k again.Friendica Developers reshared this.