FilePicker in UWP now handles more than 1,000 files

[DanielGilbert]

Description of Change

This moves the addition of every file to the FutureAccessList out of the library, as the returned token was never used. If really needed, the app can add the StorageFile object to the FutureAccessList on their own, as stated in Microsoft's Remarks on this class.

I couldn't see a way to add tests for the file picker, as its part of the system UI, so I omitted this. Also, no samples were needed, as it's not a feature that got added. Same goes for the documentation, as the previous behaviour wasn't documented.

Of course, I'm open to suggestions. :)

Bugs Fixed

• [Bug] [UWP] App crashes when picking more than 1,000 files #2061

API Changes

None

Behavioral Changes

The library no longer adds every StorageFile object to the FutureAccessList.

PR Checklist

• Has tests (if omitted, state reason in description)
• Has samples (if omitted, state reason in description)
• Rebased on top of main at time of PR
• Changes adhere to coding standard
• Updated documentation (see walkthrough)

[jfversluis]

So, in order to not lose this entire functionality here, could we consider adding the first 1000? Or better yet I guess, you want to have the latest ones, so make sure to only take the last 1000 of whatever was picked?

[DanielGilbert]

Yeah, that won't solve the problem either... I just realized that there is no way to get the IStorageFile object from the FileResult. I'm currently thinking about how to tackle that. The main issue is that, after time, the list will run full if it's in the scope of the library - and you, the consuming developer, will simply have no clue what's going on. 😅
And also, it's not possible to get the hash you'd need to access the item from the list in the current implementation.

I'm afraid this needs a cleaner implementation. So, the problem we currently have is quite easy to reproduce. Taking my example project from the PR - if you extend it by this:

var fileResult = await FilePicker.PickMultipleAsync(); //Pick at least one file

var firstResult = fileResult.FirstOrDefault();
string fullPath = firstResult.FullPath;

//Let's pretend we do not use firstResult.OpenReadAsync(),
//but instead create a new object:

var fileStream = File.OpenRead(fullPath); //System.UnauthorizedAccessException: 'Access to the path 'D:\Sample Data\random00000.test' is denied.'

You won't be able to access the file from the path alone. The implementation as-is is kind of useless - unless you go and iterate through the list itself, which is quite time consuming.

For UWP, it is necessary to have access to the IStorageFile object that the FilePicker returns. Either that, or we need a way to explicitly add it to the StorageApplicationPermissions.FutureAccessList from the FileResult, where we also return the associated hash for that list:

string hash = FileResult.AddToFutureAccessList();

With that, each item in the UWP implementation could be stored for later use without exposing the IStorageFile. And the function would return the hash that is necessary to retrieve the item later. Which would, of course, be a IStorageFile. So that would need a platform specific wrapper.

I would love to have a clean implementation on this one.

[jfversluis]

But we know how many files have been picked, right?

So can't we just add something smart like (pseudo code):

var skipCount = resultList.Count - 1000;

foreach (var file in resultList.Skip(skipCount))
    StorageApplicationPermissions.FutureAccessList.Add(file);

If we picked 1001 files, skipCount will now be 1 and the foreach will skip the first one and then proceeds to add the next 1000 entries. That way we still always get the last 1000, not losing any existing functionality, but it shouldn't break anymore. Right?

[DanielGilbert]

That way we still always get the last 1000, not losing any existing functionality, but it shouldn't break anymore. Right?

I wish it'd be that easy. But it's possible that there are already entries in this list, as the list is managed by the app itself. Sure, we could check how many entries are already in this list, and substract that from the amount of entries we could add. But that feels like duct-taping. Also, when selecting the first 1000 files, and then again selecting 1000 files, they won't get added to the list. But yes, it would solve the exception.

How do you think about my last paragraph, regarding the proposed cleaner implementation? I'd be willing to extend my PR in this regard, I just didn't manage to build Xamarin.Essentials on my system yet, as I'm running into errors regarding several dependencies.

[jfversluis]

I think that solution would imply adding more (public-ish) APIs which is what we try to avoid mostly unless we really need to do that for support scenarios and I wouldn't think this falls into that category to be honest.

@jamesmontemagno do you have any objections to this piece being removed? Technically its a breaking change from a functional standpoint, but my gut feeling says not too many people are heavily relying on this and they can easily implement something of their own in their own app which is "the right way"™️ of doing this anyway imo

[mattleibow]

I think the FileBase has a reference on the windows side? is this still required for WinUI?

[jfversluis]

I'm not sure what that means @mattleibow ?