From 883278b583644506d2552206e2193b2421b921f9 Mon Sep 17 00:00:00 2001 From: Harry Date: Sat, 20 Dec 2025 12:29:23 +0800 Subject: [PATCH] refactor: consolidate trigger subscription request models and enhance update logic - Merged `SubscriptionRebuildRequest` into `TriggerSubscriptionUpdateRequest` to streamline request handling. - Updated `TriggerSubscriptionUpdateApi` to differentiate between rename and update operations, improving clarity in subscription management. - Enhanced `TriggerProviderService` to handle credential and parameter validation more robustly during subscription updates. - Renamed `SubscriptionVerifyRequest` to `TriggerSubscriptionVerifyRequest` for consistency across the API. --- .../console/workspace/trigger_providers.py | 116 +++++++----------- .../trigger/trigger_provider_service.py | 5 +- 2 files changed, 51 insertions(+), 70 deletions(-) diff --git a/api/controllers/console/workspace/trigger_providers.py b/api/controllers/console/workspace/trigger_providers.py index 4ba8e2115b..27dd6342f2 100644 --- a/api/controllers/console/workspace/trigger_providers.py +++ b/api/controllers/console/workspace/trigger_providers.py @@ -38,19 +38,15 @@ logger = logging.getLogger(__name__) class TriggerSubscriptionUpdateRequest(BaseModel): """Request payload for updating a trigger subscription""" - name: str | None = Field(default=None, description="Subscription instance name") - properties: dict[str, Any] | None = Field(default=None, description="Subscription properties") - - -class SubscriptionRebuildRequest(BaseModel): - """Request payload for rebuilding an existing subscription.""" - name: str | None = Field(default=None, description="The name for the subscription") credentials: Mapping[str, Any] | None = Field(default=None, description="The credentials for the subscription") - parameters: Mapping[str, Any] = Field(default_factory=dict, description="The parameters for the subscription") + parameters: Mapping[str, Any] | None = Field( + default_factory=dict, description="The parameters for the subscription" + ) + properties: Mapping[str, Any] | None = Field(default=None, description="The properties for the subscription") -class SubscriptionVerifyRequest(BaseModel): +class TriggerSubscriptionVerifyRequest(BaseModel): """Request payload for verifying subscription credentials.""" credentials: Mapping[str, Any] = Field(description="The credentials to verify") @@ -62,13 +58,8 @@ console_ns.schema_model( ) console_ns.schema_model( - SubscriptionRebuildRequest.__name__, - SubscriptionRebuildRequest.model_json_schema(ref_template="#/definitions/{model}"), -) - -console_ns.schema_model( - SubscriptionVerifyRequest.__name__, - SubscriptionVerifyRequest.model_json_schema(ref_template="#/definitions/{model}"), + TriggerSubscriptionVerifyRequest.__name__, + TriggerSubscriptionVerifyRequest.model_json_schema(ref_template="#/definitions/{model}"), ) @@ -352,17 +343,44 @@ class TriggerSubscriptionUpdateApi(Resource): if not subscription: raise NotFoundError(f"Subscription {subscription_id} not found") - if subscription.credential_type is not CredentialType.UNAUTHORIZED: - raise Forbidden("Only unauthorized subscriptions can be update directly") + provider_id = TriggerProviderID(subscription.provider_id) try: - TriggerProviderService.update_trigger_subscription( - tenant_id=user.current_tenant_id, - subscription_id=subscription_id, - name=args.name, - properties=args.properties, - ) - return 200 + # rename only for update name + is_rename_only = args.name is not None and args.credentials is None and args.parameters is None + if is_rename_only: + TriggerProviderService.update_trigger_subscription( + tenant_id=user.current_tenant_id, + subscription_id=subscription_id, + name=args.name, + ) + return 200 + + # rebuild for create automatically by the provider + match subscription.credential_type: + case CredentialType.UNAUTHORIZED: + TriggerProviderService.update_trigger_subscription( + tenant_id=user.current_tenant_id, + subscription_id=subscription_id, + name=args.name, + properties=args.properties, + ) + return 200 + case CredentialType.API_KEY | CredentialType.OAUTH2: + if not args.credentials or not args.parameters: + raise BadRequest("Credentials and parameters are required for rebuild") + + TriggerProviderService.rebuild_trigger_subscription( + tenant_id=user.current_tenant_id, + name=args.name, + provider_id=provider_id, + subscription_id=subscription_id, + credentials=args.credentials, + parameters=args.parameters, + ) + return 200 + case _: + raise BadRequest("Invalid credential type") except ValueError as e: raise BadRequest(str(e)) except Exception as e: @@ -406,48 +424,6 @@ class TriggerSubscriptionDeleteApi(Resource): raise -@console_ns.route( - "/workspaces/current/trigger-provider//subscriptions//rebuild", -) -class TriggerSubscriptionRebuildApi(Resource): - @console_ns.expect(console_ns.models[SubscriptionRebuildRequest.__name__]) - @setup_required - @login_required - @edit_permission_required - @account_initialization_required - def post(self, provider: str, subscription_id: str): - """ - Rebuild an existing subscription instance. - - This will: - 1. Unsubscribe from the provider (delete webhook on provider side) - 2. Create a new subscription with the request data and keep the same subscription_id and endpoint_id - - The user can then go through the normal build flow to re-create the webhook. - """ - user = current_user - assert user.current_tenant_id is not None - - rebuild_request: SubscriptionRebuildRequest = SubscriptionRebuildRequest.model_validate(console_ns.payload) - - try: - TriggerProviderService.rebuild_trigger_subscription( - tenant_id=user.current_tenant_id, - name=rebuild_request.name, - provider_id=TriggerProviderID(provider), - subscription_id=subscription_id, - credentials=rebuild_request.credentials or {}, - parameters=rebuild_request.parameters, - ) - - return 200 - except ValueError as e: - raise BadRequest(str(e)) - except Exception as e: - logger.exception("Error rebuilding subscription", exc_info=e) - raise - - @console_ns.route("/workspaces/current/trigger-provider//subscriptions/oauth/authorize") class TriggerOAuthAuthorizeApi(Resource): @setup_required @@ -705,7 +681,7 @@ class TriggerOAuthClientManageApi(Resource): "/workspaces/current/trigger-provider//subscriptions/verify/", ) class TriggerSubscriptionVerifyApi(Resource): - @console_ns.expect(console_ns.models[SubscriptionVerifyRequest.__name__]) + @console_ns.expect(console_ns.models[TriggerSubscriptionVerifyRequest.__name__]) @setup_required @login_required @edit_permission_required @@ -715,7 +691,9 @@ class TriggerSubscriptionVerifyApi(Resource): user = current_user assert user.current_tenant_id is not None - verify_request: SubscriptionVerifyRequest = SubscriptionVerifyRequest.model_validate(console_ns.payload) + verify_request: TriggerSubscriptionVerifyRequest = TriggerSubscriptionVerifyRequest.model_validate( + console_ns.payload + ) try: result = TriggerProviderService.verify_subscription_credentials( diff --git a/api/services/trigger/trigger_provider_service.py b/api/services/trigger/trigger_provider_service.py index 012f1828bc..889bf75b98 100644 --- a/api/services/trigger/trigger_provider_service.py +++ b/api/services/trigger/trigger_provider_service.py @@ -895,7 +895,10 @@ class TriggerProviderService: credentials=encrypter.decrypt(subscription.credentials), credential_type=credential_type, ) - new_credentials = credentials or subscription.credentials + new_credentials: dict[str, Any] = { + key: value if value != HIDDEN_VALUE else subscription.credentials.get(key, UNKNOWN_VALUE) + for key, value in credentials.items() + } # Create a new subscription with the same subscription_id and endpoint_id new_subscription: TriggerSubscriptionEntity = TriggerManager.subscribe_trigger( tenant_id=tenant_id,