yv01p/Umbraco-CMS
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fixes content service Saving events so that they are fired before the model's values are published when the service is publishing

Browse Source
This commit is contained in:
Shannon
2019-02-21 14:46:14 +11:00
parent 7a9bc19ffd
commit 13a1c02507
9 changed files with 438 additions and 222 deletions
Download Patch File Download Diff File Expand all files Collapse all files

58
src/Umbraco.Core/Models/ContentRepositoryExtensions.cs
View File

@@ -1,7 +1,11 @@
using System;
using System.Collections.Generic;
using System.Linq;
using Umbraco.Core.Collections;
using Umbraco.Core.Composing;
using Umbraco.Core.Exceptions;
using Umbraco.Core.PropertyEditors;
using Umbraco.Core.Services;
namespace Umbraco.Core.Models
{
@@ -101,35 +105,6 @@ namespace Umbraco.Core.Models
}
}
/// <summary>
/// Validates the content item's properties pass variant rules
/// </summary>
/// <para>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.</para>
public static Property[] ValidateProperties(this IContentBase content, string culture = "*")
{
// select invalid properties
return content.Properties.Where(x =>
{
// if culture is null, we validate invariant properties only
// if culture is '*' we validate both variant and invariant properties, automatically
// if culture is specific eg 'en-US' we both too, but explicitly
var varies = x.PropertyType.VariesByCulture();
if (culture == null)
return !(varies || x.IsValid(null)); // validate invariant property, invariant culture
if (culture == "*")
return !x.IsValid(culture); // validate property, all cultures
return varies
? !x.IsValid(culture) // validate variant property, explicit culture
: !x.IsValid(null); // validate invariant property, explicit culture
})
.ToArray();
}
public static void SetPublishInfo(this IContent content, string culture, string name, DateTime date)
{
if (string.IsNullOrWhiteSpace(name))
@@ -191,15 +166,19 @@ namespace Umbraco.Core.Models
content.CultureInfos.AddOrUpdate(culture, name, date);
}
public static bool PublishCulture(this IContent content, string culture = "*")
/// <summary>
/// This will set the publishing values for names and properties for the content/culture
/// </summary>
/// <param name="content"></param>
/// <param name="culture"></param>
/// <returns>
/// A boolean if it's possible to publish the values for the provided culture. This may fail required names are not set.
/// </returns>
/// <remarks>
/// This does not validation property data
/// </remarks>
public static bool PublishCulture(this IContent content,string culture = "*")
{
return PublishCulture(content, out _, culture);
}
public static bool PublishCulture(this IContent content, out Property[] invalidProperties, string culture = "*")
{
invalidProperties = null;
culture = culture.NullOrWhiteSpaceAsNull();
// the variation should be supported by the content type properties
@@ -208,11 +187,6 @@ namespace Umbraco.Core.Models
if (!content.ContentType.SupportsPropertyVariation(culture, "*", true))
throw new NotSupportedException($"Culture \"{culture}\" is not supported by content type \"{content.ContentType.Alias}\" with variation \"{content.ContentType.Variations}\".");
// the values we want to publish should be valid
invalidProperties = content.ValidateProperties(culture);
if (invalidProperties.Length > 0)
return false;
var alsoInvariant = false;
if (culture == "*") // all cultures
{

50
src/Umbraco.Core/Models/Property.cs
View File

@@ -332,56 +332,6 @@ namespace Umbraco.Core.Models
return (pvalue, change);
}
/// <summary>
/// Gets a value indicating whether the property has valid values.
/// </summary>
internal bool IsValid(string culture = "*", string segment = "*")
{
// TODO: validating values shouldn't be done here, this calls in to IsValidValue
culture = culture.NullOrWhiteSpaceAsNull();
segment = segment.NullOrWhiteSpaceAsNull();
// if validating invariant/neutral, and it is supported, validate
// (including ensuring that the value exists, if mandatory)
if ((culture == null || culture == "*") && (segment == null || segment == "*") && PropertyType.SupportsVariation(null, null))
if (!IsValidValue(_pvalue?.EditedValue))
return false;
// if validating only invariant/neutral, we are good
if (culture == null && segment == null)
return true;
// if nothing else to validate, we are good
if ((culture == null || culture == "*") && (segment == null || segment == "*") && !PropertyType.VariesByCulture())
return true;
// for anything else, validate the existing values (including mandatory),
// but we cannot validate mandatory globally (we don't know the possible cultures and segments)
if (_vvalues == null) return culture == "*" || IsValidValue(null);
var pvalues = _vvalues.Where(x =>
PropertyType.SupportsVariation(x.Value.Culture, x.Value.Segment, true) && // the value variation is ok
(culture == "*" || x.Value.Culture.InvariantEquals(culture)) && // the culture matches
(segment == "*" || x.Value.Segment.InvariantEquals(segment))) // the segment matches
.Select(x => x.Value)
.ToList();
return pvalues.Count == 0 || pvalues.All(x => IsValidValue(x.EditedValue));
}
/// <summary>
/// Boolean indicating whether the passed in value is valid
/// </summary>
/// <param name="value"></param>
/// <returns>True is property value is valid, otherwise false</returns>
private bool IsValidValue(object value)
{
// TODO: this shouldn't exist here, the model itself shouldn't be responsible for it's own validation and this requires singleton access
return PropertyType.IsPropertyValueValid(value);
}
protected override void PerformDeepClone(object clone)
{
base.PerformDeepClone(clone);

12
src/Umbraco.Core/Models/PropertyType.cs
View File

@@ -363,18 +363,6 @@ namespace Umbraco.Core.Models
}
// TODO: this and other value validation methods should be a service level (not a model) thing. Changing this to internal for now
/// <summary>
/// Determines whether a value is valid for this property type.
/// </summary>
internal bool IsPropertyValueValid(object value)
{
var editor = Current.PropertyEditors[_propertyEditorAlias]; // TODO: inject
var configuration = Current.Services.DataTypeService.GetDataType(_dataTypeId).Configuration; // TODO: inject
var valueEditor = editor.GetValueEditor(configuration);
return !valueEditor.Validate(value, Mandatory, ValidationRegExp).Any();
}
/// <summary>
/// Sanitizes a property type alias.
/// </summary>

278
src/Umbraco.Core/Services/Implement/ContentService.cs
View File

@@ -10,6 +10,7 @@ using Umbraco.Core.Models.Membership;
using Umbraco.Core.Persistence.DatabaseModelDefinitions;
using Umbraco.Core.Persistence.Querying;
using Umbraco.Core.Persistence.Repositories;
using Umbraco.Core.PropertyEditors;
using Umbraco.Core.Scoping;
using Umbraco.Core.Services.Changes;
@@ -27,6 +28,8 @@ namespace Umbraco.Core.Services.Implement
private readonly IDocumentBlueprintRepository _documentBlueprintRepository;
private readonly ILanguageRepository _languageRepository;
private IQuery<IContent> _queryNotTrashed;
//TODO: The non-lazy object should be injected
private readonly Lazy<PropertyValidationService> _propertyValidationService = new Lazy<PropertyValidationService>(() => new PropertyValidationService());
#region Constructors
@@ -193,7 +196,7 @@ namespace Umbraco.Core.Services.Implement
var content = new Content(name, parentId, contentType);
using (var scope = ScopeProvider.CreateScope())
{
CreateContent(scope, content, parent, userId, false);
CreateContent(scope, content, userId, false);
scope.Complete();
}
@@ -227,7 +230,7 @@ namespace Umbraco.Core.Services.Implement
throw new ArgumentException("No content type with that alias.", nameof(contentTypeAlias)); // causes rollback
var content = new Content(name, parent, contentType);
CreateContent(scope, content, parent, userId, false);
CreateContent(scope, content, userId, false);
scope.Complete();
return content;
@@ -261,7 +264,7 @@ namespace Umbraco.Core.Services.Implement
throw new ArgumentException("No content with that id.", nameof(parentId)); // causes rollback
var content = parentId > 0 ? new Content(name, parent, contentType) : new Content(name, parentId, contentType);
CreateContent(scope, content, parent, userId, true);
CreateContent(scope, content, userId, true);
scope.Complete();
return content;
@@ -293,14 +296,14 @@ namespace Umbraco.Core.Services.Implement
throw new ArgumentException("No content type with that alias.", nameof(contentTypeAlias)); // causes rollback
var content = new Content(name, parent, contentType);
CreateContent(scope, content, parent, userId, true);
CreateContent(scope, content, userId, true);
scope.Complete();
return content;
}
}
private void CreateContent(IScope scope, Content content, IContent parent, int userId, bool withIdentity)
private void CreateContent(IScope scope, IContent content, int userId, bool withIdentity)
{
content.CreatorId = userId;
content.WriterId = userId;
@@ -311,12 +314,12 @@ namespace Umbraco.Core.Services.Implement
// if saving is cancelled, content remains without an identity
var saveEventArgs = new ContentSavingEventArgs(content, evtMsgs);
if (scope.Events.DispatchCancelable(Saving, this, saveEventArgs, "Saving"))
if (scope.Events.DispatchCancelable(Saving, this, saveEventArgs, nameof(Saving)))
return;
_documentRepository.Save(content);
scope.Events.Dispatch(Saved, this, saveEventArgs.ToContentSavedEventArgs(), "Saved");
scope.Events.Dispatch(Saved, this, saveEventArgs.ToContentSavedEventArgs(), nameof(Saved));
scope.Events.Dispatch(TreeChanged, this, new TreeChange<IContent>(content, TreeChangeTypes.RefreshNode).ToEventArgs());
}
@@ -758,7 +761,7 @@ namespace Umbraco.Core.Services.Implement
using (var scope = ScopeProvider.CreateScope())
{
var saveEventArgs = new ContentSavingEventArgs(content, evtMsgs);
if (raiseEvents && scope.Events.DispatchCancelable(Saving, this, saveEventArgs, "Saving"))
if (raiseEvents && scope.Events.DispatchCancelable(Saving, this, saveEventArgs, nameof(Saving)))
{
scope.Complete();
return OperationResult.Cancel(evtMsgs);
@@ -783,7 +786,7 @@ namespace Umbraco.Core.Services.Implement
if (raiseEvents)
{
scope.Events.Dispatch(Saved, this, saveEventArgs.ToContentSavedEventArgs(), "Saved");
scope.Events.Dispatch(Saved, this, saveEventArgs.ToContentSavedEventArgs(), nameof(Saved));
}
var changeType = TreeChangeTypes.RefreshNode;
scope.Events.Dispatch(TreeChanged, this, new TreeChange<IContent>(content, changeType).ToEventArgs());
@@ -813,7 +816,7 @@ namespace Umbraco.Core.Services.Implement
using (var scope = ScopeProvider.CreateScope())
{
var saveEventArgs = new ContentSavingEventArgs(contentsA, evtMsgs);
if (raiseEvents && scope.Events.DispatchCancelable(Saving, this, saveEventArgs, "Saving"))
if (raiseEvents && scope.Events.DispatchCancelable(Saving, this, saveEventArgs, nameof(Saving)))
{
scope.Complete();
return OperationResult.Cancel(evtMsgs);
@@ -833,7 +836,7 @@ namespace Umbraco.Core.Services.Implement
if (raiseEvents)
{
scope.Events.Dispatch(Saved, this, saveEventArgs.ToContentSavedEventArgs(), "Saved");
scope.Events.Dispatch(Saved, this, saveEventArgs.ToContentSavedEventArgs(), nameof(Saved));
}
scope.Events.Dispatch(TreeChanged, this, treeChanges.ToEventArgs());
Audit(AuditType.Save, userId == -1 ? 0 : userId, Constants.System.Root, "Saved multiple content");
@@ -866,35 +869,51 @@ namespace Umbraco.Core.Services.Implement
throw new NotSupportedException($"Culture \"{culture}\" is not supported by invariant content types.");
}
// if culture is specific, first publish the invariant values, then publish the culture itself.
// if culture is '*', then publish them all (including variants)
Property[] invalidProperties;
// explicitly SaveAndPublish a specific culture also publishes invariant values
if (!culture.IsNullOrWhiteSpace() && culture != "*")
using (var scope = ScopeProvider.CreateScope())
{
// publish the invariant values
var publishInvariant = content.PublishCulture(out invalidProperties, null);
if (!publishInvariant)
scope.WriteLock(Constants.Locks.ContentTree);
var saveEventArgs = new ContentSavingEventArgs(content, evtMsgs);
if (raiseEvents && scope.Events.DispatchCancelable(Saving, this, saveEventArgs, nameof(Saving)))
return new PublishResult(PublishResultType.FailedPublishCancelledByEvent, evtMsgs, content);
Property[] invalidProperties;
// if culture is specific, first publish the invariant values, then publish the culture itself.
// if culture is '*', then publish them all (including variants)
// explicitly SaveAndPublish a specific culture also publishes invariant values
if (!culture.IsNullOrWhiteSpace() && culture != "*")
{
// publish the invariant values
var publishInvariant = content.PublishCulture(null);
if (!publishInvariant)
return new PublishResult(PublishResultType.FailedPublishContentInvalid, evtMsgs, content);
//validate the property values
if (!_propertyValidationService.Value.IsPropertyDataValid(content, out invalidProperties))
return new PublishResult(PublishResultType.FailedPublishContentInvalid, evtMsgs, content)
{
InvalidProperties = invalidProperties
};
}
// publish the culture(s)
var publishCulture = content.PublishCulture(culture);
if (!publishCulture)
return new PublishResult(PublishResultType.FailedPublishContentInvalid, evtMsgs, content);
//validate the property values
if (!_propertyValidationService.Value.IsPropertyDataValid(content, out invalidProperties))
return new PublishResult(PublishResultType.FailedPublishContentInvalid, evtMsgs, content)
{
InvalidProperties = invalidProperties ?? Enumerable.Empty<Property>()
InvalidProperties = invalidProperties
};
var result = CommitDocumentChangesInternal(scope, content, saveEventArgs, userId, raiseEvents);
scope.Complete();
return result;
}
// publish the culture(s)
var publishCulture = content.PublishCulture(out invalidProperties, culture);
if (!publishCulture)
return new PublishResult(PublishResultType.FailedPublishContentInvalid, evtMsgs, content)
{
InvalidProperties = invalidProperties ?? Enumerable.Empty<Property>()
};
// finally, "save publishing"
// what happens next depends on whether the content can be published or not
return CommitDocumentChanges(content, userId, raiseEvents);
}
/// <inheritdoc />
@@ -903,23 +922,40 @@ namespace Umbraco.Core.Services.Implement
if (content == null) throw new ArgumentNullException(nameof(content));
if (cultures == null) throw new ArgumentNullException(nameof(cultures));
var evtMsgs = EventMessagesFactory.Get();
var varies = content.ContentType.VariesByCulture();
if (cultures.Length == 0)
using (var scope = ScopeProvider.CreateScope())
{
//no cultures specified and doesn't vary, so publish it, else nothing to publish
return !varies
? SaveAndPublish(content, userId: userId, raiseEvents: raiseEvents)
: new PublishResult(PublishResultType.FailedPublishNothingToPublish, evtMsgs, content);
scope.WriteLock(Constants.Locks.ContentTree);
var evtMsgs = EventMessagesFactory.Get();
var saveEventArgs = new ContentSavingEventArgs(content, evtMsgs);
if (raiseEvents && scope.Events.DispatchCancelable(Saving, this, saveEventArgs, nameof(Saving)))
return new PublishResult(PublishResultType.FailedPublishCancelledByEvent, evtMsgs, content);
var varies = content.ContentType.VariesByCulture();
if (cultures.Length == 0)
{
//no cultures specified and doesn't vary, so publish it, else nothing to publish
return !varies
? SaveAndPublish(content, userId: userId, raiseEvents: raiseEvents)
: new PublishResult(PublishResultType.FailedPublishNothingToPublish, evtMsgs, content);
}
if (cultures.Select(content.PublishCulture).Any(isValid => !isValid))
return new PublishResult(PublishResultType.FailedPublishContentInvalid, evtMsgs, content);
//validate the property values
if (!_propertyValidationService.Value.IsPropertyDataValid(content, out var invalidProperties))
return new PublishResult(PublishResultType.FailedPublishContentInvalid, evtMsgs, content)
{
InvalidProperties = invalidProperties
};
var result = CommitDocumentChangesInternal(scope, content, saveEventArgs, userId, raiseEvents);
scope.Complete();
return result;
}
// TODO: currently, no way to know which one failed
if (cultures.Select(content.PublishCulture).Any(isValid => !isValid))
return new PublishResult(PublishResultType.FailedPublishContentInvalid, evtMsgs, content);
return CommitDocumentChanges(content, userId, raiseEvents);
}
/// <inheritdoc />
@@ -952,56 +988,87 @@ namespace Umbraco.Core.Services.Implement
if (!content.Published)
return new PublishResult(PublishResultType.SuccessUnpublishAlready, evtMsgs, content);
// all cultures = unpublish whole
if (culture == "*" || (!content.ContentType.VariesByCulture() && culture == null))
using (var scope = ScopeProvider.CreateScope())
{
content.PublishedState = PublishedState.Unpublishing;
}
else
{
// If the culture we want to unpublish was already unpublished, nothing to do.
// To check for that we need to lookup the persisted content item
var persisted = content.HasIdentity ? GetById(content.Id) : null;
scope.WriteLock(Constants.Locks.ContentTree);
if (persisted != null && !persisted.IsCulturePublished(culture))
return new PublishResult(PublishResultType.SuccessUnpublishAlready, evtMsgs, content);
var saveEventArgs = new ContentSavingEventArgs(content, evtMsgs);
if (scope.Events.DispatchCancelable(Saving, this, saveEventArgs, nameof(Saving)))
return new PublishResult(PublishResultType.FailedPublishCancelledByEvent, evtMsgs, content);
// unpublish the culture
content.UnpublishCulture(culture);
// all cultures = unpublish whole
if (culture == "*" || (!content.ContentType.VariesByCulture() && culture == null))
{
content.PublishedState = PublishedState.Unpublishing;
}
else
{
// If the culture we want to unpublish was already unpublished, nothing to do.
// To check for that we need to lookup the persisted content item
var persisted = content.HasIdentity ? GetById(content.Id) : null;
if (persisted != null && !persisted.IsCulturePublished(culture))
return new PublishResult(PublishResultType.SuccessUnpublishAlready, evtMsgs, content);
// unpublish the culture
content.UnpublishCulture(culture);
}
var result = CommitDocumentChangesInternal(scope, content, saveEventArgs, userId);
scope.Complete();
return result;
}
// finally, "save publishing"
return CommitDocumentChanges(content, userId);
}
/// <summary>
/// Saves a document and publishes/unpublishes any pending publishing changes made to the document.
/// </summary>
/// <remarks>
/// <para>
/// This MUST NOT be called from within this service, this used to be a public API and must only be used outside of this service.
/// Internally in this service, calls must be made to CommitDocumentChangesInternal
/// </para>
///
/// <para>This is the underlying logic for both publishing and unpublishing any document</para>
/// <para>Pending publishing/unpublishing changes on a document are made with calls to <see cref="IContent.PublishCulture"/> and
/// <see cref="IContent.UnpublishCulture"/>.</para>
/// <para>Pending publishing/unpublishing changes on a document are made with calls to <see cref="ContentRepositoryExtensions.PublishCulture"/> and
/// <see cref="ContentRepositoryExtensions.UnpublishCulture"/>.</para>
/// <para>When publishing or unpublishing a single culture, or all cultures, use <see cref="SaveAndPublish"/>
/// and <see cref="Unpublish"/>. But if the flexibility to both publish and unpublish in a single operation is required
/// then this method needs to be used in combination with <see cref="IContent.PublishCulture"/> and <see cref="IContent.UnpublishCulture"/>
/// then this method needs to be used in combination with <see cref="ContentRepositoryExtensions.PublishCulture"/> and <see cref="ContentRepositoryExtensions.UnpublishCulture"/>
/// on the content itself - this prepares the content, but does not commit anything - and then, invoke
/// <see cref="CommitDocumentChanges"/> to actually commit the changes to the database.</para>
/// <para>The document is *always* saved, even when publishing fails.</para>
/// </remarks>
internal PublishResult CommitDocumentChanges(IContent content, int userId = Constants.Security.SuperUserId, bool raiseEvents = true)
internal PublishResult CommitDocumentChanges(IContent content,
int userId = Constants.Security.SuperUserId, bool raiseEvents = true)
{
using (var scope = ScopeProvider.CreateScope())
{
var evtMsgs = EventMessagesFactory.Get();
scope.WriteLock(Constants.Locks.ContentTree);
var result = CommitDocumentChangesInternal(scope, content, userId, raiseEvents);
var saveEventArgs = new ContentSavingEventArgs(content, evtMsgs);
if (raiseEvents && scope.Events.DispatchCancelable(Saving, this, saveEventArgs, nameof(Saving)))
return new PublishResult(PublishResultType.FailedPublishCancelledByEvent, evtMsgs, content);
var result = CommitDocumentChangesInternal(scope, content, saveEventArgs, userId, raiseEvents);
scope.Complete();
return result;
}
}
private PublishResult CommitDocumentChangesInternal(IScope scope, IContent content, int userId = Constants.Security.SuperUserId, bool raiseEvents = true, bool branchOne = false, bool branchRoot = false)
private PublishResult CommitDocumentChangesInternal(IScope scope, IContent content,
ContentSavingEventArgs saveEventArgs,
int userId = Constants.Security.SuperUserId, bool raiseEvents = true, bool branchOne = false, bool branchRoot = false)
{
var evtMsgs = EventMessagesFactory.Get();
if (scope == null) throw new ArgumentNullException(nameof(scope));
if (content == null) throw new ArgumentNullException(nameof(content));
if (saveEventArgs == null) throw new ArgumentNullException(nameof(saveEventArgs));
var evtMsgs = saveEventArgs.Messages;
PublishResult publishResult = null;
PublishResult unpublishResult = null;
@@ -1027,10 +1094,6 @@ namespace Umbraco.Core.Services.Implement
var changeType = isNew ? TreeChangeTypes.RefreshNode : TreeChangeTypes.RefreshBranch;
var previouslyPublished = content.HasIdentity && content.Published;
// always save
var saveEventArgs = new ContentSavingEventArgs(content, evtMsgs);
if (raiseEvents && scope.Events.DispatchCancelable(Saving, this, saveEventArgs, "Saving"))
return new PublishResult(PublishResultType.FailedPublishCancelledByEvent, evtMsgs, content);
if (publishing)
{
@@ -1116,7 +1179,7 @@ namespace Umbraco.Core.Services.Implement
// raise the Saved event, always
if (raiseEvents)
{
scope.Events.Dispatch(Saved, this, saveEventArgs.ToContentSavedEventArgs(), "Saved");
scope.Events.Dispatch(Saved, this, saveEventArgs.ToContentSavedEventArgs(), nameof(Saved));
}
if (unpublishing) // we have tried to unpublish - won't happen in a branch
@@ -1251,7 +1314,13 @@ namespace Umbraco.Core.Services.Implement
.Distinct()
.ToList();
Property[] invalidProperties = null;
if (pendingCultures.Count == 0)
break; //shouldn't happen but no point in continuing if there's nothing there
var saveEventArgs = new ContentSavingEventArgs(d, evtMsgs);
if (scope.Events.DispatchCancelable(Saving, this, saveEventArgs, nameof(Saving)))
yield return new PublishResult(PublishResultType.FailedPublishCancelledByEvent, evtMsgs, d);
var publishing = true;
foreach (var culture in pendingCultures)
{
@@ -1260,19 +1329,24 @@ namespace Umbraco.Core.Services.Implement
if (d.Trashed) continue; // won't publish
publishing &= d.PublishCulture(out invalidProperties, culture); //set the culture to be published
//publish the culture values and validate the property values, if validation fails, log the invalid properties so the develeper has an idea of what has failed
Property[] invalidProperties = null;
var tryPublish = d.PublishCulture(culture) && _propertyValidationService.Value.IsPropertyDataValid(d, out invalidProperties);
if (invalidProperties != null && invalidProperties.Length > 0)
Logger.Warn<ContentService>("Scheduled publishing will fail for document {DocumentId} and culture {Culture} because of invalid properties {InvalidProperties}",
d.Id, culture, string.Join(",", invalidProperties.Select(x => x.Alias)));
publishing &= tryPublish; //set the culture to be published
if (!publishing) break; // no point continuing
}
if (d.Trashed)
result = new PublishResult(PublishResultType.FailedPublishIsTrashed, evtMsgs, d);
else if (!publishing)
result = new PublishResult(PublishResultType.FailedPublishContentInvalid, evtMsgs, d)
{
InvalidProperties = invalidProperties ?? Enumerable.Empty<Property>()
};
result = new PublishResult(PublishResultType.FailedPublishContentInvalid, evtMsgs, d);
else
result = CommitDocumentChanges(d, d.WriterId);
result = CommitDocumentChangesInternal(scope, d, saveEventArgs, d.WriterId);
if (result.Success == false)
Logger.Error<ContentService>(null, "Failed to publish document id={DocumentId}, reason={Reason}.", d.Id, result.Result);
@@ -1306,6 +1380,13 @@ namespace Umbraco.Core.Services.Implement
.Distinct()
.ToList();
if (pendingCultures.Count == 0)
break; //shouldn't happen but no point in continuing if there's nothing there
var saveEventArgs = new ContentSavingEventArgs(d, evtMsgs);
if (scope.Events.DispatchCancelable(Saving, this, saveEventArgs, nameof(Saving)))
yield return new PublishResult(PublishResultType.FailedPublishCancelledByEvent, evtMsgs, d);
foreach (var c in pendingCultures)
{
//Clear this schedule for this culture
@@ -1314,13 +1395,11 @@ namespace Umbraco.Core.Services.Implement
d.UnpublishCulture(c);
}
if (pendingCultures.Count > 0)
{
result = CommitDocumentChanges(d, d.WriterId);
if (result.Success == false)
Logger.Error<ContentService>(null, "Failed to publish document id={DocumentId}, reason={Reason}.", d.Id, result.Result);
yield return result;
}
result = CommitDocumentChangesInternal(scope, d, saveEventArgs, d.WriterId);
if (result.Success == false)
Logger.Error<ContentService>(null, "Failed to publish document id={DocumentId}, reason={Reason}.", d.Id, result.Result);
yield return result;
}
else
{
@@ -1341,13 +1420,15 @@ namespace Umbraco.Core.Services.Implement
}
}
private bool SaveAndPublishBranch_PublishCultures(IContent c, HashSet<string> culturesToPublish)
private bool SaveAndPublishBranch_PublishCultures(IContent content, HashSet<string> culturesToPublish)
{
//TODO: This does not support being able to return invalid property details to bubble up to the UI
// variant content type - publish specified cultures
// invariant content type - publish only the invariant culture
return c.ContentType.VariesByCulture()
? culturesToPublish.All(c.PublishCulture)
: c.PublishCulture();
return content.ContentType.VariesByCulture()
? culturesToPublish.All(culture => content.PublishCulture(culture) && _propertyValidationService.Value.IsPropertyDataValid(content, out _))
: content.PublishCulture() && _propertyValidationService.Value.IsPropertyDataValid(content, out _);
}
private HashSet<string> SaveAndPublishBranch_ShouldPublish3(ref HashSet<string> cultures, string c, bool published, bool edited, bool isRoot, bool force)
@@ -1547,15 +1628,18 @@ namespace Umbraco.Core.Services.Implement
if (culturesToPublish.Count == 0) // empty = already published
return new PublishResult(PublishResultType.SuccessPublishAlready, evtMsgs, document);
var saveEventArgs = new ContentSavingEventArgs(document, evtMsgs);
if (scope.Events.DispatchCancelable(Saving, this, saveEventArgs, nameof(Saving)))
return new PublishResult(PublishResultType.FailedPublishCancelledByEvent, evtMsgs, document);
// publish & check if values are valid
if (!publishCultures(document, culturesToPublish))
{
//TODO: Based on this callback behavior there is no way to know which properties may have been invalid if this failed, see other results of FailedPublishContentInvalid
return new PublishResult(PublishResultType.FailedPublishContentInvalid, evtMsgs, document);
}
}
var result = CommitDocumentChangesInternal(scope, document, userId, branchOne: true, branchRoot: isRoot);
var result = CommitDocumentChangesInternal(scope, document, saveEventArgs, userId, branchOne: true, branchRoot: isRoot);
if (result.Success)
publishedDocuments.Add(document);
return result;

