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

feat: reintroducing thumbnails #3821

Merged
merged 5 commits into from
Aug 29, 2024
Merged

feat: reintroducing thumbnails #3821

merged 5 commits into from
Aug 29, 2024

Conversation

RoccoSmit
Copy link
Contributor

PR to cover the Image Preview issue
#3685

Design specs:

  • Thumbnails should reside in the local file system as an optional cache.
  • Only attempt to access the thumbnail when requested, and if it doesn't exist, create it and then return it.

Changes made:

  • Updated api request signature to allow for a thumbnail query string variable
  • Followed the logic from the previous resource flow resource/resource.go with these notable exceptions
    • The resource request will look for a thumbnail first and return it if found, rather than load the original image first then generate the thumbnail after. This limits the number of larger images being read from disk.
    • Newly generated thumbnail images are converted to byte arrays on the fly and returned, removing the need to write images to the disk and then reading them back to get the byte array.
  • Moved thumbnail logic into its own file as it gets used across services
  • Added thumbnail deleting logic in individual services. Resource files are deleted in the store logic, but no version of thumbnails have db related data so store logic did not feel like the place to put thumbnail file deletion.

Notes:

  • The first load of the home / resource pages after this feature is enabled will cause stress on the system as each image request will attempt to generate a thumbnail. Subsequent loads should be improved as thumbnails will load.
  • These changes were manually tested with sqlite and local storage storage options and work as expected

@RoccoSmit RoccoSmit requested a review from boojack as a code owner August 21, 2024 13:21
Copy link
Collaborator

@johnnyjoygh johnnyjoygh left a comment

Choose a reason for hiding this comment

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

LGTM, otherwise

server/router/api/v1/thumbnail.go Outdated Show resolved Hide resolved
server/router/api/v1/thumbnail.go Outdated Show resolved Hide resolved
server/router/api/v1/thumbnail.go Outdated Show resolved Hide resolved
server/router/api/v1/thumbnail.go Outdated Show resolved Hide resolved
- changed method names to start with lower case as they are not used outside of their package
- made receiver types for struct funcs to be pointers to not need to create copies

Trying to cover all linting issues
- converted slog warning to use attributes when logging warnings
- seperated imports to have package files in their own section
@RoccoSmit RoccoSmit mentioned this pull request Aug 22, 2024
@boojack boojack changed the title Reintroducing thumbnails feat: reintroducing thumbnails Aug 29, 2024
@boojack boojack merged commit 9b1adfb into usememos:main Aug 29, 2024
5 checks passed
@RoccoSmit RoccoSmit deleted the thumbnails branch August 30, 2024 10:26
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.

4 participants