From 548ecd4171aba328aa8dcc443fc9901162b76239 Mon Sep 17 00:00:00 2001 From: azivner Date: Wed, 3 Jan 2018 21:29:13 -0500 Subject: [PATCH] removed unique index again - from now on the invariant is that there's unique undeleted relationship between note and parent note --- db/schema.sql | 3 - .../0062__change_index_back_to_non_unique.sql | 9 + routes/api/notes_move.js | 166 ++++++------------ services/app_info.js | 2 +- 4 files changed, 68 insertions(+), 112 deletions(-) create mode 100644 migrations/0062__change_index_back_to_non_unique.sql diff --git a/db/schema.sql b/db/schema.sql index 4d6b029dc..7aaea2fbd 100644 --- a/db/schema.sql +++ b/db/schema.sql @@ -67,9 +67,6 @@ CREATE INDEX `IDX_sync_sync_date` ON `sync` ( CREATE INDEX `IDX_notes_is_deleted` ON `notes` ( `is_deleted` ); -CREATE INDEX `IDX_notes_tree_note_tree_id` ON `notes_tree` ( - `note_tree_id` -); CREATE UNIQUE INDEX `IDX_notes_tree_note_id_parent_note_id` ON `notes_tree` ( `note_id`, `parent_note_id` diff --git a/migrations/0062__change_index_back_to_non_unique.sql b/migrations/0062__change_index_back_to_non_unique.sql new file mode 100644 index 000000000..33b575b59 --- /dev/null +++ b/migrations/0062__change_index_back_to_non_unique.sql @@ -0,0 +1,9 @@ +DROP INDEX IDX_notes_tree_note_id_parent_note_id; + +CREATE INDEX `IDX_notes_tree_note_id_parent_note_id` ON `notes_tree` ( + `note_id`, + `parent_note_id` +); + +-- dropping this as it's just duplicate of primary key +DROP INDEX IDX_notes_tree_note_tree_id; \ No newline at end of file diff --git a/routes/api/notes_move.js b/routes/api/notes_move.js index 84b43d118..105df5d9d 100644 --- a/routes/api/notes_move.js +++ b/routes/api/notes_move.js @@ -14,20 +14,8 @@ router.put('/:noteTreeId/move-to/:parentNoteId', auth.checkApiAuth, async (req, const noteToMove = await sql.getFirst("SELECT * FROM notes_tree WHERE note_tree_id = ?", [noteTreeId]); - const existing = await getExistingNoteTree(parentNoteId, noteToMove.note_id); - - if (existing && !existing.is_deleted && existing.note_tree_id !== noteTreeId) { - return res.send({ - success: false, - message: 'This note already exists in target parent note.' - }); - } - - if (!await checkTreeCycle(parentNoteId, noteToMove.note_id)) { - return res.send({ - success: false, - message: 'Moving note here would create cycle.' - }); + if (!await validateParentChild(res, parentNoteId, noteToMove.note_id, noteTreeId)) { + return; } const maxNotePos = await sql.getFirstValue('SELECT MAX(note_position) FROM notes_tree WHERE parent_note_id = ? AND is_deleted = 0', [parentNoteId]); @@ -53,44 +41,27 @@ router.put('/:noteTreeId/move-before/:beforeNoteTreeId', auth.checkApiAuth, asyn const noteToMove = await sql.getFirst("SELECT * FROM notes_tree WHERE note_tree_id = ?", [noteTreeId]); const beforeNote = await sql.getFirst("SELECT * FROM notes_tree WHERE note_tree_id = ?", [beforeNoteTreeId]); - const existing = await getExistingNoteTree(beforeNote.parent_note_id, noteToMove.note_id); - - if (existing && !existing.is_deleted && existing.note_tree_id !== noteTreeId) { - return res.send({ - success: false, - message: 'This note already exists in target parent note.' - }); + if (!await validateParentChild(res, beforeNote.parent_note_id, noteToMove.note_id, noteTreeId)) { + return; } - if (!await checkTreeCycle(beforeNote.parent_note_id, noteToMove.note_id)) { - return res.send({ - success: false, - message: 'Moving note here would create cycle.' - }); - } + await sql.doInTransaction(async () => { + // we don't change date_modified so other changes are prioritized in case of conflict + // also we would have to sync all those modified note trees otherwise hash checks would fail + await sql.execute("UPDATE notes_tree SET note_position = note_position + 1 WHERE parent_note_id = ? AND note_position >= ? AND is_deleted = 0", + [beforeNote.parent_note_id, beforeNote.note_position]); - if (beforeNote) { - await sql.doInTransaction(async () => { - // we don't change date_modified so other changes are prioritized in case of conflict - // also we would have to sync all those modified note trees otherwise hash checks would fail - await sql.execute("UPDATE notes_tree SET note_position = note_position + 1 WHERE parent_note_id = ? AND note_position >= ? AND is_deleted = 0", - [beforeNote.parent_note_id, beforeNote.note_position]); + await sync_table.addNoteReorderingSync(beforeNote.parent_note_id, sourceId); - await sync_table.addNoteReorderingSync(beforeNote.parent_note_id, sourceId); + const now = utils.nowDate(); - const now = utils.nowDate(); + await sql.execute("UPDATE notes_tree SET parent_note_id = ?, note_position = ?, date_modified = ? WHERE note_tree_id = ?", + [beforeNote.parent_note_id, beforeNote.note_position, now, noteTreeId]); - await sql.execute("UPDATE notes_tree SET parent_note_id = ?, note_position = ?, date_modified = ? WHERE note_tree_id = ?", - [beforeNote.parent_note_id, beforeNote.note_position, now, noteTreeId]); + await sync_table.addNoteTreeSync(noteTreeId, sourceId); + }); - await sync_table.addNoteTreeSync(noteTreeId, sourceId); - }); - - res.send({ success: true }); - } - else { - res.status(500).send("Before note " + beforeNoteTreeId + " doesn't exist."); - } + res.send({ success: true }); }); router.put('/:noteTreeId/move-after/:afterNoteTreeId', auth.checkApiAuth, async (req, res, next) => { @@ -101,42 +72,25 @@ router.put('/:noteTreeId/move-after/:afterNoteTreeId', auth.checkApiAuth, async const noteToMove = await sql.getFirst("SELECT * FROM notes_tree WHERE note_tree_id = ?", [noteTreeId]); const afterNote = await sql.getFirst("SELECT * FROM notes_tree WHERE note_tree_id = ?", [afterNoteTreeId]); - const existing = await getExistingNoteTree(afterNote.parent_note_id, noteToMove.note_id); - - if (existing && !existing.is_deleted && existing.note_tree_id !== noteTreeId) { - return res.send({ - success: false, - message: 'This note already exists in target parent note.' - }); + if (!await validateParentChild(res, afterNote.parent_note_id, noteToMove.note_id, noteTreeId)) { + return; } - if (!await checkTreeCycle(afterNote.parent_note_id, noteToMove.note_id)) { - return res.send({ - success: false, - message: 'Moving note here would create cycle.' - }); - } + await sql.doInTransaction(async () => { + // we don't change date_modified so other changes are prioritized in case of conflict + // also we would have to sync all those modified note trees otherwise hash checks would fail + await sql.execute("UPDATE notes_tree SET note_position = note_position + 1 WHERE parent_note_id = ? AND note_position > ? AND is_deleted = 0", + [afterNote.parent_note_id, afterNote.note_position]); - if (afterNote) { - await sql.doInTransaction(async () => { - // we don't change date_modified so other changes are prioritized in case of conflict - // also we would have to sync all those modified note trees otherwise hash checks would fail - await sql.execute("UPDATE notes_tree SET note_position = note_position + 1 WHERE parent_note_id = ? AND note_position > ? AND is_deleted = 0", - [afterNote.parent_note_id, afterNote.note_position]); + await sync_table.addNoteReorderingSync(afterNote.parent_note_id, sourceId); - await sync_table.addNoteReorderingSync(afterNote.parent_note_id, sourceId); + await sql.execute("UPDATE notes_tree SET parent_note_id = ?, note_position = ?, date_modified = ? WHERE note_tree_id = ?", + [afterNote.parent_note_id, afterNote.note_position + 1, utils.nowDate(), noteTreeId]); - await sql.execute("UPDATE notes_tree SET parent_note_id = ?, note_position = ?, date_modified = ? WHERE note_tree_id = ?", - [afterNote.parent_note_id, afterNote.note_position + 1, utils.nowDate(), noteTreeId]); + await sync_table.addNoteTreeSync(noteTreeId, sourceId); + }); - await sync_table.addNoteTreeSync(noteTreeId, sourceId); - }); - - res.send({ success: true }); - } - else { - res.status(500).send("After note " + afterNoteTreeId + " doesn't exist."); - } + res.send({ success: true }); }); router.put('/:childNoteId/clone-to/:parentNoteId', auth.checkApiAuth, async (req, res, next) => { @@ -145,20 +99,8 @@ router.put('/:childNoteId/clone-to/:parentNoteId', auth.checkApiAuth, async (req const prefix = req.body.prefix; const sourceId = req.headers.source_id; - const existing = await getExistingNoteTree(parentNoteId, childNoteId); - - if (existing && !existing.is_deleted) { - return res.send({ - success: false, - message: 'This note already exists in target parent note.' - }); - } - - if (!await checkTreeCycle(parentNoteId, childNoteId)) { - return res.send({ - success: false, - message: 'Cloning note here would create cycle.' - }); + if (!await validateParentChild(res, parentNoteId, childNoteId)) { + return; } const maxNotePos = await sql.getFirstValue('SELECT MAX(note_position) FROM notes_tree WHERE parent_note_id = ? AND is_deleted = 0', [parentNoteId]); @@ -193,24 +135,8 @@ router.put('/:noteId/clone-after/:afterNoteTreeId', auth.checkApiAuth, async (re const afterNote = await sql.getFirst("SELECT * FROM notes_tree WHERE note_tree_id = ?", [afterNoteTreeId]); - if (!afterNote) { - return res.status(500).send("After note " + afterNoteTreeId + " doesn't exist."); - } - - const existing = await getExistingNoteTree(afterNote.parent_note_id, noteId); - - if (existing && !existing.is_deleted) { - return res.send({ - success: false, - message: 'This note already exists in target parent note.' - }); - } - - if (!await checkTreeCycle(afterNote.parent_note_id, noteId)) { - return res.send({ - success: false, - message: 'Cloning note here would create cycle.' - }); + if (!await validateParentChild(res, afterNote.parent_note_id, noteId)) { + return; } await sql.doInTransaction(async () => { @@ -249,8 +175,32 @@ async function loadSubTreeNoteIds(parentNoteId, subTreeNoteIds) { } } +async function validateParentChild(res, parentNoteId, childNoteId, noteTreeId = null) { + const existing = await getExistingNoteTree(parentNoteId, childNoteId); + + if (existing && (noteTreeId === null || existing.note_tree_id !== noteTreeId)) { + res.send({ + success: false, + message: 'This note already exists in target parent note.' + }); + + return false; + } + + if (!await checkTreeCycle(parentNoteId, childNoteId)) { + res.send({ + success: false, + message: 'Moving note here would create cycle.' + }); + + return false; + } + + return true; +} + async function getExistingNoteTree(parentNoteId, childNoteId) { - return await sql.getFirst('SELECT * FROM notes_tree WHERE note_id = ? AND parent_note_id = ?', [childNoteId, parentNoteId]); + return await sql.getFirst('SELECT * FROM notes_tree WHERE note_id = ? AND parent_note_id = ? AND is_deleted = 0', [childNoteId, parentNoteId]); } /** diff --git a/services/app_info.js b/services/app_info.js index 3fbb1ac14..c3092206f 100644 --- a/services/app_info.js +++ b/services/app_info.js @@ -3,7 +3,7 @@ const build = require('./build'); const packageJson = require('../package'); -const APP_DB_VERSION = 61; +const APP_DB_VERSION = 62; module.exports = { app_version: packageJson.version,