Skip to content
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Src/Notion.Client/Api/ApiEndpoints.cs
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ public static class AuthenticationUrls
public static class FileUploadsApiUrls
{
public static string Create() => "/v1/file_uploads";
public static string Send(string fileUploadId) => $"/v1/file_uploads/{fileUploadId}/send";
}
}
}
14 changes: 14 additions & 0 deletions Src/Notion.Client/Api/FileUploads/IFileUploadsClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,19 @@ Task<CreateFileUploadResponse> CreateAsync(
CreateFileUploadRequest fileUploadObjectRequest,
CancellationToken cancellationToken = default
);

/// <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(
Comment on lines +19 to +28
Copy link

Copilot AI Oct 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.
SendFileUploadRequest sendFileUploadRequest,
CancellationToken cancellationToken = default
);
}
}
40 changes: 40 additions & 0 deletions Src/Notion.Client/Api/FileUploads/Send/FileUploadsClient.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
using System;
using System.Threading;
using System.Threading.Tasks;

namespace Notion.Client
{
public sealed partial class FileUploadsClient
{
public async Task<SendFileUploadResponse> SendAsync(
SendFileUploadRequest sendFileUploadRequest,
CancellationToken cancellationToken = default)
{
if (sendFileUploadRequest == null)
{
throw new ArgumentNullException(nameof(sendFileUploadRequest));
}

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

Copilot AI Oct 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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 uses AI. Check for mistakes.

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.");
}
}
Comment on lines 13 to 29
Copy link

Copilot AI Oct 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.

Copy link

Copilot AI Oct 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.
var path = ApiEndpoints.FileUploadsApiUrls.Send(sendFileUploadRequest.FileUploadId);

return await _restClient.PostAsync<SendFileUploadResponse>(
path,
formData: sendFileUploadRequest,
cancellationToken: cancellationToken
);
}
}
}
22 changes: 22 additions & 0 deletions Src/Notion.Client/Api/FileUploads/Send/Request/FileData.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
using System.IO;

