Add ability to change [FileMode] of the download by introducing [Dio… #2281

shehabmohamed0 · 2024-08-14T01:34:21Z

…FileMode]

New Pull Request Checklist

[Yes] I have read the Documentation
[Yes] I have searched for a similar pull request in the project and found none
[Yes] I have updated this branch with the latest main branch to avoid conflicts (via merge from master or rebase)
[Yes] I have added the required tests to prove the fix/feature I'm adding
[Yes] I have updated the documentation (if necessary)
[Yes] I have run the tests without failures
[No] I have updated the CHANGELOG.md in the corresponding package

Additional context and info (if any)

Add the ability to change the [FileMode] of the download without altering the default value, which is [FileMode.write], to make resuming downloads easier to implement by introducing [DioFileMode] and mapping it to FileMode in [DioForNative]
without breaking web_adapter

…ileMode]

ueman · 2024-08-14T08:24:08Z

Can you please explain how this change enables resuming downloads?
Please also add tests for both cases, so it will not get broken in the future.

shehabmohamed0 · 2024-08-14T19:45:46Z

Can you please explain how this change enables resuming downloads? Please also add tests for both cases, so it will not get broken in the future.

When downloading a file a stream is opened and a Random Access File are opened in the client side to store bytes
the current implementation to open a Random Access File is [FileMode.write] this mode creates a new file and overwrite the existing one if it already exists

changing the file access mode to [FileMode.append] or [FileMode.writeOnlyAppend] with change the behavior to write to the end of the file

So if you had already started downloading a file and for some reason the download stopped let's say a network problem and you set [deleteOnError = false], the file had already downloaded 50% and it's not deleted it exists but is not complete

You check if the file exists and get it's size then you request to download the file again by requesting a partial content setting the range header to file size, that means skip downloading the 50% you already downloaded then you append the remaining 50% to the end of the file and here it's you resumed your download with minimum ease.

shehabmohamed0 · 2024-08-14T19:46:33Z

Can you please explain how this change enables resuming downloads? Please also add tests for both cases, so it will not get broken in the future.

Ok, I will add tests.

ueman · 2024-08-14T20:13:12Z

I wonder whether it makes sense to automatically add range header in the case of a resuming download, that way it would work out of the box, without the user needing to implement anything. The server needs to support them though. (This is not something which necessarily needs to be added to this PR though, I guess)

shehabmohamed0 · 2024-08-15T04:08:31Z

I wonder whether it makes sense to automatically add range header in the case of a resuming download, that way it would work out of the box, without the user needing to implement anything. The server needs to support them though. (This is not something which necessarily needs to be added to this PR though, I guess)

Yes, it's a good idea. I think it's rare to find a server that doesn't support range requests.
Should I change the implementation to just a boolean parameter in [Dio.download] called resume ?
If the server doesn't support range requests don't use it

shehabmohamed0 · 2024-09-02T10:20:06Z

I wonder whether it makes sense to automatically add range header in the case of a resuming download, that way it would work out of the box, without the user needing to implement anything. The server needs to support them though. (This is not something which necessarily needs to be added to this PR though, I guess)

I added test case for the file mode append, now can you review the code again

dio_test/lib/src/test/download_tests.dart

AlexV525 · 2024-10-19T11:16:46Z

dio/lib/src/dio_file_mode.dart

