Deduplicate file names by adding a suffix on the file name

[jenseralmeida]

Fixes #620

[bartekpacia]

What about iOS?

[jenseralmeida]

What about iOS?

iOS has a different model for the file system, where we do not share files between applications, without using the Share option. Besides that, the lines 948 to 950 in FlutterDownloaderPlugin.m, highlighted below remove the file before downloading it. In that way, duplicate file names does not appear to be an issue for iOS.

        if ([fileManager fileExistsAtPath:[destinationURL path]]) {
            [fileManager removeItemAtURL:destinationURL error:nil];
        }

So, my question is, should I change the above lines to replicate the Android behaviour? If you want to change this, I can do it on this PR, or as a separate PR.

[bartekpacia]

Ah, OK, thanks for explaining :)

1. What happens currently on Android when file name already exists?

2. Regarding the issue – I think that the most important thing is to have the same behavior across all platforms so that if a file with the same name on iOS exists, it is deleted, then Android should work the same way. We risk overwriting some user files, but I don't see a better solution as of now. Alternatively, we could other way around and deduplicate file names on iOS as well – then I'd ask you to write some Objective-C to implement this on iOS.

3. In the longer term, I think we could provide some mechanism like FileSaveStrategy, which would be an enum with 3 states: fail, duplicate, or overwrite. User would configure this strategy globally, in the first call to initialize(), and also per-call, in enqueue.

Overall, what do you think @jenseralmeida?

[jenseralmeida]

I think we should change the iOS behavior and deduplicate the file name also. I'll work on the objective-C and push an update anytime next week.

I do agree also to add the options as a long-term solution, as we can have a fix for an issue ASAP, and a better plan to manage the files on a future version.

Together with the long-term solution, I think that a new SourceFileName or OriginalFileName property should be added to the DownloadTask class, allowing better control of what to do, by the plugin users.

Best regards,

Jenser Almeida

---

From: Bartek Pacia @.> Sent: Friday, August 19, 2022 12:44:46 PM To: fluttercommunity/flutter_downloader @.> Cc: Jenser Almeida @.>; Mention @.> Subject: Re: [fluttercommunity/flutter_downloader] Deduplicate files names by adding a (#dedup-name) suffix on the file name (PR #708)

Ah, OK, thanks for explaining :)

1. What happens currently on Android when file name already exists?

2. Regarding the issue – I think that the most important thing is to have the same behavior across all platforms so that if a file with the same name on iOS exists, it is deleted, then Android should work the same way. We risk overwriting some user files, but I don't see a better solution as of now.

Alternatively, we could other way around and deduplicate file names on iOS as well – then I'd ask you to write some Objective-C to implement this on iOS.

1. In the longer term, I think we could provide some mechanism like FileSaveStrategy, which would be an enum with 3 states: fail, duplicate, or overwrite.

Overall, what do you think @jenseralmeidahttps://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fjenseralmeida&data=05%7C01%7C%7C168274d2a94043f19cba08da821b4592%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637965350885605722%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=Ox3xRhXhjL0OlB4cSnstlzvpGna4Vx0qBu5PACr467U%3D&reserved=0?

— Reply to this email directly, view it on GitHubhttps://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ffluttercommunity%2Fflutter_downloader%2Fpull%2F708%23issuecomment-1221038592&data=05%7C01%7C%7C168274d2a94043f19cba08da821b4592%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637965350885605722%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=wrup1BSIJr6pIjnnAJA1%2BmGpDPz3zyaiFR5UVY87BEE%3D&reserved=0, or unsubscribehttps://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAAFBE2ZFB2PGNLZKP2FVSTDVZ7P25ANCNFSM567BEH7Q&data=05%7C01%7C%7C168274d2a94043f19cba08da821b4592%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637965350885605722%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=7BtbNm1vkSfRLMusDkhZlfydQ7Lj8JwVkoMAzPR24Cw%3D&reserved=0. You are receiving this because you were mentioned.Message ID: @.***>

[jenseralmeida]

Sorry, for the first question, about what happens to Android when the file name already exists, the answer is, the download fails, without any reasonable feedback.

Please, look into the issue #620 for the details, and the reason for this PR is to fix this issue, which in my case, is a blocker to promote an application to production.

Best regards,

Jenser Almeida

---

