Fix error on local user replies to local users

[e-five256]

When fetching the AP for a local user replying to a local user, a 500 was being thrown:

Error:
Call to a member function modify() on null

  at src/Service/ActivityPubManager.php:134
  at App\Service\ActivityPubManager->findActorOrCreate('@user1')
     (src/Service/ActivityPub/Wrapper/MentionsWrapper.php:29)
  at App\Service\ActivityPub\Wrapper\MentionsWrapper->build(array('@user1'), 'remote entry comment')
     (src/Factory/ActivityPub/EntryCommentNoteFactory.php:71)
  at App\Factory\ActivityPub\EntryCommentNoteFactory->create(object(EntryComment), true)
     (src/Controller/ActivityPub/EntryCommentController.php:41)
  at App\Controller\ActivityPub\EntryCommentController->__invoke(object(Magazine), object(Entry), object(EntryComment), object(Request))
     (vendor/symfony/http-kernel/HttpKernel.php:181)
  at Symfony\Component\HttpKernel\HttpKernel->handleRaw(object(Request), 1)
     (vendor/symfony/http-kernel/HttpKernel.php:76)
  at Symfony\Component\HttpKernel\HttpKernel->handle(object(Request), 1, true)
     (vendor/symfony/http-kernel/Kernel.php:197)
  at Symfony\Component\HttpKernel\Kernel->handle(object(Request))
     (vendor/symfony/runtime/Runner/Symfony/HttpKernelRunner.php:35)
  at Symfony\Component\Runtime\Runner\Symfony\HttpKernelRunner->run()
     (vendor/autoload_runtime.php:29)
  at require_once('/var/www/mbin/vendor/autoload_runtime.php')
     (public/index.php:7)

this line

mbin/src/Service/ActivityPubManager.php

Line 134 in 823c83c

if ($user->apFetchedAt->modify('+1 hour') < (new \DateTime())) {

was being run on local users, and modify() was throwing an error when apFetchedAt was null. this check makes it only emit the dispatch if the user is remote

I'm not sure if I should instead check isset($user->apFetchedAt), I went with the usual sentinel value for whether a user is remote or not instead, since we also access apProfileId

[melroy89]

Could you also check on line (notice the OR and not AND):

mbin/src/Service/ActivityPubManager.php

Line 167 in 57a816d

if (!$user->apFetchedAt || $user->apFetchedAt->modify('+1 hour') < (new \DateTime())) {

?

[e-five256]

This method is really difficult to read...

I think by this point

mbin/src/Service/ActivityPubManager.php

Lines 145 to 148 in 823c83c

	if (\in_array(
	parse_url($actorUrl, PHP_URL_HOST),
	[$this->settingsManager->get('KBIN_DOMAIN'), 'localhost', '127.0.0.1']
	)) {

that checks if the user is local and if so, return, meaning anything after it knows the user isn't local

so

mbin/src/Service/ActivityPubManager.php

Line 167 in 823c83c

if (!$user->apFetchedAt || $user->apFetchedAt->modify('+1 hour') < (new \DateTime())) {

is fine because it fires off if the user hasn't been fetched at all or not recently, and we know they have an apProfileId because if they were local it would've returned earlier

but above all those checks where this was happening I don't think we know whether the user is local or remote yet. I am assuming calling

mbin/src/Service/ActivityPubManager.php

Line 135 in 823c83c

$this->bus->dispatch(new UpdateActorMessage($user->apProfileId));

on remote users is pointless/should be avoided, but I could be mistaken

[e-five256]

I think you're right that it should have the same check for having never been fetched, at the very least to match the way it's done later. Assuming my reading of the code was correct, which is highly possibly not, the only difference here is we don't know if it's a local or remote user returned yet, but later we do so don't need the check...

I could really use some comments on figuring out what this method is doing. Like I think I understand the, if actor url is local, lookup local user and return. Next it does the remote check and updates if necessary. But what is this one I'm change the line in. What is it even for? It also looks up the user. Why was it needed compared to the two below it. There's three different $this->userRepository->findOneBy but only two of them make sense to me 🤔

[e-five256]

It now matches more closely to

mbin/src/Service/ActivityPubManager.php

Lines 666 to 668 in 823c83c

	if ($magazine and $magazine->apId and $magazine->apFetchedAt->modify('+1 Day') < (new \DateTime())) {
	$this->bus->dispatch(new UpdateActorMessage($magazine->apPublicUrl));
	}

which makes me feel slightly more confident. Though that doesn't have a check for never been fetched, hmm, it might always be called from methods for remote magazines

[BentiGorlich]

There are 3 findOneBy calls because a user can be specified like

• @user@domain -> username
• https://domain.tld/u/user (remote url) -> apProfileId
• or https://localhost/u/user (local url) -> remove url and -> username

I think you can remove one of the uodate actor dispatches because fhe user will always be local for it (the local url call)

[BentiGorlich]

Nevermind on the remove part.
LGTM, though I want to look at it in my IDE to be sure 😁