Skip to content
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.

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

Make resolve_object not require auth #3685 #3716

Merged

Conversation

marsara9
Copy link
Contributor

Resolves issue #3685

If user isn't authenticated with resolve_object, only allow a local search instead of possibly making an http request.

Changing resolve_object to take an optional auth parameter. When auth is null, the request is only allowed to search the local database rather than do a remote lookup.

This is primarily to resolve a few issue:

  1. In my own project https://www.github.com/marsara9/lemmy-search using /api/v3/post... on every Lemmy instance in the fediverse, is extremely slow. Current estimate, is around ~13 years worst case, if no new posts get added.
  2. This will be first step in enabling lemmy-ui to be able to rewrite links to post to always be in the user's home instance Instance agnostic links #2987.

Original PR for reference: #3706 (as it was easier to decline that one and open a new one with the requested changes).

If user isn't authenticated with resolve_object, only allow a local search instead of possibly making an http request.
@marsara9
Copy link
Contributor Author

It doesn't look like this is actually blocking invalid tokens. I'll push an update tonight to fix that.

@marsara9
Copy link
Contributor Author

Fixed the issue where I wasn't properly locking invalid tokens.

context: &Data<LemmyContext>,
) -> Result<SearchableObjects, LemmyError> {
let url = Url::parse(query)?;
ObjectId::from(url).dereference_local(context).await
Copy link
Member

@Nutomic Nutomic Jul 26, 2023

Choose a reason for hiding this comment

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

You can directly call ObjectId::parse which calls Url::parse internally. And in fact it doesnt make much sense to have a separate function for this single line. Otherwise looks good.

@@ -84,7 +84,7 @@ pub struct SearchResponse {
pub struct ResolveObject {
/// Can be the full url, or a shortened version like: !fediverse@lemmy.ml
pub q: String,
pub auth: Sensitive<String>,
pub auth: Option<Sensitive<String>>,
Copy link
Member

Choose a reason for hiding this comment

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

API change, but it shouldn't be breaking.

@dessalines dessalines merged commit dc45724 into LemmyNet:main Jul 26, 2023
1 check passed
Nutomic pushed a commit that referenced this pull request Jul 27, 2023
* Resolves issue #3685

If user isn't authenticated with resolve_object, only allow a local search instead of possibly making an http request.

* Making sure to validate auth before doing a potential remote lookup.
@marsara9 marsara9 deleted the feature/resolve-object-auth-changes branch July 29, 2023 17:32
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.

None yet

3 participants