Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent creation of duplicate genres via the UI #59

Merged
merged 1 commit into from
Aug 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/MusicCatalogue.Api/Controllers/AlbumsController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ public async Task<ActionResult<IEnumerable<Album>>> GetAlbumsAsync([FromBody] Al
public async Task<ActionResult<Album>> AddAlbumAsync([FromBody] Album template)
{
// Make sure the "other" Genre exists as a fallback for album updates where no genre is given
var otherGenre = _factory.Genres.AddAsync(OtherGenre);
var otherGenre = _factory.Genres.AddAsync(OtherGenre, false);

// Add the album
var album = await _factory.Albums.AddAsync(
Expand Down Expand Up @@ -120,7 +120,7 @@ public async Task<ActionResult<Album>> AddAlbumAsync([FromBody] Album template)
public async Task<ActionResult<Album>> UpdateAlbumAsync([FromBody] Album template)
{
// Make sure the "other" Genre exists as a fallback for album updates where no genre is given
var otherGenre = _factory.Genres.AddAsync(OtherGenre);
var otherGenre = _factory.Genres.AddAsync(OtherGenre, false);

// Attempt the update
var album = await _factory.Albums.UpdateAsync(
Expand Down
2 changes: 1 addition & 1 deletion src/MusicCatalogue.Api/Controllers/GenresController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public async Task<ActionResult<Genre>> GetGenreByIdAsync(int id)
[Route("")]
public async Task<ActionResult<Genre>> AddGenreAsync([FromBody] Genre template)
{
var genre = await _factory.Genres.AddAsync(template.Name);
var genre = await _factory.Genres.AddAsync(template.Name, true);
return genre;
}

Expand Down
6 changes: 3 additions & 3 deletions src/MusicCatalogue.Api/MusicCatalogue.Api.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@

<PropertyGroup>
<TargetFramework>net8.0</TargetFramework>
<ReleaseVersion>1.33.0.0</ReleaseVersion>
<FileVersion>1.33.0.0</FileVersion>
<ProductVersion>1.33.0</ProductVersion>
<ReleaseVersion>1.34.0.0</ReleaseVersion>
<FileVersion>1.34.0.0</FileVersion>
<ProductVersion>1.34.0</ProductVersion>
<Nullable>enable</Nullable>
<ImplicitUsings>enable</ImplicitUsings>
</PropertyGroup>
Expand Down
4 changes: 2 additions & 2 deletions src/MusicCatalogue.Data/MusicCatalogue.Data.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<ImplicitUsings>enable</ImplicitUsings>
<Nullable>enable</Nullable>
<PackageId>MusicCatalogue.Data</PackageId>
<PackageVersion>1.24.0.0</PackageVersion>
<PackageVersion>1.25.0.0</PackageVersion>
<Authors>Dave Walker</Authors>
<Copyright>Copyright (c) Dave Walker 2023</Copyright>
<Owners>Dave Walker</Owners>
Expand All @@ -17,7 +17,7 @@
<PackageProjectUrl>https://github.com/davewalker5/MusicCatalogue</PackageProjectUrl>
<PackageLicenseExpression>MIT</PackageLicenseExpression>
<PackageRequireLicenseAcceptance>false</PackageRequireLicenseAcceptance>
<ReleaseVersion>1.24.0.0</ReleaseVersion>
<ReleaseVersion>1.25.0.0</ReleaseVersion>
</PropertyGroup>

<ItemGroup>
Expand Down
21 changes: 21 additions & 0 deletions src/MusicCatalogue.Entities/Exceptions/DuplicateGenreException.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
using System.Diagnostics.CodeAnalysis;

namespace MusicCatalogue.Entities.Exceptions
{
[Serializable]
[ExcludeFromCodeCoverage]
public class DuplicateGenreException : Exception

Check warning on line 7 in src/MusicCatalogue.Entities/Exceptions/DuplicateGenreException.cs

View workflow job for this annotation

GitHub Actions / build

Update this implementation of 'ISerializable' to conform to the recommended serialization pattern. Add a 'protected' constructor 'DuplicateGenreException(SerializationInfo, StreamingContext)'. (https://rules.sonarsource.com/csharp/RSPEC-3925)
{
public DuplicateGenreException()
{
}

public DuplicateGenreException(string message) : base(message)
{
}

public DuplicateGenreException(string message, Exception inner) : base(message, inner)
{
}
}
}
21 changes: 21 additions & 0 deletions src/MusicCatalogue.Entities/Exceptions/GenreNotFoundException.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
using System.Diagnostics.CodeAnalysis;

namespace MusicCatalogue.Entities.Exceptions
{
[Serializable]
[ExcludeFromCodeCoverage]
public class GenreNotFoundException : Exception

Check warning on line 7 in src/MusicCatalogue.Entities/Exceptions/GenreNotFoundException.cs

View workflow job for this annotation

GitHub Actions / build

Update this implementation of 'ISerializable' to conform to the recommended serialization pattern. Add a 'protected' constructor 'GenreNotFoundException(SerializationInfo, StreamingContext)'. (https://rules.sonarsource.com/csharp/RSPEC-3925)
{
public GenreNotFoundException()
{
}

public GenreNotFoundException(string message) : base(message)
{
}

public GenreNotFoundException(string message, Exception inner) : base(message, inner)
{
}
}
}
2 changes: 1 addition & 1 deletion src/MusicCatalogue.Entities/Interfaces/IGenreManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ namespace MusicCatalogue.Entities.Interfaces
{
public interface IGenreManager
{
Task<Genre> AddAsync(string name);
Task<Genre> AddAsync(string name, bool raiseErrorIfExists);
Task<Genre> GetAsync(Expression<Func<Genre, bool>> predicate);
Task<List<Genre>> ListAsync(Expression<Func<Genre, bool>> predicate);
Task<Genre?> UpdateAsync(int genreId, string name);
Expand Down
4 changes: 2 additions & 2 deletions src/MusicCatalogue.Entities/MusicCatalogue.Entities.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<ImplicitUsings>enable</ImplicitUsings>
<Nullable>enable</Nullable>
<PackageId>MusicCatalogue.Entities</PackageId>
<PackageVersion>1.24.0.0</PackageVersion>
<PackageVersion>1.25.0.0</PackageVersion>
<Authors>Dave Walker</Authors>
<Copyright>Copyright (c) Dave Walker 2023</Copyright>
<Owners>Dave Walker</Owners>
Expand All @@ -17,7 +17,7 @@
<PackageProjectUrl>https://github.com/davewalker5/MusicCatalogue</PackageProjectUrl>
<PackageLicenseExpression>MIT</PackageLicenseExpression>
<PackageRequireLicenseAcceptance>false</PackageRequireLicenseAcceptance>
<ReleaseVersion>1.24.0.0</ReleaseVersion>
<ReleaseVersion>1.25.0.0</ReleaseVersion>
</PropertyGroup>

<ItemGroup>
Expand Down
2 changes: 1 addition & 1 deletion src/MusicCatalogue.Logic/Collection/AlbumLookupManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ private async Task<Album> StoreAlbumLocally(string artistName, Album template, b
// Save the artist details, first. As with all the database calls in this method, the
// logic to prevent duplication of artistsand genres is in the management class
var artist = await _factory.Artists.AddAsync(artistName);
var genre = await _factory.Genres.AddAsync(template.Genre!.Name);
var genre = await _factory.Genres.AddAsync(template.Genre!.Name, false);

// Save the album details
var album = await _factory.Albums.AddAsync(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public async Task Import(string file)
// Inflate the CSV record to a track and save the artist and genre
var track = FlattenedTrack.FromCsv(fields!);
var artist = await _factory.Artists.AddAsync(track.ArtistName);
var genre = await _factory.Genres.AddAsync(track.Genre!);
var genre = await _factory.Genres.AddAsync(track.Genre!, false);

// Add the retailer
int? retailerId = null;
Expand Down
13 changes: 12 additions & 1 deletion src/MusicCatalogue.Logic/Database/GenreManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,9 @@ public async Task<List<Genre>> ListAsync(Expression<Func<Genre, bool>> predicate
/// Add a genre, if it doesn't already exist
/// </summary>
/// <param name="name"></param>
/// <param name="raiseErrorIfExists"></param>
/// <returns></returns>
public async Task<Genre> AddAsync(string name)
public async Task<Genre> AddAsync(string name, bool raiseErrorIfExists)
{
var clean = StringCleaner.Clean(name)!;
var genre = await GetAsync(a => a.Name == clean);
Expand All @@ -56,6 +57,11 @@ public async Task<Genre> AddAsync(string name)
await Context.Genres.AddAsync(genre);
await Context.SaveChangesAsync();
}
else if (raiseErrorIfExists)
{
var message = $"Cannot create duplicate genre {genre.Name}";
throw new DuplicateGenreException(message);
}

return genre;
}
Expand All @@ -74,6 +80,11 @@ public async Task<Genre> AddAsync(string name)
genre.Name = StringCleaner.Clean(name)!;
await Context.SaveChangesAsync();
}
else
{
var message = $"Genre '{name}' not found";
throw new GenreNotFoundException(message);
}
return genre;
}

Expand Down
4 changes: 2 additions & 2 deletions src/MusicCatalogue.Logic/MusicCatalogue.Logic.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<ImplicitUsings>enable</ImplicitUsings>
<Nullable>enable</Nullable>
<PackageId>MusicCatalogue.Logic</PackageId>
<PackageVersion>1.24.0.0</PackageVersion>
<PackageVersion>1.25.0.0</PackageVersion>
<Authors>Dave Walker</Authors>
<Copyright>Copyright (c) Dave Walker 2023</Copyright>
<Owners>Dave Walker</Owners>
Expand All @@ -17,7 +17,7 @@
<PackageProjectUrl>https://github.com/davewalker5/MusicCatalogue</PackageProjectUrl>
<PackageLicenseExpression>MIT</PackageLicenseExpression>
<PackageRequireLicenseAcceptance>false</PackageRequireLicenseAcceptance>
<ReleaseVersion>1.24.0.0</ReleaseVersion>
<ReleaseVersion>1.25.0.0</ReleaseVersion>
</PropertyGroup>

<ItemGroup>
Expand Down
2 changes: 1 addition & 1 deletion src/MusicCatalogue.Tests/AlbumLookupManagerTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ public void ArtistInDbButAlbumNotInDbTest()
public void ArtistAndAlbumInDbTest()
{
var artistId = Task.Run(() => _factory!.Artists.AddAsync(ArtistName)).Result.Id;
var genreId = Task.Run(() => _factory!.Genres.AddAsync(Genre)).Result.Id;
var genreId = Task.Run(() => _factory!.Genres.AddAsync(Genre, false)).Result.Id;
Task.Run(() => _factory!.Albums.AddAsync(artistId, genreId, AlbumTitle, Released, CoverUrl, false, null, null, null)).Wait();
var album = Task.Run(() => _manager!.LookupAlbum(ArtistName, AlbumTitle, false)).Result;

Expand Down
2 changes: 1 addition & 1 deletion src/MusicCatalogue.Tests/AlbumManagerTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public void TestInitialize()

// Add the test entities to the database
_retailerId = Task.Run(() => _factory.Retailers.AddAsync(RetailerName)).Result.Id;
_genreId = Task.Run(() => _factory.Genres.AddAsync(Genre)).Result.Id;
_genreId = Task.Run(() => _factory.Genres.AddAsync(Genre, false)).Result.Id;
_artistId = Task.Run(() => _factory.Artists.AddAsync(ArtistName)).Result.Id;
_albumId = Task.Run(() => _factory.Albums.AddAsync(_artistId, _genreId, AlbumTitle, Released, CoverUrl, false, null, null, null)).Result.Id;
Task.Run(() => _factory.Tracks.AddAsync(_albumId, TrackTitle, TrackNumber, TrackDuration)).Wait();
Expand Down
4 changes: 2 additions & 2 deletions src/MusicCatalogue.Tests/AlbumSearchManagerTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ public void TestInitialize()
_factory = new MusicCatalogueFactory(context);

// Add the genres
_jazzGenreId = Task.Run(() => _factory.Genres.AddAsync("Jazz")).Result.Id;
_popGenreId = Task.Run(() => _factory.Genres.AddAsync("Pop")).Result.Id;
_jazzGenreId = Task.Run(() => _factory.Genres.AddAsync("Jazz", false)).Result.Id;
_popGenreId = Task.Run(() => _factory.Genres.AddAsync("Pop", false)).Result.Id;

// Add the artists
_jazzArtistId = Task.Run(() => _factory.Artists.AddAsync("Diana Krall")).Result.Id;
Expand Down
2 changes: 1 addition & 1 deletion src/MusicCatalogue.Tests/ArtistManagerTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ public async Task DeleteTest()
[ExpectedException(typeof(ArtistInUseException))]
public async Task CannotDeleteWithAlbumTest()
{
var genreId = Task.Run(() => _factory!.Genres.AddAsync(Genre)).Result.Id;
var genreId = Task.Run(() => _factory!.Genres.AddAsync(Genre, false)).Result.Id;
Task.Run(() => _factory!.Albums.AddAsync(_artistId, genreId, AlbumTitle, Released, null, false, null, null, null)).Wait();
await _factory!.Artists.DeleteAsync(_artistId);
}
Expand Down
4 changes: 2 additions & 2 deletions src/MusicCatalogue.Tests/ArtistSearchManagerTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ public void TestInitialize()
_factory = new MusicCatalogueFactory(context);

// Add the genres
_jazzGenreId = Task.Run(() => _factory.Genres.AddAsync("Jazz")).Result.Id;
_popGenreId = Task.Run(() => _factory.Genres.AddAsync("Pop")).Result.Id;
_jazzGenreId = Task.Run(() => _factory.Genres.AddAsync("Jazz", false)).Result.Id;
_popGenreId = Task.Run(() => _factory.Genres.AddAsync("Pop", false)).Result.Id;

// Add the artists
var jazzArtistId = Task.Run(() => _factory.Artists.AddAsync(JazzArtistName)).Result.Id;
Expand Down
2 changes: 1 addition & 1 deletion src/MusicCatalogue.Tests/CatalogueDataExchangeTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public void Initialise()
// Add an artist, an album and one track
var retailer = Task.Run(() => _factory.Retailers.AddAsync(RetailerName)).Result;
var artist = Task.Run(() => _factory.Artists.AddAsync(ArtistName)).Result;
var genre = Task.Run(() => _factory.Genres.AddAsync(Genre)).Result;
var genre = Task.Run(() => _factory.Genres.AddAsync(Genre, false)).Result;
var album = Task.Run(() => _factory.Albums.AddAsync(artist.Id, genre.Id, AlbumName, Released, CoverUrl, false, Purchased, Price, retailer.Id)).Result;
Task.Run(() => _factory.Tracks.AddAsync(album.Id, TrackName, TrackNumber, Duration)).Wait();
}
Expand Down
18 changes: 13 additions & 5 deletions src/MusicCatalogue.Tests/GenreManagerTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,21 @@ public void TestInitialize()
{
MusicCatalogueDbContext context = MusicCatalogueDbContextFactory.CreateInMemoryDbContext();
_factory = new MusicCatalogueFactory(context);
_genreId = Task.Run(() => _factory.Genres.AddAsync(Genre)).Result.Id;
_genreId = Task.Run(() => _factory.Genres.AddAsync(Genre, false)).Result.Id;
}

[TestMethod]
public async Task AddDuplicateTest()
[ExpectedException(typeof(DuplicateGenreException))]
public async Task AddDuplicateWithErrorTest()
{
await _factory!.Genres.AddAsync(Genre);
await _factory!.Genres.AddAsync(Genre, true);
}

[TestMethod]

public async Task AddDuplicateWithNoErrorTest()
{
await _factory!.Genres.AddAsync(Genre, false);
var genres = await _factory!.Genres.ListAsync(x => true);
Assert.AreEqual(1, genres.Count);
}
Expand Down Expand Up @@ -82,10 +90,10 @@ public async Task UpdateTest()
}

[TestMethod]
[ExpectedException(typeof(GenreNotFoundException))]
public async Task UpdateMissingTest()
{
var genre = await _factory!.Genres.UpdateAsync(-1, UpdatedGenre);
Assert.IsNull(genre);
_ = await _factory!.Genres.UpdateAsync(-1, UpdatedGenre);
}

[TestMethod]
Expand Down
4 changes: 2 additions & 2 deletions src/MusicCatalogue.Tests/GenreSearchManagerTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ public void TestInitialize()
_factory = new MusicCatalogueFactory(context);

// Add the genres
_jazzGenreId = Task.Run(() => _factory.Genres.AddAsync("Jazz")).Result.Id;
_popGenreId = Task.Run(() => _factory.Genres.AddAsync("Pop")).Result.Id;
_jazzGenreId = Task.Run(() => _factory.Genres.AddAsync("Jazz", false)).Result.Id;
_popGenreId = Task.Run(() => _factory.Genres.AddAsync("Pop", false)).Result.Id;

// Add the artists
_jazzArtistId = Task.Run(() => _factory.Artists.AddAsync("Diana Krall")).Result.Id;
Expand Down
2 changes: 1 addition & 1 deletion src/MusicCatalogue.Tests/RetailerManagerTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ public async Task DeleteInUseTest()
{
// Add an album that uses the retailer
var artist = await _factory!.Artists.AddAsync(ArtistName);
var genre = await _factory.Genres.AddAsync(Genre);
var genre = await _factory.Genres.AddAsync(Genre, false);
await _factory.Albums.AddAsync(artist.Id, genre.Id, AlbumTitle, Released, CoverUrl, false, null, null, _retailerId);

// Now try to delete the retailer - this should raise an exception
Expand Down
2 changes: 1 addition & 1 deletion src/MusicCatalogue.Tests/TrackManagerTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public void TestInitialize()

// Set up an artist and album for the tracks to belong to
_artistId = Task.Run(() => factory.Artists.AddAsync(ArtistName)).Result.Id;
var genreId = Task.Run(() => factory.Genres.AddAsync(Genre)).Result.Id;
var genreId = Task.Run(() => factory.Genres.AddAsync(Genre, false)).Result.Id;
_albumId = Task.Run(() => factory.Albums.AddAsync(_artistId, genreId, AlbumTitle, Released, CoverUrl, false, null, null, null)).Result.Id;

// Create a track manager and add a test track
Expand Down
Loading