diff --git a/src/Umbraco.Core/Migrations/Upgrade/UmbracoPlan.cs b/src/Umbraco.Core/Migrations/Upgrade/UmbracoPlan.cs index 0ba3aa8b8b..03ba58d15e 100644 --- a/src/Umbraco.Core/Migrations/Upgrade/UmbracoPlan.cs +++ b/src/Umbraco.Core/Migrations/Upgrade/UmbracoPlan.cs @@ -193,7 +193,7 @@ namespace Umbraco.Core.Migrations.Upgrade // to 8.7.0... To("{a78e3369-8ea3-40ec-ad3f-5f76929d2b20}"); - + //FINAL } } diff --git a/src/Umbraco.Core/Models/Blocks/BlockEditorData.cs b/src/Umbraco.Core/Models/Blocks/BlockEditorData.cs new file mode 100644 index 0000000000..3fe832dc04 --- /dev/null +++ b/src/Umbraco.Core/Models/Blocks/BlockEditorData.cs @@ -0,0 +1,66 @@ +using Newtonsoft.Json; +using Newtonsoft.Json.Linq; +using System; +using System.Collections.Generic; +using Umbraco.Core.Serialization; + +namespace Umbraco.Core.Models.Blocks +{ + + /// + /// Converted block data from json + /// + public class BlockEditorData + { + public static BlockEditorData Empty { get; } = new BlockEditorData(); + + private BlockEditorData() + { + } + + public BlockEditorData(JToken layout, IReadOnlyList layoutBlockReferences, IReadOnlyList blocks) + { + Layout = layout ?? throw new ArgumentNullException(nameof(layout)); + LayoutBlockReferences = layoutBlockReferences ?? throw new ArgumentNullException(nameof(layoutBlockReferences)); + Blocks = blocks ?? throw new ArgumentNullException(nameof(blocks)); + } + + public JToken Layout { get; } + public IReadOnlyList LayoutBlockReferences { get; } = new List(); + public IReadOnlyList Blocks { get; } = new List(); + + internal class BlockValue + { + [JsonProperty("layout")] + public IDictionary Layout { get; set; } + + [JsonProperty("data")] + public IEnumerable Data { get; set; } + } + + /// + /// Represents a single block's data in raw form + /// + public class BlockItemData + { + [JsonProperty("contentTypeKey")] + public Guid ContentTypeKey { get; set; } + + [JsonProperty("udi")] + [JsonConverter(typeof(UdiJsonConverter))] + public Udi Udi { get; set; } + + /// + /// The remaining properties will be serialized to a dictionary + /// + /// + /// The JsonExtensionDataAttribute is used to put the non-typed properties into a bucket + /// http://www.newtonsoft.com/json/help/html/DeserializeExtensionData.htm + /// NestedContent serializes to string, int, whatever eg + /// "stringValue":"Some String","numericValue":125,"otherNumeric":null + /// + [JsonExtensionData] + public Dictionary RawPropertyValues { get; set; } = new Dictionary(); + } + } +} diff --git a/src/Umbraco.Core/Models/Blocks/BlockEditorDataConverter.cs b/src/Umbraco.Core/Models/Blocks/BlockEditorDataConverter.cs new file mode 100644 index 0000000000..cd1931fbb0 --- /dev/null +++ b/src/Umbraco.Core/Models/Blocks/BlockEditorDataConverter.cs @@ -0,0 +1,46 @@ +using Newtonsoft.Json; +using Newtonsoft.Json.Linq; +using System.Linq; +using System; +using System.Collections.Generic; +using static Umbraco.Core.Models.Blocks.BlockEditorData; + +namespace Umbraco.Core.Models.Blocks +{ + + /// + /// Converts the block json data into objects + /// + public abstract class BlockEditorDataConverter + { + private readonly string _propertyEditorAlias; + + protected BlockEditorDataConverter(string propertyEditorAlias) + { + _propertyEditorAlias = propertyEditorAlias; + } + + public BlockEditorData Convert(string json) + { + var value = JsonConvert.DeserializeObject(json); + + if (value.Layout == null) + return BlockEditorData.Empty; + + if (!value.Layout.TryGetValue(_propertyEditorAlias, out var layout)) + return BlockEditorData.Empty; + + var references = GetBlockReferences(layout); + var blocks = value.Data.ToList(); + return new BlockEditorData(layout, references, blocks); + } + + /// + /// Return the collection of from the block editor's Layout (which could be an array or an object depending on the editor) + /// + /// + /// + protected abstract IReadOnlyList GetBlockReferences(JToken jsonLayout); + + } +} diff --git a/src/Umbraco.Core/Models/Blocks/BlockListEditorDataConverter.cs b/src/Umbraco.Core/Models/Blocks/BlockListEditorDataConverter.cs new file mode 100644 index 0000000000..eea2dfafd0 --- /dev/null +++ b/src/Umbraco.Core/Models/Blocks/BlockListEditorDataConverter.cs @@ -0,0 +1,22 @@ +using Newtonsoft.Json.Linq; +using System.Linq; +using System.Collections.Generic; + +namespace Umbraco.Core.Models.Blocks +{ + /// + /// Data converter for the block list property editor + /// + public class BlockListEditorDataConverter : BlockEditorDataConverter + { + public BlockListEditorDataConverter() : base(Constants.PropertyEditors.Aliases.BlockList) + { + } + + protected override IReadOnlyList GetBlockReferences(JToken jsonLayout) + { + var blockListLayout = jsonLayout.ToObject>(); + return blockListLayout.Select(x => x.Udi).ToList(); + } + } +} diff --git a/src/Umbraco.Core/Models/Blocks/BlockListLayoutItem.cs b/src/Umbraco.Core/Models/Blocks/BlockListLayoutItem.cs new file mode 100644 index 0000000000..d05906a6c4 --- /dev/null +++ b/src/Umbraco.Core/Models/Blocks/BlockListLayoutItem.cs @@ -0,0 +1,19 @@ +using Newtonsoft.Json; +using Umbraco.Core.Serialization; +using static Umbraco.Core.Models.Blocks.BlockEditorData; + +namespace Umbraco.Core.Models.Blocks +{ + /// + /// Used for deserializing the block list layout + /// + public class BlockListLayoutItem + { + [JsonProperty("settings")] + public BlockItemData Settings { get; set; } + + [JsonProperty("udi")] + [JsonConverter(typeof(UdiJsonConverter))] + public Udi Udi { get; set; } + } +} diff --git a/src/Umbraco.Core/Models/Blocks/IBlockElement.cs b/src/Umbraco.Core/Models/Blocks/IBlockElement.cs index 38b4e96aae..7577a7bedc 100644 --- a/src/Umbraco.Core/Models/Blocks/IBlockElement.cs +++ b/src/Umbraco.Core/Models/Blocks/IBlockElement.cs @@ -1,15 +1,5 @@ namespace Umbraco.Core.Models.Blocks { - /// - /// Represents a data item reference for a Block Editor implementation - /// - /// - /// see: https://github.com/umbraco/rfcs/blob/907f3758cf59a7b6781296a60d57d537b3b60b8c/cms/0011-block-data-structure.md#strongly-typed - /// - public interface IBlockReference - { - Udi Udi { get; } - } // TODO: IBlockElement doesn't make sense, this is a reference to an actual element with some settings // and always has to do with the "Layout", should possibly be called IBlockReference or IBlockLayout or IBlockLayoutReference diff --git a/src/Umbraco.Core/Models/Blocks/IBlockReference.cs b/src/Umbraco.Core/Models/Blocks/IBlockReference.cs new file mode 100644 index 0000000000..d63e5e4ca9 --- /dev/null +++ b/src/Umbraco.Core/Models/Blocks/IBlockReference.cs @@ -0,0 +1,13 @@ +namespace Umbraco.Core.Models.Blocks +{ + /// + /// Represents a data item reference for a Block Editor implementation + /// + /// + /// see: https://github.com/umbraco/rfcs/blob/907f3758cf59a7b6781296a60d57d537b3b60b8c/cms/0011-block-data-structure.md#strongly-typed + /// + public interface IBlockReference + { + Udi Udi { get; } + } +} diff --git a/src/Umbraco.Core/Persistence/FaultHandling/Strategies/SqlAzureTransientErrorDetectionStrategy.cs b/src/Umbraco.Core/Persistence/FaultHandling/Strategies/SqlAzureTransientErrorDetectionStrategy.cs index 849fd35fad..f763594616 100644 --- a/src/Umbraco.Core/Persistence/FaultHandling/Strategies/SqlAzureTransientErrorDetectionStrategy.cs +++ b/src/Umbraco.Core/Persistence/FaultHandling/Strategies/SqlAzureTransientErrorDetectionStrategy.cs @@ -4,6 +4,10 @@ using System.Data.SqlClient; namespace Umbraco.Core.Persistence.FaultHandling.Strategies { + // See https://docs.microsoft.com/en-us/azure/azure-sql/database/troubleshoot-common-connectivity-issues + // Also we could just use the nuget package instead https://www.nuget.org/packages/EnterpriseLibrary.TransientFaultHandling/ ? + // but i guess that's not netcore so we'll just leave it. + /// /// Provides the transient error detection logic for transient faults that are specific to SQL Azure. /// @@ -71,7 +75,7 @@ namespace Umbraco.Core.Persistence.FaultHandling.Strategies /// Determines whether the specified exception represents a transient failure that can be compensated by a retry. /// /// The exception object to be verified. - /// True if the specified exception is considered as transient, otherwise false. + /// true if the specified exception is considered as transient; otherwise, false. public bool IsTransient(Exception ex) { if (ex != null) @@ -97,40 +101,50 @@ namespace Umbraco.Core.Persistence.FaultHandling.Strategies return true; - // SQL Error Code: 40197 - // The service has encountered an error processing your request. Please try again. - case 40197: + // SQL Error Code: 10928 + // Resource ID: %d. The %s limit for the database is %d and has been reached. + case 10928: + // SQL Error Code: 10929 + // Resource ID: %d. The %s minimum guarantee is %d, maximum limit is %d and the current usage for the database is %d. + // However, the server is currently too busy to support requests greater than %d for this database. + case 10929: // SQL Error Code: 10053 // A transport-level error has occurred when receiving results from the server. // An established connection was aborted by the software in your host machine. case 10053: // SQL Error Code: 10054 - // A transport-level error has occurred when sending the request to the server. + // A transport-level error has occurred when sending the request to the server. // (provider: TCP Provider, error: 0 - An existing connection was forcibly closed by the remote host.) case 10054: // SQL Error Code: 10060 - // A network-related or instance-specific error occurred while establishing a connection to SQL Server. - // The server was not found or was not accessible. Verify that the instance name is correct and that SQL Server - // is configured to allow remote connections. (provider: TCP Provider, error: 0 - A connection attempt failed - // because the connected party did not properly respond after a period of time, or established connection failed + // A network-related or instance-specific error occurred while establishing a connection to SQL Server. + // The server was not found or was not accessible. Verify that the instance name is correct and that SQL Server + // is configured to allow remote connections. (provider: TCP Provider, error: 0 - A connection attempt failed + // because the connected party did not properly respond after a period of time, or established connection failed // because connected host has failed to respond.)"} case 10060: + // SQL Error Code: 40197 + // The service has encountered an error processing your request. Please try again. + case 40197: + // SQL Error Code: 40540 + // The service has encountered an error processing your request. Please try again. + case 40540: // SQL Error Code: 40613 - // Database XXXX on server YYYY is not currently available. Please retry the connection later. If the problem persists, contact customer + // Database XXXX on server YYYY is not currently available. Please retry the connection later. If the problem persists, contact customer // support, and provide them the session tracing ID of ZZZZZ. case 40613: // SQL Error Code: 40143 // The service has encountered an error processing your request. Please try again. case 40143: // SQL Error Code: 233 - // The client was unable to establish a connection because of an error during connection initialization process before login. - // Possible causes include the following: the client tried to connect to an unsupported version of SQL Server; the server was too busy - // to accept new connections; or there was a resource limitation (insufficient memory or maximum allowed connections) on the server. + // The client was unable to establish a connection because of an error during connection initialization process before login. + // Possible causes include the following: the client tried to connect to an unsupported version of SQL Server; the server was too busy + // to accept new connections; or there was a resource limitation (insufficient memory or maximum allowed connections) on the server. // (provider: TCP Provider, error: 0 - An existing connection was forcibly closed by the remote host.) case 233: // SQL Error Code: 64 - // A connection was successfully established with the server, but then an error occurred during the login process. - // (provider: TCP Provider, error: 0 - The specified network name is no longer available.) + // A connection was successfully established with the server, but then an error occurred during the login process. + // (provider: TCP Provider, error: 0 - The specified network name is no longer available.) case 64: // DBNETLIB Error Code: 20 // The instance of SQL Server you attempted to connect to does not support encryption. diff --git a/src/Umbraco.Core/Persistence/Repositories/IDocumentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/IDocumentRepository.cs index fc5382499f..0971b2047a 100644 --- a/src/Umbraco.Core/Persistence/Repositories/IDocumentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/IDocumentRepository.cs @@ -12,6 +12,11 @@ namespace Umbraco.Core.Persistence.Repositories /// void ClearSchedule(DateTime date); + void ClearSchedule(DateTime date, ContentScheduleAction action); + + bool HasContentForExpiration(DateTime date); + bool HasContentForRelease(DateTime date); + /// /// Gets objects having an expiration date before (lower than, or equal to) a specified date. /// diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs index db1e2b350d..e4ef939870 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs @@ -1017,6 +1017,37 @@ namespace Umbraco.Core.Persistence.Repositories.Implement Database.Execute(sql); } + /// + public void ClearSchedule(DateTime date, ContentScheduleAction action) + { + var a = action.ToString(); + var sql = Sql().Delete().Where(x => x.Date <= date && x.Action == a); + Database.Execute(sql); + } + + private Sql GetSqlForHasScheduling(ContentScheduleAction action, DateTime date) + { + var template = SqlContext.Templates.Get("Umbraco.Core.DocumentRepository.GetSqlForHasScheduling", tsql => tsql + .SelectCount() + .From() + .Where(x => x.Action == SqlTemplate.Arg("action") && x.Date <= SqlTemplate.Arg("date"))); + + var sql = template.Sql(action.ToString(), date); + return sql; + } + + public bool HasContentForExpiration(DateTime date) + { + var sql = GetSqlForHasScheduling(ContentScheduleAction.Expire, date); + return Database.ExecuteScalar(sql) > 0; + } + + public bool HasContentForRelease(DateTime date) + { + var sql = GetSqlForHasScheduling(ContentScheduleAction.Release, date); + return Database.ExecuteScalar(sql) > 0; + } + /// public IEnumerable GetContentForRelease(DateTime date) { diff --git a/src/Umbraco.Core/Runtime/SqlMainDomLock.cs b/src/Umbraco.Core/Runtime/SqlMainDomLock.cs index 5f5d0d607f..04abc83ba2 100644 --- a/src/Umbraco.Core/Runtime/SqlMainDomLock.cs +++ b/src/Umbraco.Core/Runtime/SqlMainDomLock.cs @@ -21,12 +21,11 @@ namespace Umbraco.Core.Runtime private const string MainDomKeyPrefix = "Umbraco.Core.Runtime.SqlMainDom"; private const string UpdatedSuffix = "_updated"; private readonly ILogger _logger; - private IUmbracoDatabase _db; private CancellationTokenSource _cancellationTokenSource = new CancellationTokenSource(); private SqlServerSyntaxProvider _sqlServerSyntax = new SqlServerSyntaxProvider(); private bool _mainDomChanging = false; private readonly UmbracoDatabaseFactory _dbFactory; - private bool _hasError; + private bool _errorDuringAcquiring; private object _locker = new object(); public SqlMainDomLock(ILogger logger) @@ -56,25 +55,24 @@ namespace Umbraco.Core.Runtime _logger.Debug("Acquiring lock..."); - var db = GetDatabase(); - var tempId = Guid.NewGuid().ToString(); + using var db = _dbFactory.CreateDatabase(); + using var transaction = db.GetTransaction(IsolationLevel.ReadCommitted); + try { - db.BeginTransaction(IsolationLevel.ReadCommitted); - try { // wait to get a write lock _sqlServerSyntax.WriteLock(db, TimeSpan.FromMilliseconds(millisecondsTimeout), Constants.Locks.MainDom); } - catch (Exception ex) + catch(SqlException ex) { if (IsLockTimeoutException(ex)) { _logger.Error(ex, "Sql timeout occurred, could not acquire MainDom."); - _hasError = true; + _errorDuringAcquiring = true; return false; } @@ -82,15 +80,12 @@ namespace Umbraco.Core.Runtime throw; } - var result = InsertLockRecord(tempId); //we change the row to a random Id to signal other MainDom to shutdown + var result = InsertLockRecord(tempId, db); //we change the row to a random Id to signal other MainDom to shutdown if (result == RecordPersistenceType.Insert) { // if we've inserted, then there was no MainDom so we can instantly acquire - // TODO: see the other TODO, could we just delete the row and that would indicate that we - // are MainDom? then we don't leave any orphan rows behind. - - InsertLockRecord(_lockId); // so update with our appdomain id + InsertLockRecord(_lockId, db); // so update with our appdomain id _logger.Debug("Acquired with ID {LockId}", _lockId); return true; } @@ -100,23 +95,23 @@ namespace Umbraco.Core.Runtime } catch (Exception ex) { - ResetDatabase(); // unexpected _logger.Error(ex, "Unexpected error, cannot acquire MainDom"); - _hasError = true; + _errorDuringAcquiring = true; return false; } finally { - db?.CompleteTransaction(); + transaction.Complete(); } + return await WaitForExistingAsync(tempId, millisecondsTimeout); } public Task ListenAsync() { - if (_hasError) + if (_errorDuringAcquiring) { _logger.Warn("Could not acquire MainDom, listening is canceled."); return Task.CompletedTask; @@ -142,8 +137,15 @@ namespace Umbraco.Core.Runtime { while (true) { - // poll every 1 second - Thread.Sleep(1000); + // poll every couple of seconds + // local testing shows the actual query to be executed from client/server is approx 300ms but would change depending on environment/IO + Thread.Sleep(2000); + + if (!_dbFactory.Configured) + { + // if we aren't configured, we just keep looping since we can't query the db + continue; + } if (!_dbFactory.Configured) { @@ -160,20 +162,14 @@ namespace Umbraco.Core.Runtime if (_cancellationTokenSource.IsCancellationRequested) return; - var db = GetDatabase(); - + using var db = _dbFactory.CreateDatabase(); + using var transaction = db.GetTransaction(IsolationLevel.ReadCommitted); try { - db.BeginTransaction(IsolationLevel.ReadCommitted); - // get a read lock _sqlServerSyntax.ReadLock(db, Constants.Locks.MainDom); - // TODO: We could in theory just check if the main dom row doesn't exist, that could indicate that - // we are still the maindom. An empty value might be better because then we won't have any orphan rows - // if the app is terminated. Could that work? - - if (!IsMainDomValue(_lockId)) + if (!IsMainDomValue(_lockId, db)) { // we are no longer main dom, another one has come online, exit _mainDomChanging = true; @@ -183,38 +179,23 @@ namespace Umbraco.Core.Runtime } catch (Exception ex) { - ResetDatabase(); - // unexpected - _logger.Error(ex, "Unexpected error, listening is canceled."); - _hasError = true; - return; + _logger.Error(ex, "Unexpected error during listening."); + + // We need to keep on listening unless we've been notified by our own AppDomain to shutdown since + // we don't want to shutdown resources controlled by MainDom inadvertently. We'll just keep listening otherwise. + if (_cancellationTokenSource.IsCancellationRequested) + return; + } finally { - db?.CompleteTransaction(); + transaction.Complete(); } } } } - private void ResetDatabase() - { - if (_db.InTransaction) - _db.AbortTransaction(); - _db.Dispose(); - _db = null; - } - - private IUmbracoDatabase GetDatabase() - { - if (_db != null) - return _db; - - _db = _dbFactory.CreateDatabase(); - return _db; - } - /// /// Wait for any existing MainDom to release so we can continue booting /// @@ -227,121 +208,131 @@ namespace Umbraco.Core.Runtime return Task.Run(() => { - var db = GetDatabase(); + using var db = _dbFactory.CreateDatabase(); + var watch = new Stopwatch(); watch.Start(); - while(true) + while (true) { // poll very often, we need to take over as fast as we can - Thread.Sleep(100); + // local testing shows the actual query to be executed from client/server is approx 300ms but would change depending on environment/IO + Thread.Sleep(1000); - try - { - db.BeginTransaction(IsolationLevel.ReadCommitted); - - // get a read lock - _sqlServerSyntax.ReadLock(db, Constants.Locks.MainDom); - - // the row - var mainDomRows = db.Fetch("SELECT * FROM umbracoKeyValue WHERE [key] = @key", new { key = MainDomKey }); - - if (mainDomRows.Count == 0 || mainDomRows[0].Value == updatedTempId) - { - // the other main dom has updated our record - // Or the other maindom shutdown super fast and just deleted the record - // which indicates that we - // can acquire it and it has shutdown. - - _sqlServerSyntax.WriteLock(db, Constants.Locks.MainDom); - - // so now we update the row with our appdomain id - InsertLockRecord(_lockId); - _logger.Debug("Acquired with ID {LockId}", _lockId); - return true; - } - else if (mainDomRows.Count == 1 && !mainDomRows[0].Value.StartsWith(tempId)) - { - // in this case, the prefixed ID is different which means - // another new AppDomain has come online and is wanting to take over. In that case, we will not - // acquire. - - _logger.Debug("Cannot acquire, another booting application detected."); - - return false; - } - } - catch (Exception ex) - { - ResetDatabase(); - - if (IsLockTimeoutException(ex)) - { - _logger.Error(ex, "Sql timeout occurred, waiting for existing MainDom is canceled."); - _hasError = true; - return false; - } - // unexpected - _logger.Error(ex, "Unexpected error, waiting for existing MainDom is canceled."); - _hasError = true; - return false; - } - finally - { - db?.CompleteTransaction(); - } + var acquired = TryAcquire(db, tempId, updatedTempId); + if (acquired.HasValue) + return acquired.Value; if (watch.ElapsedMilliseconds >= millisecondsTimeout) { - // if the timeout has elapsed, it either means that the other main dom is taking too long to shutdown, - // or it could mean that the previous appdomain was terminated and didn't clear out the main dom SQL row - // and it's just been left as an orphan row. - // There's really know way of knowing unless we are constantly updating the row for the current maindom - // which isn't ideal. - // So... we're going to 'just' take over, if the writelock works then we'll assume we're ok - - _logger.Debug("Timeout elapsed, assuming orphan row, acquiring MainDom."); - - try - { - db.BeginTransaction(IsolationLevel.ReadCommitted); - - _sqlServerSyntax.WriteLock(db, Constants.Locks.MainDom); - - // so now we update the row with our appdomain id - InsertLockRecord(_lockId); - _logger.Debug("Acquired with ID {LockId}", _lockId); - return true; - } - catch (Exception ex) - { - ResetDatabase(); - - if (IsLockTimeoutException(ex)) - { - // something is wrong, we cannot acquire, not much we can do - _logger.Error(ex, "Sql timeout occurred, could not forcibly acquire MainDom."); - _hasError = true; - return false; - } - _logger.Error(ex, "Unexpected error, could not forcibly acquire MainDom."); - _hasError = true; - return false; - } - finally - { - db?.CompleteTransaction(); - } + return AcquireWhenMaxWaitTimeElapsed(db); } } }, _cancellationTokenSource.Token); } + private bool? TryAcquire(IUmbracoDatabase db, string tempId, string updatedTempId) + { + using var transaction = db.GetTransaction(IsolationLevel.ReadCommitted); + + try + { + // get a read lock + _sqlServerSyntax.ReadLock(db, Constants.Locks.MainDom); + + // the row + var mainDomRows = db.Fetch("SELECT * FROM umbracoKeyValue WHERE [key] = @key", new { key = MainDomKey }); + + if (mainDomRows.Count == 0 || mainDomRows[0].Value == updatedTempId) + { + // the other main dom has updated our record + // Or the other maindom shutdown super fast and just deleted the record + // which indicates that we + // can acquire it and it has shutdown. + + _sqlServerSyntax.WriteLock(db, Constants.Locks.MainDom); + + // so now we update the row with our appdomain id + InsertLockRecord(_lockId, db); + _logger.Debug("Acquired with ID {LockId}", _lockId); + return true; + } + else if (mainDomRows.Count == 1 && !mainDomRows[0].Value.StartsWith(tempId)) + { + // in this case, the prefixed ID is different which means + // another new AppDomain has come online and is wanting to take over. In that case, we will not + // acquire. + + _logger.Debug("Cannot acquire, another booting application detected."); + return false; + } + } + catch (Exception ex) + { + if (IsLockTimeoutException(ex as SqlException)) + { + _logger.Error(ex, "Sql timeout occurred, waiting for existing MainDom is canceled."); + _errorDuringAcquiring = true; + return false; + } + // unexpected + _logger.Error(ex, "Unexpected error, waiting for existing MainDom is canceled."); + _errorDuringAcquiring = true; + return false; + } + finally + { + transaction.Complete(); + } + + return null; // continue + } + + private bool AcquireWhenMaxWaitTimeElapsed(IUmbracoDatabase db) + { + // if the timeout has elapsed, it either means that the other main dom is taking too long to shutdown, + // or it could mean that the previous appdomain was terminated and didn't clear out the main dom SQL row + // and it's just been left as an orphan row. + // There's really know way of knowing unless we are constantly updating the row for the current maindom + // which isn't ideal. + // So... we're going to 'just' take over, if the writelock works then we'll assume we're ok + + _logger.Debug("Timeout elapsed, assuming orphan row, acquiring MainDom."); + + using var transaction = db.GetTransaction(IsolationLevel.ReadCommitted); + + try + { + _sqlServerSyntax.WriteLock(db, Constants.Locks.MainDom); + + // so now we update the row with our appdomain id + InsertLockRecord(_lockId, db); + _logger.Debug("Acquired with ID {LockId}", _lockId); + return true; + } + catch (Exception ex) + { + if (IsLockTimeoutException(ex as SqlException)) + { + // something is wrong, we cannot acquire, not much we can do + _logger.Error(ex, "Sql timeout occurred, could not forcibly acquire MainDom."); + _errorDuringAcquiring = true; + return false; + } + _logger.Error(ex, "Unexpected error, could not forcibly acquire MainDom."); + _errorDuringAcquiring = true; + return false; + } + finally + { + transaction.Complete(); + } + } + /// /// Inserts or updates the key/value row /// - private RecordPersistenceType InsertLockRecord(string id) + private RecordPersistenceType InsertLockRecord(string id, IUmbracoDatabase db) { - var db = GetDatabase(); return db.InsertOrUpdate(new KeyValueDto { Key = MainDomKey, @@ -354,9 +345,8 @@ namespace Umbraco.Core.Runtime /// Checks if the DB row value is equals the value /// /// - private bool IsMainDomValue(string val) + private bool IsMainDomValue(string val, IUmbracoDatabase db) { - var db = GetDatabase(); return db.ExecuteScalar("SELECT COUNT(*) FROM umbracoKeyValue WHERE [key] = @key AND [value] = @val", new { key = MainDomKey, val = val }) == 1; } @@ -366,7 +356,7 @@ namespace Umbraco.Core.Runtime /// /// /// - private bool IsLockTimeoutException(Exception exception) => exception is SqlException sqlException && sqlException.Number == 1222; + private bool IsLockTimeoutException(SqlException sqlException) => sqlException?.Number == 1222; #region IDisposable Support private bool _disposedValue = false; // To detect redundant calls @@ -385,11 +375,11 @@ namespace Umbraco.Core.Runtime if (_dbFactory.Configured) { - var db = GetDatabase(); + using var db = _dbFactory.CreateDatabase(); + using var transaction = db.GetTransaction(IsolationLevel.ReadCommitted); + try { - db.BeginTransaction(IsolationLevel.ReadCommitted); - // get a write lock _sqlServerSyntax.WriteLock(db, Constants.Locks.MainDom); @@ -402,24 +392,21 @@ namespace Umbraco.Core.Runtime if (_mainDomChanging) { _logger.Debug("Releasing MainDom, updating row, new application is booting."); - db.Execute($"UPDATE umbracoKeyValue SET [value] = [value] + '{UpdatedSuffix}' WHERE [key] = @key", new { key = MainDomKey }); + var count = db.Execute($"UPDATE umbracoKeyValue SET [value] = [value] + '{UpdatedSuffix}' WHERE [key] = @key", new { key = MainDomKey }); } else { _logger.Debug("Releasing MainDom, deleting row, application is shutting down."); - db.Execute("DELETE FROM umbracoKeyValue WHERE [key] = @key", new { key = MainDomKey }); + var count = db.Execute("DELETE FROM umbracoKeyValue WHERE [key] = @key", new { key = MainDomKey }); } } catch (Exception ex) { - ResetDatabase(); _logger.Error(ex, "Unexpected error during dipsose."); - _hasError = true; } finally { - db?.CompleteTransaction(); - ResetDatabase(); + transaction.Complete(); } } } diff --git a/src/Umbraco.Core/Services/Implement/ContentService.cs b/src/Umbraco.Core/Services/Implement/ContentService.cs index 068864a558..882b565e53 100644 --- a/src/Umbraco.Core/Services/Implement/ContentService.cs +++ b/src/Umbraco.Core/Services/Implement/ContentService.cs @@ -30,7 +30,7 @@ namespace Umbraco.Core.Services.Implement private IQuery _queryNotTrashed; //TODO: The non-lazy object should be injected private readonly Lazy _propertyValidationService = new Lazy(() => new PropertyValidationService()); - + #region Constructors @@ -879,7 +879,7 @@ namespace Umbraco.Core.Services.Implement throw new NotSupportedException($"Culture \"{culture}\" is not supported by invariant content types."); } - if(content.Name != null && content.Name.Length > 255) + if (content.Name != null && content.Name.Length > 255) { throw new InvalidOperationException("Name cannot be more than 255 characters in length."); } @@ -1247,7 +1247,7 @@ namespace Umbraco.Core.Services.Implement if (culturesUnpublishing != null) { // This will mean that that we unpublished a mandatory culture or we unpublished the last culture. - + var langs = string.Join(", ", allLangs .Where(x => culturesUnpublishing.InvariantContains(x.IsoCode)) .Select(x => x.CultureName)); @@ -1256,7 +1256,7 @@ namespace Umbraco.Core.Services.Implement if (publishResult == null) throw new PanicException("publishResult == null - should not happen"); - switch(publishResult.Result) + switch (publishResult.Result) { case PublishResultType.FailedPublishMandatoryCultureMissing: //occurs when a mandatory culture was unpublished (which means we tried publishing the document without a mandatory culture) @@ -1270,7 +1270,7 @@ namespace Umbraco.Core.Services.Implement Audit(AuditType.Unpublish, userId, content.Id, "Unpublished (last language unpublished)"); return new PublishResult(PublishResultType.SuccessUnpublishLastCulture, evtMsgs, content); } - + } Audit(AuditType.Unpublish, userId, content.Id); @@ -1290,7 +1290,7 @@ namespace Umbraco.Core.Services.Implement changeType = TreeChangeTypes.RefreshBranch; // whole branch else if (isNew == false && previouslyPublished) changeType = TreeChangeTypes.RefreshNode; // single node - + // invalidate the node/branch if (!branchOne) // for branches, handled by SaveAndPublishBranch @@ -1364,24 +1364,90 @@ namespace Umbraco.Core.Services.Implement /// public IEnumerable PerformScheduledPublish(DateTime date) - => PerformScheduledPublishInternal(date).ToList(); - - // beware! this method yields results, so the returned IEnumerable *must* be - // enumerated for anything to happen - dangerous, so private + exposed via - // the public method above, which forces ToList(). - private IEnumerable PerformScheduledPublishInternal(DateTime date) { + var allLangs = new Lazy>(() => _languageRepository.GetMany().ToList()); var evtMsgs = EventMessagesFactory.Get(); + var results = new List(); - using (var scope = ScopeProvider.CreateScope()) + PerformScheduledPublishingRelease(date, results, evtMsgs, allLangs); + PerformScheduledPublishingExpiration(date, results, evtMsgs, allLangs); + + return results; + } + + private void PerformScheduledPublishingExpiration(DateTime date, List results, EventMessages evtMsgs, Lazy> allLangs) + { + using var scope = ScopeProvider.CreateScope(); + + // do a fast read without any locks since this executes often to see if we even need to proceed + if (_documentRepository.HasContentForExpiration(date)) { + // now take a write lock since we'll be updating scope.WriteLock(Constants.Locks.ContentTree); - var allLangs = _languageRepository.GetMany().ToList(); + foreach (var d in _documentRepository.GetContentForExpiration(date)) + { + if (d.ContentType.VariesByCulture()) + { + //find which cultures have pending schedules + var pendingCultures = d.ContentSchedule.GetPending(ContentScheduleAction.Expire, date) + .Select(x => x.Culture) + .Distinct() + .ToList(); + + if (pendingCultures.Count == 0) + continue; //shouldn't happen but no point in processing this document if there's nothing there + + var saveEventArgs = new ContentSavingEventArgs(d, evtMsgs); + if (scope.Events.DispatchCancelable(Saving, this, saveEventArgs, nameof(Saving))) + { + results.Add(new PublishResult(PublishResultType.FailedPublishCancelledByEvent, evtMsgs, d)); + continue; + } + + foreach (var c in pendingCultures) + { + //Clear this schedule for this culture + d.ContentSchedule.Clear(c, ContentScheduleAction.Expire, date); + //set the culture to be published + d.UnpublishCulture(c); + } + + var result = CommitDocumentChangesInternal(scope, d, saveEventArgs, allLangs.Value, d.WriterId); + if (result.Success == false) + Logger.Error(null, "Failed to publish document id={DocumentId}, reason={Reason}.", d.Id, result.Result); + results.Add(result); + + } + else + { + //Clear this schedule + d.ContentSchedule.Clear(ContentScheduleAction.Expire, date); + var result = Unpublish(d, userId: d.WriterId); + if (result.Success == false) + Logger.Error(null, "Failed to unpublish document id={DocumentId}, reason={Reason}.", d.Id, result.Result); + results.Add(result); + } + } + + _documentRepository.ClearSchedule(date, ContentScheduleAction.Expire); + } + + scope.Complete(); + } + + private void PerformScheduledPublishingRelease(DateTime date, List results, EventMessages evtMsgs, Lazy> allLangs) + { + using var scope = ScopeProvider.CreateScope(); + + // do a fast read without any locks since this executes often to see if we even need to proceed + if (_documentRepository.HasContentForRelease(date)) + { + // now take a write lock since we'll be updating + scope.WriteLock(Constants.Locks.ContentTree); foreach (var d in _documentRepository.GetContentForRelease(date)) { - PublishResult result; if (d.ContentType.VariesByCulture()) { //find which cultures have pending schedules @@ -1391,11 +1457,14 @@ namespace Umbraco.Core.Services.Implement .ToList(); if (pendingCultures.Count == 0) - break; //shouldn't happen but no point in continuing if there's nothing there + continue; //shouldn't happen but no point in processing this document 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); + { + results.Add(new PublishResult(PublishResultType.FailedPublishCancelledByEvent, evtMsgs, d)); + continue; // this document is canceled move next + } var publishing = true; foreach (var culture in pendingCultures) @@ -1407,94 +1476,51 @@ namespace Umbraco.Core.Services.Implement //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 impact = CultureImpact.Explicit(culture, IsDefaultCulture(allLangs, culture)); + var impact = CultureImpact.Explicit(culture, IsDefaultCulture(allLangs.Value, culture)); var tryPublish = d.PublishCulture(impact) && _propertyValidationService.Value.IsPropertyDataValid(d, out invalidProperties, impact); if (invalidProperties != null && invalidProperties.Length > 0) Logger.Warn("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 (!publishing) continue; // move to next document } + PublishResult result; + if (d.Trashed) result = new PublishResult(PublishResultType.FailedPublishIsTrashed, evtMsgs, d); else if (!publishing) result = new PublishResult(PublishResultType.FailedPublishContentInvalid, evtMsgs, d); else - result = CommitDocumentChangesInternal(scope, d, saveEventArgs, allLangs, d.WriterId); - + result = CommitDocumentChangesInternal(scope, d, saveEventArgs, allLangs.Value, d.WriterId); if (result.Success == false) Logger.Error(null, "Failed to publish document id={DocumentId}, reason={Reason}.", d.Id, result.Result); - yield return result; + results.Add(result); } else { //Clear this schedule d.ContentSchedule.Clear(ContentScheduleAction.Release, date); - result = d.Trashed + var result = d.Trashed ? new PublishResult(PublishResultType.FailedPublishIsTrashed, evtMsgs, d) : SaveAndPublish(d, userId: d.WriterId); if (result.Success == false) Logger.Error(null, "Failed to publish document id={DocumentId}, reason={Reason}.", d.Id, result.Result); - yield return result; + results.Add(result); } } - foreach (var d in _documentRepository.GetContentForExpiration(date)) - { - PublishResult result; - if (d.ContentType.VariesByCulture()) - { - //find which cultures have pending schedules - var pendingCultures = d.ContentSchedule.GetPending(ContentScheduleAction.Expire, date) - .Select(x => x.Culture) - .Distinct() - .ToList(); + _documentRepository.ClearSchedule(date, ContentScheduleAction.Release); - 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 - d.ContentSchedule.Clear(c, ContentScheduleAction.Expire, date); - //set the culture to be published - d.UnpublishCulture(c); - } - - result = CommitDocumentChangesInternal(scope, d, saveEventArgs, allLangs, d.WriterId); - if (result.Success == false) - Logger.Error(null, "Failed to publish document id={DocumentId}, reason={Reason}.", d.Id, result.Result); - yield return result; - - } - else - { - //Clear this schedule - d.ContentSchedule.Clear(ContentScheduleAction.Expire, date); - result = Unpublish(d, userId: d.WriterId); - if (result.Success == false) - Logger.Error(null, "Failed to unpublish document id={DocumentId}, reason={Reason}.", d.Id, result.Result); - yield return result; - } - - - } - - _documentRepository.ClearSchedule(date); - - scope.Complete(); } + + scope.Complete(); } // utility 'PublishCultures' func used by SaveAndPublishBranch @@ -2650,7 +2676,7 @@ namespace Umbraco.Core.Services.Implement // there will be nothing to publish/unpublish. return new PublishResult(PublishResultType.FailedPublishNothingToPublish, evtMsgs, content); } - + // missing mandatory culture = cannot be published var mandatoryCultures = allLangs.Where(x => x.IsMandatory).Select(x => x.IsoCode); @@ -3163,6 +3189,6 @@ namespace Umbraco.Core.Services.Implement #endregion - + } } diff --git a/src/Umbraco.Core/Services/PropertyValidationService.cs b/src/Umbraco.Core/Services/PropertyValidationService.cs index a037a83920..d83628a316 100644 --- a/src/Umbraco.Core/Services/PropertyValidationService.cs +++ b/src/Umbraco.Core/Services/PropertyValidationService.cs @@ -1,9 +1,7 @@ using System; using System.Collections.Generic; +using System.ComponentModel.DataAnnotations; 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; @@ -15,19 +13,73 @@ namespace Umbraco.Core.Services { private readonly PropertyEditorCollection _propertyEditors; private readonly IDataTypeService _dataTypeService; + private readonly ILocalizedTextService _textService; - public PropertyValidationService(PropertyEditorCollection propertyEditors, IDataTypeService dataTypeService) + public PropertyValidationService(PropertyEditorCollection propertyEditors, IDataTypeService dataTypeService, ILocalizedTextService textService) { _propertyEditors = propertyEditors; _dataTypeService = dataTypeService; + _textService = textService; } //TODO: Remove this method in favor of the overload specifying all dependencies public PropertyValidationService() - : this(Current.PropertyEditors, Current.Services.DataTypeService) + : this(Current.PropertyEditors, Current.Services.DataTypeService, Current.Services.TextService) { } + public IEnumerable ValidatePropertyValue( + PropertyType propertyType, + object postedValue) + { + if (propertyType is null) throw new ArgumentNullException(nameof(propertyType)); + var dataType = _dataTypeService.GetDataType(propertyType.DataTypeId); + if (dataType == null) throw new InvalidOperationException("No data type found by id " + propertyType.DataTypeId); + + var editor = _propertyEditors[propertyType.PropertyEditorAlias]; + if (editor == null) throw new InvalidOperationException("No property editor found by alias " + propertyType.PropertyEditorAlias); + + return ValidatePropertyValue(_textService, editor, dataType, postedValue, propertyType.Mandatory, propertyType.ValidationRegExp, propertyType.MandatoryMessage, propertyType.ValidationRegExpMessage); + } + + internal static IEnumerable ValidatePropertyValue( + ILocalizedTextService textService, + IDataEditor editor, + IDataType dataType, + object postedValue, + bool isRequired, + string validationRegExp, + string isRequiredMessage, + string validationRegExpMessage) + { + // Retrieve default messages used for required and regex validatation. We'll replace these + // if set with custom ones if they've been provided for a given property. + var requiredDefaultMessages = new[] + { + textService.Localize("validation", "invalidNull"), + textService.Localize("validation", "invalidEmpty") + }; + var formatDefaultMessages = new[] + { + textService.Localize("validation", "invalidPattern"), + }; + + var valueEditor = editor.GetValueEditor(dataType.Configuration); + foreach (var validationResult in valueEditor.Validate(postedValue, isRequired, validationRegExp)) + { + // If we've got custom error messages, we'll replace the default ones that will have been applied in the call to Validate(). + if (isRequired && !string.IsNullOrWhiteSpace(isRequiredMessage) && requiredDefaultMessages.Contains(validationResult.ErrorMessage, StringComparer.OrdinalIgnoreCase)) + { + validationResult.ErrorMessage = isRequiredMessage; + } + if (!string.IsNullOrWhiteSpace(validationRegExp) && !string.IsNullOrWhiteSpace(validationRegExpMessage) && formatDefaultMessages.Contains(validationResult.ErrorMessage, StringComparer.OrdinalIgnoreCase)) + { + validationResult.ErrorMessage = validationRegExpMessage; + } + yield return validationResult; + } + } + /// /// Validates the content item's properties pass validation rules /// diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index 9e4e36928e..a5c1141d8b 100755 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -132,6 +132,11 @@ + + + + + diff --git a/src/Umbraco.Examine/IndexRebuilder.cs b/src/Umbraco.Examine/IndexRebuilder.cs index 786aecac71..b14ff25c57 100644 --- a/src/Umbraco.Examine/IndexRebuilder.cs +++ b/src/Umbraco.Examine/IndexRebuilder.cs @@ -3,6 +3,8 @@ using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; using Examine; +using Umbraco.Core.Composing; +using Umbraco.Core.Logging; namespace Umbraco.Examine { @@ -12,12 +14,20 @@ namespace Umbraco.Examine /// public class IndexRebuilder { + private readonly IProfilingLogger _logger; private readonly IEnumerable _populators; public IExamineManager ExamineManager { get; } + [Obsolete("Use constructor with all dependencies")] public IndexRebuilder(IExamineManager examineManager, IEnumerable populators) + : this(Current.ProfilingLogger, examineManager, populators) + { + } + + public IndexRebuilder(IProfilingLogger logger, IExamineManager examineManager, IEnumerable populators) { _populators = populators; + _logger = logger; ExamineManager = examineManager; } @@ -50,8 +60,18 @@ namespace Umbraco.Examine index.CreateIndex(); // clear the index } - //run the populators in parallel against all indexes - Parallel.ForEach(_populators, populator => populator.Populate(indexes)); + // run each populator over the indexes + foreach(var populator in _populators) + { + try + { + populator.Populate(indexes); + } + catch (Exception e) + { + _logger.Error(e, "Index populating failed for populator {Populator}", populator.GetType()); + } + } } } diff --git a/src/Umbraco.ModelsBuilder.Embedded/PureLiveModelFactory.cs b/src/Umbraco.ModelsBuilder.Embedded/PureLiveModelFactory.cs index 0e125759c6..912d0e3363 100644 --- a/src/Umbraco.ModelsBuilder.Embedded/PureLiveModelFactory.cs +++ b/src/Umbraco.ModelsBuilder.Embedded/PureLiveModelFactory.cs @@ -542,8 +542,10 @@ namespace Umbraco.ModelsBuilder.Embedded if (modelInfos.TryGetValue(typeName, out var modelInfo)) throw new InvalidOperationException($"Both types {type.FullName} and {modelInfo.ModelType.FullName} want to be a model type for content type with alias \"{typeName}\"."); - // fixme use Core's ReflectionUtilities.EmitCtor !! + // TODO: use Core's ReflectionUtilities.EmitCtor !! // Yes .. DynamicMethod is uber slow + // TODO: But perhaps https://docs.microsoft.com/en-us/dotnet/api/system.reflection.emit.constructorbuilder?view=netcore-3.1 is better still? + // See CtorInvokeBenchmarks var meth = new DynamicMethod(string.Empty, typeof(IPublishedElement), ctorArgTypes, type.Module, true); var gen = meth.GetILGenerator(); gen.Emit(OpCodes.Ldarg_0); diff --git a/src/Umbraco.TestData/LoadTestController.cs b/src/Umbraco.TestData/LoadTestController.cs new file mode 100644 index 0000000000..97665dd084 --- /dev/null +++ b/src/Umbraco.TestData/LoadTestController.cs @@ -0,0 +1,371 @@ +using System; +using System.Threading; +using System.Linq; +using System.Web.Mvc; +using Umbraco.Core.Services; +using Umbraco.Core.Models; +using System.Web; +using System.Web.Hosting; +using System.Web.Routing; +using System.Diagnostics; +using Umbraco.Core.Composing; +using System.Configuration; + +// see https://github.com/Shazwazza/UmbracoScripts/tree/master/src/LoadTesting + +namespace Umbraco.TestData +{ + public class LoadTestController : Controller + { + public LoadTestController(ServiceContext serviceContext) + { + _serviceContext = serviceContext; + } + + private static readonly Random _random = new Random(); + private static readonly object _locko = new object(); + + private static volatile int _containerId = -1; + + private const string _containerAlias = "LoadTestContainer"; + private const string _contentAlias = "LoadTestContent"; + private const int _textboxDefinitionId = -88; + private const int _maxCreate = 1000; + + private static readonly string HeadHtml = @" + + LoadTest + + + +
+

