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

Implement persistent subscription cache #5185

Merged

Conversation

PikachuEXE
Copy link
Collaborator

@PikachuEXE PikachuEXE commented May 28, 2024

Pull Request Type

  • Bugfix
  • Feature Implementation
  • Documentation
  • Other

Related issue

Closes #4803
Closes #4152

Description

Make subscription cache sync across multiple windows and persistent over app restarts

Screenshots

N/A

Testing

Many cases, generally (with local & IV API):

  • Disable Fetch Feed Automatically
  • Switch/mess around with profiles if wanted
  • Load videos, live, shorts, community posts
  • Open new window(s) to see effects
  • Reboot app
  • Switch to different profiles to see effects

Desktop

  • OS:
  • OS Version:
  • FreeTube version:

Additional context

@efb4f5ff-1298-471a-8973-3d47447115dc

This comment was marked as outdated.

@PikachuEXE

This comment was marked as resolved.

@PikachuEXE PikachuEXE marked this pull request as ready for review May 29, 2024 01:37
@github-actions github-actions bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label May 29, 2024
@FreeTubeBot FreeTubeBot enabled auto-merge (squash) May 29, 2024 01:37
Copy link
Contributor

github-actions bot commented Jun 1, 2024

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot added PR: merge conflicts / rebase needed and removed PR: waiting for review For PRs that are complete, tested, and ready for review labels Jun 1, 2024
* development: (36 commits)
  Remove unused database compacting IPC channel (FreeTubeApp#5212)
  Translated using Weblate (Hungarian)
  Translated using Weblate (Portuguese)
  Translated using Weblate (Finnish)
  Compressed Images (FreeTubeApp#5209)
  Cleanup unused channel hiding code in ft-list-video-numbered (FreeTubeApp#5208)
  Translated using Weblate (Serbian)
  Hide recommendations from the Watch component, instead of hiding itself (FreeTubeApp#5203)
  Consolidate clear subscriptions cache into one mutation (FreeTubeApp#5202)
  Translated using Weblate (Spanish)
  Translated using Weblate (French)
  Translated using Weblate (German)
  Translated using Weblate (Polish)
  Translated using Weblate (Turkish)
  Translated using Weblate (Hungarian)
  Translated using Weblate (Croatian)
  Store comment reply tokens separately to avoid reactivity (FreeTubeApp#5190)
  Translated using Weblate (Croatian)
  Translated using Weblate (German)
  Translated using Weblate (Czech)
  ...

# Conflicts:
#	src/datastores/handlers/base.js
#	src/renderer/store/modules/subscriptions.js
Copy link
Contributor

github-actions bot commented Jun 3, 2024

Conflicts have been resolved. A maintainer will review the pull request shortly.

* development:
  Translated using Weblate (Italian)
  Translated using Weblate (Chinese (Traditional))
  Translated using Weblate (Chinese (Simplified))
  Translated using Weblate (French)
  Translated using Weblate (Spanish)
  Translated using Weblate (Portuguese (Brazil))
  Translated using Weblate (Spanish)
  Make adding duplicate disabled by default (FreeTubeApp#5044)
  Switch settings sections and form elements to created lifecycle hook (FreeTubeApp#5224)
  Translated using Weblate (French)
  Translated using Weblate (Estonian)
  Bump sass from 1.77.2 to 1.77.4 (FreeTubeApp#5222)
  Bump lefthook from 1.6.13 to 1.6.15 (FreeTubeApp#5223)
  Bump the eslint group with 2 updates (FreeTubeApp#5218)
  Bump electron from 30.0.8 to 30.0.9 (FreeTubeApp#5221)
  Bump swiper from 11.1.3 to 11.1.4 (FreeTubeApp#5220)
  Bump stylelint from 16.6.0 to 16.6.1 in the stylelint group (FreeTubeApp#5219)
  Add missing IPC channel constants (FreeTubeApp#5216)
@PikachuEXE
Copy link
Collaborator Author

Updated code & DB file name to subscription cache per dev chat comment

@PikachuEXE PikachuEXE added the PR: waiting for review For PRs that are complete, tested, and ready for review label Jun 6, 2024
@efb4f5ff-1298-471a-8973-3d47447115dc

From the looks of it this also solves #4152

* development: (102 commits)
  Update version number to v0.21.0
  Remove limited donation methods (FreeTubeApp#5290)
  Translated using Weblate (Portuguese)
  Translated using Weblate (Portuguese (Portugal))
  Avoid cloning all profiles when subscribing and unsubscribing (FreeTubeApp#5289)
  Fix arrow keys not working in the Create New Playlist prompt (FreeTubeApp#5243)
  Bump ws from 8.16.0 to 8.17.1 (FreeTubeApp#5291)
  Remove a few bits of unused code (FreeTubeApp#5287)
  Update About page to display correct Freetube logo based on currently set theme (FreeTubeApp#5126)
  Translated using Weblate (Croatian)
  * Update playlist page titles (FreeTubeApp#5271)
  Update Invidious instances list (FreeTubeApp#5288)
  Translated using Weblate (Polish)
  Bump the eslint group with 2 updates (FreeTubeApp#5275)
  Bump marked from 12.0.2 to 13.0.0 (FreeTubeApp#5276)
  Bump sass from 1.77.4 to 1.77.5 (FreeTubeApp#5277)
  Bump lefthook from 1.6.15 to 1.6.16 (FreeTubeApp#5279)
  Bump webpack from 5.91.0 to 5.92.0 (FreeTubeApp#5278)
  Update Flatpak PR Workflow to work with updated module (FreeTubeApp#5270)
  Translated using Weblate (Portuguese)
  ...
Copy link
Contributor

This PR is stale because it has been open 28 days with no activity. Remove stale label or comment or this will be closed in 14 days.

* development: (59 commits)
  Bump the stylelint group with 2 updates (FreeTubeApp#5411)
  Bump rimraf from 5.0.8 to 6.0.1 (FreeTubeApp#5412)
  Translated using Weblate (Indonesian)
  Bump the babel group across 1 directory with 3 updates (FreeTubeApp#5419)
  Bump sass from 1.77.6 to 1.77.8 (FreeTubeApp#5413)
  Bump webpack from 5.92.1 to 5.93.0 (FreeTubeApp#5414)
  Bump lefthook from 1.7.1 to 1.7.2 (FreeTubeApp#5415)
  Bump electron from 31.1.0 to 31.2.0 (FreeTubeApp#5416)
  Fix about page not reacting to locale changes (FreeTubeApp#5393)
  Delete .github/workflows/report.yml (FreeTubeApp#5395)
  Bump version number to v0.21.1
  Fix Invidious DASH manifest generation (FreeTubeApp#5387)
  ^ Update YouTube.js to 10.1.0 (FreeTubeApp#5384)
  Fix Invidious API error toasts saying undefined (FreeTubeApp#5380)
  Bump lefthook from 1.6.18 to 1.7.1 (FreeTubeApp#5376)
  Bump eslint-plugin-vue from 9.26.0 to 9.27.0 in the eslint group (FreeTubeApp#5371)
  Bump rimraf from 5.0.7 to 5.0.8 (FreeTubeApp#5373)
  Bump npm-run-all2 from 6.2.0 to 6.2.2 (FreeTubeApp#5374)
  Bump marked from 13.0.1 to 13.0.2 (FreeTubeApp#5375)
  Translated using Weblate (Portuguese (Brazil))
  ...
@efb4f5ff-1298-471a-8973-3d47447115dc

I will revisit this after the shaka PR.

* development: (102 commits)
  Translated using Weblate (Estonian)
  Translated using Weblate (Dutch)
  Translated using Weblate (Chinese (Traditional))
  Use randomArrayItem helper in more places to reduce duplicate code (FreeTubeApp#5576)
  Translated using Weblate (Croatian)
  Translated using Weblate (Hungarian)
  Bump lefthook from 1.7.12 to 1.7.14 (FreeTubeApp#5585)
  Bump stylelint from 16.8.1 to 16.8.2 in the stylelint group (FreeTubeApp#5583)
  Bump electron from 31.3.1 to 31.4.0 (FreeTubeApp#5584)
  Translated using Weblate (German)
  Translated using Weblate (Polish)
  Translated using Weblate (Japanese)
  Translated using Weblate (English (United Kingdom))
  Translated using Weblate (Japanese)
  Update the Invidious instances list (FreeTubeApp#5575)
  Translated using Weblate (Icelandic)
  Translated using Weblate (Serbian)
  Translated using Weblate (Czech)
  Translated using Weblate (French)
  Translated using Weblate (Chinese (Simplified))
  ...
Copy link
Contributor

github-actions bot commented Sep 2, 2024

This pull request has conflicts, please resolve those before we can evaluate the pull request.

* development: (24 commits)
  Add ability to see comment replies of community posts through invidious (FreeTubeApp#5589)
  Bump electron-builder from 24.13.3 to 25.0.5 (FreeTubeApp#5674)
  Change Sponsorblock Default Category Colors (FreeTubeApp#5686)
  Proxy live streams when "Proxy Videos Through Invidious" is enabled (FreeTubeApp#5649)
  Translated using Weblate (Belarusian)
  Translated using Weblate (Hebrew)
  Fix saving Invidious thumbnail URLs for subscriptions (FreeTubeApp#5662)
  Bump peter-evans/create-pull-request from 6 to 7 (FreeTubeApp#5676)
  Bump webpack-dev-server from 5.0.4 to 5.1.0 (FreeTubeApp#5672)
  Use auto-generated playlists for the videos tab on artist topic channels (FreeTubeApp#5661)
  Rewrite locale file updating to be more Vue 3 friendly (FreeTubeApp#5660)
  Bump sass from 1.77.8 to 1.78.0 (FreeTubeApp#5673)
  Bump electron from 32.0.1 to 32.0.2 (FreeTubeApp#5671)
  Bump marked from 14.1.0 to 14.1.2 (FreeTubeApp#5670)
  Bump postcss from 8.4.44 to 8.4.45 in the stylelint group (FreeTubeApp#5669)
  Bump the eslint group with 2 updates (FreeTubeApp#5668)
  Cleanup a few vue-i18n usages (FreeTubeApp#5663)
  Use nextTick instead of setTimeout to wait until Vue has rendered changes (FreeTubeApp#5664)
  Translated using Weblate (Estonian)
  Translated using Weblate (Belarusian)
  ...

# Conflicts:
#	src/renderer/components/ft-community-post/ft-community-post.js
#	src/renderer/helpers/api/local.js
Copy link
Contributor

Conflicts have been resolved. A maintainer will review the pull request shortly.

absidue
absidue previously approved these changes Sep 12, 2024
@PikachuEXE
Copy link
Collaborator Author

@efb4f5ff-1298-471a-8973-3d47447115dc you approved the doc PR FreeTubeApp/FreeTube-Docs#143 but not this PR?
image

Copy link
Member

@efb4f5ff-1298-471a-8973-3d47447115dc efb4f5ff-1298-471a-8973-3d47447115dc left a comment

Choose a reason for hiding this comment

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

Why cant it persist the cache when subscribing to a new channel? Lets say the last refresh timestamp was on 8hrs ago and I want to keep it that way. Now this completely disappears. Users must refresh otherwise they cant see videos. I would expect that my feed doesnt disappear and when i press refresh i will see the videos from the new channel i subscribed to

VirtualBoxVM_WJzsrmuoQc.mp4

Timestamp doesnt appear after refresh

VirtualBoxVM_aobK1BINPu.mp4

@PikachuEXE
Copy link
Collaborator Author

Why cant it persist the cache when subscribing to a new channel?

That's A3 in #3668

Timestamp doesnt appear after refresh

Will fix

@efb4f5ff-1298-471a-8973-3d47447115dc efb4f5ff-1298-471a-8973-3d47447115dc added the PR: waiting for review For PRs that are complete, tested, and ready for review label Sep 16, 2024
@efb4f5ff-1298-471a-8973-3d47447115dc
Copy link
Member

efb4f5ff-1298-471a-8973-3d47447115dc commented Sep 16, 2024

Why cant it persist the cache when subscribing to a new channel?

That's A3 in #3668

Sorry i have to pushback on this because I think that this is a dedradation of the UX if it goes in like this

At that time i had no issues with it but with this feature it doenst feel like the correct path to take

* development: (29 commits)
  Translated using Weblate (Flemish (West))
  Added translation using Weblate (Flemish (West))
  Translated using Weblate (Slovak)
  Improve history import performance and fix some bugs (FreeTubeApp#5666)
  Bump electron from 32.0.2 to 32.1.0 (FreeTubeApp#5710)
  Local API: Use IOS HLS manifest for livestreams (FreeTubeApp#5705)
  Translated using Weblate (Slovak)
  Translated using Weblate (Japanese)
  Bump the stylelint group with 2 updates (FreeTubeApp#5706)
  Bump swiper from 11.1.12 to 11.1.14 (FreeTubeApp#5709)
  Fix a few memory leaks while tearing down the player (FreeTubeApp#5698)
  Fix audio track selection (FreeTubeApp#5697)
  Bump shaka-player from 4.10.12 to 4.11.1 (FreeTubeApp#5677)
  Translated using Weblate (Czech)
  Translated using Weblate (Serbian)
  Translated using Weblate (Polish)
  Translated using Weblate (German)
  Translated using Weblate (Chinese (Simplified Han script))
  Bump express from 4.19.2 to 4.20.0 (FreeTubeApp#5687)
  Translated using Weblate (Turkish)
  ...
@PikachuEXE
Copy link
Collaborator Author

Fixed Timestamp doesnt appear after refresh

Why cant it persist the cache when subscribing to a new channel?

I think you are looking for something like kommunarr#2
It's quite big and I don't think this PR should include that additional amount of code change = more time for review & test
This PR improves for those with no new channel subscribed after refresh, just no improvement after new channel subscribed (worst case just refresh like now)

@FreeTubeBot FreeTubeBot merged commit ddb495a into FreeTubeApp:development Sep 18, 2024
5 checks passed
@github-actions github-actions bot removed the PR: waiting for review For PRs that are complete, tested, and ready for review label Sep 18, 2024
@PikachuEXE PikachuEXE deleted the feature/subscription-cache branch September 19, 2024 00:02
PikachuEXE added a commit to PikachuEXE/FreeTube that referenced this pull request Sep 21, 2024
* development:
  Fix video start time handling (FreeTubeApp#5719)
  Local API: Handle new shorts node (FreeTubeApp#5679)
  Update subscription cache when subscribing from the channel page (FreeTubeApp#5717)
  Translated using Weblate (English (United Kingdom))
  Implement persistent subscription cache (FreeTubeApp#5185)
  Translated using Weblate (Belarusian)
  Bump npm-run-all2 from 6.2.2 to 6.2.3 (FreeTubeApp#5708)
  Translated using Weblate (Swedish)
  Bump shaka-player from 4.11.1 to 4.11.3 (FreeTubeApp#5707)
  Translated using Weblate (Persian)
  v Downgrade electron-builder 25.x > 24.x (FreeTubeApp#5712)
  Translated using Weblate (Persian)
  Translated using Weblate (Persian)

# Conflicts:
#	src/renderer/views/Channel/Channel.js
#	src/renderer/views/Channel/Channel.vue
@efb4f5ff-1298-471a-8973-3d47447115dc

After seeing the report in #5724 I think we need to change something. I think allot of users will see this as broken behavior just like the one that reported it. I suggest to bypass the cache when on application startup.

@PikachuEXE
Copy link
Collaborator Author

#5724 looks like a bug we didn't catch in this PR (forgot to add test case with Fetch Feed Automatically enabled, marginal case test)

Fixing today

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants