(Part I) Removing the unhandled "catch" from module controllers. If there are "expected" exceptions, move them closer to the APIs where they occur.

--HG--
branch : 1.x
extra : transplant_source : %C9q%EC%E3%C3%E3%98QL%27%2B%D68%A9x%DD%40J%93A

src/Orchard.Web/Modules/Orchard.Comments/Controllers/AdminController.cs

@@ -50,7 +50,6 @@ namespace Orchard.Comments.Controllers {
 
             // Filtering
             IContentQuery<CommentPart, CommentPartRecord> comments;
-            try {
             switch (options.Filter) {
                 case CommentIndexFilter.All:
                     comments = _commentService.GetComments();
@@ -80,12 +79,8 @@ namespace Orchard.Comments.Controllers {
                 Options = options,
                 Pager = pagerShape
             };
-                return View(model);
-            } catch (Exception exception) {
-                this.Error(exception, T("Listing comments failed: {0}", exception.Message), Logger, Services.Notifier);
 
-                return View(new CommentsIndexViewModel());
+            return View(model);
-            }
         }
 
         [HttpPost]
@@ -94,7 +89,6 @@ namespace Orchard.Comments.Controllers {
             var viewModel = new CommentsIndexViewModel { Comments = new List<CommentEntry>(), Options = new CommentIndexOptions() };
             UpdateModel(viewModel);
 
-            try {
             IEnumerable<CommentEntry> checkedEntries = viewModel.Comments.Where(c => c.IsChecked);
             switch (viewModel.Options.BulkAction) {
                 case CommentIndexBulkAction.None:
@@ -135,11 +129,6 @@ namespace Orchard.Comments.Controllers {
                 default:
                     throw new ArgumentOutOfRangeException();
             }
-            } catch (Exception exception) {
-                this.Error(exception, T("Editing comments failed: {0}", exception.Message), Logger, Services.Notifier);
-
-                return RedirectToAction("Index", "Admin", new { options = viewModel.Options });
-            }
 
             return RedirectToAction("Index");
         }
@@ -151,7 +140,6 @@ namespace Orchard.Comments.Controllers {
 
             // Filtering
             IContentQuery<CommentPart, CommentPartRecord> comments;
-            try {
             switch (options.Filter) {
                 case CommentDetailsFilter.All:
                     comments = _commentService.GetCommentsForCommentedContent(id);
@@ -177,11 +165,6 @@ namespace Orchard.Comments.Controllers {
                 CommentsClosedOnItem = _commentService.CommentsDisabledForCommentedContent(id),
             };
             return View(model);
-            } catch (Exception exception) {
-                this.Error(exception, T("Listing comments failed: {0}", exception.Message), Logger, Services.Notifier);
-
-                return RedirectToAction("Index");
-            }
         }
 
         [HttpPost]
@@ -190,7 +173,6 @@ namespace Orchard.Comments.Controllers {
             var viewModel = new CommentsDetailsViewModel { Comments = new List<CommentEntry>(), Options = new CommentDetailsOptions() };
             UpdateModel(viewModel);
 
-            try {
             IEnumerable<CommentEntry> checkedEntries = viewModel.Comments.Where(c => c.IsChecked);
             switch (viewModel.Options.BulkAction) {
                 case CommentDetailsBulkAction.None:
@@ -231,44 +213,34 @@ namespace Orchard.Comments.Controllers {
                 default:
                     throw new ArgumentOutOfRangeException();
             }
-            } catch (Exception exception) {
-                this.Error(exception, T("Editing comments failed: {0}", exception.Message), Logger, Services.Notifier);
-
-                return Details(viewModel.CommentedItemId, viewModel.Options);
-            }
 
             return RedirectToAction("Index");
         }
 
         [HttpPost]
         public ActionResult Disable(int commentedItemId, string returnUrl) {
-            try {
             if (!Services.Authorizer.Authorize(Permissions.ManageComments, T("Couldn't disable comments")))
                 return new HttpUnauthorizedResult();
 
             _commentService.DisableCommentsForCommentedContent(commentedItemId);
-            } catch (Exception exception) {
-                this.Error(exception, T("Disabling Comments failed: {0}", exception.Message), Logger, Services.Notifier);
-            }
             return this.RedirectLocal(returnUrl, () => RedirectToAction("Index"));
         }
 
         [HttpPost]
         public ActionResult Enable(int commentedItemId, string returnUrl) {
-            try {
             if (!Services.Authorizer.Authorize(Permissions.ManageComments, T("Couldn't enable comments")))
                 return new HttpUnauthorizedResult();
                 
             _commentService.EnableCommentsForCommentedContent(commentedItemId);
-            } catch (Exception exception) {
+
-                this.Error(exception, T("Enabling Comments failed: {0}", exception.Message), Logger, Services.Notifier);
-            }
             return this.RedirectLocal(returnUrl, () => RedirectToAction("Index"));
         }
 
         public ActionResult Edit(int id) {
-            try {
             CommentPart commentPart = _commentService.GetComment(id);
+            if (commentPart == null)
+                return new HttpNotFoundResult();
+
             var viewModel = new CommentsEditViewModel {
                 CommentText = commentPart.Record.CommentText,
                 Email = commentPart.Record.Email,
@@ -278,97 +250,77 @@ namespace Orchard.Comments.Controllers {
                 Status = commentPart.Record.Status,
             };
             return View(viewModel);
-
-            } catch (Exception exception) {
-                this.Error(exception, T("Editing comment failed: {0}", exception.Message), Logger, Services.Notifier);
-
-                return RedirectToAction("Index");
-            }
         }
 
         [HttpPost]
         public ActionResult Edit(FormCollection input) {
             var viewModel = new CommentsEditViewModel();
-            try {
             UpdateModel(viewModel);
             if (!Services.Authorizer.Authorize(Permissions.ManageComments, T("Couldn't edit comment")))
                 return new HttpUnauthorizedResult();
 
             _commentService.UpdateComment(viewModel.Id, viewModel.Name, viewModel.Email, viewModel.SiteName, viewModel.CommentText, viewModel.Status);
             return RedirectToAction("Index");
-            } catch (Exception exception) {
-                this.Error(exception, T("Editing comment failed: {0}", exception.Message), Logger, Services.Notifier);
-
-                return View(viewModel);
-            }
         }
 
         [HttpPost]
         public ActionResult Approve(int id, string returnUrl) {
-            try {
             if (!Services.Authorizer.Authorize(Permissions.ManageComments, T("Couldn't approve comment")))
                 return new HttpUnauthorizedResult();
 
-                int commentedOn = _commentService.GetComment(id).Record.CommentedOn;
+            var commentPart = _commentService.GetComment(id);
+            if (commentPart == null)
+                return new HttpNotFoundResult();
+
+            int commentedOn = commentPart.Record.CommentedOn;
             _commentService.ApproveComment(id);
 
             return this.RedirectLocal(returnUrl, () => RedirectToAction("Details", new { id = commentedOn }));
-            } catch (Exception exception) {
-                this.Error(exception, T("Approving comment failed: {0}", exception.Message), Logger, Services.Notifier);
-
-                return this.RedirectLocal(returnUrl, () => RedirectToAction("Index"));
-            }
         }
 
         [HttpPost]
         public ActionResult Unapprove(int id, string returnUrl) {
-            try {
             if (!Services.Authorizer.Authorize(Permissions.ManageComments, T("Couldn't unapprove comment")))
                 return new HttpUnauthorizedResult();
 
-                int commentedOn = _commentService.GetComment(id).Record.CommentedOn;
+            var commentPart = _commentService.GetComment(id);
+            if (commentPart == null)
+                return new HttpNotFoundResult();
+
+            int commentedOn = commentPart.Record.CommentedOn;
             _commentService.UnapproveComment(id);
 
             return this.RedirectLocal(returnUrl, () => RedirectToAction("Details", new { id = commentedOn }));
-            } catch (Exception exception) {
-                this.Error(exception, T("Unapproving comment failed: {0}", exception.Message), Logger, Services.Notifier);
-
-                return this.RedirectLocal(returnUrl, () => RedirectToAction("Index"));
-            }
         }
 
         [HttpPost]
         public ActionResult MarkAsSpam(int id, string returnUrl) {
-            try {
             if (!Services.Authorizer.Authorize(Permissions.ManageComments, T("Couldn't mark comment as spam")))
                 return new HttpUnauthorizedResult();
 
-                int commentedOn = _commentService.GetComment(id).Record.CommentedOn;
+            var commentPart = _commentService.GetComment(id);
+            if (commentPart == null)
+                return new HttpNotFoundResult();
+
+            int commentedOn = commentPart.Record.CommentedOn;
             _commentService.MarkCommentAsSpam(id);
 
             return this.RedirectLocal(returnUrl, () => RedirectToAction("Details", new { id = commentedOn }));
-            } catch (Exception exception) {
-                this.Error(exception, T("Marking comment as spam failed: {0}", exception.Message), Logger, Services.Notifier);
-
-                return this.RedirectLocal(returnUrl, () => RedirectToAction("Index"));
-            }
         }
 
         [HttpPost]
         public ActionResult Delete(int id, string returnUrl) {
-            try {
             if (!Services.Authorizer.Authorize(Permissions.ManageComments, T("Couldn't delete comment")))
                 return new HttpUnauthorizedResult();
 
-                int commentedOn = _commentService.GetComment(id).Record.CommentedOn;
+            var commentPart = _commentService.GetComment(id);
+            if (commentPart == null)
+                return new HttpNotFoundResult();
+
+            int commentedOn = commentPart.Record.CommentedOn;
             _commentService.DeleteComment(id);
 
             return this.RedirectLocal(returnUrl, () => RedirectToAction("Details", new { id = commentedOn }));
-            } catch (Exception exception) {
-                this.Error(exception, T("Deleting comment failed: {0}", exception.Message), Logger, Services.Notifier);
-
-                return this.RedirectLocal(returnUrl, () => RedirectToAction("Index"));
-            }
         }
 
         private CommentEntry CreateCommentEntry(CommentPartRecord commentPart) {

src/Orchard.Web/Modules/Orchard.Experimental/Controllers/CommandsController.cs

@@ -50,7 +50,6 @@ namespace Orchard.Experimental.Controllers {
             if (_shellSettings.Name != ShellSettings.DefaultName || !Services.Authorizer.Authorize(StandardPermissions.SiteOwner, T("Not authorized to use the web console")))
                 return new HttpUnauthorizedResult();
 
-            try {
             using (var writer = new StringWriter()) {
                 var commandLine = model.CommandLine.Trim();
                 CommandParameters parameters = GetCommandParameters(commandLine, writer);
@@ -62,11 +61,7 @@ namespace Orchard.Experimental.Controllers {
                     .ToArray();
                 model.Results = writer.ToString();
             }
-            } catch(Exception exception) {
-                this.Error(exception, T("Error executing command: {0}", exception.Message), Logger, Services.Notifier);
 
-                Services.TransactionManager.Cancel();
-            }
             return View(model);
         }
 

src/Orchard.Web/Modules/Orchard.ImportExport/Controllers/AdminController.cs

@@ -36,20 +36,18 @@ namespace Orchard.ImportExport.Controllers {
             if (!Services.Authorizer.Authorize(Permissions.Import, T("Not allowed to import.")))
                 return new HttpUnauthorizedResult();
 
-            try {
             if (String.IsNullOrEmpty(Request.Files["RecipeFile"].FileName)) {
-                    throw new ArgumentException(T("Please choose a recipe file to import.").Text);
+                ModelState.AddModelError("RecipeFile", T("Please choose a recipe file to import.").Text);
+                Services.Notifier.Error(T("Please choose a recipe file to import."));
             }
-                _importExportService.Import(new StreamReader(Request.Files["RecipeFile"].InputStream).ReadToEnd());
 
+            if (ModelState.IsValid) {
+                _importExportService.Import(new StreamReader(Request.Files["RecipeFile"].InputStream).ReadToEnd());
                 Services.Notifier.Information(T("Your recipe has been imported."));
+            }
+
             return RedirectToAction("Import");
         }
-            catch (Exception exception) {
-                Services.Notifier.Error(T("Import failed: {0}", exception.Message));
-                return View();
-            }
-        }
 
         public ActionResult Export() {
             var viewModel = new ExportViewModel { ContentTypes = new List<ContentTypeEntry>() };
@@ -66,7 +64,6 @@ namespace Orchard.ImportExport.Controllers {
 
             var viewModel = new ExportViewModel { ContentTypes = new List<ContentTypeEntry>() };
 
-            try {
             UpdateModel(viewModel);
             var contentTypesToExport = viewModel.ContentTypes.Where(c => c.IsChecked).Select(c => c.ContentTypeName);
             var exportOptions = new ExportOptions { ExportMetadata = viewModel.Metadata, ExportSiteSettings = viewModel.SiteSettings };
@@ -77,10 +74,5 @@ namespace Orchard.ImportExport.Controllers {
             var exportFilePath = _importExportService.Export(contentTypesToExport, exportOptions);
             return File(exportFilePath, "text/xml", "export.xml");
         }
-            catch (Exception exception) {
-                Services.Notifier.Error(T("Export failed: {0}", exception.Message));
-                return View(viewModel);
-            }
-        }
     }
 }

src/Orchard.Web/Modules/Orchard.Media/Controllers/AdminController.cs

@@ -37,23 +37,23 @@ namespace Orchard.Media.Controllers {
 
         [HttpPost]
         public ActionResult Index(FormCollection input) {
-            try {
             foreach (string key in input.Keys) {
                 if (key.StartsWith("Checkbox.") && input[key] == "true") {
                     string folderName = key.Substring("Checkbox.".Length);
                     if (!Services.Authorizer.Authorize(Permissions.ManageMedia, T("Couldn't delete media folder")))
                         return new HttpUnauthorizedResult();
+                        
+                    try {
                         _mediaService.DeleteFolder(folderName);
                     }
+                    catch (Exception exception) {
+                        Services.Notifier.Error(T("Deleting Folder failed: {0}", exception.Message));
+                        return View();
+                    }
+                }
             }
             return RedirectToAction("Index");
         }
-            catch (Exception exception) {
-                this.Error(exception, T("Deleting Folder failed: {0}", exception.Message), Logger, Services.Notifier);
-
-                return View();
-            }
-        }
 
         public ActionResult Create(string mediaPath) {
             return View(new MediaFolderCreateViewModel { MediaPath = mediaPath });
@@ -65,19 +65,18 @@ namespace Orchard.Media.Controllers {
                 return new HttpUnauthorizedResult();
 
             var viewModel = new MediaFolderCreateViewModel();
-            try {
             UpdateModel(viewModel);
 
+            try {
                 _mediaService.CreateFolder(viewModel.MediaPath, viewModel.Name);
-
                 Services.Notifier.Information(T("Media folder created"));
-                return RedirectToAction("Index");
             }
-            catch (Exception exception) {
+            catch(ArgumentException argumentException) {
-                this.Error(exception, T("Creating Folder failed: {0}", exception.Message), Logger, Services.Notifier);
+                Services.Notifier.Error(T("Creating Folder failed: {0}", argumentException.Message));
-
                 return View(viewModel);
             }
+
+            return RedirectToAction("Index");
         }
 
         public ActionResult Edit(string name, string mediaPath) {
@@ -87,41 +86,45 @@ namespace Orchard.Media.Controllers {
                 var model = new MediaFolderEditViewModel { FolderName = name, MediaFiles = mediaFiles, MediaFolders = mediaFolders, MediaPath = mediaPath };
                 return View(model);
             }
-            catch (Exception exception) {
+            catch(ArgumentException exception) {
-                this.Error(exception, T("Editing failed: {0}", exception.Message), Logger, Services.Notifier);
+                Services.Notifier.Error(T("Editing failed: {0}", exception.Message));
-
                 return RedirectToAction("Index");
             }
         }
 
         [HttpPost]
         public ActionResult Edit(FormCollection input) {
-            try {
             foreach (string key in input.Keys) {
                 if (key.StartsWith("Checkbox.File.") && input[key] == "true") {
                     string fileName = key.Substring("Checkbox.File.".Length);
                     string folderName = input[fileName];
                     if (!Services.Authorizer.Authorize(Permissions.ManageMedia, T("Couldn't delete media file")))
                         return new HttpUnauthorizedResult();
-                        _mediaService.DeleteFile(folderName, fileName);
 
+                    try {
+                        _mediaService.DeleteFile(folderName, fileName);
                         Services.Notifier.Information(T("Media file deleted"));
                     }
+                    catch (ArgumentException argumentException) {
+                        Services.Notifier.Error(T("Deleting failed: {0}", argumentException.Message));
+                    } 
+                }
                 else if (key.StartsWith("Checkbox.Folder.") && input[key] == "true") {
                     string folderName = key.Substring("Checkbox.Folder.".Length);
                     string folderPath = input[folderName];
                     if (!Services.Authorizer.Authorize(Permissions.ManageMedia, T("Couldn't delete media folder")))
                         return new HttpUnauthorizedResult();
-                        _mediaService.DeleteFolder(folderPath);
 
+                    try {
+                        _mediaService.DeleteFolder(folderPath);
                         Services.Notifier.Information(T("Media folder deleted"));
                     }
+                    catch(ArgumentException argumentException) {
+                        Services.Notifier.Error(T("Deleting failed: {0}", argumentException.Message));
+                    }
+                }
             }
             return RedirectToAction("Index");
-            } catch (Exception exception) {
-                this.Error(exception, T("Deleting failed: {0}", exception.Message), Logger, Services.Notifier);
-                return RedirectToAction("Index");
-            }
         }
 
         public ActionResult EditProperties(string folderName, string mediaPath) {
@@ -136,18 +139,17 @@ namespace Orchard.Media.Controllers {
                 return new HttpUnauthorizedResult();
 
             var viewModel = new MediaFolderEditPropertiesViewModel();
-            try {
             UpdateModel(viewModel);
-
+            try {
                 _mediaService.DeleteFolder(viewModel.MediaPath);
-
                 Services.Notifier.Information(T("Media folder deleted"));
-                return RedirectToAction("Index");
+            }
-            } catch (Exception exception) {
+            catch(ArgumentException argumentException) {
-                this.Error(exception, T("Deleting media folder failed: {0}", exception.Message), Logger, Services.Notifier);
+                Services.Notifier.Error(T("Deleting media folder failed: {0}", argumentException.Message));
-
                 return View(viewModel);
             }
+
+            return RedirectToAction("Index");
         }
 
         [HttpPost, ActionName("EditProperties")]
@@ -157,18 +159,17 @@ namespace Orchard.Media.Controllers {
                 return new HttpUnauthorizedResult();
 
             var viewModel = new MediaFolderEditPropertiesViewModel();
-            try {
             UpdateModel(viewModel);
-
+            try {
                 _mediaService.RenameFolder(viewModel.MediaPath, viewModel.Name);
-
                 Services.Notifier.Information(T("Media folder properties modified"));
-                return RedirectToAction("Index");
+            }
-            } catch (Exception exception) {
+            catch(ArgumentException argumentException) {
-                this.Error(exception, T("Modifying media folder properties failed: {0}", exception.Message), Logger, Services.Notifier);
+                Services.Notifier.Error(T("Modifying media folder properties failed: {0}", argumentException.Message));
-
                 return View(viewModel);
             }
+
+            return RedirectToAction("Index");
         }
 
         public ActionResult Add(string folderName, string mediaPath) {
@@ -182,7 +183,7 @@ namespace Orchard.Media.Controllers {
                 return new HttpUnauthorizedResult();
 
             var viewModel = new MediaItemAddViewModel();
-            try {
+
             UpdateModel(viewModel);
 
             if(String.IsNullOrWhiteSpace(Request.Files[0].FileName)) {
@@ -193,16 +194,17 @@ namespace Orchard.Media.Controllers {
                 return View(viewModel);
 
             foreach (string fileName in Request.Files) {
+                try {
                     _mediaService.UploadMediaFile(viewModel.MediaPath, Request.Files[fileName], viewModel.ExtractZip);
                 }
+                catch (ArgumentException argumentException) {
+                    Services.Notifier.Error(T("Uploading media file failed:"));
+                    return View(viewModel);   
+                }
+            }
 
             Services.Notifier.Information(T("Media file(s) uploaded"));
             return RedirectToAction("Edit", new { name = viewModel.FolderName, mediaPath = viewModel.MediaPath });
-            } catch (Exception exception) {
-                this.Error(exception, T("Uploading media file failed:"), Logger, Services.Notifier);
-
-                return View(viewModel);
-            }
         }
 
         [HttpPost]
@@ -211,7 +213,6 @@ namespace Orchard.Media.Controllers {
                 return Content(string.Format("<script type=\"text/javascript\">var result = {{ error: \"{0}\" }};</script>", T("ERROR: You don't have permission to upload media files")));
 
             var viewModel = new MediaItemAddViewModel();
-            try {
             UpdateModel(viewModel);
 
             if (Request.Files.Count < 1 || Request.Files[0].ContentLength == 0)
@@ -226,12 +227,13 @@ namespace Orchard.Media.Controllers {
             }
 
             var file = Request.Files[0];
-                var publicUrl = _mediaService.UploadMediaFile(viewModel.MediaPath, file, viewModel.ExtractZip);
                 
+            try {
+                var publicUrl = _mediaService.UploadMediaFile(viewModel.MediaPath, file, viewModel.ExtractZip);
                 return Content(string.Format("<script type=\"text/javascript\">var result = {{ url: \"{0}\" }};</script>", publicUrl));
             }
-            catch (Exception exception) {
+            catch (ArgumentException argumentException) {
-                return Content(string.Format("<script type=\"text/javascript\">var result = {{ error: \"{0}\" }};</script>", T("ERROR: Uploading media file failed: {0}", exception.Message)));
+                return Content(string.Format("<script type=\"text/javascript\">var result = {{ error: \"{0}\" }};</script>", T("ERROR: Uploading media file failed: {0}", argumentException.Message)));
             }
         }
 
@@ -247,18 +249,17 @@ namespace Orchard.Media.Controllers {
                 return new HttpUnauthorizedResult();
 
             var viewModel = new MediaItemEditViewModel();
-            try {
             UpdateModel(viewModel);
-
+            try {
                 _mediaService.DeleteFile(viewModel.Name, viewModel.MediaPath);
-
                 Services.Notifier.Information(T("Media deleted"));
-                return RedirectToAction("Edit", new { name = viewModel.FolderName, mediaPath = viewModel.MediaPath });
+            }
-            } catch (Exception exception) {
+            catch (ArgumentException argumentException) {
-                this.Error(exception, T("Removing media file failed: {0}", exception.Message), Logger, Services.Notifier);
+                Services.Notifier.Error(T("Removing media file failed: {0}", argumentException.Message));
-
                 return View(viewModel);
             }
+
+            return RedirectToAction("Edit", new { name = viewModel.FolderName, mediaPath = viewModel.MediaPath });
         }
 
         [HttpPost, ActionName("EditMedia")]
@@ -268,13 +269,18 @@ namespace Orchard.Media.Controllers {
                 return new HttpUnauthorizedResult();
 
             var viewModel = new MediaItemEditViewModel();
-            try {
             UpdateModel(viewModel);
             string viewModelName = viewModel.Name;
 
             // Rename
             if (!String.Equals(viewModel.Name, input["NewName"], StringComparison.OrdinalIgnoreCase)) {
+                try {
                     _mediaService.RenameFile(viewModel.MediaPath, viewModel.Name, input["NewName"]);
+                }
+                catch (ArgumentException argumentException) {
+                    Services.Notifier.Error(T("Editing media file failed."));
+                    return EditMedia(viewModel);
+                }
                 viewModelName = input["NewName"];
             }
 
@@ -286,10 +292,5 @@ namespace Orchard.Media.Controllers {
                                                         folderName = viewModel.FolderName,
                                                         mediaPath = viewModel.MediaPath });
         }
-            catch (Exception exception) {
-                this.Error(exception, T("Editing media file failed."), Logger, Services.Notifier);
-                return EditMedia(viewModel);
-            }
-        }
     }
 }

src/Orchard.Web/Modules/Orchard.MediaPicker/Controllers/AdminController.cs

@@ -1,13 +1,10 @@
 using System;
-using System.Collections.Generic;
 using System.Web;
 using System.Web.Mvc;
 using Orchard.Localization;
 using Orchard.Media;
-using Orchard.Media.Models;
 using Orchard.Media.Services;
 using Orchard.Media.ViewModels;
-using Orchard.Security;
 using Orchard.Themes;
 
 namespace Orchard.MediaPicker.Controllers {