Skip to content
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

Remove CreateAttachment::id field #2771

Merged
merged 1 commit into from
Feb 17, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
60 changes: 41 additions & 19 deletions src/builder/create_attachment.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use std::borrow::Cow;
use std::path::Path;

use serde::ser::{Serialize, SerializeSeq, Serializer};
use tokio::fs::File;
use tokio::io::AsyncReadExt;

Expand All @@ -11,20 +12,17 @@ use crate::http::Http;
use crate::model::channel::Message;
use crate::model::id::AttachmentId;

/// Enum that allows a user to pass a [`Path`] or a [`File`] type to [`send_files`]
/// Struct that allows a user to pass a [`Path`] or a [`File`] type to [`send_files`]
///
/// [Discord docs](https://discord.com/developers/docs/resources/channel#attachment-object-attachment-structure).
///
/// [`send_files`]: crate::model::id::ChannelId::send_files
#[derive(Clone, Debug, Serialize)]
#[derive(Clone, Debug)]
#[non_exhaustive]
#[must_use]
pub struct CreateAttachment<'a> {
pub(crate) id: u64, // Placeholder ID will be filled in when sending the request
pub filename: Cow<'static, str>,
pub description: Option<Cow<'a, str>>,

#[serde(skip)]
pub data: Cow<'static, [u8]>,
}

Expand All @@ -38,7 +36,6 @@ impl<'a> CreateAttachment<'a> {
data: data.into(),
filename: filename.into(),
description: None,
id: 0,
}
}

Expand Down Expand Up @@ -116,13 +113,12 @@ impl<'a> CreateAttachment<'a> {
}
}

#[derive(Debug, Clone, serde::Serialize)]
#[derive(Clone, Debug, Serialize)]
struct ExistingAttachment {
id: AttachmentId,
}

#[derive(Debug, Clone, serde::Serialize)]
#[serde(untagged)]
#[derive(Clone, Debug)]
enum NewOrExisting<'a> {
New(CreateAttachment<'a>),
Existing(ExistingAttachment),
Expand Down Expand Up @@ -183,8 +179,7 @@ enum NewOrExisting<'a> {
///
/// Internally, this type is used not just for message editing endpoints, but also for message
/// creation endpoints.
#[derive(Default, Debug, Clone, serde::Serialize)]
#[serde(transparent)]
#[derive(Default, Debug, Clone)]
#[must_use]
pub struct EditAttachments<'a> {
new_and_existing_attachments: Vec<NewOrExisting<'a>>,
Expand Down Expand Up @@ -258,22 +253,15 @@ impl<'a> EditAttachments<'a> {
/// are needed for the multipart form data. The data is taken out of `self` in the process, so
/// this method can only be called once.
pub(crate) fn take_files(&mut self) -> Vec<CreateAttachment<'a>> {
let mut id_placeholder = 0;

let mut files = Vec::new();
for attachment in &mut self.new_and_existing_attachments {
if let NewOrExisting::New(attachment) = attachment {
let mut cloned_attachment = CreateAttachment::bytes(
let cloned_attachment = CreateAttachment::bytes(
std::mem::take(&mut attachment.data),
attachment.filename.clone(),
);

// Assign placeholder IDs so Discord can match metadata to file contents
attachment.id = id_placeholder;
cloned_attachment.id = id_placeholder;
files.push(cloned_attachment);

id_placeholder += 1;
}
}
files
Expand All @@ -284,3 +272,37 @@ impl<'a> EditAttachments<'a> {
self.new_and_existing_attachments.is_empty()
}
}

impl<'a> Serialize for EditAttachments<'a> {
fn serialize<S: Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
#[derive(Serialize)]
struct NewAttachment<'a> {
id: u64,
filename: &'a Cow<'static, str>,
description: &'a Option<Cow<'a, str>>,
}

// Instead of an `AttachmentId`, the `id` field for new attachments corresponds to the
// index of the new attachment in the multipart payload. The attachment data will be
// labeled with `files[{id}]` in the multipart body. See `Multipart::build_form`.
let mut id = 0;
let mut seq = serializer.serialize_seq(Some(self.new_and_existing_attachments.len()))?;
for attachment in &self.new_and_existing_attachments {
match attachment {
NewOrExisting::New(new_attachment) => {
let attachment = NewAttachment {
id,
filename: &new_attachment.filename,
description: &new_attachment.description,
};
id += 1;
seq.serialize_element(&attachment)?;
},
NewOrExisting::Existing(existing_attachment) => {
seq.serialize_element(existing_attachment)?;
},
}
}
GnomedDev marked this conversation as resolved.
Show resolved Hide resolved
seq.end()
}
}
8 changes: 4 additions & 4 deletions src/http/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,7 @@ impl Http {
self.fire(Request {
body: None,
multipart: Some(Multipart {
upload: MultipartUpload::Attachments(files.into_iter().collect()),
upload: MultipartUpload::Attachments(files),
payload_json: Some(to_string(map)?),
fields: vec![],
}),
Expand Down Expand Up @@ -2067,7 +2067,7 @@ impl Http {
request.body = Some(to_vec(map)?);
} else {
request.multipart = Some(Multipart {
upload: MultipartUpload::Attachments(new_attachments.into_iter().collect()),
upload: MultipartUpload::Attachments(new_attachments),
payload_json: Some(to_string(map)?),
fields: vec![],
});
Expand Down Expand Up @@ -2520,7 +2520,7 @@ impl Http {
request.body = Some(to_vec(map)?);
} else {
request.multipart = Some(Multipart {
upload: MultipartUpload::Attachments(files.into_iter().collect()),
upload: MultipartUpload::Attachments(files),
payload_json: Some(to_string(map)?),
fields: vec![],
});
Expand Down Expand Up @@ -4393,7 +4393,7 @@ impl Http {
request.body = Some(to_vec(map)?);
} else {
request.multipart = Some(Multipart {
upload: MultipartUpload::Attachments(files.into_iter().collect()),
upload: MultipartUpload::Attachments(files),
payload_json: Some(to_string(map)?),
fields: vec![],
});
Expand Down
4 changes: 2 additions & 2 deletions src/http/multipart.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ impl<'a> Multipart<'a> {
multipart = multipart.part("file", upload_file.into_part()?);
},
MultipartUpload::Attachments(attachment_files) => {
for file in attachment_files {
multipart = multipart.part(format!("files[{}]", file.id), file.into_part()?);
for (idx, file) in attachment_files.into_iter().enumerate() {
multipart = multipart.part(format!("files[{idx}]"), file.into_part()?);
}
},
}
Expand Down
Loading