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

[bugfix] Tidy up rss feed serving; don't error on empty feed #1970

Merged
merged 3 commits into from
Jul 10, 2023

Conversation

tsmethurst
Copy link
Contributor

Description

If this is a code change, please include a summary of what you've coded, and link to the issue(s) it closes/implements.

If this is a documentation change, please briefly describe what you've changed and why.

This PR tidies up some of the very messy rss feed logic and adds comments for clarity.

Also allows RSS feeds to be served even when the account owner hasn't posted anything yet (closes #1951). In such a case, the result will be a feed that just doesn't have any items in it.

Example with items:

<?xml version="1.0" encoding="UTF-8"?><rss version="2.0" xmlns:content="http://purl.org/rss/1.0/modules/content/">
  <channel>
    <title>Posts from @the_mighty_zork@localhost:8080</title>
    <link>http://localhost:8080/@the_mighty_zork</link>
    <description>Posts from @the_mighty_zork@localhost:8080</description>
    <pubDate>Wed, 20 Oct 2021 10:40:37 +0000</pubDate>
    <lastBuildDate>Wed, 20 Oct 2021 10:40:37 +0000</lastBuildDate>
    <image>
      <url>http://localhost:8080/fileserver/01F8MH1H7YV1Z7D2C8K2730QBF/avatar/small/01F8MH58A357CV5K7R7TJMSH6S.jpg</url>
      <title>Avatar for @the_mighty_zork@localhost:8080</title>
      <link>http://localhost:8080/@the_mighty_zork</link>
    </image>
    <item>
      <title>introduction post</title>
      <link>http://localhost:8080/@the_mighty_zork/statuses/01F8MHAMCHF6Y650WCRSCP4WMY</link>
      <description>@the_mighty_zork@localhost:8080 made a new post: &#34;hello everyone!&#34;</description>
      <content:encoded><![CDATA[hello everyone!]]></content:encoded>
      <author>@the_mighty_zork@localhost:8080</author>
      <guid>http://localhost:8080/@the_mighty_zork/statuses/01F8MHAMCHF6Y650WCRSCP4WMY</guid>
      <pubDate>Wed, 20 Oct 2021 10:40:37 +0000</pubDate>
      <source>http://localhost:8080/@the_mighty_zork/feed.rss</source>
    </item>
  </channel>
</rss>

Example with no items:

<?xml version="1.0" encoding="UTF-8"?><rss version="2.0" xmlns:content="http://purl.org/rss/1.0/modules/content/">
  <channel>
    <title>Posts from @the_mighty_zork@localhost:8080</title>
    <link>http://localhost:8080/@the_mighty_zork</link>
    <description>Posts from @the_mighty_zork@localhost:8080</description>
    <pubDate>Fri, 20 May 2022 11:09:18 +0000</pubDate>
    <lastBuildDate>Fri, 20 May 2022 11:09:18 +0000</lastBuildDate>
    <image>
      <url>http://localhost:8080/fileserver/01F8MH1H7YV1Z7D2C8K2730QBF/avatar/small/01F8MH58A357CV5K7R7TJMSH6S.jpg</url>
      <title>Avatar for @the_mighty_zork@localhost:8080</title>
      <link>http://localhost:8080/@the_mighty_zork</link>
    </image>
  </channel>
</rss>

Checklist

Please put an x inside each checkbox to indicate that you've read and followed it: [ ] -> [x]

If this is a documentation change, only the first checkbox must be filled (you can delete the others if you want).

  • I/we have read the GoToSocial contribution guidelines.
  • I/we have discussed the proposed changes already, either in an issue on the repository, or in the Matrix chat.
  • I/we have performed a self-review of added code.
  • I/we have written code that is legible and maintainable by others.
  • I/we have commented the added code, particularly in hard-to-understand areas.
  • I/we have made any necessary changes to documentation.
  • I/we have added tests that cover new code.
  • I/we have run tests and they pass locally with the changes.
  • I/we have run go fmt ./... and golangci-lint run.

}

// Retrieve latest statuses as they'd be shown on the web view of the account profile.
statuses, err := p.state.DB.GetAccountWebStatuses(ctx, account.ID, rssFeedLength, "")
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if this whole section, up to the feed.ToRss(), could be made part of the else block? When lastPostAt.IsZero() is true, we can skip doing the database call entirely in that case. The query GetAccountWebStatuses generates is kinda beefy, although it should be really quick I suppose if there's nothing in it.

I'm fine with either honestly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point! Fixed it now by returning early.

return true
}

return t1.Unix() > t2.Unix()
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 curious as to why we're comparing the .Unix() values instead of t1.After(t2)? Looking at the Go playground, I think using the time functions directly always do what we want: https://go.dev/play/p/z35W9c5ch7M

Copy link
Contributor Author

@tsmethurst tsmethurst Jul 10, 2023

Choose a reason for hiding this comment

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

It's timezone related, basically. I found that doing After wasn't always reliable when you're comparing dates from different timezones, but casting them to unix time always seemed to get the expected result.

edit: there's probably a nicer, go-approved way of doing this, i just found it easier to cast them to unix because it makes sense to me

@daenney
Copy link
Member

daenney commented Jul 10, 2023

:shipit:

@tsmethurst tsmethurst merged commit ca5492b into main Jul 10, 2023
@tsmethurst tsmethurst deleted the empty_rss_feed_fix branch July 10, 2023 15:06
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.

[bug] Error 500 for RSS feed without posts
2 participants