From e2fa610358f7b912d30c1c05e6f246bddba48446 Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 16 Jul 2013 18:23:20 +1000 Subject: [PATCH] Lots of work for validation. We have a different type of validation model in Umbraco where we still save content even if some things are invalid (we just don't publish) so we have to take all of this into account. We also have other rules where if it is new content but required fields like 'name' are empty we cannot continue to save. Also started working on dealing with server side validation errors for content fields (not just user defined fields). --- src/Umbraco.Core/StringExtensions.cs | 15 ++ src/Umbraco.Tests/Umbraco.Tests.csproj | 1 + .../directives/valpropertymsg.directive.js | 6 +- .../services/servervalidation.service.js | 173 +++++++++++--- .../views/content/contentedit.controller.js | 97 +++++--- .../views/directives/umb-content-name.html | 2 +- src/Umbraco.Web/Editors/ContentController.cs | 49 +++- src/Umbraco.Web/Editors/MediaController.cs | 3 +- src/Umbraco.Web/ModelStateExtensions.cs | 17 ++ .../Models/ContentEditing/ContentItemBasic.cs | 1 + .../ContentEditing/ContentItemDisplay.cs | 15 ++ .../Models/ContentEditing/ContentItemSave.cs | 6 +- .../ContentEditing/ContentPropertyDisplay.cs | 1 + .../ContentEditing/IHaveUploadedFiles.cs | 9 + .../Models/ContentEditing/MediaItemDisplay.cs | 1 + src/Umbraco.Web/Umbraco.Web.csproj | 2 + .../WebApi/Binders/ContentItemBaseBinder.cs | 41 +++- .../ContentItemValidationFilterAttribute.cs | 223 +++--------------- .../Filters/ContentItemValidationHelper.cs | 179 ++++++++++++++ .../WebApi/ValidationFilterAttribute.cs | 4 +- 20 files changed, 564 insertions(+), 281 deletions(-) create mode 100644 src/Umbraco.Web/Models/ContentEditing/IHaveUploadedFiles.cs create mode 100644 src/Umbraco.Web/WebApi/Filters/ContentItemValidationHelper.cs diff --git a/src/Umbraco.Core/StringExtensions.cs b/src/Umbraco.Core/StringExtensions.cs index 7f8cc95a13..dd4ced35d9 100644 --- a/src/Umbraco.Core/StringExtensions.cs +++ b/src/Umbraco.Core/StringExtensions.cs @@ -28,6 +28,21 @@ namespace Umbraco.Core [UmbracoWillObsolete("Do not use this constants. See IShortStringHelper.CleanStringForSafeAliasJavaScriptCode.")] public const string UmbracoInvalidFirstCharacters = "01234567890"; + /// + /// Returns a stream from a string + /// + /// + /// + internal static Stream GenerateStreamFromString(this string s) + { + var stream = new MemoryStream(); + var writer = new StreamWriter(stream); + writer.Write(s); + writer.Flush(); + stream.Position = 0; + return stream; + } + /// /// This will append the query string to the URL /// diff --git a/src/Umbraco.Tests/Umbraco.Tests.csproj b/src/Umbraco.Tests/Umbraco.Tests.csproj index 82aa5c480d..7e6cf80bcd 100644 --- a/src/Umbraco.Tests/Umbraco.Tests.csproj +++ b/src/Umbraco.Tests/Umbraco.Tests.csproj @@ -87,6 +87,7 @@ ..\packages\RhinoMocks.3.6.1\lib\net\Rhino.Mocks.dll + diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/valpropertymsg.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/valpropertymsg.directive.js index c3662690af..2fcd21d0ec 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/valpropertymsg.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/valpropertymsg.directive.js @@ -53,7 +53,7 @@ function valPropertyMsg(serverValidationService) { hasError = true; //update the validation message if we don't already have one assigned. if (showValidation && scope.errorMsg === "") { - scope.errorMsg = serverValidationService.getError(scope.currentProperty, ""); + scope.errorMsg = serverValidationService.getPropertyError(scope.currentProperty, ""); } } else { @@ -71,7 +71,7 @@ function valPropertyMsg(serverValidationService) { scope.$on("saving", function (ev, args) { showValidation = true; if (hasError && scope.errorMsg === "") { - scope.errorMsg = serverValidationService.getError(scope.currentProperty, ""); + scope.errorMsg = serverValidationService.getPropertyError(scope.currentProperty, ""); } else if (!hasError) { scope.errorMsg = ""; @@ -111,7 +111,7 @@ function valPropertyMsg(serverValidationService) { //error collection... it is a 'one-time' usage so that when the field is invalidated //again, the message we display is the client side message. //NOTE: 'this' in the subscribe callback context is the validation manager object. - this.removeError(scope.currentProperty); + this.removePropertyError(scope.currentProperty); //flag that the current validator is invalid formCtrl.$setValidity('valPropertyMsg', false); } diff --git a/src/Umbraco.Web.UI.Client/src/common/services/servervalidation.service.js b/src/Umbraco.Web.UI.Client/src/common/services/servervalidation.service.js index 322f9f1654..4584d8b3d5 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/servervalidation.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/servervalidation.service.js @@ -4,7 +4,9 @@ * @function * * @description - * used to handle server side validation and wires up the UI with the messages + * Used to handle server side validation and wires up the UI with the messages. There are 2 types of validation messages, one + * is for user defined properties (called Properties) and the other is for field properties which are attached to the native + * model objects (not user defined). The methods below are named according to these rules: Properties vs Fields. */ function serverValidationService() { @@ -23,35 +25,60 @@ function serverValidationService() { * a particular field, otherwise we can only pinpoint that there is an error for a content property, not the * property's specific field. This is used with the val-server directive in which the directive specifies the * field alias to listen for. + * If contentProperty is null, then this subscription is for a field property (not a user defined property) */ subscribe: function (contentProperty, fieldName, callback) { - if (!contentProperty || !callback) { + if (!callback) { return; } - //don't add it if it already exists - var exists = _.find(callbacks, function(item) { - return item.propertyAlias === contentProperty.alias && item.fieldName === fieldName; - }); - if (!exists) { - callbacks.push({ propertyAlias: contentProperty.alias, fieldName: fieldName, callback: callback }); - } + + if (contentProperty === null) { + //don't add it if it already exists + var exists1 = _.find(callbacks, function (item) { + return item.propertyAlias === null && item.fieldName === fieldName; + }); + if (!exists1) { + callbacks.push({ propertyAlias: contentProperty.alias, fieldName: fieldName, callback: callback }); + } + } + else if (contentProperty !== undefined) { + //don't add it if it already exists + var exists2 = _.find(callbacks, function (item) { + return item.propertyAlias === contentProperty.alias && item.fieldName === fieldName; + }); + if (!exists2) { + callbacks.push({ propertyAlias: contentProperty.alias, fieldName: fieldName, callback: callback }); + } + } }, unsubscribe: function(contentProperty, fieldName) { - if (!contentProperty) { - return; + + if (contentProperty === null) { + + //remove all callbacks for the content field + callbacks = _.reject(callbacks, function (item) { + return item.propertyAlias === null && item.fieldName === fieldName; + }); + } - callbacks = _.reject(callbacks, function (item) { - return item.propertyAlias === contentProperty.alias && - (item.fieldName === fieldName || - ((item.fieldName === undefined || item.fieldName === "") && (fieldName === undefined || fieldName === ""))); - }); + else if (contentProperty !== undefined) { + + //remove all callbacks for the content property + callbacks = _.reject(callbacks, function (item) { + return item.propertyAlias === contentProperty.alias && + (item.fieldName === fieldName || + ((item.fieldName === undefined || item.fieldName === "") && (fieldName === undefined || fieldName === ""))); + }); + } + + }, /** * @ngdoc function - * @name getCallbacks + * @name getPropertyCallbacks * @methodOf umbraco.services.serverValidationService * @function * @@ -60,7 +87,7 @@ function serverValidationService() { * This will always return any callbacks registered for just the property (i.e. field name is empty) and for ones with an * explicit field name set. */ - getCallbacks: function (contentProperty, fieldName) { + getPropertyCallbacks: function (contentProperty, fieldName) { var found = _.filter(callbacks, function (item) { //returns any callback that have been registered directly against the field and for only the property return (item.propertyAlias === contentProperty.alias && (item.fieldName === fieldName || (item.fieldName === undefined || item.fieldName === ""))); @@ -70,20 +97,75 @@ function serverValidationService() { /** * @ngdoc function - * @name addError + * @name getFieldCallbacks + * @methodOf umbraco.services.serverValidationService + * @function + * + * @description + * Gets all callbacks that has been registered using the subscribe method for the field. + */ + getFieldCallbacks: function (contentProperty, fieldName) { + var found = _.filter(callbacks, function (item) { + //returns any callback that have been registered directly against the field + return (item.propertyAlias === null && item.fieldName === fieldName); + }); + return found; + }, + + /** + * @ngdoc function + * @name addFieldError + * @methodOf umbraco.services.serverValidationService + * @function + * + * @description + * Adds an error message for a native content item field (not a user defined property, for Example, 'Name') + */ + addFieldError: function(fieldName, errorMsg) { + if (!fieldName) { + return; + } + + //only add the item if it doesn't exist + if (!this.hasFieldError(fieldName)) { + this.items.push({ + propertyAlias: null, + fieldName: fieldName, + errorMsg: errorMsg + }); + } + + //find all errors for this item + var errorsForCallback = _.filter(this.items, function (item) { + return (item.propertyAlias === null && item.fieldName === fieldName); + }); + //we should now call all of the call backs registered for this error + var cbs = this.getFieldCallbacks(fieldName); + //call each callback for this error + for (var cb in cbs) { + cbs[cb].callback.apply(this, [ + false, //pass in a value indicating it is invalid + errorsForCallback, //pass in the errors for this item + this.items]); //pass in all errors in total + } + }, + + /** + * @ngdoc function + * @name addPropertyError * @methodOf umbraco.services.serverValidationService * @function * * @description * Adds an error message for the content property */ - addError: function (contentProperty, fieldName, errorMsg) { + addPropertyError: function (contentProperty, fieldName, errorMsg) { if (!contentProperty) { return; } //only add the item if it doesn't exist - if (!this.hasError(contentProperty, fieldName)) { + if (!this.hasPropertyError(contentProperty, fieldName)) { this.items.push({ propertyAlias: contentProperty.alias, fieldName: fieldName, @@ -96,7 +178,7 @@ function serverValidationService() { return (item.propertyAlias === contentProperty.alias && item.fieldName === fieldName); }); //we should now call all of the call backs registered for this error - var cbs = this.getCallbacks(contentProperty, fieldName); + var cbs = this.getPropertyCallbacks(contentProperty, fieldName); //call each callback for this error for (var cb in cbs) { cbs[cb].callback.apply(this, [ @@ -108,14 +190,14 @@ function serverValidationService() { /** * @ngdoc function - * @name removeError + * @name removePropertyError * @methodOf umbraco.services.serverValidationService * @function * * @description * Removes an error message for the content property */ - removeError: function (contentProperty, fieldName) { + removePropertyError: function (contentProperty, fieldName) { if (!contentProperty) { return; @@ -147,14 +229,14 @@ function serverValidationService() { /** * @ngdoc function - * @name getError + * @name getPropertyError * @methodOf umbraco.services.serverValidationService * @function * * @description * Gets the error message for the content property */ - getError: function (contentProperty, fieldName) { + getPropertyError: function (contentProperty, fieldName) { var err = _.find(this.items, function (item) { //return true if the property alias matches and if an empty field name is specified or the field name matches return (item.propertyAlias === contentProperty.alias && (item.fieldName === fieldName || (fieldName === undefined || fieldName === ""))); @@ -165,14 +247,32 @@ function serverValidationService() { /** * @ngdoc function - * @name hasError + * @name getFieldError + * @methodOf umbraco.services.serverValidationService + * @function + * + * @description + * Gets the error message for a content field + */ + getFieldError: function (fieldName) { + var err = _.find(this.items, function (item) { + //return true if the property alias matches and if an empty field name is specified or the field name matches + return (item.propertyAlias === null && item.fieldName === fieldName); + }); + //return generic property error message if the error doesn't exist + return err ? err : "Field has errors"; + }, + + /** + * @ngdoc function + * @name hasPropertyError * @methodOf umbraco.services.serverValidationService * @function * * @description * Checks if the content property + field name combo has an error */ - hasError: function (contentProperty, fieldName) { + hasPropertyError: function (contentProperty, fieldName) { var err = _.find(this.items, function (item) { //return true if the property alias matches and if an empty field name is specified or the field name matches return (item.propertyAlias === contentProperty.alias && (item.fieldName === fieldName || (fieldName === undefined || fieldName === ""))); @@ -180,6 +280,23 @@ function serverValidationService() { return err ? true : false; }, + /** + * @ngdoc function + * @name hasFieldError + * @methodOf umbraco.services.serverValidationService + * @function + * + * @description + * Checks if a content field has an error + */ + hasFieldError: function (fieldName) { + var err = _.find(this.items, function (item) { + //return true if the property alias matches and if an empty field name is specified or the field name matches + return (item.propertyAlias === null && item.fieldName === fieldName); + }); + return err ? true : false; + }, + /** The array of error messages */ items: [] }; diff --git a/src/Umbraco.Web.UI.Client/src/views/content/contentedit.controller.js b/src/Umbraco.Web.UI.Client/src/views/content/contentedit.controller.js index eb9dccf2d2..46c9ce6b17 100644 --- a/src/Umbraco.Web.UI.Client/src/views/content/contentedit.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/content/contentedit.controller.js @@ -8,6 +8,61 @@ */ function ContentEditController($scope, $routeParams, contentResource, notificationsService, angularHelper, serverValidationService) { + + /** + * @ngdoc function + * @name handleValidationErrors + * @methodOf ContentEditController + * @function + * + * @description + * A function to handle the validation (ModelState) errors collection which will happen on a 403 error indicating validation errors + */ + function handleValidationErrors(modelState) { + //get a list of properties since they are contained in tabs + var allProps = []; + for (var i = 0; i < $scope.content.tabs.length; i++) { + for (var p = 0; p < $scope.content.tabs[i].properties.length; p++) { + allProps.push($scope.content.tabs[i].properties[p]); + } + } + + for (var e in modelState) { + //the alias in model state can be in dot notation which indicates + // * the first part is the content property alias + // * the second part is the field to which the valiation msg is associated with + //There will always be at least 2 parts since all model errors for properties are prefixed with "Properties" + var parts = e.split("."); + if (parts.length > 1) { + var propertyAlias = parts[1]; + + //find the content property for the current error + var contentProperty = _.find(allProps, function(item) { + return (item.alias === propertyAlias); + }); + + if (contentProperty) { + //if it contains 2 '.' then we will wire it up to a property's field + if (parts.length > 2) { + //add an error with a reference to the field for which the validation belongs too + serverValidationService.addPropertyError(contentProperty, parts[2], modelState[e][0]); + } + else { + //add a generic error for the property, no reference to a specific field + serverValidationService.addPropertyError(contentProperty, "", modelState[e][0]); + } + } + } + else { + //the parts are only 1, this means its not a property but a native content property + serverValidationService.addFieldError(parts[0], modelState[e][0]); + } + + //add to notifications + notificationsService.error("Validation", modelState[e][0]); + } + } + /** * @ngdoc function * @name handleSaveError @@ -17,55 +72,19 @@ function ContentEditController($scope, $routeParams, contentResource, notificati * @description * A function to handle what happens when we have validation issues from the server side */ - function handleSaveError(err) { //When the status is a 403 status, we have validation errors. //Otherwise the error is probably due to invalid data (i.e. someone mucking around with the ids or something). //Or, some strange server error if (err.status == 403) { //now we need to look through all the validation errors - if (err.data && err.data.ModelState) { - - //get a list of properties since they are contained in tabs - var allProps = []; - for (var i = 0; i < $scope.content.tabs.length; i++) { - for (var p = 0; p < $scope.content.tabs[i].properties.length; p++) { - allProps.push($scope.content.tabs[i].properties[p]); - } - } - - for (var e in err.data.ModelState) { - //the alias in model state can be in dot notation which indicates - // * the first part is the content property alias - // * the second part is the field to which the valiation msg is associated with - var parts = e.split("."); - var propertyAlias = parts[0]; - - //find the content property for the current error - var contentProperty = _.find(allProps, function (item) { - return (item.alias === propertyAlias); - }); - if (contentProperty) { - //if it contains a '.' then we will wire it up to a property's field - if (parts.length > 1) { - //add an error with a reference to the field for which the validation belongs too - serverValidationService.addError(contentProperty, parts[1], err.data.ModelState[e][0]); - } - else { - //add a generic error for the property, no reference to a specific field - serverValidationService.addError(contentProperty, "", err.data.ModelState[e][0]); - } - - //add to notifications - notificationsService.error("Validation", err.data.ModelState[e][0]); - } - } - + if (err.data && (err.data.ModelState)) { + + handleValidationErrors(err.data.ModelState); } } else { //TODO: Implement an overlay showing the full YSOD like we had in v5 - //alert("failed!"); notificationsService.error("Validation failed", err); } } diff --git a/src/Umbraco.Web.UI.Client/src/views/directives/umb-content-name.html b/src/Umbraco.Web.UI.Client/src/views/directives/umb-content-name.html index 6602e76e18..8b412eeb8e 100644 --- a/src/Umbraco.Web.UI.Client/src/views/directives/umb-content-name.html +++ b/src/Umbraco.Web.UI.Client/src/views/directives/umb-content-name.html @@ -1,6 +1,6 @@
- + Required
\ No newline at end of file diff --git a/src/Umbraco.Web/Editors/ContentController.cs b/src/Umbraco.Web/Editors/ContentController.cs index bbdfbd739f..f92e284c0e 100644 --- a/src/Umbraco.Web/Editors/ContentController.cs +++ b/src/Umbraco.Web/Editors/ContentController.cs @@ -4,6 +4,7 @@ using System.Net; using System.Net.Http; using System.Web.Http; using System.Web.Http.ModelBinding; +using Umbraco.Core; using Umbraco.Core.Logging; using Umbraco.Core.Models; using Umbraco.Core.Models.Editors; @@ -23,7 +24,6 @@ namespace Umbraco.Web.Editors /// The API controller used for editing content /// [PluginController("UmbracoApi")] - [ValidationFilter] public class ContentController : UmbracoAuthorizedJsonController { private readonly ContentModelMapper _contentModelMapper; @@ -95,7 +95,6 @@ namespace Umbraco.Web.Editors /// Saves content /// /// - [ContentItemValidationFilter(typeof(ContentItemValidationHelper))] [FileUploadCleanupFilter] public ContentItemDisplay PostSave( [ModelBinder(typeof(ContentItemBinder))] @@ -107,9 +106,43 @@ namespace Umbraco.Web.Editors // * any file attachments have been saved to their temporary location for us to use // * we have a reference to the DTO object and the persisted object + //We need to manually check the validation results here because: + // * We still need to save the entity even if there are validation value errors + // * Depending on if the entity is new, and if there are non property validation errors (i.e. the name is null) + // then we cannot continue saving, we can only display errors + // * If there are validation errors and they were attempting to publish, we can only save, NOT publish and display + // a message indicating this + // TODO: WE need to implement the above points! + + if (!ModelState.IsValid) + { + if (ModelState["Name"] != null && ModelState["Name"].Errors.Any() + && (contentItem.Action == ContentSaveAction.SaveNew || contentItem.Action == ContentSaveAction.PublishNew)) + { + //ok, so the absolute mandatory data is invalid and it's new, we cannot actually continue! + throw new HttpResponseException(Request.CreateErrorResponse(HttpStatusCode.Forbidden, ModelState)); + } + + //if the model state is not valid we cannot publish so change it to save + switch (contentItem.Action) + { + case ContentSaveAction.Publish: + contentItem.Action = ContentSaveAction.Save; + break; + case ContentSaveAction.PublishNew: + contentItem.Action = ContentSaveAction.SaveNew; + break; + } + } + //Now, we just need to save the data - contentItem.PersistedContent.Name = contentItem.Name; + //Don't update the name if it is empty + if (!contentItem.Name.IsNullOrWhiteSpace()) + { + contentItem.PersistedContent.Name = contentItem.Name; + } + //TODO: We'll need to save the new template, publishat, etc... values here //Save the property values @@ -152,7 +185,15 @@ namespace Umbraco.Web.Editors //return the updated model - return _contentModelMapper.ToContentItemDisplay(contentItem.PersistedContent); + var display = _contentModelMapper.ToContentItemDisplay(contentItem.PersistedContent); + //lasty, if it is not valid, add the modelstate to the outgoing object and throw a 403 + if (!ModelState.IsValid) + { + display.Errors = ModelState.ToErrorDictionary(); + throw new HttpResponseException(Request.CreateResponse(HttpStatusCode.Forbidden, display)); + } + + return display; } } diff --git a/src/Umbraco.Web/Editors/MediaController.cs b/src/Umbraco.Web/Editors/MediaController.cs index f9a183dd93..5e9dc789e7 100644 --- a/src/Umbraco.Web/Editors/MediaController.cs +++ b/src/Umbraco.Web/Editors/MediaController.cs @@ -111,8 +111,7 @@ namespace Umbraco.Web.Editors /// /// Saves content /// - /// - [ContentItemValidationFilter(typeof(ContentItemValidationHelper))] + /// [FileUploadCleanupFilter] public MediaItemDisplay PostSave( [ModelBinder(typeof(MediaItemBinder))] diff --git a/src/Umbraco.Web/ModelStateExtensions.cs b/src/Umbraco.Web/ModelStateExtensions.cs index d2980c5aab..964b7712b4 100644 --- a/src/Umbraco.Web/ModelStateExtensions.cs +++ b/src/Umbraco.Web/ModelStateExtensions.cs @@ -1,3 +1,5 @@ +using System; +using System.Collections.Generic; using System.Linq; using System.Web.Mvc; @@ -46,6 +48,21 @@ namespace Umbraco.Web // state.AddModelError("DataValidation", errorMessage); //} + public static IDictionary ToErrorDictionary(this System.Web.Http.ModelBinding.ModelStateDictionary modelState) + { + var modelStateError = new Dictionary(); + foreach (var keyModelStatePair in modelState) + { + var key = keyModelStatePair.Key; + var errors = keyModelStatePair.Value.Errors; + if (errors != null && errors.Count > 0) + { + modelStateError.Add(key, errors.Select(error => error.ErrorMessage)); + } + } + return modelStateError; + } + /// /// Serializes the ModelState to JSON for JavaScript to interrogate the errors /// diff --git a/src/Umbraco.Web/Models/ContentEditing/ContentItemBasic.cs b/src/Umbraco.Web/Models/ContentEditing/ContentItemBasic.cs index be12bab4fb..590b1a1cbd 100644 --- a/src/Umbraco.Web/Models/ContentEditing/ContentItemBasic.cs +++ b/src/Umbraco.Web/Models/ContentEditing/ContentItemBasic.cs @@ -4,6 +4,7 @@ using System.ComponentModel.DataAnnotations; using System.Runtime.Serialization; using Newtonsoft.Json; using Umbraco.Core.Models; +using Umbraco.Web.WebApi; namespace Umbraco.Web.Models.ContentEditing { diff --git a/src/Umbraco.Web/Models/ContentEditing/ContentItemDisplay.cs b/src/Umbraco.Web/Models/ContentEditing/ContentItemDisplay.cs index 0fc93d9502..a5856a1763 100644 --- a/src/Umbraco.Web/Models/ContentEditing/ContentItemDisplay.cs +++ b/src/Umbraco.Web/Models/ContentEditing/ContentItemDisplay.cs @@ -1,6 +1,9 @@ using System; +using System.Collections.Generic; using System.ComponentModel.DataAnnotations; using System.Runtime.Serialization; +using System.Web.Http; +using System.Web.Http.ModelBinding; using Umbraco.Core.Models; namespace Umbraco.Web.Models.ContentEditing @@ -8,11 +11,23 @@ namespace Umbraco.Web.Models.ContentEditing /// /// A model representing a content item to be displayed in the back office /// + [DataContract(Name = "content", Namespace = "")] public class ContentItemDisplay : TabbedContentItem { [DataMember(Name = "publishDate")] public DateTime? PublishDate { get; set; } + /// + /// This is used for validation of a content item. + /// + /// + /// A content item can be invalid but still be saved. This occurs when there's property validation errors, we will + /// still save the item but it cannot be published. So we need a way of returning validation errors as well as the + /// updated model. + /// + [DataMember(Name = "ModelState")] + public IDictionary Errors { get; set; } + } } \ No newline at end of file diff --git a/src/Umbraco.Web/Models/ContentEditing/ContentItemSave.cs b/src/Umbraco.Web/Models/ContentEditing/ContentItemSave.cs index 6d160cbeb8..b462c75280 100644 --- a/src/Umbraco.Web/Models/ContentEditing/ContentItemSave.cs +++ b/src/Umbraco.Web/Models/ContentEditing/ContentItemSave.cs @@ -6,14 +6,10 @@ using Umbraco.Core.Models; namespace Umbraco.Web.Models.ContentEditing { - public interface IHaveUploadedFiles - { - List UploadedFiles { get; } - } - /// /// A model representing a content item to be saved /// + [DataContract(Name = "content", Namespace = "")] public class ContentItemSave : ContentItemBasic, IHaveUploadedFiles where TPersisted : IContentBase { diff --git a/src/Umbraco.Web/Models/ContentEditing/ContentPropertyDisplay.cs b/src/Umbraco.Web/Models/ContentEditing/ContentPropertyDisplay.cs index 9123432993..ac01601e27 100644 --- a/src/Umbraco.Web/Models/ContentEditing/ContentPropertyDisplay.cs +++ b/src/Umbraco.Web/Models/ContentEditing/ContentPropertyDisplay.cs @@ -7,6 +7,7 @@ namespace Umbraco.Web.Models.ContentEditing /// /// Represents a content property that is displayed in the UI /// + [DataContract(Name = "property", Namespace = "")] public class ContentPropertyDisplay : ContentPropertyBasic { [DataMember(Name = "label", IsRequired = true)] diff --git a/src/Umbraco.Web/Models/ContentEditing/IHaveUploadedFiles.cs b/src/Umbraco.Web/Models/ContentEditing/IHaveUploadedFiles.cs new file mode 100644 index 0000000000..9c96176063 --- /dev/null +++ b/src/Umbraco.Web/Models/ContentEditing/IHaveUploadedFiles.cs @@ -0,0 +1,9 @@ +using System.Collections.Generic; + +namespace Umbraco.Web.Models.ContentEditing +{ + public interface IHaveUploadedFiles + { + List UploadedFiles { get; } + } +} \ No newline at end of file diff --git a/src/Umbraco.Web/Models/ContentEditing/MediaItemDisplay.cs b/src/Umbraco.Web/Models/ContentEditing/MediaItemDisplay.cs index 5900aa756d..956194034b 100644 --- a/src/Umbraco.Web/Models/ContentEditing/MediaItemDisplay.cs +++ b/src/Umbraco.Web/Models/ContentEditing/MediaItemDisplay.cs @@ -7,6 +7,7 @@ namespace Umbraco.Web.Models.ContentEditing /// /// A model representing a content item to be displayed in the back office /// + [DataContract(Name = "content", Namespace = "")] public class MediaItemDisplay : TabbedContentItem { diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index 1116a4b9b1..43d3725b83 100644 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -306,6 +306,7 @@ + @@ -466,6 +467,7 @@ + diff --git a/src/Umbraco.Web/WebApi/Binders/ContentItemBaseBinder.cs b/src/Umbraco.Web/WebApi/Binders/ContentItemBaseBinder.cs index 8ea7e3df66..9c0c589b79 100644 --- a/src/Umbraco.Web/WebApi/Binders/ContentItemBaseBinder.cs +++ b/src/Umbraco.Web/WebApi/Binders/ContentItemBaseBinder.cs @@ -1,16 +1,29 @@ -using System.IO; +using System; +using System.ComponentModel; +using System.IO; using System.Linq; using System.Net; using System.Net.Http; +using System.Net.Http.Formatting; +using System.Text; using System.Threading.Tasks; using System.Web; using System.Web.Http; using System.Web.Http.Controllers; -using System.Web.Http.ModelBinding; +using System.Web.Http.ModelBinding.Binders; +using System.Web.Http.Validation; +using System.Web.ModelBinding; using Newtonsoft.Json; +using Newtonsoft.Json.Serialization; using Umbraco.Core; using Umbraco.Core.Models; using Umbraco.Web.Models.ContentEditing; +using Umbraco.Web.WebApi.Filters; +using IModelBinder = System.Web.Http.ModelBinding.IModelBinder; +using ModelBindingContext = System.Web.Http.ModelBinding.ModelBindingContext; +using ModelMetadata = System.Web.Http.Metadata.ModelMetadata; +using ModelMetadataProvider = System.Web.Http.Metadata.ModelMetadataProvider; +using MutableObjectModelBinder = System.Web.Http.ModelBinding.Binders.MutableObjectModelBinder; using Task = System.Threading.Tasks.Task; namespace Umbraco.Web.WebApi.Binders @@ -45,13 +58,18 @@ namespace Umbraco.Web.WebApi.Binders Directory.CreateDirectory(root); var provider = new MultipartFormDataStreamProvider(root); - var task = Task.Run(() => GetModel(actionContext.Request, provider)) + var task = Task.Run(() => GetModel(actionContext, bindingContext, provider)) .ContinueWith(x => { if (x.IsFaulted && x.Exception != null) { throw x.Exception; } + + //now that everything is binded, validate the properties + var contentItemValidator = new ContentItemValidationHelper(ApplicationContext); + contentItemValidator.ValidateItem(actionContext, x.Result); + bindingContext.Model = x.Result; }); @@ -63,11 +81,14 @@ namespace Umbraco.Web.WebApi.Binders /// /// Builds the model from the request contents /// - /// + /// + /// /// /// - private async Task> GetModel(HttpRequestMessage request, MultipartFormDataStreamProvider provider) + private async Task> GetModel(HttpActionContext actionContext, ModelBindingContext bindingContext, MultipartFormDataStreamProvider provider) { + var request = actionContext.Request; + //IMPORTANT!!! We need to ensure the umbraco context here because this is running in an async thread UmbracoContext.EnsureContext(request.Properties["MS_HttpContext"] as HttpContextBase, ApplicationContext.Current); @@ -87,8 +108,14 @@ namespace Umbraco.Web.WebApi.Binders //get the string json from the request var contentItem = result.FormData["contentItem"]; - //transform the json into an object + //deserialize into our model var model = JsonConvert.DeserializeObject>(contentItem); + + //get the default body validator and validate the object + var bodyValidator = actionContext.ControllerContext.Configuration.Services.GetBodyModelValidator(); + var metadataProvider = actionContext.ControllerContext.Configuration.Services.GetModelMetadataProvider(); + //all validation errors will not contain a prefix + bodyValidator.Validate(model, typeof (ContentItemSave), metadataProvider, actionContext, ""); //get the files foreach (var file in result.FileData) @@ -150,7 +177,7 @@ namespace Umbraco.Web.WebApi.Binders /// /// private static void MapPropertyValuesFromSaved(ContentItemSave saveModel, ContentItemDto dto) - { + { foreach (var p in saveModel.Properties) { dto.Properties.Single(x => x.Alias == p.Alias).Value = p.Value; diff --git a/src/Umbraco.Web/WebApi/Filters/ContentItemValidationFilterAttribute.cs b/src/Umbraco.Web/WebApi/Filters/ContentItemValidationFilterAttribute.cs index 58a8c7a275..4aaf621e05 100644 --- a/src/Umbraco.Web/WebApi/Filters/ContentItemValidationFilterAttribute.cs +++ b/src/Umbraco.Web/WebApi/Filters/ContentItemValidationFilterAttribute.cs @@ -1,208 +1,49 @@ using System; using System.Collections.Generic; -using System.Linq; -using System.Net; -using System.Net.Http; using System.Web.Http.Controllers; using System.Web.Http.Filters; using System.Web.UI; -using Umbraco.Core; -using Umbraco.Core.Logging; -using Umbraco.Core.Models; using Umbraco.Core.PropertyEditors; -using Umbraco.Web.Models.ContentEditing; using Umbraco.Web.Models.Mapping; namespace Umbraco.Web.WebApi.Filters { - internal class ContentItemValidationHelper - where TPersisted : IContentBase - { - private readonly ApplicationContext _applicationContext; + ///// + ///// Validates the content item + ///// + ///// + ///// There's various validation happening here both value validation and structure validation + ///// to ensure that malicious folks are not trying to post invalid values or to invalid properties. + ///// + //internal class ContentItemValidationFilterAttribute : ActionFilterAttribute + //{ + // private readonly Type _helperType; + // private readonly dynamic _helper; - public ContentItemValidationHelper(ApplicationContext applicationContext) - { - _applicationContext = applicationContext; - } + // public ContentItemValidationFilterAttribute(Type helperType) + // { + // _helperType = helperType; + // _helper = Activator.CreateInstance(helperType); + // } - public ContentItemValidationHelper() - : this(ApplicationContext.Current) - { - - } + // /// + // /// Returns true so that other filters can execute along with this one + // /// + // public override bool AllowMultiple + // { + // get { return true; } + // } - public void ValidateItem(HttpActionContext actionContext) - { - var contentItem = actionContext.ActionArguments["contentItem"] as ContentItemSave; - if (contentItem == null) - { - actionContext.Response = actionContext.Request.CreateErrorResponse(HttpStatusCode.BadRequest, "No " + typeof(ContentItemSave) + " found in request"); - return; - } - - //now do each validation step - if (ValidateExistingContent(contentItem, actionContext) == false) return; - if (ValidateProperties(contentItem, actionContext) == false) return; - if (ValidateData(contentItem, actionContext) == false) return; - } - - /// - /// Ensure the content exists - /// - /// - /// - /// - private bool ValidateExistingContent(ContentItemBasic postedItem, HttpActionContext actionContext) - { - if (postedItem.PersistedContent == null) - { - var message = string.Format("content with id: {0} was not found", postedItem.Id); - actionContext.Response = actionContext.Request.CreateErrorResponse(HttpStatusCode.NotFound, message); - return false; - } - - return true; - } - - /// - /// Ensure all of the ids in the post are valid - /// - /// - /// - /// - //private bool ValidateProperties(ContentItemSave postedItem, ContentItemDto realItem, HttpActionContext actionContext) - private bool ValidateProperties(ContentItemBasic postedItem, HttpActionContext actionContext) - { - foreach (var p in postedItem.Properties) - { - //ensure the property actually exists in our server side properties - if (postedItem.ContentDto.Properties.Contains(p) == false) - { - //TODO: Do we return errors here ? If someone deletes a property whilst their editing then should we just - //save the property data that remains? Or inform them they need to reload... not sure. This problem exists currently too i think. - - var message = string.Format("property with id: {0} was not found", p.Id); - actionContext.Response = actionContext.Request.CreateErrorResponse(HttpStatusCode.NotFound, message); - return false; - } - } - return true; - } - - private bool ValidateData(ContentItemBasic postedItem, HttpActionContext actionContext) - { - foreach (var p in postedItem.ContentDto.Properties) - { - var editor = p.PropertyEditor; - if (editor == null) - { - var message = string.Format("The property editor with id: {0} was not found for property with id {1}", p.DataType.ControlId, p.Id); - LogHelper.Warn>(message); - //actionContext.Response = actionContext.Request.CreateErrorResponse(HttpStatusCode.NotFound, message); - //return false; - continue; - } - - //get the posted value for this property - var postedValue = postedItem.Properties.Single(x => x.Alias == p.Alias).Value; - - //get the pre-values for this property - var preValues = _applicationContext.Services.DataTypeService.GetPreValueAsString(p.DataType.Id); - - //TODO: when we figure out how to 'override' certain pre-value properties we'll either need to: - // * Combine the preValues with the overridden values stored with the document type property (but how to combine?) - // * Or, pass in the overridden values stored with the doc type property separately - - foreach (var v in editor.ValueEditor.Validators) - { - foreach (var result in v.Validate(postedValue, preValues, editor)) - { - //if there are no member names supplied then we assume that the validation message is for the overall property - // not a sub field on the property editor - if (!result.MemberNames.Any()) - { - //add a model state error for the entire property - actionContext.ModelState.AddModelError(p.Alias, result.ErrorMessage); - } - else - { - //there's assigned field names so we'll combine the field name with the property name - // so that we can try to match it up to a real sub field of this editor - foreach (var field in result.MemberNames) - { - actionContext.ModelState.AddModelError(string.Format("{0}.{1}", p.Alias, field), result.ErrorMessage); - } - } - } - } - - //Now we need to validate the property based on the PropertyType validation (i.e. regex and required) - // NOTE: These will become legacy once we have pre-value overrides. - if (p.IsRequired) - { - foreach (var result in p.PropertyEditor.ValueEditor.RequiredValidator.Validate(postedValue, "", preValues, editor)) - { - //add a model state error for the entire property - actionContext.ModelState.AddModelError(p.Alias, result.ErrorMessage); - } - } - - if (!p.ValidationRegExp.IsNullOrWhiteSpace()) - { - foreach (var result in p.PropertyEditor.ValueEditor.RegexValidator.Validate(postedValue, p.ValidationRegExp, preValues, editor)) - { - //add a model state error for the entire property - actionContext.ModelState.AddModelError(p.Alias, result.ErrorMessage); - } - } - } - - //create the response if there any errors - if (!actionContext.ModelState.IsValid) - { - actionContext.Response = actionContext.Request.CreateErrorResponse(HttpStatusCode.Forbidden, actionContext.ModelState); - } - - return actionContext.ModelState.IsValid; - } - } - - /// - /// Validates the content item - /// - /// - /// There's various validation happening here both value validation and structure validation - /// to ensure that malicious folks are not trying to post invalid values or to invalid properties. - /// - internal class ContentItemValidationFilterAttribute : ActionFilterAttribute - { - private readonly Type _helperType; - private dynamic _helper; - - public ContentItemValidationFilterAttribute(Type helperType) - { - _helperType = helperType; - _helper = Activator.CreateInstance(helperType); - } - - /// - /// Returns true so that other filters can execute along with this one - /// - public override bool AllowMultiple - { - get { return true; } - } - - /// - /// Performs the validation - /// - /// - public override void OnActionExecuting(HttpActionContext actionContext) - { - _helper.ValidateItem(actionContext); - } + // /// + // /// Performs the validation + // /// + // /// + // public override void OnActionExecuting(HttpActionContext actionContext) + // { + // _helper.ValidateItem(actionContext); + // } - } + //} } \ No newline at end of file diff --git a/src/Umbraco.Web/WebApi/Filters/ContentItemValidationHelper.cs b/src/Umbraco.Web/WebApi/Filters/ContentItemValidationHelper.cs new file mode 100644 index 0000000000..ff85558cad --- /dev/null +++ b/src/Umbraco.Web/WebApi/Filters/ContentItemValidationHelper.cs @@ -0,0 +1,179 @@ +using System.Linq; +using System.Net; +using System.Net.Http; +using System.Web.Http.Controllers; +using Umbraco.Core; +using Umbraco.Core.Logging; +using Umbraco.Core.Models; +using Umbraco.Web.Models.ContentEditing; + +namespace Umbraco.Web.WebApi.Filters +{ + /// + /// A validation helper class used with ContentItemValidationFilterAttribute to be shared between content, media, etc... + /// + /// + /// + /// If any severe errors occur then the response gets set to an error and execution will not continue. Property validation + /// errors will just be added to the ModelState. + /// + internal class ContentItemValidationHelper + where TPersisted : IContentBase + { + private readonly ApplicationContext _applicationContext; + + public ContentItemValidationHelper(ApplicationContext applicationContext) + { + _applicationContext = applicationContext; + } + + public ContentItemValidationHelper() + : this(ApplicationContext.Current) + { + + } + + public void ValidateItem(HttpActionContext actionContext, string argumentName) + { + var contentItem = actionContext.ActionArguments[argumentName] as ContentItemSave; + if (contentItem == null) + { + actionContext.Response = actionContext.Request.CreateErrorResponse(HttpStatusCode.BadRequest, "No " + typeof(ContentItemSave) + " found in request"); + return; + } + + ValidateItem(actionContext, contentItem); + + } + + public void ValidateItem(HttpActionContext actionContext, ContentItemSave contentItem) + { + //now do each validation step + if (ValidateExistingContent(contentItem, actionContext) == false) return; + if (ValidateProperties(contentItem, actionContext) == false) return; + if (ValidatePropertyData(contentItem, actionContext) == false) return; + } + + /// + /// Ensure the content exists + /// + /// + /// + /// + private bool ValidateExistingContent(ContentItemBasic postedItem, HttpActionContext actionContext) + { + if (postedItem.PersistedContent == null) + { + var message = string.Format("content with id: {0} was not found", postedItem.Id); + actionContext.Response = actionContext.Request.CreateErrorResponse(HttpStatusCode.NotFound, message); + return false; + } + + return true; + } + + /// + /// Ensure all of the ids in the post are valid + /// + /// + /// + /// + private bool ValidateProperties(ContentItemBasic postedItem, HttpActionContext actionContext) + { + foreach (var p in postedItem.Properties) + { + //ensure the property actually exists in our server side properties + if (postedItem.ContentDto.Properties.Contains(p) == false) + { + //TODO: Do we return errors here ? If someone deletes a property whilst their editing then should we just + //save the property data that remains? Or inform them they need to reload... not sure. This problem exists currently too i think. + + var message = string.Format("property with id: {0} was not found", p.Id); + actionContext.Response = actionContext.Request.CreateErrorResponse(HttpStatusCode.NotFound, message); + return false; + } + } + return true; + } + + /// + /// Validates the data for each property + /// + /// + /// + /// + /// + /// All property data validation goes into the modelstate with a prefix of "Properties" + /// + private bool ValidatePropertyData(ContentItemBasic postedItem, HttpActionContext actionContext) + { + foreach (var p in postedItem.ContentDto.Properties) + { + var editor = p.PropertyEditor; + if (editor == null) + { + var message = string.Format("The property editor with id: {0} was not found for property with id {1}", p.DataType.ControlId, p.Id); + LogHelper.Warn>(message); + //actionContext.Response = actionContext.Request.CreateErrorResponse(HttpStatusCode.NotFound, message); + //return false; + continue; + } + + //get the posted value for this property + var postedValue = postedItem.Properties.Single(x => x.Alias == p.Alias).Value; + + //get the pre-values for this property + var preValues = _applicationContext.Services.DataTypeService.GetPreValueAsString(p.DataType.Id); + + //TODO: when we figure out how to 'override' certain pre-value properties we'll either need to: + // * Combine the preValues with the overridden values stored with the document type property (but how to combine?) + // * Or, pass in the overridden values stored with the doc type property separately + + foreach (var v in editor.ValueEditor.Validators) + { + foreach (var result in v.Validate(postedValue, preValues, editor)) + { + //if there are no member names supplied then we assume that the validation message is for the overall property + // not a sub field on the property editor + if (!result.MemberNames.Any()) + { + //add a model state error for the entire property + actionContext.ModelState.AddModelError(string.Format("{0}.{1}", "Properties", p.Alias), result.ErrorMessage); + } + else + { + //there's assigned field names so we'll combine the field name with the property name + // so that we can try to match it up to a real sub field of this editor + foreach (var field in result.MemberNames) + { + actionContext.ModelState.AddModelError(string.Format("{0}.{1}.{2}", "Properties", p.Alias, field), result.ErrorMessage); + } + } + } + } + + //Now we need to validate the property based on the PropertyType validation (i.e. regex and required) + // NOTE: These will become legacy once we have pre-value overrides. + if (p.IsRequired) + { + foreach (var result in p.PropertyEditor.ValueEditor.RequiredValidator.Validate(postedValue, "", preValues, editor)) + { + //add a model state error for the entire property + actionContext.ModelState.AddModelError(string.Format("{0}.{1}", "Properties", p.Alias), result.ErrorMessage); + } + } + + if (!p.ValidationRegExp.IsNullOrWhiteSpace()) + { + foreach (var result in p.PropertyEditor.ValueEditor.RegexValidator.Validate(postedValue, p.ValidationRegExp, preValues, editor)) + { + //add a model state error for the entire property + actionContext.ModelState.AddModelError(string.Format("{0}.{1}", "Properties", p.Alias), result.ErrorMessage); + } + } + } + + return actionContext.ModelState.IsValid; + } + } +} \ No newline at end of file diff --git a/src/Umbraco.Web/WebApi/ValidationFilterAttribute.cs b/src/Umbraco.Web/WebApi/ValidationFilterAttribute.cs index 11c54f8e1d..07cabcbb16 100644 --- a/src/Umbraco.Web/WebApi/ValidationFilterAttribute.cs +++ b/src/Umbraco.Web/WebApi/ValidationFilterAttribute.cs @@ -1,10 +1,12 @@ -using System.Net; +using System.ComponentModel.DataAnnotations; +using System.Net; using System.Net.Http; using System.Web.Http.Controllers; using System.Web.Http.Filters; namespace Umbraco.Web.WebApi { + /// /// An action filter used to do basic validation against the model and return a result /// straight away if it fails.