Skip to content
This repository was archived by the owner on Jun 9, 2023. It is now read-only.

Support for reddit galleries #312

Merged
merged 30 commits into from
Sep 7, 2020
Merged

Support for reddit galleries #312

merged 30 commits into from
Sep 7, 2020
+478 −283

Conversation

AbsurdlySuspicious
Copy link
Collaborator

This is a pretty large PR since it involved unlinking media albums from imgur.

Regarding new jraw dependency, I think its fork too should be moved in githib org as I proposed in #311.
Changes to jraw: AbsurdlySuspicious/JRAW@master...AbsurdlySuspicious:gallery

Links for testing:

@AbsurdlySuspicious
Copy link
Collaborator Author

There's also problem with cache. Any galleries that were put to cache as external link will still be external links. Should it be cleared?

@AbsurdlySuspicious
Copy link
Collaborator Author

Build fails due to #311

msfjarvis added a commit to msfjarvis/Dawn that referenced this pull request Sep 6, 2020
@msfjarvis
Merge pull request Tunous#312 from AbsurdlySuspicious/gallery/feature

Verified

This commit was signed with the committer’s verified signature.
williamdes William Desportes
GPG key ID: 90A0EF1B8251A889
Verified
Learn about vigilant mode
ec8dcd3 
Support for reddit galleries

* github.com:Tunous/Dawn:
  Support thumbs for gallery posts
  Support multi-variant preview in gallery links
  Don't left static image in lq gif urls
  Support image/jpg case
  Support mp4 for gifs
  Full might be absent in media_metadata
  Unlink title/description from ImgurLink
  Captions support
  GIFs support
  Preview with multiple variations for MediaLink
  Add support for media_metadata for ImageWithMultipleVariants
  Generalize ImageWithMultipleVariants
  Move ImageWithMultipleVariants to utils
  RedditGalleryLink
  Rename to MediaAlbumLink
  Generalize media albums
  Superclass for album links
  UrlParser branch
  Update synthetic
  Switch jraw dependency

Signed-off-by: Harsh Shandilya <[email protected]>
Tunous
Tunous suggested changes Sep 6, 2020
View reviewed changes
Copy link
Owner

@Tunous Tunous left a comment

Choose a reason for hiding this comment

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

There's also problem with cache. Any galleries that were put to cache as external link will still be external links. Should it be cleared?

Yeah I think it's ok to clear it somehow.

@AbsurdlySuspicious
Switch jraw dependency

Verified

This commit was signed with the committer’s verified signature.
williamdes William Desportes
GPG key ID: 90A0EF1B8251A889
Verified
Learn about vigilant mode
405a5e2
@AbsurdlySuspicious
Update synthetic

Verified

This commit was signed with the committer’s verified signature.
williamdes William Desportes
GPG key ID: 90A0EF1B8251A889
Verified
Learn about vigilant mode
6fbbf92
@AbsurdlySuspicious
UrlParser branch

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
14019a4
@AbsurdlySuspicious
Superclass for album links
b276f6f
@AbsurdlySuspicious
Generalize media albums
f11fdfc
@AbsurdlySuspicious
Rename to MediaAlbumLink
a748847
@AbsurdlySuspicious
RedditGalleryLink
5b43e36
@AbsurdlySuspicious
Move ImageWithMultipleVariants to utils
1bcc02e
@AbsurdlySuspicious
Generalize ImageWithMultipleVariants
9badc7b
@AbsurdlySuspicious
Add support for media_metadata for ImageWithMultipleVariants
76af92a
@AbsurdlySuspicious
Preview with multiple variations for MediaLink
032fe62
@AbsurdlySuspicious
GIFs support
23d4e36
@AbsurdlySuspicious
Captions support
9b8ce0e
@AbsurdlySuspicious
Unlink title/description from ImgurLink
805a937
@AbsurdlySuspicious
Full might be absent in media_metadata
f38cbe2
@AbsurdlySuspicious
Support mp4 for gifs
168f14e
@AbsurdlySuspicious
Support image/jpg case
21e71b5
@AbsurdlySuspicious
Don't left static image in lq gif urls
037a789
@AbsurdlySuspicious
Support multi-variant preview in gallery links
e817004
@AbsurdlySuspicious
Support thumbs for gallery posts
994f8f4
@AbsurdlySuspicious
Use service provided previews if needed
aebc79f
@AbsurdlySuspicious
Generalize album title
d2a8fbc
@AbsurdlySuspicious
Get rid of exceptions in RedditGalleryLink
264d7bd
@AbsurdlySuspicious
Syntax
5aefa54
@AbsurdlySuspicious
Update actions
5d0bf92
@AbsurdlySuspicious
Some metadata fields are nullable now
01f8a01
AbsurdlySuspicious
AbsurdlySuspicious commented Sep 6, 2020
View reviewed changes
Copy link
Collaborator Author

@AbsurdlySuspicious AbsurdlySuspicious left a comment •
edited
Loading

Choose a reason for hiding this comment

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

aebc79f

Since reddit doesn't provide previews for galleries in usual way, use previews from media_metadata for data saver

264d7bd

All failures are now falling back to external link

5d0bf92

Cache clearing happens here. It's also usable for db schema updates in case that'll happen in future

Tunous
Tunous suggested changes Sep 7, 2020
View reviewed changes
Copy link
Owner

@Tunous Tunous left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looking at the cache clearing code I'm not entirely sure how it's supposed to work. Right now it appears like it would clear room database and not the media cache which seems reverse to what I would expect.

@AbsurdlySuspicious
Copy link
Collaborator Author

it would clear room database

It's intended. Room database stores old serialized version of Submission without any gallery-related fields. This is the problem, not media cache.

fallbacks to destructive migration

So the main purpose of this section is clearing database when no migration happened, but it's known that db contains stale/poisoned cache

CLEAR_MEDIA_CACHE_AFTER = -1 will result in nothing being done

I've included routines for clearing media cache too in case that will be needed in future

introduce this class via dependency injection

I don't think it's worth over-complicating it since it's used once during initialization. I used class instead of object just as a convenient way of passing context, but on second thought, given that there is only 2 functions it can be passed as argument without any pain

@Tunous
Copy link
Owner

Tunous commented Sep 7, 2020

Oh, that explains a lot.

@AbsurdlySuspicious
Copy link
Collaborator Author

This way it should be more intuitive

Tunous
Tunous approved these changes Sep 7, 2020
View reviewed changes
Copy link
Owner

@Tunous Tunous left a comment

Choose a reason for hiding this comment

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

Great, thanks for the changes. Looks good to me now 👍

@Tunous Tunous added this to the 0.11.0 milestone Sep 7, 2020
@Tunous Tunous added the feature New feature label Sep 7, 2020
@Tunous Tunous merged commit 1eae1d2 into Tunous:master Sep 7, 2020
@AbsurdlySuspicious AbsurdlySuspicious mentioned this pull request Sep 8, 2020
Merged
@AbsurdlySuspicious AbsurdlySuspicious deleted the gallery/feature branch January 23, 2021 05:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Tunous Tunous

Assignees
No one assigned
Labels
feature New feature
Projects
None yet
Milestone
0.11.0
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@AbsurdlySuspicious @Tunous