From 99f7285e80175408772b9499c2ac600bd72d83ba Mon Sep 17 00:00:00 2001 From: Rob King Date: Tue, 2 Jun 2015 11:03:13 +0100 Subject: [PATCH 1/4] Added logging for invalid recipe files --- .../Recipes/Services/RecipeManagerTests.cs | 4 ++-- .../Controllers/AdminController.cs | 1 + .../Services/RecipeHarvester.cs | 18 +++++++++++++---- .../Orchard.Recipes/Services/RecipeParser.cs | 20 +++++++++++++++---- 4 files changed, 33 insertions(+), 10 deletions(-) diff --git a/src/Orchard.Tests.Modules/Recipes/Services/RecipeManagerTests.cs b/src/Orchard.Tests.Modules/Recipes/Services/RecipeManagerTests.cs index c5d3902bd..086e8abb3 100644 --- a/src/Orchard.Tests.Modules/Recipes/Services/RecipeManagerTests.cs +++ b/src/Orchard.Tests.Modules/Recipes/Services/RecipeManagerTests.cs @@ -136,8 +136,8 @@ namespace Orchard.Tests.Modules.Recipes.Services { } [Test] - public void ParseRecipeThrowsOnInvalidXml() { - Assert.Throws(typeof(XmlException), () => _recipeParser.ParseRecipe("")); + public void ParseRecipeReturnsNullOnInvalidXml() { + Assert.That(_recipeParser.ParseRecipe("") == null); } [Test] diff --git a/src/Orchard.Web/Modules/Orchard.Modules/Controllers/AdminController.cs b/src/Orchard.Web/Modules/Orchard.Modules/Controllers/AdminController.cs index 856fdcb12..fc55ff591 100644 --- a/src/Orchard.Web/Modules/Orchard.Modules/Controllers/AdminController.cs +++ b/src/Orchard.Web/Modules/Orchard.Modules/Controllers/AdminController.cs @@ -133,6 +133,7 @@ namespace Orchard.Modules.Controllers { Module = x, Recipes = _recipeHarvester.HarvestRecipes(x.Descriptor.Id).Where(recipe => !recipe.IsSetupRecipe).ToList() }) + .ToList() .Where(x => x.Recipes.Any()); } diff --git a/src/Orchard.Web/Modules/Orchard.Recipes/Services/RecipeHarvester.cs b/src/Orchard.Web/Modules/Orchard.Recipes/Services/RecipeHarvester.cs index 880fb0790..46f6b9750 100644 --- a/src/Orchard.Web/Modules/Orchard.Recipes/Services/RecipeHarvester.cs +++ b/src/Orchard.Web/Modules/Orchard.Recipes/Services/RecipeHarvester.cs @@ -35,10 +35,20 @@ namespace Orchard.Recipes.Services { if (extension != null) { var recipeLocation = Path.Combine(extension.Location, extensionId, "Recipes"); var recipeFiles = _webSiteFolder.ListFiles(recipeLocation, true); - recipes.AddRange( - from recipeFile in recipeFiles - where recipeFile.EndsWith(".recipe.xml", StringComparison.OrdinalIgnoreCase) - select _recipeParser.ParseRecipe(_webSiteFolder.ReadFile(recipeFile))); + + recipeFiles.Where(r => r.EndsWith(".recipe.xml", StringComparison.OrdinalIgnoreCase)).ToList().ForEach(r => + { + var recipe = _recipeParser.ParseRecipe(_webSiteFolder.ReadFile(r)); + + if (recipe == null) + { + Logger.Error(new Exception(string.Format("Invalid recipe file: {0}", r)), "Invalid recipe file: {0}", r); + } + else + { + recipes.Add(recipe); + } + }); } else { Logger.Error("Could not discover recipes because module '{0}' was not found.", extensionId); diff --git a/src/Orchard.Web/Modules/Orchard.Recipes/Services/RecipeParser.cs b/src/Orchard.Web/Modules/Orchard.Recipes/Services/RecipeParser.cs index e63fab583..c2f53b2e0 100644 --- a/src/Orchard.Web/Modules/Orchard.Recipes/Services/RecipeParser.cs +++ b/src/Orchard.Web/Modules/Orchard.Recipes/Services/RecipeParser.cs @@ -1,6 +1,5 @@ using System; using System.Collections.Generic; -using System.IO; using System.Xml; using System.Xml.Linq; using Orchard.Localization; @@ -21,10 +20,23 @@ namespace Orchard.Recipes.Services { var recipe = new Recipe(); try { + if (string.IsNullOrEmpty(recipeText)) + { + return null; + } + + XElement recipeTree; + + try + { + recipeTree = XElement.Parse(recipeText, LoadOptions.PreserveWhitespace); + } + catch + { + return null; + } + var recipeSteps = new List(); - TextReader textReader = new StringReader(recipeText); - var recipeTree = XElement.Load(textReader, LoadOptions.PreserveWhitespace); - textReader.Close(); foreach (var element in recipeTree.Elements()) { // Recipe mETaDaTA From 67de77fe08b9ce5e01dcad75c8b03a2c17e663e6 Mon Sep 17 00:00:00 2001 From: Rob King Date: Tue, 2 Jun 2015 14:43:54 +0100 Subject: [PATCH 2/4] Moved ToList after Where clause --- .../Modules/Orchard.Modules/Controllers/AdminController.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Orchard.Web/Modules/Orchard.Modules/Controllers/AdminController.cs b/src/Orchard.Web/Modules/Orchard.Modules/Controllers/AdminController.cs index fc55ff591..1cad73228 100644 --- a/src/Orchard.Web/Modules/Orchard.Modules/Controllers/AdminController.cs +++ b/src/Orchard.Web/Modules/Orchard.Modules/Controllers/AdminController.cs @@ -133,8 +133,8 @@ namespace Orchard.Modules.Controllers { Module = x, Recipes = _recipeHarvester.HarvestRecipes(x.Descriptor.Id).Where(recipe => !recipe.IsSetupRecipe).ToList() }) - .ToList() - .Where(x => x.Recipes.Any()); + .Where(x => x.Recipes.Any()) + .ToList(); } return View(viewModel); From 8421b2f7e859344b5fadfb2a11d265436c8c4729 Mon Sep 17 00:00:00 2001 From: Rob King Date: Thu, 4 Jun 2015 21:01:35 +0100 Subject: [PATCH 3/4] Changed to log error details in addition to file name --- .../Orchard.Recipes/Services/RecipeHarvester.cs | 15 ++++++--------- .../Orchard.Recipes/Services/RecipeParser.cs | 17 ++++------------- 2 files changed, 10 insertions(+), 22 deletions(-) diff --git a/src/Orchard.Web/Modules/Orchard.Recipes/Services/RecipeHarvester.cs b/src/Orchard.Web/Modules/Orchard.Recipes/Services/RecipeHarvester.cs index 46f6b9750..768261930 100644 --- a/src/Orchard.Web/Modules/Orchard.Recipes/Services/RecipeHarvester.cs +++ b/src/Orchard.Web/Modules/Orchard.Recipes/Services/RecipeHarvester.cs @@ -36,18 +36,15 @@ namespace Orchard.Recipes.Services { var recipeLocation = Path.Combine(extension.Location, extensionId, "Recipes"); var recipeFiles = _webSiteFolder.ListFiles(recipeLocation, true); - recipeFiles.Where(r => r.EndsWith(".recipe.xml", StringComparison.OrdinalIgnoreCase)).ToList().ForEach(r => - { - var recipe = _recipeParser.ParseRecipe(_webSiteFolder.ReadFile(r)); + recipeFiles.Where(r => r.EndsWith(".recipe.xml", StringComparison.OrdinalIgnoreCase)).ToList().ForEach(r => { - if (recipe == null) - { - Logger.Error(new Exception(string.Format("Invalid recipe file: {0}", r)), "Invalid recipe file: {0}", r); + try { + recipes.Add(_recipeParser.ParseRecipe(_webSiteFolder.ReadFile(r))); } - else - { - recipes.Add(recipe); + catch (Exception ex) { + Logger.Error(new Exception(string.Format("Invalid recipe file: {0}\nError: {1}", r, ex.Message)), "Invalid recipe file: {0}\nError: {1}", r, ex.Message); } + }); } else { diff --git a/src/Orchard.Web/Modules/Orchard.Recipes/Services/RecipeParser.cs b/src/Orchard.Web/Modules/Orchard.Recipes/Services/RecipeParser.cs index c2f53b2e0..697d1f95a 100644 --- a/src/Orchard.Web/Modules/Orchard.Recipes/Services/RecipeParser.cs +++ b/src/Orchard.Web/Modules/Orchard.Recipes/Services/RecipeParser.cs @@ -20,21 +20,12 @@ namespace Orchard.Recipes.Services { var recipe = new Recipe(); try { - if (string.IsNullOrEmpty(recipeText)) - { - return null; + + if (string.IsNullOrEmpty(recipeText)) { + throw new Exception("Recipe is empty"); } - XElement recipeTree; - - try - { - recipeTree = XElement.Parse(recipeText, LoadOptions.PreserveWhitespace); - } - catch - { - return null; - } + XElement recipeTree = XElement.Parse(recipeText, LoadOptions.PreserveWhitespace); var recipeSteps = new List(); From 8f281ef9d933f0dcd60485a0fb0d2d2a959824d0 Mon Sep 17 00:00:00 2001 From: Rob King Date: Thu, 4 Jun 2015 21:03:23 +0100 Subject: [PATCH 4/4] Reverted unit test --- .../Recipes/Services/RecipeManagerTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Orchard.Tests.Modules/Recipes/Services/RecipeManagerTests.cs b/src/Orchard.Tests.Modules/Recipes/Services/RecipeManagerTests.cs index 086e8abb3..c5d3902bd 100644 --- a/src/Orchard.Tests.Modules/Recipes/Services/RecipeManagerTests.cs +++ b/src/Orchard.Tests.Modules/Recipes/Services/RecipeManagerTests.cs @@ -136,8 +136,8 @@ namespace Orchard.Tests.Modules.Recipes.Services { } [Test] - public void ParseRecipeReturnsNullOnInvalidXml() { - Assert.That(_recipeParser.ParseRecipe("") == null); + public void ParseRecipeThrowsOnInvalidXml() { + Assert.Throws(typeof(XmlException), () => _recipeParser.ParseRecipe("")); } [Test]