Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ async Task<string> InternalSaveAsync(string initialPath, string fileName, Stream
taskCompetedSource?.TrySetCanceled(CancellationToken.None);
var tcs = taskCompetedSource = new(cancellationToken);

documentPickerViewController = new([fileUrl])
documentPickerViewController = new([fileUrl], true)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we delete temp file in this case if we copy it instead of move?

Copy link
Author

Choose a reason for hiding this comment

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

I think so. I started refactoring this, I wanted to see if I could make it a more logical flow, and added a Cleanup method that handled that and the dispose. But I didn't want to introduce too many changes.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ideally if we don't use the temp file at all and write directly to the target location.

To make it simple for now you can wrap the code in try/finally block and delete temp file in finally section if it exists. It should not add complexity and many changes.

Copy link
Author

Choose a reason for hiding this comment

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

Ideally if we don't use the temp file at all and write directly to the target location.

I've spent a bit of time looking into this and I don't think it's practical. The only way to write directly outside the app's sandbox is using a security scoped URL and something like NSFileCoordinator. The problem is, this requires the user to grant access to it first, so it's not something we can do with a new file.

Additionally, we would have to track that the user has granted access and have safety checks that fall back to the temp file approach if not. And the problem with the first part is that it requires app-wide tracking and coordination. It's not impossible (far from it) but it's outside the scope of the vertical slice of this API.

The current approach (creating a temp file, then moving) is the canonical approach with Apple's API.

To make it simple for now you can wrap the code in try/finally block and delete temp file in finally section if it exists. It should not add complexity and many changes.

Agreed 🙂 Done in 47ddfe5. Please let me know if this is ok, or if you want me to make any other changes.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I made a tiny change to ensure the temp file is always deleted. Please let me know if it looks good

{
DirectoryUrl = NSUrl.FromString(initialPath)
};
Expand Down
Loading