From: Jenser Almeida @.> Sent: Friday, August 19, 2022 2:02:58 PM To: fluttercommunity/flutter_downloader @.>; fluttercommunity/flutter_downloader @.> Cc: Mention @.> Subject: Re: [fluttercommunity/flutter_downloader] Deduplicate files names by adding a (#dedup-name) suffix on the file name (PR #708)

I think we should change the iOS behavior and deduplicate the file name also. I'll work on the objective-C and push an update anytime next week.

I do agree also to add the options as a long-term solution, as we can have a fix for an issue ASAP, and a better plan to manage the files on a future version.

Together with the long-term solution, I think that a new SourceFileName or OriginalFileName property should be added to the DownloadTask class, allowing better control of what to do, by the plugin users.

Best regards,

Jenser Almeida

---

From: Bartek Pacia @.> Sent: Friday, August 19, 2022 12:44:46 PM To: fluttercommunity/flutter_downloader @.> Cc: Jenser Almeida @.>; Mention @.> Subject: Re: [fluttercommunity/flutter_downloader] Deduplicate files names by adding a (#dedup-name) suffix on the file name (PR #708)

Ah, OK, thanks for explaining :)

1. What happens currently on Android when file name already exists?

2. Regarding the issue – I think that the most important thing is to have the same behavior across all platforms so that if a file with the same name on iOS exists, it is deleted, then Android should work the same way. We risk overwriting some user files, but I don't see a better solution as of now.

Alternatively, we could other way around and deduplicate file names on iOS as well – then I'd ask you to write some Objective-C to implement this on iOS.

1. In the longer term, I think we could provide some mechanism like FileSaveStrategy, which would be an enum with 3 states: fail, duplicate, or overwrite.

Overall, what do you think @jenseralmeidahttps://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fjenseralmeida&data=05%7C01%7C%7C168274d2a94043f19cba08da821b4592%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637965350885605722%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=Ox3xRhXhjL0OlB4cSnstlzvpGna4Vx0qBu5PACr467U%3D&reserved=0?

— Reply to this email directly, view it on GitHubhttps://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ffluttercommunity%2Fflutter_downloader%2Fpull%2F708%23issuecomment-1221038592&data=05%7C01%7C%7C168274d2a94043f19cba08da821b4592%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637965350885605722%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=wrup1BSIJr6pIjnnAJA1%2BmGpDPz3zyaiFR5UVY87BEE%3D&reserved=0, or unsubscribehttps://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAAFBE2ZFB2PGNLZKP2FVSTDVZ7P25ANCNFSM567BEH7Q&data=05%7C01%7C%7C168274d2a94043f19cba08da821b4592%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637965350885605722%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=7BtbNm1vkSfRLMusDkhZlfydQ7Lj8JwVkoMAzPR24Cw%3D&reserved=0. You are receiving this because you were mentioned.Message ID: @.***>

[bartekpacia]

@jenseralmeida Okay, so I'm waiting for you to add deduplication of file name for iOS as well, and then I'll merge this because we'll have consistent behavior across iOS and Android.

Also, please take a look at my comment :)

[bartekpacia]

Also:

Together with the long-term solution, I think that a new SourceFileName or OriginalFileName property should be added to the DownloadTask class, allowing better control of what to do, by the plugin users.

I think this is a great idea. I needed this feature in my private app which uses this plugin (in the end, I think I found some workaround or something – don't remember exactly). If you have time and are willing to contribute this feature as well, I'd be more than happy to review it and (hopefully) merge :)

[IlyaMax]

Why is this done not on dart side, if you want this to be on both ios/android?

[bartekpacia]

@IlyaMax Currently all the Dart side does is forwarding methods to native side. I agree that having more logic on the Dart side would be desirable (some work is being done here), but I'd prefer not make such big changes in this PR.

[IlyaMax]

@bartekpacia I've made simple function and written tests for it. Why you think that big changes is bad in this case? It doesn't break the api as I understand.

class FileUtils {
  final FileOperator _fileOperator;

  FileUtils(this._fileOperator);
  String deduplicatedFileName(String savedDir, String fileName) {
    final fileExists = _fileOperator.isFileWithNameExists('$savedDir/$fileName');
    if (fileExists) {
      final startIndexOfExtension = fileName.lastIndexOf('.');
      final fileExtension = fileName.substring(startIndexOfExtension);
      final withoutExtension = fileName.substring(0, startIndexOfExtension);
      final startIndexOfPotentialDuplicationNumber =
          withoutExtension.lastIndexOf('(');
      final duplicationNumber = int.tryParse(withoutExtension.substring(
        startIndexOfPotentialDuplicationNumber + 1,
        withoutExtension.length - 1,
      ));
      if (duplicationNumber == null) {
        return '$withoutExtension(1)$fileExtension';
      } else {
        final withoutDuplicationNumber =
            fileName.substring(0, startIndexOfPotentialDuplicationNumber);
        return '$withoutDuplicationNumber(${duplicationNumber + 1})$fileExtension';
      }
    }
    return fileName;
  }
}

[bartekpacia]

@IlyaMax I'm only maintaining this package and would prefer not to break any dependents. Just trying to stay on the safe side. But now I see that indeed, we can duplicate file names on the Dart side.

After all, this is @jenseralmeida's PR, so it's up to him to implement this functionality. You might want to create a PR to his fork with your Dart changes.

[jenseralmeida]

Hi @IlyaMax, I can take a look into it, and as I did not write it on Objective-C yet, and @bartekpacia feels that it does fit as a safe option then I will move it out from Java to Dart and apply it to both platforms.

On the timeframe side, I am stuck with other things, so I am not sure when I will work on this. If @IlyaMax wants to open another PR with his proposed changes, he can do it. When I find time to work on this, I will check if this is still a pending issue and complete its work.

[lugael]

Hi, any news about this?

[bartekpacia]

@lugael Hi, unfortunately, no news. If you're willing to pick this up, then here's a rough TODO list:

• add unit tests to the existing Java implementation (done by the author of this PR)
• write deduplication for iOS in Obj-C
• test it and make sure that it's not breaking