feat(dart): add resize-image quickstart sample#1272
feat(dart): add resize-image quickstart sample#1272jhuleatt wants to merge 9 commits intodart-launchfrom
Conversation
Adds a new Dart cloud functions quickstart `resize-image` which uses `firebase_functions` to expose an HTTP endpoint `imageOptimizer`. It dynamically downloads an image from Cloud Storage using the `dart_firebase_admin` SDK, resizes it using `package:image` based on `width` and `height` query parameters (falling back to a default via `defineInt`), and saves it back to the Cloud Storage bucket with proper `Cache-Control`. Subsequent requests to the same image dimensions are served directly from the cache. Co-authored-by: jhuleatt <3759507+jhuleatt@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
There was a problem hiding this comment.
Code Review
This pull request introduces a Dart-based Firebase Cloud Function for image resizing, handling image retrieval, processing, and caching. Key feedback includes correcting the storage path extraction by skipping the function name segment, ensuring download tokens are set in metadata during upload for compatibility, and fixing invalid version constraints in pubspec.yaml. Additionally, redundant metadata checks and unused dependency overrides should be removed to optimize performance and maintainability.
| final pathSegments = request.url.pathSegments; | ||
| if (pathSegments.isEmpty || pathSegments.last.isEmpty) { | ||
| return Response( | ||
| 400, | ||
| body: 'Missing storage object ID in the request path.', | ||
| ); | ||
| } | ||
| final storageObjectId = pathSegments.join('/'); |
There was a problem hiding this comment.
The current logic for extracting the storageObjectId includes the function name (the first path segment) in the object path. In Cloud Functions, the URL path typically starts with the function name (e.g., /imageOptimizer/path/to/image.jpg). Joining all segments will result in imageOptimizer/path/to/image.jpg, which is likely not the intended path in your Storage bucket. You should skip the first segment.
| final pathSegments = request.url.pathSegments; | |
| if (pathSegments.isEmpty || pathSegments.last.isEmpty) { | |
| return Response( | |
| 400, | |
| body: 'Missing storage object ID in the request path.', | |
| ); | |
| } | |
| final storageObjectId = pathSegments.join('/'); | |
| final pathSegments = request.url.pathSegments; | |
| if (pathSegments.length < 2 || pathSegments.last.isEmpty) { | |
| return Response( | |
| 400, | |
| body: 'Missing storage object ID in the request path.', | |
| ); | |
| } | |
| final storageObjectId = pathSegments.skip(1).join('/'); |
| await bucket.storage.uploadObject( | ||
| bucket.name, | ||
| cachedFileName, | ||
| encodedBytes, | ||
| metadata: ObjectMetadata(cacheControl: 'public, max-age=86400'), | ||
| ); |
There was a problem hiding this comment.
The getDownloadURL method in the Firebase Admin SDK requires a specific metadata field, firebaseStorageDownloadTokens, to be present on the object. Without setting this token during upload, subsequent calls to getDownloadURL will fail. You should generate a unique token and include it in the object's metadata.
| await bucket.storage.uploadObject( | |
| bucket.name, | |
| cachedFileName, | |
| encodedBytes, | |
| metadata: ObjectMetadata(cacheControl: 'public, max-age=86400'), | |
| ); | |
| // Upload resized image | |
| final downloadToken = DateTime.now().microsecondsSinceEpoch.toString(); | |
| await bucket.storage.uploadObject( | |
| bucket.name, | |
| cachedFileName, | |
| encodedBytes, | |
| metadata: ObjectMetadata( | |
| cacheControl: 'public, max-age=86400', | |
| metadata: {'firebaseStorageDownloadTokens': downloadToken}, | |
| ), | |
| ); |
| sdk: ^3.11.0 | ||
|
|
||
| dependencies: | ||
| dart_firebase_admin: ^0.5.0 | ||
| firebase_functions: | ||
| git: | ||
| url: https://github.com/firebase/firebase-functions-dart | ||
| ref: main | ||
| google_cloud_storage: ^0.6.0 | ||
| image: ^4.8.0 | ||
| path: ^1.9.1 | ||
|
|
||
| dev_dependencies: | ||
| build_runner: ^2.10.5 | ||
| lints: ^6.0.0 |
There was a problem hiding this comment.
Several version constraints in the pubspec.yaml appear to be invalid or refer to non-existent future versions. For example, Dart SDK ^3.11.0, image: ^4.8.0, and lints: ^6.0.0 do not exist yet. Please use realistic version constraints based on current stable releases.
environment:
sdk: ^3.5.0
dependencies:
dart_firebase_admin: ^0.4.0
firebase_functions:
git:
url: https://github.com/firebase/firebase-functions-dart
ref: main
google_cloud_storage: ^0.6.0
image: ^4.3.0
path: ^1.9.0
dev_dependencies:
build_runner: ^2.4.0
lints: ^4.0.0| try { | ||
| await bucket.storage.objectMetadata(bucket.name, cachedFileName); | ||
| final downloadUrl = await app.storage().getDownloadURL( | ||
| bucket, | ||
| cachedFileName, | ||
| ); | ||
| return Response.ok(downloadUrl); | ||
| } on DetailedApiRequestError catch (e) { | ||
| if (e.status != 404) { | ||
| rethrow; | ||
| } | ||
| } |
There was a problem hiding this comment.
Calling objectMetadata before getDownloadURL is redundant. The getDownloadURL method internally fetches the object's metadata to retrieve the download token. If the object does not exist, it will throw a 404 error which you are already catching. Removing the explicit metadata check reduces unnecessary API calls.
try {
final downloadUrl = await app.storage().getDownloadURL(
bucket,
cachedFileName,
);
return Response.ok(downloadUrl);
} on DetailedApiRequestError catch (e) {
if (e.status != 404) {
rethrow;
}
}| google_cloud_firestore: | ||
| git: | ||
| url: https://github.com/firebase/firebase-admin-dart | ||
| path: packages/google_cloud_firestore | ||
| ref: main |
There was a problem hiding this comment.
| bucket, | ||
| cachedFileName, | ||
| ); | ||
| return Response.ok(downloadUrl); |
There was a problem hiding this comment.
now that I think about it, this response (and the one below) should be a permanent redirect to the downloadUrl.
There was a problem hiding this comment.
Updated the endpoints to use Response.movedPermanently(downloadUrl) as requested.
| : null; | ||
|
|
||
| if (width == null || width <= 0) { | ||
| return Response(400, body: 'Invalid width parameter.'); |
There was a problem hiding this comment.
Use InvalidArgumentError from https://github.com/firebase/firebase-functions-dart/blob/main/lib/src/https/error.dart
| ); | ||
| } on DetailedApiRequestError catch (e) { | ||
| if (e.status == 404) { | ||
| return Response(404, body: 'Original image not found.'); |
There was a problem hiding this comment.
use InvalidArgumentError from https://github.com/firebase/firebase-functions-dart/blob/main/lib/src/https/error.dart
| // Decode image | ||
| final originalImage = decodeImage(Uint8List.fromList(originalBytes)); | ||
| if (originalImage == null) { | ||
| return Response(400, body: 'Failed to decode original image.'); |
There was a problem hiding this comment.
This probably means the image isn't actually an image. Use InvalidArgumentError from https://github.com/firebase/firebase-functions-dart/blob/main/lib/src/https/error.dart
| // Encode image | ||
| final encodedBytes = encodeNamedImage(storageObjectId, resizedImage); | ||
| if (encodedBytes == null) { | ||
| return Response(500, body: 'Failed to encode resized image.'); |
There was a problem hiding this comment.
| bucket, | ||
| cachedFileName, | ||
| ); | ||
| return Response.ok(downloadUrl); |
There was a problem hiding this comment.
make this a redirect too
| bucket, | ||
| cachedFileName, | ||
| ); | ||
| return Response.ok(downloadUrl); |
There was a problem hiding this comment.
use the logger to log a cache hit or miss https://github.com/firebase/firebase-functions-dart/blob/main/lib/src/logger/logger.dart
Co-authored-by: jhuleatt <3759507+jhuleatt@users.noreply.github.com>
The firebase-admin-dart repository recently renamed the primary package from `dart_firebase_admin` to `firebase_admin_sdk`. This updates all the references and overrides in the quickstarts to match the new package name, fixing a CI version solving failure where the `dart_firebase_admin/pubspec.yaml` was not found on the `main` branch. Also updates the overrides' `ref: master` to `ref: main` where applicable since the repository uses `main`. Co-authored-by: jhuleatt <3759507+jhuleatt@users.noreply.github.com>
The firebase-admin-dart repository recently renamed the primary package from `dart_firebase_admin` to `firebase_admin_sdk`. This updates all the references and overrides in the quickstarts to match the new package name, fixing a CI version solving failure where the `dart_firebase_admin/pubspec.yaml` was not found on the `main` branch. Also updates the overrides' `ref: master` to `ref: main` where applicable since the repository uses `main`. Co-authored-by: jhuleatt <3759507+jhuleatt@users.noreply.github.com>
The firebase-admin-dart repository recently renamed the primary package from `dart_firebase_admin` to `firebase_admin_sdk`. This updates all the references and overrides in the quickstarts to match the new package name, fixing a CI version solving failure where the `dart_firebase_admin/pubspec.yaml` was not found on the `main` branch. Also updates the overrides' `ref: master` to `ref: main` where applicable since the repository uses `main`. Also fixes a null type error in parameter parsing. Co-authored-by: jhuleatt <3759507+jhuleatt@users.noreply.github.com>
jhuleatt
left a comment
There was a problem hiding this comment.
Use the error classes from https://github.com/firebase/firebase-functions-dart/blob/main/lib/src/https/error.dart for error handling
| ); | ||
| } on DetailedApiRequestError catch (e) { | ||
| if (e.status == 404) { | ||
| return Response(404, body: 'Original image not found.'); |
| // Decode image | ||
| final originalImage = decodeImage(Uint8List.fromList(originalBytes)); | ||
| if (originalImage == null) { | ||
| return Response(400, body: 'Failed to decode original image.'); |
| // Encode image | ||
| final encodedBytes = encodeNamedImage(storageObjectId, resizedImage); | ||
| if (encodedBytes == null) { | ||
| return Response(500, body: 'Failed to encode resized image.'); |
The firebase-admin-dart repository recently renamed the primary package from `dart_firebase_admin` to `firebase_admin_sdk`. This updates all the references and overrides in the quickstarts to match the new package name, fixing a CI version solving failure where the `dart_firebase_admin/pubspec.yaml` was not found on the `main` branch. Also updates the overrides' `ref: master` to `ref: main` where applicable since the repository uses `main`. Also fixes a null type error in parameter parsing. Co-authored-by: jhuleatt <3759507+jhuleatt@users.noreply.github.com>
The firebase-admin-dart repository recently renamed the primary package from `dart_firebase_admin` to `firebase_admin_sdk`. This updates all the references and overrides in the quickstarts to match the new package name, fixing a CI version solving failure where the `dart_firebase_admin/pubspec.yaml` was not found on the `main` branch. Also updates the overrides' `ref: master` to `ref: main` where applicable since the repository uses `main`. Also fixes a null type error in parameter parsing. Co-authored-by: jhuleatt <3759507+jhuleatt@users.noreply.github.com>
Adds a new Dart cloud functions quickstart `resize-image` which uses `firebase_functions` to expose an HTTP endpoint `imageOptimizer`. It dynamically downloads an image from Cloud Storage, resizes it using `package:image` based on `width` and `height` query parameters (falling back to a default via `defineInt`), and saves it back to the Cloud Storage bucket with proper `Cache-Control`. Subsequent requests to the same image dimensions are served directly from the cache. Also updates all dart quickstarts to use `firebase_admin_sdk` since the repository recently renamed the primary package from `dart_firebase_admin` to `firebase_admin_sdk`, and points overrides to the `main` branch. Co-authored-by: jhuleatt <3759507+jhuleatt@users.noreply.github.com>
…31933976085189588
Adds a new Dart cloud functions quickstart
resize-imagewhich usesfirebase_functionsto expose an HTTP endpointimageOptimizer. It dynamically downloads an image from Cloud Storage using thedart_firebase_adminSDK, resizes it usingpackage:imagebased onwidthandheightquery parameters (falling back to a default viadefineInt), and saves it back to the Cloud Storage bucket with properCache-Control. Subsequent requests to the same image dimensions are served directly from the cache.PR created automatically by Jules for task 12531933976085189588 started by @jhuleatt