133
src/Umbraco.Core/Services/PropertyValidationService.cs Normal file
View File

@@ -0,0 +1,133 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using Umbraco.Core.Collections;
using Umbraco.Core.Composing;
using Umbraco.Core.Models;
using Umbraco.Core.PropertyEditors;
namespace Umbraco.Core.Services
{
//TODO: We should make this an interface and inject it into the ContentService
internal class PropertyValidationService
{
private readonly PropertyEditorCollection _propertyEditors;
private readonly IDataTypeService _dataTypeService;
public PropertyValidationService(PropertyEditorCollection propertyEditors, IDataTypeService dataTypeService)
{
_propertyEditors = propertyEditors;
_dataTypeService = dataTypeService;
}
//TODO: Remove this method in favor of the overload specifying all dependencies
public PropertyValidationService()
: this(Current.PropertyEditors, Current.Services.DataTypeService)
{
}
/// <summary>
/// Validates the content item's properties pass validation rules
/// </summary>
/// <para>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.</para>
public bool IsPropertyDataValid(IContentBase content, out Property[] invalidProperties, string culture = "*")
{
// select invalid properties
invalidProperties = content.Properties.Where(x =>
{
// if culture is null, we validate invariant properties only
// if culture is '*' we validate both variant and invariant properties, automatically
// if culture is specific eg 'en-US' we both too, but explicitly
var varies = x.PropertyType.VariesByCulture();
if (culture == null)
return !(varies || IsPropertyValid(x, null)); // validate invariant property, invariant culture
if (culture == "*")
return !IsPropertyValid(x, culture); // validate property, all cultures
return varies
? !IsPropertyValid(x, culture) // validate variant property, explicit culture
: !IsPropertyValid(x, null); // validate invariant property, explicit culture
})
.ToArray();
return invalidProperties.Length == 0;
}
/// <summary>
/// Gets a value indicating whether the property has valid values.
/// </summary>
public bool IsPropertyValid(Property property, string culture = "*", string segment = "*")
{
//NOTE - the pvalue and vvalues logic in here is borrowed directly from the Property.Values setter so if you are wondering what that's all about, look there.
// The underlying Property._pvalue and Property._vvalues are not exposed but we can re-create these values ourselves which is what it's doing.
culture = culture.NullOrWhiteSpaceAsNull();
segment = segment.NullOrWhiteSpaceAsNull();
Property.PropertyValue pvalue = null;
// if validating invariant/neutral, and it is supported, validate
// (including ensuring that the value exists, if mandatory)
if ((culture == null || culture == "*") && (segment == null || segment == "*") && property.PropertyType.SupportsVariation(null, null))
{
pvalue = property.Values.FirstOrDefault(x => x.Culture == null && x.Segment == null);
if (!IsValidPropertyValue(property, pvalue?.EditedValue))
return false;
}
// if validating only invariant/neutral, we are good
if (culture == null && segment == null)
return true;
// if nothing else to validate, we are good
if ((culture == null || culture == "*") && (segment == null || segment == "*") && !property.PropertyType.VariesByCulture())
return true;
// for anything else, validate the existing values (including mandatory),
// but we cannot validate mandatory globally (we don't know the possible cultures and segments)
var vvalues = property.Values.Count > (pvalue == null ? 0 : 1)
? property.Values.Where(x => x != pvalue).ToDictionary(x => new CompositeNStringNStringKey(x.Culture, x.Segment), x => x)
: null;
if (vvalues == null) return culture == "*" || IsValidPropertyValue(property,null);
var pvalues = vvalues.Where(x =>
property.PropertyType.SupportsVariation(x.Value.Culture, x.Value.Segment, true) && // the value variation is ok
(culture == "*" || x.Value.Culture.InvariantEquals(culture)) && // the culture matches
(segment == "*" || x.Value.Segment.InvariantEquals(segment))) // the segment matches
.Select(x => x.Value)
.ToList();
return pvalues.Count == 0 || pvalues.All(x => IsValidPropertyValue(property, x.EditedValue));
}
/// <summary>
/// Boolean indicating whether the passed in value is valid
/// </summary>
/// <param name="property"></param>
/// <param name="value"></param>
/// <returns>True is property value is valid, otherwise false</returns>
private bool IsValidPropertyValue(Property property, object value)
{
return IsPropertyValueValid(property.PropertyType, value);
}
/// <summary>
/// Determines whether a value is valid for this property type.
/// </summary>
private bool IsPropertyValueValid(PropertyType propertyType, object value)
{
var editor = _propertyEditors[propertyType.PropertyEditorAlias];
var configuration = _dataTypeService.GetDataType(propertyType.DataTypeId).Configuration;
var valueEditor = editor.GetValueEditor(configuration);
return !valueEditor.Validate(value, propertyType.Mandatory, propertyType.ValidationRegExp).Any();
}
}
}

