fix: ic-asset::sync() accepts multiple source directories (#355)

* This is a breaking change

fix: ic-asset::sync() accepts multiple source directories

It also now skips filenames and directories that begin with "." (as dfx does),
and reports an error if more than one asset resolves to the same key.

This is to account for asset source directories that may in the future contain
a configuration file .ic-asset.json.  More than one source directory may contain
such a file, but its configuration should only apply to the assets in the
corresponding directory.

CHANGELOG.md

@@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+Breaking change: ic-asset::sync() now synchronizes from multiple source directories.
+
+This is to allow for configuration files located alongside assets in asset source directories.
+
+Also, ic-asset::sync:
+- skips files and directories that begin with a ".", as dfx does when copying assets to an output directory.
+- reports an error if more than one asset file would resolve to the same asset key
+
 ## [0.17.1] - 2022-06-22
 
 [agent-rs/349](https://github.com/dfinity/agent-rs/pull/349) feat: add with_max_response_body_size to ReqwestHttpReplicaV2Transport

e2e/bash/icx-asset.bash

@@ -20,7 +20,7 @@ teardown() {
 
 icx_asset_sync() {
   CANISTER_ID=$(dfx canister id e2e_project_assets)
-  assert_command "$ICX_ASSET" --pem "$HOME"/.config/dfx/identity/default/identity.pem sync "$CANISTER_ID" src/e2e_project_assets/assets
+  assert_command "$ICX_ASSET" --pem "$HOME"/.config/dfx/identity/default/identity.pem sync "$CANISTER_ID" "${@:-src/e2e_project_assets/assets}"
 }
 
 icx_asset_list() {
@@ -104,3 +104,53 @@ icx_asset_list() {
     assert_command_fail dfx canister ${DFX_NO_WALLET:-} call --query e2e_project_assets get '(record{key="/sample-asset.txt";accept_encodings=vec{"arbitrary"}})'
 }
 
+@test "synchronizes multiple directories" {
+    mkdir -p multiple/a
+    mkdir -p multiple/b
+    echo "x_contents" >multiple/a/x
+    echo "y_contents" >multiple/b/y
+
+    icx_asset_sync multiple/a multiple/b
+    # shellcheck disable=SC2086
+    assert_command dfx canister ${DFX_NO_WALLET:-} call --query e2e_project_assets get '(record{key="/x";accept_encodings=vec{"identity"}})'
+    assert_match "x_contents"
+    # shellcheck disable=SC2086
+    assert_command dfx canister ${DFX_NO_WALLET:-} call --query e2e_project_assets get '(record{key="/y";accept_encodings=vec{"identity"}})'
+    assert_match "y_contents"
+}
+
+@test "reports errors about assets with the same key from multiple sources" {
+    mkdir -p multiple/a
+    mkdir -p multiple/b
+    echo "a_duplicate_contents" >multiple/a/duplicate
+    echo "b_duplicate_contents" >multiple/b/duplicate
+
+    assert_command_fail icx_asset_sync multiple/a multiple/b
+    assert_match "Asset with key '/duplicate' defined at multiple/b/duplicate and multiple/a/duplicate"
+}
+
+@test "ignores filenames and directories starting with a dot" {
+    touch src/e2e_project_assets/assets/.not-seen
+    touch src/e2e_project_assets/assets/is-seen
+
+    mkdir -p src/e2e_project_assets/assets/.dir-skipped
+    touch src/e2e_project_assets/assets/.dir-skipped/also-ignored
+
+    mkdir -p src/e2e_project_assets/assets/dir-not-skipped
+    touch src/e2e_project_assets/assets/dir-not-skipped/not-ignored
+
+    icx_asset_sync
+
+    assert_command dfx canister call --query e2e_project_assets get '(record{key="/is-seen";accept_encodings=vec{"identity"}})'
+    assert_command dfx canister call --query e2e_project_assets get '(record{key="/dir-not-skipped/not-ignored";accept_encodings=vec{"identity"}})'
+    assert_command_fail dfx canister call --query e2e_project_assets get '(record{key="/.not-seen";accept_encodings=vec{"identity"}})'
+    assert_command_fail dfx canister call --query e2e_project_assets get '(record{key="/.dir-skipped/also-ignored";accept_encodings=vec{"identity"}})'
+
+    assert_command dfx canister call --query e2e_project_assets list  '(record{})'
+
+    assert_match 'is-seen'
+    assert_match 'not-ignored'
+
+    assert_not_match 'not-seen'
+    assert_not_match 'also-ignored'
+}

ic-asset/src/sync.rs

@@ -7,15 +7,20 @@ use crate::operations::{
     create_new_assets, delete_obsolete_assets, set_encodings, unset_obsolete_encodings,
 };
 use crate::plumbing::{make_project_assets, AssetLocation, ProjectAsset};
+use anyhow::bail;
 use ic_utils::Canister;
 use std::collections::HashMap;
 use std::path::Path;
 use std::time::Duration;
 use walkdir::WalkDir;
 
 /// Sets the contents of the asset canister to the contents of a directory, including deleting old assets.
-pub async fn sync(canister: &Canister<'_>, dir: &Path, timeout: Duration) -> anyhow::Result<()> {
-    let asset_locations = gather_asset_locations(dir);
+pub async fn sync(
+    canister: &Canister<'_>,
+    dirs: &[&Path],
+    timeout: Duration,
+) -> anyhow::Result<()> {
+    let asset_locations = gather_asset_locations(dirs)?;
 
     let canister_call_params = CanisterCallParams { canister, timeout };
 
@@ -43,19 +48,43 @@ pub async fn sync(canister: &Canister<'_>, dir: &Path, timeout: Duration) -> any
     Ok(())
 }
 
-fn gather_asset_locations(dir: &Path) -> Vec<AssetLocation> {
-    WalkDir::new(dir)
-        .into_iter()
-        .filter_map(|r| {
-            r.ok().filter(|entry| entry.file_type().is_file()).map(|e| {
-                let source = e.path().to_path_buf();
-                let relative = source.strip_prefix(dir).expect("cannot strip prefix");
-                let key = String::from("/") + relative.to_string_lossy().as_ref();
+fn filename_starts_with_dot(entry: &walkdir::DirEntry) -> bool {
+    entry
+        .file_name()
+        .to_str()
+        .map(|s| s.starts_with('.'))
+        .unwrap_or(false)
+}
 
-                AssetLocation { source, key }
+fn gather_asset_locations(dirs: &[&Path]) -> anyhow::Result<Vec<AssetLocation>> {
+    let mut asset_descriptors: HashMap<String, AssetLocation> = HashMap::new();
+    for dir in dirs {
+        let asset_locations = WalkDir::new(dir)
+            .into_iter()
+            .filter_entry(|entry| !filename_starts_with_dot(entry))
+            .filter_map(|r| {
+                r.ok().filter(|entry| entry.file_type().is_file()).map(|e| {
+                    let source = e.path().to_path_buf();
+                    let relative = source.strip_prefix(dir).expect("cannot strip prefix");
+                    let key = String::from("/") + relative.to_string_lossy().as_ref();
+
+                    AssetLocation { source, key }
+                })
             })
-        })
-        .collect()
+            .collect::<Vec<_>>();
+        for asset_location in asset_locations {
+            if let Some(already_seen) = asset_descriptors.get(&asset_location.key) {
+                bail!(
+                    "Asset with key '{}' defined at {} and {}",
+                    &asset_location.key,
+                    asset_location.source.display(),
+                    already_seen.source.display()
+                )
+            }
+            asset_descriptors.insert(asset_location.key.clone(), asset_location);
+        }
+    }
+    Ok(asset_descriptors.into_values().collect())
 }
 
 fn assemble_synchronization_operations(

icx-asset/README.md

@@ -3,14 +3,14 @@ A command line tool to manage an asset storage canister.
 
 ## icx-asset sync
 
-Synchronize a directory to an asset canister.
+Synchronize one or more directories to an asset canister.
 
-Usage: `icx-asset sync <directory>`
+Usage: `icx-asset sync <canister id> <source directory>...`
 
 Example:
 ```
-# same asset synchronization as dfx deploy
-$ icx-asset sync src/<project>/assets   
+# same asset synchronization as dfx deploy for a default project, if you've already run dfx build
+$ icx-asset --pem ~/.config/dfx/identity/default/identity.pem sync <canister id> src/prj_assets/assets dist/prj_assets  
 ```
 
 ## icx-asset ls

icx-asset/src/commands/sync.rs

@@ -1,4 +1,5 @@
 use ic_utils::Canister;
+use std::path::Path;
 
 use crate::{support, SyncOpts};
 use std::time::Duration;
@@ -8,6 +9,7 @@ pub(crate) async fn sync(
     timeout: Duration,
     o: &SyncOpts,
 ) -> support::Result {
-    ic_asset::sync(canister, &o.directory, timeout).await?;
+    let dirs: Vec<&Path> = o.directory.iter().map(|d| d.as_path()).collect();
+    ic_asset::sync(canister, &dirs, timeout).await?;
     Ok(())
 }

icx-asset/src/main.rs

@@ -65,9 +65,9 @@ struct SyncOpts {
     #[clap()]
     canister_id: String,
 
-    /// The directory to synchronize
+    /// The directories to synchronize
     #[clap()]
-    directory: PathBuf,
+    directory: Vec<PathBuf>,
 }
 
 #[derive(Parser)]