fix: delete assets that no longer exist in the project (#1540)

breaking changes:
- `list()` now takes an empty record parameter, returns an array of records
- removed `keys()` method

Fixes dfinity/dx-triage#115

CHANGELOG.adoc

@@ -1,3 +1,20 @@
+= 0.7.0-beta.1
+
+== DFX
+
+=== fix: now deletes from the asset canister assets that no longer exist in the project
+
+== Asset Canister
+
+=== Breaking change: change to list() method signature
+
+- now takes a parameter, which is an empty record
+- now returns an array of records
+
+=== Breaking change: removed the keys() method
+
+- use list() instead
+
 = 0.7.0-beta.0
 
 == DFX

docs/design/asset-canister.adoc

@@ -228,6 +228,7 @@ service: {
     content_type: text;
     encodings: vec record {
       content_encoding: text;
+      sha256: opt blob; // sha256 of entire asset encoding, calculated by dfx and passed in SetAssetContentArguments
     };
   }) query;
 

e2e/tests-dfx/assetscanister.bash

@@ -115,20 +115,15 @@ CHERRIES" "$stdout"
     diff src/e2e_project_assets/assets/large-asset.bin curl-output.bin
 }
 
-@test "list() and keys() return asset keys" {
+@test "list() return assets" {
     install_asset assetscanister
 
     dfx_start
     dfx canister create --all
     dfx build
     dfx canister install e2e_project_assets
 
-    assert_command dfx canister call --query e2e_project_assets list
-    assert_match '"/binary/noise.txt"'
-    assert_match '"/text-with-newlines.txt"'
-    assert_match '"/sample-asset.txt"'
-
-    assert_command dfx canister call --query e2e_project_assets keys
+    assert_command dfx canister call --query e2e_project_assets list '(record{})'
     assert_match '"/binary/noise.txt"'
     assert_match '"/text-with-newlines.txt"'
     assert_match '"/sample-asset.txt"'
@@ -165,3 +160,23 @@ CHERRIES" "$stdout"
     assert_command dfx canister call --query e2e_project_assets get '(record{key="/index.js.LICENSE.txt";accept_encodings=vec{"identity"}})'
     assert_match 'content_type = "text/plain"'
 }
+
+@test "deletes assets that are removed from project" {
+    install_asset assetscanister
+
+    dfx_start
+
+    touch src/e2e_project_assets/assets/will-delete-this.txt
+    dfx deploy
+
+    assert_command dfx canister call --query e2e_project_assets get '(record{key="/will-delete-this.txt";accept_encodings=vec{"identity"}})'
+    assert_command dfx canister call --query e2e_project_assets list  '(record{})'
+    assert_match '"/will-delete-this.txt"'
+
+    rm src/e2e_project_assets/assets/will-delete-this.txt
+    dfx deploy
+
+    assert_command_fail dfx canister call --query e2e_project_assets get '(record{key="/will-delete-this.txt";accept_encodings=vec{"identity"}})'
+    assert_command dfx canister call --query e2e_project_assets list  '(record{})'
+    assert_not_match '"/will-delete-this.txt"'
+}

src/dfx/src/lib/installers/assets.rs

@@ -11,13 +11,15 @@ use ic_types::Principal;
 use mime::Mime;
 use openssl::sha::Sha256;
 use serde::Deserialize;
+use std::collections::HashMap;
 use std::path::PathBuf;
 use std::time::Duration;
 use walkdir::WalkDir;
 
 const CREATE_BATCH: &str = "create_batch";
 const CREATE_CHUNK: &str = "create_chunk";
 const COMMIT_BATCH: &str = "commit_batch";
+const LIST: &str = "list";
 const MAX_CHUNK_SIZE: usize = 1_900_000;
 
 #[derive(CandidType, Debug)]
@@ -54,6 +56,22 @@ struct GetResponse {
     content_encoding: String,
 }
 
+#[derive(CandidType, Debug)]
+struct ListAssetsRequest {}
+
+#[derive(CandidType, Debug, Deserialize)]
+struct AssetEncodingDetails {
+    content_encoding: String,
+    sha256: Option<Vec<u8>>,
+}
+
+#[derive(CandidType, Debug, Deserialize)]
+struct AssetDetails {
+    key: String,
+    encodings: Vec<AssetEncodingDetails>,
+    content_type: String,
+}
+
 #[derive(CandidType, Debug)]
 struct CreateAssetArguments {
     key: String,
@@ -259,27 +277,36 @@ async fn commit_batch(
     timeout: Duration,
     batch_id: &Nat,
     chunked_assets: Vec<ChunkedAsset>,
+    current_assets: HashMap<String, AssetDetails>,
 ) -> DfxResult {
-    let operations: Vec<_> = chunked_assets
-        .into_iter()
-        .map(|chunked_asset| {
-            let key = chunked_asset.asset_location.key;
-            vec![
-                BatchOperationKind::DeleteAsset(DeleteAssetArguments { key: key.clone() }),
-                BatchOperationKind::CreateAsset(CreateAssetArguments {
-                    key: key.clone(),
-                    content_type: chunked_asset.media_type.to_string(),
-                }),
-                BatchOperationKind::SetAssetContent(SetAssetContentArguments {
-                    key,
-                    content_encoding: "identity".to_string(),
-                    chunk_ids: chunked_asset.chunk_ids,
-                    sha256: Some(chunked_asset.sha256),
-                }),
-            ]
-        })
-        .flatten()
+    let chunked_assets: HashMap<_, _> = chunked_assets
+        .iter()
+        .map(|e| (e.asset_location.key.clone(), e))
         .collect();
+    let mut operations = vec![];
+    for (key, _) in current_assets {
+        if !chunked_assets.contains_key(&key) {
+            operations.push(BatchOperationKind::DeleteAsset(DeleteAssetArguments {
+                key: key.clone(),
+            }));
+        }
+    }
+    for (key, chunked_asset) in chunked_assets {
+        let mut ops = vec![
+            BatchOperationKind::DeleteAsset(DeleteAssetArguments { key: key.clone() }),
+            BatchOperationKind::CreateAsset(CreateAssetArguments {
+                key: key.clone(),
+                content_type: chunked_asset.media_type.to_string(),
+            }),
+            BatchOperationKind::SetAssetContent(SetAssetContentArguments {
+                key: key.clone(),
+                content_encoding: "identity".to_string(),
+                chunk_ids: chunked_asset.chunk_ids.clone(),
+                sha256: Some(chunked_asset.sha256.clone()),
+            }),
+        ];
+        operations.append(&mut ops);
+    }
     let arg = CommitBatchArguments {
         batch_id,
         operations,
@@ -319,12 +346,22 @@ pub async fn post_install_store_assets(
 
     let canister_id = info.get_canister_id().expect("Could not find canister ID.");
 
+    let current_assets = list_assets(agent, &canister_id, timeout).await?;
+
     let batch_id = create_batch(agent, &canister_id, timeout).await?;
 
     let chunked_assets =
         make_chunked_assets(agent, &canister_id, timeout, &batch_id, asset_locations).await?;
 
-    commit_batch(agent, &canister_id, timeout, &batch_id, chunked_assets).await?;
+    commit_batch(
+        agent,
+        &canister_id,
+        timeout,
+        &batch_id,
+        chunked_assets,
+        current_assets,
+    )
+    .await?;
 
     Ok(())
 }
@@ -340,3 +377,24 @@ async fn create_batch(agent: &Agent, canister_id: &Principal, timeout: Duration)
     let create_batch_response = candid::Decode!(&response, CreateBatchResponse)?;
     Ok(create_batch_response.batch_id)
 }
+
+async fn list_assets(
+    agent: &Agent,
+    canister_id: &Principal,
+    timeout: Duration,
+) -> DfxResult<HashMap<String, AssetDetails>> {
+    let args = ListAssetsRequest {};
+    let response = agent
+        .update(&canister_id, LIST)
+        .with_arg(candid::Encode!(&args)?)
+        .expire_after(timeout)
+        .call_and_wait(waiter_with_timeout(timeout))
+        .await?;
+
+    let assets: HashMap<_, _> = candid::Decode!(&response, Vec<AssetDetails>)?
+        .into_iter()
+        .map(|d| (d.key.clone(), d))
+        .collect();
+
+    Ok(assets)
+}

src/distributed/assetstorage/Asset.mo

@@ -43,6 +43,8 @@ module {
       encodings.delete(encodingType)
     };
 
+    public func encodingEntries() : Iter.Iter<(Text,AssetEncoding)> = encodings.entries();
+
     public func toStableAsset() : StableAsset = {
       contentType = contentType;
       encodings = Iter.toArray(encodings.entries());

src/distributed/assetstorage/Main.mo

@@ -97,22 +97,30 @@ shared ({caller = creator}) actor class () {
     };
   };
 
-  func listKeys(): [T.Path] {
-    let iter = Iter.map<(Text, A.Asset), T.Path>(assets.entries(), func (key, _) = key);
-    Iter.toArray(iter)
+  func entryToAssetDetails((key: T.Key, asset: A.Asset)) : T.AssetDetails {
+    let assetEncodings = Iter.toArray(
+      Iter.map<(Text, A.AssetEncoding), T.AssetEncodingDetails>(
+        asset.encodingEntries(), entryToAssetEncodingDetails
+      )
+    );
+
+    {
+      key = key;
+      content_type = asset.contentType;
+      encodings = assetEncodings;
+    }
   };
 
-  // deprecated: the signature of this method will change, to take an empty record as
-  // a parameter and to return an array of records.
-  // For now, call keys() instead
-  public query func list() : async [T.Path] {
-    listKeys()
+  func entryToAssetEncodingDetails((name: Text, assetEncoding: A.AssetEncoding)) : T.AssetEncodingDetails {
+    {
+      content_encoding = assetEncoding.contentEncoding;
+      sha256 = assetEncoding.sha256;
+    }
   };
 
-  // Returns an array of the keys of all assets contained in the asset canister.
-  // This method will be deprecated after the signature of list() changes.
-  public query func keys() : async [T.Path] {
-    listKeys()
+  public query func list(arg:{}) : async [T.AssetDetails] {
+    let iter = Iter.map<(Text, A.Asset), T.AssetDetails>(assets.entries(), entryToAssetDetails);
+    Iter.toArray(iter)
   };
 
   func isSafe(caller: Principal) : Bool {

src/distributed/assetstorage/Types.mo

@@ -7,6 +7,17 @@ module Types {
   public type Key = Text;
   public type Time = Int;
 
+  public type AssetEncodingDetails = {
+    content_encoding: Text;
+    sha256: ?Blob;
+  };
+
+  public type AssetDetails = {
+    key: Key;
+    content_type: Text;
+    encodings: [AssetEncodingDetails];
+  };
+
   public type CreateAssetArguments = {
     key: Key;
     content_type: Text;