-
Notifications
You must be signed in to change notification settings - Fork 13
feat: add rust method to upload chat media #739
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
WalkthroughAdds end-to-end chat media upload: new Rust API and types (MediaFile, FileMetadata), Rust↔Dart bridge and FFI wiring, SSE/DCO (de)serializers for new types, dispatcher/funcId updates, and a whitenoise dependency revision plus a small groups call change. Changes
Sequence Diagram(s)sequenceDiagram
participant Dart as Dart App
participant Bridge as Dart API Bridge
participant FFI as FFI Layer
participant Rust as Rust API
participant WN as WhiteNoise
Dart->>Bridge: uploadChatMedia(accountPubkey, groupId, filePath)
Bridge->>FFI: crateApiMediaFilesUploadChatMedia (encode args)
FFI->>Rust: wire__crate__api__media_files__upload_chat_media_impl
Rust->>WN: upload_chat_media(account_pubkey, group_id, file_path)
WN-->>Rust: WhitenoiseMediaFile
Rust->>Rust: Convert WhitenoiseMediaFile -> MediaFile (+FileMetadata)
Rust-->>FFI: Result<MediaFile, ApiError> (SSE)
FFI->>Bridge: decode MediaFile
Bridge-->>Dart: Future<MediaFile>
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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
🧹 Nitpick comments (4)
rust/src/api/groups.rs (1)
370-372: Validate server_url (https-only) and allow defaulting to None.Currently any scheme parses. Consider rejecting non-https and treating empty string as “use default server” to preserve old behavior and avoid misuse.
Example:
- let server = Url::parse(&server_url)?; - let (encrypted_hash, image_key, image_nonce) = whitenoise - .upload_group_image(&account, &group_id, &file_path, Some(server), None) + let server_opt = if server_url.trim().is_empty() { + None + } else { + let server = Url::parse(&server_url)?; + if server.scheme() != "https" { + return Err(ApiError::from(anyhow::anyhow!("server_url must use https"))); + } + Some(server) + }; + let (encrypted_hash, image_key, image_nonce) = whitenoise + .upload_group_image(&account, &group_id, &file_path, server_opt, None) .await?;Please confirm whether server_url can originate from untrusted UI input. If yes, the https guard should be prioritized.
lib/src/rust/api/media_files.dart (1)
47-55: Consider modeling fileBlossomUrl as nullable to reflect absence.Rust maps an optional blossom_url; representing it as String? here would avoid conflating “unset” with “empty string” and simplifies UI checks.
rust/src/api/media_files.rs (2)
27-35: Preserve optional blossom_url; avoid defaulting to empty string.Expose file_blossom_url as Option and keep None when absent.
Apply:
pub struct MediaFile { pub mls_group_id: String, pub file_path: String, pub file_mime_type: String, pub file_media_type: String, - pub file_blossom_url: String, + pub file_blossom_url: Option<String>, pub file_metadata: Option<FileMetadata>, pub original_file_path: Option<String>, }Self { mls_group_id: group_id_to_string(&media_file.mls_group_id), file_path: media_file.file_path.to_string_lossy().to_string(), file_mime_type: media_file.mime_type.to_string(), file_media_type: media_file.media_type.to_string(), - file_blossom_url: media_file.blossom_url.unwrap_or_default(), + file_blossom_url: media_file.blossom_url, file_metadata: media_file.file_metadata.map(|metadata| metadata.into()), original_file_path: None, }Pair with the Dart change to make fileBlossomUrl nullable.
Also applies to: 37-49
51-69: Optional: early file existence check for clearer errors.Validate file_path before calling into whitenoise to fail fast with a clear message.
Example:
use std::path::Path; if !Path::new(&file_path).exists() { return Err(ApiError::from(std::io::Error::new( std::io::ErrorKind::NotFound, format!("File not found: {file_path}"), ))); }Adjust error mapping to your ApiError variants as needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
ios/Podfile.lockis excluded by!**/*.lockrust/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
lib/src/rust/api/media_files.dart(1 hunks)lib/src/rust/frb_generated.dart(20 hunks)lib/src/rust/frb_generated.io.dart(13 hunks)rust/Cargo.toml(1 hunks)rust/src/api/groups.rs(1 hunks)rust/src/api/media_files.rs(1 hunks)rust/src/api/mod.rs(2 hunks)rust/src/frb_generated.rs(11 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.rs
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
Use the whitenoise Rust crate (via flutter_rust_bridge) as the sole data source and change trigger
Files:
rust/src/api/mod.rsrust/src/api/media_files.rsrust/src/api/groups.rsrust/src/frb_generated.rs
**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
**/*.dart: Always declare the type of each variable and function (parameters and return value)
Avoid using dynamic and Object without justification
Create necessary types instead of overusing primitives or dynamic
One export per file
Use PascalCase for classes
Use camelCase for variables, functions, and methods
Avoid magic numbers and define constants
Start each function name with a verb
Use verbs for boolean variables (e.g., isLoading, hasError, canDelete)
Write short functions with a single purpose (under ~20 instructions)
Name functions with a verb plus context; for booleans use isX/hasX/canX; for void use executeX/saveX
Avoid deep nesting via early returns and extraction to utility functions
Use higher-order functions (map, where/filter, reduce) to avoid nesting
Use arrow functions for simple functions (under ~3 statements); use named functions otherwise
Use default parameter values instead of null checks
Reduce function parameters using RO-RO: pass/return parameter objects with declared types
Maintain a single level of abstraction within functions
Encapsulate data in composite types; avoid overusing primitives
Prefer validating data within classes rather than in functions
Prefer immutability; use final for runtime constants and const for compile-time constants
Use const constructors and const literals where possible
Follow SOLID principles
Prefer composition over inheritance
Declare interfaces (abstract classes) to define contracts
Write small classes with a single purpose (under ~200 instructions, <10 public methods, <10 properties)
Use exceptions for unexpected errors
Only catch exceptions to fix expected problems or add context; otherwise use a global handler
Files:
lib/src/rust/api/media_files.dartlib/src/rust/frb_generated.io.dartlib/src/rust/frb_generated.dart
lib/**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
lib/**/*.dart: Use flutter_rust_bridge to access core app functionality
Use Riverpod for state management; prefer StreamProviders for Rust API streams; use keepAlive if needed
Use freezed to model/manage UI states
Controllers should expose methods as inputs and update UI state that drives the UI
Use AutoRoute for navigation and use extras to pass data between pages
Use Dart extensions to manage reusable code
Use ThemeData to manage themes
Use AppLocalizations for translations
Use constants to manage constant values
Avoid deeply nested widget trees; aim for a flatter widget structure for performance and readability
Break down large widgets into smaller, focused, reusable components
Keep the widget tree shallow to simplify state management and data flow
Utilize const constructors and const widgets wherever possible to reduce rebuilds
Files:
lib/src/rust/api/media_files.dartlib/src/rust/frb_generated.io.dartlib/src/rust/frb_generated.dart
🧬 Code graph analysis (2)
rust/src/api/media_files.rs (1)
rust/src/api/utils.rs (2)
group_id_from_string(56-59)group_id_to_string(51-53)
rust/src/frb_generated.rs (1)
rust/src/api/media_files.rs (1)
upload_chat_media(52-69)
🔇 Additional comments (5)
rust/src/api/mod.rs (1)
61-61: Module wiring LGTM.media_files is properly added and re-exported; this keeps the API surface consistent.
Also applies to: 74-74
rust/Cargo.toml (1)
32-32: No issues found. Lockfile and APIs confirmed in sync.Cargo.lock is already updated to the exact rev (
5c79a9b66cb358146113dd485e3cf5987b6371a1), bothupload_chat_media(rust/src/api/media_files.rs:63) andupload_group_image(rust/src/api/groups.rs:371) are actively used in the codebase, confirming the APIs exist in the pinned commit. No action needed.rust/src/frb_generated.rs (1)
1-5124: LGTM! Auto-generated bridge code correctly reflects the new media upload API.This is auto-generated code by
flutter_rust_bridgev2.11.1 that adds the FFI bindings for the newupload_chat_mediafunction. The implementation follows the established patterns consistently:
- Wire function correctly decodes three string parameters and calls
crate::api::media_files::upload_chat_media- Dispatch table properly routes
funcId 61to the new function and shifts subsequent IDs- Complete serialization support added for
FileMetadataandMediaFile(SSE decode/encode, IntoDart, optional variants)- Function signature matches the Rust API from
rust/src/api/media_files.rslib/src/rust/frb_generated.dart (1)
1-2: LGTM: Auto-generated FFI bindings are consistent.The auto-generated FFI bindings for the new
uploadChatMediamethod follow the established patterns correctly. The method signature, encoding/decoding paths, and metadata are properly generated.However, ensure:
- The source Rust implementation exists and is correct
- The
MediaFileandFileMetadatatypes are properly defined inapi/media_files.dart- Run
just precommitas noted in the PR checklistAlso applies to: 15-15, 81-81, 317-321, 2359-2393
lib/src/rust/frb_generated.io.dart (1)
1-2: LGTM: Platform-specific FFI abstractions properly generated.The abstract method declarations for
FileMetadataandMediaFileserialization follow the correct patterns for platform-specific implementations. All necessary encode/decode methods for both SSE and DCO codecs are present.Also applies to: 16-16, 152-153, 178-179, 256-257, 268-269
jgmontoya
left a 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.
Just one comment! Looks good though :D
| #[derive(Debug, Clone)] | ||
| pub struct MediaFile { | ||
| pub mls_group_id: String, | ||
| pub file_path: String, | ||
| pub file_mime_type: String, | ||
| pub file_media_type: String, | ||
| pub file_blossom_url: String, | ||
| pub file_metadata: Option<FileMetadata>, | ||
| pub original_file_path: Option<String>, | ||
| } | ||
|
|
||
| impl From<WhitenoiseMediaFile> for MediaFile { | ||
| fn from(media_file: WhitenoiseMediaFile) -> Self { | ||
| Self { | ||
| mls_group_id: group_id_to_string(&media_file.mls_group_id), | ||
| file_path: media_file.file_path.to_string_lossy().to_string(), | ||
| file_mime_type: media_file.mime_type.to_string(), | ||
| file_media_type: media_file.media_type.to_string(), | ||
| file_blossom_url: media_file.blossom_url.unwrap_or_default(), | ||
| file_metadata: media_file.file_metadata.map(|metadata| metadata.into()), | ||
| original_file_path: None, | ||
| } | ||
| } | ||
| } |
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.
This is the actual type on the Whitenoise crate:
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct MediaFile {
pub id: Option<i64>,
pub mls_group_id: GroupId,
pub account_pubkey: PublicKey,
pub file_path: PathBuf,
pub file_hash: Vec<u8>,
pub mime_type: String,
pub media_type: String,
pub blossom_url: Option<String>,
pub nostr_key: Option<String>,
pub file_metadata: Option<FileMetadata>,
pub created_at: DateTime<Utc>,
}I'm not sure if here we need to replicate it 100% though (I see you're adding original_filepath for example) but I'm leaving it here because it could be useful
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.
Even though it's not strictly necessary, I think I would try and mimic the original type as much as possible unless you have very good reason not to.
What's the purpose of the original_filepath field?
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.
The reason why I added it is that when uploading, there is the original file path, after uploaded, the file path changes to a new one. In the preview of the image, if the path changes, the image component is re rendered and it looks like it blinks and it bothers the sight
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.
Maybe I could just move that original path to the object of MediaFileUpload class (a draft PR #740 that goes next that besides the media file considers the state of the upload: uploading, uploaded or failed)... that would make more sense to you? @jgmontoya @erskingardner
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.
That seems like a good alternative 👍
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.
I removed the original file path and added the missing fields with the same name as the rust type in a fixup commit :)
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.
Oh no I missed some
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.
Now it's ok
codeswot
left a 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.
LGTM
erskingardner
left a 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.
Same small comment as @jgmontoya, but looks great! We'll make a rust dev of you yet @josefinalliende!
| #[derive(Debug, Clone)] | ||
| pub struct MediaFile { | ||
| pub mls_group_id: String, | ||
| pub file_path: String, | ||
| pub file_mime_type: String, | ||
| pub file_media_type: String, | ||
| pub file_blossom_url: String, | ||
| pub file_metadata: Option<FileMetadata>, | ||
| pub original_file_path: Option<String>, | ||
| } | ||
|
|
||
| impl From<WhitenoiseMediaFile> for MediaFile { | ||
| fn from(media_file: WhitenoiseMediaFile) -> Self { | ||
| Self { | ||
| mls_group_id: group_id_to_string(&media_file.mls_group_id), | ||
| file_path: media_file.file_path.to_string_lossy().to_string(), | ||
| file_mime_type: media_file.mime_type.to_string(), | ||
| file_media_type: media_file.media_type.to_string(), | ||
| file_blossom_url: media_file.blossom_url.unwrap_or_default(), | ||
| file_metadata: media_file.file_metadata.map(|metadata| metadata.into()), | ||
| original_file_path: None, | ||
| } | ||
| } | ||
| } |
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.
Even though it's not strictly necessary, I think I would try and mimic the original type as much as possible unless you have very good reason not to.
What's the purpose of the original_filepath field?
b46f5fc to
7b18b3b
Compare
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: 0
🧹 Nitpick comments (1)
rust/src/api/media_files.rs (1)
62-78: Document the None parameters for clarity.Line 74 passes two
Nonearguments towhitenoise.upload_chat_media, but their purpose isn't clear from the code. Consider adding a comment explaining what these optional parameters represent (e.g., server URL, metadata overrides) for future maintainability. Otherwise, the function structure is sound.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
rust/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
lib/src/rust/api/media_files.dart(1 hunks)lib/src/rust/frb_generated.dart(20 hunks)lib/src/rust/frb_generated.io.dart(13 hunks)rust/Cargo.toml(1 hunks)rust/src/api/groups.rs(1 hunks)rust/src/api/media_files.rs(1 hunks)rust/src/api/mod.rs(2 hunks)rust/src/frb_generated.rs(11 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- rust/src/api/groups.rs
- lib/src/rust/api/media_files.dart
- rust/Cargo.toml
🧰 Additional context used
📓 Path-based instructions (3)
**/*.rs
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
Use the whitenoise Rust crate (via flutter_rust_bridge) as the sole data source and change trigger
Files:
rust/src/api/media_files.rsrust/src/api/mod.rsrust/src/frb_generated.rs
**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
**/*.dart: Always declare the type of each variable and function (parameters and return value)
Avoid using dynamic and Object without justification
Create necessary types instead of overusing primitives or dynamic
One export per file
Use PascalCase for classes
Use camelCase for variables, functions, and methods
Avoid magic numbers and define constants
Start each function name with a verb
Use verbs for boolean variables (e.g., isLoading, hasError, canDelete)
Write short functions with a single purpose (under ~20 instructions)
Name functions with a verb plus context; for booleans use isX/hasX/canX; for void use executeX/saveX
Avoid deep nesting via early returns and extraction to utility functions
Use higher-order functions (map, where/filter, reduce) to avoid nesting
Use arrow functions for simple functions (under ~3 statements); use named functions otherwise
Use default parameter values instead of null checks
Reduce function parameters using RO-RO: pass/return parameter objects with declared types
Maintain a single level of abstraction within functions
Encapsulate data in composite types; avoid overusing primitives
Prefer validating data within classes rather than in functions
Prefer immutability; use final for runtime constants and const for compile-time constants
Use const constructors and const literals where possible
Follow SOLID principles
Prefer composition over inheritance
Declare interfaces (abstract classes) to define contracts
Write small classes with a single purpose (under ~200 instructions, <10 public methods, <10 properties)
Use exceptions for unexpected errors
Only catch exceptions to fix expected problems or add context; otherwise use a global handler
Files:
lib/src/rust/frb_generated.dartlib/src/rust/frb_generated.io.dart
lib/**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
lib/**/*.dart: Use flutter_rust_bridge to access core app functionality
Use Riverpod for state management; prefer StreamProviders for Rust API streams; use keepAlive if needed
Use freezed to model/manage UI states
Controllers should expose methods as inputs and update UI state that drives the UI
Use AutoRoute for navigation and use extras to pass data between pages
Use Dart extensions to manage reusable code
Use ThemeData to manage themes
Use AppLocalizations for translations
Use constants to manage constant values
Avoid deeply nested widget trees; aim for a flatter widget structure for performance and readability
Break down large widgets into smaller, focused, reusable components
Keep the widget tree shallow to simplify state management and data flow
Utilize const constructors and const widgets wherever possible to reduce rebuilds
Files:
lib/src/rust/frb_generated.dartlib/src/rust/frb_generated.io.dart
🧬 Code graph analysis (2)
rust/src/api/media_files.rs (1)
rust/src/api/utils.rs (2)
group_id_from_string(56-59)group_id_to_string(51-53)
rust/src/frb_generated.rs (1)
rust/src/api/media_files.rs (1)
upload_chat_media(63-78)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Flutter CI
🔇 Additional comments (20)
rust/src/api/mod.rs (1)
61-61: LGTM! Clean module integration.The media_files module is properly declared and re-exported, following the established pattern for other API modules in the crate.
Also applies to: 74-74
rust/src/api/media_files.rs (3)
9-25: LGTM! Clean metadata structure and conversion.The FileMetadata struct and its From implementation provide a straightforward mapping from the WhiteNoise crate's FileMetadata type.
26-41: MediaFile struct looks comprehensive.The struct appropriately bridges WhiteNoise's MediaFile type to Flutter, using String representations for cross-platform compatibility. Based on learnings, the
original_file_pathfield was discussed and accepted as part of the design.
43-60: Verify the id field's default value semantics.Line 46 converts
Option<i64>to String usingunwrap_or_default(), which mapsNoneto"0". This could be ambiguous on the Dart side—a newly created file withNonewould appear identical to one with an actual ID of 0. Consider whether the Dart MediaFile should useString?foridto preserve the None semantics, or verify that this default is acceptable for your use case.lib/src/rust/frb_generated.dart (1)
1-5197: LGTM! Generated FFI bindings are correct.The flutter_rust_bridge codegen has properly created the Dart bindings for the new MediaFile API, including both DCO and SSE serialization paths, API method declarations, and type conversions. As per coding guidelines, this correctly uses flutter_rust_bridge to access core app functionality.
Based on coding guidelines
rust/src/frb_generated.rs (11)
44-44: Generated content hash updated — no actionAuto-generated drift only.
2353-2395: New wire handler for upload_chat_media — LGTMDecodes 3 strings, forwards to crate::api::media_files::upload_chat_media, returns MediaFile via transform_result_sse. Matches API snippet.
2847-2859: SSE decode for FileMetadata — LGTMField order and optionals look consistent.
3279-3290: SSE decode for Option — LGTM
3947-3967: IntoDart for FileMetadata — LGTMOrder matches decode/encode.
4131-4160: IntoDart for MediaFile — LGTMArray order matches SSE (decode/encode).
4590-4597: SSE encode for FileMetadata — LGTM
4861-4877: SSE encode for MediaFile — LGTM
4911-4919: SSE encode for Option<FileMetadata) — LGTM
3203-3234: SSE decode for MediaFile — Dart model parity verifiedAll 12 fields are present in identical order with matching types:
- MediaFile: id, mlsGroupId, accountPubkey, filePath, fileHash, fileMimeType, fileMediaType, fileBlossomUrl, nostrKey, fileMetadata, originalFilePath, createdAt
- Nested FileMetadata class properly defined with originalFilename, dimensions, blurhash
Type mapping is correct (Rust Option ↔ Dart T?, DateTime ↔ DateTime). No mismatches detected.
3715-3721: Dart side regeneration verified—funcId mapping is correctThe Dart implementation of
crateApiMediaFilesUploadChatMediaconfirms funcId 61 is properly set and matches the Rust dispatcher mapping. The function correctly serializes arguments and invokes the FFI layer with the new ID.lib/src/rust/frb_generated.io.dart (4)
16-16: Add import for media_files.dart — LGTM
152-154: DCO decoders for FileMetadata/MediaFile — LGTMPresence of box_autoadd, plain, and optional variants looks complete.
Also applies to: 178-180, 256-258, 268-270
445-449: SSE decoders for FileMetadata/MediaFile — LGTMMatches Rust side additions.
Also applies to: 479-481, 571-573, 585-589
788-793: SSE encoders for FileMetadata/MediaFile — LGTMSymmetric with decoders; ready for round‑trip.
Also applies to: 827-829, 938-940, 956-961
cc9c48a to
21b4de5
Compare
21b4de5 to
99df9dc
Compare
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
🧹 Nitpick comments (2)
rust/src/api/media_files.rs (2)
28-40: Preserve nullability parity with Whitenoise for id/blossom_url/nostr_key.Current Dart-facing struct forces non-null Strings, but Whitenoise exposes Option for these. This loses “unknown/absent” semantics and invites sentinel values. Recommend exposing these as Option on Rust (String?) across FRB, or document invariants if you must keep non-null.
1-7: Tighten imports (optional).
nostr_sdk::prelude::*pulls a lot; importnostr_sdk::prelude::PublicKeyto reduce surface. No behavior change.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
lib/src/rust/api/media_files.dart(1 hunks)lib/src/rust/frb_generated.dart(20 hunks)rust/src/api/media_files.rs(1 hunks)rust/src/frb_generated.rs(11 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.rs
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
Use the whitenoise Rust crate (via flutter_rust_bridge) as the sole data source and change trigger
Files:
rust/src/api/media_files.rsrust/src/frb_generated.rs
**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
**/*.dart: Always declare the type of each variable and function (parameters and return value)
Avoid using dynamic and Object without justification
Create necessary types instead of overusing primitives or dynamic
One export per file
Use PascalCase for classes
Use camelCase for variables, functions, and methods
Avoid magic numbers and define constants
Start each function name with a verb
Use verbs for boolean variables (e.g., isLoading, hasError, canDelete)
Write short functions with a single purpose (under ~20 instructions)
Name functions with a verb plus context; for booleans use isX/hasX/canX; for void use executeX/saveX
Avoid deep nesting via early returns and extraction to utility functions
Use higher-order functions (map, where/filter, reduce) to avoid nesting
Use arrow functions for simple functions (under ~3 statements); use named functions otherwise
Use default parameter values instead of null checks
Reduce function parameters using RO-RO: pass/return parameter objects with declared types
Maintain a single level of abstraction within functions
Encapsulate data in composite types; avoid overusing primitives
Prefer validating data within classes rather than in functions
Prefer immutability; use final for runtime constants and const for compile-time constants
Use const constructors and const literals where possible
Follow SOLID principles
Prefer composition over inheritance
Declare interfaces (abstract classes) to define contracts
Write small classes with a single purpose (under ~200 instructions, <10 public methods, <10 properties)
Use exceptions for unexpected errors
Only catch exceptions to fix expected problems or add context; otherwise use a global handler
Files:
lib/src/rust/api/media_files.dartlib/src/rust/frb_generated.dart
lib/**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
lib/**/*.dart: Use flutter_rust_bridge to access core app functionality
Use Riverpod for state management; prefer StreamProviders for Rust API streams; use keepAlive if needed
Use freezed to model/manage UI states
Controllers should expose methods as inputs and update UI state that drives the UI
Use AutoRoute for navigation and use extras to pass data between pages
Use Dart extensions to manage reusable code
Use ThemeData to manage themes
Use AppLocalizations for translations
Use constants to manage constant values
Avoid deeply nested widget trees; aim for a flatter widget structure for performance and readability
Break down large widgets into smaller, focused, reusable components
Keep the widget tree shallow to simplify state management and data flow
Utilize const constructors and const widgets wherever possible to reduce rebuilds
Files:
lib/src/rust/api/media_files.dartlib/src/rust/frb_generated.dart
🧠 Learnings (1)
📚 Learning: 2025-10-12T22:44:49.606Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-10-12T22:44:49.606Z
Learning: Applies to lib/**/*.dart : Use flutter_rust_bridge to access core app functionality
Applied to files:
lib/src/rust/api/media_files.dart
🧬 Code graph analysis (2)
rust/src/api/media_files.rs (1)
rust/src/api/utils.rs (2)
group_id_from_string(56-59)group_id_to_string(51-53)
rust/src/frb_generated.rs (1)
rust/src/api/media_files.rs (1)
upload_chat_media(61-76)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Flutter CI
🔇 Additional comments (6)
lib/src/rust/api/media_files.dart (1)
13-21: Wrapper aligns with FRB surface; OK.Signature mirrors Rust upload_chat_media and routes through RustLib.
lib/src/rust/frb_generated.dart (4)
317-322: API surface wiring looks consistent.New RustLibApi method crateApiMediaFilesUploadChatMedia matches wrapper and args order.
2359-2394: FFI call setup and funcId are coherent.Serializer encodes (accountPubkey, groupId, filePath) and uses funcId 61; constMeta debugName matches Rust fn. Downstream shifts (62–65) handled.
Please run your app smoke test for any stale call sites relying on hardcoded funcIds (if any custom bindings exist).
2849-2858: Codec for FileMetadata/MediaFile matches Rust field order and counts.
- FileMetadata: 3 fields (originalFilename, dimensions, blurhash) — OK.
- MediaFile: 11 fields — OK and consistent with Rust struct order.
If you change Rust field optionality (e.g., make id/blossom_url/nostr_key optional), regenerate FRB to update these codecs.Also applies to: 3062-3079, 3658-3669, 3998-4026, 4920-4934
3573-3580: Optional FileMetadata encode/decode paths added correctly.opt-box codecs match non-opaque usage and align with the new types.
Also applies to: 4581-4589, 4973-4984
rust/src/frb_generated.rs (1)
1-5246: LGTM! Auto-generated FFI bindings for media upload are correct.The flutter_rust_bridge code generation correctly reflects the new
upload_chat_mediaAPI and types (MediaFile,FileMetadata) from the upstream Rust module. The wire function, SSE codec implementations, and Dart interop all follow established patterns consistently.
Description
We are adding media support, starting by images. This PR updates the whitenoise crate and adds the media_files.rs 🦀 to add the upload chat media method, that is needed in #728 .
Type of Change
Checklist
just precommitto ensure that formatting and linting are correctjust check-flutter-coverageto ensure that flutter coverage rules are passingCHANGELOG.mdfile with your changes (if they affect the user experience)Summary by CodeRabbit