media: improve Metadata typing

content_type is now optional; if not specified, it will remain
empty. `application/octet-stream` will be put on read in the
frontend.

Length is now represented as NonZeroUsize - why would you upload a
zero-byte file when you can just conjure one from the void whenever
you need one? This should save me a little bit of memory.

Representing content_type as a typed MIME value would be the next
logical step.

1 files changed, 11 insertions, 7 deletions

diff --git a/kittybox-rs/src/media/storage/file.rs b/kittybox-rs/src/media/storage/file.rs
index b46cdb2..c554d9e 100644
--- a/kittybox-rs/src/media/storage/file.rs
+++ b/kittybox-rs/src/media/storage/file.rs
@@ -83,6 +83,8 @@ impl MediaStore for FileStore {
         let mut length: usize = 0;
 
         while let Some(chunk) = content.next().await {
+            // TODO consider getting rid of this Arc as we only need immutable references
+            // I don't think we quite need refcounting here
             let chunk = std::sync::Arc::new(chunk.map_err(|err| MediaStoreError {
                 kind: ErrorKind::Backend,
                 source: Some(Box::new(err)),
@@ -113,8 +115,10 @@ impl MediaStore for FileStore {
             }
             hasher = _hasher;
         }
+        // Manually flush the buffer and drop the handle to close the file
         tempfile.flush().await?;
         tempfile.into_inner().sync_all().await?;
+
         let hash = hasher.finalize();
         debug!("Pending upload hash: {}", hex::encode(&hash));
         let filename = format!(
@@ -125,12 +129,12 @@ impl MediaStore for FileStore {
             hex::encode([hash[3]]),
             hex::encode(&hash[4..32])
         );
-        metadata.length = Some(length);
         let domain_str = domain.to_string();
         let filepath = self.base.join(domain_str.as_str()).join(&filename);
         let metafilename = filename.clone() + ".json";
         let metapath = self.base.join(domain_str.as_str()).join(&metafilename);
         let metatemppath = self.base.join(domain_str.as_str()).join(metafilename + ".tmp");
+        metadata.length = std::num::NonZeroUsize::new(length);
         debug!("File path: {}, metadata: {}", filepath.display(), metapath.display());
         {
             let parent = filepath.parent().unwrap();
@@ -174,7 +178,7 @@ impl MediaStore for FileStore {
                 // From the logs it looks like we're reading 4KiB at a time
                 // Buffering file contents seems to double download speed
                 // How to benchmark this?
-                tokio::io::BufReader::with_capacity(BUF_CAPACITY, file)
+                BufReader::with_capacity(BUF_CAPACITY, file)
             )
             // Sprinkle some salt in form of protective log wrapping
                 .inspect_ok(|chunk| debug!("Read {} bytes from file", chunk.len()))
@@ -205,7 +209,7 @@ mod tests {
         let stream = tokio_stream::iter(file.chunks(100).map(|i| Ok(bytes::Bytes::copy_from_slice(i))));
         let metadata = Metadata {
             filename: Some("style.css".to_string()),
-            content_type: "text/css".to_string(),
+            content_type: Some("text/css".to_string()),
             length: None
         };
 
@@ -231,9 +235,9 @@ mod tests {
                 .join("fireburn.ru")
                 .join(filename.clone() + ".json")
         ).await.unwrap()).unwrap();
-        assert_eq!(&meta.content_type, "text/css");
+        assert_eq!(meta.content_type.as_deref(), Some("text/css"));
         assert_eq!(meta.filename.as_deref(), Some("style.css"));
-        assert_eq!(meta.length, Some(file.len()));
+        assert_eq!(meta.length.map(|i| i.get()), Some(file.len()));
 
         // read back the data using the interface
         let (metadata, read_back) = {
@@ -250,9 +254,9 @@ mod tests {
         };
 
         assert_eq!(read_back, file);
-        assert_eq!(&metadata.content_type, "text/css");
+        assert_eq!(metadata.content_type.as_deref(), Some("text/css"));
         assert_eq!(meta.filename.as_deref(), Some("style.css"));
-        assert_eq!(meta.length, Some(file.len()));
+        assert_eq!(meta.length.map(|i| i.get()), Some(file.len()));
 
     }
 }