diff --git a/addon_service/addon_operation_invocation/views.py b/addon_service/addon_operation_invocation/views.py index c58d6719..6e2a86bc 100644 --- a/addon_service/addon_operation_invocation/views.py +++ b/addon_service/addon_operation_invocation/views.py @@ -87,7 +87,15 @@ def retrieve_related(self, request, *args, **kwargs): def perform_create(self, serializer): super().perform_create(serializer) # after creating the AddonOperationInvocation, look into invoking it - _invocation = serializer.instance + _invocation = ( + AddonOperationInvocation.objects.filter(pk=serializer.instance.pk) + .select_related( + *self._get_narrowed_down_selects(serializer), + "thru_account___credentials", + ) + .first() + ) + _invocation.thru_addon.base_account = _invocation.thru_account _operation_type = _invocation.operation.operation_type match _operation_type: case AddonOperationType.REDIRECT | AddonOperationType.IMMEDIATE: @@ -96,3 +104,15 @@ def perform_create(self, serializer): perform_invocation__celery.delay(_invocation.pk) case _: raise ValueError(f"unknown operation type: {_operation_type}") + serializer.instance = _invocation + + def _get_narrowed_down_selects(self, serializer): + addon_resource_name = serializer.initial_data.get( + "thru_addon", serializer.initial_data.get("thru_account") + )["type"] + addon_type = addon_resource_name.split("-")[1] + return [ + f"thru_addon__configured{addon_type}addon", + f"thru_account__authorized{addon_type}account", + f"thru_account__external_service__external{addon_type}service", + ] diff --git a/addon_service/common/network.py b/addon_service/common/network.py index 1e923a60..0e8d5f21 100644 --- a/addon_service/common/network.py +++ b/addon_service/common/network.py @@ -83,7 +83,9 @@ async def _do_send(self, request: HttpRequestInfo): async with self._try_send(request) as _response: yield _response except exceptions.ExpiredAccessToken: - await _PrivateNetworkInfo.get(self).account.refresh_oauth2_access_token() + await _PrivateNetworkInfo.get( + self + ).account.refresh_oauth_access_token__async() # if this one fails, don't try refreshing again async with self._try_send(request) as _response: yield _response diff --git a/addon_service/common/osf.py b/addon_service/common/osf.py index 5add3e58..10a7582b 100644 --- a/addon_service/common/osf.py +++ b/addon_service/common/osf.py @@ -91,6 +91,7 @@ async def has_osf_permission_on_resource( params={ "resolve": "f", # do not redirect to the referent "embed": "referent", # instead, include the referent in the response + "fields[nodes]": "current_user_permissions", }, headers=[ *_get_osf_auth_headers(request), diff --git a/addon_service/credentials/models.py b/addon_service/credentials/models.py index a0ebe128..627d1690 100644 --- a/addon_service/credentials/models.py +++ b/addon_service/credentials/models.py @@ -88,37 +88,32 @@ def _key_parameters(self, value: encryption.KeyParameters) -> None: ### @property - def authorized_accounts(self): + def authorized_account(self): """Returns the list of all accounts that point to this set of credentials. For now, this will just be a single AuthorizedAccount, but in the future other types of accounts for the same user could point to the same set of credentials """ try: - return [ - *filter( - bool, - [ - getattr(self, "authorized_account", None), - getattr(self, "temporary_authorized_account", None), - ], - ) - ] + if account := getattr(self, "authorized_account", None): + return account + else: + return getattr(self, "temporary_authorized_account", None) except ExternalCredentials.authorized_account.RelatedObjectDoesNotExist: return None @property def format(self): - if not self.authorized_accounts: + if not self.authorized_account: return None - return self.authorized_accounts[0].external_service.credentials_format + return self.authorized_account.external_service.credentials_format def clean_fields(self, *args, **kwargs): super().clean_fields(*args, **kwargs) self._validate_credentials() def _validate_credentials(self): - if not self.authorized_accounts: + if not self.authorized_account: return try: self.decrypted_credentials diff --git a/addon_service/tasks/invocation.py b/addon_service/tasks/invocation.py index a76cb5e3..430becad 100644 --- a/addon_service/tasks/invocation.py +++ b/addon_service/tasks/invocation.py @@ -17,33 +17,34 @@ def perform_invocation__blocking(invocation: AddonOperationInvocation) -> None: """perform the given invocation: run an operation thru an addon and handle any errors""" # implemented as a sync function for django transactions - with dibs(invocation): # TODO: handle dibs errors - try: - _imp = get_addon_instance__blocking( - invocation.imp_cls, # type: ignore[arg-type] #(TODO: generic impstantiation) - invocation.thru_account, - invocation.config, - ) - _operation = invocation.operation - # inner transaction to contain database errors, - # so status can be saved in the outer transaction (from `dibs`) - with transaction.atomic(): - _result = _imp.invoke_operation__blocking( - _operation.declaration, - invocation.operation_kwargs, - ) - invocation.operation_result = json_for_typed_value( - _operation.declaration.result_dataclass, - _result, + try: + _imp = get_addon_instance__blocking( + invocation.imp_cls, # type: ignore[arg-type] #(TODO: generic impstantiation) + invocation.thru_account, + invocation.config, + ) + _operation = invocation.operation + # inner transaction to contain database errors, + # so status can be saved in the outer transaction (from `dibs`) + with transaction.atomic(): + _result = _imp.invoke_operation__blocking( + _operation.declaration, + invocation.operation_kwargs, ) - invocation.invocation_status = InvocationStatus.SUCCESS - except BaseException as _e: - invocation.set_exception(_e) - raise # TODO: or swallow? - finally: - invocation.save() + invocation.operation_result = json_for_typed_value( + _operation.declaration.result_dataclass, + _result, + ) + invocation.invocation_status = InvocationStatus.SUCCESS + except BaseException as _e: + invocation.set_exception(_e) + raise # TODO: or swallow? + finally: + invocation.save() @celery.shared_task(acks_late=True) def perform_invocation__celery(invocation_pk: str) -> None: - perform_invocation__blocking(AddonOperationInvocation.objects.get(pk=invocation_pk)) + invocation = AddonOperationInvocation.objects.get(pk=invocation_pk) + with dibs(invocation): # TODO: handle dibs errors + perform_invocation__blocking(invocation)