How should I register movers?

[binki]

I was playing about and came up with this code:

using SharpFileSystem;
using SharpFileSystem.FileSystems;
using System.IO;

class Program
{
    static void Main(string[] args)
    {
        var fs = new PhysicalFileSystem(".");
        var originalPath = FileSystemPath.Root.AppendFile("x");
        using (var x = fs.CreateFile(originalPath))
        using (var xWriter = new StreamWriter(x))
            xWriter.WriteLine("hi");
        fs.Move(originalPath, fs, FileSystemPath.Root.AppendFile("y"));
    }
}

However, it exits like this:

Unhandled Exception: System.ArgumentException: The specified combination of file-systems is not supported.
   at SharpFileSystem.FileSystemExtensions.Move(IFileSystem sourceFileSystem, FileSystemPath sourcePath, IFileSystem destinationFileSystem, FileSystemPath destinationPath) in C:\Users\ohnob\repos\sharpfilesystem\SharpFileSystem\FileSystemExtensions.cs:line 64
   at Program.Main(String[] args) in c:\users\ohnob\onedrive\documents\visual studio 2015\Projects\sharpfilesystemPlay\sharpfilesystemPlay\Program.cs:line 14

I see that there is a PhysicalFileSystemMover and I don’t get this error if I add this line:

        EntityMovers.Registration.AddLast(typeof(PhysicalFileSystem), typeof(PhysicalFileSystem), new PhysicalEntityMover());

But is this really something I should have to orchestrate manually—it feels like I’m writing boilerplate at that point? Is there a quick shortcut to loading all such available plugins, maybe using something like MEF? Could a small snippet be added to the README to demo some simple file operations to get people started in this framework?

On the other hand, I do like that this seems to be quite extensible. The pattern used by Move() is something I could use to easily add a safe, transactional Replace() without needing to alter sharpfilesystem’s code at all.

[FrozenCow]

I still need to write some documentation on this, though I'm not really sure the current manually 'registering'-approach is the right one.

However, I've added a test for Registration that should give you some impression how it works. The same can be done for EntityMovers.Registration and EntityCopiers.Registration.

Basically, moving files between two (or the same) physical filesystems should be fast. If the IFileSystem was used it would've gone slow because there is no relation to physical files in there: you'd basically needed to copy the file and delete afterwards. PhysicalEntityMover is only compatible with the PhysicalFileSystem-PhysicalFileSystem combination, nothing else.

The initial registration step might be simplified by adding a extension/static method to easily register the DefaultEntityMover and PhysicalEntityMover.

Also, wrapping PhysicalFileSystem using (for example) FileSystemWrapper or ReadOnlyFileSystem will fall back to the StandardEntityMover. Additions might be added to support PhysicalFileSystems inside wrapping ones.

About using MEF, or some kind of DI. Not all DIs support matching on two generic types. I don't want to depend on a single one, because that can be extremely annoying.

[binki]

Basically, moving files between two (or the same) physical filesystems should be fast.

I’m actually more worried about atomic operations than performance myself. When accessing a filesystem with a Windows API mindset, setting a file’s contents transactionally can be accomplished with a combination of File.Move() and File.Replace() (File.Replace() when the destination exists, File.Move() when it doesn’t). If coming from POSIX, you’ll be looking for a rename() which covers both situations. I have some code which moves things around which assumes the level of atomicity the real .net calls provide, but sharpfilesystem’s provision of StandardEntityMover makes it possible for something that looks like a .net Move() call to be non-atomic. Of course, as I mentioned earlier, I can always use a similar technique to the existing registration pattern to add my own atomic operations to make myself feel better ;-). Though it might be nice if sharpfilesystem provided an atomic Move()/Replace() equivalent out of the box too. I should open a new issue…

Also, wrapping PhysicalFileSystem using (for example) FileSystemWrapper or ReadOnlyFileSystem will fall back to the StandardEntityMover. Additions might be added to support PhysicalFileSystems inside wrapping ones.

Wrapping is an issue when trying to implement atomic operations using the registration+extension methods style. For ReadOnlyFileSystem, you never want the caller to be able to unwrap it. And, fair enough, you would never be able to perform atomic operations on a read only system. My idea for approaching the other filesystems is to have, e.g., MountedEntityMover support the pairs FileSystemMounter, IFileSystem and IFileSystem, FileSystemMounter, resolve the mountpoints, and then re-dispatch the Move() call on the underlying filesystems. Though maybe it would be better for the operation registration resolver to support this unwrapping and for wrapping filesystems to actually implement an interface to enable unwrapping.

About using MEF, or some kind of DI. Not all DIs support matching on two generic types. I don't want to depend on a single one, because that can be extremely annoying.

I understand. Choosing a particular DI framework can add baggage and scare away potential adopters. I’ve yet to think up a good way to handle this. I do like MEF myself but there’s no need for SharpFileSystem to adopt it for me to use it ;-).

One thought I have had on this, however, is how the registration is static. It would be easy if different codepaths initialized sharpfilesystem to end up with IEntityMovers and IEntityCopiers double-registered. Or, consider this case: in one place I want to use an IFileSystem with a special IEntityMover that does something fancy but I want to use the normal list of movers in a different area of code. The current signature of Move() as an extension method onto IFileSystem requires a static registration:

public static void Move(this IFileSystem sourceFileSystem, FileSystemPath sourcePath, IFileSystem destinationFileSystem, FileSystemPath destinationPath)

