Refactor content updates to shift responsibility downwards (#15807)

kjac · web-flow · commit d8659ca53e5a · 2024-03-01T10:45:19.000+01:00
diff --git a/src/Umbraco.Cms.Api.Management/Controllers/Document/UpdateDocumentController.cs b/src/Umbraco.Cms.Api.Management/Controllers/Document/UpdateDocumentController.cs
@@ -24,7 +24,7 @@ public UpdateDocumentController(
         IContentEditingService contentEditingService,
         IDocumentEditingPresentationFactory documentEditingPresentationFactory,
         IBackOfficeSecurityAccessor backOfficeSecurityAccessor)
-        : base(authorizationService, contentEditingService)
+        : base(authorizationService)
     {
         _contentEditingService = contentEditingService;
         _documentEditingPresentationFactory = documentEditingPresentationFactory;
@@ -37,11 +37,11 @@ public UpdateDocumentController(
     [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status400BadRequest)]
     [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)]
     public async Task<IActionResult> Update(Guid id, UpdateDocumentRequestModel requestModel)
-        => await HandleRequest(id, requestModel, async content =>
+        => await HandleRequest(id, requestModel, async () =>
         {
             ContentUpdateModel model = _documentEditingPresentationFactory.MapUpdateModel(requestModel);
             Attempt<ContentUpdateResult, ContentEditingOperationStatus> result =
-                await _contentEditingService.UpdateAsync(content, model, CurrentUserKey(_backOfficeSecurityAccessor));
+                await _contentEditingService.UpdateAsync(id, model, CurrentUserKey(_backOfficeSecurityAccessor));
 
             return result.Success
                 ? Ok()
diff --git a/src/Umbraco.Cms.Api.Management/Controllers/Document/UpdateDocumentControllerBase.cs b/src/Umbraco.Cms.Api.Management/Controllers/Document/UpdateDocumentControllerBase.cs
@@ -1,11 +1,8 @@
 ﻿using Microsoft.AspNetCore.Authorization;
 using Microsoft.AspNetCore.Mvc;
-using Umbraco.Cms.Api.Management.Security.Authorization.Content;
 using Umbraco.Cms.Api.Management.ViewModels.Document;
 using Umbraco.Cms.Core.Actions;
-using Umbraco.Cms.Core.Models;
 using Umbraco.Cms.Core.Security.Authorization;
-using Umbraco.Cms.Core.Services;
 using Umbraco.Cms.Web.Common.Authorization;
 using Umbraco.Extensions;
 
@@ -14,15 +11,11 @@ namespace Umbraco.Cms.Api.Management.Controllers.Document;
 public abstract class UpdateDocumentControllerBase : DocumentControllerBase
 {
     private readonly IAuthorizationService _authorizationService;
-    private readonly IContentEditingService _contentEditingService;
 
-    protected UpdateDocumentControllerBase(IAuthorizationService authorizationService, IContentEditingService contentEditingService)
-    {
-        _authorizationService = authorizationService;
-        _contentEditingService = contentEditingService;
-    }
+    protected UpdateDocumentControllerBase(IAuthorizationService authorizationService)
+        => _authorizationService = authorizationService;
 
-    protected async Task<IActionResult> HandleRequest(Guid id, UpdateDocumentRequestModel requestModel, Func<IContent, Task<IActionResult>> authorizedHandler)
+    protected async Task<IActionResult> HandleRequest(Guid id, UpdateDocumentRequestModel requestModel, Func<Task<IActionResult>> authorizedHandler)
     {
         IEnumerable<string> cultures = requestModel.Variants
             .Where(v => v.Culture is not null)
@@ -37,12 +30,6 @@ protected async Task<IActionResult> HandleRequest(Guid id, UpdateDocumentRequest
             return Forbidden();
         }
 
-        IContent? content = await _contentEditingService.GetAsync(id);
-        if (content is null)
-        {
-            return DocumentNotFound();
-        }
-
-        return await authorizedHandler(content);
+        return await authorizedHandler();
     }
 }
diff --git a/src/Umbraco.Cms.Api.Management/Controllers/Document/ValidateUpdateDocumentController.cs b/src/Umbraco.Cms.Api.Management/Controllers/Document/ValidateUpdateDocumentController.cs
@@ -21,7 +21,7 @@ public ValidateUpdateDocumentController(
         IAuthorizationService authorizationService,
         IContentEditingService contentEditingService,
         IDocumentEditingPresentationFactory documentEditingPresentationFactory)
-        : base(authorizationService, contentEditingService)
+        : base(authorizationService)
     {
         _contentEditingService = contentEditingService;
         _documentEditingPresentationFactory = documentEditingPresentationFactory;
@@ -33,10 +33,10 @@ public ValidateUpdateDocumentController(
     [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status400BadRequest)]
     [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)]
     public async Task<IActionResult> Validate(Guid id, UpdateDocumentRequestModel requestModel)
-        => await HandleRequest(id, requestModel, async content =>
+        => await HandleRequest(id, requestModel, async () =>
         {
             ContentUpdateModel model = _documentEditingPresentationFactory.MapUpdateModel(requestModel);
-            Attempt<ContentValidationResult, ContentEditingOperationStatus> result = await _contentEditingService.ValidateUpdateAsync(content, model);
+            Attempt<ContentValidationResult, ContentEditingOperationStatus> result = await _contentEditingService.ValidateUpdateAsync(id, model);
 
             return result.Success
                 ? Ok()
diff --git a/src/Umbraco.Cms.Api.Management/Controllers/Media/UpdateMediaController.cs b/src/Umbraco.Cms.Api.Management/Controllers/Media/UpdateMediaController.cs
@@ -24,7 +24,7 @@ public UpdateMediaController(
         IMediaEditingService mediaEditingService,
         IMediaEditingPresentationFactory mediaEditingPresentationFactory,
         IBackOfficeSecurityAccessor backOfficeSecurityAccessor)
-        : base(authorizationService, mediaEditingService)
+        : base(authorizationService)
     {
         _mediaEditingService = mediaEditingService;
         _mediaEditingPresentationFactory = mediaEditingPresentationFactory;
@@ -37,11 +37,11 @@ public UpdateMediaController(
     [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status400BadRequest)]
     [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)]
     public async Task<IActionResult> Update(Guid id, UpdateMediaRequestModel requestModel)
-        => await HandleRequest(id, async media =>
+        => await HandleRequest(id, async () =>
         {
             MediaUpdateModel model = _mediaEditingPresentationFactory.MapUpdateModel(requestModel);
             Attempt<MediaUpdateResult, ContentEditingOperationStatus> result =
-                await _mediaEditingService.UpdateAsync(media, model, CurrentUserKey(_backOfficeSecurityAccessor));
+                await _mediaEditingService.UpdateAsync(id, model, CurrentUserKey(_backOfficeSecurityAccessor));
 
             return result.Success
                 ? Ok()
diff --git a/src/Umbraco.Cms.Api.Management/Controllers/Media/UpdateMediaControllerBase.cs b/src/Umbraco.Cms.Api.Management/Controllers/Media/UpdateMediaControllerBase.cs
@@ -1,9 +1,6 @@
 ﻿using Microsoft.AspNetCore.Authorization;
 using Microsoft.AspNetCore.Mvc;
-using Umbraco.Cms.Api.Management.Security.Authorization.Media;
-using Umbraco.Cms.Core.Models;
 using Umbraco.Cms.Core.Security.Authorization;
-using Umbraco.Cms.Core.Services;
 using Umbraco.Cms.Web.Common.Authorization;
 using Umbraco.Extensions;
 
@@ -12,15 +9,11 @@ namespace Umbraco.Cms.Api.Management.Controllers.Media;
 public abstract class UpdateMediaControllerBase : MediaControllerBase
 {
     private readonly IAuthorizationService _authorizationService;
-    private readonly IMediaEditingService _mediaEditingService;
 
-    protected UpdateMediaControllerBase(IAuthorizationService authorizationService, IMediaEditingService mediaEditingService)
-    {
-        _authorizationService = authorizationService;
-        _mediaEditingService = mediaEditingService;
-    }
+    protected UpdateMediaControllerBase(IAuthorizationService authorizationService)
+        => _authorizationService = authorizationService;
 
-    protected async Task<IActionResult> HandleRequest(Guid id, Func<IMedia, Task<IActionResult>> authorizedHandler)
+    protected async Task<IActionResult> HandleRequest(Guid id, Func<Task<IActionResult>> authorizedHandler)
     {
         AuthorizationResult authorizationResult = await _authorizationService.AuthorizeResourceAsync(
             User,
@@ -32,12 +25,6 @@ protected async Task<IActionResult> HandleRequest(Guid id, Func<IMedia, Task<IAc
             return Forbidden();
         }
 
-        IMedia? media = await _mediaEditingService.GetAsync(id);
-        if (media is null)
-        {
-            return MediaNotFound();
-        }
-
-        return await authorizedHandler(media);
+        return await authorizedHandler();
     }
 }
diff --git a/src/Umbraco.Cms.Api.Management/Controllers/Media/ValidateUpdateMediaController.cs b/src/Umbraco.Cms.Api.Management/Controllers/Media/ValidateUpdateMediaController.cs
@@ -21,7 +21,7 @@ public ValidateUpdateMediaController(
         IAuthorizationService authorizationService,
         IMediaEditingService mediaEditingService,
         IMediaEditingPresentationFactory mediaEditingPresentationFactory)
-        : base(authorizationService, mediaEditingService)
+        : base(authorizationService)
     {
         _mediaEditingService = mediaEditingService;
         _mediaEditingPresentationFactory = mediaEditingPresentationFactory;
@@ -33,10 +33,10 @@ public ValidateUpdateMediaController(
     [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status400BadRequest)]
     [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)]
     public async Task<IActionResult> Validate(Guid id, UpdateMediaRequestModel requestModel)
-        => await HandleRequest(id, async content =>
+        => await HandleRequest(id, async () =>
         {
             MediaUpdateModel model = _mediaEditingPresentationFactory.MapUpdateModel(requestModel);
-            Attempt<ContentValidationResult, ContentEditingOperationStatus> result = await _mediaEditingService.ValidateUpdateAsync(content, model);
+            Attempt<ContentValidationResult, ContentEditingOperationStatus> result = await _mediaEditingService.ValidateUpdateAsync(id, model);
 
             return result.Success
                 ? Ok()
diff --git a/src/Umbraco.Cms.Api.Management/Controllers/Member/UpdateMemberController.cs b/src/Umbraco.Cms.Api.Management/Controllers/Member/UpdateMemberController.cs
@@ -4,7 +4,6 @@
 using Umbraco.Cms.Api.Management.Factories;
 using Umbraco.Cms.Api.Management.ViewModels.Member;
 using Umbraco.Cms.Core;
-using Umbraco.Cms.Core.Models;
 using Umbraco.Cms.Core.Models.ContentEditing;
 using Umbraco.Cms.Core.Security;
 using Umbraco.Cms.Core.Services;
@@ -36,14 +35,8 @@ public UpdateMemberController(
     [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)]
     public async Task<IActionResult> Update(Guid id, UpdateMemberRequestModel updateRequestModel)
     {
-        IMember? member = await _memberEditingService.GetAsync(id);
-        if (member == null)
-        {
-            return MemberNotFound();
-        }
-
         MemberUpdateModel model = _memberEditingPresentationFactory.MapUpdateModel(updateRequestModel);
-        Attempt<MemberUpdateResult, MemberEditingStatus> result = await _memberEditingService.UpdateAsync(member, model, CurrentUser(_backOfficeSecurityAccessor));
+        Attempt<MemberUpdateResult, MemberEditingStatus> result = await _memberEditingService.UpdateAsync(id, model, CurrentUser(_backOfficeSecurityAccessor));
 
         return result.Success
             ? Ok()
diff --git a/src/Umbraco.Cms.Api.Management/Controllers/Member/ValidateUpdateMemberController.cs b/src/Umbraco.Cms.Api.Management/Controllers/Member/ValidateUpdateMemberController.cs
@@ -4,7 +4,6 @@
 using Umbraco.Cms.Api.Management.Factories;
 using Umbraco.Cms.Api.Management.ViewModels.Member;
 using Umbraco.Cms.Core;
-using Umbraco.Cms.Core.Models;
 using Umbraco.Cms.Core.Models.ContentEditing;
 using Umbraco.Cms.Core.Services;
 using Umbraco.Cms.Core.Services.OperationStatus;
@@ -32,14 +31,8 @@ public ValidateUpdateMemberController(
     [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)]
     public async Task<IActionResult> Validate(Guid id, UpdateMemberRequestModel requestModel)
     {
-        IMember? member = await _memberEditingService.GetAsync(id);
-        if (member is null)
-        {
-            return MemberNotFound();
-        }
-
         MemberUpdateModel model = _memberEditingPresentationFactory.MapUpdateModel(requestModel);
-        Attempt<ContentValidationResult, ContentEditingOperationStatus> result = await _memberEditingService.ValidateUpdateAsync(member, model);
+        Attempt<ContentValidationResult, ContentEditingOperationStatus> result = await _memberEditingService.ValidateUpdateAsync(id, model);
 
         return result.Success
             ? Ok()
diff --git a/src/Umbraco.Core/Models/ContentEditing/ContentUpdateResultBase.cs b/src/Umbraco.Core/Models/ContentEditing/ContentUpdateResultBase.cs
@@ -3,7 +3,7 @@
 public abstract class ContentUpdateResultBase<TContent>
     where TContent : class, IContentBase
 {
-    public TContent Content { get; init; } = null!;
+    public TContent? Content { get; init; }
 
     public ContentValidationResult ValidationResult { get; init; } = new();
 }
diff --git a/src/Umbraco.Core/Services/ContentEditingService.cs b/src/Umbraco.Core/Services/ContentEditingService.cs
@@ -1,7 +1,6 @@
 ﻿using Microsoft.Extensions.Logging;
 using Umbraco.Cms.Core.Models;
 using Umbraco.Cms.Core.Models.ContentEditing;
-using Umbraco.Cms.Core.Models.ContentEditing.Validation;
 using Umbraco.Cms.Core.PropertyEditors;
 using Umbraco.Cms.Core.Scoping;
 using Umbraco.Cms.Core.Services.OperationStatus;
@@ -37,8 +36,13 @@ public ContentEditingService(
         return await Task.FromResult(content);
     }
 
-    public async Task<Attempt<ContentValidationResult, ContentEditingOperationStatus>> ValidateUpdateAsync(IContent content, ContentUpdateModel updateModel)
-        => await ValidatePropertiesAsync(updateModel, content.ContentType.Key);
+    public async Task<Attempt<ContentValidationResult, ContentEditingOperationStatus>> ValidateUpdateAsync(Guid key, ContentUpdateModel updateModel)
+    {
+        IContent? content = ContentService.GetById(key);
+        return content is not null
+            ? await ValidatePropertiesAsync(updateModel, content.ContentType.Key)
+            : Attempt.FailWithStatus(ContentEditingOperationStatus.NotFound, new ContentValidationResult());
+    }
 
     public async Task<Attempt<ContentValidationResult, ContentEditingOperationStatus>> ValidateCreateAsync(ContentCreateModel createModel)
         => await ValidatePropertiesAsync(createModel, createModel.ContentTypeKey);
@@ -69,8 +73,14 @@ public async Task<Attempt<ContentCreateResult, ContentEditingOperationStatus>> C
             : Attempt.FailWithStatus(saveStatus, new ContentCreateResult { Content = content });
     }
 
-    public async Task<Attempt<ContentUpdateResult, ContentEditingOperationStatus>> UpdateAsync(IContent content, ContentUpdateModel updateModel, Guid userKey)
+    public async Task<Attempt<ContentUpdateResult, ContentEditingOperationStatus>> UpdateAsync(Guid key, ContentUpdateModel updateModel, Guid userKey)
     {
+        IContent? content = ContentService.GetById(key);
+        if (content is null)
+        {
+            return Attempt.FailWithStatus(ContentEditingOperationStatus.NotFound, new ContentUpdateResult());
+        }
+
         Attempt<ContentUpdateResult, ContentEditingOperationStatus> result = await MapUpdate<ContentUpdateResult>(content, updateModel);
         if (result.Success == false)
         {
diff --git a/src/Umbraco.Core/Services/IContentEditingService.cs b/src/Umbraco.Core/Services/IContentEditingService.cs
@@ -11,11 +11,11 @@ public interface IContentEditingService
 
     Task<Attempt<ContentValidationResult, ContentEditingOperationStatus>> ValidateCreateAsync(ContentCreateModel createModel);
 
-    Task<Attempt<ContentValidationResult, ContentEditingOperationStatus>> ValidateUpdateAsync(IContent content, ContentUpdateModel updateModel);
+    Task<Attempt<ContentValidationResult, ContentEditingOperationStatus>> ValidateUpdateAsync(Guid key, ContentUpdateModel updateModel);
 
     Task<Attempt<ContentCreateResult, ContentEditingOperationStatus>> CreateAsync(ContentCreateModel createModel, Guid userKey);
 
-    Task<Attempt<ContentUpdateResult, ContentEditingOperationStatus>> UpdateAsync(IContent content, ContentUpdateModel updateModel, Guid userKey);
+    Task<Attempt<ContentUpdateResult, ContentEditingOperationStatus>> UpdateAsync(Guid key, ContentUpdateModel updateModel, Guid userKey);
 
     Task<Attempt<IContent?, ContentEditingOperationStatus>> MoveToRecycleBinAsync(Guid key, Guid userKey);
 
diff --git a/src/Umbraco.Core/Services/IMediaEditingService.cs b/src/Umbraco.Core/Services/IMediaEditingService.cs
@@ -11,11 +11,11 @@ public interface IMediaEditingService
 
     Task<Attempt<ContentValidationResult, ContentEditingOperationStatus>> ValidateCreateAsync(MediaCreateModel createModel);
 
-    Task<Attempt<ContentValidationResult, ContentEditingOperationStatus>> ValidateUpdateAsync(IMedia media, MediaUpdateModel updateModel);
+    Task<Attempt<ContentValidationResult, ContentEditingOperationStatus>> ValidateUpdateAsync(Guid key, MediaUpdateModel updateModel);
 
     Task<Attempt<MediaCreateResult, ContentEditingOperationStatus>> CreateAsync(MediaCreateModel createModel, Guid userKey);
 
-    Task<Attempt<MediaUpdateResult, ContentEditingOperationStatus>> UpdateAsync(IMedia media, MediaUpdateModel updateModel, Guid userKey);
+    Task<Attempt<MediaUpdateResult, ContentEditingOperationStatus>> UpdateAsync(Guid key, MediaUpdateModel updateModel, Guid userKey);
 
     Task<Attempt<IMedia?, ContentEditingOperationStatus>> MoveToRecycleBinAsync(Guid key, Guid userKey);
 
diff --git a/src/Umbraco.Core/Services/IMemberEditingService.cs b/src/Umbraco.Core/Services/IMemberEditingService.cs
@@ -11,11 +11,11 @@ public interface IMemberEditingService
 
     Task<Attempt<ContentValidationResult, ContentEditingOperationStatus>> ValidateCreateAsync(MemberCreateModel createModel);
 
-    Task<Attempt<ContentValidationResult, ContentEditingOperationStatus>> ValidateUpdateAsync(IMember member, MemberUpdateModel updateModel);
+    Task<Attempt<ContentValidationResult, ContentEditingOperationStatus>> ValidateUpdateAsync(Guid key, MemberUpdateModel updateModel);
 
     Task<Attempt<MemberCreateResult, MemberEditingStatus>> CreateAsync(MemberCreateModel createModel, IUser user);
 
-    Task<Attempt<MemberUpdateResult, MemberEditingStatus>> UpdateAsync(IMember member, MemberUpdateModel updateModel, IUser user);
+    Task<Attempt<MemberUpdateResult, MemberEditingStatus>> UpdateAsync(Guid key, MemberUpdateModel updateModel, IUser user);
 
     Task<Attempt<IMember?, MemberEditingStatus>> DeleteAsync(Guid key, Guid userKey);
 }
diff --git a/src/Umbraco.Core/Services/MediaEditingService.cs b/src/Umbraco.Core/Services/MediaEditingService.cs
@@ -1,7 +1,6 @@
 ﻿using Microsoft.Extensions.Logging;
 using Umbraco.Cms.Core.Models;
 using Umbraco.Cms.Core.Models.ContentEditing;
-using Umbraco.Cms.Core.Models.ContentEditing.Validation;
 using Umbraco.Cms.Core.PropertyEditors;
 using Umbraco.Cms.Core.Scoping;
 using Umbraco.Cms.Core.Services.OperationStatus;
@@ -32,8 +31,13 @@ public MediaEditingService(
         return await Task.FromResult(media);
     }
 
-    public async Task<Attempt<ContentValidationResult, ContentEditingOperationStatus>> ValidateUpdateAsync(IMedia media, MediaUpdateModel updateModel)
-        => await ValidatePropertiesAsync(updateModel, media.ContentType.Key);
+    public async Task<Attempt<ContentValidationResult, ContentEditingOperationStatus>> ValidateUpdateAsync(Guid key, MediaUpdateModel updateModel)
+    {
+        IMedia? media = ContentService.GetById(key);
+        return media is not null
+            ? await ValidatePropertiesAsync(updateModel, media.ContentType.Key)
+            : Attempt.FailWithStatus(ContentEditingOperationStatus.NotFound, new ContentValidationResult());
+    }
 
     public async Task<Attempt<ContentValidationResult, ContentEditingOperationStatus>> ValidateCreateAsync(MediaCreateModel createModel)
         => await ValidatePropertiesAsync(createModel, createModel.ContentTypeKey);
@@ -60,8 +64,14 @@ public async Task<Attempt<MediaCreateResult, ContentEditingOperationStatus>> Cre
             : Attempt.FailWithStatus(operationStatus, new MediaCreateResult { Content = media });
     }
 
-    public async Task<Attempt<MediaUpdateResult, ContentEditingOperationStatus>> UpdateAsync(IMedia media, MediaUpdateModel updateModel, Guid userKey)
+    public async Task<Attempt<MediaUpdateResult, ContentEditingOperationStatus>> UpdateAsync(Guid key, MediaUpdateModel updateModel, Guid userKey)
     {
+        IMedia? media = ContentService.GetById(key);
+        if (media is null)
+        {
+            return Attempt.FailWithStatus(ContentEditingOperationStatus.NotFound, new MediaUpdateResult());
+        }
+
         Attempt<MediaUpdateResult, ContentEditingOperationStatus> result = await MapUpdate<MediaUpdateResult>(media, updateModel);
         if (result.Success == false)
         {
diff --git a/src/Umbraco.Infrastructure/Services/MemberEditingService.cs b/src/Umbraco.Infrastructure/Services/MemberEditingService.cs
diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/ContentEditingServiceTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/ContentEditingServiceTests.cs
diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentEditingServiceTests.Update.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentEditingServiceTests.Update.cs
diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/MemberEditingServiceTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/MemberEditingServiceTests.cs

Original file line number	Diff line number	Diff line change
`@@ -3,7 +3,7 @@`
`3`	`3`	`public abstract class ContentUpdateResultBase<TContent>`
`4`	`4`	`where TContent : class, IContentBase`
`5`	`5`	`{`
`6`		`- public TContent Content { get; init; } = null!;`
	`6`	`+ public TContent? Content { get; init; }`
`7`	`7`
`8`	`8`	`public ContentValidationResult ValidationResult { get; init; } = new();`
`9`	`9`	`}`