-
-
Notifications
You must be signed in to change notification settings - Fork 466
fix: container parsing #948
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughTwo utility modules were updated to add defensive null checks and type conversion. Image parsing now uses helper functions to safely convert JSON values, and container stats processing now guards against null items and filters by container ID length. Changes
Sequence DiagramsequenceDiagram
autonumber
participant Provider as Provider
participant Parser as Parser
participant Items as Items List
rect rgb(200, 220, 240)
Note over Provider: Old flow (crashes on null)
Provider->>Parser: parse stats
Parser->>Items: iterate over state.items!
Items--xParser: null dereference → crash
end
rect rgb(220, 240, 200)
Note over Provider: New flow (handles null safely)
Provider->>Parser: parse stats
Parser->>Items: check if items null?
alt items is null
Parser-->>Provider: early return
else items exists
loop for each item
Parser->>Items: check id.length >= 5
alt valid id length
Parser->>Parser: process item
else short id
Parser->>Parser: skip item
end
end
end
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Defensive improvements with consistent patterns across both files. Straightforward null-checks and type-conversion helpers require verification that they integrate cleanly with existing parsing logic and don't mask valid edge cases. Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lib/data/model/container/image.dart(2 hunks)lib/data/provider/container.dart(2 hunks)
🔇 Additional comments (4)
lib/data/model/container/image.dart (2)
39-46: LGTM! Safer JSON parsing with helper functions.The refactored parsing using
_asStringand_asInthelpers improves robustness against malformed JSON data from Podman, directly addressing the parsing failures described in issue #946.
129-134: LGTM! Correct nullable int conversion.The
_asInthelper correctly preserves null values and handles multiple numeric types, aligning well with the nullableint?fields inPodmanImg.lib/data/provider/container.dart (2)
176-178: Good defensive check! Fixes the null-check crash.Extracting
state.itemsand returning early if null prevents the "Null check operator used on a null value" error reported in issue #946. This is the correct fix since there are no container items to update with stats.
182-182: LGTM! Prevents substring out-of-bounds.Skipping containers with
id.length < 5is a sensible safeguard since line 186 usessubstring(0, 5)for matching. This prevents crashes on malformed or abbreviated container IDs that might appear in Podman output.
lib/data/model/container/image.dart
Outdated
| String _asString(dynamic val) { | ||
| if (val == null) return ''; | ||
| if (val is String) return val; | ||
| return val.toString(); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider preserving null instead of empty string.
The _asString helper returns an empty string for null values, but PodmanImg fields (repository, tag, id) are declared as nullable String?. This changes the semantics: a missing JSON field now becomes '' instead of null. Downstream code that distinguishes between null (missing) and empty (present but empty) may behave unexpectedly.
Consider preserving null:
String _asString(dynamic val) {
- if (val == null) return '';
+ if (val == null) return null;
if (val is String) return val;
return val.toString();
}Then update the return type:
-String _asString(dynamic val) {
+String? _asString(dynamic val) {🤖 Prompt for AI Agents
In lib/data/model/container/image.dart around lines 123 to 127, _asString
currently converts null to an empty string which loses the original null
semantics of PodmanImg nullable fields; change _asString to return String?
(nullable) and return null when val is null, keep returning String for String
inputs and val.toString() for other non-null inputs, then update any call sites
or variable typings that expect a non-null String to accept String? so
repository, tag, id remain nullable rather than becoming ''.
Fixes #946
Summary by CodeRabbit