From 0fa617e7f09965e23b1becb1aa877e518083f9e6 Mon Sep 17 00:00:00 2001 From: Andrew Ma Date: Thu, 15 Apr 2010 21:53:32 -0700 Subject: [PATCH 01/14] Updating the User module use the Localizer class to localize strings. Adding an extension method for ModelStateDictionary.AddModelError to accept LocalizedString objects. --- .../Controllers/AccountController.cs | 38 +++++++++---------- .../Controllers/AdminController.cs | 12 +++--- .../ModelStateDictionaryExtensions.cs | 11 ++++++ src/Orchard/Orchard.Framework.csproj | 1 + 4 files changed, 34 insertions(+), 28 deletions(-) create mode 100644 src/Orchard/Mvc/Extensions/ModelStateDictionaryExtensions.cs diff --git a/src/Orchard.Web/Modules/Orchard.Users/Controllers/AccountController.cs b/src/Orchard.Web/Modules/Orchard.Users/Controllers/AccountController.cs index 5975abe20..6de70a07f 100644 --- a/src/Orchard.Web/Modules/Orchard.Users/Controllers/AccountController.cs +++ b/src/Orchard.Web/Modules/Orchard.Users/Controllers/AccountController.cs @@ -4,6 +4,7 @@ using System.Globalization; using System.Security.Principal; using System.Web.Mvc; using System.Web.Security; +using Orchard.Localization; using Orchard.Logging; using Orchard.Mvc.Extensions; using Orchard.Mvc.ViewModels; @@ -26,9 +27,11 @@ namespace Orchard.Users.Controllers { _membershipService = membershipService; _userService = userService; Logger = NullLogger.Instance; + T = NullLocalizer.Instance; } public ILogger Logger { get; set; } + public Localizer T { get; set; } public ActionResult AccessDenied() { var returnUrl = Request.QueryString["ReturnUrl"]; @@ -98,7 +101,7 @@ namespace Orchard.Users.Controllers { return Redirect("~/"); } else { - ModelState.AddModelError("_FORM", ErrorCodeToString(/*createStatus*/MembershipCreateStatus.ProviderError)); + ModelState.AddModelError("_FORM", T(ErrorCodeToString(/*createStatus*/MembershipCreateStatus.ProviderError))); } } @@ -132,13 +135,12 @@ namespace Orchard.Users.Controllers { return RedirectToAction("ChangePasswordSuccess"); } else { - ModelState.AddModelError("_FORM", - "The current password is incorrect or the new password is invalid."); + ModelState.AddModelError("_FORM", T("The current password is incorrect or the new password is invalid.")); return ChangePassword(); } } catch { - ModelState.AddModelError("_FORM", "The current password is incorrect or the new password is invalid."); + ModelState.AddModelError("_FORM", T("The current password is incorrect or the new password is invalid.")); return ChangePassword(); } } @@ -157,17 +159,14 @@ namespace Orchard.Users.Controllers { private bool ValidateChangePassword(string currentPassword, string newPassword, string confirmPassword) { if (String.IsNullOrEmpty(currentPassword)) { - ModelState.AddModelError("currentPassword", "You must specify a current password."); + ModelState.AddModelError("currentPassword", T("You must specify a current password.")); } if (newPassword == null || newPassword.Length < MinPasswordLength) { - ModelState.AddModelError("newPassword", - String.Format(CultureInfo.CurrentCulture, - "You must specify a new password of {0} or more characters.", - MinPasswordLength)); + ModelState.AddModelError("newPassword", T("You must specify a new password of {0} or more characters.", MinPasswordLength)); } if (!String.Equals(newPassword, confirmPassword, StringComparison.Ordinal)) { - ModelState.AddModelError("_FORM", "The new password and confirmation password do not match."); + ModelState.AddModelError("_FORM", T("The new password and confirmation password do not match.")); } return ModelState.IsValid; @@ -175,14 +174,14 @@ namespace Orchard.Users.Controllers { private IUser ValidateLogOn(string userName, string password) { if (String.IsNullOrEmpty(userName)) { - ModelState.AddModelError("username", "You must specify a username."); + ModelState.AddModelError("username", T("You must specify a username.")); } if (String.IsNullOrEmpty(password)) { - ModelState.AddModelError("password", "You must specify a password."); + ModelState.AddModelError("password", T("You must specify a password.")); } var user = _membershipService.ValidateUser(userName, password); if (user == null) { - ModelState.AddModelError("_FORM", "The username or password provided is incorrect."); + ModelState.AddModelError("_FORM", T("The username or password provided is incorrect.")); } return user; @@ -190,23 +189,20 @@ namespace Orchard.Users.Controllers { private bool ValidateRegistration(string userName, string email, string password, string confirmPassword) { if (String.IsNullOrEmpty(userName)) { - ModelState.AddModelError("username", "You must specify a username."); + ModelState.AddModelError("username", T("You must specify a username.")); } if (String.IsNullOrEmpty(email)) { - ModelState.AddModelError("email", "You must specify an email address."); + ModelState.AddModelError("email", T("You must specify an email address.")); } string userUnicityMessage = _userService.VerifyUserUnicity(userName, email); if (userUnicityMessage != null) { - ModelState.AddModelError("userExists", userUnicityMessage); + ModelState.AddModelError("userExists", T(userUnicityMessage)); } if (password == null || password.Length < MinPasswordLength) { - ModelState.AddModelError("password", - String.Format(CultureInfo.CurrentCulture, - "You must specify a password of {0} or more characters.", - MinPasswordLength)); + ModelState.AddModelError("password", T("You must specify a password of {0} or more characters.", MinPasswordLength)); } if (!String.Equals(password, confirmPassword, StringComparison.Ordinal)) { - ModelState.AddModelError("_FORM", "The new password and confirmation password do not match."); + ModelState.AddModelError("_FORM", T("The new password and confirmation password do not match.")); } return ModelState.IsValid; } diff --git a/src/Orchard.Web/Modules/Orchard.Users/Controllers/AdminController.cs b/src/Orchard.Web/Modules/Orchard.Users/Controllers/AdminController.cs index d6f95b0ca..3502f0449 100644 --- a/src/Orchard.Web/Modules/Orchard.Users/Controllers/AdminController.cs +++ b/src/Orchard.Web/Modules/Orchard.Users/Controllers/AdminController.cs @@ -1,7 +1,8 @@ using System.Linq; using System.Web.Mvc; -using Orchard.Localization; using Orchard.ContentManagement; +using Orchard.Localization; +using Orchard.Mvc.Extensions; using Orchard.Security; using Orchard.UI.Notify; using Orchard.Users.Drivers; @@ -70,11 +71,11 @@ namespace Orchard.Users.Controllers { string userExistsMessage = _userService.VerifyUserUnicity(model.UserName, model.Email); if (userExistsMessage != null) { - AddModelError("NotUniqueUserName", T(userExistsMessage)); + ModelState.AddModelError("NotUniqueUserName", T(userExistsMessage)); } if (model.Password != model.ConfirmPassword) { - AddModelError("ConfirmPassword", T("Password confirmation must match")); + ModelState.AddModelError("ConfirmPassword", T("Password confirmation must match")); } user = _membershipService.CreateUser(new CreateUserParams( @@ -122,7 +123,7 @@ namespace Orchard.Users.Controllers { string userExistsMessage = _userService.VerifyUserUnicity(id, model.UserName, model.Email); if (userExistsMessage != null) { - AddModelError("NotUniqueUserName", T(userExistsMessage)); + ModelState.AddModelError("NotUniqueUserName", T(userExistsMessage)); } if (!ModelState.IsValid) { @@ -147,9 +148,6 @@ namespace Orchard.Users.Controllers { bool IUpdateModel.TryUpdateModel(TModel model, string prefix, string[] includeProperties, string[] excludeProperties) { return TryUpdateModel(model, prefix, includeProperties, excludeProperties); } - public void AddModelError(string key, LocalizedString errorMessage) { - ModelState.AddModelError(key, errorMessage.ToString()); - } } } diff --git a/src/Orchard/Mvc/Extensions/ModelStateDictionaryExtensions.cs b/src/Orchard/Mvc/Extensions/ModelStateDictionaryExtensions.cs new file mode 100644 index 000000000..e68990229 --- /dev/null +++ b/src/Orchard/Mvc/Extensions/ModelStateDictionaryExtensions.cs @@ -0,0 +1,11 @@ +using System; +using System.Web.Mvc; +using Orchard.Localization; + +namespace Orchard.Mvc.Extensions { + public static class ModelStateDictionaryExtensions { + public static void AddModelError(this ModelStateDictionary modelStateDictionary, string key, LocalizedString errorMessage) { + modelStateDictionary.AddModelError(key, errorMessage.ToString()); + } + } +} diff --git a/src/Orchard/Orchard.Framework.csproj b/src/Orchard/Orchard.Framework.csproj index ef1ac119c..742c0a02d 100644 --- a/src/Orchard/Orchard.Framework.csproj +++ b/src/Orchard/Orchard.Framework.csproj @@ -178,6 +178,7 @@ + From 31c154ebc4eb1aab9ab6825fa0c4a547da8a2a5f Mon Sep 17 00:00:00 2001 From: Suha Can Date: Fri, 16 Apr 2010 12:47:06 -0700 Subject: [PATCH 02/14] - Fixing build. --- .../Orchard.Users/Controllers/AccountController.cs | 1 - .../Orchard.Users/Controllers/AdminController.cs | 11 +++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/Orchard.Web/Modules/Orchard.Users/Controllers/AccountController.cs b/src/Orchard.Web/Modules/Orchard.Users/Controllers/AccountController.cs index 6de70a07f..685f496a9 100644 --- a/src/Orchard.Web/Modules/Orchard.Users/Controllers/AccountController.cs +++ b/src/Orchard.Web/Modules/Orchard.Users/Controllers/AccountController.cs @@ -1,6 +1,5 @@ using System; using System.Diagnostics.CodeAnalysis; -using System.Globalization; using System.Security.Principal; using System.Web.Mvc; using System.Web.Security; diff --git a/src/Orchard.Web/Modules/Orchard.Users/Controllers/AdminController.cs b/src/Orchard.Web/Modules/Orchard.Users/Controllers/AdminController.cs index 3502f0449..e85de7d77 100644 --- a/src/Orchard.Web/Modules/Orchard.Users/Controllers/AdminController.cs +++ b/src/Orchard.Web/Modules/Orchard.Users/Controllers/AdminController.cs @@ -2,7 +2,6 @@ using System.Linq; using System.Web.Mvc; using Orchard.ContentManagement; using Orchard.Localization; -using Orchard.Mvc.Extensions; using Orchard.Security; using Orchard.UI.Notify; using Orchard.Users.Drivers; @@ -71,11 +70,11 @@ namespace Orchard.Users.Controllers { string userExistsMessage = _userService.VerifyUserUnicity(model.UserName, model.Email); if (userExistsMessage != null) { - ModelState.AddModelError("NotUniqueUserName", T(userExistsMessage)); + AddModelError("NotUniqueUserName", T(userExistsMessage)); } if (model.Password != model.ConfirmPassword) { - ModelState.AddModelError("ConfirmPassword", T("Password confirmation must match")); + AddModelError("ConfirmPassword", T("Password confirmation must match")); } user = _membershipService.CreateUser(new CreateUserParams( @@ -123,7 +122,7 @@ namespace Orchard.Users.Controllers { string userExistsMessage = _userService.VerifyUserUnicity(id, model.UserName, model.Email); if (userExistsMessage != null) { - ModelState.AddModelError("NotUniqueUserName", T(userExistsMessage)); + AddModelError("NotUniqueUserName", T(userExistsMessage)); } if (!ModelState.IsValid) { @@ -148,6 +147,10 @@ namespace Orchard.Users.Controllers { bool IUpdateModel.TryUpdateModel(TModel model, string prefix, string[] includeProperties, string[] excludeProperties) { return TryUpdateModel(model, prefix, includeProperties, excludeProperties); } + + public void AddModelError(string key, LocalizedString errorMessage) { + ModelState.AddModelError(key, errorMessage.ToString()); + } } } From 13783e748ab017b9e9df5befdabd3e60ded5f556 Mon Sep 17 00:00:00 2001 From: Suha Can Date: Fri, 16 Apr 2010 13:14:45 -0700 Subject: [PATCH 03/14] - Patch for issue #16396 by Andrew Ma --- .../Controllers/AccountController.cs | 49 ++++++++++--------- .../Services/MembershipService.cs | 6 ++- .../Orchard.Users/Views/Account/LogOn.ascx | 6 +-- src/Orchard/Security/IMembershipService.cs | 2 +- 4 files changed, 35 insertions(+), 28 deletions(-) diff --git a/src/Orchard.Web/Modules/Orchard.Users/Controllers/AccountController.cs b/src/Orchard.Web/Modules/Orchard.Users/Controllers/AccountController.cs index 685f496a9..7eb3b4767 100644 --- a/src/Orchard.Web/Modules/Orchard.Users/Controllers/AccountController.cs +++ b/src/Orchard.Web/Modules/Orchard.Users/Controllers/AccountController.cs @@ -1,9 +1,9 @@ using System; using System.Diagnostics.CodeAnalysis; +using System.Globalization; using System.Security.Principal; using System.Web.Mvc; using System.Web.Security; -using Orchard.Localization; using Orchard.Logging; using Orchard.Mvc.Extensions; using Orchard.Mvc.ViewModels; @@ -26,11 +26,9 @@ namespace Orchard.Users.Controllers { _membershipService = membershipService; _userService = userService; Logger = NullLogger.Instance; - T = NullLocalizer.Instance; } public ILogger Logger { get; set; } - public Localizer T { get; set; } public ActionResult AccessDenied() { var returnUrl = Request.QueryString["ReturnUrl"]; @@ -57,8 +55,8 @@ namespace Orchard.Users.Controllers { [HttpPost] [SuppressMessage("Microsoft.Design", "CA1054:UriParametersShouldNotBeStrings", Justification = "Needs to take same parameter type as Controller.Redirect()")] - public ActionResult LogOn(string userName, string password, bool rememberMe) { - var user = ValidateLogOn(userName, password); + public ActionResult LogOn(string userNameOrEmail, string password, bool rememberMe) { + var user = ValidateLogOn(userNameOrEmail, password); if (!ModelState.IsValid) { return View("LogOn", new LogOnViewModel {Title = "Log On"}); } @@ -100,7 +98,7 @@ namespace Orchard.Users.Controllers { return Redirect("~/"); } else { - ModelState.AddModelError("_FORM", T(ErrorCodeToString(/*createStatus*/MembershipCreateStatus.ProviderError))); + ModelState.AddModelError("_FORM", ErrorCodeToString(/*createStatus*/MembershipCreateStatus.ProviderError)); } } @@ -134,12 +132,13 @@ namespace Orchard.Users.Controllers { return RedirectToAction("ChangePasswordSuccess"); } else { - ModelState.AddModelError("_FORM", T("The current password is incorrect or the new password is invalid.")); + ModelState.AddModelError("_FORM", + "The current password is incorrect or the new password is invalid."); return ChangePassword(); } } catch { - ModelState.AddModelError("_FORM", T("The current password is incorrect or the new password is invalid.")); + ModelState.AddModelError("_FORM", "The current password is incorrect or the new password is invalid."); return ChangePassword(); } } @@ -158,29 +157,32 @@ namespace Orchard.Users.Controllers { private bool ValidateChangePassword(string currentPassword, string newPassword, string confirmPassword) { if (String.IsNullOrEmpty(currentPassword)) { - ModelState.AddModelError("currentPassword", T("You must specify a current password.")); + ModelState.AddModelError("currentPassword", "You must specify a current password."); } if (newPassword == null || newPassword.Length < MinPasswordLength) { - ModelState.AddModelError("newPassword", T("You must specify a new password of {0} or more characters.", MinPasswordLength)); + ModelState.AddModelError("newPassword", + String.Format(CultureInfo.CurrentCulture, + "You must specify a new password of {0} or more characters.", + MinPasswordLength)); } if (!String.Equals(newPassword, confirmPassword, StringComparison.Ordinal)) { - ModelState.AddModelError("_FORM", T("The new password and confirmation password do not match.")); + ModelState.AddModelError("_FORM", "The new password and confirmation password do not match."); } return ModelState.IsValid; } - private IUser ValidateLogOn(string userName, string password) { - if (String.IsNullOrEmpty(userName)) { - ModelState.AddModelError("username", T("You must specify a username.")); + private IUser ValidateLogOn(string userNameOrEmail, string password) { + if (String.IsNullOrEmpty(userNameOrEmail)) { + ModelState.AddModelError("userNameOrEmail", "You must specify a username or e-mail."); } if (String.IsNullOrEmpty(password)) { - ModelState.AddModelError("password", T("You must specify a password.")); + ModelState.AddModelError("password", "You must specify a password."); } - var user = _membershipService.ValidateUser(userName, password); + var user = _membershipService.ValidateUser(userNameOrEmail, password); if (user == null) { - ModelState.AddModelError("_FORM", T("The username or password provided is incorrect.")); + ModelState.AddModelError("_FORM", "The username or e-mail or password provided is incorrect."); } return user; @@ -188,20 +190,23 @@ namespace Orchard.Users.Controllers { private bool ValidateRegistration(string userName, string email, string password, string confirmPassword) { if (String.IsNullOrEmpty(userName)) { - ModelState.AddModelError("username", T("You must specify a username.")); + ModelState.AddModelError("username", "You must specify a username."); } if (String.IsNullOrEmpty(email)) { - ModelState.AddModelError("email", T("You must specify an email address.")); + ModelState.AddModelError("email", "You must specify an email address."); } string userUnicityMessage = _userService.VerifyUserUnicity(userName, email); if (userUnicityMessage != null) { - ModelState.AddModelError("userExists", T(userUnicityMessage)); + ModelState.AddModelError("userExists", userUnicityMessage); } if (password == null || password.Length < MinPasswordLength) { - ModelState.AddModelError("password", T("You must specify a password of {0} or more characters.", MinPasswordLength)); + ModelState.AddModelError("password", + String.Format(CultureInfo.CurrentCulture, + "You must specify a password of {0} or more characters.", + MinPasswordLength)); } if (!String.Equals(password, confirmPassword, StringComparison.Ordinal)) { - ModelState.AddModelError("_FORM", T("The new password and confirmation password do not match.")); + ModelState.AddModelError("_FORM", "The new password and confirmation password do not match."); } return ModelState.IsValid; } diff --git a/src/Orchard.Web/Modules/Orchard.Users/Services/MembershipService.cs b/src/Orchard.Web/Modules/Orchard.Users/Services/MembershipService.cs index a7fb519fb..94eb85f91 100644 --- a/src/Orchard.Web/Modules/Orchard.Users/Services/MembershipService.cs +++ b/src/Orchard.Web/Modules/Orchard.Users/Services/MembershipService.cs @@ -51,8 +51,10 @@ namespace Orchard.Users.Services { return _contentManager.Get(userRecord.Id); } - public IUser ValidateUser(string username, string password) { - var userRecord = _userRepository.Get(x => x.NormalizedUserName == username.ToLower()); + public IUser ValidateUser(string userNameOrEmail, string password) { + var userRecord = _userRepository.Get(x => x.NormalizedUserName == userNameOrEmail.ToLower()); + if(userRecord == null) + userRecord = _userRepository.Get(x => x.Email == userNameOrEmail.ToLower()); if (userRecord == null || ValidatePassword(userRecord, password) == false) return null; diff --git a/src/Orchard.Web/Modules/Orchard.Users/Views/Account/LogOn.ascx b/src/Orchard.Web/Modules/Orchard.Users/Views/Account/LogOn.ascx index d910ccb98..de5f098cd 100644 --- a/src/Orchard.Web/Modules/Orchard.Users/Views/Account/LogOn.ascx +++ b/src/Orchard.Web/Modules/Orchard.Users/Views/Account/LogOn.ascx @@ -8,9 +8,9 @@ using (Html.BeginFormAntiForgeryPost(Url.Action("LogOn", new {ReturnUrl = Reques
<%=_Encoded("Account Information")%>
- - <%= Html.TextBox("username")%> - <%= Html.ValidationMessage("username")%> + + <%= Html.TextBox("userNameOrEmail")%> + <%= Html.ValidationMessage("userNameOrEmail")%>
diff --git a/src/Orchard/Security/IMembershipService.cs b/src/Orchard/Security/IMembershipService.cs index 44578cac1..0821fcdb1 100644 --- a/src/Orchard/Security/IMembershipService.cs +++ b/src/Orchard/Security/IMembershipService.cs @@ -5,7 +5,7 @@ IUser CreateUser(CreateUserParams createUserParams); IUser GetUser(string username); - IUser ValidateUser(string username, string password); + IUser ValidateUser(string userNameOrEmail, string password); void SetPassword(IUser user, string password); } } From d4b79ea27255886109e4025e7e34c11d3ceb4dd7 Mon Sep 17 00:00:00 2001 From: Suha Can Date: Fri, 16 Apr 2010 13:33:45 -0700 Subject: [PATCH 04/14] - Merge --- .../Controllers/AccountController.cs | 38 +++++++++---------- 1 file changed, 17 insertions(+), 21 deletions(-) diff --git a/src/Orchard.Web/Modules/Orchard.Users/Controllers/AccountController.cs b/src/Orchard.Web/Modules/Orchard.Users/Controllers/AccountController.cs index 7eb3b4767..c4f13ee73 100644 --- a/src/Orchard.Web/Modules/Orchard.Users/Controllers/AccountController.cs +++ b/src/Orchard.Web/Modules/Orchard.Users/Controllers/AccountController.cs @@ -1,6 +1,6 @@ using System; using System.Diagnostics.CodeAnalysis; -using System.Globalization; +using Orchard.Localization; using System.Security.Principal; using System.Web.Mvc; using System.Web.Security; @@ -26,9 +26,11 @@ namespace Orchard.Users.Controllers { _membershipService = membershipService; _userService = userService; Logger = NullLogger.Instance; + T = NullLocalizer.Instance; } public ILogger Logger { get; set; } + public Localizer T { get; set; } public ActionResult AccessDenied() { var returnUrl = Request.QueryString["ReturnUrl"]; @@ -98,7 +100,7 @@ namespace Orchard.Users.Controllers { return Redirect("~/"); } else { - ModelState.AddModelError("_FORM", ErrorCodeToString(/*createStatus*/MembershipCreateStatus.ProviderError)); + ModelState.AddModelError("_FORM", T(ErrorCodeToString(/*createStatus*/MembershipCreateStatus.ProviderError))); } } @@ -133,12 +135,12 @@ namespace Orchard.Users.Controllers { } else { ModelState.AddModelError("_FORM", - "The current password is incorrect or the new password is invalid."); + T("The current password is incorrect or the new password is invalid.")); return ChangePassword(); } } catch { - ModelState.AddModelError("_FORM", "The current password is incorrect or the new password is invalid."); + ModelState.AddModelError("_FORM", T("The current password is incorrect or the new password is invalid.")); return ChangePassword(); } } @@ -157,17 +159,14 @@ namespace Orchard.Users.Controllers { private bool ValidateChangePassword(string currentPassword, string newPassword, string confirmPassword) { if (String.IsNullOrEmpty(currentPassword)) { - ModelState.AddModelError("currentPassword", "You must specify a current password."); + ModelState.AddModelError("currentPassword", T("You must specify a current password.")); } if (newPassword == null || newPassword.Length < MinPasswordLength) { - ModelState.AddModelError("newPassword", - String.Format(CultureInfo.CurrentCulture, - "You must specify a new password of {0} or more characters.", - MinPasswordLength)); + ModelState.AddModelError("newPassword", T("You must specify a new password of {0} or more characters.", MinPasswordLength)); } if (!String.Equals(newPassword, confirmPassword, StringComparison.Ordinal)) { - ModelState.AddModelError("_FORM", "The new password and confirmation password do not match."); + ModelState.AddModelError("_FORM", T("The new password and confirmation password do not match.")); } return ModelState.IsValid; @@ -175,14 +174,14 @@ namespace Orchard.Users.Controllers { private IUser ValidateLogOn(string userNameOrEmail, string password) { if (String.IsNullOrEmpty(userNameOrEmail)) { - ModelState.AddModelError("userNameOrEmail", "You must specify a username or e-mail."); + ModelState.AddModelError("userNameOrEmail", T("You must specify a username or e-mail.")); } if (String.IsNullOrEmpty(password)) { - ModelState.AddModelError("password", "You must specify a password."); + ModelState.AddModelError("password", T("You must specify a password.")); } var user = _membershipService.ValidateUser(userNameOrEmail, password); if (user == null) { - ModelState.AddModelError("_FORM", "The username or e-mail or password provided is incorrect."); + ModelState.AddModelError("_FORM", T("The username or e-mail or password provided is incorrect.")); } return user; @@ -190,23 +189,20 @@ namespace Orchard.Users.Controllers { private bool ValidateRegistration(string userName, string email, string password, string confirmPassword) { if (String.IsNullOrEmpty(userName)) { - ModelState.AddModelError("username", "You must specify a username."); + ModelState.AddModelError("username", T("You must specify a username.")); } if (String.IsNullOrEmpty(email)) { - ModelState.AddModelError("email", "You must specify an email address."); + ModelState.AddModelError("email", T("You must specify an email address.")); } string userUnicityMessage = _userService.VerifyUserUnicity(userName, email); if (userUnicityMessage != null) { - ModelState.AddModelError("userExists", userUnicityMessage); + ModelState.AddModelError("userExists", T(userUnicityMessage)); } if (password == null || password.Length < MinPasswordLength) { - ModelState.AddModelError("password", - String.Format(CultureInfo.CurrentCulture, - "You must specify a password of {0} or more characters.", - MinPasswordLength)); + ModelState.AddModelError("password", T("You must specify a password of {0} or more characters.", MinPasswordLength)); } if (!String.Equals(password, confirmPassword, StringComparison.Ordinal)) { - ModelState.AddModelError("_FORM", "The new password and confirmation password do not match."); + ModelState.AddModelError("_FORM", T("The new password and confirmation password do not match.")); } return ModelState.IsValid; } From bea0266688b6ec47e792c53da8611c6a06fc74ec Mon Sep 17 00:00:00 2001 From: Sebastien Ros Date: Fri, 16 Apr 2010 13:31:36 -0700 Subject: [PATCH 05/14] =?UTF-8?q?Added=20an=20=20ArgumentNullException=20s?= =?UTF-8?q?anity=20check=20to=20let=20devs=20know=20=20it=E2=80=99s=20not?= =?UTF-8?q?=20allowed=20here,=20and=20checked=20before=20calling=20the=20m?= =?UTF-8?q?ethod.=20The=20standard=20validation=20pipeline=20does=20the=20?= =?UTF-8?q?rest=20as=20it=20is=20marked=20[Required]?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/Orchard.Web/Core/Common/Handlers/CommonAspectHandler.cs | 3 ++- .../Modules/Orchard.Users/Services/MembershipService.cs | 3 +++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Orchard.Web/Core/Common/Handlers/CommonAspectHandler.cs b/src/Orchard.Web/Core/Common/Handlers/CommonAspectHandler.cs index 4ad9f4337..a2f3d4366 100644 --- a/src/Orchard.Web/Core/Common/Handlers/CommonAspectHandler.cs +++ b/src/Orchard.Web/Core/Common/Handlers/CommonAspectHandler.cs @@ -172,7 +172,7 @@ namespace Orchard.Core.Common.Handlers { var priorOwner = viewModel.Owner; context.Updater.TryUpdateModel(viewModel, "CommonAspect", null, null); - if (viewModel.Owner != priorOwner) { + if (viewModel.Owner != null && viewModel.Owner != priorOwner) { var newOwner = _membershipService.GetUser(viewModel.Owner); if (newOwner == null) { context.Updater.AddModelError("CommonAspect.Owner", T("Invalid user name")); @@ -181,6 +181,7 @@ namespace Orchard.Core.Common.Handlers { instance.Owner = newOwner; } } + context.AddEditor(new TemplateViewModel(viewModel, "CommonAspect") { TemplateName = "Parts/Common.Owner", ZoneName = "primary", Position = "999" }); } } diff --git a/src/Orchard.Web/Modules/Orchard.Users/Services/MembershipService.cs b/src/Orchard.Web/Modules/Orchard.Users/Services/MembershipService.cs index 94eb85f91..3d44d1169 100644 --- a/src/Orchard.Web/Modules/Orchard.Users/Services/MembershipService.cs +++ b/src/Orchard.Web/Modules/Orchard.Users/Services/MembershipService.cs @@ -44,6 +44,9 @@ namespace Orchard.Users.Services { } public IUser GetUser(string username) { + if(username == null) { + throw new ArgumentNullException("username"); + } var userRecord = _userRepository.Get(x => x.NormalizedUserName == username.ToLower()); if (userRecord == null) { return null; From 1b214b3ac94a86c8fd7edda177d9202fd8cc1dcc Mon Sep 17 00:00:00 2001 From: Sebastien Ros Date: Mon, 19 Apr 2010 16:35:05 -0700 Subject: [PATCH 06/14] Added unit tests on CommonAspect.Owner validation --- .../Providers/CommonAspectProviderTests.cs | 87 ++++++++++++++++++- 1 file changed, 85 insertions(+), 2 deletions(-) diff --git a/src/Orchard.Core.Tests/Common/Providers/CommonAspectProviderTests.cs b/src/Orchard.Core.Tests/Common/Providers/CommonAspectProviderTests.cs index bed6e4d2d..a6a6f8aab 100644 --- a/src/Orchard.Core.Tests/Common/Providers/CommonAspectProviderTests.cs +++ b/src/Orchard.Core.Tests/Common/Providers/CommonAspectProviderTests.cs @@ -6,13 +6,18 @@ using JetBrains.Annotations; using Moq; using NUnit.Framework; using Orchard.ContentManagement.Aspects; +using Orchard.Core.Common; using Orchard.Core.Common.Handlers; using Orchard.Core.Common.Models; using Orchard.ContentManagement; using Orchard.ContentManagement.Handlers; using Orchard.ContentManagement.Records; +using Orchard.Localization; using Orchard.Security; using Orchard.Tests.Modules; +using Orchard.Mvc.ViewModels; +using Orchard.Core.Common.ViewModels; +using System.Web.Mvc; namespace Orchard.Core.Tests.Common.Providers { [TestFixture] @@ -29,6 +34,7 @@ namespace Orchard.Core.Tests.Common.Providers { _authn = new Mock(); _authz = new Mock(); _membership = new Mock(); + builder.RegisterInstance(_authn.Object); builder.RegisterInstance(_authz.Object); builder.RegisterInstance(_membership.Object); @@ -87,7 +93,83 @@ namespace Orchard.Core.Tests.Common.Providers { } [Test] - public void PublishingShouldSetPublishUtc() { + public void PublishingShouldFailIfOwnerIsUnknown() + { + var contentManager = _container.Resolve(); + var updateModel = new Mock(); + + var user = contentManager.New("user"); + _authn.Setup(x => x.GetAuthenticatedUser()).Returns(user); + + var createUtc = _clock.UtcNow; + var item = contentManager.Create("test-item", VersionOptions.Draft, init => { }); + var viewModel = new OwnerEditorViewModel() { Owner = "user" }; + updateModel.Setup(x => x.TryUpdateModel(viewModel, "", null, null)).Returns(true); + contentManager.UpdateEditorModel(item.ContentItem, updateModel.Object); + } + + class UpdatModelStub : IUpdateModel { + + ModelStateDictionary _modelState = new ModelStateDictionary(); + + public ModelStateDictionary ModelErrors + { + get { return _modelState; } + } + + public string Owner { get; set; } + + public bool TryUpdateModel(TModel model, string prefix, string[] includeProperties, string[] excludeProperties) where TModel : class { + (model as OwnerEditorViewModel).Owner = Owner; + return true; + } + + public void AddModelError(string key, LocalizedString errorMessage) { + _modelState.AddModelError(key, errorMessage.ToString()); + } + } + + [Test] + public void PublishingShouldNotThrowExceptionIfOwnerIsNull() + { + var contentManager = _container.Resolve(); + + var item = contentManager.Create("test-item", VersionOptions.Draft, init => { }); + + var user = contentManager.New("user"); + _authn.Setup(x => x.GetAuthenticatedUser()).Returns(user); + _authz.Setup(x => x.TryCheckAccess(Permissions.ChangeOwner, user, item)).Returns(true); + + item.Owner = user; + + var updater = new UpdatModelStub() { Owner = null }; + + contentManager.UpdateEditorModel(item.ContentItem, updater); + } + + [Test] + public void PublishingShouldFailIfOwnerIsEmpty() + { + var contentManager = _container.Resolve(); + + var item = contentManager.Create("test-item", VersionOptions.Draft, init => { }); + + var user = contentManager.New("user"); + _authn.Setup(x => x.GetAuthenticatedUser()).Returns(user); + _authz.Setup(x => x.TryCheckAccess(Permissions.ChangeOwner, user, item)).Returns(true); + + item.Owner = user; + + var updater = new UpdatModelStub() {Owner = ""}; + + contentManager.UpdateEditorModel(item.ContentItem, updater); + + Assert.That(updater.ModelErrors.ContainsKey("CommonAspect.Owner"), Is.True); + } + + [Test] + public void PublishingShouldSetPublishUtc() + { var contentManager = _container.Resolve(); var createUtc = _clock.UtcNow; @@ -95,7 +177,7 @@ namespace Orchard.Core.Tests.Common.Providers { Assert.That(item.CreatedUtc, Is.EqualTo(createUtc)); Assert.That(item.PublishedUtc, Is.Null); - + _clock.Advance(TimeSpan.FromMinutes(1)); var publishUtc = _clock.UtcNow; @@ -105,6 +187,7 @@ namespace Orchard.Core.Tests.Common.Providers { Assert.That(item.PublishedUtc, Is.EqualTo(publishUtc)); } + [Test] public void VersioningItemShouldCreatedAndPublishedUtcValuesPerVersion() { var contentManager = _container.Resolve(); From 8aebc2475aac7b69bc91d8017393c86e68e63695 Mon Sep 17 00:00:00 2001 From: Sebastien Ros Date: Tue, 20 Apr 2010 16:09:13 -0700 Subject: [PATCH 07/14] Moved slugs management from Blogs and Pages to Routable Corrected slug generation on client side --- .../Core/Common/Drivers/RoutableDriver.cs | 21 +++++++ .../Common/Handlers/RoutableAspectHandler.cs | 6 +- .../Core/Common/Services/IRoutableService.cs | 16 +++++ .../Core/Common/Services/RoutableService.cs | 55 +++++++++++++++- .../Orchard.Blogs/Drivers/BlogDriver.cs | 42 +------------ .../Orchard.Blogs/Handlers/BlogPostHandler.cs | 24 +------ .../Orchard.Pages/Drivers/PageDriver.cs | 63 ------------------- .../Orchard.Pages/Handlers/PageHandler.cs | 23 +------ 8 files changed, 98 insertions(+), 152 deletions(-) diff --git a/src/Orchard.Web/Core/Common/Drivers/RoutableDriver.cs b/src/Orchard.Web/Core/Common/Drivers/RoutableDriver.cs index 9261c0a81..55f46c473 100644 --- a/src/Orchard.Web/Core/Common/Drivers/RoutableDriver.cs +++ b/src/Orchard.Web/Core/Common/Drivers/RoutableDriver.cs @@ -3,16 +3,30 @@ using Orchard.ContentManagement; using Orchard.ContentManagement.Drivers; using Orchard.Core.Common.Models; using Orchard.Core.Common.ViewModels; +using Orchard.Core.Common.Services; +using Orchard.Localization; namespace Orchard.Core.Common.Drivers { [UsedImplicitly] public class Routable : ContentPartDriver { private const string TemplateName = "Parts/Common.Routable"; + private readonly IOrchardServices _services; + private readonly IRoutableService _routableService; + private Localizer T { get; set; } + protected override string Prefix { get { return "Routable"; } } + public Routable(IOrchardServices services, IRoutableService routableService) + { + _services = services; + _routableService = routableService; + + T = NullLocalizer.Instance; + } + protected override DriverResult Editor(RoutableAspect part) { var model = new RoutableEditorViewModel { Prefix = Prefix, RoutableAspect = part }; return ContentPartTemplate(model, TemplateName, Prefix).Location("primary", "before.5"); @@ -21,7 +35,14 @@ namespace Orchard.Core.Common.Drivers { protected override DriverResult Editor(RoutableAspect part, IUpdateModel updater) { var model = new RoutableEditorViewModel { Prefix = Prefix, RoutableAspect = part }; updater.TryUpdateModel(model, Prefix, null, null); + + if (!_routableService.IsSlugValid(part.Slug)){ + updater.AddModelError("Routable.Slug", T("Please do not use any of the following characters in your slugs: \"/\", \":\", \"?\", \"#\", \"[\", \"]\", \"@\", \"!\", \"$\", \"&\", \"'\", \"(\", \")\", \"*\", \"+\", \",\", \";\", \"=\". No spaces are allowed (please use dashes or underscores instead).").ToString()); + } + + _routableService.ProcessSlug(part); return ContentPartTemplate(model, TemplateName, Prefix).Location("primary", "before.5"); } + } } \ No newline at end of file diff --git a/src/Orchard.Web/Core/Common/Handlers/RoutableAspectHandler.cs b/src/Orchard.Web/Core/Common/Handlers/RoutableAspectHandler.cs index b902fb2c6..b1e4244a0 100644 --- a/src/Orchard.Web/Core/Common/Handlers/RoutableAspectHandler.cs +++ b/src/Orchard.Web/Core/Common/Handlers/RoutableAspectHandler.cs @@ -7,6 +7,7 @@ using Orchard.ContentManagement; using Orchard.ContentManagement.Drivers; using Orchard.ContentManagement.Handlers; using Orchard.Core.Common.Models; +using Orchard.Core.Common.Services; using Orchard.Data; namespace Orchard.Core.Common.Handlers { @@ -14,7 +15,7 @@ namespace Orchard.Core.Common.Handlers { public class RoutableAspectHandler : ContentHandler { private readonly IEnumerable _contentItemDrivers; - public RoutableAspectHandler(IRepository repository, IEnumerable contentItemDrivers, UrlHelper urlHelper) { + public RoutableAspectHandler(IRepository repository, IEnumerable contentItemDrivers, IRoutableService routableService, UrlHelper urlHelper) { _contentItemDrivers = contentItemDrivers; Filters.Add(StorageFilter.For(repository)); @@ -32,6 +33,9 @@ namespace Orchard.Core.Common.Handlers { routable.ContentItemBasePath = url; }); + + OnPublished((context, bp) => routableService.ProcessSlug(bp)); + } private static RouteValueDictionary GetRouteValues(IContentItemDriver driver, ContentItem contentItem) { diff --git a/src/Orchard.Web/Core/Common/Services/IRoutableService.cs b/src/Orchard.Web/Core/Common/Services/IRoutableService.cs index 2780a4789..deb7b8476 100644 --- a/src/Orchard.Web/Core/Common/Services/IRoutableService.cs +++ b/src/Orchard.Web/Core/Common/Services/IRoutableService.cs @@ -7,5 +7,21 @@ namespace Orchard.Core.Common.Services { void FillSlug(TModel model) where TModel : RoutableAspect; void FillSlug(TModel model, Func generateSlug) where TModel : RoutableAspect; string GenerateUniqueSlug(string slugCandidate, IEnumerable existingSlugs); + + /// + /// Returns any content item of the specified content type with similar slugs + /// + string[] GetSimilarSlugs(string contentType, string slug); + + /// + /// Validates the given slug + /// + bool IsSlugValid(string slug); + + /// + /// Defines the slug of a RoutableAspect and validate its unicity + /// + void ProcessSlug(RoutableAspect part); + } } \ No newline at end of file diff --git a/src/Orchard.Web/Core/Common/Services/RoutableService.cs b/src/Orchard.Web/Core/Common/Services/RoutableService.cs index f0f737444..1ebad4869 100644 --- a/src/Orchard.Web/Core/Common/Services/RoutableService.cs +++ b/src/Orchard.Web/Core/Common/Services/RoutableService.cs @@ -4,10 +4,23 @@ using System.Linq; using System.Text.RegularExpressions; using JetBrains.Annotations; using Orchard.Core.Common.Models; +using Orchard.ContentManagement; +using Orchard.Localization; +using Orchard.UI.Notify; namespace Orchard.Core.Common.Services { [UsedImplicitly] public class RoutableService : IRoutableService { + private readonly IOrchardServices _services; + private readonly IContentManager _contentManager; + private Localizer T { get; set; } + + public RoutableService(IOrchardServices services, IContentManager contentManager) { + _services = services; + _contentManager = contentManager; + T = NullLocalizer.Instance; + } + public void FillSlug(TModel model) where TModel : RoutableAspect { if (!string.IsNullOrEmpty(model.Slug) || string.IsNullOrEmpty(model.Title)) return; @@ -31,8 +44,6 @@ namespace Orchard.Core.Common.Services { model.Slug = generateSlug(model.Title).ToLowerInvariant(); } - - public string GenerateUniqueSlug(string slugCandidate, IEnumerable existingSlugs) { if (existingSlugs == null || !existingSlugs.Contains(slugCandidate)) return slugCandidate; @@ -55,5 +66,45 @@ namespace Orchard.Core.Common.Services { ? (int?)++v : null; } + + public string[] GetSimilarSlugs(string contentType, string slug) + { + return + _contentManager.Query(contentType).Join() + .Where(rr => rr.Slug.StartsWith(slug, StringComparison.OrdinalIgnoreCase)) + .List() + .Cast() + .Select(i => i.Slug) + .ToArray(); + } + + public bool IsSlugValid(string slug) { + return slug == null || String.IsNullOrEmpty(slug.Trim()) || !Regex.IsMatch(slug, @"^[^/:?#\[\]@!$&'()*+,;=\s]+$"); + } + + public void ProcessSlug(RoutableAspect part) + { + FillSlug(part); + + if (string.IsNullOrEmpty(part.Slug)) + { + return; + } + + var slugsLikeThis = GetSimilarSlugs(part.ContentItem.ContentType, part.Slug); + + //todo: (heskew) need better messages + if (slugsLikeThis.Length > 0) + { + var originalSlug = part.Slug; + //todo: (heskew) make auto-uniqueness optional + part.Slug = GenerateUniqueSlug(part.Slug, slugsLikeThis); + + if (originalSlug != part.Slug) { + _services.Notifier.Warning(T("Slugs in conflict. \"{0}\" is already set for a previously created {2} so now it has the slug \"{1}\"", + originalSlug, part.Slug, part.ContentItem.ContentType)); + } + } + } } } \ No newline at end of file diff --git a/src/Orchard.Web/Modules/Orchard.Blogs/Drivers/BlogDriver.cs b/src/Orchard.Web/Modules/Orchard.Blogs/Drivers/BlogDriver.cs index 61485ccf5..99126d397 100644 --- a/src/Orchard.Web/Modules/Orchard.Blogs/Drivers/BlogDriver.cs +++ b/src/Orchard.Web/Modules/Orchard.Blogs/Drivers/BlogDriver.cs @@ -27,14 +27,12 @@ namespace Orchard.Blogs.Drivers { private readonly IContentManager _contentManager; private readonly IBlogService _blogService; private readonly IBlogPostService _blogPostService; - private readonly IRoutableService _routableService; - public BlogDriver(IOrchardServices services, IContentManager contentManager, IBlogService blogService, IBlogPostService blogPostService, IRoutableService routableService) { + public BlogDriver(IOrchardServices services, IContentManager contentManager, IBlogService blogService, IBlogPostService blogPostService) { Services = services; _contentManager = contentManager; _blogService = blogService; _blogPostService = blogPostService; - _routableService = routableService; T = NullLocalizer.Instance; } @@ -97,47 +95,9 @@ namespace Orchard.Blogs.Drivers { protected override DriverResult Editor(Blog blog, IUpdateModel updater) { updater.TryUpdateModel(blog, Prefix, null, null); - //todo: (heskew) something better needs to be done with this...still feels shoehorned in here - ProcessSlug(blog, updater); - return Combined( ContentItemTemplate("Items/Blogs.Blog"), ContentPartTemplate(blog, "Parts/Blogs.Blog.Fields").Location("primary", "1")); } - - private void ProcessSlug(Blog blog, IUpdateModel updater) { - _routableService.FillSlug(blog.As()); - - - if (string.IsNullOrEmpty(blog.Slug)) { - return; - - // OR - - // updater.AddModelError("Routable.Slug", T("The slug is required.").ToString()); - // return; - } - - - if (!Regex.IsMatch(blog.Slug, @"^[^/:?#\[\]@!$&'()*+,;=\s]+$")) { - //todo: (heskew) get rid of the hard-coded prefix - updater.AddModelError("Routable.Slug", T("Please do not use any of the following characters in your slugs: \"/\", \":\", \"?\", \"#\", \"[\", \"]\", \"@\", \"!\", \"$\", \"&\", \"'\", \"(\", \")\", \"*\", \"+\", \",\", \";\", \"=\". No spaces are allowed (please use dashes or underscores instead).").ToString()); - } - - var slugsLikeThis = _blogService.Get().Where( - b => b.Slug.StartsWith(blog.Slug, StringComparison.OrdinalIgnoreCase) && - b.Id != blog.Id).Select(b => b.Slug); - - //todo: (heskew) need better messages - if (slugsLikeThis.Count() > 0) { - var originalSlug = blog.Slug; - //todo: (heskew) make auto-uniqueness optional - blog.Slug = _routableService.GenerateUniqueSlug(blog.Slug, slugsLikeThis); - - if (originalSlug != blog.Slug) - Services.Notifier.Warning(T("Slugs in conflict. \"{0}\" is already set for a previously created blog so this blog now has the slug \"{1}\"", - originalSlug, blog.Slug)); - } - } } } \ No newline at end of file diff --git a/src/Orchard.Web/Modules/Orchard.Blogs/Handlers/BlogPostHandler.cs b/src/Orchard.Web/Modules/Orchard.Blogs/Handlers/BlogPostHandler.cs index 7e7864d3d..e77a70fe4 100644 --- a/src/Orchard.Web/Modules/Orchard.Blogs/Handlers/BlogPostHandler.cs +++ b/src/Orchard.Web/Modules/Orchard.Blogs/Handlers/BlogPostHandler.cs @@ -16,12 +16,10 @@ namespace Orchard.Blogs.Handlers { [UsedImplicitly] public class BlogPostHandler : ContentHandler { private readonly IBlogPostService _blogPostService; - private readonly IRoutableService _routableService; private readonly IOrchardServices _orchardServices; - public BlogPostHandler(IBlogService blogService, IBlogPostService blogPostService, IRoutableService routableService, IOrchardServices orchardServices, RequestContext requestContext) { + public BlogPostHandler(IBlogService blogService, IBlogPostService blogPostService, IOrchardServices orchardServices, RequestContext requestContext) { _blogPostService = blogPostService; - _routableService = routableService; _orchardServices = orchardServices; T = NullLocalizer.Instance; @@ -63,8 +61,6 @@ namespace Orchard.Blogs.Handlers { OnVersioned((context, bp1, bp2) => updateBlogPostCount(bp2.Blog)); OnRemoved((context, bp) => updateBlogPostCount(bp.Blog)); - OnPublished((context, bp) => ProcessSlug(bp)); - OnRemoved( (context, b) => blogPostService.Get(context.ContentItem.As()).ToList().ForEach( @@ -72,23 +68,5 @@ namespace Orchard.Blogs.Handlers { } Localizer T { get; set; } - - private void ProcessSlug(BlogPost post) { - _routableService.FillSlug(post.As()); - - var slugsLikeThis = _blogPostService.Get(post.Blog, VersionOptions.Published).Where( - p => p.Slug.StartsWith(post.Slug, StringComparison.OrdinalIgnoreCase) && - p.Id != post.Id).Select(p => p.Slug); - - //todo: (heskew) need better messages - if (slugsLikeThis.Count() > 0) { - //todo: (heskew) need better messages - var originalSlug = post.Slug; - post.Slug = _routableService.GenerateUniqueSlug(post.Slug, slugsLikeThis); - - if (originalSlug != post.Slug) - _orchardServices.Notifier.Warning(T("A different blog post is already published with this same slug ({0}) so a unique slug ({1}) was generated for this post.", originalSlug, post.Slug)); - } - } } } \ No newline at end of file diff --git a/src/Orchard.Web/Modules/Orchard.Pages/Drivers/PageDriver.cs b/src/Orchard.Web/Modules/Orchard.Pages/Drivers/PageDriver.cs index 32b92971a..c06589e1f 100644 --- a/src/Orchard.Web/Modules/Orchard.Pages/Drivers/PageDriver.cs +++ b/src/Orchard.Web/Modules/Orchard.Pages/Drivers/PageDriver.cs @@ -76,74 +76,11 @@ namespace Orchard.Pages.Drivers { protected override DriverResult Editor(Page page, IUpdateModel updater) { updater.TryUpdateModel(page, Prefix, null, null); - //todo: (heskew) something better needs to be done with this...still feels shoehorned in here - ProcessSlug(page, updater); - DateTime scheduled; if (DateTime.TryParse(string.Format("{0} {1}", page.ScheduledPublishUtcDate, page.ScheduledPublishUtcTime), out scheduled)) page.ScheduledPublishUtc = scheduled; return Editor(page); } - - private void ProcessSlug(Page page, IUpdateModel updater) { - _routableService.FillSlug(page.As(), Slugify); - - if (string.IsNullOrEmpty(page.Slug)) { - return; - - // OR - - //updater.AddModelError("Routable.Slug", T("The slug is required.").ToString()); - //return; - } - - if (!Regex.IsMatch(page.Slug, @"^[^/:?#\[\]@!$&'()*+,;=\s](?(?=/)/[^/:?#\[\]@!$&'()*+,;=\s]|[^:?#\[\]@!$&'()*+,;=\s])*$")) { - updater.AddModelError("Routable.Slug", T("Please do not use any of the following characters in your slugs: \":\", \"?\", \"#\", \"[\", \"]\", \"@\", \"!\", \"$\", \"&\", \"'\", \"(\", \")\", \"*\", \"+\", \",\", \";\", \"=\". No spaces are allowed (please use dashes or underscores instead).").ToString()); - return; - } - - var slugsLikeThis = _pageService.Get(PageStatus.Published).Where( - p => p.Slug.StartsWith(page.Slug, StringComparison.OrdinalIgnoreCase) && - p.Id != page.Id).Select(p => p.Slug); - - if (slugsLikeThis.Count() > 0) { - //todo: (heskew) need better messages - Services.Notifier.Warning(T("A different page is already published with this same slug.")); - - if (page.ContentItem.VersionRecord == null || page.ContentItem.VersionRecord.Published) { - var originalSlug = page.Slug; - //todo: (heskew) make auto-uniqueness optional - page.Slug = _routableService.GenerateUniqueSlug(page.Slug, slugsLikeThis); - - //todo: (heskew) need better messages - if (originalSlug != page.Slug) - Services.Notifier.Warning(T("Slugs in conflict. \"{0}\" is already set for a previously published page so this page now has the slug \"{1}\"", - originalSlug, page.Slug)); - } - } - } - - private static string Slugify(string value) - { - if (!string.IsNullOrEmpty(value)) - { - //todo: (heskew) improve - just doing multi-pass regex replaces for now with the simple rules of - // (1) can't begin with a '/', (2) can't have adjacent '/'s and (3) can't have these characters - var startsoffbad = new Regex(@"^[\s/]+"); - var slashhappy = new Regex("/{2,}"); - var dissallowed = new Regex(@"[:?#\[\]@!$&'()*+,;=\s]+"); - - value = startsoffbad.Replace(value, "-"); - value = slashhappy.Replace(value, "/"); - value = dissallowed.Replace(value, "-"); - value = value.Trim('-'); - - if (value.Length > 1000) - value = value.Substring(0, 1000); - } - - return value; - } } } \ No newline at end of file diff --git a/src/Orchard.Web/Modules/Orchard.Pages/Handlers/PageHandler.cs b/src/Orchard.Web/Modules/Orchard.Pages/Handlers/PageHandler.cs index f000eb348..6b3b2e7a9 100644 --- a/src/Orchard.Web/Modules/Orchard.Pages/Handlers/PageHandler.cs +++ b/src/Orchard.Web/Modules/Orchard.Pages/Handlers/PageHandler.cs @@ -15,12 +15,10 @@ namespace Orchard.Pages.Handlers { [UsedImplicitly] public class PageHandler : ContentHandler { private readonly IPageService _pageService; - private readonly IRoutableService _routableService; private readonly IOrchardServices _orchardServices; - public PageHandler(IPageService pageService, IRoutableService routableService, IOrchardServices orchardServices) { + public PageHandler(IPageService pageService, IOrchardServices orchardServices) { _pageService = pageService; - _routableService = routableService; _orchardServices = orchardServices; T = NullLocalizer.Instance; @@ -31,27 +29,8 @@ namespace Orchard.Pages.Handlers { Filters.Add(new ActivatingFilter(PageDriver.ContentType.Name)); OnLoaded((context, p) => p.ScheduledPublishUtc = _pageService.GetScheduledPublishUtc(p)); - OnPublished((context, p) => ProcessSlug(p)); } Localizer T { get; set; } - - private void ProcessSlug(Page page) { - _routableService.FillSlug(page.As()); - - var slugsLikeThis = _pageService.Get(PageStatus.Published).Where( - p => p.Slug.StartsWith(page.Slug, StringComparison.OrdinalIgnoreCase) && - p.Id != page.Id).Select(p => p.Slug); - - //todo: (heskew) need better messages - if (slugsLikeThis.Count() > 0) { - //todo: (heskew) need better messages - var originalSlug = page.Slug; - page.Slug = _routableService.GenerateUniqueSlug(page.Slug, slugsLikeThis); - - if (originalSlug != page.Slug) - _orchardServices.Notifier.Warning(T("A different page is already published with this same slug ({0}) so a unique slug ({1}) was generated for this page.", originalSlug, page.Slug)); - } - } } } \ No newline at end of file From 2647c1f8d445e44df1ba0ad2fed02d0adef17ece Mon Sep 17 00:00:00 2001 From: Sebastien Ros Date: Tue, 20 Apr 2010 17:59:09 -0700 Subject: [PATCH 08/14] Added unit tests on slugs generation unicity among the sae content types --- .../Common/Services/RoutableServiceTests.cs | 89 +++++++++++++++++++ .../Orchard.Core.Tests.csproj | 3 + .../Core/Common/Drivers/RoutableDriver.cs | 8 +- .../Common/Handlers/RoutableAspectHandler.cs | 2 +- .../Core/Common/Services/IRoutableService.cs | 3 +- .../Core/Common/Services/RoutableService.cs | 20 ++--- 6 files changed, 110 insertions(+), 15 deletions(-) diff --git a/src/Orchard.Core.Tests/Common/Services/RoutableServiceTests.cs b/src/Orchard.Core.Tests/Common/Services/RoutableServiceTests.cs index 5a3588b21..b587f0c54 100644 --- a/src/Orchard.Core.Tests/Common/Services/RoutableServiceTests.cs +++ b/src/Orchard.Core.Tests/Common/Services/RoutableServiceTests.cs @@ -10,6 +10,10 @@ using Orchard.ContentManagement.Records; using Orchard.Core.Common.Models; using Orchard.Core.Common.Services; using Orchard.Tests.Modules; +using Orchard.Core.Common.Handlers; +using System.Web.Mvc; +using System.Web.Routing; +using Orchard.Tests.Stubs; namespace Orchard.Core.Tests.Common.Services { [TestFixture] @@ -23,7 +27,13 @@ namespace Orchard.Core.Tests.Common.Services { public override void Register(ContainerBuilder builder) { builder.RegisterType().As(); builder.RegisterType().As(); + builder.RegisterType().As(); builder.RegisterType().As(); + + builder.RegisterType().As(); + builder.RegisterInstance(new UrlHelper(new RequestContext(new StubHttpContext("~/"), new RouteData()))).As(); + builder.RegisterType().As(); + } private IRoutableService _routableService; @@ -111,6 +121,47 @@ namespace Orchard.Core.Tests.Common.Services { Assert.That(thing.Slug, Is.EqualTo("this-is-some-interesting-title")); } + [Test] + public void GeneratedSlugsShouldBeUniqueAmongContentType() + { + var contentManager = _container.Resolve(); + + var thing1 = contentManager.Create(ThingDriver.ContentType.Name, t => + { + t.As().Record = new RoutableRecord(); + t.Title = "This Is Some Interesting Title"; + }); + + var thing2 = contentManager.Create(ThingDriver.ContentType.Name , t => + { + t.As().Record = new RoutableRecord(); + t.Title = "This Is Some Interesting Title"; + }); + + Assert.AreNotEqual(thing1.Slug, thing2.Slug); + } + + [Test] + public void SlugsCanBeDuplicatedAccrossContentTypes() + { + var contentManager = _container.Resolve(); + + var thing = contentManager.Create(ThingDriver.ContentType.Name, t => + { + t.As().Record = new RoutableRecord(); + t.Title = "This Is Some Interesting Title"; + }); + + var stuff = contentManager.Create(StuffDriver.ContentType.Name, s => + { + s.As().Record = new RoutableRecord(); + s.Title = "This Is Some Interesting Title"; + }); + + Assert.AreEqual(thing.Slug, stuff.Slug); + } + + protected override IEnumerable DatabaseTypes { get { return new[] { @@ -155,5 +206,43 @@ namespace Orchard.Core.Tests.Common.Services { DisplayName = "Thing" }; } + + [UsedImplicitly] + public class StuffHandler : ContentHandler + { + public StuffHandler() + { + Filters.Add(new ActivatingFilter(StuffDriver.ContentType.Name)); + Filters.Add(new ActivatingFilter>(StuffDriver.ContentType.Name)); + Filters.Add(new ActivatingFilter(StuffDriver.ContentType.Name)); + Filters.Add(new ActivatingFilter(StuffDriver.ContentType.Name)); + } + } + + public class Stuff : ContentPart + { + public int Id { get { return ContentItem.Id; } } + + public string Title + { + get { return this.As().Title; } + set { this.As().Title = value; } + } + + public string Slug + { + get { return this.As().Slug; } + set { this.As().Slug = value; } + } + } + + public class StuffDriver : ContentItemDriver + { + public readonly static ContentType ContentType = new ContentType + { + Name = "stuff", + DisplayName = "Stuff" + }; + } } } \ No newline at end of file diff --git a/src/Orchard.Core.Tests/Orchard.Core.Tests.csproj b/src/Orchard.Core.Tests/Orchard.Core.Tests.csproj index 490fa4e25..7eb333c40 100644 --- a/src/Orchard.Core.Tests/Orchard.Core.Tests.csproj +++ b/src/Orchard.Core.Tests/Orchard.Core.Tests.csproj @@ -60,6 +60,9 @@ ..\..\lib\sqlite\System.Data.SQLite.DLL True + + 3.5 + False ..\..\lib\aspnetmvc\System.Web.Mvc.dll diff --git a/src/Orchard.Web/Core/Common/Drivers/RoutableDriver.cs b/src/Orchard.Web/Core/Common/Drivers/RoutableDriver.cs index 55f46c473..1abaf162e 100644 --- a/src/Orchard.Web/Core/Common/Drivers/RoutableDriver.cs +++ b/src/Orchard.Web/Core/Common/Drivers/RoutableDriver.cs @@ -5,6 +5,7 @@ using Orchard.Core.Common.Models; using Orchard.Core.Common.ViewModels; using Orchard.Core.Common.Services; using Orchard.Localization; +using Orchard.UI.Notify; namespace Orchard.Core.Common.Drivers { [UsedImplicitly] @@ -40,7 +41,12 @@ namespace Orchard.Core.Common.Drivers { updater.AddModelError("Routable.Slug", T("Please do not use any of the following characters in your slugs: \"/\", \":\", \"?\", \"#\", \"[\", \"]\", \"@\", \"!\", \"$\", \"&\", \"'\", \"(\", \")\", \"*\", \"+\", \",\", \";\", \"=\". No spaces are allowed (please use dashes or underscores instead).").ToString()); } - _routableService.ProcessSlug(part); + string originalSlug = part.Slug; + if(!_routableService.ProcessSlug(part)) { + _services.Notifier.Warning(T("Slugs in conflict. \"{0}\" is already set for a previously created {2} so now it has the slug \"{1}\"", + originalSlug, part.Slug, part.ContentItem.ContentType)); + } + return ContentPartTemplate(model, TemplateName, Prefix).Location("primary", "before.5"); } diff --git a/src/Orchard.Web/Core/Common/Handlers/RoutableAspectHandler.cs b/src/Orchard.Web/Core/Common/Handlers/RoutableAspectHandler.cs index b1e4244a0..8f654378b 100644 --- a/src/Orchard.Web/Core/Common/Handlers/RoutableAspectHandler.cs +++ b/src/Orchard.Web/Core/Common/Handlers/RoutableAspectHandler.cs @@ -34,7 +34,7 @@ namespace Orchard.Core.Common.Handlers { routable.ContentItemBasePath = url; }); - OnPublished((context, bp) => routableService.ProcessSlug(bp)); + OnCreated((context, ra) => routableService.ProcessSlug(ra)); } diff --git a/src/Orchard.Web/Core/Common/Services/IRoutableService.cs b/src/Orchard.Web/Core/Common/Services/IRoutableService.cs index deb7b8476..65f0fac1a 100644 --- a/src/Orchard.Web/Core/Common/Services/IRoutableService.cs +++ b/src/Orchard.Web/Core/Common/Services/IRoutableService.cs @@ -21,7 +21,8 @@ namespace Orchard.Core.Common.Services { /// /// Defines the slug of a RoutableAspect and validate its unicity /// - void ProcessSlug(RoutableAspect part); + /// True if the slug has been created, False if a conflict occured + bool ProcessSlug(RoutableAspect part); } } \ No newline at end of file diff --git a/src/Orchard.Web/Core/Common/Services/RoutableService.cs b/src/Orchard.Web/Core/Common/Services/RoutableService.cs index 1ebad4869..5b901ae14 100644 --- a/src/Orchard.Web/Core/Common/Services/RoutableService.cs +++ b/src/Orchard.Web/Core/Common/Services/RoutableService.cs @@ -11,14 +11,10 @@ using Orchard.UI.Notify; namespace Orchard.Core.Common.Services { [UsedImplicitly] public class RoutableService : IRoutableService { - private readonly IOrchardServices _services; private readonly IContentManager _contentManager; - private Localizer T { get; set; } - public RoutableService(IOrchardServices services, IContentManager contentManager) { - _services = services; + public RoutableService(IContentManager contentManager) { _contentManager = contentManager; - T = NullLocalizer.Instance; } public void FillSlug(TModel model) where TModel : RoutableAspect { @@ -71,10 +67,9 @@ namespace Orchard.Core.Common.Services { { return _contentManager.Query(contentType).Join() - .Where(rr => rr.Slug.StartsWith(slug, StringComparison.OrdinalIgnoreCase)) .List() - .Cast() - .Select(i => i.Slug) + .Select(i => i.As().Slug) + .Where(rr => rr.StartsWith(slug, StringComparison.OrdinalIgnoreCase)) // todo: for some reason the filter doesn't work within the query, even without StringComparison or StartsWith .ToArray(); } @@ -82,13 +77,13 @@ namespace Orchard.Core.Common.Services { return slug == null || String.IsNullOrEmpty(slug.Trim()) || !Regex.IsMatch(slug, @"^[^/:?#\[\]@!$&'()*+,;=\s]+$"); } - public void ProcessSlug(RoutableAspect part) + public bool ProcessSlug(RoutableAspect part) { FillSlug(part); if (string.IsNullOrEmpty(part.Slug)) { - return; + return true; } var slugsLikeThis = GetSimilarSlugs(part.ContentItem.ContentType, part.Slug); @@ -101,10 +96,11 @@ namespace Orchard.Core.Common.Services { part.Slug = GenerateUniqueSlug(part.Slug, slugsLikeThis); if (originalSlug != part.Slug) { - _services.Notifier.Warning(T("Slugs in conflict. \"{0}\" is already set for a previously created {2} so now it has the slug \"{1}\"", - originalSlug, part.Slug, part.ContentItem.ContentType)); + return false; } } + + return true; } } } \ No newline at end of file From 3604a52d74fcc67955fb123ecd1b6b2dc265107e Mon Sep 17 00:00:00 2001 From: Sebastien Ros Date: Wed, 21 Apr 2010 10:01:57 -0700 Subject: [PATCH 09/14] Corrected slugs validation and added unit tests --- .../Common/Services/RoutableServiceTests.cs | 18 +++++++ .../Core/Common/Services/RoutableService.cs | 3 +- .../Orchard.Blogs/Drivers/BlogPostDriver.cs | 49 +------------------ 3 files changed, 21 insertions(+), 49 deletions(-) diff --git a/src/Orchard.Core.Tests/Common/Services/RoutableServiceTests.cs b/src/Orchard.Core.Tests/Common/Services/RoutableServiceTests.cs index b587f0c54..1a3b18f47 100644 --- a/src/Orchard.Core.Tests/Common/Services/RoutableServiceTests.cs +++ b/src/Orchard.Core.Tests/Common/Services/RoutableServiceTests.cs @@ -52,6 +52,24 @@ namespace Orchard.Core.Tests.Common.Services { Assert.That(thing.Slug, Is.EqualTo("please-do-not-use-any-of-the-following-characters-in-your-slugs-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"")); } + [Test] + public void EmptySlugsShouldBeConsideredValid() { + // so that automatic generation on Publish occurs + Assert.That(_routableService.IsSlugValid(null), Is.True); + Assert.That(_routableService.IsSlugValid(String.Empty), Is.True); + Assert.That(_routableService.IsSlugValid(" "), Is.True); + } + + [Test] + public void InvalidCharacterShouldBeRefusedInSlugs() { + Assert.That(_routableService.IsSlugValid("aaaa-_aaaa"), Is.True); + + foreach (var c in @"/:?#[]@!$&'()*+,;= ") { + Assert.That(_routableService.IsSlugValid("a" + c + "b"), Is.False); + } + } + + [Test] public void VeryLongStringTruncatedTo1000Chars() { var contentManager = _container.Resolve(); diff --git a/src/Orchard.Web/Core/Common/Services/RoutableService.cs b/src/Orchard.Web/Core/Common/Services/RoutableService.cs index 5b901ae14..cfddb58b5 100644 --- a/src/Orchard.Web/Core/Common/Services/RoutableService.cs +++ b/src/Orchard.Web/Core/Common/Services/RoutableService.cs @@ -74,7 +74,8 @@ namespace Orchard.Core.Common.Services { } public bool IsSlugValid(string slug) { - return slug == null || String.IsNullOrEmpty(slug.Trim()) || !Regex.IsMatch(slug, @"^[^/:?#\[\]@!$&'()*+,;=\s]+$"); + // see http://tools.ietf.org/html/rfc3987 for prohibited chars + return slug == null || String.IsNullOrEmpty(slug.Trim()) || Regex.IsMatch(slug, @"^[^/:?#\[\]@!$&'()*+,;=\s]+$"); } public bool ProcessSlug(RoutableAspect part) diff --git a/src/Orchard.Web/Modules/Orchard.Blogs/Drivers/BlogPostDriver.cs b/src/Orchard.Web/Modules/Orchard.Blogs/Drivers/BlogPostDriver.cs index adec3f1ff..8ac6ab501 100644 --- a/src/Orchard.Web/Modules/Orchard.Blogs/Drivers/BlogPostDriver.cs +++ b/src/Orchard.Web/Modules/Orchard.Blogs/Drivers/BlogPostDriver.cs @@ -1,19 +1,15 @@ using System; -using System.Linq; -using System.Text.RegularExpressions; using System.Web.Routing; using JetBrains.Annotations; using Orchard.Blogs.Models; using Orchard.Blogs.Services; using Orchard.ContentManagement; using Orchard.ContentManagement.Drivers; -using Orchard.Core.Common.Models; using Orchard.Core.Common.Services; using Orchard.Localization; -using Orchard.UI.Notify; namespace Orchard.Blogs.Drivers { - [UsedImplicitly] + [UsedImplicitly] public class BlogPostDriver : ContentItemDriver { public IOrchardServices Services { get; set; } private readonly IBlogPostService _blogPostService; @@ -78,54 +74,11 @@ namespace Orchard.Blogs.Drivers { protected override DriverResult Editor(BlogPost post, IUpdateModel updater) { updater.TryUpdateModel(post, Prefix, null, null); - //todo: (heskew) something better needs to be done with this...still feels shoehorned in here - ProcessSlug(post, updater); - DateTime scheduled; if (DateTime.TryParse(string.Format("{0} {1}", post.ScheduledPublishUtcDate, post.ScheduledPublishUtcTime), out scheduled)) post.ScheduledPublishUtc = scheduled; return Editor(post); } - - private void ProcessSlug(BlogPost post, IUpdateModel updater) { - _routableService.FillSlug(post.As()); - - if (string.IsNullOrEmpty(post.Slug)) { - return; - - // OR - - //updater.AddModelError("Routable.Slug", T("The slug is required.").ToString()); - //return; - } - - if (!Regex.IsMatch(post.Slug, @"^[^/:?#\[\]@!$&'()*+,;=\s]+$")) { - //todo: (heskew) get rid of the hard-coded prefix - updater.AddModelError("Routable.Slug", T("Please do not use any of the following characters in your slugs: \"/\", \":\", \"?\", \"#\", \"[\", \"]\", \"@\", \"!\", \"$\", \"&\", \"'\", \"(\", \")\", \"*\", \"+\", \",\", \";\", \"=\". No spaces are allowed (please use dashes or underscores instead).").ToString()); - return; - } - - var slugsLikeThis = _blogPostService.Get(post.Blog, VersionOptions.Published).Where( - p => p.Slug.StartsWith(post.Slug, StringComparison.OrdinalIgnoreCase) && - p.Id != post.Id).Select(p => p.Slug); - - //todo: (heskew) need better messages - if (slugsLikeThis.Count() > 0) { - //todo: (heskew) need better messages - Services.Notifier.Warning(T("A different blog post is already published with this same slug.")); - - if (post.ContentItem.VersionRecord == null || post.ContentItem.VersionRecord.Published) - { - var originalSlug = post.Slug; - //todo: (heskew) make auto-uniqueness optional - post.Slug = _routableService.GenerateUniqueSlug(post.Slug, slugsLikeThis); - - if (originalSlug != post.Slug) - Services.Notifier.Warning(T("Slugs in conflict. \"{0}\" is already set for a previously created blog post so this post now has the slug \"{1}\"", - originalSlug, post.Slug)); - } - } - } } } \ No newline at end of file From 5080860c77b3719001f03fa17037f858e7ef2be4 Mon Sep 17 00:00:00 2001 From: Sebastien Ros Date: Wed, 21 Apr 2010 15:12:23 -0700 Subject: [PATCH 10/14] [BUG 16267] Temporary fix for empty username bug while submiting comments. --- .../Controllers/CommentController.cs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/Orchard.Web/Modules/Orchard.Comments/Controllers/CommentController.cs b/src/Orchard.Web/Modules/Orchard.Comments/Controllers/CommentController.cs index b10ed9166..1f9bb75ef 100644 --- a/src/Orchard.Web/Modules/Orchard.Comments/Controllers/CommentController.cs +++ b/src/Orchard.Web/Modules/Orchard.Comments/Controllers/CommentController.cs @@ -9,6 +9,7 @@ using Orchard.Localization; using Orchard.Security; using Orchard.Settings; using Orchard.UI.Notify; +using Orchard.Utility.Extensions; namespace Orchard.Comments.Controllers { public class CommentController : Controller { @@ -42,7 +43,15 @@ namespace Orchard.Comments.Controllers { var viewModel = new CommentsCreateViewModel(); try { - UpdateModel(viewModel); + + // UpdateModel(viewModel); + + if(!TryUpdateModel(viewModel)) { + if (Request.Form["Name"].IsNullOrEmptyTrimmed()) { + _notifier.Error("You must provide a Name in order to comment"); + } + return Redirect(returnUrl); + } var context = new CreateCommentContext { Author = viewModel.Name, @@ -63,7 +72,8 @@ namespace Orchard.Comments.Controllers { } catch (Exception exception) { _notifier.Error(T("Creating Comment failed: " + exception.Message)); - return View(viewModel); + // return View(viewModel); + return Redirect(returnUrl); } } } From dd4c4f57302643b377c4a1f4b6398947c7343210 Mon Sep 17 00:00:00 2001 From: Renaud Paquay Date: Fri, 30 Apr 2010 15:00:10 -0700 Subject: [PATCH 11/14] Re-add Blog as one of the default feature to enable during setup --HG-- branch : dev --- .../Modules/Orchard.Setup/Controllers/SetupController.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Orchard.Web/Modules/Orchard.Setup/Controllers/SetupController.cs b/src/Orchard.Web/Modules/Orchard.Setup/Controllers/SetupController.cs index ec5667457..a1a0df2cf 100644 --- a/src/Orchard.Web/Modules/Orchard.Setup/Controllers/SetupController.cs +++ b/src/Orchard.Web/Modules/Orchard.Setup/Controllers/SetupController.cs @@ -82,6 +82,7 @@ namespace Orchard.Setup.Controllers { "Orchard.Themes", "Orchard.MultiTenancy", "Orchard.Pages", + "Orchard.Blogs", "Orchard.Comments" }; var setupContext = new SetupContext { From 3f70ab177fe2ce9576e13422fca51133f470da51 Mon Sep 17 00:00:00 2001 From: Renaud Paquay Date: Fri, 30 Apr 2010 19:49:58 -0700 Subject: [PATCH 12/14] Fix empty RouteData issue in IoC RequestContext Since we wrap the MvcHandler with our own handler, we need to dig down the RequestContext instance a bit deeper. --HG-- branch : dev --- src/Orchard/Mvc/IHasRequestContext.cs | 7 +++++++ src/Orchard/Mvc/MvcModule.cs | 6 ++++++ src/Orchard/Mvc/Routes/ShellRoute.cs | 10 ++++++++-- src/Orchard/Orchard.Framework.csproj | 1 + 4 files changed, 22 insertions(+), 2 deletions(-) create mode 100644 src/Orchard/Mvc/IHasRequestContext.cs diff --git a/src/Orchard/Mvc/IHasRequestContext.cs b/src/Orchard/Mvc/IHasRequestContext.cs new file mode 100644 index 000000000..db3217376 --- /dev/null +++ b/src/Orchard/Mvc/IHasRequestContext.cs @@ -0,0 +1,7 @@ +using System.Web.Routing; + +namespace Orchard.Mvc { + public interface IHasRequestContext { + RequestContext RequestContext { get; } + } +} \ No newline at end of file diff --git a/src/Orchard/Mvc/MvcModule.cs b/src/Orchard/Mvc/MvcModule.cs index 5adf8b0f5..c88c941b0 100644 --- a/src/Orchard/Mvc/MvcModule.cs +++ b/src/Orchard/Mvc/MvcModule.cs @@ -48,6 +48,12 @@ namespace Orchard.Mvc { return mvcHandler.RequestContext; } + var hasRequestContext = httpContext.Handler as IHasRequestContext; + if (hasRequestContext != null) { + if (hasRequestContext.RequestContext != null) + return hasRequestContext.RequestContext; + } + return new RequestContext(httpContext, new RouteData()); } diff --git a/src/Orchard/Mvc/Routes/ShellRoute.cs b/src/Orchard/Mvc/Routes/ShellRoute.cs index 3a0585bb4..a514e0c7e 100644 --- a/src/Orchard/Mvc/Routes/ShellRoute.cs +++ b/src/Orchard/Mvc/Routes/ShellRoute.cs @@ -122,7 +122,7 @@ namespace Orchard.Mvc.Routes { } } - class HttpHandler : IHttpHandler, IRequiresSessionState { + class HttpHandler : IHttpHandler, IRequiresSessionState, IHasRequestContext { protected readonly ContainerProvider _containerProvider; private readonly IHttpHandler _httpHandler; @@ -144,6 +144,13 @@ namespace Orchard.Mvc.Routes { _containerProvider.EndRequestLifetime(); } } + + public RequestContext RequestContext { + get { + var mvcHandler = _httpHandler as MvcHandler; + return mvcHandler == null ? null : mvcHandler.RequestContext; + } + } } class HttpAsyncHandler : HttpHandler, IHttpAsyncHandler { @@ -173,7 +180,6 @@ namespace Orchard.Mvc.Routes { _containerProvider.EndRequestLifetime(); } } - } } } diff --git a/src/Orchard/Orchard.Framework.csproj b/src/Orchard/Orchard.Framework.csproj index cba9e445b..2de8742aa 100644 --- a/src/Orchard/Orchard.Framework.csproj +++ b/src/Orchard/Orchard.Framework.csproj @@ -197,6 +197,7 @@ + From 16cc07cf7a4eadc9b66a500da75a5ce36da15f04 Mon Sep 17 00:00:00 2001 From: Renaud Paquay Date: Fri, 30 Apr 2010 20:17:54 -0700 Subject: [PATCH 13/14] Temporary fix until we get feature activation complete In order to load the list of slugs for pages and blogs posts, we need to call "HackSimulateExtensionActivation" when a shell is activated. We had lost this call with the multi-tenancy work. --HG-- branch : dev --- src/Orchard/Environment/DefaultOrchardHost.cs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/Orchard/Environment/DefaultOrchardHost.cs b/src/Orchard/Environment/DefaultOrchardHost.cs index 35a1f394b..97d62fa8c 100644 --- a/src/Orchard/Environment/DefaultOrchardHost.cs +++ b/src/Orchard/Environment/DefaultOrchardHost.cs @@ -83,18 +83,22 @@ namespace Orchard.Environment { return allSettings.Select( settings => { var context = CreateShellContext(settings); - context.Shell.Activate(); - _runningShellTable.Add(settings); + ActivateShell(context); return context; }); } var setupContext = CreateSetupContext(); - setupContext.Shell.Activate(); - _runningShellTable.Add(setupContext.Settings); + ActivateShell(setupContext); return new[] { setupContext }; } + private void ActivateShell(ShellContext context) { + context.Shell.Activate(); + _runningShellTable.Add(context.Settings); + HackSimulateExtensionActivation(context.LifetimeScope); + } + ShellContext CreateSetupContext() { Logger.Debug("Creating shell context for root setup"); return _shellContextFactory.CreateSetupContext(new ShellSettings { Name = "Default" }); From 11e1fef85a0f0f81ca1792ef5031df31d027e7be Mon Sep 17 00:00:00 2001 From: Renaud Paquay Date: Fri, 30 Apr 2010 21:56:35 -0700 Subject: [PATCH 14/14] Fix bug with blog post blog post container field wasn't set properly. This is due to a change in order of events between CommonAspect and BlogPost activation. As a temporary workound, change CommonAspect to force setting container and owner record fields if they have a value when Activated event is processed. --HG-- branch : dev --- .../Core/Common/Handlers/CommonAspectHandler.cs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/Orchard.Web/Core/Common/Handlers/CommonAspectHandler.cs b/src/Orchard.Web/Core/Common/Handlers/CommonAspectHandler.cs index a2f3d4366..99c652d12 100644 --- a/src/Orchard.Web/Core/Common/Handlers/CommonAspectHandler.cs +++ b/src/Orchard.Web/Core/Common/Handlers/CommonAspectHandler.cs @@ -129,6 +129,10 @@ namespace Orchard.Core.Common.Handlers { return user; }); + // Force call to setter if we had already set a value + if (aspect.OwnerField.Value != null) + aspect.OwnerField.Value = aspect.OwnerField.Value; + aspect.ContainerField.Setter(container => { if (container == null) { aspect.Record.Container = null; @@ -138,6 +142,10 @@ namespace Orchard.Core.Common.Handlers { } return container; }); + + // Force call to setter if we had already set a value + if (aspect.ContainerField.Value != null) + aspect.ContainerField.Value = aspect.ContainerField.Value; }