From 99d9c49c9ff02f9176bd73cda2dc6b9bb614d0a0 Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 19 Nov 2018 17:54:36 +1100 Subject: [PATCH] Gets sln building and manually merges the entity/media service changes --- .../Alter/Table/AlterTableBuilder.cs | 1 + .../Create/Column/CreateColumnBuilder.cs | 1 + .../Create/Table/CreateTableBuilder.cs | 1 + .../Factories/ContentBaseFactory.cs | 3 ++ .../Implement/EntityRepository.cs | 44 ++++++++++++++----- src/Umbraco.Core/Services/IMediaService.cs | 13 +----- .../Services/Implement/EntityService.cs | 17 +++++++ .../Services/Implement/MediaService.cs | 19 +++++--- .../Editors/ContentTypeController.cs | 2 +- src/Umbraco.Web/Editors/MediaController.cs | 2 +- src/Umbraco.Web/Editors/PasswordChanger.cs | 2 +- .../Trees/ContentTreeController.cs | 2 +- .../Trees/ContentTreeControllerBase.cs | 9 ++-- src/Umbraco.Web/Trees/MediaTreeController.cs | 3 +- .../RelationTypes/EditRelationType.aspx.cs | 2 +- 15 files changed, 80 insertions(+), 41 deletions(-) diff --git a/src/Umbraco.Core/Migrations/Expressions/Alter/Table/AlterTableBuilder.cs b/src/Umbraco.Core/Migrations/Expressions/Alter/Table/AlterTableBuilder.cs index f5bef4144c..eee9826a85 100644 --- a/src/Umbraco.Core/Migrations/Expressions/Alter/Table/AlterTableBuilder.cs +++ b/src/Umbraco.Core/Migrations/Expressions/Alter/Table/AlterTableBuilder.cs @@ -4,6 +4,7 @@ using Umbraco.Core.Migrations.Expressions.Alter.Expressions; using Umbraco.Core.Migrations.Expressions.Common.Expressions; using Umbraco.Core.Migrations.Expressions.Create.Expressions; using Umbraco.Core.Persistence; +using Umbraco.Core.Persistence.DatabaseAnnotations; using Umbraco.Core.Persistence.DatabaseModelDefinitions; namespace Umbraco.Core.Migrations.Expressions.Alter.Table diff --git a/src/Umbraco.Core/Migrations/Expressions/Create/Column/CreateColumnBuilder.cs b/src/Umbraco.Core/Migrations/Expressions/Create/Column/CreateColumnBuilder.cs index 3c59c5ffd7..656aedcea0 100644 --- a/src/Umbraco.Core/Migrations/Expressions/Create/Column/CreateColumnBuilder.cs +++ b/src/Umbraco.Core/Migrations/Expressions/Create/Column/CreateColumnBuilder.cs @@ -1,6 +1,7 @@ using System.Data; using NPoco; using Umbraco.Core.Migrations.Expressions.Common.Expressions; +using Umbraco.Core.Persistence.DatabaseAnnotations; using Umbraco.Core.Persistence.DatabaseModelDefinitions; namespace Umbraco.Core.Migrations.Expressions.Create.Column diff --git a/src/Umbraco.Core/Migrations/Expressions/Create/Table/CreateTableBuilder.cs b/src/Umbraco.Core/Migrations/Expressions/Create/Table/CreateTableBuilder.cs index 8f7eda89c2..f765a58169 100644 --- a/src/Umbraco.Core/Migrations/Expressions/Create/Table/CreateTableBuilder.cs +++ b/src/Umbraco.Core/Migrations/Expressions/Create/Table/CreateTableBuilder.cs @@ -3,6 +3,7 @@ using NPoco; using Umbraco.Core.Migrations.Expressions.Common.Expressions; using Umbraco.Core.Migrations.Expressions.Create.Expressions; using Umbraco.Core.Persistence; +using Umbraco.Core.Persistence.DatabaseAnnotations; using Umbraco.Core.Persistence.DatabaseModelDefinitions; namespace Umbraco.Core.Migrations.Expressions.Create.Table diff --git a/src/Umbraco.Core/Persistence/Factories/ContentBaseFactory.cs b/src/Umbraco.Core/Persistence/Factories/ContentBaseFactory.cs index ec364c7c6a..472b2c64f0 100644 --- a/src/Umbraco.Core/Persistence/Factories/ContentBaseFactory.cs +++ b/src/Umbraco.Core/Persistence/Factories/ContentBaseFactory.cs @@ -302,6 +302,9 @@ namespace Umbraco.Core.Persistence.Factories // more dark magic ;-( internal static bool TryMatch(string text, out string path) { + //fixme: In v8 we should allow exposing this via the property editor in a much nicer way so that the property editor + // can tell us directly what any URL is for a given property if it contains an asset + path = null; if (string.IsNullOrWhiteSpace(text)) return false; diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/EntityRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/EntityRepository.cs index fb8c2732e6..8d6f67e9db 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/EntityRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/EntityRepository.cs @@ -66,6 +66,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement if (isContent) BuildVariants(entities.Cast()); + //fixme - see https://github.com/umbraco/Umbraco-CMS/pull/3460#issuecomment-434903930 we need to not load any property data at all for media if (isMedia) BuildProperties(entities, dtos); @@ -110,14 +111,14 @@ namespace Umbraco.Core.Persistence.Repositories.Implement return GetEntity(sql, isContent, isMedia); } - public virtual IEntitySlim Get(int id) + public IEntitySlim Get(int id) { var sql = GetBaseWhere(false, false, false, id); var dto = Database.FirstOrDefault(sql); return dto == null ? null : BuildEntity(false, false, dto); } - public virtual IEntitySlim Get(int id, Guid objectTypeId) + public IEntitySlim Get(int id, Guid objectTypeId) { var isContent = objectTypeId == Constants.ObjectTypes.Document || objectTypeId == Constants.ObjectTypes.DocumentBlueprint; var isMedia = objectTypeId == Constants.ObjectTypes.Media; @@ -126,21 +127,21 @@ namespace Umbraco.Core.Persistence.Repositories.Implement return GetEntity(sql, isContent, isMedia); } - public virtual IEnumerable GetAll(Guid objectType, params int[] ids) + public IEnumerable GetAll(Guid objectType, params int[] ids) { return ids.Length > 0 ? PerformGetAll(objectType, sql => sql.WhereIn(x => x.NodeId, ids.Distinct())) : PerformGetAll(objectType); } - public virtual IEnumerable GetAll(Guid objectType, params Guid[] keys) + public IEnumerable GetAll(Guid objectType, params Guid[] keys) { return keys.Length > 0 ? PerformGetAll(objectType, sql => sql.WhereIn(x => x.UniqueId, keys.Distinct())) : PerformGetAll(objectType); } - private IEnumerable GetEntities(Sql sql, bool isContent, bool isMedia) + private IEnumerable GetEntities(Sql sql, bool isContent, bool isMedia, bool loadMediaProperties) { //isContent is going to return a 1:M result now with the variants so we need to do different things if (isContent) @@ -157,7 +158,8 @@ namespace Umbraco.Core.Persistence.Repositories.Implement var entities = dtos.Select(x => BuildEntity(false, isMedia, x)).ToArray(); - if (isMedia) + //fixme - see https://github.com/umbraco/Umbraco-CMS/pull/3460#issuecomment-434903930 we need to not load any property data at all for media + if (isMedia && loadMediaProperties) BuildProperties(entities, dtos); return entities; @@ -169,17 +171,17 @@ namespace Umbraco.Core.Persistence.Repositories.Implement var isMedia = objectType == Constants.ObjectTypes.Media; var sql = GetFullSqlForEntityType(isContent, isMedia, objectType, filter); - return GetEntities(sql, isContent, isMedia); + return GetEntities(sql, isContent, isMedia, true); } - public virtual IEnumerable GetAllPaths(Guid objectType, params int[] ids) + public IEnumerable GetAllPaths(Guid objectType, params int[] ids) { return ids.Any() ? PerformGetAllPaths(objectType, sql => sql.WhereIn(x => x.NodeId, ids.Distinct())) : PerformGetAllPaths(objectType); } - public virtual IEnumerable GetAllPaths(Guid objectType, params Guid[] keys) + public IEnumerable GetAllPaths(Guid objectType, params Guid[] keys) { return keys.Any() ? PerformGetAllPaths(objectType, sql => sql.WhereIn(x => x.UniqueId, keys.Distinct())) @@ -193,7 +195,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement return Database.Fetch(sql); } - public virtual IEnumerable GetByQuery(IQuery query) + public IEnumerable GetByQuery(IQuery query) { var sqlClause = GetBase(false, false, null); var translator = new SqlTranslator(sqlClause, query); @@ -203,7 +205,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement return dtos.Select(x => BuildEntity(false, false, x)).ToList(); } - public virtual IEnumerable GetByQuery(IQuery query, Guid objectType) + public IEnumerable GetByQuery(IQuery query, Guid objectType) { var isContent = objectType == Constants.ObjectTypes.Document || objectType == Constants.ObjectTypes.DocumentBlueprint; var isMedia = objectType == Constants.ObjectTypes.Media; @@ -214,7 +216,22 @@ namespace Umbraco.Core.Persistence.Repositories.Implement sql = translator.Translate(); sql = AddGroupBy(isContent, isMedia, sql); - return GetEntities(sql, isContent, isMedia); + return GetEntities(sql, isContent, isMedia, true); + } + + //fixme - see https://github.com/umbraco/Umbraco-CMS/pull/3460#issuecomment-434903930 we need to not load any property data at all for media + internal IEnumerable GetMediaByQueryWithoutPropertyData(IQuery query) + { + var isContent = false; + var isMedia = true; + + var sql = GetBaseWhere(isContent, isMedia, false, null, Constants.ObjectTypes.Media); + + var translator = new SqlTranslator(sql, query); + sql = translator.Translate(); + sql = AddGroupBy(isContent, isMedia, sql); + + return GetEntities(sql, isContent, isMedia, false); } public UmbracoObjectTypes GetObjectType(int id) @@ -241,6 +258,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement return Database.ExecuteScalar(sql) > 0; } + //fixme - see https://github.com/umbraco/Umbraco-CMS/pull/3460#issuecomment-434903930 we need to not load any property data at all for media private void BuildProperties(EntitySlim entity, BaseDto dto) { var pdtos = Database.Fetch(GetPropertyData(dto.VersionId)); @@ -248,6 +266,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement BuildProperty(entity, pdto); } + //fixme - see https://github.com/umbraco/Umbraco-CMS/pull/3460#issuecomment-434903930 we need to not load any property data at all for media private void BuildProperties(EntitySlim[] entities, List dtos) { var versionIds = dtos.Select(x => x.VersionId).Distinct().ToList(); @@ -263,6 +282,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement } } + //fixme - see https://github.com/umbraco/Umbraco-CMS/pull/3460#issuecomment-434903930 we need to not load any property data at all for media private void BuildProperty(EntitySlim entity, PropertyDataDto pdto) { // explain ?! diff --git a/src/Umbraco.Core/Services/IMediaService.cs b/src/Umbraco.Core/Services/IMediaService.cs index 9ff149b355..ce46b197a0 100644 --- a/src/Umbraco.Core/Services/IMediaService.cs +++ b/src/Umbraco.Core/Services/IMediaService.cs @@ -10,15 +10,6 @@ using Umbraco.Core.Persistence.Querying; namespace Umbraco.Core.Services { - /// - /// Moves an object to a new location - /// - /// The to move - /// Id of the Media's new Parent - /// Id of the User moving the Media - /// True if moving succeeded, otherwise False - Attempt Move(IMedia media, int parentId, int userId = 0); - /// /// Defines the Media Service, which is an easy access to operations involving /// @@ -160,9 +151,9 @@ namespace Umbraco.Core.Services /// The to move /// Id of the Media's new Parent /// Id of the User moving the Media - void Move(IMedia media, int parentId, int userId = 0); + /// True if moving succeeded, otherwise False + Attempt Move(IMedia media, int parentId, int userId = 0); - /// /// Deletes an object by moving it to the Recycle Bin /// diff --git a/src/Umbraco.Core/Services/Implement/EntityService.cs b/src/Umbraco.Core/Services/Implement/EntityService.cs index 385b5eabe0..45c229214a 100644 --- a/src/Umbraco.Core/Services/Implement/EntityService.cs +++ b/src/Umbraco.Core/Services/Implement/EntityService.cs @@ -11,6 +11,7 @@ using Umbraco.Core.Persistence.DatabaseModelDefinitions; using Umbraco.Core.Persistence.Dtos; using Umbraco.Core.Persistence.Querying; using Umbraco.Core.Persistence.Repositories; +using Umbraco.Core.Persistence.Repositories.Implement; using Umbraco.Core.Scoping; namespace Umbraco.Core.Services.Implement @@ -373,6 +374,22 @@ namespace Umbraco.Core.Services.Implement } } + /// + /// Gets a collection of children by the parent's Id and UmbracoObjectType without adding property data + /// + /// Id of the parent to retrieve children for + /// An enumerable list of objects + internal IEnumerable GetMediaChildrenWithoutPropertyData(int parentId) + { + using (ScopeProvider.CreateScope(autoComplete: true)) + { + var query = Query().Where(x => x.ParentId == parentId); + + //fixme - see https://github.com/umbraco/Umbraco-CMS/pull/3460#issuecomment-434903930 we need to not load any property data at all for media + return ((EntityRepository)_entityRepository).GetMediaByQueryWithoutPropertyData(query); + } + } + /// public virtual IEnumerable GetDescendants(int id) { diff --git a/src/Umbraco.Core/Services/Implement/MediaService.cs b/src/Umbraco.Core/Services/Implement/MediaService.cs index 5f35e35acf..aadea82dd4 100644 --- a/src/Umbraco.Core/Services/Implement/MediaService.cs +++ b/src/Umbraco.Core/Services/Implement/MediaService.cs @@ -888,7 +888,8 @@ namespace Umbraco.Core.Services.Implement var originalPath = media.Path; - if (scope.Events.DispatchCancelable(Trashing, this, new MoveEventArgs(new MoveEventInfo(media, originalPath, Constants.System.RecycleBinMedia)), nameof(Trashing))) + var moveEventArgs = new MoveEventArgs(true, evtMsgs, new MoveEventInfo(media, originalPath, Constants.System.RecycleBinMedia)); + if (scope.Events.DispatchCancelable(Trashing, this, moveEventArgs, nameof(Trashing))) { scope.Complete(); return OperationResult.Attempt.Cancel(evtMsgs); @@ -899,8 +900,9 @@ namespace Umbraco.Core.Services.Implement scope.Events.Dispatch(TreeChanged, this, new TreeChange(media, TreeChangeTypes.RefreshBranch).ToEventArgs()); var moveInfo = moves.Select(x => new MoveEventInfo(x.Item1, x.Item2, x.Item1.ParentId)) .ToArray(); - - scope.Events.Dispatch(Trashed, this, new MoveEventArgs(false, evtMsgs, moveInfo), nameof(Trashed)); + moveEventArgs.MoveInfoCollection = moveInfo; + moveEventArgs.CanCancel = false; + scope.Events.Dispatch(Trashed, this, moveEventArgs, nameof(Trashed)); Audit(AuditType.Move, userId, media.Id, "Move Media to recycle bin"); scope.Complete(); @@ -915,13 +917,15 @@ namespace Umbraco.Core.Services.Implement /// The to move /// Id of the Media's new Parent /// Id of the User moving the Media - public void Move(IMedia media, int parentId, int userId = 0) + public Attempt Move(IMedia media, int parentId, int userId = 0) { + var evtMsgs = EventMessagesFactory.Get(); + // if moving to the recycle bin then use the proper method if (parentId == Constants.System.RecycleBinMedia) { MoveToRecycleBin(media, userId); - return; + return OperationResult.Attempt.Succeed(evtMsgs); } var moves = new List>(); @@ -935,11 +939,11 @@ namespace Umbraco.Core.Services.Implement throw new InvalidOperationException("Parent does not exist or is trashed."); // causes rollback // causes rollback var moveEventInfo = new MoveEventInfo(media, media.Path, parentId); - var moveEventArgs = new MoveEventArgs(moveEventInfo); + var moveEventArgs = new MoveEventArgs(true, evtMsgs, moveEventInfo); if (scope.Events.DispatchCancelable(Moving, this, moveEventArgs, nameof(Moving))) { scope.Complete(); - return; + return OperationResult.Attempt.Cancel(evtMsgs); } // if media was trashed, and since we're not moving to the recycle bin, @@ -964,6 +968,7 @@ namespace Umbraco.Core.Services.Implement Audit(AuditType.Move, userId, media.Id); scope.Complete(); } + return OperationResult.Attempt.Succeed(evtMsgs); } // MUST be called from within WriteLock diff --git a/src/Umbraco.Web/Editors/ContentTypeController.cs b/src/Umbraco.Web/Editors/ContentTypeController.cs index 63346f177a..397f6b3e9d 100644 --- a/src/Umbraco.Web/Editors/ContentTypeController.cs +++ b/src/Umbraco.Web/Editors/ContentTypeController.cs @@ -234,7 +234,7 @@ namespace Umbraco.Web.Editors Services.ContentTypeService.Save(collectionDocType); // test if the parent exist and then allow the collection underneath - var parentCt = Services.ContentTypeService.GetContentType(parentId); + var parentCt = Services.ContentTypeService.Get(parentId); if (parentCt != null) { var allowedCts = parentCt.AllowedContentTypes.ToList(); diff --git a/src/Umbraco.Web/Editors/MediaController.cs b/src/Umbraco.Web/Editors/MediaController.cs index fe15c4d33b..67ae63dd6b 100644 --- a/src/Umbraco.Web/Editors/MediaController.cs +++ b/src/Umbraco.Web/Editors/MediaController.cs @@ -427,7 +427,7 @@ namespace Umbraco.Web.Editors var destinationParentID = move.ParentId; var sourceParentID = toMove.ParentId; - var moveResult = Services.MediaService.WithResult().Move(toMove, move.ParentId); + var moveResult = Services.MediaService.Move(toMove, move.ParentId); if (sourceParentID == destinationParentID) { diff --git a/src/Umbraco.Web/Editors/PasswordChanger.cs b/src/Umbraco.Web/Editors/PasswordChanger.cs index ad8a2a6f60..54c3d12ea9 100644 --- a/src/Umbraco.Web/Editors/PasswordChanger.cs +++ b/src/Umbraco.Web/Editors/PasswordChanger.cs @@ -186,7 +186,7 @@ namespace Umbraco.Web.Editors } catch (Exception ex) { - _logger.WarnWithException("Could not change member password", ex); + _logger.Warn("Could not change member password", ex); return Attempt.Fail(new PasswordChangedModel { ChangeError = new ValidationResult("Could not change password, error: " + ex.Message + " (see log for full details)", new[] { "value" }) }); } } diff --git a/src/Umbraco.Web/Trees/ContentTreeController.cs b/src/Umbraco.Web/Trees/ContentTreeController.cs index d9f872ab85..8fdb267f6a 100644 --- a/src/Umbraco.Web/Trees/ContentTreeController.cs +++ b/src/Umbraco.Web/Trees/ContentTreeController.cs @@ -202,7 +202,7 @@ namespace Umbraco.Web.Trees return HasPathAccess(entity, queryStrings); } - internal override IEnumerable GetChildrenFromEntityService(int entityId) + internal override IEnumerable GetChildrenFromEntityService(int entityId) => Services.EntityService.GetChildren(entityId, UmbracoObjectType).ToList(); protected override IEnumerable GetChildEntities(string id, FormDataCollection queryStrings) diff --git a/src/Umbraco.Web/Trees/ContentTreeControllerBase.cs b/src/Umbraco.Web/Trees/ContentTreeControllerBase.cs index 560c15b568..a66180321d 100644 --- a/src/Umbraco.Web/Trees/ContentTreeControllerBase.cs +++ b/src/Umbraco.Web/Trees/ContentTreeControllerBase.cs @@ -197,9 +197,8 @@ namespace Umbraco.Web.Trees entityId = entity.Id; } - return GetChildrenFromEntityService(entityId); + IEntitySlim[] result; - /* // if a request is made for the root node but user has no access to // root node, return start nodes instead if (entityId == Constants.System.Root && UserStartNodes.Contains(Constants.System.Root) == false) @@ -210,10 +209,10 @@ namespace Umbraco.Web.Trees } else { - result = Services.EntityService.GetChildren(entityId, UmbracoObjectType).ToArray(); + result = GetChildrenFromEntityService(entityId).ToArray(); } - return result;*/ + return result; } /// @@ -221,7 +220,7 @@ namespace Umbraco.Web.Trees /// /// /// - internal abstract IEnumerable GetChildrenFromEntityService(int entityId); + internal abstract IEnumerable GetChildrenFromEntityService(int entityId); /// /// Returns true or false if the current user has access to the node based on the user's allowed start node (path) access diff --git a/src/Umbraco.Web/Trees/MediaTreeController.cs b/src/Umbraco.Web/Trees/MediaTreeController.cs index 68bd538748..74b810a961 100644 --- a/src/Umbraco.Web/Trees/MediaTreeController.cs +++ b/src/Umbraco.Web/Trees/MediaTreeController.cs @@ -17,6 +17,7 @@ using Umbraco.Web.WebApi.Filters; using Umbraco.Web.Models.ContentEditing; using Umbraco.Web.Search; using Constants = Umbraco.Core.Constants; +using Umbraco.Core.Services.Implement; namespace Umbraco.Web.Trees { @@ -163,7 +164,7 @@ namespace Umbraco.Web.Trees return _treeSearcher.ExamineSearch(Umbraco, query, UmbracoEntityTypes.Media, pageSize, pageIndex, out totalFound, searchFrom); } - internal override IEnumerable GetChildrenFromEntityService(int entityId) + internal override IEnumerable GetChildrenFromEntityService(int entityId) // Not pretty having to cast the service, but it is the only way to get to use an internal method that we // do not want to make public on the interface. Unfortunately also prevents this from being unit tested. // See this issue for details on why we need this: diff --git a/src/Umbraco.Web/umbraco.presentation/umbraco/developer/RelationTypes/EditRelationType.aspx.cs b/src/Umbraco.Web/umbraco.presentation/umbraco/developer/RelationTypes/EditRelationType.aspx.cs index 5a2a136832..74c639b4a7 100644 --- a/src/Umbraco.Web/umbraco.presentation/umbraco/developer/RelationTypes/EditRelationType.aspx.cs +++ b/src/Umbraco.Web/umbraco.presentation/umbraco/developer/RelationTypes/EditRelationType.aspx.cs @@ -1,5 +1,5 @@ //TODO: Rebuild with new tree format and apis and then remove -using umbraco.uicontrols; +//using umbraco.uicontrols; //using System; //using System.Collections.Generic;