-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Introduce abstract storage layer #9544
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@d2dyno1 We agree on the essentials. I will clarify my vocabulary so we understand the same thing. Storage. This is a technical layer that uses IStorageItem from uwp. This roughly corresponds to the current Files.Uwp.Filesystem namespace. It is a technical layer but is used at high level by the application (wrongly). Storage uses FullTrust but this is a limitation of uwp. I hadn't planned to use an abstraction for this layer because ListedItem will encapsulate it and it will be used a lot less. However, your suggestion to add an abstraction convinced me. This will allow us to easily replace this layer if necessary, especially if we leave uwp. Filesystem. This is an abstraction that encapsulates Storage (and libraries such as FluentFTP) to be able to manipulate ftp cloud zip and other system files and folders. It roughly corresponds to ListedItem and operations (copy, rename, ...). Currently, there is no abstraction of operations. The ListedItem abstraction has major flaws and is not standardized. Operations directly use the storage layer, which should be avoided. Filesystem contains 2 parts. The sdk (public) and the implementation (private, except enumeration services or factory to be able to instantiate the services). I had planned to place these 2 parts in the same project. The separation can be done later if necessary. I had planned to place Storage in the Files.Filesystem project, without abstraction, but encapsulated. An important point is the breakdown of the application into several projects. I tend to create a lot of projects for an application to have a clear and impossible to violate architecture. I didn't apply it here because there may be team objections to creating multiple projects. Here is a suggestion of some projects we could create. |
Let's keep things simple for now. We'll try to start with this layout in mind: Files.Sdk.Storage - storage abstractions We can later on decide and separate the ListedItem layer (we want to avoid confusion and have a clear foundation that we can expand upon later). The current goal is to separate the storage (- ftp, win32, shell, etc.) from the backend (- ViewModels). The abstractions for backend view models are out of scope of this PR 🙂 |
I've implemented storage abstractions for FTP. We'll need to implement other classes this way as well :) |
This is ok for me. |
@yaichenbaum @d2dyno1 @gave92 Here are some ideas about Storage. Storage is the part of the application that manages file systems (whatever they are). You need a Storage backend and a Storage viewModel. The backend needs an abstraction, many implementations (which may change if we leave uwp), tests, etc. This layer must be self-contained, with no dependencies on user settings or localization. This part is not specific to Files. The viewmodel will present Storage in a more convenient way. It will be independent of the real file system (cloud, ftp, ...) but will use user settings and location. This part is specific to Files. The most important point is therefore the backend. A CommunityToolkit.Storage project is in the works, but we can't afford to wait for it (if it comes to fruition). I propose to no longer integrate Storage into the Files project. It is a standalone project that could be used by another application (which is not a file manager). We can create a new Repository project "Files Community/Storage". This will contain projects for abstraction, uwp implementations, testing, and its own documentation. This ensures our own independence. In parallel, we can collaborate with CommunityToolkit.Storage to integrate our own improvements. This with the aim, in the long term, of using it to replace our solution (which allows us to have a head start). |
What bothers me most is the IModifiable interface.
One solution is to provide an IStorableAction type interface that contains Note: A handy option to add would be an option in the copy list for limit to a single current copy. It's more efficient. It should be possible to choose which one is in progress. Steam has a similar behavior for multiple downloads.
I haven't read it all yet. This is a first list of remarks. It may seem like a lot of remarks but it's a great job and I quibble. |
The current names are gramatically consistent
I don't know what arguments you're referring to
The delete operation should be on parent folder since it needs to be contained within some container to be able to delete it. The parent folder also needs to be notified of the deletion
If a folder is read-only, it cannot be cast to
These could be extended with additional interfaces or wrapped around interfaces like |
@gave92 can I merge this? |
Details of Changes
This PR continues work on the infrastructure, making the storage layer more broad and flexible. The motivation for this change is to eliminate the convoluted file system code that Files is built on - this requires major changes both in frontend and backend part of the codebase. This work is a stretch goal to ultimately replace all storage calls with
Files.Sdk.Storage
abstractions. This PR is a work-in-progress.TODO
The following list is limited to this PR and doesn't include future changes
Implement NativeFile/FolderImplement ShellFile/FoldeImplement SystemFile/FolderImplement ZipFile/Folder(Crossed out items will be done in following PRs)
Validation
How did you test these changes?