feat: replace cache with createAsset. Deprecate cache

Discourage use of Storage.cache, and provide asset factory storage.createAsset.

Towards scratchfoundation/scratch-vm#1577

Author: Ray Schamp

src/Asset.js

@@ -13,6 +13,7 @@ if (typeof TextDecoder === 'undefined' || typeof TextEncoder === 'undefined') {
 }
 
 const base64js = require('base64-js');
+const md5 = require('js-md5');
 
 const memoizedToString = (function () {
     const strings = {};
@@ -31,21 +32,22 @@ class Asset {
      * @param {string} assetId - The ID of this asset.
      * @param {DataFormat} [dataFormat] - The format of the data (WAV, PNG, etc.); required iff `data` is present.
      * @param {Buffer} [data] - The in-memory data for this asset; optional.
+     * @param {bool} [generateId] - Whether to create id from an md5 hash of data
      */
-    constructor (assetType, assetId, dataFormat, data) {
+    constructor (assetType, assetId, dataFormat, data, generateId) {
         /** @type {AssetType} */
         this.assetType = assetType;
 
         /** @type {string} */
         this.assetId = assetId;
 
-        this.setData(data, dataFormat || assetType.runtimeFormat);
+        this.setData(data, dataFormat || assetType.runtimeFormat, generateId);
 
         /** @type {Asset[]} */
         this.dependencies = [];
     }
 
-    setData (data, dataFormat) {
+    setData (data, dataFormat, generateId) {
         if (data && !dataFormat) {
             throw new Error('Data provided without specifying its format');
         }
@@ -55,6 +57,8 @@ class Asset {
 
         /** @type {Buffer} */
         this.data = data;
+
+        if (generateId) this.assetId = md5(data);
     }
 
     /**
@@ -69,10 +73,11 @@ class Asset {
      * Same as `setData` but encodes text first.
      * @param {string} data - the text data to encode and store.
      * @param {DataFormat} dataFormat - the format of the data (DataFormat.SVG for example).
+     * @param {bool} generateId - after setting data, set the id to an md5 of the data?
      */
-    encodeTextData (data, dataFormat) {
+    encodeTextData (data, dataFormat, generateId) {
         const encoder = new _TextEncoder();
-        this.setData(encoder.encode(data), dataFormat);
+        this.setData(encoder.encode(data), dataFormat, generateId);
     }
 
     /**

src/BuiltinHelper.js

@@ -63,7 +63,7 @@ class BuiltinHelper extends Helper {
         this.assets = {};
 
         BuiltinAssets.forEach(assetRecord => {
-            assetRecord.id = this.store(assetRecord.type, assetRecord.format, assetRecord.data, assetRecord.id);
+            assetRecord.id = this._store(assetRecord.type, assetRecord.format, assetRecord.data, assetRecord.id);
         });
     }
 
@@ -109,14 +109,28 @@ class BuiltinHelper extends Helper {
     }
 
     /**
-     * Cache an asset for future lookups by ID.
+     * Deprecated external API for _store
+     * @deprecated Not for external use. Create assets and keep track of them outside of the storage instance.
      * @param {AssetType} assetType - The type of the asset to cache.
      * @param {DataFormat} dataFormat - The dataFormat of the data for the cached asset.
      * @param {Buffer} data - The data for the cached asset.
      * @param {(string|number)} id - The id for the cached asset.
      * @returns {string} The calculated id of the cached asset, or the supplied id if the asset is mutable.
      */
     store (assetType, dataFormat, data, id) {
+        log.warn('Deprecation: use Storage.createAsset. BuiltinHelper is for internal use only.');
+        return this._store(assetType, dataFormat, data, id);
+    }
+
+    /**
+     * Cache an asset for future lookups by ID.
+     * @param {AssetType} assetType - The type of the asset to cache.
+     * @param {DataFormat} dataFormat - The dataFormat of the data for the cached asset.
+     * @param {Buffer} data - The data for the cached asset.
+     * @param {(string|number)} id - The id for the cached asset.
+     * @returns {string} The calculated id of the cached asset, or the supplied id if the asset is mutable.
+     */
+    _store (assetType, dataFormat, data, id) {
         if (!dataFormat) throw new Error('Data cached without specifying its format');
         if (id !== '' && id !== null && typeof id !== 'undefined') {
             if (this.assets.hasOwnProperty(id) && assetType.immutable) return id;

src/ScratchStorage.js

@@ -68,15 +68,30 @@ class ScratchStorage {
     }
 
     /**
-     * Cache an asset for future lookups by ID.
+     * Deprecated API for caching built-in assets. Use createAsset.
      * @param {AssetType} assetType - The type of the asset to cache.
      * @param {DataFormat} dataFormat - The dataFormat of the data for the cached asset.
      * @param {Buffer} data - The data for the cached asset.
      * @param {string} id - The id for the cached asset.
      * @returns {string} The calculated id of the cached asset, or the supplied id if the asset is mutable.
      */
     cache (assetType, dataFormat, data, id) {
-        return this.builtinHelper.store(assetType, dataFormat, data, id);
+        log.warn('Deprecation: Storage.cache is deprecated. Use Storage.createAsset, and store assets externally.');
+        return this.builtinHelper._store(assetType, dataFormat, data, id);
+    }
+
+    /**
+     * Construct an Asset, and optionally generate an md5 hash of its data to create an id
+     * @param {AssetType} assetType - The type of the asset to cache.
+     * @param {DataFormat} dataFormat - The dataFormat of the data for the cached asset.
+     * @param {Buffer} data - The data for the cached asset.
+     * @param {string} [id] - The id for the cached asset.
+     * @param {bool} [generateId] - flag to set id to an md5 hash of data if `id` isn't supplied
+     * @returns {Asset} generated Asset with `id` attribute set if not supplied
+     */
+    createAsset (assetType, dataFormat, data, id, generateId) {
+        if (!dataFormat) throw new Error('Tried to create asset without a dataFormat');
+        return new _Asset(assetType, id, dataFormat, data, generateId);
     }
 
     /**
@@ -154,7 +169,7 @@ class ScratchStorage {
                                 } else {
                                     // TODO? this.localHelper.cache(assetType, assetId, asset);
                                     if (helper !== this.builtinHelper && assetType.immutable) {
-                                        asset.assetId = this.builtinHelper.cache(
+                                        asset.assetId = this.builtinHelper._store(
                                             assetType,
                                             asset.dataFormat,
                                             asset.data,
@@ -198,7 +213,7 @@ class ScratchStorage {
             (resolve, reject) =>
                 this.webHelper.store(assetType, dataFormat, data, assetId)
                     .then(body => {
-                        this.builtinHelper.store(assetType, dataFormat, data, body.id);
+                        this.builtinHelper._store(assetType, dataFormat, data, body.id);
                         return resolve(body);
                     })
                     .catch(error => reject(error))