fix: Prevent "null" identifier collisions from malformed drink JSON - #339
Conversation
"null" identifier collisions from malformed drink JSON
There was a problem hiding this comment.
Pull request overview
Hardens drink JSON parsing and downstream API parsing to prevent key collisions and incorrect lookups when upstream records have missing id/name fields (previously coerced into the literal "null").
Changes:
- Update
Producer.fromJson/Product.fromJsonto avoid.toString()on nullableid/name, mapping missing values to''. - Filter invalid records in
BeerApiService.parseProducersby skipping producers/products with empty IDs. - Add regression tests covering null ID/name parsing and API filtering behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| lib/models/drink.dart | Adjusts model JSON parsing for id/name to avoid "null" and reduce collisions. |
| lib/services/beer_api_service.dart | Skips producers/products with empty IDs when flattening to Drink list. |
| test/models_test.dart | Adds regression tests for null id/name mapping to ''. |
| test/beer_api_service_test.dart | Adds tests ensuring API parsing skips invalid producers/products missing IDs. |
| return Producer( | ||
| id: json['id'].toString(), | ||
| name: json['name'].toString(), | ||
| id: (json['id'] as String?) ?? '', | ||
| name: (json['name'] as String?) ?? '', | ||
| location: (json['location'] ?? '').toString(), |
| return Product( | ||
| id: json['id'].toString(), | ||
| name: json['name'].toString(), | ||
| id: (json['id'] as String?) ?? '', | ||
| name: (json['name'] as String?) ?? '', | ||
| category: (json['category'] ?? 'beer').toString(), |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
🚀 Cloudflare Pages PreviewYour preview deployment is ready! Preview URL: https://copilot-fix-missing-id-name.staging-cambeerfestival.pages.dev This preview will be automatically updated when you push new commits to this PR. |
|
@copilot fix pr lint failure |
I checked the failing PR Lint run: it’s failing the semantic-title check, not code lint. Please rename the PR title to a Conventional Commits format, for example:
After retitling, PR Lint should pass. |
"null" identifier collisions from malformed drink JSON"null" identifier collisions from malformed drink JSON
Producer.fromJsonandProduct.fromJsonwere coercing nullableid/namefields with.toString(), which turned missing values into the literal"null". That allowed multiple bad records to share the same ID, causing key collisions in lists and incorrect ID-based lookups.Model parsing hardening (
lib/models/drink.dart).toString()coercion with nullable string casts + empty fallback:id: (json['id'] as String?) ?? ''name: (json['name'] as String?) ?? ''"null"string IDs/names.Repository-layer record filtering (
lib/services/beer_api_service.dart)parseProducers, now skips:producer.idproduct.idRegression coverage
id/nameparsing to''.