From 70d4a87cd56ab6fe049f5cba02d38545e7288620 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Thu, 11 Jun 2026 14:48:52 -0400 Subject: [PATCH] fix: include Release nav on all TrackRepository query paths; add unique constraint on release(title, artist) --- .../Configurations/ReleaseConfiguration.cs | 7 + ...32_AddReleaseUniqueTitleArtist.Designer.cs | 178 ++++++++++++++++++ ...60611184732_AddReleaseUniqueTitleArtist.cs | 28 +++ .../DeepDrftContextModelSnapshot.cs | 4 + DeepDrftData/Repositories/TrackRepository.cs | 10 + DeepDrftData/TrackManager.cs | 26 ++- 6 files changed, 246 insertions(+), 7 deletions(-) create mode 100644 DeepDrftData/Migrations/20260611184732_AddReleaseUniqueTitleArtist.Designer.cs create mode 100644 DeepDrftData/Migrations/20260611184732_AddReleaseUniqueTitleArtist.cs diff --git a/DeepDrftData/Data/Configurations/ReleaseConfiguration.cs b/DeepDrftData/Data/Configurations/ReleaseConfiguration.cs index cd49e94..d0260aa 100644 --- a/DeepDrftData/Data/Configurations/ReleaseConfiguration.cs +++ b/DeepDrftData/Data/Configurations/ReleaseConfiguration.cs @@ -56,5 +56,12 @@ public class ReleaseConfiguration : BaseEntityConfiguration // calls HasIndex(e => e.IsDeleted); this adds HasDatabaseName so EF always uses // "IX_release_is_deleted" regardless of auto-naming conventions. builder.HasIndex(e => e.IsDeleted).HasDatabaseName("IX_release_is_deleted"); + + // Unique constraint on the natural key (title + artist). Prevents duplicate release rows + // from concurrent uploads of the same album. The FindOrCreateRelease path catches the + // resulting ClassifiedDbException (UniqueViolation) and re-queries for the winning row. + builder.HasIndex(e => new { e.Title, e.Artist }) + .IsUnique() + .HasDatabaseName("IX_release_title_artist"); } } diff --git a/DeepDrftData/Migrations/20260611184732_AddReleaseUniqueTitleArtist.Designer.cs b/DeepDrftData/Migrations/20260611184732_AddReleaseUniqueTitleArtist.Designer.cs new file mode 100644 index 0000000..1bb5211 --- /dev/null +++ b/DeepDrftData/Migrations/20260611184732_AddReleaseUniqueTitleArtist.Designer.cs @@ -0,0 +1,178 @@ +// +using System; +using DeepDrftData.Data; +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Infrastructure; +using Microsoft.EntityFrameworkCore.Migrations; +using Microsoft.EntityFrameworkCore.Storage.ValueConversion; +using Npgsql.EntityFrameworkCore.PostgreSQL.Metadata; + +#nullable disable + +namespace DeepDrftData.Migrations +{ + [DbContext(typeof(DeepDrftContext))] + [Migration("20260611184732_AddReleaseUniqueTitleArtist")] + partial class AddReleaseUniqueTitleArtist + { + /// + protected override void BuildTargetModel(ModelBuilder modelBuilder) + { +#pragma warning disable 612, 618 + modelBuilder + .HasAnnotation("ProductVersion", "10.0.7") + .HasAnnotation("Relational:MaxIdentifierLength", 63); + + NpgsqlModelBuilderExtensions.UseIdentityByDefaultColumns(modelBuilder); + + modelBuilder.Entity("DeepDrftModels.Entities.ReleaseEntity", b => + { + b.Property("Id") + .ValueGeneratedOnAdd() + .HasColumnType("bigint") + .HasColumnName("id"); + + NpgsqlPropertyBuilderExtensions.UseIdentityByDefaultColumn(b.Property("Id")); + + b.Property("Artist") + .IsRequired() + .HasMaxLength(200) + .HasColumnType("character varying(200)") + .HasColumnName("artist"); + + b.Property("CreatedAt") + .HasColumnType("timestamp with time zone") + .HasColumnName("created_at"); + + b.Property("CreatedByUserId") + .HasColumnType("bigint") + .HasColumnName("created_by_user_id"); + + b.Property("Genre") + .HasMaxLength(100) + .HasColumnType("character varying(100)") + .HasColumnName("genre"); + + b.Property("ImagePath") + .HasMaxLength(500) + .HasColumnType("character varying(500)") + .HasColumnName("image_path"); + + b.Property("IsDeleted") + .ValueGeneratedOnAdd() + .HasColumnType("boolean") + .HasDefaultValue(false) + .HasColumnName("is_deleted"); + + b.Property("ReleaseDate") + .HasColumnType("date") + .HasColumnName("release_date"); + + b.Property("ReleaseType") + .IsRequired() + .ValueGeneratedOnAdd() + .HasMaxLength(20) + .HasColumnType("character varying(20)") + .HasDefaultValue("Single") + .HasColumnName("release_type"); + + b.Property("Title") + .IsRequired() + .HasMaxLength(200) + .HasColumnType("character varying(200)") + .HasColumnName("title"); + + b.Property("UpdatedAt") + .HasColumnType("timestamp with time zone") + .HasColumnName("updated_at"); + + b.HasKey("Id"); + + b.HasIndex("IsDeleted") + .HasDatabaseName("IX_release_is_deleted"); + + b.HasIndex("Title", "Artist") + .IsUnique() + .HasDatabaseName("IX_release_title_artist"); + + b.ToTable("release", (string)null); + }); + + modelBuilder.Entity("DeepDrftModels.Entities.TrackEntity", b => + { + b.Property("Id") + .ValueGeneratedOnAdd() + .HasColumnType("bigint") + .HasColumnName("id"); + + NpgsqlPropertyBuilderExtensions.UseIdentityByDefaultColumn(b.Property("Id")); + + b.Property("CreatedAt") + .HasColumnType("timestamp with time zone") + .HasColumnName("created_at"); + + b.Property("EntryKey") + .IsRequired() + .HasMaxLength(100) + .HasColumnType("character varying(100)") + .HasColumnName("entry_key"); + + b.Property("IsDeleted") + .ValueGeneratedOnAdd() + .HasColumnType("boolean") + .HasDefaultValue(false) + .HasColumnName("is_deleted"); + + b.Property("OriginalFileName") + .HasMaxLength(500) + .HasColumnType("character varying(500)") + .HasColumnName("original_file_name"); + + b.Property("ReleaseId") + .HasColumnType("bigint") + .HasColumnName("release_id"); + + b.Property("TrackName") + .IsRequired() + .HasMaxLength(200) + .HasColumnType("character varying(200)") + .HasColumnName("track_name"); + + b.Property("TrackNumber") + .ValueGeneratedOnAdd() + .HasColumnType("integer") + .HasDefaultValue(1) + .HasColumnName("track_number"); + + b.Property("UpdatedAt") + .HasColumnType("timestamp with time zone") + .HasColumnName("updated_at"); + + b.HasKey("Id"); + + b.HasIndex("IsDeleted") + .HasDatabaseName("IX_track_is_deleted"); + + b.HasIndex("ReleaseId"); + + b.ToTable("track", (string)null); + }); + + modelBuilder.Entity("DeepDrftModels.Entities.TrackEntity", b => + { + b.HasOne("DeepDrftModels.Entities.ReleaseEntity", "Release") + .WithMany("Tracks") + .HasForeignKey("ReleaseId") + .OnDelete(DeleteBehavior.SetNull); + + b.Navigation("Release"); + }); + + modelBuilder.Entity("DeepDrftModels.Entities.ReleaseEntity", b => + { + b.Navigation("Tracks"); + }); +#pragma warning restore 612, 618 + } + } +} diff --git a/DeepDrftData/Migrations/20260611184732_AddReleaseUniqueTitleArtist.cs b/DeepDrftData/Migrations/20260611184732_AddReleaseUniqueTitleArtist.cs new file mode 100644 index 0000000..3c05849 --- /dev/null +++ b/DeepDrftData/Migrations/20260611184732_AddReleaseUniqueTitleArtist.cs @@ -0,0 +1,28 @@ +using Microsoft.EntityFrameworkCore.Migrations; + +#nullable disable + +namespace DeepDrftData.Migrations +{ + /// + public partial class AddReleaseUniqueTitleArtist : Migration + { + /// + protected override void Up(MigrationBuilder migrationBuilder) + { + migrationBuilder.CreateIndex( + name: "IX_release_title_artist", + table: "release", + columns: new[] { "title", "artist" }, + unique: true); + } + + /// + protected override void Down(MigrationBuilder migrationBuilder) + { + migrationBuilder.DropIndex( + name: "IX_release_title_artist", + table: "release"); + } + } +} diff --git a/DeepDrftData/Migrations/DeepDrftContextModelSnapshot.cs b/DeepDrftData/Migrations/DeepDrftContextModelSnapshot.cs index 84aa92a..1a1c42e 100644 --- a/DeepDrftData/Migrations/DeepDrftContextModelSnapshot.cs +++ b/DeepDrftData/Migrations/DeepDrftContextModelSnapshot.cs @@ -88,6 +88,10 @@ namespace DeepDrftData.Migrations b.HasIndex("IsDeleted") .HasDatabaseName("IX_release_is_deleted"); + b.HasIndex("Title", "Artist") + .IsUnique() + .HasDatabaseName("IX_release_title_artist"); + b.ToTable("release", (string)null); }); diff --git a/DeepDrftData/Repositories/TrackRepository.cs b/DeepDrftData/Repositories/TrackRepository.cs index b10c602..b39d33a 100644 --- a/DeepDrftData/Repositories/TrackRepository.cs +++ b/DeepDrftData/Repositories/TrackRepository.cs @@ -26,6 +26,16 @@ public class TrackRepository : Repository _context = context; } + // Override base GetByIdAsync to include the Release navigation. Without this, the base + // Query has no .Include, so Release is null on every entity (no lazy-loading proxies). + public override async Task GetByIdAsync(long id) + => await Query.Include(t => t.Release).FirstOrDefaultAsync(e => e.Id == id); + + // Override base GetAllAsync for the same reason — include Release so callers (e.g. + // TrackManager.GetAll) receive fully-populated entities without a separate query. + public override async Task> GetAllAsync() + => await Query.Include(t => t.Release).ToListAsync(); + // Lookup by vault entry key. The base Repository<> only exposes id-based queries, so this // uses Query (soft-delete filtered) rather than the raw DbSet. Includes Release so the // converter can project the release-cardinal fields. diff --git a/DeepDrftData/TrackManager.cs b/DeepDrftData/TrackManager.cs index e92e37e..043d3c8 100644 --- a/DeepDrftData/TrackManager.cs +++ b/DeepDrftData/TrackManager.cs @@ -1,3 +1,4 @@ +using Data.Errors; using Data.Managers; using DeepDrftData.Repositories; using DeepDrftModels.DTOs; @@ -121,13 +122,12 @@ public class TrackManager } }; - // An all-null filter must produce identical results to no filter, so collapse it to - // null and take the unfiltered base path (preserves backward compatibility). + // Always route through GetPagedFilteredAsync — it handles a null filter by skipping + // all Where predicates, and it always includes Release. This removes the base-class + // GetPagedAsync path, which has no .Include and would return entities with null Release. var effectiveFilter = filter is null || filter.IsEmpty ? null : filter; - var page = effectiveFilter is null - ? await Repository.GetPagedAsync(parameters) - : await Repository.GetPagedFilteredAsync(parameters, effectiveFilter, cancellationToken); + var page = await Repository.GetPagedFilteredAsync(parameters, effectiveFilter, cancellationToken); var dtoPage = PagedResult.From(page, page.Items.Select(TrackConverter.Convert)); return ResultContainer>.CreatePassResult(dtoPage); @@ -178,8 +178,20 @@ public class TrackManager entity.Title = title; entity.Artist = artist; - var added = await Repository.AddReleaseAsync(entity, cancellationToken); - return ResultContainer.CreatePassResult(TrackConverter.Convert(added)); + try + { + var added = await Repository.AddReleaseAsync(entity, cancellationToken); + return ResultContainer.CreatePassResult(TrackConverter.Convert(added)); + } + catch (ClassifiedDbException ex) when (ex.Error.Category == DbErrorCategory.UniqueViolation) + { + // Concurrent upload inserted the same (title, artist) between our read and write. + // Re-query and return the winning row. Should not return null here since the + // constraint just fired, but re-throw if it does so the caller sees an error. + var race = await Repository.GetReleaseByTitleAndArtistAsync(title, artist, cancellationToken); + if (race is null) throw; + return ResultContainer.CreatePassResult(TrackConverter.Convert(race)); + } } catch (Exception e) {