From 9911f0374579d04c951cacb75e8e649f3f062f1a Mon Sep 17 00:00:00 2001 From: Sandro Ciervo Date: Thu, 7 Nov 2013 20:09:06 +0100 Subject: [PATCH 1/4] Fixed potential DbReaderException - Use Fetch instead of Query because we create another DbReader inside the loop with DoesMacroHaveParameters(). --- src/Umbraco.Web/UI/Controls/InsertMacroSplitButton.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Umbraco.Web/UI/Controls/InsertMacroSplitButton.cs b/src/Umbraco.Web/UI/Controls/InsertMacroSplitButton.cs index 50968413ae..4fa9146dbc 100644 --- a/src/Umbraco.Web/UI/Controls/InsertMacroSplitButton.cs +++ b/src/Umbraco.Web/UI/Controls/InsertMacroSplitButton.cs @@ -71,7 +71,7 @@ namespace Umbraco.Web.UI.Controls var divMacroItemContainer = new TagBuilder("div"); divMacroItemContainer.Attributes.Add("style", "width: 285px;display:none;"); divMacroItemContainer.Attributes.Add("class", "sbMenu"); - var macros = ApplicationContext.DatabaseContext.Database.Query("select id, macroAlias, macroName from cmsMacro order by macroName"); + var macros = ApplicationContext.DatabaseContext.Database.Fetch("select id, macroAlias, macroName from cmsMacro order by macroName"); foreach (var macro in macros) { var divMacro = new TagBuilder("div"); From e94ca7513ba0d4dcd8bd35b257f7ff22ba6bf36b Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 12 Nov 2013 20:50:17 +1100 Subject: [PATCH 2/4] Fixes: U4-3473 Cannot edit Partial views - getting YSOD data reader error - PetaPoco error will backport to 6.2 --- src/Umbraco.Web/UI/Controls/InsertMacroSplitButton.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Umbraco.Web/UI/Controls/InsertMacroSplitButton.cs b/src/Umbraco.Web/UI/Controls/InsertMacroSplitButton.cs index 50968413ae..4fa9146dbc 100644 --- a/src/Umbraco.Web/UI/Controls/InsertMacroSplitButton.cs +++ b/src/Umbraco.Web/UI/Controls/InsertMacroSplitButton.cs @@ -71,7 +71,7 @@ namespace Umbraco.Web.UI.Controls var divMacroItemContainer = new TagBuilder("div"); divMacroItemContainer.Attributes.Add("style", "width: 285px;display:none;"); divMacroItemContainer.Attributes.Add("class", "sbMenu"); - var macros = ApplicationContext.DatabaseContext.Database.Query("select id, macroAlias, macroName from cmsMacro order by macroName"); + var macros = ApplicationContext.DatabaseContext.Database.Fetch("select id, macroAlias, macroName from cmsMacro order by macroName"); foreach (var macro in macros) { var divMacro = new TagBuilder("div"); From abb6b6519fcf719817ee0cf06e241a236aeb5ed0 Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 12 Nov 2013 20:52:55 +1100 Subject: [PATCH 3/4] Fixes gravatar JS issue if user times out --- .../src/views/common/main.controller.js | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/Umbraco.Web.UI.Client/src/views/common/main.controller.js b/src/Umbraco.Web.UI.Client/src/views/common/main.controller.js index 9141369783..6234d6df70 100644 --- a/src/Umbraco.Web.UI.Client/src/views/common/main.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/common/main.controller.js @@ -99,16 +99,18 @@ function MainController($scope, $location, $routeParams, $rootScope, $timeout, $ } if($scope.user.emailHash){ - $timeout(function(){ + $timeout(function () { //yes this is wrong.. - $("#avatar-img").fadeTo(1000, 0, function(){ - - $timeout(function(){ - $scope.avatar = "http://www.gravatar.com/avatar/" + $scope.user.emailHash +".jpg?s=64&d=mm"; - }); - - $("#avatar-img").fadeTo(1000, 1); + $("#avatar-img").fadeTo(1000, 0, function () { + $timeout(function () { + //this can be null if they time out + if ($scope.user && $scope.user.emailHash) { + $scope.avatar = "http://www.gravatar.com/avatar/" + $scope.user.emailHash + ".jpg?s=64&d=mm"; + } + }); + $("#avatar-img").fadeTo(1000, 1); }); + }, 3000); } From 2129ac824115d1eb66099824e5b9b5354e3249ec Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 12 Nov 2013 22:28:43 +1100 Subject: [PATCH 4/4] Adds template repository tests, updates template repo business logic to be able to delete the whole template tree, updates some script repo tests too. --- src/Umbraco.Core/Models/Template.cs | 8 - .../Repositories/TemplateRepository.cs | 54 ++++ .../Repositories/ScriptRepositoryTest.cs | 19 +- .../Repositories/TemplateRepositoryTest.cs | 268 ++++++++++++++++++ src/Umbraco.Tests/Umbraco.Tests.csproj | 1 + 5 files changed, 337 insertions(+), 13 deletions(-) create mode 100644 src/Umbraco.Tests/Persistence/Repositories/TemplateRepositoryTest.cs diff --git a/src/Umbraco.Core/Models/Template.cs b/src/Umbraco.Core/Models/Template.cs index bb8ff3b011..33bc3e6b86 100644 --- a/src/Umbraco.Core/Models/Template.cs +++ b/src/Umbraco.Core/Models/Template.cs @@ -30,14 +30,6 @@ namespace Umbraco.Core.Models //private static readonly PropertyInfo MasterTemplateIdSelector = ExpressionHelper.GetPropertyInfo(x => x.MasterTemplateId); private static readonly PropertyInfo MasterTemplateAliasSelector = ExpressionHelper.GetPropertyInfo(x => x.MasterTemplateAlias); - - internal Template(string path) - : base(path) - { - base.Path = path; - ParentId = -1; - } - public Template(string path, string name, string alias) : base(path) { diff --git a/src/Umbraco.Core/Persistence/Repositories/TemplateRepository.cs b/src/Umbraco.Core/Persistence/Repositories/TemplateRepository.cs index 5cc827680b..22ffba9f6d 100644 --- a/src/Umbraco.Core/Persistence/Repositories/TemplateRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/TemplateRepository.cs @@ -271,8 +271,62 @@ namespace Umbraco.Core.Persistence.Repositories ((ICanBeDirty)entity).ResetDirtyProperties(); } + private void PersistDeletedTemplate(TemplateDto dto) + { + //we need to get the real template for this item unfortunately to remove it + var template = Get(dto.NodeId); + if (template != null) + { + //NOTE: We must cast here so that it goes to the outter method to + // ensure the cache is updated. + PersistDeletedItem((IEntity)template); + } + } + + /// + /// Returns a list of templates in order of descendants from the parent + /// + /// + /// + /// + private static List GenerateTemplateHierarchy(TemplateDto template, IEnumerable allChildTemplates) + { + var hierarchy = new List {template}; + foreach (var t in allChildTemplates.Where(x => x.Master == template.NodeId)) + { + hierarchy.AddRange(GenerateTemplateHierarchy(t, allChildTemplates)); + } + return hierarchy; + } + protected override void PersistDeletedItem(ITemplate entity) { + + //TODO: This isn't the most ideal way to delete a template tree, because below it will actually end up + // recursing back to this method for each descendant and re-looking up the template list causing an extrac + // SQL call - not ideal but there shouldn't ever be a heaping list of descendant templates. + //The easiest way to overcome this is to expose the underlying cache upwards so that the repository has access + // to it, then in the PersistDeletedTemplate we wouldn't recurse the underlying function, we'd just call + // PersistDeletedItem with a Template object and clear it's cache. + + var sql = new Sql(); + sql.Select("*").From().Where(dto => dto.Master != null || dto.NodeId == entity.Id); + var dtos = Database.Fetch(sql); + var self = dtos.Single(x => x.NodeId == entity.Id); + var allChildren = dtos.Except(new[] {self}); + var hierarchy = GenerateTemplateHierarchy(self, allChildren); + //remove ourselves + hierarchy.Remove(self); + //change the order so it goes bottom up! + hierarchy.Reverse(); + + //delete the hierarchy + foreach (var descendant in hierarchy) + { + PersistDeletedTemplate(descendant); + } + + //now we can delete this one base.PersistDeletedItem(entity); //Check for file under the Masterpages filesystem diff --git a/src/Umbraco.Tests/Persistence/Repositories/ScriptRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/ScriptRepositoryTest.cs index 1f8cd7d582..566c2c3c62 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/ScriptRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/ScriptRepositoryTest.cs @@ -7,20 +7,25 @@ using Umbraco.Core.Models; using Umbraco.Core.Persistence; using Umbraco.Core.Persistence.Repositories; using Umbraco.Core.Persistence.UnitOfWork; +using Umbraco.Tests.TestHelpers; namespace Umbraco.Tests.Persistence.Repositories { [TestFixture] - public class ScriptRepositoryTest + public class ScriptRepositoryTest : BaseUmbracoApplicationTest { private IFileSystem _fileSystem; [SetUp] - public void Initialize() + public override void Initialize() { + base.Initialize(); + _fileSystem = new PhysicalFileSystem(SystemDirectories.Scripts); - var stream = CreateStream("Umbraco.Sys.registerNamespace(\"Umbraco.Utils\");"); - _fileSystem.AddFile("test-script.js", stream); + using (var stream = CreateStream("Umbraco.Sys.registerNamespace(\"Umbraco.Utils\");")) + { + _fileSystem.AddFile("test-script.js", stream); + } } [Test] @@ -93,6 +98,8 @@ namespace Umbraco.Tests.Persistence.Repositories // Assert + Assert.IsFalse(repository.Exists("test-script.js")); + } [Test] @@ -180,8 +187,10 @@ namespace Umbraco.Tests.Persistence.Repositories } [TearDown] - public void TearDown() + public override void TearDown() { + base.TearDown(); + _fileSystem = null; //Delete all files var fs = new PhysicalFileSystem(SystemDirectories.Scripts); diff --git a/src/Umbraco.Tests/Persistence/Repositories/TemplateRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/TemplateRepositoryTest.cs new file mode 100644 index 0000000000..46e4076711 --- /dev/null +++ b/src/Umbraco.Tests/Persistence/Repositories/TemplateRepositoryTest.cs @@ -0,0 +1,268 @@ +using System; +using System.IO; +using System.Linq; +using System.Text; +using NUnit.Framework; +using Umbraco.Core.IO; +using Umbraco.Core.Models; +using Umbraco.Core.Persistence; +using Umbraco.Core.Persistence.Caching; +using Umbraco.Core.Persistence.Repositories; +using Umbraco.Core.Persistence.UnitOfWork; +using Umbraco.Tests.TestHelpers; + +namespace Umbraco.Tests.Persistence.Repositories +{ + [TestFixture] + public class TemplateRepositoryTest : BaseDatabaseFactoryTest + { + private IFileSystem _masterPageFileSystem; + private IFileSystem _viewsFileSystem; + + [SetUp] + public override void Initialize() + { + base.Initialize(); + + _masterPageFileSystem = new PhysicalFileSystem(SystemDirectories.Masterpages); + _viewsFileSystem = new PhysicalFileSystem(SystemDirectories.MvcViews); + } + + [Test] + public void Can_Instantiate_Repository_From_Resolver() + { + // Arrange + var provider = new PetaPocoUnitOfWorkProvider(); + var unitOfWork = provider.GetUnitOfWork(); + + // Act + var repository = RepositoryResolver.Current.ResolveByType(unitOfWork); + + // Assert + Assert.That(repository, Is.Not.Null); + } + + [Test] + public void Can_Instantiate_Repository() + { + // Arrange + var provider = new PetaPocoUnitOfWorkProvider(); + var unitOfWork = provider.GetUnitOfWork(); + + // Act + var repository = new TemplateRepository(unitOfWork, NullCacheProvider.Current, _masterPageFileSystem, _viewsFileSystem); + + // Assert + Assert.That(repository, Is.Not.Null); + } + + [Test] + public void Can_Perform_Add_MasterPage() + { + // Arrange + var provider = new PetaPocoUnitOfWorkProvider(); + var unitOfWork = provider.GetUnitOfWork(); + var repository = new TemplateRepository(unitOfWork, NullCacheProvider.Current, _masterPageFileSystem, _viewsFileSystem); + + // Act + var template = new Template("test-add-masterpage.master", "test", "test") { Content = @"<%@ Master Language=""C#"" %>" }; + repository.AddOrUpdate(template); + unitOfWork.Commit(); + + //Assert + Assert.That(repository.Get("test"), Is.Not.Null); + Assert.That(_masterPageFileSystem.FileExists("test.master"), Is.True); + } + + [Test] + public void Can_Perform_Update_MasterPage() + { + // Arrange + var provider = new PetaPocoUnitOfWorkProvider(); + var unitOfWork = provider.GetUnitOfWork(); + var repository = new TemplateRepository(unitOfWork, NullCacheProvider.Current, _masterPageFileSystem, _viewsFileSystem); + + // Act + var template = new Template("test-updated-masterpage.master", "test", "test") { Content = @"<%@ Master Language=""C#"" %>" }; + repository.AddOrUpdate(template); + unitOfWork.Commit(); + + template.Content = @"<%@ Master Language=""VB"" %>"; + repository.AddOrUpdate(template); + unitOfWork.Commit(); + + var updated = repository.Get("test"); + + // Assert + Assert.That(_masterPageFileSystem.FileExists("test.master"), Is.True); + Assert.That(updated.Content, Is.EqualTo(@"<%@ Master Language=""VB"" %>")); + } + + [Test] + public void Can_Perform_Delete() + { + // Arrange + var provider = new PetaPocoUnitOfWorkProvider(); + var unitOfWork = provider.GetUnitOfWork(); + var repository = new TemplateRepository(unitOfWork, NullCacheProvider.Current, _masterPageFileSystem, _viewsFileSystem); + + var template = new Template("test-add-masterpage.master", "test", "test") { Content = @"<%@ Master Language=""C#"" %>" }; + repository.AddOrUpdate(template); + unitOfWork.Commit(); + + // Act + var templates = repository.Get("test"); + repository.Delete(templates); + unitOfWork.Commit(); + + // Assert + Assert.IsNull(repository.Get("test")); + } + + [Test] + public void Can_Perform_Delete_On_Nested_Templates() + { + // Arrange + var provider = new PetaPocoUnitOfWorkProvider(); + var unitOfWork = provider.GetUnitOfWork(); + var repository = new TemplateRepository(unitOfWork, NullCacheProvider.Current, _masterPageFileSystem, _viewsFileSystem); + + var parent = new Template("test-parent-masterpage.master", "parent", "parent") { Content = @"<%@ Master Language=""C#"" %>" }; + var child = new Template("test-child-masterpage.master", "child", "child") { Content = @"<%@ Master Language=""C#"" %>" }; + var baby = new Template("test-baby-masterpage.master", "baby", "baby") { Content = @"<%@ Master Language=""C#"" %>" }; + child.MasterTemplateAlias = parent.Alias; + child.MasterTemplateId = new Lazy(() => parent.Id); + baby.MasterTemplateAlias = child.Alias; + baby.MasterTemplateId = new Lazy(() => child.Id); + repository.AddOrUpdate(parent); + repository.AddOrUpdate(child); + repository.AddOrUpdate(baby); + unitOfWork.Commit(); + + // Act + var templates = repository.Get("parent"); + repository.Delete(templates); + unitOfWork.Commit(); + + // Assert + Assert.IsNull(repository.Get("test")); + } + + //[Test] + //public void Can_Perform_Get_On_ScriptRepository() + //{ + // // Arrange + // var provider = new FileUnitOfWorkProvider(); + // var unitOfWork = provider.GetUnitOfWork(); + // var repository = new ScriptRepository(unitOfWork, _masterPageFileSystem); + + // // Act + // var exists = repository.Get("test-script.js"); + + // // Assert + // Assert.That(exists, Is.Not.Null); + // Assert.That(exists.Alias, Is.EqualTo("test-script")); + // Assert.That(exists.Name, Is.EqualTo("test-script.js")); + //} + + //[Test] + //public void Can_Perform_GetAll_On_ScriptRepository() + //{ + // // Arrange + // var provider = new FileUnitOfWorkProvider(); + // var unitOfWork = provider.GetUnitOfWork(); + // var repository = new ScriptRepository(unitOfWork, _masterPageFileSystem); + + // 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 Can_Perform_GetAll_With_Params_On_ScriptRepository() + //{ + // // Arrange + // var provider = new FileUnitOfWorkProvider(); + // var unitOfWork = provider.GetUnitOfWork(); + // var repository = new ScriptRepository(unitOfWork, _masterPageFileSystem); + + // 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("test-script1.js", "test-script2.js"); + + // // 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)); + //} + + //[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() + { + base.TearDown(); + + _masterPageFileSystem = null; + _viewsFileSystem = null; + //Delete all files + var fsMaster = new PhysicalFileSystem(SystemDirectories.Masterpages); + var masterPages = fsMaster.GetFiles("", "*.master"); + foreach (var file in masterPages) + { + fsMaster.DeleteFile(file); + } + var fsViews = new PhysicalFileSystem(SystemDirectories.MvcViews); + var views = fsMaster.GetFiles("", "*.cshtml"); + foreach (var file in views) + { + fsMaster.DeleteFile(file); + } + } + + protected Stream CreateStream(string contents = null) + { + if (string.IsNullOrEmpty(contents)) + contents = "/* test */"; + + var bytes = Encoding.UTF8.GetBytes(contents); + var stream = new MemoryStream(bytes); + + return stream; + } + } +} \ No newline at end of file diff --git a/src/Umbraco.Tests/Umbraco.Tests.csproj b/src/Umbraco.Tests/Umbraco.Tests.csproj index a05a8b8f0f..16d227422a 100644 --- a/src/Umbraco.Tests/Umbraco.Tests.csproj +++ b/src/Umbraco.Tests/Umbraco.Tests.csproj @@ -262,6 +262,7 @@ +