From ab502867bbd5ab680fb0fede406f69351bc2d224 Mon Sep 17 00:00:00 2001 From: Cory Gehr Date: Tue, 21 Jul 2020 10:39:03 -0700 Subject: [PATCH 1/4] Add Delete functionality to Service and Repository objects for MessageContainers. --- .../CosmosMessageContainerRepository.cs | 84 +++++++++++++++---- .../CovidSafe.DAL/Repositories/IRepository.cs | 6 ++ .../CovidSafe.DAL/Services/IMessageService.cs | 6 ++ .../CovidSafe.DAL/Services/MessageService.cs | 21 +++++ 4 files changed, 100 insertions(+), 17 deletions(-) diff --git a/CovidSafe/CovidSafe.DAL/Repositories/Cosmos/CosmosMessageContainerRepository.cs b/CovidSafe/CovidSafe.DAL/Repositories/Cosmos/CosmosMessageContainerRepository.cs index 6931d4a..8fd4587 100644 --- a/CovidSafe/CovidSafe.DAL/Repositories/Cosmos/CosmosMessageContainerRepository.cs +++ b/CovidSafe/CovidSafe.DAL/Repositories/Cosmos/CosmosMessageContainerRepository.cs @@ -11,7 +11,6 @@ using CovidSafe.Entities.Messages; using Microsoft.Azure.Cosmos; using Microsoft.Azure.Cosmos.Linq; -using Microsoft.Azure.Cosmos.Spatial; namespace CovidSafe.DAL.Repositories.Cosmos { @@ -32,6 +31,44 @@ public CosmosMessageContainerRepository(CosmosContext dbContext) : base(dbContex ); } + /// + /// Retrieves a object by its unique identifier + /// + /// Unique identifier + /// Cancellation token + /// matching provided identifier, or null + /// + /// Some functions require the Cosmos record attributes of a matching document, whereas + /// by definition returns the object represented + /// by this repository. Hence, this function was created to retrieve the actual + /// implementation to allow examination of the additional attributes. + /// + private async Task _getRecordById(string id, CancellationToken cancellationToken) + { + // Create LINQ query + var queryable = this.Container + .GetItemLinqQueryable(); + + // Execute query + var iterator = queryable + .Where(r => + r.Id == id + && r.Version == MessageContainerRecord.CURRENT_RECORD_VERSION + ) + .ToFeedIterator(); + + List results = new List(); + + while (iterator.HasMoreResults) + { + results.AddRange(await iterator.ReadNextAsync()); + } + + // There should only ever be one result + // This is a semantic of how SELECT works in the LINQ/CosmosDB SDK + return results.FirstOrDefault(); + } + /// /// Returns the most restrictive timestamp filter, based on the application /// configuration and the one provided by the user @@ -59,29 +96,42 @@ private long _getTimestampFilter(long timestampFilter) } /// - public async Task GetAsync(string messageId, CancellationToken cancellationToken = default) + public async Task DeleteAsync(string id, CancellationToken cancellationToken = default) { - // Create LINQ query - var queryable = this.Container - .GetItemLinqQueryable(); + if(String.IsNullOrEmpty(id)) + { + throw new ArgumentNullException(nameof(id)); + } - // Execute query - var iterator = queryable - .Where(r => - r.Id == messageId - && r.Version == MessageContainerRecord.CURRENT_RECORD_VERSION - ) - .Select(r => r.Value) - .ToFeedIterator(); + // Retrieve the record to be deleted + // Necessary to get PartitionKey + MessageContainerRecord toDelete = await this._getRecordById(id, cancellationToken); - List results = new List(); + await this.Container.DeleteItemAsync( + id, + new PartitionKey(toDelete.PartitionKey), + cancellationToken: cancellationToken); + } - while (iterator.HasMoreResults) + /// + public async Task GetAsync(string messageId, CancellationToken cancellationToken = default) + { + if(String.IsNullOrEmpty(messageId)) { - results.AddRange(await iterator.ReadNextAsync()); + throw new ArgumentNullException(nameof(messageId)); } - return results.FirstOrDefault(); + // Get result from private function + MessageContainerRecord record = await this._getRecordById(messageId, cancellationToken); + + if (record != null) + { + return record.Value; + } + else + { + return null; + } } /// diff --git a/CovidSafe/CovidSafe.DAL/Repositories/IRepository.cs b/CovidSafe/CovidSafe.DAL/Repositories/IRepository.cs index 586d256..ffe153a 100644 --- a/CovidSafe/CovidSafe.DAL/Repositories/IRepository.cs +++ b/CovidSafe/CovidSafe.DAL/Repositories/IRepository.cs @@ -10,6 +10,12 @@ namespace CovidSafe.DAL.Repositories /// Type used by the primary key public interface IRepository { + /// + /// Deletes an object matching the provided identifier + /// + /// Unique object identifier + /// Cancellation token + Task DeleteAsync(TT id, CancellationToken cancellationToken = default); /// /// Retrieves an object which matches the provided identifier /// diff --git a/CovidSafe/CovidSafe.DAL/Services/IMessageService.cs b/CovidSafe/CovidSafe.DAL/Services/IMessageService.cs index 92f8f57..d80387b 100644 --- a/CovidSafe/CovidSafe.DAL/Services/IMessageService.cs +++ b/CovidSafe/CovidSafe.DAL/Services/IMessageService.cs @@ -12,6 +12,12 @@ namespace CovidSafe.DAL.Services /// public interface IMessageService : IService { + /// + /// Deletes a based on its unique identifier + /// + /// Identifier of to delete + /// Cancellation token + Task DeleteMessageByIdAsync(string id, CancellationToken cancellationToken = default); /// /// Retrieves a collection of objects by their unique identifiers /// diff --git a/CovidSafe/CovidSafe.DAL/Services/MessageService.cs b/CovidSafe/CovidSafe.DAL/Services/MessageService.cs index f6618e8..db6789e 100644 --- a/CovidSafe/CovidSafe.DAL/Services/MessageService.cs +++ b/CovidSafe/CovidSafe.DAL/Services/MessageService.cs @@ -46,6 +46,27 @@ public MessageService(IMessageContainerRepository messageRepo) this._reportRepo = messageRepo; } + /// + public async Task DeleteMessageByIdAsync(string id, CancellationToken cancellationToken = default) + { + if(String.IsNullOrEmpty(id)) + { + throw new ArgumentNullException(nameof(id)); + } + + // Confirm ID is valid + RequestValidationResult validationStatus = Validator.ValidateGuid(id, nameof(id)); + + if(validationStatus.Passed) + { + await this._reportRepo.DeleteAsync(id, cancellationToken); + } + else + { + throw new RequestValidationFailedException(validationStatus); + } + } + /// public async Task> GetByIdsAsync(IEnumerable ids, CancellationToken cancellationToken = default) { From 0125d57dc22dfda707f4bfa2f067b808f15f3235 Mon Sep 17 00:00:00 2001 From: Cory Gehr Date: Tue, 21 Jul 2020 10:43:43 -0700 Subject: [PATCH 2/4] Add HTTP DELETE to AnnounceController (no need to add new api-version since this doesn't alter existing API calls) --- .../MessageControllers/AnnounceController.cs | 38 ++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/CovidSafe/CovidSafe.API/v20200611/Controllers/MessageControllers/AnnounceController.cs b/CovidSafe/CovidSafe.API/v20200611/Controllers/MessageControllers/AnnounceController.cs index 1acc818..05934ec 100644 --- a/CovidSafe/CovidSafe.API/v20200611/Controllers/MessageControllers/AnnounceController.cs +++ b/CovidSafe/CovidSafe.API/v20200611/Controllers/MessageControllers/AnnounceController.cs @@ -42,13 +42,49 @@ public AnnounceController(IMapper map, IMessageService reportService) this._reportService = reportService; } + /// + /// Deletes a + /// + /// + /// Sample request: + /// + /// DELETE /api/Messages/Announce?messageId=00000000-0000-0000-0000-000000000000&api-version=2020-06-11 + /// + /// + /// Unique to delete + /// Cancellation token (not required in API call) + /// Delete successful + /// Malformed or invalid request + [HttpDelete] + [Consumes("application/x-protobuf", "application/json")] + [ProducesResponseType(StatusCodes.Status200OK)] + [ResponseCache(NoStore = true, Location = ResponseCacheLocation.None)] + public async Task DeleteAsync([Required] string messageId, CancellationToken cancellationToken = default) + { + try + { + // Attempt delete + await this._reportService.DeleteMessageByIdAsync(messageId, cancellationToken); + return Ok(); + } + catch (RequestValidationFailedException ex) + { + // Only return validation issues + return BadRequest(ex.ValidationResult); + } + catch (ArgumentNullException) + { + return BadRequest(); + } + } + /// /// Publish a for distribution among devices /// /// /// Sample request: /// - /// PUT /api/Messages/Announce&api-version=2020-06-11 + /// PUT /api/Messages/Announce?api-version=2020-06-11 /// { /// "userMessage": "Monitor symptoms for one week.", /// "area": { From 4b00d1164915532bc266b5460c2ad4b58f59e422 Mon Sep 17 00:00:00 2001 From: Cory Gehr Date: Tue, 21 Jul 2020 10:44:22 -0700 Subject: [PATCH 3/4] Rename InfectionReportServiceTests to MessageServiceTests --- .../{InfectionReportServiceTests.cs => MessageServiceTests.cs} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename CovidSafe/CovidSafe.DAL.Tests/Services/{InfectionReportServiceTests.cs => MessageServiceTests.cs} (100%) diff --git a/CovidSafe/CovidSafe.DAL.Tests/Services/InfectionReportServiceTests.cs b/CovidSafe/CovidSafe.DAL.Tests/Services/MessageServiceTests.cs similarity index 100% rename from CovidSafe/CovidSafe.DAL.Tests/Services/InfectionReportServiceTests.cs rename to CovidSafe/CovidSafe.DAL.Tests/Services/MessageServiceTests.cs From 8d42c4707fdf27fe4592123bdf4b3f881ee585fa Mon Sep 17 00:00:00 2001 From: Cory Gehr Date: Tue, 21 Jul 2020 11:06:00 -0700 Subject: [PATCH 4/4] Return null if the item is not found when attempting a delete Add unit tests --- .../AnnounceControllerTests.cs | 65 +++++++++++++++ .../MessageControllers/AnnounceController.cs | 5 ++ .../Services/MessageServiceTests.cs | 82 +++++++++++++++++++ .../CosmosMessageContainerRepository.cs | 20 +++-- .../CovidSafe.DAL/Repositories/IRepository.cs | 3 +- .../CovidSafe.DAL/Services/MessageService.cs | 8 +- 6 files changed, 176 insertions(+), 7 deletions(-) diff --git a/CovidSafe/CovidSafe.API.Tests/v20200611/Controllers/MessageControllers/AnnounceControllerTests.cs b/CovidSafe/CovidSafe.API.Tests/v20200611/Controllers/MessageControllers/AnnounceControllerTests.cs index d7921af..23009e3 100644 --- a/CovidSafe/CovidSafe.API.Tests/v20200611/Controllers/MessageControllers/AnnounceControllerTests.cs +++ b/CovidSafe/CovidSafe.API.Tests/v20200611/Controllers/MessageControllers/AnnounceControllerTests.cs @@ -56,6 +56,71 @@ public AnnounceControllerTests() this._controller.ControllerContext.HttpContext = new DefaultHttpContext(); } + /// + /// + /// returns when no objects are provided + /// with request + /// + [TestMethod] + public async Task DeleteAsync_BadRequestObjectWithInvalidMessageId() + { + // Arrange + string invalidId = "this is an invalid ID."; + + // Act + ActionResult controllerResponse = await this._controller + .DeleteAsync(invalidId, CancellationToken.None); + + // Assert + Assert.IsNotNull(controllerResponse); + Assert.IsInstanceOfType(controllerResponse, typeof(BadRequestObjectResult)); + } + + /// + /// + /// returns when no + /// objects are matched by the provided 'messageId' parameter + /// + [TestMethod] + public async Task DeleteAsync_NotFoundWithInvalidMessageId() + { + // Arrange + string unmatchedId = "00000000-0000-0000-0000-000000000001"; + this._repo + .Setup(r => r.DeleteAsync(It.IsAny(), CancellationToken.None)) + .Returns(Task.FromResult(true)); + + // Act + ActionResult controllerResponse = await this._controller + .DeleteAsync(unmatchedId, CancellationToken.None); + + // Assert + Assert.IsNotNull(controllerResponse); + Assert.IsInstanceOfType(controllerResponse, typeof(NotFoundResult)); + } + + /// + /// + /// returns with valid input data + /// + [TestMethod] + public async Task DeleteAsync_OkWithValidInputs() + { + // Arrange + string validId = "00000000-0000-0000-0000-000000000001"; + this._repo + .Setup(r => r.DeleteAsync(validId, CancellationToken.None)) + .Returns(Task.FromResult(true)); + + // Act + ActionResult controllerResponse = await this._controller + .DeleteAsync(validId, CancellationToken.None); + + // Assert + Assert.IsNotNull(controllerResponse); + Assert.IsInstanceOfType(controllerResponse, typeof(OkResult)); + } + /// /// /// returns when no objects are provided diff --git a/CovidSafe/CovidSafe.API/v20200611/Controllers/MessageControllers/AnnounceController.cs b/CovidSafe/CovidSafe.API/v20200611/Controllers/MessageControllers/AnnounceController.cs index 05934ec..d9f00f0 100644 --- a/CovidSafe/CovidSafe.API/v20200611/Controllers/MessageControllers/AnnounceController.cs +++ b/CovidSafe/CovidSafe.API/v20200611/Controllers/MessageControllers/AnnounceController.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.ComponentModel.DataAnnotations; using System.Threading; using System.Threading.Tasks; @@ -67,6 +68,10 @@ public async Task DeleteAsync([Required] string messageId, Cancell await this._reportService.DeleteMessageByIdAsync(messageId, cancellationToken); return Ok(); } + catch (KeyNotFoundException) + { + return NotFound(); + } catch (RequestValidationFailedException ex) { // Only return validation issues diff --git a/CovidSafe/CovidSafe.DAL.Tests/Services/MessageServiceTests.cs b/CovidSafe/CovidSafe.DAL.Tests/Services/MessageServiceTests.cs index cbf5ed3..c9379b3 100644 --- a/CovidSafe/CovidSafe.DAL.Tests/Services/MessageServiceTests.cs +++ b/CovidSafe/CovidSafe.DAL.Tests/Services/MessageServiceTests.cs @@ -8,6 +8,7 @@ using CovidSafe.DAL.Services; using CovidSafe.Entities.Geospatial; using CovidSafe.Entities.Messages; +using CovidSafe.Entities.Validation; using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; @@ -40,6 +41,87 @@ public MessageServiceTests() this._service = new MessageService(this._repo.Object); } + /// + /// + /// throws with empty 'id' parameter + /// + [TestMethod] + [ExpectedException(typeof(ArgumentNullException))] + public async Task DeleteMessageById_ArgumentNullOnEmptyId() + { + // Arrange + // N/A + + // Act + await this._service + .DeleteMessageByIdAsync(null, CancellationToken.None); + + // Assert + // Exception caught by decorator + } + + /// + /// + /// throws with unmatched 'id' parameter + /// + [TestMethod] + [ExpectedException(typeof(KeyNotFoundException))] + public async Task DeleteMessageById_KeyNotFoundOnUnmatchedId() + { + // Arrange + string unmatchedId = "00000000-0000-0000-0000-000000000001"; + this._repo + .Setup(r => r.DeleteAsync(It.IsAny(), CancellationToken.None)) + .Returns(Task.FromResult(false)); + + // Act + await this._service + .DeleteMessageByIdAsync(unmatchedId, CancellationToken.None); + + // Assert + // Exception caught by decorator + } + + /// + /// + /// throws with non-GUID 'id' parameter + /// + [TestMethod] + [ExpectedException(typeof(RequestValidationFailedException))] + public async Task DeleteMessageById_RequestValidationFailedOnInvalidId() + { + // Arrange + string invalidId = "this is not a valid ID"; + + // Act + await this._service + .DeleteMessageByIdAsync(invalidId, CancellationToken.None); + + // Assert + // Exception caught by decorator + } + + /// + /// + /// succeeds with valid 'id' parameter + /// + [TestMethod] + public async Task DeleteMessageById_SucceedsOnValidId() + { + // Arrange + string validId = "00000000-0000-0000-0000-000000000001"; + this._repo + .Setup(r => r.DeleteAsync(validId, CancellationToken.None)) + .Returns(Task.FromResult(true)); + + // Act + await this._service + .DeleteMessageByIdAsync(validId, CancellationToken.None); + + // Assert + // No exceptions should be thrown + } + /// /// /// throws with empty 'ids' parameter diff --git a/CovidSafe/CovidSafe.DAL/Repositories/Cosmos/CosmosMessageContainerRepository.cs b/CovidSafe/CovidSafe.DAL/Repositories/Cosmos/CosmosMessageContainerRepository.cs index 8fd4587..fec20f2 100644 --- a/CovidSafe/CovidSafe.DAL/Repositories/Cosmos/CosmosMessageContainerRepository.cs +++ b/CovidSafe/CovidSafe.DAL/Repositories/Cosmos/CosmosMessageContainerRepository.cs @@ -96,7 +96,7 @@ private long _getTimestampFilter(long timestampFilter) } /// - public async Task DeleteAsync(string id, CancellationToken cancellationToken = default) + public async Task DeleteAsync(string id, CancellationToken cancellationToken = default) { if(String.IsNullOrEmpty(id)) { @@ -107,10 +107,20 @@ public async Task DeleteAsync(string id, CancellationToken cancellationToken = d // Necessary to get PartitionKey MessageContainerRecord toDelete = await this._getRecordById(id, cancellationToken); - await this.Container.DeleteItemAsync( - id, - new PartitionKey(toDelete.PartitionKey), - cancellationToken: cancellationToken); + if(toDelete != null) + { + await this.Container.DeleteItemAsync( + id, + new PartitionKey(toDelete.PartitionKey), + cancellationToken: cancellationToken + ); + + return true; + } + else + { + return false; + } } /// diff --git a/CovidSafe/CovidSafe.DAL/Repositories/IRepository.cs b/CovidSafe/CovidSafe.DAL/Repositories/IRepository.cs index ffe153a..c548fe8 100644 --- a/CovidSafe/CovidSafe.DAL/Repositories/IRepository.cs +++ b/CovidSafe/CovidSafe.DAL/Repositories/IRepository.cs @@ -15,7 +15,8 @@ public interface IRepository /// /// Unique object identifier /// Cancellation token - Task DeleteAsync(TT id, CancellationToken cancellationToken = default); + /// True if record was found and deleted, false if no matching record was found + Task DeleteAsync(TT id, CancellationToken cancellationToken = default); /// /// Retrieves an object which matches the provided identifier /// diff --git a/CovidSafe/CovidSafe.DAL/Services/MessageService.cs b/CovidSafe/CovidSafe.DAL/Services/MessageService.cs index db6789e..e253a8d 100644 --- a/CovidSafe/CovidSafe.DAL/Services/MessageService.cs +++ b/CovidSafe/CovidSafe.DAL/Services/MessageService.cs @@ -59,7 +59,13 @@ public async Task DeleteMessageByIdAsync(string id, CancellationToken cancellati if(validationStatus.Passed) { - await this._reportRepo.DeleteAsync(id, cancellationToken); + bool result = await this._reportRepo.DeleteAsync(id, cancellationToken); + + if(!result) + { + // No matching records, throw exception to signal this + throw new KeyNotFoundException(); + } } else {