From 4e70ca6fd0232a0dc5104c1923a3a1f47b3a5c05 Mon Sep 17 00:00:00 2001 From: Laurent Cozic Date: Thu, 16 Sep 2021 17:36:06 +0100 Subject: [PATCH 01/15] Server: Exclude certain queries from slow log --- packages/server/src/db.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/server/src/db.ts b/packages/server/src/db.ts index 8fed626d97..f8ba558981 100644 --- a/packages/server/src/db.ts +++ b/packages/server/src/db.ts @@ -122,8 +122,16 @@ export function setupSlowQueryLog(connection: DbConnection, slowQueryLogMinDurat const queryInfos: Record = {}; + // These queries do not return a response, so "query-response" is not + // called. + const ignoredQueries = /^BEGIN|SAVEPOINT|RELEASE SAVEPOINT|COMMIT|ROLLBACK/gi; + connection.on('query', (data) => { - const timeoutId = makeSlowQueryHandler(slowQueryLogMinDuration, connection, data.sql, data.bindings); + const sql: string = data.sql; + + if (!sql || sql.match(ignoredQueries)) return; + + const timeoutId = makeSlowQueryHandler(slowQueryLogMinDuration, connection, sql, data.bindings); queryInfos[data.__knexQueryUid] = { timeoutId, From b56177a4e398332e5a55c4bdf7d86bdc3c7d6177 Mon Sep 17 00:00:00 2001 From: Laurent Cozic Date: Fri, 17 Sep 2021 10:59:10 +0100 Subject: [PATCH 02/15] Tools: Added tools to stress test Joplin Server --- .eslintignore | 6 + .gitignore | 6 + packages/app-cli/.gitignore | 4 +- packages/app-cli/app/cli-utils.js | 2 +- packages/app-cli/app/command-testing.ts | 95 +++++++++++++++ packages/app-cli/createUsers.sh | 52 +++++++++ packages/app-cli/package.json | 1 + packages/app-cli/tools/populateDatabase.ts | 86 ++++++++++++++ .../lib/services/debug/populateDatabase.ts | 59 ++++++---- packages/server/src/models/UserModel.ts | 8 ++ packages/server/src/routes/api/debug.ts | 15 ++- packages/server/src/tools/debugTools.ts | 109 ++++++++++++------ 12 files changed, 379 insertions(+), 64 deletions(-) create mode 100644 packages/app-cli/app/command-testing.ts create mode 100755 packages/app-cli/createUsers.sh create mode 100644 packages/app-cli/tools/populateDatabase.ts diff --git a/.eslintignore b/.eslintignore index ae5fa58570..4c2fe82b05 100644 --- a/.eslintignore +++ b/.eslintignore @@ -76,6 +76,9 @@ packages/app-cli/app/command-e2ee.js.map packages/app-cli/app/command-settingschema.d.ts packages/app-cli/app/command-settingschema.js packages/app-cli/app/command-settingschema.js.map +packages/app-cli/app/command-testing.d.ts +packages/app-cli/app/command-testing.js +packages/app-cli/app/command-testing.js.map packages/app-cli/app/services/plugins/PluginRunner.d.ts packages/app-cli/app/services/plugins/PluginRunner.js packages/app-cli/app/services/plugins/PluginRunner.js.map @@ -109,6 +112,9 @@ packages/app-cli/tests/services/plugins/sandboxProxy.js.map packages/app-cli/tests/testUtils.d.ts packages/app-cli/tests/testUtils.js packages/app-cli/tests/testUtils.js.map +packages/app-cli/tools/populateDatabase.d.ts +packages/app-cli/tools/populateDatabase.js +packages/app-cli/tools/populateDatabase.js.map packages/app-desktop/ElectronAppWrapper.d.ts packages/app-desktop/ElectronAppWrapper.js packages/app-desktop/ElectronAppWrapper.js.map diff --git a/.gitignore b/.gitignore index ce68bab617..fce6debad4 100644 --- a/.gitignore +++ b/.gitignore @@ -61,6 +61,9 @@ packages/app-cli/app/command-e2ee.js.map packages/app-cli/app/command-settingschema.d.ts packages/app-cli/app/command-settingschema.js packages/app-cli/app/command-settingschema.js.map +packages/app-cli/app/command-testing.d.ts +packages/app-cli/app/command-testing.js +packages/app-cli/app/command-testing.js.map packages/app-cli/app/services/plugins/PluginRunner.d.ts packages/app-cli/app/services/plugins/PluginRunner.js packages/app-cli/app/services/plugins/PluginRunner.js.map @@ -94,6 +97,9 @@ packages/app-cli/tests/services/plugins/sandboxProxy.js.map packages/app-cli/tests/testUtils.d.ts packages/app-cli/tests/testUtils.js packages/app-cli/tests/testUtils.js.map +packages/app-cli/tools/populateDatabase.d.ts +packages/app-cli/tools/populateDatabase.js +packages/app-cli/tools/populateDatabase.js.map packages/app-desktop/ElectronAppWrapper.d.ts packages/app-desktop/ElectronAppWrapper.js packages/app-desktop/ElectronAppWrapper.js.map diff --git a/packages/app-cli/.gitignore b/packages/app-cli/.gitignore index 8651549eb3..48c78f239f 100644 --- a/packages/app-cli/.gitignore +++ b/packages/app-cli/.gitignore @@ -23,4 +23,6 @@ tests/support/dropbox-auth.txt tests/support/nextcloud-auth.json tests/support/onedrive-auth.txt build/ -patches/ \ No newline at end of file +patches/ +createUsers-*.txt +tools/temp/ diff --git a/packages/app-cli/app/cli-utils.js b/packages/app-cli/app/cli-utils.js index e19ca6d7a4..108e47bedd 100644 --- a/packages/app-cli/app/cli-utils.js +++ b/packages/app-cli/app/cli-utils.js @@ -89,7 +89,7 @@ cliUtils.makeCommandArgs = function(cmd, argv) { flags = cliUtils.parseFlags(flags); if (!flags.arg) { - booleanFlags.push(flags.short); + if (flags.short) booleanFlags.push(flags.short); if (flags.long) booleanFlags.push(flags.long); } diff --git a/packages/app-cli/app/command-testing.ts b/packages/app-cli/app/command-testing.ts new file mode 100644 index 0000000000..aa438ba770 --- /dev/null +++ b/packages/app-cli/app/command-testing.ts @@ -0,0 +1,95 @@ +const { BaseCommand } = require('./base-command.js'); +import { reg } from '@joplin/lib/registry'; +import Note from '@joplin/lib/models/Note'; +import uuid from '@joplin/lib/uuid'; +import populateDatabase from '@joplin/lib/services/debug/populateDatabase'; + +function randomElement(array: any[]): any { + if (!array.length) return null; + return array[Math.floor(Math.random() * array.length)]; +} + +function itemCount(args: any) { + const count = Number(args.arg0); + if (!count || isNaN(count)) throw new Error('Note count must be specified'); + return count; +} + +class Command extends BaseCommand { + usage() { + return 'testing [arg0]'; + } + + description() { + return 'testing'; + } + + enabled() { + return false; + } + + options(): any[] { + return [ + ['--folder-count ', 'Folders to create'], + ['--note-count ', 'Notes to create'], + ['--tag-count ', 'Tags to create'], + ['--tags-per-note ', 'Tags per note'], + ['--silent', 'Silent'], + ]; + } + + async action(args: any) { + const { command, options } = args; + + if (command === 'populate') { + await populateDatabase(reg.db(), { + folderCount: options['folder-count'], + noteCount: options['note-count'], + tagCount: options['tag-count'], + tagsPerNote: options['tags-per-note'], + silent: options['silent'], + }); + } + + const promises: any[] = []; + + if (command === 'createRandomNotes') { + const noteCount = itemCount(args); + + for (let i = 0; i < noteCount; i++) { + promises.push(Note.save({ + title: `Note ${uuid.createNano()}`, + })); + } + } + + if (command === 'updateRandomNotes') { + const noteCount = itemCount(args); + + const noteIds = await Note.allIds(); + + for (let i = 0; i < noteCount; i++) { + const noteId = randomElement(noteIds); + promises.push(Note.save({ + id: noteId, + title: `Note ${uuid.createNano()}`, + })); + } + } + + if (command === 'deleteRandomNotes') { + const noteCount = itemCount(args); + const noteIds = await Note.allIds(); + + for (let i = 0; i < noteCount; i++) { + const noteId = randomElement(noteIds); + promises.push(Note.delete(noteId)); + } + } + + await Promise.all(promises); + } + +} + +module.exports = Command; diff --git a/packages/app-cli/createUsers.sh b/packages/app-cli/createUsers.sh new file mode 100755 index 0000000000..0a7aa43432 --- /dev/null +++ b/packages/app-cli/createUsers.sh @@ -0,0 +1,52 @@ +#!/bin/bash + +# Start the server with: +# +# JOPLIN_IS_TESTING=1 npm run start-dev + +SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )" + +# curl --data '{"action": "clearDatabase"}' -H 'Content-Type: application/json' http://api.joplincloud.local:22300/api/debug + +# SMALL + +# curl --data '{"action": "createTestUsers", "count": 400, "fromNum": 1}' -H 'Content-Type: application/json' http://api.joplincloud.local:22300/api/debug + +NUM=398 +while [ "$NUM" -lt 400 ]; do + NUM=$(( NUM + 1 )) + + echo "User $NUM" + + CMD_FILE="$SCRIPT_DIR/createUsers-$NUM.txt" + PROFILE_DIR=~/.config/joplindev-testing-$NUM + USER_EMAIL="user$NUM@example.com" + + rm -rf "$CMD_FILE" "$PROFILE_DIR" + touch "$CMD_FILE" + + FLAG_FOLDER_COUNT=100 + FLAG_NOTE_COUNT=1000 + FLAG_TAG_COUNT=20 + + if [ "$NUM" -gt 300 ]; then + FLAG_FOLDER_COUNT=2000 + FLAG_NOTE_COUNT=10000 + FLAG_TAG_COUNT=200 + fi + + if [ "$NUM" -gt 399 ]; then + FLAG_FOLDER_COUNT=10000 + FLAG_NOTE_COUNT=150000 + FLAG_TAG_COUNT=2000 + fi + + echo "testing populate --silent --folder-count $FLAG_FOLDER_COUNT --note-count $FLAG_NOTE_COUNT --tag-count $FLAG_TAG_COUNT" >> "$CMD_FILE" + echo "config keychain.supported 0" >> "$CMD_FILE" + echo "config sync.target 10" >> "$CMD_FILE" + echo "config sync.10.username $USER_EMAIL" >> "$CMD_FILE" + echo "config sync.10.password hunter1hunter2hunter3" >> "$CMD_FILE" + echo "sync" >> "$CMD_FILE" + + npm start -- --profile "$PROFILE_DIR" batch "$CMD_FILE" +done diff --git a/packages/app-cli/package.json b/packages/app-cli/package.json index 850c09c85f..cd20a23af7 100644 --- a/packages/app-cli/package.json +++ b/packages/app-cli/package.json @@ -10,6 +10,7 @@ "test-ci": "jest --config=jest.config.js --forceExit", "build": "gulp build", "start": "gulp build -L && node \"build/main.js\" --stack-trace-enabled --log-level debug --env dev", + "start-no-build": "node \"build/main.js\" --stack-trace-enabled --log-level debug --env dev", "tsc": "node node_modules/typescript/bin/tsc --project tsconfig.json", "watch": "node node_modules/typescript/bin/tsc --watch --project tsconfig.json" }, diff --git a/packages/app-cli/tools/populateDatabase.ts b/packages/app-cli/tools/populateDatabase.ts new file mode 100644 index 0000000000..c073e8d495 --- /dev/null +++ b/packages/app-cli/tools/populateDatabase.ts @@ -0,0 +1,86 @@ +import * as fs from 'fs-extra'; +import { homedir } from 'os'; +import { execCommand2 } from '@joplin/tools/tool-utils'; +import { chdir } from 'process'; + +const minUserNum = 1; +const maxUserNum = 400; + +const cliDir = `${__dirname}/..`; +const tempDir = `${__dirname}/temp`; + +function randomInt(min: number, max: number) { + min = Math.ceil(min); + max = Math.floor(max); + return Math.floor(Math.random() * (max - min + 1)) + min; +} + +const processing_: Record = {}; + +const processUser = async (userNum: number) => { + if (processing_[userNum]) { + console.info(`User already being processed: ${userNum} - skipping`); + return; + } + + processing_[userNum] = true; + + try { + const userEmail = `user${userNum}@example.com`; + const userPassword = 'hunter1hunter2hunter3'; + const commandFile = `${tempDir}/populateDatabase-${userNum}.txt`; + const profileDir = `${homedir()}/.config/joplindev-populate/joplindev-testing-${userNum}`; + + const commands: string[] = []; + const jackpot = Math.random() >= 0.95 ? 100 : 1; + + commands.push(`testing createRandomNotes ${randomInt(1, 500 * jackpot)}`); + commands.push(`testing updateRandomNotes ${randomInt(1, 1500 * jackpot)}`); + commands.push(`testing deleteRandomNotes ${randomInt(1, 200 * jackpot)}`); + commands.push('config keychain.supported 0'); + commands.push('config sync.target 10'); + commands.push(`config sync.10.username ${userEmail}`); + commands.push(`config sync.10.password ${userPassword}`); + commands.push('sync'); + + await fs.writeFile(commandFile, commands.join('\n'), 'utf8'); + + await chdir(cliDir); + + await execCommand2(['npm', 'run', 'start-no-build', '--', '--profile', profileDir, 'batch', commandFile]); + } catch (error) { + console.error(`Could not process user ${userNum}:`, error); + } finally { + delete processing_[userNum]; + } +}; + +const waitForProcessing = (count: number) => { + return new Promise((resolve) => { + const iid = setInterval(() => { + if (Object.keys(processing_).length <= count) { + clearInterval(iid); + resolve(null); + } + }, 100); + }); +}; + +const main = async () => { + await fs.mkdirp(tempDir); + + // Build the app once before starting, because we'll use start-no-build to + // run the scripts (faster) + await execCommand2(['npm', 'run', 'build']); + + while (true) { + const userNum = randomInt(minUserNum, maxUserNum); + void processUser(userNum); + await waitForProcessing(10); + } +}; + +main().catch((error) => { + console.error('Fatal error', error); + process.exit(1); +}); diff --git a/packages/lib/services/debug/populateDatabase.ts b/packages/lib/services/debug/populateDatabase.ts index c666328a62..f9d85ed874 100644 --- a/packages/lib/services/debug/populateDatabase.ts +++ b/packages/lib/services/debug/populateDatabase.ts @@ -2,6 +2,15 @@ import Folder from '../../models/Folder'; import Note from '../../models/Note'; import Tag from '../../models/Tag'; +export interface Options { + folderCount?: number; + noteCount?: number; + tagCount?: number; + tagsPerNote?: number; + silent?: number; + clearDatabase?: boolean; +} + function randomIndex(array: any[]): number { return Math.round(Math.random() * (array.length - 1)); } @@ -31,19 +40,23 @@ function randomElement(array: any[]): any { // Use the constants below to define how many folders, notes and tags // should be created. -export default async function populateDatabase(db: any) { - await db.clearForTesting(); +export default async function populateDatabase(db: any, options: Options = null) { + options = { + folderCount: 0, + noteCount: 0, + tagCount: 0, + tagsPerNote: 0, + clearDatabase: false, + ...options, + }; - const folderCount = 20; - const noteCount = 200; - const tagCount = 2000; - const tagsPerNote = 3; + if (options.clearDatabase) await db.clearForTesting(); const createdFolderIds: string[] = []; const createdNoteIds: string[] = []; const createdTagIds: string[] = []; - for (let i = 0; i < folderCount; i++) { + for (let i = 0; i < options.folderCount; i++) { const folder: any = { title: `folder${i}`, }; @@ -58,15 +71,15 @@ export default async function populateDatabase(db: any) { const savedFolder = await Folder.save(folder); createdFolderIds.push(savedFolder.id); - console.info(`Folders: ${i} / ${folderCount}`); + if (!options.silent) console.info(`Folders: ${i} / ${options.folderCount}`); } let tagBatch = []; - for (let i = 0; i < tagCount; i++) { + for (let i = 0; i < options.tagCount; i++) { const tagTitle = randomElement(wordList); // `tag${i}` tagBatch.push(Tag.save({ title: tagTitle }, { dispatchUpdateAction: false }).then((savedTag: any) => { createdTagIds.push(savedTag.id); - console.info(`Tags: ${i} / ${tagCount}`); + if (!options.silent) console.info(`Tags: ${i} / ${options.tagCount}`); })); if (tagBatch.length > 1000) { @@ -81,14 +94,14 @@ export default async function populateDatabase(db: any) { } let noteBatch = []; - for (let i = 0; i < noteCount; i++) { + for (let i = 0; i < options.noteCount; i++) { const note: any = { title: `note${i}`, body: `This is note num. ${i}` }; const parentIndex = randomIndex(createdFolderIds); note.parent_id = createdFolderIds[parentIndex]; noteBatch.push(Note.save(note, { dispatchUpdateAction: false }).then((savedNote: any) => { createdNoteIds.push(savedNote.id); - console.info(`Notes: ${i} / ${noteCount}`); + console.info(`Notes: ${i} / ${options.noteCount}`); })); if (noteBatch.length > 1000) { @@ -102,21 +115,23 @@ export default async function populateDatabase(db: any) { noteBatch = []; } - let noteTagBatch = []; - for (const noteId of createdNoteIds) { - const tagIds = randomElements(createdTagIds, tagsPerNote); - noteTagBatch.push(Tag.setNoteTagsByIds(noteId, tagIds)); + if (options.tagsPerNote) { + let noteTagBatch = []; + for (const noteId of createdNoteIds) { + const tagIds = randomElements(createdTagIds, options.tagsPerNote); + noteTagBatch.push(Tag.setNoteTagsByIds(noteId, tagIds)); - if (noteTagBatch.length > 1000) { + if (noteTagBatch.length > 1000) { + await Promise.all(noteTagBatch); + noteTagBatch = []; + } + } + + if (noteTagBatch.length) { await Promise.all(noteTagBatch); noteTagBatch = []; } } - - if (noteTagBatch.length) { - await Promise.all(noteTagBatch); - noteTagBatch = []; - } } const wordList = ['a', 'ability', 'able', 'about', 'above', 'accept', 'according', 'account', 'across', 'act', 'action', 'activity', 'actually', 'add', 'address', 'administration', 'admit', 'adult', 'affect', 'after', 'again', 'against', 'age', 'agency', 'agent', 'ago', 'agree', 'agreement', 'ahead', 'air', 'all', 'allow', 'almost', 'alone', 'along', 'already', 'also', 'although', 'always', 'American', 'among', 'amount', 'analysis', 'and', 'animal', 'another', 'answer', 'any', 'anyone', 'anything', 'appear', 'apply', 'approach', 'area', 'argue', 'arm', 'around', 'arrive', 'art', 'article', 'artist', 'as', 'ask', 'assume', 'at', 'attack', 'attention', 'attorney', 'audience', 'author', 'authority', 'available', 'avoid', 'away', 'baby', 'back', 'bad', 'bag', 'ball', 'bank', 'bar', 'base', 'be', 'beat', 'beautiful', 'because', 'become', 'bed', 'before', 'begin', 'behavior', 'behind', 'believe', 'benefit', 'best', 'better', 'between', 'beyond', 'big', 'bill', 'billion', 'bit', 'black', 'blood', 'blue', 'board', 'body', 'book', 'born', 'both', 'box', 'boy', 'break', 'bring', 'brother', 'budget', 'build', 'building', 'business', 'but', 'buy', 'by', 'call', 'camera', 'campaign', 'can', 'cancer', 'candidate', 'capital', 'car', 'card', 'care', 'career', 'carry', 'case', 'catch', 'cause', 'cell', 'center', 'central', 'century', 'certain', 'certainly', 'chair', 'challenge', 'chance', 'change', 'character', 'charge', 'check', 'child', 'choice', 'choose', 'church', 'citizen', 'city', 'civil', 'claim', 'class', 'clear', 'clearly', 'close', 'coach', 'cold', 'collection', 'college', 'color', 'come', 'commercial', 'common', 'community', 'company', 'compare', 'computer', 'concern', 'condition', 'conference', 'Congress', 'consider', 'consumer', 'contain', 'continue', 'control', 'cost', 'could', 'country', 'couple', 'course', 'court', 'cover', 'create', 'crime', 'cultural', 'culture', 'cup', 'current', 'customer', 'cut', 'dark', 'data', 'daughter', 'day', 'dead', 'deal', 'death', 'debate', 'decade', 'decide', 'decision', 'deep', 'defense', 'degree', 'Democrat', 'democratic', 'describe', 'design', 'despite', 'detail', 'determine', 'develop', 'development', 'die', 'difference', 'different', 'difficult', 'dinner', 'direction', 'director', 'discover', 'discuss', 'discussion', 'disease', 'do', 'doctor', 'dog', 'door', 'down', 'draw', 'dream', 'drive', 'drop', 'drug', 'during', 'each', 'early', 'east', 'easy', 'eat', 'economic', 'economy', 'edge', 'education', 'effect', 'effort', 'eight', 'either', 'election', 'else', 'employee', 'end', 'energy', 'enjoy', 'enough', 'enter', 'entire', 'environment', 'environmental', 'especially', 'establish', 'even', 'evening', 'event', 'ever', 'every', 'everybody', 'everyone', 'everything', 'evidence', 'exactly', 'example', 'executive', 'exist', 'expect', 'experience', 'expert', 'explain', 'eye', 'face', 'fact', 'factor', 'fail', 'fall', 'family', 'far', 'fast', 'father', 'fear', 'federal', 'feel', 'feeling', 'few', 'field', 'fight', 'figure', 'fill', 'film', 'final', 'finally', 'financial', 'find', 'fine', 'finger', 'finish', 'fire', 'firm', 'first', 'fish', 'five', 'floor', 'fly', 'focus', 'follow', 'food', 'foot', 'for', 'force', 'foreign', 'forget', 'form', 'former', 'forward', 'four', 'free', 'friend', 'from', 'front', 'full', 'fund', 'future', 'game', 'garden', 'gas', 'general', 'generation', 'get', 'girl', 'give', 'glass', 'go', 'goal', 'good', 'government', 'great', 'green', 'ground', 'group', 'grow', 'growth', 'guess', 'gun', 'guy', 'hair', 'half', 'hand', 'hang', 'happen', 'happy', 'hard', 'have', 'he', 'head', 'health', 'hear', 'heart', 'heat', 'heavy', 'help', 'her', 'here', 'herself', 'high', 'him', 'himself', 'his', 'history', 'hit', 'hold', 'home', 'hope', 'hospital', 'hot', 'hotel', 'hour', 'house', 'how', 'however', 'huge', 'human', 'hundred', 'husband', 'I', 'idea', 'identify', 'if', 'image', 'imagine', 'impact', 'important', 'improve', 'in', 'include', 'including', 'increase', 'indeed', 'indicate', 'individual', 'industry', 'information', 'inside', 'instead', 'institution', 'interest', 'interesting', 'international', 'interview', 'into', 'investment', 'involve', 'issue', 'it', 'item', 'its', 'itself', 'job', 'join', 'just', 'keep', 'key', 'kid', 'kill', 'kind', 'kitchen', 'know', 'knowledge', 'land', 'language', 'large', 'last', 'late', 'later', 'laugh', 'law', 'lawyer', 'lay', 'lead', 'leader', 'learn', 'least', 'leave', 'left', 'leg', 'legal', 'less', 'let', 'letter', 'level', 'lie', 'life', 'light', 'like', 'likely', 'line', 'list', 'listen', 'little', 'live', 'local', 'long', 'look', 'lose', 'loss', 'lot', 'love', 'low', 'machine', 'magazine', 'main', 'maintain', 'major', 'majority', 'make', 'man', 'manage', 'management', 'manager', 'many', 'market', 'marriage', 'material', 'matter', 'may', 'maybe', 'me', 'mean', 'measure', 'media', 'medical', 'meet', 'meeting', 'member', 'memory', 'mention', 'message', 'method', 'middle', 'might', 'military', 'million', 'mind', 'minute', 'miss', 'mission', 'model', 'modern', 'moment', 'money', 'month', 'more', 'morning', 'most', 'mother', 'mouth', 'move', 'movement', 'movie', 'Mr', 'Mrs', 'much', 'music', 'must', 'my', 'myself', 'name', 'nation', 'national', 'natural', 'nature', 'near', 'nearly', 'necessary', 'need', 'network', 'never', 'new', 'news', 'newspaper', 'next', 'nice', 'night', 'no', 'none', 'nor', 'north', 'not', 'note', 'nothing', 'notice', 'now', 'n\'t', 'number', 'occur', 'of', 'off', 'offer', 'office', 'officer', 'official', 'often', 'oh', 'oil', 'ok', 'old', 'on', 'once', 'one', 'only', 'onto', 'open', 'operation', 'opportunity', 'option', 'or', 'order', 'organization', 'other', 'others', 'our', 'out', 'outside', 'over', 'own', 'owner', 'page', 'pain', 'painting', 'paper', 'parent', 'part', 'participant', 'particular', 'particularly', 'partner', 'party', 'pass', 'past', 'patient', 'pattern', 'pay', 'peace', 'people', 'per', 'perform', 'performance', 'perhaps', 'period', 'person', 'personal', 'phone', 'physical', 'pick', 'picture', 'piece', 'place', 'plan', 'plant', 'play', 'player', 'PM', 'point', 'police', 'policy', 'political', 'politics', 'poor', 'popular', 'population', 'position', 'positive', 'possible', 'power', 'practice', 'prepare', 'present', 'president', 'pressure', 'pretty', 'prevent', 'price', 'private', 'probably', 'problem', 'process', 'produce', 'product', 'production', 'professional', 'professor', 'program', 'project', 'property', 'protect', 'prove', 'provide', 'public', 'pull', 'purpose', 'push', 'put', 'quality', 'question', 'quickly', 'quite', 'race', 'radio', 'raise', 'range', 'rate', 'rather', 'reach', 'read', 'ready', 'real', 'reality', 'realize', 'really', 'reason', 'receive', 'recent', 'recently', 'recognize', 'record', 'red', 'reduce', 'reflect', 'region', 'relate', 'relationship', 'religious', 'remain', 'remember', 'remove', 'report', 'represent', 'Republican', 'require', 'research', 'resource', 'respond', 'response', 'responsibility', 'rest', 'result', 'return', 'reveal', 'rich', 'right', 'rise', 'risk', 'road', 'rock', 'role', 'room', 'rule', 'run', 'safe', 'same', 'save', 'say', 'scene', 'school', 'science', 'scientist', 'score', 'sea', 'season', 'seat', 'second', 'section', 'security', 'see', 'seek', 'seem', 'sell', 'send', 'senior', 'sense', 'series', 'serious', 'serve', 'service', 'set', 'seven', 'several', 'sex', 'sexual', 'shake', 'share', 'she', 'shoot', 'short', 'shot', 'should', 'shoulder', 'show', 'side', 'sign', 'significant', 'similar', 'simple', 'simply', 'since', 'sing', 'single', 'sister', 'sit', 'site', 'situation', 'six', 'size', 'skill', 'skin', 'small', 'smile', 'so', 'social', 'society', 'soldier', 'some', 'somebody', 'someone', 'something', 'sometimes', 'son', 'song', 'soon', 'sort', 'sound', 'source', 'south', 'southern', 'space', 'speak', 'special', 'specific', 'speech', 'spend', 'sport', 'spring', 'staff', 'stage', 'stand', 'standard', 'star', 'start', 'state', 'statement', 'station', 'stay', 'step', 'still', 'stock', 'stop', 'store', 'story', 'strategy', 'street', 'strong', 'structure', 'student', 'study', 'stuff', 'style', 'subject', 'success', 'successful', 'such', 'suddenly', 'suffer', 'suggest', 'summer', 'support', 'sure', 'surface', 'system', 'table', 'take', 'talk', 'task', 'tax', 'teach', 'teacher', 'team', 'technology', 'television', 'tell', 'ten', 'tend', 'term', 'test', 'than', 'thank', 'that', 'the', 'their', 'them', 'themselves', 'then', 'theory', 'there', 'these', 'they', 'thing', 'think', 'third', 'this', 'those', 'though', 'thought', 'thousand', 'threat', 'three', 'through', 'throughout', 'throw', 'thus', 'time', 'to', 'today', 'together', 'tonight', 'too', 'top', 'total', 'tough', 'toward', 'town', 'trade', 'traditional', 'training', 'travel', 'treat', 'treatment', 'tree', 'trial', 'trip', 'trouble', 'true', 'truth', 'try', 'turn', 'TV', 'two', 'type', 'under', 'understand', 'unit', 'until', 'up', 'upon', 'us', 'use', 'usually', 'value', 'various', 'very', 'victim', 'view', 'violence', 'visit', 'voice', 'vote', 'wait', 'walk', 'wall', 'want', 'war', 'watch', 'water', 'way', 'we', 'weapon', 'wear', 'week', 'weight', 'well', 'west', 'western', 'what', 'whatever', 'when', 'where', 'whether', 'which', 'while', 'white', 'who', 'whole', 'whom', 'whose', 'why', 'wide', 'wife', 'will', 'win', 'wind', 'window', 'wish', 'with', 'within', 'without', 'woman', 'wonder', 'word', 'work', 'worker', 'world', 'worry', 'would', 'write', 'writer', 'wrong', 'yard', 'yeah', 'year', 'yes', 'yet', 'you', 'young', 'your', 'yourself']; diff --git a/packages/server/src/models/UserModel.ts b/packages/server/src/models/UserModel.ts index fcc8d0cab2..921ed429d3 100644 --- a/packages/server/src/models/UserModel.ts +++ b/packages/server/src/models/UserModel.ts @@ -469,4 +469,12 @@ export default class UserModel extends BaseModel { }, 'UserModel::save'); } + public async saveMulti(users: User[], options: SaveOptions = {}): Promise { + await this.withTransaction(async () => { + for (const user of users) { + await this.save(user, options); + } + }, 'UserModel::saveMulti'); + } + } diff --git a/packages/server/src/routes/api/debug.ts b/packages/server/src/routes/api/debug.ts index 8e7b7363ea..3b4282a24a 100644 --- a/packages/server/src/routes/api/debug.ts +++ b/packages/server/src/routes/api/debug.ts @@ -1,5 +1,5 @@ import config from '../../config'; -import { createTestUsers } from '../../tools/debugTools'; +import { clearDatabase, createTestUsers, CreateTestUsersOptions } from '../../tools/debugTools'; import { bodyFields } from '../../utils/requestUtils'; import Router from '../../utils/Router'; import { RouteType } from '../../utils/types'; @@ -12,6 +12,8 @@ router.public = true; interface Query { action: string; + count?: number; + fromNum?: number; } router.post('api/debug', async (_path: SubPath, ctx: AppContext) => { @@ -20,7 +22,16 @@ router.post('api/debug', async (_path: SubPath, ctx: AppContext) => { console.info(`Action: ${query.action}`); if (query.action === 'createTestUsers') { - await createTestUsers(ctx.joplin.db, config()); + const options: CreateTestUsersOptions = {}; + + if ('count' in query) options.count = query.count; + if ('fromNum' in query) options.fromNum = query.fromNum; + + await createTestUsers(ctx.joplin.db, config(), options); + } + + if (query.action === 'clearDatabase') { + await clearDatabase(ctx.joplin.db); } }); diff --git a/packages/server/src/tools/debugTools.ts b/packages/server/src/tools/debugTools.ts index d30fec16fb..b841825479 100644 --- a/packages/server/src/tools/debugTools.ts +++ b/packages/server/src/tools/debugTools.ts @@ -1,9 +1,14 @@ import { DbConnection, dropTables, migrateLatest } from '../db'; import newModelFactory from '../models/factory'; import { AccountType } from '../models/UserModel'; -import { UserFlagType } from '../services/database/types'; +import { User, UserFlagType } from '../services/database/types'; import { Config } from '../utils/types'; +export interface CreateTestUsersOptions { + count?: number; + fromNum?: number; +} + export async function handleDebugCommands(argv: any, db: DbConnection, config: Config): Promise { if (argv.debugCreateTestUsers) { await createTestUsers(db, config); @@ -14,51 +19,79 @@ export async function handleDebugCommands(argv: any, db: DbConnection, config: C return true; } -export async function createTestUsers(db: DbConnection, config: Config) { +export async function clearDatabase(db: DbConnection) { await dropTables(db); await migrateLatest(db); +} + +export async function createTestUsers(db: DbConnection, config: Config, options: CreateTestUsersOptions = null) { + options = { + count: 0, + fromNum: 1, + ...options, + }; const password = 'hunter1hunter2hunter3'; - const models = newModelFactory(db, config); - for (let userNum = 1; userNum <= 2; userNum++) { - await models.user().save({ - email: `user${userNum}@example.com`, - password, - full_name: `User ${userNum}`, - }); - } + if (options.count) { + const models = newModelFactory(db, config); - { - const { user } = await models.subscription().saveUserAndSubscription( - 'usersub@example.com', - 'With Sub', - AccountType.Basic, - 'usr_111', - 'sub_111' - ); - await models.user().save({ id: user.id, password }); - } + const users: User[] = []; - { - const { user, subscription } = await models.subscription().saveUserAndSubscription( - 'userfailedpayment@example.com', - 'Failed Payment', - AccountType.Basic, - 'usr_222', - 'sub_222' - ); - await models.user().save({ id: user.id, password }); - await models.subscription().handlePayment(subscription.stripe_subscription_id, false); - } + for (let i = 0; i < options.count; i++) { + const userNum = i + options.fromNum; + users.push({ + email: `user${userNum}@example.com`, + password, + full_name: `User ${userNum}`, + }); + } - { - const user = await models.user().save({ - email: 'userwithflags@example.com', - password, - full_name: 'User Withflags', - }); + await models.user().saveMulti(users); + } else { + await dropTables(db); + await migrateLatest(db); + const models = newModelFactory(db, config); - await models.userFlag().add(user.id, UserFlagType.AccountOverLimit); + for (let userNum = 1; userNum <= 2; userNum++) { + await models.user().save({ + email: `user${userNum}@example.com`, + password, + full_name: `User ${userNum}`, + }); + } + + { + const { user } = await models.subscription().saveUserAndSubscription( + 'usersub@example.com', + 'With Sub', + AccountType.Basic, + 'usr_111', + 'sub_111' + ); + await models.user().save({ id: user.id, password }); + } + + { + const { user, subscription } = await models.subscription().saveUserAndSubscription( + 'userfailedpayment@example.com', + 'Failed Payment', + AccountType.Basic, + 'usr_222', + 'sub_222' + ); + await models.user().save({ id: user.id, password }); + await models.subscription().handlePayment(subscription.stripe_subscription_id, false); + } + + { + const user = await models.user().save({ + email: 'userwithflags@example.com', + password, + full_name: 'User Withflags', + }); + + await models.userFlag().add(user.id, UserFlagType.AccountOverLimit); + } } } From f91b4edb30d45b92474bd87f4a177d32e0ca9da1 Mon Sep 17 00:00:00 2001 From: Laurent Cozic Date: Fri, 17 Sep 2021 18:27:25 +0100 Subject: [PATCH 03/15] Tools: Tweak to stress test script --- packages/app-cli/tools/populateDatabase.ts | 24 +++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/packages/app-cli/tools/populateDatabase.ts b/packages/app-cli/tools/populateDatabase.ts index c073e8d495..b791b9cbb4 100644 --- a/packages/app-cli/tools/populateDatabase.ts +++ b/packages/app-cli/tools/populateDatabase.ts @@ -1,3 +1,20 @@ +// This script can be used to simulate a running production environment, by +// having multiple users in parallel changing notes and synchronising. +// +// To get it working: +// +// - Run the Postgres database -- `sudo docker-compose --file docker-compose.db-dev.yml up` +// - Update the DB parameters in ~/joplin-credentials/server.env to use the dev +// database +// - Run the server - `JOPLIN_IS_TESTING=1 npm run start-dev` +// - Then run this script - `node populateDatabase.js` +// +// Currently it doesn't actually create the users, so that should be done using: +// +// curl --data '{"action": "createTestUsers", "count": 400, "fromNum": 1}' -H 'Content-Type: application/json' http://api.joplincloud.local:22300/api/debug +// +// That will create n users with email `user@example.com` + import * as fs from 'fs-extra'; import { homedir } from 'os'; import { execCommand2 } from '@joplin/tools/tool-utils'; @@ -73,8 +90,13 @@ const main = async () => { // run the scripts (faster) await execCommand2(['npm', 'run', 'build']); + const focusUserNum = 400; + while (true) { - const userNum = randomInt(minUserNum, maxUserNum); + let userNum = randomInt(minUserNum, maxUserNum); + + if (Math.random() >= .7) userNum = focusUserNum; + void processUser(userNum); await waitForProcessing(10); } From cd877f64cd0dc404524c06bc1c89a931922d2663 Mon Sep 17 00:00:00 2001 From: Laurent Cozic Date: Sat, 18 Sep 2021 11:29:24 +0100 Subject: [PATCH 04/15] Server: Improved support for background tasks and added admin UI to view them --- .../src/middleware/notificationHandler.ts | 9 +- .../server/src/models/NotificationModel.ts | 10 +- .../server/src/models/SubscriptionModel.ts | 2 +- packages/server/src/routes/index/tasks.ts | 135 ++++++++++++++++++ packages/server/src/routes/routes.ts | 18 +-- packages/server/src/services/CronService.ts | 42 ------ .../server/src/services/TaskService.test.ts | 82 +++++++++++ packages/server/src/services/TaskService.ts | 108 ++++++++++++++ .../server/src/services/database/types.ts | 1 + packages/server/src/services/types.ts | 4 +- packages/server/src/utils/routeUtils.ts | 1 + packages/server/src/utils/setupAppContext.ts | 4 +- packages/server/src/utils/setupTaskService.ts | 49 +++++++ packages/server/src/utils/startServices.ts | 2 +- packages/server/src/utils/time.ts | 3 +- packages/server/src/utils/types.ts | 2 +- packages/server/src/utils/views/table.ts | 27 ++-- .../server/src/views/index/tasks.mustache | 11 ++ .../server/src/views/partials/navbar.mustache | 3 + .../src/views/partials/notifications.mustache | 2 +- .../server/src/views/partials/table.mustache | 34 ++--- .../src/views/partials/tableHeader.mustache | 7 +- .../src/views/partials/tableRowItem.mustache | 7 +- 23 files changed, 476 insertions(+), 87 deletions(-) create mode 100644 packages/server/src/routes/index/tasks.ts delete mode 100644 packages/server/src/services/CronService.ts create mode 100644 packages/server/src/services/TaskService.test.ts create mode 100644 packages/server/src/services/TaskService.ts create mode 100644 packages/server/src/utils/setupTaskService.ts create mode 100644 packages/server/src/views/index/tasks.mustache diff --git a/packages/server/src/middleware/notificationHandler.ts b/packages/server/src/middleware/notificationHandler.ts index 90a784d4bc..1ba22ba56e 100644 --- a/packages/server/src/middleware/notificationHandler.ts +++ b/packages/server/src/middleware/notificationHandler.ts @@ -42,6 +42,13 @@ async function handleSqliteInProdNotification(ctx: AppContext) { } } +function levelClassName(level: NotificationLevel): string { + if (level === NotificationLevel.Important) return 'is-warning'; + if (level === NotificationLevel.Normal) return 'is-info'; + if (level === NotificationLevel.Error) return 'is-danger'; + throw new Error(`Unknown level: ${level}`); +} + async function makeNotificationViews(ctx: AppContext): Promise { const markdownIt = new MarkdownIt(); @@ -52,7 +59,7 @@ async function makeNotificationViews(ctx: AppContext): Promise { level: NotificationLevel.Normal, message: 'Thank you! Your account has been successfully upgraded to Pro.', }, + [NotificationKey.Any]: { + level: NotificationLevel.Normal, + message: '', + }, }; const type = notificationTypes[key]; @@ -72,7 +78,9 @@ export default class NotificationModel extends BaseModel { } } - return this.save({ key, message, level, owner_id: userId }); + const actualKey = key === NotificationKey.Any ? `any_${uuidgen()}` : key; + + return this.save({ key: actualKey, message, level, owner_id: userId }); } public async markAsRead(userId: Uuid, key: NotificationKey): Promise { diff --git a/packages/server/src/models/SubscriptionModel.ts b/packages/server/src/models/SubscriptionModel.ts index d29eaeaa27..0002b02526 100644 --- a/packages/server/src/models/SubscriptionModel.ts +++ b/packages/server/src/models/SubscriptionModel.ts @@ -98,7 +98,7 @@ export default class SubscriptionModel extends BaseModel { // failed. // // We don't update the user can_upload and enabled properties here - // because it's done after a few days from CronService. + // because it's done after a few days from TaskService. if (!sub.last_payment_failed_time) { const user = await this.models().user().load(sub.user_id, { fields: ['email', 'id', 'full_name'] }); diff --git a/packages/server/src/routes/index/tasks.ts b/packages/server/src/routes/index/tasks.ts new file mode 100644 index 0000000000..c4e75292a3 --- /dev/null +++ b/packages/server/src/routes/index/tasks.ts @@ -0,0 +1,135 @@ +import { makeUrl, redirect, SubPath, UrlType } from '../../utils/routeUtils'; +import Router from '../../utils/Router'; +import { RouteType } from '../../utils/types'; +import { AppContext } from '../../utils/types'; +import { bodyFields } from '../../utils/requestUtils'; +import { ErrorBadRequest, ErrorForbidden } from '../../utils/errors'; +import defaultView from '../../utils/defaultView'; +import { makeTableView, Row, Table } from '../../utils/views/table'; +import { yesOrNo } from '../../utils/strings'; +import { formatDateTime } from '../../utils/time'; +import { createCsrfTag } from '../../utils/csrf'; +import { RunType } from '../../services/TaskService'; +import { NotificationKey } from '../../models/NotificationModel'; +import { NotificationLevel } from '../../services/database/types'; + +const router: Router = new Router(RouteType.Web); + +router.post('tasks', async (_path: SubPath, ctx: AppContext) => { + const user = ctx.joplin.owner; + if (!user.is_admin) throw new ErrorForbidden(); + + const taskService = ctx.joplin.services.tasks; + const fields: any = await bodyFields(ctx.req); + + if (fields.startTaskButton) { + const errors: Error[] = []; + + for (const k of Object.keys(fields)) { + if (k.startsWith('checkbox_')) { + const taskId = k.substr(9); + try { + void taskService.runTask(taskId, RunType.Manual); + } catch (error) { + errors.push(error); + } + } + } + + if (errors.length) { + await ctx.joplin.models.notification().add( + user.id, + NotificationKey.Any, + NotificationLevel.Error, + `Some tasks could not be started: ${errors.join('. ')}` + ); + } + } else { + throw new ErrorBadRequest('Invalid action'); + } + + return redirect(ctx, makeUrl(UrlType.Tasks)); +}); + +router.get('tasks', async (_path: SubPath, ctx: AppContext) => { + const user = ctx.joplin.owner; + if (!user.is_admin) throw new ErrorForbidden(); + + const taskService = ctx.joplin.services.tasks; + + const taskRows: Row[] = []; + for (const [taskId, task] of Object.entries(taskService.tasks)) { + const state = taskService.taskState(taskId); + + taskRows.push([ + { + value: `checkbox_${taskId}`, + checkbox: true, + }, + { + value: taskId, + }, + { + value: task.description, + }, + { + value: task.schedule, + }, + { + value: yesOrNo(state.running), + }, + { + value: state.lastRunTime ? formatDateTime(state.lastRunTime) : '-', + }, + { + value: state.lastCompletionTime ? formatDateTime(state.lastCompletionTime) : '-', + }, + ]); + } + + const table: Table = { + headers: [ + { + name: 'select', + label: '', + }, + { + name: 'id', + label: 'ID', + }, + { + name: 'description', + label: 'Description', + }, + { + name: 'schedule', + label: 'Schedule', + }, + { + name: 'running', + label: 'Running', + }, + { + name: 'lastRunTime', + label: 'Last Run', + }, + { + name: 'lastCompletionTime', + label: 'Last Completion', + }, + ], + rows: taskRows, + }; + + return { + ...defaultView('tasks', 'Tasks'), + content: { + itemTable: makeTableView(table), + postUrl: makeUrl(UrlType.Tasks), + csrfTag: await createCsrfTag(ctx), + }, + cssFiles: ['index/tasks'], + }; +}); + +export default router; diff --git a/packages/server/src/routes/routes.ts b/packages/server/src/routes/routes.ts index 745f60f60a..9246943a6c 100644 --- a/packages/server/src/routes/routes.ts +++ b/packages/server/src/routes/routes.ts @@ -1,31 +1,32 @@ import { Routers } from '../utils/routeUtils'; import apiBatch from './api/batch'; +import apiBatchItems from './api/batch_items'; import apiDebug from './api/debug'; import apiEvents from './api/events'; -import apiBatchItems from './api/batch_items'; import apiItems from './api/items'; import apiPing from './api/ping'; import apiSessions from './api/sessions'; -import apiUsers from './api/users'; import apiShares from './api/shares'; import apiShareUsers from './api/share_users'; +import apiUsers from './api/users'; import indexChanges from './index/changes'; +import indexHelp from './index/help'; import indexHome from './index/home'; import indexItems from './index/items'; import indexLogin from './index/login'; import indexLogout from './index/logout'; import indexNotifications from './index/notifications'; import indexPassword from './index/password'; -import indexSignup from './index/signup'; -import indexShares from './index/shares'; -import indexUsers from './index/users'; -import indexStripe from './index/stripe'; -import indexTerms from './index/terms'; import indexPrivacy from './index/privacy'; +import indexShares from './index/shares'; +import indexSignup from './index/signup'; +import indexStripe from './index/stripe'; +import indexTasks from './index/tasks'; +import indexTerms from './index/terms'; import indexUpgrade from './index/upgrade'; -import indexHelp from './index/help'; +import indexUsers from './index/users'; import defaultRoute from './default'; @@ -56,6 +57,7 @@ const routes: Routers = { 'privacy': indexPrivacy, 'upgrade': indexUpgrade, 'help': indexHelp, + 'tasks': indexTasks, '': defaultRoute, }; diff --git a/packages/server/src/services/CronService.ts b/packages/server/src/services/CronService.ts deleted file mode 100644 index 98f2d382fd..0000000000 --- a/packages/server/src/services/CronService.ts +++ /dev/null @@ -1,42 +0,0 @@ -import Logger from '@joplin/lib/Logger'; -import BaseService from './BaseService'; -const cron = require('node-cron'); - -const logger = Logger.create('cron'); - -async function runCronTask(name: string, callback: Function) { - const startTime = Date.now(); - logger.info(`Running task "${name}"`); - try { - await callback(); - } catch (error) { - logger.error(`On task "${name}"`, error); - } - logger.info(`Completed task "${name}" in ${Date.now() - startTime}ms`); -} - -export default class CronService extends BaseService { - - public async runInBackground() { - cron.schedule('0 */6 * * *', async () => { - await runCronTask('deleteExpiredTokens', async () => this.models.token().deleteExpiredTokens()); - }); - - cron.schedule('0 * * * *', async () => { - await runCronTask('updateTotalSizes', async () => this.models.item().updateTotalSizes()); - }); - - cron.schedule('0 12 * * *', async () => { - await runCronTask('handleBetaUserEmails', async () => this.models.user().handleBetaUserEmails()); - }); - - cron.schedule('0 13 * * *', async () => { - await runCronTask('handleFailedPaymentSubscriptions', async () => this.models.user().handleFailedPaymentSubscriptions()); - }); - - cron.schedule('0 14 * * *', async () => { - await runCronTask('handleOversizedAccounts', async () => this.models.user().handleOversizedAccounts()); - }); - } - -} diff --git a/packages/server/src/services/TaskService.test.ts b/packages/server/src/services/TaskService.test.ts new file mode 100644 index 0000000000..295105ace9 --- /dev/null +++ b/packages/server/src/services/TaskService.test.ts @@ -0,0 +1,82 @@ +import config from '../config'; +import { Models } from '../models/factory'; +import { afterAllTests, beforeAllDb, beforeEachDb, expectThrow, models, msleep } from '../utils/testing/testUtils'; +import { Env } from '../utils/types'; +import TaskService, { RunType, Task } from './TaskService'; + +const newService = () => { + return new TaskService(Env.Dev, models(), config()); +}; + +describe('TaskService', function() { + + beforeAll(async () => { + await beforeAllDb('TaskService'); + }); + + afterAll(async () => { + await afterAllTests(); + }); + + beforeEach(async () => { + await beforeEachDb(); + }); + + test('should register a task', async function() { + const service = newService(); + + const task: Task = { + id: 'test', + description: '', + run: (_models: Models) => {}, + schedule: '', + }; + + service.registerTask(task); + + expect(service.tasks['test']).toBeTruthy(); + await expectThrow(async () => service.registerTask(task)); + }); + + test('should run a task', async function() { + const service = newService(); + + let finishTask = false; + let taskHasRan = false; + + const task: Task = { + id: 'test', + description: '', + run: async (_models: Models) => { + const iid = setInterval(() => { + if (finishTask) { + clearInterval(iid); + taskHasRan = true; + } + }, 1); + }, + schedule: '', + }; + + service.registerTask(task); + + expect(service.taskState('test').running).toBe(false); + + const startTime = new Date(); + + void service.runTask('test', RunType.Manual); + expect(service.taskState('test').running).toBe(true); + expect(service.taskState('test').lastCompletionTime).toBeFalsy(); + expect(service.taskState('test').lastRunTime.getTime()).toBeGreaterThanOrEqual(startTime.getTime()); + + await msleep(1); + finishTask = true; + await msleep(3); + + expect(taskHasRan).toBe(true); + expect(service.taskState('test').running).toBe(false); + expect(service.taskState('test').lastCompletionTime.getTime()).toBeGreaterThan(startTime.getTime()); + + }); + +}); diff --git a/packages/server/src/services/TaskService.ts b/packages/server/src/services/TaskService.ts new file mode 100644 index 0000000000..ae427e7c50 --- /dev/null +++ b/packages/server/src/services/TaskService.ts @@ -0,0 +1,108 @@ +import Logger from '@joplin/lib/Logger'; +import { Models } from '../models/factory'; +import BaseService from './BaseService'; +const cron = require('node-cron'); + +const logger = Logger.create('TaskService'); + +type TaskId = string; + +export enum RunType { + Scheduled = 1, + Manual = 2, +} + +const runTypeToString = (runType: RunType) => { + if (runType === RunType.Scheduled) return 'scheduled'; + if (runType === RunType.Manual) return 'manual'; + throw new Error(`Unknown run type: ${runType}`); +}; + +export interface Task { + id: TaskId; + description: string; + schedule: string; + run(models: Models): void; +} + +export type Tasks = Record; + +interface TaskState { + running: boolean; + lastRunTime: Date; + lastCompletionTime: Date; +} + +const defaultTaskState: TaskState = { + running: false, + lastRunTime: null, + lastCompletionTime: null, +}; + +export default class TaskService extends BaseService { + + private tasks_: Tasks = {}; + private taskStates_: Record = {}; + + public registerTask(task: Task) { + if (this.tasks_[task.id]) throw new Error(`Already a task with this ID: ${task.id}`); + this.tasks_[task.id] = task; + this.taskStates_[task.id] = { ...defaultTaskState }; + } + + public registerTasks(tasks: Task[]) { + for (const task of tasks) this.registerTask(task); + } + + public get tasks(): Tasks { + return this.tasks_; + } + + public taskState(id: TaskId): TaskState { + if (!this.taskStates_[id]) throw new Error(`No such task: ${id}`); + return this.taskStates_[id]; + } + + // TODO: add tests + + public async runTask(id: TaskId, runType: RunType) { + const state = this.taskState(id); + if (state.running) throw new Error(`Task is already running: ${id}`); + + const startTime = Date.now(); + + this.taskStates_[id] = { + ...this.taskStates_[id], + running: true, + lastRunTime: new Date(), + }; + + try { + logger.info(`Running "${id}" (${runTypeToString(runType)})...`); + await this.tasks_[id].run(this.models); + } catch (error) { + logger.error(`On task "${id}"`, error); + } + + this.taskStates_[id] = { + ...this.taskStates_[id], + running: false, + lastCompletionTime: new Date(), + }; + + logger.info(`Completed "${id}" in ${Date.now() - startTime}ms`); + } + + public async runInBackground() { + for (const [taskId, task] of Object.entries(this.tasks_)) { + if (!task.schedule) continue; + + logger.info(`Scheduling task "${taskId}": ${task.schedule}`); + + cron.schedule(task.schedule, async () => { + await this.runTask(taskId, RunType.Scheduled); + }); + } + } + +} diff --git a/packages/server/src/services/database/types.ts b/packages/server/src/services/database/types.ts index 6029856e16..54bb987670 100644 --- a/packages/server/src/services/database/types.ts +++ b/packages/server/src/services/database/types.ts @@ -6,6 +6,7 @@ export enum ItemAddressingType { } export enum NotificationLevel { + Error = 5, Important = 10, Normal = 20, } diff --git a/packages/server/src/services/types.ts b/packages/server/src/services/types.ts index f30793eee4..02d2783777 100644 --- a/packages/server/src/services/types.ts +++ b/packages/server/src/services/types.ts @@ -1,11 +1,11 @@ -import CronService from './CronService'; import EmailService from './EmailService'; import MustacheService from './MustacheService'; import ShareService from './ShareService'; +import TaskService from './TaskService'; export interface Services { share: ShareService; email: EmailService; - cron: CronService; mustache: MustacheService; + tasks: TaskService; } diff --git a/packages/server/src/utils/routeUtils.ts b/packages/server/src/utils/routeUtils.ts index cd03862e60..fbb6714fd8 100644 --- a/packages/server/src/utils/routeUtils.ts +++ b/packages/server/src/utils/routeUtils.ts @@ -271,6 +271,7 @@ export enum UrlType { Login = 'login', Terms = 'terms', Privacy = 'privacy', + Tasks = 'tasks', } export function makeUrl(urlType: UrlType): string { diff --git a/packages/server/src/utils/setupAppContext.ts b/packages/server/src/utils/setupAppContext.ts index da274daeab..d3dce8e655 100644 --- a/packages/server/src/utils/setupAppContext.ts +++ b/packages/server/src/utils/setupAppContext.ts @@ -7,15 +7,15 @@ import routes from '../routes/routes'; import ShareService from '../services/ShareService'; import { Services } from '../services/types'; import EmailService from '../services/EmailService'; -import CronService from '../services/CronService'; import MustacheService from '../services/MustacheService'; +import setupTaskService from './setupTaskService'; async function setupServices(env: Env, models: Models, config: Config): Promise { const output: Services = { share: new ShareService(env, models, config), email: new EmailService(env, models, config), - cron: new CronService(env, models, config), mustache: new MustacheService(config.viewDir, config.baseUrl), + tasks: setupTaskService(env, models, config), }; await output.mustache.loadPartials(); diff --git a/packages/server/src/utils/setupTaskService.ts b/packages/server/src/utils/setupTaskService.ts new file mode 100644 index 0000000000..a7374fd3c5 --- /dev/null +++ b/packages/server/src/utils/setupTaskService.ts @@ -0,0 +1,49 @@ +import { Models } from '../models/factory'; +import TaskService, { Task } from '../services/TaskService'; +import { Config, Env } from './types'; + +export default function(env: Env, models: Models, config: Config): TaskService { + const taskService = new TaskService(env, models, config); + + let tasks: Task[] = [ + { + id: 'deleteExpiredTokens', + description: 'Delete expired tokens', + schedule: '0 */6 * * *', + run: (models: Models) => models.token().deleteExpiredTokens(), + }, + { + id: 'updateTotalSizes', + description: 'Update total sizes', + schedule: '0 * * * *', + run: (models: Models) => models.item().updateTotalSizes(), + }, + { + id: 'handleOversizedAccounts', + description: 'Process oversized accounts', + schedule: '0 14 * * *', + run: (models: Models) => models.user().handleOversizedAccounts(), + }, + ]; + + if (config.isJoplinCloud) { + tasks = tasks.concat([ + { + id: 'handleBetaUserEmails', + description: 'Process beta user emails', + schedule: '0 12 * * *', + run: (models: Models) => models.user().handleBetaUserEmails(), + }, + { + id: 'handleFailedPaymentSubscriptions', + description: 'Process failed payment subscriptions', + schedule: '0 13 * * *', + run: (models: Models) => models.user().handleFailedPaymentSubscriptions(), + }, + ]); + } + + taskService.registerTasks(tasks); + + return taskService; +} diff --git a/packages/server/src/utils/startServices.ts b/packages/server/src/utils/startServices.ts index 2d925a6e77..52f29f49e8 100644 --- a/packages/server/src/utils/startServices.ts +++ b/packages/server/src/utils/startServices.ts @@ -3,5 +3,5 @@ import { Services } from '../services/types'; export default async function startServices(services: Services) { void services.share.runInBackground(); void services.email.runInBackground(); - void services.cron.runInBackground(); + void services.tasks.runInBackground(); } diff --git a/packages/server/src/utils/time.ts b/packages/server/src/utils/time.ts index 03dc7c3866..5ce3fe5b27 100644 --- a/packages/server/src/utils/time.ts +++ b/packages/server/src/utils/time.ts @@ -29,7 +29,8 @@ export function msleep(ms: number) { }); } -export function formatDateTime(ms: number): string { +export function formatDateTime(ms: number | Date): string { + ms = ms instanceof Date ? ms.getTime() : ms; return `${dayjs(ms).format('D MMM YY HH:mm:ss')} (${defaultTimezone()})`; } diff --git a/packages/server/src/utils/types.ts b/packages/server/src/utils/types.ts index 437c9be7d6..6669f4693e 100644 --- a/packages/server/src/utils/types.ts +++ b/packages/server/src/utils/types.ts @@ -17,7 +17,7 @@ export enum Env { export interface NotificationView { id: Uuid; messageHtml: string; - level: string; + levelClassName: string; closeUrl: string; } diff --git a/packages/server/src/utils/views/table.ts b/packages/server/src/utils/views/table.ts index a60118bbea..859e446c78 100644 --- a/packages/server/src/utils/views/table.ts +++ b/packages/server/src/utils/views/table.ts @@ -4,11 +4,13 @@ import { setQueryParameters } from '../urlUtils'; const defaultSortOrder = PaginationOrderDir.ASC; function headerIsSelectedClass(name: string, pagination: Pagination): string { + if (!pagination) return ''; const orderBy = pagination.order[0].by; return name === orderBy ? 'is-selected' : ''; } function headerSortIconDir(name: string, pagination: Pagination): string { + if (!pagination) return ''; const orderBy = pagination.order[0].by; const orderDir = orderBy === name ? pagination.order[0].dir : defaultSortOrder; return orderDir === PaginationOrderDir.ASC ? 'up' : 'down'; @@ -35,6 +37,7 @@ interface HeaderView { interface RowItem { value: string; + checkbox?: boolean; url?: string; stretch?: boolean; } @@ -45,6 +48,7 @@ interface RowItemView { value: string; classNames: string[]; url: string; + checkbox: boolean; } type RowView = RowItemView[]; @@ -52,10 +56,10 @@ type RowView = RowItemView[]; export interface Table { headers: Header[]; rows: Row[]; - baseUrl: string; - requestQuery: any; - pageCount: number; - pagination: Pagination; + baseUrl?: string; + requestQuery?: any; + pageCount?: number; + pagination?: Pagination; } export interface TableView { @@ -77,7 +81,7 @@ export function makeTablePagination(query: any, defaultOrderField: string, defau function makeHeaderView(header: Header, parentBaseUrl: string, baseUrlQuery: PaginationQueryParams, pagination: Pagination): HeaderView { return { label: header.label, - sortLink: setQueryParameters(parentBaseUrl, { ...baseUrlQuery, 'order_by': header.name, 'order_dir': headerNextOrder(header.name, pagination) }), + sortLink: !pagination ? null : setQueryParameters(parentBaseUrl, { ...baseUrlQuery, 'order_by': header.name, 'order_dir': headerNextOrder(header.name, pagination) }), classNames: [header.stretch ? 'stretch' : 'nowrap', headerIsSelectedClass(header.name, pagination)], iconDir: headerSortIconDir(header.name, pagination), }; @@ -89,14 +93,21 @@ function makeRowView(row: Row): RowView { value: rowItem.value, classNames: [rowItem.stretch ? 'stretch' : 'nowrap'], url: rowItem.url, + checkbox: rowItem.checkbox, }; }); } export function makeTableView(table: Table): TableView { - const baseUrlQuery = filterPaginationQueryParams(table.requestQuery); - const pagination = table.pagination; - const paginationLinks = createPaginationLinks(pagination.page, table.pageCount, setQueryParameters(table.baseUrl, { ...baseUrlQuery, 'page': 'PAGE_NUMBER' })); + let paginationLinks: PageLink[] = []; + let baseUrlQuery: PaginationQueryParams = null; + let pagination: Pagination = null; + + if (table.pageCount) { + baseUrlQuery = filterPaginationQueryParams(table.requestQuery); + pagination = table.pagination; + paginationLinks = createPaginationLinks(pagination.page, table.pageCount, setQueryParameters(table.baseUrl, { ...baseUrlQuery, 'page': 'PAGE_NUMBER' })); + } return { headers: table.headers.map(h => makeHeaderView(h, table.baseUrl, baseUrlQuery, pagination)), diff --git a/packages/server/src/views/index/tasks.mustache b/packages/server/src/views/index/tasks.mustache new file mode 100644 index 0000000000..924dc981e3 --- /dev/null +++ b/packages/server/src/views/index/tasks.mustache @@ -0,0 +1,11 @@ +
+ {{{csrfTag}}} + + {{#itemTable}} + {{>table}} + {{/itemTable}} + +
+ +
+
\ No newline at end of file diff --git a/packages/server/src/views/partials/navbar.mustache b/packages/server/src/views/partials/navbar.mustache index 39bbdf4068..d923fece6a 100644 --- a/packages/server/src/views/partials/navbar.mustache +++ b/packages/server/src/views/partials/navbar.mustache @@ -16,6 +16,9 @@ {{/global.owner.is_admin}} Items Log + {{#global.owner.is_admin}} + Tasks + {{/global.owner.is_admin}}
{{#global.isJoplinCloud}} diff --git a/packages/server/src/views/partials/notifications.mustache b/packages/server/src/views/partials/notifications.mustache index 25dd91a37e..d517598990 100644 --- a/packages/server/src/views/partials/notifications.mustache +++ b/packages/server/src/views/partials/notifications.mustache @@ -1,6 +1,6 @@ {{#global.hasNotifications}} {{#global.notifications}} -
+
{{{messageHtml}}}
diff --git a/packages/server/src/views/partials/table.mustache b/packages/server/src/views/partials/table.mustache index b9cda365fc..42b77eb7f6 100644 --- a/packages/server/src/views/partials/table.mustache +++ b/packages/server/src/views/partials/table.mustache @@ -1,20 +1,22 @@ - - - - {{#headers}} - {{>tableHeader}} - {{/headers}} - - - - {{#rows}} +
+
+ - {{#.}} - {{>tableRowItem}} - {{/.}} + {{#headers}} + {{>tableHeader}} + {{/headers}} - {{/rows}} - -
+ + + {{#rows}} + + {{#.}} + {{>tableRowItem}} + {{/.}} + + {{/rows}} + + +
{{>pagination}} \ No newline at end of file diff --git a/packages/server/src/views/partials/tableHeader.mustache b/packages/server/src/views/partials/tableHeader.mustache index fef7e2a96f..4911dfbe35 100644 --- a/packages/server/src/views/partials/tableHeader.mustache +++ b/packages/server/src/views/partials/tableHeader.mustache @@ -1,3 +1,8 @@ - {{label}} + {{#sortLink}} + {{label}} + {{/sortLink}} + {{^sortLink}} + {{label}} + {{/sortLink}} \ No newline at end of file diff --git a/packages/server/src/views/partials/tableRowItem.mustache b/packages/server/src/views/partials/tableRowItem.mustache index fc366b6ae9..1a2240973c 100644 --- a/packages/server/src/views/partials/tableRowItem.mustache +++ b/packages/server/src/views/partials/tableRowItem.mustache @@ -1,3 +1,8 @@ - {{#url}}{{/url}}{{value}} + {{#checkbox}} + + {{/checkbox}} + {{^checkbox}} + {{#url}}{{/url}}{{value}} + {{/checkbox}} \ No newline at end of file From 024967ce60e4d9724e4c6173f8aaf5992a8b8168 Mon Sep 17 00:00:00 2001 From: Laurent Cozic Date: Sat, 18 Sep 2021 14:46:10 +0100 Subject: [PATCH 05/15] Server: Fixed calculating total item size after an item has been deleted --- packages/server/src/models/ItemModel.test.ts | 28 ++++++++++++++++++++ packages/server/src/models/ItemModel.ts | 6 ++++- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/packages/server/src/models/ItemModel.test.ts b/packages/server/src/models/ItemModel.test.ts index cab1681005..b4efbfd311 100644 --- a/packages/server/src/models/ItemModel.test.ts +++ b/packages/server/src/models/ItemModel.test.ts @@ -196,6 +196,34 @@ describe('ItemModel', function() { expect((await models().user().load(user3.id)).total_item_size).toBe(totalSize3); }); + test('should update total size when an item is deleted', async function() { + const { user: user1 } = await createUserAndSession(1); + + await createItemTree3(user1.id, '', '', [ + { + id: '000000000000000000000000000000F1', + children: [ + { + id: '00000000000000000000000000000001', + }, + ], + }, + ]); + + const folder1 = await models().item().loadByJopId(user1.id, '000000000000000000000000000000F1'); + const note1 = await models().item().loadByJopId(user1.id, '00000000000000000000000000000001'); + + await models().item().updateTotalSizes(); + + expect((await models().user().load(user1.id)).total_item_size).toBe(folder1.content_size + note1.content_size); + + await models().item().delete(note1.id); + + await models().item().updateTotalSizes(); + + expect((await models().user().load(user1.id)).total_item_size).toBe(folder1.content_size); + }); + test('should include shared items in total size calculation', async function() { const { user: user1, session: session1 } = await createUserAndSession(1); const { user: user2, session: session2 } = await createUserAndSession(2); diff --git a/packages/server/src/models/ItemModel.ts b/packages/server/src/models/ItemModel.ts index c54546cfa5..cdf06d0cdb 100644 --- a/packages/server/src/models/ItemModel.ts +++ b/packages/server/src/models/ItemModel.ts @@ -623,7 +623,11 @@ export default class ItemModel extends BaseModel { } else { const itemIds: Uuid[] = unique(changes.map(c => c.item_id)); const userItems: UserItem[] = await this.db('user_items').select('user_id').whereIn('item_id', itemIds); - const userIds: Uuid[] = unique(userItems.map(u => u.user_id)); + const userIds: Uuid[] = unique( + userItems + .map(u => u.user_id) + .concat(changes.map(c => c.user_id)) + ); const totalSizes: TotalSizeRow[] = []; for (const userId of userIds) { From 43c594b6b21b703f7cb5c0ca32f382cc2d32d61c Mon Sep 17 00:00:00 2001 From: Laurent Cozic Date: Sat, 18 Sep 2021 15:02:24 +0100 Subject: [PATCH 06/15] Server, Desktop: Sync deleted items first to allow fixing oversized accounts --- packages/lib/Synchronizer.ts | 50 ++++++++++++------------- packages/server/src/models/UserModel.ts | 24 +++++++----- packages/server/src/routes/api/items.ts | 4 +- packages/server/src/utils/errors.ts | 14 +++++++ 4 files changed, 57 insertions(+), 35 deletions(-) diff --git a/packages/lib/Synchronizer.ts b/packages/lib/Synchronizer.ts index 0f929fd0ed..c4e314e3ec 100644 --- a/packages/lib/Synchronizer.ts +++ b/packages/lib/Synchronizer.ts @@ -479,6 +479,31 @@ export default class Synchronizer { void this.cancel(); }); + // ======================================================================== + // 2. DELETE_REMOTE + // ------------------------------------------------------------------------ + // Delete the remote items that have been deleted locally. + // ======================================================================== + + if (syncSteps.indexOf('delete_remote') >= 0) { + const deletedItems = await BaseItem.deletedItems(syncTargetId); + for (let i = 0; i < deletedItems.length; i++) { + if (this.cancelling()) break; + + const item = deletedItems[i]; + const path = BaseItem.systemPath(item.item_id); + this.logSyncOperation('deleteRemote', null, { id: item.item_id }, 'local has been deleted'); + await this.apiCall('delete', path); + + if (item.item_type === BaseModel.TYPE_RESOURCE) { + const remoteContentPath = resourceRemotePath(item.item_id); + await this.apiCall('delete', remoteContentPath); + } + + await BaseItem.remoteDeletedItem(syncTargetId, item.item_id); + } + } // DELETE_REMOTE STEP + // ======================================================================== // 1. UPLOAD // ------------------------------------------------------------------------ @@ -763,31 +788,6 @@ export default class Synchronizer { } } // UPLOAD STEP - // ======================================================================== - // 2. DELETE_REMOTE - // ------------------------------------------------------------------------ - // Delete the remote items that have been deleted locally. - // ======================================================================== - - if (syncSteps.indexOf('delete_remote') >= 0) { - const deletedItems = await BaseItem.deletedItems(syncTargetId); - for (let i = 0; i < deletedItems.length; i++) { - if (this.cancelling()) break; - - const item = deletedItems[i]; - const path = BaseItem.systemPath(item.item_id); - this.logSyncOperation('deleteRemote', null, { id: item.item_id }, 'local has been deleted'); - await this.apiCall('delete', path); - - if (item.item_type === BaseModel.TYPE_RESOURCE) { - const remoteContentPath = resourceRemotePath(item.item_id); - await this.apiCall('delete', remoteContentPath); - } - - await BaseItem.remoteDeletedItem(syncTargetId, item.item_id); - } - } // DELETE_REMOTE STEP - // ------------------------------------------------------------------------ // 3. DELTA // ------------------------------------------------------------------------ diff --git a/packages/server/src/models/UserModel.ts b/packages/server/src/models/UserModel.ts index 921ed429d3..c8e1b575f1 100644 --- a/packages/server/src/models/UserModel.ts +++ b/packages/server/src/models/UserModel.ts @@ -180,7 +180,7 @@ export default class UserModel extends BaseModel { } public async checkMaxItemSizeLimit(user: User, buffer: Buffer, item: Item, joplinItem: any) { - // If the item is encrypted, we apply a multipler because encrypted + // If the item is encrypted, we apply a multiplier because encrypted // items can be much larger (seems to be up to twice the size but for // safety let's go with 2.2). @@ -198,14 +198,20 @@ export default class UserModel extends BaseModel { )); } - // Also apply a multiplier to take into account E2EE overhead - const maxTotalItemSize = getMaxTotalItemSize(user) * 1.5; - if (maxTotalItemSize && user.total_item_size + itemSize >= maxTotalItemSize) { - throw new ErrorPayloadTooLarge(_('Cannot save %s "%s" because it would go over the total allowed size (%s) for this account', - isNote ? _('note') : _('attachment'), - itemTitle ? itemTitle : item.name, - formatBytes(maxTotalItemSize) - )); + // We allow lock files to go through so that sync can happen, which in + // turns allow user to fix oversized account by deleting items. + const isWhiteListed = itemSize < 200 && item.name.startsWith('locks/'); + + if (!isWhiteListed) { + // Also apply a multiplier to take into account E2EE overhead + const maxTotalItemSize = getMaxTotalItemSize(user) * 1.5; + if (maxTotalItemSize && user.total_item_size + itemSize >= maxTotalItemSize) { + throw new ErrorPayloadTooLarge(_('Cannot save %s "%s" because it would go over the total allowed size (%s) for this account', + isNote ? _('note') : _('attachment'), + itemTitle ? itemTitle : item.name, + formatBytes(maxTotalItemSize) + )); + } } } diff --git a/packages/server/src/routes/api/items.ts b/packages/server/src/routes/api/items.ts index 5ac8e945f2..04a1d231ca 100644 --- a/packages/server/src/routes/api/items.ts +++ b/packages/server/src/routes/api/items.ts @@ -5,7 +5,7 @@ import Router from '../../utils/Router'; import { RouteType } from '../../utils/types'; import { AppContext } from '../../utils/types'; import * as fs from 'fs-extra'; -import { ErrorForbidden, ErrorMethodNotAllowed, ErrorNotFound, ErrorPayloadTooLarge } from '../../utils/errors'; +import { ErrorForbidden, ErrorMethodNotAllowed, ErrorNotFound, ErrorPayloadTooLarge, errorToPlainObject } from '../../utils/errors'; import ItemModel, { ItemSaveOption, SaveFromRawContentItem } from '../../models/ItemModel'; import { requestDeltaPagination, requestPagination } from '../../models/utils/pagination'; import { AclAction } from '../../models/BaseModel'; @@ -66,7 +66,9 @@ export async function putItemContents(path: SubPath, ctx: AppContext, isBatch: b const output = await ctx.joplin.models.item().saveFromRawContent(ctx.joplin.owner, items, saveOptions); for (const [name] of Object.entries(output)) { if (output[name].item) output[name].item = ctx.joplin.models.item().toApiOutput(output[name].item) as Item; + if (output[name].error) output[name].error = errorToPlainObject(output[name].error); } + return output; } diff --git a/packages/server/src/utils/errors.ts b/packages/server/src/utils/errors.ts index 0124ee6935..5b7cd7e53b 100644 --- a/packages/server/src/utils/errors.ts +++ b/packages/server/src/utils/errors.ts @@ -114,3 +114,17 @@ export function errorToString(error: Error): string { if (error.stack) msg.push(error.stack); return msg.join(': '); } + +interface PlainObjectError { + httpCode?: number; + message?: string; + code?: string; +} + +export function errorToPlainObject(error: any): PlainObjectError { + const output: PlainObjectError = {}; + if ('httpCode' in error) output.httpCode = error.httpCode; + if ('code' in error) output.code = error.code; + if ('message' in error) output.message = error.message; + return output; +} From c99aba0dff1d285c5105636b31cdecdd55615ab7 Mon Sep 17 00:00:00 2001 From: a1346054 <36859588+a1346054@users.noreply.github.com> Date: Sun, 19 Sep 2021 01:59:26 +0000 Subject: [PATCH 07/15] Desktop: Linux: Installer: properly quote variables (#5476) --- Joplin_install_and_update.sh | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Joplin_install_and_update.sh b/Joplin_install_and_update.sh index e928adedea..71043648a8 100755 --- a/Joplin_install_and_update.sh +++ b/Joplin_install_and_update.sh @@ -137,8 +137,8 @@ fi #----------------------------------------------------- print 'Downloading Joplin...' TEMP_DIR=$(mktemp -d) -wget -O ${TEMP_DIR}/Joplin.AppImage https://github.com/laurent22/joplin/releases/download/v${RELEASE_VERSION}/Joplin-${RELEASE_VERSION}.AppImage -wget -O ${TEMP_DIR}/joplin.png https://joplinapp.org/images/Icon512.png +wget -O "${TEMP_DIR}/Joplin.AppImage" "https://github.com/laurent22/joplin/releases/download/v${RELEASE_VERSION}/Joplin-${RELEASE_VERSION}.AppImage" +wget -O "${TEMP_DIR}/joplin.png" https://joplinapp.org/images/Icon512.png #----------------------------------------------------- print 'Installing Joplin...' @@ -149,7 +149,7 @@ rm -f ~/.joplin/*.AppImage ~/.local/share/applications/joplin.desktop ~/.joplin/ mkdir -p ~/.joplin/ # Download the latest version -mv ${TEMP_DIR}/Joplin.AppImage ~/.joplin/Joplin.AppImage +mv "${TEMP_DIR}/Joplin.AppImage" ~/.joplin/Joplin.AppImage # Gives execution privileges chmod +x ~/.joplin/Joplin.AppImage @@ -159,7 +159,7 @@ print "${COLOR_GREEN}OK${COLOR_RESET}" #----------------------------------------------------- print 'Installing icon...' mkdir -p ~/.local/share/icons/hicolor/512x512/apps -mv ${TEMP_DIR}/joplin.png ~/.local/share/icons/hicolor/512x512/apps/joplin.png +mv "${TEMP_DIR}/joplin.png" ~/.local/share/icons/hicolor/512x512/apps/joplin.png print "${COLOR_GREEN}OK${COLOR_RESET}" # Detect desktop environment @@ -222,7 +222,7 @@ fi print "${COLOR_GREEN}Joplin version${COLOR_RESET} ${RELEASE_VERSION} ${COLOR_GREEN}installed.${COLOR_RESET}" # Record version -echo $RELEASE_VERSION > ~/.joplin/VERSION +echo "$RELEASE_VERSION" > ~/.joplin/VERSION #----------------------------------------------------- if [[ "$SHOW_CHANGELOG" == true ]]; then @@ -232,5 +232,5 @@ fi #----------------------------------------------------- print "Cleaning up..." -rm -rf $TEMP_DIR +rm -rf "$TEMP_DIR" print "${COLOR_GREEN}OK${COLOR_RESET}" From afe1cf747d555e0a085f39898c664c2cac18c32d Mon Sep 17 00:00:00 2001 From: JackGruber <24863925+JackGruber@users.noreply.github.com> Date: Sun, 19 Sep 2021 12:31:38 +0200 Subject: [PATCH 08/15] All: Fixes #5444: Misinterpreted search term after filter in quotation marks (#5445) --- .../searchengine/SearchFilter.test.js | 28 +++++++++++++++++++ .../lib/services/searchengine/filterParser.ts | 1 + 2 files changed, 29 insertions(+) diff --git a/packages/lib/services/searchengine/SearchFilter.test.js b/packages/lib/services/searchengine/SearchFilter.test.js index 9cf8a8505a..9a75f46c8c 100644 --- a/packages/lib/services/searchengine/SearchFilter.test.js +++ b/packages/lib/services/searchengine/SearchFilter.test.js @@ -386,6 +386,34 @@ describe('services_SearchFilter', function() { expect(ids(rows).sort()).toEqual(ids(notes0).concat(ids(notes00).concat(ids(notes1))).sort()); })); + it('should support filtering and search term', (async () => { + const notebook1 = await Folder.save({ title: 'notebook1' }); + const notebook2 = await Folder.save({ title: 'notebook2' }); + const note1 = await Note.save({ title: 'note1', body: 'abcdefg', parent_id: notebook1.id }); + await Note.save({ title: 'note2', body: 'body', parent_id: notebook1.id }); + await Note.save({ title: 'note3', body: 'abcdefg', parent_id: notebook2.id }); + await Note.save({ title: 'note4', body: 'body', parent_id: notebook2.id }); + + await engine.syncTables(); + + const testCases = [ + { searchQuery: 'notebook:notebook1 abcdefg' }, + { searchQuery: 'notebook:notebook1 "abcdefg"' }, + { searchQuery: 'notebook:"notebook1" abcdefg' }, + { searchQuery: 'notebook:"notebook1" "abcdefg"' }, + { searchQuery: 'notebook:"notebook1" -tag:* "abcdefg"' }, + { searchQuery: 'notebook:"notebook1" -tag:* abcdefg' }, + { searchQuery: 'notebook:"notebook1" -tag:"*" abcdefg' }, + { searchQuery: 'notebook:"notebook1" -tag:"*" "abcdefg"' }, + ]; + + for (const testCase of testCases) { + const rows = await engine.search(testCase.searchQuery, { searchType }); + expect(rows.length).toBe(1); + expect(ids(rows)).toContain(note1.id); + } + })); + it('should support filtering by created date', (async () => { let rows; const n1 = await Note.save({ title: 'I made this on', body: 'May 20 2020', user_created_time: Date.parse('2020-05-20') }); diff --git a/packages/lib/services/searchengine/filterParser.ts b/packages/lib/services/searchengine/filterParser.ts index b1dd8dddff..82a0aa5587 100644 --- a/packages/lib/services/searchengine/filterParser.ts +++ b/packages/lib/services/searchengine/filterParser.ts @@ -36,6 +36,7 @@ const getTerms = (query: string, validFilters: Set): Term[] => { if (inQuote) { terms.push(makeTerm(currentCol, currentTerm)); currentTerm = ''; + currentCol = '_'; inQuote = false; } else { inQuote = true; From 12ec7b0c1dce48bef11bffe8b26626c44f4fe2c2 Mon Sep 17 00:00:00 2001 From: "Helmut K. C. Tessarek" Date: Sun, 19 Sep 2021 06:32:58 -0400 Subject: [PATCH 09/15] macOS: Added Cmd+Backspace shortcut to delete line (#5478) --- .../gui/NoteEditor/NoteBody/CodeMirror/utils/useKeymap.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/utils/useKeymap.ts b/packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/utils/useKeymap.ts index eb6f80a956..f65d7dadc2 100644 --- a/packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/utils/useKeymap.ts +++ b/packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/utils/useKeymap.ts @@ -171,6 +171,7 @@ export default function useKeymap(CodeMirror: any) { 'Cmd-Right': 'goLineRightSmart', 'Alt-Backspace': 'delGroupBefore', 'Alt-Delete': 'delGroupAfter', + 'Cmd-Backspace': 'delWrappedLineLeft', 'fallthrough': 'basic', }; From 2d72d1435e2c8b76572723e7bd61d6c7272404a8 Mon Sep 17 00:00:00 2001 From: Marph Date: Sun, 19 Sep 2021 13:34:04 +0300 Subject: [PATCH 10/15] Desktop: Support for user-data-dir flag (#5467) This flag is passed by chromedriver. --- packages/lib/BaseApplication.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/lib/BaseApplication.ts b/packages/lib/BaseApplication.ts index de94b4f87f..ecd2eb47da 100644 --- a/packages/lib/BaseApplication.ts +++ b/packages/lib/BaseApplication.ts @@ -251,6 +251,12 @@ export default class BaseApplication { continue; } + if (arg.indexOf('--user-data-dir=') === 0) { + // Electron-specific flag. Allows users to run the app with chromedriver. + argv.splice(0, 1); + continue; + } + if (arg.length && arg[0] == '-') { throw new JoplinError(_('Unknown flag: %s', arg), 'flagError'); } else { From 7a9ec627ee63f6640be9bfecac5fe06170319997 Mon Sep 17 00:00:00 2001 From: Caleb John Date: Sun, 19 Sep 2021 03:36:23 -0700 Subject: [PATCH 11/15] Desktop: Resolves #5233: Fire resize event whenever the layout changes (#5344) * Fire resize event whenever the layout changes This solves an issue where the markdown editor was changing size physically, but the refresh function wasn't being called so the editor would lose track of it's size and place the cursor wrongly. The editor was able to correctly resize when the window resize event was fired, but this didn't happen when the sidebars were toggled. The solution implemented here is to hook in to the function where layout props are changed, and emit a resize event there. This means that anytime the layout changes (whether or not it affects sizing), the resize event will be fired. --- .../gui/MainScreen/commands/toggleNoteList.ts | 5 +++++ .../gui/MainScreen/commands/toggleSideBar.ts | 5 +++++ .../NoteBody/CodeMirror/CodeMirror.tsx | 5 +++-- .../NoteBody/CodeMirror/utils/index.ts | 20 +------------------ 4 files changed, 14 insertions(+), 21 deletions(-) diff --git a/packages/app-desktop/gui/MainScreen/commands/toggleNoteList.ts b/packages/app-desktop/gui/MainScreen/commands/toggleNoteList.ts index c7d169c1d6..7730634246 100644 --- a/packages/app-desktop/gui/MainScreen/commands/toggleNoteList.ts +++ b/packages/app-desktop/gui/MainScreen/commands/toggleNoteList.ts @@ -19,6 +19,11 @@ export const runtime = (): CommandRuntime => { visible: !layoutItemProp(layout, 'noteList', 'visible'), }); + // Toggling the sidebar will affect the size of most other on-screen components. + // Dispatching a window resize event is a bit of a hack, but it ensures that any + // component that watches for resizes will be accurately notified + window.dispatchEvent(new Event('resize')); + context.dispatch({ type: 'MAIN_LAYOUT_SET', value: newLayout, diff --git a/packages/app-desktop/gui/MainScreen/commands/toggleSideBar.ts b/packages/app-desktop/gui/MainScreen/commands/toggleSideBar.ts index 1079949b8f..698896ee1a 100644 --- a/packages/app-desktop/gui/MainScreen/commands/toggleSideBar.ts +++ b/packages/app-desktop/gui/MainScreen/commands/toggleSideBar.ts @@ -19,6 +19,11 @@ export const runtime = (): CommandRuntime => { visible: !layoutItemProp(layout, 'sideBar', 'visible'), }); + // Toggling the sidebar will affect the size of most other on-screen components. + // Dispatching a window resize event is a bit of a hack, but it ensures that any + // component that watches for resizes will be accurately notified + window.dispatchEvent(new Event('resize')); + context.dispatch({ type: 'MAIN_LAYOUT_SET', value: newLayout, diff --git a/packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/CodeMirror.tsx b/packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/CodeMirror.tsx index fca3a23874..edb28bfa39 100644 --- a/packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/CodeMirror.tsx +++ b/packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/CodeMirror.tsx @@ -6,7 +6,8 @@ import { EditorCommand, NoteBodyEditorProps } from '../../utils/types'; import { commandAttachFileToBody, handlePasteEvent } from '../../utils/resourceHandling'; import { ScrollOptions, ScrollOptionTypes } from '../../utils/types'; import { CommandValue } from '../../utils/types'; -import { useScrollHandler, usePrevious, cursorPositionToTextOffset, useRootSize } from './utils'; +import { useScrollHandler, usePrevious, cursorPositionToTextOffset } from './utils'; +import useElementSize from '@joplin/lib/hooks/useElementSize'; import Toolbar from './Toolbar'; import styles_ from './styles'; import { RenderedBody, defaultRenderedBody } from './utils/types'; @@ -59,7 +60,7 @@ function CodeMirror(props: NoteBodyEditorProps, ref: any) { const props_onChangeRef = useRef(null); props_onChangeRef.current = props.onChange; - const rootSize = useRootSize({ rootRef }); + const rootSize = useElementSize(rootRef); usePluginServiceRegistration(ref); diff --git a/packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/utils/index.ts b/packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/utils/index.ts index 3918c04fda..8767410f5e 100644 --- a/packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/utils/index.ts +++ b/packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/utils/index.ts @@ -1,4 +1,4 @@ -import { useEffect, useCallback, useRef, useState } from 'react'; +import { useEffect, useCallback, useRef } from 'react'; import shim from '@joplin/lib/shim'; export function cursorPositionToTextOffset(cursorPos: any, body: string) { @@ -89,21 +89,3 @@ export function useScrollHandler(editorRef: any, webviewRef: any, onScroll: Func return { resetScroll, setEditorPercentScroll, setViewerPercentScroll, editor_scroll }; } - -export function useRootSize(dependencies: any) { - const { rootRef } = dependencies; - - const [rootSize, setRootSize] = useState({ width: 0, height: 0 }); - - useEffect(() => { - if (!rootRef.current) return; - - const { width, height } = rootRef.current.getBoundingClientRect(); - - if (rootSize.width !== width || rootSize.height !== height) { - setRootSize({ width: width, height: height }); - } - }); - - return rootSize; -} From 0d40026d8bbee3af1b9aa008bee7134d4ec351b6 Mon Sep 17 00:00:00 2001 From: Nikhil Gautam <40800932+nik-gautam@users.noreply.github.com> Date: Sun, 19 Sep 2021 16:07:33 +0530 Subject: [PATCH 12/15] Desktop: Resolves #5299: Display 0/0 when no search results are found in editor (#5360) --- packages/app-desktop/gui/NoteSearchBar.jsx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/app-desktop/gui/NoteSearchBar.jsx b/packages/app-desktop/gui/NoteSearchBar.jsx index 5156a0b3f8..15f398b1a0 100644 --- a/packages/app-desktop/gui/NoteSearchBar.jsx +++ b/packages/app-desktop/gui/NoteSearchBar.jsx @@ -139,9 +139,10 @@ class NoteSearchBarComponent extends React.Component { color: theme.colorFaded, backgroundColor: theme.backgroundColor, }); - const matchesFoundString = (query.length > 0 && this.props.resultCount > 0) ? ( + + const matchesFoundString = (query.length > 0) ? (
- {`${this.props.selectedIndex + 1} / ${this.props.resultCount}`} + {`${this.props.resultCount === 0 ? 0 : this.props.selectedIndex + 1} / ${this.props.resultCount}`}
) : null; From 6d981864effd692fd12ab14f75ca17f6ca53fd75 Mon Sep 17 00:00:00 2001 From: Laurent Cozic Date: Sun, 19 Sep 2021 12:03:16 +0100 Subject: [PATCH 13/15] Desktop: Fixes #5461: Editor max width was not always applied in Rich Text editor --- .../gui/NoteEditor/NoteBody/TinyMCE/TinyMCE.tsx | 2 +- packages/app-desktop/gui/NoteEditor/NoteEditor.tsx | 9 ++++++--- packages/app-desktop/gui/NoteEditor/utils/types.ts | 6 +++++- packages/renderer/noteStyle.ts | 4 +++- 4 files changed, 15 insertions(+), 6 deletions(-) diff --git a/packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/TinyMCE.tsx b/packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/TinyMCE.tsx index 64b1c2b9c5..10188d4f77 100644 --- a/packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/TinyMCE.tsx +++ b/packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/TinyMCE.tsx @@ -793,7 +793,7 @@ const TinyMCE = (props: NoteBodyEditorProps, ref: any) => { }; } - await loadDocumentAssets(editor, await props.allAssets(props.contentMarkupLanguage)); + await loadDocumentAssets(editor, await props.allAssets(props.contentMarkupLanguage, { contentMaxWidthTarget: '.mce-content-body' })); dispatchDidUpdate(editor); }; diff --git a/packages/app-desktop/gui/NoteEditor/NoteEditor.tsx b/packages/app-desktop/gui/NoteEditor/NoteEditor.tsx index dfa6135beb..d2d2722c41 100644 --- a/packages/app-desktop/gui/NoteEditor/NoteEditor.tsx +++ b/packages/app-desktop/gui/NoteEditor/NoteEditor.tsx @@ -14,7 +14,7 @@ import useMarkupToHtml from './utils/useMarkupToHtml'; import useFormNote, { OnLoadEvent } from './utils/useFormNote'; import useFolder from './utils/useFolder'; import styles_ from './styles'; -import { NoteEditorProps, FormNote, ScrollOptions, ScrollOptionTypes, OnChangeEvent, NoteBodyEditorProps } from './utils/types'; +import { NoteEditorProps, FormNote, ScrollOptions, ScrollOptionTypes, OnChangeEvent, NoteBodyEditorProps, AllAssetsOptions } from './utils/types'; import ResourceEditWatcher from '@joplin/lib/services/ResourceEditWatcher/index'; import CommandService from '@joplin/lib/services/CommandService'; import ToolbarButton from '../ToolbarButton/ToolbarButton'; @@ -151,7 +151,7 @@ function NoteEditor(props: NoteEditorProps) { plugins: props.plugins, }); - const allAssets = useCallback(async (markupLanguage: number): Promise => { + const allAssets = useCallback(async (markupLanguage: number, options: AllAssetsOptions = null): Promise => { const theme = themeStyle(props.themeId); const markupToHtml = markupLanguageUtils.newMarkupToHtml({}, { @@ -159,7 +159,10 @@ function NoteEditor(props: NoteEditorProps) { customCss: props.customCss, }); - return markupToHtml.allAssets(markupLanguage, theme, { contentMaxWidth: props.contentMaxWidth }); + return markupToHtml.allAssets(markupLanguage, theme, { + contentMaxWidth: props.contentMaxWidth, + contentMaxWidthTarget: options.contentMaxWidthTarget, + }); }, [props.themeId, props.customCss, props.contentMaxWidth]); const handleProvisionalFlag = useCallback(() => { diff --git a/packages/app-desktop/gui/NoteEditor/utils/types.ts b/packages/app-desktop/gui/NoteEditor/utils/types.ts index 0fa19bde13..24d9c39b4b 100644 --- a/packages/app-desktop/gui/NoteEditor/utils/types.ts +++ b/packages/app-desktop/gui/NoteEditor/utils/types.ts @@ -6,6 +6,10 @@ import { MarkupLanguage } from '@joplin/renderer'; import { RenderResult, RenderResultPluginAsset } from '@joplin/renderer/MarkupToHtml'; import { MarkupToHtmlOptions } from './useMarkupToHtml'; +export interface AllAssetsOptions { + contentMaxWidthTarget?: string; +} + export interface ToolbarButtonInfos { [key: string]: ToolbarButtonInfo; } @@ -55,7 +59,7 @@ export interface NoteBodyEditorProps { onScroll(event: any): void; markupToHtml: (markupLanguage: MarkupLanguage, markup: string, options: MarkupToHtmlOptions)=> Promise; htmlToMarkdown: Function; - allAssets: (markupLanguage: MarkupLanguage)=> Promise; + allAssets: (markupLanguage: MarkupLanguage, options: AllAssetsOptions)=> Promise; disabled: boolean; dispatch: Function; noteToolbar: any; diff --git a/packages/renderer/noteStyle.ts b/packages/renderer/noteStyle.ts index d1bbe1749b..e8f695e819 100644 --- a/packages/renderer/noteStyle.ts +++ b/packages/renderer/noteStyle.ts @@ -9,6 +9,7 @@ function formatCssSize(v: any): string { export interface Options { contentMaxWidth?: number; + contentMaxWidthTarget?: string; } export default function(theme: any, options: Options = null) { @@ -21,8 +22,9 @@ export default function(theme: any, options: Options = null) { const fontFamily = '\'Avenir\', \'Arial\', sans-serif'; + const maxWidthTarget = options.contentMaxWidthTarget ? options.contentMaxWidthTarget : '#rendered-md'; const maxWidthCss = options.contentMaxWidth ? ` - #rendered-md { + ${maxWidthTarget} { max-width: ${options.contentMaxWidth}px; margin-left: auto; margin-right: auto; From 87f83236cf62236af0c8aa43c6394a0c39396ff3 Mon Sep 17 00:00:00 2001 From: Laurent Cozic Date: Sun, 19 Sep 2021 12:35:06 +0100 Subject: [PATCH 14/15] Desktop: Fixes #5480: Underline was not applied when using Cmd+U in Rich Text editor --- .../html_to_md/sub_sup_insert_strikethrough.html | 2 +- .../html_to_md/sub_sup_insert_strikethrough.md | 2 +- packages/turndown/src/commonmark-rules.js | 12 ++++++++++-- packages/turndown/src/utilities.js | 13 +++++++++++++ 4 files changed, 25 insertions(+), 4 deletions(-) diff --git a/packages/app-cli/tests/html_to_md/sub_sup_insert_strikethrough.html b/packages/app-cli/tests/html_to_md/sub_sup_insert_strikethrough.html index d1149f97cc..10db976c29 100644 --- a/packages/app-cli/tests/html_to_md/sub_sup_insert_strikethrough.html +++ b/packages/app-cli/tests/html_to_md/sub_sup_insert_strikethrough.html @@ -1 +1 @@ -X1 X1 Insert Strike \ No newline at end of file +X1 X1 Insert Insert alt Strike \ No newline at end of file diff --git a/packages/app-cli/tests/html_to_md/sub_sup_insert_strikethrough.md b/packages/app-cli/tests/html_to_md/sub_sup_insert_strikethrough.md index 2a17e25d65..2f15f103bc 100644 --- a/packages/app-cli/tests/html_to_md/sub_sup_insert_strikethrough.md +++ b/packages/app-cli/tests/html_to_md/sub_sup_insert_strikethrough.md @@ -1 +1 @@ -X1 X1 Insert ~~Strike~~ \ No newline at end of file +X1 X1 Insert Insert alt ~~Strike~~ \ No newline at end of file diff --git a/packages/turndown/src/commonmark-rules.js b/packages/turndown/src/commonmark-rules.js index fda46b78aa..475bf03c8e 100644 --- a/packages/turndown/src/commonmark-rules.js +++ b/packages/turndown/src/commonmark-rules.js @@ -1,4 +1,4 @@ -import { repeat, isCodeBlockSpecialCase1, isCodeBlockSpecialCase2, isCodeBlock } from './utilities' +import { repeat, isCodeBlockSpecialCase1, isCodeBlockSpecialCase2, isCodeBlock, getStyleProp } from './utilities' const Entities = require('html-entities').AllHtmlEntities; const htmlentities = (new Entities()).encode; @@ -73,7 +73,15 @@ rules.highlight = { // HTML to avoid any ambiguity. rules.insert = { - filter: 'ins', + filter: function (node, options) { + // TinyMCE represents this either with an tag (when pressing the + // toolbar button) or using style "text-decoration" (when using shortcut + // Cmd+U) + // + // https://github.com/laurent22/joplin/issues/5480 + if (node.nodeName === 'INS') return true; + return getStyleProp(node, 'text-decoration') === 'underline'; + }, replacement: function (content, node, options) { return '' + content + '' diff --git a/packages/turndown/src/utilities.js b/packages/turndown/src/utilities.js index d3da0dc839..0554f00804 100644 --- a/packages/turndown/src/utilities.js +++ b/packages/turndown/src/utilities.js @@ -78,3 +78,16 @@ export function isCodeBlock(node) { node.firstChild.nodeName === 'CODE' ) } + +export function getStyleProp(node, name) { + const style = node.getAttribute('style'); + if (!style) return null; + + name = name.toLowerCase(); + if (!style.toLowerCase().includes(name)) return null; + + const o = css.parse('div {' + style + '}'); + if (!o.stylesheet.rules.length) return null; + const prop = o.stylesheet.rules[0].declarations.find(d => d.property.toLowerCase() === name); + return prop ? prop.value : null; +} From e447acc076efee45631d9b48d565f8293fbacfe2 Mon Sep 17 00:00:00 2001 From: Laurent Cozic Date: Sun, 19 Sep 2021 13:00:06 +0100 Subject: [PATCH 15/15] Desktop: Resolves #5440: Do not escape content when copying from Rich Text editor --- packages/app-cli/tests/HtmlToMd.ts | 6 ++++++ .../gui/NoteEditor/utils/clipboardUtils.ts | 11 ++++++++++- packages/lib/HtmlToMd.ts | 2 ++ packages/turndown/src/turndown.js | 3 +++ 4 files changed, 21 insertions(+), 1 deletion(-) diff --git a/packages/app-cli/tests/HtmlToMd.ts b/packages/app-cli/tests/HtmlToMd.ts index db043efbec..c7e6915bb4 100644 --- a/packages/app-cli/tests/HtmlToMd.ts +++ b/packages/app-cli/tests/HtmlToMd.ts @@ -84,4 +84,10 @@ describe('HtmlToMd', function() { } })); + it('should allow disabling escape', async () => { + const htmlToMd = new HtmlToMd(); + expect(htmlToMd.parse('https://test.com/1_2_3.pdf', { disableEscapeContent: true })).toBe('https://test.com/1_2_3.pdf'); + expect(htmlToMd.parse('https://test.com/1_2_3.pdf', { disableEscapeContent: false })).toBe('https://test.com/1\\_2\\_3.pdf'); + }); + }); diff --git a/packages/app-desktop/gui/NoteEditor/utils/clipboardUtils.ts b/packages/app-desktop/gui/NoteEditor/utils/clipboardUtils.ts index 995353e37e..99660398fb 100644 --- a/packages/app-desktop/gui/NoteEditor/utils/clipboardUtils.ts +++ b/packages/app-desktop/gui/NoteEditor/utils/clipboardUtils.ts @@ -69,8 +69,17 @@ export function htmlToClipboardData(html: string): ClipboardData { // In that case we need to set both HTML and Text context, otherwise it // won't be possible to paste the text in, for example, a text editor. // https://github.com/laurent22/joplin/issues/4788 + // + // Also we don't escape the content produced in HTML to MD conversion + // because it's not what would be expected. For example, if the content is + // `* something`, strictly speaking it would be correct to escape to `\* + // something`, however this is not what the user would expect when copying + // text. Likewise for URLs that contain "_". So the resulting Markdown might + // not be perfectly valid but would be closer to what a user would expect. + // If they want accurate MArkdown they can switch to the MD editor. + // https://github.com/laurent22/joplin/issues/5440 return { - text: htmlToMd().parse(copyableContent), + text: htmlToMd().parse(copyableContent, { disableEscapeContent: true }), html: cleanUpCodeBlocks(copyableContent), }; } diff --git a/packages/lib/HtmlToMd.ts b/packages/lib/HtmlToMd.ts index 91f19e23f0..e38f0d090d 100644 --- a/packages/lib/HtmlToMd.ts +++ b/packages/lib/HtmlToMd.ts @@ -6,6 +6,7 @@ export interface ParseOptions { anchorNames?: string[]; preserveImageTagsWithSize?: boolean; baseUrl?: string; + disableEscapeContent?: boolean; } export default class HtmlToMd { @@ -20,6 +21,7 @@ export default class HtmlToMd { emDelimiter: '*', strongDelimiter: '**', br: '', + disableEscapeContent: 'disableEscapeContent' in options ? options.disableEscapeContent : false, }); turndown.use(turndownPluginGfm); turndown.remove('script'); diff --git a/packages/turndown/src/turndown.js b/packages/turndown/src/turndown.js index 299fc43793..f916226e92 100644 --- a/packages/turndown/src/turndown.js +++ b/packages/turndown/src/turndown.js @@ -23,6 +23,7 @@ export default function TurndownService (options) { linkReferenceStyle: 'full', anchorNames: [], br: ' ', + disableEscapeContent: false, blankReplacement: function (content, node) { return node.isBlock ? '\n\n' : '' }, @@ -181,6 +182,8 @@ TurndownService.prototype = { */ function process (parentNode, escapeContent = 'auto') { + if (this.options.disableEscapeContent) escapeContent = false; + var self = this return reduce.call(parentNode.childNodes, function (output, node) { node = new Node(node)