diff --git a/docs/web/database.md b/docs/web/database.md index 70204a8d..2ffb894f 100644 --- a/docs/web/database.md +++ b/docs/web/database.md @@ -76,14 +76,14 @@ updates (`UPDATE ... WHERE ... AND RowVersion = @RowVersion`). | `UserAccount` | `Domain.Entities.UserAccount`, `Features.Users` | `Username` and `Email` are unique | | `UserCredential` | `Domain.Entities.UserCredential`, `Features.Users` | Argon2id password hash; `IsRevoked`/`RevokedAt` track credential rotation, not JWTs | | `UserVerification` | `Domain.Entities.UserVerification`, `Features.Users` | Written when a user confirms their email | -| `UserProfile` | `Domain.Entities.UserProfile`, `Features.Users` | 1:1 with `UserAccount`; holds the user's biography; written via `IUserProfileRepository`, read via `IUserListRepository.GetByIdAsync`'s left join | -| `UserAvatar` | `Domain.Entities.UserAvatar`, `Features.Users` | Links a `UserProfile` to a `Photo` as its avatar; written via `IUserProfileRepository.SaveAvatarAsync` | +| `UserProfile` | `Domain.Entities.UserProfile`, `Features.Users` | 1:1 with `UserAccount`; holds the user's biography; written via `IUserProfileRepository`, read via `IUserListRepository`'s left joins in both `GetByIdAsync` and `GetAllAsync` | +| `UserAvatar` | `Domain.Entities.UserAvatar`, `Features.Users` | Links a `UserProfile` to a `Photo` as its avatar; written via `IUserProfileRepository.SaveAvatarAsync`, read via `IUserListRepository`'s `ValidTo IS NULL` join for the active avatar | | `BreweryPost` | `Domain.Entities.BreweryPost`, `Features.Breweries` | A user-submitted brewery listing | | `BreweryPostLocation` | `Domain.Entities.BreweryPostLocation`, `Features.Breweries` | 1:1 with `BreweryPost`; `Coordinates` is a `GEOGRAPHY` column, read via a plain Dapper query that selects `CONVERT(varbinary(max), Coordinates)`, since Dapper can't deserialize SQL Server UDTs directly | | `City` | `Domain.Entities.City`, `Features.Locations` | Referenced by `BreweryPostLocation.CityID`; `CityName` and `StateProvinceID` together are unique; written via `ILocationRepository`, read via `ILocationRepository` and `IBreweryRepository`'s location joins, currently written only by `Database.Seed` | | `StateProvince` | `Domain.Entities.StateProvince`, `Features.Locations` | Referenced by `City.StateProvinceID`; created on demand by `ILocationRepository.GetOrCreateCityIdAsync`, read via `IBreweryRepository`'s location joins | | `Country` | `Domain.Entities.Country`, `Features.Locations` | Referenced by `StateProvince.CountryID`; created on demand by `ILocationRepository.GetOrCreateCityIdAsync`, read via `IBreweryRepository`'s location joins | -| `Photo` | `Domain.Entities.Photo`, `Features.PhotoUpload` | Written via `IPhotoUploadRepository.CreateAsync`, sent as an `UploadPhotoCommand` from other features' handlers; not yet seeded by `Database.Seed` | +| `Photo` | `Domain.Entities.Photo`, `Features.PhotoUpload` | Written via `IPhotoUploadRepository.CreateAsync`, sent as an `UploadPhotoCommand` from other features' handlers; `Hyperlink` holds the object key within the bucket, which readers sign into a time-limited URL via `IFileStorageProvider.GetPresignedUrl`; the active avatar's row is also read via `IUserListRepository`'s `Media.Photo` join; not yet seeded by `Database.Seed` | ### Tables defined in the schema but not yet wired to a feature slice diff --git a/web/backend/Domain/Domain.Entities/Entities/UserAccount.cs b/web/backend/Domain/Domain.Entities/Entities/UserAccount.cs index 8a974e94..648bbaa4 100644 --- a/web/backend/Domain/Domain.Entities/Entities/UserAccount.cs +++ b/web/backend/Domain/Domain.Entities/Entities/UserAccount.cs @@ -1,3 +1,5 @@ +using System.Text.Json.Serialization; + namespace Domain.Entities; public sealed class UserAccount @@ -20,4 +22,10 @@ public sealed class UserAccount // Navigation properties public UserCredential? UserCredential { get; set; } public UserProfile? UserProfile { get; set; } + + // GET /api/user/authenticated serializes this entity directly, and the avatar graph carries the + // storage key and uploader of the underlying Photo. Clients read the avatar from + // PublicUserProfileDto.AvatarUrl, which is a signed URL, so keep this one out of the response. + [JsonIgnore] + public UserAvatar? UserAvatar { get; set; } } diff --git a/web/backend/Features/Features.Users.Tests/Queries/GetAllUsersHandlerTests.cs b/web/backend/Features/Features.Users.Tests/Queries/GetAllUsersHandlerTests.cs index c7f8bd44..75240529 100644 --- a/web/backend/Features/Features.Users.Tests/Queries/GetAllUsersHandlerTests.cs +++ b/web/backend/Features/Features.Users.Tests/Queries/GetAllUsersHandlerTests.cs @@ -1,4 +1,5 @@ using Domain.Entities; +using Features.ImageUploads.Services; using Features.Users.Dtos; using Features.Users.Queries.GetAllUsers; using Features.Users.Repository; @@ -9,27 +10,36 @@ namespace Features.Users.Tests.Queries; public class GetAllUsersHandlerTests { + private readonly Mock _repoMock = new(); + private readonly Mock _storageMock = new(); + private readonly GetAllUsersHandler _handler; + + public GetAllUsersHandlerTests() + { + _storageMock + .Setup(s => s.GetPresignedUrl(It.IsAny(), It.IsAny())) + .Returns((string key, TimeSpan _) => $"https://storage.test/{key}?signed"); + + _handler = new GetAllUsersHandler(_repoMock.Object, _storageMock.Object); + } + [Fact] public async Task Handle_PassesLimitAndOffset_ToRepository() { - Mock repoMock = new(); - GetAllUsersHandler handler = new(repoMock.Object); - repoMock.Setup(r => r.GetAllAsync(10, 5)).ReturnsAsync(Array.Empty()); + _repoMock.Setup(r => r.GetAllAsync(10, 5)).ReturnsAsync(Array.Empty()); - IEnumerable result = await handler.Handle( + IEnumerable result = await _handler.Handle( new GetAllUsersQuery(10, 5), CancellationToken.None ); result.Should().BeEmpty(); - repoMock.Verify(r => r.GetAllAsync(10, 5), Times.Once); + _repoMock.Verify(r => r.GetAllAsync(10, 5), Times.Once); } [Fact] public async Task Handle_MapsUsersToPublicProfileDtos_AndExcludesPrivateFields() { - Mock repoMock = new(); - GetAllUsersHandler handler = new(repoMock.Object); UserAccount user = new() { UserAccountId = Guid.NewGuid(), @@ -38,13 +48,60 @@ public class GetAllUsersHandlerTests LastName = "One", CreatedAt = DateTime.UtcNow, }; - repoMock.Setup(r => r.GetAllAsync(null, null)).ReturnsAsync([user]); + _repoMock.Setup(r => r.GetAllAsync(null, null)).ReturnsAsync([user]); - IEnumerable result = await handler.Handle( + IEnumerable result = await _handler.Handle( new GetAllUsersQuery(null, null), CancellationToken.None ); - result.Should().ContainSingle().Which.Should().BeEquivalentTo(user.ToPublicProfileDto()); + result + .Should() + .ContainSingle() + .Which.Should() + .BeEquivalentTo(user.ToPublicProfileDto(_storageMock.Object)); + } + + [Fact] + public async Task Handle_MapsEachUsersOwnAvatarUrl() + { + string avatarKey = $"avatars/{Guid.NewGuid()}/{Guid.NewGuid()}"; + UserAccount withAvatar = new() + { + UserAccountId = Guid.NewGuid(), + Username = "has-avatar", + FirstName = "Has", + LastName = "Avatar", + CreatedAt = DateTime.UtcNow, + UserProfile = new UserProfile { Biography = "Drinks lagers." }, + UserAvatar = new UserAvatar { Photo = new Photo { Hyperlink = avatarKey } }, + }; + UserAccount withoutAvatar = new() + { + UserAccountId = Guid.NewGuid(), + Username = "no-avatar", + FirstName = "No", + LastName = "Avatar", + CreatedAt = DateTime.UtcNow, + }; + _repoMock.Setup(r => r.GetAllAsync(null, null)).ReturnsAsync([withAvatar, withoutAvatar]); + + IEnumerable result = await _handler.Handle( + new GetAllUsersQuery(null, null), + CancellationToken.None + ); + + result + .Select(dto => dto.AvatarUrl) + .Should() + .Equal($"https://storage.test/{avatarKey}?signed", string.Empty); + _storageMock.Verify( + s => s.GetPresignedUrl(avatarKey, UserDtoMapper.AvatarUrlLifetime), + Times.Once + ); + _storageMock.Verify( + s => s.GetPresignedUrl(It.IsAny(), It.IsAny()), + Times.Once + ); } } diff --git a/web/backend/Features/Features.Users.Tests/Queries/GetPublicUserProfileByIdHandlerTests.cs b/web/backend/Features/Features.Users.Tests/Queries/GetPublicUserProfileByIdHandlerTests.cs index 73d57615..028dd59b 100644 --- a/web/backend/Features/Features.Users.Tests/Queries/GetPublicUserProfileByIdHandlerTests.cs +++ b/web/backend/Features/Features.Users.Tests/Queries/GetPublicUserProfileByIdHandlerTests.cs @@ -1,5 +1,6 @@ using Domain.Entities; using Domain.Exceptions; +using Features.ImageUploads.Services; using Features.Users.Dtos; using Features.Users.Queries.GetPublicUserProfileById; using Features.Users.Repository; @@ -12,15 +13,21 @@ public class GetPublicUserProfileByIdHandlerTests { private readonly GetPublicUserProfileByIdHandler _handler; private readonly Mock _repoMock = new(); + private readonly Mock _storageMock = new(); public GetPublicUserProfileByIdHandlerTests() { - _handler = new GetPublicUserProfileByIdHandler(_repoMock.Object); + _storageMock + .Setup(s => s.GetPresignedUrl(It.IsAny(), It.IsAny())) + .Returns((string key, TimeSpan _) => $"https://storage.test/{key}?signed"); + + _handler = new GetPublicUserProfileByIdHandler(_repoMock.Object, _storageMock.Object); } [Fact] public async Task Handle_ReturnsPublicProfile_WhenFound() { + string avatarKey = $"avatars/{Guid.NewGuid()}/{Guid.NewGuid()}"; UserAccount user = new() { UserAccountId = Guid.NewGuid(), @@ -31,6 +38,7 @@ public class GetPublicUserProfileByIdHandlerTests DateOfBirth = new DateTime(1990, 1, 1), CreatedAt = new DateTime(2024, 1, 1), UserProfile = new UserProfile { Biography = "Drinks lagers." }, + UserAvatar = new UserAvatar { Photo = new Photo { Hyperlink = avatarKey } }, }; _repoMock.Setup(r => r.GetByIdAsync(user.UserAccountId)).ReturnsAsync(user); @@ -48,13 +56,48 @@ public class GetPublicUserProfileByIdHandlerTests user.FirstName, user.LastName, "Drinks lagers.", + $"https://storage.test/{avatarKey}?signed", user.CreatedAt ) ); } + /// + /// Photo.Hyperlink holds an object key, so the DTO has to carry a signed URL rather than + /// the stored value: a bare key resolves against the calling page's own origin. + /// [Fact] - public async Task Handle_ReturnsEmptyBiography_WhenProfileMissing() + public async Task Handle_SignsTheAvatarKey_RatherThanReturningIt() + { + string avatarKey = $"avatars/{Guid.NewGuid()}/{Guid.NewGuid()}"; + UserAccount user = new() + { + UserAccountId = Guid.NewGuid(), + Username = "test", + FirstName = "Jane", + LastName = "Doe", + CreatedAt = new DateTime(2024, 1, 1), + UserAvatar = new UserAvatar { Photo = new Photo { Hyperlink = avatarKey } }, + }; + _repoMock.Setup(r => r.GetByIdAsync(user.UserAccountId)).ReturnsAsync(user); + + PublicUserProfileDto result = await _handler.Handle( + new GetPublicUserProfileByIdQuery(user.UserAccountId), + CancellationToken.None + ); + + result.AvatarUrl.Should().NotBe(avatarKey); + Uri.TryCreate(result.AvatarUrl, UriKind.Absolute, out Uri? uri).Should().BeTrue(); + uri!.Scheme.Should().BeOneOf("http", "https"); + uri.AbsolutePath.Should().EndWith(avatarKey); + _storageMock.Verify( + s => s.GetPresignedUrl(avatarKey, UserDtoMapper.AvatarUrlLifetime), + Times.Once + ); + } + + [Fact] + public async Task Handle_ReturnsEmptyBiographyAndAvatarUrl_WhenProfileMissing() { UserAccount user = new() { @@ -72,6 +115,57 @@ public class GetPublicUserProfileByIdHandlerTests ); result.Biography.Should().BeEmpty(); + result.AvatarUrl.Should().BeEmpty(); + } + + [Fact] + public async Task Handle_ReturnsEmptyAvatarUrl_WhenProfileHasNoAvatar() + { + UserAccount user = new() + { + UserAccountId = Guid.NewGuid(), + Username = "test", + FirstName = "Jane", + LastName = "Doe", + CreatedAt = new DateTime(2024, 1, 1), + UserProfile = new UserProfile { Biography = "Drinks lagers." }, + }; + _repoMock.Setup(r => r.GetByIdAsync(user.UserAccountId)).ReturnsAsync(user); + + PublicUserProfileDto result = await _handler.Handle( + new GetPublicUserProfileByIdQuery(user.UserAccountId), + CancellationToken.None + ); + + result.Biography.Should().Be("Drinks lagers."); + result.AvatarUrl.Should().BeEmpty(); + } + + [Fact] + public async Task Handle_ReturnsEmptyAvatarUrl_WhenAvatarHasNoPhoto() + { + UserAccount user = new() + { + UserAccountId = Guid.NewGuid(), + Username = "test", + FirstName = "Jane", + LastName = "Doe", + CreatedAt = new DateTime(2024, 1, 1), + UserProfile = new UserProfile { Biography = "Drinks lagers." }, + UserAvatar = new UserAvatar(), + }; + _repoMock.Setup(r => r.GetByIdAsync(user.UserAccountId)).ReturnsAsync(user); + + PublicUserProfileDto result = await _handler.Handle( + new GetPublicUserProfileByIdQuery(user.UserAccountId), + CancellationToken.None + ); + + result.AvatarUrl.Should().BeEmpty(); + _storageMock.Verify( + s => s.GetPresignedUrl(It.IsAny(), It.IsAny()), + Times.Never + ); } [Fact] diff --git a/web/backend/Features/Features.Users/Dtos/UserDtoMapper.cs b/web/backend/Features/Features.Users/Dtos/UserDtoMapper.cs index 47277365..a56d6566 100644 --- a/web/backend/Features/Features.Users/Dtos/UserDtoMapper.cs +++ b/web/backend/Features/Features.Users/Dtos/UserDtoMapper.cs @@ -1,19 +1,39 @@ using Domain.Entities; +using Features.ImageUploads.Services; namespace Features.Users.Dtos; /// Converts user domain entities into API response models. public static class UserDtoMapper { + /// + /// How long a presigned avatar URL stays valid. Long enough to outlive the page that embeds it, + /// short enough that a leaked URL stops working. + /// + public static readonly TimeSpan AvatarUrlLifetime = TimeSpan.FromHours(1); + /// Converts a user account into its public-profile representation. - public static PublicUserProfileDto ToPublicProfileDto(this UserAccount user) + /// The account to convert. + /// + /// Signs the download URL for the account's avatar. Photo.Hyperlink holds an object key + /// rather than a URL, so it has to be signed before a client can fetch it. + /// + public static PublicUserProfileDto ToPublicProfileDto( + this UserAccount user, + IFileStorageProvider fileStorageProvider + ) { + string? avatarKey = user.UserAvatar?.Photo?.Hyperlink; + return new PublicUserProfileDto( UserAccountId : user.UserAccountId, Username : user.Username, FirstName : user.FirstName, LastName : user.LastName, Biography : user.UserProfile?.Biography ?? string.Empty, + AvatarUrl : string.IsNullOrWhiteSpace(avatarKey) + ? string.Empty + : fileStorageProvider.GetPresignedUrl(avatarKey, AvatarUrlLifetime), CreatedAt : user.CreatedAt ); } diff --git a/web/backend/Features/Features.Users/Dtos/UserDtos.cs b/web/backend/Features/Features.Users/Dtos/UserDtos.cs index 2aee8af4..39ec4a9a 100644 --- a/web/backend/Features/Features.Users/Dtos/UserDtos.cs +++ b/web/backend/Features/Features.Users/Dtos/UserDtos.cs @@ -7,5 +7,6 @@ public record PublicUserProfileDto( string FirstName, string LastName, string Biography, + string AvatarUrl, DateTime CreatedAt ); diff --git a/web/backend/Features/Features.Users/Queries/GetAllUsers/GetAllUsersHandler.cs b/web/backend/Features/Features.Users/Queries/GetAllUsers/GetAllUsersHandler.cs index 9ccabf84..9722bf5f 100644 --- a/web/backend/Features/Features.Users/Queries/GetAllUsers/GetAllUsersHandler.cs +++ b/web/backend/Features/Features.Users/Queries/GetAllUsers/GetAllUsersHandler.cs @@ -1,12 +1,15 @@ using Domain.Entities; +using Features.ImageUploads.Services; using Features.Users.Dtos; using Features.Users.Repository; using MediatR; namespace Features.Users.Queries.GetAllUsers; -public class GetAllUsersHandler(IUserListRepository repository) - : IRequestHandler> +public class GetAllUsersHandler( + IUserListRepository repository, + IFileStorageProvider fileStorageProvider +) : IRequestHandler> { public async Task> Handle( GetAllUsersQuery request, @@ -14,6 +17,6 @@ public class GetAllUsersHandler(IUserListRepository repository) ) { IEnumerable users = await repository.GetAllAsync(request.Limit, request.Offset); - return users.Select(user => user.ToPublicProfileDto()); + return users.Select(user => user.ToPublicProfileDto(fileStorageProvider)); } } diff --git a/web/backend/Features/Features.Users/Queries/GetPublicUserProfileById/GetPublicUserProfileByIdHandler.cs b/web/backend/Features/Features.Users/Queries/GetPublicUserProfileById/GetPublicUserProfileByIdHandler.cs index 47965586..0769b37e 100644 --- a/web/backend/Features/Features.Users/Queries/GetPublicUserProfileById/GetPublicUserProfileByIdHandler.cs +++ b/web/backend/Features/Features.Users/Queries/GetPublicUserProfileById/GetPublicUserProfileByIdHandler.cs @@ -1,13 +1,16 @@ using Domain.Entities; using Domain.Exceptions; +using Features.ImageUploads.Services; using Features.Users.Dtos; using Features.Users.Repository; using MediatR; namespace Features.Users.Queries.GetPublicUserProfileById; -public class GetPublicUserProfileByIdHandler(IUserListRepository repository) - : IRequestHandler +public class GetPublicUserProfileByIdHandler( + IUserListRepository repository, + IFileStorageProvider fileStorageProvider +) : IRequestHandler { /// Thrown when no user account exists with the given ID. public async Task Handle( @@ -18,6 +21,6 @@ public class GetPublicUserProfileByIdHandler(IUserListRepository repository) UserAccount? user = await repository.GetByIdAsync(request.UserAccountId); if (user is null) throw new NotFoundException($"User with ID {request.UserAccountId} not found"); - return user.ToPublicProfileDto(); + return user.ToPublicProfileDto(fileStorageProvider); } } diff --git a/web/backend/Features/Features.Users/Repository/UserListRepository.cs b/web/backend/Features/Features.Users/Repository/UserListRepository.cs index f5dc8543..91781dc7 100644 --- a/web/backend/Features/Features.Users/Repository/UserListRepository.cs +++ b/web/backend/Features/Features.Users/Repository/UserListRepository.cs @@ -15,7 +15,10 @@ public class UserListRepository(ISqlConnectionFactory connectionFactory) /// /// /// Left-joins Social.UserProfile, so is - /// null for an account whose profile has yet to be created. + /// null for an account whose profile has yet to be created, and left-joins the + /// account's active Social.UserAvatar (ValidTo IS NULL) with its + /// Media.Photo, so is null for an account + /// with no avatar. /// public async Task GetByIdAsync(Guid id) { @@ -23,51 +26,89 @@ public class UserListRepository(ISqlConnectionFactory connectionFactory) IEnumerable results = await connection.QueryAsync< UserAccount, UserProfile, + Photo, + UserAvatar, UserAccount >( """ SELECT - ua.UserAccountID, - ua.Username, - ua.FirstName, ua.LastName, ua.Email, ua.CreatedAt, ua.UpdatedAt, - ua.DateOfBirth, ua.RowVersion, - up.UserProfileID, up.UserAccountID, up.Biography, up.RowVersion + ua.UserAccountID, ua.Username, ua.FirstName, ua.LastName, + ua.Email, ua.CreatedAt, ua.UpdatedAt, ua.DateOfBirth, + ua.RowVersion, up.UserProfileID, up.UserAccountID, + up.Biography, up.RowVersion, p.PhotoID, p.UploadedByID, + p.Hyperlink, p.UploadedAt, p.RowVersion, av.UserAvatarID, + av.UserProfileID, av.PhotoID, av.ValidFrom, av.ValidTo, + av.RowVersion FROM Auth.UserAccount ua - LEFT JOIN Social.UserProfile up ON ua.UserAccountID = up.UserAccountID + LEFT JOIN Social.UserProfile up + ON ua.UserAccountID = up.UserAccountID + LEFT JOIN Social.UserAvatar av + ON up.UserProfileID = av.UserProfileID + AND av.ValidTo IS NULL + LEFT JOIN Media.Photo p ON av.PhotoID = p.PhotoID WHERE ua.UserAccountID = @UserAccountId """, MapUserRow, new { UserAccountId = id }, - splitOn: "UserProfileID" + splitOn: AvatarSplitOn ); return results.SingleOrDefault(); } - private static UserAccount MapUserRow(UserAccount user, UserProfile? profile) - { - user.UserProfile = profile; - return user; - } - /// /// /// Always applies pagination via OFFSET/FETCH, ordered by creation date descending. /// A null / is treated as "no limit"/"no offset". - /// Does not select Email or DateOfBirth: this listing is consumed only as public - /// profiles, so those fields are left at their default rather than read from the database. + /// Joins the same profile and active-avatar rows as , both of which are + /// part of a public profile. Does not select Email or DateOfBirth: this listing is + /// consumed only as public profiles, so those fields are left at their default rather than read + /// from the database. /// public async Task> GetAllAsync(int? limit, int? offset) { await using DbConnection connection = await CreateConnection(); - return await connection.QueryAsync( + return await connection.QueryAsync< + UserAccount, + UserProfile, + Photo, + UserAvatar, + UserAccount + >( """ - SELECT UserAccountID, Username, FirstName, LastName, CreatedAt, UpdatedAt, RowVersion - FROM Auth.UserAccount - ORDER BY CreatedAt DESC + SELECT + ua.UserAccountID, ua.Username, ua.FirstName, ua.LastName, + ua.CreatedAt, ua.UpdatedAt, ua.RowVersion, + up.UserProfileID, up.UserAccountID, up.Biography, up.RowVersion, + p.PhotoID, p.UploadedByID, p.Hyperlink, p.UploadedAt, p.RowVersion, + av.UserAvatarID, av.UserProfileID, av.PhotoID, av.ValidFrom, av.ValidTo, + av.RowVersion + FROM Auth.UserAccount ua + LEFT JOIN Social.UserProfile up ON ua.UserAccountID = up.UserAccountID + LEFT JOIN Social.UserAvatar av + ON up.UserProfileID = av.UserProfileID AND av.ValidTo IS NULL + LEFT JOIN Media.Photo p ON av.PhotoID = p.PhotoID + ORDER BY ua.CreatedAt DESC OFFSET @Offset ROWS FETCH NEXT @Limit ROWS ONLY """, - new { Offset = offset ?? 0, Limit = limit ?? int.MaxValue } + MapUserRow, + new { Offset = offset ?? 0, Limit = limit ?? int.MaxValue }, + splitOn: AvatarSplitOn ); } + + private const string AvatarSplitOn = "UserProfileID,PhotoID,UserAvatarID"; + + private static UserAccount MapUserRow( + UserAccount user, + UserProfile? profile, + Photo? photo, + UserAvatar? avatar + ) + { + user.UserProfile = profile; + avatar?.Photo = photo; + user.UserAvatar = avatar; + return user; + } }