Fix query and API bugs: 403 for non-owners, page bounds, lifecycle filters, cover fallback, race-safe inserts, async album delete
This commit is contained in:
@@ -3,6 +3,7 @@ using Mediator;
|
||||
using Microsoft.EntityFrameworkCore;
|
||||
using Microsoft.Extensions.Logging;
|
||||
using Musify.Application.Contracts;
|
||||
using Musify.Application.Shared;
|
||||
using Musify.Domain.Entities;
|
||||
|
||||
namespace Musify.Application.Albums;
|
||||
@@ -21,48 +22,30 @@ public class AddTrackToAlbumCommandHandler(
|
||||
.AsNoTracking()
|
||||
.SingleOrDefaultAsync(a => a.Id == request.AlbumId, cancellationToken);
|
||||
if (album == null)
|
||||
{
|
||||
logger.LogInformation("Album {AlbumId} not found", request.AlbumId);
|
||||
return Error.NotFound();
|
||||
}
|
||||
return AppErrors.NotFound("Album", request.AlbumId);
|
||||
|
||||
if (album.OwnerUserId != request.UserId)
|
||||
{
|
||||
logger.LogWarning("User {UserId} is not the owner of album {AlbumId}", request.UserId, request.AlbumId);
|
||||
return Error.Unauthorized();
|
||||
}
|
||||
return AppErrors.Forbidden("Album", request.AlbumId);
|
||||
|
||||
var track = await database.Tracks
|
||||
.AsNoTracking()
|
||||
.SingleOrDefaultAsync(t => t.Id == request.TrackId, cancellationToken);
|
||||
if (track == null)
|
||||
{
|
||||
logger.LogInformation("Track {TrackId} not found", request.TrackId);
|
||||
return Error.NotFound(description: $"Track {request.TrackId} not found");
|
||||
}
|
||||
return AppErrors.NotFound("Track", request.TrackId);
|
||||
|
||||
if (track.OwnerUserId != request.UserId)
|
||||
{
|
||||
logger.LogWarning("User {UserId} does not own track {TrackId}", request.UserId, request.TrackId);
|
||||
return Error.Unauthorized();
|
||||
}
|
||||
return AppErrors.Forbidden("Track", request.TrackId);
|
||||
|
||||
var alreadyAdded = await database.AlbumHasTracks
|
||||
.AsNoTracking()
|
||||
.AnyAsync(albumTrack => albumTrack.AlbumId == request.AlbumId && albumTrack.TrackId == request.TrackId, cancellationToken);
|
||||
if (alreadyAdded)
|
||||
{
|
||||
logger.LogInformation("Track {TrackId} already in album {AlbumId}", request.TrackId, request.AlbumId);
|
||||
return Error.Conflict(description: "Track is already in the album.");
|
||||
}
|
||||
return AlreadyInAlbum;
|
||||
|
||||
var albumTracksQuery = database.AlbumHasTracks
|
||||
.Where(albumTrack => albumTrack.AlbumId == request.AlbumId);
|
||||
|
||||
var hasTracks = await albumTracksQuery.AnyAsync(cancellationToken);
|
||||
var nextTrackNumber = hasTracks
|
||||
? await albumTracksQuery.MaxAsync(albumTrack => albumTrack.TrackNumber, cancellationToken) + 1
|
||||
: 1;
|
||||
// Album track numbers are user-visible, so they start at 1.
|
||||
var nextTrackNumber = (await database.AlbumHasTracks
|
||||
.Where(albumTrack => albumTrack.AlbumId == request.AlbumId)
|
||||
.MaxAsync(albumTrack => (int?)albumTrack.TrackNumber, cancellationToken) ?? 0) + 1;
|
||||
|
||||
await database.AlbumHasTracks.AddAsync(new AlbumHasTrack
|
||||
{
|
||||
@@ -71,11 +54,15 @@ public class AddTrackToAlbumCommandHandler(
|
||||
TrackNumber = nextTrackNumber
|
||||
}, cancellationToken);
|
||||
|
||||
await database.SaveChangesAsync(cancellationToken);
|
||||
var saved = await database.TrySaveChangesAsync(AlreadyInAlbum, cancellationToken);
|
||||
if (saved.IsError)
|
||||
return saved;
|
||||
|
||||
logger.LogInformation("Added track {TrackId} to album {AlbumId} as number {TrackNumber}",
|
||||
request.TrackId, request.AlbumId, nextTrackNumber);
|
||||
|
||||
return new Success();
|
||||
return Result.Success;
|
||||
}
|
||||
|
||||
private static Error AlreadyInAlbum => AppErrors.Conflict("Album.TrackAlreadyAdded", "Track is already in the album.");
|
||||
}
|
||||
|
||||
@@ -57,13 +57,7 @@ public class CreateAlbumCommandHandler(
|
||||
NormalizedTitle = title.ToUpperInvariant(),
|
||||
Description = request.Description,
|
||||
ReleaseYear = request.ReleaseYear,
|
||||
Pictures = new AlbumPictures
|
||||
{
|
||||
OriginalName = intent.ObjectName,
|
||||
SmallName = intent.ObjectName,
|
||||
MediumName = intent.ObjectName,
|
||||
LargeName = intent.ObjectName
|
||||
}
|
||||
Pictures = AlbumPictures.Pending(intent.ObjectName)
|
||||
};
|
||||
|
||||
await database.Albums.AddAsync(album, cancellationToken);
|
||||
|
||||
@@ -3,6 +3,9 @@ using Mediator;
|
||||
using Microsoft.EntityFrameworkCore;
|
||||
using Microsoft.Extensions.Logging;
|
||||
using Musify.Application.Contracts;
|
||||
using Musify.Application.Events;
|
||||
using Musify.Application.Shared;
|
||||
using Musify.Domain.ValueObjects;
|
||||
|
||||
namespace Musify.Application.Albums;
|
||||
|
||||
@@ -11,39 +14,27 @@ public record DeleteAlbumCommand(long UserId, Guid AlbumId)
|
||||
|
||||
public class DeleteAlbumCommandHandler(
|
||||
IDatabase database,
|
||||
IEventBus eventBus,
|
||||
ILogger<DeleteAlbumCommandHandler> logger)
|
||||
: ICommandHandler<DeleteAlbumCommand, ErrorOr<Success>>
|
||||
{
|
||||
public async ValueTask<ErrorOr<Success>> Handle(DeleteAlbumCommand request, CancellationToken cancellationToken)
|
||||
{
|
||||
var album = await database.Albums
|
||||
.SingleOrDefaultAsync(a => a.Id == request.AlbumId, cancellationToken);
|
||||
.SingleOrDefaultAsync(a => a.Id == request.AlbumId && a.LifeCycleStatus == LifeCycleStatus.Active, cancellationToken);
|
||||
if (album == null)
|
||||
{
|
||||
logger.LogDebug("Album {AlbumId} not found", request.AlbumId);
|
||||
return Error.NotFound();
|
||||
}
|
||||
return AppErrors.NotFound("Album", request.AlbumId);
|
||||
|
||||
if (album.OwnerUserId != request.UserId)
|
||||
{
|
||||
logger.LogWarning("Album {AlbumId} does not belong to user {UserId}", request.AlbumId, request.UserId);
|
||||
return Error.Unauthorized();
|
||||
}
|
||||
return AppErrors.Forbidden("Album", request.AlbumId);
|
||||
|
||||
database.Albums.Remove(album);
|
||||
album.LifeCycleStatus = LifeCycleStatus.Removing;
|
||||
|
||||
try
|
||||
{
|
||||
await database.SaveChangesAsync(cancellationToken);
|
||||
}
|
||||
catch (Exception exception)
|
||||
{
|
||||
logger.LogError(exception, "Failed to delete album {AlbumId}", request.AlbumId);
|
||||
return Error.Failure(description: $"Failed to delete album {request.AlbumId}");
|
||||
}
|
||||
await eventBus.PublishAsync(new DeleteAlbumEvent(album.Id, request.UserId), cancellationToken);
|
||||
await database.SaveChangesAsync(cancellationToken);
|
||||
|
||||
logger.LogInformation("Deleted album {AlbumId} for user {UserId}", request.AlbumId, request.UserId);
|
||||
logger.LogInformation("Marked album {AlbumId} of user {UserId} as removing", request.AlbumId, request.UserId);
|
||||
|
||||
return new Success();
|
||||
return Result.Success;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -3,6 +3,7 @@ using Mediator;
|
||||
using Microsoft.EntityFrameworkCore;
|
||||
using Musify.Application.Albums.Responses;
|
||||
using Musify.Application.Contracts;
|
||||
using Musify.Domain.ValueObjects;
|
||||
|
||||
namespace Musify.Application.Albums;
|
||||
|
||||
@@ -16,7 +17,7 @@ public class GetAlbumByIdQueryHandler(IDatabase database)
|
||||
{
|
||||
var album = await database.Albums
|
||||
.AsNoTracking()
|
||||
.Where(a => a.Id == request.Id)
|
||||
.Where(a => a.Id == request.Id && a.LifeCycleStatus == LifeCycleStatus.Active)
|
||||
.Select(a => new
|
||||
{
|
||||
Album = a,
|
||||
|
||||
@@ -4,6 +4,8 @@ using Microsoft.EntityFrameworkCore;
|
||||
using MimeMapping;
|
||||
using Musify.Application.Configuration;
|
||||
using Musify.Application.Contracts;
|
||||
using Musify.Application.Shared;
|
||||
using Musify.Domain.ValueObjects;
|
||||
|
||||
namespace Musify.Application.Albums;
|
||||
|
||||
@@ -20,14 +22,15 @@ public class GetAlbumCoverQueryHandler(
|
||||
{
|
||||
public async ValueTask<ErrorOr<AlbumCoverLocation>> Handle(GetAlbumCoverQuery request, CancellationToken cancellationToken)
|
||||
{
|
||||
var pictures = await database.Albums.AsNoTracking()
|
||||
.Where(a => a.Id == request.AlbumId)
|
||||
.Select(a => a.Pictures)
|
||||
var owned = await database.Albums.AsNoTracking()
|
||||
.Where(e => e.Id == request.AlbumId && e.LifeCycleStatus == LifeCycleStatus.Active)
|
||||
.Select(e => new { OwnerUserId = e.OwnerUserId, e.Pictures })
|
||||
.SingleOrDefaultAsync(cancellationToken);
|
||||
|
||||
if (pictures == null)
|
||||
return Error.NotFound();
|
||||
if (owned?.Pictures == null)
|
||||
return AppErrors.NotFound("AlbumCover", request.AlbumId);
|
||||
|
||||
var pictures = owned.Pictures;
|
||||
var routes = albumConfiguration.Routes;
|
||||
var (name, key) = request.Size.ToLowerInvariant() switch
|
||||
{
|
||||
@@ -36,8 +39,15 @@ public class GetAlbumCoverQueryHandler(
|
||||
_ => (pictures.MediumName, BuildKey(routes.BuildMediumPicturePath, pictures.MediumName))
|
||||
};
|
||||
|
||||
// The resized files only exist once processing has finished; until then serve the original.
|
||||
if (string.IsNullOrEmpty(name) || string.IsNullOrEmpty(key))
|
||||
return Error.NotFound();
|
||||
{
|
||||
name = pictures.OriginalName;
|
||||
key = string.IsNullOrEmpty(name) ? null : routes.BuildOriginalPicturePath(owned.OwnerUserId, name);
|
||||
}
|
||||
|
||||
if (string.IsNullOrEmpty(name) || string.IsNullOrEmpty(key))
|
||||
return AppErrors.NotFound("AlbumCover", request.AlbumId);
|
||||
|
||||
var contentType = MimeUtility.GetMimeMapping(name);
|
||||
return new AlbumCoverLocation(storageConfiguration.Bucket, key, contentType);
|
||||
|
||||
@@ -6,6 +6,7 @@ using Musify.Application.Shared;
|
||||
using Musify.Application.Tracks.Responses;
|
||||
using X.PagedList;
|
||||
using X.PagedList.EF;
|
||||
using Musify.Domain.ValueObjects;
|
||||
|
||||
namespace Musify.Application.Albums;
|
||||
|
||||
@@ -22,7 +23,7 @@ public class GetAlbumTracksQueryHandler(IDatabase database)
|
||||
{
|
||||
var albumExists = await database.Albums
|
||||
.AsNoTracking()
|
||||
.AnyAsync(album => album.Id == request.AlbumId, cancellationToken);
|
||||
.AnyAsync(album => album.Id == request.AlbumId && album.LifeCycleStatus == LifeCycleStatus.Active, cancellationToken);
|
||||
if (!albumExists)
|
||||
return Error.NotFound();
|
||||
|
||||
@@ -30,7 +31,7 @@ public class GetAlbumTracksQueryHandler(IDatabase database)
|
||||
.AsNoTracking()
|
||||
.Include(albumTrack => albumTrack.Track.Owner)
|
||||
.Include(albumTrack => albumTrack.Track.Tags)
|
||||
.Where(albumTrack => albumTrack.AlbumId == request.AlbumId);
|
||||
.Where(albumTrack => albumTrack.AlbumId == request.AlbumId && albumTrack.Track.LifeCycleStatus == LifeCycleStatus.Active);
|
||||
|
||||
var totalCount = await tracksQuery.CountAsync(cancellationToken);
|
||||
|
||||
|
||||
@@ -4,6 +4,7 @@ using Microsoft.EntityFrameworkCore;
|
||||
using Musify.Application.Albums.Responses;
|
||||
using Musify.Application.Contracts;
|
||||
using X.PagedList.EF;
|
||||
using Musify.Domain.ValueObjects;
|
||||
|
||||
namespace Musify.Application.Albums;
|
||||
|
||||
@@ -17,7 +18,7 @@ public class GetAlbumsQueryHandler(IDatabase database)
|
||||
{
|
||||
var albumsQuery = database.Albums
|
||||
.AsNoTracking()
|
||||
.AsQueryable();
|
||||
.Where(album => album.LifeCycleStatus == LifeCycleStatus.Active);
|
||||
|
||||
if (!string.IsNullOrEmpty(request.Title))
|
||||
{
|
||||
|
||||
@@ -6,6 +6,7 @@ using Musify.Application.Contracts;
|
||||
using Musify.Application.Shared;
|
||||
using X.PagedList;
|
||||
using X.PagedList.EF;
|
||||
using Musify.Domain.ValueObjects;
|
||||
|
||||
namespace Musify.Application.Albums;
|
||||
|
||||
@@ -25,7 +26,7 @@ public class GetAlbumsByUserIdQueryHandler(IDatabase database)
|
||||
|
||||
var albumsQuery = database.Albums
|
||||
.AsNoTracking()
|
||||
.Where(album => album.OwnerUserId == request.UserId);
|
||||
.Where(album => album.OwnerUserId == request.UserId && album.LifeCycleStatus == LifeCycleStatus.Active);
|
||||
|
||||
var totalCount = await albumsQuery.CountAsync(cancellationToken);
|
||||
|
||||
|
||||
@@ -3,6 +3,7 @@ using Mediator;
|
||||
using Microsoft.EntityFrameworkCore;
|
||||
using Musify.Application.Albums.Responses;
|
||||
using Musify.Application.Contracts;
|
||||
using Musify.Domain.ValueObjects;
|
||||
|
||||
namespace Musify.Application.Albums;
|
||||
|
||||
@@ -38,7 +39,7 @@ public class GetRecentlyListenedAlbumsQueryHandler(IDatabase database)
|
||||
|
||||
var albums = await database.Albums
|
||||
.AsNoTracking()
|
||||
.Where(album => albumIds.Contains(album.Id))
|
||||
.Where(album => albumIds.Contains(album.Id) && album.LifeCycleStatus == LifeCycleStatus.Active)
|
||||
.Select(album => new
|
||||
{
|
||||
Album = album,
|
||||
|
||||
@@ -3,6 +3,7 @@ using Mediator;
|
||||
using Microsoft.EntityFrameworkCore;
|
||||
using Microsoft.Extensions.Logging;
|
||||
using Musify.Application.Contracts;
|
||||
using Musify.Application.Shared;
|
||||
|
||||
namespace Musify.Application.Albums;
|
||||
|
||||
@@ -20,16 +21,10 @@ public class RemoveTrackFromAlbumCommandHandler(
|
||||
.AsNoTracking()
|
||||
.SingleOrDefaultAsync(a => a.Id == request.AlbumId, cancellationToken);
|
||||
if (album == null)
|
||||
{
|
||||
logger.LogInformation("Album {AlbumId} not found", request.AlbumId);
|
||||
return Error.NotFound();
|
||||
}
|
||||
return AppErrors.NotFound("Album", request.AlbumId);
|
||||
|
||||
if (album.OwnerUserId != request.UserId)
|
||||
{
|
||||
logger.LogWarning("User {UserId} is not the owner of album {AlbumId}", request.UserId, request.AlbumId);
|
||||
return Error.Unauthorized();
|
||||
}
|
||||
return AppErrors.Forbidden("Album", request.AlbumId);
|
||||
|
||||
var albumTracks = await database.AlbumHasTracks
|
||||
.Where(albumTrack => albumTrack.AlbumId == request.AlbumId)
|
||||
|
||||
@@ -45,7 +45,7 @@ public class UpdateAlbumCommandHandler(
|
||||
if (album.OwnerUserId != request.UserId)
|
||||
{
|
||||
logger.LogWarning("Album {AlbumId} does not belong to user {UserId}", request.AlbumId, request.UserId);
|
||||
return Error.Unauthorized();
|
||||
return AppErrors.Forbidden("Album", request.AlbumId);
|
||||
}
|
||||
|
||||
var title = request.Title.Trim();
|
||||
@@ -67,13 +67,7 @@ public class UpdateAlbumCommandHandler(
|
||||
return validation.Errors;
|
||||
|
||||
pictureIntent = validation.Value;
|
||||
album.Pictures = new AlbumPictures
|
||||
{
|
||||
OriginalName = pictureIntent.ObjectName,
|
||||
SmallName = pictureIntent.ObjectName,
|
||||
MediumName = pictureIntent.ObjectName,
|
||||
LargeName = pictureIntent.ObjectName
|
||||
};
|
||||
album.Pictures = AlbumPictures.Pending(pictureIntent.ObjectName);
|
||||
finalPictureKey = albumConfiguration.Routes.BuildOriginalPicturePath(request.UserId, pictureIntent.ObjectName);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user