From 9a7999c131672586c356b41e3425288ae20ccd2e Mon Sep 17 00:00:00 2001 From: Tom Jones Date: Mon, 2 Dec 2024 11:00:49 +0000 Subject: [PATCH 1/4] EES-5583: Add endpoint to return a zip archive upload plan. --- .../Controllers/Api/ReleasesController.cs | 20 +- .../Models/DataFileInfo.cs | 2 +- .../Services/DataArchiveValidationService.cs | 104 ++++---- .../IDataArchiveValidationService.cs | 11 +- .../Interfaces/IReleaseDataFileService.cs | 18 +- .../Services/ReleaseDataFileService.cs | 224 ++++++++++++------ .../Validators/ValidationMessages.cs | 30 ++- .../ViewModels/ArchiveDataSetFileViewModel.cs | 23 ++ .../BlobContainers.cs | 3 +- .../Services/BlobStorageService.cs | 42 ++-- .../Services/FileStoragePathUtils.cs | 4 +- .../Interfaces/IBlobStorageService.cs | 16 +- .../Services/PrivateReleaseFileBlobService.cs | 2 +- .../Services/PublicReleaseFileBlobService.cs | 2 +- 14 files changed, 333 insertions(+), 168 deletions(-) create mode 100644 src/GovUk.Education.ExploreEducationStatistics.Admin/ViewModels/ArchiveDataSetFileViewModel.cs diff --git a/src/GovUk.Education.ExploreEducationStatistics.Admin/Controllers/Api/ReleasesController.cs b/src/GovUk.Education.ExploreEducationStatistics.Admin/Controllers/Api/ReleasesController.cs index f192797c8fa..331b6667e50 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Admin/Controllers/Api/ReleasesController.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Admin/Controllers/Api/ReleasesController.cs @@ -136,15 +136,27 @@ public async Task> UploadDataSetAsZip(Guid releaseVer .HandleFailuresOrOk(); } - [HttpPost("release/{releaseVersionId:guid}/bulk-zip-data")] + [HttpPost("release/{releaseVersionId:guid}/upload-bulk-zip-data")] [DisableRequestSizeLimit] [RequestFormLimits(ValueLengthLimit = int.MaxValue, MultipartBodyLengthLimit = int.MaxValue)] - public async Task>> UploadDataSetsAsBulkZip( + public async Task>> UploadBulkZipDataSetsToTempStorage( Guid releaseVersionId, - IFormFile zipFile) + IFormFile zipFile, + CancellationToken cancellationToken) + { + return await _releaseDataFileService + .ValidateAndUploadBulkZip(releaseVersionId, zipFile, cancellationToken) + .HandleFailuresOrOk(); + } + + [HttpPost("release/{releaseVersionId:guid}/import-bulk-zip-data")] + public async Task>> ImportBulkZipDataSetsFromTempStorage( + Guid releaseVersionId, + List dataSetFiles, + CancellationToken cancellationToken) { return await _releaseDataFileService - .UploadAsBulkZip(releaseVersionId: releaseVersionId, bulkZipFormFile: zipFile) + .SaveDataSetsFromTemporaryBlobStorage(releaseVersionId, dataSetFiles, cancellationToken) .HandleFailuresOrOk(); } diff --git a/src/GovUk.Education.ExploreEducationStatistics.Admin/Models/DataFileInfo.cs b/src/GovUk.Education.ExploreEducationStatistics.Admin/Models/DataFileInfo.cs index 5a4e0eeffe0..e044a931505 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Admin/Models/DataFileInfo.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Admin/Models/DataFileInfo.cs @@ -1,9 +1,9 @@ #nullable enable -using System; using GovUk.Education.ExploreEducationStatistics.Common.Converters; using GovUk.Education.ExploreEducationStatistics.Common.Model; using GovUk.Education.ExploreEducationStatistics.Content.Model; using Newtonsoft.Json; +using System; namespace GovUk.Education.ExploreEducationStatistics.Admin.Models { diff --git a/src/GovUk.Education.ExploreEducationStatistics.Admin/Services/DataArchiveValidationService.cs b/src/GovUk.Education.ExploreEducationStatistics.Admin/Services/DataArchiveValidationService.cs index d0ca5865233..71153aea010 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Admin/Services/DataArchiveValidationService.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Admin/Services/DataArchiveValidationService.cs @@ -1,9 +1,4 @@ #nullable enable -using System; -using System.Collections.Generic; -using System.IO.Compression; -using System.Linq; -using System.Threading.Tasks; using GovUk.Education.ExploreEducationStatistics.Admin.Models; using GovUk.Education.ExploreEducationStatistics.Admin.Services.Interfaces; using GovUk.Education.ExploreEducationStatistics.Admin.Validators; @@ -15,7 +10,11 @@ using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; using Microsoft.EntityFrameworkCore; -using static GovUk.Education.ExploreEducationStatistics.Common.Model.FileType; +using System; +using System.Collections.Generic; +using System.IO.Compression; +using System.Linq; +using System.Threading.Tasks; namespace GovUk.Education.ExploreEducationStatistics.Admin.Services { @@ -89,16 +88,10 @@ public async Task> ValidateDataArchiveF return archiveDataSet; } - public async Task>> ValidateBulkDataArchiveFile( + public async Task>> ValidateBulkDataArchiveFiles( Guid releaseVersionId, IFormFile zipFile) { - var errors = await IsValidZipFile(zipFile); - if (errors.Count > 0) - { - return Common.Validators.ValidationUtils.ValidationResult(errors); - } - await using var stream = zipFile.OpenReadStream(); using var archive = new ZipArchive(stream); @@ -109,26 +102,27 @@ public async Task>> ValidateBulkDa if (dataSetNamesFile == null) { return Common.Validators.ValidationUtils.ValidationResult(new ErrorViewModel - { - Code = ValidationMessages.BulkDataZipMustContainDatasetNamesCsv.Code, - Message = ValidationMessages.BulkDataZipMustContainDatasetNamesCsv.Message, - }); + { + Code = ValidationMessages.BulkDataZipMustContainDatasetNamesCsv.Code, + Message = ValidationMessages.BulkDataZipMustContainDatasetNamesCsv.Message, + }); } unprocessedArchiveFiles.Remove(dataSetNamesFile); List headers; - await using (var dataSetNamesStream = dataSetNamesFile.Open()) { + await using (var dataSetNamesStream = dataSetNamesFile.Open()) + { headers = await CsvUtils.GetCsvHeaders(dataSetNamesStream); } if (headers is not ["file_name", "dataset_name"]) { return Common.Validators.ValidationUtils.ValidationResult(new ErrorViewModel - { - Code = ValidationMessages.DatasetNamesCsvIncorrectHeaders.Code, - Message = ValidationMessages.DatasetNamesCsvIncorrectHeaders.Message, - }); + { + Code = ValidationMessages.DatasetNamesCsvIncorrectHeaders.Code, + Message = ValidationMessages.DatasetNamesCsvIncorrectHeaders.Message, + }); } var fileNameIndex = headers[0] == "file_name" ? 0 : 1; @@ -149,6 +143,8 @@ public async Task>> ValidateBulkDa dataSetNamesCsvEntries.Add((BaseFilename: filename, Title: datasetName)); } + var errors = new List(); + dataSetNamesCsvEntries .Select(entry => entry.BaseFilename) .Where(baseFilename => baseFilename.EndsWith(".csv")) @@ -158,7 +154,7 @@ public async Task>> ValidateBulkDa errors.Add(ValidationMessages.GenerateErrorDatasetNamesCsvFilenamesShouldNotEndDotCsv(baseFilename)); }); - // Check for duplicate data set titles - because the bulk zip itself main contain duplicates! + // Check for duplicate data set titles - because the bulk zip itself may contain duplicates! dataSetNamesCsvEntries .GroupBy(entry => entry.Title) .Where(group => group.Count() > 1) @@ -170,9 +166,9 @@ public async Task>> ValidateBulkDa .GenerateErrorDataSetTitleShouldBeUnique(duplicateTitle)); }); - // Check for duplicate data set filenames - because the bulk zip itself main contain duplicates! + // Check for duplicate data set filenames - because the bulk zip itself may contain duplicates! dataSetNamesCsvEntries - .GroupBy(entry => entry.BaseFilename) + .GroupBy(entry => entry.BaseFilename) .Where(group => group.Count() > 1) .Select(group => group.Key) .ToList() @@ -188,7 +184,7 @@ public async Task>> ValidateBulkDa } var dataSetFiles = new List(); - foreach(var entry in dataSetNamesCsvEntries) + foreach (var entry in dataSetNamesCsvEntries) { var dataFile = archive.GetEntry($"{entry.BaseFilename}.csv"); var metaFile = archive.GetEntry($"{entry.BaseFilename}.meta.csv"); @@ -204,7 +200,7 @@ public async Task>> ValidateBulkDa if (metaFile == null) { - errors.Add(ValidationMessages.GenerateErrorFileNotFoundInZip($"{entry.BaseFilename}.meta.csv", Metadata)); + errors.Add(ValidationMessages.GenerateErrorFileNotFoundInZip($"{entry.BaseFilename}.meta.csv", FileType.Metadata)); } else { @@ -213,16 +209,17 @@ public async Task>> ValidateBulkDa if (dataFile != null && metaFile != null) { - // We replace files with the same title. If there is no releaseFile with the same title, - // it's a new data set. - var releaseFileToBeReplaced = contentDbContext.ReleaseFiles - .Include(rf => rf.File) - .SingleOrDefault(rf => - rf.ReleaseVersionId == releaseVersionId - && rf.File.Type == FileType.Data - && rf.Name == entry.Title); - - var dataArchiveFile = new ArchiveDataSetFile( + try + { + // We replace files with the same title. If there is no releaseFile with the same title, it's a new data set. + var releaseFileToBeReplaced = await contentDbContext.ReleaseFiles + .Include(rf => rf.File) + .SingleOrDefaultAsync(rf => + rf.ReleaseVersionId == releaseVersionId + && rf.File.Type == FileType.Data + && rf.Name == entry.Title); + + var dataArchiveFile = new ArchiveDataSetFile( entry.Title, dataFile.FullName, metaFile.FullName, @@ -230,17 +227,22 @@ public async Task>> ValidateBulkDa metaFile.Length, releaseFileToBeReplaced?.File); - await using (var dataFileStream = dataFile.Open()) - await using (var metaFileStream = metaFile.Open()) + await using (var dataFileStream = dataFile.Open()) + await using (var metaFileStream = metaFile.Open()) + { + errors.AddRange(await fileUploadsValidatorService.ValidateDataSetFilesForUpload( + releaseVersionId, + dataArchiveFile, + dataFileStream: dataFileStream, + metaFileStream: metaFileStream)); + } + + dataSetFiles.Add(dataArchiveFile); + } + catch (InvalidOperationException) { - errors.AddRange(await fileUploadsValidatorService.ValidateDataSetFilesForUpload( - releaseVersionId, - dataArchiveFile, - dataFileStream: dataFileStream, - metaFileStream: metaFileStream)); + errors.Add(ValidationMessages.GenerateErrorDataReplacementAlreadyInProgress()); } - - dataSetFiles.Add(dataArchiveFile); } } @@ -261,14 +263,20 @@ public async Task>> ValidateBulkDa return dataSetFiles; } - private async Task> IsValidZipFile(IFormFile zipFile) + public async Task> IsValidZipFile(IFormFile zipFile) { List errors = []; + if (zipFile is null) + { + errors.Add(ValidationMessages.GenerateErrorFileIsNull()); + return errors; + } + if (zipFile.FileName.Length > MaxFilenameSize) { errors.Add(ValidationMessages.GenerateErrorFilenameTooLong( - zipFile.FileName, MaxFilenameSize)); + zipFile.FileName, MaxFilenameSize)); } if (!zipFile.FileName.ToLower().EndsWith(".zip")) diff --git a/src/GovUk.Education.ExploreEducationStatistics.Admin/Services/Interfaces/IDataArchiveValidationService.cs b/src/GovUk.Education.ExploreEducationStatistics.Admin/Services/Interfaces/IDataArchiveValidationService.cs index f1f3007f8c1..181620d6b87 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Admin/Services/Interfaces/IDataArchiveValidationService.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Admin/Services/Interfaces/IDataArchiveValidationService.cs @@ -1,12 +1,13 @@ #nullable enable -using System; -using System.Collections.Generic; -using System.Threading.Tasks; using GovUk.Education.ExploreEducationStatistics.Admin.Models; using GovUk.Education.ExploreEducationStatistics.Common.Model; +using GovUk.Education.ExploreEducationStatistics.Common.ViewModels; using GovUk.Education.ExploreEducationStatistics.Content.Model; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; +using System; +using System.Collections.Generic; +using System.Threading.Tasks; namespace GovUk.Education.ExploreEducationStatistics.Admin.Services.Interfaces { @@ -18,8 +19,10 @@ Task> ValidateDataArchiveFile( IFormFile zipFile, File? replacingFile = null); - Task>> ValidateBulkDataArchiveFile( + Task>> ValidateBulkDataArchiveFiles( Guid releaseVersionId, IFormFile zipFile); + + Task> IsValidZipFile(IFormFile zipFile); } } diff --git a/src/GovUk.Education.ExploreEducationStatistics.Admin/Services/Interfaces/IReleaseDataFileService.cs b/src/GovUk.Education.ExploreEducationStatistics.Admin/Services/Interfaces/IReleaseDataFileService.cs index fe71e7d4868..08bce86cb69 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Admin/Services/Interfaces/IReleaseDataFileService.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Admin/Services/Interfaces/IReleaseDataFileService.cs @@ -1,11 +1,13 @@ #nullable enable -using System; -using System.Collections.Generic; -using System.Threading.Tasks; using GovUk.Education.ExploreEducationStatistics.Admin.Models; +using GovUk.Education.ExploreEducationStatistics.Admin.ViewModels; using GovUk.Education.ExploreEducationStatistics.Common.Model; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; +using System; +using System.Collections.Generic; +using System.Threading; +using System.Threading.Tasks; namespace GovUk.Education.ExploreEducationStatistics.Admin.Services.Interfaces { @@ -44,8 +46,14 @@ Task> UploadAsZip( string? dataSetTitle, Guid? replacingFileId); - Task>> UploadAsBulkZip( + Task>> ValidateAndUploadBulkZip( + Guid releaseVersionId, + IFormFile zipFile, + CancellationToken cancellationToken); + + Task>> SaveDataSetsFromTemporaryBlobStorage( Guid releaseVersionId, - IFormFile bulkZipFormFile); + List archiveDataSetFiles, + CancellationToken cancellationToken); } } diff --git a/src/GovUk.Education.ExploreEducationStatistics.Admin/Services/ReleaseDataFileService.cs b/src/GovUk.Education.ExploreEducationStatistics.Admin/Services/ReleaseDataFileService.cs index 30286f4414b..8da1a64fc67 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Admin/Services/ReleaseDataFileService.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Admin/Services/ReleaseDataFileService.cs @@ -1,13 +1,11 @@ #nullable enable -using System; -using System.Collections.Generic; -using System.Linq; -using System.Threading.Tasks; using GovUk.Education.ExploreEducationStatistics.Admin.Models; using GovUk.Education.ExploreEducationStatistics.Admin.Services.Interfaces; using GovUk.Education.ExploreEducationStatistics.Admin.Services.Interfaces.Security; +using GovUk.Education.ExploreEducationStatistics.Admin.ViewModels; using GovUk.Education.ExploreEducationStatistics.Common.Extensions; using GovUk.Education.ExploreEducationStatistics.Common.Model; +using GovUk.Education.ExploreEducationStatistics.Common.Services; using GovUk.Education.ExploreEducationStatistics.Common.Services.Interfaces; using GovUk.Education.ExploreEducationStatistics.Common.Services.Interfaces.Security; using GovUk.Education.ExploreEducationStatistics.Common.Utils; @@ -18,10 +16,15 @@ using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; using Microsoft.EntityFrameworkCore; +using System; +using System.Collections.Generic; +using System.IO.Compression; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; using static GovUk.Education.ExploreEducationStatistics.Admin.Validators.ValidationErrorMessages; using static GovUk.Education.ExploreEducationStatistics.Admin.Validators.ValidationUtils; using static GovUk.Education.ExploreEducationStatistics.Common.BlobContainers; -using static GovUk.Education.ExploreEducationStatistics.Common.Model.FileType; using IReleaseVersionRepository = GovUk.Education.ExploreEducationStatistics.Admin.Services.Interfaces.IReleaseVersionRepository; using Unit = GovUk.Education.ExploreEducationStatistics.Common.Model.Unit; @@ -75,7 +78,7 @@ public async Task> Delete(Guid releaseVersionId, Guid fileId, bool forceDelete = false) { - return await Delete(releaseVersionId, [ fileId ], forceDelete: forceDelete); + return await Delete(releaseVersionId, [fileId], forceDelete: forceDelete); } public async Task> Delete(Guid releaseVersionId, @@ -277,7 +280,8 @@ public async Task> Upload(Guid releaseVersion return newDataSetTitle; }) - .OnSuccess(async tuple => { + .OnSuccess(async tuple => + { var (replacingFile, newDataSetTitle) = tuple; @@ -309,7 +313,7 @@ await _releaseVersionRepository subjectId: subjectId, filename: metaFormFile.FileName.ToLower(), contentLength: metaFormFile.Length, - type: Metadata, + type: FileType.Metadata, createdById: _userService.GetUserId()); await UploadFileToStorage(dataFile, dataFormFile); @@ -367,7 +371,7 @@ public async Task> UploadAsZip(Guid releaseVe ContentLength = zipFormFile.Length, ContentType = zipFormFile.ContentType, Filename = zipFormFile.FileName.ToLower(), - Type = DataZip, + Type = FileType.DataZip, }; _contentDbContext.Files.Add(zipFile); await _contentDbContext.SaveChangesAsync(); @@ -401,7 +405,7 @@ public async Task> UploadAsZip(Guid releaseVe subjectId: subjectId, filename: archiveDataSet.MetaFilename, contentLength: archiveDataSet.MetaFileSize, - type: Metadata, + type: FileType.Metadata, createdById: _userService.GetUserId(), source: zipFile); @@ -428,89 +432,138 @@ public async Task> UploadAsZip(Guid releaseVe }); } - public async Task>> UploadAsBulkZip( + public async Task>> ValidateAndUploadBulkZip( Guid releaseVersionId, - IFormFile bulkZipFormFile) + IFormFile zipFile, + CancellationToken cancellationToken) { return await _persistenceHelper .CheckEntityExists(releaseVersionId) .OnSuccess(_userService.CheckCanUpdateReleaseVersion) - .OnSuccess(_ => _dataArchiveValidationService.ValidateBulkDataArchiveFile( - releaseVersionId, bulkZipFormFile) - .OnSuccess(async dataArchiveFiles => + .OnSuccessVoid(async _ => { - var bulkZipFile = new File + var errors = await _dataArchiveValidationService.IsValidZipFile(zipFile); + if (errors.Count > 0) { - CreatedById = _userService.GetUserId(), - RootPath = releaseVersionId, - ContentLength = bulkZipFormFile.Length, - ContentType = bulkZipFormFile.ContentType, - Filename = bulkZipFormFile.FileName.ToLower(), - Type = BulkDataZip, - }; - _contentDbContext.Files.Add(bulkZipFile); - await _contentDbContext.SaveChangesAsync(); + return new Either(Common.Validators.ValidationUtils.ValidationResult(errors)); + } + + return Unit.Instance; + }) + .OnSuccess(async _ => await _dataArchiveValidationService.ValidateBulkDataArchiveFiles(releaseVersionId, zipFile)) + .OnSuccess(async dataSetFiles => + { + await using var stream = zipFile.OpenReadStream(); + var archive = new ZipArchive(stream); - // each data/meta file pair are extracted to blob storage by _dataImportService.Import - await UploadFileToStorage(bulkZipFile, bulkZipFormFile); + var viewModels = new List(); - var results = new List(); + foreach (var file in dataSetFiles) + { + var dataFileId = await UploadFileToTempStorage(releaseVersionId, archive.GetEntry(file.DataFilename)!, FileType.Data, cancellationToken); + var metaFileId = await UploadFileToTempStorage(releaseVersionId, archive.GetEntry(file.MetaFilename)!, FileType.Metadata, cancellationToken); - foreach (var archiveFile in dataArchiveFiles) + viewModels.Add(new ArchiveDataSetFileViewModel { - var subjectId = await _releaseVersionRepository - .CreateStatisticsDbReleaseAndSubjectHierarchy(releaseVersionId); + Title = file.Title, + DataFilename = file.DataFilename, + MetaFilename = file.MetaFilename, + DataFileId = dataFileId, + MetaFileId = metaFileId, + DataFileSize = file.DataFileSize, + MetaFileSize = file.MetaFileSize, + ReplacingFileId = file.ReplacingFile?.Id, + }); + } - var releaseDataFileOrder = await GetNextDataFileOrder( - releaseVersionId, archiveFile.ReplacingFile); + return viewModels; + }); + } - var dataFile = await _releaseDataFileRepository.Create( - releaseVersionId: releaseVersionId, - subjectId: subjectId, - filename: archiveFile.DataFilename, - contentLength: archiveFile.DataFileSize, - type: FileType.Data, - createdById: _userService.GetUserId(), - name: archiveFile.Title, - replacingDataFile: archiveFile.ReplacingFile, - source: bulkZipFile, - order: releaseDataFileOrder); + public async Task>> SaveDataSetsFromTemporaryBlobStorage( + Guid releaseVersionId, + List archiveDataSetFiles, + CancellationToken cancellationToken) + { + return await _persistenceHelper + .CheckEntityExists(releaseVersionId) + .OnSuccess(_userService.CheckCanUpdateReleaseVersion) + .OnSuccess(() => archiveDataSetFiles + .Select(importRequest => ValidateTempDataSetFileExistence(releaseVersionId, importRequest)) + .OnSuccessAll()) + .OnSuccess(async _ => + { + var releaseFiles = new List(); - var dataReleaseFile = await _contentDbContext.ReleaseFiles - .Include(rf => rf.File) - .SingleAsync(rf => - rf.ReleaseVersionId == releaseVersionId - && rf.FileId == dataFile.Id); + foreach (var archiveDataSetFile in archiveDataSetFiles) + { + var subjectId = await _releaseVersionRepository.CreateStatisticsDbReleaseAndSubjectHierarchy(releaseVersionId); - var metaFile = await _releaseDataFileRepository.Create( - releaseVersionId: releaseVersionId, - subjectId: subjectId, - filename: archiveFile.MetaFilename, - contentLength: archiveFile.MetaFileSize, - type: Metadata, - createdById: _userService.GetUserId(), - source: bulkZipFile); + var replacingFile = archiveDataSetFile.ReplacingFileId is null + ? null + : await _contentDbContext.Files.FirstAsync(f => f.Id == archiveDataSetFile.ReplacingFileId); - var dataImport = await _dataImportService.Import( - subjectId: subjectId, - dataFile: dataFile, - metaFile: metaFile, - sourceZipFile: bulkZipFile); + var releaseDataFileOrder = await GetNextDataFileOrder(releaseVersionId, replacingFile); - var permissions = await _userService.GetDataFilePermissions(dataFile); + var dataFile = await _releaseDataFileRepository.Create( + releaseVersionId: releaseVersionId, + subjectId: subjectId, + filename: archiveDataSetFile.DataFilename, + contentLength: archiveDataSetFile.DataFileSize, + type: FileType.Data, + createdById: _userService.GetUserId(), + name: archiveDataSetFile.Title, + replacingDataFile: replacingFile, + order: releaseDataFileOrder); + + var sourceDataFilePath = FileStoragePathUtils.FilesPath(releaseVersionId, FileType.Data, archiveDataSetFile.DataFileId); + var destinationDataFilePath = FileStoragePathUtils.FilesPath(releaseVersionId, FileType.Data, dataFile.Id); // Same path, but a new ID has been generated by the creation step above + + var dataReleaseFile = await _contentDbContext.ReleaseFiles + .Include(rf => rf.File) + .SingleAsync(rf => + rf.ReleaseVersionId == releaseVersionId + && rf.FileId == dataFile.Id); + + releaseFiles.Add(dataReleaseFile); + + var metaFile = await _releaseDataFileRepository.Create( + releaseVersionId: releaseVersionId, + subjectId: subjectId, + filename: archiveDataSetFile.MetaFilename, + contentLength: archiveDataSetFile.MetaFileSize, + type: FileType.Metadata, + createdById: _userService.GetUserId()); + + var sourceMetaFilePath = FileStoragePathUtils.FilesPath(releaseVersionId, FileType.Metadata, archiveDataSetFile.MetaFileId); + var destinationMetaFilePath = FileStoragePathUtils.FilesPath(releaseVersionId, FileType.Metadata, metaFile.Id); // Same path, but a new ID has been generated by the creation step above + + await _dataImportService.Import( + subjectId: subjectId, + dataFile: dataFile, + metaFile: metaFile); + + await _privateBlobStorageService.MoveBlob(PrivateReleaseTempFiles, sourceDataFilePath, destinationDataFilePath, PrivateReleaseFiles); + await _privateBlobStorageService.MoveBlob(PrivateReleaseTempFiles, sourceMetaFilePath, destinationMetaFilePath, PrivateReleaseFiles); + } - results.Add( - BuildDataFileViewModel( - dataReleaseFile: dataReleaseFile, - metaFile: metaFile, - archiveFile.Title, - dataImport.TotalRows, - dataImport.Status, - permissions)); - } + return await BuildDataFileViewModels(releaseFiles); + }); + } + + private async Task> ValidateTempDataSetFileExistence( + Guid releaseVersionId, + ArchiveDataSetFileViewModel fileImportRequest) + { + var dataBlobExists = await _privateBlobStorageService.CheckBlobExists(PrivateReleaseTempFiles, $"{FileStoragePathUtils.FilesPath(releaseVersionId, FileType.Data, fileImportRequest.DataFileId)}"); + var metaBlobExists = await _privateBlobStorageService.CheckBlobExists(PrivateReleaseTempFiles, $"{FileStoragePathUtils.FilesPath(releaseVersionId, FileType.Metadata, fileImportRequest.MetaFileId)}"); + + if (!dataBlobExists || !metaBlobExists) + { + throw new Exception("Unable to locate temporary files at the locations specified"); + } - return results; - })); + return Unit.Instance; } private async Task BuildDataFileViewModel(ReleaseFile releaseFile) @@ -636,14 +689,33 @@ await _privateBlobStorageService.UploadFile( ); } + private async Task UploadFileToTempStorage( + Guid releaseVersionId, + ZipArchiveEntry archiveEntry, + FileType fileType, + CancellationToken cancellationToken) + { + var fileId = Guid.NewGuid(); + var path = $"{FileStoragePathUtils.FilesPath(releaseVersionId, fileType)}{fileId}"; + + await _privateBlobStorageService.UploadStream( + containerName: PrivateReleaseTempFiles, + path: path, + stream: archiveEntry.Open(), + contentType: ContentTypes.Csv, + cancellationToken: cancellationToken); + + return fileId; + } + private async Task GetAssociatedMetaFile(Guid releaseVersionId, File dataFile) { return await _contentDbContext.ReleaseFiles .Include(rf => rf.File) .Where(rf => rf.ReleaseVersionId == releaseVersionId - && rf.File.Type == Metadata - && rf.File.SubjectId == dataFile.SubjectId) + && rf.File.Type == FileType.Metadata + && rf.File.SubjectId == dataFile.SubjectId) .Select(rf => rf.File) .SingleAsync(); } diff --git a/src/GovUk.Education.ExploreEducationStatistics.Admin/Validators/ValidationMessages.cs b/src/GovUk.Education.ExploreEducationStatistics.Admin/Validators/ValidationMessages.cs index 68a5d90c604..46f26173e4b 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Admin/Validators/ValidationMessages.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Admin/Validators/ValidationMessages.cs @@ -1,9 +1,9 @@ #nullable enable -using System.Collections.Generic; using GovUk.Education.ExploreEducationStatistics.Common.Extensions; using GovUk.Education.ExploreEducationStatistics.Common.Model; using GovUk.Education.ExploreEducationStatistics.Common.ViewModels; using GovUk.Education.ExploreEducationStatistics.Public.Data.Model; +using System.Collections.Generic; namespace GovUk.Education.ExploreEducationStatistics.Admin.Validators; @@ -177,6 +177,34 @@ public static ErrorViewModel GenerateErrorDatasetNamesCsvFilenamesShouldBeUnique Message: "Inside dataset_names.csv, file_name cell entries should not end in '.csv' i.e. should be 'filename' not 'filename.csv'. Filename found with extension: '{0}'." ); + public static readonly LocalizableMessage FileIsNull = new( + Code: nameof(FileIsNull), + Message: "No file provided." + ); + + public static ErrorViewModel GenerateErrorFileIsNull() + { + return new ErrorViewModel + { + Code = FileIsNull.Code, + Message = FileIsNull.Message, + }; + } + + public static readonly LocalizableMessage DataReplacementAlreadyInProgress = new( + Code: nameof(DataReplacementAlreadyInProgress), + Message: "Data replacement already in progress" + ); + + public static ErrorViewModel GenerateErrorDataReplacementAlreadyInProgress() + { + return new ErrorViewModel + { + Code = DataReplacementAlreadyInProgress.Code, + Message = DataReplacementAlreadyInProgress.Message, + }; + } + public static ErrorViewModel GenerateErrorDatasetNamesCsvFilenamesShouldNotEndDotCsv(string filename) { return new ErrorViewModel diff --git a/src/GovUk.Education.ExploreEducationStatistics.Admin/ViewModels/ArchiveDataSetFileViewModel.cs b/src/GovUk.Education.ExploreEducationStatistics.Admin/ViewModels/ArchiveDataSetFileViewModel.cs new file mode 100644 index 00000000000..d7826d13742 --- /dev/null +++ b/src/GovUk.Education.ExploreEducationStatistics.Admin/ViewModels/ArchiveDataSetFileViewModel.cs @@ -0,0 +1,23 @@ +#nullable enable +using System; + +namespace GovUk.Education.ExploreEducationStatistics.Admin.ViewModels; + +public record ArchiveDataSetFileViewModel +{ + public string Title { get; set; } = string.Empty; + + public string DataFilename { get; set; } = string.Empty; + + public Guid DataFileId { get; set; } + + public long DataFileSize { get; set; } + + public string MetaFilename { get; set; } = string.Empty; + + public Guid MetaFileId { get; set; } + + public long MetaFileSize { get; set; } + + public Guid? ReplacingFileId { get; set; } +} diff --git a/src/GovUk.Education.ExploreEducationStatistics.Common/BlobContainers.cs b/src/GovUk.Education.ExploreEducationStatistics.Common/BlobContainers.cs index 5ef238e86bb..1569c9d91c1 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Common/BlobContainers.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Common/BlobContainers.cs @@ -1,4 +1,4 @@ -namespace GovUk.Education.ExploreEducationStatistics.Common +namespace GovUk.Education.ExploreEducationStatistics.Common { public interface IBlobContainer { @@ -9,6 +9,7 @@ public interface IBlobContainer public static class BlobContainers { public static readonly IBlobContainer PrivateReleaseFiles = new BlobContainer("releases"); + public static readonly IBlobContainer PrivateReleaseTempFiles = new BlobContainer("releases-temp"); public static readonly IBlobContainer PublicReleaseFiles = new BlobContainer("downloads"); public static readonly IBlobContainer PrivateContent = new PrivateBlobContainer("cache"); public static readonly IBlobContainer PublicContent = new PublicBlobContainer("cache"); diff --git a/src/GovUk.Education.ExploreEducationStatistics.Common/Services/BlobStorageService.cs b/src/GovUk.Education.ExploreEducationStatistics.Common/Services/BlobStorageService.cs index 5abd806a12e..9a82c518df9 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Common/Services/BlobStorageService.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Common/Services/BlobStorageService.cs @@ -1,10 +1,4 @@ #nullable enable -using System; -using System.Collections.Generic; -using System.IO; -using System.Net.Mime; -using System.Threading; -using System.Threading.Tasks; using Azure; using Azure.Storage.Blobs; using Azure.Storage.Blobs.Models; @@ -21,6 +15,12 @@ using Microsoft.Azure.Storage.DataMovement; using Microsoft.Extensions.Logging; using Newtonsoft.Json; +using System; +using System.Collections.Generic; +using System.IO; +using System.Net.Mime; +using System.Threading; +using System.Threading.Tasks; using BlobInfo = GovUk.Education.ExploreEducationStatistics.Common.Model.BlobInfo; using BlobProperties = Azure.Storage.Blobs.Models.BlobProperties; using CopyStatus = Azure.Storage.Blobs.Models.CopyStatus; @@ -223,27 +223,33 @@ await blob.UploadAsync( } public async Task MoveBlob( - IBlobContainer containerName, + IBlobContainer sourceContainer, string sourcePath, - string destinationPath) + string destinationPath, + IBlobContainer? destinationContainer = null) { - var blobContainer = await GetBlobContainer(containerName); + var sourceContainerClient = await GetBlobContainer(sourceContainer); + var destinationContainerClient = destinationContainer is not null + ? await GetBlobContainer(destinationContainer) + : sourceContainerClient; - var destinationBlob = blobContainer.GetBlobClient(destinationPath); - if (await destinationBlob.ExistsAsync()) + var sourceBlob = sourceContainerClient.GetBlobClient(sourcePath); + if (!await sourceBlob.ExistsAsync()) { _logger.LogWarning( - "Destination already exists while moving blob. Source: '{source}' Destination: '{destination}'", - sourcePath, destinationPath); + "Source blob not found while moving blob. Source: '{Source}' Destination: '{Destination}'", + sourcePath, + destinationPath); return false; } - var sourceBlob = blobContainer.GetBlobClient(sourcePath); - if (!await sourceBlob.ExistsAsync()) + var destinationBlob = destinationContainerClient.GetBlobClient(destinationPath); + if (await destinationBlob.ExistsAsync()) { _logger.LogWarning( - "Source blob not found while moving blob. Source: '{source}' Destination: '{destination}'", - sourcePath, destinationPath); + "Destination already exists while moving blob. Source: '{Source}' Destination: '{Destination}'", + sourcePath, + destinationPath); return false; } @@ -263,7 +269,7 @@ public async Task MoveBlob( while (destinationProperties.CopyStatus == CopyStatus.Pending) { await Task.Delay(1000); - _logger.LogInformation("Copy progress: {progress}", destinationProperties.CopyProgress); + _logger.LogInformation("Copy progress: {Progress}", destinationProperties.CopyProgress); destinationProperties = await destinationBlob.GetPropertiesAsync(); } diff --git a/src/GovUk.Education.ExploreEducationStatistics.Common/Services/FileStoragePathUtils.cs b/src/GovUk.Education.ExploreEducationStatistics.Common/Services/FileStoragePathUtils.cs index 3b1c6bee4a5..2c1740207a0 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Common/Services/FileStoragePathUtils.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Common/Services/FileStoragePathUtils.cs @@ -99,10 +99,10 @@ public static string PublicContentReleaseSubjectsPath(string publicationSlug, st return $"{PublicContentReleaseParentPath(publicationSlug, releaseSlug)}/subjects.json"; } - public static string FilesPath(Guid rootPath, FileType type) + public static string FilesPath(Guid rootPath, FileType type, Guid? filePath = null) { var typeFolder = (type == Metadata ? Data : type).GetEnumLabel(); - return $"{rootPath}/{typeFolder}/"; + return $"{rootPath}/{typeFolder}/{filePath}"; } private static string AppendPathSeparator(string? segment = null) diff --git a/src/GovUk.Education.ExploreEducationStatistics.Common/Services/Interfaces/IBlobStorageService.cs b/src/GovUk.Education.ExploreEducationStatistics.Common/Services/Interfaces/IBlobStorageService.cs index c567ec07ba5..79e9f3d2d8e 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Common/Services/Interfaces/IBlobStorageService.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Common/Services/Interfaces/IBlobStorageService.cs @@ -1,15 +1,15 @@ #nullable enable +using GovUk.Education.ExploreEducationStatistics.Common.Model; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Microsoft.Azure.Storage.DataMovement; +using Newtonsoft.Json; using System; using System.Collections.Generic; using System.IO; using System.Text.RegularExpressions; using System.Threading; using System.Threading.Tasks; -using GovUk.Education.ExploreEducationStatistics.Common.Model; -using Microsoft.AspNetCore.Http; -using Microsoft.AspNetCore.Mvc; -using Microsoft.Azure.Storage.DataMovement; -using Newtonsoft.Json; namespace GovUk.Education.ExploreEducationStatistics.Common.Services.Interfaces { @@ -36,7 +36,11 @@ public Task DeleteBlobs( public Task DeleteBlob(IBlobContainer containerName, string path); - public Task MoveBlob(IBlobContainer containerName, string sourcePath, string destinationPath); + public Task MoveBlob( + IBlobContainer sourceContainer, + string sourcePath, + string destinationPath, + IBlobContainer? destinationContainer = null); public Task UploadFile( IBlobContainer containerName, diff --git a/src/GovUk.Education.ExploreEducationStatistics.Content.Model/Services/PrivateReleaseFileBlobService.cs b/src/GovUk.Education.ExploreEducationStatistics.Content.Model/Services/PrivateReleaseFileBlobService.cs index a2e9b12519a..da21dc1943a 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Content.Model/Services/PrivateReleaseFileBlobService.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Content.Model/Services/PrivateReleaseFileBlobService.cs @@ -46,7 +46,7 @@ public Task DeleteFile(ReleaseFile releaseFile) public Task MoveBlob(ReleaseFile releaseFile, string destinationPath) { return _privateBlobStorageService.MoveBlob( - containerName: PrivateReleaseFiles, + sourceContainer: PrivateReleaseFiles, sourcePath: releaseFile.Path(), destinationPath: destinationPath ); diff --git a/src/GovUk.Education.ExploreEducationStatistics.Content.Model/Services/PublicReleaseFileBlobService.cs b/src/GovUk.Education.ExploreEducationStatistics.Content.Model/Services/PublicReleaseFileBlobService.cs index 5771fd82f85..1436b680ab1 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Content.Model/Services/PublicReleaseFileBlobService.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Content.Model/Services/PublicReleaseFileBlobService.cs @@ -46,7 +46,7 @@ public Task DeleteFile(ReleaseFile releaseFile) public Task MoveBlob(ReleaseFile releaseFile, string destinationPath) { return _publicBlobStorageService.MoveBlob( - containerName: PublicReleaseFiles, + sourceContainer: PublicReleaseFiles, sourcePath: releaseFile.PublicPath(), destinationPath: destinationPath); } From d2cc7b47fc96ebce84c506aef619b11dc12acc5f Mon Sep 17 00:00:00 2001 From: Tom Jones Date: Mon, 2 Dec 2024 11:01:57 +0000 Subject: [PATCH 2/4] EES-5584: Add modal to display file upload/replacement plan. --- .../release/data/ReleaseDataFilePage.tsx | 11 +++ .../components/BulkZipUploadModalConfirm.tsx | 51 ++++++++++++++ .../data/components/DataFileUploadForm.tsx | 3 + .../components/ReleaseDataUploadsSection.tsx | 70 ++++++++++++++----- .../src/services/releaseDataFileService.ts | 28 ++++++-- 5 files changed, 140 insertions(+), 23 deletions(-) create mode 100644 src/explore-education-statistics-admin/src/pages/release/data/components/BulkZipUploadModalConfirm.tsx diff --git a/src/explore-education-statistics-admin/src/pages/release/data/ReleaseDataFilePage.tsx b/src/explore-education-statistics-admin/src/pages/release/data/ReleaseDataFilePage.tsx index eabe7adef74..1fbbd215c72 100644 --- a/src/explore-education-statistics-admin/src/pages/release/data/ReleaseDataFilePage.tsx +++ b/src/explore-education-statistics-admin/src/pages/release/data/ReleaseDataFilePage.tsx @@ -15,6 +15,8 @@ import { generatePath, RouteComponentProps } from 'react-router'; import LoadingSpinner from '@common/components/LoadingSpinner'; import Yup from '@common/validation/yup'; import Button from '@common/components/Button'; +import { useQueryClient } from '@tanstack/react-query'; +import releaseDataFileQueries from '@admin/queries/releaseDataFileQueries'; interface FormValues { title: string; @@ -31,11 +33,20 @@ export default function ReleaseDataFilePage({ [releaseId, fileId], ); + const queryClient = useQueryClient(); + const handleSubmit = async (values: FormValues) => { await releaseDataFileService.updateFile(releaseId, fileId, { title: values.title, }); + queryClient.removeQueries({ + queryKey: releaseDataFileQueries.list(releaseId).queryKey, + }); + await queryClient.invalidateQueries({ + queryKey: releaseDataFileQueries.list._def, + }); + history.push( generatePath(releaseDataRoute.path, { publicationId, diff --git a/src/explore-education-statistics-admin/src/pages/release/data/components/BulkZipUploadModalConfirm.tsx b/src/explore-education-statistics-admin/src/pages/release/data/components/BulkZipUploadModalConfirm.tsx new file mode 100644 index 00000000000..fd5b252d451 --- /dev/null +++ b/src/explore-education-statistics-admin/src/pages/release/data/components/BulkZipUploadModalConfirm.tsx @@ -0,0 +1,51 @@ +import ModalConfirm from '@common/components/ModalConfirm'; +import WarningMessage from '@common/components/WarningMessage'; +import { ArchiveDataSetFile } from 'src/services/releaseDataFileService'; +import React from 'react'; + +interface Props { + bulkUploadPlan: ArchiveDataSetFile[]; + onConfirm: (bulkUploadPlan: ArchiveDataSetFile[]) => void; + onCancel: () => void; +} + +export default function BulkZipUploadModalConfirm({ + bulkUploadPlan, + onConfirm, + onCancel, +}: Props) { + return ( + onConfirm(bulkUploadPlan)} + onExit={onCancel} + onCancel={onCancel} + > + + + + + + + + + + {bulkUploadPlan.map(archiveDataSet => ( + + + + + + ))} + +
Dataset nameData fileAdditional information
{archiveDataSet.title}{archiveDataSet.dataFilename} + {archiveDataSet.replacingFileId && ( + + Upload will initiate a file replacement + + )} +
+ + ); +} diff --git a/src/explore-education-statistics-admin/src/pages/release/data/components/DataFileUploadForm.tsx b/src/explore-education-statistics-admin/src/pages/release/data/components/DataFileUploadForm.tsx index aa8da292515..62380b403f0 100644 --- a/src/explore-education-statistics-admin/src/pages/release/data/components/DataFileUploadForm.tsx +++ b/src/explore-education-statistics-admin/src/pages/release/data/components/DataFileUploadForm.tsx @@ -80,6 +80,8 @@ function baseErrorMappings( 'DatasetNamesCsvFilenamesShouldBeUnique', FileNotFoundInZip: 'FileNotFoundInZip', ZipContainsUnusedFiles: 'ZipContainsUnusedFiles', + DataReplacementAlreadyInProgress: + 'Data replacement already in progress', }, }), ]; @@ -218,6 +220,7 @@ export default function DataFileUploadForm({ > {({ formState, reset, getValues }) => { const uploadType = getValues('uploadType'); + return (
diff --git a/src/explore-education-statistics-admin/src/pages/release/data/components/ReleaseDataUploadsSection.tsx b/src/explore-education-statistics-admin/src/pages/release/data/components/ReleaseDataUploadsSection.tsx index 89f58586370..8238ffb61de 100644 --- a/src/explore-education-statistics-admin/src/pages/release/data/components/ReleaseDataUploadsSection.tsx +++ b/src/explore-education-statistics-admin/src/pages/release/data/components/ReleaseDataUploadsSection.tsx @@ -17,6 +17,7 @@ import { } from '@admin/routes/releaseRoutes'; import permissionService from '@admin/services/permissionService'; import releaseDataFileService, { + ArchiveDataSetFile, DataFile, DataFileImportStatus, DeleteDataFilePlan, @@ -32,7 +33,7 @@ import DataUploadCancelButton from '@admin/pages/release/data/components/DataUpl import React, { useCallback, useEffect, useState } from 'react'; import { generatePath } from 'react-router'; import { useQuery } from '@tanstack/react-query'; -import { reverse, uniqBy } from 'lodash'; +import BulkZipUploadModalConfirm from './BulkZipUploadModalConfirm'; interface Props { publicationId: string; @@ -55,17 +56,18 @@ const ReleaseDataUploadsSection = ({ const [deleteDataFile, setDeleteDataFile] = useState(); const [activeFileIds, setActiveFileIds] = useState(); const [dataFiles, setDataFiles] = useState([]); + const [bulkUploadPlan, setBulkUploadPlan] = useState(); - const { data: initialDataFiles = [], isLoading } = useQuery( - releaseDataFileQueries.list(releaseId), - ); + const { + data: initialDataFiles = [], + isLoading, + refetch: refetchDataFiles, + } = useQuery(releaseDataFileQueries.list(releaseId)); // Store the data files on state so we can reliably update them // when the permissions/status change. useEffect(() => { - if (!isLoading) { - setDataFiles(initialDataFiles); - } + setDataFiles(initialDataFiles); }, [initialDataFiles, isLoading, setDataFiles]); useEffect(() => { @@ -85,6 +87,26 @@ const ReleaseDataUploadsSection = ({ ); }; + const confirmBulkUploadPlan = useCallback( + async (archiveDataSetFiles: ArchiveDataSetFile[]) => { + const newFiles = await releaseDataFileService.importBulkZipDataFile( + releaseId, + archiveDataSetFiles, + ); + + setBulkUploadPlan(undefined); + setActiveFileIds([...dataFiles, ...newFiles].map(file => file.id)); + refetchDataFiles(); + }, + [ + releaseId, + setBulkUploadPlan, + setActiveFileIds, + dataFiles, + refetchDataFiles, + ], + ); + const handleStatusChange = async ( dataFile: DataFile, { totalRows, status }: DataFileImportStatus, @@ -126,6 +148,7 @@ const ReleaseDataUploadsSection = ({ metadataFile: values.metadataFile as File, }), ); + refetchDataFiles(); break; } case 'zip': { @@ -138,26 +161,25 @@ const ReleaseDataUploadsSection = ({ zipFile: values.zipFile as File, }), ); + refetchDataFiles(); break; } - case 'bulkZip': - newFiles.push( - ...(await releaseDataFileService.uploadBulkZipDataFile( + case 'bulkZip': { + const uploadPlan = + await releaseDataFileService.getUploadBulkZipDataFilePlan( releaseId, - values.bulkZipFile as File, - )), - ); + values.bulkZipFile!, + ); + + setBulkUploadPlan(uploadPlan); break; + } default: break; } setActiveFileIds(newFiles.map(file => file.id)); - setDataFiles(currentDataFiles => - // double reverse keeps *new* files as uniq item and at the end of the list - reverse(uniqBy(reverse([...currentDataFiles, ...newFiles]), 'title')), - ); }, - [releaseId], + [releaseId, refetchDataFiles], ); return ( @@ -191,7 +213,17 @@ const ReleaseDataUploadsSection = ({ {canUpdateRelease ? ( - + <> + + {bulkUploadPlan === undefined || + bulkUploadPlan.length === 0 ? null : ( + setBulkUploadPlan(undefined)} + /> + )} + ) : ( This release has been approved, and can no longer be updated. diff --git a/src/explore-education-statistics-admin/src/services/releaseDataFileService.ts b/src/explore-education-statistics-admin/src/services/releaseDataFileService.ts index 2a1c496e809..354352c2f0b 100644 --- a/src/explore-education-statistics-admin/src/services/releaseDataFileService.ts +++ b/src/explore-education-statistics-admin/src/services/releaseDataFileService.ts @@ -65,6 +65,17 @@ export type UploadZipDataFileRequest = zipFile: File; }; +export type ArchiveDataSetFile = { + title: string; + dataFilename: string; + dataFileId: string; + dataFileSize: number; + metaFilename: string; + metaFileId: string; + metaFileSize: number; + replacingFileId?: string; +}; + export interface DataFileUpdateRequest { title: string; } @@ -156,17 +167,26 @@ const releaseDataFileService = { return mapFile(file); }, - async uploadBulkZipDataFile( + async getUploadBulkZipDataFilePlan( releaseId: string, zipFile: File, - ): Promise { + ): Promise { const data = new FormData(); data.append('zipFile', zipFile); - const files = await client.post( - `/release/${releaseId}/bulk-zip-data`, + return client.post( + `/release/${releaseId}/upload-bulk-zip-data`, data, ); + }, + async importBulkZipDataFile( + releaseId: string, + dataSetFiles: ArchiveDataSetFile[], + ): Promise { + const files = await client.post( + `/release/${releaseId}/import-bulk-zip-data`, + dataSetFiles, + ); return files.map(file => mapFile(file)); }, From 02915092a18db65917df1a512c9f65b2a98b3cc5 Mon Sep 17 00:00:00 2001 From: Tom Jones Date: Tue, 3 Dec 2024 17:39:50 +0000 Subject: [PATCH 3/4] EES-5583/4: Update backend and frontend tests. --- .../Api/ReleasesControllerTests.cs | 66 ++++ .../DataArchiveValidationServiceTests.cs | 256 +++++---------- .../MethodologyImageServiceTests.cs | 8 +- .../ReleaseDataFileServicePermissionTests.cs | 31 +- .../Services/ReleaseDataFileServiceTests.cs | 293 ++++-------------- .../Services/ReleaseFileServiceTests.cs | 16 +- .../Services/ReleaseImageServiceTests.cs | 8 +- .../Services/BlobStorageServiceTests.cs | 78 ++++- .../Utils/MockFormTestUtils.cs | 26 +- .../components/ReleaseDataUploadsSection.tsx | 6 +- .../__tests__/DataFileUploadForm.test.tsx | 19 ++ .../ReleaseDataUploadsSection.test.tsx | 260 +++++----------- 12 files changed, 432 insertions(+), 635 deletions(-) diff --git a/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Controllers/Api/ReleasesControllerTests.cs b/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Controllers/Api/ReleasesControllerTests.cs index d91043be350..fced1e589a3 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Controllers/Api/ReleasesControllerTests.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Controllers/Api/ReleasesControllerTests.cs @@ -448,6 +448,72 @@ public async Task CreateReleaseAmendment() result.AssertOkResult(amendmentCreatedResponse); } + [Fact] + public async Task UploadBulkZipDataSetsToTempStorage() + { + // Arrange + var dataSetFiles = new List(); + + var releaseDataFileService = new Mock(Strict); + + releaseDataFileService + .Setup(s => s.ValidateAndUploadBulkZip( + It.IsAny(), + It.IsAny(), + default)) + .ReturnsAsync(dataSetFiles); + + var controller = BuildController(releaseDataFileService: releaseDataFileService.Object); + + // Act + var result = await controller.UploadBulkZipDataSetsToTempStorage( + Guid.NewGuid(), + MockFile("bulk.zip"), + default); + + // Assert + VerifyAllMocks(releaseDataFileService); + + result.AssertOkResult(dataSetFiles); + } + + [Fact] + public async Task ImportBulkZipDataSetsFromTempStorage() + { + // Arrange + var dataFileInfo = new List + { + new() { FileName = "one.csv", Name = "Data set title", Size = "1024" }, + }; + + var importRequests = new List + { + new(){ DataFileId = Guid.NewGuid(), MetaFileId = Guid.NewGuid(), Title = "Data set title", DataFilename = "one.csv", MetaFilename = "one.meta.csv", DataFileSize = 1024, MetaFileSize = 128 } + }; + + var releaseDataFileService = new Mock(Strict); + + releaseDataFileService + .Setup(s => s.SaveDataSetsFromTemporaryBlobStorage( + It.IsAny(), + It.IsAny>(), + default)) + .ReturnsAsync(dataFileInfo); + + var controller = BuildController(releaseDataFileService: releaseDataFileService.Object); + + // Act + var result = await controller.ImportBulkZipDataSetsFromTempStorage( + Guid.NewGuid(), + importRequests, + default); + + // Assert + VerifyAllMocks(releaseDataFileService); + + result.AssertOkResult(dataFileInfo); + } + private static IFormFile MockFile(string fileName) { var fileMock = new Mock(Strict); diff --git a/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/DataArchiveValidationServiceTests.cs b/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/DataArchiveValidationServiceTests.cs index 6917b2b33a3..9faafa5ccdd 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/DataArchiveValidationServiceTests.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/DataArchiveValidationServiceTests.cs @@ -1,8 +1,4 @@ #nullable enable -using System; -using System.IO; -using System.Reflection; -using System.Threading.Tasks; using GovUk.Education.ExploreEducationStatistics.Admin.Models; using GovUk.Education.ExploreEducationStatistics.Admin.Services; using GovUk.Education.ExploreEducationStatistics.Admin.Services.Interfaces; @@ -14,9 +10,13 @@ using GovUk.Education.ExploreEducationStatistics.Content.Model.Database; using Microsoft.AspNetCore.Http; using Moq; +using System; +using System.IO; +using System.Threading.Tasks; +using static GovUk.Education.ExploreEducationStatistics.Common.Tests.Extensions.ValidationProblemViewModelTestExtensions; +using static GovUk.Education.ExploreEducationStatistics.Common.Tests.Utils.MockFormTestUtils; using static GovUk.Education.ExploreEducationStatistics.Common.Tests.Utils.MockUtils; using static Moq.MockBehavior; -using File = System.IO.File; using ValidationMessages = GovUk.Education.ExploreEducationStatistics.Admin.Validators.ValidationMessages; namespace GovUk.Education.ExploreEducationStatistics.Admin.Tests.Services @@ -24,7 +24,7 @@ namespace GovUk.Education.ExploreEducationStatistics.Admin.Tests.Services public class DataArchiveValidationServiceTests { [Fact] - public async Task ValidateDataArchiveFile_Success() + public async Task ValidateDataArchiveFile_Success_ReturnsArchiveSummary() { var releaseVersionId = Guid.NewGuid(); var archive = CreateFormFileFromResource("data-zip-valid.zip"); @@ -63,44 +63,7 @@ public async Task ValidateDataArchiveFile_Success() } [Fact] - public async Task ValidateDataArchiveFile_ZipFilenameTooLong_DoesNotEndInDotZip() - { - var fileTypeService = new Mock(Strict); - - var fileName = - "LoremipsumdolorsitametconsecteturadipiscingelitInsitametelitaccumsanbibendumlacusutmattismaurisCrasvehiculaaccumsaneratidelementumaugueposuereatNuncege.zipp"; - var archive = CreateFormFileFromResource("data-zip-valid.zip", fileName); - - fileTypeService - .Setup(s => s.IsValidZipFile(archive)) - .ReturnsAsync(() => true); - - var contentDbContextId = Guid.NewGuid().ToString(); - await using (var contentDbContext = DbUtils.InMemoryApplicationDbContext(contentDbContextId)) - { - var service = SetupDataArchiveValidationService( - contentDbContext: contentDbContext, - fileTypeService: fileTypeService.Object); - - var result = await service.ValidateDataArchiveFile( - Guid.NewGuid(), - "Data set title", - archive); - - result - .AssertLeft() - .AssertBadRequestWithValidationErrors([ - ValidationMessages.GenerateErrorFilenameTooLong( - fileName, DataArchiveValidationService.MaxFilenameSize), - ValidationMessages.GenerateErrorZipFilenameMustEndDotZip(fileName), - ]); - } - - VerifyAllMocks(fileTypeService); - } - - [Fact] - public async Task ValidateDataArchiveFile_DataZipShouldContainTwoFiles() + public async Task ValidateDataArchiveFile_ArchiveContainsOnlyOneFile_ReturnsValidationError() { var fileTypeService = new Mock(Strict); @@ -137,20 +100,18 @@ public async Task ValidateDataArchiveFile_DataZipShouldContainTwoFiles() } [Fact] - public async Task ValidateBulkDataArchiveFile_Success() + public async Task ValidateBulkDataArchiveFiles_Success_ReturnsArchiveSummary() { var releaseVersionId = Guid.NewGuid(); var archive = CreateFormFileFromResource("bulk-data-zip-valid.zip"); var fileUploadsValidatorService = new Mock(Strict); - var fileTypeService = new Mock(Strict); var contentDbContextId = Guid.NewGuid().ToString(); await using (var contentDbContext = DbUtils.InMemoryApplicationDbContext(contentDbContextId)) { var service = SetupDataArchiveValidationService( contentDbContext: contentDbContext, - fileTypeService: fileTypeService.Object, fileUploadsValidatorService: fileUploadsValidatorService.Object); fileUploadsValidatorService.Setup(mock => mock.ValidateDataSetFilesForUpload( @@ -160,11 +121,7 @@ public async Task ValidateBulkDataArchiveFile_Success() It.IsAny())) .ReturnsAsync([]); - fileTypeService - .Setup(s => s.IsValidZipFile(archive)) - .ReturnsAsync(true); - - var result = await service.ValidateBulkDataArchiveFile( + var result = await service.ValidateBulkDataArchiveFiles( releaseVersionId, archive); @@ -187,17 +144,16 @@ public async Task ValidateBulkDataArchiveFile_Success() Assert.Null(archiveDataSets[1].ReplacingFile); } - VerifyAllMocks(fileUploadsValidatorService, fileTypeService); + VerifyAllMocks(fileUploadsValidatorService); } [Fact] - public async Task ValidateBulkDataArchiveFile_Replacement_Success() + public async Task ValidateBulkDataArchiveFiles_WithFileReplacement_ReturnsArchiveSummary() { var releaseVersionId = Guid.NewGuid(); var archive = CreateFormFileFromResource("bulk-data-zip-valid.zip"); var fileUploadsValidatorService = new Mock(Strict); - var fileTypeService = new Mock(Strict); var releaseFile = new ReleaseFile { @@ -226,7 +182,6 @@ public async Task ValidateBulkDataArchiveFile_Replacement_Success() { var service = SetupDataArchiveValidationService( contentDbContext: contentDbContext, - fileTypeService: fileTypeService.Object, fileUploadsValidatorService: fileUploadsValidatorService.Object); fileUploadsValidatorService.Setup(mock => mock.ValidateDataSetFilesForUpload( @@ -236,11 +191,7 @@ public async Task ValidateBulkDataArchiveFile_Replacement_Success() It.IsAny())) .ReturnsAsync([]); - fileTypeService - .Setup(s => s.IsValidZipFile(archive)) - .ReturnsAsync(true); - - var result = await service.ValidateBulkDataArchiveFile( + var result = await service.ValidateBulkDataArchiveFiles( releaseVersionId, archive); @@ -263,67 +214,22 @@ public async Task ValidateBulkDataArchiveFile_Replacement_Success() Assert.Null(archiveDataSets[1].ReplacingFile); } - VerifyAllMocks(fileUploadsValidatorService, fileTypeService); - } - - [Fact] - public async Task ValidateBulkDataArchiveFile_Fail_IsValidZipFile() - { - var releaseVersionId = Guid.NewGuid(); - var fileTypeService = new Mock(Strict); - - var longFilename = - "loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooongfilename.csv"; - var archive = CreateFormFileFromResource("test-data.csv", longFilename); - - fileTypeService - .Setup(s => s.IsValidZipFile(archive)) - .ReturnsAsync(false); - - var contentDbContextId = Guid.NewGuid().ToString(); - await using (var contentDbContext = DbUtils.InMemoryApplicationDbContext(contentDbContextId)) - { - var service = SetupDataArchiveValidationService( - contentDbContext: contentDbContext, - fileTypeService: fileTypeService.Object); - - var result = await service.ValidateBulkDataArchiveFile( - releaseVersionId, - archive); - - result - .AssertLeft() - .AssertBadRequestWithValidationErrors([ - ValidationMessages.GenerateErrorFilenameTooLong(longFilename, - FileUploadsValidatorService.MaxFilenameSize), - ValidationMessages.GenerateErrorZipFilenameMustEndDotZip(longFilename), - ValidationMessages.GenerateErrorMustBeZipFile(longFilename), - ]); - } - - VerifyAllMocks(fileTypeService); + VerifyAllMocks(fileUploadsValidatorService); } [Fact] - public async Task ValidateBulkDataArchiveFile_Fail_NoDatasetNamesCsv() + public async Task ValidateBulkDataArchiveFiles_IndexFileMissing_ReturnsValidationError() { var releaseVersionId = Guid.NewGuid(); var archive = CreateFormFileFromResource("bulk-data-zip-invalid-no-datasetnames-csv.zip"); - var fileTypeService = new Mock(Strict); - - fileTypeService - .Setup(s => s.IsValidZipFile(archive)) - .ReturnsAsync(true); - var contentDbContextId = Guid.NewGuid().ToString(); await using (var contentDbContext = DbUtils.InMemoryApplicationDbContext(contentDbContextId)) { var service = SetupDataArchiveValidationService( - contentDbContext: contentDbContext, - fileTypeService: fileTypeService.Object); + contentDbContext: contentDbContext); - var result = await service.ValidateBulkDataArchiveFile( + var result = await service.ValidateBulkDataArchiveFiles( releaseVersionId, archive); @@ -337,30 +243,22 @@ public async Task ValidateBulkDataArchiveFile_Fail_NoDatasetNamesCsv() } ]); } - - VerifyAllMocks(fileTypeService); } [Fact] - public async Task ValidateBulkDataArchiveFile_Fail_DatasetNamesCsvIncorrectHeaders() + public async Task ValidateBulkDataArchiveFiles_IndexFileHasIncorrectHeaders_ReturnsValidationError() { var releaseVersionId = Guid.NewGuid(); - var fileTypeService = new Mock(Strict); var archive = CreateFormFileFromResource("bulk-data-zip-invalid-datasetnames-headers.zip"); - fileTypeService - .Setup(s => s.IsValidZipFile(archive)) - .ReturnsAsync(true); - var contentDbContextId = Guid.NewGuid().ToString(); await using (var contentDbContext = DbUtils.InMemoryApplicationDbContext(contentDbContextId)) { var service = SetupDataArchiveValidationService( - contentDbContext: contentDbContext, - fileTypeService: fileTypeService.Object); + contentDbContext: contentDbContext); - var result = await service.ValidateBulkDataArchiveFile( + var result = await service.ValidateBulkDataArchiveFiles( releaseVersionId, archive); @@ -374,30 +272,21 @@ public async Task ValidateBulkDataArchiveFile_Fail_DatasetNamesCsvIncorrectHeade }, ]); } - - VerifyAllMocks(fileTypeService); } [Fact] - public async Task ValidateBulkDataArchiveFile_Fail_FilesNotFoundInZip() + public async Task ValidateBulkDataArchiveFiles_ReferencedFilesNotFoundInArchive_ReturnsValidationError() { var releaseVersionId = Guid.NewGuid(); var archive = CreateFormFileFromResource("bulk-data-zip-invalid-files-not-found.zip"); - var fileTypeService = new Mock(Strict); - - fileTypeService - .Setup(s => s.IsValidZipFile(archive)) - .ReturnsAsync(true); - var contentDbContextId = Guid.NewGuid().ToString(); await using (var contentDbContext = DbUtils.InMemoryApplicationDbContext(contentDbContextId)) { var service = SetupDataArchiveValidationService( - contentDbContext: contentDbContext, - fileTypeService: fileTypeService.Object); + contentDbContext: contentDbContext); - var result = await service.ValidateBulkDataArchiveFile( + var result = await service.ValidateBulkDataArchiveFiles( releaseVersionId, archive); @@ -408,30 +297,21 @@ public async Task ValidateBulkDataArchiveFile_Fail_FilesNotFoundInZip() ValidationMessages.GenerateErrorFileNotFoundInZip("two.csv", FileType.Data), ]); } - - VerifyAllMocks(fileTypeService); } [Fact] - public async Task ValidateBulkDataArchiveFile_Fail_DuplicateDataSetTitlesAndFilenames() + public async Task ValidateBulkDataArchiveFiles_DuplicateDataSetTitlesAndFileNames_ReturnsValidationError() { var releaseVersionId = Guid.NewGuid(); var archive = CreateFormFileFromResource("bulk-data-zip-invalid-duplicate-names.zip"); - var fileTypeService = new Mock(Strict); - - fileTypeService - .Setup(s => s.IsValidZipFile(archive)) - .ReturnsAsync(true); - var contentDbContextId = Guid.NewGuid().ToString(); await using (var contentDbContext = DbUtils.InMemoryApplicationDbContext(contentDbContextId)) { var service = SetupDataArchiveValidationService( - contentDbContext: contentDbContext, - fileTypeService: fileTypeService.Object); + contentDbContext: contentDbContext); - var result = await service.ValidateBulkDataArchiveFile( + var result = await service.ValidateBulkDataArchiveFiles( releaseVersionId, archive); @@ -442,29 +322,20 @@ public async Task ValidateBulkDataArchiveFile_Fail_DuplicateDataSetTitlesAndFile ValidationMessages.GenerateErrorDatasetNamesCsvFilenamesShouldBeUnique("one"), ]); } - - VerifyAllMocks(fileTypeService); } [Fact] - public async Task ValidateBulkDataArchiveFile_Fail_DataSetNamesCsvFilesnamesShouldNotEndDotCsv() + public async Task ValidateBulkDataArchiveFiles_Fail_DataSetNamesCsvFilesnamesShouldNotEndDotCsv() { var releaseVersionId = Guid.NewGuid(); - var fileTypeService = new Mock(Strict); - var archive = CreateFormFileFromResource("bulk-data-zip-invalid-filename-contains-extension.zip"); - fileTypeService - .Setup(s => s.IsValidZipFile(archive)) - .ReturnsAsync(true); - var contentDbContextId = Guid.NewGuid().ToString(); await using (var contentDbContext = DbUtils.InMemoryApplicationDbContext(contentDbContextId)) { var service = SetupDataArchiveValidationService( - contentDbContext: contentDbContext, - fileTypeService: fileTypeService.Object); - var result = await service.ValidateBulkDataArchiveFile( + contentDbContext: contentDbContext); + var result = await service.ValidateBulkDataArchiveFiles( releaseVersionId, archive); @@ -474,18 +345,15 @@ public async Task ValidateBulkDataArchiveFile_Fail_DataSetNamesCsvFilesnamesShou ValidationMessages.GenerateErrorDatasetNamesCsvFilenamesShouldNotEndDotCsv("one.csv") ]); } - - VerifyAllMocks(fileTypeService); } [Fact] - public async Task ValidateBulkDataArchiveFile_Fail_UnusedFilesInZip() + public async Task ValidateBulkDataArchiveFiles_IndexFileMissingReferenceToArchiveFile_ReturnsValidationError() { var releaseVersionId = Guid.NewGuid(); var archive = CreateFormFileFromResource("bulk-data-zip-invalid-unused-files.zip"); var fileUploadsValidatorService = new Mock(Strict); - var fileTypeService = new Mock(Strict); fileUploadsValidatorService.Setup(mock => mock.ValidateDataSetFilesForUpload( releaseVersionId, @@ -494,19 +362,14 @@ public async Task ValidateBulkDataArchiveFile_Fail_UnusedFilesInZip() It.IsAny())) .ReturnsAsync([]); - fileTypeService - .Setup(s => s.IsValidZipFile(archive)) - .ReturnsAsync(true); - var contentDbContextId = Guid.NewGuid().ToString(); await using (var contentDbContext = DbUtils.InMemoryApplicationDbContext(contentDbContextId)) { var service = SetupDataArchiveValidationService( contentDbContext: contentDbContext, - fileTypeService: fileTypeService.Object, fileUploadsValidatorService: fileUploadsValidatorService.Object); - var result = await service.ValidateBulkDataArchiveFile( + var result = await service.ValidateBulkDataArchiveFiles( releaseVersionId, archive); @@ -517,29 +380,60 @@ public async Task ValidateBulkDataArchiveFile_Fail_UnusedFilesInZip() ]); } - VerifyAllMocks(fileUploadsValidatorService, fileTypeService); + VerifyAllMocks(fileUploadsValidatorService); } - private static IFormFile CreateFormFileFromResource(string fileName, string? newFileName = null) + [Fact] + public async Task IsValidZipFile_NoFileProvided_ReturnsValidationError() { - var filePath = Path.Combine(Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location)!, - "Resources" + Path.DirectorySeparatorChar + fileName); + // Arrange + var contentDbContextId = Guid.NewGuid().ToString(); + await using var contentDbContext = DbUtils.InMemoryApplicationDbContext(contentDbContextId); + + var service = SetupDataArchiveValidationService(contentDbContext: contentDbContext); + + IFormFile? archive = null; + + // Act + var result = await service.IsValidZipFile(archive!); - return CreateFormFileFromResourceWithPath(filePath, newFileName ?? fileName); + // Assert + AssertHasErrors(result, [ + ValidationMessages.GenerateErrorFileIsNull(), + ]); } - private static IFormFile CreateFormFileFromResourceWithPath(string filePath, string fileName) + [Fact] + public async Task IsValidZipFile_InvalidFileNameAndType_ReturnsValidationErrors() { - var formFile = new Mock(); - formFile - .Setup(f => f.OpenReadStream()) - .Returns(() => File.OpenRead(filePath)); + // Arrange + var contentDbContextId = Guid.NewGuid().ToString(); + await using var contentDbContext = DbUtils.InMemoryApplicationDbContext(contentDbContextId); + + + var longFilename = "loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooongfilename.csv"; + var archive = CreateFormFileFromResource("test-data.csv", longFilename); - formFile - .Setup(f => f.FileName) - .Returns(() => fileName); + var fileTypeService = new Mock(Strict); + fileTypeService + .Setup(s => s.IsValidZipFile(archive)) + .ReturnsAsync(false); - return formFile.Object; + var service = SetupDataArchiveValidationService( + contentDbContext: contentDbContext, + fileTypeService: fileTypeService.Object); + + // Act + var result = await service.IsValidZipFile(archive); + + // Assert + AssertHasErrors(result, [ + ValidationMessages.GenerateErrorFilenameTooLong(longFilename, DataArchiveValidationService.MaxFilenameSize), + ValidationMessages.GenerateErrorZipFilenameMustEndDotZip(longFilename), + ValidationMessages.GenerateErrorMustBeZipFile(longFilename), + ]); + + VerifyAllMocks(fileTypeService); } private static DataArchiveValidationService SetupDataArchiveValidationService( diff --git a/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/Methodologies/MethodologyImageServiceTests.cs b/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/Methodologies/MethodologyImageServiceTests.cs index f08631159d0..be12dcb553e 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/Methodologies/MethodologyImageServiceTests.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/Methodologies/MethodologyImageServiceTests.cs @@ -1,6 +1,4 @@ #nullable enable -using System; -using System.Threading.Tasks; using GovUk.Education.ExploreEducationStatistics.Admin.Services.Interfaces; using GovUk.Education.ExploreEducationStatistics.Admin.Services.Interfaces.Methodologies; using GovUk.Education.ExploreEducationStatistics.Admin.Services.Methodologies; @@ -18,6 +16,8 @@ using GovUk.Education.ExploreEducationStatistics.Content.Model.Repository.Interfaces; using Microsoft.EntityFrameworkCore; using Moq; +using System; +using System.Threading.Tasks; using static GovUk.Education.ExploreEducationStatistics.Admin.Tests.Services.DbUtils; using static GovUk.Education.ExploreEducationStatistics.Admin.Validators.ValidationErrorMessages; using static GovUk.Education.ExploreEducationStatistics.Common.BlobContainers; @@ -714,7 +714,7 @@ public async Task Upload() privateBlobStorageService.Setup(mock => mock.UploadFile(PrivateMethodologyFiles, It.Is(path => - path.Contains(FilesPath(methodologyVersion.Id, Image))), + path.Contains(FilesPath(methodologyVersion.Id, Image, null))), formFile )).Returns(Task.CompletedTask); @@ -740,7 +740,7 @@ public async Task Upload() privateBlobStorageService.Verify(mock => mock.UploadFile(PrivateMethodologyFiles, It.Is(path => - path.Contains(FilesPath(methodologyVersion.Id, Image))), + path.Contains(FilesPath(methodologyVersion.Id, Image, null))), formFile ), Times.Once); diff --git a/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/ReleaseDataFileServicePermissionTests.cs b/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/ReleaseDataFileServicePermissionTests.cs index 52d47d98bd4..54c6dddf040 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/ReleaseDataFileServicePermissionTests.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/ReleaseDataFileServicePermissionTests.cs @@ -1,10 +1,8 @@ #nullable enable -using System; -using System.Collections.Generic; -using System.Threading.Tasks; using GovUk.Education.ExploreEducationStatistics.Admin.Security; using GovUk.Education.ExploreEducationStatistics.Admin.Services; using GovUk.Education.ExploreEducationStatistics.Admin.Services.Interfaces; +using GovUk.Education.ExploreEducationStatistics.Admin.ViewModels; using GovUk.Education.ExploreEducationStatistics.Common.Services.Interfaces; using GovUk.Education.ExploreEducationStatistics.Common.Services.Interfaces.Security; using GovUk.Education.ExploreEducationStatistics.Common.Tests.Utils; @@ -17,6 +15,9 @@ using GovUk.Education.ExploreEducationStatistics.Data.Model.Database; using Microsoft.AspNetCore.Http; using Moq; +using System; +using System.Collections.Generic; +using System.Threading.Tasks; using static GovUk.Education.ExploreEducationStatistics.Admin.Security.SecurityPolicies; using static GovUk.Education.ExploreEducationStatistics.Common.Model.FileType; using static GovUk.Education.ExploreEducationStatistics.Common.Tests.Utils.PermissionTestUtils; @@ -190,7 +191,24 @@ await PolicyCheckBuilder() } [Fact] - public async Task UploadAsBulkZip() + public async Task ValidateAndUploadBulkZip() + { + await PolicyCheckBuilder() + .SetupResourceCheckToFail(_releaseVersion, CanUpdateSpecificRelease) + .AssertForbidden( + userService => + { + var service = SetupReleaseDataFileService(userService: userService.Object); + return service.ValidateAndUploadBulkZip( + releaseVersionId: _releaseVersion.Id, + zipFile: new Mock().Object, + default); + } + ); + } + + [Fact] + public async Task SaveDataSetsFromTemporaryBlobStorage() { await PolicyCheckBuilder() .SetupResourceCheckToFail(_releaseVersion, CanUpdateSpecificRelease) @@ -198,9 +216,10 @@ await PolicyCheckBuilder() userService => { var service = SetupReleaseDataFileService(userService: userService.Object); - return service.UploadAsBulkZip( + return service.SaveDataSetsFromTemporaryBlobStorage( releaseVersionId: _releaseVersion.Id, - bulkZipFormFile: new Mock().Object); + archiveDataSetFiles: new Mock>().Object, + default); } ); } diff --git a/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/ReleaseDataFileServiceTests.cs b/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/ReleaseDataFileServiceTests.cs index 9769665c676..be5c4778261 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/ReleaseDataFileServiceTests.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/ReleaseDataFileServiceTests.cs @@ -2,6 +2,7 @@ using GovUk.Education.ExploreEducationStatistics.Admin.Models; using GovUk.Education.ExploreEducationStatistics.Admin.Services; using GovUk.Education.ExploreEducationStatistics.Admin.Services.Interfaces; +using GovUk.Education.ExploreEducationStatistics.Admin.Validators; using GovUk.Education.ExploreEducationStatistics.Common.Model; using GovUk.Education.ExploreEducationStatistics.Common.Services.Interfaces; using GovUk.Education.ExploreEducationStatistics.Common.Services.Interfaces.Security; @@ -9,6 +10,7 @@ using GovUk.Education.ExploreEducationStatistics.Common.Tests.Fixtures; using GovUk.Education.ExploreEducationStatistics.Common.Tests.Utils; using GovUk.Education.ExploreEducationStatistics.Common.Utils; +using GovUk.Education.ExploreEducationStatistics.Common.ViewModels; using GovUk.Education.ExploreEducationStatistics.Content.Model; using GovUk.Education.ExploreEducationStatistics.Content.Model.Database; using GovUk.Education.ExploreEducationStatistics.Content.Model.Extensions; @@ -23,6 +25,7 @@ using Moq; using System; using System.Collections.Generic; +using System.IO; using System.Linq; using System.Threading.Tasks; using static GovUk.Education.ExploreEducationStatistics.Admin.Tests.Services.DbUtils; @@ -1839,14 +1842,14 @@ public async Task Upload() privateBlobStorageService.Setup(mock => mock.UploadFile(PrivateReleaseFiles, It.Is(path => - path.Contains(FilesPath(releaseVersion.Id, FileType.Data))), + path.Contains(FilesPath(releaseVersion.Id, FileType.Data, null))), dataFormFile )).Returns(Task.CompletedTask); privateBlobStorageService.Setup(mock => mock.UploadFile(PrivateReleaseFiles, It.Is(path => - path.Contains(FilesPath(releaseVersion.Id, FileType.Data))), + path.Contains(FilesPath(releaseVersion.Id, FileType.Data, null))), metaFormFile )).Returns(Task.CompletedTask); @@ -2011,14 +2014,14 @@ public async Task Upload_Replacing() privateBlobStorageService.Setup(mock => mock.UploadFile(PrivateReleaseFiles, It.Is(path => - path.Contains(FilesPath(releaseVersion.Id, FileType.Data))), + path.Contains(FilesPath(releaseVersion.Id, FileType.Data, null))), dataFormFile )).Returns(Task.CompletedTask); privateBlobStorageService.Setup(mock => mock.UploadFile(PrivateReleaseFiles, It.Is(path => - path.Contains(FilesPath(releaseVersion.Id, FileType.Data))), + path.Contains(FilesPath(releaseVersion.Id, FileType.Data, null))), metaFormFile )).Returns(Task.CompletedTask); @@ -2221,14 +2224,14 @@ public async Task Upload_Order() privateBlobStorageService.Setup(mock => mock.UploadFile(PrivateReleaseFiles, It.Is(path => - path.Contains(FilesPath(releaseVersion.Id, FileType.Data))), + path.Contains(FilesPath(releaseVersion.Id, FileType.Data, null))), dataFormFile )).Returns(Task.CompletedTask); privateBlobStorageService.Setup(mock => mock.UploadFile(PrivateReleaseFiles, It.Is(path => - path.Contains(FilesPath(releaseVersion.Id, FileType.Data))), + path.Contains(FilesPath(releaseVersion.Id, FileType.Data, null))), metaFormFile )).Returns(Task.CompletedTask); @@ -2355,7 +2358,7 @@ public async Task UploadAsZip() privateBlobStorageService.Setup(mock => mock.UploadFile(PrivateReleaseFiles, It.Is(path => - path.Contains(FilesPath(releaseVersion.Id, DataZip))), + path.Contains(FilesPath(releaseVersion.Id, DataZip, null))), zipFormFile )).Returns(Task.CompletedTask); @@ -2520,7 +2523,7 @@ public async Task UploadAsZip_Order() privateBlobStorageService.Setup(mock => mock.UploadFile(PrivateReleaseFiles, It.Is(path => - path.Contains(FilesPath(releaseVersion.Id, DataZip))), + path.Contains(FilesPath(releaseVersion.Id, DataZip, null))), zipFormFile )).Returns(Task.CompletedTask); @@ -2673,7 +2676,7 @@ public async Task UploadAsZip_Replacing() privateBlobStorageService.Setup(mock => mock.UploadFile(PrivateReleaseFiles, It.Is(path => - path.Contains(FilesPath(releaseVersion.Id, DataZip))), + path.Contains(FilesPath(releaseVersion.Id, DataZip, null))), zipFormFile )).Returns(Task.CompletedTask); @@ -2780,10 +2783,9 @@ public async Task UploadAsZip_Replacing() } [Fact] - public async Task UploadAsBulkZip() + public async Task ValidateAndUploadBulkZip_InvalidZipFile_ReturnsValidationResult() { - const string zipFileName = "test-data-archive.zip"; - + // Arrange var releaseVersion = new ReleaseVersion { Id = Guid.NewGuid(), @@ -2800,7 +2802,6 @@ public async Task UploadAsBulkZip() }; var contentDbContextId = Guid.NewGuid().ToString(); - var statisticsDbContextId = Guid.NewGuid().ToString(); await using (var contentDbContext = InMemoryApplicationDbContext(contentDbContextId)) { @@ -2809,144 +2810,35 @@ public async Task UploadAsBulkZip() await contentDbContext.SaveChangesAsync(); } - var zipFormFile = CreateFormFileMock(zipFileName, "application/zip").Object; - - var archiveFile1 = new ArchiveDataSetFile("One", "one.csv", "one.meta.csv"); - var archiveFile2 = new ArchiveDataSetFile("Two", "two.csv", "two.meta.csv"); + var archive = CreateFormFileFromResource("bulk-data-zip-invalid-unused-files.zip"); - var privateBlobStorageService = new Mock(Strict); var dataArchiveValidationService = new Mock(Strict); - var fileUploadsValidatorService = new Mock(Strict); - var dataImportService = new Mock(Strict); + + dataArchiveValidationService + .Setup(s => s.IsValidZipFile(archive)) + .ReturnsAsync([new ErrorViewModel { + Code = ValidationMessages.ZipContainsUnusedFiles.Code, + }]); await using (var contentDbContext = InMemoryApplicationDbContext(contentDbContextId)) - await using (var statisticsDbContext = InMemoryStatisticsDbContext(statisticsDbContextId)) { - dataArchiveValidationService - .Setup(s => s.ValidateBulkDataArchiveFile( - releaseVersion.Id, - zipFormFile)) - .ReturnsAsync(new List - { - archiveFile1, archiveFile2, - }); - - dataImportService - .Setup(s => s.Import( - It.IsAny(), - It.Is(file => file.Type == FileType.Data && file.Filename == "one.csv"), - It.Is(file => file.Type == Metadata && file.Filename == "one.meta.csv"), - It.Is(file => file.Type == BulkDataZip && file.Filename == zipFileName))) - .ReturnsAsync(new DataImport - { - Status = QUEUED - }); - dataImportService - .Setup(s => s.Import( - It.IsAny(), - It.Is(file => file.Type == FileType.Data && file.Filename == "two.csv"), - It.Is(file => file.Type == Metadata && file.Filename == "two.meta.csv"), - It.Is(file => file.Type == BulkDataZip && file.Filename == zipFileName))) - .ReturnsAsync(new DataImport - { - Status = QUEUED - }); - - privateBlobStorageService.Setup(mock => - mock.UploadFile(PrivateReleaseFiles, - It.Is(path => - path.Contains(FilesPath(releaseVersion.Id, BulkDataZip))), - zipFormFile - )).Returns(Task.CompletedTask); - var service = SetupReleaseDataFileService( contentDbContext: contentDbContext, - statisticsDbContext: statisticsDbContext, - privateBlobStorageService: privateBlobStorageService.Object, - dataImportService: dataImportService.Object, - dataArchiveValidationService: dataArchiveValidationService.Object, - fileUploadsValidatorService: fileUploadsValidatorService.Object + dataArchiveValidationService: dataArchiveValidationService.Object ); - var result = (await service.UploadAsBulkZip( - releaseVersionId: releaseVersion.Id, - bulkZipFormFile: zipFormFile)) - .AssertRight(); - - MockUtils.VerifyAllMocks(privateBlobStorageService, - dataArchiveValidationService, - fileUploadsValidatorService, - dataImportService); - - Assert.Equal(2, result.Count); - - Assert.Equal("One", result[0].Name); - Assert.Equal("one.csv", result[0].FileName); - Assert.Equal("one.meta.csv", result[0].MetaFileName); - - Assert.Equal("Two", result[1].Name); - Assert.Equal("two.csv", result[1].FileName); - Assert.Equal("two.meta.csv", result[1].MetaFileName); - } - - await using (var contentDbContext = InMemoryApplicationDbContext(contentDbContextId)) - { - var files = contentDbContext.Files.ToList(); - - Assert.Equal(5, files.Count); - - var zipFile = files - .Single(f => f.Filename == zipFileName); - Assert.Equal(10240, zipFile.ContentLength); - Assert.Equal("application/zip", zipFile.ContentType); - Assert.Equal(BulkDataZip, zipFile.Type); - - var dataFile1 = files - .Single(f => f.Filename == archiveFile1.DataFilename); - Assert.Equal(1048576, dataFile1.ContentLength); - Assert.Equal("text/csv", dataFile1.ContentType); - Assert.Equal(FileType.Data, dataFile1.Type); - Assert.Equal(zipFile.Id, dataFile1.SourceId); - - var metaFile1 = files - .Single(f => f.Filename == archiveFile1.MetaFilename); - Assert.Equal(1024, metaFile1.ContentLength); - Assert.Equal("text/csv", metaFile1.ContentType); - Assert.Equal(Metadata, metaFile1.Type); - - var dataFile2 = files - .Single(f => f.Filename == archiveFile2.DataFilename); - Assert.Equal(1048576, dataFile2.ContentLength); - Assert.Equal("text/csv", dataFile2.ContentType); - Assert.Equal(FileType.Data, dataFile2.Type); - Assert.Equal(zipFile.Id, dataFile2.SourceId); - - var metaFile2 = files - .Single(f => f.Filename == archiveFile2.MetaFilename); - Assert.Equal(1024, metaFile2.ContentLength); - Assert.Equal("text/csv", metaFile2.ContentType); - Assert.Equal(Metadata, metaFile2.Type); + // Act & Assert + (await service + .ValidateAndUploadBulkZip(releaseVersion.Id, archive, default)) + .AssertBadRequestWithValidationProblem(); - var releaseFiles = contentDbContext.ReleaseFiles.ToList(); - - Assert.Equal(4, releaseFiles.Count); - - Assert.NotNull(releaseFiles.SingleOrDefault(rf => - rf.ReleaseVersionId == releaseVersion.Id && rf.FileId == dataFile1.Id)); - Assert.NotNull(releaseFiles.SingleOrDefault(rf => - rf.ReleaseVersionId == releaseVersion.Id && rf.FileId == metaFile1.Id)); - Assert.NotNull(releaseFiles.SingleOrDefault(rf => - rf.ReleaseVersionId == releaseVersion.Id && rf.FileId == dataFile2.Id)); - Assert.NotNull(releaseFiles.SingleOrDefault(rf => - rf.ReleaseVersionId == releaseVersion.Id && rf.FileId == metaFile2.Id)); + MockUtils.VerifyAllMocks(dataArchiveValidationService); } } [Fact] - public async Task UploadAsBulkZip_Order() + public async Task ValidateAndUploadBulkZip() { - const string zipFileName = "test-data-archive.zip"; - var releaseVersion = new ReleaseVersion { Id = Guid.NewGuid(), @@ -2962,150 +2854,79 @@ public async Task UploadAsBulkZip_Order() } }; - var releaseFiles = new List - { - new() - { - Name = "Original one", - ReleaseVersion = releaseVersion, - File = new File { Type = FileType.Data }, - Order = 0, - }, - new() - { - Name = "Original two", - ReleaseVersion = releaseVersion, - File = new File { Type = FileType.Data }, - Order = 1, - }, - new() - { - Name = "Original three", - ReleaseVersion = releaseVersion, - File = new File { Type = FileType.Data }, - Order = 3, - }, - new() // Ancillary files should be ignored - { - ReleaseVersion = releaseVersion, - File = new File { Type = FileType.Ancillary }, - Order = 5, - }, - }; var contentDbContextId = Guid.NewGuid().ToString(); - var statisticsDbContextId = Guid.NewGuid().ToString(); await using (var contentDbContext = InMemoryApplicationDbContext(contentDbContextId)) { contentDbContext.ReleaseVersions.Add(releaseVersion); - contentDbContext.ReleaseFiles.AddRange(releaseFiles); await contentDbContext.SaveChangesAsync(); } - var zipFormFile = CreateFormFileMock(zipFileName, "application/zip").Object; + var archive = CreateFormFileFromResource("bulk-data-zip-valid.zip"); var archiveFile1 = new ArchiveDataSetFile("One", "one.csv", "one.meta.csv"); var archiveFile2 = new ArchiveDataSetFile("Two", "two.csv", "two.meta.csv"); - var privateBlobStorageService = new Mock(Strict); var dataArchiveValidationService = new Mock(Strict); var fileUploadsValidatorService = new Mock(Strict); - var dataImportService = new Mock(Strict); + var privateBlobStorageService = new Mock(Strict); + + dataArchiveValidationService + .Setup(s => s.IsValidZipFile(archive)) + .ReturnsAsync([]); await using (var contentDbContext = InMemoryApplicationDbContext(contentDbContextId)) - await using (var statisticsDbContext = InMemoryStatisticsDbContext(statisticsDbContextId)) { dataArchiveValidationService - .Setup(s => s.ValidateBulkDataArchiveFile( + .Setup(s => s.ValidateBulkDataArchiveFiles( releaseVersion.Id, - zipFormFile)) + archive)) .ReturnsAsync(new List { archiveFile1, archiveFile2, }); - dataImportService - .Setup(s => s.Import( - It.IsAny(), - It.Is(file => file.Type == FileType.Data && file.Filename == "one.csv"), - It.Is(file => file.Type == Metadata && file.Filename == "one.meta.csv"), - It.Is(file => file.Type == BulkDataZip && file.Filename == zipFileName))) - .ReturnsAsync(new DataImport - { - Status = QUEUED - }); - dataImportService - .Setup(s => s.Import( - It.IsAny(), - It.Is(file => file.Type == FileType.Data && file.Filename == "two.csv"), - It.Is(file => file.Type == Metadata && file.Filename == "two.meta.csv"), - It.Is(file => file.Type == BulkDataZip && file.Filename == zipFileName))) - .ReturnsAsync(new DataImport - { - Status = QUEUED - }); - privateBlobStorageService.Setup(mock => - mock.UploadFile(PrivateReleaseFiles, - It.Is(path => - path.Contains(FilesPath(releaseVersion.Id, BulkDataZip))), - zipFormFile + mock.UploadStream( + PrivateReleaseTempFiles, + It.IsAny(), + It.IsAny(), + ContentTypes.Csv, + null, + default )).Returns(Task.CompletedTask); var service = SetupReleaseDataFileService( contentDbContext: contentDbContext, - statisticsDbContext: statisticsDbContext, - privateBlobStorageService: privateBlobStorageService.Object, - dataImportService: dataImportService.Object, dataArchiveValidationService: dataArchiveValidationService.Object, - fileUploadsValidatorService: fileUploadsValidatorService.Object + fileUploadsValidatorService: fileUploadsValidatorService.Object, + privateBlobStorageService: privateBlobStorageService.Object ); - var result = (await service.UploadAsBulkZip( - releaseVersionId: releaseVersion.Id, - bulkZipFormFile: zipFormFile)) + var result = (await service + .ValidateAndUploadBulkZip(releaseVersion.Id, archive, default)) .AssertRight(); - MockUtils.VerifyAllMocks(privateBlobStorageService, + MockUtils.VerifyAllMocks( dataArchiveValidationService, fileUploadsValidatorService, - dataImportService); + privateBlobStorageService); Assert.Equal(2, result.Count); - } - - await using (var contentDbContext = InMemoryApplicationDbContext(contentDbContextId)) - { - var files = contentDbContext.Files.ToList(); - Assert.Equal(9, files.Count); - - var dbReleaseFiles = contentDbContext.ReleaseFiles - .Where(rf => rf.File.Type == FileType.Data) - .OrderBy(rf => rf.Order) - .ToList(); - - Assert.Equal(5, dbReleaseFiles.Count); - - Assert.Equal(0, dbReleaseFiles[0].Order); - Assert.Equal("Original one", dbReleaseFiles[0].Name); - - Assert.Equal(1, dbReleaseFiles[1].Order); - Assert.Equal("Original two", dbReleaseFiles[1].Name); - // NOTE: The Orders of the original ReleaseFiles were 0,1,3 - no releaseFile had an order of 2 + Assert.Equal("One", result[0].Title); + Assert.Equal("one.csv", result[0].DataFilename); + Assert.Equal("one.meta.csv", result[0].MetaFilename); - Assert.Equal(3, dbReleaseFiles[2].Order); - Assert.Equal("Original three", dbReleaseFiles[2].Name); - - Assert.Equal(4, dbReleaseFiles[3].Order); - Assert.Equal("One", dbReleaseFiles[3].Name); - - Assert.Equal(5, dbReleaseFiles[4].Order); - Assert.Equal("Two", dbReleaseFiles[4].Name); + Assert.Equal("Two", result[1].Title); + Assert.Equal("two.csv", result[1].DataFilename); + Assert.Equal("two.meta.csv", result[1].MetaFilename); } } + + private ReleaseDataFileService SetupReleaseDataFileService( ContentDbContext contentDbContext, StatisticsDbContext? statisticsDbContext = null, diff --git a/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/ReleaseFileServiceTests.cs b/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/ReleaseFileServiceTests.cs index e70650a2a9f..56eaef34916 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/ReleaseFileServiceTests.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/ReleaseFileServiceTests.cs @@ -2491,7 +2491,7 @@ public async Task UploadAncillary() privateBlobStorageService.Setup(mock => mock.UploadFile(PrivateReleaseFiles, It.Is(path => - path.Contains(FilesPath(releaseVersion.Id, Ancillary))), + path.Contains(FilesPath(releaseVersion.Id, Ancillary, null))), formFile )).Returns(Task.CompletedTask); @@ -2525,7 +2525,7 @@ public async Task UploadAncillary() privateBlobStorageService.Verify(mock => mock.UploadFile(PrivateReleaseFiles, It.Is(path => - path.Contains(FilesPath(releaseVersion.Id, Ancillary))), + path.Contains(FilesPath(releaseVersion.Id, Ancillary, null))), formFile ), Times.Once); @@ -2605,7 +2605,7 @@ public async Task UpdateAncillary() privateBlobStorageService.Setup(mock => mock.UploadFile(PrivateReleaseFiles, It.Is(path => - path.Contains(FilesPath(releaseVersion.Id, Ancillary))), + path.Contains(FilesPath(releaseVersion.Id, Ancillary, null))), newFormFile )).Returns(Task.CompletedTask); @@ -2639,7 +2639,7 @@ public async Task UpdateAncillary() privateBlobStorageService.Verify(mock => mock.UploadFile(PrivateReleaseFiles, It.Is(path => - path.Contains(FilesPath(releaseVersion.Id, Ancillary))), + path.Contains(FilesPath(releaseVersion.Id, Ancillary, null))), newFormFile ), Times.Once); @@ -2729,7 +2729,7 @@ public async Task UpdateAncillary_DoNotRemoveFileAttachedToOtherRelease() privateBlobStorageService.Setup(mock => mock.UploadFile(PrivateReleaseFiles, It.Is(path => - path.Contains(FilesPath(releaseVersion.Id, Ancillary))), + path.Contains(FilesPath(releaseVersion.Id, Ancillary, null))), newFormFile )).Returns(Task.CompletedTask); @@ -2763,7 +2763,7 @@ public async Task UpdateAncillary_DoNotRemoveFileAttachedToOtherRelease() privateBlobStorageService.Verify(mock => mock.UploadFile(PrivateReleaseFiles, It.Is(path => - path.Contains(FilesPath(releaseVersion.Id, Ancillary))), + path.Contains(FilesPath(releaseVersion.Id, Ancillary, null))), newFormFile ), Times.Once); @@ -2886,7 +2886,7 @@ public async Task UploadChart() privateBlobStorageService.Setup(mock => mock.UploadFile(PrivateReleaseFiles, It.Is(path => - path.Contains(FilesPath(releaseVersion.Id, Chart))), + path.Contains(FilesPath(releaseVersion.Id, Chart, null))), formFile )).Returns(Task.CompletedTask); @@ -2912,7 +2912,7 @@ public async Task UploadChart() privateBlobStorageService.Verify(mock => mock.UploadFile(PrivateReleaseFiles, It.Is(path => - path.Contains(FilesPath(releaseVersion.Id, Chart))), + path.Contains(FilesPath(releaseVersion.Id, Chart, null))), formFile ), Times.Once); diff --git a/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/ReleaseImageServiceTests.cs b/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/ReleaseImageServiceTests.cs index cea675a8087..5ee616de33a 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/ReleaseImageServiceTests.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/ReleaseImageServiceTests.cs @@ -1,6 +1,4 @@ #nullable enable -using System; -using System.Threading.Tasks; using GovUk.Education.ExploreEducationStatistics.Admin.Services; using GovUk.Education.ExploreEducationStatistics.Admin.Services.Interfaces; using GovUk.Education.ExploreEducationStatistics.Common.Extensions; @@ -17,6 +15,8 @@ using GovUk.Education.ExploreEducationStatistics.Content.Model.Repository.Interfaces; using Microsoft.EntityFrameworkCore; using Moq; +using System; +using System.Threading.Tasks; using static GovUk.Education.ExploreEducationStatistics.Admin.Tests.Services.DbUtils; using static GovUk.Education.ExploreEducationStatistics.Common.BlobContainers; using static GovUk.Education.ExploreEducationStatistics.Common.Model.FileType; @@ -208,7 +208,7 @@ public async Task Upload() privateBlobStorageService.Setup(mock => mock.UploadFile(PrivateReleaseFiles, It.Is(path => - path.Contains(FilesPath(releaseVersion.Id, Image))), + path.Contains(FilesPath(releaseVersion.Id, Image, null))), formFile )).Returns(Task.CompletedTask); @@ -234,7 +234,7 @@ public async Task Upload() privateBlobStorageService.Verify(mock => mock.UploadFile(PrivateReleaseFiles, It.Is(path => - path.Contains(FilesPath(releaseVersion.Id, Image))), + path.Contains(FilesPath(releaseVersion.Id, Image, null))), formFile ), Times.Once); diff --git a/src/GovUk.Education.ExploreEducationStatistics.Common.Tests/Services/BlobStorageServiceTests.cs b/src/GovUk.Education.ExploreEducationStatistics.Common.Tests/Services/BlobStorageServiceTests.cs index 3099fde10b1..8a3d7196f82 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Common.Tests/Services/BlobStorageServiceTests.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Common.Tests/Services/BlobStorageServiceTests.cs @@ -1,12 +1,4 @@ -#nullable enable -using System; -using System.Collections.Generic; -using System.Collections.Immutable; -using System.IO; -using System.Linq; -using System.Net.Mime; -using System.Text.RegularExpressions; -using System.Threading.Tasks; +#nullable enable using Azure; using Azure.Storage.Blobs; using Azure.Storage.Blobs.Models; @@ -19,6 +11,14 @@ using Microsoft.Extensions.Logging; using Moq; using Newtonsoft.Json; +using System; +using System.Collections.Generic; +using System.Collections.Immutable; +using System.IO; +using System.Linq; +using System.Net.Mime; +using System.Text.RegularExpressions; +using System.Threading.Tasks; using Xunit; using static Azure.Storage.Blobs.Models.BlobsModelFactory; using static GovUk.Education.ExploreEducationStatistics.Common.BlobContainers; @@ -494,7 +494,7 @@ await service.DeleteBlobs( } [Fact] - private async Task DeleteBlobs_FilterPrioritisesExcludeRegex() + public async Task DeleteBlobs_FilterPrioritisesExcludeRegex() { var blobContainerClient = MockBlobContainerClient(PublicReleaseFiles.Name); var blobServiceClient = MockBlobServiceClient(blobContainerClient); @@ -634,6 +634,62 @@ await service.DeleteBlobs( Assert.Equal("publications/item-4", deletedBlobs[2]); } + [Fact] + public async Task MoveBlob_SourceBlobNotFound_ReturnsFalse() + { + // Arrange + const string sourcePath = "path/to/test.pdf"; + const string destinationPath = "new/path/to/test.pdf"; + + var sourceBlobClient = MockBlobClient(name: sourcePath, exists: false); + + var sourceBlobContainerClient = MockBlobContainerClient(PrivateReleaseTempFiles.Name, sourceBlobClient); + + sourceBlobContainerClient + .Setup(client => client.GetBlobClient(sourcePath)) + .Returns(sourceBlobClient.Object); + + var blobServiceClient = MockBlobServiceClient(sourceBlobContainerClient); + + var service = SetupTestBlobStorageService(blobServiceClient.Object); + + // Act + var result = await service.MoveBlob(PrivateReleaseTempFiles, sourcePath, destinationPath); + + // Assert + Assert.False(result); + } + + [Fact] + public async Task MoveBlob_DestinationBlobAlreadyExists_ReturnsFalse() + { + // Arrange + const string sourcePath = "path/to/test.pdf"; + const string destinationPath = "new/path/to/test.pdf"; + + var sourceBlobClient = MockBlobClient(name: sourcePath, exists: true); + var destinationBlobClient = MockBlobClient(name: destinationPath, exists: true); + + var blobContainerClient = MockBlobContainerClient(PrivateReleaseTempFiles.Name, sourceBlobClient, destinationBlobClient); + + blobContainerClient + .Setup(client => client.GetBlobClient(sourcePath)) + .Returns(sourceBlobClient.Object); + + blobContainerClient + .Setup(client => client.GetBlobClient(destinationPath)) + .Returns(destinationBlobClient.Object); + + var blobServiceClient = MockBlobServiceClient(blobContainerClient); + + var service = SetupTestBlobStorageService(blobServiceClient.Object); + + // Act + var result = await service.MoveBlob(PrivateReleaseTempFiles, sourcePath, destinationPath); + + // Assert + Assert.False(result); + } private static Mock MockBlobServiceClient( params Mock[] blobContainerClients) @@ -719,7 +775,7 @@ public TestBlobStorageService( ILogger logger, IStorageInstanceCreationUtil storageInstanceCreationUtil) : base(connectionString, client, logger, storageInstanceCreationUtil) - {} + { } } private IEnumerable> CreatePages(params List[] pages) diff --git a/src/GovUk.Education.ExploreEducationStatistics.Common.Tests/Utils/MockFormTestUtils.cs b/src/GovUk.Education.ExploreEducationStatistics.Common.Tests/Utils/MockFormTestUtils.cs index 22a12280c5f..da5af913a61 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Common.Tests/Utils/MockFormTestUtils.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Common.Tests/Utils/MockFormTestUtils.cs @@ -1,6 +1,8 @@ -#nullable enable +#nullable enable using Microsoft.AspNetCore.Http; using Moq; +using System.IO; +using System.Reflection; namespace GovUk.Education.ExploreEducationStatistics.Common.Tests.Utils { @@ -34,5 +36,27 @@ public static Mock CreateFormFileMock(string filename, string content return formFile; } + + public static IFormFile CreateFormFileFromResource(string fileName, string? newFileName = null) + { + var filePath = Path.Combine(Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location)!, + "Resources" + Path.DirectorySeparatorChar + fileName); + + return CreateFormFileFromResourceWithPath(filePath, newFileName ?? fileName); + } + + public static IFormFile CreateFormFileFromResourceWithPath(string filePath, string fileName) + { + var formFile = new Mock(); + formFile + .Setup(f => f.OpenReadStream()) + .Returns(() => System.IO.File.OpenRead(filePath)); + + formFile + .Setup(f => f.FileName) + .Returns(() => fileName); + + return formFile.Object; + } } } diff --git a/src/explore-education-statistics-admin/src/pages/release/data/components/ReleaseDataUploadsSection.tsx b/src/explore-education-statistics-admin/src/pages/release/data/components/ReleaseDataUploadsSection.tsx index 8238ffb61de..85043e0f208 100644 --- a/src/explore-education-statistics-admin/src/pages/release/data/components/ReleaseDataUploadsSection.tsx +++ b/src/explore-education-statistics-admin/src/pages/release/data/components/ReleaseDataUploadsSection.tsx @@ -59,7 +59,7 @@ const ReleaseDataUploadsSection = ({ const [bulkUploadPlan, setBulkUploadPlan] = useState(); const { - data: initialDataFiles = [], + data: initialDataFiles, isLoading, refetch: refetchDataFiles, } = useQuery(releaseDataFileQueries.list(releaseId)); @@ -67,8 +67,8 @@ const ReleaseDataUploadsSection = ({ // Store the data files on state so we can reliably update them // when the permissions/status change. useEffect(() => { - setDataFiles(initialDataFiles); - }, [initialDataFiles, isLoading, setDataFiles]); + setDataFiles(initialDataFiles ?? []); + }, [initialDataFiles]); useEffect(() => { onDataFilesChange?.(dataFiles); diff --git a/src/explore-education-statistics-admin/src/pages/release/data/components/__tests__/DataFileUploadForm.test.tsx b/src/explore-education-statistics-admin/src/pages/release/data/components/__tests__/DataFileUploadForm.test.tsx index f3baa08ecc1..2664e4cfe5c 100644 --- a/src/explore-education-statistics-admin/src/pages/release/data/components/__tests__/DataFileUploadForm.test.tsx +++ b/src/explore-education-statistics-admin/src/pages/release/data/components/__tests__/DataFileUploadForm.test.tsx @@ -149,6 +149,25 @@ describe('DataFileUploadForm', () => { }); }); + test('form submits without error when suitable file is used', async () => { + const onSubmit = jest.fn(); + const { user } = render(); + + const file = new File(['hello, world!'], 'test.zip', { + type: 'application/zip', + }); + + await user.click(screen.getByLabelText('Bulk ZIP upload')); + await user.upload(screen.getByLabelText('Upload bulk ZIP file'), file); + await user.click( + screen.getByRole('button', { + name: 'Upload data files', + }), + ); + + expect(onSubmit).toHaveBeenCalledTimes(1); + }); + test('shows validation message when bulk ZIP file is empty', async () => { const { user } = render(); diff --git a/src/explore-education-statistics-admin/src/pages/release/data/components/__tests__/ReleaseDataUploadsSection.test.tsx b/src/explore-education-statistics-admin/src/pages/release/data/components/__tests__/ReleaseDataUploadsSection.test.tsx index 1ff7d876220..fcf8607f330 100644 --- a/src/explore-education-statistics-admin/src/pages/release/data/components/__tests__/ReleaseDataUploadsSection.test.tsx +++ b/src/explore-education-statistics-admin/src/pages/release/data/components/__tests__/ReleaseDataUploadsSection.test.tsx @@ -4,6 +4,7 @@ import _releaseDataFileService, { DataFile, UploadDataFilesRequest, UploadZipDataFileRequest, + ArchiveDataSetFile, } from '@admin/services/releaseDataFileService'; import { screen, waitFor, within } from '@testing-library/react'; import React from 'react'; @@ -816,7 +817,7 @@ describe('ReleaseDataUploadsSection', () => { }); }); - test('successful submit with CSV files renders with uploaded data file appended to list', async () => { + test('successful submit with CSV files refetches data files', async () => { releaseDataFileService.uploadDataFiles.mockResolvedValue( testUploadedDataFile, ); @@ -863,59 +864,13 @@ describe('ReleaseDataUploadsSection', () => { } as UploadDataFilesRequest, ); - expect(screen.getAllByTestId('accordionSection')).toHaveLength(3); + expect(releaseDataFileService.getDataFiles).toHaveBeenCalledWith( + 'release-1', + ); }); - - const sections = screen.getAllByTestId('accordionSection'); - - const section1 = within(sections[0]); - - expect( - section1.getByRole('button', { name: /Test data 1/ }), - ).toBeInTheDocument(); - - const section2 = within(sections[1]); - - expect( - section2.getByRole('button', { name: /Test data 2/ }), - ).toBeInTheDocument(); - - expect(section2.getByTestId('Subject title')).toHaveTextContent( - 'Test data 2', - ); - - const section3 = within(sections[2]); - - expect( - section3.getByRole('button', { name: /Test title/ }), - ).toBeInTheDocument(); - - expect(section3.getByTestId('Subject title')).toHaveTextContent( - 'Test title', - ); - - expect(section3.getByTestId('Data file')).toHaveTextContent( - 'test-data.csv', - ); - expect(section3.getByTestId('Metadata file')).toHaveTextContent( - 'test-data.meta.csv', - ); - expect(section3.getByTestId('Data file size')).toHaveTextContent( - '150 Kb', - ); - expect(section3.getByTestId('Number of rows')).toHaveTextContent( - 'Unknown', - ); - expect(section3.getByTestId('Status')).toHaveTextContent('Queued'); - expect(section3.getByTestId('Uploaded by')).toHaveTextContent( - 'user1@test.com', - ); - expect(section3.getByTestId('Date uploaded')).toHaveTextContent( - '18 August 2020 12:00', - ); }); - test('successful submit with ZIP file renders with uploaded data file appended to list', async () => { + test('successful submit with zip file refetches data files', async () => { releaseDataFileService.uploadZipDataFile.mockResolvedValue({ ...testUploadedZipFile, }); @@ -956,68 +911,50 @@ describe('ReleaseDataUploadsSection', () => { zipFile, } as UploadZipDataFileRequest, ); - }); - - const sections = screen.getAllByTestId('accordionSection'); - const section1 = within(sections[0]); - const section2 = within(sections[1]); - const section3 = within(sections[2]); - - expect(sections).toHaveLength(3); - expect( - section1.getByRole('button', { name: /Test data 1/ }), - ).toBeInTheDocument(); - - expect( - section2.getByRole('button', { name: /Test data 2/ }), - ).toBeInTheDocument(); - - expect(section2.getByTestId('Subject title')).toHaveTextContent( - 'Test data 2', - ); - - expect( - section3.getByRole('button', { name: /Test zip title/ }), - ).toBeInTheDocument(); - - expect(section3.getByTestId('Subject title')).toHaveTextContent( - 'Test zip title', - ); - - expect(section3.getByTestId('Data file')).toHaveTextContent( - 'test-data.zip', - ); - expect(section3.getByTestId('Metadata file')).toHaveTextContent( - 'test-data.meta.zip', - ); - expect(section3.getByTestId('Data file size')).toHaveTextContent( - '150 Kb', - ); - expect(section3.getByTestId('Number of rows')).toHaveTextContent( - 'Unknown', - ); - expect(section3.getByTestId('Status')).toHaveTextContent('Queued'); - expect(section3.getByTestId('Uploaded by')).toHaveTextContent( - 'user1@test.com', - ); - expect(section3.getByTestId('Date uploaded')).toHaveTextContent( - '18 August 2020 12:00', - ); + expect(releaseDataFileService.getDataFiles).toHaveBeenCalledWith( + 'release-1', + ); + }); }); - test('successful submit with ZIP file renders with uploaded data file appended to list', async () => { - releaseDataFileService.uploadBulkZipDataFile.mockResolvedValue([ - testUploadedZipFile, - { - ...testUploadedZipFile, - id: 'zip-file-2', - title: 'Test zip title 2', - fileName: 'test-data-2.zip', - metaFileId: 'file-2-meta', - metaFileName: 'test-data-2.meta.zip', + test('successful submit with bulk zip file refetches data files', async () => { + const data: ArchiveDataSetFile = { + dataFileId: 'data-file-1', + dataFilename: 'test.csv', + dataFileSize: 1024, + metaFileId: 'meta-file-1', + metaFilename: 'test.meta.csv', + metaFileSize: 128, + title: 'Data set 1', + }; + + releaseDataFileService.getUploadBulkZipDataFilePlan.mockResolvedValue([ + data, + ]); + + const testDataFile: DataFile = { + id: 'file-1', + rows: 100, + fileName: 'data.csv', + fileSize: { + size: 200, + unit: 'B', + }, + userName: 'test@test.com', + title: 'Test data', + metaFileId: 'file-meta-1', + metaFileName: 'meta.csv', + status: 'COMPLETE', + permissions: { + canCancelImport: false, }, + }; + + releaseDataFileService.importBulkZipDataFile.mockResolvedValue([ + testDataFile, ]); + releaseDataFileService.getDataFileImportStatus.mockResolvedValue( testQueuedImportStatus, ); @@ -1044,80 +981,17 @@ describe('ReleaseDataUploadsSection', () => { name: 'Upload data files', }), ); + await user.click( + screen.getByRole('button', { + name: 'Confirm', + }), + ); await waitFor(() => { - expect( - releaseDataFileService.uploadBulkZipDataFile, - ).toHaveBeenCalledWith('release-1', zipFile); + expect(releaseDataFileService.getDataFiles).toHaveBeenCalledWith( + 'release-1', + ); }); - - const sections = screen.getAllByTestId('accordionSection'); - const section1 = within(sections[0]); - const section2 = within(sections[1]); - const section3 = within(sections[2]); - const section4 = within(sections[3]); - - expect(sections).toHaveLength(4); - - expect( - section1.getByRole('button', { name: /Test data 1/ }), - ).toBeInTheDocument(); - - expect( - section2.getByRole('button', { name: /Test data 2/ }), - ).toBeInTheDocument(); - - expect( - section3.getByRole('button', { name: /Test zip title/ }), - ).toBeInTheDocument(); - expect(section3.getByTestId('Subject title')).toHaveTextContent( - 'Test zip title', - ); - expect(section3.getByTestId('Data file')).toHaveTextContent( - 'test-data.zip', - ); - expect(section3.getByTestId('Metadata file')).toHaveTextContent( - 'test-data.meta.zip', - ); - expect(section3.getByTestId('Data file size')).toHaveTextContent( - '150 Kb', - ); - expect(section3.getByTestId('Number of rows')).toHaveTextContent( - 'Unknown', - ); - expect(section3.getByTestId('Status')).toHaveTextContent('Queued'); - expect(section3.getByTestId('Uploaded by')).toHaveTextContent( - 'user1@test.com', - ); - expect(section3.getByTestId('Date uploaded')).toHaveTextContent( - '18 August 2020 12:00', - ); - - expect( - section4.getByRole('button', { name: /Test zip title 2/ }), - ).toBeInTheDocument(); - expect(section4.getByTestId('Subject title')).toHaveTextContent( - 'Test zip title 2', - ); - expect(section4.getByTestId('Data file')).toHaveTextContent( - 'test-data-2.zip', - ); - expect(section4.getByTestId('Metadata file')).toHaveTextContent( - 'test-data-2.meta.zip', - ); - expect(section4.getByTestId('Data file size')).toHaveTextContent( - '150 Kb', - ); - expect(section4.getByTestId('Number of rows')).toHaveTextContent( - 'Unknown', - ); - expect(section4.getByTestId('Status')).toHaveTextContent('Queued'); - expect(section4.getByTestId('Uploaded by')).toHaveTextContent( - 'user1@test.com', - ); - expect(section4.getByTestId('Date uploaded')).toHaveTextContent( - '18 August 2020 12:00', - ); }); test('updates the number of rows after uploading CSV file when status changes', async () => { @@ -1128,6 +1002,16 @@ describe('ReleaseDataUploadsSection', () => { .mockResolvedValue(testQueuedImportStatus) .mockResolvedValueOnce(testImportingImportStatus); + const testUploadedDataFile2 = { + ...testUploadedDataFile, + title: 'Test title 2', + }; + + releaseDataFileService.getDataFiles.mockResolvedValue([ + ...testDataFiles, + testUploadedDataFile2, + ]); + permissionService.getDataFilePermissions.mockResolvedValue( {} as DataFilePermissions, ); @@ -1171,6 +1055,10 @@ describe('ReleaseDataUploadsSection', () => { metadataFile, } as UploadDataFilesRequest, ); + + expect(releaseDataFileService.getDataFiles).toHaveBeenCalledWith( + 'release-1', + ); }); const sections = screen.getAllByTestId('accordionSection'); @@ -1179,7 +1067,7 @@ describe('ReleaseDataUploadsSection', () => { await waitFor(() => expect( releaseDataFileService.getDataFileImportStatus, - ).toHaveBeenCalledWith('release-1', testUploadedDataFile), + ).toHaveBeenCalledWith('release-1', testUploadedDataFile2), ); await waitFor(() => { expect(section3.getByTestId('Number of rows')).toHaveTextContent('100'); @@ -1195,6 +1083,16 @@ describe('ReleaseDataUploadsSection', () => { .mockResolvedValue(testQueuedImportStatus) .mockResolvedValueOnce(testImportingImportStatus); + const testUploadedDataFile2 = { + ...testUploadedDataFile, + title: 'Test title 2', + }; + + releaseDataFileService.getDataFiles.mockResolvedValue([ + ...testDataFiles, + testUploadedDataFile2, + ]); + permissionService.getDataFilePermissions.mockResolvedValue( {} as DataFilePermissions, ); @@ -1240,7 +1138,7 @@ describe('ReleaseDataUploadsSection', () => { await waitFor(() => expect( releaseDataFileService.getDataFileImportStatus, - ).toHaveBeenCalledWith('release-1', testUploadedZipFile), + ).toHaveBeenCalledWith('release-1', testUploadedDataFile2), ); await waitFor(() => { expect(section3.getByTestId('Number of rows')).toHaveTextContent('100'); From dc0e17f124970e3ab86f8ea9a263d47a9149c270 Mon Sep 17 00:00:00 2001 From: Tom Jones Date: Fri, 6 Dec 2024 15:16:07 +0000 Subject: [PATCH 4/4] EES-5583: Use alternative path builder method, and revert optional parameter change. --- .../DataArchiveValidationServiceTests.cs | 4 +--- .../MethodologyImageServiceTests.cs | 4 ++-- .../ReleaseDataFileServicePermissionTests.cs | 4 ++-- .../Services/ReleaseDataFileServiceTests.cs | 18 +++++++++--------- .../Services/ReleaseFileServiceTests.cs | 16 ++++++++-------- .../Services/ReleaseImageServiceTests.cs | 4 ++-- .../Services/ReleaseDataFileService.cs | 12 ++++++------ .../Services/FileStoragePathUtils.cs | 4 ++-- 8 files changed, 32 insertions(+), 34 deletions(-) diff --git a/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/DataArchiveValidationServiceTests.cs b/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/DataArchiveValidationServiceTests.cs index 9faafa5ccdd..27fca5a4bca 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/DataArchiveValidationServiceTests.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/DataArchiveValidationServiceTests.cs @@ -387,8 +387,7 @@ public async Task ValidateBulkDataArchiveFiles_IndexFileMissingReferenceToArchiv public async Task IsValidZipFile_NoFileProvided_ReturnsValidationError() { // Arrange - var contentDbContextId = Guid.NewGuid().ToString(); - await using var contentDbContext = DbUtils.InMemoryApplicationDbContext(contentDbContextId); + await using var contentDbContext = DbUtils.InMemoryApplicationDbContext(); var service = SetupDataArchiveValidationService(contentDbContext: contentDbContext); @@ -410,7 +409,6 @@ public async Task IsValidZipFile_InvalidFileNameAndType_ReturnsValidationErrors( var contentDbContextId = Guid.NewGuid().ToString(); await using var contentDbContext = DbUtils.InMemoryApplicationDbContext(contentDbContextId); - var longFilename = "loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooongfilename.csv"; var archive = CreateFormFileFromResource("test-data.csv", longFilename); diff --git a/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/Methodologies/MethodologyImageServiceTests.cs b/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/Methodologies/MethodologyImageServiceTests.cs index be12dcb553e..f1a4e5ee689 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/Methodologies/MethodologyImageServiceTests.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/Methodologies/MethodologyImageServiceTests.cs @@ -714,7 +714,7 @@ public async Task Upload() privateBlobStorageService.Setup(mock => mock.UploadFile(PrivateMethodologyFiles, It.Is(path => - path.Contains(FilesPath(methodologyVersion.Id, Image, null))), + path.Contains(FilesPath(methodologyVersion.Id, Image))), formFile )).Returns(Task.CompletedTask); @@ -740,7 +740,7 @@ public async Task Upload() privateBlobStorageService.Verify(mock => mock.UploadFile(PrivateMethodologyFiles, It.Is(path => - path.Contains(FilesPath(methodologyVersion.Id, Image, null))), + path.Contains(FilesPath(methodologyVersion.Id, Image))), formFile ), Times.Once); diff --git a/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/ReleaseDataFileServicePermissionTests.cs b/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/ReleaseDataFileServicePermissionTests.cs index 54c6dddf040..d73cbe0b940 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/ReleaseDataFileServicePermissionTests.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/ReleaseDataFileServicePermissionTests.cs @@ -202,7 +202,7 @@ await PolicyCheckBuilder() return service.ValidateAndUploadBulkZip( releaseVersionId: _releaseVersion.Id, zipFile: new Mock().Object, - default); + cancellationToken: default); } ); } @@ -219,7 +219,7 @@ await PolicyCheckBuilder() return service.SaveDataSetsFromTemporaryBlobStorage( releaseVersionId: _releaseVersion.Id, archiveDataSetFiles: new Mock>().Object, - default); + cancellationToken: default); } ); } diff --git a/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/ReleaseDataFileServiceTests.cs b/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/ReleaseDataFileServiceTests.cs index be5c4778261..93e8f001fe5 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/ReleaseDataFileServiceTests.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/ReleaseDataFileServiceTests.cs @@ -1842,14 +1842,14 @@ public async Task Upload() privateBlobStorageService.Setup(mock => mock.UploadFile(PrivateReleaseFiles, It.Is(path => - path.Contains(FilesPath(releaseVersion.Id, FileType.Data, null))), + path.Contains(FilesPath(releaseVersion.Id, FileType.Data))), dataFormFile )).Returns(Task.CompletedTask); privateBlobStorageService.Setup(mock => mock.UploadFile(PrivateReleaseFiles, It.Is(path => - path.Contains(FilesPath(releaseVersion.Id, FileType.Data, null))), + path.Contains(FilesPath(releaseVersion.Id, FileType.Data))), metaFormFile )).Returns(Task.CompletedTask); @@ -2014,14 +2014,14 @@ public async Task Upload_Replacing() privateBlobStorageService.Setup(mock => mock.UploadFile(PrivateReleaseFiles, It.Is(path => - path.Contains(FilesPath(releaseVersion.Id, FileType.Data, null))), + path.Contains(FilesPath(releaseVersion.Id, FileType.Data))), dataFormFile )).Returns(Task.CompletedTask); privateBlobStorageService.Setup(mock => mock.UploadFile(PrivateReleaseFiles, It.Is(path => - path.Contains(FilesPath(releaseVersion.Id, FileType.Data, null))), + path.Contains(FilesPath(releaseVersion.Id, FileType.Data))), metaFormFile )).Returns(Task.CompletedTask); @@ -2224,14 +2224,14 @@ public async Task Upload_Order() privateBlobStorageService.Setup(mock => mock.UploadFile(PrivateReleaseFiles, It.Is(path => - path.Contains(FilesPath(releaseVersion.Id, FileType.Data, null))), + path.Contains(FilesPath(releaseVersion.Id, FileType.Data))), dataFormFile )).Returns(Task.CompletedTask); privateBlobStorageService.Setup(mock => mock.UploadFile(PrivateReleaseFiles, It.Is(path => - path.Contains(FilesPath(releaseVersion.Id, FileType.Data, null))), + path.Contains(FilesPath(releaseVersion.Id, FileType.Data))), metaFormFile )).Returns(Task.CompletedTask); @@ -2358,7 +2358,7 @@ public async Task UploadAsZip() privateBlobStorageService.Setup(mock => mock.UploadFile(PrivateReleaseFiles, It.Is(path => - path.Contains(FilesPath(releaseVersion.Id, DataZip, null))), + path.Contains(FilesPath(releaseVersion.Id, DataZip))), zipFormFile )).Returns(Task.CompletedTask); @@ -2523,7 +2523,7 @@ public async Task UploadAsZip_Order() privateBlobStorageService.Setup(mock => mock.UploadFile(PrivateReleaseFiles, It.Is(path => - path.Contains(FilesPath(releaseVersion.Id, DataZip, null))), + path.Contains(FilesPath(releaseVersion.Id, DataZip))), zipFormFile )).Returns(Task.CompletedTask); @@ -2676,7 +2676,7 @@ public async Task UploadAsZip_Replacing() privateBlobStorageService.Setup(mock => mock.UploadFile(PrivateReleaseFiles, It.Is(path => - path.Contains(FilesPath(releaseVersion.Id, DataZip, null))), + path.Contains(FilesPath(releaseVersion.Id, DataZip))), zipFormFile )).Returns(Task.CompletedTask); diff --git a/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/ReleaseFileServiceTests.cs b/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/ReleaseFileServiceTests.cs index 56eaef34916..e70650a2a9f 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/ReleaseFileServiceTests.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/ReleaseFileServiceTests.cs @@ -2491,7 +2491,7 @@ public async Task UploadAncillary() privateBlobStorageService.Setup(mock => mock.UploadFile(PrivateReleaseFiles, It.Is(path => - path.Contains(FilesPath(releaseVersion.Id, Ancillary, null))), + path.Contains(FilesPath(releaseVersion.Id, Ancillary))), formFile )).Returns(Task.CompletedTask); @@ -2525,7 +2525,7 @@ public async Task UploadAncillary() privateBlobStorageService.Verify(mock => mock.UploadFile(PrivateReleaseFiles, It.Is(path => - path.Contains(FilesPath(releaseVersion.Id, Ancillary, null))), + path.Contains(FilesPath(releaseVersion.Id, Ancillary))), formFile ), Times.Once); @@ -2605,7 +2605,7 @@ public async Task UpdateAncillary() privateBlobStorageService.Setup(mock => mock.UploadFile(PrivateReleaseFiles, It.Is(path => - path.Contains(FilesPath(releaseVersion.Id, Ancillary, null))), + path.Contains(FilesPath(releaseVersion.Id, Ancillary))), newFormFile )).Returns(Task.CompletedTask); @@ -2639,7 +2639,7 @@ public async Task UpdateAncillary() privateBlobStorageService.Verify(mock => mock.UploadFile(PrivateReleaseFiles, It.Is(path => - path.Contains(FilesPath(releaseVersion.Id, Ancillary, null))), + path.Contains(FilesPath(releaseVersion.Id, Ancillary))), newFormFile ), Times.Once); @@ -2729,7 +2729,7 @@ public async Task UpdateAncillary_DoNotRemoveFileAttachedToOtherRelease() privateBlobStorageService.Setup(mock => mock.UploadFile(PrivateReleaseFiles, It.Is(path => - path.Contains(FilesPath(releaseVersion.Id, Ancillary, null))), + path.Contains(FilesPath(releaseVersion.Id, Ancillary))), newFormFile )).Returns(Task.CompletedTask); @@ -2763,7 +2763,7 @@ public async Task UpdateAncillary_DoNotRemoveFileAttachedToOtherRelease() privateBlobStorageService.Verify(mock => mock.UploadFile(PrivateReleaseFiles, It.Is(path => - path.Contains(FilesPath(releaseVersion.Id, Ancillary, null))), + path.Contains(FilesPath(releaseVersion.Id, Ancillary))), newFormFile ), Times.Once); @@ -2886,7 +2886,7 @@ public async Task UploadChart() privateBlobStorageService.Setup(mock => mock.UploadFile(PrivateReleaseFiles, It.Is(path => - path.Contains(FilesPath(releaseVersion.Id, Chart, null))), + path.Contains(FilesPath(releaseVersion.Id, Chart))), formFile )).Returns(Task.CompletedTask); @@ -2912,7 +2912,7 @@ public async Task UploadChart() privateBlobStorageService.Verify(mock => mock.UploadFile(PrivateReleaseFiles, It.Is(path => - path.Contains(FilesPath(releaseVersion.Id, Chart, null))), + path.Contains(FilesPath(releaseVersion.Id, Chart))), formFile ), Times.Once); diff --git a/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/ReleaseImageServiceTests.cs b/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/ReleaseImageServiceTests.cs index 5ee616de33a..967e65bda7e 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/ReleaseImageServiceTests.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Admin.Tests/Services/ReleaseImageServiceTests.cs @@ -208,7 +208,7 @@ public async Task Upload() privateBlobStorageService.Setup(mock => mock.UploadFile(PrivateReleaseFiles, It.Is(path => - path.Contains(FilesPath(releaseVersion.Id, Image, null))), + path.Contains(FilesPath(releaseVersion.Id, Image))), formFile )).Returns(Task.CompletedTask); @@ -234,7 +234,7 @@ public async Task Upload() privateBlobStorageService.Verify(mock => mock.UploadFile(PrivateReleaseFiles, It.Is(path => - path.Contains(FilesPath(releaseVersion.Id, Image, null))), + path.Contains(FilesPath(releaseVersion.Id, Image))), formFile ), Times.Once); diff --git a/src/GovUk.Education.ExploreEducationStatistics.Admin/Services/ReleaseDataFileService.cs b/src/GovUk.Education.ExploreEducationStatistics.Admin/Services/ReleaseDataFileService.cs index 8da1a64fc67..04f050d63a7 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Admin/Services/ReleaseDataFileService.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Admin/Services/ReleaseDataFileService.cs @@ -516,8 +516,8 @@ public async Task>> SaveDataSetsFromTemp replacingDataFile: replacingFile, order: releaseDataFileOrder); - var sourceDataFilePath = FileStoragePathUtils.FilesPath(releaseVersionId, FileType.Data, archiveDataSetFile.DataFileId); - var destinationDataFilePath = FileStoragePathUtils.FilesPath(releaseVersionId, FileType.Data, dataFile.Id); // Same path, but a new ID has been generated by the creation step above + var sourceDataFilePath = FileExtensions.Path(releaseVersionId, FileType.Data, archiveDataSetFile.DataFileId); + var destinationDataFilePath = FileExtensions.Path(releaseVersionId, FileType.Data, dataFile.Id); // Same path, but a new ID has been generated by the creation step above var dataReleaseFile = await _contentDbContext.ReleaseFiles .Include(rf => rf.File) @@ -535,8 +535,8 @@ public async Task>> SaveDataSetsFromTemp type: FileType.Metadata, createdById: _userService.GetUserId()); - var sourceMetaFilePath = FileStoragePathUtils.FilesPath(releaseVersionId, FileType.Metadata, archiveDataSetFile.MetaFileId); - var destinationMetaFilePath = FileStoragePathUtils.FilesPath(releaseVersionId, FileType.Metadata, metaFile.Id); // Same path, but a new ID has been generated by the creation step above + var sourceMetaFilePath = FileExtensions.Path(releaseVersionId, FileType.Metadata, archiveDataSetFile.MetaFileId); + var destinationMetaFilePath = FileExtensions.Path(releaseVersionId, FileType.Metadata, metaFile.Id); // Same path, but a new ID has been generated by the creation step above await _dataImportService.Import( subjectId: subjectId, @@ -555,8 +555,8 @@ private async Task> ValidateTempDataSetFileExistence( Guid releaseVersionId, ArchiveDataSetFileViewModel fileImportRequest) { - var dataBlobExists = await _privateBlobStorageService.CheckBlobExists(PrivateReleaseTempFiles, $"{FileStoragePathUtils.FilesPath(releaseVersionId, FileType.Data, fileImportRequest.DataFileId)}"); - var metaBlobExists = await _privateBlobStorageService.CheckBlobExists(PrivateReleaseTempFiles, $"{FileStoragePathUtils.FilesPath(releaseVersionId, FileType.Metadata, fileImportRequest.MetaFileId)}"); + var dataBlobExists = await _privateBlobStorageService.CheckBlobExists(PrivateReleaseTempFiles, $"{FileExtensions.Path(releaseVersionId, FileType.Data, fileImportRequest.DataFileId)}"); + var metaBlobExists = await _privateBlobStorageService.CheckBlobExists(PrivateReleaseTempFiles, $"{FileExtensions.Path(releaseVersionId, FileType.Metadata, fileImportRequest.MetaFileId)}"); if (!dataBlobExists || !metaBlobExists) { diff --git a/src/GovUk.Education.ExploreEducationStatistics.Common/Services/FileStoragePathUtils.cs b/src/GovUk.Education.ExploreEducationStatistics.Common/Services/FileStoragePathUtils.cs index 2c1740207a0..3b1c6bee4a5 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Common/Services/FileStoragePathUtils.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Common/Services/FileStoragePathUtils.cs @@ -99,10 +99,10 @@ public static string PublicContentReleaseSubjectsPath(string publicationSlug, st return $"{PublicContentReleaseParentPath(publicationSlug, releaseSlug)}/subjects.json"; } - public static string FilesPath(Guid rootPath, FileType type, Guid? filePath = null) + public static string FilesPath(Guid rootPath, FileType type) { var typeFolder = (type == Metadata ? Data : type).GetEnumLabel(); - return $"{rootPath}/{typeFolder}/{filePath}"; + return $"{rootPath}/{typeFolder}/"; } private static string AppendPathSeparator(string? segment = null)