search

Arlodotexe / OwlCore.Storage

The most flexible file system abstraction, ever. Built in partnership with the UWP Community.
16 stars 4 forks source link

`CreateCopyOfAsync` & `MoveFromAsync` slowpaths do not account for streams without ability to set length #70

Closed itsWindows11 closed 2 months ago

itsWindows11 commented 4 months ago

In some implementations such as FluentFTP, we use Nerdbank.Streams (specifically CombinedStream, which is full duplex), which throws when attempting to set length. Network streams generally do not support getting/setting length either.

In our slowpath implementation, we set the destination stream's length to 0 to ensure that there's no extra bytes when copying/moving.

We may need alternatives to setting the destination length for copying/moving.

Arlodotexe commented 4 months ago

Good catch. Perhaps a more fitting alternative is to delete the file from the destination folder.

More specifically, instead of calling DeleteAsync we'd be providing overwrite: true when calling CreateFileAsync on the destination, rather than passing it in from the containing method CreateCopyOfAsync.

This would not remove the need for the overwrite parameter in the CreateCopyOfAsync method except for the fallback delegate. That precheck runs before any fallback and non-fallback code and simply throws if overwrite is false and the file exists, which is functionality we want to keep.

Overwriting the existing file on create means we should get an empty file. This behavior is part of the storage contract and is enforced by our CommonTests. It should remove the need to call SetLength before copying data to the destination file.

Arlodotexe commented 2 months ago

Will try to get this out in the next update.