1
src/Umbraco.Core/Umbraco.Core.csproj
View File

@@ -210,6 +210,7 @@
<Compile Include="Migrations\Upgrade\V_8_0_0\MergeDateAndDateTimePropertyEditor.cs" />
<Compile Include="PropertyEditors\DateTimeConfiguration.cs" />
<Compile Include="Migrations\Upgrade\V_8_0_0\RenameLabelAndRichTextPropertyEditorAliases.cs" />
<Compile Include="Services\PropertyValidationService.cs" />
<Compile Include="TypeLoaderExtensions.cs" />
<Compile Include="Composing\WeightAttribute.cs" />
<Compile Include="Composing\WeightedCollectionBuilderBase.cs" />

7
src/Umbraco.Tests/Models/VariationTests.cs
View File

@@ -486,14 +486,15 @@ namespace Umbraco.Tests.Models
prop.SetValue("a");
Assert.AreEqual("a", prop.GetValue());
Assert.IsNull(prop.GetValue(published: true));
var propertyValidationService = new PropertyValidationService(Current.Factory.GetInstance<PropertyEditorCollection>(), Current.Factory.GetInstance<ServiceContext>().DataTypeService);
Assert.IsTrue(prop.IsValid());
Assert.IsTrue(propertyValidationService.IsPropertyValid(prop));
propertyType.Mandatory = true;
Assert.IsTrue(prop.IsValid());
Assert.IsTrue(propertyValidationService.IsPropertyValid(prop));
prop.SetValue(null);
Assert.IsFalse(prop.IsValid());
Assert.IsFalse(propertyValidationService.IsPropertyValid(prop));
// can publish, even though invalid
prop.PublishValues();