LoadTest

+
" + System.Configuration.ConfigurationManager.AppSettings["umbracoConfigurationStatus"] + @"
+
+"; + + private const string FootHtml = @" +"; + + private static readonly string _containerTemplateText = @" +@inherits Umbraco.Web.Mvc.UmbracoViewPage +@{ + Layout = null; + var container = Umbraco.ContentAtRoot().OfTypes(""" + _containerAlias + @""").FirstOrDefault(); + var contents = container.Children().ToArray(); + var groups = contents.GroupBy(x => x.Value(""origin"")); + var id = contents.Length > 0 ? contents[0].Id : -1; + var wurl = Request.QueryString[""u""] == ""1""; + var missing = contents.Length > 0 && contents[contents.Length - 1].Id - contents[0].Id >= contents.Length; +} +" + HeadHtml + @" +
+@contents.Length items +
    +@foreach (var group in groups) +{ +
  • @group.Key: @group.Count()
    • +} +
+
+@foreach (var content in contents) +{ + while (content.Id > id) + { +
@id :: MISSING
+ id++; + } + if (wurl) + { +
@content.Id :: @content.Name :: @content.Url
+ } + else + { +
@content.Id :: @content.Name
+ } id++; +} +
+" + FootHtml; + private readonly ServiceContext _serviceContext; + + private ActionResult ContentHtml(string s) + { + return Content(HeadHtml + s + FootHtml); + } + + public ActionResult Index() + { + var res = EnsureInitialize(); + if (res != null) return res; + + var html = @"Welcome. You can: + +"; + + return ContentHtml(html); + } + + private ActionResult EnsureInitialize() + { + if (_containerId > 0) return null; + + lock (_locko) + { + if (_containerId > 0) return null; + + var contentTypeService = _serviceContext.ContentTypeService; + var contentType = contentTypeService.Get(_contentAlias); + if (contentType == null) + return ContentHtml("Not installed, first you must install."); + + var containerType = contentTypeService.Get(_containerAlias); + if (containerType == null) + return ContentHtml("Panic! Container type is missing."); + + var contentService = _serviceContext.ContentService; + var container = contentService.GetPagedOfType(containerType.Id, 0, 100, out _, null).FirstOrDefault(); + if (container == null) + return ContentHtml("Panic! Container is missing."); + + _containerId = container.Id; + return null; + } + } + + public ActionResult Install() + { + var dataTypeService = _serviceContext.DataTypeService; + + //var dataType = dataTypeService.GetAll(Constants.DataTypes.DefaultContentListView); + + + //if (!dict.ContainsKey("pageSize")) dict["pageSize"] = new PreValue("10"); + //dict["pageSize"].Value = "200"; + //dataTypeService.SavePreValues(dataType, dict); + + var contentTypeService = _serviceContext.ContentTypeService; + + var contentType = new ContentType(-1) + { + Alias = _contentAlias, + Name = "LoadTest Content", + Description = "Content for LoadTest", + Icon = "icon-document" + }; + var def = _serviceContext.DataTypeService.GetDataType(_textboxDefinitionId); + contentType.AddPropertyType(new PropertyType(def) + { + Name = "Origin", + Alias = "origin", + Description = "The origin of the content.", + }); + contentTypeService.Save(contentType); + + var containerTemplate = ImportTemplate(_serviceContext, + "LoadTestContainer", "LoadTestContainer", _containerTemplateText); + + var containerType = new ContentType(-1) + { + Alias = _containerAlias, + Name = "LoadTest Container", + Description = "Container for LoadTest content", + Icon = "icon-document", + AllowedAsRoot = true, + IsContainer = true + }; + containerType.AllowedContentTypes = containerType.AllowedContentTypes.Union(new[] + { + new ContentTypeSort(new Lazy(() => contentType.Id), 0, contentType.Alias), + }); + containerType.AllowedTemplates = containerType.AllowedTemplates.Union(new[] { containerTemplate }); + containerType.SetDefaultTemplate(containerTemplate); + contentTypeService.Save(containerType); + + var contentService = _serviceContext.ContentService; + var content = contentService.Create("LoadTestContainer", -1, _containerAlias); + contentService.SaveAndPublish(content); + + return ContentHtml("Installed."); + } + + public ActionResult Create(int n = 1, int r = 0, string o = null) + { + var res = EnsureInitialize(); + if (res != null) return res; + + if (r < 0) r = 0; + if (r > 100) r = 100; + var restart = GetRandom(0, 100) > (100 - r); + + var contentService = _serviceContext.ContentService; + + if (n < 1) n = 1; + if (n > _maxCreate) n = _maxCreate; + for (int i = 0; i < n; i++) + { + var name = Guid.NewGuid().ToString("N").ToUpper() + "-" + (restart ? "R" : "X") + "-" + o; + var content = contentService.Create(name, _containerId, _contentAlias); + content.SetValue("origin", o); + contentService.SaveAndPublish(content); + } + + if (restart) + DoRestart(); + + return ContentHtml("Created " + n + " content" + + (restart ? ", and restarted" : "") + + "."); + } + + private int GetRandom(int minValue, int maxValue) + { + lock (_locko) + { + return _random.Next(minValue, maxValue); + } + } + + public ActionResult Clear() + { + var res = EnsureInitialize(); + if (res != null) return res; + + var contentType = _serviceContext.ContentTypeService.Get(_contentAlias); + _serviceContext.ContentService.DeleteOfType(contentType.Id); + + return ContentHtml("Cleared."); + } + + private void DoRestart() + { + HttpContext.User = null; + System.Web.HttpContext.Current.User = null; + Thread.CurrentPrincipal = null; + HttpRuntime.UnloadAppDomain(); + } + + public ActionResult Restart() + { + DoRestart(); + + return ContentHtml("Restarted."); + } + + public ActionResult Die() + { + var timer = new System.Threading.Timer(_ => + { + throw new Exception("die!"); + }); + timer.Change(100, 0); + + return ContentHtml("Dying."); + } + + public ActionResult Domains() + { + var currentDomain = AppDomain.CurrentDomain; + var currentName = currentDomain.FriendlyName; + var pos = currentName.IndexOf('-'); + if (pos > 0) currentName = currentName.Substring(0, pos); + + var text = new System.Text.StringBuilder(); + text.Append("
Process ID: " + Process.GetCurrentProcess().Id + "
"); + text.Append("
"); + text.Append("
IIS Site: " + HostingEnvironment.ApplicationHost.GetSiteName() + "
"); + text.Append("
App ID: " + currentName + "
"); + //text.Append("
AppPool: " + Zbu.WebManagement.AppPoolHelper.GetCurrentApplicationPoolName() + "
"); + text.Append("
"); + + text.Append("
Domains:
    "); + text.Append("
  • Not implemented.
    • "); + /* + foreach (var domain in Zbu.WebManagement.AppDomainHelper.GetAppDomains().OrderBy(x => x.Id)) + { + var name = domain.FriendlyName; + pos = name.IndexOf('-'); + if (pos > 0) name = name.Substring(0, pos); + text.Append("
  • " + +"[" + domain.Id + "] " + name + + (domain.IsDefaultAppDomain() ? " (default)" : "") + + (domain.Id == currentDomain.Id ? " (current)" : "") + + "
    • "); + } + */ + text.Append("
"); + + return ContentHtml(text.ToString()); + } + + public ActionResult Recycle() + { + return ContentHtml("Not implemented—please use IIS console."); + } + + private static Template ImportTemplate(ServiceContext svces, string name, string alias, string text, ITemplate master = null) + { + var t = new Template(name, alias) { Content = text }; + if (master != null) + t.SetMasterTemplate(master); + svces.FileService.SaveTemplate(t); + return t; + } + } + + public class TestComponent : IComponent + { + public void Initialize() + { + if (ConfigurationManager.AppSettings["Umbraco.TestData.Enabled"] != "true") + return; + + RouteTable.Routes.MapRoute( + name: "LoadTest", + url: "LoadTest/{action}", + defaults: new + { + controller = "LoadTest", + action = "Index" + }, + namespaces: new[] { "Umbraco.TestData" } + ); + } + + public void Terminate() + { + } + } + + public class TestComposer : ComponentComposer, IUserComposer + { + public override void Compose(Composition composition) + { + base.Compose(composition); + + if (ConfigurationManager.AppSettings["Umbraco.TestData.Enabled"] != "true") + return; + + composition.Register(typeof(LoadTestController), Lifetime.Request); + } + } +} diff --git a/src/Umbraco.TestData/Umbraco.TestData.csproj b/src/Umbraco.TestData/Umbraco.TestData.csproj index d61321ebb8..a3753cc17b 100644 --- a/src/Umbraco.TestData/Umbraco.TestData.csproj +++ b/src/Umbraco.TestData/Umbraco.TestData.csproj @@ -41,6 +41,7 @@ + diff --git a/src/Umbraco.Tests.Benchmarks/CtorInvokeBenchmarks.cs b/src/Umbraco.Tests.Benchmarks/CtorInvokeBenchmarks.cs index 8d15613791..37fe952851 100644 --- a/src/Umbraco.Tests.Benchmarks/CtorInvokeBenchmarks.cs +++ b/src/Umbraco.Tests.Benchmarks/CtorInvokeBenchmarks.cs @@ -16,6 +16,8 @@ namespace Umbraco.Tests.Benchmarks // - it's faster to get+invoke the ctor // - emitting the ctor is unless if invoked only 1 + // TODO: Check out https://docs.microsoft.com/en-us/dotnet/api/system.reflection.emit.constructorbuilder?view=netcore-3.1 ? + //[Config(typeof(Config))] [MemoryDiagnoser] public class CtorInvokeBenchmarks diff --git a/src/Umbraco.Tests/Models/VariationTests.cs b/src/Umbraco.Tests/Models/VariationTests.cs index ab5f726894..b87ff499b6 100644 --- a/src/Umbraco.Tests/Models/VariationTests.cs +++ b/src/Umbraco.Tests/Models/VariationTests.cs @@ -442,7 +442,10 @@ namespace Umbraco.Tests.Models [Test] public void ContentPublishValuesWithMixedPropertyTypeVariations() { - var propertyValidationService = new PropertyValidationService(Current.Factory.GetInstance(), Current.Factory.GetInstance().DataTypeService); + var propertyValidationService = new PropertyValidationService( + Current.Factory.GetInstance(), + Current.Factory.GetInstance().DataTypeService, + Current.Factory.GetInstance().TextService); const string langFr = "fr-FR"; // content type varies by Culture @@ -574,7 +577,10 @@ 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(), Current.Factory.GetInstance().DataTypeService); + var propertyValidationService = new PropertyValidationService( + Current.Factory.GetInstance(), + Current.Factory.GetInstance().DataTypeService, + Current.Factory.GetInstance().TextService); Assert.IsTrue(propertyValidationService.IsPropertyValid(prop)); diff --git a/src/Umbraco.Tests/PropertyEditors/BlockListPropertyValueConverterTests.cs b/src/Umbraco.Tests/PropertyEditors/BlockListPropertyValueConverterTests.cs index 655db4e337..d2bc34e633 100644 --- a/src/Umbraco.Tests/PropertyEditors/BlockListPropertyValueConverterTests.cs +++ b/src/Umbraco.Tests/PropertyEditors/BlockListPropertyValueConverterTests.cs @@ -165,7 +165,7 @@ namespace Umbraco.Tests.PropertyEditors Assert.AreEqual(0, converted.Layout.Count()); json = @"{ -layout: [], +layout: {}, data: []}"; converted = editor.ConvertIntermediateToObject(publishedElement, propertyType, PropertyCacheLevel.None, json, false) as BlockListModel; diff --git a/src/Umbraco.Tests/Services/ContentServiceTests.cs b/src/Umbraco.Tests/Services/ContentServiceTests.cs index 553d1b97ba..041dabe7d2 100644 --- a/src/Umbraco.Tests/Services/ContentServiceTests.cs +++ b/src/Umbraco.Tests/Services/ContentServiceTests.cs @@ -1197,7 +1197,7 @@ namespace Umbraco.Tests.Services Assert.IsFalse(content.HasIdentity); // content cannot publish values because they are invalid - var propertyValidationService = new PropertyValidationService(Factory.GetInstance(), ServiceContext.DataTypeService); + var propertyValidationService = new PropertyValidationService(Factory.GetInstance(), ServiceContext.DataTypeService, ServiceContext.TextService); var isValid = propertyValidationService.IsPropertyDataValid(content, out var invalidProperties, CultureImpact.Invariant); Assert.IsFalse(isValid); Assert.IsNotEmpty(invalidProperties); diff --git a/src/Umbraco.Tests/Services/PropertyValidationServiceTests.cs b/src/Umbraco.Tests/Services/PropertyValidationServiceTests.cs index e1e19918ce..28bdf59373 100644 --- a/src/Umbraco.Tests/Services/PropertyValidationServiceTests.cs +++ b/src/Umbraco.Tests/Services/PropertyValidationServiceTests.cs @@ -36,7 +36,7 @@ namespace Umbraco.Tests.Services var propEditors = new PropertyEditorCollection(new DataEditorCollection(new[] { dataEditor })); - validationService = new PropertyValidationService(propEditors, dataTypeService.Object); + validationService = new PropertyValidationService(propEditors, dataTypeService.Object, Mock.Of()); } [Test] diff --git a/src/Umbraco.Tests/TestHelpers/Entities/MockedContentTypes.cs b/src/Umbraco.Tests/TestHelpers/Entities/MockedContentTypes.cs index c55467431d..b4cd4ab05e 100644 --- a/src/Umbraco.Tests/TestHelpers/Entities/MockedContentTypes.cs +++ b/src/Umbraco.Tests/TestHelpers/Entities/MockedContentTypes.cs @@ -41,12 +41,12 @@ namespace Umbraco.Tests.TestHelpers.Entities }; var contentCollection = new PropertyTypeCollection(true); - contentCollection.Add(new PropertyType("test", ValueStorageType.Ntext) { Alias = "title", Name = "Title", Description = "", Mandatory = false, SortOrder = 1, DataTypeId = -88 }); - contentCollection.Add(new PropertyType("test", ValueStorageType.Ntext) { Alias = "bodyText", Name = "Body Text", Description = "", Mandatory = false, SortOrder = 2, DataTypeId = -87 }); + contentCollection.Add(new PropertyType("test", ValueStorageType.Ntext) { Alias = "title", Name = "Title", Description = "", Mandatory = false, SortOrder = 1, DataTypeId = Constants.DataTypes.Textbox }); + contentCollection.Add(new PropertyType("test", ValueStorageType.Ntext) { Alias = "bodyText", Name = "Body Text", Description = "", Mandatory = false, SortOrder = 2, DataTypeId = Constants.DataTypes.RichtextEditor }); var metaCollection = new PropertyTypeCollection(true); - metaCollection.Add(new PropertyType("test", ValueStorageType.Ntext) { Alias = "keywords", Name = "Meta Keywords", Description = "", Mandatory = false, SortOrder = 1, DataTypeId = -88 }); - metaCollection.Add(new PropertyType("test", ValueStorageType.Ntext) { Alias = "description", Name = "Meta Description", Description = "", Mandatory = false, SortOrder = 2, DataTypeId = -89 }); + metaCollection.Add(new PropertyType("test", ValueStorageType.Ntext) { Alias = "keywords", Name = "Meta Keywords", Description = "", Mandatory = false, SortOrder = 1, DataTypeId = Constants.DataTypes.Textbox }); + metaCollection.Add(new PropertyType("test", ValueStorageType.Ntext) { Alias = "description", Name = "Meta Description", Description = "", Mandatory = false, SortOrder = 2, DataTypeId = Constants.DataTypes.Textarea }); contentType.PropertyGroups.Add(new PropertyGroup(contentCollection) { Name = "Content", SortOrder = 1 }); contentType.PropertyGroups.Add(new PropertyGroup(metaCollection) { Name = "Meta", SortOrder = 2 }); diff --git a/src/Umbraco.Tests/Umbraco.Tests.csproj b/src/Umbraco.Tests/Umbraco.Tests.csproj index 82464f3560..f803f1cd1d 100644 --- a/src/Umbraco.Tests/Umbraco.Tests.csproj +++ b/src/Umbraco.Tests/Umbraco.Tests.csproj @@ -267,7 +267,7 @@ - + @@ -514,6 +514,7 @@ + diff --git a/src/Umbraco.Tests/Web/Validation/ContentModelValidatorTests.cs b/src/Umbraco.Tests/Web/Validation/ContentModelValidatorTests.cs new file mode 100644 index 0000000000..fbb2ce9c80 --- /dev/null +++ b/src/Umbraco.Tests/Web/Validation/ContentModelValidatorTests.cs @@ -0,0 +1,369 @@ +using Moq; +using NUnit.Framework; +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; +using Umbraco.Core.Services; +using Umbraco.Core.Logging; +using Umbraco.Web; +using Umbraco.Web.Editors.Filters; +using Umbraco.Tests.TestHelpers.Entities; +using Umbraco.Core.Models; +using Umbraco.Web.Models.ContentEditing; +using Umbraco.Web.Editors.Binders; +using Umbraco.Core; +using Umbraco.Tests.Testing; +using Umbraco.Core.Mapping; +using Umbraco.Core.PropertyEditors; +using Umbraco.Core.Composing; +using System.Web.Http.ModelBinding; +using Umbraco.Web.PropertyEditors; +using System.ComponentModel.DataAnnotations; +using Umbraco.Tests.TestHelpers; +using System.Globalization; +using Newtonsoft.Json; +using Newtonsoft.Json.Linq; +using Umbraco.Web.PropertyEditors.Validation; + +namespace Umbraco.Tests.Web.Validation +{ + [UmbracoTest(Mapper = true, WithApplication = true, Logger = UmbracoTestOptions.Logger.Console)] + [TestFixture] + public class ContentModelValidatorTests : UmbracoTestBase + { + private const int ComplexDataTypeId = 9999; + private const string ContentTypeAlias = "textPage"; + private ContentType _contentType; + + public override void SetUp() + { + base.SetUp(); + + _contentType = MockedContentTypes.CreateTextPageContentType(ContentTypeAlias); + // add complex editor + _contentType.AddPropertyType( + new PropertyType("complexTest", ValueStorageType.Ntext) { Alias = "complex", Name = "Complex", Description = "", Mandatory = false, SortOrder = 1, DataTypeId = ComplexDataTypeId }, + "Content"); + + // make them all validate with a regex rule that will not pass + foreach (var prop in _contentType.PropertyTypes) + { + prop.ValidationRegExp = "^donotmatch$"; + prop.ValidationRegExpMessage = "Does not match!"; + } + } + + protected override void Compose() + { + base.Compose(); + + var complexEditorConfig = new NestedContentConfiguration + { + ContentTypes = new[] + { + new NestedContentConfiguration.ContentType { Alias = "feature" } + } + }; + var dataTypeService = new Mock(); + dataTypeService.Setup(x => x.GetDataType(It.IsAny())) + .Returns((int id) => id == ComplexDataTypeId + ? Mock.Of(x => x.Configuration == complexEditorConfig) + : Mock.Of()); + + var contentTypeService = new Mock(); + contentTypeService.Setup(x => x.GetAll(It.IsAny())) + .Returns(() => new List + { + _contentType + }); + + var textService = new Mock(); + textService.Setup(x => x.Localize("validation/invalidPattern", It.IsAny(), It.IsAny>())).Returns(() => "invalidPattern"); + textService.Setup(x => x.Localize("validation/invalidNull", It.IsAny(), It.IsAny>())).Returns("invalidNull"); + textService.Setup(x => x.Localize("validation/invalidEmpty", It.IsAny(), It.IsAny>())).Returns("invalidEmpty"); + + Composition.RegisterUnique(x => Mock.Of(x => x.GetDataType(It.IsAny()) == Mock.Of())); + Composition.RegisterUnique(x => dataTypeService.Object); + Composition.RegisterUnique(x => contentTypeService.Object); + Composition.RegisterUnique(x => textService.Object); + + Composition.WithCollectionBuilder() + .Add() + .Add(); + } + + [Test] + public void Test_Serializer() + { + var nestedLevel2 = new ComplexEditorValidationResult(); + var id1 = Guid.NewGuid(); + var addressInfoElementTypeResult = new ComplexEditorElementTypeValidationResult("addressInfo", id1); + var cityPropertyTypeResult = new ComplexEditorPropertyTypeValidationResult("city"); + cityPropertyTypeResult.AddValidationResult(new ValidationResult("City is invalid")); + cityPropertyTypeResult.AddValidationResult(new ValidationResult("City cannot be empty")); + cityPropertyTypeResult.AddValidationResult(new ValidationResult("City is not in Australia", new[] { "country" })); + cityPropertyTypeResult.AddValidationResult(new ValidationResult("Not a capital city", new[] { "capital" })); + addressInfoElementTypeResult.ValidationResults.Add(cityPropertyTypeResult); + nestedLevel2.ValidationResults.Add(addressInfoElementTypeResult); + + var nestedLevel1 = new ComplexEditorValidationResult(); + var id2 = Guid.NewGuid(); + var addressBookElementTypeResult = new ComplexEditorElementTypeValidationResult("addressBook", id2); + var addressesPropertyTypeResult = new ComplexEditorPropertyTypeValidationResult("addresses"); + addressesPropertyTypeResult.AddValidationResult(new ValidationResult("Must have at least 3 addresses", new[] { "counter" })); + addressesPropertyTypeResult.AddValidationResult(nestedLevel2); // This is a nested result within the level 1 + addressBookElementTypeResult.ValidationResults.Add(addressesPropertyTypeResult); + var bookNamePropertyTypeResult = new ComplexEditorPropertyTypeValidationResult("bookName"); + bookNamePropertyTypeResult.AddValidationResult(new ValidationResult("Invalid address book name", new[] { "book" })); + addressBookElementTypeResult.ValidationResults.Add(bookNamePropertyTypeResult); + nestedLevel1.ValidationResults.Add(addressBookElementTypeResult); + + var id3 = Guid.NewGuid(); + var addressBookElementTypeResult2 = new ComplexEditorElementTypeValidationResult("addressBook", id3); + var addressesPropertyTypeResult2 = new ComplexEditorPropertyTypeValidationResult("addresses"); + addressesPropertyTypeResult2.AddValidationResult(new ValidationResult("Must have at least 2 addresses", new[] { "counter" })); + addressBookElementTypeResult2.ValidationResults.Add(addressesPropertyTypeResult); + var bookNamePropertyTypeResult2 = new ComplexEditorPropertyTypeValidationResult("bookName"); + bookNamePropertyTypeResult2.AddValidationResult(new ValidationResult("Name is too long")); + addressBookElementTypeResult2.ValidationResults.Add(bookNamePropertyTypeResult2); + nestedLevel1.ValidationResults.Add(addressBookElementTypeResult2); + + // books is the outer most validation result and doesn't have it's own direct ValidationResult errors + var outerError = new ComplexEditorValidationResult(); + var id4 = Guid.NewGuid(); + var addressBookCollectionElementTypeResult = new ComplexEditorElementTypeValidationResult("addressBookCollection", id4); + var booksPropertyTypeResult= new ComplexEditorPropertyTypeValidationResult("books"); + booksPropertyTypeResult.AddValidationResult(nestedLevel1); // books is the outer most validation result + addressBookCollectionElementTypeResult.ValidationResults.Add(booksPropertyTypeResult); + outerError.ValidationResults.Add(addressBookCollectionElementTypeResult); + + var serialized = JsonConvert.SerializeObject(outerError, Formatting.Indented, new ValidationResultConverter()); + Console.WriteLine(serialized); + + var jsonError = JsonConvert.DeserializeObject(serialized); + + Assert.IsNotNull(jsonError.SelectToken("$[0]")); + Assert.AreEqual(id4.ToString(), jsonError.SelectToken("$[0].$id").Value()); + Assert.AreEqual("addressBookCollection", jsonError.SelectToken("$[0].$elementTypeAlias").Value()); + Assert.AreEqual(string.Empty, jsonError.SelectToken("$[0].ModelState['_Properties.books.invariant.null'][0]").Value()); + + var error0 = jsonError.SelectToken("$[0].books") as JArray; + Assert.IsNotNull(error0); + Assert.AreEqual(id2.ToString(), error0.SelectToken("$[0].$id").Value()); + Assert.AreEqual("addressBook", error0.SelectToken("$[0].$elementTypeAlias").Value()); + Assert.IsNotNull(error0.SelectToken("$[0].ModelState")); + Assert.AreEqual(string.Empty, error0.SelectToken("$[0].ModelState['_Properties.addresses.invariant.null'][0]").Value()); + var error1 = error0.SelectToken("$[0].ModelState['_Properties.addresses.invariant.null.counter']") as JArray; + Assert.IsNotNull(error1); + Assert.AreEqual(1, error1.Count); + var error2 = error0.SelectToken("$[0].ModelState['_Properties.bookName.invariant.null.book']") as JArray; + Assert.IsNotNull(error2); + Assert.AreEqual(1, error2.Count); + + Assert.AreEqual(id3.ToString(), error0.SelectToken("$[1].$id").Value()); + Assert.AreEqual("addressBook", error0.SelectToken("$[1].$elementTypeAlias").Value()); + Assert.IsNotNull(error0.SelectToken("$[1].ModelState")); + Assert.AreEqual(string.Empty, error0.SelectToken("$[1].ModelState['_Properties.addresses.invariant.null'][0]").Value()); + var error6 = error0.SelectToken("$[1].ModelState['_Properties.addresses.invariant.null.counter']") as JArray; + Assert.IsNotNull(error6); + Assert.AreEqual(1, error6.Count); + var error7 = error0.SelectToken("$[1].ModelState['_Properties.bookName.invariant.null']") as JArray; + Assert.IsNotNull(error7); + Assert.AreEqual(1, error7.Count); + + Assert.IsNotNull(error0.SelectToken("$[0].addresses")); + Assert.AreEqual(id1.ToString(), error0.SelectToken("$[0].addresses[0].$id").Value()); + Assert.AreEqual("addressInfo", error0.SelectToken("$[0].addresses[0].$elementTypeAlias").Value()); + Assert.IsNotNull(error0.SelectToken("$[0].addresses[0].ModelState")); + var error3 = error0.SelectToken("$[0].addresses[0].ModelState['_Properties.city.invariant.null.country']") as JArray; + Assert.IsNotNull(error3); + Assert.AreEqual(1, error3.Count); + var error4 = error0.SelectToken("$[0].addresses[0].ModelState['_Properties.city.invariant.null.capital']") as JArray; + Assert.IsNotNull(error4); + Assert.AreEqual(1, error4.Count); + var error5 = error0.SelectToken("$[0].addresses[0].ModelState['_Properties.city.invariant.null']") as JArray; + Assert.IsNotNull(error5); + Assert.AreEqual(2, error5.Count); + } + + [Test] + public void Validating_ContentItemSave() + { + var validator = new ContentSaveModelValidator( + Factory.GetInstance(), + Mock.Of(), + Factory.GetInstance()); + + var content = MockedContent.CreateTextpageContent(_contentType, "test", -1); + + var id1 = new Guid("c8df5136-d606-41f0-9134-dea6ae0c2fd9"); + var id2 = new Guid("f916104a-4082-48b2-a515-5c4bf2230f38"); + var id3 = new Guid("77E15DE9-1C79-47B2-BC60-4913BC4D4C6A"); + + // TODO: Ok now test with a 4th level complex nested editor + + var complexValue = @"[{ + ""key"": """ + id1.ToString() + @""", + ""name"": ""Hello world"", + ""ncContentTypeAlias"": """ + ContentTypeAlias + @""", + ""title"": ""Hello world"", + ""bodyText"": ""The world is round"" + }, { + ""key"": """ + id2.ToString() + @""", + ""name"": ""Super nested"", + ""ncContentTypeAlias"": """ + ContentTypeAlias + @""", + ""title"": ""Hi there!"", + ""bodyText"": ""Well hello there"", + ""complex"" : [{ + ""key"": """ + id3.ToString() + @""", + ""name"": ""I am a sub nested content"", + ""ncContentTypeAlias"": """ + ContentTypeAlias + @""", + ""title"": ""Hello up there :)"", + ""bodyText"": ""Hello way up there on a different level"" + }] + } + ]"; + content.SetValue("complex", complexValue); + + // map the persisted properties to a model representing properties to save + //var saveProperties = content.Properties.Select(x => Mapper.Map(x)).ToList(); + var saveProperties = content.Properties.Select(x => + { + return new ContentPropertyBasic + { + Alias = x.Alias, + Id = x.Id, + Value = x.GetValue() + }; + }).ToList(); + + var saveVariants = new List + { + new ContentVariantSave + { + Culture = string.Empty, + Segment = string.Empty, + Name = content.Name, + Save = true, + Properties = saveProperties + } + }; + + var save = new ContentItemSave + { + Id = content.Id, + Action = ContentSaveAction.Save, + ContentTypeAlias = _contentType.Alias, + ParentId = -1, + PersistedContent = content, + TemplateAlias = null, + Variants = saveVariants + }; + + // This will map the ContentItemSave.Variants.PropertyCollectionDto and then map the values in the saved model + // back onto the persisted IContent model. + ContentItemBinder.BindModel(save, content); + + var modelState = new ModelStateDictionary(); + var isValid = validator.ValidatePropertiesData(save, saveVariants[0], saveVariants[0].PropertyCollectionDto, modelState); + + // list results for debugging + foreach (var state in modelState) + { + Console.WriteLine(state.Key); + foreach (var error in state.Value.Errors) + { + Console.WriteLine("\t" + error.ErrorMessage); + } + } + + // assert + + Assert.IsFalse(isValid); + Assert.AreEqual(11, modelState.Keys.Count); + const string complexPropertyKey = "_Properties.complex.invariant.null"; + Assert.IsTrue(modelState.Keys.Contains(complexPropertyKey)); + foreach (var state in modelState.Where(x => x.Key != complexPropertyKey)) + { + foreach (var error in state.Value.Errors) + { + Assert.IsFalse(error.ErrorMessage.DetectIsJson()); // non complex is just an error message + } + } + var complexEditorErrors = modelState.Single(x => x.Key == complexPropertyKey).Value.Errors; + Assert.AreEqual(1, complexEditorErrors.Count); + var nestedError = complexEditorErrors[0]; + var jsonError = JsonConvert.DeserializeObject(nestedError.ErrorMessage); + + var modelStateKeys = new[] { "_Properties.title.invariant.null.innerFieldId", "_Properties.title.invariant.null.value", "_Properties.bodyText.invariant.null.innerFieldId", "_Properties.bodyText.invariant.null.value" }; + AssertNestedValidation(jsonError, 0, id1, modelStateKeys); + AssertNestedValidation(jsonError, 1, id2, modelStateKeys.Concat(new[] { "_Properties.complex.invariant.null.innerFieldId", "_Properties.complex.invariant.null.value" }).ToArray()); + var nestedJsonError = jsonError.SelectToken("$[1].complex") as JArray; + Assert.IsNotNull(nestedJsonError); + AssertNestedValidation(nestedJsonError, 0, id3, modelStateKeys); + } + + private void AssertNestedValidation(JArray jsonError, int index, Guid id, string[] modelStateKeys) + { + Assert.IsNotNull(jsonError.SelectToken("$[" + index + "]")); + Assert.AreEqual(id.ToString(), jsonError.SelectToken("$[" + index + "].$id").Value()); + Assert.AreEqual("textPage", jsonError.SelectToken("$[" + index + "].$elementTypeAlias").Value()); + Assert.IsNotNull(jsonError.SelectToken("$[" + index + "].ModelState")); + foreach (var key in modelStateKeys) + { + var error = jsonError.SelectToken("$[" + index + "].ModelState['" + key + "']") as JArray; + Assert.IsNotNull(error); + Assert.AreEqual(1, error.Count); + } + } + + [HideFromTypeFinder] + [DataEditor("complexTest", "test", "test")] + public class ComplexTestEditor : NestedContentPropertyEditor + { + public ComplexTestEditor(ILogger logger, Lazy propertyEditors, IDataTypeService dataTypeService, IContentTypeService contentTypeService) : base(logger, propertyEditors, dataTypeService, contentTypeService) + { + } + + protected override IDataValueEditor CreateValueEditor() + { + var editor = base.CreateValueEditor(); + editor.Validators.Add(new NeverValidateValidator()); + return editor; + } + } + + [HideFromTypeFinder] + [DataEditor("test", "test", "test")] // This alias aligns with the prop editor alias for all properties created from MockedContentTypes.CreateTextPageContentType + public class TestEditor : DataEditor + { + public TestEditor(ILogger logger) + : base(logger) + { + } + + protected override IDataValueEditor CreateValueEditor() => new TestValueEditor(Attribute); + + private class TestValueEditor : DataValueEditor + { + public TestValueEditor(DataEditorAttribute attribute) + : base(attribute) + { + Validators.Add(new NeverValidateValidator()); + } + + } + } + + public class NeverValidateValidator : IValueValidator + { + public IEnumerable Validate(object value, string valueType, object dataTypeConfiguration) + { + yield return new ValidationResult("WRONG!", new[] { "innerFieldId" }); + } + } + + } +} diff --git a/src/Umbraco.Tests/Web/ModelStateExtensionsTests.cs b/src/Umbraco.Tests/Web/Validation/ModelStateExtensionsTests.cs similarity index 99% rename from src/Umbraco.Tests/Web/ModelStateExtensionsTests.cs rename to src/Umbraco.Tests/Web/Validation/ModelStateExtensionsTests.cs index 7b25e60b5a..0355705378 100644 --- a/src/Umbraco.Tests/Web/ModelStateExtensionsTests.cs +++ b/src/Umbraco.Tests/Web/Validation/ModelStateExtensionsTests.cs @@ -6,7 +6,7 @@ using NUnit.Framework; using Umbraco.Core.Services; using Umbraco.Web; -namespace Umbraco.Tests.Web +namespace Umbraco.Tests.Web.Validation { [TestFixture] public class ModelStateExtensionsTests diff --git a/src/Umbraco.Web.UI.Client/lib/umbraco/Extensions.js b/src/Umbraco.Web.UI.Client/lib/umbraco/Extensions.js index 823d3d526d..54fda13a0d 100644 --- a/src/Umbraco.Web.UI.Client/lib/umbraco/Extensions.js +++ b/src/Umbraco.Web.UI.Client/lib/umbraco/Extensions.js @@ -69,6 +69,18 @@ }; } + if (!String.prototype.trimStartSpecial) { + /** trimSpecial extension method for string */ + // Removes all non printable chars from beginning of a string + String.prototype.trimStartSpecial = function () { + var index = 0; + while (this.charCodeAt(index) <= 46) { + index++; + } + return this.substr(index); + }; + } + if (!String.prototype.startsWith) { /** startsWith extension method for string */ String.prototype.startsWith = function (str) { diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/components/application/umbtour.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/components/application/umbtour.directive.js index 514e26f38f..b34a9eb8fd 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/components/application/umbtour.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/components/application/umbtour.directive.js @@ -11,7 +11,7 @@ You can easily add you own tours to the Help-drawer or show and start tours from anywhere in the Umbraco backoffice. To see a real world example of a custom tour implementation, install The Starter Kit in Umbraco 7.8

