From 88d33b42c24b735724c291ea07249e092b8ced07 Mon Sep 17 00:00:00 2001 From: Diego Mello Date: Thu, 10 Feb 2022 17:16:10 -0300 Subject: [PATCH] [FIX] Remove deprecated database methods and other database operations (#3686) * Fix PK error on subscriptions/room * Instead of checking for pending update, wrap the call on a try catch and return null in case of error * Generate delete operations before create/update to prevent errors * Apply same logic on encryption * Fix database operations on getRoles * Fix a few database issues found on Bugsnag on ThreadMessagesView * Run prettier :( --- app/lib/database/services/Role.ts | 17 +++++ app/lib/encryption/encryption.js | 36 +++++----- app/lib/methods/getRoles.js | 38 +++++----- app/lib/methods/subscriptions/room.js | 91 +++++++++++++----------- app/lib/methods/updateMessages.js | 96 +++++++++++++------------- app/views/ThreadMessagesView/index.tsx | 24 ++++--- 6 files changed, 166 insertions(+), 136 deletions(-) create mode 100644 app/lib/database/services/Role.ts diff --git a/app/lib/database/services/Role.ts b/app/lib/database/services/Role.ts new file mode 100644 index 000000000..e033afb72 --- /dev/null +++ b/app/lib/database/services/Role.ts @@ -0,0 +1,17 @@ +import database from '..'; +import { TAppDatabase } from '../interfaces'; +import { ROLES_TABLE } from '../model'; +import { TRoleModel } from '../../../definitions'; + +const getCollection = (db: TAppDatabase) => db.get(ROLES_TABLE); + +export const getRoleById = async (id: string): Promise => { + const db = database.active; + const roleCollection = getCollection(db); + try { + const result = await roleCollection.find(id); + return result; + } catch (error) { + return null; + } +}; diff --git a/app/lib/encryption/encryption.js b/app/lib/encryption/encryption.js index 2d7f808fd..106a27ffe 100644 --- a/app/lib/encryption/encryption.js +++ b/app/lib/encryption/encryption.js @@ -240,15 +240,15 @@ class Encryption { msg, tmsg }); - if (message._hasPendingUpdate) { - console.log(message); - return; + try { + return message.prepareUpdate( + protectedFunction(m => { + Object.assign(m, newMessage); + }) + ); + } catch { + return null; } - return message.prepareUpdate( - protectedFunction(m => { - Object.assign(m, newMessage); - }) - ); }) ); @@ -281,15 +281,15 @@ class Encryption { subsToDecrypt.map(async sub => { const { rid, lastMessage } = sub; const newSub = await this.decryptSubscription({ rid, lastMessage }); - if (sub._hasPendingUpdate) { - console.log(sub); - return; + try { + return sub.prepareUpdate( + protectedFunction(m => { + Object.assign(m, newSub); + }) + ); + } catch { + return null; } - return sub.prepareUpdate( - protectedFunction(m => { - Object.assign(m, newSub); - }) - ); }) ); @@ -352,13 +352,15 @@ class Encryption { ); // If the subscription already exists but doesn't have the E2EKey yet } else if (!subRecord.E2EKey && subscription.E2EKey) { - if (!subRecord._hasPendingUpdate) { + try { // Let's update the subscription with the received E2EKey batch.push( subRecord.prepareUpdate(s => { s.E2EKey = subscription.E2EKey; }) ); + } catch (e) { + log(e); } } diff --git a/app/lib/methods/getRoles.js b/app/lib/methods/getRoles.js index d43ca00ef..5f9c91ee5 100644 --- a/app/lib/methods/getRoles.js +++ b/app/lib/methods/getRoles.js @@ -1,6 +1,7 @@ import { sanitizedRaw } from '@nozbe/watermelondb/RawRecord'; import database from '../database'; +import { getRoleById } from '../database/services/Role'; import log from '../../utils/log'; import { store as reduxStore } from '../auxStore'; import { removeRoles, setRoles as setRolesAction, updateRoles } from '../../actions/roles'; @@ -20,41 +21,38 @@ export async function onRolesChanged(ddpMessage) { const db = database.active; const rolesCollection = db.get('roles'); try { - const rolesRecord = await rolesCollection.find(_id); - try { - await db.action(async () => { - await rolesRecord.update(u => { + const roleRecord = await getRoleById(_id); + if (roleRecord) { + await db.write(async () => { + await roleRecord.update(u => { u.description = description; }); }); - } catch (e) { - log(e); - } - reduxStore.dispatch(updateRoles(_id, description)); - } catch (err) { - try { - await db.action(async () => { + } else { + await db.write(async () => { await rolesCollection.create(post => { post._raw = sanitizedRaw({ id: _id, description }, rolesCollection.schema); }); }); - } catch (e) { - log(e); } reduxStore.dispatch(updateRoles(_id, description || _id)); + } catch (e) { + log(e); } } if (/removed/.test(type)) { const db = database.active; const rolesCollection = db.get('roles'); try { - const rolesRecord = await rolesCollection.find(_id); - await db.action(async () => { - await rolesRecord.destroyPermanently(); - }); - reduxStore.dispatch(removeRoles(_id)); - } catch (err) { - console.log(err); + const roleRecord = await getRoleById(_id); + if (roleRecord) { + await db.write(async () => { + await roleRecord.destroyPermanently(); + }); + reduxStore.dispatch(removeRoles(_id)); + } + } catch (e) { + log(e); } } } diff --git a/app/lib/methods/subscriptions/room.js b/app/lib/methods/subscriptions/room.js index 33ebb7df5..3aa0a206d 100644 --- a/app/lib/methods/subscriptions/room.js +++ b/app/lib/methods/subscriptions/room.js @@ -6,6 +6,9 @@ import log from '../../../utils/log'; import protectedFunction from '../helpers/protectedFunction'; import buildMessage from '../helpers/buildMessage'; import database from '../../database'; +import { getMessageById } from '../../database/services/Message'; +import { getThreadById } from '../../database/services/Thread'; +import { getThreadMessageById } from '../../database/services/ThreadMessage'; import reduxStore from '../../createStore'; import { addUserTyping, clearUserTyping, removeUserTyping } from '../../../actions/usersTyping'; import debounce from '../../../utils/debounce'; @@ -170,75 +173,81 @@ export default class RoomSubscription { // Create or update message try { - const messageRecord = await msgCollection.find(message._id); - if (!messageRecord._hasPendingUpdate) { - const update = messageRecord.prepareUpdate( + let operation = null; + const messageRecord = await getMessageById(message._id); + if (messageRecord) { + operation = messageRecord.prepareUpdate( protectedFunction(m => { Object.assign(m, message); }) ); - this._messagesBatch[message._id] = update; + } else { + operation = msgCollection.prepareCreate( + protectedFunction(m => { + m._raw = sanitizedRaw({ id: message._id }, msgCollection.schema); + m.subscription.id = this.rid; + Object.assign(m, message); + }) + ); } - } catch { - const create = msgCollection.prepareCreate( - protectedFunction(m => { - m._raw = sanitizedRaw({ id: message._id }, msgCollection.schema); - m.subscription.id = this.rid; - Object.assign(m, message); - }) - ); - this._messagesBatch[message._id] = create; + this._messagesBatch[message._id] = operation; + } catch (e) { + log(e); } // Create or update thread if (message.tlm) { try { - const threadRecord = await threadsCollection.find(message._id); - if (!threadRecord._hasPendingUpdate) { - const updateThread = threadRecord.prepareUpdate( + let operation = null; + const threadRecord = await getThreadById(message._id); + if (threadRecord) { + operation = threadRecord.prepareUpdate( protectedFunction(t => { Object.assign(t, message); }) ); - this._threadsBatch[message._id] = updateThread; + } else { + operation = threadsCollection.prepareCreate( + protectedFunction(t => { + t._raw = sanitizedRaw({ id: message._id }, threadsCollection.schema); + t.subscription.id = this.rid; + Object.assign(t, message); + }) + ); } - } catch { - const createThread = threadsCollection.prepareCreate( - protectedFunction(t => { - t._raw = sanitizedRaw({ id: message._id }, threadsCollection.schema); - t.subscription.id = this.rid; - Object.assign(t, message); - }) - ); - this._threadsBatch[message._id] = createThread; + this._threadsBatch[message._id] = operation; + } catch (e) { + log(e); } } // Create or update thread message if (message.tmid) { try { - const threadMessageRecord = await threadMessagesCollection.find(message._id); - if (!threadMessageRecord._hasPendingUpdate) { - const updateThreadMessage = threadMessageRecord.prepareUpdate( + let operation = null; + const threadMessageRecord = await getThreadMessageById(message._id); + if (threadMessageRecord) { + operation = threadMessageRecord.prepareUpdate( protectedFunction(tm => { Object.assign(tm, message); tm.rid = message.tmid; delete tm.tmid; }) ); - this._threadMessagesBatch[message._id] = updateThreadMessage; + } else { + operation = threadMessagesCollection.prepareCreate( + protectedFunction(tm => { + tm._raw = sanitizedRaw({ id: message._id }, threadMessagesCollection.schema); + Object.assign(tm, message); + tm.subscription.id = this.rid; + tm.rid = message.tmid; + delete tm.tmid; + }) + ); } - } catch { - const createThreadMessage = threadMessagesCollection.prepareCreate( - protectedFunction(tm => { - tm._raw = sanitizedRaw({ id: message._id }, threadMessagesCollection.schema); - Object.assign(tm, message); - tm.subscription.id = this.rid; - tm.rid = message.tmid; - delete tm.tmid; - }) - ); - this._threadMessagesBatch[message._id] = createThreadMessage; + this._threadMessagesBatch[message._id] = operation; + } catch (e) { + log(e); } } diff --git a/app/lib/methods/updateMessages.js b/app/lib/methods/updateMessages.js index c39066b20..4b7dfdeb7 100644 --- a/app/lib/methods/updateMessages.js +++ b/app/lib/methods/updateMessages.js @@ -24,7 +24,7 @@ export default function updateMessages({ rid, update = [], remove = [], loaderIt sub = await subCollection.find(rid); } catch (error) { sub = { id: rid }; - console.log('updateMessages: subscription not found'); + log(new Error('updateMessages: subscription not found')); } const messagesIds = [...update.map(m => m._id), ...remove.map(m => m._id)]; @@ -58,6 +58,25 @@ export default function updateMessages({ rid, update = [], remove = [], loaderIt // filter loaders to delete let loadersToDelete = allMessagesRecords.filter(i1 => update.find(i2 => i1.id === generateLoadMoreId(i2._id))); + // Delete + let msgsToDelete = []; + let threadsToDelete = []; + let threadMessagesToDelete = []; + if (remove && remove.length) { + msgsToDelete = allMessagesRecords.filter(i1 => remove.find(i2 => i1.id === i2._id)); + msgsToDelete = msgsToDelete.map(m => m.prepareDestroyPermanently()); + threadsToDelete = allThreadsRecords.filter(i1 => remove.find(i2 => i1.id === i2._id)); + threadsToDelete = threadsToDelete.map(t => t.prepareDestroyPermanently()); + threadMessagesToDelete = allThreadMessagesRecords.filter(i1 => remove.find(i2 => i1.id === i2._id)); + threadMessagesToDelete = threadMessagesToDelete.map(tm => tm.prepareDestroyPermanently()); + } + + // Delete loaders + loadersToDelete = loadersToDelete.map(m => m.prepareDestroyPermanently()); + if (loaderItem) { + loadersToDelete.push(loaderItem.prepareDestroyPermanently()); + } + // Create msgsToCreate = msgsToCreate.map(message => msgCollection.prepareCreate( @@ -92,62 +111,43 @@ export default function updateMessages({ rid, update = [], remove = [], loaderIt // Update msgsToUpdate = msgsToUpdate.map(message => { const newMessage = update.find(m => m._id === message.id); - if (message._hasPendingUpdate) { - console.log(message); - return; + try { + return message.prepareUpdate( + protectedFunction(m => { + Object.assign(m, newMessage); + }) + ); + } catch { + return null; } - return message.prepareUpdate( - protectedFunction(m => { - Object.assign(m, newMessage); - }) - ); }); threadsToUpdate = threadsToUpdate.map(thread => { - if (thread._hasPendingUpdate) { - console.log(thread); - return; - } const newThread = allThreads.find(t => t._id === thread.id); - return thread.prepareUpdate( - protectedFunction(t => { - Object.assign(t, newThread); - }) - ); + try { + return thread.prepareUpdate( + protectedFunction(t => { + Object.assign(t, newThread); + }) + ); + } catch { + return null; + } }); threadMessagesToUpdate = threadMessagesToUpdate.map(threadMessage => { - if (threadMessage._hasPendingUpdate) { - console.log(threadMessage); - return; - } const newThreadMessage = allThreadMessages.find(t => t._id === threadMessage.id); - return threadMessage.prepareUpdate( - protectedFunction(tm => { - Object.assign(tm, newThreadMessage); - tm.rid = threadMessage.tmid; - delete threadMessage.tmid; - }) - ); + try { + return threadMessage.prepareUpdate( + protectedFunction(tm => { + Object.assign(tm, newThreadMessage); + tm.rid = threadMessage.tmid; + delete threadMessage.tmid; + }) + ); + } catch { + return null; + } }); - // Delete - let msgsToDelete = []; - let threadsToDelete = []; - let threadMessagesToDelete = []; - if (remove && remove.length) { - msgsToDelete = allMessagesRecords.filter(i1 => remove.find(i2 => i1.id === i2._id)); - msgsToDelete = msgsToDelete.map(m => m.prepareDestroyPermanently()); - threadsToDelete = allThreadsRecords.filter(i1 => remove.find(i2 => i1.id === i2._id)); - threadsToDelete = threadsToDelete.map(t => t.prepareDestroyPermanently()); - threadMessagesToDelete = allThreadMessagesRecords.filter(i1 => remove.find(i2 => i1.id === i2._id)); - threadMessagesToDelete = threadMessagesToDelete.map(tm => tm.prepareDestroyPermanently()); - } - - // Delete loaders - loadersToDelete = loadersToDelete.map(m => m.prepareDestroyPermanently()); - if (loaderItem) { - loadersToDelete.push(loaderItem.prepareDestroyPermanently()); - } - const allRecords = [ ...msgsToCreate, ...msgsToUpdate, diff --git a/app/views/ThreadMessagesView/index.tsx b/app/views/ThreadMessagesView/index.tsx index ce154bd18..9c5060da8 100644 --- a/app/views/ThreadMessagesView/index.tsx +++ b/app/views/ThreadMessagesView/index.tsx @@ -281,6 +281,11 @@ class ThreadMessagesView extends React.Component remove.find(i2 => i1.id === i2._id)); + threadsToDelete = threadsToDelete.map(t => t.prepareDestroyPermanently()); + } + if (update && update.length) { update = update.map(m => buildMessage(m)); // filter threads @@ -297,19 +302,18 @@ class ThreadMessagesView extends React.Component { const newThread = update.find(t => t._id === thread.id); - return thread.prepareUpdate( - protectedFunction((t: any) => { - Object.assign(t, newThread); - }) - ); + try { + return thread.prepareUpdate( + protectedFunction((t: any) => { + Object.assign(t, newThread); + }) + ); + } catch { + return null; + } }); } - if (remove && remove.length) { - threadsToDelete = allThreadsRecords.filter((i1: { id: string }) => remove.find(i2 => i1.id === i2._id)); - threadsToDelete = threadsToDelete.map(t => t.prepareDestroyPermanently()); - } - await db.write(async () => { await db.batch( ...threadsToCreate,