feat: Add support for HomeWidget.saveFile and HomeWidget.saveImage#409
Merged
Conversation
|
To view this pull requests documentation preview, visit the following URL: docs.page/abausg/home_widget~409 Documentation is deployed and generated using docs.page. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds file and image persistence to home_widget: new APIs Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This was
linked to
issues
Apr 9, 2026
Closed
Closed
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #409 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 3 3
Lines 115 163 +48
=========================================
+ Hits 115 163 +48 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (7)
packages/home_widget/test/utils/test_png.dart (1)
7-7: Optional: add explicit type for the decoded fixture bytes.Inference works, but a concrete type (
Uint8ListorList<int>) makes test intent clearer.Suggested tweak
+import 'dart:typed_data'; import 'dart:convert'; @@ -final kTestPngBytes = base64Decode(kTestPngBase64); +final Uint8List kTestPngBytes = base64Decode(kTestPngBase64);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/home_widget/test/utils/test_png.dart` at line 7, The test fixture variable kTestPngBytes is currently declared via inference; make its type explicit (e.g., Uint8List or List<int>) to clarify intent—update the declaration of kTestPngBytes (the value produced by base64Decode(kTestPngBase64)) to have the chosen concrete type so readers and tooling see it as a byte buffer for PNG test data.examples/file_and_images/README.md (1)
1-3: Expand this README with purpose and quick-start steps.Line 3 is too generic for an example meant to showcase
saveFile/saveImage; a short “what this demonstrates” and “how to run” section would make it much more usable.📝 Suggested README update
# file_and_images -A new Flutter project. +Example app demonstrating `HomeWidget.saveFile` and `HomeWidget.saveImage`. + +## What it shows +- Saving a local file for widget consumption +- Saving an image for widget rendering +- Clearing widget data with file cleanup behavior + +## Run +```bash +flutter pub get +flutter run +```🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/file_and_images/README.md` around lines 1 - 3, Update the README.md for the file_and_images example to explain what the example demonstrates (that it showcases the saveFile and saveImage APIs), list quick-start steps to run it (e.g., fetch dependencies with "flutter pub get" and run with "flutter run"), and include a short usage note describing where saved files/images are placed and how to trigger the demo actions in the app; reference the example name "file_and_images" and the functions "saveFile" and "saveImage" so readers immediately know the intent and how to run it.examples/file_and_images/ios/RunnerTests/RunnerTests.swift (1)
7-10: Replace the placeholder test with an assertion-based test (or remove it).Lines 7–10 currently provide no verification and can create a false sense of coverage.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/file_and_images/ios/RunnerTests/RunnerTests.swift` around lines 7 - 10, The placeholder test testExample() in RunnerTests currently has no assertions—either remove it or replace it with a real assertion-based test; update the testExample() method inside the RunnerTests class to perform at least one XCTAssert (for example XCTAssertTrue, XCTAssertEqual, or XCTAssertNotNil) that verifies a deterministic property (app starts, bundle identifier, or a known value) so the test provides real verification instead of being empty.docs/usage/sync-data.mdx (1)
20-20: Clarify method-to-payload mapping for readability.Nice addition. Consider explicitly stating
saveFilefor arbitrary bytes/files andsaveImagefor image providers to remove any ambiguity in this sentence.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/usage/sync-data.mdx` at line 20, Clarify the mapping between methods and payload types: explicitly state that for arbitrary binary data or files you should use saveFile (e.g., JSON, blobs, any bytes) and for image-specific payloads coming from image providers you should use saveImage, and that both use the same id which getWidgetData will return as the absolute file path for native opening; mention saveFile, saveImage, and getWidgetData by name so readers can locate the methods.examples/file_and_images/android/app/src/main/kotlin/es/antonborri/file_and_images/ImageWidgetHomeWidget.kt (1)
43-44: Prefer sampled bitmap decoding to avoid widget memory pressure.Line 43 and Line 44 decode the original file at full size. Large images can cause avoidable memory spikes in the widget process. Consider decoding with
BitmapFactory.Optionsand aninSampleSize.♻️ Proposed refactor
- val bitmap = - imagePath?.takeIf { File(it).isFile }?.let { path -> BitmapFactory.decodeFile(path) } + val bitmap = + imagePath?.takeIf { File(it).isFile }?.let { path -> + decodeSampledBitmap(path, reqWidth = 1024, reqHeight = 1024) + }private fun decodeSampledBitmap(path: String, reqWidth: Int, reqHeight: Int) = BitmapFactory.Options().run { inJustDecodeBounds = true BitmapFactory.decodeFile(path, this) inSampleSize = 1 while ((outWidth / inSampleSize) > reqWidth || (outHeight / inSampleSize) > reqHeight) { inSampleSize *= 2 } inJustDecodeBounds = false BitmapFactory.decodeFile(path, this) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/file_and_images/android/app/src/main/kotlin/es/antonborri/file_and_images/ImageWidgetHomeWidget.kt` around lines 43 - 44, The current direct BitmapFactory.decodeFile call that assigns bitmap (using imagePath) can OOM on large images; replace it by implementing a sampled decoder (e.g., decodeSampledBitmap(path: String, reqWidth: Int, reqHeight: Int): Bitmap?) which uses BitmapFactory.Options with inJustDecodeBounds to compute an inSampleSize power-of-two and then decodes with that inSampleSize, and call that helper instead of BitmapFactory.decodeFile when creating the bitmap in ImageWidgetHomeWidget (replace the bitmap assignment that uses imagePath). Use the widget's target width/height (or a sensible max dimension) as reqWidth/reqHeight so the decoded bitmap is appropriately downsampled.examples/file_and_images/ios/Runner.xcodeproj/project.pbxproj (1)
762-767: Minor: Duplicate entry inLD_RUNPATH_SEARCH_PATHS.The
@executable_path/../../Frameworkspath appears twice in the Profile configuration for ImageWidgetHomeWidget. This is harmless but creates unnecessary noise.The same duplication exists for FileWidgetHomeWidget's Profile configuration (lines 1023-1028).
🔧 Suggested fix for ImageWidgetHomeWidget Profile config
LD_RUNPATH_SEARCH_PATHS = ( "$(inherited)", "@executable_path/Frameworks", "@executable_path/../../Frameworks", - "@executable_path/../../Frameworks", );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/file_and_images/ios/Runner.xcodeproj/project.pbxproj` around lines 762 - 767, Remove the duplicate "@executable_path/../../Frameworks" entry from the LD_RUNPATH_SEARCH_PATHS arrays in the Profile configurations for ImageWidgetHomeWidget and FileWidgetHomeWidget; locate the LD_RUNPATH_SEARCH_PATHS block (the tuple containing "$(inherited)", "@executable_path/Frameworks", and two "@executable_path/../../Frameworks" entries) and delete the redundant "@executable_path/../../Frameworks" so each path appears only once.packages/home_widget/lib/src/home_widget.dart (1)
163-185: Good defensive validation for file path components.The
_normalizeExtensionand_validateKeyhelpers prevent path traversal and injection issues by rejecting separators (/,\,..) and ensuring non-empty values. Note these are debug-only assertions, so in release builds invalid input would still be accepted.💡 Consider runtime validation for release builds
If invalid keys/extensions in release builds could cause security or data integrity issues, consider throwing instead of asserting:
static void _validateKey(String key) { - assert(key.isNotEmpty, 'key must not be empty'); - assert( - !key.contains('/') && - !key.contains(r'\') && - !key.contains('..') && - !key.contains(' '), - 'key must not contain /, \\, .., or spaces', - ); + if (key.isEmpty || + key.contains('/') || + key.contains(r'\') || + key.contains('..') || + key.contains(' ')) { + throw ArgumentError('key must be non-empty and not contain /, \\, .., or spaces'); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/home_widget/lib/src/home_widget.dart` around lines 163 - 185, The current validation in _normalizeExtension and _validateKey uses debug-only assert checks, so in release invalid inputs slip through; change these assertions to real runtime checks that throw descriptive exceptions (e.g., ArgumentError or FormatException) instead of using assert: in _normalizeExtension, after trimming and removing a leading '.', throw if ext.isEmpty or if ext contains '/', '\\' or '..' with a clear message; in _validateKey, perform the same checks and throw if key.isEmpty or contains '/', '\\', '..', or spaces, using consistent exception types/messages so callers receive failures in release builds.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@examples/file_and_images/android/app/src/main/res/drawable/launch_background.xml`:
- Line 4: The launch splash currently hardcodes `@android`:color/white in
launch_background.xml which causes a white flash in dark mode; replace that
hardcoded reference with a night-aware resource by either (a) creating
drawable-night/launch_background.xml that sets a dark background for the same
drawable name, or (b) change the item to reference a color resource (e.g.,
`@color/launch_background`) and add a values-night/colors.xml providing a dark
variant; update any references in styles.xml that point to this drawable to
continue using the same resource name so the night qualifier is picked up.
In `@examples/file_and_images/android/build.gradle.kts`:
- Around line 1-17: Add cspell ignore directives for the Gradle DSL identifiers
causing false positives: add a single-line comment at the top of the file like
"// cspell:ignore allprojects subprojects newSubprojectBuildDir newBuildDir" so
the spellchecker skips those tokens; this targets the symbols allprojects,
subprojects, newSubprojectBuildDir and newBuildDir referenced in the file.
In `@examples/file_and_images/lib/main.dart`:
- Around line 92-96: The ImageProvider creation uses _imageUrl! when imageType
== ImageType.network which can throw if _imageUrl is null/empty; update the
switch in the widget that builds ImageProvider (the block creating ImageProvider
and using ImageType.flutter, ImageType.dash, ImageType.network) to guard against
null/empty _imageUrl by either using a fallback AssetImage or excluding the
NetworkImage branch until a valid URL exists, or disable the "Update" action
when imageType == ImageType.network and _imageUrl is null/empty; ensure checks
reference ImageType.network and _imageUrl (not using _imageUrl!) and pick a
clear fallback (e.g., AssetImage('assets/placeholder.png')) or prevent the
update action.
In `@packages/home_widget/lib/src/home_widget.dart`:
- Around line 200-218: The code builds a file path from a possibly null
directory returned by PathProviderFoundation.getContainerPath; add a null check
immediately after assigning directory (before computing path =
'$directory/home_widget/$key.$ext') and handle it by throwing a clear exception
or returning an error (include contextual info like HomeWidget.groupId and
suggest verifying app group configuration) so the app never constructs
"null/..." paths; update the block around Platform.isIOS and the later path
construction (the local variable directory and the final path computation) to
perform this validation and fail fast with a helpful message.
---
Nitpick comments:
In `@docs/usage/sync-data.mdx`:
- Line 20: Clarify the mapping between methods and payload types: explicitly
state that for arbitrary binary data or files you should use saveFile (e.g.,
JSON, blobs, any bytes) and for image-specific payloads coming from image
providers you should use saveImage, and that both use the same id which
getWidgetData will return as the absolute file path for native opening; mention
saveFile, saveImage, and getWidgetData by name so readers can locate the
methods.
In
`@examples/file_and_images/android/app/src/main/kotlin/es/antonborri/file_and_images/ImageWidgetHomeWidget.kt`:
- Around line 43-44: The current direct BitmapFactory.decodeFile call that
assigns bitmap (using imagePath) can OOM on large images; replace it by
implementing a sampled decoder (e.g., decodeSampledBitmap(path: String,
reqWidth: Int, reqHeight: Int): Bitmap?) which uses BitmapFactory.Options with
inJustDecodeBounds to compute an inSampleSize power-of-two and then decodes with
that inSampleSize, and call that helper instead of BitmapFactory.decodeFile when
creating the bitmap in ImageWidgetHomeWidget (replace the bitmap assignment that
uses imagePath). Use the widget's target width/height (or a sensible max
dimension) as reqWidth/reqHeight so the decoded bitmap is appropriately
downsampled.
In `@examples/file_and_images/ios/Runner.xcodeproj/project.pbxproj`:
- Around line 762-767: Remove the duplicate "@executable_path/../../Frameworks"
entry from the LD_RUNPATH_SEARCH_PATHS arrays in the Profile configurations for
ImageWidgetHomeWidget and FileWidgetHomeWidget; locate the
LD_RUNPATH_SEARCH_PATHS block (the tuple containing "$(inherited)",
"@executable_path/Frameworks", and two "@executable_path/../../Frameworks"
entries) and delete the redundant "@executable_path/../../Frameworks" so each
path appears only once.
In `@examples/file_and_images/ios/RunnerTests/RunnerTests.swift`:
- Around line 7-10: The placeholder test testExample() in RunnerTests currently
has no assertions—either remove it or replace it with a real assertion-based
test; update the testExample() method inside the RunnerTests class to perform at
least one XCTAssert (for example XCTAssertTrue, XCTAssertEqual, or
XCTAssertNotNil) that verifies a deterministic property (app starts, bundle
identifier, or a known value) so the test provides real verification instead of
being empty.
In `@examples/file_and_images/README.md`:
- Around line 1-3: Update the README.md for the file_and_images example to
explain what the example demonstrates (that it showcases the saveFile and
saveImage APIs), list quick-start steps to run it (e.g., fetch dependencies with
"flutter pub get" and run with "flutter run"), and include a short usage note
describing where saved files/images are placed and how to trigger the demo
actions in the app; reference the example name "file_and_images" and the
functions "saveFile" and "saveImage" so readers immediately know the intent and
how to run it.
In `@packages/home_widget/lib/src/home_widget.dart`:
- Around line 163-185: The current validation in _normalizeExtension and
_validateKey uses debug-only assert checks, so in release invalid inputs slip
through; change these assertions to real runtime checks that throw descriptive
exceptions (e.g., ArgumentError or FormatException) instead of using assert: in
_normalizeExtension, after trimming and removing a leading '.', throw if
ext.isEmpty or if ext contains '/', '\\' or '..' with a clear message; in
_validateKey, perform the same checks and throw if key.isEmpty or contains '/',
'\\', '..', or spaces, using consistent exception types/messages so callers
receive failures in release builds.
In `@packages/home_widget/test/utils/test_png.dart`:
- Line 7: The test fixture variable kTestPngBytes is currently declared via
inference; make its type explicit (e.g., Uint8List or List<int>) to clarify
intent—update the declaration of kTestPngBytes (the value produced by
base64Decode(kTestPngBase64)) to have the chosen concrete type so readers and
tooling see it as a byte buffer for PNG test data.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 278d839e-1290-466b-ae8f-84dcc20474f6
⛔ Files ignored due to path filters (25)
examples/file_and_images/android/app/src/main/res/mipmap-hdpi/ic_launcher.pngis excluded by!**/*.pngexamples/file_and_images/android/app/src/main/res/mipmap-mdpi/ic_launcher.pngis excluded by!**/*.pngexamples/file_and_images/android/app/src/main/res/mipmap-xhdpi/ic_launcher.pngis excluded by!**/*.pngexamples/file_and_images/android/app/src/main/res/mipmap-xxhdpi/ic_launcher.pngis excluded by!**/*.pngexamples/file_and_images/android/app/src/main/res/mipmap-xxxhdpi/ic_launcher.pngis excluded by!**/*.pngexamples/file_and_images/assets/dash.pngis excluded by!**/*.pngexamples/file_and_images/ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-1024x1024@1x.pngis excluded by!**/*.pngexamples/file_and_images/ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-20x20@1x.pngis excluded by!**/*.pngexamples/file_and_images/ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-20x20@2x.pngis excluded by!**/*.pngexamples/file_and_images/ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-20x20@3x.pngis excluded by!**/*.pngexamples/file_and_images/ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-29x29@1x.pngis excluded by!**/*.pngexamples/file_and_images/ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-29x29@2x.pngis excluded by!**/*.pngexamples/file_and_images/ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-29x29@3x.pngis excluded by!**/*.pngexamples/file_and_images/ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-40x40@1x.pngis excluded by!**/*.pngexamples/file_and_images/ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-40x40@2x.pngis excluded by!**/*.pngexamples/file_and_images/ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-40x40@3x.pngis excluded by!**/*.pngexamples/file_and_images/ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-60x60@2x.pngis excluded by!**/*.pngexamples/file_and_images/ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-60x60@3x.pngis excluded by!**/*.pngexamples/file_and_images/ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-76x76@1x.pngis excluded by!**/*.pngexamples/file_and_images/ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-76x76@2x.pngis excluded by!**/*.pngexamples/file_and_images/ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-83.5x83.5@2x.pngis excluded by!**/*.pngexamples/file_and_images/ios/Runner/Assets.xcassets/LaunchImage.imageset/LaunchImage.pngis excluded by!**/*.pngexamples/file_and_images/ios/Runner/Assets.xcassets/LaunchImage.imageset/LaunchImage@2x.pngis excluded by!**/*.pngexamples/file_and_images/ios/Runner/Assets.xcassets/LaunchImage.imageset/LaunchImage@3x.pngis excluded by!**/*.pngpackages/home_widget/example/assets/integration_test.pngis excluded by!**/*.png
📒 Files selected for processing (68)
docs.jsondocs/features/render-flutter-widgets.mdxdocs/features/save-files.mdxdocs/usage/sync-data.mdxexamples/file_and_images/.gitignoreexamples/file_and_images/.metadataexamples/file_and_images/README.mdexamples/file_and_images/analysis_options.yamlexamples/file_and_images/android/.gitignoreexamples/file_and_images/android/app/build.gradle.ktsexamples/file_and_images/android/app/src/debug/AndroidManifest.xmlexamples/file_and_images/android/app/src/main/AndroidManifest.xmlexamples/file_and_images/android/app/src/main/kotlin/es/antonborri/file_and_images/FileWidgetHomeWidget.ktexamples/file_and_images/android/app/src/main/kotlin/es/antonborri/file_and_images/FileWidgetHomeWidgetReceiver.ktexamples/file_and_images/android/app/src/main/kotlin/es/antonborri/file_and_images/ImageWidgetHomeWidget.ktexamples/file_and_images/android/app/src/main/kotlin/es/antonborri/file_and_images/ImageWidgetHomeWidgetReceiver.ktexamples/file_and_images/android/app/src/main/kotlin/es/antonborri/file_and_images/MainActivity.ktexamples/file_and_images/android/app/src/main/res/drawable-v21/launch_background.xmlexamples/file_and_images/android/app/src/main/res/drawable/launch_background.xmlexamples/file_and_images/android/app/src/main/res/values-night/styles.xmlexamples/file_and_images/android/app/src/main/res/values/styles.xmlexamples/file_and_images/android/app/src/main/res/xml/file_widget_home_widget.xmlexamples/file_and_images/android/app/src/main/res/xml/image_widget_home_widget.xmlexamples/file_and_images/android/app/src/profile/AndroidManifest.xmlexamples/file_and_images/android/build.gradle.ktsexamples/file_and_images/android/gradle.propertiesexamples/file_and_images/android/gradle/wrapper/gradle-wrapper.propertiesexamples/file_and_images/android/settings.gradle.ktsexamples/file_and_images/ios/.gitignoreexamples/file_and_images/ios/FileWidgetHomeWidget.entitlementsexamples/file_and_images/ios/FileWidgetHomeWidget/Info.plistexamples/file_and_images/ios/FileWidgetHomeWidget/Widget.swiftexamples/file_and_images/ios/FileWidgetHomeWidget/WidgetBundle.swiftexamples/file_and_images/ios/Flutter/AppFrameworkInfo.plistexamples/file_and_images/ios/Flutter/Debug.xcconfigexamples/file_and_images/ios/Flutter/Release.xcconfigexamples/file_and_images/ios/ImageWidgetHomeWidget.entitlementsexamples/file_and_images/ios/ImageWidgetHomeWidget/Info.plistexamples/file_and_images/ios/ImageWidgetHomeWidget/Widget.swiftexamples/file_and_images/ios/ImageWidgetHomeWidget/WidgetBundle.swiftexamples/file_and_images/ios/Podfileexamples/file_and_images/ios/Runner.xcodeproj/project.pbxprojexamples/file_and_images/ios/Runner.xcodeproj/project.xcworkspace/xcshareddata/IDEWorkspaceChecks.plistexamples/file_and_images/ios/Runner.xcodeproj/project.xcworkspace/xcshareddata/WorkspaceSettings.xcsettingsexamples/file_and_images/ios/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcschemeexamples/file_and_images/ios/Runner.xcworkspace/xcshareddata/IDEWorkspaceChecks.plistexamples/file_and_images/ios/Runner.xcworkspace/xcshareddata/WorkspaceSettings.xcsettingsexamples/file_and_images/ios/Runner/AppDelegate.swiftexamples/file_and_images/ios/Runner/Assets.xcassets/AppIcon.appiconset/Contents.jsonexamples/file_and_images/ios/Runner/Assets.xcassets/LaunchImage.imageset/Contents.jsonexamples/file_and_images/ios/Runner/Assets.xcassets/LaunchImage.imageset/README.mdexamples/file_and_images/ios/Runner/Base.lproj/LaunchScreen.storyboardexamples/file_and_images/ios/Runner/Base.lproj/Main.storyboardexamples/file_and_images/ios/Runner/Info.plistexamples/file_and_images/ios/Runner/Runner-Bridging-Header.hexamples/file_and_images/ios/Runner/Runner.entitlementsexamples/file_and_images/ios/RunnerTests/RunnerTests.swiftexamples/file_and_images/lib/main.dartexamples/file_and_images/pubspec.yamlpackages/home_widget/example/integration_test/android_test.dartpackages/home_widget/example/integration_test/ios_test.dartpackages/home_widget/example/ios/HomeWidgetExampleExtension.entitlementspackages/home_widget/example/ios/Runner/Runner.entitlementspackages/home_widget/example/pubspec.yamlpackages/home_widget/lib/src/home_widget.dartpackages/home_widget/test/home_widget_test.dartpackages/home_widget/test/utils/test_png.dartpubspec.yaml
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
examples/file_and_images/README.md (1)
3-3: Polish wording in the intro sentence (Line 3).The sentence is understandable but reads a bit rough; a quick grammar/casing cleanup improves clarity.
Suggested wording
-A simple Demo app showing how to save Images and Files from Flutter and accessing the Data from native Widgets +A simple demo app showing how to save images and files from Flutter and access the data from native widgets.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/file_and_images/README.md` at line 3, The intro sentence in the README is awkwardly worded; replace it with a polished version to fix grammar and casing: change "A simple Demo app showing how to save Images and Files from Flutter and accessing the Data from native Widgets" to "A simple demo app showing how to save images and files from Flutter and access the data from native widgets." Update the README intro sentence accordingly (the first descriptive sentence in the file) so casing is consistent and verbs match.packages/home_widget/example/integration_test/ios_test.dart (1)
38-45: Expand setup cleanup to include new file/image test keys.Line 42 currently iterates only
testData.keys. The added file tests use separate keys, and thedeleteFile: falsepath can leave artifacts between reruns.♻️ Suggested cleanup adjustment
setUp(() async { // Add Group Id await HomeWidget.setAppGroupId('group.es.antonborri.integrationTest'); // Clear all Data - for (final key in testData.keys) { + final cleanupKeys = <String>{ + ...testData.keys, + 'integration_json_file_key', + 'integration_png_file_key', + 'integration_save_image_key', + 'integration_savefile_clear_key', + 'integration_savefile_clear_no_delete_key', + }; + for (final key in cleanupKeys) { await HomeWidget.saveWidgetData(key, null); } });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/home_widget/example/integration_test/ios_test.dart` around lines 38 - 45, The setup cleanup only clears keys in testData.keys but misses the new file/image test keys and the case where deleteFile: false can leave artifacts; update the setUp block (referencing setUp and HomeWidget.saveWidgetData) to also iterate and clear the file/image test keys (e.g., the constants or list you added for file tests) and ensure you call HomeWidget.saveWidgetData for those keys with null and deleteFile: true (or explicitly remove the files via the appropriate HomeWidget file-delete API) so no artifacts remain between reruns.packages/home_widget/example/integration_test/android_test.dart (1)
26-30: Include file-API test keys in per-test cleanup.Line 27 only clears
testDatakeys, but the new file tests use different keys and one path is intentionally left on disk (deleteFile: falseat Line 193). This can leak state across reruns.♻️ Suggested cleanup adjustment
setUp(() async { - for (final key in testData.keys) { + final cleanupKeys = <String>{ + ...testData.keys, + 'integration_json_file_key', + 'integration_png_file_key', + 'integration_save_image_key', + 'integration_savefile_clear_key', + 'integration_savefile_clear_no_delete_key', + }; + for (final key in cleanupKeys) { await HomeWidget.saveWidgetData(key, null); } });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/home_widget/example/integration_test/android_test.dart` around lines 26 - 30, The setup cleanup only clears keys from testData but misses the file-API test keys (and one file left on disk via deleteFile: false), which can leak state; update the setUp to also iterate over the file-API test keys and call HomeWidget.saveWidgetData(key, null) for each of them (or use the file-API teardown to remove the file path), and for the specific test that used deleteFile: false ensure the leftover path is explicitly removed during setup/teardown (or flip deleteFile to true) so no file remains between runs; reference testData, setUp, HomeWidget.saveWidgetData and the deleteFile flag when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/file_and_images/README.md`:
- Line 29: The docs snippet uses the token Charsets
(File(jsonPath).readText(Charsets.UTF_8)) which cspell flags; either add
"Charsets" to the project's cspell allowlist (cspell.json / words list) or
change the example to avoid the Charsets token (e.g., use a standard alternative
such as java.nio.charset.StandardCharsets.UTF_8 or pass "UTF-8" as the charset)
so the documentation no longer contains the flagged token; update the README.md
example accordingly and keep the call site File(jsonPath).readText(...)
semantics unchanged.
In `@packages/home_widget/lib/src/home_widget.dart`:
- Around line 24-37: The auto-delete logic in saveWidgetData is unsafe: when
data==null it fetches getWidgetData(id) and may synchronously delete any string
that happens to be a valid path; change saveWidgetData to (1) treat the returned
value strictly as a stored file path only after verifying it is a String and
that it resides under your app/widget data directory or matches the expected
storage filename pattern (use a canonical/normalized path check), (2) perform
file operations asynchronously (await file.exists() and await file.delete()) to
avoid sync I/O blocking, and (3) only delete when deleteFile==true and the path
validation passes; update references to getWidgetData and the File unlink logic
inside saveWidgetData accordingly.
- Around line 163-185: The current _normalizeExtension and _validateKey use
assert() so checks are stripped in release builds; replace those asserts with
runtime exceptions (e.g., throw ArgumentError or FormatException) so invalid
inputs cannot reach path construction from public APIs (saveFile, saveImage,
renderFlutterWidget). In _normalizeExtension(String extension) keep the trim and
leading-dot removal, then if ext.isEmpty throw a descriptive exception and if
ext contains '/' or '\' or '..' throw an exception stating invalid/forbidden
characters. In _validateKey(String key) check key.isNotEmpty and throw if empty,
and if key contains '/' or '\' or '..' or ' ' throw a descriptive exception;
ensure messages reference the offending parameter for easier debugging.
---
Nitpick comments:
In `@examples/file_and_images/README.md`:
- Line 3: The intro sentence in the README is awkwardly worded; replace it with
a polished version to fix grammar and casing: change "A simple Demo app showing
how to save Images and Files from Flutter and accessing the Data from native
Widgets" to "A simple demo app showing how to save images and files from Flutter
and access the data from native widgets." Update the README intro sentence
accordingly (the first descriptive sentence in the file) so casing is consistent
and verbs match.
In `@packages/home_widget/example/integration_test/android_test.dart`:
- Around line 26-30: The setup cleanup only clears keys from testData but misses
the file-API test keys (and one file left on disk via deleteFile: false), which
can leak state; update the setUp to also iterate over the file-API test keys and
call HomeWidget.saveWidgetData(key, null) for each of them (or use the file-API
teardown to remove the file path), and for the specific test that used
deleteFile: false ensure the leftover path is explicitly removed during
setup/teardown (or flip deleteFile to true) so no file remains between runs;
reference testData, setUp, HomeWidget.saveWidgetData and the deleteFile flag
when making the change.
In `@packages/home_widget/example/integration_test/ios_test.dart`:
- Around line 38-45: The setup cleanup only clears keys in testData.keys but
misses the new file/image test keys and the case where deleteFile: false can
leave artifacts; update the setUp block (referencing setUp and
HomeWidget.saveWidgetData) to also iterate and clear the file/image test keys
(e.g., the constants or list you added for file tests) and ensure you call
HomeWidget.saveWidgetData for those keys with null and deleteFile: true (or
explicitly remove the files via the appropriate HomeWidget file-delete API) so
no artifacts remain between reruns.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 8a1c27c6-b3a6-4ffa-b37d-bf8e0316dadc
📒 Files selected for processing (4)
examples/file_and_images/README.mdpackages/home_widget/example/integration_test/android_test.dartpackages/home_widget/example/integration_test/ios_test.dartpackages/home_widget/lib/src/home_widget.dart
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/home_widget/lib/src/home_widget.dart (1)
170-173:⚠️ Potential issue | 🟠 MajorHarden managed-path detection before delete.
Line 172 uses substring matching only. A path like
/.../home_widget/../...still passes and can point outside the managed directory when Line 31 decides deletion eligibility.🛡️ Suggested hardening
-static bool _isHomeWidgetManagedFilePath(String path) { - final normalized = path.replaceAll(r'\', '/'); - return normalized.contains('/home_widget/'); -} +static bool _isHomeWidgetManagedFilePath(String path) { + final normalized = File(path).absolute.path.replaceAll(r'\', '/'); + return normalized.contains('/home_widget/') && + !normalized.contains('/../') && + !normalized.endsWith('/..'); +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/home_widget/lib/src/home_widget.dart` around lines 170 - 173, The _isHomeWidgetManagedFilePath function currently checks for '/home_widget/' via simple substring matching which can be bypassed by path segments like '../'; change it to normalize and resolve the path components (replace backslashes, use path normalization/canonicalization to collapse '..' and '.') and then check that one of the path segments equals 'home_widget' (e.g., split the normalized path and verify a segment matches exactly) or verify the resolved absolute path is contained within the intended managed directory root; update _isHomeWidgetManagedFilePath to perform this canonical check so deletion eligibility (used elsewhere) cannot be tricked by crafted paths.
🧹 Nitpick comments (1)
packages/home_widget/lib/src/home_widget.dart (1)
257-259: Preserve stack trace when rethrowing save failures.Line 258 wraps errors but discards the original stack trace, which makes failures harder to debug.
🔧 Suggested refactor
- } catch (e) { - throw Exception('Failed to save file to widget container: $e'); + } catch (e, st) { + Error.throwWithStackTrace( + Exception('Failed to save file to widget container: $e'), + st, + ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/home_widget/lib/src/home_widget.dart` around lines 257 - 259, The catch block that currently does "catch (e) { throw Exception('Failed to save file to widget container: $e'); }" discards the original stack trace — change it to capture the stack trace (catch (e, st)) and rethrow the new Exception while preserving the original stack trace using Error.throwWithStackTrace(Exception('Failed to save file to widget container: $e'), st); this keeps the descriptive message and the original stack trace for debugging.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/home_widget/lib/src/home_widget.dart`:
- Around line 226-234: The assert on HomeWidget.groupId is inactive in release
builds and the subsequent force-unwrap (HomeWidget.groupId!) can crash; replace
the assert with an explicit runtime guard that checks HomeWidget.groupId and, if
null, throws a StateError with a clear message (e.g., "No groupId defined. Did
you forget to call `HomeWidget.setAppGroupId`?") before calling
PathProviderFoundation().getContainerPath so getContainerPath is only invoked
with a non-null appGroupIdentifier.
---
Duplicate comments:
In `@packages/home_widget/lib/src/home_widget.dart`:
- Around line 170-173: The _isHomeWidgetManagedFilePath function currently
checks for '/home_widget/' via simple substring matching which can be bypassed
by path segments like '../'; change it to normalize and resolve the path
components (replace backslashes, use path normalization/canonicalization to
collapse '..' and '.') and then check that one of the path segments equals
'home_widget' (e.g., split the normalized path and verify a segment matches
exactly) or verify the resolved absolute path is contained within the intended
managed directory root; update _isHomeWidgetManagedFilePath to perform this
canonical check so deletion eligibility (used elsewhere) cannot be tricked by
crafted paths.
---
Nitpick comments:
In `@packages/home_widget/lib/src/home_widget.dart`:
- Around line 257-259: The catch block that currently does "catch (e) { throw
Exception('Failed to save file to widget container: $e'); }" discards the
original stack trace — change it to capture the stack trace (catch (e, st)) and
rethrow the new Exception while preserving the original stack trace using
Error.throwWithStackTrace(Exception('Failed to save file to widget container:
$e'), st); this keeps the descriptive message and the original stack trace for
debugging.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 31daf0b2-b3d2-4ba5-9fbe-3e418b8aa405
📒 Files selected for processing (5)
examples/file_and_images/README.mdpackages/home_widget/example/integration_test/android_test.dartpackages/home_widget/example/integration_test/ios_test.dartpackages/home_widget/lib/src/home_widget.dartpackages/home_widget/test/home_widget_test.dart
✅ Files skipped from review due to trivial changes (1)
- examples/file_and_images/README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/home_widget/test/home_widget_test.dart
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
HomeWidget.saveFileandHomeWidget.saveImageChecklist
exampleor documentation.Breaking Change?
Related Issues