Skip to content

Commit

Permalink
Fix old cache being used and the random mag bug
Browse files Browse the repository at this point in the history
- re fetch magazines when a post is added to them and the last fetch was at least 1 day ago
- fix bug where posts would get put in the random mag, because it was not present on the instance. Now the remote mag is added and the post is put there
- delete cached ap responses when a moderator was added/removed
- fix exception in FollowHandler
- fix exception in ApHttpClient
- add logging to SearchController
  • Loading branch information
BentiGorlich committed Nov 23, 2023
1 parent 7236e10 commit 475ed19
Show file tree
Hide file tree
Showing 7 changed files with 89 additions and 25 deletions.
12 changes: 11 additions & 1 deletion src/Controller/SearchController.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
use App\Service\SettingsManager;
use App\Service\SubjectOverviewManager;
use App\Utils\RegPatterns;
use Psr\Log\LoggerInterface;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\Messenger\MessageBusInterface;
Expand All @@ -25,7 +26,8 @@ public function __construct(
private readonly MessageBusInterface $bus,
private readonly ApHttpClient $apHttpClient,
private readonly SubjectOverviewManager $overviewManager,
private readonly SettingsManager $settingsManager
private readonly SettingsManager $settingsManager,
private readonly LoggerInterface $logger,
) {
}

Expand All @@ -44,14 +46,17 @@ public function __invoke(Request $request): Response
);
}

$this->logger->debug("searching for $query");
$objects = [];
if (str_contains($query, '@') && (!$this->settingsManager->get('KBIN_FEDERATED_SEARCH_ONLY_LOGGEDIN') || $this->getUser())) {
$name = str_starts_with($query, '@') ? $query : '@'.$query;
preg_match(RegPatterns::AP_USER, $name, $matches);
if (\count(array_filter($matches)) >= 4) {
$this->logger->debug("searching for a matched webfinger $query");
try {
$webfinger = $this->activityPubManager->webfinger($name);
foreach ($webfinger->getProfileIds() as $profileId) {
$this->logger->debug("Found '$profileId' at '$name'");
$object = $this->activityPubManager->findActorOrCreate($profileId);
// Check if object is not empty
if (!empty($object)) {
Expand All @@ -68,8 +73,13 @@ public function __invoke(Request $request): Response
}
}
} catch (\Exception $e) {
$this->logger->warning("an error occurred during lookup of '$query': ".\get_class($e).": {$e->getMessage()}", [$e]);
}
} else {
$this->logger->debug("query doesn't match the pattern...", [$matches]);
}
} else {
$this->logger->debug('something bla bla bla', [str_contains($query, '@'), !$this->settingsManager->get('KBIN_FEDERATED_SEARCH_ONLY_LOGGEDIN'), $this->getUser()]);
}

if (false !== filter_var($query, FILTER_VALIDATE_URL)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,23 @@

namespace App\EventSubscriber\ActivityPub;

use App\Entity\Magazine;
use App\Event\Magazine\MagazineModeratorAddedEvent;
use App\Event\Magazine\MagazineModeratorRemovedEvent;
use App\Message\ActivityPub\Outbox\AddMessage;
use App\Message\ActivityPub\Outbox\RemoveMessage;
use Psr\Cache\InvalidArgumentException;
use Psr\Log\LoggerInterface;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Symfony\Component\Messenger\MessageBusInterface;
use Symfony\Contracts\Cache\CacheInterface;

class MagazineModeratorAddedRemovedSubscriber implements EventSubscriberInterface
{
public function __construct(
private readonly MessageBusInterface $bus,
private readonly CacheInterface $cache,
private readonly LoggerInterface $logger,
) {
}

Expand All @@ -24,6 +30,7 @@ public function onModeratorAdded(MagazineModeratorAddedEvent $event): void
if (!$event->magazine->apId or (null !== $event->addedBy and !$event->addedBy->apId)) {
$this->bus->dispatch(new AddMessage($event->addedBy->getId(), $event->magazine->getId(), $event->user->getId()));
}
$this->deleteCache($event->magazine);
}

public function onModeratorRemoved(MagazineModeratorRemovedEvent $event): void
Expand All @@ -32,6 +39,7 @@ public function onModeratorRemoved(MagazineModeratorRemovedEvent $event): void
if (!$event->magazine->apId or (null !== $event->removedBy and !$event->removedBy->apId)) {
$this->bus->dispatch(new RemoveMessage($event->removedBy->getId(), $event->magazine->getId(), $event->user->getId()));
}
$this->deleteCache($event->magazine);
}

public static function getSubscribedEvents(): array
Expand All @@ -41,4 +49,16 @@ public static function getSubscribedEvents(): array
MagazineModeratorRemovedEvent::class => 'onModeratorRemoved',
];
}

