fix null characters, read permission and path (#196)

* fix null characters, read permission and path

* yank version

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "subspace-cli"
-version = "0.4.0"
+version = "0.4.1"
 edition = "2021"
 
 [dependencies]
@@ -37,6 +37,9 @@ whoami = "1"
 
 subspace-sdk = { git = "https://github.com/subspace/subspace-sdk", rev = "faf31d4e8079c45f4512ebe77aa77bfb27326b01" }
 
+[dev-dependencies]
+rand = "0.8.5"
+
 # The only triple tested and confirmed as working in `jemallocator` crate is `x86_64-unknown-linux-gnu`
 [target.'cfg(all(target_arch = "x86_64", target_vendor = "unknown", target_os = "linux", target_env = "gnu"))'.dependencies]
 jemallocator = "0.5.0"

src/summary.rs

@@ -13,7 +13,7 @@ use serde::{Deserialize, Serialize};
 use subspace_sdk::node::BlockNumber;
 use subspace_sdk::ByteSize;
 use tokio::fs::{create_dir_all, File, OpenOptions};
-use tokio::io::{AsyncReadExt, AsyncWriteExt};
+use tokio::io::{AsyncReadExt, AsyncSeekExt, AsyncWriteExt};
 use tokio::sync::Mutex;
 use tracing::instrument;
 
@@ -58,9 +58,7 @@ impl SummaryFile {
     #[instrument]
     pub(crate) async fn new(user_space_pledged: Option<ByteSize>) -> Result<SummaryFile> {
         let summary_path = summary_path();
-        let summary_dir = dirs::cache_dir()
-            .expect("couldn't get the default local data directory!")
-            .join("subspace-cli");
+        let summary_dir = summary_dir();
 
         let mut summary_file;
         // providing `Some` value for `user_space_pledged` means, we are creating a new
@@ -83,6 +81,7 @@ impl SummaryFile {
                 let summary_text =
                     toml::to_string(&initialization).context("Failed to serialize Summary")?;
                 summary_file = OpenOptions::new()
+                    .read(true)
                     .write(true)
                     .truncate(true)
                     .open(&summary_path)
@@ -92,12 +91,18 @@ impl SummaryFile {
                     .write_all(summary_text.as_bytes())
                     .await
                     .context("write to summary failed")?;
+                summary_file.flush().await.context("flush at creation failed")?;
+                summary_file
+                    .seek(std::io::SeekFrom::Start(0))
+                    .await
+                    .context("couldn't seek to the beginning of the summary file")?;
 
                 return Ok(SummaryFile { inner: Arc::new(Mutex::new(summary_file)) });
             }
         }
         // for all the other cases, the SummaryFile should be there
         summary_file = OpenOptions::new()
+            .read(true)
             .write(true)
             .open(&summary_path)
             .await
@@ -110,13 +115,19 @@ impl SummaryFile {
     pub(crate) async fn parse(&self) -> Result<Summary> {
         let mut guard = self.inner.lock().await;
         let mut contents = String::new();
+
         guard
             .read_to_string(&mut contents)
             .await
             .context("couldn't read the contents of the summary file")?;
         let summary: Summary =
             toml::from_str(&contents).context("couldn't serialize the summary content")?;
 
+        guard
+            .seek(std::io::SeekFrom::Start(0))
+            .await
+            .context("couldn't seek to the beginning of the summary file")?;
+
         Ok(summary)
     }
 
@@ -136,7 +147,7 @@ impl SummaryFile {
             new_parsed_blocks,
         }: SummaryUpdateFields,
     ) -> Result<Summary> {
-        let mut summary: Summary = Default::default();
+        let mut summary = self.parse().await.context("couldn't parse summary in update method")?;
 
         if is_plotting_finished {
             summary.initial_plotting_finished = true;
@@ -152,14 +163,16 @@ impl SummaryFile {
 
         let serialized_summary =
             toml::to_string(&summary).context("Failed to serialize Summary")?;
-        // this will only create the pointer, and will not override the file
         let mut guard = self.inner.lock().await;
-        // truncate the file
         guard.set_len(0).await.context("couldn't truncate the summary file")?;
         guard
             .write_all(serialized_summary.as_bytes())
             .await
             .context("couldn't write to summary file")?;
+        guard
+            .seek(std::io::SeekFrom::Start(0))
+            .await
+            .context("couldn't seek to the beginning of the summary file")?;
         guard.flush().await.context("flushing failed for summary file")?;
 
         Ok(summary)
@@ -174,8 +187,11 @@ pub(crate) fn delete_summary() -> Result<()> {
 
 /// returns the path for the summary file
 #[instrument]
-fn summary_path() -> PathBuf {
-    let summary_path =
-        dirs::data_local_dir().expect("couldn't get the default local data directory!");
-    summary_path.join("subspace-cli").join("summary.toml")
+pub(crate) fn summary_path() -> PathBuf {
+    summary_dir().join("summary.toml")
+}
+
+#[instrument]
+fn summary_dir() -> PathBuf {
+    dirs::cache_dir().expect("couldn't get the  directory!").join("subspace-cli")
 }

src/tests.rs

@@ -1,12 +1,74 @@
 use std::str::FromStr;
 
+use rand::rngs::SmallRng;
+use rand::{Rng, SeedableRng};
+use subspace_sdk::ByteSize;
+
 use crate::config::ChainConfig;
+use crate::summary::*;
 use crate::utils::{
     apply_extra_options, cache_directory_getter, custom_log_dir, directory_parser,
     node_directory_getter, node_name_parser, plot_directory_getter, reward_address_parser,
     size_parser, yes_or_no_parser,
 };
 
+async fn update_summary_file_randomly(summary_file: SummaryFile) {
+    let mut rng = SmallRng::from_entropy();
+
+    for _ in 0..5 {
+        let update_fields = SummaryUpdateFields {
+            is_plotting_finished: false,
+            new_authored_count: rng.gen_range(1..10),
+            new_vote_count: rng.gen_range(1..10),
+            new_reward: Rewards(rng.gen_range(1..1000)),
+            new_parsed_blocks: rng.gen_range(1..100),
+        };
+        let result = summary_file.update(update_fields).await;
+        assert!(result.is_ok(), "Failed to update summary file");
+    }
+}
+
+#[tokio::test(flavor = "multi_thread")]
+async fn summary_file_integration() {
+    // this test is mainly for CI, in which, summary file won't exist
+    // if there is a summary file (user env), we don't want to modify the existing
+    // summary file for test
+    if SummaryFile::new(None).await.is_ok() {
+        return;
+    }
+
+    // create summary file
+    let plot_size = ByteSize::gb(1);
+    let summary_file =
+        SummaryFile::new(Some(plot_size)).await.expect("Failed to create summary file");
+
+    // sequential update trial
+    let update_fields = SummaryUpdateFields {
+        is_plotting_finished: true,
+        new_authored_count: 11,
+        new_vote_count: 11,
+        new_reward: Rewards(1001),
+        new_parsed_blocks: 101,
+    };
+    summary_file.update(update_fields).await.expect("Failed to update summary file");
+
+    // create two concurrent tasks, they try to write to summary file 5 times each
+    let task1 = tokio::spawn(update_summary_file_randomly(summary_file.clone()));
+    let task2 = tokio::spawn(update_summary_file_randomly(summary_file.clone()));
+
+    // Wait for both tasks to complete concurrently
+    let (result1, result2) = tokio::join!(task1, task2);
+
+    assert!(result1.is_ok(), "Task 1 encountered an error: {:?}", result1.unwrap_err());
+    assert!(result2.is_ok(), "Task 2 encountered an error: {:?}", result2.unwrap_err());
+
+    // parse the summary after updates
+    summary_file.parse().await.expect("Failed to parse the summary file after updates");
+
+    // Clean up the summary file
+    delete_summary().expect("summary deletion failed");
+}
+
 #[test]
 fn extra_options() {
     let cargo_toml = toml::toml! {