From 5cd28c871161deb130cf59bd4cba0c2bcd2c91ce Mon Sep 17 00:00:00 2001 From: Elitsa Marinovska Date: Mon, 1 Feb 2021 16:07:42 +0100 Subject: [PATCH] Fixing CreateMemember and CreateContent to raise events and logging only when we persist the objs not when we only create them --- src/Umbraco.Core/Models/Content.cs | 32 +++++++- src/Umbraco.Core/Models/Member.cs | 52 ++++++++++++- .../Services/Implement/ContentService.cs | 77 ++++++++----------- .../Services/Implement/MemberService.cs | 67 +++++++--------- 4 files changed, 140 insertions(+), 88 deletions(-) diff --git a/src/Umbraco.Core/Models/Content.cs b/src/Umbraco.Core/Models/Content.cs index 04f49e704e..9479dde085 100644 --- a/src/Umbraco.Core/Models/Content.cs +++ b/src/Umbraco.Core/Models/Content.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Collections.Specialized; using System.Linq; @@ -39,6 +39,21 @@ namespace Umbraco.Core.Models : this(name, parent, contentType, new PropertyCollection(), culture) { } + /// + /// Constructor for creating a Content object + /// + /// Name of the content + /// Parent object + /// ContentType for the current Content object + /// The identifier of the user creating the Content object + /// An optional culture. + public Content(string name, IContent parent, IContentType contentType, int userId, string culture = null) + : this(name, parent, contentType, new PropertyCollection(), culture) + { + CreatorId = userId; + WriterId = userId; + } + /// /// Constructor for creating a Content object /// @@ -66,6 +81,21 @@ namespace Umbraco.Core.Models : this(name, parentId, contentType, new PropertyCollection(), culture) { } + /// + /// Constructor for creating a Content object + /// + /// Name of the content + /// Id of the Parent content + /// ContentType for the current Content object + /// The identifier of the user creating the Content object + /// An optional culture. + public Content(string name, int parentId, IContentType contentType, int userId, string culture = null) + : this(name, parentId, contentType, new PropertyCollection(), culture) + { + CreatorId = userId; + WriterId = userId; + } + /// /// Constructor for creating a Content object /// diff --git a/src/Umbraco.Core/Models/Member.cs b/src/Umbraco.Core/Models/Member.cs index 8a765b2f25..168ef607ba 100644 --- a/src/Umbraco.Core/Models/Member.cs +++ b/src/Umbraco.Core/Models/Member.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.ComponentModel; using System.Runtime.Serialization; @@ -79,6 +79,34 @@ namespace Umbraco.Core.Models _rawPasswordValue = ""; } + /// + /// Constructor for creating a Member object + /// + /// + /// + /// + /// + /// + /// + public Member(string name, string email, string username, IMemberType contentType, int userId, bool isApproved = true) + : base(name, -1, contentType, new PropertyCollection()) + { + if (name == null) throw new ArgumentNullException(nameof(name)); + if (string.IsNullOrWhiteSpace(name)) throw new ArgumentException("Value can't be empty or consist only of white-space characters.", nameof(name)); + if (email == null) throw new ArgumentNullException(nameof(email)); + if (string.IsNullOrWhiteSpace(email)) throw new ArgumentException("Value can't be empty or consist only of white-space characters.", nameof(email)); + if (username == null) throw new ArgumentNullException(nameof(username)); + if (string.IsNullOrWhiteSpace(username)) throw new ArgumentException("Value can't be empty or consist only of white-space characters.", nameof(username)); + + _email = email; + _username = username; + CreatorId = userId; + IsApproved = isApproved; + + //this cannot be null but can be empty + _rawPasswordValue = ""; + } + /// /// Constructor for creating a Member object /// @@ -118,6 +146,28 @@ namespace Umbraco.Core.Models IsApproved = isApproved; } + /// + /// Constructor for creating a Member object + /// + /// + /// + /// + /// + /// The password value passed in to this parameter should be the encoded/encrypted/hashed format of the member's password + /// + /// + /// + /// + public Member(string name, string email, string username, string rawPasswordValue, IMemberType contentType, bool isApproved, int userId) + : base(name, -1, contentType, new PropertyCollection()) + { + _email = email; + _username = username; + _rawPasswordValue = rawPasswordValue; + IsApproved = isApproved; + CreatorId = userId; + } + /// /// Gets or sets the Username /// diff --git a/src/Umbraco.Infrastructure/Services/Implement/ContentService.cs b/src/Umbraco.Infrastructure/Services/Implement/ContentService.cs index 69dadb2b21..153665debf 100644 --- a/src/Umbraco.Infrastructure/Services/Implement/ContentService.cs +++ b/src/Umbraco.Infrastructure/Services/Implement/ContentService.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Globalization; using System.Linq; @@ -197,12 +197,7 @@ namespace Umbraco.Core.Services.Implement if (parentId > 0 && parent == null) throw new ArgumentException("No content with that id.", nameof(parentId)); - var content = new Content(name, parentId, contentType); - using (var scope = ScopeProvider.CreateScope()) - { - CreateContent(scope, content, userId, false); - scope.Complete(); - } + var content = new Content(name, parentId, contentType, userId); return content; } @@ -225,20 +220,13 @@ namespace Umbraco.Core.Services.Implement if (parent == null) throw new ArgumentNullException(nameof(parent)); - using (var scope = ScopeProvider.CreateScope()) - { - // not locking since not saving anything + var contentType = GetContentType(contentTypeAlias); + if (contentType == null) + throw new ArgumentException("No content type with that alias.", nameof(contentTypeAlias)); // causes rollback - var contentType = GetContentType(contentTypeAlias); - if (contentType == null) - throw new ArgumentException("No content type with that alias.", nameof(contentTypeAlias)); // causes rollback + var content = new Content(name, parent, contentType, userId); - var content = new Content(name, parent, contentType); - CreateContent(scope, content, userId, false); - - scope.Complete(); - return content; - } + return content; } /// @@ -267,8 +255,20 @@ namespace Umbraco.Core.Services.Implement if (parentId > 0 && parent == null) throw new ArgumentException("No content with that id.", nameof(parentId)); // causes rollback - var content = parentId > 0 ? new Content(name, parent, contentType) : new Content(name, parentId, contentType); - CreateContent(scope, content, userId, true); + var content = parentId > 0 ? new Content(name, parent, contentType, userId) : new Content(name, parentId, contentType, userId); + var evtMsgs = EventMessagesFactory.Get(); + + // if saving is cancelled, content remains without an identity + var saveEventArgs = new ContentSavingEventArgs(content, evtMsgs); + if (!scope.Events.DispatchCancelable(Saving, this, saveEventArgs, nameof(Saving))) + { + _documentRepository.Save(content); + + scope.Events.Dispatch(Saved, this, saveEventArgs.ToContentSavedEventArgs(), nameof(Saved)); + scope.Events.Dispatch(TreeChanged, this, new TreeChange(content, TreeChangeTypes.RefreshNode).ToEventArgs()); + + Audit(AuditType.New, content.CreatorId, content.Id, $"Content '{content.Name}' was created with Id {content.Id}"); + } scope.Complete(); return content; @@ -299,38 +299,25 @@ namespace Umbraco.Core.Services.Implement if (contentType == null) throw new ArgumentException("No content type with that alias.", nameof(contentTypeAlias)); // causes rollback - var content = new Content(name, parent, contentType); - CreateContent(scope, content, userId, true); + var content = new Content(name, parent, contentType, userId); - scope.Complete(); - return content; - } - } - - private void CreateContent(IScope scope, IContent content, int userId, bool withIdentity) - { - content.CreatorId = userId; - content.WriterId = userId; - - if (withIdentity) - { var evtMsgs = EventMessagesFactory.Get(); // if saving is cancelled, content remains without an identity var saveEventArgs = new ContentSavingEventArgs(content, evtMsgs); - if (scope.Events.DispatchCancelable(Saving, this, saveEventArgs, nameof(Saving))) - return; + if (!scope.Events.DispatchCancelable(Saving, this, saveEventArgs, nameof(Saving))) + { + _documentRepository.Save(content); - _documentRepository.Save(content); + scope.Events.Dispatch(Saved, this, saveEventArgs.ToContentSavedEventArgs(), nameof(Saved)); + scope.Events.Dispatch(TreeChanged, this, new TreeChange(content, TreeChangeTypes.RefreshNode).ToEventArgs()); - scope.Events.Dispatch(Saved, this, saveEventArgs.ToContentSavedEventArgs(), nameof(Saved)); - scope.Events.Dispatch(TreeChanged, this, new TreeChange(content, TreeChangeTypes.RefreshNode).ToEventArgs()); + Audit(AuditType.New, content.CreatorId, content.Id, $"Content '{content.Name}' was created with Id {content.Id}"); + } + + scope.Complete(); + return content; } - - if (withIdentity == false) - return; - - Audit(AuditType.New, content.CreatorId, content.Id, $"Content '{content.Name}' was created with Id {content.Id}"); } #endregion diff --git a/src/Umbraco.Infrastructure/Services/Implement/MemberService.cs b/src/Umbraco.Infrastructure/Services/Implement/MemberService.cs index 721c0eee21..90cbaa4fd6 100644 --- a/src/Umbraco.Infrastructure/Services/Implement/MemberService.cs +++ b/src/Umbraco.Infrastructure/Services/Implement/MemberService.cs @@ -120,10 +120,7 @@ namespace Umbraco.Infrastructure.Services.Implement throw new ArgumentException("No member type with that alias.", nameof(memberTypeAlias)); } - var member = new Member(name, email.ToLower().Trim(), username, memberType); - using IScope scope = ScopeProvider.CreateScope(); - CreateMember(scope, member, 0, false); - scope.Complete(); + var member = new Member(name, email.ToLower().Trim(), username, memberType, 0); return member; } @@ -143,12 +140,7 @@ namespace Umbraco.Infrastructure.Services.Implement { if (memberType == null) throw new ArgumentNullException(nameof(memberType)); - var member = new Member(name, email.ToLower().Trim(), username, memberType); - using (var scope = ScopeProvider.CreateScope()) - { - CreateMember(scope, member, 0, false); - scope.Complete(); - } + var member = new Member(name, email.ToLower().Trim(), username, memberType, 0); return member; } @@ -225,8 +217,18 @@ namespace Umbraco.Infrastructure.Services.Implement if (memberType == null) throw new ArgumentException("No member type with that alias.", nameof(memberTypeAlias)); // causes rollback // causes rollback - var member = new Member(name, email.ToLower().Trim(), username, passwordValue, memberType, isApproved); - CreateMember(scope, member, -1, true); + var member = new Member(name, email.ToLower().Trim(), username, passwordValue, memberType, isApproved, -1); + + var saveEventArgs = new SaveEventArgs(member); + if (!scope.Events.DispatchCancelable(Saving, this, saveEventArgs)) + { + _memberRepository.Save(member); + + saveEventArgs.CanCancel = false; + scope.Events.Dispatch(Saved, this, saveEventArgs); + + Audit(AuditType.New, member.CreatorId, member.Id, $"Member '{member.Name}' was created with Id {member.Id}"); + } scope.Complete(); return member; @@ -297,41 +299,24 @@ namespace Umbraco.Infrastructure.Services.Implement if (vrfy == null || vrfy.Id != memberType.Id) throw new ArgumentException($"Member type with alias {memberType.Alias} does not exist or is a different member type."); // causes rollback - var member = new Member(name, email.ToLower().Trim(), username, passwordValue, memberType, isApproved); + var member = new Member(name, email.ToLower().Trim(), username, passwordValue, memberType, isApproved, -1); + + var saveEventArgs = new SaveEventArgs(member); + if (!scope.Events.DispatchCancelable(Saving, this, saveEventArgs)) + { + _memberRepository.Save(member); + + saveEventArgs.CanCancel = false; + scope.Events.Dispatch(Saved, this, saveEventArgs); + + Audit(AuditType.New, member.CreatorId, member.Id, $"Member '{member.Name}' was created with Id {member.Id}"); + } - CreateMember(scope, member, -1, true); scope.Complete(); return member; } } - private void CreateMember(IScope scope, Member member, int userId, bool withIdentity) - { - member.CreatorId = userId; - - if (withIdentity) - { - // if saving is cancelled, media remains without an identity - var saveEventArgs = new SaveEventArgs(member); - if (scope.Events.DispatchCancelable(Saving, this, saveEventArgs)) - { - return; - } - - _memberRepository.Save(member); - - saveEventArgs.CanCancel = false; - scope.Events.Dispatch(Saved, this, saveEventArgs); - } - - if (withIdentity == false) - { - return; - } - - Audit(AuditType.New, member.CreatorId, member.Id, $"Member '{member.Name}' was created with Id {member.Id}"); - } - #endregion #region Get, Has, Is, Exists...