diff --git a/addon_service/addon_operation_invocation/serializers.py b/addon_service/addon_operation_invocation/serializers.py index 07460742..8aefc312 100644 --- a/addon_service/addon_operation_invocation/serializers.py +++ b/addon_service/addon_operation_invocation/serializers.py @@ -68,7 +68,12 @@ class Meta: thru_addon = CustomPolymorphicResourceRelatedField( many=False, required=False, - queryset=ConfiguredAddon.objects.active(), + queryset=ConfiguredAddon.objects.active().select_related( + "base_account__external_service", + "base_account__authorizedstorageaccount", + "base_account__authorizedcitationaccount", + "base_account__account_owner", + ), related_link_view_name=view_names.related_view(RESOURCE_TYPE), polymorphic_serializer=ConfiguredAddonPolymorphicSerializer, ) @@ -92,6 +97,10 @@ class Meta: "by_user": "addon_service.serializers.UserReferenceSerializer", } + def to_internal_value(self, data): + validated_data = super().to_internal_value(data) + return validated_data + def create(self, validated_data): _thru_addon = validated_data.get("thru_addon") _thru_account = validated_data.get("thru_account") diff --git a/addon_service/addon_operation_invocation/views.py b/addon_service/addon_operation_invocation/views.py index c58d6719..b1b10e32 100644 --- a/addon_service/addon_operation_invocation/views.py +++ b/addon_service/addon_operation_invocation/views.py @@ -87,7 +87,16 @@ 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() + ) + if _invocation.thru_addon: + _invocation.thru_addon.base_account = _invocation.thru_account _operation_type = _invocation.operation.operation_type match _operation_type: case AddonOperationType.REDIRECT | AddonOperationType.IMMEDIATE: @@ -96,3 +105,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/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/common/serializer_fields.py b/addon_service/common/serializer_fields.py index a818a434..1d63488d 100644 --- a/addon_service/common/serializer_fields.py +++ b/addon_service/common/serializer_fields.py @@ -8,6 +8,9 @@ SkipDataMixin, ) +from addon_service.authorized_account.models import AuthorizedAccount +from addon_service.configured_addon.models import ConfiguredAddon + class ReadOnlyResourceRelatedField( json_api_serializers.ResourceRelatedField, drf_serializers.ReadOnlyField @@ -54,16 +57,18 @@ def get(self, *, pk): class CustomPolymorphicResourceRelatedField(PolymorphicResourceRelatedField): def to_representation(self, value): data = super().to_representation(value) - if hasattr(value, "authorizedcitationaccount"): - data["type"] = "authorized-citation-accounts" - elif hasattr(value, "authorizedcomputingaccount"): - data["type"] = "authorized-computing-accounts" - elif hasattr(value, "authorizedstorageaccount"): - data["type"] = "authorized-storage-accounts" - elif hasattr(value, "configuredcitationaddon"): - data["type"] = "configured-citation-addons" - elif hasattr(value, "configuredcomputingaddon"): - data["type"] = "configured-computing-addons" - elif hasattr(value, "configuredstorageaccount"): - data["type"] = "configured-storage-addons" + if isinstance(value, AuthorizedAccount): + if hasattr(value, "authorizedcitationaccount"): + data["type"] = "authorized-citation-accounts" + elif hasattr(value, "authorizedstorageaccount"): + data["type"] = "authorized-storage-accounts" + elif hasattr(value, "authorizedcomputingaccount"): + data["type"] = "authorized-computing-accounts" + elif isinstance(value, ConfiguredAddon): + if hasattr(value, "configuredcitationaddon"): + data["type"] = "configured-citation-addons" + elif hasattr(value, "configuredstorageaddon"): + data["type"] = "configured-storage-addons" + elif hasattr(value, "configuredcomputingaddon"): + data["type"] = "configured-computing-addons" return data diff --git a/addon_service/configured_addon/storage/views.py b/addon_service/configured_addon/storage/views.py index 10d4a521..722900e9 100644 --- a/addon_service/configured_addon/storage/views.py +++ b/addon_service/configured_addon/storage/views.py @@ -12,7 +12,12 @@ class ConfiguredStorageAddonViewSet(ConfiguredAddonViewSet): - queryset = ConfiguredStorageAddon.objects.active() + queryset = ConfiguredStorageAddon.objects.active().select_related( + "base_account__authorizedstorageaccount", + "base_account__account_owner", + "base_account__external_service__externalstorageservice", + "authorized_resource", + ) serializer_class = ConfiguredStorageAddonSerializer @action( diff --git a/addon_service/external_service/citation/views.py b/addon_service/external_service/citation/views.py index c267ce42..b8e9fa03 100644 --- a/addon_service/external_service/citation/views.py +++ b/addon_service/external_service/citation/views.py @@ -5,5 +5,7 @@ class ExternalCitationServiceViewSet(ReadOnlyModelViewSet): - queryset = ExternalCitationService.objects.all() + queryset = ExternalCitationService.objects.all().select_related( + "oauth2_client_config", "oauth1_client_config" + ) serializer_class = ExternalCitationServiceSerializer diff --git a/addon_service/external_service/computing/views.py b/addon_service/external_service/computing/views.py index 146b5adb..b52baf0e 100644 --- a/addon_service/external_service/computing/views.py +++ b/addon_service/external_service/computing/views.py @@ -5,5 +5,7 @@ class ExternalComputingServiceViewSet(ReadOnlyModelViewSet): - queryset = ExternalComputingService.objects.all() + queryset = ExternalComputingService.objects.all().select_related( + "oauth2_client_config" + ) serializer_class = ExternalComputingServiceSerializer diff --git a/addon_service/external_service/storage/views.py b/addon_service/external_service/storage/views.py index fe487475..703f48c2 100644 --- a/addon_service/external_service/storage/views.py +++ b/addon_service/external_service/storage/views.py @@ -5,5 +5,7 @@ class ExternalStorageServiceViewSet(ReadOnlyModelViewSet): - queryset = ExternalStorageService.objects.all() + queryset = ExternalStorageService.objects.all().select_related( + "oauth2_client_config" + ) serializer_class = ExternalStorageServiceSerializer diff --git a/addon_service/resource_reference/models.py b/addon_service/resource_reference/models.py index d8725d00..d9004be6 100644 --- a/addon_service/resource_reference/models.py +++ b/addon_service/resource_reference/models.py @@ -12,17 +12,35 @@ class ResourceReference(AddonsServiceBaseModel): @property def configured_storage_addons(self): - return ConfiguredStorageAddon.objects.filter(authorized_resource=self).order_by( - Lower("_display_name") + return ( + ConfiguredStorageAddon.objects.filter(authorized_resource=self) + .select_related( + "base_account__external_service__externalstorageservice", + "base_account__authorizedstorageaccount", + "base_account__account_owner", + ) + .order_by(Lower("_display_name")) ) @property def configured_citation_addons(self): - return ConfiguredCitationAddon.objects.filter(authorized_resource=self) + return ConfiguredCitationAddon.objects.filter( + authorized_resource=self + ).select_related( + "base_account__external_service__externalcitationservice", + "base_account__authorizedcitationaccount", + "base_account__account_owner", + ) @property def configured_computing_addons(self): - return ConfiguredComputingAddon.objects.filter(authorized_resource=self) + return ConfiguredComputingAddon.objects.filter( + authorized_resource=self + ).select_related( + "base_account__external_service__externalcomputingservice", + "base_account__authorizedcitationaccount", + "base_account__account_owner", + ) class Meta: verbose_name = "Resource Reference" 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) diff --git a/addon_service/user_reference/models.py b/addon_service/user_reference/models.py index 05fc108e..e54ad38d 100644 --- a/addon_service/user_reference/models.py +++ b/addon_service/user_reference/models.py @@ -39,15 +39,21 @@ def configured_resources(self): @property def authorized_storage_accounts(self): - return AuthorizedStorageAccount.objects.filter(account_owner=self) + return AuthorizedStorageAccount.objects.filter( + account_owner=self + ).select_related("external_service") @property def authorized_citation_accounts(self): - return AuthorizedCitationAccount.objects.filter(account_owner=self) + return AuthorizedCitationAccount.objects.filter( + account_owner=self + ).select_related("external_service") @property def authorized_computing_accounts(self): - return AuthorizedComputingAccount.objects.filter(account_owner=self) + return AuthorizedComputingAccount.objects.filter( + account_owner=self + ).select_related("external_service") class Meta: verbose_name = "User Reference" diff --git a/app/env.py b/app/env.py index 1efaf036..25d60e49 100644 --- a/app/env.py +++ b/app/env.py @@ -2,8 +2,10 @@ """ import os +import sys +TESTING = "test" in sys.argv DEBUG = bool(os.environ.get("DEBUG")) # any non-empty value enables debug mode SECRET_KEY = os.environ.get("SECRET_KEY") # used by django for cryptographic signing ALLOWED_HOSTS = list(filter(bool, os.environ.get("ALLOWED_HOSTS", "").split(","))) @@ -16,6 +18,7 @@ NEW_RELIC_ENVIRONMENT = os.environ.get("NEW_RELIC_ENVIRONMENT") SESSION_COOKIE_DOMAIN = os.environ.get("SESSION_COOKIE_DOMAIN") + ### # databases diff --git a/app/settings.py b/app/settings.py index 636dc875..1a6c6f6e 100644 --- a/app/settings.py +++ b/app/settings.py @@ -95,11 +95,6 @@ "addon_service", ] -if DEBUG: - # run under ASGI locally: - INSTALLED_APPS.insert(0, "daphne") # django's reference asgi server - ASGI_APPLICATION = "app.asgi.application" - MIDDLEWARE = [ "corsheaders.middleware.CorsMiddleware", "django.middleware.security.SecurityMiddleware", @@ -111,6 +106,17 @@ "django.middleware.clickjacking.XFrameOptionsMiddleware", ] + +if DEBUG and not env.TESTING: + # add django-silk to enable profiling + INSTALLED_APPS.append("silk") + MIDDLEWARE.insert(0, "silk.middleware.SilkyMiddleware") +if DEBUG: + # run under ASGI locally: + INSTALLED_APPS.insert(0, "daphne") # django's reference asgi server + ASGI_APPLICATION = "app.asgi.application" + + ROOT_URLCONF = "app.urls" TEMPLATES = [ @@ -168,6 +174,7 @@ }, } +SILKY_PYTHON_PROFILER = True DATABASE_ROUTERS = ["addon_service.osf_models.db_router.OsfDatabaseRouter"] REST_FRAMEWORK = { diff --git a/app/urls.py b/app/urls.py index 9d8a7df2..1e307e51 100644 --- a/app/urls.py +++ b/app/urls.py @@ -1,3 +1,4 @@ +from django.conf import settings from django.contrib import admin from django.urls import ( include, @@ -15,3 +16,6 @@ name="docs-root", ), ] + +if "silk" in settings.INSTALLED_APPS: + urlpatterns.append(path("silk/", include("silk.urls", namespace="silk"))) diff --git a/poetry.lock b/poetry.lock index d195bc8f..dc30143b 100644 --- a/poetry.lock +++ b/poetry.lock @@ -199,6 +199,20 @@ files = [ [package.extras] visualize = ["Twisted (>=16.1.1)", "graphviz (>0.5.1)"] +[[package]] +name = "autopep8" +version = "2.3.1" +description = "A tool that automatically formats Python code to conform to the PEP 8 style guide" +optional = false +python-versions = ">=3.8" +files = [ + {file = "autopep8-2.3.1-py2.py3-none-any.whl", hash = "sha256:a203fe0fcad7939987422140ab17a930f684763bf7335bdb6709991dd7ef6c2d"}, + {file = "autopep8-2.3.1.tar.gz", hash = "sha256:8d6c87eba648fdcfc83e29b788910b8643171c395d9c4bcf115ece035b9c9dda"}, +] + +[package.dependencies] +pycodestyle = ">=2.12.0" + [[package]] name = "billiard" version = "4.2.0" @@ -733,6 +747,23 @@ files = [ [package.dependencies] Django = ">=4.2" +[[package]] +name = "django-silk" +version = "5.3.2" +description = "Silky smooth profiling for the Django Framework" +optional = false +python-versions = ">=3.9" +files = [ + {file = "django_silk-5.3.2-py3-none-any.whl", hash = "sha256:49f1caebfda28b1707f0cfef524e0476beb82b8c5e40f5ccff7f73a6b4f6d3ac"}, + {file = "django_silk-5.3.2.tar.gz", hash = "sha256:b0db54eebedb8d16f572321bd6daccac0bd3f547ae2618bb45d96fe8fc02229d"}, +] + +[package.dependencies] +autopep8 = "*" +Django = ">=4.2" +gprof2dot = ">=2017.09.19" +sqlparse = "*" + [[package]] name = "djangorestframework" version = "3.14.0" @@ -919,6 +950,17 @@ files = [ {file = "frozenlist-1.4.1.tar.gz", hash = "sha256:c037a86e8513059a2613aaba4d817bb90b9d9b6b69aace3ce9c877e8c8ed402b"}, ] +[[package]] +name = "gprof2dot" +version = "2024.6.6" +description = "Generate a dot graph from the output of several profilers." +optional = false +python-versions = ">=3.8" +files = [ + {file = "gprof2dot-2024.6.6-py2.py3-none-any.whl", hash = "sha256:45b14ad7ce64e299c8f526881007b9eb2c6b75505d5613e96e66ee4d5ab33696"}, + {file = "gprof2dot-2024.6.6.tar.gz", hash = "sha256:fa1420c60025a9eb7734f65225b4da02a10fc6dd741b37fa129bc6b41951e5ab"}, +] + [[package]] name = "hyperlink" version = "21.0.0" @@ -2209,4 +2251,4 @@ testing = ["coverage (>=5.0.3)", "zope.event", "zope.testing"] [metadata] lock-version = "2.0" python-versions = "^3.12" -content-hash = "f2198ba94898f4260705bc395c05b70dff5b019786c9c0011da0ba92852f92b4" +content-hash = "9a504ffb729f32f4824b2e07fa523c8bf40f460e3082a51edcf7d1424208e566" diff --git a/pyproject.toml b/pyproject.toml index 84cbfc1d..17e06d9e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -32,6 +32,7 @@ flake8 = "^7.1.1" isort = "^5.13.2" pre-commit = "^3.8.0" pdoc = "14.5.1" +django-silk = "^5.3.2" [tool.poetry.group.release.dependencies] sentry-sdk = "2.7.1"