From 18d1e80189b41b1de4255ab53c063ca79741863b Mon Sep 17 00:00:00 2001 From: Andre Rodrigues Date: Wed, 8 Dec 2010 18:18:52 -0800 Subject: [PATCH] #16849: Replacing strings and console.writelines by exception throwing. --HG-- branch : dev --- .../Commands/CodeGenerationCommandsTests.cs | 6 +-- .../Orchard.Blogs/Commands/BlogCommands.cs | 14 +++--- .../Commands/CodeGenerationCommands.cs | 43 +++++++------------ .../Commands/ProfilingCommands.cs | 2 +- .../Commands/IndexingCommands.cs | 12 +++--- .../Commands/PackagingCommands.cs | 5 +-- .../Orchard.Users/Commands/UserCommands.cs | 17 ++++---- .../Controllers/AccountController.cs | 5 +-- .../Controllers/AdminController.cs | 10 ++--- .../Orchard.Users/Services/IUserService.cs | 4 +- .../Orchard.Users/Services/UserService.cs | 12 +++--- src/Orchard/Commands/CommandHostAgent.cs | 32 ++++++++++---- 12 files changed, 78 insertions(+), 84 deletions(-) diff --git a/src/Orchard.Tests.Modules/CodeGeneration/Commands/CodeGenerationCommandsTests.cs b/src/Orchard.Tests.Modules/CodeGeneration/Commands/CodeGenerationCommandsTests.cs index 8f32a906a..c9c4959e7 100644 --- a/src/Orchard.Tests.Modules/CodeGeneration/Commands/CodeGenerationCommandsTests.cs +++ b/src/Orchard.Tests.Modules/CodeGeneration/Commands/CodeGenerationCommandsTests.cs @@ -58,16 +58,14 @@ namespace Orchard.Tests.Modules.CodeGeneration.Commands { } [Test] + [ExpectedException(typeof(OrchardException))] public void CreateDataMigrationTestNonExistentFeature() { CodeGenerationCommands codeGenerationCommands = new CodeGenerationCommands(_extensionManager, _schemaCommandGenerator); TextWriter textWriterOutput = new StringWriter(); codeGenerationCommands.Context = new CommandContext { Output = textWriterOutput }; - bool result = codeGenerationCommands.CreateDataMigration("feature"); - - Assert.That(result, Is.False); - Assert.That(textWriterOutput.ToString(), Is.EqualTo("Creating Data Migration for feature\r\nCreating data migration failed: target Feature feature could not be found.\r\n")); + codeGenerationCommands.CreateDataMigration("feature"); } } } \ No newline at end of file diff --git a/src/Orchard.Web/Modules/Orchard.Blogs/Commands/BlogCommands.cs b/src/Orchard.Web/Modules/Orchard.Blogs/Commands/BlogCommands.cs index 0940393d8..8164f42c9 100644 --- a/src/Orchard.Web/Modules/Orchard.Blogs/Commands/BlogCommands.cs +++ b/src/Orchard.Web/Modules/Orchard.Blogs/Commands/BlogCommands.cs @@ -53,12 +53,11 @@ namespace Orchard.Blogs.Commands { var owner = _membershipService.GetUser(Owner); if ( owner == null ) { - Context.Output.WriteLine(); - return T("Invalid username: {0}", Owner).Text; + throw new OrchardException(T("Invalid username: {0}", Owner)); } if(!IsSlugValid(Slug)) { - return "Invalid Slug provided. Blog creation failed."; + throw new OrchardException(T("Invalid Slug provided. Blog creation failed.")); } var blog = _contentManager.New("Blog"); @@ -83,8 +82,7 @@ namespace Orchard.Blogs.Commands { var owner = _membershipService.GetUser(Owner); if(owner == null) { - Context.Output.WriteLine(); - return T("Invalid username: {0}", Owner).Text; + throw new OrchardException(T("Invalid username: {0}", Owner)); } XDocument doc; @@ -95,14 +93,13 @@ namespace Orchard.Blogs.Commands { Context.Output.WriteLine("Found {0} items", doc.Descendants("item").Count()); } catch ( Exception ex ) { - Context.Output.WriteLine(T("An error occured while loading the file: " + ex.Message)); - return "Import terminated."; + throw new OrchardException(T("An error occured while loading the file: {0}", ex.Message)); } var blog = _blogService.Get(Slug); if ( blog == null ) { - return "Blog not found at specified slug: " + Slug; + throw new OrchardException(T("Blog not found at specified slug: {0}", Slug)); } foreach ( var item in doc.Descendants("item") ) { @@ -122,7 +119,6 @@ namespace Orchard.Blogs.Commands { } } - return "Import feed completed."; } diff --git a/src/Orchard.Web/Modules/Orchard.CodeGeneration/Commands/CodeGenerationCommands.cs b/src/Orchard.Web/Modules/Orchard.CodeGeneration/Commands/CodeGenerationCommands.cs index 2a43f4c05..b0c561845 100644 --- a/src/Orchard.Web/Modules/Orchard.CodeGeneration/Commands/CodeGenerationCommands.cs +++ b/src/Orchard.Web/Modules/Orchard.CodeGeneration/Commands/CodeGenerationCommands.cs @@ -48,15 +48,14 @@ namespace Orchard.CodeGeneration.Commands { [CommandHelp("codegen datamigration \r\n\t" + "Create a new Data Migration class")] [CommandName("codegen datamigration")] - public bool CreateDataMigration(string featureName) { + public void CreateDataMigration(string featureName) { Context.Output.WriteLine(T("Creating Data Migration for {0}", featureName)); ExtensionDescriptor extensionDescriptor = _extensionManager.AvailableExtensions().FirstOrDefault(extension => DefaultExtensionTypes.IsModule(extension.ExtensionType) && extension.Features.Any(feature => String.Equals(feature.Id, featureName, StringComparison.OrdinalIgnoreCase))); if (extensionDescriptor == null) { - Context.Output.WriteLine(T("Creating data migration failed: target Feature {0} could not be found.", featureName)); - return false; + throw new OrchardException(T("Creating data migration failed: target Feature {0} could not be found.", featureName)); } string dataMigrationFolderPath = HostingEnvironment.MapPath("~/Modules/" + extensionDescriptor.Id + "/"); @@ -69,8 +68,7 @@ namespace Orchard.CodeGeneration.Commands { } if (File.Exists(dataMigrationFilePath)) { - Context.Output.WriteLine(T("Data migration already exists in target Module {0}.", extensionDescriptor.Id)); - return false; + throw new OrchardException(T("Data migration already exists in target Module {0}.", extensionDescriptor.Id)); } List commands = _schemaCommandGenerator.GetCreateFeatureCommands(featureName, false).ToList(); @@ -104,25 +102,20 @@ namespace Orchard.CodeGeneration.Commands { File.WriteAllText(moduleCsProjPath, projectFileText); TouchSolution(Context.Output); Context.Output.WriteLine(T("Data migration created successfully in Module {0}", extensionDescriptor.Id)); - - return true; } [CommandHelp("codegen module [/IncludeInSolution:true|false]\r\n\t" + "Create a new Orchard module")] [CommandName("codegen module")] [OrchardSwitches("IncludeInSolution")] - public bool CreateModule(string moduleName) { + public void CreateModule(string moduleName) { Context.Output.WriteLine(T("Creating Module {0}", moduleName)); if ( _extensionManager.AvailableExtensions().Any(extension => String.Equals(moduleName, extension.Name, StringComparison.OrdinalIgnoreCase)) ) { - Context.Output.WriteLine(T("Creating Module {0} failed: a module of the same name already exists", moduleName)); - return false; + throw new OrchardException(T("Creating Module {0} failed: a module of the same name already exists", moduleName)); } IntegrateModule(moduleName); Context.Output.WriteLine(T("Module {0} created successfully", moduleName)); - - return true; } [CommandName("codegen theme")] @@ -131,20 +124,18 @@ namespace Orchard.CodeGeneration.Commands { public void CreateTheme(string themeName) { Context.Output.WriteLine(T("Creating Theme {0}", themeName)); if (_extensionManager.AvailableExtensions().Any(extension => String.Equals(themeName, extension.Id, StringComparison.OrdinalIgnoreCase))) { - Context.Output.WriteLine(T("Creating Theme {0} failed: an extention of the same name already exists", themeName)); + throw new OrchardException(T("Creating Theme {0} failed: an extention of the same name already exists", themeName)); } - else { - if (!string.IsNullOrEmpty(BasedOn)) { - if (!_extensionManager.AvailableExtensions().Any(extension => - string.Equals(extension.ExtensionType, DefaultExtensionTypes.Theme, StringComparison.OrdinalIgnoreCase) && - string.Equals(BasedOn, extension.Id, StringComparison.OrdinalIgnoreCase))) { - Context.Output.WriteLine(T("Creating Theme {0} failed: base theme named {1} was not found.", themeName, BasedOn)); - return; - } + + if (!string.IsNullOrEmpty(BasedOn)) { + if (!_extensionManager.AvailableExtensions().Any(extension => + string.Equals(extension.ExtensionType, DefaultExtensionTypes.Theme, StringComparison.OrdinalIgnoreCase) && + string.Equals(BasedOn, extension.Id, StringComparison.OrdinalIgnoreCase))) { + throw new OrchardException(T("Creating Theme {0} failed: base theme named {1} was not found.", themeName, BasedOn)); } - IntegrateTheme(themeName, BasedOn); - Context.Output.WriteLine(T("Theme {0} created successfully", themeName)); } + IntegrateTheme(themeName, BasedOn); + Context.Output.WriteLine(T("Theme {0} created successfully", themeName)); } [CommandHelp("codegen controller \r\n\t" + "Create a new Orchard controller in a module")] @@ -156,8 +147,7 @@ namespace Orchard.CodeGeneration.Commands { string.Equals(moduleName, extension.Name, StringComparison.OrdinalIgnoreCase)); if (extensionDescriptor == null) { - Context.Output.WriteLine(T("Creating Controller {0} failed: target Module {1} could not be found.", controllerName, moduleName)); - return; + throw new OrchardException(T("Creating Controller {0} failed: target Module {1} could not be found.", controllerName, moduleName)); } string moduleControllersPath = HostingEnvironment.MapPath("~/Modules/" + extensionDescriptor.Id + "/Controllers/"); @@ -169,8 +159,7 @@ namespace Orchard.CodeGeneration.Commands { Directory.CreateDirectory(moduleControllersPath); } if (File.Exists(controllerPath)) { - Context.Output.WriteLine(T("Controller {0} already exists in target Module {1}.", controllerName, moduleName)); - return; + throw new OrchardException(T("Controller {0} already exists in target Module {1}.", controllerName, moduleName)); } string controllerText = File.ReadAllText(templatesPath + "Controller.txt"); diff --git a/src/Orchard.Web/Modules/Orchard.Experimental/Commands/ProfilingCommands.cs b/src/Orchard.Web/Modules/Orchard.Experimental/Commands/ProfilingCommands.cs index f2729e26f..abd61336d 100644 --- a/src/Orchard.Web/Modules/Orchard.Experimental/Commands/ProfilingCommands.cs +++ b/src/Orchard.Web/Modules/Orchard.Experimental/Commands/ProfilingCommands.cs @@ -69,7 +69,7 @@ namespace Orchard.Experimental.Commands { for (int i = 0; i < 1000; i++) { var user = _membershipService.CreateUser(new CreateUserParams("user" + i, "1234567", "user" + i + "@orchardproject.net", null, null, true)); if (user == null) - return "The authentication provider returned an error"; + throw new OrchardException(T("The authentication provider returned an error")); } return "Success"; diff --git a/src/Orchard.Web/Modules/Orchard.Indexing/Commands/IndexingCommands.cs b/src/Orchard.Web/Modules/Orchard.Indexing/Commands/IndexingCommands.cs index d3295b516..e87d5e983 100644 --- a/src/Orchard.Web/Modules/Orchard.Indexing/Commands/IndexingCommands.cs +++ b/src/Orchard.Web/Modules/Orchard.Indexing/Commands/IndexingCommands.cs @@ -38,7 +38,7 @@ namespace Orchard.Indexing.Commands { [OrchardSwitches("IndexName")] public string Update() { if ( !_indexManager.HasIndexProvider() ) { - return "No index available"; + throw new OrchardException(T("No index available")); } var indexName = String.IsNullOrWhiteSpace(IndexName) ? SearchIndexName : IndexName; @@ -54,7 +54,7 @@ namespace Orchard.Indexing.Commands { [OrchardSwitches("IndexName")] public string Rebuild() { if ( !_indexManager.HasIndexProvider() ) { - return "No index available"; + throw new OrchardException(T("No index available")); } var indexName = String.IsNullOrWhiteSpace(IndexName) ? SearchIndexName : IndexName; @@ -71,7 +71,7 @@ namespace Orchard.Indexing.Commands { [OrchardSwitches("Query,IndexName")] public string Search() { if ( !_indexManager.HasIndexProvider() ) { - return "No index available"; + throw new OrchardException(T("No index available")); } var indexName = String.IsNullOrWhiteSpace(IndexName) ? SearchIndexName : IndexName; var searchBuilder = _indexManager.GetSearchIndexProvider().CreateSearchBuilder(indexName); @@ -99,7 +99,7 @@ namespace Orchard.Indexing.Commands { [OrchardSwitches("IndexName")] public string Stats() { if ( !_indexManager.HasIndexProvider() ) { - return "No index available"; + throw new OrchardException(T("No index available")); } var indexName = String.IsNullOrWhiteSpace(IndexName) ? SearchIndexName : IndexName; Context.Output.WriteLine("Number of indexed documents: {0}", _indexManager.GetSearchIndexProvider().NumDocs(indexName)); @@ -112,7 +112,7 @@ namespace Orchard.Indexing.Commands { public string Refresh() { int contentItemId; if ( !int.TryParse(ContentItemId, out contentItemId) ) { - return "Invalid content item id. Not an integer."; + throw new OrchardException(T("Invalid content item id. Not an integer.")); } var contentItem = _contentManager.Get(contentItemId); @@ -127,7 +127,7 @@ namespace Orchard.Indexing.Commands { public string Delete() { int contenItemId; if(!int.TryParse(ContentItemId, out contenItemId)) { - return "Invalid content item id. Not an integer."; + throw new OrchardException(T("Invalid content item id. Not an integer.")); } var contentItem = _contentManager.Get(contenItemId); diff --git a/src/Orchard.Web/Modules/Orchard.Packaging/Commands/PackagingCommands.cs b/src/Orchard.Web/Modules/Orchard.Packaging/Commands/PackagingCommands.cs index 118458cca..1fb15ead8 100644 --- a/src/Orchard.Web/Modules/Orchard.Packaging/Commands/PackagingCommands.cs +++ b/src/Orchard.Web/Modules/Orchard.Packaging/Commands/PackagingCommands.cs @@ -28,8 +28,7 @@ namespace Orchard.Packaging.Commands { public void CreatePackage(string extensionName, string path) { var packageData = _packageManager.Harvest(extensionName); if (packageData == null) { - Context.Output.WriteLine(T("Module or Theme \"{0}\" does not exist in this Orchard installation.", extensionName)); - return; + throw new OrchardException(T("Module or Theme \"{0}\" does not exist in this Orchard installation.", extensionName)); } // append "Orchard.[ExtensionType]" to prevent conflicts with other packages (e.g, TinyMce, jQuery, ...) @@ -79,7 +78,7 @@ namespace Orchard.Packaging.Commands { } catch(Exception e) { // Exceptions area thrown by NuGet as error messages - Context.Output.WriteLine(HttpUtility.HtmlDecode(T("Could not unintall the package: {0}", e.Message).Text)); + throw new OrchardException(T(HttpUtility.HtmlDecode(T("Could not unintall the package: {0}", e.Message).Text))); } } diff --git a/src/Orchard.Web/Modules/Orchard.Users/Commands/UserCommands.cs b/src/Orchard.Web/Modules/Orchard.Users/Commands/UserCommands.cs index 61103becc..1337b6924 100644 --- a/src/Orchard.Web/Modules/Orchard.Users/Commands/UserCommands.cs +++ b/src/Orchard.Web/Modules/Orchard.Users/Commands/UserCommands.cs @@ -30,19 +30,20 @@ namespace Orchard.Users.Commands { [CommandHelp("user create /UserName: /Password: /Email:\r\n\t" + "Creates a new User")] [OrchardSwitches("UserName,Password,Email")] public string Create() { - string userUnicityMessage = _userService.VerifyUserUnicity(UserName, Email); - if (userUnicityMessage != null) { - return userUnicityMessage; + if (!_userService.VerifyUserUnicity(UserName, Email)) { + throw new OrchardException(T("User with that username and/or email already exists.")); } + if (Password == null || Password.Length < MinPasswordLength) { - return T("You must specify a password of {0} or more characters.", MinPasswordLength).ToString(); + throw new OrchardException(T("You must specify a password of {0} or more characters.", MinPasswordLength)); } var user = _membershipService.CreateUser(new CreateUserParams(UserName, Password, Email, null, null, true)); - if (user != null) - return T("User created successfully").ToString(); - - return T("The authentication provider returned an error").ToString(); + if (user == null) { + throw new OrchardException(T("The authentication provider returned an error")); + } + + return T("User created successfully").ToString(); } int MinPasswordLength { diff --git a/src/Orchard.Web/Modules/Orchard.Users/Controllers/AccountController.cs b/src/Orchard.Web/Modules/Orchard.Users/Controllers/AccountController.cs index a638e1118..bc53029ca 100644 --- a/src/Orchard.Web/Modules/Orchard.Users/Controllers/AccountController.cs +++ b/src/Orchard.Web/Modules/Orchard.Users/Controllers/AccountController.cs @@ -333,9 +333,8 @@ namespace Orchard.Users.Controllers { if (!validate) return false; - string userUnicityMessage = _userService.VerifyUserUnicity(userName, email); - if (userUnicityMessage != null) { - ModelState.AddModelError("userExists", T(userUnicityMessage)); + if (!_userService.VerifyUserUnicity(userName, email)) { + ModelState.AddModelError("userExists", T("User with that username and/or email already exists.")); } if (password == null || password.Length < MinPasswordLength) { ModelState.AddModelError("password", T("You must specify a password of {0} or more characters.", MinPasswordLength)); diff --git a/src/Orchard.Web/Modules/Orchard.Users/Controllers/AdminController.cs b/src/Orchard.Web/Modules/Orchard.Users/Controllers/AdminController.cs index 20f91fa65..9da562383 100644 --- a/src/Orchard.Web/Modules/Orchard.Users/Controllers/AdminController.cs +++ b/src/Orchard.Web/Modules/Orchard.Users/Controllers/AdminController.cs @@ -77,9 +77,8 @@ namespace Orchard.Users.Controllers { return new HttpUnauthorizedResult(); if (!string.IsNullOrEmpty(createModel.UserName)) { - string userExistsMessage = _userService.VerifyUserUnicity(createModel.UserName, createModel.Email); - if (userExistsMessage != null) { - AddModelError("NotUniqueUserName", T(userExistsMessage)); + if (!_userService.VerifyUserUnicity(createModel.UserName, createModel.Email)) { + AddModelError("NotUniqueUserName", T("User with that username and/or email already exists.")); } } @@ -139,9 +138,8 @@ namespace Orchard.Users.Controllers { var editModel = new UserEditViewModel {User = user}; if (TryUpdateModel(editModel)) { - string userExistsMessage = _userService.VerifyUserUnicity(id, editModel.UserName, editModel.Email); - if (userExistsMessage != null) { - AddModelError("NotUniqueUserName", T(userExistsMessage)); + if (!_userService.VerifyUserUnicity(id, editModel.UserName, editModel.Email)) { + AddModelError("NotUniqueUserName", T("User with that username and/or email already exists.")); } else { // also update the Super user if this is the renamed account diff --git a/src/Orchard.Web/Modules/Orchard.Users/Services/IUserService.cs b/src/Orchard.Web/Modules/Orchard.Users/Services/IUserService.cs index acd7a211f..b42b78af3 100644 --- a/src/Orchard.Web/Modules/Orchard.Users/Services/IUserService.cs +++ b/src/Orchard.Web/Modules/Orchard.Users/Services/IUserService.cs @@ -2,8 +2,8 @@ using Orchard.Security; using System; namespace Orchard.Users.Services { public interface IUserService : IDependency { - string VerifyUserUnicity(string userName, string email); - string VerifyUserUnicity(int id, string userName, string email); + bool VerifyUserUnicity(string userName, string email); + bool VerifyUserUnicity(int id, string userName, string email); void SendChallengeEmail(IUser user, Func createUrl); IUser ValidateChallenge(string challengeToken); diff --git a/src/Orchard.Web/Modules/Orchard.Users/Services/UserService.cs b/src/Orchard.Web/Modules/Orchard.Users/Services/UserService.cs index 1355e48e0..6b84fb063 100644 --- a/src/Orchard.Web/Modules/Orchard.Users/Services/UserService.cs +++ b/src/Orchard.Web/Modules/Orchard.Users/Services/UserService.cs @@ -36,7 +36,7 @@ namespace Orchard.Users.Services { public ILogger Logger { get; set; } - public string VerifyUserUnicity(string userName, string email) { + public bool VerifyUserUnicity(string userName, string email) { string normalizedUserName = userName.ToLower(); if (_contentManager.Query() @@ -44,13 +44,13 @@ namespace Orchard.Users.Services { user.NormalizedUserName == normalizedUserName || user.Email == email) .List().Any()) { - return "User with that username and/or email already exists."; + return false; } - return null; + return true; } - public string VerifyUserUnicity(int id, string userName, string email) { + public bool VerifyUserUnicity(int id, string userName, string email) { string normalizedUserName = userName.ToLower(); if (_contentManager.Query() @@ -58,10 +58,10 @@ namespace Orchard.Users.Services { user.NormalizedUserName == normalizedUserName || user.Email == email) .List().Any(user => user.Id != id)) { - return "User with that username and/or email already exists."; + return false; } - return null; + return true; } public string CreateNonce(IUser user, TimeSpan delay) { diff --git a/src/Orchard/Commands/CommandHostAgent.cs b/src/Orchard/Commands/CommandHostAgent.cs index 46e7e4254..3c54ed1f4 100644 --- a/src/Orchard/Commands/CommandHostAgent.cs +++ b/src/Orchard/Commands/CommandHostAgent.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.IO; using System.Linq; +using System.Reflection; using System.Web.Mvc; using System.Web.Routing; using Autofac; @@ -99,6 +100,12 @@ namespace Orchard.Commands { return CommandReturnCodes.Retry; } catch (Exception e) { + if (e is TargetInvocationException && + e.InnerException != null) { + // If this is an exception coming from reflection and there is an innerexception which is the actual one, redirect + e = e.InnerException; + } + OutputException(output, T("Error executing command \"{0}\"", string.Join(" ", args)), e); return CommandReturnCodes.Fail; } @@ -138,7 +145,6 @@ namespace Orchard.Commands { // Display header output.WriteLine(); output.WriteLine(T("{0}", title)); - output.WriteLine(T("--------------------------------------------------------------------------------")); // Push exceptions in a stack so we display from inner most to outer most var errors = new Stack(); @@ -148,18 +154,26 @@ namespace Orchard.Commands { // Display inner most exception details exception = errors.Peek(); + output.WriteLine(T("--------------------------------------------------------------------------------")); + output.WriteLine(); output.WriteLine(T("{0}", exception.Message)); output.WriteLine(); - output.WriteLine(T("Exception Details: {0}: {1}", exception.GetType().FullName, exception.Message)); - output.WriteLine(); - output.WriteLine(T("Stack Trace:")); - output.WriteLine(); - // Display exceptions from inner most to outer most - foreach(var error in errors) { - output.WriteLine(T("[{0}: {1}]", error.GetType().Name, error.Message)); - output.WriteLine(T("{0}", error.StackTrace)); + if (!((exception is OrchardException || + exception is OrchardCoreException) && + exception.InnerException == null)) { + + output.WriteLine(T("Exception Details: {0}: {1}", exception.GetType().FullName, exception.Message)); output.WriteLine(); + output.WriteLine(T("Stack Trace:")); + output.WriteLine(); + + // Display exceptions from inner most to outer most + foreach (var error in errors) { + output.WriteLine(T("[{0}: {1}]", error.GetType().Name, error.Message)); + output.WriteLine(T("{0}", error.StackTrace)); + output.WriteLine(); + } } // Display footer