fixed pr comments

addon_service/abstract/authorized_account/models.py

@@ -54,36 +54,6 @@ class AuthorizedAccount(AddonsServiceBaseModel):
     )
     _api_base_url = models.URLField(blank=True)
 
-    account_owner = models.ForeignKey(
-        "addon_service.UserReference",
-        on_delete=models.CASCADE,
-        related_name="authorized_storage_accounts",
-    )
-    _credentials = models.OneToOneField(
-        "addon_service.ExternalCredentials",
-        on_delete=models.CASCADE,
-        primary_key=False,
-        null=True,
-        blank=True,
-        related_name="authorized_storage_account",
-    )
-    _temporary_oauth1_credentials = models.OneToOneField(
-        "addon_service.ExternalCredentials",
-        on_delete=models.CASCADE,
-        primary_key=False,
-        null=True,
-        blank=True,
-        related_name="temporary_authorized_storage_account",
-    )
-    oauth2_token_metadata = models.ForeignKey(
-        "addon_service.OAuth2TokenMetadata",
-        on_delete=models.CASCADE,  # probs not
-        null=True,
-        blank=True,
-        related_name="authorized_storage_accounts",
-        related_query_name="%(class)s_authorized_storage_account",
-    )
-
     class Meta:
         abstract = True
 

addon_service/abstract/authorized_account/serializers.py

@@ -1,58 +1,49 @@
+from __future__ import annotations
+
 from abc import abstractmethod
+from typing import TYPE_CHECKING
 
 from django.core.exceptions import ValidationError as ModelValidationError
 from rest_framework_json_api import serializers
-from rest_framework_json_api.utils import get_resource_type_from_model
 
-from addon_service.abstract.authorized_account.models import AuthorizedAccount
-from addon_service.addon_operation.models import AddonOperationModel
-from addon_service.common import view_names
 from addon_service.common.credentials_formats import CredentialsFormats
-from addon_service.models import (
-    AuthorizedStorageAccount,
-    ExternalStorageService,
-    UserReference,
-)
 from addon_service.osf_models.fields import encrypt_string
 from addon_service.serializer_fields import (
     CredentialsField,
-    DataclassRelatedLinkField,
     EnumNameMultipleChoiceField,
-    ReadOnlyResourceRelatedField,
 )
 from addon_toolkit import AddonCapabilities
 
 
-RESOURCE_TYPE = get_resource_type_from_model(AuthorizedStorageAccount)
+if TYPE_CHECKING:
+    from addon_service.abstract.authorized_account.models import AuthorizedAccount
+    from addon_service.models import ExternalStorageService
+
+REQUIRED_FIELDS = frozenset(["url", "account_owner", "authorized_operations"])
 
 
 class AuthorizedAccountSerializer(serializers.HyperlinkedModelSerializer):
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
 
+        # Check whether subclasses declare all of required fields
+        if not REQUIRED_FIELDS.issubset(set(self.fields.keys())):
+            raise Exception(
+                f"{self.__class__.__name__} requires {self.REQUIRED_FIELDS} to be instantiated"
+            )
+
         # Check if it's a POST request and remove the field as it's not in our FE spec
         if "context" in kwargs and kwargs["context"]["request"].method == "POST":
             self.fields.pop("configured_storage_addons", None)
 
-    url = serializers.HyperlinkedIdentityField(
-        view_name=view_names.detail_view(RESOURCE_TYPE), required=False
-    )
     authorized_capabilities = EnumNameMultipleChoiceField(enum_cls=AddonCapabilities)
     authorized_operation_names = serializers.ListField(
         child=serializers.CharField(),
         read_only=True,
     )
     auth_url = serializers.CharField(read_only=True)
     api_base_url = serializers.URLField(allow_blank=True, required=False)
-    account_owner = ReadOnlyResourceRelatedField(
-        many=False,
-        queryset=UserReference.objects.all(),
-        related_link_view_name=view_names.related_view(RESOURCE_TYPE),
-    )
-    authorized_operations = DataclassRelatedLinkField(
-        dataclass_model=AddonOperationModel,
-        related_link_view_name=view_names.related_view(RESOURCE_TYPE),
-    )
+
     credentials = CredentialsField(write_only=True, required=False)
     initiate_oauth = serializers.BooleanField(write_only=True, required=False)
 

addon_service/addon_imp/instantiation.py

@@ -1,3 +1,5 @@
+from __future__ import annotations
+
 from typing import TYPE_CHECKING
 
 from asgiref.sync import async_to_sync
@@ -15,9 +17,9 @@
 
 
 async def get_storage_addon_instance(
-    imp_cls: "type[StorageAddonImp]",
-    account: "AuthorizedStorageAccount",
-    config: "StorageConfig",
+    imp_cls: type[StorageAddonImp],
+    account: AuthorizedStorageAccount,
+    config: StorageConfig,
 ) -> StorageAddonImp:
     """create an instance of a `StorageAddonImp`
 

addon_service/addon_operation_invocation/models.py

@@ -84,9 +84,10 @@ def clean_fields(self, *args, **kwargs):
                 {"thru_addon": "thru_addon and thru_account must agree"}
             )
 
+    @property
     def storage_imp_config(self) -> StorageConfig:
         if self.thru_addon:
-            return self.thru_addon.storage_imp_config()
+            return self.thru_addon.storage_imp_config
         return self.thru_account.storage_imp_config
 
     def set_exception(self, exception: BaseException) -> None:

addon_service/authorized_storage_account/models.py

@@ -21,6 +21,36 @@ class AuthorizedStorageAccount(AuthorizedAccount):
         related_name="authorized_storage_accounts",
     )
 
+    account_owner = models.ForeignKey(
+        "addon_service.UserReference",
+        on_delete=models.CASCADE,
+        related_name="authorized_storage_accounts",
+    )
+    _credentials = models.OneToOneField(
+        "addon_service.ExternalCredentials",
+        on_delete=models.CASCADE,
+        primary_key=False,
+        null=True,
+        blank=True,
+        related_name="authorized_storage_account",
+    )
+    _temporary_oauth1_credentials = models.OneToOneField(
+        "addon_service.ExternalCredentials",
+        on_delete=models.CASCADE,
+        primary_key=False,
+        null=True,
+        blank=True,
+        related_name="temporary_authorized_storage_account",
+    )
+    oauth2_token_metadata = models.ForeignKey(
+        "addon_service.OAuth2TokenMetadata",
+        on_delete=models.CASCADE,  # probs not
+        null=True,
+        blank=True,
+        related_name="authorized_storage_accounts",
+        related_query_name="%(class)s_authorized_storage_account",
+    )
+
     class Meta:
         verbose_name = "Authorized Storage Account"
         verbose_name_plural = "Authorized Storage Accounts"
@@ -35,7 +65,9 @@ def external_service(self) -> ExternalStorageService:
 
     async def execute_post_auth_hook(self, auth_extras: dict | None = None):
         imp = await get_storage_addon_instance(
-            self.imp_cls, self, self.storage_imp_config
+            self.imp_cls,
+            self,
+            self.storage_imp_config,
         )
         self.external_account_id = await imp.get_external_account_id(auth_extras or {})
         await self.asave()

addon_service/authorized_storage_account/serializers.py

@@ -9,7 +9,12 @@
 from addon_service.abstract.authorized_account.serializers import (
     AuthorizedAccountSerializer,
 )
+from addon_service.addon_operation.models import AddonOperationModel
 from addon_service.common import view_names
+from addon_service.common.serializer_fields import (
+    DataclassRelatedLinkField,
+    ReadOnlyResourceRelatedField,
+)
 from addon_service.models import (
     AuthorizedStorageAccount,
     ConfiguredStorageAddon,
@@ -33,6 +38,18 @@ class AuthorizedStorageAccountSerializer(AuthorizedAccountSerializer):
         related_link_view_name=view_names.related_view(RESOURCE_TYPE),
         required=False,
     )
+    url = serializers.HyperlinkedIdentityField(
+        view_name=view_names.detail_view(RESOURCE_TYPE), required=False
+    )
+    account_owner = ReadOnlyResourceRelatedField(
+        many=False,
+        queryset=UserReference.objects.all(),
+        related_link_view_name=view_names.related_view(RESOURCE_TYPE),
+    )
+    authorized_operations = DataclassRelatedLinkField(
+        dataclass_model=AddonOperationModel,
+        related_link_view_name=view_names.related_view(RESOURCE_TYPE),
+    )
 
     included_serializers = {
         "account_owner": "addon_service.serializers.UserReferenceSerializer",

addon_service/configured_storage_addon/models.py

@@ -117,6 +117,7 @@ def external_service(self):
     def imp_cls(self) -> type[AddonImp]:
         return self.base_account.imp_cls
 
+    @property
     def storage_imp_config(self) -> StorageConfig:
         return dataclasses.replace(
             self.base_account.storage_imp_config,

addon_service/oauth2/models.py

@@ -1,3 +1,5 @@
+from __future__ import annotations
+
 from datetime import timedelta
 from typing import TYPE_CHECKING
 
@@ -17,7 +19,7 @@
 
 
 if TYPE_CHECKING:
-    from addon_service.abstract.authorized_account import AuthorizedAccount
+    from addon_service.abstract.authorized_account.models import AuthorizedAccount
 
 
 class OAuth2ClientConfig(AddonsServiceBaseModel):
@@ -115,7 +117,7 @@ def validate_shared_client(self):
     @transaction.atomic
     def update_with_fresh_token(
         self, fresh_token_result: FreshTokenResult
-    ) -> "tuple[AuthorizedAccount]":
+    ) -> tuple[AuthorizedAccount]:
         # update this record's fields
         self.state_nonce = None  # one-time-use, now used
         self.refresh_token = fresh_token_result.refresh_token

addon_service/tasks/invocation.py

@@ -22,7 +22,7 @@ def perform_invocation__blocking(invocation: AddonOperationInvocation) -> None:
             _imp = get_storage_addon_instance__blocking(
                 invocation.imp_cls,  # type: ignore[arg-type]  #(TODO: generic impstantiation)
                 invocation.thru_account,
-                invocation.storage_imp_config(),
+                invocation.storage_imp_config,
             )
             _operation = invocation.operation
             # inner transaction to contain database errors,

addon_service/tests/_helpers.py

@@ -1,3 +1,5 @@
+from __future__ import annotations
+
 import contextlib
 import dataclasses
 import secrets
@@ -175,7 +177,7 @@ async def _route_post(self, url, *args, **kwargs):
 
 @dataclasses.dataclass
 class MockOAuth1ServiceProvider:
-    _external_service: "ExternalStorageService"
+    _external_service: ExternalStorageService
     _static_request_token: str
     _static_request_secret: str
     _static_verifier: str