I think it would be much easier to support composition systems and various scenarios if the registration wasn’t static but instead attached to some sort of context which maintains non-static registrations. This would, unfortunately, make filesystem access more verbose, but that could be mitigated by including the root filesystem as part of the interface. Something like:

interface IFileSystemContext { IFileSystem FileSystem { get; } }
interface IEntityMoverContext : IFileSystemContext { TypeCombinationDictionary<IEntityMover> EntityMovers { get; } }
public static void Move(this IEntityMoverContext context, FileSystemPath sourcePath, IFileSystem destinationFileSystem, FileSystemPath destinationPath)
{
    IEntityMover mover;
    if (!context.EntityMovers.Registration.TryGetSupported(context.FileSystem.GetType(), destinationFileSystem.GetType(), out mover))
    …
}
// A base class could be provided by sharpfilesystem which implements all of the contexts
// for built-in registrations—namely IEntityMoverContext, IEntityCopierContext. Others
// adding their own custom manipulation registrations could subclass this as they desire.
class FileSystemContext
    : IEntityMoverContext
    , IEntityCopierContext
{
    …
}

If you like that idea and we can go that route, I would very much like to remove the existing static registrations completely. Having both systems at the same time would make it somewhat easy to accidentally use the static system when wanting to buy into the context-based one.

Thoughts? ;-)

[FrozenCow]

I’m actually more worried about atomic operations than performance myself.

What is your specific application?

Wrapping is an issue when trying to implement atomic operations using the registration+extension methods style. For ReadOnlyFileSystem, you never want the caller to be able to unwrap it. And, fair enough, you would never be able to perform atomic operations on a read only system. My idea for approaching the other filesystems is to have, e.g., MountedEntityMover support the pairs FileSystemMounter, IFileSystem and IFileSystem, FileSystemMounter, resolve the mountpoints, and then re-dispatch the Move() call on the underlying filesystems. Though maybe it would be better for the operation registration resolver to support this unwrapping and for wrapping filesystems to actually implement an interface to enable unwrapping.

That's exactly the intention. A IEntityMover lookup can be done from MountedEnityMover.

I think it would be much easier to support composition systems and various scenarios if the registration wasn’t static but instead attached to some sort of context which maintains non-static registrations. This would, unfortunately, make filesystem access more verbose, but that could be mitigated by including the root filesystem as part of the interface.

Yes, making the registration non-static would solve a lot of potential problems. However, I'm not sure yet that the proposed solution is the right one. I imagine that there isn't a single 'root' filesystem. Two physical flesystems can be completely separate, not combined with a mounted or merged filesystem.

If the context were to be passed to Move/Copy/Replace, why not pass the registration itself for that specific operation?

public static void Move(this TypeCombinationDictionary<IEntityMover> context, FileSystemPath sourcePath, IFileSystem destinationFileSystem, FileSystemPath destinationPath)

That way the system does stay flexible. What might also make the system less verbose is to wrap the context in a class:

public class Mover
{
    public TypeCombinationDictionary<IEntityMover> Context {get;set;}
    public Mover(TypeCombinationDictionary<IEntityMover> context) { this.Context = context; }
    public void Move(IFileSystem sourceFileSystem, FileSystemPath sourcePath, IFileSystem destinationFileSystem, FileSystemPath destinationPath);
}

That way any DI can inject TypeCombinationDictionary<IEntityMover> when a Mover is fetched from the DI. The move operation itself is simple after that. Same can be done for Copy or Replace and other operations can be created the same way as well.

That said, it would be nice if FileSystem could get a simple Move extension method. I'm a bit sad to see that go, but the alternative (no static objects) is better.

[binki]

I’m actually more worried about atomic operations than performance myself.

What is your specific application?

Please see my reply on #8.

If the context were to be passed to Move/Copy/Replace, why not pass the registration itself for that specific operation?

That way the system does stay flexible. What might also make the system less verbose is to wrap the context in a class:

Enabling concision and the existing extension pattern is the whole reason I am suggesting interfaces instead of classes here. There’s no need for IFileSystemContext, but if you have IMoversContext and ICopiersContext, you can compose these into a single class like so:

public class SharpFileSystemContext : IMoversContext, ICopiersContext
{
    public TypeCombinationDictionary<IEntityMover> EntityMovers { get; }
    public TypeCombinationDictionary<IEntityCopier> EntityCopiers { get; }
}

And key your extension methods granularly against each operation:

public static class FileSystemExtensions
{
    public static void Move(this IMoversContext, …) { … }
    public static void Copy(this ICopiersContext, …) { … }
}

Then when you write your own code and add your own operations, you can just add another interface and inherit from the existing context class and figure out how to make that work with your DI:

class MyCustomFileSystemContext : SharpFileSystemContext, IMyCustomOperationContext
{
    public TypeCombinationDictionary<IMyCustomOperation> CustomOperators { get; }
}
static class MyCustomExtensions
{
    public static void MyCustomOperation(this IMyCustomOperationContext context, …) { … }
}

Then your customized context gets all of the operations that SharpFileSystem provides as core (Copy, Move) for free and you only have one context to pass around everywhere if you want. Or, if you really wanted, you could implement IMoversContext and ICopiersContext in different classes and keep track of more state variables :-p.

That said, it would be nice if FileSystem could get a simple Move extension method. I'm a bit sad to see that go, but the alternative (no static objects) is better.

Cool! I do like how the static state enables concise calls, but I think removing them to enable DI is worth it ;-).

If you’re not opposed to the contexts as interfaces as I outlined in this post, I think I could easily enough PR that. Should I go ahead?