private function deleteCache(Magazine $magazine): void
{
try {
$this->cache->delete('ap_'.hash('sha256', $magazine->apProfileId));
$this->cache->delete('ap_'.hash('sha256', $magazine->apId));
$this->cache->delete('ap_'.hash('sha256', $magazine->apAttributedToUrl));
$this->cache->delete('ap_collection'.hash('sha256', $magazine->apAttributedToUrl));
} catch (InvalidArgumentException $e) {
$this->logger->warning("There was an error while clearing the cache for magazine '{$magazine->name}' ({$magazine->getId()})");
}
}
}
10 changes: 6 additions & 4 deletions src/MessageHandler/ActivityPub/Inbox/FollowHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,12 @@ public function __invoke(FollowMessage $message)
);
break;
case 'Accept':
$this->handleAccept(
$actor,
$this->activityPubManager->findActorOrCreate($message->payload['object']['actor'])
);
if ($actor instanceof User) {
$this->handleAccept(
$actor,
$this->activityPubManager->findActorOrCreate($message->payload['object']['actor'])
);
}
break;
case 'Reject':
$this->handleReject(
Expand Down
20 changes: 14 additions & 6 deletions src/Service/ActivityPub/ApHttpClient.php
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ public function getWebfingerObject(string $url): ?array
'wf_'.hash('sha256', $url),
function (ItemInterface $item) use ($url) {
$this->logger->debug("ApHttpClient:getWebfingerObject:url: {$url}");

$r = null;
try {
$client = new CurlHttpClient();
$r = $client->request('GET', $url, [
Expand All @@ -99,7 +99,11 @@ function (ItemInterface $item) use ($url) {
'headers' => $this->getInstanceHeaders($url, null, 'get', ApRequestType::WebFinger),
]);
} catch (\Exception $e) {
throw new InvalidApPostException("WebFinger Get fail: {$url}, ".$r->getContent(false));
$msg = "WebFinger Get fail: {$url}, ex: " . get_class($e) . ": {$e->getMessage()}";
if(null !== $r) {
$msg .= ", " . $r->getContent(false);
}
throw new InvalidApPostException($msg);
}

$item->expiresAt(new \DateTime('+1 hour'));
Expand All @@ -122,7 +126,7 @@ public function getActorObject(string $apProfileId): ?array
'ap_'.hash('sha256', $apProfileId),
function (ItemInterface $item) use ($apProfileId) {
$this->logger->debug("ApHttpClient:getActorObject:url: {$apProfileId}");

$response = null;
try {
// Set-up request
$client = new CurlHttpClient();
Expand Down Expand Up @@ -153,7 +157,11 @@ function (ItemInterface $item) use ($apProfileId) {
$this->magazineRepository->save($magazine, true);
}

throw new InvalidApPostException("AP Get fail: {$apProfileId}, ".$response->getContent(false));
$msg = "AP Get fail: {$apProfileId}, ex: " . get_class($e) . ": {$e->getMessage()}";
if(null !== $response) {
$msg .= ", " . $response->getContent(false);
}
throw new InvalidApPostException($msg);
}

$item->expiresAt(new \DateTime('+1 hour'));
Expand Down Expand Up @@ -182,9 +190,9 @@ function (ItemInterface $item) use ($apAddress) {
'headers' => $this->getInstanceHeaders($apAddress, null, 'get', ApRequestType::ActivityPub),
]);
} catch (\Exception $e) {
$msg = "AP Get fail: {$apAddress}, ";
$msg = "AP Get fail: {$apAddress}, ex: " . get_class($e) . ": {$e->getMessage()}";
if (null !== $response) {
$msg .= $response->getContent(false);
$msg .= ", " . $response->getContent(false);
}
throw new InvalidApPostException($msg);
}
Expand Down
8 changes: 1 addition & 7 deletions src/Service/ActivityPub/Note.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
use App\Entity\User;
use App\Factory\ImageFactory;
use App\Repository\ApActivityRepository;
use App\Repository\MagazineRepository;
use App\Service\ActivityPubManager;
use App\Service\EntryCommentManager;
use App\Service\PostCommentManager;
Expand All @@ -32,7 +31,6 @@ public function __construct(
private readonly PostManager $postManager,
private readonly EntryCommentManager $entryCommentManager,
private readonly PostCommentManager $postCommentManager,
private readonly MagazineRepository $magazineRepository,
private readonly ActivityPubManager $activityPubManager,
private readonly EntityManagerInterface $entityManager,
private readonly MarkdownConverter $markdownConverter,
Expand Down Expand Up @@ -189,11 +187,7 @@ private function createPost(
array $object,
): ActivityPubActivityInterface {
$dto = new PostDto();
$dto->magazine = $this->magazineRepository->findByApGroupProfileId(
array_merge($object['to'], $object['cc'])
) ?? $this->magazineRepository->findOneByName(
'random'
);
$dto->magazine = $this->activityPubManager->findOrCreateMagazineByToAndCC($object);
$dto->apId = $object['id'];

$actor = $this->activityPubManager->findActorOrCreate($object['attributedTo']);
Expand Down
12 changes: 5 additions & 7 deletions src/Service/ActivityPub/Page.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,15 @@

namespace App\Service\ActivityPub;

use App\DTO\EntryCommentDto;
use App\DTO\EntryDto;
use App\DTO\PostCommentDto;
use App\DTO\PostDto;
use App\Entity\Contracts\ActivityPubActivityInterface;
use App\Entity\Contracts\VisibilityInterface;
use App\Entity\User;
use App\Factory\ImageFactory;
use App\Repository\ApActivityRepository;
use App\Repository\MagazineRepository;
use App\Service\ActivityPubManager;
use App\Service\EntryManager;
use App\Service\SettingsManager;
Expand All @@ -21,7 +23,6 @@ class Page
public function __construct(
private readonly ApActivityRepository $repository,
private readonly MarkdownConverter $markdownConverter,
private readonly MagazineRepository $magazineRepository,
private readonly EntryManager $entryManager,
private readonly ActivityPubManager $activityPubManager,
private readonly EntityManagerInterface $entityManager,
Expand Down Expand Up @@ -51,12 +52,9 @@ public function create(array $object): ActivityPubActivityInterface
$object['cc'] = [$object['cc']];
}

$magazine = $this->activityPubManager->findOrCreateMagazineByToAndCC($object);
$dto = new EntryDto();
$dto->magazine = $this->magazineRepository->findByApGroupProfileId(
array_merge($object['to'], $object['cc'])
) ?? $this->magazineRepository->findOneByName(
'random'
);
$dto->magazine = $magazine;
$dto->title = $object['name'];
$dto->apId = $object['id'];

Expand Down
32 changes: 32 additions & 0 deletions src/Service/ActivityPubManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,10 @@ public function createCcFromBody(string $body): array
*/
public function findActorOrCreate(string $actorUrlOrHandle): null|User|Magazine
{
$this->logger->debug("searching for actor at '$actorUrlOrHandle'");
if (str_contains($actorUrlOrHandle, $this->settingsManager->get('KBIN_DOMAIN').'/m/')) {
$magazine = str_replace('https://'.$this->settingsManager->get('KBIN_DOMAIN').'/m/', '', $actorUrlOrHandle);
$this->logger->debug("found magazine '$magazine'");

return $this->magazineRepository->findOneByName($magazine);
}
Expand All @@ -139,6 +141,8 @@ public function findActorOrCreate(string $actorUrlOrHandle): null|User|Magazine
$name = explode('/', $actorUrl);
$name = end($name);

$this->logger->debug("found user $name");

return $this->userRepository->findOneBy(['username' => $name]);
}

Expand All @@ -148,6 +152,7 @@ public function findActorOrCreate(string $actorUrlOrHandle): null|User|Magazine
// User (we don't make a distinction between bots with type Service as Lemmy does)
if (\in_array($actor['type'], self::USER_TYPES)) {
$user = $this->userRepository->findOneBy(['apProfileId' => $actorUrl]);
$this->logger->debug("found remote user at $actorUrl");
if (!$user) {
$user = $this->createUser($actorUrl);
} else {
Expand All @@ -166,6 +171,7 @@ public function findActorOrCreate(string $actorUrlOrHandle): null|User|Magazine
if ('Group' === $actor['type']) {
// User
$magazine = $this->magazineRepository->findOneBy(['apProfileId' => $actorUrl]);
$this->logger->debug("found magazine at $actorUrl");
if (!$magazine) {
$magazine = $this->createMagazine($actorUrl);
} else {
Expand Down Expand Up @@ -201,6 +207,7 @@ public function findUserActorOrCreateOrThrow(string $actorUrlOrHandle): User

public function webfinger(string $id): WebFinger
{
$this->logger->debug("fetching webfinger $id");
$this->webFingerFactory::setServer($this->server->create());

if (false === filter_var($id, FILTER_VALIDATE_URL)) {
Expand Down Expand Up @@ -588,4 +595,29 @@ public function updateActor(string $actorUrl): null|Magazine|User

return $this->updateMagazine($actorUrl);
}

public function findOrCreateMagazineByToAndCC(array $object): Magazine|null
{
$potentialGroups = array_filter(array_merge($object['to'], $object['cc']), fn ($s) => null !== $s and ActivityPubActivityInterface::PUBLIC_URL !== $s);
$magazine = $this->magazineRepository->findByApGroupProfileId($potentialGroups);
if ($magazine and $magazine->apId and $magazine->apFetchedAt->modify('+1 Day') < (new \DateTime())) {
$this->bus->dispatch(new UpdateActorMessage($magazine->apPublicUrl));
}

if (null === $magazine) {
foreach ($potentialGroups as $potentialGroup) {
$result = $this->findActorOrCreate($potentialGroup);
if ($result instanceof Magazine) {
$magazine = $result;
break;
}
}
}

if (null === $magazine) {
$magazine = $this->magazineRepository->findOneByName('random');
}

return $magazine;
}
}

0 comments on commit 475ed19

Please sign in to comment.