Skip to content

feat: allow replacing the default implementations of the cache manager and storage#114

Merged
OutdatedGuy merged 24 commits into
OutdatedGuy:mainfrom
AlexV525:feat/defaults-update
Dec 5, 2025
Merged

feat: allow replacing the default implementations of the cache manager and storage#114
OutdatedGuy merged 24 commits into
OutdatedGuy:mainfrom
AlexV525:feat/defaults-update

Conversation

@AlexV525

@AlexV525 AlexV525 commented Sep 1, 2025

Copy link
Copy Markdown
Contributor

The storage interface has been extracted from the default implementation. Users can implement their own instead of using Get Storage or Shared Preferences.

@OutdatedGuy OutdatedGuy left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Mostly LGTM, few questions and changes.

Comment thread lib/src/cached_video_player_plus.dart Outdated
Comment thread lib/src/cached_video_player_plus.dart Outdated
Comment thread lib/src/video_player_storage.dart
Comment thread lib/src/cached_video_player_plus.dart Outdated
Comment thread lib/src/cached_video_player_plus.dart Outdated
Comment thread lib/src/cached_video_player_plus.dart Outdated
Comment thread lib/src/cached_video_player_plus.dart Outdated
Comment thread lib/src/cached_video_player_plus.dart Outdated
Comment thread lib/src/cached_video_player_plus.dart Outdated
Comment thread lib/src/video_player_storage.dart Outdated
@AlexV525

AlexV525 commented Sep 1, 2025

Copy link
Copy Markdown
Contributor Author

I've made a couple of changes additionally:

  1. The storage interface was extracted with a more general interface that can be used to store any type of data.
  2. I've renamed VideoPlayerStorage to VideoPlayerMetadataStorage and made a type alias to deprecate VideoPlayerStorage, but I think we can safely remove it since the class was not exported directly before.

@AlexV525 AlexV525 requested a review from OutdatedGuy September 3, 2025 17:51
@AlexV525

Copy link
Copy Markdown
Contributor Author

@OutdatedGuy friendly ping

Comment thread lib/src/video_player_storage.dart Outdated
Comment thread lib/src/video_player_metadata_storage.dart
Comment thread lib/src/i_video_player_storage.dart Outdated
Comment thread lib/src/cached_video_player_plus.dart
Comment thread lib/src/cached_video_player_plus.dart
Comment thread lib/src/cached_video_player_plus.dart Outdated
OutdatedGuy
OutdatedGuy previously approved these changes Sep 29, 2025

@OutdatedGuy OutdatedGuy left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

LGTM! Will once check updated example app and then merge.

Thanks for the contribution!

@OutdatedGuy

Copy link
Copy Markdown
Owner
Screen.Recording.2025-10-04.at.12.39.48.PM.mov

@AlexV525 the example app doesn't seems to be working correctly

@AlexV525

Copy link
Copy Markdown
Contributor Author

Screen.Recording.2025-10-04.at.12.39.48.PM.mov
@AlexV525 the example app doesn't seems to be working correctly

I'll look into it next week

@OutdatedGuy

Copy link
Copy Markdown
Owner

Hi @AlexV525, do you plan on fixing the example app? :)

@AlexV525

Copy link
Copy Markdown
Contributor Author

The example fails because the file caches always read from the SharedPreferences as the previous code implemented:

final allKeys = await _asyncPrefs.getKeys();

This is a hard-coded behavior that implicitly uses the same storage as the default metadata storage.

I'll update the storage to provide an interface method like FutureOr<List<String>> get keys to get rid of this.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces extensibility to the cached video player by extracting storage into an interface and making the default cache manager and metadata storage implementations replaceable. This allows users to provide custom implementations instead of being locked into SharedPreferences or the default cache manager.

Key Changes:

  • Extracted IVideoPlayerMetadataStorage interface from the default implementation, enabling custom storage implementations
  • Made static cache manager and metadata storage fields mutable (non-final) to allow runtime replacement
  • Added optional constructor and static method parameters for custom cache manager and metadata storage instances

Reviewed Changes

Copilot reviewed 11 out of 15 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
lib/src/i_video_player_metadata_storage.dart New interface defining the contract for video metadata storage implementations
lib/src/video_player_metadata_storage.dart Renamed from VideoPlayerStorage and updated to implement the new interface
lib/src/cached_video_player_plus.dart Added mutable static fields for cache manager and metadata storage, updated constructors and static methods to accept custom implementations
lib/cached_video_player_plus.dart Exported new interface and implementation classes for public API
example/pubspec.yaml Reorganized dependencies (moved flutter SDK to top, removed direct shared_preferences dependency)
example/lib/pages/advance_cache_management_page.dart Demonstrated custom implementations with in-memory metadata storage and custom cache manager toggle functionality
example/lib/main.dart Minor UI adjustment wrapping title text in FittedBox for better layout
example/pubspec.lock Updated transitive dependency versions
example/macos/Runner.xcodeproj/project.pbxproj Updated macOS deployment target to 10.15 and added CLANG configuration
example/ios/Runner.xcodeproj/project.pbxproj Added CLANG_ALLOW_NON_MODULAR_INCLUDES_IN_FRAMEWORK_MODULES setting
example/macos/Podfile Updated minimum macOS platform version to 10.15
example/macos/Podfile.lock Updated pod checksums reflecting dependency updates
example/ios/Podfile.lock Updated Flutter framework checksum
example/macos/.gitignore Added DerivedData to ignored files
.gitignore Added devtools_options.yaml to ignored files
Comments suppressed due to low confidence (1)

lib/src/video_player_metadata_storage.dart:12

  • The comment references the old class name VideoPlayerStorage but the class has been renamed to VideoPlayerMetadataStorage. The comment should be updated to match the new class name.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/src/i_video_player_metadata_storage.dart Outdated
Comment thread lib/src/i_video_player_metadata_storage.dart Outdated
Comment thread lib/src/i_video_player_metadata_storage.dart
Comment thread lib/src/cached_video_player_plus.dart
Comment thread lib/src/cached_video_player_plus.dart

@OutdatedGuy OutdatedGuy left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Lot's of unrelated files changed.
Few nitpicks and bugs in example app

Comment thread example/ios/Runner.xcodeproj/project.pbxproj
Comment thread example/ios/Podfile.lock
Comment thread example/macos/.gitignore
Comment thread example/macos/Runner.xcodeproj/project.pbxproj
Comment thread example/macos/Podfile
Comment thread example/pubspec.yaml Outdated
Comment thread example/pubspec.lock
Comment thread .gitignore
Comment thread example/lib/pages/advance_cache_management_page.dart Outdated
Comment thread example/lib/pages/advance_cache_management_page.dart

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

lib/src/video_player_metadata_storage.dart:12

  • The comment still references the old class name VideoPlayerStorage but should be updated to VideoPlayerMetadataStorage.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/src/i_video_player_metadata_storage.dart Outdated

@OutdatedGuy OutdatedGuy left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

LGTM!

@OutdatedGuy OutdatedGuy changed the title Allow replacing the default implementations of the cache manager and storage feat: allow replacing the default implementations of the cache manager and storage Dec 5, 2025
@OutdatedGuy OutdatedGuy merged commit 4322a2c into OutdatedGuy:main Dec 5, 2025
2 checks passed
@OutdatedGuy

Copy link
Copy Markdown
Owner

Thanks for the contribution @AlexV525

@AlexV525 AlexV525 deleted the feat/defaults-update branch December 5, 2025 09:19
@OutdatedGuy OutdatedGuy mentioned this pull request Dec 5, 2025
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