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.

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.

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

RELEASE.md

codecov · 2025-02-09T14:12:00Z

Codecov Report

Attention: Patch coverage is 85.45455% with 8 lines in your changes missing coverage. Please review.

Project coverage is 95.04%. Comparing base (ca04c88) to head (dcde0cb).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3751      +/-   ##
==========================================
- Coverage   95.05%   95.04%   -0.02%     
==========================================
  Files         501      502       +1     
  Lines       32632    32682      +50     
  Branches     1694     1695       +1     
==========================================
+ Hits        31019    31061      +42     
- Misses       1341     1348       +7     
- Partials      272      273       +1

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

codspeed-hq · 2025-02-09T14:21:11Z

CodSpeed Performance Report

Merging #3751 will not alter performance

_{Comparing Maypher:sanic-file-fix (dcde0cb) with main (ca04c88)}

Summary

✅ 21 untouched benchmarks

bellini666

Hey @Maypher ,

I was revisiting this, just marking as "changes requested" to make sure I remember to check it again for any changes.

I think the only thing missing is to adjust the RELEASE.md file

Maypher · 2025-02-18T18:22:30Z

@bellini666 What exactly is there to change about RELEASE.md?

bellini666 · 2025-04-04T16:09:46Z

@bellini666 What exactly is there to change about RELEASE.md?

Hey @Maypher

Sorry for taking long to see this. I meant this comment here: https://github.com/strawberry-graphql/strawberry/pull/3751/files#r1912412757

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

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

patrick91

Thanks you for this! I think it is fine to keep the release file as it is 😊

botberry · 2025-04-15T22:34:59Z

Thanks for contributing to Strawberry! 🎉 You've been invited to join
the Strawberry GraphQL organisation 😊

You can also request a free sticker by filling this form: https://forms.gle/dmnfQUPoY5gZbVT67

And don't forget to join our discord server: https://strawberry.rocks/discord 🔥

sourcery-ai bot reviewed Jan 10, 2025

View reviewed changes

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

patrick91 reviewed Jan 10, 2025

View reviewed changes

bellini666 approved these changes Jan 12, 2025

View reviewed changes

RELEASE.md Show resolved Hide resolved

botberry removed the bot:release-type-patch label Feb 9, 2025

bellini666 requested changes Feb 12, 2025

View reviewed changes

This comment was marked as duplicate.

Sign in to view

botberry removed the bot:has-release-file label Mar 21, 2025

Maypher and others added 4 commits April 15, 2025 17:28

The sanic utility function convert_request_to_files_dict casts files to

266f39e

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

9897f67

Add release.md

d925477

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

698f5e2

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

patrick91 force-pushed the sanic-file-fix branch from 08d64bd to 698f5e2 Compare April 15, 2025 16:28

patrick91 approved these changes Apr 15, 2025

View reviewed changes

bellini666 approved these changes Apr 15, 2025

View reviewed changes

botberry added bot:has-release-file bot:release-type-patch labels Apr 15, 2025

patrick91 force-pushed the sanic-file-fix branch 2 times, most recently from 1ad46d7 to 93f37ce Compare April 15, 2025 16:43

Fix tests

c07c3b2

patrick91 force-pushed the sanic-file-fix branch from 93f37ce to c07c3b2 Compare April 15, 2025 16:44

Merge branch 'main' into sanic-file-fix

b101484

patrick91 added 2 commits April 15, 2025 22:18

Lint fix

2d026f7

Update tests

dcde0cb

patrick91 merged commit a7ba489 into strawberry-graphql:main Apr 15, 2025
100 of 101 checks passed

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 •

edited

Loading

botberry commented Jan 10, 2025

patrick91 left a comment

bellini666 left a comment

codecov bot commented Feb 9, 2025 •

edited

Loading

codspeed-hq bot commented Feb 9, 2025 •

edited

Loading

bellini666 left a comment

This comment was marked as duplicate.

Maypher commented Feb 18, 2025 •

edited

Loading

bellini666 commented Apr 4, 2025

patrick91 left a comment

botberry commented Apr 15, 2025

Fix sanic adapter casting files to io.BytesIO #3751

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 • edited Loading

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

codecov bot commented Feb 9, 2025 • edited Loading

Codecov Report

codspeed-hq bot commented Feb 9, 2025 • edited Loading

CodSpeed Performance Report

Merging #3751 will not alter performance

Summary

bellini666 left a comment

Choose a reason for hiding this comment

This comment was marked as duplicate.

Maypher commented Feb 18, 2025 • edited Loading

bellini666 commented Apr 4, 2025

patrick91 left a comment

Choose a reason for hiding this comment

botberry commented Apr 15, 2025

Maypher commented Jan 10, 2025 •

edited by bellini666

Loading

sourcery-ai bot commented Jan 10, 2025 •

edited

Loading

botberry commented Jan 10, 2025 •

edited

Loading

codecov bot commented Feb 9, 2025 •

edited

Loading

codspeed-hq bot commented Feb 9, 2025 •

edited

Loading

Maypher commented Feb 18, 2025 •

edited

Loading