-
Notifications
You must be signed in to change notification settings - Fork 70
feat(functions): Implement Task Queue Scopes #302
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -174,3 +174,59 @@ class TaskOptions { | |
| /// Contains experimental features that may change in future releases. | ||
| final TaskOptionsExperimental? experimental; | ||
| } | ||
|
|
||
| /// Represents the scope of a function in a task queue. | ||
| sealed class FunctionScope { | ||
| const FunctionScope._(); | ||
|
|
||
| /// Targets a function co-deployed in the same codebase/deployment context. | ||
| const factory FunctionScope.current() = _CurrentFunctionScope; | ||
|
|
||
| /// Targets a global function (e.g. without kit or extension namespacing). | ||
| const factory FunctionScope.global() = _GlobalFunctionScope; | ||
|
|
||
| /// Targets a function deployed within a specific Firebase Extension. | ||
| const factory FunctionScope.extension(String extensionId) = | ||
| _ExtensionFunctionScope; | ||
| } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just clarifying, we didn't include kit here because we don't plan to expose it in the public api, yet? /// Targets a function deployed within a specific Firebase Kit.
const factory FunctionScope.kit(String kit) = _KitFunctionScope; |
||
|
|
||
| class _GlobalFunctionScope extends FunctionScope { | ||
| const _GlobalFunctionScope() : super._(); | ||
|
|
||
| @override | ||
| String toString() => 'FunctionScope.global()'; | ||
| } | ||
|
|
||
| class _ExtensionFunctionScope extends FunctionScope { | ||
| const _ExtensionFunctionScope(this.extensionId) : super._(); | ||
|
|
||
| final String extensionId; | ||
|
|
||
| @override | ||
| String toString() => 'FunctionScope.extension($extensionId)'; | ||
| } | ||
|
|
||
| class _KitFunctionScope extends FunctionScope { | ||
| const _KitFunctionScope(this.kit) : super._(); | ||
|
|
||
| final String kit; | ||
|
|
||
| @override | ||
| String toString() => 'FunctionScope.kit($kit)'; | ||
| } | ||
|
|
||
| class _CurrentFunctionScope extends FunctionScope { | ||
| const _CurrentFunctionScope() : super._(); | ||
|
|
||
| @override | ||
| String toString() => 'FunctionScope.current()'; | ||
| } | ||
|
|
||
| class _ExtensionOrKitFunctionScope extends FunctionScope { | ||
| const _ExtensionOrKitFunctionScope(this.instance) : super._(); | ||
|
|
||
| final String instance; | ||
|
|
||
| @override | ||
| String toString() => 'FunctionScope.extensionOrKit($instance)'; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,18 +23,44 @@ class TaskQueue { | |
| required String functionName, | ||
| required FunctionsRequestHandler requestHandler, | ||
| String? extensionId, | ||
| FunctionScope? scope, | ||
| }) : _functionName = functionName, | ||
| _requestHandler = requestHandler, | ||
| _extensionId = extensionId { | ||
| _requestHandler = requestHandler { | ||
| validateNonEmptyString(_functionName, 'functionName'); | ||
| if (_extensionId != null) { | ||
| validateString(_extensionId, 'extensionId'); | ||
|
|
||
| if (extensionId != null && scope != null) { | ||
| throw ArgumentError('Cannot set both extensionId and scope.'); | ||
| } | ||
|
|
||
| if (extensionId != null) { | ||
| validateString(extensionId, 'extensionId'); | ||
| _scope = extensionId.isEmpty | ||
| ? const FunctionScope.current() | ||
| : _ExtensionOrKitFunctionScope(extensionId); | ||
| } else { | ||
| _scope = scope ?? const FunctionScope.current(); | ||
| } | ||
|
|
||
| if (_scope is _ExtensionOrKitFunctionScope) { | ||
| final instance = (_scope as _ExtensionOrKitFunctionScope).instance; | ||
| final kitInstanceId = Environment.getKitInstanceId(); | ||
| if (kitInstanceId != null && | ||
| kitInstanceId.isNotEmpty && | ||
| kitInstanceId == instance) { | ||
| _scope = _KitFunctionScope(instance); | ||
| print( | ||
| 'Targeting your own extension or kit no longer requires a second parameter, ' | ||
| 'which can have performance implications. Please change the call ' | ||
| "taskQueue('$_functionName', '$instance') to taskQueue('$_functionName') " | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: should this be a named argument? |
||
| "or taskQueue('$_functionName', scope: FunctionScope.current())", | ||
| ); | ||
|
inlined marked this conversation as resolved.
|
||
| } | ||
| } | ||
| } | ||
|
|
||
| final String _functionName; | ||
| final FunctionsRequestHandler _requestHandler; | ||
| final String? _extensionId; | ||
| late FunctionScope _scope; | ||
|
|
||
| /// Enqueues a task with the given [data] payload. | ||
| /// | ||
|
|
@@ -59,8 +85,28 @@ class TaskQueue { | |
| /// ``` | ||
| /// | ||
| /// Throws [FirebaseFunctionsAdminException] if the request fails. | ||
| Future<void> enqueue(Map<String, dynamic> data, [TaskOptions? options]) { | ||
| return _requestHandler.enqueue(data, _functionName, _extensionId, options); | ||
| Future<void> enqueue( | ||
| Map<String, dynamic> data, [ | ||
| TaskOptions? options, | ||
| ]) async { | ||
| final currentScope = _scope; | ||
| if (currentScope is! _ExtensionOrKitFunctionScope) { | ||
| await _requestHandler.enqueue(data, _functionName, currentScope, options); | ||
| return; | ||
| } | ||
|
|
||
| try { | ||
| await _requestHandler.enqueue(data, _functionName, currentScope, options); | ||
| } on FirebaseFunctionsAdminException catch (err) { | ||
| if (err.errorCode != FunctionsClientErrorCode.notFound) { | ||
| rethrow; | ||
| } | ||
| final tempKitScope = _KitFunctionScope(currentScope.instance); | ||
| await _requestHandler.enqueue(data, _functionName, tempKitScope, options); | ||
| // Only upgrade the stateful scope to kit if the retry request succeeds | ||
| _scope = tempKitScope; | ||
| _logFallbackWarning(_functionName, currentScope.instance); | ||
| } | ||
| } | ||
|
|
||
| /// Deletes a task from the queue by its [id]. | ||
|
|
@@ -74,7 +120,40 @@ class TaskQueue { | |
| /// ``` | ||
| /// | ||
| /// Throws [FirebaseFunctionsAdminException] if the request fails. | ||
| Future<void> delete(String id) { | ||
| return _requestHandler.delete(id, _functionName, _extensionId); | ||
| Future<void> delete(String id) async { | ||
| try { | ||
| final currentScope = _scope; | ||
| if (currentScope is! _ExtensionOrKitFunctionScope) { | ||
| await _requestHandler.delete(id, _functionName, currentScope); | ||
| return; | ||
| } | ||
|
|
||
| try { | ||
| await _requestHandler.delete(id, _functionName, currentScope); | ||
| } on FirebaseFunctionsAdminException catch (err) { | ||
| if (err.errorCode != FunctionsClientErrorCode.notFound) { | ||
| rethrow; | ||
| } | ||
| // Not found, try fallback to kit scope. | ||
| final tempKitScope = _KitFunctionScope(currentScope.instance); | ||
| await _requestHandler.delete(id, _functionName, tempKitScope); | ||
| // Only upgrade the stateful scope to kit if the retry request succeeds | ||
| _scope = tempKitScope; | ||
| _logFallbackWarning(_functionName, currentScope.instance); | ||
| } | ||
| } on FirebaseFunctionsAdminException catch (err) { | ||
| if (err.errorCode != FunctionsClientErrorCode.notFound) { | ||
| rethrow; | ||
| } | ||
| // Swallow notFound errors as delete is idempotent. | ||
| } | ||
| } | ||
|
inlined marked this conversation as resolved.
|
||
|
|
||
| void _logFallbackWarning(String functionName, String instance) { | ||
| print( | ||
| 'Targeting kit $instance with the legacy extensions API, ' | ||
| 'which has performance implications. Please change the call ' | ||
| "taskQueue('$functionName', '$instance') to taskQueue('$functionName')", | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: should this be a named argument? |
||
| ); | ||
|
inlined marked this conversation as resolved.
|
||
| } | ||
| } | ||
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 we can a firebase doc link when this is released, so we know how/from where this
'FIREBASE_KIT_INSTANCE_ID'is injected?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.
Definitely worth considering. It is a reserved env var; I could see us documenting it, though with this API it shouldn't actually be necessary to set or read directly I believe.