From de1da33497c021ceb406a6aa15642d4860fd9fd8 Mon Sep 17 00:00:00 2001 From: Juri Leino Date: Mon, 20 Nov 2023 22:55:13 +0100 Subject: [PATCH] fix(upload): align output, report problems fixes #114 When a upload operation fails in part or in total a non-zero exit code accompanied by a meaningful message will be added. Created collections and resources are now actually counted. With `--verbose` errors on resource and collection level are shown. In general output of upload operations is now better aligned and also colored if the terminal is capable of that. Improved utitily modules: - errors.js - recognize more network errors - recognize filesystem errors - recognize XMLRPC faults - export all error categorization helpers - add `formatErrorMessage` - connection.js - new helper `getServerUrl` returns db server URL - new helper `getUserInfo` returns user connecting to DB New utility modules: - account.js - `getAccountInfo`: turns raw XMLRPC db user data into AccountInfo - message.js - `logSucces`: helper to align operation success messages - `logFailure`: helper to align operation failure messages Adapt and improve tests. --- commands/upload.js | 144 +++++++++++++++++++++++++++--------------- spec/tests/list.js | 2 +- spec/tests/upload.js | 127 ++++++++++++++++++++++++++++--------- utility/account.js | 38 +++++++++++ utility/connection.js | 37 +++++++++++ utility/errors.js | 55 ++++++++++++++-- utility/message.js | 28 ++++++++ 7 files changed, 345 insertions(+), 86 deletions(-) create mode 100644 utility/account.js create mode 100644 utility/message.js diff --git a/commands/upload.js b/commands/upload.js index 526e839..fbac4fa 100755 --- a/commands/upload.js +++ b/commands/upload.js @@ -1,8 +1,27 @@ -import { connect, getMimeType } from '@existdb/node-exist' import { statSync, readFileSync, existsSync } from 'node:fs' import { resolve } from 'node:path' import Bottleneck from 'bottleneck' import fg from 'fast-glob' +import chalk from 'chalk' +import { connect, getMimeType } from '@existdb/node-exist' + +import { logFailure, logSuccess } from '../utility/message.js' +import { formatErrorMessage, isNetworkError } from '../utility/errors.js' +import { getServerUrl, getUserInfo } from '../utility/connection.js' + +/** + * @typedef { import("../utility/account.js").AccountInfo } AccountInfo + */ + +/** + * @typedef {0|1|2|9} ExitCode + */ + +/** + * @typedef {Object} CollectionCreateResult + * @prop {Boolean} exists the collection exists + * @prop {Boolean} created the collection was created + */ const stringList = { type: 'string', @@ -13,24 +32,20 @@ const stringList = { : values.reduce((values, value) => values.concat(value.split(',').map((value) => value.trim())), []) } -async function getUserInfo (db) { - const { user } = db.client.options.basic_auth - return await db.users.getUserInfo(user) -} - /** * Upload a single resource into an existdb instance * @param {String} path * @param {String} root * @param {String} baseCollection + * @returns {Promise} upload success */ -async function uploadResource (db, verbose, path, root, baseCollection) { +async function uploadResource (db, verbose, path, root, baseCollection, targetName) { try { const localFilePath = resolve(root, path) - const remoteFilePath = baseCollection + '/' + path + const remoteFilename = targetName || path + const remoteFilePath = baseCollection + '/' + remoteFilename const fileContents = readFileSync(localFilePath) const fileHandle = await db.documents.upload(fileContents) - const options = {} if (!getMimeType(path)) { console.log('fallback mimetype for', path) @@ -38,11 +53,11 @@ async function uploadResource (db, verbose, path, root, baseCollection) { } await db.documents.parseLocal(fileHandle, remoteFilePath, options) if (verbose) { - console.log(`✔︎ ${path} uploaded`) + logSuccess(`${chalk.white(path)} uploaded`) } return true } catch (e) { - handleError(e, path) + if (verbose) { handleError(e, path) } return false } } @@ -51,6 +66,7 @@ async function uploadResource (db, verbose, path, root, baseCollection) { * Create a collection in an existdb instance * @param {String} collection * @param {String} baseCollection + * @returns {Promise} upload success */ async function createCollection (db, verbose, collection, baseCollection) { const absCollection = baseCollection + @@ -58,36 +74,41 @@ async function createCollection (db, verbose, collection, baseCollection) { collection try { + if (await db.collections.existsAndCanOpen(absCollection)) { + if (verbose) { + logSuccess(`${chalk.white(absCollection)} exists`) + } + return { exists: true, created: false } + } await db.collections.create(absCollection) if (verbose) { - console.log(`✔︎ ${absCollection} created`) + logSuccess(`${chalk.white(absCollection)} created`) } - return true + return { exists: true, created: true } } catch (e) { - handleError(e, absCollection) - return false + if (verbose) { handleError(e, absCollection) } + return { exists: false, created: false } } } /** * Handle errors uploading a resource or creating a collection - * @param {Error} e + * @param {Error} error * @param {String} path */ -function handleError (e, path) { - const message = e.faultString ? e.faultString : e.message - console.error(`✘ ${path} could not be created! Reason: ${message}`) - if (e.code === 'ECONNRESET' || e.code === 'ECONNREFUSED') { - throw e +function handleError (error, path) { + logFailure(`${chalk.white(path)} ${formatErrorMessage(error)}`) + if (isNetworkError(error)) { + throw error } } /** - * - * @param {String} source - * @param {String} target + * Upload a single file or an entire directory tree to a db into a target collection + * @param {String} source filesustem path + * @param {String} target target collection * @param {{pattern: [String], threads: Number, mintime: Number}} options - * @returns + * @returns {Promise} exit code */ async function uploadFileOrFolder (db, source, target, options) { // read parameters @@ -97,15 +118,17 @@ async function uploadFileOrFolder (db, source, target, options) { const rootStat = statSync(source) if (options.verbose) { - console.log('Uploading:', source, 'to', target) - console.log('Server:', (db.client.isSecure ? 'https' : 'http') + '://' + db.client.options.host + ':' + db.client.options.port) - console.log('User:', db.client.options.basic_auth.user) - if (options.include.length > 1 || options.include[0] !== '**') { - console.log('Include:\n', ...options.include, '\n') + console.log(`Uploading ${chalk.white(resolve(source))}`) + console.log(`To ${chalk.white(target)}`) + console.log(`On ${chalk.white(getServerUrl(db))}`) + console.log(`As ${chalk.white(db.client.options.basic_auth.user)}`) + if (options.include.length) { + console.log(`Include ${chalk.green(options.include)}`) } if (options.exclude.length) { - console.log('Exclude:\n', ...options.exclude, '\n') + console.log(`Exclude ${chalk.yellow(options.exclude)}`) } + console.log('') } if (rootStat.isFile()) { @@ -120,12 +143,18 @@ async function uploadFileOrFolder (db, source, target, options) { return 0 } // ensure target collection exists - const collectionSuccess = await createCollection(db, options.verbose, '', target) + const targetExistsAndCanOpen = await db.collections.existsAndCanOpen(target) + if (!targetExistsAndCanOpen) { + console.error(`Target ${target} must be an existing collection.`) + return 1 + } const uploadSuccess = await uploadResource(db, options.verbose, name, dir, target) - if (collectionSuccess && uploadSuccess) { - console.log(`uploaded ${source} in ${Date.now() - start}ms`) + if (uploadSuccess) { + const time = Date.now() - start + console.log(`Uploaded ${chalk.white(resolve(source))} to ${chalk.white(target)} in ${chalk.yellow(time + 'ms')}`) return 0 } + console.error(`Upload of ${resolve(source)} failed.`) return 1 } @@ -133,12 +162,11 @@ async function uploadFileOrFolder (db, source, target, options) { const collectionGlob = Object.assign({ onlyDirectories: true }, globbingOptions) const resourceGlob = Object.assign({ onlyFile: true }, globbingOptions) - // console.log(options.include) const collections = await fg(options.include, collectionGlob) const resources = await fg(options.include, resourceGlob) if (resources.length === 0 && collections.length === 0) { - console.error('nothing matched') + console.error(chalk.yellow('Nothing matched')) return 9 } @@ -166,15 +194,18 @@ async function uploadFileOrFolder (db, source, target, options) { if (options.dryRun) { if (options.applyXconf && xConf.length) { - console.log('\nIndex configurations:\n') + console.log('Index configurations:') console.log(xConf.join('\n')) + console.log('') } if (collections.length) { - console.log('\nCollections:\n') + console.log('Collections:') console.log(collections.join('\n')) + console.log('') } - console.log('\nResources:\n') + console.log('Resources:') console.log(resources.join('\n')) + console.log('') return 0 } @@ -190,23 +221,35 @@ async function uploadFileOrFolder (db, source, target, options) { const uploadResourceThrottled = limiter.wrap(uploadResource.bind(null, db, options.verbose)) // create all collections upfront - await Promise.all(collections.map(c => createCollectionThrottled(c, target))) + const collectionsUploadResults = await Promise.all( + collections.map( + c => createCollectionThrottled(c, target))) // requires user to be a member of DBA // apply collection configurations if (options.applyXconf) { - const promises = [] for (const cpath of confCols.keys()) { createCollectionThrottled(cpath, '/db/system/config') } - await Promise.all(promises) - await Promise.all(xConf.map(conf => uploadResourceThrottled(conf, root, '/db/system/config' + target))) + await Promise.all( + xConf.map( + conf => uploadResourceThrottled(conf, root, '/db/system/config' + target))) } - await Promise.all(resources.map(r => uploadResourceThrottled(r, root, target))) + const resourceUploadResults = await Promise.all( + resources.map( + resourcePath => uploadResourceThrottled(resourcePath, root, target))) + const createdCollections = collectionsUploadResults.filter(r => r.created).length + const uploadedResources = resourceUploadResults.filter(r => r).length const time = Date.now() - start - console.log(`created ${collections.length} collections and uploaded ${resources.length} resources in ${time}ms`) + console.log(`Created ${chalk.white(createdCollections + ' collections')} and uploaded ${chalk.white(uploadedResources + ' resources')} in ${chalk.yellow(time + 'ms')}`) + + if (collectionsUploadResults.filter(r => !r.exists).length || + resourceUploadResults.filter(r => !r).length) { + console.error(chalk.redBright('Upload finished with errors!')) + return 2 + } return 0 } @@ -306,10 +349,11 @@ export async function handler (argv) { throw Error('To apply collection configurations you must be member of the dba group.') } - const existsAndCanOpenCollection = await db.collections.existsAndCanOpen(target) - if (argv.verbose) { - console.log('target exists:', existsAndCanOpenCollection) + try { + const code = await uploadFileOrFolder(db, source, target, argv) + process.exit(code) + } catch (e) { + handleError(e, target) + // process.exit(1) } - - return await uploadFileOrFolder(db, source, target, argv) } diff --git a/spec/tests/list.js b/spec/tests/list.js index bbbc90d..631eb46 100644 --- a/spec/tests/list.js +++ b/spec/tests/list.js @@ -296,9 +296,9 @@ test('with fixtures uploaded', async (t) => { /db/list-test/tests /db/list-test/tests/cli.js /db/list-test/tests/info.js -/db/list-test/tests/upload.js /db/list-test/tests/configuration.js /db/list-test/tests/exec.js +/db/list-test/tests/upload.js /db/list-test/tests/rm.js /db/list-test/tests/get.js /db/list-test/tests/list.js diff --git a/spec/tests/upload.js b/spec/tests/upload.js index 576bc72..0c722c0 100644 --- a/spec/tests/upload.js +++ b/spec/tests/upload.js @@ -1,41 +1,106 @@ import { test } from 'tape' import { run, asAdmin } from '../test.js' -test("calling 'xst up modules/test.xq /db/tmp' as admin", async (t) => { - const { stderr, stdout } = await run('xst', ['up', 'spec/fixtures/test.xq', '/db/tmp'], asAdmin) - if (stderr) t.fail(stderr) - t.ok(stdout, stdout) - t.end() -}) +const testCollection = '/db/upload-test' -test("calling 'xst up modules /db/tmp' as admin", async (t) => { - const { stderr, stdout } = await run('xst', ['up', 'spec/fixtures', '/db/tmp'], asAdmin) - if (stderr) t.fail(stderr) - t.ok(stdout, stdout) - t.end() -}) +async function removeRemoteCollection (t) { + const { stderr } = await run('xst', ['rm', '-rf', testCollection], asAdmin) + if (stderr) { return console.error(stderr) } +} -test('upload dotfile', async (t) => { - const { stderr, stdout } = await run('xst', ['up', '-D', 'spec/fixtures/.env', '/db/tmp'], asAdmin) - if (stderr) t.fail(stderr) - t.ok(stdout, stdout) - t.end() -}) +test('uploading files and folders', function (t) { + t.test(`single file into non-existing collection ${testCollection}' as admin`, async (st) => { + const { stderr, stdout } = await run('xst', ['up', 'spec/fixtures/test.xq', testCollection], asAdmin) + st.notOk(stdout, stdout) + st.equal(stderr, `Target ${testCollection} must be an existing collection.\n`, stderr) + st.end() + }) -test('error on upload with more than two positional arguments', async (t) => { - const { stderr, stdout } = await run('xst', ['up', 'spec/fixtures/test-app.xar', 'spec/fixtures/test-lib.xar', '/db/tmp'], asAdmin) - t.notOk(stdout, stdout) - t.equal(stderr, 'More than two positional arguments provided.\nDid you use a globbing character, like * or ? for the source argument?\nUse --include and/or --exclude instead.\n') - t.end() -}) + t.test(`calling 'xst up modules spec/fixtures ${testCollection}' as admin`, async (st) => { + const { stderr, stdout } = await run('xst', ['up', 'spec/fixtures', testCollection], asAdmin) + if (stderr) { + st.fail(stderr) + st.end() + return + } + st.ok(stdout, stdout) + st.end() + }) + + t.test(`single, new file to ${testCollection}' as guest`, async (st) => { + const { stderr, stdout } = await run('xst', ['up', 'spec/test.js', testCollection]) + st.ok(/Upload of .+?spec\/test\.js failed\./.test(stderr), stderr) + st.notOk(stdout, stdout) + st.end() + }) + + t.test(`verbose: single, new file to ${testCollection}' as guest`, async (st) => { + const { stderr, stdout } = await run('xst', ['up', '-v', 'spec/test.js', testCollection]) + const lines = stderr.split('\n') + st.equal(lines[0], '✘ test.js Error: Write permission is not granted on the Collection.', lines[0]) + st.ok(/Upload of .+?spec\/test\.js failed\./.test(lines[1]), lines[1]) + st.ok(stdout, 'additional info is displayed') + st.end() + }) + + t.test(`calling 'xst up modules/test.xq ${testCollection}' as admin`, async (st) => { + const { stderr, stdout } = await run('xst', ['up', 'spec/fixtures/test.xq', testCollection], asAdmin) + if (stderr) { + st.fail(stderr) + st.end() + return + } + st.ok(stdout, stdout) + st.end() + }) + + t.test(`single, existing file to ${testCollection}' as guest`, async (st) => { + const { stderr, stdout } = await run('xst', ['up', 'spec/fixtures/test.xq', testCollection]) + st.ok(/Upload of .+?spec\/fixtures\/test\.xq failed\./.test(stderr)) + st.notOk(stdout, stdout) + st.end() + }) + + t.test(`verbose: single, existing file to ${testCollection}' as guest`, async (st) => { + const { stderr, stdout } = await run('xst', ['up', '-v', 'spec/fixtures/test.xq', testCollection]) + const lines = stderr.split('\n') + st.equal(lines[0], '✘ test.xq Error: A resource with the same name already exists in the target collection \'/db/upload-test\', and you do not have write access on that resource.') + st.ok(/Upload of .+?spec\/fixtures\/test\.xq failed\./.test(lines[1])) + st.ok(stdout, 'additional info is displayed') + st.end() + }) + + t.test('upload dotfile', async (st) => { + const { stderr, stdout } = await run('xst', ['up', '-D', 'spec/fixtures/.env', testCollection], asAdmin) + if (stderr) { + st.fail(stderr) + st.end() + return + } + st.ok(stdout, stdout) + st.end() + }) + + t.test('error on upload with more than two positional arguments', async (st) => { + const { stderr, stdout } = await run('xst', ['up', 'spec/fixtures/test-app.xar', 'spec/fixtures/test-lib.xar', testCollection], asAdmin) + st.notOk(stdout, stdout) + st.equal(stderr, 'More than two positional arguments provided.\nDid you use a globbing character, like * or ? for the source argument?\nUse --include and/or --exclude instead.\n') + st.end() + }) + + t.test(`calling 'xst up modules ${testCollection}' as guest`, async (st) => { + const { stderr, stdout } = await run('xst', ['up', 'modules', testCollection]) + st.ok(stdout.startsWith('Created 0 collections and uploaded 0 resources in'), stdout) -test.skip("calling 'xst up modules/test.xq /db/foo' as guest", async (t) => { - const { stderr, stdout } = await run('xst', ['up', 'modules', '/db/foo']) - if (stdout) t.fail(stdout) + const lines = stderr.split('\n') + const lineCount = lines.length + for (let i = lineCount - 3; i >= 0; i--) { + st.ok(/✘ [^ ]+ Error: Write permission is not granted on the Collection./.test(lines[i]), lines[i]) + } - const lines = stderr.split('\n') + st.equal(lines[lineCount - 2], 'Upload finished with errors!', lines[lineCount - 2]) + st.end() + }) - t.equal(lines[0], 'XPathException:') - t.ok(lines[1].startsWith('exerr:ERROR Permission to retrieve permissions is denied for user \'guest\' on \'/db/system/security\':')) - t.end() + t.teardown(removeRemoteCollection) }) diff --git a/utility/account.js b/utility/account.js new file mode 100644 index 0000000..4b222b8 --- /dev/null +++ b/utility/account.js @@ -0,0 +1,38 @@ +/** + * @typedef { import("@existdb/node-exist").NodeExist } NodeExist + */ + +/** + * @typedef {Object} AccountInfo + * @prop {Number} uid internal user id + * @prop {String} name user name + * @prop {String[]} groups the groups this user is a mebmer of + * @prop {Number} umask default permissions when creating new resources and collections + * @prop {Object} metadata additional user account information as URI-value pairs + * @prop {Boolean} enabled is this user account enabled? + * @prop {{id:number, name:string, realmId: string}} defaultGroup this user's default group info + */ + +/** + * get the user account information for a specific user + * @param {NodeExist} db client connection + * @param {String} userName user name + * @returns {Promise} user info object + */ +export async function getAccountInfo (db, userName) { + const rawUserInfo = await db.users.getUserInfo(userName) + const { uid, name, groups, umask, metadata } = rawUserInfo + return { + uid, + name, + groups, + umask, + metadata, + enabled: Boolean(rawUserInfo.enabled), + defaultGroup: { + id: rawUserInfo['default-group-id'], + name: rawUserInfo['default-group-name'], + realmId: rawUserInfo['default-group-realmId'] + } + } +} diff --git a/utility/connection.js b/utility/connection.js index 38930d5..bbfe538 100644 --- a/utility/connection.js +++ b/utility/connection.js @@ -10,6 +10,14 @@ * must be set for EXISTDB_USER to take effect */ import { readOptionsFromEnv } from '@existdb/node-exist' +import { getAccountInfo } from '../utility/account.js' + +/** + * @typedef { import("@existdb/node-exist").NodeExist } NodeExist + */ +/** + * @typedef { import("./account.js").AccountInfo } AccountInfo + */ export function readConnection (argv) { if (argv.connectionOptions) { @@ -19,3 +27,32 @@ export function readConnection (argv) { argv.connectionOptions = connectionOptions return argv } + +/** + * get server Address + * @param {NodeExist} db client + * @returns {String} server address + */ +export function getServerUrl (db) { + const { isSecure, options } = db.client + + const protocol = isSecure ? 'https:' : 'http:' + const isStdPort = + (isSecure && options.port === 443) || + (!isSecure && options.port === 80) + + if (isStdPort) { + return `${protocol}//${options.host}` + } + return `${protocol}//${options.host}:${options.port}` +} + +/** + * get the user account information that will be used to connect to the db + * @param {NodeExist} db client + * @returns {Promise} user info object + */ +export async function getUserInfo (db) { + const { user } = db.client.options.basic_auth + return await getAccountInfo(db, user) +} diff --git a/utility/errors.js b/utility/errors.js index bc4da34..615996e 100644 --- a/utility/errors.js +++ b/utility/errors.js @@ -1,21 +1,68 @@ const NETWORK_ERRORS = [ 'EPROTO', 'ECONNREFUSED', - 'ECONNRESET' + 'ECONNRESET', + 'ENOTFOUND', + 'ETIMEDOUT', + 'ECONNABORTED', + 'EHOSTUNREACH' ] -function isNetworkError (error) { +export function isNetworkError (error) { return NETWORK_ERRORS.includes(error.code) } -function isXMLRPCFault (error) { +const FS_ERRORS = [ + 'ENOENT', + 'EISDIR', + 'ENOTDIR', + 'EACCES', + 'EEXIST', + 'EPERM' +] + +export function isFilesystemError (error) { + return FS_ERRORS.includes(error.code) +} + +const XPATH_EXCEPTION = 'org.exist.xquery.XPathException: ' + +export function isXMLRPCFault (error) { return Boolean(error.faultString) } -function isYargsError (error) { +const XMLRPC_FAULT = 'org.exist.xmlrpc.RpcConnection: ' + +export function isYargsError (error) { return error.name === 'YError' } +export function formatErrorMessage (error) { + if (isNetworkError(error)) { + return `Could not connect to DB! Reason: ${error.code}` + } + if (isXMLRPCFault(error)) { + const xpathExceptionStart = error.faultString.indexOf(XPATH_EXCEPTION) + if (xpathExceptionStart > 0) { + const trimmed = error.faultString.substring(xpathExceptionStart + XPATH_EXCEPTION.length) + return `XPathException: ${trimmed}` + } + + const isMethodError = error.faultString.startsWith('Failed to invoke method') + if (isMethodError) { + const xmlrpcConnectionErrorStart = error.faultString.indexOf(XMLRPC_FAULT) + const trimmed = error.faultString.substring(xmlrpcConnectionErrorStart + XMLRPC_FAULT.length) + return `Error: ${trimmed}` + } + + return `Error executing your query ${error.faultString}` + } + if (isYargsError(error)) { + return `Problem with a provided Argument:\n${error.message}` + } + return error.message +} + export function handleError (error) { if (isNetworkError(error)) { return console.error(`Could not connect to DB! Reason: ${error.code}`) diff --git a/utility/message.js b/utility/message.js new file mode 100644 index 0000000..6c10ef9 --- /dev/null +++ b/utility/message.js @@ -0,0 +1,28 @@ +import chalk from 'chalk' + +/** + * @type {String} bright green checkmark + */ +export const check = chalk.greenBright('✔︎') +/** + * @type {String} bright red heavy cross + */ +export const fail = chalk.redBright('✘') + +/** + * log operation success message + * @param {any[]} message + * @returns {void} + */ +export function logSuccess (message) { + console.log(`${check} ${message}`) +} + +/** + * log operation failure message + * @param {any[]} message + * @returns {void} + */ +export function logFailure (message) { + console.error(`${fail} ${message}`) +}