From 36d0f2a4e05288c6353eb50db933e91372e8a8c6 Mon Sep 17 00:00:00 2001 From: "jack.lewis" Date: Wed, 16 Oct 2024 16:03:06 +0100 Subject: [PATCH] fixing code review comments --- .../API.Tests/Integration/ModifyCollectionTests.cs | 2 +- .../API/Features/Storage/Helpers/ErrorHelper.cs | 4 ++-- .../Features/Storage/Requests/CreateCollection.cs | 14 +++++++------- .../Storage/Requests/GetHierarchicalCollection.cs | 2 +- .../Storage/Requests/PostHierarchicalCollection.cs | 4 ++-- .../API/Features/Storage/StorageController.cs | 2 +- .../API/Helpers/CollectionHelperX.cs | 3 +++ 7 files changed, 17 insertions(+), 14 deletions(-) diff --git a/src/IIIFPresentation/API.Tests/Integration/ModifyCollectionTests.cs b/src/IIIFPresentation/API.Tests/Integration/ModifyCollectionTests.cs index 7e94bd4..d9424c5 100644 --- a/src/IIIFPresentation/API.Tests/Integration/ModifyCollectionTests.cs +++ b/src/IIIFPresentation/API.Tests/Integration/ModifyCollectionTests.cs @@ -181,7 +181,7 @@ public async Task CreateCollection_ReturnsError_WhenIsStorageCollectionFalseAndU // Assert response.StatusCode.Should().Be(HttpStatusCode.BadRequest); - error!.Detail.Should().Be("Error attempting to validate collection is IIIF"); + error!.Detail.Should().Be("An error occurred while attempting to validate the collection as IIIF"); } [Fact] diff --git a/src/IIIFPresentation/API/Features/Storage/Helpers/ErrorHelper.cs b/src/IIIFPresentation/API/Features/Storage/Helpers/ErrorHelper.cs index 2853e80..cbaa373 100644 --- a/src/IIIFPresentation/API/Features/Storage/Helpers/ErrorHelper.cs +++ b/src/IIIFPresentation/API/Features/Storage/Helpers/ErrorHelper.cs @@ -25,7 +25,7 @@ public static ModifyEntityResult CannotValida where TCollection : class { return ModifyEntityResult.Failure( - "Error attempting to validate collection is IIIF", ModifyCollectionType.CannotValidateIIIF, - WriteResult.BadRequest); + "An error occurred while attempting to validate the collection as IIIF", + ModifyCollectionType.CannotValidateIIIF, WriteResult.BadRequest); } } \ No newline at end of file diff --git a/src/IIIFPresentation/API/Features/Storage/Requests/CreateCollection.cs b/src/IIIFPresentation/API/Features/Storage/Requests/CreateCollection.cs index 17e27fb..c69e682 100644 --- a/src/IIIFPresentation/API/Features/Storage/Requests/CreateCollection.cs +++ b/src/IIIFPresentation/API/Features/Storage/Requests/CreateCollection.cs @@ -94,7 +94,7 @@ public async Task(); } } @@ -114,7 +114,7 @@ await dbContext.TrySaveCollection(request.CustomerId, lo return saveErrors; } - await UploadToS3IfRequiredAsync(request, collection.Id, convertedIIIFCollection!, cancellationToken); + await UploadToS3IfRequiredAsync(request.Collection, collection, convertedIIIFCollection!, cancellationToken); if (collection.Parent != null) { @@ -130,7 +130,7 @@ private static string ConvertToIIIFCollection(CreateCollection request, Collecti { var collectionAsIIIF = request.RawRequestBody.FromJson(); var convertedIIIFCollection = collectionAsIIIF.AsJson(); - var thumbnails = collectionAsIIIF.Thumbnail?.Where(c => c is Image).Select(c => (Image)c).ToList(); + var thumbnails = collectionAsIIIF.Thumbnail?.OfType().ToList(); if (thumbnails != null) { collection.Thumbnail = thumbnails.GetThumbnailPath(); @@ -138,14 +138,14 @@ private static string ConvertToIIIFCollection(CreateCollection request, Collecti return convertedIIIFCollection; } - private async Task UploadToS3IfRequiredAsync(CreateCollection request, - string id, string convertedIIIFCollection, CancellationToken cancellationToken) + private async Task UploadToS3IfRequiredAsync(UpsertFlatCollection request, + Collection collection, string convertedIIIFCollection, CancellationToken cancellationToken) { - if (!request.Collection.Behavior.IsStorageCollection()) + if (!request.Behavior.IsStorageCollection()) { await bucketWriter.WriteToBucket( new ObjectInBucket(settings.AWS.S3.StorageBucket, - $"{request.CustomerId}/collections/{id}"), + collection.GetCollectionBucketKey()), convertedIIIFCollection, "application/json", cancellationToken); } } diff --git a/src/IIIFPresentation/API/Features/Storage/Requests/GetHierarchicalCollection.cs b/src/IIIFPresentation/API/Features/Storage/Requests/GetHierarchicalCollection.cs index 07f5ffb..5f086e0 100644 --- a/src/IIIFPresentation/API/Features/Storage/Requests/GetHierarchicalCollection.cs +++ b/src/IIIFPresentation/API/Features/Storage/Requests/GetHierarchicalCollection.cs @@ -40,7 +40,7 @@ public async Task Handle(GetHierarchicalCollection request, if (!collection.IsStorageCollection) { var objectFromS3 = await bucketReader.GetObjectFromBucket(new ObjectInBucket(settings.S3.StorageBucket, - $"{request.CustomerId}/collections/{collection.Id}"), cancellationToken); + collection.GetCollectionBucketKey()), cancellationToken); if (!objectFromS3.Stream.IsNull()) { diff --git a/src/IIIFPresentation/API/Features/Storage/Requests/PostHierarchicalCollection.cs b/src/IIIFPresentation/API/Features/Storage/Requests/PostHierarchicalCollection.cs index e1e952a..40649ed 100644 --- a/src/IIIFPresentation/API/Features/Storage/Requests/PostHierarchicalCollection.cs +++ b/src/IIIFPresentation/API/Features/Storage/Requests/PostHierarchicalCollection.cs @@ -76,7 +76,7 @@ await dbContext.TrySaveCollection(request.CustomerId, logger, await bucketWriter.WriteToBucket( new ObjectInBucket(settings.AWS.S3.StorageBucket, - $"{request.CustomerId}/collections/{collection.Id}"), + collection.GetCollectionBucketKey()), collectionFromBody.AsJson(), "application/json", cancellationToken); if (collection.Parent != null) @@ -120,7 +120,7 @@ private static DatabaseCollection.Collection CreateDatabaseCollection(PostHierar } catch (Exception ex) { - logger.LogError(ex, "Error attempting to validate collection is IIIF"); + logger.LogError(ex, "An error occurred while attempting to validate the collection as IIIF"); } return collection; diff --git a/src/IIIFPresentation/API/Features/Storage/StorageController.cs b/src/IIIFPresentation/API/Features/Storage/StorageController.cs index 63aad7f..e32698d 100644 --- a/src/IIIFPresentation/API/Features/Storage/StorageController.cs +++ b/src/IIIFPresentation/API/Features/Storage/StorageController.cs @@ -105,7 +105,7 @@ public async Task Post(int customerId, [FromServices] UpsertFlatC if (collection == null) { - return this.PresentationProblem("could not deserialize collection", null, (int)HttpStatusCode.BadRequest, + return this.PresentationProblem("Could not deserialize collection", null, (int)HttpStatusCode.BadRequest, "Deserialization Error"); } diff --git a/src/IIIFPresentation/API/Helpers/CollectionHelperX.cs b/src/IIIFPresentation/API/Helpers/CollectionHelperX.cs index 9ba94eb..95ec533 100644 --- a/src/IIIFPresentation/API/Helpers/CollectionHelperX.cs +++ b/src/IIIFPresentation/API/Helpers/CollectionHelperX.cs @@ -46,6 +46,9 @@ public static Uri GenerateFlatCollectionViewLast(this Collection collection, Url new( $"{collection.GenerateFlatCollectionId(urlRoots)}?page={lastPage}&pageSize={pageSize}{orderQueryParam}"); + public static string GetCollectionBucketKey(this Collection collection) => + $"{collection.CustomerId}/collections/{collection.Id}"; + public static string GenerateFullPath(this Collection collection, string itemSlug) => $"{(collection.Parent != null ? $"{collection.Slug}/" : string.Empty)}{itemSlug}";