Skip to content

use new AP resolve code from SearchController in SearchRetrieveApi #2013

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
blued-gear wants to merge 5 commits into
base: main
Choose a base branch
from
Open

use new AP resolve code from SearchController in SearchRetrieveApi #2013

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5db735f
use new AP resolve code from SearchController in SearchRetrieveApi
blued-gear Feb 12, 2026
3a5dd9a
Merge branch 'main' into fix/api/unify-code-of-both-ap-searches
blued-gear Mar 17, 2026
572c3eb
change the search API for a more unified result schema and upgrade th…
blued-gear Mar 18, 2026
41fa628
please the linter
blued-gear Mar 18, 2026
1674294
disable tests which require web
blued-gear Mar 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
114 changes: 88 additions & 26 deletions src/Controller/Api/Search/SearchRetrieveApi.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,12 @@

namespace App\Controller\Api\Search;

use App\ActivityPub\ActorHandle;
use App\Controller\Api\BaseApi;
use App\Controller\Traits\PrivateContentTrait;
use App\Entity\Contracts\ContentInterface;
use App\Entity\Magazine;
use App\Entity\User;
use App\Factory\MagazineFactory;
use App\Factory\UserFactory;
use App\Repository\SearchRepository;
Expand Down Expand Up @@ -136,6 +139,7 @@ public function __invoke(
$headers = $this->rateLimit($apiReadLimiter, $anonymousApiReadLimiter);

$request = $this->request->getCurrentRequest();
/** @var string $q */
$q = $request->get('q');
if (null === $q) {
throw new BadRequestHttpException();
Expand All @@ -155,46 +159,104 @@ public function __invoke(
$items = $manager->findPaginated($this->getUser(), $q, $page, $perPage, authorId: $authorId, magazineId: $magazineId, specificType: $type);
$dtos = [];
foreach ($items->getCurrentPageResults() as $value) {
\assert($value instanceof ContentInterface);
array_push($dtos, $this->serializeContentInterface($value));
// TODO here we have two options
Copy link
Collaborator Author

@blued-gear blued-gear Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please give me some feedback

Copy link
Member

@jwr1 jwr1 Feb 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand what the change to the API is here. Doesn't the search API already return federated users and magazines?

Copy link
Collaborator Author

@blued-gear blued-gear Feb 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oddly enough no, not in the normal result items. Before my change it asserted, that the items can only be of ContentInterface and the OpenAPI schema lists these contents:

new OA\Schema(ref: new Model(type: EntryResponseDto::class)),
        new OA\Schema(ref: new Model(type: EntryCommentResponseDto::class)),
        new OA\Schema(ref: new Model(type: PostResponseDto::class)),
        new OA\Schema(ref: new Model(type: PostCommentResponseDto::class)),

Copy link
Member

@jwr1 jwr1 Feb 13, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the API docs, it looks like there's an apActors property returned from /search, which can be either a user or a magazine. Then it looks like there's an apObjects property which can be a thread, thread comment, microblog, or microblog comment.

I'm obviously not looking at the code though, so maybe the API docs are incorrect?

Copy link
Collaborator Author

@blued-gear blued-gear Feb 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are correct about the items not containing users or magazines. But the content of the returned object is a bit weird. As far as I understood it from the code the items contain content which matched by text, apActors contain users and magazines fetched via searched handle or AP url and apObjects contain content fetched via AP url.

Honestly, I'm in favor to rework the whole search API to make it more consistent and easy to understand, but breaking an unversioned API is probably not the best idea.

Copy link
Member

@jwr1 jwr1 Feb 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, gotcha. Well, I'm fine keeping it how it is or changing it. It would be nice if the Mbin API were versioned. Considering it's not in wide use (that I know of), you could just make the breaking change, and we can fix Interstellar to work with the change afterwards.

Copy link
Collaborator Author

@blued-gear blued-gear Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed the API to make it more consistent with other newer APIs we have. I still needed to keep one extra prop in the response for results of handle and url searches, as these are not paginated and so must be kept separately from items.

// A: skip non-content values
// B: add User and Magazine to the schema of the response; this might be a breaking API change
if ($value instanceof ContentInterface) {
$dtos[] = $this->serializeContentInterface($value);
} elseif ($value instanceof User) {
$dtos[] = $this->serializeUser($userFactory->createDto($value));
} elseif ($value instanceof Magazine) {
$dtos[] = $this->serializeMagazine($magazineFactory->createDto($value));
} else {
$this->logger->error('Unexpected result type: '.\get_class($value));
}
}

$response = $this->serializePaginated($dtos, $items);

$response['apActors'] = [];
$response['apObjects'] = [];
$actors = [];
$objects = [];
if (!$settingsManager->get('KBIN_FEDERATED_SEARCH_ONLY_LOGGEDIN') || $this->getUser()) {
$actors = $manager->findActivityPubActorsByUsername($q);
$objects = $manager->findActivityPubObjectsByURL($q);
}
if ($this->federatedSearchAllowed()) {
if ($handle = ActorHandle::parse($q)) {
$actors = $manager->findActivityPubActorsByUsername($handle);
$response['apActors'] = $this->transformActors($actors, $userFactory, $magazineFactory);
}

foreach ($actors as $actor) {
switch ($actor['type']) {
case 'user':
$response['apActors'][] = [
'type' => 'user',
'object' => $this->serializeUser($userFactory->createDto($actor['object'])),
];
break;
case 'magazine':
$response['apActors'][] = [
'type' => 'magazine',
'object' => $this->serializeMagazine($magazineFactory->createDto($actor['object'])),
];
break;
$objects = $manager->findActivityPubObjectsByURL($q);
foreach ($objects['errors'] as $error) {
/** @var \Exception $error */
$this->logger->warning(
'Exception while resolving URL {url}: {type}: {msg}',
[
'url' => $q,
'type' => \get_class($error),
'msg' => $error->getMessage(),
]
);
}
}

foreach ($objects as $object) {
\assert($object instanceof ContentInterface);
$response['apObjects'][] = $this->serializeContentInterface($object);
$transformedObjects = $this->transformObjects($objects['results'], $userFactory, $magazineFactory);
$response['apActors'] = [...$response['apActors'], ...$transformedObjects['actors']];
$response['apObjects'] = $transformedObjects['content'];
}

return new JsonResponse(
$response,
headers: $headers
);
}

private function federatedSearchAllowed(): bool
{
return !$this->settingsManager->get('KBIN_FEDERATED_SEARCH_ONLY_LOGGEDIN')
|| $this->getUser();
}

private function transformActors(array $actors, UserFactory $userFactory, MagazineFactory $magazineFactory): array
{
$ret = [];
foreach ($actors as $actor) {
$ret[] = $this->transformActor($actor, $userFactory, $magazineFactory);
}

return $ret;
}

private function transformActor(array $actor, UserFactory $userFactory, MagazineFactory $magazineFactory): array
{
return match ($actor['type']) {
'user' => [
'type' => 'user',
'object' => $this->serializeUser($userFactory->createDto($actor['object'])),
],
'magazine' => [
'type' => 'magazine',
'object' => $this->serializeMagazine($magazineFactory->createDto($actor['object'])),
],
default => throw new \LogicException('Unexpected actor type: '.$actor['type']),
};
}

private function transformObjects(array $objects, UserFactory $userFactory, MagazineFactory $magazineFactory): array
{
$actors = [];
$content = [];
foreach ($objects as $object) {
if ('user' === $object['type'] || 'magazine' === $object['type']) {
$actors[] = $this->transformActor($object, $userFactory, $magazineFactory);
} elseif ('subject' === $object['type']) {
$subject = $object['object'];
\assert($subject instanceof ContentInterface);
$content[] = $this->serializeContentInterface($subject);
} else {
throw new \LogicException('Unexpected actor type: '.$object['type']);
}
}

return [
'actors' => $actors,
'content' => $content,
];
}
}
107 changes: 6 additions & 101 deletions src/Controller/SearchController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,30 +6,20 @@

use App\ActivityPub\ActorHandle;
use App\DTO\SearchDto;
use App\Entity\Magazine;
use App\Entity\User;
use App\Form\SearchType;
use App\Message\ActivityPub\Inbox\CreateMessage;
use App\Service\ActivityPub\ApHttpClientInterface;
use App\Service\ActivityPubManager;
use App\Service\SearchManager;
use App\Service\SettingsManager;
use Psr\Log\LoggerInterface;
use Symfony\Component\HttpFoundation\JsonResponse;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\Messenger\MessageBusInterface;
use Symfony\Component\Messenger\Stamp\TransportNamesStamp;

class SearchController extends AbstractController
{
public function __construct(
private readonly SearchManager $manager,
private readonly ActivityPubManager $activityPubManager,
private readonly ApHttpClientInterface $apHttpClient,
private readonly SettingsManager $settingsManager,
private readonly LoggerInterface $logger,
private readonly MessageBusInterface $bus,
) {
}

Expand All @@ -51,7 +41,7 @@ public function __invoke(Request $request): Response
if (str_contains($query, '@') && $this->federatedSearchAllowed()) {
if ($handle = ActorHandle::parse($query)) {
$this->logger->debug('searching for a matched webfinger {query}', ['query' => $query]);
$objects = $this->lookupHandle($handle);
$objects = $this->manager->findActivityPubActorsByUsername($handle);
} else {
$this->logger->debug("query doesn't look like a valid handle...", ['query' => $query]);
}
Expand Down Expand Up @@ -107,100 +97,15 @@ private function federatedSearchAllowed(): bool
|| $this->getUser();
}

private function lookupHandle(ActorHandle $handle): array
{
$objects = [];
$name = $handle->plainHandle();

try {
$webfinger = $this->activityPubManager->webfinger($name);
foreach ($webfinger->getProfileIds() as $profileId) {
$this->logger->debug('Found "{profileId}" at "{name}"', ['profileId' => $profileId, 'name' => $name]);

// if actor object exists or successfully created
$object = $this->activityPubManager->findActorOrCreate($profileId);
if (!empty($object)) {
if ($object instanceof Magazine) {
$type = 'magazine';
} elseif ($object instanceof User && '!' !== $handle->prefix) {
$type = 'user';
}

$objects[] = [
'type' => $type,
'object' => $object,
];
}
}
} catch (\Exception $e) {
$this->logger->warning(
'an error occurred during webfinger lookup of "{handle}": {exceptionClass}: {message}',
[
'handle' => $name,
'exceptionClass' => \get_class($e),
'message' => $e->getMessage(),
]
);
}

return $objects;
}

private function findObjectsByApUrl(string $url): array
{
$objects = $this->manager->findByApId($url);
if (0 === \sizeof($objects)) {
// the url could resolve to a different id.
try {
$body = $this->apHttpClient->getActivityObject($url);
$apId = $body['id'];
$objects = $this->manager->findByApId($apId);
} catch (\Exception $e) {
$body = null;
$apId = $url;
$this->addFlash('error', $e->getMessage());
}

if (0 === \sizeof($objects) && null !== $body) {
// maybe it is an entry, post, etc.
try {
// process the message in the sync transport, so that the created content is directly visible
$this->bus->dispatch(new CreateMessage($body), [new TransportNamesStamp('sync')]);
$objects = $this->manager->findByApId($apId);
} catch (\Exception $e) {
$this->addFlash('error', $e->getMessage());
}
}
$result = $this->manager->findActivityPubObjectsByURL($url);

if (0 === \sizeof($objects)) {
// maybe it is a magazine or user
try {
$this->activityPubManager->findActorOrCreate($apId);
$objects = $this->manager->findByApId($apId);
} catch (\Exception $e) {
$this->addFlash('error', $e->getMessage());
}
}
foreach ($result['errors'] as $error) {
/** @var \Exception $error */
$this->addFlash('error', $error->getMessage());
}

return $this->mapApResultsToViewModel($objects);
}

private function mapApResultsToViewModel(array $objects): array
{
return array_map(function ($object) {
if ($object instanceof Magazine) {
$type = 'magazine';
} elseif ($object instanceof User) {
$type = 'user';
} else {
$type = 'subject';
}

return [
'type' => $type,
'object' => $object,
];
}, $objects);
return $result['results'];
}
}
Loading
Loading