Extending the help drawer with custom tours

-The easiet way to add new tours to Umbraco is through the Help-drawer. All it requires is a my-tour.json file. +The easiet way to add new tours to Umbraco is through the Help-drawer. All it requires is a my-tour.json file. Place the file in App_Plugins/{MyPackage}/backoffice/tours/{my-tour}.json and it will automatically be picked up by Umbraco and shown in the Help-drawer. @@ -277,7 +277,6 @@ In the following example you see how to run some custom logic before a step goes } startStep(); - // tour completed - final step } else { // tour completed - final step scope.loadingStep = true; diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/components/content/edit.controller.js b/src/Umbraco.Web.UI.Client/src/common/directives/components/content/edit.controller.js index c07bb9bc83..68b19abf08 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/components/content/edit.controller.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/components/content/edit.controller.js @@ -4,7 +4,8 @@ function ContentEditController($rootScope, $scope, $routeParams, $q, $window, appState, contentResource, entityResource, navigationService, notificationsService, serverValidationManager, contentEditingHelper, localizationService, formHelper, umbRequestHelper, - editorState, $http, eventsService, overlayService, $location, localStorageService, treeService) { + editorState, $http, eventsService, overlayService, $location, localStorageService, treeService, + $exceptionHandler) { var evts = []; var infiniteMode = $scope.infiniteModel && $scope.infiniteModel.infiniteMode; @@ -515,6 +516,12 @@ } } + function handleHttpException(err) { + if (!err.status) { + $exceptionHandler(err); + } + } + /** Just shows a simple notification that there are client side validation issues to be fixed */ function showValidationNotification() { //TODO: We need to make the validation UI much better, there's a lot of inconsistencies in v8 including colors, issues with the property groups and validation errors between variants @@ -575,6 +582,7 @@ overlayService.close(); }, function (err) { $scope.page.buttonGroupState = 'error'; + handleHttpException(err); }); @@ -620,8 +628,7 @@ model.submitButtonState = "error"; //re-map the dialog model since we've re-bound the properties dialog.variants = $scope.content.variants; - //don't reject, we've handled the error - return $q.when(err); + handleHttpException(err); }); }, close: function () { @@ -642,8 +649,9 @@ action: "sendToPublish" }).then(function () { $scope.page.buttonGroupState = "success"; - }, function () { + }, function (err) { $scope.page.buttonGroupState = "error"; + handleHttpException(err); });; } }; @@ -679,8 +687,7 @@ model.submitButtonState = "error"; //re-map the dialog model since we've re-bound the properties dialog.variants = $scope.content.variants; - //don't reject, we've handled the error - return $q.when(err); + handleHttpException(err); }); }, close: function () { @@ -703,8 +710,9 @@ action: "publish" }).then(function () { $scope.page.buttonGroupState = "success"; - }, function () { + }, function (err) { $scope.page.buttonGroupState = "error"; + handleHttpException(err); }); } }; @@ -742,8 +750,7 @@ model.submitButtonState = "error"; //re-map the dialog model since we've re-bound the properties dialog.variants = $scope.content.variants; - //don't reject, we've handled the error - return $q.when(err); + handleHttpException(err); }); }, close: function (oldModel) { @@ -766,8 +773,9 @@ action: "save" }).then(function () { $scope.page.saveButtonState = "success"; - }, function () { + }, function (err) { $scope.page.saveButtonState = "error"; + handleHttpException(err); }); } @@ -820,8 +828,7 @@ model.submitButtonState = "error"; //re-map the dialog model since we've re-bound the properties dialog.variants = Utilities.copy($scope.content.variants); - //don't reject, we've handled the error - return $q.when(err); + handleHttpException(err); }); }, @@ -880,8 +887,7 @@ model.submitButtonState = "error"; //re-map the dialog model since we've re-bound the properties dialog.variants = $scope.content.variants; - //don't reject, we've handled the error - return $q.when(err); + handleHttpException(err); }); }, diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/components/editor/umbeditorcontentheader.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/components/editor/umbeditorcontentheader.directive.js index 9b8f92d7c3..fdec4c98d0 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/components/editor/umbeditorcontentheader.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/components/editor/umbeditorcontentheader.directive.js @@ -2,10 +2,10 @@ 'use strict'; function EditorContentHeader(serverValidationManager, localizationService, editorState) { - function link(scope) { + function link(scope) { var unsubscribe = []; - + if (!scope.serverValidationNameField) { scope.serverValidationNameField = "Name"; } @@ -46,55 +46,55 @@ scope.vm.variantsWithError = []; scope.vm.defaultVariant = null; scope.vm.errorsOnOtherVariants = false;// indicating wether to show that other variants, than the current, have errors. - + function updateVaraintErrors() { - scope.content.variants.forEach( function (variant) { + scope.content.variants.forEach(function (variant) { variant.hasError = scope.variantHasError(variant); - + }); checkErrorsOnOtherVariants(); } function checkErrorsOnOtherVariants() { var check = false; - scope.content.variants.forEach( function (variant) { + scope.content.variants.forEach(function (variant) { if (variant.active !== true && variant.hasError) { check = true; } }); scope.vm.errorsOnOtherVariants = check; } - + function onVariantValidation(valid, errors, allErrors, culture, segment) { // only want to react to property errors: - if(errors.findIndex(error => {return error.propertyAlias !== null;}) === -1) { + if (errors.findIndex(error => { return error.propertyAlias !== null; }) === -1) { // we dont have any errors for properties, meaning we will back out. return; } // If error coming back is invariant, we will assign the error to the default variant by picking the defaultVariant language. - if(culture === "invariant") { + if (culture === "invariant" && scope.vm.defaultVariant) { culture = scope.vm.defaultVariant.language.culture; } var index = scope.vm.variantsWithError.findIndex((item) => item.culture === culture && item.segment === segment) - if(valid === true) { + if (valid === true) { if (index !== -1) { scope.vm.variantsWithError.splice(index, 1); } } else { if (index === -1) { - scope.vm.variantsWithError.push({"culture": culture, "segment": segment}); + scope.vm.variantsWithError.push({ "culture": culture, "segment": segment }); } } scope.$evalAsync(updateVaraintErrors); } - + function onInit() { - + // find default + check if we have variants. - scope.content.variants.forEach( function (variant) { + scope.content.variants.forEach(function (variant) { if (variant.language !== null && variant.language.isDefault) { scope.vm.defaultVariant = variant; } @@ -113,41 +113,41 @@ scope.vm.variantMenu = []; if (scope.vm.hasCulture) { - scope.content.variants.forEach( (v) => { + scope.content.variants.forEach((v) => { if (v.language !== null && v.segment === null) { var variantMenuEntry = { key: String.CreateGuid(), open: v.language && v.language.culture === scope.editor.culture, variant: v, - subVariants: scope.content.variants.filter( (subVariant) => subVariant.language.culture === v.language.culture && subVariant.segment !== null) + subVariants: scope.content.variants.filter((subVariant) => subVariant.language.culture === v.language.culture && subVariant.segment !== null) }; scope.vm.variantMenu.push(variantMenuEntry); } }); } else { - scope.content.variants.forEach( (v) => { + scope.content.variants.forEach((v) => { scope.vm.variantMenu.push({ key: String.CreateGuid(), variant: v }); }); } - - scope.editor.variantApps.forEach( (app) => { + + scope.editor.variantApps.forEach((app) => { if (app.alias === "umbContent") { app.anchors = scope.editor.content.tabs; } }); - scope.content.variants.forEach( function (variant) { - + scope.content.variants.forEach(function (variant) { + // if we are looking for the variant with default language then we also want to check for invariant variant. - if (variant.language && variant.language.culture === scope.vm.defaultVariant.language.culture && variant.segment === null) { + if (variant.language && scope.vm.defaultVariant && variant.language.culture === scope.vm.defaultVariant.language.culture && variant.segment === null) { unsubscribe.push(serverValidationManager.subscribe(null, "invariant", null, onVariantValidation, null)); } unsubscribe.push(serverValidationManager.subscribe(null, variant.language !== null ? variant.language.culture : null, null, onVariantValidation, variant.segment)); }); - + } scope.goBack = function () { @@ -164,15 +164,15 @@ } }; - scope.selectNavigationItem = function(item) { - if(scope.onSelectNavigationItem) { - scope.onSelectNavigationItem({"item": item}); + scope.selectNavigationItem = function (item) { + if (scope.onSelectNavigationItem) { + scope.onSelectNavigationItem({ "item": item }); } } - scope.selectAnchorItem = function(item, anchor) { - if(scope.onSelectAnchorItem) { - scope.onSelectAnchorItem({"item": item, "anchor": anchor}); + scope.selectAnchorItem = function (item, anchor) { + if (scope.onSelectAnchorItem) { + scope.onSelectAnchorItem({ "item": item, "anchor": anchor }); } } @@ -188,20 +188,20 @@ scope.onOpenInSplitView({ "variant": variant }); } }; - + /** * Check whether a variant has a error, used to display errors in variant switcher. * @param {any} culture */ - scope.variantHasError = function(variant) { - if(scope.vm.variantsWithError.find((item) => (!variant.language || item.culture === variant.language.culture) && item.segment === variant.segment) !== undefined) { + scope.variantHasError = function (variant) { + if (scope.vm.variantsWithError.find((item) => (!variant.language || item.culture === variant.language.culture) && item.segment === variant.segment) !== undefined) { return true; } return false; } onInit(); - + scope.$on('$destroy', function () { for (var u in unsubscribe) { unsubscribe[u](); diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/components/editor/umbeditors.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/components/editor/umbeditors.directive.js index 672b3fa286..20fba6eb6e 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/components/editor/umbeditors.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/components/editor/umbeditors.directive.js @@ -11,13 +11,13 @@ var sectionId = '#leftcolumn'; var isLeftColumnAbove = false; scope.editors = []; - + function addEditor(editor) { editor.inFront = true; editor.moveRight = true; editor.level = 0; editor.styleIndex = 0; - + // push the new editor to the dom scope.editors.push(editor); @@ -32,20 +32,20 @@ $timeout(() => { editor.moveRight = false; }) - + editor.animating = true; setTimeout(revealEditorContent.bind(this, editor), 400); - + updateEditors(); } - + function removeEditor(editor) { editor.moveRight = true; - + editor.animating = true; setTimeout(removeEditorFromDOM.bind(this, editor), 400); - + updateEditors(-1); if(scope.editors.length === 1){ @@ -56,17 +56,17 @@ isLeftColumnAbove = false; } } - + function revealEditorContent(editor) { - + editor.animating = false; - + scope.$digest(); - + } - + function removeEditorFromDOM(editor) { - + // push the new editor to the dom var index = scope.editors.indexOf(editor); if (index !== -1) { @@ -74,42 +74,42 @@ } updateEditors(); - + scope.$digest(); - + } - + /** update layer positions. With ability to offset positions, needed for when an item is moving out, then we dont want it to influence positions */ function updateEditors(offset) { - + offset = offset || 0;// fallback value. - + var len = scope.editors.length; var calcLen = len + offset; var ceiling = Math.min(calcLen, allowedNumberOfVisibleEditors); - var origin = Math.max(calcLen-1, 0)-ceiling; + var origin = Math.max(calcLen - 1, 0) - ceiling; var i = 0; - while(i= ceiling; i++; } } - + evts.push(eventsService.on("appState.editors.open", function (name, args) { addEditor(args.editor); })); evts.push(eventsService.on("appState.editors.close", function (name, args) { // remove the closed editor - if(args && args.editor) { + if (args && args.editor) { removeEditor(args.editor); } // close all editors - if(args && !args.editor && args.editors.length === 0) { + if (args && !args.editor && args.editors.length === 0) { scope.editors = []; } })); @@ -134,6 +134,74 @@ } + // This directive allows for us to run a custom $compile for the view within the repeater which allows + // us to maintain a $scope hierarchy with the rendered view based on the $scope that initiated the + // infinite editing. The retain the $scope hiearchy a special $parentScope property is passed in to the model. + function EditorRepeaterDirective($http, $templateCache, $compile, angularHelper) { + function link(scope, el, attr, ctrl) { + + var editor = scope && scope.$parent ? scope.$parent.model : null; + if (!editor) { + return; + } + + var unsubscribe = []; + + //if a custom parent scope is defined then we need to manually compile the view + if (editor.$parentScope) { + var element = el.find(".scoped-view"); + $http.get(editor.view, { cache: $templateCache }) + .then(function (response) { + var templateScope = editor.$parentScope.$new(); + + unsubscribe.push(function () { + templateScope.$destroy(); + }); + + // NOTE: the 'model' name here directly affects the naming convention used in infinite editors, this why you access the model + // like $scope.model.If this is changed, everything breaks.This is because we are entirely reliant upon ng-include and inheriting $scopes. + // by default without a $parentScope used for infinite editing the 'model' propety will be set because the view creates the scopes in + // ng-repeat by ng-repeat="model in editors" + templateScope.model = editor; + + element.show(); + + // if a parentForm is supplied then we can link them but to do that we need to inject a top level form + if (editor.$parentForm) { + element.html("" + response.data + ""); + } + + $compile(element)(templateScope); + + // if a parentForm is supplied then we can link them + if (editor.$parentForm) { + editor.$parentForm.$addControl(templateScope.infiniteEditorForm); + } + }); + } + + scope.$on('$destroy', function () { + for (var i = 0; i < unsubscribe.length; i++) { + unsubscribe[i](); + } + }); + } + + var directive = { + restrict: 'E', + replace: true, + transclude: true, + scope: { + editors: "=" + }, + template: "
", + link: link + }; + + return directive; + } + angular.module('umbraco.directives').directive('umbEditors', EditorsDirective); + angular.module('umbraco.directives').directive('umbEditorRepeater', EditorRepeaterDirective); })(); diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/components/overlays/umboverlay.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/components/overlays/umboverlay.directive.js index ad396e7a9a..d1bdf6f42f 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/components/overlays/umboverlay.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/components/overlays/umboverlay.directive.js @@ -280,7 +280,7 @@ Opens an overlay to show a custom YSOD.
templateScope.model = scope.model; element.html(response.data); element.show(); - $compile(element.contents())(templateScope); + $compile(element)(templateScope); }); } } @@ -463,7 +463,7 @@ Opens an overlay to show a custom YSOD.
scope.submitForm = function (model) { if (scope.model.submit) { - if (formHelper.submitForm({ scope: scope, skipValidation: scope.model.skipFormValidation })) { + if (formHelper.submitForm({ scope: scope, skipValidation: scope.model.skipFormValidation, keepServerValidation: true })) { if (scope.model.confirmSubmit && scope.model.confirmSubmit.enable && !scope.directive.enableConfirmButton) { //wrap in a when since we don't know if this is a promise or not diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/components/property/umbproperty.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/components/property/umbproperty.directive.js index ad62bcd3db..2b2f36dd7d 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/components/property/umbproperty.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/components/property/umbproperty.directive.js @@ -3,46 +3,75 @@ * @name umbraco.directives.directive:umbProperty * @restrict E **/ -angular.module("umbraco.directives") - .directive('umbProperty', function (userService) { - return { - scope: { +(function () { + 'use strict'; + + angular + .module("umbraco.directives") + .component('umbProperty', { + templateUrl: 'views/components/property/umb-property.html', + controller: UmbPropertyController, + controllerAs: 'vm', + transclude: true, + require: { + parentUmbProperty: '?^^umbProperty', + parentForm: '?^^form' + }, + bindings: { property: "=", + elementKey: "@", + // optional, if set this will be used for the property alias validation path (hack required because NC changes the actual property.alias :/) + propertyAlias: "@", showInherit: "<", inheritsFrom: "<" - }, - transclude: true, - restrict: 'E', - replace: true, - templateUrl: 'views/components/property/umb-property.html', - link: function (scope) { - - scope.controlLabelTitle = null; - if(Umbraco.Sys.ServerVariables.isDebuggingEnabled) { - userService.getCurrentUser().then(function (u) { - if(u.allowedSections.indexOf("settings") !== -1 ? true : false) { - scope.controlLabelTitle = scope.property.alias; - } - }); - } - }, - //Define a controller for this directive to expose APIs to other directives - controller: function ($scope) { - - var self = this; - - //set the API properties/methods - - self.property = $scope.property; - self.setPropertyError = function (errorMsg) { - $scope.property.propertyErrorMessage = errorMsg; - }; - - $scope.propertyActions = []; - self.setPropertyActions = function(actions) { - $scope.propertyActions = actions; - }; - } + }); + + + + function UmbPropertyController($scope, userService, serverValidationManager, udiService, angularHelper) { + + const vm = this; + + vm.$onInit = onInit; + + vm.setPropertyError = function (errorMsg) { + vm.property.propertyErrorMessage = errorMsg; }; - }); + + vm.propertyActions = []; + vm.setPropertyActions = function (actions) { + vm.propertyActions = actions; + }; + + // returns the validation path for the property to be used as the validation key for server side validation logic + vm.getValidationPath = function () { + + var parentValidationPath = vm.parentUmbProperty ? vm.parentUmbProperty.getValidationPath() : null; + var propAlias = vm.propertyAlias ? vm.propertyAlias : vm.property.alias; + // the elementKey will be empty when this is not a nested property + var valPath = vm.elementKey ? vm.elementKey + "/" + propAlias : propAlias; + return serverValidationManager.createPropertyValidationKey(valPath, parentValidationPath); + } + + function onInit() { + vm.controlLabelTitle = null; + if (Umbraco.Sys.ServerVariables.isDebuggingEnabled) { + userService.getCurrentUser().then(function (u) { + if (u.allowedSections.indexOf("settings") !== -1 ? true : false) { + vm.controlLabelTitle = vm.property.alias; + } + }); + } + + if (!vm.parentUmbProperty) { + // not found, then fallback to searching the scope chain, this may be needed when DOM inheritance isn't maintained but scope + // inheritance is (i.e.infinite editing) + var found = angularHelper.traverseScopeChain($scope, s => s && s.vm && s.vm.constructor.name === "UmbPropertyController"); + vm.parentUmbProperty = found ? found.vm : null; + } + } + + } + +})(); diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/validation/valformmanager.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/validation/valformmanager.directive.js index 6638ed4e6d..47f1145600 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/validation/valformmanager.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/validation/valformmanager.directive.js @@ -12,48 +12,88 @@ * Another thing this directive does is to ensure that any .control-group that contains form elements that are invalid will * be marked with the 'error' css class. This ensures that labels included in that control group are styled correctly. **/ -function valFormManager(serverValidationManager, $rootScope, $timeout, $location, overlayService, eventsService, $routeParams, navigationService, editorService, localizationService) { +function valFormManager(serverValidationManager, $rootScope, $timeout, $location, overlayService, eventsService, $routeParams, navigationService, editorService, localizationService, angularHelper) { var SHOW_VALIDATION_CLASS_NAME = "show-validation"; var SAVING_EVENT_NAME = "formSubmitting"; var SAVED_EVENT_NAME = "formSubmitted"; + function notify(scope) { + scope.$broadcast("valStatusChanged", { form: scope.formCtrl }); + } + + function ValFormManagerController($scope) { + //This exposes an API for direct use with this directive + + // We need this as a way to reference this directive in the scope chain. Since this directive isn't a component and + // because it's an attribute instead of an element, we can't use controllerAs or anything like that. Plus since this is + // an attribute an isolated scope doesn't work so it's a bit weird. By doing this we are able to lookup the parent valFormManager + // in the scope hierarchy even if the DOM hierarchy doesn't match (i.e. in infinite editing) + $scope.valFormManager = this; + + var unsubscribe = []; + var self = this; + + //This is basically the same as a directive subscribing to an event but maybe a little + // nicer since the other directive can use this directive's API instead of a magical event + this.onValidationStatusChanged = function (cb) { + unsubscribe.push($scope.$on("valStatusChanged", function (evt, args) { + cb.apply(self, [evt, args]); + })); + }; + + this.isShowingValidation = () => $scope.showValidation === true; + + this.notify = function () { + notify($scope); + } + + this.isValid = function () { + return !$scope.formCtrl.$invalid; + } + + //Ensure to remove the event handlers when this instance is destroyted + $scope.$on('$destroy', function () { + for (var u in unsubscribe) { + unsubscribe[u](); + } + }); + } + + /** + * Find's the valFormManager in the scope/DOM hierarchy + * @param {any} scope + * @param {any} ctrls + * @param {any} index + */ + function getAncestorValFormManager(scope, ctrls, index) { + + // first check the normal directive inheritance which relies on DOM inheritance + var found = ctrls[index]; + if (found) { + return found; + } + + // not found, then fallback to searching the scope chain, this may be needed when DOM inheritance isn't maintained but scope + // inheritance is (i.e.infinite editing) + var found = angularHelper.traverseScopeChain(scope, s => s && s.valFormManager && s.valFormManager.constructor.name === "ValFormManagerController"); + return found ? found.valFormManager : null; + } + return { require: ["form", "^^?valFormManager", "^^?valSubView"], restrict: "A", - controller: function($scope) { - //This exposes an API for direct use with this directive - - var unsubscribe = []; - var self = this; - - //This is basically the same as a directive subscribing to an event but maybe a little - // nicer since the other directive can use this directive's API instead of a magical event - this.onValidationStatusChanged = function (cb) { - unsubscribe.push($scope.$on("valStatusChanged", function(evt, args) { - cb.apply(self, [evt, args]); - })); - }; - - this.showValidation = $scope.showValidation === true; - - //Ensure to remove the event handlers when this instance is destroyted - $scope.$on('$destroy', function () { - for (var u in unsubscribe) { - unsubscribe[u](); - } - }); - }, + controller: ValFormManagerController, link: function (scope, element, attr, ctrls) { function notifySubView() { - if (subView){ + if (subView) { subView.valStatusChanged({ form: formCtrl, showValidation: scope.showValidation }); } } - var formCtrl = ctrls[0]; - var parentFormMgr = ctrls.length > 0 ? ctrls[1] : null; + var formCtrl = scope.formCtrl = ctrls[0]; + var parentFormMgr = scope.parentFormMgr = getAncestorValFormManager(scope, ctrls, 1); var subView = ctrls.length > 1 ? ctrls[2] : null; var labels = {}; @@ -72,45 +112,22 @@ function valFormManager(serverValidationManager, $rootScope, $timeout, $location }); //watch the list of validation errors to notify the application of any validation changes - scope.$watch(function () { - //the validators are in the $error collection: https://docs.angularjs.org/api/ng/type/form.FormController#$error - //since each key is the validator name (i.e. 'required') we can't just watch the number of keys, we need to watch - //the sum of the items inside of each key + scope.$watch(() => angularHelper.countAllFormErrors(formCtrl), + function (e) { + + notify(scope); + + notifySubView(); + + //find all invalid elements' .control-group's and apply the error class + var inError = element.find(".control-group .ng-invalid").closest(".control-group"); + inError.addClass("error"); + + //find all control group's that have no error and ensure the class is removed + var noInError = element.find(".control-group .ng-valid").closest(".control-group").not(inError); + noInError.removeClass("error"); - //get the lengths of each array for each key in the $error collection - var validatorLengths = _.map(formCtrl.$error, function (val, key) { - // if there are child ng-forms, include the $error collections in those as well - var innerErrorCount = _.reduce( - _.map(val, v => - _.reduce( - _.map(v.$error, e => e.length), - (m, n) => m + n - ) - ), - (memo, num) => memo + num - ); - return val.length + innerErrorCount; }); - //sum up all numbers in the resulting array - var sum = _.reduce(validatorLengths, function (memo, num) { - return memo + num; - }, 0); - //this is the value we watch to notify of any validation changes on the form - return sum; - }, function (e) { - scope.$broadcast("valStatusChanged", { form: formCtrl }); - - notifySubView(); - - //find all invalid elements' .control-group's and apply the error class - var inError = element.find(".control-group .ng-invalid").closest(".control-group"); - inError.addClass("error"); - - //find all control group's that have no error and ensure the class is removed - var noInError = element.find(".control-group .ng-valid").closest(".control-group").not(inError); - noInError.removeClass("error"); - - }); //This tracks if the user is currently saving a new item, we use this to determine // if we should display the warning dialog that they are leaving the page - if a new item @@ -119,7 +136,7 @@ function valFormManager(serverValidationManager, $rootScope, $timeout, $location var isSavingNewItem = false; //we should show validation if there are any msgs in the server validation collection - if (serverValidationManager.items.length > 0 || (parentFormMgr && parentFormMgr.showValidation)) { + if (serverValidationManager.items.length > 0 || (parentFormMgr && parentFormMgr.isShowingValidation())) { element.addClass(SHOW_VALIDATION_CLASS_NAME); scope.showValidation = true; notifySubView(); @@ -128,7 +145,7 @@ function valFormManager(serverValidationManager, $rootScope, $timeout, $location var unsubscribe = []; //listen for the forms saving event - unsubscribe.push(scope.$on(SAVING_EVENT_NAME, function(ev, args) { + unsubscribe.push(scope.$on(SAVING_EVENT_NAME, function (ev, args) { element.addClass(SHOW_VALIDATION_CLASS_NAME); scope.showValidation = true; notifySubView(); @@ -137,7 +154,7 @@ function valFormManager(serverValidationManager, $rootScope, $timeout, $location })); //listen for the forms saved event - unsubscribe.push(scope.$on(SAVED_EVENT_NAME, function(ev, args) { + unsubscribe.push(scope.$on(SAVED_EVENT_NAME, function (ev, args) { //remove validation class element.removeClass(SHOW_VALIDATION_CLASS_NAME); scope.showValidation = false; @@ -151,7 +168,7 @@ function valFormManager(serverValidationManager, $rootScope, $timeout, $location //This handles the 'unsaved changes' dialog which is triggered when a route is attempting to be changed but // the form has pending changes - var locationEvent = $rootScope.$on('$locationChangeStart', function(event, nextLocation, currentLocation) { + var locationEvent = $rootScope.$on('$locationChangeStart', function (event, nextLocation, currentLocation) { var infiniteEditors = editorService.getEditors(); @@ -178,10 +195,10 @@ function valFormManager(serverValidationManager, $rootScope, $timeout, $location "disableEscKey": true, "submitButtonLabel": labels.stayButton, "closeButtonLabel": labels.discardChangesButton, - submit: function() { + submit: function () { overlayService.close(); }, - close: function() { + close: function () { // close all editors editorService.closeAll(); // allow redirection @@ -190,7 +207,7 @@ function valFormManager(serverValidationManager, $rootScope, $timeout, $location var parts = nextPath.split("?"); var query = {}; if (parts.length > 1) { - _.each(parts[1].split("&"), function(q) { + _.each(parts[1].split("&"), function (q) { var keyVal = q.split("="); query[keyVal[0]] = keyVal[1]; }); @@ -215,13 +232,13 @@ function valFormManager(serverValidationManager, $rootScope, $timeout, $location unsubscribe.push(locationEvent); //Ensure to remove the event handler when this instance is destroyted - scope.$on('$destroy', function() { + scope.$on('$destroy', function () { for (var u in unsubscribe) { unsubscribe[u](); } }); - $timeout(function(){ + $timeout(function () { formCtrl.$setPristine(); }, 1000); diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/validation/valpropertymsg.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/validation/valpropertymsg.directive.js index 30d3530efb..cbbd78bfe1 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/validation/valpropertymsg.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/validation/valpropertymsg.directive.js @@ -8,24 +8,26 @@ * We will listen for server side validation changes * and when an error is detected for this property we'll show the error message. * In order for this directive to work, the valFormManager directive must be placed on the containing form. +* We don't set the validity of this validator to false when client side validation fails, only when server side +* validation fails however we do respond to the client side validation changes to display error and adjust UI state. **/ -function valPropertyMsg(serverValidationManager, localizationService) { +function valPropertyMsg(serverValidationManager, localizationService, angularHelper) { return { - require: ['^^form', '^^valFormManager', '^^umbProperty', '?^^umbVariantContent'], + require: ['^^form', '^^valFormManager', '^^umbProperty', '?^^umbVariantContent', '?^^valPropertyMsg'], replace: true, restrict: "E", template: "
{{errorMsg}}
", scope: {}, link: function (scope, element, attrs, ctrl) { - + var unsubscribe = []; var watcher = null; - var hasError = false; + var hasError = false; // tracks if there is a child error or an explicit error //create properties on our custom scope so we can use it in our template - scope.errorMsg = ""; - + scope.errorMsg = ""; + //the property form controller api var formCtrl = ctrl[0]; //the valFormManager controller api @@ -33,21 +35,19 @@ function valPropertyMsg(serverValidationManager, localizationService) { //the property controller api var umbPropCtrl = ctrl[2]; //the variants controller api - var umbVariantCtrl = ctrl[3]; - + var umbVariantCtrl = ctrl[3]; + var currentProperty = umbPropCtrl.property; scope.currentProperty = currentProperty; var currentCulture = currentProperty.culture; - var currentSegment = currentProperty.segment; - + var currentSegment = currentProperty.segment; + // validation object won't exist when editor loads outside the content form (ie in settings section when modifying a content type) var isMandatory = currentProperty.validation ? currentProperty.validation.mandatory : undefined; var labels = {}; - localizationService.localize("errors_propertyHasErrors").then(function (data) { - labels.propertyHasErrors = data; - }); + var showValidation = false; if (umbVariantCtrl) { //if we are inside of an umbVariantContent directive @@ -61,17 +61,16 @@ function valPropertyMsg(serverValidationManager, localizationService) { return; } } - + // if we have reached this part, and there is no culture, then lets fallback to invariant. To get the validation feedback for invariant language. currentCulture = currentCulture || "invariant"; - // Gets the error message to display function getErrorMsg() { //this can be null if no property was assigned if (scope.currentProperty) { //first try to get the error msg from the server collection - var err = serverValidationManager.getPropertyError(scope.currentProperty.alias, null, "", null); + var err = serverValidationManager.getPropertyError(umbPropCtrl.getValidationPath(), null, "", null); //if there's an error message use it if (err && err.errorMsg) { return err.errorMsg; @@ -84,43 +83,109 @@ function valPropertyMsg(serverValidationManager, localizationService) { return labels.propertyHasErrors; } - // We need to subscribe to any changes to our model (based on user input) - // This is required because when we have a server error we actually invalidate - // the form which means it cannot be resubmitted. - // So once a field is changed that has a server error assigned to it - // we need to re-validate it for the server side validator so the user can resubmit - // the form. Of course normal client-side validators will continue to execute. + // check the current errors in the form (and recursive sub forms), if there is 1 or 2 errors + // we can check if those are valPropertyMsg or valServer and can clear our error in those cases. + function checkAndClearError() { + + var errCount = angularHelper.countAllFormErrors(formCtrl); + + if (errCount === 0) { + return true; + } + + if (errCount > 2) { + return false; + } + + var hasValServer = Utilities.isArray(formCtrl.$error.valServer); + if (errCount === 1 && hasValServer) { + return true; + } + + var hasOwnErr = hasExplicitError(); + if ((errCount === 1 && hasOwnErr) || (errCount === 2 && hasOwnErr && hasValServer)) { + + var propertyValidationPath = umbPropCtrl.getValidationPath(); + // check if we can clear it based on child server errors, if we are the only explicit one remaining we can clear ourselves + if (isLastServerError(propertyValidationPath)) { + serverValidationManager.removePropertyError(propertyValidationPath, currentCulture, "", currentSegment); + return true; + } + return false; + } + + return false; + } + + // returns true if there is an explicit valPropertyMsg validation error on the form + function hasExplicitError() { + return Utilities.isArray(formCtrl.$error.valPropertyMsg); + } + + // returns true if there is only a single server validation error for this property validation key in it's validation path + function isLastServerError(propertyValidationPath) { + var nestedErrs = serverValidationManager.getPropertyErrorsByValidationPath( + propertyValidationPath, + currentCulture, + currentSegment, + { matchType: "prefix" }); + if (nestedErrs.length === 0 || (nestedErrs.length === 1 && nestedErrs[0].propertyAlias === propertyValidationPath)) { + + return true; + } + return false; + } + + // a custom $validator function called on when each child ngModelController changes a value. + function resetServerValidityValidator(fieldController) { + const storedFieldController = fieldController; // pin a reference to this + return (modelValue, viewValue) => { + // if the ngModelController value has changed, then we can check and clear the error + if (storedFieldController.$dirty) { + if (checkAndClearError()) { + resetError(); + } + } + return true; // this validator is always 'valid' + }; + } + + // collect all ng-model controllers recursively within the umbProperty form + // until it reaches the next nested umbProperty form + function collectAllNgModelControllersRecursively(controls, ngModels) { + controls.forEach(ctrl => { + if (angularHelper.isForm(ctrl)) { + // if it's not another umbProperty form then recurse + if (ctrl.$name !== formCtrl.$name) { + collectAllNgModelControllersRecursively(ctrl.$getControls(), ngModels); + } + } + else if (ctrl.hasOwnProperty('$modelValue') && Utilities.isObject(ctrl.$validators)) { + ngModels.push(ctrl); + } + }); + } + + // We start the watch when there's server validation errors detected. + // We watch on the current form's properties and on first watch or if they are dynamically changed + // we find all ngModel controls recursively on this form (but stop recursing before we get to the next) + // umbProperty form). Then for each ngModelController we assign a new $validator. This $validator + // will execute whenever the value is changed which allows us to check and reset the server validator + // TODO: Is this even needed? function startWatch() { - //if there's not already a watch if (!watcher) { - watcher = scope.$watch("currentProperty.value", - function (newValue, oldValue) { - if (angular.equals(newValue, oldValue)) { - return; - } - var errCount = 0; - - for (var e in formCtrl.$error) { - if (Utilities.isArray(formCtrl.$error[e])) { - errCount++; + watcher = scope.$watchCollection( + () => formCtrl, + function (updatedFormController) { + var ngModels = []; + collectAllNgModelControllersRecursively(updatedFormController.$getControls(), ngModels); + ngModels.forEach(x => { + if (!x.$validators.serverValidityResetter) { + x.$validators.serverValidityResetter = resetServerValidityValidator(x); } - } - - //we are explicitly checking for valServer errors here, since we shouldn't auto clear - // based on other errors. We'll also check if there's no other validation errors apart from valPropertyMsg, if valPropertyMsg - // is the only one, then we'll clear. - - if (errCount === 0 - || (errCount === 1 && Utilities.isArray(formCtrl.$error.valPropertyMsg)) - || (formCtrl.$invalid && Utilities.isArray(formCtrl.$error.valServer))) { - scope.errorMsg = ""; - formCtrl.$setValidity('valPropertyMsg', true); - } else if (showValidation && scope.errorMsg === "") { - formCtrl.$setValidity('valPropertyMsg', false); - scope.errorMsg = getErrorMsg(); - } - }, true); + }); + }); } } @@ -132,22 +197,31 @@ function valPropertyMsg(serverValidationManager, localizationService) { } } + function resetError() { + stopWatch(); + hasError = false; + formCtrl.$setValidity('valPropertyMsg', true); + scope.errorMsg = ""; + + } + + // This deals with client side validation changes and is executed anytime validators change on the containing + // valFormManager. This allows us to know when to display or clear the property error data for non-server side errors. function checkValidationStatus() { if (formCtrl.$invalid) { //first we need to check if the valPropertyMsg validity is invalid if (formCtrl.$error.valPropertyMsg && formCtrl.$error.valPropertyMsg.length > 0) { //since we already have an error we'll just return since this means we've already set the - // hasError and errorMsg properties which occurs below in the serverValidationManager.subscribe + //hasError and errorMsg properties which occurs below in the serverValidationManager.subscribe return; } //if there are any errors in the current property form that are not valPropertyMsg else if (_.without(_.keys(formCtrl.$error), "valPropertyMsg").length > 0) { - + // errors exist, but if the property is NOT mandatory and has no value, the errors should be cleared if (isMandatory !== undefined && isMandatory === false && !currentProperty.value) { - hasError = false; - showValidation = false; - scope.errorMsg = ""; + + resetError(); // if there's no value, the controls can be reset, which clears the error state on formCtrl for (let control of formCtrl.$getControls()) { @@ -164,99 +238,107 @@ function valPropertyMsg(serverValidationManager, localizationService) { } } else { - hasError = false; - scope.errorMsg = ""; + resetError(); } } else { - hasError = false; - scope.errorMsg = ""; + resetError(); } } - //if there's any remaining errors in the server validation service then we should show them. - var showValidation = serverValidationManager.items.length > 0; - if (!showValidation) { - //We can either get the form submitted status by the parent directive valFormManager (if we add a property to it) - //or we can just check upwards in the DOM for the css class (easier for now). - //The initial hidden state can't always be hidden because when we switch variants in the content editor we cannot - //reset the status. - showValidation = element.closest(".show-validation").length > 0; - } + function onInit() { + localizationService.localize("errors_propertyHasErrors").then(function (data) { - //listen for form validation changes. - //The alternative is to add a watch to formCtrl.$invalid but that would lead to many more watches then - // subscribing to this single watch. - valFormManager.onValidationStatusChanged(function (evt, args) { - checkValidationStatus(); - }); + labels.propertyHasErrors = data; - //listen for the forms saving event - unsubscribe.push(scope.$on("formSubmitting", function (ev, args) { - showValidation = true; - if (hasError && scope.errorMsg === "") { - scope.errorMsg = getErrorMsg(); - startWatch(); - } - else if (!hasError) { - scope.errorMsg = ""; - stopWatch(); - } - })); - - //listen for the forms saved event - unsubscribe.push(scope.$on("formSubmitted", function (ev, args) { - showValidation = false; - scope.errorMsg = ""; - formCtrl.$setValidity('valPropertyMsg', true); - stopWatch(); - })); - - //listen for server validation changes - // NOTE: we pass in "" in order to listen for all validation changes to the content property, not for - // validation changes to fields in the property this is because some server side validators may not - // return the field name for which the error belongs too, just the property for which it belongs. - // It's important to note that we need to subscribe to server validation changes here because we always must - // indicate that a content property is invalid at the property level since developers may not actually implement - // the correct field validation in their property editors. - - if (scope.currentProperty) { //this can be null if no property was assigned - - function serverValidationManagerCallback(isValid, propertyErrors, allErrors) { - hasError = !isValid; - if (hasError) { - //set the error message to the server message - scope.errorMsg = propertyErrors[0].errorMsg; - //flag that the current validator is invalid - formCtrl.$setValidity('valPropertyMsg', false); - startWatch(); + //if there's any remaining errors in the server validation service then we should show them. + showValidation = serverValidationManager.items.length > 0; + if (!showValidation) { + //We can either get the form submitted status by the parent directive valFormManager (if we add a property to it) + //or we can just check upwards in the DOM for the css class (easier for now). + //The initial hidden state can't always be hidden because when we switch variants in the content editor we cannot + //reset the status. + showValidation = element.closest(".show-validation").length > 0; } - else { - scope.errorMsg = ""; - //flag that the current validator is valid - formCtrl.$setValidity('valPropertyMsg', true); - stopWatch(); + + //listen for form validation changes. + //The alternative is to add a watch to formCtrl.$invalid but that would lead to many more watches then + // subscribing to this single watch. + valFormManager.onValidationStatusChanged(function (evt, args) { + checkValidationStatus(); + }); + + //listen for the forms saving event + unsubscribe.push(scope.$on("formSubmitting", function (ev, args) { + showValidation = true; + if (hasError && scope.errorMsg === "") { + scope.errorMsg = getErrorMsg(); + startWatch(); + } + else if (!hasError) { + resetError(); + } + })); + + //listen for the forms saved event + unsubscribe.push(scope.$on("formSubmitted", function (ev, args) { + showValidation = false; + resetError(); + })); + + if (scope.currentProperty) { //this can be null if no property was assigned + + // listen for server validation changes for property validation path prefix. + // We pass in "" in order to listen for all validation changes to the content property, not for + // validation changes to fields in the property this is because some server side validators may not + // return the field name for which the error belongs too, just the property for which it belongs. + // It's important to note that we need to subscribe to server validation changes here because we always must + // indicate that a content property is invalid at the property level since developers may not actually implement + // the correct field validation in their property editors. + + function serverValidationManagerCallback(isValid, propertyErrors, allErrors) { + var hadError = hasError; + hasError = !isValid; + if (hasError) { + //set the error message to the server message + scope.errorMsg = propertyErrors.length > 1 ? labels.propertyHasErrors : propertyErrors[0].errorMsg || labels.propertyHasErrors; + //flag that the current validator is invalid + formCtrl.$setValidity('valPropertyMsg', false); + startWatch(); + + + if (propertyErrors.length === 1 && hadError && !formCtrl.$pristine) { + var propertyValidationPath = umbPropCtrl.getValidationPath(); + serverValidationManager.removePropertyError(propertyValidationPath, currentCulture, "", currentSegment); + resetError(); + } + } + else { + resetError(); + } + } + + unsubscribe.push(serverValidationManager.subscribe( + umbPropCtrl.getValidationPath(), + currentCulture, + "", + serverValidationManagerCallback, + currentSegment, + { matchType: "prefix" } // match property validation path prefix + )); } - } - - unsubscribe.push(serverValidationManager.subscribe(scope.currentProperty.alias, - currentCulture, - "", - serverValidationManagerCallback, - currentSegment - ) - ); + }); } //when the scope is disposed we need to unsubscribe scope.$on('$destroy', function () { stopWatch(); - for (var u in unsubscribe) { - unsubscribe[u](); - } + unsubscribe.forEach(u => u()); }); + + onInit(); } diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/validation/valserver.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/validation/valserver.directive.js index 3fa9220f7b..ea6087d4e9 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/validation/valserver.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/validation/valserver.directive.js @@ -13,14 +13,14 @@ function valServer(serverValidationManager) { link: function (scope, element, attr, ctrls) { var modelCtrl = ctrls[0]; - var umbPropCtrl = ctrls.length > 1 ? ctrls[1] : null; + var umbPropCtrl = ctrls[1]; if (!umbPropCtrl) { //we cannot proceed, this validator will be disabled return; } // optional reference to the varaint-content-controller, needed to avoid validation when the field is invariant on non-default languages. - var umbVariantCtrl = ctrls.length > 2 ? ctrls[2] : null; + var umbVariantCtrl = ctrls[2]; var currentProperty = umbPropCtrl.property; var currentCulture = currentProperty.culture; @@ -55,8 +55,11 @@ function valServer(serverValidationManager) { } } - //Need to watch the value model for it to change, previously we had subscribed to - //modelCtrl.$viewChangeListeners but this is not good enough if you have an editor that + // Get the property validation path if there is one, this is how wiring up any nested/virtual property validation works + var propertyValidationPath = umbPropCtrl ? umbPropCtrl.getValidationPath() : currentProperty.alias; + + // Need to watch the value model for it to change, previously we had subscribed to + // modelCtrl.$viewChangeListeners but this is not good enough if you have an editor that // doesn't specifically have a 2 way ng binding. This is required because when we // have a server error we actually invalidate the form which means it cannot be // resubmitted. So once a field is changed that has a server error assigned to it @@ -75,8 +78,10 @@ function valServer(serverValidationManager) { if (modelCtrl.$invalid) { modelCtrl.$setValidity('valServer', true); + //clear the server validation entry - serverValidationManager.removePropertyError(currentProperty.alias, currentCulture, fieldName, currentSegment); + serverValidationManager.removePropertyError(propertyValidationPath, currentCulture, fieldName, currentSegment); + stopWatch(); } }, true); @@ -105,7 +110,9 @@ function valServer(serverValidationManager) { stopWatch(); } } - unsubscribe.push(serverValidationManager.subscribe(currentProperty.alias, + + unsubscribe.push(serverValidationManager.subscribe( + propertyValidationPath, currentCulture, fieldName, serverValidationManagerCallback, @@ -114,9 +121,7 @@ function valServer(serverValidationManager) { scope.$on('$destroy', function () { stopWatch(); - for (var u in unsubscribe) { - unsubscribe[u](); - } + unsubscribe.forEach(u => u()); }); } }; diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/validation/valservermatch.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/validation/valservermatch.directive.js new file mode 100644 index 0000000000..b8e1b5cac8 --- /dev/null +++ b/src/Umbraco.Web.UI.Client/src/common/directives/validation/valservermatch.directive.js @@ -0,0 +1,89 @@ + +function valServerMatch(serverValidationManager) { + return { + require: ['form', '^^umbProperty', '?^^umbVariantContent'], + restrict: "A", + scope: { + valServerMatch: "=" + }, + link: function (scope, element, attr, ctrls) { + + var formCtrl = ctrls[0]; + var umbPropCtrl = ctrls[1]; + if (!umbPropCtrl) { + //we cannot proceed, this validator will be disabled + return; + } + + // optional reference to the varaint-content-controller, needed to avoid validation when the field is invariant on non-default languages. + var umbVariantCtrl = ctrls[2]; + + var currentProperty = umbPropCtrl.property; + var currentCulture = currentProperty.culture; + var currentSegment = currentProperty.segment; + + if (umbVariantCtrl) { + //if we are inside of an umbVariantContent directive + + var currentVariant = umbVariantCtrl.editor.content; + + // Lets check if we have variants and we are on the default language then ... + if (umbVariantCtrl.content.variants.length > 1 && (!currentVariant.language || !currentVariant.language.isDefault) && !currentCulture && !currentSegment && !currentProperty.unlockInvariantValue) { + //This property is locked cause its a invariant property shown on a non-default language. + //Therefor do not validate this field. + return; + } + } + + // if we have reached this part, and there is no culture, then lets fallback to invariant. To get the validation feedback for invariant language. + currentCulture = currentCulture || "invariant"; + + var unsubscribe = []; + + //subscribe to the server validation changes + function serverValidationManagerCallback(isValid, propertyErrors, allErrors) { + if (!isValid) { + formCtrl.$setValidity('valServerMatch', false); + } + else { + formCtrl.$setValidity('valServerMatch', true); + } + } + + if (Utilities.isObject(scope.valServerMatch)) { + var allowedKeys = ["contains", "prefix", "suffix"]; + Object.keys(scope.valServerMatch).forEach(k => { + if (allowedKeys.indexOf(k) === -1) { + throw "valServerMatch dictionary keys must be one of " + allowedKeys.join(); + } + + unsubscribe.push(serverValidationManager.subscribe( + scope.valServerMatch[k], + currentCulture, + "", + serverValidationManagerCallback, + currentSegment, + { matchType: k } // specify the match type + )); + + }); + } + else if (Utilities.isString(scope.valServerMatch)) { + unsubscribe.push(serverValidationManager.subscribe( + scope.valServerMatch, + currentCulture, + "", + serverValidationManagerCallback, + currentSegment)); + } + else { + throw "valServerMatch value must be a string or a dictionary"; + } + + scope.$on('$destroy', function () { + unsubscribe.forEach(u => u()); + }); + } + }; +} +angular.module('umbraco.directives.validation').directive("valServerMatch", valServerMatch); diff --git a/src/Umbraco.Web.UI.Client/src/common/services/angularhelper.service.js b/src/Umbraco.Web.UI.Client/src/common/services/angularhelper.service.js index f2ff711ac9..fd620bac18 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/angularhelper.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/angularhelper.service.js @@ -7,8 +7,86 @@ * Some angular helper/extension methods */ function angularHelper($q) { + + var requiredFormProps = ["$error", "$name", "$dirty", "$pristine", "$valid", "$submitted", "$pending"]; + + function collectAllFormErrorsRecursively(formCtrl, allErrors) { + // loop over the error dictionary (see https://docs.angularjs.org/api/ng/type/form.FormController#$error) + var keys = Object.keys(formCtrl.$error); + if (keys.length === 0) { + return; + } + keys.forEach(validationKey => { + var ctrls = formCtrl.$error[validationKey]; + ctrls.forEach(ctrl => { + if (!ctrl) { + // this happens when $setValidity('err', true) is called on a form controller without specifying the 3rd parameter for the control/form + // which just means that this is an error on the formCtrl itself + allErrors.push(formCtrl); // add the error + } + else if (isForm(ctrl)) { + // sometimes the control in error is the same form so we cannot recurse else we'll cause an infinite loop + // and in this case it means the error is assigned directly to the form, not a control + if (ctrl === formCtrl) { + allErrors.push(ctrl); // add the error + return; + } + // recurse with the sub form + collectAllFormErrorsRecursively(ctrl, allErrors); + } + else { + // it's a normal control + allErrors.push(ctrl); // add the error + } + }); + }); + } + + function isForm(obj) { + // a method to check that the collection of object prop names contains the property name expected + function allPropertiesExist(objectPropNames) { + //ensure that every required property name exists on the current object + return _.every(requiredFormProps, function (item) { + return _.contains(objectPropNames, item); + }); + } + + //get the keys of the property names for the current object + var props = _.keys(obj); + //if the length isn't correct, try the next prop + if (props.length < requiredFormProps.length) { + return false; + } + + //ensure that every required property name exists on the current scope property + return allPropertiesExist(props); + } + return { + countAllFormErrors: function (formCtrl) { + var allErrors = []; + collectAllFormErrorsRecursively(formCtrl, allErrors); + return allErrors.length; + }, + + /** + * Will traverse up the $scope chain to all ancestors until the predicate matches for the current scope or until it's at the root. + * @param {any} scope + * @param {any} predicate + */ + traverseScopeChain: function (scope, predicate) { + var s = scope.$parent; + while (s) { + var result = predicate(s); + if (result === true) { + return s; + } + s = s.$parent; + } + return null; + }, + /** * Method used to re-run the $parsers for a given ngModel * @param {} scope @@ -83,6 +161,9 @@ function angularHelper($q) { } }, + + isForm: isForm, + /** * @ngdoc function * @name getCurrentForm @@ -104,31 +185,10 @@ function angularHelper($q) { // is to inject the $element object and use: $element.inheritedData('$formController'); var form = null; - var requiredFormProps = ["$error", "$name", "$dirty", "$pristine", "$valid", "$submitted", "$pending"]; - - // a method to check that the collection of object prop names contains the property name expected - function propertyExists(objectPropNames) { - //ensure that every required property name exists on the current scope property - return _.every(requiredFormProps, function (item) { - - return _.contains(objectPropNames, item); - }); - } for (var p in scope) { - if (_.isObject(scope[p]) && p !== "this" && p.substr(0, 1) !== "$") { - //get the keys of the property names for the current property - var props = _.keys(scope[p]); - //if the length isn't correct, try the next prop - if (props.length < requiredFormProps.length) { - continue; - } - - //ensure that every required property name exists on the current scope property - var containProperty = propertyExists(props); - - if (containProperty) { + if (this.isForm(scope[p])) { form = scope[p]; break; } diff --git a/src/Umbraco.Web.UI.Client/src/common/services/blockeditormodelobject.service.js b/src/Umbraco.Web.UI.Client/src/common/services/blockeditormodelobject.service.js index 5cf0bbf944..4d520c02d0 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/blockeditormodelobject.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/blockeditormodelobject.service.js @@ -129,9 +129,9 @@ var prop = tab.properties[p]; // Watch value of property since this is the only value we want to keep synced. - // Do notice that it is not performing a deep watch, meaning that we are only watching primatives and changes directly to the object of property-value. - // But we like to sync non-primative values as well! Yes, and this does happen, just not through this code, but through the nature of JavaScript. - // Non-primative values act as references to the same data and are therefor synced. + // Do notice that it is not performing a deep watch, meaning that we are only watching primitive and changes directly to the object of property-value. + // But we like to sync non-primitive values as well! Yes, and this does happen, just not through this code, but through the nature of JavaScript. + // Non-primitive values act as references to the same data and are therefor synced. blockObject.__watchers.push(isolatedScope.$watch("blockObjects._" + blockObject.key + "." + field + ".variants[0].tabs[" + t + "].properties[" + p + "].value", watcherCreator(blockObject, prop))); // We also like to watch our data model to be able to capture changes coming from other places. @@ -257,13 +257,34 @@ this.isolatedScope.blockObjects = {}; this.__watchers.push(this.isolatedScope.$on("$destroy", this.destroy.bind(this))); - this.__watchers.push(propertyEditorScope.$on("postFormSubmitting", this.sync.bind(this))); }; BlockEditorModelObject.prototype = { + update: function (propertyModelValue, propertyEditorScope) { + // clear watchers + this.__watchers.forEach(w => { w(); }); + delete this.__watchers; + + // clear block objects + for (const key in this.isolatedScope.blockObjects) { + this.destroyBlockObject(this.isolatedScope.blockObjects[key]); + } + this.isolatedScope.blockObjects = {}; + + // update our values + this.value = propertyModelValue; + this.value.layout = this.value.layout || {}; + this.value.data = this.value.data || []; + + // re-create the watchers + this.__watchers = []; + this.__watchers.push(this.isolatedScope.$on("$destroy", this.destroy.bind(this))); + this.__watchers.push(propertyEditorScope.$on("postFormSubmitting", this.sync.bind(this))); + }, + /** * @ngdoc method * @name getBlockConfiguration @@ -280,8 +301,8 @@ * @ngdoc method * @name load * @methodOf umbraco.services.blockEditorModelObject - * @description Load the scaffolding models for the given configuration, these are needed to provide usefull models for each block. - * @param {Object} blockObject BlockObject to recive data values from. + * @description Load the scaffolding models for the given configuration, these are needed to provide useful models for each block. + * @param {Object} blockObject BlockObject to receive data values from. * @returns {Promise} A Promise object which resolves when all scaffold models are loaded. */ load: function() { @@ -296,7 +317,7 @@ } }); - // removing dublicates. + // removing duplicates. scaffoldKeys = scaffoldKeys.filter((value, index, self) => self.indexOf(value) === index); scaffoldKeys.forEach((contentTypeKey => { @@ -376,7 +397,7 @@ * @name getBlockObject * @methodOf umbraco.services.blockEditorModelObject * @description Retrieve a Block Object for the given layout entry. - * The Block Object offers the nesecary data to display and edit a block. + * The Block Object offers the necessary data to display and edit a block. * The Block Object setups live syncronization of content and settings models back to the data of your Property Editor model. * The returned object, named ´BlockObject´, contains several usefull models to make editing of this block happen. * The ´BlockObject´ contains the following properties: @@ -449,6 +470,8 @@ // make basics from scaffold blockObject.content = Utilities.copy(contentScaffold); blockObject.content.udi = udi; + // Change the content.key to the GUID part of the udi, else it's just random which we don't want, it should be consistent + blockObject.content.key = udiService.getKey(udi); mapToElementModel(blockObject.content, dataModel); @@ -482,7 +505,7 @@ } } - blockObject.retriveValuesFrom = function(content, settings) { + blockObject.retrieveValuesFrom = function(content, settings) { if (this.content !== null) { mapElementValues(content, this.content); } @@ -492,7 +515,7 @@ } - blockObject.sync = function() { + blockObject.sync = function () { if (this.content !== null) { mapToPropertyModel(this.content, this.data); } @@ -509,13 +532,14 @@ addWatchers(blockObject, this.isolatedScope); addWatchers(blockObject, this.isolatedScope, true); - blockObject.destroy = function() { + blockObject.destroy = function () { // remove property value watchers: this.__watchers.forEach(w => { w(); }); delete this.__watchers; // help carbage collector: delete this.config; + delete this.layout; delete this.data; delete this.settingsData; @@ -524,9 +548,12 @@ // remove model from isolatedScope. delete this.__scope.blockObjects["_" + this.key]; + // NOTE: It seems like we should call this.__scope.$destroy(); since that is the only way to remove a scope from it's parent, + // however that is not the case since __scope is actually this.isolatedScope which gets cleaned up when the outer scope is + // destroyed. If we do that here it breaks the scope chain and validation. delete this.__scope; - // removes this method, making it unposible to destroy again. + // removes this method, making it impossible to destroy again. delete this.destroy; // lets remove the key to make things blow up if this is still referenced: @@ -639,8 +666,6 @@ }, - - /** * @ngdoc method * @name sync @@ -654,6 +679,7 @@ }, // private + // TODO: Then this can just be a method in the outer scope _createDataEntry: function(elementTypeKey) { var content = { contentTypeKey: elementTypeKey, @@ -663,6 +689,7 @@ return content.udi; }, // private + // TODO: Then this can just be a method in the outer scope _getDataByUdi: function(udi) { return this.value.contentData.find(entry => entry.udi === udi) || null; }, diff --git a/src/Umbraco.Web.UI.Client/src/common/services/clipboard.service.js b/src/Umbraco.Web.UI.Client/src/common/services/clipboard.service.js index d7d6ec862c..cb583546a5 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/clipboard.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/clipboard.service.js @@ -129,6 +129,21 @@ function clipboardService(notificationsService, eventsService, localStorageServi }; + /** + * @ngdoc method + * @name umbraco.services.clipboardService#registrerPropertyClearingResolver + * @methodOf umbraco.services.clipboardService + * + * @param {string} function A method executed for every property and inner properties copied. + * + * @description + * Executed for all properties including inner properties when performing a copy action. + */ + service.registrerClearPropertyResolver = function(resolver) { + clearPropertyResolvers.push(resolver); + }; + + /** * @ngdoc method * @name umbraco.services.clipboardService#copy diff --git a/src/Umbraco.Web.UI.Client/src/common/services/contenteditinghelper.service.js b/src/Umbraco.Web.UI.Client/src/common/services/contenteditinghelper.service.js index bfcc0d536e..c73b6e1848 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/contenteditinghelper.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/contenteditinghelper.service.js @@ -32,6 +32,26 @@ function contentEditingHelper(fileManager, $q, $location, $routeParams, editorSt return true; } + function showNotificationsForModelsState(ms) { + for (const [key, value] of Object.entries(ms)) { + + var errorMsg = value[0]; + // if the error message is json it's a complex editor validation response that we need to parse + if ((Utilities.isString(errorMsg) && errorMsg.startsWith("[")) || Utilities.isArray(errorMsg)) { + // flatten the json structure, create validation paths for each property and add each as a property error + var idsToErrors = serverValidationManager.parseComplexEditorError(errorMsg, ""); + idsToErrors.forEach(x => { + if (x.modelState) { + showNotificationsForModelsState(x.modelState); + } + }); + } + else if (value[0]) { + notificationsService.error("Validation", value[0]); + } + } + } + return { //TODO: We need to move some of this to formHelper for saving, too many editors use this method for saving when this entire @@ -614,10 +634,9 @@ function contentEditingHelper(fileManager, $q, $location, $routeParams, editorSt formHelper.handleServerValidation(args.err.data.ModelState); //add model state errors to notifications + // TODO: Need to ignore complex messages if (args.showNotifications) { - for (var e in args.err.data.ModelState) { - notificationsService.error("Validation", args.err.data.ModelState[e][0]); - } + showNotificationsForModelsState(args.err.data.ModelState); } if (!this.redirectToCreatedContent(args.err.data.id, args.softRedirect) || args.softRedirect) { diff --git a/src/Umbraco.Web.UI.Client/src/common/services/formhelper.service.js b/src/Umbraco.Web.UI.Client/src/common/services/formhelper.service.js index 9c789d0bfb..962961729b 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/formhelper.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/formhelper.service.js @@ -44,7 +44,7 @@ function formHelper(angularHelper, serverValidationManager, notificationsService //the first thing any form must do is broadcast the formSubmitting event args.scope.$broadcast("formSubmitting", { scope: args.scope, action: args.action }); - // Some property editors need to performe an action after all property editors have reacted to the formSubmitting. + // Some property editors need to perform an action after all property editors have reacted to the formSubmitting. args.scope.$broadcast("postFormSubmitting", { scope: args.scope, action: args.action }); //then check if the form is valid @@ -54,8 +54,13 @@ function formHelper(angularHelper, serverValidationManager, notificationsService } } - //reset the server validations - serverValidationManager.reset(); + //reset the server validations if required (default is true), otherwise notify existing ones of changes + if (!args.keepServerValidation) { + serverValidationManager.reset(); + } + else { + serverValidationManager.notify(); + } return true; }, @@ -147,73 +152,7 @@ function formHelper(angularHelper, serverValidationManager, notificationsService * @param {object} err The error object returned from the http promise */ handleServerValidation: function (modelState) { - for (var e in modelState) { - - //This is where things get interesting.... - // We need to support validation for all editor types such as both the content and content type editors. - // The Content editor ModelState is quite specific with the way that Properties are validated especially considering - // that each property is a User Developer property editor. - // The way that Content Type Editor ModelState is created is simply based on the ASP.Net validation data-annotations - // system. - // So, to do this there's some special ModelState syntax we need to know about. - // For Content Properties, which are user defined, we know that they will exist with a prefixed - // ModelState of "_Properties.", so if we detect this, then we know it's for a content Property. - - //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 4 parts for content properties since all model errors for properties are prefixed with "_Properties" - //If it is not prefixed with "_Properties" that means the error is for a field of the object directly. - - // Example: "_Properties.headerImage.en-US.mySegment.myField" - // * it's for a property since it has a _Properties prefix - // * it's for the headerImage property type - // * it's for the en-US culture - // * it's for the mySegment segment - // * it's for the myField html field (optional) - - var parts = e.split("."); - - //Check if this is for content properties - specific to content/media/member editors because those are special - // user defined properties with custom controls. - if (parts.length > 1 && parts[0] === "_Properties") { - - var propertyAlias = parts[1]; - - var culture = null; - if (parts.length > 2) { - culture = parts[2]; - //special check in case the string is formatted this way - if (culture === "null") { - culture = null; - } - } - - var segment = null; - if (parts.length > 3) { - segment = parts[3]; - //special check in case the string is formatted this way - if (segment === "null") { - segment = null; - } - } - - var htmlFieldReference = ""; - if (parts.length > 4) { - htmlFieldReference = parts[4] || ""; - } - - // add a generic error for the property - serverValidationManager.addPropertyError(propertyAlias, culture, htmlFieldReference, modelState[e][0], segment); - - } else { - - //Everthing else is just a 'Field'... the field name could contain any level of 'parts' though, for example: - // Groups[0].Properties[2].Alias - serverValidationManager.addFieldError(e, modelState[e][0]); - } - - } + serverValidationManager.addErrorsForModelState(modelState); } }; } diff --git a/src/Umbraco.Web.UI.Client/src/common/services/servervalidationmgr.service.js b/src/Umbraco.Web.UI.Client/src/common/services/servervalidationmgr.service.js index 718e44d66e..b2fdf37aa4 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/servervalidationmgr.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/servervalidationmgr.service.js @@ -10,34 +10,73 @@ */ function serverValidationManager($timeout) { + // The array of callback objects, each object is: + // - propertyAlias (this is the property's 'path' if it's a nested error) + // - culture + // - fieldName + // - segment + // - callback (function) + // - id (unique identifier, auto-generated, used internally for unsubscribing the callback) + // - options (used for complex properties, can contain options.matchType which can be either "suffix" or "prefix" or "contains") var callbacks = []; - - /** calls the callback specified with the errors specified, used internally */ - function executeCallback(self, errorsForCallback, callback, culture, segment) { - callback.apply(self, [ - false, // pass in a value indicating it is invalid + // The array of error message objects, each object 'key' is: + // - propertyAlias (this is the property's 'path' if it's a nested error) + // - culture + // - fieldName + // - segment + // The object also contains: + // - errorMsg + var items = []; + + var defaultMatchOptions = { + matchType: null + } + + /** calls the callback specified with the errors specified, used internally */ + function executeCallback(errorsForCallback, callback, culture, segment, isValid) { + + callback.apply(instance, [ + isValid, // pass in a value indicating it is invalid errorsForCallback, // pass in the errors for this item - self.items, // pass in all errors in total + items, // pass in all errors in total culture, // pass the culture that we are listing for. segment // pass the segment that we are listing for. ] ); } - function getFieldErrors(self, fieldName) { + /** + * @ngdoc function + * @name notify + * @methodOf umbraco.services.serverValidationManager + * @function + * + * @description + * Notifies all subscriptions again. Called when there are changes to subscriptions or errors. + */ + function notify() { + + $timeout(function () { + for (var i = 0; i < items.length; i++) { + var item = items[i]; + } + notifyCallbacks(); + }); + } + + function getFieldErrors(fieldName) { if (!Utilities.isString(fieldName)) { throw "fieldName must be a string"; } //find errors for this field name - return _.filter(self.items, function (item) { + return _.filter(items, function (item) { return (item.propertyAlias === null && item.culture === "invariant" && item.fieldName === fieldName); }); } - - function getPropertyErrors(self, propertyAlias, culture, segment, fieldName) { + function getPropertyErrors(propertyAlias, culture, segment, fieldName, options) { if (!Utilities.isString(propertyAlias)) { throw "propertyAlias must be a string"; } @@ -52,13 +91,36 @@ function serverValidationManager($timeout) { segment = null; } + if (!options) { + options = defaultMatchOptions; + } + //find all errors for this property - return _.filter(self.items, function (item) { - return (item.propertyAlias === propertyAlias && item.culture === culture && item.segment === segment && (item.fieldName === fieldName || (fieldName === undefined || fieldName === ""))); + return _.filter(items, function (item) { + + if (!item.propertyAlias) { + return false; + } + + var matchProp = item.propertyAlias === propertyAlias + ? true + : options.matchType === "prefix" + ? item.propertyAlias.startsWith(propertyAlias + '/') + : options.matchType === "suffix" + ? item.propertyAlias.endsWith('/' + propertyAlias) + : options.matchType === "contains" + ? item.propertyAlias.includes('/' + propertyAlias + '/') + : false; + + return matchProp + && item.culture === culture + && item.segment === segment + // ignore field matching if match options are used + && (options.matchType || (item.fieldName === fieldName || (fieldName === undefined || fieldName === ""))); }); } - - function getVariantErrors(self, culture, segment) { + + function getVariantErrors(culture, segment) { if (!culture) { culture = "invariant"; @@ -68,39 +130,441 @@ function serverValidationManager($timeout) { } //find all errors for this property - return _.filter(self.items, function (item) { + return _.filter(items, function (item) { return (item.culture === culture && item.segment === segment); }); } - function notifyCallbacks(self) { - for (var cb in callbacks) { - if (callbacks[cb].propertyAlias === null && callbacks[cb].fieldName !== null) { - //its a field error callback - var fieldErrors = getFieldErrors(self, callbacks[cb].fieldName); - if (fieldErrors.length > 0) { - executeCallback(self, fieldErrors, callbacks[cb].callback, callbacks[cb].culture, callbacks[cb].segment); - } + function notifyCallback(cb) { + if (cb.propertyAlias === null && cb.fieldName !== null) { + //its a field error callback + const fieldErrors = getFieldErrors(cb.fieldName); + const valid = fieldErrors.length === 0; + executeCallback(fieldErrors, cb.callback, cb.culture, cb.segment, valid); + } + else if (cb.propertyAlias != null) { + //its a property error + const propErrors = getPropertyErrors(cb.propertyAlias, cb.culture, cb.segment, cb.fieldName, cb.options); + const valid = propErrors.length === 0; + executeCallback(propErrors, cb.callback, cb.culture, cb.segment, valid); + } + else { + //its a variant error + const variantErrors = getVariantErrors(cb.culture, cb.segment); + const valid = variantErrors.length === 0; + executeCallback(variantErrors, cb.callback, cb.culture, cb.segment, valid); + } + } + + /** Call all registered callbacks indicating if the data they are subscribed to is valid or invalid */ + function notifyCallbacks() { + + // nothing to call + if (items.length === 0) { + return; + } + + callbacks.forEach(cb => notifyCallback(cb)); + } + + /** + * Flattens the complex errror result json into an array of the block's id/parent id and it's corresponding validation ModelState + * @param {any} errorMsg + * @param {any} parentPropertyAlias The parent property alias for the json error + */ + function parseComplexEditorError(errorMsg, parentPropertyAlias) { + + var json = Utilities.isArray(errorMsg) ? errorMsg : JSON.parse(errorMsg); + + var result = []; + + function extractModelState(validation, parentPath) { + if (validation.$id && validation.ModelState) { + var ms = { + validationPath: `${parentPath}/${validation.$id}`, + modelState: validation.ModelState + }; + result.push(ms); + return ms; } - else if (callbacks[cb].propertyAlias != null) { - //its a property error - var propErrors = getPropertyErrors(self, callbacks[cb].propertyAlias, callbacks[cb].culture, callbacks[cb].segment, callbacks[cb].fieldName); - if (propErrors.length > 0) { - executeCallback(self, propErrors, callbacks[cb].callback, callbacks[cb].culture, callbacks[cb].segment); + return null; + } + + function iterateErrorBlocks(blocks, parentPath) { + for (var i = 0; i < blocks.length; i++) { + var validation = blocks[i]; + var ms = extractModelState(validation, parentPath); + if (!ms) { + continue; } - } - else { - //its a variant error - var variantErrors = getVariantErrors(self, callbacks[cb].culture, callbacks[cb].segment); - if (variantErrors.length > 0) { - executeCallback(self, variantErrors, callbacks[cb].callback, callbacks[cb].culture, callbacks[cb].segment); + var nested = _.omit(validation, "$id", "$elementTypeAlias", "ModelState"); + for (const [key, value] of Object.entries(nested)) { + if (Array.isArray(value)) { + iterateErrorBlocks(value, `${ms.validationPath}/${key}`); // recurse + } } } } + + iterateErrorBlocks(json, parentPropertyAlias); + + return result; } - - return { - + + /** + * @ngdoc function + * @name getPropertyCallbacks + * @methodOf umbraco.services.serverValidationManager + * @function + * + * @description + * Gets all callbacks that has been registered using the subscribe method for the propertyAlias + fieldName combo. + * 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. + */ + function getPropertyCallbacks(propertyAlias, culture, fieldName, segment) { + + //normalize culture to "invariant" + if (!culture) { + culture = "invariant"; + } + //normalize segment to null + if (!segment) { + segment = null; + } + + var found = _.filter(callbacks, function (cb) { + + if (!cb.options) { + cb.options = defaultMatchOptions; + } + + var matchProp = cb.propertyAlias === propertyAlias + ? true + : cb.options.matchType === "prefix" + ? propertyAlias.startsWith(cb.propertyAlias + '/') + : cb.options.matchType === "suffix" + ? propertyAlias.endsWith('/' + cb.propertyAlias) + : cb.options.matchType === "contains" + ? propertyAlias.includes('/' + cb.propertyAlias + '/') + : false; + + //returns any callback that have been registered directly against the field and for only the property + return matchProp + && cb.culture === culture + && cb.segment === segment + // ignore field matching if match options are used + && (cb.options.matchType || (cb.fieldName === fieldName || (cb.fieldName === undefined || cb.fieldName === ""))); + }); + return found; + } + + /** + * @ngdoc function + * @name getFieldCallbacks + * @methodOf umbraco.services.serverValidationManager + * @function + * + * @description + * Gets all callbacks that has been registered using the subscribe method for the field. + */ + function getFieldCallbacks(fieldName) { + var found = _.filter(callbacks, function (item) { + //returns any callback that have been registered directly against the field + return (item.propertyAlias === null && item.culture === "invariant" && item.segment === null && item.fieldName === fieldName); + }); + return found; + } + + /** + * @ngdoc function + * @name getVariantCallbacks + * @methodOf umbraco.services.serverValidationManager + * @function + * + * @description + * Gets all callbacks that has been registered using the subscribe method for the culture and segment. + */ + function getVariantCallbacks(culture, segment) { + var found = _.filter(callbacks, function (item) { + //returns any callback that have been registered directly against the given culture and given segment. + return (item.culture === culture && item.segment === segment && item.propertyAlias === null && item.fieldName === null); + }); + return found; + } + + /** + * @ngdoc function + * @name addFieldError + * @methodOf umbraco.services.serverValidationManager + * @function + * + * @description + * Adds an error message for a native content item field (not a user defined property, for Example, 'Name') + */ + function addFieldError(fieldName, errorMsg) { + if (!fieldName) { + return; + } + + //only add the item if it doesn't exist + if (!hasFieldError(fieldName)) { + items.push({ + propertyAlias: null, + culture: "invariant", + segment: null, + fieldName: fieldName, + errorMsg: errorMsg + }); + } + + //find all errors for this item + var errorsForCallback = getFieldErrors(fieldName); + //we should now call all of the call backs registered for this error + var cbs = getFieldCallbacks(fieldName); + //call each callback for this error + for (var cb in cbs) { + executeCallback(errorsForCallback, cbs[cb].callback, null, null, false); + } + } + + /** + * @ngdoc function + * @name addPropertyError + * @methodOf umbraco.services.serverValidationManager + * @function + * + * @description + * Adds an error message for the content property + */ + function addPropertyError(propertyAlias, culture, fieldName, errorMsg, segment) { + + if (!propertyAlias) { + return; + } + + //normalize culture to "invariant" + if (!culture) { + culture = "invariant"; + } + //normalize segment to null + if (!segment) { + segment = null; + } + //normalize errorMsg to empty + if (!errorMsg) { + errorMsg = ""; + } + + // remove all non printable chars and whitespace from the string + // (this can be a json string for complex errors and in some test cases contains odd whitespace) + if (Utilities.isString(errorMsg)) { + errorMsg = errorMsg.trimStartSpecial().trim(); + } + + // if the error message is json it's a complex editor validation response that we need to parse + if ((Utilities.isString(errorMsg) && errorMsg.startsWith("[")) || Utilities.isArray(errorMsg)) { + + // flatten the json structure, create validation paths for each property and add each as a property error + var idsToErrors = parseComplexEditorError(errorMsg, propertyAlias); + idsToErrors.forEach(x => addErrorsForModelState(x.modelState, x.validationPath)); + + // We need to clear the error message else it will show up as a giant json block against the property + errorMsg = ""; + } + + //only add the item if it doesn't exist + if (!hasPropertyError(propertyAlias, culture, fieldName, segment)) { + items.push({ + propertyAlias: propertyAlias, + culture: culture, + segment: segment, + fieldName: fieldName, + errorMsg: errorMsg + }); + } + } + + /** + * @ngdoc function + * @name hasPropertyError + * @methodOf umbraco.services.serverValidationManager + * @function + * + * @description + * Checks if the content property + culture + field name combo has an error + */ + function hasPropertyError(propertyAlias, culture, fieldName, segment) { + + //normalize culture to null + if (!culture) { + culture = "invariant"; + } + //normalize segment to null + if (!segment) { + segment = null; + } + + var err = _.find(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 === propertyAlias && item.culture === culture && item.segment === segment && (item.fieldName === fieldName || (fieldName === undefined || fieldName === ""))); + }); + return err ? true : false; + } + + /** + * @ngdoc function + * @name hasFieldError + * @methodOf umbraco.services.serverValidationManager + * @function + * + * @description + * Checks if a content field has an error + */ + function hasFieldError(fieldName) { + var err = _.find(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.culture === "invariant" && item.segment === null && item.fieldName === fieldName); + }); + return err ? true : false; + } + + /** + * @ngdoc function + * @name addErrorsForModelState + * @methodOf umbraco.services.serverValidationManager + * @param {any} modelState + * @param {any} parentValidationPath optional parameter specifying a nested element's UDI for which this property belongs (for complex editors) + * @description + * This wires up all of the server validation model state so that valServer and valServerField directives work + */ + function addErrorsForModelState(modelState, parentValidationPath) { + + if (!Utilities.isObject(modelState)) { + throw "modelState is not an object"; + } + + for (const [key, value] of Object.entries(modelState)) { + + //This is where things get interesting.... + // We need to support validation for all editor types such as both the content and content type editors. + // The Content editor ModelState is quite specific with the way that Properties are validated especially considering + // that each property is a User Developer property editor. + // The way that Content Type Editor ModelState is created is simply based on the ASP.Net validation data-annotations + // system. + // So, to do this there's some special ModelState syntax we need to know about. + // For Content Properties, which are user defined, we know that they will exist with a prefixed + // ModelState of "_Properties.", so if we detect this, then we know it's for a content Property. + + //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 4 parts for content properties since all model errors for properties are prefixed with "_Properties" + //If it is not prefixed with "_Properties" that means the error is for a field of the object directly. + + // Example: "_Properties.headerImage.en-US.mySegment.myField" + // * it's for a property since it has a _Properties prefix + // * it's for the headerImage property type + // * it's for the en-US culture + // * it's for the mySegment segment + // * it's for the myField html field (optional) + + var parts = key.split("."); + + //Check if this is for content properties - specific to content/media/member editors because those are special + // user defined properties with custom controls. + if (parts.length > 1 && parts[0] === "_Properties") { + + // create the validation key, might just be the prop alias but if it's nested will be validation path + // like "myBlockEditor/34E3A26C-103D-4A05-AB9D-7E14032309C3/addresses/7170A4DD-2441-4B1B-A8D3-437D75C4CBC9/city" + var propertyValidationKey = createPropertyValidationKey(parts[1], parentValidationPath); + + var culture = null; + if (parts.length > 2) { + culture = parts[2]; + //special check in case the string is formatted this way + if (culture === "null") { + culture = null; + } + } + + var segment = null; + if (parts.length > 3) { + segment = parts[3]; + //special check in case the string is formatted this way + if (segment === "null") { + segment = null; + } + } + + var htmlFieldReference = ""; + if (parts.length > 4) { + htmlFieldReference = parts[4] || ""; + } + + // add a generic error for the property + addPropertyError(propertyValidationKey, culture, htmlFieldReference, value && Array.isArray(value) && value.length > 0 ? value[0] : null, segment); + hasPropertyErrors = true; + } + else { + + //Everthing else is just a 'Field'... the field name could contain any level of 'parts' though, for example: + // Groups[0].Properties[2].Alias + addFieldError(key, value[0]); + } + } + + if (hasPropertyError) { + // ensure all callbacks are called after property errors are added + notifyCallbacks(); + } + } + + function createPropertyValidationKey(propertyAlias, parentValidationPath) { + return parentValidationPath ? (parentValidationPath + "/" + propertyAlias) : propertyAlias; + } + + /** + * @ngdoc function + * @name reset + * @methodOf umbraco.services.serverValidationManager + * @function + * + * @description + * Clears all errors and notifies all callbacks that all server errros are now valid - used when submitting a form + */ + function reset() { + clear(); + for (var cb in callbacks) { + callbacks[cb].callback.apply(instance, [ + true, //pass in a value indicating it is VALID + [], //pass in empty collection + [], + null, + null] + ); + } + } + + /** + * @ngdoc function + * @name clear + * @methodOf umbraco.services.serverValidationManager + * @function + * + * @description + * Clears all errors + */ + function clear() { + items = []; + } + + var instance = { + + addErrorsForModelState: addErrorsForModelState, + parseComplexEditorError: parseComplexEditorError, + createPropertyValidationKey: createPropertyValidationKey, + /** * @ngdoc function * @name notifyAndClearAllSubscriptions @@ -108,42 +572,27 @@ function serverValidationManager($timeout) { * @function * * @description - * This method needs to be called once all field and property errors are wired up. + * This method can be called once all field and property errors are wired up. * * In some scenarios where the error collection needs to be persisted over a route change * (i.e. when a content item (or any item) is created and the route redirects to the editor) * the controller should call this method once the data is bound to the scope * so that any persisted validation errors are re-bound to their controls. Once they are re-binded this then clears the validation * colleciton so that if another route change occurs, the previously persisted validation errors are not re-bound to the new item. + * + * In the case of content with complex editors, variants and different views, those editors don't call this method and instead + * manage the server validation manually by calling notify when necessary and clear/reset when necessary. */ notifyAndClearAllSubscriptions: function() { - var self = this; - $timeout(function () { - notifyCallbacks(self); + notifyCallbacks(); //now that they are all executed, we're gonna clear all of the errors we have - self.clear(); + clear(); }); }, - /** - * @ngdoc function - * @name notify - * @methodOf umbraco.services.serverValidationManager - * @function - * - * @description - * This method isn't used very often but can be used if all subscriptions need to be notified again. This can be - * handy if a view needs to be reloaded/rebuild like when switching variants in the content editor. - */ - notify: function() { - var self = this; - - $timeout(function () { - notifyCallbacks(self); - }); - }, + notify: notify, /** * @ngdoc function @@ -158,7 +607,7 @@ function serverValidationManager($timeout) { * field alias to listen for. * If propertyAlias is null, then this subscription is for a field property (not a user defined property). */ - subscribe: function (propertyAlias, culture, fieldName, callback, segment) { + subscribe: function (propertyAlias, culture, fieldName, callback, segment, options) { if (!callback) { return; } @@ -173,30 +622,34 @@ function serverValidationManager($timeout) { if (!segment) { segment = null; } - + + let cb = null; + if (propertyAlias === null) { - callbacks.push({ + cb = { propertyAlias: null, culture: culture, segment: segment, fieldName: fieldName, callback: callback, id: id - }); + }; } else if (propertyAlias !== undefined) { - //normalize culture to null - - callbacks.push({ + + cb = { propertyAlias: propertyAlias, - culture: culture, + culture: culture, segment: segment, fieldName: fieldName, callback: callback, - id: id - }); + id: id, + options: options + }; } + callbacks.push(cb); + function unsubscribeId() { //remove all callbacks for the content field callbacks = _.reject(callbacks, function (item) { @@ -204,6 +657,9 @@ function serverValidationManager($timeout) { }); } + // Notify the new callback + notifyCallback(cb); + //return a function to unsubscribe this subscription by uniqueId return unsubscribeId; }, @@ -244,52 +700,8 @@ function serverValidationManager($timeout) { } }, - - /** - * @ngdoc function - * @name getPropertyCallbacks - * @methodOf umbraco.services.serverValidationManager - * @function - * - * @description - * Gets all callbacks that has been registered using the subscribe method for the propertyAlias + fieldName combo. - * 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. - */ - getPropertyCallbacks: function (propertyAlias, culture, fieldName, segment) { - - //normalize culture to "invariant" - if (!culture) { - culture = "invariant"; - } - //normalize segment to null - if (!segment) { - segment = null; - } - - 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 === propertyAlias && item.culture === culture && item.segment === segment && (item.fieldName === fieldName || (item.fieldName === undefined || item.fieldName === ""))); - }); - return found; - }, - - /** - * @ngdoc function - * @name getFieldCallbacks - * @methodOf umbraco.services.serverValidationManager - * @function - * - * @description - * Gets all callbacks that has been registered using the subscribe method for the field. - */ - getFieldCallbacks: function (fieldName) { - var found = _.filter(callbacks, function (item) { - //returns any callback that have been registered directly against the field - return (item.propertyAlias === null && item.culture === "invariant" && item.segment === null && item.fieldName === fieldName); - }); - return found; - }, + getPropertyCallbacks: getPropertyCallbacks, + getFieldCallbacks: getFieldCallbacks, /** * @ngdoc function @@ -308,107 +720,12 @@ function serverValidationManager($timeout) { return found; }, - /** - * @ngdoc function - * @name getVariantCallbacks - * @methodOf umbraco.services.serverValidationManager - * @function - * - * @description - * Gets all callbacks that has been registered using the subscribe method for the culture and segment. - */ - getVariantCallbacks: function (culture, segment) { - var found = _.filter(callbacks, function (item) { - //returns any callback that have been registered directly against the given culture and given segment. - return (item.culture === culture && item.segment === segment && item.propertyAlias === null && item.fieldName === null); - }); - return found; - }, - - /** - * @ngdoc function - * @name addFieldError - * @methodOf umbraco.services.serverValidationManager - * @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, - culture: "invariant", - segment: null, - fieldName: fieldName, - errorMsg: errorMsg - }); - } - - //find all errors for this item - var errorsForCallback = getFieldErrors(this, 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) { - executeCallback(this, errorsForCallback, cbs[cb].callback, null, null); - } - }, + getVariantCallbacks: getVariantCallbacks, + addFieldError: addFieldError, - /** - * @ngdoc function - * @name addPropertyError - * @methodOf umbraco.services.serverValidationManager - * @function - * - * @description - * Adds an error message for the content property - */ addPropertyError: function (propertyAlias, culture, fieldName, errorMsg, segment) { - if (!propertyAlias) { - return; - } - - //normalize culture to "invariant" - if (!culture) { - culture = "invariant"; - } - //normalize segment to null - if (!segment) { - segment = null; - } - - //only add the item if it doesn't exist - if (!this.hasPropertyError(propertyAlias, culture, fieldName, segment)) { - this.items.push({ - propertyAlias: propertyAlias, - culture: culture, - segment: segment, - fieldName: fieldName, - errorMsg: errorMsg - }); - } - - //find all errors for this item - var errorsForCallback = getPropertyErrors(this, propertyAlias, culture, segment, fieldName); - //we should now call all of the call backs registered for this error - var cbs = this.getPropertyCallbacks(propertyAlias, culture, fieldName, segment); - //call each callback for this error - for (var cb in cbs) { - executeCallback(this, errorsForCallback, cbs[cb].callback, culture, segment); - } - - //execute variant specific callbacks here too when a propery error is added - var variantCbs = this.getVariantCallbacks(culture, segment); - //call each callback for this error - for (var cb in variantCbs) { - executeCallback(this, errorsForCallback, variantCbs[cb].callback, culture, segment); - } + addPropertyError(propertyAlias, culture, fieldName, errorMsg, segment); + notifyCallbacks(); // ensure all callbacks are called }, /** @@ -420,61 +737,19 @@ function serverValidationManager($timeout) { * @description * Removes an error message for the content property */ - removePropertyError: function (propertyAlias, culture, fieldName, segment) { + removePropertyError: function (propertyAlias, culture, fieldName, segment, options) { - if (!propertyAlias) { - return; - } + var errors = getPropertyErrors(propertyAlias, culture, segment, fieldName, options); + items = items.filter(v => errors.indexOf(v) === -1); - //normalize culture to null - if (!culture) { - culture = "invariant"; - } - //normalize segment to null - if (!segment) { - segment = null; - } - - //remove the item - this.items = _.reject(this.items, function (item) { - return (item.propertyAlias === propertyAlias && item.culture === culture && item.segment === segment && (item.fieldName === fieldName || (fieldName === undefined || fieldName === ""))); - }); - }, - - /** - * @ngdoc function - * @name reset - * @methodOf umbraco.services.serverValidationManager - * @function - * - * @description - * Clears all errors and notifies all callbacks that all server errros are now valid - used when submitting a form - */ - reset: function () { - this.clear(); - for (var cb in callbacks) { - callbacks[cb].callback.apply(this, [ - true, //pass in a value indicating it is VALID - [], //pass in empty collection - [], - null, - null] - ); + if (errors.length > 0) { + // removal was successful, re-notify all subscribers + notifyCallbacks(); } }, - /** - * @ngdoc function - * @name clear - * @methodOf umbraco.services.serverValidationManager - * @function - * - * @description - * Clears all errors - */ - clear: function() { - this.items = []; - }, + reset: reset, + clear: clear, /** * @ngdoc function @@ -486,23 +761,17 @@ function serverValidationManager($timeout) { * Gets the error message for the content property */ getPropertyError: function (propertyAlias, culture, fieldName, segment) { - - //normalize culture to "invariant" - if (!culture) { - culture = "invariant"; + var errors = getPropertyErrors(propertyAlias, culture, segment, fieldName); + if (errors.length > 0) { // should only ever contain one + return errors[0]; } - //normalize segment to null - if (!segment) { - segment = null; - } - - 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 === propertyAlias && item.culture === culture && item.segment === segment && (item.fieldName === fieldName || (fieldName === undefined || fieldName === ""))); - }); - return err; + return undefined; }, - + + getPropertyErrorsByValidationPath: function (propertyValidationPath, culture, segment, options) { + return getPropertyErrors(propertyValidationPath, culture, segment, "", options); + }, + /** * @ngdoc function * @name getFieldError @@ -513,56 +782,15 @@ function serverValidationManager($timeout) { * Gets the error message for a content field */ getFieldError: function (fieldName) { - var err = _.find(this.items, function (item) { + var err = _.find(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.culture === "invariant" && item.segment === null && item.fieldName === fieldName); }); return err; }, - /** - * @ngdoc function - * @name hasPropertyError - * @methodOf umbraco.services.serverValidationManager - * @function - * - * @description - * Checks if the content property + culture + field name combo has an error - */ - hasPropertyError: function (propertyAlias, culture, fieldName, segment) { - - //normalize culture to null - if (!culture) { - culture = "invariant"; - } - //normalize segment to null - if (!segment) { - segment = null; - } - - 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 === propertyAlias && item.culture === culture && item.segment === segment && (item.fieldName === fieldName || (fieldName === undefined || fieldName === ""))); - }); - return err ? true : false; - }, - - /** - * @ngdoc function - * @name hasFieldError - * @methodOf umbraco.services.serverValidationManager - * @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.culture === "invariant" && item.segment === null && item.fieldName === fieldName); - }); - return err ? true : false; - }, + hasPropertyError: hasPropertyError, + hasFieldError: hasFieldError, /** * @ngdoc function @@ -580,7 +808,7 @@ function serverValidationManager($timeout) { culture = "invariant"; } - var err = _.find(this.items, function (item) { + var err = _.find(items, function (item) { return (item.culture === culture && item.segment === null); }); return err ? true : false; @@ -606,14 +834,25 @@ function serverValidationManager($timeout) { segment = null; } - var err = _.find(this.items, function (item) { + var err = _.find(items, function (item) { return (item.culture === culture && item.segment === segment); }); return err ? true : false; - }, - /** The array of error messages */ - items: [] + } + }; + + // Used to return the 'items' array as a reference/getter + Object.defineProperty(instance, "items", { + get: function () { + return items; + }, + set: function (value) { + throw "Cannot set the items array"; + } + }); + + return instance; } angular.module('umbraco.services').factory('serverValidationManager', serverValidationManager); diff --git a/src/Umbraco.Web.UI.Client/src/common/services/udi.service.js b/src/Umbraco.Web.UI.Client/src/common/services/udi.service.js index 8c178533bd..7b08c68e4e 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/udi.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/udi.service.js @@ -16,13 +16,32 @@ * @function * * @description - * Generates a Udi string. + * Generates a Udi string with a new ID * * @param {string} entityType The entityType as a string. * @returns {string} The generated UDI */ create: function(entityType) { - return "umb://" + entityType + "/" + (String.CreateGuid().replace(/-/g, "")); + return this.build(entityType, String.CreateGuid()); + }, + + build: function (entityType, guid) { + return "umb://" + entityType + "/" + (guid.replace(/-/g, "")); + }, + + getKey: function (udi) { + if (!Utilities.isString(udi)) { + throw "udi is not a string"; + } + if (!udi.startsWith("umb://")) { + throw "udi does not start with umb://"; + } + var withoutScheme = udi.substr("umb://".length); + var withoutHost = withoutScheme.substr(withoutScheme.indexOf("/") + 1).trim(); + if (withoutHost.length !== 32) { + throw "udi is not 32 chars"; + } + return `${withoutHost.substr(0, 8)}-${withoutHost.substr(8, 4)}-${withoutHost.substr(12, 4)}-${withoutHost.substr(16, 4)}-${withoutHost.substr(20)}`; } } } diff --git a/src/Umbraco.Web.UI.Client/src/common/services/umbrequesthelper.service.js b/src/Umbraco.Web.UI.Client/src/common/services/umbrequesthelper.service.js index 4cbc5e567a..7b43b239ea 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/umbrequesthelper.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/umbrequesthelper.service.js @@ -165,11 +165,9 @@ function umbRequestHelper($http, $q, notificationsService, eventsService, formHe return; //sometimes oddly this happens, nothing we can do } - if (!response.status && response.message && response.stack) { - //this is a JS/angular error that we should deal with - return $q.reject({ - errorMsg: response.message - }); + if (!response.status) { + //this is a JS/angular error + return $q.reject(response); } //invoke the callback @@ -280,7 +278,11 @@ function umbRequestHelper($http, $q, notificationsService, eventsService, formHe //the data returned is the up-to-date data so the UI will refresh return $q.resolve(response.data); }, function (response) { - //failure callback + + if (!response.status) { + //this is a JS/angular error + return $q.reject(response); + } //when there's a 500 (unhandled) error show a YSOD overlay if debugging is enabled. if (response.status >= 500 && response.status < 600) { diff --git a/src/Umbraco.Web.UI.Client/src/less/components/umb-nested-content.less b/src/Umbraco.Web.UI.Client/src/less/components/umb-nested-content.less index 716693c778..0f2b1402dc 100644 --- a/src/Umbraco.Web.UI.Client/src/less/components/umb-nested-content.less +++ b/src/Umbraco.Web.UI.Client/src/less/components/umb-nested-content.less @@ -38,6 +38,15 @@ position: relative; text-align: left; background: @white; + border: 1px solid @gray-9; + border-radius: @baseBorderRadius; + transition: border-color 120ms; + margin-bottom: 4px; + margin-top: 4px; + + &.--error { + border-color: @formErrorBorder !important; + } } .umb-nested-content__item.ui-sortable-placeholder { @@ -54,7 +63,7 @@ } .umb-nested-content__header-bar { - border-bottom: 1px solid @gray-9; + cursor: pointer; background-color: @white; @@ -207,9 +216,9 @@ .umb-nested-content__content { border-top: 1px solid transparent; - border-bottom: 1px solid @gray-9; - border-left: 1px solid @gray-9; - border-right: 1px solid @gray-9; + border-bottom: 1px solid transparent; + border-left: 1px solid transparent; + border-right: 1px solid transparent; border-radius: 0 0 3px 3px; } diff --git a/src/Umbraco.Web.UI.Client/src/less/main.less b/src/Umbraco.Web.UI.Client/src/less/main.less index 57d867ccd5..ce20b8dc88 100644 --- a/src/Umbraco.Web.UI.Client/src/less/main.less +++ b/src/Umbraco.Web.UI.Client/src/less/main.less @@ -151,7 +151,7 @@ h6.-black { } } -.umb-property:last-of-type .umb-control-group { +umb-property:last-of-type .umb-control-group { &::after { margin-top: 0px; height: 0; diff --git a/src/Umbraco.Web.UI.Client/src/views/common/infiniteeditors/blockeditor/blockeditor.controller.js b/src/Umbraco.Web.UI.Client/src/views/common/infiniteeditors/blockeditor/blockeditor.controller.js index 923fe37279..daded4d9a9 100644 --- a/src/Umbraco.Web.UI.Client/src/views/common/infiniteeditors/blockeditor/blockeditor.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/common/infiniteeditors/blockeditor/blockeditor.controller.js @@ -4,18 +4,16 @@ angular.module("umbraco") var vm = this; vm.model = $scope.model; - + vm.model = $scope.model; + vm.tabs = []; localizationService.localizeMany([ - $scope.model.liveEditing ? "prompt_discardChanges" : "general_close", - $scope.model.liveEditing ? "buttons_confirmActionConfirm" : "buttons_submitChanges" + vm.model.liveEditing ? "prompt_discardChanges" : "general_close", + vm.model.liveEditing ? "buttons_confirmActionConfirm" : "buttons_submitChanges" ]).then(function (data) { vm.closeLabel = data[0]; vm.submitLabel = data[1]; }); - - vm.tabs = []; - if ($scope.model.content && $scope.model.content.variants) { var apps = $scope.model.content.apps; @@ -24,11 +22,11 @@ angular.module("umbraco") // replace view of content app. var contentApp = apps.find(entry => entry.alias === "umbContent"); - if(contentApp) { + if (contentApp) { contentApp.view = "views/common/infiniteeditors/blockeditor/blockeditor.content.html"; - if($scope.model.hideContent) { + if(vm.model.hideContent) { apps.splice(apps.indexOf(contentApp), 1); - } else if ($scope.model.openSettings !== true) { + } else if (vm.model.openSettings !== true) { contentApp.active = true; } } @@ -39,7 +37,7 @@ angular.module("umbraco") } - if ($scope.model.settings && $scope.model.settings.variants) { + if (vm.model.settings && vm.model.settings.variants) { localizationService.localize("blockEditor_tabBlockSettings").then( function (settingsName) { var settingsTab = { @@ -49,7 +47,7 @@ angular.module("umbraco") "view": "views/common/infiniteeditors/blockeditor/blockeditor.settings.html" }; vm.tabs.push(settingsTab); - if ($scope.model.openSettings) { + if (vm.model.openSettings) { settingsTab.active = true; } } @@ -57,17 +55,30 @@ angular.module("umbraco") } vm.submitAndClose = function () { - if ($scope.model && $scope.model.submit) { - if (formHelper.submitForm({ scope: $scope })) { - $scope.model.submit($scope.model); + if (vm.model && vm.model.submit) { + // always keep server validations since this will be a nested editor and server validations are global + if (formHelper.submitForm({ scope: $scope, formCtrl: vm.blockForm, keepServerValidation: true })) { + vm.model.submit(vm.model); } } } - vm.close = function() { - if ($scope.model && $scope.model.close) { + vm.close = function () { + if (vm.model && vm.model.close) { + // TODO: At this stage there could very well have been server errors that have been cleared + // but if we 'close' we are basically cancelling the value changes which means we'd want to cancel + // all of the server errors just cleared. It would be possible to do that but also quite annoying. + // The rudimentary way would be to: + // * Track all cleared server errors here by subscribing to the prefix validation of controls contained here + // * If this is closed, re-add all of those server validation errors + // A more robust way to do this would be to: + // * Add functionality to the serverValidationManager whereby we can remove validation errors and it will + // maintain a copy of the original errors + // * It would have a 'commit' method to commit the removed errors - which we would call in the formHelper.submitForm when it's successful + // * It would have a 'rollback' method to reset the removed errors - which we would call here + // TODO: check if content/settings has changed and ask user if they are sure. - $scope.model.close($scope.model); + vm.model.close(vm.model); } } diff --git a/src/Umbraco.Web.UI.Client/src/views/common/infiniteeditors/blockeditor/blockeditor.html b/src/Umbraco.Web.UI.Client/src/views/common/infiniteeditors/blockeditor/blockeditor.html index ce157c95f5..de18f13d2c 100644 --- a/src/Umbraco.Web.UI.Client/src/views/common/infiniteeditors/blockeditor/blockeditor.html +++ b/src/Umbraco.Web.UI.Client/src/views/common/infiniteeditors/blockeditor/blockeditor.html @@ -1,6 +1,7 @@
- + + -
+ +
-
+ +
+
-
+ +
diff --git a/src/Umbraco.Web.UI.Client/src/views/components/elementeditor/umb-element-editor-content.component.html b/src/Umbraco.Web.UI.Client/src/views/components/elementeditor/umb-element-editor-content.component.html index 5308173c72..fae639562f 100644 --- a/src/Umbraco.Web.UI.Client/src/views/components/elementeditor/umb-element-editor-content.component.html +++ b/src/Umbraco.Web.UI.Client/src/views/components/elementeditor/umb-element-editor-content.component.html @@ -1,8 +1,8 @@
+ data-element="group-{{group.alias}}" + ng-repeat="group in vm.model.variants[0].tabs track by group.label">
{{ group.label }}
@@ -13,6 +13,7 @@ data-element="property-{{property.alias}}" ng-repeat="property in group.properties track by property.alias" property="property" + element-key="{{vm.model.key}}" show-inherit="vm.model.variants.length > 1 && !property.culture && !activeVariant.language.isDefault" inherits-from="defaultVariant.language.name"> diff --git a/src/Umbraco.Web.UI.Client/src/views/components/elementeditor/umbelementeditorcontent.component.js b/src/Umbraco.Web.UI.Client/src/views/components/elementeditor/umbelementeditorcontent.component.js index 6dfcb07098..b4a650a0c5 100644 --- a/src/Umbraco.Web.UI.Client/src/views/components/elementeditor/umbelementeditorcontent.component.js +++ b/src/Umbraco.Web.UI.Client/src/views/components/elementeditor/umbelementeditorcontent.component.js @@ -1,6 +1,8 @@ (function () { 'use strict'; + // TODO: Add docs - this component is used to render a content item based on an Element Type as a nested editor + angular .module('umbraco.directives') .component('umbElementEditorContent', { diff --git a/src/Umbraco.Web.UI.Client/src/views/components/property/umb-property.html b/src/Umbraco.Web.UI.Client/src/views/components/property/umb-property.html index 4c757f0951..14ca023046 100644 --- a/src/Umbraco.Web.UI.Client/src/views/components/property/umb-property.html +++ b/src/Umbraco.Web.UI.Client/src/views/components/property/umb-property.html @@ -1,29 +1,30 @@
-
+
-
- - {{inheritsFrom}} +
+ + {{vm.inheritsFrom}} -
diff --git a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/blocklist/blocklist.html b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/blocklist/blocklist.html index efadc4dfd6..8c3bced573 100644 --- a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/blocklist/blocklist.html +++ b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/blocklist/blocklist.html @@ -1 +1 @@ - + diff --git a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/blocklist/blocklistentryeditors/inlineblock/inlineblock.editor.html b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/blocklist/blocklistentryeditors/inlineblock/inlineblock.editor.html index 60b3542d6c..360eeed8c0 100644 --- a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/blocklist/blocklistentryeditors/inlineblock/inlineblock.editor.html +++ b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/blocklist/blocklistentryeditors/inlineblock/inlineblock.editor.html @@ -1,5 +1,9 @@ -
- diff --git a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/blocklist/blocklistentryeditors/labelblock/labelblock.editor.less b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/blocklist/blocklistentryeditors/labelblock/labelblock.editor.less index 0ebcfaea53..f9fde1da97 100644 --- a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/blocklist/blocklistentryeditors/labelblock/labelblock.editor.less +++ b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/blocklist/blocklistentryeditors/labelblock/labelblock.editor.less @@ -1,5 +1,4 @@ .blockelement-labelblock-editor { - position: relative; display: block; margin-bottom: 4px; @@ -8,24 +7,21 @@ min-height: 48px; border: 1px solid @gray-9; border-radius: @baseBorderRadius; - cursor: pointer; color: @ui-action-discreet-type; - text-align: left; padding-left: 20px; padding-bottom: 2px; - user-select: none; - transition: border-color 120ms; - + i { font-size: 22px; margin-right: 5px; display: inline-block; vertical-align: middle; } + span { display: inline-block; vertical-align: middle; @@ -41,4 +37,8 @@ border-color: @ui-active; background-color: @ui-active; } + + &.--error { + border-color: @formErrorBorder !important; + } } diff --git a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/blocklist/umb-block-list-block.component.js b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/blocklist/umb-block-list-block.component.js deleted file mode 100644 index a01b5a0f51..0000000000 --- a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/blocklist/umb-block-list-block.component.js +++ /dev/null @@ -1,35 +0,0 @@ -(function () { - "use strict"; - - /** - * @ngdoc directive - * @name umbraco.directives.directive:umbBlockListBlock - * @description - * The component for a style-inheriting block of the block list property editor. - */ - - angular - .module("umbraco") - .component("umbBlockListBlock", { - template: '
', - controller: BlockListBlockController, - controllerAs: "model", - bindings: { - view: "@", - block: "=", - api: "<", - index: "<" - } - } - ); - - function BlockListBlockController($scope) { - var model = this; - model.$onInit = function() { - $scope.block = model.block; - $scope.api = model.api; - }; - } - - -})(); diff --git a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/blocklist/umb-block-list-property-editor.html b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/blocklist/umb-block-list-property-editor.html index af9719d70c..fb94a00631 100644 --- a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/blocklist/umb-block-list-property-editor.html +++ b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/blocklist/umb-block-list-property-editor.html @@ -18,47 +18,10 @@
+
-
- - - - - - -
- - - -
-
+ +
diff --git a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/blocklist/umb-block-list-row.html b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/blocklist/umb-block-list-row.html new file mode 100644 index 0000000000..62f4627aad --- /dev/null +++ b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/blocklist/umb-block-list-row.html @@ -0,0 +1,36 @@ + +
+ + + + +
+ + + +
+
+ diff --git a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/blocklist/umb-block-list-scoped-block.component.js b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/blocklist/umb-block-list-scoped-block.component.js deleted file mode 100644 index 8878db8ec0..0000000000 --- a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/blocklist/umb-block-list-scoped-block.component.js +++ /dev/null @@ -1,45 +0,0 @@ -(function () { - "use strict"; - - /** - * @ngdoc directive - * @name umbraco.directives.directive:umbBlockListScopedBlock - * @description - * The component for a style-scoped block of the block list property editor. - * Uses a ShadowDom to make a scoped element. - * This way the backoffice styling does not collide with the block style. - */ - - angular - .module("umbraco") - .component("umbBlockListScopedBlock", { - controller: BlockListScopedBlockController, - controllerAs: "model", - bindings: { - stylesheet: "@", - view: "@", - block: "=", - api: "<", - index: "<" - } - } - ); - - function BlockListScopedBlockController($compile, $element, $scope) { - var model = this; - model.$onInit = function() { - $scope.block = model.block; - $scope.api = model.api; - var shadowRoot = $element[0].attachShadow({mode:'open'}); - shadowRoot.innerHTML = ` - -
- `; - $compile(shadowRoot)($scope); - } - } - - -})(); diff --git a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/blocklist/umbBlockListPropertyEditor.component.js b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/blocklist/umbBlockListPropertyEditor.component.js index 02e53826b5..aa66472778 100644 --- a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/blocklist/umbBlockListPropertyEditor.component.js +++ b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/blocklist/umbBlockListPropertyEditor.component.js @@ -17,10 +17,10 @@ controller: BlockListController, controllerAs: "vm", bindings: { - model: "=", - propertyForm: "=" + model: "=" }, require: { + propertyForm: "^form", umbProperty: "?^umbProperty", umbVariantContent: '?^^umbVariantContent', umbVariantContentEditors: '?^^umbVariantContentEditors', @@ -28,7 +28,7 @@ } }); - function BlockListController($scope, editorService, clipboardService, localizationService, overlayService, blockEditorService) { + function BlockListController($scope, editorService, clipboardService, localizationService, overlayService, blockEditorService, udiService, serverValidationManager, angularHelper) { var unsubscribe = []; var modelObject; @@ -53,8 +53,8 @@ } vm.supportCopy = clipboardService.isSupported(); - vm.layout = [];// The layout object specific to this Block Editor, will be a direct reference from Property Model. - vm.availableBlockTypes = [];// Available block entries of this property editor. + vm.layout = []; // The layout object specific to this Block Editor, will be a direct reference from Property Model. + vm.availableBlockTypes = []; // Available block entries of this property editor. var labels = {}; vm.labels = labels; @@ -63,11 +63,20 @@ labels.content_createEmpty = data[1]; }); - - - - vm.$onInit = function() { + if (!vm.umbVariantContent) { + // not found, then fallback to searching the scope chain, this may be needed when DOM inheritance isn't maintained but scope + // inheritance is (i.e.infinite editing) + var found = angularHelper.traverseScopeChain($scope, s => s && s.vm && s.vm.constructor.name === "umbVariantContentController"); + vm.umbVariantContent = found ? found.vm : null; + if (!vm.umbVariantContent) { + throw "Could not find umbVariantContent in the $scope chain"; + } + } + + // set the onValueChanged callback, this will tell us if the block list model changed on the server + // once the data is submitted. If so we need to re-initialize + vm.model.onValueChanged = onServerValueChanged; inlineEditing = vm.model.config.useInlineEditingAsDefault; liveEditing = vm.model.config.useLiveEditing; @@ -121,8 +130,12 @@ } }; - - + // Called when we save the value, the server may return an updated data and our value is re-synced + // we need to deal with that here so that our model values are all in sync so we basically re-initialize. + function onServerValueChanged(newVal, oldVal) { + modelObject.update(newVal, $scope); + onLoaded(); + } function setDirty() { if (vm.propertyForm) { @@ -137,7 +150,7 @@ // Append the blockObjects to our layout. vm.layout.forEach(entry => { - // $block must have the data property to be a valid BlockObject, if not its concidered as a destroyed blockObject. + // $block must have the data property to be a valid BlockObject, if not its considered as a destroyed blockObject. if (entry.$block === undefined || entry.$block === null || entry.$block.data === undefined) { var block = getBlockObject(entry); @@ -172,11 +185,25 @@ if (block === null) return null; + // ensure that the containing content variant language/culture is transfered along + // to the scaffolded content object representing this block. This is required for validation + // along with ensuring that the umb-property inheritance is constently maintained. + if (vm.umbVariantContent.editor.content.language) { + block.content.language = vm.umbVariantContent.editor.content.language; + // currently we only ever deal with invariant content for blocks so there's only one + block.content.variants[0].tabs.forEach(tab => { + tab.properties.forEach(prop => { + prop.culture = vm.umbVariantContent.editor.content.language.culture; + }); + }); + } + + // TODO: Why is there a '/' prefixed? that means this will never work with virtual directories block.view = (block.config.view ? "/" + block.config.view : getDefaultViewForBlock(block)); block.hideContentInOverlay = block.config.forceHideContentEditorInOverlay === true || inlineEditing === true; block.showSettings = block.config.settingsElementTypeKey != null; - block.showCopy = vm.supportCopy && block.config.contentTypeKey != null;// if we have content, otherwise it dosnt make sense to copy. + block.showCopy = vm.supportCopy && block.config.contentTypeKey != null;// if we have content, otherwise it doesn't make sense to copy. return block; } @@ -211,15 +238,22 @@ } - - function deleteBlock(block) { var layoutIndex = vm.layout.findIndex(entry => entry.udi === block.content.udi); - if(layoutIndex === -1) { + if (layoutIndex === -1) { throw new Error("Could not find layout entry of block with udi: "+block.content.udi) } - vm.layout.splice(layoutIndex, 1); + + setDirty(); + + var removed = vm.layout.splice(layoutIndex, 1); + removed.forEach(x => { + // remove any server validation errors associated + var guid = udiService.getKey(x.udi); + serverValidationManager.removePropertyError(guid, vm.umbProperty.property.culture, vm.umbProperty.property.segment, "", { matchType: "contains" }); + }); + modelObject.removeDataAndDestroyModel(block); } @@ -234,7 +268,12 @@ blockObject.active = true; } - function editBlock(blockObject, openSettings) { + function editBlock(blockObject, openSettings, blockIndex, parentForm) { + + // this must be set + if (blockIndex === undefined) { + throw "blockIndex was not specified on call to editBlock"; + } var wasNotActiveBefore = blockObject.active !== true; @@ -259,17 +298,20 @@ } var blockEditorModel = { + $parentScope: $scope, // pass in a $parentScope, this maintains the scope inheritance in infinite editing + $parentForm: parentForm || vm.propertyForm, // pass in a $parentForm, this maintains the FormController hierarchy with the infinite editing view (if it contains a form) hideContent: blockObject.hideContentInOverlay, openSettings: openSettings === true, liveEditing: liveEditing, title: blockObject.label, + index: blockIndex, view: "views/common/infiniteeditors/blockeditor/blockeditor.html", size: blockObject.config.editorSize || "medium", submit: function(blockEditorModel) { if (liveEditing === false) { // transfer values when submitting in none-liveediting mode. - blockObject.retriveValuesFrom(blockEditorModel.content, blockEditorModel.settings); + blockObject.retrieveValuesFrom(blockEditorModel.content, blockEditorModel.settings); } blockObject.active = false; @@ -279,7 +321,7 @@ if (liveEditing === true) { // revert values when closing in liveediting mode. - blockObject.retriveValuesFrom(blockContentClone, blockSettingsClone); + blockObject.retrieveValuesFrom(blockContentClone, blockSettingsClone); } if (wasNotActiveBefore === true) { @@ -314,6 +356,8 @@ var amountOfAvailableTypes = vm.availableBlockTypes.length; var blockPickerModel = { + $parentScope: $scope, // pass in a $parentScope, this maintains the scope inheritance in infinite editing + $parentForm: vm.propertyForm, // pass in a $parentForm, this maintains the FormController hierarchy with the infinite editing view (if it contains a form) availableItems: vm.availableBlockTypes, title: vm.labels.grid_addElement, orderBy: "$index", @@ -347,7 +391,7 @@ if (inlineEditing === true) { activateBlock(vm.layout[createIndex].$block); } else if (inlineEditing === false && vm.layout[createIndex].$block.hideContentInOverlay !== true) { - editBlock(vm.layout[createIndex].$block); + editBlock(vm.layout[createIndex].$block, false, createIndex, blockPickerModel.$parentForm); } } } @@ -490,19 +534,18 @@ }); } - function openSettingsForBlock(block) { - editBlock(block, true); + // TODO: We'll need to pass in a parentForm here too + function openSettingsForBlock(block, blockIndex) { + editBlock(block, true, blockIndex); } - - vm.blockEditorApi = { activateBlock: activateBlock, editBlock: editBlock, copyBlock: copyBlock, requestDeleteBlock: requestDeleteBlock, deleteBlock: deleteBlock, - openSettingsForBlock: openSettingsForBlock + openSettingsForBlock: openSettingsForBlock } vm.sortableOptions = { diff --git a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/blocklist/umbblocklistblock.component.js b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/blocklist/umbblocklistblock.component.js new file mode 100644 index 0000000000..4474dbd55c --- /dev/null +++ b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/blocklist/umbblocklistblock.component.js @@ -0,0 +1,76 @@ +(function () { + "use strict"; + + /** + * @ngdoc directive + * @name umbraco.directives.directive:umbBlockListBlock + * @description + * The component to render the view for a block. + * If a stylesheet is used then this uses a ShadowDom to make a scoped element. + * This way the backoffice styling does not collide with the block style. + */ + + angular + .module("umbraco") + .component("umbBlockListBlock", { + controller: BlockListBlockController, + controllerAs: "model", + bindings: { + stylesheet: "@", + view: "@", + block: "=", + api: "<", + index: "<", + parentForm: "<" + }, + require: { + valFormManager: "^^valFormManager" + } + } + ); + + function BlockListBlockController($scope, $compile, $element) { + var model = this; + + model.$onInit = function () { + // This is ugly and is only necessary because we are not using components and instead + // relying on ng-include. It is definitely possible to compile the contents + // of the view into the DOM using $templateCache and $http instead of using + // ng - include which means that the controllerAs flows directly to the view. + // This would mean that any custom components would need to be updated instead of relying on $scope. + // Guess we'll leave it for now but means all things need to be copied to the $scope and then all + // primitives need to be watched. + + $scope.block = model.block; + $scope.api = model.api; + $scope.index = model.index; + $scope.parentForm = model.parentForm; + $scope.valFormManager = model.valFormManager; + + if (model.stylesheet) { + // TODO: Not sure why this needs a prefixed /? this means it will never work in a virtual directory + model.stylesheet = "/" + model.stylesheet; + var shadowRoot = $element[0].attachShadow({ mode: 'open' }); + shadowRoot.innerHTML = ` + +
+ `; + $compile(shadowRoot)($scope); + } + else { + $element.append($compile('
')($scope)); + } + }; + + // We need to watch for changes on primitive types and upate the $scope values. + model.$onChanges = function (changes) { + if (changes.index) { + $scope.index = changes.index.currentValue; + } + } + } + + +})(); diff --git a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/blocklist/umbblocklistrow.component.js b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/blocklist/umbblocklistrow.component.js new file mode 100644 index 0000000000..96a3648192 --- /dev/null +++ b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/blocklist/umbblocklistrow.component.js @@ -0,0 +1,34 @@ +(function () { + "use strict"; + + /** + * @ngdoc directive + * @name umbraco.directives.directive:umbBlockListRow + * @description + * renders each row for the block list editor + */ + + angular + .module("umbraco") + .component("umbBlockListRow", { + templateUrl: 'views/propertyeditors/blocklist/umb-block-list-row.html', + controller: BlockListRowController, + controllerAs: "vm", + bindings: { + blockEditorApi: "<", + layout: "<", + index: "<" + } + } + ); + + function BlockListRowController($scope) { + + var vm = this; + + vm.$onInit = function () { + + }; + } + +})(); diff --git a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/nestedcontent/nestedcontent.controller.js b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/nestedcontent/nestedcontent.controller.js index 59aaec662b..fe9725a7d8 100644 --- a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/nestedcontent/nestedcontent.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/nestedcontent/nestedcontent.controller.js @@ -65,7 +65,7 @@ } }); - function NestedContentController($scope, $interpolate, $filter, $timeout, contentResource, localizationService, iconHelper, clipboardService, eventsService, overlayService) { + function NestedContentController($scope, $interpolate, $filter, serverValidationManager, contentResource, localizationService, iconHelper, clipboardService, eventsService, overlayService) { var vm = this; var model = $scope.$parent.$parent.model; @@ -298,8 +298,15 @@ } function deleteNode(idx) { - vm.nodes.splice(idx, 1); + var removed = vm.nodes.splice(idx, 1); + setDirty(); + + removed.forEach(x => { + // remove any server validation errors associated + serverValidationManager.removePropertyError(x.key, vm.umbProperty.property.culture, vm.umbProperty.property.segment, "", { matchType: "contains" }); + }); + updateModel(); validate(); }; @@ -588,8 +595,14 @@ for (var p = 0; p < tab.properties.length; p++) { var prop = tab.properties[p]; + // store the original alias before we change below, see notes prop.propertyAlias = prop.alias; + + // NOTE: This is super ugly, the reason it is like this is because it controls the label/html id in the umb-property component at a higher level. + // not pretty :/ but we can't change this now since it would require a bunch of plumbing to be able to change the id's higher up. prop.alias = model.alias + "___" + prop.alias; + + // TODO: Do we need to deal with this separately? // Force validation to occur server side as this is the // only way we can have consistency between mandatory and // regex validation messages. Not ideal, but it works. @@ -664,8 +677,8 @@ ]; this.$onInit = function () { - if (this.umbProperty) { - this.umbProperty.setPropertyActions(propertyActions); + if (vm.umbProperty) { + vm.umbProperty.setPropertyActions(propertyActions); } }; diff --git a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/nestedcontent/nestedcontent.editor.html b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/nestedcontent/nestedcontent.editor.html index a2105c5675..125e920fe6 100644 --- a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/nestedcontent/nestedcontent.editor.html +++ b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/nestedcontent/nestedcontent.editor.html @@ -1,13 +1,19 @@ 
-
+
- - - + -
+ -

{{property.notSupportedMessage}}

+ -
+
+ +

{{property.notSupportedMessage}}

+ +
diff --git a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/nestedcontent/nestedcontent.propertyeditor.html b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/nestedcontent/nestedcontent.propertyeditor.html index da6e466b50..896f8cf4a4 100644 --- a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/nestedcontent/nestedcontent.propertyeditor.html +++ b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/nestedcontent/nestedcontent.propertyeditor.html @@ -6,30 +6,41 @@
-
+
-
+ -
+
-
- - +
+ +
+ +
+ + +
+
+ + +
+ +
-
- -
- -
+
diff --git a/src/Umbraco.Web.UI.Client/test/unit/common/services/block-editor-service.spec.js b/src/Umbraco.Web.UI.Client/test/unit/common/services/block-editor-service.spec.js index 8936f731bb..7af4b7a74a 100644 --- a/src/Umbraco.Web.UI.Client/test/unit/common/services/block-editor-service.spec.js +++ b/src/Umbraco.Web.UI.Client/test/unit/common/services/block-editor-service.spec.js @@ -145,7 +145,7 @@ }); - it('getBlockObject syncs primative values', function (done) { + it('getBlockObject syncs primitive values', function (done) { var propertyModel = angular.copy(propertyModelMock); diff --git a/src/Umbraco.Web.UI.Client/test/unit/common/services/server-validation-manager.spec.js b/src/Umbraco.Web.UI.Client/test/unit/common/services/server-validation-manager.spec.js index 5d2a618f46..ab2a5cdd69 100644 --- a/src/Umbraco.Web.UI.Client/test/unit/common/services/server-validation-manager.spec.js +++ b/src/Umbraco.Web.UI.Client/test/unit/common/services/server-validation-manager.spec.js @@ -1,10 +1,13 @@ describe('serverValidationManager tests', function () { - var serverValidationManager; + var $rootScope, serverValidationManager, $timeout; beforeEach(module('umbraco.services')); beforeEach(inject(function ($injector) { - serverValidationManager = $injector.get('serverValidationManager'); + $rootScope = $injector.get('$rootScope'); + $timeout = $injector.get('$timeout'); + serverValidationManager = $injector.get('serverValidationManager'); + serverValidationManager.clear(); })); describe('managing field validation errors', function () { @@ -314,6 +317,158 @@ }); + describe('managing complex editor validation errors', function () { + + // this root element doesn't have it's own attached errors, instead it has model state just + // showing that it has errors within it's nested properties. that ModelState is automatically + // added on the server side. + var nonRootLevelComplexValidationMsg = `[ + { + "$elementTypeAlias": "addressBook", + "$id": "34E3A26C-103D-4A05-AB9D-7E14032309C3", + "addresses": + [ + { + "$elementTypeAlias": "addressInfo", + "$id": "FBEAEE8F-4BC9-43EE-8B81-FCA8978850F1", + "ModelState": + { + "_Properties.city.invariant.null.country": [ + "City is not in Australia" + ], + "_Properties.city.invariant.null.capital": [ + "Not a capital city" + ] + } + }, + { + "$elementTypeAlias": "addressInfo", + "$id": "7170A4DD-2441-4B1B-A8D3-437D75C4CBC9", + "ModelState": + { + "_Properties.city.invariant.null.country": [ + "City is not in Australia" + ], + "_Properties.city.invariant.null.capital": [ + "Not a capital city" + ] + } + } + ], + "ModelState": + { + "_Properties.addresses.invariant.null": [ + "" + ] + } + } +]`; + + it('create dictionary of id to ModelState', function () { + + //arrange + var complexValidationMsg = `[ + { + "$elementTypeAlias": "addressBook", + "$id": "34E3A26C-103D-4A05-AB9D-7E14032309C3", + "addresses": + [ + { + "$elementTypeAlias": "addressInfo", + "$id": "FBEAEE8F-4BC9-43EE-8B81-FCA8978850F1", + "ModelState": + { + "_Properties.city.invariant.null.country": [ + "City is not in Australia" + ], + "_Properties.city.invariant.null.capital": [ + "Not a capital city" + ] + } + }, + { + "$elementTypeAlias": "addressInfo", + "$id": "7170A4DD-2441-4B1B-A8D3-437D75C4CBC9", + "ModelState": + { + "_Properties.city.invariant.null.country": [ + "City is not in Australia" + ], + "_Properties.city.invariant.null.capital": [ + "Not a capital city" + ] + } + } + ], + "ModelState": + { + "_Properties.addresses.invariant.null.counter": [ + "Must have at least 3 addresses" + ], + "_Properties.bookName.invariant.null.book": [ + "Invalid address book name" + ] + } + } +]`; + + //act + var ms = serverValidationManager.parseComplexEditorError(complexValidationMsg, "myBlockEditor"); + + //assert + expect(ms.length).toEqual(3); + expect(ms[0].validationPath).toEqual("myBlockEditor/34E3A26C-103D-4A05-AB9D-7E14032309C3"); + expect(ms[1].validationPath).toEqual("myBlockEditor/34E3A26C-103D-4A05-AB9D-7E14032309C3/addresses/FBEAEE8F-4BC9-43EE-8B81-FCA8978850F1"); + expect(ms[2].validationPath).toEqual("myBlockEditor/34E3A26C-103D-4A05-AB9D-7E14032309C3/addresses/7170A4DD-2441-4B1B-A8D3-437D75C4CBC9"); + + }); + + it('create dictionary of id to ModelState with inherited errors', function () { + + //act + var ms = serverValidationManager.parseComplexEditorError(nonRootLevelComplexValidationMsg, "myBlockEditor"); + + //assert + expect(ms.length).toEqual(3); + expect(ms[0].validationPath).toEqual("myBlockEditor/34E3A26C-103D-4A05-AB9D-7E14032309C3"); + var item0ModelState = ms[0].modelState; + expect(Object.keys(item0ModelState).length).toEqual(1); + expect(item0ModelState["_Properties.addresses.invariant.null"].length).toEqual(1); + expect(item0ModelState["_Properties.addresses.invariant.null"][0]).toEqual(""); + expect(ms[1].validationPath).toEqual("myBlockEditor/34E3A26C-103D-4A05-AB9D-7E14032309C3/addresses/FBEAEE8F-4BC9-43EE-8B81-FCA8978850F1"); + expect(ms[2].validationPath).toEqual("myBlockEditor/34E3A26C-103D-4A05-AB9D-7E14032309C3/addresses/7170A4DD-2441-4B1B-A8D3-437D75C4CBC9"); + + }); + + it('add errors for ModelState with inherited errors', function () { + + //act + let modelState = { + "_Properties.blockFeatures.invariant.null": [ + nonRootLevelComplexValidationMsg + ] + }; + serverValidationManager.addErrorsForModelState(modelState); + + //assert + var propertyErrors = [ + "blockFeatures", + "blockFeatures/34E3A26C-103D-4A05-AB9D-7E14032309C3/addresses", + "blockFeatures/34E3A26C-103D-4A05-AB9D-7E14032309C3/addresses/FBEAEE8F-4BC9-43EE-8B81-FCA8978850F1/city", + "blockFeatures/34E3A26C-103D-4A05-AB9D-7E14032309C3/addresses/7170A4DD-2441-4B1B-A8D3-437D75C4CBC9/city" + ] + // These will all exist + propertyErrors.forEach(x => expect(serverValidationManager.getPropertyError(x)).toBeDefined()); + + // These field errors also exist + expect(serverValidationManager.getPropertyError(propertyErrors[2], null, "country")).toBeDefined(); + expect(serverValidationManager.getPropertyError(propertyErrors[2], null, "capital")).toBeDefined(); + expect(serverValidationManager.getPropertyError(propertyErrors[3], null, "country")).toBeDefined(); + expect(serverValidationManager.getPropertyError(propertyErrors[3], null, "capital")).toBeDefined(); + }); + + }); + describe('validation error subscriptions', function() { it('can subscribe to a field error', function() { @@ -327,7 +482,9 @@ allErrors: allErrors }; }, null); + + //act serverValidationManager.addFieldError("Name", "Required"); serverValidationManager.addPropertyError("myProperty", null, "value1", "Some value 1", null); @@ -348,7 +505,9 @@ var cb2 = function () { }; serverValidationManager.subscribe(null, null, "Name", cb1, null); + serverValidationManager.subscribe(null, null, "Title", cb2, null); + //act serverValidationManager.addFieldError("Name", "Required"); @@ -374,6 +533,7 @@ var args1; var args2; var numCalled = 0; + var numCalledWithErrors = 0; //arrange serverValidationManager.subscribe("myProperty", null, "value1", function (isValid, propertyErrors, allErrors) { @@ -384,15 +544,19 @@ }; }, null); + serverValidationManager.subscribe("myProperty", null, "", function (isValid, propertyErrors, allErrors) { numCalled++; + if (propertyErrors.length > 0) { + numCalledWithErrors++; + } args2 = { isValid: isValid, propertyErrors: propertyErrors, allErrors: allErrors }; }, null); - + //act serverValidationManager.addPropertyError("myProperty", null, "value1", "Some value 1", null); serverValidationManager.addPropertyError("myProperty", null, "value2", "Some value 2", null); @@ -415,28 +579,36 @@ expect(args2.propertyErrors[0].errorMsg).toEqual("Some value 1"); expect(args2.propertyErrors[1].errorMsg).toEqual("Some value 2"); expect(args2.allErrors.length).toEqual(2); - //Even though only 2 errors are added, the callback is called 3 times because any call to addPropertyError will invoke the callback - // if the property has errors existing. - expect(numCalled).toEqual(3); + //3 errors are added but a call to subscribe also calls the callback + expect(numCalled).toEqual(4); + expect(numCalledWithErrors).toEqual(3); }); it('can subscribe to a culture error for both a property and its sub field', function () { var args1; var args2; var numCalled = 0; + var numCalledWithErrors = 0; //arrange serverValidationManager.subscribe(null, "en-US", null, function (isValid, propertyErrors, allErrors) { numCalled++; + if (propertyErrors.length > 0) { + numCalledWithErrors++; + } args1 = { isValid: isValid, propertyErrors: propertyErrors, allErrors: allErrors }; }, null); + serverValidationManager.subscribe(null, "es-ES", null, function (isValid, propertyErrors, allErrors) { numCalled++; + if (propertyErrors.length > 0) { + numCalledWithErrors++; + } args2 = { isValid: isValid, propertyErrors: propertyErrors, @@ -445,20 +617,162 @@ }, null); //act - serverValidationManager.addPropertyError("myProperty", null, "value1", "Some value 1", null); - serverValidationManager.addPropertyError("myProperty", "en-US", "value1", "Some value 1", null); - serverValidationManager.addPropertyError("myProperty", "en-US", "value2", "Some value 2", null); - serverValidationManager.addPropertyError("myProperty", "fr-FR", "", "Some value 3", null); + serverValidationManager.addPropertyError("myProperty", null, "value1", "Some value 1", null); // doesn't match + serverValidationManager.addPropertyError("myProperty", "en-US", "value1", "Some value 1", null); // matches callback 1 + serverValidationManager.addPropertyError("myProperty", "en-US", "value2", "Some value 2", null); // matches callback 1 + serverValidationManager.addPropertyError("myProperty", "fr-FR", "", "Some value 3", null); // doesn't match - but all callbacks still execute //assert - expect(args1).not.toBeUndefined(); expect(args1.isValid).toBe(false); + expect(args2.isValid).toBe(true); // no errors registered for this callback - expect(args2).toBeUndefined(); - - expect(numCalled).toEqual(2); + expect(numCalled).toEqual(10); // both subscriptions will be called once per addPropertyError and also called on subscribe + expect(numCalledWithErrors).toEqual(3); // the first subscription is called 3 times with errors because the 4th time we call addPropertyError all callbacks still execute }); - + + it('can subscribe to a property validation path prefix', function () { + var callbackA = []; + var callbackB = []; + + //arrange + serverValidationManager.subscribe("myProperty", null, null, function (isValid, propertyErrors, allErrors) { + if (propertyErrors.length > 0) { + callbackA.push(propertyErrors); + } + }, null, { matchType: "prefix" }); + + serverValidationManager.subscribe("myProperty/34E3A26C-103D-4A05-AB9D-7E14032309C3/addresses", null, null, function (isValid, propertyErrors, allErrors) { + if (propertyErrors.length > 0) { + callbackB.push(propertyErrors); + } + }, null, { matchType: "prefix" }); + + //act + // will match A: + serverValidationManager.addPropertyError("myProperty", null, null, "property error", null); + serverValidationManager.addPropertyError("myProperty", null, "value1", "value error", null); + // will match A + B + serverValidationManager.addPropertyError("myProperty/34E3A26C-103D-4A05-AB9D-7E14032309C3/addresses/FBEAEE8F-4BC9-43EE-8B81-FCA8978850F1/city", null, null, "property error", null); + serverValidationManager.addPropertyError("myProperty/34E3A26C-103D-4A05-AB9D-7E14032309C3/addresses/FBEAEE8F-4BC9-43EE-8B81-FCA8978850F1/city", null, "value1", "value error", null); + // won't match: + serverValidationManager.addPropertyError("myProperty", "en-US", null, "property error", null); + serverValidationManager.addPropertyError("myProperty/34E3A26C-103D-4A05-AB9D-7E14032309C3/addresses/FBEAEE8F-4BC9-43EE-8B81-FCA8978850F1/city", "en-US", null, "property error", null); + serverValidationManager.addPropertyError("otherProperty", null, null, "property error", null); + serverValidationManager.addPropertyError("otherProperty", null, "value1", "value error", null); + + //assert + + // both will be called each time addPropertyError is called + expect(callbackA.length).toEqual(8); + expect(callbackB.length).toEqual(6); // B - will only be called 6 times with errors because the first 2 calls to addPropertyError haven't added errors for B yet + expect(callbackA[callbackA.length - 1].length).toEqual(4); // 4 errors for A + expect(callbackB[callbackB.length - 1].length).toEqual(2); // 2 errors for B + + // clear the data and notify + callbackA = []; + callbackB = []; + + serverValidationManager.notify(); + $timeout.flush(); + + expect(callbackA.length).toEqual(1); + expect(callbackB.length).toEqual(1); + expect(callbackA[0].length).toEqual(4); // 4 errors for A + expect(callbackB[0].length).toEqual(2); // 2 errors for B + + }); + + it('can subscribe to a property validation path suffix', function () { + var callbackA = []; + var callbackB = []; + + //arrange + serverValidationManager.subscribe("myProperty", null, null, function (isValid, propertyErrors, allErrors) { + if (propertyErrors.length > 0) { + callbackA.push(propertyErrors); + } + }, null, { matchType: "suffix" }); + + serverValidationManager.subscribe("city", null, null, function (isValid, propertyErrors, allErrors) { + if (propertyErrors.length > 0) { + callbackB.push(propertyErrors); + } + }, null, { matchType: "suffix" }); + + //act + // will match A: + serverValidationManager.addPropertyError("myProperty", null, null, "property error", null); + serverValidationManager.addPropertyError("myProperty", null, "value1", "value error", null); + // will match B + serverValidationManager.addPropertyError("myProperty/34E3A26C-103D-4A05-AB9D-7E14032309C3/addresses/FBEAEE8F-4BC9-43EE-8B81-FCA8978850F1/city", null, null, "property error", null); + serverValidationManager.addPropertyError("myProperty/34E3A26C-103D-4A05-AB9D-7E14032309C3/addresses/FBEAEE8F-4BC9-43EE-8B81-FCA8978850F1/city", null, "value1", "value error", null); + // won't match: + serverValidationManager.addPropertyError("myProperty", "en-US", null, "property error", null); + serverValidationManager.addPropertyError("myProperty/34E3A26C-103D-4A05-AB9D-7E14032309C3/addresses/FBEAEE8F-4BC9-43EE-8B81-FCA8978850F1/city", "en-US", null, "property error", null); + serverValidationManager.addPropertyError("otherProperty", null, null, "property error", null); + serverValidationManager.addPropertyError("otherProperty", null, "value1", "value error", null); + + //assert + + // both will be called each time addPropertyError is called + expect(callbackA.length).toEqual(8); + expect(callbackB.length).toEqual(6); // B - will only be called 6 times with errors because the first 2 calls to addPropertyError haven't added errors for B yet + expect(callbackA[callbackA.length - 1].length).toEqual(2); // 2 errors for A + expect(callbackB[callbackB.length - 1].length).toEqual(2); // 2 errors for B + + // clear the data and notify + callbackA = []; + callbackB = []; + + serverValidationManager.notify(); + $timeout.flush(); + + expect(callbackA.length).toEqual(1); + expect(callbackB.length).toEqual(1); + expect(callbackA[0].length).toEqual(2); // 2 errors for A + expect(callbackB[0].length).toEqual(2); // 2 errors for B + + }); + + it('can subscribe to a property validation path contains', function () { + var callbackA = []; + + //arrange + serverValidationManager.subscribe("addresses", null, null, function (isValid, propertyErrors, allErrors) { + if (propertyErrors.length > 0) { + callbackA.push(propertyErrors); + } + }, null, { matchType: "contains" }); + + //act + // will match A: + serverValidationManager.addPropertyError("addresses", null, null, "property error", null); + serverValidationManager.addPropertyError("addresses", null, "value1", "value error", null); + serverValidationManager.addPropertyError("myProperty/34E3A26C-103D-4A05-AB9D-7E14032309C3/addresses/FBEAEE8F-4BC9-43EE-8B81-FCA8978850F1/city", null, null, "property error", null); + serverValidationManager.addPropertyError("myProperty/34E3A26C-103D-4A05-AB9D-7E14032309C3/addresses/FBEAEE8F-4BC9-43EE-8B81-FCA8978850F1/city", null, "value1", "value error", null); + // won't match: + serverValidationManager.addPropertyError("addresses", "en-US", null, "property error", null); + serverValidationManager.addPropertyError("addresses/34E3A26C-103D-4A05-AB9D-7E14032309C3/addresses/FBEAEE8F-4BC9-43EE-8B81-FCA8978850F1/city", "en-US", null, "property error", null); + serverValidationManager.addPropertyError("otherProperty", null, null, "property error", null); + serverValidationManager.addPropertyError("otherProperty", null, "value1", "value error", null); + + //assert + + // both will be called each time addPropertyError is called + expect(callbackA.length).toEqual(8); + expect(callbackA[callbackA.length - 1].length).toEqual(4); // 4 errors for A + + // clear the data and notify + callbackA = []; + + serverValidationManager.notify(); + $timeout.flush(); + + expect(callbackA.length).toEqual(1); + expect(callbackA[0].length).toEqual(4); // 4 errors for A + + }); + // TODO: Finish testing the rest! }); diff --git a/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj b/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj index 8a5f223bd5..87b47a55c3 100644 --- a/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj +++ b/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj @@ -126,6 +126,10 @@ {52ac0ba8-a60e-4e36-897b-e8b97a54ed1c} Umbraco.ModelsBuilder.Embedded + + {fb5676ed-7a69-492c-b802-e7b24144c0fc} + Umbraco.TestData + {651e1350-91b6-44b7-bd60-7207006d7003} Umbraco.Web @@ -349,6 +353,9 @@ 8700 / http://localhost:8700 + 8640 + / + http://localhost:8640 False False @@ -430,4 +437,4 @@ - + \ No newline at end of file diff --git a/src/Umbraco.Web/Compose/DatabaseServerRegistrarAndMessengerComponent.cs b/src/Umbraco.Web/Compose/DatabaseServerRegistrarAndMessengerComponent.cs index 6dfad8c45b..2fa9d80779 100644 --- a/src/Umbraco.Web/Compose/DatabaseServerRegistrarAndMessengerComponent.cs +++ b/src/Umbraco.Web/Compose/DatabaseServerRegistrarAndMessengerComponent.cs @@ -218,7 +218,7 @@ namespace Umbraco.Web.Compose } catch (Exception e) { - _logger.Error("Failed (will repeat).", e); + _logger.Error(e, "Failed (will repeat)."); } return true; // repeat } diff --git a/src/Umbraco.Web/Editors/Binders/BlueprintItemBinder.cs b/src/Umbraco.Web/Editors/Binders/BlueprintItemBinder.cs index eb4d482b14..bd86d74f5d 100644 --- a/src/Umbraco.Web/Editors/Binders/BlueprintItemBinder.cs +++ b/src/Umbraco.Web/Editors/Binders/BlueprintItemBinder.cs @@ -11,8 +11,8 @@ namespace Umbraco.Web.Editors.Binders { } - public BlueprintItemBinder(ILogger logger, ServiceContext services, IUmbracoContextAccessor umbracoContextAccessor) - : base(logger, services, umbracoContextAccessor) + public BlueprintItemBinder(ServiceContext services) + : base(services) { } diff --git a/src/Umbraco.Web/Editors/Binders/ContentItemBinder.cs b/src/Umbraco.Web/Editors/Binders/ContentItemBinder.cs index a6cba6ce41..609974bef7 100644 --- a/src/Umbraco.Web/Editors/Binders/ContentItemBinder.cs +++ b/src/Umbraco.Web/Editors/Binders/ContentItemBinder.cs @@ -17,16 +17,13 @@ namespace Umbraco.Web.Editors.Binders /// internal class ContentItemBinder : IModelBinder { - private readonly ContentModelBinderHelper _modelBinderHelper; - - public ContentItemBinder() : this(Current.Logger, Current.Services, Current.UmbracoContextAccessor) + public ContentItemBinder() : this(Current.Services) { } - public ContentItemBinder(ILogger logger, ServiceContext services, IUmbracoContextAccessor umbracoContextAccessor) + public ContentItemBinder(ServiceContext services) { Services = services; - _modelBinderHelper = new ContentModelBinderHelper(); } protected ServiceContext Services { get; } @@ -39,10 +36,20 @@ namespace Umbraco.Web.Editors.Binders /// public bool BindModel(HttpActionContext actionContext, ModelBindingContext bindingContext) { - var model = _modelBinderHelper.BindModelFromMultipartRequest(actionContext, bindingContext); + var model = ContentModelBinderHelper.BindModelFromMultipartRequest(actionContext, bindingContext); if (model == null) return false; - model.PersistedContent = ContentControllerBase.IsCreatingAction(model.Action) ? CreateNew(model) : GetExisting(model); + BindModel(model, ContentControllerBase.IsCreatingAction(model.Action) ? CreateNew(model) : GetExisting(model)); + + return true; + } + + internal static void BindModel(ContentItemSave model, IContent persistedContent) + { + if (model is null) throw new ArgumentNullException(nameof(model)); + if (persistedContent is null) throw new ArgumentNullException(nameof(persistedContent)); + + model.PersistedContent = persistedContent; //create the dto from the persisted model if (model.PersistedContent != null) @@ -60,11 +67,9 @@ namespace Umbraco.Web.Editors.Binders }); //now map all of the saved values to the dto - _modelBinderHelper.MapPropertyValuesFromSaved(variant, variant.PropertyCollectionDto); + ContentModelBinderHelper.MapPropertyValuesFromSaved(variant, variant.PropertyCollectionDto); } } - - return true; } protected virtual IContent GetExisting(ContentItemSave model) diff --git a/src/Umbraco.Web/Editors/Binders/ContentModelBinderHelper.cs b/src/Umbraco.Web/Editors/Binders/ContentModelBinderHelper.cs index b962de74ac..a017ae5afb 100644 --- a/src/Umbraco.Web/Editors/Binders/ContentModelBinderHelper.cs +++ b/src/Umbraco.Web/Editors/Binders/ContentModelBinderHelper.cs @@ -15,9 +15,9 @@ namespace Umbraco.Web.Editors.Binders /// /// Helper methods to bind media/member models /// - internal class ContentModelBinderHelper + internal static class ContentModelBinderHelper { - public TModelSave BindModelFromMultipartRequest(HttpActionContext actionContext, ModelBindingContext bindingContext) + public static TModelSave BindModelFromMultipartRequest(HttpActionContext actionContext, ModelBindingContext bindingContext) where TModelSave : IHaveUploadedFiles { var result = actionContext.ReadAsMultipart(SystemDirectories.TempFileUploads); @@ -86,7 +86,7 @@ namespace Umbraco.Web.Editors.Binders /// /// /// - public void MapPropertyValuesFromSaved(IContentProperties saveModel, ContentPropertyCollectionDto dto) + public static void MapPropertyValuesFromSaved(IContentProperties saveModel, ContentPropertyCollectionDto dto) { //NOTE: Don't convert this to linq, this is much quicker foreach (var p in saveModel.Properties) diff --git a/src/Umbraco.Web/Editors/Binders/MediaItemBinder.cs b/src/Umbraco.Web/Editors/Binders/MediaItemBinder.cs index 1084aa16ea..bfd5c853d5 100644 --- a/src/Umbraco.Web/Editors/Binders/MediaItemBinder.cs +++ b/src/Umbraco.Web/Editors/Binders/MediaItemBinder.cs @@ -13,7 +13,6 @@ namespace Umbraco.Web.Editors.Binders /// internal class MediaItemBinder : IModelBinder { - private readonly ContentModelBinderHelper _modelBinderHelper; private readonly ServiceContext _services; public MediaItemBinder() : this(Current.Services) @@ -23,7 +22,6 @@ namespace Umbraco.Web.Editors.Binders public MediaItemBinder(ServiceContext services) { _services = services; - _modelBinderHelper = new ContentModelBinderHelper(); } /// @@ -34,7 +32,7 @@ namespace Umbraco.Web.Editors.Binders /// public bool BindModel(HttpActionContext actionContext, ModelBindingContext bindingContext) { - var model = _modelBinderHelper.BindModelFromMultipartRequest(actionContext, bindingContext); + var model = ContentModelBinderHelper.BindModelFromMultipartRequest(actionContext, bindingContext); if (model == null) return false; model.PersistedContent = ContentControllerBase.IsCreatingAction(model.Action) ? CreateNew(model) : GetExisting(model); @@ -44,7 +42,7 @@ namespace Umbraco.Web.Editors.Binders { model.PropertyCollectionDto = Current.Mapper.Map(model.PersistedContent); //now map all of the saved values to the dto - _modelBinderHelper.MapPropertyValuesFromSaved(model, model.PropertyCollectionDto); + ContentModelBinderHelper.MapPropertyValuesFromSaved(model, model.PropertyCollectionDto); } model.Name = model.Name.Trim(); diff --git a/src/Umbraco.Web/Editors/Binders/MemberBinder.cs b/src/Umbraco.Web/Editors/Binders/MemberBinder.cs index 60b4f85c21..bd51d268ee 100644 --- a/src/Umbraco.Web/Editors/Binders/MemberBinder.cs +++ b/src/Umbraco.Web/Editors/Binders/MemberBinder.cs @@ -20,7 +20,6 @@ namespace Umbraco.Web.Editors.Binders /// internal class MemberBinder : IModelBinder { - private readonly ContentModelBinderHelper _modelBinderHelper; private readonly ServiceContext _services; public MemberBinder() : this(Current.Services) @@ -30,7 +29,6 @@ namespace Umbraco.Web.Editors.Binders public MemberBinder(ServiceContext services) { _services = services; - _modelBinderHelper = new ContentModelBinderHelper(); } /// @@ -41,7 +39,7 @@ namespace Umbraco.Web.Editors.Binders /// public bool BindModel(HttpActionContext actionContext, ModelBindingContext bindingContext) { - var model = _modelBinderHelper.BindModelFromMultipartRequest(actionContext, bindingContext); + var model = ContentModelBinderHelper.BindModelFromMultipartRequest(actionContext, bindingContext); if (model == null) return false; model.PersistedContent = ContentControllerBase.IsCreatingAction(model.Action) ? CreateNew(model) : GetExisting(model); @@ -51,7 +49,7 @@ namespace Umbraco.Web.Editors.Binders { model.PropertyCollectionDto = Current.Mapper.Map(model.PersistedContent); //now map all of the saved values to the dto - _modelBinderHelper.MapPropertyValuesFromSaved(model, model.PropertyCollectionDto); + ContentModelBinderHelper.MapPropertyValuesFromSaved(model, model.PropertyCollectionDto); } model.Name = model.Name.Trim(); diff --git a/src/Umbraco.Web/Editors/Filters/ContentModelValidator.cs b/src/Umbraco.Web/Editors/Filters/ContentModelValidator.cs index 810c2d1bea..9de27248a6 100644 --- a/src/Umbraco.Web/Editors/Filters/ContentModelValidator.cs +++ b/src/Umbraco.Web/Editors/Filters/ContentModelValidator.cs @@ -1,15 +1,19 @@ -using System; +using Newtonsoft.Json; +using System; using System.Collections.Generic; +using System.ComponentModel.DataAnnotations; using System.Linq; using System.Net; using System.Net.Http; using System.Web.Http.Controllers; using System.Web.Http.ModelBinding; +using Umbraco.Core; using Umbraco.Core.Logging; using Umbraco.Core.Models; using Umbraco.Core.PropertyEditors; using Umbraco.Core.Services; using Umbraco.Web.Models.ContentEditing; +using Umbraco.Web.PropertyEditors.Validation; namespace Umbraco.Web.Editors.Filters { @@ -125,18 +129,6 @@ namespace Umbraco.Web.Editors.Filters { var properties = modelWithProperties.Properties.ToDictionary(x => x.Alias, x => x); - // Retrieve default messages used for required and regex validatation. We'll replace these - // if set with custom ones if they've been provided for a given property. - var requiredDefaultMessages = new[] - { - _textService.Localize("validation", "invalidNull"), - _textService.Localize("validation", "invalidEmpty") - }; - var formatDefaultMessages = new[] - { - _textService.Localize("validation", "invalidPattern"), - }; - foreach (var p in dto.Properties) { var editor = p.PropertyEditor; @@ -156,7 +148,7 @@ namespace Umbraco.Web.Editors.Filters var postedValue = postedProp.Value; - ValidatePropertyValue(model, modelWithProperties, editor, p, postedValue, modelState, requiredDefaultMessages, formatDefaultMessages); + ValidatePropertyValue(model, modelWithProperties, editor, p, postedValue, modelState); } @@ -180,26 +172,29 @@ namespace Umbraco.Web.Editors.Filters IDataEditor editor, ContentPropertyDto property, object postedValue, - ModelStateDictionary modelState, - string[] requiredDefaultMessages, - string[] formatDefaultMessages) + ModelStateDictionary modelState) { - var valueEditor = editor.GetValueEditor(property.DataType.Configuration); - foreach (var r in valueEditor.Validate(postedValue, property.IsRequired, property.ValidationRegExp)) + if (property is null) throw new ArgumentNullException(nameof(property)); + if (property.DataType is null) throw new InvalidOperationException($"{nameof(property)}.{nameof(property.DataType)} cannot be null"); + + foreach (var validationResult in PropertyValidationService.ValidatePropertyValue( + _textService, editor, property.DataType, postedValue, property.IsRequired, + property.ValidationRegExp, property.IsRequiredMessage, property.ValidationRegExpMessage)) { - // If we've got custom error messages, we'll replace the default ones that will have been applied in the call to Validate(). - if (property.IsRequired && !string.IsNullOrWhiteSpace(property.IsRequiredMessage) && requiredDefaultMessages.Contains(r.ErrorMessage, StringComparer.OrdinalIgnoreCase)) - { - r.ErrorMessage = property.IsRequiredMessage; - } - - if (!string.IsNullOrWhiteSpace(property.ValidationRegExp) && !string.IsNullOrWhiteSpace(property.ValidationRegExpMessage) && formatDefaultMessages.Contains(r.ErrorMessage, StringComparer.OrdinalIgnoreCase)) - { - r.ErrorMessage = property.ValidationRegExpMessage; - } - - modelState.AddPropertyError(r, property.Alias, property.Culture, property.Segment); + AddPropertyError(model, modelWithProperties, editor, property, validationResult, modelState); } } + + protected virtual void AddPropertyError( + TModelSave model, + TModelWithProperties modelWithProperties, + IDataEditor editor, + ContentPropertyDto property, + ValidationResult validationResult, + ModelStateDictionary modelState) + { + modelState.AddPropertyError(validationResult, property.Alias, property.Culture, property.Segment); + } + } } diff --git a/src/Umbraco.Web/Editors/Filters/ContentSaveModelValidator.cs b/src/Umbraco.Web/Editors/Filters/ContentSaveModelValidator.cs index 39bd6ab0f4..09995c1104 100644 --- a/src/Umbraco.Web/Editors/Filters/ContentSaveModelValidator.cs +++ b/src/Umbraco.Web/Editors/Filters/ContentSaveModelValidator.cs @@ -1,5 +1,8 @@ -using Umbraco.Core.Logging; +using System.ComponentModel.DataAnnotations; +using System.Web.Http.ModelBinding; +using Umbraco.Core.Logging; using Umbraco.Core.Models; +using Umbraco.Core.PropertyEditors; using Umbraco.Core.Services; using Umbraco.Web.Models.ContentEditing; @@ -13,5 +16,6 @@ namespace Umbraco.Web.Editors.Filters public ContentSaveModelValidator(ILogger logger, IUmbracoContextAccessor umbracoContextAccessor, ILocalizedTextService textService) : base(logger, umbracoContextAccessor, textService) { } + } } diff --git a/src/Umbraco.Web/ModelStateExtensions.cs b/src/Umbraco.Web/ModelStateExtensions.cs index 10b1b5dadd..d03472ef5e 100644 --- a/src/Umbraco.Web/ModelStateExtensions.cs +++ b/src/Umbraco.Web/ModelStateExtensions.cs @@ -1,4 +1,5 @@ -using System; +using Newtonsoft.Json; +using System; using System.Collections.Generic; using System.ComponentModel.DataAnnotations; using System.Linq; @@ -6,6 +7,7 @@ using System.Web.Mvc; using Umbraco.Core; using Umbraco.Core.Models; using Umbraco.Core.Services; +using Umbraco.Web.PropertyEditors.Validation; namespace Umbraco.Web { @@ -40,9 +42,9 @@ namespace Umbraco.Web { return state.Where(v => v.Key.StartsWith(prefix + ".")).All(v => !v.Value.Errors.Any()); } - + /// - /// Adds the error to model state correctly for a property so we can use it on the client side. + /// Adds an error to model state for a property so we can use it on the client side. /// /// /// @@ -51,9 +53,24 @@ namespace Umbraco.Web internal static void AddPropertyError(this System.Web.Http.ModelBinding.ModelStateDictionary modelState, ValidationResult result, string propertyAlias, string culture = "", string segment = "") { - if (culture == null) - culture = ""; - modelState.AddValidationError(result, "_Properties", propertyAlias, + modelState.AddPropertyValidationError(new ContentPropertyValidationResult(result, culture, segment), propertyAlias, culture, segment); + } + + /// + /// Adds the to the model state with the appropriate keys for property errors + /// + /// + /// + /// + /// + /// + internal static void AddPropertyValidationError(this System.Web.Http.ModelBinding.ModelStateDictionary modelState, + ValidationResult result, string propertyAlias, string culture = "", string segment = "") + { + modelState.AddValidationError( + result, + "_Properties", + propertyAlias, //if the culture is null, we'll add the term 'invariant' as part of the key culture.IsNullOrWhiteSpace() ? "invariant" : culture, // if the segment is null, we'll add the term 'null' as part of the key @@ -166,12 +183,12 @@ namespace Umbraco.Web var delimitedParts = string.Join(".", parts); foreach (var memberName in result.MemberNames) { - modelState.TryAddModelError($"{delimitedParts}.{memberName}", result.ErrorMessage); + modelState.TryAddModelError($"{delimitedParts}.{memberName}", result.ToString()); withNames = true; } if (!withNames) { - modelState.TryAddModelError($"{delimitedParts}", result.ErrorMessage); + modelState.TryAddModelError($"{delimitedParts}", result.ToString()); } } @@ -236,5 +253,6 @@ namespace Umbraco.Web }; } + } } diff --git a/src/Umbraco.Web/Models/ContentEditing/ContentItemSave.cs b/src/Umbraco.Web/Models/ContentEditing/ContentItemSave.cs index 4dbbb1385a..aace4645dd 100644 --- a/src/Umbraco.Web/Models/ContentEditing/ContentItemSave.cs +++ b/src/Umbraco.Web/Models/ContentEditing/ContentItemSave.cs @@ -14,7 +14,7 @@ namespace Umbraco.Web.Models.ContentEditing [DataContract(Name = "content", Namespace = "")] public class ContentItemSave : IContentSave { - protected ContentItemSave() + public ContentItemSave() { UploadedFiles = new List(); Variants = new List(); diff --git a/src/Umbraco.Web/Models/Mapping/ContentPropertyBasicMapper.cs b/src/Umbraco.Web/Models/Mapping/ContentPropertyBasicMapper.cs index cf1bc3c253..2e035430df 100644 --- a/src/Umbraco.Web/Models/Mapping/ContentPropertyBasicMapper.cs +++ b/src/Umbraco.Web/Models/Mapping/ContentPropertyBasicMapper.cs @@ -44,6 +44,9 @@ namespace Umbraco.Web.Models.Mapping property.PropertyType.PropertyEditorAlias); editor = _propertyEditors[Constants.PropertyEditors.Aliases.Label]; + + if (editor == null) + throw new InvalidOperationException($"Could not resolve the property editor {Constants.PropertyEditors.Aliases.Label}"); } dest.Id = property.Id; diff --git a/src/Umbraco.Web/PropertyEditors/BlockEditorPropertyEditor.cs b/src/Umbraco.Web/PropertyEditors/BlockEditorPropertyEditor.cs index b32a91c058..d2d144272c 100644 --- a/src/Umbraco.Web/PropertyEditors/BlockEditorPropertyEditor.cs +++ b/src/Umbraco.Web/PropertyEditors/BlockEditorPropertyEditor.cs @@ -2,9 +2,7 @@ using Newtonsoft.Json.Linq; using System; using System.Collections.Generic; -using System.ComponentModel.DataAnnotations; using System.Linq; -using System.Text.RegularExpressions; using Umbraco.Core; using Umbraco.Core.Logging; using Umbraco.Core.Models; @@ -12,9 +10,11 @@ using Umbraco.Core.Models.Blocks; using Umbraco.Core.Models.Editors; using Umbraco.Core.PropertyEditors; using Umbraco.Core.Services; +using static Umbraco.Core.Models.Blocks.BlockEditorData; namespace Umbraco.Web.PropertyEditors { + /// /// Abstract class for block editor based editors /// @@ -22,15 +22,15 @@ namespace Umbraco.Web.PropertyEditors { public const string ContentTypeKeyPropertyKey = "contentTypeKey"; public const string UdiPropertyKey = "udi"; - private readonly IBlockEditorDataHelper _dataHelper; + private readonly ILocalizedTextService _localizedTextService; private readonly Lazy _propertyEditors; private readonly IDataTypeService _dataTypeService; private readonly IContentTypeService _contentTypeService; - public BlockEditorPropertyEditor(ILogger logger, Lazy propertyEditors, IDataTypeService dataTypeService, IContentTypeService contentTypeService, IBlockEditorDataHelper dataHelper) + public BlockEditorPropertyEditor(ILogger logger, Lazy propertyEditors, IDataTypeService dataTypeService, IContentTypeService contentTypeService, ILocalizedTextService localizedTextService) : base(logger) { - _dataHelper = dataHelper; + _localizedTextService = localizedTextService; _propertyEditors = propertyEditors; _dataTypeService = dataTypeService; _contentTypeService = contentTypeService; @@ -41,23 +41,21 @@ namespace Umbraco.Web.PropertyEditors #region Value Editor - protected override IDataValueEditor CreateValueEditor() => new BlockEditorPropertyValueEditor(Attribute, _dataHelper, PropertyEditors, _dataTypeService, _contentTypeService); + protected override IDataValueEditor CreateValueEditor() => new BlockEditorPropertyValueEditor(Attribute, PropertyEditors, _dataTypeService, _contentTypeService, _localizedTextService); internal class BlockEditorPropertyValueEditor : DataValueEditor, IDataValueReference { - private readonly IBlockEditorDataHelper _dataHelper; private readonly PropertyEditorCollection _propertyEditors; - private readonly IDataTypeService _dataTypeService; + private readonly IDataTypeService _dataTypeService; // TODO: Not used yet but we'll need it to fill in the FromEditor/ToEditor private readonly BlockEditorValues _blockEditorValues; - public BlockEditorPropertyValueEditor(DataEditorAttribute attribute, IBlockEditorDataHelper dataHelper, PropertyEditorCollection propertyEditors, IDataTypeService dataTypeService, IContentTypeService contentTypeService) + public BlockEditorPropertyValueEditor(DataEditorAttribute attribute, PropertyEditorCollection propertyEditors, IDataTypeService dataTypeService, IContentTypeService contentTypeService, ILocalizedTextService textService) : base(attribute) { - _dataHelper = dataHelper; _propertyEditors = propertyEditors; _dataTypeService = dataTypeService; - _blockEditorValues = new BlockEditorValues(dataHelper, contentTypeService); - Validators.Add(new BlockEditorValidator(propertyEditors, dataTypeService, _blockEditorValues)); + _blockEditorValues = new BlockEditorValues(new BlockListEditorDataConverter(), contentTypeService); + Validators.Add(new BlockEditorValidator(_blockEditorValues, propertyEditors, dataTypeService, textService)); } public IEnumerable GetReferences(object value) @@ -66,210 +64,146 @@ namespace Umbraco.Web.PropertyEditors var result = new List(); - foreach (var row in _blockEditorValues.GetPropertyValues(rawJson, out _)) + foreach (var row in _blockEditorValues.GetPropertyValues(rawJson)) { - if (row.PropType == null) continue; + foreach (var prop in row.PropertyValues) + { + var propEditor = _propertyEditors[prop.Value.PropertyType.PropertyEditorAlias]; - var propEditor = _propertyEditors[row.PropType.PropertyEditorAlias]; + var valueEditor = propEditor?.GetValueEditor(); + if (!(valueEditor is IDataValueReference reference)) continue; - var valueEditor = propEditor?.GetValueEditor(); - if (!(valueEditor is IDataValueReference reference)) continue; + var val = prop.Value.Value?.ToString(); - var val = row.JsonRowValue[row.PropKey]?.ToString(); + var refs = reference.GetReferences(val); - var refs = reference.GetReferences(val); - - result.AddRange(refs); + result.AddRange(refs); + } } return result; } } - internal class BlockEditorValidator : IValueValidator + internal class BlockEditorValidator : ComplexEditorValidator { - private readonly PropertyEditorCollection _propertyEditors; - private readonly IDataTypeService _dataTypeService; private readonly BlockEditorValues _blockEditorValues; - public BlockEditorValidator(PropertyEditorCollection propertyEditors, IDataTypeService dataTypeService, BlockEditorValues blockEditorValues) + public BlockEditorValidator(BlockEditorValues blockEditorValues, PropertyEditorCollection propertyEditors, IDataTypeService dataTypeService, ILocalizedTextService textService) : base(propertyEditors, dataTypeService, textService) { - _propertyEditors = propertyEditors; - _dataTypeService = dataTypeService; _blockEditorValues = blockEditorValues; } - public IEnumerable Validate(object rawValue, string valueType, object dataTypeConfiguration) + protected override IEnumerable GetElementTypeValidation(object value) { - var validationResults = new List(); - - foreach (var row in _blockEditorValues.GetPropertyValues(rawValue, out _)) + foreach (var row in _blockEditorValues.GetPropertyValues(value)) { - if (row.PropType == null) continue; - - var config = _dataTypeService.GetDataType(row.PropType.DataTypeId).Configuration; - var propertyEditor = _propertyEditors[row.PropType.PropertyEditorAlias]; - if (propertyEditor == null) continue; - - foreach (var validator in propertyEditor.GetValueEditor().Validators) + var elementValidation = new ElementTypeValidationModel(row.ContentTypeAlias, row.Id); + foreach (var prop in row.PropertyValues) { - foreach (var result in validator.Validate(row.JsonRowValue[row.PropKey], propertyEditor.GetValueEditor().ValueType, config)) - { - result.ErrorMessage = "Item " + (row.RowIndex + 1) + " '" + row.PropType.Name + "' " + result.ErrorMessage; - validationResults.Add(result); - } - } - - // Check mandatory - if (row.PropType.Mandatory) - { - if (row.JsonRowValue[row.PropKey] == null) - { - var message = string.IsNullOrWhiteSpace(row.PropType.MandatoryMessage) - ? $"'{row.PropType.Name}' cannot be null" - : row.PropType.MandatoryMessage; - validationResults.Add(new ValidationResult($"Item {(row.RowIndex + 1)}: {message}", new[] { row.PropKey })); - } - else if (row.JsonRowValue[row.PropKey].ToString().IsNullOrWhiteSpace() || (row.JsonRowValue[row.PropKey].Type == JTokenType.Array && !row.JsonRowValue[row.PropKey].HasValues)) - { - var message = string.IsNullOrWhiteSpace(row.PropType.MandatoryMessage) - ? $"'{row.PropType.Name}' cannot be empty" - : row.PropType.MandatoryMessage; - validationResults.Add(new ValidationResult($"Item {(row.RowIndex + 1)}: {message}", new[] { row.PropKey })); - } - } - - // Check regex - if (!row.PropType.ValidationRegExp.IsNullOrWhiteSpace() - && row.JsonRowValue[row.PropKey] != null && !row.JsonRowValue[row.PropKey].ToString().IsNullOrWhiteSpace()) - { - var regex = new Regex(row.PropType.ValidationRegExp); - if (!regex.IsMatch(row.JsonRowValue[row.PropKey].ToString())) - { - var message = string.IsNullOrWhiteSpace(row.PropType.ValidationRegExpMessage) - ? $"'{row.PropType.Name}' is invalid, it does not match the correct pattern" - : row.PropType.ValidationRegExpMessage; - validationResults.Add(new ValidationResult($"Item {(row.RowIndex + 1)}: {message}", new[] { row.PropKey })); - } + elementValidation.AddPropertyTypeValidation( + new PropertyTypeValidationModel(prop.Value.PropertyType, prop.Value.Value)); } + yield return elementValidation; } - - return validationResults; } } internal class BlockEditorValues { - private readonly IBlockEditorDataHelper _dataHelper; private readonly Lazy> _contentTypes; + private readonly BlockEditorDataConverter _dataConverter; - public BlockEditorValues(IBlockEditorDataHelper dataHelper, IContentTypeService contentTypeService) + public BlockEditorValues(BlockEditorDataConverter dataConverter, IContentTypeService contentTypeService) { - _dataHelper = dataHelper; _contentTypes = new Lazy>(() => contentTypeService.GetAll().ToDictionary(c => c.Key)); + _dataConverter = dataConverter; } - private IContentType GetElementType(JObject item) + private IContentType GetElementType(BlockItemData item) { - Guid contentTypeKey = item[ContentTypeKeyPropertyKey]?.ToObject() ?? Guid.Empty; - _contentTypes.Value.TryGetValue(contentTypeKey, out var contentType); + _contentTypes.Value.TryGetValue(item.ContentTypeKey, out var contentType); return contentType; } - public IEnumerable GetPropertyValues(object propertyValue, out List deserialized) + public IReadOnlyList GetPropertyValues(object propertyValue) { - var rowValues = new List(); - - deserialized = null; - if (propertyValue == null || string.IsNullOrWhiteSpace(propertyValue.ToString())) - return Enumerable.Empty(); + return new List(); - var data = JsonConvert.DeserializeObject(propertyValue.ToString()); - if (data?.Layout == null || data.Data == null || data.Data.Count == 0) - return Enumerable.Empty(); + var converted = _dataConverter.Convert(propertyValue.ToString()); - var blockRefs = _dataHelper.GetBlockReferences(data.Layout); - if (blockRefs == null) - return Enumerable.Empty(); + if (converted.Blocks.Count == 0) + return new List(); - var dataMap = new Dictionary(data.Data.Count); - data.Data.ForEach(d => + var contentTypePropertyTypes = new Dictionary>(); + var result = new List(); + + foreach(var block in converted.Blocks) { - var udiObj = d?[UdiPropertyKey]; - if (Udi.TryParse(udiObj == null || udiObj.Type != JTokenType.String ? null : udiObj.ToString(), out var udi)) - dataMap[udi] = d; - }); - - deserialized = blockRefs.Select(r => dataMap.TryGetValue(r.Udi, out var block) ? block : null).Where(r => r != null).ToList(); - if (deserialized == null || deserialized.Count == 0) - return Enumerable.Empty(); - - var index = 0; - - foreach (var o in deserialized) - { - var propValues = o; - - var contentType = GetElementType(propValues); + var contentType = GetElementType(block); if (contentType == null) continue; - var propertyTypes = contentType.CompositionPropertyTypes.ToDictionary(x => x.Alias, x => x); - var propAliases = propValues.Properties().Select(x => x.Name); - foreach (var propAlias in propAliases) + // get the prop types for this content type but keep a dictionary of found ones so we don't have to keep re-looking and re-creating + // objects on each iteration. + if (!contentTypePropertyTypes.TryGetValue(contentType.Alias, out var propertyTypes)) + propertyTypes = contentTypePropertyTypes[contentType.Alias] = contentType.CompositionPropertyTypes.ToDictionary(x => x.Alias, x => x); + + var propValues = new Dictionary(); + + // find any keys that are not real property types and remove them + foreach (var prop in block.RawPropertyValues.ToList()) { - propertyTypes.TryGetValue(propAlias, out var propType); - rowValues.Add(new RowValue(propAlias, propType, propValues, index)); + // doesn't exist so remove it + if (!propertyTypes.TryGetValue(prop.Key, out var propType)) + { + block.RawPropertyValues.Remove(prop.Key); + } + else + { + // set the value to include the resolved property type + propValues[prop.Key] = new BlockPropertyValue + { + PropertyType = propType, + Value = prop.Value + }; + } } - index++; + + result.Add(new BlockValue + { + ContentTypeAlias = contentType.Alias, + PropertyValues = propValues, + Id = ((GuidUdi)block.Udi).Guid + }); } - return rowValues; + return result; } - internal class RowValue + /// + /// Used during deserialization to populate the property value/property type of a nested content row property + /// + internal class BlockPropertyValue { - public RowValue(string propKey, PropertyType propType, JObject propValues, int index) - { - PropKey = propKey ?? throw new ArgumentNullException(nameof(propKey)); - PropType = propType; - JsonRowValue = propValues ?? throw new ArgumentNullException(nameof(propValues)); - RowIndex = index; - } - - /// - /// The current property key being iterated for the row value - /// - public string PropKey { get; } - - /// - /// The of the value (if any), this may be null - /// - public PropertyType PropType { get; } - - /// - /// The json values for the current row - /// - public JObject JsonRowValue { get; } - - /// - /// The Nested Content row index - /// - public int RowIndex { get; } + public object Value { get; set; } + public PropertyType PropertyType { get; set; } } - private class BlockEditorData + /// + /// Used during deserialization to populate the content type alias and property values of a block + /// + internal class BlockValue { - [JsonProperty("layout")] - public JObject Layout { get; set; } - - [JsonProperty("data")] - public List Data { get; set; } + public Guid Id { get; set; } + public string ContentTypeAlias { get; set; } + public IDictionary PropertyValues { get; set; } = new Dictionary(); } + } + #endregion - private static bool IsSystemPropertyKey(string propertyKey) => ContentTypeKeyPropertyKey == propertyKey || UdiPropertyKey == propertyKey; } } diff --git a/src/Umbraco.Web/PropertyEditors/BlockListPropertyEditor.cs b/src/Umbraco.Web/PropertyEditors/BlockListPropertyEditor.cs index 3f8288b0ac..42023382f1 100644 --- a/src/Umbraco.Web/PropertyEditors/BlockListPropertyEditor.cs +++ b/src/Umbraco.Web/PropertyEditors/BlockListPropertyEditor.cs @@ -22,8 +22,8 @@ namespace Umbraco.Web.PropertyEditors Icon = "icon-thumbnail-list")] public class BlockListPropertyEditor : BlockEditorPropertyEditor { - public BlockListPropertyEditor(ILogger logger, Lazy propertyEditors, IDataTypeService dataTypeService, IContentTypeService contentTypeService) - : base(logger, propertyEditors, dataTypeService, contentTypeService, new DataHelper()) + public BlockListPropertyEditor(ILogger logger, Lazy propertyEditors, IDataTypeService dataTypeService, IContentTypeService contentTypeService, ILocalizedTextService localizedTextService) + : base(logger, propertyEditors, dataTypeService, contentTypeService, localizedTextService) { } #region Pre Value Editor @@ -31,36 +31,5 @@ namespace Umbraco.Web.PropertyEditors protected override IConfigurationEditor CreateConfigurationEditor() => new BlockListConfigurationEditor(); #endregion - - #region IBlockEditorDataHelper - - private class DataHelper : IBlockEditorDataHelper - { - public IEnumerable GetBlockReferences(JObject layout) - { - if (!(layout?[Constants.PropertyEditors.Aliases.BlockList] is JArray blLayouts)) - yield break; - - foreach (var blLayout in blLayouts) - { - if (!(blLayout is JObject blockRef) || !(blockRef[UdiPropertyKey] is JValue udiRef) || udiRef.Type != JTokenType.String || !Udi.TryParse(udiRef.ToString(), out var udi)) continue; - yield return new SimpleRef(udi); - } - } - - public bool IsEditorSpecificPropertyKey(string propertyKey) => false; - - private class SimpleRef : IBlockReference - { - public SimpleRef(Udi udi) - { - Udi = udi; - } - - public Udi Udi { get; } - } - } - - #endregion } } diff --git a/src/Umbraco.Web/PropertyEditors/ComplexEditorValidator.cs b/src/Umbraco.Web/PropertyEditors/ComplexEditorValidator.cs new file mode 100644 index 0000000000..365a3fff6b --- /dev/null +++ b/src/Umbraco.Web/PropertyEditors/ComplexEditorValidator.cs @@ -0,0 +1,116 @@ +using System; +using System.Collections.Generic; +using System.ComponentModel.DataAnnotations; +using System.Linq; +using Umbraco.Core; +using Umbraco.Core.Models; +using Umbraco.Core.PropertyEditors; +using Umbraco.Core.Services; +using Umbraco.Web.PropertyEditors.Validation; + +namespace Umbraco.Web.PropertyEditors +{ + /// + /// Used to validate complex editors that contain nested editors + /// + public abstract class ComplexEditorValidator : IValueValidator + { + private readonly PropertyValidationService _propertyValidationService; + + public ComplexEditorValidator(PropertyEditorCollection propertyEditors, IDataTypeService dataTypeService, ILocalizedTextService textService) + { + _propertyValidationService = new PropertyValidationService(propertyEditors, dataTypeService, textService); + } + + /// + /// Return a single for all sub nested validation results in the complex editor + /// + /// + /// + /// + /// + public IEnumerable Validate(object value, string valueType, object dataTypeConfiguration) + { + var elementTypeValues = GetElementTypeValidation(value).ToList(); + var rowResults = GetNestedValidationResults(elementTypeValues).ToList(); + + if (rowResults.Count > 0) + { + var result = new ComplexEditorValidationResult(); + foreach(var rowResult in rowResults) + { + result.ValidationResults.Add(rowResult); + } + return result.Yield(); + } + + return Enumerable.Empty(); + } + + + protected abstract IEnumerable GetElementTypeValidation(object value); + + /// + /// Return a nested validation result per row (Element Type) + /// + /// + /// + protected IEnumerable GetNestedValidationResults(IEnumerable elements) + { + foreach (var row in elements) + { + var elementTypeValidationResult = new ComplexEditorElementTypeValidationResult(row.ElementTypeAlias, row.Id); + + foreach (var prop in row.PropertyTypeValidation) + { + var propValidationResult = new ComplexEditorPropertyTypeValidationResult(prop.PropertyType.Alias); + + foreach (var validationResult in _propertyValidationService.ValidatePropertyValue(prop.PropertyType, prop.PostedValue)) + { + // add the result to the property results + propValidationResult.AddValidationResult(validationResult); + } + + // add the property results to the element type results + if (propValidationResult.ValidationResults.Count > 0) + elementTypeValidationResult.ValidationResults.Add(propValidationResult); + } + + if (elementTypeValidationResult.ValidationResults.Count > 0) + { + yield return elementTypeValidationResult; + } + } + } + + public class PropertyTypeValidationModel + { + public PropertyTypeValidationModel(PropertyType propertyType, object postedValue) + { + PostedValue = postedValue; + PropertyType = propertyType ?? throw new ArgumentNullException(nameof(propertyType)); + } + + public object PostedValue { get; } + public PropertyType PropertyType { get; } + } + + public class ElementTypeValidationModel + { + public ElementTypeValidationModel(string elementTypeAlias, Guid id) + { + ElementTypeAlias = elementTypeAlias; + Id = id; + } + + private List _list = new List(); + + public IEnumerable PropertyTypeValidation => _list; + + public string ElementTypeAlias { get; } + public Guid Id { get; } + + public void AddPropertyTypeValidation(PropertyTypeValidationModel propValidation) => _list.Add(propValidation); + } + } +} diff --git a/src/Umbraco.Web/PropertyEditors/NestedContentPropertyEditor.cs b/src/Umbraco.Web/PropertyEditors/NestedContentPropertyEditor.cs index 564630c574..4767dc19cd 100644 --- a/src/Umbraco.Web/PropertyEditors/NestedContentPropertyEditor.cs +++ b/src/Umbraco.Web/PropertyEditors/NestedContentPropertyEditor.cs @@ -4,6 +4,7 @@ using System.Collections.Generic; using System.ComponentModel.DataAnnotations; using System.Linq; using System.Text.RegularExpressions; +using Microsoft.CodeAnalysis.CSharp.Syntax; using Newtonsoft.Json; using Newtonsoft.Json.Linq; using Umbraco.Core; @@ -13,9 +14,11 @@ using Umbraco.Core.Models; using Umbraco.Core.Models.Editors; using Umbraco.Core.PropertyEditors; using Umbraco.Core.Services; +using Umbraco.Web.PropertyEditors.Validation; namespace Umbraco.Web.PropertyEditors { + /// /// Represents a nested content property editor. /// @@ -31,14 +34,20 @@ namespace Umbraco.Web.PropertyEditors private readonly Lazy _propertyEditors; private readonly IDataTypeService _dataTypeService; private readonly IContentTypeService _contentTypeService; + private readonly ILocalizedTextService _localizedTextService; internal const string ContentTypeAliasPropertyKey = "ncContentTypeAlias"; + [Obsolete("Use the constructor specifying all parameters instead")] public NestedContentPropertyEditor(ILogger logger, Lazy propertyEditors, IDataTypeService dataTypeService, IContentTypeService contentTypeService) + : this(logger, propertyEditors, dataTypeService, contentTypeService, Current.Services.TextService) { } + + public NestedContentPropertyEditor(ILogger logger, Lazy propertyEditors, IDataTypeService dataTypeService, IContentTypeService contentTypeService, ILocalizedTextService localizedTextService) : base (logger) { _propertyEditors = propertyEditors; _dataTypeService = dataTypeService; _contentTypeService = contentTypeService; + _localizedTextService = localizedTextService; } // has to be lazy else circular dep in ctor @@ -52,7 +61,7 @@ namespace Umbraco.Web.PropertyEditors #region Value Editor - protected override IDataValueEditor CreateValueEditor() => new NestedContentPropertyValueEditor(Attribute, PropertyEditors, _dataTypeService, _contentTypeService); + protected override IDataValueEditor CreateValueEditor() => new NestedContentPropertyValueEditor(Attribute, PropertyEditors, _dataTypeService, _contentTypeService, _localizedTextService); internal class NestedContentPropertyValueEditor : DataValueEditor, IDataValueReference { @@ -60,13 +69,13 @@ namespace Umbraco.Web.PropertyEditors private readonly IDataTypeService _dataTypeService; private readonly NestedContentValues _nestedContentValues; - public NestedContentPropertyValueEditor(DataEditorAttribute attribute, PropertyEditorCollection propertyEditors, IDataTypeService dataTypeService, IContentTypeService contentTypeService) + public NestedContentPropertyValueEditor(DataEditorAttribute attribute, PropertyEditorCollection propertyEditors, IDataTypeService dataTypeService, IContentTypeService contentTypeService, ILocalizedTextService textService) : base(attribute) { _propertyEditors = propertyEditors; _dataTypeService = dataTypeService; _nestedContentValues = new NestedContentValues(contentTypeService); - Validators.Add(new NestedContentValidator(propertyEditors, dataTypeService, _nestedContentValues)); + Validators.Add(new NestedContentValidator(_nestedContentValues, propertyEditors, dataTypeService, textService)); } /// @@ -89,41 +98,37 @@ namespace Umbraco.Web.PropertyEditors public override string ConvertDbToString(PropertyType propertyType, object propertyValue, IDataTypeService dataTypeService) { - var vals = _nestedContentValues.GetPropertyValues(propertyValue, out var deserialized).ToList(); + var rows = _nestedContentValues.GetPropertyValues(propertyValue); - if (vals.Count == 0) + if (rows.Count == 0) return string.Empty; - foreach (var row in vals) + foreach (var row in rows.ToList()) { - if (row.PropType == null) - { - // type not found, and property is not system: just delete the value - if (IsSystemPropertyKey(row.PropKey) == false) - row.JsonRowValue[row.PropKey] = null; - } - else + foreach(var prop in row.PropertyValues.ToList()) { try { // convert the value, and store the converted value - var propEditor = _propertyEditors[row.PropType.PropertyEditorAlias]; + var propEditor = _propertyEditors[prop.Value.PropertyType.PropertyEditorAlias]; if (propEditor == null) continue; - var tempConfig = dataTypeService.GetDataType(row.PropType.DataTypeId).Configuration; + var tempConfig = dataTypeService.GetDataType(prop.Value.PropertyType.DataTypeId).Configuration; var valEditor = propEditor.GetValueEditor(tempConfig); - var convValue = valEditor.ConvertDbToString(row.PropType, row.JsonRowValue[row.PropKey]?.ToString(), dataTypeService); - row.JsonRowValue[row.PropKey] = convValue; + var convValue = valEditor.ConvertDbToString(prop.Value.PropertyType, prop.Value.Value, dataTypeService); + + // update the raw value since this is what will get serialized out + row.RawPropertyValues[prop.Key] = convValue; } catch (InvalidOperationException) { // deal with weird situations by ignoring them (no comment) - row.JsonRowValue[row.PropKey] = null; + row.RawPropertyValues.Remove(prop.Key); } } } - return JsonConvert.SerializeObject(deserialized).ToXmlString(); + return JsonConvert.SerializeObject(rows).ToXmlString(); } #endregion @@ -134,98 +139,92 @@ namespace Umbraco.Web.PropertyEditors // note: there is NO variant support here + // TODO: What does this do? public override object ToEditor(Property property, IDataTypeService dataTypeService, string culture = null, string segment = null) { var val = property.GetValue(culture, segment); - var vals = _nestedContentValues.GetPropertyValues(val, out var deserialized).ToList(); + var rows = _nestedContentValues.GetPropertyValues(val); - if (vals.Count == 0) + if (rows.Count == 0) return string.Empty; - foreach (var row in vals) + foreach (var row in rows.ToList()) { - if (row.PropType == null) - { - // type not found, and property is not system: just delete the value - if (IsSystemPropertyKey(row.PropKey) == false) - row.JsonRowValue[row.PropKey] = null; - } - else + foreach(var prop in row.PropertyValues.ToList()) { try { // create a temp property with the value // - force it to be culture invariant as NC can't handle culture variant element properties - row.PropType.Variations = ContentVariation.Nothing; - var tempProp = new Property(row.PropType); - tempProp.SetValue(row.JsonRowValue[row.PropKey] == null ? null : row.JsonRowValue[row.PropKey].ToString()); + prop.Value.PropertyType.Variations = ContentVariation.Nothing; + var tempProp = new Property(prop.Value.PropertyType); + + tempProp.SetValue(prop.Value.Value); // convert that temp property, and store the converted value - var propEditor = _propertyEditors[row.PropType.PropertyEditorAlias]; + var propEditor = _propertyEditors[prop.Value.PropertyType.PropertyEditorAlias]; if (propEditor == null) { - row.JsonRowValue[row.PropKey] = tempProp.GetValue()?.ToString(); + // update the raw value since this is what will get serialized out + row.RawPropertyValues[prop.Key] = tempProp.GetValue()?.ToString(); continue; } - var tempConfig = dataTypeService.GetDataType(row.PropType.DataTypeId).Configuration; + var tempConfig = dataTypeService.GetDataType(prop.Value.PropertyType.DataTypeId).Configuration; var valEditor = propEditor.GetValueEditor(tempConfig); var convValue = valEditor.ToEditor(tempProp, dataTypeService); - row.JsonRowValue[row.PropKey] = convValue == null ? null : JToken.FromObject(convValue); + + // update the raw value since this is what will get serialized out + row.RawPropertyValues[prop.Key] = convValue == null ? null : JToken.FromObject(convValue); } catch (InvalidOperationException) { // deal with weird situations by ignoring them (no comment) - row.JsonRowValue[row.PropKey] = null; + row.RawPropertyValues.Remove(prop.Key); } } } // return json - return deserialized; + return rows; } + // TODO: What does this do? public override object FromEditor(ContentPropertyData editorValue, object currentValue) { if (editorValue.Value == null || string.IsNullOrWhiteSpace(editorValue.Value.ToString())) return null; - var vals = _nestedContentValues.GetPropertyValues(editorValue.Value, out var deserialized).ToList(); + var rows = _nestedContentValues.GetPropertyValues(editorValue.Value); - if (vals.Count == 0) + if (rows.Count == 0) return string.Empty; - foreach (var row in vals) + foreach (var row in rows.ToList()) { - if (row.PropType == null) - { - // type not found, and property is not system: just delete the value - if (IsSystemPropertyKey(row.PropKey) == false) - row.JsonRowValue[row.PropKey] = null; - } - else + foreach(var prop in row.PropertyValues.ToList()) { // Fetch the property types prevalue - var propConfiguration = _dataTypeService.GetDataType(row.PropType.DataTypeId).Configuration; + var propConfiguration = _dataTypeService.GetDataType(prop.Value.PropertyType.DataTypeId).Configuration; // Lookup the property editor - var propEditor = _propertyEditors[row.PropType.PropertyEditorAlias]; + var propEditor = _propertyEditors[prop.Value.PropertyType.PropertyEditorAlias]; if (propEditor == null) continue; // Create a fake content property data object - var contentPropData = new ContentPropertyData(row.JsonRowValue[row.PropKey], propConfiguration); + var contentPropData = new ContentPropertyData(prop.Value.Value, propConfiguration); // Get the property editor to do it's conversion - var newValue = propEditor.GetValueEditor().FromEditor(contentPropData, row.JsonRowValue[row.PropKey]); + var newValue = propEditor.GetValueEditor().FromEditor(contentPropData, prop.Value.Value); - // Store the value back - row.JsonRowValue[row.PropKey] = (newValue == null) ? null : JToken.FromObject(newValue); + // update the raw value since this is what will get serialized out + row.RawPropertyValues[prop.Key] = (newValue == null) ? null : JToken.FromObject(newValue); } } // return json - return JsonConvert.SerializeObject(deserialized); + return JsonConvert.SerializeObject(rows); } #endregion @@ -235,98 +234,58 @@ namespace Umbraco.Web.PropertyEditors var result = new List(); - foreach (var row in _nestedContentValues.GetPropertyValues(rawJson, out _)) + foreach (var row in _nestedContentValues.GetPropertyValues(rawJson)) { - if (row.PropType == null) continue; + foreach(var prop in row.PropertyValues) + { + var propEditor = _propertyEditors[prop.Value.PropertyType.PropertyEditorAlias]; - var propEditor = _propertyEditors[row.PropType.PropertyEditorAlias]; + var valueEditor = propEditor?.GetValueEditor(); + if (!(valueEditor is IDataValueReference reference)) continue; - var valueEditor = propEditor?.GetValueEditor(); - if (!(valueEditor is IDataValueReference reference)) continue; + var val = prop.Value.Value?.ToString(); - var val = row.JsonRowValue[row.PropKey]?.ToString(); + var refs = reference.GetReferences(val); - var refs = reference.GetReferences(val); - - result.AddRange(refs); + result.AddRange(refs); + } } return result; } } - internal class NestedContentValidator : IValueValidator + /// + /// Validator for nested content to ensure that all nesting of editors is validated + /// + internal class NestedContentValidator : ComplexEditorValidator { - private readonly PropertyEditorCollection _propertyEditors; - private readonly IDataTypeService _dataTypeService; private readonly NestedContentValues _nestedContentValues; - public NestedContentValidator(PropertyEditorCollection propertyEditors, IDataTypeService dataTypeService, NestedContentValues nestedContentValues) + public NestedContentValidator(NestedContentValues nestedContentValues, PropertyEditorCollection propertyEditors, IDataTypeService dataTypeService, ILocalizedTextService textService) + : base(propertyEditors, dataTypeService, textService) { - _propertyEditors = propertyEditors; - _dataTypeService = dataTypeService; _nestedContentValues = nestedContentValues; } - public IEnumerable Validate(object rawValue, string valueType, object dataTypeConfiguration) + protected override IEnumerable GetElementTypeValidation(object value) { - var validationResults = new List(); - - foreach(var row in _nestedContentValues.GetPropertyValues(rawValue, out _)) + foreach (var row in _nestedContentValues.GetPropertyValues(value)) { - if (row.PropType == null) continue; - - var config = _dataTypeService.GetDataType(row.PropType.DataTypeId).Configuration; - var propertyEditor = _propertyEditors[row.PropType.PropertyEditorAlias]; - if (propertyEditor == null) continue; - - foreach (var validator in propertyEditor.GetValueEditor().Validators) + var elementValidation = new ElementTypeValidationModel(row.ContentTypeAlias, row.Id); + foreach (var prop in row.PropertyValues) { - foreach (var result in validator.Validate(row.JsonRowValue[row.PropKey], propertyEditor.GetValueEditor().ValueType, config)) - { - result.ErrorMessage = "Item " + (row.RowIndex + 1) + " '" + row.PropType.Name + "' " + result.ErrorMessage; - validationResults.Add(result); - } - } - - // Check mandatory - if (row.PropType.Mandatory) - { - if (row.JsonRowValue[row.PropKey] == null) - { - var message = string.IsNullOrWhiteSpace(row.PropType.MandatoryMessage) - ? $"'{row.PropType.Name}' cannot be null" - : row.PropType.MandatoryMessage; - validationResults.Add(new ValidationResult($"Item {(row.RowIndex + 1)}: {message}", new[] { row.PropKey })); - } - else if (row.JsonRowValue[row.PropKey].ToString().IsNullOrWhiteSpace() || (row.JsonRowValue[row.PropKey].Type == JTokenType.Array && !row.JsonRowValue[row.PropKey].HasValues)) - { - var message = string.IsNullOrWhiteSpace(row.PropType.MandatoryMessage) - ? $"'{row.PropType.Name}' cannot be empty" - : row.PropType.MandatoryMessage; - validationResults.Add(new ValidationResult($"Item {(row.RowIndex + 1)}: {message}", new[] { row.PropKey })); - } - } - - // Check regex - if (!row.PropType.ValidationRegExp.IsNullOrWhiteSpace() - && row.JsonRowValue[row.PropKey] != null && !row.JsonRowValue[row.PropKey].ToString().IsNullOrWhiteSpace()) - { - var regex = new Regex(row.PropType.ValidationRegExp); - if (!regex.IsMatch(row.JsonRowValue[row.PropKey].ToString())) - { - var message = string.IsNullOrWhiteSpace(row.PropType.ValidationRegExpMessage) - ? $"'{row.PropType.Name}' is invalid, it does not match the correct pattern" - : row.PropType.ValidationRegExpMessage; - validationResults.Add(new ValidationResult($"Item {(row.RowIndex + 1)}: {message}", new[] { row.PropKey })); - } + elementValidation.AddPropertyTypeValidation( + new PropertyTypeValidationModel(prop.Value.PropertyType, prop.Value.Value)); } + yield return elementValidation; } - - return validationResults; } } + /// + /// Used to deserialize the nested content serialized value + /// internal class NestedContentValues { private readonly Lazy> _contentTypes; @@ -336,84 +295,110 @@ namespace Umbraco.Web.PropertyEditors _contentTypes = new Lazy>(() => contentTypeService.GetAll().ToDictionary(c => c.Alias)); } - private IContentType GetElementType(JObject item) + private IContentType GetElementType(NestedContentRowValue item) { - var contentTypeAlias = item[ContentTypeAliasPropertyKey]?.ToObject() ?? string.Empty; - _contentTypes.Value.TryGetValue(contentTypeAlias, out var contentType); + _contentTypes.Value.TryGetValue(item.ContentTypeAlias, out var contentType); return contentType; } - public IEnumerable GetPropertyValues(object propertyValue, out List deserialized) + /// + /// Deserialize the raw json property value + /// + /// + /// + public IReadOnlyList GetPropertyValues(object propertyValue) { - var rowValues = new List(); - - deserialized = null; - if (propertyValue == null || string.IsNullOrWhiteSpace(propertyValue.ToString())) - return Enumerable.Empty(); + return new List(); - deserialized = JsonConvert.DeserializeObject>(propertyValue.ToString()); + var rowValues = JsonConvert.DeserializeObject>(propertyValue.ToString()); // There was a note here about checking if the result had zero items and if so it would return null, so we'll continue to do that // The original note was: "Issue #38 - Keep recursive property lookups working" // Which is from the original NC tracker: https://github.com/umco/umbraco-nested-content/issues/38 // This check should be used everywhere when iterating NC prop values, instead of just the one previous place so that // empty values don't get persisted when there is nothing, it should actually be null. - if (deserialized == null || deserialized.Count == 0) - return Enumerable.Empty(); + if (rowValues == null || rowValues.Count == 0) + return new List(); - var index = 0; + var contentTypePropertyTypes = new Dictionary>(); - foreach (var o in deserialized) + foreach (var row in rowValues) { - var propValues = o; - - var contentType = GetElementType(propValues); + var contentType = GetElementType(row); if (contentType == null) continue; - var propertyTypes = contentType.CompositionPropertyTypes.ToDictionary(x => x.Alias, x => x); - var propAliases = propValues.Properties().Select(x => x.Name); - foreach (var propAlias in propAliases) + // get the prop types for this content type but keep a dictionary of found ones so we don't have to keep re-looking and re-creating + // objects on each iteration. + if (!contentTypePropertyTypes.TryGetValue(contentType.Alias, out var propertyTypes)) + propertyTypes = contentTypePropertyTypes[contentType.Alias] = contentType.CompositionPropertyTypes.ToDictionary(x => x.Alias, x => x); + + // find any keys that are not real property types and remove them + foreach(var prop in row.RawPropertyValues.ToList()) { - propertyTypes.TryGetValue(propAlias, out var propType); - rowValues.Add(new RowValue(propAlias, propType, propValues, index)); + if (IsSystemPropertyKey(prop.Key)) continue; + + // doesn't exist so remove it + if (!propertyTypes.TryGetValue(prop.Key, out var propType)) + { + row.RawPropertyValues.Remove(prop.Key); + } + else + { + // set the value to include the resolved property type + row.PropertyValues[prop.Key] = new NestedContentPropertyValue + { + PropertyType = propType, + Value = prop.Value + }; + } } - index++; } return rowValues; } - internal class RowValue + /// + /// Used during deserialization to populate the property value/property type of a nested content row property + /// + internal class NestedContentPropertyValue { - public RowValue(string propKey, PropertyType propType, JObject propValues, int index) - { - PropKey = propKey ?? throw new ArgumentNullException(nameof(propKey)); - PropType = propType; - JsonRowValue = propValues ?? throw new ArgumentNullException(nameof(propValues)); - RowIndex = index; - } + public object Value { get; set; } + public PropertyType PropertyType { get; set; } + } + + /// + /// Used to deserialize a nested content row + /// + internal class NestedContentRowValue + { + [JsonProperty("key")] + public Guid Id{ get; set; } + + [JsonProperty("name")] + public string Name { get; set; } + + [JsonProperty("ncContentTypeAlias")] + public string ContentTypeAlias { get; set; } /// - /// The current property key being iterated for the row value + /// The remaining properties will be serialized to a dictionary /// - public string PropKey { get; } + /// + /// The JsonExtensionDataAttribute is used to put the non-typed properties into a bucket + /// http://www.newtonsoft.com/json/help/html/DeserializeExtensionData.htm + /// NestedContent serializes to string, int, whatever eg + /// "stringValue":"Some String","numericValue":125,"otherNumeric":null + /// + [JsonExtensionData] + public IDictionary RawPropertyValues { get; set; } /// - /// The of the value (if any), this may be null + /// Used during deserialization to convert the raw property data into data with a property type context /// - public PropertyType PropType { get; } - - /// - /// The json values for the current row - /// - public JObject JsonRowValue { get; } - - /// - /// The Nested Content row index - /// - public int RowIndex { get; } + [JsonIgnore] + public IDictionary PropertyValues { get; set; } = new Dictionary(); } } diff --git a/src/Umbraco.Web/PropertyEditors/Validation/ComplexEditorElementTypeValidationResult.cs b/src/Umbraco.Web/PropertyEditors/Validation/ComplexEditorElementTypeValidationResult.cs new file mode 100644 index 0000000000..17a1433db5 --- /dev/null +++ b/src/Umbraco.Web/PropertyEditors/Validation/ComplexEditorElementTypeValidationResult.cs @@ -0,0 +1,37 @@ +using System; +using System.Collections.Generic; +using System.ComponentModel.DataAnnotations; + +namespace Umbraco.Web.PropertyEditors.Validation +{ + /// + /// A collection of for an element type within complex editor represented by an Element Type + /// + public class ComplexEditorElementTypeValidationResult : ValidationResult + { + public ComplexEditorElementTypeValidationResult(string elementTypeAlias, Guid blockId) + : base(string.Empty) + { + ElementTypeAlias = elementTypeAlias; + BlockId = blockId; + } + + public IList ValidationResults { get; } = new List(); + + /// + /// The element type alias of the validation result + /// + /// + /// This is useful for debugging purposes but it's not actively used in the angular app + /// + public string ElementTypeAlias { get; } + + /// + /// The Block ID of the validation result + /// + /// + /// This is the GUID id of the content item based on the element type + /// + public Guid BlockId { get; } + } +} diff --git a/src/Umbraco.Web/PropertyEditors/Validation/ComplexEditorPropertyTypeValidationResult.cs b/src/Umbraco.Web/PropertyEditors/Validation/ComplexEditorPropertyTypeValidationResult.cs new file mode 100644 index 0000000000..1872648f54 --- /dev/null +++ b/src/Umbraco.Web/PropertyEditors/Validation/ComplexEditorPropertyTypeValidationResult.cs @@ -0,0 +1,32 @@ +using System; +using System.Collections.Generic; +using System.ComponentModel.DataAnnotations; +using System.Linq; + +namespace Umbraco.Web.PropertyEditors.Validation +{ + /// + /// A collection of for a property type within a complex editor represented by an Element Type + /// + public class ComplexEditorPropertyTypeValidationResult : ValidationResult + { + public ComplexEditorPropertyTypeValidationResult(string propertyTypeAlias) + : base(string.Empty) + { + PropertyTypeAlias = propertyTypeAlias; + } + + private readonly List _validationResults = new List(); + + public void AddValidationResult(ValidationResult validationResult) + { + if (validationResult is ComplexEditorValidationResult && _validationResults.Any(x => x is ComplexEditorValidationResult)) + throw new InvalidOperationException($"Cannot add more than one {typeof(ComplexEditorValidationResult)}"); + + _validationResults.Add(validationResult); + } + + public IReadOnlyList ValidationResults => _validationResults; + public string PropertyTypeAlias { get; } + } +} diff --git a/src/Umbraco.Web/PropertyEditors/Validation/ComplexEditorValidationResult.cs b/src/Umbraco.Web/PropertyEditors/Validation/ComplexEditorValidationResult.cs new file mode 100644 index 0000000000..eb1efbc64f --- /dev/null +++ b/src/Umbraco.Web/PropertyEditors/Validation/ComplexEditorValidationResult.cs @@ -0,0 +1,22 @@ +using System.Collections.Generic; +using System.ComponentModel.DataAnnotations; + +namespace Umbraco.Web.PropertyEditors.Validation +{ + + /// + /// A collection of for a complex editor represented by an Element Type + /// + /// + /// For example, each represents validation results for a row in Nested Content + /// + public class ComplexEditorValidationResult : ValidationResult + { + public ComplexEditorValidationResult() + : base(string.Empty) + { + } + + public IList ValidationResults { get; } = new List(); + } +} diff --git a/src/Umbraco.Web/PropertyEditors/Validation/ContentPropertyValidationResult.cs b/src/Umbraco.Web/PropertyEditors/Validation/ContentPropertyValidationResult.cs new file mode 100644 index 0000000000..e161e277ac --- /dev/null +++ b/src/Umbraco.Web/PropertyEditors/Validation/ContentPropertyValidationResult.cs @@ -0,0 +1,47 @@ +using Newtonsoft.Json; +using System.ComponentModel.DataAnnotations; + +namespace Umbraco.Web.PropertyEditors.Validation +{ + /// + /// Custom for content properties + /// + /// + /// This clones the original result and then ensures the nested result if it's the correct type + /// + public class ContentPropertyValidationResult : ValidationResult + { + private readonly string _culture; + private readonly string _segment; + + public ContentPropertyValidationResult(ValidationResult nested, string culture, string segment) + : base(nested.ErrorMessage, nested.MemberNames) + { + ComplexEditorResults = nested as ComplexEditorValidationResult; + _culture = culture; + _segment = segment; + } + + /// + /// Nested validation results for the content property + /// + /// + /// There can be nested results for complex editors that contain other editors + /// + public ComplexEditorValidationResult ComplexEditorResults { get; } + + /// + /// Return the if is null, else the serialized + /// complex validation results + /// + /// + public override string ToString() + { + if (ComplexEditorResults == null) + return base.ToString(); + + var json = JsonConvert.SerializeObject(this, new ValidationResultConverter(_culture, _segment)); + return json; + } + } +} diff --git a/src/Umbraco.Web/PropertyEditors/Validation/ValidationResultConverter.cs b/src/Umbraco.Web/PropertyEditors/Validation/ValidationResultConverter.cs new file mode 100644 index 0000000000..85f2c5f81e --- /dev/null +++ b/src/Umbraco.Web/PropertyEditors/Validation/ValidationResultConverter.cs @@ -0,0 +1,152 @@ +using Newtonsoft.Json; +using Newtonsoft.Json.Linq; +using Newtonsoft.Json.Serialization; +using System; +using System.ComponentModel.DataAnnotations; +using System.Linq; +using System.Web.Http.ModelBinding; +using Umbraco.Core; + +namespace Umbraco.Web.PropertyEditors.Validation +{ + /// + /// Custom json converter for and + /// + /// + /// This converter is specifically used to convert validation results for content in order to be able to have nested + /// validation results for complex editors. + /// + internal class ValidationResultConverter : JsonConverter + { + private readonly string _culture; + private readonly string _segment; + + /// + /// Constructor + /// + /// The culture of the containing property which will be transfered to all child model state + /// The segment of the containing property which will be transfered to all child model state + public ValidationResultConverter(string culture = "", string segment = "") + { + _culture = culture; + _segment = segment; + } + + public override bool CanConvert(Type objectType) => typeof(ValidationResult).IsAssignableFrom(objectType); + + public override object ReadJson(JsonReader reader, Type objectType, object existingValue, JsonSerializer serializer) + { + throw new NotImplementedException(); + } + + public override void WriteJson(JsonWriter writer, object value, JsonSerializer serializer) + { + var camelCaseSerializer = new JsonSerializer + { + ContractResolver = new CamelCasePropertyNamesContractResolver(), + DefaultValueHandling = DefaultValueHandling.Ignore + }; + foreach (var c in serializer.Converters) + camelCaseSerializer.Converters.Add(c); + + var validationResult = (ValidationResult)value; + + if (validationResult is ComplexEditorValidationResult nestedResult && nestedResult.ValidationResults.Count > 0) + { + var ja = new JArray(); + foreach(var nested in nestedResult.ValidationResults) + { + // recurse to write out the ComplexEditorElementTypeValidationResult + var block = JObject.FromObject(nested, camelCaseSerializer); + ja.Add(block); + } + if (nestedResult.ValidationResults.Count > 0) + { + ja.WriteTo(writer); + } + } + else if (validationResult is ComplexEditorElementTypeValidationResult elementTypeValidationResult && elementTypeValidationResult.ValidationResults.Count > 0) + { + var joElementType = new JObject + { + { "$id", elementTypeValidationResult.BlockId }, + + // We don't use this anywhere, though it's nice for debugging + { "$elementTypeAlias", elementTypeValidationResult.ElementTypeAlias } + }; + + var modelState = new ModelStateDictionary(); + + // loop over property validations + foreach (var propTypeResult in elementTypeValidationResult.ValidationResults) + { + // group the results by their type and iterate the groups + foreach (var result in propTypeResult.ValidationResults.GroupBy(x => x.GetType())) + { + // if the group's type isn't ComplexEditorValidationResult then it will in 99% of cases be + // just ValidationResult for whcih we want to create the sub "ModelState" data. If it's not a normal + // ValidationResult it will still just be converted to normal ModelState. + + if (result.Key == typeof(ComplexEditorValidationResult)) + { + // if it's ComplexEditorValidationResult then there can only be one which is validated so just get the single + if (result.Any()) + { + var complexResult = result.Single(); + // recurse to get the validation result object + var obj = JToken.FromObject(complexResult, camelCaseSerializer); + joElementType.Add(propTypeResult.PropertyTypeAlias, obj); + + // For any nested property error we add the model state as empty state for that nested property + // NOTE: Instead of the empty validation message we could put in the translated + // "errors/propertyHasErrors" message, however I think that leaves for less flexibility since it could/should be + // up to the front-end validator to show whatever message it wants (if any) for an error indicating a nested property error. + // Will leave blank. + modelState.AddPropertyValidationError(new ValidationResult(string.Empty), propTypeResult.PropertyTypeAlias, _culture, _segment); + } + } + else + { + foreach (var v in result) + { + modelState.AddPropertyValidationError(v, propTypeResult.PropertyTypeAlias, _culture, _segment); + } + } + } + } + + if (modelState.Count > 0) + { + joElementType.Add("ModelState", JObject.FromObject(modelState.ToErrorDictionary())); + } + + joElementType.WriteTo(writer); + } + else + { + + if (validationResult is ContentPropertyValidationResult propertyValidationResult + && propertyValidationResult.ComplexEditorResults?.ValidationResults.Count > 0) + { + // recurse to write out the NestedValidationResults + var obj = JToken.FromObject(propertyValidationResult.ComplexEditorResults, camelCaseSerializer); + obj.WriteTo(writer); + } + + var jo = new JObject(); + if (!validationResult.ErrorMessage.IsNullOrWhiteSpace()) + { + var errObj = JToken.FromObject(validationResult.ErrorMessage, camelCaseSerializer); + jo.Add("errorMessage", errObj); + } + if (validationResult.MemberNames.Any()) + { + var memberNamesObj = JToken.FromObject(validationResult.MemberNames, camelCaseSerializer); + jo.Add("memberNames", memberNamesObj); + } + if (jo.HasValues) + jo.WriteTo(writer); + } + } + } +} diff --git a/src/Umbraco.Web/PropertyEditors/ValueConverters/BlockEditorConverter.cs b/src/Umbraco.Web/PropertyEditors/ValueConverters/BlockEditorConverter.cs index 917462e2f2..83c612e9f7 100644 --- a/src/Umbraco.Web/PropertyEditors/ValueConverters/BlockEditorConverter.cs +++ b/src/Umbraco.Web/PropertyEditors/ValueConverters/BlockEditorConverter.cs @@ -2,12 +2,16 @@ using System; using System.Collections.Generic; using Umbraco.Core; +using Umbraco.Core.Models.Blocks; using Umbraco.Core.Models.PublishedContent; using Umbraco.Core.PropertyEditors; using Umbraco.Web.PublishedCache; namespace Umbraco.Web.PropertyEditors.ValueConverters { + /// + /// Converts json block objects into + /// public sealed class BlockEditorConverter { private readonly IPublishedSnapshotAccessor _publishedSnapshotAccessor; @@ -20,36 +24,25 @@ namespace Umbraco.Web.PropertyEditors.ValueConverters } public IPublishedElement ConvertToElement( - JObject sourceObject, string contentTypeKeyPropertyKey, + BlockEditorData.BlockItemData data, PropertyCacheLevel referenceCacheLevel, bool preview) { - var elementTypeKey = sourceObject[contentTypeKeyPropertyKey]?.ToObject(); - if (!elementTypeKey.HasValue) - return null; - - // hack! we need to cast, we have n ochoice beacuse we cannot make breaking changes. + // hack! we need to cast, we have no choice beacuse we cannot make breaking changes. var publishedContentCache = _publishedSnapshotAccessor.PublishedSnapshot.Content as IPublishedContentCache2; if (publishedContentCache == null) throw new InvalidOperationException("The published content cache is not " + typeof(IPublishedContentCache2)); // only convert element types - content types will cause an exception when PublishedModelFactory creates the model - var publishedContentType = publishedContentCache.GetContentType(elementTypeKey.Value); + var publishedContentType = publishedContentCache.GetContentType(data.ContentTypeKey); if (publishedContentType == null || publishedContentType.IsElement == false) return null; - var propertyValues = sourceObject.ToObject>(); + var propertyValues = data.RawPropertyValues; - if (!propertyValues.TryGetValue("key", out var keyo) || !Guid.TryParse(keyo.ToString(), out var key)) - { - if (propertyValues.TryGetValue("udi", out var udio) && udio is string udis && GuidUdi.TryParse(udis, out var udi)) - { - key = udi.Guid; - } - else - { - key = Guid.Empty; - } - } + // Get the udi from the deserialized object. If this is empty we can fallback to checking the 'key' if there is one + var key = (data.Udi is GuidUdi gudi) ? gudi.Guid : Guid.Empty; + if (propertyValues.TryGetValue("key", out var keyo)) + Guid.TryParse(keyo.ToString(), out key); IPublishedElement element = new PublishedElement(publishedContentType, key, propertyValues, preview, referenceCacheLevel, _publishedSnapshotAccessor); element = _publishedModelFactory.CreateModel(element); diff --git a/src/Umbraco.Web/PropertyEditors/ValueConverters/BlockListPropertyValueConverter.cs b/src/Umbraco.Web/PropertyEditors/ValueConverters/BlockListPropertyValueConverter.cs index a833efa2e5..a72ed4385b 100644 --- a/src/Umbraco.Web/PropertyEditors/ValueConverters/BlockListPropertyValueConverter.cs +++ b/src/Umbraco.Web/PropertyEditors/ValueConverters/BlockListPropertyValueConverter.cs @@ -61,22 +61,16 @@ namespace Umbraco.Web.PropertyEditors.ValueConverters var value = (string)inter; if (string.IsNullOrWhiteSpace(value)) return model; - var objects = JsonConvert.DeserializeObject(value); - if (objects.Count == 0) return model; + var converter = new BlockListEditorDataConverter(); + var converted = converter.Convert(value); + if (converted.Blocks.Count == 0) return model; - var jsonLayout = objects["layout"] as JObject; - if (jsonLayout == null) return model; - - var jsonData = objects["data"] as JArray; - if (jsonData == null) return model; - - var blockListLayouts = jsonLayout[Constants.PropertyEditors.Aliases.BlockList] as JArray; - if (blockListLayouts == null) return model; + var blockListLayout = converted.Layout.ToObject>(); // parse the data elements - foreach (var data in jsonData.Cast()) + foreach (var data in converted.Blocks) { - var element = _blockConverter.ConvertToElement(data, BlockEditorPropertyEditor.ContentTypeKeyPropertyKey, referenceCacheLevel, preview); + var element = _blockConverter.ConvertToElement(data, referenceCacheLevel, preview); if (element == null) continue; elements[element.Key] = element; } @@ -84,14 +78,12 @@ namespace Umbraco.Web.PropertyEditors.ValueConverters // if there's no elements just return since if there's no data it doesn't matter what is stored in layout if (elements.Count == 0) return model; - foreach (var blockListLayout in blockListLayouts) + foreach (var layoutItem in blockListLayout) { - var settingsJson = blockListLayout["settings"] as JObject; // the result of this can be null, that's ok - var element = settingsJson != null ? _blockConverter.ConvertToElement(settingsJson, BlockEditorPropertyEditor.ContentTypeKeyPropertyKey, referenceCacheLevel, preview) : null; + var element = layoutItem.Settings != null ? _blockConverter.ConvertToElement(layoutItem.Settings, referenceCacheLevel, preview) : null; - if (!Udi.TryParse(blockListLayout.Value("udi"), out var udi) || !(udi is GuidUdi guidUdi)) - continue; + var guidUdi = (GuidUdi)layoutItem.Udi; // get the data reference if (!elements.TryGetValue(guidUdi.Guid, out var data)) @@ -107,7 +99,7 @@ namespace Umbraco.Web.PropertyEditors.ValueConverters if (element != null && string.IsNullOrWhiteSpace(blockConfig.SettingsElementTypeKey)) element = null; - var layoutRef = new BlockListLayoutReference(udi, data, element); + var layoutRef = new BlockListLayoutReference(layoutItem.Udi, data, element); layout.Add(layoutRef); } diff --git a/src/Umbraco.Web/PublishedCache/NuCache/DataSource/DatabaseDataSource.cs b/src/Umbraco.Web/PublishedCache/NuCache/DataSource/DatabaseDataSource.cs index 694dac04df..f62014a368 100644 --- a/src/Umbraco.Web/PublishedCache/NuCache/DataSource/DatabaseDataSource.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/DataSource/DatabaseDataSource.cs @@ -208,7 +208,7 @@ namespace Umbraco.Web.PublishedCache.NuCache.DataSource if (dto.EditData == null) { if (Debugger.IsAttached) - throw new Exception("Missing cmsContentNu edited content for node " + dto.Id + ", consider rebuilding."); + throw new InvalidOperationException("Missing cmsContentNu edited content for node " + dto.Id + ", consider rebuilding."); Current.Logger.Warn("Missing cmsContentNu edited content for node {NodeId}, consider rebuilding.", dto.Id); } else @@ -235,7 +235,7 @@ namespace Umbraco.Web.PublishedCache.NuCache.DataSource if (dto.PubData == null) { if (Debugger.IsAttached) - throw new Exception("Missing cmsContentNu published content for node " + dto.Id + ", consider rebuilding."); + throw new InvalidOperationException("Missing cmsContentNu published content for node " + dto.Id + ", consider rebuilding."); Current.Logger.Warn("Missing cmsContentNu published content for node {NodeId}, consider rebuilding.", dto.Id); } else @@ -274,7 +274,7 @@ namespace Umbraco.Web.PublishedCache.NuCache.DataSource private static ContentNodeKit CreateMediaNodeKit(ContentSourceDto dto) { if (dto.EditData == null) - throw new Exception("No data for media " + dto.Id); + throw new InvalidOperationException("No data for media " + dto.Id); var nested = DeserializeNestedData(dto.EditData); diff --git a/src/Umbraco.Web/Scheduling/HealthCheckNotifier.cs b/src/Umbraco.Web/Scheduling/HealthCheckNotifier.cs index 746bc61e34..9a4a4f0e2c 100644 --- a/src/Umbraco.Web/Scheduling/HealthCheckNotifier.cs +++ b/src/Umbraco.Web/Scheduling/HealthCheckNotifier.cs @@ -5,6 +5,7 @@ using Umbraco.Core; using Umbraco.Core.Composing; using Umbraco.Core.Configuration; using Umbraco.Core.Logging; +using Umbraco.Core.Scoping; using Umbraco.Core.Sync; using Umbraco.Web.HealthCheck; @@ -15,16 +16,17 @@ namespace Umbraco.Web.Scheduling private readonly IRuntimeState _runtimeState; private readonly HealthCheckCollection _healthChecks; private readonly HealthCheckNotificationMethodCollection _notifications; + private readonly IScopeProvider _scopeProvider; private readonly IProfilingLogger _logger; public HealthCheckNotifier(IBackgroundTaskRunner runner, int delayMilliseconds, int periodMilliseconds, HealthCheckCollection healthChecks, HealthCheckNotificationMethodCollection notifications, - IRuntimeState runtimeState, - IProfilingLogger logger) + IScopeProvider scopeProvider, IRuntimeState runtimeState, IProfilingLogger logger) : base(runner, delayMilliseconds, periodMilliseconds) { _healthChecks = healthChecks; _notifications = notifications; + _scopeProvider = scopeProvider; _runtimeState = runtimeState; _logger = logger; } @@ -51,6 +53,10 @@ namespace Umbraco.Web.Scheduling return false; // do NOT repeat, going down } + // Ensure we use an explicit scope since we are running on a background thread and plugin health + // checks can be making service/database calls so we want to ensure the CallContext/Ambient scope + // isn't used since that can be problematic. + using (var scope = _scopeProvider.CreateScope()) using (_logger.DebugDuration("Health checks executing", "Health checks complete")) { var healthCheckConfig = Current.Configs.HealthChecks(); diff --git a/src/Umbraco.Web/Scheduling/LogScrubber.cs b/src/Umbraco.Web/Scheduling/LogScrubber.cs index db13a80f9b..ffdb584c7a 100644 --- a/src/Umbraco.Web/Scheduling/LogScrubber.cs +++ b/src/Umbraco.Web/Scheduling/LogScrubber.cs @@ -70,8 +70,7 @@ namespace Umbraco.Web.Scheduling return false; // do NOT repeat, going down } - // running on a background task, and Log.CleanLogs uses the old SqlHelper, - // better wrap in a scope and ensure it's all cleaned up and nothing leaks + // Ensure we use an explicit scope since we are running on a background thread. using (var scope = _scopeProvider.CreateScope()) using (_logger.DebugDuration("Log scrubbing executing", "Log scrubbing complete")) { diff --git a/src/Umbraco.Web/Scheduling/ScheduledPublishing.cs b/src/Umbraco.Web/Scheduling/ScheduledPublishing.cs index 2e79e40d7a..97afe25e22 100644 --- a/src/Umbraco.Web/Scheduling/ScheduledPublishing.cs +++ b/src/Umbraco.Web/Scheduling/ScheduledPublishing.cs @@ -2,6 +2,7 @@ using System.Linq; using Umbraco.Core; using Umbraco.Core.Logging; +using Umbraco.Core.Scoping; using Umbraco.Core.Services; using Umbraco.Core.Sync; @@ -55,30 +56,24 @@ namespace Umbraco.Web.Scheduling try { - // ensure we run with an UmbracoContext, because this may run in a background task, - // yet developers may be using the 'current' UmbracoContext in the event handlers - // - // TODO: or maybe not, CacheRefresherComponent already ensures a context when handling events - // - UmbracoContext 'current' needs to be refactored and cleaned up - // - batched messenger should not depend on a current HttpContext - // but then what should be its "scope"? could we attach it to scopes? - // - and we should definitively *not* have to flush it here (should be auto) - // - using (var contextReference = _umbracoContextFactory.EnsureUmbracoContext()) + // We don't need an explicit scope here because PerformScheduledPublish creates it's own scope + // so it's safe as it will create it's own ambient scope. + // Ensure we run with an UmbracoContext, because this will run in a background task, + // and developers may be using the UmbracoContext in the event handlers. + + using var contextReference = _umbracoContextFactory.EnsureUmbracoContext(); + try { - try - { - // run - var result = _contentService.PerformScheduledPublish(DateTime.Now); - foreach (var grouped in result.GroupBy(x => x.Result)) - _logger.Info("Scheduled publishing result: '{StatusCount}' items with status {Status}", grouped.Count(), grouped.Key); - } - finally - { - // if running on a temp context, we have to flush the messenger - if (contextReference.IsRoot && Composing.Current.ServerMessenger is BatchedDatabaseServerMessenger m) - m.FlushBatch(); - } + // run + var result = _contentService.PerformScheduledPublish(DateTime.Now); + foreach (var grouped in result.GroupBy(x => x.Result)) + _logger.Info("Scheduled publishing result: '{StatusCount}' items with status {Status}", grouped.Count(), grouped.Key); + } + finally + { + // if running on a temp context, we have to flush the messenger + if (contextReference.IsRoot && Composing.Current.ServerMessenger is BatchedDatabaseServerMessenger m) + m.FlushBatch(); } } catch (Exception ex) diff --git a/src/Umbraco.Web/Scheduling/SchedulerComponent.cs b/src/Umbraco.Web/Scheduling/SchedulerComponent.cs index a08289186f..f6ce11f939 100644 --- a/src/Umbraco.Web/Scheduling/SchedulerComponent.cs +++ b/src/Umbraco.Web/Scheduling/SchedulerComponent.cs @@ -155,7 +155,7 @@ namespace Umbraco.Web.Scheduling } var periodInMilliseconds = healthCheckConfig.NotificationSettings.PeriodInHours * 60 * 60 * 1000; - var task = new HealthCheckNotifier(_healthCheckRunner, delayInMilliseconds, periodInMilliseconds, healthChecks, notifications, _runtime, logger); + var task = new HealthCheckNotifier(_healthCheckRunner, delayInMilliseconds, periodInMilliseconds, healthChecks, notifications, _scopeProvider, _runtime, logger); _healthCheckRunner.TryAdd(task); return task; } diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index 24a2c98fc8..a0ffacf8f3 100644 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -249,8 +249,14 @@ + + + + + +