namespace Notion.Client
{
public class FileData
{
/// <summary>
/// The name of the file being uploaded.
/// </summary>
public string FileName { get; set; }

/// <summary>
/// The content of the file being uploaded.
/// </summary>
public Stream Data { get; set; }

/// <summary>
/// The MIME type of the file being uploaded.
/// </summary>
public string ContentType { get; set; }
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
using Newtonsoft.Json;

Comment on lines +1 to +2
Copy link

Copilot AI Oct 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.
namespace Notion.Client
{
public interface ISendFileUploadFormDataParameters
{
/// <summary>
/// The raw binary file contents to upload.
/// </summary>
FileData File { get; }

/// <summary>
/// When using a mode=multi_part File Upload to send files greater than 20 MB in parts, this is the current part number.
/// Must be an integer between 1 and 1000 provided as a string form field.
/// </summary>
[JsonProperty("part_number")]
string PartNumber { get; }
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
namespace Notion.Client
{
public interface ISendFileUploadPathParameters
{
/// <summary>
/// The `file_upload_id` obtained from the `id` of the Create File Upload API response.
/// </summary>
string FileUploadId { get; }
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
namespace Notion.Client
{
public class SendFileUploadRequest : ISendFileUploadFormDataParameters, ISendFileUploadPathParameters
{
public FileData File { get; private set; }
public string PartNumber { get; private set; }
public string FileUploadId { get; private set; }

private SendFileUploadRequest() { }

public static SendFileUploadRequest Create(string fileUploadId, FileData file, string partNumber = null)
{
return new SendFileUploadRequest
{
FileUploadId = fileUploadId,
File = file,
PartNumber = partNumber
};
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
namespace Notion.Client
{
public class SendFileUploadResponse : FileObjectResponse
{
}
}
9 changes: 9 additions & 0 deletions Src/Notion.Client/RestClient/IRestClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,15 @@ Task<T> PostAsync<T>(
IBasicAuthenticationParameters basicAuthenticationParameters = null,
CancellationToken cancellationToken = default);

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);
Comment on lines +26 to +33
Copy link

Copilot AI Oct 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
Comment on lines +26 to +33
Copy link

Copilot AI Oct 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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 uses AI. Check for mistakes.

Task<T> PatchAsync<T>(
string uri,
object body,
Expand Down
40 changes: 40 additions & 0 deletions Src/Notion.Client/RestClient/RestClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,46 @@ void AttachContent(HttpRequestMessage httpRequest)
return await response.ParseStreamAsync<T>(serializerSettings);
}

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)
Comment on lines +73 to +80
Copy link

Copilot AI Oct 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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 uses AI. Check for mistakes.
{
void AttachContent(HttpRequestMessage httpRequest)
{
var fileContent = new StreamContent(formData.File.Data);
fileContent.Headers.ContentType = new MediaTypeHeaderValue(formData.File.ContentType);

var form = new MultipartFormDataContent
{
{ fileContent, "file", formData.File.FileName }
};

if (!string.IsNullOrEmpty(formData.PartNumber))
{
form.Add(new StringContent(formData.PartNumber), "part_number");
}

httpRequest.Content = form;
}

var response = await SendAsync(
uri,
HttpMethod.Post,
queryParams,
headers,
AttachContent,
basicAuthenticationParameters,
cancellationToken
);

return await response.ParseStreamAsync<T>(serializerSettings);
}

public async Task<T> PatchAsync<T>(
string uri,
object body,
Expand Down
32 changes: 32 additions & 0 deletions Test/Notion.IntegrationTests/FIleUploadsClientTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
// Arrange
var createRequest = new CreateFileUploadRequest
{
Mode = FileUploadMode.SinglePart,
FileName = "notion-logo.png",
};

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

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);

Copy link

Copilot AI Oct 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.
// Assert
Assert.NotNull(sendResponse);
Assert.Equal(createResponse.Id, sendResponse.Id);
Assert.Equal("notion-logo.png", sendResponse.FileName);
Assert.Equal("uploaded", sendResponse.Status);
}
}
}
6 changes: 6 additions & 0 deletions Test/Notion.IntegrationTests/Notion.IntegrationTests.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@
<ProjectReference Include="..\..\Src\Notion.Client\Notion.Client.csproj" />
</ItemGroup>

<ItemGroup>
<Content Include="assets\notion-logo.png">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</Content>
</ItemGroup>

<ItemGroup>
<PackageReference Include="FluentAssertions" Version="5.10.3" />
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.9.4" />
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
113 changes: 112 additions & 1 deletion Test/Notion.UnitTests/FileUploadClientTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -67,4 +67,115 @@ public async Task CreateAsync_CallsRestClientPostAsync_WithCorrectParameters()

_restClientMock.VerifyAll();
}
}

[Fact]
public async Task SendAsync_ThrowsArgumentNullException_WhenRequestIsNull()
{
// Act & Assert
var exception = await Assert.ThrowsAsync<ArgumentNullException>(() => _fileUploadClient.SendAsync(null));
Assert.Equal("sendFileUploadRequest", exception.ParamName);
Assert.Equal("Value cannot be null. (Parameter 'sendFileUploadRequest')", exception.Message);
}

[Fact]
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" });

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

[Theory]
[InlineData("0")]
[InlineData("1001")]
[InlineData("-5")]
[InlineData("abc")]
public async Task SendAsync_ThrowsArgumentOutOfRangeException_WhenPartNumberIsInvalid(string partNumber)
{
// Arrange
var request = SendFileUploadRequest.Create(fileUploadId: "valid-id", file: new FileData { FileName = "testfile.txt", Data = new System.IO.MemoryStream(), ContentType = "text/plain" }, partNumber: partNumber);

// Act & Assert
var exception = await Assert.ThrowsAsync<ArgumentOutOfRangeException>(() => _fileUploadClient.SendAsync(request));
Assert.Equal("PartNumber", exception.ParamName);
Assert.Contains("PartNumber must be between 1 and 1000.", exception.Message);
}

[Theory]
[InlineData("1")]
[InlineData("500")]
[InlineData("1000")]
public async Task SendAsync_DoesNotThrow_WhenPartNumberIsValid(string partNumber)
{
// Arrange
var request = SendFileUploadRequest.Create(fileUploadId: "valid-id", file: new FileData { FileName = "testfile.txt", Data = new System.IO.MemoryStream(), ContentType = "text/plain" }, partNumber: partNumber);

var expectedResponse = new SendFileUploadResponse
{
Id = "valid-id",
Status = "uploaded",
};

_restClientMock
.Setup(client => client.PostAsync<SendFileUploadResponse>(
It.Is<string>(url => url == ApiEndpoints.FileUploadsApiUrls.Send("valid-id")),
It.IsAny<ISendFileUploadFormDataParameters>(),
It.IsAny<IEnumerable<KeyValuePair<string, string>>>(),
It.IsAny<IDictionary<string, string>>(),
It.IsAny<JsonSerializerSettings>(),
It.IsAny<IBasicAuthenticationParameters>(),
It.IsAny<CancellationToken>()))
.ReturnsAsync(expectedResponse);

// Act
var exception = await Record.ExceptionAsync(() => _fileUploadClient.SendAsync(request));

// Assert
Assert.Null(exception);
_restClientMock.VerifyAll();
}

[Fact]
public async Task SendAsync_CallsRestClientPostAsync_WithCorrectParameters()
{
// Arrange
var fileUploadId = Guid.NewGuid().ToString();
var request = SendFileUploadRequest.Create(
fileUploadId: fileUploadId,
file: new FileData
{
FileName = "testfile.txt",
Data = new System.IO.MemoryStream(),
ContentType = "text/plain"
}
);

var expectedResponse = new SendFileUploadResponse
{
Id = fileUploadId.ToString(),
Status = "uploaded",
};

_restClientMock
.Setup(client => client.PostAsync<SendFileUploadResponse>(
It.Is<string>(url => url == ApiEndpoints.FileUploadsApiUrls.Send(fileUploadId)),
It.IsAny<ISendFileUploadFormDataParameters>(),
It.IsAny<IEnumerable<KeyValuePair<string, string>>>(),
It.IsAny<IDictionary<string, string>>(),
It.IsAny<JsonSerializerSettings>(),
It.IsAny<IBasicAuthenticationParameters>(),
It.IsAny<CancellationToken>()))
.ReturnsAsync(expectedResponse);
// Act
var response = await _fileUploadClient.SendAsync(request);

// Assert
Assert.Equal(expectedResponse.Status, response.Status);
Assert.Equal(expectedResponse.Id, response.Id);
_restClientMock.VerifyAll();
}
}
Loading