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/missing emoji urls #3707

Merged

Conversation

NyaaaWhatsUpDoc
Copy link
Member

Description

This is a partial fix for #3605, but honestly we could do with some better logic for emoji refreshing of URLs that haven't been touched in a while.

Post v0.18.0 I'm think we have some kind of perma-url for emojis that triggers a refresh and redirects the client to the latest image URLs. Or something along those lines, the idea and implementation are up for discussion :p, but ultimately I would like to be the one to work on it as there's some related improvements I want to make to determining if an emoji is fresh / needs uncaching. And same goes for media.

Checklist

  • 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 not leveraged AI to create the proposed changes.
  • 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.

@tsmethurst tsmethurst merged commit 91cef34 into superseriousbusiness:main Jan 30, 2025
3 checks passed
var err error

// Fetch GTS models for emoji IDs
emojis, err = c.state.DB.GetEmojisByIDs(ctx, emojiIDs)
if err != nil {
errs.Appendf("error fetching emojis from database: %w", err)
return nil, gtserror.Newf("db error fetching emojis: %w", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

Ideally, if media-emoji-local-max-size and media-emoji-remote-max-size are 1, omit the error message?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, one can set them to 0 apparently? (from the “32-bit” part of the last release’s info)

Will change them to 0 on my instance and test this.

Copy link
Contributor

Choose a reason for hiding this comment

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

(I was wary that 0 could mean “unlimited”.)

Copy link
Member Author

Choose a reason for hiding this comment

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

this will only return an error message if it fails fetching emojis, even with a 'size' parameter of zero your instance will likely either have entries in the emoji table just forever marked uncached (which would then get handled without an error below), or you just won't have any emoji entries at all in which case the input array "emojiIDs" here will be empty and this function won't throw an error. i forget exactly how gotosocial would handle things with a size of zero, but either way this shouldn't result in any more error messages :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm.

gotosocial=> SELECT COUNT(*) FROM emojis;
 count
-------
 17065
(1 row)
gotosocial=> SELECT * FROM emojis LIMIT 5;
             id             |          created_at           |          updated_at           |   shortcode    |        domain        |                                        image_remote_url                                         | image_static_remote_url | image_url | image_static_url | image_path | image_static_path | image_content_type | image_static_content_type | image_file_size | image_static_file_size |       image_updated_at        | disabled |                       uri                       | visible_in_picker | category_id | cached 
----------------------------+-------------------------------+-------------------------------+----------------+----------------------+-------------------------------------------------------------------------------------------------+-------------------------+-----------+------------------+------------+-------------------+--------------------+---------------------------+-----------------+------------------------+-------------------------------+----------+-------------------------------------------------+-------------------+-------------+--------
 01J8JSCXM373WBE5HWPKX4HG4G | 2024-09-24 19:51:10.723155+00 | 2024-09-24 19:51:10.809754+00 | flan_awe       | mastodon.infra.de    | https://mastodon.infra.de/system/custom_emojis/images/000/029/024/original/a6bb0f280bd36405.png |                         |           |                  |            |                   |                    |                           |               0 |                      0 | 2024-09-24 19:51:10.723394+00 | f        | https://mastodon.infra.de/emojis/29024          | t                 |             | f
 01J8JSCXPV20PWKBE8M88ND7ZQ | 2024-09-24 19:51:10.811564+00 | 2024-09-24 19:51:10.834831+00 | flan_despair   | mastodon.infra.de    | https://mastodon.infra.de/system/custom_emojis/images/000/029/025/original/611ef45839bebfd6.png |                         |           |                  |            |                   |                    |                           |               0 |                      0 | 2024-09-24 19:51:10.811754+00 | f        | https://mastodon.infra.de/emojis/29025          | t                 |             | f
 01JBSAQCFB96BZD452Q1GEPN6J | 2024-11-03 15:36:31.467472+00 | 2024-11-25 07:04:42.732678+00 | blobcatlove    | sharknado.tombies.eu | https://sharknado.tombies.eu/files/117cf22a-bf74-41eb-9507-284d974cada2                         |                         |           |                  |            |                   |                    |                           |               0 |                      0 | 2024-11-03 15:36:31.467705+00 | f        | https://sharknado.tombies.eu/emojis/blobcatlove | t                 |             | f
 01J8RK8333FX3BFNHW2Q664J6H | 2024-09-27 01:59:07.6193+00   | 2025-01-26 07:20:22.26221+00  | neurodiversity | hackers.town         | https://hackers.town/system/custom_emojis/images/000/075/367/original/ed22ab6205c9bfe8.png      |                         |           |                  |            |                   |                    |                           |               0 |                      0 | 2024-09-27 01:59:07.619478+00 | f        | https://hackers.town/emojis/75367               | t                 |             | f
 01J8JSHXD0G1RKJSNF3N7M5WVJ | 2024-09-24 19:53:54.336507+00 | 2025-01-30 07:43:00.512154+00 | cyberpunk      | corteximplant.com    | https://corteximplant.com/system/custom_emojis/images/000/017/367/original/cf7e960891f5af05.png |                         |           |                  |            |                   |                    |                           |               0 |                      0 | 2024-09-24 19:53:54.336671+00 | f        | https://corteximplant.com/emojis/17367          | t                 |             | f
(5 rows)

But yes, the cached column is false for all entries.

Copy link
Member Author

Choose a reason for hiding this comment

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

yep in that case they'll all get filtered out as uncached from any frontend api responses :)

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.

3 participants