Skip to content
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

Dependency on hard-coded DiskStore #78

Closed
d2dyno1 opened this issue Nov 4, 2024 · 3 comments
Closed

Dependency on hard-coded DiskStore #78

d2dyno1 opened this issue Nov 4, 2024 · 3 comments

Comments

@d2dyno1
Copy link

d2dyno1 commented Nov 4, 2024

Background

NWebDav relies primarily on hard-coded file handling. Its dependency on FileInfo and DirectoryInfo prevents potential users from consuming the API on platforms with unordinary/limited file system access (like Android or iOS).

The problem is compounded when trying to re-implement the DiskStore layer -- the whole code needs to be duplicated in order to apply minor modifications (depending on the requirements, of course).

Solution

A potential solution would be to drop both FileInfo and DirectoryInfo and replace them with IFile and IFolder essentially wrapping the storage access (the native file system) with a WebDav layer.

Architecture

To achieve such abstraction, the whole file system layer would have to be reengineered. A good place to start would be to incorporate (or at least partially implement) an existing storage layer abstraction library.

I had great success with OwlCore.Storage as my go-to solution in projects. It's incredibly flexible and even the most basic interfaces could be added to work with the existing code with minimal changes. Please read the Motivation section of the library before proceeding (The readme is a bit dated but it should still convey the main idea behind the logic. Nevertheless, please take a look at the current interfaces and extensions)

Regarding the architecture of NWebDav specifically, a few poor decisions were made with the organization of base interfaces, namely IStoreItem and IStoreCollection. These have trickled-down causing unnecessary methods to be exposed in the API. The most reasonable way to approach this issue would be to use a base interface for both like OwlCore's IStorable:

public interface IStorable
{
  string Id { get; } // Equivalent of UniqueKey, serves as a path

  string Name { get; } // Already present
}

This in turn would allow for using an abstracted base without substituting IStoreCollection with IStoreItem (which was made to work with files).

// IFile -> IStorable
public interface IStoreItem : IFile
{
  // Properties: Name and Id (UniqueKey), are already present in IStorable
  IPropertyManager? PropertyManager { get; }

  // Methods: An unified method OpenStreamAsync is already present in IFile
  
  // TODO: Find a way to use OpenStreamAsync instead of UploadFromStreamAsync
  Task<DavStatusCode> UploadFromStreamAsync(Stream source, CancellationToken cancellationToken);

  // Note the absence of CopyAsync
}

...and the IStoreCollection would no longer inherit from IStoreItem getting rid of redundant stream-opening methods:

// IGetFirstByName -> IFolder -> IStorable
// IModifiableFolder -> IFolder
public interface IStoreCollection : IGetFirstByName, IModifiableFolder
{
  // Properties: Name and Id (UniqueKey), are already present in IStorable
  IPropertyManager? PropertyManager { get; }

  InfiniteDepthMode DepthMode { get; }

  // Methods: An unified method GetItemsAsync is already present in IFolder
  //   GetFirstByNameAsync is already present in IGetFirstByName
  //   DeleteAsync(IStorableChild, CancellationToken) is already present in IModifiableFolder
  //   CreateFileAsync(string, bool, CancellationToken) - IModifiableFolder
  //   CreateFolderAsync(string, bool, CancellationToken) - IModifiableFolder
  // (Note that some parameters are different)

  // Note the absence of SupportsFastMove, MoveItemAsync, CopyAsync
}

Storage operations

The lack of Copying and Moving methods on the base interfaces is deliberate. It is the responsibility of the implementor to specify whether it can support such operations; For this, two new interfaces should be added to support moving and copying of items. Keep in mind that OwlCore.Storage already includes methods for such use cases, however, these don't meet our needs:

// Derivative modification of: https://github.com/Arlodotexe/OwlCore.Storage/blob/main/src/Extensions/ICreateCopyOf.cs
public interface ISupportsCopy : IModifiableFolder
{
  Task<IStorableChild> CopyAsync(IStorable storable, bool overwrite, CancellationToken cancellationToken);
}

// Derivative modification of: https://github.com/Arlodotexe/OwlCore.Storage/blob/main/src/Extensions/IMoveFrom.cs
public interface ISupportsMove : IModifiableFolder
{
  Task<IStorableChild> MoveAsync(IStorableChild storable, IModifiableFolder source, bool overwrite, CancellationToken cancellationToken);
}

If no such interfaces are defined on an object (i.e. the implementation cannot be cast to a particular interface), an extension could be used to supplement the functionality. Overall, a copy operation is creating an item with identical contents, and move on the other hand is copying the data while also deleting the source -- all methods for doing so are already present in the IModifiableFolder.

Moving forward

The best way to encompass these abstractions would be to roll-out gradual changes. IFile, IFolder, and IStorable methods and properties should come first while the rest can be implemented later as time goes on.

@d2dyno1
Copy link
Author

d2dyno1 commented Nov 4, 2024

Related:

@ramondeklein
Copy link
Owner

This is exactly what IStoreCollection the interface is for. It has been used to also fetch information from databases to provide some kind of virtual hierarchy. One of the main goals of this library is to provide flexibility and don't rely on third party packages that may be poorly maintained.

The behavior that you need can be accomplished with IStoreCollection, so feel free to use that or fork this library instead.

@Arlodotexe
Copy link

Arlodotexe commented Nov 5, 2024

One of the main goals of this library is to provide flexibility and don't rely on third party packages that may be poorly maintained.

Understandable to want to avoid third-party dependencies, but it's worth noting exactly what this library is while we're here.

The OwlCore.Storage library enables early adoption of the proposed CommunityToolkit.Storage Labs experiment. Aware that creating a good storage abstraction is no small feat, we drafted an API surface that aimed to maximize flexibility while also minimizing effort on implementors, consumers and maintainers.

Since we began two years ago, we've had 33 releases with only 7 breaking updates. We've taken every precaution to make breaking changes both infrequent and less frequent over time.

The library is quite stable by now and is still under constant usage and maintenance, but until the library is out of both OwlCore and Toolkit Labs, it's potentially (but not likely) to receive further breaking changes as we learn more about the various scenarios the library is used in. Don't feel pressured to try or rely on it in the meantime.

Feedback from community members have been shaping this library since the first draft, so we'd be happy to hear from you. If you change your mind or have questions, we're always listening to feedback both in our repo and in our Discord channel in the Windows App Community.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants