File deletion (#227)

* Re-factor. GetFile query was excessively complex so split up part into separate query. Also updated FileEntity to use Actor entity consistenly.

* Added application code for deleting file

* Added Hangfire support

* Added deletion after everyone has confirmed download

* Added code to add service owner to database

* Added corresponding field to File

* Add logic to trigger Hangfire jobs for deleting file on all confirmed downloaded and file initialized

* Removed unused return value

* Formatting

* Consolidated naming

* Formatting

* Fix test data

* Known issue in Serilog: serilog/serilog-aspnetcore#289 and we get around it by not using BootstrapLogger. We do not need it anyway

* Try with joining the repeatable migration sripts together. Seems one fails to run if it is dependent on the other one when in a brand new environment. Should find better way to split up in more test scripts if we get big script.

* Hangfire in memory during tests to avoid issues with concurrent initialization

Author: Ceredron

Test/Altinn.Broker.Tests/Altinn.Broker.Tests.csproj

@@ -12,6 +12,7 @@
   <ItemGroup>
     <PackageReference Include="Microsoft.AspNetCore.Mvc.Testing" Version="8.0.0" />
     <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.8.0" />
+    <PackageReference Include="Moq" Version="4.20.70" />
     <PackageReference Include="xunit" Version="2.6.2" />
     <PackageReference Include="xunit.runner.visualstudio" Version="2.5.4">
       <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>

Test/Altinn.Broker.Tests/Data/R__Prepare_File.sql

@@ -0,0 +1,8 @@
+INSERT INTO broker.service_owner (service_owner_id_pk, service_owner_name, file_time_to_live)
+VALUES ('0192:991825827', 'Digitaliseringsdirektoratet Avd Oslo', '1 Days');
+
+INSERT INTO broker.storage_provider (storage_provider_id_pk, service_owner_id_fk, created, storage_provider_type, resource_name)
+VALUES (DEFAULT, '0192:991825827', NOW(), 'Altinn3Azure', 'dummy-value');
+
+INSERT INTO broker.file (file_id_pk, service_owner_id_fk, filename, checksum, sender_actor_id_fk, external_file_reference, created, storage_provider_id_fk, file_location, expiration_time)
+VALUES ('ed76ce89-3768-481a-bca1-4e4262d9098b', '0192:991825827', 'filename.txt', NULL, 1, 'external_reference', NOW(), 1, 'https://blob-storage-example', NOW() + INTERVAL '1 DAY')

Test/Altinn.Broker.Tests/Data/R__Prepare_ServiceOwner.sql

@@ -9,14 +9,17 @@
 using Altinn.Broker.Tests.Factories;
 using Altinn.Broker.Tests.Helpers;
 
-using Microsoft.AspNetCore.Mvc.Testing;
+using Hangfire.Common;
+using Hangfire.States;
+
+using Moq;
 
 using Xunit;
 
 namespace Altinn.Broker.Tests;
 public class FileControllerTests : IClassFixture<CustomWebApplicationFactory>
 {
-    private readonly WebApplicationFactory<Program> _factory;
+    private readonly CustomWebApplicationFactory _factory;
     private readonly HttpClient _senderClient;
     private readonly HttpClient _recipientClient;
     private readonly JsonSerializerOptions _responseSerializerOptions;
@@ -77,6 +80,11 @@ public async Task NormalFlow_WhenAllIsOK_Success()
         Assert.NotNull(confirmedFileDetails);
         Assert.True(confirmedFileDetails.FileStatus == FileStatusExt.AllConfirmedDownloaded);
         Assert.Contains(confirmedFileDetails.RecipientFileStatusHistory, recipient => recipient.RecipientFileStatusCode == RecipientFileStatusExt.DownloadConfirmed);
+
+        // Confirm that it has been enqueued for deletion
+        _factory.HangfireBackgroundJobClient?.Verify(jobClient => jobClient.Create(
+            It.Is<Job>(job => (job.Method.DeclaringType != null) && job.Method.DeclaringType.Name == "DeleteFileCommandHandler" && ((Guid)job.Args[0] == Guid.Parse(fileId))),
+            It.IsAny<EnqueuedState>()));
     }
 
     [Fact]

Test/Altinn.Broker.Tests/Data/R__Prepare_Test_Data.sql

@@ -1,3 +1,6 @@
+using Hangfire;
+using Hangfire.MemoryStorage;
+
 using Microsoft.AspNetCore.Authentication;
 using Microsoft.AspNetCore.Authentication.JwtBearer;
 using Microsoft.AspNetCore.Hosting;
@@ -7,8 +10,11 @@
 using Microsoft.IdentityModel.JsonWebTokens;
 using Microsoft.IdentityModel.Tokens;
 
+using Moq;
+
 public class CustomWebApplicationFactory : WebApplicationFactory<Program>
 {
+    internal Mock<IBackgroundJobClient>? HangfireBackgroundJobClient;
     protected override void ConfigureWebHost(
         IWebHostBuilder builder)
     {
@@ -39,6 +45,11 @@ protected override void ConfigureWebHost(
                     }
                 };
             });
+            services.AddHangfire(config =>
+                config.UseMemoryStorage()
+            );
+            HangfireBackgroundJobClient = new Mock<IBackgroundJobClient>();
+            services.AddSingleton(HangfireBackgroundJobClient.Object);
         });
     }
 }

Test/Altinn.Broker.Tests/FileControllerTests.cs

@@ -14,6 +14,7 @@
   </ItemGroup>
 
   <ItemGroup>
+    <PackageReference Include="Hangfire.Core" Version="1.8.6" />
     <PackageReference Include="Microsoft.AspNetCore.Mvc.Core" Version="2.2.5" />
     <PackageReference Include="Microsoft.Extensions.Configuration.Abstractions" Version="8.0.0" />
     <PackageReference Include="Microsoft.Extensions.DependencyInjection.Abstractions" Version="8.0.0" />

Test/Altinn.Broker.Tests/Helpers/CustomWebApplicationFactory.cs

@@ -1,9 +1,12 @@
 using Altinn.Broker.Application;
 using Altinn.Broker.Application.ConfirmDownloadCommand;
+using Altinn.Broker.Application.DeleteFileCommand;
 using Altinn.Broker.Core.Application;
 using Altinn.Broker.Core.Domain.Enums;
 using Altinn.Broker.Core.Repositories;
 
+using Hangfire;
+
 using Microsoft.Extensions.Logging;
 
 using OneOf;
@@ -14,14 +17,16 @@ public class ConfirmDownloadCommandHandler : IHandler<ConfirmDownloadCommandRequ
     private readonly IFileRepository _fileRepository;
     private readonly IFileStatusRepository _fileStatusRepository;
     private readonly IActorFileStatusRepository _actorFileStatusRepository;
+    private readonly IBackgroundJobClient _backgroundJobClient;
     private readonly ILogger<ConfirmDownloadCommandHandler> _logger;
 
-    public ConfirmDownloadCommandHandler(IServiceOwnerRepository serviceOwnerRepository, IFileRepository fileRepository, IFileStatusRepository fileStatusRepository, IActorFileStatusRepository actorFileStatusRepository, ILogger<ConfirmDownloadCommandHandler> logger)
+    public ConfirmDownloadCommandHandler(IServiceOwnerRepository serviceOwnerRepository, IFileRepository fileRepository, IFileStatusRepository fileStatusRepository, IActorFileStatusRepository actorFileStatusRepository, IBackgroundJobClient backgroundJobClient, ILogger<ConfirmDownloadCommandHandler> logger)
     {
         _serviceOwnerRepository = serviceOwnerRepository;
         _fileRepository = fileRepository;
         _fileStatusRepository = fileStatusRepository;
         _actorFileStatusRepository = actorFileStatusRepository;
+        _backgroundJobClient = backgroundJobClient;
         _logger = logger;
     }
     public async Task<OneOf<ConfirmDownloadCommandResponse, Error>> Process(ConfirmDownloadCommandRequest request)
@@ -36,7 +41,7 @@ public async Task<OneOf<ConfirmDownloadCommandResponse, Error>> Process(ConfirmD
         {
             return Errors.FileNotFound;
         }
-        if (!file.ActorEvents.Any(actorEvent => actorEvent.Actor.ActorExternalId == request.Consumer))
+        if (!file.RecipientCurrentStatuses.Any(actorEvent => actorEvent.Actor.ActorExternalId == request.Consumer))
         {
             return Errors.FileNotFound;
         }
@@ -46,13 +51,12 @@ public async Task<OneOf<ConfirmDownloadCommandResponse, Error>> Process(ConfirmD
         }
 
         await _actorFileStatusRepository.InsertActorFileStatus(request.FileId, ActorFileStatus.DownloadConfirmed, request.Consumer);
-        var recipientStatuses = file.ActorEvents
-            .Where(actorEvent => actorEvent.Actor.ActorExternalId != file.Sender && actorEvent.Actor.ActorExternalId != request.Consumer)
-            .GroupBy(actorEvent => actorEvent.Actor.ActorExternalId)
-            .Select(group => group.Max(statusEvent => statusEvent.Status))
-            .ToList();
-        bool shouldConfirmAll = recipientStatuses.All(status => status >= ActorFileStatus.DownloadConfirmed);
-        await _fileStatusRepository.InsertFileStatus(request.FileId, FileStatus.AllConfirmedDownloaded);
+        bool shouldConfirmAll = file.RecipientCurrentStatuses.Where(recipientStatus => recipientStatus.Actor.ActorExternalId != request.Consumer).All(status => status.Status >= ActorFileStatus.DownloadConfirmed);
+        if (shouldConfirmAll)
+        {
+            await _fileStatusRepository.InsertFileStatus(request.FileId, FileStatus.AllConfirmedDownloaded);
+            _backgroundJobClient.Enqueue<DeleteFileCommandHandler>((deleteFileCommandHandler) => deleteFileCommandHandler.Process(request.FileId));
+        }
 
         return new ConfirmDownloadCommandResponse();
     }

src/Altinn.Broker.Application/Altinn.Broker.Application.csproj

@@ -0,0 +1,54 @@
+using Altinn.Broker.Core.Application;
+using Altinn.Broker.Core.Repositories;
+
+using Microsoft.Extensions.Logging;
+
+using OneOf;
+
+namespace Altinn.Broker.Application.DeleteFileCommand;
+public class DeleteFileCommandHandler : IHandler<Guid, Task>
+{
+    private readonly IFileRepository _fileRepository;
+    private readonly IFileStatusRepository _fileStatusRepository;
+    private readonly IServiceOwnerRepository _serviceOwnerRepository;
+    private readonly IBrokerStorageService _brokerStorageService;
+    private readonly ILogger<DeleteFileCommandHandler> _logger;
+
+    public DeleteFileCommandHandler(IFileRepository fileRepository, IFileStatusRepository fileStatusRepository, IServiceOwnerRepository serviceOwnerRepository, IBrokerStorageService brokerStorageService, ILogger<DeleteFileCommandHandler> logger)
+    {
+        _fileRepository = fileRepository;
+        _fileStatusRepository = fileStatusRepository;
+        _serviceOwnerRepository = serviceOwnerRepository;
+        _brokerStorageService = brokerStorageService;
+        _logger = logger;
+    }
+
+    public async Task<OneOf<Task, Error>> Process(Guid fileId)
+    {
+        _logger.LogInformation("Deleting file with id {fileId}", fileId.ToString());
+        var file = await _fileRepository.GetFile(fileId);
+        if (file is null)
+        {
+            return Errors.FileNotFound;
+        }
+        var serviceOwner = await _serviceOwnerRepository.GetServiceOwner(file.ServiceOwnerId);
+        if (serviceOwner is null)
+        {
+            return Errors.ServiceOwnerNotConfigured;
+        }
+        if (file.FileStatus >= Core.Domain.Enums.FileStatus.Deleted)
+        {
+            _logger.LogInformation("File has already been set to deleted");
+        }
+
+        await _fileStatusRepository.InsertFileStatus(fileId, Core.Domain.Enums.FileStatus.Deleted);
+        await _brokerStorageService.DeleteFile(serviceOwner, file);
+        var recipientsWhoHaveNotDownloaded = file.RecipientCurrentStatuses.Where(latestStatus => latestStatus.Status <= Core.Domain.Enums.ActorFileStatus.DownloadConfirmed).ToList();
+        foreach (var recipient in recipientsWhoHaveNotDownloaded)
+        {
+            _logger.LogError("Recipient {recipientExternalReference} did not download the file with id {fileId}", recipient.Actor.ActorExternalId, recipient.FileId.ToString());
+            // TODO, send events
+        }
+        return Task.CompletedTask;
+    }
+}

src/Altinn.Broker.Application/ConfirmDownloadCommand/ConfirmDownloadCommandHandler.cs

@@ -36,7 +36,7 @@ public async Task<OneOf<DownloadFileQueryResponse, Error>> Process(DownloadFileQ
         {
             return Errors.FileNotFound;
         }
-        if (!file.ActorEvents.Any(actorEvent => actorEvent.Actor.ActorExternalId == request.Consumer))
+        if (!file.RecipientCurrentStatuses.Any(actorEvent => actorEvent.Actor.ActorExternalId == request.Consumer))
         {
             return Errors.FileNotFound;
         }

src/Altinn.Broker.Application/DeleteFileCommand/DeleteFileCommandHandler.cs

@@ -32,7 +32,8 @@ public async Task<OneOf<GetFileDetailsQueryResponse, Error>> Process(GetFileDeta
         {
             return Errors.FileNotFound;
         }
-        if (!file.ActorEvents.Any(actorEvent => actorEvent.Actor.ActorExternalId == request.Consumer))
+        if (file.Sender.ActorExternalId != request.Consumer &&
+            !file.RecipientCurrentStatuses.Any(actorEvent => actorEvent.Actor.ActorExternalId == request.Consumer))
         {
             return Errors.FileNotFound;
         }

src/Altinn.Broker.Application/DownloadFileQuery/DownloadFileQueryHandler.cs

@@ -33,7 +33,8 @@ public async Task<OneOf<GetFileOverviewQueryResponse, Error>> Process(GetFileOve
         {
             return Errors.FileNotFound;
         }
-        if (!file.ActorEvents.Any(actorEvent => actorEvent.Actor.ActorExternalId == request.Consumer))
+        if (file.Sender.ActorExternalId != request.Consumer &&
+            !file.RecipientCurrentStatuses.Any(actorEvent => actorEvent.Actor.ActorExternalId == request.Consumer))
         {
             return Errors.FileNotFound;
         }

src/Altinn.Broker.Application/GetFileDetailsQuery/GetFileDetailsQueryHandler.cs

@@ -1,8 +1,11 @@
-using Altinn.Broker.Core.Application;
+using Altinn.Broker.Application.DeleteFileCommand;
+using Altinn.Broker.Core.Application;
 using Altinn.Broker.Core.Domain.Enums;
 using Altinn.Broker.Core.Repositories;
 using Altinn.Broker.Core.Services;
 
+using Hangfire;
+
 using Microsoft.Extensions.Logging;
 
 using OneOf;
@@ -15,15 +18,24 @@ public class InitializeFileCommandHandler : IHandler<InitializeFileCommandReques
     private readonly IFileStatusRepository _fileStatusRepository;
     private readonly IActorFileStatusRepository _actorFileStatusRepository;
     private readonly IResourceManager _resourceManager;
+    private readonly IBackgroundJobClient _backgroundJobClient;
     private readonly ILogger<InitializeFileCommandHandler> _logger;
 
-    public InitializeFileCommandHandler(IServiceOwnerRepository serviceOwnerRepository, IFileRepository fileRepository, IFileStatusRepository fileStatusRepository, IActorFileStatusRepository actorFileStatusRepository, IResourceManager resourceManager, ILogger<InitializeFileCommandHandler> logger)
+    public InitializeFileCommandHandler(
+        IServiceOwnerRepository serviceOwnerRepository,
+        IFileRepository fileRepository,
+        IFileStatusRepository fileStatusRepository,
+        IActorFileStatusRepository actorFileStatusRepository,
+        IResourceManager resourceManager,
+        IBackgroundJobClient backgroundJobClient,
+        ILogger<InitializeFileCommandHandler> logger)
     {
         _serviceOwnerRepository = serviceOwnerRepository;
         _fileRepository = fileRepository;
         _fileStatusRepository = fileStatusRepository;
         _actorFileStatusRepository = actorFileStatusRepository;
         _resourceManager = resourceManager;
+        _backgroundJobClient = backgroundJobClient;
         _logger = logger;
     }
 
@@ -45,7 +57,7 @@ public async Task<OneOf<Guid, Error>> Process(InitializeFileCommandRequest reque
         }
         var fileId = await _fileRepository.AddFile(serviceOwner, request.Filename, request.SendersFileReference, request.SenderExternalId, request.RecipientExternalIds, request.PropertyList, request.Checksum);
         await _fileStatusRepository.InsertFileStatus(fileId, FileStatus.Initialized);
-        var addRecipientEventTasks = request.RecipientExternalIds.Concat([request.SenderExternalId]).Select(recipientId => _actorFileStatusRepository.InsertActorFileStatus(fileId, ActorFileStatus.Initialized, recipientId));
+        var addRecipientEventTasks = request.RecipientExternalIds.Select(recipientId => _actorFileStatusRepository.InsertActorFileStatus(fileId, ActorFileStatus.Initialized, recipientId));
         try
         {
             await Task.WhenAll(addRecipientEventTasks);
@@ -54,6 +66,8 @@ public async Task<OneOf<Guid, Error>> Process(InitializeFileCommandRequest reque
         {
             _logger.LogError("Failed when adding recipient initialized events.");
         }
+        _backgroundJobClient.Schedule<DeleteFileCommandHandler>((deleteFileCommandHandler) => deleteFileCommandHandler.Process(fileId), serviceOwner.FileTimeToLive);
+
         return fileId;
     }
 }

src/Altinn.Broker.Application/GetFileOverviewQuery/GetFileOverviewQueryHandler.cs

@@ -45,7 +45,7 @@ public async Task<OneOf<Guid, Error>> Process(UploadFileCommandRequest request)
         {
             return Errors.FileNotFound;
         }
-        if (request.Consumer != file.Sender)
+        if (request.Consumer != file.Sender.ActorExternalId)
         {
             return Errors.FileNotFound;
         }

src/Altinn.Broker.Application/InitializeFileCommand/InitializeFileCommandHandler.cs

@@ -6,12 +6,13 @@ public class FileEntity
 {
     public Guid FileId { get; set; }
     public string ServiceOwnerId { get; set; }
-    public string Sender { get; set; } // Joined in
+    public ActorEntity Sender { get; set; } // Joined in
     public string SendersFileReference { get; set; }
     public FileStatus FileStatus { get; set; }
     public DateTimeOffset FileStatusChanged { get; set; }
-    public DateTimeOffset Uploaded { get; set; }
-    public List<ActorFileStatusEntity> ActorEvents { get; set; } // Joined in
+    public DateTimeOffset Created { get; set; }
+    public DateTimeOffset ExpirationTime { get; set; }
+    public List<ActorFileStatusEntity> RecipientCurrentStatuses { get; set; } // Joined in
     public StorageProviderEntity StorageProvider { get; set; }
     public string? FileLocation { get; set; }
     public string Filename { get; set; }

src/Altinn.Broker.Application/UploadFileCommand/UploadFileCommandHandler.cs

@@ -5,4 +5,5 @@ public class ServiceOwnerEntity
     public string Id { get; set; }
     public string Name { get; set; }
     public StorageProviderEntity? StorageProvider { get; set; }
+    public TimeSpan FileTimeToLive { get; set; }
 }

src/Altinn.Broker.Core/Domain/FileEntity.cs

@@ -4,6 +4,7 @@ public interface IFileStore
     {
         Task<Stream> GetFileStream(Guid fileId, string connectionString);
         Task UploadFile(Stream filestream, Guid fileId, string connectionString);
+        Task DeleteFile(Guid fileId, string connectionString);
         Task<bool> IsOnline(string connectionString);
     }
 }

src/Altinn.Broker.Core/Domain/ServiceOwnerEntity.cs

@@ -3,7 +3,7 @@
 namespace Altinn.Broker.Core.Repositories;
 public interface IServiceOwnerRepository
 {
-    Task InitializeServiceOwner(string sub, string name);
+    Task InitializeServiceOwner(string sub, string name, TimeSpan fileTimeToLive);
     Task<ServiceOwnerEntity?> GetServiceOwner(string sub);
     Task InitializeStorageProvider(string sub, string resourceName, StorageProviderType storageType);
 }

src/Altinn.Broker.Core/Repositories/IFileStore.cs

@@ -14,4 +14,5 @@ public interface IBrokerStorageService
     Task UploadFile(ServiceOwnerEntity serviceOwnerEntity, FileEntity fileEntity, Stream stream);
 
     Task<Stream> DownloadFile(ServiceOwnerEntity serviceOwnerEntity, FileEntity file);
+    Task DeleteFile(ServiceOwnerEntity serviceOwnerEntity, FileEntity fileEntity);
 }

src/Altinn.Broker.Core/Repositories/IServiceOwnerRepository.cs

@@ -33,6 +33,12 @@ public async Task<Stream> DownloadFile(ServiceOwnerEntity serviceOwnerEntity, Fi
         return await _fileStore.GetFileStream(fileEntity.FileId, connectionString);
     }
 
+    public async Task DeleteFile(ServiceOwnerEntity serviceOwnerEntity, FileEntity fileEntity)
+    {
+        var connectionString = await GetConnectionString(serviceOwnerEntity);
+        await _fileStore.DeleteFile(fileEntity.FileId, connectionString);
+    }
+
     private async Task<string> GetConnectionString(ServiceOwnerEntity serviceOwnerEntity)
     {
         if (_hostEnvironment.IsDevelopment())

src/Altinn.Broker.Core/Services/IBrokerStorageService.cs

@@ -51,4 +51,12 @@ public async Task<bool> IsOnline(string connectionString)
             return false;
         }
     }
+
+    public async Task DeleteFile(Guid fileId, string connectionString)
+    {
+        var blobServiceClient = new BlobServiceClient(connectionString);
+        var containerClient = blobServiceClient.GetBlobContainerClient("brokerfiles");
+        BlobClient blobClient = containerClient.GetBlobClient(fileId.ToString());
+        await blobClient.DeleteIfExistsAsync();
+    }
 }