From 5cd28c871161deb130cf59bd4cba0c2bcd2c91ce Mon Sep 17 00:00:00 2001 From: Elitsa Marinovska Date: Mon, 1 Feb 2021 16:07:42 +0100 Subject: [PATCH 1/2] 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... From 1399fbabe4f5fb076a5264a75771cf5d233cfcec Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Thu, 25 Feb 2021 08:08:02 +0100 Subject: [PATCH 2/2] Reuse save methods.. --- .../Services/Implement/ContentService.cs | 35 +++---------------- .../Services/Implement/MemberService.cs | 32 +++-------------- 2 files changed, 10 insertions(+), 57 deletions(-) diff --git a/src/Umbraco.Infrastructure/Services/Implement/ContentService.cs b/src/Umbraco.Infrastructure/Services/Implement/ContentService.cs index 153665debf..40e3a28de8 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; @@ -242,7 +242,7 @@ namespace Umbraco.Core.Services.Implement { // TODO: what about culture? - using (var scope = ScopeProvider.CreateScope()) + using (var scope = ScopeProvider.CreateScope(autoComplete:true)) { // locking the content tree secures content types too scope.WriteLock(Constants.Locks.ContentTree); @@ -256,21 +256,9 @@ namespace Umbraco.Core.Services.Implement throw new ArgumentException("No content with that id.", nameof(parentId)); // causes rollback var content = parentId > 0 ? new Content(name, parent, contentType, 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); + Save(content, userId); - 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; } } @@ -290,7 +278,7 @@ namespace Umbraco.Core.Services.Implement if (parent == null) throw new ArgumentNullException(nameof(parent)); - using (var scope = ScopeProvider.CreateScope()) + using (var scope = ScopeProvider.CreateScope(autoComplete:true)) { // locking the content tree secures content types too scope.WriteLock(Constants.Locks.ContentTree); @@ -301,21 +289,8 @@ namespace Umbraco.Core.Services.Implement var content = new Content(name, parent, contentType, userId); - var evtMsgs = EventMessagesFactory.Get(); + Save(content, userId); - // 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; } } diff --git a/src/Umbraco.Infrastructure/Services/Implement/MemberService.cs b/src/Umbraco.Infrastructure/Services/Implement/MemberService.cs index 90cbaa4fd6..0ec08fb051 100644 --- a/src/Umbraco.Infrastructure/Services/Implement/MemberService.cs +++ b/src/Umbraco.Infrastructure/Services/Implement/MemberService.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Linq; using Microsoft.Extensions.Logging; @@ -219,18 +219,7 @@ namespace Umbraco.Infrastructure.Services.Implement 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(); + Save(member); return member; } } @@ -301,18 +290,7 @@ namespace Umbraco.Infrastructure.Services.Implement 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, -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(); + Save(member); return member; } } @@ -428,7 +406,7 @@ namespace Umbraco.Infrastructure.Services.Implement { using (var scope = ScopeProvider.CreateScope(autoComplete: true)) { - scope.ReadLock(Constants.Locks.MemberTree); + scope.ReadLock(Constants.Locks.MemberTree); return _memberRepository.GetByUsername(username); } } @@ -783,7 +761,7 @@ namespace Umbraco.Infrastructure.Services.Implement public void SetLastLogin(string username, DateTime date) { using (var scope = ScopeProvider.CreateScope()) - { + { _memberRepository.SetLastLogin(username, date); scope.Complete(); }