From 4a06aca9d396ac6449e9f4e5fc633a229201bb00 Mon Sep 17 00:00:00 2001 From: Mike Lyttle Date: Thu, 11 Jul 2024 13:28:57 -0700 Subject: [PATCH] Remove unnecessary payload when removing dependent AB#16794 (#6256) --- .../src/Controllers/DependentController.cs | 13 ++----- .../src/Services/DependentService.cs | 10 ++--- .../src/Services/IDependentService.cs | 9 +++-- .../DependentControllerTests.cs | 39 ++----------------- .../Services.Test/DependentServiceTests.cs | 11 ++---- 5 files changed, 20 insertions(+), 62 deletions(-) diff --git a/Apps/GatewayApi/src/Controllers/DependentController.cs b/Apps/GatewayApi/src/Controllers/DependentController.cs index 5495635658..1c34d363aa 100644 --- a/Apps/GatewayApi/src/Controllers/DependentController.cs +++ b/Apps/GatewayApi/src/Controllers/DependentController.cs @@ -114,10 +114,9 @@ public async Task> AddDependent([FromBody] AddDepe /// /// Deletes a dependent from the database. /// - /// The http status. + /// An empty dependent model wrapped in a RequestResult. /// The Delegate hdid. /// The Dependent hdid. - /// The dependent model object to be deleted. /// to manage the async request. /// The Dependent record was deleted. /// The request is invalid. @@ -130,15 +129,9 @@ public async Task> AddDependent([FromBody] AddDepe [HttpDelete] [Authorize(Policy = UserProfilePolicy.Write)] [Route("{hdid}/[controller]/{dependentHdid}")] - public async Task>> Delete(string hdid, string dependentHdid, [FromBody] DependentModel dependent, CancellationToken ct) + public async Task>> Delete(string hdid, string dependentHdid, CancellationToken ct) { - if (dependent.OwnerId != dependentHdid || dependent.DelegateId != hdid) - { - this.logger.LogError("Parameters do not match body of delete"); - return new BadRequestResult(); - } - - return await this.dependentService.RemoveAsync(dependent, ct); + return await this.dependentService.RemoveAsync(hdid, dependentHdid, ct); } } } diff --git a/Apps/GatewayApi/src/Services/DependentService.cs b/Apps/GatewayApi/src/Services/DependentService.cs index 94ac69de2f..63cad8101e 100644 --- a/Apps/GatewayApi/src/Services/DependentService.cs +++ b/Apps/GatewayApi/src/Services/DependentService.cs @@ -239,12 +239,12 @@ public async Task>> GetDependent } /// - public async Task> RemoveAsync(DependentModel dependent, CancellationToken ct = default) + public async Task> RemoveAsync(string delegateHdid, string dependentHdid, CancellationToken ct = default) { - ResourceDelegate? resourceDelegate = (await this.resourceDelegateDelegate.GetAsync(dependent.DelegateId, 0, 500, ct)).FirstOrDefault(d => d.ResourceOwnerHdid == dependent.OwnerId); + ResourceDelegate? resourceDelegate = (await this.resourceDelegateDelegate.GetAsync(delegateHdid, 0, 500, ct)).FirstOrDefault(d => d.ResourceOwnerHdid == dependentHdid); if (resourceDelegate == null) { - throw new NotFoundException($"Dependent {dependent.OwnerId} not found for delegate {dependent.DelegateId}"); + throw new NotFoundException($"Dependent {dependentHdid} not found for delegate {delegateHdid}"); } // commit to the database if change feed is disabled; if change feed enabled, commit will happen when message sender is called @@ -255,11 +255,11 @@ public async Task> RemoveAsync(DependentModel depe throw new DatabaseException(dbDependent.Message); } - await this.UpdateNotificationSettingsAsync(dependent.OwnerId, dependent.DelegateId, true, ct); + await this.UpdateNotificationSettingsAsync(dependentHdid, delegateHdid, true, ct); if (this.dependentsChangeFeedEnabled) { - await this.messageSender.SendAsync(new[] { new MessageEnvelope(new DependentRemovedEvent(dependent.DelegateId, dependent.OwnerId), dependent.DelegateId) }, ct); + await this.messageSender.SendAsync([new MessageEnvelope(new DependentRemovedEvent(delegateHdid, dependentHdid), delegateHdid)], ct); } return RequestResultFactory.Success(new DependentModel()); diff --git a/Apps/GatewayApi/src/Services/IDependentService.cs b/Apps/GatewayApi/src/Services/IDependentService.cs index 5bcf2ba698..59ea003c31 100644 --- a/Apps/GatewayApi/src/Services/IDependentService.cs +++ b/Apps/GatewayApi/src/Services/IDependentService.cs @@ -58,11 +58,12 @@ public interface IDependentService Task> AddDependentAsync(string delegateHdid, AddDependentRequest addDependentRequest, CancellationToken ct = default); /// - /// Removes a dependent delegate relation. + /// Removes a dependent-delegate relation. /// - /// The dependent model to be deleted. + /// The HDID of the delegate. + /// The HDID of the dependent. /// to manage the async request. - /// A dependent model wrapped in a RequestResult. - Task> RemoveAsync(DependentModel dependent, CancellationToken ct = default); + /// An empty dependent model wrapped in a RequestResult. + Task> RemoveAsync(string delegateHdid, string dependentHdid, CancellationToken ct = default); } } diff --git a/Apps/GatewayApi/test/unit/Controllers.Test/DependentControllerTests.cs b/Apps/GatewayApi/test/unit/Controllers.Test/DependentControllerTests.cs index 4f0f2c5e3c..92ccefccfa 100644 --- a/Apps/GatewayApi/test/unit/Controllers.Test/DependentControllerTests.cs +++ b/Apps/GatewayApi/test/unit/Controllers.Test/DependentControllerTests.cs @@ -122,16 +122,15 @@ public async Task ShouldDeleteDependent() { string delegateId = this.hdid; string dependentId = "123"; - DependentModel dependentModel = new() { DelegateId = delegateId, OwnerId = dependentId }; RequestResult expectedResult = new() { - ResourcePayload = dependentModel, + ResourcePayload = new(), ResultStatus = ResultType.Success, }; Mock dependentServiceMock = new(); - dependentServiceMock.Setup(s => s.RemoveAsync(dependentModel, CancellationToken.None)).ReturnsAsync(expectedResult); + dependentServiceMock.Setup(s => s.RemoveAsync(delegateId, dependentId, CancellationToken.None)).ReturnsAsync(expectedResult); Mock httpContextAccessorMock = CreateValidHttpContext(this.token, this.userId, this.hdid); @@ -139,42 +138,10 @@ public async Task ShouldDeleteDependent() new Mock>().Object, dependentServiceMock.Object, httpContextAccessorMock.Object); - ActionResult> actualResult = await dependentController.Delete(delegateId, dependentId, dependentModel, CancellationToken.None); + ActionResult> actualResult = await dependentController.Delete(delegateId, dependentId, CancellationToken.None); actualResult.Value.ShouldDeepEqual(expectedResult); } - /// - /// DeleteDependent - BadRequest path scenario. - /// - /// A representing the asynchronous unit test. - [Fact] - public async Task ShouldFailDeleteDependent() - { - string delegateId = this.hdid; - string dependentId = "123"; - DependentModel dependentModel = new() { DelegateId = delegateId, OwnerId = dependentId }; - - RequestResult expectedResult = new() - { - ResourcePayload = dependentModel, - ResultStatus = ResultType.Success, - }; - - Mock dependentServiceMock = new(); - dependentServiceMock.Setup(s => s.RemoveAsync(dependentModel, CancellationToken.None)).ReturnsAsync(expectedResult); - - Mock httpContextAccessorMock = CreateValidHttpContext(this.token, this.userId, delegateId); - - DependentController dependentController = new( - new Mock>().Object, - dependentServiceMock.Object, - httpContextAccessorMock.Object); - - ActionResult> actualResult = await dependentController.Delete("anotherId", "wrongId", dependentModel, CancellationToken.None); - - Assert.IsType(actualResult.Result); - } - private static IEnumerable GetMockDependents() { List dependentModels = []; diff --git a/Apps/GatewayApi/test/unit/Services.Test/DependentServiceTests.cs b/Apps/GatewayApi/test/unit/Services.Test/DependentServiceTests.cs index 85d5df6a53..a4b26cdd66 100644 --- a/Apps/GatewayApi/test/unit/Services.Test/DependentServiceTests.cs +++ b/Apps/GatewayApi/test/unit/Services.Test/DependentServiceTests.cs @@ -410,7 +410,6 @@ public async Task ValidateAddProtectedDependentNotAllowed() [Fact] public async Task ValidateRemove() { - DependentModel delegateModel = new() { OwnerId = this.mockHdId, DelegateId = this.mockParentHdid }; IList resourceDelegates = new[] { new ResourceDelegate { ProfileHdid = this.mockParentHdid, ResourceOwnerHdid = this.mockHdId } }; DbResult dbResult = new() { @@ -418,7 +417,7 @@ public async Task ValidateRemove() }; IDependentService service = this.SetupMockForRemoveDependent(resourceDelegates, dbResult); - RequestResult actualResult = await service.RemoveAsync(delegateModel); + RequestResult actualResult = await service.RemoveAsync(this.mockParentHdid, this.mockHdId); Assert.Equal(ResultType.Success, actualResult.ResultStatus); } @@ -426,12 +425,11 @@ public async Task ValidateRemove() [Fact] public async Task ValidateRemoveNotFound() { - DependentModel delegateModel = new() { OwnerId = this.mockHdId, DelegateId = this.mockParentHdid }; IDependentService service = this.SetupMockForRemoveDependent([], new()); async Task Actual() { - await service.RemoveAsync(delegateModel); + await service.RemoveAsync(this.mockParentHdid, this.mockHdId); } await Assert.ThrowsAsync(Actual); @@ -440,8 +438,7 @@ async Task Actual() [Fact] public async Task ValidateRemoveDatabaseException() { - DependentModel delegateModel = new() { OwnerId = this.mockHdId, DelegateId = this.mockParentHdid }; - IList resourceDelegates = new[] { new ResourceDelegate { ProfileHdid = this.mockParentHdid, ResourceOwnerHdid = this.mockHdId } }; + IList resourceDelegates = [new ResourceDelegate { ProfileHdid = this.mockParentHdid, ResourceOwnerHdid = this.mockHdId }]; DbResult dbResult = new() { Status = DbStatusCode.Error, @@ -450,7 +447,7 @@ public async Task ValidateRemoveDatabaseException() async Task Actual() { - await service.RemoveAsync(delegateModel); + await service.RemoveAsync(this.mockParentHdid, this.mockHdId); } await Assert.ThrowsAsync(Actual);