diff --git a/src/IIIFPresentation/API.Tests/Integration/ModifyCollectionTests.cs b/src/IIIFPresentation/API.Tests/Integration/ModifyCollectionTests.cs index d791b2c..dac7f05 100644 --- a/src/IIIFPresentation/API.Tests/Integration/ModifyCollectionTests.cs +++ b/src/IIIFPresentation/API.Tests/Integration/ModifyCollectionTests.cs @@ -464,4 +464,114 @@ public async Task UpdateCollection_FailsToUpdateCollection_WhenCalledWithoutNeed // Assert response.StatusCode.Should().Be(HttpStatusCode.Forbidden); } + + [Fact] + public async Task DeleteCollection_DeletesCollection_WhenAllValuesProvided() + { + // Arrange + var initialCollection = new Collection() + { + Id = "DeleteTester", + Slug = "delete-test", + UsePath = true, + Label = new LanguageMap + { + { "en", new List { "update testing" } } + }, + Thumbnail = "some/location", + Created = DateTime.UtcNow, + Modified = DateTime.UtcNow, + CreatedBy = "admin", + Tags = "some, tags", + IsStorageCollection = true, + IsPublic = false, + CustomerId = 1, + Parent = "RootStorage" + }; + + await dbContext.Collections.AddAsync(initialCollection); + await dbContext.SaveChangesAsync(); + + + var deleteRequestMessage = HttpRequestMessageBuilder.GetPrivateRequest(HttpMethod.Delete, + $"{Customer}/collections/{initialCollection.Id}"); + + // Act + var response = await httpClient.AsCustomer(1).SendAsync(deleteRequestMessage); + + var fromDatabase = dbContext.Collections.FirstOrDefault(c => c.Id == initialCollection.Id); + + // Assert + response.StatusCode.Should().Be(HttpStatusCode.NoContent); + fromDatabase.Should().BeNull(); + } + + [Fact] + public async Task DeleteCollection_FailsToDeleteCollection_WhenNotFound() + { + // Arrange + var deleteRequestMessage = HttpRequestMessageBuilder.GetPrivateRequest(HttpMethod.Delete, + $"{Customer}/collections/doesNotExist"); + + // Act + var response = await httpClient.AsCustomer(1).SendAsync(deleteRequestMessage); + + // Assert + response.StatusCode.Should().Be(HttpStatusCode.NotFound); + } + + [Fact] + public async Task DeleteCollection_FailsToDeleteCollection_WhenAttemptingToDeleteRoot() + { + // Arrange + var deleteRequestMessage = HttpRequestMessageBuilder.GetPrivateRequest(HttpMethod.Delete, + $"{Customer}/collections/root"); + + // Act + var response = await httpClient.AsCustomer(1).SendAsync(deleteRequestMessage); + + var errorResponse = await response.ReadAsPresentationResponseAsync(); + + // Assert + response.StatusCode.Should().Be(HttpStatusCode.BadRequest); + errorResponse!.ErrorTypeUri.Should().Be("http://localhost/errors/DeleteCollectionType/CannotDeleteRootCollection"); + errorResponse.Detail.Should().Be("Cannot delete a root collection"); + } + + + [Fact] + public async Task DeleteCollection_FailsToDeleteCollection_WhenAttemptingToDeleteRootDirectly() + { + // Arrange + var deleteRequestMessage = HttpRequestMessageBuilder.GetPrivateRequest(HttpMethod.Delete, + $"{Customer}/collections/RootStorage"); + + // Act + var response = await httpClient.AsCustomer(1).SendAsync(deleteRequestMessage); + + var errorResponse = await response.ReadAsPresentationResponseAsync(); + + // Assert + response.StatusCode.Should().Be(HttpStatusCode.BadRequest); + errorResponse!.ErrorTypeUri.Should().Be("http://localhost/errors/DeleteCollectionType/CannotDeleteRootCollection"); + errorResponse.Detail.Should().Be("Cannot delete a root collection"); + } + + [Fact] + public async Task DeleteCollection_FailsToDeleteCollection_WhenAttemptingToDeleteCollectionWithItems() + { + // Arrange + var deleteRequestMessage = HttpRequestMessageBuilder.GetPrivateRequest(HttpMethod.Delete, + $"{Customer}/collections/FirstChildCollection"); + + // Act + var response = await httpClient.AsCustomer(1).SendAsync(deleteRequestMessage); + + var errorResponse = await response.ReadAsPresentationResponseAsync(); + + // Assert + response.StatusCode.Should().Be(HttpStatusCode.BadRequest); + errorResponse!.ErrorTypeUri.Should().Be("http://localhost/errors/DeleteCollectionType/CollectionNotEmpty"); + errorResponse.Detail.Should().Be("Cannot delete a collection with child items"); + } } \ No newline at end of file diff --git a/src/IIIFPresentation/API/Features/Storage/Requests/DeleteCollection.cs b/src/IIIFPresentation/API/Features/Storage/Requests/DeleteCollection.cs new file mode 100644 index 0000000..4c1fd63 --- /dev/null +++ b/src/IIIFPresentation/API/Features/Storage/Requests/DeleteCollection.cs @@ -0,0 +1,71 @@ +using Core; +using MediatR; +using Microsoft.EntityFrameworkCore; +using Models.API.General; +using Repository; + +namespace API.Features.Storage.Requests; + +public class DeleteCollection (int customerId, string collectionId) : IRequest> +{ + public int CustomerId { get; } = customerId; + + public string CollectionId { get; } = collectionId; +} + +public class DeleteCollectionHandler( + PresentationContext dbContext, + ILogger logger) + : IRequestHandler> +{ + private const string RootCollection = "root"; + + public async Task> Handle(DeleteCollection request, CancellationToken cancellationToken) + { + logger.LogDebug("Deleting collection {CollectionId} for customer {CustomerId}", request.CollectionId, + request.CustomerId); + + if (request.CollectionId.Equals(RootCollection, StringComparison.OrdinalIgnoreCase)) + { + return new ResultMessage(DeleteResult.BadRequest, + DeleteCollectionType.CannotDeleteRootCollection, "Cannot delete a root collection"); + } + + var collection = await dbContext.Collections.FirstOrDefaultAsync( + c => c.Id == request.CollectionId && c.CustomerId == request.CustomerId, + cancellationToken: cancellationToken); + + if (collection is null) return new ResultMessage(DeleteResult.NotFound); + + if (collection.Parent is null) + { + return new ResultMessage(DeleteResult.BadRequest, + DeleteCollectionType.CannotDeleteRootCollection, "Cannot delete a root collection"); + } + + var hasItems = await dbContext.Collections.AnyAsync( + c => c.CustomerId == request.CustomerId && c.Parent == collection.Id, + cancellationToken: cancellationToken); + + if (hasItems) + { + return new ResultMessage(DeleteResult.BadRequest, + DeleteCollectionType.CollectionNotEmpty, "Cannot delete a collection with child items"); + } + + dbContext.Collections.Remove(collection); + try + { + await dbContext.SaveChangesAsync(cancellationToken); + } + catch (DbUpdateConcurrencyException ex) + { + logger.LogError(ex, "Error attempting to delete collection {CollectionId} for customer {CustomerId}", + request.CollectionId, request.CustomerId); + return new ResultMessage(DeleteResult.Error, + DeleteCollectionType.Unknown, "Error deleting collection"); + } + + return new ResultMessage(DeleteResult.Deleted); + } +} \ No newline at end of file diff --git a/src/IIIFPresentation/API/Features/Storage/StorageController.cs b/src/IIIFPresentation/API/Features/Storage/StorageController.cs index 4249e36..1424cc8 100644 --- a/src/IIIFPresentation/API/Features/Storage/StorageController.cs +++ b/src/IIIFPresentation/API/Features/Storage/StorageController.cs @@ -27,7 +27,7 @@ public async Task GetHierarchicalCollection(int customerId, strin { var storageRoot = await Mediator.Send(new GetHierarchicalCollection(customerId, slug)); - if (storageRoot.Collection is not { IsPublic: true }) return NotFound(); + if (storageRoot.Collection is not { IsPublic: true }) return this.PresentationNotFound(); if (Request.ShowExtraProperties()) { @@ -51,7 +51,7 @@ public async Task Get(int customerId, string id, int? page = 1, i var storageRoot = await Mediator.Send(new GetCollection(customerId, id, page.Value, pageSize.Value, orderBy, descending)); - if (storageRoot.Collection == null) return NotFound(); + if (storageRoot.Collection == null) return this.PresentationNotFound(); if (Request.ShowExtraProperties()) { @@ -59,7 +59,7 @@ public async Task Get(int customerId, string id, int? page = 1, i storageRoot.TotalItems, storageRoot.Items, orderByField)); } - return SeeOther(storageRoot.Collection.GenerateHierarchicalCollectionId(GetUrlRoots())); ; + return SeeOther(storageRoot.Collection.GenerateHierarchicalCollectionId(GetUrlRoots())); } [HttpPost("collections")] @@ -69,7 +69,7 @@ public async Task Post(int customerId, [FromBody] UpsertFlatColle { if (!Request.ShowExtraProperties()) { - return Problem(statusCode: (int)HttpStatusCode.Forbidden); + return this.PresentationProblem(statusCode: (int)HttpStatusCode.Forbidden); } var validation = await validator.ValidateAsync(collection); @@ -89,7 +89,7 @@ public async Task Put(int customerId, string id, [FromBody] Upser { if (!Request.ShowExtraProperties()) { - return Problem(statusCode: (int)HttpStatusCode.Forbidden); + return this.PresentationProblem(statusCode: (int)HttpStatusCode.Forbidden); } var validation = await validator.ValidateAsync(collection); @@ -102,6 +102,17 @@ public async Task Put(int customerId, string id, [FromBody] Upser return await HandleUpsert(new UpdateCollection(customerId, id, collection, GetUrlRoots())); } + [HttpDelete("collections/{id}")] + public async Task Delete(int customerId, string id) + { + if (!Request.ShowExtraProperties()) + { + return this.PresentationProblem(statusCode: (int)HttpStatusCode.Forbidden); + } + + return await HandleDelete(new DeleteCollection(customerId, id)); + } + private IActionResult SeeOther(string location) { Response.Headers.Location = location; diff --git a/src/IIIFPresentation/API/Infrastructure/ControllerBaseX.cs b/src/IIIFPresentation/API/Infrastructure/ControllerBaseX.cs index ebba5da..043ecdc 100644 --- a/src/IIIFPresentation/API/Infrastructure/ControllerBaseX.cs +++ b/src/IIIFPresentation/API/Infrastructure/ControllerBaseX.cs @@ -6,6 +6,7 @@ using Core.Helpers; using FluentValidation.Results; using Microsoft.AspNetCore.Mvc; +using Models.API.General; namespace API.Infrastructure; @@ -27,17 +28,17 @@ public static IActionResult FetchResultToHttpResult(this ControllerBase contr where T : class { if (entityResult.Error) - return new ObjectResult(entityResult.ErrorMessage) - { - StatusCode = 500 - }; + { + return controller.PresentationProblem(detail: entityResult.ErrorMessage, + statusCode: (int)HttpStatusCode.InternalServerError); + } - if (entityResult.EntityNotFound || entityResult.Entity == null) return new NotFoundResult(); + if (entityResult.EntityNotFound || entityResult.Entity == null) return controller.PresentationNotFound(); return controller.Ok(entityResult.Entity); } - - /// + + /// /// Create an IActionResult from specified ModifyEntityResult{T}. /// This will be the model + 200/201 on success. Or an /// error and appropriate status code if failed. @@ -63,15 +64,15 @@ public static IActionResult ModifyResultToHttpResult(this ControllerBase cont WriteResult.Updated => controller.Ok(entityResult.Entity), WriteResult.Created => controller.Created(controller.Request.GetDisplayUrl(), entityResult.Entity), WriteResult.NotFound => controller.NotFound(entityResult.Error), - WriteResult.Error => controller.Problem(entityResult.Error, instance, (int)HttpStatusCode.InternalServerError, errorTitle), - WriteResult.BadRequest => controller.Problem(entityResult.Error, instance, (int)HttpStatusCode.BadRequest, errorTitle), - WriteResult.Conflict => controller.Problem(entityResult.Error, instance, (int)HttpStatusCode.Conflict, + WriteResult.Error => controller.PresentationProblem(entityResult.Error, instance, (int)HttpStatusCode.InternalServerError, errorTitle), + WriteResult.BadRequest => controller.PresentationProblem(entityResult.Error, instance, (int)HttpStatusCode.BadRequest, errorTitle), + WriteResult.Conflict => controller.PresentationProblem(entityResult.Error, instance, (int)HttpStatusCode.Conflict, $"{errorTitle}: Conflict"), - WriteResult.FailedValidation => controller.Problem(entityResult.Error, instance, (int)HttpStatusCode.BadRequest, + WriteResult.FailedValidation => controller.PresentationProblem(entityResult.Error, instance, (int)HttpStatusCode.BadRequest, $"{errorTitle}: Validation failed"), - WriteResult.StorageLimitExceeded => controller.Problem(entityResult.Error, instance, (int)HttpStatusCode.InsufficientStorage, + WriteResult.StorageLimitExceeded => controller.PresentationProblem(entityResult.Error, instance, (int)HttpStatusCode.InsufficientStorage, $"{errorTitle}: Storage limit exceeded"), - _ => controller.Problem(entityResult.Error, instance, (int)HttpStatusCode.InternalServerError, errorTitle), + _ => controller.PresentationProblem(entityResult.Error, instance, (int)HttpStatusCode.InternalServerError, errorTitle), }; /// @@ -105,4 +106,45 @@ public static ObjectResult ValidationFailed(this ControllerBase controller, Vali return orderByField; } + + /// + /// Creates an that produces a response. + /// + /// The value for . + /// The value for . + /// The value for . + /// The value for . + /// The value for . + /// The created for the response. + public static ObjectResult PresentationProblem( + this ControllerBase controller, + string? detail = null, + string? instance = null, + int? statusCode = null, + string? title = null, + string? type = null) + { + var error = new Error + { + Detail = detail, + Instance = instance ?? controller.Request.GetDisplayUrl(), + Status = statusCode ?? 500, + Title = title, + ErrorTypeUri = type + }; + + return new ObjectResult(error) + { + StatusCode = error.Status + }; + } + + /// + /// Creates an that produces a response with 404 status code. + /// + /// The created for the response. + public static ObjectResult PresentationNotFound(this ControllerBase controller, string? detail = null) + { + return controller.PresentationProblem(detail, null, (int)HttpStatusCode.NotFound, "Not Found"); + } } \ No newline at end of file diff --git a/src/IIIFPresentation/API/Infrastructure/PresentationController.cs b/src/IIIFPresentation/API/Infrastructure/PresentationController.cs index cc43bec..8317dbf 100644 --- a/src/IIIFPresentation/API/Infrastructure/PresentationController.cs +++ b/src/IIIFPresentation/API/Infrastructure/PresentationController.cs @@ -1,10 +1,12 @@ -using API.Converters; +using System.Net; +using API.Converters; using API.Exceptions; using API.Infrastructure.Requests; using API.Settings; using Core; using MediatR; using Microsoft.AspNetCore.Mvc; +using Models.API.General; namespace API.Infrastructure; @@ -81,7 +83,7 @@ protected async Task HandleUpsert( /// /// This will be replaced with overload that takes DeleteEntityResult in future protected async Task HandleDelete( - IRequest> request, + IRequest> request, string? errorTitle = "Delete failed", CancellationToken cancellationToken = default) { @@ -89,7 +91,7 @@ protected async Task HandleDelete( { var result = await Mediator.Send(request, cancellationToken); - return ConvertDeleteToHttp(result.Value, result.Message); + return ConvertDeleteToHttp(result.Value, result.Message, result.Type); }, errorTitle); } @@ -114,23 +116,26 @@ protected async Task HandleDelete( { var result = await Mediator.Send(request, cancellationToken); - return ConvertDeleteToHttp(result.Value, result.Message); + return ConvertDeleteToHttp(result.Value, result.Message, result.Type); }, errorTitle); } - private IActionResult ConvertDeleteToHttp(DeleteResult result, string? message) + private IActionResult ConvertDeleteToHttp(DeleteResult result, string? message, TType type) { - // Note: this is temporary until DeleteResult used for all deletions return result switch { - DeleteResult.NotFound => NotFound(), - DeleteResult.Conflict => new ObjectResult(message) { StatusCode = 409 }, - DeleteResult.Error => new ObjectResult(message) { StatusCode = 500 }, + DeleteResult.NotFound => this.PresentationNotFound(), + DeleteResult.Conflict => this.PresentationProblem(detail: message, type: GetErrorType(type), statusCode: (int)HttpStatusCode.Conflict, title: "Conflict"), + DeleteResult.Error => this.PresentationProblem(detail: message, type: GetErrorType(type), statusCode: (int)HttpStatusCode.InternalServerError, title: "Internal Server Error"), + DeleteResult.BadRequest => this.PresentationProblem(detail: message, type: GetErrorType(type), statusCode: (int)HttpStatusCode.BadRequest, title: "Bad Request"), DeleteResult.Deleted => NoContent(), _ => throw new ArgumentOutOfRangeException(nameof(DeleteResult), $"No deletion value of {result}") }; } + private string GetErrorType(TType type) => $"{GetUrlRoots().BaseUrl}/errors/{type?.GetType().Name}/{type}"; + + /// /// Handle a GET request - this takes a IRequest which returns a FetchEntityResult{T}. /// The request is sent and result is transformed to an http result. diff --git a/src/IIIFPresentation/API/Infrastructure/Requests/DeleteEntityResult.cs b/src/IIIFPresentation/API/Infrastructure/Requests/DeleteEntityResult.cs index e97bafe..b905ee7 100644 --- a/src/IIIFPresentation/API/Infrastructure/Requests/DeleteEntityResult.cs +++ b/src/IIIFPresentation/API/Infrastructure/Requests/DeleteEntityResult.cs @@ -3,26 +3,28 @@ namespace API.Infrastructure.Requests; /// -/// Represents the result of a request to delete an entity +/// Represents the result of a request to delete an entity /// public class DeleteEntityResult : IModifyRequest { /// - /// The associated value. + /// The associated value. /// public DeleteResult Value { get; private init; } /// - /// The message related to the result + /// The message related to the result /// public string? Message { get; private init; } + + public string? Type { get; private init; } public static DeleteEntityResult Success => new() { Value = DeleteResult.Deleted }; public bool IsSuccess => Value == DeleteResult.Deleted; - public static DeleteEntityResult Failure(string message, DeleteResult result) + public static DeleteEntityResult Failure(string message, DeleteResult result, string type) { - return new DeleteEntityResult { Message = message, Value = result }; + return new DeleteEntityResult { Message = message, Value = result, Type = type}; } } \ No newline at end of file diff --git a/src/IIIFPresentation/Core/DeleteResult.cs b/src/IIIFPresentation/Core/DeleteResult.cs index e6889c0..1547ad5 100644 --- a/src/IIIFPresentation/Core/DeleteResult.cs +++ b/src/IIIFPresentation/Core/DeleteResult.cs @@ -20,8 +20,13 @@ public enum DeleteResult /// Conflict, + /// + /// There was an internal error deleting + /// + Error, + /// /// There was an error deleting /// - Error + BadRequest } \ No newline at end of file diff --git a/src/IIIFPresentation/Core/ResultMessage.cs b/src/IIIFPresentation/Core/ResultMessage.cs index fc46a8c..8ea18ca 100644 --- a/src/IIIFPresentation/Core/ResultMessage.cs +++ b/src/IIIFPresentation/Core/ResultMessage.cs @@ -1,25 +1,43 @@ namespace Core; -public class ResultMessage +public class ResultMessage { /// - /// Initializes a new instance of the class. + /// Initializes a new instance of the class. /// + /// The value. + /// A type used to generate an error /// A message related to the result + public ResultMessage(T value, TEnum? type, string? message = null) + { + Value = value; + Message = message; + Type = type; + } + + /// + /// Initializes a new instance of the class. + /// /// The value. - public ResultMessage(string message, T value) + /// A message related to the result + public ResultMessage(T value, string? message = null) { Value = value; Message = message; } /// - /// The associated value. + /// The associated value. /// public T Value { get; } /// - /// The message related to the result + /// The message related to the result + /// + public string? Message { get; } + + /// + /// A code associated with the result message /// - public string Message { get; } + public TEnum? Type { get; } } \ No newline at end of file diff --git a/src/IIIFPresentation/Models/API/General/DeleteCollectionType.cs b/src/IIIFPresentation/Models/API/General/DeleteCollectionType.cs new file mode 100644 index 0000000..4cdc34e --- /dev/null +++ b/src/IIIFPresentation/Models/API/General/DeleteCollectionType.cs @@ -0,0 +1,8 @@ +namespace Models.API.General; + +public enum DeleteCollectionType +{ + CannotDeleteRootCollection = 1, + CollectionNotEmpty = 2, + Unknown = 3 +} \ No newline at end of file diff --git a/src/IIIFPresentation/Models/API/General/Error.cs b/src/IIIFPresentation/Models/API/General/Error.cs index 84475bb..7306989 100644 --- a/src/IIIFPresentation/Models/API/General/Error.cs +++ b/src/IIIFPresentation/Models/API/General/Error.cs @@ -1,12 +1,18 @@ using IIIF; +using Newtonsoft.Json; namespace Models.API.General; public class Error : JsonLdBase { - public string Type => "Error"; + public string? Title { get; set; } public string? Detail { get; set; } + public string? Instance { get; set; } + + [JsonProperty("type")] + public string? ErrorTypeUri { get; set; } + public int Status { get; set; } } \ No newline at end of file