-
-
Notifications
You must be signed in to change notification settings - Fork 6.9k
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
Hook up URL-based resource look-up to ActivityPub #4589
Conversation
end | ||
|
||
return nil if atom_url.nil? | ||
process_atom(atom_url, body) | ||
case protocol |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
case protocol
-> case protocol
(remove trailing whitespace)
end | ||
|
||
return nil if atom_url.nil? | ||
process_atom(atom_url, body) | ||
case protocol |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
case protocol
-> case protocol
(remove trailing whitespace)
3834fa4
to
9716918
Compare
return [url, fetch(url)] if response.mime_type == 'application/atom+xml' | ||
return process_headers(url, response) if response['Link'].present? | ||
process_html(fetch(url)) | ||
perform_request |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So what is happening here:
Before | After |
---|---|
First, HEAD request to inspect headers, check mime type, if it fits - do GET request, if not, check Link header, GET from it, if there is no Link header, GET the page and try to find a link in the HTML | Do a GET request, check mime type - if it fits, fine, if not, check headers, if no headers, check HTML. If something is in headers or HTML, do another GET to that |
In the best case that means HEAD + GET have become a single GET, in the worst case, HEAD + GET + GET has become GET + GET. Only in case of 404 it has become GET instead of HEAD, which miiiiight be a heavier request, but unless the body is actually read, maybe it doesn't matter.
app/services/fetch_atom_service.rb
Outdated
def process_html(body) | ||
Rails.logger.debug 'Processing HTML' | ||
def perform_request | ||
@response = Request.new(:get, @url).perform |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doesn't this need to have Accept: activity+json, application/ld+json; profile="whatever"
to actually get an activitypub resource?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're actually right, thanks.
9716918
to
083b7cf
Compare
No description provided.