From 217d19815456966c0fd268e31153d6bf8ac57e63 Mon Sep 17 00:00:00 2001 From: Richard de Boer Date: Sun, 12 Apr 2020 00:26:08 +0200 Subject: [PATCH 1/3] Add a "data" section to apps.json, with data files to clean on uninstall These are added to `appid.info` as "dataFiles" or "storageFiles", and can contain wildcards. --- README.md | 25 ++++++++++++++++++++----- bin/sanitycheck.js | 33 +++++++++++++++++++++++++++++++++ js/appinfo.js | 13 +++++++++++++ js/comms.js | 24 +++++++++++++++++++++--- js/index.js | 13 +++++++++++++ js/utils.js | 12 ++++++++++++ 6 files changed, 112 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index ca874ad2f..7a96ad335 100644 --- a/README.md +++ b/README.md @@ -202,6 +202,13 @@ and which gives information about the app for the Launcher. "files:"file1,file2,file3", // added by BangleApps loader on upload - lists all files // that belong to the app so it can be deleted + "dataFiles":"appid.data.json,appid.data?.json" + // added by BangleApps loader on upload - lists files that + // the app might write, so they can be deleted on uninstall + // typically these files are not uploaded, but created by the app + // these can include '*' or '?' wildcards + "storageFiles":" + // same as "dataFiles", except the app handles these as storageFile } ``` @@ -240,16 +247,27 @@ and which gives information about the app for the Launcher. "evaluate":true // if supplied, data isn't quoted into a String before upload // (eg it's evaluated as JS) }, + ] + "data": [ // list of files the app writes to + {"name":"appid.data.json", // filename used in storage + "storageFile":true // if supplied, file is treated as storageFile + }, + {"wildcard":"appid.data.*" // wildcard of filenames used in storage + }, // this is mutually exclusive with using "name" + ], "sortorder" : 0, // optional - choose where in the list this goes. // this should only really be used to put system // stuff at the top - ] } ``` * name, icon and description present the app in the app loader. * tags is used for grouping apps in the library, separate multiple entries by comma. Known tags are `tool`, `system`, `clock`, `game`, `sound`, `gps`, `widget`, `launcher` or empty. * storage is used to identify the app files and how to handle them +* data is used to clean up files when the app is uninstalled + (If the app has settings but no data section, it is assumed settings are + stored in `appid.settings.json`, so there is no need to add a data section + containing only that file) ### `apps.json`: `custom` element @@ -351,19 +369,16 @@ Example `settings.js` E.showMenu(appMenu) }) ``` -In this example the app needs to add both `app.settings.js` and -`app.settings.json` to `apps.json`: +In this example the app needs to add `app.settings.js` to `apps.json`: ```json { "id": "app", ... "storage": [ ... {"name":"app.settings.js","url":"settings.js"}, - {"name":"app.settings.json","content":"{}"} ] }, ``` -That way removing the app also cleans up `app.settings.json`. ## Coding hints diff --git a/bin/sanitycheck.js b/bin/sanitycheck.js index 62b111ae0..fdf15a26b 100755 --- a/bin/sanitycheck.js +++ b/bin/sanitycheck.js @@ -74,6 +74,8 @@ apps.forEach((app,appIdx) => { var fileNames = []; app.storage.forEach((file) => { if (!file.name) ERROR(`App ${app.id} has a file with no name`); + if (file.name.includes('?') || file.name.includes('*')) + ERROR(`App ${app.id} storage file ${file.name} contains wildcards`); if (fileNames.includes(file.name)) ERROR(`App ${app.id} file ${file.name} is a duplicate`); fileNames.push(file.name); @@ -115,6 +117,37 @@ apps.forEach((app,appIdx) => { if (!STORAGE_KEYS.includes(key)) ERROR(`App ${app.id}'s ${file.name} has unknown key ${key}`); } }); + let dataNames = []; + (app.data||[]).forEach((data)=>{ + if (!data.name && !data.wildcard) ERROR(`App ${app.id} has a data file with no name`); + if (dataNames.includes(data.name||data.wildcard)) + ERROR(`App ${app.id} data file ${data.name||data.wildcard} is a duplicate`); + dataNames.push(data.name||data.wildcard) + if ('name' in data && 'wildcard' in data) + ERROR(`App ${app.id} data file ${data.name} has both name and wildcard`); + if (data.name) { + if (data.name.includes('?') || data.name.includes('*')) + ERROR(`App ${app.id} data file name ${data.name} contains wildcards`); + } + if (data.wildcard) { + if (!data.wildcard.includes('?') && !data.wildcard.includes('*')) + ERROR(`App ${app.id} data file wildcard ${data.wildcard} does not actually contains wildcard`); + if (data.wildcard.replace(/\?|\*/g,'') === '') + ERROR(`App ${app.id} data file wildcard ${data.wildcard} does not contain regular characters`); + else if (data.wildcard.replace(/\?|\*/g,'').length < 3) + WARN(`App ${app.id} data file wildcard ${data.wildcard} is very broad`); + else if (!data.wildcard.includes(app.id)) + WARN(`App ${app.id} data file wildcard ${data.wildcard} does not include app ID`); + } + if ('storageFile' in data && typeof data.storageFile !== 'boolean') + ERROR(`App ${app.id} data file ${data.name||data.wildcard} has non-boolean value for "storageFile"`); + for (const key in data) { + if (!['name','wildcard','storageFile'].includes(key)) + ERROR(`App ${app.id} data file ${data.name||data.wildcard} has unknown property "${key}"`); + } + }); + if (fileNames.includes(app.id+".settings.js") && dataNames.length===1 && dataNames[0] === app.id+'.settings.json') + WARN(`App ${app.id} has settings, so does not need to declare data file ${app.id+'.settings.json'}`) //console.log(fileNames); if (isApp && !fileNames.includes(app.id+".app.js")) ERROR(`App ${app.id} has no entrypoint`); if (isApp && !fileNames.includes(app.id+".img")) ERROR(`App ${app.id} has no JS icon`); diff --git a/js/appinfo.js b/js/appinfo.js index f4ab498b1..04c5da893 100644 --- a/js/appinfo.js +++ b/js/appinfo.js @@ -69,6 +69,19 @@ var AppInfo = { var fileList = fileContents.map(storageFile=>storageFile.name); fileList.unshift(appJSONName); // do we want this? makes life easier! json.files = fileList.join(","); + let dataFileList = [], storageFileList = []; + if ('data' in app) { + // add "data" files to appropriate list + app.data.forEach(d=>{ + if (d.storageFile) storageFileList.push(d.name||d.wildcard) + else dataFileList.push(d.name||d.wildcard) + }) + } else if (json.settings) { + // settings but no data files: assume app uses .settings.json file + dataFileList.push(app.id + '.settings.json') + } + if (dataFileList.length) json.dataFiles = dataFileList.join(","); + if (storageFileList.length) json.storageFiles = storageFileList.join(","); fileContents.push({ name : appJSONName, content : JSON.stringify(json) diff --git a/js/comms.js b/js/comms.js index 1f840ada7..1e8250305 100644 --- a/js/comms.js +++ b/js/comms.js @@ -94,10 +94,28 @@ getInstalledApps : () => { }); }, removeApp : app => { // expects an appid.info structure (i.e. with `files`) - if (app.files === '') return Promise.resolve(); // nothing to erase + if (!app.files && !app.dataFiles && !app.storageFiles) return Promise.resolve(); // nothing to erase Progress.show({title:`Removing ${app.name}`,sticky:true}); - var cmds = app.files.split(',').map(file=>{ - return `\x10require("Storage").erase(${toJS(file)});\n`; + let cmds = '\x10const s=require("Storage");\n'; + // remove App files (regular files, exact names only) + cmds += app.files.split(',').map(file => `\x10s.erase(${toJS(file)});\n`).join(""); + // remove Data files (regular files, can use wildcards) + cmds += (app.dataFiles||[]).split(',').map(file => { + const isGlob = (file.includes('*') || file.includes('?')) + if (!isGlob) return `\x10s.erase(${toJS(file)});\n`; + const regex = new RegExp(globToRegex(file)) + return `\x10s.list(${regex}).forEach(f=>s.erase(f));\n`; + }).join(""); + // remove Storage files (storageFiles, can use wildcards) + cmds += (app.storageFiles||[]).split(',').map(file => { + const isGlob = (file.includes('*') || file.includes('?')) + if (!isGlob) return `\x10s.open(${toJS(file)},'r').erase();\n`; + // storageFiles have a chunk number appended to their real name + const regex = globToRegex(file+'\u0001') + // open() doesn't want the chunk number though + let cmd = `\x10s.list(${regex}).forEach(f=>s.open(f.substring(0,f.length-1),'r').erase());\n` + // using a literal \u0001 char fails (not sure why), so escape it + return cmd.replace('\u0001', '\\x01') }).join(""); console.log("removeApp", cmds); return Comms.reset().then(new Promise((resolve,reject) => { diff --git a/js/index.js b/js/index.js index ef9bcb4f1..c48920315 100644 --- a/js/index.js +++ b/js/index.js @@ -349,6 +349,19 @@ function updateApp(app) { .filter(f => f !== app.id + '.info') .filter(f => !app.storage.some(s => s.name === f)) .join(','); + let dataFiles = (remove.dataFiles||'').split(','), + storageFiles = (remove.storageFiles||'').split(',') + if ('data' in app) { + // keep declared (in new version) data files + dataFiles = dataFiles.filter(f => app.data.some(d => (d.name||d.wildcard) === f)) + storageFiles = storageFiles.filter(f => app.data.some(d => (d.name||d.wildcard) === f)) + } + else if (remove.settings || app.settings) { + // app with settings but no data files declared: keep .settings.json + dataFiles = dataFiles.filter(f => f !== (app.id + '.settings.json')) + } + if (dataFiles.length) remove.dataFiles = dataFiles.join(','); + if (storageFiles.length) remove.storageFiles = storageFiles.join(',') return Comms.removeApp(remove); }).then(()=>{ showToast(`Updating ${app.name}...`); diff --git a/js/utils.js b/js/utils.js index d8c1b8063..50d319338 100644 --- a/js/utils.js +++ b/js/utils.js @@ -8,6 +8,18 @@ function escapeHtml(text) { }; return text.replace(/[&<>"']/g, function(m) { return map[m]; }); } +// simple glob to regex conversion, only supports "*" and "?" wildcards +function globToRegex(pattern) { + const ESCAPE = '.*+-?^${}()|[]\\'; + const regex = pattern.replace(/./g, c => { + switch (c) { + case '?': return '.'; + case '*': return '.*'; + default: return ESCAPE.includes(c) ? ('\\' + c) : c; + } + }); + return new RegExp('^'+regex+'$'); +} function htmlToArray(collection) { return [].slice.call(collection); } From 9e0fd91339dc48acd38c4c0687000f199ddb5c23 Mon Sep 17 00:00:00 2001 From: Richard de Boer Date: Wed, 15 Apr 2020 21:30:44 +0200 Subject: [PATCH 2/3] Data files: save all data files as a single string Separate regular and storage files by a semicolon --- README.md | 4 +--- bin/sanitycheck.js | 5 +++-- js/appinfo.js | 39 ++++++++++++++++++++++++++++++++------- js/comms.js | 14 ++++++++------ js/index.js | 18 ++++++++---------- 5 files changed, 52 insertions(+), 28 deletions(-) diff --git a/README.md b/README.md index 7a96ad335..a04ee95b2 100644 --- a/README.md +++ b/README.md @@ -202,13 +202,11 @@ and which gives information about the app for the Launcher. "files:"file1,file2,file3", // added by BangleApps loader on upload - lists all files // that belong to the app so it can be deleted - "dataFiles":"appid.data.json,appid.data?.json" + "data":"appid.data.json,appid.data?.json;appidStorageFile,appidStorageFile*" // added by BangleApps loader on upload - lists files that // the app might write, so they can be deleted on uninstall // typically these files are not uploaded, but created by the app // these can include '*' or '?' wildcards - "storageFiles":" - // same as "dataFiles", except the app handles these as storageFile } ``` diff --git a/bin/sanitycheck.js b/bin/sanitycheck.js index fdf15a26b..197ebf57e 100755 --- a/bin/sanitycheck.js +++ b/bin/sanitycheck.js @@ -39,9 +39,10 @@ try{ const APP_KEYS = [ 'id', 'name', 'shortName', 'version', 'icon', 'description', 'tags', 'type', - 'sortorder', 'readme', 'custom', 'interface', 'storage', 'allow_emulator', + 'sortorder', 'readme', 'custom', 'interface', 'storage', 'data', 'allow_emulator', ]; const STORAGE_KEYS = ['name', 'url', 'content', 'evaluate']; +const DATA_KEYS = ['name', 'wildcard', 'storageFile']; apps.forEach((app,appIdx) => { if (!app.id) ERROR(`App ${appIdx} has no id`); @@ -142,7 +143,7 @@ apps.forEach((app,appIdx) => { if ('storageFile' in data && typeof data.storageFile !== 'boolean') ERROR(`App ${app.id} data file ${data.name||data.wildcard} has non-boolean value for "storageFile"`); for (const key in data) { - if (!['name','wildcard','storageFile'].includes(key)) + if (!DATA_KEYS.includes(key)) ERROR(`App ${app.id} data file ${data.name||data.wildcard} has unknown property "${key}"`); } }); diff --git a/js/appinfo.js b/js/appinfo.js index 04c5da893..413098bc4 100644 --- a/js/appinfo.js +++ b/js/appinfo.js @@ -69,26 +69,51 @@ var AppInfo = { var fileList = fileContents.map(storageFile=>storageFile.name); fileList.unshift(appJSONName); // do we want this? makes life easier! json.files = fileList.join(","); - let dataFileList = [], storageFileList = []; + let data = {dataFiles: [], storageFiles: []}; if ('data' in app) { // add "data" files to appropriate list app.data.forEach(d=>{ - if (d.storageFile) storageFileList.push(d.name||d.wildcard) - else dataFileList.push(d.name||d.wildcard) + if (d.storageFile) data.storageFiles.push(d.name||d.wildcard) + else data.dataFiles.push(d.name||d.wildcard) }) } else if (json.settings) { // settings but no data files: assume app uses .settings.json file - dataFileList.push(app.id + '.settings.json') + data.dataFiles.push(app.id + '.settings.json') } - if (dataFileList.length) json.dataFiles = dataFileList.join(","); - if (storageFileList.length) json.storageFiles = storageFileList.join(","); + const dataString = AppInfo.makeDataString(data) + if (dataString) json.data = dataString fileContents.push({ name : appJSONName, content : JSON.stringify(json) }); resolve(fileContents); }); - } + }, + // (.info).data holds filenames of data: both regular and storageFiles + // These are stored as: (note comma vs semicolons) + // "fil1,file2", "file1,file2;storageFileA,storageFileB" or ";storageFileA" + /** + * Convert appid.info "data" to object with file names/patterns + * Passing in undefined works + * @param data "data" as stored in appid.info + * @returns {{storageFiles:[], dataFiles:[]}} + */ + parseDataString(data) { + data = data || ''; + let [files = [], storage = []] = data.split(';').map(d => d.split(',')) + return {dataFiles: files, storageFiles: storage} + }, + /** + * Convert object with file names/patterns to appid.info "data" string + * Passing in an incomplete object will not work + * @param data {{storageFiles:[], dataFiles:[]}} + * @returns {string} "data" to store in appid.info + */ + makeDataString(data) { + if (!data.dataFiles.length && !data.storageFiles.length) { return '' } + if (!data.storageFiles.length) { return data.dataFiles.join(',') } + return [data.dataFiles.join(','),data.storageFiles.join(',')].join(';') + }, }; if ("undefined"!=typeof module) diff --git a/js/comms.js b/js/comms.js index 1e8250305..736a2b7c7 100644 --- a/js/comms.js +++ b/js/comms.js @@ -94,20 +94,22 @@ getInstalledApps : () => { }); }, removeApp : app => { // expects an appid.info structure (i.e. with `files`) - if (!app.files && !app.dataFiles && !app.storageFiles) return Promise.resolve(); // nothing to erase + if (!app.files && !app.data) return Promise.resolve(); // nothing to erase Progress.show({title:`Removing ${app.name}`,sticky:true}); let cmds = '\x10const s=require("Storage");\n'; - // remove App files (regular files, exact names only) + // remove App files: regular files, exact names only cmds += app.files.split(',').map(file => `\x10s.erase(${toJS(file)});\n`).join(""); - // remove Data files (regular files, can use wildcards) - cmds += (app.dataFiles||[]).split(',').map(file => { + // remove app Data: (dataFiles and storageFiles) + const data = AppInfo.parseDataString(app.data) + // regular files, can use wildcards + cmds += data.dataFiles.map(file => { const isGlob = (file.includes('*') || file.includes('?')) if (!isGlob) return `\x10s.erase(${toJS(file)});\n`; const regex = new RegExp(globToRegex(file)) return `\x10s.list(${regex}).forEach(f=>s.erase(f));\n`; }).join(""); - // remove Storage files (storageFiles, can use wildcards) - cmds += (app.storageFiles||[]).split(',').map(file => { + // storageFiles, can use wildcards + cmds += data.storageFiles.map(file => { const isGlob = (file.includes('*') || file.includes('?')) if (!isGlob) return `\x10s.open(${toJS(file)},'r').erase();\n`; // storageFiles have a chunk number appended to their real name diff --git a/js/index.js b/js/index.js index c48920315..41a43730e 100644 --- a/js/index.js +++ b/js/index.js @@ -349,19 +349,17 @@ function updateApp(app) { .filter(f => f !== app.id + '.info') .filter(f => !app.storage.some(s => s.name === f)) .join(','); - let dataFiles = (remove.dataFiles||'').split(','), - storageFiles = (remove.storageFiles||'').split(',') + let data = AppInfo.parseDataString(remove.data) if ('data' in app) { - // keep declared (in new version) data files - dataFiles = dataFiles.filter(f => app.data.some(d => (d.name||d.wildcard) === f)) - storageFiles = storageFiles.filter(f => app.data.some(d => (d.name||d.wildcard) === f)) - } - else if (remove.settings || app.settings) { + // only remove data files which are no longer declared in new app version + const removeData = (f) => !app.data.some(d => (d.name || d.wildcard)===f) + data.dataFiles = data.dataFiles.filter(removeData) + data.storageFiles = data.storageFiles.filter(removeData) + } else if (remove.settings || app.settings) { // app with settings but no data files declared: keep .settings.json - dataFiles = dataFiles.filter(f => f !== (app.id + '.settings.json')) + data.dataFiles = data.dataFiles.filter(f => f!==(app.id+'.settings.json')) } - if (dataFiles.length) remove.dataFiles = dataFiles.join(','); - if (storageFiles.length) remove.storageFiles = storageFiles.join(',') + remove.data = AppInfo.makeDataString(data) return Comms.removeApp(remove); }).then(()=>{ showToast(`Updating ${app.name}...`); From 9f0adf190019b5dd0bcef5f713c34dabd8ca1c3d Mon Sep 17 00:00:00 2001 From: Richard de Boer Date: Thu, 16 Apr 2020 17:06:25 +0200 Subject: [PATCH 3/3] Data files: remove settings magic, some more sanitychecks --- README.md | 13 +++++---- bin/sanitycheck.js | 69 ++++++++++++++++++++++++++++++++++++++++------ js/appinfo.js | 9 ++---- js/comms.js | 7 ++--- js/index.js | 3 -- 5 files changed, 73 insertions(+), 28 deletions(-) diff --git a/README.md b/README.md index a04ee95b2..a45647daf 100644 --- a/README.md +++ b/README.md @@ -263,9 +263,6 @@ and which gives information about the app for the Launcher. * tags is used for grouping apps in the library, separate multiple entries by comma. Known tags are `tool`, `system`, `clock`, `game`, `sound`, `gps`, `widget`, `launcher` or empty. * storage is used to identify the app files and how to handle them * data is used to clean up files when the app is uninstalled - (If the app has settings but no data section, it is assumed settings are - stored in `appid.settings.json`, so there is no need to add a data section - containing only that file) ### `apps.json`: `custom` element @@ -351,10 +348,10 @@ Example `settings.js` ```js // make sure to enclose the function in parentheses (function(back) { - let settings = require('Storage').readJSON('app.settings.json',1)||{}; + let settings = require('Storage').readJSON('app.json',1)||{}; function save(key, value) { settings[key] = value; - require('Storage').write('app.settings.json',settings); + require('Storage').write('app.json',settings); } const appMenu = { '': {'title': 'App Settings'}, @@ -367,13 +364,17 @@ Example `settings.js` E.showMenu(appMenu) }) ``` -In this example the app needs to add `app.settings.js` to `apps.json`: +In this example the app needs to add `app.settings.js` to `storage` in `apps.json`. +It should also add `app.json` to `data`, to make sure it is cleaned up when the app is uninstalled. ```json { "id": "app", ... "storage": [ ... {"name":"app.settings.js","url":"settings.js"}, + ], + "data": [ + {"name":"app.json"} ] }, ``` diff --git a/bin/sanitycheck.js b/bin/sanitycheck.js index 197ebf57e..51230f6fa 100755 --- a/bin/sanitycheck.js +++ b/bin/sanitycheck.js @@ -43,7 +43,22 @@ const APP_KEYS = [ ]; const STORAGE_KEYS = ['name', 'url', 'content', 'evaluate']; const DATA_KEYS = ['name', 'wildcard', 'storageFile']; +const FORBIDDEN_FILE_NAME_CHARS = /[,;]/; // used as separators in appid.info +function globToRegex(pattern) { + const ESCAPE = '.*+-?^${}()|[]\\'; + const regex = pattern.replace(/./g, c => { + switch (c) { + case '?': return '.'; + case '*': return '.*'; + default: return ESCAPE.includes(c) ? ('\\' + c) : c; + } + }); + return new RegExp('^'+regex+'$'); +} +const isGlob = f => /[?*]/.test(f) +// All storage+data files in all apps: {app:,[file: | data:]} +let allFiles = []; apps.forEach((app,appIdx) => { if (!app.id) ERROR(`App ${appIdx} has no id`); //console.log(`Checking ${app.id}...`); @@ -75,11 +90,13 @@ apps.forEach((app,appIdx) => { var fileNames = []; app.storage.forEach((file) => { if (!file.name) ERROR(`App ${app.id} has a file with no name`); - if (file.name.includes('?') || file.name.includes('*')) - ERROR(`App ${app.id} storage file ${file.name} contains wildcards`); + if (isGlob(file.name)) ERROR(`App ${app.id} storage file ${file.name} contains wildcards`); + let char = file.name.match(FORBIDDEN_FILE_NAME_CHARS) + if (char) ERROR(`App ${app.id} storage file ${file.name} contains invalid character "${char[0]}"`) if (fileNames.includes(file.name)) ERROR(`App ${app.id} file ${file.name} is a duplicate`); fileNames.push(file.name); + allFiles.push({app: app.id, file: file.name}); if (file.url) if (!fs.existsSync(appDir+file.url)) ERROR(`App ${app.id} file ${file.url} doesn't exist`); if (!file.url && !file.content && !app.custom) ERROR(`App ${app.id} file ${file.name} has no contents`); var fileContents = ""; @@ -124,14 +141,13 @@ apps.forEach((app,appIdx) => { if (dataNames.includes(data.name||data.wildcard)) ERROR(`App ${app.id} data file ${data.name||data.wildcard} is a duplicate`); dataNames.push(data.name||data.wildcard) + allFiles.push({app: app.id, data: (data.name||data.wildcard)}); if ('name' in data && 'wildcard' in data) ERROR(`App ${app.id} data file ${data.name} has both name and wildcard`); - if (data.name) { - if (data.name.includes('?') || data.name.includes('*')) - ERROR(`App ${app.id} data file name ${data.name} contains wildcards`); - } + if (isGlob(data.name)) + ERROR(`App ${app.id} data file name ${data.name} contains wildcards`); if (data.wildcard) { - if (!data.wildcard.includes('?') && !data.wildcard.includes('*')) + if (!isGlob(data.wildcard)) ERROR(`App ${app.id} data file wildcard ${data.wildcard} does not actually contains wildcard`); if (data.wildcard.replace(/\?|\*/g,'') === '') ERROR(`App ${app.id} data file wildcard ${data.wildcard} does not contain regular characters`); @@ -140,6 +156,8 @@ apps.forEach((app,appIdx) => { else if (!data.wildcard.includes(app.id)) WARN(`App ${app.id} data file wildcard ${data.wildcard} does not include app ID`); } + let char = (data.name||data.wildcard).match(FORBIDDEN_FILE_NAME_CHARS) + if (char) ERROR(`App ${app.id} data file ${data.name||data.wildcard} contains invalid character "${char[0]}"`) if ('storageFile' in data && typeof data.storageFile !== 'boolean') ERROR(`App ${app.id} data file ${data.name||data.wildcard} has non-boolean value for "storageFile"`); for (const key in data) { @@ -147,8 +165,24 @@ apps.forEach((app,appIdx) => { ERROR(`App ${app.id} data file ${data.name||data.wildcard} has unknown property "${key}"`); } }); - if (fileNames.includes(app.id+".settings.js") && dataNames.length===1 && dataNames[0] === app.id+'.settings.json') - WARN(`App ${app.id} has settings, so does not need to declare data file ${app.id+'.settings.json'}`) + // prefer "appid.json" over "appid.settings.json" (TODO: change to ERROR once all apps comply?) + if (dataNames.includes(app.id+".settings.json") && !dataNames.includes(app.id+".json")) + WARN(`App ${app.id} uses data file ${app.id+'.settings.json'} instead of ${app.id+'.json'}`) + // settings files should be listed under data, not storage (TODO: change to ERROR once all apps comply?) + if (fileNames.includes(app.id+".settings.json")) + WARN(`App ${app.id} uses storage file ${app.id+'.settings.json'} instead of data file`) + if (fileNames.includes(app.id+".json")) + WARN(`App ${app.id} uses storage file ${app.id+'.json'} instead of data file`) + // warn if storage file matches data file of same app + dataNames.forEach(dataName=>{ + const glob = globToRegex(dataName) + fileNames.forEach(fileName=>{ + if (glob.test(fileName)) { + if (isGlob(dataName)) WARN(`App ${app.id} storage file ${fileName} matches data wildcard ${dataName}`) + else WARN(`App ${app.id} storage file ${fileName} is also listed in data`) + } + }) + }) //console.log(fileNames); if (isApp && !fileNames.includes(app.id+".app.js")) ERROR(`App ${app.id} has no entrypoint`); if (isApp && !fileNames.includes(app.id+".img")) ERROR(`App ${app.id} has no JS icon`); @@ -157,3 +191,20 @@ apps.forEach((app,appIdx) => { if (!APP_KEYS.includes(key)) ERROR(`App ${app.id} has unknown key ${key}`); } }); +// Do not allow files from different apps to collide +let fileA +while(fileA=allFiles.pop()) { + const nameA = (fileA.file||fileA.data), + globA = globToRegex(nameA), + typeA = fileA.file?'storage':'data' + allFiles.forEach(fileB => { + const nameB = (fileB.file||fileB.data), + globB = globToRegex(nameB), + typeB = fileB.file?'storage':'data' + if (globA.test(nameB)||globB.test(nameA)) { + if (isGlob(nameA)||isGlob(nameB)) + ERROR(`App ${fileB.app} ${typeB} file ${nameB} matches app ${fileA.app} ${typeB} file ${nameA}`) + else ERROR(`App ${fileB.app} ${typeB} file ${nameB} is also listed as ${typeA} file for app ${fileA.app}`) + } + }) +} diff --git a/js/appinfo.js b/js/appinfo.js index 413098bc4..56b6ff2f8 100644 --- a/js/appinfo.js +++ b/js/appinfo.js @@ -69,19 +69,16 @@ var AppInfo = { var fileList = fileContents.map(storageFile=>storageFile.name); fileList.unshift(appJSONName); // do we want this? makes life easier! json.files = fileList.join(","); - let data = {dataFiles: [], storageFiles: []}; if ('data' in app) { + let data = {dataFiles: [], storageFiles: []}; // add "data" files to appropriate list app.data.forEach(d=>{ if (d.storageFile) data.storageFiles.push(d.name||d.wildcard) else data.dataFiles.push(d.name||d.wildcard) }) - } else if (json.settings) { - // settings but no data files: assume app uses .settings.json file - data.dataFiles.push(app.id + '.settings.json') + const dataString = AppInfo.makeDataString(data) + if (dataString) json.data = dataString } - const dataString = AppInfo.makeDataString(data) - if (dataString) json.data = dataString fileContents.push({ name : appJSONName, content : JSON.stringify(json) diff --git a/js/comms.js b/js/comms.js index 736a2b7c7..b825a06ad 100644 --- a/js/comms.js +++ b/js/comms.js @@ -101,17 +101,16 @@ removeApp : app => { // expects an appid.info structure (i.e. with `files`) cmds += app.files.split(',').map(file => `\x10s.erase(${toJS(file)});\n`).join(""); // remove app Data: (dataFiles and storageFiles) const data = AppInfo.parseDataString(app.data) + const isGlob = f => /[?*]/.test(f) // regular files, can use wildcards cmds += data.dataFiles.map(file => { - const isGlob = (file.includes('*') || file.includes('?')) - if (!isGlob) return `\x10s.erase(${toJS(file)});\n`; + if (!isGlob(file)) return `\x10s.erase(${toJS(file)});\n`; const regex = new RegExp(globToRegex(file)) return `\x10s.list(${regex}).forEach(f=>s.erase(f));\n`; }).join(""); // storageFiles, can use wildcards cmds += data.storageFiles.map(file => { - const isGlob = (file.includes('*') || file.includes('?')) - if (!isGlob) return `\x10s.open(${toJS(file)},'r').erase();\n`; + if (!isGlob(file)) return `\x10s.open(${toJS(file)},'r').erase();\n`; // storageFiles have a chunk number appended to their real name const regex = globToRegex(file+'\u0001') // open() doesn't want the chunk number though diff --git a/js/index.js b/js/index.js index 41a43730e..483dc09c7 100644 --- a/js/index.js +++ b/js/index.js @@ -355,9 +355,6 @@ function updateApp(app) { const removeData = (f) => !app.data.some(d => (d.name || d.wildcard)===f) data.dataFiles = data.dataFiles.filter(removeData) data.storageFiles = data.storageFiles.filter(removeData) - } else if (remove.settings || app.settings) { - // app with settings but no data files declared: keep .settings.json - data.dataFiles = data.dataFiles.filter(f => f!==(app.id+'.settings.json')) } remove.data = AppInfo.makeDataString(data) return Comms.removeApp(remove);