blb-ventures / strawberry-django-plus

Enhanced Strawberry GraphQL integration with Django
MIT License
179 stars 47 forks source link

Can't make a union of `gql.Connection | OperationMessage` (part 2) #225

Closed pomidoroshev closed 1 year ago

pomidoroshev commented 1 year ago

The problem of the original issue #223 was fixed, but there's another one:

  File "<...>/lib/python3.11/site-packages/strawberry_django_plus/relay.py", line 1079, in resolve_connection
    return return_type.from_nodes(nodes, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'StrawberryUnion' object has no attribute 'from_nodes'
pomidoroshev commented 1 year ago

I tried a naive fix with a test, and it turned out that there's another problem: StrawberryUnion can't match the types strawberry.types.types.Connection and FruitConnection and raises UnallowedReturnTypeForUnion:

  File "<...>/site-packages/strawberry/union.py", line 201, in _resolve_union_type
    raise UnallowedReturnTypeForUnion(
strawberry.exceptions.UnallowedReturnTypeForUnion: The type "<class 'strawberry.types.types.Connection'>" of the field "fruitsOrError" is not in the list of the types of the union: "['FruitConnection', 'OperationMessage']"

Github action log: https://github.com/pomidoroshev/strawberry-django-plus/actions/runs/5179448035/jobs/9332339724#step:7:952

bellini666 commented 1 year ago

@pomidoroshev hrm, I overlooked that =P

You probably need to resolve the union the same way as in the other function above it. It is not great doing that twice, but I think it is not a major issue, specially because this PR should soon be merged and the way of resolving this will be greatly simplified.

Feel free to try to send a PR for that. Otherwise I will try to fix that tomorrow :)

pomidoroshev commented 1 year ago

@bellini666

You probably need to resolve the union the same way as in the other function above it.

Yep, I did that, just copy-pasted your solution https://github.com/pomidoroshev/strawberry-django-plus/commit/7c21431e9136e9812360a692074406e9d3db9daa#diff-bf0a824c6ab1aff4a9b72c271bc38ebeae4db3b82f27073eccfc2db59d34f534R1078-R1089

The problem happens afterwards, when the nodes are resolved, and Strawberry checks the actual returned types against the types inside the union. The union contains FruitConnection, the returned value is strawberry.types.types.Connection, and it causes UnallowedReturnTypeForUnion

pomidoroshev commented 1 year ago

I submitted a PR to illustrate the problem of UnallowedReturnTypeForUnion #226

bellini666 commented 1 year ago

Hey @pomidoroshev ,

There's indeed something strange happening here..

Having said that, the relay PR got merged yesterday on strawberry and I'll try to refactor strawberrry-django-plus to use it this weekend. If everything goes accordingly to the plan, this issue will be solved as well.

bellini666 commented 1 year ago

@pomidoroshev I just released the new version which uses the official integration. Could you check if it solves your issue?

pomidoroshev commented 1 year ago

@bellini666 Unfortunately, no. I haven't fixed the test in the related PR yet, but I tried on my own project.

The following code works fine:

from strawberry import relay
from strawberry_django_plus import gql
from strawberry_django_plus.permissions import IsAuthenticated
from strawberry_django_plus.types import OperationMessage

@gql.django.type(Profile)
class ProfileType(relay.Node):
    first_name: str
    last_name: str
    # ...

@gql.type
class ProfilesQuery:
    all_profiles: relay.ListConnection[ProfileType] = gql.django.connection(
        directives=[IsAuthenticated()]
    )

But if I add | OperationMessage, it raises TypeError: Query fields cannot be resolved.:

@gql.type
class ProfilesQuery:
    all_profiles: relay.ListConnection[ProfileType] | OperationMessage = gql.django.connection(
        directives=[IsAuthenticated()]
    )
Stack trace ``` @cached_property def fields(self) -> GraphQLFieldMap: """Get provided fields, wrapping them as GraphQLFields if needed.""" try: > fields = resolve_thunk(self._fields) /<...>/python3.11/site-packages/graphql/type/definition.py:808: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ thunk = . at 0x105e14c20> def resolve_thunk(thunk: Thunk[T]) -> T: """Resolve the given thunk. Used while defining GraphQL types to allow for circular references in otherwise immutable type definitions. """ > return thunk() if callable(thunk) else thunk /<...>/python3.11/site-packages/graphql/type/definition.py:300: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ > fields=lambda: self.get_graphql_fields(object_type), interfaces=list(map(self.from_interface, object_type.interfaces)), description=object_type.description, is_type_of=_get_is_type_of(), extensions={ GraphQLCoreConverter.DEFINITION_BACKREF: object_type, }, ) /<...>/python3.11/site-packages/strawberry/schema/schema_converter.py:438: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ self = type_definition = TypeDefinition(name='Query', is_input=False, is_interface=False, origin=, ...,hash=None,compare=True,metadata=mappingproxy({}),kw_only=True,_field_type=_FIELD)], concrete_of=None, type_var_map={}) def get_graphql_fields( self, type_definition: TypeDefinition ) -> Dict[str, GraphQLField]: > return self._get_thunk_mapping( type_definition=type_definition, name_converter=self.config.name_converter.from_field, field_converter=self.from_field, ) /<...>/python3.11/site-packages/strawberry/schema/schema_converter.py:327: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ type_definition = TypeDefinition(name='Query', is_input=False, is_interface=False, origin=, ...,hash=None,compare=True,metadata=mappingproxy({}),kw_only=True,_field_type=_FIELD)], concrete_of=None, type_var_map={}) name_converter = > field_converter = > @staticmethod def _get_thunk_mapping( type_definition: TypeDefinition, name_converter: Callable[[StrawberryField], str], field_converter: Callable[[StrawberryField], FieldType], ) -> Dict[str, FieldType]: """Create a GraphQL core `ThunkMapping` mapping of field names to field types. This method filters out remaining `strawberry.Private` annotated fields that could not be filtered during the initialization of a `TypeDefinition` due to postponed type-hint evaluation (PEP-563). Performing this filtering now (at schema conversion time) ensures that all types to be included in the schema should have already been resolved. Raises: TypeError: If the type of a field in ``fields`` is `UNRESOLVED` """ thunk_mapping = {} for field in type_definition.fields: if field.type is UNRESOLVED: raise UnresolvedFieldTypeError(type_definition, field) if not is_private(field.type): > thunk_mapping[name_converter(field)] = field_converter(field) /<...>/python3.11/site-packages/strawberry/schema/schema_converter.py:320: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ self = field = Field(name='all_profiles',type=,default=,init=True,repr=True,hash=None,compare=True,metadata=mappingproxy({}),kw_only=True,_field_type=_FIELD) def from_field(self, field: StrawberryField) -> GraphQLField: # self.from_resolver needs to be called before accessing field.type because # in there a field extension might want to change the type during its apply > resolver = self.from_resolver(field) /<...>/python3.11/site-packages/strawberry/schema/schema_converter.py:250: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ self = field = Field(name='all_profiles',type=,default=,init=True,repr=True,hash=None,compare=True,metadata=mappingproxy({}),kw_only=True,_field_type=_FIELD) def from_resolver( self, field: StrawberryField ) -> Callable: # TODO: Take StrawberryResolver field.default_resolver = self.config.default_resolver if field.is_basic_field: def _get_basic_result(_source: Any, *args: str, **kwargs: Any): # Call `get_result` without an info object or any args or # kwargs because this is a basic field with no resolver. return field.get_result(_source, info=None, args=[], kwargs={}) _get_basic_result._is_default = True # type: ignore return _get_basic_result def _get_arguments( source: Any, info: Info, kwargs: Any, ) -> Tuple[List[Any], Dict[str, Any]]: # TODO: An extension might have changed the resolver arguments, # but we need them here since we are calling it. # This is a bit of a hack, but it's the easiest way to get the arguments # This happens in mutation.InputMutationExtension field_arguments = field.arguments[:] if field.base_resolver: existing = {arg.python_name for arg in field_arguments} field_arguments.extend( [ arg for arg in field.base_resolver.arguments if arg.python_name not in existing ] ) kwargs = convert_arguments( kwargs, field_arguments, scalar_registry=self.scalar_registry, config=self.config, ) # the following code allows to omit info and root arguments # by inspecting the original resolver arguments, # if it asks for self, the source will be passed as first argument # if it asks for root, the source it will be passed as kwarg # if it asks for info, the info will be passed as kwarg args = [] if field.base_resolver: if field.base_resolver.self_parameter: args.append(source) root_parameter = field.base_resolver.root_parameter if root_parameter: kwargs[root_parameter.name] = source info_parameter = field.base_resolver.info_parameter if info_parameter: kwargs[info_parameter.name] = info return args, kwargs def _check_permissions(source: Any, info: Info, kwargs: Any): """ Checks if the permission should be accepted and raises an exception if not """ for permission_class in field.permission_classes: permission = permission_class() if not permission.has_permission(source, info, **kwargs): message = getattr(permission, "message", None) raise PermissionError(message) async def _check_permissions_async(source: Any, info: Info, kwargs: Any): for permission_class in field.permission_classes: permission = permission_class() has_permission: bool has_permission = await await_maybe( permission.has_permission(source, info, **kwargs) ) if not has_permission: message = getattr(permission, "message", None) raise PermissionError(message) def _strawberry_info_from_graphql(info: GraphQLResolveInfo) -> Info: return Info( _raw_info=info, _field=field, ) def _get_result( _source: Any, info: Info, field_args: List[Any], field_kwargs: Dict[str, Any], ): return field.get_result( _source, info=info, args=field_args, kwargs=field_kwargs ) def wrap_field_extensions() -> Callable[..., Any]: """Wrap the provided field resolver with the middleware.""" for extension in field.extensions: extension.apply(field) extension_functions = build_field_extension_resolvers(field) def extension_resolver( _source: Any, info: Info, **kwargs: Any, ): # parse field arguments into Strawberry input types and convert # field names to Python equivalents field_args, field_kwargs = _get_arguments( source=_source, info=info, kwargs=kwargs ) resolver_requested_info = False if "info" in field_kwargs: resolver_requested_info = True # remove info from field_kwargs because we're passing it # explicitly to the extensions field_kwargs.pop("info") # `_get_result` expects `field_args` and `field_kwargs` as # separate arguments so we have to wrap the function so that we # can pass them in def wrapped_get_result(_source: Any, info: Info, **kwargs: Any): # if the resolver function requested the info object info # then put it back in the kwargs dictionary if resolver_requested_info: kwargs["info"] = info return _get_result( _source, info, field_args=field_args, field_kwargs=kwargs ) # combine all the extension resolvers return reduce( lambda chained_fn, next_fn: partial(next_fn, chained_fn), extension_functions, wrapped_get_result, )(_source, info, **field_kwargs) return extension_resolver > _get_result_with_extensions = wrap_field_extensions() /<...>/python3.11/site-packages/strawberry/schema/schema_converter.py:607: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ def wrap_field_extensions() -> Callable[..., Any]: """Wrap the provided field resolver with the middleware.""" for extension in field.extensions: > extension.apply(field) /<...>/python3.11/site-packages/strawberry/schema/schema_converter.py:563: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ self = field = Field(name='all_profiles',type=,default=,init=True,repr=True,hash=None,compare=True,metadata=mappingproxy({}),kw_only=True,_field_type=_FIELD) def apply(self, field: StrawberryDjangoField) -> None: # NOTE: Because we have a base_resolver defined, our parents will not add # order/filters resolvers in here, so we need to add them by hand (unless they # are somewhat in there). We are not adding pagination because it doesn't make # sense together with a Connection > args_names = {a.python_name for a in field.arguments} /<...>/python3.11/site-packages/strawberry_django_plus/field.py:345: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ self = Field(name='all_profiles',type=,default=,init=True,repr=True,hash=None,compare=True,metadata=mappingproxy({}),kw_only=True,_field_type=_FIELD) @property def arguments(self) -> List[StrawberryArgument]: > args = super().arguments /<...>/python3.11/site-packages/strawberry_django_plus/field.py:99: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ self = Field(name='all_profiles',type=,default=,init=True,repr=True,hash=None,compare=True,metadata=mappingproxy({}),kw_only=True,_field_type=_FIELD) @property def arguments(self) -> List[StrawberryArgument]: arguments = [] if not self.base_resolver: pagination = self.get_pagination() if pagination and pagination is not UNSET: arguments.append(argument("pagination", OffsetPaginationInput)) > return super().arguments + arguments /<...>/python3.11/site-packages/strawberry_django/pagination.py:40: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ self = Field(name='all_profiles',type=,default=,init=True,repr=True,hash=None,compare=True,metadata=mappingproxy({}),kw_only=True,_field_type=_FIELD) @property def arguments(self) -> List[StrawberryArgument]: arguments = [] if not self.base_resolver: > order = self.get_order() /<...>/python3.11/site-packages/strawberry_django/ordering.py:69: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ self = Field(name='all_profiles',type=,default=,init=True,repr=True,hash=None,compare=True,metadata=mappingproxy({}),kw_only=True,_field_type=_FIELD) def get_order(self) -> Optional[Type]: # FIXME: This should be done on strawberry-graphql-django order = super().get_order() if order in (None, UNSET): > t_origin = self.type_origin /<...>/python3.11/site-packages/strawberry_django_plus/field.py:154: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ self = instance = Field(name='all_profiles',type=,default=,init=True,repr=True,hash=None,compare=True,metadata=mappingproxy({}),kw_only=True,_field_type=_FIELD) owner = def __get__(self, instance, owner=None): if instance is None: return self if self.attrname is None: raise TypeError( "Cannot use cached_property instance without calling __set_name__ on it.") try: cache = instance.__dict__ except AttributeError: # not all objects have __dict__ (e.g. class defines slots) msg = ( f"No '__dict__' attribute on {type(instance).__name__!r} " f"instance to cache {self.attrname!r} property." ) raise TypeError(msg) from None val = cache.get(self.attrname, _NOT_FOUND) if val is _NOT_FOUND: with self.lock: # check if another thread filled cache while we awaited lock val = cache.get(self.attrname, _NOT_FOUND) if val is _NOT_FOUND: > val = self.func(instance) /<...>/lib/python3.11/functools.py:1001: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ self = Field(name='all_profiles',type=,default=,init=True,repr=True,hash=None,compare=True,metadata=mappingproxy({}),kw_only=True,_field_type=_FIELD) @cached_property def type_origin(self) -> Type: origin = self.type tdef = cast(Optional[TypeDefinition], getattr(origin, "_type_definition", None)) if tdef and tdef.concrete_of and issubclass(tdef.concrete_of.origin, relay.Connection): origin = tdef.type_var_map[relay.NodeType] # type: ignore if isinstance(origin, LazyType): origin = origin.resolve_type() while isinstance(origin, StrawberryContainer): origin = origin.of_type if isinstance(origin, StrawberryUnion): olist = [] for t in origin.types: while isinstance(t, StrawberryContainer): t = t.of_type # noqa: PLW2901 if hasattr(t, "_django_type"): olist.append(t) > assert len(olist) == 1 E AssertionError /<...>/python3.11/site-packages/strawberry_django_plus/field.py:192: AssertionError The above exception was the direct cause of the following exception: @pytest.fixture() def graphql_url(): > return reverse("graphql") ../../../../pytest_plugins/graphql.py:8: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ /<...>/python3.11/site-packages/django/urls/base.py:88: in reverse return resolver._reverse_with_prefix(view, prefix, *args, **kwargs) /<...>/python3.11/site-packages/django/urls/resolvers.py:746: in _reverse_with_prefix self._populate() /<...>/python3.11/site-packages/django/urls/resolvers.py:543: in _populate for url_pattern in reversed(self.url_patterns): /<...>/python3.11/site-packages/django/utils/functional.py:57: in __get__ res = instance.__dict__[self.name] = self.func(instance) /<...>/python3.11/site-packages/django/urls/resolvers.py:715: in url_patterns patterns = getattr(self.urlconf_module, "urlpatterns", self.urlconf_module) /<...>/python3.11/site-packages/django/utils/functional.py:57: in __get__ res = instance.__dict__[self.name] = self.func(instance) /<...>/python3.11/site-packages/django/urls/resolvers.py:708: in urlconf_module return import_module(self.urlconf_name) /<...>/lib/python3.11/importlib/__init__.py:126: in import_module return _bootstrap._gcd_import(name[level:], package, level) ../../../../urls.py:6: in from myproject.core.graphql.schema import schema ../../../graphql/schema.py:39: in schema = Schema( /<...>/python3.11/site-packages/strawberry/schema/schema.py:140: in __init__ self._schema = GraphQLSchema( /<...>/python3.11/site-packages/graphql/type/schema.py:224: in __init__ collect_referenced_types(query) /<...>/python3.11/site-packages/graphql/type/schema.py:432: in collect_referenced_types for field in named_type.fields.values(): /<...>/lib/python3.11/functools.py:1001: in __get__ val = self.func(instance) _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ self = @cached_property def fields(self) -> GraphQLFieldMap: """Get provided fields, wrapping them as GraphQLFields if needed.""" try: fields = resolve_thunk(self._fields) except Exception as error: cls = GraphQLError if isinstance(error, GraphQLError) else TypeError > raise cls(f"{self.name} fields cannot be resolved. {error}") from error E TypeError: Query fields cannot be resolved. /<...>/python3.11/site-packages/graphql/type/definition.py:811: TypeError ```
pomidoroshev commented 1 year ago

Since the relay mode is a native Strawberry feature, I tested it there and submitted an issue as well https://github.com/strawberry-graphql/strawberry/issues/2893

bellini666 commented 1 year ago

@pomidoroshev was actually going to comment that here :)

Let's continue this discussion there