From 8fb7aae8437649e3349808dbc3ad3f1ec5feb45f Mon Sep 17 00:00:00 2001 From: mikestock-nimble Date: Mon, 15 Jan 2024 14:43:45 +0000 Subject: [PATCH 1/2] 152950: fixed the educational performance tests added separate migrations per context, mstr and edperf added checks for absence data in the educational performance tests --- .../EdperfContext.cs | 11 ++++ .../EdPerf/20240115130213_Initial.Designer.cs | 59 +++++++++++++++++++ .../EdPerf/20240115130213_Initial.cs | 41 +++++++++++++ .../EdPerf/EdperfContextModelSnapshot.cs | 57 ++++++++++++++++++ .../20240115130158_Initial.Designer.cs} | 4 +- .../20240115130158_Initial.cs} | 2 +- .../{ => Mstr}/MstrContextModelSnapshot.cs | 2 +- README.md | 12 +++- TramsDataApi.Test/DbFixture.cs | 1 + .../EducationPerformanceIntegrationTests.cs | 55 +++++++++-------- 10 files changed, 214 insertions(+), 30 deletions(-) create mode 100644 Dfe.Academies.Api.Infrastructure/Migrations/EdPerf/20240115130213_Initial.Designer.cs create mode 100644 Dfe.Academies.Api.Infrastructure/Migrations/EdPerf/20240115130213_Initial.cs create mode 100644 Dfe.Academies.Api.Infrastructure/Migrations/EdPerf/EdperfContextModelSnapshot.cs rename Dfe.Academies.Api.Infrastructure/Migrations/{20231218104547_Initial.Designer.cs => Mstr/20240115130158_Initial.Designer.cs} (99%) rename Dfe.Academies.Api.Infrastructure/Migrations/{20231218104547_Initial.cs => Mstr/20240115130158_Initial.cs} (99%) rename Dfe.Academies.Api.Infrastructure/Migrations/{ => Mstr}/MstrContextModelSnapshot.cs (99%) diff --git a/Dfe.Academies.Api.Infrastructure/EdperfContext.cs b/Dfe.Academies.Api.Infrastructure/EdperfContext.cs index a7a56343c..1f9dc5dd8 100644 --- a/Dfe.Academies.Api.Infrastructure/EdperfContext.cs +++ b/Dfe.Academies.Api.Infrastructure/EdperfContext.cs @@ -26,6 +26,17 @@ public EdperfContext(DbContextOptions options) : base(options) } + protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder) + { + if (!optionsBuilder.IsConfigured) + { + // Ed perf is split across two contexts, Legacy and Edperf + // We need to use the docker image, because this context only has part of the schema + // When the migration is generated it needs to only include the differences between the docker image and this context + optionsBuilder.UseSqlServer("Server=localhost,1433;Database=sip;Integrated Security=true;TrustServerCertificate=True"); + } + } + public DbSet SchoolAbsences { get; set; } = null!; protected override void OnModelCreating(ModelBuilder modelBuilder) diff --git a/Dfe.Academies.Api.Infrastructure/Migrations/EdPerf/20240115130213_Initial.Designer.cs b/Dfe.Academies.Api.Infrastructure/Migrations/EdPerf/20240115130213_Initial.Designer.cs new file mode 100644 index 000000000..580714866 --- /dev/null +++ b/Dfe.Academies.Api.Infrastructure/Migrations/EdPerf/20240115130213_Initial.Designer.cs @@ -0,0 +1,59 @@ +// +using System; +using Dfe.Academies.Infrastructure; +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Infrastructure; +using Microsoft.EntityFrameworkCore.Metadata; +using Microsoft.EntityFrameworkCore.Migrations; +using Microsoft.EntityFrameworkCore.Storage.ValueConversion; + +#nullable disable + +namespace Dfe.Academies.Infrastructure.Migrations.EdPerf +{ + [DbContext(typeof(EdperfContext))] + [Migration("20240115130213_Initial")] + partial class Initial + { + protected override void BuildTargetModel(ModelBuilder modelBuilder) + { +#pragma warning disable 612, 618 + modelBuilder + .HasAnnotation("ProductVersion", "6.0.25") + .HasAnnotation("Relational:MaxIdentifierLength", 128); + + SqlServerModelBuilderExtensions.UseIdentityColumns(modelBuilder, 1L, 1); + + modelBuilder.Entity("Dfe.Academies.Domain.EducationalPerformance.SchoolAbsence", b => + { + b.Property("DownloadYear") + .HasColumnType("nvarchar(450)"); + + b.Property("URN") + .HasColumnType("nvarchar(450)"); + + b.Property("LA") + .HasColumnType("nvarchar(450)"); + + b.Property("ESTAB") + .HasColumnType("nvarchar(450)"); + + b.Property("DateAndTimeImported") + .HasColumnType("datetimeoffset"); + + b.Property("PERCTOT") + .IsRequired() + .HasColumnType("nvarchar(max)"); + + b.Property("PPERSABS10") + .IsRequired() + .HasColumnType("nvarchar(max)"); + + b.HasKey("DownloadYear", "URN", "LA", "ESTAB"); + + b.ToTable("download_PUPILABSENCE_england_ALL", "edperf"); + }); +#pragma warning restore 612, 618 + } + } +} diff --git a/Dfe.Academies.Api.Infrastructure/Migrations/EdPerf/20240115130213_Initial.cs b/Dfe.Academies.Api.Infrastructure/Migrations/EdPerf/20240115130213_Initial.cs new file mode 100644 index 000000000..196b99ab0 --- /dev/null +++ b/Dfe.Academies.Api.Infrastructure/Migrations/EdPerf/20240115130213_Initial.cs @@ -0,0 +1,41 @@ +using System; +using Microsoft.EntityFrameworkCore.Migrations; + +#nullable disable + +namespace Dfe.Academies.Infrastructure.Migrations.EdPerf +{ + public partial class Initial : Migration + { + protected override void Up(MigrationBuilder migrationBuilder) + { + migrationBuilder.EnsureSchema( + name: "edperf"); + + migrationBuilder.CreateTable( + name: "download_PUPILABSENCE_england_ALL", + schema: "edperf", + columns: table => new + { + DownloadYear = table.Column(type: "nvarchar(450)", nullable: false), + LA = table.Column(type: "nvarchar(450)", nullable: false), + ESTAB = table.Column(type: "nvarchar(450)", nullable: false), + URN = table.Column(type: "nvarchar(450)", nullable: false), + DateAndTimeImported = table.Column(type: "datetimeoffset", nullable: false), + PERCTOT = table.Column(type: "nvarchar(max)", nullable: false), + PPERSABS10 = table.Column(type: "nvarchar(max)", nullable: false) + }, + constraints: table => + { + table.PrimaryKey("PK_download_PUPILABSENCE_england_ALL", x => new { x.DownloadYear, x.URN, x.LA, x.ESTAB }); + }); + } + + protected override void Down(MigrationBuilder migrationBuilder) + { + migrationBuilder.DropTable( + name: "download_PUPILABSENCE_england_ALL", + schema: "edperf"); + } + } +} diff --git a/Dfe.Academies.Api.Infrastructure/Migrations/EdPerf/EdperfContextModelSnapshot.cs b/Dfe.Academies.Api.Infrastructure/Migrations/EdPerf/EdperfContextModelSnapshot.cs new file mode 100644 index 000000000..73ff41de1 --- /dev/null +++ b/Dfe.Academies.Api.Infrastructure/Migrations/EdPerf/EdperfContextModelSnapshot.cs @@ -0,0 +1,57 @@ +// +using System; +using Dfe.Academies.Infrastructure; +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Infrastructure; +using Microsoft.EntityFrameworkCore.Metadata; +using Microsoft.EntityFrameworkCore.Storage.ValueConversion; + +#nullable disable + +namespace Dfe.Academies.Infrastructure.Migrations.EdPerf +{ + [DbContext(typeof(EdperfContext))] + partial class EdperfContextModelSnapshot : ModelSnapshot + { + protected override void BuildModel(ModelBuilder modelBuilder) + { +#pragma warning disable 612, 618 + modelBuilder + .HasAnnotation("ProductVersion", "6.0.25") + .HasAnnotation("Relational:MaxIdentifierLength", 128); + + SqlServerModelBuilderExtensions.UseIdentityColumns(modelBuilder, 1L, 1); + + modelBuilder.Entity("Dfe.Academies.Domain.EducationalPerformance.SchoolAbsence", b => + { + b.Property("DownloadYear") + .HasColumnType("nvarchar(450)"); + + b.Property("URN") + .HasColumnType("nvarchar(450)"); + + b.Property("LA") + .HasColumnType("nvarchar(450)"); + + b.Property("ESTAB") + .HasColumnType("nvarchar(450)"); + + b.Property("DateAndTimeImported") + .HasColumnType("datetimeoffset"); + + b.Property("PERCTOT") + .IsRequired() + .HasColumnType("nvarchar(max)"); + + b.Property("PPERSABS10") + .IsRequired() + .HasColumnType("nvarchar(max)"); + + b.HasKey("DownloadYear", "URN", "LA", "ESTAB"); + + b.ToTable("download_PUPILABSENCE_england_ALL", "edperf"); + }); +#pragma warning restore 612, 618 + } + } +} diff --git a/Dfe.Academies.Api.Infrastructure/Migrations/20231218104547_Initial.Designer.cs b/Dfe.Academies.Api.Infrastructure/Migrations/Mstr/20240115130158_Initial.Designer.cs similarity index 99% rename from Dfe.Academies.Api.Infrastructure/Migrations/20231218104547_Initial.Designer.cs rename to Dfe.Academies.Api.Infrastructure/Migrations/Mstr/20240115130158_Initial.Designer.cs index 056b335d1..9a50e4e26 100644 --- a/Dfe.Academies.Api.Infrastructure/Migrations/20231218104547_Initial.Designer.cs +++ b/Dfe.Academies.Api.Infrastructure/Migrations/Mstr/20240115130158_Initial.Designer.cs @@ -9,10 +9,10 @@ #nullable disable -namespace Dfe.Academies.Infrastructure.Migrations +namespace Dfe.Academies.Infrastructure.Migrations.Mstr { [DbContext(typeof(MstrContext))] - [Migration("20231218104547_Initial")] + [Migration("20240115130158_Initial")] partial class Initial { protected override void BuildTargetModel(ModelBuilder modelBuilder) diff --git a/Dfe.Academies.Api.Infrastructure/Migrations/20231218104547_Initial.cs b/Dfe.Academies.Api.Infrastructure/Migrations/Mstr/20240115130158_Initial.cs similarity index 99% rename from Dfe.Academies.Api.Infrastructure/Migrations/20231218104547_Initial.cs rename to Dfe.Academies.Api.Infrastructure/Migrations/Mstr/20240115130158_Initial.cs index 3b5572bfd..2ddb6ca1a 100644 --- a/Dfe.Academies.Api.Infrastructure/Migrations/20231218104547_Initial.cs +++ b/Dfe.Academies.Api.Infrastructure/Migrations/Mstr/20240115130158_Initial.cs @@ -3,7 +3,7 @@ #nullable disable -namespace Dfe.Academies.Infrastructure.Migrations +namespace Dfe.Academies.Infrastructure.Migrations.Mstr { public partial class Initial : Migration { diff --git a/Dfe.Academies.Api.Infrastructure/Migrations/MstrContextModelSnapshot.cs b/Dfe.Academies.Api.Infrastructure/Migrations/Mstr/MstrContextModelSnapshot.cs similarity index 99% rename from Dfe.Academies.Api.Infrastructure/Migrations/MstrContextModelSnapshot.cs rename to Dfe.Academies.Api.Infrastructure/Migrations/Mstr/MstrContextModelSnapshot.cs index 6fd6a0097..07d70bea3 100644 --- a/Dfe.Academies.Api.Infrastructure/Migrations/MstrContextModelSnapshot.cs +++ b/Dfe.Academies.Api.Infrastructure/Migrations/Mstr/MstrContextModelSnapshot.cs @@ -8,7 +8,7 @@ #nullable disable -namespace Dfe.Academies.Infrastructure.Migrations +namespace Dfe.Academies.Infrastructure.Migrations.Mstr { [DbContext(typeof(MstrContext))] partial class MstrContextModelSnapshot : ModelSnapshot diff --git a/README.md b/README.md index 5b56e7d35..d69553a3c 100644 --- a/README.md +++ b/README.md @@ -23,7 +23,7 @@ In the latest v4 endpoints, the decision was made to have a migration that enabl V2 and V3 tests will run against the docker image (legacy) -V4 uses the migrations specified in `trams-data-api\Dfe.Academies.Api.Infrastructure/Migrations` +V4 uses the migrations specified in `trams-data-api\Dfe.Academies.Api.Infrastructure/Migrations/` Now we can easily rebuild the environment on each test run and also build an environment for testing, if we need to @@ -33,12 +33,20 @@ Likely we might encounter some small differences very occasionally, but the bene ### EntityFramework and Migrations -We currently have two database contexts defined: `LegacyTramsDbContext` and `TramsDbContext`. Both database contexts manage the same database, but are used to manage different sets of tables. +We currently have a number of database contexts: + +#### trams-data-api `LegacyTramsDbContext` is used to manage our models for tables which exist in the `sip` database and we have no control over - we treat these tables as read-only and don't commit migrations for them. If you do generate migrations for this context, it should not be commited to the repository. `TramsDbContext` is the db context for models that we _do_ control, and we can generate migrations for. These migrations will be applied to the database in `dev`, `pre-prod`, and `prod`, and so should be commited to the repository when changes are made to models. +#### trams-data-api\Dfe.Academies.Api.Infrastructure + +`MstrContext` is used to create a database for all tables in the mstr schema, we use this for local development and automated testing, please note this is for local use only + +`EdperfContext` is used to create any updates to the edperf schema that do not exist in the docker image. Right now the majority of the schema exists in the docker image and only the latest changes do not. Since this context does not contain all the tables, it can only be used to update the docker image. In future if we have a clean break like the mstr schema, we could rebuild all objects in the schema. + ### Generating Migrations To generate migrations for `TramsDbContext`, use the following command: diff --git a/TramsDataApi.Test/DbFixture.cs b/TramsDataApi.Test/DbFixture.cs index 7c3878747..b8ca2e83e 100644 --- a/TramsDataApi.Test/DbFixture.cs +++ b/TramsDataApi.Test/DbFixture.cs @@ -49,6 +49,7 @@ public DbFixture() _tramsDbContext.Database.Migrate(); _edperfContext.Database.EnsureCreated(); + _edperfContext.Database.Migrate(); _legacyTransaction = _legacyTramsDbContext.Database.BeginTransaction(); _tramsTransaction = _tramsDbContext.Database.BeginTransaction(); diff --git a/TramsDataApi.Test/Integration/EducationPerformanceIntegrationTests.cs b/TramsDataApi.Test/Integration/EducationPerformanceIntegrationTests.cs index 81a2ef748..7dcce9a15 100644 --- a/TramsDataApi.Test/Integration/EducationPerformanceIntegrationTests.cs +++ b/TramsDataApi.Test/Integration/EducationPerformanceIntegrationTests.cs @@ -4,6 +4,10 @@ using System.Net; using System.Net.Http; using System.Threading.Tasks; +using AutoFixture; +using Dfe.Academies.Contracts.V1.EducationalPerformance; +using Dfe.Academies.Domain.EducationalPerformance; +using Dfe.Academies.Infrastructure; using FizzWare.NBuilder; using FluentAssertions; using Microsoft.Extensions.DependencyInjection; @@ -16,11 +20,13 @@ namespace TramsDataApi.Test.Integration { [Collection("Database")] - public class EducationPerformanceIntegrationTests : IClassFixture, IDisposable + public class EducationPerformanceIntegrationTests : IClassFixture { private readonly HttpClient _client; private readonly LegacyTramsDbContext _legacyDbContext; + private readonly EdperfContext _edperfContext; private readonly RandomGenerator _randomGenerator; + private static Fixture _autoFixture = new(); public EducationPerformanceIntegrationTests(TramsDataApiFactory fixture) @@ -29,12 +35,20 @@ public EducationPerformanceIntegrationTests(TramsDataApiFactory fixture) _client.DefaultRequestHeaders.Add("ApiKey", "testing-api-key"); _legacyDbContext = fixture.Services.GetRequiredService(); _randomGenerator = new RandomGenerator(); + + _legacyDbContext.Account.RemoveRange(_legacyDbContext.Account); + _legacyDbContext.SipPhonics.RemoveRange(_legacyDbContext.SipPhonics); + _legacyDbContext.SipEducationalperformancedata.RemoveRange(_legacyDbContext.SipEducationalperformancedata); + _legacyDbContext.GlobalOptionSetMetadata.RemoveRange(_legacyDbContext.GlobalOptionSetMetadata); + _legacyDbContext.SaveChanges(); + + _edperfContext = fixture.Services.GetRequiredService(); + _edperfContext.SchoolAbsences.RemoveRange(_edperfContext.SchoolAbsences); } [Fact] public async Task CanGetEducationPerformanceDataWithKeyStage1and2Data() - { - + { var accountGuid = Guid.NewGuid(); var accountUrn = "147259"; @@ -205,6 +219,12 @@ public async Task CanGetEducationPerformanceDataWithKeyStage1and2Data() }); _legacyDbContext.SaveChanges(); + var absenceData = _autoFixture.CreateMany().ToList(); + absenceData.ForEach(a => a.URN = accountUrn); + _edperfContext.SchoolAbsences.AddRange(_autoFixture.Create()); + _edperfContext.AddRange(absenceData); + _edperfContext.SaveChanges(); + var expectedKs1Response = phonics.Select(ph => new KeyStage1PerformanceResponse { Year = ph.SipYear, @@ -307,7 +327,12 @@ public async Task CanGetEducationPerformanceDataWithKeyStage1and2Data() KeyStage2 = expectedKs2Response, KeyStage4 = expectedKs4Response, KeyStage5 = expectedKs5Response, - AbsenceData = new List() + AbsenceData = absenceData.Select(a => new SchoolAbsenceDataDto + { + Year = a.DownloadYear, + PersistentAbsence = a.PPERSABS10, + OverallAbsence = a.PERCTOT + }).ToList() }; var response = await _client.GetAsync($"/educationPerformance/{accountUrn}"); @@ -316,11 +341,6 @@ public async Task CanGetEducationPerformanceDataWithKeyStage1and2Data() response.StatusCode.Should().Be(HttpStatusCode.OK); result.Should().BeEquivalentTo(expected); - - _legacyDbContext.Account.RemoveRange(_legacyDbContext.Account); - _legacyDbContext.SipPhonics.RemoveRange(_legacyDbContext.SipPhonics); - _legacyDbContext.SipEducationalperformancedata.RemoveRange(_legacyDbContext.SipEducationalperformancedata); - _legacyDbContext.GlobalOptionSetMetadata.RemoveRange(_legacyDbContext.GlobalOptionSetMetadata); } [Fact] @@ -328,7 +348,7 @@ public async Task CanGetEducationPerformanceDataWithKeyStage4And5Data() { var accountGuid = Guid.NewGuid(); - var accountUrn = "147259"; + var accountUrn = "126559"; var account = Builder.CreateNew() .With(a => a.Name = "Gillshill Primary School") @@ -760,7 +780,7 @@ public async Task CanGetEducationPerformanceDataWithKeyStage4And5Data() KeyStage2 = new List { expectedKeyStage2Response }, KeyStage4 = new List { expectedKeyStage4Response }, KeyStage5 = new List { expectedKeyStage5Response }, - AbsenceData = new List() + AbsenceData = new List() }; var response = await _client.GetAsync($"/educationPerformance/{accountUrn}"); @@ -769,19 +789,6 @@ public async Task CanGetEducationPerformanceDataWithKeyStage4And5Data() response.StatusCode.Should().Be(HttpStatusCode.OK); result.Should().BeEquivalentTo(expected); - - _legacyDbContext.Account.RemoveRange(_legacyDbContext.Account); - _legacyDbContext.SipEducationalperformancedata.RemoveRange(_legacyDbContext.SipEducationalperformancedata); - _legacyDbContext.GlobalOptionSetMetadata.RemoveRange(_legacyDbContext.GlobalOptionSetMetadata); - } - - - public void Dispose() - { - _legacyDbContext.Account.RemoveRange(_legacyDbContext.Account); - _legacyDbContext.SipPhonics.RemoveRange(_legacyDbContext.SipPhonics); - _legacyDbContext.SipEducationalperformancedata.RemoveRange(_legacyDbContext.SipEducationalperformancedata); - _legacyDbContext.SaveChanges(); } } } \ No newline at end of file From 0b2c06089603dd0170a781d2e56afb1cce47cbca Mon Sep 17 00:00:00 2001 From: mikestock-nimble Date: Mon, 15 Jan 2024 14:49:05 +0000 Subject: [PATCH 2/2] updated the readme to improve readability --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index d69553a3c..7ddc7de65 100644 --- a/README.md +++ b/README.md @@ -41,9 +41,9 @@ We currently have a number of database contexts: `TramsDbContext` is the db context for models that we _do_ control, and we can generate migrations for. These migrations will be applied to the database in `dev`, `pre-prod`, and `prod`, and so should be commited to the repository when changes are made to models. -#### trams-data-api\Dfe.Academies.Api.Infrastructure +#### trams-data-api\Dfe.Academies.Api.Infrastructure (local development and testing only) -`MstrContext` is used to create a database for all tables in the mstr schema, we use this for local development and automated testing, please note this is for local use only +`MstrContext` is used to create a database for all tables in the mstr schema `EdperfContext` is used to create any updates to the edperf schema that do not exist in the docker image. Right now the majority of the schema exists in the docker image and only the latest changes do not. Since this context does not contain all the tables, it can only be used to update the docker image. In future if we have a clean break like the mstr schema, we could rebuild all objects in the schema.