+/// - [DioFileMode.append]: Mode for opening a file for reading and writing
+///    to the end of it. The file is created if it does not already exist.
+/// {@endtemplate}
+enum DioFileMode {


The name is a bit overwhelming. The type is only used for the download process.

Also the enhanced enum is not working on our minimum SDK support.

I will convert it to extension

any suggestion for naming

dio/lib/src/dio/dio_for_native.dart

change [DioFileMode] to [FileAccessMode] change [Dio.download(fileMode:)] to [dio.download(fileAccessMode:)]

AlexV525 · 2024-11-26T03:26:46Z

@ueman Could you follow above reviews and stamp this if applicable?

dio/lib/src/file_access_mode.dart

dio_test/lib/src/test/download_tests.dart

Co-authored-by: Alex Li <[email protected]> Signed-off-by: shehab mohame <[email protected]>

Remove [FileAccessModeExtension]

AlexV525 · 2024-12-17T03:22:49Z

The CI is complaining about the v1 of dio_web_adapter, which should be updated after the PR has landed (I'll do that).

dio/lib/src/dio.dart

dio_test/lib/src/test/download_tests.dart

Co-authored-by: Alex Li <[email protected]> Signed-off-by: shehab mohame <[email protected]>

AlexV525

RSLGTM. Also you need to update CHANGELOGs for the main package and the web adapter, just see how other PRs do this.

AlexV525 · 2024-12-17T12:22:38Z

dio/lib/src/dio.dart

@@ -254,6 +258,7 @@ abstract class Dio {
    ProgressCallback? onReceiveProgress,
    CancelToken? cancelToken,
    bool deleteOnError = true,
+    FileAccessMode mode = FileAccessMode.write,


Oh this should be fileAccessMode.

AlexV525 · 2024-12-17T12:23:30Z

dio/lib/src/dio_mixin.dart

@@ -286,6 +286,7 @@ abstract class DioMixin implements Dio {
    ProgressCallback? onReceiveProgress,
    CancelToken? cancelToken,
    bool deleteOnError = true,
+    FileAccessMode mode = FileAccessMode.write,


This should be fileAccessMode too.

AlexV525 · 2024-12-17T12:23:56Z

dio_test/lib/src/test/download_tests.dart

+        );
+
+        final cancelToken2 = CancelToken();
+        var recievedBytes2 = 0;


Suggested change

var recievedBytes2 = 0;

int recievedBytes2 = 0;

Add ability to change [FileMode] of the download by introducing [DioF…

fe609eb

…ileMode]

shehabmohamed0 requested a review from a team as a code owner August 14, 2024 01:34

Add append download test with the new [DioFileMode]

3266e0f

shehabmohamed0 force-pushed the main branch from 7d721d9 to 3266e0f Compare September 1, 2024 06:53

shehabmohamed0 and others added 3 commits September 3, 2024 21:35

Merge branch 'main' into main

53a762d

Apply dart formatting

cf5f6bf

Merge branch 'main' into main

87ad6d2

shehabmohamed0 requested a review from a team as a code owner October 15, 2024 01:25

AlexV525 reviewed Oct 19, 2024

View reviewed changes

dio_test/lib/src/test/download_tests.dart Show resolved Hide resolved

AlexV525 reviewed Oct 19, 2024

View reviewed changes

dio/lib/src/dio/dio_for_native.dart Outdated Show resolved Hide resolved

shehabmohamed0 and others added 4 commits October 21, 2024 22:29

Merge branch 'main' into main

66de32f

change dio_file_mode.dart to file_access_mode.dart

8eee312

change [DioFileMode] to [FileAccessMode] change [Dio.download(fileMode:)] to [dio.download(fileAccessMode:)]

Merge branch 'main' into main

0c4614c

Merge branch 'main' into main

6f2d1ab

Merge branch 'main' into main

9a9ee71

AlexV525 requested changes Dec 16, 2024

View reviewed changes

dio/lib/src/file_access_mode.dart Outdated Show resolved Hide resolved

dio/lib/src/file_access_mode.dart Outdated Show resolved Hide resolved

dio_test/lib/src/test/download_tests.dart Show resolved Hide resolved

shehabmohamed0 and others added 3 commits December 16, 2024 17:58

Apply suggestions from code review

27bb332

Co-authored-by: Alex Li <[email protected]> Signed-off-by: shehab mohame <[email protected]>

Moving FileAccessMode from "file_access_mode.dart" to "options.dart"

4276742

Remove [FileAccessModeExtension]

Merge branch 'main' into main

238e185

AlexV525 reviewed Dec 17, 2024

View reviewed changes

dio/lib/src/dio.dart Outdated Show resolved Hide resolved

dio_test/lib/src/test/download_tests.dart Show resolved Hide resolved

Fix [FileAccessMode] comments in dio/lib/src/dio.dart

6c45ae5

Co-authored-by: Alex Li <[email protected]> Signed-off-by: shehab mohame <[email protected]>

format dio_test/lib/src/test/download_tests.dart

c2a23e8

Co-authored-by: Alex Li <[email protected]> Signed-off-by: shehab mohame <[email protected]>

shehabmohamed0 requested a review from AlexV525 December 17, 2024 10:26

AlexV525 reviewed Dec 17, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ability to change [FileMode] of the download by introducing [Dio… #2281

Add ability to change [FileMode] of the download by introducing [Dio… #2281

shehabmohamed0 commented Aug 14, 2024 •

edited

Loading

ueman commented Aug 14, 2024

shehabmohamed0 commented Aug 14, 2024

shehabmohamed0 commented Aug 14, 2024

ueman commented Aug 14, 2024

shehabmohamed0 commented Aug 15, 2024

shehabmohamed0 commented Sep 2, 2024

AlexV525 Oct 19, 2024

shehabmohamed0 Oct 20, 2024

shehabmohamed0 Oct 20, 2024

AlexV525 commented Nov 26, 2024

AlexV525 commented Dec 17, 2024

AlexV525 left a comment

AlexV525 Dec 17, 2024

AlexV525 Dec 17, 2024

AlexV525 Dec 17, 2024

Add ability to change [FileMode] of the download by introducing [Dio… #2281

Are you sure you want to change the base?

Add ability to change [FileMode] of the download by introducing [Dio… #2281

Conversation

shehabmohamed0 commented Aug 14, 2024 • edited Loading

New Pull Request Checklist

Additional context and info (if any)

ueman commented Aug 14, 2024

shehabmohamed0 commented Aug 14, 2024

shehabmohamed0 commented Aug 14, 2024

ueman commented Aug 14, 2024

shehabmohamed0 commented Aug 15, 2024

shehabmohamed0 commented Sep 2, 2024

AlexV525 Oct 19, 2024

Choose a reason for hiding this comment

shehabmohamed0 Oct 20, 2024

Choose a reason for hiding this comment

shehabmohamed0 Oct 20, 2024

Choose a reason for hiding this comment

AlexV525 commented Nov 26, 2024

AlexV525 commented Dec 17, 2024

AlexV525 left a comment

Choose a reason for hiding this comment

AlexV525 Dec 17, 2024

Choose a reason for hiding this comment

AlexV525 Dec 17, 2024

Choose a reason for hiding this comment

AlexV525 Dec 17, 2024

Choose a reason for hiding this comment

shehabmohamed0 commented Aug 14, 2024 •

edited

Loading