Skip to content
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

Object Type Extensions #2605

Open
2 tasks
erikwrede opened this issue Mar 2, 2023 · 6 comments
Open
2 tasks

Object Type Extensions #2605

erikwrede opened this issue Mar 2, 2023 · 6 comments

Comments

@erikwrede
Copy link
Member

erikwrede commented Mar 2, 2023

Currently, Strawberry lacks a mechanism for extending types with reusable logic, such as pagination, database model mappers for libraries like sqlalchemy, or compatibility extensions like strawberry.experimental.pydantic. Much of the logic necessary for these to work has to be written separately for each use case. Opinionating the way we extend strawberry types could standardize behavior and improve compatibility and maintainability. This issue proposes an ObjectTypeExtension API that provides a foundation to address the mentioned points.

API proposal

ObjectTypeExtensions provide custom functionality, can modify the underlying TypeDefinition or may offer the possibility to implement standardized hooks (more on that later).

class ObjectTypeExtension(ABC):

    def apply(strawberry_type: TypeDefinition):
		"""
		In this method, modifications can be made to TypeDefinition. 
		Custom fields or directives can be added here. It will be called 		 
         after the TypeDefinition is initialized
		"""
		pass

A DjangoModelExtension could use apply to setup all the automatic model fields, similar to a PydanticModelExtension or SQLAlchemyModelExtension

Similar to Field Extensions (#2168 , #2567), ObjectTypeExtension instances are passed to the @strawberry.type annotation:

@strawberry.type(extensions=[DjangoModelExtension(model=CarModel)])
class Car:
	...

will resolve to:

type Car{
	id: ID!
	manufacturer: String!
	model: String!
	doors: Int!
}

Extensions and Polymorphism

Using the annotation to define extensions is favorable over polymorphism of the actual MyType class, as that is a dataclass with resolver logic. The Extensions will provide behavioral logic and extended functionality and are a better fit for StrawberryType. Extensions themselves support polymorphism. The DjangoExtension could natively support OffsetPaginationExtension or RelayPaginationExtension.

Initialization

Extensions are initialized after TypeDefinition initialization. Additionally, we can provide helper methods to make dealing with polymorphic extensions easier:

@dataclass
class TypeDefinition(StrawberrryType):
    extensions: List[ObjectTypeExtension]

    def __post_init__():
    	for extension in self.extensions:
        	extension.apply(self)
   			 ...
    		#rest of current post init

    def get_extension(extension_type: Type[ObjectTypeExtension]):
      # extensions can be polymorphic (DjangoModelExtension can inherit from PaginationExtension...)
      return next(filter(self.extensions, lambda x: isinstance(x, extension_type)))

Interacting with Object Type Extensions

A major API to interact with extensions will be the FieldExtensionAPI. In cases like Pagination, FieldExtensions have a synergy with ObjectTypeExtensions by defining the user-facing pagination logic on the FieldExtension and using the ObjectTypeExtension to actually resolve the data. This way, only one FieldExtension is necessary to implement OffsetPagination, which is compatible with both DjangoModelExtension, SQLAlchemyModelExtension and more:

class DjangoModelExtension(OffsetPaginatableTypeExtension, RelayTypeExtension):
	def __init__(model: DjangoModel)
	def resolve_offset_paginated_items(offset, limit):
		# all the django db logic here
@strawberry.type
class Query
   cars: list[Car] = strawberry.field(extensions=[OffsetPaginatedFieldExtension(default_limit=100)])
   relayed_cars: list[Car] = strawberry.field(extensions=[RelayPaginatedFieldExtension()])

resolves to

type Query   {
	cars(offset: Int, limit: Int =  100): [Car!]!
	relayedCars(
    	before: String
		after: String
		first: Int
		last: Int
  ): CarConnection!
}

using

class OffsetPaginatedFieldExtension(FieldExtension):
   def apply(field: StrawberryField):
		assert isinstance(field.type, StrawberryList)
		# side note: some helpers may be useful here
		listed_type : TypeDefinition = get_nested_type_defintion(field)
		# Returns DjangoModelExtension 	in this case
		self.offset_pagination_extension = listed_type.get_extension(OffsetPaginatableTypeExtension) 
		assert offset_pagination_extension
		# add necessary arguments etc
	
	def resolve(info, offset, limit):
		# shortened
		return self.offset_pagination_extension.resolve_offset_paginated_items(info, offset, limit)

Using this approach streamlines user-facing behavior and helps standardize the internal logic for all extensions of strawberry. Filtering or sorting are other great options to use the combination of FieldExtensions and ObjectTypeExtensions

Decisions

  • Dealing with type hints
    We need to decide how to handle automatic database model-derived fields. Should we require strawberry.auto-typed fields on the actual types (similar to the current pydantic extension), or can the user just pass an empty object type? strawberry.auto provides little benefit to the user in case of manual use, as it will not reveal any type information. As such, it might be better to only enfore its use in override-cases (e.g. change the default description, type or alias a model field)

  • Implement hooks
    Hooks such as wrap_resolve wrapping the resolver of each object type could provide additonal on-resolve functionality.
    Cases like unnecessary database calls just to resolve an ID field may be avoided using wrap_resolve by parsing the selection_set before any resolver is actually called. However, their use might be an antipattern to the proposed ObjectTypeExtension + FieldExtension synergy as it's easier and more explicit to implement that using FieldExtensions. My personal preference is to not provide standardized resolve-time hooks and implement that functionality using FieldExtensions instead.

Upvote & Fund

  • We're using Polar.sh so you can upvote and help fund this issue.
  • We receive the funding once the issue is completed & confirmed by you.
  • Thank you in advance for helping prioritize & fund our backlog.
Fund with Polar
@bellini666
Copy link
Member

I really like the idea!

Speaking from the experience I have defining custom type transformers and StrawberryField subclasses for strawberry-django-plus and now on strawberry-graphql-django, there are 2 issues that I can see being solved by this proposal:

  1. The auto type, which requires some extra config (e.g. the "django model) and some introspection to generate it.

  2. Related 1, when processing the type those integrations will usually replace StrawberryFields by their own implementations, to be able to add extra "magic" arguments and also to customize how the resolver should work in case a custom resolver for the field doesn't exist.

1 can probably be solved by also adding an optional resolve_auto function. Then, when finding an auto annotation the type processing could ask the extensions to solve it. It can ask in the same order it was defined, and the first resolved type is used.

2 can probably be solved by apply/resolve

I wonder if the logic to add extra arguments wouldn't be too complicated in apply instead of maybe providing a hook get_extra_field_args

Also, if I understood correctly, that resolve would then be used in place of the field's get_result in case it doesn't have a base_resolver (which by default would do getattr(type, field_name)? If so, this should solve the issue for the custom resolver I mentioned.

@erikwrede
Copy link
Member Author

The auto type, which requires some extra config (e.g. the "django model) and some introspection to generate it.
2 options here:

  1. We just move the entire auto logic into an implementation ObjectTypeExtension.apply (called after TypeDefinition initialization) which collects all auto fields and replaces them with the correctly typed fields based on the model.

  2. We add native support for an abstract AutoFieldObjectTypeExtension which is implemented by DjangoModelExtension. Whenever encountering an auto field, we use get_extension(AutoFieldObjectTypeExtension) to do the conversion.

I prefer the first variant because it follows the extension approach closely by only extending the functionality and properties of a TypeDefinition without actually requiring modification the the internals of the extension.

Regarding auto's future use, we'd need to standardize an API for overrides (e.g. don't use the automatic description from a SQLAlchemy Model Field, or use every info from a Django Column, but I want a different Type + these extensions). Maybe that's better suited as a follow-up.

Related 1, when processing the type those integrations will usually replace StrawberryFields by their own implementations, to be able to add extra "magic" arguments and also to customize how the resolver should work in case a custom resolver for the field doesn't exist.

Exactly, we don't need custom StrawberryFields for that anymore since we've got FieldExtensions now. The ObjectTypeExtensions synergize with the FieldExtensions by providing the data for the FieldExtension's interface (e.g. resolvers, etc)

I wonder if the logic to add extra arguments wouldn't be too complicated in apply instead of maybe providing a hook get_extra_field_args
Sounds like a great follow-up for the FieldExtension PR!

Also, if I understood correctly, that resolve would then be used in place of the field's get_result in case it doesn't have a base_resolver (which by default would do getattr(type, field_name)? If so, this should solve the issue for the custom resolver I mentioned.
Yes that seems to be accurate.

I believe a great way to go forward with this would be to prototype ObjectTypeExtensions and try to re-implement the custom strawberry-django Type as an ObjectTypeExtension with FieldExtensions. There's a lot to be thought of here, but we have the chance to standardize a lot of logic for good.

@jkimbo
Copy link
Member

jkimbo commented Mar 3, 2023

I believe a great way to go forward with this would be to prototype ObjectTypeExtensions and try to re-implement the custom strawberry-django Type as an ObjectTypeExtension with FieldExtensions. There's a lot to be thought of here, but we have the chance to standardize a lot of logic for good.

@erikwrede I think this is a very good idea and would help expose any flaws in the API quickly. The pydantic integration would also be a good candidate for conversion and might be a simpler one to do first.

@erikwrede
Copy link
Member Author

@jkimbo Pydantic is a great start, but I'm worried we won't catch some of the implications that result from integrating a Database in the background, especially when it comes to pagination extensions. Definitely great to simplify things for the initial prototype though

@erikwrede
Copy link
Member Author

erikwrede commented Mar 6, 2023

Prototype (WIP) here: #2612
Implementing this for all strawberry.auto cases was much easier than I expected with the added bonus of not having to redefine the strawberry type annotation for each extension anymore.
Check out the new test class for usage 🙂

I also renamed the ObjectTypeExtensions to TypeExtensions in the PR since it seems to work perfectly fine for Input- and Interface types, too.

@Kitefiko
Copy link

Kitefiko commented Feb 11, 2024

Hello,
I have created prototype #3379 of my take on object extending. I think this version allows more freedom.
The question that I would have is how much freedom should there be?
For example I have constrained the ability to completely override _get_fields because it seemed like too much.

The biggest difference vs previous prototype is the ability to to change StrawberryObjectDefinition and extend it for custom methods etc. Having list[StrawberryTypeExtension] and then iterating over them all would limit modification of StrawberryObjectDefinition to metadata field only, but is more friendly and simpler API.

Sample usage might look something like this:

class StrawberryDjangoField(StrawberryField):
    def __init__(self, *args, model: DjangoModel, **kwargs):
        super().__init__(*args, **kwargs)
        self._model = model


class StrawberryDjangoObjectDefinition(StrawberryObjectDefinition):
    model: DjangoModel


class DjangoTypeExtension(TypeExtension):
    def __init__(
        self, 
        model: DjangoModel, 
        field_class: type[StrawberryDjangoField], 
        fields: list[str] | None,
    ):
        self._model = model
        self._field_class = field_class
        self._fields = fields

    def before_wrap_dataclass(self, cls: type):
        # add fields from model
        for field_name in self._fields or []:
            cls.__annotations__[field_name] = strawberry.auto

    def on_field(self, field: dataclasses.Field[Any]):
        if is_auto(field):
            field = StrawberryField( 
                python_name=field.name,
                graphql_name=None,
                type_annotation=type_for_field(self._model, field.name),
            )

        return field
    
    def create_object_definition(self, *args, **kwargs):
        return StrawberryDjangoObjectDefinition(
            *args, model=self._model, **kwargs
        )

@strawberry.type(extension=DjangoTypeExtension(model=..., field_cls=..., fields=...))
class Something:
    pass

# Or restrict to extension arguments

def django_type(
    model: type[DjangoModel],
    *,
    name: str | None = None,
    field_cls: type[StrawberryDjangoField] = StrawberryDjangoField,
    is_input: bool = False,
    is_interface: bool = False,
    description: str | None = None,
    directives: Sequence[object] | None = (),
    extend: bool = False,
    fields: list[str] | None = None,
):
    extension = DjangoTypeExtension(
        model=model,
        field_cls=field_cls,
        fields=fields,
    )

    return strawberry.type(
        name=name,
        is_input=is_input,
        is_interface=is_interface,
        description=description,
        directives=directives,
        extend=extend,
        extension=extension ,
    )

EDIT:
Cleaned up draft and renamed builder to TypeExtension

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants