search

tusdotnet / tusdotnet

.NET server implementation of the Tus protocol for resumable file uploads. Read more at https://tus.io
MIT License
664 stars 70 forks source link

Use System.IO.Abstractions in TusDiskStore #224

Open sant11h opened 3 weeks ago

sant11h commented 3 weeks ago

Hello! I wanted to ask for a refactor in TusDiskStore. More specifically in InternalFileRep, all file operations are handled there but it is using the File static class. Using that approach we cannot inject a custom FileSystem (e.g.: a mock file system) for testing purposes.

The proposal is to refactor that code and use the interface IFileSystem provided by the package System.IO.Abstractions. With this approach we still support the current implementation and we open the possibility to inject a custom file system.

Thoughts?

smatsson commented 3 weeks ago

What's the use case where you need to test the internals of TusDiskStore? This seems like the wrong type of abstraction to me. I'm also not happy about adding a dependency on some third party lib using the System.IO namespace.

The correct way to do this is to mock the entire store which is already supported through ITusStore and friends. See for instance https://github.com/tusdotnet/tusdotnet/blob/master/Source/tusdotnet.test/Extensions/ITusStoreExtensions.cs#L30 that uses NSubstitute for mocking.

sant11h commented 3 weeks ago

I'm not testing any internals of TusDiskStore, but I have some tests that involves using the TUS endpoint for creating a file, but since in our code we use the abstraction IFileSystem with a MockFileSystem implementation we have a problem because TUS writes files using the File static class and the files that are created by TUS are not visible in the MockFileSystem. They are basically in different file systems, makes sense?

smatsson commented 1 week ago

@sant11h Excuse the late reply, life got in the way. I understand what you are trying to achieve, and I would consider adding support for this if the library you are using were an official package from Microsoft. The problem now is that I am adding a dependency on a library that I do not control, which locks me into using this library indefinitely. This makes it more challenging to develop the disk store and also opens up potential security issues. The correct way to test tusdotnet is to abstract away the store entirely, similar to how you would abstract away the database connection completely rather than replacing the TCP/IP layer within the connection (a somewhat flawed comparison but generally accurate).

I would recommend you look at the code linked from my previous post or check out the InMemory store available at https://github.com/tusdotnet/tusdotnet/blob/master/Source/tusdotnet.benchmark/InMemoryStore.cs.