strawberry-graphql / strawberry-django

Strawberry GraphQL Django extension
https://strawberry.rocks/docs/django
MIT License
402 stars 116 forks source link

Incompatible with django-fsm and it's ConcurrentTransitionMixin #578

Open pfcodes opened 1 month ago

pfcodes commented 1 month ago

Strawberry GraphQL has weird behavior when DjangoOptimizerExtension is enabled and its resolving objects based on django models that inherit ConcurrentTransitionMixin from the django-fsm package.

Resolving one type resulted in a RecursionError: maximum recursion depth exceeded error. Another type triggered a Direct state modification not allowed error.

From the ConcurrentTransitionMixin docs:

"""
Protects a Model from undesirable effects caused by concurrently executed transitions,
e.g. running the same transition multiple times at the same time, or running different
transitions with the same SOURCE state at the same time.

This behavior is achieved using an idea based on optimistic locking. No additional
version field is required though; only the state field(s) is/are used for the tracking.
This scheme is not that strict as true *optimistic locking* mechanism, it is however
more lightweight - leveraging the specifics of FSM models.

Instance of a model based on this Mixin will be prevented from saving into DB if any
of its state fields (instances of FSMFieldMixin) has been changed since the object
was fetched from the database. *ConcurrentTransition* exception will be raised in such
cases.

For guaranteed protection against such race conditions, make sure:
* Your transitions do not have any side effects except for changes in the database,
* You always run the save() method on the object within django.db.transaction.atomic()
block.

Following these recommendations, you can rely on ConcurrentTransitionMixin to cause
a rollback of all the changes that have been executed in an inconsistent (out of sync)
state, thus practically negating their effect.
"""

Graphene doesn't behave in this way. Would love to be able to migrate over to Strawberry without losing the benefits of the `ConcurrentTransitionMixin`

Upvote & Fund

Fund with Polar

bellini666 commented 1 month ago

Hi @pfcodes ,

Hrm, that's interesting... Wondering what might be causing it.

Can you share more details about that, like the traceback you get?