feat: Phase 10e — data layer models and API repository #31
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/flutter-data-models"
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?
Phase 10e: Data Layer and API Models
Freezed models matching the backend JSON envelopes, Dart enums matching the C# wire format, and the DoujinApiRepository implementation.
Key Finding: Enum Wire Format
The backend (ASP.NET Core default JSON serializer) serializes enums as integers (
"readingDirection": 1,"kind": 0), not strings. Sending string enums results in 500 errors. All Dart enum values use@JsonValue(int)annotations to match the exact wire representation. This was empirically verified by running the backend and capturing the HTTP response.What's included
Enums (
domain/entities/enums.dart):ReadingDirection,TitleKind,VariantKind,PersonRole—@JsonValue(int)for integer wire formatEnvelope models (
data/models/envelope.dart):ResourceResponse<T>,CollectionResponse<T>— generic,genericArgumentFactories: trueHypermediaLink,HypermediaAction,PageInfoDomain models (
data/models/doujin_models.dart) — 12 freezed models:DoujinSummary,DoujinDetail,Title,VariantSummary,VariantDetail,Chapter,Page,Tag,Person,Circle,DoujinPerson,SearchResultSearchQuery (
data/models/search_query.dart):POST /api/doujins/searchDoujinRepository (
domain/repositories/doujin_repository.dart):listDoujins,getDoujin,search+Paginated<T>typedefDoujinApiRepository (
data/repositories/doujin_api_repository.dart):Verification
flutter analyze: No issues found ✅flutter test: 56/56 passed (34 existing + 22 new) ✅Domain enums (matching backend C# wire format): - ReadingDirection, TitleKind, VariantKind, PersonRole - @JsonValue(int) annotations — backend serializes enums as integers, not strings (empirically verified by running the backend) Envelope models (freezed + genericArgumentFactories): - ResourceResponse<T>, CollectionResponse<T> for hypermedia envelopes - HypermediaLink, HypermediaAction, PageInfo Domain models (freezed + json_serializable): - DoujinSummary, DoujinDetail, Title, VariantSummary, VariantDetail, Chapter, Page, Tag, Person, Circle, DoujinPerson, SearchResult - All match backend JSON property names exactly - GUIDs as String, timestamps as DateTime, nullable fields as nullable SearchQuery — request body builder for POST /api/doujins/search DoujinRepository interface — listDoujins, getDoujin, search DoujinApiRepository — Dio-backed implementation: - GET /api/doujins?page={page}&pageSize={pageSize} - GET /api/doujins/{id} - POST /api/doujins/search - Parses hypermedia envelopes into domain models + page info Tests: 22 new (models + repository), 56 total pass flutter analyze: 0 issuesFlutter Coverage
Total: 72.2% (455 of 630)
🤖 Hermes automated review: minor comments
Reviewed head
4186c537(13 new files, +6214 / −0). I cross-checked every hand-written Dart model against the backend C# DTOs (DoujinDtos.cs,MetadataDtos.cs,SearchDtos.cs,ResponseEnvelope.cs) and the C# enum definitions to verify the wire-format contract.What was verified ✅
Wire-format contract (the critical part of this PR):
@JsonValue(0..5)forReadingDirection,TitleKind,VariantKind,PersonRoleall align with the C# enum declarations (Artist=0 … Other=5, etc.).EnvelopeJsonOptions.Defaultregisters noJsonStringEnumConverter, so System.Text.Json serializes enums as integers by default — the PR's central claim is correct. The PR description's note that sending string enums → 500 is consistent with this.personRole/variantKindcorrectly typedString?. The backendSearchDoujinsDtodeclares these asstring?(not the enum types), matching Dart. If these had been Dart enums, the search filter body would have serialized to integers and silently failed to match the backend's string parsing.[JsonPropertyName]attributes on the C# DTOs (camelCase).data/page/links/actions,PageInfofields).GuidandDateTime.Repository implementation (
doujin_api_repository.dart):queryParametersmap is used). No path-injection risk:getDoujinbuilds/api/doujins/$idand the backend route constraint/{id:guid}validates format at routing time (malformed id → 404).PageInfofallback when the envelope omitspageis harmless — the backend always sets it (verified: everyCollectionResponseconstruction across all 6 endpoint files passes a non-nullPageInfo).Static security scan: clean — no hardcoded secrets, shell injection,
eval/exec, pickle, SQL injection, or insecure-transport patterns.Minor non-blocking notes
listDoujins/searchtotalItems fallback is imprecise (doujin_api_repository.dart:20,:69). Whenenvelope.pageis null, the synthesizedPageInfousestotalItems: envelope.data.length. This is the current page size, not the true total. Since the backend always populatespage, this never triggers in practice — but if it ever did, the UI would show "page 1 of 1" for a multi-page result. Low risk; consider documenting as a fallback-only path or logging when it occurs.doujin_models.g.dart/envelope.g.dartcoverage is low (50.8% / 50.7% per CI). This is expected for generatedfromJson/toJsoncode (the_$XFromJsonconstructors have many branches for nullable/defaulted fields). Not actionable, just noting it explains the 72.2% total.SearchQuery.toJson()includes null fields (search_query.dart). The testmodels_test.dartassertsjson.containsKey('excludeTags')is true for unset fields. This is harmless (backend ignores nulls) and arguably more debuggable, but it diverges from a "sparse PATCH" style. The backendSearchDoujinsDtotreats null filters as "no filter" either way, so no correctness impact.No security issues. No logic errors. No blocking findings.
Verification
CI/CD: Flutter coverage comment #491 is current for head
4186c537(72.2%, 455/630 lines) and was generated at 23:57:34Z — matching the PR head. Theflutter-ci.ymljob posts coverage only on a successfulflutter analyze+flutter test, which the PR body confirms (56/56 passed: 34 existing + 22 new). Local build/test skipped per CI-evidence policy.Note: backend coverage comment was not posted for this PR (head branch is Flutter-only changes), which is expected — the C# backend is unchanged here.
Automated daily review. I never merge PRs. This is a PR conversation comment with file references; it is not a formal Forgejo review-approval state (the current MCP integration cannot create those).
Thanks for the thorough review — the wire-format verification is much appreciated.
All 3 minor notes are acknowledged as non-actionable:
page. Will add adebugPrintif the fallback ever triggers.fromJson/toJsonbranches; will exclude*.g.dartfrom coverage reporting in Phase 10f (theanalysis_options.yamlalready excludes them from analysis).No blocking findings — ready for merge.