115
src/Umbraco.Tests/Services/ContentServiceEventTests.cs
View File

@@ -81,9 +81,15 @@ namespace Umbraco.Tests.Services
ContentService.Saving += OnSaving;
ContentService.Saved += OnSaved;
contentService.Save(document);
ContentService.Saving -= OnSaving;
ContentService.Saved -= OnSaved;
try
{
contentService.Save(document);
}
finally
{
ContentService.Saving -= OnSaving;
ContentService.Saved -= OnSaved;
}
}
[Test]
@@ -123,9 +129,16 @@ namespace Umbraco.Tests.Services
ContentService.Saving += OnSaving;
ContentService.Saved += OnSaved;
contentService.Save(document);
ContentService.Saving -= OnSaving;
ContentService.Saved -= OnSaved;
try
{
contentService.Save(document);
}
finally
{
ContentService.Saving -= OnSaving;
ContentService.Saved -= OnSaved;
}
}
[Test]
@@ -179,9 +192,15 @@ namespace Umbraco.Tests.Services
ContentService.Publishing += OnPublishing;
ContentService.Published += OnPublished;
contentService.SaveAndPublish(document, "fr-FR");
ContentService.Publishing -= OnPublishing;
ContentService.Published -= OnPublished;
try
{
contentService.SaveAndPublish(document, "fr-FR");
}
finally
{
ContentService.Publishing -= OnPublishing;
ContentService.Published -= OnPublished;
}
document = contentService.GetById(document.Id);
@@ -225,13 +244,69 @@ namespace Umbraco.Tests.Services
Assert.AreEqual("title", propValue.PublishedValue);
}
//We are binding to Saving (not Publishing), because it shouldn't make a difference, when setting a property value
//during Saving, this should become published during a SaveAndPublish operation
//We are binding to Saving (not Publishing), because the Publishing event is really just used for cancelling, it should not be
//used for setting values and it won't actually work! This is because the Publishing event is raised AFTER the values on the model
//are published, but Saving is raised BEFORE.
ContentService.Saving += OnSaving;
ContentService.Saved += OnSaved;
contentService.SaveAndPublish(document);
ContentService.Saving -= OnSaving;
ContentService.Saved -= OnSaved;
try
{
contentService.SaveAndPublish(document);
}
finally
{
ContentService.Saving -= OnSaving;
ContentService.Saved -= OnSaved;
}
}
[Test]
public void Publishing_Set_Mandatory_Value()
{
var contentTypeService = ServiceContext.ContentTypeService;
var contentType = MockedContentTypes.CreateTextPageContentType();
var titleProperty = contentType.PropertyTypes.First(x => x.Alias == "title");
titleProperty.Mandatory = true; // make this required!
ServiceContext.FileService.SaveTemplate(contentType.DefaultTemplate);
contentTypeService.Save(contentType);
var contentService = ServiceContext.ContentService;
IContent document = new Content("content", -1, contentType);
var result = contentService.SaveAndPublish(document);
Assert.IsFalse(result.Success);
Assert.AreEqual("title", result.InvalidProperties.First().Alias);
//TODO: The ContentService doesn't reset the document's PublishedState so since the above fails, if we then try to do
// a SaveAndPublish again, we will get an exception: "Cannot save-and-publish (un)publishing content, use the dedicated CommitDocumentChanges method."
// but this exception is misleading and is caused because the document's PublishedState wasn't reset.
// So instead, we'll just re-create it.
document = new Content("content", -1, contentType);
void OnSaving(IContentService sender, ContentSavingEventArgs e)
{
var saved = e.SavedEntities.First();
Assert.IsTrue(document.GetValue<string>("title").IsNullOrWhiteSpace());
saved.SetValue("title", "title");
}
//We are binding to Saving (not Publishing), because the Publishing event is really just used for cancelling, it should not be
//used for setting values and it won't actually work! This is because the Publishing event is raised AFTER the values on the model
//are published, but Saving is raised BEFORE.
ContentService.Saving += OnSaving;
try
{
result = contentService.SaveAndPublish(document);
Assert.IsTrue(result.Success); //will succeed now because we were able to specify the required value in the Saving event
}
finally
{
ContentService.Saving -= OnSaving;
}
}
[Test]
@@ -293,9 +368,15 @@ namespace Umbraco.Tests.Services
ContentService.Publishing += OnPublishing;
ContentService.Published += OnPublished;
contentService.CommitDocumentChanges(document);
ContentService.Publishing -= OnPublishing;
ContentService.Published -= OnPublished;
try
{
contentService.CommitDocumentChanges(document);
}
finally
{
ContentService.Publishing -= OnPublishing;
ContentService.Published -= OnPublished;
}
document = contentService.GetById(document.Id);

6
src/Umbraco.Tests/Services/ContentServiceTests.cs
View File

@@ -18,6 +18,7 @@ using Umbraco.Core.Services.Implement;
using Umbraco.Tests.Testing;
using Umbraco.Core.Persistence.DatabaseModelDefinitions;
using Umbraco.Core.Cache;
using Umbraco.Core.PropertyEditors;
using Umbraco.Tests.LegacyXmlPublishedCache;
namespace Umbraco.Tests.Services
@@ -986,7 +987,10 @@ namespace Umbraco.Tests.Services
Assert.IsTrue(parent.Published);
// content cannot publish values because they are invalid
Assert.IsNotEmpty(content.ValidateProperties());
var propertyValidationService = new PropertyValidationService(Factory.GetInstance<PropertyEditorCollection>(), ServiceContext.DataTypeService);
var isValid = propertyValidationService.IsPropertyDataValid(content, out var invalidProperties);
Assert.IsFalse(isValid);
Assert.IsNotEmpty(invalidProperties);
// and therefore cannot be published,
// because it did not have a published version at all