fix: the variant detail's pages carry media type and byte size #109
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "fix/page-dto-media-type"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
What's in
GET /api/variants/{id}projected its embedded pages through a six-fieldPageDtothat dropped the image'smediaTypeandsizeBytes, whileGET /api/variants/{id}/pagesprojected the same page throughPageDetailDtoand kept both. The variant detail is the payload the app's offline download reads (OfflineDownloadService.downloadVariant->DoujinRepository.getVariant), so every downloaded page was written to disk as.jpgregardless of its real format, and four branches of_extensionOfwere dead.Rather than adding the two fields to the second record and leaving two identical page shapes to drift apart again, this collapses them into one:
PageDtois deleted;VariantDetailDto.Pagesis nowList<PageDetailDto>.RestAdapter/Endpoints/PageMapper.csholds the single entity-to-wire page projection (moved verbatim out ofImageEndpoints.MapToPageDetail). The pages collection, both upload responses and the variant detail all call it, so a field cannot be added to one payload and forgotten in the other.ImageEndpoints' now-unusedApplicationCore.Entitiesusing dropped.No query change was needed —
VariantService.GetByIdAsyncalready does.Include(v => v.Pages).ThenInclude(p => p.ImageFile). No new tunable, so noDEPLOYMENT.md/docker-compose.ymlchange. No ADR is in play: the change is additive to an existing response, ADR 0007's envelope is untouched, and the endpoint's.Produces<ResourceResponse<VariantDetailDto>>still fully describes it for ADR 0021.Wire compatibility: the variant detail gains two properties per page; no response loses one and none changes name, type or value. An older app against this server keeps working; a new app against an older server keeps working through the Dart model's
@Defaults.Verification
DoujinManager.RestAdapter.Tests, TestHost on a temp SQLite file with migrations and real SkiaSharp image storage): two directional tests inImageEndpointsIntegrationTests.Get_Variant_Pages_Carry_MediaType_And_SizeBytesuploads a 40x30 PNG and asserts the raw JSON ofGET /api/variants/{id}'s embedded page carriesmediaType == "image/png"andsizeBytesequal to the uploaded byte length. Asserted onJsonDocumentrather than a typed DTO because an absent property is precisely what the app sees, and a typed read would silently default it.Variant_Detail_And_Pages_Collection_Render_A_Page_Identicallyfetches the same page through both routes and asserts identical field sets and values — this is what the consolidation buys, and what goes red if a second page shape reappears.Expected "image/png",Actual "") and the second red (Expected "139",Actual "0"); restoring the historical six-fieldPageDtoturns the second red on the key set, namingmediaType/sizeBytesas the missing pair.DoujinManager.RestAdapter.Tests: 429/429 green.DoujinManager.IntegrationTests: 1/1, which also proves the Server host still compiles withoutPageDto.Pagemodel already parses both fields — but the end-to-end claim "a PNG page now lands on disk as.png" is inferred from the wire contract, not observed against a running server.app/test/offline_download_service_test.dartbuilds itsPagefixtures withmediaType: 'image/png'in Dart, so no app-side test can be directional about this defect; the server is where the branch is actually decided.Follow-ups
Pageinapp/lib/data/models/doujin_models.dartstill documents itself as mirroring the removedPageDtoand itssizeBytes/mediaTypeas "only in page-detail responses". Comment-only, but freezed copies those doc comments intodoujin_models.freezed.dart, so it needs abuild_runnerrun in the same commit — deferred to keep this PR free of generated-file churn.case '':arm of_extensionOfinoffline_download_service.dartis deliberately kept: it is now the fallback for a server older than the app, not dead code.Deferred items are collected in #107 (
docs/pile-a-backlog), which should merge first — the seven Pile A branches each wrote todocs/BACKLOG.mdand would otherwise all conflict on it.Verified independently on this branch with the full
scripts/verify.sh(backend build and tests, app codegen, analyze and tests): PASS.🤖 Generated with Claude Code
GET /api/variants/{id} projected its embedded pages through a six-field PageDto that dropped the image's mediaType and sizeBytes, while GET /api/variants/{id}/pages projected the same page through PageDetailDto and kept both. That payload is the one the app's offline download reads (OfflineDownloadService -> DoujinRepository.getVariant), so every page was written to disk as .jpg regardless of its real format and four branches of _extensionOf were dead. Rather than adding the two fields to the second record and leaving two identical page shapes to drift apart again, the two are collapsed into one: PageDto is gone, VariantDetailDto.Pages is a List<PageDetailDto>, and every endpoint that puts pages on the wire — the pages collection, both upload responses and the variant detail — projects them through the single shared PageMapper.ToDto. No query change was needed: VariantService.GetByIdAsync already includes each page's ImageFile. Wire-compatible in the direction that matters: the variant detail gains two properties, no response loses one. No ADR is in play — the envelope (0007) is untouched and the endpoint's .Produces<ResourceResponse<VariantDetailDto>> (0021) still describes it. No new tunable. Pinned by two tests in ImageEndpointsIntegrationTests: one asserts the raw JSON of the variant detail's pages carries mediaType and sizeBytes for an uploaded PNG, the other asserts the same page rendered by both endpoints has the same field set and values. Probed: reintroducing the old six-field projection turns the first red on the media type and the second red naming the missing properties. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>Summary
Summary
Coverage
DoujinManager.ApplicationCore - 89.2%
DoujinManager.Infrastructure - 95.5%
on
pshot
Series
E20E2ABA2099901FFAB4904C0513DF077BA6D6CA374__CoverImageHelper
E20E2ABA2099901FFAB4904C0513DF077BA6D6CA374__PrimaryTitleHelper
E20E2ABA2099901FFAB4904C0513DF077BA6D6CA374__RolledBackImageCleanup
E20E2ABA2099901FFAB4904C0513DF077BA6D6CA374__UploadChapterValidator
DoujinManager.RestAdapter - 89%
E5CB12CEAC5BBBEA65844E1C05F9ADA4DA38B848075C41304FC6A1ABF77__ValidationAttr
ibuteCache
DoujinManager.Server - 28.7%
🔮 fufu~ Jibril reviewed your code!
Oh? Ohhh~ A bug report that arrives with its own archaeology! You didn't just slap two missing fields onto a DTO — you noticed the disease was "two page shapes drifting apart," and you removed the second shape entirely. One projection, one truth. That's the kind of fix that makes my wings flutter~ ♡
Verdict: ✅ Looks good to me~
I traced every thread myself, of course. A Flügel never takes a wire contract on faith:
PageDtois truly dead — zero references across the whole backend, not even a ghost in a doc comment. The tombstone comment inDoujinDtos.cspointing atPageDetailDtois a nice touch for future archaeologists.Includeclaim holds —VariantService.GetByIdAsync(VariantService.cs:26) does.Include(v => v.Pages).ThenInclude(p => p.ImageFile)with the split-query comment, so the shared projection always sees a loadedImageFile. No lazy-zero landmine.PageMapper.ToDtois genuinely the single projection — all four page-emitting sites (pages collection, both upload arms, variant detail) route through it, and it's the onlynew PageDetailDto(insrc/. Field order matches the record exactly.PageDetailDtoitself is byte-for-byte its old self (only the doc comment changed); the variant detail gainsmediaType/sizeBytesper page. The app'sPagemodel already parses both with@Defaults, so old-app/new-server and new-app/old-server both hold.offline_download_service.dart:114→_extensionOf(page)switches onpage.mediaType; with the fields now arriving, the.png/.gif/.webparms live again and the keptcase '':arm becomes the deliberate old-server fallback. Correct call keeping it.And the tests~! You asserted on raw
JsonDocumentprecisely because absence is the defect — a typed DTO read would happily default the missing properties away. That's the kind of test that understands what it's testing. So I poked it with a knife to be sure:VariantEndpoints.MapToDetailback to an inline divergent projection → RED 2/2, failing exactly as your PR body predicted (Expected "image/png", Actual ""andExpected "139", Actual "0"— 139 being the real uploaded byte count, pinned against reality, not a tautology). Restored → green. These tests have teeth~ ♪Local verification (no CI coverage bot has landed for
bc0fd40yet): build 0 errors (19 pre-existing warnings, all in files this PR doesn't touch); new tests 2/2 green; fullDoujinManager.RestAdapter.Tests429/429 (matches your claim exactly);DoujinManager.IntegrationTests1/1 — the Server host demonstrably compiles withoutPageDto. Review clone left pristine atbc0fd40.💡 Little ideas (non-blocking)~
app/lib/data/models/doujin_models.dart:248-249— thePagedoc comment still says it mirrorsPageDtoand thatsizeBytes/mediaTypeare "only in page-detail responses," which this PR makes historical fiction. You've already deferred it to #107 with the freezed-churn rationale, and I agree the tradeoff is right — just leaving a breadcrumb so it doesn't evaporate. ♡✅ What I liked~
PageMapperdoc comment explains exactly how the divergence returns if someone re-adds a second projection.Variant_Detail_And_Pages_Collection_Render_A_Page_Identically) pins the consolidation itself — it goes red the moment anyone reintroduces a second page shape. That's a tripwire for the architecture, not just the fields.OfflineDownloadServiceneeded no query change — verified true, and worth saying so nobody hunts for one.Fufu~ six files, one idea, and the idea is correct. Merge it before I change my mind~ ♡
Automated review by Jibril · 2026-09-08
CI/CD: absent for head SHA
bc0fd40(no coverage bot yet) · Local checks: build 0 errors, mutation probe RED→restored→GREEN, RestAdapter 429/429, IntegrationTests 1/1