From 857535f5fac7eb11a620d78854413937f612884e Mon Sep 17 00:00:00 2001 From: Andrey Sobolev Date: Fri, 2 Feb 2024 22:50:22 +0700 Subject: [PATCH] UBERF-4319: Improve performance (#4501) Signed-off-by: Andrey Sobolev --- .github/workflows/main.yml | 1 + packages/core/src/server.ts | 16 +- packages/presentation/src/configuration.ts | 34 ++-- plugins/client-resources/src/index.ts | 62 +++++-- .../src/components/CreateIssue.svelte | 2 +- server/core/src/adapter.ts | 4 +- server/core/src/storage.ts | 6 +- server/middleware/src/spaceSecurity.ts | 163 ++++++++++-------- server/mongo/src/storage.ts | 67 ++++--- 9 files changed, 209 insertions(+), 146 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index b08a044247..cc07639a29 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -277,6 +277,7 @@ jobs: env: DOCKER_CLI_HINTS: false - name: Login to Docker Hub + if: ${{ github.ref == 'refs/heads/main' }} uses: docker/login-action@v3 with: username: hardcoreeng diff --git a/packages/core/src/server.ts b/packages/core/src/server.ts index c215b3a829..8eccbdf92c 100644 --- a/packages/core/src/server.ts +++ b/packages/core/src/server.ts @@ -13,21 +13,21 @@ // limitations under the License. // -import { MeasureContext } from './measurements' -import type { Doc, Class, Ref, Domain, Timestamp } from './classes' +import { LoadModelResponse } from '.' +import type { Class, Doc, Domain, Ref, Timestamp } from './classes' import { Hierarchy } from './hierarchy' +import { MeasureContext } from './measurements' import { ModelDb } from './memdb' import type { DocumentQuery, FindOptions, FindResult, - TxResult, - SearchQuery, SearchOptions, - SearchResult + SearchQuery, + SearchResult, + TxResult } from './storage' import type { Tx } from './tx' -import { LoadModelResponse } from '.' /** * @public @@ -72,7 +72,9 @@ export interface ServerStorage extends LowLevelStorage { ctx: MeasureContext, _class: Ref>, query: DocumentQuery, - options?: FindOptions + options?: FindOptions & { + domain?: Domain // Allow to find for Doc's in specified domain only. + } ) => Promise> searchFulltext: (ctx: MeasureContext, query: SearchQuery, options: SearchOptions) => Promise tx: (ctx: MeasureContext, tx: Tx) => Promise<[TxResult, Tx[]]> diff --git a/packages/presentation/src/configuration.ts b/packages/presentation/src/configuration.ts index 5b95f522fb..6a71867cb2 100644 --- a/packages/presentation/src/configuration.ts +++ b/packages/presentation/src/configuration.ts @@ -13,11 +13,10 @@ // limitations under the License. // -import core, { type PluginConfiguration, SortingOrder } from '@hcengineering/core' -import { type Plugin, type Resource, getResourcePlugin } from '@hcengineering/platform' -import { get, writable } from 'svelte/store' -import { createQuery } from '.' -import { location as platformLocation } from '@hcengineering/ui' +import core, { SortingOrder, type PluginConfiguration, type TxUpdateDoc } from '@hcengineering/core' +import { getResourcePlugin, type Plugin, type Resource } from '@hcengineering/platform' +import { writable } from 'svelte/store' +import { addTxListener, createQuery } from '.' /** * @public @@ -47,9 +46,17 @@ export const configurationStore = writable(configuration) const configQuery = createQuery(true) -let hashString = '' -let workspaceId: string = '' - +addTxListener((tx) => { + if (tx._class === core.class.TxUpdateDoc) { + const cud = tx as TxUpdateDoc + if (cud.objectClass === core.class.PluginConfiguration) { + if (cud.operations.enabled !== undefined) { + // Plugin enabled/disabled we need to refresh + location.reload() + } + } + } +}) /** * @public */ @@ -61,17 +68,8 @@ configQuery.query( core.class.PluginConfiguration, {}, (res) => { - const newHash = res.map((it) => `${it.pluginId}=${it.enabled ? '+' : '-'}`).join('&') - const wsId = get(platformLocation).path[1] - - if (hashString !== '' && hashString !== newHash && workspaceId !== '' && workspaceId === wsId) { - // Configuration is changed for same workspace. - location.reload() - } - workspaceId = wsId - hashString = newHash configuration = new ConfigurationManager(res, new Map(res.map((it) => [it.pluginId, it]))) configurationStore.set(configuration) }, - { sort: { label: SortingOrder.Ascending } } + { sort: { pluginId: SortingOrder.Ascending } } ) diff --git a/plugins/client-resources/src/index.ts b/plugins/client-resources/src/index.ts index dfd592194a..668db40b4e 100644 --- a/plugins/client-resources/src/index.ts +++ b/plugins/client-resources/src/index.ts @@ -17,6 +17,7 @@ import clientPlugin from '@hcengineering/client' import core, { AccountClient, ClientConnectEvent, + LoadModelResponse, TxHandler, TxPersistenceStore, TxWorkspaceEvent, @@ -79,31 +80,62 @@ export default async () => { } } function createModelPersistence (token: string): TxPersistenceStore | undefined { + let dbRequest: IDBOpenDBRequest | undefined + let dbPromise: Promise = Promise.resolve(undefined) + + if (typeof localStorage !== 'undefined') { + dbPromise = new Promise((resolve) => { + dbRequest = indexedDB.open('model.db.persistence', 2) + + dbRequest.onupgradeneeded = function () { + const db = (dbRequest as IDBOpenDBRequest).result + if (!db.objectStoreNames.contains('model')) { + db.createObjectStore('model', { keyPath: 'id' }) + } + } + dbRequest.onsuccess = function () { + const db = (dbRequest as IDBOpenDBRequest).result + resolve(db) + } + }) + } return { load: async () => { - if (typeof localStorage !== 'undefined') { - const storedValue = localStorage.getItem('platform.model') ?? null - try { - const model = storedValue != null ? JSON.parse(storedValue) : undefined - if (token !== model?.token) { - return { - full: false, - transactions: [], - hash: [] - } + const db = await dbPromise + if (db !== undefined) { + const transaction = db.transaction('model', 'readwrite') // (1) + const models = transaction.objectStore('model') // (2) + const model = await new Promise<{ id: string, model: LoadModelResponse } | undefined>((resolve) => { + const storedValue: IDBRequest<{ id: string, model: LoadModelResponse }> = models.get(token) + storedValue.onsuccess = function () { + resolve(storedValue.result) } - return model.model - } catch {} + storedValue.onerror = function () { + resolve(undefined) + } + }) + + if (model == null) { + return { + full: false, + transactions: [], + hash: '' + } + } + return model.model } return { full: true, transactions: [], - hash: [] + hash: '' } }, store: async (model) => { - if (typeof localStorage !== 'undefined') { - localStorage.setItem('platform.model', JSON.stringify({ token, model })) + const db = await dbPromise + if (db !== undefined) { + const transaction = db.transaction('model', 'readwrite') // (1) + const models = transaction.objectStore('model') // (2) + models.put({ id: token, model }) } } } diff --git a/plugins/tracker-resources/src/components/CreateIssue.svelte b/plugins/tracker-resources/src/components/CreateIssue.svelte index 7a0089ce0a..b289897731 100644 --- a/plugins/tracker-resources/src/components/CreateIssue.svelte +++ b/plugins/tracker-resources/src/components/CreateIssue.svelte @@ -442,11 +442,11 @@ issueUrl: currentProject != null && generateIssueShortLink(getIssueId(currentProject, value as Issue)) } ) - console.log('createIssue measure', await doneOp()) draftController.remove() descriptionBox?.removeDraft(false) isAssigneeTouched = false + console.log('createIssue measure', doneOp()) } catch (err: any) { console.error(err) await doneOp() // Complete in case of error diff --git a/server/core/src/adapter.ts b/server/core/src/adapter.ts index 9b1301c5cc..6421e89233 100644 --- a/server/core/src/adapter.ts +++ b/server/core/src/adapter.ts @@ -49,7 +49,9 @@ export interface DbAdapter { findAll: ( _class: Ref>, query: DocumentQuery, - options?: FindOptions + options?: FindOptions & { + domain?: Domain // Allow to find for Doc's in specified domain only. + } ) => Promise> tx: (...tx: Tx[]) => Promise diff --git a/server/core/src/storage.ts b/server/core/src/storage.ts index 9e6e94ff81..095e543fcb 100644 --- a/server/core/src/storage.ts +++ b/server/core/src/storage.ts @@ -372,9 +372,11 @@ class TServerStorage implements ServerStorage { ctx: MeasureContext, clazz: Ref>, query: DocumentQuery, - options?: FindOptions + options?: FindOptions & { + domain?: Domain // Allow to find for Doc's in specified domain only. + } ): Promise> { - const domain = this.hierarchy.getDomain(clazz) + const domain = options?.domain ?? this.hierarchy.getDomain(clazz) if (query?.$search !== undefined) { return await ctx.with('client-fulltext-find-all', {}, (ctx) => this.fulltext.findAll(ctx, clazz, query, options)) } diff --git a/server/middleware/src/spaceSecurity.ts b/server/middleware/src/spaceSecurity.ts index 269c9c3331..1e3b65e941 100644 --- a/server/middleware/src/spaceSecurity.ts +++ b/server/middleware/src/spaceSecurity.ts @@ -18,6 +18,7 @@ import core, { Class, Doc, DocumentQuery, + Domain, FindOptions, FindResult, LookupData, @@ -47,14 +48,19 @@ import { BroadcastFunc, Middleware, SessionContext, TxMiddlewareResult } from '@ import { BaseMiddleware } from './base' import { getUser, isOwner, isSystem, mergeTargets } from './utils' +type SpaceWithMembers = Pick + /** * @public */ export class SpaceSecurityMiddleware extends BaseMiddleware implements Middleware { private allowedSpaces: Record, Ref[]> = {} - private privateSpaces: Record, Space | undefined> = {} - private domainSpaces: Record>> = {} + private privateSpaces: Record, SpaceWithMembers | undefined> = {} + private readonly _domainSpaces = new Map> | Promise>>>() private publicSpaces: Ref[] = [] + + private spaceMeasureCtx!: MeasureContext + private readonly systemSpaces = [ core.space.Configuration, core.space.DerivedTx, @@ -78,9 +84,8 @@ export class SpaceSecurityMiddleware extends BaseMiddleware implements Middlewar next?: Middleware ): Promise { const res = new SpaceSecurityMiddleware(broadcast, storage, next) - await ctx.with('space chain', {}, async (ctx) => { - await res.init(ctx) - }) + res.spaceMeasureCtx = ctx.newChild('space chain', {}) + await res.init(res.spaceMeasureCtx) return res } @@ -90,7 +95,7 @@ export class SpaceSecurityMiddleware extends BaseMiddleware implements Middlewar this.allowedSpaces[member] = arr } - private addSpace (space: Space): void { + private addSpace (space: SpaceWithMembers): void { this.privateSpaces[space._id] = space for (const member of space.members) { this.addMemberSpace(member, space._id) @@ -98,49 +103,24 @@ export class SpaceSecurityMiddleware extends BaseMiddleware implements Middlewar } private async init (ctx: MeasureContext): Promise { - const spaces = await this.storage.findAll(ctx, core.class.Space, { private: true }) - for (const space of spaces) { - this.addSpace(space) - } - this.publicSpaces = (await this.storage.findAll(ctx, core.class.Space, { private: false })).map((p) => p._id) - await this.initDomains(ctx) - } - - private async initDomains (ctx: MeasureContext): Promise { - const classesPerDomain: Record>[]> = {} - const classes = this.storage.hierarchy.getDescendants(core.class.Doc) - for (const _class of classes) { - const clazz = this.storage.hierarchy.getClass(_class) - if (clazz.domain === undefined) continue - const domain = clazz.domain - classesPerDomain[domain] = classesPerDomain[domain] ?? [] - classesPerDomain[domain].push(_class) - } - for (const domain in classesPerDomain) { - for (const _class of classesPerDomain[domain]) { - const field = this.getKey(_class) - const map = this.domainSpaces[domain] ?? new Set() - this.domainSpaces[domain] = map - - while (true) { - const spaces = await this.storage.findAll( - ctx, - _class, - { - [field]: { $nin: Array.from(map.values()) } - }, - { - projection: { [field]: 1 }, - limit: 1000 - } - ) - if (spaces.length === 0) { - break - } - spaces.forEach((p) => map.add((p as any)[field] as Ref)) + const spaces: SpaceWithMembers[] = await this.storage.findAll( + ctx, + core.class.Space, + {}, + { + projection: { + private: 1, + _id: 1, + members: 1 } } + ) + for (const space of spaces) { + if (space.private) { + this.addSpace(space) + } } + this.publicSpaces = spaces.filter((it) => !it.private).map((p) => p._id) } private removeMemberSpace (member: Ref, space: Ref): void { @@ -210,7 +190,7 @@ export class SpaceSecurityMiddleware extends BaseMiddleware implements Middlewar } } - private async syncMembers (members: Ref[], space: Space): Promise { + private async syncMembers (members: Ref[], space: SpaceWithMembers): Promise { const oldMembers = new Set(space.members) const newMembers = new Set(members) const changed: Ref[] = [] @@ -253,7 +233,7 @@ export class SpaceSecurityMiddleware extends BaseMiddleware implements Middlewar this.broadcast([tx], targets) } - private async broadcastNonMembers (space: Space | undefined): Promise { + private async broadcastNonMembers (space: SpaceWithMembers | undefined): Promise { const users = await this.storage.modelDb.findAll(core.class.Account, { _id: { $nin: space?.members } }) await this.brodcastEvent(users.map((p) => p._id)) } @@ -291,7 +271,7 @@ export class SpaceSecurityMiddleware extends BaseMiddleware implements Middlewar if (updateDoc.operations.$pull?.members !== undefined) { await this.pullMembersHandle(updateDoc.operations.$pull.members, space._id) } - space = TxProcessor.updateDoc2Doc(space, updateDoc) + space = TxProcessor.updateDoc2Doc(space as any, updateDoc) } } @@ -349,7 +329,7 @@ export class SpaceSecurityMiddleware extends BaseMiddleware implements Middlewar return targets } - private processTxSpaceDomain (tx: TxCUD): void { + private async processTxSpaceDomain (tx: TxCUD): Promise { const actualTx = TxProcessor.extractTx(tx) if (actualTx._class === core.class.TxCreateDoc) { const ctx = actualTx as TxCreateDoc @@ -358,21 +338,19 @@ export class SpaceSecurityMiddleware extends BaseMiddleware implements Middlewar const space = (doc as any)[key] if (space === undefined) return const domain = this.storage.hierarchy.getDomain(ctx.objectClass) - this.domainSpaces[domain] = this.domainSpaces[domain] ?? new Set() - this.domainSpaces[domain].add(space) + ;(await this.getDomainSpaces(domain)).add(space) } else if (actualTx._class === core.class.TxUpdateDoc) { const updTx = actualTx as TxUpdateDoc const key = this.getKey(updTx.objectClass) const space = (updTx.operations as any)[key] if (space !== undefined) { const domain = this.storage.hierarchy.getDomain(updTx.objectClass) - this.domainSpaces[domain] = this.domainSpaces[domain] ?? new Set() - this.domainSpaces[domain].add(space) + ;(await this.getDomainSpaces(domain)).add(space) } } } - private async proccessTx (ctx: SessionContext, tx: Tx): Promise { + private async processTx (ctx: SessionContext, tx: Tx): Promise { const h = this.storage.hierarchy if (h.isDerived(tx._class, core.class.TxCUD)) { const cudTx = tx as TxCUD @@ -380,7 +358,7 @@ export class SpaceSecurityMiddleware extends BaseMiddleware implements Middlewar if (isSpace) { await this.handleTx(ctx, cudTx as TxCUD) } - this.processTxSpaceDomain(tx as TxCUD) + await this.processTxSpaceDomain(tx as TxCUD) if (h.isDerived(cudTx.objectClass, core.class.Account) && cudTx._class === core.class.TxUpdateDoc) { const ctx = cudTx as TxUpdateDoc if (ctx.operations.role !== undefined) { @@ -391,21 +369,24 @@ export class SpaceSecurityMiddleware extends BaseMiddleware implements Middlewar } async tx (ctx: SessionContext, tx: Tx): Promise { - await this.proccessTx(ctx, tx) + await this.processTx(ctx, tx) const targets = await this.getTxTargets(ctx, tx) const res = await this.provideTx(ctx, tx) for (const tx of res[1]) { - await this.proccessTx(ctx, tx) + await this.processTx(ctx, tx) } return [res[0], res[1], mergeTargets(targets, res[2])] } handleBroadcast (tx: Tx[], targets?: string[]): Tx[] { - for (const t of tx) { - if (this.storage.hierarchy.isDerived(t._class, core.class.TxCUD)) { - this.processTxSpaceDomain(t as TxCUD) + const process = async (): Promise => { + for (const t of tx) { + if (this.storage.hierarchy.isDerived(t._class, core.class.TxCUD)) { + await this.processTxSpaceDomain(t as TxCUD) + } } } + void process() return this.provideHandleBroadcast(tx, targets) } @@ -419,18 +400,52 @@ export class SpaceSecurityMiddleware extends BaseMiddleware implements Middlewar } } - private filterByDomain (domain: string, spaces: Ref[]): Ref[] { - const domainSpaces = this.domainSpaces[domain] - if (domainSpaces === undefined) return [] + async loadDomainSpaces (ctx: MeasureContext, domain: Domain): Promise>> { + const map = new Set>() + const field = this.getKey(domain) + while (true) { + const spaces = await this.storage.findAll( + ctx, + core.class.Doc, + { + [field]: { $nin: Array.from(map.values()) } + }, + { + projection: { [field]: 1 }, + limit: 1000, + domain + } + ) + if (spaces.length === 0) { + break + } + spaces.forEach((p) => map.add((p as any)[field] as Ref)) + } + return map + } + + async getDomainSpaces (domain: Domain): Promise>> { + let domainSpaces = this._domainSpaces.get(domain) + if (domainSpaces === undefined) { + const p = this.loadDomainSpaces(this.spaceMeasureCtx, domain) + this._domainSpaces.set(domain, p) + domainSpaces = await p + this._domainSpaces.set(domain, domainSpaces) + } + return domainSpaces instanceof Promise ? await domainSpaces : domainSpaces + } + + private async filterByDomain (domain: Domain, spaces: Ref[]): Promise[]> { + const domainSpaces = await this.getDomainSpaces(domain) return spaces.filter((p) => domainSpaces.has(p)) } - private mergeQuery( + private async mergeQuery( account: Account, query: ObjQueryType, - domain: string - ): ObjQueryType { - const spaces = this.filterByDomain(domain, this.getAllAllowedSpaces(account)) + domain: Domain + ): Promise> { + const spaces = await this.filterByDomain(domain, this.getAllAllowedSpaces(account)) if (query == null) { return { $in: spaces } } @@ -446,12 +461,8 @@ export class SpaceSecurityMiddleware extends BaseMiddleware implements Middlewar return query } - private getKey(_class: Ref>): string { - return this.storage.hierarchy.isDerived(_class, core.class.Tx) - ? 'objectSpace' - : this.storage.hierarchy.isDerived(_class, core.class.Space) - ? '_id' - : 'space' + private getKey (domain: string): string { + return domain === 'tx' ? 'objectSpace' : domain === 'space' ? '_id' : 'space' } override async findAll( @@ -468,9 +479,9 @@ export class SpaceSecurityMiddleware extends BaseMiddleware implements Middlewar if (!isSystem(account)) { if (!isOwner(account) || !this.storage.hierarchy.isDerived(_class, core.class.Space)) { if (query[field] !== undefined) { - ;(newQuery as any)[field] = this.mergeQuery(account, query[field], domain) + ;(newQuery as any)[field] = await this.mergeQuery(account, query[field], domain) } else { - const spaces = this.filterByDomain(domain, this.getAllAllowedSpaces(account)) + const spaces = await this.filterByDomain(domain, this.getAllAllowedSpaces(account)) ;(newQuery as any)[field] = { $in: spaces } } } diff --git a/server/mongo/src/storage.ts b/server/mongo/src/storage.ts index e1f3b22358..cc06cafc56 100644 --- a/server/mongo/src/storage.ts +++ b/server/mongo/src/storage.ts @@ -168,34 +168,41 @@ abstract class MongoAdapterBase implements DbAdapter { translated[tkey] = value } const baseClass = this.hierarchy.getBaseClass(clazz) - const classes = this.hierarchy.getDescendants(baseClass) + if (baseClass !== core.class.Doc) { + const classes = this.hierarchy.getDescendants(baseClass) - // Only replace if not specified - if (translated._class === undefined) { - translated._class = { $in: classes } - } else if (typeof translated._class === 'string') { - if (!classes.includes(translated._class)) { + // Only replace if not specified + if (translated._class === undefined) { translated._class = { $in: classes } - } - } else if (typeof translated._class === 'object' && translated._class !== null) { - let descendants: Ref>[] = classes + } else if (typeof translated._class === 'string') { + if (!classes.includes(translated._class)) { + translated._class = { $in: classes } + } + } else if (typeof translated._class === 'object' && translated._class !== null) { + let descendants: Ref>[] = classes - if (Array.isArray(translated._class.$in)) { - const classesIds = new Set(classes) - descendants = translated._class.$in.filter((c: Ref>) => classesIds.has(c)) + if (Array.isArray(translated._class.$in)) { + const classesIds = new Set(classes) + descendants = translated._class.$in.filter((c: Ref>) => classesIds.has(c)) + } + + if (translated._class != null && Array.isArray(translated._class.$nin)) { + const excludedClassesIds = new Set>>(translated._class.$nin) + descendants = descendants.filter((c) => !excludedClassesIds.has(c)) + } + + translated._class = { $in: descendants } } - if (translated._class != null && Array.isArray(translated._class.$nin)) { - const excludedClassesIds = new Set>>(translated._class.$nin) - descendants = descendants.filter((c) => !excludedClassesIds.has(c)) + if (baseClass !== clazz) { + // Add an mixin to be exists flag + translated[clazz] = { $exists: true } + } + } else { + // No need to pass _class in case of fixed domain search. + if ('_class' in translated) { + delete translated._class } - - translated._class = { $in: descendants } - } - - if (baseClass !== clazz) { - // Add an mixin to be exists flag - translated[clazz] = { $exists: true } } return translated } @@ -416,7 +423,9 @@ abstract class MongoAdapterBase implements DbAdapter { private async findWithPipeline( clazz: Ref>, query: DocumentQuery, - options?: FindOptions + options?: FindOptions & { + domain?: Domain // Allow to find for Doc's in specified domain only. + } ): Promise> { const pipeline = [] const match = { $match: this.translateQuery(clazz, query) } @@ -454,7 +463,8 @@ abstract class MongoAdapterBase implements DbAdapter { ...(options?.total === true ? { totalCount: [{ $count: 'count' }] } : {}) } }) - const domain = this.hierarchy.getDomain(clazz) + // const domain = this.hierarchy.getDomain(clazz) + const domain = options?.domain ?? this.hierarchy.getDomain(clazz) const cursor = this.db.collection(domain).aggregate(pipeline, { checkKeys: false, enableUtf8Validation: false @@ -562,13 +572,15 @@ abstract class MongoAdapterBase implements DbAdapter { async findAll( _class: Ref>, query: DocumentQuery, - options?: FindOptions + options?: FindOptions & { + domain?: Domain // Allow to find for Doc's in specified domain only. + } ): Promise> { // TODO: rework this if (options != null && (options?.lookup != null || this.isEnumSort(_class, options) || this.isRulesSort(options))) { return await this.findWithPipeline(_class, query, options) } - const domain = this.hierarchy.getDomain(_class) + const domain = options?.domain ?? this.hierarchy.getDomain(_class) const coll = this.db.collection(domain) const mongoQuery = this.translateQuery(_class, query) let cursor = coll.find(mongoQuery, { @@ -719,6 +731,9 @@ abstract class MongoAdapterBase implements DbAdapter { } async load (domain: Domain, docs: Ref[]): Promise { + if (docs.length === 0) { + return [] + } const cursor = this.db.collection(domain).find({ _id: { $in: docs } }) const result = await this.toArray(cursor) return this.stripHash(this.stripHash(result))