Skip to content

use new AP resolve code from SearchController in SearchRetrieveApi#2013

Open
blued-gear wants to merge 5 commits intomainfrom
fix/api/unify-code-of-both-ap-searches
Open

use new AP resolve code from SearchController in SearchRetrieveApi#2013
blued-gear wants to merge 5 commits intomainfrom
fix/api/unify-code-of-both-ap-searches

Conversation

@blued-gear
Copy link
Collaborator

@blued-gear blued-gear commented Feb 12, 2026

This PR modifies the search API endpoint to

  • use the new functionality of SearchManager
  • return EntryComment, PostComment, Magazine and user additionally to the previous result types
  • use a DTO like in the ContentApi to differentiate result item types
  • clean up search by handle and URL

This change is breaking to the current REST-API contract.

New API for #1001

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

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

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

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

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

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

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

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.

@blued-gear blued-gear self-assigned this Mar 8, 2026
@blued-gear blued-gear marked this pull request as ready for review March 18, 2026 07:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants