Add support for send file upload API #488

KoditkarVedant · 2025-10-11T16:14:36Z

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Fixes #478

Type of change

Please delete options that are not relevant.

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

Checklist:

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes
Any dependent changes have been merged and published in downstream modules
I have checked my code and corrected any misspellings

…upload-endpoint

Copilot

Pull Request Overview

Adds support for sending (uploading) a created file upload to the Notion API, including client, request/response models, REST client multipart handling, and accompanying unit/integration tests.

Introduces SendFileUploadRequest/Response models and FileUploadsClient.SendAsync with validation.
Adds multipart/form-data PostAsync overload to RestClient for streaming file data.
Extends tests (unit + integration) and project assets (logo file) for end‑to‑end upload flow.

Reviewed Changes

Copilot reviewed 13 out of 14 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
Test/Notion.UnitTests/FileUploadClientTests.cs	Adds unit tests for SendAsync argument validation and success path.
Test/Notion.IntegrationTests/Notion.IntegrationTests.csproj	Includes image asset needed for integration test file upload.
Test/Notion.IntegrationTests/FIleUploadsClientTests.cs	Adds integration test for full single-part upload flow (and introduces naming/disposal concerns).
Src/Notion.Client/RestClient/RestClient.cs	Adds multipart/form-data PostAsync overload to send file streams.
Src/Notion.Client/RestClient/IRestClient.cs	Declares new multipart PostAsync overload (missing XML docs).
Src/Notion.Client/Api/FileUploads/Send/Response/SendFileUploadResponse.cs	Adds response model inheriting existing FileObjectResponse.
Src/Notion.Client/Api/FileUploads/Send/Request/SendFileUploadRequest.cs	Adds request builder implementing form/path parameter interfaces.
Src/Notion.Client/Api/FileUploads/Send/Request/ISendFileUploadPathParameters.cs	Defines path parameter interface for file upload id.
Src/Notion.Client/Api/FileUploads/Send/Request/ISendFileUploadFormDataParameters.cs	Defines form data interface (includes possibly misleading JsonProperty attribute).
Src/Notion.Client/Api/FileUploads/Send/Request/FileData.cs	Defines file metadata + stream container.
Src/Notion.Client/Api/FileUploads/Send/FileUploadsClient.cs	Implements SendAsync with validation (missing file null checks, minor exception choice issue).
Src/Notion.Client/Api/FileUploads/IFileUploadsClient.cs	Adds SendAsync method to public interface with summary.
Src/Notion.Client/Api/ApiEndpoints.cs	Adds endpoint helper for send action.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-10-12T14:32:58Z

Test/Notion.UnitTests/FileUploadsClientTests.cs

+        var exception = await Assert.ThrowsAsync<ArgumentNullException>(() => _fileUploadClient.SendAsync(null));
+        Assert.Equal("sendFileUploadRequest", exception.ParamName);
+        Assert.Equal("Value cannot be null. (Parameter 'sendFileUploadRequest')", exception.Message);


[nitpick] Asserting the full framework-generated exception.Message is brittle (it can vary by runtime or localization); asserting ParamName plus exception type is sufficient—remove the line 77 exact message assertion.

Suggested change

Assert.Equal("Value cannot be null. (Parameter 'sendFileUploadRequest')", exception.Message);

Copilot · 2025-10-12T14:32:59Z

Test/Notion.UnitTests/FileUploadsClientTests.cs

+        var exception = await Assert.ThrowsAsync<ArgumentNullException>(() => _fileUploadClient.SendAsync(request));
+        Assert.Equal("FileUploadId", exception.ParamName);
+        Assert.Equal("Value cannot be null. (Parameter 'FileUploadId')", exception.Message);


[nitpick] Same brittleness: remove or relax the exact exception.Message assertion on line 89; keep validating exception type and ParamName.

Suggested change

Assert.Equal("Value cannot be null. (Parameter 'FileUploadId')", exception.Message);

Copilot · 2025-10-12T14:32:59Z

Src/Notion.Client/Api/FileUploads/Send/FileUploadsClient.cs

+            if (sendFileUploadRequest == null)
+            {
+                throw new ArgumentNullException(nameof(sendFileUploadRequest));
+            }
+
+            if (string.IsNullOrEmpty(sendFileUploadRequest.FileUploadId))
+            {
+                throw new ArgumentNullException(nameof(sendFileUploadRequest.FileUploadId));
+            }
+
+            if (sendFileUploadRequest.PartNumber != null)
+            {
+                if (!int.TryParse(sendFileUploadRequest.PartNumber, out int partNumberValue) || partNumberValue < 1 || partNumberValue > 1000)
+                {
+                    throw new ArgumentOutOfRangeException(nameof(sendFileUploadRequest.PartNumber), "PartNumber must be between 1 and 1000.");
+                }
+            }


Missing validation for sendFileUploadRequest.File and sendFileUploadRequest.File.Data can lead to a NullReferenceException later in RestClient when creating StreamContent; add explicit ArgumentNullException checks for File and File.Data before using them.

Copilot · 2025-10-12T14:32:59Z

Src/Notion.Client/Api/FileUploads/Send/FileUploadsClient.cs

+            if (string.IsNullOrEmpty(sendFileUploadRequest.FileUploadId))
+            {
+                throw new ArgumentNullException(nameof(sendFileUploadRequest.FileUploadId));
+            }


[nitpick] Using ArgumentNullException for an empty string is misleading; consider distinguishing null vs empty and throw ArgumentException for empty (or use string.IsNullOrWhiteSpace and ArgumentException) to better reflect the invalid value scenario.

Suggested change

if (string.IsNullOrEmpty(sendFileUploadRequest.FileUploadId))

{

throw new ArgumentNullException(nameof(sendFileUploadRequest.FileUploadId));

}

if (sendFileUploadRequest.FileUploadId == null)

{

throw new ArgumentNullException(nameof(sendFileUploadRequest.FileUploadId));

}

if (sendFileUploadRequest.FileUploadId == string.Empty)

{

throw new ArgumentException("FileUploadId cannot be empty.", nameof(sendFileUploadRequest.FileUploadId));

}

Copilot · 2025-10-12T14:32:59Z

Test/Notion.IntegrationTests/FIleUploadsClientTests.cs

+            var sendRequest = SendFileUploadRequest.Create(
+                createResponse.Id,
+                new FileData
+                {
+                    FileName = "notion-logo.png",
+                    Data = System.IO.File.OpenRead("assets/notion-logo.png"),
+                    ContentType = createResponse.ContentType
+                }
+            );
+
+            // Act
+            var sendResponse = await Client.FileUploads.SendAsync(sendRequest);
+


The opened file stream is never disposed, risking a file handle leak; wrap the stream in a using statement (using var stream = File.OpenRead(...)) and pass the stream variable.

Suggested change

var sendRequest = SendFileUploadRequest.Create(

createResponse.Id,

new FileData

{

FileName = "notion-logo.png",

Data = System.IO.File.OpenRead("assets/notion-logo.png"),

ContentType = createResponse.ContentType

}

);

// Act

var sendResponse = await Client.FileUploads.SendAsync(sendRequest);

using (var stream = System.IO.File.OpenRead("assets/notion-logo.png"))

{

var sendRequest = SendFileUploadRequest.Create(

createResponse.Id,

new FileData

{

FileName = "notion-logo.png",

Data = stream,

ContentType = createResponse.ContentType

}

);

// Act

var sendResponse = await Client.FileUploads.SendAsync(sendRequest);

// Assert

Assert.NotNull(sendResponse);

Assert.Equal(createResponse.Id, sendResponse.Id);

Assert.Equal("notion-logo.png", sendResponse.FileName);

Assert.Equal("uploaded", sendResponse.Status);

}

Copilot · 2025-10-12T14:33:00Z

Test/Notion.IntegrationTests/FileUploadsClientTests.cs

@@ -25,5 +25,37 @@ public async Task CreateAsync()
            Assert.NotNull(response.Status);
            Assert.Equal("sample-image.jpg", response.FileName);
        }
+
+        [Fact]
+        public async Task Verify_file_upload_flow()


Correct file name typo: current file is named FIleUploadsClientTests.cs (uppercase I after F); should be FileUploadsClientTests.cs to match standard casing.

Copilot · 2025-10-12T14:33:00Z

Src/Notion.Client/RestClient/IRestClient.cs

+        Task<T> PostAsync<T>(
+            string uri,
+            ISendFileUploadFormDataParameters formData,
+            IEnumerable<KeyValuePair<string, string>> queryParams = null,
+            IDictionary<string, string> headers = null,
+            JsonSerializerSettings serializerSettings = null,
+            IBasicAuthenticationParameters basicAuthenticationParameters = null,
+            CancellationToken cancellationToken = default);


New overload lacks XML documentation differentiating it from the existing PostAsync (body JSON) overload; add summary and parameter docs clarifying it sends multipart/form-data with a file stream.

Src/Notion.Client/Api/FileUploads/Send/Request/ISendFileUploadFormDataParameters.cs

…t as Json to notion Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull Request Overview

Copilot reviewed 15 out of 16 changed files in this pull request and generated 9 comments.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-10-12T15:04:50Z

Src/Notion.Client/Api/FileUploads/Send/FileUploadsClient.cs

+                    throw new ArgumentOutOfRangeException(nameof(sendFileUploadRequest.PartNumber), "PartNumber must be between 1 and 1000.");
+                }
+            }
+


The method validates FileUploadId and PartNumber but never validates sendFileUploadRequest.File (or its Data stream); a null File will cause a NullReferenceException later in RestClient.PostAsync when accessing formData.File.Data. Add a null check (and optionally ensure File.Data is not null) to throw a clear ArgumentNullException before invoking the REST call.

Suggested change

if (sendFileUploadRequest.File == null)

{

throw new ArgumentNullException(nameof(sendFileUploadRequest.File), "File must not be null.");

}

if (sendFileUploadRequest.File.Data == null)

{

throw new ArgumentNullException(nameof(sendFileUploadRequest.File.Data), "File.Data must not be null.");

}

Copilot · 2025-10-12T15:04:50Z

Src/Notion.Client/RestClient/RestClient.cs

+        public async Task<T> PostAsync<T>(
+            string uri,
+            ISendFileUploadFormDataParameters formData,
+            IEnumerable<KeyValuePair<string, string>> queryParams = null,
+            IDictionary<string, string> headers = null,
+            JsonSerializerSettings serializerSettings = null,
+            IBasicAuthenticationParameters basicAuthenticationParameters = null,
+            CancellationToken cancellationToken = default)


[nitpick] The new PostAsync overload differs only by the second parameter type from the existing PostAsync(string, object, ...); calls with a null second argument (e.g., _restClient.PostAsync(uri, null)) will be ambiguous for consumers. Consider renaming this overload (e.g., PostMultipartAsync) or introducing a distinct request wrapper type to avoid overload ambiguity.

Copilot · 2025-10-12T15:04:51Z

Src/Notion.Client/RestClient/IRestClient.cs

+        Task<T> PostAsync<T>(
+            string uri,
+            ISendFileUploadFormDataParameters formData,
+            IEnumerable<KeyValuePair<string, string>> queryParams = null,
+            IDictionary<string, string> headers = null,
+            JsonSerializerSettings serializerSettings = null,
+            IBasicAuthenticationParameters basicAuthenticationParameters = null,
+            CancellationToken cancellationToken = default);


[nitpick] Mirroring the implementation, this overload creates potential ambiguity with the existing PostAsync signature when null is provided for formData; a differently named method (e.g., PostMultipartAsync) would make the API clearer and prevent ambiguous invocation errors.

Copilot · 2025-10-12T15:04:51Z

Src/Notion.Client/Api/FileUploads/Send/Request/ISendFileUploadFormDataParameters.cs

+using Newtonsoft.Json;
+


Unused using directive (Newtonsoft.Json) in this file; remove it to reduce unnecessary dependencies and keep the interface lean.

Suggested change

using Newtonsoft.Json;

Copilot · 2025-10-12T15:04:51Z

Src/Notion.Client/Api/FileUploads/IFileUploadsClient.cs

