From 5d75f8bea5d670f7698568ebcba14c951e120225 Mon Sep 17 00:00:00 2001 From: bitbiter-dev Date: Mon, 19 Oct 2026 15:10:01 +1300 Subject: [PATCH 1/2] feat(api): Star, like and hide an asset through PATCH /assets/{id}/interaction (#46) The caller's interaction state is written by a Core writer that takes and returns plain records, so the API still never touches AssetInteraction. It checks only VisibleTo, so a hidden asset can be unhidden; an empty change creates no row; and a first write that loses the (UserId, AssetId) unique race reloads and reapplies once. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 4 +- .../Endpoints/AssetDtoMapperTests.cs | 8 + .../Endpoints/MediaEndpointsTests.cs | 115 +++++++ .../ApiServiceRegistrationTests.cs | 1 + .../Infrastructure/RouteTableTests.cs | 10 +- src/Anichron.API/Endpoints/AssetDtoMapper.cs | 3 + src/Anichron.API/Endpoints/MediaEndpoints.cs | 20 ++ src/Anichron.API/Infrastructure/ApiPaths.cs | 1 + .../ServiceCollectionExtensions.cs | 1 + .../Data/ModelInvariantsTests.cs | 11 + .../Repository/AssetInteractionWriterTests.cs | 304 ++++++++++++++++++ .../Data/InteractionIndexNames.cs | 6 + .../Data/Repository/AssetInteractionWriter.cs | 62 ++++ 13 files changed, 540 insertions(+), 6 deletions(-) create mode 100644 src/Anichron.Core.Tests.Unit/Data/Repository/AssetInteractionWriterTests.cs create mode 100644 src/Anichron.Core/Data/InteractionIndexNames.cs create mode 100644 src/Anichron.Core/Data/Repository/AssetInteractionWriter.cs diff --git a/CLAUDE.md b/CLAUDE.md index f37c17b4..782b2acb 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -84,12 +84,12 @@ Four projects with strict separation of concerns: |---------|------| | `Anichron.Core` | Single source of truth: domain models, `AnichronDbContext`, all Fluent API config, shared utilities (XXHash64, path parsing). Zero dependencies on other projects (every other project references Core, so a reference back would be circular and fail the build). | | `Anichron.Infrastructure` | Shared host plumbing used by API and Worker: configuration (`DatabaseConfiguration` reads the connection from `POSTGRES_CONNECTION__*` env vars or Docker secrets; INI bootstrap; `ConfigurationList`), migration under a PostgreSQL advisory lock, and the API's read-only stores (`IProxyStore`, `IOriginalStore`). Each host wires its own DI. | -| `Anichron.API` | ASP.NET Core Minimal APIs (the only web project: `libraries-are-not-web` keeps ASP.NET Core out of Core, Infrastructure and Worker, and the two hosts never use each other) — owns all identity/auth/admin writes (users, storage configs, refresh tokens, invites, lockout); read-only for media data. Media endpoints so far: proxy serving (#43) and On This Day (`GET /api/v1/flashbacks/on-this-day`, #38); originals (#44) are deferred past v1.0. All routes prefixed `/api/v1/`. | +| `Anichron.API` | ASP.NET Core Minimal APIs (the only web project: `libraries-are-not-web` keeps ASP.NET Core out of Core, Infrastructure and Worker, and the two hosts never use each other) — owns all identity/auth/admin writes (users, storage configs, refresh tokens, invites, lockout); read-only for media data. Media endpoints so far: proxy serving (#43), On This Day (`GET /api/v1/flashbacks/on-this-day`, #38) and the caller's star/like/hide state (`PATCH /api/v1/assets/{id}/interaction`, #46); originals (#44) are deferred past v1.0. All routes prefixed `/api/v1/`. | | `Anichron.Worker` | `BackgroundService`s — NAS crawling, EXIF extraction, FFmpeg transcoding, proxy generation, expired-refresh-token cleanup; reconciliation is planned (Epic 4) | **Write ownership is split by domain, not centralized.** - **Worker is the only writer of *media* data** — `MediaAsset`, `Metadata`, `ProxyFile`, `Burst`, plus all reconciliation and soft-deletes happen here. The API never writes media: outside its DI wiring it never touches `AnichronDbContext` (architecture rule `api-reaches-data-through-repositories`), and the media write repository stays out of it (`only-worker-writes-media`). Its one identity-side write is `TokenCleanupService` deleting expired refresh tokens. -- **API owns all *identity/auth/admin* writes** — registration, login/refresh-token issuance and revocation, lockout state, invites, and admin user/storage-config CRUD all persist through API services (`AuthService`, `AdminUserService`, `AdminStorageConfigService`, `TokenService`, etc.) via `IUnitOfWork.SaveChangesAsync`. The API is read-only *only with respect to media*. The one exception is `AssetInteraction` (star/like/hide, #46): per-user state, to be written by the API. +- **API owns all *identity/auth/admin* writes** — registration, login/refresh-token issuance and revocation, lockout state, invites, and admin user/storage-config CRUD all persist through API services (`AuthService`, `AdminUserService`, `AdminStorageConfigService`, `TokenService`, etc.) via `IUnitOfWork.SaveChangesAsync`. The API is read-only *only with respect to media*. The one exception is `AssetInteraction` (star/like/hide, #46): per-user state, written by the API through `IAssetInteractionWriter` (Core), which takes and returns plain records so no API type touches the entity. It checks only `VisibleTo` (never `NotHiddenFor`, so a hidden asset can be unhidden), and a first write that loses the `(UserId, AssetId)` unique race reloads and reapplies once. - Both hosts apply EF migrations at startup, serialised by a PostgreSQL advisory lock (`MigrateWithAdvisoryLockAsync`). A planned `Anichron.UI` (NextJS/React) for the Instagram Story-style flashback experience is documented but not yet in the solution. diff --git a/src/Anichron.API.Tests.Unit/Endpoints/AssetDtoMapperTests.cs b/src/Anichron.API.Tests.Unit/Endpoints/AssetDtoMapperTests.cs index 26620bb2..db47920f 100644 --- a/src/Anichron.API.Tests.Unit/Endpoints/AssetDtoMapperTests.cs +++ b/src/Anichron.API.Tests.Unit/Endpoints/AssetDtoMapperTests.cs @@ -132,6 +132,14 @@ public void LikedAsset_WeighsDouble() .Should().Be(new AssetInteractionDto(false, true, false, 2.0)); } + [Theory] + [InlineData(true, false, false, 1.0)] + [InlineData(false, true, false, 2.0)] + [InlineData(false, false, true, 1.0)] + public void InteractionState_MapsEachFlagAndWeighsALikeDouble(bool starred, bool liked, bool hidden, double weight) + => AssetDtoMapper.ToDto(new InteractionState(starred, liked, hidden)) + .Should().Be(new AssetInteractionDto(starred, liked, hidden, weight)); + [Theory] [InlineData(MediaType.Image, "Image")] [InlineData(MediaType.Video, "Video")] diff --git a/src/Anichron.API.Tests.Unit/Endpoints/MediaEndpointsTests.cs b/src/Anichron.API.Tests.Unit/Endpoints/MediaEndpointsTests.cs index b9174ff9..af603267 100644 --- a/src/Anichron.API.Tests.Unit/Endpoints/MediaEndpointsTests.cs +++ b/src/Anichron.API.Tests.Unit/Endpoints/MediaEndpointsTests.cs @@ -6,6 +6,7 @@ using Microsoft.Extensions.DependencyInjection; using System.Net; using System.Net.Http.Headers; +using System.Net.Http.Json; namespace Anichron.API.Tests.Unit.Endpoints; @@ -29,6 +30,7 @@ private sealed class TestFixture { public FakeProxyStore Proxies { get; } = new(); public IMediaAssetReadRepository Assets { get; } = Substitute.For(); + public IAssetInteractionWriter Interactions { get; } = Substitute.For(); public void HasProxy(ProxyType type, string proxyPath, byte[]? bytes) { @@ -42,6 +44,7 @@ public Task StartAsync() => AuthenticatedTestApp.StartAsync(api => api.MapMediaEndpoints(), services => { services.AddSingleton(Assets); + services.AddSingleton(Interactions); services.AddSingleton(Proxies); }); } @@ -191,4 +194,116 @@ public async Task GetProxy_Anonymous_ReturnsUnauthorizedAsync() response.StatusCode.Should().Be(HttpStatusCode.Unauthorized); } + + // ========================================================================== + // PATCH interaction (#46) + // ========================================================================== + + private static HttpRequestMessage Patch(object body, Guid? userId = null) + { + var request = new HttpRequestMessage(HttpMethod.Patch, $"/api/v1/assets/{AssetId}/interaction") + { + Content = JsonContent.Create(body), + }; + if (userId is { } caller) + request.Headers.Add(AuthenticatedTestApp.UserHeader, caller.ToString()); + return request; + } + + [Fact] + public async Task PatchInteraction_OwnAsset_PassesThePresentFlagsAndReturnsTheResultingStateAsync() + { + var fixture = new TestFixture(); + fixture.Interactions.ApplyAsync(UserId, AssetId, new InteractionChange(null, true, null), Arg.Any()) + .Returns(new InteractionState(Starred: true, Liked: true, Hidden: false)); + await using var app = await fixture.StartAsync(); + using var client = app.Client; + + using var response = await client.SendAsync(Patch(new { liked = true }, UserId), TestContext.Current.CancellationToken); + + response.StatusCode.Should().Be(HttpStatusCode.OK); + (await response.Content.ReadFromJsonAsync(TestContext.Current.CancellationToken)) + .Should().Be(new AssetInteractionDto(Starred: true, Liked: true, Hidden: false, DisplayWeight: 2.0)); + } + + [Fact] + public async Task PatchInteraction_EveryFlag_IsPassedAsSentAsync() + { + var fixture = new TestFixture(); + fixture.Interactions.ApplyAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(new InteractionState(false, false, false)); + await using var app = await fixture.StartAsync(); + using var client = app.Client; + + using var response = await client.SendAsync( + Patch(new { starred = true, liked = false, hidden = true }, UserId), TestContext.Current.CancellationToken); + + response.StatusCode.Should().Be(HttpStatusCode.OK); + await fixture.Interactions.Received(1).ApplyAsync( + UserId, AssetId, new InteractionChange(true, false, true), Arg.Any()); + } + + [Fact] + public async Task PatchInteraction_EmptyBody_PassesNoChangeAsync() + { + var fixture = new TestFixture(); + fixture.Interactions.ApplyAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(new InteractionState(false, false, false)); + await using var app = await fixture.StartAsync(); + using var client = app.Client; + + using var response = await client.SendAsync(Patch(new { }, UserId), TestContext.Current.CancellationToken); + + response.StatusCode.Should().Be(HttpStatusCode.OK); + (await response.Content.ReadFromJsonAsync(TestContext.Current.CancellationToken)) + .Should().Be(new AssetInteractionDto(false, false, false, 1.0)); + await fixture.Interactions.Received(1).ApplyAsync( + UserId, AssetId, new InteractionChange(null, null, null), Arg.Any()); + } + + [Fact] + public async Task PatchInteraction_BodyNamingAnotherUser_WritesForTheCallerAsync() + { + var fixture = new TestFixture(); + fixture.Interactions.ApplyAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(new InteractionState(true, false, false)); + await using var app = await fixture.StartAsync(); + using var client = app.Client; + var other = Guid.Parse("cccccccc-0000-0000-0000-000000000003"); + + using var response = await client.SendAsync( + Patch(new { userId = other, starred = true }, UserId), TestContext.Current.CancellationToken); + + response.StatusCode.Should().Be(HttpStatusCode.OK); + await fixture.Interactions.Received(1).ApplyAsync( + UserId, AssetId, new InteractionChange(true, null, null), Arg.Any()); + await fixture.Interactions.DidNotReceive().ApplyAsync( + other, Arg.Any(), Arg.Any(), Arg.Any()); + } + + [Fact] + public async Task PatchInteraction_NotVisibleToTheCaller_ReturnsNotFoundAsync() + { + var fixture = new TestFixture(); + await using var app = await fixture.StartAsync(); + using var client = app.Client; + + using var response = await client.SendAsync(Patch(new { starred = true }, UserId), TestContext.Current.CancellationToken); + + response.StatusCode.Should().Be(HttpStatusCode.NotFound); + } + + [Fact] + public async Task PatchInteraction_Anonymous_ReturnsUnauthorizedWithoutAWriteAsync() + { + var fixture = new TestFixture(); + await using var app = await fixture.StartAsync(); + using var client = app.Client; + + using var response = await client.SendAsync(Patch(new { starred = true }), TestContext.Current.CancellationToken); + + response.StatusCode.Should().Be(HttpStatusCode.Unauthorized); + await fixture.Interactions.DidNotReceive().ApplyAsync( + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()); + } } diff --git a/src/Anichron.API.Tests.Unit/Infrastructure/ApiServiceRegistrationTests.cs b/src/Anichron.API.Tests.Unit/Infrastructure/ApiServiceRegistrationTests.cs index d840210f..007757c5 100644 --- a/src/Anichron.API.Tests.Unit/Infrastructure/ApiServiceRegistrationTests.cs +++ b/src/Anichron.API.Tests.Unit/Infrastructure/ApiServiceRegistrationTests.cs @@ -62,6 +62,7 @@ private static ServiceCollection AuthServices(IConfiguration? configuration = nu { typeof(IUserStorageConfigRepository), typeof(EfUserStorageConfigRepository), ServiceLifetime.Scoped }, { typeof(IMediaAssetReadRepository), typeof(EfMediaAssetReadRepository), ServiceLifetime.Scoped }, { typeof(IAssetViewReader), typeof(EfAssetViewReader), ServiceLifetime.Scoped }, + { typeof(IAssetInteractionWriter), typeof(EfAssetInteractionWriter), ServiceLifetime.Scoped }, { typeof(IProxyStore), typeof(ProxyStore), ServiceLifetime.Singleton }, { typeof(IOriginalStore), typeof(OriginalStore), ServiceLifetime.Singleton }, { typeof(IRegistrationValidator), typeof(RegistrationValidator), ServiceLifetime.Scoped }, diff --git a/src/Anichron.API.Tests.Unit/Infrastructure/RouteTableTests.cs b/src/Anichron.API.Tests.Unit/Infrastructure/RouteTableTests.cs index 1e737762..2e0af9cc 100644 --- a/src/Anichron.API.Tests.Unit/Infrastructure/RouteTableTests.cs +++ b/src/Anichron.API.Tests.Unit/Infrastructure/RouteTableTests.cs @@ -107,10 +107,12 @@ public void UserRoutes_AreTaggedUsers() => Routes.Where(r => r.Pattern.StartsWith("/api/v1/users", StringComparison.Ordinal)) .Should().OnlyContain(r => r.Tags.SequenceEqual(new[] { "Users" })); - [Fact] - public void ProxyRoute_RequiresAnAuthenticatedUserButNotAdmin() + [Theory] + [InlineData("GET", "/api/v1/assets/{assetId:guid}/proxies/{type}")] + [InlineData("PATCH", "/api/v1/assets/{assetId:guid}/interaction")] + public void AssetRoutes_RequireAnAuthenticatedUserButNotAdmin(string method, string pattern) { - var route = Find("GET", "/api/v1/assets/{assetId:guid}/proxies/{type}"); + var route = Find(method, pattern); route.Anonymous.Should().BeFalse(); route.Authenticated.Should().BeTrue(); @@ -143,5 +145,5 @@ public void Healthz_IsAnonymous() [Fact] public void EveryMappedRoute_IsCoveredByTheTestsAbove() - => Routes.Should().HaveCount(21); + => Routes.Should().HaveCount(22); } diff --git a/src/Anichron.API/Endpoints/AssetDtoMapper.cs b/src/Anichron.API/Endpoints/AssetDtoMapper.cs index c55cb0a4..8c794005 100644 --- a/src/Anichron.API/Endpoints/AssetDtoMapper.cs +++ b/src/Anichron.API/Endpoints/AssetDtoMapper.cs @@ -30,6 +30,9 @@ internal static AssetDto ToDto(AssetView view) new AssetInteractionDto(view.IsStarred, view.IsLiked, Hidden: false, FlashbackWeight.For(view.IsLiked))); } + internal static AssetInteractionDto ToDto(InteractionState state) + => new(state.Starred, state.Liked, state.Hidden, FlashbackWeight.For(state.Liked)); + private static string PublishedMediaType(MediaType type) => type switch { diff --git a/src/Anichron.API/Endpoints/MediaEndpoints.cs b/src/Anichron.API/Endpoints/MediaEndpoints.cs index 2dbd23cd..dcf27871 100644 --- a/src/Anichron.API/Endpoints/MediaEndpoints.cs +++ b/src/Anichron.API/Endpoints/MediaEndpoints.cs @@ -7,6 +7,8 @@ namespace Anichron.API.Endpoints; +public sealed record PatchInteractionRequest(bool? Starred, bool? Liked, bool? Hidden); + public static class MediaEndpoints { internal const string ProxyCacheControl = "private, max-age=31536000, immutable"; @@ -16,6 +18,8 @@ public static IEndpointRouteBuilder MapMediaEndpoints(this IEndpointRouteBuilder var group = app.MapGroup(ApiPaths.Assets.Group).WithTags("Media"); group.MapGet(ApiPaths.Assets.Proxy, GetProxyAsync) .RequireAuthorization(); + group.MapPatch(ApiPaths.Assets.Interaction, PatchInteractionAsync) + .RequireAuthorization(); return app; } @@ -48,4 +52,20 @@ internal static async Task GetProxyAsync( entityTag: new EntityTagHeaderValue($"\"{proxy.ContentHash}-{kind.Type}\""), enableRangeProcessing: true); } + + internal static async Task PatchInteractionAsync( + Guid assetId, + PatchInteractionRequest request, + ClaimsPrincipal user, + [FromServices] IAssetInteractionWriter interactions, + CancellationToken ct) + { + if (!Guid.TryParse(user.FindFirstValue(ClaimTypes.NameIdentifier), out var userId)) + return Results.Unauthorized(); + + var change = new InteractionChange(request.Starred, request.Liked, request.Hidden); + return await interactions.ApplyAsync(userId, assetId, change, ct) is { } state + ? Results.Ok(AssetDtoMapper.ToDto(state)) + : Results.NotFound(); + } } diff --git a/src/Anichron.API/Infrastructure/ApiPaths.cs b/src/Anichron.API/Infrastructure/ApiPaths.cs index 0ab89c0b..d6025f4b 100644 --- a/src/Anichron.API/Infrastructure/ApiPaths.cs +++ b/src/Anichron.API/Infrastructure/ApiPaths.cs @@ -45,6 +45,7 @@ internal static class Assets { internal const string Group = "assets"; internal const string Proxy = "{assetId:guid}/proxies/{type}"; + internal const string Interaction = "{assetId:guid}/interaction"; internal static string ProxyUrl(Guid assetId, string type) => $"{Group}/{assetId}/proxies/{type}"; diff --git a/src/Anichron.API/Infrastructure/ServiceCollectionExtensions.cs b/src/Anichron.API/Infrastructure/ServiceCollectionExtensions.cs index 340ce260..92c84b1e 100644 --- a/src/Anichron.API/Infrastructure/ServiceCollectionExtensions.cs +++ b/src/Anichron.API/Infrastructure/ServiceCollectionExtensions.cs @@ -104,6 +104,7 @@ public IServiceCollection AddAuthServices(IConfiguration configuration) services.AddScoped(); services.AddScoped(); services.AddScoped(); + services.AddScoped(); services.AddSingleton(); services.AddSingleton(); // AnichronDbContext is already scoped via AddDbContext; reuse the same instance for IUnitOfWork diff --git a/src/Anichron.Core.Tests.Unit/Data/ModelInvariantsTests.cs b/src/Anichron.Core.Tests.Unit/Data/ModelInvariantsTests.cs index 0a76ad92..460d4e63 100644 --- a/src/Anichron.Core.Tests.Unit/Data/ModelInvariantsTests.cs +++ b/src/Anichron.Core.Tests.Unit/Data/ModelInvariantsTests.cs @@ -24,6 +24,17 @@ public void ContentHash_IsUniqueOnlyPerStorageConfigAndOnlyAmongActiveAssets() index.GetDatabaseName().Should().Be(MediaAssetIndexNames.ActiveContentHashUnique); } + [Fact] + public void Interactions_AreUniquePerUserAndAssetUnderTheNameTheWriterRetriesOn() + { + using var context = AnichronDbContexts.NpgsqlModel(); + var interaction = context.Model.FindEntityType(typeof(AssetInteraction))!; + + var index = interaction.GetIndexes().Where(index => index.IsUnique).Should().ContainSingle().Which; + index.Properties.Select(p => p.Name).Should().Equal(nameof(AssetInteraction.UserId), nameof(AssetInteraction.AssetId)); + index.GetDatabaseName().Should().Be(InteractionIndexNames.OnePerUserAndAsset); + } + [Fact] public void QueryFilters_AreSoftDeleteOnly() { diff --git a/src/Anichron.Core.Tests.Unit/Data/Repository/AssetInteractionWriterTests.cs b/src/Anichron.Core.Tests.Unit/Data/Repository/AssetInteractionWriterTests.cs new file mode 100644 index 00000000..3d481e31 --- /dev/null +++ b/src/Anichron.Core.Tests.Unit/Data/Repository/AssetInteractionWriterTests.cs @@ -0,0 +1,304 @@ +using Anichron.Core.Data; +using Anichron.Core.Data.Repository; +using Anichron.Core.Domain; +using Anichron.Testing.Data; +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Diagnostics; +using NodaTime; + +namespace Anichron.Core.Tests.Unit.Data.Repository; + +public sealed class AssetInteractionWriterTests +{ + private sealed class Library + { + public Library() + { + MiraConfig = new UserStorageConfig { Id = Guid.NewGuid(), UserId = Mira.Id, RootPath = "/data/originals/mira", IsActive = true }; + Photo = new MediaAsset + { + Id = Guid.NewGuid(), + StorageConfigId = MiraConfig.Id, + FilePath = "2019/beach.jpg", + FileName = "beach.jpg", + ContentHash = "0123456789abcdef", + DateCaptured = new LocalDateTime(2019, 7, 14, 9, 30), + MediaType = MediaType.Image, + }; + } + + public DbContextOptions Options { get; } = AnichronDbContexts.InMemoryOptions(); + public User Mira { get; } = new() { Id = Guid.NewGuid(), Username = "mira", Email = "mira@example.com" }; + public User Alice { get; } = new() { Id = Guid.NewGuid(), Username = "alice", Email = "alice@example.com" }; + public UserStorageConfig MiraConfig { get; } + public MediaAsset Photo { get; } + + public async Task SeedAsync(params AssetInteraction[] interactions) + { + await using var db = new AnichronDbContext(Options); + db.Users.AddRange(Mira, Alice); + db.StorageConfigs.Add(MiraConfig); + db.MediaAssets.Add(Photo); + db.Interactions.AddRange(interactions); + await db.SaveChangesAsync(TestContext.Current.CancellationToken); + } + + public async Task ApplyAsync(User caller, InteractionChange change, IInterceptor? interceptor = null) + { + var options = interceptor is null + ? Options + : new DbContextOptionsBuilder(Options).AddInterceptors(interceptor).Options; + await using var db = new AnichronDbContext(options); + return await new EfAssetInteractionWriter(db) + .ApplyAsync(caller.Id, Photo.Id, change, TestContext.Current.CancellationToken); + } + + public async Task> OnThisDayAsync(User caller) + { + await using var db = new AnichronDbContext(Options); + return await new EfAssetViewReader(db) + .ReadAsync(caller.Id, new OnThisDay(new LocalDate(2026, 7, 14)), TestContext.Current.CancellationToken); + } + + public async Task> StoredAsync() + { + await using var db = new AnichronDbContext(Options); + return await db.Interactions.IgnoreQueryFilters().ToListAsync(TestContext.Current.CancellationToken); + } + } + + [Theory] + [InlineData(true, null, null)] + [InlineData(null, true, null)] + [InlineData(null, null, true)] + public async Task ApplyAsync_FirstWrite_CreatesTheCallersRowAndReturnsItsState(bool? starred, bool? liked, bool? hidden) + { + var library = new Library(); + await library.SeedAsync(); + + var state = await library.ApplyAsync(library.Mira, new InteractionChange(starred, liked, hidden)); + + var expected = new InteractionState(starred ?? false, liked ?? false, hidden ?? false); + state.Should().Be(expected); + var row = (await library.StoredAsync()).Should().ContainSingle().Which; + row.UserId.Should().Be(library.Mira.Id); + row.AssetId.Should().Be(library.Photo.Id); + new InteractionState(row.IsStarred, row.IsLiked, row.IsHidden).Should().Be(expected); + } + + private sealed class ConcurrentFirstWrite(Library library, string indexName, int losses = 1) : SaveChangesInterceptor + { + public int Saves { get; private set; } + + public override async ValueTask> SavingChangesAsync( + DbContextEventData eventData, InterceptionResult result, CancellationToken cancellationToken = default) + { + Saves++; + if (Saves > losses) + return result; + + await using var other = new AnichronDbContext(library.Options); + other.Interactions.Add(MirasRow(library, liked: true)); + await other.SaveChangesAsync(cancellationToken); + throw DatabaseErrors.UniqueViolation(indexName); + } + } + + private static AssetInteraction MirasRow(Library library, bool starred = false, bool liked = false, bool hidden = false) + => new() + { + Id = Guid.NewGuid(), + UserId = library.Mira.Id, + AssetId = library.Photo.Id, + IsStarred = starred, + IsLiked = liked, + IsHidden = hidden, + }; + + [Fact] + public async Task ApplyAsync_PartialChange_KeepsTheOtherFlags() + { + var library = new Library(); + await library.SeedAsync(MirasRow(library, starred: true, hidden: true)); + + var state = await library.ApplyAsync(library.Mira, new InteractionChange(Starred: null, Liked: true, Hidden: null)); + + state.Should().Be(new InteractionState(Starred: true, Liked: true, Hidden: true)); + var row = (await library.StoredAsync()).Should().ContainSingle().Which; + (row.IsStarred, row.IsLiked, row.IsHidden).Should().Be((true, true, true)); + } + + [Fact] + public async Task ApplyAsync_FalseFlag_ClearsOnlyThatFlag() + { + var library = new Library(); + await library.SeedAsync(MirasRow(library, starred: true, liked: true)); + + var state = await library.ApplyAsync(library.Mira, new InteractionChange(Starred: false, Liked: null, Hidden: null)); + + state.Should().Be(new InteractionState(Starred: false, Liked: true, Hidden: false)); + var row = (await library.StoredAsync()).Should().ContainSingle().Which; + (row.IsStarred, row.IsLiked, row.IsHidden).Should().Be((false, true, false)); + } + + [Fact] + public async Task ApplyAsync_HiddenAsset_CanBeUnhidden() + { + var library = new Library(); + await library.SeedAsync(MirasRow(library, hidden: true)); + + var state = await library.ApplyAsync(library.Mira, new InteractionChange(Starred: null, Liked: null, Hidden: false)); + + state.Should().Be(new InteractionState(Starred: false, Liked: false, Hidden: false)); + (await library.StoredAsync()).Should().ContainSingle().Which.IsHidden.Should().BeFalse(); + } + + [Fact] + public async Task ApplyAsync_EmptyChangeWithoutARow_ReturnsTheDefaultStateAndStoresNothing() + { + var library = new Library(); + await library.SeedAsync(); + + var state = await library.ApplyAsync(library.Mira, new InteractionChange(Starred: null, Liked: null, Hidden: null)); + + state.Should().Be(new InteractionState(Starred: false, Liked: false, Hidden: false)); + (await library.StoredAsync()).Should().BeEmpty(); + } + + [Fact] + public async Task ApplyAsync_EmptyChangeWithARow_ReturnsTheStoredState() + { + var library = new Library(); + await library.SeedAsync(MirasRow(library, liked: true)); + + var state = await library.ApplyAsync(library.Mira, new InteractionChange(Starred: null, Liked: null, Hidden: null)); + + state.Should().Be(new InteractionState(Starred: false, Liked: true, Hidden: false)); + } + + [Fact] + public async Task ApplyAsync_AnotherUsersAsset_ReturnsNullAndLeavesTheOwnersRowAlone() + { + var library = new Library(); + await library.SeedAsync(MirasRow(library)); + + var state = await library.ApplyAsync(library.Alice, new InteractionChange(Starred: true, Liked: true, Hidden: true)); + + state.Should().BeNull(); + var row = (await library.StoredAsync()).Should().ContainSingle().Which; + row.UserId.Should().Be(library.Mira.Id); + (row.IsStarred, row.IsLiked, row.IsHidden).Should().Be((false, false, false)); + } + + [Fact] + public async Task ApplyAsync_UnknownAsset_ReturnsNullAndStoresNothing() + { + var library = new Library(); + await library.SeedAsync(); + await using var db = new AnichronDbContext(library.Options); + + var state = await new EfAssetInteractionWriter(db).ApplyAsync( + library.Mira.Id, Guid.NewGuid(), new InteractionChange(Starred: true, Liked: null, Hidden: null), + TestContext.Current.CancellationToken); + + state.Should().BeNull(); + (await library.StoredAsync()).Should().BeEmpty(); + } + + [Fact] + public async Task ApplyAsync_SoftDeletedAsset_ReturnsNull() + { + var library = new Library(); + library.Photo.IsSoftDeleted = true; + await library.SeedAsync(); + + var state = await library.ApplyAsync(library.Mira, new InteractionChange(Starred: true, Liked: null, Hidden: null)); + + state.Should().BeNull(); + (await library.StoredAsync()).Should().BeEmpty(); + } + + [Fact] + public async Task ApplyAsync_AssetOfAnInactiveConfig_ReturnsNull() + { + var library = new Library(); + library.MiraConfig.IsActive = false; + await library.SeedAsync(); + + var state = await library.ApplyAsync(library.Mira, new InteractionChange(Starred: true, Liked: null, Hidden: null)); + + state.Should().BeNull(); + (await library.StoredAsync()).Should().BeEmpty(); + } + + [Fact] + public async Task ApplyAsync_LosingTheFirstWriteRace_AppliesTheChangeToTheWinnersRow() + { + var library = new Library(); + await library.SeedAsync(); + var race = new ConcurrentFirstWrite(library, InteractionIndexNames.OnePerUserAndAsset); + + var state = await library.ApplyAsync(library.Mira, new InteractionChange(Starred: true, Liked: null, Hidden: null), race); + + state.Should().Be(new InteractionState(Starred: true, Liked: true, Hidden: false)); + var row = (await library.StoredAsync()).Should().ContainSingle().Which; + (row.IsStarred, row.IsLiked, row.IsHidden).Should().Be((true, true, false)); + race.Saves.Should().Be(2); + } + + [Fact] + public async Task ApplyAsync_LosingTwice_Throws() + { + var library = new Library(); + await library.SeedAsync(); + var race = new ConcurrentFirstWrite(library, InteractionIndexNames.OnePerUserAndAsset, losses: 2); + + var apply = () => library.ApplyAsync(library.Mira, new InteractionChange(Starred: true, Liked: null, Hidden: null), race); + + await apply.Should().ThrowAsync(); + race.Saves.Should().Be(2); + } + + [Fact] + public async Task ApplyAsync_UniqueViolationOfAnotherIndex_IsNotRetried() + { + var library = new Library(); + await library.SeedAsync(); + var race = new ConcurrentFirstWrite(library, "ix_some_other_index"); + + var apply = () => library.ApplyAsync(library.Mira, new InteractionChange(Starred: true, Liked: null, Hidden: null), race); + + await apply.Should().ThrowAsync(); + race.Saves.Should().Be(1); + } + + // ========================================================================== + // First caller: what the caller's On This Day read sees after a write + // ========================================================================== + + [Fact] + public async Task ApplyAsync_StarAndLike_AreReadBackOnThisDay() + { + var library = new Library(); + await library.SeedAsync(); + + await library.ApplyAsync(library.Mira, new InteractionChange(Starred: true, Liked: true, Hidden: null)); + + var view = (await library.OnThisDayAsync(library.Mira)).Should().ContainSingle().Which; + view.Id.Should().Be(library.Photo.Id); + (view.IsStarred, view.IsLiked).Should().Be((true, true)); + } + + [Fact] + public async Task ApplyAsync_Hide_DropsTheAssetFromOnThisDayAndUnhideBringsItBack() + { + var library = new Library(); + await library.SeedAsync(); + + await library.ApplyAsync(library.Mira, new InteractionChange(Starred: null, Liked: null, Hidden: true)); + (await library.OnThisDayAsync(library.Mira)).Should().BeEmpty(); + + await library.ApplyAsync(library.Mira, new InteractionChange(Starred: null, Liked: null, Hidden: false)); + (await library.OnThisDayAsync(library.Mira)).Should().ContainSingle().Which.Id.Should().Be(library.Photo.Id); + } +} diff --git a/src/Anichron.Core/Data/InteractionIndexNames.cs b/src/Anichron.Core/Data/InteractionIndexNames.cs new file mode 100644 index 00000000..1993bcd1 --- /dev/null +++ b/src/Anichron.Core/Data/InteractionIndexNames.cs @@ -0,0 +1,6 @@ +namespace Anichron.Core.Data; + +public static class InteractionIndexNames +{ + public const string OnePerUserAndAsset = "IX_Interactions_UserId_AssetId"; +} diff --git a/src/Anichron.Core/Data/Repository/AssetInteractionWriter.cs b/src/Anichron.Core/Data/Repository/AssetInteractionWriter.cs new file mode 100644 index 00000000..e87854a6 --- /dev/null +++ b/src/Anichron.Core/Data/Repository/AssetInteractionWriter.cs @@ -0,0 +1,62 @@ +using Anichron.Core.Domain; +using Microsoft.EntityFrameworkCore; + +namespace Anichron.Core.Data.Repository; + +public sealed record InteractionChange(bool? Starred, bool? Liked, bool? Hidden); + +public sealed record InteractionState(bool Starred, bool Liked, bool Hidden); + +/// +/// Applies a change to the caller's own interaction with an asset they can see; a null flag keeps its +/// stored value, and the row is created on the first change. Returns null when the asset is not visible +/// to the caller. +/// +public interface IAssetInteractionWriter +{ + Task ApplyAsync(Guid userId, Guid assetId, InteractionChange change, CancellationToken ct); +} + +public sealed class EfAssetInteractionWriter(AnichronDbContext db) : IAssetInteractionWriter +{ + private sealed record Target(AssetInteraction? Interaction); + + public async Task ApplyAsync(Guid userId, Guid assetId, InteractionChange change, CancellationToken ct) + { + try + { + return await TryApplyAsync(userId, assetId, change, ct); + } + catch (DbUpdateException ex) when (ex.IsUniqueViolation(InteractionIndexNames.OnePerUserAndAsset)) + { + db.ChangeTracker.Clear(); + return await TryApplyAsync(userId, assetId, change, ct); + } + } + + private async Task TryApplyAsync(Guid userId, Guid assetId, InteractionChange change, CancellationToken ct) + { + var target = await db.MediaAssets.VisibleTo(userId) + .Where(a => a.Id == assetId) + .Select(a => new Target(a.Interactions.FirstOrDefault(i => i.UserId == userId))) + .FirstOrDefaultAsync(ct); + if (target is null) + return null; + + var interaction = target.Interaction; + if (interaction is null) + { + if (change is { Starred: null, Liked: null, Hidden: null }) + return new InteractionState(Starred: false, Liked: false, Hidden: false); + + interaction = new AssetInteraction { UserId = userId, AssetId = assetId }; + db.Interactions.Add(interaction); + } + + interaction.IsStarred = change.Starred ?? interaction.IsStarred; + interaction.IsLiked = change.Liked ?? interaction.IsLiked; + interaction.IsHidden = change.Hidden ?? interaction.IsHidden; + await db.SaveChangesAsync(ct); + return new InteractionState(interaction.IsStarred, interaction.IsLiked, interaction.IsHidden); + } +} From 3b1231d83cae3167de8b5449139d971a42b02b15 Mon Sep 17 00:00:00 2001 From: bitbiter-dev Date: Tue, 20 Oct 2026 14:08:38 +1300 Subject: [PATCH 2/2] fix(api): A PATCH with no body is no change; pin the writer's SQL (#46) Review follow-ups: a bodiless PATCH now returns 200 with the current state instead of 400, as decided for an empty body. The writer's load query is pinned by ToQueryString on Npgsql (VisibleTo and the caller's row, no hidden rule), the index constant is wired with HasDatabaseName like its siblings, and the view mapping reuses the InteractionState overload. Co-Authored-By: Claude Opus 5.5 --- .../Endpoints/MediaEndpointsTests.cs | 20 +++++++++++++ src/Anichron.API/Endpoints/AssetDtoMapper.cs | 2 +- src/Anichron.API/Endpoints/MediaEndpoints.cs | 4 +-- .../Repository/AssetInteractionWriterTests.cs | 29 ++++++++++++++----- src/Anichron.Core/Data/AnichronDbContext.cs | 2 +- .../Data/Repository/AssetInteractionWriter.cs | 16 +++++----- 6 files changed, 55 insertions(+), 18 deletions(-) diff --git a/src/Anichron.API.Tests.Unit/Endpoints/MediaEndpointsTests.cs b/src/Anichron.API.Tests.Unit/Endpoints/MediaEndpointsTests.cs index af603267..c51cb3a2 100644 --- a/src/Anichron.API.Tests.Unit/Endpoints/MediaEndpointsTests.cs +++ b/src/Anichron.API.Tests.Unit/Endpoints/MediaEndpointsTests.cs @@ -261,6 +261,26 @@ await fixture.Interactions.Received(1).ApplyAsync( UserId, AssetId, new InteractionChange(null, null, null), Arg.Any()); } + [Fact] + public async Task PatchInteraction_NoBodyAtAll_PassesNoChangeAsync() + { + var fixture = new TestFixture(); + fixture.Interactions.ApplyAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(new InteractionState(false, true, false)); + await using var app = await fixture.StartAsync(); + using var client = app.Client; + using var request = new HttpRequestMessage(HttpMethod.Patch, $"/api/v1/assets/{AssetId}/interaction"); + request.Headers.Add(AuthenticatedTestApp.UserHeader, UserId.ToString()); + + using var response = await client.SendAsync(request, TestContext.Current.CancellationToken); + + response.StatusCode.Should().Be(HttpStatusCode.OK); + (await response.Content.ReadFromJsonAsync(TestContext.Current.CancellationToken)) + .Should().Be(new AssetInteractionDto(false, true, false, 2.0)); + await fixture.Interactions.Received(1).ApplyAsync( + UserId, AssetId, new InteractionChange(null, null, null), Arg.Any()); + } + [Fact] public async Task PatchInteraction_BodyNamingAnotherUser_WritesForTheCallerAsync() { diff --git a/src/Anichron.API/Endpoints/AssetDtoMapper.cs b/src/Anichron.API/Endpoints/AssetDtoMapper.cs index 8c794005..d71becc7 100644 --- a/src/Anichron.API/Endpoints/AssetDtoMapper.cs +++ b/src/Anichron.API/Endpoints/AssetDtoMapper.cs @@ -27,7 +27,7 @@ internal static AssetDto ToDto(AssetView view) view is { PairedMovHasVideo: true, PairedMovId: { } movId } ? ProxyKinds.UrlOf(movId, ProxyType.WebVideo) : null, - new AssetInteractionDto(view.IsStarred, view.IsLiked, Hidden: false, FlashbackWeight.For(view.IsLiked))); + ToDto(new InteractionState(view.IsStarred, view.IsLiked, Hidden: false))); } internal static AssetInteractionDto ToDto(InteractionState state) diff --git a/src/Anichron.API/Endpoints/MediaEndpoints.cs b/src/Anichron.API/Endpoints/MediaEndpoints.cs index dcf27871..97c15e83 100644 --- a/src/Anichron.API/Endpoints/MediaEndpoints.cs +++ b/src/Anichron.API/Endpoints/MediaEndpoints.cs @@ -55,7 +55,7 @@ internal static async Task GetProxyAsync( internal static async Task PatchInteractionAsync( Guid assetId, - PatchInteractionRequest request, + PatchInteractionRequest? request, ClaimsPrincipal user, [FromServices] IAssetInteractionWriter interactions, CancellationToken ct) @@ -63,7 +63,7 @@ internal static async Task PatchInteractionAsync( if (!Guid.TryParse(user.FindFirstValue(ClaimTypes.NameIdentifier), out var userId)) return Results.Unauthorized(); - var change = new InteractionChange(request.Starred, request.Liked, request.Hidden); + var change = new InteractionChange(request?.Starred, request?.Liked, request?.Hidden); return await interactions.ApplyAsync(userId, assetId, change, ct) is { } state ? Results.Ok(AssetDtoMapper.ToDto(state)) : Results.NotFound(); diff --git a/src/Anichron.Core.Tests.Unit/Data/Repository/AssetInteractionWriterTests.cs b/src/Anichron.Core.Tests.Unit/Data/Repository/AssetInteractionWriterTests.cs index 3d481e31..c9bf5994 100644 --- a/src/Anichron.Core.Tests.Unit/Data/Repository/AssetInteractionWriterTests.cs +++ b/src/Anichron.Core.Tests.Unit/Data/Repository/AssetInteractionWriterTests.cs @@ -43,14 +43,15 @@ public async Task SeedAsync(params AssetInteraction[] interactions) await db.SaveChangesAsync(TestContext.Current.CancellationToken); } - public async Task ApplyAsync(User caller, InteractionChange change, IInterceptor? interceptor = null) + public async Task ApplyAsync( + User caller, InteractionChange change, IInterceptor? interceptor = null, Guid? assetId = null) { var options = interceptor is null ? Options : new DbContextOptionsBuilder(Options).AddInterceptors(interceptor).Options; await using var db = new AnichronDbContext(options); return await new EfAssetInteractionWriter(db) - .ApplyAsync(caller.Id, Photo.Id, change, TestContext.Current.CancellationToken); + .ApplyAsync(caller.Id, assetId ?? Photo.Id, change, TestContext.Current.CancellationToken); } public async Task> OnThisDayAsync(User caller) @@ -195,16 +196,30 @@ public async Task ApplyAsync_UnknownAsset_ReturnsNullAndStoresNothing() { var library = new Library(); await library.SeedAsync(); - await using var db = new AnichronDbContext(library.Options); - var state = await new EfAssetInteractionWriter(db).ApplyAsync( - library.Mira.Id, Guid.NewGuid(), new InteractionChange(Starred: true, Liked: null, Hidden: null), - TestContext.Current.CancellationToken); + var state = await library.ApplyAsync( + library.Mira, new InteractionChange(Starred: true, Liked: null, Hidden: null), assetId: Guid.NewGuid()); state.Should().BeNull(); (await library.StoredAsync()).Should().BeEmpty(); } + [Fact] + public void Query_FiltersVisibilityAndTheInteractionOnTheCaller() + { + using var db = AnichronDbContexts.NpgsqlModel(); + + var sql = EfAssetInteractionWriter.Query(db, Guid.NewGuid(), Guid.NewGuid()).ToQueryString(); + + sql.Should().Contain("JOIN \"StorageConfigs\"") + .And.Contain("\"IsActive\"") + .And.Contain("\"IsSoftDeleted\"") + .And.Contain("FROM \"Interactions\"") + .And.NotContain("EXISTS", "a hidden asset must stay writable, so it can be unhidden"); + sql.Split("\"UserId\" = @").Should().HaveCount(3, + "the owned-config check and the interaction lookup each filter on the caller"); + } + [Fact] public async Task ApplyAsync_SoftDeletedAsset_ReturnsNull() { @@ -273,7 +288,7 @@ public async Task ApplyAsync_UniqueViolationOfAnotherIndex_IsNotRetried() } // ========================================================================== - // First caller: what the caller's On This Day read sees after a write + // Read back On This Day // ========================================================================== [Fact] diff --git a/src/Anichron.Core/Data/AnichronDbContext.cs b/src/Anichron.Core/Data/AnichronDbContext.cs index f2ad85f0..16dc8740 100644 --- a/src/Anichron.Core/Data/AnichronDbContext.cs +++ b/src/Anichron.Core/Data/AnichronDbContext.cs @@ -49,7 +49,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder) + "the same file each get their own row. Drives flashback weighting — hidden " + "excludes, starred always includes and sorts first, liked raises DisplayWeight." )); - entity.HasIndex(i => new { i.UserId, i.AssetId }).IsUnique(); + entity.HasIndex(i => new { i.UserId, i.AssetId }).IsUnique().HasDatabaseName(InteractionIndexNames.OnePerUserAndAsset); entity.HasQueryFilter(i => !i.Asset.IsSoftDeleted); }); diff --git a/src/Anichron.Core/Data/Repository/AssetInteractionWriter.cs b/src/Anichron.Core/Data/Repository/AssetInteractionWriter.cs index e87854a6..3258c092 100644 --- a/src/Anichron.Core/Data/Repository/AssetInteractionWriter.cs +++ b/src/Anichron.Core/Data/Repository/AssetInteractionWriter.cs @@ -19,7 +19,7 @@ public interface IAssetInteractionWriter public sealed class EfAssetInteractionWriter(AnichronDbContext db) : IAssetInteractionWriter { - private sealed record Target(AssetInteraction? Interaction); + internal sealed record VisibleAsset(AssetInteraction? Interaction); public async Task ApplyAsync(Guid userId, Guid assetId, InteractionChange change, CancellationToken ct) { @@ -36,14 +36,11 @@ private sealed record Target(AssetInteraction? Interaction); private async Task TryApplyAsync(Guid userId, Guid assetId, InteractionChange change, CancellationToken ct) { - var target = await db.MediaAssets.VisibleTo(userId) - .Where(a => a.Id == assetId) - .Select(a => new Target(a.Interactions.FirstOrDefault(i => i.UserId == userId))) - .FirstOrDefaultAsync(ct); - if (target is null) + var asset = await Query(db, userId, assetId).FirstOrDefaultAsync(ct); + if (asset is null) return null; - var interaction = target.Interaction; + var interaction = asset.Interaction; if (interaction is null) { if (change is { Starred: null, Liked: null, Hidden: null }) @@ -59,4 +56,9 @@ private sealed record Target(AssetInteraction? Interaction); await db.SaveChangesAsync(ct); return new InteractionState(interaction.IsStarred, interaction.IsLiked, interaction.IsHidden); } + + internal static IQueryable Query(AnichronDbContext db, Guid userId, Guid assetId) + => db.MediaAssets.VisibleTo(userId) + .Where(a => a.Id == assetId) + .Select(a => new VisibleAsset(a.Interactions.FirstOrDefault(i => i.UserId == userId))); }