[pbs-devel] [PATCH v4 proxmox 1/3] compression: Refactor ZipEntry creation and add FileType enum

[Filip Schauer <f.schauer@proxmox.com>]

Refactor the process of adding an entry to the ZipEncoder. Instead of
passing a ZipEntry and the content seperately, pass all parameters
required to create a ZipEntry directly to add_entry, including a new
FileType enum that contains the file content if it is a regular file.

Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
---
 proxmox-compression/src/zip.rs | 145 +++++++++++++++++++--------------
 1 file changed, 82 insertions(+), 63 deletions(-)

diff --git a/proxmox-compression/src/zip.rs b/proxmox-compression/src/zip.rs
index d2d3fd8..6229990 100644
--- a/proxmox-compression/src/zip.rs
+++ b/proxmox-compression/src/zip.rs
@@ -17,6 +17,7 @@ use std::time::SystemTime;
 use anyhow::{format_err, Error, Result};
 use endian_trait::Endian;
 use futures::ready;
+use libc::{S_IFDIR, S_IFREG};
 use tokio::io::{AsyncRead, AsyncWrite, AsyncWriteExt, ReadBuf};
 
 use crc32fast::Hasher;
@@ -71,6 +72,20 @@ fn epoch_to_dos(epoch: i64) -> (u16, u16) {
     (date, time)
 }
 
+pub enum FileType<R> {
+    Directory,
+    Regular(R),
+}
+
+impl<R: AsyncRead + Unpin + Send> FileType<R> {
+    fn take_reader(&mut self) -> Option<HashWrapper<&mut (dyn AsyncRead + Unpin + Send)>> {
+        match self {
+            FileType::Directory => None,
+            FileType::Regular(content) => Some(HashWrapper::new(content)),
+        }
+    }
+}
+
 #[derive(Endian)]
 #[repr(C, packed)]
 struct Zip64Field {
@@ -192,9 +207,7 @@ where
 }
 
 /// Represents an Entry in a ZIP File