+        /// <summary>
+        /// Send a file upload
+        /// 
+        /// Requires a `file_upload_id`, obtained from the `id` of the Create File Upload API response.
+        /// 
+        /// </summary>
+        /// <param name="sendFileUploadRequest"></param>
+        /// <param name="cancellationToken"></param>
+        /// <returns></returns>
+        Task<SendFileUploadResponse> SendAsync(


The summary omits important details (e.g., required File fields, behavior of optional PartNumber, constraints 1–1000, SinglePart vs MultiPart usage); expand the XML doc to describe expected request state and validation rules to aid consumers.

Copilot · 2025-10-12T15:04:52Z

Test/Notion.UnitTests/FileUploadsClientTests.cs

+        // Act & Assert
+        var exception = await Assert.ThrowsAsync<ArgumentNullException>(() => _fileUploadClient.CreateAsync(null));
+        Assert.Equal("fileUploadObjectRequest", exception.ParamName);
+        Assert.Equal("Value cannot be null. (Parameter 'fileUploadObjectRequest')", exception.Message);


Asserting the full exception.Message is brittle across framework/localization changes; prefer asserting ParamName only (already done) or using Assert.StartsWith / Contains for the custom portion rather than the entire message.

Suggested change

Assert.Equal("Value cannot be null. (Parameter 'fileUploadObjectRequest')", exception.Message);

Assert.StartsWith("Value cannot be null", exception.Message);

Copilot · 2025-10-12T15:04:52Z

Test/Notion.UnitTests/FileUploadsClientTests.cs

+        var exception = await Assert.ThrowsAsync<ArgumentNullException>(() => _fileUploadClient.SendAsync(null));
+        Assert.Equal("sendFileUploadRequest", exception.ParamName);
+        Assert.Equal("Value cannot be null. (Parameter 'sendFileUploadRequest')", exception.Message);


Full message equality makes the test fragile to runtime message format changes; restrict the assertion to ParamName (and optionally a Contains check) for resilience.

Copilot · 2025-10-12T15:04:52Z

Test/Notion.UnitTests/FileUploadsClientTests.cs

+    public async Task SendAsync_ThrowsArgumentNullException_WhenFileUploadIdIsNullOrEmpty()
+    {
+        // Arrange
+        var request = SendFileUploadRequest.Create(fileUploadId: null, file: new FileData { FileName = "testfile.txt", Data = new System.IO.MemoryStream(), ContentType = "text/plain" });


The MemoryStream instances created for FileData (also at lines 100, 115, 152) are never disposed; wrap them in using statements or use a using var pattern to explicitly dispose and signal intent, even in tests.

Copilot · 2025-10-12T15:04:52Z

Test/Notion.IntegrationTests/FileUploadsClientTests.cs

+
+            var createResponse = await Client.FileUploads.CreateAsync(createRequest);
+
+            using (var fileStream = File.OpenRead("assets/notion-logo.png"))


[nitpick] Consider specifying FileShare.Read (File.OpenRead defaults to FileShare.Read) or using File.Open with explicit modes for clarity; additionally, for large files asynchronous stream (FileOptions.Asynchronous) could improve throughput if awaited operations are extended.

Suggested change

using (var fileStream = File.OpenRead("assets/notion-logo.png"))

using (var fileStream = File.Open(

"assets/notion-logo.png",

FileMode.Open,

FileAccess.Read,

FileShare.Read,

4096,

FileOptions.Asynchronous))

KoditkarVedant added 9 commits October 11, 2025 21:41

Add support for send file upload api

e090418

Merge branch 'main' into 478-add-support-for-send-file-upload-endpoint

3ed25e8

Fix code factor warnings

5f36bbf

Fix code factor warnings

c58e2ab

Merge branch '477-add-unit-tests' into 478-add-support-for-send-file-…

ea1d551

…upload-endpoint

Adjust property name in integration test

e48333f

Add validation for part number if provided should be between 1 to 1000

eed81b4

Add unit tests for Send file upload api endpoint

8254b8b

Merge branch 'main' into 478-add-support-for-send-file-upload-endpoint

70fbb3e

KoditkarVedant requested a review from Copilot October 12, 2025 14:31

Copilot AI reviewed Oct 12, 2025

View reviewed changes

KoditkarVedant and others added 6 commits October 12, 2025 20:19

Remove JsonProperty attribute from PartNumber as the model is not sen…

204478f

…t as Json to notion Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Rename file to FileUploadsClientTests

33ea735

Rename test file to FileUploadsClientTests

8766e0e

Rename unit test file to FileUploadsClientTests

95a2f93

Dispose stream to avoid potential leak

66fb939

Use IsNullOrWhiteSpace to check file upload id is present

fd6c4fb

KoditkarVedant requested a review from Copilot October 12, 2025 15:02

Copilot AI reviewed Oct 12, 2025

View reviewed changes

+            if (sendFileUploadRequest.File == null)
+            {
+                throw new ArgumentNullException(nameof(sendFileUploadRequest.File), "File must not be null.");
+            }
+            if (sendFileUploadRequest.File.Data == null)
+            {
+                throw new ArgumentNullException(nameof(sendFileUploadRequest.File.Data), "File.Data must not be null.");
+            }

	Assert.Equal("Value cannot be null. (Parameter 'fileUploadObjectRequest')", exception.Message);
	Assert.StartsWith("Value cannot be null", exception.Message);


		var createResponse = await Client.FileUploads.CreateAsync(createRequest);

		using (var fileStream = File.OpenRead("assets/notion-logo.png"))

-            using (var fileStream = File.OpenRead("assets/notion-logo.png"))
+            using (var fileStream = File.Open(
+                "assets/notion-logo.png",
+                FileMode.Open,
+                FileAccess.Read,
+                FileShare.Read,
+,
+                FileOptions.Asynchronous))

Add support for send file upload API #488

Are you sure you want to change the base?

Add support for send file upload API #488

Conversation

KoditkarVedant commented Oct 11, 2025

Description

Type of change

Checklist:

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Oct 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 12, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Oct 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 12, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant