Fix sanic adapter casting files to io.BytesIO #3751

Maypher · 2025-01-10T20:23:38Z

The current implementation of strawberry.sanic.utils.convert_request_to_files_dict casts uploaded files to io.BytesIO, making it return a type that doesn't coincide with the docs. This commit removes this casting and makes the function work in accordance with the docs.

Description

Changed files_dict[field_name] = BytesIO(file_list[0].body) to files_dict[field_name] = file_list[0]

Types of Changes

Issues Fixed or Closed by This PR

Fix #3750

Checklist

My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have read the CONTRIBUTING document.
I have added tests to cover my changes.
I have tested the changes and verified that they work and don't break anything (as well as I can manage).

Summary by Sourcery

Bug Fixes:

Uploaded files are now correctly handled as Sanic File objects instead of being cast to io.BytesIO.

io.BytesIO which is the filetype used by AIOHTTP. This commit fixes this issue by directly assigning the file instead of casting it.

sourcery-ai · 2025-01-10T20:23:44Z

Reviewer's Guide by Sourcery

This PR fixes a bug in the strawberry.sanic.utils.convert_request_to_files_dict function where uploaded files were incorrectly cast to io.BytesIO. The change ensures the function returns the correct sanic.request.File type, aligning it with the documentation.

Sequence diagram for Sanic file upload handling before and after fix

sequenceDiagram
    participant Client
    participant SanicServer
    participant FileHandler

    Note over Client,FileHandler: Before Fix
    Client->>SanicServer: Upload file
    SanicServer->>FileHandler: convert_request_to_files_dict()
    FileHandler->>FileHandler: Convert to BytesIO
    FileHandler-->>SanicServer: Return BytesIO object

    Note over Client,FileHandler: After Fix
    Client->>SanicServer: Upload file
    SanicServer->>FileHandler: convert_request_to_files_dict()
    FileHandler-->>SanicServer: Return original Sanic File object

Class diagram showing the file handling type changes

classDiagram
    class Request {
        +files: dict
    }

    class FileHandler {
        +convert_request_to_files_dict(request: Request) dict
    }

    class File {
        +body: bytes
        +name: str
        +type: str
    }

    note for FileHandler "Changed return type from\ndict[str, BytesIO] to\ndict[str, File]"

    Request -- FileHandler
    FileHandler -- File

File-Level Changes

Change	Details	Files
Removed the casting of uploaded files to `io.BytesIO`.	Changed `files_dict[field_name] = BytesIO(file_list[0].body)` to `files_dict[field_name] = file_list[0]`	`strawberry/sanic/utils.py`
Added tests to verify the fix and ensure the function behaves as expected.	Created a new test file `test_file_upload.py` with tests covering the file upload functionality. Added a test case to check that files are correctly converted to `sanic.request.File` objects. Added a test case to verify that the GraphQL API handles file uploads correctly end-to-end. Added a test case to verify that the file name is correctly returned after the upload process is completed	`tests/sanic/test_file_upload.py`
Updated the documentation to reflect the change.	Added a new release note in `RELEASE.md` to document the bug fix and the change in the returned file type	`RELEASE.md`

Possibly linked issues

#0: The PR fixes the bug where uploaded files were cast to io.BytesIO instead of sanic.request.File.

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 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 for questions or feedback.
Visit our documentation for detailed guides and information.
Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

for more information, see https://pre-commit.ci

sourcery-ai

Hey @Maypher - I've reviewed your changes and they look great!

Here's what I looked at during the review

🟡 General issues: 2 issues found
🟢 Security: all looks good
🟢 Testing: all looks good
🟢 Complexity: all looks good
🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

sourcery-ai · 2025-01-10T20:24:37Z

strawberry/sanic/utils.py

@@ -29,12 +28,12 @@ def convert_request_to_files_dict(request: Request) -> dict[str, Any]:
    if not request_files:
        return {}

-    files_dict: dict[str, Union[BytesIO, list[BytesIO]]] = {}
+    files_dict: dict[str, Union[File, list[File]]] = {}

    for field_name, file_list in request_files.items():
        assert len(file_list) == 1


issue: Consider handling multiple files per field name instead of asserting there will be exactly one

The assertion could cause server crashes if multiple files are uploaded with the same field name. Consider returning a list of File objects when multiple files are present, matching the type hint Union[File, list[File]].

sourcery-ai · 2025-01-10T20:24:37Z

RELEASE.md

+Release type: patch
+
+Fix bug where files would be converted into io.BytesIO when using the sanic GraphQLView
+instead of using the sanic File type


suggestion: Check casing and specific name for "sanic File type"

Should it be "Sanic File", "Sanic file type", or something more specific? Referencing the Sanic documentation might help.

botberry · 2025-01-10T20:24:55Z

Thanks for adding the RELEASE.md file!

Here's a preview of the changelog:

Fix bug where files would be converted into io.BytesIO when using the sanic GraphQLView
instead of using the sanic File type

Here's the tweet text:

🆕 Release (next) is out! Thanks to Maypher for the PR 👏

Get it here 👉 https://strawberry.rocks/release/(next)

botberry · 2025-01-10T20:25:03Z

Thanks for adding the RELEASE.md file!

Here's a preview of the changelog:

Fix bug where files would be converted into io.BytesIO when using the sanic GraphQLView
instead of using the sanic File type

Here's the tweet text:

🆕 Release (next) is out! Thanks to Maypher for the PR 👏

Get it here 👉 https://strawberry.rocks/release/(next)

patrick91

This looks great, I might spend a bit of time moving the tests to the http tests, so we have one test for all integrations 😊

bellini666

LGTM :)

Will leave final review to @patrick91

bellini666 · 2025-01-12T10:02:27Z

RELEASE.md

+Release type: patch
+
+Fix bug where files would be converted into io.BytesIO when using the sanic GraphQLView
+instead of using the sanic File type


thought: although a bug, someone using it might have gotten already used to it being BytesIO, so maybe we should mark this as a minor and mention that anyone relying on old behavior should adjust their code?

What do you think @patrick91 ?

Maypher added 3 commits January 10, 2025 13:44

The sanic utility function convert_request_to_files_dict casts files to

daadec3

io.BytesIO which is the filetype used by AIOHTTP. This commit fixes this issue by directly assigning the file instead of casting it.

Add tests

995d72b

Add release.md

d3b065e

[pre-commit.ci] auto fixes from pre-commit.com hooks

08d64bd

for more information, see https://pre-commit.ci

sourcery-ai bot reviewed Jan 10, 2025

View reviewed changes

botberry added bot:has-release-file bot:release-type-patch labels Jan 10, 2025

patrick91 reviewed Jan 10, 2025

View reviewed changes

bellini666 approved these changes Jan 12, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix sanic adapter casting files to io.BytesIO #3751

Fix sanic adapter casting files to io.BytesIO #3751

Maypher commented Jan 10, 2025 •

edited by bellini666

Loading

sourcery-ai bot commented Jan 10, 2025 •

edited

Loading

Interacting with Sourcery

Customizing Your Experience

Getting Help

sourcery-ai bot left a comment

sourcery-ai bot Jan 10, 2025

sourcery-ai bot Jan 10, 2025

botberry commented Jan 10, 2025

botberry commented Jan 10, 2025

patrick91 left a comment

bellini666 left a comment

bellini666 Jan 12, 2025

Fix sanic adapter casting files to io.BytesIO #3751

Are you sure you want to change the base?

Fix sanic adapter casting files to io.BytesIO #3751

Conversation

Maypher commented Jan 10, 2025 • edited by bellini666 Loading

Description

Types of Changes

Issues Fixed or Closed by This PR

Checklist

Summary by Sourcery

sourcery-ai bot commented Jan 10, 2025 • edited Loading

Reviewer's Guide by Sourcery

Sequence diagram for Sanic file upload handling before and after fix

Class diagram showing the file handling type changes

File-Level Changes

Possibly linked issues

Interacting with Sourcery

Customizing Your Experience

Getting Help

sourcery-ai bot left a comment

Choose a reason for hiding this comment

sourcery-ai bot Jan 10, 2025

Choose a reason for hiding this comment

sourcery-ai bot Jan 10, 2025

Choose a reason for hiding this comment

botberry commented Jan 10, 2025

botberry commented Jan 10, 2025

patrick91 left a comment

Choose a reason for hiding this comment

bellini666 left a comment

Choose a reason for hiding this comment

bellini666 Jan 12, 2025

Choose a reason for hiding this comment

Maypher commented Jan 10, 2025 •

edited by bellini666

Loading

sourcery-ai bot commented Jan 10, 2025 •

edited

Loading