-///
-/// used to add to a ZipEncoder
-pub struct ZipEntry {
+struct ZipEntry {
     filename: OsString,
     mtime: i64,
     mode: u16,
@@ -202,16 +215,20 @@ pub struct ZipEntry {
     uncompressed_size: u64,
     compressed_size: u64,
     offset: u64,
-    is_file: bool,
     is_utf8_filename: bool,
 }
 
 impl ZipEntry {
     /// Creates a new ZipEntry
     ///
-    /// if is_file is false the path will contain an trailing separator,
+    /// if file_type is Directory the path will contain a trailing separator,
     /// so that the zip file understands that it is a directory
-    pub fn new<P: AsRef<Path>>(path: P, mtime: i64, mode: u16, is_file: bool) -> Self {
+    fn new<P: AsRef<Path>, R: AsyncRead + Unpin>(
+        path: P,
+        mtime: i64,
+        mode: u16,
+        file_type: &FileType<R>,
+    ) -> Self {
         let mut relpath = PathBuf::new();
 
         for comp in path.as_ref().components() {
@@ -220,22 +237,26 @@ impl ZipEntry {
             }
         }
 
-        if !is_file {
+        if matches!(file_type, FileType::Directory) {
             relpath.push(""); // adds trailing slash
         }
 
         let filename: OsString = relpath.into();
         let is_utf8_filename = filename.to_str().is_some();
 
+        let file_type_attr = match file_type {
+            FileType::Regular(_) => S_IFREG,
+            FileType::Directory => S_IFDIR,
+        };
+
         Self {
             filename,
             crc32: 0,
             mtime,
-            mode,
+            mode: mode | file_type_attr as u16,
             uncompressed_size: 0,
             compressed_size: 0,
             offset: 0,
-            is_file,
             is_utf8_filename,
         }
     }
@@ -342,6 +363,8 @@ impl ZipEntry {
             )
         };
 
+        let is_directory = (self.mode & 0xF000) == S_IFDIR as u16;
+
         write_struct(
             &mut buf,
             CentralDirectoryFileHeader {
@@ -360,7 +383,7 @@ impl ZipEntry {
                 comment_len: 0,
                 start_disk: 0,
                 internal_flags: 0,
-                external_flags: (self.mode as u32) << 16 | (!self.is_file as u32) << 4,
+                external_flags: (self.mode as u32) << 16 | (is_directory as u32) << 4,
                 offset,
             },
         )
@@ -437,7 +460,7 @@ where
 /// use anyhow::{Error, Result};
 /// use tokio::fs::File;
 ///
-/// use proxmox_compression::zip::{ZipEncoder, ZipEntry};
+/// use proxmox_compression::zip::{FileType, ZipEncoder};
 ///
 /// //#[tokio::main]
 /// async fn main_() -> Result<(), Error> {
@@ -445,12 +468,12 @@ where
 ///     let mut source = File::open("foo.txt").await?;
 ///
 ///     let mut zip = ZipEncoder::new(target);
-///     zip.add_entry(ZipEntry::new(
+///     zip.add_entry(
 ///         "foo.txt",
 ///         0,
 ///         0o100755,
-///         true,
-///     ), Some(source)).await?;
+///         FileType::Regular(source),
+///     ).await?;
 ///
 ///     zip.finish().await?;
 ///
@@ -475,34 +498,38 @@ impl<W: AsyncWrite + Unpin> ZipEncoder<W> {
         }
     }
 
-    pub async fn add_entry<R: AsyncRead + Unpin>(
+    pub async fn add_entry<P: AsRef<Path>, R: AsyncRead + Unpin + Send>(
         &mut self,
-        mut entry: ZipEntry,
-        content: Option<R>,
+        path: P,
+        mtime: i64,
+        mode: u16,
+        mut file_type: FileType<R>,
     ) -> Result<(), Error> {
         let mut target = self
             .target
             .take()
             .ok_or_else(|| format_err!("had no target during add entry"))?;
+        let mut entry = ZipEntry::new(path, mtime, mode, &file_type);
         entry.offset = self.byte_count.try_into()?;
         self.byte_count += entry.write_local_header(&mut target).await?;
-        if let Some(content) = content {
-            let mut reader = HashWrapper::new(content);
+
+        if !matches!(file_type, FileType::Directory) {
             let mut enc = DeflateEncoder::with_quality(target, Level::Fastest);
 
-            enc.compress(&mut reader).await?;
+            if let Some(mut reader) = file_type.take_reader() {
+                enc.compress(&mut reader).await?;
+                entry.crc32 = reader.finish().0;
+            }
+
             let total_in = enc.total_in();
             let total_out = enc.total_out();
             target = enc.into_inner();
 
-            let (crc32, _reader) = reader.finish();
-
             self.byte_count += total_out as usize;
             entry.compressed_size = total_out;
             entry.uncompressed_size = total_in;
-
-            entry.crc32 = crc32;
         }
+
         self.byte_count += entry.write_data_descriptor(&mut target).await?;
         self.target = Some(target);
 
@@ -640,46 +667,38 @@ where
             }
         };
 
-        let entry_path = entry.path().to_owned();
         let encoder = &mut encoder;
-
-        let entry = async move {
-            let entry_path_no_base = entry.path().strip_prefix(base_path)?;
-            let metadata = entry.metadata()?;
-            let mtime = match metadata
-                .modified()
-                .unwrap_or_else(|_| SystemTime::now())
-                .duration_since(SystemTime::UNIX_EPOCH)
-            {
-                Ok(dur) => dur.as_secs() as i64,
-                Err(time_error) => -(time_error.duration().as_secs() as i64),
-            };
-            let mode = metadata.mode() as u16;
-
-            if entry.file_type().is_file() {
-                let file = tokio::fs::File::open(entry.path()).await?;
-                let ze = ZipEntry::new(entry_path_no_base, mtime, mode, true);
-                Ok(Some((ze, Some(file))))
-            } else if entry.file_type().is_dir() {
-                let ze = ZipEntry::new(entry_path_no_base, mtime, mode, false);
-                let content: Option<tokio::fs::File> = None;
-                Ok(Some((ze, content)))
-            } else {
-                // ignore other file types
-                Ok::<_, Error>(None)
-            }
-        }
-        .await;
-        match entry {
-            Ok(Some((ze, content))) => encoder.add_entry(ze, content).await?,
-            Ok(None) => {}
-            Err(err) => {
-                eprintln!(
-                    "zip: error encoding file or directory '{}': {}",
-                    entry_path.display(),
-                    err
-                );
-            }
+        let entry_path_no_base = entry.path().strip_prefix(base_path)?;
+        let metadata = entry.metadata()?;
+        let mtime = match metadata
+            .modified()
+            .unwrap_or_else(|_| SystemTime::now())
+            .duration_since(SystemTime::UNIX_EPOCH)
+        {
+            Ok(dur) => dur.as_secs() as i64,
+            Err(time_error) => -(time_error.duration().as_secs() as i64),
+        };
+        let mode = metadata.mode() as u16;
+
+        if entry.file_type().is_file() {
+            let file = tokio::fs::File::open(entry.path()).await?;
+            encoder
+                .add_entry(
+                    entry_path_no_base,
+                    mtime,
+                    mode,
+                    FileType::Regular(Box::new(file)),
+                )
+                .await?;
+        } else if entry.file_type().is_dir() {
+            encoder
+                .add_entry(
+                    entry_path_no_base,
+                    mtime,
+                    mode,
+                    FileType::<&[u8]>::Directory,
+                )
+                .await?;
         }
     }
 
-- 
2.39.2