From 6b5057b8708eb82f2af278225e3e0075dbb7f2b4 Mon Sep 17 00:00:00 2001
From: Stephan <stephane@umbraco.dk>
Date: Tue, 8 Sep 2015 17:48:26 +0200
Subject: [PATCH] U4-7048 - more filesystems cleanup

---
 src/Umbraco.Core/IO/IOHelper.cs               |  27 +++-
 src/Umbraco.Core/IO/PhysicalFileSystem.cs     |  19 +--
 .../Repositories/FileRepository.cs            |   4 -
 .../Interfaces/IPartialViewRepository.cs      |   1 +
 .../Repositories/PartialViewRepository.cs     |  45 ++++--
 .../Repositories/ScriptRepository.cs          |  31 +---
 .../Repositories/StylesheetRepository.cs      |  14 +-
 .../Repositories/TemplateRepository.cs        |  72 +++++++--
 src/Umbraco.Core/Services/FileService.cs      |  16 +-
 src/Umbraco.Core/Services/IFileService.cs     |   1 +
 .../Repositories/ScriptRepositoryTest.cs      |  32 ++++
 .../umbraco/settings/views/EditView.aspx      |   1 +
 .../umbraco/settings/views/EditView.aspx.cs   |  25 ++-
 .../settings/views/EditView.aspx.designer.cs  |   9 ++
 .../umbraco_client/Editors/EditScript.js      |  30 +++-
 .../umbraco_client/Editors/EditView.js        |  22 +--
 .../Trees/PartialViewMacrosTree.cs            |   2 +-
 src/Umbraco.Web/Trees/PartialViewsTree.cs     |   2 +-
 .../WebServices/SaveFileController.cs         | 145 +++++++-----------
 19 files changed, 287 insertions(+), 211 deletions(-)

diff --git a/src/Umbraco.Core/IO/IOHelper.cs b/src/Umbraco.Core/IO/IOHelper.cs
index 8378081394..286acf0285 100644
--- a/src/Umbraco.Core/IO/IOHelper.cs
+++ b/src/Umbraco.Core/IO/IOHelper.cs
@@ -177,12 +177,23 @@ namespace Umbraco.Core.IO
         /// <returns>A value indicating whether the filepath is valid.</returns>
         internal static bool VerifyEditPath(string filePath, IEnumerable<string> validDirs)
         {
+            // this is called from ScriptRepository, PartialViewRepository, etc.
+            // filePath is the fullPath (rooted, filesystem path, can be trusted)
+            // validDirs are virtual paths (eg ~/Views)
+            //
+            // except that for templates, filePath actually is a virtual path
+
+            //TODO
+            // what's below is dirty, there are too many ways to get the root dir, etc.
+            // not going to fix everything today
+
             var mappedRoot = MapPath(SystemDirectories.Root);
             if (filePath.StartsWith(mappedRoot) == false)
                 filePath = MapPath(filePath);
 
-            // don't trust what we get, it may contain relative segments
-            filePath = Path.GetFullPath(filePath);
+            // yes we can (see above)
+            //// don't trust what we get, it may contain relative segments
+            //filePath = Path.GetFullPath(filePath);
 
             foreach (var dir in validDirs)
             {
@@ -190,7 +201,7 @@ namespace Umbraco.Core.IO
                 if (validDir.StartsWith(mappedRoot) == false)
                     validDir = MapPath(validDir);
 
-                if (filePath.StartsWith(validDir))
+                if (PathStartsWith(filePath, validDir, Path.DirectorySeparatorChar))
                     return true;
             }
 
@@ -237,6 +248,16 @@ namespace Umbraco.Core.IO
             return true;
         }
 
+        public static bool PathStartsWith(string path, string root, char separator)
+        {
+            // either it is identical to root,
+            // or it is root + separator + anything
+
+            if (path.StartsWith(root, StringComparison.OrdinalIgnoreCase) == false) return false;
+            if (path.Length == root.Length) return true;
+            if (path.Length < root.Length) return false;
+            return path[root.Length] == separator;
+        }
 
         /// <summary>
         /// Returns the path to the root of the application, by getting the path to where the assembly where this
diff --git a/src/Umbraco.Core/IO/PhysicalFileSystem.cs b/src/Umbraco.Core/IO/PhysicalFileSystem.cs
index 79b453e9cb..b801f05b8f 100644
--- a/src/Umbraco.Core/IO/PhysicalFileSystem.cs
+++ b/src/Umbraco.Core/IO/PhysicalFileSystem.cs
@@ -186,14 +186,14 @@ namespace Umbraco.Core.IO
         {
             // test url
             var path = EnsureUrlSeparatorChar(fullPathOrUrl);
-            if (PathStartsWith(path, _rootUrl, '/'))
+            if (IOHelper.PathStartsWith(path, _rootUrl, '/'))
                 return path.Substring(_rootUrl.Length)  
                     .Replace('/', Path.DirectorySeparatorChar)
                     .TrimStart(Path.DirectorySeparatorChar);
 
             // test path
             path = EnsureDirectorySeparatorChar(fullPathOrUrl);
-            if (PathStartsWith(path, _rootPath, Path.DirectorySeparatorChar))
+            if (IOHelper.PathStartsWith(path, _rootPath, Path.DirectorySeparatorChar))
                 return path.Substring(_rootPath.Length)
                     .TrimStart(Path.DirectorySeparatorChar);
 
@@ -231,7 +231,7 @@ namespace Umbraco.Core.IO
                 path = GetRelativePath(path);
 
             // if already a full path, return
-            if (PathStartsWith(path, _rootPath, Path.DirectorySeparatorChar))
+            if (IOHelper.PathStartsWith(path, _rootPath, Path.DirectorySeparatorChar))
                 return path;
 
             // else combine and sanitize, ie GetFullPath will take care of any relative
@@ -243,23 +243,12 @@ namespace Umbraco.Core.IO
             // at that point, path is within legal parts of the filesystem, ie we have
             // permissions to reach that path, but it may nevertheless be outside of
             // our root path, due to relative segments, so better check
-            if (PathStartsWith(fpath, _rootPath, Path.DirectorySeparatorChar))
+            if (IOHelper.PathStartsWith(fpath, _rootPath, Path.DirectorySeparatorChar))
                 return fpath;
 
             throw new FileSecurityException("File '" + opath + "' is outside this filesystem's root.");
         }
 
-        private static bool PathStartsWith(string path, string root, char separator)
-        {
-            // either it is identical to root,
-            // or it is root + separator + anything
-
-            if (path.StartsWith(root, StringComparison.OrdinalIgnoreCase) == false) return false;
-            if (path.Length == root.Length) return true;
-            if (path.Length < root.Length) return false;
-            return path[root.Length] == separator;
-        }
-
         public string GetUrl(string path)
         {
             path = EnsureUrlSeparatorChar(path).Trim('/');
diff --git a/src/Umbraco.Core/Persistence/Repositories/FileRepository.cs b/src/Umbraco.Core/Persistence/Repositories/FileRepository.cs
index 04ac5b6f62..e1e4c3105a 100644
--- a/src/Umbraco.Core/Persistence/Repositories/FileRepository.cs
+++ b/src/Umbraco.Core/Persistence/Repositories/FileRepository.cs
@@ -147,10 +147,6 @@ namespace Umbraco.Core.Persistence.Repositories
 
         protected virtual void PersistUpdatedItem(TEntity entity)
         {
-            //TODO: A big problem here is if the entities 'Path' changes, if that is the case then 
-            // we'd need to rename the underlying file, BUT how would we do this since we aren't storing an 
-            // original path property.
-
             using (var stream = GetContentStream(entity.Content))
             {
                 FileSystem.AddFile(entity.Path, stream, true);
diff --git a/src/Umbraco.Core/Persistence/Repositories/Interfaces/IPartialViewRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Interfaces/IPartialViewRepository.cs
index 8ba28eeaee..3fbcdd3283 100644
--- a/src/Umbraco.Core/Persistence/Repositories/Interfaces/IPartialViewRepository.cs
+++ b/src/Umbraco.Core/Persistence/Repositories/Interfaces/IPartialViewRepository.cs
@@ -6,5 +6,6 @@ namespace Umbraco.Core.Persistence.Repositories
     {
         void AddFolder(string folderPath);
         void DeleteFolder(string folderPath);
+        bool ValidatePartialView(IPartialView partialView);
     }
 }
\ No newline at end of file
diff --git a/src/Umbraco.Core/Persistence/Repositories/PartialViewRepository.cs b/src/Umbraco.Core/Persistence/Repositories/PartialViewRepository.cs
index 254d2cbc0c..e9352a945f 100644
--- a/src/Umbraco.Core/Persistence/Repositories/PartialViewRepository.cs
+++ b/src/Umbraco.Core/Persistence/Repositories/PartialViewRepository.cs
@@ -24,20 +24,24 @@ namespace Umbraco.Core.Persistence.Repositories
 
         public override IPartialView Get(string id)
         {
-            if (FileSystem.FileExists(id) == false)
-            {
-                return null;
-            }
-
+            // get the relative path within the filesystem
+            // (though... id should be relative already)
             var path = FileSystem.GetRelativePath(id);
+
+            if (FileSystem.FileExists(path) == false)
+                return null;
+
+            // content will be lazy-loaded when required
             var created = FileSystem.GetCreated(path).UtcDateTime;
             var updated = FileSystem.GetLastModified(path).UtcDateTime;
+            //var content = GetFileContent(path);
 
-            var script = new PartialView(path, file => GetFileContent(file.Path))
+            var view = new PartialView(path, file => GetFileContent(file.OriginalPath))
             {
                 //id can be the hash
                 Id = path.GetHashCode(),
                 Key = path.EncodeAsGuid(),
+                //Content = content,
                 CreateDate = created,
                 UpdateDate = updated,
                 VirtualPath = FileSystem.GetUrl(id),
@@ -46,9 +50,9 @@ namespace Umbraco.Core.Persistence.Repositories
 
             //on initial construction we don't want to have dirty properties tracked
             // http://issues.umbraco.org/issue/U4-1946
-            script.ResetDirtyProperties(false);
+            view.ResetDirtyProperties(false);
 
-            return script;
+            return view;
         }
 
         public override void AddOrUpdate(IPartialView entity)
@@ -61,7 +65,7 @@ namespace Umbraco.Core.Persistence.Repositories
 
             // ensure that from now on, content is lazy-loaded
             if (partialView != null && partialView.GetFileContent == null)
-                partialView.GetFileContent = file => GetFileContent(file.Path);
+                partialView.GetFileContent = file => GetFileContent(file.OriginalPath);
         }
 
         public override IEnumerable<IPartialView> GetAll(params string[] ids)
@@ -86,6 +90,29 @@ namespace Umbraco.Core.Persistence.Repositories
             }
         }
 
+        private static readonly List<string> ValidExtensions = new List<string> { "cshtml" };
+
+        public virtual bool ValidatePartialView(IPartialView partialView)
+        {
+            // get full path
+            string fullPath;
+            try
+            {
+                // may throw for security reasons
+                fullPath = FileSystem.GetFullPath(partialView.Path);
+            }
+            catch
+            {
+                return false;
+            }
+
+            // validate path & extension
+            var validDir = SystemDirectories.MvcViews;
+            var isValidPath = IOHelper.VerifyEditPath(fullPath, validDir);
+            var isValidExtension = IOHelper.VerifyFileExtension(fullPath, ValidExtensions);
+            return isValidPath && isValidExtension;
+        }
+
         /// <summary>
         /// Gets a stream that is used to write to the file
         /// </summary>
diff --git a/src/Umbraco.Core/Persistence/Repositories/ScriptRepository.cs b/src/Umbraco.Core/Persistence/Repositories/ScriptRepository.cs
index e288efdc85..13b2877c50 100644
--- a/src/Umbraco.Core/Persistence/Repositories/ScriptRepository.cs
+++ b/src/Umbraco.Core/Persistence/Repositories/ScriptRepository.cs
@@ -38,7 +38,7 @@ namespace Umbraco.Core.Persistence.Repositories
             var updated = FileSystem.GetLastModified(path).UtcDateTime;
             //var content = GetFileContent(path);
 
-            var script = new Script(path, file => GetFileContent(file.Path))
+            var script = new Script(path, file => GetFileContent(file.OriginalPath))
             {
                 //id can be the hash
                 Id = path.GetHashCode(),
@@ -62,7 +62,7 @@ namespace Umbraco.Core.Persistence.Repositories
 
             // ensure that from now on, content is lazy-loaded
             if (entity.GetFileContent == null)
-                entity.GetFileContent = file => GetFileContent(file.Path);
+                entity.GetFileContent = file => GetFileContent(file.OriginalPath);
         }
 
         public override IEnumerable<Script> GetAll(params string[] ids)
@@ -89,23 +89,7 @@ namespace Umbraco.Core.Persistence.Repositories
 
         public bool ValidateScript(Script script)
         {
-            //NOTE Since a script file can be both JS, Razor Views, Razor Macros and Xslt
-            //it might be an idea to create validations for all 3 and divide the validation 
-            //into 4 private methods.
-            //See codeEditorSave.asmx.cs for reference.
-
-            var exts = _contentConfig.ScriptFileTypes.ToList();
-            /*if (UmbracoSettings.DefaultRenderingEngine == RenderingEngine.Mvc)
-            {
-                exts.Add("cshtml");
-                exts.Add("vbhtml");
-            }*/
-
-            var dirs = SystemDirectories.Scripts;
-            /*if (UmbracoSettings.DefaultRenderingEngine == RenderingEngine.Mvc)
-                dirs += "," + SystemDirectories.MvcViews;*/
-
-            //Validate file
+            // get full path
             string fullPath;
             try
             {
@@ -116,11 +100,12 @@ namespace Umbraco.Core.Persistence.Repositories
             {
                 return false;
             }
-            var isValidPath = IOHelper.VerifyEditPath(fullPath, dirs.Split(','));
-
-            //Validate extension
-            var isValidExtension = IOHelper.VerifyFileExtension(script.Path, exts);
 
+            // validate path & extension
+            var validDir = SystemDirectories.Scripts;
+            var isValidPath = IOHelper.VerifyEditPath(fullPath, validDir);
+            var validExts = _contentConfig.ScriptFileTypes.ToList();
+            var isValidExtension = IOHelper.VerifyFileExtension(script.Path, validExts);
             return isValidPath && isValidExtension;
         }
 
diff --git a/src/Umbraco.Core/Persistence/Repositories/StylesheetRepository.cs b/src/Umbraco.Core/Persistence/Repositories/StylesheetRepository.cs
index b30564bbc9..6f9138de9a 100644
--- a/src/Umbraco.Core/Persistence/Repositories/StylesheetRepository.cs
+++ b/src/Umbraco.Core/Persistence/Repositories/StylesheetRepository.cs
@@ -38,7 +38,7 @@ namespace Umbraco.Core.Persistence.Repositories
             var updated = FileSystem.GetLastModified(path).UtcDateTime;
             //var content = GetFileContent(path);
 
-            var stylesheet = new Stylesheet(path, file => GetFileContent(file.Path))
+            var stylesheet = new Stylesheet(path, file => GetFileContent(file.OriginalPath))
             {
                 //Content = content,
                 Key = path.EncodeAsGuid(),
@@ -62,7 +62,7 @@ namespace Umbraco.Core.Persistence.Repositories
 
             // ensure that from now on, content is lazy-loaded
             if (entity.GetFileContent == null)
-                entity.GetFileContent = file => GetFileContent(file.Path);
+                entity.GetFileContent = file => GetFileContent(file.OriginalPath);
         }
 
         public override IEnumerable<Stylesheet> GetAll(params string[] ids)
@@ -106,9 +106,7 @@ namespace Umbraco.Core.Persistence.Repositories
 
         public bool ValidateStylesheet(Stylesheet stylesheet)
         {
-            var dirs = SystemDirectories.Css;
-
-            //Validate file
+            // get full path
             string fullPath;
             try
             {
@@ -119,11 +117,11 @@ namespace Umbraco.Core.Persistence.Repositories
             {
                 return false;
             }
-            var isValidPath = IOHelper.VerifyEditPath(fullPath, dirs.Split(','));
 
-            //Validate extension
+            // validate path and extension
+            var validDir = SystemDirectories.Css;
+            var isValidPath = IOHelper.VerifyEditPath(fullPath, validDir);
             var isValidExtension = IOHelper.VerifyFileExtension(stylesheet.Path, ValidExtensions);
-
             return isValidPath && isValidExtension;
         }
 
diff --git a/src/Umbraco.Core/Persistence/Repositories/TemplateRepository.cs b/src/Umbraco.Core/Persistence/Repositories/TemplateRepository.cs
index 8ebcad777a..0a0dbe37dd 100644
--- a/src/Umbraco.Core/Persistence/Repositories/TemplateRepository.cs
+++ b/src/Umbraco.Core/Persistence/Repositories/TemplateRepository.cs
@@ -402,6 +402,36 @@ namespace Umbraco.Core.Persistence.Repositories
 
         private string GetFileContent(ITemplate template, bool init)
         {
+            var path = template.OriginalPath;
+            if (string.IsNullOrWhiteSpace(path))
+            {
+                // we need to discover the path
+                path = string.Concat(template.Alias, ".cshtml");
+                if (_viewsFileSystem.FileExists(path))
+                    return GetFileContent(template, _viewsFileSystem, path, init);
+                path = string.Concat(template.Alias, ".vbhtml");
+                if (_viewsFileSystem.FileExists(path))
+                    return GetFileContent(template, _viewsFileSystem, path, init);
+                path = string.Concat(template.Alias, ".master");
+                if (_masterpagesFileSystem.FileExists(path))
+                    return GetFileContent(template, _masterpagesFileSystem, path, init);
+            }
+            else
+            {
+                // we know the path already
+                var ext = Path.GetExtension(path);
+                switch (ext)
+                {
+                    case ".cshtml":
+                    case ".vbhtml":
+                        return GetFileContent(template, _viewsFileSystem, path, init);
+                    case ".master":
+                        return GetFileContent(template, _masterpagesFileSystem, path, init);
+                    default:
+                        return string.Empty;
+                }
+            }
+
             var fsname = string.Concat(template.Alias, ".cshtml");
             if (_viewsFileSystem.FileExists(fsname))
                 return GetFileContent(template, _viewsFileSystem, fsname, init);
@@ -422,6 +452,15 @@ namespace Umbraco.Core.Persistence.Repositories
             {
                 template.UpdateDate = fs.GetLastModified(filename).UtcDateTime;
             }
+
+            // TODO
+            //  see if this could enable us to update UpdateDate without messing with change tracking
+            //  and then we'd want to do it for scripts, stylesheets and partial views too (ie files)
+            //var xtemplate = template as Template;
+            //xtemplate.DisableChangeTracking();
+            //template.UpdateDate = fs.GetLastModified(filename).UtcDateTime;
+            //xtemplate.EnableChangeTracking();
+
             template.VirtualPath = fs.GetUrl(filename);
 
             return init ? null : GetFileContent(fs, filename);
@@ -682,27 +721,32 @@ namespace Umbraco.Core.Persistence.Repositories
         /// <returns>True if Script is valid, otherwise false</returns>
         public bool ValidateTemplate(ITemplate template)
         {
-            var exts = new List<string>();
+            // get path
+            // TODO
+            //  templates should have a real Path somehow - but anyways
+            //  are we using Path for something else?!
+            var path = template.VirtualPath;
+
+            // get valid paths
+            var validDirs = _templateConfig.DefaultRenderingEngine == RenderingEngine.Mvc
+                ? new[] { SystemDirectories.Masterpages, SystemDirectories.MvcViews }
+                : new[] { SystemDirectories.Masterpages };
+
+            // get valid extensions
+            var validExts = new List<string>();
             if (_templateConfig.DefaultRenderingEngine == RenderingEngine.Mvc)
             {
-                exts.Add("cshtml");
-                exts.Add("vbhtml");
+                validExts.Add("cshtml");
+                validExts.Add("vbhtml");
             }
             else
             {
-                exts.Add(_templateConfig.UseAspNetMasterPages ? "master" : "aspx");
+                validExts.Add(_templateConfig.UseAspNetMasterPages ? "master" : "aspx");
             }
 
-            var dirs = SystemDirectories.Masterpages;
-            if (_templateConfig.DefaultRenderingEngine == RenderingEngine.Mvc)
-                dirs += "," + SystemDirectories.MvcViews;
-
-            //Validate file
-            var validFile = IOHelper.VerifyEditPath(template.VirtualPath, dirs.Split(','));
-
-            //Validate extension
-            var validExtension = IOHelper.VerifyFileExtension(template.VirtualPath, exts);
-
+            // validate path and extension
+            var validFile = IOHelper.VerifyEditPath(path, validDirs);
+            var validExtension = IOHelper.VerifyFileExtension(path, validExts);
             return validFile && validExtension;
         }
 
diff --git a/src/Umbraco.Core/Services/FileService.cs b/src/Umbraco.Core/Services/FileService.cs
index 1dcc7050a8..aa14b1afa0 100644
--- a/src/Umbraco.Core/Services/FileService.cs
+++ b/src/Umbraco.Core/Services/FileService.cs
@@ -687,10 +687,20 @@ namespace Umbraco.Core.Services
 
         public bool ValidatePartialView(PartialView partialView)
         {
-            var validatePath = IOHelper.ValidateEditPath(partialView.Path, new[] { SystemDirectories.MvcViews + "/Partials/", SystemDirectories.MvcViews + "/MacroPartials/" });
-            var verifyFileExtension = IOHelper.VerifyFileExtension(partialView.Path, new List<string> { "cshtml" });
+            var uow = _dataUowProvider.GetUnitOfWork();
+            using (var repository = _repositoryFactory.CreatePartialViewRepository(uow))
+            {
+                return repository.ValidatePartialView(partialView);
+            }
+        }
 
-            return validatePath && verifyFileExtension;
+        public bool ValidatePartialViewMacro(PartialView partialView)
+        {
+            var uow = _dataUowProvider.GetUnitOfWork();
+            using (var repository = _repositoryFactory.CreatePartialViewMacroRepository(uow))
+            {
+                return repository.ValidatePartialView(partialView);
+            }
         }
 
         internal string StripPartialViewHeader(string contents)
diff --git a/src/Umbraco.Core/Services/IFileService.cs b/src/Umbraco.Core/Services/IFileService.cs
index e1c0f1428c..e56abc71b5 100644
--- a/src/Umbraco.Core/Services/IFileService.cs
+++ b/src/Umbraco.Core/Services/IFileService.cs
@@ -21,6 +21,7 @@ namespace Umbraco.Core.Services
         Attempt<IPartialView> SavePartialView(IPartialView partialView, int userId = 0);
         Attempt<IPartialView> SavePartialViewMacro(IPartialView partialView, int userId = 0);
         bool ValidatePartialView(PartialView partialView);
+        bool ValidatePartialViewMacro(PartialView partialView);
 
         /// <summary>
         /// Gets a list of all <see cref="Stylesheet"/> objects
diff --git a/src/Umbraco.Tests/Persistence/Repositories/ScriptRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/ScriptRepositoryTest.cs
index c907a8a4da..b98109ca8a 100644
--- a/src/Umbraco.Tests/Persistence/Repositories/ScriptRepositoryTest.cs
+++ b/src/Umbraco.Tests/Persistence/Repositories/ScriptRepositoryTest.cs
@@ -188,6 +188,38 @@ namespace Umbraco.Tests.Persistence.Repositories
             Assert.That(exists, Is.True);
         }
 
+        [Test]
+        public void Can_Perform_Move_On_ScriptRepository()
+        {
+            const string content = "/// <reference name=\"MicrosoftAjax.js\"/>";
+
+            // Arrange
+            var provider = new FileUnitOfWorkProvider();
+            var unitOfWork = provider.GetUnitOfWork();
+            var repository = new ScriptRepository(unitOfWork, _fileSystem, Mock.Of<IContentSection>());
+
+            var script = new Script("test-move-script.js") { Content = content };
+            repository.AddOrUpdate(script);
+            unitOfWork.Commit();
+
+            // Act
+            script = repository.Get("test-move-script.js");
+            script.Path = "moved/test-move-script.js";
+            repository.AddOrUpdate(script);
+            unitOfWork.Commit();
+
+            var existsOld = repository.Exists("test-move-script.js");
+            var existsNew = repository.Exists("moved/test-move-script.js");
+
+            script = repository.Get("moved/test-move-script.js");
+
+            // Assert
+            Assert.IsNotNull(script);
+            Assert.IsFalse(existsOld);
+            Assert.IsTrue(existsNew);
+            Assert.AreEqual(content, script.Content);
+        }
+
         [TearDown]
         public override void TearDown()
         {
diff --git a/src/Umbraco.Web.UI/umbraco/settings/views/EditView.aspx b/src/Umbraco.Web.UI/umbraco/settings/views/EditView.aspx
index ba464262d0..ca68fc4b2c 100644
--- a/src/Umbraco.Web.UI/umbraco/settings/views/EditView.aspx
+++ b/src/Umbraco.Web.UI/umbraco/settings/views/EditView.aspx
@@ -66,6 +66,7 @@
         <cc1:Pane ID="Pane7" runat="server" >
             
             <cc1:PropertyPanel ID="pp_name" runat="server">
+                <asp:Literal runat="server" ID="PathPrefix"></asp:Literal>
                 <asp:TextBox ID="NameTxt" runat="server"></asp:TextBox>
             </cc1:PropertyPanel>
             
diff --git a/src/Umbraco.Web.UI/umbraco/settings/views/EditView.aspx.cs b/src/Umbraco.Web.UI/umbraco/settings/views/EditView.aspx.cs
index e4d0692385..a72e03b57e 100644
--- a/src/Umbraco.Web.UI/umbraco/settings/views/EditView.aspx.cs
+++ b/src/Umbraco.Web.UI/umbraco/settings/views/EditView.aspx.cs
@@ -114,28 +114,23 @@ namespace Umbraco.Web.UI.Umbraco.Settings.Views
 					NameTxt.Text = _template.GetRawText();
 					AliasTxt.Text = _template.Alias;
 					editorSource.Text = _template.Design;
-
+				    PathPrefix.Visible = false;
 				}
 				else
 				{
 					//configure editor for editing a file....
 
 					NameTxt.Text = OriginalFileName;
-					var file = IOHelper.MapPath(SystemDirectories.MvcViews.EnsureEndsWith('/') + OriginalFileName);
+				    var svce = ApplicationContext.Current.Services.FileService;
+                    var file = EditorType == ViewEditorType.PartialView
+				        ? svce.GetPartialView(OriginalFileName)
+                        : svce.GetPartialViewMacro(OriginalFileName);
+				    editorSource.Text = file.Content;
 
-                    // validate file path
-                    if (file.StartsWith(IOHelper.MapPath(SystemDirectories.MvcViews.EnsureEndsWith('/')))) {
-
-					using (var sr = File.OpenText(file))
-					{
-						var s = sr.ReadToEnd();
-						editorSource.Text = s;
-					}
-                    } else
-                    {
-                        throw new ArgumentException("Couldn't open file - illegal path");
-                    }
-				
+                    const string prefixFormat = "<span style=\"display: inline-block; height: 20px; line-height: 20px; margin-bottom: 0px; padding: 4px 6px;\">{0}</span>";
+                    PathPrefix.Text = string.Format(prefixFormat, EditorType == ViewEditorType.PartialView
+				        ? "Partials/"
+				        : "MacroPartials/");
 				}							
 			}
             
diff --git a/src/Umbraco.Web.UI/umbraco/settings/views/EditView.aspx.designer.cs b/src/Umbraco.Web.UI/umbraco/settings/views/EditView.aspx.designer.cs
index 4f523639a3..dd81da023a 100644
--- a/src/Umbraco.Web.UI/umbraco/settings/views/EditView.aspx.designer.cs
+++ b/src/Umbraco.Web.UI/umbraco/settings/views/EditView.aspx.designer.cs
@@ -75,6 +75,15 @@ namespace Umbraco.Web.UI.Umbraco.Settings.Views {
         /// </remarks>
         protected global::umbraco.uicontrols.PropertyPanel pp_name;
         
+        /// <summary>
+        /// PathPrefix control.
+        /// </summary>
+        /// <remarks>
+        /// Auto-generated field.
+        /// To modify move field declaration from designer file to code-behind file.
+        /// </remarks>
+        protected global::System.Web.UI.WebControls.Literal PathPrefix;
+        
         /// <summary>
         /// NameTxt control.
         /// </summary>
diff --git a/src/Umbraco.Web.UI/umbraco_client/Editors/EditScript.js b/src/Umbraco.Web.UI/umbraco_client/Editors/EditScript.js
index a286eed582..022f421ec6 100644
--- a/src/Umbraco.Web.UI/umbraco_client/Editors/EditScript.js
+++ b/src/Umbraco.Web.UI/umbraco_client/Editors/EditScript.js
@@ -76,19 +76,37 @@
                 UmbEditor.SetCode(args.contents);
             }
 
-            top.UmbSpeechBubble.ShowMessage("save", header, msg);
             UmbClientMgr.mainTree().setActiveTreeType("scripts");
             if (pathChanged) {
-                UmbClientMgr.mainTree().moveNode(this._opts.originalFileName, path);
-                this._opts.treeSyncPath = args.path;
-                this._opts.lttPathElement.prop("href", args.url).html(args.url);
+                // no! file is used in url so we need to redirect
+                //UmbClientMgr.mainTree().moveNode(this._opts.originalFileName, path);
+                //this._opts.treeSyncPath = args.path;
+                //this._opts.lttPathElement.prop("href", args.url).html(args.url);
+
+                var qs = window.location.search;
+                if (qs.startsWith("?")) qs = qs.substring("?".length);
+                var qp1 = qs.split("&");
+                var qp2 = [];
+                for (var i = 0; i < qp1.length; i++)
+                    if (!qp1[i].startsWith("file="))
+                        qp2.push(qp1[i]);
+
+                var location = window.location.pathname + "?" + qp2.join("&") + "&file=" + args.name;
+                UmbClientMgr.contentFrame(location);
+
+                // need to do it after we navigate otherwise the navigation waits until the message timeout is done
+                top.UmbSpeechBubble.ShowMessage("save", header, msg);
             }
             else {
+                top.UmbSpeechBubble.ShowMessage("save", header, msg);
+                this._opts.lttPathElement.prop("href", args.url).html(args.url);
+                this._opts.originalFileName = args.name;
+                this._opts.treeSyncPath = args.path;
                 UmbClientMgr.mainTree().syncTree(path, true);
             }
 
-            this._opts.lttPathElement.prop("href", args.url).html(args.url);
-            this._opts.originalFileName = args.name;
+            //this._opts.lttPathElement.prop("href", args.url).html(args.url);
+            //this._opts.originalFileName = args.name;
         },
 
         submitFailure: function(err, header) {
diff --git a/src/Umbraco.Web.UI/umbraco_client/Editors/EditView.js b/src/Umbraco.Web.UI/umbraco_client/Editors/EditView.js
index 554d0bafc8..28398d081a 100644
--- a/src/Umbraco.Web.UI/umbraco_client/Editors/EditView.js
+++ b/src/Umbraco.Web.UI/umbraco_client/Editors/EditView.js
@@ -8,24 +8,6 @@
         //private methods/variables
         _opts: null,
 
-        _updateNewFileProperties: function(filePath) {
-            /// <summary>Updates the current treeSyncPath and original file name to have the new file name</summary>
-            
-            //update the originalFileName prop
-            this._opts.originalFileName = filePath;
-
-            //re-create the new path
-            var subPath = this._opts.treeSyncPath.split(",");
-            //remove the last element
-            subPath.pop();
-            //add the new element
-            var parts = filePath.split("/");
-            //remove the first bit which will either be "Partials" or "MacroPartials"
-            parts.shift();
-            subPath.push(parts.join("/"));
-            this._opts.treeSyncPath = subPath.join();
-        },
-
         // Constructor
         constructor: function (opts) {
             // Merge options with default
@@ -207,8 +189,8 @@
                     
                     top.UmbSpeechBubble.ShowMessage('save', header, msg);
 
-                    //then we need to update our current tree sync path to represent the new one
-                    this._updateNewFileProperties(newFilePath);
+                    this._opts.originalFileName = args.name;
+                    this._opts.treeSyncPath = args.path;
 
                     UmbClientMgr.mainTree().syncTree(path, true, null, newFilePath.split("/")[1]);
                 }                
diff --git a/src/Umbraco.Web/Trees/PartialViewMacrosTree.cs b/src/Umbraco.Web/Trees/PartialViewMacrosTree.cs
index f5a9df97c2..70b8407e11 100644
--- a/src/Umbraco.Web/Trees/PartialViewMacrosTree.cs
+++ b/src/Umbraco.Web/Trees/PartialViewMacrosTree.cs
@@ -28,7 +28,7 @@ namespace Umbraco.Web.Trees
 			javascript.Append(
                 @"
 		                 function openMacroPartialView(id) {
-		                    UmbClientMgr.contentFrame('Settings/Views/EditView.aspx?treeType=partialViewMacros&file=MacroPartials%2f' + id);
+		                    UmbClientMgr.contentFrame('Settings/Views/EditView.aspx?treeType=partialViewMacros&file=' + id);
 					    }
 		                ");
 
diff --git a/src/Umbraco.Web/Trees/PartialViewsTree.cs b/src/Umbraco.Web/Trees/PartialViewsTree.cs
index 08bbd5aadd..5208dadf6f 100644
--- a/src/Umbraco.Web/Trees/PartialViewsTree.cs
+++ b/src/Umbraco.Web/Trees/PartialViewsTree.cs
@@ -30,7 +30,7 @@ namespace Umbraco.Web.Trees
 			javascript.Append(
                 @"
 		                 function openPartialView(id) {
-		                    UmbClientMgr.contentFrame('Settings/Views/EditView.aspx?treeType=partialViews&file=Partials%2f' + id);
+		                    UmbClientMgr.contentFrame('Settings/Views/EditView.aspx?treeType=partialViews&file=' + id);
 					    }
 		                ");
 		}
diff --git a/src/Umbraco.Web/WebServices/SaveFileController.cs b/src/Umbraco.Web/WebServices/SaveFileController.cs
index 3abfd31f52..bdae3e0804 100644
--- a/src/Umbraco.Web/WebServices/SaveFileController.cs
+++ b/src/Umbraco.Web/WebServices/SaveFileController.cs
@@ -38,56 +38,14 @@ namespace Umbraco.Web.WebServices
         [HttpPost]
         public JsonResult SavePartialViewMacro(string filename, string oldName, string contents)
         {
-            //NOTE: This is a bit of a hack because we're sharing the View editor with templates/partial views/partial view macros, so the path starts with
-            // 'Partials' which we need to remove because the path that we construct the partial view with is a relative path to the root path of 
-            // Views/Partials
-            if (filename.InvariantStartsWith("MacroPartials/"))
-            {
-                filename = filename.TrimStart("MacroPartials/");
-            }
-            if (oldName.IsNullOrWhiteSpace() == false)
-            {
-                if (oldName.InvariantStartsWith("MacroPartials/"))
-                {
-                    oldName = oldName.TrimStart("MacroPartials/");
-                }
-            }
+            var svce = (FileService) Services.FileService;
 
-            var fileService = (FileService)Services.FileService;
-
-            //try to get the file by the old name first if they are different and delete that file
-            if (filename.Trim().InvariantEquals(oldName.Trim()) == false)
-            {
-                var existing = fileService.GetPartialViewMacro(oldName);
-                if (existing != null)
-                {
-                    var success = fileService.DeletePartialViewMacro(existing.Path, Security.GetUserId());
-                    if (success == false)
-                    {
-                        return Failed(
-                            ui.Text("speechBubbles", "partialViewErrorText"), ui.Text("speechBubbles", "partialViewErrorHeader"),
-                            //pass in a new exception ... this will also append the the message
-                            new Exception("Could not delete old file: " + oldName));
-                    }
-                }
-            }
-
-            var partialView = new PartialView(filename)
-            {
-                Content = contents
-            };
-
-            var attempt = fileService.SavePartialViewMacro(partialView);
-
-            if (attempt.Success == false)
-            {
-                return Failed(
-                    ui.Text("speechBubbles", "partialViewErrorText"), ui.Text("speechBubbles", "partialViewErrorHeader"),
-                    //pass in a new exception ... this will also append the the message
-                    attempt.Exception);
-            }
-
-            return Success(ui.Text("speechBubbles", "partialViewSavedText"), ui.Text("speechBubbles", "partialViewSavedHeader"));
+            return SavePartialView(svce,
+                filename, oldName, contents,
+                "MacroPartials/",
+                (s, n) => s.GetPartialViewMacro(n),
+                (s, v) => s.ValidatePartialViewMacro((PartialView) v),
+                (s, v) => s.SavePartialViewMacro(v));
         }
 
         /// <summary>
@@ -100,53 +58,62 @@ namespace Umbraco.Web.WebServices
         [HttpPost]
         public JsonResult SavePartialView(string filename, string oldName, string contents)
         {
-            //NOTE: This is a bit of a hack because we're sharing the View editor with templates/partial views/partial view macros, so the path starts with
-            // 'Partials' which we need to remove because the path that we construct the partial view with is a relative path to the root path of 
-            // Views/Partials
-            if (filename.InvariantStartsWith("Partials/"))
-            {
-                filename = filename.TrimStart("Partials/");
-            }
-            if (oldName.IsNullOrWhiteSpace() == false)
-            {
-                if (oldName.InvariantStartsWith("Partials/"))
-                {
-                    oldName = oldName.TrimStart("Partials/");
-                }    
-            }
+            var svce = (FileService) Services.FileService;
 
-            var fileService = (FileService)Services.FileService;
-            
-            //try to get the file by the old name first if they are different and delete that file
-            if (filename.Trim().InvariantEquals(oldName.Trim()) == false)
+            return SavePartialView(svce,
+                filename, oldName, contents,
+                "Partials/",
+                (s, n) => s.GetPartialView(n),
+                (s, v) => s.ValidatePartialView((PartialView) v),
+                (s, v) => s.SavePartialView(v));
+        }
+
+        private JsonResult SavePartialView(IFileService svce,
+            string filename, string oldname, string contents,
+            string pathPrefix,
+            Func<IFileService, string, IPartialView> get,
+            Func<IFileService, IPartialView, bool> validate,
+            Func<IFileService, IPartialView, Attempt<IPartialView>> save)
+        {
+            // sharing the editor with partial views & partial view macros,
+            // using path prefix to differenciate,
+            // but the file service manages different filesystems,
+            // and we need to come back to filesystem-relative paths
+
+            // not sure why we still need this but not going to change it now
+
+            if (filename.InvariantStartsWith(pathPrefix))
+                filename = filename.TrimStart(pathPrefix);
+
+            if (oldname != null && oldname.InvariantStartsWith(pathPrefix))
+                oldname = oldname.TrimStart(pathPrefix);
+
+            var view = get(svce, oldname);
+            if (view == null)
+                view = new PartialView(filename);
+            else
+                view.Path = filename;
+            view.Content = contents;
+
+            Attempt<IPartialView> attempt;
+            try
             {
-                var existing = fileService.GetPartialView(oldName);
-                if (existing != null)
-                {
-                    var success = fileService.DeletePartialView(existing.Path, Security.GetUserId());
-                    if (success == false)
-                    {
-                        return Failed(
-                            ui.Text("speechBubbles", "partialViewErrorText"), ui.Text("speechBubbles", "partialViewErrorHeader"),
-                            //pass in a new exception ... this will also append the the message
-                            new Exception("Could not delete old file: " + oldName));
-                    }
-                }
+                var partialView = view as PartialView;
+                if (partialView != null && validate != null && validate(svce, partialView) == false)
+                    return Failed(ui.Text("speechBubbles", "partialViewErrorText"), ui.Text("speechBubbles", "partialViewErrorHeader"),
+                                    new FileSecurityException("File '" + view.Path + "' is not a valid partial view file."));
+
+                attempt = save(svce, view);
             }
-            
-            var partialView = new PartialView(filename)
+            catch (Exception e)
             {
-                Content = contents
-            };
-            
-            var attempt = fileService.SavePartialView(partialView);
+                return Failed(ui.Text("speechBubbles", "partialViewErrorText"), ui.Text("speechBubbles", "partialViewErrorHeader"), e);
+            }
 
             if (attempt.Success == false)
             {
-                return Failed(
-                    ui.Text("speechBubbles", "partialViewErrorText"), ui.Text("speechBubbles", "partialViewErrorHeader"),
-                    //pass in a new exception ... this will also append the the message
-                    attempt.Exception);
+                return Failed(ui.Text("speechBubbles", "partialViewErrorText"), ui.Text("speechBubbles", "partialViewErrorHeader"),
+                                attempt.Exception);
             }
 
             return Success(ui.Text("speechBubbles", "partialViewSavedText"), ui.Text("speechBubbles", "partialViewSavedHeader"));