From ff601982b9cdfa139dde53193625d75d5d0e9d8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Espino=20Garc=C3=ADa?= Date: Mon, 7 Aug 2023 09:43:00 +0200 Subject: [PATCH] Add some performance improvements (#7481) * Add some performance improvements * fix tests --- app/actions/local/user.ts | 9 ++++++--- app/actions/remote/post.ts | 5 ++++- app/actions/remote/user.ts | 13 ++++++++++--- .../operator/base_data_operator/index.ts | 9 +++++++-- .../server_data_operator/comparators/files.ts | 18 ++++++++++++++++++ .../server_data_operator/comparators/thread.ts | 16 ++++++++++++++++ .../server_data_operator/comparators/user.ts | 8 ++++++++ .../server_data_operator/handlers/post.ts | 3 +++ .../handlers/thread.test.ts | 2 ++ .../server_data_operator/handlers/thread.ts | 2 ++ .../server_data_operator/handlers/user.test.ts | 2 ++ .../server_data_operator/handlers/user.ts | 2 ++ types/database/database.ts | 2 ++ 13 files changed, 82 insertions(+), 9 deletions(-) create mode 100644 app/database/operator/server_data_operator/comparators/files.ts create mode 100644 app/database/operator/server_data_operator/comparators/thread.ts create mode 100644 app/database/operator/server_data_operator/comparators/user.ts diff --git a/app/actions/local/user.ts b/app/actions/local/user.ts index 5f3503e16..016086039 100644 --- a/app/actions/local/user.ts +++ b/app/actions/local/user.ts @@ -20,11 +20,14 @@ export async function setCurrentUserStatus(serverUrl: string, status: string) { throw new Error(`No current user for ${serverUrl}`); } - user.prepareStatus(status); - await operator.batchRecords([user], 'setCurrentUserStatusOffline'); + if (user.status !== status) { + user.prepareStatus(status); + await operator.batchRecords([user], 'setCurrentUserStatus'); + } + return null; } catch (error) { - logError('Failed setCurrentUserStatusOffline', error); + logError('Failed setCurrentUserStatus', error); return {error}; } } diff --git a/app/actions/remote/post.ts b/app/actions/remote/post.ts index 3c26ff0ac..24535fd99 100644 --- a/app/actions/remote/post.ts +++ b/app/actions/remote/post.ts @@ -378,7 +378,10 @@ export async function fetchPosts(serverUrl: string, channelId: string, page = 0, models.push(...threadModels); } } - await operator.batchRecords(models, 'fetchPosts'); + + if (models.length) { + await operator.batchRecords(models, 'fetchPosts'); + } } return result; } catch (error) { diff --git a/app/actions/remote/user.ts b/app/actions/remote/user.ts index acc5e92a5..5c0facba2 100644 --- a/app/actions/remote/user.ts +++ b/app/actions/remote/user.ts @@ -344,12 +344,19 @@ export async function fetchStatusByIds(serverUrl: string, userIds: string[], fet return result; }, {}); + const usersToBatch = []; for (const user of users) { - const status = userStatuses[user.id]; - user.prepareStatus(status?.status || General.OFFLINE); + const receivedStatus = userStatuses[user.id]; + const statusToSet = receivedStatus?.status || General.OFFLINE; + if (statusToSet !== user.status) { + user.prepareStatus(statusToSet); + usersToBatch.push(user); + } } - await operator.batchRecords(users, 'fetchStatusByIds'); + if (usersToBatch.length) { + await operator.batchRecords(usersToBatch, 'fetchStatusByIds'); + } } return {statuses}; diff --git a/app/database/operator/base_data_operator/index.ts b/app/database/operator/base_data_operator/index.ts index 8089c7658..aace2f2aa 100644 --- a/app/database/operator/base_data_operator/index.ts +++ b/app/database/operator/base_data_operator/index.ts @@ -45,7 +45,7 @@ export default class BaseDataOperator { * @param {(existing: Model, newElement: RawValue) => boolean} inputsArg.buildKeyRecordBy * @returns {Promise<{ProcessRecordResults}>} */ - processRecords = async ({createOrUpdateRawValues = [], deleteRawValues = [], tableName, buildKeyRecordBy, fieldName}: ProcessRecordsArgs): Promise> => { + processRecords = async ({createOrUpdateRawValues = [], deleteRawValues = [], tableName, buildKeyRecordBy, fieldName, shouldUpdate}: ProcessRecordsArgs): Promise> => { const getRecords = async (rawValues: RawValue[]) => { // We will query a table where one of its fields can match a range of values. Hence, here we are extracting all those potential values. const columnValues: string[] = getRangeOfValues({fieldName, raws: rawValues}); @@ -92,6 +92,10 @@ export default class BaseDataOperator { // We found a record in the database that matches this element; hence, we'll proceed for an UPDATE operation if (existingRecord) { + if (shouldUpdate && !shouldUpdate(existingRecord, newElement)) { + continue; + } + // Some raw value has an update_at field. We'll proceed to update only if the update_at value is different from the record's value in database const updateRecords = getValidRecordsForUpdate({ tableName, @@ -205,7 +209,7 @@ export default class BaseDataOperator { * @param {string} handleRecordsArgs.tableName * @returns {Promise} */ - async handleRecords({buildKeyRecordBy, fieldName, transformer, createOrUpdateRawValues, deleteRawValues = [], tableName, prepareRecordsOnly = true}: HandleRecordsArgs, description: string): Promise { + async handleRecords({buildKeyRecordBy, fieldName, transformer, createOrUpdateRawValues, deleteRawValues = [], tableName, prepareRecordsOnly = true, shouldUpdate}: HandleRecordsArgs, description: string): Promise { if (!createOrUpdateRawValues.length) { logWarning( `An empty "rawValues" array has been passed to the handleRecords method for tableName ${tableName}`, @@ -219,6 +223,7 @@ export default class BaseDataOperator { tableName, buildKeyRecordBy, fieldName, + shouldUpdate, }); let models: T[] = []; diff --git a/app/database/operator/server_data_operator/comparators/files.ts b/app/database/operator/server_data_operator/comparators/files.ts new file mode 100644 index 000000000..0a786ba09 --- /dev/null +++ b/app/database/operator/server_data_operator/comparators/files.ts @@ -0,0 +1,18 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import type FileModel from '@typings/database/models/servers/file'; + +export const shouldUpdateFileRecord = (e: FileModel, n: FileInfo): boolean => { + return Boolean( + (n.post_id !== e.postId) || + (n.name !== e.name) || + (n.extension !== e.extension) || + (n.size !== e.size) || + ((n.mime_type || '') !== e.mimeType) || + (n.width && n.width !== e.width) || + (n.height && n.height !== e.height) || + (n.mini_preview && n.mini_preview !== e.imageThumbnail) || + (n.localPath && n.localPath !== e.localPath), + ); +}; diff --git a/app/database/operator/server_data_operator/comparators/thread.ts b/app/database/operator/server_data_operator/comparators/thread.ts new file mode 100644 index 000000000..42651b0c4 --- /dev/null +++ b/app/database/operator/server_data_operator/comparators/thread.ts @@ -0,0 +1,16 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import type ThreadModel from '@typings/database/models/servers/thread'; + +export const shouldUpdateThreadRecord = (e: ThreadModel, n: ThreadWithLastFetchedAt): boolean => { + return ( + ((n.last_reply_at != null) && n.last_reply_at !== e.lastReplyAt) || + ((n.lastFetchedAt || 0) > e.lastFetchedAt) || + ((n.last_viewed_at != null) && e.lastViewedAt !== n.last_viewed_at) || + (e.replyCount !== n.reply_count) || + ((n.is_following != null) && e.isFollowing !== n.is_following) || + ((n.unread_replies != null) && e.unreadReplies !== n.unread_replies) || + ((n.unread_mentions != null) && e.unreadMentions !== n.unread_mentions) + ); +}; diff --git a/app/database/operator/server_data_operator/comparators/user.ts b/app/database/operator/server_data_operator/comparators/user.ts new file mode 100644 index 000000000..093315c57 --- /dev/null +++ b/app/database/operator/server_data_operator/comparators/user.ts @@ -0,0 +1,8 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import type UserModel from '@typings/database/models/servers/user'; + +export function shouldUpdateUserRecord(e: UserModel, n: UserProfile) { + return Boolean(n.update_at > e.updateAt || (n.status && n.status !== e.status)); +} diff --git a/app/database/operator/server_data_operator/handlers/post.ts b/app/database/operator/server_data_operator/handlers/post.ts index a970ed96f..2b0d92e3e 100644 --- a/app/database/operator/server_data_operator/handlers/post.ts +++ b/app/database/operator/server_data_operator/handlers/post.ts @@ -6,6 +6,7 @@ import {Q} from '@nozbe/watermelondb'; import {ActionType} from '@constants'; import {MM_TABLES} from '@constants/database'; import {buildDraftKey} from '@database/operator/server_data_operator/comparators'; +import {shouldUpdateFileRecord} from '@database/operator/server_data_operator/comparators/files'; import { transformDraftRecord, transformFileRecord, @@ -234,6 +235,7 @@ const PostHandler = >(supercla deleteRawValues: pendingPostsToDelete, tableName, fieldName: 'id', + shouldUpdate: (e: PostModel, n: Post) => n.update_at > e.updateAt, })); const preparedPosts = (await this.prepareRecords({ @@ -309,6 +311,7 @@ const PostHandler = >(supercla tableName: FILE, fieldName: 'id', deleteRawValues: [], + shouldUpdate: shouldUpdateFileRecord, })); const postFiles = await this.prepareRecords({ diff --git a/app/database/operator/server_data_operator/handlers/thread.test.ts b/app/database/operator/server_data_operator/handlers/thread.test.ts index b7a5ea2ab..eed4595a2 100644 --- a/app/database/operator/server_data_operator/handlers/thread.test.ts +++ b/app/database/operator/server_data_operator/handlers/thread.test.ts @@ -2,6 +2,7 @@ // See LICENSE.txt for license information. import DatabaseManager from '@database/manager'; +import {shouldUpdateThreadRecord} from '@database/operator/server_data_operator/comparators/thread'; import {transformThreadRecord, transformThreadParticipantRecord, transformThreadInTeamRecord, transformTeamThreadsSyncRecord} from '@database/operator/server_data_operator/transformers/thread'; import type ServerDataOperator from '..'; @@ -59,6 +60,7 @@ describe('*** Operator: Thread Handlers tests ***', () => { createOrUpdateRawValues: threads, tableName: 'Thread', prepareRecordsOnly: true, + shouldUpdate: shouldUpdateThreadRecord, }, 'handleThreads(NEVER)'); // Should handle participants diff --git a/app/database/operator/server_data_operator/handlers/thread.ts b/app/database/operator/server_data_operator/handlers/thread.ts index 47cb7ad58..921516da2 100644 --- a/app/database/operator/server_data_operator/handlers/thread.ts +++ b/app/database/operator/server_data_operator/handlers/thread.ts @@ -4,6 +4,7 @@ import {Q} from '@nozbe/watermelondb'; import {MM_TABLES} from '@constants/database'; +import {shouldUpdateThreadRecord} from '@database/operator/server_data_operator/comparators/thread'; import { transformThreadRecord, transformThreadParticipantRecord, @@ -110,6 +111,7 @@ const ThreadHandler = >(superc prepareRecordsOnly: true, createOrUpdateRawValues: createOrUpdateThreads, tableName: THREAD, + shouldUpdate: shouldUpdateThreadRecord, }, 'handleThreads(NEVER)'); // Add the models to be batched here diff --git a/app/database/operator/server_data_operator/handlers/user.test.ts b/app/database/operator/server_data_operator/handlers/user.test.ts index 7f47c4702..b4a230db3 100644 --- a/app/database/operator/server_data_operator/handlers/user.test.ts +++ b/app/database/operator/server_data_operator/handlers/user.test.ts @@ -3,6 +3,7 @@ import DatabaseManager from '@database/manager'; import {buildPreferenceKey} from '@database/operator/server_data_operator/comparators'; +import {shouldUpdateUserRecord} from '@database/operator/server_data_operator/comparators/user'; import { transformPreferenceRecord, transformUserRecord, @@ -102,6 +103,7 @@ describe('*** Operator: User Handlers tests ***', () => { tableName: 'User', prepareRecordsOnly: false, transformer: transformUserRecord, + shouldUpdate: shouldUpdateUserRecord, }, 'handleUsers'); }); diff --git a/app/database/operator/server_data_operator/handlers/user.ts b/app/database/operator/server_data_operator/handlers/user.ts index b375b779e..b5c2813b2 100644 --- a/app/database/operator/server_data_operator/handlers/user.ts +++ b/app/database/operator/server_data_operator/handlers/user.ts @@ -3,6 +3,7 @@ import {MM_TABLES} from '@constants/database'; import {buildPreferenceKey} from '@database/operator/server_data_operator/comparators'; +import {shouldUpdateUserRecord} from '@database/operator/server_data_operator/comparators/user'; import { transformPreferenceRecord, transformUserRecord, @@ -126,6 +127,7 @@ const UserHandler = >(supercla createOrUpdateRawValues, tableName: USER, prepareRecordsOnly, + shouldUpdate: shouldUpdateUserRecord, }, 'handleUsers'); }; }; diff --git a/types/database/database.ts b/types/database/database.ts index 257dda66d..cc26b7fe9 100644 --- a/types/database/database.ts +++ b/types/database/database.ts @@ -155,6 +155,7 @@ export type ProcessRecordsArgs = { tableName: string; fieldName: string; buildKeyRecordBy?: (obj: Record) => string; + shouldUpdate?: (existing: Record, newRaw: Record) => boolean; }; export type HandleRecordsArgs = { @@ -165,6 +166,7 @@ export type HandleRecordsArgs = { deleteRawValues?: RawValue[]; tableName: string; prepareRecordsOnly: boolean; + shouldUpdate?: (existingRecord: T, newRaw: RawValue) => boolean; }; export type RangeOfValueArgs = {