From fffc5a63462ed98894f5062d8df2e376e7014612 Mon Sep 17 00:00:00 2001 From: Duncan Watson Date: Fri, 15 Nov 2024 15:34:37 +0000 Subject: [PATCH 1/6] EES-5660 - using special "draft" folder to hold the latest draft version of a data set prior to its publishing rather than including version in the folder name, as the version can change multiple times prior to publishing. --- .../Extensions/EnumerableExtensionsTests.cs | 90 +++++++++++++++++++ .../Extensions/EnumerableExtensions.cs | 21 +++++ .../DataSetVersionStatus.cs | 17 ++++ .../DataSetVersionAuthExtensions.cs | 10 +-- ...etionOfNextDataSetVersionFunctionsTests.cs | 55 ------------ ...nOfNextDataSetVersionOrchestrationTests.cs | 3 +- .../Functions/ActivityNames.cs | 2 - ...CompletionOfNextDataSetVersionFunctions.cs | 27 ------ ...letionOfNextDataSetVersionOrchestration.cs | 1 - .../DataSetVersionPathResolverTests.cs | 55 ++++++++++-- .../TestDataSetVersionPathResolver.cs | 7 +- .../DataSetVersionPathResolver.cs | 19 +++- .../Interfaces/IDataSetVersionPathResolver.cs | 4 +- .../Services/DataSetPublishingServiceTests.cs | 29 ++++++ .../appsettings.IntegrationTest.json | 3 + ...xploreEducationStatistics.Publisher.csproj | 1 + .../PublisherHostBuilderExtensions.cs | 6 ++ .../Services/DataSetPublishingService.cs | 19 ++-- 18 files changed, 256 insertions(+), 113 deletions(-) diff --git a/src/GovUk.Education.ExploreEducationStatistics.Common.Tests/Extensions/EnumerableExtensionsTests.cs b/src/GovUk.Education.ExploreEducationStatistics.Common.Tests/Extensions/EnumerableExtensionsTests.cs index 08917f36617..0ccf2a08f0a 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Common.Tests/Extensions/EnumerableExtensionsTests.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Common.Tests/Extensions/EnumerableExtensionsTests.cs @@ -466,6 +466,96 @@ public void ContainsAll_SourceListContainsAllValues_ReturnsTrue() Assert.True(containsAll); } + public class CartesianTests + { + [Fact] + public void TwoLists_Cartesian() + { + List list1 = [1, 2]; + List list2 = ["3", "4"]; + + List> expected = + [ + new Tuple(1, "3"), + new Tuple(1, "4"), + new Tuple(2, "3"), + new Tuple(2, "4") + ]; + + var actual = list1.Cartesian(list2); + Assert.Equal(expected, actual); + } + + [Fact] + public void TwoLists_EmptyList() + { + List list1 = [1, 2]; + Assert.Empty(list1.Cartesian(new List())); + } + + [Fact] + public void TwoLists_NullList() + { + List list1 = [1, 2]; + Assert.Empty(list1.Cartesian((List?) null)); + } + + [Fact] + public void ThreeLists_Cartesian() + { + List list1 = [1, 2]; + List list2 = ["3", "4"]; + List list3 = ['5', '6']; + + List> expected = + [ + new Tuple(1, "3", '5'), + new Tuple(1, "3", '6'), + new Tuple(1, "4", '5'), + new Tuple(1, "4", '6'), + new Tuple(2, "3", '5'), + new Tuple(2, "3", '6'), + new Tuple(2, "4", '5'), + new Tuple(2, "4", '6'), + ]; + + var actual = list1.Cartesian(list2, list3); + Assert.Equal(expected, actual); + } + + [Fact] + public void ThreeLists_EmptyFirstList() + { + List list1 = [1, 2]; + List list3 = ['5', '6']; + Assert.Empty(list1.Cartesian(new List(), list3)); + } + + [Fact] + public void ThreeLists_EmptySecondList() + { + List list1 = [1, 2]; + List list2 = ["3", "4"]; + Assert.Empty(list1.Cartesian(list2, new List())); + } + + [Fact] + public void ThreeLists_NullFirstList() + { + List list1 = [1, 2]; + List list3 = ['5', '6']; + Assert.Empty(list1.Cartesian((List?) null, list3)); + } + + [Fact] + public void ThreeLists_NullSecondList() + { + List list1 = [1, 2]; + List list2 = ["3", "4"]; + Assert.Empty(list1.Cartesian(list2, (List?) null)); + } + } + private static async Task> GetSuccessfulEither(int value) { await Task.Delay(5); diff --git a/src/GovUk.Education.ExploreEducationStatistics.Common/Extensions/EnumerableExtensions.cs b/src/GovUk.Education.ExploreEducationStatistics.Common/Extensions/EnumerableExtensions.cs index 4a780e7f1d3..36b32889a16 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Common/Extensions/EnumerableExtensions.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Common/Extensions/EnumerableExtensions.cs @@ -319,5 +319,26 @@ public static IOrderedEnumerable NaturalThenBy( { return source.ThenBy(keySelector, comparison.WithNaturalSort()); } + + public static IEnumerable> Cartesian( + this IEnumerable list1, + IEnumerable? list2) + { + return list2 == null + ? [] + : list1.Join(list2, _ => true, _ => true,(t1, t2) => new Tuple(t1, t2)); + } + + public static IEnumerable> Cartesian( + this IEnumerable list1, + IEnumerable? list2, + IEnumerable? list3) + { + return list2 == null || list3 == null + ? [] + : list1 + .Join(list2, _ => true, _ => true, (t1, t2) => new Tuple(t1, t2)) + .Join(list3, _ => true, _ => true, (tuple, t3) => new Tuple(tuple.Item1, tuple.Item2, t3)); + } } } diff --git a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Model/DataSetVersionStatus.cs b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Model/DataSetVersionStatus.cs index f62f4ac3578..6124daf0eea 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Model/DataSetVersionStatus.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Model/DataSetVersionStatus.cs @@ -1,4 +1,5 @@ using System.Text.Json.Serialization; +using GovUk.Education.ExploreEducationStatistics.Common.Utils; using Newtonsoft.Json.Converters; namespace GovUk.Education.ExploreEducationStatistics.Public.Data.Model; @@ -16,3 +17,19 @@ public enum DataSetVersionStatus Withdrawn, Cancelled, } + +public class DataSetVersionStatusConstants +{ + public static readonly IReadOnlyList PublicStatuses = new List( + [ + DataSetVersionStatus.Published, + DataSetVersionStatus.Withdrawn, + DataSetVersionStatus.Deprecated + ] + ); + + public static readonly IReadOnlyList PrivateStatuses = EnumUtil + .GetEnums() + .Except(PublicStatuses) + .ToList(); +} diff --git a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Model/Extensions/DataSetVersionAuthExtensions.cs b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Model/Extensions/DataSetVersionAuthExtensions.cs index 40170ae2c23..44d01584a51 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Model/Extensions/DataSetVersionAuthExtensions.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Model/Extensions/DataSetVersionAuthExtensions.cs @@ -4,14 +4,6 @@ namespace GovUk.Education.ExploreEducationStatistics.Public.Data.Model.Extension public static class DataSetVersionAuthExtensions { - private static readonly IReadOnlyList PublicStatuses = new List( - [ - DataSetVersionStatus.Published, - DataSetVersionStatus.Withdrawn, - DataSetVersionStatus.Deprecated - ] - ); - public static bool IsPublicStatus(this DataSetVersion dataSetVersion) => IsPublicStatus().Compile()(dataSetVersion); @@ -19,5 +11,5 @@ public static IQueryable WherePublicStatus(this IQueryable queryable.Where(IsPublicStatus()); private static Expression> IsPublicStatus() - => dataSetVersion => PublicStatuses.Contains(dataSetVersion.Status); + => dataSetVersion => DataSetVersionStatusConstants.PublicStatuses.Contains(dataSetVersion.Status); } diff --git a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Processor.Tests/Functions/ProcessCompletionOfNextDataSetVersionFunctionsTests.cs b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Processor.Tests/Functions/ProcessCompletionOfNextDataSetVersionFunctionsTests.cs index ad18ad3daa2..e28cb2e1847 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Processor.Tests/Functions/ProcessCompletionOfNextDataSetVersionFunctionsTests.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Processor.Tests/Functions/ProcessCompletionOfNextDataSetVersionFunctionsTests.cs @@ -2990,61 +2990,6 @@ .. DataFixture.DefaultTimePeriodMeta() } } - public class UpdateFileStoragePathTests( - ProcessorFunctionsIntegrationTestFixture fixture) - : ProcessCompletionOfNextDataSetVersionFunctionsTests(fixture) - { - private const DataSetVersionImportStage Stage = DataSetVersionImportStage.ManualMapping; - - [Fact] - public async Task Success_PathUpdated() - { - var (_, nextDataSetVersion, instanceId) = await CreateDataSetInitialAndNextVersion( - nextVersionStatus: DataSetVersionStatus.Mapping, - nextVersionImportStage: Stage); - - var dataSetVersionPathResolver = GetRequiredService(); - var originalStoragePath = dataSetVersionPathResolver.DirectoryPath(nextDataSetVersion); - Directory.CreateDirectory(originalStoragePath); - - await using var publicDataDbContext = GetDbContext(); - - nextDataSetVersion.VersionMajor++; - - publicDataDbContext.DataSetVersions.Update(nextDataSetVersion); - await publicDataDbContext.SaveChangesAsync(); - - var newStoragePath = dataSetVersionPathResolver.DirectoryPath(nextDataSetVersion); - - await UpdateFileStoragePath(instanceId); - - Assert.False(Directory.Exists(originalStoragePath)); - Assert.True(Directory.Exists(newStoragePath)); - } - - [Fact] - public async Task Success_PathNotUpdated() - { - var (_, nextDataSetVersion, instanceId) = await CreateDataSetInitialAndNextVersion( - nextVersionStatus: DataSetVersionStatus.Mapping, - nextVersionImportStage: Stage); - - var dataSetVersionPathResolver = GetRequiredService(); - var originalStoragePath = dataSetVersionPathResolver.DirectoryPath(nextDataSetVersion); - Directory.CreateDirectory(originalStoragePath); - - await UpdateFileStoragePath(instanceId); - - Assert.True(Directory.Exists(originalStoragePath)); - } - - private async Task UpdateFileStoragePath(Guid instanceId) - { - var function = GetRequiredService(); - await function.UpdateFileStoragePath(instanceId, CancellationToken.None); - } - } - public class CompleteNextDataSetVersionImportProcessingTests( ProcessorFunctionsIntegrationTestFixture fixture) : ProcessCompletionOfNextDataSetVersionFunctionsTests(fixture) diff --git a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Processor.Tests/Functions/ProcessCompletionOfNextDataSetVersionOrchestrationTests.cs b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Processor.Tests/Functions/ProcessCompletionOfNextDataSetVersionOrchestrationTests.cs index 4cf6c417a07..c4fc07e53de 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Processor.Tests/Functions/ProcessCompletionOfNextDataSetVersionOrchestrationTests.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Processor.Tests/Functions/ProcessCompletionOfNextDataSetVersionOrchestrationTests.cs @@ -27,7 +27,6 @@ public async Task Success() string[] expectedActivitySequence = [ - ActivityNames.UpdateFileStoragePath, ActivityNames.ImportMetadata, ActivityNames.CreateChanges, ActivityNames.ImportData, @@ -60,7 +59,7 @@ public async Task ActivityFunctionThrowsException_CallsHandleFailureActivity() mockOrchestrationContext .InSequence(activitySequence) .Setup(context => - context.CallActivityAsync(ActivityNames.UpdateFileStoragePath, + context.CallActivityAsync(ActivityNames.ImportMetadata, mockOrchestrationContext.Object.InstanceId, null)) .Throws(); diff --git a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Processor/Functions/ActivityNames.cs b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Processor/Functions/ActivityNames.cs index f1f002e15c4..77b7a53dda9 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Processor/Functions/ActivityNames.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Processor/Functions/ActivityNames.cs @@ -18,8 +18,6 @@ internal static class ActivityNames public const string CreateChanges = nameof(ProcessCompletionOfNextDataSetVersionFunctions.CreateChanges); - public const string UpdateFileStoragePath = - nameof(ProcessCompletionOfNextDataSetVersionFunctions.UpdateFileStoragePath); public const string CompleteNextDataSetVersionImportProcessing = nameof(ProcessCompletionOfNextDataSetVersionFunctions.CompleteNextDataSetVersionImportProcessing); } diff --git a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Processor/Functions/ProcessCompletionOfNextDataSetVersionFunctions.cs b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Processor/Functions/ProcessCompletionOfNextDataSetVersionFunctions.cs index e9bae2c8a76..af874a9ba47 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Processor/Functions/ProcessCompletionOfNextDataSetVersionFunctions.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Processor/Functions/ProcessCompletionOfNextDataSetVersionFunctions.cs @@ -24,33 +24,6 @@ public async Task CreateChanges( await dataSetVersionChangeService.CreateChanges(dataSetVersionImport.DataSetVersionId, cancellationToken); } - [Function(ActivityNames.UpdateFileStoragePath)] - public async Task UpdateFileStoragePath( - [ActivityTrigger] Guid instanceId, - CancellationToken cancellationToken) - { - var dataSetVersionImport = await GetDataSetVersionImport(instanceId, cancellationToken); - - var dataSetVersion = dataSetVersionImport.DataSetVersion; - - var currentLiveVersion = await publicDataDbContext - .DataSets - .Where(dataSet => dataSet.Id == dataSetVersion.DataSetId) - .Select(dataSet => dataSet.LatestLiveVersion!) - .SingleAsync(cancellationToken); - - var originalPath = dataSetVersionPathResolver.DirectoryPath( - dataSetVersion: dataSetVersion, - versionNumber: currentLiveVersion.DefaultNextVersion()); - - var newPath = dataSetVersionPathResolver.DirectoryPath(dataSetVersion); - - if (originalPath != newPath) - { - Directory.Move(sourceDirName: originalPath, destDirName: newPath); - } - } - [Function(ActivityNames.CompleteNextDataSetVersionImportProcessing)] public async Task CompleteNextDataSetVersionImportProcessing( [ActivityTrigger] Guid instanceId, diff --git a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Processor/Functions/ProcessCompletionOfNextDataSetVersionOrchestration.cs b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Processor/Functions/ProcessCompletionOfNextDataSetVersionOrchestration.cs index 57db11c7180..299f3d1bcb1 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Processor/Functions/ProcessCompletionOfNextDataSetVersionOrchestration.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Processor/Functions/ProcessCompletionOfNextDataSetVersionOrchestration.cs @@ -23,7 +23,6 @@ public static async Task ProcessCompletionOfNextDataSetVersionImport( try { - await context.CallActivity(ActivityNames.UpdateFileStoragePath, logger, context.InstanceId); await context.CallActivityExclusively(ActivityNames.ImportMetadata, logger, context.InstanceId); await context.CallActivity(ActivityNames.CreateChanges, logger, context.InstanceId); await context.CallActivity(ActivityNames.ImportData, logger, context.InstanceId); diff --git a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Services.Tests/DataSetVersionPathResolverTests.cs b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Services.Tests/DataSetVersionPathResolverTests.cs index 3e8efed3b8a..e988bf36e26 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Services.Tests/DataSetVersionPathResolverTests.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Services.Tests/DataSetVersionPathResolverTests.cs @@ -9,7 +9,6 @@ using GovUk.Education.ExploreEducationStatistics.Public.Data.Services.Options; using Microsoft.AspNetCore.Hosting; using Microsoft.Extensions.Hosting; -using Microsoft.Extensions.Options; using Moq; using Semver; @@ -39,12 +38,20 @@ public void EmptyBasePath_Throws(string basePath) public class PathTests : DataSetVersionPathResolverTests { - public static readonly TheoryData GetEnvironmentNames = new() - { + private static readonly string[] EnvironmentNames = + [ Environments.Development, HostEnvironmentExtensions.IntegrationTestEnvironment, Environments.Production - }; + ]; + + public static readonly TheoryData GetEnvironmentNames = new(EnvironmentNames); + + public static readonly TheoryData> GetEnvironmentNamesAndPublicStatuses = + new(EnvironmentNames.Cartesian(DataSetVersionStatusConstants.PublicStatuses)); + + public static readonly TheoryData> GetEnvironmentNamesAndPrivateStatuses = + new(EnvironmentNames.Cartesian(DataSetVersionStatusConstants.PrivateStatuses)); [Fact] public void DevelopmentEnv_ValidBasePath() @@ -116,10 +123,14 @@ public void ProductionEnv_ValidBasePath() } [Theory] - [MemberData(nameof(GetEnvironmentNames))] - public void ValidDirectoryPath(string environmentName) + [MemberData(nameof(GetEnvironmentNamesAndPublicStatuses))] + public void ValidDirectoryPath_PublicVersion(Tuple environmentNameAndStatus) { - DataSetVersion version = _dataFixture.DefaultDataSetVersion(); + var (environmentName, status) = environmentNameAndStatus; + + DataSetVersion version = _dataFixture + .DefaultDataSetVersion() + .WithStatus(status); _webHostEnvironmentMock .SetupGet(s => s.EnvironmentName) @@ -139,9 +150,37 @@ public void ValidDirectoryPath(string environmentName) resolver.DirectoryPath(version)); } + [Theory] + [MemberData(nameof(GetEnvironmentNamesAndPrivateStatuses))] + public void ValidDirectoryPath_PrivateVersion(Tuple environmentNameAndStatus) + { + var (environmentName, status) = environmentNameAndStatus; + + DataSetVersion version = _dataFixture + .DefaultDataSetVersion() + .WithStatus(status); + + _webHostEnvironmentMock + .SetupGet(s => s.EnvironmentName) + .Returns(environmentName); + + var resolver = BuildService(options: new DataFilesOptions + { + BasePath = Path.Combine("data", "data-files") + }); + + Assert.Equal( + Path.Combine( + resolver.DataSetsPath(), + version.DataSetId.ToString(), + "draft" + ), + resolver.DirectoryPath(version)); + } + [Theory] [MemberData(nameof(GetEnvironmentNames))] - public void ValidDirectoryPath_OptionalVersionArgument(string environmentName) + public void ValidDirectoryPath_VersionArgument(string environmentName) { DataSetVersion version = _dataFixture.DefaultDataSetVersion(); diff --git a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Services.Tests/TestDataSetVersionPathResolver.cs b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Services.Tests/TestDataSetVersionPathResolver.cs index b94f8f2faeb..3b74f9bdc81 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Services.Tests/TestDataSetVersionPathResolver.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Services.Tests/TestDataSetVersionPathResolver.cs @@ -18,7 +18,12 @@ public class TestDataSetVersionPathResolver : IDataSetVersionPathResolver public string Directory { get; set; } = string.Empty; - public string DirectoryPath(DataSetVersion dataSetVersion, SemVersion? versionNumber = null) + public string DirectoryPath(DataSetVersion dataSetVersion) + { + return Path.Combine(_basePath, Directory); + } + + public string DirectoryPath(DataSetVersion dataSetVersion, SemVersion versionNumber) { return Path.Combine(_basePath, Directory); } diff --git a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Services/DataSetVersionPathResolver.cs b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Services/DataSetVersionPathResolver.cs index 85c53c51d19..c8e16d37ce2 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Services/DataSetVersionPathResolver.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Services/DataSetVersionPathResolver.cs @@ -1,6 +1,7 @@ using GovUk.Education.ExploreEducationStatistics.Common.Extensions; using GovUk.Education.ExploreEducationStatistics.Common.Utils; using GovUk.Education.ExploreEducationStatistics.Public.Data.Model; +using GovUk.Education.ExploreEducationStatistics.Public.Data.Model.Extensions; using GovUk.Education.ExploreEducationStatistics.Public.Data.Services.Interfaces; using GovUk.Education.ExploreEducationStatistics.Public.Data.Services.Options; using Microsoft.AspNetCore.Hosting; @@ -12,6 +13,8 @@ namespace GovUk.Education.ExploreEducationStatistics.Public.Data.Services; public class DataSetVersionPathResolver : IDataSetVersionPathResolver { + private const string DraftVersionFolderName = "draft"; + private readonly IOptions _options; private readonly IWebHostEnvironment _environment; private readonly string _basePath; @@ -34,12 +37,24 @@ public DataSetVersionPathResolver(IOptions options, IWebHostEn public string BasePath() => _basePath; - public string DirectoryPath(DataSetVersion dataSetVersion, SemVersion? versionNumber = null) + public string DirectoryPath(DataSetVersion dataSetVersion) { + var folderName = !dataSetVersion.IsPublicStatus() + ? DraftVersionFolderName + : $"v{dataSetVersion.SemVersion()}"; + return Path.Combine( ((IDataSetVersionPathResolver) this).DataSetsPath(), dataSetVersion.DataSetId.ToString(), - $"v{versionNumber ?? dataSetVersion.SemVersion()}"); + folderName); + } + + public string DirectoryPath(DataSetVersion dataSetVersion, SemVersion versionNumber) + { + return Path.Combine( + ((IDataSetVersionPathResolver) this).DataSetsPath(), + dataSetVersion.DataSetId.ToString(), + $"v{versionNumber}"); } private string GetBasePath() diff --git a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Services/Interfaces/IDataSetVersionPathResolver.cs b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Services/Interfaces/IDataSetVersionPathResolver.cs index 8bff9e4e50b..22c865671c1 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Services/Interfaces/IDataSetVersionPathResolver.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Services/Interfaces/IDataSetVersionPathResolver.cs @@ -10,7 +10,9 @@ public interface IDataSetVersionPathResolver string DataSetsPath() => Path.Combine(BasePath(), DataSetFilenames.DataSetsDirectory); - string DirectoryPath(DataSetVersion dataSetVersion, SemVersion? versionNumber = null); + string DirectoryPath(DataSetVersion dataSetVersion); + + string DirectoryPath(DataSetVersion dataSetVersion, SemVersion versionNumber); string CsvDataPath(DataSetVersion dataSetVersion) => Path.Combine(DirectoryPath(dataSetVersion), DataSetFilenames.CsvDataFile); diff --git a/src/GovUk.Education.ExploreEducationStatistics.Publisher.Tests/Services/DataSetPublishingServiceTests.cs b/src/GovUk.Education.ExploreEducationStatistics.Publisher.Tests/Services/DataSetPublishingServiceTests.cs index 0b1eaea2cb9..e0239943b25 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Publisher.Tests/Services/DataSetPublishingServiceTests.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Publisher.Tests/Services/DataSetPublishingServiceTests.cs @@ -1,4 +1,5 @@ using System; +using System.IO; using System.Threading.Tasks; using GovUk.Education.ExploreEducationStatistics.Common.Extensions; using GovUk.Education.ExploreEducationStatistics.Common.Model; @@ -9,9 +10,11 @@ using GovUk.Education.ExploreEducationStatistics.Public.Data.Model; using GovUk.Education.ExploreEducationStatistics.Public.Data.Model.Database; using GovUk.Education.ExploreEducationStatistics.Public.Data.Model.Tests.Fixtures; +using GovUk.Education.ExploreEducationStatistics.Public.Data.Services.Interfaces; using GovUk.Education.ExploreEducationStatistics.Publisher.Services.Interfaces; using Microsoft.EntityFrameworkCore; using Xunit; +using File = GovUk.Education.ExploreEducationStatistics.Content.Model.File; namespace GovUk.Education.ExploreEducationStatistics.Publisher.Tests.Services; @@ -53,6 +56,10 @@ await AddTestData(context => dataSet.LatestDraftVersionId = dataSetVersion.Id; }); + + var dataSetVersionPathResolver = GetRequiredService(); + var draftFolderPath = dataSetVersionPathResolver.DirectoryPath(dataSetVersion); + Directory.CreateDirectory(draftFolderPath); var service = GetRequiredService(); await service.PublishDataSets([releaseVersion.Id]); @@ -76,6 +83,13 @@ await AddTestData(context => // Data set should be published at the same time as the latest live version Assert.Equal(publishedDataSet.LatestLiveVersion.Published, publishedDataSet.Published); + + // The Public API folder for the data set version should be updated to reflect its + // version number at the time of publishing. + var versionedFolderPath = dataSetVersionPathResolver.DirectoryPath(publishedDataSet.LatestLiveVersion); + Assert.False(Directory.Exists(draftFolderPath)); + Assert.True(Directory.Exists(versionedFolderPath)); + Assert.EndsWith($"v{publishedDataSet.LatestLiveVersion.SemVersion()}", versionedFolderPath); } [Fact] @@ -116,6 +130,10 @@ await AddTestData(context => dataSet.LatestLiveVersionId = dataSetVersions[0].Id; dataSet.LatestDraftVersionId = dataSetVersions[1].Id; }); + + var dataSetVersionPathResolver = GetRequiredService(); + var draftFolderPath = dataSetVersionPathResolver.DirectoryPath(dataSetVersions[1]); + Directory.CreateDirectory(draftFolderPath); var service = GetRequiredService(); await service.PublishDataSets([releaseVersion.Id]); @@ -144,6 +162,13 @@ await AddTestData(context => Assert.Equal(2, publishedDataSet.Versions.Count); publishedDataSet.Versions.ForEach(version => Assert.Equal(DataSetVersionStatus.Published, version.Status)); + + // The Public API folder for the data set version should be updated to reflect its + // version number at the time of publishing. + var versionedFolderPath = dataSetVersionPathResolver.DirectoryPath(publishedDataSet.LatestLiveVersion); + Assert.False(Directory.Exists(draftFolderPath)); + Assert.True(Directory.Exists(versionedFolderPath)); + Assert.EndsWith($"v{publishedDataSet.LatestLiveVersion.SemVersion()}", versionedFolderPath); } [Fact] @@ -336,6 +361,10 @@ await AddTestData(context => dataSet.LatestLiveVersionId = dataSetVersion1.Id; dataSet.LatestDraftVersionId = dataSetVersion2.Id; }); + + var dataSetVersionPathResolver = GetRequiredService(); + var draftFolderPath = dataSetVersionPathResolver.DirectoryPath(dataSetVersion2); + Directory.CreateDirectory(draftFolderPath); var service = GetRequiredService(); await service.PublishDataSets([amendmentReleaseVersion.Id]); diff --git a/src/GovUk.Education.ExploreEducationStatistics.Publisher.Tests/appsettings.IntegrationTest.json b/src/GovUk.Education.ExploreEducationStatistics.Publisher.Tests/appsettings.IntegrationTest.json index 69bb2edb9b6..502829a54d9 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Publisher.Tests/appsettings.IntegrationTest.json +++ b/src/GovUk.Education.ExploreEducationStatistics.Publisher.Tests/appsettings.IntegrationTest.json @@ -5,5 +5,8 @@ "NotifierStorageConnectionString": "this-will-be-overridden-at-startup", "PublisherStorageConnectionString": "this-will-be-overridden-at-startup" }, + "DataFiles": { + "BasePath": "Resources/DataFiles" + }, "PublicDataDbExists": true } diff --git a/src/GovUk.Education.ExploreEducationStatistics.Publisher/GovUk.Education.ExploreEducationStatistics.Publisher.csproj b/src/GovUk.Education.ExploreEducationStatistics.Publisher/GovUk.Education.ExploreEducationStatistics.Publisher.csproj index f16b100adeb..3534a088a69 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Publisher/GovUk.Education.ExploreEducationStatistics.Publisher.csproj +++ b/src/GovUk.Education.ExploreEducationStatistics.Publisher/GovUk.Education.ExploreEducationStatistics.Publisher.csproj @@ -30,6 +30,7 @@ + diff --git a/src/GovUk.Education.ExploreEducationStatistics.Publisher/PublisherHostBuilderExtensions.cs b/src/GovUk.Education.ExploreEducationStatistics.Publisher/PublisherHostBuilderExtensions.cs index 05a69e197fa..3fc0b16ebdd 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Publisher/PublisherHostBuilderExtensions.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Publisher/PublisherHostBuilderExtensions.cs @@ -21,6 +21,9 @@ using GovUk.Education.ExploreEducationStatistics.Data.Model.Repository.Interfaces; using GovUk.Education.ExploreEducationStatistics.Notifier.Model; using GovUk.Education.ExploreEducationStatistics.Public.Data.Model.Database; +using GovUk.Education.ExploreEducationStatistics.Public.Data.Services; +using GovUk.Education.ExploreEducationStatistics.Public.Data.Services.Interfaces; +using GovUk.Education.ExploreEducationStatistics.Public.Data.Services.Options; using GovUk.Education.ExploreEducationStatistics.Publisher.Model; using GovUk.Education.ExploreEducationStatistics.Publisher.Options; using GovUk.Education.ExploreEducationStatistics.Publisher.Services; @@ -132,6 +135,9 @@ public static IHostBuilder ConfigurePublisherHostBuilder(this IHostBuilder hostB // TODO EES-5073 Remove this check when the Public Data db is available in all Azure environments. if (publicDataDbExists) { + services.AddOptions() + .Bind(configuration.GetRequiredSection(DataFilesOptions.Section)); + services.AddScoped(); services.AddScoped(); } else diff --git a/src/GovUk.Education.ExploreEducationStatistics.Publisher/Services/DataSetPublishingService.cs b/src/GovUk.Education.ExploreEducationStatistics.Publisher/Services/DataSetPublishingService.cs index 1e31d317953..e74d05a7390 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Publisher/Services/DataSetPublishingService.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Publisher/Services/DataSetPublishingService.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.IO; using System.Linq; using System.Threading.Tasks; using GovUk.Education.ExploreEducationStatistics.Common.Model; @@ -8,6 +9,7 @@ using GovUk.Education.ExploreEducationStatistics.Notifier.Model; using GovUk.Education.ExploreEducationStatistics.Public.Data.Model; using GovUk.Education.ExploreEducationStatistics.Public.Data.Model.Database; +using GovUk.Education.ExploreEducationStatistics.Public.Data.Services.Interfaces; using GovUk.Education.ExploreEducationStatistics.Publisher.Services.Interfaces; using Microsoft.EntityFrameworkCore; @@ -16,7 +18,8 @@ namespace GovUk.Education.ExploreEducationStatistics.Publisher.Services; public class DataSetPublishingService( ContentDbContext contentDbContext, PublicDataDbContext publicDataDbContext, - INotifierClient notifierClient + INotifierClient notifierClient, + IDataSetVersionPathResolver dataSetVersionPathResolver ) : IDataSetPublishingService { public async Task PublishDataSets(Guid[] releaseVersionIds) @@ -46,8 +49,11 @@ private async Task> PromoteDraftDataSetVersions(IE foreach (var dataSet in dataSets) { - var currentDraftVersion = dataSet.LatestDraftVersion; - currentDraftVersion!.Status = DataSetVersionStatus.Published; + var currentDraftVersion = dataSet.LatestDraftVersion!; + + var originalFilePath = dataSetVersionPathResolver.DirectoryPath(currentDraftVersion); + + currentDraftVersion.Status = DataSetVersionStatus.Published; currentDraftVersion.Published = DateTimeOffset.UtcNow; dataSet.Status = DataSetStatus.Published; @@ -58,9 +64,12 @@ private async Task> PromoteDraftDataSetVersions(IE dataSet.LatestDraftVersion = null; dataSet.LatestDraftVersionId = null; - } + + var newFilePath = dataSetVersionPathResolver.DirectoryPath(currentDraftVersion); + Directory.Move(sourceDirName: originalFilePath, destDirName: newFilePath); - await publicDataDbContext.SaveChangesAsync(); + await publicDataDbContext.SaveChangesAsync(); + } return dataSets .Select(ds => ds.LatestLiveVersion!) From 34db1b37dfec6550c52fc886395d039aec53e062 Mon Sep 17 00:00:00 2001 From: Duncan Watson Date: Mon, 18 Nov 2024 14:26:28 +0000 Subject: [PATCH 2/6] EES-5560 - added custom migration to update draft DataSetVersion folders to be "draft" rather than version-based names. --- .../Migrations/Custom/ICustomMigration.cs | 7 - .../Startup.cs | 13 +- .../Database/ICustomMigration.cs | 6 + .../Extensions/EnumerableExtensions.cs | 32 ++-- ...rateDraftDataSetVersionFolderNamesTests.cs | 174 ++++++++++++++++++ ...0_MigrateDraftDataSetVersionFolderNames.cs | 48 +++++ .../Startup.cs | 31 +++- .../Fixtures/DataSetGeneratorExtensions.cs | 12 +- .../DataSetVersionGeneratorExtensions.cs | 10 + .../DataSetVersionAuthExtensions.cs | 9 + ...ucationStatistics.Public.Data.Model.csproj | 76 ++++---- ...nOfNextDataSetVersionOrchestrationTests.cs | 5 + 12 files changed, 351 insertions(+), 72 deletions(-) delete mode 100644 src/GovUk.Education.ExploreEducationStatistics.Admin/Migrations/Custom/ICustomMigration.cs create mode 100644 src/GovUk.Education.ExploreEducationStatistics.Common/Database/ICustomMigration.cs create mode 100644 src/GovUk.Education.ExploreEducationStatistics.Public.Data.Api.Tests/Migrations/EES5660_MigrateDraftDataSetVersionFolderNamesTests.cs create mode 100644 src/GovUk.Education.ExploreEducationStatistics.Public.Data.Api/Migrations/EES5660_MigrateDraftDataSetVersionFolderNames.cs diff --git a/src/GovUk.Education.ExploreEducationStatistics.Admin/Migrations/Custom/ICustomMigration.cs b/src/GovUk.Education.ExploreEducationStatistics.Admin/Migrations/Custom/ICustomMigration.cs deleted file mode 100644 index afe6a91620b..00000000000 --- a/src/GovUk.Education.ExploreEducationStatistics.Admin/Migrations/Custom/ICustomMigration.cs +++ /dev/null @@ -1,7 +0,0 @@ -namespace GovUk.Education.ExploreEducationStatistics.Admin.Migrations.Custom -{ - public interface ICustomMigration - { - void Apply(); - } -} \ No newline at end of file diff --git a/src/GovUk.Education.ExploreEducationStatistics.Admin/Startup.cs b/src/GovUk.Education.ExploreEducationStatistics.Admin/Startup.cs index 736cacf8089..cd6e7f0f16d 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Admin/Startup.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Admin/Startup.cs @@ -3,7 +3,6 @@ using GovUk.Education.ExploreEducationStatistics.Admin.Database; using GovUk.Education.ExploreEducationStatistics.Admin.Hubs; using GovUk.Education.ExploreEducationStatistics.Admin.Hubs.Filters; -using GovUk.Education.ExploreEducationStatistics.Admin.Migrations.Custom; using GovUk.Education.ExploreEducationStatistics.Admin.Models; using GovUk.Education.ExploreEducationStatistics.Admin.Options; using GovUk.Education.ExploreEducationStatistics.Admin.Requests.Public.Data; @@ -782,9 +781,9 @@ private void UpdateDatabase(IApplicationBuilder app, IWebHostEnvironment env) { context.Database.SetCommandTimeout(int.MaxValue); context.Database.Migrate(); - - ApplyCustomMigrations(); } + + ApplyCustomMigrations(app); } if (env.IsDevelopment()) @@ -798,8 +797,14 @@ private void UpdateDatabase(IApplicationBuilder app, IWebHostEnvironment env) } } - private static void ApplyCustomMigrations(params ICustomMigration[] migrations) + private static void ApplyCustomMigrations(IApplicationBuilder app) { + using var serviceScope = app.ApplicationServices + .GetRequiredService() + .CreateScope(); + + var migrations = serviceScope.ServiceProvider.GetServices(); + foreach (var migration in migrations) { migration.Apply(); diff --git a/src/GovUk.Education.ExploreEducationStatistics.Common/Database/ICustomMigration.cs b/src/GovUk.Education.ExploreEducationStatistics.Common/Database/ICustomMigration.cs new file mode 100644 index 00000000000..a899c9b3e16 --- /dev/null +++ b/src/GovUk.Education.ExploreEducationStatistics.Common/Database/ICustomMigration.cs @@ -0,0 +1,6 @@ +namespace GovUk.Education.ExploreEducationStatistics.Common.Database; + +public interface ICustomMigration +{ + void Apply(); +} diff --git a/src/GovUk.Education.ExploreEducationStatistics.Common/Extensions/EnumerableExtensions.cs b/src/GovUk.Education.ExploreEducationStatistics.Common/Extensions/EnumerableExtensions.cs index 36b32889a16..130bb844d3b 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Common/Extensions/EnumerableExtensions.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Common/Extensions/EnumerableExtensions.cs @@ -221,7 +221,7 @@ public static IEnumerable WhereNotNull(this IEnumerable source) } public static IAsyncEnumerable WhereNotNull(this IAsyncEnumerable source) - where T: class + where T : class { return source.Where(item => item is not null)!; } @@ -264,7 +264,7 @@ public static bool IsSameAsIgnoringOrder(this IEnumerable first, IEnumerab return !(firstNotInSecond.Any() || secondNotInFirst.Any()); } - + public static Tuple ToTuple2(this IEnumerable collection) where T : class { @@ -275,10 +275,10 @@ public static Tuple ToTuple2(this IEnumerable collection) throw new ArgumentException( $"Expected 2 list items when constructing a 2-tuple, but found {list.Count}"); } - + return new Tuple(list[0], list[1]); } - + public static Tuple ToTuple3(this IEnumerable collection) where T : class { @@ -289,7 +289,7 @@ public static Tuple ToTuple3(this IEnumerable collection) throw new ArgumentException( $"Expected 3 list items when constructing a 3-tuple, but found {list.Count}"); } - + return new Tuple(list[0], list[1], list[2]); } @@ -297,7 +297,7 @@ public static bool ContainsAll(this IEnumerable source, IEnumerable val { return values.All(id => source.Contains(id)); } - + /// /// Order some objects, according to a string key, in natural order for humans to read. /// @@ -320,25 +320,29 @@ public static IOrderedEnumerable NaturalThenBy( return source.ThenBy(keySelector, comparison.WithNaturalSort()); } - public static IEnumerable> Cartesian( + public static List> Cartesian( this IEnumerable list1, IEnumerable? list2) { - return list2 == null - ? [] - : list1.Join(list2, _ => true, _ => true,(t1, t2) => new Tuple(t1, t2)); + return list2 == null + ? [] + : list1 + .Join(list2, _ => true, _ => true, (t1, t2) => new Tuple(t1, t2)) + .ToList(); } - - public static IEnumerable> Cartesian( + + public static List> Cartesian( this IEnumerable list1, IEnumerable? list2, IEnumerable? list3) { return list2 == null || list3 == null - ? [] + ? [] : list1 .Join(list2, _ => true, _ => true, (t1, t2) => new Tuple(t1, t2)) - .Join(list3, _ => true, _ => true, (tuple, t3) => new Tuple(tuple.Item1, tuple.Item2, t3)); + .Join(list3, _ => true, _ => true, + (tuple, t3) => new Tuple(tuple.Item1, tuple.Item2, t3)) + .ToList(); } } } diff --git a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Api.Tests/Migrations/EES5660_MigrateDraftDataSetVersionFolderNamesTests.cs b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Api.Tests/Migrations/EES5660_MigrateDraftDataSetVersionFolderNamesTests.cs new file mode 100644 index 00000000000..450d88f0f11 --- /dev/null +++ b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Api.Tests/Migrations/EES5660_MigrateDraftDataSetVersionFolderNamesTests.cs @@ -0,0 +1,174 @@ +using GovUk.Education.ExploreEducationStatistics.Common.Extensions; +using GovUk.Education.ExploreEducationStatistics.Common.Tests.Extensions; +using GovUk.Education.ExploreEducationStatistics.Common.Utils; +using GovUk.Education.ExploreEducationStatistics.Public.Data.Api.Migrations; +using GovUk.Education.ExploreEducationStatistics.Public.Data.Api.Tests.Fixture; +using GovUk.Education.ExploreEducationStatistics.Public.Data.Api.Tests.TheoryData; +using GovUk.Education.ExploreEducationStatistics.Public.Data.Model; +using GovUk.Education.ExploreEducationStatistics.Public.Data.Model.Database; +using GovUk.Education.ExploreEducationStatistics.Public.Data.Model.Tests.Fixtures; +using GovUk.Education.ExploreEducationStatistics.Public.Data.Services.Interfaces; +using Microsoft.Extensions.DependencyInjection; + +namespace GovUk.Education.ExploreEducationStatistics.Public.Data.Api.Tests.Migrations; + +public class EES5660_MigrateDraftDataSetVersionFolderNamesTests(TestApplicationFactory testApp) + : IntegrationTestFixture(testApp) +{ + public static readonly TheoryData> + PrivateDataSetVersionStatusAndTypes = new(DataSetVersionStatusConstants + .PrivateStatuses + .Cartesian(EnumUtil.GetEnums())); + + public static readonly TheoryData> + PublicDataSetVersionStatusAndTypes = new(DataSetVersionStatusConstants + .PublicStatuses + .Cartesian(EnumUtil.GetEnums())); + + [Theory] + [MemberData(nameof(PrivateDataSetVersionStatusAndTypes))] + public async Task Success_NonMigratedDraft(Tuple statusAndType) + { + var (status, type) = statusAndType; + + DataSetVersion dataSetVersion = DataFixture + .DefaultDataSetVersion() + .WithId(Guid.NewGuid()) + .WithDataSet(DataFixture + .DefaultDataSet()) + .WithStatus(status) + .WithVersionNumber( + major: type == DataSetVersionType.Major ? 2 : 1, + minor: type == DataSetVersionType.Major ? 0 : 1); + + await TestApp.AddTestData(context => context.DataSetVersions.Add(dataSetVersion)); + + var pathResolver = TestApp.Services.GetRequiredService(); + + var versionedFolder = pathResolver.DirectoryPath(dataSetVersion, dataSetVersion.SemVersion()); + Directory.CreateDirectory(versionedFolder); + File.Create(Path.Combine(versionedFolder, "test.txt")); + + var migration = TestApp.Services.GetRequiredService(); + + migration.Apply(); + + // Assert that the original folder that used the draft's version in its name no longer exists. + Assert.False(Directory.Exists(versionedFolder)); + + // Assert that the original folder has moved to use the new special "draft" folder. + var expectedDraftFolder = pathResolver.DirectoryPath(dataSetVersion); + Assert.True(Directory.Exists(expectedDraftFolder)); + Assert.True(File.Exists(Path.Combine(expectedDraftFolder, "test.txt"))); + } + + [Theory] + [MemberData(nameof(PrivateDataSetVersionStatusAndTypes))] + public async Task Success_AlreadyMigratedDraft(Tuple statusAndType) + { + var (status, type) = statusAndType; + + DataSetVersion dataSetVersion = DataFixture + .DefaultDataSetVersion() + .WithId(Guid.NewGuid()) + .WithDataSet(DataFixture + .DefaultDataSet() + .WithId(Guid.NewGuid())) + .WithStatus(status) + .WithVersionNumber( + major: type == DataSetVersionType.Major ? 2 : 1, + minor: type == DataSetVersionType.Major ? 0 : 1); + + await TestApp.AddTestData(context => context.DataSetVersions.Add(dataSetVersion)); + + var pathResolver = TestApp.Services.GetRequiredService(); + + var draftFolder = pathResolver.DirectoryPath(dataSetVersion); + Directory.CreateDirectory(draftFolder); + File.Create(Path.Combine(draftFolder, "test.txt")); + + var migration = TestApp.Services.GetRequiredService(); + + migration.Apply(); + + // Assert that the existing draft folder is left untouched. + Assert.True(Directory.Exists(draftFolder)); + Assert.True(File.Exists(Path.Combine(draftFolder, "test.txt"))); + } + + [Fact] + public async Task Failure_DraftFolderAndVersionedFolderExist() + { + DataSetVersion dataSetVersion = DataFixture + .DefaultDataSetVersion() + .WithDataSet(DataFixture.DefaultDataSet()) + .WithStatus(DataSetVersionStatus.Draft) + .WithVersionNumber(major: 2, minor: 0); + + await TestApp.AddTestData(context => context.DataSetVersions.Add(dataSetVersion)); + + var pathResolver = TestApp.Services.GetRequiredService(); + + var versionedFolder = pathResolver.DirectoryPath(dataSetVersion, dataSetVersion.SemVersion()); + Directory.CreateDirectory(versionedFolder); + File.Create(Path.Combine(versionedFolder, "versioned.txt")); + + var draftFolder = pathResolver.DirectoryPath(dataSetVersion); + Directory.CreateDirectory(draftFolder); + File.Create(Path.Combine(draftFolder, "draft.txt")); + + var migration = TestApp.Services.GetRequiredService(); + + var exception = Assert.Throws(migration.Apply); + + Assert.Equal("The following DataSetVersions have both a versioned " + + "and a draft folder: " + dataSetVersion.Id, exception.Message); + + // Assert that the versioned folder still exists. + Assert.True(Directory.Exists(versionedFolder)); + Assert.True(File.Exists(Path.Combine(versionedFolder, "versioned.txt"))); + + // Assert that the draft folder still exists. + Assert.True(Directory.Exists(draftFolder)); + Assert.True(File.Exists(Path.Combine(draftFolder, "draft.txt"))); + } + + [Theory] + [MemberData(nameof(PublicDataSetVersionStatusAndTypes))] + public async Task Success_PublicVersionsNotMigrated(Tuple statusAndType) + { + var (status, type) = statusAndType; + + DataSetVersion dataSetVersion = DataFixture + .DefaultDataSetVersion() + .WithId(Guid.NewGuid()) + .WithDataSet(DataFixture + .DefaultDataSet() + .WithId(Guid.NewGuid())) + .WithStatus(status) + .WithVersionNumber( + major: type == DataSetVersionType.Major ? 2 : 1, + minor: type == DataSetVersionType.Major ? 0 : 1); + + await TestApp.AddTestData(context => context.DataSetVersions.Add(dataSetVersion)); + + var pathResolver = TestApp.Services.GetRequiredService(); + + var versionedFolder = pathResolver.DirectoryPath(dataSetVersion, dataSetVersion.SemVersion()); + Directory.CreateDirectory(versionedFolder); + File.Create(Path.Combine(versionedFolder, "test.txt")); + + var migration = TestApp.Services.GetRequiredService(); + + migration.Apply(); + + // Assert that the original folder has been left unaffected. + Assert.True(Directory.Exists(versionedFolder)); + Assert.True(File.Exists(Path.Combine(versionedFolder, "test.txt"))); + + // Assert that no draft folder has been created. + dataSetVersion.Status = DataSetVersionStatus.Draft; + var draftFolder = pathResolver.DirectoryPath(dataSetVersion); + Assert.False(Directory.Exists(draftFolder)); + } +} diff --git a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Api/Migrations/EES5660_MigrateDraftDataSetVersionFolderNames.cs b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Api/Migrations/EES5660_MigrateDraftDataSetVersionFolderNames.cs new file mode 100644 index 00000000000..92a5fe4845c --- /dev/null +++ b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Api/Migrations/EES5660_MigrateDraftDataSetVersionFolderNames.cs @@ -0,0 +1,48 @@ +using GovUk.Education.ExploreEducationStatistics.Common.Database; +using GovUk.Education.ExploreEducationStatistics.Common.Extensions; +using GovUk.Education.ExploreEducationStatistics.Public.Data.Model.Database; +using GovUk.Education.ExploreEducationStatistics.Public.Data.Model.Extensions; +using GovUk.Education.ExploreEducationStatistics.Public.Data.Services.Interfaces; +using Microsoft.EntityFrameworkCore; + +namespace GovUk.Education.ExploreEducationStatistics.Public.Data.Api.Migrations; + +public class EES5660_MigrateDraftDataSetVersionFolderNames( + PublicDataDbContext publicDataDbContext, + IDataSetVersionPathResolver pathResolver) : ICustomMigration +{ + public void Apply() + { + var draftDataSetVersions = publicDataDbContext + .DataSetVersions + .WherePrivateStatus() + .ToList(); + + var failureDataSetVersionIds = new List(); + + draftDataSetVersions.ForEach(dataSetVersion => + { + var versionedFolder = pathResolver.DirectoryPath(dataSetVersion, dataSetVersion.SemVersion()); + + if (Directory.Exists(versionedFolder)) + { + var newDraftFolder = pathResolver.DirectoryPath(dataSetVersion); + + if (Directory.Exists(newDraftFolder)) + { + failureDataSetVersionIds.Add(dataSetVersion.Id); + } + else + { + Directory.Move(versionedFolder, newDraftFolder); + } + } + }); + + if (failureDataSetVersionIds.Count > 0) + { + throw new Exception($"The following DataSetVersions have both a versioned " + + $"and a draft folder: {failureDataSetVersionIds.JoinToString(",")}"); + } + } +} diff --git a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Api/Startup.cs b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Api/Startup.cs index 5863265cb28..d4dfff4d17a 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Api/Startup.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Api/Startup.cs @@ -1,11 +1,17 @@ +using System.Diagnostics.CodeAnalysis; +using System.Net; +using System.Text.Json; +using System.Text.Json.Serialization; using AngleSharp.Io; using Dapper; using FluentValidation; using GovUk.Education.ExploreEducationStatistics.Common.Config; +using GovUk.Education.ExploreEducationStatistics.Common.Database; using GovUk.Education.ExploreEducationStatistics.Common.Extensions; using GovUk.Education.ExploreEducationStatistics.Common.ModelBinding; using GovUk.Education.ExploreEducationStatistics.Common.Rules; using GovUk.Education.ExploreEducationStatistics.Public.Data.Api.Extensions; +using GovUk.Education.ExploreEducationStatistics.Public.Data.Api.Migrations; using GovUk.Education.ExploreEducationStatistics.Public.Data.Api.Options; using GovUk.Education.ExploreEducationStatistics.Public.Data.Api.Repository; using GovUk.Education.ExploreEducationStatistics.Public.Data.Api.Repository.Interfaces; @@ -26,10 +32,6 @@ using Microsoft.AspNetCore.Rewrite; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Options; -using System.Diagnostics.CodeAnalysis; -using System.Net; -using System.Text.Json; -using System.Text.Json.Serialization; using RequestTimeoutOptions = GovUk.Education.ExploreEducationStatistics.Public.Data.Api.Options.RequestTimeoutOptions; namespace GovUk.Education.ExploreEducationStatistics.Public.Data.Api; @@ -191,7 +193,8 @@ public void ConfigureServices(IServiceCollection services) // Services services.AddFluentValidation(); - services.AddValidatorsFromAssembly(typeof(DataSetGetQueryLocations.Validator).Assembly); // Adds *all* validators from Public.Data.Requests + services.AddValidatorsFromAssembly(typeof(DataSetGetQueryLocations.Validator) + .Assembly); // Adds *all* validators from Public.Data.Requests services.AddFluentValidationRulesToSwagger(); services.AddHttpClient((provider, httpClient) => @@ -219,6 +222,8 @@ public void ConfigureServices(IServiceCollection services) services.AddScoped(); services.AddScoped(); services.AddScoped(); + + services.AddScoped(); } // This method gets called by the runtime. Use this method to configure the HTTP request pipeline. @@ -289,5 +294,21 @@ private static void UpdateDatabase(IApplicationBuilder app) context.Database.SetCommandTimeout(300); context.Database.Migrate(); + + ApplyCustomMigrations(app); + } + + private static void ApplyCustomMigrations(IApplicationBuilder app) + { + using var serviceScope = app.ApplicationServices + .GetRequiredService() + .CreateScope(); + + var migrations = serviceScope.ServiceProvider.GetServices(); + + foreach (var migration in migrations) + { + migration.Apply(); + } } } diff --git a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Model.Tests/Fixtures/DataSetGeneratorExtensions.cs b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Model.Tests/Fixtures/DataSetGeneratorExtensions.cs index 91cb1679c09..cb09d7e8c09 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Model.Tests/Fixtures/DataSetGeneratorExtensions.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Model.Tests/Fixtures/DataSetGeneratorExtensions.cs @@ -13,6 +13,9 @@ public static Generator WithDefaults(this Generator generator) public static Generator WithPublicationId(this Generator generator, Guid publicationId) => generator.ForInstance(s => s.SetPublicationId(publicationId)); + public static Generator WithId(this Generator generator, Guid id) + => generator.ForInstance(s => s.SetId(id)); + public static Generator WithTitle(this Generator generator, string title) => generator.ForInstance(s => s.SetTitle(title)); @@ -39,7 +42,7 @@ public static Generator WithWithdrawn(this Generator generator public static Generator WithSupersedingDataSet(this Generator generator, DataSet? dataSet) => generator.ForInstance(s => s.SetSupersedingDataSet(dataSet)); - + public static Generator WithLatestDraftVersion( this Generator generator, DataSetVersion? dataSetVersion) @@ -78,6 +81,11 @@ public static InstanceSetters SetDefaults(this InstanceSetters .SetDefault(ds => ds.PublicationId) .Set(ds => ds.Status, DataSetStatus.Draft); + public static InstanceSetters SetId( + this InstanceSetters instanceSetter, + Guid id) + => instanceSetter.Set(ds => ds.Id, id); + public static InstanceSetters SetTitle( this InstanceSetters instanceSetter, string title) @@ -162,7 +170,7 @@ public static InstanceSetters SetLatestLiveVersion( ds.LatestLiveVersionId = dsv?.Id; } ); - + public static InstanceSetters SetLatestDraftVersion( this InstanceSetters instanceSetter, DataSetVersion? dataSetVersion) diff --git a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Model.Tests/Fixtures/DataSetVersionGeneratorExtensions.cs b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Model.Tests/Fixtures/DataSetVersionGeneratorExtensions.cs index c681d890f4b..4ace459af53 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Model.Tests/Fixtures/DataSetVersionGeneratorExtensions.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Model.Tests/Fixtures/DataSetVersionGeneratorExtensions.cs @@ -32,6 +32,11 @@ public static Generator DefaultDataSetVersion( public static Generator WithDefaults(this Generator generator) => generator.ForInstance(s => s.SetDefaults()); + public static Generator WithId( + this Generator generator, + Guid id) + => generator.ForInstance(s => s.SetId(id)); + public static Generator WithDataSet( this Generator generator, DataSet dataSet) @@ -212,6 +217,11 @@ public static InstanceSetters SetDefaults(this InstanceSetters dsv.TotalResults, f => f.Random.Long(min: 10000, max: 10_000_000)) .Set(dsv => dsv.Status, DataSetVersionStatus.Draft); + public static InstanceSetters SetId( + this InstanceSetters instanceSetter, + Guid id) + => instanceSetter.Set(dsv => dsv.Id, id); + public static InstanceSetters SetDataSet( this InstanceSetters instanceSetter, DataSet dataSet) diff --git a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Model/Extensions/DataSetVersionAuthExtensions.cs b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Model/Extensions/DataSetVersionAuthExtensions.cs index 44d01584a51..3216eea2872 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Model/Extensions/DataSetVersionAuthExtensions.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Model/Extensions/DataSetVersionAuthExtensions.cs @@ -12,4 +12,13 @@ public static IQueryable WherePublicStatus(this IQueryable> IsPublicStatus() => dataSetVersion => DataSetVersionStatusConstants.PublicStatuses.Contains(dataSetVersion.Status); + + public static bool IsPrivateStatus(this DataSetVersion dataSetVersion) + => IsPrivateStatus().Compile()(dataSetVersion); + + public static IQueryable WherePrivateStatus(this IQueryable queryable) + => queryable.Where(IsPrivateStatus()); + + private static Expression> IsPrivateStatus() + => dataSetVersion => DataSetVersionStatusConstants.PrivateStatuses.Contains(dataSetVersion.Status); } diff --git a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Model/GovUk.Education.ExploreEducationStatistics.Public.Data.Model.csproj b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Model/GovUk.Education.ExploreEducationStatistics.Public.Data.Model.csproj index 56b96f01507..b7a2eee43b5 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Model/GovUk.Education.ExploreEducationStatistics.Public.Data.Model.csproj +++ b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Model/GovUk.Education.ExploreEducationStatistics.Public.Data.Model.csproj @@ -1,44 +1,40 @@ - - net8.0 - enable - enable - GovUk.Education.ExploreEducationStatistics.Public.Data.Model - true - - - - - - - - - - - - - - - all - runtime; build; native; contentfiles; analyzers; buildtransitive - - - - - - - - - - - - - - - - PreserveNewest - - + + net8.0 + enable + enable + GovUk.Education.ExploreEducationStatistics.Public.Data.Model + true + + + + + + + + + + + + + + + all + runtime; build; native; contentfiles; analyzers; buildtransitive + + + + + + + + + + + + PreserveNewest + + diff --git a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Processor.Tests/Functions/ProcessCompletionOfNextDataSetVersionOrchestrationTests.cs b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Processor.Tests/Functions/ProcessCompletionOfNextDataSetVersionOrchestrationTests.cs index c4fc07e53de..aeab7710c35 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Processor.Tests/Functions/ProcessCompletionOfNextDataSetVersionOrchestrationTests.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Processor.Tests/Functions/ProcessCompletionOfNextDataSetVersionOrchestrationTests.cs @@ -56,6 +56,11 @@ public async Task ActivityFunctionThrowsException_CallsHandleFailureActivity() var activitySequence = new MockSequence(); + var mockEntityFeature = new Mock(MockBehavior.Strict); + mockEntityFeature.SetupLockForActivity(ActivityNames.ImportMetadata); + mockOrchestrationContext.SetupGet(context => context.Entities) + .Returns(mockEntityFeature.Object); + mockOrchestrationContext .InSequence(activitySequence) .Setup(context => From b01b1f1792916655ca612d962422014178819ee1 Mon Sep 17 00:00:00 2001 From: Duncan Watson Date: Mon, 18 Nov 2024 14:59:32 +0000 Subject: [PATCH 3/6] EES-5660 - tidy-up prior to raising PR --- .../Database/ICustomMigration.cs | 7 ++ ...rateDraftDataSetVersionFolderNamesTests.cs | 6 +- ...0_MigrateDraftDataSetVersionFolderNames.cs | 9 ++- .../Startup.cs | 1 + .../DataSetVersionStatus.cs | 17 ----- .../DataSetVersionAuthExtensions.cs | 18 ++++- ...ucationStatistics.Public.Data.Model.csproj | 76 ++++++++++--------- .../DataSetVersionPathResolverTests.cs | 7 +- .../Interfaces/IDataSetVersionPathResolver.cs | 4 +- 9 files changed, 81 insertions(+), 64 deletions(-) diff --git a/src/GovUk.Education.ExploreEducationStatistics.Common/Database/ICustomMigration.cs b/src/GovUk.Education.ExploreEducationStatistics.Common/Database/ICustomMigration.cs index a899c9b3e16..6497e05e27d 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Common/Database/ICustomMigration.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Common/Database/ICustomMigration.cs @@ -1,5 +1,12 @@ namespace GovUk.Education.ExploreEducationStatistics.Common.Database; +/// +/// Custom migrations that run on deployment / startup. +/// These differ in use from EF database migrations in that they are not limited +/// to running against a single database / DbContext. These can be simple +/// standalone implementations or can use dependency injection if registered +/// with a DI container. +/// public interface ICustomMigration { void Apply(); diff --git a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Api.Tests/Migrations/EES5660_MigrateDraftDataSetVersionFolderNamesTests.cs b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Api.Tests/Migrations/EES5660_MigrateDraftDataSetVersionFolderNamesTests.cs index 450d88f0f11..36e45765070 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Api.Tests/Migrations/EES5660_MigrateDraftDataSetVersionFolderNamesTests.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Api.Tests/Migrations/EES5660_MigrateDraftDataSetVersionFolderNamesTests.cs @@ -6,22 +6,24 @@ using GovUk.Education.ExploreEducationStatistics.Public.Data.Api.Tests.TheoryData; using GovUk.Education.ExploreEducationStatistics.Public.Data.Model; using GovUk.Education.ExploreEducationStatistics.Public.Data.Model.Database; +using GovUk.Education.ExploreEducationStatistics.Public.Data.Model.Extensions; using GovUk.Education.ExploreEducationStatistics.Public.Data.Model.Tests.Fixtures; using GovUk.Education.ExploreEducationStatistics.Public.Data.Services.Interfaces; using Microsoft.Extensions.DependencyInjection; namespace GovUk.Education.ExploreEducationStatistics.Public.Data.Api.Tests.Migrations; +// TODO EES-5660 - remove this migration after it has been run against each Public API-enabled environment. public class EES5660_MigrateDraftDataSetVersionFolderNamesTests(TestApplicationFactory testApp) : IntegrationTestFixture(testApp) { public static readonly TheoryData> - PrivateDataSetVersionStatusAndTypes = new(DataSetVersionStatusConstants + PrivateDataSetVersionStatusAndTypes = new(DataSetVersionAuthExtensions .PrivateStatuses .Cartesian(EnumUtil.GetEnums())); public static readonly TheoryData> - PublicDataSetVersionStatusAndTypes = new(DataSetVersionStatusConstants + PublicDataSetVersionStatusAndTypes = new(DataSetVersionAuthExtensions .PublicStatuses .Cartesian(EnumUtil.GetEnums())); diff --git a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Api/Migrations/EES5660_MigrateDraftDataSetVersionFolderNames.cs b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Api/Migrations/EES5660_MigrateDraftDataSetVersionFolderNames.cs index 92a5fe4845c..324c0075b65 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Api/Migrations/EES5660_MigrateDraftDataSetVersionFolderNames.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Api/Migrations/EES5660_MigrateDraftDataSetVersionFolderNames.cs @@ -7,6 +7,7 @@ namespace GovUk.Education.ExploreEducationStatistics.Public.Data.Api.Migrations; +// TODO EES-5660 - remove this migration after it has been run against each Public API-enabled environment. public class EES5660_MigrateDraftDataSetVersionFolderNames( PublicDataDbContext publicDataDbContext, IDataSetVersionPathResolver pathResolver) : ICustomMigration @@ -18,7 +19,7 @@ public void Apply() .WherePrivateStatus() .ToList(); - var failureDataSetVersionIds = new List(); + var failedDataSetVersionIds = new List(); draftDataSetVersions.ForEach(dataSetVersion => { @@ -30,7 +31,7 @@ public void Apply() if (Directory.Exists(newDraftFolder)) { - failureDataSetVersionIds.Add(dataSetVersion.Id); + failedDataSetVersionIds.Add(dataSetVersion.Id); } else { @@ -39,10 +40,10 @@ public void Apply() } }); - if (failureDataSetVersionIds.Count > 0) + if (failedDataSetVersionIds.Count > 0) { throw new Exception($"The following DataSetVersions have both a versioned " + - $"and a draft folder: {failureDataSetVersionIds.JoinToString(",")}"); + $"and a draft folder: {failedDataSetVersionIds.JoinToString(",")}"); } } } diff --git a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Api/Startup.cs b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Api/Startup.cs index d4dfff4d17a..32a6574a0e6 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Api/Startup.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Api/Startup.cs @@ -223,6 +223,7 @@ public void ConfigureServices(IServiceCollection services) services.AddScoped(); services.AddScoped(); + // TODO EES-5660 - remove this migration after it has been run against each Public API-enabled environment. services.AddScoped(); } diff --git a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Model/DataSetVersionStatus.cs b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Model/DataSetVersionStatus.cs index 6124daf0eea..f62f4ac3578 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Model/DataSetVersionStatus.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Model/DataSetVersionStatus.cs @@ -1,5 +1,4 @@ using System.Text.Json.Serialization; -using GovUk.Education.ExploreEducationStatistics.Common.Utils; using Newtonsoft.Json.Converters; namespace GovUk.Education.ExploreEducationStatistics.Public.Data.Model; @@ -17,19 +16,3 @@ public enum DataSetVersionStatus Withdrawn, Cancelled, } - -public class DataSetVersionStatusConstants -{ - public static readonly IReadOnlyList PublicStatuses = new List( - [ - DataSetVersionStatus.Published, - DataSetVersionStatus.Withdrawn, - DataSetVersionStatus.Deprecated - ] - ); - - public static readonly IReadOnlyList PrivateStatuses = EnumUtil - .GetEnums() - .Except(PublicStatuses) - .ToList(); -} diff --git a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Model/Extensions/DataSetVersionAuthExtensions.cs b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Model/Extensions/DataSetVersionAuthExtensions.cs index 3216eea2872..093ad188419 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Model/Extensions/DataSetVersionAuthExtensions.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Model/Extensions/DataSetVersionAuthExtensions.cs @@ -1,9 +1,23 @@ using System.Linq.Expressions; +using GovUk.Education.ExploreEducationStatistics.Common.Utils; namespace GovUk.Education.ExploreEducationStatistics.Public.Data.Model.Extensions; public static class DataSetVersionAuthExtensions { + public static readonly IReadOnlyList PublicStatuses = new List( + [ + DataSetVersionStatus.Published, + DataSetVersionStatus.Withdrawn, + DataSetVersionStatus.Deprecated + ] + ); + + public static readonly IReadOnlyList PrivateStatuses = EnumUtil + .GetEnums() + .Except(PublicStatuses) + .ToList(); + public static bool IsPublicStatus(this DataSetVersion dataSetVersion) => IsPublicStatus().Compile()(dataSetVersion); @@ -11,7 +25,7 @@ public static IQueryable WherePublicStatus(this IQueryable queryable.Where(IsPublicStatus()); private static Expression> IsPublicStatus() - => dataSetVersion => DataSetVersionStatusConstants.PublicStatuses.Contains(dataSetVersion.Status); + => dataSetVersion => PublicStatuses.Contains(dataSetVersion.Status); public static bool IsPrivateStatus(this DataSetVersion dataSetVersion) => IsPrivateStatus().Compile()(dataSetVersion); @@ -20,5 +34,5 @@ public static IQueryable WherePrivateStatus(this IQueryable queryable.Where(IsPrivateStatus()); private static Expression> IsPrivateStatus() - => dataSetVersion => DataSetVersionStatusConstants.PrivateStatuses.Contains(dataSetVersion.Status); + => dataSetVersion => PrivateStatuses.Contains(dataSetVersion.Status); } diff --git a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Model/GovUk.Education.ExploreEducationStatistics.Public.Data.Model.csproj b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Model/GovUk.Education.ExploreEducationStatistics.Public.Data.Model.csproj index b7a2eee43b5..56b96f01507 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Model/GovUk.Education.ExploreEducationStatistics.Public.Data.Model.csproj +++ b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Model/GovUk.Education.ExploreEducationStatistics.Public.Data.Model.csproj @@ -1,40 +1,44 @@ - - net8.0 - enable - enable - GovUk.Education.ExploreEducationStatistics.Public.Data.Model - true - - - - - - - - - - - - - - - all - runtime; build; native; contentfiles; analyzers; buildtransitive - - - - - - - - - - - - PreserveNewest - - + + net8.0 + enable + enable + GovUk.Education.ExploreEducationStatistics.Public.Data.Model + true + + + + + + + + + + + + + + + all + runtime; build; native; contentfiles; analyzers; buildtransitive + + + + + + + + + + + + + + + + PreserveNewest + + diff --git a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Services.Tests/DataSetVersionPathResolverTests.cs b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Services.Tests/DataSetVersionPathResolverTests.cs index e988bf36e26..14fcd86c36a 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Services.Tests/DataSetVersionPathResolverTests.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Services.Tests/DataSetVersionPathResolverTests.cs @@ -3,6 +3,7 @@ using GovUk.Education.ExploreEducationStatistics.Common.Tests.Fixtures; using GovUk.Education.ExploreEducationStatistics.Common.Utils; using GovUk.Education.ExploreEducationStatistics.Public.Data.Model; +using GovUk.Education.ExploreEducationStatistics.Public.Data.Model.Extensions; using GovUk.Education.ExploreEducationStatistics.Public.Data.Model.Parquet.Tables; using GovUk.Education.ExploreEducationStatistics.Public.Data.Model.Tests.Fixtures; using GovUk.Education.ExploreEducationStatistics.Public.Data.Services.Interfaces; @@ -48,10 +49,10 @@ public class PathTests : DataSetVersionPathResolverTests public static readonly TheoryData GetEnvironmentNames = new(EnvironmentNames); public static readonly TheoryData> GetEnvironmentNamesAndPublicStatuses = - new(EnvironmentNames.Cartesian(DataSetVersionStatusConstants.PublicStatuses)); + new(EnvironmentNames.Cartesian(DataSetVersionAuthExtensions.PublicStatuses)); public static readonly TheoryData> GetEnvironmentNamesAndPrivateStatuses = - new(EnvironmentNames.Cartesian(DataSetVersionStatusConstants.PrivateStatuses)); + new(EnvironmentNames.Cartesian(DataSetVersionAuthExtensions.PrivateStatuses)); [Fact] public void DevelopmentEnv_ValidBasePath() @@ -178,6 +179,8 @@ public void ValidDirectoryPath_PrivateVersion(Tuple Path.Combine(BasePath(), DataSetFilenames.DataSetsDirectory); string DirectoryPath(DataSetVersion dataSetVersion); - + + // TODO EES-5660 - remove after draft DataSetVersion folders are migrated for each + // Public API-enabled environment. string DirectoryPath(DataSetVersion dataSetVersion, SemVersion versionNumber); string CsvDataPath(DataSetVersion dataSetVersion) From c5c4b05828fbd4d688b6e385aa31f8e59b8ef87c Mon Sep 17 00:00:00 2001 From: Duncan Watson Date: Tue, 19 Nov 2024 14:01:34 +0000 Subject: [PATCH 4/6] EES-5660 - making infrastructure changes to allow the Publisher to access the Public API fileshare --- infrastructure/templates/template.json | 28 ++++++++++++++++++- .../Startup.cs | 7 ++--- .../DataSetVersionPathResolver.cs | 2 +- .../appsettings.json | 3 ++ 4 files changed, 33 insertions(+), 7 deletions(-) diff --git a/infrastructure/templates/template.json b/infrastructure/templates/template.json index 2262edd6e97..cbb3706dde4 100644 --- a/infrastructure/templates/template.json +++ b/infrastructure/templates/template.json @@ -1197,7 +1197,10 @@ "[concat('Microsoft.Web/sites/', variables('importerAppName'))]", "[concat('Microsoft.Web/sites/', variables('contentAppName'))]", "[concat('Microsoft.Web/sites/', variables('dataAppName'))]" - ] + ], + "publicDataFileshareMountPath": "/data/public-api-data", + "publicDataFileshareName": "[concat(parameters('subscription'), '-ees-papi-fs-data')]", + "publicDataStorageAccountName": "[concat(parameters('subscription'), 'eespapisa')]" }, "resources": [ { @@ -3283,9 +3286,32 @@ "App:NotifierStorageConnectionString": "[concat('@Microsoft.KeyVault(SecretUri=', reference(variables('ees-storage-notifications')).secretUriWithVersion, ')')]", "App:PublicStorageConnectionString": "[concat('@Microsoft.KeyVault(SecretUri=', reference(variables('ees-storage-public')).secretUriWithVersion, ')')]", "App:PublisherStorageConnectionString": "[concat('@Microsoft.KeyVault(SecretUri=', reference(variables('ees-storage-publisher')).secretUriWithVersion, ')')]", + "DataFiles:BasePath": "[parameters('publicDataFileshareMountPath')]", "PublicDataDbExists": "[parameters('publicDataDbExists')]" } }, + { + "condition": "[parameters('publicDataDbExists')]", + "name": "[concat(variables('publisherAppName'), '/azurestorageaccounts')]", + "type": "Microsoft.Web/sites/config", + "location": "[resourceGroup().location]", + "apiVersion": "2019-08-01", + "dependsOn": [ + "[resourceId('Microsoft.Web/sites', variables('publisherAppName'))]", + "[variables('publicDataFileshareMountPath')]", + "[variables('publicDataFileshareName')]", + "[variables('publicDataStorageAccountName')]" + ], + "properties": { + "[variables('publicDataFileshareName')]": { + "type": "AzureFiles", + "accountName": "[variables('publicDataStorageAccountName')]", + "shareName": "[variables('publicDataFileshareName')]", + "mountPath": "[variables('publicDataFileshareMountPath')]", + "protocol": "Smb" + } + } + }, { "type": "Microsoft.Storage/storageAccounts", "name": "[variables('publisherStorageAccountName')]", diff --git a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Api/Startup.cs b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Api/Startup.cs index 32a6574a0e6..29eb5f30ded 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Api/Startup.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Api/Startup.cs @@ -224,7 +224,7 @@ public void ConfigureServices(IServiceCollection services) services.AddScoped(); // TODO EES-5660 - remove this migration after it has been run against each Public API-enabled environment. - services.AddScoped(); + services.AddScoped(); } // This method gets called by the runtime. Use this method to configure the HTTP request pipeline. @@ -307,9 +307,6 @@ private static void ApplyCustomMigrations(IApplicationBuilder app) var migrations = serviceScope.ServiceProvider.GetServices(); - foreach (var migration in migrations) - { - migration.Apply(); - } + migrations.ForEach(migration => migration.Apply()); } } diff --git a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Services/DataSetVersionPathResolver.cs b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Services/DataSetVersionPathResolver.cs index c8e16d37ce2..4f471d5ff42 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Services/DataSetVersionPathResolver.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Services/DataSetVersionPathResolver.cs @@ -39,7 +39,7 @@ public DataSetVersionPathResolver(IOptions options, IWebHostEn public string DirectoryPath(DataSetVersion dataSetVersion) { - var folderName = !dataSetVersion.IsPublicStatus() + var folderName = dataSetVersion.IsPrivateStatus() ? DraftVersionFolderName : $"v{dataSetVersion.SemVersion()}"; diff --git a/src/GovUk.Education.ExploreEducationStatistics.Publisher/appsettings.json b/src/GovUk.Education.ExploreEducationStatistics.Publisher/appsettings.json index df8caea97e1..89cd0d9e249 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Publisher/appsettings.json +++ b/src/GovUk.Education.ExploreEducationStatistics.Publisher/appsettings.json @@ -12,5 +12,8 @@ "PublicStorageConnectionString": "DefaultEndpointsProtocol=http;AccountName=devstoreaccount1;AccountKey=Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw==;BlobEndpoint=http://data-storage:10000/devstoreaccount1;TableEndpoint=http://data-storage:10002/devstoreaccount1;", "PublisherStorageConnectionString": "DefaultEndpointsProtocol=http;AccountName=devstoreaccount1;AccountKey=Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw==;BlobEndpoint=http://data-storage:10000/devstoreaccount1;QueueEndpoint=http://data-storage:10001/devstoreaccount1;TableEndpoint=http://data-storage:10002/devstoreaccount1;" }, + "DataFiles": { + "BasePath": "data/public-api-data" + }, "PublicDataDbExists": false } From fb0426b7907a3f08652e52d255eefca4369a5ce8 Mon Sep 17 00:00:00 2001 From: Duncan Watson Date: Tue, 19 Nov 2024 17:53:30 +0000 Subject: [PATCH 5/6] EES-5660 - supressing the running of custom migrations during startup if running integration tests, as otherwise they fire on every individual integration test method run. --- .../Startup.cs | 5 ++- ...rateDraftDataSetVersionFolderNamesTests.cs | 36 ++++++++++--------- .../Startup.cs | 9 +++-- 3 files changed, 30 insertions(+), 20 deletions(-) diff --git a/src/GovUk.Education.ExploreEducationStatistics.Admin/Startup.cs b/src/GovUk.Education.ExploreEducationStatistics.Admin/Startup.cs index cd6e7f0f16d..b04002a6bbd 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Admin/Startup.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Admin/Startup.cs @@ -783,7 +783,10 @@ private void UpdateDatabase(IApplicationBuilder app, IWebHostEnvironment env) context.Database.Migrate(); } - ApplyCustomMigrations(app); + if (!env.IsIntegrationTest()) + { + ApplyCustomMigrations(app); + } } if (env.IsDevelopment()) diff --git a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Api.Tests/Migrations/EES5660_MigrateDraftDataSetVersionFolderNamesTests.cs b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Api.Tests/Migrations/EES5660_MigrateDraftDataSetVersionFolderNamesTests.cs index 36e45765070..63c0806d5e6 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Api.Tests/Migrations/EES5660_MigrateDraftDataSetVersionFolderNamesTests.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Api.Tests/Migrations/EES5660_MigrateDraftDataSetVersionFolderNamesTests.cs @@ -1,3 +1,4 @@ +using GovUk.Education.ExploreEducationStatistics.Common.Database; using GovUk.Education.ExploreEducationStatistics.Common.Extensions; using GovUk.Education.ExploreEducationStatistics.Common.Tests.Extensions; using GovUk.Education.ExploreEducationStatistics.Common.Utils; @@ -37,7 +38,8 @@ public async Task Success_NonMigratedDraft(Tuple(); - - migration.Apply(); + GetMigration().Apply(); // Assert that the original folder that used the draft's version in its name no longer exists. Assert.False(Directory.Exists(versionedFolder)); @@ -63,7 +63,7 @@ public async Task Success_NonMigratedDraft(Tuple statusAndType) @@ -89,9 +89,7 @@ public async Task Success_AlreadyMigratedDraft(Tuple(); - - migration.Apply(); + GetMigration().Apply(); // Assert that the existing draft folder is left untouched. Assert.True(Directory.Exists(draftFolder)); @@ -106,7 +104,7 @@ public async Task Failure_DraftFolderAndVersionedFolderExist() .WithDataSet(DataFixture.DefaultDataSet()) .WithStatus(DataSetVersionStatus.Draft) .WithVersionNumber(major: 2, minor: 0); - + await TestApp.AddTestData(context => context.DataSetVersions.Add(dataSetVersion)); var pathResolver = TestApp.Services.GetRequiredService(); @@ -119,17 +117,15 @@ public async Task Failure_DraftFolderAndVersionedFolderExist() Directory.CreateDirectory(draftFolder); File.Create(Path.Combine(draftFolder, "draft.txt")); - var migration = TestApp.Services.GetRequiredService(); + var exception = Assert.Throws(GetMigration().Apply); - var exception = Assert.Throws(migration.Apply); - Assert.Equal("The following DataSetVersions have both a versioned " + "and a draft folder: " + dataSetVersion.Id, exception.Message); // Assert that the versioned folder still exists. Assert.True(Directory.Exists(versionedFolder)); Assert.True(File.Exists(Path.Combine(versionedFolder, "versioned.txt"))); - + // Assert that the draft folder still exists. Assert.True(Directory.Exists(draftFolder)); Assert.True(File.Exists(Path.Combine(draftFolder, "draft.txt"))); @@ -160,9 +156,7 @@ public async Task Success_PublicVersionsNotMigrated(Tuple(); - - migration.Apply(); + GetMigration().Apply(); // Assert that the original folder has been left unaffected. Assert.True(Directory.Exists(versionedFolder)); @@ -173,4 +167,14 @@ public async Task Success_PublicVersionsNotMigrated(Tuple() + .Where(migration => migration.GetType() == typeof(EES5660_MigrateDraftDataSetVersionFolderNames)) + .Cast() + .Single(); + } } diff --git a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Api/Startup.cs b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Api/Startup.cs index 29eb5f30ded..b09040d725e 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Api/Startup.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Api/Startup.cs @@ -230,7 +230,7 @@ public void ConfigureServices(IServiceCollection services) // This method gets called by the runtime. Use this method to configure the HTTP request pipeline. public void Configure(IApplicationBuilder app, IWebHostEnvironment env) { - UpdateDatabase(app); + UpdateDatabase(app, env); if (_miniProfilerOptions.Enabled) { @@ -285,7 +285,7 @@ public void Configure(IApplicationBuilder app, IWebHostEnvironment env) app.UseHealthChecks("/health"); } - private static void UpdateDatabase(IApplicationBuilder app) + private static void UpdateDatabase(IApplicationBuilder app, IHostEnvironment env) { using var serviceScope = app.ApplicationServices .GetRequiredService() @@ -296,7 +296,10 @@ private static void UpdateDatabase(IApplicationBuilder app) context.Database.SetCommandTimeout(300); context.Database.Migrate(); - ApplyCustomMigrations(app); + if (!env.IsIntegrationTest()) + { + ApplyCustomMigrations(app); + } } private static void ApplyCustomMigrations(IApplicationBuilder app) From 963d6e558165bf96191a37a87cb020b849a11c1a Mon Sep 17 00:00:00 2001 From: Duncan Watson Date: Mon, 2 Dec 2024 21:03:52 +0000 Subject: [PATCH 6/6] EES-5660 - responding to PR comments. Replacing explicit Tuple instances with named tuples. --- .../Extensions/EnumerableExtensionsTests.cs | 28 +++++++++---------- .../Extensions/EnumerableExtensions.cs | 10 +++---- ...rateDraftDataSetVersionFolderNamesTests.cs | 10 +++---- .../DataSetVersionPathResolverTests.cs | 8 +++--- 4 files changed, 28 insertions(+), 28 deletions(-) diff --git a/src/GovUk.Education.ExploreEducationStatistics.Common.Tests/Extensions/EnumerableExtensionsTests.cs b/src/GovUk.Education.ExploreEducationStatistics.Common.Tests/Extensions/EnumerableExtensionsTests.cs index 0ccf2a08f0a..d70fa90b556 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Common.Tests/Extensions/EnumerableExtensionsTests.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Common.Tests/Extensions/EnumerableExtensionsTests.cs @@ -474,12 +474,12 @@ public void TwoLists_Cartesian() List list1 = [1, 2]; List list2 = ["3", "4"]; - List> expected = + List<(int, string)> expected = [ - new Tuple(1, "3"), - new Tuple(1, "4"), - new Tuple(2, "3"), - new Tuple(2, "4") + new(1, "3"), + new(1, "4"), + new(2, "3"), + new(2, "4") ]; var actual = list1.Cartesian(list2); @@ -507,16 +507,16 @@ public void ThreeLists_Cartesian() List list2 = ["3", "4"]; List list3 = ['5', '6']; - List> expected = + List<(int, string, char)> expected = [ - new Tuple(1, "3", '5'), - new Tuple(1, "3", '6'), - new Tuple(1, "4", '5'), - new Tuple(1, "4", '6'), - new Tuple(2, "3", '5'), - new Tuple(2, "3", '6'), - new Tuple(2, "4", '5'), - new Tuple(2, "4", '6'), + new(1, "3", '5'), + new(1, "3", '6'), + new(1, "4", '5'), + new(1, "4", '6'), + new(2, "3", '5'), + new(2, "3", '6'), + new(2, "4", '5'), + new(2, "4", '6'), ]; var actual = list1.Cartesian(list2, list3); diff --git a/src/GovUk.Education.ExploreEducationStatistics.Common/Extensions/EnumerableExtensions.cs b/src/GovUk.Education.ExploreEducationStatistics.Common/Extensions/EnumerableExtensions.cs index 130bb844d3b..5ff99b81aff 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Common/Extensions/EnumerableExtensions.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Common/Extensions/EnumerableExtensions.cs @@ -320,18 +320,18 @@ public static IOrderedEnumerable NaturalThenBy( return source.ThenBy(keySelector, comparison.WithNaturalSort()); } - public static List> Cartesian( + public static List<(T1, T2)> Cartesian( this IEnumerable list1, IEnumerable? list2) { return list2 == null ? [] : list1 - .Join(list2, _ => true, _ => true, (t1, t2) => new Tuple(t1, t2)) + .Join(list2, _ => true, _ => true, (t1, t2) => (t1, t2)) .ToList(); } - public static List> Cartesian( + public static List<(T1, T2, T3)> Cartesian( this IEnumerable list1, IEnumerable? list2, IEnumerable? list3) @@ -339,9 +339,9 @@ public static List> Cartesian( return list2 == null || list3 == null ? [] : list1 - .Join(list2, _ => true, _ => true, (t1, t2) => new Tuple(t1, t2)) + .Join(list2, _ => true, _ => true, (t1, t2) => (t1, t2)) .Join(list3, _ => true, _ => true, - (tuple, t3) => new Tuple(tuple.Item1, tuple.Item2, t3)) + (tuple, t3) => (tuple.t1, tuple.t2, t3)) .ToList(); } } diff --git a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Api.Tests/Migrations/EES5660_MigrateDraftDataSetVersionFolderNamesTests.cs b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Api.Tests/Migrations/EES5660_MigrateDraftDataSetVersionFolderNamesTests.cs index 63c0806d5e6..5a2f0463548 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Api.Tests/Migrations/EES5660_MigrateDraftDataSetVersionFolderNamesTests.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Api.Tests/Migrations/EES5660_MigrateDraftDataSetVersionFolderNamesTests.cs @@ -18,19 +18,19 @@ namespace GovUk.Education.ExploreEducationStatistics.Public.Data.Api.Tests.Migra public class EES5660_MigrateDraftDataSetVersionFolderNamesTests(TestApplicationFactory testApp) : IntegrationTestFixture(testApp) { - public static readonly TheoryData> + public static readonly TheoryData<(DataSetVersionStatus, DataSetVersionType)> PrivateDataSetVersionStatusAndTypes = new(DataSetVersionAuthExtensions .PrivateStatuses .Cartesian(EnumUtil.GetEnums())); - public static readonly TheoryData> + public static readonly TheoryData<(DataSetVersionStatus, DataSetVersionType)> PublicDataSetVersionStatusAndTypes = new(DataSetVersionAuthExtensions .PublicStatuses .Cartesian(EnumUtil.GetEnums())); [Theory] [MemberData(nameof(PrivateDataSetVersionStatusAndTypes))] - public async Task Success_NonMigratedDraft(Tuple statusAndType) + public async Task Success_NonMigratedDraft((DataSetVersionStatus, DataSetVersionType) statusAndType) { var (status, type) = statusAndType; @@ -66,7 +66,7 @@ public async Task Success_NonMigratedDraft(Tuple statusAndType) + public async Task Success_AlreadyMigratedDraft((DataSetVersionStatus, DataSetVersionType) statusAndType) { var (status, type) = statusAndType; @@ -133,7 +133,7 @@ public async Task Failure_DraftFolderAndVersionedFolderExist() [Theory] [MemberData(nameof(PublicDataSetVersionStatusAndTypes))] - public async Task Success_PublicVersionsNotMigrated(Tuple statusAndType) + public async Task Success_PublicVersionsNotMigrated((DataSetVersionStatus, DataSetVersionType) statusAndType) { var (status, type) = statusAndType; diff --git a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Services.Tests/DataSetVersionPathResolverTests.cs b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Services.Tests/DataSetVersionPathResolverTests.cs index 14fcd86c36a..aad25a6c7df 100644 --- a/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Services.Tests/DataSetVersionPathResolverTests.cs +++ b/src/GovUk.Education.ExploreEducationStatistics.Public.Data.Services.Tests/DataSetVersionPathResolverTests.cs @@ -48,10 +48,10 @@ public class PathTests : DataSetVersionPathResolverTests public static readonly TheoryData GetEnvironmentNames = new(EnvironmentNames); - public static readonly TheoryData> GetEnvironmentNamesAndPublicStatuses = + public static readonly TheoryData<(string, DataSetVersionStatus)> GetEnvironmentNamesAndPublicStatuses = new(EnvironmentNames.Cartesian(DataSetVersionAuthExtensions.PublicStatuses)); - public static readonly TheoryData> GetEnvironmentNamesAndPrivateStatuses = + public static readonly TheoryData<(string, DataSetVersionStatus)> GetEnvironmentNamesAndPrivateStatuses = new(EnvironmentNames.Cartesian(DataSetVersionAuthExtensions.PrivateStatuses)); [Fact] @@ -125,7 +125,7 @@ public void ProductionEnv_ValidBasePath() [Theory] [MemberData(nameof(GetEnvironmentNamesAndPublicStatuses))] - public void ValidDirectoryPath_PublicVersion(Tuple environmentNameAndStatus) + public void ValidDirectoryPath_PublicVersion((string, DataSetVersionStatus) environmentNameAndStatus) { var (environmentName, status) = environmentNameAndStatus; @@ -153,7 +153,7 @@ public void ValidDirectoryPath_PublicVersion(Tuple [Theory] [MemberData(nameof(GetEnvironmentNamesAndPrivateStatuses))] - public void ValidDirectoryPath_PrivateVersion(Tuple environmentNameAndStatus) + public void ValidDirectoryPath_PrivateVersion((string, DataSetVersionStatus) environmentNameAndStatus) { var (environmentName, status) = environmentNameAndStatus;