Adding ability to restrict allowed characters in an asset id and allowing ( and )

src/protagonist/API.Tests/Features/Assets/ApiAssetRepositoryTests.cs

@@ -28,6 +28,7 @@ public class ApiAssetRepositoryTests
     private readonly DlcsContext contextForTests;
     private readonly DlcsContext dbContext;
     private readonly ApiAssetRepository sut;
+    private readonly char[] restrictedCharacters = Array.Empty<char>();
 
     public ApiAssetRepositoryTests(DlcsDatabaseFixture dbFixture)
     {
@@ -71,7 +72,7 @@ public async Task AssetRepository_Saves_New_Asset()
             DeliveryChannels = Array.Empty<string>()
         };
 
-        var result = AssetPreparer.PrepareAssetForUpsert(null, newAsset, false, false);
+        var result = AssetPreparer.PrepareAssetForUpsert(null, newAsset, false, false, restrictedCharacters);
         result.Success.Should().BeTrue();
 
         await sut.Save(newAsset, false, CancellationToken.None);
@@ -92,7 +93,7 @@ public async Task AssetRepository_Saves_New_Asset_IncrementsCounter_CountersDoNo
             DeliveryChannels = Array.Empty<string>()
         };
 
-        var result = AssetPreparer.PrepareAssetForUpsert(null, newAsset, false, false);
+        var result = AssetPreparer.PrepareAssetForUpsert(null, newAsset, false, false, restrictedCharacters);
         result.Success.Should().BeTrue();
 
         // Act
@@ -127,7 +128,7 @@ public async Task AssetRepository_Saves_New_Asset_IncrementsCounter_CountersExis
             new EntityCounter { Customer = 10120, Scope = "99", Next = 10, Type = "space-images" });
         await contextForTests.SaveChangesAsync();
 
-        var result = AssetPreparer.PrepareAssetForUpsert(null, newAsset, false, false);
+        var result = AssetPreparer.PrepareAssetForUpsert(null, newAsset, false, false, restrictedCharacters);
         result.Success.Should().BeTrue();
 
         // Act
@@ -155,7 +156,7 @@ public async Task AssetRepository_Saves_New_Asset_UsingResultFromPreparer()
             Reference1 = "I am new", Origin = "https://example.org/image1.tiff", DeliveryChannels = Array.Empty<string>()
         };
 
-        var result = AssetPreparer.PrepareAssetForUpsert(null, newAsset, false, false);
+        var result = AssetPreparer.PrepareAssetForUpsert(null, newAsset, false, false, restrictedCharacters);
         result.Success.Should().BeTrue();
 
         await sut.Save(result.UpdatedAsset!, false, CancellationToken.None);
@@ -185,7 +186,7 @@ await contextForTests.Images.AddTestAsset(assetId, ref1: "I am original 1",
             Space = 1
         };
 
-        var result = AssetPreparer.PrepareAssetForUpsert(existingAsset, patch, false, false);
+        var result = AssetPreparer.PrepareAssetForUpsert(existingAsset, patch, false, false, restrictedCharacters);
         result.Success.Should().BeTrue();
 
         // Act
@@ -195,7 +196,7 @@ await contextForTests.Images.AddTestAsset(assetId, ref1: "I am original 1",
         dbAsset.Entity.Reference1.Should().Be("I am changed");
         dbAsset.Entity.Reference2.Should().Be("I am original 2");
     }
-    
+
     [Fact]
     public async Task AssetRepository_Saves_Existing_Asset_DoesNotIncrementCounters()
     {
@@ -220,7 +221,7 @@ await contextForTests.Images.AddTestAsset(assetId, ref1: "I am original 1",
             Space = 1
         };
 
-        var result = AssetPreparer.PrepareAssetForUpsert(existingAsset, patch, false, false);
+        var result = AssetPreparer.PrepareAssetForUpsert(existingAsset, patch, false, false, restrictedCharacters);
         result.Success.Should().BeTrue();
 
         // Act
@@ -256,7 +257,7 @@ public async Task AssetRepository_Saves_Existing_Asset_UsingResultFromPreparer()
             Space = 1
         };
 
-        var result = AssetPreparer.PrepareAssetForUpsert(existingAsset, patch, false, false);
+        var result = AssetPreparer.PrepareAssetForUpsert(existingAsset, patch, false, false, restrictedCharacters);
         result.Success.Should().BeTrue();
 
         // Act
@@ -266,6 +267,51 @@ public async Task AssetRepository_Saves_Existing_Asset_UsingResultFromPreparer()
         dbAsset.Entity.Reference1.Should().Be("I am changed");
         dbAsset.Entity.Reference2.Should().Be("I am original 2");
     }
+
+    [Fact]
+    public async Task AssetRepository_SavesExistingAsset_WithRestrictedCharters()
+    {
+        // Arrange
+        var assetId = AssetId.FromString($"100/10/id with restricted characters");
+        var dbAsset =
+            await contextForTests.Images.AddTestAsset(assetId, ref1: "I am original 1",
+                ref2: "I am original 2");
+        await contextForTests.SaveChangesAsync();
+
+        var existingAsset = await dbContext.Images.FirstAsync(a => a.Id == assetId);
+        var patch = new Asset
+        {
+            Id = assetId,
+            Reference1 = "I am changed",
+            Customer = 99,
+            Space = 1
+        };
+
+        var result = AssetPreparer.PrepareAssetForUpsert(existingAsset, patch, false, false, new []{' '});
+        result.Success.Should().BeTrue();
+
+        // Act
+        await sut.Save(existingAsset, true, CancellationToken.None);
+
+        await contextForTests.Entry(dbAsset.Entity).ReloadAsync();
+        dbAsset.Entity.Reference1.Should().Be("I am changed");
+        dbAsset.Entity.Reference2.Should().Be("I am original 2");
+    }
+
+    [Fact]
+    public async Task AssetRepository_FailsToSaveAsset_WhichHasRestrictedCharacters()
+    {
+        var assetId = AssetId.FromString("100/10/id with restricted characters 2");
+        var newAsset = new Asset(assetId)
+        {
+            Reference1 = "I am new", Origin = "https://example.org/image1.tiff",
+            DeliveryChannels = Array.Empty<string>()
+        };
+
+        var result = AssetPreparer.PrepareAssetForUpsert(null, newAsset, false, false, new []{' '});
+        result.Success.Should().BeFalse();
+        result.ErrorMessage.Should().Be("Asset id contains at least one of the following restricted characters. Valid values are:  ");
+    }
 
     [Fact]
     public async Task DeleteAsset_ReturnsCorrectStatus_IfNotFound()

src/protagonist/API.Tests/Features/Images/Ingest/AssetProcessorTest.cs

@@ -10,6 +10,7 @@
 using DLCS.Model.Storage;
 using FakeItEasy;
 using Microsoft.Extensions.Options;
+using Test.Helpers.Settings;
 
 namespace API.Tests.Features.Images.Ingest;
 
@@ -22,12 +23,14 @@ public class AssetProcessorTest
 
     public AssetProcessorTest()
     {
-        var dlcsSettings = new DlcsSettings();
+        var apiSettings = new ApiSettings();
         storageRepository = A.Fake<IStorageRepository>();
         policyRepository = A.Fake<IPolicyRepository>();
         assetRepository = A.Fake<IApiAssetRepository>();
+
+        var optionsMonitor = OptionsHelpers.GetOptionsMonitor(apiSettings);
 
-        sut = new AssetProcessor(assetRepository, storageRepository, policyRepository, Options.Create(dlcsSettings));
+        sut = new AssetProcessor(assetRepository, storageRepository, policyRepository, optionsMonitor);
     }
 
     [Fact]

src/protagonist/API.Tests/Features/Images/Validation/HydraImageValidatorTests.cs

@@ -13,7 +13,7 @@ public class HydraImageValidatorTests
 
     public HydraImageValidatorTests()
     {
-        var apiSettings = new ApiSettings() { DeliveryChannelsEnabled = true};
+        var apiSettings = new ApiSettings() { DeliveryChannelsEnabled = true, RestrictedAssetIdCharacterString = "\\ "};
         sut = new HydraImageValidator(Options.Create(apiSettings));
     }
 

src/protagonist/API.Tests/Integration/CustomerQueueTests.cs

@@ -687,6 +687,48 @@ public async Task Post_CreateBatch_201_WithMixtureOfIdSet()
         Guid.TryParse(assetInDatabase.ToList()[1].Id.Asset, out _).Should().BeTrue();
         assetInDatabase.ToList()[2].Id.Asset.Should().Be("someId");
     }
+
+    [Fact]
+    public async Task Post_CreateBatch_400_WithInvalidIdsSet()
+    {
+        const int customerId = 15;
+        const int space = 4;
+        await dbContext.Customers.AddTestCustomer(customerId);
+        await dbContext.Spaces.AddTestSpace(customerId, space);
+        await dbContext.CustomerStorages.AddTestCustomerStorage(customerId);
+        await dbContext.SaveChangesAsync();
+
+        // Arrange
+        var hydraImageBody = @"{
+    ""@context"": ""http://www.w3.org/ns/hydra/context.jsonld"",
+    ""@type"": ""Collection"",
+    ""member"": [
+        {
+          ""id"": ""some\id"",
+          ""origin"": ""https://example.org/vid.mp4"",
+          ""space"": 4,
+          ""family"": ""T"",
+          ""mediaType"": ""video/mp4""
+        },
+        {
+          ""id"": ""some Id"",
+          ""origin"": ""https://example.org/vid.mp4"",
+          ""space"": 4,
+          ""family"": ""T"",
+          ""mediaType"": ""video/mp4""
+        }
+    ]
+}";
+
+        var content = new StringContent(hydraImageBody, Encoding.UTF8, "application/json");
+        var path = $"/customers/{customerId}/queue";
+
+        // Act
+        var response = await httpClient.AsCustomer(customerId).PostAsync(path, content);
+
+        // Assert
+        response.StatusCode.Should().Be(HttpStatusCode.BadRequest);
+    }
 
     [Fact]
     public async Task Post_CreateBatch_400_IfSpaceNotFound()

src/protagonist/API.Tests/Integration/ModifyAssetTests.cs

@@ -100,6 +100,30 @@ public async Task Put_NewImageAsset_Creates_Asset()
         asset.ImageOptimisationPolicy.Should().Be("fast-higher");
     }
 
+    [Fact]
+    public async Task Put_NewImageAsset_BadRequest_WhenCalledWithInvalidId()
+    {
+        var assetId = new AssetId(99, 1, "invalid id");
+        var hydraImageBody = $@"{{
+  ""@type"": ""Image"",
+  ""origin"": ""https://example.org/{assetId.Asset}.tiff"",
+  ""family"": ""I"",
+  ""mediaType"": ""image/tiff""
+}}";
+        A.CallTo(() =>
+                EngineClient.SynchronousIngest(
+                    A<IngestAssetRequest>.That.Matches(r => r.Asset.Id == assetId), false,
+                    A<CancellationToken>._))
+            .Returns(HttpStatusCode.OK);
+
+        // act
+        var content = new StringContent(hydraImageBody, Encoding.UTF8, "application/json");
+        var response = await httpClient.AsCustomer(99).PutAsync(assetId.ToApiResourcePath(), content);
+
+        // assert
+        response.StatusCode.Should().Be(HttpStatusCode.BadRequest);
+    }
+
     [Fact]
     public async Task Put_NewImageAsset_FailsToCreateAsset_whenMediaTypeAndFamilyNotSet()
     {

src/protagonist/API.Tests/appsettings.Testing.json

@@ -42,5 +42,6 @@
       "LegacySupport": true,
       "NovelSpaces": ["1","4"]
     }
-  }
+  },
+  "RestrictedAssetIdCharacterString": "\\ "
 }

src/protagonist/API/Features/Image/ImageController.cs

@@ -66,7 +66,7 @@ public async Task<IActionResult> GetImage(int customerId, int spaceId, string im
     /// <param name="hydraAsset">The body of the request contains the Asset in Hydra JSON-LD form (Image class)</param>
     /// <returns>The created or updated Hydra Image object for the Asset</returns>
     /// <remarks>
-    /// Sample requests:
+    /// Sample request:
     ///
     ///     PUT: /customers/1/spaces/1/images/my-image
     ///     {
@@ -76,13 +76,6 @@ public async Task<IActionResult> GetImage(int customerId, int spaceId, string im
     ///         "mediaType": "image/jpeg",
     ///         "string1": "my-metadata"
     ///     }
-    ///
-    ///     PUT: /customers/1/spaces/1/images/my-image
-    ///     {
-    ///         "@type":"Image",
-    ///         "family": "I",
-    ///         "file": "/9j/4AAQSkZJRgABAQAAAQABAAD/2wBDAAM...."
-    ///     }
     /// </remarks>
     [ProducesResponseType((int)HttpStatusCode.OK, Type = typeof(DLCS.HydraModel.Image))]
     [ProducesResponseType((int)HttpStatusCode.Created, Type = typeof(DLCS.HydraModel.Image))]
@@ -106,6 +99,11 @@ public async Task<IActionResult> PutImage(
         {
             hydraAsset = LegacyModeConverter.VerifyAndConvertToModernFormat(hydraAsset);
         }
+
+        if (hydraAsset.ModelId == null)
+        {
+            hydraAsset.ModelId = imageId;
+        }
 
         var validationResult = await validator.ValidateAsync(hydraAsset, cancellationToken);
         if (!validationResult.IsValid)
@@ -212,9 +210,8 @@ public Task<IActionResult> ReingestAsset([FromRoute] int customerId, [FromRoute]
     }
 
     /// <summary>
-    /// <para>Ingest specified file bytes to DLCS.
-    /// "File" property should be base64 encoded image.</para>
-    /// <para>This route is now deprecated. <see cref="PutImage"/> should be used instead.</para>
+    /// Ingest specified file bytes to DLCS. Only "I" family assets are accepted.
+    /// "File" property should be base64 encoded image. 
     /// </summary>
     /// <remarks>
     /// Sample request:
@@ -238,10 +235,12 @@ public async Task<IActionResult> PostImageWithFileBytes(
         [FromServices] HydraImageValidator validator,
         CancellationToken cancellationToken)
     {
+
         logger.LogWarning(
             "Warning: POST /customers/{CustomerId}/spaces/{SpaceId}/images/{ImageId} was called. This route is deprecated.",
             customerId, spaceId, imageId);
 
+
         return await PutImage(customerId, spaceId, imageId, hydraAsset, validator, cancellationToken);
     }
 

src/protagonist/API/Features/Image/Ingest/AssetProcessor.cs

@@ -1,5 +1,6 @@
 using API.Features.Assets;
 using API.Infrastructure.Requests;
+using API.Settings;
 using DLCS.Core;
 using DLCS.Core.Collections;
 using DLCS.Core.Settings;
@@ -20,18 +21,18 @@ public class AssetProcessor
     private readonly IPolicyRepository policyRepository;
     private readonly IApiAssetRepository assetRepository;
     private readonly IStorageRepository storageRepository;
-    private readonly DlcsSettings settings;
+    private readonly ApiSettings settings;
 
     public AssetProcessor(
         IApiAssetRepository assetRepository,
         IStorageRepository storageRepository,
         IPolicyRepository policyRepository,
-        IOptions<DlcsSettings> dlcsSettings)
+        IOptionsMonitor<ApiSettings> apiSettings)
     {
         this.assetRepository = assetRepository;
         this.storageRepository = storageRepository;
         this.policyRepository = policyRepository;
-        this.settings = dlcsSettings.Value;
+        settings = apiSettings.CurrentValue;
     }
 
     /// <summary>
@@ -94,7 +95,7 @@ public async Task<ProcessAssetResult> Process(Asset asset, bool mustExist, bool
             }
 
             var assetPreparationResult =
-                AssetPreparer.PrepareAssetForUpsert(existingAsset, asset, false, isBatchUpdate);
+                AssetPreparer.PrepareAssetForUpsert(existingAsset, asset, false, isBatchUpdate, settings.RestrictedAssetIdCharacters);
 
             if (!assetPreparationResult.Success)
             {
@@ -110,7 +111,7 @@ public async Task<ProcessAssetResult> Process(Asset asset, bool mustExist, bool
 
             if (existingAsset == null)
             {
-                var preset = settings.IngestDefaults.GetPresets((char)updatedAsset.Family!,
+                var preset = settings.DLCS.IngestDefaults.GetPresets((char)updatedAsset.Family!,
                     updatedAsset.MediaType ?? string.Empty);
                 var deliveryChannelChanged = SetDeliveryChannel(updatedAsset, preset);
                 if (deliveryChannelChanged)

src/protagonist/API/Features/Image/Validation/HydraImageValidator.cs

@@ -35,7 +35,7 @@ public HydraImageValidator(IOptions<ApiSettings> apiSettings)
         RuleFor(a => a.DeliveryChannels).Must(d => d.IsNullOrEmpty())
             .When(_ => !apiSettings.Value.DeliveryChannelsEnabled)
             .WithMessage("Delivery channels are disabled");
-        
+
         RuleForEach(a => a.DeliveryChannels)
             .Must(dc => AssetDeliveryChannels.All.Contains(dc))
             .WithMessage($"DeliveryChannel must be one of {AssetDeliveryChannels.AllString}");