-
Notifications
You must be signed in to change notification settings - Fork 57
Add support for send file upload API #488
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
e090418
3ed25e8
5f36bbf
c58e2ab
ea1d551
e48333f
eed81b4
8254b8b
70fbb3e
204478f
33ea735
8766e0e
95a2f93
66fb939
fd6c4fb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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.IsNullOrWhiteSpace(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."); | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||||||||||||||||||||
var path = ApiEndpoints.FileUploadsApiUrls.Send(sendFileUploadRequest.FileUploadId); | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
return await _restClient.PostAsync<SendFileUploadResponse>( | ||||||||||||||||||||||||
path, | ||||||||||||||||||||||||
formData: sendFileUploadRequest, | ||||||||||||||||||||||||
cancellationToken: cancellationToken | ||||||||||||||||||||||||
); | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
} |
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,18 @@ | ||||
using Newtonsoft.Json; | ||||
|
||||
Comment on lines
+1
to
+2
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||
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> | ||||
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 | ||
{ | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. Positive FeedbackNegative Feedback
Comment on lines
+26
to
+33
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. Positive FeedbackNegative Feedback |
||
|
||
Task<T> PatchAsync<T>( | ||
string uri, | ||
object body, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. Positive FeedbackNegative Feedback |
||
{ | ||
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, | ||
|
This file was deleted.
Original file line number | Diff line number | Diff line change | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,65 @@ | ||||||||||||||||||
using System.IO; | ||||||||||||||||||
using System.Threading.Tasks; | ||||||||||||||||||
using Notion.Client; | ||||||||||||||||||
using Xunit; | ||||||||||||||||||
|
||||||||||||||||||
namespace Notion.IntegrationTests | ||||||||||||||||||
{ | ||||||||||||||||||
public class FileUploadsClientTests : IntegrationTestBase | ||||||||||||||||||
{ | ||||||||||||||||||
[Fact] | ||||||||||||||||||
public async Task CreateAsync() | ||||||||||||||||||
{ | ||||||||||||||||||
// Arrange | ||||||||||||||||||
var request = new CreateFileUploadRequest | ||||||||||||||||||
{ | ||||||||||||||||||
Mode = FileUploadMode.ExternalUrl, | ||||||||||||||||||
ExternalUrl = "https://unsplash.com/photos/hOhlYhAiizc/download?ixid=M3wxMjA3fDB8MXxhbGx8fHx8fHx8fHwxNzYwMTkxNzc3fA&force=true", | ||||||||||||||||||
FileName = "sample-image.jpg", | ||||||||||||||||||
}; | ||||||||||||||||||
|
||||||||||||||||||
// Act | ||||||||||||||||||
var response = await Client.FileUploads.CreateAsync(request); | ||||||||||||||||||
|
||||||||||||||||||
// Assert | ||||||||||||||||||
Assert.NotNull(response); | ||||||||||||||||||
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); | ||||||||||||||||||
|
||||||||||||||||||
using (var fileStream = File.OpenRead("assets/notion-logo.png")) | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [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
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||||||||||||||
{ | ||||||||||||||||||
var sendRequest = SendFileUploadRequest.Create( | ||||||||||||||||||
createResponse.Id, | ||||||||||||||||||
new FileData | ||||||||||||||||||
{ | ||||||||||||||||||
FileName = "notion-logo.png", | ||||||||||||||||||
Data = fileStream, | ||||||||||||||||||
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); | ||||||||||||||||||
} | ||||||||||||||||||
} | ||||||||||||||||||
} | ||||||||||||||||||
} |
This file was deleted.
There was a problem hiding this comment.
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.