Hi James

There was a session on V8 at Codegarden, and they did show that IoC is indeed a part of the core of V8. I do not know to which extent it is used yet, but it is there.

I agree, that if something like an IFileSystem is exposed for third parties to implement, then the core should only use the interface, to ensure it is actually pluggable.

With regards to xml config vs code, I think there does need to be a way for people to compose a site out of packages, without having to write any C# code. So that would mean that replacing the FileSystem implementation should be possible from a config file.

I'm all for redesigning this in v8. There's no IFileSystem2 in v8, much of it has already been cleaned up. I urge you to clone the v8 code and see what is there currently and how things are setup.

Stephan has done a considerably amount of work in v8 over the past few years to clean things up and streamline things and I'm sure he has his own opinions of this and how things might work. I myself and not fully familiar with the IFileSystem changes already done in v8 so please have a look to see what is there now.

On the point of IOC, I think going with LightInject is a risky decision, Microsoft's DI package works with .NET Framework projects and is forward compatible.

I disagree - MS's DI framework offer the most basic DI functionality but we need more than just basic functionality which is already in use in many places in v8. LightInject is also forward compatible. I don't really see why this is 'risky' but we can argue that point on another thread if necessary.

Just for some background info in case it helps, I'd like to mention how IFileSystem is structured in Umbraco today : IFileSystem is the underlying file system that can be replaced but there are strongly typed file systems like MediaFileSystem that "extends" the interface by wrapping it. As you mentioned MediaFileSystem.GetMediaPath is not part of IFileSystem, but that was by design. Sure we could instead make an interface for a MediaFileSystem but in v6 (it dates way back to then) it was decided to make concrete IFileSystems wrap an underlying swap-able IFileSystem

So in the current file system config, you can replace the underlying IFileSystem for a given strongly typed file system.

The purpose is that a strongly typed file system might have specific functionality and that also allows for adding extension methods to the strongly typed file system while wrapping the basic IO IFileSystem implementation for the reading/writing of the actual files.

Comments:

I am all (as in, absolutely totally convinced) for structuring the codebase with clear contract interfaces, and then implementations. There are tons of places where we totally don't respect this, today. Just look for all the places where we do:

public void DoSomething(IContent content)
{
   var c = (Content) content;
   // ... and then use c everywhere
}

Now, well, there are tons of these, and we can only cleanup so fast. They are not a choice, there are legacy.

The IFileSystem thing (yes, IFileSystem2 is gone in v8) is definitively one that I don't like - although, as Shannon mentioned, one need to pay attention that (1) we may want a media "super filesystem" that adds some functionality for dealing with media and (2) there's the whole shadowing thing.

Totally happy to discuss here proposals for IFileSystem refactoring. Better discuss first, so we all agree on changes.

On configuration files vs. code: to me it's the same as pure DI vs service locator, obviously I like the "pure" approaches but we need to transition and be friendly. There has to be a way to get things done, quick (and maybe dirty).

The way it's solved for DI is... v8 is all DI, we try to inject everything, and then there's this Current service locator. Don't use it if you don't like it, but it's there.

For configuration files vs. code, methink we need a clean code API for configuration (what James really wants ;-) and then, we can totally write a configuration file-parsing thing on top of it for when/if it's needed.

TL;DR:

• Totally wanting to cleanup IFileSystem
• Other things to do before, community can help
• Discuss target architecture here beforehand
• Configuration API + file on top of it

Thoughts?

public void DoSomething(IContent content)
{
   var c = (Content) content;
   // ... and then use c everywhere
}

Mein Gott!! That should have come with a warning! :)

Glad to hear that you are on board.

@shannon There's a breaking change in MediaFileSystem somewhere in 7.6+ that alters the naming of replaced media files. That has broken my Azure provider in a way I cannot (without help from HQ) fix. This exemplifies, to me, why wrapping in this manner is unwise. (Though I do wish the entire naming convention for media was different and simply meant passing the uid and extension to the IFileSystem).

Regarding DI, I meant Microsoft.Extensions.DependencyInjection which, as I understand it can be used within classic applications and is fully featured (I've yet to find a limitation in .NET Core projects but I'm no expert so I could be talking crap).

@stephan Regarding configuration. Yes, I would love something like IUmbracoBuilder that handled (Add\Remove\Replace)(Service\PropertyValueConverter) etc. It's a common pattern in the new fangled way of doing things, I'm using that approach in the ImageSharp.Web middleware and it's just really neat for the consumer.

Another benefit of course is that if you have a great configuration API you can dramatically reduce the amount of work you need to do at startup (No more looking through dll's to find plugins for starters)

Adding a configuration file binding API on top of that should be much easier, Off the top of my head, imagine something in your API like:

// For standard provider setup
IUmbracoBuilder.ReplaceFileSystem<TFileSystemOld, TFileSystemNew> ()
    where TFileSystemOld : IFileSystem
    where TFileSystemNew : IFileSystem

and

// For something that requires additional setup on init.
IUmbracoBuilder.ReplaceFileSystem<TFileSystem>(Function<IFileSystem> setupFunc) 
    where TFileSystem : IFileSystem

where I could then write a plugin extension method that did something like this:

IUmbracoBuilder UseAzureFileSystem(this IUmbracoBuilder builder,  Action<AzureFileSystemOptions> setupAction)
{
    // This method in your API would register the options as a singleton. 
    // It could theoretically be used for all options.
    builder.Services.Configure(setupAction);

    // We call your registration method now.
    builder.ReplaceFileSystem<PhysicalFileSystem, AzureFileSystem>();
    return builder; 
}

Calling this would then involve your addition configuration API that could parse XML or Json. This would be super powerful then.

builder.UseAzureFileSystem(options => this.Configuration.Bind("AzureFileSystemOptions", options));

You could use the same approach for adding IPropertyValueConverter implementation and all sorts.

Let me know how I can help.

There's a breaking change in MediaFileSystem somewhere in 7.6+ that alters the naming of replaced media files. That has broken my Azure provider in a way I cannot (without help from HQ) fix. This exemplifies, to me, why wrapping in this manner is unwise. (Though I do wish the entire naming convention for media was different and simply meant passing the uid and extension to the IFileSystem

I was only mentioning how it currently works with the wrapping, not how it probably should work ;) Off the top of my head I don't recall the name change thing but happy to help if i can. The naming convention for media must be changed in v8. The integer thing doesn't work well since it's just a simple INT counter which also leads to issues in LB env. There's a flag already in v7/v8 to change this naming convention so that media is stored in GUID based folders that are based on the property ID, so you would always know what the files in a folder belong to. We'll need to flip this switch for v8 and this will also allow us to proxy media better (i.e. we'll store in App_Data and proxy via /media).

Microsoft.Extensions.DependencyInjection which, as I understand it can be used within classic applications and is fully featured

It is a basic DI implementation and works well but if you require property injection or custom injection than it doesn't suffice (and we are using some of these techniques). LightInject is forward compatible and integrates with the DI framework for .NET core, it was one of the first to support .NET Core.

And yup i'm all for the code approach you've listed above and this is how v8 is structure already (more or less). If config files are needed for anything, these should be added/supported later.

Oh I forgot to ask, What does the ShadowFileSystem do? I had a look at the code but couldn't figure it out.

Chaps,

I've created an issue regarding the DI approach taken in the codebase. It's troubling me and I don't want you to have problems down the line.

http://issues.umbraco.org/issue/U4-11427

is working on a reply...