strawberry-graphql / strawberry-django

Strawberry GraphQL Django extension
MIT License
393 stars 115 forks source link

Mutations on proxy models issue #388

Open sdobbelaere opened 9 months ago

sdobbelaere commented 9 months ago

It would seem that mutations on proxy-models aren't working in my setup. Can you see the mistake in my setup?

A simplified, reproducible example with details included:

Managers:

from django.db.models import Queryset, Manager

class ClientQueryset(Queryset):
    def create(self, **kwargs):
        kwargs.update({'is_client': True})
        return super().create(**kwargs)

class ClientManager(Manager):
    def get_queryset(self):
        return ClientQueryset(self.model, using=self._db).\
            filter(is_client=True)

Models:

from django.db import models
from .managers import ClientManger

class Company(models.Model):
    name = models.CharField(max_length=100)
    is_client = models.BooleanField(default=False)

class Client(Company):
    objects = ClientManager()
    class Meta:
        proxy = True

Based on this setup, I'm perfectly capable of creating the client objects in the classic ORM way:

from contacts.models import Client

client = Client.objects.create(name='client orm')
print(client.is_client, client.name)

Next up, let's setup the strawberry mutations.

schema:

from strawberry import relay, auto
import strawberry
import strawberry_django
from typing import List

from .models import Company, Client

@strawberry_django.filter(Company)
class CompanyFilter:
    id: auto
    name: auto

@strawberry_django.filter(Client)
class ClientFilter:
    id: auto
    name: auto

@strawberry_django.input(Company, fields="__all__")
class CompanyInput:
    pass

@strawberry_django.partial(Company, fields="__all__")
class CompanyPartialInput(strawberry_django.NodeInput):
    pass

@strawberry_django.input(Client, fields="__all__")
class ClientInput:
    pass

@strawberry_django.partial(Client, fields="__all__")
class ClientPartialInput(strawberry_django.NodeInput):
    pass

@strawberry_django.order(Company)
class CompanyOrder:
    name: auto

@strawberry_django.order(Client)
class ClientOrder:
    name: auto

@strawberry_django.type(Company, filters=CompanyFilter, order=CompanyOrder, pagination=True, fields='__all__')
class CompanyType(strawberry.relay.Node):
    pass

@strawberry_django.type(Client, filters=ClientFilter, order=ClientOrder, pagination=True, fields='__all__')
class ClientType(strawberry.relay.Node):
    pass

@strawberry.type
class Mutation:
    create_company: CompanyType = strawberry_django.mutations.create(CompanyInput)
    create_companies: List[CompanyType] = strawberry_django.mutations.create(CompanyInput)
    update_company: CompanyType = strawberry_django.mutations.update(CompanyPartialInput)
    delete_company: CompanyType = strawberry_django.mutations.delete()
    delete_companies: List[CompanyType] = strawberry_django.mutations.delete()

    create_client: ClientType = strawberry_django.mutations.create(ClientInput)
    create_client: List[ClientType] = strawberry_django.mutations.create(ClientInput)
    update_client: ClientType = strawberry_django.mutations.update(ClientPartialInput)
    delete_client: ClientType = strawberry_django.mutations.delete()
    delete_client: List[ClientType] = strawberry_django.mutations.delete()

@strawberry.type
class Query:
    company: CompanyType = strawberry_django.node()
    companies: strawberry_django.relay.ListConnectionWithTotalCount[CompanyType] = strawberry_django.connection()

    client: ClientType = strawberry_django.node()
    clients: strawberry_django.relay.ListConnectionWithTotalCount[ClientType] = strawberry_django.connection()

schema = strawberry.Schema(
    query=Query,
    mutation=Mutation,
)

Now that the schema is setup, let's run some mutations:

First, the base model "Compay":

mutation createCompany{
  createCompany(data:{name: "company"}){
    name
    isClient
  }
}

yields, successfully

{
  "data": {
    "createCompany": {
      "name": "company",
      "isClient": false
    }
  }
}

Now, let's try the proxy-model:

mutation createClient{
  createClient(data:{name: "client"}){
    name
    isClient
  }
}

Yields, unsuccessfully:

{
  "data": null,
  "errors": [
    {
      "message": "Client matching query does not exist.",
      "locations": [
        {
          "line": 2,
          "column": 3
        }
      ],
      "path": [
        "createClient"
      ]
    }
  ]
}

Can you see the mistake in my setup?

Upvote & Fund

Fund with Polar

bellini666 commented 9 months ago

Hrm, I have never used mutations with proxy models, but it should work. Unless we need to workaround any kind of corner case they have.

Do you have the full traceback of the error? I want to check where it happened

sdobbelaere commented 9 months ago

I do, see below:

Client matching query does not exist.

GraphQL request:2:3
1 | mutation createClient{
2 |   createClient(data:{name: "client"}){
  |   ^
3 |     name
Traceback (most recent call last):
  File "/Users/sascha/Downloads/test_django/venv/lib/python3.11/site-packages/graphql/execution/execute.py", line 528, in await_result
    return_type, field_nodes, info, path, await result
                                          ^^^^^^^^^^^^
  File "/Users/sascha/Downloads/test_django/venv/lib/python3.11/site-packages/asgiref/sync.py", line 479, in __call__
    ret: _R = await loop.run_in_executor(
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/Cellar/python@3.11/3.11.5/Frameworks/Python.framework/Versions/3.11/lib/python3.11/concurrent/futures/thread.py", line 58, in run
    result = self.fn(*self.args, **self.kwargs)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/sascha/Downloads/test_django/venv/lib/python3.11/site-packages/asgiref/sync.py", line 538, in thread_handler
    return func(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/Users/sascha/Plugins/strawberry-graphql-django/strawberry_django/resolvers.py", line 97, in async_resolver
    return sync_resolver(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/sascha/Plugins/strawberry-graphql-django/strawberry_django/resolvers.py", line 75, in sync_resolver
    retval = resolver(*args, **kwargs)
             ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/Cellar/python@3.11/3.11.5/Frameworks/Python.framework/Versions/3.11/lib/python3.11/contextlib.py", line 81, in inner
    return func(*args, **kwds)
           ^^^^^^^^^^^^^^^^^^^
  File "/Users/sascha/Plugins/strawberry-graphql-django/strawberry_django/mutations/fields.py", line 233, in resolver
    return [
           ^
  File "/Users/sascha/Plugins/strawberry-graphql-django/strawberry_django/mutations/fields.py", line 234, in <listcomp>
    self.create(resolvers.parse_input(info, vars(d)), info=info)
  File "/Users/sascha/Plugins/strawberry-graphql-django/strawberry_django/mutations/fields.py", line 250, in create
    return resolvers.create(
           ^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/Cellar/python@3.11/3.11.5/Frameworks/Python.framework/Versions/3.11/lib/python3.11/contextlib.py", line 81, in inner
    return func(*args, **kwds)
           ^^^^^^^^^^^^^^^^^^^
  File "/Users/sascha/Plugins/strawberry-graphql-django/strawberry_django/mutations/resolvers.py", line 225, in create
    return update(
           ^^^^^^^
  File "/opt/homebrew/Cellar/python@3.11/3.11.5/Frameworks/Python.framework/Versions/3.11/lib/python3.11/contextlib.py", line 81, in inner
    return func(*args, **kwds)
           ^^^^^^^^^^^^^^^^^^^
  File "/Users/sascha/Plugins/strawberry-graphql-django/strawberry_django/mutations/resolvers.py", line 354, in update
    retval.append(instance.__class__._default_manager.get(pk=instance.pk))
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/sascha/Downloads/test_django/venv/lib/python3.11/site-packages/django/db/models/manager.py", line 87, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/sascha/Downloads/test_django/venv/lib/python3.11/site-packages/django/db/models/query.py", line 637, in get
    raise self.model.DoesNotExist(
contacts.models.Client.DoesNotExist: Client matching query does not exist.
Stack (most recent call last):
  File "/opt/homebrew/Cellar/python@3.11/3.11.5/Frameworks/Python.framework/Versions/3.11/lib/python3.11/threading.py", line 995, in _bootstrap
    self._bootstrap_inner()
  File "/opt/homebrew/Cellar/python@3.11/3.11.5/Frameworks/Python.framework/Versions/3.11/lib/python3.11/threading.py", line 1038, in _bootstrap_inner
    self.run()
  File "/opt/homebrew/Cellar/python@3.11/3.11.5/Frameworks/Python.framework/Versions/3.11/lib/python3.11/threading.py", line 975, in run
    self._target(*self._args, **self._kwargs)
  File "/Users/sascha/Downloads/test_django/venv/lib/python3.11/site-packages/django/utils/autoreload.py", line 64, in wrapper
    fn(*args, **kwargs)
  File "/Users/sascha/Downloads/test_django/venv/lib/python3.11/site-packages/daphne/management/commands/runserver.py", line 135, in inner_run
    ).run()
  File "/Users/sascha/Downloads/test_django/venv/lib/python3.11/site-packages/daphne/server.py", line 149, in run
    reactor.run(installSignalHandlers=self.signal_handlers)
  File "/Users/sascha/Downloads/test_django/venv/lib/python3.11/site-packages/twisted/internet/asyncioreactor.py", line 255, in run
    self._asyncioEventloop.run_forever()
  File "/opt/homebrew/Cellar/python@3.11/3.11.5/Frameworks/Python.framework/Versions/3.11/lib/python3.11/asyncio/base_events.py", line 607, in run_forever
    self._run_once()
  File "/opt/homebrew/Cellar/python@3.11/3.11.5/Frameworks/Python.framework/Versions/3.11/lib/python3.11/asyncio/base_events.py", line 1922, in _run_once
    handle._run()
  File "/opt/homebrew/Cellar/python@3.11/3.11.5/Frameworks/Python.framework/Versions/3.11/lib/python3.11/asyncio/events.py", line 80, in _run
    self._context.run(self._callback, *self._args)
  File "/Users/sascha/Downloads/test_django/venv/lib/python3.11/site-packages/django/contrib/staticfiles/handlers.py", line 101, in __call__
    return await self.application(scope, receive, send)
  File "/Users/sascha/Downloads/test_django/venv/lib/python3.11/site-packages/django/core/handlers/asgi.py", line 160, in __call__
    await self.handle(scope, receive, send)
  File "/Users/sascha/Downloads/test_django/venv/lib/python3.11/site-packages/django/core/handlers/asgi.py", line 183, in handle
    response = await self.get_response_async(request)
  File "/Users/sascha/Downloads/test_django/venv/lib/python3.11/site-packages/django/core/handlers/base.py", line 162, in get_response_async
    response = await self._middleware_chain(request)
  File "/Users/sascha/Downloads/test_django/venv/lib/python3.11/site-packages/django/core/handlers/exception.py", line 42, in inner
    response = await get_response(request)
  File "/Users/sascha/Downloads/test_django/venv/lib/python3.11/site-packages/django/utils/deprecation.py", line 150, in __acall__
    response = response or await self.get_response(request)
  File "/Users/sascha/Downloads/test_django/venv/lib/python3.11/site-packages/django/core/handlers/exception.py", line 42, in inner
    response = await get_response(request)
  File "/Users/sascha/Downloads/test_django/venv/lib/python3.11/site-packages/django/utils/deprecation.py", line 150, in __acall__
    response = response or await self.get_response(request)
  File "/Users/sascha/Downloads/test_django/venv/lib/python3.11/site-packages/django/core/handlers/exception.py", line 42, in inner
    response = await get_response(request)
  File "/Users/sascha/Downloads/test_django/venv/lib/python3.11/site-packages/django/utils/deprecation.py", line 150, in __acall__
    response = response or await self.get_response(request)
  File "/Users/sascha/Downloads/test_django/venv/lib/python3.11/site-packages/django/core/handlers/exception.py", line 42, in inner
    response = await get_response(request)
  File "/Users/sascha/Downloads/test_django/venv/lib/python3.11/site-packages/django/utils/deprecation.py", line 150, in __acall__
    response = response or await self.get_response(request)
  File "/Users/sascha/Downloads/test_django/venv/lib/python3.11/site-packages/django/core/handlers/exception.py", line 42, in inner
    response = await get_response(request)
  File "/Users/sascha/Downloads/test_django/venv/lib/python3.11/site-packages/django/utils/deprecation.py", line 150, in __acall__
    response = response or await self.get_response(request)
  File "/Users/sascha/Downloads/test_django/venv/lib/python3.11/site-packages/django/core/handlers/exception.py", line 42, in inner
    response = await get_response(request)
  File "/Users/sascha/Downloads/test_django/venv/lib/python3.11/site-packages/django/utils/deprecation.py", line 150, in __acall__
    response = response or await self.get_response(request)
  File "/Users/sascha/Downloads/test_django/venv/lib/python3.11/site-packages/django/core/handlers/exception.py", line 42, in inner
    response = await get_response(request)
  File "/Users/sascha/Downloads/test_django/venv/lib/python3.11/site-packages/django/utils/deprecation.py", line 150, in __acall__
    response = response or await self.get_response(request)
  File "/Users/sascha/Downloads/test_django/venv/lib/python3.11/site-packages/django/core/handlers/exception.py", line 42, in inner
    response = await get_response(request)
  File "/Users/sascha/Downloads/test_django/venv/lib/python3.11/site-packages/django/core/handlers/base.py", line 253, in _get_response_async
    response = await wrapped_callback(
  File "/Users/sascha/Downloads/test_django/venv/lib/python3.11/site-packages/strawberry/django/views.py", line 252, in dispatch
    return await self.run(request=request)
  File "/Users/sascha/Downloads/test_django/venv/lib/python3.11/site-packages/strawberry/http/async_base_view.py", line 186, in run
    result = await self.execute_operation(
  File "/Users/sascha/Downloads/test_django/venv/lib/python3.11/site-packages/strawberry/http/async_base_view.py", line 118, in execute_operation
    return await self.schema.execute(
  File "/Users/sascha/Downloads/test_django/venv/lib/python3.11/site-packages/strawberry/schema/schema.py", line 256, in execute
    result = await execute(
  File "/Users/sascha/Downloads/test_django/venv/lib/python3.11/site-packages/strawberry/schema/execute.py", line 156, in execute
    process_errors(result.errors, execution_context)
HTTP POST /graphql/ 200 [0.02, 127.0.0.1:60025]
sdobbelaere commented 9 months ago

At first I thought the _default_manager might be the issue - but it's quite easy to see that it simply uses the same manager as declared on objects. So that shouldn't be it.

sdobbelaere commented 8 months ago

@bellini666 Any thoughts on this?

sdobbelaere commented 8 months ago

ok, after some more digging I'm noticing https://github.com/strawberry-graphql/strawberry-graphql-django/blob/25e3ca2e7952a4299dc18b8a18b559953538a897/strawberry_django/mutations/resolvers.py#L354

    for instance in instances:
        for file_field, value in files:
            file_field.save_form_data(instance, value)

        if pre_save_hook is not None:
            pre_save_hook(instance)

        full_clean_options = full_clean if isinstance(full_clean, dict) else {}
        if full_clean:
            instance.full_clean(**full_clean_options)  # type: ignore

        instance.save()

        for field, value in m2m:
            update_m2m(info, instance, field, value)

        retval.append(instance.__class__._default_manager.get(pk=instance.pk))

What is going wrong?

Reading this code, I see two problems with this:

  1. The least important, but notable: If the goals is to refresh the instance, why not simply call instance.refresh_from_db()
  2. The real issue: the object is created is technically correct, and works tremendously well for updating. However, when creating by setting the fields manually on a fresh instance we're ignoring the adjusted create-methods which is common on proxy-classes. These create-methods are used to assign default values on fields that are the basis of these proxy classes.

Pull request: #394

Feedback, thought and improvements are welcomed.

bellini666 commented 8 months ago

Hey @sdobbelaere,

Sorry for taking long to reply. As I mentioned in the PR, I was out for the djangocon conference and got back this week.

Let me know if you need any help with the PR, either by commenting here or there, or even pinging me on discord :)