Merge pull request #237 from teloxide/file_meta_meta

Properly fix file size/path

CHANGELOG.md

@@ -15,13 +15,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 [pr238]: https://github.com/teloxide/teloxide-core/pull/238
 
-### Fixed
-
-- `File::{file_size, file_path}` are now optional ([#233][pr233])
-
 ### Changed
 
--  `InlineKeyboardButtonKind::Pay`'s only field now has type `True` ([#231][pr231])
+- `InlineKeyboardButtonKind::Pay`'s only field now has type `True` ([#231][pr231])
+- `file_size` fields are now always `u32` ([#237][pr237])
+- `File` is now split into `File` and `FileMeta`, the latter is used in `UploadStickerFile` and `Sticker::premium_animation` ([#237][pr237])
+
+[pr237]: https://github.com/teloxide/teloxide-core/pull/237
 
 ### Deprecated
 

src/net/download.rs

@@ -44,7 +44,7 @@ pub trait Download<'w>
     ///
     /// let TgFile { file_path, .. } = bot.get_file("*file_id*").send().await?;
     /// let mut file = File::create("/tmp/test.png").await?;
-    /// bot.download_file(&file_path.unwrap(), &mut file).await?;
+    /// bot.download_file(&file_path, &mut file).await?;
     /// # Ok(()) }
     /// ```
     ///

src/payloads/upload_sticker_file.rs

@@ -8,13 +8,13 @@
 // [`schema`]: https://github.com/WaffleLapkin/tg-methods-schema
 use serde::Serialize;
 
-use crate::types::{File, InputFile, UserId};
+use crate::types::{FileMeta, InputFile, UserId};
 
 impl_payload! {
     @[multipart = png_sticker]
     /// Use this method to upload a .PNG file with a sticker for later use in _createNewStickerSet_ and _addStickerToSet_ methods (can be used multiple times). Returns the uploaded File on success.
     #[derive(Debug, Clone, Serialize)]
-    pub UploadStickerFile (UploadStickerFileSetters) => File {
+    pub UploadStickerFile (UploadStickerFileSetters) => FileMeta {
         required {
             /// User identifier of sticker file owner
             pub user_id: UserId,

src/types/animation.rs

@@ -38,7 +38,8 @@ pub struct Animation {
     pub mime_type: Option<Mime>,
 
     /// File size in bytes.
-    pub file_size: Option<u64>,
+    #[serde(default = "crate::types::file::file_size_fallback")]
+    pub file_size: u32,
 }
 
 #[cfg(test)]
@@ -74,11 +75,11 @@ mod tests {
                 file_unique_id: "".to_string(),
                 width: 320,
                 height: 320,
-                file_size: Some(3452),
+                file_size: 3452,
             }),
             file_name: Some("some".to_string()),
             mime_type: Some("video/gif".parse().unwrap()),
-            file_size: Some(6500),
+            file_size: 6500,
         };
         let actual = serde_json::from_str::<Animation>(json).unwrap();
         assert_eq!(actual, expected)

src/types/audio.rs

@@ -35,7 +35,8 @@ pub struct Audio {
     pub mime_type: Option<Mime>,
 
     /// File size in bytes.
-    pub file_size: Option<u64>,
+    #[serde(default = "crate::types::file::file_size_fallback")]
+    pub file_size: u32,
 
     /// A thumbnail of the album cover to which the music file belongs.
     pub thumb: Option<PhotoSize>,
@@ -70,13 +71,13 @@ mod tests {
             performer: Some("Performer".to_string()),
             title: Some("Title".to_string()),
             mime_type: Some("application/zip".parse().unwrap()),
-            file_size: Some(123_456),
+            file_size: 123_456,
             thumb: Some(PhotoSize {
                 file_id: "id".to_string(),
                 file_unique_id: "".to_string(),
                 width: 320,
                 height: 320,
-                file_size: Some(3452),
+                file_size: 3452,
             }),
             file_name: None,
         };

src/types/document.rs

@@ -33,5 +33,6 @@ pub struct Document {
     pub mime_type: Option<Mime>,
 
     /// File size in bytes.
-    pub file_size: Option<u64>,
+    #[serde(default = "crate::types::file::file_size_fallback")]
+    pub file_size: u32,
 }

src/types/file.rs

@@ -1,3 +1,5 @@
+use std::ops::Deref;
+
 use serde::{Deserialize, Serialize};
 
 /// This object represents a file ready to be downloaded.
@@ -13,6 +15,22 @@ use serde::{Deserialize, Serialize};
 /// [`Bot::download_file(file_path, dst)`]: crate::net::Download::download_file
 #[derive(Clone, Debug, Eq, Hash, PartialEq, Serialize, Deserialize)]
 pub struct File {
+    /// Metadata of the file.
+    ///
+    /// Note that [`FileMeta`]'s fields are available on `File` too (via deref
+    /// coercion).
+    #[serde(flatten)]
+    pub meta: FileMeta,
+
+    /// File path. Use [`Bot::download_file(file_path, dst)`] to get the file.
+    ///
+    /// [`Bot::download_file(file_path, dst)`]: crate::net::Download::download_file
+    pub file_path: String,
+}
+
+/// Metadata of the [`File`].
+#[derive(Clone, Debug, Eq, Hash, PartialEq, Serialize, Deserialize)]
+pub struct FileMeta {
     /// Identifier for this file.
     pub file_id: String,
 
@@ -21,19 +39,49 @@ pub struct File {
     /// file.
     pub file_unique_id: String,
 
-    /// File size in bytes, if known.
-    pub file_size: Option<u64>,
+    /// File size in bytes.
+    // This should never be necessary in practice,
+    // but just in case something goes wrong with the TBA server
+    // (see the test below)
+    #[serde(default = "file_size_fallback")]
+    pub file_size: u32,
+}
 
-    /// File path. Use [`Bot::download_file(file_path, dst)`] to get the file.
-    ///
-    /// [`Bot::download_file(file_path, dst)`]: crate::net::Download::download_file
-    pub file_path: Option<String>,
+pub(crate) const fn file_size_fallback() -> u32 {
+    u32::MAX
+}
+
+/// Allows access to [`FileMeta`]'s fields for [`File`].
+///
+/// ## Examples
+///
+/// ```rust
+/// use teloxide_core::types::File;
+/// #
+/// # let get_file = || File { meta: teloxide_core::types::FileMeta { file_id: String::new(), file_unique_id: String::new(), file_size: 0 }, file_path: String::new() };
+/// let file: File = get_file();
+///
+/// let file_id: &str = &file.file_id;
+/// let file_unique_id: &str = &file.file_unique_id;
+/// let file_size: u32 = file.file_size;
+/// #
+/// # let _ = (file_id, file_unique_id, file_size);
+/// ```
+impl Deref for File {
+    type Target = FileMeta;
+
+    fn deref(&self) -> &Self::Target {
+        &self.meta
+    }
 }
 
 #[cfg(test)]
 mod tests {
-    use crate::types::File;
+    use crate::types::{File, FileMeta};
 
+    // As per <https://github.com/tdlib/telegram-bot-api/issues/192> file size is **not** optional,
+    // But <https://github.com/tdlib/telegram-bot-api/issues/294> suggests that it can be missing in case Telegram servers are going insane.
+    // To be safe, we use a placeholder value.
     #[test]
     fn no_file_size() {
         let json =
@@ -43,26 +91,47 @@ mod tests {
         assert_eq!(
             file,
             File {
+                meta: FileMeta {
+                    file_id: "FILE_ID".to_owned(),
+                    file_unique_id: "FILE_UNIQUE_ID".to_owned(),
+                    file_size: u32::MAX,
+                },
+                file_path: "FILE_PATH".to_owned(),
+            }
+        );
+    }
+
+    // In some places file metadata w/o path is returned. Make sure that we can
+    // deserialize it.
+    #[test]
+    fn no_file_path() {
+        let json = r#"{"file_id":"FILE_ID","file_unique_id":"FILE_UNIQUE_ID","file_size":42}"#;
+        let file: FileMeta = serde_json::from_str(json).unwrap();
+
+        assert_eq!(
+            file,
+            FileMeta {
                 file_id: "FILE_ID".to_owned(),
                 file_unique_id: "FILE_UNIQUE_ID".to_owned(),
-                file_size: None,
-                file_path: Some("FILE_PATH".to_owned()),
+                file_size: 42,
             }
         );
     }
 
     #[test]
-    fn no_file_path() {
-        let json = r#"{"file_id":"FILE_ID","file_unique_id":"FILE_UNIQUE_ID","file_size":42}"#;
+    fn full_file() {
+        let json = r#"{"file_id":"FILE_ID","file_unique_id":"FILE_UNIQUE_ID","file_size":42,"file_path":"FILE_PATH"}"#;
         let file: File = serde_json::from_str(json).unwrap();
 
         assert_eq!(
             file,
             File {
-                file_id: "FILE_ID".to_owned(),
-                file_unique_id: "FILE_UNIQUE_ID".to_owned(),
-                file_size: Some(42),
-                file_path: None,
+                meta: FileMeta {
+                    file_id: "FILE_ID".to_owned(),
+                    file_unique_id: "FILE_UNIQUE_ID".to_owned(),
+                    file_size: 42,
+                },
+                file_path: "FILE_PATH".to_owned(),
             }
         );
     }

src/types/passport_file.rs

@@ -18,7 +18,7 @@ pub struct PassportFile {
     pub file_unique_id: String,
 
     /// File size in bytes.
-    pub file_size: u64, // FIXME: should be u32
+    pub file_size: u32,
 
     /// Time when the file was uploaded.
     #[serde(with = "crate::types::serde_date_from_unix_timestamp")]

src/types/photo_size.rs

@@ -22,7 +22,8 @@ pub struct PhotoSize {
     pub height: u32,
 
     /// File size in bytes.
-    pub file_size: Option<u32>,
+    #[serde(default = "crate::types::file::file_size_fallback")]
+    pub file_size: u32,
 }
 
 #[cfg(test)]
@@ -38,7 +39,7 @@ mod tests {
             file_unique_id: "".to_string(),
             width: 320,
             height: 320,
-            file_size: Some(3452),
+            file_size: 3452,
         };
         let actual = serde_json::from_str::<PhotoSize>(json).unwrap();
         assert_eq!(actual, expected);

src/types/sticker.rs

@@ -2,7 +2,7 @@ use std::{convert::TryFrom, ops::Deref};
 
 use serde::{Deserialize, Serialize};
 
-use crate::types::{File, MaskPosition, PhotoSize};
+use crate::types::{FileMeta, MaskPosition, PhotoSize};
 
 /// This object represents a sticker.
 ///
@@ -38,13 +38,14 @@ pub struct Sticker {
     pub set_name: Option<String>,
 
     /// Premium animation for the sticker, if the sticker is premium.
-    pub premium_animation: Option<File>,
+    pub premium_animation: Option<FileMeta>,
 
     /// For mask stickers, the position where the mask should be placed.
     pub mask_position: Option<MaskPosition>,
 
     /// File size in bytes.
-    pub file_size: Option<u32>,
+    #[serde(default = "crate::types::file::file_size_fallback")]
+    pub file_size: u32,
 }
 
 /// Kind of a sticker - webp, animated or video.

src/types/video.rs

@@ -37,5 +37,6 @@ pub struct Video {
     pub mime_type: Option<Mime>,
 
     /// File size in bytes.
-    pub file_size: Option<u64>,
+    #[serde(default = "crate::types::file::file_size_fallback")]
+    pub file_size: u32,
 }

src/types/video_note.rs

@@ -31,5 +31,6 @@ pub struct VideoNote {
     pub thumb: Option<PhotoSize>,
 
     /// File size in bytes.
-    pub file_size: Option<u32>,
+    #[serde(default = "crate::types::file::file_size_fallback")]
+    pub file_size: u32,
 }

src/types/voice.rs

@@ -23,5 +23,6 @@ pub struct Voice {
     pub mime_type: Option<Mime>,
 
     /// File size in bytes.
-    pub file_size: Option<u64>,
+    #[serde(default = "crate::types::file::file_size_fallback")]
+    pub file_size: u32,
 }