From 9241e1fcda9b8bf32f845b88d7faddc5b0fa183d Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 31 Jul 2018 17:50:24 +1000 Subject: [PATCH] WIP Getting the content controller a bit further, now need to pull apart the content binder --- src/Umbraco.Core/Models/ContentBase.cs | 25 +----- src/Umbraco.Core/Models/IContentBase.cs | 9 +- .../Umbraco/config/lang/en_us.xml | 1 - .../Editors/Binders/ContentItemBaseBinder.cs | 27 +----- .../Editors/Binders/ContentItemBinder.cs | 1 + src/Umbraco.Web/Editors/ContentController.cs | 90 ++++++++----------- .../Editors/ContentControllerBase.cs | 9 +- src/Umbraco.Web/Umbraco.Web.csproj | 2 - 8 files changed, 49 insertions(+), 115 deletions(-) diff --git a/src/Umbraco.Core/Models/ContentBase.cs b/src/Umbraco.Core/Models/ContentBase.cs index 29e36829d2..d5d9fcfdac 100644 --- a/src/Umbraco.Core/Models/ContentBase.cs +++ b/src/Umbraco.Core/Models/ContentBase.cs @@ -376,30 +376,7 @@ namespace Umbraco.Core.Models #endregion #region Validation - - /// - public bool IsValid(string culture = "*") - { - culture = culture.NullOrWhiteSpaceAsNull(); - - if (culture == null) - { - if (Name.IsNullOrWhiteSpace()) return false; - return ValidateProperties(null).Length == 0; - } - - if (culture != "*") - { - var name = GetCultureName(culture); - if (name.IsNullOrWhiteSpace()) return false; - return ValidateProperties(culture).Length == 0; - } - - // 'all cultures' - // those that have a name are ok, those without a name... we don't validate - return AvailableCultures.All(c => ValidateProperties(c).Length == 0); - } - + /// public virtual Property[] ValidateProperties(string culture = "*") { diff --git a/src/Umbraco.Core/Models/IContentBase.cs b/src/Umbraco.Core/Models/IContentBase.cs index 3c56b2c737..2cf2d85024 100644 --- a/src/Umbraco.Core/Models/IContentBase.cs +++ b/src/Umbraco.Core/Models/IContentBase.cs @@ -137,14 +137,7 @@ namespace Umbraco.Core.Models void CopyFrom(IContent other, string culture = "*"); // fixme validate published cultures? - - /// - /// Checks if the content and property values are valid in order to be persisted. - /// - /// If the content type is variant, then culture can be either '*' or an actual culture, but neither 'null' nor - /// 'empty'. If the content type is invariant, then culture can be either '*' or null or empty. - bool IsValid(string culture = "*"); - + /// /// Validates the content item's properties. /// diff --git a/src/Umbraco.Web.UI/Umbraco/config/lang/en_us.xml b/src/Umbraco.Web.UI/Umbraco/config/lang/en_us.xml index 86b27d0ff3..4f419ccece 100644 --- a/src/Umbraco.Web.UI/Umbraco/config/lang/en_us.xml +++ b/src/Umbraco.Web.UI/Umbraco/config/lang/en_us.xml @@ -1458,7 +1458,6 @@ To manage your website, simply open the Umbraco back office and start adding con Invite user Invitation has been re-sent to %0% Cannot publish the document since the required '%0%' is not published - Validation failed for language '%0%' Unexpected validation failed for language '%0%' Document type was exported to file An error occurred while exporting the document type diff --git a/src/Umbraco.Web/Editors/Binders/ContentItemBaseBinder.cs b/src/Umbraco.Web/Editors/Binders/ContentItemBaseBinder.cs index ec7158c10b..7ff348ebe7 100644 --- a/src/Umbraco.Web/Editors/Binders/ContentItemBaseBinder.cs +++ b/src/Umbraco.Web/Editors/Binders/ContentItemBaseBinder.cs @@ -1,49 +1,28 @@ 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.Binders; -using System.Web.Http.Validation; -using System.Web.ModelBinding; -using System.Web.Mvc; using Newtonsoft.Json; -using Newtonsoft.Json.Serialization; -using Umbraco.Core; using Umbraco.Core.IO; using Umbraco.Core.Models; using Umbraco.Core.Models.Editors; using Umbraco.Core.Services; using Umbraco.Web.Composing; -using Umbraco.Web.Editors; using Umbraco.Web.Models.ContentEditing; -using Umbraco.Web.PublishedCache; -using Umbraco.Web.Routing; -using Umbraco.Web.Security; -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.Editors.Binders -{ - /// +{ /// /// Binds the content model to the controller action for the posted multi-part Post /// internal abstract class ContentItemBaseBinder : IModelBinder - where TPersisted : class, IContentBase - //where TModelSave : ContentBaseItemSave + where TPersisted : class, IContentBase + where TModelSave : IContentSave { protected Core.Logging.ILogger Logger { get; } protected ServiceContext Services { get; } diff --git a/src/Umbraco.Web/Editors/Binders/ContentItemBinder.cs b/src/Umbraco.Web/Editors/Binders/ContentItemBinder.cs index df5569fb34..a83e192224 100644 --- a/src/Umbraco.Web/Editors/Binders/ContentItemBinder.cs +++ b/src/Umbraco.Web/Editors/Binders/ContentItemBinder.cs @@ -17,6 +17,7 @@ using Umbraco.Web.WebApi.Filters; namespace Umbraco.Web.Editors.Binders { + /// internal class ContentItemBinder : ContentItemBaseBinder { public ContentItemBinder() : this(Current.Logger, Current.Services, Current.UmbracoContextAccessor) diff --git a/src/Umbraco.Web/Editors/ContentController.cs b/src/Umbraco.Web/Editors/ContentController.cs index 71a9f898c2..1c19ee1345 100644 --- a/src/Umbraco.Web/Editors/ContentController.cs +++ b/src/Umbraco.Web/Editors/ContentController.cs @@ -19,7 +19,6 @@ using Umbraco.Web.Models.ContentEditing; using Umbraco.Web.Models.Mapping; using Umbraco.Web.Mvc; using Umbraco.Web.WebApi; -using Umbraco.Web.WebApi.Binders; using Umbraco.Web.WebApi.Filters; using Umbraco.Core.Persistence.Querying; using Umbraco.Web.PublishedCache; @@ -29,9 +28,9 @@ using Umbraco.Web.Models; using Umbraco.Web.WebServices; using Umbraco.Web._Legacy.Actions; using Constants = Umbraco.Core.Constants; -using ContentVariation = Umbraco.Core.Models.ContentVariation; using Language = Umbraco.Web.Models.ContentEditing.Language; using Umbraco.Core.PropertyEditors; +using Umbraco.Web.Editors.Binders; using Umbraco.Web.Editors.Filters; namespace Umbraco.Web.Editors @@ -653,7 +652,7 @@ namespace Umbraco.Web.Editors { //ok, so the absolute mandatory data is invalid and it's new, we cannot actually continue! // add the modelstate to the outgoing object and throw a validation message - var forDisplay = MapToDisplay(contentItem.PersistedContent, contentItem.Culture); + var forDisplay = MapToDisplay(contentItem.PersistedContent); forDisplay.Errors = ModelState.ToErrorDictionary(); throw new HttpResponseException(Request.CreateValidationErrorResponse(forDisplay)); @@ -693,7 +692,7 @@ namespace Umbraco.Web.Editors } //get the updated model - var display = MapToDisplay(contentItem.PersistedContent, contentItem.Culture); + var display = MapToDisplay(contentItem.PersistedContent); //lasty, if it is not valid, add the modelstate to the outgoing object and throw a 403 HandleInvalidModelState(display); @@ -757,6 +756,8 @@ namespace Umbraco.Web.Editors /// private void PublishInternal(ContentItemSave contentItem, ref PublishResult publishStatus, ref bool wasCancelled) { + if (publishStatus == null) throw new ArgumentNullException(nameof(publishStatus)); + if (!contentItem.PersistedContent.ContentType.VariesByCulture()) { //its invariant, proceed normally @@ -767,47 +768,35 @@ namespace Umbraco.Web.Editors { var canPublish = true; + //All variants in this collection should have a culture if we get here! but we'll double check and filter here + var cultureVariants = contentItem.Variants.Where(x => !x.Culture.IsNullOrWhiteSpace()).ToList(); + //check if we are publishing other variants and validate them var allLangs = Services.LocalizationService.GetAllLanguages().ToDictionary(x => x.IsoCode, x => x, StringComparer.InvariantCultureIgnoreCase); - var otherVariantsToValidate = contentItem.PublishVariations.Where(x => !x.Culture.InvariantEquals(contentItem.Culture)).ToList(); - + //validate any mandatory variants that are not in the list - var mandatoryLangs = Mapper.Map, IEnumerable>(allLangs.Values) - .Where(x => otherVariantsToValidate.All(v => !v.Culture.InvariantEquals(x.IsoCode))) //don't include variants above - .Where(x => !x.IsoCode.InvariantEquals(contentItem.Culture)) //don't include the current variant - .Where(x => x.Mandatory); + var mandatoryLangs = Mapper.Map, IEnumerable>(allLangs.Values).Where(x => x.Mandatory); + foreach (var lang in mandatoryLangs) { - //cannot continue publishing since a required language that is not currently being published isn't published - if (!contentItem.PersistedContent.IsCulturePublished(lang.IsoCode)) + //Check if a mandatory language is missing from being published + //TODO: This logic is wrong, we need to also check if this language doesn't already have a published version + if (cultureVariants.Any(x => x.Culture == lang.IsoCode && !x.Publish)) { - var errMsg = Services.TextService.Localize("speechBubbles/contentReqCulturePublishError", new[] { allLangs[lang.IsoCode].CultureName }); - ModelState.AddModelError("publish_variant_" + lang.IsoCode + "_", errMsg); - canPublish = false; - } - } - - if (canPublish) - { - //validate all other variants to be published - foreach (var publishVariation in otherVariantsToValidate) - { - //validate the content item and the culture property values, we don't need to validate any invariant property values here because they will have - //been validated in the post. - var valid = contentItem.PersistedContent.IsValid(publishVariation.Culture); - if (!valid) + //cannot continue publishing since a required language that is not currently being published isn't published + if (!contentItem.PersistedContent.IsCulturePublished(lang.IsoCode)) { - var errMsg = Services.TextService.Localize("speechBubbles/contentCultureValidationError", new[] { allLangs[publishVariation.Culture].CultureName }); - ModelState.AddModelError("publish_variant_" + publishVariation.Culture + "_", errMsg); + var errMsg = Services.TextService.Localize("speechBubbles/contentReqCulturePublishError", new[] { allLangs[lang.IsoCode].CultureName }); + ModelState.AddModelError("publish_variant_" + lang.IsoCode + "_", errMsg); canPublish = false; } } } - + if (canPublish) { //try to publish all the values on the model - canPublish = PublishCulture(contentItem, otherVariantsToValidate, allLangs); + canPublish = PublishCulture(contentItem.PersistedContent, cultureVariants, allLangs); } if (canPublish) @@ -827,25 +816,22 @@ namespace Umbraco.Web.Editors } /// - /// This will call TryPublishValues on the content item for each culture that needs to be published including the invariant culture + /// This will call PublishCulture on the content item for each culture that needs to be published including the invariant culture /// - /// - /// + /// + /// /// /// - private bool PublishCulture(ContentItemSave contentItem, IEnumerable otherVariantsToValidate, IDictionary allLangs) + private bool PublishCulture(IContent persistentContent, IEnumerable cultureVariants, IDictionary allLangs) { - var culturesToPublish = new List { contentItem.Culture }; - culturesToPublish.AddRange(otherVariantsToValidate.Select(x => x.Culture)); - - foreach(var culture in culturesToPublish) + foreach(var variant in cultureVariants.Where(x => x.Publish)) { // publishing any culture, implies the invariant culture - var valid = contentItem.PersistedContent.PublishCulture(culture); + var valid = persistentContent.PublishCulture(variant.Culture); if (!valid) { - var errMsg = Services.TextService.Localize("speechBubbles/contentCultureUnexpectedValidationError", new[] { allLangs[culture].CultureName }); - ModelState.AddModelError("publish_variant_" + culture + "_", errMsg); + var errMsg = Services.TextService.Localize("speechBubbles/contentCultureUnexpectedValidationError", new[] { allLangs[variant.Culture].CultureName }); + ModelState.AddModelError("publish_variant_" + variant.Culture + "_", errMsg); return false; } } @@ -1220,8 +1206,11 @@ namespace Umbraco.Web.Editors /// private void MapPropertyValues(ContentItemSave contentSave) { - //set the names for the variants - foreach(var variant in contentSave.Variants) + //inline method to determine if a property type varies + bool Varies(Property property) => property.PropertyType.VariesByCulture(); + + //loop through each variant, set the correct name and property values + foreach (var variant in contentSave.Variants) { //Don't update the name if it is empty if (!variant.Name.IsNullOrWhiteSpace()) @@ -1237,6 +1226,12 @@ namespace Umbraco.Web.Editors contentSave.PersistedContent.Name = variant.Name; } } + + //for each variant, map the property values + MapPropertyValues( + contentSave, + (save, property) => Varies(property) ? property.GetValue(variant.Culture) : property.GetValue(), //get prop val + (save, property, v) => { if (Varies(property)) property.SetValue(v, variant.Culture); else property.SetValue(v); }); //set prop val } //TODO: We need to support 'send to publish' @@ -1262,13 +1257,6 @@ namespace Umbraco.Web.Editors contentSave.PersistedContent.Template = template; } } - - bool Varies(Property property) => property.PropertyType.VariesByCulture(); - - MapPropertyValues( - contentSave, - (save, property) => Varies(property) ? property.GetValue(save.Culture) : property.GetValue(), //get prop val - (save, property, v) => { if (Varies(property)) property.SetValue(v, save.Culture); else property.SetValue(v); }); //set prop val } /// diff --git a/src/Umbraco.Web/Editors/ContentControllerBase.cs b/src/Umbraco.Web/Editors/ContentControllerBase.cs index afed9cb776..cd124d1d9c 100644 --- a/src/Umbraco.Web/Editors/ContentControllerBase.cs +++ b/src/Umbraco.Web/Editors/ContentControllerBase.cs @@ -44,12 +44,12 @@ namespace Umbraco.Web.Editors /// /// /// - protected void MapPropertyValues( + internal void MapPropertyValues( TSaved contentItem, Func getPropertyValue, Action savePropertyValue) where TPersisted : IContentBase - where TSaved : ContentBaseSave + where TSaved : IContentSave { // map the property values foreach (var propertyDto in contentItem.ContentDto.Properties) @@ -69,6 +69,7 @@ namespace Umbraco.Web.Editors // get the property var property = contentItem.PersistedContent.Properties[propertyDto.Alias]; + //TODO: Need to update this API to support variants and/or basically any sort of 'key' // prepare files, if any var files = contentItem.UploadedFiles.Where(x => x.PropertyAlias == propertyDto.Alias).ToArray(); foreach (var file in files) @@ -99,9 +100,7 @@ namespace Umbraco.Web.Editors } } - protected void HandleInvalidModelState(ContentItemDisplayBase display) - where TPersisted : IContentBase - where T : ContentPropertyBasic + protected void HandleInvalidModelState(IErrorModel display) { //lasty, if it is not valid, add the modelstate to the outgoing object and throw a 403 if (!ModelState.IsValid) diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index 4ca0c398fc..8195298b55 100644 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -213,8 +213,6 @@ - -