strawberry-graphql / strawberry-django

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

fix: Make sure that async fields always return Awaitables #646

Closed bellini666 closed 1 month ago

bellini666 commented 1 month ago

Async fields sometimes can return non-awaitable values, like when the optimizer is enabled and already prefetched the nested field.

This can break some places that expect an awaitable, like strawberry's base field permissions.

This change ensures that async fields always return an awaitable.

Fix #639

Summary by Sourcery

Fix async fields to consistently return awaitables, addressing issues with components expecting awaitable results, and add tests to validate this behavior.

Bug Fixes:

Tests:

sourcery-ai[bot] commented 1 month ago

Reviewer's Guide by Sourcery

This pull request fixes an issue where async fields could return non-awaitable values, potentially breaking functionality that expects awaitables. The change ensures that async fields always return an awaitable, addressing issue #639.

Class diagram for AsyncPermission and Query classes

classDiagram
    class AsyncPermission {
        +async has_permission(source: Any, info: Info, **kwargs: Any) Union[bool, Awaitable[bool]]
    }
    class Query {
        +list[Color] colors
    }
    class Color {
        +strawberry.auto name
        +list[Fruit] fruits
    }
    class Fruit {
        +strawberry.auto name
    }
    Query --> Color : has
    Color --> Fruit : has
    Color --> AsyncPermission : uses
    Fruit --> AsyncPermission : uses

File-Level Changes

Change Details Files
Modify the get_result function to always return an awaitable for async fields
  • Change the condition to check for both is_awaitable and self.is_async
  • Use await_maybe function to handle both awaitable and non-awaitable results
strawberry_django/fields/field.py
Add comprehensive test cases for field permissions with async and sync scenarios
  • Create test cases for async permissions
  • Create test cases for sync permissions
  • Add tests with and without the DjangoOptimizerExtension
  • Test both async and sync schema execution
tests/test_field_permissions.py

Assessment against linked issues

Issue Objective Addressed Explanation
#639 Fix the 'object list can't be used in 'await' expression' error when using async has_permission with optimizer's enable_prefetch_related_optimization set to True
#639 Ensure async fields always return Awaitables

Possibly linked issues


Tips and commands #### Interacting with Sourcery - **Trigger a new review:** Comment `@sourcery-ai review` on the pull request. - **Continue discussions:** Reply directly to Sourcery's review comments. - **Generate a GitHub issue from a review comment:** Ask Sourcery to create an issue from a review comment by replying to it. - **Generate a pull request title:** Write `@sourcery-ai` anywhere in the pull request title to generate a title at any time. - **Generate a pull request summary:** Write `@sourcery-ai summary` anywhere in the pull request body to generate a PR summary at any time. You can also use this command to specify where the summary should be inserted. #### Customizing Your Experience Access your [dashboard](https://app.sourcery.ai) to: - Enable or disable review features such as the Sourcery-generated pull request summary, the reviewer's guide, and others. - Change the review language. - Add, remove or edit custom review instructions. - Adjust other review settings. #### Getting Help - [Contact our support team](mailto:support@sourcery.ai) for questions or feedback. - Visit our [documentation](https://docs.sourcery.ai) for detailed guides and information. - Keep in touch with the Sourcery team by following us on [X/Twitter](https://x.com/SourceryAI), [LinkedIn](https://www.linkedin.com/company/sourcery-ai/) or [GitHub](https://github.com/sourcery-ai).
codecov-commenter commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 88.77%. Comparing base (92c1221) to head (ab3bbe3).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #646 +/- ## ========================================== + Coverage 88.74% 88.77% +0.03% ========================================== Files 41 41 Lines 3606 3607 +1 ========================================== + Hits 3200 3202 +2 + Misses 406 405 -1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.