From 95845c8f2e6c943ca56af7d30e39e4104a01db71 Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 1 Dec 2014 17:34:22 +1100 Subject: [PATCH] Almost completed U4-5847 Ensure the path is set correctly for templates in the db but needs a db migration script as well, more unit tests --- src/Umbraco.Core/IO/MasterPageHelper.cs | 36 ++-- src/Umbraco.Core/IO/ViewHelper.cs | 160 ++++++++--------- .../Repositories/TemplateRepository.cs | 67 +++---- .../Repositories/TemplateRepositoryTest.cs | 166 ++++++++++-------- .../businesslogic/template/Template.cs | 3 +- 5 files changed, 224 insertions(+), 208 deletions(-) diff --git a/src/Umbraco.Core/IO/MasterPageHelper.cs b/src/Umbraco.Core/IO/MasterPageHelper.cs index 8ddb8491d4..3677d99f23 100644 --- a/src/Umbraco.Core/IO/MasterPageHelper.cs +++ b/src/Umbraco.Core/IO/MasterPageHelper.cs @@ -24,26 +24,26 @@ namespace Umbraco.Core.IO return IOHelper.MapPath(SystemDirectories.Masterpages + "/" + t.Alias.Replace(" ", "") + ".master"); } - internal static string CreateMasterPage(ITemplate t, IFileService fileService, bool overWrite = false) - { - string masterpageContent = ""; + //internal static string CreateMasterPage(ITemplate t, IFileService fileService, bool overWrite = false) + //{ + // string masterpageContent = ""; - if (System.IO.File.Exists(GetFilePath(t)) == false || overWrite) - { - masterpageContent = CreateDefaultMasterPageContent(t, t.Alias, fileService); - SaveDesignToFile(t, null, masterpageContent); - } - else - { - using (var tr = new StreamReader(GetFilePath(t))) - { - masterpageContent = tr.ReadToEnd(); - tr.Close(); - } - } + // if (System.IO.File.Exists(GetFilePath(t)) == false || overWrite) + // { + // masterpageContent = CreateDefaultMasterPageContent(t, t.Alias, fileService); + // SaveDesignToFile(t, null, masterpageContent); + // } + // else + // { + // using (var tr = new StreamReader(GetFilePath(t))) + // { + // masterpageContent = tr.ReadToEnd(); + // tr.Close(); + // } + // } - return masterpageContent; - } + // return masterpageContent; + //} internal static string GetFileContents(ITemplate t) { diff --git a/src/Umbraco.Core/IO/ViewHelper.cs b/src/Umbraco.Core/IO/ViewHelper.cs index 9592664e2d..ba092f4e9c 100644 --- a/src/Umbraco.Core/IO/ViewHelper.cs +++ b/src/Umbraco.Core/IO/ViewHelper.cs @@ -26,43 +26,43 @@ namespace Umbraco.Core.IO return _viewFileSystem.GetFullPath(ViewPath(t.Alias)); } - internal string GetFileContents(ITemplate t) - { - string viewContent = ""; - string path = ViewPath(t.Alias); + //internal string GetFileContents(ITemplate t) + //{ + // string viewContent = ""; + // string path = ViewPath(t.Alias); - if (_viewFileSystem.FileExists(path)) - { - using (var tr = new StreamReader(_viewFileSystem.OpenFile(path))) - { - viewContent = tr.ReadToEnd(); - tr.Close(); - } - } + // if (_viewFileSystem.FileExists(path)) + // { + // using (var tr = new StreamReader(_viewFileSystem.OpenFile(path))) + // { + // viewContent = tr.ReadToEnd(); + // tr.Close(); + // } + // } - return viewContent; - } + // return viewContent; + //} - internal string CreateViewFile(ITemplate t, bool overWrite = false) - { - string viewContent; - string path = ViewPath(t.Alias); + //internal string CreateViewFile(ITemplate t, bool overWrite = false) + //{ + // string viewContent; + // string path = ViewPath(t.Alias); - if (_viewFileSystem.FileExists(path) == false || overWrite) - { - viewContent = SaveTemplateToFile(t, t.Alias); - } - else - { - using (var tr = new StreamReader(_viewFileSystem.OpenFile(path))) - { - viewContent = tr.ReadToEnd(); - tr.Close(); - } - } + // if (_viewFileSystem.FileExists(path) == false || overWrite) + // { + // viewContent = SaveTemplateToFile(t, t.Alias); + // } + // else + // { + // using (var tr = new StreamReader(_viewFileSystem.OpenFile(path))) + // { + // viewContent = tr.ReadToEnd(); + // tr.Close(); + // } + // } - return viewContent; - } + // return viewContent; + //} internal static string GetDefaultFileContent(string layoutPageAlias = null) { @@ -77,61 +77,61 @@ namespace Umbraco.Core.IO return design; } - internal string SaveTemplateToFile(ITemplate template, string currentAlias) - { - var design = EnsureInheritedLayout(template); - var path = ViewPath(template.Alias); + //internal string SaveTemplateToFile(ITemplate template, string currentAlias) + //{ + // var design = EnsureInheritedLayout(template); + // var path = ViewPath(template.Alias); - using (var ms = new MemoryStream()) - using (var writer = new StreamWriter(ms, Encoding.UTF8)) - { - writer.Write(design); - _viewFileSystem.AddFile(path, ms, true); - } + // using (var ms = new MemoryStream()) + // using (var writer = new StreamWriter(ms, Encoding.UTF8)) + // { + // writer.Write(design); + // _viewFileSystem.AddFile(path, ms, true); + // } - return template.Content; - } + // return template.Content; + //} - internal string UpdateViewFile(ITemplate t, string currentAlias = null) - { - var path = ViewPath(t.Alias); + //internal string UpdateViewFile(ITemplate t, string currentAlias = null) + //{ + // var path = ViewPath(t.Alias); - if (string.IsNullOrEmpty(currentAlias) == false && currentAlias != t.Alias) - { - //NOTE: I don't think this is needed for MVC, this was ported over from the - // masterpages helper but I think only relates to when templates are stored in the db. - ////Ensure that child templates have the right master masterpage file name - //if (t.HasChildren) - //{ - // var c = t.Children; - // foreach (CMSNode cmn in c) - // UpdateViewFile(new Template(cmn.Id), null); - //} + // if (string.IsNullOrEmpty(currentAlias) == false && currentAlias != t.Alias) + // { + // //NOTE: I don't think this is needed for MVC, this was ported over from the + // // masterpages helper but I think only relates to when templates are stored in the db. + // ////Ensure that child templates have the right master masterpage file name + // //if (t.HasChildren) + // //{ + // // var c = t.Children; + // // foreach (CMSNode cmn in c) + // // UpdateViewFile(new Template(cmn.Id), null); + // //} - //then kill the old file.. - var oldFile = ViewPath(currentAlias); - if (_viewFileSystem.FileExists(oldFile)) - _viewFileSystem.DeleteFile(oldFile); - } + // //then kill the old file.. + // var oldFile = ViewPath(currentAlias); + // if (_viewFileSystem.FileExists(oldFile)) + // _viewFileSystem.DeleteFile(oldFile); + // } - using (var ms = new MemoryStream()) - using (var writer = new StreamWriter(ms, Encoding.UTF8)) - { - writer.Write(t.Content); - _viewFileSystem.AddFile(path, ms, true); - } - return t.Content; - } + // using (var ms = new MemoryStream()) + // using (var writer = new StreamWriter(ms, Encoding.UTF8)) + // { + // writer.Write(t.Content); + // _viewFileSystem.AddFile(path, ms, true); + // } + // return t.Content; + //} - internal void RemoveViewFile(string alias) - { - if (string.IsNullOrWhiteSpace(alias) == false) - { - var file = ViewPath(alias); - if (_viewFileSystem.FileExists(file)) - _viewFileSystem.DeleteFile(file); - } - } + //internal void RemoveViewFile(string alias) + //{ + // if (string.IsNullOrWhiteSpace(alias) == false) + // { + // var file = ViewPath(alias); + // if (_viewFileSystem.FileExists(file)) + // _viewFileSystem.DeleteFile(file); + // } + //} public string ViewPath(string alias) { diff --git a/src/Umbraco.Core/Persistence/Repositories/TemplateRepository.cs b/src/Umbraco.Core/Persistence/Repositories/TemplateRepository.cs index 55d6c9dbb2..46603ddb2c 100644 --- a/src/Umbraco.Core/Persistence/Repositories/TemplateRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/TemplateRepository.cs @@ -201,26 +201,23 @@ namespace Umbraco.Core.Persistence.Repositories var factory = new TemplateFactory(NodeObjectTypeId, _viewsFileSystem); var dto = factory.BuildDto(template); - //NOTE: There is no reason for sort order, path or level with templates, also the ParentId column is NOT used, need to fix: - // http://issues.umbraco.org/issue/U4-5846 - //var parent = Database.First("WHERE id = @ParentId", new { ParentId = template.ParentId }); - //int level = parent.Level + 1; - //int sortOrder = - // Database.ExecuteScalar("SELECT COUNT(*) FROM umbracoNode WHERE parentID = @ParentId AND nodeObjectType = @NodeObjectType", - // new { ParentId = template.ParentId, NodeObjectType = NodeObjectTypeId }); - //Create the (base) node data - umbracoNode var nodeDto = dto.NodeDto; nodeDto.Path = "-1," + dto.NodeDto.NodeId; - nodeDto.Level = 1; - nodeDto.SortOrder = 0; var o = Database.IsNew(nodeDto) ? Convert.ToInt32(Database.Insert(nodeDto)) : Database.Update(nodeDto); - //NOTE: Templates don't have paths, but they could i suppose if we wanted, once the ParentId thing is fixed we could - // do it, but we haven't had that since the beginning of time so don't think it's necessary at all. //Update with new correct path - //nodeDto.Path = string.Concat(parent.Path, ",", nodeDto.NodeId); - + //TODO: need to fix: + // http://issues.umbraco.org/issue/U4-5846 + var parent = Get(template.MasterTemplateId.Value); + if (parent != null) + { + nodeDto.Path = string.Concat(parent.Path, ",", nodeDto.NodeId); + } + else + { + nodeDto.Path = "-1," + dto.NodeDto.NodeId; + } Database.Update(nodeDto); //Insert template dto @@ -268,25 +265,25 @@ namespace Umbraco.Core.Persistence.Repositories } } - //NOTE: There is no reason for sort order, path or level with templates, also the ParentId column is NOT used, need to fix: + var template = (Template)entity; + + //TODO: need to fix: // http://issues.umbraco.org/issue/U4-5846 - ////Look up parent to get and set the correct Path if ParentId has changed - //if (entity.IsPropertyDirty("ParentId")) - //{ - // var parent = Database.First("WHERE id = @ParentId", new { ParentId = ((Template)entity).ParentId }); - // entity.Path = string.Concat(parent.Path, ",", entity.Id); - // ((Template)entity).Level = parent.Level + 1; - // var maxSortOrder = - // Database.ExecuteScalar( - // "SELECT coalesce(max(sortOrder),0) FROM umbracoNode WHERE parentid = @ParentId AND nodeObjectType = @NodeObjectType", - // new { ParentId = ((Template)entity).ParentId, NodeObjectType = NodeObjectTypeId }); - // ((Template)entity).SortOrder = maxSortOrder + 1; - //} + // And use the ParentId column instead of the extra template parent id column + if (entity.IsPropertyDirty("MasterTemplateId")) + { + var parent = Get(template.MasterTemplateId.Value); + if (parent != null) + { + entity.Path = string.Concat(parent.Path, ",", entity.Id); + } + + } //Get TemplateDto from db to get the Primary key of the entity var templateDto = Database.SingleOrDefault("WHERE nodeId = @Id", new { Id = entity.Id }); //Save updated entity to db - var template = (Template)entity; + template.UpdateDate = DateTime.Now; var factory = new TemplateFactory(templateDto.PrimaryKey, NodeObjectTypeId); var dto = factory.BuildDto(template); @@ -420,14 +417,11 @@ namespace Umbraco.Core.Persistence.Repositories string path = string.Empty; using (var stream = _viewsFileSystem.OpenFile(fileName)) + using (var reader = new StreamReader(stream, Encoding.UTF8)) { - byte[] bytes = new byte[stream.Length]; - stream.Position = 0; - stream.Read(bytes, 0, (int)stream.Length); - content = Encoding.UTF8.GetString(bytes); + content = reader.ReadToEnd(); } - template.Path = _viewsFileSystem.GetRelativePath(fileName); template.UpdateDate = _viewsFileSystem.GetLastModified(path).UtcDateTime; //Currently set with db values, but will eventually be changed //template.CreateDate = _viewsFileSystem.GetCreated(path).UtcDateTime; @@ -442,14 +436,11 @@ namespace Umbraco.Core.Persistence.Repositories string path = string.Empty; using (var stream = _masterpagesFileSystem.OpenFile(fileName)) + using (var reader = new StreamReader(stream, Encoding.UTF8)) { - byte[] bytes = new byte[stream.Length]; - stream.Position = 0; - stream.Read(bytes, 0, (int)stream.Length); - content = Encoding.UTF8.GetString(bytes); + content = reader.ReadToEnd(); } - template.Path = _masterpagesFileSystem.GetRelativePath(fileName); template.UpdateDate = _masterpagesFileSystem.GetLastModified(path).UtcDateTime; //Currently set with db values, but will eventually be changed //template.CreateDate = _masterpagesFileSystem.GetCreated(path).UtcDateTime; diff --git a/src/Umbraco.Tests/Persistence/Repositories/TemplateRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/TemplateRepositoryTest.cs index 262fb93b0a..22c5ab08b3 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/TemplateRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/TemplateRepositoryTest.cs @@ -423,89 +423,113 @@ namespace Umbraco.Tests.Persistence.Repositories } - //[Test] - //public void Can_Perform_Get_On_ScriptRepository() - //{ - // // Arrange - // var provider = new FileUnitOfWorkProvider(); - // var unitOfWork = provider.GetUnitOfWork(); - // var repository = new ScriptRepository(unitOfWork, _masterPageFileSystem); + [Test] + public void Path_Is_Set_Correctly_On_Creation() + { + // Arrange + var provider = new PetaPocoUnitOfWorkProvider(); + var unitOfWork = provider.GetUnitOfWork(); + using (var repository = new TemplateRepository(unitOfWork, NullCacheProvider.Current, _masterPageFileSystem, _viewsFileSystem, + Mock.Of(t => t.DefaultRenderingEngine == RenderingEngine.Mvc))) + { + var parent = new Template("parent", "parent"); + var child1 = new Template("child1", "child1"); + var toddler1 = new Template("toddler1", "toddler1"); + var toddler2 = new Template("toddler2", "toddler2"); + var baby1 = new Template("baby1", "baby1"); + var child2 = new Template("child2", "child2"); + var toddler3 = new Template("toddler3", "toddler3"); + var toddler4 = new Template("toddler4", "toddler4"); + var baby2 = new Template("baby2", "baby2"); - // // Act - // var exists = repository.Get("test-script.js"); + child1.MasterTemplateAlias = parent.Alias; + child1.MasterTemplateId = new Lazy(() => parent.Id); + child2.MasterTemplateAlias = parent.Alias; + child2.MasterTemplateId = new Lazy(() => parent.Id); + toddler1.MasterTemplateAlias = child1.Alias; + toddler1.MasterTemplateId = new Lazy(() => child1.Id); + toddler2.MasterTemplateAlias = child1.Alias; + toddler2.MasterTemplateId = new Lazy(() => child1.Id); + toddler3.MasterTemplateAlias = child2.Alias; + toddler3.MasterTemplateId = new Lazy(() => child2.Id); + toddler4.MasterTemplateAlias = child2.Alias; + toddler4.MasterTemplateId = new Lazy(() => child2.Id); + baby1.MasterTemplateAlias = toddler2.Alias; + baby1.MasterTemplateId = new Lazy(() => toddler2.Id); + baby2.MasterTemplateAlias = toddler4.Alias; + baby2.MasterTemplateId = new Lazy(() => toddler4.Id); + - // // Assert - // Assert.That(exists, Is.Not.Null); - // Assert.That(exists.Alias, Is.EqualTo("test-script")); - // Assert.That(exists.Name, Is.EqualTo("test-script.js")); - //} + // Act + repository.AddOrUpdate(parent); + repository.AddOrUpdate(child1); + repository.AddOrUpdate(child2); + repository.AddOrUpdate(toddler1); + repository.AddOrUpdate(toddler2); + repository.AddOrUpdate(toddler3); + repository.AddOrUpdate(toddler4); + repository.AddOrUpdate(baby1); + repository.AddOrUpdate(baby2); + unitOfWork.Commit(); - //[Test] - //public void Can_Perform_GetAll_On_ScriptRepository() - //{ - // // Arrange - // var provider = new FileUnitOfWorkProvider(); - // var unitOfWork = provider.GetUnitOfWork(); - // var repository = new ScriptRepository(unitOfWork, _masterPageFileSystem); + // Assert + Assert.AreEqual(string.Format("-1,{0}", parent.Id), parent.Path); + Assert.AreEqual(string.Format("-1,{0},{1}", parent.Id, child1.Id), child1.Path); + Assert.AreEqual(string.Format("-1,{0},{1}", parent.Id, child2.Id), child2.Path); + Assert.AreEqual(string.Format("-1,{0},{1}", parent.Id, child2.Id), child2.Path); + Assert.AreEqual(string.Format("-1,{0},{1},{2}", parent.Id, child1.Id, toddler1.Id), toddler1.Path); + Assert.AreEqual(string.Format("-1,{0},{1},{2}", parent.Id, child1.Id, toddler2.Id), toddler2.Path); + Assert.AreEqual(string.Format("-1,{0},{1},{2}", parent.Id, child2.Id, toddler3.Id), toddler3.Path); + Assert.AreEqual(string.Format("-1,{0},{1},{2}", parent.Id, child2.Id, toddler4.Id), toddler4.Path); + Assert.AreEqual(string.Format("-1,{0},{1},{2},{3}", parent.Id, child1.Id, toddler2.Id, baby1.Id), baby1.Path); + Assert.AreEqual(string.Format("-1,{0},{1},{2},{3}", parent.Id, child2.Id, toddler4.Id, baby2.Id), baby2.Path); + } - // var script = new Script("test-script1.js") { Content = "/// " }; - // repository.AddOrUpdate(script); - // var script2 = new Script("test-script2.js") { Content = "/// " }; - // repository.AddOrUpdate(script2); - // var script3 = new Script("test-script3.js") { Content = "/// " }; - // repository.AddOrUpdate(script3); - // unitOfWork.Commit(); - // // Act - // var scripts = repository.GetAll(); + } - // // Assert - // Assert.That(scripts, Is.Not.Null); - // Assert.That(scripts.Any(), Is.True); - // Assert.That(scripts.Any(x => x == null), Is.False); - // Assert.That(scripts.Count(), Is.EqualTo(4)); - //} + [Test] + public void Path_Is_Set_Correctly_On_Update() + { + // Arrange + var provider = new PetaPocoUnitOfWorkProvider(); + var unitOfWork = provider.GetUnitOfWork(); + using (var repository = new TemplateRepository(unitOfWork, NullCacheProvider.Current, _masterPageFileSystem, _viewsFileSystem, + Mock.Of(t => t.DefaultRenderingEngine == RenderingEngine.Mvc))) + { + var parent = new Template("parent", "parent"); + var child1 = new Template("child1", "child1"); + var child2 = new Template("child2", "child2"); + var toddler1 = new Template("toddler1", "toddler1"); + var toddler2 = new Template("toddler2", "toddler2"); - //[Test] - //public void Can_Perform_GetAll_With_Params_On_ScriptRepository() - //{ - // // Arrange - // var provider = new FileUnitOfWorkProvider(); - // var unitOfWork = provider.GetUnitOfWork(); - // var repository = new ScriptRepository(unitOfWork, _masterPageFileSystem); + child1.MasterTemplateAlias = parent.Alias; + child1.MasterTemplateId = new Lazy(() => parent.Id); + child2.MasterTemplateAlias = parent.Alias; + child2.MasterTemplateId = new Lazy(() => parent.Id); + toddler1.MasterTemplateAlias = child1.Alias; + toddler1.MasterTemplateId = new Lazy(() => child1.Id); + toddler2.MasterTemplateAlias = child1.Alias; + toddler2.MasterTemplateId = new Lazy(() => child1.Id); - // var script = new Script("test-script1.js") { Content = "/// " }; - // repository.AddOrUpdate(script); - // var script2 = new Script("test-script2.js") { Content = "/// " }; - // repository.AddOrUpdate(script2); - // var script3 = new Script("test-script3.js") { Content = "/// " }; - // repository.AddOrUpdate(script3); - // unitOfWork.Commit(); + repository.AddOrUpdate(parent); + repository.AddOrUpdate(child1); + repository.AddOrUpdate(child2); + repository.AddOrUpdate(toddler1); + repository.AddOrUpdate(toddler2); + unitOfWork.Commit(); - // // Act - // var scripts = repository.GetAll("test-script1.js", "test-script2.js"); + //Act + toddler2.SetMasterTemplate(child2); + repository.AddOrUpdate(toddler2); + unitOfWork.Commit(); - // // Assert - // Assert.That(scripts, Is.Not.Null); - // Assert.That(scripts.Any(), Is.True); - // Assert.That(scripts.Any(x => x == null), Is.False); - // Assert.That(scripts.Count(), Is.EqualTo(2)); - //} + //Assert + Assert.AreEqual(string.Format("-1,{0},{1},{2}", parent.Id, child2.Id, toddler2.Id), toddler2.Path); - //[Test] - //public void Can_Perform_Exists_On_ScriptRepository() - //{ - // // Arrange - // var provider = new FileUnitOfWorkProvider(); - // var unitOfWork = provider.GetUnitOfWork(); - // var repository = new ScriptRepository(unitOfWork, _masterPageFileSystem); + } + } - // // Act - // var exists = repository.Exists("test-script.js"); - - // // Assert - // Assert.That(exists, Is.True); - //} [TearDown] public override void TearDown() diff --git a/src/umbraco.cms/businesslogic/template/Template.cs b/src/umbraco.cms/businesslogic/template/Template.cs index 1999ba8989..d359cfbcf4 100644 --- a/src/umbraco.cms/businesslogic/template/Template.cs +++ b/src/umbraco.cms/businesslogic/template/Template.cs @@ -119,7 +119,8 @@ namespace umbraco.cms.businesslogic.template //return base.Text; } - //TODO: This is the name of the template, which can apparenlty be localized using the umbraco dictionary, so we need to cater for this! + //TODO: This is the name of the template, which can apparenlty be localized using the umbraco dictionary, so we need to cater for this but that + // shoud really be done as part of mapping logic for models that are being consumed in the UI, not at the business logic layer. public override string Text { get