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

feat(http,model,util,validate)!: use Cow<'a, [u8]> for attachments #1923

Open
wants to merge 3 commits into
base: old-next
Choose a base branch
from
Open
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
10 changes: 5 additions & 5 deletions examples/model-webhook-slash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,12 @@ static PUB_KEY: Lazy<PublicKey> = Lazy::new(|| {
///
/// Responses are made by giving a function that takes a Interaction and returns
/// a InteractionResponse or a error.
async fn interaction_handler<F>(
async fn interaction_handler<'a, F>(
req: Request<Body>,
f: impl Fn(Box<CommandData>) -> F,
) -> anyhow::Result<Response<Body>>
where
F: Future<Output = anyhow::Result<InteractionResponse>>,
F: Future<Output = anyhow::Result<InteractionResponse<'a>>>,
{
// Check that the method used is a POST, all other methods are not allowed.
if req.method() != Method::POST {
Expand Down Expand Up @@ -133,7 +133,7 @@ where

/// Interaction handler that matches on the name of the interaction that
/// have been dispatched from Discord.
async fn handler(data: Box<CommandData>) -> anyhow::Result<InteractionResponse> {
async fn handler<'a>(data: Box<CommandData>) -> anyhow::Result<InteractionResponse<'a>> {
match data.name.as_ref() {
"vroom" => vroom(data).await,
"debug" => debug(data).await,
Expand All @@ -142,7 +142,7 @@ async fn handler(data: Box<CommandData>) -> anyhow::Result<InteractionResponse>
}

/// Example of a handler that returns the formatted version of the interaction.
async fn debug(data: Box<CommandData>) -> anyhow::Result<InteractionResponse> {
async fn debug<'a>(data: Box<CommandData>) -> anyhow::Result<InteractionResponse<'a>> {
Ok(InteractionResponse {
kind: InteractionResponseType::ChannelMessageWithSource,
data: Some(InteractionResponseData {
Expand All @@ -153,7 +153,7 @@ async fn debug(data: Box<CommandData>) -> anyhow::Result<InteractionResponse> {
}

/// Example of interaction that responds with a message saying "Vroom vroom".
async fn vroom(_: Box<CommandData>) -> anyhow::Result<InteractionResponse> {
async fn vroom<'a>(_: Box<CommandData>) -> anyhow::Result<InteractionResponse<'a>> {
Ok(InteractionResponse {
kind: InteractionResponseType::ChannelMessageWithSource,
data: Some(InteractionResponseData {
Expand Down
2 changes: 1 addition & 1 deletion twilight-http/src/client/interaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ impl<'a> InteractionClient<'a> {
&'a self,
interaction_id: Id<InteractionMarker>,
interaction_token: &'a str,
response: &'a InteractionResponse,
response: &'a InteractionResponse<'_>,
) -> CreateResponse<'a> {
CreateResponse::new(self.client, interaction_id, interaction_token, response)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ impl<'a> CreateFollowup<'a> {
/// [`AttachmentFilename`]: twilight_validate::message::MessageValidationErrorType::AttachmentFilename
pub fn attachments(
mut self,
attachments: &'a [Attachment],
attachments: &'a [Attachment<'_>],
) -> Result<Self, MessageValidationError> {
attachments.iter().try_for_each(validate_attachment)?;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use twilight_model::{
pub struct CreateResponse<'a> {
interaction_id: Id<InteractionMarker>,
interaction_token: &'a str,
response: &'a InteractionResponse,
response: &'a InteractionResponse<'a>,
http: &'a Client,
}

Expand All @@ -26,7 +26,7 @@ impl<'a> CreateResponse<'a> {
http: &'a Client,
interaction_id: Id<InteractionMarker>,
interaction_token: &'a str,
response: &'a InteractionResponse,
response: &'a InteractionResponse<'_>,
) -> Self {
Self {
interaction_id,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ impl<'a> UpdateFollowup<'a> {
/// [`AttachmentFilename`]: twilight_validate::message::MessageValidationErrorType::AttachmentFilename
pub fn attachments(
mut self,
attachments: &'a [Attachment],
attachments: &'a [Attachment<'_>],
) -> Result<Self, MessageValidationError> {
attachments.iter().try_for_each(validate_attachment)?;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ impl<'a> UpdateResponse<'a> {
/// [`AttachmentFilename`]: twilight_validate::message::MessageValidationErrorType::AttachmentFilename
pub fn attachments(
mut self,
attachments: &'a [Attachment],
attachments: &'a [Attachment<'_>],
) -> Result<Self, MessageValidationError> {
attachments.iter().try_for_each(validate_attachment)?;

Expand Down
4 changes: 2 additions & 2 deletions twilight-http/src/request/attachment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use twilight_model::{
};

pub struct AttachmentManager<'a> {
files: Vec<&'a Attachment>,
files: Vec<&'a Attachment<'a>>,
ids: Vec<Id<AttachmentMarker>>,
}

Expand Down Expand Up @@ -54,7 +54,7 @@ impl<'a> AttachmentManager<'a> {
}

#[must_use = "has no effect if not built into a Form"]
pub fn set_files(mut self, files: Vec<&'a Attachment>) -> Self {
pub fn set_files(mut self, files: Vec<&'a Attachment<'a>>) -> Self {
self.files = files;

self
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ impl<'a> CreateMessage<'a> {
/// [`AttachmentFilename`]: twilight_validate::message::MessageValidationErrorType::AttachmentFilename
pub fn attachments(
mut self,
attachments: &'a [Attachment],
attachments: &'a [Attachment<'_>],
) -> Result<Self, MessageValidationError> {
attachments.iter().try_for_each(validate_attachment)?;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ impl<'a> UpdateMessage<'a> {
/// [`AttachmentFilename`]: twilight_validate::message::MessageValidationErrorType::AttachmentFilename
pub fn attachments(
mut self,
attachments: &'a [Attachment],
attachments: &'a [Attachment<'a>],
) -> Result<Self, MessageValidationError> {
attachments.iter().try_for_each(validate_attachment)?;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ impl<'a> ExecuteWebhook<'a> {
/// [`AttachmentFilename`]: twilight_validate::message::MessageValidationErrorType::AttachmentFilename
pub fn attachments(
mut self,
attachments: &'a [Attachment],
attachments: &'a [Attachment<'_>],
) -> Result<Self, MessageValidationError> {
attachments.iter().try_for_each(validate_attachment)?;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ impl<'a> UpdateWebhookMessage<'a> {
/// [`AttachmentFilename`]: twilight_validate::message::MessageValidationErrorType::AttachmentFilename
pub fn attachments(
mut self,
attachments: &'a [Attachment],
attachments: &'a [Attachment<'_>],
) -> Result<Self, MessageValidationError> {
attachments.iter().try_for_each(validate_attachment)?;

Expand Down
16 changes: 10 additions & 6 deletions twilight-model/src/http/attachment.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
//! Models used when sending attachments to Discord.

use std::borrow::Cow;

use serde::{Deserialize, Serialize};

/// Attachments used in messages.
Expand All @@ -10,6 +12,7 @@ use serde::{Deserialize, Serialize};
/// description for screen readers:
///
/// ```
/// use std::borrow::Cow;
/// use twilight_model::http::attachment::Attachment;
///
/// let filename = "twilight_sparkle.json".to_owned();
Expand All @@ -21,18 +24,18 @@ use serde::{Deserialize, Serialize};
/// .to_vec();
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't encourage needless allocations.

Suggested change
/// .to_vec();

/// let id = 1;
///
/// let mut attachment = Attachment::from_bytes(filename, file_content, id);
/// let mut attachment = Attachment::from_bytes(filename, Cow::from(file_content), id);
Copy link
Member

Choose a reason for hiding this comment

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

Cow::from is not smart enough for &'static [u8] data (needed for it to work without .to_vec())

Suggested change
/// let mut attachment = Attachment::from_bytes(filename, Cow::from(file_content), id);
/// let mut attachment = Attachment::from_bytes(filename, Cow::Borrowed(file_content), id);

/// attachment.description("Raw data about Twilight Sparkle".to_owned());
/// ```
#[derive(Clone, Debug, Deserialize, Eq, Hash, PartialEq, Serialize)]
pub struct Attachment {
pub struct Attachment<'a> {
/// Description of the attachment, useful for screen readers and users
/// requiring alt text.
#[serde(skip_serializing_if = "Option::is_none")]
pub description: Option<String>,
/// Content of the file.
#[serde(skip)]
pub file: Vec<u8>,
pub file: Cow<'a, [u8]>,
/// Name of the file.
///
/// Examples may be "twilight_sparkle.png", "cat.jpg", or "logs.txt".
Expand All @@ -46,23 +49,24 @@ pub struct Attachment {
pub id: u64,
}

impl Attachment {
impl<'a> Attachment<'a> {
/// Create an attachment from a filename and bytes.
///
/// # Examples
///
/// Create an attachment with a grocery list named "grocerylist.txt":
///
/// ```
/// use std::borrow::Cow;
/// use twilight_model::http::attachment::Attachment;
///
/// let filename = "grocerylist.txt".to_owned();
/// let file_content = b"Apples\nGrapes\nLemonade".to_vec();
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// let file_content = b"Apples\nGrapes\nLemonade".to_vec();
/// let file_content = b"Apples\nGrapes\nLemonade";

/// let id = 1;
///
/// let attachment = Attachment::from_bytes(filename, file_content, id);
/// let attachment = Attachment::from_bytes(filename, Cow::from(file_content), id);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// let attachment = Attachment::from_bytes(filename, Cow::from(file_content), id);
/// let attachment = Attachment::from_bytes(filename, Cow::Borrowed(file_content), id);

/// ```
pub const fn from_bytes(filename: String, file: Vec<u8>, id: u64) -> Self {
pub const fn from_bytes(filename: String, file: Cow<'a, [u8]>, id: u64) -> Self {
Self {
description: None,
file,
Expand Down
12 changes: 6 additions & 6 deletions twilight-model/src/http/interaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use serde_repr::{Deserialize_repr, Serialize_repr};
///
/// [Discord Docs/Interaction Object]: https://discord.com/developers/docs/interactions/receiving-and-responding#interaction-object-interaction-structure
#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)]
pub struct InteractionResponse {
pub struct InteractionResponse<'a> {
/// Type of the response.
#[serde(rename = "type")]
pub kind: InteractionResponseType,
Expand All @@ -31,18 +31,18 @@ pub struct InteractionResponse {
/// [`Modal`]: InteractionResponseType::Modal
/// [`UpdateMessage`]: InteractionResponseType::UpdateMessage
#[serde(skip_serializing_if = "Option::is_none")]
pub data: Option<InteractionResponseData>,
pub data: Option<InteractionResponseData<'a>>,
}

/// Data included in an interaction response.
#[derive(Clone, Debug, Default, Deserialize, PartialEq, Serialize)]
pub struct InteractionResponseData {
pub struct InteractionResponseData<'a> {
/// Allowed mentions of the response.
#[serde(skip_serializing_if = "Option::is_none")]
pub allowed_mentions: Option<AllowedMentions>,
/// List of attachments on the response.
#[serde(skip_serializing_if = "Option::is_none")]
pub attachments: Option<Vec<Attachment>>,
pub attachments: Option<Vec<Attachment<'a>>>,
/// List of autocomplete alternatives.
///
/// Can only be used with
Expand Down Expand Up @@ -126,7 +126,7 @@ mod tests {
tts
);
assert_impl_all!(
InteractionResponseData: Clone,
InteractionResponseData<'_>: Clone,
Debug,
Deserialize<'static>,
PartialEq,
Expand Down Expand Up @@ -187,7 +187,7 @@ mod tests {
data: Some(InteractionResponseData {
attachments: Some(Vec::from([Attachment {
description: None,
file: "file data".into(),
file: "file data".as_bytes().into(),
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
file: "file data".as_bytes().into(),
file: Cow::Borrowed(b"file data"),

filename: "filename.jpg".into(),
id: 1,
}])),
Expand Down
12 changes: 6 additions & 6 deletions twilight-util/src/builder/interaction_response_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ use twilight_model::{
/// ```
#[derive(Clone, Debug)]
#[must_use = "builders have no effect if unused"]
pub struct InteractionResponseDataBuilder(InteractionResponseData);
pub struct InteractionResponseDataBuilder<'a>(InteractionResponseData<'a>);

impl InteractionResponseDataBuilder {
impl<'a> InteractionResponseDataBuilder<'a> {
/// Create a new builder to construct an [`InteractionResponseData`].
pub const fn new() -> Self {
Self(InteractionResponseData {
Expand All @@ -57,7 +57,7 @@ impl InteractionResponseDataBuilder {
/// Consume the builder, returning an [`InteractionResponseData`].
#[allow(clippy::missing_const_for_fn)]
#[must_use = "builders have no effect if unused"]
pub fn build(self) -> InteractionResponseData {
pub fn build(self) -> InteractionResponseData<'a> {
self.0
}

Expand All @@ -74,7 +74,7 @@ impl InteractionResponseDataBuilder {
/// Set the attachments of the message.
///
/// Defaults to [`None`].
pub fn attachments(mut self, attachments: impl IntoIterator<Item = Attachment>) -> Self {
pub fn attachments(mut self, attachments: impl IntoIterator<Item = Attachment<'a>>) -> Self {
self.0.attachments = Some(attachments.into_iter().collect());

self
Expand Down Expand Up @@ -161,7 +161,7 @@ impl InteractionResponseDataBuilder {
}
}

impl Default for InteractionResponseDataBuilder {
impl<'a> Default for InteractionResponseDataBuilder<'a> {
fn default() -> Self {
Self::new()
}
Expand All @@ -178,7 +178,7 @@ mod tests {
};

assert_impl_all!(
InteractionResponseDataBuilder: Clone,
InteractionResponseDataBuilder<'_>: Clone,
Debug,
Default,
Send,
Expand Down
2 changes: 1 addition & 1 deletion twilight-validate/src/message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ pub enum MessageValidationErrorType {
///
/// [`AttachmentDescriptionTooLarge`]: MessageValidationErrorType::AttachmentDescriptionTooLarge
/// [`AttachmentFilename`]: MessageValidationErrorType::AttachmentFilename
pub fn attachment(attachment: &Attachment) -> Result<(), MessageValidationError> {
pub fn attachment(attachment: &Attachment<'_>) -> Result<(), MessageValidationError> {
attachment_filename(&attachment.filename)?;

if let Some(description) = &attachment.description {
Expand Down